Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Type of a Link created on Windows can be changed #53684

Closed
sgrekhov opened this issue Oct 4, 2023 · 11 comments
Closed

Type of a Link created on Windows can be changed #53684

sgrekhov opened this issue Oct 4, 2023 · 11 comments
Assignees
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-io type-documentation A request to add or improve documentation

Comments

@sgrekhov
Copy link
Contributor

sgrekhov commented Oct 4, 2023

According to the Link.create() documentation

sdk/sdk/lib/io/link.dart

Lines 50 to 54 in 24ec82e

/// On the Windows platform, this call will create a true symbolic link
/// instead of a junction. The link represents a file or directory and
/// does not change its type after creation. If [target] exists then
/// the type of the link will match the type [target], otherwise a file
/// symlink is created.

link created by Link.create() cannot change it's type. What exactly does not change its type after creation mean? I understand it as Directory link cannot be turned into File link and vice versa. But it isn't so. Proof:

import "dart:io";

main() {
  Directory sandbox = Directory.systemTemp.createTempSync();
  try {
    Directory target1 = sandbox.createTempSync();
    File target2 = new File(sandbox.path + Platform.pathSeparator + "file.tmp");
    target2.createSync();
    // Create link to a Directory
    Link link = new Link(sandbox.path + Platform.pathSeparator + "link.tmp");
    link.createSync(target1.path);
    print(FileSystemEntity.typeSync(link.path)); // directory
    // Change target of the link to a file
    link.updateSync(target2.path);
    print(FileSystemEntity.typeSync(link.path)); // file
  } finally {
    sandbox.deleteSync(recursive: true);
  }
}

cc @brianquinlan

Tested on Dart SDK version: 3.2.0-220.0.dev (dev) (Sat Sep 30 13:02:52 2023 -0700) on "windows_x64" on Windows 11 Pro

@sgrekhov sgrekhov added area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-io labels Oct 4, 2023
@lrhn
Copy link
Member

lrhn commented Oct 4, 2023

I think, only from reading the linked doc, that a Windows symbolic link inherently knows whether it points to a file or directory, and the symbolic link won't change that even if the target is removed or replaced by something else.
The type of link created by the method is that of what it's target currently is, or a file link if there is no entry at the target path.

I have no idea what the effect is of following a file link to a directory, or vice versa.

Which is (probably) why the (woefully under-documented) updateSync exists, to re-create the link with the type of what its path now points to.

The link doesn't change, but you can change it explicitly, which probably amounts to overwriting it with a new symbolic link.

@sgrekhov
Copy link
Contributor Author

sgrekhov commented Oct 5, 2023

Let's try to create a link pointing to a directory, then delete the target and create a file with the same name/path and to see the link's type

import "dart:io";

main() {
  Directory sandbox = Directory.systemTemp.createTempSync();
  try {
    Directory target1 = sandbox.createTempSync();
    String path = target1.path;
    // Create link to a Directory
    Link link = new Link(sandbox.path + Platform.pathSeparator + "link.tmp");
    link.createSync(target1.path);
    print(FileSystemEntity.typeSync(link.path)); // Windows: directory, Linux: directory
    print(FileSystemEntity.typeSync(link.path, followLinks: false)); // Windows:link , Linux: link
    // Delete those Directory and create a File with the same path
    target1.deleteSync();
    print(target1.existsSync()); // false
    print(FileSystemEntity.typeSync(link.path)); // Windows: link, Linux: notFound
    print(FileSystemEntity.typeSync(link.path, followLinks: false)); // Windows: link, Linux: link
    File target2 = File(path);
    target2.createSync();
    print(target2.existsSync()); // true
    print(FileSystemEntity.typeSync(link.path)); // Windows: link, Linux: file
    print(FileSystemEntity.typeSync(link.path, followLinks: false)); // Windows: link, Linux: link
  } finally {
    sandbox.deleteSync(recursive: true);
  }
}

I'd say that Linux behaves exactly like I'd intuitive expect. As for Windows it seems that actual behaviour contradicts at least two documentation statement

  • Link.create(): "The link represents a file or directory and does not change its type after creation"
  • FileSystemEntity.typeSync(): "Returns FileSystemEntityType.link only if followLinks is false and if path points to a link.". In the example above the path points to a link, but followLinks has default value true and FileSystemEntity.typeSync() returns FileSystemEntityType.link anyway.

Even if it's not a bug but expected behavior we should have more detailed documentation

@sgrekhov
Copy link
Contributor Author

sgrekhov commented Oct 5, 2023

And one more Windows (documentation?) issue.

If target exists then the type of the link will match the type target, otherwise a file symlink is created.

On Windows type of the link, pointing to a not existing entity is link, not file (notFound on Linux)

import "dart:io";

main() {
  Directory sandbox = Directory.systemTemp.createTempSync();
  try {
    Directory target1 = sandbox.createTempSync();
    target1.deleteSync();
    Link link = new Link(sandbox.path + Platform.pathSeparator + "link.tmp");
    link.createSync(target1.path);
    print(FileSystemEntity.typeSync(link.path)); // Windows: link, Linux: notFound
    print(FileSystemEntity.typeSync(link.path, followLinks: false)); // Windows: link, Linux: link
  } finally {
    sandbox.deleteSync(recursive: true);
  }
}

@lrhn
Copy link
Member

lrhn commented Oct 5, 2023

On Windows type of the link, pointing to a not existing entity is link, not file (notFound on Linux)

That can still be a file link with no target. The behavior seen her it's consistent with a file or directory link with no valid target reporting itself as a link.

What happens if one creates either a file or directory at the target of the link?

I guess, somewhat blindly, that it will start reporting itself as a file link if it points to a file, and still as a link if it points to a director, because it's a file link.

Could be. Would be consistent with the documentation. The documentation could say more, though. It's easy to be consistent with something vague.

@sgrekhov
Copy link
Contributor Author

sgrekhov commented Oct 5, 2023

What happens if one creates either a file or directory at the target of the link?

I guess, somewhat blindly, that it will start reporting itself as a file link if it points to a file, and still as a link if it points to a director, because it's a file link.

It's not possible to create "file/directory" only link. Link target is a String containing absolutely or relative path to some entity which can be both a directory or a file.

So, if you create a link pointing to nowhere, then, on Linux it'll have type notFound. After that you can create a Directory at the target of the link. It's type will became directory. Then delete it and create a file with the same name. It's type will became file. On Windows if we create a file at the link's target it'll became a file type link. If we create a directory it stays link

import "dart:io";

main() {
  Directory sandbox = Directory.systemTemp.createTempSync();
  try {
    String notExisting = "something";
    Link link = Link(sandbox.path + Platform.pathSeparator + "link");
    String path = sandbox.path + Platform.pathSeparator + notExisting;
    link.createSync(path);
    print(FileSystemEntity.typeSync(link.path)); // Linux: notFound, Windows: link
    File file = File(path);
    file.createSync();
    print(FileSystemEntity.typeSync(link.path)); // Linux: file, Windows: file
    file.deleteSync();
    print(FileSystemEntity.typeSync(link.path)); // Linux: notFound, Windows: link
    Directory dir = Directory(path);
    dir.createSync();
    print(FileSystemEntity.typeSync(link.path)); // Linux: directory, Windows: link
  } finally {
    sandbox.deleteSync(recursive: true);
  }
}

@lrhn
Copy link
Member

lrhn commented Oct 5, 2023

That still looks consistent.

It's not possible to create "file/directory" only link. Link target is a String containing absolutely or relative path to some entity which can be both a directory or a file.

That's apparently not true on Windows. Every link you create is either a file link or a directory link.
The Dart API doesn't allow you to choose, so it picks the type of the thing it currently points to, and if there isn't anything at the target, it picks file.

A file link reports its transitive (followLinks: true) type as file if its "valid" (target is a file, or target is a valid file link), otherwise it reports its type as link. If it points to a directory, it's not valid and reports its type as link.

Symmetrically, a directory link reports its transitive type as directory if its valid, its target is a directory, or a valid directory link, otherwise it reports it as link.

That matches the prints in code above:

  1. File link, no target: type is link.
  2. File link, file target: type is file.
  3. File link, no target: type is link
  4. File link, directory target: type is link.

Try this:

import "dart:io";

void main() async {
  var tmp = Directory.systemTemp.createTempSync("winlink");
  try {
    var file = File(join(tmp.path, "file"));
    file.writeAsStringSync("banana");
    var dir = Directory(join(tmp.path, "dir"));
    dir.createSync();
    var fileLink = Link(join(tmp.path, "file-link"));
    fileLink.createSync(file.path);
    var dirLink = Link(join(tmp.path, "dir-link"));
    dirLink.createSync(dir.path);

    // File link, file target: type file
    log(fileLink); // ...\file-link:link-->  ...\file:file (expected file)
    // Dir link, dir target: type dir
    log(dirLink); // ...\dir-link:link-->  ...\dir:directory (expected directory)

    await (Future.delayed(Duration(milliseconds: 250)));
    file.deleteSync();
    dir.deleteSync(recursive: true);

    {
      var newFile = File(dir.path);
      newFile.createSync();

      var newDir = Directory(file.path);
      newDir.createSync();

      // File link, dir target: type link, and cannot resolve
      log(fileLink); // ...\file-link:link-->  no target:notFound (expected link)
      // Dir link, fil target: type link, and cannot resolve
      log(dirLink); // ...\dir-link:link-->  no target:notFound (expected link)

      await (Future.delayed(Duration(milliseconds: 250)));

      newFile.deleteSync();
      newDir.deleteSync();
    }

    {
      var newFile = File(dir.path);
      newFile.createSync();
      var newLink = Link(file.path);
      newLink.createSync(newFile.path);
      // File link, valid file link target: type file
      log(fileLink); // ...\file-link:link-->  ...\dir:file (expected file)
      // File link, file target: type file
      log(newLink); // ...\file:link-->  ...\dir:file (expected file)
      await (Future.delayed(Duration(milliseconds: 250)));
      newFile.deleteSync();
      newLink.deleteSync();
    }
  } finally {
    await (Future.delayed(Duration(milliseconds: 250)));

    tmp.deleteSync(recursive: true);
  }
}

String join(String dir, String name) =>
    [dir, name].join(Platform.pathSeparator);

void log(Link fse) {
  var exists = fse.existsSync();
  assert(exists);
  var transitiveType = FileSystemEntity.typeSync(fse.path, followLinks: true);
  var directType = FileSystemEntity.typeSync(fse.path, followLinks: false);
  //assert(directType == stat.type);
  var target = "no target";
  var targetType = FileSystemEntityType.notFound;
  if (transitiveType == FileSystemEntityType.file ||
      transitiveType == FileSystemEntityType.directory) {
    target = fse.resolveSymbolicLinksSync();
    targetType = FileSystemEntity.isFileSync(target)
        ? FileSystemEntityType.file
        : FileSystemEntity.isDirectorySync(target)
            ? FileSystemEntityType.directory
            : FileSystemEntity.isLinkSync(target)
                ? FileSystemEntityType.link
                : FileSystemEntityType.notFound;
  }
  print("${fse.path}:$directType"
      "${directType == FileSystemEntityType.link ? //
          "-->  $target:$targetType" : ""}"
      " (expected $transitiveType)");
}

I think the documentation is technically correct, it's just that where it uses the word "type" in this paragraph, it really means "Windows link supported target type", not FileSystemEntityType.
That's confusingly ambiguous and should be fixed.

@sgrekhov
Copy link
Contributor Author

sgrekhov commented Oct 6, 2023

@lrhn thank you for your assistance! Let me try to summarize the above.

  1. The current behaviour around Link type is correct, no changes in API in this area needed but documentation
  2. Requested documentation changes:

2.1. Link.update/updateSynk(): should clearly state that it deletes the link and creates the new one

2.2. FileSystemEntity.type/typeSync():

Returns FileSystemEntityType.link only if followLinks is false and if path points to a link.

Add that on Windows it returns FileSystemEntityType.link on any Link with "broken" target when followLinks is true. "Broken" target means a link with not existing target or file link pointing to a directory and vice versa, or, transitively, link to a link with "broken" target.

Returns FileSystemEntityType.notFound if path does not point to a file system object or if any other error occurs in looking up the path.

Add that on non-Windows systems it returns FileSystemEntityType.notFound if followLink is true and path points to a Link with a target to nowhere

2.3. Link.create/createSync()

On the Windows platform,... The link represents a file or directory and does not change its type after creation.

Need to rewrite this statement in more details. Link, created with a directory as a target can point to directories only (or orher links pointing to a directories). Link, created with a target file/nowhere/link to file/link to nowhere cannot point to a directory/link to directory

2.4. Link.create/createSync(): if Windows relative links have any restrictions it make sense to describe them here as well. See #53689

2.5. Link.resolveSymbolicLinks/resolveSymbolicLinksSync(): add that on Windows if a file-link points to a directory or a dir-link points to a file, then FileSystemException is thrown even if the target exists

Did I miss anything?

@lrhn
Copy link
Member

lrhn commented Oct 6, 2023

Agree that docs need improvement.
The listed items seem to cover everything discussed here, and match how I (now) understand Windows behavior.

@brianquinlan
Copy link
Contributor

Sorry for jumping in late - I wrote the text "The link represents a file or directory and does not change its type after creation."

The meaning of that is Windows represents links-to-files and links-to-directories differently. For example, to delete a link-to-file, you'd use DeleteFile but for a link-to-directory, you'd use RemoveDirectory.

I'll work on documentation fixes.

@brianquinlan
Copy link
Contributor

brianquinlan commented Oct 23, 2023

Stuff to do:

  • Link.update/updateSynk(): should clearly state that it deletes the link and creates the new one
  • FileSystemEntity.type/typeSync():
  • Link.create/createSync()
  • Link.resolveSymbolicLinks/resolveSymbolicLinksSync()

@brianquinlan
Copy link
Contributor

I have a PR that fixes these issues (https://dart-review.googlesource.com/c/sdk/+/331841) except for the issues related to #53689, which I will fix separately.

Instead of documenting the difference in FileSystemEntity.type behavior between Windows and other platforms, I modified the Windows behavior so that this test passes on all platforms:

testBrokenLinkTypeSync() {
  String base = Directory.systemTemp.createTempSync('dart_link').path;
  String link = join(base, 'link');

  Link(link).createSync('does not exist');

  Expect.equals(FileSystemEntityType.link,
      FileSystemEntity.typeSync(link, followLinks: false));

  Expect.equals(FileSystemEntityType.notFound,
      FileSystemEntity.typeSync(link, followLinks: true));
}

Several co19 tests must be updated before this PR can land without breakage:
new_test_failures__logs_.txt

copybara-service bot pushed a commit that referenced this issue Oct 25, 2023
…resolving a link that points to a non-existent file results in a type of `notFound`, which is consistent with all other platforms.

Bug:#53684
Change-Id: I1b594e1a85906d1f510358eec71792ea15ab801b
CoreLibraryReviewExempt: VM- and doc-only
Tested: unit tests
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/331841
Reviewed-by: Lasse Nielsen <lrn@google.com>
Commit-Queue: Brian Quinlan <bquinlan@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-io type-documentation A request to add or improve documentation
Projects
None yet
Development

No branches or pull requests

3 participants