Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/891#discussion_r39621962
--- Diff:
core/src/main/java/org/apache/brooklyn/core/mgmt/persist/FileBasedObjectStore.java
---
@@ -346,53 +346,27 @@ protected File backupDirByMoving(File dir) throws
InterruptedException, IOExcept
return newDir;
}
- private static boolean WARNED_ON_NON_ATOMIC_FILE_UPDATES = false;
/**
* Attempts an fs level atomic move then fall back to pure java rename.
* Assumes files are on same mount point.
- * <p>
- * TODO Java 7 gives an atomic Files.move() which would be preferred.
+ * Overwriting existing destFile
*/
static void moveFile(File srcFile, File destFile) throws IOException,
InterruptedException {
- // Try rename first - it is a *much* cheaper call than invoking a
system call in Java.
- // However, rename is not guaranteed cross platform to succeed if
the destination exists,
- // and not guaranteed to be atomic, but it usually seems to do the
right thing...
- boolean result;
- result = srcFile.renameTo(destFile);
- if (result) {
- if (log.isTraceEnabled()) log.trace("java rename of {} to {}
completed", srcFile, destFile);
- return;
+ if (destFile.isDirectory()) {
+ deleteCompletely(destFile);
}
-
- if (!Os.isMicrosoftWindows()) {
- // this command, if it succeeds, is guaranteed to be atomic,
and it will usually overwrite
- String cmd = "mv '"+srcFile.getAbsolutePath()+"'
'"+destFile.getAbsolutePath()+"'";
-
- int exitStatus = new
ProcessTool().execCommands(MutableMap.<String,String>of(), MutableList.of(cmd),
null);
- // prefer the above to the below because it wraps it in the
appropriate bash
-// Process proc = Runtime.getRuntime().exec(cmd);
-// result = proc.waitFor();
-
- if (log.isTraceEnabled()) log.trace("FS move of {} to {}
completed, code {}", new Object[] { srcFile, destFile, exitStatus });
- if (exitStatus == 0) return;
+
+ try {
+ Files.move(srcFile.toPath(), destFile.toPath(),
StandardCopyOption.ATOMIC_MOVE, StandardCopyOption.REPLACE_EXISTING);
--- End diff --
Note the `REPLACE_EXISTING` is ignored when ATOMIC_MOVE is used - see
http://docs.oracle.com/javase/7/docs/api/java/nio/file/Files.html#move(java.nio.file.Path,%20java.nio.file.Path,%20java.nio.file.CopyOption...),
which says "all other options are ignored".
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
