Web lists-archives.com

Re: [PATCH] doc-diff: don't `cd_to_toplevel` before calling `usage`




On Sun, 3 Feb 2019 at 10:12, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote:
>
> On Sun, Feb 3, 2019 at 4:08 AM Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote:
> > I wonder if a more fruitful, longer-term fix which would save us from
> > having to worry about this in the future, would be to make
> > git-sh-setup.sh remember the original $0 before cd_to_toplevel() and
> > then employ the original value when usage() re-execs with the -h
> > option. That would also avoid the slightly ugly repeated
> > cd_to_top_level() and 'tmp' assignment in this patch.
>
> By "original $0", I meant a path which would be suitable for
> re-exec'ing (which wouldn't be the literal original $0). Sorry for the
> confusion.

I thought about this, and I probably should have said something about it
in the commit message. My uneducated guess was that "all" other users are
in $PATH and aren't being called like `./foobar`, but just `foobar`. Or,
they're internal helpers where the caller has already done the grunt
setup work, so their cd-ing is a no-op. I could be very wrong.

To be honest, I wasn't very tempted to risk breaking git-sh-setup
only(?) to help a development helper script like this. But let's see if
I can spend some more time on this...

The only way I'd know how to do something like this would be with
readlink or realpath. Judging by d2addc3b96 ("t7800: readlink may not be
available", 2016-05-31), we can't count on readlink. And I'd expect that
commit to have switched to realpath if THAT were available everywhere.
That commit instead goes for "ls | sed" and notes that "[t]his is no
universal readlink replacement but works in the controlled test
environment well enough."

Ok, so I am not too eager to try and tackle this with fallback
strategies and what-not. What would you say if I punted on this? I could
add something like this to the commit message:

  A more general fix would be to teach git-sh-setup to save away the
  absolute path for $0 and then use that, instead. I'm not aware of any
  portable way of doing that, see, e.g., d2addc3b96 ("t7800: readlink
  may not be available", 2016-05-31), so let's just fix this user
  instead.

What do you think? Thanks for your comments.

Martin