| Message ID | 39a4e7bf8f3ebc5803393f357d1ee7dc9806252f.1540251936.git.steadmon@google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v3] archive: initialize archivers earlier | expand |
steadmon@google.com writes: > diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh > index 2a97b27b0a..cfd5ca492f 100755 > --- a/t/t5000-tar-tree.sh > +++ b/t/t5000-tar-tree.sh > @@ -39,6 +39,8 @@ test_lazy_prereq TAR_NEEDS_PAX_FALLBACK ' > > test_lazy_prereq GZIP 'gzip --version' > > +test_lazy_prereq ZIP 'zip --version' > + There are a handful of zip implementations; Info-ZIP found on many Linux distros does support 'zip --version', but we may want to make sure this test covers different implementations of zip sufficiently. Queuing this patch (or an update of it) on 'pu' and hoping those with zip from different origins to try it would not help very much, either, as zip implementations that do not react to "zip --version" would silently turn the prereq off without breaking anything. In any case, please refrain from adding any ZIP prerequiste to t5000 which is about tar; t5003-archive-zip may be a much better fit. It has an already working machinery that validates the generated zip archive under UNZIP prerequisite, so we may not even have to invent our own ZIP prereq if we did so. > @@ -206,6 +208,19 @@ test_expect_success 'git archive with --output, override inferred format' ' > test_cmp_bin b.tar d4.zip > ' > > +test_expect_success GZIP 'git archive with --output and --remote creates .tgz' ' > + git archive --output=d5.tgz --remote=. HEAD && > + gzip -d -c < d5.tgz > d5.tar && > + test_cmp_bin b.tar d5.tar > +' We try to write redirections without SP between redirection operator and target filename, i.e. "gzip -d -c <d5.tgz >d5.tar". Thanks.
On 2018.10.23 13:09, Junio C Hamano wrote: > steadmon@google.com writes: > > > diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh > > index 2a97b27b0a..cfd5ca492f 100755 > > --- a/t/t5000-tar-tree.sh > > +++ b/t/t5000-tar-tree.sh > > @@ -39,6 +39,8 @@ test_lazy_prereq TAR_NEEDS_PAX_FALLBACK ' > > > > test_lazy_prereq GZIP 'gzip --version' > > > > +test_lazy_prereq ZIP 'zip --version' > > + > > There are a handful of zip implementations; Info-ZIP found on many > Linux distros does support 'zip --version', but we may want to make > sure this test covers different implementations of zip sufficiently. > > Queuing this patch (or an update of it) on 'pu' and hoping those > with zip from different origins to try it would not help very much, > either, as zip implementations that do not react to "zip --version" > would silently turn the prereq off without breaking anything. > > In any case, please refrain from adding any ZIP prerequiste to t5000 > which is about tar; t5003-archive-zip may be a much better fit. It > has an already working machinery that validates the generated zip > archive under UNZIP prerequisite, so we may not even have to invent > our own ZIP prereq if we did so. Ack. This has been removed in v4. V4 also has a test case in t5003 based on Jeff's advice. > > @@ -206,6 +208,19 @@ test_expect_success 'git archive with --output, override inferred format' ' > > test_cmp_bin b.tar d4.zip > > ' > > > > +test_expect_success GZIP 'git archive with --output and --remote creates .tgz' ' > > + git archive --output=d5.tgz --remote=. HEAD && > > + gzip -d -c < d5.tgz > d5.tar && > > + test_cmp_bin b.tar d5.tar > > +' > > We try to write redirections without SP between redirection operator > and target filename, i.e. "gzip -d -c <d5.tgz >d5.tar". Fixed in v5.
diff --git a/archive.c b/archive.c index c1870105eb..ce0f8a0362 100644 --- a/archive.c +++ b/archive.c @@ -29,6 +29,12 @@ void register_archiver(struct archiver *ar) archivers[nr_archivers++] = ar; } +void init_archivers(void) +{ + init_tar_archiver(); + init_zip_archiver(); +} + static void format_subst(const struct commit *commit, const char *src, size_t len, struct strbuf *buf) @@ -531,9 +537,6 @@ int write_archive(int argc, const char **argv, const char *prefix, git_config_get_bool("uploadarchive.allowunreachable", &remote_allow_unreachable); git_config(git_default_config, NULL); - init_tar_archiver(); - init_zip_archiver(); - args.repo = repo; argc = parse_archive_args(argc, argv, &ar, &args, name_hint, remote); if (!startup_info->have_repository) { diff --git a/archive.h b/archive.h index d4f97a00f5..21ac010699 100644 --- a/archive.h +++ b/archive.h @@ -43,6 +43,7 @@ extern void register_archiver(struct archiver *); extern void init_tar_archiver(void); extern void init_zip_archiver(void); +extern void init_archivers(void); typedef int (*write_archive_entry_fn_t)(struct archiver_args *args, const struct object_id *oid, diff --git a/builtin/archive.c b/builtin/archive.c index e74f675390..d2455237ce 100644 --- a/builtin/archive.c +++ b/builtin/archive.c @@ -97,6 +97,8 @@ int cmd_archive(int argc, const char **argv, const char *prefix) argc = parse_options(argc, argv, prefix, local_opts, NULL, PARSE_OPT_KEEP_ALL); + init_archivers(); + if (output) create_output_file(output); diff --git a/builtin/upload-archive.c b/builtin/upload-archive.c index 25d9116356..018879737a 100644 --- a/builtin/upload-archive.c +++ b/builtin/upload-archive.c @@ -28,6 +28,8 @@ int cmd_upload_archive_writer(int argc, const char **argv, const char *prefix) if (!enter_repo(argv[1], 0)) die("'%s' does not appear to be a git repository", argv[1]); + init_archivers(); + /* put received options in sent_argv[] */ argv_array_push(&sent_argv, "git-upload-archive"); for (;;) { diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh index 2a97b27b0a..cfd5ca492f 100755 --- a/t/t5000-tar-tree.sh +++ b/t/t5000-tar-tree.sh @@ -39,6 +39,8 @@ test_lazy_prereq TAR_NEEDS_PAX_FALLBACK ' test_lazy_prereq GZIP 'gzip --version' +test_lazy_prereq ZIP 'zip --version' + get_pax_header() { file=$1 header=$2= @@ -206,6 +208,19 @@ test_expect_success 'git archive with --output, override inferred format' ' test_cmp_bin b.tar d4.zip ' +test_expect_success GZIP 'git archive with --output and --remote creates .tgz' ' + git archive --output=d5.tgz --remote=. HEAD && + gzip -d -c < d5.tgz > d5.tar && + test_cmp_bin b.tar d5.tar +' + +test_expect_success ZIP 'git archive with --output and --remote creates .zip' ' + git archive --output=d6.zip --remote=. HEAD && + zip -sf d6.zip | sed "/^[^ ]/d" | sed "s/^ //" | sort > zip_manifest && + "$TAR" tf b.tar | sort > tar_manifest && + test_cmp zip_manifest tar_manifest +' + test_expect_success 'git archive --list outside of a git repo' ' nongit git archive --list '
Initialize archivers as soon as possible when running git-archive. Various non-obvious behavior depends on having the archivers initialized, such as determining the desired archival format from the provided filename. Since 08716b3c11 ("archive: refactor file extension format-guessing", 2011-06-21), archive_format_from_filename() has used the registered archivers to match filenames (provided via --output) to archival formats. However, when git-archive is executed with --remote, format detection happens before the archivers have been registered. This causes archives from remotes to always be generated as TAR files, regardless of the actual filename (unless an explicit --format is provided). This patch fixes that behavior; archival format is determined properly from the output filename, even when --remote is used. Signed-off-by: Josh Steadmon <steadmon@google.com> Helped-by: Jeff King <peff@peff.net> --- Range-diff against v2: 1: bc6f20274d ! 1: 39a4e7bf8f archive: initialize archivers earlier @@ -78,26 +78,43 @@ --- a/builtin/upload-archive.c +++ b/builtin/upload-archive.c @@ - } + if (!enter_repo(argv[1], 0)) + die("'%s' does not appear to be a git repository", argv[1]); - /* parse all options sent by the client */ + init_archivers(); - return write_archive(sent_argv.argc, sent_argv.argv, prefix, - the_repository, NULL, 1); - } ++ + /* put received options in sent_argv[] */ + argv_array_push(&sent_argv, "git-upload-archive"); + for (;;) { diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh --- a/t/t5000-tar-tree.sh +++ b/t/t5000-tar-tree.sh @@ + + test_lazy_prereq GZIP 'gzip --version' + ++test_lazy_prereq ZIP 'zip --version' ++ + get_pax_header() { + file=$1 + header=$2= +@@ test_cmp_bin b.tar d4.zip ' -+test_expect_success GZIP 'git archive with --output and --remote uses expected format' ' ++test_expect_success GZIP 'git archive with --output and --remote creates .tgz' ' + git archive --output=d5.tgz --remote=. HEAD && + gzip -d -c < d5.tgz > d5.tar && + test_cmp_bin b.tar d5.tar +' ++ ++test_expect_success ZIP 'git archive with --output and --remote creates .zip' ' ++ git archive --output=d6.zip --remote=. HEAD && ++ zip -sf d6.zip | sed "/^[^ ]/d" | sed "s/^ //" | sort > zip_manifest && ++ "$TAR" tf b.tar | sort > tar_manifest && ++ test_cmp zip_manifest tar_manifest ++' + test_expect_success 'git archive --list outside of a git repo' ' nongit git archive --list archive.c | 9 ++++++--- archive.h | 1 + builtin/archive.c | 2 ++ builtin/upload-archive.c | 2 ++ t/t5000-tar-tree.sh | 15 +++++++++++++++ 5 files changed, 26 insertions(+), 3 deletions(-)