widelands / wl_addons_server

Provides the add-ons server and all add-ons for the Widelands game.
https://www.widelands.org
Other
6 stars 4 forks source link

Testing locally: Addons are not copied from /tmp/ to addons/ #125

Closed frankystone closed 2 years ago

frankystone commented 2 years ago

https://github.com/widelands/wl_addons_server/blob/c6bc3626d232177f8404fb4ee563e9650b28dfbe/wl/server/HandleCommand.java#L1042-L1043

Looks like renameTo() doesn't work if sourceDir and destinationDir are on different partitions, at least i have such an setup. The filesystems are equal, but on different partitions. /tmp is on /dev/sdc1/ (root-partition) and my testing environment for the add-ons-server is on /dev/sdc3 (home-partition):

$:> lsblk
NAME   LABEL         FSTYPE   SIZE FSAVAIL MOUNTPOINT
sdc                         447,1G                          
├─sdc1 arch_root     ext4      40G   18,7G /
├─sdc2 arch_swap     swap       4G         [SWAP]
├─sdc3 arch_home     ext4     100G   54,6G /home

After some testing this worked for me:

     boolean succ = tempDir.renameTo(addOnDir);
         if (!succ) {
             Utils.bash("mv", tempDir.getPath(), addOnDir.getAbsolutePath());
         }
    out.println("ENDOFSTREAM");

Is there a better way to get around the not working renameTo() function?

Noordfrees commented 2 years ago

From the doc:

Many aspects of the behavior of this method are inherently platform-dependent: The rename operation might not be able to move a file from one filesystem to another, it might not be atomic, and it might not succeed if a file with the destination abstract pathname already exists.

Files.move() might work instead, which would look something like:

Files.move(tempDir.ToPath(), addOnDir.toPath());

I don't like working with the Files API much, File is less powerful but much more intuitive to use…

frankystone commented 2 years ago

Nope:

[Sun Jan 30 17:24:23 CET 2022 @ Client#000001] ERROR: java.nio.file.DirectoryNotEmptyException: /tmp/14605112963691785347
java.nio.file.DirectoryNotEmptyException: /tmp/14605112963691785347

I have searched the web for a (java) solution and most comments says: Either use a third party library (like apache.commons.io) or write an own function which moves/copy a directory including subfolders and files to a new destination. I don't know...

Noordfrees commented 2 years ago

Including 3rd party stuff in Java is bad practice except for stuff that would be really hard to re-implement, but it should be easy to write a recursive move function similar to this recursive delete: https://github.com/widelands/wl_addons_server/blob/c6bc3626d232177f8404fb4ee563e9650b28dfbe/wl/server/ServerUtils.java#L338-L349 It'd only need some mkdir() calls to build the directory tree, Files.move for each regular file, and delete() to clean up afterwards. Quick sketch (untested):

    synchronized public static void doMove(File src, File target) throws Exception {
        if (src.isDirectory()) {
            target.mkdir();
            for (File file : src.listFiles()) {
                doMove(file, new File(target, file.getName()));
            }
            src.delete();
        } else {
            Files.move(src.toPath(), target.toPath());
        }
    }
doMove(tempDir, addOnDir);
frankystone commented 2 years ago

Works :+1: