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
Add a script that "diffs" nix environment generations #22156
Conversation
@LnL7 what do you think? |
I don't think I need to use eval in the |
Anybody have a preference of the "default" generations to diff, if any? |
maintainers/scripts/lib_bash/temp
Outdated
@@ -0,0 +1,27 @@ | |||
#!/bin/bash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This probably shouldn't have a shebang if it's only meant to be sourced.
If it should have a shebang, probably should have it match the one in nix-diff.sh at least.
maintainers/scripts/nix-diff.sh
Outdated
@@ -0,0 +1,71 @@ | |||
#!/usr/bin/env bash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see other scripts here using /usr/bin/env bash
but perhaps this should be using a nix-shell shebang?
The 'fetch-kde-qt.sh' script is a good example:
https://github.com/NixOS/nixpkgs/blob/master/maintainers/scripts/fetch-kde-qt.sh#L1
It also ensures you have whatever dependencies you need, like the coreutils bit you mention.
Since this script uses nix tools already it seems safe to assume nix-shell is available :).
No preference re:default generations, although that's mostly because I can't think of reasonable defaults :). Maybe make the 'to' generation optional, defaulting to the current one? Also-- don't suppose this could also be used for diff'ing NixOS generations? ... Sorry for the feature creep suggestions but just thinking out loud a bit. |
I think, you should add more information to commit message and/or script header about what is this, what problem does it solve and basic example of a usage. |
maintainers/scripts/nix-diff.sh
Outdated
eval add=$(temp_name) | ||
[ -n "$DEBUG" ] && echo DEBUG: "add=$add" >&2 | ||
sed -r 's:^[^-]*-(.*)$:\1+:' <(echo "$in") \ | ||
| grep -v env-manifest.nix \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This only works for imperative user profiles (nix-env), perhaps the script name should reflect that?
maintainers/scripts/nix-diff.sh
Outdated
@@ -0,0 +1,71 @@ | |||
#!/usr/bin/env bash | |||
|
|||
SCRIPT_PATH=$(realpath "${BASH_SOURCE[0]}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
realpath
isn't available on darwin by default.
see also NixOS/nix#1164 |
@shanemikel what's happening with this PR? |
@puffnfresh sorry, I will try to get back to this over the weekend. Right now I'm dealing with some exams and essays :( |
Sorry, I kind of lost track of this. |
no worries @LnL7, the ball wasn't in your court anyways. I closed it, planning to improve it, but if @puffnfresh wants to use it now, I can see no harm in merging and my submitting another PR later upon revision. |
Motivation for this change
I found a script like this in matthiasbeyer/nixos-scripts, but it didn't seem to work. I think it's a useful thing to have, and would like to see it eventually usable via the Nix CLI. I saw this feature in Guix, and was impressed. I think it should work for most, but it has a dependency on a coreutils command which isn't present on OSX out of the box (but is present in the nixpkgs coreutils package).
Not sure how people will feel about the style. Like any code, I try to abstract my Bash, which tends to be hackish and clunky. Anyway, I think I found a nice way to do something like an import/include... If y'all don't like it, I can just copy/paste the lib_bash/temp code directly into the script.