#453 Add command to check poetry.lock freshness

[cliebBS]

Fixes #453

This PR adds a new command, poetry lock --check, which returns 0 if poetry.lock is consistent with pyproject.toml and 1 if it is not. This is useful for CI/CD tooling to have a very fast way of checking the lockfile before building without the re-resolution of the dependency tree that poetry lock performs.

Pull Request Check List

This is just a reminder about the most common mistakes. Please make sure that you tick all appropriate boxes. But please read our contribution guide at least once, it will save you unnecessary review cycles!

• Added tests for changed code.
• Updated documentation for changed code.

Note: If your Pull Request introduces a new feature or changes the current behavior, it should be based
on the develop branch. If it's a bug fix or only a documentation update, it should be based on the master branch.

If you have any questions to any of the points above, just submit and ask! This checklist is here to help you, not to deter you from contributing!

[finswimmer]

Nice 👍

Because this is a new feature @sdispater will decide if this get included.

Thanks a lot for your contribution!

[k4nar]

As there is already a return in the previous if, I think it would be better to remove this else. It will help with further additions to the command.

[cliebBS]

Just to make sure I understand, you would prefer the following?

Suggested change

	return installer.run()
	if self.option("check"):
	return (expr)
	installer.lock()
	return installer.run()

[k4nar]

Yes exactly 👍 .

[cliebBS]

Done!

[k4nar]

👍

[cjw296]

Hmm, sorry for the comment I deleted, I wasn't aware this was actually related to #453 , which I posted! :-)

So, there's a lot of checking I want to do in one easy place:

• check the lock file matches the constraints specified in pyproject.toml
• check the environment poetry is managing (usually a virtualenv in the cache) matches the poetry.lock file.

The latter might seem paranoid, but with venv in a project it's easy sometimes to forget to use poetry and just pip install something. Also, I often work with a flaky network connection, and if poetry is struggling, I frequently need to Ctrl-C, and I'm never sure what state the virtualenv has been left in.

[seansfkelley]

Would this flag also check things like content-hash being up-to-date if someone changes non-dependency-related metadata like the owner or description?

What's the status of this PR? I'd love to have it!

[tetienne]

Hi, any update on this really nice PR?

[briggySmalls]

Would love this feature!

[nicoleclearmetal]

Any reason this hasn't been merged?

[thejcannon]

I'd love to see this feature as well, anything the community can do to help it along?

The base branch was changed.

[abn]

@cliebBS any chance you can get this rebased and conflicts resolved please? Additionally, would be great to have a test case for this one.

[cliebBS]

I've fixed the merge conflict, the only test failure was a network error on one of the FreeBSD builds during test setup, unrelated to my change. I can't figure out how to rerun the test, as I'm sure it would pass without issue.

As for the tests that you've asked for, I'm not sure how to setup a test for the lock command as there are no pre-existing examples for it. I thought that the install command could yield some useful info on how to write a test for lock, but it doesn't have any tests either.

I have tested my changes manually by first running poetry lock --check against the pyproject.toml and poetry.lock on my branch, which correctly returned 0. I then updated the required Python version to be ^2.7 || ^3.6 and reran poetry lock --check, which resulted in the expected return value of 1. I then ran poetry lock followed by poetry lock --check and saw that it now returned the expected 0.

[yardensachs]

Whats blocking this PR so far?

[abn]

There is a fresher version of this change at #3106.

@cliebBS this got lost in the notifications. Can you rebase this etc.?

[cliebBS]

@finswimmer I've merged master and rewritten my test to match the new test pattern used in the other test in test_lock.py

[finswimmer]

Thanks a lot for your contribution 👍 Looks good to me. Just one minor question about a test.

[finswimmer]

What's happening under windows? Do you know why?

[cliebBS]

That was many moons ago and I've totally forgotten why I added it. I'm going to try removing it to see if the test works on Windows now.

[cliebBS]

Hey, wha'd'ya know? Totally unnecessary anymore :)

[finswimmer]

Perfect 👍 Could you please add a test for a non outdated lock file as well?

[cliebBS]

I've added a new test and refactored the testing a bit. I keep getting failures on the FreeBSD/Py36 tests while installing dependencies, though. All of the other Py36 testing is passing, so I can only figure that something is broken on that test runner.

[cliebBS]

Ok, today tests are all passing, including the new test :)

[finswimmer]

LGTM 👍 Thanks a lot!

[finswimmer]

github is telling me it cannot merge due to conflicts, but it seems to be a secrets what conflicts 🤔 Can you please rebase once more?

[cliebBS]

I just tried and there are no new commits on upstream/master to merge in. Also, I'm seeing on the PR that there are no confiicts 🤔

[1ace]

I think an actual git rebase (instead of a series of git merge crossing back) might help.

@cliebBS as your individual commits don't seem to carry much meaning (beyond just "these are the steps in which I tried things"), I suggest you squash all of them to avoid conflicts when rebasing.

Specifically, that means:

• assuming your upstream remote is origin, start with a git fetch origin to get the latest commits
• git rebase -i origin to initiate an interactive rebase on top of the default branch on that remote. "interactive" means git will open your configured editor and present you with a list of actions to perform on each of your commits.
• all the actions are pick by default, which means "keep the commit as is". You'll want to keep that for the first commit, and replace all the ones after with fixup (or just f), which will squash them into the first commit.
• save and exit, and git will perform as instructed
• the end result will be your commit, rebased on top of the current latest version of upstream's default branch

The reason I'm worried about conflicts (that you might not know how to handle) is that you've been doing a lot of back and forth with your commits, doing things in a way that's incompatible with upstream then fixing them later, which will result in a conflict when doing the wrong thing and another one between the resolution of that conflict and correcting to the right thing. A general rule for making rebases painless is to fix your commits instead of leaving the issues and adding other commits on top.

One last thing, in interactive rebases, removing a commit from the list actually removes the commit from your branch, so make sure you don't remove something thinking that will "make git ignore that commit" or something. I've seen enough colleagues make that mistake to always point this out when presenting rebase to someone ^^

[cliebBS]

Ok, everything has been squashed down to a single commit and all tests are passing :)

[sdispater]

Sorry for chiming in late but I think we need to revert and change this :-(

From a pure API/CLI standpoint it does not make sense to have an option modify the core behavior of a command: the lock command is meant to, well, lock the dependencies and not to check anything about it.

It would be much more suited as an option or subcommand of the check command, I think.

[nicoleclearmetal]

@sdispater there are a lot of CLIs that do this, some examples:

black --check

  --check                         Don't write the files back, just return the
                                  status.  Return code 0 means nothing would
                                  change.  Return code 1 means some files
                                  would be reformatted. Return code 123 means
                                  there was an internal error.

isort --check

  -c, --check-only, --check
                        Checks the file for unsorted / unformatted imports and prints them to the command line without modifying the file. Returns 0 when nothing would change and returns 1
                        when the file would be reformatted.

so I think it's a pretty common flag that users would be familiar with

[sdispater]

@nicoleclearmetal The examples you give are not really relevant to this case since the --check option itself acts as a command. Neither black nor isort have sub commands so they have to have a way to enable this "check only" behavior.

Poetry has a sub command system and also has a check command, so it would be better for discoverability to uses these as much as possible.

[max-wittig]

@sdispater This option doesn't seem to be available in poetry 1.1.5. As it's kind of important in a production environment, are there any plans to create a new release with this feature? It's security relevant.

[2m]

Has this feature been reverted before releasing 1.1.6? I do not see this PR in 1.1.5...1.1.6

[Crocmagnon]

Has this feature been reverted

No, the changes are still on master

I do not see this PR in 1.1.5...1.1.6

Seems OK to me, the feature is in master, not in the 1.1 branch. Furthermore, patch bump should only contain bug fixes as per semver. We should see this feature in 1.2.0 hopefully 🙂

[2m]

Ahh. Sorry for the noise then. :) I thought since it was in master, it was released.

[github-actions]

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.