~andir/nixpkgs-dev

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
9 4

[PATCH] git: Tell users how to install removed sub-commands

Klemens Nanni <klemens@posteo.de>
Details
Message ID
<20201102224317.39935-1-klemens@posteo.de>
DKIM signature
missing
Download raw message
Patch: +7 -1
git is one of those packages that come with a `*Full` package as well,
i.e. some commands like git-send-email(1) are omitted in the "normal"
package and left to `gitFull` due to its dependencies.

Nothing around these packages indicates any differences and users are
left with reading the code to find out which is best to install;
furthermore, error messages caused by such nixpkgs implementation
details look like error messages from git(1) proper:

	$ nix-shell -p git
	nix-shell$ git send-email -h
	git: 'send-email' is not a git command. See 'git --help'.

This is extremely bad user experience, especially given that manuals are
still installed nonetheless, possibly leading users into debugging git
itself for no reason:

	$ man -w git-send-email
	/nix/store/fayjl0glacc00mk037nzvs0mnjysnz5v-git-2.28.0/share/man/man1/git-send-email.1.gz

Until the commit below nixpkgs would hint at building git with custom
options;  while I agree that error messages should not contain
implementation details and build instructions, I fail to see how falling
back to the above demonstrated behaviour is any better...

	commit a68c12d35b64bf4b97e772860967af1ea085cdf4
	Author: Shea Levy <shea@shealevy.com>
	Date:   Sat Apr 5 18:39:53 2014 -0400

	    git: Remove phony not-supported wrappers.

	    Fixes #1751

To aid users in finding the right package quickly, tell about `gitFull`
when "bloated" features were tried with the `git` package installed:

	$ nix-shell -p git
	nix-shell$ git send-email -h
	nixpkgs: git-send-email is not available, install nixos.gitFull instead.
---
 .../version-management/git-and-tools/git/default.nix      | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/pkgs/applications/version-management/git-and-tools/git/default.nix b/pkgs/applications/version-management/git-and-tools/git/default.nix
