source-controller
source-controller copied to clipboard
GitRepository Ignore Behavior Does Not Match Documentation
Currently, the documentation for the ignore key in GitRepository says:
// Ignore overrides the set of excluded patterns in the .sourceignore format
// (which is the same as .gitignore). If not provided, a default will be used,
// consult the documentation for your version to find out what those are.
// +optional
but it does not "override the set of excluded patterns", it "appends to the set of excluded patterns".
To illustrate, given the currently listed defaults (.git/, .gitignore, .gitmodules, .gitattributes) and a repository containing both .git/ and .gitignore, setting ignore: .git/ results in a tarball that is missing both .git/ and .gitignore. This wouldn't be override behavior because I'd have expected .gitignore (which is not in the overriden ignore key) to be in the tarball. However using ignore: !.gitignore results in a tarball that is missing .git/ and has .gitignore in it showing the append behavior.
I see two options to address this discrepancy
- Update the documentation to describe the append behavior
- Update the code to override instead of append
My personal preference is option 2 because it doesn't require a CR reader combine both what they can read as well as a piece of documentation they might not have close by in order to understand the outcome. That being said, I'm pretty ambivalent about which is chosen.
The goal of the feature is as described in your option one, but the chosen wordings are a bit unfortunate. What the documentation block is trying to say (and this makes a bit more sense once you are familiar with the .gitignore format rules), is that rules added there allow overwriting the defaults or in-source ones, as the appended rules take precedence and/or allow overwriting defaults.
Option 2 is a no-go for me because the default exclusions have been carefully selected, and are sane defaults for the majority of sources. The extra mile you have to take as a user to figure out what rules you need to (over)write can thus be seen as a safety precaution to ensure users do not accidentally end up with large (non-YAML) files in the produced artifact, something that also helps us as maintainers to reduce the stream of support requests around resource usage and network traffic, etc.
I'm trying to add a directory to the ignore list and was a bit concerned that adding a spec.ignore would replace the default ignored list. But as this issue points out, it only appends my Ignore value to the set of ExcludeVCS entries and removes the other Exclude{CI, Ext, Extra} entries - meaning if I still want those excluded I need to add them back.
Maybe what we need are two fields
- spec.ignore - that matches the documentation and is a complete replacement
- spec.localIgnore - that appends the list of default ignored entries to the entries specified here
This enables the user to completely override the ignore list and just add new entries to the existing defaults.
If both fields are specified, the spec.ignore completely replaces the defaults and then is appended to spec.localIgnore entries