Skip to content

[25.05] nushellPlugins.*: refactor, add checks, mark some as broken#421135

Merged
wolfgangwalther merged 13 commits intoNixOS:release-25.05from
dtomvan:backport-420675-to-release-25.05
Jul 11, 2025
Merged

[25.05] nushellPlugins.*: refactor, add checks, mark some as broken#421135
wolfgangwalther merged 13 commits intoNixOS:release-25.05from
dtomvan:backport-420675-to-release-25.05

Conversation

@dtomvan
Copy link
Copy Markdown
Contributor

@dtomvan dtomvan commented Jun 29, 2025

Manual backport of #420675.

These were refactorings, but I want (automatic) backports of version bumps to still be possible, so I imagined it to be a good idea to introduce them to stable as well. Also because the dbus plugin is broken on 25.05 too.

I didn't cherry-pick 3 commits:

  • nushellPlugins.units: mark as broken, because it's still compatible on 25.05
  • nushellPlugins.net: mark as broken, idem
  • nushellPlugins.hcl: refactor, because it doesn't exist on stable

Tested with: NIXPKGS_ALLOW_BROKEN=1 nom-build -E 'with import ./. { config.allowAliases = false; }; lib.mapAttrs (_n: p: p.tests.loadCheck) (lib.filterAttrs (_p: lib.isDerivation) nushellPlugins)' --keep-going

Only dbus fails the tests but that's intended as it's broken right now.

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/)
  • Nixpkgs 25.11 Release Notes (or backporting 24.11 and 25.05 Nixpkgs Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
  • NixOS 25.11 Release Notes (or backporting 24.11 and 25.05 NixOS Release notes)
    • (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, pkgs/README.md, maintainers/README.md and other contributing documentation in corresponding paths.

Add a 👍 reaction to pull requests you find important.

Copy link
Copy Markdown
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This report is automatically generated by the check-cherry-picks CI workflow.

Some of the commits in this PR have not been cherry-picked exactly and require the author's and reviewer's attention.

Please make sure to follow the backporting guidelines and cherry-pick with the -x flag. This requires changes to go to the unstable branches (master / staging) first, before backporting them.

Occasionally, it is not possible to cherry-pick exactly the same patch. This most frequently happens when resolving merge conflicts while cherry-picking or when updating minor versions of packages which have already advanced to the next major on unstable. If you need to merge this PR despite the warnings, please dismiss this review.

Warning

Difference between c2fcc21 and original 7095018 may warrant inspection.

Show diff
@@ Metadata
  ## Commit message ##
     nushellPlugins.formats: refactor
 
+    (cherry picked from commit 7095018016f35d90c1bc65c2829bb36d0aec76c6)
+
  ## pkgs/shells/nushell/plugins/formats.nix ##
 @@
    nix-update-script,
@@ pkgs/shells/nushell/plugins/formats.nix
    };
 -}
 +})
-
- ## pkgs/shells/nushell/plugins/hcl.nix ##
-@@ pkgs/shells/nushell/plugins/hcl.nix: rustPlatform.buildRustPackage rec {
-     tag = version;
-     hash = "sha256-V1RKZ0Tqq0LTGbHS2lLMyf6M4AgAgWSzkDeFUighO4k=";
-   };
--  useFetchCargoVendor = true;
-+
-   cargoHash = "sha256-UbqKfQxut+76yB9F1gT8FEapbX/kHvaShltHpWUdhgc=";
- 
-   nativeBuildInputs = [ pkg-config ] ++ lib.optionals stdenv.cc.isClang [ rustPlatform.bindgenHook ];

Warning

Difference between 5f10f42 and original bd065b6 may warrant inspection.

Show diff
@@ Commit message
     - `versionCheckHook` cannot find the binaries without setting a special
       variable
 
+    (cherry picked from commit bd065b68884efe1bed308af1bc43c6db06bfe420)
+
  ## pkgs/shells/nushell/plugins/formats.nix ##
 @@
  }:
@@ pkgs/shells/nushell/plugins/gstat.nix
  
    nativeBuildInputs = [ pkg-config ] ++ lib.optionals stdenv.cc.isClang [ rustPlatform.bindgenHook ];
 
- ## pkgs/shells/nushell/plugins/hcl.nix ##
-@@
- }:
- 
- rustPlatform.buildRustPackage (finalAttrs: {
--  pname = "nushell_plugin_hcl";
-+  pname = "nu_plugin_hcl";
-   version = "0.105.1";
- 
-   src = fetchFromGitHub {
-
  ## pkgs/shells/nushell/plugins/highlight.nix ##
 @@
  }:
@@ pkgs/shells/nushell/plugins/highlight.nix
  rustPlatform.buildRustPackage (finalAttrs: {
 -  pname = "nushell_plugin_highlight";
 +  pname = "nu_plugin_highlight";
-   version = "1.4.7+0.105.1";
+   version = "1.4.5+0.104.0";
  
    src = fetchFromGitHub {
 
@@ pkgs/shells/nushell/plugins/net.nix
 @@
  }:
  
- rustPlatform.buildRustPackage (finalAttrs: {
+ rustPlatform.buildRustPackage rec {
 -  pname = "nushell_plugin_net";
 +  pname = "nu_plugin_net";
    version = "1.10.0";

Warning

Difference between 32675f4 and original 8541ae6 may warrant inspection.

Show diff
@@ Commit message
     Also because when new plugins get added or plugins get updated, problems
     can be easily spotted because the errors are really loud.
 
+    (cherry picked from commit 8541ae6c6a08b21b2882452ccaaea490e8ac30b5)
+
  ## pkgs/shells/nushell/plugins/default.nix ##
 @@
    config,
@@ pkgs/shells/nushell/plugins/default.nix
 -    };
 -    skim = callPackage ./skim.nix { };
 -    semver = callPackage ./semver.nix { };
--    hcl = callPackage ./hcl.nix { };
 -  }
 -  // lib.optionalAttrs config.allowAliases {
 -    regex = throw "`nu_plugin_regex` is no longer compatible with the current Nushell release.";
@@ pkgs/shells/nushell/plugins/default.nix
 +        };
 +        skim = callPackage ./skim.nix { };
 +        semver = callPackage ./semver.nix { };
-+        hcl = callPackage ./hcl.nix { };
 +      }
 +      // lib.optionalAttrs config.allowAliases {
 +        regex = throw "`nu_plugin_regex` is no longer compatible with the current Nushell release.";

Warning

Difference between 4ccaab8 and original 94b1ae5 may warrant inspection.

Show diff
@@ Metadata
  ## Commit message ##
     nushellPlugins.*: `doCheck = false;` where applicable
 
- ## pkgs/shells/nushell/plugins/hcl.nix ##
-@@ pkgs/shells/nushell/plugins/hcl.nix: rustPlatform.buildRustPackage (finalAttrs: {
- 
-   nativeBuildInputs = [ pkg-config ] ++ lib.optionals stdenv.cc.isClang [ rustPlatform.bindgenHook ];
- 
-+  # there are no tests
-+  doCheck = false;
-+
-   passthru.updateScript = nix-update-script { };
- 
-   meta = {
+    (cherry picked from commit 94b1ae52c486c1e45a41123c48929f02a4156d8a)
 
  ## pkgs/shells/nushell/plugins/highlight.nix ##
 @@ pkgs/shells/nushell/plugins/highlight.nix: rustPlatform.buildRustPackage (finalAttrs: {
@@ pkgs/shells/nushell/plugins/highlight.nix: rustPlatform.buildRustPackage (finalA
    meta = {
 
  ## pkgs/shells/nushell/plugins/net.nix ##
-@@ pkgs/shells/nushell/plugins/net.nix: rustPlatform.buildRustPackage (finalAttrs: {
+@@ pkgs/shells/nushell/plugins/net.nix: rustPlatform.buildRustPackage rec {
  
-   nativeBuildInputs = lib.optionals stdenv.cc.isClang [ rustPlatform.bindgenHook ];
+   nativeBuildInputs = [ rustPlatform.bindgenHook ];
  
 +  # there are no tests
 +  doCheck = false;
 +
    passthru.updateScript = nix-update-script { };
  
-   meta = {
+   meta = with lib; {

Hint: The full diffs are also available in the runner logs with slightly better highlighting.

@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 4.workflow: backport This targets a stable branch labels Jun 29, 2025
@dtomvan
Copy link
Copy Markdown
Contributor Author

dtomvan commented Jun 29, 2025

Cannot dismiss the review I think, but this is all intentional. I had to resolve some conflicts.

@dtomvan
Copy link
Copy Markdown
Contributor Author

dtomvan commented Jun 29, 2025

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 421135
Commit: 39b9d2b262ec5af9cd11c23fcba27b05864f707b


x86_64-linux

✅ 9 packages built:
  • nushellPlugins.formats
  • nushellPlugins.gstat
  • nushellPlugins.highlight
  • nushellPlugins.net
  • nushellPlugins.polars
  • nushellPlugins.query
  • nushellPlugins.semver
  • nushellPlugins.skim
  • nushellPlugins.units

@dtomvan dtomvan marked this pull request as ready for review June 29, 2025 23:50
@nixpkgs-ci nixpkgs-ci bot added 12.approvals: 1 This PR was reviewed and approved by one person. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages. labels Jun 30, 2025
Copy link
Copy Markdown
Member

@mdaniels5757 mdaniels5757 left a comment

Choose a reason for hiding this comment

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

Conflict resolution LGTM, although it would have been much easier to review if git cherry-pick -x were used instead of git cherry-pick.

@nixpkgs-ci nixpkgs-ci bot added 12.approvals: 2 This PR was reviewed and approved by two persons. and removed 12.approvals: 1 This PR was reviewed and approved by one person. labels Jul 1, 2025
@dtomvan
Copy link
Copy Markdown
Contributor Author

dtomvan commented Jul 1, 2025

Hmm. I did jj duplicate... I guess I'll use plain git next time, as indeed is mentioned in CONTRIBUTING.md.... I don't think jj has a flag to mimic -x.

Copy link
Copy Markdown
Contributor

@wolfgangwalther wolfgangwalther left a comment

Choose a reason for hiding this comment

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

Please backport all these commits with git cherry-pick -x, otherwise I have to check every single commit. Especially when the commits have conflicts, because then I can get a diff of diffs in the automated review.

If it's easier, after the fact, for you, you can also add the respective lines in the commit message with the right hashes for those commits manually. Just make sure they have the same format as some of the properly cherry-picked commits.

dtomvan added 8 commits July 10, 2025 03:08
(cherry picked from commit a76d03b)
(cherry picked from commit d22a786)
(cherry picked from commit 7095018)
(cherry picked from commit 17144a6)
(cherry picked from commit 4627382)
(cherry picked from commit 2e5baee)
(cherry picked from commit aa9629d)
(cherry picked from commit f79eb5c)
dtomvan added 5 commits July 10, 2025 03:09
Because otherwise:
- The names are inconsistent with the other nushell plugins
- `versionCheckHook` cannot find the binaries without setting a special
  variable

(cherry picked from commit bd065b6)
Why for each all at the same time? Because all nushell plugins should
respond in the same way with either no arguments passed or with
`--help`, which contains their version.

Also because when new plugins get added or plugins get updated, problems
can be easily spotted because the errors are really loud.

(cherry picked from commit 8541ae6)
@dtomvan dtomvan force-pushed the backport-420675-to-release-25.05 branch from 39b9d2b to 8aae88f Compare July 10, 2025 01:12
@dtomvan
Copy link
Copy Markdown
Contributor Author

dtomvan commented Jul 10, 2025

I just did git log --oneline 374676...7c489 | tac | awk '{print $1}' | xargs -pn1 git cherry-pick -x and said no to the commits that don't need cherrypicking. Also rebased on latest release-25.05. nushellPlugins.dbus has already been marked as broken there, so also didn't cherrypick that one. Hope this is correct this time.

@mdaniels5757
Copy link
Copy Markdown
Member

mdaniels5757 commented Jul 10, 2025

I think the current version (8aae88f) is missing 8f7a6ec and 7c48977?


Command run: git diff 39b9d2b262ec5af9cd11c23fcba27b05864f707b..8aae88f5984848f6ecfa210d80c308d828ba23ab -- pkgs/shells/nushell

diff --git a/pkgs/shells/nushell/plugins/dbus.nix b/pkgs/shells/nushell/plugins/dbus.nix
index 6d322641b541..8b37fad33a8e 100644
--- a/pkgs/shells/nushell/plugins/dbus.nix
+++ b/pkgs/shells/nushell/plugins/dbus.nix
@@ -33,8 +33,5 @@ rustPlatform.buildRustPackage (finalAttrs: {
     license = lib.licenses.mit;
     maintainers = with lib.maintainers; [ aftix ];
     platforms = lib.platforms.linux;
-    # "Plugin `dbus` is compiled for nushell version 0.101.0, which is not
-    # compatible with version 0.104.0"
-    broken = true;
   };
 })
diff --git a/pkgs/shells/nushell/plugins/net.nix b/pkgs/shells/nushell/plugins/net.nix
index 900b53e7a0df..17994aaaa15d 100644
--- a/pkgs/shells/nushell/plugins/net.nix
+++ b/pkgs/shells/nushell/plugins/net.nix
@@ -1,36 +1,36 @@
 {
-  stdenv,
   lib,
   rustPlatform,
   fetchFromGitHub,
   nix-update-script,
 }:

-rustPlatform.buildRustPackage (finalAttrs: {
+rustPlatform.buildRustPackage rec {
   pname = "nu_plugin_net";
   version = "1.10.0";

   src = fetchFromGitHub {
     owner = "fennewald";
     repo = "nu_plugin_net";
-    tag = "${finalAttrs.version}";
+    rev = "refs/tags/${version}";
     hash = "sha256-HiNydU40FprxVmRRZtnXom2kFYI04mbeuGTq8+BMh7o=";
   };

+  useFetchCargoVendor = true;
   cargoHash = "sha256-tq0XqY2B7tC2ep8vH6T3nkAqxqhniqzYnhbkfB3SbHU=";

-  nativeBuildInputs = lib.optionals stdenv.cc.isClang [ rustPlatform.bindgenHook ];
+  nativeBuildInputs = [ rustPlatform.bindgenHook ];

   # there are no tests
   doCheck = false;

   passthru.updateScript = nix-update-script { };

-  meta = {
+  meta = with lib; {
     description = "Nushell plugin to list system network interfaces";
     homepage = "https://github.com/fennewald/nu_plugin_net";
-    license = lib.licenses.mit;
-    maintainers = with lib.maintainers; [ happysalada ];
+    license = licenses.mit;
+    maintainers = with maintainers; [ happysalada ];
     mainProgram = "nu_plugin_net";
   };
-})
+}

Copy link
Copy Markdown
Member

@mdaniels5757 mdaniels5757 left a comment

Choose a reason for hiding this comment

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

Missing the two above.

@wolfgangwalther wolfgangwalther dismissed their stale review July 10, 2025 06:31

cherry-picked with -x, nice

@dtomvan
Copy link
Copy Markdown
Contributor Author

dtomvan commented Jul 10, 2025

I think the current version (8aae88f) is missing 8f7a6ec and 7c48977?

Intentionally so:

  • dbus plugin already got set to broken somewhere between the original PR and the backport PR "rework"
  • net plugin still works on 25.05

@wolfgangwalther wolfgangwalther dismissed github-actions[bot]’s stale review July 11, 2025 06:48

conflict resolution LGTM

@wolfgangwalther
Copy link
Copy Markdown
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 421135
Commit: 8aae88f5984848f6ecfa210d80c308d828ba23ab


x86_64-linux

✅ 10 packages built:
  • nushellPlugins.dbus
  • nushellPlugins.formats
  • nushellPlugins.gstat
  • nushellPlugins.highlight
  • nushellPlugins.net
  • nushellPlugins.polars
  • nushellPlugins.query
  • nushellPlugins.semver
  • nushellPlugins.skim
  • nushellPlugins.units

aarch64-linux

✅ 10 packages built:
  • nushellPlugins.dbus
  • nushellPlugins.formats
  • nushellPlugins.gstat
  • nushellPlugins.highlight
  • nushellPlugins.net
  • nushellPlugins.polars
  • nushellPlugins.query
  • nushellPlugins.semver
  • nushellPlugins.skim
  • nushellPlugins.units

x86_64-darwin

✅ 8 packages built:
  • nushellPlugins.formats
  • nushellPlugins.gstat
  • nushellPlugins.highlight
  • nushellPlugins.net
  • nushellPlugins.polars
  • nushellPlugins.query
  • nushellPlugins.skim
  • nushellPlugins.units

aarch64-darwin

✅ 8 packages built:
  • nushellPlugins.formats
  • nushellPlugins.gstat
  • nushellPlugins.highlight
  • nushellPlugins.net
  • nushellPlugins.polars
  • nushellPlugins.query
  • nushellPlugins.skim
  • nushellPlugins.units

@wolfgangwalther wolfgangwalther merged commit e9c666a into NixOS:release-25.05 Jul 11, 2025
24 of 27 checks passed
@dtomvan dtomvan deleted the backport-420675-to-release-25.05 branch July 11, 2025 20:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4.workflow: backport This targets a stable branch 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. 12.approvals: 2 This PR was reviewed and approved by two persons. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants