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.

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

fix: pass --extra-meta for rattler-build #2037

Merged
merged 5 commits into from
Sep 5, 2024

Conversation

nichmor
Copy link
Contributor

@nichmor nichmor commented Aug 16, 2024

Checklist

  • Added a news entry
  • Regenerated schema JSON if schema altered (python conda_smithy/schema.py)

Closes #2034

Actual run can be seen here: conda-forge/jolt-physics-feedstock#2

@nichmor nichmor requested a review from a team as a code owner August 16, 2024 15:59
@nichmor nichmor marked this pull request as draft August 16, 2024 15:59
Copy link
Member

@beckermr beckermr left a comment

Choose a reason for hiding this comment

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

Does rattler not support the --no-test option? We need that for some cross-compile cases when emulation is turned off or we are working with osx-arm on osx-64.

@baszalmstra
Copy link
Member

There is a --no-test for rattler-build but Im not entirely sure if it does the same thing as conda-build. Can you check @nichmor ?

@nichmor
Copy link
Contributor Author

nichmor commented Sep 4, 2024

should also fix: conda-forge/crane-feedstock#6

@baszalmstra
Copy link
Member

@wolfv Can you verify that --no-test for rattler-build do the same thing as --no-test for conda-build?

@wolfv
Copy link
Member

wolfv commented Sep 5, 2024

I did just double check, and rattler-build unconditionally skips test execution which is what conda-build also does.

if [[ "${HOST_PLATFORM}" != "${BUILD_PLATFORM}" ]] && [[ "${BUILD_WITH_CONDA_DEBUG:-0}" != 1 ]]; then
EXTRA_CB_OPTIONS="${EXTRA_CB_OPTIONS:-} --no-test"
fi
{%- elif test == "native_and_emulated" -%}
{%- elif test == "native_and_emulated" and conda_build_tool != "rattler-build" -%}
Copy link
Member

Choose a reason for hiding this comment

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

is this correct? If we need --no-test then this is going to be missing in the rattler-build case, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Initially it was disabled by me, but now that --no-test is working for rattler-build I need to remove this check

Copy link
Member

Choose a reason for hiding this comment

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

I agree this looks wrong.

Copy link
Member

Choose a reason for hiding this comment

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

I reverted these changes.

@wolfv
Copy link
Member

wolfv commented Sep 5, 2024

Do we need to set the rattler-build version somewhere to >=0.21.0?

Looks good to me!

@nichmor
Copy link
Contributor Author

nichmor commented Sep 5, 2024

Do we need to set the rattler-build version somewhere to >=0.21.0?

Looks good to me!

I think the minimal should be >=0.20.0 as it was released there, and we need to set it here: https://github.com/conda-forge/rattler-build-conda-compat-feedstock/blob/main/recipe/meta.yaml#L24.

but we can merge this anyway,no?

@wolfv wolfv merged commit a77a55a into conda-forge:main Sep 5, 2024
2 checks passed
@wolfv
Copy link
Member

wolfv commented Sep 5, 2024

Yep!

@beckermr
Copy link
Member

beckermr commented Sep 5, 2024

Please do not merge if I have requested changes. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pass --extra-meta for rattler-build
4 participants