[sources/path] Support leading slash in glob patterns #4378
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Background: rust-lang#4268 This diff takes us to **Stage 1.1** of the migration plan by allowing glob patterns to include a leading slash, so that glob patterns can be updated, if needed, to start with a slash, closer to the future behavior with gitignore-like matching. Why is this stage needed? It's common to have `package.include` set like this: ``` include = ["src/**"] ``` In old interpretation, this would only include all files under the `src` directory under the package root. With the new interpretation, this would match any path with some directory called `src`, even if it's not directly under the package root. After this patch, package owners can start marking glob patters with a leading slash to fix the warning thrown, if any. One thing to notice here is that there are no extra matchings, but, if a manifest has already a pattern with a leading slash, this would silently start matching it with the paths. I believe this is fine, since the old behavior would have been for the pattern to not match anything, therefore the item was useless. See also <rust-lang#4377> for suggestion to throw warning on useless/invalid patterns in these fields.
|
(rust_highfive has picked a reviewer for you, use r? to override) |
|
📌 Commit 5641cbd has been approved by |
bors
added a commit
that referenced
this pull request
Aug 8, 2017
[sources/path] Support leading slash in glob patterns Background: #4268 This diff takes us to **Stage 1.1** of the migration plan by allowing glob patterns to include a leading slash, so that glob patterns can be updated, if needed, to start with a slash, closer to the future behavior with gitignore-like matching. Why is this stage needed? It's common to have `package.include` set like this: ``` include = ["src/**"] ``` In old interpretation, this would only include all files under the `src` directory under the package root. With the new interpretation, this would match any path with some directory called `src`, even if it's not directly under the package root. After this patch, package owners can start marking glob patters with a leading slash to fix the warning thrown, if any. One thing to notice here is that there are no extra matchings, but, if a manifest has already a pattern with a leading slash, this would silently start matching it with the paths. I believe this is fine, since the old behavior would have been for the pattern to not match anything, therefore the item was useless. See also <#4377> for suggestion to throw warning on useless/invalid patterns in these fields.
|
@bors: r=alexcrichton |
|
@behnam: 🔑 Insufficient privileges: Not in reviewers |
|
Sure, @alexcrichton! BTW, looks like deleting my remote branch from CLI before it lands on master has bors/homu to stop the land process and needs another r+. Would you please stamp again? |
|
💥 Test timed out |
|
Also, @alexcrichton, I think this should be backported to |
|
@bors: r+ |
|
💡 This pull request was already approved, no need to approve it again.
|
|
📌 Commit 5641cbd has been approved by |
bors
added a commit
that referenced
this pull request
Aug 9, 2017
[sources/path] Support leading slash in glob patterns Background: #4268 This diff takes us to **Stage 1.1** of the migration plan by allowing glob patterns to include a leading slash, so that glob patterns can be updated, if needed, to start with a slash, closer to the future behavior with gitignore-like matching. Why is this stage needed? It's common to have `package.include` set like this: ``` include = ["src/**"] ``` In old interpretation, this would only include all files under the `src` directory under the package root. With the new interpretation, this would match any path with some directory called `src`, even if it's not directly under the package root. After this patch, package owners can start marking glob patters with a leading slash to fix the warning thrown, if any. One thing to notice here is that there are no extra matchings, but, if a manifest has already a pattern with a leading slash, this would silently start matching it with the paths. I believe this is fine, since the old behavior would have been for the pattern to not match anything, therefore the item was useless. See also <#4377> for suggestion to throw warning on useless/invalid patterns in these fields.
|
☀️ Test successful - status-appveyor, status-travis |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Background: #4268
This diff takes us to Stage 1.1 of the migration plan by allowing
glob patterns to include a leading slash, so that glob patterns can be
updated, if needed, to start with a slash, closer to the future behavior
with gitignore-like matching.
Why is this stage needed?
It's common to have
package.includeset like this:In old interpretation, this would only include all files under the
srcdirectory under the package root. With the new interpretation, this
would match any path with some directory called
src, even if it's notdirectly under the package root.
After this patch, package owners can start marking glob patters with a
leading slash to fix the warning thrown, if any.
One thing to notice here is that there are no extra matchings, but, if
a manifest has already a pattern with a leading slash, this would
silently start matching it with the paths. I believe this is fine, since
the old behavior would have been for the pattern to not match anything,
therefore the item was useless.
See also #4377 for suggestion
to throw warning on useless/invalid patterns in these fields.