diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.http.apache.feature/feature.xml b/org.eclipse.jgit.packaging/org.eclipse.jgit.http.apache.feature/feature.xml index 17a732b4a..8da600137 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.http.apache.feature/feature.xml +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.http.apache.feature/feature.xml @@ -47,4 +47,11 @@ version="0.0.0" unpack="false"/> + + diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CloneCommandTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CloneCommandTest.java index cee898ab8..3d0dacab3 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CloneCommandTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CloneCommandTest.java @@ -408,6 +408,7 @@ public void testBareCloneRepositoryMirror() throws Exception { Git git2 = command.call(); addRepoToClose(git2.getRepository()); assertNotNull(git2); + assertTrue(git2.getRepository().isBare()); assertNotNull(git2.getRepository().resolve("tag-for-blob")); assertNotNull(git2.getRepository().resolve("tag-initial")); assertEquals(git2.getRepository().getFullBranch(), "refs/heads/master"); @@ -448,6 +449,60 @@ public void testCloneRepositoryOnlyOneTag() throws Exception { specs.get(0)); } + @Test + public void testCloneRepositoryAllBranchesTakesPreference() + throws Exception { + File directory = createTempDirectory( + "testCloneRepositoryAllBranchesTakesPreference"); + CloneCommand command = Git.cloneRepository(); + command.setCloneAllBranches(true); + command.setBranchesToClone( + Collections.singletonList("refs/heads/test")); + command.setDirectory(directory); + command.setURI(fileUri()); + Git git2 = command.call(); + addRepoToClose(git2.getRepository()); + assertNotNull(git2); + assertEquals(git2.getRepository().getFullBranch(), "refs/heads/test"); + // Expect both remote branches to exist; setCloneAllBranches(true) + // should override any setBranchesToClone(). + assertNotNull( + git2.getRepository().resolve("refs/remotes/origin/master")); + assertNotNull(git2.getRepository().resolve("refs/remotes/origin/test")); + RemoteConfig cfg = new RemoteConfig(git2.getRepository().getConfig(), + Constants.DEFAULT_REMOTE_NAME); + List specs = cfg.getFetchRefSpecs(); + assertEquals(1, specs.size()); + assertEquals(new RefSpec("+refs/heads/*:refs/remotes/origin/*"), + specs.get(0)); + } + + @Test + public void testCloneRepositoryAllBranchesIndependent() throws Exception { + File directory = createTempDirectory( + "testCloneRepositoryAllBranchesIndependent"); + CloneCommand command = Git.cloneRepository(); + command.setCloneAllBranches(true); + command.setBranchesToClone( + Collections.singletonList("refs/heads/test")); + command.setCloneAllBranches(false); + command.setDirectory(directory); + command.setURI(fileUri()); + Git git2 = command.call(); + addRepoToClose(git2.getRepository()); + assertNotNull(git2); + assertEquals(git2.getRepository().getFullBranch(), "refs/heads/test"); + // Expect only the test branch; allBranches was re-set to false + assertNull(git2.getRepository().resolve("refs/remotes/origin/master")); + assertNotNull(git2.getRepository().resolve("refs/remotes/origin/test")); + RemoteConfig cfg = new RemoteConfig(git2.getRepository().getConfig(), + Constants.DEFAULT_REMOTE_NAME); + List specs = cfg.getFetchRefSpecs(); + assertEquals(1, specs.size()); + assertEquals(new RefSpec("+refs/heads/test:refs/remotes/origin/test"), + specs.get(0)); + } + public static String allRefNames(List refs) { StringBuilder sb = new StringBuilder(); for (Ref f : refs) { diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/FileReftableTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/FileReftableTest.java index 9da35075d..b13cdb9fe 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/FileReftableTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/FileReftableTest.java @@ -26,6 +26,7 @@ import java.io.File; import java.io.IOException; +import java.security.SecureRandom; import java.util.ArrayList; import java.util.List; @@ -509,6 +510,43 @@ public void testRenameAtomic() throws IOException { assertEquals(RefUpdate.Result.LOCK_FAILURE, rename.rename()); } + @Test + public void compactFully() throws Exception { + FileReftableDatabase refDb = (FileReftableDatabase) db.getRefDatabase(); + PersonIdent person = new PersonIdent("jane", "jane@invalid"); + + ObjectId aId = db.exactRef("refs/heads/a").getObjectId(); + ObjectId bId = db.exactRef("refs/heads/b").getObjectId(); + + SecureRandom random = new SecureRandom(); + List strs = new ArrayList<>(); + for (int i = 0; i < 1024; i++) { + strs.add(String.format("%02x", + Integer.valueOf(random.nextInt(256)))); + } + + String randomStr = String.join("", strs); + String refName = "branch"; + for (long i = 0; i < 2; i++) { + RefUpdate ru = refDb.newUpdate(refName, false); + ru.setNewObjectId(i % 2 == 0 ? aId : bId); + ru.setForceUpdate(true); + // Only write a large string in the first table, so it becomes much larger + // than the second, and the result is not autocompacted. + ru.setRefLogMessage(i == 0 ? randomStr : "short", false); + ru.setRefLogIdent(person); + + RefUpdate.Result res = ru.update(); + assertTrue(res == Result.NEW || res == FORCED); + } + + assertEquals(refDb.exactRef(refName).getObjectId(), bId); + assertTrue(randomStr.equals(refDb.getReflogReader(refName).getReverseEntry(1).getComment())); + refDb.compactFully(); + assertEquals(refDb.exactRef(refName).getObjectId(), bId); + assertTrue(randomStr.equals(refDb.getReflogReader(refName).getReverseEntry(1).getComment())); + } + @Test public void reftableRefsStorageClass() throws IOException { Ref b = db.exactRef("refs/heads/b"); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/CloneCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/CloneCommand.java index eef794001..78afe18f3 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/CloneCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/CloneCommand.java @@ -71,7 +71,9 @@ public class CloneCommand extends TransportCommand { private ProgressMonitor monitor = NullProgressMonitor.INSTANCE; - private FETCH_TYPE fetchType = FETCH_TYPE.ALL_BRANCHES; + private boolean cloneAllBranches; + + private boolean mirror; private boolean cloneSubmodules; @@ -85,6 +87,8 @@ public class CloneCommand extends TransportCommand { private boolean gitDirExistsInitially; + private FETCH_TYPE fetchType; + private enum FETCH_TYPE { MULTIPLE_BRANCHES, ALL_BRANCHES, MIRROR } @@ -162,6 +166,7 @@ public Git call() throws GitAPIException, InvalidRemoteException, throw new InvalidRemoteException( MessageFormat.format(JGitText.get().invalidURL, uri), e); } + setFetchType(); @SuppressWarnings("resource") // Closed by caller Repository repository = init(); FetchResult fetchResult = null; @@ -206,6 +211,20 @@ public Git call() throws GitAPIException, InvalidRemoteException, return new Git(repository, true); } + private void setFetchType() { + if (mirror) { + fetchType = FETCH_TYPE.MIRROR; + setBare(true); + } else if (cloneAllBranches) { + fetchType = FETCH_TYPE.ALL_BRANCHES; + } else if (branchesToClone != null && !branchesToClone.isEmpty()) { + fetchType = FETCH_TYPE.MULTIPLE_BRANCHES; + } else { + // Default: neither mirror nor all nor specific refs given + fetchType = FETCH_TYPE.ALL_BRANCHES; + } + } + private static boolean isNonEmptyDirectory(File dir) { if (dir != null && dir.exists()) { File[] files = dir.listFiles(); @@ -587,8 +606,7 @@ public CloneCommand setProgressMonitor(ProgressMonitor monitor) { * @return {@code this} */ public CloneCommand setCloneAllBranches(boolean cloneAllBranches) { - this.fetchType = cloneAllBranches ? FETCH_TYPE.ALL_BRANCHES - : this.fetchType; + this.cloneAllBranches = cloneAllBranches; return this; } @@ -608,10 +626,7 @@ public CloneCommand setCloneAllBranches(boolean cloneAllBranches) { * @since 5.6 */ public CloneCommand setMirror(boolean mirror) { - if (mirror) { - this.fetchType = FETCH_TYPE.MIRROR; - setBare(true); - } + this.mirror = mirror; return this; } @@ -632,8 +647,9 @@ public CloneCommand setCloneSubmodules(boolean cloneSubmodules) { * Set the branches or tags to clone. *

* This is ignored if {@link #setCloneAllBranches(boolean) - * setCloneAllBranches(true)} is used. If {@code branchesToClone} is - * {@code null} or empty, it's also ignored and all branches will be cloned. + * setCloneAllBranches(true)} or {@link #setMirror(boolean) setMirror(true)} + * is used. If {@code branchesToClone} is {@code null} or empty, it's also + * ignored. *

* * @param branchesToClone @@ -643,13 +659,7 @@ public CloneCommand setCloneSubmodules(boolean cloneSubmodules) { * @return {@code this} */ public CloneCommand setBranchesToClone(Collection branchesToClone) { - if (branchesToClone == null || branchesToClone.isEmpty()) { - // fallback to default - fetchType = FETCH_TYPE.ALL_BRANCHES; - } else { - this.fetchType = FETCH_TYPE.MULTIPLE_BRANCHES; - this.branchesToClone = branchesToClone; - } + this.branchesToClone = branchesToClone; return this; } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableDatabase.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableDatabase.java index dc9c28c1b..aea14de75 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableDatabase.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableDatabase.java @@ -106,6 +106,7 @@ public void compactFully() throws IOException { reftableDatabase.getLock().lock(); try { reftableStack.compactFully(); + reftableDatabase.clearCache(); } finally { reftableDatabase.getLock().unlock(); } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableStack.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableStack.java index 5b540bd7e..4cb0bd3e8 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableStack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableStack.java @@ -237,8 +237,13 @@ void reload() throws IOException { long max = 1000; long delay = 0; boolean success = false; - while (System.currentTimeMillis() < deadline) { + + // Don't check deadline for the first 3 retries, so we can step with a + // debugger without worrying about deadlines. + int tries = 0; + while (tries < 3 || System.currentTimeMillis() < deadline) { List names = readTableNames(); + tries++; try { reloadOnce(names); success = true; @@ -260,9 +265,6 @@ void reload() throws IOException { } if (!success) { - // TODO: should reexamine the 'refs' file to see if it was the same - // if it didn't change, then we must have corruption. If it did, - // retry. throw new LockFailedException(stackPath); } @@ -374,7 +376,7 @@ private String filename(long low, long high) { * * @param w * writer to write data to a reftable under construction - * @return true if the transaction. + * @return true if the transaction was successful. * @throws IOException * on I/O problems */ diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackFile.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackFile.java index 984c16366..b99733875 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackFile.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackFile.java @@ -71,13 +71,12 @@ */ public class PackFile implements Iterable { private final static Logger LOG = LoggerFactory.getLogger(PackFile.class); - /** Sorts PackFiles to be most recently created to least recently created. */ - public static final Comparator SORT = new Comparator() { - @Override - public int compare(PackFile a, PackFile b) { - return b.packLastModified.compareTo(a.packLastModified); - } - }; + + /** + * Sorts PackFiles to be most recently created to least recently created. + */ + public static final Comparator SORT = (a, b) -> b.packLastModified + .compareTo(a.packLastModified); private final File packFile; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java index 9a6ec760b..e8f0e1fd9 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java @@ -142,43 +142,42 @@ public class PackWriter implements AutoCloseable { private static final Map, Boolean> instances = new ConcurrentHashMap<>(); - private static final Iterable instancesIterable = new Iterable() { + private static final Iterable instancesIterable = () -> new Iterator() { + + private final Iterator> it = instances + .keySet().iterator(); + + private PackWriter next; + @Override - public Iterator iterator() { - return new Iterator() { - private final Iterator> it = - instances.keySet().iterator(); - private PackWriter next; - - @Override - public boolean hasNext() { - if (next != null) - return true; - while (it.hasNext()) { - WeakReference ref = it.next(); - next = ref.get(); - if (next != null) - return true; - it.remove(); - } - return false; + public boolean hasNext() { + if (next != null) { + return true; + } + while (it.hasNext()) { + WeakReference ref = it.next(); + next = ref.get(); + if (next != null) { + return true; } + it.remove(); + } + return false; + } - @Override - public PackWriter next() { - if (hasNext()) { - PackWriter result = next; - next = null; - return result; - } - throw new NoSuchElementException(); - } + @Override + public PackWriter next() { + if (hasNext()) { + PackWriter result = next; + next = null; + return result; + } + throw new NoSuchElementException(); + } - @Override - public void remove() { - throw new UnsupportedOperationException(); - } - }; + @Override + public void remove() { + throw new UnsupportedOperationException(); } }; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/LogCursor.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/LogCursor.java index 9d410aef9..f78975af3 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/LogCursor.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/LogCursor.java @@ -12,6 +12,7 @@ import java.io.IOException; +import org.eclipse.jgit.annotations.Nullable; import org.eclipse.jgit.lib.ReflogEntry; /** @@ -45,8 +46,9 @@ public abstract class LogCursor implements AutoCloseable { /** * Get current log entry. * - * @return current log entry. + * @return current log entry. Maybe null if we are producing deletions. */ + @Nullable public abstract ReflogEntry getReflogEntry(); /** {@inheritDoc} */ diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/AmazonS3.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/AmazonS3.java index 1e7f3b1f0..9210ec172 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/AmazonS3.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/AmazonS3.java @@ -33,6 +33,7 @@ import java.text.SimpleDateFormat; import java.util.ArrayList; import java.util.Collections; +import java.util.Comparator; import java.util.Date; import java.util.HashSet; import java.util.Iterator; @@ -44,6 +45,8 @@ import java.util.SortedMap; import java.util.TimeZone; import java.util.TreeMap; +import java.util.stream.Collectors; +import java.time.Instant; import javax.crypto.Mac; import javax.crypto.spec.SecretKeySpec; @@ -288,6 +291,8 @@ public InputStream decrypt(URLConnection u) throws IOException { *

* This method is primarily meant for obtaining a "recursive directory * listing" rooted under the specified bucket and prefix location. + * It returns the keys sorted in reverse order of LastModified time + * (freshest keys first). * * @param bucket * name of the bucket whose objects should be listed. @@ -311,7 +316,10 @@ public List list(String bucket, String prefix) do { lp.list(); } while (lp.truncated); - return lp.entries; + + Comparator comparator = Comparator.comparingLong(KeyInfo::getLastModifiedSecs); + return lp.entries.stream().sorted(comparator.reversed()) + .map(KeyInfo::getName).collect(Collectors.toList()); } /** @@ -620,8 +628,26 @@ static Properties properties(File authFile) return p; } + /** + * KeyInfo enables sorting of keys by lastModified time + */ + private static final class KeyInfo { + private final String name; + private final long lastModifiedSecs; + public KeyInfo(String aname, long lsecs) { + name = aname; + lastModifiedSecs = lsecs; + } + public String getName() { + return name; + } + public long getLastModifiedSecs() { + return lastModifiedSecs; + } + } + private final class ListParser extends DefaultHandler { - final List entries = new ArrayList<>(); + final List entries = new ArrayList<>(); private final String bucket; @@ -630,6 +656,8 @@ private final class ListParser extends DefaultHandler { boolean truncated; private StringBuilder data; + private String keyName; + private Instant keyLastModified; ListParser(String bn, String p) { bucket = bn; @@ -641,7 +669,7 @@ void list() throws IOException { if (prefix.length() > 0) args.put("prefix", prefix); //$NON-NLS-1$ if (!entries.isEmpty()) - args.put("marker", prefix + entries.get(entries.size() - 1)); //$NON-NLS-1$ + args.put("marker", prefix + entries.get(entries.size() - 1).getName()); //$NON-NLS-1$ for (int curAttempt = 0; curAttempt < maxAttempts; curAttempt++) { final HttpURLConnection c = open("GET", bucket, "", args); //$NON-NLS-1$ //$NON-NLS-2$ @@ -650,6 +678,8 @@ void list() throws IOException { case HttpURLConnection.HTTP_OK: truncated = false; data = null; + keyName = null; + keyLastModified = null; final XMLReader xr; try { @@ -683,8 +713,13 @@ void list() throws IOException { public void startElement(final String uri, final String name, final String qName, final Attributes attributes) throws SAXException { - if ("Key".equals(name) || "IsTruncated".equals(name)) //$NON-NLS-1$ //$NON-NLS-2$ + if ("Key".equals(name) || "IsTruncated".equals(name) || "LastModified".equals(name)) { //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ data = new StringBuilder(); + } + if ("Contents".equals(name)) { //$NON-NLS-1$ + keyName = null; + keyLastModified = null; + } } @Override @@ -704,10 +739,16 @@ public void characters(char[] ch, int s, int n) @Override public void endElement(final String uri, final String name, final String qName) throws SAXException { - if ("Key".equals(name)) //$NON-NLS-1$ - entries.add(data.toString().substring(prefix.length())); - else if ("IsTruncated".equals(name)) //$NON-NLS-1$ + if ("Key".equals(name)) { //$NON-NLS-1$ + keyName = data.toString().substring(prefix.length()); + } else if ("IsTruncated".equals(name)) { //$NON-NLS-1$ truncated = StringUtils.equalsIgnoreCase("true", data.toString()); //$NON-NLS-1$ + } else if ("LastModified".equals(name)) { //$NON-NLS-1$ + keyLastModified = Instant.parse(data.toString()); + } else if ("Contents".equals(name)) { //$NON-NLS-1$ + entries.add(new KeyInfo(keyName, keyLastModified.getEpochSecond())); + } + data = null; } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportAmazonS3.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportAmazonS3.java index 94f36d285..784f56615 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportAmazonS3.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportAmazonS3.java @@ -23,6 +23,7 @@ import java.util.Collections; import java.util.EnumSet; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Properties; import java.util.Set; @@ -241,11 +242,14 @@ WalkRemoteObjectDatabase openAlternate(String location) @Override Collection getPackNames() throws IOException { + // s3.list returns most recently modified packs first. + // These are the packs most likely to contain missing refs. + final List packList = s3.list(bucket, resolveKey("pack")); //$NON-NLS-1$ final HashSet have = new HashSet<>(); - have.addAll(s3.list(bucket, resolveKey("pack"))); //$NON-NLS-1$ + have.addAll(packList); final Collection packs = new ArrayList<>(); - for (String n : have) { + for (String n : packList) { if (!n.startsWith("pack-") || !n.endsWith(".pack")) //$NON-NLS-1$ //$NON-NLS-2$ continue; diff --git a/pom.xml b/pom.xml index 06e2f308a..d0a5c83a1 100644 --- a/pom.xml +++ b/pom.xml @@ -163,7 +163,7 @@ 4.3.1 3.1.0 9.4.25.v20191220 - 0.14.1 + 0.14.3 4.5.10 4.4.12 1.7.2 @@ -175,7 +175,7 @@ 3.1.12.2 3.0.0 3.0.0 - 3.0.0-M3 + 3.0.0-M4 ${maven-surefire-plugin-version} 3.8.1 @@ -298,7 +298,7 @@ org.apache.maven.plugins maven-pmd-plugin - 3.12.0 + 3.13.0 utf-8 100