From 7723b585c0bebdff7696a288004304f8f7a3c0c0 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Thu, 26 Mar 2009 12:42:47 -0700 Subject: [PATCH] Simplify the PackWriter.preparePack() API It has always bothered me that we had two boolean "mode" parameters as part of the preparePack method call, when most other options were set by standard setter methods. In all current callers (except the unit tests) the ignoreMissingUninteresting was always set to true, and in many of the callers, packthin was false. Moving these to setters with a reasonable default simplifies callers considerably. Signed-off-by: Shawn O. Pearce Signed-off-by: Robin Rosenberg --- .../tst/org/spearce/jgit/lib/PackWriterTest.java | 5 +- .../src/org/spearce/jgit/lib/PackWriter.java | 78 ++++++++++++++-------- .../jgit/transport/BasePackPushConnection.java | 3 +- .../org/spearce/jgit/transport/BundleWriter.java | 3 +- .../src/org/spearce/jgit/transport/UploadPack.java | 3 +- .../spearce/jgit/transport/WalkPushConnection.java | 2 +- 6 files changed, 62 insertions(+), 32 deletions(-) diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/PackWriterTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/PackWriterTest.java index 636059f3..8bd51dc0 100644 --- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/PackWriterTest.java +++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/PackWriterTest.java @@ -473,8 +473,9 @@ public class PackWriterTest extends RepositoryTestCase { final Collection uninterestings, final boolean thin, final boolean ignoreMissingUninteresting) throws MissingObjectException, IOException { - writer.preparePack(interestings, uninterestings, thin, - ignoreMissingUninteresting); + writer.setThin(thin); + writer.setIgnoreMissingUninteresting(ignoreMissingUninteresting); + writer.preparePack(interestings, uninterestings); writer.writePack(cos); verifyOpenPack(thin); } diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/PackWriter.java b/org.spearce.jgit/src/org/spearce/jgit/lib/PackWriter.java index 2d05c4ef..c2e6c4b8 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/lib/PackWriter.java +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/PackWriter.java @@ -75,7 +75,7 @@ import org.spearce.jgit.util.NB; * Typical usage consists of creating instance intended for some pack, * configuring options, preparing the list of objects by calling * {@link #preparePack(Iterator)} or - * {@link #preparePack(Collection, Collection, boolean, boolean)}, and finally + * {@link #preparePack(Collection, Collection)}, and finally * producing the stream with {@link #writePack(OutputStream)}. *

*

@@ -96,7 +96,7 @@ public class PackWriter { * Title of {@link ProgressMonitor} task used during counting objects to * pack. * - * @see #preparePack(Collection, Collection, boolean, boolean) + * @see #preparePack(Collection, Collection) */ public static final String COUNTING_OBJECTS_PROGRESS = "Counting objects"; @@ -191,18 +191,20 @@ public class PackWriter { private boolean thin; + private boolean ignoreMissingUninteresting = true; + /** * Create writer for specified repository. *

* Objects for packing are specified in {@link #preparePack(Iterator)} or - * {@link #preparePack(Collection, Collection, boolean, boolean)}. + * {@link #preparePack(Collection, Collection)}. * * @param repo * repository where objects are stored. * @param monitor * operations progress monitor, used within * {@link #preparePack(Iterator)}, - * {@link #preparePack(Collection, Collection, boolean, boolean)} + * {@link #preparePack(Collection, Collection)} * , or {@link #writePack(OutputStream)}. */ public PackWriter(final Repository repo, final ProgressMonitor monitor) { @@ -213,15 +215,14 @@ public class PackWriter { * Create writer for specified repository. *

* Objects for packing are specified in {@link #preparePack(Iterator)} or - * {@link #preparePack(Collection, Collection, boolean, boolean)}. + * {@link #preparePack(Collection, Collection)}. * * @param repo * repository where objects are stored. * @param imonitor * operations progress monitor, used within * {@link #preparePack(Iterator)}, - * {@link #preparePack(Collection, Collection, boolean, boolean)} - * ; + * {@link #preparePack(Collection, Collection)} * @param wmonitor * operations progress monitor, used within * {@link #writePack(OutputStream)}. @@ -254,7 +255,7 @@ public class PackWriter { * use it if possible. Normally, only deltas with base to another object * existing in set of objects to pack will be used. Exception is however * thin-pack (see - * {@link #preparePack(Collection, Collection, boolean, boolean)} and + * {@link #preparePack(Collection, Collection)} and * {@link #preparePack(Iterator)}) where base object must exist on other * side machine. *

@@ -362,6 +363,45 @@ public class PackWriter { this.maxDeltaDepth = maxDeltaDepth; } + /** @return true if this writer is producing a thin pack. */ + public boolean isThin() { + return thin; + } + + /** + * @param packthin + * a boolean indicating whether writer may pack objects with + * delta base object not within set of objects to pack, but + * belonging to party repository (uninteresting/boundary) as + * determined by set; this kind of pack is used only for + * transport; true - to produce thin pack, false - otherwise. + */ + public void setThin(final boolean packthin) { + thin = packthin; + } + + /** + * @return true to ignore objects that are uninteresting and also not found + * on local disk; false to throw a {@link MissingObjectException} + * out of {@link #preparePack(Collection, Collection)} if an + * uninteresting object is not in the source repository. By default, + * true, permitting gracefully ignoring of uninteresting objects. + */ + public boolean isIgnoreMissingUninteresting() { + return ignoreMissingUninteresting; + } + + /** + * @param ignore + * true if writer should ignore non existing uninteresting + * objects during construction set of objects to pack; false + * otherwise - non existing uninteresting objects may cause + * {@link MissingObjectException} + */ + public void setIgnoreMissingUninteresting(final boolean ignore) { + ignoreMissingUninteresting = ignore; + } + /** * Set the pack index file format version this instance will create. * @@ -442,28 +482,15 @@ public class PackWriter { * @param uninterestingObjects * collection of objects to be marked as uninteresting (end * points of graph traversal). - * @param packthin - * a boolean indicating whether writer may pack objects with - * delta base object not within set of objects to pack, but - * belonging to party repository (uninteresting/boundary) as - * determined by set; this kind of pack is used only for - * transport; true - to produce thin pack, false - otherwise. - * @param ignoreMissingUninteresting - * true if writer should ignore non existing uninteresting - * objects during construction set of objects to pack; false - * otherwise - non existing uninteresting objects may cause - * {@link MissingObjectException} * @throws IOException * when some I/O problem occur during reading objects. */ public void preparePack( final Collection interestingObjects, - final Collection uninterestingObjects, - final boolean packthin, final boolean ignoreMissingUninteresting) + final Collection uninterestingObjects) throws IOException { - this.thin = packthin; ObjectWalk walker = setUpWalker(interestingObjects, - uninterestingObjects, ignoreMissingUninteresting); + uninterestingObjects); findObjectsToPack(walker); } @@ -497,7 +524,7 @@ public class PackWriter { * Create an index file to match the pack file just written. *

* This method can only be invoked after {@link #preparePack(Iterator)} or - * {@link #preparePack(Collection, Collection, boolean, boolean)} has been + * {@link #preparePack(Collection, Collection)} has been * invoked and completed successfully. Writing a corresponding index is an * optional feature that not all pack users may require. * @@ -755,8 +782,7 @@ public class PackWriter { private ObjectWalk setUpWalker( final Collection interestingObjects, - final Collection uninterestingObjects, - final boolean ignoreMissingUninteresting) + final Collection uninterestingObjects) throws MissingObjectException, IOException, IncorrectObjectTypeException { final ObjectWalk walker = new ObjectWalk(db); diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackPushConnection.java b/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackPushConnection.java index a078d7ef..dde1d261 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackPushConnection.java +++ b/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackPushConnection.java @@ -201,7 +201,8 @@ class BasePackPushConnection extends BasePackConnection implements newObjects.add(r.getNewObjectId()); } - writer.preparePack(newObjects, remoteObjects, thinPack, true); + writer.setThin(thinPack); + writer.preparePack(newObjects, remoteObjects); writer.writePack(out); } diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/BundleWriter.java b/org.spearce.jgit/src/org/spearce/jgit/transport/BundleWriter.java index a22a31dd..8000837d 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/transport/BundleWriter.java +++ b/org.spearce.jgit/src/org/spearce/jgit/transport/BundleWriter.java @@ -166,7 +166,8 @@ public class BundleWriter { inc.addAll(include.values()); for (final RevCommit r : assume) exc.add(r.getId()); - packWriter.preparePack(inc, exc, exc.size() > 0, true); + packWriter.setThin(exc.size() > 0); + packWriter.preparePack(inc, exc); final Writer w = new OutputStreamWriter(os, Constants.CHARSET); w.write(TransportBundle.V2_BUNDLE_SIGNATURE); diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/UploadPack.java b/org.spearce.jgit/src/org/spearce/jgit/transport/UploadPack.java index 204859c3..45d57b37 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/transport/UploadPack.java +++ b/org.spearce.jgit/src/org/spearce/jgit/transport/UploadPack.java @@ -464,7 +464,8 @@ public class UploadPack { final PackWriter pw; pw = new PackWriter(db, pm, NullProgressMonitor.INSTANCE); pw.setDeltaBaseAsOffset(options.contains(OPTION_OFS_DELTA)); - pw.preparePack(wantAll, commonBase, thin, true); + pw.setThin(thin); + pw.preparePack(wantAll, commonBase); if (options.contains(OPTION_INCLUDE_TAG)) { for (final Ref r : refs.values()) { final RevObject o; diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/WalkPushConnection.java b/org.spearce.jgit/src/org/spearce/jgit/transport/WalkPushConnection.java index 3246ee65..acdb5b8b 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/transport/WalkPushConnection.java +++ b/org.spearce.jgit/src/org/spearce/jgit/transport/WalkPushConnection.java @@ -210,7 +210,7 @@ class WalkPushConnection extends BaseConnection implements PushConnection { if (r.getPeeledObjectId() != null) have.add(r.getPeeledObjectId()); } - pw.preparePack(need, have, false, true); + pw.preparePack(need, have); // We don't have to continue further if the pack will // be an empty pack, as the remote has all objects it -- 2.11.4.GIT