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 run export substring check #5575

Merged
merged 11 commits into from
Feb 27, 2025
Merged

Fix run export substring check #5575

merged 11 commits into from
Feb 27, 2025

Conversation

vyasr
Copy link
Contributor

@vyasr vyasr commented Dec 31, 2024

Description

This PR fixes #5529.

It also adds a constraint to the test requirements file that was previously missing but is required to get a compatible version of py-lief, and it updates the gitignore for vim swap files.

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 @vyasr.

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#1088), and ping the bot to refresh the PR.

@vyasr
Copy link
Contributor Author

vyasr commented Dec 31, 2024

I just signed the CLA. Let me know if I need to forward the confirmation email to anyone.

@vyasr
Copy link
Contributor Author

vyasr commented Dec 31, 2024

One note that isn't directly related to this PR but did affect me is that the test requirements file is also missing a few additional requirements.

  • pytest: This one should definitely be there to run the tests. I do see it in the requirements-ci.txt file, but I'm not sure what the purpose of requirements.txt is if not to include everything needed to at least run the tests locally as well.
  • pytest-cov: This probably shouldn't be a hard requirement, but it currently is due to the hardcoded addopts in pyproject.toml.
  • binstar_client: I guess this is a legacy pre-anaconda project, so I'm not sure where it should be coming from or if it should still be used here.

@vyasr
Copy link
Contributor Author

vyasr commented Dec 31, 2024

I also need a bit of help figuring out the right way to test this. I've verified the result manually, but I couldn't find an easy programmatic way of doing this with conda_build APIs since rendering isn't sufficient so we can't just inspect metadata. The best I could come up with is running the build and then manually extracting and inspecting the metadata of the result. I'm not sure if there is a better way, though.

@vyasr
Copy link
Contributor Author

vyasr commented Jan 2, 2025

@conda-bot check

@conda-bot conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Jan 2, 2025
@vyasr vyasr marked this pull request as ready for review January 2, 2025 17:27
@vyasr vyasr requested a review from a team as a code owner January 2, 2025 17:27
Copy link

codspeed-hq bot commented Jan 2, 2025

CodSpeed Performance Report

Merging #5575 will not alter performance

Comparing vyasr:fix/issue_5529 (b299c26) with main (bfb9496)

Summary

✅ 5 untouched benchmarks

@vyasr
Copy link
Contributor Author

vyasr commented Jan 10, 2025

The test failures look completely unrelated to my change, but I do still need some help with the things I listed above before this PR would be ready.

Copy link
Contributor

@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.

The test needs to check that the run deps of the package are correct in addition to ensuring that the package builds.

@vyasr
Copy link
Contributor Author

vyasr commented Feb 16, 2025

The test needs to check that the run deps of the package are correct in addition to ensuring that the package builds.

Thanks for taking a look @beckermr. I had some questions above about how to test, could you please provide some guidance?

@beckermr
Copy link
Contributor

Yes. Running the build and then manually testing the resulting package is perfectly fine.

beckermr
beckermr previously approved these changes Feb 27, 2025
@beckermr
Copy link
Contributor

The rendered recipe is printed to stdout, so I added a check for the correct run export there.

@beckermr beckermr enabled auto-merge (squash) February 27, 2025 17:52
@beckermr
Copy link
Contributor

ack only conda-forge outputs the python_abi package.

@beckermr beckermr disabled auto-merge February 27, 2025 18:24
@beckermr
Copy link
Contributor

windows failures are unrelated and appear to be a network outage - will rerun.

@beckermr beckermr enabled auto-merge (squash) February 27, 2025 18:46
@beckermr beckermr merged commit 6567049 into conda:main Feb 27, 2025
28 checks passed
@vyasr
Copy link
Contributor Author

vyasr commented Feb 28, 2025

@beckermr thanks for finishing this up for me! I meant to get back to it but hadn't quite gotten around to it quite yet. I appreciate the assist!

@vyasr vyasr deleted the fix/issue_5529 branch February 28, 2025 05:15
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
Projects
Status: 🏁 Done
Development

Successfully merging this pull request may close these issues.

Ignored run exports are matched by substring and not exactly
5 participants