boulder: Fix pattern matching #402
Open
+14
−4
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Found another bug, figuring it out |
|
are you sure..? check the manifest, the splitting appears fine, I had to fix some wild globs in it a while back |
ReillyBrogan
force-pushed
the
fix-pattern
branch
from
b6985b2 to
e14121c
Compare
ReillyBrogan
force-pushed
the
fix-pattern
branch
from
e14121c to
8ce3633
Compare
Boulder had a needless check for `starts_with` which was causing erroneous package splitting in several cases. For instance, consider the following simplified patterns:
```
- llvm-libs:
paths:
- /usr/lib/libLLVM.so.*
- llvm-devel:
paths:
- /usr/lib/libLLVM.so
```
With this setup the file `/usr/lib/libLLVM.so.18.1` should be patterned into `llvm-libs`, however since the `starts_with` match was checked first it would end up in `llvm-devel`. The point of this was to ensure that we could have "globs" that matched directories that matched subfiles/subdirectories and in order to preserve that behavior we instead match twice, once on the submitted pattern and again with `/**` appended to get a recursive glob.
Also while we're at it apply a minor adjustment to escape the directory so that all glob patterns work correctly.
Signed-off-by: Reilly Brogan <[email protected]>
ReillyBrogan
force-pushed
the
fix-pattern
branch
from
8ce3633 to
a8c40c2
Compare
|
@ikeycode From an engineering-led perspective, perhaps the smart thing to do here would be to set up a test suite for the pattern matching code, and having said test suite become the "specification" in a sense? Would make the discussion very simple in that it'd simply be: "Well, what do the tests show?" If you agree, I could put up an issue and then we could work on getting that part up to snuff with suitable test patterns in a separate PR? |
|
Yeah we do need to extend tests |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Boulder had a needless check for
starts_withwhich was causing erroneous package splitting in several cases. For instance, consider the following simplified patterns:With this setup the file
/usr/lib/libLLVM.so.18.1should be patterned intollvm-libs, however since thestarts_withmatch was checked first it would end up inllvm-devel. The point of this was to ensure that we could have "globs" that matched directories that matched subfiles/subdirectories and in order to preserve that behavior we instead match twice, once on the submitted pattern and again with/**appended to get a recursive glob.Also while we're at it apply a minor adjustment to escape the directory so that all glob patterns work correctly.