Skip to content
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.

Sign up for GitHub

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

add run_constrained on xz from liblzma #48

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

add run_constrained on xz from liblzma #48

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2d70ab2
add circular constraint on xz to liblzma
minrk Jan 9, 2025
7e8134b
MNT: Re-rendered with conda-build 24.9.0, conda-smithy 3.45.1, and co…
minrk Jan 9, 2025
74d3b34
list
minrk Jan 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions .ci_support/linux_64_.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,3 @@ docker_image:
- quay.io/condaforge/linux-anvil-x86_64:alma9
target_platform:
- linux-64
xz:
- '5'
2 changes: 0 additions & 2 deletions .ci_support/linux_aarch64_.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,3 @@ docker_image:
- quay.io/condaforge/linux-anvil-aarch64:alma9
target_platform:
- linux-aarch64
xz:
- '5'
2 changes: 0 additions & 2 deletions .ci_support/linux_ppc64le_.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,3 @@ docker_image:
- quay.io/condaforge/linux-anvil-ppc64le:alma9
target_platform:
- linux-ppc64le
xz:
- '5'
2 changes: 0 additions & 2 deletions .ci_support/osx_64_.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,3 @@ macos_machine:
- x86_64-apple-darwin13.4.0
target_platform:
- osx-64
xz:
- '5'
2 changes: 0 additions & 2 deletions .ci_support/osx_arm64_.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,3 @@ macos_machine:
- arm64-apple-darwin20.0.0
target_platform:
- osx-arm64
xz:
- '5'
2 changes: 0 additions & 2 deletions .ci_support/win_64_.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,3 @@ channel_targets:
- conda-forge main
target_platform:
- win-64
xz:
- '5'
7 changes: 6 additions & 1 deletion recipe/meta.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
{% set version = "5.6.3" %}
{% set build = 2 %}

package:
name: xz-split
Expand All @@ -9,7 +10,7 @@ source:
sha256: b1d45295d3f71f25a4c9101bd7c8d16cb56348bbef3bbc738da0351e17c73317

build:
number: 1
number: {{ build }}
run_exports:
# XZ's track record of backcompat is very good. Keep default pins (next major version)
# https://abi-laboratory.pro/tracker/timeline/xz/
Expand Down Expand Up @@ -39,6 +40,10 @@ outputs:
- cmake-no-system # [win]
- ninja # [win]
- make # [not win]
run_constrained:
# circular constraint on this build's xz to prevent accepting pre-split xz
# this is as close to 'exact' as we can get
- xz =={{ version }} *_{{ build }}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need the build number, because the split was introduced with a new version (#42). So already the version constraint avoids pulling in old builds. I would just say

Suggested change
- xz =={{ version }} *_{{ build }}
# avoid co-installation with pre-split xz packages
- xz =={{ version }}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The build number is required for consistency with the future, e.g. to avoid introducing the same problem again in a later refactor. The split could have happened within a version, and similar changes could happen in the future within a version, so I think this is the safest and most correct way to go (pin_subpackage("xz", exact=True) is certainly what should go here, but doesn't work for circular dependency reasons).

FWIW, I tried =={{ version }}[build={{ build }}] which I believe is valid and avoids the build string glob, but conda-build mistakenly parses that matchspec as a selector comment, leading to an error.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The split could have happened within a version, and similar changes could happen in the future within a version [...]

It very plainly didn't happen within a version, and we shouldn't try to prematurely optimize for a completely hypothetical split, which (if it ever happens), will be responsible for handling its own migration story.

Ultimately I'm not a maintainer on this feedstock though, so I'll leave this to y'all to hash out.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm just not sure what the downside is, when the intended constraint seems pretty clearly to be "this build of this package" which is version + build number. If there's a downside I'm happy to go with something better, I just don't see what it would be.

test:
commands:
- if not exist %LIBRARY_PREFIX%\bin\liblzma.dll exit 1 # [win]
Expand Down