RE: [PATCH v2 2/6] Add tar extract install options override in installation processing.

["Randall S. Becker" <rsbecker@nexbridge.com>]

On January 20, 2018 7:31 AM, René Scharfe wrote:
> Am 19.01.2018 um 18:34 schrieb randall.s.becker@rogers.com:
> > From: "Randall S. Becker" <rsbecker@nexbridge.com>
> >
> > * Makefile: Add TAR_EXTRACT_OPTIONS to allow platform options to be
> >    specified if needed. The default is xof.
> >
> > Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>
> > ---
> >   Makefile | 6 +++++-
> >   1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/Makefile b/Makefile
> > index 1a9b23b67..040e9eacd 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -429,6 +429,9 @@ all::
> >   # running the test scripts (e.g., bash has better support for "set -x"
> >   # tracing).
> >   #
> > +# Define TAR_EXTRACT_OPTIONS if you want to change the default
> > +behaviour # from xvf to something else during installation.
> 
> "xof" instead of "xvf"?

When I look at the parent commit, it says xof, so I wanted to preserve existing behaviour by default. Our install process wants to see the actual set of files, so we wanted to use xvof but that hardly seemed of general interest. I was hoping an option to control it would be agreeable.

> > +#
> >   # When cross-compiling, define HOST_CPU as the canonical name of the
> CPU on
> >   # which the built Git will run (for instance "x86_64").
> >
> > @@ -452,6 +455,7 @@ LDFLAGS =
> >   ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS)
> >   ALL_LDFLAGS = $(LDFLAGS)
> >   STRIP ?= strip
> > +TAR_EXTRACT_OPTIONS = xof
> >
> >   # Create as necessary, replace existing, make ranlib unneeded.
> >   ARFLAGS = rcs
> > @@ -2569,7 +2573,7 @@ install: all
> >   ifndef NO_GETTEXT
> >   	$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(localedir_SQ)'
> >   	(cd po/build/locale && $(TAR) cf - .) | \
> > -	(cd '$(DESTDIR_SQ)$(localedir_SQ)' && umask 022 && $(TAR) xof -)
> > +	(cd '$(DESTDIR_SQ)$(localedir_SQ)' && umask 022 && $(TAR)
> > +$(TAR_EXTRACT_OPTIONS) -)
> 
> Hmm.  TAR_EXTRACT_OPTIONS always needs to have f (or -f, or --file) at the
> end to go together with the following dash, meaning to extract from stdin.
> And x (or -x, or --extract) is probably needed in all cases as well.  So wouldn't
> it make more sense to only put the o (or -o, or
> --no-same-owner) into TAR_EXTRACT_OPTIONS and enforce x and f?

This is a good suggestion, and I'd love to do that, if I could guarantee a modern tar, which I can't. The platform comes with a really old-school tar from some old (seemingly BSD4.3) epoch that only takes one option set. There is a more modern tar that can be optionally installed if the sysadmin decides to that takes a slightly more modern set, which could support your request, and my team also has a gnu port that is very modern. I can't control what customers are choosing to have installed, unfortunately. Your point is well made and I am completely on board with it, but it introduces a configuration requirement.

As with the broadening setbuf (patch 2/6) change, I would like to consider this for the future, having a slightly different more complex idea. I could introduce something like this:

1. HAS_ANCIENT_TAR=UnfortunatelyYes in config.mak.uname that disables this capability all together
2. HAS_ANCIENT_TAR=AreYouKiddingMe (joke) then set up TAR_EXTRACT_ADDITIONAL_OPTIONS above and beyond the default, so --file, --no-same-owner would always be in effect for that operation.

The micro-project would also, logically, need to apply to other tar occurrences throughout the code and potentially need a test case written for it (not entirely sure what that would test, yet).

Is that a reasonable approach?

> 
> >   endif
> >   ifndef NO_PERL
> >   	$(MAKE) -C perl prefix='$(prefix_SQ)' DESTDIR='$(DESTDIR_SQ)'
> > install
> >