Skip to content

build: let's start pinning dev requirements (?) #1813

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

Closed
jku opened this issue Feb 1, 2022 · 5 comments · Fixed by #1867
Closed

build: let's start pinning dev requirements (?) #1813

jku opened this issue Feb 1, 2022 · 5 comments · Fixed by #1867
Assignees
Labels
backlog Issues to address with priority for current development goals

Comments

@jku
Copy link
Member

jku commented Feb 1, 2022

Issue #1811 is an example of build failing when it succeeded just moments before: A new black release was made that changes whitespace rules for a specific situation.

We may want to start pinning dev-requirements as well: this increases the amount of work (as we would start getting more dependabot PRs for these requirements) but it would mean our build would be less prone to spontaneously change. Whether we can pin the all transient build/test/release requirements is another question: there might be a lot of it?

@joshuagl opinions?

@MVrachev
Copy link
Collaborator

MVrachev commented Feb 1, 2022

I think it's better to have more dependabot prs compared to a sudden failure in a long discussed pr because of an unexpected new change in one of our linters.
Maybe we can pin only our linters for a start? As they are the main packages responsible for the unexpected failures.

@joshuagl
Copy link
Member

joshuagl commented Feb 1, 2022

I agree with you both, our test environment should be deterministic too. CI failing because upstream made a new release we didn't account for is bad. CI doing bad things because i.e. black was compromised and made a malicious release would not be great either.

@jku
Copy link
Member Author

jku commented Feb 9, 2022

Documenting the current "non-library-runtime" requirements sets we have:

  • requirements-test: unit test requirements like linters, installed inside tox environment
  • requirements-docs: doc build requirements, installed inside tox environment
  • requirements-dev: the package build requirements (but this also includes requirements-test and is documented as the generic "developer requirements" file)

It might make sense to decouple package build requirements and what we suggest developers install: controlling the package build environment seems like priority 1 if we want to automate that process.

@jku jku added the backlog Issues to address with priority for current development goals label Feb 9, 2022
@lukpueh
Copy link
Member

lukpueh commented Feb 14, 2022

Two more thoughts:

  1. Should we flatten out the files -- requirements-{test, docs, dev}.txt? If you run pip-compile on either of them, it will also list all the requirements included via -r or -e, e.g.:

    -r requirements-test.txt
    -e .

    Or should we pip-compile (pin, add transitive deps) of the direct deps in the file, e.g:
    build
    tox
    twine
    wheel

  2. Should we pin for multiple versions? I just ran pip-compile for requirements-dev.txt for each Python version we support in runtime, and 3.7 resolves to a few more than the newer ones.
    I guess for dev we don't need to pin for all, because we can expect developers to use a given new version of Python, but for tests we should, because we run tests on all those Python versions in CI.

@lukpueh
Copy link
Member

lukpueh commented Feb 15, 2022

#1867 only pins test requirements. This should fix issues with breaking CI on upstream changes of testing tools like #1811, which seemed to be the original motivation of this issue.

Regarding the other requirements files:

  • I didn't pin requirements-docs.txt because I was unsure for which Python version to pin - on the RTD interface we can only select 2.x or 3.x - and the results of pip-compile were different for different versions. An unexpected update of docs dependencies (sphinx and sphinx-rtd-theme) can still break our CI because RTD builds are part of it, but I don't think they are very likely.
  • I also didn't pin requirements-dev.txt, because devs should be able to use whatever Python and tooling versions they want. The package build part of requirements-dev is a different story (I think the only requirement there is build?). Should we create a new ticket for that? Or fix it as part of Automated release builds #1550 or Verifiably reproducible build artefacts #1269)?

At any rate, I think we can close this issue with #1867

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog Issues to address with priority for current development goals
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants