-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
🐛 Don't recurse into hidden directories #1972
Conversation
Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA. It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
Welcome @daenney! |
Hi @daenney. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: daenney The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
checkDir() currently recurses into hidden directories at the root. This is a problem, because only the directory itself will have a . at the root of it's path, but any files within it won't when calling info.Name. This causes init to fail on a directory containing any hidden directory with files within them, such as .git. Since we use filepath.Walk(".") we can't skip a directory that just starts with ., as that would include the current directory. So instead we also check that Name() doesn't return just "."
@@ -207,6 +207,9 @@ func checkDir() error { | |||
if err != nil { | |||
return err | |||
} | |||
if info.IsDir() && info.Name() != "." && strings.HasPrefix(info.Name(), ".") { | |||
return filepath.SkipDir | |||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will not it allow scaffold projects in a directory that has another directory as well?
Then, note that it goes against the purpose of the check. We would like to not allow it because it means that we cannot ensure that the tool is able to do properly all required scaffolds.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe the check is too paranoid. it needs to not only exclude files starting with a dot, it needs to not recurse into hidden directories.
It will only allow you to scaffold into directories that have hidden directories at the root. For example, if you try to scaffold right now in a directory that you've just run a git init
on, it'll fail. Though .git
has a .
prefix, the HEAD
file in .git/HEAD
does not, so it'll fail because info.Name()
will return HEAD
, not .git/HEAD
.
Similarly if you have opened that directory in an editor already and something like a .vscode
exists it'll fail to scaffold. None of these directories should have any impact on the ability to scaffold. If we ignore hidden files, we should ignore hidden directories too.
Closed in favour of #1944 |
checkDir() currently recurses into hidden directories at the root. This
is a problem, because only the directory itself will have a . at the
root of it's path, but any files within it won't when calling info.Name.
This causes init to fail on a directory containing any hidden directory
with files within them, such as .git.
Since we use filepath.Walk(".") we can't skip a directory that just
starts with ., as that would include the current directory. So instead
we also check that Name() doesn't return just "."
Fixes #1971