Git development
 help / color / mirror / Atom feed
* [PATCH v5 2/2] gitweb: append short hash ids to snapshot files
From: Mark Rada @ 2009-09-26 17:46 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jakub Narebski, git

Teach gitweb how to produce nicer snapshot names by only using the
short hash id. If clients make requests using a tree-ish that is not a
partial or full SHA-1 hash, then the short hash will also be appended
to whatever they asked for.

This also includes tests cases for t9502-gitweb-standalone-parse-output.

Signed-off-by: Mark Rada <marada@uwaterloo.ca>
---


	Changes since v4:
		- moved git_get_full_hash into this commit
		- changed test case format, suggested by Junio
		- explicity request at least a length of 7 for short hashes


 gitweb/gitweb.perl                        |   40 +++++++++++++++--
 t/t9502-gitweb-standalone-parse-output.sh |   67 +++++++++++++++++++++++++++++
 2 files changed, 103 insertions(+), 4 deletions(-)
 create mode 100644 t/t9502-gitweb-standalone-parse-output.sh

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 8d4a2ae..bc132a5 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1983,14 +1983,39 @@ sub quote_command {
 
 # get HEAD ref of given project as hash
 sub git_get_head_hash {
+	return git_get_full_hash(shift, 'HEAD');
+}
+
+sub git_get_full_hash {
 	my $project = shift;
+	my $hash = shift;
 	my $o_git_dir = $git_dir;
 	my $retval = undef;
 	$git_dir = "$projectroot/$project";
-	if (open my $fd, "-|", git_cmd(), "rev-parse", "--verify", "HEAD") {
-		my $head = <$fd>;
+	if (open my $fd, '-|', git_cmd(), 'rev-parse', '--verify', $hash) {
+		$hash = <$fd>;
 		close $fd;
-		if (defined $head && $head =~ /^([0-9a-fA-F]{40})$/) {
+		if (defined $hash && $hash =~ /^([0-9a-fA-F]{40})$/) {
+			$retval = $1;
+		}
+	}
+	if (defined $o_git_dir) {
+		$git_dir = $o_git_dir;
+	}
+	return $retval;
+}
+
+# try and get a shorter hash id
+sub git_get_short_hash {
+	my $project = shift;
+	my $hash = shift;
+	my $o_git_dir = $git_dir;
+	my $retval = undef;
+	$git_dir = "$projectroot/$project";
+	if (open my $fd, '-|', git_cmd(), 'rev-parse', '--short=7', $hash) {
+		$hash = <$fd>;
+		close $fd;
+		if (defined $hash && $hash =~ /^([0-9a-fA-F]{7,})$/) {
 			$retval = $1;
 		}
 	}
@@ -5203,6 +5228,13 @@ sub git_snapshot {
 		die_error(400, 'Object is not a tree-ish');
 	}
 
+
+	my $full_hash = git_get_full_hash($project, $hash);
+	if ($full_hash =~ /^$hash/) {
+		$hash = git_get_short_hash($project, $hash);
+	} else {
+		$hash .= '-' . git_get_short_hash($project, $hash);
+	}
 	my $name = $project;
 	$name =~ s,([^/])/*\.git$,$1,;
 	$name = basename($name);
@@ -5213,7 +5245,7 @@ sub git_snapshot {
 	$cmd = quote_command(
 		git_cmd(), 'archive',
 		"--format=$known_snapshot_formats{$format}{'format'}",
-		"--prefix=$name/", $hash);
+		"--prefix=$name/", $full_hash);
 	if (exists $known_snapshot_formats{$format}{'compressor'}) {
 		$cmd .= ' | ' . quote_command(@{$known_snapshot_formats{$format}{'compressor'}});
 	}
diff --git a/t/t9502-gitweb-standalone-parse-output.sh b/t/t9502-gitweb-standalone-parse-output.sh
new file mode 100644
index 0000000..5f2b1d5
--- /dev/null
+++ b/t/t9502-gitweb-standalone-parse-output.sh
@@ -0,0 +1,67 @@
+#!/bin/sh
+#
+# Copyright (c) 2009 Mark Rada
+#
+
+test_description='gitweb as standalone script (parsing script output).
+
+This test runs gitweb (git web interface) as a CGI script from the
+commandline, and checks that it produces the correct output, either
+in the HTTP header or the actual script output.'
+
+
+. ./gitweb-lib.sh
+
+# ----------------------------------------------------------------------
+# snapshot file name
+
+test_commit \
+	'SnapshotFileTests' \
+	'i can has snapshot?'
+
+test_expect_success 'snapshots: give full hash' '
+	ID=`git rev-parse --verify HEAD` &&
+	gitweb_run "p=.git;a=snapshot;h=$ID;sf=tgz" &&
+	ID=`git rev-parse --short HEAD` &&
+	grep ".git-$ID.tar.gz" gitweb.output
+'
+test_debug 'cat gitweb.output'
+
+test_expect_success 'snapshots: give short hash' '
+	ID=`git rev-parse --short HEAD` &&
+	gitweb_run "p=.git;a=snapshot;h=$ID;sf=tgz" &&
+	grep ".git-$ID.tar.gz" gitweb.output
+'
+test_debug 'cat gitweb.output'
+
+test_expect_success 'snapshots: give almost full hash' '
+	ID=`git rev-parse --short=30 HEAD` &&
+	gitweb_run "p=.git;a=snapshot;h=$ID;sf=tgz" &&
+	ID=`git rev-parse --short HEAD` &&
+	grep ".git-$ID.tar.gz" gitweb.output
+'
+test_debug 'cat gitweb.output'
+
+test_expect_success 'snapshots: give HEAD tree-ish' '
+	gitweb_run "p=.git;a=snapshot;h=HEAD;sf=tgz" &&
+	ID=`git rev-parse --short HEAD` &&
+	grep ".git-HEAD-$ID.tar.gz" gitweb.output
+'
+test_debug 'cat gitweb.output'
+
+test_expect_success 'snapshots: give branch name tree-ish' '
+	gitweb_run "p=.git;a=snapshot;h=master;sf=tgz" &&
+	ID=`git rev-parse --short master` &&
+	grep ".git-master-$ID.tar.gz" gitweb.output
+'
+test_debug 'cat gitweb.output'
+
+test_expect_success 'snapshots: give tag tree-ish' '
+	gitweb_run "p=.git;a=snapshot;h=SnapshotFileTests;sf=tgz" &&
+	ID=`git rev-parse --short SnapshotFileTests` &&
+	grep ".git-SnapshotFileTests-$ID.tar.gz" gitweb.output
+'
+test_debug 'cat gitweb.output'
+
+
+test_done
-- 
1.6.4.GIT

^ permalink raw reply related

* [PATCH v5 1/2] gitweb: check given hash before trying to create snapshot
From: Mark Rada @ 2009-09-26 17:46 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jakub Narebski, git

Makes things nicer in cases when you hand craft the snapshot URL but
make a typo in defining the hash variable (e.g. netx instead of next);
you will now get an error message instead of a broken tarball.

Tests for t9501 are included to demonstrate added functionality.

Signed-off-by: Mark Rada <marada@uwaterloo.ca>
---

	Changes since v4:
		- used Jakub's suggestion for checking hash validity
			- moved git_get_full_hash to the second commit
		- changed test cases format, suggested by Junio
		- added another test case for tagged objects due to the
		  bug Junio pointed out

	Sorry it's been a while since the v4, school started and I got
	buried under a whole lot of other things I had to take care of
	first. I've got time now, so further fix ups will happen in a
	more reasonable time frame (but hopefully aren't needed!).


 gitweb/gitweb.perl                       |    7 ++++-
 t/t9501-gitweb-standalone-http-status.sh |   39 ++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 24b2193..8d4a2ae 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -5196,8 +5196,11 @@ sub git_snapshot {
 		die_error(403, "Unsupported snapshot format");
 	}
 
-	if (!defined $hash) {
-		$hash = git_get_head_hash($project);
+	my $type = git_get_type("$hash^{}");
+	if (!$type) {
+		die_error(404, 'Object does not exist');
+	}  elsif ($type eq 'blob') {
+		die_error(400, 'Object is not a tree-ish');
 	}
 
 	my $name = $project;
diff --git a/t/t9501-gitweb-standalone-http-status.sh b/t/t9501-gitweb-standalone-http-status.sh
index d0ff21d..0688a57 100644
--- a/t/t9501-gitweb-standalone-http-status.sh
+++ b/t/t9501-gitweb-standalone-http-status.sh
@@ -75,4 +75,43 @@ test_expect_success \
 test_debug 'cat gitweb.output'
 
 
+# ----------------------------------------------------------------------
+# snapshot hash ids
+
+test_expect_success 'snapshots: good tree-ish id' '
+	gitweb_run "p=.git;a=snapshot;h=master;sf=tgz" &&
+	grep "Status: 200 OK" gitweb.output
+'
+test_debug 'cat gitweb.output'
+
+test_expect_success 'snapshots: bad tree-ish id' '
+	gitweb_run "p=.git;a=snapshot;h=frizzumFrazzum;sf=tgz" &&
+	grep "404 - Object does not exist" gitweb.output
+'
+test_debug 'cat gitweb.output'
+
+test_expect_success 'snapshots: bad tree-ish id (tagged object)' '
+	echo object > tag-object &&
+	git add tag-object &&
+	git commit -m "Object to be tagged" &&
+	git tag tagged-object `git hash-object tag-object` &&
+	gitweb_run "p=.git;a=snapshot;h=tagged-object;sf=tgz" &&
+	grep "400 - Object is not a tree-ish" gitweb.output
+'
+test_debug 'cat gitweb.output'
+
+test_expect_success 'snapshots: good object id' '
+	ID=`git rev-parse --verify HEAD` &&
+	gitweb_run "p=.git;a=snapshot;h=$ID;sf=tgz" &&
+	grep "Status: 200 OK" gitweb.output
+'
+test_debug 'cat gitweb.output'
+
+test_expect_success 'snapshots: bad object id' '
+	gitweb_run "p=.git;a=snapshot;h=abcdef01234;sf=tgz" &&
+	grep "404 - Object does not exist" gitweb.output
+'
+test_debug 'cat gitweb.output'
+
+
 test_done
-- 
1.6.4.GIT

^ permalink raw reply related

* [ANN] gitsharp version 0.1.0 released
From: Meinrad Recheis @ 2009-09-26 17:36 UTC (permalink / raw)
  To: gitsharp; +Cc: git

Hello git community,

Thanks to our active development community git# is quickly making
progress. The library GitSharp.dll is about 95% complete and we have
started creating a command line interface which shall be equivalent to
the original git. It already executes the commands "init" and "clone"
(not yet via SSH), some other commands are prepared as stubs. Please
note that not yet all tests are passing and that we are still working
on porting over jgit's changes from the last few months.

Download the binaries and find more information about this release at
http://www.eqqon.com/index.php/GitSharp/v0.1.0

Enjoy our first release and have a nice weekend,
-- henon

^ permalink raw reply

* Re: Distribution size
From: Johannes Schindelin @ 2009-09-26 16:00 UTC (permalink / raw)
  To: Thomas Singer; +Cc: git, Marc Weber
In-Reply-To: <4ABE3091.5040600@syntevo.com>

Hi,

On Sat, 26 Sep 2009, Thomas Singer wrote:

> > Funny.  Git for Windows is less than 12MB [*1*].
> 
> Well, the portable Git bundle compressed with 7zip is approx. 11MB, the 
> Git installer (maybe also using 7zip internally) is at approx. the same 
> size. Unpacked/installed on disk they are at 138MB/131MB. If you try to 
> compress it with zip, it will reduce to approx. 70MB which still is 
> quite large.

Of course, the Git installer does nothing like you claim.

> We are interested, too, in having a small(er) bundle, because we want to 
> distribute Git binaries with our Git GUI front-end, SmartGit, so the 
> user will (have the option to) get an all-inclusive-bundle.

If I see something in it for myself, I might want to help.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] Remove various dead assignments and dead increments found by the clang static analyzer
From: Johannes Schindelin @ 2009-09-26 15:58 UTC (permalink / raw)
  To: Giuseppe Scrivano; +Cc: git
In-Reply-To: <87ab0hepcn.fsf@master.homenet>

Hi,

On Sat, 26 Sep 2009, Giuseppe Scrivano wrote:

> I tried the clang static analyzer on the git source code, this patch
> fixes the found dead assignments/increments.
> 
> diff --git a/archive.c b/archive.c
> index 73b8e8a..88feed7 100644
> --- a/archive.c
> +++ b/archive.c
> @@ -357,7 +357,7 @@ int write_archive(int argc, const char **argv, const char *prefix,
>  	const struct archiver *ar = NULL;
>  	struct archiver_args args;
>  
> -	argc = parse_archive_args(argc, argv, &ar, &args);
> +	parse_archive_args(argc, argv, &ar, &args);
>  	if (setup_prefix && prefix == NULL)
>  		prefix = setup_git_directory();

I understand that clang complains when argc is not really used afterwards, 
but do we really want to do this?  I mean, if somebody decides it'd be a 
good idea to check the number of arguments after parsing the arguments, 
they might be bitten by the fact that it is now actively wrong.

Ciao,
Dscho

^ permalink raw reply

* Re: Distribution size
From: Thomas Singer @ 2009-09-26 15:17 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Marc Weber
In-Reply-To: <alpine.DEB.1.00.0909261320370.4985@pacific.mpi-cbg.de>

> Funny.  Git for Windows is less than 12MB [*1*].

Well, the portable Git bundle compressed with 7zip is approx. 11MB, the Git
installer (maybe also using 7zip internally) is at approx. the same size.
Unpacked/installed on disk they are at 138MB/131MB. If you try to compress
it with zip, it will reduce to approx. 70MB which still is quite large.

We are interested, too, in having a small(er) bundle, because we want to
distribute Git binaries with our Git GUI front-end, SmartGit, so the user
will (have the option to) get an all-inclusive-bundle.

--
Best regards,
Thomas Singer
=============
syntevo GmbH
http://www.syntevo.com
http://blog.syntevo.com


Johannes Schindelin wrote:
> Hi,
> 
> On Sat, 26 Sep 2009, Marc Weber wrote:
> 
>> Has anyone thought about reducing distribution size ?
>>
>> comparison:
>>
>> git-1.6.4.4 i%du -hs .
>> 115M    .
> 
> Funny.  Git for Windows is less than 12MB [*1*].  And it includes a bash 
> and Perl.  (Did you count Python on Windows?)
> 
> So maybe your analysis is severely borked.
> 
> Ciao,
> Dscho
> 
> [*1*]: See http://msysgit.googlecode.com/
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

^ permalink raw reply

* [PATCH] Remove various dead assignments and dead increments found by the clang static analyzer
From: Giuseppe Scrivano @ 2009-09-26 14:46 UTC (permalink / raw)
  To: git

Hello,

I tried the clang static analyzer on the git source code, this patch
fixes the found dead assignments/increments.


Regards,
Giuseppe Scrivano



>From 88fe9b63d159ad1fd0579564558fbf0f900bd8e3 Mon Sep 17 00:00:00 2001
From: Giuseppe Scrivano <gscrivano@gnu.org>
Date: Sat, 26 Sep 2009 16:34:56 +0200
Subject: [PATCH] Remove various dead assignments and dead increments found by the clang static analyzer

---
 archive.c                |    2 +-
 builtin-add.c            |    2 +-
 builtin-bisect--helper.c |    2 +-
 builtin-commit.c         |    2 +-
 builtin-fetch--tool.c    |    2 +-
 builtin-fetch-pack.c     |    2 +-
 builtin-grep.c           |    2 --
 builtin-help.c           |    2 +-
 builtin-ls-files.c       |    2 +-
 builtin-mktree.c         |    2 +-
 builtin-pack-objects.c   |    2 +-
 builtin-prune-packed.c   |    2 +-
 builtin-receive-pack.c   |    6 +++---
 builtin-rev-parse.c      |    2 +-
 builtin-send-pack.c      |    2 +-
 builtin-show-branch.c    |    4 ++--
 builtin-show-ref.c       |    2 +-
 builtin-write-tree.c     |    2 +-
 color.c                  |    2 +-
 compat/mkstemps.c        |    2 +-
 connect.c                |    1 -
 diff.c                   |    2 +-
 http-fetch.c             |    3 +--
 transport.c              |    4 ++--
 upload-pack.c            |    2 +-
 25 files changed, 27 insertions(+), 31 deletions(-)

diff --git a/archive.c b/archive.c
index 73b8e8a..88feed7 100644
--- a/archive.c
+++ b/archive.c
@@ -357,7 +357,7 @@ int write_archive(int argc, const char **argv, const char *prefix,
 	const struct archiver *ar = NULL;
 	struct archiver_args args;
 
-	argc = parse_archive_args(argc, argv, &ar, &args);
+	parse_archive_args(argc, argv, &ar, &args);
 	if (setup_prefix && prefix == NULL)
 		prefix = setup_git_directory();
 
diff --git a/builtin-add.c b/builtin-add.c
index cb6e590..2788315 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -193,7 +193,7 @@ static int edit_patch(int argc, const char **argv, const char *prefix)
 	init_revisions(&rev, prefix);
 	rev.diffopt.context = 7;
 
-	argc = setup_revisions(argc, argv, &rev, NULL);
+	setup_revisions(argc, argv, &rev, NULL);
 	rev.diffopt.output_format = DIFF_FORMAT_PATCH;
 	out = open(file, O_CREAT | O_WRONLY, 0644);
 	if (out < 0)
diff --git a/builtin-bisect--helper.c b/builtin-bisect--helper.c
index 5b22639..f9c7695 100644
--- a/builtin-bisect--helper.c
+++ b/builtin-bisect--helper.c
@@ -17,7 +17,7 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 		OPT_END()
 	};
 
-	argc = parse_options(argc, argv, prefix, options,
+	parse_options(argc, argv, prefix, options,
 			     git_bisect_helper_usage, 0);
 
 	if (!next_all)
diff --git a/builtin-commit.c b/builtin-commit.c
index 200ffda..56b595f 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -1035,7 +1035,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 			parents = reduce_heads(parents);
 	} else {
 		reflog_msg = "commit";
-		pptr = &commit_list_insert(lookup_commit(head_sha1), pptr)->next;
+		commit_list_insert(lookup_commit(head_sha1), pptr)->next;
 	}
 
 	/* Finally, get the commit message */
diff --git a/builtin-fetch--tool.c b/builtin-fetch--tool.c
index 3dbdf7a..c47469f 100644
--- a/builtin-fetch--tool.c
+++ b/builtin-fetch--tool.c
@@ -169,7 +169,7 @@ static int append_fetch_head(FILE *fp,
 			note_len += sprintf(note + note_len, "%s ", kind);
 		note_len += sprintf(note + note_len, "'%s' of ", what);
 	}
-	note_len += sprintf(note + note_len, "%.*s", remote_len, remote);
+	sprintf(note + note_len, "%.*s", remote_len, remote);
 	fprintf(fp, "%s\t%s\t%s\n",
 		sha1_to_hex(commit ? commit->object.sha1 : sha1),
 		not_for_merge ? "not-for-merge" : "",
diff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c
index 629735f..583f4e3 100644
--- a/builtin-fetch-pack.c
+++ b/builtin-fetch-pack.c
@@ -555,7 +555,7 @@ static int get_pack(int xd[2], char **pack_lockfile)
 	}
 	if (*hdr_arg)
 		*av++ = hdr_arg;
-	*av++ = NULL;
+	*av = NULL;
 
 	cmd.in = demux.out;
 	cmd.git_cmd = 1;
diff --git a/builtin-grep.c b/builtin-grep.c
index 761799d..d36b59e 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -400,7 +400,6 @@ static int external_grep(struct grep_opt *opt, const char **paths, int cached)
 				if (sizeof(randarg) <= len)
 					die("maximum length of args exceeded");
 				push_arg(argptr);
-				argptr += len;
 			}
 		}
 		else {
@@ -410,7 +409,6 @@ static int external_grep(struct grep_opt *opt, const char **paths, int cached)
 			if (sizeof(randarg) <= len)
 				die("maximum length of args exceeded");
 			push_arg(argptr);
-			argptr += len;
 		}
 	}
 	for (p = opt->pattern_list; p; p = p->next) {
diff --git a/builtin-help.c b/builtin-help.c
index e1eba77..76307fd 100644
--- a/builtin-help.c
+++ b/builtin-help.c
@@ -419,7 +419,7 @@ int cmd_help(int argc, const char **argv, const char *prefix)
 	setup_git_directory_gently(&nongit);
 	git_config(git_help_config, NULL);
 
-	argc = parse_options(argc, argv, prefix, builtin_help_options,
+	parse_options(argc, argv, prefix, builtin_help_options,
 			builtin_help_usage, 0);
 
 	if (show_all) {
diff --git a/builtin-ls-files.c b/builtin-ls-files.c
index f473220..80212f4 100644
--- a/builtin-ls-files.c
+++ b/builtin-ls-files.c
@@ -481,7 +481,7 @@ int cmd_ls_files(int argc, const char **argv, const char *prefix)
 		prefix_offset = strlen(prefix);
 	git_config(git_default_config, NULL);
 
-	argc = parse_options(argc, argv, prefix, builtin_ls_files_options,
+	parse_options(argc, argv, prefix, builtin_ls_files_options,
 			ls_files_usage, 0);
 	if (show_tag || show_valid_bit) {
 		tag_cached = "H ";
diff --git a/builtin-mktree.c b/builtin-mktree.c
index 098395f..36053cf 100644
--- a/builtin-mktree.c
+++ b/builtin-mktree.c
@@ -155,7 +155,7 @@ int cmd_mktree(int ac, const char **av, const char *prefix)
 		OPT_END()
 	};
 
-	ac = parse_options(ac, av, prefix, option, mktree_usage, 0);
+	parse_options(ac, av, prefix, option, mktree_usage, 0);
 
 	while (!got_eof) {
 		while (1) {
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 02f9246..bea7141 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -2307,7 +2307,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 	 */
 
 	if (!pack_to_stdout)
-		base_name = argv[i++];
+		base_name = argv[i];
 
 	if (pack_to_stdout != !base_name)
 		usage(pack_usage);
diff --git a/builtin-prune-packed.c b/builtin-prune-packed.c
index be99eb0..9a8fcfe 100644
--- a/builtin-prune-packed.c
+++ b/builtin-prune-packed.c
@@ -78,7 +78,7 @@ int cmd_prune_packed(int argc, const char **argv, const char *prefix)
 		OPT_END()
 	};
 
-	argc = parse_options(argc, argv, prefix, prune_packed_options,
+	parse_options(argc, argv, prefix, prune_packed_options,
 			     prune_packed_usage, 0);
 
 	prune_packed_objects(opts);
diff --git a/builtin-receive-pack.c b/builtin-receive-pack.c
index b771fe9..957e7f0 100644
--- a/builtin-receive-pack.c
+++ b/builtin-receive-pack.c
@@ -391,7 +391,7 @@ static void run_update_post_hook(struct command *cmd)
 		argc++;
 	}
 	argv[argc] = NULL;
-	status = run_command_v_opt(argv, RUN_COMMAND_NO_STDIN
+	run_command_v_opt(argv, RUN_COMMAND_NO_STDIN
 			| RUN_COMMAND_STDOUT_TO_STDERR);
 }
 
@@ -506,7 +506,7 @@ static const char *unpack(void)
 		if (receive_fsck_objects)
 			unpacker[i++] = "--strict";
 		unpacker[i++] = hdr_arg;
-		unpacker[i++] = NULL;
+		unpacker[i] = NULL;
 		code = run_command_v_opt(unpacker, RUN_GIT_CMD);
 		if (!code)
 			return NULL;
@@ -528,7 +528,7 @@ static const char *unpack(void)
 		keeper[i++] = "--fix-thin";
 		keeper[i++] = hdr_arg;
 		keeper[i++] = keep_arg;
-		keeper[i++] = NULL;
+		keeper[i] = NULL;
 		memset(&ip, 0, sizeof(ip));
 		ip.argv = keeper;
 		ip.out = -1;
diff --git a/builtin-rev-parse.c b/builtin-rev-parse.c
index 45bead6..4a66ba4 100644
--- a/builtin-rev-parse.c
+++ b/builtin-rev-parse.c
@@ -396,7 +396,7 @@ static int cmd_parseopt(int argc, const char **argv, const char *prefix)
 	/* put an OPT_END() */
 	ALLOC_GROW(opts, onb + 1, osz);
 	memset(opts + onb, 0, sizeof(opts[onb]));
-	argc = parse_options(argc, argv, prefix, opts, usage,
+	parse_options(argc, argv, prefix, opts, usage,
 			keep_dashdash ? PARSE_OPT_KEEP_DASHDASH : 0 |
 			stop_at_non_option ? PARSE_OPT_STOP_AT_NON_OPTION : 0);
 
diff --git a/builtin-send-pack.c b/builtin-send-pack.c
index 37e528e..5afd542 100644
--- a/builtin-send-pack.c
+++ b/builtin-send-pack.c
@@ -55,7 +55,7 @@ static int pack_objects(int fd, struct ref *refs, struct extra_have_objects *ext
 	if (args->use_ofs_delta)
 		argv[i++] = "--delta-base-offset";
 	if (args->quiet)
-		argv[i++] = "-q";
+		argv[i] = "-q";
 	memset(&po, 0, sizeof(po));
 	po.argv = argv;
 	po.in = -1;
diff --git a/builtin-show-branch.c b/builtin-show-branch.c
index 3510a86..e567eb5 100644
--- a/builtin-show-branch.c
+++ b/builtin-show-branch.c
@@ -191,9 +191,9 @@ static void name_commits(struct commit_list *list,
 					break;
 				}
 				if (nth == 1)
-					en += sprintf(en, "^");
+					sprintf(en, "^");
 				else
-					en += sprintf(en, "^%d", nth);
+					sprintf(en, "^%d", nth);
 				name_commit(p, xstrdup(newname), 0);
 				i++;
 				name_first_parent_chain(p);
diff --git a/builtin-show-ref.c b/builtin-show-ref.c
index c46550c..8a0ae6c 100644
--- a/builtin-show-ref.c
+++ b/builtin-show-ref.c
@@ -201,7 +201,7 @@ static const struct option show_ref_options[] = {
 
 int cmd_show_ref(int argc, const char **argv, const char *prefix)
 {
-	argc = parse_options(argc, argv, prefix, show_ref_options,
+	parse_options(argc, argv, prefix, show_ref_options,
 			     show_ref_usage, PARSE_OPT_NO_INTERNAL_HELP);
 
 	if (exclude_arg)
diff --git a/builtin-write-tree.c b/builtin-write-tree.c
index b223af4..848c3e4 100644
--- a/builtin-write-tree.c
+++ b/builtin-write-tree.c
@@ -34,7 +34,7 @@ int cmd_write_tree(int argc, const char **argv, const char *unused_prefix)
 	};
 
 	git_config(git_default_config, NULL);
-	argc = parse_options(argc, argv, unused_prefix, write_tree_options,
+	parse_options(argc, argv, unused_prefix, write_tree_options,
 			     write_tree_usage, 0);
 
 	ret = write_cache_as_tree(sha1, flags, prefix);
diff --git a/color.c b/color.c
index 62977f4..5b31588 100644
--- a/color.c
+++ b/color.c
@@ -110,7 +110,7 @@ void color_parse_mem(const char *value, int value_len, const char *var,
 			}
 		}
 		if (bg >= 0) {
-			if (sep++)
+			if (sep)
 				*dst++ = ';';
 			if (bg < 8) {
 				*dst++ = '4';
diff --git a/compat/mkstemps.c b/compat/mkstemps.c
index 14179c8..dbf916e 100644
--- a/compat/mkstemps.c
+++ b/compat/mkstemps.c
@@ -45,7 +45,7 @@ int gitmkstemps(char *pattern, int suffix_len)
 		template[2] = letters[v % num_letters]; v /= num_letters;
 		template[3] = letters[v % num_letters]; v /= num_letters;
 		template[4] = letters[v % num_letters]; v /= num_letters;
-		template[5] = letters[v % num_letters]; v /= num_letters;
+		template[5] = letters[v % num_letters];
 
 		fd = open(pattern, O_CREAT | O_EXCL | O_RDWR, 0600);
 		if (fd > 0)
diff --git a/connect.c b/connect.c
index 7945e38..da6c7c1 100644
--- a/connect.c
+++ b/connect.c
@@ -18,7 +18,6 @@ static int check_ref(const char *name, int len, unsigned int flags)
 
 	/* Skip the "refs/" part */
 	name += 5;
-	len -= 5;
 
 	/* REF_NORMAL means that we don't want the magic fake tag refs */
 	if ((flags & REF_NORMAL) && check_ref_format(name) < 0)
diff --git a/diff.c b/diff.c
index e1be189..e75f58e 100644
--- a/diff.c
+++ b/diff.c
@@ -901,7 +901,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
 
 	/* Find the longest filename and max number of changes */
 	reset = diff_get_color_opt(options, DIFF_RESET);
-	set   = diff_get_color_opt(options, DIFF_PLAIN);
+	diff_get_color_opt(options, DIFF_PLAIN);
 	add_c = diff_get_color_opt(options, DIFF_FILE_NEW);
 	del_c = diff_get_color_opt(options, DIFF_FILE_OLD);
 
diff --git a/http-fetch.c b/http-fetch.c
index e8f44ba..6879904 100644
--- a/http-fetch.c
+++ b/http-fetch.c
@@ -3,7 +3,6 @@
 
 int main(int argc, const char **argv)
 {
-	const char *prefix;
 	struct walker *walker;
 	int commits_on_stdin = 0;
 	int commits;
@@ -19,7 +18,7 @@ int main(int argc, const char **argv)
 	int get_verbosely = 0;
 	int get_recover = 0;
 
-	prefix = setup_git_directory();
+	setup_git_directory();
 
 	git_config(git_default_config, NULL);
 
diff --git a/transport.c b/transport.c
index 644a30a..8ec0df6 100644
--- a/transport.c
+++ b/transport.c
@@ -308,7 +308,7 @@ static int rsync_transport_push(struct transport *transport,
 	args[i++] = "info";
 	args[i++] = get_object_directory();
 	args[i++] = buf.buf;
-	args[i++] = NULL;
+	args[i] = NULL;
 
 	if (run_command(&rsync))
 		return error("Could not push objects to %s",
@@ -334,7 +334,7 @@ static int rsync_transport_push(struct transport *transport,
 		args[i++] = "--ignore-existing";
 	args[i++] = temp_dir.buf;
 	args[i++] = rsync_url(transport->url);
-	args[i++] = NULL;
+	args[i] = NULL;
 	if (run_command(&rsync))
 		result = error("Could not push to %s",
 				rsync_url(transport->url));
diff --git a/upload-pack.c b/upload-pack.c
index 38ddac2..fb3436c 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -241,7 +241,7 @@ static void create_pack_file(void)
 		argv[arg++] = "--delta-base-offset";
 	if (use_include_tag)
 		argv[arg++] = "--include-tag";
-	argv[arg++] = NULL;
+	argv[arg] = NULL;
 
 	memset(&pack_objects, 0, sizeof(pack_objects));
 	pack_objects.in = shallow_nr ? rev_list.out : -1;
-- 
1.6.3.3

^ permalink raw reply related

* Re: how optparse can go horribly wrong
From: Sverre Rabbelier @ 2009-09-26 13:44 UTC (permalink / raw)
  To: Nicolas Sebrecht; +Cc: Shawn O. Pearce, git
In-Reply-To: <20090926015127.GA12994@vidovic>

Heya,

On Sat, Sep 26, 2009 at 03:51, Nicolas Sebrecht <nicolas.s.dev@gmx.fr> wrote:
> Doing this only to -m flag would break consistency. That said, I don't
> have any opinion in disallowing the sticked form for _all_ short
> options.

Perhaps instead disallow it for short options that do not take a
one-symbol argument, that is -n4 makes a lot of sense to me, but -m"my
commit message here" not so much.


-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* [PATCH] git branch -D: give a better error message when lockfile creation fails
From: Miklos Vajna @ 2009-09-26 13:34 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20090926033143.GA9917@coredump.intra.peff.net>

Previously the old error message just told the user that it was not
possible to delete the ref from the packed-refs file. Give instructions
on how to resolve the problem.

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---

On Fri, Sep 25, 2009 at 11:31:43PM -0400, Jeff King <peff@peff.net> wrote:
> afterwards). So probably you would need to first refactor
> unable_to_lock_index_die() to handle just printing the error without
> dying.

Agreed, I forgot builtin-remote. What about this?

I removed the NORETURN macro as otherwise gcc would issue a warning, as
it does not realise that unable_to_lock_index_die() never returns.

 cache.h    |    3 ++-
 lockfile.c |   22 +++++++++++++++++-----
 refs.c     |    4 +++-
 3 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/cache.h b/cache.h
index 1a6412d..ffec86b 100644
--- a/cache.h
+++ b/cache.h
@@ -489,7 +489,8 @@ struct lock_file {
 };
 #define LOCK_DIE_ON_ERROR 1
 #define LOCK_NODEREF 2
-extern NORETURN void unable_to_lock_index_die(const char *path, int err);
+extern int unable_to_lock_index(const char *path, int err, int noreturn);
+extern void unable_to_lock_index_die(const char *path, int err);
 extern int hold_lock_file_for_update(struct lock_file *, const char *path, int);
 extern int hold_lock_file_for_append(struct lock_file *, const char *path, int);
 extern int commit_lock_file(struct lock_file *);
diff --git a/lockfile.c b/lockfile.c
index eb931ed..aa8c444 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -156,17 +156,29 @@ static int lock_file(struct lock_file *lk, const char *path, int flags)
 }
 
 
-NORETURN void unable_to_lock_index_die(const char *path, int err)
+int unable_to_lock_index(const char *path, int err, int noreturn)
 {
+	struct strbuf buf = STRBUF_INIT;
+	int ret;
+
 	if (err == EEXIST) {
-		die("Unable to create '%s.lock': %s.\n\n"
+		strbuf_addf(&buf, "Unable to create '%s.lock': %s.\n\n"
 		    "If no other git process is currently running, this probably means a\n"
 		    "git process crashed in this repository earlier. Make sure no other git\n"
 		    "process is running and remove the file manually to continue.",
 		    path, strerror(err));
-	} else {
-		die("Unable to create '%s.lock': %s", path, strerror(err));
-	}
+	} else
+		strbuf_addf(&buf, "Unable to create '%s.lock': %s", path, strerror(err));
+	if (noreturn)
+		die(buf.buf);
+	ret = error(buf.buf);
+	strbuf_release(&buf);
+	return ret;
+}
+
+void unable_to_lock_index_die(const char *path, int err)
+{
+	unable_to_lock_index(path, err, 1);
 }
 
 int hold_lock_file_for_update(struct lock_file *lk, const char *path, int flags)
diff --git a/refs.c b/refs.c
index 24865cf..4eb4fc7 100644
--- a/refs.c
+++ b/refs.c
@@ -972,8 +972,10 @@ static int repack_without_ref(const char *refname)
 	if (!found)
 		return 0;
 	fd = hold_lock_file_for_update(&packlock, git_path("packed-refs"), 0);
-	if (fd < 0)
+	if (fd < 0) {
+		unable_to_lock_index(git_path("packed-refs"), errno, 0);
 		return error("cannot delete '%s' from packed refs", refname);
+	}
 
 	for (list = packed_ref_list; list; list = list->next) {
 		char line[PATH_MAX + 100];
-- 
1.6.5.rc1.44.ga1675.dirty

^ permalink raw reply related

* Re: git clone sending unneeded objects
From: Jason Merrill @ 2009-09-26 13:33 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Matthieu Moy, git, Hin-Tak Leung
In-Reply-To: <4ABD9C2C.60800@redhat.com>

On 09/26/2009 12:44 AM, Jason Merrill wrote:
> git config remote.origin.fetch 'refs/remotes/*:refs/remotes/origin/*'
> git fetch

git count-objects -v before:

count: 44
size: 1768
in-pack: 1399509
packs: 1
size-pack: 600456
prune-packable: 0
garbage: 0

and after (transferred 278MB):

count: 44
size: 1768
in-pack: 1947339
packs: 2
size-pack: 1178408
prune-packable: 8
garbage: 0

and then after git gc --prune=now:

count: 0
size: 0
in-pack: 1399613
packs: 1
size-pack: 839900
prune-packable: 0
garbage: 0

So I only actually needed 104 more objects, but fetch wasn't clever 
enough to see that, and my new pack is much less efficient.

I've run into the same issue using alternates to set up multiple working 
directories for different branches; if the alternate directory isn't 
completely up-to-date, fetch wants to pull down lots of data again 
rather than use what I have and only fetch the last one or two commits.

Jason

^ permalink raw reply

* Re: [PATCH] make shallow repository deepening more network efficient
From: Johannes Schindelin @ 2009-09-26 13:26 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nicolas Pitre, git
In-Reply-To: <7vmy58r72b.fsf@alter.siamese.dyndns.org>

Hi,

On Sat, 5 Sep 2009, Junio C Hamano wrote:

> Nicolas Pitre <nico@cam.org> writes:
> 
> > First of all, I can't find any reason why thin pack generation is 
> > explicitly disabled when dealing with a shallow repository.  The 
> > possible delta base objects are collected from the edge commits which 
> > are always obtained through history walking with the same shallow refs 
> > as the client, Therefore the client is always going to have those base 
> > objects available. So let's remove that restriction.
> >
> > Then we can make shallow repository deepening much more efficient by 
> > using the remote's unshallowed commits as edge commits to get preferred 
> > base objects for thin pack generation.  On git.git, this makes the data 
> > transfer for the deepening of a shallow repository from depth 1 to depth 2
> > around 134 KB instead of 3.68 MB.
> >
> > Signed-off-by: Nicolas Pitre <nico@cam.org>
> 
> Dscho, this is your code from around ed09aef (support fetching into a
> shallow repository, 2006-10-30) and f53514b (allow deepening of a shallow
> repository, 2006-10-30).  The latter disables thin pack transfer but the
> log does not attempt to justify the change.
> 
> Have any comments?

Unfortunately, I do not have any.  I tried to remember, then I tried to 
find some documentation, but stopped when I found out that I developed 
the code on an iBook which died early Nov 2006.

So no, I do not remember why the change.

Sorry,
Dscho

^ permalink raw reply

* Re: Distribution size
From: Tay Ray Chuan @ 2009-09-26 11:53 UTC (permalink / raw)
  To: Marc Weber; +Cc: git
In-Reply-To: <1253962653-sup-1882@nixos>

Hi,

On Sat, Sep 26, 2009 at 7:11 PM, Marc Weber <marco-oweber@gmx.de> wrote:
> git-1.6.4.4 i%du -hs .
> 115M    .

perhaps it's due to usage of hard links?

-- 
Cheers,
Ray Chuan

^ permalink raw reply

* Re: Distribution size
From: Jakub Narebski @ 2009-09-26 11:27 UTC (permalink / raw)
  To: Marc Weber; +Cc: git
In-Reply-To: <1253962653-sup-1882@nixos>

Marc Weber <marco-oweber@gmx.de> writes:

> Has anyone thought about reducing distribution size ?
> 
> comparison:
> 
> git-1.6.4.4 i%du -hs .
> 115M    .

Errr... what?

  $ du -hs /usr/libexec/git-core/
  8.0M

  $ du -shc `rpm -ql git`
  ...
  27M
 
But

  $ du -sh git/.git
  49M

(there is some of my commits there, some old branches pinned, and some
commits with other git(web) forks).

And

  $ git archive HEAD --output=tmp.tar
  $ du -sh tmp.tar 
  11M     tmp.tar

> mercurial-1.0.2 i%du -hs .
> 5.4M    .
> 
> bazaar-1.10rc1 i%du -hs
> 21M     .
> 
> darcs-2.2.1 i%du -hs    /nix/store/8xwfavyv22pvm3s60wvzcbq561fjk5di-darcs-2.2.1 nixos   
> 32M	.
> 
> There was an attempt to reduce size by using a shared library in 2007:
> http://markmail.org/message/vzukaie4qvbghkq5
> 
> So is there a reason why git takes up so much more disk space other than
> that its using many small executables?

Are you sure you are comparing the same thing in all cases?
And not in some cases after "make", with all object files?

-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply

* Re: Distribution size
From: Johannes Schindelin @ 2009-09-26 11:22 UTC (permalink / raw)
  To: Marc Weber; +Cc: git
In-Reply-To: <1253962653-sup-1882@nixos>

Hi,

On Sat, 26 Sep 2009, Marc Weber wrote:

> Has anyone thought about reducing distribution size ?
> 
> comparison:
> 
> git-1.6.4.4 i%du -hs .
> 115M    .

Funny.  Git for Windows is less than 12MB [*1*].  And it includes a bash 
and Perl.  (Did you count Python on Windows?)

So maybe your analysis is severely borked.

Ciao,
Dscho

[*1*]: See http://msysgit.googlecode.com/

^ permalink raw reply

* Distribution size
From: Marc Weber @ 2009-09-26 11:11 UTC (permalink / raw)
  To: git

Hi,

Has anyone thought about reducing distribution size ?

comparison:

git-1.6.4.4 i%du -hs .
115M    .

mercurial-1.0.2 i%du -hs .
5.4M    .

bazaar-1.10rc1 i%du -hs
21M     .

darcs-2.2.1 i%du -hs    /nix/store/8xwfavyv22pvm3s60wvzcbq561fjk5di-darcs-2.2.1 nixos   
32M	.

There was an attempt to reduce size by using a shared library in 2007:
http://markmail.org/message/vzukaie4qvbghkq5

So is there a reason why git takes up so much more disk space other than
that its using many small executables?

Marc Weber

^ permalink raw reply

* Re: [PATCH 1/2] Make generated MSVC solution file open from Windows  Explorer
From: Sebastian Schuberth @ 2009-09-26  9:45 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git, mstormo
In-Reply-To: <20090926000500.GE14660@spearce.org>

On Sat, Sep 26, 2009 at 02:05, Shawn O. Pearce <spearce@spearce.org> wrote:

> Odd.  If I copy and paste from Thunderbird, its fine.  But if I
> save the body out as an attachment from mutt, it fails.
>
> I wonder if it has to do with the From header appearing in the top
> of the body; this header has to be escaped with a leading space in
> mbox format.  It looks like Thunderbird might be doing some magic to
> remove that leading space from the context lines, while mutt isn't.
>
> Next time, don't include the first From line?

Will try. So what about these two patches? Should I re-send them with
the first "From" stripped?

Or will *.patch files that are attached to emails, instead of sending
the patch inline, be accepted?

-- 
Sebastian Schuberth

^ permalink raw reply

* [PATCH] git branch -D: give a better error message when lockfile creation fails
From: Miklos Vajna @ 2009-09-25 23:53 UTC (permalink / raw)
  To: git

Previously the old error message just told the user that it was not
possible to delete the ref from the packed-refs file. Give instructions
on how to resolve the problem.

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
 refs.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/refs.c b/refs.c
index 24865cf..221d49c 100644
--- a/refs.c
+++ b/refs.c
@@ -972,8 +972,10 @@ static int repack_without_ref(const char *refname)
 	if (!found)
 		return 0;
 	fd = hold_lock_file_for_update(&packlock, git_path("packed-refs"), 0);
-	if (fd < 0)
+	if (fd < 0) {
+		unable_to_lock_index_die(git_path("packed-refs"), errno);
 		return error("cannot delete '%s' from packed refs", refname);
+	}
 
 	for (list = packed_ref_list; list; list = list->next) {
 		char line[PATH_MAX + 100];
-- 
1.6.5.rc1.44.ga1675.dirty

^ permalink raw reply related

* Re: [PATCH] make 'git clone' ask the remote only for objects it cares about
From: Andreas Schwab @ 2009-09-26  7:21 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: Junio C Hamano, Jason Merrill, Matthieu Moy, git, Hin-Tak Leung
In-Reply-To: <alpine.LFD.2.00.0909252314260.4997@xanadu.home>

Nicolas Pitre <nico@fluxnic.net> writes:

> The fact that the git repository on gcc.gnu.org has lots of stuff in 
> "remote" branches that don't get cloned by default is a separate 
> configuration/policy issue on that server which might need (or not) to 
> be looked into.  For instance at least, as a bare repository, it should 
> have all the git files in gcc.git/ directly instead of gcc.git/.git/.

The remote is just a git-svn tree.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply

* Re: git clone sending unneeded objects
From: Jason Merrill @ 2009-09-26  4:44 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Matthieu Moy, git, Hin-Tak Leung
In-Reply-To: <4ABD4F7B.4030701@redhat.com>

Incidentally, somewhat related to this issue, I've noticed that if I 
fetch a branch which I don't currently have in my repository, and I have 
most of the commits on that branch in my object store (or in an 
alternate repository) but not the most recent commit, git fetch isn't 
smart enough to only grab the commits I'm actually missing, it wants to 
fetch much more.

I would expect that since the clone pulled down everything in the 
gcc.git repository, I could then do

git config remote.origin.fetch 'refs/remotes/*:refs/remotes/origin/*'
git fetch

and have all the branches, not just the ones in refs/heads.  But when I 
do this git fetch wants to fetch some 500k redundant objects.

Jason

^ permalink raw reply

* [PATCH] make 'git clone' ask the remote only for objects it cares about
From: Nicolas Pitre @ 2009-09-26  3:54 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jason Merrill, Matthieu Moy, git, Hin-Tak Leung
In-Reply-To: <alpine.LFD.2.00.0909252045290.4997@xanadu.home>

Current behavior of 'git clone' when not using --mirror is to fetch 
everything from the peer, and then filter out unwanted refs just before 
writing them out to the cloned repository.  This may become highly 
inefficient if the peer has an unusual ref namespace, or if it simply 
has "remotes" refs of its own, and those locally unwanted refs are 
connecting to a large set of objects which becomes unreferenced as soon 
as they are fetched.

Let's filter out those unwanted refs from the peer _before_ asking it 
what refs we want to fetch instead, which is the most logical thing to 
do anyway.

Signed-off-by: Nicolas Pitre <nico@fluxnic.net>
---

On Fri, 25 Sep 2009, Nicolas Pitre wrote:

> On Fri, 25 Sep 2009, Jason Merrill wrote:
> 
> > On 09/25/2009 04:47 PM, Nicolas Pitre wrote:
> > > Do you have access to the remote machine?  Is it possible to have a
> > > tarball of the gcc.git directory from there?
> > 
> > http://gcc.gnu.org/gcc-git.tar.gz
> > 
> > I'll leave it there for a few days.
> 
> Thanks, I got it now.  And I was able to reproduce the issue locally.
> 
> Cloning the original repository does transfer objects which become 
> unreferenced in the clone.  But cloning that cloned repository (before 
> pruning the unreferenced objects) does not transfer those objects again.  
> 
> Just need to find out why.

And the "why" is described above.  The problem was actually on the 
client side and was affecting clones of any repository containing 
anything outside refs/heads and refs/tags.

The fact that the git repository on gcc.gnu.org has lots of stuff in 
"remote" branches that don't get cloned by default is a separate 
configuration/policy issue on that server which might need (or not) to 
be looked into.  For instance at least, as a bare repository, it should 
have all the git files in gcc.git/ directly instead of gcc.git/.git/.

diff --git a/builtin-clone.c b/builtin-clone.c
index bab2d84..edf7c7f 100644
--- a/builtin-clone.c
+++ b/builtin-clone.c
@@ -329,24 +329,28 @@ static void remove_junk_on_signal(int signo)
 	raise(signo);
 }
 
-static struct ref *write_remote_refs(const struct ref *refs,
-		struct refspec *refspec, const char *reflog)
+static struct ref *wanted_peer_refs(const struct ref *refs,
+		struct refspec *refspec)
 {
 	struct ref *local_refs = NULL;
 	struct ref **tail = &local_refs;
-	struct ref *r;
 
 	get_fetch_map(refs, refspec, &tail, 0);
 	if (!option_mirror)
 		get_fetch_map(refs, tag_refspec, &tail, 0);
 
+	return local_refs;
+}
+
+static void write_remote_refs(const struct ref *local_refs, const char *reflog)
+{
+	const struct ref *r;
+
 	for (r = local_refs; r; r = r->next)
 		add_extra_ref(r->peer_ref->name, r->old_sha1, 0);
 
 	pack_refs(PACK_REFS_ALL);
 	clear_extra_refs();
-
-	return local_refs;
 }
 
 int cmd_clone(int argc, const char **argv, const char *prefix)
@@ -495,9 +499,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 
 	strbuf_reset(&value);
 
-	if (path && !is_bundle)
+	if (path && !is_bundle) {
 		refs = clone_local(path, git_dir);
-	else {
+		mapped_refs = wanted_peer_refs(refs, refspec);
+	} else {
 		struct remote *remote = remote_get(argv[0]);
 		transport = transport_get(remote, remote->url[0]);
 
@@ -520,14 +525,16 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 					     option_upload_pack);
 
 		refs = transport_get_remote_refs(transport);
-		if (refs)
-			transport_fetch_refs(transport, refs);
+		if (refs) {
+			mapped_refs = wanted_peer_refs(refs, refspec);
+			transport_fetch_refs(transport, mapped_refs);
+		}
 	}
 
 	if (refs) {
 		clear_extra_refs();
 
-		mapped_refs = write_remote_refs(refs, refspec, reflog_msg.buf);
+		write_remote_refs(mapped_refs, reflog_msg.buf);
 
 		remote_head = find_ref_by_name(refs, "HEAD");
 		remote_head_points_at =

^ permalink raw reply related

* Re: [PATCH] git branch -D: give a better error message when lockfile creation fails
From: Jeff King @ 2009-09-26  3:31 UTC (permalink / raw)
  To: Miklos Vajna; +Cc: git
In-Reply-To: <1253923602-17818-1-git-send-email-vmiklos@frugalware.org>

On Sat, Sep 26, 2009 at 02:06:42AM +0200, Miklos Vajna wrote:

> diff --git a/refs.c b/refs.c
> index 24865cf..221d49c 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -972,8 +972,10 @@ static int repack_without_ref(const char *refname)
>  	if (!found)
>  		return 0;
>  	fd = hold_lock_file_for_update(&packlock, git_path("packed-refs"), 0);
> -	if (fd < 0)
> +	if (fd < 0) {
> +		unable_to_lock_index_die(git_path("packed-refs"), errno);
>  		return error("cannot delete '%s' from packed refs", refname);
> +	}
>  
>  	for (list = packed_ref_list; list; list = list->next) {
>  		char line[PATH_MAX + 100];

I have several concerns about this patch:

  1. We used to return error(), but now we die. Are there any callers
     which care about the difference?

  2. If we did want to die, then the "return error()" just below is
     unreachable, and should be removed.

  3. If we did want to die, should we not just pass LOCK_DIE_ON_ERROR to
     hold_lock_file_for_update?

I suspect (2) and (3) are irrelevant because the answer to (1) is that
yes, some callers do care (e.g., it looks like builtin-remote calls
delete_ref, and notes an error but continues to do useful work
afterwards). So probably you would need to first refactor
unable_to_lock_index_die() to handle just printing the error without
dying.

-Peff

^ permalink raw reply

* Re: how optparse can go horribly wrong
From: Nicolas Sebrecht @ 2009-09-26  1:51 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git, Nicolas Sebrecht
In-Reply-To: <20090925233226.GC14660@spearce.org>

The 25/09/09, Shawn O. Pearce wrote:
> *sigh*.  Someone just ran into this today:
> 
>   $ git commit -a -ammend
>   [work ce38944] mend
>    1 files changed, 2 insertions(+), 0 deletions(-)
> 
> Omit one - and include an extra 'm', and instead of --amend you
> have -a -m mend.  Which isn't exactly what you wanted.
> 
> We do catch -amend with an error though:
> 
>   $ git commit -amend
>   error: did you mean `--amend` (with two dashes ?)

OTOH, this is a bit odd because a commit with the message "end" makes
perfect sense for a "fast and crappy commit local workflow".

And we allow -ammend (with two 'm')

  $ git commit -ammend
  [next 101f014] mend
   1 files changed, 1 insertions(+), 0 deletions(-)
  $

> I wonder, should the -m flag on commit not allow cuddling its
> value against the switch when its combined in short form with
> other switches?

Doing this only to -m flag would break consistency. That said, I don't
have any opinion in disallowing the sticked form for _all_ short
options.

-- 
Nicolas Sebrecht

^ permalink raw reply

* Re: git clone sending unneeded objects
From: Nicolas Pitre @ 2009-09-26  0:49 UTC (permalink / raw)
  To: Jason Merrill; +Cc: Matthieu Moy, git, Hin-Tak Leung
In-Reply-To: <4ABD4F7B.4030701@redhat.com>

On Fri, 25 Sep 2009, Jason Merrill wrote:

> On 09/25/2009 04:47 PM, Nicolas Pitre wrote:
> > Do you have access to the remote machine?  Is it possible to have a
> > tarball of the gcc.git directory from there?
> 
> http://gcc.gnu.org/gcc-git.tar.gz
> 
> I'll leave it there for a few days.

Thanks, I got it now.  And I was able to reproduce the issue locally.

Cloning the original repository does transfer objects which become 
unreferenced in the clone.  But cloning that cloned repository (before 
pruning the unreferenced objects) does not transfer those objects again.  

Just need to find out why.


Nicolas

^ permalink raw reply

* Re: git clone sending unneeded objects
From: Hin-Tak Leung @ 2009-09-26  0:43 UTC (permalink / raw)
  To: Jason Merrill; +Cc: Nicolas Pitre, Matthieu Moy, git
In-Reply-To: <4ABD25FE.2040902@redhat.com>

On Fri, Sep 25, 2009 at 9:20 PM, Jason Merrill <jason@redhat.com> wrote:
> On 09/25/2009 03:53 PM, Nicolas Pitre wrote:
>>
>> I did reproduce the issue with git:// back when this discussion started.
>> I also asked for more information about the remote which didn't come
>> forth.
>
> Looking back, I only see you asking about the git version on the server,
> which is 1.6.4.

Hmm, I was under the impression from the previous thread that the
server is a bit older and/or have more backward compatible settings to
cater for older git clients?

>
> So again:
>
> git clone git://gcc.gnu.org/git/gcc.git
>  (1399509 objects, ~600MB .git dir)
> git gc --prune=now (988906 objects, ~450MB .git dir)
>
> ...then
>
> git clone git://gcc.gnu.org/git/gcc.git --reference $firstclone
>  (573401 objects, ~550MB .git dir)
> git fsck (clean)
> git gc --prune=now (5 objects, ~7MB .git dir)
>
> What's going on here?

FWIW, I still have my clone (git://) and do my periodic 'git fetch'
and 'git gc prune=now' (learned my lessons!) and it is currently .git
dir is about 350MB. (from previous discussion the optimal at the time
was about 300MB, so it has grown a bit in the last couple of months).

And thanks everybody for all the discussion and advice. git is a great
tool. (and I have essentially stopped using svn, prefering git-svn!).

^ permalink raw reply

* [PATCH] git branch -D: give a better error message when lockfile creation fails
From: Miklos Vajna @ 2009-09-26  0:06 UTC (permalink / raw)
  To: git

Previously the old error message just told the user that it was not
possible to delete the ref from the packed-refs file. Give instructions
on how to resolve the problem.

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
 refs.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/refs.c b/refs.c
index 24865cf..221d49c 100644
--- a/refs.c
+++ b/refs.c
@@ -972,8 +972,10 @@ static int repack_without_ref(const char *refname)
 	if (!found)
 		return 0;
 	fd = hold_lock_file_for_update(&packlock, git_path("packed-refs"), 0);
-	if (fd < 0)
+	if (fd < 0) {
+		unable_to_lock_index_die(git_path("packed-refs"), errno);
 		return error("cannot delete '%s' from packed refs", refname);
+	}
 
 	for (list = packed_ref_list; list; list = list->next) {
 		char line[PATH_MAX + 100];
-- 
1.6.5.rc1.44.ga1675.dirty

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox