-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
feat: Support "automated install" usage via make
#7401
Conversation
|
||
|
||
if TYPE_CHECKING: | ||
from poetry.core.constraints.version import Version | ||
|
||
|
||
class VersionCommand(Command): | ||
class VersionCommand(InstallerCommand): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like that. InstallerCommand
requires the creation of a virtualenv. We shouldn't make commands InstallerCommand
if it's not required. Is touching the lock file really required here to cover your use case? (Bumping the version does not change the lock file.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feature is about keeping poetry.lock
's modification time newer than pyproject.toml
so that external tooling like Make is able to discern whether or not the lockfile is out of date. #7392 explains the use case.
Even though the lockfile contents aren't touched here, pyproject.toml is, meaning that Make would think the lockfile is out of date.
I suppose VersionCommand
could alternatively write to pyproject.toml
without changing its modificaiton time (perhaps by resetting it)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose
VersionCommand
could alternatively write topyproject.toml
without changing its modificaiton time (perhaps by resetting it)?
That seems also wrong to me. VersionCommand
changes pyproject.toml and thus should change the modification date of pyproject.toml. However, it doesn't change the lock file and thus shouldn't change the modification date of the lock file. Adding quirks to suffice the make
use case doesn't seem very attractable to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm up for only applying this to add
and remove
-- I'll make the change.
Any thoughts on testing? This section in commands/add.py
makes it "impossible" for TestLocker
to have self._write == True
(to materialize test lockfile to disk & direct test modification time).
@dimbleby @radoering I've added tests, would you please take a look? Also:
|
We're not accepting backports against 1.2, and new features are not likely to ever get backports -- so this would go into 1.4, which is imminent/currently in the release process. |
56dde83
to
bac4cba
Compare
make
A common annoyance when managing deps is having long sessions of repeated iteration loops of `add`, `remove`, tweaking `pyproject.toml` + `lock --no-update` + `install --sync`, re-running tests, trying to find a configuration that works. GNU `make` can implement "automated install" behavior, simplifying the loop to: "save any change, `make`, repeat if needed". Consider: ```make VENV := .venv PYTHON_VERSION := $(shell cat .python-version) PYTHON_VERSIONS_PATH ?= ~/.pyenv/versions PYTHON_INSTALL_CMD ?= pyenv install .PHONY: test test: $(VENV)/install.sentinel # tests depend on venv state poetry run pytest $(VENV)/install.sentinel: poetry.lock $(VENV)/bin/python # venv state depends on poetry.lock and venv poetry install --sync @ touch $@ .poetry.lock: pyproject.toml # poetry.lock depends on pyproject.toml poetry lock --check && touch poetry.lock || poetry lock --no-update $(VENV)/bin/python: $(PYTHON_VERSIONS_PATH)/$(PYTHON_VERSION)/bin/python @ [[ "$(shell python --version)" = "Python $(PYTHON_VERSION)" ]] || { \ echo 'Expected Python $(PYTHON_VERSION) in $$PATH' >& 2; \ exit 1; \ } poetry env use python $(PYTHON_VERSIONS_PATH)/$(PYTHON_VERSION)/bin/python: $(PYTHON_INSTALL_CMD) $(PYTHON_VERSION) ``` This greatly simplifies handling many workflow states. In all cases: * The lockfile changed and venv needs to sync * I just freshly downloaded the project * The required Python version isn't installed * I want to run tests * I changed pyproject.toml Just run `make`. These changes to `add` and `remove` speed up the "dep search iteration loop" by avoiding unnecessary `lock --check` and `lock --no-update` calls after changing `pyproject.toml` Resolves: python-poetry#7392
😞 Any chance this could be considered a "bug" b/c #75 (comment) seems to suggest this behavior used to be the case, long ago? |
This is far from being considered a bug, and the issue you are pointing at is also a "feature request", so the answer is no. |
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
A common annoyance when managing deps is having long sessions of
repeated iteration loops of
add
,remove
, tweakingpyproject.toml
+lock --no-update
+install --sync
, re-running tests, trying to finda configuration that works.
GNU
make
can implement "automated install" behavior, simplifying theloop to: "save any change,
make
, repeat if needed". Consider:This greatly simplifies handling many workflow states. In all cases:
Just run
make
.These changes to
add
andremove
speed up the "dep search iterationloop" by avoiding unnecessary
lock --check
andlock --no-update
calls after changing
pyproject.toml
Resolves: #7392
Pull Request Check List
Resolves: #7392