PackWriter: Declare preparePack object sets as @NonNull

Require callers to pass in valid sets for both want and have
collections. Offer PackWriter.NONE as a handy constant for an
empty collection for the have part of preparePack instead of null.

Change-Id: Ifda4450f5e488cbfefd728382b7d30797e229217
This commit is contained in:
Shawn Pearce 2016-01-12 16:11:36 -08:00
parent 40051505d7
commit 4e650c0d76
5 changed files with 38 additions and 42 deletions

View File

@ -866,7 +866,7 @@ public void packAndPrune() throws Exception {
Set<ObjectId> all = new HashSet<ObjectId>();
for (Ref r : db.getAllRefs().values())
all.add(r.getObjectId());
pw.preparePack(m, all, Collections.<ObjectId> emptySet());
pw.preparePack(m, all, PackWriter.NONE);
final ObjectId name = pw.computeName();

View File

@ -49,6 +49,7 @@
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.eclipse.jgit.internal.storage.pack.PackWriter.NONE;
import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
@ -88,9 +89,6 @@
public class PackWriterTest extends SampleDataRepositoryTestCase {
private static final Set<ObjectId> EMPTY_SET_OBJECT = Collections
.<ObjectId> emptySet();
private static final List<RevObject> EMPTY_LIST_REVS = Collections
.<RevObject> emptyList();
@ -171,7 +169,7 @@ public void testModifySettings() {
*/
@Test
public void testWriteEmptyPack1() throws IOException {
createVerifyOpenPack(EMPTY_SET_OBJECT, EMPTY_SET_OBJECT, false, false);
createVerifyOpenPack(NONE, NONE, false, false);
assertEquals(0, writer.getObjectCount());
assertEquals(0, pack.getObjectCount());
@ -204,8 +202,8 @@ public void testNotIgnoreNonExistingObjects() throws IOException {
final ObjectId nonExisting = ObjectId
.fromString("0000000000000000000000000000000000000001");
try {
createVerifyOpenPack(EMPTY_SET_OBJECT, Collections.singleton(
nonExisting), false, false);
createVerifyOpenPack(NONE, Collections.singleton(nonExisting),
false, false);
fail("Should have thrown MissingObjectException");
} catch (MissingObjectException x) {
// expected
@ -221,8 +219,8 @@ public void testNotIgnoreNonExistingObjects() throws IOException {
public void testIgnoreNonExistingObjects() throws IOException {
final ObjectId nonExisting = ObjectId
.fromString("0000000000000000000000000000000000000001");
createVerifyOpenPack(EMPTY_SET_OBJECT, Collections.singleton(
nonExisting), false, true);
createVerifyOpenPack(NONE, Collections.singleton(nonExisting),
false, true);
// shouldn't throw anything
}
@ -240,8 +238,8 @@ public void testIgnoreNonExistingObjectsWithBitmaps() throws IOException,
final ObjectId nonExisting = ObjectId
.fromString("0000000000000000000000000000000000000001");
new GC(db).gc();
createVerifyOpenPack(EMPTY_SET_OBJECT,
Collections.singleton(nonExisting), false, true, true);
createVerifyOpenPack(NONE, Collections.singleton(nonExisting), false,
true, true);
// shouldn't throw anything
}
@ -552,8 +550,7 @@ private static PackIndex writePack(FileRepository repo,
pw.setReuseDeltaCommits(false);
for (ObjectIdSet idx : excludeObjects)
pw.excludeObjects(idx);
pw.preparePack(NullProgressMonitor.INSTANCE, want,
Collections.<ObjectId> emptySet());
pw.preparePack(NullProgressMonitor.INSTANCE, want, NONE);
String id = pw.computeName().getName();
File packdir = new File(repo.getObjectsDirectory(), "pack");
File packFile = new File(packdir, "pack-" + id + ".pack");
@ -576,7 +573,7 @@ private void writeVerifyPack1() throws IOException {
final HashSet<ObjectId> interestings = new HashSet<ObjectId>();
interestings.add(ObjectId
.fromString("82c6b885ff600be425b4ea96dee75dca255b69e7"));
createVerifyOpenPack(interestings, EMPTY_SET_OBJECT, false, false);
createVerifyOpenPack(interestings, NONE, false, false);
final ObjectId expectedOrder[] = new ObjectId[] {
ObjectId.fromString("82c6b885ff600be425b4ea96dee75dca255b69e7"),

View File

@ -53,7 +53,6 @@
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
@ -278,7 +277,7 @@ private void packHeads(ProgressMonitor pm) throws IOException {
try (PackWriter pw = newPackWriter()) {
pw.setTagTargets(tagTargets);
pw.preparePack(pm, allHeads, none());
pw.preparePack(pm, allHeads, PackWriter.NONE);
if (0 < pw.getObjectCount())
writePack(GC, pw, pm);
}
@ -303,16 +302,12 @@ private void packRefTreeGraph(ProgressMonitor pm) throws IOException {
try (PackWriter pw = newPackWriter()) {
for (ObjectIdSet packedObjs : newPackObj)
pw.excludeObjects(packedObjs);
pw.preparePack(pm, txnHeads, none());
pw.preparePack(pm, txnHeads, PackWriter.NONE);
if (0 < pw.getObjectCount())
writePack(GC_TXN, pw, pm);
}
}
private static Set<ObjectId> none() {
return Collections.<ObjectId> emptySet();
}
private void packGarbage(ProgressMonitor pm) throws IOException {
// TODO(sop) This is ugly. The garbage pack needs to be deleted.
PackConfig cfg = new PackConfig(packConfig);

View File

@ -69,6 +69,7 @@
import java.util.Set;
import java.util.TreeMap;
import org.eclipse.jgit.annotations.NonNull;
import org.eclipse.jgit.dircache.DirCacheIterator;
import org.eclipse.jgit.errors.CorruptObjectException;
import org.eclipse.jgit.errors.IncorrectObjectTypeException;
@ -578,7 +579,7 @@ else if (RefTreeNames.isRefTree(refdb, ref.getName()))
ret.add(rest);
}
if (!txnHeads.isEmpty()) {
PackFile txn = writePack(txnHeads, null, null, excluded);
PackFile txn = writePack(txnHeads, PackWriter.NONE, null, excluded);
if (txn != null)
ret.add(txn);
}
@ -698,8 +699,8 @@ private Set<ObjectId> listNonHEADIndexObjects()
}
}
private PackFile writePack(Set<? extends ObjectId> want,
Set<? extends ObjectId> have, Set<ObjectId> tagTargets,
private PackFile writePack(@NonNull Set<? extends ObjectId> want,
@NonNull Set<? extends ObjectId> have, Set<ObjectId> tagTargets,
List<ObjectIdSet> excludeObjects) throws IOException {
File tmpPack = null;
Map<PackExt, File> tmpExts = new TreeMap<PackExt, File>(

View File

@ -80,6 +80,7 @@
import java.util.zip.Deflater;
import java.util.zip.DeflaterOutputStream;
import org.eclipse.jgit.annotations.NonNull;
import org.eclipse.jgit.errors.CorruptObjectException;
import org.eclipse.jgit.errors.IncorrectObjectTypeException;
import org.eclipse.jgit.errors.LargeObjectException;
@ -162,6 +163,9 @@
public class PackWriter implements AutoCloseable {
private static final int PACK_VERSION_GENERATED = 2;
/** Empty set of objects for {@code preparePack()}. */
public static Set<ObjectId> NONE = Collections.emptySet();
private static final Map<WeakReference<PackWriter>, Boolean> instances =
new ConcurrentHashMap<WeakReference<PackWriter>, Boolean>();
@ -670,7 +674,7 @@ public void excludeObjects(ObjectIdSet idx) {
* @throws IOException
* when some I/O problem occur during reading objects.
*/
public void preparePack(final Iterator<RevObject> objectsSource)
public void preparePack(@NonNull Iterator<RevObject> objectsSource)
throws IOException {
while (objectsSource.hasNext()) {
addObject(objectsSource.next());
@ -693,16 +697,18 @@ public void preparePack(final Iterator<RevObject> objectsSource)
* progress during object enumeration.
* @param want
* collection of objects to be marked as interesting (start
* points of graph traversal).
* points of graph traversal). Must not be {@code null}.
* @param have
* collection of objects to be marked as uninteresting (end
* points of graph traversal).
* points of graph traversal). Pass {@link #NONE} if all objects
* reachable from {@code want} are desired, such as when serving
* a clone.
* @throws IOException
* when some I/O problem occur during reading objects.
*/
public void preparePack(ProgressMonitor countingMonitor,
Set<? extends ObjectId> want,
Set<? extends ObjectId> have) throws IOException {
@NonNull Set<? extends ObjectId> want,
@NonNull Set<? extends ObjectId> have) throws IOException {
ObjectWalk ow;
if (shallowPack)
ow = new DepthWalk.ObjectWalk(reader, depth);
@ -729,17 +735,19 @@ public void preparePack(ProgressMonitor countingMonitor,
* ObjectWalk to perform enumeration.
* @param interestingObjects
* collection of objects to be marked as interesting (start
* points of graph traversal).
* points of graph traversal). Must not be {@code null}.
* @param uninterestingObjects
* collection of objects to be marked as uninteresting (end
* points of graph traversal).
* points of graph traversal). Pass {@link #NONE} if all objects
* reachable from {@code want} are desired, such as when serving
* a clone.
* @throws IOException
* when some I/O problem occur during reading objects.
*/
public void preparePack(ProgressMonitor countingMonitor,
ObjectWalk walk,
final Set<? extends ObjectId> interestingObjects,
final Set<? extends ObjectId> uninterestingObjects)
@NonNull ObjectWalk walk,
@NonNull Set<? extends ObjectId> interestingObjects,
@NonNull Set<? extends ObjectId> uninterestingObjects)
throws IOException {
if (countingMonitor == null)
countingMonitor = NullProgressMonitor.INSTANCE;
@ -1597,17 +1605,12 @@ private void writeChecksum(PackOutputStream out) throws IOException {
out.write(packcsum);
}
private void findObjectsToPack(final ProgressMonitor countingMonitor,
final ObjectWalk walker, final Set<? extends ObjectId> want,
Set<? extends ObjectId> have)
throws MissingObjectException, IOException,
IncorrectObjectTypeException {
private void findObjectsToPack(@NonNull ProgressMonitor countingMonitor,
@NonNull ObjectWalk walker, @NonNull Set<? extends ObjectId> want,
@NonNull Set<? extends ObjectId> have) throws IOException {
final long countingStart = System.currentTimeMillis();
beginPhase(PackingPhase.COUNTING, countingMonitor, ProgressMonitor.UNKNOWN);
if (have == null)
have = Collections.emptySet();
stats.interestingObjects = Collections.unmodifiableSet(new HashSet<ObjectId>(want));
stats.uninterestingObjects = Collections.unmodifiableSet(new HashSet<ObjectId>(have));