* 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 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: [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: 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: [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 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 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] 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: 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: [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: [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: 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: 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: 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
* 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: [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
* Re: crash on git diff-tree -Ganything <tree> for new files with textconv filter
From: Peter Oberndorfer @ 2012-10-29 20:19 UTC (permalink / raw)
To: Jeff King; +Cc: git, Junio C Hamano
In-Reply-To: <20121029060524.GB4457@sigill.intra.peff.net>
On 2012-10-29 07:05, Jeff King wrote:
> On Sun, Oct 28, 2012 at 08:56:39PM +0100, Peter Oberndorfer wrote:
>
>>> The patch below should fix it. I added tests, but please try your
>>> real-world test case on it to double-check.
>> I tested your patch, but now it crashes for another reason :-)
> Well, that's progress, right? :)
Sure :-)
>
>> i have a file with exactly 12288(0x3000) bytes in the repository.
>> When the file is loaded, the data is placed luckily so the data end
>> falls at a page boundary.
>> Later diff_grep() calls regexec() which calls strlen() on the loaded buffer
>> and ends up reading beyond the actual data into the next page
>> which is not allocated and causes a pagefault.
>> Or it could possibly (randomly) match the regex on data that is not
>> actually part of a file...
> Yuck. For the most part, we treat blob content (and generally most
> object content) as a sized buffer. However, there are some spots which,
> either through laziness or because a code interface expects a string, we
> pass the value as a string. This works because the object-reading code
> puts an extra NUL at the end of our buffer to handle just such an
> instance. So we might prematurely end if the object contains embedded
> NULs, but we would never read past the end.
>
> The code to read the output of a textconv filter does not do this
> explicitly. I would think it would get it for free by virtue of reading
> into a strbuf, though. I'll try to investigate.
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 :-/
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]
Greetings Peter
> -Peff
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
[1]
kumbayo@home:~/src$ mkdir git_mmap_crash2
kumbayo@home:~/src$ cd git_mmap_crash2
kumbayo@home:~/src/git_mmap_crash2$ git init
kumbayo@home:~/src/git_mmap_crash2$ echo blah>blah
kumbayo@home:~/src/git_mmap_crash2$ git add blah
kumbayo@home:~/src/git_mmap_crash2$ git commit -m blah
[master (Basis-Version) 3458422] blah
diff_populate_filespec -> xmmap for blah size:0x5 returned: 0xb7206000
1 file changed, 1 insertion(+)
create mode 100644 blah
kumbayo@home:~/src/git_mmap_crash2$ perl -e 'print "-" x 0x3000 '> asdf.txt
kumbayo@home:~/src/git_mmap_crash2$ git add asdf.txt
kumbayo@home:~/src/git_mmap_crash2$ git commit -m crashy
[master 5cf2c5f] crashy
diff_populate_filespec -> xmmap for asdf.txt size:0x3000 returned:
0xb771e000
1 file changed, 1 insertion(+)
create mode 100644 asdf.txt
kumbayo@soybean:~/src/git_mmap_crash2$ valgrind git diff-tree -Ganything HEAD
==8388== Memcheck, a memory error detector
==8388== Copyright (C) 2002-2011, and GNU GPL'd, by Julian Seward et al.
==8388== Using Valgrind-3.7.0 and LibVEX; rerun with -h for copyright info
==8388== Command: git diff-tree -Ganything HEAD
==8388==
==8388== Conditional jump or move depends on uninitialised value(s)
==8388== at 0x405ADD8: inflateReset2 (in /lib/i386-linux-gnu/libz.so.1.2.3.4)
==8388== by 0xA0: ???
==8388==
==8388== Conditional jump or move depends on uninitialised value(s)
==8388== at 0x405ADD8: inflateReset2 (in /lib/i386-linux-gnu/libz.so.1.2.3.4)
==8388== by 0x7F: ???
==8388==
==8388== Conditional jump or move depends on uninitialised value(s)
==8388== at 0x405ADD8: inflateReset2 (in /lib/i386-linux-gnu/libz.so.1.2.3.4)
==8388== by 0x30: ???
==8388==
==8388== Conditional jump or move depends on uninitialised value(s)
==8388== at 0x405ADD8: inflateReset2 (in /lib/i386-linux-gnu/libz.so.1.2.3.4)
==8388== by 0x50: ???
==8388==
diffcore_pickaxe_grep
diff_populate_filespec -> xmmap for asdf.txt size:0x3000 returned: 0x4035000
==8388== Invalid read of size 1
==8388== at 0x402C683: __GI_strlen (in
/usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==8388== by 0x430581F: regexec@@GLIBC_2.3.4 (regexec.c:245)
==8388== by 0x814489D: diff_grep (diffcore-pickaxe.c:110)
==8388== by 0x8144B89: pickaxe.constprop.6 (diffcore-pickaxe.c:40)
==8388== by 0x8144DCD: diffcore_pickaxe_grep (diffcore-pickaxe.c:155)
==8388== by 0x80DCE64: diffcore_std (diff.c:4638)
==8388== by 0x80F0B20: log_tree_diff_flush (log-tree.c:696)
==8388== Address 0x4038000 is not stack'd, malloc'd or (recently) free'd
==8388==
==8388==
==8388== Process terminating with default action of signal 11 (SIGSEGV)
==8388== Access not within mapped region at address 0x4038000
==8388== at 0x402C683: __GI_strlen (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==8388== by 0x430581F: regexec@@GLIBC_2.3.4 (regexec.c:245)
==8388== by 0x814489D: diff_grep (diffcore-pickaxe.c:110)
==8388== by 0x8144B89: pickaxe.constprop.6 (diffcore-pickaxe.c:40)
==8388== by 0x8144DCD: diffcore_pickaxe_grep (diffcore-pickaxe.c:155)
==8388== by 0x80DCE64: diffcore_std (diff.c:4638)
==8388== by 0x80F0B20: log_tree_diff_flush (log-tree.c:696)
==8388== If you believe this happened as a result of a stack
==8388== overflow in your program's main thread (unlikely but
==8388== possible), you can try to increase the size of the
==8388== main thread stack using the --main-stacksize= flag.
==8388== The main thread stack size used in this run was 8388608.
==8388==
==8388== HEAP SUMMARY:
==8388== in use at exit: 86,229 bytes in 69 blocks
==8388== total heap usage: 193 allocs, 124 frees, 259,991 bytes allocated
==8388==
==8388== LEAK SUMMARY:
==8388== definitely lost: 65 bytes in 1 blocks
==8388== indirectly lost: 0 bytes in 0 blocks
==8388== possibly lost: 0 bytes in 0 blocks
==8388== still reachable: 86,164 bytes in 68 blocks
==8388== suppressed: 0 bytes in 0 blocks
==8388== Rerun with --leak-check=full to see details of leaked memory
==8388==
==8388== For counts of detected and suppressed errors, rerun with: -v
==8388== Use --track-origins=yes to see where uninitialised values come from
==8388== ERROR SUMMARY: 7 errors from 5 contexts (suppressed: 0 from 0)
^ permalink raw reply
* Re: [PATCH] Document git-svn fetch --log-window-size parameter
From: Eric Wong @ 2012-10-29 20:18 UTC (permalink / raw)
To: Jeff King; +Cc: Gunnlaugur Þór Briem, git
In-Reply-To: <20121026133250.GI1455@sigill.intra.peff.net>
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>
and push?
^ permalink raw reply
* Re: Removing unreachable objects in the presence of broken links?
From: Geert Uytterhoeven @ 2012-10-29 19:56 UTC (permalink / raw)
To: Andreas Schwab; +Cc: git
In-Reply-To: <m2r4oiffgy.fsf@igel.home>
Hi Andreas,
On Sun, Oct 28, 2012 at 10:34 PM, Andreas Schwab <schwab@linux-m68k.org> wrote:
> Geert Uytterhoeven <geert@linux-m68k.org> writes:
>
>> Is there a way to force removing unreachable objects in the presence of broken
>> links?
>
> Does it help to forcibly expire the reflogs?
You mean "git reflog expire --all --expire=0"?
After that the reflog is empty, but "git gc" still fails.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Aw: Re: [Patch 1/1] Wire html for all files in ./technical and ./howto in Makefile
From: Thomas Ackermann @ 2012-10-29 18:33 UTC (permalink / raw)
To: peff, th.acker66; +Cc: git, gitster
In-Reply-To: <20121025094205.GI8390@sigill.intra.peff.net>
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:
---
Thomas Ackermann <th.acker66@arcor.de> writes:
> BTW1: As only the changes in the doc files where cherry-picked, currently on pu howto-index.sh
> will create invalid links in howto-index.html because it scans all .txt-files in ./howto for
> 'Content-type: text/asciidoc' and if found, creates a reference to a html file. But these are not created
> for the new asciidoc files. So the changes in Documentation/Makefile which create html for the new
> files should be merged also.
Ah, I didn't notice that.
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.
Thanks.
---
So IMHO no open issues with this patch.
----- Original Nachricht ----
Von: Jeff King <peff@peff.net>
An: Thomas Ackermann <th.acker66@arcor.de>
Datum: 25.10.2012 11:42
Betreff: Re: [Patch 1/1] Wire html for all files in ./technical and ./howto
in Makefile
> On Tue, Oct 23, 2012 at 07:58:04PM +0200, Thomas Ackermann wrote:
>
> > - target "html" creates html for all files in Documentation/howto and
> > Documentation/technical
>
> Thanks.
>
---
Thomas
^ permalink raw reply
* Re: merge --no-commit not able to report stats more verbosely?
From: Phil Hord @ 2012-10-29 18:12 UTC (permalink / raw)
To: Jeff King; +Cc: Scott R. Godin, git
In-Reply-To: <20121028112150.GD11434@sigill.intra.peff.net>
On Sun, Oct 28, 2012 at 7:21 AM, Jeff King <peff@peff.net> wrote:
> On Mon, Oct 22, 2012 at 09:39:31AM -0400, Scott R. Godin wrote:
>
>> As you can see from the below, I can't seem to get it to give me more
>> verbose results of what's being merged (as in the actual merge below)
>> with --stat or -v .. is it supposed to do that?
>
> Yes. The diffstat is shown for the completed merge, but here:
>
>> (develop)>$ git merge --no-commit --stat -v widget_twitter
>> Automatic merge went well; stopped before committing as requested
>
> You do not complete the merge.
>
>> (develop|MERGING)>$ git merge --abort
>>
>> (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?
Phil
^ permalink raw reply
* Re: [PATCHv2] git-status: show short sequencer state
From: Phil Hord @ 2012-10-29 18:05 UTC (permalink / raw)
To: Phil Hord
Cc: Jeff King, git, Junio C Hamano, konglu, Matthieu Moy, Kong Lucien,
Duperray Valentin, Jonas Franck, Nguy Thomas,
Nguyen Huynh Khoi Nguyen
In-Reply-To: <5089633C.8030307@cisco.com>
On Thu, Oct 25, 2012 at 12:05 PM, Phil Hord <hordp@cisco.com> wrote:
>
> Jeff King wrote:
>> On Tue, Oct 23, 2012 at 04:02:54PM -0400, Phil Hord wrote:
>>
>>> 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.
>>>
>>> Introduce a common function to determine the current sequencer
>>> state so the regular status function and this short version can
>>> share common code.
>>>
>>> Add a substate to wt_status_state to track more detailed
>>> information about a state, such as "conflicted" or "resolved".
>>> Move the am_empty_patch flage out of wt_status_state and into
>> This patch ended up quite long. It might be a little easier to review
>> if it were broken into refactoring steps (I have not looked at it too
>> closely yet, but it seems like the three paragraphs above could each be
>> their own commit).
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
The first one is easy but weird, imho.
The second seems silly for just one type of option.
The last one is cheap to implement, but harder to explain in Documentation/
Any opinions?
Phil
^ permalink raw reply
* Re: [PATCH v2] git-submodule add: Add -r/--record option.
From: Phil Hord @ 2012-10-29 17:38 UTC (permalink / raw)
To: Jeff King; +Cc: W. Trevor King, Shawn Pearce, Jens Lehmann, Git, Nahor
In-Reply-To: <20121029114310.GA16046@sigill.intra.peff.net>
On Mon, Oct 29, 2012 at 7:43 AM, Jeff King <peff@peff.net> wrote:
> On Mon, Oct 29, 2012 at 07:29:45AM -0400, W. Trevor King wrote:
>
>> On Mon, Oct 29, 2012 at 06:58:55AM -0400, Jeff King wrote:
>> > Can you send an updated version of the patch that summarizes the
>> > situation in the commit message?
>>
>> Sure. Should I include Phil's $submodule_<var-name> export, or would
>> you rather have that be a separate series?
>
> I think it probably makes sense as a separate patch in the same series,
> since it is meant to support the same workflows.
I agree. I did expect to clean it up some, but also to suffer some
review. Feel free to clean it up as you see fit and submit it with
your series.
> 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. :-\
Phil
^ permalink raw reply
* Re: git push tags
From: Kacper Kornet @ 2012-10-29 17:23 UTC (permalink / raw)
To: Jeff King
Cc: Drew Northup, Michael Haggerty, Angelo Borsotti, Philip Oakley,
Chris Rorvick, Johannes Sixt, git
In-Reply-To: <20121029113500.GA15597@sigill.intra.peff.net>
On Mon, Oct 29, 2012 at 07:35:00AM -0400, Jeff King wrote:
> On Mon, Oct 29, 2012 at 07:21:52AM -0400, Drew Northup wrote:
> > > I would have expected git to at least complain about updating an
> > > annotated tag with another annotated tag. But it actually uses the same
> > > fast-forward rule, just on the pointed-to commits. So a fast-forward
> > > annotated re-tag will throw away the old tag object completely. Which
> > > seems a bit crazy to me.
> > > It seems like a no-brainer to me that annotated tags should not replace
> > > each other without a force, no matter where in the refs hierarchy they
> > > go.
> > > For lightweight tags, I think it's more gray. They are just pointers
> > > into history. Some projects may use them to tag immutable official
> > > versions, but I also see them used as shared bookmarks. Requiring "-f"
> > > may make the latter use more annoying. On the other hand, bookmark tags
> > > tend not to be pushed, or if they are, it is part of a mirror-like
> > > backup which should be forcing all updates anyway.
> > Would that be an endorsement of continuing to build a patch set
> > including the snippet that Kacper posted earlier (1) in response to my
> > comment about not being sure how complicated all of this would be or
> > not?
> 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.
> 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.
--
Kacper
^ permalink raw reply
* Re: Git clone fails with "bad pack header", how to get remote log
From: Konstantin Khomoutov @ 2012-10-29 17:18 UTC (permalink / raw)
To: git-users-/JYPxA39Uh5TLH3MbocFFw
Cc: Kevin Molcard, git-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <7f498800-ed38-474d-86ad-cb937be68173-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org>
On Mon, 29 Oct 2012 09:52:54 -0700 (PDT)
Kevin Molcard <kev2041-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> I have a problem with my build system.
>
> I have a remote server with a relatively large repository (around 12
> GB, each branch having a size of 3 GB).
>
> I have also 2 build servers (Mac, Windows) that are cloning the repo
> from the remote.
>
> Sometimes (very often when several git clone are sent at the same
> time), I have the following error:
>
> remote: internal server error
> fatal: protocol error: bad pack header
>
> I know that it happens when the remote is compressing objects (thanks
> to `--progress -v` flags) because the last line of the log before the
> erro is:
> remote: Compressing objects: 93% (17959/19284) [K
>
> * So I have 2 questions, does anybody what is the problem and what
> should I do?
> * Is there a way to get a more precise log from the remote to debug
> this problem?
This reminds me of a bug fixed in 1.7.12.1 [1]:
* When "git push" triggered the automatic gc on the receiving end, a
message from "git prune" that said it was removing cruft leaked to
the standard output, breaking the communication protocol.
In any case, bugs should be reported to the main Git list (which is
git at vger.kernel.org), not here.
I'm Cc'ing the main Git list so you'll get any responses from there, if
any.
Kevin, please answer to this message (keeping all the Ccs -- use "Reply
to group" or "Reply to all" in your MUA) and describe exactly what Git
versions on which platforms your have.
1. https://raw.github.com/git/git/master/Documentation/RelNotes/1.7.12.1.txt
--
You received this message because you are subscribed to the Google Groups "Git for human beings" group.
To post to this group, send email to git-users-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
To unsubscribe from this group, send email to git-users+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit this group at http://groups.google.com/group/git-users?hl=en.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox