Git development
 help / color / mirror / Atom feed
* [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

* Re: [PATCH v2] Teach 'git pull' about --rebase
From: Johannes Schindelin @ 2007-11-28 21:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Lars Hjemli, Steven Grimm, Linus Torvalds, git
In-Reply-To: <7vhcj63uhw.fsf@gitster.siamese.dyndns.org>

Hi,

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

> "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.

Not so sure about that.  We already have too many commands, according to 
some outspoken people, and this would add to it.

Besides, the operation "pull" is about getting remote changes incorporated 
in your current branch.  IMHO "pull = fetch + merge" is only a technical 
detail, and we should not be bound by it too much.

Ciao,
Dscho

^ permalink raw reply

* Re: Rollback of git commands
From: Steven Grimm @ 2007-11-28 21:58 UTC (permalink / raw)
  To: Daniel Barkalow
  Cc: Jon Smirl, Karl Hasselström, Junio C Hamano,
	Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0711281600320.5349@iabervon.org>

On Nov 28, 2007, at 1:47 PM, Daniel Barkalow wrote:
> (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'.)

I use rebase -i for that last case and it works fine -- I mark the  
appropriate commit as "edit" in the patch list and the rebase stops  
there, at which point I can update the patch in any way I see fit:  
tweak it a bit, replace it with a different change entirely, change  
the commit message, etc. What's missing from rebase -i in that  
respect? I guess it's not as easy to script for automated patch  
replacement.

-Steve

^ permalink raw reply

* Re: [PATCH v2] Teach 'git pull' about --rebase
From: Jon Loeliger @ 2007-11-28 21:59 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Lars Hjemli, Johannes Schindelin, Steven Grimm, Linus Torvalds,
	git
In-Reply-To: <7vhcj63uhw.fsf@gitster.siamese.dyndns.org>

Junio C Hamano wrote:

> 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.

At long last, we can revive "git update"!

jdl

^ permalink raw reply

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

Steven Grimm <koreth@midwinter.com> writes:

> 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?

I do not like it at all.  git-push is about pushing what you did to the
other end; you may or may not want to refetch what other people
immediately on top of what you did, and this includes what the hook
script did in the repository you have pushed into.  If you want an
immediate refetch, you can script it so that you call push and then
fetch; the latter needs to be a separate connection to a different
service anyway.

^ permalink raw reply

* Re: [PATCH v2] Teach 'git pull' about --rebase
From: Johannes Schindelin @ 2007-11-28 22:02 UTC (permalink / raw)
  To: Jon Loeliger
  Cc: Junio C Hamano, Lars Hjemli, Steven Grimm, Linus Torvalds, git
In-Reply-To: <474DE4D5.6020407@freescale.com>

Hi,

On Wed, 28 Nov 2007, Jon Loeliger wrote:

> Junio C Hamano wrote:
> 
> > 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.
> 
> At long last, we can revive "git update"!

No, we cannot.  By now, too many aliases are called "update".

Ciao,
Dscho

^ permalink raw reply

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

On Nov 28, 2007, at 1:58 PM, Johannes Schindelin wrote:
> Besides, the operation "pull" is about getting remote changes  
> incorporated
> in your current branch.  IMHO "pull = fetch + merge" is only a  
> technical
> detail, and we should not be bound by it too much.

Yeah, I agree. I almost never use "pull" as is because I almost always  
want to rebase. That is, it's basically clutter for me as a command  
right now, and it would remain so even if a separate fetch+rebase  
command were added.

I think this may be a difference in perspective based on how one uses  
git. If I were an integrator I would probably use "pull" as is a lot  
more because I would want the merges represented explicitly in my  
history. But most of the time I'm working as a leaf node in the tree  
of repositories, interacting only with a central integration repo, and  
I basically never want a give-me-the-remote-changes operation (which  
is really more of a "sync me up with the latest changes from the rest  
of the team" operation) to put a merge in my history.

What's more, when I'm introducing git to new people in my environment,  
right now I tell them about fetch and rebase and pretty much have to  
tell them to avoid running "pull" until they're comfortable with git,  
since I know their histories would be full of meaningless merges  
otherwise. It'd be nice to have new people run one less command as  
part of their normal day-to-day work.

-Steve

^ permalink raw reply

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

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>
---

	This is on top of "next". It doesn't do any automatic fetching,
	just prints a status message.

 Documentation/git-receive-pack.txt |    8 +++-
 builtin-send-pack.c                |   65 +++++++++++++++++++++++++++-------
 cache.h                            |    1 +
 receive-pack.c                     |   68 +++++++++++++++++++++++-------------
 4 files changed, 103 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/builtin-send-pack.c b/builtin-send-pack.c
index 25ae1fe..4ba716a 100644
--- a/builtin-send-pack.c
+++ b/builtin-send-pack.c
@@ -164,7 +164,9 @@ static int receive_status(int in, struct ref *refs)
 	hint = NULL;
 	while (1) {
 		char *refname;
+		char *newsha1_hex;
 		char *msg;
+		unsigned char newsha1[20];
 		len = packet_read_line(in, line, sizeof(line));
 		if (!len)
 			break;
@@ -177,7 +179,16 @@ static int receive_status(int in, struct ref *refs)
 
 		line[strlen(line)-1] = '\0';
 		refname = line + 3;
-		msg = strchr(refname, ' ');
+		newsha1_hex = strchr(refname, ' ');
+		if (newsha1_hex) {
+			*newsha1_hex++ = '\0';
+			if (get_sha1_hex(newsha1_hex, newsha1)) {
+				fprintf(stderr, "protocol error: bad sha1 %s\n",
+					newsha1_hex);
+				newsha1_hex = NULL;
+			}
+		}
+		msg = strchr(newsha1_hex, ' ');
 		if (msg)
 			*msg++ = '\0';
 
@@ -197,8 +208,16 @@ static int receive_status(int in, struct ref *refs)
 			continue;
 		}
 
-		if (line[0] == 'o' && line[1] == 'k')
-			hint->status = REF_STATUS_OK;
+		if (line[0] == 'o' && line[1] == 'k') {
+			if (newsha1_hex != NULL &&
+			    hashcmp(hint->new_sha1, newsha1)) {
+				hint->status = REF_STATUS_REMOTE_CHANGED;
+				hashcpy(hint->new_sha1, newsha1);
+			}
+			else {
+				hint->status = REF_STATUS_OK;
+			}
+		}
 		else {
 			hint->status = REF_STATUS_REMOTE_REJECT;
 			ret = -1;
@@ -215,21 +234,34 @@ static void update_tracking_ref(struct remote *remote, struct ref *ref)
 {
 	struct refspec rs;
 
-	if (ref->status != REF_STATUS_OK)
+	if (ref->status != REF_STATUS_OK &&
+	    ref->status != REF_STATUS_REMOTE_CHANGED)
 		return;
 
 	rs.src = ref->name;
 	rs.dst = NULL;
 
 	if (!remote_find_tracking(remote, &rs)) {
-		if (args.verbose)
-			fprintf(stderr, "updating local tracking ref '%s'\n", rs.dst);
-		if (ref->deletion) {
-			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->status == REF_STATUS_REMOTE_CHANGED) {
+			if (args.verbose) {
+				fprintf(stderr, "local tracking ref '%s' "
+					"needs to be fetched\n", rs.dst);
+			}
+		}
+		else {
+			if (args.verbose) {
+				fprintf(stderr, "updating local tracking "
+					"ref '%s'\n", rs.dst);
+			}
+			if (ref->deletion) {
+				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);
 	}
 }
@@ -329,6 +361,10 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count)
 				ref->deletion ? NULL : ref->peer_ref,
 				"remote failed to report status");
 		break;
+	case REF_STATUS_REMOTE_CHANGED:
+		print_ref_status('+', "[changed]", ref, ref->peer_ref,
+				"remote made changes to revisions");
+		break;
 	case REF_STATUS_OK:
 		print_ok_ref_status(ref);
 		break;
@@ -349,12 +385,14 @@ static void print_push_status(const char *dest, struct ref *refs)
 	}
 
 	for (ref = refs; ref; ref = ref->next)
-		if (ref->status == REF_STATUS_OK)
+		if (ref->status == REF_STATUS_OK ||
+		    ref->status == REF_STATUS_REMOTE_CHANGED)
 			n += print_one_push_status(ref, dest, n);
 
 	for (ref = refs; ref; ref = ref->next) {
 		if (ref->status != REF_STATUS_NONE &&
 		    ref->status != REF_STATUS_UPTODATE &&
+		    ref->status != REF_STATUS_REMOTE_CHANGED &&
 		    ref->status != REF_STATUS_OK)
 			n += print_one_push_status(ref, dest, n);
 	}
@@ -522,6 +560,7 @@ static int do_send_pack(int in, int out, struct remote *remote, const char *dest
 		switch (ref->status) {
 		case REF_STATUS_NONE:
 		case REF_STATUS_UPTODATE:
+		case REF_STATUS_REMOTE_CHANGED:
 		case REF_STATUS_OK:
 			break;
 		default:
diff --git a/cache.h b/cache.h
index 4e59646..ae2427d 100644
--- a/cache.h
+++ b/cache.h
@@ -516,6 +516,7 @@ struct ref {
 		REF_STATUS_UPTODATE,
 		REF_STATUS_REMOTE_REJECT,
 		REF_STATUS_EXPECTING_REPORT,
+		REF_STATUS_REMOTE_CHANGED,
 	} status;
 	char *remote_status;
 	struct ref *peer_ref; /* when renaming */
diff --git a/receive-pack.c b/receive-pack.c
index ed44b89..1101e18 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,28 +201,41 @@ 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));
 		}
-		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 */
+			}
 		}
-		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";
 		}
 		return NULL; /* good */
+
+	default:
+		error("hook declined to update %s", name);
+		return "hook declined";
 	}
 }
 
@@ -415,8 +435,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);
-- 
1.5.3.6.2040.g97735-dirty

^ permalink raw reply related

* Re: [PATCH 1/3] Add basic cvsimport tests
From: Junio C Hamano @ 2007-11-28 22:14 UTC (permalink / raw)
  To: Jeff King; +Cc: Emanuele Giaquinta, git
In-Reply-To: <20071128185546.GA11320@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> +	cat <<EOF >o_fortuna &&
> +O Fortuna
> + ...

Copyrights?

> +add "O Fortuna" lyrics
> +
> +These public domain lyrics make an excellent sample text.
> +EOF

Ah, really?  Ok... 

^ permalink raw reply

* Re: [PATCH 1/3] Add basic cvsimport tests
From: David Kastrup @ 2007-11-28 22:20 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, Emanuele Giaquinta, git
In-Reply-To: <7v8x4i3tn4.fsf@gitster.siamese.dyndns.org>

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

> Jeff King <peff@peff.net> writes:
>
>> +	cat <<EOF >o_fortuna &&
>> +O Fortuna
>> + ...
>
> Copyrights?

Benediktbeuern, thirteenth century.

>> +add "O Fortuna" lyrics
>> +
>> +These public domain lyrics make an excellent sample text.
>> +EOF

As long as nobody adds Orff's musical arrangements...

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply

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

Jeff King <peff@peff.net> writes:

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

...

> +	if ($branch eq $opt_o && !$index{branch} &&
> +		!get_headref("$remote/$branch")) {
> +	my $parent = get_headref("$remote/$last_branch");
> +		if (my $sha1 = get_headref("$remote/$mparent")) {

So the definition of get_headref() is to always take everything under
but not including "refs/"?  IOW, the tip of the master branch is asked
with get_headref("heads/master")?

I think show-ref can easily be confused by its tail matching behaviour,
and is a bad command to use in a context like this.  To be safe, I think
get_headref() should be:

	sub get_headref {
        	my ($it) = (@_);
                my $r = `git-rev-parse --verify "refs/$it"`;
		return undef unless $? == 0;
		chomp $r;
		return $r;
	}

^ permalink raw reply

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

On Wed, Nov 28, 2007 at 09:58:52PM +0000, Johannes Schindelin wrote:
> Hi,
> 
> On Wed, 28 Nov 2007, Junio C Hamano wrote:
> 
> > "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.
> 
> Not so sure about that.  We already have too many commands, according to 
> some outspoken people, and this would add to it.

What they're really complaining about is the size and complexity of the
interface, and the lack of a clearly identified subset for them to learn
first.

This has so far mainly manifested itself in complaints about the number
of commands, because that's currently where a lot of our complexity is.
But they *will* complain about proliferation of commandline switches and
config options too.  (I've heard complaints about the number of switches
required on the average cvs commandline, for example.)

We're stuck expanding the interface here, whether we expand it by
another command or another commandline switch.

So, how do you decide whether to make it a new command or not?

	- Look at existing documentation that talks about pull: if that
	  documentation will still apply to the new pull, that weighs
	  for keeping it the same command.  If theat documentation would
	  apply only without having a certain config value set, then I
	  think it's better as a separate command.

	- Will this make it more or less simple to identify the subset
	  of the git syntax that a user will have to do a given job?  If
	  there are jobs for which someone might only ever need the new
	  fetch+rebase, or for which they would only ever need the
	  traditional pull, then I think it would keep the two separate,
	  to make it easier for a learner to skip over information about
	  the one they're not using.

I've got no proposal for an alternate name.  All that comes to mind is
the portmanteau "freebase", which is terrible....

--b.

^ permalink raw reply

* Re: [PATCH] Allow update hooks to update refs on their own
From: Jeff King @ 2007-11-28 22:37 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Steven Grimm, git
In-Reply-To: <7vprxu3urt.fsf@gitster.siamese.dyndns.org>

On Wed, Nov 28, 2007 at 01:49:42PM -0800, Junio C Hamano 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.
> 
> I think Jeff is referring to sp/refspec-match (605b4978).

No, I was actually referring to the jk/send-pack topic. I had thought it
graduated to master, but since Steven's work was based on a much older
version (e.g., with send-pack.c rather than builtin-send-pack.c), I
assumed it had not graduated and didn't confirm.

So let me amend my comment to "base this on a more recent master...".

-Peff

^ permalink raw reply

* Adding Git to Better SCM Initiative : Comparison
From: Jakub Narebski @ 2007-11-28 22:39 UTC (permalink / raw)
  To: git

I'd like to add Git to comparison table of SCMs at Better SCM
Initiative site:
  http://better-scm.berlios.de

To do that, I need to fill in infomration about Git.  Most
of questions / items didn't give much problem, but there
are a few on which I would like your input.

(Yes, I know that such SCM comparisons are usually biased towards the
idea of what are most important features of a version control system.
Nevertheless...)


1. Ease of Deployment

   How easy it is to deploy the software? What are
   the depenedencies and how can they be satisfied?

There are binary packages for Linux (RPM, deb) and for MS Windows
(msysGit).  Git requires POSIX shell, Perl, and POSIX utilities
(BTW. INSTALL file lacks full specification of dependences); although
there is ongoing effort towards making all commands builtin, in C.
Makefile contains ready configuration for many OS, and there is
autoconf script to generate Makefile configuration.

I'm not sure if the problems with compiling documentation to HTML and
manpage forms, which require AsciiDoc and xmlto toolchains should be
also mentioned here.  Well, that and the fact that there exist
pre-build documentation in html and man forms.


2. Command Set

   What is the command set? How compatible it is with
   the commands of CVS (the current open-source defacto
   standard)?

Should it be: "Tries to follow CVS conventions, but deviates where
there is a different design.", as for Mercurial and Monotone?
If I remember correctly command set is patterned somewhat after
BitKeeper.

"Large command set divided into plumbing (low lewel, to be used in
 scripts) and porcelain (high level commands)."


3. Portability

   How portable is the version-control system to various 
   operating systems, computer architectures, and other
   types of systems?

"Good.  Portable across all POSIX systems.
 There exists Win32 binary using MinGW."

>From the results of Git User's Surveys there are many OS on which Git
is used; but it doesn't tell us how hard was to deply Git on those
exotic operatings systems.


4. Repository Permissions

   Is it possible to define permissions on access to different
   parts of a remote repository? Or is access open for all?

"Partial (?). It is possible to lock down repository
 (access to branches and tags) using hooks."

I don't know if it is possible to do finer level ACLs, i.e. if it
is possible to lock subdirectories or files in Git.  Although for
distributed SCMs ACL doesn't matter much: check diffstat and merge or
not from trusted people.  We have "network of trust" (BTW. Karl Fogel
in OSSBook recommends 'soft' control of access control to repository,
on social rather than on technical level).


P.S. How do "svn diff" looks like?
-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: [PATCH 2/3] cvsimport: use show-ref to support packed refs
From: Jeff King @ 2007-11-28 22:43 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, Emanuele Giaquinta, git
In-Reply-To: <7v4pf63t7y.fsf@gitster.siamese.dyndns.org>

On Wed, Nov 28, 2007 at 02:23:13PM -0800, Junio C Hamano wrote:

> > +	if ($branch eq $opt_o && !$index{branch} &&
> > +		!get_headref("$remote/$branch")) {
> > +	my $parent = get_headref("$remote/$last_branch");
> > +		if (my $sha1 = get_headref("$remote/$mparent")) {
> 
> So the definition of get_headref() is to always take everything under
> but not including "refs/"?  IOW, the tip of the master branch is asked
> with get_headref("heads/master")?

No, the $remote variable is "refs/remotes/origin" or "refs/heads". See
the old version of get_headref, which actually looked up
"$git_dir/$remote/$branch".

> I think show-ref can easily be confused by its tail matching behaviour,
> and is a bad command to use in a context like this.  To be safe, I think
> get_headref() should be:
> 
> 	sub get_headref {
>         	my ($it) = (@_);
>                 my $r = `git-rev-parse --verify "refs/$it"`;
> 		return undef unless $? == 0;
> 		chomp $r;
> 		return $r;
> 	}

I have no comment on which is the best command to use (you would know
much better than I), but adding "refs/" is wrong.

-Peff

^ permalink raw reply

* Re: [PATCH v2] Replace instances of export VAR=VAL with VAR=VAL; export VAR
From: Junio C Hamano @ 2007-11-28 22:46 UTC (permalink / raw)
  To: David Kastrup
  Cc: Johannes Schindelin, Johannes Sixt, Wincent Colaiuta,
	Benoit Sigoure, Git Mailing List
In-Reply-To: <85ve7m8iix.fsf@lola.goethe.zz>

David Kastrup <dak@gnu.org> writes:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
>> It might be POSIX, but there are shells that do not like the
>> expression 'export VAR=VAL'.
>
> As long as we have no positive report about any such shell that
> _otherwise_ would be usable for git, why bother?

I thought somebody already mention that ash mishandles "export VAR=VAL"
but otherwise Ok.

^ permalink raw reply

* Re: [PATCH v2] Teach 'git pull' about --rebase
From: J. Bruce Fields @ 2007-11-28 22:47 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Junio C Hamano, Lars Hjemli, Steven Grimm, Linus Torvalds, git
In-Reply-To: <20071128223339.GF7376@fieldses.org>

On Wed, Nov 28, 2007 at 05:33:39PM -0500, J. Bruce Fields wrote:
> What they're really complaining about is the size and complexity of the
> interface, and the lack of a clearly identified subset for them to learn
> first.
> 
> This has so far mainly manifested itself in complaints about the number
> of commands, because that's currently where a lot of our complexity is.
> But they *will* complain about proliferation of commandline switches and
> config options too.  (I've heard complaints about the number of switches
> required on the average cvs commandline, for example.)
> 
> We're stuck expanding the interface here, whether we expand it by
> another command or another commandline switch.
> 
> So, how do you decide whether to make it a new command or not?
> 
> 	- Look at existing documentation that talks about pull: if that
> 	  documentation will still apply to the new pull, that weighs
> 	  for keeping it the same command.  If theat documentation would
> 	  apply only without having a certain config value set, then I
> 	  think it's better as a separate command.
> 
> 	- Will this make it more or less simple to identify the subset
> 	  of the git syntax that a user will have to do a given job?  If
> 	  there are jobs for which someone might only ever need the new
> 	  fetch+rebase, or for which they would only ever need the
> 	  traditional pull, then I think it would keep the two separate,
> 	  to make it easier for a learner to skip over information about
> 	  the one they're not using.
> 
> I've got no proposal for an alternate name.  All that comes to mind is
> the portmanteau "freebase", which is terrible....

Actually, considering the second point: people that are using
fetch+rebase don't necessarily need or (for now) want to understand pull
at all.  But they certainly *do* have to understand rebase.  Would it be
possible to add this to rebase instead of to pull?

	git rebase --url git://x.org/x.git master

where --url means "interpret <upstream> as a branch from the given
remote repository.

That interacts poorly with --onto, though.

--b.

^ permalink raw reply

* Re: [PATCH] Replace instances of export VAR=VAL with VAR=VAL; export VAR
From: Junio C Hamano @ 2007-11-28 22:48 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Johannes Sixt, Wincent Colaiuta, Benoit Sigoure, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0711281903330.27959@racer.site>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> Is there a particular reason to force linebreak when a semicolon would
>> do?
>
> D'oh.  Of course.  You want me to resend?

Semicolon I can handle but you seem to have local changes to filter
branch.

^ permalink raw reply

* Re: Rollback of git commands
From: Daniel Barkalow @ 2007-11-28 22:48 UTC (permalink / raw)
  To: Steven Grimm
  Cc: Jon Smirl, Karl Hasselström, Junio C Hamano,
	Git Mailing List
In-Reply-To: <795F24E6-9145-4007-95EB-DB63D9F6295A@midwinter.com>

On Wed, 28 Nov 2007, Steven Grimm wrote:

> On Nov 28, 2007, at 1:47 PM, Daniel Barkalow wrote:
> >(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'.)
> 
> I use rebase -i for that last case and it works fine -- I mark the appropriate
> commit as "edit" in the patch list and the rebase stops there, at which point
> I can update the patch in any way I see fit: tweak it a bit, replace it with a
> different change entirely, change the commit message, etc. What's missing from
> rebase -i in that respect? I guess it's not as easy to script for automated
> patch replacement.

Just that you have to find the patch yourself and replace it; if you're 
doing this a lot, you'll want to say "here's a new version of 
fix-the-frobnozzle-gadget, do the right thing". Also, for series items 
that are pulling some remote tree, you want it to remember the info, so 
you can just say "pull git-netdev" and have it fetch the latest from the 
appropriate remote and replace the merge commit with a new merge commit.

I think that people in this role using quilt do it by: "quilt pop -a; 
replace the patch file without looking at the series file; quilt push 
-a"; in the (hopefully) common case, you don't have to worry about any of 
the details, which is why the task remains tractable.

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

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

On Wed, Nov 28, 2007 at 05:43:13PM -0500, Jeff King wrote:

> > 	sub get_headref {
> >         	my ($it) = (@_);
> >                 my $r = `git-rev-parse --verify "refs/$it"`;
> > 		return undef unless $? == 0;
> > 		chomp $r;
> > 		return $r;
> > 	}
> 
> I have no comment on which is the best command to use (you would know
> much better than I), but adding "refs/" is wrong.

BTW, one bad thing about the new get_headref compared to the original is
that it does not distinguish very well between "ref does not exist" and
"an error occurred." Is there a lookup command that makes such a
distinction possible?

-Peff

^ permalink raw reply

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

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

> David Kastrup <dak@gnu.org> writes:
>
>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>>
>>> It might be POSIX, but there are shells that do not like the
>>> expression 'export VAR=VAL'.
>>
>> As long as we have no positive report about any such shell that
>> _otherwise_ would be usable for git, why bother?
>
> I thought somebody already mention that ash mishandles "export VAR=VAL"
> but otherwise Ok.

dak@lola:~$ ash
$ export JUNK=woozle
$ sh -c 'echo $JUNK'
woozle
$ exit
dak@lola:~$ dash
$ export JUNK=woozle
$ sh -c 'echo $JUNK'
woozle
$ exit

What problem are we talking about exactly, and with what shell?

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ 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