{"thread":{"id":"18577","subject":"[JGIT PATCH] Simplify the PackWriter.preparePack() API","startedAt":"2009-03-26T19:42:47Z","lastAt":"2009-03-26T19:42:47Z","messageCount":1,"participants":["Shawn O. Pearce"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"109564","messageId":"1238096567-27567-1-git-send-email-spearce@spearce.org","threadId":"18577","inReplyTo":null,"subject":"[JGIT PATCH] Simplify the PackWriter.preparePack() API","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-03-26T19:42:47Z","receivedAt":"2009-03-26T19:42:47Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"It has always bothered me that we had two boolean \"mode\" parameters\nas part of the preparePack method call, when most other options were\nset by standard setter methods.  In all current callers (except the\nunit tests) the ignoreMissingUninteresting was always set to true,\nand in many of the callers, packthin was false.  Moving these to\nsetters with a reasonable default simplifies callers considerably.\n\nSigned-off-by: Shawn O. Pearce <sop@google.com>\n---\n .../tst/org/spearce/jgit/lib/PackWriterTest.java   |    5 +-\n .../src/org/spearce/jgit/lib/PackWriter.java       |   78 +++++++++++++-------\n .../jgit/transport/BasePackPushConnection.java     |    3 +-\n .../org/spearce/jgit/transport/BundleWriter.java   |    3 +-\n .../src/org/spearce/jgit/transport/UploadPack.java |    3 +-\n .../spearce/jgit/transport/WalkPushConnection.java |    2 +-\n 6 files changed, 62 insertions(+), 32 deletions(-)\n\ndiff --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\nindex f7139fc..5ebca09 100644\n--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/PackWriterTest.java\n+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/PackWriterTest.java\n@@ -442,8 +442,9 @@ private void createVerifyOpenPack(final Collection<ObjectId> interestings,\n \t\t\tfinal Collection<ObjectId> uninterestings, final boolean thin,\n \t\t\tfinal boolean ignoreMissingUninteresting)\n \t\t\tthrows MissingObjectException, IOException {\n-\t\twriter.preparePack(interestings, uninterestings, thin,\n-\t\t\t\tignoreMissingUninteresting);\n+\t\twriter.setThin(thin);\n+\t\twriter.setIgnoreMissingUninteresting(ignoreMissingUninteresting);\n+\t\twriter.preparePack(interestings, uninterestings);\n \t\twriter.writePack(cos);\n \t\tverifyOpenPack(thin);\n \t}\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/PackWriter.java b/org.spearce.jgit/src/org/spearce/jgit/lib/PackWriter.java\nindex 601ce71..83506bd 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/PackWriter.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/PackWriter.java\n@@ -78,7 +78,7 @@\n  * Typical usage consists of creating instance intended for some pack,\n  * configuring options, preparing the list of objects by calling\n  * {@link #preparePack(Iterator)} or\n- * {@link #preparePack(Collection, Collection, boolean, boolean)}, and finally\n+ * {@link #preparePack(Collection, Collection)}, and finally\n  * producing the stream with {@link #writePack(OutputStream)}.\n  * </p>\n  * <p>\n@@ -99,7 +99,7 @@\n \t * Title of {@link ProgressMonitor} task used during counting objects to\n \t * pack.\n \t *\n-\t * @see #preparePack(Collection, Collection, boolean, boolean)\n+\t * @see #preparePack(Collection, Collection)\n \t */\n \tpublic static final String COUNTING_OBJECTS_PROGRESS = \"Counting objects\";\n \n@@ -196,18 +196,20 @@\n \n \tprivate boolean thin;\n \n+\tprivate boolean ignoreMissingUninteresting = true;\n+\t\n \t/**\n \t * Create writer for specified repository.\n \t * <p>\n \t * Objects for packing are specified in {@link #preparePack(Iterator)} or\n-\t * {@link #preparePack(Collection, Collection, boolean, boolean)}.\n+\t * {@link #preparePack(Collection, Collection)}.\n \t *\n \t * @param repo\n \t *            repository where objects are stored.\n \t * @param monitor\n \t *            operations progress monitor, used within\n \t *            {@link #preparePack(Iterator)},\n-\t *            {@link #preparePack(Collection, Collection, boolean, boolean)}\n+\t *            {@link #preparePack(Collection, Collection)}\n \t *            , or {@link #writePack(OutputStream)}.\n \t */\n \tpublic PackWriter(final Repository repo, final ProgressMonitor monitor) {\n@@ -218,15 +220,14 @@ public PackWriter(final Repository repo, final ProgressMonitor monitor) {\n \t * Create writer for specified repository.\n \t * <p>\n \t * Objects for packing are specified in {@link #preparePack(Iterator)} or\n-\t * {@link #preparePack(Collection, Collection, boolean, boolean)}.\n+\t * {@link #preparePack(Collection, Collection)}.\n \t *\n \t * @param repo\n \t *            repository where objects are stored.\n \t * @param imonitor\n \t *            operations progress monitor, used within\n \t *            {@link #preparePack(Iterator)},\n-\t *            {@link #preparePack(Collection, Collection, boolean, boolean)}\n-\t *            ;\n+\t *            {@link #preparePack(Collection, Collection)}\n \t * @param wmonitor\n \t *            operations progress monitor, used within\n \t *            {@link #writePack(OutputStream)}.\n@@ -259,7 +260,7 @@ public boolean isReuseDeltas() {\n \t * use it if possible. Normally, only deltas with base to another object\n \t * existing in set of objects to pack will be used. Exception is however\n \t * thin-pack (see\n-\t * {@link #preparePack(Collection, Collection, boolean, boolean)} and\n+\t * {@link #preparePack(Collection, Collection)} and\n \t * {@link #preparePack(Iterator)}) where base object must exist on other\n \t * side machine.\n \t * <p>\n@@ -367,6 +368,45 @@ public void setMaxDeltaDepth(int maxDeltaDepth) {\n \t\tthis.maxDeltaDepth = maxDeltaDepth;\n \t}\n \n+\t/** @return true if this writer is producing a thin pack. */\n+\tpublic boolean isThin() {\n+\t\treturn thin;\n+\t}\n+\n+\t/**\n+\t * @param packthin\n+\t *            a boolean indicating whether writer may pack objects with\n+\t *            delta base object not within set of objects to pack, but\n+\t *            belonging to party repository (uninteresting/boundary) as\n+\t *            determined by set; this kind of pack is used only for\n+\t *            transport; true - to produce thin pack, false - otherwise.\n+\t */\n+\tpublic void setThin(final boolean packthin) {\n+\t\tthin = packthin;\n+\t}\n+\n+\t/**\n+\t * @return true to ignore objects that are uninteresting and also not found\n+\t *         on local disk; false to throw a {@link MissingObjectException}\n+\t *         out of {@link #preparePack(Collection, Collection)} if an\n+\t *         uninteresting object is not in the source repository. By default,\n+\t *         true, permitting gracefully ignoring of uninteresting objects.\n+\t */\n+\tpublic boolean isIgnoreMissingUninteresting() {\n+\t\treturn ignoreMissingUninteresting;\n+\t}\n+\t\n+\t/**\n+\t * @param ignore\n+\t *            true if writer should ignore non existing uninteresting\n+\t *            objects during construction set of objects to pack; false\n+\t *            otherwise - non existing uninteresting objects may cause\n+\t *            {@link MissingObjectException}\n+\t */\n+\tpublic void setIgnoreMissingUninteresting(final boolean ignore) {\n+\t\tignoreMissingUninteresting = ignore;\n+\t}\n+\n \t/**\n \t * Set the pack index file format version this instance will create.\n \t *\n@@ -447,28 +487,15 @@ public void preparePack(final Iterator<RevObject> objectsSource)\n \t * @param uninterestingObjects\n \t *            collection of objects to be marked as uninteresting (end\n \t *            points of graph traversal).\n-\t * @param packthin\n-\t *            a boolean indicating whether writer may pack objects with\n-\t *            delta base object not within set of objects to pack, but\n-\t *            belonging to party repository (uninteresting/boundary) as\n-\t *            determined by set; this kind of pack is used only for\n-\t *            transport; true - to produce thin pack, false - otherwise.\n-\t * @param ignoreMissingUninteresting\n-\t *            true if writer should ignore non existing uninteresting\n-\t *            objects during construction set of objects to pack; false\n-\t *            otherwise - non existing uninteresting objects may cause\n-\t *            {@link MissingObjectException}\n \t * @throws IOException\n \t *             when some I/O problem occur during reading objects.\n \t */\n \tpublic void preparePack(\n \t\t\tfinal Collection<? extends ObjectId> interestingObjects,\n-\t\t\tfinal Collection<? extends ObjectId> uninterestingObjects,\n-\t\t\tfinal boolean packthin, final boolean ignoreMissingUninteresting)\n+\t\t\tfinal Collection<? extends ObjectId> uninterestingObjects)\n \t\t\tthrows IOException {\n-\t\tthis.thin = packthin;\n \t\tObjectWalk walker = setUpWalker(interestingObjects,\n-\t\t\t\tuninterestingObjects, ignoreMissingUninteresting);\n+\t\t\t\tuninterestingObjects);\n \t\tfindObjectsToPack(walker);\n \t}\n \n@@ -502,7 +529,7 @@ public ObjectId computeName() {\n \t * Create an index file to match the pack file just written.\n \t * <p>\n \t * This method can only be invoked after {@link #preparePack(Iterator)} or\n-\t * {@link #preparePack(Collection, Collection, boolean, boolean)} has been\n+\t * {@link #preparePack(Collection, Collection)} has been\n \t * invoked and completed successfully. Writing a corresponding index is an\n \t * optional feature that not all pack users may require.\n \t *\n@@ -760,8 +787,7 @@ private void writeChecksum() throws IOException {\n \n \tprivate ObjectWalk setUpWalker(\n \t\t\tfinal Collection<? extends ObjectId> interestingObjects,\n-\t\t\tfinal Collection<? extends ObjectId> uninterestingObjects,\n-\t\t\tfinal boolean ignoreMissingUninteresting)\n+\t\t\tfinal Collection<? extends ObjectId> uninterestingObjects)\n \t\t\tthrows MissingObjectException, IOException,\n \t\t\tIncorrectObjectTypeException {\n \t\tfinal ObjectWalk walker = new ObjectWalk(db);\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackPushConnection.java b/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackPushConnection.java\nindex a078d7e..dde1d26 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackPushConnection.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackPushConnection.java\n@@ -201,7 +201,8 @@ private void writePack(final Map<String, RemoteRefUpdate> refUpdates,\n \t\t\t\tnewObjects.add(r.getNewObjectId());\n \t\t}\n \n-\t\twriter.preparePack(newObjects, remoteObjects, thinPack, true);\n+\t\twriter.setThin(thinPack);\n+\t\twriter.preparePack(newObjects, remoteObjects);\n \t\twriter.writePack(out);\n \t}\n \ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/BundleWriter.java b/org.spearce.jgit/src/org/spearce/jgit/transport/BundleWriter.java\nindex a22a31d..8000837 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/transport/BundleWriter.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/BundleWriter.java\n@@ -166,7 +166,8 @@ public void writeBundle(OutputStream os) throws IOException {\n \t\tinc.addAll(include.values());\n \t\tfor (final RevCommit r : assume)\n \t\t\texc.add(r.getId());\n-\t\tpackWriter.preparePack(inc, exc, exc.size() > 0, true);\n+\t\tpackWriter.setThin(exc.size() > 0);\n+\t\tpackWriter.preparePack(inc, exc);\n \n \t\tfinal Writer w = new OutputStreamWriter(os, Constants.CHARSET);\n \t\tw.write(TransportBundle.V2_BUNDLE_SIGNATURE);\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/UploadPack.java b/org.spearce.jgit/src/org/spearce/jgit/transport/UploadPack.java\nindex 204859c..45d57b3 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/transport/UploadPack.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/UploadPack.java\n@@ -464,7 +464,8 @@ private void sendPack() throws IOException {\n \t\tfinal PackWriter pw;\n \t\tpw = new PackWriter(db, pm, NullProgressMonitor.INSTANCE);\n \t\tpw.setDeltaBaseAsOffset(options.contains(OPTION_OFS_DELTA));\n-\t\tpw.preparePack(wantAll, commonBase, thin, true);\n+\t\tpw.setThin(thin);\n+\t\tpw.preparePack(wantAll, commonBase);\n \t\tif (options.contains(OPTION_INCLUDE_TAG)) {\n \t\t\tfor (final Ref r : refs.values()) {\n \t\t\t\tfinal RevObject o;\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/WalkPushConnection.java b/org.spearce.jgit/src/org/spearce/jgit/transport/WalkPushConnection.java\nindex 3246ee6..acdb5b8 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/transport/WalkPushConnection.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/WalkPushConnection.java\n@@ -210,7 +210,7 @@ private void sendpack(final List<RemoteRefUpdate> updates,\n \t\t\t\tif (r.getPeeledObjectId() != null)\n \t\t\t\t\thave.add(r.getPeeledObjectId());\n \t\t\t}\n-\t\t\tpw.preparePack(need, have, false, true);\n+\t\t\tpw.preparePack(need, have);\n \n \t\t\t// We don't have to continue further if the pack will\n \t\t\t// be an empty pack, as the remote has all objects it\n-- \n1.6.2.1.471.g682837\n"}]}