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

Make __file__ an absolute path in setuptools.build_meta #3795

Merged
merged 4 commits into from
Feb 1, 2023

Conversation

hauntsaninja
Copy link
Contributor

Summary of changes

This is a difference between pip's legacy wheel build (or direct invocations of setup.py) and the PEP 517 build.

While setup.py scripts that rely on this are fragile, it was quite painful to debug! Since Python 3.4, __file__ is usually an absolute path, so this change might result in fewer surprises.

This is a difference between pip's legacy wheel build (or direct
invocations of setup.py) and the PEP 517 build.

While setup.py scripts that rely on this are fragile, it was quite
painful to debug! Since Python 3.4, `__file__` is usually an absolute
path, so this change might result in fewer surprises.
@hauntsaninja hauntsaninja marked this pull request as ready for review January 25, 2023 07:08
@hauntsaninja hauntsaninja reopened this Jan 25, 2023
@abravalheri
Copy link
Contributor

Hi @hauntsaninja, thank you very much for working on this.

Would it be possible to have a separated test, instead of adding to the parameterised ones?
The reason why I am asking it is because the backend tests are very onerous and ideally we want to minimise the amount of time they take. The parameterisation works as a cartesian product making the time to run the tests increase sharply...

@hauntsaninja
Copy link
Contributor Author

Thanks for the quick review; updated

@abravalheri
Copy link
Contributor

Thank you very much @hauntsaninja, I think this looks good to me.
If I am wrong there is no way os.path.abspath can break right? (i.e. it is a logic operation and will work even if the file does not exist, or am I missing something here?)

@hauntsaninja
Copy link
Contributor Author

Yeah, abspath works fine if the file doesn't exist

@hauntsaninja
Copy link
Contributor Author

hauntsaninja commented Jan 25, 2023

Hmm, I guess it calls os.getcwd, which can fail if the cwd has been deleted. But I think that's still okay? Or at least, I see plenty of abspath and os.getcwd in the setuptools source

@abravalheri
Copy link
Contributor

abravalheri commented Jan 25, 2023

That should be fine because CWD should contain the package code.
If I recall correctly, in PEP 517 the frontend should execute the backend entry-point with CWD set to the project root...


Yep, that is what the spec says:

...
All hooks are run with working directory set to the root of the source tree
...

@abravalheri
Copy link
Contributor

If the other maintainers have no reservations, I will include this change in the next release.

@abravalheri abravalheri merged commit de9cd7e into pypa:main Feb 1, 2023
@hauntsaninja hauntsaninja deleted the abspath branch February 1, 2023 17:38
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.

None yet

3 participants