Re: [RFC/PATCH 0/5] stop installing old libexec aliases like "git-init"
Hi Peff, On Thu, 22 Nov 2018, Jeff King wrote: > On Thu, Nov 22, 2018 at 01:48:53PM +0100, Johannes Schindelin wrote: > > > So YMMV with git-s. My rule of thumb is: if I want to use this > > myself only, I'll make it an alias. If I want to ship it (e.g. with Git > > for Windows), I'll make it a git-. > > I have a handful of personal git-* scripts: mostly ones that are big > enough to be unwieldy as an alias. But then, $PATH management is pretty > straightforward on my platforms, so it's easier to drop a script there > than to point an alias to a non-git-* script. Oh, my Git aliases pretty much *all* point to a single script that takes subcommands. Ciao, Dscho
Re: [RFC/PATCH 0/5] stop installing old libexec aliases like "git-init"
On Thu, Nov 22, 2018 at 01:48:53PM +0100, Johannes Schindelin wrote: > > So it's maybe do-able, but not quite as trivial as one might hope. > > A trivial alternative would be to recommend adding a man page for > 3rd-party git-s. > > In other words, as soon as `git-sizer` is accompanied by `git-sizer.1` in > one of the appropriate locations, it is set. Yes, it would be nice if everything did ship with a manpage. Unfortunately that complicates installation, where the installation for many such tools is "save this file to your $PATH". Tools like git-sizer may be getting big and popular enough to merit the extra infrastructure, but I think there are many smaller scripts which don't. > FWIW I do have a couple of scripts I use that I install into > `$HOME/bin/git-`. Although, granted, I essentially switched to > aliases for most of them, where the aliases still call a script that is > checked out in some folder in my home directory. The reason: this works in > more circumstances, as I do not have to add `$HOME/bin` to the `PATH`, > say, in PowerShell. > > So YMMV with git-s. My rule of thumb is: if I want to use this > myself only, I'll make it an alias. If I want to ship it (e.g. with Git > for Windows), I'll make it a git-. I have a handful of personal git-* scripts: mostly ones that are big enough to be unwieldy as an alias. But then, $PATH management is pretty straightforward on my platforms, so it's easier to drop a script there than to point an alias to a non-git-* script. -Peff
Re: [RFC/PATCH 0/5] stop installing old libexec aliases like "git-init"
Hi Peff,
On Sat, 17 Nov 2018, Jeff King wrote:
> On Fri, Nov 16, 2018 at 08:22:11PM +0100, Ævar Arnfjörð Bjarmason wrote:
>
> > So maybe we should just document this interface better. It seems one
> > implicit dependency is that we expect a manpage for the tool to exist
> > for --help.
>
> Yeah, I think this really the only problematic assumption. The rest of
> "-c", "--git-dir", etc, are just manipulating the environment, and that
> gets passed through to sub-invocations of Git (so if I have a script
> which calls git-config, it will pick up the "-c" config).
>
> It would be nice if there was a way to ask "is there a manpage?", and
> fallback to running "git-cmd --help". But:
>
> 1. I don't think there is a portable way to query that via man. And
> anyway, depending platform and config, it may be opening a browser
> to show HTML documentation (or I think even texinfo?).
>
> 2. We can just ask whether "man git-sizer" (or whatever help display
> command) returned a non-zero exit code, and fall back to "git-sizer
> --help". But there's an infinite-loop possibility here: running
> "git-sizer --help" does what we want. But if "man git-log" failed,
> we'd run "git-log --help", which in turn runs "git help log", which
> runs "man git-log", and so on.
>
> You can break that loop with an environment variable for "I already
> tried to show the manpage", which would I guess convert "--help" to
> "-h".
>
> So it's maybe do-able, but not quite as trivial as one might hope.
A trivial alternative would be to recommend adding a man page for
3rd-party git-s.
In other words, as soon as `git-sizer` is accompanied by `git-sizer.1` in
one of the appropriate locations, it is set.
(Actually, it is not: on Windows, it would have to add git-sizer.html in
the appropriate location, but we can deal with this if needed.)
> > But I don't remember the details, and can't reproduce it now with:
> >
> > $ cat ~/bin/git-sigint
> > #!/usr/bin/env perl
> > $SIG{INT} = sub { warn localtime . " " . $$ };
> > sleep 1 while 1;
> > $ git sigint # or git-sigint
> > [... behaves the same either way ...]
> >
> > So maybe it was something else, or I'm misremembering...
>
> I think that generally works because hitting ^C is going to send SIGINT
> to the whole process group. A more interesting case is:
>
> git sigint &
> kill -INT $!
>
> There $! is a parent "git" process that is just waiting on git-sigint to
> die. But that works, too, because the parent relays common signals due
> to 10c6cddd92 (dashed externals: kill children on exit, 2012-01-08). So
> you might have been remembering issues prior to that commit (or uncommon
> signals; these come from sigchain_push_common).
FWIW I do have a couple of scripts I use that I install into
`$HOME/bin/git-`. Although, granted, I essentially switched to
aliases for most of them, where the aliases still call a script that is
checked out in some folder in my home directory. The reason: this works in
more circumstances, as I do not have to add `$HOME/bin` to the `PATH`,
say, in PowerShell.
So YMMV with git-s. My rule of thumb is: if I want to use this
myself only, I'll make it an alias. If I want to ship it (e.g. with Git
for Windows), I'll make it a git-.
Ciao,
Dscho
Re: [RFC/PATCH 0/5] stop installing old libexec aliases like "git-init"
On Fri, Nov 16, 2018 at 08:22:11PM +0100, Ævar Arnfjörð Bjarmason wrote:
> So maybe we should just document this interface better. It seems one
> implicit dependency is that we expect a manpage for the tool to exist
> for --help.
Yeah, I think this really the only problematic assumption. The rest of
"-c", "--git-dir", etc, are just manipulating the environment, and that
gets passed through to sub-invocations of Git (so if I have a script
which calls git-config, it will pick up the "-c" config).
It would be nice if there was a way to ask "is there a manpage?", and
fallback to running "git-cmd --help". But:
1. I don't think there is a portable way to query that via man. And
anyway, depending platform and config, it may be opening a browser
to show HTML documentation (or I think even texinfo?).
2. We can just ask whether "man git-sizer" (or whatever help display
command) returned a non-zero exit code, and fall back to "git-sizer
--help". But there's an infinite-loop possibility here: running
"git-sizer --help" does what we want. But if "man git-log" failed,
we'd run "git-log --help", which in turn runs "git help log", which
runs "man git-log", and so on.
You can break that loop with an environment variable for "I already
tried to show the manpage", which would I guess convert "--help" to
"-h".
So it's maybe do-able, but not quite as trivial as one might hope.
> But I don't remember the details, and can't reproduce it now with:
>
> $ cat ~/bin/git-sigint
> #!/usr/bin/env perl
> $SIG{INT} = sub { warn localtime . " " . $$ };
> sleep 1 while 1;
> $ git sigint # or git-sigint
> [... behaves the same either way ...]
>
> So maybe it was something else, or I'm misremembering...
I think that generally works because hitting ^C is going to send SIGINT
to the whole process group. A more interesting case is:
git sigint &
kill -INT $!
There $! is a parent "git" process that is just waiting on git-sigint to
die. But that works, too, because the parent relays common signals due
to 10c6cddd92 (dashed externals: kill children on exit, 2012-01-08). So
you might have been remembering issues prior to that commit (or uncommon
signals; these come from sigchain_push_common).
-Peff
Re: [RFC/PATCH 0/5] stop installing old libexec aliases like "git-init"
On Fri, Nov 16 2018, Michael Haggerty wrote:
> On Fri, Nov 16, 2018 at 11:38 AM Ævar Arnfjörð Bjarmason
> wrote:
>> [...]
>> A follow-up on this: We should really fix this for other
>> reasons. I.e. compile in some "this is stuff we ourselves think is in
>> git".
>>
>> There's other manifestations of this, e.g.:
>>
>> git-sizer --help # => shows you help
>> git sizer --help # => says it doesn't have a manpage
>>
>> Because we aren't aware that git-sizer is some external tool, and that
>> we should route --help to it.
>
> That would be nice. This has been an annoying for several tools named
> `git-foo` that I have worked on (e.g., git-sizer, git-imerge,
> git-when-merged, plus many internal tools).
>
>> Non-withstanding the arguable bug that things like git-sizer shouldn't
>> be allowing themselves to be invoked by "git" like that without
>> guaranteeing that it can consume all the options 'git' expects. When I
>> had to deal with a similar problem in an external git-* command I was
>> maintaining I simply made it an error to invoke it as "git mything"
>> instead of "git-mything".
>
> Hmmm, I always thought that it was intended and supported that an
> external tool can name itself `git-foo` so that it can be invoked as
> `git foo`.
>
> Which git options do you think that such a tool should be expected to
> support? Many useful ones, like `-C `, `--git-dir=`,
> `--work-tree=`, `-c =`, and `--no-replace-objects`,
> work pretty much for free if the external tool uses `git` to interact
> with the repository. I use such options regularly with external tools.
> IMO it would be a regression for these tools to refuse to run when
> invoked as, say, `git -C path/to/repo foo`.
I don't mean we should forbid it, just that if you have an external
git-foo tool meant to be invoked like "git-foo" that and not "git foo"
it might be worth to save yourself the trouble and not support the
latter. I thought git-sizer was one such tool, since it docs just
mention "git-sizer".
But yeah, "-c" etc. are useful, and we definitely want to support this
in the general case. E.g. for "git-annex" and others that are meant to
be used like that.
So maybe we should just document this interface better. It seems one
implicit dependency is that we expect a manpage for the tool to exist
for --help.
Or, keep some list of internal git tools and treat them specially. But I
see now that would break "git annex --help" in the other direction, but
maybe that would be better. I don't know.
As I recall I stopped supporting "git" invocation for the tool I was
fixing a long time ago because of some funny interaction with terminals
& signals. I.e. git itself would eat some of them, but the tool wanted
to handle it instead.
But I don't remember the details, and can't reproduce it now with:
$ cat ~/bin/git-sigint
#!/usr/bin/env perl
$SIG{INT} = sub { warn localtime . " " . $$ };
sleep 1 while 1;
$ git sigint # or git-sigint
[... behaves the same either way ...]
So maybe it was something else, or I'm misremembering...
Re: [RFC/PATCH 0/5] stop installing old libexec aliases like "git-init"
On Fri, Nov 16, 2018 at 11:38 AM Ævar Arnfjörð Bjarmason wrote: > [...] > A follow-up on this: We should really fix this for other > reasons. I.e. compile in some "this is stuff we ourselves think is in > git". > > There's other manifestations of this, e.g.: > > git-sizer --help # => shows you help > git sizer --help # => says it doesn't have a manpage > > Because we aren't aware that git-sizer is some external tool, and that > we should route --help to it. That would be nice. This has been an annoying for several tools named `git-foo` that I have worked on (e.g., git-sizer, git-imerge, git-when-merged, plus many internal tools). > Non-withstanding the arguable bug that things like git-sizer shouldn't > be allowing themselves to be invoked by "git" like that without > guaranteeing that it can consume all the options 'git' expects. When I > had to deal with a similar problem in an external git-* command I was > maintaining I simply made it an error to invoke it as "git mything" > instead of "git-mything". Hmmm, I always thought that it was intended and supported that an external tool can name itself `git-foo` so that it can be invoked as `git foo`. Which git options do you think that such a tool should be expected to support? Many useful ones, like `-C `, `--git-dir=`, `--work-tree=`, `-c =`, and `--no-replace-objects`, work pretty much for free if the external tool uses `git` to interact with the repository. I use such options regularly with external tools. IMO it would be a regression for these tools to refuse to run when invoked as, say, `git -C path/to/repo foo`. Michael
Re: [RFC/PATCH 0/5] stop installing old libexec aliases like "git-init"
On Fri, Nov 02 2018, Ævar Arnfjörð Bjarmason wrote:
> I think up to patch 4 here should be near a state that's ready for
> inclusion.
>
> Although I'm on the fence with the approach in 1/5. Should this be a
> giant getopt switch statement like that in a helper script? An
> alternative would be to write out a shell file similar to
> GIT-BUILD-OPTIONS and source that from this thing. I don't know, what
> do you all think?
>
> The idea with 4/5 was to make this symlink mode the default in
> config.mak.uname and have a blacklist of systems like Windows that
> couldn't deal with it.
>
> Since my ad874608d8 ("Makefile: optionally symlink libexec/git-core
> binaries to bin/git", 2018-03-13) I see that e.g. Debian and GitLab
> have started shipping with the INSTALL_SYMLINKS flag, so making that
> unconditional is the next logical step.
>
> The 5th one is more radical. See
> https://public-inbox.org/git/87woyfdkoi@evledraar.gmail.com/ from
> back in March for context.
>
> I'd like to say it's ready, but I've spotted some fallout:
>
> * Help like "git ninit" suggesting "git init" doesn't work, this is
>because load_command_list() in help.c doesn't look out our
>in-memory idea of builtins, it reads the libexecdir, so if we don't
>have the programs there it doesn't know about it.
A follow-up on this: We should really fix this for other
reasons. I.e. compile in some "this is stuff we ourselves think is in
git".
There's other manifestations of this, e.g.:
git-sizer --help # => shows you help
git sizer --help # => says it doesn't have a manpage
Because we aren't aware that git-sizer is some external tool, and that
we should route --help to it.
Non-withstanding the arguable bug that things like git-sizer shouldn't
be allowing themselves to be invoked by "git" like that without
guaranteeing that it can consume all the options 'git' expects. When I
had to deal with a similar problem in an external git-* command I was
maintaining I simply made it an error to invoke it as "git mything"
instead of "git-mything".
> * GIT_TEST_INSTALLED breaks entirely under this, as early as the
>heuristic for "are we built?" being "do we have git-init in
>libexecdir?". I tried a bit to make this work, but there's a lot of
>dependencies there.
>
> * We still (and this is also true of my ad874608d8) hardlink
>everything in the build dir via a different part of the Makefile,
>ideally we should do exactly the same thing there so also normal
>tests and not just GIT_TEST_INSTALLED (if that worked) would test
>in the same mode.
>
>I gave making that work a bit of a try and gave up in the Makefile
>jungle.
>
> Ævar Arnfjörð Bjarmason (5):
> Makefile: move long inline shell loops in "install" into helper
> Makefile: conform some of the code to our coding standards
> Makefile: stop hiding failures during "install"
> Makefile: add NO_INSTALL_SYMLINKS_FALLBACK switch
> Makefile: Add a NO_INSTALL_BUILTIN_EXECDIR_ALIASES flag
>
> Makefile | 65 +++--
> install_programs | 124 +++
> 2 files changed, 151 insertions(+), 38 deletions(-)
> create mode 100755 install_programs
Re: [RFC/PATCH 0/5] stop installing old libexec aliases like "git-init"
Hi Ævar, On Fri, 2 Nov 2018, Ævar Arnfjörð Bjarmason wrote: > * GIT_TEST_INSTALLED breaks entirely under this, as early as the >heuristic for "are we built?" being "do we have git-init in >libexecdir?". I tried a bit to make this work, but there's a lot of >dependencies there. I have a couple of patches in the pipeline to improve `GIT_TEST_INSTALLED`, as I needed it to work without hardlinked copies of the built-ins. These patches might help this here isue. > * We still (and this is also true of my ad874608d8) hardlink >everything in the build dir via a different part of the Makefile, >ideally we should do exactly the same thing there so also normal >tests and not just GIT_TEST_INSTALLED (if that worked) would test >in the same mode. > >I gave making that work a bit of a try and gave up in the Makefile >jungle. Yep. Ciao, Dscho
Re: [RFC/PATCH 0/5] stop installing old libexec aliases like "git-init"
On Sat, Nov 03 2018, Junio C Hamano wrote: > Ævar Arnfjörð Bjarmason writes: > >> Although I'm on the fence with the approach in 1/5. Should this be a >> giant getopt switch statement like that in a helper script? >> >> An alternative would be to write out a shell file similar to >> GIT-BUILD-OPTIONS and source that from this thing. I don't know, >> what do you all think? > > Not really. Why do we iterate over these in a shell loop, rather > than having make to figure them out, just like we do when we "loop > over the source files and turn them into object files" without using > a shell loop? What's so special about enumerating the installation > targets and iterating over the enumeration to perform an action on > each of them? I think that is the first question we should be > asking before patch 1/5, which already assumes that it has been > decided that it must be done with a shell loop. > > I think "first install 'git' itself, and then make these > other things derived from it" should let $(MAKE) install > things in parallel just like it can naturally do many things > in parallel, and the dependency rule to do so should not be > so bad, I suspect. > > This is a tangent, but I have long been wishing that somebody would > notice that output during install and (dist)clean without V=1 is so > different from the normal targets and do something about it, and > hoped that that somebody finally turned out to be you doing so in > this series X-<. I'm all for this, but don't have enough Make skills to make it happen. Can you or someone else post a WIP patch showing how to do this? What would the targets look like? Something that's a build target for the target file in the installation directory, so e.g. if you ran "all install" and had modified just one file (and no recursive rebuilds) you'd install just that one file? Early on in the "install" target we install many of these programs, and then some of these for-loops re-install on top of them. Actually now that I think of this this is one of the reasons for the 2>/dev/null probably, i.e. run "install" twice and you don't want to get errors. Anyway, regardless of how the for-loop looks like (shell or make-powered) I split this up because it was getting really hard to maintain the *inner* part of those loops. I.e. needing to specially quote everything, end lines with \ etc. But reading on... >> I'd like to say it's ready, but I've spotted some fallout: > > I still have not recovered from the trauma I suffered post 1.6.0 > era, so I would rather *not* engage in a long discussion like this > one (it is a long thread; reserve a solid hour to read it through if > you are interested), > > https://public-inbox.org/git/alpine.lfd.1.10.0808261435470.3...@nehalem.linux-foundation.org/ > > which would be needed to defend the choice, if we decide to omit > installing the git-foo on disk in a released version. Thanks. I'll read that later. > I personally have no objection to offer a knob that can e used to > force installation of symlinks without falling back to other > methods. I think it would be ideal to do so without special casing > symbolic links---rather, it would be ideal if it were a single knob > INSTALL_GIT_FOO_METHOD=(symlinks|hardlinks|copies) that says "I want > them to be installed as (symlinks|hardlinks|copies), no fallbacks". ... If you're happy to accept a patch that rips out this whole conditional fallback logic and just makes it an if/elsif/elsif for symlink/hardlink/copy this makes things a lot easier. >> Ævar Arnfjörð Bjarmason (5): >> Makefile: move long inline shell loops in "install" into helper >> Makefile: conform some of the code to our coding standards >> Makefile: stop hiding failures during "install" >> Makefile: add NO_INSTALL_SYMLINKS_FALLBACK switch >> Makefile: Add a NO_INSTALL_BUILTIN_EXECDIR_ALIASES flag >> >> Makefile | 65 +++-- >> install_programs | 124 +++ >> 2 files changed, 151 insertions(+), 38 deletions(-) >> create mode 100755 install_programs
Re: [RFC/PATCH 0/5] stop installing old libexec aliases like "git-init"
Ævar Arnfjörð Bjarmason writes: > Although I'm on the fence with the approach in 1/5. Should this be a > giant getopt switch statement like that in a helper script? > > An alternative would be to write out a shell file similar to > GIT-BUILD-OPTIONS and source that from this thing. I don't know, > what do you all think? Not really. Why do we iterate over these in a shell loop, rather than having make to figure them out, just like we do when we "loop over the source files and turn them into object files" without using a shell loop? What's so special about enumerating the installation targets and iterating over the enumeration to perform an action on each of them? I think that is the first question we should be asking before patch 1/5, which already assumes that it has been decided that it must be done with a shell loop. I think "first install 'git' itself, and then make these other things derived from it" should let $(MAKE) install things in parallel just like it can naturally do many things in parallel, and the dependency rule to do so should not be so bad, I suspect. This is a tangent, but I have long been wishing that somebody would notice that output during install and (dist)clean without V=1 is so different from the normal targets and do something about it, and hoped that that somebody finally turned out to be you doing so in this series X-<. > I'd like to say it's ready, but I've spotted some fallout: I still have not recovered from the trauma I suffered post 1.6.0 era, so I would rather *not* engage in a long discussion like this one (it is a long thread; reserve a solid hour to read it through if you are interested), https://public-inbox.org/git/alpine.lfd.1.10.0808261435470.3...@nehalem.linux-foundation.org/ which would be needed to defend the choice, if we decide to omit installing the git-foo on disk in a released version. I personally have no objection to offer a knob that can e used to force installation of symlinks without falling back to other methods. I think it would be ideal to do so without special casing symbolic links---rather, it would be ideal if it were a single knob INSTALL_GIT_FOO_METHOD=(symlinks|hardlinks|copies) that says "I want them to be installed as (symlinks|hardlinks|copies), no fallbacks". > Ævar Arnfjörð Bjarmason (5): > Makefile: move long inline shell loops in "install" into helper > Makefile: conform some of the code to our coding standards > Makefile: stop hiding failures during "install" > Makefile: add NO_INSTALL_SYMLINKS_FALLBACK switch > Makefile: Add a NO_INSTALL_BUILTIN_EXECDIR_ALIASES flag > > Makefile | 65 +++-- > install_programs | 124 +++ > 2 files changed, 151 insertions(+), 38 deletions(-) > create mode 100755 install_programs
[RFC/PATCH 0/5] stop installing old libexec aliases like "git-init"
I think up to patch 4 here should be near a state that's ready for
inclusion.
Although I'm on the fence with the approach in 1/5. Should this be a
giant getopt switch statement like that in a helper script? An
alternative would be to write out a shell file similar to
GIT-BUILD-OPTIONS and source that from this thing. I don't know, what
do you all think?
The idea with 4/5 was to make this symlink mode the default in
config.mak.uname and have a blacklist of systems like Windows that
couldn't deal with it.
Since my ad874608d8 ("Makefile: optionally symlink libexec/git-core
binaries to bin/git", 2018-03-13) I see that e.g. Debian and GitLab
have started shipping with the INSTALL_SYMLINKS flag, so making that
unconditional is the next logical step.
The 5th one is more radical. See
https://public-inbox.org/git/87woyfdkoi@evledraar.gmail.com/ from
back in March for context.
I'd like to say it's ready, but I've spotted some fallout:
* Help like "git ninit" suggesting "git init" doesn't work, this is
because load_command_list() in help.c doesn't look out our
in-memory idea of builtins, it reads the libexecdir, so if we don't
have the programs there it doesn't know about it.
* GIT_TEST_INSTALLED breaks entirely under this, as early as the
heuristic for "are we built?" being "do we have git-init in
libexecdir?". I tried a bit to make this work, but there's a lot of
dependencies there.
* We still (and this is also true of my ad874608d8) hardlink
everything in the build dir via a different part of the Makefile,
ideally we should do exactly the same thing there so also normal
tests and not just GIT_TEST_INSTALLED (if that worked) would test
in the same mode.
I gave making that work a bit of a try and gave up in the Makefile
jungle.
Ævar Arnfjörð Bjarmason (5):
Makefile: move long inline shell loops in "install" into helper
Makefile: conform some of the code to our coding standards
Makefile: stop hiding failures during "install"
Makefile: add NO_INSTALL_SYMLINKS_FALLBACK switch
Makefile: Add a NO_INSTALL_BUILTIN_EXECDIR_ALIASES flag
Makefile | 65 +++--
install_programs | 124 +++
2 files changed, 151 insertions(+), 38 deletions(-)
create mode 100755 install_programs
--
2.19.1.930.g4563a0d9d0
