Handle overlapping members and exclude keys

[alexcrichton]

The longest key takes precedence if there's more than one that matches.

Closes #4089

[rust-highfive]

r? @matklad

(rust_highfive has picked a reviewer for you, use r? to override)

[matklad]

Hm, we support globs in members, don't we? And looks like this is_excluded function does not handle this at all?

[matklad]

I think this'll look a tiny bit neater if we add .max_by_key(|p| p.as_os_str().len()) bit here, and then do match (explicit_member, exclude) { ... } simultaneously.

[alexcrichton]

Excellent point about globs! I fixed using exclude and globs by using Gitignore, although the glob crate is still used to expand members directives, so I added a comment on the issue to switch to gitignore.

[matklad]

Let's add a test with globs! :)

I think the scenario which does not work with old is_excluded is this:

Workspace root includes a member foo via glob. Then, if we execute cargo build inside foo, cargo won't detect that it is a member of the workspace.

[matklad]

I think that members can contain globs. That is, p might be like foo/* here, and so this starts_with will fail.

[matklad]

I think the scenario which does not work with old is_excluded is this:

I though wrong :) Nevertheless, I think this test will be interesting:

~/trash/ws master*
λ tree .
.
├── Cargo.toml
└── foo
    └── baz
        └── bar
            ├── Cargo.toml
            └── src
                └── lib.rs

4 directories, 3 files

~/trash/ws master*
λ cat Cargo.toml
[workspace]
members = ["foo/baz/*"]
exclude = ["foo/"]

Looks like foo/bar/baz should be a member of the workspace.

[behnam]

@alexcrichton, should I take over this one and fix #4089 along the migration to gitignore-matching everywhere?

[behnam]

Also, another difference between workspace.member/exclude vs package.include/exclude is that the package configs are exclusive (if include is set, exclude is ignored silently), but in workspace, turns out both configs can exist at the same time.

I think whatever solution we take here better be aligned with the future of package configs, since I'm hopping we can make those non-exclusive, too.

Here's my suggestion for workspace configs (which we can implement earlier and later expand it to package configs):

• Treat member and exclude configs as rows of one gitignore, with list in member being added with a ! prefix.

• Error on negated patterns (patterns starting with !) in any of the configs. (Only allow escaped, literal exclamation mark: \!.)

What do you think?

[alexcrichton]

Sorry yeah @behnam if you wouldn't mind taking this over, that'd be great! I'm quickly losing steam on this :(

I'm also sorry that there's not really a great design here, it's all sort of ad-hoc. My main motivation for this was to remove these lines in the rust-lang/rust repo. Beyond that I've just been attempting to rationalize gitignore/glob pattern, but I get the feeling that it's just becoming more of a mess.

[behnam]

No worries, @alexcrichton. I can get back to this when I get back from the trip next week.

[alexcrichton]

ping @behnam, was just curious if you had a chance to take a look at this again! If you're busy then of course no worries!

[bors]

☔ The latest upstream changes (presumably #4469) made this pull request unmergeable. Please resolve the merge conflicts.

[alexcrichton]

This never quite panned out, so closing.

[bors]

☔ The latest upstream changes (presumably #5907) made this pull request unmergeable. Please resolve the merge conflicts.