git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [JGIT PATCH] Simplify the PackWriter.preparePack() API
@ 2009-03-26 19:42 Shawn O. Pearce
  0 siblings, 0 replies; only message in thread
From: Shawn O. Pearce @ 2009-03-26 19:42 UTC (permalink / raw
  To: Robin Rosenberg; +Cc: git, Shawn O. Pearce

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 <sop@google.com>
---
 .../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 f7139fc..5ebca09 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
@@ -442,8 +442,9 @@ private void createVerifyOpenPack(final Collection<ObjectId> interestings,
 			final Collection<ObjectId> 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 601ce71..83506bd 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/PackWriter.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/PackWriter.java
@@ -78,7 +78,7 @@
  * 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)}.
  * </p>
  * <p>
@@ -99,7 +99,7 @@
 	 * 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";
 
@@ -196,18 +196,20 @@
 
 	private boolean thin;
 
+	private boolean ignoreMissingUninteresting = true;
+	
 	/**
 	 * Create writer for specified repository.
 	 * <p>
 	 * 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) {
@@ -218,15 +220,14 @@ public PackWriter(final Repository repo, final ProgressMonitor monitor) {
 	 * Create writer for specified repository.
 	 * <p>
 	 * 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)}.
@@ -259,7 +260,7 @@ public boolean isReuseDeltas() {
 	 * 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.
 	 * <p>
@@ -367,6 +368,45 @@ public void setMaxDeltaDepth(int maxDeltaDepth) {
 		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.
 	 *
@@ -447,28 +487,15 @@ public void preparePack(final Iterator<RevObject> objectsSource)
 	 * @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<? extends ObjectId> interestingObjects,
-			final Collection<? extends ObjectId> uninterestingObjects,
-			final boolean packthin, final boolean ignoreMissingUninteresting)
+			final Collection<? extends ObjectId> uninterestingObjects)
 			throws IOException {
-		this.thin = packthin;
 		ObjectWalk walker = setUpWalker(interestingObjects,
-				uninterestingObjects, ignoreMissingUninteresting);
+				uninterestingObjects);
 		findObjectsToPack(walker);
 	}
 
@@ -502,7 +529,7 @@ public ObjectId computeName() {
 	 * Create an index file to match the pack file just written.
 	 * <p>
 	 * 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.
 	 *
@@ -760,8 +787,7 @@ private void writeChecksum() throws IOException {
 
 	private ObjectWalk setUpWalker(
 			final Collection<? extends ObjectId> interestingObjects,
-			final Collection<? extends ObjectId> uninterestingObjects,
-			final boolean ignoreMissingUninteresting)
+			final Collection<? extends ObjectId> 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 a078d7e..dde1d26 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 @@ private void writePack(final Map<String, RemoteRefUpdate> refUpdates,
 				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 a22a31d..8000837 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 void writeBundle(OutputStream os) throws IOException {
 		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 204859c..45d57b3 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 @@ private void sendPack() throws IOException {
 		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 3246ee6..acdb5b8 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 @@ private void sendpack(final List<RemoteRefUpdate> updates,
 				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
-- 
1.6.2.1.471.g682837

^ permalink raw reply related	[flat|nested] only message in thread

only message in thread, other threads:[~2009-03-26 19:44 UTC | newest]

Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-03-26 19:42 [JGIT PATCH] Simplify the PackWriter.preparePack() API Shawn O. Pearce

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).