Git development
 help / color / mirror / Atom feed
* Re: "git branch HEAD" dumps core when on detached head (NULL pointer dereference)
From: Duy Nguyen @ 2013-02-21 13:32 UTC (permalink / raw)
  To: Per Cederqvist; +Cc: git
In-Reply-To: <51261FF9.2090304@opera.com>

On Thu, Feb 21, 2013 at 8:24 PM, Per Cederqvist <cederp@opera.com> wrote:
> Sorry, but isolating the issue reporting it here is about as much time
> as I can spend on this issue. Learning the coding standard of Git and
> how to write test cases is not something I'm prepared to do, at least
> not at the moment. I hope there is a place for users and reporters of
> bugs in the Git community.

Sure. No problem. I just thought you might want to finish it off. I'll
look into it.
-- 
Duy

^ permalink raw reply

* [PATCH] branch: segfault fixes and validation
From: Nguyễn Thái Ngọc Duy @ 2013-02-21 14:18 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Per Cederqvist,
	Nguyễn Thái Ngọc Duy
In-Reply-To: <512612AD.4000609@opera.com>

branch_get() can return NULL (so far on detached HEAD only) but some
code paths in builtin/branch.c cannot deal with that and cause
segfaults. Fix it.

While at there, make sure to bail out when the user gives 2 or more
arguments, but only the first one is processed.

Reported-by: Per Cederqvist <cederp@opera.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/branch.c  | 20 ++++++++++++++++++++
 t/t3200-branch.sh | 21 +++++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/builtin/branch.c b/builtin/branch.c
