Add glob option to ignore hidden files #1791
Conversation
| /** | ||
| * Indicates whether to include hidden files (files and directories starting with a `.`). | ||
| * | ||
| * @default false |
There was a problem hiding this comment.
Don't we want the default behavior to be to exclude?
There was a problem hiding this comment.
We want the default to be exclude for the action that consumes this library, I don't think we need or should make a breaking change in this library.
There was a problem hiding this comment.
Makes sense for changes to the glob package to be backward compatible but just curious, what other actions use this package? Are there any actions other than upload-artifact where we would want to pass true?
There was a problem hiding this comment.
nit: the comment should read "Indicates whether to exclude hidden files.."
There was a problem hiding this comment.
Are there any actions other than upload-artifact where we would want to pass true?
I see actions/deploy-pages, actions/upload-artifact, and actions/download-artifact.
| @@ -128,6 +128,11 @@ export class DefaultGlobber implements Globber { | |||
| continue | |||
| } | |||
|
|
|||
| // Hidden file or directory? | |||
There was a problem hiding this comment.
What upload-artifact version(s) is this code path exercised in?
There was a problem hiding this comment.
Glob is utilized here https://github.com/actions/upload-artifact/blob/main/src/shared/search.ts
| @@ -1,6 +1,6 @@ | |||
| { | |||
| "name": "@actions/glob", | |||
| "version": "0.4.0", | |||
There was a problem hiding this comment.
Can you update the releases file if you are going to bump in this pr as well
https://github.com/actions/toolkit/blob/main/packages/glob/RELEASES.md
There was a problem hiding this comment.
No other changes outside of this, some test fixes, and package bumps for security reasons
https://github.com/actions/toolkit/commits/main/packages/glob
| await createHiddenDirectory(path.join(root, '.emptyFolder')) | ||
| await createHiddenDirectory(path.join(root, '.folder')) | ||
| await createHiddenFile(path.join(root, '.file'), 'test .file content') | ||
| await fs.writeFile( |
There was a problem hiding this comment.
I'm a little confused about the behaviour here.
I'm not sure if the behaviour for the following globs matches what's intended
ignores-hidden-files/.folder- returns nothingignores-hidden-files/.folder/- returns nothingignores-hidden-files/.folder/*- returnsfileignores-hidden-files/.folder/file- returnsfile
This PR adds an option to the glob package to exclude hidden files. This includes:
...This currently does not check for the hidden attribute that can be set on Windows files. Based on the discussion in nodejs/node#38809, this isn't natively supported in Node at the moment.
I'm open to working around that limitation, but the primary focus here is for files and directories that use the
.naming convention.