Git development
 help / color / mirror / Atom feed
* Re: [PATCH v4 00/13] New remote-hg helper
From: Jeff King @ 2012-10-29 21:26 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: git, Junio C Hamano, Sverre Rabbelier, Johannes Schindelin,
	Ilari Liusvaara, Daniel Barkalow, Michael J Gruber
In-Reply-To: <CAMP44s0RVe6i4DpNmaV_n7_5KO_aq2WxCPVafjsTukExRSR5Jw@mail.gmail.com>

On Mon, Oct 29, 2012 at 03:56:39PM +0100, Felipe Contreras wrote:

> >> I've ported the tests from hg-git and made sure that the output from remote-hg
> >> matches the output of hg-git. With these extensive tests I would consider this
> >> one ready for wide use. Not only do the tests pass, I've compared the generated
> >> repos of a few projects, and the SHA-1's are exactly the same :)
> >
> > Sounds cool. Unfortunately, the test script hangs for me, after starting
> > up xxdiff (!).
> >
> > pstree reveals that it is "hg" that starts it, but I didn't investigate
> > beyond that.
> 
> Yeah, the test script is not ready for merging, it needs to check for
> python, hg, and hg-git.
> 
> Do you have hg-git installed?

No. But it's important that it fail gracefully; I can't even take it in
pu if I can't run the test suite in a sane way.

I may try to figure it out later myself, but it's not a super high
priority for me.

-Peff

^ permalink raw reply

* gitk: crash when pressing Shift-F5 while still loading a diff
From: Peter Oberndorfer @ 2012-10-29 21:34 UTC (permalink / raw)
  To: git

Hi,

i recently tested gitk a bit more myself and on some people in the office.
And i noticed when I repeatedly press Shift-F5 gitk
crashes/displays a message box [1].
my current version: c83ae78864493a30ed5b544b4910a384371a5eaf

This also happens a lot when gitk is still loading a big diff
and using View->Some view to change the current view.
(I have made a convenience shortcut "--all" to show all branches without having
the windows people going to the commandline)

Greetings Peter

crash1:
can't read "treediffs(7affa01c56f17778e92b57fd4fc8d749a672d930)": no such variable
can't read "treediffs(7affa01c56f17778e92b57fd4fc8d749a672d930)": no such variable
    while executing
"lsearch -exact $treediffs($ids) $fname"
    (procedure "makediffhdr" line 7)
    invoked from within
"makediffhdr $fname $ids"
    (procedure "getblobdiffline" line 61)
    invoked from within
"getblobdiffline file8 7affa01c56f17778e92b57fd4fc8d749a672d930"
    ("eval" body line 1)
    invoked from within
"eval $script"
    (procedure "dorunq" line 11)
    invoked from within
"dorunq"
    ("after" script)

crash2:
can't read "varcid(0,7affa01c56f17778e92b57fd4fc8d749a672d930)": no such element in array
can't read "varcid(0,7affa01c56f17778e92b57fd4fc8d749a672d930)": no such element in array
    while executing
"set a $varcid($v,$p)"
    (procedure "insertfakerow" line 6)
    invoked from within
"insertfakerow $nullid $p"
    (procedure "readdifffiles" line 29)
    invoked from within
"readdifffiles file7 7 71"
    ("eval" body line 1)
    invoked from within
"eval $script"
    (procedure "dorunq" line 11)
    invoked from within
"dorunq"
    ("after" script)


error3 (output on stderr):
oops rowofcommit no arc for 7affa01c


7affa01c is my top commit on the current branch.

Greetings Peter

^ permalink raw reply

* Re: git push tags
From: Jeff King @ 2012-10-29 21:35 UTC (permalink / raw)
  To: Kacper Kornet
  Cc: Drew Northup, Michael Haggerty, Angelo Borsotti, Philip Oakley,
	Chris Rorvick, Johannes Sixt, git
In-Reply-To: <20121029172330.GC8359@camk.edu.pl>

On Mon, Oct 29, 2012 at 06:23:30PM +0100, Kacper Kornet wrote:

> > That patch just blocks non-forced updates to refs/tags/. I think a saner
> > start would be to disallow updating non-commit objects without a force.
> > We already do so for blobs and trees because they are not (and cannot
> > be) fast forwards. The fact that annotated tags are checked for
> > fast-forward seems to me to be a case of "it happens to work that way"
> > and not anything planned. Since such a push drops the reference to the
> > old version of the tag, it should probably require a force.
> 
> I'm not sure. Looking at 37fde87 ("Fix send-pack for non-commitish
> tags.") I have an impression that Junio allowed for fast-forward pushes
> of annotated tags on purpose.

Hmm. You're right, though I'm not sure I agree with the reasoning of
that commit. I'd certainly like to get Junio's input on the subject.

> > Then on top of that we can talk about what lightweight tags should do.
> > I'm not sure. Following the regular fast-forward rules makes some sense
> > to me, because you are never losing objects. But there may be
> > complications with updating tags in general because of fetch's rules,
> > and we would be better off preventing people from accidentally doing so.
> > I think a careful review of fetch's tag rules would be in order before
> > making any decision there.
> 
> The problem with the current behaviour is, that one can never be 100% sure
> that his push will not overwrite someone else tag.

Yes, although you do know that you are not throwing away history if you
do (because it must be a fast forward). Whereas if you have to use "-f"
to update a tag, then you have turned off all safety checks. So it is an
improvement for one case (creating a tag), but a regression for another
(updating an existing tag). I agree that the latter is probably less
common, but how much? If virtually nobody is doing it because git-fetch
makes the fetching side too difficult, then the regression is probably
not a big deal.

-Peff

^ permalink raw reply

* Re: [PATCH v4 00/13] New remote-hg helper
From: Felipe Contreras @ 2012-10-29 21:47 UTC (permalink / raw)
  To: Jeff King
  Cc: git, Junio C Hamano, Sverre Rabbelier, Johannes Schindelin,
	Ilari Liusvaara, Daniel Barkalow, Michael J Gruber
In-Reply-To: <20121029212643.GA20513@sigill.intra.peff.net>

On Mon, Oct 29, 2012 at 10:26 PM, Jeff King <peff@peff.net> wrote:
> On Mon, Oct 29, 2012 at 03:56:39PM +0100, Felipe Contreras wrote:
>
>> >> I've ported the tests from hg-git and made sure that the output from remote-hg
>> >> matches the output of hg-git. With these extensive tests I would consider this
>> >> one ready for wide use. Not only do the tests pass, I've compared the generated
>> >> repos of a few projects, and the SHA-1's are exactly the same :)
>> >
>> > Sounds cool. Unfortunately, the test script hangs for me, after starting
>> > up xxdiff (!).
>> >
>> > pstree reveals that it is "hg" that starts it, but I didn't investigate
>> > beyond that.
>>
>> Yeah, the test script is not ready for merging, it needs to check for
>> python, hg, and hg-git.
>>
>> Do you have hg-git installed?
>
> No. But it's important that it fail gracefully; I can't even take it in
> pu if I can't run the test suite in a sane way.

The contrib part is fine for 'pu'. The tests aren't even meant to
exercise stuff in 'contrib', right? There might be some exceptions,
but either way, there's plenty of stuff in 'contrib' without any
tests. The tests I'm providing are simply a little sugar.

-- 
Felipe Contreras

^ permalink raw reply

* Re: merge --no-commit not able to report stats more verbosely?
From: Jeff King @ 2012-10-29 21:51 UTC (permalink / raw)
  To: Phil Hord; +Cc: Scott R. Godin, git
In-Reply-To: <CABURp0oX5aT=yEbYeDXoOPWj_aRscVBY327_E6uyrrp5TPvcxA@mail.gmail.com>

On Mon, Oct 29, 2012 at 02:12:32PM -0400, Phil Hord wrote:

> >> (develop)>$ git merge widget_twitter
> >> Merge made by the 'recursive' strategy.
> >>  .../code/community/Dnd/Magentweet/Model/User.php   |    3 ++-
> >>  1 files changed, 2 insertions(+), 1 deletions(-)
> >
> > Whereas here you do, and you get a diffstat.
> >
> > When you are in the middle of an uncompleted merge and want to know what
> > is happening, you should look at the index using "git status" (to get an
> > overview of what is ready to be committed and what is unmerged), "git
> > diff --cached" (to see what was automatically merged and is ready for
> > commit), and "git diff" (to see conflicted entries that still need to be
> > resolved).
> 
> I think he is looking for this bit
>     "Merge made by the 'recursive' strategy."
> 
> But he is seeing this instead:
>     "Automatic merge went well; stopped before committing as requested"
> 
> Should the "what happened" output be silenced on --no-commit?

Ah. Yeah, we should not print "Merge made by the..." because we did not
make a merge (and that message is part of the process to update the
ref). But in verbose mode, we could probably say more about the on-going
process (like which strategies we are trying). Patches welcome.

-Peff

^ permalink raw reply

* Re: [PATCH v4 00/13] New remote-hg helper
From: Jeff King @ 2012-10-29 21:56 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: git, Junio C Hamano, Sverre Rabbelier, Johannes Schindelin,
	Ilari Liusvaara, Daniel Barkalow, Michael J Gruber
In-Reply-To: <CAMP44s3L0ycSQFU9s157V7e-GryUdojtQ3Vk_-d2wtPf9NFtbg@mail.gmail.com>

On Mon, Oct 29, 2012 at 10:47:04PM +0100, Felipe Contreras wrote:

> >> Yeah, the test script is not ready for merging, it needs to check for
> >> python, hg, and hg-git.
> >>
> >> Do you have hg-git installed?
> >
> > No. But it's important that it fail gracefully; I can't even take it in
> > pu if I can't run the test suite in a sane way.
> 
> The contrib part is fine for 'pu'. The tests aren't even meant to
> exercise stuff in 'contrib', right? There might be some exceptions,
> but either way, there's plenty of stuff in 'contrib' without any
> tests. The tests I'm providing are simply a little sugar.

Yeah, contrib is a bit of a wildcard. Most things do not have tests.
Completion tests run as part of the main test suite (which to me means
that completion should arguably be promoted out of contrib). Subtree
carries its own tests that build on the test suite, but do not run all
the time.

If remote-hg is going to live in contrib, it probably makes sense to
have its tests live there, too, like subtree. It means less test
exposure, but the robustness of the tests does not have to be as high.
You could also have no tests, but since you have them, it seems silly
not to include them. People know that items in contrib/ may not be as
mature as the rest of git.

-Peff

^ permalink raw reply

* Re: [PATCHv2] git-status: show short sequencer state
From: Jeff King @ 2012-10-29 21:41 UTC (permalink / raw)
  To: Phil Hord
  Cc: Phil Hord, git, Junio C Hamano, konglu, Matthieu Moy, Kong Lucien,
	Duperray Valentin, Jonas Franck, Nguy Thomas,
	Nguyen Huynh Khoi Nguyen
In-Reply-To: <CABURp0o7b5aZV6jNM=DSweh-8zVgGppxVsXisAcoNk7TxHrdgQ@mail.gmail.com>

On Mon, Oct 29, 2012 at 02:05:14PM -0400, Phil Hord wrote:

> I'm currently splitting this out into a series and reconsidering some
> of it along the way.  I need some guidance.
> 
> I want to support these two modes:
> 
>   A.  'git status --short' with sequence tokens added:
>        ## conflicted
>        ## merge
>        ?? untracked-workdir-file
>        etc.
> 
>   B.  Same as (A) but without workdir status:
>        ## conflicted
>        ## merge
> 
> The user who wants 'A' would initiate it like this:
>     git status --sequencer
>   or
>     git status -S
> 
> How do I spell the options for 'B'?  I have come up with these three
> possibilities:
>     git --sequencer-only   # Another switch
>     git --sequencer=only   # An OPTARG parser
>     git -S -S           # like git-diff -C -C, an OPT_COUNTUP

Might it be easier to spell 'A' as:

  git status --short -S

and B as:

  git status -S

this is sort of like how "-b" works (except you cannot currently ask for
it separately, but arguably you could). If we have a proliferation of
such options, then we might need config to help turn them on all the
time (I'd guess people are probably already using aliases to do this).

-Peff

^ permalink raw reply

* Re: Re: [Patch 1/1] Wire html for all files in ./technical and ./howto in Makefile
From: Jeff King @ 2012-10-29 21:57 UTC (permalink / raw)
  To: Thomas Ackermann; +Cc: git, gitster
In-Reply-To: <884264929.71955.1351535627736.JavaMail.ngmail@webmail09.arcor-online.net>

On Mon, Oct 29, 2012 at 07:33:47PM +0100, Thomas Ackermann wrote:

> This patch addresses Junios comment in WC:
> "Misapplication of a patch fixed; the ones near the tip needs to
>  update the links to point at the html files, though."
> 
> See older mail in this thread:
> [...]
> That means that for the patch [6/8], which adds content-type to the
> text files, to be complete, it needs to update Makefile to produce
> html files from them.
> [...]
> So IMHO no open issues with this patch.

OK, that explains the situation. Thanks, I'll merge it to master in the
next iteration.

-Peff

^ permalink raw reply

* Re: [PATCH] Document git-svn fetch --log-window-size parameter
From: Jeff King @ 2012-10-29 22:02 UTC (permalink / raw)
  To: Eric Wong; +Cc: Gunnlaugur Þór Briem, git
In-Reply-To: <20121029201847.GA22021@dcvr.yhbt.net>

On Mon, Oct 29, 2012 at 08:18:47PM +0000, Eric Wong wrote:

> Jeff King <peff@peff.net> wrote:
> > On Fri, Oct 26, 2012 at 09:46:02AM +0000, Eric Wong wrote:
> > > Overly large values also lead to excessive memory usage.  I may have
> > > only had 256M in my dev machine at the time I added this parameter:
> > 
> > That's probably worth mentioning. Gunnlaugur, any objection to me
> > amending your commit with:
> 
> Thanks both, I've amended and S-o-b on my end.  Shall I add:
> Signed-off-by: Jeff King <peff@peff.net>

Since it was such a small topic, I just went ahead and queued bc22b27 in
my repo. If that's not OK, I can revert it from 'next' and wait to get
it by pulling from you.

-Peff

^ permalink raw reply

* Re: [PATCH v4 00/13] New remote-hg helper
From: Felipe Contreras @ 2012-10-29 22:02 UTC (permalink / raw)
  To: Jeff King
  Cc: git, Junio C Hamano, Sverre Rabbelier, Johannes Schindelin,
	Ilari Liusvaara, Daniel Barkalow, Michael J Gruber
In-Reply-To: <20121029215631.GF20513@sigill.intra.peff.net>

On Mon, Oct 29, 2012 at 10:56 PM, Jeff King <peff@peff.net> wrote:
> On Mon, Oct 29, 2012 at 10:47:04PM +0100, Felipe Contreras wrote:
>
>> >> Yeah, the test script is not ready for merging, it needs to check for
>> >> python, hg, and hg-git.
>> >>
>> >> Do you have hg-git installed?
>> >
>> > No. But it's important that it fail gracefully; I can't even take it in
>> > pu if I can't run the test suite in a sane way.
>>
>> The contrib part is fine for 'pu'. The tests aren't even meant to
>> exercise stuff in 'contrib', right? There might be some exceptions,
>> but either way, there's plenty of stuff in 'contrib' without any
>> tests. The tests I'm providing are simply a little sugar.
>
> Yeah, contrib is a bit of a wildcard. Most things do not have tests.
> Completion tests run as part of the main test suite (which to me means
> that completion should arguably be promoted out of contrib).

I agree, I didn't think of that when I wrote the completion tests, but
now it seems appropriate, specially since there's discussion about
moving the prompt out of contrib.

> If remote-hg is going to live in contrib, it probably makes sense to
> have its tests live there, too, like subtree.

Probably, I'll check that option.

But eventually I think it should be installed by default, unless
somebody can come up for a reason not to. For now contrib might be OK.

> It means less test
> exposure, but the robustness of the tests does not have to be as high.
> You could also have no tests, but since you have them, it seems silly
> not to include them. People know that items in contrib/ may not be as
> mature as the rest of git.

Yeah, it's only a matter of figuring out how to run them.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply

* Re: [PATCH v4 00/13] New remote-hg helper
From: Jeff King @ 2012-10-29 22:06 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: git, Junio C Hamano, Sverre Rabbelier, Johannes Schindelin,
	Ilari Liusvaara, Daniel Barkalow, Michael J Gruber
In-Reply-To: <CAMP44s1SLpNpbjRXF6QHrOTO=_1=wjPo1_kV3jZV-HXOYXPbnQ@mail.gmail.com>

On Mon, Oct 29, 2012 at 11:02:31PM +0100, Felipe Contreras wrote:

> > If remote-hg is going to live in contrib, it probably makes sense to
> > have its tests live there, too, like subtree.
> 
> Probably, I'll check that option.
> 
> But eventually I think it should be installed by default, unless
> somebody can come up for a reason not to. For now contrib might be OK.

I would one day like to have it as part of the main distribution, too,
but it would be nice to prove its worth in the field for a while first.
I especially would like to find out how it compares in practice with the
work that is in msysgit.

> > It means less test exposure, but the robustness of the tests does
> > not have to be as high.  You could also have no tests, but since you
> > have them, it seems silly not to include them. People know that
> > items in contrib/ may not be as mature as the rest of git.
> 
> Yeah, it's only a matter of figuring out how to run them.

Subtree seems to copy substantial parts of t/Makefile, but I suspect you
could get away with just using an "include". I'd also be OK with just
including a test script that pulls in test-lib.sh, and letting people
run it manually (the Makefile infrastructure is really about running a
lot of tests, but if there's only one script, it's not so hard).

-Peff

^ permalink raw reply

* Re: [PATCH v2] git-submodule add: Add -r/--record option.
From: Jeff King @ 2012-10-29 21:36 UTC (permalink / raw)
  To: Phil Hord; +Cc: W. Trevor King, Shawn Pearce, Jens Lehmann, Git, Nahor
In-Reply-To: <CABURp0pFLi+2A+9wi-ZamiRze2u6z+6oyoCsNpWOLq_cq2L1rQ@mail.gmail.com>

On Mon, Oct 29, 2012 at 01:38:28PM -0400, Phil Hord wrote:

> > I am not sure it is sufficient as-is, though. It does not seem to ever
> > clear variables, only set them, which means that values could leak
> > across iterations of the loop,  [...] E.g., when
> > the first submodule has submodule.*.foo set but the second one does not,
> > you will still end up with $submodule_foo set when you process the
> > second one.
> 
> Good point.  That should not happen.
> 
> > or down to recursive calls.
> 
> Frankly, I consider that to be a feature.  However, I can see how it
> would be considered inconsistent in many ways, so it's probably best
> to squash it.  :-\

I think it would depend on the semantics of the option. Some options
would probably make sense to apply recursively, and some not.

Maybe instead of blindly converting config into the environment, it
should forward or clear specific known-meaning config.

-Peff

^ permalink raw reply

* Re: [PATCH v2] git-submodule add: Add -r/--record option.
From: Phil Hord @ 2012-10-29 22:21 UTC (permalink / raw)
  To: Jeff King; +Cc: W. Trevor King, Shawn Pearce, Jens Lehmann, Git, Nahor
In-Reply-To: <20121029213652.GC20513@sigill.intra.peff.net>

On Mon, Oct 29, 2012 at 5:36 PM, Jeff King <peff@peff.net> wrote:
> On Mon, Oct 29, 2012 at 01:38:28PM -0400, Phil Hord wrote:
>
>> > I am not sure it is sufficient as-is, though. It does not seem to ever
>> > clear variables, only set them, which means that values could leak
>> > across iterations of the loop,  [...] E.g., when
>> > the first submodule has submodule.*.foo set but the second one does not,
>> > you will still end up with $submodule_foo set when you process the
>> > second one.
>>
>> Good point.  That should not happen.
>>
>> > or down to recursive calls.
>>
>> Frankly, I consider that to be a feature.  However, I can see how it
>> would be considered inconsistent in many ways, so it's probably best
>> to squash it.  :-\
>
> I think it would depend on the semantics of the option. Some options
> would probably make sense to apply recursively, and some not.
>
> Maybe instead of blindly converting config into the environment, it
> should forward or clear specific known-meaning config.

Well, that's where we started.  I was aiming for the more generic
"never needs updating" direction.

P

^ permalink raw reply

* Re: [PATCHv2] git-status: show short sequencer state
From: Phil Hord @ 2012-10-29 22:26 UTC (permalink / raw)
  To: Jeff King
  Cc: Phil Hord, git, Junio C Hamano, konglu, Matthieu Moy, Kong Lucien,
	Duperray Valentin, Jonas Franck, Nguy Thomas,
	Nguyen Huynh Khoi Nguyen
In-Reply-To: <20121029214103.GD20513@sigill.intra.peff.net>

On Mon, Oct 29, 2012 at 5:41 PM, Jeff King <peff@peff.net> wrote:
> On Mon, Oct 29, 2012 at 02:05:14PM -0400, Phil Hord wrote:
>
>> I'm currently splitting this out into a series and reconsidering some
>> of it along the way.  I need some guidance.
>>
>> I want to support these two modes:
>>
>>   A.  'git status --short' with sequence tokens added:
>>        ## conflicted
>>        ## merge
>>        ?? untracked-workdir-file
>>        etc.
>>
>>   B.  Same as (A) but without workdir status:
>>        ## conflicted
>>        ## merge
>>
>> The user who wants 'A' would initiate it like this:
>>     git status --sequencer
>>   or
>>     git status -S
>>
>> How do I spell the options for 'B'?  I have come up with these three
>> possibilities:
>>     git --sequencer-only   # Another switch
>>     git --sequencer=only   # An OPTARG parser
>>     git -S -S           # like git-diff -C -C, an OPT_COUNTUP
>
> Might it be easier to spell 'A' as:
>
>   git status --short -S
>
> and B as:
>
>   git status -S
>
> this is sort of like how "-b" works (except you cannot currently ask for
> it separately, but arguably you could). If we have a proliferation of
> such options, then we might need config to help turn them on all the
> time (I'd guess people are probably already using aliases to do this).

I think I like this path.

I expect a common idiom to be 'git status -S --porcelain --null', and
both --porcelain and --null imply --short.  I think I can still do The
Right Thing, but the code is starting to spaghettify.  I'll take a
crack at it.

Thanks.

P

^ permalink raw reply

* Re: [PATCH v2] git-submodule add: Add -r/--record option.
From: Jeff King @ 2012-10-29 22:27 UTC (permalink / raw)
  To: Phil Hord; +Cc: W. Trevor King, Shawn Pearce, Jens Lehmann, Git, Nahor
In-Reply-To: <CABURp0otR2S1aOAWwnaFYFGRi_2cCBODbghck-BqUTw2B_ci3A@mail.gmail.com>

On Mon, Oct 29, 2012 at 06:21:08PM -0400, Phil Hord wrote:

> > Maybe instead of blindly converting config into the environment, it
> > should forward or clear specific known-meaning config.
> 
> Well, that's where we started.  I was aiming for the more generic
> "never needs updating" direction.

Then I think you are probably stuck taking the conservative approach of
not propagating recursively.

-Peff

^ permalink raw reply

* Re: crash on git diff-tree -Ganything <tree> for new files with textconv filter
From: Jeff King @ 2012-10-29 22:35 UTC (permalink / raw)
  To: Peter Oberndorfer; +Cc: git, Junio C Hamano
In-Reply-To: <508EE4E4.1080407@arcor.de>

On Mon, Oct 29, 2012 at 09:19:48PM +0100, Peter Oberndorfer wrote:

> I could reproduce with my 0x3000 bytes file on linux. The buffer is not
> read with a trailing null byte it is mapped by mmap in
> diff_populate_filespec...
> So i think we will not get away with expecting a trailing null :-/

Thanks for the reproduction recipe. I was testing with "git log", which
does not use the mmap optimization.

> For me the key to reproduce the problem was to have 2 commits.
> Adding the file in the root commit it did not work. [1]

You probably would need to pass "--root" for it to do the diff of the
initial commit.

The patch below fixes it, but it's terribly inefficient (it just detects
the situation and reallocates). It would be much better to disable the
reuse_worktree_file mmap when we populate the filespec, but it is too
late to pass an option; we may have already populated from an earlier
diffcore stage.

I guess if we teach the whole diff code that "-G" (and --pickaxe-regex)
is brittle, we can disable the optimization from the beginning based on
the diff options. I'll take a look.

diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
index b097fa7..88d1a8f 100644
--- a/diffcore-pickaxe.c
+++ b/diffcore-pickaxe.c
@@ -80,6 +80,29 @@ static void fill_one(struct diff_filespec *one,
 	if (DIFF_FILE_VALID(one)) {
 		*textconv = get_textconv(one);
 		mf->size = fill_textconv(*textconv, one, &mf->ptr);
+
+		/*
+		 * Horrible, horrible hack. If we are going to feed the result
+		 * to regexec, we must make sure it is NUL-terminated, but we
+		 * will not be if we have mmap'd a file and never munged it.
+		 *
+		 * We would do much better to turn off the reuse_worktree_file
+		 * optimization in the first place, which is the sole source of
+		 * these mmaps.
+		 */
+		if (one->should_munmap && !*textconv) { mf->ptr =
+			xmallocz(one->size); memcpy(mf->ptr, one->data,
+						    one->size);
+
+			/*
+			 * Attach the result to the filespec, which will
+			 * properly free it eventually.
+			 */
+			munmap(one->data, one->size);
+			one->should_munmap = 0;
+			one->data = mf->ptr;
+			one->should_free = 1;
+		}
 	} else {
 		memset(mf, 0, sizeof(*mf));
 	}

^ permalink raw reply related

* Re: [PATCH] Document git-svn fetch --log-window-size parameter
From: Eric Wong @ 2012-10-29 22:36 UTC (permalink / raw)
  To: Jeff King; +Cc: Gunnlaugur Þór Briem, git
In-Reply-To: <20121029220208.GH20513@sigill.intra.peff.net>

Jeff King <peff@peff.net> wrote:
> Since it was such a small topic, I just went ahead and queued bc22b27 in
> my repo. If that's not OK, I can revert it from 'next' and wait to get
> it by pulling from you.

That's fine, thanks!

^ permalink raw reply

* Re: crash on git diff-tree -Ganything <tree> for new files with textconv filter
From: Jeff King @ 2012-10-29 22:47 UTC (permalink / raw)
  To: Peter Oberndorfer; +Cc: git, Junio C Hamano
In-Reply-To: <20121029223521.GJ20513@sigill.intra.peff.net>

On Mon, Oct 29, 2012 at 06:35:21PM -0400, Jeff King wrote:

> The patch below fixes it, but it's terribly inefficient (it just detects
> the situation and reallocates). It would be much better to disable the
> reuse_worktree_file mmap when we populate the filespec, but it is too
> late to pass an option; we may have already populated from an earlier
> diffcore stage.
> 
> I guess if we teach the whole diff code that "-G" (and --pickaxe-regex)
> is brittle, we can disable the optimization from the beginning based on
> the diff options. I'll take a look.

Hmm. That is problematic for two reasons.

  1. The whole diff call chain will have to be modified to pass the
     options around, so they can make it down to the
     diff_populate_filespec level. Alternatively, we could do some kind
     of global hack, which is ugly but would work OK in practice.

  2. Reusing a working tree file is only half of the reason a filespec
     might be mmap'd. It might also be because we are literally diffing
     the working tree. "-G" was meant to be used to limit log traversal,
     but it also works to reduce the diff output for something like "git
     diff HEAD^".

I really wish there were an alternate regexec interface we could use
that took a pointer/size pair. Bleh.

-Peff

^ permalink raw reply

* [PATCHv2 0/3] git-status short sequencer state info
From: Phil Hord @ 2012-10-29 23:31 UTC (permalink / raw)
  To: git
  Cc: phil.hord, Jeff King, Junio C Hamano, konglu, Matthieu Moy,
	Kong Lucien, Duperray Valentin, Jonas Franck, Nguy Thomas
In-Reply-To: <CABURp0qBRvZQvnBbOraQ7c7DRg8v0TjnY+MOGYaWnWwjCqi23Q@mail.gmail.com>

V2 of this patch breaks it into a series, cleans up some gaffes,
and adds more flexible display options.  I still need to add some
tests.  Until I do that, maybe this should be considered a WIP.

 [PATCHv2 1/3] Refactor print_state into get_state
 [PATCHv2 2/3] wt-status: More state retrieval abstraction
 [PATCHv2 3/3] git-status: show short sequencer state

^ permalink raw reply

* [PATCHv2 1/3] Refactor print_state into get_state
From: Phil Hord @ 2012-10-29 23:31 UTC (permalink / raw)
  To: git
  Cc: phil.hord, Jeff King, Junio C Hamano, konglu, Matthieu Moy,
	Kong Lucien, Duperray Valentin, Jonas Franck, Nguy Thomas,
	Phil Hord
In-Reply-To: <1351553513-20385-1-git-send-email-hordp@cisco.com>

Recently git-status learned to display the state of the git
sequencer in long form to help the user remember an interrupted
command.  This information is useful to other callers who do
not want it printed in the same way.

Split the new print_state function into separate get_state and
print_state functions so others can get this state info and
share common code.

Signed-off-by: Phil Hord <hordp@cisco.com>
---
 wt-status.c | 31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index 2a9658b..760f52b 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -928,34 +928,41 @@ static void show_bisect_in_progress(struct wt_status *s,
 	wt_status_print_trailer(s);
 }
 
-static void wt_status_print_state(struct wt_status *s)
+static void wt_status_get_state(struct wt_status *s , struct wt_status_state *state)
 {
-	const char *state_color = color(WT_STATUS_HEADER, s);
-	struct wt_status_state state;
 	struct stat st;
 
-	memset(&state, 0, sizeof(state));
+	memset(state, 0, sizeof(*state));
 
+	/* Determine sequencer activity */
 	if (!stat(git_path("MERGE_HEAD"), &st)) {
-		state.merge_in_progress = 1;
+		state->merge_in_progress = 1;
 	} else if (!stat(git_path("rebase-apply"), &st)) {
 		if (!stat(git_path("rebase-apply/applying"), &st)) {
-			state.am_in_progress = 1;
+			state->am_in_progress = 1;
 			if (!stat(git_path("rebase-apply/patch"), &st) && !st.st_size)
-				state.am_empty_patch = 1;
+				state->am_empty_patch = 1;
 		} else {
-			state.rebase_in_progress = 1;
+			state->rebase_in_progress = 1;
 		}
 	} else if (!stat(git_path("rebase-merge"), &st)) {
 		if (!stat(git_path("rebase-merge/interactive"), &st))
-			state.rebase_interactive_in_progress = 1;
+			state->rebase_interactive_in_progress = 1;
 		else
-			state.rebase_in_progress = 1;
+			state->rebase_in_progress = 1;
 	} else if (!stat(git_path("CHERRY_PICK_HEAD"), &st)) {
-		state.cherry_pick_in_progress = 1;
+		state->cherry_pick_in_progress = 1;
 	}
 	if (!stat(git_path("BISECT_LOG"), &st))
-		state.bisect_in_progress = 1;
+		state->bisect_in_progress = 1;
+}
+
+static void wt_status_print_state(struct wt_status *s)
+{
+	const char *state_color = color(WT_STATUS_HEADER, s);
+	struct wt_status_state state;
+
+	wt_status_get_state(s, &state);
 
 	if (state.merge_in_progress)
 		show_merge_in_progress(s, &state, state_color);
-- 
1.8.0.3.gde9c7d5.dirty

^ permalink raw reply related

* [PATCHv2 2/3] wt-status: More state retrieval abstraction
From: Phil Hord @ 2012-10-29 23:31 UTC (permalink / raw)
  To: git
  Cc: phil.hord, Jeff King, Junio C Hamano, konglu, Matthieu Moy,
	Kong Lucien, Duperray Valentin, Jonas Franck, Nguy Thomas,
	Phil Hord
In-Reply-To: <1351553513-20385-1-git-send-email-hordp@cisco.com>

wt_status_print_state retrieves some sequencer state
information via wt_status_get_state, but other state
information it deduces on its own.  Replaces these
"local knowledge" deductions with wt_status variables
so we can share more common code in the future.
---
 wt-status.c | 16 +++++++++-------
 wt-status.h |  3 +++
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index 760f52b..a888120 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -781,7 +781,7 @@ static void show_merge_in_progress(struct wt_status *s,
 				struct wt_status_state *state,
 				const char *color)
 {
-	if (has_unmerged(s)) {
+	if (state->has_unmerged) {
 		status_printf_ln(s, color, _("You have unmerged paths."));
 		if (advice_status_hints)
 			status_printf_ln(s, color,
@@ -867,9 +867,7 @@ static void show_rebase_in_progress(struct wt_status *s,
 				struct wt_status_state *state,
 				const char *color)
 {
-	struct stat st;
-
-	if (has_unmerged(s)) {
+	if (state->has_unmerged) {
 		status_printf_ln(s, color, _("You are currently rebasing."));
 		if (advice_status_hints) {
 			status_printf_ln(s, color,
@@ -879,12 +877,12 @@ static void show_rebase_in_progress(struct wt_status *s,
 			status_printf_ln(s, color,
 				_("  (use \"git rebase --abort\" to check out the original branch)"));
 		}
-	} else if (state->rebase_in_progress || !stat(git_path("MERGE_MSG"), &st)) {
+	} else if (state->rebase_in_progress || state->commit_is_pending) {
 		status_printf_ln(s, color, _("You are currently rebasing."));
 		if (advice_status_hints)
 			status_printf_ln(s, color,
 				_("  (all conflicts fixed: run \"git rebase --continue\")"));
-	} else if (split_commit_in_progress(s)) {
+	} else if (state->split_in_progress) {
 		status_printf_ln(s, color, _("You are currently splitting a commit during a rebase."));
 		if (advice_status_hints)
 			status_printf_ln(s, color,
@@ -907,7 +905,7 @@ static void show_cherry_pick_in_progress(struct wt_status *s,
 {
 	status_printf_ln(s, color, _("You are currently cherry-picking."));
 	if (advice_status_hints) {
-		if (has_unmerged(s))
+		if (state->has_unmerged)
 			status_printf_ln(s, color,
 				_("  (fix conflicts and run \"git commit\")"));
 		else
@@ -955,6 +953,10 @@ static void wt_status_get_state(struct wt_status *s , struct wt_status_state *st
 	}
 	if (!stat(git_path("BISECT_LOG"), &st))
 		state->bisect_in_progress = 1;
+
+	state->has_unmerged = has_unmerged(s);
+	state->split_in_progress = split_commit_in_progress(s);
+	state->commit_is_pending = !stat(git_path("MERGE_MSG"), &st);
 }
 
 static void wt_status_print_state(struct wt_status *s)
diff --git a/wt-status.h b/wt-status.h
index 236b41f..0b866a2 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -79,6 +79,9 @@ struct wt_status_state {
 	int rebase_interactive_in_progress;
 	int cherry_pick_in_progress;
 	int bisect_in_progress;
+	int split_in_progress;
+	int has_unmerged;
+	int commit_is_pending;
 };
 
 void wt_status_prepare(struct wt_status *s);
-- 
1.8.0.3.gde9c7d5.dirty

^ permalink raw reply related

* [PATCHv2 3/3] git-status: show short sequencer state
From: Phil Hord @ 2012-10-29 23:31 UTC (permalink / raw)
  To: git
  Cc: phil.hord, Jeff King, Junio C Hamano, konglu, Matthieu Moy,
	Kong Lucien, Duperray Valentin, Jonas Franck, Nguy Thomas,
	Phil Hord
In-Reply-To: <1351553513-20385-1-git-send-email-hordp@cisco.com>

Teach git-status to report the sequencer state in short form
using a new --sequencer (-S) switch.  Output zero or more
simple state token strings indicating the deduced state of the
git sequencer.

Sequencer state info tokens are displayed in "short"
form.  'git status --short -S' will show the normal short
status including the sequencer tokens.  'git status -S'
without --short will show only the new sequencer information
tokens in whatever the requested format is.

State token strings which may be emitted and their meanings:
    merge              a merge is in progress
    am                 an am is in progress
    am-is-empty        the am patch is empty
    rebase             a rebase is in progress
    rebase-interactive an interactive rebase is in progress
    cherry-pick        a cherry-pick is in progress
    bisect             a bisect is in progress
    conflicted         there are unresolved conflicts
    commit-pending     a commit operation is waiting to be completed
    splitting          interactive rebase, commit is being split

I also considered adding these tokens, but I decided it was not
appropriate since these changes are not sequencer-related.  But
it is possible I am being too short-sighted or have chosen the
switch name poorly.
    changed-index  Changes exist in the index
    changed-files  Changes exist in the working directory
    untracked      New files exist in the working directory

Signed-off-by: Phil Hord <hordp@cisco.com>
---
 Documentation/git-status.txt | 16 ++++++++++++++++
 builtin/commit.c             | 14 ++++++++++++++
 wt-status.c                  | 35 +++++++++++++++++++++++++++++++++++
 wt-status.h                  |  7 +++++++
 4 files changed, 72 insertions(+)

diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index 67e5f53..7a699f1 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -38,6 +38,22 @@ OPTIONS
 	across git versions and regardless of user configuration. See
 	below for details.
 
+-S::
+--sequence::
+	Show the git sequencer status. This shows zero or more strings
+	describing the state of the git sequencer.
++
+	merge              a merge is in progress
+	am                 an am is in progress
+	am-is-empty        the am patch is empty
+	rebase             a rebase is in progress
+	rebase-interactive an interactive rebase is in progress
+	cherry-pick        a cherry-pick is in progress
+	bisect             a bisect is in progress
+	conflicted         there are unresolved conflicts
+	commit-pending     a commit operation is waiting to be completed
+	splitting          interactive rebase, commit is being split
+
 -u[<mode>]::
 --untracked-files[=<mode>]::
 	Show untracked files.
diff --git a/builtin/commit.c b/builtin/commit.c
index a17a5df..bf16cc6 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1060,6 +1060,11 @@ static int parse_and_validate_options(int argc, const char *argv[],
 
 	if (s->null_termination && status_format == STATUS_FORMAT_LONG)
 		status_format = STATUS_FORMAT_PORCELAIN;
+	if (s->show_sequencer && status_format != STATUS_FORMAT_SHORT) {
+		s->show_sequencer = SHOW_SEQUENCER_ONLY;
+		if (status_format == STATUS_FORMAT_LONG)
+			status_format = STATUS_FORMAT_SHORT;
+	}
 	if (status_format != STATUS_FORMAT_LONG)
 		dry_run = 1;
 
@@ -1156,6 +1161,8 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 			    N_("show status concisely"), STATUS_FORMAT_SHORT),
 		OPT_BOOLEAN('b', "branch", &s.show_branch,
 			    N_("show branch information")),
+		OPT_SET_INT('S', "sequencer", &s.show_sequencer,
+				N_("show sequencer state information"), SHOW_SEQUENCER_YES),
 		OPT_SET_INT(0, "porcelain", &status_format,
 			    N_("machine-readable output"),
 			    STATUS_FORMAT_PORCELAIN),
@@ -1188,6 +1195,11 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 
 	if (s.null_termination && status_format == STATUS_FORMAT_LONG)
 		status_format = STATUS_FORMAT_PORCELAIN;
+	if (s.show_sequencer && status_format != STATUS_FORMAT_SHORT) {
+		s.show_sequencer = SHOW_SEQUENCER_ONLY;
+		if (status_format == STATUS_FORMAT_LONG)
+			status_format = STATUS_FORMAT_SHORT;
+	}
 
 	handle_untracked_files_arg(&s);
 	if (show_ignored_in_status)
@@ -1384,6 +1396,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		OPT_SET_INT(0, "short", &status_format, N_("show status concisely"),
 			    STATUS_FORMAT_SHORT),
 		OPT_BOOLEAN(0, "branch", &s.show_branch, N_("show branch information")),
+		OPT_SET_INT(0, "sequencer", &s.show_sequencer,
+			    N_("show sequencer state information"), SHOW_SEQUENCER_YES),
 		OPT_SET_INT(0, "porcelain", &status_format,
 			    N_("machine-readable output"), STATUS_FORMAT_PORCELAIN),
 		OPT_BOOLEAN('z', "null", &s.null_termination,
diff --git a/wt-status.c b/wt-status.c
index a888120..aee9745 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1201,6 +1201,34 @@ static void wt_shortstatus_print_tracking(struct wt_status *s)
 	fputc(s->null_termination ? '\0' : '\n', s->fp);
 }
 
+static void wt_print_token(struct wt_status *s, const char * token_color, const char *token, int active)
+{
+	if (active) {
+		color_fprintf(s->fp, token_color, "## %s", token);
+		fputc(s->null_termination ? '\0' : '\n', s->fp);
+	}
+}
+
+static void wt_sequencer_print(struct wt_status *s)
+{
+	struct wt_status_state state;
+	const char *header_color = color(WT_STATUS_HEADER, s);
+
+	wt_status_get_state(s, &state);
+
+	wt_print_token(s, header_color, "merge", state.merge_in_progress);
+	wt_print_token(s, header_color, "am", state.am_in_progress);
+	wt_print_token(s, header_color, "rebase", state.rebase_in_progress);
+	wt_print_token(s, header_color, "rebase-interactive", state.rebase_interactive_in_progress);
+	wt_print_token(s, header_color, "cherry-pick", state.cherry_pick_in_progress);
+	wt_print_token(s, header_color, "bisect", state.bisect_in_progress);
+	wt_print_token(s, header_color, "am-empty", state.am_empty_patch);
+
+	wt_print_token(s, header_color, "conflicted", state.has_unmerged);
+	wt_print_token(s, header_color, "commit-pending", state.commit_is_pending);
+	wt_print_token(s, header_color, "splitting", state.split_in_progress);
+}
+
 void wt_shortstatus_print(struct wt_status *s)
 {
 	int i;
@@ -1208,6 +1236,13 @@ void wt_shortstatus_print(struct wt_status *s)
 	if (s->show_branch)
 		wt_shortstatus_print_tracking(s);
 
+	if (s->show_sequencer)
+	{
+		wt_sequencer_print(s);
+		if (s->show_sequencer == SHOW_SEQUENCER_ONLY)
+			return;
+	}
+
 	for (i = 0; i < s->change.nr; i++) {
 		struct wt_status_change_data *d;
 		struct string_list_item *it;
diff --git a/wt-status.h b/wt-status.h
index 0b866a2..df4b36d 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -24,6 +24,12 @@ enum untracked_status_type {
 	SHOW_ALL_UNTRACKED_FILES
 };
 
+enum sequencer_status_type {
+	SHOW_SEQUENCER_NO=0,
+	SHOW_SEQUENCER_YES,
+	SHOW_SEQUENCER_ONLY
+};
+
 /* from where does this commit originate */
 enum commit_whence {
 	FROM_COMMIT,     /* normal */
@@ -59,6 +65,7 @@ struct wt_status {
 	unsigned colopts;
 	int null_termination;
 	int show_branch;
+	enum sequencer_status_type show_sequencer;
 
 	/* These are computed during processing of the individual sections */
 	int commitable;
-- 
1.8.0.3.gde9c7d5.dirty

^ permalink raw reply related

* Empty name in fast-import stream.
From: Alexander Usov @ 2012-10-30  0:16 UTC (permalink / raw)
  To: git

Hi,

I'm working now on converting large repo to git, and stumbled upon a
case where fast-import from old git creates incorrect commit objects.
The case is following: when exporting the commit where the only thing
I know about committer is email, fast-import spec dictates following
format:

committer <foo@bar.com> 123456789 +100

with just 1 space between committer and opening brace. Apparently
git-fast-import in 1.7.1 would produce a commit object with just 1
space in there too. And git-fsck in newer versions of git would
complain about this commit object.

The workaround I found is to emit 2 spaces between committer and
opening bracket. Such stream is accepted by both 1.7.1 and 1.8 and
they both produce correct commit objects.

I'm wondering if it is worth changing git-fast-import manual page to
have following rule for committer (and author):

('author' SP <name>? SP LT <email> GT SP <when> LF)?
'committer' SP <name>? SP LT <email> GT SP <when> LF

This format results in the fast-import streams which are correctly
handled by both old and newer git versions.

-- 
Best regards,
  Alexander.

^ permalink raw reply

* [PATCH] test-lib: avoid full path to store test results
From: Felipe Contreras @ 2012-10-30  4:12 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jeff King, Jonathan Nieder,
	Ævar Arnfjörð Bjarmason, Johannes Sixt,
	Felipe Contreras

No reason to use the full path in case this is used externally.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 t/test-lib.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 514282c..5a3d665 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -389,7 +389,8 @@ test_done () {
 	then
 		test_results_dir="$TEST_OUTPUT_DIRECTORY/test-results"
 		mkdir -p "$test_results_dir"
-		test_results_path="$test_results_dir/${0%.sh}-$$.counts"
+		base=${0##*/}
+		test_results_path="$test_results_dir/${base%.sh}-$$.counts"
 
 		cat >>"$test_results_path" <<-EOF
 		total $test_count
-- 
1.8.0

^ permalink raw reply related

* Re: [PATCH] test-lib: avoid full path to store test results
From: Jeff King @ 2012-10-30  4:28 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: git, Junio C Hamano, Jonathan Nieder,
	Ævar Arnfjörð Bjarmason, Johannes Sixt
In-Reply-To: <1351570377-894-1-git-send-email-felipe.contreras@gmail.com>

On Tue, Oct 30, 2012 at 05:12:57AM +0100, Felipe Contreras wrote:

> No reason to use the full path in case this is used externally.

I think it is not just "no reason to", but it is actively wrong to use a
full path, as we do not take care to "mkdir -p" the intervening path
components.

However, this never comes up in practice, because all of the test
scripts assume you are running them from the test directory (i.e.,
they will fail otherwise because they will not find ./test-lib.sh).

Is this in support of putting remote-hg tests in contrib/? I had
expected you to just put

  export TEST_DIRECTORY="$(pwd)/../../../t"
  . "$TEST_DIRECTORY/test-lib.sh"

into the test script in contrib/remote-hg/t. I guess you are doing
something like:

  cd ../../../t && ../contrib/remote-hg/t/twhatever...

but the former seems much simpler to invoke (and if the goal is to get
your test-results in the right place, setting TEST_OUTPUT_DIRECTORY is
the best way to do that).

If this is part of the remote-hg series, I'd prefer to just see it as
part of the re-roll. It's much easier to evaluate it in context.

Or are you really just doing:

  cd git/t
  $PWD/t0000-basic.sh

I guess there is nothing wrong with that, though there is no reason not
to use "./" instead of $PWD.

-Peff

^ 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