Restore requirements-txt-fixer in pre-commit #7808

alexrudd2 · 2023-11-09T02:33:30Z

What do these changes do?

Repair a useless pre-commit hook, since requirements.txt was removed in #6165

This can be verified with pre-commit run --all-files.

Are there changes in behavior for the user?

No

Checklist

I think the code is well written
Unit tests for the changes exist
Documentation reflects the changes
If you provide code modification, please add yourself to CONTRIBUTORS.txt
- The format is <Name> <Surname>.
- Please keep alphabetical order, the file is sorted by names.
Add a new news fragment into the CHANGES folder
- name it <issue_id>.<type> for example (588.bugfix)
- if you don't have an issue_id change it to the pr id after creating the pr
- ensure type is one of the following:
  - .feature: Signifying a new feature.
  - .bugfix: Signifying a bug fix.
  - .doc: Signifying a documentation improvement.
  - .removal: Signifying a deprecation or removal of public API.
  - .misc: A ticket has been closed, but it is not of interest to users.
- Make sure to use full sentences with correct case and punctuation, for example: "Fix issue with non-ascii contents in doctest text files."

.pre-commit-config.yaml

This reverts commit 2504166d8465a1ef47571443fc99bbabfdf39a26.

alexrudd2 · 2023-11-27T20:39:46Z

The Github PR outages were throwing off my pushes for a bit, but I believe the branch/PR are now ready again.

Dreamsorcerer · 2023-11-27T20:44:17Z

Well, that's awkward, it requires setup.cfg to have the same ordering.

alexrudd2 · 2023-11-27T20:47:15Z

They are pip-compile/dependabot things. We had to invert the .txt files to get dependabot to work with constraints.

Aside: It's unclear to me if dependabot has been updated to support setup.cfg.

Have you considered renovatebot? It handles setup.cfg and I've generally found it far more powerful than dependabot.

codecov · 2023-11-27T20:50:09Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (5f64328) 97.42% compared to head (da8069e) 97.41%.
Report is 96 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7808      +/-   ##
==========================================
- Coverage   97.42%   97.41%   -0.01%     
==========================================
  Files         106      107       +1     
  Lines       32111    32347     +236     
  Branches     3726     3753      +27     
==========================================
+ Hits        31283    31511     +228     
- Misses        626      632       +6     
- Partials      202      204       +2

Flag	Coverage Δ
CI-GHA	`97.33% <ø> (-0.01%)`	⬇️
OS-Linux	`97.00% <ø> (-0.02%)`	⬇️
OS-Windows	`95.50% <ø> (-0.01%)`	⬇️
OS-macOS	`96.62% <ø> (-0.07%)`	⬇️
Py-3.10.11	`95.42% <ø> (-0.01%)`	⬇️
Py-3.10.13	`96.80% <ø> (-0.07%)`	⬇️
Py-3.11.6	`96.51% <ø> (-0.03%)`	⬇️
Py-3.12.0	`96.58% <ø> (-0.02%)`	⬇️
Py-3.8.10	`95.39% <ø> (-0.01%)`	⬇️
Py-3.8.18	`96.73% <ø> (-0.07%)`	⬇️
Py-3.9.13	`95.39% <ø> (-0.01%)`	⬇️
Py-3.9.18	`96.77% <ø> (-0.07%)`	⬇️
Py-pypy7.3.11	`?`
Py-pypy7.3.13	`96.23% <ø> (?)`
VM-macos	`96.62% <ø> (-0.07%)`	⬇️
VM-ubuntu	`97.00% <ø> (-0.02%)`	⬇️
VM-windows	`95.50% <ø> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Dreamsorcerer · 2023-11-27T20:59:44Z

Have you considered renovatebot? It handles setup.cfg and I've generally found it far more powerful than dependabot.

I don't think we want it to run on that file. Though there are some other nice features, like git tags, which could be useful...

Dreamsorcerer · 2023-11-27T21:03:47Z

I think we need a sorted() call in that horrific line of Python:
https://github.com/aio-libs/aiohttp/blob/master/Makefile#L192

alexrudd2 · 2023-11-27T21:31:23Z

I think we need a sorted() call in that horrific line of Python: https://github.com/aio-libs/aiohttp/blob/master/Makefile#L192

Yeah, although that's not sufficient since the two lists have to be merged before sorting. That line is amazingly awful :)

I don't think we want it to run on that file. Though there are some other nice features, like git tags, which could be useful...

Oh, the idea would be to totally replace dependabot and therefore all these files.

alexrudd2 · 2023-11-27T21:55:32Z

Here's my work in progress, which works as a standalone python file but not yet in the makefile.

from configparser import ConfigParser
from pathlib import Path
cfg = ConfigParser()
cfg.read(Path("setup.cfg"))
reqs = cfg['options']['install_requires'] + cfg.items('options.extras_require')[0][1]
reqs = sorted(reqs.split('\n'), key=str.casefold)
reqs.remove('')
print('\n'.join(reqs))

Dreamsorcerer · 2023-11-27T22:48:18Z

I don't think we want it to run on that file. Though there are some other nice features, like git tags, which could be useful...

Oh, the idea would be to totally replace dependabot and therefore all these files.

Yes, I just have some future things I've been thinking about that'd involve dependabot or alternatives bumping a git submodule as a dependency.

alexrudd2 · 2023-11-27T22:49:45Z

I've updated the branch with a fix I'm happy with, but it's not showing in the GitHub PR.

https://github.com/alexrudd2/aiohttp/commit/663f5daad40d541d31c6383a3fd5a973e010470f

for more information, see https://pre-commit.ci

requirements/sync-direct-runtime-deps.py

patchback · 2023-11-27T23:20:22Z

Backport to 3.9: 💔 cherry-picking failed — conflicts found

❌ Failed to cleanly apply 213d1b2 on top of patchback/backports/3.9/213d1b22d42ce8efc54d7858b490e920fcdb4f0a/pr-7808

Backporting merged PR #7808 into master

Ensure you have a local repo clone of your fork. Unless you cloned it
from the upstream, this would be your origin remote.
Make sure you have an upstream repo added as a remote too. In these
instructions you'll refer to it by the name upstream. If you don't
have it, here's how you can add it:
```
$ git remote add upstream https://github.com/aio-libs/aiohttp.git
```

Ensure you have the latest copy of upstream and prepare a branch
that will hold the backported code:

$ git fetch upstream
$ git checkout -b patchback/backports/3.9/213d1b22d42ce8efc54d7858b490e920fcdb4f0a/pr-7808 upstream/3.9

Now, cherry-pick PR Restore requirements-txt-fixer in pre-commit #7808 contents into that branch:
```
$ git cherry-pick -x 213d1b22d42ce8efc54d7858b490e920fcdb4f0a
```
If it'll yell at you with something like fatal: Commit 213d1b22d42ce8efc54d7858b490e920fcdb4f0a is a merge but no -m option was given., add -m 1 as follows instead:
```
$ git cherry-pick -m1 -x 213d1b22d42ce8efc54d7858b490e920fcdb4f0a
```
At this point, you'll probably encounter some merge conflicts. You must
resolve them in to preserve the patch from PR Restore requirements-txt-fixer in pre-commit #7808 as close to the
original as possible.

Push this branch to your fork on GitHub:

$ git push origin patchback/backports/3.9/213d1b22d42ce8efc54d7858b490e920fcdb4f0a/pr-7808

Create a PR, ensure that the CI is green. If it's not — update it so that
the tests and any other checks pass. This is it!
Now relax and wait for the maintainers to process your pull request
when they have some cycles to do reviews. Don't worry — they'll tell you if
any improvements are necessary when the time comes!

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback · 2023-11-27T23:20:28Z

Backport to 3.10: 💔 cherry-picking failed — conflicts found

❌ Failed to cleanly apply 213d1b2 on top of patchback/backports/3.10/213d1b22d42ce8efc54d7858b490e920fcdb4f0a/pr-7808

Backporting merged PR #7808 into master

Ensure you have a local repo clone of your fork. Unless you cloned it
from the upstream, this would be your origin remote.
Make sure you have an upstream repo added as a remote too. In these
instructions you'll refer to it by the name upstream. If you don't
have it, here's how you can add it:
```
$ git remote add upstream https://github.com/aio-libs/aiohttp.git
```

Ensure you have the latest copy of upstream and prepare a branch
that will hold the backported code:

$ git fetch upstream
$ git checkout -b patchback/backports/3.10/213d1b22d42ce8efc54d7858b490e920fcdb4f0a/pr-7808 upstream/3.10

Now, cherry-pick PR Restore requirements-txt-fixer in pre-commit #7808 contents into that branch:
```
$ git cherry-pick -x 213d1b22d42ce8efc54d7858b490e920fcdb4f0a
```
If it'll yell at you with something like fatal: Commit 213d1b22d42ce8efc54d7858b490e920fcdb4f0a is a merge but no -m option was given., add -m 1 as follows instead:
```
$ git cherry-pick -m1 -x 213d1b22d42ce8efc54d7858b490e920fcdb4f0a
```
At this point, you'll probably encounter some merge conflicts. You must
resolve them in to preserve the patch from PR Restore requirements-txt-fixer in pre-commit #7808 as close to the
original as possible.

Push this branch to your fork on GitHub:

$ git push origin patchback/backports/3.10/213d1b22d42ce8efc54d7858b490e920fcdb4f0a/pr-7808

Create a PR, ensure that the CI is green. If it's not — update it so that
the tests and any other checks pass. This is it!
Now relax and wait for the maintainers to process your pull request
when they have some cycles to do reviews. Don't worry — they'll tell you if
any improvements are necessary when the time comes!

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

alexrudd2 · 2023-11-27T23:23:25Z

🎉 Thanks for the help.

Do you care about the backports?

Dreamsorcerer · 2023-11-27T23:32:39Z

Yeah, if you could handle those, that'd be great. I'll be back on Wednesday to merge them.

…e-commit (#7920) Backport of #7808 cherry picked from commit 213d1b2

…re-commit (#7921) Backport of #7808 cherry picked from commit 213d1b2

webknjaz · 2023-11-30T16:09:24Z

.pre-commit-config.yaml

@@ -45,8 +45,7 @@ repos:
    exclude: >-
      ^docs/[^/]*\.svg$
  - id: requirements-txt-fixer
-    exclude: >-
-      ^requirements/.*\.txt$
+    files: requirements/.*\.in$


@Dreamsorcerer honestly, I'd delete this formatter altogether. It's meant to make pip freeze output predictable. But our .in files are crafted manually, and I would rather structure them manually too. Besides, IIRC such hooks don't play well with comments in the input files.

webknjaz · 2023-11-30T16:10:34Z

requirements/test.in

 -c broken-projects.in
+-r base.in


Does this change the order pip loads the files? What is the effect of it?

pip install installation order

As of v6.1.0, pip installs dependencies before their dependents, i.e. in “topological order.” This is the only commitment pip currently makes related to order. While it may be coincidentally true that pip will install things in the order of the install arguments or in the order of the items in a requirements file, this is not a promise.

Remove requirements-txt-fixer from pre-commit

1d102d5

alexrudd2 requested review from webknjaz and asvetlov as code owners November 9, 2023 02:33

Dreamsorcerer reviewed Nov 27, 2023

View reviewed changes

.pre-commit-config.yaml Show resolved Hide resolved

alexrudd2 added 3 commits November 27, 2023 14:25

Revert "Remove requirements-txt-fixer from pre-commit"

1778f99

This reverts commit 2504166d8465a1ef47571443fc99bbabfdf39a26.

Fix requirements-txt-fixer to support *.in files

d99e0a8

pre-commit run --all-files

2bbf40c

Dreamsorcerer added bot:chronographer:skip This PR does not need to include a change note backport-3.9 backport-3.10 Trigger automatic backporting to the 3.10 release branch by Patchback robot labels Nov 27, 2023

alexrudd2 changed the title ~~Remove requirements-txt-fixer from pre-commit~~ Restore requirements-txt-fixer in pre-commit Nov 27, 2023

Sort setup.cfg

da8069e

Split out Python code from makefile

663f5da

Dreamsorcerer closed this Nov 27, 2023

Dreamsorcerer reopened this Nov 27, 2023

[pre-commit.ci] auto fixes from pre-commit.com hooks

3424683

for more information, see https://pre-commit.ci

Dreamsorcerer reviewed Nov 27, 2023

View reviewed changes

requirements/sync-direct-runtime-deps.py Show resolved Hide resolved

Update requirements/sync-direct-runtime-deps.py

6d8af9e

Dreamsorcerer merged commit 213d1b2 into aio-libs:master Nov 27, 2023
30 of 33 checks passed

This was referenced Nov 28, 2023

[PR #7808/213d1b2 backport][3.9] Restore requirements-txt-fixer in pre-commit #7920

Merged

[PR #7808/213d1b2 backport][3.10] Restore requirements-txt-fixer in pre-commit #7921

Merged

alexrudd2 deleted the patch-2 branch November 28, 2023 02:58

Dreamsorcerer pushed a commit that referenced this pull request Nov 29, 2023

[PR #7808/213d1b2 backport][3.9] Restore requirements-txt-fixer in pr…

ee8f7bf

…e-commit (#7920) Backport of #7808 cherry picked from commit 213d1b2

Dreamsorcerer pushed a commit that referenced this pull request Nov 29, 2023

[PR #7808/213d1b2 backport][3.10] Restore requirements-txt-fixer in p…

1b4efff

…re-commit (#7921) Backport of #7808 cherry picked from commit 213d1b2

webknjaz reviewed Nov 30, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Restore requirements-txt-fixer in pre-commit #7808

Restore requirements-txt-fixer in pre-commit #7808

alexrudd2 commented Nov 9, 2023 •

edited

Loading

alexrudd2 commented Nov 27, 2023

Dreamsorcerer commented Nov 27, 2023

alexrudd2 commented Nov 27, 2023

codecov bot commented Nov 27, 2023 •

edited

Loading

Dreamsorcerer commented Nov 27, 2023

Dreamsorcerer commented Nov 27, 2023

alexrudd2 commented Nov 27, 2023

alexrudd2 commented Nov 27, 2023

Dreamsorcerer commented Nov 27, 2023

alexrudd2 commented Nov 27, 2023 •

edited

Loading

patchback bot commented Nov 27, 2023 •

edited

Loading

patchback bot commented Nov 27, 2023 •

edited

Loading

alexrudd2 commented Nov 27, 2023

Dreamsorcerer commented Nov 27, 2023

webknjaz Nov 30, 2023

webknjaz Nov 30, 2023

alexrudd2 Nov 30, 2023

Restore requirements-txt-fixer in pre-commit #7808

Restore requirements-txt-fixer in pre-commit #7808

Conversation

alexrudd2 commented Nov 9, 2023 • edited Loading

What do these changes do?

Are there changes in behavior for the user?

Checklist

alexrudd2 commented Nov 27, 2023

Dreamsorcerer commented Nov 27, 2023

alexrudd2 commented Nov 27, 2023

codecov bot commented Nov 27, 2023 • edited Loading

Codecov Report

Dreamsorcerer commented Nov 27, 2023

Dreamsorcerer commented Nov 27, 2023

alexrudd2 commented Nov 27, 2023

alexrudd2 commented Nov 27, 2023

Dreamsorcerer commented Nov 27, 2023

alexrudd2 commented Nov 27, 2023 • edited Loading

patchback bot commented Nov 27, 2023 • edited Loading

Backport to 3.9: 💔 cherry-picking failed — conflicts found

Backporting merged PR #7808 into master

patchback bot commented Nov 27, 2023 • edited Loading

Backport to 3.10: 💔 cherry-picking failed — conflicts found

Backporting merged PR #7808 into master

alexrudd2 commented Nov 27, 2023

Dreamsorcerer commented Nov 27, 2023

webknjaz Nov 30, 2023

Choose a reason for hiding this comment

webknjaz Nov 30, 2023

Choose a reason for hiding this comment

alexrudd2 Nov 30, 2023

Choose a reason for hiding this comment

alexrudd2 commented Nov 9, 2023 •

edited

Loading

codecov bot commented Nov 27, 2023 •

edited

Loading

alexrudd2 commented Nov 27, 2023 •

edited

Loading

patchback bot commented Nov 27, 2023 •

edited

Loading

patchback bot commented Nov 27, 2023 •

edited

Loading