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

beets: 2.0.0 -> 2.2.0 #358086

Merged
merged 8 commits into from
Dec 7, 2024
Merged

beets: 2.0.0 -> 2.2.0 #358086

merged 8 commits into from
Dec 7, 2024

Conversation

montchr
Copy link
Member

@montchr montchr commented Nov 22, 2024

https://github.com/beetbox/beets/blob/v2.1.0/docs/changelog.rst

Note that this release includes a lot of improvements to the upstream project's build system, so a refactor of common.nix was in order.

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/)
  • 25.05 Release Notes (or backporting 24.11 and 25.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.

@montchr montchr force-pushed the update/beets branch 2 times, most recently from 891d5b5 to d1a90be Compare November 22, 2024 07:49
@montchr

This comment was marked as outdated.

@montchr montchr force-pushed the update/beets branch 2 times, most recently from b72c4d8 to c493abe Compare November 22, 2024 16:34
@montchr montchr changed the title beets: 2.0.0 -> 2.1.0 beets-stable: 2.0.0 -> 2.1.0 + beets-unstable: 2.0.0-unstable-2024-05-25 -> 2.1.0-unstable-2024-11-22 Nov 22, 2024
@montchr

This comment was marked as outdated.

@montchr montchr force-pushed the update/beets branch 3 times, most recently from 00d3108 to 0995bdf Compare November 22, 2024 21:24
@montchr

This comment was marked as outdated.

@montchr

This comment was marked as resolved.

@montchr
Copy link
Member Author

montchr commented Nov 23, 2024

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 358086


aarch64-linux

✅ 16 packages built:
  • beets
  • beets-unstable
  • beets-unstable.dist
  • beets-unstable.doc
  • beets-unstable.man
  • beets.dist
  • beets.doc
  • beets.man
  • beetsPackages.alternatives
  • beetsPackages.alternatives.dist
  • beetsPackages.beets-minimal
  • beetsPackages.beets-minimal.dist
  • beetsPackages.beets-minimal.doc
  • beetsPackages.beets-minimal.man
  • beetsPackages.copyartifacts
  • beetsPackages.copyartifacts.dist

montchr added a commit to montchr/dotfield that referenced this pull request Nov 23, 2024
@montchr
Copy link
Member Author

montchr commented Nov 23, 2024

I've been using beets-stable from this PR in my configuration and it's working as expected:

montchr/dotfield@f7391d4

Up until the release of 2.1.0, I had been applying a patch via beets-unstable.override to work around beetbox/beets#5473, so I was happy to see the new release.

Copy link
Contributor

@doronbehar doronbehar left a comment

Choose a reason for hiding this comment

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

Looks pretty good to me! Thanks a lot, I wonder why only now I got pinged for a review. Ideally, I'd have liked to see the changes in a different order. The following 4, are fixes / improvements to the current state of the package (many thanks for that!):

beets: add @montchr as co-maintainer
beetsPackages.extrafiles: remove broken/unmaintained plugin
beets: mark builtin acousticbrainz plugin as deprecated
beets: alphabetize builtin plugins
beets: small reformatting and dead code removal

And the rest of the PR is really only an update to beets-{,un}stable, so can be squashed to 2 commit:

beets-stable: 2.0.0 -> 2.1.0
beets-unstable: 2.0.0-unstable-2024-05-25 -> 2.1.0-unstable-2024-11-22

Comment on lines 125 to 131
pytestFlagsArray = [
# beets.ui.UserError: unknown command 'autobpm'
"--deselect" "test/plugins/test_autobpm.py::TestAutoBPMPlugin::test_command"

# AssertionError: assert 0 == 117
"--deselect" "test/plugins/test_autobpm.py::TestAutoBPMPlugin::test_import"
];
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't work with the disabledTests array variable?

Comment on lines -42 to -50
, testPaths ? [
# NOTE: This conditional can be removed when beets-stable is updated and
# the default plugins test path is changed
(if (lib.versions.majorMinor version) == "1.6" then
"test/test_${name}.py"
else
"test/plugins/test_${name}.py"
)
]
Copy link
Contributor

Choose a reason for hiding this comment

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

This change can be detached from the update commit.

@@ -68,26 +60,31 @@ let
disabledPlugins = lib.filterAttrs (_: p: !p.enable) allPlugins;

pluginWrapperBins = concatMap (p: p.wrapperBins) (attrValues enabledPlugins);

disabledTestPaths = lib.flatten (attrValues (lib.mapAttrs (_: v: v.testPaths) disabledPlugins));
Copy link
Contributor

Choose a reason for hiding this comment

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

This change too is very welcome, but can be detached from the update commit.

@montchr
Copy link
Member Author

montchr commented Nov 24, 2024

@doronbehar Thanks for the review! I'm working on addressing those changes.

And the rest of the PR is really only an update to beets-{,un}stable, so can be squashed to 2 commit

What about the changes to beetsPackages.copyartifacts to make its tests work with 2.1.0? Since that's technically a separate package, I would think those changes should still follow the commit convention, thus requiring a separate commit beginning with beetsPackages.copyartifacts .

@doronbehar
Copy link
Contributor

doronbehar commented Nov 25, 2024 via email

As noted in a comment in the package expression, the third-party `extrafiles`
plugin has not been maintained since 2020 and has been broken since the release
of beets version 2.0.0 in May 2024.

Since the `beets-stable` package has been updated to version 2.0.0, it is no
longer feasible to keep `beetsPackages.extrafiles` around.
montchr and others added 3 commits December 1, 2024 16:19
Most were already in order but some plugins were thrown in at the bottom of the
file.
@doronbehar
Copy link
Contributor

I fixed the commit log as I wanted, and fixed the merge conflict (due to #357903). Now this should be good after CI is green.

@doronbehar doronbehar changed the title beets-stable: 2.0.0 -> 2.1.0 + beets-unstable: 2.0.0-unstable-2024-05-25 -> 2.1.0-unstable-2024-11-22 beets: 2.0.0 -> 2.2.0 Dec 2, 2024
@doronbehar
Copy link
Contributor

Ofborg failed to eval here, but the new evaluations succeeded. I also tested manually the build on aarch64-darwin, so I'm merging.

@doronbehar doronbehar merged commit 65484f8 into NixOS:master Dec 7, 2024
22 of 24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants