Git development
 help / color / mirror / Atom feed
* Re: help needed: Splitting a git repository after subversion migration
From: Björn Steinbrink @ 2008-12-12 14:49 UTC (permalink / raw)
  To: Thomas Jarosch; +Cc: Michael J Gruber, git
In-Reply-To: <200812121522.38791.thomas.jarosch@intra2net.com>

On 2008.12.12 15:22:15 +0100, Thomas Jarosch wrote:
> On Thursday, 11. December 2008 09:10:09 you wrote:
> > > Now I'll manually check the history of the tags/ and branches/ folder
> > > for more funny tags and write down the revision. If I understood
> > > the git-svn man page correctly, I should be able to specifiy
> > > revision ranges it's going to import. I'll try to skip the broken tags.
> >
> > As long as the breakage only involves branches/tags that are completely
> > useless, it's probably a lot easier to just delete them afterwards.
> >
> > And if you accidently added changes to a tag, after it was created, it's
> > also easier to manually tag to right version in git, and just forgetting
> > about the additional commit.
> >
> > And for a bunch of other cases, rebase -i/filter-branch are probably
> > also better options ;-)
> >
> > Skipping revisions in a git-svn import sounds rather annoying and
> > error-prone.
> 
> Sounds very reasonable. When I'm done filtering with filter-branch,
> the original commits are still stored in "refs/originals" and the reflogs.
> What's the best way to get rid of those to free up the space?

See the "purging unwanted history" thread:

http://n2.nabble.com/purging-unwanted-history-td1507638.html

The commands there (starting with the "git for-each-ref") should clean
out all the pre-filter-branch stuff.

> A nice way to find the corresponding commit for a file can be found here: 
> http://stackoverflow.com/questions/223678/git-which-commit-has-this-blob

Yeah, I think something similar (or even the same?) is in the git wiki
somewhere. I never had any use for it though ;-)

Björn

^ permalink raw reply

* Unable to index file
From: Ramon Tayag @ 2008-12-12 14:47 UTC (permalink / raw)
  To: git

Hi everyone,

I've come across a problem that I don't believe lies in Rails.  You
needn't be familiar, I think, with Rails to see what's wrong.

I can't seem to add the files that are in
http://dev.rubyonrails.org/archive/rails_edge.zip

1) Unpack the zip
2) Initialize a git repo inside the folder that was unpacked
3) git add .

See the errors.. :o http://pastie.org/337571

Thanks,
Ramon Tayag

^ permalink raw reply

* Re: help needed: Splitting a git repository after subversion migration
From: Thomas Jarosch @ 2008-12-12 14:22 UTC (permalink / raw)
  To: Björn Steinbrink; +Cc: Michael J Gruber, git
In-Reply-To: <20081211081009.GA14639@atjola.homenet>

On Thursday, 11. December 2008 09:10:09 you wrote:
> > Now I'll manually check the history of the tags/ and branches/ folder
> > for more funny tags and write down the revision. If I understood
> > the git-svn man page correctly, I should be able to specifiy
> > revision ranges it's going to import. I'll try to skip the broken tags.
>
> As long as the breakage only involves branches/tags that are completely
> useless, it's probably a lot easier to just delete them afterwards.
>
> And if you accidently added changes to a tag, after it was created, it's
> also easier to manually tag to right version in git, and just forgetting
> about the additional commit.
>
> And for a bunch of other cases, rebase -i/filter-branch are probably
> also better options ;-)
>
> Skipping revisions in a git-svn import sounds rather annoying and
> error-prone.

Sounds very reasonable. When I'm done filtering with filter-branch,
the original commits are still stored in "refs/originals" and the reflogs.
What's the best way to get rid of those to free up the space?

A nice way to find the corresponding commit for a file can be found here: 
http://stackoverflow.com/questions/223678/git-which-commit-has-this-blob

Thanks for your help so far!

Thomas

PS: Yes, I have a backup copy of the repository ;-)

^ permalink raw reply

* Re: [PATCH] Fix t7606 on Cygwin: for some reasont it does not recognize a "." in PATH
From: Stefan Näwe @ 2008-12-12 14:01 UTC (permalink / raw)
  To: git
In-Reply-To: <81b0412b0812120428mc85ae84r260b722022dc3449@mail.gmail.com>

Alex Riesen <raa.lkml <at> gmail.com> writes:

> 
> The test uses the dot to add custom merge strategies
> ---
>  t/t7606-merge-custom.sh |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> Attachment (0001-Fix-t7606-on-Cygwin-it-does-not-recognize-a-.-in.patch): 
application/octet-stream, 759 bytes

Any special reason why you sent this as an (normally frowned-upon) attachment ?

For me, however, this would be the way to go to be able to download the patch 
without any hassle. (see my other post).

Regards,
  Stefan

^ permalink raw reply

* Re: Fwd: after first git clone of linux kernel repository there are changed files in working dir
From: Nick Andrew @ 2008-12-12 13:51 UTC (permalink / raw)
  To: rdkrsr; +Cc: git
In-Reply-To: <d304880b0812110958u3da52e4fs7e5154ebe9a353a@mail.gmail.com>

On Thu, Dec 11, 2008 at 06:58:01PM +0100, rdkrsr wrote:
> windows xp and msysgit for this. And the file system is NTFS. I'm
> using dual boot to sporadicly use linux and tried also linux in
> virtual box.

You could use git inside your VirtualBox linux install.

> But both isn't really good. Maybe one day I dare to use
> linux as my primary OS.

It's not scary, really.

Nick.

^ permalink raw reply

* [PATCH] Fix t7606 on Cygwin: for some reasont it does not recognize a "." in PATH
From: Alex Riesen @ 2008-12-12 12:29 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano

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

The test uses the dot to add custom merge strategies
---
 t/t7606-merge-custom.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

[-- Attachment #2: 0001-Fix-t7606-on-Cygwin-it-does-not-recognize-a-.-in.patch --]
[-- Type: application/octet-stream, Size: 759 bytes --]

From 928b2b955bd2ef0e400de698c0c0bcde5e64638b Mon Sep 17 00:00:00 2001
From: Alex Riesen <raa.lkml@gmail.com>
Date: Mon, 29 Sep 2008 17:07:00 +0200
Subject: [PATCH] Fix t7606 on Cygwin: for some reasont it does not recognize a "." in PATH

The test uses the dot to add custom merge strategies
---
 t/t7606-merge-custom.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/t/t7606-merge-custom.sh b/t/t7606-merge-custom.sh
index 52a451d..20c0907 100755
--- a/t/t7606-merge-custom.sh
+++ b/t/t7606-merge-custom.sh
@@ -11,7 +11,7 @@ cat >git-merge-theirs <<EOF
 eval git read-tree --reset -u \\\$\$#
 EOF
 chmod +x git-merge-theirs
-PATH=.:$PATH
+PATH=$(pwd):"$PATH"
 export PATH
 
 test_expect_success 'setup' '
-- 
1.6.1.rc2.48.g3c7df


^ permalink raw reply related

* Re: Clarifying "invalid tag signature file" error message from git filter-branch (and others)
From: Jim Meyering @ 2008-12-12 11:02 UTC (permalink / raw)
  To: James Youngman; +Cc: Brandon Casey, git
In-Reply-To: <c5df85930812111559p287ea6afk54a9759302288d5e@mail.gmail.com>

"James Youngman" <jay@gnu.org> wrote:
> On Thu, Dec 11, 2008 at 11:13 PM, Brandon Casey <casey@nrlssc.navy.mil> wrote:
>
>>> Before conversion:
>>> $ git cat-file tag FINDUTILS-4_1-10
>>> object ce25eb352de8dc53a9a7805ba9efc1c9215d28c2
>>> type commit
>>> tag FINDUTILS-4_1-10
>>> tagger Kevin Dalley
>>
>> The tagger field is missing an email address, a timestamp, and a timezone. It
>> should look something like:
>>
>>  tagger Kevin Dalley <kevin.dalley@somewhere.com> 1229036026 -0800
>>
>> git-mktag prevents improperly formatted tags from being created by checking
>> that these fields exist and are well formed.
>>
>> If you know the correct values for the missing fields, then you could
>
> Yes for the email address.      But as for the timestamp, it's not in
> the tag file; that only contains the sha1.
> There is a timestamp in the object being tagged, is that the timestamp
> you are talking about?
>
> $ git show --pretty=raw  ce25eb352de8dc53a9a7805ba9efc1c9215d28c2
> commit ce25eb352de8dc53a9a7805ba9efc1c9215d28c2
> tree 752cca144d39bc55d05fbe304752b274ba22641c
> parent 9a998755249b0c8c47e8657cff712fa506aa30fc
> author Kevin Dalley <kevin@seti.org> 830638152 +0000
> committer Kevin Dalley <kevin@seti.org> 830638152 +0000
>
>     *** empty log message ***
>
> diff --git a/debian.Changelog b/debian.Changelog
> index e3541eb..d0cd295 100644
> --- a/debian.Changelog
> +++ b/debian.Changelog
> @@ -1,5 +1,7 @@
>  Sat Apr 27 12:29:06 1996  Kevin Dalley
> <kevin@aplysia.iway.aimnet.com (Kevin Dalley)>
>
> +       * find.info, find.info-1, find.info-2: updated to match find.texi
> +
>         * debian.rules (debian): update debian revision to 10
>
>         * getline.c (getstr): verify that nchars_avail is *really* greater
>
>
>
>
>
>> recreate the tags before doing the filter-branch. If they are unknown, it
>> seems valid enough to use the values from the commit that the tag points
>> to.
>>
>> i.e.
>>
>>  tagger Kevin Dalley <kevin@seti.org> 830638152 -0000
>>
>> What tool was used to convert this repository to git? It should be corrected
>> to produce valid annotated tags. Especially if it is a tool within git.
>
> I don't know, Jim Meyering will know though, so I CC'ed him.

I used parsecvs, probably with git-master from the date of
the initial conversion (check the archives for actual date).
That was long enough ago that it was almost certainly before
git-mktag learned to be more strict about its inputs.

James, since you're about to rewrite the history, you may want to
start that process from a freshly-cvs-to-git-converted repository.

I'm not very happy about using cvsparse (considering it's not
really being maintained, afaik), so if the git crowd
can recommend something better, I'm all ears.

^ permalink raw reply

* [JGIT PATCH] Fix typos in comments / testcase output
From: Mike Ralphson @ 2008-12-12 10:55 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git, Mike Ralphson, Robin Rosenberg

Signed-off-by: Mike Ralphson <mike@abacus.co.uk>
---

Is it me, or is the actual maintainer of JGIT/EGIT not actually
mentioned anywhere? Apologies if I've got the to and cc round
the wrong way 8-)

This patch is the product of
find . -name "*.java" -type f -exec aspell --mode=ccpp -c {} \;

I would post my aspell wordlists but they're polluted by other
projects...

Possibly also s/packes to/packs so/ in README?

 .../org/spearce/egit/core/GitMoveDeleteHook.java   |    2 +-
 .../org/spearce/jgit/lib/RepositoryTestCase.java   |    2 +-
 .../spearce/jgit/revwalk/RevCommitParseTest.java   |    2 +-
 .../spearce/jgit/transport/BundleWriterTest.java   |    2 +-
 .../spearce/jgit/transport/FetchConnection.java    |    2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/org.spearce.egit.core/src/org/spearce/egit/core/GitMoveDeleteHook.java b/org.spearce.egit.core/src/org/spearce/egit/core/GitMoveDeleteHook.java
index cc4059c..7faa65a 100644
--- a/org.spearce.egit.core/src/org/spearce/egit/core/GitMoveDeleteHook.java
+++ b/org.spearce.egit.core/src/org/spearce/egit/core/GitMoveDeleteHook.java
@@ -122,7 +122,7 @@ public boolean moveFolder(final IResourceTree tree, final IFolder source,
 			final IFolder destination, final int updateFlags,
 			final IProgressMonitor monitor) {
 		// TODO: Implement this. Should be relatively easy, but consider that
-		// Eclipse thinks folders are real thinsgs, while Git does not care.
+		// Eclipse thinks folders are real things, while Git does not care.
 		return FINISH_FOR_ME;
 	}
 
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java
index 8937145..9e48fde 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java
@@ -94,7 +94,7 @@ protected void configure() {
 
 	/**
 	 * Utility method to delete a directory recursively. It is
-	 * also used internally. If a file or directory cannote be removed
+	 * also used internally. If a file or directory cannot be removed
 	 * it throws an AssertionFailure.
 	 *
 	 * @param dir
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevCommitParseTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevCommitParseTest.java
index 805e29e..9b95924 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevCommitParseTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevCommitParseTest.java
@@ -226,7 +226,7 @@ public void testParse_explicit_bad_encoded() throws Exception {
 	 *
 	 * What happens here is that an encoding us given, but data is not encoded
 	 * that way (and we can detect it), so we try other encodings. Here data could
-	 * actually be decoded in the stated encoding, but we overide using UTF-8.
+	 * actually be decoded in the stated encoding, but we override using UTF-8.
 	 *
 	 * @throws Exception
 	 */
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/transport/BundleWriterTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/transport/BundleWriterTest.java
index 3cfb8b1..f13d25c 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/transport/BundleWriterTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/transport/BundleWriterTest.java
@@ -117,7 +117,7 @@ assertEquals(db.resolve("c").name(), newRepo.resolve("refs/heads/cc")
 			// Check that we actually needed the first bundle
 			Repository newRepo2 = createNewEmptyRepo();
 			fetchResult = fetchFromBundle(newRepo2, bundle);
-			fail("We should not be able to fetch from bundle with prerequisistes that are not fulfilled");
+			fail("We should not be able to fetch from bundle with prerequisites that are not fulfilled");
 		} catch (MissingBundlePrerequisiteException e) {
 			assertTrue(e.getMessage()
 					.indexOf(db.resolve("refs/heads/a").name()) >= 0);
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/FetchConnection.java b/org.spearce.jgit/src/org/spearce/jgit/transport/FetchConnection.java
index 461ad71..a56ca6c 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/FetchConnection.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/FetchConnection.java
@@ -126,7 +126,7 @@ public void fetch(final ProgressMonitor monitor, final Collection<Ref> want)
 	 * able to supply a fully connected graph to the client (although it may
 	 * only be transferring the parts the client does not yet have). Its faster
 	 * to assume such remote peers are well behaved and send the correct
-	 * response to the client. In such tranports this method returns false.
+	 * response to the client. In such transports this method returns false.
 	 *
 	 * @return true if the last fetch had to perform a connectivity check on the
 	 *         client side in order to succeed; false if the last fetch assumed
-- 
1.6.0.2

^ permalink raw reply related

* Re: [PATCH v3 3/3] gitweb: Optional grouping of projects by category
From: Jakub Narebski @ 2008-12-12  9:26 UTC (permalink / raw)
  To: Sébastien Cevey
  Cc: git, Junio C Hamano, Petr Baudis, Gustavo Sverzut Barbieri
In-Reply-To: <87y6ymxf4k.wl%seb@cine7.net>

On Fri, 12 Dec 2008, Sébastien Cevey wrote:
> At Fri, 12 Dec 2008 03:03:55 +0100, Jakub Narebski wrote:
> 
> > And no, we don't need to sort by categories first.  Let me explain
> > in more detail a bit.
> 
> Thanks for the detailed explanation, I understand your preference.
> But as you said, it's a bit arbitrary, I think none is completely
> obvious.

First, I feel a bit bad for derailing this patch. Currently gitweb
does not do pagination of projects list; it is not even possible in
a sane way with current way project searching/selecting is implemented.
So the whole build_projlist_by_category() respecting $from and $to is
moot point.

So if we don't use it, even if it is nice to have for the future, we
don't need to pay cost of extra stable sorting.

> 
> I don't really have a strong opinion about which is best, but just to
> illustrate what made me go for the solution B, let me show another
> example:
> 
> name / date / cat
> 1      2006    A
> 2      2003    B
> 3      2005    B
> 4      2003    A
> 5      2000    A
> 6      2008    C
> 7      2007    C
> 8      2001    B
> 9      2005    A
> 
> We then sort by name and split in pages of N=3 (sorted by cat on each
> page):
> 
> A:sort(name) split(max=3) subsort(cat)
>   1  2006  A     4  2003  A     9  2005  A
>   2  2003  B     5  2000  A     8  2001  B
>   3  2005  B     6  2008  C     7  2007  C
> 
> B:sort(cat) subsort(name) split(max=3)
>   1  2006  A     9  2005  A     8  2001  B
>   4  2003  A     2  2003  B     6  2008  C
>   5  2000  A     3  2005  B     7  2007  C
> 
> With B, we have the second top-entry (2) relegated to the second page,
> which might be surprising because we ordered by name.  But what I find
> weird about A is that because of the per-page category sorting, the
> "top-sorted entries at the top" isn't true either (page 3).  We have
> "reshuffled" the result of 'sort(name) split(max=3)' on each page.

[...]
> It's perhaps even more apparent if we sort by date:
> 
> A:sort(year) split(max=3) subsort(cat)
>   1  2006  A     9  2005  A     4  2003  A
>   6  2008  C     3  2005  B     5  2000  A
>   7  2007  C     2  2003  B     8  2001  B
> 
> B:sort(cat) subsort(year) split(max=4)
>   1  2006  A     4  2003  A     3  2005  B
>   9  2005  A     8  2001  B     7  2007  C
>   5  2000  A     2  2003  B     6  2008  C
> 
> It feels kind of unnatural that not only projects are not sorted by
> date on each page (they are inside categories), but moreover
> categories are spread over all pages.
> 
> 
> I guess it depends on your use case, and whether categories are
> important or known by the user.  I personally don't really care (I
> never split stuff into pages in the gitweb I use), so I can do a new
> version of my patch that does A if you prefer, just let me know.  I
> just wanted to clarify that both solutions sort of suck :-)

Well, with version A you can (I think) simply change

  foreach my $cat (sort keys %categories) {

to

  foreach my $cat (sort 
  	{ cmp_cat($projlist, \%categories, $oi, $a, $b) } keys %categories) {

to have the following output (see the difference on page 3)

A':sort(name) split(max=3) subsort(sort(cat,name))
  1  2006  A     4  2003  A     7  2007  C
  2  2003  B     5  2000  A     8  2001  B
  3  2005  B     6  2008  C     9  2005  A

where sort_cat might be something like (we took advantage that
categories in %categories have at least one project):

  sub cmp_cat {
  	my ($projlist, $cats, $oi, $a, $b) = @_;
  	my ($aa, $bb);
  	# projects in categories are sorted, so we can compare first
  	# project from a category to sort categories in given ordering
  	$aa = $projlist->{$cats->{$a}[0]};
  	$bb = $projlist->{$cats->{$b}[0]};
  	if ($oi->{'type'} eq 'str') {
		return $aa->{$oi->{'key'}} cmp $bb->{$oi->{'key'}};
	} else {
		return $aa->{$oi->{'key'}} <=> $bb->{$oi->{'key'}};
	}
  }

-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: Saving patches from this list
From: Mike Ralphson @ 2008-12-12  9:07 UTC (permalink / raw)
  To: Stefan Näwe; +Cc: git, Johannes Sixt, Junio C Hamano
In-Reply-To: <loom.20081212T082629-274@post.gmane.org>

2008/12/12 Stefan Näwe <stefan.naewe+git@gmail.com>
>
> Johannes Sixt <j.sixt <at> viscovery.net> writes:
>
> >
> > Stefan Näwe schrieb:
> > > What's the best way to get patches sent to this list in a form suitable
> > > for 'git am' without subscribing to this list ?
> >
> > Subscribe to gmane.comp.version-control.git on news.gmane.org with your
> > favorite news reader and browse the list whenever you feel like it.
>
> Do you know how stubborn firewall administrators can be ?
>
> IOW, that's unfortunately not an option for me.

If it's only the occasional patch - (I see you're on gmail too), show
original and copy-and-paste into an editor with tabs set up
appropriately. Works for me.

Junio's blog[1] shows he's looking at patchwork. Personally I think it
would be fantastic to have a public patchwork server available. It
might avoid the chicken and egg problem in that it's currently easier
(for some people) to get hold of a patch to play with / review only
after it's accepted.

That said, I think including a link to a repo/branch from which the
current version of a patch series could be fetched would be an
amazingly useful addition to most [PATCH 0/n] cover-letters...

Mike

[1] http://gitster.livejournal.com/18696.html

^ permalink raw reply

* [PATCH] git-config.txt: fix a typo
From: Jim Meyering @ 2008-12-12  9:00 UTC (permalink / raw)
  To: git list


---
 Documentation/git-config.txt |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 28e1861..19a8917 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -279,7 +279,7 @@ If you want to know all the values for a multivar, do:
 % git config --get-all core.gitproxy
 ------------

-If you like to live dangerous, you can replace *all* core.gitproxy by a
+If you like to live dangerously, you can replace *all* core.gitproxy by a
 new one with

 ------------
--
1.6.1.rc2.299.gead4c

^ permalink raw reply related

* Re: Saving patches from this list
From: Stefan Näwe @ 2008-12-12  8:28 UTC (permalink / raw)
  To: git
In-Reply-To: <49421AEE.8090902@viscovery.net>

Johannes Sixt <j.sixt <at> viscovery.net> writes:

> 
> Stefan Näwe schrieb:
> > What's the best way to get patches sent to this list in a form suitable
> > for 'git am' without subscribing to this list ?
> 
> Subscribe to gmane.comp.version-control.git on news.gmane.org with your
> favorite news reader and browse the list whenever you feel like it.

Do you know how stubborn firewall administrators can be ?

IOW, that's unfortunately not an option for me.

Regards,
  Stefan

^ permalink raw reply

* Re: Saving patches from this list
From: Johannes Sixt @ 2008-12-12  8:03 UTC (permalink / raw)
  To: Stefan Näwe; +Cc: git
In-Reply-To: <loom.20081212T072326-350@post.gmane.org>

Stefan Näwe schrieb:
> What's the best way to get patches sent to this list in a form suitable
> for 'git am' without subscribing to this list ?

Subscribe to gmane.comp.version-control.git on news.gmane.org with your
favorite news reader and browse the list whenever you feel like it.

-- Hannes

^ permalink raw reply

* Saving patches from this list
From: Stefan Näwe @ 2008-12-12  7:27 UTC (permalink / raw)
  To: git

I have some kind of a 'meta question' not exactly regarding git.

What's the best way to get patches sent to this list in a form suitable
for 'git am' without subscribing to this list ?


TIA
   Stefan

^ permalink raw reply

* Re: What's cooking in git.git (Nov 2008, #06; Wed, 26)
From: Jeff King @ 2008-12-12  3:36 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Daniel Barkalow, Nguyen Thai Ngoc Duy, Shawn O. Pearce,
	Johannes Schindelin, git
In-Reply-To: <7vmyf29jd6.fsf@gitster.siamese.dyndns.org>

On Thu, Dec 11, 2008 at 07:12:53PM -0800, Junio C Hamano wrote:

> Sure, but as "sparse" does not (again, "it should not, at least to me")
> change the fact that git is about tracking the history of whole tree, not
> just a single file, nor just a subset of files, none of these operations
> should be affected about what the checkout area is.

I agree with Junio here. If you want "git grep foo HEAD^" to ignore
certain files, then sparse _checkout_ is not the right feature. In that
case you want a sparse _repo_, which is not something I think anybody is
seriously working on.

-Peff

^ permalink raw reply

* Re: git-doc CSS dependent, breaks down in text browsers
From: Jeff King @ 2008-12-12  3:33 UTC (permalink / raw)
  To: jidanni; +Cc: git, 507475
In-Reply-To: <87wse6zc9x.fsf@jidanni.org>

On Fri, Dec 12, 2008 at 04:29:14AM +0800, jidanni@jidanni.org wrote:

> E.g., pages look like
> 
> SYNOPSIS
> 
> git-config [<file-option>] [type] [-z|--null] name [value [value_regex]] git-config [<file-option>] [type] --add name
> value git-config [<file-option>] [type] --replace-all name [value [value_regex]] git-config [<file-option>] [type] [-z|
> --null] --get name [value_regex] git-config [<file-option>] [type] [-z|--null] --get-all name [value_regex] git-config...

I think this is another asciidoc issue, as git merely specifies "verse"
format for this section. Probably the most friendly thing to do would be
to use

  <pre class="verseblock-content">

instead of

  <div class="verseblock-content">

so that non-CSS browsers fall back to preserving the line boundaries
(which is what is making it look so unbearable in your text browser).
But it is definitely something to be fixed in asciidoc, not in the git
documentation.

-Peff

^ permalink raw reply

* Re: What's cooking in git.git (Nov 2008, #06; Wed, 26)
From: Junio C Hamano @ 2008-12-12  3:12 UTC (permalink / raw)
  To: Daniel Barkalow
  Cc: Nguyen Thai Ngoc Duy, Shawn O. Pearce, Johannes Schindelin, git
In-Reply-To: <alpine.LNX.1.00.0812112045120.19665@iabervon.org>

Daniel Barkalow <barkalow@iabervon.org> writes:

> "git diff" is an ambiguous model for "git grep". It equally describes 
> the behavior of "git diff" to say that it treats files outside the 
> checkout area as matching the index or to say that it never lists files 
> outside the checkout area. On the other hand, there is the question of 
> whether "git diff branch1 branch2" shows differences that are outside the 
> checkout area, and whether "git log" shows commits that only change things 
> outside the checkout area, and "git grep" should match the behavior of 
> these.

Sure, but as "sparse" does not (again, "it should not, at least to me")
change the fact that git is about tracking the history of whole tree, not
just a single file, nor just a subset of files, none of these operations
should be affected about what the checkout area is.

^ permalink raw reply

* Re: [PATCH v3 3/3] gitweb: Optional grouping of projects by category
From: Sébastien Cevey @ 2008-12-12  3:10 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git, Junio C Hamano, Petr Baudis, Gustavo Sverzut Barbieri
In-Reply-To: <200812120303.56997.jnareb@gmail.com>

At Fri, 12 Dec 2008 03:03:55 +0100, Jakub Narebski wrote:

> And no, we don't need to sort by categories first.  Let me explain
> in more detail a bit.

Thanks for the detailed explanation, I understand your preference.
But as you said, it's a bit arbitrary, I think none is completely
obvious.


I don't really have a strong opinion about which is best, but just to
illustrate what made me go for the solution B, let me show another
example:

name / date / cat
1      2006    A
2      2003    B
3      2005    B
4      2003    A
5      2000    A
6      2008    C
7      2007    C
8      2001    B
9      2005    A

We then sort by name and split in pages of N=3 (sorted by cat on each
page):

A:sort(name) split(max=3) subsort(cat)
  1  2006  A     4  2003  A     9  2005  A
  2  2003  B     5  2000  A     8  2001  B
  3  2005  B     6  2008  C     7  2007  C

B:sort(cat) subsort(name) split(max=4)
  1  2006  A     9  2005  A     8  2001  B
  4  2003  A     2  2003  B     6  2008  C
  5  2000  A     3  2005  B     7  2007  C

With B, we have the second top-entry (2) relegated to the second page,
which might be surprising because we ordered by name.  But what I find
weird about A is that because of the per-page category sorting, the
"top-sorted entries at the top" isn't true either (page 3).  We have
"reshuffled" the result of 'sort(name) split(max=3)' on each page.

To be truly fair to the main sorting, we should not try to group
categories and display the header for each consecutive group of
entries from a distinct category:

A-:sort(name) split(max=3)
  1  2006  A     4  2003  A     7  2007  C
  2  2003  B     5  2000  A     8  2001  B
  3  2005  B     6  2008  C     9  2005  A

Which in this case is painless as it only affects page 3, but it could
lead to a mess of interleaved categories, and we kind of lose the
purpose of category groups in the first place..

The point is that with A, you cannot determine whether you're on the
right page to find project P (even if you know its category) by
checking whether it's in the interval between the top and bottom
entries.


It's perhaps even more apparent if we sort by date:

A:sort(year) split(max=3) subsort(cat)
  1  2006  A     9  2005  A     4  2003  A
  6  2008  C     3  2005  B     5  2000  A
  7  2007  C     2  2003  B     8  2001  B

B:sort(cat) subsort(year) split(max=4)
  1  2006  A     4  2003  A     3  2005  B
  9  2005  A     8  2001  B     7  2007  C
  5  2000  A     2  2003  B     6  2008  C

It feels kind of unnatural that not only projects are not sorted by
date on each page (they are inside categories), but moreover
categories are spread over all pages.


I guess it depends on your use case, and whether categories are
important or known by the user.  I personally don't really care (I
never split stuff into pages in the gitweb I use), so I can do a new
version of my patch that does A if you prefer, just let me know.  I
just wanted to clarify that both solutions sort of suck :-)

> P.S. It is IMHO better to use
> 
>  	for (my $i = $from; $i <= $to; $i++) {

Ah sorry, I took the words from your earlier email too literally ("we
can [...] use loop $from..$to there"). That's what happens when
pseudocode is actually valid syntax in a language!

Cheers,

-- 
Sébastien Cevey / inso.cc

^ permalink raw reply

* Re: [PATCH 2/3 (edit v2)] gitweb: Cache $parent_commit info in git_blame()
From: Junio C Hamano @ 2008-12-12  3:05 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Nanako Shiraishi, git, Luben Tuikov
In-Reply-To: <200812110133.33124.jnareb@gmail.com>

Jakub Narebski <jnareb@gmail.com> writes:

> ...
> Alternate solutions:
> ~~~~~~~~~~~~~~~~~~~~
> ...
> Acked-by: Luben Tuikov <ltuikov@yahoo.com>
> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
> ---
> On Wed, 10 Dec 2008, Junio C Hamano wrote:
>
>> To recap, I think the commit log for this patch would have been much
>> easier to read if it were presented in this order:
>> 
>>         a paragraph to establish the context;
>> 
>>         a paragraph to state what problem it tries to solve;
>> 
>>         a paragraph (or more) to explain the solution; and finally
>> 
>>         a paragraph to discuss possible future enhancements.
>
> Like this?

Yes, basically.

The "future possibilities" section might be a bit too heavy, and also
calling it "Alternate solutions" makes it slightly unclear if it is
talking about what is implemented, or only talking about idle speculation
without an actual code (in this case, it is the latter), though.

> Only commit message has changed.

Which is a bit unnice, because it will conflict with the original [3/3]
that I queued already (with a pair of fixes, including but not limited to
the one you sent "Oops, it should have been like this" for).

I can hand wiggle the patch to make it apply, but I'd prefer if I did not
have to do this every time I receive a patch.

I think the conflict was trivial (just a single s/rev/short_rev/) and I
did not make a silly mistake when I fixed it up, but please check the
result on 'pu' after I push the results out.

Thanks.

^ permalink raw reply

* Re: user-manual.html invalid HTML
From: jidanni @ 2008-12-12  2:47 UTC (permalink / raw)
  To: peff; +Cc: git, 507476
In-Reply-To: <20081212023003.GD23128@sigill.intra.peff.net>

>>>>> "JK" == Jeff King <peff@peff.net> writes:

JK> On Fri, Dec 12, 2008 at 04:32:15AM +0800, jidanni@jidanni.org wrote:
>> Please see http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=507476
>> Which it turns out didn't get forwarded to git@vger.kernel.org after all. 

JK> The versions I build locally have:

JK> <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
JK>     "http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">

JK> in each HTML file, which is added by asciidoc.  Maybe the package you
JK> are looking at was built with an older version of asciidoc that doesn't
JK> do this (I don't actually know the history of this feature, but it seems
JK> to me that this is something asciidoc should be doing, not git).

JK> -Peff

OK, adding 507476@bugs.debian.org to the CCs.

^ permalink raw reply

* [JGIT PATCH 15/15] Treat "diff --combined" the same as "diff --cc"
From: Shawn O. Pearce @ 2008-12-12  2:46 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <1229049981-14152-15-git-send-email-spearce@spearce.org>

According to the git diff manual page these two formats
share the same file structure, so we can parse them with
the same function.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../src/org/spearce/jgit/patch/Patch.java          |   15 +++++++++++----
 1 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/patch/Patch.java b/org.spearce.jgit/src/org/spearce/jgit/patch/Patch.java
index e1e79b7..77ae02f 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/patch/Patch.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/patch/Patch.java
@@ -57,6 +57,8 @@
 
 	private static final byte[] DIFF_CC = encodeASCII("diff --cc ");
 
+	private static final byte[] DIFF_COMBINED = encodeASCII("diff --combined ");
+
 	private static final byte[][] BIN_HEADERS = new byte[][] {
 			encodeASCII("Binary files "), encodeASCII("Files "), };
 
@@ -177,7 +179,9 @@ private int parseFile(final byte[] buf, int c, final int end) {
 			if (match(buf, c, DIFF_GIT) >= 0)
 				return parseDiffGit(buf, c, end);
 			if (match(buf, c, DIFF_CC) >= 0)
-				return parseDiffCC(buf, c, end);
+				return parseDiffCombined(DIFF_CC, buf, c, end);
+			if (match(buf, c, DIFF_COMBINED) >= 0)
+				return parseDiffCombined(DIFF_COMBINED, buf, c, end);
 
 			// Junk between files? Leading junk? Traditional
 			// (non-git generated) patch?
@@ -227,9 +231,10 @@ private int parseDiffGit(final byte[] buf, final int start, final int end) {
 		return ptr;
 	}
 
-	private int parseDiffCC(final byte[] buf, final int start, final int end) {
-		final FileHeader fh = new FileHeader(buf, start);
-		int ptr = fh.parseGitFileName(start + DIFF_CC.length, end);
+	private int parseDiffCombined(final byte[] hdr, final byte[] buf,
+			final int start, final int end) {
+		final CombinedFileHeader fh = new CombinedFileHeader(buf, start);
+		int ptr = fh.parseGitFileName(start + hdr.length, end);
 		if (ptr < 0)
 			return skipFile(buf, start, end);
 
@@ -269,6 +274,8 @@ private int parseHunks(final FileHeader fh, int c, final int end) {
 				break;
 			if (match(buf, c, DIFF_CC) >= 0)
 				break;
+			if (match(buf, c, DIFF_COMBINED) >= 0)
+				break;
 			if (match(buf, c, OLD_NAME) >= 0)
 				break;
 			if (match(buf, c, NEW_NAME) >= 0)
-- 
1.6.1.rc2.306.ge5d5e

^ permalink raw reply related

* [JGIT PATCH 14/15] Abstract the hunk header testing into a method
From: Shawn O. Pearce @ 2008-12-12  2:46 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <1229049981-14152-14-git-send-email-spearce@spearce.org>

This way we can test for "@@@ -" and "@@@@@@@ -" for combined
diffs in octopus merges.  We use the same scan test for the
basic two-way case, as the format is identical but has less
marker characters.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../src/org/spearce/jgit/patch/FileHeader.java     |   36 +++++++++++++++++--
 .../src/org/spearce/jgit/patch/Patch.java          |    8 ++--
 2 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/patch/FileHeader.java b/org.spearce.jgit/src/org/spearce/jgit/patch/FileHeader.java
index a58e978..5fe2acf 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/patch/FileHeader.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/patch/FileHeader.java
@@ -87,8 +87,6 @@
 
 	static final byte[] NEW_NAME = encodeASCII("+++ ");
 
-	static final byte[] HUNK_HDR = encodeASCII("@@ -");
-
 	/** General type of change a single file-level patch describes. */
 	public static enum ChangeType {
 		/** Add a new file to the project */
@@ -358,7 +356,7 @@ int parseGitFileName(int ptr, final int end) {
 	int parseGitHeaders(int ptr, final int end) {
 		while (ptr < end) {
 			final int eol = nextLF(buf, ptr);
-			if (match(buf, ptr, HUNK_HDR) >= 0) {
+			if (isHunkHdr(buf, ptr, eol) >= 1) {
 				// First hunk header; break out and parse them later.
 				break;
 
@@ -432,7 +430,7 @@ int parseGitHeaders(int ptr, final int end) {
 	int parseTraditionalHeaders(int ptr, final int end) {
 		while (ptr < end) {
 			final int eol = nextLF(buf, ptr);
-			if (match(buf, ptr, HUNK_HDR) >= 0) {
+			if (isHunkHdr(buf, ptr, eol) >= 1) {
 				// First hunk header; break out and parse them later.
 				break;
 
@@ -519,4 +517,34 @@ private boolean eq(int aPtr, int aEnd, int bPtr, int bEnd) {
 		}
 		return true;
 	}
+
+	/**
+	 * Determine if this is a patch hunk header.
+	 * 
+	 * @param buf
+	 *            the buffer to scan
+	 * @param start
+	 *            first position in the buffer to evaluate
+	 * @param end
+	 *            last position to consider; usually the end of the buffer (
+	 *            <code>buf.length</code>) or the first position on the next
+	 *            line. This is only used to avoid very long runs of '@' from
+	 *            killing the scan loop.
+	 * @return the number of "ancestor revisions" in the hunk header. A
+	 *         traditional two-way diff ("@@ -...") returns 1; a combined diff
+	 *         for a 3 way-merge returns 3. If this is not a hunk header, 0 is
+	 *         returned instead.
+	 */
+	static int isHunkHdr(final byte[] buf, final int start, final int end) {
+		int ptr = start;
+		while (ptr < end && buf[ptr] == '@')
+			ptr++;
+		if (ptr - start < 2)
+			return 0;
+		if (ptr == end || buf[ptr++] != ' ')
+			return 0;
+		if (ptr == end || buf[ptr++] != '-')
+			return 0;
+		return (ptr - 3) - start;
+	}
 }
diff --git a/org.spearce.jgit/src/org/spearce/jgit/patch/Patch.java b/org.spearce.jgit/src/org/spearce/jgit/patch/Patch.java
index 9aca22d..e1e79b7 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/patch/Patch.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/patch/Patch.java
@@ -38,7 +38,7 @@
 package org.spearce.jgit.patch;
 
 import static org.spearce.jgit.lib.Constants.encodeASCII;
-import static org.spearce.jgit.patch.FileHeader.HUNK_HDR;
+import static org.spearce.jgit.patch.FileHeader.isHunkHdr;
 import static org.spearce.jgit.patch.FileHeader.NEW_NAME;
 import static org.spearce.jgit.patch.FileHeader.OLD_NAME;
 import static org.spearce.jgit.util.RawParseUtils.match;
@@ -162,7 +162,7 @@ public void parse(final byte[] buf, int ptr, final int end) {
 
 	private int parseFile(final byte[] buf, int c, final int end) {
 		while (c < end) {
-			if (match(buf, c, HUNK_HDR) >= 0) {
+			if (isHunkHdr(buf, c, end) >= 1) {
 				// If we find a disconnected hunk header we might
 				// have missed a file header previously. The hunk
 				// isn't valid without knowing where it comes from.
@@ -205,7 +205,7 @@ private int parseFile(final byte[] buf, int c, final int end) {
 				final int f = nextLF(buf, n);
 				if (f >= end)
 					return end;
-				if (match(buf, f, HUNK_HDR) >= 0)
+				if (isHunkHdr(buf, f, end) == 1)
 					return parseTraditionalPatch(buf, c, end);
 			}
 
@@ -274,7 +274,7 @@ private int parseHunks(final FileHeader fh, int c, final int end) {
 			if (match(buf, c, NEW_NAME) >= 0)
 				break;
 
-			if (match(buf, c, HUNK_HDR) >= 0) {
+			if (isHunkHdr(buf, c, end) == 1) {
 				final HunkHeader h = new HunkHeader(fh, c);
 				h.parseHeader(end);
 				c = h.parseBody(this, end);
-- 
1.6.1.rc2.306.ge5d5e

^ permalink raw reply related

* [JGIT PATCH 13/15] Patch parse test comparing "git log -p" output to "git log --numstat"
From: Shawn O. Pearce @ 2008-12-12  2:46 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <1229049981-14152-13-git-send-email-spearce@spearce.org>

By comparing the output of "git log -p", once parsed by our patch
parser class, to the output of "git log --numstat" we can be quite
certain we are reading the patches from Git with a high degree of
accuracy, at least for typical add/remove sorts of changes (no
rename detection).

Unfortunately two commits in our history produce an off-by-one bug
in git log --numstat.  The bug appears to be in log --numstat and
not in JGit as git apply --numstat matches JGit's result, and is
thus also differing from log --numstat.  Since this occurs on only
2 commits out of 1,211 processed during the test I'm not worrying
about the difference on these two items.  Besides the numbers from
JGit and git apply --numstat look to be more correct.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../spearce/jgit/patch/EGitPatchHistoryTest.java   |  221 ++++++++++++++++++++
 1 files changed, 221 insertions(+), 0 deletions(-)
 create mode 100644 org.spearce.jgit.test/exttst/org/spearce/jgit/patch/EGitPatchHistoryTest.java

diff --git a/org.spearce.jgit.test/exttst/org/spearce/jgit/patch/EGitPatchHistoryTest.java b/org.spearce.jgit.test/exttst/org/spearce/jgit/patch/EGitPatchHistoryTest.java
new file mode 100644
index 0000000..d0c2632
--- /dev/null
+++ b/org.spearce.jgit.test/exttst/org/spearce/jgit/patch/EGitPatchHistoryTest.java
@@ -0,0 +1,221 @@
+/*
+ * Copyright (C) 2008, Google Inc.
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
+ *
+ * - Neither the name of the Git Development Community nor the
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package org.spearce.jgit.patch;
+
+import java.io.BufferedReader;
+import java.io.IOException;
+import java.io.InputStreamReader;
+import java.io.UnsupportedEncodingException;
+import java.util.HashMap;
+import java.util.HashSet;
+
+import junit.framework.TestCase;
+
+import org.spearce.jgit.lib.Constants;
+import org.spearce.jgit.util.MutableInteger;
+import org.spearce.jgit.util.RawParseUtils;
+import org.spearce.jgit.util.TemporaryBuffer;
+
+public class EGitPatchHistoryTest extends TestCase {
+	public void testParseHistory() throws Exception {
+		final NumStatReader numstat = new NumStatReader();
+		numstat.read();
+
+		final HashMap<String, HashMap<String, StatInfo>> stats = numstat.stats;
+		assertEquals(1211, stats.size());
+
+		new PatchReader(stats).read();
+	}
+
+	static class StatInfo {
+		int added, deleted;
+	}
+
+	static class PatchReader extends CommitReader {
+		final HashSet<String> offBy1;
+
+		final HashMap<String, HashMap<String, StatInfo>> stats;
+
+		int errors;
+
+		PatchReader(final HashMap<String, HashMap<String, StatInfo>> s)
+				throws IOException {
+			super(new String[] { "-p" });
+			stats = s;
+
+			offBy1 = new HashSet<String>();
+			offBy1.add("9bda5ece6806cd797416eaa47c7b927cc6e9c3b2");
+		}
+
+		@Override
+		void onCommit(String cid, byte[] buf) {
+			final HashMap<String, StatInfo> files = stats.remove(cid);
+			assertNotNull("No files for " + cid, files);
+
+			final Patch p = new Patch();
+			p.parse(buf, 0, buf.length - 1);
+			assertEquals("File count " + cid, files.size(), p.getFiles().size());
+			if (!p.getErrors().isEmpty()) {
+				for (final FormatError e : p.getErrors()) {
+					System.out.println("error " + e.getMessage());
+					System.out.println("  at " + e.getLineText());
+				}
+				dump(buf);
+				fail("Unexpected error in " + cid);
+			}
+
+			for (final FileHeader fh : p.getFiles()) {
+				final String fileName;
+				if (fh.getChangeType() != FileHeader.ChangeType.DELETE)
+					fileName = fh.getNewName();
+				else
+					fileName = fh.getOldName();
+				final StatInfo s = files.remove(fileName);
+				final String nid = fileName + " in " + cid;
+				assertNotNull("No " + nid, s);
+				int added = 0, deleted = 0;
+				for (final HunkHeader h : fh.getHunks()) {
+					added += h.getLinesAdded();
+					deleted += h.getLinesDeleted();
+				}
+
+				if (s.added == added) {
+					//
+				} else if (s.added == added + 1 && offBy1.contains(cid)) {
+					//
+				} else {
+					dump(buf);
+					assertEquals("Added diff in " + nid, s.added, added);
+				}
+
+				if (s.deleted == deleted) {
+					//
+				} else if (s.deleted == deleted + 1 && offBy1.contains(cid)) {
+					//
+				} else {
+					dump(buf);
+					assertEquals("Deleted diff in " + nid, s.deleted, deleted);
+				}
+			}
+			assertTrue("Missed files in " + cid, files.isEmpty());
+		}
+
+		private static void dump(final byte[] buf) {
+			String str;
+			try {
+				str = new String(buf, 0, buf.length - 1, "ISO-8859-1");
+			} catch (UnsupportedEncodingException e) {
+				throw new RuntimeException(e);
+			}
+			System.out.println("<<" + str + ">>");
+		}
+	}
+
+	static class NumStatReader extends CommitReader {
+		final HashMap<String, HashMap<String, StatInfo>> stats = new HashMap<String, HashMap<String, StatInfo>>();
+
+		NumStatReader() throws IOException {
+			super(new String[] { "--numstat" });
+		}
+
+		@Override
+		void onCommit(String commitId, byte[] buf) {
+			final HashMap<String, StatInfo> files = new HashMap<String, StatInfo>();
+			final MutableInteger ptr = new MutableInteger();
+			while (ptr.value < buf.length) {
+				if (buf[ptr.value] == '\n')
+					break;
+				final StatInfo i = new StatInfo();
+				i.added = RawParseUtils.parseBase10(buf, ptr.value, ptr);
+				i.deleted = RawParseUtils.parseBase10(buf, ptr.value + 1, ptr);
+				final int eol = RawParseUtils.nextLF(buf, ptr.value);
+				final String name = RawParseUtils.decode(Constants.CHARSET,
+						buf, ptr.value + 1, eol - 1);
+				files.put(name, i);
+				ptr.value = eol;
+			}
+			stats.put(commitId, files);
+		}
+	}
+
+	static abstract class CommitReader {
+		private Process proc;
+
+		CommitReader(final String[] args) throws IOException {
+			final String[] realArgs = new String[3 + args.length + 1];
+			realArgs[0] = "git";
+			realArgs[1] = "log";
+			realArgs[2] = "--pretty=format:commit %H";
+			System.arraycopy(args, 0, realArgs, 3, args.length);
+			realArgs[3 + args.length] = "a4b98ed15ea5f165a7aa0f2fd2ea6fcce6710925";
+
+			proc = Runtime.getRuntime().exec(realArgs);
+			proc.getOutputStream().close();
+			proc.getErrorStream().close();
+		}
+
+		void read() throws IOException, InterruptedException {
+			final BufferedReader in = new BufferedReader(new InputStreamReader(
+					proc.getInputStream(), "ISO-8859-1"));
+			String commitId = null;
+			TemporaryBuffer buf = null;
+			for (;;) {
+				String line = in.readLine();
+				if (line == null)
+					break;
+				if (line.startsWith("commit ")) {
+					if (buf != null) {
+						buf.close();
+						onCommit(commitId, buf.toByteArray());
+						buf.destroy();
+					}
+					commitId = line.substring("commit ".length());
+					buf = new TemporaryBuffer();
+				} else if (buf != null) {
+					buf.write(line.getBytes("ISO-8859-1"));
+					buf.write('\n');
+				}
+			}
+			in.close();
+			assertEquals(0, proc.waitFor());
+			proc = null;
+		}
+
+		abstract void onCommit(String commitId, byte[] buf);
+	}
+}
-- 
1.6.1.rc2.306.ge5d5e

^ permalink raw reply related

* [JGIT PATCH 12/15] Correctly handle hunk headers such as "@@ -0,0 +1 @@"
From: Shawn O. Pearce @ 2008-12-12  2:46 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <1229049981-14152-12-git-send-email-spearce@spearce.org>

Sometimes these are created for single line file creations.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../src/org/spearce/jgit/patch/HunkHeader.java     |   14 ++++++++++++--
 1 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/patch/HunkHeader.java b/org.spearce.jgit/src/org/spearce/jgit/patch/HunkHeader.java
index 4fd9bae..c3bd642 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/patch/HunkHeader.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/patch/HunkHeader.java
@@ -126,10 +126,20 @@ void parseHeader(final int end) {
 		final MutableInteger ptr = new MutableInteger();
 		ptr.value = nextLF(buf, startOffset, ' ');
 		oldStartLine = -parseBase10(buf, ptr.value, ptr);
-		oldLineCount = parseBase10(buf, ptr.value + 1, ptr);
+		if (buf[ptr.value] == ',')
+			oldLineCount = parseBase10(buf, ptr.value + 1, ptr);
+		else {
+			oldLineCount = oldStartLine;
+			oldStartLine = 0;
+		}
 
 		newStartLine = parseBase10(buf, ptr.value + 1, ptr);
-		newLineCount = parseBase10(buf, ptr.value + 1, ptr);
+		if (buf[ptr.value] == ',')
+			newLineCount = parseBase10(buf, ptr.value + 1, ptr);
+		else {
+			newLineCount = newStartLine;
+			newStartLine = 0;
+		}
 	}
 
 	int parseBody(final Patch script, final int end) {
-- 
1.6.1.rc2.306.ge5d5e

^ permalink raw reply related

* [JGIT PATCH 10/15] Record patch parsing errors for later inspection by applications
From: Shawn O. Pearce @ 2008-12-12  2:46 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <1229049981-14152-10-git-send-email-spearce@spearce.org>

Errors identified while reading a patch script are now collected
into FormatError objects within the errors collection of a Patch.
These can be inspected to determine if a common form of breakage
is found within the patch script once its basic metadata is read.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../tst/org/spearce/jgit/patch/PatchErrorTest.java |  174 ++++++++++++++++++++
 .../tst/org/spearce/jgit/patch/PatchTest.java      |    4 +
 .../spearce/jgit/patch/testError_BodyTooLong.patch |   17 ++
 .../jgit/patch/testError_DisconnectedHunk.patch    |   30 ++++
 .../jgit/patch/testError_GarbageBetweenFiles.patch |   33 ++++
 .../patch/testError_GitBinaryNoForwardHunk.patch   |   10 +
 .../jgit/patch/testError_TruncatedNew.patch        |   15 ++
 .../jgit/patch/testError_TruncatedOld.patch        |   15 ++
 .../src/org/spearce/jgit/patch/FormatError.java    |   95 +++++++++++
 .../src/org/spearce/jgit/patch/HunkHeader.java     |   20 ++-
 .../src/org/spearce/jgit/patch/Patch.java          |   38 ++++-
 11 files changed, 441 insertions(+), 10 deletions(-)
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/patch/PatchErrorTest.java
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_BodyTooLong.patch
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_DisconnectedHunk.patch
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_GarbageBetweenFiles.patch
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_GitBinaryNoForwardHunk.patch
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_TruncatedNew.patch
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_TruncatedOld.patch
 create mode 100644 org.spearce.jgit/src/org/spearce/jgit/patch/FormatError.java

diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/patch/PatchErrorTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/PatchErrorTest.java
new file mode 100644
index 0000000..3d7e6b2
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/PatchErrorTest.java
@@ -0,0 +1,174 @@
+/*
+ * Copyright (C) 2008, Google Inc.
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
+ *
+ * - Neither the name of the Git Development Community nor the
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package org.spearce.jgit.patch;
+
+import java.io.IOException;
+import java.io.InputStream;
+
+import junit.framework.TestCase;
+
+public class PatchErrorTest extends TestCase {
+	public void testError_DisconnectedHunk() throws IOException {
+		final Patch p = parseTestPatchFile();
+		assertEquals(1, p.getFiles().size());
+		{
+			final FileHeader fh = p.getFiles().get(0);
+			assertEquals(
+					"org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java",
+					fh.getNewName());
+			assertEquals(1, fh.getHunks().size());
+		}
+
+		assertEquals(1, p.getErrors().size());
+		final FormatError e = p.getErrors().get(0);
+		assertSame(FormatError.Severity.ERROR, e.getSeverity());
+		assertEquals("Hunk disconnected from file", e.getMessage());
+		assertEquals(18, e.getOffset());
+		assertTrue(e.getLineText().startsWith("@@ -109,4 +109,11 @@ assert"));
+	}
+
+	public void testError_TruncatedOld() throws IOException {
+		final Patch p = parseTestPatchFile();
+		assertEquals(1, p.getFiles().size());
+		assertEquals(1, p.getErrors().size());
+
+		final FormatError e = p.getErrors().get(0);
+		assertSame(FormatError.Severity.ERROR, e.getSeverity());
+		assertEquals("Truncated hunk, at least 1 old lines is missing", e
+				.getMessage());
+		assertEquals(313, e.getOffset());
+		assertTrue(e.getLineText().startsWith("@@ -236,9 +236,9 @@ protected "));
+	}
+
+	public void testError_TruncatedNew() throws IOException {
+		final Patch p = parseTestPatchFile();
+		assertEquals(1, p.getFiles().size());
+		assertEquals(1, p.getErrors().size());
+
+		final FormatError e = p.getErrors().get(0);
+		assertSame(FormatError.Severity.ERROR, e.getSeverity());
+		assertEquals("Truncated hunk, at least 1 new lines is missing", e
+				.getMessage());
+		assertEquals(313, e.getOffset());
+		assertTrue(e.getLineText().startsWith("@@ -236,9 +236,9 @@ protected "));
+	}
+
+	public void testError_BodyTooLong() throws IOException {
+		final Patch p = parseTestPatchFile();
+		assertEquals(1, p.getFiles().size());
+		assertEquals(1, p.getErrors().size());
+
+		final FormatError e = p.getErrors().get(0);
+		assertSame(FormatError.Severity.WARNING, e.getSeverity());
+		assertEquals("Hunk header 4:11 does not match body line count of 4:12",
+				e.getMessage());
+		assertEquals(349, e.getOffset());
+		assertTrue(e.getLineText().startsWith("@@ -109,4 +109,11 @@ assert"));
+	}
+
+	public void testError_GarbageBetweenFiles() throws IOException {
+		final Patch p = parseTestPatchFile();
+		assertEquals(2, p.getFiles().size());
+		{
+			final FileHeader fh = p.getFiles().get(0);
+			assertEquals(
+					"org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java",
+					fh.getNewName());
+			assertEquals(1, fh.getHunks().size());
+		}
+		{
+			final FileHeader fh = p.getFiles().get(1);
+			assertEquals(
+					"org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java",
+					fh.getNewName());
+			assertEquals(1, fh.getHunks().size());
+		}
+
+		assertEquals(1, p.getErrors().size());
+		final FormatError e = p.getErrors().get(0);
+		assertSame(FormatError.Severity.WARNING, e.getSeverity());
+		assertEquals("Unexpected hunk trailer", e.getMessage());
+		assertEquals(926, e.getOffset());
+		assertEquals("I AM NOT HERE\n", e.getLineText());
+	}
+
+	public void testError_GitBinaryNoForwardHunk() throws IOException {
+		final Patch p = parseTestPatchFile();
+		assertEquals(2, p.getFiles().size());
+		{
+			final FileHeader fh = p.getFiles().get(0);
+			assertEquals("org.spearce.egit.ui/icons/toolbar/fetchd.png", fh
+					.getNewName());
+			assertSame(FileHeader.PatchType.GIT_BINARY, fh.getPatchType());
+			assertTrue(fh.getHunks().isEmpty());
+			assertNull(fh.getForwardBinaryHunk());
+		}
+		{
+			final FileHeader fh = p.getFiles().get(1);
+			assertEquals("org.spearce.egit.ui/icons/toolbar/fetche.png", fh
+					.getNewName());
+			assertSame(FileHeader.PatchType.UNIFIED, fh.getPatchType());
+			assertTrue(fh.getHunks().isEmpty());
+			assertNull(fh.getForwardBinaryHunk());
+		}
+
+		assertEquals(1, p.getErrors().size());
+		final FormatError e = p.getErrors().get(0);
+		assertSame(FormatError.Severity.ERROR, e.getSeverity());
+		assertEquals("Missing forward-image in GIT binary patch", e
+				.getMessage());
+		assertEquals(297, e.getOffset());
+		assertEquals("\n", e.getLineText());
+	}
+
+	private Patch parseTestPatchFile() throws IOException {
+		final String patchFile = getName() + ".patch";
+		final InputStream in = getClass().getResourceAsStream(patchFile);
+		if (in == null) {
+			fail("No " + patchFile + " test vector");
+			return null; // Never happens
+		}
+		try {
+			final Patch p = new Patch();
+			p.parse(in);
+			return p;
+		} finally {
+			in.close();
+		}
+	}
+}
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/patch/PatchTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/PatchTest.java
index 453d88e..7c69fff 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/patch/PatchTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/PatchTest.java
@@ -49,11 +49,13 @@
 	public void testEmpty() {
 		final Patch p = new Patch();
 		assertTrue(p.getFiles().isEmpty());
+		assertTrue(p.getErrors().isEmpty());
 	}
 
 	public void testParse_ConfigCaseInsensitive() throws IOException {
 		final Patch p = parseTestPatchFile();
 		assertEquals(2, p.getFiles().size());
+		assertTrue(p.getErrors().isEmpty());
 
 		final FileHeader fRepositoryConfigTest = p.getFiles().get(0);
 		final FileHeader fRepositoryConfig = p.getFiles().get(1);
@@ -145,6 +147,7 @@ assertSame(FileHeader.PatchType.UNIFIED, fRepositoryConfig
 	public void testParse_NoBinary() throws IOException {
 		final Patch p = parseTestPatchFile();
 		assertEquals(5, p.getFiles().size());
+		assertTrue(p.getErrors().isEmpty());
 
 		for (int i = 0; i < 4; i++) {
 			final FileHeader fh = p.getFiles().get(i);
@@ -179,6 +182,7 @@ public void testParse_GitBinary() throws IOException {
 		final Patch p = parseTestPatchFile();
 		final int[] binsizes = { 359, 393, 372, 404 };
 		assertEquals(5, p.getFiles().size());
+		assertTrue(p.getErrors().isEmpty());
 
 		for (int i = 0; i < 4; i++) {
 			final FileHeader fh = p.getFiles().get(i);
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_BodyTooLong.patch b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_BodyTooLong.patch
new file mode 100644
index 0000000..1d0b1c4
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_BodyTooLong.patch
@@ -0,0 +1,17 @@
+diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java
+index da7e704..34ce04a 100644
+--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java
++++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java
+@@ -109,4 +109,11 @@ assertTrue(Arrays.equals(values.toArray(), repositoryConfig
+        .getStringList("my", null, "somename")));
+    checkFile(cfgFile, "[my]\n\tsomename = value1\n\tsomename = value2\n");
+  }
++
++ public void test006_readCaseInsensitive() throws IOException {
++   final File path = writeTrashFile("config_001", "[Foo]\nBar\n");
++   RepositoryConfig repositoryConfig = new RepositoryConfig(null, path);
++BAD LINE
++   assertEquals(true, repositoryConfig.getBoolean("foo", null, "bar", false));
++   assertEquals("", repositoryConfig.getString("foo", null, "bar"));
++ }
+ }
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_DisconnectedHunk.patch b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_DisconnectedHunk.patch
new file mode 100644
index 0000000..4762928
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_DisconnectedHunk.patch
@@ -0,0 +1,30 @@
+From: A. U. Thor
+
+@@ -109,4 +109,11 @@ assertTrue(Arrays.equals(values.toArray(), repositoryConfig
+        .getStringList("my", null, "somename")));
+    checkFile(cfgFile, "[my]\n\tsomename = value1\n\tsomename = value2\n");
+  }
++
++ public void test006_readCaseInsensitive() throws IOException {
++   final File path = writeTrashFile("config_001", "[Foo]\nBar\n");
++   RepositoryConfig repositoryConfig = new RepositoryConfig(null, path);
++   assertEquals(true, repositoryConfig.getBoolean("foo", null, "bar", false));
++   assertEquals("", repositoryConfig.getString("foo", null, "bar"));
++ }
+ }
+diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
+index 45c2f8a..3291bba 100644
+--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
++++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
+@@ -236,9 +236,9 @@ protected boolean getBoolean(final String section, String subsection,
+      return defaultValue;
+ 
+    n = n.toLowerCase();
+-   if (MAGIC_EMPTY_VALUE.equals(n) || "yes".equals(n) || "true".equals(n) || "1".equals(n)) {
++   if (MAGIC_EMPTY_VALUE.equals(n) || "yes".equalsIgnoreCase(n) || "true".equalsIgnoreCase(n) || "1".equals(n)) {
+      return true;
+-   } else if ("no".equals(n) || "false".equals(n) || "0".equals(n)) {
++   } else if ("no".equalsIgnoreCase(n) || "false".equalsIgnoreCase(n) || "0".equalsIgnoreCase(n)) {
+      return false;
+    } else {
+      throw new IllegalArgumentException("Invalid boolean value: "
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_GarbageBetweenFiles.patch b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_GarbageBetweenFiles.patch
new file mode 100644
index 0000000..163357e
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_GarbageBetweenFiles.patch
@@ -0,0 +1,33 @@
+diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java
+index da7e704..34ce04a 100644
+--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java
++++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java
+@@ -109,4 +109,11 @@ assertTrue(Arrays.equals(values.toArray(), repositoryConfig
+        .getStringList("my", null, "somename")));
+    checkFile(cfgFile, "[my]\n\tsomename = value1\n\tsomename = value2\n");
+  }
++
++ public void test006_readCaseInsensitive() throws IOException {
++   final File path = writeTrashFile("config_001", "[Foo]\nBar\n");
++   RepositoryConfig repositoryConfig = new RepositoryConfig(null, path);
++   assertEquals(true, repositoryConfig.getBoolean("foo", null, "bar", false));
++   assertEquals("", repositoryConfig.getString("foo", null, "bar"));
++ }
+ }
+I AM NOT HERE
+diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
+index 45c2f8a..3291bba 100644
+--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
++++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
+@@ -236,9 +236,9 @@ protected boolean getBoolean(final String section, String subsection,
+      return defaultValue;
+ 
+    n = n.toLowerCase();
+-   if (MAGIC_EMPTY_VALUE.equals(n) || "yes".equals(n) || "true".equals(n) || "1".equals(n)) {
++   if (MAGIC_EMPTY_VALUE.equals(n) || "yes".equalsIgnoreCase(n) || "true".equalsIgnoreCase(n) || "1".equals(n)) {
+      return true;
+-   } else if ("no".equals(n) || "false".equals(n) || "0".equals(n)) {
++   } else if ("no".equalsIgnoreCase(n) || "false".equalsIgnoreCase(n) || "0".equalsIgnoreCase(n)) {
+      return false;
+    } else {
+      throw new IllegalArgumentException("Invalid boolean value: "
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_GitBinaryNoForwardHunk.patch b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_GitBinaryNoForwardHunk.patch
new file mode 100644
index 0000000..e3f3307
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_GitBinaryNoForwardHunk.patch
@@ -0,0 +1,10 @@
+ create mode 100644 org.spearce.egit.ui/icons/toolbar/pushe.png
+
+diff --git a/org.spearce.egit.ui/icons/toolbar/fetchd.png b/org.spearce.egit.ui/icons/toolbar/fetchd.png
+new file mode 100644
+index 0000000000000000000000000000000000000000..4433c543f2a52b586a3ed5e31b138244107bc239
+GIT binary patch
+
+diff --git a/org.spearce.egit.ui/icons/toolbar/fetche.png b/org.spearce.egit.ui/icons/toolbar/fetche.png
+new file mode 100644
+index 0000000000000000000000000000000000000000..0ffeb419e6ab302caa5e58661854b33853dc43dc
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_TruncatedNew.patch b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_TruncatedNew.patch
new file mode 100644
index 0000000..6bbb73d
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_TruncatedNew.patch
@@ -0,0 +1,15 @@
+diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
+index 45c2f8a..3291bba 100644
+--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
++++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
+@@ -236,9 +236,9 @@ protected boolean getBoolean(final String section, String subsection,
+      return defaultValue;
+ 
+    n = n.toLowerCase();
+-   if (MAGIC_EMPTY_VALUE.equals(n) || "yes".equals(n) || "true".equals(n) || "1".equals(n)) {
+      return true;
+-   } else if ("no".equals(n) || "false".equals(n) || "0".equals(n)) {
++   } else if ("no".equalsIgnoreCase(n) || "false".equalsIgnoreCase(n) || "0".equalsIgnoreCase(n)) {
+      return false;
+    } else {
+      throw new IllegalArgumentException("Invalid boolean value: "
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_TruncatedOld.patch b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_TruncatedOld.patch
new file mode 100644
index 0000000..c8fbdc1
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/testError_TruncatedOld.patch
@@ -0,0 +1,15 @@
+diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
+index 45c2f8a..3291bba 100644
+--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
++++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
+@@ -236,9 +236,9 @@ protected boolean getBoolean(final String section, String subsection,
+      return defaultValue;
+ 
+    n = n.toLowerCase();
+-   if (MAGIC_EMPTY_VALUE.equals(n) || "yes".equals(n) || "true".equals(n) || "1".equals(n)) {
++   if (MAGIC_EMPTY_VALUE.equals(n) || "yes".equalsIgnoreCase(n) || "true".equalsIgnoreCase(n) || "1".equals(n)) {
+      return true;
++   } else if ("no".equalsIgnoreCase(n) || "false".equalsIgnoreCase(n) || "0".equalsIgnoreCase(n)) {
+      return false;
+    } else {
+      throw new IllegalArgumentException("Invalid boolean value: "
diff --git a/org.spearce.jgit/src/org/spearce/jgit/patch/FormatError.java b/org.spearce.jgit/src/org/spearce/jgit/patch/FormatError.java
new file mode 100644
index 0000000..e6f0a03
--- /dev/null
+++ b/org.spearce.jgit/src/org/spearce/jgit/patch/FormatError.java
@@ -0,0 +1,95 @@
+/*
+ * Copyright (C) 2008, Google Inc.
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
+ *
+ * - Neither the name of the Git Development Community nor the
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package org.spearce.jgit.patch;
+
+import org.spearce.jgit.lib.Constants;
+import org.spearce.jgit.util.RawParseUtils;
+
+/** An error in a patch script */
+public class FormatError {
+	/** Classification of an error. */
+	public static enum Severity {
+		/** The error is unexpected, but can be worked around. */
+		WARNING,
+
+		/** The error indicates the script is severely flawed. */
+		ERROR;
+	}
+
+	private final byte[] buf;
+
+	private final int offset;
+
+	private final Severity severity;
+
+	private final String message;
+
+	FormatError(final byte[] buffer, final int ptr, final Severity sev,
+			final String msg) {
+		buf = buffer;
+		offset = ptr;
+		severity = sev;
+		message = msg;
+	}
+
+	/** @return the severity of the error. */
+	public Severity getSeverity() {
+		return severity;
+	}
+
+	/** @return a message describing the error. */
+	public String getMessage() {
+		return message;
+	}
+
+	/** @return the byte buffer holding the patch script. */
+	public byte[] getBuffer() {
+		return buf;
+	}
+
+	/** @return byte offset within {@link #getBuffer()} where the error is */
+	public int getOffset() {
+		return offset;
+	}
+
+	/** @return line of the patch script the error appears on. */
+	public String getLineText() {
+		final int eol = RawParseUtils.nextLF(buf, offset);
+		return RawParseUtils.decode(Constants.CHARSET, buf, offset, eol);
+	}
+}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/patch/HunkHeader.java b/org.spearce.jgit/src/org/spearce/jgit/patch/HunkHeader.java
index fc606c3..20dd6e2 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/patch/HunkHeader.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/patch/HunkHeader.java
@@ -132,7 +132,7 @@ void parseHeader() {
 		newLineCount = parseBase10(buf, ptr.value + 1, ptr);
 	}
 
-	int parseBody() {
+	int parseBody(final Patch script) {
 		final byte[] buf = file.buf;
 		final int sz = buf.length;
 		int c = nextLF(buf, startOffset), last = c;
@@ -175,9 +175,21 @@ int parseBody() {
 			return last;
 		}
 
-		if (nContext + nDeleted != oldLineCount
-				|| nContext + nAdded != newLineCount) {
-			// TODO report on truncated hunk
+		if (nContext + nDeleted < oldLineCount) {
+			final int missingCount = oldLineCount - (nContext + nDeleted);
+			script.error(buf, startOffset, "Truncated hunk, at least "
+					+ missingCount + " old lines is missing");
+
+		} else if (nContext + nAdded < newLineCount) {
+			final int missingCount = newLineCount - (nContext + nAdded);
+			script.error(buf, startOffset, "Truncated hunk, at least "
+					+ missingCount + " new lines is missing");
+
+		} else if (nContext + nDeleted > oldLineCount
+				|| nContext + nAdded > newLineCount) {
+			script.warn(buf, startOffset, "Hunk header " + oldLineCount + ":"
+					+ newLineCount + " does not match body line count of "
+					+ (nContext + nDeleted) + ":" + (nContext + nAdded));
 		}
 
 		return c;
diff --git a/org.spearce.jgit/src/org/spearce/jgit/patch/Patch.java b/org.spearce.jgit/src/org/spearce/jgit/patch/Patch.java
index 56eb327..5cc208c 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/patch/Patch.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/patch/Patch.java
@@ -69,9 +69,13 @@
 	/** The files, in the order they were parsed out of the input. */
 	private final List<FileHeader> files;
 
+	/** Formatting errors, if any were identified. */
+	private final List<FormatError> errors;
+
 	/** Create an empty patch. */
 	public Patch() {
 		files = new ArrayList<FileHeader>();
+		errors = new ArrayList<FormatError>(0);
 	}
 
 	/**
@@ -93,6 +97,21 @@ public void addFile(final FileHeader fh) {
 	}
 
 	/**
+	 * Add a formatting error to this patch script.
+	 * 
+	 * @param err
+	 *            the error description.
+	 */
+	public void addError(final FormatError err) {
+		errors.add(err);
+	}
+
+	/** @return collection of formatting errors, if any. */
+	public List<FormatError> getErrors() {
+		return errors;
+	}
+
+	/**
 	 * Parse a patch received from an InputStream.
 	 * <p>
 	 * Multiple parse calls on the same instance will concatenate the patch
@@ -149,8 +168,7 @@ private int parseFile(final byte[] buf, int c) {
 				// have missed a file header previously. The hunk
 				// isn't valid without knowing where it comes from.
 				//
-
-				// TODO handle a disconnected hunk fragment
+				error(buf, c, "Hunk disconnected from file");
 				c = nextLF(buf, c);
 				continue;
 			}
@@ -218,6 +236,7 @@ private int parseDiffCC(final byte[] buf, final int startOffset) {
 
 		// TODO Support parsing diff --cc headers
 		// TODO parse diff --cc hunks
+		warn(buf, startOffset, "diff --cc format not supported");
 		fh.endOffset = ptr;
 		addFile(fh);
 		return ptr;
@@ -259,12 +278,12 @@ private int parseHunks(final FileHeader fh, int c) {
 			if (match(buf, c, HUNK_HDR) >= 0) {
 				final HunkHeader h = new HunkHeader(fh, c);
 				h.parseHeader();
-				c = h.parseBody();
+				c = h.parseBody(this);
 				h.endOffset = c;
 				fh.addHunk(h);
 				if (c < sz && buf[c] != '@' && buf[c] != 'd'
 						&& match(buf, c, SIG_FOOTER) < 0) {
-					// TODO report on noise between hunks, might be an error
+					warn(buf, c, "Unexpected hunk trailer");
 				}
 				continue;
 			}
@@ -308,8 +327,7 @@ private int parseGitBinary(final FileHeader fh, int c) {
 		if (nEnd < 0) {
 			// Not a binary hunk.
 			//
-
-			// TODO handle invalid binary hunks
+			error(fh.buf, c, "Missing forward-image in GIT binary patch");
 			return c;
 		}
 		c = nEnd;
@@ -327,6 +345,14 @@ private int parseGitBinary(final FileHeader fh, int c) {
 		return c;
 	}
 
+	void warn(final byte[] buf, final int ptr, final String msg) {
+		addError(new FormatError(buf, ptr, FormatError.Severity.WARNING, msg));
+	}
+
+	void error(final byte[] buf, final int ptr, final String msg) {
+		addError(new FormatError(buf, ptr, FormatError.Severity.ERROR, msg));
+	}
+
 	private static boolean matchAny(final byte[] buf, final int c,
 			final byte[][] srcs) {
 		for (final byte[] s : srcs) {
-- 
1.6.1.rc2.306.ge5d5e

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox