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

Investigate replacing sort with nix eval #460

Open
bbenne10 opened this issue Jan 4, 2024 · 3 comments
Open

Investigate replacing sort with nix eval #460

bbenne10 opened this issue Jan 4, 2024 · 3 comments

Comments

@bbenne10
Copy link
Contributor

bbenne10 commented Jan 4, 2024

Nix ships with nix eval and builtins.compareVersions. Is there some reason we can't replace our version sorting/checks with this?

I think it'd be something like the following:

_require_version() {
  local cmd=$1 version=$2 required=$3
  if [ $(_nix eval --expr "builtins.compareVersions \"$required\" \"$version\"') = -1 ]; then
    _nix_direnv_error \
      "minimum required $(basename "$cmd") version is $required (installed: $version)"
    return 1
  fi

This should reduce our dependency on external tools (or maybe you can think of it as deepening our dependency on one we certainly can't remove?)

@Mic92
Copy link
Member

Mic92 commented Jan 5, 2024

Looks reasonable fast:

$ time nix eval --expr 'builtins.compareVersions "1.0.0" "2.0.0"'
nix eval --expr 'builtins.compareVersions "1.0.0" "2.0.0"'  0.01s user 0.01s system 99% cpu 0.017 total
$ time bash -c "echo '1.0.0' '2.0.0' | sort -C -V"
bash -c "echo '1.0.0' '2.0.0' | sort -C -V"  0.00s user 0.00s system 96% cpu 0.006 total

@Mic92
Copy link
Member

Mic92 commented Jan 5, 2024

Also we don't need to worry that much anymore. I think with the current flags, it works with coreutils/macOS/freebsd/openbsd/busybox/toybox.

@bbenne10
Copy link
Contributor Author

bbenne10 commented Jan 5, 2024

I'm not particularly worried about this in the general sense. I happened to be poking in the nix builtins and nixpkgs.lib for unrelated reasons and stumbled on builtins.compareVersions. I may still put a PR together for this, just to explore it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants