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

Add ability to check if lockfiles are up-to-date with all declared requirements #15723

Open
danxmoran opened this issue Jun 1, 2022 · 9 comments
Assignees
Labels
backend: Python Python backend-related issues enhancement

Comments

@danxmoran
Copy link
Contributor

Is your feature request related to a problem? Please describe.

I'd like to add a step to my team's CI pipeline to assert that our lockfiles are up-to-date. My first thought was to run ./pants generate-lockfiles and then assert on something like git status --porcelain. This doesn't work because ./pants generate-lockfiles isn't idempotent - it will eagerly bump versions of dependencies when new versions are released that satisfy our requirement constraints.

Describe the solution you'd like

Quoting @stuhood from #15704:

This could/should probably be covered by a ./pants generate-lockfiles --check mode which validates that the inputs for the lockfile are unchanged (in internals terms, that's having the same "lockfile metadata", which is stored in the header of the lockfile).

@jsirois
Copy link
Contributor

jsirois commented Jun 5, 2022

Pex supports this sort of check now via pex3 lock update --dry-run check lock.json. This is different from what @stuhood was getting at though. His suggestion just checks if the lock file was created with different root requirements than currently exist in the repo. This Pex check checks if the 3rdparty universe has advanced for the requirements written down in the lock file; i.e.: would a lock created today from the same requirements be different?

@danxmoran
Copy link
Contributor Author

@jsirois yeah, that's a different kind of check than what I'm looking for (and it can already be replicated by regenerating lockfiles via pants, though maybe it's less efficient that way). I don't want people to get dinged on PR checks because an existing transitive dependency has released a new version, but I do want them to get dinged if they changed a python_requirement and didn't regenerate the associated lock.

@jsirois
Copy link
Contributor

jsirois commented Jun 5, 2022

I will note though, that for that it is a bit odd Pants doesn't auto-regen locks. If you update requirements that are inputs to a lock, it seems to me any action that reads the lock should trigger an auto update. I can't see where you'd ever not want that.

@danxmoran
Copy link
Contributor Author

Even if (maybe especially if) the auto-update was implemented, we'd want to include a lock-check in CI to cover the case where somebody's:

  1. In a rush
  2. Commits a change to a requirements.txt
  3. Pushes the change to have CI run all the affected tests, so they don't have to burn local time running them

If Pants was auto-regenerating locks then I'd think ./pants test etc. wouldn't complain about the outdated locks in this scenario - the locks would regenerate on the CI machines, but remain outdated in git. We'd need the dedicated check to assert that the checked-in state was actually up-to-date.

@jsirois
Copy link
Contributor

jsirois commented Jun 6, 2022

Agreed. We apparently don't worry about this same scenario since, for example, there is cargo check --locked and we don't use it, but makes sense.

@rafrafek
Copy link

rafrafek commented Feb 8, 2023

Poetry solves it by providing 3 options:

  • poetry lock --check - Verify that poetry.lock is consistent with pyproject.toml. This is very helpful in CI/CD.
  • poetry lock --no-update - Do not update locked versions, only refresh lock file. This is very helpful since the purpose of a lock file is to have some stability and repeatability of the environment.
  • poetry lock - Update lock file and update versions of dependencies & sub-dependencies. By default, this will lock all dependencies to the latest available compatible versions. This is very helpful since updating all versions manually would be very time consuming.

Which of these does Pants have now?

@danxmoran
Copy link
Contributor Author

Which of these does Pants have now?

pants generate-lockfiles is essentially the same as poetry lock.

#15704 is the ticket to add the equivalent of --no-update, if I understand correctly.

Personally I'd prefer for generate-lockfiles to default to --no-update behavior, and have something like an --update or --full flag to run the full regeneration.

@rafrafek
Copy link

rafrafek commented Feb 9, 2023

Personally I'd prefer for generate-lockfiles to default to --no-update behavior, and have something like an --update or --full flag to run the full regeneration.

100% agree! I think Pants (and Poetry too) should default to --no-update.

@huonw
Copy link
Contributor

huonw commented Dec 1, 2023

I had an idea for an approach to this: have this checking behaviour run for _lockfile targets on test, lint or check goal (I don't know which one fits best).

It appears that such targets are already added implicitly for the lockfile files, so this would mean a typical CI command like pants test :: would cover it automatically, without needing to add yet another command for CI. If we go with generate-lockfiles --check We'd have a recommended command of something like pants tailor --check update-build-files --check generate-lockfiles --check lint check test package ::... which is a lot!

Downside: these targets do not currently depend on the input python_requirement targets, so we might need to change that to ensure the test reruns appropriately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend: Python Python backend-related issues enhancement
Projects
Status: No status
Development

No branches or pull requests

7 participants