Git development
 help / color / mirror / Atom feed
* [PATCH 2/3] cvsimport: use show-ref to support packed refs
From: Jeff King @ 2007-11-28 18:56 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Emanuele Giaquinta, git
In-Reply-To: <20071128185504.GA11236@coredump.intra.peff.net>

Previously, if refs were packed, git-cvsimport would assume
that particular refs did not exist. This could lead to, for
example, overwriting previous 'origin' commits that were
packed.

Signed-off-by: Jeff King <peff@peff.net>
---
 git-cvsimport.perl   |   24 +++++++++---------------
 t/t9600-cvsimport.sh |    2 ++
 2 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index efa6a0c..b852f2f 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -527,18 +527,12 @@ sub is_sha1 {
 	return $s =~ /^[a-f0-9]{40}$/;
 }
 
-sub get_headref ($$) {
-    my $name    = shift;
-    my $git_dir = shift;
-
-    my $f = "$git_dir/$remote/$name";
-    if (open(my $fh, $f)) {
-	    chomp(my $r = <$fh>);
-	    is_sha1($r) or die "Cannot get head id for $name ($r): $!";
-	    return $r;
-    }
-    die "unable to open $f: $!" unless $! == POSIX::ENOENT;
-    return undef;
+sub get_headref ($) {
+	my $name = shift;
+	my $r = `git show-ref -s '$name'`;
+	return undef unless $? == 0;
+	chomp $r;
+	return $r;
 }
 
 -d $git_tree
@@ -698,7 +692,7 @@ my (@old,@new,@skipped,%ignorebranch);
 $ignorebranch{'#CVSPS_NO_BRANCH'} = 1;
 
 sub commit {
-	if ($branch eq $opt_o && !$index{branch} && !get_headref($branch, $git_dir)) {
+	if ($branch eq $opt_o && !$index{branch} && !get_headref($branch)) {
 	    # looks like an initial commit
 	    # use the index primed by git-init
 	    $ENV{GIT_INDEX_FILE} = "$git_dir/index";
@@ -722,7 +716,7 @@ sub commit {
 	update_index(@old, @new);
 	@old = @new = ();
 	my $tree = write_tree();
-	my $parent = get_headref($last_branch, $git_dir);
+	my $parent = get_headref($last_branch);
 	print "Parent ID " . ($parent ? $parent : "(empty)") . "\n" if $opt_v;
 
 	my @commit_args;
@@ -733,7 +727,7 @@ sub commit {
 	foreach my $rx (@mergerx) {
 		next unless $logmsg =~ $rx && $1;
 		my $mparent = $1 eq 'HEAD' ? $opt_o : $1;
-		if (my $sha1 = get_headref($mparent, $git_dir)) {
+		if (my $sha1 = get_headref($mparent)) {
 			push @commit_args, '-p', $mparent;
 			print "Merge parent branch: $mparent\n" if $opt_v;
 		}
diff --git a/t/t9600-cvsimport.sh b/t/t9600-cvsimport.sh
index 1ee06bb..3338d44 100755
--- a/t/t9600-cvsimport.sh
+++ b/t/t9600-cvsimport.sh
@@ -57,6 +57,8 @@ test_expect_success 'import a trivial module' '
 
 '
 
+test_expect_success 'pack refs' 'cd module-git && git gc && cd ..'
+
 test_expect_success 'update cvs module' '
 
 	cd module-cvs &&
-- 
1.5.3.6.2039.g0495

^ permalink raw reply related

* [PATCH 3/3] cvsimport: miscellaneous packed-ref fixes
From: Jeff King @ 2007-11-28 18:56 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Emanuele Giaquinta, git
In-Reply-To: <20071128185504.GA11236@coredump.intra.peff.net>

These were found with a grep for '$git_dir'; they all
replace a direct access of "$git_dir/refs/..." with a call
to git-show-ref or git-update-ref.

Signed-off-by: Jeff King <peff@peff.net>
---
 git-cvsimport.perl |   20 +++++++++-----------
 1 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index b852f2f..1b5f187 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -864,29 +864,27 @@ while (<CVS>) {
 				print STDERR "Branch $branch erroneously stems from itself -- changed ancestor to $opt_o\n";
 				$ancestor = $opt_o;
 			}
-			if (-f "$git_dir/$remote/$branch") {
+			if (defined get_headref("$remote/$branch")) {
 				print STDERR "Branch $branch already exists!\n";
 				$state=11;
 				next;
 			}
-			unless (open(H,"$git_dir/$remote/$ancestor")) {
+			my $id = get_headref("$remote/$ancestor");
+			if (!$id) {
 				print STDERR "Branch $ancestor does not exist!\n";
 				$ignorebranch{$branch} = 1;
 				$state=11;
 				next;
 			}
-			chomp(my $id = <H>);
-			close(H);
-			unless (open(H,"> $git_dir/$remote/$branch")) {
-				print STDERR "Could not create branch $branch: $!\n";
+
+			system(qw(git update-ref -m cvsimport),
+				"$remote/$branch", $id);
+			if($? != 0) {
+				print STDERR "Could not create branch $branch\n";
 				$ignorebranch{$branch} = 1;
 				$state=11;
 				next;
 			}
-			print H "$id\n"
-				or die "Could not write branch $branch: $!";
-			close(H)
-				or die "Could not write branch $branch: $!";
 		}
 		$last_branch = $branch if $branch ne $last_branch;
 		$state = 9;
@@ -998,7 +996,7 @@ if ($orig_branch) {
 	$orig_branch = "master";
 	print "DONE; creating $orig_branch branch\n" if $opt_v;
 	system("git-update-ref", "refs/heads/master", "$remote/$opt_o")
-		unless -f "$git_dir/refs/heads/master";
+		defined get_headref('refs/heads/master');
 	system("git-symbolic-ref", "$remote/HEAD", "$remote/$opt_o")
 		if ($opt_r && $opt_o ne 'HEAD');
 	system('git-update-ref', 'HEAD', "$orig_branch");
-- 
1.5.3.6.2039.g0495

^ permalink raw reply related

* Re: [PATCH] Allow update hooks to update refs on their own
From: Junio C Hamano @ 2007-11-28 19:00 UTC (permalink / raw)
  To: Jeff King; +Cc: Daniel Barkalow, Steven Grimm, git
In-Reply-To: <20071128161033.GA20308@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

>> Couldn't you do this with a status message? ("ok <refname> changed by 
>> hook" or something.)
>
> Having just touched this code, I believe the answer is yes. receive-pack
> has always sent just "ok <refname>\n", so we could start interpreting
> anything after the <refname> bit freely...

More importantly, no send-pack choked on "ok <refname>" followed by a SP
(old ones just checked "ok" and nothing else, the current one NULs out
the SP and lets you use the rest as a message), so such a change to
receive-pack does not have to break older send-pack.

^ permalink raw reply

* Re: [PATCH] Replace instances of export VAR=VAL with VAR=VAL; export VAR
From: Johannes Schindelin @ 2007-11-28 19:03 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Johannes Sixt, Wincent Colaiuta, Benoit Sigoure, Git Mailing List
In-Reply-To: <7vr6ia5hoi.fsf@gitster.siamese.dyndns.org>

Hi,

On Wed, 28 Nov 2007, Junio C Hamano wrote:

> Johannes Sixt <j.sixt@viscovery.net> writes:
> 
> > Johannes Schindelin schrieb:
> > ...
> >>> Recently there was a report that \n in the substitution side of s/// is not
> >>> supported by all seds :-(
> >>
> >> Okay, how about replacing the line with
> >>
> >> +			s/.*/GIT_'$uid'_NAME='\''&'\''\
> >> +export GIT_'$uid'_NAME/p
> >>
> >> Hmm?  (It works here.)
> >
> > This looks good. The other case I'm refering to was also solved in this way.
> 
> That looks ugly to me.
> 
> Is there a particular reason to force linebreak when a semicolon would
> do?

D'oh.  Of course.  You want me to resend?

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH/RFC] "color.diff = true" is not "always" anymore.
From: Jeff King @ 2007-11-28 19:04 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git list
In-Reply-To: <7vd4tuakzj.fsf_-_@gitster.siamese.dyndns.org>

On Tue, Nov 27, 2007 at 11:26:56PM -0800, Junio C Hamano wrote:

> Too many people got burned by setting color.diff and color.status to
> true when they really should have set it to "auto".
> 
> This makes only "always" to do the unconditional colorization, and
> change the meaning of "true" to the same as "auto": colorize only when
> we are talking to a terminal.

I think this is a good change. However, there needs to be a matching
change for all scripts which read the color.* variables (git-svn is the
only one now, I think, but Dan's git-add--interactive patch does the
same thing).

It would be nice to have a "git config --colorbool" option, but it has
the unfortunate problem that the stdout of "git config" is piped back to
the caller, so the isatty check is meaningless (and the "pager in use"
is similarly tricky). Perhaps it should go in Git.pm, so it at least
only needs to be written once.

-Peff

^ permalink raw reply

* Re: [PATCH 2/3] cvsimport: use show-ref to support packed refs
From: Johannes Schindelin @ 2007-11-28 19:16 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, Emanuele Giaquinta, git
In-Reply-To: <20071128185611.GB11320@coredump.intra.peff.net>

Hi,

On Wed, 28 Nov 2007, Jeff King wrote:

> diff --git a/git-cvsimport.perl b/git-cvsimport.perl
> index efa6a0c..b852f2f 100755
> --- a/git-cvsimport.perl
> +++ b/git-cvsimport.perl
> @@ -527,18 +527,12 @@ sub is_sha1 {
>  	return $s =~ /^[a-f0-9]{40}$/;
>  }
>  
> -sub get_headref ($$) {
> -    my $name    = shift;
> -    my $git_dir = shift;
> -
> -    my $f = "$git_dir/$remote/$name";
> -    if (open(my $fh, $f)) {
> -	    chomp(my $r = <$fh>);
> -	    is_sha1($r) or die "Cannot get head id for $name ($r): $!";
> -	    return $r;
> -    }
> -    die "unable to open $f: $!" unless $! == POSIX::ENOENT;
> -    return undef;
> +sub get_headref ($) {
> +	my $name = shift;
> +	my $r = `git show-ref -s '$name'`;
> +	return undef unless $? == 0;
> +	chomp $r;
> +	return $r;
>  }

Where has $remote gone?

Ciao,
Dscho

^ permalink raw reply

* Re: StGit hooks
From: Junio C Hamano @ 2007-11-28 19:31 UTC (permalink / raw)
  To: Karl Hasselström; +Cc: Andreas Ericsson, Jon Smirl, Git Mailing List
In-Reply-To: <20071128172152.GB21310@diana.vm.bytemark.co.uk>

Karl Hasselström <kha@treskal.com> writes:

> On 2007-11-28 18:06:36 +0100, Andreas Ericsson wrote:
>
>> True, but there are ways of figuring out which branch you're on,
>> using the arguments passed to the hook.
>>
>> The rebase hook (which is where this discussion started) gets the
>> branches passed to "git rebase" as arguments. Figuring out if either
>> of those branches are actually under stgit control shouldn't be
>> overly tricky for one so familiar with stgit as yourself.
>
> No, that part is trivial. The hard part is not messing up the user's
> existing hook scripts.
>
> If I do go ahead and do this, I have a feeling I'll be beefing up
> git's hook dispatch mechanism first -- if I can get it past Junio,
> obviously. :-)

I do not see a fundamental problem in a scheme like this:

 * when running a hook $foo, if .git/hooks/$foo.d/. does not exist, we
   operate as we have always done.

 * if .git/hooks/$foo.d/ exists, we readdir(3) it and run the hooks found
   in the order their names sort bytewise (similar to /etc/rc$n.d/).

   - when the purpose of the $foo hook is to return a bool to refuse an
     operation, stop at the first failure;

   - when the purpose of the $foo hook is to cause effect, run them all
     in series.

 * When .git/hooks/$foo.d/ exists, there will always be "050-simple" in
   that directory.  It checks if .git/hooks/$foo exists and sources it
   if it does.  A custom hook that wants to run before or after it can
   be named NNN-mine in .git/hooks/$foo.d/, where NNN sorts earlier or
   later than 050 to define the execution order.

We probably would want to reserve a special exit code for the hooks run
for their effects so that they can signal "I do not want you to run the
remainder".

^ permalink raw reply

* [PATCH] Allow update hooks to update refs on their own
From: Steven Grimm @ 2007-11-28 19:41 UTC (permalink / raw)
  To: git
In-Reply-To: <7vmysy5h5k.fsf@gitster.siamese.dyndns.org>

This is useful in cases where a hook needs to modify an incoming commit
in some way, e.g., adding an annotation to the commit message, noting
the location of output from a profiling tool, or committing to an svn
repository using git-svn.

recieve-pack will now send the post-update SHA1 back to send-pack. If
it's different than the one that was pushed, git-push won't do the
fake update of the local tracking ref and will instead inform the user
that the tracking ref needs to be fetched.

Signed-off-by: Steven Grimm <koreth@midwinter.com>
---

	The protocol was much easier to tweak than I expected. It
	seems like a reasonable extension to always send back the
	resulting SHA1; won't stop other people from adding more
	information later.

	It would IMO be better still if git-push were to fetch
	automatically here rather than just printing a message, but
	I'm not sure if that would smack too much of automagic behavior
	to people. Comments welcome.

 Documentation/git-receive-pack.txt |    8 +++-
 receive-pack.c                     |   72 +++++++++++++++++++++++-------------
 remote.c                           |    2 +-
 remote.h                           |    2 +
 send-pack.c                        |   64 +++++++++++++++++++++++++++-----
 5 files changed, 109 insertions(+), 39 deletions(-)

diff --git a/Documentation/git-receive-pack.txt b/Documentation/git-receive-pack.txt
index 2633d94..115ae97 100644
--- a/Documentation/git-receive-pack.txt
+++ b/Documentation/git-receive-pack.txt
@@ -74,8 +74,12 @@ Note that the hook is called before the refname is updated,
 so either sha1-old is 0\{40} (meaning there is no such ref yet),
 or it should match what is recorded in refname.
 
-The hook should exit with non-zero status if it wants to disallow
-updating the named ref.  Otherwise it should exit with zero.
+The hook may optionally choose to update the ref on its own, e.g.,
+if it needs to modify incoming revisions in some way. If it updates
+the ref, it should exit with a status of 100.  The hook should exit
+with a status between 1 and 99 if it wants to disallow updating the
+named ref.  Otherwise it should exit with zero, and the ref will be
+updated automatically.
 
 Successful execution (a zero exit status) of this hook does not
 ensure the ref will actually be updated, it is only a prerequisite.
diff --git a/receive-pack.c b/receive-pack.c
index 38e35c0..d07dd6d 100644
--- a/receive-pack.c
+++ b/receive-pack.c
@@ -18,6 +18,9 @@ static int report_status;
 static char capabilities[] = " report-status delete-refs ";
 static int capabilities_sent;
 
+/* Update hook exit code: hook has updated ref on its own */
+#define EXIT_CODE_REF_UPDATED 100
+
 static int receive_pack_config(const char *var, const char *value)
 {
 	if (strcmp(var, "receive.denynonfastforwards") == 0) {
@@ -70,8 +73,11 @@ static struct command *commands;
 static const char pre_receive_hook[] = "hooks/pre-receive";
 static const char post_receive_hook[] = "hooks/post-receive";
 
-static int hook_status(int code, const char *hook_name)
+static int hook_status(int code, const char *hook_name, int ok_start)
 {
+	if (ok_start && -code >= ok_start)
+		return -code;
+
 	switch (code) {
 	case 0:
 		return 0;
@@ -121,7 +127,7 @@ static int run_hook(const char *hook_name)
 
 	code = start_command(&proc);
 	if (code)
-		return hook_status(code, hook_name);
+		return hook_status(code, hook_name, 0);
 	for (cmd = commands; cmd; cmd = cmd->next) {
 		if (!cmd->error_string) {
 			size_t n = snprintf(buf, sizeof(buf), "%s %s %s\n",
@@ -132,7 +138,7 @@ static int run_hook(const char *hook_name)
 				break;
 		}
 	}
-	return hook_status(finish_command(&proc), hook_name);
+	return hook_status(finish_command(&proc), hook_name, 0);
 }
 
 static int run_update_hook(struct command *cmd)
@@ -155,7 +161,8 @@ static int run_update_hook(struct command *cmd)
 	proc.no_stdin = 1;
 	proc.stdout_to_stderr = 1;
 
-	return hook_status(run_command(&proc), update_hook);
+	return hook_status(run_command(&proc), update_hook,
+			   EXIT_CODE_REF_UPDATED);
 }
 
 static const char *update(struct command *cmd)
@@ -194,32 +201,45 @@ static const char *update(struct command *cmd)
 			return "non-fast forward";
 		}
 	}
-	if (run_update_hook(cmd)) {
-		error("hook declined to update %s", name);
-		return "hook declined";
-	}
-
-	if (is_null_sha1(new_sha1)) {
-		if (delete_ref(name, old_sha1)) {
-			error("failed to delete %s", name);
-			return "failed to delete";
+	switch (run_update_hook(cmd)) {
+	case 0:
+		if (is_null_sha1(new_sha1)) {
+			if (delete_ref(name, old_sha1)) {
+				error("failed to delete %s", name);
+				return "failed to delete";
+			}
+			fprintf(stderr, "%s: %s -> deleted\n", name,
+				sha1_to_hex(old_sha1));
 		}
-		fprintf(stderr, "%s: %s -> deleted\n", name,
-			sha1_to_hex(old_sha1));
-		return NULL; /* good */
-	}
-	else {
-		lock = lock_any_ref_for_update(name, old_sha1, 0);
-		if (!lock) {
-			error("failed to lock %s", name);
-			return "failed to lock";
+		else {
+			lock = lock_any_ref_for_update(name, old_sha1, 0);
+			if (!lock) {
+				error("failed to lock %s", name);
+				return "failed to lock";
+			}
+			if (write_ref_sha1(lock, new_sha1, "push")) {
+				return "failed to write"; /* error() already called */
+			}
+			fprintf(stderr, "%s: %s -> %s\n", name,
+				sha1_to_hex(old_sha1), sha1_to_hex(new_sha1));
 		}
-		if (write_ref_sha1(lock, new_sha1, "push")) {
-			return "failed to write"; /* error() already called */
+		return NULL; /* good */
+
+	case EXIT_CODE_REF_UPDATED:
+		/* hook has taken care of updating ref, which means it
+		   might be a different revision than we think. */
+		if (! resolve_ref(name, new_sha1, 1, NULL)) {
+			error("can't resolve ref %s after hook updated it",
+				name);
+			return "ref not resolvable";
 		}
 		fprintf(stderr, "%s: %s -> %s\n", name,
 			sha1_to_hex(old_sha1), sha1_to_hex(new_sha1));
 		return NULL; /* good */
+
+	default:
+		error("hook declined to update %s", name);
+		return "hook declined";
 	}
 }
 
@@ -419,8 +439,8 @@ static void report(const char *unpack_status)
 		     unpack_status ? unpack_status : "ok");
 	for (cmd = commands; cmd; cmd = cmd->next) {
 		if (!cmd->error_string)
-			packet_write(1, "ok %s\n",
-				     cmd->ref_name);
+			packet_write(1, "ok %s %s\n",
+				     cmd->ref_name, sha1_to_hex(cmd->new_sha1));
 		else
 			packet_write(1, "ng %s %s\n",
 				     cmd->ref_name, cmd->error_string);
diff --git a/remote.c b/remote.c
index bec2ba1..07558c1 100644
--- a/remote.c
+++ b/remote.c
@@ -684,7 +684,7 @@ static int match_explicit_refs(struct ref *src, struct ref *dst,
 	return -errs;
 }
 
-static struct ref *find_ref_by_name(struct ref *list, const char *name)
+struct ref *find_ref_by_name(struct ref *list, const char *name)
 {
 	for ( ; list; list = list->next)
 		if (!strcmp(list->name, name))
diff --git a/remote.h b/remote.h
index 878b4ec..e822f3c 100644
--- a/remote.h
+++ b/remote.h
@@ -56,6 +56,8 @@ void ref_remove_duplicates(struct ref *ref_map);
 
 struct refspec *parse_ref_spec(int nr_refspec, const char **refspec);
 
+struct ref *find_ref_by_name(struct ref *list, const char *name);
+
 int match_refs(struct ref *src, struct ref *dst, struct ref ***dst_tail,
 	       int nr_refspec, char **refspec, int all);
 
diff --git a/send-pack.c b/send-pack.c
index b74fd45..bf564ae 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -147,9 +147,31 @@ static void get_local_heads(void)
 	for_each_ref(one_local_ref, NULL);
 }
 
-static int receive_status(int in)
+/* Verifies that a remote ref matches the revision we pushed. */
+static void verify_tracking_ref(const char *refname, const char *newsha1_hex, struct ref *remote_refs)
+{
+	struct ref *ref;
+	unsigned char newsha1[20];
+	if (get_sha1_hex(newsha1_hex, newsha1)) {
+		fprintf(stderr, "protocol error: bad sha1 %s\n", newsha1_hex);
+		return;
+	}
+	ref = find_ref_by_name(remote_refs, refname);
+	if (ref != NULL) {
+		if (hashcmp(ref->new_sha1, newsha1)) {
+			hashcpy(ref->new_sha1, newsha1);
+			ref->force = 1;
+		}
+		else {
+			ref->force = 0;
+		}
+	}
+}
+
+static int receive_status(int in, struct ref *remote_refs)
 {
 	char line[1000];
+	char *sha1;
 	int ret = 0;
 	int len = packet_read_line(in, line, sizeof(line));
 	if (len < 10 || memcmp(line, "unpack ", 7)) {
@@ -170,8 +192,22 @@ static int receive_status(int in)
 			ret = -1;
 			break;
 		}
-		if (!memcmp(line, "ok", 2))
+		if (!memcmp(line, "ok", 2)) {
+			/* If the result looks like "ok refname sha1", we can
+			 * compare the actual SHA1 for the remote ref with the
+			 * one we pushed; if they differ then the remote
+			 * may have rewritten our commits and we will need to
+			 * do a real fetch rather than just updating the
+			 * tracking refs.
+			 */
+			if (line[2] == ' ' &&
+			    (sha1 = strchr(line+3, ' '))) {
+				/* null-terminate refname */
+				*sha1++ = '\0';
+				verify_tracking_ref(line+3, sha1, remote_refs);
+			}
 			continue;
+		}
 		fputs(line, stderr);
 		ret = -1;
 	}
@@ -196,13 +232,21 @@ static void update_tracking_ref(struct remote *remote, struct ref *ref)
 		return;
 
 	if (!remote_find_tracking(remote, &rs)) {
-		fprintf(stderr, "updating local tracking ref '%s'\n", rs.dst);
-		if (is_null_sha1(ref->peer_ref->new_sha1)) {
-			if (delete_ref(rs.dst, NULL))
-				error("Failed to delete");
-		} else
-			update_ref("update by push", rs.dst,
-					ref->new_sha1, NULL, 0, 0);
+		if (ref->force) {
+			fprintf(stderr,
+				"local tracking ref '%s' needs to be fetched\n",
+				rs.dst);
+		}
+		else {
+			fprintf(stderr, "updating local tracking ref '%s'\n",
+				rs.dst);
+			if (is_null_sha1(ref->peer_ref->new_sha1)) {
+				if (delete_ref(rs.dst, NULL))
+					error("Failed to delete");
+			} else
+				update_ref("update by push", rs.dst,
+						ref->new_sha1, NULL, 0, 0);
+		}
 		free(rs.dst);
 	}
 }
@@ -343,7 +387,7 @@ static int send_pack(int in, int out, struct remote *remote, int nr_refspec, cha
 	close(out);
 
 	if (expect_status_report) {
-		if (receive_status(in))
+		if (receive_status(in, remote_refs))
 			ret = -4;
 	}
 
-- 
1.5.3.6.862.gb50ba-dirty

^ permalink raw reply related

* Re: [PATCH 2/3] cvsimport: use show-ref to support packed refs
From: Jeff King @ 2007-11-28 19:44 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, Emanuele Giaquinta, git
In-Reply-To: <Pine.LNX.4.64.0711281916140.27959@racer.site>

On Wed, Nov 28, 2007 at 07:16:36PM +0000, Johannes Schindelin wrote:

> > -sub get_headref ($$) {
> > -    my $name    = shift;
> > -    my $git_dir = shift;
> > -
> > -    my $f = "$git_dir/$remote/$name";
> > -    if (open(my $fh, $f)) {
> > -	    chomp(my $r = <$fh>);
> > -	    is_sha1($r) or die "Cannot get head id for $name ($r): $!";
> > -	    return $r;
> > -    }
> > -    die "unable to open $f: $!" unless $! == POSIX::ENOENT;
> > -    return undef;
> > +sub get_headref ($) {
> > +	my $name = shift;
> > +	my $r = `git show-ref -s '$name'`;
> > +	return undef unless $? == 0;
> > +	chomp $r;
> > +	return $r;
> >  }
> 
> Where has $remote gone?

Gah, thank you. Obviously I deleted it without looking when I removed
the now-unnecessary $git_dir. However, let me put a curse on whoever is
responsible for randomly using "$remote" as a global in this function,
when all of the other parameters (including the
obvious-candidate-for-a-global $git_dir) are passed in.

Since get_headref is useful in contexts where "$remote" is not always
prepended (see patch 3/3), I think the best solution is:

diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index 1b5f187..bbf9799 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -692,7 +692,8 @@ my (@old,@new,@skipped,%ignorebranch);
 $ignorebranch{'#CVSPS_NO_BRANCH'} = 1;
 
 sub commit {
-	if ($branch eq $opt_o && !$index{branch} && !get_headref($branch)) {
+	if ($branch eq $opt_o && !$index{branch} &&
+		!get_headref("$remote/$branch")) {
 	    # looks like an initial commit
 	    # use the index primed by git-init
 	    $ENV{GIT_INDEX_FILE} = "$git_dir/index";
@@ -716,7 +717,7 @@ sub commit {
 	update_index(@old, @new);
 	@old = @new = ();
 	my $tree = write_tree();
-	my $parent = get_headref($last_branch);
+	my $parent = get_headref("$remote/$last_branch");
 	print "Parent ID " . ($parent ? $parent : "(empty)") . "\n" if $opt_v;
 
 	my @commit_args;
@@ -727,7 +728,7 @@ sub commit {
 	foreach my $rx (@mergerx) {
 		next unless $logmsg =~ $rx && $1;
 		my $mparent = $1 eq 'HEAD' ? $opt_o : $1;
-		if (my $sha1 = get_headref($mparent)) {
+		if (my $sha1 = get_headref("$remote/$mparent")) {
 			push @commit_args, '-p', $mparent;
 			print "Merge parent branch: $mparent\n" if $opt_v;
 		}

^ permalink raw reply related

* Re: [PATCH] Allow update hooks to update refs on their own
From: Jeff King @ 2007-11-28 19:49 UTC (permalink / raw)
  To: Steven Grimm; +Cc: git
In-Reply-To: <20071128194159.GA25977@midwinter.com>

On Wed, Nov 28, 2007 at 11:41:59AM -0800, Steven Grimm wrote:

> recieve-pack will now send the post-update SHA1 back to send-pack. If
> it's different than the one that was pushed, git-push won't do the
> fake update of the local tracking ref and will instead inform the user
> that the tracking ref needs to be fetched.

Hrm, this is going to have nasty conflicts with 'next', which already
does the remote ref matching. I think the best way to implement this
would probably be on top of the jk/send-pack topic in next, and add a
new REF_STATUS_REMOTE_CHANGED status type.

-Peff

^ permalink raw reply

* Re: can't commit files that have been git add'ed because "fatal: you need to resolve your current index first"
From: Junio C Hamano @ 2007-11-28 20:08 UTC (permalink / raw)
  To: Bill Priest; +Cc: git
In-Reply-To: <820179.5481.qm@web55002.mail.re4.yahoo.com>

Bill Priest <priestwilliaml@yahoo.com> writes:

>   I merged from one branch to another and had lots of
> conflicts.  I've resolved a set of files from the
> conflicts (in a directory) and did a git-add on this
> set of files.  I wasn't able to commit these files. 
> On IRC I was told that all files must be resolved
> before I can commit any of them.  This seems pretty
> limiting.

You have two parallel development lines that lead to A and B and trying
to come up with a "merge" M:

         o---o---A
        /         \
    ---o---o---B---M

What property should the merge commit "M" have?  It must keep the
changes made on the upper line to make it better than B, and it must
keep the changes made on the lower line to make it better than A.

Let's say both upper and lower lines of development touched files frotz
and xyzzy in overlapping, different ways.  You try to merge A while you
are at B and see conflicts in these two files.

Let's say you resolved frotz; the contents in frotz is in a desired
shape, i.e you have the changes you want from the line led to A and the
other line led to B.  But you haven't resolved xyzzy yet.

You seem to want to make this half-resolved state as a commit.  First
question: what contents would you want to commit for xyzzy?

If you commit the contents from B (because you started from it), then it
should not be recorded as a proper merge.  If you did so, merging that
back to A would obliterate all the work done up to A to file xyzzy:

         o---o---A...X
        /         \ .
    ---o---o---B---*
 
because merge base of A and * (I am marking it differently because such
an incomplete merge should not be made) is A, so the result (X) will be
the half-merge * itself (fast forward).  That's not a merge as it is not
better than A -- you discarded improvements made to xyzzy by people who
built up to A.

This is inherent to what a merge is.  With proper understanding of what
a merge is, you would not feel this limiting at all.

Having said that, I think something like the following _could_ be done,
although I do not know if it makes much sense.

(1) Try merge, see it conflict, resolve only a part of it, and record
    the result as "WIP to merge in A".  Do not record it as a merge, as
    it is not.  diff between B and M will contain a squash merge of A on
    top of B minus the changes to the path xyzzy.

         o---o---A
        /         
    ---o---o---B---M

(2) Fix up the conflicts in xyzzy to resolve the conflicts fully, and
    record the result as "Final merge of A into B".  This should be
    recorded as a merge, as the result is "keeping changes done on both
    branches to come up with something better than either of them."

         o---o---A---,
        /             \
    ---o---o---B---M---N

If you look at the topology a bit differently, the above actually makes
some sense:

         .---o---o---A
        /             \
    ---o---o---B---M---N

That is, instead of merging A into B, you made "preparatory changes" to
branch B in order to make it easier to merge A.  That's what the commit
M is about.

Then you merge A on top of M.  In reality, because the difference
between B to M contains most of the squash merge of A into B, such a
merge N will contain many accidental clean merges.  But in git,
accidental clean merges are not a problem (people can apply the same
patch to their trees and later their branches can be merged).

But "git commit" after a conflicted merge will always create a merge,
and there is no canned option to do multi-step merge like the above.

You can still do that by hand, by doing something like:

	$ git merge --squash A
        $ resolve only partly
        $ git commit -m 'Prepare to merge A'
        $ git reset --hard
        $ git merge A
	$ resolve the rest
        $ git commit -m 'Fully merged A'

For such a multi-step merge to make sense, the change between B---M
should make sense by itself for people who have to read such a history
later.  Such a half-a-squash-merge may probably not make sense by itself
in most cases, so I suspect the above workflow would not be useful in
general.

^ permalink raw reply

* Re: [PATCH 2/2] Link porcelain commands from gitexecdir to bindir
From: Junio C Hamano @ 2007-11-28 20:11 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <20071128162629.GA5453@laptop>

Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:

>  The porcelain list needs eyeballs.

The source of porcelain list needs consolidation first.  See a patch
from me in the last couple of days to contrib/completion and
generate-cmdlist.sh for ideas, for example.

^ permalink raw reply

* Re: [PATCH] Allow update hooks to update refs on their own
From: Steven Grimm @ 2007-11-28 20:16 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20071128194919.GC11396@coredump.intra.peff.net>


On Nov 28, 2007, at 11:49 AM, Jeff King wrote:

> Hrm, this is going to have nasty conflicts with 'next', which already
> does the remote ref matching. I think the best way to implement this
> would probably be on top of the jk/send-pack topic in next, and add a
> new REF_STATUS_REMOTE_CHANGED status type.

Ah, yes, I see what you mean. Okay, please ignore my latest patch -- I  
will redo it on top of "next" later today/tonight.

Well, actually, I would still like opinions on one thing: What do  
people think of having git-push do a fetch if the remote side changes  
a ref to point to a revision that doesn't exist locally? Is there a  
situation where you'd ever want to *not* do that?

-Steve

^ permalink raw reply

* Re: [PATCH] Allow update hooks to update refs on their own
From: Jeff King @ 2007-11-28 20:22 UTC (permalink / raw)
  To: Steven Grimm; +Cc: git
In-Reply-To: <C1321BD5-8F6B-47F9-9BDB-C2BF819D6F17@midwinter.com>

On Wed, Nov 28, 2007 at 12:16:27PM -0800, Steven Grimm wrote:

> Well, actually, I would still like opinions on one thing: What do people 
> think of having git-push do a fetch if the remote side changes a ref to 
> point to a revision that doesn't exist locally? Is there a situation where 
> you'd ever want to *not* do that?

It can be slow, since you have to make another connection to the server,
so clearly it should only be done when you detect an update (which I
think is what you're proposing).

A raw "git-fetch" might pull a lot of extra cruft that you didn't want
to get right now. So if you did do it, I think it would make sense to
construct a set of refspecs that match only the ones which need pulling
(i.e., in update_tracking_ref, rather than doing the update, construct a
refspec of "local:tracking", and then hand all such refspecs to
git-fetch).

-Peff

^ permalink raw reply

* Re: [PATCH v2] Teach 'git pull' about --rebase
From: Steven Grimm @ 2007-11-28 20:35 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, Linus Torvalds, git
In-Reply-To: <Pine.LNX.4.64.0711281307420.27959@racer.site>

On Nov 28, 2007, at 5:11 AM, Johannes Schindelin wrote:
> As a convenience, you can set the default behavior for a branch by
> defining the config variable branch.<name>.rebase, which is
> interpreted as a bool.  This setting can be overridden on the command
> line by --rebase and --no-rebase.

I wonder if this shouldn't be branch.<name>.pulltype or something like  
that, so we can represent more than just "rebase or not." Values could  
be "rebase", "merge" (the default) and maybe even "manual" to specify  
that git-pull should neither merge nor rebase a particular branch even  
if it matches a wildcard refspec.

Not too sure about that last suggestion but it seems like there might  
be other settings than "rebase" and "merge" in the future even if  
that's not one of them.

-Steve

^ permalink raw reply

* Re: [PATCH 2/3] cvsimport: use show-ref to support packed refs
From: Johannes Schindelin @ 2007-11-28 20:37 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, Emanuele Giaquinta, git
In-Reply-To: <20071128194423.GB11396@coredump.intra.peff.net>

Hi,

On Wed, 28 Nov 2007, Jeff King wrote:

> Since get_headref is useful in contexts where "$remote" is not always
> prepended (see patch 3/3), I think the best solution is:
>
> [PATCH prefixing the argument to get_headref() with "$remote/"]

Yes, I think so, too.

Thanks,
Dscho

^ permalink raw reply

* Re: The 5th issue of the msysGit Herald
From: Johannes Gilger @ 2007-11-28 20:40 UTC (permalink / raw)
  To: Julian Phillips; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0711281632510.1706@kaos.quantumfyre.co.uk>

On 28/11/07 16:33, Julian Phillips wrote:
> Something like this perhaps?
>
> http://gitrss.q42.co.uk/

Hi Julian,

somehow the feed does not work for me at all. It is not displayed 
correctly in the firefox feed-preview or in google reader. Not correctly 
here meaning: no linebreaks.

Greetings,
Jojo

-- 
Johannes Gilger <heipei@hackvalue.de>
http://hackvalue.de/heipei/
GPG-Key: 0x42F6DE81
GPG-Fingerprint: BB49 F967 775E BB52 3A81  882C 58EE B178 42F6 DE81

^ permalink raw reply

* Re: [PATCH v2] Teach 'git pull' about --rebase
From: Johannes Schindelin @ 2007-11-28 20:40 UTC (permalink / raw)
  To: Steven Grimm; +Cc: Junio C Hamano, Linus Torvalds, git
In-Reply-To: <27E5EF3C-19EF-441C-BB12-0F5B29BEAEDB@midwinter.com>

Hi,

On Wed, 28 Nov 2007, Steven Grimm wrote:

> On Nov 28, 2007, at 5:11 AM, Johannes Schindelin wrote:
> > As a convenience, you can set the default behavior for a branch by 
> > defining the config variable branch.<name>.rebase, which is 
> > interpreted as a bool.  This setting can be overridden on the command 
> > line by --rebase and --no-rebase.
> 
> I wonder if this shouldn't be branch.<name>.pulltype or something like 
> that, so we can represent more than just "rebase or not." Values could 
> be "rebase", "merge" (the default) and maybe even "manual" to specify 
> that git-pull should neither merge nor rebase a particular branch even 
> if it matches a wildcard refspec.

I am not convinced that this is a good thing... We already have 
branch.<name>.mergeOptions for proper merges, and I want to make clear 
that this is about rebase, and not about merge.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH v2] Teach 'git pull' about --rebase
From: Lars Hjemli @ 2007-11-28 21:10 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Steven Grimm, Junio C Hamano, Linus Torvalds, git
In-Reply-To: <Pine.LNX.4.64.0711282039430.27959@racer.site>

On 11/28/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> On Wed, 28 Nov 2007, Steven Grimm wrote:
> > I wonder if this shouldn't be branch.<name>.pulltype or something like
> > that, so we can represent more than just "rebase or not." Values could
> > be "rebase", "merge" (the default) and maybe even "manual" to specify
> > that git-pull should neither merge nor rebase a particular branch even
> > if it matches a wildcard refspec.
>
> I am not convinced that this is a good thing... We already have
> branch.<name>.mergeOptions for proper merges, and I want to make clear
> that this is about rebase, and not about merge.

Maybe branch.<name>.pullOptions ?

--
larsh

^ permalink raw reply

* Re: [PATCH] Do not generate full commit log message if it not going to be used
From: Alex Riesen @ 2007-11-28 21:10 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Git Mailing List, Johannes Sixt, Junio C Hamano,
	Kristian Høgsberg
In-Reply-To: <Pine.LNX.4.64.0711281211130.27959@racer.site>

Johannes Schindelin, Wed, Nov 28, 2007 13:18:10 +0100:
> On Tue, 27 Nov 2007, Alex Riesen wrote:
> 
> > Could not stop myself. Hopefully didn't beat anyone to it :)
> > Almost all code shamelessly stolen from builtin-diff-index.c.
> 
> Then I have to wonder if it would not be a better idea to refactor the 
> code, so that other people do not have to steal the code again, but are 
> able to reuse it ;-)

Not sure it will be worth the effort. It is really short.

OTOH, I missed a diff-index interface where I could pass a resolved
sha1 (the u8 array returned by get_sha1) and index state. Something
like "int diff_tree_index(const unsigned char *sha1, struct rev_info *)".

Tempting, but I have only one use case for it. I don't actually know
Git's code that well...

> > Preprocessor trickery in DIFF_OPT_* macros is disgusting, it breaks Vim 
> > word completion and trying to use many flags in one expression looks 
> > just ugly.
> 
> How does it break Vim word completion?  And why should something like
> 
> 		DIFF_OPT_SET(&rev.diffopt, QUIET | EXIT_WITH_STATUS);
> 
> look ugly?  I find it highly readable.

Oh, this would look ok. It just wont compile: DIFF_OPT_SET prepends
second argument with DIFF_OPT_:

#define DIFF_OPT_TST(opts, flag)    ((opts)->flags & DIFF_OPT_##flag)
#define DIFF_OPT_SET(opts, flag)    ((opts)->flags |= DIFF_OPT_##flag)
#define DIFF_OPT_CLR(opts, flag)    ((opts)->flags &= ~DIFF_OPT_##flag)

> > +	if (no_edit) {
> > +		static const char *argv[] = { NULL, "HEAD", NULL };
> > +		struct rev_info rev;
> > +		unsigned char sha1[40];
> > +		int is_initial;
> > +
> > +		fclose(fp);
> > +
> > +		if (!active_nr && read_cache() < 0)
> > +			die("Cannot read index");
> > +
> > +		if (get_sha1("HEAD", sha1) != 0)
> > +			return !!active_nr;
> 
> Don't want to be anal here, but are there possibly reasons (read "possible 
> errors") other than an empty repo where this triggers?

Definitely. I just don't know. OTOH, I can only return "committable"
or "not committable". If I return "commitable" for a broken repo,
it should fail elsewhere. If I return "not commitable" git-commit
shall finish telling user there is nothing to commit, which is just
wrong.

> > +
> > +		init_revisions(&rev, "");
> > +		rev.abbrev = 0;
> > +		(void)setup_revisions(2, argv, &rev, NULL);
> 
> (void)?

Yep. Sorry, I just got carried away by recent tendency (here at work)
to shut up PC-Lint (but please don't ask).

> Besides, would this not be more elegant as
> 
> 		setup_revisions(0, NULL, &rev, "HEAD");

Hmm... And I was so puzzled as to what that "def" argument could
possibly mean... Still am, in fact. But it works.

> > +		(void)run_diff_index(&rev, 1 /* cached */);
> 
> (void)?

I'll remove them and resubmit. Stupid custom.

> Other than that (including my remark about refactoring that piece of 
> code), I like it.

Me too: I have *extensively* tested it today and a commit on the
2.6GHz/2Gb/SATA windows machine is almost as fast as on my linux
laptop now (Centrino/1.2GHz downclocked to 800MHz/384Mb/IDE).

^ permalink raw reply

* [PATCH] Do not generate full commit log message if it is not going to be used
From: Alex Riesen @ 2007-11-28 21:13 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Git Mailing List, Johannes Sixt, Junio C Hamano,
	Kristian Høgsberg
In-Reply-To: <20071128211059.GA3173@steel.home>

Like when it is already specified through -C, -F or -m to git-commit.

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---
Alex Riesen, Wed, Nov 28, 2007 22:10:59 +0100:
> Johannes Schindelin, Wed, Nov 28, 2007 13:18:10 +0100:
> > Besides, would this not be more elegant as
> > 
> > 		setup_revisions(0, NULL, &rev, "HEAD");
> 
> Hmm... And I was so puzzled as to what that "def" argument could
> possibly mean... Still am, in fact. But it works.
> 
> > > +		(void)run_diff_index(&rev, 1 /* cached */);
> > 
> > (void)?
> 
> I'll remove them and resubmit. Stupid custom.
> 

Here it goes.

 builtin-commit.c |   22 ++++++++++++++++++++++
 1 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/builtin-commit.c b/builtin-commit.c
index a35881e..1a9a256 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -367,6 +367,28 @@ static int prepare_log_message(const char *index_file, const char *prefix)
 
 	strbuf_release(&sb);
 
+	if (no_edit) {
+		struct rev_info rev;
+		unsigned char sha1[40];
+
+		fclose(fp);
+
+		if (!active_nr && read_cache() < 0)
+			die("Cannot read index");
+
+		if (get_sha1("HEAD", sha1) != 0)
+			return !!active_nr;
+
+		init_revisions(&rev, "");
+		rev.abbrev = 0;
+		setup_revisions(0, NULL, &rev, "HEAD");
+		DIFF_OPT_SET(&rev.diffopt, QUIET);
+		DIFF_OPT_SET(&rev.diffopt, EXIT_WITH_STATUS);
+		run_diff_index(&rev, 1 /* cached */);
+
+		return !!DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES);
+	}
+
 	if (in_merge && !no_edit)
 		fprintf(fp,
 			"#\n"
-- 
1.5.3.6.1014.g3543

^ permalink raw reply related

* Re: [PATCH] Do not generate full commit log message if it not going to be used
From: Johannes Schindelin @ 2007-11-28 21:43 UTC (permalink / raw)
  To: Alex Riesen
  Cc: Git Mailing List, Johannes Sixt, Junio C Hamano,
	Kristian Høgsberg
In-Reply-To: <20071128211059.GA3173@steel.home>

Hi,

On Wed, 28 Nov 2007, Alex Riesen wrote:

> Johannes Schindelin, Wed, Nov 28, 2007 13:18:10 +0100:
> > On Tue, 27 Nov 2007, Alex Riesen wrote:
> > 
> > > Could not stop myself. Hopefully didn't beat anyone to it :)
> > > Almost all code shamelessly stolen from builtin-diff-index.c.
> > 
> > Then I have to wonder if it would not be a better idea to refactor the 
> > code, so that other people do not have to steal the code again, but are 
> > able to reuse it ;-)
> 
> Not sure it will be worth the effort. It is really short.

Okay.

> > > Preprocessor trickery in DIFF_OPT_* macros is disgusting, it breaks Vim 
> > > word completion and trying to use many flags in one expression looks 
> > > just ugly.
> > 
> > How does it break Vim word completion?  And why should something like
> > 
> > 		DIFF_OPT_SET(&rev.diffopt, QUIET | EXIT_WITH_STATUS);
> > 
> > look ugly?  I find it highly readable.
> 
> Oh, this would look ok. It just wont compile: DIFF_OPT_SET prepends
> second argument with DIFF_OPT_:
> 
> #define DIFF_OPT_TST(opts, flag)    ((opts)->flags & DIFF_OPT_##flag)
> #define DIFF_OPT_SET(opts, flag)    ((opts)->flags |= DIFF_OPT_##flag)
> #define DIFF_OPT_CLR(opts, flag)    ((opts)->flags &= ~DIFF_OPT_##flag)

Ouch.  Sorry for suggesting it when I clearly had no clue.

> > > +	if (no_edit) {
> > > +		static const char *argv[] = { NULL, "HEAD", NULL };
> > > +		struct rev_info rev;
> > > +		unsigned char sha1[40];
> > > +		int is_initial;
> > > +
> > > +		fclose(fp);
> > > +
> > > +		if (!active_nr && read_cache() < 0)
> > > +			die("Cannot read index");
> > > +
> > > +		if (get_sha1("HEAD", sha1) != 0)
> > > +			return !!active_nr;
> > 
> > Don't want to be anal here, but are there possibly reasons (read "possible 
> > errors") other than an empty repo where this triggers?
> 
> Definitely. I just don't know. OTOH, I can only return "committable" or 
> "not committable".

I guess it is good enough.  Just wanted to point out that this can fail if 
.git/HEAD is not readable.

> > Besides, would this not be more elegant as
> > 
> > 		setup_revisions(0, NULL, &rev, "HEAD");
> 
> Hmm... And I was so puzzled as to what that "def" argument could
> possibly mean... Still am, in fact. But it works.

It is the mechanism which makes "git log" default to "git log HEAD".

> > Other than that (including my remark about refactoring that piece of 
> > code), I like it.
> 
> Me too: I have *extensively* tested it today and a commit on the
> 2.6GHz/2Gb/SATA windows machine is almost as fast as on my linux
> laptop now (Centrino/1.2GHz downclocked to 800MHz/384Mb/IDE).

Yes, I suspected something like this would happen.

Ciao,
Dscho

^ permalink raw reply

* Re: Rollback of git commands
From: Daniel Barkalow @ 2007-11-28 21:47 UTC (permalink / raw)
  To: Jon Smirl; +Cc: Karl Hasselström, Junio C Hamano, Git Mailing List
In-Reply-To: <9e4733910711280713n6b439866m55bea4824efd959@mail.gmail.com>

On Wed, 28 Nov 2007, Jon Smirl wrote:

> all my patches applied
> git rebase
> cursing.... I immediately knew what I had done
> update stg and install it
> stg repair
> four of my 15 patches tried to apply, I received messages that there
> were all empty
> most stg commands won't work, they complain that the commit references
> in the stg .git/* state are not correct.
> 
> I then proceed to manually attempt repair.

This sounds like the content of the applied patches got pulled into the 
non-stgit history of the branch it's working on, sort of like a stg commit 
except that stgit didn't know you'd done it. Then cleaning everything up 
from stgit's perspective caused all of those patches to become empty, 
since they were already applied in the base.

I think fundamental issue you're having is that stgit is implementing the 
functionality of quilt using git's engine, not providing a version control 
system for patch series, which is what you really want. I've actually been 
working on a design for a git builtin for the idea that the patch series 
is your work product, and you want to version control that (additionally, 
you want to use git's engine to help with working on the series and 
represent it).

Out of curiousity, are you using stgit as an integrator (with your work 
being keeping a collection of patches produced separately up-to-date) or 
as a patch developer (with your work being producing a state containing a 
single large new feature while maintaining this change as a series of 
self-contained patches)? I've been thinking primarily about the integrator 
task, in part because I've found it easy enough to do the developer task 
without anything other than current git. (That is, "git rebase -i" seems 
to work fine for making changes to a single logical patch series, all of 
whose patches are prepared locally and aren't independantly named in some 
particular fashion; the things that aren't handled are "I need to replace 
the pull of netdev.git with a new pull of netdev.git" or "I need to 
replace '[PATCH] fix-the-frobnozzle-gadget' with 
'[PATCH v2] fix-the-frobnozzle-gadget'.)

The developer assist I'd actually like to see is: "I've got a single 
commit on top of a series of commits on top of an upstream commit; I want 
to distribute the changes made in the final commit to points in the series 
where the code that gets replaced (or context that gets inserted into) in 
the final commit gets introduced, with interactive stuff for sticking 
other hunks into particular commits or into new commits at some point in 
the series." That is, I want to do my revision of a patch series on the 
final commit of the series, and then have these changes distributed to the 
appropriate points, rather than doing work on intermediate states (unless 
what I'm fixing is stub code that gets replaced again in a later patch).

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

* Re: [PATCH] Allow update hooks to update refs on their own
From: Junio C Hamano @ 2007-11-28 21:49 UTC (permalink / raw)
  To: Steven Grimm; +Cc: Jeff King, git
In-Reply-To: <20071128194919.GC11396@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> Hrm, this is going to have nasty conflicts with 'next', which already
> does the remote ref matching. I think the best way to implement this
> would probably be on top of the jk/send-pack topic in next, and add a
> new REF_STATUS_REMOTE_CHANGED status type.

I think Jeff is referring to sp/refspec-match (605b4978).

I still have doubts about having this in the update hook, as the hook is
about accepting or refusing and has never been about rewriting.

If the implementation of the svn hook were to check if you can rebase
cleanly in the update hook without actually rewriting the refs, and then
to perform the real update of the refs in post-receive or post-update
hook, that would feel much cleaner.  But the end result would be the
same as you rewrote the refs inside the update hook like your patch
does, so maybe I am worrying about conceptual cleanliness too much,
needlessly.

^ permalink raw reply

* Re: [PATCH v2] Teach 'git pull' about --rebase
From: Junio C Hamano @ 2007-11-28 21:55 UTC (permalink / raw)
  To: Lars Hjemli
  Cc: Johannes Schindelin, Steven Grimm, Junio C Hamano, Linus Torvalds,
	git
In-Reply-To: <8c5c35580711281310h8764a33pba48e65010abf859@mail.gmail.com>

"Lars Hjemli" <hjemli@gmail.com> writes:

> On 11/28/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>> On Wed, 28 Nov 2007, Steven Grimm wrote:
>> > I wonder if this shouldn't be branch.<name>.pulltype or something like
>> > that, so we can represent more than just "rebase or not." Values could
>> > be "rebase", "merge" (the default) and maybe even "manual" to specify
>> > that git-pull should neither merge nor rebase a particular branch even
>> > if it matches a wildcard refspec.
>>
>> I am not convinced that this is a good thing... We already have
>> branch.<name>.mergeOptions for proper merges, and I want to make clear
>> that this is about rebase, and not about merge.
>
> Maybe branch.<name>.pullOptions ?

Maybe not make this part of git-pull at all?  merge and rebase have
totally different impact on the resulting history, so perhaps a separate
command that is a shorthand for "git fetch && git rebase" may help
unconfuse the users.

^ 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