Skip to contentSkip to conversation

git archive

PATCH In review kenneth johansson 7 participants 14 messages Jun 15, 2016

lore
Share & tools
lore

kenneth johansson

author
#0 lore raw

I was going to make a tar of the latest stable linux kernel. Done it before but now I got a strange problem.

Quoted 1 line

git archive --format=tar v2.6.27.2

fatal: Not a valid object name

this is the output from some other command on the same bare repository.

Quoted 1 line

git tag | grep 2.6.27.2

v2.6.27.2

Quoted 1 line

git cat-file -p v2.6.27.2

object 6bcd6d778419101dd96cbbdf03eeab8d779b1d66
type commit
tag v2.6.27.2
tagger Greg Kroah-Hartman <[email protected]> Sat Oct 18 10:58:00 2008 -0700

This is the v2.6.27.2 stable release
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.9 (GNU/Linux)

iEYEABECAAYFAkj6I6sACgkQMUfUDdst+ylcigCg0/e3kQZwAqLp6wPuPqdWl7nL X0wAnjuNPJG1OqZfhMiAGccLx0QGvMQz
=INvo
-----END PGP SIGNATURE-----

Quoted 1 line

git cat-file -p 6bcd6d778419101dd96cbbdf03eeab8d779b1d66

tree a717af81e5a2e8a7ee36f3b80aa077965f570197
parent 6505670551fa3deeb6e5d7cab6983514384c7220 author Greg Kroah-Hartman <[email protected]> 1224352642 -0700 committer Greg Kroah-Hartman <[email protected]> 1224352642 -0700

Linux 2.6.27.2

Quoted 1 line

git archive --format=tar a717af81e5a2e8a7ee36f3b80aa077965f570197

fatal: not a tree object

Quoted 1 line

git cat-file -t a717af81e5a2e8a7ee36f3b80aa077965f570197

tree

#1 lore raw
Quoted 5 lines · kenneth johansson

I was going to make a tar of the latest stable linux kernel. Done it before but now I got a strange problem.

Quoted 1 line

git archive --format=tar v2.6.27.2

fatal: Not a valid object name

I had the same thing happen to me, while trying to make an archive of Git. Were you perchance working in a bare repository, as I was? I spent some time looking at it and I think git archive sets up the environment in the wrong order, though of course I never finished a patch so I'm going from memory:

After looking at the code again, I think the issue is that git_config is called in builtin-archive.c:cmd_archive before setup_git_directory is called in archive.c:write_archive. The former ends up setting GIT_DIR to be '.git' even if you're in a bare repository. My coding skills weren't up to fixing it easily; moving setup_git_directory before git_config in builtin-archive caused last test of t5000 to fail: GIT_DIR=some/nonexistent/path git archive --list should still display the archive formats.

Another vote for from me for the discussion carried on here:
http://article.gmane.org/gmane.comp.version-control.git/98800

Deskin Miller

#2 lore raw
Quoted 13 lines · Deskin Miller
Quoted 5 lines · kenneth johansson

I was going to make a tar of the latest stable linux kernel. Done it before but now I got a strange problem.

Quoted 1 line

git archive --format=tar v2.6.27.2

fatal: Not a valid object name

I had the same thing happen to me, while trying to make an archive of Git.
Were you perchance working in a bare repository, as I was? I spent some time looking at it and I think git archive sets up the environment in the wrong order, though of course I never finished a patch so I'm going from memory:

Quoted 7 lines

After looking at the code again, I think the issue is that git_config is called in builtin-archive.c:cmd_archive before setup_git_directory is called in archive.c:write_archive. The former ends up setting GIT_DIR to be '.git' even if you're in a bare repository. My coding skills weren't up to fixing it easily; moving setup_git_directory before git_config in builtin-archive caused last test of t5000 to fail: GIT_DIR=some/nonexistent/path git archive --list should still display the archive formats.

The problem affects some other commands as well. I tried the following patch, ran "make test" and discovered "git mailinfo", "git verify-pack", "git hash-object" and "git unpack-file". A bandage patch is at the end of this mail. Solution is as Jeff suggested: call setup_git_directory_gently() early.

---<---

environment.c +5 -1
index 0693cd9..00ed640 100644
@@ -49,14 +49,18 @@ static char *work_tree;
 
 static const char *git_dir;
 static char *git_object_dir, *git_index_file, *git_refs_dir, *git_graft_file;
+int git_dir_discovered;
 
 static void setup_git_env(void)
 {
 	git_dir = getenv(GIT_DIR_ENVIRONMENT);
 	if (!git_dir)
 		git_dir = read_gitfile_gently(DEFAULT_GIT_DIR_ENVIRONMENT);
-	if (!git_dir)
+	if (!git_dir) {
+		if (!git_dir_discovered)
+			die("Internal error: .git must be relocated at cwd by setup_git_*");
 		git_dir = DEFAULT_GIT_DIR_ENVIRONMENT;
+	}
 	git_object_dir = getenv(DB_ENVIRONMENT);
 	if (!git_object_dir) {
 		git_object_dir = xmalloc(strlen(git_dir) + 9);
setup.c +3 -0
index 78a8041..d404c21 100644
@@ -368,6 +368,7 @@ const char *read_gitfile_gently(const char *path)
  * We cannot decide in this function whether we are in the work tree or
  * not, since the config can only be read _after_ this function was called.
  */
+extern int git_dir_discovered;
 const char *setup_git_directory_gently(int *nongit_ok)
 {
 	const char *work_tree_env = getenv(GIT_WORK_TREE_ENVIRONMENT);

@@ -472,6 +473,8 @@ const char *setup_git_directory_gently(int *nongit_ok)
 		}
 		chdir("..");
 	}
+	/* It is safe to call setup_git_env() now */
+	git_dir_discovered = 1;
 
 	inside_git_dir = 0;
 	if (!work_tree_env)

---<---

Bandage patch:

---<---

builtin-archive.c +0 -0
index 432ce2a..5ea0a12 100644
@@ -110,7 +110,9 @@ static const char *extract_remote_arg(int *ac,

const char **av)
int cmd_archive(int argc, const char **argv, const char *prefix) {
const char *remote = NULL;
+ int nongit;

+ prefix = setup_git_directory_gently(&nongit);
git_config(git_default_config, NULL);

remote = extract_remote_arg(&argc, argv);

builtin-mailinfo.c +2 -3
index e890f7a..5d401fb 100644
@@ -916,10 +916,9 @@ static const char mailinfo_usage[] =
 int cmd_mailinfo(int argc, const char **argv, const char *prefix)
 {
 	const char *def_charset;
+	int nongit;
 
-	/* NEEDSWORK: might want to do the optional .git/ directory
-	 * discovery
-	 */
+	prefix = setup_git_directory_gently(&nongit);
 	git_config(git_default_config, NULL);
 
 	def_charset = (git_commit_encoding ? git_commit_encoding : "utf-8");
builtin-verify-pack.c +0 -0
index 25a29f1..35a4eb2 100644
@@ -115,7 +115,9 @@ int cmd_verify_pack(int argc, const char **argv,

const char *prefix)
int verbose = 0;
int no_more_options = 0;
int nothing_done = 1;
+ int nongit;

+ prefix = setup_git_directory_gently(&nongit);
git_config(git_default_config, NULL); while (1 < argc) {
if (!no_more_options && argv[1][0] == '-') {

hash-object.c +9 -8
index 20937ff..a52b6be 100644
@@ -78,19 +78,20 @@ int main(int argc, const char **argv)
 	const char *prefix = NULL;
 	int prefix_length = -1;
 	const char *errstr = NULL;
+	int nongit;
 
 	type = blob_type;
 
-	git_config(git_default_config, NULL);
-
 	argc = parse_options(argc, argv, hash_object_options, hash_object_usage, 0);
 
-	if (write_object) {
-		prefix = setup_git_directory();
-		prefix_length = prefix ? strlen(prefix) : 0;
-		if (vpath && prefix)
-			vpath = prefix_filename(prefix, prefix_length, vpath);
-	}
+	prefix = setup_git_directory_gently(&nongit);
+	git_config(git_default_config, NULL);
+	prefix_length = prefix ? strlen(prefix) : 0;
+	if (vpath && prefix)
+		vpath = prefix_filename(prefix, prefix_length, vpath);
+
+	if (write_object && nongit)
+		die("Git repository required");
 
 	if (stdin_paths) {
 		if (hashstdin)
unpack-file.c +1 -1
index bcdc8bb..1a58d72 100644
@@ -27,10 +27,10 @@ int main(int argc, char **argv)
 
 	if (argc != 2)
 		usage("git-unpack-file <sha1>");
+	setup_git_directory();
 	if (get_sha1(argv[1], sha1))
 		die("Not a valid object name %s", argv[1]);
 
-	setup_git_directory();
 	git_config(git_default_config, NULL);
 
 	puts(create_temp_file(sha1));

---<---

Signature · 1 line
Duy
#3 lore raw
Quoted 22 lines · Nguyen Thai Ngoc Duy
Quoted 13 lines · Deskin Miller
Quoted 5 lines · kenneth johansson

I was going to make a tar of the latest stable linux kernel. Done it before but now I got a strange problem.

Quoted 1 line

git archive --format=tar v2.6.27.2

fatal: Not a valid object name

I had the same thing happen to me, while trying to make an archive of Git.
Were you perchance working in a bare repository, as I was? I spent some time looking at it and I think git archive sets up the environment in the wrong order, though of course I never finished a patch so I'm going from memory:

Quoted 7 lines

After looking at the code again, I think the issue is that git_config is called in builtin-archive.c:cmd_archive before setup_git_directory is called in archive.c:write_archive. The former ends up setting GIT_DIR to be '.git' even if you're in a bare repository. My coding skills weren't up to fixing it easily; moving setup_git_directory before git_config in builtin-archive caused last test of t5000 to fail: GIT_DIR=some/nonexistent/path git archive --list should still display the archive formats.

Quoted 5 lines

The problem affects some other commands as well. I tried the following patch, ran "make test" and discovered "git mailinfo", "git verify-pack", "git hash-object" and "git unpack-file". A bandage patch is at the end of this mail. Solution is as Jeff suggested: call setup_git_directory_gently() early.

Nice work. The patches look like they're on the right track (to me at least). I'm not sure though what you want to ultimately submit as a patch; I'd suggest both, squashed into one, since the check seems like something we'd reasonably want no matter what.

Few comments spread around below; also, can we see some testcases for regression? Or, does the first patch preclude the need for testcases?

Deskin Miller

environment.c static char *work_tree; · line 52

Quoted 10 lines

---<---

environment.c +1 -0
index 0693cd9..00ed640 100644
@@ -49,14 +49,18 @@ static char *work_tree;
 
 static const char *git_dir;
 static char *git_object_dir, *git_index_file, *git_refs_dir, *git_graft_file;
+int git_dir_discovered;

Should this be 'int git_dir_discovered = 0;' ?

Quoted 46 lines
environment.c +4 -1
 static void setup_git_env(void)
 {
 	git_dir = getenv(GIT_DIR_ENVIRONMENT);
 	if (!git_dir)
 		git_dir = read_gitfile_gently(DEFAULT_GIT_DIR_ENVIRONMENT);
-	if (!git_dir)
+	if (!git_dir) {
+		if (!git_dir_discovered)
+			die("Internal error: .git must be relocated at cwd by setup_git_*");
 		git_dir = DEFAULT_GIT_DIR_ENVIRONMENT;
+	}
 	git_object_dir = getenv(DB_ENVIRONMENT);
 	if (!git_object_dir) {
 		git_object_dir = xmalloc(strlen(git_dir) + 9);
setup.c +3 -1
index 78a8041..d404c21 100644
@@ -368,6 +368,7 @@ const char *read_gitfile_gently(const char *path)
  * We cannot decide in this function whether we are in the work tree or
  * not, since the config can only be read _after_ this function was called.
  */
+extern int git_dir_discovered;
 const char *setup_git_directory_gently(int *nongit_ok)
 {
 	const char *work_tree_env = getenv(GIT_WORK_TREE_ENVIRONMENT);

@@ -472,6 +473,8 @@ const char *setup_git_directory_gently(int *nongit_ok)
 		}
 		chdir("..");
 	}
+	/* It is safe to call setup_git_env() now */
+	git_dir_discovered = 1;
 
 	inside_git_dir = 0;
 	if (!work_tree_env)
---<---

Bandage patch:

---<---

builtin-archive.c +0 -0
index 432ce2a..5ea0a12 100644

builtin-archive.c static const char *extract_remote_arg(int *ac, · line 116

Quoted 7 lines
builtin-archive.c +2 -0
@@ -110,7 +110,9 @@ static const char *extract_remote_arg(int *ac,
 const char **av)
 int cmd_archive(int argc, const char **argv, const char *prefix)
 {
 	const char *remote = NULL;
+	int nongit;
 
+	prefix = setup_git_directory_gently(&nongit);

Here and elsewhere, the 'nongit' variable isn't used. setup_git_directory_gently can be passed a NULL pointer, why not do that?

Quoted 8 lines
builtin-archive.c +0 -0
 	git_config(git_default_config, NULL);
builtin-archive.c +0 -0
 	remote = extract_remote_arg(&argc, argv);
builtin-mailinfo.c +0 -0
index e890f7a..5d401fb 100644

builtin-mailinfo.c static const char mailinfo_usage[] = · line 921

Quoted 9 lines
builtin-mailinfo.c +2 -3
@@ -916,10 +916,9 @@ static const char mailinfo_usage[] =
 int cmd_mailinfo(int argc, const char **argv, const char *prefix)
 {
 	const char *def_charset;
+	int nongit;
 
-	/* NEEDSWORK: might want to do the optional .git/ directory
-	 * discovery
-	 */
+	prefix = setup_git_directory_gently(&nongit);

Same 'nongit' issue.

Quoted 8 lines
builtin-mailinfo.c +0 -0
 	git_config(git_default_config, NULL);
 
 	def_charset = (git_commit_encoding ? git_commit_encoding : "utf-8");
builtin-verify-pack.c +0 -0
index 25a29f1..35a4eb2 100644

builtin-verify-pack.c int cmd_verify_pack(int argc, const char **argv, · line 121

Quoted 7 lines
builtin-verify-pack.c +2 -0
@@ -115,7 +115,9 @@ int cmd_verify_pack(int argc, const char **argv,
 const char *prefix)
 	int verbose = 0;
 	int no_more_options = 0;
 	int nothing_done = 1;
+	int nongit;
 
+	prefix = setup_git_directory_gently(&nongit);

Same 'nongit' issue.

Quoted 23 lines
builtin-verify-pack.c +0 -0
 	git_config(git_default_config, NULL);
 	while (1 < argc) {
builtin-verify-pack.c +0 -0
 		if (!no_more_options && argv[1][0] == '-') {
hash-object.c +1 -6
index 20937ff..a52b6be 100644
@@ -78,19 +78,20 @@ int main(int argc, const char **argv)
 	const char *prefix = NULL;
 	int prefix_length = -1;
 	const char *errstr = NULL;
+	int nongit;
 
 	type = blob_type;
 
-	git_config(git_default_config, NULL);
-
 	argc = parse_options(argc, argv, hash_object_options, hash_object_usage, 0);
 
-	if (write_object) {
-		prefix = setup_git_directory();
-		prefix_length = prefix ? strlen(prefix) : 0;
-		if (vpath && prefix)

hash-object.c int main(int argc, const char **argv) · line 94

Quoted 10 lines
hash-object.c +8 -2
-			vpath = prefix_filename(prefix, prefix_length, vpath);
-	}
+	prefix = setup_git_directory_gently(&nongit);
+	git_config(git_default_config, NULL);
+	prefix_length = prefix ? strlen(prefix) : 0;
+	if (vpath && prefix)
+		vpath = prefix_filename(prefix, prefix_length, vpath);
+
+	if (write_object && nongit)
+		die("Git repository required");

I'd move this check up to just after setup_git_directory_gently.

Quoted 21 lines
hash-object.c +0 -0
 	if (stdin_paths) {
 		if (hashstdin)
unpack-file.c +1 -2
index bcdc8bb..1a58d72 100644
@@ -27,10 +27,10 @@ int main(int argc, char **argv)
 
 	if (argc != 2)
 		usage("git-unpack-file <sha1>");
+	setup_git_directory();
 	if (get_sha1(argv[1], sha1))
 		die("Not a valid object name %s", argv[1]);
 
-	setup_git_directory();
 	git_config(git_default_config, NULL);
 
 	puts(create_temp_file(sha1));
---<---
Signature · 1 line
Duy
#4 lore raw
Quoted 30 lines · Deskin Miller
Quoted 22 lines · Nguyen Thai Ngoc Duy
Quoted 13 lines · Deskin Miller
Quoted 5 lines · kenneth johansson

I was going to make a tar of the latest stable linux kernel. Done it before but now I got a strange problem.

Quoted 1 line

git archive --format=tar v2.6.27.2

fatal: Not a valid object name

I had the same thing happen to me, while trying to make an archive of Git.
Were you perchance working in a bare repository, as I was? I spent some time looking at it and I think git archive sets up the environment in the wrong order, though of course I never finished a patch so I'm going from memory:

Quoted 7 lines

After looking at the code again, I think the issue is that git_config is called in builtin-archive.c:cmd_archive before setup_git_directory is called in archive.c:write_archive. The former ends up setting GIT_DIR to be '.git' even if you're in a bare repository. My coding skills weren't up to fixing it easily; moving setup_git_directory before git_config in builtin-archive caused last test of t5000 to fail: GIT_DIR=some/nonexistent/path git archive --list should still display the archive formats.

Quoted 5 lines

The problem affects some other commands as well. I tried the following patch, ran "make test" and discovered "git mailinfo", "git verify-pack", "git hash-object" and "git unpack-file". A bandage patch is at the end of this mail. Solution is as Jeff suggested: call setup_git_directory_gently() early.

Quoted 4 lines

Nice work. The patches look like they're on the right track (to me at least).
I'm not sure though what you want to ultimately submit as a patch; I'd suggest both, squashed into one, since the check seems like something we'd reasonably want no matter what.

No, the patches are not in good shape and have not been tested well. I just wanted to point out the problem in other commands. Ideally the check in setup_git_env() should go along with discover_git_directory() as part of git setup rework.

Quoted 14 lines

Few comments spread around below; also, can we see some testcases for regression? Or, does the first patch preclude the need for testcases?

Deskin Miller

Quoted 7 lines

---<---

environment.c +0 -0
index 0693cd9..00ed640 100644
@@ -49,14 +49,18 @@ static char *work_tree;
Quoted 6 lines

environment.c static char *work_tree; · line 52

Quoted 3 lines
environment.c +1 -0
 static const char *git_dir;
 static char *git_object_dir, *git_index_file, *git_refs_dir, *git_graft_file;
+int git_dir_discovered;

Should this be 'int git_dir_discovered = 0;' ?

It is initialized by default IIRC.

Quoted 14 lines
Quoted 14 lines

Bandage patch:

---<---

builtin-archive.c +1 -0
index 432ce2a..5ea0a12 100644
@@ -110,7 +110,9 @@ static const char *extract_remote_arg(int *ac,
 const char **av)
 int cmd_archive(int argc, const char **argv, const char *prefix)
 {
      const char *remote = NULL;
+     int nongit;
Quoted 5 lines

builtin-archive.c static const char *extract_remote_arg(int *ac, · line 116

Quoted 1 line
builtin-archive.c +1 -0
+     prefix = setup_git_directory_gently(&nongit);

Here and elsewhere, the 'nongit' variable isn't used.
setup_git_directory_gently can be passed a NULL pointer, why not do that?

Passing NULL to setup_git_directory_gently() tells it to die() if no git repo can be found. If you pass a variable to it, it will set the variable to 1 if no repo is found, 0 otherwise.

Quoted 28 lines
Quoted 28 lines
builtin-archive.c +0 -0
      git_config(git_default_config, NULL);
      while (1 < argc) {

              if (!no_more_options && argv[1][0] == '-') {
hash-object.c +4 -8
index 20937ff..a52b6be 100644
@@ -78,19 +78,20 @@ int main(int argc, const char **argv)
      const char *prefix = NULL;
      int prefix_length = -1;
      const char *errstr = NULL;
+     int nongit;
 
      type = blob_type;
 
-     git_config(git_default_config, NULL);
-
      argc = parse_options(argc, argv, hash_object_options, hash_object_usage, 0);
 
-     if (write_object) {
-             prefix = setup_git_directory();
-             prefix_length = prefix ? strlen(prefix) : 0;
-             if (vpath && prefix)
-                     vpath = prefix_filename(prefix, prefix_length, vpath);
-     }
+     prefix = setup_git_directory_gently(&nongit);
+     git_config(git_default_config, NULL);
+     prefix_length = prefix ? strlen(prefix) : 0;
Quoted 8 lines

hash-object.c int main(int argc, const char **argv) · line 94

Quoted 5 lines
hash-object.c +5 -0
+     if (vpath && prefix)
+             vpath = prefix_filename(prefix, prefix_length, vpath);
+
+     if (write_object && nongit)
+             die("Git repository required");

I'd move this check up to just after setup_git_directory_gently.

Yeah, sounds reasonable.

Signature · 1 line
Duy
author
#5 lore raw
In reply to Deskin Miller
Quoted 4 lines · Deskin Miller
Quoted 3 lines · kenneth johansson

I was going to make a tar of the latest stable linux kernel. Done it before but now I got a strange problem.

Quoted 8 lines
Quoted 2 lines
Quoted 1 line

git archive --format=tar v2.6.27.2

fatal: Not a valid object name

I had the same thing happen to me, while trying to make an archive of Git. Were you perchance working in a bare repository, as I was? I spent some time looking at it and I think git archive sets up the environment in the wrong order, though of course I never finished a patch so I'm going from memory:

Yes it was a bare repository.

Quoted 9 lines

After looking at the code again, I think the issue is that git_config is called in builtin-archive.c:cmd_archive before setup_git_directory is called in archive.c:write_archive. The former ends up setting GIT_DIR to be '.git' even if you're in a bare repository. My coding skills weren't up to fixing it easily; moving setup_git_directory before git_config in builtin-archive caused last test of t5000 to fail:
GIT_DIR=some/nonexistent/path git archive --list should still display the archive formats.

if I do
GIT_DIR=. git archive --format=tar v2.6.27.2

it does work so it looks like you are on the right track.

#6 lore raw

RFC PATCH 1/1 archive: fix setup to work in bare repositories

cmd_archive was calling git_config -> setup_git_env prior to write_archive calling setup_git_directory. In a bare repository, the former will set git_dir to be '.git' since the latter has not determined that it's operating in a bare repository yet.

Things are complicated, however, by the fact that git archive --list should work from anywhere, not just in git repositories, so that argument needs to be checked for before setup_git_directory is called.
---

Quoted 27 lines · kenneth johansson
Quoted 4 lines · Deskin Miller
Quoted 3 lines · kenneth johansson

I was going to make a tar of the latest stable linux kernel. Done it before but now I got a strange problem.

Quoted 8 lines
Quoted 2 lines
Quoted 1 line

git archive --format=tar v2.6.27.2

fatal: Not a valid object name

I had the same thing happen to me, while trying to make an archive of Git. Were you perchance working in a bare repository, as I was? I spent some time looking at it and I think git archive sets up the environment in the wrong order, though of course I never finished a patch so I'm going from memory:

Yes it was a bare repository.

Quoted 9 lines

After looking at the code again, I think the issue is that git_config is called in builtin-archive.c:cmd_archive before setup_git_directory is called in archive.c:write_archive. The former ends up setting GIT_DIR to be '.git' even if you're in a bare repository. My coding skills weren't up to fixing it easily; moving setup_git_directory before git_config in builtin-archive caused last test of t5000 to fail:
GIT_DIR=some/nonexistent/path git archive --list should still display the archive formats.

Quoted 4 lines

if I do
GIT_DIR=. git archive --format=tar v2.6.27.2

it does work so it looks like you are on the right track.

Looks like this works, but I think it's really ugly; let me know if you have any suggestions for improvement.

 archive.c           |   13 +++++++++++++
 archive.h           |    1 +
 builtin-archive.c   |    4 +++-
 t/t5000-tar-tree.sh |    9 +++++++++
 4 files changed, 26 insertions(+), 1 deletions(-)
archive.c +13 -0
index e2280df..d8e4373 100644
@@ -325,6 +325,19 @@ static int parse_archive_args(int argc, const char **argv,
 	return argc;
 }
 
+int archive_parse_options_early(int argc, const char **argv)
+{
+	int i;
+	for (i = 1; i < argc; ++i) {
+		if (!strcmp(argv[i], "--list")) {
+			for (i = 0; i < ARRAY_SIZE(archivers); i++)
+				printf("%s\n", archivers[i].name);
+			exit(0);
+		}
+	}
+	return 0;
+}
+
 int write_archive(int argc, const char **argv, const char *prefix,
 		int setup_prefix)
 {
archive.h +1 -0
index 0b15b35..ff5b6cf 100644
@@ -24,6 +24,7 @@ extern int write_tar_archive(struct archiver_args *);
 extern int write_zip_archive(struct archiver_args *);
 
 extern int write_archive_entries(struct archiver_args *args, write_archive_entry_fn_t write_entry);
+extern int archive_parse_options_early(int argc, const char **argv);
 extern int write_archive(int argc, const char **argv, const char *prefix, int setup_prefix);
 
 #endif	/* ARCHIVE_H */
builtin-archive.c +3 -1
index 432ce2a..e518113 100644
@@ -111,6 +111,8 @@ int cmd_archive(int argc, const char **argv, const char *prefix)
 {
 	const char *remote = NULL;
 
+	archive_parse_options_early(argc, argv);
+	prefix = setup_git_directory();
 	git_config(git_default_config, NULL);
 
 	remote = extract_remote_arg(&argc, argv);

@@ -119,5 +121,5 @@ int cmd_archive(int argc, const char **argv, const char *prefix)
 
 	setvbuf(stderr, NULL, _IOLBF, BUFSIZ);
 
-	return write_archive(argc, argv, prefix, 1);
+	return write_archive(argc, argv, prefix, 0);
 }
t/t5000-tar-tree.sh +9 -0
index e395ff4..53fe25c 100755
@@ -192,4 +192,13 @@ test_expect_success \
     'git archive --list outside of a git repo' \
     'GIT_DIR=some/non-existing/directory git archive --list'
 
+test_expect_success \
+    'git archive inside bare repository' \
+    'git clone --bare "$(pwd)"/.git trash-bare &&
+    cd trash-bare &&
+    git archive --format=tar HEAD >/dev/null &&
+    cd .. &&
+    rm -rf trash-bare
+    '
+
 test_done
Signature · 1 line
1.6.0.2.554.g3041b
#7 lore raw

Re: [RFC PATCH] archive: fix setup to work in bare repositories

Quoted 8 lines · Deskin Miller

cmd_archive was calling git_config -> setup_git_env prior to write_archive calling setup_git_directory. In a bare repository, the former will set git_dir to be '.git' since the latter has not determined that it's operating in a bare repository yet.

Things are complicated, however, by the fact that git archive --list should work from anywhere, not just in git repositories, so that argument needs to be checked for before setup_git_directory is called.

Should you perhaps be able to call setup_git_directory_gently(), and then once you decide that you really do need the setup, call setup_git_directory()?

You would have to add a "did we already do run" flag to setup_git_directory_gently(), but I think it is already an error to call it twice, so you wouldn't be hurting anything by that.

Note also that by moving the setup, you are moving the chdir() that happens; you may need to prefix paths to any arguments to accomodate this (I don't think it should matter, since git-archive shouldn't look at any paths until after it would have done the setup_git_directory() before, but I didn't check carefully).

-Peff

#8 lore raw

Re: [RFC PATCH] archive: fix setup to work in bare repositories

In reply to Deskin Miller
Quoted 8 lines · Deskin Miller

cmd_archive was calling git_config -> setup_git_env prior to write_archive calling setup_git_directory. In a bare repository, the former will set git_dir to be '.git' since the latter has not determined that it's operating in a bare repository yet.

Things are complicated, however, by the fact that git archive --list should work from anywhere, not just in git repositories, so that argument needs to be checked for before setup_git_directory is called.

Just for some background, I think that this issue might be an unintended consequence of this commit:

b99b5b40cffb5269e4aa38b6b60391b55039e27d

The only reason that the git_config call has been added is to ensure the correct setting of core.autocrlf before convert_to_working_tree is called on any blobs.

I haven't looked in detail, but would moving this call later make for an cleaner change?

Signature · 2 lines
Charles Bailey
http://ccgi.hashpling.plus.com/blog/
#9 lore raw

PATCH 1/1 Fixed git archive for bare repos

This moves the call to git config to a place where it doesn't break the logic for using git archive in a bare repository but retains the fix to make git archive respect core.autocrlf.

Signed-off-by Charles Bailey <[email protected]>

---

OK, I've had a chance to have a quick look at this issue and I have an altenative patch.

This is an alternative fix to the current git archive in a bare repository issue.

It's been lightly tested and doesn't re-break the zip / autocrlf issue that the previous introduction of git_config was designed to fix.

 archive.c         |    2 ++
 builtin-archive.c |    2 --
 2 files changed, 2 insertions(+), 2 deletions(-)
archive.c +2 -0
index 849eed5..9ac455d 100644
@@ -336,5 +336,7 @@ int write_archive(int argc, const char **argv, const char *prefix,
 	parse_treeish_arg(argv, &args, prefix);
 	parse_pathspec_arg(argv + 1, &args);
 
+	git_config(git_default_config, NULL);
+
 	return ar->write_archive(&args);
 }
builtin-archive.c +0 -2
index 432ce2a..5ceec43 100644
@@ -111,8 +111,6 @@ int cmd_archive(int argc, const char **argv, const char *prefix)
 {
 	const char *remote = NULL;
 
-	git_config(git_default_config, NULL);
-
 	remote = extract_remote_arg(&argc, argv);
 	if (remote)
 		return run_remote_archiver(remote, argc, argv);
Signature · 1 line
1.6.0.2.534.g5ab59
Tested-by on v1 1/1
#10 lore raw

Re: [PATCH] Fixed git archive for bare repos

Quoted 2 lines · Charles Bailey

OK, I've had a chance to have a quick look at this issue and I have an altenative patch.

Thanks for making this much simpler than my version was; it passes the testcase I wrote, as well as the rest of the testsuite, so if you want to include my testcase (or an equivalent one) and re-send, consider it

Tested-by Deskin Miller <[email protected]>
#11 lore raw

Re: [PATCH] Fixed git archive for bare repos

In reply to Charles Bailey
Quoted 3 lines · Charles Bailey

This moves the call to git config to a place where it doesn't break the logic for using git archive in a bare repository but retains the fix to make git archive respect core.autocrlf.

If one combines your patch, Deskin's commit message and test and extends on the latter a bit then I think we have a winner. :)

Here are a few more tests which create a ZIP file in addition to a tar archive and compare them to their non-bare counterparts.

Care to resend?

Thanks,
René

 t/t5000-tar-tree.sh |   21 +++++++++++++++++++++
 1 files changed, 21 insertions(+), 0 deletions(-)
t/t5000-tar-tree.sh +21 -0
index e395ff4..bf5fa25 100755
@@ -58,6 +58,11 @@ test_expect_success \
      git commit-tree $treeid </dev/null)'
 
 test_expect_success \
+    'create bare clone' \
+    'git clone --bare . bare.git &&
+     cp .gitattributes bare.git/info/attributes'
+
+test_expect_success \
     'remove ignored file' \
     'rm a/ignored'
 

@@ -74,6 +79,14 @@ test_expect_success \
     'diff b.tar b2.tar'
 
 test_expect_success \
+    'git archive in a bare repo' \
+    '(cd bare.git && git archive HEAD) >b3.tar'
+
+test_expect_success \
+    'git archive vs. the same in a bare repo' \
+    'test_cmp b.tar b3.tar'
+
+test_expect_success \
     'validate file modification time' \
     'mkdir extract &&
      "$TAR" xf b.tar -C extract a/a &&

@@ -151,6 +164,14 @@ test_expect_success \
     'git archive --format=zip' \
     'git archive --format=zip HEAD >d.zip'
 
+test_expect_success \
+    'git archive --format=zip in a bare repo' \
+    '(cd bare.git && git archive --format=zip HEAD) >d1.zip'
+
+test_expect_success \
+    'git archive --format=zip vs. the same in a bare repo' \
+    'test_cmp d.zip d1.zip'
+
 $UNZIP -v >/dev/null 2>&1
 if [ $? -eq 127 ]; then
 	echo "Skipping ZIP tests, because unzip was not found"
#12 lore raw

PATCH v2 1/1 Fixed git archive for bare repos

From 8f0dce75427283e0333cce1f1e66f4eac9978ad4 Mon Sep 17 00:00:00 2001
From: Charles Bailey <[email protected]>

This moves the call to git_config to a place where it doesn't break the logic for using git archive in a bare repository but retains the fix to make git archive respect core.autocrlf.

Signed-off-by Charles Bailey <[email protected]>
Tested-by Deskin Miller <[email protected]>

---

Quoted 8 lines · René Scharfe
Quoted 3 lines · Charles Bailey

This moves the call to git config to a place where it doesn't break the logic for using git archive in a bare repository but retains the fix to make git archive respect core.autocrlf.

If one combines your patch, Deskin's commit message and test and extends on the latter a bit then I think we have a winner. :)

Quoted 7 lines

Here are a few more tests which create a ZIP file in addition to a tar archive and compare them to their non-bare counterparts.

Care to resend?

Thanks,
René

Here's a resend. I've kept Charles's change and commit message, but added René's tests and removed my test, since it was presupposed by the new tests.

Still needs a signoff from René.

 archive.c           |    2 ++
 builtin-archive.c   |    2 --
 t/t5000-tar-tree.sh |   21 +++++++++++++++++++++
 3 files changed, 23 insertions(+), 2 deletions(-)
archive.c +2 -0
index e2280df..45d242b 100644
@@ -338,5 +338,7 @@ int write_archive(int argc, const char **argv, const char *prefix,
 	parse_treeish_arg(argv, &args, prefix);
 	parse_pathspec_arg(argv + 1, &args);
 
+	git_config(git_default_config, NULL);
+
 	return ar->write_archive(&args);
 }
builtin-archive.c +0 -2
index 432ce2a..5ceec43 100644
@@ -111,8 +111,6 @@ int cmd_archive(int argc, const char **argv, const char *prefix)
 {
 	const char *remote = NULL;
 
-	git_config(git_default_config, NULL);
-
 	remote = extract_remote_arg(&argc, argv);
 	if (remote)
 		return run_remote_archiver(remote, argc, argv);
t/t5000-tar-tree.sh +21 -0
index e395ff4..0f27d73 100755
@@ -58,6 +58,11 @@ test_expect_success \
      git commit-tree $treeid </dev/null)'
 
 test_expect_success \
+    'create bare clone' \
+    'git clone --bare . bare.git &&
+     cp .gitattributes bare.git/info/attributes'
+
+test_expect_success \
     'remove ignored file' \
     'rm a/ignored'
 

@@ -74,6 +79,14 @@ test_expect_success \
     'diff b.tar b2.tar'
 
 test_expect_success \
+    'git archive in a bare repo' \
+    '(cd bare.git && git archive HEAD) >b3.tar'
+
+test_expect_success \
+    'git archive vs. the same in a bare repo' \
+    'test_cmp b.tar b3.tar'
+
+test_expect_success \
     'validate file modification time' \
     'mkdir extract &&
      "$TAR" xf b.tar -C extract a/a &&

@@ -151,6 +164,14 @@ test_expect_success \
     'git archive --format=zip' \
     'git archive --format=zip HEAD >d.zip'
 
+test_expect_success \
+    'git archive --format=zip in a bare repo' \
+    '(cd bare.git && git archive --format=zip HEAD) >d1.zip'
+
+test_expect_success \
+    'git archive --format=zip vs. the same in a bare repo' \
+    'test_cmp d.zip d1.zip'
+
 $UNZIP -v >/dev/null 2>&1
 if [ $? -eq 127 ]; then
 	echo "Skipping ZIP tests, because unzip was not found"
Signature · 1 line
1.6.0.3.515.g304f
#13 lore raw

Re: [PATCH v2] Fixed git archive for bare repos

Thanks all. Will apply to 'maint' for 1.6.0.4.