Git development
 help / color / mirror / Atom feed
* Re: rename tracking and file-name swapping
From: Yuri D'Elia @ 2009-09-13 21:13 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Bernardo Innocenti
In-Reply-To: <7viqfmsoej.fsf@alter.siamese.dyndns.org>

On 13 Sep 2009, at 20:14, Junio C Hamano wrote:

>> % mv file1.txt file3.txt
>> % mv file2.txt file1.txt
>> % mv file3.txt file2.txt
>> % git add file1.txt file2.txt
>> % git diff -M --stat --cached
>>  file1.txt |  150 ++++++++++++++++++++++ 
>> +-------------------------------------
>>  file2.txt |  150 ++++++++++++++++++++++++++++++++++++ 
>> +-----------------------
>>  2 files changed, 150 insertions(+), 150 deletions(-)
>
> By default, if the pathname that was present in the old version still
> appears in the new version, that path is not considered as a candiate
> for rename detection.  Only "X used to be there but is gone" and "Y  
> did
> not exist but appeared" are paired up and checked if they are similar.
>
> Give the command -B option, too, to break the filepair that does not
> disappear.

That does the trick. I'm curious, is there any other use for -B  
besides rename handling?
Any reason of why it isn't a default when --find-copies-harder is in  
effect?

^ permalink raw reply

* Re: Confusing git pull error message
From: Jeff King @ 2009-09-13 21:36 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: John Tapsell, Git List
In-Reply-To: <7vfxaqpnpa.fsf@alter.siamese.dyndns.org>

On Sun, Sep 13, 2009 at 01:57:37PM -0700, Junio C Hamano wrote:

> > What you have here is precisely what we observed. But I think one of the
> > complaints was to say more explicitly "that ref doesn't exist on the
> > remote", which I think should be the case if we have got to this point
> > (anything else would have triggered an error in fetch).
> 
> Wouldn't you get into the situation with this?
> 
> 	[remote "origin"]
>         	fetch = refs/heads/master:refs/heads/master
> 	[branch "master"]
>         	remote = origin
>                 merge = refs/heads/next
> 
> I think saying "does not exist" will repeat the same mistake of
> overguessing you are trying to rectify.

You are right, of course. I think your wording makes sense, then
(otherwise we get stuck with "well, we didn't fetch it. Maybe because it
didn't exist. Or maybe because your configuration didn't include it.").

-Peff

^ permalink raw reply

* Re: [PATCH 1/4] reset: add a few tests for "git reset --merge"
From: Tony Finch @ 2009-09-13 22:01 UTC (permalink / raw)
  To: Christian Couder; +Cc: Jakub Narebski, git, Stephan Beyer, Daniel Barkalow
In-Reply-To: <200909110722.13331.chriscool@tuxfamily.org>

On Fri, 11 Sep 2009, Christian Couder wrote:
> On Thursday 10 September 2009, Jakub Narebski wrote:
> > Christian Couder wrote:
> > >
> > > exec </dev/null
> >
> > What does this do?
>
> Nothing! Yeah, this is a mistake.

It redirects stdin of the current shell.

Tony.
-- 
f.anthony.n.finch  <dot@dotat.at>  http://dotat.at/
GERMAN BIGHT HUMBER: SOUTHWEST 5 TO 7. MODERATE OR ROUGH. SQUALLY SHOWERS.
MODERATE OR GOOD.

^ permalink raw reply

* Re: Confusing git pull error message
From: Junio C Hamano @ 2009-09-13 22:11 UTC (permalink / raw)
  To: Jeff King; +Cc: John Tapsell, Git List
In-Reply-To: <20090913213609.GA6993@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

>> I think saying "does not exist" will repeat the same mistake of
>> overguessing you are trying to rectify.
>
> You are right, of course. I think your wording makes sense, then
> (otherwise we get stuck with "well, we didn't fetch it. Maybe because it
> didn't exist. Or maybe because your configuration didn't include it.").

Yeah, the saddest part of this whole story is that the message given by
the very original was succinct and accurate.

   echo >&2 "Warning: No merge candidate found because value of config option
    \"branch.${curr_branch}.merge\" does not match any remote branch fetched."
   echo >&2 "No changes."

Then we piled up different messages for various cases, and it went
downhill from there.

 - First, 8fc293c (Make git-pull complain and give advice when there is
   nothing to merge with, 2007-10-02) made the message say essentially the
   same thing, but with a lot more verbosity, covering only one _common_
   case.

 - Then 441ed41 ("git pull --tags": error out with a better message.,
   2007-12-28) added a special case for --tags.  The description before
   8fc293c would have still been correct without this change.  This was
   necessary because 8fc293c talked only about the "I am on branch and
   there is no configuration" case.

 - Then 61e6108 (git-pull.sh: better warning message for "git pull" on
   detached head., 2009-04-08) added another special case, again because
   the "helpful" instruction given by 8fc293c did not make any sense in
   the detached HEAD case.

It definitely is worth to further cover cases the message by 8fc293c did
not consider, as this thread suggested, but at the same time, I think the
advice mechanism should be used to bring back a succinct and to-the-point
message, close to the original one, e.g.

	Warning: Not merging anything, as no default merge candidate ref
	was fetched from the remote.

so that people who know what they are doing can squelch potentially
misleading "helpful" advices and diagnose/think for their own.

^ permalink raw reply

* Re: [PATCH 4/4] reset: add test cases for "--merge-dirty" option
From: Paolo Bonzini @ 2009-09-13 22:15 UTC (permalink / raw)
  To: Christian Couder
  Cc: Daniel Barkalow, Junio C Hamano, git, Johannes Schindelin,
	Stephan Beyer, Jakub Narebski, Linus Torvalds
In-Reply-To: <200909110705.35083.chriscool@tuxfamily.org>


> If they get merged, the option name '--merge-dirty" will probably be changed
> to something else like "--merge-safe" so I will have to change some patches
> anyway.

--merge-index or --merge-staged?

Also, it would be great if you would prepare tables like these for all 
cases of git-reset.

Paolo

^ permalink raw reply

* [PATCH] git-gui: search 4 directories to improve statistic of gc hint
From: Clemens Buchacher @ 2009-09-13 22:20 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, msysgit, Shawn O. Pearce, Jeff King
In-Reply-To: <20090913211916.GA5029@localhost>

On Windows, git-gui suggests running the garbage collector if it finds
1 or more files in .git/objects/42 (as opposed to 8 files on other
platforms). The probability of that happening if the repo contains
about 100 loose objects is 32%. The probability for the same to happen
when searching 4 directories is only 8%, which is bit more reasonable.

Also remove $objects_limit from the message, because we already know
that we are above (or close to) that limit. Telling the user about
that number does not really give him any useful information.

The following octave script shows the probability for at least m*q
objects to be found in q subdirectories of .git/objects if n is the
total number of objects.

q = 4;
m = [1 2 8];
n = 0:10:2000;

P = zeros(length(n), length(m));
for k = 1:length(n)
        P(k, :) = 1-binocdf(q*m-1, n(k), q/(256-q));
end
plot(n, P);

n \ q   1       4
50      18%     1%
100     32%     8%
200     54%     39%
500     86%     96%

Signed-off-by: Clemens Buchacher <drizzd@aon.at>
---

 git-gui/lib/database.tcl |   21 ++++++++++-----------
 1 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/git-gui/lib/database.tcl b/git-gui/lib/database.tcl
index a18ac8b..d4e0bed 100644
--- a/git-gui/lib/database.tcl
+++ b/git-gui/lib/database.tcl
@@ -89,27 +89,26 @@ proc do_fsck_objects {} {
 }
 
 proc hint_gc {} {
-	set object_limit 8
+	set ndirs 1
+	set limit 8
 	if {[is_Windows]} {
-		set object_limit 1
+		set ndirs 4
+		set limit 1
 	}
 
-	set objects_current [llength [glob \
-		-directory [gitdir objects 42] \
+	set count [llength [glob \
 		-nocomplain \
-		-tails \
 		-- \
-		*]]
+		[gitdir objects 4\[0-[expr {$ndirs-1}]\]/*]]]
 
-	if {$objects_current >= $object_limit} {
-		set objects_current [expr {$objects_current * 250}]
-		set object_limit    [expr {$object_limit    * 250}]
+	if {$count >= $limit * $ndirs} {
+		set objects_current [expr {$count * 256/$ndirs}]
 		if {[ask_popup \
 			[mc "This repository currently has approximately %i loose objects.
 
-To maintain optimal performance it is strongly recommended that you compress the database when more than %i loose objects exist.
+To maintain optimal performance it is strongly recommended that you compress the database.
 
-Compress the database now?" $objects_current $object_limit]] eq yes} {
+Compress the database now?" $objects_current]] eq yes} {
 			do_gc
 		}
 	}
-- 
1.6.5.rc0.164.g5f6b0

^ permalink raw reply related

* Re: Confusing git pull error message
From: Jeff King @ 2009-09-13 22:39 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: John Tapsell, Git List
In-Reply-To: <7v7hw2pmtk.fsf@alter.siamese.dyndns.org>

On Sun, Sep 13, 2009 at 02:16:39PM -0700, Junio C Hamano wrote:

> I am _not_ going to commit the following patch, because it will interfere
> with this clarification effort, but I think we want to do something along
> this line after the "clarification" settles.

Please, yes. When I first suggested the advice.* mechanism, I had a
feeling in the back of my mind that there were several such messages
which had bothered me, but I didn't read through the code base looking
for them. But now that you mention it, this "no merge candidate" message
is definitely one of them (not even because it's wrong or anything, but
because of sheer _size_ of it).

> An observation I'd like to make is that this is way too ugly:
> 
> 	[advice]
>         	pullNoMergeFound = false
>                 pushNonFastForward = false
>                 statusHints = false
> 
> than
> 
> 	[IKnowWhatIAmDoingThankYouVeryMuch]
>         	pullNoMergeFound
>                 pushNonFastForward
>                 statusHints
> 
> but this feature is for people who know what they are doing, so I guess
> the current set-up would be fine.

Maybe a nice shorthand would be '!' for negative booleans. I.e.,

  [advice]
  !pullNoMergeFound
  !pushNonFastForward
  !statusHints

-Peff

^ permalink raw reply

* Patches for git-push --confirm and --show-subjects
From: Owen Taylor @ 2009-09-13 23:31 UTC (permalink / raw)
  To: git

Here's a first try at something like what was discussed. Various notes:

 * I didn't try to implement --confirm for rsync and http pushes; it would
   require completely different code and it sounds like they will eventually
   be switched to the "push_refs" code path as well.

 * I picked the name --show-subjects for the that option because
   --show-commits/--log-commits implied a closer connection to 'git show'
   or 'git log'. --show-subjects implies (to me) something more free-form.

 * --show-subjects might actually benefit from having a short option
   but I omitted that for now.

 * I ripped off a big hunk of code from builtin-fmt-merge-msg.c to do the
   commit synopsis without completely understanding it. There are quite
   a few differences from the original and it was beyond my knowledge of
   the git code base to figure out whether some shared utility could be
   added.

   Along with differences in the input parameters and the output, there's
   one "bug fix" I made to the code - in the orginal, if you have exactly
   21 commits it will show:

     (21 commits):
      <commit 1>
      <commit 2>
      [...]
      <commit 20>
     ...

   So the last commit is pointlessly substituted with '...'; that's more
   annoying if you are showing just a few commits, so I fixed it in the
   adapted code.

 * Passing three booleans 'int verbose, int show_subjects, int porcelain'
   between functions in transport.c is somewhat error-prone, but I didn't
   want to switch to flags, since it would have made the patches here
   less incremental.

 * The interaction between --confirm and --show-subjects and --porcelain
   is a bit tricky, but I think what I ended up with right - the basic
   idea is that that '--confirm --porcelain' should let the user confirm
   then output what actually got done to the wrapper script on stdout in
   porcelain format. Didn't try to describe the details in the docs.

 * My first attempt at changing the git code, so probably some stupidity
   in there somewhere :-)

^ permalink raw reply

* [PATCH 1/4] push: add --confirm option to ask before sending updates
From: Owen Taylor @ 2009-09-13 23:31 UTC (permalink / raw)
  To: git; +Cc: Owen W. Taylor
In-Reply-To: <1252884685-9169-1-git-send-email-otaylor@redhat.com>

From: Owen W. Taylor <otaylor@fishsoup.net>

When --confirm is specified, the refs being updated are displayed
to the user first, and the user is prompted whether to proceed
or not.

Signed-off-by: Owen W. Taylor <otaylor@fishsoup.net>
---
 Documentation/git-push.txt |    9 ++++-
 builtin-push.c             |    8 +++--
 builtin-send-pack.c        |    4 ++-
 send-pack.h                |    3 +-
 transport.c                |   87 ++++++++++++++++++++++++++++++++++++++++----
 transport.h                |    3 +-
 6 files changed, 98 insertions(+), 16 deletions(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 58d2bd5..c0bbf16 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -9,8 +9,9 @@ git-push - Update remote refs along with associated objects
 SYNOPSIS
 --------
 [verse]
-'git push' [--all | --mirror | --tags] [--dry-run] [--receive-pack=<git-receive-pack>]
-	   [--repo=<repository>] [-f | --force] [-v | --verbose]
+'git push' [--all | --mirror | --tags] [--dry-run] [--confirm]
+	   [--receive-pack=<git-receive-pack>]  [--repo=<repository>]
+	   [-f | --force] [-v | --verbose]
 	   [<repository> <refspec>...]
 
 DESCRIPTION
@@ -85,6 +86,10 @@ nor in any Push line of the corresponding remotes file---see below).
 --dry-run::
 	Do everything except actually send the updates.
 
+--confirm::
+	Print a summary of what will be done, and then ask the user
+	interactively before actually sending the updates.
+
 --porcelain::
 	Produce machine-readable output.  The output status line for each ref
 	will be tab-separated and sent to stdout instead of stderr.  The full
diff --git a/builtin-push.c b/builtin-push.c
index 6eda372..231be5d 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -10,7 +10,7 @@
 #include "parse-options.h"
 
 static const char * const push_usage[] = {
-	"git push [--all | --mirror] [--dry-run] [--porcelain] [--tags] [--receive-pack=<git-receive-pack>] [--repo=<repository>] [-f | --force] [-v] [<repository> <refspec>...]",
+	"git push [--all | --mirror] [--dry-run] [--confirm] [--porcelain] [--tags] [--receive-pack=<git-receive-pack>] [--repo=<repository>] [-f | --force] [-v] [<repository> <refspec>...]",
 	NULL,
 };
 
@@ -141,6 +141,7 @@ static int do_push(const char *repo, int flags)
 			transport_get(remote, url[i]);
 		int err;
 		int nonfastforward;
+		int disconfirmed;
 		if (receivepack)
 			transport_set_option(transport,
 					     TRANS_OPT_RECEIVEPACK, receivepack);
@@ -150,10 +151,10 @@ static int do_push(const char *repo, int flags)
 		if (flags & TRANSPORT_PUSH_VERBOSE)
 			fprintf(stderr, "Pushing to %s\n", url[i]);
 		err = transport_push(transport, refspec_nr, refspec, flags,
-				     &nonfastforward);
+				     &nonfastforward, &disconfirmed);
 		err |= transport_disconnect(transport);
 
-		if (!err)
+		if (!err || disconfirmed)
 			continue;
 
 		error("failed to push some refs to '%s'", url[i]);
@@ -182,6 +183,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 		OPT_BIT( 0 , "mirror", &flags, "mirror all refs",
 			    (TRANSPORT_PUSH_MIRROR|TRANSPORT_PUSH_FORCE)),
 		OPT_BOOLEAN( 0 , "tags", &tags, "push tags"),
+		OPT_BIT( 0 , "confirm", &flags, "ask before pushing", TRANSPORT_PUSH_CONFIRM),
 		OPT_BIT( 0 , "dry-run", &flags, "dry run", TRANSPORT_PUSH_DRY_RUN),
 		OPT_BIT( 0,  "porcelain", &flags, "machine-readable output", TRANSPORT_PUSH_PORCELAIN),
 		OPT_BIT('f', "force", &flags, "force updates", TRANSPORT_PUSH_FORCE),
diff --git a/builtin-send-pack.c b/builtin-send-pack.c
index 37e528e..0264180 100644
--- a/builtin-send-pack.c
+++ b/builtin-send-pack.c
@@ -406,7 +406,9 @@ int send_pack(struct send_pack_args *args,
 			REF_STATUS_OK;
 	}
 
-	packet_flush(out);
+	/* Don't flush until the second pass of 'git push --confirm' */
+	if (!(args->dry_run && args->confirm))
+		packet_flush(out);
 	if (new_refs && !args->dry_run) {
 		if (pack_objects(out, remote_refs, extra_have, args) < 0) {
 			for (ref = remote_refs; ref; ref = ref->next)
diff --git a/send-pack.h b/send-pack.h
index 8b3cf02..f2b9292 100644
--- a/send-pack.h
+++ b/send-pack.h
@@ -8,7 +8,8 @@ struct send_pack_args {
 		force_update:1,
 		use_thin_pack:1,
 		use_ofs_delta:1,
-		dry_run:1;
+		dry_run:1,
+		confirm:1;
 };
 
 int send_pack(struct send_pack_args *args,
diff --git a/transport.c b/transport.c
index 4cb8077..aa1852d 100644
--- a/transport.c
+++ b/transport.c
@@ -766,14 +766,18 @@ static int git_transport_push(struct transport *transport, struct ref *remote_re
 	args.verbose = !!(flags & TRANSPORT_PUSH_VERBOSE);
 	args.quiet = !!(flags & TRANSPORT_PUSH_QUIET);
 	args.dry_run = !!(flags & TRANSPORT_PUSH_DRY_RUN);
+	args.confirm = !!(flags & TRANSPORT_PUSH_CONFIRM);
 
 	ret = send_pack(&args, data->fd, data->conn, remote_refs,
 			&data->extra_have);
 
-	close(data->fd[1]);
-	close(data->fd[0]);
-	ret |= finish_connect(data->conn);
-	data->conn = NULL;
+	/* On the first dry-run pass of --confirm, we need to leave the connection open */
+	if (!((flags & TRANSPORT_PUSH_CONFIRM) && (flags & TRANSPORT_PUSH_DRY_RUN))) {
+		close(data->fd[1]);
+		close(data->fd[0]);
+		ret |= finish_connect(data->conn);
+		data->conn = NULL;
+	}
 
 	return ret;
 }
@@ -867,14 +871,37 @@ int transport_set_option(struct transport *transport,
 	return 1;
 }
 
+static int prompt_yesno(const char *prompt)
+{
+	while (1) {
+		char buf[128];
+
+		fprintf(stderr, prompt);
+		if (!fgets(buf, sizeof(buf), stdin))
+			return 0;
+		if (buf[0] == 'y' || buf[0] == 'Y')
+			return 1;
+		else if (buf[0] == 'n' || buf[0] == 'N')
+			return 0;
+	}
+}
+
 int transport_push(struct transport *transport,
 		   int refspec_nr, const char **refspec, int flags,
-		   int * nonfastforward)
+		   int * nonfastforward, int * disconfirmed)
 {
+	*disconfirmed = 0;
+
 	verify_remote_names(refspec_nr, refspec);
 
-	if (transport->push)
+	if (transport->push) {
+		if (flags & TRANSPORT_PUSH_CONFIRM) {
+			fprintf(stderr, "--confirm cannot be used with remote URL %s\n", transport->url);
+			return -1;
+		}
+
 		return transport->push(transport, refspec_nr, refspec, flags);
+	}
 	if (transport->push_refs) {
 		struct ref *remote_refs =
 			transport->get_refs_list(transport, 1);
@@ -883,6 +910,8 @@ int transport_push(struct transport *transport,
 		int verbose = flags & TRANSPORT_PUSH_VERBOSE;
 		int quiet = flags & TRANSPORT_PUSH_QUIET;
 		int porcelain = flags & TRANSPORT_PUSH_PORCELAIN;
+		int dry_run = flags & TRANSPORT_PUSH_DRY_RUN;
+		int confirm = flags & TRANSPORT_PUSH_CONFIRM;
 		int ret;
 
 		if (flags & TRANSPORT_PUSH_ALL)
@@ -895,14 +924,56 @@ int transport_push(struct transport *transport,
 			return -1;
 		}
 
+		/* --confirm is a no-op when --dry-run is also specified */
+		if (confirm && (dry_run || !isatty(0))) {
+			confirm = 0;
+			flags &= ~TRANSPORT_PUSH_CONFIRM;
+		}
+
+		if (confirm) {
+			struct ref *ref;
+			int proceed = 0;
+
+			ret = transport->push_refs(transport, remote_refs,
+						   flags | TRANSPORT_PUSH_DRY_RUN);
+
+			/* Interaction with --porcelain: we do the first pass interactively
+			 * to stderr with normal formatting, and then once the user has
+			 * confirmed, send the porcelain-formatted output to stdout.
+			 */
+			print_push_status(transport->url, remote_refs,
+					  verbose, 0,
+					  nonfastforward);
+
+			if (ret)
+				return ret;
+
+			if (!refs_pushed(remote_refs)) {
+				fprintf(stderr, "Everything up-to-date\n");
+				return 0;
+			}
+
+			proceed = prompt_yesno("Proceed [y/n]? ");
+			if (!proceed) {
+				*disconfirmed = 1;
+				return -1;
+			}
+
+			for (ref = remote_refs; ref; ref = ref->next)
+				ref->status = REF_STATUS_NONE;
+		}
+
 		ret = transport->push_refs(transport, remote_refs, flags);
 
-		if (!quiet || push_had_errors(remote_refs))
+		/* For --confirm, we don't need to print the details again unless
+		 * something unexpected happened (denyNonFastforwards=true perhaps)
+		 */
+		if (!(quiet || (confirm && !porcelain)) || push_had_errors(remote_refs))
 			print_push_status(transport->url, remote_refs,
 					verbose | porcelain, porcelain,
 					nonfastforward);
 
-		if (!(flags & TRANSPORT_PUSH_DRY_RUN)) {
+		if (!dry_run) {
 			struct ref *ref;
 			for (ref = remote_refs; ref; ref = ref->next)
 				update_tracking_ref(transport->remote, ref, verbose);
diff --git a/transport.h b/transport.h
index c14da6f..1d691d7 100644
--- a/transport.h
+++ b/transport.h
@@ -37,6 +37,7 @@ struct transport {
 #define TRANSPORT_PUSH_VERBOSE 16
 #define TRANSPORT_PUSH_PORCELAIN 32
 #define TRANSPORT_PUSH_QUIET 64
+#define TRANSPORT_PUSH_CONFIRM 128
 
 /* Returns a transport suitable for the url */
 struct transport *transport_get(struct remote *, const char *);
@@ -70,7 +71,7 @@ int transport_set_option(struct transport *transport, const char *name,
 
 int transport_push(struct transport *connection,
 		   int refspec_nr, const char **refspec, int flags,
-		   int * nonfastforward);
+		   int * nonfastforward, int * disconfirmed);
 
 const struct ref *transport_get_remote_refs(struct transport *transport);
 
-- 
1.6.2.5

^ permalink raw reply related

* [PATCH 2/4] push: allow configuring default for --confirm
From: Owen Taylor @ 2009-09-13 23:31 UTC (permalink / raw)
  To: git; +Cc: Owen W. Taylor
In-Reply-To: <1252884685-9169-2-git-send-email-otaylor@redhat.com>

From: Owen W. Taylor <otaylor@fishsoup.net>

A new configuration variable push.confirm sets the default
behavior for whether 'git push' should show prompt the user
interactively before proceeding to update refs.

Signed-off-by: Owen W. Taylor <otaylor@fishsoup.net>
---
 Documentation/config.txt |    4 ++++
 builtin-push.c           |    6 +++++-
 cache.h                  |    1 +
 config.c                 |    4 ++++
 environment.c            |    1 +
 5 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index be0b8ca..3bb632f 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1303,6 +1303,10 @@ pull.octopus::
 pull.twohead::
 	The default merge strategy to use when pulling a single branch.
 
+push.confirm::
+	If set to true, linkgit:git-push[1] will act as if the --confirm
+	option was passed, unless overriden with --no-confirm.
+
 push.default::
 	Defines the action git push should take if no refspec is given
 	on the command line, no refspec is configured in the remote, and
diff --git a/builtin-push.c b/builtin-push.c
index 231be5d..63a0bb0 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -66,7 +66,6 @@ static void setup_push_tracking(void)
 
 static void setup_default_push_refspecs(void)
 {
-	git_config(git_default_config, NULL);
 	switch (push_default) {
 	default:
 	case PUSH_DEFAULT_MATCHING:
@@ -193,6 +192,11 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 		OPT_END()
 	};
 
+	git_config(git_default_config, NULL);
+
+	if (push_confirm)
+		flags |= TRANSPORT_PUSH_CONFIRM;
+
 	argc = parse_options(argc, argv, prefix, options, push_usage, 0);
 
 	if (tags)
diff --git a/cache.h b/cache.h
index 1a6412d..ef8606d 100644
--- a/cache.h
+++ b/cache.h
@@ -558,6 +558,7 @@ enum push_default_type {
 extern enum branch_track git_branch_track;
 extern enum rebase_setup_type autorebase;
 extern enum push_default_type push_default;
+extern int push_confirm;
 
 enum object_creation_mode {
 	OBJECT_CREATION_USES_HARDLINKS = 0,
diff --git a/config.c b/config.c
index c644061..1bc8e6f 100644
--- a/config.c
+++ b/config.c
@@ -575,6 +575,10 @@ static int git_default_branch_config(const char *var, const char *value)
 
 static int git_default_push_config(const char *var, const char *value)
 {
+	if (!strcmp(var, "push.confirm")) {
+		push_confirm = git_config_bool(var, value);
+		return 0;
+	}
 	if (!strcmp(var, "push.default")) {
 		if (!value)
 			return config_error_nonbool(var);
diff --git a/environment.c b/environment.c
index 5de6837..e1c82b9 100644
--- a/environment.c
+++ b/environment.c
@@ -44,6 +44,7 @@ enum safe_crlf safe_crlf = SAFE_CRLF_WARN;
 unsigned whitespace_rule_cfg = WS_DEFAULT_RULE;
 enum branch_track git_branch_track = BRANCH_TRACK_REMOTE;
 enum rebase_setup_type autorebase = AUTOREBASE_NEVER;
+int push_confirm;
 enum push_default_type push_default = PUSH_DEFAULT_MATCHING;
 #ifndef OBJECT_CREATION_MODE
 #define OBJECT_CREATION_MODE OBJECT_CREATION_USES_HARDLINKS
-- 
1.6.2.5

^ permalink raw reply related

* [PATCH 3/4] push: add --show-subjects option to show commit synopsis
From: Owen Taylor @ 2009-09-13 23:31 UTC (permalink / raw)
  To: git; +Cc: Owen W. Taylor
In-Reply-To: <1252884685-9169-3-git-send-email-otaylor@redhat.com>

From: Owen W. Taylor <otaylor@fishsoup.net>

When --show-subjects is specified, include a synopsis of added
and removed with each OK or REJECT_NONFASTFORWARD reference update.

(The code for printing the synposis is borrowed and adapted from
builtin-fmt-merge-msg.c)

Signed-off-by: Owen W. Taylor <otaylor@fishsoup.net>
---
 Documentation/git-push.txt |    4 +
 builtin-push.c             |    3 +-
 transport.c                |  174 +++++++++++++++++++++++++++++++++++++++++---
 transport.h                |    1 +
 4 files changed, 171 insertions(+), 11 deletions(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index c0bbf16..c9fd033 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -142,6 +142,10 @@ useful if you write an alias or script around 'git-push'.
 --verbose::
 	Run verbosely.
 
+--show-subjects::
+	When displaying ref updates, include a synopsis of what
+	commits are being added and removed.
+
 include::urls-remotes.txt[]
 
 OUTPUT
diff --git a/builtin-push.c b/builtin-push.c
index 63a0bb0..7c9e394 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -10,7 +10,7 @@
 #include "parse-options.h"
 
 static const char * const push_usage[] = {
-	"git push [--all | --mirror] [--dry-run] [--confirm] [--porcelain] [--tags] [--receive-pack=<git-receive-pack>] [--repo=<repository>] [-f | --force] [-v] [<repository> <refspec>...]",
+	"git push [--all | --mirror] [--dry-run] [--confirm] [--porcelain] [--tags] [--receive-pack=<git-receive-pack>] [--repo=<repository>] [-f | --force] [-v] [--show-subjects] [<repository> <refspec>...]",
 	NULL,
 };
 
@@ -177,6 +177,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 	struct option options[] = {
 		OPT_BIT('q', "quiet", &flags, "be quiet", TRANSPORT_PUSH_QUIET),
 		OPT_BIT('v', "verbose", &flags, "be verbose", TRANSPORT_PUSH_VERBOSE),
+		OPT_BIT(0, "show-subjects", &flags, "show commit subjects", TRANSPORT_PUSH_SHOW_SUBJECTS),
 		OPT_STRING( 0 , "repo", &repo, "repository", "repository"),
 		OPT_BIT( 0 , "all", &flags, "push all refs", TRANSPORT_PUSH_ALL),
 		OPT_BIT( 0 , "mirror", &flags, "mirror all refs",
diff --git a/transport.c b/transport.c
index aa1852d..c07291e 100644
--- a/transport.c
+++ b/transport.c
@@ -1,4 +1,6 @@
 #include "cache.h"
+#include "commit.h"
+#include "diff.h"
 #include "transport.h"
 #include "run-command.h"
 #include "pkt-line.h"
@@ -8,6 +10,7 @@
 #include "bundle.h"
 #include "dir.h"
 #include "refs.h"
+#include "revision.h"
 
 /* rsync support */
 
@@ -619,7 +622,151 @@ static const char *status_abbrev(unsigned char sha1[20])
 	return find_unique_abbrev(sha1, DEFAULT_ABBREV);
 }
 
-static void print_ok_ref_status(struct ref *ref, int porcelain)
+struct list {
+	char **list;
+	unsigned nr, alloc;
+};
+
+static void append_to_list(struct list *list, char *value)
+{
+	if (list->nr == list->alloc) {
+		list->alloc += 32;
+		list->list = xrealloc(list->list, sizeof(char *) * list->alloc);
+	}
+	list->list[list->nr++] = value;
+}
+
+static void free_list(struct list *list)
+{
+	int i;
+
+	if (list->alloc == 0)
+		return;
+
+	for (i = 0; i < list->nr; i++) {
+		free(list->list[i]);
+	}
+	free(list->list);
+	list->nr = list->alloc = 0;
+}
+
+static void shortlog(struct commit *from, struct commit *to,
+		     const char *heading, int limit)
+{
+	struct rev_info rev;
+	int i, count = 0;
+	struct commit *commit;
+	struct list subjects = { NULL, 0, 0 };
+	int flags = UNINTERESTING | TREESAME | SEEN | SHOWN | ADDED;
+	const char *prefix;
+
+	init_revisions(&rev, NULL);
+	rev.commit_format = CMIT_FMT_ONELINE;
+	rev.limited = 1;
+	rev.ignore_merges = 0;
+
+	setup_revisions(0, NULL, &rev, NULL);
+
+	add_pending_object(&rev, &from->object, "");
+	add_pending_object(&rev, &to->object, "");
+	from->object.flags |= UNINTERESTING;
+	if (prepare_revision_walk(&rev))
+		die("revision walk setup failed");
+	while ((commit = get_revision(&rev)) != NULL) {
+		char *oneline, *bol, *eol;
+
+		count++;
+		if (subjects.nr > limit)
+			continue;
+
+		bol = strstr(commit->buffer, "\n\n");
+		if (bol) {
+			unsigned char c;
+			do {
+				c = *++bol;
+			} while (isspace(c));
+			if (!c)
+				bol = NULL;
+		}
+
+		if (!bol) {
+			append_to_list(&subjects, xstrdup(sha1_to_hex(commit->object.sha1)));
+			continue;
+		}
+
+		eol = strchr(bol, '\n');
+		if (eol) {
+			oneline = xmemdupz(bol, eol - bol);
+		} else {
+			oneline = xstrdup(bol);
+		}
+		append_to_list(&subjects, oneline);
+	}
+
+	if (heading || count > limit) {
+		fprintf(stderr, "      ");
+		if (heading)
+			fprintf(stderr, "%s", heading);
+		if (heading && count > limit)
+			fprintf(stderr, " (%d)", count);
+		else if (count > limit)
+			fprintf(stderr, "%d commits", count);
+		fprintf(stderr, ":\n");
+		prefix = "         ";
+	} else {
+		prefix = "      ";
+	}
+
+	for (i = 0; i < count && i < limit; i++)
+		if (i == limit - 1 && count > limit)
+			fprintf(stderr, "%s...\n", prefix);
+		else
+			fprintf(stderr, "%s%s\n", prefix, subjects.list[i]);
+
+	clear_commit_marks(from, flags);
+	clear_commit_marks(to, flags);
+	free_commit_list(rev.commits);
+	rev.commits = NULL;
+	rev.pending.nr = 0;
+
+	free_list(&subjects);
+}
+
+/* Maximum lines number of subjects to show (including ...) */
+#define SUBJECTS_LIMIT 8
+
+static void print_subjects(struct ref *ref)
+{
+	struct commit *old;
+	struct commit *new;
+	struct commit_list *merge_bases;
+	int added = 1;
+	int removed = 1;
+
+	old = lookup_commit_reference_gently(ref->old_sha1, 1);
+	if (!old) {
+		fprintf(stderr, "      Unknown changes (please run 'git fetch')\n");
+		return;
+	}
+	new = lookup_commit_reference(ref->new_sha1);
+
+	merge_bases = get_merge_bases(old, new, 1);
+	if (merge_bases && !merge_bases->next && merge_bases->item == old)
+		removed = 0;
+	if (merge_bases && !merge_bases->next && merge_bases->item == new)
+		added = 0;
+
+	if (added && !removed) {
+		shortlog(old, new, NULL, SUBJECTS_LIMIT);
+	} else {
+		if (added)
+			shortlog(old, new, "Added commits", SUBJECTS_LIMIT);
+		if (removed)
+			shortlog(new, old, "Removed commits", SUBJECTS_LIMIT);
+	}
+}
+
+static void print_ok_ref_status(struct ref *ref, int porcelain, int show_subjects)
 {
 	if (ref->deletion)
 		print_ref_status('-', "[deleted]", ref, NULL, NULL, porcelain);
@@ -646,10 +793,13 @@ static void print_ok_ref_status(struct ref *ref, int porcelain)
 		strcat(quickref, status_abbrev(ref->new_sha1));
 
 		print_ref_status(type, quickref, ref, ref->peer_ref, msg, porcelain);
+
+		if (show_subjects)
+			print_subjects(ref);
 	}
 }
 
-static int print_one_push_status(struct ref *ref, const char *dest, int count, int porcelain)
+static int print_one_push_status(struct ref *ref, const char *dest, int count, int show_subjects, int porcelain)
 {
 	if (!count)
 		fprintf(stderr, "To %s\n", dest);
@@ -669,6 +819,8 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count, i
 	case REF_STATUS_REJECT_NONFASTFORWARD:
 		print_ref_status('!', "[rejected]", ref, ref->peer_ref,
 						 "non-fast forward", porcelain);
+		if (show_subjects)
+			print_subjects(ref);
 		break;
 	case REF_STATUS_REMOTE_REJECT:
 		print_ref_status('!', "[remote rejected]", ref,
@@ -681,7 +833,7 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count, i
 						 "remote failed to report status", porcelain);
 		break;
 	case REF_STATUS_OK:
-		print_ok_ref_status(ref, porcelain);
+		print_ok_ref_status(ref, porcelain, show_subjects);
 		break;
 	}
 
@@ -689,7 +841,7 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count, i
 }
 
 static void print_push_status(const char *dest, struct ref *refs,
-			      int verbose, int porcelain, int * nonfastforward)
+			      int verbose, int show_subjects, int porcelain, int * nonfastforward)
 {
 	struct ref *ref;
 	int n = 0;
@@ -697,19 +849,19 @@ static void print_push_status(const char *dest, struct ref *refs,
 	if (verbose) {
 		for (ref = refs; ref; ref = ref->next)
 			if (ref->status == REF_STATUS_UPTODATE)
-				n += print_one_push_status(ref, dest, n, porcelain);
+				n += print_one_push_status(ref, dest, n, show_subjects, porcelain);
 	}
 
 	for (ref = refs; ref; ref = ref->next)
 		if (ref->status == REF_STATUS_OK)
-			n += print_one_push_status(ref, dest, n, porcelain);
+			n += print_one_push_status(ref, dest, n, show_subjects, porcelain);
 
 	*nonfastforward = 0;
 	for (ref = refs; ref; ref = ref->next) {
 		if (ref->status != REF_STATUS_NONE &&
 		    ref->status != REF_STATUS_UPTODATE &&
 		    ref->status != REF_STATUS_OK)
-			n += print_one_push_status(ref, dest, n, porcelain);
+			n += print_one_push_status(ref, dest, n, show_subjects, porcelain);
 		if (ref->status == REF_STATUS_REJECT_NONFASTFORWARD)
 			*nonfastforward = 1;
 	}
@@ -912,6 +1064,7 @@ int transport_push(struct transport *transport,
 		int porcelain = flags & TRANSPORT_PUSH_PORCELAIN;
 		int dry_run = flags & TRANSPORT_PUSH_DRY_RUN;
 		int confirm = flags & TRANSPORT_PUSH_CONFIRM;
+		int show_subjects = flags & TRANSPORT_PUSH_SHOW_SUBJECTS;
 		int ret;
 
 		if (flags & TRANSPORT_PUSH_ALL)
@@ -942,7 +1095,7 @@ int transport_push(struct transport *transport,
 			 * confirmed, send the porcelain-formatted output to stdout.
 			 */
 			print_push_status(transport->url, remote_refs,
-					  verbose, 0,
+					  verbose, show_subjects, 0,
 					  nonfastforward);
 
 			if (ret)
@@ -970,8 +1123,9 @@ int transport_push(struct transport *transport,
 		 */
 		if (!(quiet || (confirm && !porcelain)) || push_had_errors(remote_refs))
 			print_push_status(transport->url, remote_refs,
-					verbose | porcelain, porcelain,
-					nonfastforward);
+					verbose | porcelain,
+					show_subjects && !confirm && !porcelain,
+					porcelain, nonfastforward);
 
 		if (!dry_run) {
 			struct ref *ref;
diff --git a/transport.h b/transport.h
index 1d691d7..6a002a3 100644
--- a/transport.h
+++ b/transport.h
@@ -38,6 +38,7 @@ struct transport {
 #define TRANSPORT_PUSH_PORCELAIN 32
 #define TRANSPORT_PUSH_QUIET 64
 #define TRANSPORT_PUSH_CONFIRM 128
+#define TRANSPORT_PUSH_SHOW_SUBJECTS 256
 
 /* Returns a transport suitable for the url */
 struct transport *transport_get(struct remote *, const char *);
-- 
1.6.2.5

^ permalink raw reply related

* [PATCH 4/4] push: allow configuring default for --show-subjects
From: Owen Taylor @ 2009-09-13 23:31 UTC (permalink / raw)
  To: git; +Cc: Owen W. Taylor
In-Reply-To: <1252884685-9169-4-git-send-email-otaylor@redhat.com>

From: Owen W. Taylor <otaylor@fishsoup.net>

A new configuration variable push.show-subjects sets the default
behavior for whether 'git push' should show a commit synopsis with
each updated ref.

Signed-off-by: Owen W. Taylor <otaylor@fishsoup.net>
---
 Documentation/config.txt |    4 ++++
 builtin-push.c           |    2 ++
 cache.h                  |    1 +
 config.c                 |    4 ++++
 environment.c            |    1 +
 5 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 3bb632f..83bc5a5 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1320,6 +1320,10 @@ push.default::
 * `tracking` push the current branch to its upstream branch.
 * `current` push the current branch to a branch of the same name.
 
+push.show-subjects::
+	If set to true, linkgit:git-push[1] will act as if the --show-subjects
+	option was passed, unless overriden with --no-show-subjects.
+
 rebase.stat::
 	Whether to show a diffstat of what changed upstream since the last
 	rebase. False by default.
diff --git a/builtin-push.c b/builtin-push.c
index 7c9e394..e3dc579 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -197,6 +197,8 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 
 	if (push_confirm)
 		flags |= TRANSPORT_PUSH_CONFIRM;
+	if (push_show_subjects)
+		flags |= TRANSPORT_PUSH_SHOW_SUBJECTS;
 
 	argc = parse_options(argc, argv, prefix, options, push_usage, 0);
 
diff --git a/cache.h b/cache.h
index ef8606d..9e6a1e6 100644
--- a/cache.h
+++ b/cache.h
@@ -559,6 +559,7 @@ extern enum branch_track git_branch_track;
 extern enum rebase_setup_type autorebase;
 extern enum push_default_type push_default;
 extern int push_confirm;
+extern int push_show_subjects;
 
 enum object_creation_mode {
 	OBJECT_CREATION_USES_HARDLINKS = 0,
diff --git a/config.c b/config.c
index 1bc8e6f..bc78876 100644
--- a/config.c
+++ b/config.c
@@ -597,6 +597,10 @@ static int git_default_push_config(const char *var, const char *value)
 		}
 		return 0;
 	}
+	if (!strcmp(var, "push.show-subjects")) {
+		push_show_subjects = git_config_bool(var, value);
+		return 0;
+	}
 
 	/* Add other config variables here and to Documentation/config.txt. */
 	return 0;
diff --git a/environment.c b/environment.c
index e1c82b9..303c54f 100644
--- a/environment.c
+++ b/environment.c
@@ -46,6 +46,7 @@ enum branch_track git_branch_track = BRANCH_TRACK_REMOTE;
 enum rebase_setup_type autorebase = AUTOREBASE_NEVER;
 int push_confirm;
 enum push_default_type push_default = PUSH_DEFAULT_MATCHING;
+int push_show_subjects;
 #ifndef OBJECT_CREATION_MODE
 #define OBJECT_CREATION_MODE OBJECT_CREATION_USES_HARDLINKS
 #endif
-- 
1.6.2.5

^ permalink raw reply related

* could not detach HEAD error didn't identify the cause of the issue
From: Ben Bradshaw @ 2009-09-14  0:14 UTC (permalink / raw)
  To: git

Hi,

I was asked to post this issue to the list, I've just had git tell me it
can't detach HEAD which left me scratching mine as to the cause and fix.
Turns out if I have local untracked files that the remote branch also
has then this error will trigger. I have included my shell backlog
(client data removed). In this example the site_map folder was present
on master and I had a local, untracked copy.

git can obviosly detect this issue and stop my local copy getting in a
bad state, but an error message such as "sites/all/modules/site_map/ is
present in HEAD but is untracked locally, unable to apply changes" (or
something to point the finger) would be much appreciated.

ben@carbon:redacted/$ git pull --rebase
remote: Counting objects: 91, done.
remote: Compressing objects: 100% (72/72), done.
remote: Total 74 (delta 27), reused 0 (delta 0)
Unpacking objects: 100% (74/74), done.
From git+ssh://redacted
   fb1a4c9..bf06bbd  master     -> origin/master
First, rewinding head to replay your work on top of it...
Fast-forwarded master to bf06bbd9e50a2732164fba9e60e65606ab1267ab.
ben@carbon:redacted/$ pwd
/home/ben/redacted
ben@carbon:redacted/$ git pull --rebase
First, rewinding head to replay your work on top of it...
could not detach HEAD
ben@carbon:redacted/$ git status
# On branch master
# Untracked files:
#   (use "git add <file>..." to include in what will be committed)
#
#    sites/all/modules/jquery.ui-1.6.zip
#    sites/all/modules/jquery_ui-6.x-1.3.tar.gz
#    sites/all/modules/jquery_ui/
#    sites/all/modules/modalframe-6.x-1.3.tar.gz
#    sites/all/modules/modalframe/
#    sites/all/modules/nodereference_explorer-6.x-1.1-beta6.tar.gz
#    sites/all/modules/nodereference_explorer/
#    sites/all/modules/noderelationships-6.x-1.1.tar.gz
#    sites/all/modules/noderelationships/
#    sites/all/modules/site_map-6.x-1.1.tar.gz
#    sites/all/modules/site_map/
nothing added to commit but untracked files present (use "git add" to track)
ben@carbon:redacted/$ rm -rf sites/all/modules/site_map/
ben@carbon:redacted/$ git status
# On branch master
# Untracked files:
#   (use "git add <file>..." to include in what will be committed)
#
#    sites/all/modules/jquery.ui-1.6.zip
#    sites/all/modules/jquery_ui-6.x-1.3.tar.gz
#    sites/all/modules/jquery_ui/
#    sites/all/modules/modalframe-6.x-1.3.tar.gz
#    sites/all/modules/modalframe/
#    sites/all/modules/nodereference_explorer-6.x-1.1-beta6.tar.gz
#    sites/all/modules/nodereference_explorer/
#    sites/all/modules/noderelationships-6.x-1.1.tar.gz
#    sites/all/modules/noderelationships/
#    sites/all/modules/site_map-6.x-1.1.tar.gz
nothing added to commit but untracked files present (use "git add" to track)
ben@carbon:redacted/$ git pull --rebase
First, rewinding head to replay your work on top of it...
Fast-forwarded master to 527afccc7565929baf7890b42b1a34174014fee7.


Thanks for your time (and git)
Ben

^ permalink raw reply

* Tracking remote branches
From: Thiago Farina @ 2009-09-14  0:37 UTC (permalink / raw)
  To: git

Hi,

I'm not understanding what is happening here. I tried to track a
remote branch, origin/maint from git. So I did:
$ git branch --track maint origin/maint
$ git checkout maint

I know that I could do simply, instead of above:
$ git checkout --track -b maint origin/maint

OK, now I switched to maint branch. And then I did:
$ git status

# On branch maint
# Changes to be committed
#    (use "git reset HEAD <file>..." to unstage)
#
#          new file:    git-remote-curl
#          new file:    git-replace
#

What happened here? What I have to do now?

^ permalink raw reply

* Re: Patches for git-push --confirm and --show-subjects
From: Junio C Hamano @ 2009-09-14  0:47 UTC (permalink / raw)
  To: Owen Taylor; +Cc: git
In-Reply-To: <1252884685-9169-1-git-send-email-otaylor@redhat.com>

Without reading much of the code, my knee jerk reactions are:

 * This probably can (and from the longer term perspective, should) be
   done inside a pre-push hook that can decline pushing;

 * I do not think it should use two separate push_refs call into transport
   (first with dry-run and second with real).

   Immediately after match_refs() call in transport_push(), you know if
   the push is a non-fast-forward (in which case you do not know what you
   will be losing anyway because you haven't seen what you are missing
   from the other end) or exactly what your fast-forward push will be
   sending, so between that call and the actual transport->push_refs()
   would be the ideal place to call the hook, with a list of "ref old
   new", without running a dry-run.

for a few reasons.

 (1) When push.confirm is set, you do not want to interact with the user
     when the standard input is not a terminal.  But an automated script
     that runs git-push can still use an appropriate pre-push hook to make
     the decision to intervene without human presense.

 (2) As your --show-subjects patch shows, the likes and dislikes of the
     output format for confirmation would be highly personal.  A separate
     hook that is fed list of <ref, old, new> would make it easier to
     customize this to suite people's tastes.

 (3) I do not trust the use of the fmt_merge_message() code in this
     codepath.  That code, like all the major parts of git, relies on
     being able to use the object flag bits for its own purpose, and there
     is a chance that the way transports (present and future) optimizes
     (or may want to optimize in the future) the object transfer by
     implementing clever common ancestry discovery, similar to what is
     done for the fetch-pack side.

     If we force the actual confirmation process out to a separate process
     that runs a hook, I do not have to worry about that, which is a huge
     relief for maintainability of the system.

 (4) The same objects flag bits contamination issue makes me worried about
     your approach of running one transport_push() with dry-run and then
     another without.

^ permalink raw reply

* Re: Patches for git-push --confirm and --show-subjects
From: Junio C Hamano @ 2009-09-14  0:53 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Owen Taylor, git
In-Reply-To: <7vpr9ugxn5.fsf@alter.siamese.dyndns.org>

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

> Without reading much of the code, my knee jerk reactions are:
>
>  * This probably can (and from the longer term perspective, should) be
>    done inside a pre-push hook that can decline pushing;
>
>  * I do not think it should use two separate push_refs call into transport
>    (first with dry-run and second with real).
>
>    Immediately after match_refs() call in transport_push(), you know if
>    the push is a non-fast-forward (in which case you do not know what you
>    will be losing anyway because you haven't seen what you are missing
>    from the other end) or exactly what your fast-forward push will be
>    sending, so between that call and the actual transport->push_refs()
>    would be the ideal place to call the hook, with a list of "ref old
>    new", without running a dry-run.
>
> for a few reasons.
>
>  (1) When push.confirm is set, you do not want to interact with the user
>      when the standard input is not a terminal.  But an automated script
>      that runs git-push can still use an appropriate pre-push hook to make
>      the decision to intervene without human presense.
>
>  (2) As your --show-subjects patch shows, the likes and dislikes of the
>      output format for confirmation would be highly personal.  A separate
>      hook that is fed list of <ref, old, new> would make it easier to
>      customize this to suite people's tastes.
>
>  (3) I do not trust the use of the fmt_merge_message() code in this
>      codepath.  That code, like all the major parts of git, relies on
>      being able to use the object flag bits for its own purpose, and there
>      is a chance that the way transports (present and future) optimizes
>      (or may want to optimize in the future) the object transfer by
>      implementing clever common ancestry discovery, similar to what is
>      done for the fetch-pack side.

Please add ", would be interfered with fmt_merge_message() code
contaminating the object flag bits." at the end of this sentence.

>
>      If we force the actual confirmation process out to a separate process
>      that runs a hook, I do not have to worry about that, which is a huge
>      relief for maintainability of the system.
>
>  (4) The same objects flag bits contamination issue makes me worried about
>      your approach of running one transport_push() with dry-run and then
>      another without.

^ permalink raw reply

* Re: Patches for git-push --confirm and --show-subjects
From: Owen Taylor @ 2009-09-14  2:35 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vpr9ugxn5.fsf@alter.siamese.dyndns.org>

On Sun, 2009-09-13 at 17:47 -0700, Junio C Hamano wrote:
> Without reading much of the code, my knee jerk reactions are:
> 
>  * This probably can (and from the longer term perspective, should) be
>    done inside a pre-push hook that can decline pushing;

Certainly, within the logic of the operation, there's a potential hook.
(Much like pre-receive but on the client side.)

But even if this behavior was in pre-push.sample, it would not meet what
I'm looking for. Which is an easy-to-use behavior you can turn on, no
matter how many different repositories you work in.

Would it work to do it as a helper program? - if "advanced" options are 
found it feeds the list of candidate ref updates through
git-push--helper or something?

>  * I do not think it should use two separate push_refs call into transport
>    (first with dry-run and second with real).
> 
>    Immediately after match_refs() call in transport_push(), you know if
>    the push is a non-fast-forward (in which case you do not know what you
>    will be losing anyway because you haven't seen what you are missing
>    from the other end) or exactly what your fast-forward push will be
>    sending, so between that call and the actual transport->push_refs()
>    would be the ideal place to call the hook, with a list of "ref old
>    new", without running a dry-run.

The reason I had to do two calls to transport->push_refs is not because
it actually pushes the refs twice. It's because the logic for
classifying the refs is in builtin-send-pack.c. When you pass in
args.dry_run=1 you get the classification logic without the network
traffic.

(There's a little messiness about whether it sends the "flush" 0000 or
not that I had to work around, but that's peripheral.)

The way to clean it up is pretty obvious:

 - You add another vfunc to the transport - '->get_capabilities' or
   something - that encapsulates server_supports("delete-refs").

 - You split the classification logic out into a helper function
   (maybe still in builtin-send-pack.c, maybe moved into some other
   file... don't know what's appropriate.)

   After all, if there was another push_refs backend, it shouldn't be
   duplicating the classification logic...

 - You pass pre-classified ref updates to ->push_refs

I don't know how that interacts with other planned changes to this code.

> for a few reasons.
> 
>  (1) When push.confirm is set, you do not want to interact with the user
>      when the standard input is not a terminal.  But an automated script
>      that runs git-push can still use an appropriate pre-push hook to make
>      the decision to intervene without human presense.
> 
>  (2) As your --show-subjects patch shows, the likes and dislikes of the
>      output format for confirmation would be highly personal.  A separate
>      hook that is fed list of <ref, old, new> would make it easier to
>      customize this to suite people's tastes.

The --show-subjects idea is equally useful for --dry-run. And even when
for successful/failed pushes when neither --confirm not --dry-run is
passed.

I'm not that convinced that there's that much scope for configurability
in this area. Clearly there's some arbitrary decisions I made - that
abbreviated hashes wouldn't be useful. That up to 8 commit subjects
should be shown. Etc.

But as yet, there's no data as to whether people would actually want to
make *different* arbitrary decisions.

Adding more configurability (formats, etc.) doesn't really bother me,
though it does seem like coding in advance of need. But what would
bother me is if the feature isn't useful without complex configuration
or installing custom scripts.

>  (3) I do not trust the use of the fmt_merge_message() code in this
>      codepath.  That code, like all the major parts of git, relies on
>      being able to use the object flag bits for its own purpose, and there
>      is a chance that the way transports (present and future) optimizes
>      (or may want to optimize in the future) the object transfer by
>      implementing clever common ancestry discovery, similar to what is
>      done for the fetch-pack side.
> 
>      If we force the actual confirmation process out to a separate process
>      that runs a hook, I do not have to worry about that, which is a huge
>      relief for maintainability of the system.

Well, I guess that's one way to look at the maintainability issues
involved...

I have to take your word as gospel on this ... I don't have a
comprehensive or even a non-comprehensive view of the use of flags.
Certainly almost same code could be put into a git-push--helper binary.

>  (4) The same objects flag bits contamination issue makes me worried about
>      your approach of running one transport_push() with dry-run and then
>      another without.

With only one example of a transport that implements 'push_refs', it's a
little hard to say what a transport *might* do. But as described above,
this is just a code-structure issue.

- Owen

^ permalink raw reply

* Re: [PATCH] git-gui: suggest gc only when counting at least 2 objects
From: Shawn O. Pearce @ 2009-09-14  3:39 UTC (permalink / raw)
  To: Clemens Buchacher; +Cc: Jeff King, Junio C Hamano, git, msysgit
In-Reply-To: <20090913211916.GA5029@localhost>

Clemens Buchacher <drizzd@aon.at> wrote:
> On Sun, Sep 13, 2009 at 04:44:33PM -0400, Jeff King wrote:
> > On Sun, Sep 13, 2009 at 08:41:50PM +0200, Clemens Buchacher wrote:
> > > On Sun, Sep 13, 2009 at 10:58:45AM -0700, Junio C Hamano wrote:
> > > > Somebody cares to explain why this threashold number has to be different
> > > > per platform in the first place? 
> > > 
> > > I really don't know. I vaguely remember someone claim that performance on
> > > Windows suffered from many loose objects more than on other platforms. I
> > > can't find any discussion of it though.
> > 
> > Maybe 8ff487c?

Yes, it was 8ff487c.  Back then I was using Windows on a daily
basis and this was put into git-gui because Aunt Tillie couldn't
remember do run a git-gc every so often, and performance would just
drop in the bucket.  It also quite a bit predates `git gc --auto`
being sprinkled throughout the code base on various operations.

As to why its been 200 as the loose count estimate, that was just
a WAG based on what I observed on my desktop.  2000 on UNIX is
usually fine, 2000 on Windows meant you waited an extra 30 seconds
to perform an operation.
 
> Ok. But it's been 2 years since then and if I'm not mistaken, there have
> been a number of performance improvements to msysgit. So maybe it's time to
> revisit that threshold.

msysgit may have improved, but at the time I was running Git on
Cygwin, and I doubt NTFS has really improved that much.
 
> If, on the other hand, requiring 2 objects really is too many, we should
> maybe check at least two or four directories, which would greatly improve
> the statistic.

I'm concerned about the FS cost of checking more directories, but
this is a one-time penalty on startup of git-gui so it might not
be too bad if it gets us a better estimate.
 
-- 
Shawn.

^ permalink raw reply

* Re: Tracking remote branches
From: Nicolas Sebrecht @ 2009-09-14  5:01 UTC (permalink / raw)
  To: Thiago Farina; +Cc: git
In-Reply-To: <a4c8a6d00909131737v35d7c63dsea669f47456f5acb@mail.gmail.com>

The 13/09/09, Thiago Farina wrote:
> Hi,
> 
> I know that I could do simply, instead of above:
> $ git checkout --track -b maint origin/maint

Or
$ git checkout -t origin/maint

> OK, now I switched to maint branch. And then I did:
> $ git status
> 
> # On branch maint
> # Changes to be committed
> #    (use "git reset HEAD <file>..." to unstage)
> #
> #          new file:    git-remote-curl
> #          new file:    git-replace
> #
> 
> What happened here? What I have to do now?

You've probably added these files to the index, mistakenly. If you don't
have uncommited changes, try :

  git checkout -f master
  git branch -D maint
  git checkout -t origin/maint
  git status

The files should now be shown as "Untracked files".

-- 
Nicolas Sebrecht

^ permalink raw reply

* [PATCH] Nicolas Pitre has a new email address
From: Nicolas Pitre @ 2009-09-14  6:41 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

[-- Attachment #1: Type: TEXT/PLAIN, Size: 3156 bytes --]

Due to problems at cam.org, my nico@cam.org email address is no longer 
valid.  FRom now on, nico@fluxnic.net should be used instead.

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

I was sticking with nico@cam.org since I've been using that address 
for... hmm... something like 17 years now.  A major screw up was needed 
for me to move away from it.

diff --git a/.mailmap b/.mailmap
index 373476b..975e675 100644
--- a/.mailmap
+++ b/.mailmap
@@ -41,6 +41,7 @@ Michele Ballabio <barra_cuda@katamail.com>
 Nanako Shiraishi <nanako3@bluebottle.com>
 Nanako Shiraishi <nanako3@lavabit.com>
 Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
+<nico@fluxnic.net> <nico@cam.org>
 Philippe Bruhat <book@cpan.org>
 Ramsay Allan Jones <ramsay@ramsay1.demon.co.uk>
 René Scharfe <rene.scharfe@lsrfire.ath.cx>
diff --git a/diff-delta.c b/diff-delta.c
index a4e28df..464ac3f 100644
--- a/diff-delta.c
+++ b/diff-delta.c
@@ -4,7 +4,7 @@
  * This code was greatly inspired by parts of LibXDiff from Davide Libenzi
  * http://www.xmailserver.org/xdiff-lib.html
  *
- * Rewritten for GIT by Nicolas Pitre <nico@cam.org>, (C) 2005-2007
+ * Rewritten for GIT by Nicolas Pitre <nico@fluxnic.net>, (C) 2005-2007
  *
  * This code is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
diff --git a/git.spec.in b/git.spec.in
index 4be0834..ab224f7 100644
--- a/git.spec.in
+++ b/git.spec.in
@@ -233,7 +233,7 @@ rm -rf $RPM_BUILD_ROOT
 * Tue Mar 27 2007 Eygene Ryabinkin <rea-git@codelabs.ru>
 - Added the git-p4 package: Perforce import stuff.
 
-* Mon Feb 13 2007 Nicolas Pitre <nico@cam.org>
+* Mon Feb 13 2007 Nicolas Pitre <nico@fluxnic.net>
 - Update core package description (Git isn't as stupid as it used to be)
 
 * Mon Feb 12 2007 Junio C Hamano <junkio@cox.net>
diff --git a/patch-delta.c b/patch-delta.c
index ef748ce..e02e13b 100644
--- a/patch-delta.c
+++ b/patch-delta.c
@@ -2,7 +2,7 @@
  * patch-delta.c:
  * recreate a buffer from a source and the delta produced by diff-delta.c
  *
- * (C) 2005 Nicolas Pitre <nico@cam.org>
+ * (C) 2005 Nicolas Pitre <nico@fluxnic.net>
  *
  * This code is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
diff --git a/progress.c b/progress.c
index 621c34e..132ed95 100644
--- a/progress.c
+++ b/progress.c
@@ -1,7 +1,7 @@
 /*
  * Simple text-based progress display module for GIT
  *
- * Copyright (c) 2007 by Nicolas Pitre <nico@cam.org>
+ * Copyright (c) 2007 by Nicolas Pitre <nico@fluxnic.net>
  *
  * This code is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
diff --git a/test-delta.c b/test-delta.c
index 3d885ff..af40a3c 100644
--- a/test-delta.c
+++ b/test-delta.c
@@ -1,7 +1,7 @@
 /*
  * test-delta.c: test code to exercise diff-delta.c and patch-delta.c
  *
- * (C) 2005 Nicolas Pitre <nico@cam.org>
+ * (C) 2005 Nicolas Pitre <nico@fluxnic.net>
  *
  * This code is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as

^ permalink raw reply related

* Re: rename tracking and file-name swapping
From: Johannes Sixt @ 2009-09-14  6:44 UTC (permalink / raw)
  To: Yuri D'Elia; +Cc: Junio C Hamano, git, Bernardo Innocenti
In-Reply-To: <1248088D-85CB-4335-AD8A-07DB5BAD1AAA@users.sf.net>

Yuri D'Elia schrieb:
> On 13 Sep 2009, at 20:14, Junio C Hamano wrote:
>> By default, if the pathname that was present in the old version still
>> appears in the new version, that path is not considered as a candiate
>> for rename detection.  Only "X used to be there but is gone" and "Y did
>> not exist but appeared" are paired up and checked if they are similar.
>>
>> Give the command -B option, too, to break the filepair that does not
>> disappear.
> 
> That does the trick. I'm curious, is there any other use for -B besides
> rename handling?

Yes: It can make patches easier to read (just like -M and -C do) if a file
was completely rewritten. For example, look at b9dfe51c with and without
-B, and also note the "dissimilarity index" in the diff header.

-- Hannes

^ permalink raw reply

* Re: Error with git svn show-ignore: forbidden access
From: Yann Simon @ 2009-09-14  7:30 UTC (permalink / raw)
  To: Eric Wong; +Cc: git
In-Reply-To: <20090905055708.GA22272@dcvr.yhbt.net>

Hi Eric,

2009/9/5 Eric Wong <normalperson@yhbt.net>:
> Yann Simon <yann.simon.fr@gmail.com> wrote:
>> with git version 1.6.4:
>>
>> $ git svn show-ignore > .gitignore
>> RA layer request failed: Server sent unexpected return value (403
>> Forbidden) in response to PROPFIND request for
>> '/repos/XXX/YYY/ZZZ/trunk/aaa' at /usr/lib/git-core/git-svn line 2243
>>
>> Is git svn show-ignore making request to the svn server?
>
> Yes, git svn has to read the svn:ignore property remotely since it
> doesn't do anything with it when it fetches.  Do you have read
> permissions to /repos/XXX/YYY/ZZZ/trunk/aaa on that repo?

No I do not have permissions to /repos/XXX/YYY/ZZZ/trunk/aaa.
I did some more tests and found out that git svn fetch could care with
this and checkout every folders in trunk except aaa.
Git svn fetch and git svn ignore do not have similar behavior?

> For everything besides initialization/clone, git svn reads the url in
> your $GIT_CONFIG.  --minimize-url is only used for the initial setup.
>
> You can edit it to move the URL down/up a level if you edit your
> corresponding fetch/branches/tags lines:
> [...]

I tried that and it works. Tanks a lot!

It was easy in my case because there are only 2 folders in trunk, one
I can access, and one I cannot.
But let's suppose there are x folders in 'trunk', and only one of them
cannot be accessed.
In that case, the configuration could be very complex, is not it?
But that latter configuration is probably unlikely to be used. If
someone wants to restrict access, he/she would use one folder for
every one, and one folder for restricted access.

---
Yann

^ permalink raw reply

* [PATCH] remove logical typo in documentation of sample update hook
From: Heiko Voigt @ 2009-09-14  8:47 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>
---
 Documentation/githooks.txt |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index 79f633e..06e0f31 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -245,7 +245,7 @@ Both standard output and standard error output are forwarded to
 for the user.
 
 The default 'update' hook, when enabled--and with
-`hooks.allowunannotated` config option turned on--prevents
+`hooks.allowunannotated` config option unset or set to false--prevents
 unannotated tags to be pushed.
 
 [[post-receive]]
-- 
1.6.5.rc1.dirty

^ permalink raw reply related

* [PATCH] web--browse: fix Mac OS X GUI detection for 10.6
From: Heiko Voigt @ 2009-09-14  8:49 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Since OS X 10.6 the variable $SECURITYSESSIONID does not exist anymore,
so lets look for the $TERM_PROGRAM variable as backup.

Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>
---
 git-web--browse.sh |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/git-web--browse.sh b/git-web--browse.sh
index 4f5c740..a578c3a 100755
--- a/git-web--browse.sh
+++ b/git-web--browse.sh
@@ -111,7 +111,8 @@ if test -z "$browser" ; then
 	browser_candidates="w3m links lynx"
     fi
     # SECURITYSESSIONID indicates an OS X GUI login session
-    if test -n "$SECURITYSESSIONID"; then
+    if test -n "$SECURITYSESSIONID" \
+	    -o "$TERM_PROGRAM" = "Apple_Terminal" ; then
 	browser_candidates="open $browser_candidates"
     fi
     # /bin/start indicates MinGW
-- 
1.6.5.rc1.dirty

^ permalink raw reply related

* [PATCH 2/2] remove NORETURN from function pointers
From: Erik Faye-Lund @ 2009-09-14 10:16 UTC (permalink / raw)
  To: git; +Cc: Erik Faye-Lund, Erik Faye-Lund
In-Reply-To: <1252923370-5768-1-git-send-email-kusmabite@gmail.com>

From: Erik Faye-Lund <kusmabite@googlemail.com>

Some compilers (including at least MSVC and ARM RVDS) supports
NORETURN on function declarations, but not on function pointers.

This patch makes it possible to define NORETURN for these compilers.

Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
---

__attribute__((noreturn)) is, according to the GCC documentation, about
two things: code generation (performance, really) and warnings.

On the warnings-side, we need to keep the code warning-free for
compilers who doesn't support noreturn anyway, so hiding potential
warnings through this mechanism is probably not a good idea in the
first place.

We still want the performance-side of it, though. However, the only
place this really makes a difference is to die and it's variants, since
they can potentially be called many times (or so it seems from the
compiler's point of view without a noreturn declaration).

The function pointers are only called once we're already exiting, and
they have only one potential call-site.

I hope this all makes sense ;)

 git-compat-util.h |    2 +-
 usage.c           |    6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/git-compat-util.h b/git-compat-util.h
index 5876d91..15fe08e 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -183,7 +183,7 @@ extern NORETURN void die_errno(const char *err, ...) __attribute__((format (prin
 extern int error(const char *err, ...) __attribute__((format (printf, 1, 2)));
 extern void warning(const char *err, ...) __attribute__((format (printf, 1, 2)));
 
-extern void set_die_routine(void (*routine)(const char *err, va_list params) NORETURN);
+extern void set_die_routine(void (*routine)(const char *err, va_list params));
 
 extern int prefixcmp(const char *str, const char *prefix);
 extern time_t tm_to_time_t(const struct tm *tm);
diff --git a/usage.c b/usage.c
index b6aea45..18d7f43 100644
--- a/usage.c
+++ b/usage.c
@@ -36,12 +36,12 @@ static void warn_builtin(const char *warn, va_list params)
 
 /* If we are in a dlopen()ed .so write to a global variable would segfault
  * (ugh), so keep things static. */
-static void (*usage_routine)(const char *err) NORETURN = usage_builtin;
-static void (*die_routine)(const char *err, va_list params) NORETURN = die_builtin;
+static void (*usage_routine)(const char *err) = usage_builtin;
+static void (*die_routine)(const char *err, va_list params) = die_builtin;
 static void (*error_routine)(const char *err, va_list params) = error_builtin;
 static void (*warn_routine)(const char *err, va_list params) = warn_builtin;
 
-void set_die_routine(void (*routine)(const char *err, va_list params) NORETURN)
+void set_die_routine(void (*routine)(const char *err, va_list params))
 {
 	die_routine = routine;
 }
-- 
1.6.4.msysgit.0.16.gd92d4.dirty

^ permalink raw reply related


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