[jgit] index v2 pull request

> Shawn O. Pearce wrote:
> 
> > Unfortunately the overall series diffstat doesn't show it
> > that way, but I think its now easier to follow how the control flows.
> 
> Isn't it the place then to put --dirstat instead of --diffstat?

> We now actually use the following .git/config options to control
> how the window cache limits work:
>
>   core.packedGitWindowSize
>   core.packedGitLimit
>   core.deltaBaseCacheLimit
>   core.packedGitMMAP

> Shawn O. Pearce (41):
>       Make the absolute path of a WindowedFile available to callers
>       Include the pack index file name when reporting a bad header
>       Report pack file open failures to the console
>       Refactor index reading code from PackFile to PackIndex
>       Rename PackIndex.readIndexHeader to loadVersion1
>       Read pack index files using FileInputStream and not WindowedFile
>       Don't bother checking the index file length after reading it
>       Move index:pack object count checking into PackFile class
>       Refactor PackIndex version 1 handling code into PackIndexV1
>       Fully document the PackIndex API
>       Hoist readFully and decodeUInt32 up from PackIndexV1 to PackIndex
>       Refactor PackIndex.readFully to support reading into the middle
>       Read the pack index header to perform automatic version detection
>       Add complete reading support for pack index v2

>       Support reading config file integers with k/m/g suffixes
>       Honor core.packedGitWindowSize, packedGitLimit, packedGitMMAP config
>       Enable easy access to the user's ~/.gitconfig
>       Use a single WindowCache for the entire Eclipse workspace
>       Modify the WindowedFile API to pass around a WindowCursor during reads 
>       Change ByteWindow implementations to hold data by SoftReferences 
>       Refactor WindowedFile/WindowCache so all windows are a uniform size
>       Simplify WindowCache and WindowedFile internal APIs to be private Use 

> Den Saturday 08 March 2008 03.50.27 skrev Shawn O. Pearce:
>
> > So I've started to hack on jgit again, to build out more of the lower
>  > level library.
>  Welcome back Shawn!
>
>
>  > This particular series adds support for pack file index v2, which
>  > Nico added to C Git several months ago and is slated to become the
>  > default pack index file format in a future release of Git, due to
>  > its increased error checking during delta reuse.
>  This i great news. I've been thinking about it, praying that you'd come
>  and do the work. :)
>
>
>  > We now actually use the following .git/config options to control
>  > how the window cache limits work:
>  >
>  >   core.packedGitWindowSize
>  >   core.packedGitLimit
>  >   core.deltaBaseCacheLimit
>  >   core.packedGitMMAP
>
>  Will the net effect, in practice, be the same on Jgit as on C Git?
>
>
>  >   repo.or.cz:/srv/git/egit/spearce.git master
>  Something missing in the formatting?
>
>
>  > Shawn O. Pearce (41):
>  >       Make the absolute path of a WindowedFile available to callers
>  >       Include the pack index file name when reporting a bad header
>  >       Report pack file open failures to the console
>  >       Refactor index reading code from PackFile to PackIndex
>  >       Rename PackIndex.readIndexHeader to loadVersion1
>  >       Read pack index files using FileInputStream and not WindowedFile
>  >       Don't bother checking the index file length after reading it
>  >       Move index:pack object count checking into PackFile class
>  >       Refactor PackIndex version 1 handling code into PackIndexV1
>  >       Fully document the PackIndex API
>  >       Hoist readFully and decodeUInt32 up from PackIndexV1 to PackIndex
>  >       Refactor PackIndex.readFully to support reading into the middle
>  >       Read the pack index header to perform automatic version detection
>  >       Add complete reading support for pack index v2
>  Yum, yum. My 15 minute review so far likes it.
>
> >       Stop creating "new format" loose objects as C Git no longer likes
>  > them Refactor RepositoryConfig to be stacked many levels deep
>  And there goes the unit test for reading them...
>
>
>  >       Don't make unit tests depend upon compressed file lengths
>  >       Corrected name of the method that reloads the packed-refs data
>  >       Cleanup misspelling in Javadoc of Repository class
>  >       Extract StGitPatch from Repository to its own top-level type
>  I'm considering dropping stacked git support. We should have much
>  more of it and I don't use it myself anymore to what support we have
>  gets very little testing.
>
>
>  >       Refactor RepositoryState into its own top-level type
>  \ No newline at end of file
>
>
>  >       Refactor PackFile construction to take the idxFile path from scan
>  >       Shuffle the length of a WindowedFile down into its provider
>  >       Defer opening pack files until we actually need object data
>  >       Use Java 5 bit twiddling APIs rather than hand-rolling bit counting
>  I'm surprised *you* didn't know about them bitcounting algorithm java uses
>  before. It's pretty common.
>
>
>  >       Support reading config file integers with k/m/g suffixes
>  >       Honor core.packedGitWindowSize, packedGitLimit, packedGitMMAP config
>  >       Enable easy access to the user's ~/.gitconfig
>  >       Use a single WindowCache for the entire Eclipse workspace
>  >       Modify the WindowedFile API to pass around a WindowCursor during reads
>  >       Change ByteWindow implementations to hold data by SoftReferences
>  >       Refactor WindowedFile/WindowCache so all windows are a uniform size
>  >       Simplify WindowCache and WindowedFile internal APIs to be private Use
>  a proper ReferenceQueue to avoid duplpicate window entries
>  >       New  RepositoryConfig getInt,getBoolean helper methods with no
>  subsection Change delta packed object loaders to use a PackedObjectLoader for
>  base Implement core.deltaBaseCacheLimit to improve unpacking performance
>  >       Remove unnecessary bitwise AND masks in readTree
>  Good catch
>
> >       Precount the number of tree entries during readTree
>  this one too
>
> >       Change the ObjectLoader.getType method to return int
>  >       Fix ArrayIndexOutOfBoundsException in WindowCache
>
>  Very good work (as always) Shawn. I've only read the code so far. I'll do some
>  testing too before pushing. Do you have any unit tests for the v2 pack format
>  (config reading too for that matter)?
>
>  -- robin
>
>
> --
>  To unsubscribe from this list: send the line "unsubscribe git" in
>  the body of a message to majordomo@vger.kernel.org
>  More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

> Den Monday 10 March 2008 08.32.21 skrev Imran M Yousuf:
>
> > I would like to volunteer to work in JGit;
>  > can someone let me know where I can pick some tasks to implement?
>  >
>
>  You can pick about anything you want that noone is working one. Pick something
>  that you feel is withing your grasp, or something simple outside. We need
>  more test cases (doesn't require much programming skills, but thinking). Unit
>  testing of Eclipse stuff falls into the same category. I have litte experience
>  here, but I think there are lots of examples in Eclipse itself if one starts
>  to poke around.
>
>  Diff against workspace/index doesn't work presently. The reworked
>  indexdiffwalker or the iterator could be used for that. Please tell me if you
>  start hacking on this as it's on my listl, just not yet. Should be reasonably
>  hard, involves both jgit and eclipse programming.
>
>  There are some issues with selecting versions in the history view on Windows.
>
>  Copy diff as git patch to the clipboard and vice verse. Send email.
>
>  Harder and might involve more changes all over (that either I or Shawn will
>  object to) is things like rename detection targeted fo r
>
>  Interface to quickdiff machinery for committing specific hunks.
>
>  Blame (even without rename detection) would be useful. Look at the CVS
>  annotate for inspiration.
>
>  Support for git submodules. Will touch the core parts all over the place, but
>  probably not in very intrusive way. Will require lots of test cases.
>
>  Not really git, but highlighting of trailing whitespace is on my wish list. I
>  can't find a plugin that does that.
>
>  Adjust the commit dialog for amend so it shows all to-be commited changed
>  files when amending.
>
>  The number of choices is infinite.
>
>  Ant, Cruisecontrol, Maven, etc-tasks.

>
>  Pick a pet.
>
>  -- robin
>
>

> Den Wednesday 12 March 2008 03.52.05 skrev Imran M Yousuf:
> 
> > I am currently looking into .git/config format as Shawn suggested.
> 
> That's a good one too. I have code for parsing the remotes specs, though
> e.g. "+refs/heads/master/*:refs/remotes/origin/*", but nothing for the "branch" config. I haven't used it yet, so it's a rip-out.	

> + * <pre>
> + *  [remote &quot;name&quot;]
> + *     url = URL:ish
> + *     fetch = [+]remoteref:localref
> + * </pre>
> + *
> + * There are more variants but we do not support them here yet.

> +public class RemoteSpec {
> +
> +	static class Info {
> +		boolean overwriteAlways;
> +
> +		boolean matchAny;
> +
> +		String remoteRef;
> +
> +		String localRef;
> +	}
> +
> +	Info fetch = new Info();
> +
> +	Info push = null;
> +
> +	private final String name;
> +
> +	private final String url;

> Den Wednesday 12 March 2008 03.52.05 skrev Imran M Yousuf:
>
> > Maven: Does this mean mavenizing the project? If so I would start it
>  > this weekend. If you were referring to GIT Maven SCM please also let
>  > me know.
>  Not to build using, but create SCM functions for those so thay can use
>  git without <exec> tasks. Git isn't available in any of those, while much
>  worse SCM's have support.
>

>
>  > I am currently looking into .git/config format as Shawn suggested.
>
>  That's a good one too. I have code for parsing the remotes specs, though
>  e.g. "+refs/heads/master/*:refs/remotes/origin/*", but nothing for the "branch" config. I haven't used it yet, so it's a rip-out.

>
>  -- robin
>
>  From 042fa0bef8b5d3ec8f5b1e385766ca61528e72aa Mon Sep 17 00:00:00 2001
>  From: Robin Rosenberg <robin.rosenberg@dewire.com>
>  Date: Wed, 5 Mar 2008 23:27:57 +0100
>  Subject: [PATCH] Add support for parsing remote specs
>
>  Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
>  ---
>   .../tst/org/spearce/jgit/lib/RemoteSpecTest.java   |   95 +++++++++++++
>   .../src/org/spearce/jgit/lib/RemoteSpec.java       |  142 ++++++++++++++++++++
>   .../src/org/spearce/jgit/lib/Repository.java       |   12 ++
>   3 files changed, 249 insertions(+), 0 deletions(-)
>   create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/lib/RemoteSpecTest.java
>   create mode 100644 org.spearce.jgit/src/org/spearce/jgit/lib/RemoteSpec.java
>
>  diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RemoteSpecTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RemoteSpecTest.java
>  new file mode 100644
>  index 0000000..817e697
>  --- /dev/null
>  +++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RemoteSpecTest.java
>  @@ -0,0 +1,95 @@
>  +/*
>  + *  Copyright (C) 2007  Robin Rosenberg
>  + *
>  + *  This library is free software; you can redistribute it and/or
>  + *  modify it under the terms of the GNU General Public
>  + *  License, version 2, as published by the Free Software Foundation.
>  + *
>  + *  This library is distributed in the hope that it will be useful,
>  + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
>  + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>  + *  General Public License for more details.
>  + *
>  + *  You should have received a copy of the GNU General Public
>  + *  License along with this library; if not, write to the Free Software
>  + *  Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301
>  + */
>  +package org.spearce.jgit.lib;
>  +
>  +import java.io.File;
>  +import java.io.FileOutputStream;
>  +
>  +/**
>  + * Test parsing of git remotes
>  + */
>  +public class RemoteSpecTest extends RepositoryTestCase {
>  +
>  +       /**
>  +        * Test simplest case
>  +        *
>  +        * @throws Exception
>  +        *
>  +        */
>  +       public void testSimplestOk() throws Exception {
>  +               RemoteSpec spec = new RemoteSpec("their", "git://foo.bar/zip.git",
>  +                               "refs/heads/master:refs/heads/origin",null);
>  +               assertEquals("refs/heads/master", spec.getFetchRemoteRef());
>  +               assertEquals("refs/heads/origin", spec.getFetchLocalRef());
>  +               assertFalse(spec.isFetchMatchAny());
>  +               assertFalse(spec.isFetchOverwriteAlways());
>  +       }
>  +
>  +       /**
>  +        * Test a standard case
>  +        *
>  +        * @throws Exception
>  +        */
>  +       public void testStandardOk() throws Exception {
>  +               RemoteSpec spec = new RemoteSpec("their", "git://example.com/zip.git",
>  +                               "+refs/heads/master/*:refs/remotes/origin/*",null);
>  +               assertEquals("git://example.com/zip.git", spec.getUrl());
>  +               assertEquals("refs/heads/master", spec.getFetchRemoteRef());
>  +               assertEquals("refs/remotes/origin", spec.getFetchLocalRef());
>  +               assertTrue(spec.isFetchMatchAny());
>  +               assertTrue(spec.isFetchOverwriteAlways());
>  +       }
>  +
>  +       /**
>  +        * Test a <quote>safer</quote> almost standard case
>  +        *
>  +        * @throws Exception
>  +        */
>  +       public void testNonStandardSaferOk() throws Exception {
>  +               RemoteSpec spec = new RemoteSpec("their", "git://example.com/zip.git",
>  +                               "refs/heads/master/*:refs/remotes/origin/*",null);
>  +               assertEquals("git://example.com/zip.git", spec.getUrl());
>  +               assertEquals("refs/heads/master", spec.getFetchRemoteRef());
>  +               assertEquals("refs/remotes/origin", spec.getFetchLocalRef());
>  +               assertTrue(spec.isFetchMatchAny());
>  +               assertFalse(spec.isFetchOverwriteAlways());
>  +       }
>  +
>  +       /**
>  +        * Test a case copied from a real Git repo
>  +        *
>  +        * @throws Exception
>  +        */
>  +       public void testReadFromConfig() throws Exception {
>  +               File file = new File(db.getDirectory(),"config");
>  +               FileOutputStream stream = new FileOutputStream(file,true);
>  +               try {
>  +                       stream.write(("[remote \"spearce\"]\n"+
>  +               "url = http://www.spearce.org/projects/scm/egit.git\n"+
>  +               "fetch = +refs/heads/*:refs/remotes/spearce/*\n").getBytes());
>  +               } finally {
>  +                       stream.close();
>  +               }
>  +               db.getConfig().load();
>  +               RemoteSpec remoteSpec = db.getRemoteSpec("spearce");
>  +               assertEquals("http://www.spearce.org/projects/scm/egit.git", remoteSpec.getUrl());
>  +               assertEquals("refs/heads", remoteSpec.getFetchRemoteRef());
>  +               assertEquals("refs/remotes/spearce", remoteSpec.getFetchLocalRef());
>  +               assertTrue(remoteSpec.isFetchMatchAny());
>  +               assertTrue(remoteSpec.isFetchOverwriteAlways());
>  +       }
>  +}
>  diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RemoteSpec.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RemoteSpec.java
>  new file mode 100644
>  index 0000000..a6a3b6c
>  --- /dev/null
>  +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RemoteSpec.java
>  @@ -0,0 +1,142 @@
>  +/*
>  + *  Copyright (C) 2007  Robin Rosenberg
>  + *
>  + *  This library is free software; you can redistribute it and/or
>  + *  modify it under the terms of the GNU General Public
>  + *  License, version 2, as published by the Free Software Foundation.
>  + *
>  + *  This library is distributed in the hope that it will be useful,
>  + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
>  + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>  + *  General Public License for more details.
>  + *
>  + *  You should have received a copy of the GNU General Public
>  + *  License along with this library; if not, write to the Free Software
>  + *  Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301
>  + */
>  +package org.spearce.jgit.lib;
>  +
>  +/**
>  + * Information about how to synchronize with a remote Git repository.
>  + *
>  + * A remote is stored in the <GIT_DIR>/config as
>  + *
>  + * <pre>
>  + *  [remote &quot;name&quot;]
>  + *     url = URL:ish
>  + *     fetch = [+]remoteref:localref
>  + * </pre>
>  + *
>  + * There are more variants but we do not support them here yet.
>  + */
>  +public class RemoteSpec {
>  +
>  +       static class Info {
>  +               boolean overwriteAlways;
>  +
>  +               boolean matchAny;
>  +
>  +               String remoteRef;
>  +
>  +               String localRef;
>  +       }
>  +
>  +       Info fetch = new Info();
>  +
>  +       Info push = null;
>  +
>  +       private final String name;
>  +
>  +       private final String url;
>  +
>  +       /**
>  +        * @return name of remote. This is a local short identifier
>  +        */
>  +       public String getName() {
>  +               return name;
>  +       }
>  +
>  +       /**
>  +        * @return the URL:ish location of the remote Git repository
>  +        */
>  +       public String getUrl() {
>  +               return url;
>  +       }
>  +
>  +       /**
>  +        * @return the local ref part used for fetch heads info
>  +        */
>  +       public String getFetchLocalRef() {
>  +               return fetch.localRef;
>  +       }
>  +
>  +       /**
>  +        * @return the remote ref part used for fetching refs from the remote repo
>  +        */
>  +       public String getFetchRemoteRef() {
>  +               return fetch.remoteRef;
>  +       }
>  +
>  +       /**
>  +        * @return whether the fetch matches all branches under the ref or just the
>  +        *         named ref
>  +        */
>  +       public boolean isFetchMatchAny() {
>  +               return fetch.matchAny;
>  +       }
>  +
>  +       /**
>  +        * @return whether the tracking branch is always updated, or only when the
>  +        *         update is a fast forward
>  +        */
>  +       public boolean isFetchOverwriteAlways() {
>  +               return fetch.overwriteAlways;
>  +       }
>  +
>  +       /**
>  +        * Create a representation of a git remote specification.
>  +        *
>  +        * @param name A local short identifier
>  +        * @param url The URL:ish used for fetching / pushing
>  +        * @param fetchPattern refspec for fetching
>  +        * @param pushPattern refspec for pushing or null
>  +        */
>  +       public RemoteSpec(String name, String url, String fetchPattern,
>  +                       String pushPattern) {
>  +               this.name = name;
>  +               this.url = url;
>  +               parse(fetchPattern, fetch);
>  +               if (pushPattern != null) {
>  +                       push = new Info();
>  +                       parse(pushPattern, push);
>  +               }
>  +       }
>  +
>  +       private void parse(String fetchSpec, Info info) {
>  +               int p = 0;
>  +               if (fetchSpec.charAt(p) == '+') {
>  +                       info.overwriteAlways = true;
>  +                       ++p;
>  +               }
>  +               int cp = fetchSpec.indexOf(':');
>  +               if (cp < 0)
>  +                       throw new IllegalArgumentException("Bad remote format " + fetchSpec);
>  +               info.remoteRef = fetchSpec.substring(p, cp);
>  +               info.localRef = fetchSpec.substring(cp + 1);
>  +               if (info.remoteRef.endsWith("/*")) {
>  +                       info.matchAny = true;
>  +                       info.remoteRef = info.remoteRef.substring(0, info.remoteRef
>  +                                       .length() - 2);
>  +               }
>  +               if (info.localRef.endsWith("/*")) {
>  +                       if (!info.matchAny)
>  +                               throw new IllegalArgumentException("Bad remote format "
>  +                                               + fetchSpec);
>  +                       info.localRef = info.localRef.substring(0,
>  +                                       info.localRef.length() - 2);
>  +               } else
>  +                       if (info.matchAny)
>  +                               throw new IllegalArgumentException("Bad remote format " + fetchSpec);
>  +
>  +       }
>  +}
>  diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
>  index b3fa12e..821633d 100644
>  --- a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
>  +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
>  @@ -1132,4 +1132,16 @@ public class Repository {
>                 relName = relName.replace(File.separatorChar, '/');
>                 return relName;
>         }
>  +
>  +       /**
>  +        * @param name
>  +        *            The "remote" name in this repo
>  +        * @return information about how a remote repository is beging tracked
>  +        */
>  +       public RemoteSpec getRemoteSpec(String name) {
>  +               String url = getConfig().getString("remote."+name, null, "url");
>  +               String fetchPattern = getConfig().getString("remote."+name, null, "fetch");
>  +               String pushPattern = getConfig().getString("remote."+name, null, "push");
>  +               return new RemoteSpec(name, url, fetchPattern, pushPattern);
>  +       }
>   }
>  --
>  1.5.4.3
>
>

> Imran M Yousuf <imyousuf@gmail.com> wrote:
>  > I would like to volunteer to work in JGit;
>  > can someone let me know where I can pick some tasks to implement?
>
>  One of the areas that I think is really weak in JGit and that we
>  need to do a _good_ fetch/push implementation is the branch model.
>
>  For example, if you look at Repository.getBranches() it returns
>  to the caller a Collection<String>.
>
>  But in Git a branch is a heck of a lot more data, and that data is
>  relevant to the end user when we are talking about fetch and merge.
>  There are many configuration options stored in .git/config for a
>  branch, and these are (today) created by git-branch and git-checkout
>  automatically as the user creates and deletes branches.
>
>  JGit has none of this model.  It thinks all that a branch is is a
>  String.  Sad.
>
>  Another thing that bothers me is the packed refs cache.  We toss
>  away the peeled information (the "^" lines), but that data is very
>  useful when you are talking about fetch as well as a few other
>  types of operations, like plotting tag labels onto a history graph
>  (such as how gitk does it).
>
>  The Ref class was started as a means of wrapping up the various
>  important bits of data about a ref (of which branches are a subclass
>  and annotated tags with peeled data is another) but I suspect it
>  didn't quite do everything so these string APIs got created.
>
>  The above is a farily small task, one that any good OO programmer
>  should be able to tackle, but it can be challenging if you are
>  new to Git plumbing as you'll have to learn what are the important
>  parts of a branch/ref/annotated tag.
>
>  As a starting point look at the config file format in C Git and
>  see what is possible in a "branch" and "remote" section; that
>  data needs to be pretty readily available for any Ref, if such
>  data exists.  The backward mapping of Ref->Remote (which does
>  not exist yet, unless Robin created it) is quite relevant for
>  tracking branches.

> Den Saturday 08 March 2008 03.50.27 skrev Shawn O. Pearce:
> 
> > We now actually use the following .git/config options to control
> > how the window cache limits work:
> >
> >   core.packedGitWindowSize
> >   core.packedGitLimit
> >   core.deltaBaseCacheLimit
> >   core.packedGitMMAP
> 
> Will the net effect, in practice, be the same on Jgit as on C Git?

> >       Extract StGitPatch from Repository to its own top-level type
>
> I'm considering dropping stacked git support. We should have much
> more of it and I don't use it myself anymore to what support we have
> gets very little testing.

> >       Change the ObjectLoader.getType method to return int
> >       Fix ArrayIndexOutOfBoundsException in WindowCache
> 
> Very good work (as always) Shawn. I've only read the code so far. I'll do some 
> testing too before pushing. Do you have any unit tests for the v2 pack format 
> (config reading too for that matter)?