Changes in this commit should be limited to:
* lines changing order (they are sorted now)
* jaraco.text, jaraco.collections, autocommand, inflect, typeguard are
gone now (setuptools vendored packages that were being included from
the venv)
* comments added showing why dependencies are included
Substring matches when you are actually only trying to match one
specific item is a smell, its unclear what's intended and causes the
reader to have to stop and think.
This is for parsing diffs like:
# via importlib-metadata
+
+ # The following packages were excluded from the output:
+ # setuptools
Otherwise further parsing breaks because it is looking for a package
name on the line.
see 433074c681, adf39e9f72, db83a82fe1 & 78a74a2e2a
Now that we are using pip-compile to build requirements lock files,
instead of pip-freeze, these things shouldn't be showing up in the
results.
Looks like `typeguard` was dropped in a192a3d9c7d0e87 "initial compile based bump"
I guess that was being pulled in by pip freeze too.
I find these comments useful to show why packages are included in the
final compiled requirements files.
Required a small change to `recompile_requirements` to ignore these new
comment line (it was saying there was a new requirement with an empty
name that needed a changelog entry).
The addition of the `os.path.realpath()` is to clean up the paths of the
requirements files in the annotations, eg:
check-manifest==0.49
- # via -r scripts/dev/../../misc/requirements/requirements-check-manifest.txt-raw
+ # via -r misc/requirements/requirements-check-manifest.txt-raw
It would have been convenient to have an end2end test to make sure that
the output of the two requirements file compilation methods had the same
results. But I think there is a bit too much stuff going on in
`recompile_requirements` for that atm.
Making the local repo in the temp path and install fake packages there
works fine, although setting up the virtualenv isn't quick. But the
script currently installs pip, setuptools and wheel which means we have
to either 1) hit pypi.org in the tests to get them, or 2) download them at
the start of the test suite and put them in the local repo.
Not sure it's worth the effort to go down this rabbit hole when we
already have a dozen real requirements files to verify the change with.
I'm leaving this in the commit history because it was fun to get the
local repo working!
For the pip freeze backend pip is being passed `--all` when the tox
requirements file is being processed so that pip and setuptools are
included in the requirements file. This was added in 922dca039b
for reasons I haven't fully grokked.
This commit adds similar behaviour for the pip compile backend via:
1. don't add the --no-emit-package args for the tox requirements file
2. add pip and setuptools to the tox requirements file
It seems that pip and setuptools aren't even requirements of tox, but
they are being included in the compiled requirements file anyway. Why
aren't the included in the raw requirements file? I don't know, but from
what I can figure it's not going to harm anything to have them in there.
This is to match the `pip freeze` requirements compilation method. It's
not clear to me if we actually want this behaviour or not.
If seems `pip freeze` will exclude dependencies of itself: https://pip.pypa.io/en/stable/cli/pip_freeze/#cmdoption-all
Even if there are other packages installed that depend on those
dependencies.
`uv pip compile`, and now the original `pip-compile` both have decided
to include setuptools in generated requirements files:
https://github.com/astral-sh/uv/issues/1353https://github.com/jazzband/pip-tools/issues/989#issuecomment-1134985118
So I'm not sure if we have a reason for going against this here or if
they were just being excluded because that's what pip freeze does.
Hopefully we can drop this commit and use the default behaviour in the
future. For now when I'm trying to provide the new backend it's here to
make the diff of generated files more precise.
This message prefix to identify a pip compile comment was taken from
these examples:
# The following packages were excluded from the output:
# setuptool
# The following packages are considered to be unsafe in a requirements file:
# setuptools==41.4.0 # via protobuf
`pip freeze` writes out package names as specified by the packages, `pip
compile` writes out normalized package names. Sticking to normalized
names in robot written files lets us more easily compare the output of
the two different requirement compiling methods and also mean we can
stop worrying about packages changing between `_` and `-` in dependency
updates.
Script used to do this change was:
import re
import glob
def normalize_pkg(name):
"""Normalize a package name for comparisons.
From https://packaging.python.org/en/latest/specifications/name-normalization/#name-normalization
`pip freeze` passes file names though in whatever case they are in in the
package, pip-compile will normalize them.
"""
if "/" in name: # don't change file paths
return name
return re.sub(r"[-_.]+", "-", name).lower()
def normalize_line(line):
if not line or not line.strip():
return line
if "==" not in line.split()[0]:
return line
pkg, version = line.split("==", maxsplit=1)
return "==".join([normalize_pkg(pkg), version])
for name in ["requirements.txt"] + glob.glob("misc/requirements/requirements*.txt"):
with open(name) as f:
before_lines = f.readlines()
after_lines = [normalize_line(line) for line in before_lines]
with open(name, mode="w") as f:
f.writelines(after_lines)
Since I was looking at how hard it would be to support using pip-compile
to recompute requirements, I was worried that I would break the markers
we support in the raw requirements files.
This adds two tests:
* disabled_test_markers_real_pip_and_venv
A test that sets up a local python index and runs the real pip/uv
binaries. It works (and it was fun to setup a package index factory)
but has a couple of downsides:
1. it hits the real pypi index, which is not great in a test. This can
be prevented by removing the EXTRA bit from the INDEX_URL env vars
and pre-downloading pip, wheel, setuptools and uv to the test repo
(and adding index.htmls for them). But because of the next item I'm
not sure it's worth the effort of keeping this test around
2. it's slow because of having to download packages from the internet
(even if we pre-cached them it would still have to download them, I
guess we could include a zip of fixed/vendored versions, but that
will probably require maintenance over time) and because it cals
venv to make new virtual environments, which isn't the quickest
operation (maybe uv venv is quicker?)
* test_markers_in_comments
Tests just the comment reading and line manipulation logic. Could be
made a bit more pure by just calling read_comments() and
convert_line(), but that wouldn't test that "add" marker.
There was a fair bit of duplicate code, so I've pulled out the "take a
list of requirements, give me a new one" out to separate methods. The
stuff around parsing the output can stay common thankfully!
Even if we drop one of the methods this level of abstraction is probably
fine to keep.
The CHANGELOG_URLS variable is imported by the
`./scripts/dev/misc_checks.py changelog-urls` check. Since there is a
bit of churn in package name case in this change (and a while back when
a bunch of packages switched between underscores and hyphens), update
this variable at import time so that that checker will be looking at
normalized names too.
In #8269 we saw some packages leaking into the pip freeze output that we
don't want in the requirements files (if setuptools isn't supposed to be
in there, why should its dependencies).
I've also greatly missed the comments that pip-compile puts in
requirements.txt files explaining where indirect dependencies come from.
So I took the opportunity to switch our tooling for updating and parsing
new dependencies and their versions to use pip-compile instead of `pip -U
install && pip freeze`.
It turned out to not be a big change because the pip freeze output is
largely compatible with requirements files (we are writing directly to
one after all). So we just need to switch what commands we are running
and triage any compatibility issues.
I chose `uv pip compile` instead of `pip-compile` because I like what uv
is doing (fast, aiming for compatibility, consolidating a confusing
ecosystem into a single tool). But pip-compile/tools should do the same
job if we want to go that route.
The biggest differences are:
* outputs normalized names: this generally results in a larger diff than
otherwise (I propose we go through an regenerate all the requirements
files in one go, and maybe add that commit to a blame ignore file) and
requires our comparison logic to deal with normalized package names
everywhere
* setuptools and pip not included in tox requirement file - not sure
what to do about that yet, should they be in the .text-raw file?
TODO:
* remove support for pip_args?
* change markers in raw files to lower case? Ideally don't require, if a human
can write them in any case and a robot can normalize we should do that. If
if there are patterns with `._` in them as part of names, how do we handle
that?
* pull out similar bits of `build_requirements*` methods
* maybe make it so you can pass `requirements=None` to `init_venv` to
make it not install stuff, install uv, do the uv invocation, gate
all that behind a `--method="freeze|compile"` arg?
* add pip and setuptools to tox requirements file?
* basename requirements file names so they don't have
* `script_path/../../` in them in the annotated version
* add tests for the markers (with inputs of differing cases) to make
sure they all still work
* update changelog check used in CI to normalize names too
Skip this hypothesis version pending https://github.com/HypothesisWorks/hypothesis/issues/4375
Our test suite is currently failing due to running python with `-b` and
being configured to fail on warnings.
We'll pick it up on the next update run or so.
The previous fix in 3dc212a815 was insufficient,
as the inner `getattr(extract_result, "registered_domain")` was always evaluated
first (thus triggering the deprecation warning again).
We also cannot do:
getattr(extract_result, "top_domain_under_public_suffix", None) or extract_result.registered_domain
as `""` is a valid value for it.
Speculative fix for test_early_timeout_handler in
tests/unit/utils/usertypes/test_timer.py failing:
> assert len(caplog.messages) == 1
E AssertionError: assert 5 == 1
due to:
------------------------------ Captured log call -------------------------------
WARNING misc:usertypes.py:467 Timer download-update (id 620757000) triggered too early: interval 500 but only -609.805s passed
WARNING misc:usertypes.py:467 Timer download-update (id 922746881) triggered too early: interval 500 but only -609.429s passed
WARNING misc:usertypes.py:467 Timer download-update (id 1056964613) triggered too early: interval 500 but only -609.537s passed
WARNING misc:usertypes.py:467 Timer download-update (id 1912602631) triggered too early: interval 500 but only -609.671s passed
WARNING misc:usertypes.py:467 Timer t (id -1) triggered too early: interval 3 but only 0.001s passed
We sometimes tried to use hints before the page was fully rendered (?), thus
causing no elements to be found.
It also doesn't make much sense to test leaving insert mode if we aren't in
insert mode yet, so make sure we entered it first.
See #5390
toofar
qutebrowser bot
Florian Bruhin