Re: [PATCH v2] import-tars: ignore the global PAX header

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Junio,

On Tue, 24 Mar 2020, Junio C Hamano wrote:

> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> >      +    Note: this global header might contain useful information. For example,
> >      +    in the output of `git archive`, it lists the original commit, which _is_
> >      +    useful information. A future improvement to the `import-tars.perl`
> >      +    script might be to include that information in the commit message, or do
> >      +    other things with the information (e.g. use `mtime` information
> >      +    contained in the global header as date of the commit). This patch does
> >      +    not prevent any future patch from making that happen, it only prevents
> >      +    the header from being treated as if it was a regular file.
> >
> >           Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> >
> >
> >  contrib/fast-import/import-tars.perl | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/contrib/fast-import/import-tars.perl b/contrib/fast-import/import-tars.perl
> > index e800d9f5c9c..d50ce26d5d9 100755
> > --- a/contrib/fast-import/import-tars.perl
> > +++ b/contrib/fast-import/import-tars.perl
> > @@ -139,6 +139,8 @@
> >  			print FI "\n";
> >  		}
> >
> > +		next if ($typeflag eq 'g'); # ignore global header
> > +
>
> Yeah, it is more like "don't create a phony file out of global
> header" which is exactly the point of this fix, rather than "ignore
> global header", and if the contents of the header is used for any
> other purpose (e.g. metadata that will be added to the log message
> or mode bits that is forced on all files), that can be handled
> before this line.  And the current code structure is already
> prepared for it: all that remains in the block after this point is
> to create a file at $path and store its contents.
>
> So, this makes sense.  I suspect that with an update to the comment
> in the direction, there probably is no need for the huge "Note" in
> the log message.

Too many of such in-code comments added by me became stale. I'd rather
keep this in the commit message where it explains the reasoning for the
current change.

Thanks,
Dscho

>
> Thanks.
>
> >  		my $path;
> >  		if ($prefix) {
> >  			$path = "$prefix/$name";
> >
> > base-commit: b4374e96c84ed9394fed363973eb540da308ed4f
>