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.
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
NEW: Better package splitting for multi-output recipes with negative glob in outputs/files #5216
NEW: Better package splitting for multi-output recipes with negative glob in outputs/files #5216
Changes from 40 commits
a545986
14e3548
7fbc142
67904ce
e56c573
be19766
b667724
063ff89
1ab7638
de7715d
f56e201
0537c19
30a5e1b
a92964f
a2e3963
bfac4e0
3c0b090
fd5e956
4c04d49
e16c470
e598d6a
8dd906d
642906b
d1b12a1
9dbd17c
c3a26dd
84aad48
249a065
55248d4
e42b6f0
a62e4b9
e28565f
ee89c04
b0645c2
822b477
475c0ec
4f988f9
b33f6f6
256017a
e5fdcbf
606ee08
6d33054
5d8efe9
c92f1a1
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Given the change in behavior above with only allowing new files to be included should the old behavior of including any file in the prefix be deprecated?
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.
It's still useful for eg when building
gcc_bootstrap
packages. Deprecating the implicit behaviour and adding a new flag would be useful.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 feel that the current behavior of including all files from the prefix for multi-output and only installed files from the prefix for single-output recipes is inconsistent and thus non-intuitive. rattler-build is adding a section called "always_include_files" for recipes that want to repackage artifacts from their host dependencies.
I'm favor of deprecating the old behavior, but I'm uncertain about any removal or replacement schedule.
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.
This exists in conda already, and is in use in conda-forge. Main use-case for that from my POV is to be explicit about overwriting some CMake metadata if an incremental build adds more targets to an already-existing CMake file (example).
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.
We can deprecate that behaviour in a separate PR too. This PR has been open for too long and I wouldn't want to test your patience further @carterbox <3
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.
🚀 LGTM! Just wanted to understand the thoughts/plans for the old behavior.
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'll open an issue to capture this feedback, though I think with the documentation this PR added we are in a better place already.