Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Setup should not use an absolute path to set the migrations_directory 'dir' field of diesel.toml #4436

Open
3 tasks done
the-solid-box opened this issue Jan 17, 2025 · 2 comments · May be fixed by #4439
Open
3 tasks done

Setup should not use an absolute path to set the migrations_directory 'dir' field of diesel.toml #4436

the-solid-box opened this issue Jan 17, 2025 · 2 comments · May be fixed by #4439
Labels
bug good first issue help wanted

Comments

@the-solid-box
Copy link

the-solid-box commented Jan 17, 2025 •
edited
Loading

Setup

Versions

  • Rust: 1.83.0
  • Diesel: 2.2.6
  • Database: PostgreSQL 17.0

Problem Description

The diesel setup command sets the migrations_directory dir field of the diesel.toml file to an absolute path, which both leaks personal information (at least the username of the user running the setup command) and breaks it when the local repository has a different path (for instance, when cloned by another team member).

Since it seems to be working perfectly fine with a relative path, I think it would be recommendable to have diesel setup set the migrations directory using a path relative to the root of the crate.

Checklist

  • This issue can be reproduced on Rust's stable channel. (Your issue will be
    closed if this is not the case)
  • This issue can be reproduced without requiring a third party crate
@weiznich
Copy link
Member

Thanks for reporting. I agree that this is something that should be fixed.

The relevant code for that is here:

diesel/diesel_cli/src/main.rs

Line 149 in 25cff1d

let migrations_dir_toml_string = migrations_dir.display().to_string().replace('\\', "\\\\");

We would appreciate a PR that fixes this issue + adds a test to verify the fix.

@the-solid-box
Copy link
Author

Thanks for the quick reply. I'll look into it as soon as I can.

@the-solid-box the-solid-box linked a pull request Jan 17, 2025 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug good first issue help wanted
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: setup now sets migrations dir as relative path when possible the-solid-box/diesel
2 participants
@weiznich @the-solid-box