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

python312Packages.nose: fix build #325968

Merged
merged 17 commits into from
Jul 11, 2024
Merged

Conversation

jchv
Copy link
Contributor

@jchv jchv commented Jul 9, 2024

Description of changes

This is an alternative to #325935 which uses a patch that I made independently without looking at the other patches. If we can't figure that out, then this is a backup plan.

I anticipate that my patch is incomplete, but it is complete enough to pass checks on all of the packages that I just re-enabled checks for.

This also fixes Python 3.13 support in Nose. Nose relies on 2to3, which is removed in Python 3.13, so we always use 3to2 from Python 3.12. This should buy us some more time to wait for upstreams to either get off of Nose or get removed for being unmaintained.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.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.

Add a 👍 reaction to pull requests you find important.

Comment on lines 26 to 28
+# Adapted from CPython 3.11.
+# Licensed under the PSLv2.
+# https://github.com/python/cpython/blob/3.11/LICENSE
Copy link
Member

Choose a reason for hiding this comment

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

This is just a nit because the practice is really common, but:

2. Subject to the terms and conditions of this License Agreement, PSF hereby
grants Licensee a nonexclusive, royalty-free, world-wide license to reproduce,
analyze, test, perform and/or display publicly, prepare derivative works,
distribute, and otherwise use Python alone or in any derivative version,
provided, however, that PSF's License Agreement and PSF's notice of copyright,
i.e., "Copyright (c) 2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010,
2011, 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019, 2020, 2021, 2022, 2023 Python Software Foundation;
All Rights Reserved" are retained in Python alone or in any derivative version
prepared by Licensee.

3. In the event Licensee prepares a derivative work that is based on
or incorporates Python or any part thereof, and wants to make
the derivative work available to others as provided herein, then
Licensee hereby agrees to include in any such work a brief summary of
the changes made to Python.

Would you be willing to make the patch add the full upstream LICENSE file as LICENSE.cpython file, and adapt the comment similarly to the following?

# Adapted from the CPython 3.11 imp.py code.
# Copyright (c) 2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010,
2011, 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019, 2020, 2021, 2022, 2023 Python Software Foundation; All Rights Reserved
# Originally licensed under the PSLv2 (see LICENSE.cpython) and incorporated under the LGPL 2.1 (see lgpl.txt).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, thanks!

Copy link

Choose a reason for hiding this comment

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

Hopefully all is set on the pynose side from mdmintz/pynose#33 (comment). Let me know if anything else!

Comment on lines +35 to +37
# 2to3 is removed from Python 3.13, so always use Python 3.12 2to3 for now.
preBuild = lib.optionalString isPy3k ''
${python312.pythonOnBuildForHost}/bin/2to3 -wn nose functional_tests unit_tests
Copy link
Member

@emilazy emilazy Jul 10, 2024

Choose a reason for hiding this comment

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

I guess when 3.12 leaves Nixpkgs we’ll have to do one of:

  • package 2to3 independently of the Python interpreter;
  • run it once over the nose code and just vendor the results; or
  • remove nose?

Copy link
Contributor Author

@jchv jchv Jul 10, 2024

Choose a reason for hiding this comment

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

Yeah, that's roughly my thoughts. My order of preferences for future actions: (I assume most people are in a similar frame of mind, but just to jot it down in detail.)

  • Most preferred would be getting rid of nose dependencies upstream. It's unmaintained and most users don't use very much of its functionality, so a migration should be easy. For some packages, I think this will just be a matter of getting the version in Nixpkgs updated. For some simpler stuff, nose2pytest may be enough to generate a patch that we can use + submit upstream.

  • Failing that, if Lack of attribution and licence information for code derived from CPython mdmintz/pynose#33 gets resolved then maybe we can consider the pynose licensing issue resolved and resume usage of pynose as a drop-in replacement, pending other's approval. I'm sure there will be some hesitance here though and I'm not going to argue if the thought is we'd rather play it safe.

  • Failing that, we can continue to try to patch Nose for Python 3.14+. In an earlier version of this PR I did indeed include a 2to3 patch file, but it's rather large, so I think putting it in Nixpkgs preemptively is undesirable.

  • And failing that, my thought is we can remove nose, disable tests on anything that continues to use nose, and eventually remove packages that depend on nose which are unmaintained.

If you think it is a good idea, I can try to add more context to that comment for the future. Specifically, we could outline the options for handling 2to3.

Regarding packaging 2to3 independently: maybe even better would be packaging the fissix tool by amyreese, as it essentially seems to be an improved 2to3. (Coincidentally, it is also apparently used by the nose2pytest tool.)

Copy link
Member

@mweinelt mweinelt Jul 10, 2024

Choose a reason for hiding this comment

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

Ideally by 2028 we've fixed the ecosystem enough, that nose is not relevant anymore. I wouldn't worry too much about this today.

