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

Jump to bottom

fix: add --target-platform for rattler-build invocation #2070

Merged
merged 2 commits into from
Oct 1, 2024
Merged

fix: add --target-platform for rattler-build invocation #2070

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1bfdc34
add `--target-platform` for rattler-build and add SYSTEM_VERSION_COMP…
wolfv Sep 30, 2024
5db54fc
remove system_version_compat
wolfv Oct 1, 2024
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
1 change: 1 addition & 0 deletions conda_smithy/templates/build_steps.sh.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ else
{{ BUILD_CMD }} --recipe "${RECIPE_ROOT}" \
-m "${CI_SUPPORT}/${CONFIG}.yaml" \
${EXTRA_CB_OPTIONS:-} \
--target-platform "${HOST_PLATFORM}" \
Copy link
Member

Choose a reason for hiding this comment

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

Can we call it --host-platform in rattler-build? Pretty please!

Copy link
Member Author

Choose a reason for hiding this comment

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

We can definitely add an alias but would you also want to change target_platform variable everywhere in config files / recipes? We kept with conda-build here ... but host_platform does make some sense, too.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah an alias is the best we can do. We'll need a breaking change in conda-build to clean this up more.

Copy link
Member

Choose a reason for hiding this comment

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

Make host_platform the canonical thing and make target_platform an alias is the best option.

Copy link
Member

Choose a reason for hiding this comment

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

That is make target_platform env variable and jinja variable a alias to host_platform when target_platform is not explicitly given.

Copy link
Member

Choose a reason for hiding this comment

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

Sure what order do we want to do things in? We are talking about three PRs including this one.

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 think we should merge this, and open an issue on rattler-build for an alias.

Copy link
Member

Choose a reason for hiding this comment

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

I made an issue for this on the smithy side: #2072. We can revisit once rattler-build has the option.

Copy link
Member

Choose a reason for hiding this comment

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

Also this one: prefix-dev/rattler-build#1075

Copy link
Member

Choose a reason for hiding this comment

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

@isuruf you ok resolving this comment for now and going ahead with this PR?

--extra-meta flow_run_id="${flow_run_id:-}" \
--extra-meta remote_url="${remote_url:-}" \
--extra-meta sha="${sha:-}"
Expand Down
2 changes: 1 addition & 1 deletion conda_smithy/templates/run_osx_build.sh.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,10 +117,10 @@ else
--extra-meta flow_run_id="$flow_run_id" remote_url="$remote_url" sha="$sha"

{%- else %}

{{ BUILD_CMD }} --recipe ./{{ recipe_dir }} \
-m ./.ci_support/${CONFIG}.yaml \
--output-dir ${MINIFORGE_HOME}/conda-bld ${EXTRA_CB_OPTIONS:-} \
--target-platform "${HOST_PLATFORM}" \
--extra-meta flow_run_id="$flow_run_id" \
--extra-meta remote_url="$remote_url" \
--extra-meta sha="$sha"
Expand Down
2 changes: 1 addition & 1 deletion conda_smithy/templates/run_win_build.bat.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ conda-build.exe "{{ recipe_dir }}" -m .ci_support\%CONFIG%.yaml --suppress-varia
{%- elif conda_build_tool == "conda-build" %}
conda-build.exe "{{ recipe_dir }}" -m .ci_support\%CONFIG%.yaml --suppress-variables %EXTRA_CB_OPTIONS%
{%- elif conda_build_tool == "rattler-build" %}
conda.exe run rattler-build build --recipe "{{ recipe_dir }}" -m .ci_support\%CONFIG%.yaml %EXTRA_CB_OPTIONS%
conda.exe run rattler-build build --recipe "{{ recipe_dir }}" -m .ci_support\%CONFIG%.yaml %EXTRA_CB_OPTIONS% --target-platform %HOST_PLATFORM%
{%- endif %}
if !errorlevel! neq 0 exit /b !errorlevel!

Expand Down
3 changes: 3 additions & 0 deletions news/rattler-build-cross-compile.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
**Fixed:**

* fix cross-compilation with rattler-build by setting `--target-platform=${HOST_PLATFORM}`
Loading