index 6371bf9..c1d688e 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -889,6 +889,13 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	} else if (new_upstream) {
 		struct branch *branch = branch_get(argv[0]);
 
+		if (argc > 1)
+			die(_("too many branches to set new upstream"));
+
+		if (!branch)
+			die(_("could not figure out the branch name from '%s'"),
+			    argc == 1 ? argv[0] : "HEAD");
+
 		if (!ref_exists(branch->refname))
 			die(_("branch '%s' does not exist"), branch->name);
 
@@ -901,6 +908,13 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		struct branch *branch = branch_get(argv[0]);
 		struct strbuf buf = STRBUF_INIT;
 
+		if (argc > 1)
+			die(_("too many branches to unset upstream"));
+
+		if (!branch)
+			die(_("could not figure out the branch name from '%s'"),
+			    argc == 1 ? argv[0] : "HEAD");
+
 		if (!branch_has_merge_config(branch)) {
 			die(_("Branch '%s' has no upstream information"), branch->name);
 		}
@@ -916,6 +930,12 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		int branch_existed = 0, remote_tracking = 0;
 		struct strbuf buf = STRBUF_INIT;
 
+		if (!strcmp(argv[0], "HEAD"))
+			die(_("it does not make sense to create 'HEAD' manually"));
+
+		if (!branch)
+			die(_("could not figure out the branch name from '%s'"), argv[0]);
+
 		if (kinds != REF_LOCAL_BRANCH)
 			die(_("-a and -r options to 'git branch' do not make sense with a branch name"));
 
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index f3e0e4a..12f1e4a 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -42,6 +42,10 @@ test_expect_success \
     'git branch a/b/c should create a branch' \
     'git branch a/b/c && test_path_is_file .git/refs/heads/a/b/c'
 
+test_expect_success \
+    'git branch HEAD should fail' \
+    'test_must_fail git branch HEAD'
+
 cat >expect <<EOF
 $_z40 $HEAD $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150200 +0000	branch: Created from master
 EOF
@@ -388,6 +392,14 @@ test_expect_success \
     'git tag foobar &&
      test_must_fail git branch --track my11 foobar'
 
+test_expect_success '--set-upstream-to fails on multiple branches' \
+    'test_must_fail git branch --set-upstream-to master a b c'
+
+test_expect_success '--set-upstream-to fails on detached HEAD' \
+    'git checkout HEAD^{} &&
+     test_must_fail git branch --set-upstream-to master &&
+     git checkout -'
+
 test_expect_success 'use --set-upstream-to modify HEAD' \
     'test_config branch.master.remote foo &&
      test_config branch.master.merge foo &&
@@ -417,6 +429,15 @@ test_expect_success 'test --unset-upstream on HEAD' \
      test_must_fail git branch --unset-upstream
 '
 
+test_expect_success '--unset-upstream should fail on multiple branches' \
+    'test_must_fail git branch --unset-upstream a b c'
+
+test_expect_success '--unset-upstream should fail on detached HEAD' \
+    'git checkout HEAD^{} &&
+     test_must_fail git branch --unset-upstream &&
+     git checkout -
+'
+
 test_expect_success 'test --unset-upstream on a particular branch' \
     'git branch my15
      git branch --set-upstream-to master my14 &&
-- 
1.8.1.2.536.gf441e6d

^ permalink raw reply related

* [PATCH] archive: let remote clients get reachable commits
From: Sergey Segeev @ 2013-02-21 14:24 UTC (permalink / raw)
  To: git, Jeff King, Junio C Hamano; +Cc: Sergey Segeev

Some time we need to get valid commit without a ref but with proper
tree-ish, now we can't do that.

This patch allow upload-archive's to use reachability checking
rather than checking that is a ref. This means a remote client can
fetch a tip of any valid sha1 or tree-ish.
---
 archive.c           | 24 +++++++++++++-----------
 t/t5000-tar-tree.sh |  2 ++
 2 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/archive.c b/archive.c
index 93e00bb..0a48985 100644
--- a/archive.c
+++ b/archive.c
@@ -5,6 +5,7 @@
 #include "archive.h"
 #include "parse-options.h"
 #include "unpack-trees.h"
+#include "refs.h"
 
 static char const * const archive_usage[] = {
 	N_("git archive [options] <tree-ish> [<path>...]"),
@@ -241,6 +242,13 @@ static void parse_pathspec_arg(const char **pathspec,
 	}
 }
 
+static int check_reachable(const char *refname, const unsigned char *sha1,
+		int flag, void *cb_data)
+{
+
+	return in_merge_bases(cb_data, lookup_commit_reference_gently(sha1, 1));
+}
+
 static void parse_treeish_arg(const char **argv,
 		struct archiver_args *ar_args, const char *prefix,
 		int remote)
@@ -252,22 +260,16 @@ static void parse_treeish_arg(const char **argv,
 	const struct commit *commit;
 	unsigned char sha1[20];
 
-	/* Remotes are only allowed to fetch actual refs */
-	if (remote) {
-		char *ref = NULL;
-		const char *colon = strchr(name, ':');
-		int refnamelen = colon ? colon - name : strlen(name);
-
-		if (!dwim_ref(name, refnamelen, sha1, &ref))
-			die("no such ref: %.*s", refnamelen, name);
-		free(ref);
-	}
-
 	if (get_sha1(name, sha1))
 		die("Not a valid object name");
 
 	commit = lookup_commit_reference_gently(sha1, 1);
 	if (commit) {
+
+		/* Remotes are only allowed to fetch actual objects */
+		if (remote && !for_each_ref(check_reachable, (void *)commit))
+			die("Not a valid object name");
+
 		commit_sha1 = commit->object.sha1;
 		archive_time = commit->date;
 	} else {
diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
index e7c240f..fc35406 100755
--- a/t/t5000-tar-tree.sh
+++ b/t/t5000-tar-tree.sh
@@ -194,6 +194,8 @@ test_expect_success 'clients cannot access unreachable commits' '
 	sha1=`git rev-parse HEAD` &&
 	git reset --hard HEAD^ &&
 	git archive $sha1 >remote.tar &&
+	git archive --remote=. $sha1 >remote.tar &&
+	git tag -d unreachable &&
 	test_must_fail git archive --remote=. $sha1 >remote.tar
 '
 
-- 
1.8.1.4

^ permalink raw reply related

* Re: Google Summer of Code 2013 (GSoC13)
From: Carlos Martín Nieto @ 2013-02-21 14:29 UTC (permalink / raw)
  To: Michael Schubert
  Cc: Jeff King, Thomas Rast, git, Shawn Pearce, Jakub Narebski,
	Christian Couder, Pat Thoyts, Paul Mackerras, Thomas Gummerer,
	David Michael Barr, Ramkumar Ramachandra, Jens Lehmann,
	Nguyen Thai Ngoc Duy, Vicent Marti
In-Reply-To: <51252877.5000808@schu.io>

Michael Schubert <schu@schu.io> writes:

> On 02/18/2013 06:42 PM, Jeff King wrote:
>> 
>> I will do it again, if people feel strongly about Git being a part of
>> it. However, I have gotten a little soured on the GSoC experience. Not
>> because of anything Google has done; it's a good idea, and I think they
>> do a fine of administering the program. But I have noticed that the work
>> that comes out of GSoC the last few years has quite often not been
>> merged, or not made a big impact in the codebase, and nor have the
>> participants necessarily stuck around.
>> 
>> And I do not want to blame the students here (some of whom are on the cc
>> list :) ). They are certainly under no obligation to stick around after
>> GSoC ends, and I know they have many demands on their time. But I am
>> also thinking about what Git wants to get out of GSoC (and to my mind,
>> the most important thing is contributors).
>
> Speaking of libgit2:
>
> Git provided the libgit2 project with a slot each of the last three GSOC.
> The contributions made by the former students (Disclaimer: one of them
> speaking) have been quite important for libgit2 and all three students
> are still involved. Each project was an important push towards building
> a new, feature complete Git library.

Right, speaking of libgit2. GSoC has been very successful (as Michael,
I'm also somewhat biased) for libgit2. This happens outside of the git
ML so it probably hasn't gotten as much visibility here.

I believe it's partly because there were still larger parts where most
of the work was technical and the goal was quite clear, as git had
already set the standard and expectations and the decisions had to be
mostly about how to implement it in a way that makes sense for a
library, rather than it living inside of git, which is not always easy,
but you can experiment with different uses of it.

It's also possible that part of the success was the fact that we were
already acquainted with the "release often and early" policy, as we'd
been involved with FLOSS for a while already.

The current gaping hole in libgit2 is the lack of merge support, which
is the last hurdle to a stable 1.0 release. There is already some work
by Edward Thomson that needs to be reviewed and merged. I'm not sure
that there's enough for a whole summer there, but you could throw in the
review and merge of another missing feature, which is making the
reference storage generic, as it currently only supports the
git-compatible file-based one. There's other nice-to-have things like
thin-pack support that you could use to fill up a summer, though I'm not
sure that goes with the spirit of the programme.

Something else that needs love is Git for Windows. I believe both git
and libgit2 would benefit a lot from a project to take some parts of git
that are implemented in a scripting language and port them to use
libgit2. As Git for Windows needs to ship a ton of dependencies anyway,
using a pre-1.0 library wouldn't be an issue and it can be used to
experiment with an eventual porting of git to be one user of libgit2
rather than a completely different implementation. The more immediate
benefit for Git for Windows would be less reliance on languages that are
awkward to use on Windows and need their own environment. Mentoring from
the libgit2 probably wouldn't be much of an issue to organise, though
I'm not sure if the GfW team would have time for the part that involves
its peculiarities.

So there's a couple of projects that could be done with some realistic
chance of being merged upstream, as they'd be technical, as long as we
do tell the student to send small units of work to be reviewed often.

Cheers,
   cmn

^ permalink raw reply

* Re: Google Summer of Code 2013 (GSoC13)
From: Thomas Rast @ 2013-02-21 15:41 UTC (permalink / raw)
  To: Shawn Pearce
  Cc: Jeff King, git, Jakub Narebski, Christian Couder, Pat Thoyts,
	Paul Mackerras, Carlos Martín Nieto, Thomas Gummerer,
	David Michael Barr, Ramkumar Ramachandra, Jens Lehmann,
	Nguyen Thai Ngoc Duy
In-Reply-To: <CAJo=hJvknVedGba5OxjjvZi2=JZyDuDoP2tD+LKQKdZNJ4NcsA@mail.gmail.com>

Shawn Pearce <spearce@spearce.org> writes:

> On Mon, Feb 18, 2013 at 9:42 AM, Jeff King <peff@peff.net> wrote:
>> On Mon, Feb 18, 2013 at 06:23:01PM +0100, Thomas Rast wrote:
>>
>>> * We need an org admin.  AFAIK this was done by Peff and Shawn in
>>>   tandem last year.  Would you do it again?
>>
>> I will do it again, if people feel strongly about Git being a part of
>> it. However, I have gotten a little soured on the GSoC experience. Not
>> because of anything Google has done; it's a good idea, and I think they
>> do a fine of administering the program. But I have noticed that the work
>> that comes out of GSoC the last few years has quite often not been
>> merged, or not made a big impact in the codebase, and nor have the
>> participants necessarily stuck around.
>
> This.
>
> I actually think Git should take a year off from GSoC and not
> participate. Consequently I will not be volunteering as backup org
> admin.

Fair enough.  But I think if that's the decision (and modulo libgit2
praise, it seems to be pretty much the consensus?), we should probably
have some Idea why we are doing it?

You wrote:

> Git has been involved since 2007. In all of that time we have had very
> few student projects merge successfully into their upstream project
> (e.g. git.git, JGit or libgit2) before the end of GSoC. Even fewer
> students have stuck around and remained active contributors. When I
> look at the amount of effort we contributors put into GSoC, I think we
> are misusing our limited time and resources. The intention of the GSoC
> program is to grow new open source developers, and increase our
> community of contributors. Somehow I think Git is falling well short
> of its potential here. This is especially true if you compare Git's
> GSoC program to some other equally long-running GSoC programs.

If that's the outset (and it's certainly true for a lot of the
projects), aren't the options (not limited to just one):

* We have some discussion about why we fail, what to do better, etc. and
  hopefully also manage to clean up some old projects and get them
  included.  That way we can learn something from it.

* We try to look at how more successful communities are doing it
  (e.g. there were some posts about how KDE bumped their student
  retention rate).

* We try to "mentor" some projects that aren't GSoC sponsered.  That way
  we can hope to gain mentoring experience.

I'm not very optimistic about any of these, as:

- There weren't any in-depth discussions post-GSoC to analyze what went
  wrong.

- Contributor time is so limited that we're usually short on reviews.
  Adding "mentoring for the sake of trying it" to duties isn't very
  promising.

Thus I'm a bit afraid that after a year off, we won't have learned
anything new.  To the contrary, some previous mentors/students will
inevitably have disappeared into the mists of time, and with them their
experience.  Unless we do something about it, next year we'll be in an
_even worse_ position than this year.

I'm mildly pessimistic about "doing something" over the list, but
perhaps we can have an extended discussion at git-merge provided enough
of you show up there?  I can try to prepare some material.


(Maybe we should all make a bunch of clones of ourselves.  We can put
one copy each into a room so they can figure out GSoC, and have another
group doing our favorite git hacking while we're at it.)

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

^ permalink raw reply

* Re: [PATCH] archive: let remote clients get reachable commits
From: Jeff King @ 2013-02-21 15:52 UTC (permalink / raw)
  To: Sergey Segeev; +Cc: git, Junio C Hamano
In-Reply-To: <1361456643-51851-1-git-send-email-gurugray@yandex.ru>

On Thu, Feb 21, 2013 at 06:24:03PM +0400, Sergey Segeev wrote:

> Some time we need to get valid commit without a ref but with proper
> tree-ish, now we can't do that.
> 
> This patch allow upload-archive's to use reachability checking
> rather than checking that is a ref. This means a remote client can
> fetch a tip of any valid sha1 or tree-ish.

That sounds like a good goal, but...

> @@ -252,22 +260,16 @@ static void parse_treeish_arg(const char **argv,
>  	const struct commit *commit;
>  	unsigned char sha1[20];
>  
> -	/* Remotes are only allowed to fetch actual refs */
> -	if (remote) {
> -		char *ref = NULL;
> -		const char *colon = strchr(name, ':');
> -		int refnamelen = colon ? colon - name : strlen(name);
> -
> -		if (!dwim_ref(name, refnamelen, sha1, &ref))
> -			die("no such ref: %.*s", refnamelen, name);
> -		free(ref);
> -	}

The point of this was to allow "commit:path" syntax, and check that
commit pointed to a ref. The natural extension would be to also check
that the commit part is reachable.

I think it is also not sufficient to just check whether the left-hand
side of the colon is a reachable commit. You would also want to handle
non-commits which are directly pointed-to by a ref or its tag (e.g.,
think of a tag pointing directly to a tree, like the v2.6.11 tag in the
linux repo).

Your check...

>  	commit = lookup_commit_reference_gently(sha1, 1);
>  	if (commit) {
> +
> +		/* Remotes are only allowed to fetch actual objects */
> +		if (remote && !for_each_ref(check_reachable, (void *)commit))
> +			die("Not a valid object name");
> +
>  		commit_sha1 = commit->object.sha1;
>  		archive_time = commit->date;
>  	} else {

...will do nothing if we do not have a commit reference (e.g., an arbitrary
sha1, or commit:path syntax). We follow the "else" of this branch, and
allow arbitrary sha1's to be fetched (like "unreachable_sha1:subdir").

-Peff

^ permalink raw reply

* Re: "git branch --contains x y" creates a branch instead of checking containment
From: Jeff King @ 2013-02-21 15:58 UTC (permalink / raw)
  To: Per Cederqvist; +Cc: git
In-Reply-To: <51261A6B.5020909@opera.com>

On Thu, Feb 21, 2013 at 02:00:27PM +0100, Per Cederqvist wrote:

> That command does something completely different,
> though. The "--contains x" part is silently ignored,
> so it creates a branch named "y" pointing at HEAD.
> 
> Tested in git 1.8.1.1 and 1.8.1.4.
> 
> In my opinion, there are two ways to fix this:
> 
>  - change the "git branch" implementation so
>    that --contains implies --list.

I think that is the best option, too. In fact, I even wrote a patch. :)

It's d040350 (branch: let branch filters imply --list, 2013-01-31), and
it's already in v1.8.2-rc0.

-Peff

^ permalink raw reply

* Re: "git branch --contains x y" creates a branch instead of checking containment
From: Per Cederqvist @ 2013-02-21 16:05 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20130221155827.GA20640@sigill.intra.peff.net>

On 02/21/13 16:58, Jeff King wrote:
> On Thu, Feb 21, 2013 at 02:00:27PM +0100, Per Cederqvist wrote:
>
>> That command does something completely different,
>> though. The "--contains x" part is silently ignored,
>> so it creates a branch named "y" pointing at HEAD.
>>
>> Tested in git 1.8.1.1 and 1.8.1.4.
>>
>> In my opinion, there are two ways to fix this:
>>
>>   - change the "git branch" implementation so
>>     that --contains implies --list.
>
> I think that is the best option, too. In fact, I even wrote a patch. :)
>
> It's d040350 (branch: let branch filters imply --list, 2013-01-31), and
> it's already in v1.8.2-rc0.
>
> -Peff

Great! Thanks for the quick fix of my bug report. Negative response
time... not bad. Not bad at all. :-)

     /ceder

^ permalink raw reply

* Re: RFD: concatening textconv filters
From: Junio C Hamano @ 2013-02-21 17:39 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: Git Mailing List
In-Reply-To: <5125F6CF.50105@drmicha.warpmail.net>

Michael J Gruber <git@drmicha.warpmail.net> writes:

> ... but wondering
> whether we could and should support concatenating filters by either
>
> - making it easy to request it (say by setting
> "filter.gpgtopdftotext.textconvpipe" to a list of textconv filter names
> which are to be applied in sequence)
>
> or
>
> - doing it automatically (remove the pattern which triggered the filter,
> and apply attributes again to the resulting pathspec)

I think what you are getting at is to start from something like this:

	= .gitattributes =
	*.gpg	diff=gpg
        *.pdf	diff=pdf

and have "git cat-file --textconv frotz.pdf.gpg" (and other textconv
users) notice that:

 (1) The path matches "*.gpg" pattern, and calls for
     diff.gpg.textconv conversion.  This already happens in the
     current system.

 (2) After stripping the "*.gpg" pattern (i.e. look at the part of
     the path that matched the wildcard part * in the attribute
     selector), notice that the remainder, "frotz.pdf", could match
     the "*.pdf" pattern.  The output from the previous filter could
     be treated as if it were a blob that is stored in that path.

A few issues that need to be addressed while designing this feature
that come to my mind at random are:

 * This seems to call for a new concept, but what exactly is that
   concept?  Your RFD sounds as if you desire a "cascadable
   textconv", but it may be of a somewhat larger scope, "virtual
   blob at a virtual path", which the last sentence in (2) above
   seems to suggest.

 * What is this new concept an attribute to?  If we express this as
   "the textconv conversion result of any path with attribute
   diff=gpg can be treated as the contents of a virtual blob", then
   we are making it an attribute of the gpg "type", i.e.

	= .git/config =
	[diff "gpg"]
		textconv = gpg -v
		textconvProducesVirtualBlob = yes

   To me, that seems sufficient for this particular application at
   the first glance, but are there other attributes that may want to
   produce such virtual blob for further processing?  Is limiting
   this to textconv too restrictive?  I do not know.

 * What is the rule to come up with the "virtual path" to base the
   attribute look-up on for the "virtual blob contents"?  In the
   above example, the pattern was a simple "*.gpg", and we used a
   naïve "what did the asterisk match?", but imagine a case where
   you have some documents that you want to do "gpg -v" and some you
   don't.  You express this by having the former class of files
   named with "conv-" prefix, or some convention that is convenient
   for you.

   Your .gitattributes may say something like:

	= .gitattributes =
        conv-*.gpg	diff=gpg

   When deciding what attributes to use to further process the
   result of conversion (i.e. "virtual blob contents") for
   conv-frotz.pdf.gpg, what virtual path should we use?  Should we
   use "conv-frotz.pdf", or just "frotz.pdf"?

   "The difference does not matter--either would work" is not a
   satisfactory answer, once you consider that you may want to have
   two or more classes of pdf files that you may want to treat
   differently, just like you did for gpg encrypted files in this
   example setting.  It seems to suggest that we want to use
   conv-frotz.pdf as the virtual path, but how would we derive that
   from the pattern "conv-*.gpg" and path "conv-frotz.pdf.gpg"?  It
   appears to me that you would need a way to say between the two
   literal parts in the pattern, "conv-" part needs to be kept but
   ".gpg" part needs to be stripped when forming the result.

^ permalink raw reply

* Re: [PATCH] branch: segfault fixes and validation
From: Junio C Hamano @ 2013-02-21 17:47 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git, Per Cederqvist
In-Reply-To: <1361456285-29611-1-git-send-email-pclouds@gmail.com>

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

> branch_get() can return NULL (so far on detached HEAD only)...

Do you anticipate any other cases where the API call should validly
return NULL?  I offhand do not, ...

> but some
> code paths in builtin/branch.c cannot deal with that and cause
> segfaults. Fix it.
>
> While at there, make sure to bail out when the user gives 2 or more
> arguments, but only the first one is processed.
>
> Reported-by: Per Cederqvist <cederp@opera.com>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  builtin/branch.c  | 20 ++++++++++++++++++++
>  t/t3200-branch.sh | 21 +++++++++++++++++++++
>  2 files changed, 41 insertions(+)
>
> diff --git a/builtin/branch.c b/builtin/branch.c
> index 6371bf9..c1d688e 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -889,6 +889,13 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
>  	} else if (new_upstream) {
>  		struct branch *branch = branch_get(argv[0]);
>  
> +		if (argc > 1)
> +			die(_("too many branches to set new upstream"));
> +
> +		if (!branch)
> +			die(_("could not figure out the branch name from '%s'"),
> +			    argc == 1 ? argv[0] : "HEAD");

... and find this "could not figure out" very unfriendly to the
user.  Is it a bug in the implementation, silly Git failing to find
what branch the user meant?  What recourse does the user have at
this point?

Or is it a user error to ask Git to operate on the branch pointed at
by HEAD, when HEAD does not refer to any branch?  If that is the
case, then the message should say that there is no current branch to
operate on because the user is on a detached HEAD.  That would point
the user in the right direction to correct the user error, no?

Of course, argv[0] may not be HEAD and in that case you may have to
say "no such branch %s" % argv[0] or something.  The point is that
"could not figure out" feels a bit too lazy.

> @@ -901,6 +908,13 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
>  		struct branch *branch = branch_get(argv[0]);
>  		struct strbuf buf = STRBUF_INIT;
>  
> +		if (argc > 1)
> +			die(_("too many branches to unset upstream"));
> +
> +		if (!branch)
> +			die(_("could not figure out the branch name from '%s'"),
> +			    argc == 1 ? argv[0] : "HEAD");

Likewise.

> @@ -916,6 +930,12 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
>  		int branch_existed = 0, remote_tracking = 0;
>  		struct strbuf buf = STRBUF_INIT;
>  
> +		if (!strcmp(argv[0], "HEAD"))
> +			die(_("it does not make sense to create 'HEAD' manually"));
> +
> +		if (!branch)
> +			die(_("could not figure out the branch name from '%s'"), argv[0]);

Likewise.

Thanks.

^ permalink raw reply

* Re: "git branch --contains x y" creates a branch instead of checking containment
From: Junio C Hamano @ 2013-02-21 17:48 UTC (permalink / raw)
  To: Per Cederqvist; +Cc: Jeff King, git
In-Reply-To: <512645D1.4040607@opera.com>

Per Cederqvist <cederp@opera.com> writes:

> On 02/21/13 16:58, Jeff King wrote:
>> On Thu, Feb 21, 2013 at 02:00:27PM +0100, Per Cederqvist wrote:
>>
>>> That command does something completely different,
>>> though. The "--contains x" part is silently ignored,
>>> so it creates a branch named "y" pointing at HEAD.
>>>
>>> Tested in git 1.8.1.1 and 1.8.1.4.
>>>
>>> In my opinion, there are two ways to fix this:
>>>
>>>   - change the "git branch" implementation so
>>>     that --contains implies --list.
>>
>> I think that is the best option, too. In fact, I even wrote a patch. :)
>>
>> It's d040350 (branch: let branch filters imply --list, 2013-01-31), and
>> it's already in v1.8.2-rc0.
>>
>> -Peff
>
> Great! Thanks for the quick fix of my bug report. Negative response
> time... not bad. Not bad at all. :-)

Yeah, Jeff has a time machine ;-)

^ permalink raw reply

* Re: QNX support
From: Junio C Hamano @ 2013-02-21 17:49 UTC (permalink / raw)
  To: Matt Kraai; +Cc: git, David Ondřich
In-Reply-To: <430B4DD0-B796-4DB2-861D-C1F81302A4D1@aveco.com>

"David Ondřich" <david.ondrich@aveco.com> writes:

jch: redirecting to the guilty party ;-)

> I've read [1] recently, there's been some QNX port being
> initiated. Does that involve also old versions of QNX 4?
>
> Since we are using QNX both internally and for our customers we
> started porting Git on QNX ourselves some time ago and we do have
> some experiences. Basically, it's possible to get Git up and
> running but there are some limitations, and some hacks have to be
> applied.
>
> If some additional info wanted, please contact me.

^ permalink raw reply

* RE: QNX support
From: Kraai, Matt @ 2013-02-21 18:04 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, David Ondřich
In-Reply-To: <7vmwuxv9ou.fsf@alter.siamese.dyndns.org>

Junio C Hamano writes:
> "David Ondřich" <david.ondrich@aveco.com> writes:
> > I've read [1] recently, there's been some QNX port being
> > initiated. Does that involve also old versions of QNX 4?

No, I haven't been working on QNX 4 support.  I've been targeting QNX 6.3.2, with a little testing on QNX 6.5.0.  I doubt what I've done would work on QNX 4 since it's so different from QNX 6.
 
> > Since we are using QNX both internally and for our customers we
> > started porting Git on QNX ourselves some time ago and we do have
> > some experiences. Basically, it's possible to get Git up and
> > running but there are some limitations, and some hacks have to be
> > applied.
> >
> > If some additional info wanted, please contact me.

Now that Git is building and usable, the next logical step is to investigate and fix the test suite failures.  If you have any information about these, that could be helpful.

-- 
Matt

^ permalink raw reply

* Re: [PATCH v4.1 09/12] sequencer.c: teach append_signoff to avoid adding a duplicate newline
From: Junio C Hamano @ 2013-02-21 18:51 UTC (permalink / raw)
  To: Brandon Casey; +Cc: git, pclouds, jrnieder, Brandon Casey
In-Reply-To: <1360665222-3166-1-git-send-email-drafnel@gmail.com>

Brandon Casey <drafnel@gmail.com> writes:

> Teach append_signoff to detect whether a blank line exists at the position
> that the signed-off-by line will be added, and refrain from adding an
> additional one if one already exists.  Or, add an additional line if one
> is needed to make sure the new footer is separated from the message body
> by a blank line.
>
> Signed-off-by: Brandon Casey <bcasey@nvidia.com>
> ---
>
>
> A slight tweak.  And I promise, no more are coming.

When I do

	$ git commit -s

it should start my editor with this in the buffer:

	----------------------------------------------------------------

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

and the cursor blinking at the beginning of the file.  Annoyingly
this step breaks it by removing the leading blank line.

^ permalink raw reply

* Re: Credentials and the Secrets API...
From: John Szakmeister @ 2013-02-21 19:27 UTC (permalink / raw)
  To: Ted Zlatanov; +Cc: git
In-Reply-To: <87k3q2yl4y.fsf@lifelogs.com>

On Wed, Feb 20, 2013 at 12:01 PM, Ted Zlatanov <tzz@lifelogs.com> wrote:
> On Sat, 9 Feb 2013 05:58:47 -0500 John Szakmeister <john@szakmeister.net> wrote:
[snip]
>
> JS> Yes, I think it has.  Several other applications appear to be using
> JS> it, including some things considered "core" in Fedora--which is a good
> JS> sign.
>
> Wonderful.  Do you still have interest in working on this credential?

I do, but I'm a bit short on time right now.  So if you or someone
else wants to pick up and run with it, please feel free.  If I get
some cycles over the next couple of months, I'll give it a go though.

-John

^ permalink raw reply

* Re: Re* [PATCH 2/2] check-ignore.c: fix segfault with '.' argument from repo root
From: Junio C Hamano @ 2013-02-21 20:15 UTC (permalink / raw)
  To: Adam Spiers; +Cc: git list
In-Reply-To: <20130220104720.GD7860@pacific.linksys.moosehall>

Adam Spiers <git@adamspiers.org> writes:

> On Tue, Feb 19, 2013 at 06:53:07PM -0800, Junio C Hamano wrote:
>> Adam Spiers <git@adamspiers.org> writes:
>> 
>> > OK, thanks for the information.  IMHO it would be nice if 'git
>> > format-patch' and 'git am' supported this style of inline patch
>> > inclusion, but maybe there are good reasons to discourage it?
>> 
>> "git am --scissors" is a way to process such e-mail where the patch
>> submitter continues discussion in the top part of a message,
>> concludes the message with:
>> 
>> 	A patch to do so is attached.
>> 	-- >8 --
>> 
>> and then tells the MUA to read in an output from format-patch into
>> the e-mail buffer.
>
> Ah, nice!  I didn't know about that.
>
>>  You still need to strip out unneeded headers
>> like the "From ", "From: " and "Date: " lines when you add the
>> scissors anyway, and this is applicable only for a single-patch
>> series, so the "feature" does not fit well as a format-patch option.
>
> Rather than requiring the user to manually strip out unneeded headers,
> wouldn't it be friendlier and less error-prone to add a new --inline
> option to format-patch which omitted them in the first place?  It
> should be easy to make it bail with an error when multiple revisions
> are requested.

Perhaps.

^ permalink raw reply

* [PATCH 1/2] format-patch: rename "no_inline" field
From: Junio C Hamano @ 2013-02-21 20:17 UTC (permalink / raw)
  To: git list; +Cc: Adam Spiers
In-Reply-To: <7vehg9v2xj.fsf@alter.siamese.dyndns.org>

The name of the fields invites a misunderstanding that setting it to
false, saying "No, I will not to tell you not to inline", make the
patch inlined in the body of the message, but that is not what it
does.  The result is still a MIME attachment as long as
mime_boundary is set.  This field only controls if the content
disposition of a MIME attachment is set to "attachment" or "inline".

Rename it to clarify what it is used for.  Besides, a toggle whose
name is "no_frotz" is asking for a double-negation.  Calling it
"disposition-attachment" allows us to naturally read setting the
field to true as "Yes, I want to set content-disposition to
'attachment'".

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

 * This is a general "clean-up" patch that does not add any feature
   nor fixes any bug.

 builtin/log.c | 6 +++---
 log-tree.c    | 2 +-
 revision.h    | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index 8f0b2e8..30265d8 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -983,7 +983,7 @@ static int attach_callback(const struct option *opt, const char *arg, int unset)
 		rev->mime_boundary = arg;
 	else
 		rev->mime_boundary = git_version_string;
-	rev->no_inline = unset ? 0 : 1;
+	rev->disposition_attachment = unset ? 0 : 1;
 	return 0;
 }
 
@@ -996,7 +996,7 @@ static int inline_callback(const struct option *opt, const char *arg, int unset)
 		rev->mime_boundary = arg;
 	else
 		rev->mime_boundary = git_version_string;
-	rev->no_inline = 0;
+	rev->disposition_attachment = 0;
 	return 0;
 }
 
@@ -1172,7 +1172,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 
 	if (default_attach) {
 		rev.mime_boundary = default_attach;
-		rev.no_inline = 1;
+		rev.disposition_attachment = 1;
 	}
 
 	/*
diff --git a/log-tree.c b/log-tree.c
index 5dc45c4..34ec20d 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -408,7 +408,7 @@ void log_write_email_headers(struct rev_info *opt, struct commit *commit,
 			 " filename=\"%s\"\n\n",
 			 mime_boundary_leader, opt->mime_boundary,
 			 filename.buf,
-			 opt->no_inline ? "attachment" : "inline",
+			 opt->disposition_attachment ? "attachment" : "inline",
 			 filename.buf);
 		opt->diffopt.stat_sep = buffer;
 		strbuf_release(&filename);
diff --git a/revision.h b/revision.h
index 5da09ee..90813dd 100644
--- a/revision.h
+++ b/revision.h
@@ -142,7 +142,7 @@ struct rev_info {
 	const char	*extra_headers;
 	const char	*log_reencode;
 	const char	*subject_prefix;
-	int		no_inline;
+	int		disposition_attachment;
 	int		show_log_size;
 	struct string_list *mailmap;
 
-- 
1.8.2.rc0.129.gcce6fe7

^ permalink raw reply related

* [PATCH v2 0/2] Documentation: filter-branch env-filter example
From: Tadeusz Andrzej Kadłubowski @ 2013-02-21 20:21 UTC (permalink / raw)
  To: git

Hello,

Indeed the use case proposed by Junio C. Hamano is more realistic than mine.

I also included a minor clarification of how identity environment variables are
used to cover the edge case raised by Jeff King. As far as the need to export
those variables goes, I saw the wrong behavior on Solaris.

-- 
Tadeusz Andrzej Kadłubowski

^ permalink raw reply

* [PATCH 1/2] git-filter-branch.txt: clarify ident variables usage
From: Tadeusz Andrzej Kadłubowski @ 2013-02-21 20:22 UTC (permalink / raw)
  To: git

There is a rare edge case of git-filter-branch: a filter that unsets
identity variables from the environment. Link to git-commit-tree
clarifies how Git would fall back in this situation.

Signed-off-by: Tadeusz Andrzej Kadłubowski <yess@hell.org.pl>
---
 Documentation/git-filter-branch.txt | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-filter-branch.txt b/Documentation/git-filter-branch.txt
index dfd12c9..e50ee2f 100644
--- a/Documentation/git-filter-branch.txt
+++ b/Documentation/git-filter-branch.txt
@@ -65,9 +65,9 @@ Prior to that, the $GIT_COMMIT environment variable will be set to contain
 the id of the commit being rewritten.  Also, GIT_AUTHOR_NAME,
 GIT_AUTHOR_EMAIL, GIT_AUTHOR_DATE, GIT_COMMITTER_NAME, GIT_COMMITTER_EMAIL,
 and GIT_COMMITTER_DATE are set according to the current commit.  The values
-of these variables after the filters have run, are used for the new commit.
-If any evaluation of <command> returns a non-zero exit status, the whole
-operation will be aborted.
+of these variables after the filters have run, are used for the new commit
+(see linkgit:git-commit-tree[1] for details).  If any evaluation of <command>
+returns a non-zero exit status, the whole operation will be aborted.
  A 'map' function is available that takes an "original sha1 id" argument
 and outputs a "rewritten sha1 id" if the commit has been already
-- 
1.7.11.7

^ permalink raw reply related

* [PATCH 2/2] Documentation: filter-branch env-filter example
From: Tadeusz Andrzej Kadłubowski @ 2013-02-21 20:23 UTC (permalink / raw)
  To: git

filter-branch --env-filter example that shows how to change the email
address in all commits before publishing a project.

Signed-off-by: Tadeusz Andrzej Kadłubowski <yess@hell.org.pl>
---
 Documentation/git-filter-branch.txt | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/Documentation/git-filter-branch.txt b/Documentation/git-filter-branch.txt
index e50ee2f..660bd32 100644
--- a/Documentation/git-filter-branch.txt
+++ b/Documentation/git-filter-branch.txt
@@ -329,6 +329,27 @@ git filter-branch --msg-filter '
 ' HEAD~10..HEAD
 --------------------------------------------------------
 +The `--env-filter` option can be used to modify committer and/or author
+identity.  For example, if you found out that your commits have the wrong
+identity due to a misconfigured user.email, you can make a correction,
+before publishing the project, like this:
+
+
+--------------------------------------------------------
+git filter-branch --env-filter '
+	if test "$GIT_AUTHOR_EMAIL" = "root@localhost"
+	then
+		GIT_AUTHOR_EMAIL=john@example.com
+		export GIT_AUTHOR_EMAIL
+	fi
+	if test "$GIT_COMMITTER_EMAIL" = "root@localhost"
+	then
+		GIT_COMMITTER_EMAIL=john@example.com
+		export GIT_COMMITTER_EMAIL
+	fi
+' -- --all
+--------------------------------------------------------
+
 To restrict rewriting to only part of the history, specify a revision
 range in addition to the new branch name.  The new branch name will
 point to the top-most revision that a 'git rev-list' of this range
-- 
1.7.11.7

^ permalink raw reply related

* [PATCH 2/2] format-patch: --inline-single
From: Junio C Hamano @ 2013-02-21 20:26 UTC (permalink / raw)
  To: git list; +Cc: Adam Spiers
In-Reply-To: <7vehg9v2xj.fsf@alter.siamese.dyndns.org>

Some people may find it convenient to append a simple patch at the
bottom of a discussion e-mail separated by a "scissors" mark, ready
to be applied with "git am -c".  Introduce "--inline-single" option
to format-patch to do so.  A typical usage example might be to start
'F'ollow-up to a discussion, write your message, conclude with "a
patch to do so may look like this.", and 

    \C-u M-! git format-patch --inline-single -1 HEAD <ENTER>

if you are an Emacs user.  Users of other MUA's may want to consult
their manuals to find equivalent command to append output from an
external command to the message being composed.

It does not make any sense to use this mode when formatting multiple
patches, or to combine this with options such as --attach, --inline,
and --cover-letter, so some of such uses are forbidden.  There may
be more insane combination the check in this patch may not even
bother to reject.  Caveat emptor.

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

 * I did this as a lunch-time hack, but I'll leave it to interested
   readers as an exercise to find corner case "bugs", e.g. some
   insane combinations of options may not be diagnosed as usage
   errors, and to update the tests and documentation.

   Personally, "git format-patch --stdout -1 HEAD" with manual
   editing is more flexible, so I am not interested in spending
   cycles to polish this further myself.

   The preliminary patch 1/2 I sent earlier is worth doing, though.

 builtin/log.c | 32 ++++++++++++++++++++++++++++++++
 commit.h      |  1 +
 log-tree.c    |  7 ++++++-
 pretty.c      | 27 ++++++++++++++++++++++++++-
 revision.h    |  1 +
 5 files changed, 66 insertions(+), 2 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index 30265d8..5ad0837 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1000,6 +1000,19 @@ static int inline_callback(const struct option *opt, const char *arg, int unset)
 	return 0;
 }
 
+static int inline_single_callback(const struct option *opt, const char *arg, int unset)
+{
+	struct rev_info *rev = (struct rev_info *)opt->value;
+	rev->mime_boundary = NULL;
+	rev->inline_single = 1;
+
+	/* defeat configured format.attach, format.thread, etc. */
+	free(default_attach);
+	default_attach = NULL;
+	thread = 0;
+	return 0;
+}
+
 static int header_callback(const struct option *opt, const char *arg, int unset)
 {
 	if (unset) {
@@ -1149,6 +1162,10 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 			    PARSE_OPT_OPTARG, thread_callback },
 		OPT_STRING(0, "signature", &signature, N_("signature"),
 			    N_("add a signature")),
+		{ OPTION_CALLBACK, 0, "inline-single", &rev, NULL,
+		  N_("single patch appendable to the end of an e-mail body"),
+		  PARSE_OPT_NOARG | PARSE_OPT_NONEG,
+		  inline_single_callback },
 		OPT_BOOLEAN(0, "quiet", &quiet,
 			    N_("don't print the patch filenames")),
 		OPT_END()
@@ -1185,6 +1202,17 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 			     PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN |
 			     PARSE_OPT_KEEP_DASHDASH);
 
+	/* Set defaults and check incompatible options */
+	if (rev.inline_single) {
+		use_stdout = 1;
+		if (cover_letter)
+			die(_("inline-single and cover-letter are incompatible."));
+		if (thread)
+			die(_("inline-single and thread are incompatible."));
+		if (output_directory)
+			die(_("inline-single and output-directory are incompatible."));
+	}
+
 	if (0 < reroll_count) {
 		struct strbuf sprefix = STRBUF_INIT;
 		strbuf_addf(&sprefix, "%s v%d",
@@ -1373,6 +1401,10 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		list[nr - 1] = commit;
 	}
 	total = nr;
+
+	if (rev.inline_single && total != 1)
+		die(_("inline-single is only for a single commit"));
+
 	if (!keep_subject && auto_number && total > 1)
 		numbered = 1;
 	if (numbered)
diff --git a/commit.h b/commit.h
index 4138bb4..f3d9959 100644
--- a/commit.h
+++ b/commit.h
@@ -85,6 +85,7 @@ struct pretty_print_context {
 	int preserve_subject;
 	enum date_mode date_mode;
 	unsigned date_mode_explicit:1;
+	unsigned inline_single:1;
 	int need_8bit_cte;
 	char *notes_message;
 	struct reflog_walk_info *reflog_info;
diff --git a/log-tree.c b/log-tree.c
index 34ec20d..15c9749 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -358,7 +358,11 @@ void log_write_email_headers(struct rev_info *opt, struct commit *commit,
 		subject = "Subject: ";
 	}
 
-	printf("From %s Mon Sep 17 00:00:00 2001\n", name);
+	if (opt->inline_single)
+		printf("-- >8 --\n");
+	else
+		printf("From %s Mon Sep 17 00:00:00 2001\n", name);
+
 	graph_show_oneline(opt->graph);
 	if (opt->message_id) {
 		printf("Message-Id: <%s>\n", opt->message_id);
@@ -683,6 +687,7 @@ void show_log(struct rev_info *opt)
 	ctx.fmt = opt->commit_format;
 	ctx.mailmap = opt->mailmap;
 	ctx.color = opt->diffopt.use_color;
+	ctx.inline_single = opt->inline_single;
 	pretty_print_commit(&ctx, commit, &msgbuf);
 
 	if (opt->add_signoff)
diff --git a/pretty.c b/pretty.c
index eae57ad..363b3d9 100644
--- a/pretty.c
+++ b/pretty.c
@@ -383,6 +383,29 @@ static void add_rfc2047(struct strbuf *sb, const char *line, int len,
 	strbuf_addstr(sb, "?=");
 }
 
+static int is_current_user(const struct pretty_print_context *pp,
+			   const char *email, size_t emaillen,
+			   const char *name, size_t namelen)
+{
+	const char *me = git_committer_info(0);
+	const char *myname, *mymail;
+	size_t mynamelen, mymaillen;
+	struct ident_split ident;
+
+	if (split_ident_line(&ident, me, strlen(me)))
+		return 0; /* play safe, as we do not know */
+	mymail = ident.mail_begin;
+	mymaillen = ident.mail_end - ident.mail_begin;
+	myname = ident.name_begin;
+	mynamelen = ident.name_end - ident.name_begin;
+	if (pp->mailmap)
+		map_user(pp->mailmap, &mymail, &mymaillen, &myname, &mynamelen);
+	return (mymaillen == emaillen &&
+		mynamelen == namelen &&
+		!memcmp(mymail, email, emaillen) &&
+		!memcmp(myname, name, namelen));
+}
+
 void pp_user_info(const struct pretty_print_context *pp,
 		  const char *what, struct strbuf *sb,
 		  const char *line, const char *encoding)
@@ -412,7 +435,6 @@ void pp_user_info(const struct pretty_print_context *pp,
 	if (split_ident_line(&ident, line, linelen))
 		return;
 
-
 	mailbuf = ident.mail_begin;
 	maillen = ident.mail_end - ident.mail_begin;
 	namebuf = ident.name_begin;
@@ -421,6 +443,9 @@ void pp_user_info(const struct pretty_print_context *pp,
 	if (pp->mailmap)
 		map_user(pp->mailmap, &mailbuf, &maillen, &namebuf, &namelen);
 
+	if (pp->inline_single && is_current_user(pp, mailbuf, maillen, namebuf, namelen))
+		return;
+
 	strbuf_init(&mail, 0);
 	strbuf_init(&name, 0);
 
diff --git a/revision.h b/revision.h
index 90813dd..0c6d955 100644
--- a/revision.h
+++ b/revision.h
@@ -143,6 +143,7 @@ struct rev_info {
 	const char	*log_reencode;
 	const char	*subject_prefix;
 	int		disposition_attachment;
+	int		inline_single;
 	int		show_log_size;
 	struct string_list *mailmap;
 
-- 
1.8.2.rc0.129.gcce6fe7

^ permalink raw reply related

* Re: [PATCH v4.1 09/12] sequencer.c: teach append_signoff to avoid adding a duplicate newline
From: Brandon Casey @ 2013-02-21 20:26 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, pclouds, jrnieder, Brandon Casey
In-Reply-To: <7vip5lv6tv.fsf@alter.siamese.dyndns.org>

On Thu, Feb 21, 2013 at 10:51 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Brandon Casey <drafnel@gmail.com> writes:
>
>> Teach append_signoff to detect whether a blank line exists at the position
>> that the signed-off-by line will be added, and refrain from adding an
>> additional one if one already exists.  Or, add an additional line if one
>> is needed to make sure the new footer is separated from the message body
>> by a blank line.
>>
>> Signed-off-by: Brandon Casey <bcasey@nvidia.com>
>> ---
>>
>>
>> A slight tweak.  And I promise, no more are coming.
>
> When I do
>
>         $ git commit -s
>
> it should start my editor with this in the buffer:
>
>         ----------------------------------------------------------------
>
>         Signed-off-by: Junio C Hamano <gitster@pobox.com>
>         ----------------------------------------------------------------
>
> and the cursor blinking at the beginning of the file.  Annoyingly
> this step breaks it by removing the leading blank line.

Yes.  The fix described by John Keeping restores the above behavior
for 'commit -s'.  Or the fix I described which inserts two preceding
newlines so it looks like this:

   ----------------------------------------------------------------


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

So then the cursor would be placed on the first line and a space would
separate it from the sob which is arguably a better indication to the
user that a blank line should separate the commit message body from
the sob.

But, this does not fix the same problem for 'cherry-pick --edit -s'
when used to cherry-pick a commit without a sob.  The cherry-pick part
of it would add the extra preceding newlines, but then cherry-pick
passes the buffer to 'git commit' via .git/MERGE_MSG which then
"cleans" the buffer, removing the empty lines, in prepare_to_commit()
before allowing the editor to operate on it.

Using 'cherry-pick --edit -s' to cherry-pick a commit with an empty
commit message is going to be a pretty rare corner case.  It would be
nice to have the same behavior for it that we decide to have for
'commit -s', but it's probably not worth going through contortions to
make it happen.

-Brandon

^ permalink raw reply

* Re: [PATCH v4.1 09/12] sequencer.c: teach append_signoff to avoid adding a duplicate newline
From: Brandon Casey @ 2013-02-21 20:29 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, pclouds, jrnieder, Brandon Casey
In-Reply-To: <CA+sFfMcNWvPKuQpNWnacegbfgN0ZP=zfuDPDRkXs1G2FMrb+iA@mail.gmail.com>

On Thu, Feb 21, 2013 at 12:26 PM, Brandon Casey <drafnel@gmail.com> wrote:

> But, this does not fix the same problem for 'cherry-pick --edit -s'
> when used to cherry-pick a commit without a sob.

Correction: "when used to cherry-pick a commit with an empty commit message."

^ permalink raw reply

* Re: [PATCH 1/2] git-filter-branch.txt: clarify ident variables usage
From: Junio C Hamano @ 2013-02-21 20:46 UTC (permalink / raw)
  To: Tadeusz Andrzej Kadłubowski; +Cc: git
In-Reply-To: <5126821A.9020800@hell.org.pl>

Tadeusz Andrzej Kadłubowski  <yess@hell.org.pl> writes:

> There is a rare edge case of git-filter-branch: a filter that unsets
> identity variables from the environment. Link to git-commit-tree
> clarifies how Git would fall back in this situation.

I find it unclear in the updated text _why_ the reader may want to
refer to that other documentation.

> Signed-off-by: Tadeusz Andrzej Kadłubowski <yess@hell.org.pl>
> ---
>  Documentation/git-filter-branch.txt | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/git-filter-branch.txt b/Documentation/git-filter-branch.txt
> index dfd12c9..e50ee2f 100644
> --- a/Documentation/git-filter-branch.txt
> +++ b/Documentation/git-filter-branch.txt
> @@ -65,9 +65,9 @@ Prior to that, the $GIT_COMMIT environment variable will be set to contain
>  the id of the commit being rewritten.  Also, GIT_AUTHOR_NAME,
>  GIT_AUTHOR_EMAIL, GIT_AUTHOR_DATE, GIT_COMMITTER_NAME, GIT_COMMITTER_EMAIL,
>  and GIT_COMMITTER_DATE are set according to the current commit.  The values
> -of these variables after the filters have run, are used for the new commit.
> -If any evaluation of <command> returns a non-zero exit status, the whole
> -operation will be aborted.
> +of these variables after the filters have run, are used for the new commit
> +(see linkgit:git-commit-tree[1] for details).  If any evaluation of <command>
> +returns a non-zero exit status, the whole operation will be aborted.

Here is my attempt to clarify it a bit.

	Also, ...<variables>... are taken from the current commit
	and exported to the environment, in order to affect the
	author and committer identities of the replacement commit
	created by linkgit:git-commit-tree[1] after the filters have
	run.

A user who contemplates to unset them should be able to guess the
consequences, even though the above text does not single out such an
insane misuse to put an undue stress on it.

>   A 'map' function is available that takes an "original sha1 id" argument
>  and outputs a "rewritten sha1 id" if the commit has been already

^ permalink raw reply

* Re: [PATCH 2/2] Documentation: filter-branch env-filter example
From: Junio C Hamano @ 2013-02-21 20:49 UTC (permalink / raw)
  To: Tadeusz Andrzej Kadłubowski; +Cc: git
In-Reply-To: <5126824A.2060903@hell.org.pl>

Tadeusz Andrzej Kadłubowski  <yess@hell.org.pl> writes:

> filter-branch --env-filter example that shows how to change the email
> address in all commits before publishing a project.
>
> Signed-off-by: Tadeusz Andrzej Kadłubowski <yess@hell.org.pl>
> ---

Assuming that the result formats well both as html and manpage, the
added example looks good to me.  I somehow suspect that there is a
patch-mangling going on, though.

>  Documentation/git-filter-branch.txt | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>
> diff --git a/Documentation/git-filter-branch.txt b/Documentation/git-filter-branch.txt
> index e50ee2f..660bd32 100644
> --- a/Documentation/git-filter-branch.txt
> +++ b/Documentation/git-filter-branch.txt
> @@ -329,6 +329,27 @@ git filter-branch --msg-filter '
>  ' HEAD~10..HEAD
>  --------------------------------------------------------
>  +The `--env-filter` option can be used to modify committer and/or author
> +identity.  For example, if you found out that your commits have the wrong
> +identity due to a misconfigured user.email, you can make a correction,
> +before publishing the project, like this:
> +
> +
> +--------------------------------------------------------
> +git filter-branch --env-filter '
> +	if test "$GIT_AUTHOR_EMAIL" = "root@localhost"
> +	then
> +		GIT_AUTHOR_EMAIL=john@example.com
> +		export GIT_AUTHOR_EMAIL
> +	fi
> +	if test "$GIT_COMMITTER_EMAIL" = "root@localhost"
> +	then
> +		GIT_COMMITTER_EMAIL=john@example.com
> +		export GIT_COMMITTER_EMAIL
> +	fi
> +' -- --all
> +--------------------------------------------------------
> +
>  To restrict rewriting to only part of the history, specify a revision
>  range in addition to the new branch name.  The new branch name will
>  point to the top-most revision that a 'git rev-list' of this range

^ permalink raw reply


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