Skip to content

python312Packages.{webassets, flask-assets}: remove nose; modernize#348621

Merged
emilazy merged 5 commits intoNixOS:masterfrom
pyrox0:denose/webassets
Oct 15, 2024
Merged

python312Packages.{webassets, flask-assets}: remove nose; modernize#348621
emilazy merged 5 commits intoNixOS:masterfrom
pyrox0:denose/webassets

Conversation

@pyrox0
Copy link
Member

@pyrox0 pyrox0 commented Oct 14, 2024

Follows up and includes the work from #334483. Fixes some broken subsitutions in flask-assets and enables some tests in webassets.

Part of #326513.

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.

@github-actions github-actions bot added the 6.topic: python Python is a high-level, general-purpose programming language. label Oct 14, 2024
@pyrox0
Copy link
Member Author

pyrox0 commented Oct 14, 2024

Currently running nixpkgs-review, will add output of that below once I've finished running it.

@pyrox0
Copy link
Member Author

pyrox0 commented Oct 14, 2024

Getting some build errors, trying to isolate the issue now.

Edit: Fixed some dukpy test errors. Investigating some flask-mail test errors, which is weird because I don't think it's got a dependency on anything here that's been changed.

@pyrox0
Copy link
Member Author

pyrox0 commented Oct 14, 2024

Confirmed that the flask-mail test errors are unrelated to this PR. Pushing my dukpy changes, this is ready to review.

Comment on lines +24 to +28
postPatch = ''
substituteInPlace tests/test_webassets_filter.py \
--replace-fail "class PyTestTemp" "class _Temp" \
--replace-fail "PyTestTemp" "Temp"
'';
Copy link
Member

Choose a reason for hiding this comment

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

Do you know what’s going on here? Was it broken already or do the changes in this PR somehow make this necessary?

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I see, it’s adapting for the changes in webassets.test. Okay, that seems fine then; hopefully upstream will release and the ecosystem will adapt at some point so that we no longer need to carry this.

@ofborg ofborg bot requested review from abbradar and ruby0b October 14, 2024 23:33
@ofborg ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Oct 14, 2024
Copy link
Member

@emilazy emilazy left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

It’s a shame to have to carry a patch like this around but there’s not much we can do when upstream hasn’t properly finished the migration and it’s too widely depended upon to drop. (And anyway, that’s inherited from the previous PR.)

We’re almost at the finish line!

Result of nixpkgs-review pr 348621 run on aarch64-linux 1

1 package failed to build:
  • powerdns-admin
14 packages built:
  • pyload-ng
  • pyload-ng.dist
  • python311Packages.dukpy
  • python311Packages.dukpy.dist
  • python311Packages.flask-assets
  • python311Packages.flask-assets.dist
  • python311Packages.webassets
  • python311Packages.webassets.dist
  • python312Packages.dukpy
  • python312Packages.dukpy.dist
  • python312Packages.flask-assets
  • python312Packages.flask-assets.dist
  • python312Packages.webassets
  • python312Packages.webassets.dist

Comment on lines +24 to +28
postPatch = ''
substituteInPlace tests/test_webassets_filter.py \
--replace-fail "class PyTestTemp" "class _Temp" \
--replace-fail "PyTestTemp" "Temp"
'';
Copy link
Member

Choose a reason for hiding this comment

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

Ah, I see, it’s adapting for the changes in webassets.test. Okay, that seems fine then; hopefully upstream will release and the ecosystem will adapt at some point so that we no longer need to carry this.

@emilazy emilazy merged commit df70905 into NixOS:master Oct 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: python Python is a high-level, general-purpose programming language. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants