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

Unclear specification of File's fullPath, fullPathSync, and name #1016

Closed
peter-ahe-google opened this issue Jan 2, 2012 · 17 comments
Closed
Assignees
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-io
Milestone

Comments

@peter-ahe-google
Copy link
Contributor

The File interface contains the following:

  /**
   * Get the canonical full path corresponding to the file name. The
   * [fullPathHandler] is called when the fullPath operation
   * completes.
   */
  String fullPath();

  /**
   * Synchronously get the canonical full path corresponding to the file name.
   */
  String fullPathSync();

  /**
   * Get the name of the file.
   */
  String get name();

This leads to the following questions:

  1. What is returned by fullPath?
  2. Does the returned valued of fullPathSync use platform dependent file delimiters?
  3. What does "canonical" mean? Generally, the concept of canonical file names is broken. In both Windows and Unix, you can refer to the "same" file using different paths without having the ability to say one is the canonical name. If the intent is to remove symlinks, please reconsider. Removing symlinks from file paths is generally harmful as it is a common idiom in Unix system administration to setup links to hide "implementation details" such as specific disk that stores a file.
  4. What is the name of a file?

I suggest the following:

  /**
   * Get the absolute(define this) normalized(define this) path corresponding to this file. The
   * [fullPathHandler] is called with one argument, the absolute normalized path, when the fullPath operation
   * completes.
   */
  void fullPath();

  /**
   * Synchronously get the absolute(define this) normalized(define this) path corresponding to this file.
   */
  String fullPathSync();

  /**
   * Get the path of this file as was provided when this file object was created.
   */
  String get name();

However, I question the need for an async version of fullPath. This path should be known at construction time without calling any blocking IO.

Furthermore, the File interface lacks a comment which describes its purpose. Finally, the documentation of the constructor is redundant.

@whesse
Copy link
Contributor

whesse commented Jan 2, 2012

The key reason for introducing fullPath was to get absolute paths from relative paths. The idea is to use the OS's notion of a full path by using the platform's full path method. I'll look into all these specification issues.


Set owner to @whesse.
Added Area-Library, Accepted labels.

@peter-ahe-google
Copy link
Contributor Author

On Unix, what is the platform's full-path method?

@peter-ahe-google
Copy link
Contributor Author

My comment #­2 reminds me: what happens if the full path isn't available? I think it is possible to be running in a directory that you cannot read. See the error codes of getcwd(2).

@whesse
Copy link
Contributor

whesse commented Jan 19, 2012

We are using the realpath() method on unix, which does resolve symbolic links, and can return failures due to access violations or other problems. I don't see a reasonable alternative that does everything except resolve symbolic links. The alternative would be to just combine the CWD with a relative path, and resolve . and .. using the Join method of a Path library (following the semantics of URI path components). This doesn't do IO or use the operating system at all.

Do you think there are other possibilities? Otherwise, which of these two do you think should be the semantics of fullPath?

If we do go to the OS, then we should throw an exception, or call an error handler, with the system error message corresponding to any actual error code we get.

@peter-ahe-google
Copy link
Contributor Author

I prefer combining CWD.

@DartBot
Copy link

DartBot commented Aug 28, 2012

This comment was originally written by jjinux...@google.com


By the way, I had a hard time finding fullPath because it didn't have the word "absolute" in it, and because it was only present in File, not in Directory (I happened to be working with directories at the time).

@floitschG
Copy link
Contributor

Removed Area-Library label.
Added Area-IO label.

@whesse
Copy link
Contributor

whesse commented Mar 14, 2013

We want to make a clear specification of what fullPath does, or replace it with other methods. We probably should have a method that gets absolute paths, with an appropriate name, and a method that resolves symbolic links and .. directories, with an appropriate name.

These methods should be available on all of File, Directory, Link, and maybe FileSystemEntity. Redirecting bugs about "Directory missing FullPath" here.

@whesse
Copy link
Contributor

whesse commented Mar 14, 2013

Issue #8318 has been merged into this issue.

@whesse
Copy link
Contributor

whesse commented Mar 14, 2013

Issue #8992 has been merged into this issue.

@sgjesse
Copy link
Contributor

sgjesse commented Apr 16, 2013

Added this to the M5 milestone.

@sgjesse
Copy link
Contributor

sgjesse commented May 28, 2013

Removed this from the M5 milestone.
Added this to the M6 milestone.

@sgjesse
Copy link
Contributor

sgjesse commented Jul 23, 2013

This should move to FileSystemEntity. "full" should be renamed to "absolute" and the comment updated to say that this is the absolute path provided by the OS.

@larsbak
Copy link

larsbak commented Aug 28, 2013

Removed this from the M6 milestone.
Added this to the M7 milestone.

@whesse
Copy link
Contributor

whesse commented Sep 4, 2013

This is being moved to FileSystemEntity.resolveSymbolicLinks, and a different FileSystemEntity.absolutePath method will be added, that will simply join the current working directory with relative paths, without accessing the filesystem.

We think that people looking for an absolutePath method want the second behavior, because the resolveSymbolicLinks behavior gives unexpected results and can be a performance bottleneck.

The path package also provides absolutePath, but we need to provide it on the classes File, Directory, and Link so that people can find it, and don't use the former fullPath method instead.


Added Started label.

@whesse
Copy link
Contributor

whesse commented Sep 18, 2013

Fixed in r27612. absoutePath and resolveSymbolicLinks have both been added to FileSystemEntity, with full documentation and testing.


Added Fixed label.

@kevmoo
Copy link
Member

kevmoo commented May 14, 2014

Removed Area-IO label.
Added Area-Library, Library-IO labels.

@peter-ahe-google peter-ahe-google added Type-Defect area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-io labels May 14, 2014
@peter-ahe-google peter-ahe-google added this to the M7 milestone May 14, 2014
copybara-service bot pushed a commit that referenced this issue Sep 14, 2023
Revisions updated by `dart tools/rev_sdk_deps.dart`.

ecosystem (https://github.com/dart-lang/ecosystem/compare/e96fbdb..babf5d1):
  babf5d1  2023-09-13  Devon Carew  add additional lints to dart_flutter_team_lints (#167)
  7740bef  2023-09-13  Moritz  Write comments on forks for `firehose` (#165)

http (https://github.com/dart-lang/http/compare/de19214..e19094a):
  e19094a  2023-09-14  Brian Quinlan  Use efficient operations when copying bytes between Dart and Java (#1019)
  d7e4375  2023-09-13  Parker Lougheed  Cleanup `package:http` utils (#1011)
  eafbbb0  2023-09-12  Brian Quinlan  Separate the cronet callbacks from the `send` method (#1017)
  2cbb703  2023-09-12  Brian Quinlan  Switch `cronet_http` to use jnigen (#1016)

native (https://github.com/dart-lang/native/compare/bbcbc1f..7faf62c):
  7faf62c  2023-09-14  Gabriel Terwesten  Add `includes`, `flags`, `std`, `language`, `cppLinkStdLib` options (#125)

shelf (https://github.com/dart-lang/shelf/compare/2926f76..e2a02b7):
  e2a02b7  2023-09-13  Kevin Moore  Move to latest pkg:dart_flutter_team_lints, bump min sdk to Dart 3 (#378)

tools (https://github.com/dart-lang/tools/compare/fa01f9b..1512f3d):
  1512f3d  2023-09-13  Elias Yishak  Add Fake Analytics instance that uses list to save events sent (#149)

webdev (https://github.com/dart-lang/webdev/compare/6b21ecf..501ccc2):
  501ccc28  2023-09-12  Elliott Brooks  Update DCM triggers to match Dart DevTools (#2230)

Change-Id: Ic3dc1924da48454a28e4c0d8705244ac1565e74a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/325967
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Devon Carew <devoncarew@google.com>
This issue was closed.
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
Projects
None yet
Development

No branches or pull requests

7 participants