You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Change pip default cache path to **/pyproject.toml #529
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 ?
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?
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.
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.
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.
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.
+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.
Reacted by Alex DeLorenzo, Jason Jewik, Maxime Churin, Heberto Mayorquin, Rudyard Richter, CoolCat467, Peter Paluska, Peter Morrow and Matthew Timms
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.
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
@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
Activity
vsafonkin commentedon Oct 21, 2022
Hi @flying-sheep, we will take a loot at this.
dmitry-shibanov commentedon Oct 23, 2022
Hello @flying-sheep. The action uses
requirements.txtto generate has for primary and restore keys. Do you want to introduce apyproject.tomlfile as a fallback ifrequirements.txtfile is missing ?flying-sheep commentedon Oct 24, 2022
The only problem with that is that the pure existence of
pyproject.tomldoesn’t mean the file has PEP 621 dependency metadata (i.e. aproject.dependenciesarray). E.g. some projects configureblackinpyproject.tomlwhile also specifying their dependencies in asetup.pyorsetup.cfg. I think it’s helpful that the action is throwing an error if the default value ofcache-dependency-pathdoesn’t point to a file containing dependencies.Maybe we change the abstraction to be smarter (or more flexible) than the combination of
cacheandcache-dependency-pathto handle the above?dmitry-shibanov commentedon Oct 24, 2022
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 commentedon Oct 25, 2022
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 commentedon Oct 25, 2022
In this case we can use my previous suggestion and change logic to use
pyproject.tomlas fallback if requirements.txt was not found. Moreover, I think a lot of customers still userequirements.txtas a single file for dependencies and it can break builds by removingrequirements.txt.flying-sheep commentedon Oct 25, 2022
Sure, if that’s how you want to do it.
I think once a standard python lockfile exists, we can get rid of
requirements.txtas default (as a major version update of this action). Until then there’s people using both pyproject.toml for abstract dependencies andrequirements.txtas a lockfile.I’m curious: Why did you do that? This issue is neither fixed nor invalid.
dmitry-shibanov commentedon Oct 25, 2022
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 commentedon Nov 22, 2022
+1,
actions/setup-pythonshould support the official Python packaging standard (PEP 621 & cie).At minima, the doc should indicate usage when using
pyproject.tomlbased-dependencies.ci: add a requirements file because that's what the setup python acti…
ci: add a requirements file because that's what the setup python acti…
alexdelorenzo commentedon Dec 29, 2022
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.txtfile in order forsetup-pythonto cache dependencies properly. That makes having a single source of truth for dependencies, ideallypyproject.tomlas 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.
`setup-python` does not support PEP 621 `pyproject.toml` for dependen…
`setup-python` does not support PEP 621 `pyproject.toml` for dependen…
Add requiremnts.txt file
janosh commentedon Feb 28, 2023
The docs on this are confusing. I read this readme section as saying I don't manually have to specify
pyproject.tomlwhen settingcache: pipwhich in fact I do have to.The error for not setting
cache-dependency-path: pyproject.toml:janosh commentedon Feb 28, 2023
My bad, just saw this was added recently but not released yet.
dsame commentedon 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
@maintag:Please feel free reopen this issue or create another one in case if the problem still exists
flying-sheep commentedon Mar 26, 2023
sounds good to me!
CoolCat467 commentedon May 9, 2023
I think you should pin this issue. I had the same issue and it took a bit to find this.