Git development
 help / color / mirror / Atom feed
* Re: Debugging corrupt object generation
From: Julian Phillips @ 2007-11-02  0:26 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: git
In-Reply-To: <alpine.LFD.0.9999.0711011947220.21255@xanadu.home>

On Thu, 1 Nov 2007, Nicolas Pitre wrote:

> On Thu, 1 Nov 2007, Julian Phillips wrote:
>
>> After a certain commit, and attempt to use that commit generates a "fatal:
>> unable to apply delta".  This appears to be coming from unpack_delta_entry
>> in sha1_file.c.
>
> I suppose you mean "fatal: failed to apply delta", because "unable to
> apply delta" doesn't appear anywhere in the current source tree.

I did indeed.  Read 'n' type never seems to be as reliable as copy 'n' 
paste ...

>> Can anyone give me any hints as to how I find out what is causing the
>> problem?  I'm not even sure what it is that isn't working ... and all
>> attempts to replicate the problem with my test repository have failed.
>
> Well, something is screwed for sure.  Some object you're requesting is
> made of a delta, and that delta is bad, therefore patch_delta() returns
> NULL (you could instrument it to determine exactly why).

Ah, ok - that sounds like it might be useful.  I guess that looking at the 
opposite side where fast-import is creating the data might also prove 
fruitful.

> Maybe fast-import hasn't flushed the needed data to the pack yet?

Well, fast-import completes quite happily and outputs the normal summary 
status.  I can look at logs and trees etc provided that I don't try and 
look at one particular part of the tree on one particular commit.  I 
think the problem is that I've managed to do something inside fast-import 
that corrupts one particular tree object (though I've no idea what that 
might be).

I shall have to start instrumenting and try again.

Thanks.

-- 
Julian

  ---
Many changes of mind and mood; do not hesitate too long.

^ permalink raw reply

* [PATCH] gc: use parse_options
From: James Bowes @ 2007-11-02  0:28 UTC (permalink / raw)
  To: git, gitster

Signed-off-by: James Bowes <jbowes@dangerouslyinc.com>
---
 builtin-gc.c |   42 ++++++++++++++++++++----------------------
 1 files changed, 20 insertions(+), 22 deletions(-)

diff --git a/builtin-gc.c b/builtin-gc.c
index 3a2ca4f..7bb873c 100644
--- a/builtin-gc.c
+++ b/builtin-gc.c
@@ -12,11 +12,15 @@
 
 #include "builtin.h"
 #include "cache.h"
+#include "parse-options.h"
 #include "run-command.h"
 
 #define FAILED_RUN "failed to run %s"
 
-static const char builtin_gc_usage[] = "git-gc [--prune] [--aggressive]";
+static const char * const builtin_gc_usage[] = {
+	"git-gc [options]",
+	NULL
+};
 
 static int pack_refs = 1;
 static int aggressive_window = -1;
