search

rust-lang / cargo

The Rust package manager
https://doc.rust-lang.org/cargo
Apache License 2.0
12.69k stars 2.41k forks source link

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

Closed behnam closed 3 years ago

behnam commented 7 years ago

https://github.com/rust-lang/cargo/blob/36ddeff03a31f710095cfbe49287503bb60e294d/src/cargo/util/vcs.rs#L57-L63

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 commented 7 years ago

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 commented 7 years ago

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: https://github.com/rust-lang/cargo/blob/36ddeff03a31f710095cfbe49287503bb60e294d/src/cargo/ops/cargo_new.rs#L395-L404

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: https://github.com/rust-lang/cargo/blob/36ddeff03a31f710095cfbe49287503bb60e294d/src/cargo/util/vcs.rs#L57-L63

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.

PaulDance commented 3 years ago

@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!