Merge branch 'stable-7.0' into stable-7.1 * stable-7.0: FileSnapshot: silence "Not a Directory" exceptions FileSnapshot: refactor to share error handling Mark Attribute#getValue as @Nullable Fix potential NPE in ResolveMerger#getAttributesContentMergeStrategy Fix NPE in DiffFormatter#getDiffDriver Pack: ensure packfile is still valid while still recoverable WindowCache: add bulk purge(), call from bulk sites UploadPack#implies: add missing @since tag Disable MergeToolTest#testEmptyToolName Change-Id: Icb25fed5b703c6a39a64231fd8ca93c1f1a581be
diff --git a/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/MergeToolTest.java b/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/MergeToolTest.java index 54c4f26..6a95f9a 100644 --- a/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/MergeToolTest.java +++ b/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/MergeToolTest.java
@@ -27,6 +27,7 @@ import org.eclipse.jgit.internal.diffmergetool.MergeTools; import org.eclipse.jgit.lib.StoredConfig; import org.junit.Before; +import org.junit.Ignore; import org.junit.Test; /** @@ -77,6 +78,7 @@ public void testUserToolWithCommandNotFoundError() throws Exception { + errorReturnCode); } + @Ignore @Test public void testEmptyToolName() throws Exception { assumeLinuxPlatform();
diff --git a/org.eclipse.jgit/.settings/.api_filters b/org.eclipse.jgit/.settings/.api_filters index 9f8018d..1408a9d 100644 --- a/org.eclipse.jgit/.settings/.api_filters +++ b/org.eclipse.jgit/.settings/.api_filters
@@ -64,4 +64,12 @@ </message_arguments> </filter> </resource> + <resource path="src/org/eclipse/jgit/transport/UploadPack.java" type="org.eclipse.jgit.transport.UploadPack$RequestPolicy"> + <filter id="1176502275"> + <message_arguments> + <message_argument value="6.10.1"/> + <message_argument value="implies(UploadPack.RequestPolicy)"/> + </message_arguments> + </filter> + </resource> </component>
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/attributes/Attribute.java b/org.eclipse.jgit/src/org/eclipse/jgit/attributes/Attribute.java index fe3e22a..9c4d870 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/attributes/Attribute.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/attributes/Attribute.java
@@ -9,6 +9,8 @@ */ package org.eclipse.jgit.attributes; +import org.eclipse.jgit.annotations.Nullable; + /** * Represents an attribute. * <p> @@ -139,6 +141,7 @@ public State getState() { * * @return the attribute value (may be <code>null</code>) */ + @Nullable public String getValue() { return value; }
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/diff/DiffFormatter.java b/org.eclipse.jgit/src/org/eclipse/jgit/diff/DiffFormatter.java index fa446e1..6375a60 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/diff/DiffFormatter.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/diff/DiffFormatter.java
@@ -1356,13 +1356,17 @@ private boolean matchesAny(List<Pattern> patterns, String text) { private DiffDriver getDiffDriver(DiffEntry entry) { Attribute diffAttr = entry.getDiffAttribute(); - if (diffAttr != null) { - try { - return DiffDriver.valueOf(diffAttr.getValue()); - } catch (IllegalArgumentException e) { - return null; - } + if (diffAttr == null) { + return null; } - return null; + String diffAttrValue = diffAttr.getValue(); + if (diffAttrValue == null) { + return null; + } + try { + return DiffDriver.valueOf(diffAttrValue); + } catch (IllegalArgumentException e) { + return null; + } } }
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileSnapshot.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileSnapshot.java index c4fa3ee..8f83a37 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileSnapshot.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileSnapshot.java
@@ -15,6 +15,7 @@ import java.io.File; import java.io.IOException; +import java.nio.file.FileSystemException; import java.nio.file.NoSuchFileException; import java.nio.file.attribute.BasicFileAttributes; import java.time.Duration; @@ -22,6 +23,7 @@ import java.time.ZoneId; import java.time.format.DateTimeFormatter; import java.util.Locale; +import java.util.NoSuchElementException; import java.util.Objects; import java.util.concurrent.TimeUnit; @@ -208,14 +210,8 @@ protected FileSnapshot(File file, boolean useConfig) { this.useConfig = useConfig; BasicFileAttributes fileAttributes = null; try { - fileAttributes = FS.DETECTED.fileAttributes(file); - } catch (NoSuchFileException e) { - this.lastModified = Instant.EPOCH; - this.size = 0L; - this.fileKey = MISSING_FILEKEY; - return; - } catch (IOException e) { - LOG.error(e.getMessage(), e); + fileAttributes = getFileAttributes(file); + } catch (NoSuchElementException e) { this.lastModified = Instant.EPOCH; this.size = 0L; this.fileKey = MISSING_FILEKEY; @@ -285,16 +281,11 @@ public boolean isModified(File path) { long currSize; Object currFileKey; try { - BasicFileAttributes fileAttributes = FS.DETECTED.fileAttributes(path); + BasicFileAttributes fileAttributes = getFileAttributes(path); currLastModified = fileAttributes.lastModifiedTime().toInstant(); currSize = fileAttributes.size(); currFileKey = getFileKey(fileAttributes); - } catch (NoSuchFileException e) { - currLastModified = Instant.EPOCH; - currSize = 0L; - currFileKey = MISSING_FILEKEY; - } catch (IOException e) { - LOG.error(e.getMessage(), e); + } catch (NoSuchElementException e) { currLastModified = Instant.EPOCH; currSize = 0L; currFileKey = MISSING_FILEKEY; @@ -552,4 +543,18 @@ private FileStoreAttributes fileStoreAttributeCache() { } return fileStoreAttributeCache; } + + private static BasicFileAttributes getFileAttributes(File path) throws NoSuchElementException { + try { + return FS.DETECTED.fileAttributes(path); + } catch (NoSuchFileException e) { + } catch (FileSystemException e) { + if (!e.getMessage().endsWith("Not a directory")) { + LOG.error(e.getMessage(), e); + } + } catch (IOException e) { + LOG.error(e.getMessage(), e); + } + throw new NoSuchElementException(path.toString()); + } }
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/Pack.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/Pack.java index 3518342..61577a9 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/Pack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/Pack.java
@@ -296,15 +296,28 @@ void resolve(Set<ObjectId> matches, AbbreviatedObjectId id, int matchLimit) } /** - * Close the resources utilized by this repository + * Close the resources utilized by these pack files + * + * @param packs + * packs to close + */ + public static void close(Set<Pack> packs) { + WindowCache.purge(packs); + packs.forEach(p -> p.closeIndices()); + } + + /** + * Close the resources utilized by this pack file */ public void close() { WindowCache.purge(this); - synchronized (this) { - loadedIdx.clear(); - reverseIdx.clear(); - bitmapIdx.clear(); - } + closeIndices(); + } + + private synchronized void closeIndices() { + loadedIdx.clear(); + reverseIdx.clear(); + bitmapIdx.clear(); } /** @@ -584,13 +597,19 @@ private void copyAsIs2(PackOutputStream out, LocalObjectToPack src, assert(crc2 != null); crc2.update(buf, 0, n); } + cnt -= n; if (!isHeaderWritten) { + if (invalid && cnt > 0) { + // Since this is not the last iteration and the packfile is invalid, + // better to assume the iterations will not all complete here while + // it is still likely recoverable. + throw new StoredObjectRepresentationNotAvailableException(invalidatingCause); + } out.writeHeader(src, inflatedLength); isHeaderWritten = true; } out.write(buf, 0, n); pos += n; - cnt -= n; } if (validate) { assert(crc2 != null);
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackDirectory.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackDirectory.java index 51a8148..e31126f 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackDirectory.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackDirectory.java
@@ -24,6 +24,7 @@ import java.util.Collections; import java.util.EnumMap; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -111,9 +112,7 @@ void create() throws IOException { void close() { PackList packs = packList.get(); if (packs != NO_PACKS && packList.compareAndSet(packs, NO_PACKS)) { - for (Pack p : packs.packs) { - p.close(); - } + Pack.close(Set.of(packs.packs)); } } @@ -484,9 +483,7 @@ private PackList scanPacksImpl(PackList old) { return old; } - for (Pack p : forReuse.values()) { - p.close(); - } + Pack.close(new HashSet<>(forReuse.values())); if (list.isEmpty()) { return new PackList(snapshot, NO_PACKS.packs);
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/WindowCache.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/WindowCache.java index 30f8240..3ec7e6e 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/WindowCache.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/WindowCache.java
@@ -17,6 +17,7 @@ import java.util.Collections; import java.util.Map; import java.util.Random; +import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.atomic.AtomicBoolean; @@ -397,7 +398,11 @@ static final ByteWindow get(Pack pack, long offset) } static final void purge(Pack pack) { - cache.removeAll(pack); + purge(Collections.singleton(pack)); + } + + static final void purge(Set<Pack> packs) { + cache.removeAll(packs); } /** cleanup released and/or garbage collected windows. */ @@ -710,20 +715,21 @@ private void removeAll() { /** * Clear all entries related to a single file. * <p> - * Typically this method is invoked during {@link Pack#close()}, when we - * know the pack is never going to be useful to us again (for example, it no - * longer exists on disk). A concurrent reader loading an entry from this - * same pack may cause the pack to become stuck in the cache anyway. + * Typically this method is invoked during {@link Pack#close()}, or + * {@link Pack#close(Set)}, when we know the packs are never going to be + * useful to us again (for example, they no longer exist on disk after gc). + * A concurrent reader loading an entry from these same packs may cause a + * pack to become stuck in the cache anyway. * - * @param pack - * the file to purge all entries of. + * @param packs + * the files to purge all entries of. */ - private void removeAll(Pack pack) { + private void removeAll(Set<Pack> packs) { for (int s = 0; s < tableSize; s++) { final Entry e1 = table.get(s); boolean hasDead = false; for (Entry e = e1; e != null; e = e.next) { - if (e.ref.getPack() == pack) { + if (packs.contains(e.ref.getPack())) { e.kill(); hasDead = true; } else if (e.dead)
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java b/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java index 033f280..dc96f65 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java
@@ -1520,9 +1520,12 @@ private MergeResult<RawText> contentMerge(CanonicalTreeParser base, private ContentMergeStrategy getAttributesContentMergeStrategy( Attributes attributes, ContentMergeStrategy strategy) { Attribute attr = attributes.get(Constants.ATTR_MERGE); - if (attr != null && attr.getValue() - .equals(Constants.ATTR_BUILTIN_UNION_MERGE_DRIVER)) { - return ContentMergeStrategy.UNION; + if (attr != null) { + String attrValue = attr.getValue(); + if (attrValue != null && attrValue + .equals(Constants.ATTR_BUILTIN_UNION_MERGE_DRIVER)) { + return ContentMergeStrategy.UNION; + } } return strategy; }
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java index 2270c5e..6013832 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
@@ -158,7 +158,7 @@ public enum RequestPolicy { * @param implied * the implied policy based on its bitmask. * @return true if the policy is implied. - * @since 7.1 + * @since 6.10.1 */ public boolean implies(RequestPolicy implied) { return (bitmask & implied.bitmask) != 0;