Add new constructors to PackFile to improve a common use case where callers know the directory, id, and extension, but previously needed to construct a valid file name (with prefix, '.', etc) to create a PackFile. Most callers can use the variant that has id as an ObjectId, but provide an id as String variant too. Change-Id: I39e4466abe8c9509f5916d5bfe675066570b8585 Signed-off-by: Nasser Grainawi <quic_nasserg@quicinc.com>tags/v5.11.0.202103091610-r
@@ -915,9 +915,8 @@ public class TestRepository<R extends Repository> implements AutoCloseable { | |||
all.add(r.getObjectId()); | |||
pw.preparePack(m, all, PackWriter.NONE); | |||
final ObjectId name = pw.computeName(); | |||
pack = nameFor(odb, name, ".pack"); | |||
pack = new PackFile(odb.getPackDirectory(), pw.computeName(), | |||
PackExt.PACK); | |||
try (OutputStream out = | |||
new BufferedOutputStream(new FileOutputStream(pack))) { | |||
pw.writePack(m, m, out); | |||
@@ -962,12 +961,6 @@ public class TestRepository<R extends Repository> implements AutoCloseable { | |||
} | |||
} | |||
private static PackFile nameFor(ObjectDirectory odb, ObjectId name, | |||
String t) { | |||
File packdir = odb.getPackDirectory(); | |||
return new PackFile(packdir, "pack-" + name.name() + t); | |||
} | |||
private void writeFile(File p, byte[] bin) throws IOException, | |||
ObjectWritingException { | |||
final LockFile lck = new LockFile(p); |
@@ -28,6 +28,7 @@ import java.util.Collection; | |||
import java.util.List; | |||
import org.eclipse.jgit.errors.AmbiguousObjectException; | |||
import org.eclipse.jgit.internal.storage.pack.PackExt; | |||
import org.eclipse.jgit.junit.LocalDiskRepositoryTestCase; | |||
import org.eclipse.jgit.junit.TestRepository; | |||
import org.eclipse.jgit.lib.AbbreviatedObjectId; | |||
@@ -144,10 +145,9 @@ public class AbbreviationTest extends LocalDiskRepositoryTestCase { | |||
objects.add(new PackedObjectInfo(ObjectId.fromRaw(idBuf))); | |||
} | |||
String packName = "pack-" + id.name(); | |||
File packDir = db.getObjectDatabase().getPackDirectory(); | |||
File idxFile = new File(packDir, packName + ".idx"); | |||
File packFile = new File(packDir, packName + ".pack"); | |||
PackFile idxFile = new PackFile(packDir, id, PackExt.INDEX); | |||
PackFile packFile = idxFile.create(PackExt.PACK); | |||
FileUtils.mkdir(packDir, true); | |||
try (OutputStream dst = new BufferedOutputStream( | |||
new FileOutputStream(idxFile))) { |
@@ -194,9 +194,10 @@ public class ConcurrentRepackTest extends RepositoryTestCase { | |||
pw.addObject(o); | |||
} | |||
final ObjectId name = pw.computeName(); | |||
final PackFile packFile = fullPackFileName(name, ".pack"); | |||
final PackFile idxFile = packFile.create(PackExt.INDEX); | |||
PackFile packFile = new PackFile( | |||
db.getObjectDatabase().getPackDirectory(), pw.computeName(), | |||
PackExt.PACK); | |||
PackFile idxFile = packFile.create(PackExt.INDEX); | |||
final File[] files = new File[] { packFile, idxFile }; | |||
write(files, pw); | |||
return files; | |||
@@ -243,11 +244,6 @@ public class ConcurrentRepackTest extends RepositoryTestCase { | |||
} | |||
} | |||
private PackFile fullPackFileName(ObjectId name, String suffix) { | |||
final File packdir = db.getObjectDatabase().getPackDirectory(); | |||
return new PackFile(packdir, "pack-" + name.name() + suffix); | |||
} | |||
private RevObject writeBlob(Repository repo, String data) | |||
throws IOException { | |||
final byte[] bytes = Constants.encode(data); |
@@ -17,10 +17,14 @@ import static org.junit.Assert.assertTrue; | |||
import java.io.File; | |||
import org.eclipse.jgit.internal.storage.pack.PackExt; | |||
import org.eclipse.jgit.lib.ObjectId; | |||
import org.junit.Test; | |||
public class PackFileTest { | |||
private static final String TEST_ID = "0123456789012345678901234567890123456789"; | |||
private static final ObjectId TEST_OID = ObjectId | |||
.fromString("0123456789012345678901234567890123456789"); | |||
private static final String TEST_ID = TEST_OID.name(); | |||
private static final String PREFIX = "pack-"; | |||
@@ -39,12 +43,20 @@ public class PackFileTest { | |||
new File(TEST_PACK_DIR, PREFIX + TEST_ID)); | |||
@Test | |||
public void idIsSameFromFileOrDirAndName() throws Exception { | |||
File pack = new File(TEST_PACK_DIR, PREFIX + TEST_ID); | |||
public void objectsAreSameFromAnyConstructor() throws Exception { | |||
String name = PREFIX + TEST_ID + "." + PackExt.PACK.getExtension(); | |||
File pack = new File(TEST_PACK_DIR, name); | |||
PackFile pf = new PackFile(pack); | |||
PackFile pfFromDirAndName = new PackFile(TEST_PACK_DIR, | |||
PREFIX + TEST_ID); | |||
assertEquals(pf.getId(), pfFromDirAndName.getId()); | |||
PackFile pfFromDirAndName = new PackFile(TEST_PACK_DIR, name); | |||
assertPackFilesEqual(pf, pfFromDirAndName); | |||
PackFile pfFromOIdAndExt = new PackFile(TEST_PACK_DIR, TEST_OID, | |||
PackExt.PACK); | |||
assertPackFilesEqual(pf, pfFromOIdAndExt); | |||
PackFile pfFromIdAndExt = new PackFile(TEST_PACK_DIR, TEST_ID, | |||
PackExt.PACK); | |||
assertPackFilesEqual(pf, pfFromIdAndExt); | |||
} | |||
@Test | |||
@@ -147,4 +159,11 @@ public class PackFileTest { | |||
preservedWithExt.getPackExt()); | |||
} | |||
} | |||
private void assertPackFilesEqual(PackFile p1, PackFile p2) { | |||
// for test purposes, considered equal if id, name, and ext are equal | |||
assertEquals(p1.getId(), p2.getId()); | |||
assertEquals(p1.getPackExt(), p2.getPackExt()); | |||
assertEquals(p1.getName(), p2.getName()); | |||
} | |||
} |
@@ -684,9 +684,9 @@ public class PackWriterTest extends SampleDataRepositoryTestCase { | |||
ObjectWalk ow = walk.toObjectWalkWithSameObjects(); | |||
pw.preparePack(NullProgressMonitor.INSTANCE, ow, want, have, NONE); | |||
String id = pw.computeName().getName(); | |||
File packdir = repo.getObjectDatabase().getPackDirectory(); | |||
PackFile packFile = new PackFile(packdir, "pack-" + id + ".pack"); | |||
PackFile packFile = new PackFile(packdir, pw.computeName(), | |||
PackExt.PACK); | |||
try (FileOutputStream packOS = new FileOutputStream(packFile)) { | |||
pw.writePack(NullProgressMonitor.INSTANCE, | |||
NullProgressMonitor.INSTANCE, packOS); |
@@ -1163,7 +1163,7 @@ public class GC { | |||
checkCancelled(); | |||
// create temporary files | |||
String id = pw.computeName().getName(); | |||
ObjectId id = pw.computeName(); | |||
File packdir = repo.getObjectDatabase().getPackDirectory(); | |||
packdir.mkdirs(); | |||
tmpPack = File.createTempFile("gc_", ".pack_tmp", packdir); //$NON-NLS-1$ //$NON-NLS-2$ | |||
@@ -1213,7 +1213,8 @@ public class GC { | |||
} | |||
// rename the temporary files to real files | |||
File realPack = nameFor(id, PackExt.PACK); | |||
File packDir = repo.getObjectDatabase().getPackDirectory(); | |||
PackFile realPack = new PackFile(packDir, id, PackExt.PACK); | |||
repo.getObjectDatabase().closeAllPackHandles(realPack); | |||
tmpPack.setReadOnly(); | |||
@@ -1223,7 +1224,7 @@ public class GC { | |||
File tmpExt = tmpEntry.getValue(); | |||
tmpExt.setReadOnly(); | |||
File realExt = nameFor(id, tmpEntry.getKey()); | |||
PackFile realExt = new PackFile(packDir, id, tmpEntry.getKey()); | |||
try { | |||
FileUtils.rename(tmpExt, realExt, | |||
StandardCopyOption.ATOMIC_MOVE); | |||
@@ -1269,11 +1270,6 @@ public class GC { | |||
} | |||
} | |||
private PackFile nameFor(String name, PackExt ext) { | |||
return new PackFile(repo.getObjectDatabase().getPackDirectory(), | |||
"pack-" + name).create(ext); //$NON-NLS-1$ | |||
} | |||
private void checkCancelled() throws CancelledException { | |||
if (pm.isCancelled() || Thread.currentThread().isInterrupted()) { | |||
throw new CancelledException(JGitText.get().operationCanceled); |
@@ -17,6 +17,7 @@ import java.util.List; | |||
import org.eclipse.jgit.internal.storage.pack.CachedPack; | |||
import org.eclipse.jgit.internal.storage.pack.ObjectToPack; | |||
import org.eclipse.jgit.internal.storage.pack.PackExt; | |||
import org.eclipse.jgit.internal.storage.pack.PackOutputStream; | |||
import org.eclipse.jgit.internal.storage.pack.StoredObjectRepresentation; | |||
@@ -88,6 +89,6 @@ class LocalCachedPack extends CachedPack { | |||
private String getPackFilePath(String packName) { | |||
final File packDir = odb.getPackDirectory(); | |||
return new File(packDir, "pack-" + packName + ".pack").getPath(); //$NON-NLS-1$ //$NON-NLS-2$ | |||
return new PackFile(packDir, packName, PackExt.PACK).getPath(); | |||
} | |||
} |
@@ -27,6 +27,7 @@ import java.util.zip.Deflater; | |||
import org.eclipse.jgit.errors.LockFailedException; | |||
import org.eclipse.jgit.internal.JGitText; | |||
import org.eclipse.jgit.internal.storage.pack.PackExt; | |||
import org.eclipse.jgit.lib.AnyObjectId; | |||
import org.eclipse.jgit.lib.Constants; | |||
import org.eclipse.jgit.lib.CoreConfig; | |||
@@ -426,10 +427,10 @@ public class ObjectDirectoryPackParser extends PackParser { | |||
d.update(oeBytes); | |||
} | |||
final String name = ObjectId.fromRaw(d.digest()).name(); | |||
final File packDir = new File(db.getDirectory(), "pack"); //$NON-NLS-1$ | |||
final File finalPack = new File(packDir, "pack-" + name + ".pack"); //$NON-NLS-1$ //$NON-NLS-2$ | |||
final File finalIdx = new File(packDir, "pack-" + name + ".idx"); //$NON-NLS-1$ //$NON-NLS-2$ | |||
ObjectId id = ObjectId.fromRaw(d.digest()); | |||
File packDir = new File(db.getDirectory(), "pack"); //$NON-NLS-1$ | |||
PackFile finalPack = new PackFile(packDir, id, PackExt.PACK); | |||
PackFile finalIdx = finalPack.create(PackExt.INDEX); | |||
final PackLock keep = new PackLock(finalPack, db.getFS()); | |||
if (!packDir.exists() && !packDir.mkdir() && !packDir.exists()) { |
@@ -16,6 +16,7 @@ import java.text.MessageFormat; | |||
import org.eclipse.jgit.internal.JGitText; | |||
import org.eclipse.jgit.internal.storage.pack.PackExt; | |||
import org.eclipse.jgit.lib.ObjectId; | |||
/** | |||
* A pack file (or pack related) File. | |||
@@ -36,6 +37,10 @@ public class PackFile extends File { | |||
private final PackExt packExt; | |||
private static String createName(String id, PackExt extension) { | |||
return PREFIX + id + '.' + extension.getExtension(); | |||
} | |||
/** | |||
* Create a PackFile for a pack or related file. | |||
* | |||
@@ -46,6 +51,34 @@ public class PackFile extends File { | |||
this(file.getParentFile(), file.getName()); | |||
} | |||
/** | |||
* Create a PackFile for a pack or related file. | |||
* | |||
* @param directory | |||
* Directory to create the PackFile in. | |||
* @param id | |||
* the {@link ObjectId} for this pack | |||
* @param ext | |||
* the <code>packExt</code> of the name. | |||
*/ | |||
public PackFile(File directory, ObjectId id, PackExt ext) { | |||
this(directory, id.name(), ext); | |||
} | |||
/** | |||
* Create a PackFile for a pack or related file. | |||
* | |||
* @param directory | |||
* Directory to create the PackFile in. | |||
* @param id | |||
* the <code>id</code> (40 Hex char) section of the pack name. | |||
* @param ext | |||
* the <code>packExt</code> of the name. | |||
*/ | |||
public PackFile(File directory, String id, PackExt ext) { | |||
this(directory, createName(id, ext)); | |||
} | |||
/** | |||
* Create a PackFile for a pack or related file. | |||
* |
@@ -76,6 +76,7 @@ import org.eclipse.jgit.errors.IncorrectObjectTypeException; | |||
import org.eclipse.jgit.errors.LargeObjectException; | |||
import org.eclipse.jgit.errors.MissingObjectException; | |||
import org.eclipse.jgit.internal.JGitText; | |||
import org.eclipse.jgit.internal.storage.pack.PackExt; | |||
import org.eclipse.jgit.lib.AbbreviatedObjectId; | |||
import org.eclipse.jgit.lib.AnyObjectId; | |||
import org.eclipse.jgit.lib.Constants; | |||
@@ -273,16 +274,16 @@ public class PackInserter extends ObjectInserter { | |||
} | |||
Collections.sort(objectList); | |||
File tmpIdx = idxFor(tmpPack); | |||
File tmpIdx = idxFor(tmpPack); // TODO(nasserg) Use PackFile? | |||
writePackIndex(tmpIdx, packHash, objectList); | |||
File realPack = new File(db.getPackDirectory(), | |||
"pack-" + computeName(objectList).name() + ".pack"); //$NON-NLS-1$ //$NON-NLS-2$ | |||
PackFile realPack = new PackFile(db.getPackDirectory(), | |||
computeName(objectList), PackExt.PACK); | |||
db.closeAllPackHandles(realPack); | |||
tmpPack.setReadOnly(); | |||
FileUtils.rename(tmpPack, realPack, ATOMIC_MOVE); | |||
File realIdx = idxFor(realPack); | |||
PackFile realIdx = realPack.create(PackExt.INDEX); | |||
tmpIdx.setReadOnly(); | |||
try { | |||
FileUtils.rename(tmpIdx, realIdx, ATOMIC_MOVE); |
@@ -13,6 +13,7 @@ package org.eclipse.jgit.transport; | |||
import static org.eclipse.jgit.transport.WalkRemoteObjectDatabase.ROOT_DIR; | |||
import java.io.BufferedOutputStream; | |||
import java.io.File; | |||
import java.io.IOException; | |||
import java.io.OutputStream; | |||
import java.util.ArrayList; | |||
@@ -26,6 +27,8 @@ import java.util.TreeMap; | |||
import org.eclipse.jgit.errors.TransportException; | |||
import org.eclipse.jgit.internal.JGitText; | |||
import org.eclipse.jgit.internal.storage.file.PackFile; | |||
import org.eclipse.jgit.internal.storage.pack.PackExt; | |||
import org.eclipse.jgit.internal.storage.pack.PackWriter; | |||
import org.eclipse.jgit.lib.AnyObjectId; | |||
import org.eclipse.jgit.lib.Constants; | |||
@@ -189,9 +192,8 @@ class WalkPushConnection extends BaseConnection implements PushConnection { | |||
private void sendpack(final List<RemoteRefUpdate> updates, | |||
final ProgressMonitor monitor) throws TransportException { | |||
String pathPack = null; | |||
String pathIdx = null; | |||
PackFile pack = null; | |||
PackFile idx = null; | |||
try (PackWriter writer = new PackWriter(transport.getPackConfig(), | |||
local.newObjectReader())) { | |||
@@ -217,31 +219,33 @@ class WalkPushConnection extends BaseConnection implements PushConnection { | |||
for (String n : dest.getPackNames()) | |||
packNames.put(n, n); | |||
final String base = "pack-" + writer.computeName().name(); //$NON-NLS-1$ | |||
final String packName = base + ".pack"; //$NON-NLS-1$ | |||
pathPack = "pack/" + packName; //$NON-NLS-1$ | |||
pathIdx = "pack/" + base + ".idx"; //$NON-NLS-1$ //$NON-NLS-2$ | |||
File packDir = new File("pack"); //$NON-NLS-1$ | |||
pack = new PackFile(packDir, writer.computeName(), | |||
PackExt.PACK); | |||
idx = pack.create(PackExt.INDEX); | |||
if (packNames.remove(packName) != null) { | |||
if (packNames.remove(pack.getName()) != null) { | |||
// The remote already contains this pack. We should | |||
// remove the index before overwriting to prevent bad | |||
// offsets from appearing to clients. | |||
// | |||
dest.writeInfoPacks(packNames.keySet()); | |||
dest.deleteFile(pathIdx); | |||
dest.deleteFile(idx.getPath()); | |||
} | |||
// Write the pack file, then the index, as readers look the | |||
// other direction (index, then pack file). | |||
// | |||
String wt = "Put " + base.substring(0, 12); //$NON-NLS-1$ | |||
String wt = "Put " + pack.getName().substring(0, 12); //$NON-NLS-1$ | |||
try (OutputStream os = new BufferedOutputStream( | |||
dest.writeFile(pathPack, monitor, wt + "..pack"))) { //$NON-NLS-1$ | |||
dest.writeFile(pack.getPath(), monitor, | |||
wt + "." + pack.getPackExt().getExtension()))) { //$NON-NLS-1$ | |||
writer.writePack(monitor, monitor, os); | |||
} | |||
try (OutputStream os = new BufferedOutputStream( | |||
dest.writeFile(pathIdx, monitor, wt + "..idx"))) { //$NON-NLS-1$ | |||
dest.writeFile(idx.getPath(), monitor, | |||
wt + "." + idx.getPackExt().getExtension()))) { //$NON-NLS-1$ | |||
writer.writeIndex(os); | |||
} | |||
@@ -250,22 +254,22 @@ class WalkPushConnection extends BaseConnection implements PushConnection { | |||
// and discover the most recent objects there. | |||
// | |||
final ArrayList<String> infoPacks = new ArrayList<>(); | |||
infoPacks.add(packName); | |||
infoPacks.add(pack.getName()); | |||
infoPacks.addAll(packNames.keySet()); | |||
dest.writeInfoPacks(infoPacks); | |||
} catch (IOException err) { | |||
safeDelete(pathIdx); | |||
safeDelete(pathPack); | |||
safeDelete(idx); | |||
safeDelete(pack); | |||
throw new TransportException(uri, JGitText.get().cannotStoreObjects, err); | |||
} | |||
} | |||
private void safeDelete(String path) { | |||
private void safeDelete(File path) { | |||
if (path != null) { | |||
try { | |||
dest.deleteFile(path); | |||
dest.deleteFile(path.getPath()); | |||
} catch (IOException cleanupFailure) { | |||
// Ignore the deletion failure. We probably are | |||
// already failing and were just trying to pick |