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

nav: init at 1.2.1 #356071

Merged
merged 3 commits into from
Dec 12, 2024
Merged

nav: init at 1.2.1 #356071

merged 3 commits into from
Dec 12, 2024

Conversation

David-Kopczynski
Copy link
Contributor

Hello :)

I added nav from https://github.com/Jojo4GH/nav with the functionality of navigating files in the terminal more easily.

While Jojo4GH is the project maintainer, I will help him keep this package up to date on nix. This is the reason, I added both of us to the maintainers list.

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.

@github-actions github-actions bot added the 8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` label Nov 15, 2024
@NixOSInfra NixOSInfra added the 12. first-time contribution This PR is the author's first one; please be gentle! label Nov 15, 2024
pkgs/by-name/na/nav/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/na/nav/package.nix Outdated Show resolved Hide resolved
aarch64-linux = "sha256-l3rKu3OU/TUUjmx3p06k9V5eN3ZDNcxbxObLqVQ2B7U=";
}
.${stdenv.hostPlatform.system} or (throw "unsupported system ${stdenv.hostPlatform.system}");
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this not be built from source, using, e.g. gradle2nix rather than downloading the release binary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

While looking for gradle2nix, I could not find any documentation in this repository at first and thus tried to get it running with gradle. However, we could not get the build running (our latest config is shown below).

This is the reason, I looked into gradle2nix again. The repository is currently migrating to V2 with functionality that is not (yet?) available for nixpkgs. We are currently wondering how to achieve the build and would like to know whether to stick to the binaries or if we should use:

let
  gradle2nix = import (fetchTarball "https://github.com/tadfisher/gradle2nix/archive/v2.tar.gz")  {};
in
gradle2nix.buildGradlePackage { ... }

(as described in the documentation for gradle2nix)

Would love to know how to proceed or where to get further information.

Nix Config for gradle.fetchDeps

We tried many variations of this file with gradleUpdateTask, gradleUpdateScript, and gradleFlags, but all of them failed at the dependency download or building step. @Jojo4GH's theory was that the dependency resolution does not work for common architectures, failing at downloading kotlin-stdlib:2.0.0 for arm64.

{
  stdenv,
  lib,
  fetchFromGitHub,
  gradle,
  makeWrapper,
  jre,
  nix-update-script,
}:

stdenv.mkDerivation (finalAttrs: rec {
  pname = "nav";
  version = "1.2.1";

  src = fetchFromGitHub {
    owner = "Jojo4GH";
    repo = "nav";
    rev = "v${version}";
    hash = "sha256-rS64lvS8lBpOsNq+abqqLrX5HJsPTdVx6Akjg9S/pyQ=";
  };

  nativeBuildInputs = [
    gradle
  ];

  mitmCache = gradle.fetchDeps {
    inherit (finalAttrs) pname;
    data = ./deps.json;
  };

  __darwinAllowLocalNetworking = true;

  installPhase = ''
    runHook preInstall

    # this was never reached...

    runHook postInstall
  '';

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

  meta = {
    description = "The Interactive and Stylish Replacement for ls & cd";
    longDescription = ''
      To make use of nav, add the following lines to your configuration:
      `programs.bash.shellInit = "eval \"$(nav --init bash)\"";` and
      `programs.zsh.shellInit = "eval \"$(nav --init zsh)\"";`
    '';
    homepage = "https://github.com/Jojo4GH/nav";
    license = lib.licenses.mit;
    maintainers = with lib.maintainers; [
      David-Kopczynski
      Jojo4GH
    ];
    platforms = lib.platforms.linux;
    sourceProvenance = with lib.sourceTypes; [
      fromSource
      binaryBytecode # for gradle dependencies
    ];
    mainProgram = "nav";
  };
})

Copy link
Contributor

@steeleduncan steeleduncan Nov 17, 2024

Choose a reason for hiding this comment

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

I've left an approving review as I don't see any real problem with packaging it as you have, and I wouldn't want to do anything to discourage someone with the commit bit merging it

I'm curious about the new gradle2nix though, so I'll have a try packaging with the v2 version, and if successful that can go as a separate PR once this is merged

@ofborg ofborg bot added 8.has: package (new) This PR adds a new package 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1 labels Nov 15, 2024
pkgs/by-name/na/nav/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/na/nav/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/na/nav/package.nix Outdated Show resolved Hide resolved
@David-Kopczynski
Copy link
Contributor Author

Thanks @FliegendeWurst for the improvements :)

@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Nov 29, 2024
@David-Kopczynski
Copy link
Contributor Author

@steeleduncan I will look into gradle2nix once I see further activity. Apart from that, ofborg seems to be finally happy with my commit :)

@FliegendeWurst FliegendeWurst added the backport release-24.11 Backport PR automatically label Dec 12, 2024
@FliegendeWurst FliegendeWurst merged commit 52a2613 into NixOS:master Dec 12, 2024
37 checks passed
@nix-backports
Copy link

nix-backports bot commented Dec 12, 2024

Successfully created backport PR for release-24.11:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1 11.by: package-maintainer This PR was created by the maintainer of the package it changes 12.approvals: 1 This PR was reviewed and approved by one reputable person 12. first-time contribution This PR is the author's first one; please be gentle! backport release-24.11 Backport PR automatically
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants