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.

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

[vcs] fossil ignore settings is different from git/hg #4482

Closed
behnam opened this issue Sep 11, 2017 · 3 comments · Fixed by #9469 or rust-lang/rust#85206
Closed

[vcs] fossil ignore settings is different from git/hg #4482

behnam opened this issue Sep 11, 2017 · 3 comments · Fixed by #9469 or rust-lang/rust#85206
Labels
A-vcs Area: general VCS issues C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted`

Comments

@behnam
Copy link
Contributor

behnam commented Sep 11, 2017

// set `target` as ignoreable and cleanable
process("fossil").cwd(cwd).arg("settings")
.arg("ignore-glob")
.arg("target");
process("fossil").cwd(cwd).arg("settings")
.arg("clean-glob")
.arg("target");

Having the ignore setting set in VSC's init() method rather than cargo_new() (https://github.com/rust-lang/cargo/blob/master/src/cargo/ops/cargo_new.rs#L391) results in a different behavior: for git/hg, the ignore setting is eventually set for the directory, no matter if the VSC is new or not; for fossil, it's only set if the repo is new.

Also, the ignore setting for fossil is not synced with git/hg, which is kind of the result of the code living in separate areas.

@ketralnis, maybe we make the fossil config follow the git/hg pattern? Or if that doesn't work for this VSC, maybe update the code to sync with git/hg and add a comment on both sides to keep them in sync?

@ketralnis
Copy link
Contributor

ketralnis commented Sep 11, 2017

I'm not an expert on either so no argument from me, but I don't know what you mean by "sync with git/hg". Are you imagining using the ignore files as a way to keep the same directory in separate git and fossil repos? For that use case you're probably better off using fossil's export tooling

@behnam
Copy link
Contributor Author

behnam commented Sep 11, 2017

Oh, let me clarify: I was referring to the fact that we want, for any VSC in use, the same set of files to be ignored.

Here's how it's implemented for git and hg:

// Please ensure that ignore and hgignore are in sync.
let ignore = ["/target/\n", "**/*.rs.bk\n",
if !opts.bin { "Cargo.lock\n" } else { "" }]
.concat();
// Mercurial glob ignores can't be rooted, so just sticking a 'syntax: glob' at the top of the
// file will exclude too much. Instead, use regexp-based ignores. See 'hg help ignore' for
// more.
let hgignore = ["^target/\n", "glob:*.rs.bk\n",
if !opts.bin { "glob:Cargo.lock\n" } else { "" }]
.concat();

As you can see here, git and hg ignore files have three rules:

  1. a directory on the package root called target,
  2. any file in the repo with .rs.bk suffix, and
  3. if it's a library, the Cargo.lock file.

As noted there in the code, the VSC-specific logic for git and hg is kept in sync. We want to have the same for all other VSCs, including fossil.

At the moment, fossil only has the first rule here (possibly, partially). Here's the current implementation:

// set `target` as ignoreable and cleanable
process("fossil").cwd(cwd).arg("settings")
.arg("ignore-glob")
.arg("target");
process("fossil").cwd(cwd).arg("settings")
.arg("clean-glob")
.arg("target");

And now that we're here, I should note that the Pijul support actually misses all ignore logic. I don't have any experience with Pijul, so not sure how it's usually handled there. But, IMHO, something worth keep track of.

@carols10cents carols10cents added A-vcs Area: general VCS issues C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` labels Oct 3, 2017
@bors bors closed this as completed in 5c45513 May 10, 2021
@PaulDance
Copy link
Contributor

@behnam #9469 changed this part of the code, hopefully for the best. If you are still using Fossil and have a bit of time to try this out when it lands on nightly, I would love to hear some feedback!

bors added a commit to rust-lang-ci/rust that referenced this issue May 12, 2021
Update cargo

8 commits in e51522ab3db23b0d8f1de54eb1f0113924896331..070e459c2d8b79c5b2ac5218064e7603329c92ae
2021-05-07 21:29:52 +0000 to 2021-05-11 18:12:23 +0000
- Fix rustdoc warnings (rust-lang/cargo#9468)
- Improve performance of git status check in `cargo package`. (rust-lang/cargo#9478)
- Link to the new rustc tests chapter. (rust-lang/cargo#9477)
- Bump index cache version to deal with semver metadata version mismatch. (rust-lang/cargo#9476)
- Fix Url::into_string deprecation warning (rust-lang/cargo#9475)
- Fix rust-lang/cargo#4482 and rust-lang/cargo#9449: set Fossil ignore and clean settings locally (rust-lang/cargo#9469)
- Improve two error messages (rust-lang/cargo#9472)
- Fix `cargo install` with a semver metadata version. (rust-lang/cargo#9467)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-vcs Area: general VCS issues C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted`
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants