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

python: decorator: fix Python 2 build #80435

Merged
merged 1 commit into from Feb 20, 2020

Conversation

KamilaBorowska
Copy link
Member

Motivation for this change

ZHF: #80379

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@KamilaBorowska KamilaBorowska changed the base branch from master to staging February 18, 2020 13:01
@KamilaBorowska
Copy link
Member Author

... hm, that won't work, not that the original attempt did work either, DocumentationTestCase appears flaky.

@worldofpeace
Copy link
Contributor

worldofpeace commented Feb 18, 2020

Can you format your commit message like pythonPackages.decorator: ...?
The contribution guidelines suggest to always try to use an attribute, and that is what's most common.

@KamilaBorowska
Copy link
Member Author

Okay, applied a patch from unreleased 4.4.2. I think 4.4.2 is going to be released very soon, so it may make more sense to wait for its release instead of updating the package twice.

@infinisil
Copy link
Member

I made a PR for the fix in micheles/decorator#79, where I also asked for a new release. I think we should wait a bit for that

See also #75554

jonringer
jonringer previously approved these changes Feb 18, 2020
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.

diff LGTM

[09:18:56] jon@jon-desktop /home/jon/projects/nixpkgs ((52fa957f827...))
$ nix build -f . pythonPackages.decorator python3Packages.decorator python38Packages.decorator
[2 built, 20 copied (138.5 MiB), 24.4 MiB DL]

@jonringer
Copy link
Contributor

wait a second, this is targeting staging.

Also, the builds are passing on 20.03

Finished executing setuptoolsCheckPhase
/nix/store/4d2bxibs9bl4609a298mvbp376hhs6dc-python2.7-decorator-4.4.1
[09:22:09] jon@jon-desktop /home/jon/projects/nixpkgs (release-20.03)
$ nix-build -A python3Packages.decorator
/nix/store/nbsca0r5y0qnpjhh74hfl96mp8a6mbvz-python3.7-decorator-4.4.1
[09:22:19] jon@jon-desktop /home/jon/projects/nixpkgs (release-20.03)
$ nix-build -A python38Packages.decorator
/nix/store/9bw8xf24v0625ppjxcx1fpfq0ly7lax9-python3.8-decorator-4.4.1
[09:22:23] jon@jon-desktop /home/jon/projects/nixpkgs (release-20.03)

@jonringer jonringer dismissed their stale review February 18, 2020 17:23

no longer relevant

@KamilaBorowska
Copy link
Member Author

KamilaBorowska commented Feb 18, 2020

The build was flaky, sometimes it passed, sometimes it didn't.

@jonringer
Copy link
Contributor

that's fair, I didn't get a cache hit on the python2 version

@worldofpeace
Copy link
Contributor

I also think we should go the way as @infinisil suggested #80435 (comment).

@jonringer
Copy link
Contributor

I'm also okay with waiting for a new patch version

@infinisil
Copy link
Member

I don't think the error is python 2 specific, it's just that it just happened to occur for the python 2 build this time

@FRidh FRidh added this to WIP in Staging via automation Feb 19, 2020
@FRidh FRidh moved this from WIP to Needs review in Staging Feb 19, 2020
@infinisil
Copy link
Member

Also we thought this might not need to go to staging, as it's only 500 rebuilds, and python ones at that, which are pretty cheap

@KamilaBorowska KamilaBorowska changed the base branch from staging to master February 19, 2020 20:25
@KamilaBorowska
Copy link
Member Author

Changed the base branch to master. This is a big rebuild, but if you are fine with master, then why not :).

@FRidh
Copy link
Member

FRidh commented Feb 19, 2020

@GrahamcOfBorg build python2Packages.decorator python3Packages.decorator python38Packages.decorator

@jonringer
Copy link
Contributor

python2 build is fixed, diff LGTM

@jonringer jonringer merged commit 686274e into NixOS:master Feb 20, 2020
Staging automation moved this from Needs review to Done Feb 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Staging
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants