Git development
 help / color / mirror / Atom feed
* Re: [PATCH 2/2] push: Add '--current', which pushes only the current branch
From: Junio C Hamano @ 2007-11-19  1:28 UTC (permalink / raw)
  To: Steffen Prohaska; +Cc: git, Johannes Schindelin
In-Reply-To: <119540238994-git-send-email-prohaska@zib.de>

Steffen Prohaska <prohaska@zib.de> writes:

> Often you want to push only the current branch to the default
> remote.  This was awkward to do in the past.

While I think --current is a handy shorthand to have, I do not
think the above description is correct.

Wouldn't your earlier patch have allowed the configuration setting:

	[remote "$there"]
        	push = HEAD
	[branch "$current"]
        	remote = $there

to work very well already?

I do not think it is "Often you want" that makes it awkward.

Instead, the awkward case is if you do the "only the current"
push NOT often enough.  If it is often enough, you set the
configuration once and the awkwardness is behind you.

If however it is not often enough, you cannot afford to have the
configuration above, because that would force you to tell from
the command line which branches, not just the current one, to
push, and that is inconvenient because it is not rare enough.

Together with your [PATCH 1/2], I like the general direction
these patckes are taking us, but it feels a bit too hasty.  I
personally am not convinced that switching to --current for
everybody is a good move.

> ...
> Maybe in two years (that's twice an eternity in git time scales):
>
> 4) make "git push --current" the default.

If these, both the uncertainly expressed by "Maybe" and "twice
an eternity" are true, which they are, the new warning in the
current patch are inappropriate.  Many people's settings depend
on a working "push the matching refs" behaviour, and we need a
very good excuse to annoy the existing happy users with such a
warning.

Remember, how much vocal the dissenters might have been on the
list in the recent discussions, we need to consider the needs of
the silent majority that has been content with the current
behaviour for a long time.

The "warning" to annoy them may be a way to get their attention
and get them involved in a discussion to decide what the default
should be.  But changing the default without giving the people
who do not like the _new_ default a way to avoid inconvenience
of always typing --matching or --current is not nice.  And
honestly, I do not think there is one single default that is
good for everybody.

We should be doing better.

A smoother transition route would be:

 - Keep "matching" the built-in default for now;

 - Take your patches (but drop "warning" bits at this point) to
   introduce 'matching' and 'current' behaviours, and a way to
   override the built-in default from the command line;

 - Introduce a configuration 'push.defaultRefs' ('matching' or
   'current') to override that built-in default, so people who
   prefer 'current' can override the built-in default, without
   having to type --current every time.

After all that happens, we can start discussing what the
built-in default should be.  When it is changed after the
discussion concludes (which may never happen), people who want
to keep 'matching' behaviour would have had the configuration
mechanism to override that built-in default for some time during
the discussion period.  So the beginning of that discussion
period is when we should start talking about "We might change
the default soon; set the configuration to your liking if you do
not want to get affected" in the warning.

Then after that, we may or may not decide to change the default.
Even if we end up not changing the default, 'current' people
will then have a way (push.matching = false) to tailor their git
for their liking, so everybody wins.

>  DESCRIPTION
> @@ -63,6 +63,12 @@ the remote repository.
>  	Instead of naming each ref to push, specifies that all
>  	refs under `$GIT_DIR/refs/heads/` be pushed.
>  
> +\--matching::
> +	Instead of naming each ref to push, specifies that matching
> +	refs under `$GIT_DIR/refs/heads/` be pushed.  Matching means
> +	the branch exists locally and at the remote under the same name.
> +	Currently, this is the default.  But this will change in the future.

For the above reason, "But this will..." is a bit premature.

^ permalink raw reply

* Re: [PATCH] Documentation: Fix references to deprecated commands
From: J. Bruce Fields @ 2007-11-19  1:54 UTC (permalink / raw)
  To: Jonas Fonseca; +Cc: Junio C Hamano, Andreas Ericsson, Johannes Schindelin, git
In-Reply-To: <20071112003251.GB21970@diku.dk>

On Mon, Nov 12, 2007 at 01:32:51AM +0100, Jonas Fonseca wrote:
> Subject: [PATCH] Documentation: Fix references to deprecated commands
> 
> ... by changing git-tar-tree reference to git-archive and removing
> seemingly unrelevant footnote about git-ssh-{fetch,upload}.

Makes sense to me, but for some reason git-am complains about a corrupt
patch when I feed it this email.  I reconstructed it by hand, fixed up
one more reference to git-tar-tree, and applied to my tree.

--b.

> 
> Signed-off-by: Jonas Fonseca <fonseca@diku.dk>
> ---
>  Documentation/core-tutorial.txt         |    5 -----
>  Documentation/git-get-tar-commit-id.txt |    4 ++--
>  2 files changed, 2 insertions(+), 7 deletions(-)
> 
>  Now with SOB and ...
> 
>  Jonas Fonseca <fonseca@diku.dk> wrote Mon, Nov 12, 2007:
>  > diff --git a/Documentation/git-get-tar-commit-id.txt b/Documentation/git-get-tar-commit-id.txt
>  > index 9b5f86f..ef1b19c 100644
>  > --- a/Documentation/git-get-tar-commit-id.txt
>  > +++ b/Documentation/git-get-tar-commit-id.txt
>  > @@ -14,12 +14,12 @@ SYNOPSIS
>  >  return code of 1.  This can happen if <tarfile> had not been created
>  > -using git-tar-tree or if the first parameter of git-tar-tree had been
>  > +using git-archive or if the first parameter of git-tar-tree had been
>  >  a tree ID instead of a commit ID or tag.
> 
>  ... s//g
> 
> diff --git a/Documentation/core-tutorial.txt b/Documentation/core-tutorial.txt
> index ebd2492..401d1de 100644
> --- a/Documentation/core-tutorial.txt
> +++ b/Documentation/core-tutorial.txt
> @@ -1090,11 +1090,6 @@ server like git Native transport does.  Any stock HTTP server
>  that does not even support directory index would suffice.  But
>  you must prepare your repository with `git-update-server-info`
>  to help dumb transport downloaders.
> -+
> -There are (confusingly enough) `git-ssh-fetch` and `git-ssh-upload`
> -programs, which are 'commit walkers'; they outlived their
> -usefulness when git Native and SSH transports were introduced,
> -and are not used by `git pull` or `git push` scripts.
>  
>  Once you fetch from the remote repository, you `merge` that
>  with your current branch.
> diff --git a/Documentation/git-get-tar-commit-id.txt b/Documentation/git-get-tar-commit-id.txt
> index 9b5f86f..ef1b19c 100644
> --- a/Documentation/git-get-tar-commit-id.txt
> +++ b/Documentation/git-get-tar-commit-id.txt
> @@ -14,12 +14,12 @@ SYNOPSIS
>  DESCRIPTION
>  -----------
>  Acts as a filter, extracting the commit ID stored in archives created by
> -git-tar-tree.  It reads only the first 1024 bytes of input, thus its
> +gitlink:git-archive[1].  It reads only the first 1024 bytes of input, thus its
>  runtime is not influenced by the size of <tarfile> very much.
>  
>  If no commit ID is found, git-get-tar-commit-id quietly exists with a
>  return code of 1.  This can happen if <tarfile> had not been created
> -using git-tar-tree or if the first parameter of git-tar-tree had been
> +using git-archive or if the <treeish> parameter of git-archive had been
>  a tree ID instead of a commit ID or tag.
> 
> -- 
> Jonas Fonseca
> -
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* git merge no longer handles add/add
From: Martin Langhoff @ 2007-11-19  3:18 UTC (permalink / raw)
  To: git list

git used to be really good at merging 2 branches that had gotten the
same code added independently. Right now, both master
(1.5.3.5.737.gdee1b)
and next (9445b41) are telling me that the exact same file (same
sha1), with the same mode added on 2 branches is a merge conflict. On
_one_ of the branches there is an extra commit.

Hmmm.

I am pretty sure that early git (pre 1.0) could handle this correctly.
I remember sending git-archimport to Junio and immediately doing 3 or
4 commits on top, and having the next pull work correctly without
conflict because the merge machinery spotted the "last common sha1"
for that file and saw that only one side had changed since then.

Happens with recursive and resolve. Happy to provide a minimal repro
case, but I think this is fairly obvious...

cheers,



martin

^ permalink raw reply

* Re: git merge no longer handles add/add
From: Martin Langhoff @ 2007-11-19  3:29 UTC (permalink / raw)
  To: git list
In-Reply-To: <46a038f90711181918s2743137amc6a827db6d1a6a0@mail.gmail.com>

On Nov 19, 2007 4:18 PM, Martin Langhoff <martin.langhoff@gmail.com> wrote:
> Happens with recursive and resolve. Happy to provide a minimal repro
> case, but I think this is fairly obvious...

Steps to repro...

    mkdir repro
    cd repro/
    cp /etc/resolv.conf ./resolv.conf
    git init
    git add resolv.conf
    git commit -m 'a' resolv.conf
    cp resolv.conf resolv-1.conf
    git add resolv-1.conf
    git commit -m 'a1' resolv-1.conf
    echo blabla >> resolv-1.conf
    git commit -m 'a2' resolv-1.conf
    git-branch other HEAD^^
    git checkout other
    cp resolv.conf resolv-1.conf
    git add resolv-1.conf
    git commit -m 'b'
    git checkout master
    git merge other
    ...
   Auto-merged resolv-1.conf
   CONFLICT (add/add): Merge conflict in resolv-1.conf
   Automatic merge failed; fix conflicts and then commit the result.




m

^ permalink raw reply

* [PATCH v2] git-send-email: show all headers when sending mail
From: David D. Kilzer @ 2007-11-19  4:14 UTC (permalink / raw)
  To: gitster; +Cc: git, David D. Kilzer
In-Reply-To: <7vbq9ywqmq.fsf@gitster.siamese.dyndns.org>

As a git newbie, it was confusing to set an In-Reply-To header but then
not see it printed when the git-send-email command was run.

This patch prints all headers that would be sent to sendmail or an SMTP
server instead of only printing From, Subject, Cc, To.  It also removes
the now-extraneous Date header after the "Log says" line.

Added test to t/t9001-send-email.sh.

Signed-off-by: David D. Kilzer <ddkilzer@kilzer.net>
---

Updated t/t9001-send-email.sh per feedback from Junio C Hamano.

 git-send-email.perl   |    4 ++--
 t/t9001-send-email.sh |   37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 2b1f1b5..f4539a0 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -575,7 +575,7 @@ X-Mailer: git-send-email $gitversion
 	if ($quiet) {
 		printf (($dry_run ? "Dry-" : "")."Sent %s\n", $subject);
 	} else {
-		print (($dry_run ? "Dry-" : "")."OK. Log says:\nDate: $date\n");
+		print (($dry_run ? "Dry-" : "")."OK. Log says:\n");
 		if ($smtp_server !~ m#^/#) {
 			print "Server: $smtp_server\n";
 			print "MAIL FROM:<$raw_from>\n";
@@ -583,7 +583,7 @@ X-Mailer: git-send-email $gitversion
 		} else {
 			print "Sendmail: $smtp_server ".join(' ',@sendmail_parameters)."\n";
 		}
-		print "From: $sanitized_sender\nSubject: $subject\nCc: $cc\nTo: $to\n\n";
+		print $header, "\n";
 		if ($smtp) {
 			print "Result: ", $smtp->code, ' ',
 				($smtp->message =~ /\n([^\n]+\n)$/s), "\n";
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 83f9470..659f9c7 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -41,4 +41,41 @@ test_expect_success \
     'Verify commandline' \
     'diff commandline expected'
 
+cat >expected-show-all-headers <<\EOF
+0001-Second.patch
+(mbox) Adding cc: A <author@example.com> from line 'From: A <author@example.com>'
+Dry-OK. Log says:
+Server: relay.example.com
+MAIL FROM:<from@example.com>
+RCPT TO:<to@example.com>,<cc@example.com>,<author@example.com>,<bcc@example.com>
+From: Example <from@example.com>
+To: to@example.com
+Cc: cc@example.com, A <author@example.com>
+Subject: [PATCH 1/1] Second.
+Date: DATE-STRING
+Message-Id: MESSAGE-ID-STRING
+X-Mailer: X-MAILER-STRING
+In-Reply-To: <unique-message-id@example.com>
+References: <unique-message-id@example.com>
+
+Result: OK
+EOF
+
+test_expect_success 'Show all headers' '
+	git send-email \
+		--dry-run \
+		--from="Example <from@example.com>" \
+		--to=to@example.com \
+		--cc=cc@example.com \
+		--bcc=bcc@example.com \
+		--in-reply-to="<unique-message-id@example.com>" \
+		--smtp-server relay.example.com \
+		$patches |
+	sed	-e "s/^\(Date:\).*/\1 DATE-STRING/" \
+		-e "s/^\(Message-Id:\).*/\1 MESSAGE-ID-STRING/" \
+		-e "s/^\(X-Mailer:\).*/\1 X-MAILER-STRING/" \
+		>actual-show-all-headers &&
+	diff -u expected-show-all-headers actual-show-all-headers
+'
+
 test_done
-- 
1.5.3.4

^ permalink raw reply related

* [PATCH/RFC] parse-options: allow to define hidden synonym options
From: Junio C Hamano @ 2007-11-19  6:09 UTC (permalink / raw)
  To: Pierre Habouzit; +Cc: git

By setting the help member to NULL, you can implement an option
that is not shown in the "git-cmd -h" help output.  This is
useful to support backward compatible synonyms without
cluttering the help text.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 parse-options.c |    2 ++
 parse-options.h |    2 +-
 2 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index d3e608a..1ee2c81 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -284,6 +284,8 @@ void usage_with_options(const char * const *usagestr,
 			if (*opts->help)
 				fprintf(stderr, "%s\n", opts->help);
 			continue;
+		} else if (!opts->help) {
+			continue;
 		}
 
 		pos = fprintf(stderr, "    ");
diff --git a/parse-options.h b/parse-options.h
index a8760ac..de249a0 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -49,7 +49,7 @@ typedef int parse_opt_cb(const struct option *, const char *arg, int unset);
  *
  * `help`::
  *   the short help associated to what the option does.
- *   Must never be NULL (except for OPTION_END).
+ *   option with this field set to NULL does not appear in the help listing.
  *   OPTION_GROUP uses this pointer to store the group header.
  *
  * `flags`::
-- 
1.5.3.6.1779.ga3ed

^ permalink raw reply related

* Re: [PATCH 2/2] push: Add '--current', which pushes only the current branch
From: Steffen Prohaska @ 2007-11-19  6:41 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Johannes Schindelin
In-Reply-To: <7vwssfqb0w.fsf@gitster.siamese.dyndns.org>

[-- Attachment #1: Type: text/plain, Size: 6202 bytes --]


On Nov 19, 2007, at 2:28 AM, Junio C Hamano wrote:

> Steffen Prohaska <prohaska@zib.de> writes:
>
>> Often you want to push only the current branch to the default
>> remote.  This was awkward to do in the past.
>
> While I think --current is a handy shorthand to have, I do not
> think the above description is correct.
>
> Wouldn't your earlier patch have allowed the configuration setting:
>
> 	[remote "$there"]
>         	push = HEAD
> 	[branch "$current"]
>         	remote = $there
>
> to work very well already?

No.  This was the case in the verst first version of the patch
series.  Someone, I don't remember who, proposed to put the
resolution of HEAD into builtin-push.c.  This simplified code
a lot.  Now, HEAD is resolved when parsing the command line.
At that time it is replaced with the full local refname.
The refspec parsing code never sees HEAD, and it won't
understand it.  Try the above setup, and you'll see that it
doesn't work.

Anyway it's not needed if we proceed as outlined below.


> I do not think it is "Often you want" that makes it awkward.
>
> Instead, the awkward case is if you do the "only the current"
> push NOT often enough.  If it is often enough, you set the
> configuration once and the awkwardness is behind you.
>
> If however it is not often enough, you cannot afford to have the
> configuration above, because that would force you to tell from
> the command line which branches, not just the current one, to
> push, and that is inconvenient because it is not rare enough.

Will try to rephrase the commit message.


> Together with your [PATCH 1/2], I like the general direction
> these patckes are taking us, but it feels a bit too hasty.  I
> personally am not convinced that switching to --current for
> everybody is a good move.
>
>> ...
>> Maybe in two years (that's twice an eternity in git time scales):
>>
>> 4) make "git push --current" the default.
>
> If these, both the uncertainly expressed by "Maybe" and "twice
> an eternity" are true, which they are, the new warning in the
> current patch are inappropriate.  Many people's settings depend
> on a working "push the matching refs" behaviour, and we need a
> very good excuse to annoy the existing happy users with such a
> warning.

I think 3) is the interesting case.  "git push" should do
nothing by default.  Either you can configure "git push" to do
something by setting a remote.$remote.push line or you need
to provide a command line switch.  But if you do not tell
explicitly what you want, "git push" will not do anything
for you.

I'm not sure if we ever switch to 4).  But already with 3) the
default changed.  So a warning seems to be appropriate.  But if
we go as outlined below, it's probably a different warning.

I attached a patch that illustrates what "do nothing by default"
means.  This patch should _not_ be applied.  It's only an
illustration what I'm working on.


> Remember, how much vocal the dissenters might have been on the
> list in the recent discussions, we need to consider the needs of
> the silent majority that has been content with the current
> behaviour for a long time.
>
> The "warning" to annoy them may be a way to get their attention
> and get them involved in a discussion to decide what the default
> should be.  But changing the default without giving the people
> who do not like the _new_ default a way to avoid inconvenience
> of always typing --matching or --current is not nice.  And
> honestly, I do not think there is one single default that is
> good for everybody.

Personally, I'd switch to the do-nothing default immediately.
But you are right.  More work is needed to have a smooth transition.


> We should be doing better.
>
> A smoother transition route would be:
>
>  - Keep "matching" the built-in default for now;
>
>  - Take your patches (but drop "warning" bits at this point) to
>    introduce 'matching' and 'current' behaviours, and a way to
>    override the built-in default from the command line;
>
>  - Introduce a configuration 'push.defaultRefs' ('matching' or
>    'current') to override that built-in default, so people who
>    prefer 'current' can override the built-in default, without
>    having to type --current every time.

Sounds like a plan.

If we have the configuration variable, maybe we could switch
off the default behaviour immediately.  Setting a single global
config variable once would be sufficient to get it back.  So,
we could change the default and print a recommendation to run
'git config --global push.defaultRefs matching' to get it back.

...

> After all that happens, we can start discussing what the
> built-in default should be.  When it is changed after the
> discussion concludes (which may never happen), people who want
> to keep 'matching' behaviour would have had the configuration
> mechanism to override that built-in default for some time during
> the discussion period.  So the beginning of that discussion
> period is when we should start talking about "We might change
> the default soon; set the configuration to your liking if you do
> not want to get affected" in the warning.

... And we'd not even start the discussion.  Because there's no
need to.  Every user should make a choice, once.  We do not
provide a default (which obviously will trigger another discussion ;)


> Then after that, we may or may not decide to change the default.
> Even if we end up not changing the default, 'current' people
> will then have a way (push.matching = false) to tailor their git
> for their liking, so everybody wins.
>
>>  DESCRIPTION
>> @@ -63,6 +63,12 @@ the remote repository.
>>  	Instead of naming each ref to push, specifies that all
>>  	refs under `$GIT_DIR/refs/heads/` be pushed.
>>
>> +\--matching::
>> +	Instead of naming each ref to push, specifies that matching
>> +	refs under `$GIT_DIR/refs/heads/` be pushed.  Matching means
>> +	the branch exists locally and at the remote under the same name.
>> +	Currently, this is the default.  But this will change in the  
>> future.
>
> For the above reason, "But this will..." is a bit premature.

I'll change the plan and will come back with a full
implementation.

Thanks for the helpful comments.

	Steffen



[-- Attachment #2: 0001-push-do-nothing-by-default.patch --]
[-- Type: application/octet-stream, Size: 3054 bytes --]

From a97c117794c631f556f87419a3dbaa702b858d95 Mon Sep 17 00:00:00 2001
From: Steffen Prohaska <prohaska@zib.de>
Date: Sun, 18 Nov 2007 19:22:30 +0100
Subject: [PATCH] push: do nothing by default

We used to push all matching branches.  This behaviour
was suitable in many situations, but sometimes confusing.

This commit switches off the default.  push now dies instead.

WORK IN PROGRESS.  NOT-SIGNED-OFF.
---
 builtin-push.c        |    9 +++++++--
 t/t5516-fetch-push.sh |   13 ++++---------
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/builtin-push.c b/builtin-push.c
index e5646d4..e637540 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -143,8 +143,13 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 		fprintf(stderr, "--all, --matching, and --current are mutual exclusive.\n");
 		usage_with_options(push_usage, options);
 	}
-	if ((all || matching || current) && refspec)
-		usage_with_options(push_usage, options);
+	if (all || matching || current) {
+		if (refspec)
+			usage_with_options(push_usage, options);
+	} else {
+		if (!refspec)
+			die("No default action found.");
+	}
 	if (!all && !matching && !current && !refspec)
 		fprintf(stderr, "Warning: assuming '--matching'."
 		                " This default will change in the future.\n");
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 20e0796..48689b9 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -108,11 +108,6 @@ test_expect_code 129 'push command line options (2)' '
 	git push --matching testrepo master
 '
 
-test_expect_success 'push command line options (3)' '
-	git push testrepo 2>stderr.txt &&
-	grep -q "Warning: assuming.*--matching" stderr.txt
-'
-
 test_expect_code 129 'push command line options (4)' '
 	git push --all --current testrepo
 '
@@ -154,7 +149,7 @@ test_expect_success 'push with wildcard' '
 test_expect_success 'push with matching heads' '
 
 	mk_test heads/master &&
-	git push testrepo &&
+	git push --matching testrepo &&
 	check_push_result $the_commit heads/master
 
 '
@@ -319,7 +314,7 @@ test_expect_success 'push with dry-run' '
 	cd testrepo &&
 	old_commit=$(git show-ref -s --verify refs/heads/master) &&
 	cd .. &&
-	git push --dry-run testrepo &&
+	git push --dry-run --matching testrepo &&
 	check_push_result $old_commit heads/master
 '
 
@@ -331,7 +326,7 @@ test_expect_success 'push updates local refs' '
 	cd .. &&
 	git clone parent child && cd child &&
 		echo two >foo && git commit -a -m two &&
-		git push &&
+		git push --matching &&
 	test $(git rev-parse master) = $(git rev-parse remotes/origin/master)
 
 '
@@ -346,7 +341,7 @@ test_expect_success 'push does not update local refs on failure' '
 	cd .. &&
 	git clone parent child && cd child &&
 		echo two >foo && git commit -a -m two || exit 1
-		git push && exit 1
+		git push --matching && exit 1
 	test $(git rev-parse master) != $(git rev-parse remotes/origin/master)
 
 '
-- 
1.5.3.5.750.gc43b


[-- Attachment #3: Type: text/plain, Size: 1 bytes --]



^ permalink raw reply related

* Re: git merge no longer handles add/add
From: Junio C Hamano @ 2007-11-19  6:43 UTC (permalink / raw)
  To: Martin Langhoff; +Cc: git list
In-Reply-To: <46a038f90711181929x4bf0794eue73a5dbac8e2688a@mail.gmail.com>

"Martin Langhoff" <martin.langhoff@gmail.com> writes:

> On Nov 19, 2007 4:18 PM, Martin Langhoff <martin.langhoff@gmail.com> wrote:
>> Happens with recursive and resolve. Happy to provide a minimal repro
>> case, but I think this is fairly obvious...
>
> Steps to repro...
>
>     mkdir repro
>     cd repro/
>     cp /etc/resolv.conf ./resolv.conf
>     git init
>     git add resolv.conf
>     git commit -m 'a' resolv.conf
>     cp resolv.conf resolv-1.conf
>     git add resolv-1.conf
>     git commit -m 'a1' resolv-1.conf
>     echo blabla >> resolv-1.conf
      **************
>     git commit -m 'a2' resolv-1.conf
>     git-branch other HEAD^^
>     git checkout other
>     cp resolv.conf resolv-1.conf
>     git add resolv-1.conf
>     git commit -m 'b'
>     git checkout master
>     git merge other
>     ...
>    Auto-merged resolv-1.conf
>    CONFLICT (add/add): Merge conflict in resolv-1.conf
>    Automatic merge failed; fix conflicts and then commit the result.

As far as the point of the merge is concerned, that's an add/add
of _different_ contents, and we have always left the conflict to
resolve for you since day one.  The only case we handle without
complaining is the accidental *clean* merge.  Both branches adds
the path *identically* compared to the common ancestor.

The very initial implementation of merge may have used the total
emptyness as the common ancestor for the merge, and later we
made it a bit more pleasant to resolve by computing the common
part of the file from the two branches to be used as a fake
ancestor contents.  But the fact we left the result as conflict
for you to validate hasn't changed and will not change.

^ permalink raw reply

* Re: [PATCH 2/2] push: Add '--current', which pushes only the current branch
From: Junio C Hamano @ 2007-11-19  7:27 UTC (permalink / raw)
  To: Steffen Prohaska; +Cc: git, Johannes Schindelin
In-Reply-To: <EA5C3227-12E1-43C4-96E8-43BABF26792B@zib.de>

Steffen Prohaska <prohaska@zib.de> writes:

>> Together with your [PATCH 1/2], I like the general direction
>> these patckes are taking us, but it feels a bit too hasty.  I
>> personally am not convinced that switching to --current for
>> everybody is a good move.
>>
>>> ...
>>
> I think 3) is the interesting case.  "git push" should do
> nothing by default.

NO WAY.

Making things cumbersome to everybody does not achieve anything
useful except for a false sense of equality, does it?

Drop that step (3).  That is not useful to anybody.

^ permalink raw reply

* Re: [PATCH] Fix start_command closing cmd->out/in regardless of cmd->close_out/in
From: Johannes Sixt @ 2007-11-19  7:39 UTC (permalink / raw)
  To: Ping Yin; +Cc: git
In-Reply-To: <1195407366-1610-1-git-send-email-pkufranky@gmail.com>

Ping Yin schrieb:
> When 'FILE *fp' is assigned to child_process.out and then start_command or
> run_command is run, the standard output of the child process is expected to
> be outputed to fp. However, sometimes fp is not expected to be closed since
> further IO may be still performmed on fp.
> ---
>  run-command.c |    4 ----
>  1 files changed, 0 insertions(+), 4 deletions(-)
> 
> diff --git a/run-command.c b/run-command.c
> index 476d00c..4e5f58d 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -115,13 +115,9 @@ int start_command(struct child_process *cmd)
>  
>  	if (need_in)
>  		close(fdin[0]);
> -	else if (cmd->in)
> -		close(cmd->in);
>  
>  	if (need_out)
>  		close(fdout[1]);
> -	else if (cmd->out > 1)
> -		close(cmd->out);
>  
>  	if (need_err)
>  		close(fderr[1]);

This is dangerous! You have to audit all current callers whether they close 
cmd->in or cmd->out (if they don't need the fd anymore). Otherwise you risk 
to keep a writable pipe end open and then the reader hangs, waiting for 
input that will never arrive.

-- Hannes

^ permalink raw reply

* Re: [PATCH] Documentation: Fix references to deprecated commands
From: Jonas Fonseca @ 2007-11-19  7:44 UTC (permalink / raw)
  To: J. Bruce Fields
  Cc: Junio C Hamano, Andreas Ericsson, Johannes Schindelin, git
In-Reply-To: <20071119015411.GA4978@fieldses.org>

J. Bruce Fields <bfields@fieldses.org> wrote Sun, Nov 18, 2007:
> On Mon, Nov 12, 2007 at 01:32:51AM +0100, Jonas Fonseca wrote:
> > Subject: [PATCH] Documentation: Fix references to deprecated commands
> > 
> > ... by changing git-tar-tree reference to git-archive and removing
> > seemingly unrelevant footnote about git-ssh-{fetch,upload}.
> 
> Makes sense to me, but for some reason git-am complains about a corrupt
> patch when I feed it this email.  I reconstructed it by hand, fixed up
> one more reference to git-tar-tree, and applied to my tree.

It should have been merged in a4e57e75c95c66c32da6b106313bc847110794ba.
And yes, as Junio also pointed out, I deleted some context lines at the
end of the patch.

-- 
Jonas Fonseca

^ permalink raw reply

* Re: [PATCH 2/2] push: Add '--current', which pushes only the current branch
From: Junio C Hamano @ 2007-11-19  7:50 UTC (permalink / raw)
  To: Steffen Prohaska; +Cc: git, Johannes Schindelin
In-Reply-To: <7vejempudf.fsf@gitster.siamese.dyndns.org>

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

> Steffen Prohaska <prohaska@zib.de> writes:
> ...
>> I think 3) is the interesting case.  "git push" should do
>> nothing by default.
>
> NO WAY.
>
> Making things cumbersome to everybody does not achieve anything
> useful except for a false sense of equality, does it?
>
> Drop that step (3).  That is not useful to anybody.

Thinking about it a bit more, I think my wording was a bit too
strong and needs clarifying explanations.

In a case like this, a fix of a "misfeature" for somebody is a
regression for somebody else.  Because there is no single right
default that is appropriate for everybody.

At least having _one_ default (and picking arbitrarily the
historical default as that one default) is better than no
default at all.  The former will not inconvenience anybody by
forcing what has been necessary from before.  The latter will
hurt the lucky ones whose preferred way happened to be the
historical default.

Keeping the status quo, however, will inconvinience unfortunate
people whose preferred way was not the historical default.
That's where we start to tackle the problem, by introducing the
configuration variable.

If we can come up with a way to tell projects that use the
workflow better served with --current, perhaps when a remote is
added to the repository (either the initial clone or "git remote
add") and/or when a new branch is created.  If we automatically
set up the configuration "push.defaultRefs = current" in such a
case, I suspect that we do not have to have the built-in default
(at least, the value of the built-in default would not matter
much).

^ permalink raw reply

* [PATCH Illustration] branch --contains=<commit>
From: Junio C Hamano @ 2007-11-19  8:04 UTC (permalink / raw)
  To: Pierre Habouzit; +Cc: git
In-Reply-To: <7v4pfircka.fsf@gitster.siamese.dyndns.org>

This renames the --with=<commit> option to --contains=<commit>, but still
supports the earlier --with=<commit> variant as a hidden synonym.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * An example usage of the ".help = NULL" member in the options
   structure.

 builtin-branch.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/builtin-branch.c b/builtin-branch.c
index fd36e4f..5dc0c57 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -562,9 +562,12 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		OPT_BOOLEAN( 0 , "color",  &branch_use_color, "use colored output"),
 		OPT_SET_INT('r', NULL,     &kinds, "act on remote-tracking branches",
 			REF_REMOTE_BRANCH),
-		OPT_CALLBACK(0, "with",    &with_commit, "commit",
+		OPT_CALLBACK(0, "contains",&with_commit, "commit",
 			     "print only branches that contain the commit",
 			     opt_parse_with_commit),
+		OPT_CALLBACK(0, "with",    &with_commit, "commit",
+			     NULL,
+			     opt_parse_with_commit),
 		OPT__ABBREV(&abbrev),
 
 		OPT_GROUP("Specific git-branch actions:"),
-- 
1.5.3.6.1788.gcc5c4

^ permalink raw reply related

* [PATCH] Further clarify clean.requireForce changes
From: Wincent Colaiuta @ 2007-11-19  8:06 UTC (permalink / raw)
  To: git; +Cc: gitster, Wincent Colaiuta

Mention the -f switch in the release notes for clean.requireForce to avoid
possible misunderstandings.

Signed-off-by: Wincent Colaiuta <win@wincent.com>
---
 Documentation/RelNotes-1.5.4.txt |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/RelNotes-1.5.4.txt b/Documentation/RelNotes-1.5.4.txt
index 96ec55e..a4a2a7f 100644
--- a/Documentation/RelNotes-1.5.4.txt
+++ b/Documentation/RelNotes-1.5.4.txt
@@ -33,8 +33,9 @@ Updates since v1.5.3
    too many loose objects.
 
  * You need to explicitly set clean.requireForce to "false" to allow
-   git-clean to do any damage (lack of the configuration variable
-   used to mean "do not require", but we now use the safer default).
+   git-clean without -f to do any damage (lack of the configuration
+   variable used to mean "do not require", but we now use the safer
+   default).
 
  * git-push has been rewritten in C.
 
-- 
1.5.3.5.737.gdee1b

^ permalink raw reply related

* Re: [PATCH 2/2] push: Add '--current', which pushes only the current branch
From: Steffen Prohaska @ 2007-11-19  8:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Johannes Schindelin
In-Reply-To: <7vejempudf.fsf@gitster.siamese.dyndns.org>


On Nov 19, 2007, at 8:27 AM, Junio C Hamano wrote:

> Steffen Prohaska <prohaska@zib.de> writes:
>
>>> Together with your [PATCH 1/2], I like the general direction
>>> these patckes are taking us, but it feels a bit too hasty.  I
>>> personally am not convinced that switching to --current for
>>> everybody is a good move.
>>>
>>>> ...
>>>
>> I think 3) is the interesting case.  "git push" should do
>> nothing by default.
>
> NO WAY.
>
> Making things cumbersome to everybody does not achieve anything
> useful except for a false sense of equality, does it?

If forces people to make a decision.  And it will hopefully reduce
the amount of questions
"What does '! [rejected] master -> master (non-fast forward)' mean"?
At least I am pretty sure that it will reduce the number of
times I'll be asked this question.  Because I'll ask people to
set a reasonable default.  But if they forget to do so, they'll be
reminded by "git push", before calling me.

And with "push.defaultRefs = matching" it's easy to tell push
once and forever which decision you made.


> Drop that step (3).  That is not useful to anybody.


 From you follow-up mail:

> Thinking about it a bit more, I think my wording was a bit too
> strong and needs clarifying explanations.
>
> In a case like this, a fix of a "misfeature" for somebody is a
> regression for somebody else.  Because there is no single right
> default that is appropriate for everybody.
>
> At least having _one_ default (and picking arbitrarily the
> historical default as that one default) is better than no
> default at all.  The former will not inconvenience anybody by
> forcing what has been necessary from before.  The latter will
> hurt the lucky ones whose preferred way happened to be the
> historical default.

Well, here we disagree.  My point is: if there's no single
right default, then it's better to force the user to make
the decision.


> Keeping the status quo, however, will inconvinience unfortunate
> people whose preferred way was not the historical default.
> That's where we start to tackle the problem, by introducing the
> configuration variable.
>
> If we can come up with a way to tell projects that use the
> workflow better served with --current, perhaps when a remote is
> added to the repository (either the initial clone or "git remote
> add") and/or when a new branch is created.  If we automatically
> set up the configuration "push.defaultRefs = current" in such a
> case, I suspect that we do not have to have the built-in default
> (at least, the value of the built-in default would not matter
> much).

That's beyond what I plan to implement.

Anyway, I'll not change the default behaviour.

But then, if I think a bit more, I don't see a point in
providing the push.defaultRefs configuration.  The default
_is_ and _will forever be_ '--matching'.  That's it.  If a
user wants to have something different, either he needs to
tell on the command line (e.g. '--all', '--current'); or he
can use the a remote.*.push configuration.

That's easier to explain than a configuration variable
"push.defaultRefs", which he can set to "current".  Or he is
allowed to set it to "matching" without triggering an error.  But
this wouldn't change anything, because it's the default anyway.
Then why should someone ever set it to "matching"?

And further, if we have no plans for changing the default, it's
useless to introduce a switch "--matching".  So if the plan is
to stick with the current default forever, then I'll withdraw
my patch that introduces "--matching".

What's left is a new switch "--current".  Less code, easy
to explain.

	Steffen

^ permalink raw reply

* Re: [PATCH v2] git-send-email: show all headers when sending mail
From: Junio C Hamano @ 2007-11-19  8:17 UTC (permalink / raw)
  To: David D. Kilzer; +Cc: git
In-Reply-To: <1195445695-27262-1-git-send-email-ddkilzer@kilzer.net>

Thanks.  Looks nice and obviously correct.

One thing that has been bugging me for a long time now stands
out like a sore thumb much more: empty Cc: is shown.

    $ git-send-email --dry-run --to=junio@my.isp.net 0001-branch-contains.txt
    Who should the emails appear to be from? [Junio C Hamano <gitster@pobox.com>]
    Emails will be sent from: Junio C Hamano <gitster@pobox.com>
    Message-ID to be used as In-Reply-To for the first email?
    0001-branch-contains.txt
    Dry-OK. Log says:
    Date: Mon, 19 Nov 2007 00:10:04 -0800
    Server: my.isp.net
    MAIL FROM:<gitster@pobox.com>
    RCPT TO:<junio@my.isp.net>
    From: Junio C Hamano <gitster@pobox.com>
    Subject: [PATCH] branch --contains=<commit>
    Cc:
    To: junkio@cox.net

    Result: OK

^ permalink raw reply

* Re: [PATCH 2/2] push: Add '--current', which pushes only the current branch
From: Junio C Hamano @ 2007-11-19  8:35 UTC (permalink / raw)
  To: Steffen Prohaska; +Cc: git, Johannes Schindelin
In-Reply-To: <53F12F4D-73C5-446E-9A97-9D2D4CA9DF9F@zib.de>

Steffen Prohaska <prohaska@zib.de> writes:

> What's left is a new switch "--current".  Less code, easy
> to explain.

But won't that force the "current" people always type that from
the command line, as your previous point was that your earlier
patch to say "remote.$there.push = HEAD" does not work that way?
If that configuration works as expected, then I'd 100% agree
that we would not need push.defaultRefs.  Either you do not have
"push" at all if your preference is --matching, or you do have
"push = HEAD" if your preference is --current.  But if it
doesn't (which was what I gathered from your earlier response),
having a configuration would help them, wouldn't it?

Changing the default, if it will ever happen, is _NOT_ to help
people who are already using git and want "current" NOW.  The
current users cannot be helped _unless_ we switch overnight, but
that is not an option as it introduces a regression to people's
established workflow.

Changing the default is to help new users who will come in the
future, if majority of the existing users find --current easier
to explain to new people _they_ need to train.

^ permalink raw reply

* Re: [PATCH] Fix start_command closing cmd->out/in regardless of cmd->close_out/in
From: Junio C Hamano @ 2007-11-19  8:46 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Ping Yin, git
In-Reply-To: <47413DB9.9030306@viscovery.net>

Johannes Sixt <j.sixt@viscovery.net> writes:

> Ping Yin schrieb:
>> When 'FILE *fp' is assigned to child_process.out and then start_command or
>> run_command is run, the standard output of the child process is expected to
>> be outputed to fp. However, sometimes fp is not expected to be closed since
>> further IO may be still performmed on fp.
>> ---
>>  run-command.c |    4 ----
>>  1 files changed, 0 insertions(+), 4 deletions(-)
>>
>> diff --git a/run-command.c b/run-command.c
>> index 476d00c..4e5f58d 100644
>> --- a/run-command.c
>> +++ b/run-command.c
>> @@ -115,13 +115,9 @@ int start_command(struct child_process *cmd)
>>   	if (need_in)
>>  		close(fdin[0]);
>> -	else if (cmd->in)
>> -		close(cmd->in);
>>   	if (need_out)
>>  		close(fdout[1]);
>> -	else if (cmd->out > 1)
>> -		close(cmd->out);
>>   	if (need_err)
>>  		close(fderr[1]);
>
> This is dangerous! You have to audit all current callers whether they
> close cmd->in or cmd->out (if they don't need the fd
> anymore).

I am reasonably sure that they are already relying on these auto
closing of the file descriptors.

Funny that somebody falls into the trap the day after we
discussed it on another thread.

^ permalink raw reply

* Re: [PATCH/RFC] parse-options: allow to define hidden synonym options
From: Pierre Habouzit @ 2007-11-19  9:22 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v4pfircka.fsf@gitster.siamese.dyndns.org>

[-- Attachment #1: Type: text/plain, Size: 4852 bytes --]

On lun, nov 19, 2007 at 06:09:41 +0000, Junio C Hamano wrote:
> By setting the help member to NULL, you can implement an option
> that is not shown in the "git-cmd -h" help output.  This is
> useful to support backward compatible synonyms without
> cluttering the help text.

  Sorry I'm not very present those days, but oh well...

  The idea looks nice, though if we are doing this, I'd like to see it
fix other problems at the same time. We sometimes have "internal"
commands switches and other cases of alias.

  I wonder if it wouldn't be better that we have some kind of --help-all
switch that would show them _all_ and a way (through another flag ?) to
hide those additional options by default.

  And defining an alias would then be:

  { OPTION_SOME_TYPE, 0, "alias-name", &some_value, "some-arg",
    "backward compatibility alias for \"foo\"", PARSE_OPT_HIDE, ... }

or:

  { OPTION_SOME_TYPE, 0, "plumbing-dark-think", &some_value, "some-arg",
    "internal plumbing switch used in", PARSE_OPT_HIDE, ... }


  I personnaly don't like that parse-opt sees more options than what it
says it sees, else with your patch, if the user gives ambiguous
abbreviated long switches, he'll get "--w is ambiguous, could be
`--whith` or `--wibble`" whereas he never knew that --with existed in
the first place.

  That gives something more along the lines of :

From 860a5bf335e44dd2bbe5f30620c99d174b697f69 Mon Sep 17 00:00:00 2001
From: Pierre Habouzit <madcoder@debian.org>
Date: Mon, 19 Nov 2007 10:21:44 +0100
Subject: [PATCH] parse-options: Allow to hide options from the default usage.

This is useful for backward-compatibility aliases, or very advanced command
line switches introduced for internal git usages and have no real use for a
user.

parse-options still shows them if the user asks for --help-all.

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 parse-options.c |   17 +++++++++++++++--
 parse-options.h |    3 +++
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index d3e608a..807e443 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -216,6 +216,9 @@ is_abbreviated:
 	return error("unknown option `%s'", arg);
 }
 
+static NORETURN void usage_with_options_internal(const char * const *,
+                                                 const struct option *, int);
+
 int parse_options(int argc, const char **argv, const struct option *options,
                   const char * const usagestr[], int flags)
 {
@@ -249,6 +252,8 @@ int parse_options(int argc, const char **argv, const struct option *options,
 			break;
 		}
 
+		if (!strcmp(arg + 2, "help-all"))
+			usage_with_options_internal(usagestr, options, 1);
 		if (!strcmp(arg + 2, "help"))
 			usage_with_options(usagestr, options);
 		if (parse_long_opt(&args, arg + 2, options))
@@ -263,8 +268,8 @@ int parse_options(int argc, const char **argv, const struct option *options,
 #define USAGE_OPTS_WIDTH 24
 #define USAGE_GAP         2
 
-void usage_with_options(const char * const *usagestr,
-                        const struct option *opts)
+void usage_with_options_internal(const char * const *usagestr,
+                                 const struct option *opts, int full)
 {
 	fprintf(stderr, "usage: %s\n", *usagestr++);
 	while (*usagestr && **usagestr)
@@ -285,6 +290,8 @@ void usage_with_options(const char * const *usagestr,
 				fprintf(stderr, "%s\n", opts->help);
 			continue;
 		}
+		if (!full & (opts->flags & PARSE_OPT_HIDDEN))
+			continue;
 
 		pos = fprintf(stderr, "    ");
 		if (opts->short_name)
@@ -335,6 +342,12 @@ void usage_with_options(const char * const *usagestr,
 	exit(129);
 }
 
+void usage_with_options(const char * const *usagestr,
+                        const struct option *opts)
+{
+	usage_with_options_internal(usagestr, opts, 0);
+}
+
 /*----- some often used options -----*/
 #include "cache.h"
 
diff --git a/parse-options.h b/parse-options.h
index a8760ac..102ac31 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -24,6 +24,7 @@ enum parse_opt_option_flags {
 	PARSE_OPT_OPTARG  = 1,
 	PARSE_OPT_NOARG   = 2,
 	PARSE_OPT_NONEG   = 4,
+	PARSE_OPT_HIDDEN  = 8,
 };
 
 struct option;
@@ -57,6 +58,8 @@ typedef int parse_opt_cb(const struct option *, const char *arg, int unset);
  *   PARSE_OPT_OPTARG: says that the argument is optionnal (not for BOOLEANs)
  *   PARSE_OPT_NOARG: says that this option takes no argument, for CALLBACKs
  *   PARSE_OPT_NONEG: says that this option cannot be negated
+ *   PARSE_OPT_HIDDEN this option is skipped in the default usage, showed in
+ *                    the long one.
  *
  * `callback`::
  *   pointer to the callback to use for OPTION_CALLBACK.
-- 
1.5.3.5.1795.g5421e-dirty


[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply related

* Re: [PATCH 2/2] push: Add '--current', which pushes only the current branch
From: Andreas Ericsson @ 2007-11-19  9:24 UTC (permalink / raw)
  To: Steffen Prohaska; +Cc: Junio C Hamano, git, Johannes Schindelin
In-Reply-To: <EA5C3227-12E1-43C4-96E8-43BABF26792B@zib.de>

Steffen Prohaska wrote:
> 
> On Nov 19, 2007, at 2:28 AM, Junio C Hamano wrote:
> 
> 
> 
>> I do not think it is "Often you want" that makes it awkward.
>>
>> Instead, the awkward case is if you do the "only the current"
>> push NOT often enough.  If it is often enough, you set the
>> configuration once and the awkwardness is behind you.
>>
>> If however it is not often enough, you cannot afford to have the
>> configuration above, because that would force you to tell from
>> the command line which branches, not just the current one, to
>> push, and that is inconvenient because it is not rare enough.
> 
> Will try to rephrase the commit message.
> 
> 
>> Together with your [PATCH 1/2], I like the general direction
>> these patckes are taking us, but it feels a bit too hasty.  I
>> personally am not convinced that switching to --current for
>> everybody is a good move.
>>
>>> ...
>>> Maybe in two years (that's twice an eternity in git time scales):
>>>
>>> 4) make "git push --current" the default.
>>
>> If these, both the uncertainly expressed by "Maybe" and "twice
>> an eternity" are true, which they are, the new warning in the
>> current patch are inappropriate.  Many people's settings depend
>> on a working "push the matching refs" behaviour, and we need a
>> very good excuse to annoy the existing happy users with such a
>> warning.
> 
> I think 3) is the interesting case.  "git push" should do
> nothing by default.  Either you can configure "git push" to do
> something by setting a remote.$remote.push line or you need
> to provide a command line switch.  But if you do not tell
> explicitly what you want, "git push" will not do anything
> for you.
> 

I'd really, really hate that. I often have changes on several branches
when I push. I like the behaviour as it is today.

> 
>> Remember, how much vocal the dissenters might have been on the
>> list in the recent discussions, we need to consider the needs of
>> the silent majority that has been content with the current
>> behaviour for a long time.
>>
>> The "warning" to annoy them may be a way to get their attention
>> and get them involved in a discussion to decide what the default
>> should be.  But changing the default without giving the people
>> who do not like the _new_ default a way to avoid inconvenience
>> of always typing --matching or --current is not nice.  And
>> honestly, I do not think there is one single default that is
>> good for everybody.
> 
> Personally, I'd switch to the do-nothing default immediately.
> But you are right.  More work is needed to have a smooth transition.
> 
> 
>> We should be doing better.
>>
>> A smoother transition route would be:
>>
>>  - Keep "matching" the built-in default for now;
>>
>>  - Take your patches (but drop "warning" bits at this point) to
>>    introduce 'matching' and 'current' behaviours, and a way to
>>    override the built-in default from the command line;
>>
>>  - Introduce a configuration 'push.defaultRefs' ('matching' or
>>    'current') to override that built-in default, so people who
>>    prefer 'current' can override the built-in default, without
>>    having to type --current every time.
> 
> Sounds like a plan.
> 
> If we have the configuration variable, maybe we could switch
> off the default behaviour immediately.  Setting a single global
> config variable once would be sufficient to get it back.  So,
> we could change the default and print a recommendation to run
> 'git config --global push.defaultRefs matching' to get it back.
> 

Ugh. People who neither know nor care about git development will
wonder why the hell they now have to tell git something in order
for it to do something it's always done anyway. The majority of
git users never read release-notes. They just do "yum update" and
then go about their business the same way they've always done.

Newcomers that obviously have no such configuration will wonder
why they're getting warnings from using the standard command-set.

> ...
> 
>> After all that happens, we can start discussing what the
>> built-in default should be.  When it is changed after the
>> discussion concludes (which may never happen), people who want
>> to keep 'matching' behaviour would have had the configuration
>> mechanism to override that built-in default for some time during
>> the discussion period.  So the beginning of that discussion
>> period is when we should start talking about "We might change
>> the default soon; set the configuration to your liking if you do
>> not want to get affected" in the warning.
> 
> ... And we'd not even start the discussion.  Because there's no
> need to.  Every user should make a choice, once.  We do not
> provide a default (which obviously will trigger another discussion ;)
> 

If the default's to be changed, making it default to no-op is really
the only sensible thing to do. Otherwise I'm guessing a lot of people
that actually count on the current behaviour will get quite vexed, and
--current is definitely not the universally correct default thing to do.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ permalink raw reply

* Re: [PATCH 2/2] push: Add '--current', which pushes only the current branch
From: Andreas Ericsson @ 2007-11-19  9:27 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Steffen Prohaska, git, Johannes Schindelin
In-Reply-To: <7v4pfiptb5.fsf@gitster.siamese.dyndns.org>

Junio C Hamano wrote:
> 
> If we can come up with a way to tell projects that use the
> workflow better served with --current, perhaps when a remote is
> added to the repository (either the initial clone or "git remote
> add") and/or when a new branch is created.  If we automatically
> set up the configuration "push.defaultRefs = current" in such a
> case, I suspect that we do not have to have the built-in default
> (at least, the value of the built-in default would not matter
> much).

So when someone who's been using git of today adds a new remote
with a newer git, all of a sudden the default push behaviour
changes for that repo?

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ permalink raw reply

* Re: [PATCH] Improve description of git-branch -d and -D in man page.
From: Sergei Organov @ 2007-11-19  9:49 UTC (permalink / raw)
  To: Jan Hudec; +Cc: Jakub Narebski, Steffen Prohaska, git, Junio C Hamano
In-Reply-To: <20071117195144.GF5198@efreet.light.src>

Jan Hudec <bulb@ucw.cz> writes:
> Some users expect that deleting a remote-tracking branch would prevent
> fetch from creating it again, so be explcit about that it's not the case.
> Also be a little more explicit about what fully merged means.
>
> Signed-off-by: Jan Hudec <bulb@ucw.cz>
> ---
>
> On Sat, Nov 17, 2007 at 20:12:56 +0100, Jan Hudec wrote:
>> On Tue, Nov 13, 2007 at 20:58:20 +0300, osv@javad.com wrote:
>> > <quote Documentation/git-branch.txt>
>> > Delete unneeded branch::
>> > +
>> > ------------
>> > $ git clone git://git.kernel.org/.../git.git my.git
>> > $ cd my.git
>> > $ git branch -d -r origin/todo origin/html origin/man   <1>
>> > $ git branch -D test                                    <2>
>> > ------------
>> > +
>> > <1> Delete remote-tracking branches "todo", "html", "man"
>> > </quote>
>> > 
>> > That's *exactly* what I did! And it *doesn't work*! Well, it does delete
>> > the branches, but they are automagically re-created on the next fetch,
>> > so "deleting" them this way is useless.
>> 
>> Of course it *does* work. It *deletes* the branches. There is not a single
>> word about stopping fetch getting them!
>> 
>> Obviously given that the example is slightly contrived, it should really
>> be mentioned that it does not affect fetch at all.
>
> Would this make the description obvious enough?

Yes, I think now it describes the actual behavior much much better, --
thanks.

-- 
Sergei.

^ permalink raw reply

* Re: [PATCH 2/2] push: Add '--current', which pushes only the current branch
From: Steffen Prohaska @ 2007-11-19  9:54 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Johannes Schindelin
In-Reply-To: <7vk5oeocnr.fsf@gitster.siamese.dyndns.org>


On Nov 19, 2007, at 9:35 AM, Junio C Hamano wrote:

> Steffen Prohaska <prohaska@zib.de> writes:
>
>> What's left is a new switch "--current".  Less code, easy
>> to explain.
>
> But won't that force the "current" people always type that from
> the command line, as your previous point was that your earlier
> patch to say "remote.$there.push = HEAD" does not work that way?
> If that configuration works as expected, then I'd 100% agree
> that we would not need push.defaultRefs.  Either you do not have
> "push" at all if your preference is --matching, or you do have
> "push = HEAD" if your preference is --current.  But if it
> doesn't (which was what I gathered from your earlier response),
> having a configuration would help them, wouldn't it?

My main point is that I want to have something that _always_
works as expected without manually tweaking the configuration
variables.  In a setting with shared repos, "git push" doesn't.
Sooner or later it will complain about refusing an update
because of non-fast-forward.  The only thing that currently
works it "git push $correct-remote $correct-branch", but this
depends on the local configuration and on the branch you're on.

"git push --current" would always work as expected; without
setting any configuration variable.

I can tell my users that their workflow is

	git checkout foo
	git pull
	work work work ...
	git push --current

That simple.  I'm fine with that.


> Changing the default, if it will ever happen, is _NOT_ to help
> people who are already using git and want "current" NOW.  The
> current users cannot be helped _unless_ we switch overnight, but
> that is not an option as it introduces a regression to people's
> established workflow.
>
> Changing the default is to help new users who will come in the
> future, if majority of the existing users find --current easier
> to explain to new people _they_ need to train.

I'll not change the default.  I'll only add the --current switch.

	Steffen

^ permalink raw reply

* Re: [BUG] encoding problem with format-patch + send-email
From: Uwe Kleine-König @ 2007-11-19 10:49 UTC (permalink / raw)
  To: Jeff King
  Cc: Junio C Hamano, git, Brian Swetland, Russell King - ARM Linux,
	Nicolas Pitre
In-Reply-To: <7vlk8xwvbu.fsf@gitster.siamese.dyndns.org>

Hello Jeff

Brian sent another mail to the linux-arm-kernel mailing list, now
spotting:

	Content-Type: text/plain; charset=UTF-8

but no Content-Transfer-Encoding:.  This yield a 7bit mail with 8bit
characters.

I think we should add

	Content-Transfer-Encoding: 8bit

, too.

Best regards
Uwe

-- 
Uwe Kleine-König

$ dc << EOF
[d1-d1<a]sa99d1<a1[rdn555760928P*pz1<a]salax
EOF

^ permalink raw reply

* [PATCH] gitview: import only one of gtksourceview and gtksourceview2
From: Anton Gyllenberg @ 2007-11-19 10:37 UTC (permalink / raw)
  To: git, gitster; +Cc: Anton Gyllenberg

Importing both gtksourceview and gtksourceview2 will make python segfault
on my system (ubuntu 7.10). Change so that gtksourceview is only imported
if importing gtksourceview2 fails. This should be safe as gtksourceview
is only used if gtksourceview2 is not available.

Signed-off-by: Anton Gyllenberg <anton@iki.fi>
---
 contrib/gitview/gitview |   17 +++++++----------
 1 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/contrib/gitview/gitview b/contrib/gitview/gitview
index 449ee69..4c99dfb 100755
--- a/contrib/gitview/gitview
+++ b/contrib/gitview/gitview
@@ -27,20 +27,17 @@ import math
 import string
 import fcntl
 
+have_gtksourceview2 = False
+have_gtksourceview = False
 try:
     import gtksourceview2
     have_gtksourceview2 = True
 except ImportError:
-    have_gtksourceview2 = False
-
-try:
-    import gtksourceview
-    have_gtksourceview = True
-except ImportError:
-    have_gtksourceview = False
-
-if not have_gtksourceview2 and not have_gtksourceview:
-    print "Running without gtksourceview2 or gtksourceview module"
+    try:
+        import gtksourceview
+        have_gtksourceview = True
+    except ImportError:
+        print "Running without gtksourceview2 or gtksourceview module"
 
 re_ident = re.compile('(author|committer) (?P<ident>.*) (?P<epoch>\d+) (?P<tz>[+-]\d{4})')
 
-- 
1.5.3.2

^ 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