index 7e40366142a..34ac9a63c6f 100644
--- a/pkgs/applications/version-management/git-and-tools/git/default.nix
+++ b/pkgs/applications/version-management/git-and-tools/git/default.nix
@@ -141,7 +141,13 @@ stdenv.mkDerivation {
  postInstall =
    ''
      notSupported() {
        unlink $1 || true
        # XXX using a here-document or different quoting patterns causes
        # unexpected shell errors (somewhere in nixpkgs)
        {
          echo '#!/bin/sh'
          echo 'printf "nixpkgs: %s is not available, install nixos.gitFull instead.\n" "$(basename "$0")" 1>&2'
          echo 'exit 1'
        } >| "$1"
      }

      # Install git-subtree.
-- 
2.28.0
Details
Message ID
<20201103113209.zg7hxrdzljn7i6gq@wrt>
In-Reply-To
<20201102224317.39935-1-klemens@posteo.de> (view parent)
DKIM signature
missing
Download raw message
On 23:43 02.11.20, Klemens Nanni wrote:
> git is one of those packages that come with a `*Full` package as well,
> i.e. some commands like git-send-email(1) are omitted in the "normal"
> package and left to `gitFull` due to its dependencies.
> 
> Nothing around these packages indicates any differences and users are
> left with reading the code to find out which is best to install;
> furthermore, error messages caused by such nixpkgs implementation
> details look like error messages from git(1) proper:
> 
> 	$ nix-shell -p git
> 	nix-shell$ git send-email -h
> 	git: 'send-email' is not a git command. See 'git --help'.
> 
> This is extremely bad user experience, especially given that manuals are
> still installed nonetheless, possibly leading users into debugging git
> itself for no reason:
> 
> 	$ man -w git-send-email
> 	/nix/store/fayjl0glacc00mk037nzvs0mnjysnz5v-git-2.28.0/share/man/man1/git-send-email.1.gz
> 
> Until the commit below nixpkgs would hint at building git with custom
> options;  while I agree that error messages should not contain
> implementation details and build instructions, I fail to see how falling
> back to the above demonstrated behaviour is any better...
> 
> 	commit a68c12d35b64bf4b97e772860967af1ea085cdf4
> 	Author: Shea Levy <shea@shealevy.com>
> 	Date:   Sat Apr 5 18:39:53 2014 -0400
> 
> 	    git: Remove phony not-supported wrappers.
> 
> 	    Fixes #1751
> 
> To aid users in finding the right package quickly, tell about `gitFull`
> when "bloated" features were tried with the `git` package installed:
> 
> 	$ nix-shell -p git
> 	nix-shell$ git send-email -h
> 	nixpkgs: git-send-email is not available, install nixos.gitFull instead.

The motivation is nice an clear. Thank you for that one!

I am not so sure if we should have the `nixos.` prefix on the package
name as nixpkgs isn't just used on NixOS but also on other systems
(FreeBSD, Darwin, Ubuntu) where the reference to nixos might not be
helpful. I'd just drop that prefix. Since this comes out of a Nix build
we can probably assume the user can do that connection.

> diff --git a/pkgs/applications/version-management/git-and-tools/git/default.nix b/pkgs/applications/version-management/git-and-tools/git/default.nix
> index 7e40366142a..34ac9a63c6f 100644
> --- a/pkgs/applications/version-management/git-and-tools/git/default.nix
> +++ b/pkgs/applications/version-management/git-and-tools/git/default.nix
> @@ -141,7 +141,13 @@ stdenv.mkDerivation {
>    postInstall =
>      ''
>        notSupported() {
> -        unlink $1 || true
> +        # XXX using a here-document or different quoting patterns causes
> +        # unexpected shell errors (somewhere in nixpkgs)
> +        {
> +          echo '#!/bin/sh'
> +          echo 'printf "nixpkgs: %s is not available, install nixos.gitFull instead.\n" "$(basename "$0")" 1>&2'
> +          echo 'exit 1'
> +        } >| "$1"
>        }

Usually we try to be explicit about dependencies. In this case that
means not writing `/bin/sh` in the script as it might not exist or might
be a different implementation during runtime. You can fix the absolut
shell path and the shell escape hell by doing something like this
instead:

> @@ -141,7 +141,10 @@ stdenv.mkDerivation {
>    postInstall =
>      ''
>        notSupported() {
> -        unlink $1 || true
> +        ln -sf ${writeShellScript "git-command-not-supported" ''
> +          printf 'nixpkgs: %s is not available, install nixos.gitFull instead.\n' "$(basename "$0")"
> +          exit 1
> +        ''} "$1"
>        }

The `writeShellScript` function does create a script that starts with a
proper shebang (that points to the stdenv default shell in the nix
store) and writes the given text as body of the script. The "returned"
value is the path to the script.

So in this case we are writing a proper shell script and then just
symlinking it to the desired commands location.
Klemens Nanni <klemens@posteo.de>
Details
Message ID
<20201103201355.GD28765@eru>
In-Reply-To
<20201103113209.zg7hxrdzljn7i6gq@wrt> (view parent)
DKIM signature
missing
Download raw message
On Tue, Nov 03, 2020 at 12:32:09PM +0100, andi@srht.l.notmuch.email wrote:
> I am not so sure if we should have the `nixos.` prefix on the package
> name as nixpkgs isn't just used on NixOS but also on other systems
> (FreeBSD, Darwin, Ubuntu) where the reference to nixos might not be
> helpful. I'd just drop that prefix. Since this comes out of a Nix build
> we can probably assume the user can do that connection.
The point was to disambiguate from git, i.e. make clear that this is an error
the build system produced, not git itself - that is an important detail for the
user, I'd say.

Fair point wrt. portability, I'd simply go for "nix: " as prefix instead.

> > diff --git a/pkgs/applications/version-management/git-and-tools/git/default.nix b/pkgs/applications/version-management/git-and-tools/git/default.nix
> > index 7e40366142a..34ac9a63c6f 100644
> > --- a/pkgs/applications/version-management/git-and-tools/git/default.nix
> > +++ b/pkgs/applications/version-management/git-and-tools/git/default.nix
> > @@ -141,7 +141,13 @@ stdenv.mkDerivation {
> >    postInstall =
> >      ''
> >        notSupported() {
> > -        unlink $1 || true
> > +        # XXX using a here-document or different quoting patterns causes
> > +        # unexpected shell errors (somewhere in nixpkgs)
> > +        {
> > +          echo '#!/bin/sh'
> > +          echo 'printf "nixpkgs: %s is not available, install nixos.gitFull instead.\n" "$(basename "$0")" 1>&2'
> > +          echo 'exit 1'
> > +        } >| "$1"
> >        }
> 
> Usually we try to be explicit about dependencies. In this case that
> means not writing `/bin/sh` in the script as it might not exist or might
> be a different implementation during runtime. You can fix the absolut
> shell path and the shell escape hell by doing something like this
> instead:
I'm pretty sure that nix replaced the sheband with a more complex bash one after
I built and tested my diff like so:

	$ nix-build `git rev-parse --show-toplevel` -A git

and peeked at result/libexec/git-core/git-send-email;  I don't have that build
at hand anymore, though.

> > @@ -141,7 +141,10 @@ stdenv.mkDerivation {
> >    postInstall =
> >      ''
> >        notSupported() {
> > -        unlink $1 || true
> > +        ln -sf ${writeShellScript "git-command-not-supported" ''
> > +          printf 'nixpkgs: %s is not available, install nixos.gitFull instead.\n' "$(basename "$0")"
> > +          exit 1
> > +        ''} "$1"
> >        }
> 
> The `writeShellScript` function does create a script that starts with a
> proper shebang (that points to the stdenv default shell in the nix
> store) and writes the given text as body of the script. The "returned"
> value is the path to the script.
That's a lot better wrt. (nested) shell code, thanks.  I had to enlist this
function in the file's header (not sure what to call this).

Grepping the tree also reveals writeShellScriptBin but I could not find the
difference at a glance and documentation has failed me so far, i.e.
https://nixos.org/manual/nixpkgs/unstable/ does not mention those functions,
this pages search functionality only searches packages it seems,
https://nixos.wiki/index.php?search=writeShellScript&go=Go yields no result
and search for "nix manual writeShellScript" only points me at example code or
related sites.

Can you tell where to read up on this function?  What are the differences?

> So in this case we are writing a proper shell script and then just
> symlinking it to the desired commands location.
Will it be the same file across all invocations, i.e. will different sub-commands
wrapped by it share the same identical file?
Details
Message ID
<20201105135931.2vmxsbhdq2rl5xwa@wrt>
In-Reply-To
<20201103201355.GD28765@eru> (view parent)
DKIM signature
missing
Download raw message
On 21:13 03.11.20, Klemens Nanni wrote:
> On Tue, Nov 03, 2020 at 12:32:09PM +0100, andi@srht.l.notmuch.email wrote:
> > I am not so sure if we should have the `nixos.` prefix on the package
> > name as nixpkgs isn't just used on NixOS but also on other systems
> > (FreeBSD, Darwin, Ubuntu) where the reference to nixos might not be
> > helpful. I'd just drop that prefix. Since this comes out of a Nix build
> > we can probably assume the user can do that connection.
> The point was to disambiguate from git, i.e. make clear that this is an error
> the build system produced, not git itself - that is an important detail for the
> user, I'd say.
> 
> Fair point wrt. portability, I'd simply go for "nix: " as prefix instead.
> 
> > > diff --git a/pkgs/applications/version-management/git-and-tools/git/default.nix b/pkgs/applications/version-management/git-and-tools/git/default.nix
> > > index 7e40366142a..34ac9a63c6f 100644
> > > --- a/pkgs/applications/version-management/git-and-tools/git/default.nix
> > > +++ b/pkgs/applications/version-management/git-and-tools/git/default.nix
> > > @@ -141,7 +141,13 @@ stdenv.mkDerivation {
> > >    postInstall =
> > >      ''
> > >        notSupported() {
> > > -        unlink $1 || true
> > > +        # XXX using a here-document or different quoting patterns causes
> > > +        # unexpected shell errors (somewhere in nixpkgs)
> > > +        {
> > > +          echo '#!/bin/sh'
> > > +          echo 'printf "nixpkgs: %s is not available, install nixos.gitFull instead.\n" "$(basename "$0")" 1>&2'
> > > +          echo 'exit 1'
> > > +        } >| "$1"
> > >        }
> > 
> > Usually we try to be explicit about dependencies. In this case that
> > means not writing `/bin/sh` in the script as it might not exist or might
> > be a different implementation during runtime. You can fix the absolut
> > shell path and the shell escape hell by doing something like this
> > instead:
> I'm pretty sure that nix replaced the sheband with a more complex bash one after
> I built and tested my diff like so:
> 
> 	$ nix-build `git rev-parse --show-toplevel` -A git
> 
> and peeked at result/libexec/git-core/git-send-email;  I don't have that build
> at hand anymore, though.
> 
> > > @@ -141,7 +141,10 @@ stdenv.mkDerivation {
> > >    postInstall =
> > >      ''
> > >        notSupported() {
> > > -        unlink $1 || true
> > > +        ln -sf ${writeShellScript "git-command-not-supported" ''
> > > +          printf 'nixpkgs: %s is not available, install nixos.gitFull instead.\n' "$(basename "$0")"
> > > +          exit 1
> > > +        ''} "$1"
> > >        }
> > 
> > The `writeShellScript` function does create a script that starts with a
> > proper shebang (that points to the stdenv default shell in the nix
> > store) and writes the given text as body of the script. The "returned"
> > value is the path to the script.
> That's a lot better wrt. (nested) shell code, thanks.  I had to enlist this
> function in the file's header (not sure what to call this).

I am not sure there is an explicit term here but usually those are
imports or (injected) dependencies.


> Grepping the tree also reveals writeShellScriptBin but I could not find the
> difference at a glance and documentation has failed me so far, i.e.
> https://nixos.org/manual/nixpkgs/unstable/ does not mention those functions,
> this pages search functionality only searches packages it seems,
> https://nixos.wiki/index.php?search=writeShellScript&go=Go yields no result
> and search for "nix manual writeShellScript" only points me at example code or
> related sites.
> 
> Can you tell where to read up on this function?  What are the differences?

The source code of these function is documented fairly well. I've
recently started adding them to the generated nixpkgs manual but that
work is still pretty mich in-progress.

You can search all the Nix repos via the webbrowser here: https://search.nix.gsc.io/?q=writeShellScriptBin%20%3D&i=nope&files=&repos=

The file with the implementation (and docstrings) can be found here: https://github.com/NixOS/nixpkgs/blob/bd40824f97242271ec1704763bb40874ef05c9fc/pkgs/build-support/trivial-builders.nix#L192-L217

> > So in this case we are writing a proper shell script and then just
> > symlinking it to the desired commands location.
> Will it be the same file across all invocations, i.e. will different sub-commands
> wrapped by it share the same identical file?

Yes, it will be the same file for all the the calls. The string
interpolation is done once while Nix expression is being evaluated. In
the resulting builder script there will only be a reference to the
script in the path but no instructions on how that came to be (that is
handled through the derivation inputs, implicitly).
Klemens Nanni <klemens@posteo.de>
Details
Message ID
<X7wy69aWy5wHZjsG@eru>
In-Reply-To
<20201105135931.2vmxsbhdq2rl5xwa@wrt> (view parent)
DKIM signature
missing
Download raw message
Patch: +9 -5
On Thu, Nov 05, 2020 at 02:59:31PM +0100, andreas@rammhold.de wrote:
> The file with the implementation (and docstrings) can be found here: https://github.com/NixOS/nixpkgs/blob/bd40824f97242271ec1704763bb40874ef05c9fc/pkgs/build-support/trivial-builders.nix#L192-L217

Thanks, so the difference between `writeShellScript{,Bin}' is that the latter
merely puts it under a different path inside the nix store.

> Yes, it will be the same file for all the the calls. The string
> interpolation is done once while Nix expression is being evaluated. In
> the resulting builder script there will only be a reference to the
> script in the path but no instructions on how that came to be (that is
> handled through the derivation inputs, implicitly).

Cool, as desired.

Updated patch below, still with a prefix in the error message but being "nix:"
this time;  as elaborated in my previous mail, I still think it is worth it to
clearly tell nix made errors from git made errors apart.

Is that acceptable for merge?

-- >8 --
Subject: [PATCH] git: Tell users how to install removed sub-commands

git is one of those packages that come with a `*Full` package as well,
i.e. some commands like git-send-email(1) are omitted in the "normal"
package and left to `gitFull` due to its dependencies.

Nothing around these packages indicates any differences and users are
left with reading the code to find out which is best to install;
furthermore, error messages caused by such nixpkgs implementation
details look like error messages from git(1) proper:

	$ nix-shell -p git
	nix-shell$ git send-email -h
	git: 'send-email' is not a git command. See 'git --help'.

This is extremely bad user experience, especially given that manuals are
still installed nonetheless, possibly leading users into debugging git
itself for no reason:

	$ man -w git-send-email
	/nix/store/fayjl0glacc00mk037nzvs0mnjysnz5v-git-2.28.0/share/man/man1/git-send-email.1.gz

Until the commit below nixpkgs would hint at building git with custom
options;  while I agree that error messages should not contain
implementation details and build instructions, I fail to see how falling
back to the above demonstrated behaviour is any better...

	commit a68c12d35b64bf4b97e772860967af1ea085cdf4
	Author: Shea Levy <shea@shealevy.com>
	Date:   Sat Apr 5 18:39:53 2014 -0400

	    git: Remove phony not-supported wrappers.

	    Fixes #1751

To aid users in finding the right package quickly, tell about `gitFull`
when "bloated" features were tried with the `git` package installed:

	$ nix-shell -p git
	nix-shell$ git send-email -h
	nix: 'git-send-email' is not available, install nixos.gitFull instead.
---
 .../git-and-tools/git/default.nix                  | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/pkgs/applications/version-management/git-and-tools/git/default.nix b/pkgs/applications/version-management/git-and-tools/git/default.nix
index b6efb78513c..6eb914ba311 100644
--- a/pkgs/applications/version-management/git-and-tools/git/default.nix
+++ b/pkgs/applications/version-management/git-and-tools/git/default.nix
@@ -16,6 +16,7 @@
, withLibsecret ? false
, pkgconfig, glib, libsecret
, gzip # needed at runtime by gitweb.cgi
, writeShellScript
}:

assert sendEmailSupport -> perlSupport;
@@ -140,8 +141,11 @@ stdenv.mkDerivation {

  postInstall =
    ''
      notSupported() {
        unlink $1 || true
      notAvailable() {
          ln -sf ${writeShellScript "git-command-not-available" ''
            printf "nix: '%s' is not available, install nixos.gitFull instead.\n" "$(basename "$0")" 1>&2
            exit 1
          ''} "$1"
      }

      # Install git-subtree.
@@ -217,7 +221,7 @@ stdenv.mkDerivation {
                     --set GITPERLLIB "$out/${perlPackages.perl.libPrefix}:${perlPackages.makePerlPath (perlLibs ++ [svn.out])}" \
                     --prefix PATH : "${svn.out}/bin" ''
       else '' # replace git-svn by notification script
        notSupported $out/libexec/git-core/git-svn
        notAvailable $out/libexec/git-core/git-svn
     '')

   + (if sendEmailSupport then ''
@@ -226,7 +230,7 @@ stdenv.mkDerivation {
                     --set GITPERLLIB "$out/${perlPackages.perl.libPrefix}:${perlPackages.makePerlPath smtpPerlLibs}"
      '' else ''
        # replace git-send-email by notification script
        notSupported $out/libexec/git-core/git-send-email
        notAvailable $out/libexec/git-core/git-send-email
      '')

   + stdenv.lib.optionalString withManual ''# Install man pages and Info manual
@@ -244,7 +248,7 @@ stdenv.mkDerivation {
     '' else ''
       # Don't wrap Tcl/Tk, replace them by notification scripts
       for prog in bin/gitk libexec/git-core/git-gui; do
         notSupported "$out/$prog"
         notAvailable "$out/$prog"
       done
     '')
   + stdenv.lib.optionalString stdenv.isDarwin ''
-- 
2.29.2
Details
Message ID
<20201124163834.qsjb76vbkaqn4cao@ws.flokli.de>
In-Reply-To
<X7wy69aWy5wHZjsG@eru> (view parent)
DKIM signature
missing
Download raw message
s/install nixos.gitFull instead/use nixos.gitFull instead/, otherwise
LGTM. Haven't tested it yet, though.

On 20-11-23 23:08:43, Klemens Nanni wrote:
>On Thu, Nov 05, 2020 at 02:59:31PM +0100, andreas@rammhold.de wrote:
>> The file with the implementation (and docstrings) can be found here: https://github.com/NixOS/nixpkgs/blob/bd40824f97242271ec1704763bb40874ef05c9fc/pkgs/build-support/trivial-builders.nix#L192-L217
>
>Thanks, so the difference between `writeShellScript{,Bin}' is that the latter
>merely puts it under a different path inside the nix store.
>
>> Yes, it will be the same file for all the the calls. The string
>> interpolation is done once while Nix expression is being evaluated. In
>> the resulting builder script there will only be a reference to the
>> script in the path but no instructions on how that came to be (that is
>> handled through the derivation inputs, implicitly).
>
>Cool, as desired.
>
>Updated patch below, still with a prefix in the error message but being "nix:"
>this time;  as elaborated in my previous mail, I still think it is worth it to
>clearly tell nix made errors from git made errors apart.
>
>Is that acceptable for merge?
>
>-- >8 --
>Subject: [PATCH] git: Tell users how to install removed sub-commands
>
>git is one of those packages that come with a `*Full` package as well,
>i.e. some commands like git-send-email(1) are omitted in the "normal"
>package and left to `gitFull` due to its dependencies.
>
>Nothing around these packages indicates any differences and users are
>left with reading the code to find out which is best to install;
>furthermore, error messages caused by such nixpkgs implementation
>details look like error messages from git(1) proper:
>
>	$ nix-shell -p git
>	nix-shell$ git send-email -h
>	git: 'send-email' is not a git command. See 'git --help'.
>
>This is extremely bad user experience, especially given that manuals are
>still installed nonetheless, possibly leading users into debugging git
>itself for no reason:
>
>	$ man -w git-send-email
>	/nix/store/fayjl0glacc00mk037nzvs0mnjysnz5v-git-2.28.0/share/man/man1/git-send-email.1.gz
>
>Until the commit below nixpkgs would hint at building git with custom
>options;  while I agree that error messages should not contain
>implementation details and build instructions, I fail to see how falling
>back to the above demonstrated behaviour is any better...
>
>	commit a68c12d35b64bf4b97e772860967af1ea085cdf4
>	Author: Shea Levy <shea@shealevy.com>
>	Date:   Sat Apr 5 18:39:53 2014 -0400
>
>	    git: Remove phony not-supported wrappers.
>
>	    Fixes #1751
>
>To aid users in finding the right package quickly, tell about `gitFull`
>when "bloated" features were tried with the `git` package installed:
>
>	$ nix-shell -p git
>	nix-shell$ git send-email -h
>	nix: 'git-send-email' is not available, install nixos.gitFull instead.
>---
> .../git-and-tools/git/default.nix                  | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
>diff --git a/pkgs/applications/version-management/git-and-tools/git/default.nix b/pkgs/applications/version-management/git-and-tools/git/default.nix
>index b6efb78513c..6eb914ba311 100644
>--- a/pkgs/applications/version-management/git-and-tools/git/default.nix
>+++ b/pkgs/applications/version-management/git-and-tools/git/default.nix
>@@ -16,6 +16,7 @@
> , withLibsecret ? false
> , pkgconfig, glib, libsecret
> , gzip # needed at runtime by gitweb.cgi
>+, writeShellScript
> }:
>
> assert sendEmailSupport -> perlSupport;
>@@ -140,8 +141,11 @@ stdenv.mkDerivation {
>
>   postInstall =
>     ''
>-      notSupported() {
>-        unlink $1 || true
>+      notAvailable() {
>+          ln -sf ${writeShellScript "git-command-not-available" ''
>+            printf "nix: '%s' is not available, install nixos.gitFull instead.\n" "$(basename "$0")" 1>&2
>+            exit 1
>+          ''} "$1"
>       }
>
>       # Install git-subtree.
>@@ -217,7 +221,7 @@ stdenv.mkDerivation {
>                      --set GITPERLLIB "$out/${perlPackages.perl.libPrefix}:${perlPackages.makePerlPath (perlLibs ++ [svn.out])}" \
>                      --prefix PATH : "${svn.out}/bin" ''
>        else '' # replace git-svn by notification script
>-        notSupported $out/libexec/git-core/git-svn
>+        notAvailable $out/libexec/git-core/git-svn
>      '')
>
>    + (if sendEmailSupport then ''
>@@ -226,7 +230,7 @@ stdenv.mkDerivation {
>                      --set GITPERLLIB "$out/${perlPackages.perl.libPrefix}:${perlPackages.makePerlPath smtpPerlLibs}"
>       '' else ''
>         # replace git-send-email by notification script
>-        notSupported $out/libexec/git-core/git-send-email
>+        notAvailable $out/libexec/git-core/git-send-email
>       '')
>
>    + stdenv.lib.optionalString withManual ''# Install man pages and Info manual
>@@ -244,7 +248,7 @@ stdenv.mkDerivation {
>      '' else ''
>        # Don't wrap Tcl/Tk, replace them by notification scripts
>        for prog in bin/gitk libexec/git-core/git-gui; do
>-         notSupported "$out/$prog"
>+         notAvailable "$out/$prog"
>        done
>      '')
>    + stdenv.lib.optionalString stdenv.isDarwin ''
>-- 
>2.29.2

-- 
Florian Klink
Klemens Nanni <klemens@posteo.de>
Details
Message ID
<X706BtDRvZnRcaQb@eru>
In-Reply-To
<20201124163834.qsjb76vbkaqn4cao@ws.flokli.de> (view parent)
DKIM signature
missing
Download raw message
On Tue, Nov 24, 2020 at 05:38:34PM +0100, Florian Klink wrote:
> s/install nixos.gitFull instead/use nixos.gitFull instead/, otherwise
> LGTM. Haven't tested it yet, though.
*User has "git" installed, sees the error and decides to do a one-off to
for git-send-email(1) instead of switching permanently*:

	$ nix-shell -p gitFull --run 'git send-email ...'

Does that count as installation or usage?  I think either is fine but
only the latter includes the former;  I'll gladly send an updated and
final diff if need be to avoid bike-shedding once overall consensus is
reached but will avoid such churn for now.
Details
Message ID
<20201124180635.w37kfdcwkzrwdgx2@ws.flokli.de>
In-Reply-To
<X706BtDRvZnRcaQb@eru> (view parent)
DKIM signature
missing
Download raw message
>> s/install nixos.gitFull instead/use nixos.gitFull instead/, otherwise
>> LGTM. Haven't tested it yet, though.
>*User has "git" installed, sees the error and decides to do a one-off to
>for git-send-email(1) instead of switching permanently*:
>
>	$ nix-shell -p gitFull --run 'git send-email ...'
>
>Does that count as installation or usage?  I think either is fine but
>only the latter includes the former;  I'll gladly send an updated and
>final diff if need be to avoid bike-shedding once overall consensus is
>reached but will avoid such churn for now.

This is a usage, not an installation, "installing" is very much
understood to be "adding it to your profile, either by changing the
configuration and rebuilding the config, or by manually, imperatively
adding it to your profile via `nix-env --install …`, which we shouldn't
encourage users to do.

But yeah, no need to re-roll the patch if you don't want to, let's give
this some more testing first.
Klemens Nanni <klemens@posteo.de>
Details
Message ID
<X9oemzzRCMjeUUVk@eru>
In-Reply-To
<20201124180635.w37kfdcwkzrwdgx2@ws.flokli.de> (view parent)
DKIM signature
missing
Download raw message
On Tue, Nov 24, 2020 at 07:06:35PM +0100, Florian Klink wrote:
> But yeah, no need to re-roll the patch if you don't want to, let's give
> this some more testing first.
Is there any chance you've carried out "some more testing" by now?

Would it be possible for you to open a PR with this change?
Sadly enough, it seems the community is not yet interested in alternative
ways of contribution.
Details
Message ID
<20201217115753.d34et5al72pb2sfh@ws.flokli.de>
In-Reply-To
<X9oemzzRCMjeUUVk@eru> (view parent)
DKIM signature
missing
Download raw message
On 20-12-16 15:50:03, Klemens Nanni wrote:
>On Tue, Nov 24, 2020 at 07:06:35PM +0100, Florian Klink wrote:
>> But yeah, no need to re-roll the patch if you don't want to, let's give
>> this some more testing first.
>Is there any chance you've carried out "some more testing" by now?

I didn't do some testing, and currently don't have the bandwidth to do
so. This should be made more visible to the broader community (through a
PR, and a post in discourse for example).

>Would it be possible for you to open a PR with this change?
>Sadly enough, it seems the community is not yet interested in alternative
>ways of contribution.

I'd be very interested in something like a self-hosted Gerrit for
nixpkgs, which allows a way better modelling of relationship between
commits, tagging of "subsystem" maintainers etc, more sophisticated
checks.
I'd be very interested to bounce some ideas on this back and forth some
time in the future.

Personally, I also don't think having others relay PRs for you is an
efficient interim solution. There will be back and forth, and I
personally don't have the time to play human proxy in between, as done
in https://github.com/NixOS/nixpkgs/pull/106995 for example.

As for this patch, it also still misses the changes requested in
20201124163834.qsjb76vbkaqn4cao@ws.flokli.de. Sending a new version
including this changes would at least avoid one round-trip that another
human proxy would need to make.

-- 
flokli
Reply to thread Export thread (mbox)