8000 Change pip default cache path to `**/pyproject.toml` · Issue #529 · actions/setup-python · GitHub
[go: up one dir, main page]
Skip to content

Change pip default cache path to **/pyproject.toml #529

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
Closed
flying-sheep opened this issue Oct 21, 2022 · 15 comments
8000
Closed

Change pip default cache path to **/pyproject.toml #529

flying-sheep opened this issue Oct 21, 2022 · 15 comments
Assignees
@e-korolevskii
Labels
bug Something isn't working feature request New feature or request to improve the current logic

Comments

@flying-sheep
Copy link
flying-sheep commented Oct 21, 2022 •
edited
Loading

Description:
requirements.txt is a convention. The only existing standard for where to specify Python dependencies is PEP 621

Related to #502, but not following standards is IMHO a bug, not a missing feature.

Action version:
v4

Platform, Runner type, Tools version:
N/A

Repro steps:
Create a standard Python project, e.g. via hatch new or by following the official tutorial: https://packaging.python.org/en/latest/tutorials/packaging-projects/

Expected behavior:
The default path(s) should end with pyproject.toml.

An option would be to be smart about it and only consider pyproject.toml if it contains a project.dependencies array.

Actual behavior:
The default cache path is **/requirements.txt.
@flying-sheep flying-sheep added bug Something isn't working needs triage labels Oct 21, 2022
@vsafonkin
Copy link

Hi @flying-sheep, we will take a loot at this.

@dmitry-shibanov
Copy link
Contributor

Hello @flying-sheep. The action uses requirements.txt to generate has for primary and restore keys. Do you want to introduce a pyproject.toml file as a fallback if requirements.txt file is missing ?

@flying-sheep
Copy link
Author

The only problem with that is that the pure existence of pyproject.toml doesn’t mean the file has PEP 621 dependency metadata (i.e. a project.dependencies array). E.g. some projects configure black in pyproject.toml while also specifying their dependencies in a setup.py or setup.cfg. I think it’s helpful that the action is throwing an error if the default value of cache-dependency-path doesn’t point to a file containing dependencies.

Maybe we change the abstraction to be smarter (or more flexible) than the combination of cache and cache-dependency-path to handle the above?

@dmitry-shibanov
Copy link
Contributor

In this case I think you should specify multiple files in cache-dependency-path. But if you need more complex logic the proper solution would be using the actions/cache to achieve it.

For now I'm going to close the issue.

@flying-sheep
Copy link
Author
flying-sheep commented Oct 25, 2022 •
edited
Loading

why? my point is that pyproject.toml is the standard way to specify dependencies in Python.

requirements.txt is not. It’s an old convention.

The default settings should support the standard. Supporting the old convention should be second priority.

@dmitry-shibanov
Copy link
Contributor

In this case we can use my previous suggestion and change logic to use pyproject.toml as fallback if requirements.txt was not found. Moreover, I think a lot of customers still use requirements.txt as a single file for dependencies and it can break builds by removing requirements.txt.

@flying-sheep
Copy link
Author

Sure, if that’s how you want to do it.

I think once a standard python lockfile exists, we can get rid of requirements.txt as default (as a major version update of this action). Until then there’s people using both pyproject.toml for abstract dependencies and requirements.txt as a lockfile.

For now I'm going to close the issue.

I’m curious: Why did you do that? This issue is neither fixed nor invalid.

@dmitry-shibanov
Copy link
Contributor

I closed the issue related to your previous comment. As I understood you'd like to introducing changes for parsing both files to get metadata and identify which file should be used for hash. For more complex caching logic it's better to use actions/cache that is why I closed it.

For now I'm going to reopen it.

@Conchylicultor
Copy link

+1, actions/setup-python should support the official Python packaging standard (PEP 621 & cie).
At minima, the doc should indicate usage when using pyproject.toml based-dependencies.

@zanieb zanieb mentioned this issue Nov 23, 2022
Merged
EdAyers added a commit to hitsave-io/xyz that referenced this issue Dec 14, 2022
@EdAyers
ci: add a requirements file because that's what the setup python acti…
b223fe1 
…on is looking for

Waiting on actions/setup-python#529
EdAyers added a commit to hitsave-io/xyz that referenced this issue Dec 16, 2022
@EdAyers
ci: add a requirements file because that's what the setup python acti…
044f1f9 
…on is looking for

Waiting on actions/setup-python#529
@alexdelorenzo
Copy link
alexdelorenzo commented Dec 29, 2022 •
edited
Loading

This would be a nice feature/fix 👍

New projects using the PEP 621 standard for packaging metadata will have to duplicate their dependencies in the non-standard requirements.txt file in order for setup-python to cache dependencies properly. That makes having a single source of truth for dependencies, ideally pyproject.toml as part of the standard, hard if they want to use this in their CI pipeline.

I agree with @Conchylicultor about indicating this in the docs, as well.

alexdelorenzo added a commit to alexdelorenzo/console that referenced this issue Dec 29, 2022
@alexdelorenzo
setup-python does not support PEP 621 pyproject.toml for dependen…
70ac6b7 
…cy caching

See actions/setup-python#529 (comment)
@br3ndonland br3ndonland mentioned this issue Jan 2, 2023
Closed
alexdelorenzo added a commit to alexdelorenzo/console that referenced this issue Jan 11, 2023
@alexdelorenzo
setup-python does not support PEP 621 pyproject.toml for dependen…
f8c5d97 
…cy caching

See actions/setup-python#529 (comment)
@e-korolevskii e-korolevskii added the feature request New feature or request to improve the current logic label Jan 18, 2023
@e-korolevskii e-korolevskii mentioned this issue Jan 20, 2023
Open
2 tasks
jasonjewik added a commit to aditya-grover/climate-learn that referenced this issue Feb 6, 2023
@jasonjewik
Add requiremnts.txt file
d8f5aa5 
Although the current Python packaging standard now uses pyproject.toml,
GitHub Actions currently does not support that. Hence I include a
requirements.txt file. Discussion: actions/setup-python#529
@e-korolevskii e-korolevskii mentioned this issue Feb 9, 2023
Merged
2 tasks
@janosh
Copy link
janosh commented Feb 28, 2023

The docs on this are confusing. I read this readme section as saying I don't manually have to specify pyproject.toml when setting cache: pip which in fact I do have to.

The action defaults to searching for a dependency file (requirements.txt or pyproject.toml for pip...

The error for not setting cache-dependency-path: pyproject.toml:

Error: No file in /home/runner/work/matbench-discovery/matbench-discovery matched to [**/requirements.txt], make sure you have checked out the target repository

@janosh
Copy link
janosh commented Feb 28, 2023

My bad, just saw this was added recently but not released yet.

@dsame
Copy link
Contributor
dsame commented Mar 26, 2023

@flying-sheep i am closing the issue because the PR #604 has been merged and will be available on the next action release. It is possible to use the change before the release with @main tag:

- uses: actions/setup-python@main

Please feel free reopen this issue or create another one in case if the problem still exists

@flying-sheep
Copy link
Author

sounds good to me!

@CoolCat467
Copy link

I think you should pin this issue. I had the same issue and it took a bit to find this.

@koxudaxi koxudaxi mentioned this issue Dec 2, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@e-korolevskii e-korolevskii

Labels
bug Something isn't working feature request New feature or request to improve the current logic
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
9 participants
@dsame @flying-sheep @alexdelorenzo @vsafonkin @Conchylicultor @dmitry-shibanov @janosh @CoolCat467 @e-korolevskii
0