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

excluding a folder should exclude all it's content #3578

Closed
grosser opened this issue Jan 23, 2017 · 7 comments
Closed

excluding a folder should exclude all it's content #3578

grosser opened this issue Jan 23, 2017 · 7 comments

Comments

@grosser
Copy link

grosser commented Jan 23, 2017

for example here the complete content of the foo folder should be excluded ... but it needs to be foo/* atm ... which means I always shipped a ton of useless stuff ... so please make folders exclude the whole folder + content

exclude = [
    "foo"
]
@froydnj
Copy link
Contributor

froydnj commented Jul 6, 2017

We ran into the same issue in servo/rust-cssparser#169.

behnam added a commit to behnam/rust-unicode-bidi that referenced this issue Jul 6, 2017
* We need to use `*` or `**` in the excludes, otherwise they don't work.
<rust-lang/cargo#3578> is tracking the issue.
@behnam
Copy link
Contributor

behnam commented Jul 6, 2017

And we just fell into this trap too!

Here's the status quo:

These work:

  • "dir/*"
  • "dir/**"

These DON'T work:

  • "dir/"
  • "dir"

@behnam
Copy link
Contributor

behnam commented Jul 6, 2017

https://github.com/rust-lang/cargo/blame/master/src/cargo/sources/path.rs#L113

(CC'ing from blame: @alexcrichton, @mbrubeck, @llogiq)

Is the status quo the desired behavior? If so, I can add a note to the docs to prevent people from facing the same issue.

If not, I suppose we should replace Pattern::matches_path() calls with something that also covers no-asterisks prefix matchings.

What do you think?

@alexcrichton
Copy link
Member

Sounds reasonable to me to add support for this! To me seems like the existing behavior is a bug.

@behnam
Copy link
Contributor

behnam commented Jul 7, 2017

Okay, so I'll give this a shot soon.

behnam added a commit to behnam/rust-cargo that referenced this issue Jul 7, 2017
NOTE: This is a major change in pattern matching for `include` and
`exclude` fields, and can result in additional inclusion/exclusion for
some patterns.

Previously, for inclusion/exclusion matters, Cargo only works with paths
of files in a package/repository, and glob pattern matching has been
applying only to these file paths.

The old behavior results in some unexpected behavior. For example,
having:

```toml
exclude = ["data"]
```

in a manifest next to a `data` directory, it will not exclude the
directory. To make it work, a pattern must be provided that matches the
*files* under this directory, like:

```toml
exclude = ["data/*"]
```

To make Cargo's inclusion/exclusion behavior more intutional, and bring
it on par with similar systems, like `gitignore`, we need to also match
these patterns with the *directories*. The directories are seen
internally as *parents* of the files. Therefore, this diff expands the
pattern matching to files and their parent directories.

Now, it's easier to exclude all data files:

```toml
exclude = ["data"]
```

or include only the `src` directory:

```toml
include = ["src"]
```

Fixes <rust-lang#3578>
@behnam
Copy link
Contributor

behnam commented Aug 11, 2017

I believe we can now close this issue in favor of #4268, which is implementing the feature and tracking the migration.

@alexcrichton
Copy link
Member

👍
Thanks @behnam!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants