linkTo seams to be broken

[scalway]

Current implementation of symbolicLinkTo and linkTo seams to be incorect.

//both functions do almost same thing (one is symbolic of course but... yea)
//this one'll return f
link.symbolicLinkTo(f) 
// and this one returns link
f.linkTo(link) 
//but both make link to shows on file f.

Current code:

def symbolicLinkTo(destination: File)(implicit attributes: File.Attributes = File.Attributes.default): destination.type = {
    Files.createSymbolicLink(path, destination.path, attributes: _*)  // <-- here
    destination
  }

  def linkTo(destination: File, symbolic: Boolean = false)(implicit attributes: File.Attributes = File.Attributes.default): destination.type = {
    if (symbolic) {
      symbolicLinkTo(destination)(attributes)
    } else {
      Files.createLink(destination.path, path) // <-- here
      destination
    }
  }

Java API looks like this.

//java.nio.file.Files
//--------------------------
public static Path createSymbolicLink(Path link, Path target, FileAttribute<?>... attrs)
public static Path createLink(Path link, Path existing)

In java path link is always first argument!, and in current implementation it is inconsistant.

[pathikrit]

Isn't this obvious from the types? The return types of both method return the destination.type ...

[scalway]

No... I was not precise enough... sorry.

Problem is when java API is called. symbolicLinkTo and linkTo creates links in totally different manner.
symbolicLinkTo assumes to be called on link object and target f needs to be passed as argument
linkTo needs to be called on target f file and takes link file as argument!

it means destination is different in both and it'll satisfy compiler contract in both cases.

link.symbolicLinkTo(f) 
// will call Files.createSymbolicLink(link, f)
// and return f

link.linkTo(f) //expected same api like in symbolicLinkTo
/** 
  will call Files.createLink(f, link)... ops
  and throw Error
  java.nio.file.NoSuchFileException: /tmp/491470459606103590ar-152-Swieta_Bozonarodzeniowe.ppt -> 
  /tmp/1058432223189661722
  sun.nio.fs.UnixException.translateToIOException(UnixException.java:86)
  sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:102)
  sun.nio.fs.UnixFileSystemProvider.createLink(UnixFileSystemProvider.java:476)
  java.nio.file.Files.createLink(Files.java:1086)
  better.files.File.linkTo(File.scala:674 
*/

f.linkTo(link)  
// will call  Files.createLink(link, f)
//and return link

f.symbolicLinkTo(link)
/** 
java.nio.file.FileAlreadyExistsException: /tmp/491470459606103590ar-152-Swieta_Bozonarodzeniowe.ppt
  sun.nio.fs.UnixException.translateToIOException(UnixException.java:88)
  sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:102)
  sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:107)
  sun.nio.fs.UnixFileSystemProvider.createSymbolicLink(UnixFileSystemProvider.java:457)
  java.nio.file.Files.createSymbolicLink(Files.java:1043)
  better.files.File.symbolicLinkTo(File.scala:668)

*/

To be honest it seams to be messed!
Problematic part is that someone could depends on this behavior already :(.
linkTo is not so often used and i thing it should be changed

[pathikrit]

I intentionally named these methods with to suffixes to make this clear e.g. I called it symbolicLinkTo instead of symbolicLink and linkTo instead of link. I think the problem is bad design on the Java API which I should have wrapped in a consistent manner in better-files but no way to fix now without breaking existing code.

[scalway]

java api is consistent here:
link path is always first param.
I propose to deprecate linkTo with appriopriate comment, and create def hardLinkTo().

Dsl.ln is stil problematic part (currently it is defined in terms of linkTo).

[pathikrit]

I still don't understand - can you send a quick PR of what you want?

[gblahaerath]

I got hit by this too. It seems like the intended behavior is to have File(newLinkPath).linkTo(existingFile) in both cases, but its actually swapped depending on the symbolic link flag. Right now its this:

File(existingFilePath).linkTo(newLinkFile, false)
File(newLInkPath).linkTo(existingFile, true)

It looks like an unintended argument swap on the Java method call more than anything, as it does seem pretty unexpected to have the source/dest change based on an optional flag. If you agree, let me know the intended order and I'll create a PR to fix it. If this is the intended behavior, I can add something to your docs or wiki that explains the working of the feature.

[pathikrit]

@gblahaerath : Yes intention is to have consistent File(source).linkTo(destination) in both cases. Can you send a PR please?