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

python310Packages.flake8-future-import: fix build #144650

Merged
merged 4 commits into from
Nov 12, 2021

Conversation

wamserma
Copy link
Member

@wamserma wamserma commented Nov 4, 2021

Motivation for this change

xZise/flake8-future-import#24
ZHF: #144627
ping @NixOS/nixos-release-managers

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review pr 144650"
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 21.11 Release Notes (or backporting 21.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@SuperSandro2000
Copy link
Member

Please target staging

@risicle
Copy link
Contributor

risicle commented Nov 4, 2021

python310Packages.flake8-future-import building successfully here, macos 10.15.

@wamserma
Copy link
Member Author

wamserma commented Nov 5, 2021

re-targeted to staging, should be backported after branch-off

@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Nov 5, 2021
@risicle
Copy link
Contributor

risicle commented Nov 5, 2021

@ofborg eval

@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Nov 5, 2021
@jonringer
Copy link
Contributor

re-targeted to staging, should be backported after branch-off

If it's fixing regressions which already exist, they should be merged.

The "breaking changes to staging" block is to prevent new regressions from entering

@wamserma
Copy link
Member Author

wamserma commented Nov 8, 2021

re-targeted to staging, should be backported after branch-off

If it's fixing regressions which already exist, they should be merged.

The "breaking changes to staging" block is to prevent new regressions from entering

It's an un-breaking PR, but as it includes a bump of python3.flake8 we have quite a number of rebuilds.

Let the @NixOS/nixos-release-managers decide where they prefer this PR. Feel free to re-target as required before merging.

@SuperSandro2000
Copy link
Member

we have quite a number of rebuilds.

That does not matter for the breaking change policy. It is about not bumping compilers like gcc to a breaking version.

Let the @NixOS/nixos-release-managers decide where they prefer this PR. Feel free to re-target as required before merging.

jonringer is the release manager.

@SuperSandro2000
Copy link
Member

@ofborg build python310Packages.flake8-future-import python310Packages.flake8 python39Packages.flake8-future-import python39Packages.flake8

Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is causing breakages with the manual. the flake8 bump will need to wait til after ZHF

https://github.com/NixOS/nixpkgs/runs/4137245989?check_suite_focus=true

@wamserma
Copy link
Member Author

wamserma commented Nov 9, 2021

@jonringer (hopefully) fixed

@SuperSandro2000
Copy link
Member

@ofborg eval
@ofborg build

@SuperSandro2000
Copy link
Member

I think it is failing due to coreutils.

@wamserma
Copy link
Member Author

wamserma commented Nov 9, 2021

I think it is failing due to coreutils.

Found this in the logs if the coreutils checks:

SKIP: tests/du/2g
=================

2g.sh: skipped test: very expensive: disabled by default
This test is very expensive, so it is disabled by default.
To run it anyway, rerun make check with the RUN_VERY_EXPENSIVE_TESTS
environment variable set to yes.  E.g.,

  env RUN_VERY_EXPENSIVE_TESTS=yes make check

or use the shortcut target of the toplevel Makefile,

  make check-very-expensive

./tests/init.sh: line 106: printf: write error: Broken pipe
SKIP tests/du/2g.sh (exit status: 77)

FAIL: tests/du/basic
====================

--- exp 2021-11-09 11:58:41.313614539 +0000
+++ out 2021-11-09 11:58:41.278614356 +0000
@@ -1,7 +1,7 @@
-9      d/sub/2
-10     d/sub
-9      d/1
-20     d
+1      d/sub/2
+2      d/sub
+1      d/1
+4      d
 ===
-10     d/sub
-10     d
+2      d/sub
+2      d
FAIL tests/du/basic.sh (exit status: 1)

So it might be an intermittent error.

@wamserma
Copy link
Member Author

wamserma commented Nov 9, 2021

Rebased, now
@ofborg eval
@ofborg build

@wamserma
Copy link
Member Author

@SuperSandro2000 fixed

@SuperSandro2000
Copy link
Member

@jonringer I think we can merge this now. What do you think?

@SuperSandro2000
Copy link
Member

SuperSandro2000 commented Nov 12, 2021

This is a semi-automatic executed nixpkgs-review with nixpkgs-review-checks extension. It is checked by a human on a best effort basis and does not build all packages (e.g. lumo, tensorflow or pytorch).

Result of nixpkgs-review pr 144650 run on x86_64-linux 1

1 package failed to build and are new build failure:
  • home-assistant: log was empty

verified manually that home-assistant builds

@SuperSandro2000 SuperSandro2000 merged commit 73bf5b8 into NixOS:staging Nov 12, 2021
@wamserma wamserma deleted the zhf-flake8-future-import branch November 12, 2021 15:50
@trofi
Copy link
Contributor

trofi commented Nov 13, 2021

Does patch fetch work for you? Fails for me as:

$ nix build -f. python3Packages.pytest-flake8 -L

eda4ef74c0f25b856fe282742ea206b21e94c24c.patch> trying https://github.com/tholo/pytest-flake8/commit/eda4ef74c0f25b856fe282742ea206b21e94c24c.patch
eda4ef74c0f25b856fe282742ea206b21e94c24c.patch>   % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
eda4ef74c0f25b856fe282742ea206b21e94c24c.patch>                                  Dload  Upload   Total   Spent    Left  Speed
eda4ef74c0f25b856fe282742ea206b21e94c24c.patch> 100  3155  100  3155    0     0  37639      0 --:--:-- --:--:-- --:--:-- 37559
error: hash mismatch in fixed-output derivation '/nix/store/qnplqiw1cs5c73099virwa7i60krzq9z-eda4ef74c0f25b856fe282742ea206b21e94c24c.patch.drv':
         specified: sha256-/QBKi1nR5pCCEvODf4cmEyxdXihLLJgRUljDloKPLAA=
            got:    sha256-kfll+I9akZM4Vp62Q1M9r2Mvs2TM2Lm5mRkA3aDmAE8=
error: 1 dependencies of derivation '/nix/store/1vjdyv87g6fjdh93p9dascwy8p984xyy-python3.9-pytest-flake8-1.0.7.drv' failed to build

@wamserma
Copy link
Member Author

@trofi When I build from current master it simply pulls /nix/store/2743gl0snph4z967yi1kqvaaiqp75cg6-python3.9-pytest-flake8-1.0.7 from the binary cache.

> nix-prefetch-url https://github.com/tholo/pytest-flake8/commit/eda4ef74c0f25b856fe282742ea206b21e94c24c.patch
[0.0 MiB DL]
path is '/nix/store/2h9wk9cicphjzpbpg0pp3za25mq5l4dw-eda4ef74c0f25b856fe282742ea206b21e94c24c.patch'
001ciy19dhsqa88rhb2b51g5sb0k4s3pz0zk2a191rnib65ll07x

@trofi
Copy link
Contributor

trofi commented Nov 13, 2021

nix-prefetch-url https://github.com/tholo/pytest-flake8/commit/eda4ef74c0f25b856fe282742ea206b21e94c24c.patch

Ah, that explains it. fetchpatch (as opposed to fetchurl) does mangle patch a bit before calculating the checksum.

Proposed hash fix as #145786

@wamserma
Copy link
Member Author

Right, just didn't show up, because the patch was already fetched.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants