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

Add a script that "diffs" nix environment generations #22156

Closed
wants to merge 0 commits into from
Closed

Add a script that "diffs" nix environment generations #22156

wants to merge 0 commits into from

Conversation

shanemikel
Copy link
Contributor

@shanemikel shanemikel commented Jan 25, 2017

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.

@shanemikel
Copy link
Contributor Author

@LnL7 what do you think?

@shanemikel
Copy link
Contributor Author

shanemikel commented Jan 27, 2017

I don't think I need to use eval in the temp library. I'm gonna close and resubmit eventually.. also, the use of a "library" is still open for discussion.

@shanemikel shanemikel closed this Jan 27, 2017
@shanemikel
Copy link
Contributor Author

Anybody have a preference of the "default" generations to diff, if any?

@shanemikel shanemikel reopened this Jan 27, 2017
@@ -0,0 +1,27 @@
#!/bin/bash
Copy link
Member

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.

@@ -0,0 +1,71 @@
#!/usr/bin/env bash
Copy link
Member

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 :).

@dtzWill
Copy link
Member

dtzWill commented Jan 27, 2017

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?
If that's not a problem, I think there'd be a moderate amount of interest in some way to combine "nixos-rebuild build --upgrade && nix-diff.sh $JUST_BUILT_GENERATION $CURRENT_SYSTEM && prompt_if_should_attempt_to_do_nixos-rebuild-switch" O:).

... Sorry for the feature creep suggestions but just thinking out loud a bit.

@danbst
Copy link
Contributor

danbst commented Jan 30, 2017

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.

eval add=$(temp_name)
[ -n "$DEBUG" ] && echo DEBUG: "add=$add" >&2
sed -r 's:^[^-]*-(.*)$:\1+:' <(echo "$in") \
| grep -v env-manifest.nix \
Copy link
Member

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?

@@ -0,0 +1,71 @@
#!/usr/bin/env bash

SCRIPT_PATH=$(realpath "${BASH_SOURCE[0]}")
Copy link
Member

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.

@matthewbauer
Copy link
Member

see also NixOS/nix#1164

@shanemikel shanemikel closed this Mar 1, 2017
@puffnfresh
Copy link
Contributor

@shanemikel what's happening with this PR?

@shanemikel
Copy link
Contributor Author

@puffnfresh sorry, I will try to get back to this over the weekend. Right now I'm dealing with some exams and essays :(

@LnL7
Copy link
Member

LnL7 commented Mar 2, 2017

Sorry, I kind of lost track of this.

@shanemikel
Copy link
Contributor Author

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.

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

Successfully merging this pull request may close these issues.

None yet

7 participants