Correctly match regex with tree artifact #16949
Conversation
| break; | ||
| if (output.isTreeArtifact()) { | ||
| var children = metadataHandler.getTreeArtifactChildren((SpecialArtifact) output); | ||
| if (outputMatchesPattern(output)) { |
There was a problem hiding this comment.
If the pattern has a $, would this incorrectly match tree artifact paths?
There was a problem hiding this comment.
Do you have a concrete example?
If the pattern matches tree root, all files inside the tree are downloaded. (See newly added test cases)
There was a problem hiding this comment.
So that seems like a bug to me, since if we weren't using a tree artifact we wouldn't download everything under a/path with the pattern a/path$ (for example, if we had an output of a/path/and/more/lib.a).
There was a problem hiding this comment.
The check is guarded by output.isTreeArtifact(). File output a/path/and/more/lib.a won't match pattern a/path$ so it won't be downloaded. Added another test case to illustrate that.
There was a problem hiding this comment.
Sure, but I'm saying that the behavior shouldn't differ between if it's a tree artifact or not. We shouldn't match on the tree artifact itself if it has children, if using $ in the pattern.
There was a problem hiding this comment.
(If I change the lib.a example from a normal output to a tree artifact output, the download behavior of any patterns shouldn't change.)
There was a problem hiding this comment.
I see. Good catch! Updated the code.
It doesn't work when setting `--experimental_remote_download_regex` to match files inside tree artifact after bazelbuild@e01e7f5. This change fixes that. Fixes bazelbuild#16922. Closes bazelbuild#16949. PiperOrigin-RevId: 493838296 Change-Id: I6eceffbffce30949173d10120a9120c6c608a983
It doesn't work when setting
--experimental_remote_download_regexto match files inside tree artifact after e01e7f5. This change fixes that.Fixes #16922.