@@ -165,38 +169,32 @@ static int need_to_gc(void)
 
 int cmd_gc(int argc, const char **argv, const char *prefix)
 {
-	int i;
 	int prune = 0;
+	int aggressive = 0;
 	int auto_gc = 0;
 	char buf[80];
 
+	struct option builtin_gc_options[] = {
+		OPT_BOOLEAN(0, "prune", &prune, "prune unused objects"),
+		OPT_BOOLEAN(0, "aggressive", &aggressive, "be more thorough (increased runtime)"),
+		OPT_BOOLEAN(0, "auto", &auto_gc, "enable auto-gc mode"),
+		OPT_END()
+	};
+
 	git_config(gc_config);
 
 	if (pack_refs < 0)
 		pack_refs = !is_bare_repository();
 
-	for (i = 1; i < argc; i++) {
-		const char *arg = argv[i];
-		if (!strcmp(arg, "--prune")) {
-			prune = 1;
-			continue;
-		}
-		if (!strcmp(arg, "--aggressive")) {
-			append_option(argv_repack, "-f", MAX_ADD);
-			if (aggressive_window > 0) {
-				sprintf(buf, "--window=%d", aggressive_window);
-				append_option(argv_repack, buf, MAX_ADD);
-			}
-			continue;
-		}
-		if (!strcmp(arg, "--auto")) {
-			auto_gc = 1;
-			continue;
+	parse_options(argc, argv, builtin_gc_options, builtin_gc_usage, 0);
+
+	if (aggressive) {
+		append_option(argv_repack, "-f", MAX_ADD);
+		if (aggressive_window > 0) {
+			sprintf(buf, "--window=%d", aggressive_window);
+			append_option(argv_repack, buf, MAX_ADD);
 		}
-		break;
 	}
-	if (i != argc)
-		usage(builtin_gc_usage);
 
 	if (auto_gc) {
 		/*
-- 
1.5.3.4.1481.g854da

^ permalink raw reply related

* Re: What's cooking in git.git (topics)
From: Junio C Hamano @ 2007-11-02  0:32 UTC (permalink / raw)
  To: Mike Hommey; +Cc: Geert Bosch, Linus Torvalds, git
In-Reply-To: <7vhck5acj4.fsf@gitster.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> I am personally very much in favor of removing "git clean", but
> having many people on the list saying loudly that it is a bad
> command is not good enough justification, as people who are
> content with the status quo tend to be silent.
>
> The steps I think is sensible to transition to that goal would
> be:
>
>  - Change clean.requireForce to default to 'true' in the next
>    (or one after) version of git, to make 'clean' even safer.
>    See if anybody complains (I do not expect any).
>
>  - Implement the same functionarity as a new option to "git rm",
>    which is already in C.
>
>  - Do "git clean" in C, but sharing the code with "git rm"
>    implementation above.
>
>  - Discuss deprecation and removal of redundant commands.  Ship
>    a version of git with deprecation and future removal notice.
>    Outline how to achieve the same thing as the deprecated
>    command used to do (or give convincing argument why what the
>    deprecated command used to do was a bad thing and do not
>    offer an alternative).
>
>  - Wait for a while (6 months to 1 year) and then remove them.

And this is the first step.

---

 Documentation/config.txt |    4 ++--
 git-clean.sh             |    6 +++++-
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index edf50cd..2144855 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -345,8 +345,8 @@ branch.<name>.mergeoptions::
 	supported.
 
 clean.requireForce::
-	A boolean to make git-clean do nothing unless given -f or -n.  Defaults
-	to false.
+	A boolean to make git-clean do nothing unless given -f
+	or -n.   Defaults to true.
 
 color.branch::
 	A boolean to enable/disable color in the output of
diff --git a/git-clean.sh b/git-clean.sh
index 4491738..f4965b8 100755
--- a/git-clean.sh
+++ b/git-clean.sh
@@ -20,12 +20,16 @@ require_work_tree
 ignored=
 ignoredonly=
 cleandir=
-disabled="`git config --bool clean.requireForce`"
 rmf="rm -f --"
 rmrf="rm -rf --"
 rm_refuse="echo Not removing"
 echo1="echo"
 
+# requireForce used to default to false but now it defaults to true.
+# IOW, lack of explicit "clean.requireForce = false" is taken as
+# "clean.requireForce = true".
+disabled=$(git config --bool clean.requireForce || echo true)
+
 while test $# != 0
 do
 	case "$1" in

^ permalink raw reply related

* [PATCH] post-update hook: update working copy
From: Sam Vilain @ 2007-11-02  0:45 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Sam Vilain

Now that git-stash is available, it is not so unsafe to push to a
non-bare repository, but care needs to be taken to preserve any dirty
working copy or index state.  This hook script does that, using
git-stash.

Signed-off-by: Sam Vilain <sam.vilain@catalyst.net.nz>
---
 templates/hooks--post-update |   76 ++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 73 insertions(+), 3 deletions(-)
 mode change 100644 => 100755 templates/hooks--post-update

diff --git a/templates/hooks--post-update b/templates/hooks--post-update
old mode 100644
new mode 100755
index bcba893..352a432
--- a/templates/hooks--post-update
+++ b/templates/hooks--post-update
@@ -1,8 +1,78 @@
 #!/bin/sh
 #
-# An example hook script to prepare a packed repository for use over
-# dumb transports.
+# This hook does two things:
+#
+#  1. update the "info" files that allow the list of references to be
+#     queries over dumb transports such as http
+#
+#  2. if this repository looks like it is a non-bare repository, and
+#     the checked-out branch is pushed to, then update the working copy.
+#     This makes "push" function somewhat similarly to darcs and bzr.
 #
 # To enable this hook, make this file executable by "chmod +x post-update".
 
-exec git-update-server-info
+git-update-server-info
+
+is_bare=$(git-config --get --bool core.bare)
+
+if [ -z "$is_bare" ]
+then
+	# for compatibility's sake, guess
+	git_dir_full=$(cd $GIT_DIR; pwd)
+	case $git_dir_full in */.git) is_bare=false;; *) is_bare=true;; esac
+fi
+
+update_wc() {
+	ref=$1
+	echo "Push to checked out branch $ref" >&2
+	if (cd $GIT_WORK_TREE; git-diff-files -q --exit-code >/dev/null)
+	then
+		wc_dirty=0
+	else
+		echo "W:unstaged changes found in working copy" >&2
+		wc_dirty=1
+		desc="working copy"
+	fi
+	if git diff-index HEAD@{1} >/dev/null
+	then
+		index_dirty=0
+	else
+		echo "W:uncommitted, staged changes found" >&2
+		index_dirty=1
+		if [ -n "$desc" ]
+		then
+			desc="$desc and index"
+		else
+			desc="index"
+		fi
+	fi
+	if [ "$wc_dirty" -ne 0 -o "$index_dirty" -ne 0 ]
+	then
+		new=$(git rev-parse HEAD)
+		git-update-ref --no-deref HEAD HEAD@{1}
+		echo "W:stashing dirty $desc - see git-stash(1)" >&2
+		(cd $GIT_WORK_TREE
+		git stash save "dirty $desc before update to $new")
+		git-symbolic-ref HEAD "$ref"
+	fi
+
+	# eye candy - show the WC updates :)
+	echo "Updating working copy" >&2
+	(cd $GIT_WORK_TREE
+	git-diff-index -R --name-status HEAD >&2
+	git-reset --hard HEAD)
+}
+
+if [ "$is_bare" = "false" ]
+then
+	active_branch=`git-symbolic-ref HEAD`
+	export GIT_DIR=$(cd $GIT_DIR; pwd)
+	GIT_WORK_TREE=${GIT_WORK_TREE-..}
+	for ref
+	do
+		if [ "$ref" = "$active_branch" ]
+		then
+			update_wc $ref
+		fi
+	done
+fi
-- 
1.5.3.2.3.g2f2dcc-dirty

^ permalink raw reply related

* Re: [PATCH] gc: use parse_options
From: Junio C Hamano @ 2007-11-02  0:49 UTC (permalink / raw)
  To: James Bowes; +Cc: git, gitster
In-Reply-To: <20071102002856.GB3282@crux.yyz.redhat.com>

James Bowes <jbowes@dangerouslyinc.com> writes:

> +	struct option builtin_gc_options[] = {
> +		OPT_BOOLEAN(0, "prune", &prune, "prune unused objects"),

I would write "unreferenced loose" instead of "unused" here...

> +		OPT_BOOLEAN(0, "aggressive", &aggressive, "be more thorough (increased runtime)"),
> +		OPT_BOOLEAN(0, "auto", &auto_gc, "enable auto-gc mode"),
> +		OPT_END()
> +	};
> +
>  	git_config(gc_config);
>  
>  	if (pack_refs < 0)
>  		pack_refs = !is_bare_repository();
>  
> +	parse_options(argc, argv, builtin_gc_options, builtin_gc_usage, 0);
> +
> +	if (aggressive) {
> +		append_option(argv_repack, "-f", MAX_ADD);
> +		if (aggressive_window > 0) {
> +			sprintf(buf, "--window=%d", aggressive_window);
> +			append_option(argv_repack, buf, MAX_ADD);
>  		}
>  	}
> -	if (i != argc)
> -		usage(builtin_gc_usage);

Now, what makes the command report error when the user says:

	$ git gc unwanted parameter

Other than that, this is a good thing to have, I think.

^ permalink raw reply

* Re: [PATCH] post-update hook: update working copy
From: Junio C Hamano @ 2007-11-02  1:02 UTC (permalink / raw)
  To: Sam Vilain; +Cc: git
In-Reply-To: <1193964304-10847-1-git-send-email-sam.vilain@catalyst.net.nz>

Sam Vilain <sam.vilain@catalyst.net.nz> writes:

> Now that git-stash is available, it is not so unsafe to push to a
> non-bare repository, but care needs to be taken to preserve any dirty
> working copy or index state.  This hook script does that, using
> git-stash.

Honestly, I am reluctant to do things that _encourages_ pushing
into a live tree.

 - Who guarantees that the reflog is enabled for the HEAD?

 - Who guarantees that a human user is not actively editing the
   work tree files without saving?  You would not see "dirty
   state", the editor would notice "the file was modified since
   you started editing it" and tell so to the user, but the user
   cannot recover from the situation without knowing to do the
   three-way merge between HEAD@{1}, HEAD and the index _anyway_.

> +update_wc() {
> +	ref=$1
> +	echo "Push to checked out branch $ref" >&2
> +	if (cd $GIT_WORK_TREE; git-diff-files -q --exit-code >/dev/null)
> +	then
> +		wc_dirty=0
> +	else
> +		echo "W:unstaged changes found in working copy" >&2
> +		wc_dirty=1
> +		desc="working copy"
> +	fi
> +	if git diff-index HEAD@{1} >/dev/null

Are you missing "--cached" here?

> +	if [ "$wc_dirty" -ne 0 -o "$index_dirty" -ne 0 ]
> +	then
> +		new=$(git rev-parse HEAD)
> +		git-update-ref --no-deref HEAD HEAD@{1}
> +		echo "W:stashing dirty $desc - see git-stash(1)" >&2
> +		(cd $GIT_WORK_TREE
> +		git stash save "dirty $desc before update to $new")
> +		git-symbolic-ref HEAD "$ref"

This part feels somewhat dangerous.  What happens if we are
interrupted in the middle of these commands?

> +	fi
> +
> +	# eye candy - show the WC updates :)
> +	echo "Updating working copy" >&2
> +	(cd $GIT_WORK_TREE
> +	git-diff-index -R --name-status HEAD >&2
> +	git-reset --hard HEAD)
> +}

And I would have expected you would unstash the dirty state here.
Are there any reason not to?

^ permalink raw reply

* [PATCH] gc: use parse_options
From: James Bowes @ 2007-11-02  1:02 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vhck579pm.fsf@gitster.siamese.dyndns.org>

Signed-off-by: James Bowes <jbowes@dangerouslyinc.com>
---

Junio C Hamano <gitster@pobox.com> writes:
> Now, what makes the command report error when the user says:
>
>	$ git gc unwanted parameter

Ah yes. I forgot about that :)

This version of the patch errors out with extra args, and calls them
'unreferenced loose objects' rather than unused.

-James

 builtin-gc.c |   44 ++++++++++++++++++++++----------------------
 1 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/builtin-gc.c b/builtin-gc.c
index 3a2ca4f..c5bce89 100644
--- a/builtin-gc.c
+++ b/builtin-gc.c
@@ -12,11 +12,15 @@
 
 #include "builtin.h"
 #include "cache.h"
+#include "parse-options.h"
 #include "run-command.h"
 
 #define FAILED_RUN "failed to run %s"
 
-static const char builtin_gc_usage[] = "git-gc [--prune] [--aggressive]";
+static const char * const builtin_gc_usage[] = {
+	"git-gc [options]",
+	NULL
+};
 
 static int pack_refs = 1;
 static int aggressive_window = -1;
@@ -165,38 +169,34 @@ static int need_to_gc(void)
 
 int cmd_gc(int argc, const char **argv, const char *prefix)
 {
-	int i;
 	int prune = 0;
+	int aggressive = 0;
 	int auto_gc = 0;
 	char buf[80];
 
+	struct option builtin_gc_options[] = {
+		OPT_BOOLEAN(0, "prune", &prune, "prune unreferenced loose objects"),
+		OPT_BOOLEAN(0, "aggressive", &aggressive, "be more thorough (increased runtime)"),
+		OPT_BOOLEAN(0, "auto", &auto_gc, "enable auto-gc mode"),
+		OPT_END()
+	};
+
 	git_config(gc_config);
 
 	if (pack_refs < 0)
 		pack_refs = !is_bare_repository();
 
-	for (i = 1; i < argc; i++) {
-		const char *arg = argv[i];
-		if (!strcmp(arg, "--prune")) {
-			prune = 1;
-			continue;
-		}
-		if (!strcmp(arg, "--aggressive")) {
-			append_option(argv_repack, "-f", MAX_ADD);
-			if (aggressive_window > 0) {
-				sprintf(buf, "--window=%d", aggressive_window);
-				append_option(argv_repack, buf, MAX_ADD);
-			}
-			continue;
-		}
-		if (!strcmp(arg, "--auto")) {
-			auto_gc = 1;
-			continue;
+	argc = parse_options(argc, argv, builtin_gc_options, builtin_gc_usage, 0);
+	if (argc > 0)
+		usage_with_options(builtin_gc_usage, builtin_gc_options);
+
+	if (aggressive) {
+		append_option(argv_repack, "-f", MAX_ADD);
+		if (aggressive_window > 0) {
+			sprintf(buf, "--window=%d", aggressive_window);
+			append_option(argv_repack, buf, MAX_ADD);
 		}
-		break;
 	}
-	if (i != argc)
-		usage(builtin_gc_usage);
 
 	if (auto_gc) {
 		/*
-- 
1.5.3.4.1481.g854da

^ permalink raw reply related

* git rm --cached
From: Jing Xue @ 2007-11-02  2:17 UTC (permalink / raw)
  To: git


In the following scenario, why do I have to run 'git reset' following
'git rm --cached 1.txt' to revert to exactly where I was before 'git add
1.txt'?  Shouldn't 'git rm --cached' have done that already?

jingxue@fawkes:~/workspace/t1.git$ git status
# On branch master
# Changed but not updated:
#   (use "git add <file>..." to update what will be committed)
#
#       modified:   1.txt
#
no changes added to commit (use "git add" and/or "git commit -a")
jingxue@fawkes:~/workspace/t1.git$ git add 1.txt
jingxue@fawkes:~/workspace/t1.git$ git status
# On branch master
# Changes to be committed:
#   (use "git reset HEAD <file>..." to unstage)
#
#       modified:   1.txt
#
jingxue@fawkes:~/workspace/t1.git$ git rm --cached 1.txt
rm '1.txt'
jingxue@fawkes:~/workspace/t1.git$ git status
# On branch master
# Changes to be committed:
#   (use "git reset HEAD <file>..." to unstage)
#
#       deleted:    1.txt
#
# Untracked files:
#   (use "git add <file>..." to include in what will be committed)
#
#       1.txt
jingxue@fawkes:~/workspace/t1.git$ git reset
1.txt: needs update
jingxue@fawkes:~/workspace/t1.git$ git status
# On branch master
# Changed but not updated:
#   (use "git add <file>..." to update what will be committed)
#
#       modified:   1.txt
#
no changes added to commit (use "git add" and/or "git commit -a")

Thanks.
-- 
Jing Xue

^ permalink raw reply

* Re: What's cooking in git.git (topics)
From: Petr Baudis @ 2007-11-02  2:19 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Junio C Hamano, Johan Herland, git, Geert Bosch
In-Reply-To: <alpine.LFD.0.999.0711011459490.3342@woody.linux-foundation.org>

On Thu, Nov 01, 2007 at 03:05:44PM -0700, Linus Torvalds wrote:
> On Thu, 1 Nov 2007, Junio C Hamano wrote:
> > 
> > "git clean" is about things that git usually do not care about
> > (i.e. things not in .gitignore, or even in .gitignore when -x is
> > given).  Everything else including "git stash" is all about what
> > git cares about (i.e. tracked paths).

BTW, it comes from Cogito. Pavel Roskin is the author of the original
Cogito script and I'm not sure if it came there from anywhere else or is
an original "invention".

> Right. I *love* "git clean". Real men have everything they care about 
> tracked by git, and thus by definition "git clean" is the safest operation 
> possible. I don't understand how anybody can call it "unsafe".

I agree! If you want to keep anything around in git-tracked tree, tell
git about it! (Either marking it as ignored or adding it to the index.)
I think I wasn't too fond of it originally but now tend to use it a lot
to keep my trees clean of temporary cruft.

> I just wish it was quiet by default - right now it takes a _loong_ time to 
> clean out your crud just because it scrolls forever talking about all 
> those girly files you don't want to keep - and that it had -x and -d on by 
> default, so that us *real* men wouldn't have to type so much.

I do not agree with either, though. Having it verbose by default makes
it at least obvious that something potentially, uh, surprising is going
on; and I _prefer_ the non-x behaviour. If I told git that it should
ignore $file, it better not touch it.

> But yeah, I guess we could make the "clean.Imagirlyman" option (or however 
> the "please-don't-hurt-me-by-default" option is spelled) the default. That 
> way I'd just feel *extra* manly for immediately disabling it, and laughing 
> at you wimps.

Yeah!

-- 
				Petr "Pasky, laughing at the wimps" Baudis
We don't know who it was that discovered water, but we're pretty sure
that it wasn't a fish.		-- Marshall McLuhan

^ permalink raw reply

* Re: What's cooking in git.git (topics)
From: Petr Baudis @ 2007-11-02  2:23 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <fgdphj$6ga$1@ger.gmane.org>

On Fri, Nov 02, 2007 at 01:04:03AM +0100, Jakub Narebski wrote:
> Is 'getopts' bash-ism, or is it in POSIX?

	http://www.opengroup.org/onlinepubs/009695399/utilities/getopts.html

(Also, most modern distributions have manpage section 1p (3p, ...) with
the same contents, so you can check for this stuff pretty easily.)

-- 
				Petr "Pasky" Baudis
We don't know who it was that discovered water, but we're pretty sure
that it wasn't a fish.		-- Marshall McLuhan

^ permalink raw reply

* [PATCH] make the pack index version configurable
From: Nicolas Pitre @ 2007-11-02  3:26 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

It is a good idea to use pack index version 2 all the time since it has
proper protection against propagation of certain pack corruptions when
repacking which is not possible with index version 1, as demonstrated
in test t5302.

Hence this config option.

The default is still pack index version 1.

Signed-off-by: Nicolas Pitre <nico@cam.org>
---
diff --git a/Documentation/config.txt b/Documentation/config.txt
index d4a476e..862b79e 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -660,6 +660,15 @@ pack.threads::
 	machines. The required amount of memory for the delta search window
 	is however multiplied by the number of threads.
 
+pack.indexVersion::
+	Specify the default pack index version.  Valid values are 1 for
+	legacy pack index used by Git versions prior to 1.5.2, and 2 for
+	the new pack index with capabilities for packs larger than 4 GB
+	as well as proper protection against the repacking of corrupted
+	packs.  Version 2 is selected and this config option ignored
+	whenever the corresponding pack is larger than 2 GB.  Otherwise
+	the default is 1.
+
 pull.octopus::
 	The default merge strategy to use when pulling multiple branches
 	at once.
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 25ec65d..e923689 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -1768,6 +1768,12 @@ static int git_pack_config(const char *k, const char *v)
 #endif
 		return 0;
 	}
+	if (!strcmp(k, "pack.indexversion")) {
+		pack_idx_default_version = git_config_int(k, v);
+		if (pack_idx_default_version > 2)
+			die("bad pack.indexversion=%d", pack_idx_default_version);
+		return 0;
+	}
 	return git_default_config(k, v);
 }
 
diff --git a/index-pack.c b/index-pack.c
index 61ea762..715a5bb 100644
--- a/index-pack.c
+++ b/index-pack.c
@@ -683,6 +683,17 @@ static void final(const char *final_pack_name, const char *curr_pack_name,
 	}
 }
 
+static int git_index_pack_config(const char *k, const char *v)
+{
+	if (!strcmp(k, "pack.indexversion")) {
+		pack_idx_default_version = git_config_int(k, v);
+		if (pack_idx_default_version > 2)
+			die("bad pack.indexversion=%d", pack_idx_default_version);
+		return 0;
+	}
+	return git_default_config(k, v);
+}
+
 int main(int argc, char **argv)
 {
 	int i, fix_thin_pack = 0;
@@ -693,6 +704,8 @@ int main(int argc, char **argv)
 	struct pack_idx_entry **idx_objects;
 	unsigned char sha1[20];
 
+	git_config(git_index_pack_config);
+
 	for (i = 1; i < argc; i++) {
 		char *arg = argv[i];
 

^ permalink raw reply related

* Re: [PATCH] post-update hook: update working copy
From: Sam Vilain @ 2007-11-02  3:36 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Sam Vilain, git
In-Reply-To: <7vd4ut7948.fsf@gitster.siamese.dyndns.org>

Junio C Hamano wrote:
>> Now that git-stash is available, it is not so unsafe to push to a
>> non-bare repository, but care needs to be taken to preserve any dirty
>> working copy or index state.  This hook script does that, using
>> git-stash.
> 
> Honestly, I am reluctant to do things that _encourages_ pushing
> into a live tree.

I think a lot of people want this, and explaining why it doesn't work to
people is especially tiresome given that I know it can work, and the
caveats get smaller and smaller each time.  I still think that the
operation can be made safe enough for general use.

>  - Who guarantees that the reflog is enabled for the HEAD?

Ok, so I guess if that's the case then the old behaviour is OK.
However, this would not be required if implemented in receive-pack, as
the only thing I'm using it for is to get the revision that the current
index is based on.

The other option, which was writing two hooks, both of which need to be
enabled, seemed far too ugly!

>  - Who guarantees that a human user is not actively editing the
>    work tree files without saving?  You would not see "dirty
>    state", the editor would notice "the file was modified since
>    you started editing it" and tell so to the user, but the user
>    cannot recover from the situation without knowing to do the
>    three-way merge between HEAD@{1}, HEAD and the index _anyway_.

There seems to be a lot of focus on this.  However I don't think it is a
showstopper; in this instance that the person who has their life ruined
by the incoming push can blame the person who did it, blame themselves
for giving that stupid user access to their working directory, and then
accept that the tool is doing the best that it can.

>> +	if git diff-index HEAD@{1} >/dev/null
> 
> Are you missing "--cached" here?

Ah, yes, good catch.

>> +	if [ "$wc_dirty" -ne 0 -o "$index_dirty" -ne 0 ]
>> +	then
>> +		new=$(git rev-parse HEAD)
>> +		git-update-ref --no-deref HEAD HEAD@{1}
>> +		echo "W:stashing dirty $desc - see git-stash(1)" >&2
>> +		(cd $GIT_WORK_TREE
>> +		git stash save "dirty $desc before update to $new")
>> +		git-symbolic-ref HEAD "$ref"
> 
> This part feels somewhat dangerous.  What happens if we are
> interrupted in the middle of these commands?

Heh.  What seems to happen is that the local command is aborted and the
remote one continues.  Nonetheless I'll add a trap statement, but again
if implemented in receive-pack it could probably be more resilient.

>> +	fi
>> +
>> +	# eye candy - show the WC updates :)
>> +	echo "Updating working copy" >&2
>> +	(cd $GIT_WORK_TREE
>> +	git-diff-index -R --name-status HEAD >&2
>> +	git-reset --hard HEAD)
>> +}
> 
> And I would have expected you would unstash the dirty state here.
> Are there any reason not to?

If git-stash could support stashing conflicted merges, I don't think so.

However, if the user is a git-shell user, then they won't be able to
resolve the conflict and they won't be able to re-push as the stash will
fail (a condition not visited by the above).

Sam.

changes as you suggested are below;

diff --git a/templates/hooks--post-update b/templates/hooks--post-update
index 352a432..b15c711 100755
--- a/templates/hooks--post-update
+++ b/templates/hooks--post-update
@@ -25,6 +25,11 @@ fi
 update_wc() {
 	ref=$1
 	echo "Push to checked out branch $ref" >&2
+	if [ ! -f $GIT_DIR/logs/HEAD ]
+	then
+		echo "E:push to non-bare repository requires a HEAD reflog" >&2
+		exit 1
+	fi
 	if (cd $GIT_WORK_TREE; git-diff-files -q --exit-code >/dev/null)
 	then
 		wc_dirty=0
@@ -33,7 +38,7 @@ update_wc() {
 		wc_dirty=1
 		desc="working copy"
 	fi
-	if git diff-index HEAD@{1} >/dev/null
+	if git diff-index --cached HEAD@{1} >/dev/null
 	then
 		index_dirty=0
 	else
@@ -49,11 +54,13 @@ update_wc() {
 	if [ "$wc_dirty" -ne 0 -o "$index_dirty" -ne 0 ]
 	then
 		new=$(git rev-parse HEAD)
-		git-update-ref --no-deref HEAD HEAD@{1}
 		echo "W:stashing dirty $desc - see git-stash(1)" >&2
-		(cd $GIT_WORK_TREE
-		git stash save "dirty $desc before update to $new")
+		( trap 'echo trapped $$; git symbolic-ref HEAD "'"$ref"'"' 2 3 13 15 ERR EXIT
+		git-update-ref --no-deref HEAD HEAD@{1}
+		cd $GIT_WORK_TREE
+		git stash save "dirty $desc before update to $new";
 		git-symbolic-ref HEAD "$ref"
+		)
 	fi
 
 	# eye candy - show the WC updates :)

^ permalink raw reply related

* git-p4: detect changes to executable bit and push to p4
From: Chris Pettitt @ 2007-11-02  3:41 UTC (permalink / raw)
  To: Simon Hausmann, git


The following two patches add support to git-p4 for detecting changes to the
executable bit in git and including these changes in the p4 changeset during
git-p4 submit.

^ permalink raw reply

* Re: [PATCH] make the pack index version configurable
From: David Symonds @ 2007-11-02  3:42 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.LFD.0.9999.0711012312480.21255@xanadu.home>

On 11/2/07, Nicolas Pitre <nico@cam.org> wrote:
>
> +pack.indexVersion::
> +       Specify the default pack index version.  Valid values are 1 for
> +       legacy pack index used by Git versions prior to 1.5.2, and 2 for
> +       the new pack index with capabilities for packs larger than 4 GB
> +       as well as proper protection against the repacking of corrupted
> +       packs.  Version 2 is selected and this config option ignored
> +       whenever the corresponding pack is larger than 2 GB.  Otherwise
> +       the default is 1.

If you're trying to force a previous pack version for some reason
(backward compatibility, or whatever), this "feature" of automatic
forcing version 2 for 2 GB packs might come as a nasty suprise.
Wouldn't it be better to fail with an error?


Dave.

^ permalink raw reply

* [PATCH] git-p4: Add a helper function to parse the full git diff-tree output.
From: Chris Pettitt @ 2007-11-02  3:43 UTC (permalink / raw)
  To: Simon Hausmann, git; +Cc: Chris Pettitt


Signed-off-by: Chris Pettitt <cpettitt@gmail.com>
---
 contrib/fast-import/git-p4 |   49 ++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index bf33f74..c7fc564 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -71,6 +71,46 @@ def isP4Exec(kind):
     a plus sign, it is also executable"""
     return (re.search(r"(^[cku]?x)|\+.*x", kind) != None)
 
+def diffTreePattern():
+    # This is a simple generator for the diff tree regex pattern. This could be
+    # a class variable if this and parseDiffTreeEntry were a part of a class.
+    pattern = re.compile(':(\d+) (\d+) (\w+) (\w+) ([A-Z])(\d+)?\t(.*?)((\t(.*))|$)')
+    while True:
+        yield pattern
+
+def parseDiffTreeEntry(entry):
+    """Parses a single diff tree entry into its component elements.
+
+    See git-diff-tree(1) manpage for details about the format of the diff
+    output. This method returns a dictionary with the following elements:
+
+    src_mode - The mode of the source file
+    dst_mode - The mode of the destination file
+    src_sha1 - The sha1 for the source file
+    dst_sha1 - The sha1 fr the destination file
+    status - The one letter status of the diff (i.e. 'A', 'M', 'D', etc)
+    status_score - The score for the status (applicable for 'C' and 'R'
+                   statuses). This is None if there is no score.
+    src - The path for the source file.
+    dst - The path for the destination file. This is only present for
+          copy or renames. If it is not present, this is None.
+
+    If the pattern is not matched, None is returned."""
+
+    match = diffTreePattern().next().match(entry)
+    if match:
+        return {
+            'src_mode': match.group(1),
+            'dst_mode': match.group(2),
+            'src_sha1': match.group(3),
+            'dst_sha1': match.group(4),
+            'status': match.group(5),
+            'status_score': match.group(6),
+            'src': match.group(7),
+            'dst': match.group(10)
+        }
+    return None
+
 def p4CmdList(cmd, stdin=None, stdin_mode='w+b'):
     cmd = "p4 -G %s" % cmd
     if verbose:
@@ -494,13 +534,14 @@ class P4Submit(Command):
         else:
             print "Applying %s" % (read_pipe("git log --max-count=1 --pretty=oneline %s" % id))
             diffOpts = ("", "-M")[self.detectRename]
-            diff = read_pipe_lines("git diff-tree -r --name-status %s \"%s^\" \"%s\"" % (diffOpts, id, id))
+            diff = read_pipe_lines("git diff-tree -r %s \"%s^\" \"%s\"" % (diffOpts, id, id))
         filesToAdd = set()
         filesToDelete = set()
         editedFiles = set()
         for line in diff:
-            modifier = line[0]
-            path = line[1:].strip()
+            diff = parseDiffTreeEntry(line)
+            modifier = diff['status']
+            path = diff['src']
             if modifier == "M":
                 system("p4 edit \"%s\"" % path)
                 editedFiles.add(path)
@@ -513,7 +554,7 @@ class P4Submit(Command):
                 if path in filesToAdd:
                     filesToAdd.remove(path)
             elif modifier == "R":
-                src, dest = line.strip().split("\t")[1:3]
+                src, dest = diff['src'], diff['dst']
                 system("p4 integrate -Dt \"%s\" \"%s\"" % (src, dest))
                 system("p4 edit \"%s\"" % (dest))
                 os.unlink(dest)
-- 
1.5.3.4.498.g9c514

^ permalink raw reply related

* [PATCH] git-p4: Detect changes to executable bit and include them in p4 submit.
From: Chris Pettitt @ 2007-11-02  3:43 UTC (permalink / raw)
  To: Simon Hausmann, git; +Cc: Chris Pettitt
In-Reply-To: <1193974994-19211-1-git-send-email-cpettitt@gmail.com>

This changeset takes advantage of the new parseDiffTreeEntry(...) function to
detect changes to the execute bit in the git repository.  During submit, git-p4
now looks for changes to the executable bit and if it finds them it "reopens"
the file in perforce, which allows it to change the file type.

The logic for adding the executable bit in perforce is straightforward: the +x
modifier can be used. Removing the executable bit in perforce requires that the
entire filetype be redefined (there is no way to join remove the bit with a -x
modifier, for example). This changeset includes logic to remove the executable
bit from the full file type while preserving the base file type and other
modifiers.

Signed-off-by: Chris Pettitt <cpettitt@gmail.com>
---
 contrib/fast-import/git-p4 |   44 ++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 44 insertions(+), 0 deletions(-)

diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index c7fc564..c148b5a 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -71,6 +71,31 @@ def isP4Exec(kind):
     a plus sign, it is also executable"""
     return (re.search(r"(^[cku]?x)|\+.*x", kind) != None)
 
+def setP4ExecBit(file, mode):
+    # Reopens an already open file and changes the execute bit to match
+    # the execute bit setting in the passed in mode.
+
+    p4Type = "+x"
+
+    if not isModeExec(mode):
+        p4Type = getP4OpenedType(file)
+        p4Type = re.sub('^([cku]?)x(.*)', '\\1\\2', p4Type)
+        p4Type = re.sub('(.*?\+.*?)x(.*?)', '\\1\\2', p4Type)
+        if p4Type[-1] == "+":
+            p4Type = p4Type[0:-1]
+
+    system("p4 reopen -t %s %s" % (p4Type, file))
+
+def getP4OpenedType(file):
+    # Returns the perforce file type for the given file.
+
+    result = read_pipe("p4 opened %s" % file)
+    match = re.match(".*\((.+)\)$", result)
+    if match:
+        return match.group(1)
+    else:
+        die("Could not determine file type for %s" % file)
+
 def diffTreePattern():
     # This is a simple generator for the diff tree regex pattern. This could be
     # a class variable if this and parseDiffTreeEntry were a part of a class.
@@ -111,6 +136,14 @@ def parseDiffTreeEntry(entry):
         }
     return None
 
+def isModeExec(mode):
+    # Returns True if the given git mode represents an executable file,
+    # otherwise False.
+    return mode[-3:] == "755"
+
+def isModeExecChanged(src_mode, dst_mode):
+    return isModeExec(src_mode) != isModeExec(dst_mode)
+
 def p4CmdList(cmd, stdin=None, stdin_mode='w+b'):
     cmd = "p4 -G %s" % cmd
     if verbose:
@@ -538,15 +571,19 @@ class P4Submit(Command):
         filesToAdd = set()
         filesToDelete = set()
         editedFiles = set()
+        filesToChangeExecBit = {}
         for line in diff:
             diff = parseDiffTreeEntry(line)
             modifier = diff['status']
             path = diff['src']
             if modifier == "M":
                 system("p4 edit \"%s\"" % path)
+                if isModeExecChanged(diff['src_mode'], diff['dst_mode']):
+                    filesToChangeExecBit[path] = diff['dst_mode']
                 editedFiles.add(path)
             elif modifier == "A":
                 filesToAdd.add(path)
+                filesToChangeExecBit[path] = diff['dst_mode']
                 if path in filesToDelete:
                     filesToDelete.remove(path)
             elif modifier == "D":
@@ -557,6 +594,8 @@ class P4Submit(Command):
                 src, dest = diff['src'], diff['dst']
                 system("p4 integrate -Dt \"%s\" \"%s\"" % (src, dest))
                 system("p4 edit \"%s\"" % (dest))
+                if isModeExecChanged(diff['src_mode'], diff['dst_mode']):
+                    filesToChangeExecBit[dest] = diff['dst_mode']
                 os.unlink(dest)
                 editedFiles.add(dest)
                 filesToDelete.add(src)
@@ -609,6 +648,11 @@ class P4Submit(Command):
             system("p4 revert \"%s\"" % f)
             system("p4 delete \"%s\"" % f)
 
+        # Set/clear executable bits
+        for f in filesToChangeExecBit.keys():
+            mode = filesToChangeExecBit[f]
+            setP4ExecBit(f, mode)
+
         logMessage = ""
         if not self.directSubmit:
             logMessage = extractLogMessageFromGitCommit(id)
-- 
1.5.3.4.498.g9c514

^ permalink raw reply related

* [PATCH] pack-objects: get rid of an ugly cast
From: Nicolas Pitre @ 2007-11-02  3:43 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

... when calling write_idx_file().

Signed-off-by: Nicolas Pitre <nico@cam.org>
---
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 25ec65d..a9d7633 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -57,7 +57,7 @@ struct object_entry {
  * nice "minimum seek" order.
  */
 static struct object_entry *objects;
-static struct object_entry **written_list;
+static struct pack_idx_entry **written_list;
 static uint32_t nr_objects, nr_alloc, nr_result, nr_written;
 
 static int non_empty;
@@ -577,7 +577,7 @@ static off_t write_one(struct sha1file *f,
 		e->idx.offset = 0;
 		return 0;
 	}
-	written_list[nr_written++] = e;
+	written_list[nr_written++] = &e->idx;
 
 	/* make sure off_t is sufficiently large not to wrap */
 	if (offset > offset + size)
@@ -599,7 +599,7 @@ static void write_pack_file(void)
 
 	if (do_progress)
 		progress_state = start_progress("Writing objects", nr_result);
-	written_list = xmalloc(nr_objects * sizeof(struct object_entry *));
+	written_list = xmalloc(nr_objects * sizeof(*written_list));
 
 	do {
 		unsigned char sha1[20];
@@ -651,9 +651,8 @@ static void write_pack_file(void)
 			umask(mode);
 			mode = 0444 & ~mode;
 
-			idx_tmp_name = write_idx_file(NULL,
-					(struct pack_idx_entry **) written_list,
-					nr_written, sha1);
+			idx_tmp_name = write_idx_file(NULL, written_list,
+						      nr_written, sha1);
 			snprintf(tmpname, sizeof(tmpname), "%s-%s.pack",
 				 base_name, sha1_to_hex(sha1));
 			if (adjust_perm(pack_tmp_name, mode))
@@ -677,7 +676,7 @@ static void write_pack_file(void)
 
 		/* mark written objects as written to previous pack */
 		for (j = 0; j < nr_written; j++) {
-			written_list[j]->idx.offset = (off_t)-1;
+			written_list[j]->offset = (off_t)-1;
 		}
 		nr_remaining -= nr_written;
 	} while (nr_remaining && i < nr_objects);

^ permalink raw reply related

* [PATCH] Fixed a gcc 4.0.1 complaint about an uninitialized variable
From: Blake Ramsdell @ 2007-11-02  2:38 UTC (permalink / raw)
  To: gitster, git; +Cc: Blake Ramsdell
In-Reply-To: <1193971102-61907-1-git-send-email-blaker@gmail.com>

Signed-off-by: Blake Ramsdell <blaker@gmail.com>
---
 transport.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/transport.c b/transport.c
index 400af71..cac1870 100644
--- a/transport.c
+++ b/transport.c
@@ -107,7 +107,7 @@ static void insert_packed_refs(const char *packed_refs, struct ref **list)
 		return;
 
 	for (;;) {
-		int cmp, len;
+		int cmp = 0, len;
 
 		if (!fgets(buffer, sizeof(buffer), f)) {
 			fclose(f);
-- 
1.5.3.GIT

^ permalink raw reply related

* [PATCH] Mac OS X 10.5 does not require the OLD_ICONV flag set
From: Blake Ramsdell @ 2007-11-02  2:38 UTC (permalink / raw)
  To: gitster, git; +Cc: Blake Ramsdell

Signed-off-by: Blake Ramsdell <blaker@gmail.com>
---
 Makefile |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index 71479a2..5d83756 100644
--- a/Makefile
+++ b/Makefile
@@ -401,7 +401,9 @@ endif
 ifeq ($(uname_S),Darwin)
 	NEEDS_SSL_WITH_CRYPTO = YesPlease
 	NEEDS_LIBICONV = YesPlease
-	OLD_ICONV = UnfortunatelyYes
+	ifneq ($(uname_R),9.0.0)
+		OLD_ICONV = UnfortunatelyYes
+	endif
 	NO_STRLCPY = YesPlease
 	NO_MEMMEM = YesPlease
 endif
-- 
1.5.3.GIT

^ permalink raw reply related

* Re: [PATCH] make the pack index version configurable
From: Nicolas Pitre @ 2007-11-02  4:01 UTC (permalink / raw)
  To: David Symonds; +Cc: Junio C Hamano, git
In-Reply-To: <ee77f5c20711012042r39c6303em2a8140e8348051d@mail.gmail.com>

On Fri, 2 Nov 2007, David Symonds wrote:

> On 11/2/07, Nicolas Pitre <nico@cam.org> wrote:
> >
> > +pack.indexVersion::
> > +       Specify the default pack index version.  Valid values are 1 for
> > +       legacy pack index used by Git versions prior to 1.5.2, and 2 for
> > +       the new pack index with capabilities for packs larger than 4 GB
> > +       as well as proper protection against the repacking of corrupted
> > +       packs.  Version 2 is selected and this config option ignored
> > +       whenever the corresponding pack is larger than 2 GB.  Otherwise
> > +       the default is 1.
> 
> If you're trying to force a previous pack version for some reason
> (backward compatibility, or whatever), this "feature" of automatic
> forcing version 2 for 2 GB packs might come as a nasty suprise.
> Wouldn't it be better to fail with an error?

No.  The fact is that you don't know in advance what the pack size will 
be, and reaching that limit might take a long while already if 
repacking, or even more so if the pack is fetched over a network 
connection.  That would be wasteful to simply throw away all the whole 
repack/download with an error which would be an even nastier surprise at 
that point just because you specified index v1.

And such a pack would be impossible to work with using an old Git 
version anyway, so the old Git version will happily give you the error, 
suggesting that you upgrade to a later version.

And this has been the implemented behavior for the last 7 months 
already.

And the idea is to switch the default to version 2 eventually.

Etc.


Nicolas

^ permalink raw reply

* Re: [PATCH 3/3] Show total transferred as part of throughput progress
From: Shawn O. Pearce @ 2007-11-02  4:59 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Junio C Hamano, git
In-Reply-To: <1193950797-29631-4-git-send-email-nico@cam.org>

Nicolas Pitre <nico@cam.org> wrote:
> Right now it is infeasible to offer to the user a reasonable concept
> of when a clone will be complete as we aren't able to come up with
> the final pack size until after we have actually transferred the
> entire thing to the client.  However in many cases users can work
> with a rough rule-of-thumb; for example it is somewhat well known
> that git.git is about 16 MiB today and that linux-2.6.git is over
> 120 MiB.
...
> [from an initial proposal from Shawn O. Pearce]

Thanks for rewriting this.  I agree, your replacement patch is
much better looking than my proposal.  I also see Junio has already
applied it to next.  Excellent.
 
-- 
Shawn.

^ permalink raw reply

* Re: [PATCH 1/3] Introduce --dirty option to git-rebase, allowing you to start from a dirty state.
From: Shawn O. Pearce @ 2007-11-02  5:08 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Simon Sasburg, git
In-Reply-To: <7vir4l8ug4.fsf@gitster.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Doesn't this have the exact same problem with the one in 'next'
> > that uses "git-stash create", which Shawn said he was upset
> > about, and I said I will revert?
> 
> Sorry, --dirty is not the default, which changes everything.
> Forget what I said, sorry for the noise.

I'm happy with having --dirty, but AS AN OPTION.  Heck, I'd probably
use it sometimes, but only if it also reapplies the stash after
the rebase is complete.  But doing that stash/apply automatically
is really freaking annoying.

For the same reasons why I like git-checkout not defaulting to -m.
I want Git to stop by default if I'm about to possibly go into a
command that is going to cause conflicts, as I may not be ready to
deal with them right now.

-- 
Shawn.

^ permalink raw reply

* Re: Debugging corrupt object generation
From: Shawn O. Pearce @ 2007-11-02  5:15 UTC (permalink / raw)
  To: Julian Phillips; +Cc: Nicolas Pitre, git
In-Reply-To: <Pine.LNX.4.64.0711020018400.18429@beast.quantumfyre.co.uk>

Julian Phillips <julian@quantumfyre.co.uk> wrote:
> On Thu, 1 Nov 2007, Nicolas Pitre wrote:
> >
> >Maybe fast-import hasn't flushed the needed data to the pack yet?
> 
> Well, fast-import completes quite happily and outputs the normal summary 
> status.  I can look at logs and trees etc provided that I don't try and 
> look at one particular part of the tree on one particular commit.  I 
> think the problem is that I've managed to do something inside fast-import 
> that corrupts one particular tree object (though I've no idea what that 
> might be).

Ahhh.  I'm betting you messed up the version 0 and version 1 arrays
inside of the struct tree_entry.  This could cause the delta
generator to look at the wrong base information when it creates
the delta, thus causing the delta to be created for a different
base than what the object is actually using in the packfile.

The version[0] is meant to hold the mode and SHA-1 of the tree_entry
in the base object.  The version[1] is meant to hold the current
mode and SHA-1 of the tree_entry in the new object.  A mode of 0
means the entry doesn't exist in that particular tree; so an add
is shown as "version[0].mode = 0; version[1].mode = 0100644".

Look at the store_tree function, this is where we regenerate the
canonical representation of both the version[0] and the version[1]
trees so the store_object function can generate a delta.  Note that
we assume the base object name is root->versions[0].sha1.  Maybe the
version[0] array doesn't actually match the tree named by that sha1?

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH] Fixed a gcc 4.0.1 complaint about an uninitialized variable
From: Junio C Hamano @ 2007-11-02  5:37 UTC (permalink / raw)
  To: Blake Ramsdell; +Cc: git
In-Reply-To: <1193971102-61907-2-git-send-email-blaker@gmail.com>

Blake Ramsdell <blaker@gmail.com> writes:

> diff --git a/transport.c b/transport.c
> index 400af71..cac1870 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -107,7 +107,7 @@ static void insert_packed_refs(const char *packed_refs, struct ref **list)
>  		return;
>  
>  	for (;;) {
> -		int cmp, len;
> +		int cmp = 0, len;

Yeah, if you follow the logic, it is clear that the variable is
never used while unset, but gcc is not careful enough to see it.

It is customary to use

	int cmp = cmp;

for something like this.  There are already other instances of
such phony initializations in the code elsewhere.

^ permalink raw reply

* Re: What's cooking in git.git (topics)
From: Miles Bader @ 2007-11-02  6:06 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.LFD.0.999.0711011129460.3342@woody.linux-foundation.org>

Linus Torvalds <torvalds@linux-foundation.org> writes:
> So while you can combine flags for *most* programs, you still won't 
> be able to say things like
>
> 	git clean -qdx
>
> just because that's still a shellscript, and doing any fancy argument 
> parsing in shell is just painful.

Indeed... but for my personal shell scripts I like to use a construct
like the following for parsing args:

   while :; do
     case "$1" in
        ... lots of cases to handle normal args ...

       -[!-]?*)
         # split concatenated single-letter options apart
         FIRST="$1"; shift
         set -- `echo $FIRST | $SED 's/-\(.\)\(.*\)/-\1 -\2/'` "$@"
         ;;

       -*)
         # unrecognized option
         unrec_opt "$1"; exit 1;;
       *)
         # non-option
         break;
     esac
   done

I'm sure there are problems with it, but it generally seems to work
pretty reasonably for short options.

-Miles
-- 
97% of everything is grunge

^ permalink raw reply


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