buildozer: Respect .bazelignore#1343
Conversation
121595d to
ef60cb0
Compare
ef60cb0 to
8d38f91
Compare
|
Hi @vladmos, are you available to review this PR? Thank you! |
edit/buildozer.go
Outdated
| rel = filepath.ToSlash(rel) | ||
|
|
||
| for _, prefix := range ignoredPrefixes { | ||
| if strings.HasPrefix(rel, prefix) { |
There was a problem hiding this comment.
Will foobar be ignored if foo is among ignored prefixes?
There was a problem hiding this comment.
Ah yes, good call. I changed the comparison to only accept exact match or match with prefix+"/", and changed getIgnoredPaths to Clean the paths so that / suffixes in .bazelignore are stripped from it. 111bc6e
…me is in .bazelignore.
|
Could you please add a boolean option to Opts (e.g. This is a useful feature for the open-source version of buildozer, however, not required by the internal one and may add some unnecessary latency for file operations there, it'd be nice if it can be configurable. |
@vladmos PTAL; I named |
vladmos
left a comment
There was a problem hiding this comment.
Thanks! I'll merge it within a couple of days, there's currently another chunk of changes being imported at the moment, don't want multiple imports to interfere with each other.
|
@vladmos no particular hurry, but is this unblock to be merged? Thank you! |
|
Sorry for the delay, merging it. |
Currently, when expanding
/...targets, buildozer does not respect.bazelignorefiles, which can be surprising. This implements reading.bazelignoreif it exists and ignoring any paths from it to match bazel's behavior.Note that we only do prefix comparison, with no support for globs, as that is bazel's behavior. We also ignore any absolute paths, which, in bazel itself, results in an error. Reference implementation:
https://github.com/bazelbuild/bazel/blob/8dbfcfae924b014366a0b47cd26632c5076b51a2/src/main/java/com/google/devtools/build/lib/skyframe/IgnoredSubdirectoriesFunction.java#L76-L103
Part of #623 (this only applies for buildozer)