Copy link
Member

Choose a reason for hiding this comment

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

I didn’t realize that we kept Python releases for that many years. Agreed that it’s not worth thinking about in that case.

@jchv
Copy link
Contributor Author

jchv commented Jul 10, 2024

Things have gotten a bit scattered here.

  • I think this PR is good to go. I have been trying to get a nixpkgs-review run to finish but keep running into OOMs. The partial results look about in line with what is expected. (80 failures with <50 to go, all of the failures I've checked were already occurring before, just getting stuck on the really big compilations.) I'll still keep trying to push it through, though.

  • Separately, the pynose licensing issues are resolved as far as I know. I'm not sure who should call the shots as to whether a package can or should be re-instated in this situation so I'll defer to someone else regarding that. From my perspective though it seems like we're not blocked on license issues anymore.

  • Regarding python312Packages.nose: fix, use pep517 builder #325935, I think I'll close it for now. We can always resurrect it later if we want to use the Alpine patches again (assuming they are updated shortly.)

@mweinelt mweinelt marked this pull request as draft July 10, 2024 21:55
@mweinelt
Copy link
Member

Due to the rebuild count this should target the staging branch.

@emilazy
Copy link
Member

emilazy commented Jul 10, 2024

I am also running a nixpkgs-review run on the community builder, but I don’t know if I’ll get better outcomes than you :)

I feel more confident in this approach than #325935. The other patch seems incomplete too and I still feel more assured about the provenance of this one.

@jchv
Copy link
Contributor Author

jchv commented Jul 10, 2024

Thanks, re-rebased.

@jchv jchv marked this pull request as ready for review July 10, 2024 23:31
@emilazy
Copy link
Member

emilazy commented Jul 11, 2024

I forgot that things like Tensorflow would be in the build chain…

It was at 78 failures after >1,400 successful builds and 239 builds left to finish, which seems good to me. I forgot to start the job in the builder in tmux so I’ll leave it at that for now. If you get a full nixpkgs-review report I can do an automated check against the latest Hydra unstable job and the one before the 3.12 merge.

@natsukium
Copy link
Member

Since nose is a test suite and does not propagate, I guess it is not necessary to build many other things if those that directly depend on nose can be built.

@mweinelt mweinelt merged commit 60f2cc2 into NixOS:staging Jul 11, 2024
22 checks passed
@jchv
Copy link
Contributor Author

jchv commented Jul 11, 2024

Thanks for all of the help everybody! I had no ideal what an ordeal I was in for when I set out to try to fix this. Fingers crossed, but hopefully we can close the book on this one for now.

@mudrii
Copy link
Contributor

mudrii commented Jul 17, 2024

I am still facing issue on darwin-nix aarch64-darwin reference #326151

Dependence on

… while evaluating derivation 'python3.12-powerline-2.8.3'
         whose name attribute is located at /nix/store/77ijmdbv96rhqj2sr7aq0i5q8p16ii0d-source/pkgs/stdenv/generic/make-derivation.nix:334:7

       … while evaluating attribute 'propagatedBuildInputs' of derivation 'python3.12-powerline-2.8.3'

         at /nix/store/77ijmdbv96rhqj2sr7aq0i5q8p16ii0d-source/pkgs/stdenv/generic/make-derivation.nix:388:7:

          387|       depsHostHostPropagated      = elemAt (elemAt propagatedDependencies 1) 0;
          388|       propagatedBuildInputs       = elemAt (elemAt propagatedDependencies 1) 1;
             |       ^
          389|       depsTargetTargetPropagated  = elemAt (elemAt propagatedDependencies 2) 0;

       … while evaluating derivation 'python3.12-python-hglib-2.6.2'
         whose name attribute is located at /nix/store/77ijmdbv96rhqj2sr7aq0i5q8p16ii0d-source/pkgs/stdenv/generic/make-derivation.nix:334:7

       … while evaluating attribute 'nativeBuildInputs' of derivation 'python3.12-python-hglib-2.6.2'

         at /nix/store/77ijmdbv96rhqj2sr7aq0i5q8p16ii0d-source/pkgs/stdenv/generic/make-derivation.nix:378:7:

          377|       depsBuildBuild              = elemAt (elemAt dependencies 0) 0;
          378|       nativeBuildInputs           = elemAt (elemAt dependencies 0) 1;
             |       ^
          379|       depsBuildTarget             = elemAt (elemAt dependencies 0) 2;

       error: nose-1.3.7 not supported for interpreter python3.12

@mweinelt
Copy link
Member

This change has to pass through staging first. Check back in two weeks.

https://github.com/NixOS/rfcs/blob/master/rfcs/0026-staging-workflow.md

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/error-nose-1-3-7-not-supported-for-interpreter-python3-12/48703/40

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.

7 participants