Git development
 help / color / mirror / Atom feed
* Re: [PATCH] tile: support GENERIC_KERNEL_THREAD and GENERIC_KERNEL_EXECVE
From: Jeff King @ 2012-10-23 20:51 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Al Viro, Chris Metcalf, LKML, linux-arch, Linus Torvalds,
	Catalin Marinas, git, Junio C Hamano
In-Reply-To: <alpine.LFD.2.02.1210232232070.2756@ionos>

On Tue, Oct 23, 2012 at 10:47:28PM +0200, Thomas Gleixner wrote:

> I agree that this is a common issue. Acked-by/Reviewed-by mails come
> in after the fact that the patch has been committed to an immutable
> (i.e no-rebase mode) branch or if the change in question already hit
> Linus tree.
> 
> Still it would be nice to have a recording of that in the git tree
> itself.
> 
> Something like: "git --attach SHA1 <comment>" would be appreciated!

It is spelled:

  git notes add -m <comment> SHA1

The resulting notes are stored in a separate revision-controlled branch
and can be pushed and pulled like regular refs. Note, though, that the
default refspecs do not yet include refs/notes, so you'd have to add
them manually. The workflows around notes are not very mature yet, so if
you start using them, feedback would be appreciated.

-Peff

^ permalink raw reply

* Re: Git submodule for a local branch?
From: Jens Lehmann @ 2012-10-23 20:57 UTC (permalink / raw)
  To: W. Trevor King; +Cc: Git
In-Reply-To: <20121022123714.GL25563@odin.tremily.us>

Am 22.10.2012 14:37, schrieb W. Trevor King:
> I have a bunch of branches in my repo (a, b, c, …), and I'd like to
> check them out into subdirectories of another branch (index).  My
> initial inclination was to use something like
> 
>   $ git checkout index
>   $ git branch
>     a
>     b
>     c
>   * index
>   $ git submodule add -b a --reference ./ ./ dir-for-a/
>   $ git submodule add -b b --reference ./ ./ dir-for-b/
>   $ git submodule add -b c --reference ./ ./ dir-for-c/
> 
> but cloning a remote repository (vs. checking out a local branch)
> seems to be baked into the submodule implementation.  Should I be
> thinking about generalizing git-submodule.sh, or am I looking under
> the wrong rock?  My ideal syntax would be something like
> 
>   $ git submodule add -b c --local dir-for-c/

But then we'd have to be able to have two (or more) work trees using
the same git directory, which current submodule code can't.

> The motivation is that I have website that contains a bunch of
> sub-sites, and the sub-sites share content.  I have per-sub-site
> branches (a, b, c) and want a master branch (index) that aggregates
> them.  Perhaps this is too much to wedge into a single repository?

To me this sounds upside-down. I'd put the three sub-sites into
different repositories and the shared content into a submodule that
all three sub-sites use. At least that is how I do all my content
sharing on the websites I have done ... does that make sense?

^ permalink raw reply

* [PATCH] git-submodule: wrap branch option with "<>" in usage strings.
From: W. Trevor King @ 2012-10-23 21:00 UTC (permalink / raw)
  To: Git; +Cc: W. Trevor King

From: "W. Trevor King" <wking@tremily.us>

Use "-b <branch>" instead of "-b branch".  This brings the usage
strings in line with other options, e.g. "--reference <repository>".

Signed-off-by: W. Trevor King <wking@tremily.us>
---
 Documentation/git-submodule.txt | 2 +-
 git-submodule.sh                | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index b4683bb..a65f38e 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -9,7 +9,7 @@ git-submodule - Initialize, update or inspect submodules
 SYNOPSIS
 --------
 [verse]
-'git submodule' [--quiet] add [-b branch] [-f|--force]
+'git submodule' [--quiet] add [-b <branch>] [-f|--force]
 	      [--reference <repository>] [--] <repository> [<path>]
 'git submodule' [--quiet] status [--cached] [--recursive] [--] [<path>...]
 'git submodule' [--quiet] init [--] [<path>...]
diff --git a/git-submodule.sh b/git-submodule.sh
index ab6b110..02f82e9 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -5,7 +5,7 @@
 # Copyright (c) 2007 Lars Hjemli
 
 dashless=$(basename "$0" | sed -e 's/-/ /')
-USAGE="[--quiet] add [-b branch] [-f|--force] [--reference <repository>] [--] <repository> [<path>]
+USAGE="[--quiet] add [-b <branch>] [-f|--force] [--reference <repository>] [--] <repository> [<path>]
    or: $dashless [--quiet] status [--cached] [--recursive] [--] [<path>...]
    or: $dashless [--quiet] init [--] [<path>...]
    or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
-- 
1.8.0.2.g09b91ca

^ permalink raw reply related

* Re: [PATCH] tile: support GENERIC_KERNEL_THREAD and GENERIC_KERNEL_EXECVE
From: Catalin Marinas @ 2012-10-23 21:09 UTC (permalink / raw)
  To: Jeff King
  Cc: Thomas Gleixner, Al Viro, Chris Metcalf, LKML, linux-arch,
	Linus Torvalds, git, Junio C Hamano
In-Reply-To: <20121023205119.GA27729@sigill.intra.peff.net>

On 23 October 2012 21:51, Jeff King <peff@peff.net> wrote:
> On Tue, Oct 23, 2012 at 10:47:28PM +0200, Thomas Gleixner wrote:
>
>> I agree that this is a common issue. Acked-by/Reviewed-by mails come
>> in after the fact that the patch has been committed to an immutable
>> (i.e no-rebase mode) branch or if the change in question already hit
>> Linus tree.
>>
>> Still it would be nice to have a recording of that in the git tree
>> itself.
>>
>> Something like: "git --attach SHA1 <comment>" would be appreciated!
>
> It is spelled:
>
>   git notes add -m <comment> SHA1
>
> The resulting notes are stored in a separate revision-controlled branch
> and can be pushed and pulled like regular refs. Note, though, that the
> default refspecs do not yet include refs/notes, so you'd have to add
> them manually. The workflows around notes are not very mature yet, so if
> you start using them, feedback would be appreciated.

What would be nice is that notes are pushed/pulled automatically with
standard git push/fetch/pull commands. Usually git walks the DAG
starting with the pulled commit or tag and following the parents. With
notes, the reference is reversed, the note pointing to the commit and
not the other way around. So handling this automatically in Git would
be really useful.

The other feature I'd like is that notes are automatically folded in
the log during git rebase (maybe similar to the squash option). If you
rebase, you lose all the notes (though this depends on the workflow,
it may not be needed with published branches).

-- 
Catalin

^ permalink raw reply

* Re: [PATCH] tile: support GENERIC_KERNEL_THREAD and GENERIC_KERNEL_EXECVE
From: Jeff King @ 2012-10-23 21:22 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: Thomas Gleixner, Al Viro, Chris Metcalf, LKML, linux-arch,
	Linus Torvalds, git, Junio C Hamano
In-Reply-To: <CAHkRjk6x9ToVzY7jv1ZxPt57F6agcH7SfHZpZNpHC3QP3PZp3Q@mail.gmail.com>

On Tue, Oct 23, 2012 at 10:09:46PM +0100, Catalin Marinas wrote:

> > It is spelled:
> >
> >   git notes add -m <comment> SHA1
> >
> > The resulting notes are stored in a separate revision-controlled branch
> > and can be pushed and pulled like regular refs. Note, though, that the
> > default refspecs do not yet include refs/notes, so you'd have to add
> > them manually. The workflows around notes are not very mature yet, so if
> > you start using them, feedback would be appreciated.
> 
> What would be nice is that notes are pushed/pulled automatically with
> standard git push/fetch/pull commands. Usually git walks the DAG
> starting with the pulled commit or tag and following the parents. With
> notes, the reference is reversed, the note pointing to the commit and
> not the other way around. So handling this automatically in Git would
> be really useful.

Right, that's what I meant about the refspecs. You can configure git to
push or pull them automatically, but it is not the default. Something
like:

  git config --add remote.origin.fetch '+refs/notes/*:refs/notes/origin/*'

would be a start, but you'd also want to "git notes merge" upstream's
changes into your local notes (you _could_ just fetch straight into
refs/notes/, but if you are making your own notes locally, you have to
resolve it somehow). Exactly how to make this smooth is one of the workflow
considerations; there's been discussion, but most people aren't using
the feature, so we don't have a lot of data.

If you are asking whether we could "auto-follow" notes for commits that
have been fetched like we do for tags, the answer is "not really". The
notes tree is version-controlled as a whole, so you generally fetch the
whole thing or not. And the remote does not advertise note information
the same way we advertise peeled tag references, so a client does not
know which notes are available for fetch. The intended strategy is to
pull in the notes or not (though you can have multiple notes refs with
different names, and fetch just a subset of them).

> The other feature I'd like is that notes are automatically folded in
> the log during git rebase (maybe similar to the squash option). If you
> rebase, you lose all the notes (though this depends on the workflow,
> it may not be needed with published branches).

Git-rebase can automatically copy notes from one commit to another
during a rebase, but you need to set notes.rewriteRef to do so (see "git
help config" for details). The reason for this conservative default is
that some notes may not be appropriate for automatic copying (e.g., a
notes tree containing QA approval should probably be invalidated during
a rebase, whereas one with commentary probably should).

Squashing the notes into the commit message during rebase would be a
useful feature (at least for some type of notes), but that feature does
not currently exist (and as far as I recall, this is the first it has
been proposed).

Again, I think a lot of this comes down to the fact that not many people
are really using notes for their daily workflow, so these itches are not
coming up and getting scratched.

-Peff

^ permalink raw reply

* Re: [PATCH] tile: support GENERIC_KERNEL_THREAD and GENERIC_KERNEL_EXECVE
From: Thomas Gleixner @ 2012-10-23 21:25 UTC (permalink / raw)
  To: Jeff King
  Cc: Al Viro, Chris Metcalf, LKML, linux-arch, Linus Torvalds,
	Catalin Marinas, git, Junio C Hamano
In-Reply-To: <20121023205119.GA27729@sigill.intra.peff.net>

On Tue, 23 Oct 2012, Jeff King wrote:

> On Tue, Oct 23, 2012 at 10:47:28PM +0200, Thomas Gleixner wrote:
> 
> > I agree that this is a common issue. Acked-by/Reviewed-by mails come
> > in after the fact that the patch has been committed to an immutable
> > (i.e no-rebase mode) branch or if the change in question already hit
> > Linus tree.
> > 
> > Still it would be nice to have a recording of that in the git tree
> > itself.
> > 
> > Something like: "git --attach SHA1 <comment>" would be appreciated!
> 
> It is spelled:
> 
>   git notes add -m <comment> SHA1

Cool!

> The resulting notes are stored in a separate revision-controlled branch

Which branch(es) is/are that ? What are the semantics of that?

Assume I commit something to branch "foo"

Now I get that late Ack/Reviewed-by and want to associate that to that
commit in branch "foo". Does that go into "notes/foo" ?

If yes, good. (Any other sensible prefix is good as well). If no,
where does it go to?

Later when I send a pull request to my upstream maintainer for branch
"foo" does he get "notes/foo" automagically or do I have to request to
pull him that separately?

Either way is fine for me, though something which lets me "automate"
that would be appreciated. I can work around that easily as my pull
requests are generated via scripts, so I can add the secondary one for
the dependent "notes" branch if necessary. Though it would be nice to
avoid that. Avoiding that, i.e having a straight connection (maybe
configurable) between "foo" and "notes/foo" and the commits which have
not yet hit my upstream maintainer would make my life easier. I.e. I
just have to check "foo" for stuff which is not upstream yet instead
of checking both, but that might just be my laziness.

Thoughts?

	tglx

^ permalink raw reply

* Re: [PATCH] git-submodule add: Record branch name in .gitmodules
From: W. Trevor King @ 2012-10-23 20:44 UTC (permalink / raw)
  To: Nahor, Phil Hord; +Cc: git
In-Reply-To: <20121023194436.GD28592@odin.tremily.us>

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

On Tue, Oct 23, 2012 at 03:44:36PM -0400, W. Trevor King wrote:
> On Tue, Oct 23, 2012 at 12:16:22PM -0700, Nahor wrote:
> > On 2012-10-22 09:34, W. Trevor King wrote:
> > For instance, the module may later be updated to a commit in branch B 
> > instead of branch A. Unless you remember to also update .gitmodule, you 
> > have then inconsistent information.
> 
> But you're explicitly *using* the configured setting in
> 
>   git config --file $toplevel/.gitmodules submodule.$name.branch
> 
> That should be a reminder that the configuration is important, and
> you'll remember to change it.

To make my case more cleanly, people already handle all the
troublesome cases for branch.$name.remote, so handling similar
upstream volatility for submodule.$name.branch should not be too
difficult or surprising.

On Tue, Oct 23, 2012 at 03:58:48PM -0400, Phil Hord wrote:
> On Mon, Oct 22, 2012 at 6:55 PM, W. Trevor King <wking@tremily.us> wrote:
> > How about -r/--record, with the recorded name being optional?
> >
> >   --record-branch[=<recorded_name>]
> 
> I like that just fine.
> 
> > This would satisfy Gerrit users that wanted to use '.', but also
> > satisfy me with:
> >
> >   git submodule add -rb=master foo bar
> >
> > However, there is a change that people would see that, and then use
> >
> >   git submodule add -r -b=master foo bar
> >
> > which would checkout the HEAD from foo and store `-b=master` in
> > submodule.$name.branch.
> 
> I don't think it would.

Ah, right, forcing the =<name> attached case would mean they'd have to
use

  git submodule add -r=-b=master

which doesn't sound like the sort of thing you'd do accidentally.

> Though I see in rev-parse--parseopts that the use of
> optional-argument options "is discouraged".

I'll work up a v2 patch and we'll see if anyone complains ;).

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH] git-submodule add: Record branch name in .gitmodules
From: Nahor @ 2012-10-23 21:45 UTC (permalink / raw)
  To: git
In-Reply-To: <20121023194436.GD28592@odin.tremily.us>

On 2012-10-23 12:44, W. Trevor King wrote:
> On Tue, Oct 23, 2012 at 12:16:22PM -0700, Nahor wrote:
>> On 2012-10-22 09:34, W. Trevor King wrote:
>> For instance, the module may later be updated to a commit in branch B
>> instead of branch A. Unless you remember to also update .gitmodule, you
>> have then inconsistent information.
>
> But you're explicitly *using* the configured setting in
>
>    git config --file $toplevel/.gitmodules submodule.$name.branch
>
> That should be a reminder that the configuration is important, and
> you'll remember to change it.

 From my experience with my colleagues at work, that's not going to 
happen. People are very good at forgetting to do something ;)


> Plus, the text from git-pull should
> clearly display the branch you are pulling, which gives you a second
> change to notice if something is going wrong.

That's assuming that the user knows the branch that should be pulled and 
that he's paying attention to the output and not just quick-scanning for 
errors/warnings.
Again, from my experience, that's not going to be the case.

Plus, there isn't always a human being behind a git-pull, e.g. build bots.


>> I think a better place to store that kind of information is using
>> git-notes. That way, if the branch is renamed or deleted, you can easily
>> update the old notes to use the new name instead.
>
> Interesting.  What would you attach the note too?

To the commits in the super-repo where a module branch is selected, i.e.:
- where a module was added
- where the module changed branch
- where a super-branch was merged and there was a conflict on the 
module's branch name

^ permalink raw reply

* Re: [PATCH] tile: support GENERIC_KERNEL_THREAD and GENERIC_KERNEL_EXECVE
From: Jeff King @ 2012-10-23 21:47 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Al Viro, Chris Metcalf, LKML, linux-arch, Linus Torvalds,
	Catalin Marinas, git, Junio C Hamano
In-Reply-To: <alpine.LFD.2.02.1210232307480.2756@ionos>

On Tue, Oct 23, 2012 at 11:25:06PM +0200, Thomas Gleixner wrote:

> > The resulting notes are stored in a separate revision-controlled branch
> 
> Which branch(es) is/are that ? What are the semantics of that?

They are stored in refs/notes/commits by default, but you can have
multiple notes refs if you want to store logically distinct sets of
notes.

A notes ref's tree is just a tree whose entries are sha1s, and the file
contents contain the notes themselves (the sha1s are broken down into
subdirectories for performance, but "git notes" handles this behind the
scenes). Technically you could check it out as a branch, edit, and
commit, but "git checkout" is not happy to have a HEAD outside of
refs/heads/, so you are stuck with plumbing like:

  $ git checkout `git rev-parse refs/notes/commits`
  $ edit edit edit
  $ git commit ...
  $ git update-ref refs/notes/commits HEAD

It's probably not good for much beyond exploring how notes are
implemented. See "git help notes" for more discussion.

> Assume I commit something to branch "foo"
> 
> Now I get that late Ack/Reviewed-by and want to associate that to that
> commit in branch "foo". Does that go into "notes/foo" ?

No. It would go into refs/notes/commits, or you could ask it to go to
refs/notes/acks if you wanted to keep them separate from your default
notes. It is indexed by commit object, not by branch (so if that branch
later goes away, the notes are always still attached to the commit
objects, assuming they got merged in).

> Later when I send a pull request to my upstream maintainer for branch
> "foo" does he get "notes/foo" automagically or do I have to request to
> pull him that separately?

No, he would have to pull your notes separately. Most of the discussion
around sharing has been configuring the default refspec configuration to
fetch notes.  But in the kernel you guys use a lot of one-off pulls
without configured remotes. I'm not sure what the right workflow would
be. It might simply be to ask git to always pull particular notes
commits at the same time (so you might push your notes to
refs/notes/for-linus, and then git would automatically grab the notes
when somebody pulls refs/heads/for-linus).

> Either way is fine for me, though something which lets me "automate"
> that would be appreciated. I can work around that easily as my pull
> requests are generated via scripts, so I can add the secondary one for
> the dependent "notes" branch if necessary. Though it would be nice to
> avoid that. Avoiding that, i.e having a straight connection (maybe
> configurable) between "foo" and "notes/foo" and the commits which have
> not yet hit my upstream maintainer would make my life easier. I.e. I
> just have to check "foo" for stuff which is not upstream yet instead
> of checking both, but that might just be my laziness.
> 
> Thoughts?

That all makes sense. Putting extra work on the puller is not a good
long-term solution. So while sending them an extra "also pull these
notes" line, even if it ends up being a cut-and-pastable single-liner,
is not great (even if it is the most flexible thing). Using a convention
based on name-equivalence seems like a sensible compromise.

-Peff

^ permalink raw reply

* Re: [PATCH] git-submodule add: Record branch name in .gitmodules
From: W. Trevor King @ 2012-10-23 20:55 UTC (permalink / raw)
  To: Jens Lehmann; +Cc: Phil Hord, Nahor, git
In-Reply-To: <5086FFDC.2050700@web.de>

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

On Tue, Oct 23, 2012 at 10:36:44PM +0200, Jens Lehmann wrote:
> Except recording the branch name might raise expectations about what git
> will do with it. And as far as this patch goes, git won't do anything
> with it (yet).

As Phil pointed out, doing anything with this variable is ambiguous:

On Mon, Oct 22, 2012 at 06:03:53PM -0400, Phil Hord wrote:
> Some projects now use the 'branch' config value to record the tracking
> branch for the submodule.  Some ascribe different meaning to the
> configuration if the value is given vs. undefined.  For example, see
> the Gerrit submodule-subscription mechanism.  This change will cause
> those workflows to behave differently than they do now.

On Tue, Oct 23, 2012 at 10:36:44PM +0200, Jens Lehmann wrote:
> But I'd rather see a patch series properly implementing the always-tip
> mode Ævar mentions in f030c96d86 (and which is requested by some users),
> especially the interesting parts: What should git record as commit in
> that case and how are "git status" and "git diff" going to handle
> submodules which shall follow a specific branch. I assume "git submodule
> update" is the right point in time to fetch that branch again and check
> out a newer branch tip if necessary, but should that commit be added to
> the superproject for that submodule automagically or not? This patch
> falls short of this, as it does the easy part but not the interesting
> ones ;-)

I agree that I'm not working on always-tip.  I'm just making that
easier.  For people that aren't interested in always-tip submodules
(e.g. Gerrit folks), this patch is still useful.  It would certainly
be possible to build an always-tip implementation on top of
submodule.$name.branch (as Ævar's one-liner does), but that would be
another patch series.

Personally, I think truly updates should be made explicitly, with a
hand written commit message about why the updates are occuring.  I
also think that setting up and running auto-updates should be easy
one-liners, not long, complicated ones ;).

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* [PATCH v2] git-submodule add: Add -r/--record option.
From: W. Trevor King @ 2012-10-23 21:57 UTC (permalink / raw)
  To: Git; +Cc: Nahor, Phil Hord, W. Trevor King
In-Reply-To: <20121023204437.GE28592@odin.tremily.us>

From: "W. Trevor King" <wking@tremily.us>

This option allows you to record a submodule.<name>.branch option in
.gitmodules.  Git does not currently use this configuration option for
anything, but users have used it for several things, so it makes sense
to add some syntactic sugar for initializing the value.

Current consumers:

Ævar uses this setting to designate the upstream branch for pulling
submodule updates:

  $ git submodule foreach 'git checkout $(git config --file $toplevel/.gitmodules submodule.$name.branch) && git pull'

as he describes in

  commit f030c96d8643fa0a1a9b2bd9c2f36a77721fb61f
  Author: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
  Date:   Fri May 21 16:10:10 2010 +0000

    git-submodule foreach: Add $toplevel variable

Gerrit uses this setting to

  “indicate the branch of a submodule project that when updated will
  trigger automatic update of its registered gitlink.” [1]

I'm not clear on what that means, but they accept special values like
'.', so their usage is not compatible with Ævar's proposal.

By remaining agnostic on the variable usage, this patch makes
submodule setup more convenient for all parties.

[1] https://gerrit.googlesource.com/gerrit/+/master/Documentation/user-submodules.txt

Signed-off-by: W. Trevor King <wking@tremily.us>
---
 Documentation/git-submodule.txt | 11 ++++++++++-
 git-submodule.sh                | 19 ++++++++++++++++++-
 t/t7400-submodule-basic.sh      | 25 +++++++++++++++++++++++++
 3 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index b4683bb..f9c74d6 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -9,7 +9,7 @@ git-submodule - Initialize, update or inspect submodules
 SYNOPSIS
 --------
 [verse]
-'git submodule' [--quiet] add [-b branch] [-f|--force]
+'git submodule' [--quiet] add [-b branch] [--record[=<branch>]] [-f|--force]
 	      [--reference <repository>] [--] <repository> [<path>]
 'git submodule' [--quiet] status [--cached] [--recursive] [--] [<path>...]
 'git submodule' [--quiet] init [--] [<path>...]
@@ -209,6 +209,15 @@ OPTIONS
 --branch::
 	Branch of repository to add as submodule.
 
+-r::
+--record::
+  Record a branch name used as `submodule.<path>.branch` in
+  `.gitmodules` for future reference.  If you do not list an explicit
+  name here, the name given with `--branch` will be recorded.  If that
+  is not set either, `HEAD` will be recorded.  Because the branch name
+  is optional, you must use the equal-sign form (`-r=<branch>`), not
+  `-r <branch>`.
+
 -f::
 --force::
 	This option is only valid for add and update commands.
diff --git a/git-submodule.sh b/git-submodule.sh
index ab6b110..bc33112 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -5,7 +5,7 @@
 # Copyright (c) 2007 Lars Hjemli
 
 dashless=$(basename "$0" | sed -e 's/-/ /')
-USAGE="[--quiet] add [-b branch] [-f|--force] [--reference <repository>] [--] <repository> [<path>]
+USAGE="[--quiet] add [-b branch] [--record[=<branch>]] [-f|--force] [--reference <repository>] [--] <repository> [<path>]
    or: $dashless [--quiet] status [--cached] [--recursive] [--] [<path>...]
    or: $dashless [--quiet] init [--] [<path>...]
    or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
@@ -20,6 +20,8 @@ require_work_tree
 
 command=
 branch=
+record_branch=
+record_branch_empty=
 force=
 reference=
 cached=
@@ -257,6 +259,12 @@ cmd_add()
 			branch=$2
 			shift
 			;;
+		-r | --record)
+			record_branch_empty=true
+			;;
+		-r=* | --record=*)
+			record_branch="${1#*=}"
+			;;
 		-f | --force)
 			force=$1
 			;;
@@ -328,6 +336,11 @@ cmd_add()
 	git ls-files --error-unmatch "$sm_path" > /dev/null 2>&1 &&
 	die "$(eval_gettext "'\$sm_path' already exists in the index")"
 
+	if test -z "$record_branch" && test "$record_branch_empty" = "true"
+	then
+		record_branch="${branch:=HEAD}"
+	fi
+
 	if test -z "$force" && ! git add --dry-run --ignore-missing "$sm_path" > /dev/null 2>&1
 	then
 		eval_gettextln "The following path is ignored by one of your .gitignore files:
@@ -366,6 +379,10 @@ Use -f if you really want to add it." >&2
 
 	git config -f .gitmodules submodule."$sm_path".path "$sm_path" &&
 	git config -f .gitmodules submodule."$sm_path".url "$repo" &&
+	if test -n "$branch"
+	then
+		git config -f .gitmodules submodule."$sm_path".branch "$record_branch"
+	fi &&
 	git add --force .gitmodules ||
 	die "$(eval_gettext "Failed to register submodule '\$sm_path'")"
 }
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 5397037..88ae74c 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -133,6 +133,7 @@ test_expect_success 'submodule add --branch' '
 	(
 		cd addtest &&
 		git submodule add -b initial "$submodurl" submod-branch &&
+		test -z "$(git config -f .gitmodules submodule.submod-branch.branch)" &&
 		git submodule init
 	) &&
 
@@ -211,6 +212,30 @@ test_expect_success 'submodule add with ./, /.. and // in path' '
 	test_cmp empty untracked
 '
 
+test_expect_success 'submodule add --record' '
+	(
+		cd addtest &&
+		git submodule add -r "$submodurl" submod-record-head &&
+		test "$(git config -f .gitmodules submodule.submod-record-head.branch)" = "HEAD"
+	)
+'
+
+test_expect_success 'submodule add --record --branch' '
+	(
+		cd addtest &&
+		git submodule add -r -b initial "$submodurl" submod-auto-record &&
+		test "$(git config -f .gitmodules submodule.submod-auto-record.branch)" = "initial"
+	)
+'
+
+test_expect_success 'submodule add --record=<name> --branch' '
+	(
+		cd addtest &&
+		git submodule add -r=final -b initial "$submodurl" submod-record &&
+		test "$(git config -f .gitmodules submodule.submod-record.branch)" = "final"
+	)
+'
+
 test_expect_success 'setup - add an example entry to .gitmodules' '
 	GIT_CONFIG=.gitmodules \
 	git config submodule.example.url git://example.com/init.git
-- 
1.8.0.1.g61a31f6.dirty

^ permalink raw reply related

* Re: [PATCH] git-submodule add: Record branch name in .gitmodules
From: Nahor @ 2012-10-23 22:02 UTC (permalink / raw)
  To: git; +Cc: wking
In-Reply-To: <5086FFDC.2050700@web.de>

On 2012-10-23 13:36, Jens Lehmann wrote:
> Am 23.10.2012 21:16, schrieb Nahor:
>> Last issue, the branch that exists in your local repository may not
 >> exist in someone else's repository, either because the branch is
 >> purely local, or because it has a different name on the remote repo.
>
> You'll always face this kind of problems with commits too when using
> submodules, so I don't see that as a problem here.

Commits can't change or disappear during "normal" git operation (i.e. 
without using "git push -f" or "git branch -D").
A commit also has the same id in all the clones repository so there is 
no issue of a different name between the local and the remote repositories.

^ permalink raw reply

* Re: [PATCH] tile: support GENERIC_KERNEL_THREAD and GENERIC_KERNEL_EXECVE
From: Jeff King @ 2012-10-23 22:23 UTC (permalink / raw)
  To: Marc Gauthier
  Cc: Thomas Gleixner, Al Viro, Chris Metcalf, LKML,
	linux-arch@vger.kernel.org, Linus Torvalds, Catalin Marinas,
	git@vger.kernel.org, Junio C Hamano
In-Reply-To: <522C1DF17AF50042AD8AE87F7887BD3D0B60880C30@exch.hq.tensilica.com>

On Tue, Oct 23, 2012 at 03:06:59PM -0700, Marc Gauthier wrote:

> Can a later commit be eventually be made to reference some set
> of notes added so far, so they become part of the whole history
> signed by the HEAD SHA1?  hence pulled/pushed automatically as
> well.  Otherwise do you not end up with a forever growing separate
> tree of notes that loses some of the properties of being behind
> the head SHA1 (and perhaps less scalable in manageability)?
> Also that way notes are separate only temporarily.

Interesting idea. It would be tough to do with existing objects. There
are really only two ways for a commit to reference objects:

  1. Via a parent header. But we would not want to include the notes
     tree as a separate parent. The semantics are all wrong, and would
     make your commit look like a nonsense merge.

  2. As an entry in a tree. But we do not enforce connectivity of
     commits referenced in trees, because that is the way that
     submodules are implemented.

So I think we would have to add a new header that says "also, here are
some notes for my history". That has two problems:

  1. It's not backwards compatible. People with the new version of git
     will expect to have objects referenced by the new header, but older
     servers may not provide those objects (and vice versa). We can add
     a protocol extension to communicate this, but fundamentally you are
     going to lose the object connection any time it passes through a
     repo running an older git.

  2. It's impure from the perspective of git's data model. Adding in the
     notes reference is not really a property of the commit. It's more
     like saying "Oh, these other things happened to _past_ commits, and
     I'm just now mentioning them". So you pick an arbitrary commit to
     attach it to. What are the semantics with relation to that commit's
     position in the history graph? If I have a commit that is identical
     but without the notes reference, it will have a different sha1. But
     is it the same commit?

So it's a bit ugly. I think I'd rather build out the transfer
infrastructure to pass the notes references around more gracefully
without trying to shoehorn them into the commit graph.

> As for automating the inclusion of notes in the flow, can that
> be conditional on some pattern in the note, so that e.g. the
> Acked-by's get included and folded in automatically, whereas
> others do not, according to settings?

Yeah. You can store arbitrary data in notes (e.g., one of the existing
uses of notes is to record metadata on the patch emails that led to a
commit). Right now you typically separate it out by data type into
separate refs, and then you ask git log to show you particular ones (so
we see refs/notes/commits with "--notes", but you can do "--notes=foo"
to see refs/notes/foo, or even show multiple refs).

For the fold-on-rebase idea, I'd think you would want something similar,
like setting rebase.foldNotes to "foo" to say "refs/notes/foo contains
pseudo-headers that should be folded in like a signed-off-by".

-Peff

^ permalink raw reply

* [PATCH 0/8] fix git-config with duplicate variable entries
From: Jeff King @ 2012-10-23 22:35 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <CACBZZX5mOb7_i9r8AqNK5V3r-gVnzN+rkeY9xrhecGv1rS-anA@mail.gmail.com>

On Tue, Oct 23, 2012 at 04:13:44PM +0200, Ævar Arnfjörð Bjarmason wrote:

> > It fails a few tests in t1300, but it looks like those tests are testing
> > for the behavior we have identified as wrong, and should be fixed.
> 
> I think this patch looks good.

Thanks. It had a few minor flaws (like a memory leak). I fixed those,
updated the tests, and split it out into a few more readable commits. In
the process, I managed to uncover and fix a few other memory leaks in
the area. I think this version is much more readable, and writing the
rationale for patch 7 convinced me that it's the right thing to do.
Another round of review would be appreciated.

  [1/8]: t1300: style updates
  [2/8]: t1300: remove redundant test
  [3/8]: t1300: test "git config --get-all" more thoroughly
  [4/8]: git-config: remove memory leak of key regexp
  [5/8]: git-config: fix regexp memory leaks on error conditions
  [6/8]: git-config: collect values instead of immediately printing
  [7/8]: git-config: do not complain about duplicate entries
  [8/8]: git-config: use git_config_with_options

For those just joining us, the interesting bit is patch 7, which fixes
some inconsistencies between the "git-config" tool and how the internal
config callbacks work.

> One other thing I think is worth clarifying (and I think should be
> broken) is if you write a configuration like:
> 
>     [foo]
>         bar = one
>     [foo]
>         bar = two
>     [foo]
>         bar = three
> 
> "git-{config,var} -l" will both give you:
> 
>     foo.bar=one
>     foo.bar=two
>     foo.bar=three

Yes, that looks right.

> And git config --get foo.bar will give you:
> 
>     $ git config -f /tmp/test --get foo.bar
>     one
>     error: More than one value for the key foo.bar: two
>     error: More than one value for the key foo.bar: three
> 
> I think that it would be better if the config mechanism just silently
> overwrote keys that clobbered earlier keys like your patch does.

Right.

> But in addition can we simplify things for the consumers of
> "git-{config,var} -l" by only printing:
> 
>     foo.bar=three
> 
> Or are there too many variables like "include.path" that can
> legitimately appear more than once.

No. Some variables can legitimately appear multiple times. E.g.,
remote.*.fetch, remote.*.push, and remote.*.url. Probably more that I am
forgetting. There are not many, but they do exist.

It's OK to tweak the regular "get" for them, since they are already
broken for that case[1]. You need to use "--get-all" if you expect the
variable to have multiple values.  But when we are listing, we do not
have the hint as to what is expected, and we need to show all entries.

-Peff

[1] So the one useful thing that the current duplicate check is doing is
    flagging errors where you wanted to use --get-all, but forgot to.
    However, it's not really a sufficient safeguard anyway, since it
    would not catch cases where the list was split across multiple
    files (which does work with the internal callbacks that handle
    lists, since they never even see that multiple files are involved).
    It's much more important for git-config to be consistent with the
    internal parsing behavior.

^ permalink raw reply

* [PATCH 1/8] t1300: style updates
From: Jeff King @ 2012-10-23 22:35 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <20121023223502.GA23194@sigill.intra.peff.net>

The t1300 test script is quite old, and does not use our
modern techniques or styles. This patch updates it in the
following ways:

  1. Use test_cmp instead of cmp (to make failures easier to
     debug).

  2. Use test_cmp instead of 'test $(command) = expected'.
     This makes failures much easier to debug, and also
     makes sure that $(command) exits appropriately.

  3. Write tests with the usual style of:

       test_expect_success 'test name' '
               test commands &&
	       ...
       '

     rather than one-liners, or using backslash-continuation.
     This is purely a style fixup.

There are still a few command happening outside of
test_expect invocations, but they are all innoccuous system
commands like "cat" and "cp". In an ideal world, each test
would be self sufficient and all commands would happen
inside test_expect, but it is not immediately obvious how
the grouping should work (some of the commands impact the
subsequent tests, and some of them are setting up and
modifying state that many tests depend on). This patch just
picks the low-hanging style fruit, and we can do more fixes
on top later.

Signed-off-by: Jeff King <peff@peff.net>
---
 t/t1300-repo-config.sh | 185 ++++++++++++++++++++++++++++++-------------------
 1 file changed, 113 insertions(+), 72 deletions(-)

diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index e127f35..e12dd4a 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -55,11 +55,13 @@ test_expect_success 'replace with non-match' \
 	test_cmp expect .git/config
 '
 
-test_expect_success 'replace with non-match' \
-	'git config core.penguin kingpin !blue'
+test_expect_success 'replace with non-match' '
+	git config core.penguin kingpin !blue
+'
 
-test_expect_success 'replace with non-match (actually matching)' \
-	'git config core.penguin "very blue" !kingpin'
+test_expect_success 'replace with non-match (actually matching)' '
+	git config core.penguin "very blue" !kingpin
+'
 
 cat > expect << EOF
 [core]
@@ -108,8 +110,9 @@ EOF
 lines
 EOF
 
-test_expect_success 'unset with cont. lines' \
-	'git config --unset beta.baz'
+test_expect_success 'unset with cont. lines' '
+	git config --unset beta.baz
+'
 
 cat > expect <<\EOF
 [alpha]
@@ -133,8 +136,9 @@ cp .git/config .git/config2
 
 cp .git/config .git/config2
 
-test_expect_success 'multiple unset' \
-	'git config --unset-all beta.haha'
+test_expect_success 'multiple unset' '
+	git config --unset-all beta.haha
+'
 
 cat > expect << EOF
 [beta] ; silly comment # another comment
@@ -145,7 +149,9 @@ EOF
 [nextSection] noNewline = ouch
 EOF
 
-test_expect_success 'multiple unset is correct' 'test_cmp expect .git/config'
+test_expect_success 'multiple unset is correct' '
+	test_cmp expect .git/config
+'
 
 cp .git/config2 .git/config
 
@@ -156,8 +162,9 @@ rm .git/config2
 
 rm .git/config2
 
-test_expect_success '--replace-all' \
-	'git config --replace-all beta.haha gamma'
+test_expect_success '--replace-all' '
+	git config --replace-all beta.haha gamma
+'
 
 cat > expect << EOF
 [beta] ; silly comment # another comment
@@ -169,7 +176,9 @@ EOF
 [nextSection] noNewline = ouch
 EOF
 
-test_expect_success 'all replaced' 'test_cmp expect .git/config'
+test_expect_success 'all replaced' '
+	test_cmp expect .git/config
+'
 
 cat > expect << EOF
 [beta] ; silly comment # another comment
@@ -200,7 +209,11 @@ test_expect_success 'really really mean test' '
 	test_cmp expect .git/config
 '
 
-test_expect_success 'get value' 'test alpha = $(git config beta.haha)'
+test_expect_success 'get value' '
+	echo alpha >expect &&
+	git config beta.haha >actual &&
+	test_cmp expect actual
+'
 
 cat > expect << EOF
 [beta] ; silly comment # another comment
@@ -231,18 +244,21 @@ test_expect_success 'ambiguous get' '
 	test_cmp expect .git/config
 '
 
-test_expect_success 'non-match' \
-	'git config --get nextsection.nonewline !for'
+test_expect_success 'non-match' '
+	git config --get nextsection.nonewline !for
+'
 
-test_expect_success 'non-match value' \
-	'test wow = $(git config --get nextsection.nonewline !for)'
+test_expect_success 'non-match value' '
+	test wow = $(git config --get nextsection.nonewline !for)
+'
 
 test_expect_success 'ambiguous get' '
 	test_must_fail git config --get nextsection.nonewline
 '
 
-test_expect_success 'get multivar' \
-	'git config --get-all nextsection.nonewline'
+test_expect_success 'get multivar' '
+	git config --get-all nextsection.nonewline
+'
 
 cat > expect << EOF
 [beta] ; silly comment # another comment
@@ -290,8 +306,9 @@ test_expect_success 'correct key' 'git config 123456.a123 987'
 
 test_expect_success 'correct key' 'git config 123456.a123 987'
 
-test_expect_success 'hierarchical section' \
-	'git config Version.1.2.3eX.Alpha beta'
+test_expect_success 'hierarchical section' '
+	git config Version.1.2.3eX.Alpha beta
+'
 
 cat > expect << EOF
 [beta] ; silly comment # another comment
@@ -307,7 +324,9 @@ EOF
 	Alpha = beta
 EOF
 
-test_expect_success 'hierarchical section value' 'test_cmp expect .git/config'
+test_expect_success 'hierarchical section value' '
+	test_cmp expect .git/config
+'
 
 cat > expect << EOF
 beta.noindent=sillyValue
@@ -316,9 +335,10 @@ EOF
 version.1.2.3eX.alpha=beta
 EOF
 
-test_expect_success 'working --list' \
-	'git config --list > output && cmp output expect'
-
+test_expect_success 'working --list' '
+	git config --list > output &&
+	test_cmp expect output
+'
 cat > expect << EOF
 EOF
 
@@ -332,8 +352,9 @@ EOF
 nextsection.nonewline wow2 for me
 EOF
 
-test_expect_success '--get-regexp' \
-	'git config --get-regexp in > output && cmp output expect'
+test_expect_success '--get-regexp' '
+	git config --get-regexp in > output && test_cmp expect output
+'
 
 cat > expect << EOF
 wow2 for me
@@ -353,41 +374,47 @@ echo false > expect
 	variable =
 EOF
 
-test_expect_success 'get variable with no value' \
-	'git config --get novalue.variable ^$'
+test_expect_success 'get variable with no value' '
+	git config --get novalue.variable ^$
+'
 
-test_expect_success 'get variable with empty value' \
-	'git config --get emptyvalue.variable ^$'
+test_expect_success 'get variable with empty value' '
+	git config --get emptyvalue.variable ^$
+'
 
 echo novalue.variable > expect
 
-test_expect_success 'get-regexp variable with no value' \
-	'git config --get-regexp novalue > output &&
-	 cmp output expect'
+test_expect_success 'get-regexp variable with no value' '
+	git config --get-regexp novalue > output &&
+	 test_cmp expect output'
 
 echo 'novalue.variable true' > expect
 
-test_expect_success 'get-regexp --bool variable with no value' \
-	'git config --bool --get-regexp novalue > output &&
-	 cmp output expect'
+test_expect_success 'get-regexp --bool variable with no value' '
+	git config --bool --get-regexp novalue > output &&
+	test_cmp expect output
+'
 
 echo 'emptyvalue.variable ' > expect
 
-test_expect_success 'get-regexp variable with empty value' \
-	'git config --get-regexp emptyvalue > output &&
-	 cmp output expect'
+test_expect_success 'get-regexp variable with empty value' '
+	git config --get-regexp emptyvalue > output &&
+	test_cmp expect output
+'
 
 echo true > expect
 
-test_expect_success 'get bool variable with no value' \
-	'git config --bool novalue.variable > output &&
-	 cmp output expect'
+test_expect_success 'get bool variable with no value' '
+	git config --bool novalue.variable > output &&
+	test_cmp expect output
+'
 
 echo false > expect
 
-test_expect_success 'get bool variable with empty value' \
-	'git config --bool emptyvalue.variable > output &&
-	 cmp output expect'
+test_expect_success 'get bool variable with empty value' '
+	git config --bool emptyvalue.variable > output &&
+	test_cmp expect output
+'
 
 test_expect_success 'no arguments, but no crash' '
 	test_must_fail git config >output 2>&1 &&
@@ -427,8 +454,9 @@ test_expect_success 'new variable inserts into proper section' '
 	test_cmp expect .git/config
 '
 
-test_expect_success 'alternative GIT_CONFIG (non-existing file should fail)' \
-	'test_must_fail git config --file non-existing-config -l'
+test_expect_success 'alternative GIT_CONFIG (non-existing file should fail)' '
+	test_must_fail git config --file non-existing-config -l
+'
 
 cat > other-config << EOF
 [ein]
@@ -444,8 +472,10 @@ test_expect_success 'alternative GIT_CONFIG' '
 	test_cmp expect output
 '
 
-test_expect_success 'alternative GIT_CONFIG (--file)' \
-	'git config --file other-config -l > output && cmp output expect'
+test_expect_success 'alternative GIT_CONFIG (--file)' '
+	git config --file other-config -l > output &&
+	test_cmp expect output
+'
 
 test_expect_success 'refer config from subdirectory' '
 	mkdir x &&
@@ -489,8 +519,9 @@ EOF
 weird
 EOF
 
-test_expect_success "rename section" \
-	"git config --rename-section branch.eins branch.zwei"
+test_expect_success 'rename section' '
+	git config --rename-section branch.eins branch.zwei
+'
 
 cat > expect << EOF
 # Hallo
@@ -503,17 +534,22 @@ test_expect_success "rename succeeded" "test_cmp expect .git/config"
 weird
 EOF
 
-test_expect_success "rename succeeded" "test_cmp expect .git/config"
+test_expect_success 'rename succeeded' '
+	test_cmp expect .git/config
+'
 
-test_expect_success "rename non-existing section" '
+test_expect_success 'rename non-existing section' '
 	test_must_fail git config --rename-section \
 		branch."world domination" branch.drei
 '
 
-test_expect_success "rename succeeded" "test_cmp expect .git/config"
+test_expect_success 'rename succeeded' '
+	test_cmp expect .git/config
+'
 
-test_expect_success "rename another section" \
-	'git config --rename-section branch."1 234 blabl/a" branch.drei'
+test_expect_success 'rename another section' '
+	git config --rename-section branch."1 234 blabl/a" branch.drei
+'
 
 cat > expect << EOF
 # Hallo
@@ -526,14 +562,17 @@ EOF
 weird
 EOF
 
-test_expect_success "rename succeeded" "test_cmp expect .git/config"
+test_expect_success 'rename succeeded' '
+	test_cmp expect .git/config
+'
 
 cat >> .git/config << EOF
 [branch "vier"] z = 1
 EOF
 
-test_expect_success "rename a section with a var on the same line" \
-	'git config --rename-section branch.vier branch.zwei'
+test_expect_success 'rename a section with a var on the same line' '
+	git config --rename-section branch.vier branch.zwei
+'
 
 cat > expect << EOF
 # Hallo
@@ -548,7 +587,9 @@ EOF
 	z = 1
 EOF
 
-test_expect_success "rename succeeded" "test_cmp expect .git/config"
+test_expect_success 'rename succeeded' '
+	test_cmp expect .git/config
+'
 
 test_expect_success 'renaming empty section name is rejected' '
 	test_must_fail git config --rename-section branch.zwei ""
@@ -562,7 +603,9 @@ EOF
   [branch "zwei"] a = 1 [branch "vier"]
 EOF
 
-test_expect_success "remove section" "git config --remove-section branch.zwei"
+test_expect_success 'remove section' '
+	git config --remove-section branch.zwei
+'
 
 cat > expect << EOF
 # Hallo
@@ -571,8 +614,9 @@ EOF
 weird
 EOF
 
-test_expect_success "section was removed properly" \
-	"test_cmp expect .git/config"
+test_expect_success 'section was removed properly' '
+	test_cmp expect .git/config
+'
 
 cat > expect << EOF
 [gitcvs]
@@ -583,7 +627,6 @@ test_expect_success 'section ending' '
 EOF
 
 test_expect_success 'section ending' '
-
 	rm -f .git/config &&
 	git config gitcvs.enabled true &&
 	git config gitcvs.ext.dbname %Ggitcvs1.%a.%m.sqlite &&
@@ -593,7 +636,6 @@ test_expect_success numbers '
 '
 
 test_expect_success numbers '
-
 	git config kilo.gram 1k &&
 	git config mega.ton 1m &&
 	k=$(git config --int --get kilo.gram) &&
@@ -607,7 +649,6 @@ test_expect_success 'invalid unit' '
 EOF
 
 test_expect_success 'invalid unit' '
-
 	git config aninvalid.unit "1auto" &&
 	s=$(git config aninvalid.unit) &&
 	test "z1auto" = "z$s" &&
@@ -616,7 +657,7 @@ test_expect_success 'invalid unit' '
 		echo config should have failed
 		false
 	fi &&
-	cmp actual expect
+	test_cmp actual expect
 '
 
 cat > expect << EOF
@@ -646,7 +687,7 @@ test_expect_success bool '
 	    git config --bool --get bool.true$i >>result
 	    git config --bool --get bool.false$i >>result
         done &&
-	cmp expect result'
+	test_cmp expect result'
 
 test_expect_success 'invalid bool (--get)' '
 
@@ -680,7 +721,7 @@ test_expect_success 'set --bool' '
 	git config --bool bool.false2 "" &&
 	git config --bool bool.false3 nO &&
 	git config --bool bool.false4 FALSE &&
-	cmp expect .git/config'
+	test_cmp expect .git/config'
 
 cat > expect <<\EOF
 [int]
@@ -695,7 +736,7 @@ test_expect_success 'set --int' '
 	git config --int int.val1 01 &&
 	git config --int int.val2 -1 &&
 	git config --int int.val3 5m &&
-	cmp expect .git/config'
+	test_cmp expect .git/config'
 
 cat >expect <<\EOF
 [bool]
@@ -844,7 +885,7 @@ test_expect_success 'value continued on next line' '
 
 test_expect_success 'value continued on next line' '
 	git config --list > result &&
-	cmp result expect
+	test_cmp result expect
 '
 
 cat > .git/config <<\EOF
-- 
1.8.0.3.g3456896

^ permalink raw reply related

* [PATCH 2/8] t1300: remove redundant test
From: Jeff King @ 2012-10-23 22:36 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <20121023223502.GA23194@sigill.intra.peff.net>

This test checks that git-config fails for an ambiguous
"get", but we check the exact same thing 3 tests beforehand.

Signed-off-by: Jeff King <peff@peff.net>
---
I update the matching test later in the series, and I didn't want to
have to do it twice.

 t/t1300-repo-config.sh | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index e12dd4a..c6489dc 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -275,10 +275,6 @@ test_expect_success 'multivar replace' '
 	test_cmp expect .git/config
 '
 
-test_expect_success 'ambiguous value' '
-	test_must_fail git config nextsection.nonewline
-'
-
 test_expect_success 'ambiguous unset' '
 	test_must_fail git config --unset nextsection.nonewline
 '
-- 
1.8.0.3.g3456896

^ permalink raw reply related

* [PATCH 3/8] t1300: test "git config --get-all" more thoroughly
From: Jeff King @ 2012-10-23 22:36 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <20121023223502.GA23194@sigill.intra.peff.net>

We check that we can "--get-all" a multi-valued variable,
but we do not actually confirm that the output is sensible.
Doing so reveals that it works fine, but this will help us
ensure we do not have regressions in the next few patches,
which will touch this area.

Signed-off-by: Jeff King <peff@peff.net>
---
 t/t1300-repo-config.sh | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index c6489dc..74a297e 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -256,8 +256,13 @@ test_expect_success 'ambiguous get' '
 	test_must_fail git config --get nextsection.nonewline
 '
 
-test_expect_success 'get multivar' '
-	git config --get-all nextsection.nonewline
+test_expect_success 'multi-valued get-all returns all' '
+	cat >expect <<-\EOF &&
+	wow
+	wow2 for me
+	EOF
+	git config --get-all nextsection.nonewline >actual &&
+	test_cmp expect actual
 '
 
 cat > expect << EOF
-- 
1.8.0.3.g3456896

^ permalink raw reply related

* [PATCH 4/8] git-config: remove memory leak of key regexp
From: Jeff King @ 2012-10-23 22:36 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <20121023223502.GA23194@sigill.intra.peff.net>

This is only called once per invocation, so it's not a major
leak, but it's easy to fix.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/config.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/builtin/config.c b/builtin/config.c
index e1c33e0..e660d48 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -248,6 +248,10 @@ static int get_value(const char *key_, const char *regex_)
 		git_config_from_file(fn, system_wide, data);
 
 	free(key);
+	if (key_regexp) {
+		regfree(key_regexp);
+		free(key_regexp);
+	}
 	if (regexp) {
 		regfree(regexp);
 		free(regexp);
-- 
1.8.0.3.g3456896

^ permalink raw reply related

* [PATCH 5/8] git-config: fix regexp memory leaks on error conditions
From: Jeff King @ 2012-10-23 22:38 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <20121023223502.GA23194@sigill.intra.peff.net>

The get_value function has a goto label for cleaning up on
errors, but it only cleans up half of what the function
might allocate. Let's also clean up the key and regexp
variables there.

Note that we need to take special care when compiling the
regex fails to clean it up ourselves, since it is in a
half-constructed state (we would want to free it, but not
regfree it).

Similarly, we fix git_config_parse_key to return NULL when it
fails, not a pointer to some already-freed memory.

Signed-off-by: Jeff King <peff@peff.net>
---
The diff is annoying in an interesting way: what I actually did was move
the regex cleanup code down, but it shows it as moving the bottom bits
up. I think it is just one of those ambiguous cases where either way is
equally valid and minimal.

 builtin/config.c | 23 +++++++++++++----------
 config.c         |  1 +
 2 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index e660d48..60d36e7 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -195,7 +195,8 @@ static int get_value(const char *key_, const char *regex_)
 		key_regexp = (regex_t*)xmalloc(sizeof(regex_t));
 		if (regcomp(key_regexp, key, REG_EXTENDED)) {
 			fprintf(stderr, "Invalid key pattern: %s\n", key_);
-			free(key);
+			free(key_regexp);
+			key_regexp = NULL;
 			ret = CONFIG_INVALID_PATTERN;
 			goto free_strings;
 		}
@@ -215,6 +216,8 @@ static int get_value(const char *key_, const char *regex_)
 		regexp = (regex_t*)xmalloc(sizeof(regex_t));
 		if (regcomp(regexp, regex_, REG_EXTENDED)) {
 			fprintf(stderr, "Invalid pattern: %s\n", regex_);
+			free(regexp);
+			regexp = NULL;
 			ret = CONFIG_INVALID_PATTERN;
 			goto free_strings;
 		}
@@ -247,6 +250,15 @@ static int get_value(const char *key_, const char *regex_)
 	if (!do_all && !seen && system_wide)
 		git_config_from_file(fn, system_wide, data);
 
+	if (do_all)
+		ret = !seen;
+	else
+		ret = (seen == 1) ? 0 : seen > 1 ? 2 : 1;
+
+free_strings:
+	free(repo_config);
+	free(global);
+	free(xdg);
 	free(key);
 	if (key_regexp) {
 		regfree(key_regexp);
@@ -257,15 +269,6 @@ static int get_value(const char *key_, const char *regex_)
 		free(regexp);
 	}
 
-	if (do_all)
-		ret = !seen;
-	else
-		ret = (seen == 1) ? 0 : seen > 1 ? 2 : 1;
-
-free_strings:
-	free(repo_config);
-	free(global);
-	free(xdg);
 	return ret;
 }
 
diff --git a/config.c b/config.c
index 08e47e2..2fbe634 100644
--- a/config.c
+++ b/config.c
@@ -1280,6 +1280,7 @@ out_free_ret_1:
 
 out_free_ret_1:
 	free(*store_key);
+	*store_key = NULL;
 	return -CONFIG_INVALID_KEY;
 }
 
-- 
1.8.0.3.g3456896

^ permalink raw reply related

* [PATCH 6/8] git-config: collect values instead of immediately printing
From: Jeff King @ 2012-10-23 22:40 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <20121023223502.GA23194@sigill.intra.peff.net>

This is a refactor that will allow us to more easily tweak
the behavior for multi-valued variables, and it will
ultimately allow us to remove a lot git-config's custom code
in favor of the regular git_config code.

It does mean we're no longer streaming, and we're storing
more in memory for the --get-all case, but in practice it is
a tiny amount of data, and the results are instantaneous.

Signed-off-by: Jeff King <peff@peff.net>
---
The increase in line count is nicely offset by the next two patches.

 builtin/config.c | 50 +++++++++++++++++++++++++++++++++++---------------
 1 file changed, 35 insertions(+), 15 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index 60d36e7..08e83fc 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -15,7 +15,6 @@ static int do_not_match;
 static int use_key_regexp;
 static int do_all;
 static int do_not_match;
-static int seen;
 static char delim = '=';
 static char key_delim = ' ';
 static char term = '\n';
@@ -95,8 +94,16 @@ static int show_config(const char *key_, const char *value_, void *cb)
 	return 0;
 }
 
-static int show_config(const char *key_, const char *value_, void *cb)
+struct strbuf_list {
+	struct strbuf *items;
+	int nr;
+	int alloc;
+};
+
+static int collect_config(const char *key_, const char *value_, void *cb)
 {
+	struct strbuf_list *values = cb;
+	struct strbuf *buf;
 	char value[256];
 	const char *vptr = value;
 	int must_free_vptr = 0;
@@ -111,11 +118,15 @@ static int show_config(const char *key_, const char *value_, void *cb)
 	    (do_not_match ^ !!regexec(regexp, (value_?value_:""), 0, NULL, 0)))
 		return 0;
 
+	ALLOC_GROW(values->items, values->nr + 1, values->alloc);
+	buf = &values->items[values->nr++];
+	strbuf_init(buf, 0);
+
 	if (show_keys) {
-		printf("%s", key_);
+		strbuf_addstr(buf, key_);
 		must_print_delim = 1;
 	}
-	if (seen && !do_all)
+	if (values->nr > 1 && !do_all)
 		dup_error = 1;
 	if (types == TYPE_INT)
 		sprintf(value, "%d", git_config_int(key_, value_?value_:""));
@@ -138,15 +149,15 @@ static int show_config(const char *key_, const char *value_, void *cb)
 		vptr = "";
 		must_print_delim = 0;
 	}
-	seen++;
 	if (dup_error) {
 		error("More than one value for the key %s: %s",
 				key_, vptr);
 	}
 	else {
 		if (must_print_delim)
-			printf("%c", key_delim);
-		printf("%s%c", vptr, term);
+			strbuf_addch(buf, key_delim);
+		strbuf_addstr(buf, vptr);
+		strbuf_addch(buf, term);
 	}
 	if (must_free_vptr)
 		/* If vptr must be freed, it's a pointer to a
@@ -166,6 +177,8 @@ static int get_value(const char *key_, const char *regex_)
 	struct config_include_data inc = CONFIG_INCLUDE_INIT;
 	config_fn_t fn;
 	void *data;
+	struct strbuf_list values = {0};
+	int i;
 
 	local = given_config_file;
 	if (!local) {
@@ -223,8 +236,8 @@ static int get_value(const char *key_, const char *regex_)
 		}
 	}
 
-	fn = show_config;
-	data = NULL;
+	fn = collect_config;
+	data = &values;
 	if (respect_includes) {
 		inc.fn = fn;
 		inc.data = data;
@@ -241,19 +254,26 @@ static int get_value(const char *key_, const char *regex_)
 	if (do_all)
 		git_config_from_file(fn, local, data);
 	git_config_from_parameters(fn, data);
-	if (!do_all && !seen)
+	if (!do_all && !values.nr)
 		git_config_from_file(fn, local, data);
-	if (!do_all && !seen && global)
+	if (!do_all && !values.nr && global)
 		git_config_from_file(fn, global, data);
-	if (!do_all && !seen && xdg)
+	if (!do_all && !values.nr && xdg)
 		git_config_from_file(fn, xdg, data);
-	if (!do_all && !seen && system_wide)
+	if (!do_all && !values.nr && system_wide)
 		git_config_from_file(fn, system_wide, data);
 
 	if (do_all)
-		ret = !seen;
+		ret = !values.nr;
 	else
-		ret = (seen == 1) ? 0 : seen > 1 ? 2 : 1;
+		ret = (values.nr == 1) ? 0 : values.nr > 1 ? 2 : 1;
+
+	for (i = 0; i < values.nr; i++) {
+		struct strbuf *buf = values.items + i;
+		fwrite(buf->buf, 1, buf->len, stdout);
+		strbuf_release(buf);
+	}
+	free(values.items);
 
 free_strings:
 	free(repo_config);
-- 
1.8.0.3.g3456896

^ permalink raw reply related

* [PATCH 7/8] git-config: do not complain about duplicate entries
From: Jeff King @ 2012-10-23 22:41 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <20121023223502.GA23194@sigill.intra.peff.net>

If git-config is asked for a single value, it will complain
and exit with an error if it finds multiple instances of
that value. This is unlike the usual internal config
parsing, however, which will generally overwrite previous
values, leaving only the final one. For example:

  [set a multivar]
  $ git config user.email one@example.com
  $ git config --add user.email two@example.com

  [use the internal parser to fetch it]
  $ git var GIT_AUTHOR_IDENT
  Your Name <two@example.com> ...

  [use git-config to fetch it]
  $ git config user.email
  one@example.com
  error: More than one value for the key user.email: two@example.com

This overwriting behavior is critical for the regular
parser, which starts with the lowest-priority file (e.g.,
/etc/gitconfig) and proceeds to the highest-priority file
($GIT_DIR/config). Overwriting yields the highest priority
value at the end.

Git-config solves this problem by implementing its own
parsing. It goes from highest to lowest priorty, but does
not proceed to the next file if it has seen a value.

So in practice, this distinction never mattered much,
because it only triggered for values in the same file. And
there was not much point in doing that; the real value is in
overwriting values from lower-priority files.

However, this changed with the implementation of config
include files. Now we might see an include overriding a
value from the parent file, which is a sensible thing to do,
but git-config will flag as a duplication.

This patch drops the duplicate detection for git-config and
switches to a pure-overwrite model (for the single case;
--get-all can still be used if callers want to do something
more fancy).

As is shown by the modifications to the test suite, this is
a user-visible change in behavior. An alternative would be
to just change the include case, but this is much cleaner
for a few reasons:

  1. If you change the include case, then to what? If you
     just stop parsing includes after getting a value, then
     you will get a _different_ answer than the regular
     config parser (you'll get the first value instead of
     the last value). So you'd want to implement overwrite
     semantics anyway.

  2. Even though it is a change in behavior for git-config,
     it is bringing us in line with what the internal
     parsers already do.

  3. The file-order reimplementation is the only thing
     keeping us from sharing more code with the internal
     config parser, which will help keep differences to a
     minimum.

Going under the assumption that the primary purpose of
git-config is to behave identically to how git's internal
parsing works, this change can be seen as a bug-fix.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/config.c       | 27 +++++++++------------------
 t/t1300-repo-config.sh |  6 ++++--
 t/t9700/test.pl        |  3 +--
 3 files changed, 14 insertions(+), 22 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index 08e83fc..77efa69 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -107,7 +107,6 @@ static int collect_config(const char *key_, const char *value_, void *cb)
 	char value[256];
 	const char *vptr = value;
 	int must_free_vptr = 0;
-	int dup_error = 0;
 	int must_print_delim = 0;
 
 	if (!use_key_regexp && strcmp(key_, key))
@@ -126,8 +125,6 @@ static int collect_config(const char *key_, const char *value_, void *cb)
 		strbuf_addstr(buf, key_);
 		must_print_delim = 1;
 	}
-	if (values->nr > 1 && !do_all)
-		dup_error = 1;
 	if (types == TYPE_INT)
 		sprintf(value, "%d", git_config_int(key_, value_?value_:""));
 	else if (types == TYPE_BOOL)
@@ -149,16 +146,12 @@ static int collect_config(const char *key_, const char *value_, void *cb)
 		vptr = "";
 		must_print_delim = 0;
 	}
-	if (dup_error) {
-		error("More than one value for the key %s: %s",
-				key_, vptr);
-	}
-	else {
-		if (must_print_delim)
-			strbuf_addch(buf, key_delim);
-		strbuf_addstr(buf, vptr);
-		strbuf_addch(buf, term);
-	}
+
+	if (must_print_delim)
+		strbuf_addch(buf, key_delim);
+	strbuf_addstr(buf, vptr);
+	strbuf_addch(buf, term);
+
 	if (must_free_vptr)
 		/* If vptr must be freed, it's a pointer to a
 		 * dynamically allocated buffer, it's safe to cast to
@@ -263,14 +256,12 @@ static int get_value(const char *key_, const char *regex_)
 	if (!do_all && !values.nr && system_wide)
 		git_config_from_file(fn, system_wide, data);
 
-	if (do_all)
-		ret = !values.nr;
-	else
-		ret = (values.nr == 1) ? 0 : values.nr > 1 ? 2 : 1;
+	ret = !values.nr;
 
 	for (i = 0; i < values.nr; i++) {
 		struct strbuf *buf = values.items + i;
-		fwrite(buf->buf, 1, buf->len, stdout);
+		if (do_all || i == values.nr - 1)
+			fwrite(buf->buf, 1, buf->len, stdout);
 		strbuf_release(buf);
 	}
 	free(values.items);
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 74a297e..8cb45f1 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -252,8 +252,10 @@ test_expect_success 'non-match value' '
 	test wow = $(git config --get nextsection.nonewline !for)
 '
 
-test_expect_success 'ambiguous get' '
-	test_must_fail git config --get nextsection.nonewline
+test_expect_success 'multi-valued get returns final one' '
+	echo "wow2 for me" >expect &&
+	git config --get nextsection.nonewline >actual &&
+	test_cmp expect actual
 '
 
 test_expect_success 'multi-valued get-all returns all' '
diff --git a/t/t9700/test.pl b/t/t9700/test.pl
index 3b9b484..0d4e366 100755
--- a/t/t9700/test.pl
+++ b/t/t9700/test.pl
@@ -46,8 +46,7 @@ BEGIN
 # Save and restore STDERR; we will probably extract this into a
 # "dies_ok" method and possibly move the STDERR handling to Git.pm.
 open our $tmpstderr, ">&STDERR" or die "cannot save STDERR"; close STDERR;
-eval { $r->config("test.dupstring") };
-ok($@, "config: duplicate entry in scalar context fails");
+is($r->config("test.dupstring"), "value2", "config: multivar");
 eval { $r->config_bool("test.boolother") };
 ok($@, "config_bool: non-boolean values fail");
 open STDERR, ">&", $tmpstderr or die "cannot restore STDERR";
-- 
1.8.0.3.g3456896

^ permalink raw reply related

* [PATCH 8/8] git-config: use git_config_with_options
From: Jeff King @ 2012-10-23 22:41 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <20121023223502.GA23194@sigill.intra.peff.net>

The git-config command has always implemented its own file
lookup and parsing order. This was necessary because its
duplicate-entry handling did not match the way git's
internal callbacks worked. Now that this is no longer the
case, we are free to reuse the existing parsing code.

This saves us a few lines of code, but most importantly, it
means that the logic for which files are examined is
contained only in one place and cannot diverge.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/config.c | 44 ++------------------------------------------
 1 file changed, 2 insertions(+), 42 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index 77efa69..f881053 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -165,22 +165,9 @@ static int get_value(const char *key_, const char *regex_)
 static int get_value(const char *key_, const char *regex_)
 {
 	int ret = CONFIG_GENERIC_ERROR;
-	char *global = NULL, *xdg = NULL, *repo_config = NULL;
-	const char *system_wide = NULL, *local;
-	struct config_include_data inc = CONFIG_INCLUDE_INIT;
-	config_fn_t fn;
-	void *data;
 	struct strbuf_list values = {0};
 	int i;
 
-	local = given_config_file;
-	if (!local) {
-		local = repo_config = git_pathdup("config");
-		if (git_config_system())
-			system_wide = git_etc_gitconfig();
-		home_config_paths(&global, &xdg, "config");
-	}
-
 	if (use_key_regexp) {
 		char *tl;
 
@@ -229,32 +216,8 @@ static int get_value(const char *key_, const char *regex_)
 		}
 	}
 
-	fn = collect_config;
-	data = &values;
-	if (respect_includes) {
-		inc.fn = fn;
-		inc.data = data;
-		fn = git_config_include;
-		data = &inc;
-	}
-
-	if (do_all && system_wide)
-		git_config_from_file(fn, system_wide, data);
-	if (do_all && xdg)
-		git_config_from_file(fn, xdg, data);
-	if (do_all && global)
-		git_config_from_file(fn, global, data);
-	if (do_all)
-		git_config_from_file(fn, local, data);
-	git_config_from_parameters(fn, data);
-	if (!do_all && !values.nr)
-		git_config_from_file(fn, local, data);
-	if (!do_all && !values.nr && global)
-		git_config_from_file(fn, global, data);
-	if (!do_all && !values.nr && xdg)
-		git_config_from_file(fn, xdg, data);
-	if (!do_all && !values.nr && system_wide)
-		git_config_from_file(fn, system_wide, data);
+	git_config_with_options(collect_config, &values,
+				given_config_file, respect_includes);
 
 	ret = !values.nr;
 
@@ -267,9 +230,6 @@ free_strings:
 	free(values.items);
 
 free_strings:
-	free(repo_config);
-	free(global);
-	free(xdg);
 	free(key);
 	if (key_regexp) {
 		regfree(key_regexp);
-- 
1.8.0.3.g3456896

^ permalink raw reply related

* [PATCH] Add -S, --gpg-sign option to manpage of "git commit"
From: Tom Jones @ 2012-10-21 19:46 UTC (permalink / raw)
  To: git; +Cc: tom
In-Reply-To: <7vbofvfup7.fsf@alter.siamese.dyndns.org>

git commit -S, --gpg-sign was mentioned in the program's help message,
but not in the manpage.

This adds an equivalent entry for the option in the manpage.

Signed-off-by: Tom Jones <tom@oxix.org>
---
On Sun, Oct 21, 2012 at 01:15:16PM -0700, Junio C Hamano wrote:
> Are you sure about this?  The order [...]

Good point.  Please find a revised patch, with the newly documented
option before the optional double dashes, below.

> Sign off?

Now added, too.

 Documentation/git-commit.txt |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 9594ac8..4b78bd0 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -13,7 +13,7 @@ SYNOPSIS
 	   [-F <file> | -m <msg>] [--reset-author] [--allow-empty]
 	   [--allow-empty-message] [--no-verify] [-e] [--author=<author>]
 	   [--date=<date>] [--cleanup=<mode>] [--status | --no-status]
-	   [-i | -o] [--] [<file>...]
+	   [-i | -o] [-S[keyid]] [--] [<file>...]
 
 DESCRIPTION
 -----------
@@ -276,6 +276,10 @@ configuration variable documented in linkgit:git-config[1].
 	commit message template when using an editor to prepare the
 	default commit message.
 
+-S[<keyid>]::
+--gpg-sign[=<keyid>]::
+	GPG-sign commit.
+
 \--::
 	Do not interpret any more arguments as options.
 
-- 
1.7.2.5

^ permalink raw reply related

* Re: [PATCH 2/2] git svn: canonicalize_url(): use svn_path_canonicalize when available
From: Eric Wong @ 2012-10-23 22:58 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Michael G Schwern, git, gitster, robbat2, bwalton
In-Reply-To: <20121014114857.GB21106@elie.Belkin>

Jonathan Nieder <jrnieder@gmail.com> wrote:
> Until Subversion 1.7 (more precisely r873487), the standard way to
> canonicalize a URI was to call svn_path_canonicalize().  Use it.
> 
> This saves "git svn" from having to rely on our imperfect
> reimplementation of the same.  If the function doesn't exist or
> returns undef, though, it can use the fallback code, which we keep to
> be conservative.  Since svn_path_canonicalize() was added before
> Subversion 1.1, hopefully that doesn't happen often.

Hi Jonathan, this fails for me using http (but not file:// or svn://).
1/2 of this RFC looks fine, though.

subversion 1.6.12dfsg-6, apache2-mpm-prefork 2.2.16-6+squeeze8
(Debian squeeze)

$ SVN_HTTPD_PORT=12345 sh t9118-git-svn-funky-branch-names.sh -v
Initialized empty Git repository in /home/ew/git-core/t/trash directory.t9118-git-svn-funky-branch-names/.git/
expecting success: 
	mkdir project project/trunk project/branches project/tags &&
	echo foo > project/trunk/foo &&
	svn_cmd import -m "$test_description" project "$svnrepo/pr ject" &&
	rm -rf project &&
	svn_cmd cp -m "fun" "$svnrepo/pr ject/trunk" \
	                "$svnrepo/pr ject/branches/fun plugin" &&
	svn_cmd cp -m "more fun!" "$svnrepo/pr ject/branches/fun plugin" \
	                      "$svnrepo/pr ject/branches/more fun plugin!" &&
	svn_cmd cp -m "scary" "$svnrepo/pr ject/branches/fun plugin" \
	              "$svnrepo/pr ject/branches/$scary_uri" &&
	svn_cmd cp -m "leading dot" "$svnrepo/pr ject/trunk" \
			"$svnrepo/pr ject/branches/.leading_dot" &&
	svn_cmd cp -m "trailing dot" "$svnrepo/pr ject/trunk" \
			"$svnrepo/pr ject/branches/trailing_dot." &&
	svn_cmd cp -m "trailing .lock" "$svnrepo/pr ject/trunk" \
			"$svnrepo/pr ject/branches/trailing_dotlock.lock" &&
	svn_cmd cp -m "reflog" "$svnrepo/pr ject/trunk" \
			"$svnrepo/pr ject/branches/not-a@{0}reflog@" &&
	start_httpd
	
Adding         project/trunk
Adding         project/trunk/foo
Adding         project/branches
Adding         project/tags

Committed revision 1.

Committed revision 2.

Committed revision 3.

Committed revision 4.

Committed revision 5.

Committed revision 6.

Committed revision 7.

Committed revision 8.
ok 1 - setup svnrepo

expecting success: 
	git svn clone -s "$svnrepo/pr ject" project &&
	(
		cd project &&
		git rev-parse "refs/remotes/fun%20plugin" &&
		git rev-parse "refs/remotes/more%20fun%20plugin!" &&
		git rev-parse "refs/remotes/$scary_ref" &&
		git rev-parse "refs/remotes/%2Eleading_dot" &&
		git rev-parse "refs/remotes/trailing_dot%2E" &&
		git rev-parse "refs/remotes/trailing_dotlock%2Elock" &&
		git rev-parse "refs/remotes/$non_reflog"
	)
	
Initialized empty Git repository in /home/ew/git-core/t/trash directory.t9118-git-svn-funky-branch-names/project/.git/
Bad URL passed to RA layer: URL 'http://127.0.0.1:12345/pr ject' is malformed or the scheme or host or path is missing at /home/ew/git-core/perl/blib/lib/Git/SVN.pm line 310

not ok - 2 test clone with funky branch names
#	
#		git svn clone -s "$svnrepo/pr ject" project &&
#		(
#			cd project &&
#			git rev-parse "refs/remotes/fun%20plugin" &&
#			git rev-parse "refs/remotes/more%20fun%20plugin!" &&
#			git rev-parse "refs/remotes/$scary_ref" &&
#			git rev-parse "refs/remotes/%2Eleading_dot" &&
#			git rev-parse "refs/remotes/trailing_dot%2E" &&
#			git rev-parse "refs/remotes/trailing_dotlock%2Elock" &&
#			git rev-parse "refs/remotes/$non_reflog"
#		)
#		

expecting success: 
	(
		cd project &&
		git reset --hard 'refs/remotes/more%20fun%20plugin!' &&
		echo hello >> foo &&
		git commit -m 'hello' -- foo &&
		git svn dcommit
	)
	
fatal: ambiguous argument 'refs/remotes/more%20fun%20plugin!': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'
not ok - 3 test dcommit to funky branch
#	
#		(
#			cd project &&
#			git reset --hard 'refs/remotes/more%20fun%20plugin!' &&
#			echo hello >> foo &&
#			git commit -m 'hello' -- foo &&
#			git svn dcommit
#		)
#		

expecting success: 
	(
		cd project &&
		git reset --hard "refs/remotes/$scary_ref" &&
		echo urls are scary >> foo &&
		git commit -m "eep" -- foo &&
		git svn dcommit
	)
	
fatal: ambiguous argument 'refs/remotes/Abo-Uebernahme%20(Bug%20#994)': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'
not ok - 4 test dcommit to scary branch
#	
#		(
#			cd project &&
#			git reset --hard "refs/remotes/$scary_ref" &&
#			echo urls are scary >> foo &&
#			git commit -m "eep" -- foo &&
#			git svn dcommit
#		)
#		

expecting success: 
	(
		cd project &&
		git reset --hard "refs/remotes/trailing_dotlock%2Elock" &&
		echo who names branches like this anyway? >> foo &&
		git commit -m "bar" -- foo &&
		git svn dcommit
	)
	
fatal: ambiguous argument 'refs/remotes/trailing_dotlock%2Elock': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'
not ok - 5 test dcommit to trailing_dotlock branch
#	
#		(
#			cd project &&
#			git reset --hard "refs/remotes/trailing_dotlock%2Elock" &&
#			echo who names branches like this anyway? >> foo &&
#			git commit -m "bar" -- foo &&
#			git svn dcommit
#		)
#		

# failed 4 among 5 test(s)
1..5

^ permalink raw reply

* Re: Git submodule for a local branch?
From: W. Trevor King @ 2012-10-23 22:09 UTC (permalink / raw)
  To: Jens Lehmann; +Cc: Git
In-Reply-To: <508704D5.9020902@web.de>

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

On Tue, Oct 23, 2012 at 10:57:57PM +0200, Jens Lehmann wrote:
> Am 22.10.2012 14:37, schrieb W. Trevor King:
> > but cloning a remote repository (vs. checking out a local branch)
> > seems to be baked into the submodule implementation.  Should I be
> > thinking about generalizing git-submodule.sh, or am I looking under
> > the wrong rock?  My ideal syntax would be something like
> > 
> >   $ git submodule add -b c --local dir-for-c/
> 
> But then we'd have to be able to have two (or more) work trees using
> the same git directory, which current submodule code can't.

And that's the problem I'm trying to solve ;).

> > The motivation is that I have website that contains a bunch of
> > sub-sites, and the sub-sites share content.  I have per-sub-site
> > branches (a, b, c) and want a master branch (index) that aggregates
> > them.  Perhaps this is too much to wedge into a single repository?
> 
> To me this sounds upside-down. I'd put the three sub-sites into
> different repositories and the shared content into a submodule that
> all three sub-sites use. At least that is how I do all my content
> sharing on the websites I have done ... does that make sense?

That makes sense, however the problem is not in the common content, it
is in the final index:

  index
  |-- sub-site a (branch of sub-site-x)
  |-- sub-site b (branch of sub-site-x)
  `-- sub-site c (branch of sub-site-x)

All of the sub-sites are branches of a single sub-site-x master:

  o--o--o--o   sub-site-x
   \--o--o--o  sub-site-1
       \--o    sub-site-2
        \--o   sub-site-3

So they all live in the same repository.  My index repository will
have submodules for each of the sub-sites, and I'd like the index
branch to *also* live in same repository as the subsites.  This last
bit is the sticky part.

For a proof-of-concept example (where I currently use public
repositories for the sub-site submodules), see

  http://wking.github.com/swc-workshop/

which uses gh-pages as the index branch, and master, 2012-10-caltech,
and 2012-10-lbl for the sub-site branches.

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ 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