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: in-repo plugin requirements.txt not loading #20355

Merged
merged 4 commits into from
Jan 9, 2024
Merged

Conversation

idan-at
Copy link
Contributor

@idan-at idan-at commented Dec 31, 2023

This fixes the change that was introduced on #19406: Due to the fact sys.path was not yet altered, _collect_backends_requirements fails to load in-repo plugins, thus ignoring the requirements.txt file.

This fixed the change that was introduced on pantsbuild#19406: Due to the fact sys.path was not yet altered, `_collect_backends_requirements` fails to load in-repo plugins, thus ignoring the requirements.txt file.
@idan-at
Copy link
Contributor Author

idan-at commented Dec 31, 2023

@kaos Unfortunately the original PR had an issue :(
Any chance you can take a look at this one? 🙏

Thanks

@huonw
Copy link
Contributor

huonw commented Jan 1, 2024

Is this something we can add a regression test for?

@huonw huonw requested a review from kaos January 1, 2024 21:52
@huonw huonw added the category:bugfix Bug fixes for released features label Jan 1, 2024
@idan-at
Copy link
Contributor Author

idan-at commented Jan 2, 2024

I’m not sure, maybe you’ll have a suggestion. The problem only occurs when you try to load a in-repo plugin that has 3rd party dependencies, and it’s path is not already in sys.path

idan-at and others added 3 commits January 8, 2024 17:02
This fixed the change that was introduced on pantsbuild#19406: Due to the fact sys.path was not yet altered, `_collect_backends_requirements` fails to load in-repo plugins, thus ignoring the requirements.txt file.
Copy link
Member

@kaos kaos left a comment

Choose a reason for hiding this comment

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

Thanks. would be great with a test for it, but let's not hold perfect as the enemy of landing this here.. :)

@kaos kaos added this to the 2.19.x milestone Jan 9, 2024
@idan-at
Copy link
Contributor Author

idan-at commented Jan 9, 2024

Thanks for approving! I've updated the existing test to reflect the change, so we're covered :)

@kaos kaos merged commit 4a5eca0 into pantsbuild:main Jan 9, 2024
24 checks passed
WorkerPants pushed a commit that referenced this pull request Jan 9, 2024
This fixes the change that was introduced on #19406: Due to the fact
sys.path was not yet altered, `_collect_backends_requirements` fails to
load in-repo plugins, thus ignoring the requirements.txt file.

---------

Co-authored-by: Idan Attias <[email protected]>
@WorkerPants
Copy link
Member

I tried to automatically cherry-pick this change back to each relevant milestone, so that it is available in those older releases of Pants.

✔️ 2.19.x

Successfully opened #20381.


Thanks again for your contributions!

🤖 Beep Boop here's my run link

@idan-at idan-at deleted the patch-1 branch January 9, 2024 12:39
kaos pushed a commit that referenced this pull request Jan 29, 2024
…) (#20381)

This fixes the change that was introduced on #19406: Due to the fact
sys.path was not yet altered, `_collect_backends_requirements` fails to
load in-repo plugins, thus ignoring the requirements.txt file.

Co-authored-by: Idan Attias <[email protected]>
Co-authored-by: Idan Attias <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:bugfix Bug fixes for released features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants