* Re: Heads up: rebase -i -p will be made sane again
From: Johannes Schindelin @ 2009-01-27 17:59 UTC (permalink / raw)
To: Stephen Haberman; +Cc: git
In-Reply-To: <20090127085418.e113ad5a.stephen@exigencecorp.com>
Hi,
On Tue, 27 Jan 2009, Stephen Haberman wrote:
> > As for the design bug I want to fix: imagine this history:
> >
> > ------A
> > / /
> > / /
> > ---- B
> > \ \
> > \ \
> > C-----D-----E = HEAD
> >
> > A, C and D touch the same file, and A and D agree on the contents.
> >
> > Now, rebase -p A does the following at the moment:
> >
> > ------A-----E' = HEAD
> > / /
> > / /
> > ---- B
> >
> > In other words, C is truly forgotten, and it is pretended that D never
> > happened, either. That is exactly what test case 2 in t3410 tests for
> > [*1*].
> >
> > This is insane.
>
> Agreed.
Good! I already feared that you would be disagreeing with me.
> Does this mean you're just getting rid of the code that calls "rev list
> --cherry-pick"?
Not exactly. The idea of rebasing is to stay on top of an upstream. If
that upstream has your changes already, you do not want to reapply them --
even with --preserve-merges.
Now, a merge cannot be sent as a patch mail, for good reasons. So
whatever merge might look like yours, it is not. So it is your
responsibility to say that yours is obsolete, and delete it from the
rebase script.
If your merge is in upstream (because a pull-request was heeded, for
example), then you will not see the commits anyway.
> A few times I've pondered just removing the --cherry-pick/drop commit
> part of rebase-p, but assumed it was there for a reason.
I will find the "dropped" commits using git log -p | git patch-id.
It is still nice to tell the user if she wants to merge a parent that is
already in upstream, so I would not like to miss out on that information.
> > [*1*] The code in t3410 was not really easy to read, even if there was
> > an explanation what it tried to do, but the test code was inconsitent,
> > sometimes tagging, sometimes not, sometimes committing with -a,
> > sometimes "git add"ing first, yet almost repetitive.
> >
> > In my endeavor not only to understand it, and either fix my code or
> > the code in t3410, I refactored it so that others should have a much
> > easier time to understand what it actually does.
>
> Thanks for cleaning it up.
>
> I recently saw a test of yours use a `test_commit` bash function that I
> really like. My last patch submission debacle had a patch cleaning up
> t3411 by introducing `test_commit`--I can brave `git send-email` again
> if you have any interest in me resending it.
Heh... so I sent that part of the patches. Hopefully they will get in
soon, as they should be rather obvious, and I have a lot more to come...
Ciao,
Dscho
^ permalink raw reply
* Re: "malloc failed"
From: Johannes Schindelin @ 2009-01-27 18:02 UTC (permalink / raw)
To: David Abrahams; +Cc: git
In-Reply-To: <878wow7pth.fsf@mcbain.luannocracy.com>
Hi,
On Tue, 27 Jan 2009, David Abrahams wrote:
> I've been abusing Git for a purpose it wasn't intended to serve:
> archiving a large number of files with many duplicates and
> near-duplicates.
Hah! My first UGFWIINI contender! Unfortunately, I listed that purpose
explicitely already...
> Every once in a while, when trying to do something really big, it tells
> me "malloc failed" and bails out (I think it's during "git add" but
> because of the way I issued the commands I can't tell: it could have
> been a commit or a gc). This is on a 64-bit linux machine with 8G of
> ram and plenty of swap space, so I'm surprised.
Yes, I am surprised, too. I would expect that some kind of arbitrary
user-specifiable limit hit you. Haven't had time to look at the code,
though.
Ciao,
Dscho
^ permalink raw reply
* Re: Heads up: major rebase -i -p rework coming up
From: Johannes Schindelin @ 2009-01-27 18:08 UTC (permalink / raw)
To: Stephen Haberman; +Cc: git, spearce, Thomas Rast, Björn Steinbrink
In-Reply-To: <20090127092117.d13f24e7.stephen@exigencecorp.com>
Hi,
On Tue, 27 Jan 2009, Stephen Haberman wrote:
> > I am very sorry if somebody actually scripted rebase -i -p (by setting
> > GIT_EDITOR with a script), but I am very certain that this cleanup is
> > absolutely necessary to make rebase -i -p useful.
>
> I have scripted rebase-i-p, but with GIT_EDITOR=: [1]. I assume this
> will still work and just accept the default script?
Yes, this will still work. AFAICT this is actually how git
--no-interactive -p is implemented...
> (Er, maybe I can just use rebase-p...I forget why [1] is using the
> GIT_EDITOR=: with -i.)
See above... :-)
> My primary pain point with rebase-i-p has been rebasing a branch that
> has merged in another branch that has a lot of commits on it. E.g.:
>
> a -- b -- c origin/feature
> \
> d -- e feature
> /
> ... g origin/master
>
> Where e is merging in, say, a latest release that had a few hundred
> commits in the master branch. After resolving conflicts/etc. in e, I
> want to rebase d..e from a to be on c.
>
> The two problems have been:
>
> 1) `git pull` with rebase set uses rebase-i, with no -p, so all of the
> commits from the latest release branch that got merged in with e are
> flattened/duplicated.
Maybe teach git pull about --rebase=preserve[-merges] and
branch.<name>.rebase=preserve[-merges]?
> 2) With manual invocation of `rebase-i-p`, previously you'd get a
> laundry list of commits from the e merge that are new to the feature
> branch, but since g and its ancestors aren't changing, you don't need
> to consider them in the script and so its (potentially a lot of)
> noise. This is what the parent probing back port from git sequencer
> addressed.
I always meant to handle that in the fast-forward handling of pick_one().
> So, I don't mean to rehash old complaints, as I'd love to see the
> rebase-i-p code cleaned up by someone who can really refactor it vs. my
> hack patches. But I wanted to emphasize the motivation for my hacks over
> their implementation so that hopefully you can still address these use
> cases in the new version.
Well, let's see how things turn out once I use the patches for my own
work...
Thanks,
Dscho
^ permalink raw reply
* Re: friendlier names
From: Johannes Schindelin @ 2009-01-27 18:10 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: David Abrahams, git
In-Reply-To: <20090127153837.GB1321@spearce.org>
Hi,
On Tue, 27 Jan 2009, Shawn O. Pearce wrote:
> David Abrahams <dave@boostpro.com> wrote:
> >
> > For example, why couldn't the "index" be called the "stage" instead?
> > That, along with knowing that "git add" was a synonym for "git stage"
> > would have flattened the learning curve considerably for me.
>
> Historical reasons...
>
> Waaay back the "index" was an index of the files git knows about in
> your working directory. It made sense to call it an index, as like
> a book index it was a full listing of what's here, sorted by name.
>
> That's pre-1.0 days. Like the very first version Linus ever
> released. Aka commit e83c5163316f89bfbde7d9ab23ca2e25604af290.
Actually, it was known by the name "dircache" back then :-)
Ciao,
Dscho
^ permalink raw reply
* Re: rebase failure if commit message looks like a patch
From: Johannes Schindelin @ 2009-01-27 18:15 UTC (permalink / raw)
To: Anton; +Cc: git
In-Reply-To: <loom.20090127T173025-125@post.gmane.org>
Hi,
On Tue, 27 Jan 2009, Anton wrote:
> I have found a strange behaviour of "git rebase", present in following
> versions:
That is a known issue. Please work around it with redoing the rebase with
-m.
I could imagine that we should scan the commit messages before actually
running rebase, and either activate -m, or escape the "diff" with a
backslash (as is frequently done with "From" in mails).
Ciao,
Dscho
^ permalink raw reply
* Re: Hosting from Windows XP.
From: Tim Visher @ 2009-01-27 18:24 UTC (permalink / raw)
To: Johannes Gilger; +Cc: git
In-Reply-To: <glkvpp$fvr$1@ger.gmane.org>
Well, I ended up going with Cygwin. Didn't feel like the hassle of
all of the rest of that stuff. Thanks so much, everyone. :)
--
In Christ,
Timmy V.
http://burningones.com/
http://five.sentenc.es/ - Spend less time on e-mail
^ permalink raw reply
* Re: Anyone have access to 64-bit Vista?
From: Stephen Haberman @ 2009-01-27 18:24 UTC (permalink / raw)
To: Geoffrey Lee; +Cc: git
In-Reply-To: <83d7aaa40901270130g3bebb3d6we4839b567f310e46@mail.gmail.com>
> I'm trying to develop for TortoiseGit on a 64-bit Vista SP1 system,
> but I'm having trouble registering the shell extensions with explorer.
> I'm hoping somebody else here who has access to 64-bit Vista can tell
> me if TortoiseGit runs on their machine?
It kind of works on my Vista 64-bit system--I do not see the shell
extensions in the native Windows Explorer (which is 64 bit), but I do
see the shell extensions in an Explorer replacement I use (Xplorer2)
that is 32-bit.
I've seen other oddities in 32-bit vs. 64-bit programs--e.g. my alt tab
replacement (Joe), which is 32-bit, works great with 32-bit programs
but cannot remove focus from 64-bit programs (IE, Windows Explorer,
etc.). Ironically, very few of the programs I use are 64-bit, so I get
by with the alt tab replacement.
- Stephen
^ permalink raw reply
* Re: Building Documentation in Cygwin
From: Tim Visher @ 2009-01-27 18:27 UTC (permalink / raw)
To: Tay Ray Chuan; +Cc: git
In-Reply-To: <c115fd3c0901270944p7a4b6cc5n5dfe9b2f268f038c@mail.gmail.com>
On Tue, Jan 27, 2009 at 12:44 PM, Tim Visher <tim.visher@gmail.com> wrote:
>> you might also want to try getting the cygwin package docbook-xml42.
>
> Sure enough, I didn't have that installed. Turns out I should've done
> that part first too, as it appears to have fixed my problem. Thanks
> so much!
Well, I take that back. I built the docs successfully on the machine
that is connected to the net. I couldn't build them on the offline
box. Currently my solution is to build them on this box and then
manually move them over to the offline box. It'd be nice to not have
to do this though... :\
Anyway, further help is appreciated! :)
--
In Christ,
Timmy V.
http://burningones.com/
http://five.sentenc.es/ - Spend less time on e-mail
^ permalink raw reply
* Re: friendlier names
From: Junio C Hamano @ 2009-01-27 18:28 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: David Abrahams, git
In-Reply-To: <20090127153837.GB1321@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> writes:
> David Abrahams <dave@boostpro.com> wrote:
>>
>> For example, why couldn't the "index" be called the "stage" instead?
>> That, along with knowing that "git add" was a synonym for "git stage"
>> would have flattened the learning curve considerably for me.
>
> Historical reasons...
>
> Waaay back the "index" was an index of the files git knows about in
> your working directory. It made sense to call it an index, as like
> a book index it was a full listing of what's here, sorted by name.
>
> That's pre-1.0 days. Like the very first version Linus ever
> released. Aka commit e83c5163316f89bfbde7d9ab23ca2e25604af290.
This part is a bit misleading, if not completely incorrect.
Since day 1, the entity we now call "the index aka staging area" has been
just that. It was originally called "cache" and it implemented a cache of
_contents_ that are meant to go in the tree the next commit records. It
was renamed to the "index" because it is an index, which you can consult
with a pathname as the key to get to the _contents_ as its value.
It never was "list of pathnames" without _contents_. So it was a staging
area from day one.
The way to update the cache was called "update-cache" then "update-index".
Because it usually is much rare to actually add a new entry to the index
than updating an existing entry in the index, the command had a safeguard
against "update-cache a-newfile" without explicit request from the user to
say "oh by the way I know I am adding new entries". "git add" came much
later to give you a shorthand for "update-index --add". Updating existing
entries in the index was still done with "update-index".
Later Nico taught (after much discussion) "git add" to also serve as
"update-index" for existing entries in the index.
We could have called it "git update-index" when we did that switch-over,
because the operation is exactly that --- updating the index.
But the name somehow stuck.
> Only late last October at the GitTogether did we start to talk about
> creating a command called "git stage", because people have started to
> realize we seem to call it a "staging area" as we train newcomers...
Yeah, you may have to consider the possibility that that particular
training lingo is inconsistent with the rest of the system, exactly
because it came from outside.
^ permalink raw reply
* Re: Building Documentation in Cygwin
From: Junio C Hamano @ 2009-01-27 18:32 UTC (permalink / raw)
To: Tim Visher; +Cc: Tay Ray Chuan, git
In-Reply-To: <c115fd3c0901271027s61a33273lfe9dd702a66aac6@mail.gmail.com>
Tim Visher <tim.visher@gmail.com> writes:
> On Tue, Jan 27, 2009 at 12:44 PM, Tim Visher <tim.visher@gmail.com> wrote:
>
>>> you might also want to try getting the cygwin package docbook-xml42.
>>
>> Sure enough, I didn't have that installed. Turns out I should've done
>> that part first too, as it appears to have fixed my problem. Thanks
>> so much!
>
> Well, I take that back. I built the docs successfully on the machine
> that is connected to the net. I couldn't build them on the offline
> box. Currently my solution is to build them on this box and then
> manually move them over to the offline box. It'd be nice to not have
> to do this though... :\
>
> Anyway, further help is appreciated! :)
The xml toolchain has a tendency to download dtds from the original source
with a way for you (typically your distribution) to tell it to use a
locally installed version. "Can build online, fail when offline" is a
sign that it is not using the local copy.
^ permalink raw reply
* Re: Valgrind updates
From: Linus Torvalds @ 2009-01-27 18:55 UTC (permalink / raw)
To: Johannes Schindelin, zlib
Cc: Mark Brown, Jeff King, Junio C Hamano, Git Mailing List
In-Reply-To: <alpine.DEB.1.00.0901271742430.3586@pacific.mpi-cbg.de>
On Tue, 27 Jan 2009, Johannes Schindelin wrote:
>
> Come to think of it, the word "suppression" is probably a good indicator
> that valgrind never claimed it would mark the zlib buffer as properly
> initialized.
Hmm. The zlib faq has a note about zlib doing a conditional on
uninitialized memory that doesn't matter, and that is what the suppression
should be about (to avoid a warning about "Conditional jump or move
depends on uninitialised value").
But that one is documented to not matter for the actual output (zlib
FAQ#36).
It's possible that zlib really does leave padding bytes around that
literally don't matter, and that don't get initialized. That really would
be bad, because it means that the output of git wouldn't be repeatable.
But I doubt this is the case - original git used to actually do the SHA1
over the _compressed_ data, which was admittedly a totally and utterly
broken design (and we fixed it), but it did work. Maybe it worked by luck,
but I somehow doubt it.
Some googling did find this:
http://mailman.few.vu.nl/pipermail/sysprog/2008-October/000298.html
which looks very similar: an uninitialized byte in the middle of a
deflate() packet.
Anyway, I'm just going to Cc 'zlib@gzip.org', since this definitely is
_not_ the same issue as in the FAQ, and we're not the only ones seeing it.
For the zlib people: the code is literally this:
/* Set it up */
memset(&stream, 0, sizeof(stream));
deflateInit(&stream, zlib_compression_level);
size = 8 + deflateBound(&stream, len+hdrlen);
compressed = xmalloc(size);
/* Compress it */
stream.next_out = compressed;
stream.avail_out = size;
/* First header.. */
stream.next_in = (unsigned char *)hdr;
stream.avail_in = hdrlen;
while (deflate(&stream, 0) == Z_OK)
/* nothing */;
/* Then the data itself.. */
stream.next_in = buf;
stream.avail_in = len;
ret = deflate(&stream, Z_FINISH);
if (ret != Z_STREAM_END)
die("unable to deflate new object %s (%d)", sha1_to_hex(sha1), ret);
ret = deflateEnd(&stream);
if (ret != Z_OK)
die("deflateEnd on object %s failed (%d)", sha1_to_hex(sha1), ret);
size = stream.total_out;
if (write_buffer(fd, compressed, size) < 0)
die("unable to write sha1 file");
and valgrind complains that the "write_buffer()" call will touch an
uninitialized byte (just one byte, and in the _middle_ of the buffer, no
less):
> Yet, the buffer in question is 195 bytes, stream.total_count (which
> totally agrees with size - stream.avail_out) says it is 58 bytes, and
> valgrind says that the byte with offset 51 is uninitialized.
The thing to note here is that what we are passing in to "write_buffer()"
is _exactly_ what zlib deflated for us:
- 'compressed' is the allocation, and is what we used to initialize
'stream.next_out' with (at the top of the code sequence above)
- 'size' is gotten from 'stream.total_out' at the end of the compression.
Maybe the zlib people can tell us that we're idiots and the above is
buggy, but maybe there is a real bug in zlib. Maybe it's triggered by our
use of using two different input buffers to deflate() (ie we compress the
header first, and then the body of the actual data, and put it all in one
single output buffer), which may be unusual usage of zlib routines and may
be why there aren't tons of reports of this.
(Our use of just depending on deflate() returning Z_BUF_ERROR after
consuming all of the header data is probably also "unusual", but the
manual explicitly says that it's not fatal and that deflate can be called
again with more buffers).
Oh Gods of zlib, please hear our plea for clarification..
Linus
^ permalink raw reply
* Re: friendlier names
From: Jakub Narebski @ 2009-01-27 19:17 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Shawn O. Pearce, David Abrahams, git
In-Reply-To: <7vwscgy56b.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
>> David Abrahams <dave@boostpro.com> wrote:
>>>
>>> For example, why couldn't the "index" be called the "stage" instead?
>>> That, along with knowing that "git add" was a synonym for "git stage"
>>> would have flattened the learning curve considerably for me.
>>
>> Historical reasons...
[...]
> The way to update the cache was called "update-cache" then "update-index".
> Because it usually is much rare to actually add a new entry to the index
> than updating an existing entry in the index, the command had a safeguard
> against "update-cache a-newfile" without explicit request from the user to
> say "oh by the way I know I am adding new entries". "git add" came much
> later to give you a shorthand for "update-index --add". Updating existing
> entries in the index was still done with "update-index".
>
> Later Nico taught (after much discussion) "git add" to also serve as
> "update-index" for existing entries in the index.
>
> We could have called it "git update-index" when we did that switch-over,
> because the operation is exactly that --- updating the index.
>
> But the name somehow stuck.
[...]
It is a bit of pity that "git add" was overloaded to also add new
contents and not only add new file (and its contents!), instead of
having new command "git stage" to be porcelain version of
"git update-index" porcelain. And perhaps "git resolved" to only
mark resolved entries (so e.g. "git resolved ." would not add new
files, nor add new contents of files which were not in conflict).
Now we have to explain that "git add" adds new contents... OTOH
it is perhaps good idea to emphasize differences between Git and
other lesser^W SCMs. ;-) And introduce "git add -N"...
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply
* Re: Building Documentation in Cygwin
From: Tim Visher @ 2009-01-27 19:27 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Tay Ray Chuan, git
In-Reply-To: <7vskn4y4zt.fsf@gitster.siamese.dyndns.org>
On Tue, Jan 27, 2009 at 1:32 PM, Junio C Hamano <gitster@pobox.com> wrote:
> The xml toolchain has a tendency to download dtds from the original source
> with a way for you (typically your distribution) to tell it to use a
> locally installed version. "Can build online, fail when offline" is a
> sign that it is not using the local copy.
Sounds like we're on the same page. I'm not familiar enough with any
of these tools to know how to do anything about that. Does anyone
know how to explicitly inform xmlto to use a local DTD? I have it
installed, as far as I know, via cygwin (the docbook-xml42 package)
but I don't know how to tell xmlto about it.
Thanks in advance!
--
In Christ,
Timmy V.
http://burningones.com/
http://five.sentenc.es/ - Spend less time on e-mail
^ permalink raw reply
* Re: friendlier names
From: Junio C Hamano @ 2009-01-27 19:50 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Shawn O. Pearce, David Abrahams, git
In-Reply-To: <m37i4gy2z6.fsf@localhost.localdomain>
Jakub Narebski <jnareb@gmail.com> writes:
> It is a bit of pity that "git add" was overloaded to also add new
> contents and not only add new file (and its contents!), instead of
> having new command "git stage" to be porcelain version of
> "git update-index" porcelain. And perhaps "git resolved" to only
> mark resolved entries (so e.g. "git resolved ." would not add new
> files, nor add new contents of files which were not in conflict).
I do not think so.
People who are taught with various means (including "git stage" alias)
understand that you prepare the contents you want to record in the commit
you are about to make by updating the contents registered in the index aka
staging area, then you do not need "git resolved".
You resolve, you have the desired content in your work tree, and you
register the updated contents from your work tree to the index aka staging
area, in exactly the same way as you do when you want to include updated
contents for any commit.
^ permalink raw reply
* Re: Building Documentation in Cygwin
From: Junio C Hamano @ 2009-01-27 20:05 UTC (permalink / raw)
To: Tim Visher; +Cc: Tay Ray Chuan, git
In-Reply-To: <c115fd3c0901271127q2a7f0f58i59ff446060982fd7@mail.gmail.com>
Tim Visher <tim.visher@gmail.com> writes:
> On Tue, Jan 27, 2009 at 1:32 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
>> The xml toolchain has a tendency to download dtds from the original source
>> with a way for you (typically your distribution) to tell it to use a
>> locally installed version. "Can build online, fail when offline" is a
>> sign that it is not using the local copy.
>
> Sounds like we're on the same page. I'm not familiar enough with any
> of these tools to know how to do anything about that. Does anyone
> know how to explicitly inform xmlto to use a local DTD? I have it
> installed, as far as I know, via cygwin (the docbook-xml42 package)
> but I don't know how to tell xmlto about it.
>
> Thanks in advance!
I do not know how Cygwin packages things, but here is an exchange I had
with k.org admin when they updated the machine I prepare the preformatted
documentation pages and I had similar trouble.
-- >8 -- from here -- >8 --
/usr/share/sgml/docbook/xmlcatalog on master.kernel.org is empty:
$ ls -l /usr/share/sgml/docbook/xmlcatalog
-rw-r--r-- 1 root root 236 2008-06-30 06:51 /usr/share/sgml/docbook/xmlcatalog
$ cat /usr/share/sgml/docbook/xmlcatalog
<?xml version="1.0"?>
<!DOCTYPE catalog PUBLIC "-//OASIS//DTD Entity Resolution XML Catalog V1.0//EN"
"http://www.oasis-open.org/committees/entity/release/1.0/catalog.dtd">
<catalog xmlns="urn:oasis:names:tc:entity:xmlns:xml:catalog"/>
On a good machine, however, it is much larger:
$ ls -l /usr/share/sgml/docbook/xmlcatalog
-rw-r--r-- 1 root root 7907 Jul 4 14:08 /usr/share/sgml/docbook/xmlcatalog
$ grep docbookx /usr/share/sgml/docbook/xmlcatalog
<public publicId="-//OASIS//DTD DocBook XML V4.1.2//EN" uri="xml-dtd-4.1.2-1.0-35.fc9/docbookx.dtd"/>
<public publicId="-//OASIS//DTD DocBook XML V4.2//EN" uri="xml-dtd-4.2-1.0-35.fc9/docbookx.dtd"/>
<public publicId="-//OASIS//DTD DocBook XML V4.3//EN" uri="xml-dtd-4.3-1.0-35.fc9/docbookx.dtd"/>
<public publicId="-//OASIS//DTD DocBook XML V4.4//EN" uri="xml-dtd-4.5-1.0-35.fc9/docbookx.dtd"/>
The lack of these entries seems to cause the processing to go to the
network instead of using the files locally installed.
-- 8< -- to here -- 8< --
and a response I got from them:
-- >8 -- from here -- >8 --
Hmmm tried reinstalling docbook-dtds-1.0-35.fc9.noarch.rpm and things
seem to have showed up like your expecting - clearly a bad dependency in
the rpm. Anyway give it a whirl.
-- 8< -- to here -- 8< --
After that exchange, things did work much better.
I hope the above (words like "docbook-dtds") has enough hints for you to
find corresponding set of packages on Cygwin to additionally install or
futz with to get the formatting working.
^ permalink raw reply
* Re: [PATCH 1/6] t3404 & t3411: undo copy&paste
From: Junio C Hamano @ 2009-01-27 21:01 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Stephen Haberman, Thomas Rast, git
In-Reply-To: <alpine.DEB.1.00.0901271845380.3586@pacific.mpi-cbg.de>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> Rather than copying and pasting, which is prone to lead to fixes
> missing in one version, move the fake-editor generator to t/t3404/.
>
> While at it, fix a typo that causes head-scratching: use
> ${SHELL_PATH-/bin/sh} instead of $SHELL_PATH.
I've learned to be cautious whenever I see "while at it".
> diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
> new file mode 100644
> index 0000000..8c8caab
> --- /dev/null
> +++ b/t/lib-rebase.sh
> @@ -0,0 +1,36 @@
> +#!/bin/sh
> +
> +set_fake_editor () {
> + echo "#!${SHELL_PATH-/bin_sh}" >fake-editor.sh
It is unclear why you would want to do this. It was unclear what "typo"
you were referring to in your commit log message, either.
The tests are supposed to run under the shell the user specified, so if
there is a case you found that $SHELL_PATH is unset, that is a bug we
would want to fix, and ${SHELL_PATH-/bin/sh} is sweeping the problem under
the rug to make it harder to fix, isn't it?
I would understand if it were
${SHELL_PATH?"SHELL_PATH Not Set --- bug in tests?"}
though.
Besides, it's /bin/sh, not /bin_sh ;-)
> + cat >> fake-editor.sh <<\EOF
> +case "$1" in
> +*/COMMIT_EDITMSG)
> + test -z "$FAKE_COMMIT_MESSAGE" || echo "$FAKE_COMMIT_MESSAGE" > "$1"
> + test -z "$FAKE_COMMIT_AMEND" || echo "$FAKE_COMMIT_AMEND" >> "$1"
> + exit
> + ;;
> +esac
> +test -z "$EXPECT_COUNT" ||
> + test "$EXPECT_COUNT" = $(sed -e '/^#/d' -e '/^$/d' < "$1" | wc -l) ||
> + exit
> +test -z "$FAKE_LINES" && exit
> +grep -v '^#' < "$1" > "$1".tmp
> +rm -f "$1"
> +cat "$1".tmp
> +action=pick
> +for line in $FAKE_LINES; do
> + case $line in
> + squash|edit)
> + action="$line";;
> + *)
> + echo sed -n "${line}s/^pick/$action/p"
> + sed -n "${line}p" < "$1".tmp
> + sed -n "${line}s/^pick/$action/p" < "$1".tmp >> "$1"
I looked at the output from this and wondered what these "sed -n" shown in
the "-v" output were about last night. I do think it is a good idea to
show what edit was done to the insn stream, but I suspect it may be easier
to read the output if you did this instead:
> + sed -n "${line}p" < "$1".tmp
> + sed -n "${line}s/^pick/$action/p" < "$1".tmp >> "$1"
> + sed -n "${line}s/^pick/$action/p" < "$1".tmp
^ permalink raw reply
* Re: [PATCH 2/6] lib-rebase.sh: Document what set_fake_editor() does
From: Junio C Hamano @ 2009-01-27 21:03 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Stephen Haberman, Thomas Rast, git
In-Reply-To: <alpine.DEB.1.00.0901271846340.3586@pacific.mpi-cbg.de>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> rnyn
> Make it easy for other authors to use rebase tests' fake-editor.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
Perhaps a very welcome addition, except that I did not find rnyn in my
dictionary, and the patch textually depends on /bin_sh bug ;-)
^ permalink raw reply
* Re: [PATCH 3/6] lib-rebase.sh: introduce test_commit() and test_merge() helpers
From: Junio C Hamano @ 2009-01-27 21:09 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Stephen Haberman, Thomas Rast, git
In-Reply-To: <alpine.DEB.1.00.0901271847070.3586@pacific.mpi-cbg.de>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> This may want to live in test-lib.sh instead.
Yeah, I tend to agree.
> diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
> index cda7778..37430f3 100644
> --- a/t/lib-rebase.sh
> +++ b/t/lib-rebase.sh
> @@ -46,3 +46,29 @@ EOF
> test_set_editor "$(pwd)/fake-editor.sh"
> chmod a+x fake-editor.sh
> }
> +
> +# Call test_commit with the arguments "<message> [<file> [<contents>]]"
> +#
> +# This will commit a file with the given contents and the given commit
> +# message. It will also add a tag with <message> as name.
> +#
> +# Both <file> and <contents> default to <message>.
> +
> +test_commit () {
> + file=$2
> + test -z "$2" && file=$(echo "$1" | tr 'A-Z' 'a-z')
file=${2:-$(echo "$1" | tr 'A-Z' 'a-z')}
might be more consistent with this:
> + echo ${3-$1} > $file &&
and may be easier to read.
I'd suggest dquoting argument to echo above, i.e. "${3-$1}", and all the
references to positional arguments in the remainder of the patch, though.
> + git add $file &&
as well as "$file" here.
> + test_tick &&
> + git commit -m $1 &&
> + git tag $1
> +}
> +
> +# Call test_merge with the arguments "<message> <commit>", where <commit>
> +# can be a tag pointing to the commit-to-merge.
> +
> +test_merge () {
> + test_tick &&
> + git merge -m $1 $2 &&
> + git tag $1
> +}
> --
> 1.6.1.482.g7d54be
^ permalink raw reply
* Re: Valgrind updates
From: Johannes Schindelin @ 2009-01-27 21:52 UTC (permalink / raw)
To: Linus Torvalds
Cc: zlib, valgrind-users, Mark Brown, Jeff King, Junio C Hamano,
Git Mailing List
In-Reply-To: <alpine.LFD.2.00.0901271006060.3123@localhost.localdomain>
Hi,
[Cc'ed the valgrind-users list, maybe the valgrind Gods can see that our
case is pretty strange, and tell us what we do wrong.]
Note to valgrind experts: this is _not_ about the Conditional thing in
zlib, but about an uninitialized byte _in the middle_ of the zlib output
buffer.
On Tue, 27 Jan 2009, Linus Torvalds wrote:
> Hmm. The zlib faq has a note about zlib doing a conditional on
> uninitialized memory that doesn't matter, and that is what the
> suppression should be about (to avoid a warning about "Conditional jump
> or move depends on uninitialised value").
>
> But that one is documented to not matter for the actual output (zlib
> FAQ#36).
>
> It's possible that zlib really does leave padding bytes around that
> literally don't matter, and that don't get initialized. That really
> would be bad, because it means that the output of git wouldn't be
> repeatable. But I doubt this is the case - original git used to actually
> do the SHA1 over the _compressed_ data, which was admittedly a totally
> and utterly broken design (and we fixed it), but it did work. Maybe it
> worked by luck, but I somehow doubt it.
>
> Some googling did find this:
>
> http://mailman.few.vu.nl/pipermail/sysprog/2008-October/000298.html
>
> which looks very similar: an uninitialized byte in the middle of a
> deflate() packet.
>
> Anyway, I'm just going to Cc 'zlib@gzip.org', since this definitely is
> _not_ the same issue as in the FAQ, and we're not the only ones seeing it.
>
> [...]
>
> Dscho wrote:
>
> > Yet, the buffer in question is 195 bytes, stream.total_count (which
> > totally agrees with size - stream.avail_out) says it is 58 bytes, and
> > valgrind says that the byte with offset 51 is uninitialized.
>
> The thing to note here is that what we are passing in to "write_buffer()"
> is _exactly_ what zlib deflated for us:
>
> - 'compressed' is the allocation, and is what we used to initialize
> 'stream.next_out' with (at the top of the code sequence above)
>
> - 'size' is gotten from 'stream.total_out' at the end of the compression.
>
> Oh Gods of zlib, please hear our plea for clarification..
To help ye Gods, I put together this almost minimal C program:
-- snip --
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <zlib.h>
int main(int argc, char **argv)
{
const char hdr[] = {
0x74, 0x72, 0x65, 0x65, 0x20, 0x31, 0x36, 0x35,
0x00,
};
int hdrlen = sizeof(hdr);
const char buf[] = {
0x31, 0x30, 0x30, 0x36, 0x34, 0x34, 0x20, 0x66,
0x69, 0x6c, 0x65, 0x31, 0x00, 0x10, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x31, 0x30, 0x30, 0x36, 0x34, 0x34, 0x20,
0x66, 0x69, 0x6c, 0x65, 0x32, 0x00, 0x20, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x31, 0x30, 0x30, 0x36, 0x34, 0x34,
0x20, 0x66, 0x69, 0x6c, 0x65, 0x33, 0x00, 0x30,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x31, 0x30, 0x30, 0x36, 0x34,
0x34, 0x20, 0x66, 0x69, 0x6c, 0x65, 0x34, 0x00,
0x40, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x31, 0x30, 0x30, 0x36,
0x34, 0x34, 0x20, 0x66, 0x69, 0x6c, 0x65, 0x35,
0x00, 0x50, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00,
};
int len = sizeof(buf);
z_stream stream;
unsigned char *compressed;
int size, ret, i;
FILE *out;
memset(&stream, 0, sizeof(stream));
deflateInit(&stream, Z_BEST_SPEED);
size = 8 + deflateBound(&stream, len+hdrlen);
compressed = malloc(size);
if (!compressed)
return 1;
stream.next_out = compressed;
stream.avail_out = size;
stream.next_in = (unsigned char *)hdr;
stream.avail_in = hdrlen;
while ((ret = deflate(&stream, 0)) == Z_OK)
/* nothing */;
/* deflate() returns Z_BUF_ERROR at this point */
stream.next_in = (unsigned char *)buf;
stream.avail_in = len;
ret = deflate(&stream, Z_FINISH);
if (ret != Z_STREAM_END)
return 1;
if (deflateEnd(&stream) != Z_OK)
return 1;
out = fopen("/dev/null", "w");
fwrite(compressed + 51, 51, 1, out);
fwrite(compressed + 51, 1, 1, stderr);
fflush(out);
fclose(out);
free(compressed);
return 0;
}
-- snap --
... which produces this output...
-- snip --
==6348== Memcheck, a memory error detector.
==6348== Copyright (C) 2002-2008, and GNU GPL'd, by Julian Seward et al.
==6348== Using LibVEX rev exported, a library for dynamic binary translation.
==6348== Copyright (C) 2004-2008, and GNU GPL'd, by OpenWorks LLP.
==6348== Using valgrind-3.5.0.SVN, a dynamic binary instrumentation framework.
==6348== Copyright (C) 2000-2008, and GNU GPL'd, by Julian Seward et al.
==6348== For more details, rerun with: -v
==6348==
==6348== Use of uninitialised value of size 8
==6348== at 0x4E2FC5B: (within /usr/lib/libz.so.1.2.3.3)
==6348== by 0x4E317B6: (within /usr/lib/libz.so.1.2.3.3)
==6348== by 0x4E2DF9C: (within /usr/lib/libz.so.1.2.3.3)
==6348== by 0x4E2E654: deflate (in /usr/lib/libz.so.1.2.3.3)
==6348== by 0x400957: main (valgrind-testcase.c:60)
==6348==
==6348== Syscall param write(buf) points to uninitialised byte(s)
==6348== at 0x5103D50: write (in /lib/libc-2.6.1.so)
==6348== by 0x50A9AE2: _IO_file_write (in /lib/libc-2.6.1.so)
==6348== by 0x50A9748: (within /lib/libc-2.6.1.so)
==6348== by 0x50A9A4B: _IO_file_xsputn (in /lib/libc-2.6.1.so)
==6348== by 0x509FDBA: fwrite (in /lib/libc-2.6.1.so)
==6348== by 0x4009D7: main (valgrind-testcase.c:69)
==6348== Address 0x53da87b is 51 bytes inside a block of size 195 alloc'd
==6348== at 0x4C222CB: malloc (in /usr/local/lib/valgrind/amd64-linux/vgpreload_memcheck.so)
==6348== by 0x4008D7: main (valgrind-testcase.c:45)
,==6348==
==6348== Syscall param write(buf) points to uninitialised byte(s)
==6348== at 0x5103D50: write (in /lib/libc-2.6.1.so)
==6348== by 0x50A9AE2: _IO_file_write (in /lib/libc-2.6.1.so)
==6348== by 0x50A9748: (within /lib/libc-2.6.1.so)
==6348== by 0x50A9A83: _IO_do_write (in /lib/libc-2.6.1.so)
==6348== by 0x50AA048: _IO_file_sync (in /lib/libc-2.6.1.so)
==6348== by 0x509EDB9: fflush (in /lib/libc-2.6.1.so)
==6348== by 0x4009E0: main (valgrind-testcase.c:70)
==6348== Address 0x4020000 is not stack'd, malloc'd or (recently) free'd
==6348==
==6348== ERROR SUMMARY: 3 errors from 3 contexts (suppressed: 15 from 4)
==6348== malloc/free: in use at exit: 0 bytes in 0 blocks.
==6348== malloc/free: 7 allocs, 7 frees, 268,835 bytes allocated.
==6348== For counts of detected errors, rerun with: -v
==6348== Use --track-origins=yes to see where uninitialised values come from
==6348== All heap blocks were freed -- no leaks are possible.
-- snap --
Note that the error only occurs when fwrite()ing to stderr, not
any other file.
This is with valgrind compiled from a git-svn mirror updated today, i.e.
valgrind-3.5.0.SVN.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 1/6] t3404 & t3411: undo copy&paste
From: Johannes Schindelin @ 2009-01-27 21:57 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Stephen Haberman, Thomas Rast, git
In-Reply-To: <7v7i4g31lj.fsf@gitster.siamese.dyndns.org>
Hi,
On Tue, 27 Jan 2009, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > Rather than copying and pasting, which is prone to lead to fixes
> > missing in one version, move the fake-editor generator to t/t3404/.
> >
> > While at it, fix a typo that causes head-scratching: use
> > ${SHELL_PATH-/bin/sh} instead of $SHELL_PATH.
>
> I've learned to be cautious whenever I see "while at it".
Heh.
> > diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
> > new file mode 100644
> > index 0000000..8c8caab
> > --- /dev/null
> > +++ b/t/lib-rebase.sh
> > @@ -0,0 +1,36 @@
> > +#!/bin/sh
> > +
> > +set_fake_editor () {
> > + echo "#!${SHELL_PATH-/bin_sh}" >fake-editor.sh
>
> It is unclear why you would want to do this. It was unclear what "typo"
> you were referring to in your commit log message, either.
>
> The tests are supposed to run under the shell the user specified, so if
> there is a case you found that $SHELL_PATH is unset, that is a bug we
> would want to fix, and ${SHELL_PATH-/bin/sh} is sweeping the problem under
> the rug to make it harder to fix, isn't it?
I call the scripts directly, and I do not think it would be a good idea to
force the user to use GIT_TEST_OPTS and make when calling the script
directly is so much easier. Plus, this way I can pass "sh -x $SCRIPT"
easily.
I am really puzzled that it works, BTW. With an empty SHELL_PATH,
apparently.
> Besides, it's /bin/sh, not /bin_sh ;-)
Right. The commit message was right, at least!
> > + cat >> fake-editor.sh <<\EOF
> > +case "$1" in
> > +*/COMMIT_EDITMSG)
> > + test -z "$FAKE_COMMIT_MESSAGE" || echo "$FAKE_COMMIT_MESSAGE" > "$1"
> > + test -z "$FAKE_COMMIT_AMEND" || echo "$FAKE_COMMIT_AMEND" >> "$1"
> > + exit
> > + ;;
> > +esac
> > +test -z "$EXPECT_COUNT" ||
> > + test "$EXPECT_COUNT" = $(sed -e '/^#/d' -e '/^$/d' < "$1" | wc -l) ||
> > + exit
> > +test -z "$FAKE_LINES" && exit
> > +grep -v '^#' < "$1" > "$1".tmp
> > +rm -f "$1"
> > +cat "$1".tmp
> > +action=pick
> > +for line in $FAKE_LINES; do
> > + case $line in
> > + squash|edit)
> > + action="$line";;
> > + *)
> > + echo sed -n "${line}s/^pick/$action/p"
> > + sed -n "${line}p" < "$1".tmp
> > + sed -n "${line}s/^pick/$action/p" < "$1".tmp >> "$1"
>
> I looked at the output from this and wondered what these "sed -n" shown
> in the "-v" output were about last night. I do think it is a good idea
> to show what edit was done to the insn stream, but I suspect it may be
> easier to read the output if you did this instead:
>
> > + sed -n "${line}p" < "$1".tmp
> > + sed -n "${line}s/^pick/$action/p" < "$1".tmp >> "$1"
> > + sed -n "${line}s/^pick/$action/p" < "$1".tmp
Probably. It is for debugging, anyway. As everything you only see with
-v.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 2/6] lib-rebase.sh: Document what set_fake_editor() does
From: Johannes Schindelin @ 2009-01-27 21:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Stephen Haberman, Thomas Rast, git
In-Reply-To: <7v3af431iz.fsf@gitster.siamese.dyndns.org>
Hi,
On Tue, 27 Jan 2009, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > rnyn
> > Make it easy for other authors to use rebase tests' fake-editor.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
>
> Perhaps a very welcome addition, except that I did not find rnyn in my
> dictionary, and the patch textually depends on /bin_sh bug ;-)
Oh, that? It is perfectly *snarf* normal. Just my *pucker* Tourette
syndrome kicking in.
Ciao,
Dscho
^ permalink raw reply
* git-am annoyance
From: Sverre Rabbelier @ 2009-01-27 22:07 UTC (permalink / raw)
To: Git Mailinglist
Heya,
Observe what happens if, on accident, rather than running a alias
(amendall), the 'tab' didn't catch on:
$ git am
^C
$ # ok, now what do I do?
$ git status
# On branch checker-caching
nothing to commit (working directory clean)
$ # looks like everything is ok, great
$ # ... some time later
$ # same thing happens
$ git am
cat: /home/sverre/code/Melange/.git/rebase-apply/next: No such file or directory
cat: /home/sverre/code/Melange/.git/rebase-apply/utf8: No such file or directory
cat: /home/sverre/code/Melange/.git/rebase-apply/keep: No such file or directory
cat: /home/sverre/code/Melange/.git/rebase-apply/threeway: No such
file or directory
cat: /home/sverre/code/Melange/.git/rebase-apply/apply-opt: No such
file or directory
cat: /home/sverre/code/Melange/.git/rebase-apply/sign: No such file or directory
cat: /home/sverre/code/Melange/.git/rebase-apply/next: No such file or directory
/usr/local/libexec/git-core//git-am: line 319: test: : integer
expression expected
/usr/local/libexec/git-core//git-am: line 326: test: : integer
expression expected
$ # whoops?!
Wouldn't it be nice if "git am" without any arguments just prints a
usage message? Either that, or provides you with a way to bail out? Or
if it'd clean up after itself so that it doesn't crash?
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: [PATCH] mergetool merge/skip/abort
From: Charles Bailey @ 2009-01-27 22:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Theodore Tso, git
In-Reply-To: <20090126225835.GB10118@mit.edu>
On Mon, Jan 26, 2009 at 05:58:35PM -0500, Theodore Tso wrote:
> I was the original author of mergetool, and for a while I was the
> person that was reviewing and managing the mergetool patches for
> Junio. Unfortunately, in the last couple of months I just haven't had
> the time keep up with the various mergetool proposed patch updates.
>
> So maybe it's time for me to hand it off to someone who has the time
> and interest in continuing to hack mergetool, and has the necessary
> "good taste" and such that Junio would be willing to trust that person
> to be the git mergetool patch wrangler?
>
> - Ted
A quick blame session has shown that after Ted I've probably touched
the next most number of lines of mergetool. It's a crude measure and
not necessarily a sign of competence, I admit.
Although not rolling in spare time, I feel I'd be able review
mergetool patches at roughly the rate that they tend to appear at the
moment.
Given the above, if I pass the "good taste" and "Junio trust" tests I
feel that I should offer my services as mergetool patch wrangler.
--
Charles Bailey
http://ccgi.hashpling.plus.com/blog/
^ permalink raw reply
* Re: Heads up: major rebase -i -p rework coming up
From: Nanako Shiraishi @ 2009-01-27 22:10 UTC (permalink / raw)
To: Stephen Haberman
Cc: Johannes Schindelin, git, spearce, Thomas Rast, Bjrn Steinbrink
In-Reply-To: <20090127092117.d13f24e7.stephen@exigencecorp.com>
Quoting Stephen Haberman <stephen@exigencecorp.com>:
> My primary pain point with rebase-i-p has been rebasing a branch that
> has merged in another branch that has a lot of commits on it. E.g.:
>
> a -- b -- c origin/feature
> \
> d -- e feature
> /
> ... g origin/master
>
> Where e is merging in, say, a latest release that had a few hundred
> commits in the master branch. After resolving conflicts/etc. in e, I
> want to rebase d..e from a to be on c.
Sorry for asking a basic question, but if "feature" is a topic branch for advance the feature, why are you merging origin/master into it? Doesn't it blur the theme of the branch by including "development of the feature and all the random things that happened while it was being developed in other places"?
--
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/
^ permalink raw reply
* [EGIT PATCH] Throw created exception
From: Robin Rosenberg @ 2009-01-27 22:11 UTC (permalink / raw)
To: spearce; +Cc: git, Robin Rosenberg
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
.../org/spearce/jgit/lib/UnpackedObjectLoader.java | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/UnpackedObjectLoader.java b/org.spearce.jgit/src/org/spearce/jgit/lib/UnpackedObjectLoader.java
index 0560c3a..f2cae87 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/UnpackedObjectLoader.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/UnpackedObjectLoader.java
@@ -89,7 +89,7 @@ public UnpackedObjectLoader(final Repository db, final AnyObjectId id)
/**
* Construct an ObjectLoader from a loose object's compressed form.
- *
+ *
* @param compressed
* entire content of the loose object file.
* @throws CorruptObjectException
@@ -187,7 +187,7 @@ private void decompress(final AnyObjectId id, final Inflater inf, int p)
throw coe;
}
if (p != objectSize)
- new CorruptObjectException(id, "incorrect length");
+ throw new CorruptObjectException(id, "incorrect length");
}
public int getType() {
--
1.6.1.285.g35d8b
^ permalink raw reply related
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