import-tars: ignore the global PAX header

From: Johannes Schindelin <[email protected]>

Git's own git archive inserts that header, but it often gets into the way of import-tars.perl e.g. when a prefix was specified (for example via --prefix=my-project-1.0.0/, or when downloading a .tar.gz from GitHub releases): this prefix _should_ be stripped.

Let's just skip it.

---
Ignore the global PAX header in import-tars.perl

This problem came up in Pacman-related work, where PKGBUILD definitions would reference the tarballs downloaded from GitHub, and patches would be applied on top. To work on those patches efficiently (e.g. when an upgrade to a new version of the project no longer lets those patches apply), I need to be able to import those tarballs into playground worktrees and work on them. I like to use contrib/fast-import/import-tars.perl for that purpose, but it really needs to strip the prefix, otherwise it is too tedious to work with it.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-577%2Fdscho%2Fimport-tars-skip-pax-header-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-577/dscho/import-tars-skip-pax-header-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/577

 contrib/fast-import/import-tars.perl | 2 ++
 1 file changed, 2 insertions(+)

index e800d9f5c9c..d50ce26d5d9 100755
@@ -139,6 +139,8 @@
 			print FI "\n";
 		}
 
+		next if ($typeflag eq 'g'); # ignore global header
+
 		my $path;
 		if ($prefix) {
 			$path = "$prefix/$name";

base-commit: b4374e96c84ed9394fed363973eb540da308ed4f

gitgitgadget

Am 23.03.20 um 14:08 schrieb Johannes Schindelin via GitGitGadget:

git archive uses a global pax header to pass the ID of the archived commit as a comment, and for mtime values after 2242-03-16. Ignoring it in a simple importer seems reasonable for now, but I don't understand how this relates to prefixes. Is it because the header is treated as a regular file with the full path "pax_global_header" (independently from any prefix for actual files) and can thus be placed outside the expected destination directory?

Thanks,
René

Thanks for asking the question, as I was also curious if we are throwing away too much (perhaps "prefix is given as a global pax header, and ignoring all global pax headers is the most expedite way" was the reason the patch was written that way?). I agree with you that for the purpose of simple-minded importer, it probably is acceptable to take such a short-cut, but it would help future developers if we clearly documented that it is a short-cut that throws too much. That would welcome their effort to enhance the importer, if they find it more useful to keep some other information found in global headers, without breaking the intent of this change.

Having said all that, even before "git archive" existed, release tarballs by many projects had leading prefix so that a tarball extract would be made inside a versioned directory. To truly help users of the importer, doesn't the logic to allow the user to say "please strip one leading level of directory from all the tarballs I feed you, as I know they are versioned directories" belong to the command line option of the importer?

Hi René,

The tar importer in contrib/fast-import/import-tars.perl has a very convenient feature that I took for public knowledge: if _all_ paths stored in the imported .tar start with a common prefix, e.g. my-project-1.0.0/ as indicated in the commit message (or git-2.26.0/ in the tar at https://github.com/git/git/archive/v2.26.0.tar.gz), then this prefix is stripped.

This feature makes a ton of sense because it is relatively common to import two or more revisions of the same project into Git, and obviously we don't want all files to live in a tree whose name changes from revision to revision.

Now, the problem with that feature is that it breaks down if there is a pax_global_header "file" located outside of said prefix.

Exactly.

Thanks,
Dscho

Hi Junio,

I don't think that we're throwing away anything because the PAX header is intended to be a _header_, not a _file_, yet contrib/fast-import/import-tars.perl currently treats PAX headers that way.

I guess nobody needed an explicit way to strip path prefixes yet, since the implicit way works so well.

Ciao,
Dscho

What I meant (and wrote) was information contained within the header. You could store such metadata (e.g. this tree came from this commit from the upstream project) in the commit object while importing, for example.

As I wrote, I do not think we need to implement such feature right now. I am just saying that we should make sure we are not unintendely discouraging future developers from doing so by giving an impression that we are claiming "a pax header is intended to be a header and has no interesting information---never look at it". If we said "We discard the headers because it is the most expedient way and we currently have no use for them", that would make it clear that it is OK for them (the future ourselves developers) to extend the code not to lose the information, as long as they keep ignoring the prefix thing alone, if they want to follow the course set by this change.

Now you confused me. If implicit way already works well, why are we adding this patch to make it implicitly ignore?

Ahh, scratch what I said in my recent response. No, the above is not public knowledge, and that was missing from the log message that confused me.

With that spelled out, yes, it makes sense not to allow the phony "file" at the top level created out of the pax headers.

So, I think with a bit more explanation in the log, the patch would
be OK. We do not want _any_ header.

Thanks.

Hi brian,

I agree that we need a different solution for this case.

At the same time, I would like to point out that I am trying to address a _different_ problem than "we're not using the information contained within the PAX global header": I want to prevent that header from polluting the top-level tree.

For what it's worth, my patch does not prevent future patches from using the information contained within the global header :-)

Thanks,
Dscho

From: Johannes Schindelin <[email protected]>

The tar importer in contrib/fast-import/import-tars.perl has a very convenient feature: if _all_ paths stored in the imported .tar start with a common prefix, e.g. git-2.26.0/ in the tar at
https://github.com/git/git/archive/v2.26.0.tar.gz, then this prefix is stripped.

This feature makes a ton of sense because it is relatively common to import two or more revisions of the same project into Git, and obviously we don't want all files to live in a tree whose name changes from revision to revision.

Now, the problem with that feature is that it breaks down if there is a pax_global_header "file" located outside of said prefix, at the top of the tree. This is the case for .tar files generated by Git's very own git archive command: it inserts that header, and git archive allows specifying a common prefix (that the header does _not_ share with the other files contained in the archive) via --prefix=my-project-1.0.0/.

Let's just skip any global header when importing .tar files into Git.

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.

---
Ignore the global PAX header in import-tars.perl

This problem came up in Pacman-related work, where PKGBUILD definitions would reference the tarballs downloaded from GitHub, and patches would be applied on top. To work on those patches efficiently (e.g. when an upgrade to a new version of the project no longer lets those patches apply), I need to be able to import those tarballs into playground worktrees and work on them. I like to use contrib/fast-import/import-tars.perl for that purpose, but it really needs to strip the prefix, otherwise it is too tedious to work with it.

Changes since v1:

* Mentioned the implicit prefix-stripping feature of import-tars.perl in the commit message; Without that context, it is really hard to understand the motivation for this patch.
* Clarified in the commit message that this patch does not prevent any future patches that would use the information contained in the global header.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-577%2Fdscho%2Fimport-tars-skip-pax-header-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-577/dscho/import-tars-skip-pax-header-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/577

Range-diff vs v1:

1: 718bde8f4a7 ! 1: 842dabe6128 import-tars: ignore the global PAX header
@@ -2,12 +2,34 @@

import-tars: ignore the global PAX header

- Git's own git archive inserts that header, but it often gets into the
- way of import-tars.perl e.g. when a prefix was specified (for example
- via --prefix=my-project-1.0.0/, or when downloading a .tar.gz from
- GitHub releases): this prefix _should_ be stripped.
+ The tar importer in contrib/fast-import/import-tars.perl has a very
+ convenient feature: if _all_ paths stored in the imported .tar start
+ with a common prefix, e.g. git-2.26.0/ in the tar at
+ https://github.com/git/git/archive/v2.26.0.tar.gz, then this prefix is
+ stripped.

- Let's just skip it.
+ This feature makes a ton of sense because it is relatively common to
+ import two or more revisions of the same project into Git, and obviously
+ we don't want all files to live in a tree whose name changes from
+ revision to revision.
+
+ Now, the problem with that feature is that it breaks down if there is a
+ pax_global_header "file" located outside of said prefix, at the top of
+ the tree. This is the case for .tar files generated by Git's very own
+ git archive command: it inserts that header, and git archive allows
+ specifying a common prefix (that the header does _not_ share with the
+ other files contained in the archive) via --prefix=my-project-1.0.0/.
+
+ Let's just skip any global header when importing .tar files into Git.
+
+ 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.

 contrib/fast-import/import-tars.perl | 2 ++
 1 file changed, 2 insertions(+)

index e800d9f5c9c..d50ce26d5d9 100755
@@ -139,6 +139,8 @@
 			print FI "\n";
 		}
 
+		next if ($typeflag eq 'g'); # ignore global header
+
 		my $path;
 		if ($prefix) {
 			$path = "$prefix/$name";

base-commit: b4374e96c84ed9394fed363973eb540da308ed4f

gitgitgadget

"Johannes Schindelin via GitGitGadget" <[email protected]>

Hi Junio,

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

I was updating this week's "What's cooking" report, and described this one like so:

* js/import-tars-do-not-make-phony-files-from-pax-headers (2020-03-24) 1 commit
- import-tars: ignore the global PAX header

The import-tars importer (in contrib/fast-import/) used to create phony files at the top-level of the repository when the archive contains global PAX headers, which made its own logic to detect and omit the common leading directory ineffective, which has been corrected.

I guess we are ready to mark it as "Will merge".

Thanks.