Skip to content

use actual platform-specific perl path during TEST stage #5160

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jvolkening
Copy link

Description

In troubleshooting some failed win_64 CI builds for PRs submitted to conda-forge staged-recipes (conda-forge/staged-recipes#25137), I believe I narrowed the failed tests to the fact that the path determined for perl in the code modified here was setting the wrong path. Specifically, although these are noarch: generic recipes, they are being tested by the staged-recipes CI on Windows in addition to Linux and OSX. Since the value of metadata.config.host_platform here is set to noarch but the code that sets the path to perl is testing if platform.startswith("win"):, the resulting path is incorrect.

I saw another issue and commit relating to the use of host_platform instead of sys.platform in another part of the codebase in order to enable cross-compiling. In this instance, I can't foresee a situation in which the command string being built up in write_test_scripts would be run by test on a platform different from that on which it was called.

I haven't performed the points below yet, but I will if the change proposed seems valid. I'd like to get feedback on the change first.

Checklist - did you ...

  • Add a file to the news directory (using the template) for the next release's release notes?
  • Add / update necessary tests?
  • Add / update outdated documentation?

@conda-bot
Copy link
Contributor

We require contributors to sign our Contributor License Agreement and we don't have one on file for @jvolkening.

In order for us to review and merge your code, please e-sign the Contributor License Agreement PDF. We then need to manually verify your signature, merge the PR (conda/infrastructure#873), and ping the bot to refresh the PR.

@jvolkening
Copy link
Author

I signed the CLA

@conda-bot conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Feb 7, 2024
@@ -3235,7 +3235,7 @@ def _write_test_run_script(
tf.write(
'"{perl}" "{test_file}"\n'.format(
perl=metadata.config.perl_bin(
metadata.config.test_prefix, metadata.config.host_platform
metadata.config.test_prefix, sys.platform
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be metadata.config.build_platform?

Copy link
Author

Choose a reason for hiding this comment

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

Not entirely sure, but I can't see where that is defined or used in the codebase. sys.platform seems to be used in a number of places.

Copy link

Hi there, thank you for your contribution!

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed automatically if no further activity occurs.

If you would like this pull request to remain open please:

  1. Rebase and verify the changes still work
  2. Leave a comment with the current status

NOTE: If this pull request was closed prematurely, please leave a comment.

Thanks!

1 similar comment
Copy link

Hi there, thank you for your contribution!

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed automatically if no further activity occurs.

If you would like this pull request to remain open please:

  1. Rebase and verify the changes still work
  2. Leave a comment with the current status

NOTE: If this pull request was closed prematurely, please leave a comment.

Thanks!

@github-actions github-actions bot added the stale [bot] marked as stale due to inactivity label Apr 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed [bot] added once the contributor has signed the CLA stale [bot] marked as stale due to inactivity
Projects
Status: 🆕 New
Development

Successfully merging this pull request may close these issues.

3 participants