Git development
 help / color / mirror / Atom feed
* Re: [PATCH v5 11/15] remote-testgit: make clear the 'done' feature
From: Sverre Rabbelier @ 2012-11-21 19:20 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Max Horn, Felipe Contreras, git, Johannes Schindelin, Jeff King,
	Brandon Casey, Brandon Casey, Jonathan Nieder, Ilari Liusvaara,
	Pete Wyckoff, Ben Walton, Matthieu Moy, Julian Phillips
In-Reply-To: <7vy5hu3h11.fsf@alter.siamese.dyndns.org>

On Wed, Nov 21, 2012 at 10:11 AM, Junio C Hamano <gitster@pobox.com> wrote:
> I'd state it like this, but I may have guessed what Felipe intended
> incorrectly.
>
>     remote-testgit: advertise "done" feature and write "done" ourselves
>
>     Instead of letting "fast-export" advertise the feature and ending
>     its stream with "done", do it ourselves.  This way, it would make it
>     more clear to people who want to write their own remote-helper to
>     produce fast-export streams without using "fast-export
>     --use-done-feature" that they are supposed to end their stream with
>     "done".

With that commit message:

Acked-by: Sverre Rabbelier <srabbelier@gmail.com>

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: [PATCH 0/8] fix git-config with duplicate variable entries
From: Jeff King @ 2012-11-21 19:27 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Ævar Arnfjörð Bjarmason, Git Mailing List
In-Reply-To: <7vsj84rt1g.fsf@alter.siamese.dyndns.org>

On Tue, Nov 20, 2012 at 11:08:43AM -0800, Junio C Hamano wrote:

> > 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.
> 
> The way for the Porcelain scripts to mimick the internal "last one
> wins" has been to grab values out of --get-all and do the "last one
> wins" themselves, and I agree that a mode that frees them from
> having to worry about it is a good *addition*.  I would even say
> that if we were designing "git config" plumbing without existing
> users, it would be the only sensible behaviour for "--get".
> 
> But "git config --get" users have been promised to get errors on
> duplicate values so far, so I have to say this needs to come with a
> big red sign about API breakage.

The problem is that scripts currently using "--get" are broken by
duplicate entries in include files, whereas internal git handles them
just fine.  Introducing a new switch means that everybody also has to
start using it.

That is not an excuse for breakage, of course, but only a motivation for
considering it. And here is what I think mitigates that breakage:

  1. If you are a script that cares about multiple values and choosing
     one (whether last-one-wins or otherwise), you are already using
     --get-all and are not affected.

  2. If you are a script that only wants to mimic how get retrieves
     a single value, then this is a bug fix, as it brings "--get" in
     line with git's internal use.

  3. If you are a script that only wants a single value, but cares about
     duplicates, you are already failing to notice them when the
     duplicates are cross-file. That is, we already do "last one wins"
     if an entry is found in ~/.gitconfig and .git/config.

I would argue that anybody fetching standard git config variables (and
not using --list or --get-all) falls into case (2) and is being fixed,
as they will not otherwise match the behavior of git itself.

For other variables that porcelains want to stuff inside the config
files, I suppose they could fall into case (3). But I am not sure why
they would care about duplicates. They have asked git for a single
value, and if they care more deeply about multiple values (but only
within a single file!), what do they hope to accomplish by not calling
--get-all? That is, what is the use case where this makes any sense?

The only case I can think of where the distinction even matters is:

  1. Porcelain foo writes to the .gitfoo file via "git config -f
     .gitfoo".

  2. It accidentally writes using "--add" instead of just setting the
     value.

  3. It fetches via "git config -f .gitfoo --get foo.var". Before my
     patch, duplicate detection would notice the bug in (2) and barf.
     Now, it silently takes the most recently added value (which is
     probably what was meant anyway).

IOW, I do not see a legitimate use case for this, but see it as an extra
check on broken config. But it is catching an unlikely condition, and is
an overly restrictive check, in that it is triggering on totally
reasonable config. So we are not breaking a use case so much as a
loosening a (in my opinion) useless check.

But maybe I am missing some more sane case.

> I would feel safer to introduce --get-one or something now, and
> worry about migration as a separate step.

Part of my impetus for fixing --get is that it let us cleanup and
harmonize the git_config() and git-config implementations. If we are not
going to do that, adding an extra option is not that appealing, as we
are stuck with the old duplicated code, anyway. As I mentioned above,
this only really affects include files (because from git-config's
perspective, the entries it sees are all "the same file", as they come
from the same call into git_config_from_file). If we are not going to
fix --get for all callers, it would probably make more sense to just
omit the duplicate suppression for entries across include-file
boundaries (which are something that callers would have to opt into when
using a specific file, anyway). IOW, to treat it as a bug in the include
system and fix it there.

-Peff

^ permalink raw reply

* Re: Re* [PATCH] gitweb: make remote_heads config setting work.
From: Jeff King @ 2012-11-21 19:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Phil Pennock, git
In-Reply-To: <7vpq37rk3v.fsf_-_@alter.siamese.dyndns.org>

On Tue, Nov 20, 2012 at 02:21:40PM -0800, Junio C Hamano wrote:

> > Good catch. I think the "return" in the existing code suffers from the
> > same problem: it will bail on non-word characters in the $mi part, but
> > that part should allow arbitrary characters.
> 
> I am tired of keeping the "expecting reroll" entries and having to
> worry about them, so let's do this
> 
> -- >8 --
> Subject: [squash] gitweb: make remote_heads config setting work
> 
> Only the top-level and bottom-level names are case insensitive and
> spelled without "_".  Protect future support of subsection names
> from name mangling.

I think the end result is fine, though you are really fixing a bug here
(the /\W/ check is moved up), and then also putting the remote_heads
fix (s/_//g) into the right place. IOW, if you are going to squash, you
should probably note the bug-fix part in the commit message explicitly.

-Peff

^ permalink raw reply

* Re: Topics currently in the Stalled category
From: Jeff King @ 2012-11-21 19:34 UTC (permalink / raw)
  To: Krzysztof Mazur; +Cc: Paul Fox, Junio C Hamano, git
In-Reply-To: <20121121092750.GA8262@shrek.podlesie.net>

On Wed, Nov 21, 2012 at 10:27:50AM +0100, Krzysztof Mazur wrote:

> >  > * pf/editor-ignore-sigint (2012-11-11) 5 commits
> >  > 
> >  >  Avoid confusing cases where the user hits Ctrl-C while in the editor
> >  >  session, not realizing git will receive the signal. Since most editors
> >  >  will take over the terminal and will block SIGINT, this is not likely
> >  >  to confuse anyone.
> >  > 
> >  >  Some people raised issues with emacsclient, which are addressed by this
> >  >  re-roll. It should probably also handle SIGQUIT, and there were a
> >  >  handful of other review comments.
> >  > 
> >  >  Anybody interested in moving this forward?
> > 
> > i started this, but then jeff took it and ran with it and made it
> > right.  i think the remaining changes are small -- if jeff would
> > prefer, i can probably finish it.  (but i won't guarantee not to
> > mess up the "From:" lines.  :-)
> > 
> 
> I'm also interested. I sometimes use ":r !command" in vim, so far I never
> needed to use Ctrl-C, but maybe in future.
> 
> The SIGINT part seems to be finished, we need to decide what about SIGQUIT.

My plan was to just add in SIGQUIT[1] alongside SIGINT (and I think
there may have been one or two other minor comments in response to the
series). I am on vacation this week, but can revisit it next week. If
somebody wants to re-roll it in the meantime, that would be fine with
me.

-Peff

[1] Given the core-dumping behavior of SIGQUIT, I suspect it is not
    nearly as widely used as SIGINT, but it sounds more like the
    principle of least surprise to treat them the same.

^ permalink raw reply

* Re: [PATCH] remote-curl.c: Fix a compiler warning
From: Jeff King @ 2012-11-21 19:34 UTC (permalink / raw)
  To: Ramsay Jones; +Cc: Junio C Hamano, GIT Mailing-list
In-Reply-To: <50AD26C3.2090407@ramsay1.demon.co.uk>

On Wed, Nov 21, 2012 at 07:08:51PM +0000, Ramsay Jones wrote:

> In particular, gcc issues an "'gzip_size' might be used uninitialized"
> warning (-Wuninitialized). However, this warning is a false positive,
> since the 'gzip_size' variable would not, in fact, be used uninitialized.
> In order to suppress the warning, we simply initialise the variable to
> zero in it's declaration.
> 
> Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
> ---
> 
> Hi Junio,
> 
> This is on top of next. (commit df126e108: "remote-curl: hoist gzip
> buffer size to top of post_rpc", 31-10-2012).

Thanks. I meant to apply this during my tenure as maintainer, but it
slipped through the cracks.

-Peff

^ permalink raw reply

* Re: [PATCH 0/8] fix git-config with duplicate variable entries
From: Junio C Hamano @ 2012-11-21 19:46 UTC (permalink / raw)
  To: Jeff King; +Cc: Ævar Arnfjörð Bjarmason, Git Mailing List
In-Reply-To: <20121121192738.GA16280@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

>> The way for the Porcelain scripts to mimick the internal "last one
>> wins" has been to grab values out of --get-all and do the "last one
>> wins" themselves, and I agree that a mode that frees them from
>> having to worry about it is a good *addition*.  I would even say
>> that if we were designing "git config" plumbing without existing
>> users, it would be the only sensible behaviour for "--get".
>> 
>> But "git config --get" users have been promised to get errors on
>> duplicate values so far, so I have to say this needs to come with a
>> big red sign about API breakage.
>
> The problem is that scripts currently using "--get" are broken by
> duplicate entries in include files, whereas internal git handles them
> just fine.  Introducing a new switch means that everybody also has to
> start using it.

Not exactly.  There are three classes of people:

 - wrote scripts using --get; found out that --get barfs if you feed
   two or more of the same, and have long learned to accept it as a
   limitation and adjusted their configuration files to avoid it.
   They have been doing just fine and wouldn't benefit from this
   series at all.

 - wrote scripts using --get, relying on it barfing if fed zero
   (i.e. missing) or two or more (i.e. ambiguous), perhaps a way to
   keep their configuration files (arguably unnecessarily) clean.
   They are directly harmed by this series.

 - wrote scripts using --get-all and did the last-one-wins
   themselves.  They wouldn't benefit directly from this series,
   unless they are willing to spend a bit of time to remove their
   own last-one-wins logic and replace --get-all with --get (but the
   same effort is needed to migrate to --get-one).

 - wanted to write scripts using --get, but after finding out that
   it barfs if you feed two, gave up emulating the internal, without
   realizing that they could do so with --get-all.  They can now
   write their scripts without using --get-all.

The only people who benefit are from the last class; it does not
matter if they have to write --get-one or --get.

> That is not an excuse for breakage, of course, but only a motivation for
> considering it. And here is what I think mitigates that breakage:
>
>   1. If you are a script that cares about multiple values and choosing
>      one (whether last-one-wins or otherwise), you are already using
>      --get-all and are not affected.

Correct.

>   2. If you are a script that only wants to mimic how get retrieves
>      a single value, then this is a bug fix, as it brings "--get" in
>      line with git's internal use.

But by definition, no such script exists (if it used "--get", it
didn't mimic the internal in the first place).

>   3. If you are a script that only wants a single value, but cares about
>      duplicates, you are already failing to notice them when the
>      duplicates are cross-file. That is, we already do "last one wins"
>      if an entry is found in ~/.gitconfig and .git/config.

Yeah, so the only ones that can be harmed is a config lint that uses
the --get option with --file to make sure variables they know must
be single value are indeed so, and they are not doing a thorough
job, unless they are checking across files themselves, at which
point they are better off using --get-all piped to "sort | uniq -c"
or something.  So breakages do not matter much for correctly written
scripts.

> I would argue that anybody fetching standard git config variables (and
> not using --list or --get-all) falls into case (2) and is being fixed,
> as they will not otherwise match the behavior of git itself.

As people shove random stuff that git itself does not care about in
their config files, they may not care, though.

> IOW, I do not see a legitimate use case for this, but see it as an extra
> check on broken config.

Yes, I agree with the latter half of that sentence.

^ permalink raw reply

* Re: [PATCH v5 15/15] fast-export: don't handle uninteresting refs
From: Jeff King @ 2012-11-21 19:48 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Jonathan Nieder, Felipe Contreras, git, Johannes Schindelin,
	Max Horn, Sverre Rabbelier, Brandon Casey, Brandon Casey,
	Ilari Liusvaara, Pete Wyckoff, Ben Walton, Matthieu Moy,
	Julian Phillips
In-Reply-To: <7vfw43pmp7.fsf@alter.siamese.dyndns.org>

On Tue, Nov 20, 2012 at 09:08:36PM -0800, Junio C Hamano wrote:

> With such a one-sided discussion, I've been having a hard time
> convincing myself if Felipe's effort is making the interface better,
> or just breaking it even more for existing remote helpers, only to
> fit his world model better.

Felipe responded in more detail, but I will just add the consensus we
came to earlier in the discussion: the series does make things better
for users of fast-export that use marks, but does not make things any
better for users of negative refs on the command line. However, I do not
think that it makes things worse for them, either (neither by changing
the behavior negatively, nor by making the code harder for a more
complete fix later).

So while fixing everybody might be nice, there is no need to hold up
progress for the marks case. Which, as he has noted, is probably the
sanest way to implement a remote-helper[1].

-Peff

[1] There are other possible use cases for fast-export which might
    benefit from negative refs working more sanely, but since they are
    in the minority and are not being made worse, I think the partial
    fix is OK.

^ permalink raw reply

* Re: Topics currently in the Stalled category
From: Paul Fox @ 2012-11-21 19:50 UTC (permalink / raw)
  To: Jeff King; +Cc: Krzysztof Mazur, git
In-Reply-To: <20121121193401.GC16280@sigill.intra.peff.net>

jeff wrote:
 > On Wed, Nov 21, 2012 at 10:27:50AM +0100, Krzysztof Mazur wrote:
 > 
 > > >  > * pf/editor-ignore-sigint (2012-11-11) 5 commits
 > > >  > 
 > > >  >  Avoid confusing cases where the user hits Ctrl-C while in the editor
 > > >  >  session, not realizing git will receive the signal. Since most editors
 > > >  >  will take over the terminal and will block SIGINT, this is not likely
 > > >  >  to confuse anyone.
 > > >  > 
 > > >  >  Some people raised issues with emacsclient, which are addressed by this
 > > >  >  re-roll. It should probably also handle SIGQUIT, and there were a
 > > >  >  handful of other review comments.
 > > >  > 
 > > >  >  Anybody interested in moving this forward?
 > > > 
 > > > i started this, but then jeff took it and ran with it and made it
 > > > right.  i think the remaining changes are small -- if jeff would
 > > > prefer, i can probably finish it.  (but i won't guarantee not to
 > > > mess up the "From:" lines.  :-)
 > > > 
 > > 
 > > I'm also interested. I sometimes use ":r !command" in vim, so far I never
 > > needed to use Ctrl-C, but maybe in future.
 > > 
 > > The SIGINT part seems to be finished, we need to decide what about SIGQUIT.
 > 
 > My plan was to just add in SIGQUIT[1] alongside SIGINT (and I think
 > there may have been one or two other minor comments in response to the
 > series). I am on vacation this week, but can revisit it next week. If
 > somebody wants to re-roll it in the meantime, that would be fine with
 > me.
 > 
 > -Peff
 > 
 > [1] Given the core-dumping behavior of SIGQUIT, I suspect it is not
 >     nearly as widely used as SIGINT, but it sounds more like the
 >     principle of least surprise to treat them the same.

i see no real point in treating them the same -- as you suggest, one
would only use SIGQUIT if SIGINT didn't work, and then you'd want them
to be treated differently.  so i'd be happy with the current code in
that regard.

i think krzysiek said that since editors usually catch SIGQUIT, git
should kill the editor when receiving SIGQUIT, essentially translating
the SIGQUIT to SIGTERM for the editor.  (please correct me if i
misunderstood.)  since well-behaved editors will die quickly anyway
(they get EIO on their next read from stdin), i'm not sure there's a
compelling reason for that extra step.

but i have no real objection to that behavior if others think it's
right -- there's certainly logic in saying that if git dies it should
ensure the editor does too.

(i'm away for the rest of the week also.)

paul
=---------------------
 paul fox, pgf@foxharp.boston.ma.us (arlington, ma, where it's 44.6 degrees)

^ permalink raw reply

* Re: Topics currently in the Stalled category
From: Junio C Hamano @ 2012-11-21 19:53 UTC (permalink / raw)
  To: Jeff King; +Cc: Krzysztof Mazur, Paul Fox, git
In-Reply-To: <20121121193401.GC16280@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> [1] Given the core-dumping behavior of SIGQUIT, I suspect it is not
>     nearly as widely used as SIGINT, but it sounds more like the
>     principle of least surprise to treat them the same.

Sounds sensible.  I wonder what happens when the editor is suspended
;-)

^ permalink raw reply

* Re: [PATCH 0/8] fix git-config with duplicate variable entries
From: Jeff King @ 2012-11-21 20:06 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Ævar Arnfjörð Bjarmason, Git Mailing List
In-Reply-To: <7vehjm20yu.fsf@alter.siamese.dyndns.org>

On Wed, Nov 21, 2012 at 11:46:33AM -0800, Junio C Hamano wrote:

> > The problem is that scripts currently using "--get" are broken by
> > duplicate entries in include files, whereas internal git handles them
> > just fine.  Introducing a new switch means that everybody also has to
> > start using it.
> 
> Not exactly.  There are three classes of people:
> 
>  - wrote scripts using --get; found out that --get barfs if you feed
>    two or more of the same, and have long learned to accept it as a
>    limitation and adjusted their configuration files to avoid it.
>    They have been doing just fine and wouldn't benefit from this
>    series at all.
> 
>  - wrote scripts using --get, relying on it barfing if fed zero
>    (i.e. missing) or two or more (i.e. ambiguous), perhaps a way to
>    keep their configuration files (arguably unnecessarily) clean.
>    They are directly harmed by this series.
> 
>  - wrote scripts using --get-all and did the last-one-wins
>    themselves.  They wouldn't benefit directly from this series,
>    unless they are willing to spend a bit of time to remove their
>    own last-one-wins logic and replace --get-all with --get (but the
>    same effort is needed to migrate to --get-one).
> 
>  - wanted to write scripts using --get, but after finding out that
>    it barfs if you feed two, gave up emulating the internal, without
>    realizing that they could do so with --get-all.  They can now
>    write their scripts without using --get-all.

I have a feeling that your cases 2 and 4 do not really exist. Because we
did "last one wins" in the case that it mattered (between different
files), it was always "good enough" to just assume that using "--get"
behaved like git did internally, and nobody really noticed or cared that
we did duplicate detection at all. But that is just from my own
perspective; it is not like I did a complete survey of git-config users.

More compelling to me is that the only ones negatively affected are your
case 2, and that is qualified by the "arguably unnecessary" you wrote.

Everyone else is not negatively impacted, and can later move to using
"--get" if they want to give up any home-grown --get-all code.

> >   2. If you are a script that only wants to mimic how get retrieves
> >      a single value, then this is a bug fix, as it brings "--get" in
> >      line with git's internal use.
> 
> But by definition, no such script exists (if it used "--get", it
> didn't mimic the internal in the first place).

They do not exist if we assume that anyone using "--get" carefully
thought about the distinction. But I have the impression that is not the
case, and that they _meant_ to behave just like git, and did not realize
they were not doing so. Even our own scripts are guilty of this.

> Yeah, so the only ones that can be harmed is a config lint that uses
> the --get option with --file to make sure variables they know must
> be single value are indeed so, and they are not doing a thorough
> job, unless they are checking across files themselves, at which
> point they are better off using --get-all piped to "sort | uniq -c"
> or something.  So breakages do not matter much for correctly written
> scripts.

Right.

> > IOW, I do not see a legitimate use case for this, but see it as an extra
> > check on broken config.
> 
> Yes, I agree with the latter half of that sentence.

So what do we want to do?

-Peff

^ permalink raw reply

* Re: Topics currently in the Stalled category
From: Jeff King @ 2012-11-21 20:13 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Krzysztof Mazur, Paul Fox, git
In-Reply-To: <7va9ua20nz.fsf@alter.siamese.dyndns.org>

On Wed, Nov 21, 2012 at 11:53:04AM -0800, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > [1] Given the core-dumping behavior of SIGQUIT, I suspect it is not
> >     nearly as widely used as SIGINT, but it sounds more like the
> >     principle of least surprise to treat them the same.
> 
> Sounds sensible.  I wonder what happens when the editor is suspended
> ;-)

I think we would want to leave SIGTSTP alone; the editor should
typically respect it, and we would want to also pause until we get
SIGCONT (although even if we did continue, we would just be blocking on
wait() for the editor, anyway, so it is not a big deal).

Implicit in my "least surprise" comment above is that handling SIGQUIT
would match what system(3) does, so it makes sense to me to match that
(it also blocks SIGCHLD, but I do not think that really matters from a
user-visible perspective).

-Peff

^ permalink raw reply

* Re: [PATCH 2/2] pickaxe: use textconv for -S counting
From: Jeff King @ 2012-11-21 20:27 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Peter Oberndorfer, git
In-Reply-To: <7vr4npt7zd.fsf@alter.siamese.dyndns.org>

On Mon, Nov 19, 2012 at 04:48:22PM -0800, Junio C Hamano wrote:

> Junio C Hamano <gitster@pobox.com> writes:
> 
> >> Exact renames are the obvious one, but they are not handled here.
> >
> > That is half true.  Before this change, we will find the same number
> > of needles and this function would have said "no differences" in a
> > very inefficient way.  After this change, we may apply different
> > textconv filters and this function will say "there is a difference",
> > even though we wouldn't see such a difference at the content level
> > if there wasn't any rename.
> 
> ... but I think that is a good thing anyway.
> 
> If you renamed foo.c to foo.cc with different conversions from C
> code to the text that explain what the code does, if we special case
> only the exact rename case but let pickaxe examine the converted
> result in a case where blobs are modified only by one byte, we would
> get drastically different results between the two cases.

Right, exactly. I think the only sane thing is to always textconv or
always not textconv (whether they are identical renames or not), and any
"these are the same" optimization for identical content needs to take
into account whether we _would have_ done a different textconv (which
most of the time is going to be "no", as textconv is either not in use,
or both paths use the same diff driver; but it is not too expensive to
look up).

The diff_unmodified_pair at the top off diff_flush_patch is correct,
because it treats renames as interesting (because we have to show the
diff header, anyway). I do not know offhand if we avoid feeding
identical content to xdiff at all, but if so, we should be doing so only
after checking that the textconv filters are identical.

> Corollary to this is what should happen when you update the attributes
> between two trees so that textconv for a path that did not change
> between preimage and postimage are different.  Ideally, we should
> notice that the two converted result are different, perhaps, but I
> do not like the performance implications very much.

The content to compare cannot be different unless either the input
content changed or the path changed, and we treat either as
"interesting" in most code paths. So I do not think there are any
performance implications, except that we may need to make sure to look
up textconvs a few lines sooner in some cases.

I'll re-roll the series next week and break out the rename-optimization
bits separately so it is more obvious that it is doing the right thing.

As an aside, I also need to revisit the regex half of that code, which
is still buggy (before and after my patch, due to the expecting-a-NUL
behavior we talked about a week or two ago).  That is a separate topic,
but the same area of code.

-Peff

^ permalink raw reply

* Re: [PATCH 0/8] fix git-config with duplicate variable entries
From: Junio C Hamano @ 2012-11-21 20:42 UTC (permalink / raw)
  To: Jeff King; +Cc: Ævar Arnfjörð Bjarmason, Git Mailing List
In-Reply-To: <20121121200624.GF16280@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> So what do we want to do?

Nothing.  We'd just let it graduate along with other topics, and
deal with a case where somebody screams, which is unlikely to happen
;-).

^ permalink raw reply

* [PATCH v2 3/4] format-patch: update append_signoff prototype
From: Nguyễn Thái Ngọc Duy @ 2012-11-22 16:38 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <1353602289-9418-1-git-send-email-pclouds@gmail.com>

This is a preparation step for merging with append_signoff from
sequencer.c

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/log.c | 13 +------------
 log-tree.c    | 21 +++++++++++++--------
 revision.h    |  2 +-
 3 files changed, 15 insertions(+), 21 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index 09cf43e..f201070 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1053,7 +1053,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	struct commit *origin = NULL, *head = NULL;
 	const char *in_reply_to = NULL;
 	struct patch_ids ids;
-	char *add_signoff = NULL;
 	struct strbuf buf = STRBUF_INIT;
 	int use_patch_format = 0;
 	int quiet = 0;
@@ -1148,16 +1147,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 			     PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN |
 			     PARSE_OPT_KEEP_DASHDASH);
 
-	if (do_signoff) {
-		const char *committer;
-		const char *endpos;
-		committer = git_committer_info(IDENT_STRICT);
-		endpos = strchr(committer, '>');
-		if (!endpos)
-			die(_("bogus committer info %s"), committer);
-		add_signoff = xmemdupz(committer, endpos - committer + 1);
-	}
-
 	for (i = 0; i < extra_hdr.nr; i++) {
 		strbuf_addstr(&buf, extra_hdr.items[i].string);
 		strbuf_addch(&buf, '\n');
@@ -1348,7 +1337,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		total++;
 		start_number--;
 	}
-	rev.add_signoff = add_signoff;
+	rev.add_signoff = do_signoff;
 	while (0 <= --nr) {
 		int shown;
 		commit = list[nr];
diff --git a/log-tree.c b/log-tree.c
index aff7c0a..7e50545 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -253,9 +253,11 @@ static int detect_any_signoff(char *letter, int size)
 	return seen_head && seen_name;
 }
 
-static void append_signoff(struct strbuf *sb, const char *signoff)
+static void append_signoff(struct strbuf *sb, int flags)
 {
-	static const char signed_off_by[] = "Signed-off-by: ";
+	char* signoff = xstrdup(fmt_name(getenv("GIT_COMMITTER_NAME"),
+					 getenv("GIT_COMMITTER_EMAIL")));
+	static const char sign_off_header[] = "Signed-off-by: ";
 	size_t signoff_len = strlen(signoff);
 	int has_signoff = 0;
 	char *cp;
@@ -274,9 +276,9 @@ static void append_signoff(struct strbuf *sb, const char *signoff)
 	}
 
 	/* First see if we already have the sign-off by the signer */
-	while ((cp = strstr(cp, signed_off_by))) {
+	while ((cp = strstr(cp, sign_off_header))) {
 		const char *s = cp;
-		cp += strlen(signed_off_by);
+		cp += strlen(sign_off_header);
 
 		if (!has_signoff && s > sb->buf) {
 			/*
@@ -317,9 +319,10 @@ static void append_signoff(struct strbuf *sb, const char *signoff)
 	if (!has_signoff)
 		strbuf_addch(sb, '\n');
 
-	strbuf_addstr(sb, signed_off_by);
+	strbuf_addstr(sb, sign_off_header);
 	strbuf_add(sb, signoff, signoff_len);
 	strbuf_addch(sb, '\n');
+	free(signoff);
 }
 
 static unsigned int digits_in_number(unsigned int number)
@@ -684,8 +687,10 @@ void show_log(struct rev_info *opt)
 	/*
 	 * And then the pretty-printed message itself
 	 */
-	if (ctx.need_8bit_cte >= 0)
-		ctx.need_8bit_cte = has_non_ascii(opt->add_signoff);
+	if (ctx.need_8bit_cte >= 0 && opt->add_signoff)
+		ctx.need_8bit_cte =
+			has_non_ascii(fmt_name(getenv("GIT_COMMITTER_NAME"),
+					       getenv("GIT_COMMITTER_EMAIL")));
 	ctx.date_mode = opt->date_mode;
 	ctx.date_mode_explicit = opt->date_mode_explicit;
 	ctx.abbrev = opt->diffopt.abbrev;
@@ -696,7 +701,7 @@ void show_log(struct rev_info *opt)
 	pretty_print_commit(&ctx, commit, &msgbuf);
 
 	if (opt->add_signoff)
-		append_signoff(&msgbuf, opt->add_signoff);
+		append_signoff(&msgbuf, 0);
 	if (opt->show_log_size) {
 		printf("log size %i\n", (int)msgbuf.len);
 		graph_show_oneline(opt->graph);
diff --git a/revision.h b/revision.h
index a95bd0b..af35325 100644
--- a/revision.h
+++ b/revision.h
@@ -136,7 +136,7 @@ struct rev_info {
 	int		numbered_files;
 	char		*message_id;
 	struct string_list *ref_message_ids;
-	const char	*add_signoff;
+	int              add_signoff;
 	const char	*extra_headers;
 	const char	*log_reencode;
 	const char	*subject_prefix;
-- 
1.8.0.4.g5d0415a

^ permalink raw reply related

* Re: [PATCH v4 2/4] diff: introduce diff.submodule configuration variable
From: Ramkumar Ramachandra @ 2012-11-22  9:40 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git List
In-Reply-To: <7vk3tht7yz.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:
> Ramkumar Ramachandra <artagnon@gmail.com> writes:
>
>> diff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh
>> index 6c01d0c..e401814 100755
>> --- a/t/t4041-diff-submodule-option.sh
>> +++ b/t/t4041-diff-submodule-option.sh
>> @@ -33,6 +33,7 @@ test_create_repo sm1 &&
>>  add_file . foo >/dev/null
>>
>>  head1=$(add_file sm1 foo1 foo2)
>> +fullhead1=$(cd sm1; git rev-list --max-count=1 $head1)
>
> That looks like quite a roundabout way to say
>
>         $(cd sm1; git rev-parse --verify HEAD)
>
> doesn't it?  I know this is just moved code from the original, so I
> am not saying this should be fixed in this commit.

Exactly what I was thinking.

> Existing code in t7401 may want to be cleaned up, perhaps after this
> topic settles.  The add_file() function, for example, has at least
> these points:
>
>  - do we know 7 hexdigits is always the right precision?
>  - what happens when it fails to create a commit in one of the steps
>    while looping over "$@" in its loop?
>  - the function uses the "cd there and then come back", not
>    "go there in a subshell and do whatever it needs to" pattern.

Okay, will do.

>> +test_expect_success 'added submodule, set diff.submodule' "
>
> s/added/add/;

I see that the topic is already in `next`.  Do you want to fix it up there?

> Shouldn't it test the base case where no diff.submodule is set?  For
> those people, the patch should not change the output when they do or
> do not pass --submodule option, right?

When no diff.submodule is set, `git diff --submodule` should just work
as before; isn't this tested by the other tests in the file?

Ram

^ permalink raw reply

* [PATCH 1/5] git-send-email: remove garbage after email address
From: Krzysztof Mazur @ 2012-11-22 18:12 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Felipe Contreras, Andreas Schwab, Felipe Balbi,
	Tomi Valkeinen, Krzysztof Mazur
In-Reply-To: <7v8v9vrgc9.fsf@alter.siamese.dyndns.org>

In some cases it's very useful to add some additional information
after email in Cc-list, for instance:

"Cc: Stable kernel <stable@vger.kernel.org> #v3.4 v3.5 v3.6"

Currently the git refuses to add such invalid email to Cc-list,
when the Email::Valid perl module is available or just uses whole line
as the email address.

Now in sanitize_address() everything after the email address is
removed, so the resulting line is correct email address and Email::Valid
validates it correctly.

Signed-off-by: Krzysztof Mazur <krzysiek@podlesie.net>
Tested-by: Felipe Balbi <balbi@ti.com>
---
 git-send-email.perl | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/git-send-email.perl b/git-send-email.perl
index 5a7c29d..9840d0a 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -924,6 +924,10 @@ sub quote_subject {
 # use the simplest quoting being able to handle the recipient
 sub sanitize_address {
 	my ($recipient) = @_;
+
+	# remove garbage after email address
+	$recipient =~ s/(.*>).*$/$1/;
+
 	my ($recipient_name, $recipient_addr) = ($recipient =~ /^(.*?)\s*(<.*)/);
 
 	if (not $recipient_name) {
-- 
1.8.0.393.gcc9701d

^ permalink raw reply related

* What's cooking in git.git (Nov 2012, #07; Wed, 21)
From: Junio C Hamano @ 2012-11-21 22:18 UTC (permalink / raw)
  To: git

What's cooking in git.git (Nov 2012, #07; Wed, 21)
--------------------------------------------------

Here are the topics that have been cooking.  Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.

Many topics have been merged to 'maint' in preparation for 1.8.0.1;
and some more dormant topics have been moved to the stalled category
(to be discarded without prejudice unless they see some activities).
The upcoming 1.8.1 release is slowly taking shape.

You can find the changes described here in the integration branches
of the repositories listed at

    http://git-blame.blogspot.com/p/git-public-repositories.html

--------------------------------------------------
[New Topics]

* nd/maint-compat-fnmatch-fix (2012-11-20) 1 commit
  (merged to 'next' on 2012-11-21 at ce6fbe5)
 + compat/fnmatch: fix off-by-one character class's length check

 Will merge to 'master' and then 'maint'.

--------------------------------------------------
[Graduated to "master"]

* cn/config-missing-path (2012-11-15) 1 commit
  (merged to 'next' on 2012-11-18 at c08b73c)
 + config: don't segfault when given --path with a missing value


* jk/checkout-out-of-unborn (2012-11-15) 1 commit
  (merged to 'next' on 2012-11-18 at 7d2aa24)
 + checkout: print a message when switching unborn branches


* jk/config-ignore-duplicates (2012-10-29) 9 commits
  (merged to 'next' on 2012-10-29 at 67fa0a2)
 + builtin/config.c: Fix a sparse warning
  (merged to 'next' on 2012-10-25 at 233df08)
 + git-config: use git_config_with_options
 + git-config: do not complain about duplicate entries
 + git-config: collect values instead of immediately printing
 + git-config: fix regexp memory leaks on error conditions
 + git-config: remove memory leak of key regexp
 + t1300: test "git config --get-all" more thoroughly
 + t1300: remove redundant test
 + t1300: style updates

 Drop duplicate detection from git-config; this lets it better match
 the internal config callbacks, which clears up some corner cases
 with includes.  This is an API breakage, though.


* jk/maint-gitweb-xss (2012-11-12) 1 commit
  (merged to 'next' on 2012-11-14 at 7a667bc)
 + gitweb: escape html in rss title

 Fixes an XSS vulnerability in gitweb.


* jk/maint-http-half-auth-fetch (2012-10-31) 2 commits
  (merged to 'next' on 2012-11-09 at af69926)
 + remote-curl: retry failed requests for auth even with gzip
 + remote-curl: hoist gzip buffer size to top of post_rpc

 Fixes fetch from servers that ask for auth only during the actual
 packing phase. This is not really a recommended configuration, but it
 cleans up the code at the same time.


* jl/submodule-rm (2012-11-14) 1 commit
  (merged to 'next' on 2012-11-18 at bf4525d)
 + docs: move submodule section

 Documentation correction for d21240f (Merge branch
 'jl/submodule-rm', 2012-10-29) that needs to be fast-tracked.


* kb/preload-index-more (2012-11-02) 1 commit
  (merged to 'next' on 2012-11-09 at a750ebd)
 + update-index/diff-index: use core.preloadindex to improve performance

 Use preloadindex in more places, which has a nice speedup on systems
 with slow stat calls (and even on Linux).


* mg/replace-resolve-delete (2012-11-13) 1 commit
  (merged to 'next' on 2012-11-14 at fa785ae)
 + replace: parse revision argument for -d

 Be more user friendly to people using "git replace -d".


* mh/alt-odb-string-list-cleanup (2012-11-08) 2 commits
  (merged to 'next' on 2012-11-13 at 2bf41d9)
 + link_alt_odb_entries(): take (char *, len) rather than two pointers
 + link_alt_odb_entries(): use string_list_split_in_place()

 Cleanups in the alternates code. Fixes a potential bug and makes the
 code much cleaner.


* ml/cygwin-mingw-headers (2012-11-18) 2 commits
  (merged to 'next' on 2012-11-19 at f9964da)
 + USE CGYWIN_V15_WIN32API as macro to select api for cygwin
  (merged to 'next' on 2012-11-15 at 22e11b3)
 + Update cygwin.c for new mingw-64 win32 api headers

 Make git work on newer cygwin.


* pw/maint-p4-rcs-expansion-newline (2012-11-08) 1 commit
  (merged to 'next' on 2012-11-13 at e90cc7c)
 + git p4: RCS expansion should not span newlines

 I do not have p4 to play with, but looks obviously correct to me.


* rh/maint-gitweb-highlight-ext (2012-11-08) 1 commit
  (merged to 'next' on 2012-11-13 at c57d856)
 + gitweb.perl: fix %highlight_ext mappings

 Fixes a clever misuse of perl's list interpretation.


* so/prompt-command (2012-10-17) 4 commits
  (merged to 'next' on 2012-10-25 at 79565a1)
 + coloured git-prompt: paint detached HEAD marker in red
 + Fix up colored git-prompt
 + show color hints based on state of the git tree
 + Allow __git_ps1 to be used in PROMPT_COMMAND

 Updates __git_ps1 so that it can be used as $PROMPT_COMMAND,
 instead of being used for command substitution in $PS1, to embed
 color escape sequences in its output.

 Will merge to 'master' in the seventh batch.


* ta/doc-cleanup (2012-10-25) 6 commits
  (merged to 'next' on 2012-11-13 at e11fafd)
 + Documentation: build html for all files in technical and howto
 + Documentation/howto: convert plain text files to asciidoc
 + Documentation/technical: convert plain text files to asciidoc
 + Change headline of technical/send-pack-pipeline.txt to not confuse its content with content from git-send-pack.txt
 + Shorten two over-long lines in git-bisect-lk2009.txt by abbreviating some sha1
 + Split over-long synopsis in git-fetch-pack.txt into several lines

--------------------------------------------------
[Stalled]

* pf/editor-ignore-sigint (2012-11-11) 5 commits
 - launch_editor: propagate SIGINT from editor to git
 - run-command: do not warn about child death by SIGINT
 - run-command: drop silent_exec_failure arg from wait_or_whine
 - launch_editor: ignore SIGINT while the editor has control
 - launch_editor: refactor to use start/finish_command

 Avoid confusing cases where the user hits Ctrl-C while in the editor
 session, not realizing git will receive the signal. Since most editors
 will take over the terminal and will block SIGINT, this is not likely
 to confuse anyone.

 Some people raised issues with emacsclient, which are addressed by this
 re-roll. It should probably also handle SIGQUIT, and there were a
 handful of other review comments.

 Expecting a re-roll.


* mo/cvs-server-updates (2012-10-16) 10 commits
 - cvsserver Documentation: new cvs ... -r support
 - cvsserver: add t9402 to test branch and tag refs
 - cvsserver: support -r and sticky tags for most operations
 - cvsserver: Add version awareness to argsfromdir
 - cvsserver: generalize getmeta() to recognize commit refs
 - cvsserver: implement req_Sticky and related utilities
 - cvsserver: add misc commit lookup, file meta data, and file listing functions
 - cvsserver: define a tag name character escape mechanism
 - cvsserver: cleanup extra slashes in filename arguments
 - cvsserver: factor out git-log parsing logic

 Needs review by folks interested in cvsserver.


* jn/warn-on-inaccessible-loosen (2012-10-14) 4 commits
 - config: exit on error accessing any config file
 - doc: advertise GIT_CONFIG_NOSYSTEM
 - config: treat user and xdg config permission problems as errors
 - config, gitignore: failure to access with ENOTDIR is ok

 An RFC to deal with a situation where .config/git is a file and we
 notice .config/git/config is not readable due to ENOTDIR, not
 ENOENT; I think a bit more refactored approach to consistently
 address permission errors across config, exclude and attrs is
 desirable.  Don't we also need a check for an opposite situation
 where we open .config/git/config or .gitattributes for reading but
 they turn out to be directories?


* as/check-ignore (2012-11-08) 14 commits
 - t0007: fix tests on Windows
 - Documentation/check-ignore: we show the deciding match, not the first
 - Add git-check-ignore sub-command
 - dir.c: provide free_directory() for reclaiming dir_struct memory
 - pathspec.c: move reusable code from builtin/add.c
 - dir.c: refactor treat_gitlinks()
 - dir.c: keep track of where patterns came from
 - dir.c: refactor is_path_excluded()
 - dir.c: refactor is_excluded()
 - dir.c: refactor is_excluded_from_list()
 - dir.c: rename excluded() to is_excluded()
 - dir.c: rename excluded_from_list() to is_excluded_from_list()
 - dir.c: rename path_excluded() to is_path_excluded()
 - dir.c: rename cryptic 'which' variable to more consistent name

 Duy helped to reroll this.

 Expecting a re-roll.


* aw/rebase-am-failure-detection (2012-10-11) 1 commit
 - rebase: Handle cases where format-patch fails

 I am unhappy a bit about the possible performance implications of
 having to store the output in a temporary file only for a rare case
 of format-patch aborting.


* jk/lua-hackery (2012-10-07) 6 commits
 - pretty: fix up one-off format_commit_message calls
 - Minimum compilation fixup
 - Makefile: make "lua" a bit more configurable
 - add a "lua" pretty format
 - add basic lua infrastructure
 - pretty: make some commit-parsing helpers more public

 Interesting exercise. When we do this for real, we probably would want
 to wrap a commit to make it more like an "object" with methods like
 "parents", etc.


* fc/remote-testgit-feature-done (2012-10-29) 1 commit
 - remote-testgit: properly check for errors

 Is this still in "Needs review" state?  Are people involved in the
 remote interface happy with this change?


* jk/send-email-sender-prompt (2012-11-15) 8 commits
 - send-email: do not prompt for explicit repo ident
 - Git.pm: teach "ident" to query explicitness
 - var: provide explicit/implicit ident information
 - var: accept multiple variables on the command line
 - ident: keep separate "explicit" flags for author and committer
 - ident: make user_ident_explicitly_given static
 - t7502: factor out autoident prerequisite
 - test-lib: allow negation of prerequisites

 Avoid annoying sender prompt in git-send-email, but only when it is
 safe to do so.

 Perhaps keep only the first three patches, and replace the rest
 with the one from Felipe that takes a much simpler approach (the
 rationale of that patch needs to be cleaned up first, along the
 lines Jeff outlined, though).  Frozen until that happens.


* nd/unify-appending-of-s-o-b (2012-11-15) 1 commit
 - Unify appending signoff in format-patch, commit and sequencer

 I am not sure if the logic to refrain from adding a sign-off based
 on the existing run of sign-offs is done correctly in this change.


* nd/pretty-placeholder-with-color-option (2012-09-30) 9 commits
 . pretty: support %>> that steal trailing spaces
 . pretty: support truncating in %>, %< and %><
 . pretty: support padding placeholders, %< %> and %><
 . pretty: two phase conversion for non utf-8 commits
 . utf8.c: add utf8_strnwidth() with the ability to skip ansi sequences
 . utf8.c: move display_mode_esc_sequence_len() for use by other functions
 . pretty: support %C(auto[,N]) to turn on coloring on next placeholder(s)
 . pretty: split parsing %C into a separate function
 . pretty: share code between format_decoration and show_decorations

 This causes warnings with -Wuninitialized, so I've ejected it from pu
 for the time being.


* rc/maint-complete-git-p4 (2012-09-24) 1 commit
  (merged to 'next' on 2012-10-29 at af52cef)
 + Teach git-completion about git p4

 Comment from Pete will need to be addressed in a follow-up patch.


* as/test-tweaks (2012-09-20) 7 commits
 - tests: paint unexpectedly fixed known breakages in bold red
 - tests: test the test framework more thoroughly
 - [SQUASH] t/t0000-basic.sh: quoting of TEST_DIRECTORY is screwed up
 - tests: refactor mechanics of testing in a sub test-lib
 - tests: paint skipped tests in bold blue
 - tests: test number comes first in 'not ok $count - $message'
 - tests: paint known breakages in bold yellow

 Various minor tweaks to the test framework to paint its output
 lines in colors that match what they mean better.

 Has the "is this really blue?" issue Peff raised resolved???


* jc/maint-name-rev (2012-09-17) 7 commits
 - describe --contains: use "name-rev --algorithm=weight"
 - name-rev --algorithm=weight: tests and documentation
 - name-rev --algorithm=weight: cache the computed weight in notes
 - name-rev --algorithm=weight: trivial optimization
 - name-rev: --algorithm option
 - name_rev: clarify the logic to assign a new tip-name to a commit
 - name-rev: lose unnecessary typedef

 "git name-rev" names the given revision based on a ref that can be
 reached in the smallest number of steps from the rev, but that is
 not useful when the caller wants to know which tag is the oldest one
 that contains the rev.  This teaches a new mode to the command that
 uses the oldest ref among those which contain the rev.

 I am not sure if this is worth it; for one thing, even with the help
 from notes-cache, it seems to make the "describe --contains" even
 slower. Also the command will be unusably slow for a user who does
 not have a write access (hence unable to create or update the
 notes-cache).

 Stalled mostly due to lack of responses.


* jc/xprm-generation (2012-09-14) 1 commit
 - test-generation: compute generation numbers and clock skews

 A toy to analyze how bad the clock skews are in histories of real
 world projects.

 Stalled mostly due to lack of responses.


* jc/blame-no-follow (2012-09-21) 2 commits
 - blame: pay attention to --no-follow
 - diff: accept --no-follow option

 Teaches "--no-follow" option to "git blame" to disable its
 whole-file rename detection.

 Stalled mostly due to lack of responses.


* jc/doc-default-format (2012-10-07) 2 commits
 - [SQAUSH] allow "cd Doc* && make DEFAULT_DOC_TARGET=..."
 - Allow generating a non-default set of documentation

 Need to address the installation half if this is to be any useful.


* mk/maint-graph-infinity-loop (2012-09-25) 1 commit
 - graph.c: infinite loop in git whatchanged --graph -m

 The --graph code fell into infinite loop when asked to do what the
 code did not expect ;-)

 Anybody who worked on "--graph" wants to comment?
 Stalled mostly due to lack of responses.


* jc/add-delete-default (2012-08-13) 1 commit
 - git add: notice removal of tracked paths by default

 "git add dir/" updated modified files and added new files, but does
 not notice removed files, which may be "Huh?" to some users.  They
 can of course use "git add -A dir/", but why should they?

 Resurrected from graveyard, as I thought it was a worthwhile thing
 to do in the longer term.

 Waiting for comments.


* mb/remote-default-nn-origin (2012-07-11) 6 commits
 - Teach get_default_remote to respect remote.default.
 - Test that plain "git fetch" uses remote.default when on a detached HEAD.
 - Teach clone to set remote.default.
 - Teach "git remote" about remote.default.
 - Teach remote.c about the remote.default configuration setting.
 - Rename remote.c's default_remote_name static variables.

 When the user does not specify what remote to interact with, we
 often attempt to use 'origin'.  This can now be customized via a
 configuration variable.

 Expecting a re-roll.

 "The first remote becomes the default" bit is better done as a
 separate step.


* mh/ceiling (2012-10-29) 8 commits
 - string_list_longest_prefix(): remove function
 - setup_git_directory_gently_1(): resolve symlinks in ceiling paths
 - longest_ancestor_length(): require prefix list entries to be normalized
 - longest_ancestor_length(): take a string_list argument for prefixes
 - longest_ancestor_length(): use string_list_split()
 - Introduce new function real_path_if_valid()
 - real_path_internal(): add comment explaining use of cwd
 - Introduce new static function real_path_internal()

 Elements of GIT_CEILING_DIRECTORIES list may not match the real
 pathname we obtain from getcwd(), leading the GIT_DIR discovery
 logic to escape the ceilings the user thought to have specified.

--------------------------------------------------
[Cooking]

* fc/fast-export-fixes (2012-11-21) 19 commits
 - fast-export: don't handle uninteresting refs
 - fast-export: make sure updated refs get updated
 - fast-export: fix comparison in tests
 - fast-export: trivial cleanup
 - remote-testgit: advertise "done" feature and write "done" ourselves
 - fixup! remote-testgit: report success after an import
 - remote-testgit: report success after an import
 - fixup! remote-testgit: exercise non-default refspec feature
 - remote-testgit: exercise non-default refspec feature
 - remote-testgit: cleanup tests
 - remote-testgit: remove irrelevant test
 - remote-testgit: remove non-local tests
 - fixup! Add git-remote-testgit
 - Add git-remote-testgit
 - Rename git-remote-testgit to git-remote-testpy
 - remote-helpers: fix failure message
 - remote-testgit: fix direction of marks
 - fixup! fast-export: avoid importing blob marks
 - fast-export: avoid importing blob marks

 Replaced with the last re-roll posted to the list, queued with
 various fixup! commits to record suggested changes (most are
 trivial style fixes).


* pp/gitweb-config-underscore (2012-11-21) 1 commit
 - gitweb: make remote_heads config setting work

 The key "gitweb.remote_heads" is not legal git config; this maps it to
 "gitweb.remoteheads".


* jc/apply-trailing-blank-removal (2012-10-12) 1 commit
 - apply.c:update_pre_post_images(): the preimage can be truncated

 Fix to update_pre_post_images() that did not take into account the
 possibility that whitespace fix could shrink the preimage and
 change the number of lines in it.

 Will merge to 'next'.


* nd/pathspec-wildcard (2012-11-19) 4 commits
 - tree_entry_interesting: do basedir compare on wildcard patterns when possible
 - pathspec: apply "*.c" optimization from exclude
 - pathspec: do exact comparison on the leading non-wildcard part
 - pathspec: save the non-wildcard length part


* sg/complete-help-undup (2012-11-14) 1 commit
  (merged to 'next' on 2012-11-18 at eadd0f3)
 + completion: remove 'help' duplicate from porcelain commands

 Will merge to 'master' in the seventh batch.


* bc/do-not-recurse-in-die (2012-11-15) 1 commit
  (merged to 'next' on 2012-11-18 at 79d62a8)
 + usage.c: detect recursion in die routines and bail out immediately

 Will merge to 'master' in the seventh batch.


* mk/complete-tcsh (2012-11-16) 1 commit
  (merged to 'next' on 2012-11-19 at 8309029)
 + tcsh-completion re-using git-completion.bash

 Will merge to 'master' in the seventh batch.


* mm/status-push-pull-advise (2012-11-16) 1 commit
 - status: add advice on how to push/pull to tracking branch

 Will merge to 'next'.


* lt/diff-stat-show-0-lines (2012-10-17) 1 commit
  (merged to 'next' on 2012-11-19 at 0037290)
 + Fix "git diff --stat" for interesting - but empty - file changes

 We failed to mention a file without any content change but whose
 permission bit was modified, or (worse yet) a new file without any
 content in the "git diff --stat" output.

 Will merge to 'master' in the seventh batch.


* fc/zsh-completion (2012-11-19) 2 commits
 - completion: start moving to the new zsh completion
 - completion: add new zsh completion

 Replaced by shedding large changes to other independent topics.
 Any comments from zsh users?


* nd/wildmatch (2012-11-20) 14 commits
  (merged to 'next' on 2012-11-21 at 151288f)
 + test-wildmatch: avoid Windows path mangling
  (merged to 'next' on 2012-10-25 at 510e8df)
 + Support "**" wildcard in .gitignore and .gitattributes
 + wildmatch: make /**/ match zero or more directories
 + wildmatch: adjust "**" behavior
 + wildmatch: fix case-insensitive matching
 + wildmatch: remove static variable force_lower_case
 + wildmatch: make wildmatch's return value compatible with fnmatch
 + t3070: disable unreliable fnmatch tests
 + Integrate wildmatch to git
 + wildmatch: follow Git's coding convention
 + wildmatch: remove unnecessary functions
 + Import wildmatch from rsync
 + ctype: support iscntrl, ispunct, isxdigit and isprint
 + ctype: make sane_ctype[] const array

 Allows pathname patterns in .gitignore and .gitattributes files
 with double-asterisks "foo/**/bar" to match any number of directory
 hierarchies.

 I suspect that this needs to be plugged to pathspec matching code;
 otherwise "git log -- 'Docum*/**/*.txt'" would not show the log for
 commits that touch Documentation/git.txt, which would be confusing
 to the users.

 Will cook in 'next'.


* jh/update-ref-d-through-symref (2012-10-21) 2 commits
  (merged to 'next' on 2012-11-19 at 6bcca4c)
 + Fix failure to delete a packed ref through a symref
 + t1400-update-ref: Add test verifying bug with symrefs in delete_ref()

 "update-ref -d --deref SYM" to delete a ref through a symbolic ref
 that points to it did not remove it correctly.

 Will merge to 'master' in the seventh batch.


* fc/completion-test-simplification (2012-11-16) 6 commits
 - completion: simplify __gitcomp() test helper
 - completion: refactor __gitcomp related tests
 - completion: consolidate test_completion*() tests
 - completion: simplify tests using test_completion_long()
 - completion: standardize final space marker in tests
 - completion: add comment for test_completion()

 Clean up completion tests.  Use of conslidated helper may make
 instrumenting one particular test during debugging of the test
 itself, but I think that issue should be addressed in some other
 way (e.g. making sure individual tests in 9902 can be skipped).


* jk/pickaxe-textconv (2012-10-28) 2 commits
 - pickaxe: use textconv for -S counting
 - pickaxe: hoist empty needle check

 Use textconv filters when searching with "log -S".

 Will merge to 'next'.


* fc/remote-bzr (2012-11-08) 5 commits
  (merged to 'next' on 2012-11-18 at 86add07)
 + remote-bzr: update working tree
 + remote-bzr: add support for remote repositories
 + remote-bzr: add support for pushing
 + remote-bzr: add simple tests
 + Add new remote-bzr transport helper

 New remote helper for bzr.

 Will merge to 'master' in the seventh batch.


* fc/remote-hg (2012-11-12) 20 commits
  (merged to 'next' on 2012-11-18 at 4a4f2e4)
 + remote-hg: avoid bad refs
 + remote-hg: try the 'tip' if no checkout present
 + remote-hg: fix compatibility with older versions of hg
 + remote-hg: add missing config for basic tests
 + remote-hg: the author email can be null
 + remote-hg: add option to not track branches
 + remote-hg: add extra author test
 + remote-hg: add tests to compare with hg-git
 + remote-hg: add bidirectional tests
 + test-lib: avoid full path to store test results
 + remote-hg: add basic tests
 + remote-hg: fake bookmark when there's none
 + remote-hg: add compat for hg-git author fixes
 + remote-hg: add support for hg-git compat mode
 + remote-hg: match hg merge behavior
 + remote-hg: make sure the encoding is correct
 + remote-hg: add support to push URLs
 + remote-hg: add support for remote pushing
 + remote-hg: add support for pushing
 + Add new remote-hg transport helper

 New remote helper for hg.

 Will merge to 'master' in the seventh batch.


* cr/push-force-tag-update (2012-11-19) 5 commits
 - push: update remote tags only with force
 - push: flag updates that require force
 - push: keep track of "update" state separately
 - push: add advice for rejected tag reference
 - push: return reject reasons via a mask

 Require "-f" for push to update a tag, even if it is a fast-forward.


* rr/submodule-diff-config (2012-11-18) 4 commits
  (merged to 'next' on 2012-11-19 at 355319e)
 + submodule: display summary header in bold
 + diff: rename "set" variable
 + diff: introduce diff.submodule configuration variable
 + Documentation: move diff.wordRegex from config.txt to diff-config.txt

 Lets "git diff --submodule=log" become the default via configuration.

 Will merge to 'master' in the seventh batch.

^ permalink raw reply

* [PATCH 5/5] git-send-email: allow edit invalid email address
From: Krzysztof Mazur @ 2012-11-22 18:12 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Felipe Contreras, Andreas Schwab, Felipe Balbi,
	Tomi Valkeinen, Krzysztof Mazur
In-Reply-To: <1353607932-10436-1-git-send-email-krzysiek@podlesie.net>

In some cases the user may want to send email with "Cc:" line with
email address we cannot extract. Now we allow user to extract
such email address for us.

Signed-off-by: Krzysztof Mazur <krzysiek@podlesie.net>
---
 git-send-email.perl | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index d42dca2..9996735 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -851,10 +851,10 @@ sub extract_valid_address_or_die {
 
 sub validate_address {
 	my $address = shift;
-	if (!extract_valid_address($address)) {
+	while (!extract_valid_address($address)) {
 		print STDERR "error: unable to extract a valid address from: $address\n";
-		$_ = ask("What to do with this address? ([q]uit|[d]rop): ",
-			valid_re => qr/^(?:quit|q|drop|d)/i,
+		$_ = ask("What to do with this address? ([q]uit|[d]rop|[e]dit): ",
+			valid_re => qr/^(?:quit|q|drop|d|edit|e)/i,
 			default => 'q');
 		if (/^d/i) {
 			return undef;
@@ -862,6 +862,9 @@ sub validate_address {
 			cleanup_compose_files();
 			exit(0);
 		}
+		$address = ask("Who should the email be sent to (if any)? ",
+			default => "",
+			valid_re => qr/\@.*\./, confirm_only => 1);
 	}
 	return $address;
 }
-- 
1.8.0.393.gcc9701d

^ permalink raw reply related

* [PATCH] Fix bash completion when `egrep` is aliased to `egrep --color=always`
From: Adam Tkac @ 2012-11-22 15:41 UTC (permalink / raw)
  To: git

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

Hello all,

attached patch fixes bash completion when `egrep` is aliased to `egrep --color=always`.

Comments are welcomed.

Regards, Adam

-- 
Adam Tkac, Red Hat, Inc.

[-- Attachment #2: 0001-Fix-bash-completion-when-egrep-is-aliased-to-egrep-c.patch --]
[-- Type: text/plain, Size: 998 bytes --]

>From 2b62bd71af1158129492f74f0b77c9840a49507f Mon Sep 17 00:00:00 2001
From: Adam Tkac <atkac@redhat.com>
Date: Thu, 22 Nov 2012 16:34:58 +0100
Subject: [PATCH] Fix bash completion when `egrep` is aliased to `egrep
 --color=always`

Originally reported as https://bugzilla.redhat.com/show_bug.cgi?id=863780

Signed-off-by: Adam Tkac <atkac@redhat.com>
Signed-off-by: Holger Arnold <holgerar@gmail.com>
---
 contrib/completion/git-completion.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index bc0657a..47613f9 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -565,7 +565,7 @@ __git_complete_strategy ()
 __git_list_all_commands ()
 {
 	local i IFS=" "$'\n'
-	for i in $(git help -a|egrep '^  [a-zA-Z0-9]')
+	for i in $(git help -a|egrep --color=never '^  [a-zA-Z0-9]')
 	do
 		case $i in
 		*--*)             : helper pattern;;
-- 
1.8.0


^ permalink raw reply related

* Fwd: Remote hung up during `git fetch`
From: Yichao Yu @ 2012-11-22 18:39 UTC (permalink / raw)
  To: git
In-Reply-To: <CAMvDr+R__wWRwm2xNC3-v0T1RVu_rKdjKUJhb8cHwEXvuX9OMQ@mail.gmail.com>

Hi everyone,

I sent this email yesterday to the git mailing list but I cannot find
it in any archive so I decide to send it again.
Does anyone know what has happened to the mailing list? I haven't
receive any email from several kernel related busy mailing lists for
several hours....

Yichao Yu

---------- Forwarded message ----------
From: Yichao Yu <yyc1992@gmail.com>
Date: Wed, Nov 21, 2012 at 11:18 PM
Subject: Remote hung up during `git fetch`
To: git@vger.kernel.org


Hi everyone,

I want to build packages for snap shoot of different branches from
different remote git repositories in the same local directory (so that
I don't need to recompile everything everytime.) and I am using a
combination of `git clone/checkout/reset/fetch` to do that. However,
during git-fetch, the remote sometimes stop responding or simply reset
the connection. This happens occasionally at least for both ssh and
git protocol (not sure about http/https) on github, bitbucket and also
kernel.org so I think it is probably not due to a weird behavior of a
certain host. Does anyone know the reason or is there anything I have
done wrong? And is there a better way to set the local tree to a
certain branch at a certain url? THX

My git version is ArchLinux package 1.8.0-1. (timezone
America/New_York in case the time stamp somehow matters)

Here is a script that always triggers the issue (at least now) and
it's output. (No I am not trying to merge git and the kernel... These
are just random public repos on kernel.org that can trigger the issue.
Although I am pulling from two repos from different project here, the
same thing can also happen on other hosts when the two repos are
actually the same project)

Yichao Yu

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

#!/bin/bash

repo_name=git
# remote1='git://github.com/torvalds/linux.git'
remote1='git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git'
branch1='master'
# remote2='git://github.com/git/git.git'
remote2='git://git.kernel.org/pub/scm/git/git.git'
branch2='next'

git clone --depth 1 --single-branch --branch "$branch1" "$remote1" "$repo_name"
cd "$repo_name"
git fetch -vvv "$remote2" # "$branch2:$branch2"

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

Cloning into 'git'...
remote: Counting objects: 43215, done.
remote: Compressing objects: 100% (41422/41422), done.
remote: Total 43215 (delta 3079), reused 22032 (delta 1247)
Receiving objects: 100% (43215/43215), 119.06 MiB | 1.60 MiB/s, done.
Resolving deltas: 100% (3079/3079), done.
Checking out files: 100% (40905/40905), done.
fatal: destination path 'git' already exists and is not an empty directory.
Server supports multi_ack_detailed
Server supports side-band-64k
Server supports ofs-delta
want 2d242fb3fc19fc9ba046accdd9210be8b9913f64 (HEAD)
have ef6c5be658f6a70c1256fbd18e18ee0dc24c3386
have db9d8c60266a5010e905829e10cd722519e14777
done
fatal: The remote end hung up unexpectedly

^ permalink raw reply

* Re: Topics currently in the Stalled category
From: Nguyen Thai Ngoc Duy @ 2012-11-22 11:24 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vobirq0q2.fsf_-_@alter.siamese.dyndns.org>

On Wed, Nov 21, 2012 at 7:05 AM, Junio C Hamano <gitster@pobox.com> wrote:
> * nd/unify-appending-of-s-o-b (2012-11-15) 1 commit
>  - Unify appending signoff in format-patch, commit and sequencer
>
>  I am not sure if the logic to refrain from adding a sign-off based
>  on the existing run of sign-offs is done correctly in this change.

I'm not sure what's the right logic here. The original code in
format-patch has exactly one test just to verify that --signoff does
_something_. There's another signoff code in git-am.sh and we should
make sure at least the behavior is consistent. I guess I should start
over by writing new tests to record the current behavior, then fixes
and finally the unification. I think you can drop this for now.
-- 
Duy

^ permalink raw reply

* Re: [PATCH v5 06/15] remote-testgit: get rid of non-local functionality
From: Felipe Contreras @ 2012-11-21 23:44 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Johannes Schindelin, Max Horn, Jeff King, Sverre Rabbelier,
	Brandon Casey, Brandon Casey, Jonathan Nieder, Ilari Liusvaara,
	Pete Wyckoff, Ben Walton, Matthieu Moy, Julian Phillips
In-Reply-To: <7v624y3h0q.fsf@alter.siamese.dyndns.org>

On Wed, Nov 21, 2012 at 7:26 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> This only makes sense for the python remote helpers framework.
>
> A better explanation is sorely needed for this.  If the test were
> feeding python snippet to be sourced by python remote helper to be
> tested, the new remote-testgit.bash would not have any hope (nor
> need) to grok it, and "this only makes sense for python" makes
> perfect sense and clear enough, but that is not the case.
>
> If the justification were like this:
>
>     remote-testgit: remove non-local tests
>
>     The simplified remote-testgit does not talk with any remote
>     repository and incapable of running non-local tests.  Remove
>     them.
>
> I would understand it, and I wouldn't say it is a regression in the
> test not to test "non-local", as that is not essential aspect of
> these tests (we are only interested in testing the object/ref
> transfer over remote-helper interface and do not care what the
> "other side" really is).
>
> But I am not quite sure what you really mean by "non-local"
> functionality in the first place.  The original test weren't opening
> network port to emulate multi-host remote transfer, were it?

No, that's not it at all.

By local, I mean 'file:///home/user/foo', by remote, I mean
'http://user.org/foo'. How each of these URLs is handled is entirely
up to the remote helper.

bzr for example doesn't need any change at all, the same API works on
both cases. hg OTOH has different APIs, so the code needs a local
clone to do most operations. The python remote helper framework has
APIs to make it easier to implement the local clone functionality (for
the remote helpers that need it).

This has absolutely nothing to do with with remote helpers, this is
100% a python remote helper feature. So we don't need those tests
here, they belong in git-remote-testpy.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply

* Re: Remote hung up during `git fetch`
From: Shawn Pearce @ 2012-11-22 19:01 UTC (permalink / raw)
  To: Yichao Yu; +Cc: git
In-Reply-To: <CAMvDr+QuMpfdTdkOMOiYyEnHvQjia2cDCt3sx2rQwwLcJiCVmw@mail.gmail.com>

On Thu, Nov 22, 2012 at 10:39 AM, Yichao Yu <yyc1992@gmail.com> wrote:
> I sent this email yesterday to the git mailing list but I cannot find
> it in any archive so I decide to send it again.

If it was HTML formatted it would have been silently dropped by the list.

> Does anyone know what has happened to the mailing list? I haven't
> receive any email from several kernel related busy mailing lists for
> several hours....

US holiday today? The list traffic tends to be down during holidays.

> I want to build packages for snap shoot of different branches from
> different remote git repositories in the same local directory (so that
> I don't need to recompile everything everytime.) and I am using a
> combination of `git clone/checkout/reset/fetch` to do that. However,
> during git-fetch, the remote sometimes stop responding or simply reset
> the connection. This happens occasionally at least for both ssh and
> git protocol (not sure about http/https) on github, bitbucket and also
> kernel.org so I think it is probably not due to a weird behavior of a
> certain host. Does anyone know the reason or is there anything I have
> done wrong? And is there a better way to set the local tree to a
> certain branch at a certain url? THX

If the remote server is really busy it might be OOM'ing the server
process which would disconnect the client. Or maybe its your ISP
sending a rogue RST packet to kill the connection because they don't
like traffic that leaves their network and costs them money on a
peering agreement. Or... ?

> #!/bin/bash
>
> repo_name=git
> # remote1='git://github.com/torvalds/linux.git'
> remote1='git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git'
> branch1='master'
> # remote2='git://github.com/git/git.git'
> remote2='git://git.kernel.org/pub/scm/git/git.git'
> branch2='next'
>
> git clone --depth 1 --single-branch --branch "$branch1" "$remote1" "$repo_name"
> cd "$repo_name"
> git fetch -vvv "$remote2" # "$branch2:$branch2"
>
> -----------------------------------------------
>
> Cloning into 'git'...
> remote: Counting objects: 43215, done.
> remote: Compressing objects: 100% (41422/41422), done.
> remote: Total 43215 (delta 3079), reused 22032 (delta 1247)
> Receiving objects: 100% (43215/43215), 119.06 MiB | 1.60 MiB/s, done.
> Resolving deltas: 100% (3079/3079), done.
> Checking out files: 100% (40905/40905), done.
> fatal: destination path 'git' already exists and is not an empty directory.

Why does this error come up? It looks like git already exists locally.
Git should have aborted much earlier in clone when the directory
exists.

> Server supports multi_ack_detailed
> Server supports side-band-64k
> Server supports ofs-delta
> want 2d242fb3fc19fc9ba046accdd9210be8b9913f64 (HEAD)
> have ef6c5be658f6a70c1256fbd18e18ee0dc24c3386
> have db9d8c60266a5010e905829e10cd722519e14777
> done
> fatal: The remote end hung up unexpectedly

This looks like its from the fetch command. Since the server doesn't
report any errors to the client, its hard to say why the server just
gave up right there. I wonder if this is related to the fact that you
did a shallow clone initially. The shallow clone may have confused the
server when fetch ran because it only sent 2 have lines and gave up.

Try exporting GIT_TRACE_PACKET=1 and seeing if you can get more
detailed information from the protocol on the client side.

FYI, https://kernel.googlesource.com/ mirrors git://git.kernel.org/ so
you can also try pulling from that server (e.g.
https://kernel.googlesource.com/pub/scm/git/git.git).

^ permalink raw reply

* [PATCH] diff: Fixes shortstat number of files
From: Antoine Pelisse @ 2012-11-21 21:26 UTC (permalink / raw)
  To: git; +Cc: Antoine Pelisse

There is a discrepancy between the last line of `git diff --stat`
and `git diff --shortstat` in case of a merge.
The unmerged files are actually counted twice, thus doubling the
value of "file changed".

In fact, while stat decrements number of files when seeing an unmerged
file, shortstat doesn't.

Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
---
 diff.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/diff.c b/diff.c
index e89a201..5c6bcbd 100644
--- a/diff.c
+++ b/diff.c
@@ -1704,9 +1704,8 @@ static void show_shortstats(struct diffstat_t *data, struct diff_options *option
 		int added = data->files[i]->added;
 		int deleted= data->files[i]->deleted;
 
-		if (data->files[i]->is_unmerged)
-			continue;
-		if (!data->files[i]->is_renamed && (added + deleted == 0)) {
+		if (data->files[i]->is_unmerged ||
+		  (!data->files[i]->is_renamed && (added + deleted == 0))) {
 			total_files--;
 		} else if (!data->files[i]->is_binary) { /* don't count bytes */
 			adds += added;
-- 
1.7.9.5

^ permalink raw reply related

* Re: [PATCH v5 08/15] remote-testgit: cleanup tests
From: Felipe Contreras @ 2012-11-22  0:55 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Johannes Schindelin, Max Horn, Jeff King, Sverre Rabbelier,
	Brandon Casey, Brandon Casey, Jonathan Nieder, Ilari Liusvaara,
	Pete Wyckoff, Ben Walton, Matthieu Moy, Julian Phillips
In-Reply-To: <7vzk2a22g8.fsf@alter.siamese.dyndns.org>

On Wed, Nov 21, 2012 at 7:28 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> We don't need a bare 'server' and an intermediary 'public'. The repos
>> can talk to each other directly; that's what we want to exercise.
>
> The previous patch to remove the test (the one that covered a case
> where a bug was fixed in an older git-remote-testpy and tried to
> catch the bug when it resurfaced) made sense even with its
> ultra-short justification "irrelevant".
>
> But I am not sure if this one is so cut-and-dried.  The repos can
> talk to each other directly, but at the same time the tests were
> exercising interactions between bare and non-bare repositories,
> weren't they?  Talking to each other may be one of the things we
> want to exercise, but that does not necessarily be the only thing.
>
> If it were explained like this (note that I am *guessing* what you
> meant to achieve by this patch, which may be wrong, in which case
> the log message needs further clarification):
>
>         Going through an intermediary 'public' may have exercised
>         interactions among combinations of bare and non-bare
>         repositories a bit more, but that is not an issue specific
>         to the remote-helper transfer that we want to be testing in
>         this script.  Simplify the tests to let two repositories
>         talk directly with each other.

Right. I don't think bare vs. non-bare has anything to do with it; the
intermediary repository was there to have 3 types of repos interacting
with each other local testpy, remote testpy, local git. But this
doesn't exercise anything from transport helper.

Cheers.

-- 
Felipe Contreras

^ 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