motiejus
/
jgit
1
Fork 0
Code Releases Activity

Merge branch 'stable-4.8' into stable-4.9 

* stable-4.8:
  Fix atomic lock file creation on NFS
  Use constant for ".lock"
  Fix handling of option core.supportsAtomicCreateNewFile
  GC: Avoid logging errors when deleting non-empty folders

Change-Id: Id7f68f4e8fb07a21737e3b7090f600507a13bbbe
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
This commit is contained in:
Matthias Sohn 2018-09-08 09:02:08 +02:00
parent a074203fa9 8699a95d2d
commit 4ce79bf738
12 changed files with 239 additions and 76 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/RefUpdateTest.java
View File

@ -47,6 +47,7 @@
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.eclipse.jgit.junit.Assert.assertEquals;
import static org.eclipse.jgit.lib.Constants.LOCK_SUFFIX;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
@ -819,11 +820,11 @@ public void tryRenameWhenLocked(String toLock, String fromName,
// Check that the involved refs are the same despite the failure
assertExists(false, toName);
if (!toLock.equals(toName))
assertExists(false, toName + ".lock");
assertExists(true, toLock + ".lock");
assertExists(false, toName + LOCK_SUFFIX);
assertExists(true, toLock + LOCK_SUFFIX);
if (!toLock.equals(fromName))
assertExists(false, "logs/" + fromName + ".lock");
assertExists(false, "logs/" + toName + ".lock");
assertExists(false, "logs/" + fromName + LOCK_SUFFIX);
assertExists(false, "logs/" + toName + LOCK_SUFFIX);
assertEquals(oldHeadId, db.resolve(Constants.HEAD));
assertEquals(oldfromId, db.resolve(fromName));
assertNull(db.resolve(toName));

75
org.eclipse.jgit/.settings/.api_filters
View File

@ -1,12 +1,6 @@
<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<component id="org.eclipse.jgit" version="2">
<resource path="META-INF/MANIFEST.MF">
<filter id="924844039">
<message_arguments>
<message_argument value="4.9.1"/>
<message_argument value="4.9.0"/>
</message_arguments>
</filter>
<filter id="924844039">
<message_arguments>
<message_argument value="4.9.4"/>
@ -15,24 +9,12 @@
</filter>
</resource>
<resource path="src/org/eclipse/jgit/lib/ConfigConstants.java" type="org.eclipse.jgit.lib.ConfigConstants">
<filter id="336658481">
<message_arguments>
<message_argument value="org.eclipse.jgit.lib.ConfigConstants"/>
<message_argument value="CONFIG_KEY_IN_CORE_LIMIT"/>
</message_arguments>
</filter>
<filter id="336658481">
<message_arguments>
<message_argument value="org.eclipse.jgit.lib.ConfigConstants"/>
<message_argument value="CONFIG_KEY_SUPPORTSATOMICFILECREATION"/>
</message_arguments>
</filter>
<filter id="336658481">
<message_arguments>
<message_argument value="org.eclipse.jgit.lib.ConfigConstants"/>
<message_argument value="CONFIG_MERGE_SECTION"/>
</message_arguments>
</filter>
<filter id="1141899266">
<message_arguments>
<message_argument value="4.5"/>
@ -41,23 +23,12 @@
</message_arguments>
</filter>
</resource>
<resource path="src/org/eclipse/jgit/lib/ReflogEntry.java" type="org.eclipse.jgit.lib.ReflogEntry">
<filter comment="adding enum constant does not break binary compatibility" id="403767336">
<resource path="src/org/eclipse/jgit/lib/Constants.java" type="org.eclipse.jgit.lib.Constants">
<filter id="1141899266">
<message_arguments>
<message_argument value="org.eclipse.jgit.lib.ReflogEntry"/>
<message_argument value="PREFIX_CREATED"/>
</message_arguments>
</filter>
<filter comment="adding enum constant does not break binary compatibility" id="403767336">
<message_arguments>
<message_argument value="org.eclipse.jgit.lib.ReflogEntry"/>
<message_argument value="PREFIX_FAST_FORWARD"/>
</message_arguments>
</filter>
<filter comment="adding enum constant does not break binary compatibility" id="403767336">
<message_arguments>
<message_argument value="org.eclipse.jgit.lib.ReflogEntry"/>
<message_argument value="PREFIX_FORCED_UPDATE"/>
<message_argument value="4.8"/>
<message_argument value="4.9"/>
<message_argument value="LOCK_SUFFIX"/>
</message_arguments>
</filter>
</resource>
@ -70,26 +41,6 @@
</message_arguments>
</filter>
</resource>
<resource path="src/org/eclipse/jgit/transport/http/HttpConnection.java" type="org.eclipse.jgit.transport.http.HttpConnection">
<filter id="403767336">
<message_arguments>
<message_argument value="org.eclipse.jgit.transport.http.HttpConnection"/>
<message_argument value="HTTP_11_MOVED_TEMP"/>
</message_arguments>
</filter>
<filter id="403767336">
<message_arguments>
<message_argument value="org.eclipse.jgit.transport.http.HttpConnection"/>
<message_argument value="HTTP_MOVED_TEMP"/>
</message_arguments>
</filter>
<filter id="403767336">
<message_arguments>
<message_argument value="org.eclipse.jgit.transport.http.HttpConnection"/>
<message_argument value="HTTP_SEE_OTHER"/>
</message_arguments>
</filter>
</resource>
<resource path="src/org/eclipse/jgit/util/FS.java" type="org.eclipse.jgit.util.FS">
<filter id="1141899266">
<message_arguments>
@ -105,5 +56,21 @@
<message_argument value="supportsAtomicCreateNewFile()"/>
</message_arguments>
</filter>
<filter id="1141899266">
<message_arguments>
<message_argument value="4.7"/>
<message_argument value="4.9"/>
<message_argument value="createNewFileAtomic(File)"/>
</message_arguments>
</filter>
</resource>
<resource path="src/org/eclipse/jgit/util/FS.java" type="org.eclipse.jgit.util.FS$LockToken">
<filter id="1141899266">
<message_arguments>
<message_argument value="4.7"/>
<message_argument value="4.9"/>
<message_argument value="LockToken"/>
</message_arguments>
</filter>
</resource>
</component>

3
org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
View File

@ -120,6 +120,7 @@ checkoutUnexpectedResult=Checkout returned unexpected result {0}
classCastNotA=Not a {0}
cloneNonEmptyDirectory=Destination path "{0}" already exists and is not an empty directory
closed=closed
closeLockTokenFailed=Closing LockToken ''{0}'' failed
collisionOn=Collision on {0}
commandClosedStderrButDidntExit=Command {0} closed stderr stream but didn''t exit within timeout {1} seconds
commandRejectedByHook=Rejected by "{0}" hook.\n{1}
@ -289,6 +290,7 @@ expectedLessThanGot=expected less than ''{0}'', got ''{1}''
expectedPktLineWithService=expected pkt-line with ''# service=-'', got ''{0}''
expectedReceivedContentType=expected Content-Type {0}; received Content-Type {1}
expectedReportForRefNotReceived={0}: expected report for ref {1} not received
failedAtomicFileCreation=Atomic file creation failed, number of hard links to file {0} was not 2 but {1}"
failedToDetermineFilterDefinition=An exception occured while determining filter definitions
failedUpdatingRefs=failed updating refs
failureDueToOneOfTheFollowing=Failure due to one of the following:
@ -707,6 +709,7 @@ unknownObjectType2=unknown
unknownRepositoryFormat=Unknown repository format
unknownRepositoryFormat2=Unknown repository format "{0}"; expected "0".
unknownZlibError=Unknown zlib error.
unlockLockFileFailed=Unlocking LockFile ''{0}'' failed
unmergedPath=Unmerged path: {0}
unmergedPaths=Repository contains unmerged paths
unpackException=Exception while parsing pack stream

3
org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
View File

@ -178,6 +178,7 @@ public static JGitText get() {
/***/ public String checkoutUnexpectedResult;
/***/ public String classCastNotA;
/***/ public String cloneNonEmptyDirectory;
/***/ public String closeLockTokenFailed;
/***/ public String closed;
/***/ public String collisionOn;
/***/ public String commandClosedStderrButDidntExit;
@ -348,6 +349,7 @@ public static JGitText get() {
/***/ public String expectedPktLineWithService;
/***/ public String expectedReceivedContentType;
/***/ public String expectedReportForRefNotReceived;
/***/ public String failedAtomicFileCreation;
/***/ public String failedToDetermineFilterDefinition;
/***/ public String failedUpdatingRefs;
/***/ public String failureDueToOneOfTheFollowing;
@ -766,6 +768,7 @@ public static JGitText get() {
/***/ public String unknownRepositoryFormat;
/***/ public String unknownRepositoryFormat2;
/***/ public String unknownZlibError;
/***/ public String unlockLockFileFailed;
/***/ public String unmergedPath;
/***/ public String unmergedPaths;
/***/ public String unpackException;

3
org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java
View File

@ -54,6 +54,7 @@
import java.io.StringWriter;
import java.nio.channels.Channels;
import java.nio.channels.FileChannel;
import java.nio.file.DirectoryNotEmptyException;
import java.nio.file.DirectoryStream;
import java.nio.file.Files;
import java.nio.file.Path;
@ -947,6 +948,8 @@ private boolean isDirectory(Path p) {
private void delete(Path d) {
try {
Files.delete(d);
} catch (DirectoryNotEmptyException e) {
// Don't log
} catch (IOException e) {
LOG.error(MessageFormat.format(JGitText.get().cannotDeleteFile, d),
e);

37
org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/LockFile.java
View File

@ -44,6 +44,8 @@
package org.eclipse.jgit.internal.storage.file;
import static org.eclipse.jgit.lib.Constants.LOCK_SUFFIX;
import java.io.File;
import java.io.FileInputStream;
import java.io.FileNotFoundException;
@ -62,7 +64,10 @@
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.util.FS;
import org.eclipse.jgit.util.FS.LockToken;
import org.eclipse.jgit.util.FileUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/**
* Git style file locking and replacement.
@ -75,7 +80,7 @@
* name.
*/
public class LockFile {
static final String SUFFIX = ".lock"; //$NON-NLS-1$
private final static Logger LOG = LoggerFactory.getLogger(LockFile.class);
/**
* Unlock the given file.
@ -105,14 +110,15 @@ public static boolean unlock(final File file) {
* @return lock file
*/
static File getLockFile(File file) {
return new File(file.getParentFile(), file.getName() + SUFFIX);
return new File(file.getParentFile(),
file.getName() + LOCK_SUFFIX);
}
/** Filter to skip over active lock files when listing a directory. */
static final FilenameFilter FILTER = new FilenameFilter() {
@Override
public boolean accept(File dir, String name) {
return !name.endsWith(SUFFIX);
return !name.endsWith(LOCK_SUFFIX);
}
};
@ -130,6 +136,8 @@ public boolean accept(File dir, String name) {
private FileSnapshot commitSnapshot;
private LockToken token;
/**
* Create a new lock for any file.
*
@ -168,7 +176,8 @@ public LockFile(final File f) {
*/
public boolean lock() throws IOException {
FileUtils.mkdirs(lck.getParentFile(), true);
if (FS.DETECTED.createNewFile(lck)) {
token = FS.DETECTED.createNewFileAtomic(lck);
if (token.isCreated()) {
haveLck = true;
try {
os = new FileOutputStream(lck);
@ -176,6 +185,8 @@ public boolean lock() throws IOException {
unlock();
throw ioe;
}
} else {
closeToken();
}
return haveLck;
}
@ -456,6 +467,7 @@ public boolean commit() {
try {
FileUtils.rename(lck, ref, StandardCopyOption.ATOMIC_MOVE);
haveLck = false;
closeToken();
return true;
} catch (IOException e) {
unlock();
@ -463,6 +475,13 @@ public boolean commit() {
}
}
private void closeToken() {
if (token != null) {
token.close();
token = null;
}
}
private void saveStatInformation() {
if (needSnapshot)
commitSnapshot = FileSnapshot.save(lck);
@ -501,8 +520,9 @@ public void unlock() {
if (os != null) {
try {
os.close();
} catch (IOException ioe) {
// Ignore this
} catch (IOException e) {
LOG.error(MessageFormat
.format(JGitText.get().unlockLockFileFailed, lck), e);
}
os = null;
}
@ -512,7 +532,10 @@ public void unlock() {
try {
FileUtils.delete(lck, FileUtils.RETRY);
} catch (IOException e) {
// couldn't delete the file even after retry.
LOG.error(MessageFormat
.format(JGitText.get().unlockLockFileFailed, lck), e);
} finally {
closeToken();
}
}
}

5
org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ReflogWriter.java
View File

@ -46,6 +46,7 @@
package org.eclipse.jgit.internal.storage.file;
import static org.eclipse.jgit.lib.Constants.HEAD;
import static org.eclipse.jgit.lib.Constants.LOCK_SUFFIX;
import static org.eclipse.jgit.lib.Constants.R_HEADS;
import static org.eclipse.jgit.lib.Constants.R_NOTES;
import static org.eclipse.jgit.lib.Constants.R_REFS;
@ -84,7 +85,7 @@ public class ReflogWriter {
* @return the name of the ref's lock ref.
*/
public static String refLockFor(String name) {
return name + LockFile.SUFFIX;
return name + LOCK_SUFFIX;
}
private final RefDirectory refdb;
@ -252,4 +253,4 @@ private boolean shouldAutoCreateLog(String refName) {
|| refName.startsWith(R_REMOTES)
|| refName.startsWith(R_NOTES);
}
}
}

7
org.eclipse.jgit/src/org/eclipse/jgit/lib/Constants.java
View File

@ -679,6 +679,13 @@ public static byte[] encode(final String str) {
public static final ObjectId EMPTY_BLOB_ID = ObjectId
.fromString("e69de29bb2d1d6434b8b29ae775ad8c2e48c5391");
/**
* Suffix of lock file name
*
* @since 4.8
*/
public static final String LOCK_SUFFIX = ".lock"; //$NON-NLS-1$
private Constants() {
// Hide the default constructor
}

8
org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java
View File

@ -48,6 +48,8 @@
package org.eclipse.jgit.lib;
import static org.eclipse.jgit.lib.Constants.LOCK_SUFFIX;
import java.io.BufferedOutputStream;
import java.io.File;
import java.io.FileNotFoundException;
@ -1326,10 +1328,12 @@ public RepositoryState getRepositoryState() {
*/
public static boolean isValidRefName(final String refName) {
final int len = refName.length();
if (len == 0)
if (len == 0) {
return false;
if (refName.endsWith(".lock")) //$NON-NLS-1$
}
if (refName.endsWith(LOCK_SUFFIX)) {
return false;
}
// Refs may be stored as loose files so invalid paths
// on the local system must also be invalid refs.

4
org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportSftp.java
View File

@ -43,6 +43,8 @@
package org.eclipse.jgit.transport;
import static org.eclipse.jgit.lib.Constants.LOCK_SUFFIX;
import java.io.BufferedReader;
import java.io.FileNotFoundException;
import java.io.IOException;
@ -341,7 +343,7 @@ OutputStream writeFile(final String path,
@Override
void writeFile(final String path, final byte[] data) throws IOException {
final String lock = path + ".lock"; //$NON-NLS-1$
final String lock = path + LOCK_SUFFIX;
try {
super.writeFile(lock, data);
try {

71
org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java
View File

@ -45,6 +45,7 @@
import java.io.BufferedReader;
import java.io.ByteArrayInputStream;
import java.io.Closeable;
import java.io.File;
import java.io.IOException;
import java.io.InputStream;
@ -52,6 +53,8 @@
import java.io.OutputStream;
import java.io.PrintStream;
import java.nio.charset.Charset;
import java.nio.file.Files;
import java.nio.file.Path;
import java.security.AccessController;
import java.security.PrivilegedAction;
import java.text.MessageFormat;
@ -59,6 +62,7 @@
import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.TimeUnit;
@ -827,13 +831,78 @@ public void createSymLink(File path, String target) throws IOException {
* the file to be created
* @return <code>true</code> if the file was created, <code>false</code> if
* the file already existed
* @throws IOException
* @throws java.io.IOException
* @deprecated use {@link #createNewFileAtomic(File)} instead
* @since 4.5
*/
@Deprecated
public boolean createNewFile(File path) throws IOException {
return path.createNewFile();
}
/**
* A token representing a file created by
* {@link #createNewFileAtomic(File)}. The token must be retained until the
* file has been deleted in order to guarantee that the unique file was
* created atomically. As soon as the file is no longer needed the lock
* token must be closed.
*
* @since 4.7
*/
public static class LockToken implements Closeable {
private boolean isCreated;
private Optional<Path> link;
LockToken(boolean isCreated, Optional<Path> link) {
this.isCreated = isCreated;
this.link = link;
}
/**
* @return {@code true} if the file was created successfully
*/
public boolean isCreated() {
return isCreated;
}
@Override
public void close() {
if (link.isPresent()) {
try {
Files.delete(link.get());
} catch (IOException e) {
LOG.error(MessageFormat.format(JGitText.get().closeLockTokenFailed,
this), e);
}
}
}
@Override
public String toString() {
return "LockToken [lockCreated=" + isCreated + //$NON-NLS-1$
", link=" //$NON-NLS-1$
+ (link.isPresent() ? link.get().getFileName() + "]" //$NON-NLS-1$
: "<null>]"); //$NON-NLS-1$
}
}
/**
* Create a new file. See {@link java.io.File#createNewFile()}. Subclasses
* of this class may take care to provide a safe implementation for this
* even if {@link #supportsAtomicCreateNewFile()} is <code>false</code>
*
* @param path
* the file to be created
* @return LockToken this token must be closed after the created file was
* deleted
* @throws IOException
* @since 4.7
*/
public LockToken createNewFileAtomic(File path) throws IOException {
return new LockToken(path.createNewFile(), Optional.empty());
}
/**
* See {@link FileUtils#relativizePath(String, String, String, boolean)}.
*

90
org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java
View File

@ -52,14 +52,19 @@
import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.attribute.PosixFilePermission;
import java.text.MessageFormat;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.UUID;
import org.eclipse.jgit.annotations.Nullable;
import org.eclipse.jgit.api.errors.JGitInternalException;
import org.eclipse.jgit.errors.CommandFailedException;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.internal.JGitText;
import org.eclipse.jgit.lib.ConfigConstants;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.Repository;
@ -360,9 +365,12 @@ public boolean supportsAtomicCreateNewFile() {
* multiple clients manage to create the same lock file nlink would be
* greater than 2 showing the error.
*
* @see https://www.time-travellers.org/shane/papers/NFS_considered_harmful.html
* @see "https://www.time-travellers.org/shane/papers/NFS_considered_harmful.html"
*
* @deprecated use {@link FS_POSIX#createNewFileAtomic(File)} instead
* @since 4.5
*/
@Deprecated
public boolean createNewFile(File lock) throws IOException {
if (!lock.createNewFile()) {
return false;
@ -371,22 +379,94 @@ public boolean createNewFile(File lock) throws IOException {
return true;
}
Path lockPath = lock.toPath();
Path link = Files.createLink(Paths.get(lock.getAbsolutePath() + ".lnk"), //$NON-NLS-1$
lockPath);
Path link = null;
try {
link = Files.createLink(
Paths.get(lock.getAbsolutePath() + ".lnk"), //$NON-NLS-1$
lockPath);
Integer nlink = (Integer) (Files.getAttribute(lockPath,
"unix:nlink")); //$NON-NLS-1$
if (nlink != 2) {
if (nlink > 2) {
LOG.warn("nlink of link to lock file {0} was not 2 but {1}", //$NON-NLS-1$
lock.getPath(), nlink);
return false;
} else if (nlink < 2) {
supportsUnixNLink = false;
}
return true;
} catch (UnsupportedOperationException | IllegalArgumentException e) {
supportsUnixNLink = false;
return true;
} finally {
Files.delete(link);
if (link != null) {
Files.delete(link);
}
}
}
/**
* {@inheritDoc}
* <p>
* An implementation of the File#createNewFile() semantics which can create
* a unique file atomically also on NFS. If the config option
* {@code core.supportsAtomicCreateNewFile = true} (which is the default)
* then simply File#createNewFile() is called.
*
* But if {@code core.supportsAtomicCreateNewFile = false} then after
* successful creation of the lock file a hard link to that lock file is
* created and the attribute nlink of the lock file is checked to be 2. If
* multiple clients manage to create the same lock file nlink would be
* greater than 2 showing the error. The hard link needs to be retained
* until the corresponding file is no longer needed in order to prevent that
* another process can create the same file concurrently using another NFS
* client which might not yet see the file due to caching.
*
* @see "https://www.time-travellers.org/shane/papers/NFS_considered_harmful.html"
* @param file
* the unique file to be created atomically
* @return LockToken this lock token must be held until the file is no
* longer needed
* @throws IOException
* @since 5.0
*/
@Override
public LockToken createNewFileAtomic(File file) throws IOException {
if (!file.createNewFile()) {
return token(false, null);
}
if (supportsAtomicCreateNewFile() || !supportsUnixNLink) {
return token(true, null);
}
Path link = null;
Path path = file.toPath();
try {
link = Files.createLink(Paths.get(uniqueLinkPath(file)), path);
Integer nlink = (Integer) (Files.getAttribute(path,
"unix:nlink")); //$NON-NLS-1$
if (nlink.intValue() > 2) {
LOG.warn(MessageFormat.format(
JGitText.get().failedAtomicFileCreation, path, nlink));
return token(false, link);
} else if (nlink.intValue() < 2) {
supportsUnixNLink = false;
}
return token(true, link);
} catch (UnsupportedOperationException | IllegalArgumentException e) {
supportsUnixNLink = false;
return token(true, link);
}
}
private static LockToken token(boolean created, @Nullable Path p) {
return ((p != null) && Files.exists(p))
? new LockToken(created, Optional.of(p))
: new LockToken(created, Optional.empty());
}
private static String uniqueLinkPath(File file) {
UUID id = UUID.randomUUID();
return file.getAbsolutePath() + "." //$NON-NLS-1$
+ Long.toHexString(id.getMostSignificantBits())
+ Long.toHexString(id.getLeastSignificantBits());
}
}