From 8a9e1ab59f4346a02f1aebc26a41691d0e6edea0 Mon Sep 17 00:00:00 2001 From: kkkon Date: Sun, 17 Feb 2013 13:19:28 +0900 Subject: [PATCH 1/2] [FIXED JENKINS-16846] fix logic and add test FilePath#moveAllChildrenTo faild, when same name directory exist. change Void to void, recursive_invoke is internal helper function. thanks review by jglick --- core/src/main/java/hudson/FilePath.java | 31 +++++++++++++++++++-- core/src/test/java/hudson/FilePathTest.java | 29 +++++++++++++++++++ 2 files changed, 57 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/hudson/FilePath.java b/core/src/main/java/hudson/FilePath.java index 25362a7f94..1fce4cca8a 100644 --- a/core/src/main/java/hudson/FilePath.java +++ b/core/src/main/java/hudson/FilePath.java @@ -1657,15 +1657,40 @@ public final class FilePath implements Serializable { } act(new FileCallable() { private static final long serialVersionUID = 1L; - public Void invoke(File f, VirtualChannel channel) throws IOException { - File t = new File(target.getRemote()); - + private void recursive_invoke(final File t, final File f ) throws IOException { for(File child : f.listFiles()) { File target = new File(t, child.getName()); + LOGGER.log(Level.FINE,"child=" + child.getAbsolutePath() + "\ntarget=" + target.getAbsolutePath() ); + if ( child.isDirectory() && target.isDirectory() ) + { + final String childName = child.getName(); + final String targetName = target.getName(); + if ( null != childName && null != targetName ) + { + if ( 0 == childName.compareTo( targetName ) ) + { + if ( Util.isSymlink( child ) ) { + if(!child.renameTo(target)) { + throw new IOException("Failed to rename "+child+" to "+target); + } + } else { + // avoid fail to exec child.renameTo(taget) + recursive_invoke( target, child ); + } + continue; + } + } + } if(!child.renameTo(target)) throw new IOException("Failed to rename "+child+" to "+target); } f.delete(); + } + public Void invoke(File f, VirtualChannel channel) throws IOException { + File t = new File(target.getRemote()); + + recursive_invoke( t, f ); + return null; } }); diff --git a/core/src/test/java/hudson/FilePathTest.java b/core/src/test/java/hudson/FilePathTest.java index 8641bcef1c..69af37ad99 100644 --- a/core/src/test/java/hudson/FilePathTest.java +++ b/core/src/test/java/hudson/FilePathTest.java @@ -577,4 +577,33 @@ public class FilePathTest extends ChannelTestCase { return new ByteArrayInputStream(buf.toByteArray()); } + + @Bug(16846) + public void testMoveAllChildrenTo() throws IOException, InterruptedException { + final File tmp = Util.createTempDir(); + try + { + final String dirname = "sub"; + final File top = new File(tmp, "test"); + final File sub = new File(top, dirname); + final File subsub = new File(sub, dirname); + + subsub.mkdirs(); + + final File subFile1 = new File( sub.getAbsolutePath() + "/file1.txt" ); + subFile1.createNewFile(); + final File subFile2 = new File( subsub.getAbsolutePath() + "/file2.txt" ); + subFile2.createNewFile(); + + final FilePath src = new FilePath(sub); + final FilePath dst = new FilePath(top); + + // test conflict subdir + src.moveAllChildrenTo(dst); + } + finally + { + Util.deleteRecursive(tmp); + } + } } From 20347431aee7960846550538c4d48d11225d28d3 Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Sat, 19 Apr 2014 18:30:52 -0700 Subject: [PATCH 2/2] [JENKINS-16846] This is a much simpler fix --- core/src/main/java/hudson/FilePath.java | 41 +++++++------------------ 1 file changed, 11 insertions(+), 30 deletions(-) diff --git a/core/src/main/java/hudson/FilePath.java b/core/src/main/java/hudson/FilePath.java index 1fce4cca8a..f852b56b46 100644 --- a/core/src/main/java/hudson/FilePath.java +++ b/core/src/main/java/hudson/FilePath.java @@ -1657,40 +1657,21 @@ public final class FilePath implements Serializable { } act(new FileCallable() { private static final long serialVersionUID = 1L; - private void recursive_invoke(final File t, final File f ) throws IOException { - for(File child : f.listFiles()) { + public Void invoke(File f, VirtualChannel channel) throws IOException { + // JENKINS-16846: if f.getName() is the same as one of the files/directories in f, + // then the rename op will fail + File tmp = new File(f.getAbsolutePath()+".__rename"); + if (!f.renameTo(tmp)) + throw new IOException("Failed to rename "+f+" to "+tmp); + + File t = new File(target.getRemote()); + + for(File child : tmp.listFiles()) { File target = new File(t, child.getName()); - LOGGER.log(Level.FINE,"child=" + child.getAbsolutePath() + "\ntarget=" + target.getAbsolutePath() ); - if ( child.isDirectory() && target.isDirectory() ) - { - final String childName = child.getName(); - final String targetName = target.getName(); - if ( null != childName && null != targetName ) - { - if ( 0 == childName.compareTo( targetName ) ) - { - if ( Util.isSymlink( child ) ) { - if(!child.renameTo(target)) { - throw new IOException("Failed to rename "+child+" to "+target); - } - } else { - // avoid fail to exec child.renameTo(taget) - recursive_invoke( target, child ); - } - continue; - } - } - } if(!child.renameTo(target)) throw new IOException("Failed to rename "+child+" to "+target); } - f.delete(); - } - public Void invoke(File f, VirtualChannel channel) throws IOException { - File t = new File(target.getRemote()); - - recursive_invoke( t, f ); - + tmp.delete(); return null; } });