* [JGIT PATCH 0/7] Misc. RevWalk, bundle transport improvements @ 2008-09-04 23:42 Shawn O. Pearce 2008-09-04 23:42 ` [JGIT PATCH 1/7] Cleanup RevWalk.parseCommit, parseTree to not throw ClassCastException Shawn O. Pearce 0 siblings, 1 reply; 8+ messages in thread From: Shawn O. Pearce @ 2008-09-04 23:42 UTC (permalink / raw) To: Robin Rosenberg; +Cc: git The bundle transport improvements are needed for another open source project I'm starting to find myself working on. Details to be made available at a later date, but its possibly something quite cool for the Git community in general. Anyway... Shawn O. Pearce (7): Cleanup RevWalk.parseCommit, parseTree to not throw ClassCastException Cleanup RevWalk.parseTree semantics Fix potential NullPointerException in RevWalk.parseTree Add prerequisite verification to the bundle transport Include URIish in bundle transport within any TransportExceptions Refactor TransportBundle to not be dependent on FileInputStream Refactor bundle transport to permit streaming from application .../jgit/pgm/opt/AbstractTreeIteratorHandler.java | 2 - .../org/spearce/jgit/pgm/opt/RevCommitHandler.java | 2 - .../org/spearce/jgit/pgm/opt/RevTreeHandler.java | 2 - .../errors/MissingBundlePrerequisiteException.java | 73 ++++++++++ .../src/org/spearce/jgit/revwalk/RevWalk.java | 28 +++- .../spearce/jgit/transport/BasePackConnection.java | 2 +- .../jgit/transport/BasePackFetchConnection.java | 2 - .../src/org/spearce/jgit/transport/IndexPack.java | 20 ++- .../src/org/spearce/jgit/transport/Transport.java | 4 +- .../spearce/jgit/transport/TransportBundle.java | 150 +++++++++++--------- .../jgit/transport/TransportBundleFile.java | 82 +++++++++++ .../jgit/transport/TransportBundleStream.java | 105 ++++++++++++++ 12 files changed, 384 insertions(+), 88 deletions(-) create mode 100644 org.spearce.jgit/src/org/spearce/jgit/errors/MissingBundlePrerequisiteException.java create mode 100644 org.spearce.jgit/src/org/spearce/jgit/transport/TransportBundleFile.java create mode 100644 org.spearce.jgit/src/org/spearce/jgit/transport/TransportBundleStream.java ^ permalink raw reply [flat|nested] 8+ messages in thread
* [JGIT PATCH 1/7] Cleanup RevWalk.parseCommit, parseTree to not throw ClassCastException 2008-09-04 23:42 [JGIT PATCH 0/7] Misc. RevWalk, bundle transport improvements Shawn O. Pearce @ 2008-09-04 23:42 ` Shawn O. Pearce 2008-09-04 23:42 ` [JGIT PATCH 2/7] Cleanup RevWalk.parseTree semantics Shawn O. Pearce 0 siblings, 1 reply; 8+ messages in thread From: Shawn O. Pearce @ 2008-09-04 23:42 UTC (permalink / raw) To: Robin Rosenberg; +Cc: git Throwing ClassCastException here for non-commits is really difficult to work with at the caller level because we may catch the wrong sort of ClassCastException and may mask a bug deep inside of RevWalk's parsing code. It is cleaner to throw IncorrectObjectTypeException and catch that. Besides, the method javadoc says that is what gets thrown if either method is given the wrong type. Signed-off-by: Shawn O. Pearce <spearce@spearce.org> --- .../jgit/pgm/opt/AbstractTreeIteratorHandler.java | 2 -- .../org/spearce/jgit/pgm/opt/RevCommitHandler.java | 2 -- .../org/spearce/jgit/pgm/opt/RevTreeHandler.java | 2 -- .../src/org/spearce/jgit/revwalk/RevWalk.java | 7 ++++++- .../jgit/transport/BasePackFetchConnection.java | 2 -- 5 files changed, 6 insertions(+), 9 deletions(-) diff --git a/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/opt/AbstractTreeIteratorHandler.java b/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/opt/AbstractTreeIteratorHandler.java index e439c87..9432d5f 100644 --- a/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/opt/AbstractTreeIteratorHandler.java +++ b/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/opt/AbstractTreeIteratorHandler.java @@ -112,8 +112,6 @@ public int parseArguments(final Parameters params) throws CmdLineException { final CanonicalTreeParser p = new CanonicalTreeParser(); try { p.reset(clp.getRepository(), clp.getRevWalk().parseTree(id)); - } catch (ClassCastException e) { - throw new CmdLineException(name + " is not a tree"); } catch (MissingObjectException e) { throw new CmdLineException(name + " is not a tree"); } catch (IncorrectObjectTypeException e) { diff --git a/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/opt/RevCommitHandler.java b/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/opt/RevCommitHandler.java index 772a8d7..5bfc61e 100644 --- a/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/opt/RevCommitHandler.java +++ b/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/opt/RevCommitHandler.java @@ -115,8 +115,6 @@ private void addOne(final String name, final boolean interesting) final RevCommit c; try { c = clp.getRevWalk().parseCommit(id); - } catch (ClassCastException e) { - throw new CmdLineException(name + " is not a commit"); } catch (MissingObjectException e) { throw new CmdLineException(name + " is not a commit"); } catch (IncorrectObjectTypeException e) { diff --git a/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/opt/RevTreeHandler.java b/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/opt/RevTreeHandler.java index de64336..0a0c5d2 100644 --- a/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/opt/RevTreeHandler.java +++ b/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/opt/RevTreeHandler.java @@ -88,8 +88,6 @@ public int parseArguments(final Parameters params) throws CmdLineException { final RevTree c; try { c = clp.getRevWalk().parseTree(id); - } catch (ClassCastException e) { - throw new CmdLineException(name + " is not a tree"); } catch (MissingObjectException e) { throw new CmdLineException(name + " is not a tree"); } catch (IncorrectObjectTypeException e) { diff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java index c90f2a3..243d9b3 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java +++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java @@ -608,6 +608,9 @@ public RevCommit parseCommit(final AnyObjectId id) c = ((RevTag) c).getObject(); parse(c); } + if (!(c instanceof RevCommit)) + throw new IncorrectObjectTypeException(id.toObjectId(), + Constants.TYPE_COMMIT); return (RevCommit) c; } @@ -639,7 +642,9 @@ public RevTree parseTree(final AnyObjectId id) if (c instanceof RevCommit) { c = ((RevCommit) c).getTree(); parse(c); - } + } else if (!(c instanceof RevTree)) + throw new IncorrectObjectTypeException(id.toObjectId(), + Constants.TYPE_TREE); final RevTree t = (RevTree) c; if (db.openObject(t).getType() != Constants.OBJ_TREE) throw new IncorrectObjectTypeException(t, Constants.TYPE_TREE); diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackFetchConnection.java b/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackFetchConnection.java index 39fa761..a22b33d 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackFetchConnection.java +++ b/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackFetchConnection.java @@ -197,8 +197,6 @@ private void markReachable(final int maxTime) throws IOException { reachableCommits.add(o); } catch (IOException readError) { // If we cannot read the value of the ref skip it. - } catch (ClassCastException cce) { - // Not a commit type. } } -- 1.6.0.1.319.g9f32b ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [JGIT PATCH 2/7] Cleanup RevWalk.parseTree semantics 2008-09-04 23:42 ` [JGIT PATCH 1/7] Cleanup RevWalk.parseCommit, parseTree to not throw ClassCastException Shawn O. Pearce @ 2008-09-04 23:42 ` Shawn O. Pearce 2008-09-04 23:42 ` [JGIT PATCH 3/7] Fix potential NullPointerException in RevWalk.parseTree Shawn O. Pearce 0 siblings, 1 reply; 8+ messages in thread From: Shawn O. Pearce @ 2008-09-04 23:42 UTC (permalink / raw) To: Robin Rosenberg; +Cc: git When we call parseAny we may have opened the ObjectLoader for the tree we are going after. If that worked we know the object exists in the repository and is certainly recorded as type "tree" so we do not need to open the object a second time before returning to the caller. Signed-off-by: Shawn O. Pearce <spearce@spearce.org> --- .../src/org/spearce/jgit/revwalk/RevWalk.java | 18 +++++++++++++----- 1 files changed, 13 insertions(+), 5 deletions(-) diff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java index 243d9b3..5a29dcf 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java +++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java @@ -639,15 +639,21 @@ public RevTree parseTree(final AnyObjectId id) c = ((RevTag) c).getObject(); parse(c); } - if (c instanceof RevCommit) { - c = ((RevCommit) c).getTree(); - parse(c); - } else if (!(c instanceof RevTree)) + + final RevTree t; + if (c instanceof RevCommit) + t = ((RevCommit) c).getTree(); + else if (!(c instanceof RevTree)) throw new IncorrectObjectTypeException(id.toObjectId(), Constants.TYPE_TREE); - final RevTree t = (RevTree) c; + else + t = (RevTree) c; + + if ((t.flags & PARSED) != 0) + return t; if (db.openObject(t).getType() != Constants.OBJ_TREE) throw new IncorrectObjectTypeException(t, Constants.TYPE_TREE); + t.flags |= PARSED; return t; } @@ -685,10 +691,12 @@ public RevObject parseAny(final AnyObjectId id) } case Constants.OBJ_TREE: { r = new RevTree(ldr.getId()); + r.flags |= PARSED; break; } case Constants.OBJ_BLOB: { r = new RevBlob(ldr.getId()); + r.flags |= PARSED; break; } case Constants.OBJ_TAG: { -- 1.6.0.1.319.g9f32b ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [JGIT PATCH 3/7] Fix potential NullPointerException in RevWalk.parseTree 2008-09-04 23:42 ` [JGIT PATCH 2/7] Cleanup RevWalk.parseTree semantics Shawn O. Pearce @ 2008-09-04 23:42 ` Shawn O. Pearce 2008-09-04 23:42 ` [JGIT PATCH 4/7] Add prerequisite verification to the bundle transport Shawn O. Pearce 0 siblings, 1 reply; 8+ messages in thread From: Shawn O. Pearce @ 2008-09-04 23:42 UTC (permalink / raw) To: Robin Rosenberg; +Cc: git If we have previously obtained this RevTree by lookupTree (such as in a commit) but we are missing in the repository and call parseTree we want a MissingObjectException to be thrown, not an NPE. The NPE tells us nothing and could be the same as a bug internal to RevWalk. Signed-off-by: Shawn O. Pearce <spearce@spearce.org> --- .../src/org/spearce/jgit/revwalk/RevWalk.java | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java index 5a29dcf..0dead10 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java +++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java @@ -651,7 +651,10 @@ else if (!(c instanceof RevTree)) if ((t.flags & PARSED) != 0) return t; - if (db.openObject(t).getType() != Constants.OBJ_TREE) + final ObjectLoader ldr = db.openObject(t); + if (ldr == null) + throw new MissingObjectException(t, Constants.TYPE_TREE); + if (ldr.getType() != Constants.OBJ_TREE) throw new IncorrectObjectTypeException(t, Constants.TYPE_TREE); t.flags |= PARSED; return t; -- 1.6.0.1.319.g9f32b ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [JGIT PATCH 4/7] Add prerequisite verification to the bundle transport 2008-09-04 23:42 ` [JGIT PATCH 3/7] Fix potential NullPointerException in RevWalk.parseTree Shawn O. Pearce @ 2008-09-04 23:42 ` Shawn O. Pearce 2008-09-04 23:42 ` [JGIT PATCH 5/7] Include URIish in bundle transport within any TransportExceptions Shawn O. Pearce 0 siblings, 1 reply; 8+ messages in thread From: Shawn O. Pearce @ 2008-09-04 23:42 UTC (permalink / raw) To: Robin Rosenberg; +Cc: git Before we can fetch the objects contained within the bundle into the repository we need to know if we have the necessary bases in our reachable history, thus ensuring that we have what is needed to fill out the thin pack and access everything contained within it. If we are missing any of the commits named as prerequisites in the bundle header we cannot import it. I forgot to implement this when I initially added bundle support to the transport implementation. Better late then never. Signed-off-by: Shawn O. Pearce <spearce@spearce.org> --- .../errors/MissingBundlePrerequisiteException.java | 73 ++++++++++++++++++++ .../spearce/jgit/transport/TransportBundle.java | 68 ++++++++++++++++++ 2 files changed, 141 insertions(+), 0 deletions(-) create mode 100644 org.spearce.jgit/src/org/spearce/jgit/errors/MissingBundlePrerequisiteException.java diff --git a/org.spearce.jgit/src/org/spearce/jgit/errors/MissingBundlePrerequisiteException.java b/org.spearce.jgit/src/org/spearce/jgit/errors/MissingBundlePrerequisiteException.java new file mode 100644 index 0000000..ccd9281 --- /dev/null +++ b/org.spearce.jgit/src/org/spearce/jgit/errors/MissingBundlePrerequisiteException.java @@ -0,0 +1,73 @@ +/* + * 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.errors; + +import java.util.Collection; + +import org.spearce.jgit.lib.ObjectId; +import org.spearce.jgit.transport.URIish; + +/** + * Indicates a base/common object was required, but is not found. + */ +public class MissingBundlePrerequisiteException extends TransportException { + private static final long serialVersionUID = 1L; + + private static String format(final Collection<ObjectId> ids) { + final StringBuilder r = new StringBuilder(); + r.append("missing prerequisite commits:"); + for (final ObjectId p : ids) { + r.append("\n "); + r.append(p.name()); + } + return r.toString(); + } + + /** + * Constructs a MissingBundlePrerequisiteException for a set of objects. + * + * @param uri + * URI used for transport + * @param ids + * the ids of the base/common object(s) we don't have. + */ + public MissingBundlePrerequisiteException(final URIish uri, + final Collection<ObjectId> ids) { + super(uri, format(ids)); + } +} diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportBundle.java b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportBundle.java index d8c2ba4..e502619 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportBundle.java +++ b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportBundle.java @@ -1,6 +1,7 @@ /* * Copyright (C) 2008, Robin Rosenberg <robin.rosenberg@dewire.com> * Copyright (C) 2008, Shawn O. Pearce <spearce@spearce.org> + * Copyright (C) 2008, Google Inc. * * All rights reserved. * @@ -44,11 +45,15 @@ import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; +import java.util.ArrayList; import java.util.Collection; import java.util.HashSet; import java.util.LinkedHashMap; +import java.util.List; import java.util.Set; +import org.spearce.jgit.errors.MissingObjectException; +import org.spearce.jgit.errors.MissingBundlePrerequisiteException; import org.spearce.jgit.errors.NotSupportedException; import org.spearce.jgit.errors.PackProtocolException; import org.spearce.jgit.errors.TransportException; @@ -57,6 +62,10 @@ import org.spearce.jgit.lib.ProgressMonitor; import org.spearce.jgit.lib.Ref; import org.spearce.jgit.lib.Repository; +import org.spearce.jgit.revwalk.RevCommit; +import org.spearce.jgit.revwalk.RevFlag; +import org.spearce.jgit.revwalk.RevObject; +import org.spearce.jgit.revwalk.RevWalk; import org.spearce.jgit.util.FS; import org.spearce.jgit.util.RawParseUtils; @@ -200,6 +209,7 @@ private String readLine(final byte[] hdrbuf) throws IOException { @Override protected void doFetch(final ProgressMonitor monitor, final Collection<Ref> want) throws TransportException { + verifyPrerequisites(); try { final IndexPack ip = IndexPack.create(local, in); ip.setFixThin(true); @@ -214,6 +224,64 @@ protected void doFetch(final ProgressMonitor monitor, } } + private void verifyPrerequisites() throws TransportException { + if (prereqs.isEmpty()) + return; + + final RevWalk rw = new RevWalk(local); + final RevFlag PREREQ = rw.newFlag("PREREQ"); + final RevFlag SEEN = rw.newFlag("SEEN"); + + final List<ObjectId> missing = new ArrayList<ObjectId>(); + final List<RevObject> commits = new ArrayList<RevObject>(); + for (final ObjectId p : prereqs) { + try { + final RevCommit c = rw.parseCommit(p); + if (!c.has(PREREQ)) { + c.add(PREREQ); + commits.add(c); + } + } catch (MissingObjectException notFound) { + missing.add(p); + } catch (IOException err) { + throw new TransportException(uri, "Cannot read commit " + + p.name(), err); + } + } + if (!missing.isEmpty()) + throw new MissingBundlePrerequisiteException(uri, missing); + + for (final Ref r : local.getAllRefs().values()) { + try { + rw.markStart(rw.parseCommit(r.getObjectId())); + } catch (IOException readError) { + // If we cannot read the value of the ref skip it. + } + } + + int remaining = commits.size(); + try { + RevCommit c; + while ((c = rw.next()) != null) { + if (c.has(PREREQ)) { + c.add(SEEN); + if (--remaining == 0) + break; + } + } + } catch (IOException err) { + throw new TransportException(uri, "Cannot read object", err); + } + + if (remaining > 0) { + for (final RevObject o : commits) { + if (!o.has(SEEN)) + missing.add(o); + } + throw new MissingBundlePrerequisiteException(uri, missing); + } + } + @Override public void close() { if (in != null) { -- 1.6.0.1.319.g9f32b ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [JGIT PATCH 5/7] Include URIish in bundle transport within any TransportExceptions 2008-09-04 23:42 ` [JGIT PATCH 4/7] Add prerequisite verification to the bundle transport Shawn O. Pearce @ 2008-09-04 23:42 ` Shawn O. Pearce 2008-09-04 23:42 ` [JGIT PATCH 6/7] Refactor TransportBundle to not be dependent on FileInputStream Shawn O. Pearce 0 siblings, 1 reply; 8+ messages in thread From: Shawn O. Pearce @ 2008-09-04 23:42 UTC (permalink / raw) To: Robin Rosenberg; +Cc: git This way the bundle path (as entered by the user) is reported as part of the error. Signed-off-by: Shawn O. Pearce <spearce@spearce.org> --- .../spearce/jgit/transport/TransportBundle.java | 21 ++++++++----------- 1 files changed, 9 insertions(+), 12 deletions(-) diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportBundle.java b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportBundle.java index e502619..de62fb8 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportBundle.java +++ b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportBundle.java @@ -128,7 +128,7 @@ BundleFetchConnection() throws TransportException { in = new FileInputStream(bundle); bin = new RewindBufferedInputStream(in); } catch (FileNotFoundException err) { - throw new TransportException(bundle.getPath() + ": not found"); + throw new TransportException(uri, "not found"); } try { @@ -137,8 +137,7 @@ BundleFetchConnection() throws TransportException { readBundleV2(); break; default: - throw new TransportException(bundle.getPath() - + ": not a bundle"); + throw new TransportException(uri, "not a bundle"); } in.getChannel().position( @@ -149,12 +148,10 @@ BundleFetchConnection() throws TransportException { throw err; } catch (IOException err) { close(); - throw new TransportException(bundle.getPath() + ": " - + err.getMessage(), err); + throw new TransportException(uri, err.getMessage(), err); } catch (RuntimeException err) { close(); - throw new TransportException(bundle.getPath() + ": " - + err.getMessage(), err); + throw new TransportException(uri, err.getMessage(), err); } } @@ -162,7 +159,7 @@ private int readSignature() throws IOException { final String rev = readLine(new byte[1024]); if (V2_BUNDLE_SIGNATURE.equals(rev)) return 2; - throw new TransportException(bundle.getPath() + ": not a bundle"); + throw new TransportException(uri, "not a bundle"); } private void readBundleV2() throws IOException { @@ -189,8 +186,8 @@ private void readBundleV2() throws IOException { } private PackProtocolException duplicateAdvertisement(final String name) { - return new PackProtocolException("duplicate advertisements of " - + name); + return new PackProtocolException(uri, + "duplicate advertisements of " + name); } private String readLine(final byte[] hdrbuf) throws IOException { @@ -217,10 +214,10 @@ protected void doFetch(final ProgressMonitor monitor, ip.renameAndOpenPack(); } catch (IOException err) { close(); - throw new TransportException(err.getMessage(), err); + throw new TransportException(uri, err.getMessage(), err); } catch (RuntimeException err) { close(); - throw new TransportException(err.getMessage(), err); + throw new TransportException(uri, err.getMessage(), err); } } -- 1.6.0.1.319.g9f32b ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [JGIT PATCH 6/7] Refactor TransportBundle to not be dependent on FileInputStream 2008-09-04 23:42 ` [JGIT PATCH 5/7] Include URIish in bundle transport within any TransportExceptions Shawn O. Pearce @ 2008-09-04 23:42 ` Shawn O. Pearce 2008-09-04 23:42 ` [JGIT PATCH 7/7] Refactor bundle transport to permit streaming from application Shawn O. Pearce 0 siblings, 1 reply; 8+ messages in thread From: Shawn O. Pearce @ 2008-09-04 23:42 UTC (permalink / raw) To: Robin Rosenberg; +Cc: git By using a straight up BufferedInputStream and making it play nicely with IndexPack's internal buffering we can permit TransportBundle to accept input from any sort of InputStream, not just those that have a positionable Channel underneath of them. This makes it possible to stream a bundle over the network, or from a byte[] in memory. IndexPack now publishes its BUFFER_SIZE member so callers can create their own BufferedInputStream with the same size buffer. This way we can be sure that reads after the first bypass BufferedInputStream's buffer and go directly into IndexPack's buffer. The BUFFER_SIZE is increased to 8192 as that is the same size that is used by default in BufferedInputStream in Java 6. Signed-off-by: Shawn O. Pearce <spearce@spearce.org> --- .../spearce/jgit/transport/BasePackConnection.java | 2 +- .../src/org/spearce/jgit/transport/IndexPack.java | 20 +++++++-- .../spearce/jgit/transport/TransportBundle.java | 43 ++++++-------------- 3 files changed, 30 insertions(+), 35 deletions(-) diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackConnection.java b/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackConnection.java index 6101802..184a5fd 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackConnection.java +++ b/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackConnection.java @@ -101,7 +101,7 @@ BasePackConnection(final PackTransport packTransport) { protected void init(final InputStream myIn, final OutputStream myOut) { in = myIn instanceof BufferedInputStream ? myIn - : new BufferedInputStream(myIn); + : new BufferedInputStream(myIn, IndexPack.BUFFER_SIZE); out = myOut instanceof BufferedOutputStream ? myOut : new BufferedOutputStream(myOut); diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java b/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java index 2c98823..bc52896 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java +++ b/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java @@ -76,20 +76,30 @@ /** Progress message when computing names of delta compressed objects. */ public static final String PROGRESS_RESOLVE_DELTA = "Resolving deltas"; - private static final int BUFFER_SIZE = 4096; + /** + * Size of the internal stream buffer. + * <p> + * If callers are going to be supplying IndexPack a BufferedInputStream they + * should use this buffer size as the size of the buffer for that + * BufferedInputStream, and any other its may be wrapping. This way the + * buffers will cascade efficiently and only the IndexPack buffer will be + * receiving the bulk of the data stream. + */ + public static final int BUFFER_SIZE = 8192; /** * Create an index pack instance to load a new pack into a repository. * <p> * The received pack data and generated index will be saved to temporary - * files within the repository's <code>objects</code> directory. To use - * the data contained within them call {@link #renameAndOpenPack()} once the + * files within the repository's <code>objects</code> directory. To use the + * data contained within them call {@link #renameAndOpenPack()} once the * indexing is complete. * * @param db * the repository that will receive the new pack. * @param is - * stream to read the pack data from. + * stream to read the pack data from. If the stream is buffered + * use {@link #BUFFER_SIZE} as the buffer size for the stream. * @return a new index pack instance. * @throws IOException * a temporary file could not be created. @@ -164,6 +174,8 @@ public static IndexPack create(final Repository db, final InputStream is) * * @param db * @param src + * stream to read the pack data from. If the stream is buffered + * use {@link #BUFFER_SIZE} as the buffer size for the stream. * @param dstBase * @throws IOException * the output packfile could not be created. diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportBundle.java b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportBundle.java index de62fb8..765fc0f 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportBundle.java +++ b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportBundle.java @@ -102,7 +102,13 @@ TransportBundle(final Repository local, final URIish uri) { @Override public FetchConnection openFetch() throws NotSupportedException, TransportException { - return new BundleFetchConnection(); + final InputStream src; + try { + src = new FileInputStream(bundle); + } catch (FileNotFoundException err) { + throw new TransportException(uri, "not found"); + } + return new BundleFetchConnection(src); } @Override @@ -117,20 +123,12 @@ public void close() { } class BundleFetchConnection extends BaseFetchConnection { - FileInputStream in; - - RewindBufferedInputStream bin; + InputStream bin; final Set<ObjectId> prereqs = new HashSet<ObjectId>(); - BundleFetchConnection() throws TransportException { - try { - in = new FileInputStream(bundle); - bin = new RewindBufferedInputStream(in); - } catch (FileNotFoundException err) { - throw new TransportException(uri, "not found"); - } - + BundleFetchConnection(final InputStream src) throws TransportException { + bin = new BufferedInputStream(src, IndexPack.BUFFER_SIZE); try { switch (readSignature()) { case 2: @@ -139,10 +137,6 @@ BundleFetchConnection() throws TransportException { default: throw new TransportException(uri, "not a bundle"); } - - in.getChannel().position( - in.getChannel().position() - bin.buffered()); - bin = null; } catch (TransportException err) { close(); throw err; @@ -208,7 +202,7 @@ protected void doFetch(final ProgressMonitor monitor, final Collection<Ref> want) throws TransportException { verifyPrerequisites(); try { - final IndexPack ip = IndexPack.create(local, in); + final IndexPack ip = IndexPack.create(local, bin); ip.setFixThin(true); ip.index(monitor); ip.renameAndOpenPack(); @@ -281,26 +275,15 @@ private void verifyPrerequisites() throws TransportException { @Override public void close() { - if (in != null) { + if (bin != null) { try { - in.close(); + bin.close(); } catch (IOException ie) { // Ignore close failures. } finally { - in = null; bin = null; } } } - - class RewindBufferedInputStream extends BufferedInputStream { - RewindBufferedInputStream(final InputStream src) { - super(src); - } - - int buffered() { - return (count - pos); - } - } } } -- 1.6.0.1.319.g9f32b ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [JGIT PATCH 7/7] Refactor bundle transport to permit streaming from application 2008-09-04 23:42 ` [JGIT PATCH 6/7] Refactor TransportBundle to not be dependent on FileInputStream Shawn O. Pearce @ 2008-09-04 23:42 ` Shawn O. Pearce 0 siblings, 0 replies; 8+ messages in thread From: Shawn O. Pearce @ 2008-09-04 23:42 UTC (permalink / raw) To: Robin Rosenberg; +Cc: git The new TransportBundleStream is public and can be created by an application directly from any InputStream. This permits the app to obtain the bundle data on its own via any means and feed it into the bundle transport. Signed-off-by: Shawn O. Pearce <spearce@spearce.org> --- .../src/org/spearce/jgit/transport/Transport.java | 4 +- .../spearce/jgit/transport/TransportBundle.java | 36 +------- .../jgit/transport/TransportBundleFile.java | 82 +++++++++++++++ .../jgit/transport/TransportBundleStream.java | 105 ++++++++++++++++++++ 4 files changed, 191 insertions(+), 36 deletions(-) create mode 100644 org.spearce.jgit/src/org/spearce/jgit/transport/TransportBundleFile.java create mode 100644 org.spearce.jgit/src/org/spearce/jgit/transport/TransportBundleStream.java diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/Transport.java b/org.spearce.jgit/src/org/spearce/jgit/transport/Transport.java index 65f2c6b..7284b28 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/transport/Transport.java +++ b/org.spearce.jgit/src/org/spearce/jgit/transport/Transport.java @@ -204,8 +204,8 @@ else if (TransportGitAnon.canHandle(remote)) else if (TransportAmazonS3.canHandle(remote)) return new TransportAmazonS3(local, remote); - else if (TransportBundle.canHandle(remote)) - return new TransportBundle(local, remote); + else if (TransportBundleFile.canHandle(remote)) + return new TransportBundleFile(local, remote); else if (TransportLocal.canHandle(remote)) return new TransportLocal(local, remote); diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportBundle.java b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportBundle.java index 765fc0f..fed34e8 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportBundle.java +++ b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportBundle.java @@ -40,9 +40,6 @@ package org.spearce.jgit.transport; import java.io.BufferedInputStream; -import java.io.File; -import java.io.FileInputStream; -import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; import java.util.ArrayList; @@ -52,8 +49,8 @@ import java.util.List; import java.util.Set; -import org.spearce.jgit.errors.MissingObjectException; import org.spearce.jgit.errors.MissingBundlePrerequisiteException; +import org.spearce.jgit.errors.MissingObjectException; import org.spearce.jgit.errors.NotSupportedException; import org.spearce.jgit.errors.PackProtocolException; import org.spearce.jgit.errors.TransportException; @@ -66,7 +63,6 @@ import org.spearce.jgit.revwalk.RevFlag; import org.spearce.jgit.revwalk.RevObject; import org.spearce.jgit.revwalk.RevWalk; -import org.spearce.jgit.util.FS; import org.spearce.jgit.util.RawParseUtils; /** @@ -76,39 +72,11 @@ * communicate with to decide what the peer already knows. So push is not * supported by the bundle transport. */ -class TransportBundle extends PackTransport { +abstract class TransportBundle extends PackTransport { static final String V2_BUNDLE_SIGNATURE = "# v2 git bundle"; - static boolean canHandle(final URIish uri) { - if (uri.getHost() != null || uri.getPort() > 0 || uri.getUser() != null - || uri.getPass() != null || uri.getPath() == null) - return false; - - if ("file".equals(uri.getScheme()) || uri.getScheme() == null) { - final File f = FS.resolve(new File("."), uri.getPath()); - return f.isFile() || f.getName().endsWith(".bundle"); - } - - return false; - } - - private final File bundle; - TransportBundle(final Repository local, final URIish uri) { super(local, uri); - bundle = FS.resolve(new File("."), uri.getPath()).getAbsoluteFile(); - } - - @Override - public FetchConnection openFetch() throws NotSupportedException, - TransportException { - final InputStream src; - try { - src = new FileInputStream(bundle); - } catch (FileNotFoundException err) { - throw new TransportException(uri, "not found"); - } - return new BundleFetchConnection(src); } @Override diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportBundleFile.java b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportBundleFile.java new file mode 100644 index 0000000..c9ff1b2 --- /dev/null +++ b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportBundleFile.java @@ -0,0 +1,82 @@ +/* + * Copyright (C) 2008, Shawn O. Pearce <spearce@spearce.org> + * + * 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.transport; + +import java.io.File; +import java.io.FileInputStream; +import java.io.FileNotFoundException; +import java.io.InputStream; + +import org.spearce.jgit.errors.NotSupportedException; +import org.spearce.jgit.errors.TransportException; +import org.spearce.jgit.lib.Repository; +import org.spearce.jgit.util.FS; + +class TransportBundleFile extends TransportBundle { + static boolean canHandle(final URIish uri) { + if (uri.getHost() != null || uri.getPort() > 0 || uri.getUser() != null + || uri.getPass() != null || uri.getPath() == null) + return false; + + if ("file".equals(uri.getScheme()) || uri.getScheme() == null) { + final File f = FS.resolve(new File("."), uri.getPath()); + return f.isFile() || f.getName().endsWith(".bundle"); + } + + return false; + } + + private final File bundle; + + TransportBundleFile(final Repository local, final URIish uri) { + super(local, uri); + bundle = FS.resolve(new File("."), uri.getPath()).getAbsoluteFile(); + } + + @Override + public FetchConnection openFetch() throws NotSupportedException, + TransportException { + final InputStream src; + try { + src = new FileInputStream(bundle); + } catch (FileNotFoundException err) { + throw new TransportException(uri, "not found"); + } + return new BundleFetchConnection(src); + } +} diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportBundleStream.java b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportBundleStream.java new file mode 100644 index 0000000..0272261 --- /dev/null +++ b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportBundleStream.java @@ -0,0 +1,105 @@ +/* + * 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.transport; + +import java.io.IOException; +import java.io.InputStream; + +import org.spearce.jgit.errors.TransportException; +import org.spearce.jgit.lib.Repository; + +/** + * Single shot fetch from a streamed Git bundle. + * <p> + * The bundle is read from an unbuffered input stream, which limits the + * transport to opening at most one FetchConnection before needing to recreate + * the transport instance. + */ +public class TransportBundleStream extends TransportBundle { + private InputStream src; + + /** + * Create a new transport to fetch objects from a streamed bundle. + * <p> + * The stream can be unbuffered (buffering is automatically provided + * internally to smooth out short reads) and unpositionable (the stream is + * read from only once, sequentially). + * <p> + * When the FetchConnection or the this instance is closed the supplied + * input stream is also automatically closed. This frees callers from + * needing to keep track of the supplied stream. + * + * @param db + * repository the fetched objects will be loaded into. + * @param uri + * symbolic name of the source of the stream. The URI can + * reference a non-existent resource. It is used only for + * exception reporting. + * @param in + * the stream to read the bundle from. + */ + public TransportBundleStream(final Repository db, final URIish uri, + final InputStream in) { + super(db, uri); + src = in; + } + + @Override + public FetchConnection openFetch() throws TransportException { + if (src == null) + throw new TransportException(uri, "Only one fetch supported"); + try { + return new BundleFetchConnection(src); + } finally { + src = null; + } + } + + @Override + public void close() { + if (src != null) { + try { + src.close(); + } catch (IOException err) { + // Ignore a close error. + } finally { + src = null; + } + } + } +} -- 1.6.0.1.319.g9f32b ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2008-09-04 23:44 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-09-04 23:42 [JGIT PATCH 0/7] Misc. RevWalk, bundle transport improvements Shawn O. Pearce 2008-09-04 23:42 ` [JGIT PATCH 1/7] Cleanup RevWalk.parseCommit, parseTree to not throw ClassCastException Shawn O. Pearce 2008-09-04 23:42 ` [JGIT PATCH 2/7] Cleanup RevWalk.parseTree semantics Shawn O. Pearce 2008-09-04 23:42 ` [JGIT PATCH 3/7] Fix potential NullPointerException in RevWalk.parseTree Shawn O. Pearce 2008-09-04 23:42 ` [JGIT PATCH 4/7] Add prerequisite verification to the bundle transport Shawn O. Pearce 2008-09-04 23:42 ` [JGIT PATCH 5/7] Include URIish in bundle transport within any TransportExceptions Shawn O. Pearce 2008-09-04 23:42 ` [JGIT PATCH 6/7] Refactor TransportBundle to not be dependent on FileInputStream Shawn O. Pearce 2008-09-04 23:42 ` [JGIT PATCH 7/7] Refactor bundle transport to permit streaming from application Shawn O. Pearce
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).