Git development
 help / color / mirror / Atom feed
* Re: git auto-repack is broken...
From: Nicolas Pitre @ 2011-12-08  0:18 UTC (permalink / raw)
  To: Jeff King
  Cc: Brandon Casey, Junio C Hamano, Linus Torvalds,
	Ævar Arnfjörð, Git Mailing List
In-Reply-To: <20111207225318.GA21852@sigill.intra.peff.net>

On Wed, 7 Dec 2011, Jeff King wrote:

> On Wed, Dec 07, 2011 at 05:12:14PM -0500, Nicolas Pitre wrote:
> 
> > Maybe  FETCH_HEAD should have a reflog too?
> 
> That might be nice. However, there is a complication, in that FETCH_HEAD
> may contain many sha1s, but each reflog entry only has room for a single
> sha1 transition. You could obviously encode it as a series of reflog
> entries, but then "git show FETCH_HEAD@{1}" is not very meaningful.

What does "git show FETCH_HEAD" do now?  If it shows only one 
(presumably the first) SHA1 then its reflog doesn't have to be smarter, 
which would properly cover most cases already.  I certainly never did a 
multi-ref fetch myself.


Nicolas

^ permalink raw reply

* Re: git auto-repack is broken...
From: Jeff King @ 2011-12-08  0:45 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: Brandon Casey, Junio C Hamano, Linus Torvalds,
	Ævar Arnfjörð, Git Mailing List
In-Reply-To: <alpine.LFD.2.02.1112071912570.2907@xanadu.home>

On Wed, Dec 07, 2011 at 07:18:13PM -0500, Nicolas Pitre wrote:

> > > Maybe  FETCH_HEAD should have a reflog too?
> > 
> > That might be nice. However, there is a complication, in that FETCH_HEAD
> > may contain many sha1s, but each reflog entry only has room for a single
> > sha1 transition. You could obviously encode it as a series of reflog
> > entries, but then "git show FETCH_HEAD@{1}" is not very meaningful.
> 
> What does "git show FETCH_HEAD" do now?  If it shows only one
> (presumably the first) SHA1 then its reflog doesn't have to be
> smarter, which would properly cover most cases already.

Are you proposing that it only store the first ref in the reflog, or
that we accept that a single fetch may write lots of reflog entries?

If the former, then you are missing the expiration/connectivity
properties.

If the latter, then it is not just "we only show the first one for
FETCH_HEAD@{1}", but also "the thing that used to be FETCH_HEAD@{1} does
not graduate to FETCH_HEAD@{2}, but rather FETCH_HEAD@{n} for some
unknown n". That may be an acceptable limitation; I just wanted to
mention it in case somebody can think of some clever solution.

> I certainly never did a multi-ref fetch myself.

Not consciously, perhaps, but you do it all the time without realizing
it:

  $ git clone git://git.kernel.org/pub/scm/git/git.git
  $ cd git
  $ git fetch -v origin
   = [up to date]      maint      -> origin/maint
   = [up to date]      master     -> origin/master
   = [up to date]      next       -> origin/next
   = [up to date]      pu         -> origin/pu
   = [up to date]      todo       -> origin/todo
  $ cat .git/FETCH_HEAD
  b1af9630d758e1728fc0008b3f18d90d8f87f4c5        not-for-merge   branch 'maint' of git://git.kernel.org/pub/scm/git/git
  4cb5d10b14dcbe0155bed9c45ccb94e83bd4c599                branch 'master' of git://git.kernel.org/pub/scm/git/git
  03e5527c5df33d4550ccc1446d861c0aa5689d58        not-for-merge   branch 'next' of git://git.kernel.org/pub/scm/git/git
  cc4e3f01fc6a5e09ae5bbdc464965981fae4cf39        not-for-merge   branch 'pu' of git://git.kernel.org/pub/scm/git/git
  7a02dba15bd28826344f9c14a5e2b5c57eeb7e50        not-for-merge   branch 'todo' of git://git.kernel.org/pub/scm/git/git

-Peff

^ permalink raw reply

* Re: git auto-repack is broken...
From: Jeff King @ 2011-12-08  0:49 UTC (permalink / raw)
  To: Brandon Casey
  Cc: Junio C Hamano, Linus Torvalds, Ævar Arnfjörð,
	Git Mailing List
In-Reply-To: <CA+sFfMdeVoz8XU5j4hNn1qCHHzaiDi0Bw=QbbuU3cwT9mMPZOA@mail.gmail.com>

On Sat, Dec 03, 2011 at 01:42:22PM -0600, Brandon Casey wrote:

> >  4. "git gc" runs "git repack -Ad", which will eject X from the pack
> >     into a loose form (because it is not becoming part of the new pack
> >     we are writing).
> 
> Actually, it is right here when the newly loosened unreferenced
> objects will be deleted.  Objects ejected from a pack _are_ given the
> timestamp of the pack they were ejected from.  So, if the pack is
> older than two weeks (90 days in your example), then so will be the
> loosened objects, and git prune will delete them when called by git
> gc.

Thanks, I didn't notice that when looking at the code.

> Decreasing gc.pruneExpire as you suggested should make it much less
> likely to run into this problem.

I'd be more comfortable with that solution if we had data on what the
timestamps look like when it actually happens (e.g., an "ls -lR" listing
of a repository that in practice is wanting to auto-gc too often).

> I wonder if it is worth trying to limit how often gc --auto is run to
> not be more often than gc.pruneExpire or something.  If we modified
> the timestamp that is assigned to fetched packs, maybe we could use
> the pack timestamps as an indicator of how recently git gc has run.

I'm worried you run into other corner cases, there. Like a repository
which is generating new, referenced objects at a fast rate (e.g.,
because you're importing something) should trigger auto-gc much sooner
than that, and this rule would prevent it.

-Peff

^ permalink raw reply

* Re: Debugging git-commit slowness on a large repo
From: Nguyen Thai Ngoc Duy @ 2011-12-08  1:39 UTC (permalink / raw)
  To: Joshua Redstone
  Cc: Carlos Martín Nieto, Tomas Carnecky, Junio C Hamano,
	git@vger.kernel.org
In-Reply-To: <CB051EFC.2C795%joshua.redstone@fb.com>

On Thu, Dec 8, 2011 at 5:48 AM, Joshua Redstone <joshua.redstone@fb.com> wrote:
> Hi Duy,
> Thanks for the documentation link.
>
> git ls-files shows 100k files, which matches # of files in the working
> tree ('find . -type f -print | wc -l').

Any chance you can split it into smaller repositories, or remove files
from working directory (e.g. if you store logs, you don't have to keep
logs from all time in working directory, they can be retrieved from
history).

> I added a 'git read-tree HEAD' before the git-add, and a 'git write-tree'
> after the add.  With that, the commit time slowed down to 8 seconds per
> commit, plus 4 more seconds for the read-tree/add/write-tree ops.  The
> read-tree/add/write-tree each took about a second.

read-tree destroys stat info in index, refreshing 100k entries in
index in this case may take some time. Try this to see if commit time
reduces and how much time update-index takes

read-tree HEAD
update-index --refresh
add ....
write-tree
commit -q

> As an experiment, I also tried removing the 'git read-tree' and just
> having the git-write-tree.  That sped up commits to 0.6 seconds, but the
> overall time for add/write-tree/commit was still 3 to 6 seconds.

overall time is not really important because we duplicate work here
(write-tree is done as part of commit again). What I'm trying to do is
to determine how much time each operation in commit may take.
-- 
Duy

^ permalink raw reply

* [PATCH] git-send-email: Add auto-cc to all body signatures
From: Joe Perches @ 2011-12-08  2:58 UTC (permalink / raw)
  To: git; +Cc: jeffrey.t.kirsher
In-Reply-To: <1311903782.20837.42.camel@jtkirshe-mobl>

Many types of signatures are used by various projects.

The most common type is formatted:
	"[some_signature_type]-by: First Last <email@domain.tld>"
e.g:
	"Reported-by: First Last <email@domain.tld>" (no quotes are used)

Make git-send-email use these signatures as "CC:" entries.

Add command line option --suppress-cc=signatures to avoid
adding these entries to the cc.

Signed-off-by: Joe Perches <joe@perches.com>
Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 Documentation/git-send-email.txt |    3 ++-
 git-send-email.perl              |   11 ++++++-----
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 327233c..17ea825 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -246,8 +246,9 @@ Automating
   patch body (commit message) except for self (use 'self' for that).
 - 'sob' will avoid including anyone mentioned in Signed-off-by lines except
    for self (use 'self' for that).
+- 'signatures' will avoid including anyone mentioned in any "<foo>-by:" lines.
 - 'cccmd' will avoid running the --cc-cmd.
-- 'body' is equivalent to 'sob' + 'bodycc'
+- 'body' is equivalent to 'sob' + 'bodycc + signatures'
 - 'all' will suppress all auto cc values.
 --
 +
diff --git a/git-send-email.perl b/git-send-email.perl
index d491db9..fc5bf41 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -75,7 +75,7 @@ git send-email [options] <file | directory | rev-list options >
     --identity              <str>  * Use the sendemail.<id> options.
     --to-cmd                <str>  * Email To: via `<str> \$patch_path`
     --cc-cmd                <str>  * Email Cc: via `<str> \$patch_path`
-    --suppress-cc           <str>  * author, self, sob, cc, cccmd, body, bodycc, all.
+    --suppress-cc           <str>  * author, self, sob, cc, cccmd, body, bodycc, signatures, all.
     --[no-]signed-off-by-cc        * Send to Signed-off-by: addresses. Default on.
     --[no-]suppress-from           * Send to self. Default off.
     --[no-]chain-reply-to          * Chain In-Reply-To: fields. Default off.
@@ -393,13 +393,13 @@ my(%suppress_cc);
 if (@suppress_cc) {
 	foreach my $entry (@suppress_cc) {
 		die "Unknown --suppress-cc field: '$entry'\n"
-			unless $entry =~ /^(?:all|cccmd|cc|author|self|sob|body|bodycc)$/;
+			unless $entry =~ /^(?:all|cccmd|cc|author|self|sob|body|bodycc|signatures)$/;
 		$suppress_cc{$entry} = 1;
 	}
 }
 
 if ($suppress_cc{'all'}) {
-	foreach my $entry (qw (cccmd cc author self sob body bodycc)) {
+	foreach my $entry (qw (cccmd cc author self sob body bodycc signatures)) {
 		$suppress_cc{$entry} = 1;
 	}
 	delete $suppress_cc{'all'};
@@ -410,7 +410,7 @@ $suppress_cc{'self'} = $suppress_from if defined $suppress_from;
 $suppress_cc{'sob'} = !$signed_off_by_cc if defined $signed_off_by_cc;
 
 if ($suppress_cc{'body'}) {
-	foreach my $entry (qw (sob bodycc)) {
+	foreach my $entry (qw (sob bodycc signatures)) {
 		$suppress_cc{$entry} = 1;
 	}
 	delete $suppress_cc{'body'};
@@ -1276,7 +1276,7 @@ foreach my $t (@files) {
 	# Now parse the message body
 	while(<$fh>) {
 		$message .=  $_;
-		if (/^(Signed-off-by|Cc): (.*)$/i) {
+		if (/^(Signed-off-by|Cc|[a-z_-]+by): (.*)$/i) {
 			chomp;
 			my ($what, $c) = ($1, $2);
 			chomp $c;
@@ -1285,6 +1285,7 @@ foreach my $t (@files) {
 			} else {
 				next if $suppress_cc{'sob'} and $what =~ /Signed-off-by/i;
 				next if $suppress_cc{'bodycc'} and $what =~ /Cc/i;
+				next if $suppress_cc{'signatures'} and $what =~ /by$/i;
 			}
 			push @cc, $c;
 			printf("(body) Adding cc: %s from line '%s'\n",
-- 
1.7.8.dirty

^ permalink raw reply related

* Re: [PATCH] index-pack: eliminate unlimited recursion in get_delta_base()
From: Shawn Pearce @ 2011-12-08  3:02 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git, Junio C Hamano
In-Reply-To: <1323280223-7990-1-git-send-email-pclouds@gmail.com>

2011/12/7 Nguyễn Thái Ngọc Duy <pclouds@gmail.com>:
> Revert the order of delta applying so that by the time a delta is
> applied, its base is either non-delta or already inflated.
> get_delta_base() is still recursive, but because base's data is always
> ready, the inner get_delta_base() call never has any chance to call
> itself again.
...
> @@ -508,10 +508,25 @@ static void *get_base_data(struct base_data *c)
>  {
>        if (!c->data) {
>                struct object_entry *obj = c->obj;
> +               struct base_data **delta = NULL;
> +               int delta_nr = 0, delta_alloc = 0;
>
> -               if (is_delta_type(obj->type)) {
> -                       void *base = get_base_data(c->base);
> -                       void *raw = get_data_from_pack(obj);

I think you missed the critical recursion. The real work is the
recursion within find_unresolved_deltas(). This little helper
get_base_data() shouldn't be tripping over these cases unless we have
run out of delta_base_cache_limit and released objects near the base
end of the delta chain, in which case this will restore them.

Maybe this is useful on its own, but in my opinion its not an
interesting patch to consider without first fixing
find_unresolved_deltas's recursion.

^ permalink raw reply

* Re: git auto-repack is broken...
From: Nicolas Pitre @ 2011-12-08  3:35 UTC (permalink / raw)
  To: Jeff King
  Cc: Brandon Casey, Junio C Hamano, Linus Torvalds,
	Ævar Arnfjörð, Git Mailing List
In-Reply-To: <20111208004515.GA23015@sigill.intra.peff.net>

On Wed, 7 Dec 2011, Jeff King wrote:

> On Wed, Dec 07, 2011 at 07:18:13PM -0500, Nicolas Pitre wrote:
> 
> > I certainly never did a multi-ref fetch myself.
> 
> Not consciously, perhaps, but you do it all the time without realizing
> it:
> 
>   $ git clone git://git.kernel.org/pub/scm/git/git.git
>   $ cd git
>   $ git fetch -v origin
>    = [up to date]      maint      -> origin/maint
>    = [up to date]      master     -> origin/master
>    = [up to date]      next       -> origin/next
>    = [up to date]      pu         -> origin/pu
>    = [up to date]      todo       -> origin/todo
>   $ cat .git/FETCH_HEAD
>   b1af9630d758e1728fc0008b3f18d90d8f87f4c5        not-for-merge   branch 'maint' of git://git.kernel.org/pub/scm/git/git
>   4cb5d10b14dcbe0155bed9c45ccb94e83bd4c599                branch 'master' of git://git.kernel.org/pub/scm/git/git
>   03e5527c5df33d4550ccc1446d861c0aa5689d58        not-for-merge   branch 'next' of git://git.kernel.org/pub/scm/git/git
>   cc4e3f01fc6a5e09ae5bbdc464965981fae4cf39        not-for-merge   branch 'pu' of git://git.kernel.org/pub/scm/git/git
>   7a02dba15bd28826344f9c14a5e2b5c57eeb7e50        not-for-merge   branch 'todo' of git://git.kernel.org/pub/scm/git/git

OK, nevermind.  I admitedly never have been close enough to the related 
code.

And I don't think this particular case is interesting anyway as the 
reflogs for the various branches alre already involved.  I was thinking 
more about the "git fetch git://some.random.repo foobar" case where the 
summary also explicitly shows:

From: git://some.random.repo
  ......  foobar   -> FETCH_HEAD

In that case the only reference to the fetched branch is stored in 
FETCH_HEAD and that is what might be worthwile for a reflog.


Nicolas

^ permalink raw reply

* Re: git auto-repack is broken...
From: Jeff King @ 2011-12-08  3:40 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: Brandon Casey, Junio C Hamano, Linus Torvalds,
	Ævar Arnfjörð, Git Mailing List
In-Reply-To: <alpine.LFD.2.02.1112072227510.2907@xanadu.home>

On Wed, Dec 07, 2011 at 10:35:00PM -0500, Nicolas Pitre wrote:

> And I don't think this particular case is interesting anyway as the 
> reflogs for the various branches alre already involved.  I was thinking 
> more about the "git fetch git://some.random.repo foobar" case where the 
> summary also explicitly shows:
> 
> From: git://some.random.repo
>   ......  foobar   -> FETCH_HEAD
> 
> In that case the only reference to the fetched branch is stored in 
> FETCH_HEAD and that is what might be worthwile for a reflog.

I agree that is the interesting case. Perhaps we could just not bother
writing the other case into the reflog at all. So the reflog would be
sensible and contain only the set of things they had fetched or pulled
explicitly by URL. If they really want to do a multi-ref one-off fetch
from some URL, then we write multiple reflog entries. But at least the
user is very aware of what they've done, so they're not surprised by the
reflog advancing by more than 1 entry.

-Peff

^ permalink raw reply

* Re: [PATCH 07/15] t1510 (worktree): fix && chaining
From: Ramkumar Ramachandra @ 2011-12-08  4:39 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Junio C Hamano, Git List
In-Reply-To: <20111207215100.GB2911@elie.hsd1.il.comcast.net>

Hi Jonathan,

Jonathan Nieder wrote:
> [...]
> On some shells, like /usr/xpg4/bin/sh on Solaris, unset returns nonzero
> status when the variable passed was already unset.  Will this work on
> such platforms, or does it need to be changed to use sane_unset?

Interesting.  Thanks for catching this.

-- Ram

^ permalink raw reply

* Re: [PATCH 04/15] t1007 (hash-object): fix && chaining
From: Ramkumar Ramachandra @ 2011-12-08  4:42 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Junio C Hamano, Git List
In-Reply-To: <20111207214716.GA2911@elie.hsd1.il.comcast.net>

Hi Jonathan,

Jonathan Nieder wrote:
> Ramkumar Ramachandra wrote:
> [...]
>
> IMHO if the patches are only being sent to me, Junio, and the
> mailing list (and not cc-ed to different people), then there's no
> reason to split them up when they have the same topic.

Right.  I'll re-roll with similar patches squashed together -- it's
easier to review and fixup in this form.

>> --- a/t/t1007-hash-object.sh
>> +++ b/t/t1007-hash-object.sh
>> @@ -154,13 +154,13 @@ test_expect_success 'check that --no-filters option works with --stdin-paths' '
>>  pop_repo
>>
>>  for args in "-w --stdin" "--stdin -w"; do
>> -     push_repo
>> +     push_repo &&
>>
>>       test_expect_success "hash from stdin and write to database ($args)" '
>>               test $example_sha1 = $(git hash-object $args < example)
>> -     '
>> +     ' &&
>
> I don't see how this would have any effect.  Is it intended?

Oops, looks like I was in a bit of a rush.  Good catch.

-- Ram

^ permalink raw reply

* Re: [PATCH 08/15] t3200 (branch): fix && chaining
From: Ramkumar Ramachandra @ 2011-12-08  4:47 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Junio C Hamano, Git List
In-Reply-To: <20111207215509.GC2911@elie.hsd1.il.comcast.net>

Hi Jonathan,

Jonathan Nieder wrote:
> Ramkumar Ramachandra wrote:
>
>> --- a/t/t3200-branch.sh
>> +++ b/t/t3200-branch.sh
>> @@ -22,7 +22,7 @@ test_expect_success \
>>
>>  test_expect_success \
>>      'git branch --help should not have created a bogus branch' '
>> -     git branch --help </dev/null >/dev/null 2>/dev/null;
>> +     git branch --help </dev/null >/dev/null 2>/dev/null &&
>>       test_path_is_missing .git/refs/heads/--help
>
> Won't this break when running tests for the first time, before the git
> manpages are installed?

Used test_might_fail to guard it this time.  Thanks.

-- Ram

^ permalink raw reply

* Re: Query on git commit amend
From: Viresh Kumar @ 2011-12-08  5:26 UTC (permalink / raw)
  To: Junio C Hamano, Vipin KUMAR
  Cc: Vijay Lakshminarayanan, git@vger.kernel.org, Shiraz HASHIM
In-Reply-To: <87fwgxwvn9.fsf@gmail.com>

On 12/6/2011 9:16 PM, Vijay Lakshminarayanan wrote:
> I've found 
> 
> $ GIT_EDITOR=cat git commit --amend
> 
> useful.
> 
> The benefit of this technique is that it even works for git-rebase -i.
> 
> In my typical git usage, I do a lot of git-commit --fixup's.  After
> reaching a level of stability, I change the history with:
> 
> GIT_EDITOR=cat git rebase -i --autosquash
> 
> and my history is adjusted without requiring manual intervention.

Hi Junio,

After going through autosquash option for rebase, i was wondering
if there is a way to create a new commit easily for autosquash.

For autosquash to work, we need to keep the same commit log/title,
prefixed with squash! or Fixup! etc. What about adding another option
in commit amend which adds squash! or Fixup! automatically. So, manual
intervention at all. :)

I don't know if it already exist.

-- 
viresh

^ permalink raw reply

* Re: Query on git commit amend
From: Viresh Kumar @ 2011-12-08  5:30 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Vipin KUMAR, Vijay Lakshminarayanan, git@vger.kernel.org,
	Shiraz HASHIM
In-Reply-To: <4EE04A6D.5020503@st.com>

On 12/8/2011 10:56 AM, Viresh Kumar wrote:
> For autosquash to work, we need to keep the same commit log/title,
> prefixed with squash! or Fixup! etc. What about adding another option
> in commit amend which adds squash! or Fixup! automatically. So, manual
> intervention at all. :)

Done. It is already there in git commit.

-- 
viresh

^ permalink raw reply

* Re: &&-chaining tester
From: Matthieu Moy @ 2011-12-08  8:06 UTC (permalink / raw)
  To: Ramkumar Ramachandra; +Cc: Jonathan Nieder, Junio C Hamano, Git List
In-Reply-To: <1323286611-4806-1-git-send-email-artagnon@gmail.com>

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> Hm, involves a huge amount of janitorial work.  Anyway, thanks for
> pointing me in the right direction- here's a small start.

Patches 1-14 look good to me (with Jonathan's remarks). I didn't review
patch 15.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply

* [PATCHv2 0/7] getpass refactoring
From: Jeff King @ 2011-12-08  8:21 UTC (permalink / raw)
  To: git; +Cc: Erik Faye-Lund, Junio C Hamano

This is a re-roll of the earlier "echo usernames as they are typed"
series here:

  http://article.gmane.org/gmane.comp.version-control.git/185970

It turns out that getpass() isn't even really good at getting passwords.
See patch 5/7 for details. This series replaces it entirely if you set
HAVE_DEV_TTY (otherwise, we continue to fallback to getpass()).

  [1/7]: imap-send: avoid buffer overflow
  [2/7]: imap-send: don't check return value of git_getpass
  [3/7]: move git_getpass to its own source file
  [4/7]: refactor git_getpass into generic prompt function
  [5/7]: add generic terminal prompt function
  [6/7]: prompt: use git_terminal_prompt
  [7/7]: credential: use git_prompt instead of git_getpass

It should be applied on top of the jk/credentials topic.

-Peff

^ permalink raw reply

* [PATCH 1/7] imap-send: avoid buffer overflow
From: Jeff King @ 2011-12-08  8:23 UTC (permalink / raw)
  To: git; +Cc: Erik Faye-Lund, Junio C Hamano
In-Reply-To: <20111208082118.GA1507@sigill.intra.peff.net>

We format the password prompt in an 80-character static
buffer. It contains the remote host and username, so it's
unlikely to overflow (or be exploitable by a remote
attacker), but there's no reason not to be careful and use
a strbuf.

Signed-off-by: Jeff King <peff@peff.net>
---
Just something I noticed while doing the cleanup in the next patch.

 imap-send.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/imap-send.c b/imap-send.c
index e1ad1a4..4c1e897 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -1209,9 +1209,10 @@ static int auth_cram_md5(struct imap_store *ctx, struct imap_cmd *cmd, const cha
 			goto bail;
 		}
 		if (!srvc->pass) {
-			char prompt[80];
-			sprintf(prompt, "Password (%s@%s): ", srvc->user, srvc->host);
-			arg = git_getpass(prompt);
+			struct strbuf prompt = STRBUF_INIT;
+			strbuf_addf(&prompt, "Password (%s@%s): ", srvc->user, srvc->host);
+			arg = git_getpass(prompt.buf);
+			strbuf_release(&prompt);
 			if (!arg) {
 				perror("getpass");
 				exit(1);
-- 
1.7.8.rc2.8.gf0f4f

^ permalink raw reply related

* [PATCH 2/7] imap-send: don't check return value of git_getpass
From: Jeff King @ 2011-12-08  8:24 UTC (permalink / raw)
  To: git; +Cc: Erik Faye-Lund, Junio C Hamano
In-Reply-To: <20111208082118.GA1507@sigill.intra.peff.net>

git_getpass will always die() if we weren't able to get
input, so there's no point looking for NULL.

Signed-off-by: Jeff King <peff@peff.net>
---
Not a bug, but just useless code I noticed.

 imap-send.c |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/imap-send.c b/imap-send.c
index 4c1e897..227253e 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -1213,10 +1213,6 @@ static int auth_cram_md5(struct imap_store *ctx, struct imap_cmd *cmd, const cha
 			strbuf_addf(&prompt, "Password (%s@%s): ", srvc->user, srvc->host);
 			arg = git_getpass(prompt.buf);
 			strbuf_release(&prompt);
-			if (!arg) {
-				perror("getpass");
-				exit(1);
-			}
 			if (!*arg) {
 				fprintf(stderr, "Skipping account %s@%s, no password\n", srvc->user, srvc->host);
 				goto bail;
-- 
1.7.8.rc2.8.gf0f4f

^ permalink raw reply related

* [PATCH 3/7] move git_getpass to its own source file
From: Jeff King @ 2011-12-08  8:24 UTC (permalink / raw)
  To: git; +Cc: Erik Faye-Lund, Junio C Hamano
In-Reply-To: <20111208082118.GA1507@sigill.intra.peff.net>

This is currently in connect.c, but really has nothing to
do with the git protocol itself. Let's make a new source
file all about prompting the user, which will make it
cleaner to refactor.

Signed-off-by: Jeff King <peff@peff.net>
---
Same as patch 1 from the previous series.

 Makefile     |    2 ++
 cache.h      |    1 -
 connect.c    |   44 --------------------------------------------
 credential.c |    1 +
 imap-send.c  |    1 +
 prompt.c     |   48 ++++++++++++++++++++++++++++++++++++++++++++++++
 prompt.h     |    6 ++++++
 7 files changed, 58 insertions(+), 45 deletions(-)
 create mode 100644 prompt.c
 create mode 100644 prompt.h

diff --git a/Makefile b/Makefile
index a26f58a..b024e2f 100644
--- a/Makefile
+++ b/Makefile
@@ -563,6 +563,7 @@ LIB_H += parse-options.h
 LIB_H += patch-ids.h
 LIB_H += pkt-line.h
 LIB_H += progress.h
+LIB_H += prompt.h
 LIB_H += quote.h
 LIB_H += reflog-walk.h
 LIB_H += refs.h
@@ -669,6 +670,7 @@ LIB_OBJS += pkt-line.o
 LIB_OBJS += preload-index.o
 LIB_OBJS += pretty.o
 LIB_OBJS += progress.o
+LIB_OBJS += prompt.o
 LIB_OBJS += quote.o
 LIB_OBJS += reachable.o
 LIB_OBJS += read-cache.o
diff --git a/cache.h b/cache.h
index 2e6ad36..f320c98 100644
--- a/cache.h
+++ b/cache.h
@@ -1024,7 +1024,6 @@ struct ref {
 extern struct ref *find_ref_by_name(const struct ref *list, const char *name);
 
 #define CONNECT_VERBOSE       (1u << 0)
-extern char *git_getpass(const char *prompt);
 extern struct child_process *git_connect(int fd[2], const char *url, const char *prog, int flags);
 extern int finish_connect(struct child_process *conn);
 extern int git_connection_is_socket(struct child_process *conn);
diff --git a/connect.c b/connect.c
index 51990fa..519e527 100644
--- a/connect.c
+++ b/connect.c
@@ -619,47 +619,3 @@ int finish_connect(struct child_process *conn)
 	free(conn);
 	return code;
 }
-
-char *git_getpass(const char *prompt)
-{
-	const char *askpass;
-	struct child_process pass;
-	const char *args[3];
-	static struct strbuf buffer = STRBUF_INIT;
-
-	askpass = getenv("GIT_ASKPASS");
-	if (!askpass)
-		askpass = askpass_program;
-	if (!askpass)
-		askpass = getenv("SSH_ASKPASS");
-	if (!askpass || !(*askpass)) {
-		char *result = getpass(prompt);
-		if (!result)
-			die_errno("Could not read password");
-		return result;
-	}
-
-	args[0] = askpass;
-	args[1]	= prompt;
-	args[2] = NULL;
-
-	memset(&pass, 0, sizeof(pass));
-	pass.argv = args;
-	pass.out = -1;
-
-	if (start_command(&pass))
-		exit(1);
-
-	strbuf_reset(&buffer);
-	if (strbuf_read(&buffer, pass.out, 20) < 0)
-		die("failed to read password from %s\n", askpass);
-
-	close(pass.out);
-
-	if (finish_command(&pass))
-		exit(1);
-
-	strbuf_setlen(&buffer, strcspn(buffer.buf, "\r\n"));
-
-	return buffer.buf;
-}
diff --git a/credential.c b/credential.c
index a17eafe..fbb7231 100644
--- a/credential.c
+++ b/credential.c
@@ -3,6 +3,7 @@
 #include "string-list.h"
 #include "run-command.h"
 #include "url.h"
+#include "prompt.h"
 
 void credential_init(struct credential *c)
 {
diff --git a/imap-send.c b/imap-send.c
index 227253e..43588e8 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -25,6 +25,7 @@
 #include "cache.h"
 #include "exec_cmd.h"
 #include "run-command.h"
+#include "prompt.h"
 #ifdef NO_OPENSSL
 typedef void *SSL;
 #else
diff --git a/prompt.c b/prompt.c
new file mode 100644
index 0000000..42a1c9f
--- /dev/null
+++ b/prompt.c
@@ -0,0 +1,48 @@
+#include "cache.h"
+#include "run-command.h"
+#include "strbuf.h"
+#include "prompt.h"
+
+char *git_getpass(const char *prompt)
+{
+	const char *askpass;
+	struct child_process pass;
+	const char *args[3];
+	static struct strbuf buffer = STRBUF_INIT;
+
+	askpass = getenv("GIT_ASKPASS");
+	if (!askpass)
+		askpass = askpass_program;
+	if (!askpass)
+		askpass = getenv("SSH_ASKPASS");
+	if (!askpass || !(*askpass)) {
+		char *result = getpass(prompt);
+		if (!result)
+			die_errno("Could not read password");
+		return result;
+	}
+
+	args[0] = askpass;
+	args[1]	= prompt;
+	args[2] = NULL;
+
+	memset(&pass, 0, sizeof(pass));
+	pass.argv = args;
+	pass.out = -1;
+
+	if (start_command(&pass))
+		exit(1);
+
+	strbuf_reset(&buffer);
+	if (strbuf_read(&buffer, pass.out, 20) < 0)
+		die("failed to read password from %s\n", askpass);
+
+	close(pass.out);
+
+	if (finish_command(&pass))
+		exit(1);
+
+	strbuf_setlen(&buffer, strcspn(buffer.buf, "\r\n"));
+
+	return buffer.buf;
+}
diff --git a/prompt.h b/prompt.h
new file mode 100644
index 0000000..0fd7bd9
--- /dev/null
+++ b/prompt.h
@@ -0,0 +1,6 @@
+#ifndef PROMPT_H
+#define PROMPT_H
+
+char *git_getpass(const char *prompt);
+
+#endif /* PROMPT_H */
-- 
1.7.8.rc2.8.gf0f4f

^ permalink raw reply related

* [PATCH 4/7] refactor git_getpass into generic prompt function
From: Jeff King @ 2011-12-08  8:31 UTC (permalink / raw)
  To: git; +Cc: Erik Faye-Lund, Junio C Hamano
In-Reply-To: <20111208082118.GA1507@sigill.intra.peff.net>

This will allow callers to specify more options (e.g.,
leaving echo on). The original git_getpass becomes a slim
wrapper around the new function.

Signed-off-by: Jeff King <peff@peff.net>
---
Similar to patch 2 from the previous series. Two big differences:

 1. The first series accidentally dropped the "die if we don't get a
    password" behavior during the refactor, but we want to keep it.

 2. The first series had a special "name" parameter just for generating
    error messages. This drops it in the name of simplicity, so error
    messages have gone from (assuming you don't have a tty):

      Could not read password: No such device or address

    to:

      Could not read 'Username for 'https://example.com': ': No such
      device or address

    which is verbose, yes, but contains a little more useful
    information. The formatting is rather unfortunate, but I don't think
    it's worth worrying too much about.

 prompt.c |   46 ++++++++++++++++++++++++++++++----------------
 prompt.h |    3 +++
 2 files changed, 33 insertions(+), 16 deletions(-)

diff --git a/prompt.c b/prompt.c
index 42a1c9f..2002644 100644
--- a/prompt.c
+++ b/prompt.c
@@ -3,26 +3,13 @@
 #include "strbuf.h"
 #include "prompt.h"
 
-char *git_getpass(const char *prompt)
+static char *do_askpass(const char *cmd, const char *prompt)
 {
-	const char *askpass;
 	struct child_process pass;
 	const char *args[3];
 	static struct strbuf buffer = STRBUF_INIT;
 
-	askpass = getenv("GIT_ASKPASS");
-	if (!askpass)
-		askpass = askpass_program;
-	if (!askpass)
-		askpass = getenv("SSH_ASKPASS");
-	if (!askpass || !(*askpass)) {
-		char *result = getpass(prompt);
-		if (!result)
-			die_errno("Could not read password");
-		return result;
-	}
-
-	args[0] = askpass;
+	args[0] = cmd;
 	args[1]	= prompt;
 	args[2] = NULL;
 
@@ -35,7 +22,7 @@
 
 	strbuf_reset(&buffer);
 	if (strbuf_read(&buffer, pass.out, 20) < 0)
-		die("failed to read password from %s\n", askpass);
+		die("failed to get '%s' from %s\n", prompt, cmd);
 
 	close(pass.out);
 
@@ -46,3 +33,30 @@
 
 	return buffer.buf;
 }
+
+char *git_prompt(const char *prompt, int flags)
+{
+	char *r;
+
+	if (flags & PROMPT_ASKPASS) {
+		const char *askpass;
+
+		askpass = getenv("GIT_ASKPASS");
+		if (!askpass)
+			askpass = askpass_program;
+		if (!askpass)
+			askpass = getenv("SSH_ASKPASS");
+		if (askpass && *askpass)
+			return do_askpass(askpass, prompt);
+	}
+
+	r = getpass(prompt);
+	if (!r)
+		die_errno("could not read '%s'", prompt);
+	return r;
+}
+
+char *git_getpass(const char *prompt)
+{
+	return git_prompt(prompt, PROMPT_ASKPASS);
+}
diff --git a/prompt.h b/prompt.h
index 0fd7bd9..9ab85a7 100644
--- a/prompt.h
+++ b/prompt.h
@@ -1,6 +1,9 @@
 #ifndef PROMPT_H
 #define PROMPT_H
 
+#define PROMPT_ASKPASS (1<<0)
+
+char *git_prompt(const char *prompt, int flags);
 char *git_getpass(const char *prompt);
 
 #endif /* PROMPT_H */
-- 
1.7.8.rc2.8.gf0f4f

^ permalink raw reply related

* [PATCH 5/7] add generic terminal prompt function
From: Jeff King @ 2011-12-08  8:33 UTC (permalink / raw)
  To: git; +Cc: Erik Faye-Lund, Junio C Hamano
In-Reply-To: <20111208082118.GA1507@sigill.intra.peff.net>

When we need to prompt the user for input interactively, we
want to access their terminal directly. We can't rely on
stdio because it may be connected to pipes or files, rather
than the terminal. Instead, we use "getpass()", because it
abstracts the idea of prompting and reading from the
terminal.  However, it has some problems:

  1. It never echoes the typed characters, which makes it OK
     for passwords but annoying for other input (like usernames).

  2. Some implementations of getpass() have an extremely
     small input buffer (e.g., Solaris 8 is reported to
     support only 8 characters).

  3. Some implementations of getpass() will fall back to
     reading from stdin (e.g., glibc). We explicitly don't
     want this, because our stdin may be connected to a pipe
     speaking a particular protocol, and reading will
     disrupt the protocol flow (e.g., the remote-curl
     helper).

  4. Some implementations of getpass() turn off signals, so
     that hitting "^C" on the terminal does not break out of
     the password prompt. This can be a mild annoyance.

Instead, let's provide an abstract "git_terminal_prompt"
function that addresses these concerns. This patch includes
an implementation based on /dev/tty, enabled by setting
HAVE_DEV_TTY. The fallback is to use getpass() as before.

For now, only Linux enables this by default. People on other
/dev/tty-enabled systems can submit patches to turn it on
once they have tested it.

Signed-off-by: Jeff King <peff@peff.net>
---
This is the interesting one. I suspect most Unixes will want to use
this, but I was conservative about enabling it. msysgit can just drop
its own function into compat/terminal.c, and the existing custom
getpass() implementation can just go away.

 Makefile          |   10 ++++++
 compat/terminal.c |   81 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 compat/terminal.h |    6 ++++
 3 files changed, 97 insertions(+), 0 deletions(-)
 create mode 100644 compat/terminal.c
 create mode 100644 compat/terminal.h

diff --git a/Makefile b/Makefile
index b024e2f..2a9bb3d 100644
--- a/Makefile
+++ b/Makefile
@@ -227,6 +227,9 @@ all::
 #
 # Define NO_REGEX if you have no or inferior regex support in your C library.
 #
+# Define HAVE_DEV_TTY if your system can open /dev/tty to interact with the
+# user.
+#
 # Define GETTEXT_POISON if you are debugging the choice of strings marked
 # for translation.  In a GETTEXT_POISON build, you can turn all strings marked
 # for translation into gibberish by setting the GIT_GETTEXT_POISON variable
@@ -523,6 +526,7 @@ LIB_H += compat/bswap.h
 LIB_H += compat/cygwin.h
 LIB_H += compat/mingw.h
 LIB_H += compat/obstack.h
+LIB_H += compat/terminal.h
 LIB_H += compat/win32/pthread.h
 LIB_H += compat/win32/syslog.h
 LIB_H += compat/win32/poll.h
@@ -610,6 +614,7 @@ LIB_OBJS += color.o
 LIB_OBJS += combine-diff.o
 LIB_OBJS += commit.o
 LIB_OBJS += compat/obstack.o
+LIB_OBJS += compat/terminal.o
 LIB_OBJS += config.o
 LIB_OBJS += connect.o
 LIB_OBJS += connected.o
@@ -833,6 +838,7 @@ ifeq ($(uname_S),Linux)
 	NO_STRLCPY = YesPlease
 	NO_MKSTEMPS = YesPlease
 	HAVE_PATHS_H = YesPlease
+	HAVE_DEV_TTY = YesPlease
 endif
 ifeq ($(uname_S),GNU/kFreeBSD)
 	NO_STRLCPY = YesPlease
@@ -1639,6 +1645,10 @@ ifdef HAVE_PATHS_H
 	BASIC_CFLAGS += -DHAVE_PATHS_H
 endif
 
+ifdef HAVE_DEV_TTY
+	BASIC_CFLAGS += -DHAVE_DEV_TTY
+endif
+
 ifdef DIR_HAS_BSD_GROUP_SEMANTICS
 	COMPAT_CFLAGS += -DDIR_HAS_BSD_GROUP_SEMANTICS
 endif
diff --git a/compat/terminal.c b/compat/terminal.c
new file mode 100644
index 0000000..49f16ca
--- /dev/null
+++ b/compat/terminal.c
@@ -0,0 +1,81 @@
+#include "git-compat-util.h"
+#include "compat/terminal.h"
+#include "sigchain.h"
+#include "strbuf.h"
+
+#ifndef NO_DEV_TTY
+
+static int term_fd = -1;
+static struct termios old_term;
+
+static void restore_term(void)
+{
+	if (term_fd < 0)
+		return;
+
+	tcsetattr(term_fd, TCSAFLUSH, &old_term);
+	term_fd = -1;
+}
+
+static void restore_term_on_signal(int sig)
+{
+	restore_term();
+	sigchain_pop(sig);
+	raise(sig);
+}
+
+char *git_terminal_prompt(const char *prompt, int echo)
+{
+	static struct strbuf buf = STRBUF_INIT;
+	int r;
+	FILE *fh;
+
+	fh = fopen("/dev/tty", "w+");
+	if (!fh)
+		return NULL;
+
+	if (!echo) {
+		struct termios t;
+
+		if (tcgetattr(fileno(fh), &t) < 0) {
+			fclose(fh);
+			return NULL;
+		}
+
+		old_term = t;
+		term_fd = fileno(fh);
+		sigchain_push_common(restore_term_on_signal);
+
+		t.c_lflag &= ~ECHO;
+		if (tcsetattr(fileno(fh), TCSAFLUSH, &t) < 0) {
+			term_fd = -1;
+			fclose(fh);
+			return NULL;
+		}
+	}
+
+	fputs(prompt, fh);
+	fflush(fh);
+
+	r = strbuf_getline(&buf, fh, '\n');
+	if (!echo) {
+		putc('\n', fh);
+		fflush(fh);
+	}
+
+	restore_term();
+	fclose(fh);
+
+	if (r == EOF)
+		return NULL;
+	return buf.buf;
+}
+
+#else
+
+char *git_terminal_prompt(const char *prompt, int echo)
+{
+	return getpass(prompt);
+}
+
+#endif
diff --git a/compat/terminal.h b/compat/terminal.h
new file mode 100644
index 0000000..97db7cd
--- /dev/null
+++ b/compat/terminal.h
@@ -0,0 +1,6 @@
+#ifndef COMPAT_TERMINAL_H
+#define COMPAT_TERMINAL_H
+
+char *git_terminal_prompt(const char *prompt, int echo);
+
+#endif /* COMPAT_TERMINAL_H */
-- 
1.7.8.rc2.8.gf0f4f

^ permalink raw reply related

* [PATCH 6/7] prompt: use git_terminal_prompt
From: Jeff King @ 2011-12-08  8:33 UTC (permalink / raw)
  To: git; +Cc: Erik Faye-Lund, Junio C Hamano
In-Reply-To: <20111208082118.GA1507@sigill.intra.peff.net>

Our custom implementation of git_terminal_prompt has many
advantages over regular getpass(), as described in the prior
commit.

This also lets us implement a PROMPT_ECHO flag for callers
who want it.

Signed-off-by: Jeff King <peff@peff.net>
---
 prompt.c |    3 ++-
 prompt.h |    1 +
 2 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/prompt.c b/prompt.c
index 2002644..72ab9de 100644
--- a/prompt.c
+++ b/prompt.c
@@ -2,6 +2,7 @@
 #include "run-command.h"
 #include "strbuf.h"
 #include "prompt.h"
+#include "compat/terminal.h"
 
 static char *do_askpass(const char *cmd, const char *prompt)
 {
@@ -50,7 +51,7 @@
 			return do_askpass(askpass, prompt);
 	}
 
-	r = getpass(prompt);
+	r = git_terminal_prompt(prompt, flags & PROMPT_ECHO);
 	if (!r)
 		die_errno("could not read '%s'", prompt);
 	return r;
diff --git a/prompt.h b/prompt.h
index 9ab85a7..04f321a 100644
--- a/prompt.h
+++ b/prompt.h
@@ -2,6 +2,7 @@
 #define PROMPT_H
 
 #define PROMPT_ASKPASS (1<<0)
+#define PROMPT_ECHO    (1<<1)
 
 char *git_prompt(const char *prompt, int flags);
 char *git_getpass(const char *prompt);
-- 
1.7.8.rc2.8.gf0f4f

^ permalink raw reply related

* [PATCH 7/7] credential: use git_prompt instead of git_getpass
From: Jeff King @ 2011-12-08  8:33 UTC (permalink / raw)
  To: git; +Cc: Erik Faye-Lund, Junio C Hamano
In-Reply-To: <20111208082118.GA1507@sigill.intra.peff.net>

We use git_getpass to retrieve the username and password
from the terminal. However, git_getpass will not echo the
username as the user types. We can fix this by using the
more generic git_prompt, which underlies git_getpass but
lets us specify an "echo" option.

Signed-off-by: Jeff King <peff@peff.net>
---
 credential.c |   15 +++++++--------
 1 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/credential.c b/credential.c
index fbb7231..62d1c56 100644
--- a/credential.c
+++ b/credential.c
@@ -109,7 +109,8 @@ static void credential_describe(struct credential *c, struct strbuf *out)
 		strbuf_addf(out, "/%s", c->path);
 }
 
-static char *credential_ask_one(const char *what, struct credential *c)
+static char *credential_ask_one(const char *what, struct credential *c,
+				int flags)
 {
 	struct strbuf desc = STRBUF_INIT;
 	struct strbuf prompt = STRBUF_INIT;
@@ -121,11 +122,7 @@ static void credential_describe(struct credential *c, struct strbuf *out)
 	else
 		strbuf_addf(&prompt, "%s: ", what);
 
-	/* FIXME: for usernames, we should do something less magical that
-	 * actually echoes the characters. However, we need to read from
-	 * /dev/tty and not stdio, which is not portable (but getpass will do
-	 * it for us). http.c uses the same workaround. */
-	r = git_getpass(prompt.buf);
+	r = git_prompt(prompt.buf, flags);
 
 	strbuf_release(&desc);
 	strbuf_release(&prompt);
@@ -135,9 +132,11 @@ static void credential_describe(struct credential *c, struct strbuf *out)
 static void credential_getpass(struct credential *c)
 {
 	if (!c->username)
-		c->username = credential_ask_one("Username", c);
+		c->username = credential_ask_one("Username", c,
+						 PROMPT_ASKPASS|PROMPT_ECHO);
 	if (!c->password)
-		c->password = credential_ask_one("Password", c);
+		c->password = credential_ask_one("Password", c,
+						 PROMPT_ASKPASS);
 }
 
 int credential_read(struct credential *c, FILE *fp)
-- 
1.7.8.rc2.8.gf0f4f

^ permalink raw reply related

* Re: Auto update submodules after merge and reset
From: andreas.t.auer_gtml_37453 @ 2011-12-08  9:13 UTC (permalink / raw)
  To: Jens Lehmann; +Cc: Andreas T.Auer, git
In-Reply-To: <4EDFE75C.5050201@web.de>



On 07.12.2011 23:23 Jens Lehmann wrote:
>  Am 07.12.2011 10:07, schrieb Andreas T.Auer:
> > Jens Lehmann wrote:
> >
> >> Am 30.11.2011 01:55, schrieb Max Krasnyansky: I'm working on a
> >> patch series to teach Git to optionally update the submodules
> >> work trees on checkout, reset merge and so on, but I'm not there
> >> yet.
> >>
> >>> I'm thinking about adding a config option that would enable
> >>> automatic submodule update but wanted to see if there is some
> >>> fundamental reason why it would not be accepted.
> > Because there is no good way to do so. It would be fine when you
> > just track the submodules "read-only", but if you are actually
> > working on submodules, it is a bad idea to always get a detached
> > HEAD.
>
>  YMMV. We get along *really* well with this because all developers
>  know that if they want to hack on a submodule, they have to create a
>  branch in there first (and if they forget to do that, git status and
>  friends will tell them).
Sorry, my fault. I was answering to the question why auto-update is not 
the default, but replied to the  wrong text block. (I should have heeded 
the note to self about the coffee in the morning ;-) )
Having the config option is fine, of course. But it is not easy to 
choose a good default auto-update method, because you need different 
workflows for different submodules/users .
>  What bugs us is that submodule HEADs don't follow what is checked
>  out (or merged, or reset ...) in the superproject. We had some
>  really nasty mismerges because of that, so we need the option to
>  enable it.
>
Full ack. Using the auto-update method "disabled" is a bad choice, too. ;-)

> > Because if you are working on a maint branch in the submodule and
> > then you checkout a pu branch in the superproject, because you
> > have forgotten that maint branch in the submodule then all the
> > proposed updates go to the maintenance branch -> bad.
>
>  Nope, checkout will fail and not do anything as it will detect
>  changes in the submodule to be updated by the checkout (just as it
>  would do with a regular file).
>
Without auto-update you can easily checkout the pu branch in the 
superproject. And when you execute
git submodule update --merge
the pu referenced commit of the submodule will be merged into the 
currently checkedout maint branch of the submodule without warning 
unless you have merge conflicts.
And when auto-update is just running git submodule update automatically 
it would act as I described.
But you are right, with auto-update the submodule's HEAD can be checked 
against the old gitlink before it is changed. If doing it in two steps 
it is not possible to have this check.

> >
> > I was thinking about submodule integration and had the idea to
> > bind a submodule to the superproject by having special references
> > in the submodule like refs/super/master, refs/super/featureX... So
> > these references are like tracking branches for the refs/heads/* of
> > the superproject.
>
>  Having stuff in the submodule reference branches in the superproject
>   sounds upside down, as a superproject has (and should have) zero
>  knowledge about the superproject (as it could have many different of
>  them).
>
My viewpoint is that I have a big project that is divided into 
submodules because not all developerss need all parts of the project. 
Therefore I wanted something that uses submodules as separate repos, but 
from the users viewpoint it should be as if the submodules are just 
subdirectories. It would include that diffs of submodules are not shown 
as a summary of commit messages but as a diff of the sources. And from 
that perspective it makes more sense to have tracking branches in the 
submodule that are owned by the superproject. In the first thought these 
tracking refs were meant to be readonly in the submodule and only 
updatable from the superproject, but then I thought the possibility of  
detaching and re-attaching is nice, too. One thing I've forgot to 
mention: the refs/super/* are not SHA1-refs to the superproject (that 
would be stupid indeed), but they contain the corresponding gitlink-SHA1 
from the revision referenced by refs/heads/*. So when you have a 
detached HEAD after auto-update you would simply "git checkout -B 
super/<superproject-branchname>" in the submodule, with the difference 
that it shouldn't update refs/heads/super/*, but refs/super/* so that 
these branches can be treated specially.

> > If you have tracking branches, the supermodule can just update the
> > corresponding branch. If this branch is currently checkedout and
> > the work area is clean, then the work area is updated, too. If
> > there is currently a local branch or a diffent super-branch
> > checked out then the working area should be considered "detached"
> > from the superproject and not updated.
>
>  This sounds a lot like the "follow branch tip" model we discussed
>  recently (which could be configured via .gitmodules), but I'm not
>  sure you really are in the same boat here.
When I understood that correctly it was just a configuration to what 
branch should be automatically checked out in the submodule. This seems 
to be too complicated IMO, because when you have different branches in 
the superproject then you may want to have different branches in the 
submodules, too, but you would need to configure that submodule branch 
in .gitmodules for each branch separately. I.e. in the master branch the 
.gitmodule may contain "master", in the maint branch the .gitmodules may 
have "maint" as the branch to follow.
I do want to follow the tip of the branch, if the superproject has that 
currently checked out. If the superproject checks out a tagged version 
for a rebuild, then the submodule should not follow the tip, but should 
get a detached HEAD of the corresponding commit, just as the 
superproject. When the superproject goes back to the branch, the 
submodule should go back to its tracking branch.
>
> > With this concept you could even switch branches in the
> > superproject and the attached submodules follow - still having no
> > detached HEAD. When you want to do some local work on the
> > submodule you checkout a local branch and merge back into the super
> > branch later.
>
>  You lost me here. How can you merge a submodule branch into one of
>  the superproject?
It wouldn't work, if the super/* branch would contain a superproject's 
SHA-1, that is right. But as explained above, it points to a commit of 
the submodule.
>
>  But we would want to have a deterministic update procedure, no? (And
>  what has more freedom than a detached HEAD? ;-)__
I think my proposal would be deterministic.
And everything where you can commit to has more freedom than a detached HEAD

>
> > Even though it will raise a lot of detailed questions like "should
> > the refs/super/* be pushed/pulled when syncing the submodule
> > repositories".
>
>  I doubt that is a good idea, as that might conflict with the same
>  submodule sitting in a different superproject. But I'm interested to
>  hear how you want to solve that.
The first answer to my question was "yes, you need to transfer the refs 
or you get unreferenced objects" and "no, you can't transfer the refs, 
because they are owned by the superproject, not the submodule."
But binding a submodule to a superproject makes perfect sense if it is 
_one_ project that is split into submodules. In that case you only have 
one superproject for a submodule and for that purpose it would be good 
workflow. It is even nice to see which commits in the submodule belong 
to what branches in the superproject or to what release version (so 
tracking superproject tags would make sense, too). If you have a 
submodule that has more than one superproject but these are 
well-defined, it could be solved using refspecs (e.g. refs/super/foo/* 
for one and refs/super/bar/* for the other superproject), but currently 
I can't think of a context where this makes sense.

Of course there are other types of submodules, so using refs/super/* 
wouldn't be a good default variant for auto-update either. E.g. if you 
use a 3rdParty lib, then the detached HEAD is fine, because usually you 
don't touch it except when you switch to a new version from time to time.

^ permalink raw reply

* Re: Undo a commit that is already pushed to central server and merged to several branches
From: Matthias Fechner @ 2011-12-08  9:46 UTC (permalink / raw)
  To: Johan Herland; +Cc: Ramkumar Ramachandra, Git Mailing List
In-Reply-To: <CALKQrgcQ5jv+oDXxDoTGUhmP-Dg344-oSotb+q-4a3fnEBY1Zw@mail.gmail.com>

Dear Ramkumar and Johan,

Am 07.12.11 17:01, schrieb Johan Herland:
> Use "git revert $commit" to undo the effects of the given $commit.
> This must be applied to all affected branches (either by reverting in
> the master branch and remerging master to the other branches, or by
> using "git revert" in each individual branch).

thanks a lot for your help.
The steps I did now was at first undo everything i did locally with:
git reset --hard origin/master

Then undo the bad commit:
git revert commit-id
git commit

Now the bad commit was undone and I merged the master branch in all 
other branches.

I create a new branch and cherry-picked the bad commit into it so I can 
correct the problem there and later merge this branch in all the other ones.

Hopefully this short summary will help other users having the same problem.

Bye
Matthias

-- 
"Programming today is a race between software engineers striving to 
build bigger and better idiot-proof programs, and the universe trying to 
produce bigger and better idiots. So far, the universe is winning." -- 
Rich Cook

^ permalink raw reply

* Re: [PATCH] index-pack: eliminate unlimited recursion in get_delta_base()
From: Nguyen Thai Ngoc Duy @ 2011-12-08 11:06 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: git, Junio C Hamano
In-Reply-To: <CAJo=hJvrk3Jzg3dQhQnfbmKAFovLuEtJAP4rakHPFeuZ0T5R7g@mail.gmail.com>

2011/12/8 Shawn Pearce <spearce@spearce.org>:
> I think you missed the critical recursion. The real work is the
> recursion within find_unresolved_deltas(). This little helper
> get_base_data() shouldn't be tripping over these cases unless we have
> run out of delta_base_cache_limit and released objects near the base
> end of the delta chain, in which case this will restore them.
>
> Maybe this is useful on its own, but in my opinion its not an
> interesting patch to consider without first fixing
> find_unresolved_deltas's recursion.

Thanks. I missed that function. Will try to fix it.
-- 
Duy

^ 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