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

Add glob support for extra file permissions #2345

Merged
merged 7 commits into from
Mar 24, 2020
Merged

Conversation

TadCordle
Copy link
Contributor

@TadCordle TadCordle commented Mar 19, 2020

Fixes #1200.

Can now use glob patterns when setting extra files permissions. Permissions configured with explicit paths will take priority over globs (e.g. if both '/a/b/file': '777' and '**/file': '444' are configured, /a/b/file will have 777 permissions, and all other files named file will have 444 permissions).

If a file matches multiple globs (e.g. if there's a file /a/b/file, and permissions are configured for both /a/b/* and **/file), behavior is undefined at the moment first configured match wins.

@TadCordle TadCordle requested a review from a team March 19, 2020 17:22
@loosebazooka
Copy link
Member

However, if a file matches multiple globs (e.g. if there's a file /a/b/file, and permissions are configured for both /a/b/* and **/file), behavior is undefined at the moment. Maybe we should sort the entry set or something when searching for matches so the behavior is more predictable?

It should be deterministic, like last configured pattern wins.

@TadCordle
Copy link
Contributor Author

Actually, looks like Groovy maps are LinkedHashMaps, so behavior should already be predictable (first match wins). But if you think last match wins is better UX we can do that.

@loosebazooka
Copy link
Member

Oh actually I don't know, it was just a suggestion. I was thinking we apply all rules we find to a file, but it seems like we go looking for matches and apply the first one we find -- which actually makes sense.

Should the pattern only match files? Can we accidentally end up making directories un-traversable?

@TadCordle
Copy link
Contributor Author

TadCordle commented Mar 19, 2020

We still traverse over every file/directory, but some glob patterns could end up being completely skipped. e.g. this will set everything to 777:

extraDirectories {
  permissions = [
    '/**': '777',
    '/a/**': '444'
  ]
}

But I think you can avoid this by just configuring the more specific patterns first. And again, if you configure permissions on files/directories without using a glob, those will override any permissions set using globs.

@loosebazooka
Copy link
Member

And again, if you configure permissions on files/directories without using a glob, those will override any permissions set using globs.

so order doesn't matter for those?

@TadCordle
Copy link
Contributor Author

Correct.

@TadCordle
Copy link
Contributor Author

Actually, since we take the map from the gradle config and put it in a new map, we might not be maintaining order... will fix.

@TadCordle TadCordle merged commit a02a88f into master Mar 24, 2020
@TadCordle TadCordle deleted the i1200-permission-globs branch March 24, 2020 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support globs in file permission configuration
4 participants