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

Provide a canonical way to find the Dart SDK root #16994

Closed
blois opened this issue Feb 20, 2014 · 19 comments
Closed

Provide a canonical way to find the Dart SDK root #16994

blois opened this issue Feb 20, 2014 · 19 comments
Assignees
Labels
area-pkg Used for miscellaneous pkg/ packages not associated with specific area- teams. P1 A high priority bug; for example, a single project is unusable or has many test failures type-enhancement A request for a change that isn't a bug
Milestone

Comments

@blois
Copy link
Contributor

blois commented Feb 20, 2014

In the development of pub transformers which resolve Dart source code, in order to resolve all of the code the Analyzer needs a path to the Dart SDK filter (in order to find the sources for dart: source code).

Unfortunately there does not seem to be a good way to find the SDK folder for the current Dart executable. I do have a user-configurable path, but this falls down when the code is used in unit-tests, transformers and other portable locations (location of SDK can differ across machines and test configurations).

Additional consideration- it might be good to look at something like .NET's reference assemblies which has a fixed root folder containing all versions installed, providing a structured environment for side-by-side installation of runtimes.

@blois
Copy link
Contributor Author

blois commented Feb 21, 2014

Konstantin/Rico- I'm wondering if there are some tricks that I'm missing for how to do this. Is there code elsewhere either for Analyzer tests or for other tests which finds the Dart SDK folder?

I just seem to be running into many different configurations which is breaking the detection.


cc @scheglov.
cc @ricowind.

@scheglov
Copy link
Contributor

Yes, had this question for months.
Currently I use some hackish implementation in pkg/analyzer/lib/src/generated/java_io.dart JavaSystemIO.getProperty().

@lrhn
Copy link
Member

lrhn commented May 5, 2014

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

@kevmoo
Copy link
Member

kevmoo commented May 14, 2014

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

@DartBot
Copy link

DartBot commented Dec 6, 2014

This comment was originally written by @zoechi


See also http://dartbug.com/21225

@sigmundch
Copy link
Member

Issue #21783 has been merged into this issue.

@sigmundch
Copy link
Member

Søren/Anders - any ideas if this is something we can support in dart:io?


cc @sgjesse.
cc @skabet.

@sgjesse
Copy link
Contributor

sgjesse commented Dec 9, 2014

I don't think this is something which fits into dart:io. dart:io has no awareness of a Dart SDK, it is part of the standalone dart executable. So adding specific logic trying to "guess" the SDK location that it might be part of using a number of probes is not well suited for dart:io.

Looking at the code in issue #21783, why is there not an initial check for the DART_SDK environment variable? Maybe requiring DART_SDK to be set and failing otherwise is a better approach.


Removed Type-Defect, Library-IO labels.
Added Type-Enhancement label.

@DartBot
Copy link

DartBot commented Dec 9, 2014

This comment was originally written by @zoechi


DART_SDK seems not to be the preferred way of finding the SDK (see http://dartbug.com/14967#c4, http://dartbug.com/15019)

If there is code in pub (issue #15019) that can reliably get the SDK directory, this should be made available to others and I still think dart:io (Platform) is the best place but I'm fine with other packages too. At least code for this task should exist only in one place.

That there is no simple and official way to do this, caused/causes troubles with every package that needs access to the SDK.

@sigmundch
Copy link
Member

I agree that this should be part of dart:io. Note that I don't think the logic should be guessed by dart:io, I believe that because dart:io and the VM are built as part of the SDK, they should encode what is their relative position to the sdk sources (this might be a simple string added by our build bots, similar to how we attach the SDK version number)

Then it should be possible to derive the exact location combining that with the location of the dart executable (resolving any symlinks first, as it's common for some installations of dart to add this level of indirection)

Thoughts?

@lrhn
Copy link
Member

lrhn commented Dec 10, 2014

The VM might not have an SDK.
You can copy the standalone VM executable to anywhere, and run it with no access to any SDK.
That means the VM and the dart:io library are independent of any SDK directory even existing, and there is no canonical relation between the standalone VM executable and an SDK directory.

@pq
Copy link
Member

pq commented Dec 10, 2014

Another interested party here. Every service that builds on the analyzer is repeating and perpetuating these hacks. (And I'm about to embark on another!)

+1 to Siggi's points on baking in at build time rather than guessing as well.

As for the VM not having an SDK, I get that this might be a little bothersome from an API hygiene perspective but in practice I don't think this will detract from the value added.

@DartBot
Copy link

DartBot commented Dec 10, 2014

This comment was originally written by munifice...@gmail.com


In the development of pub transformers which resolve Dart source code, in order to resolve all of the code the Analyzer needs a path to the Dart SDK filter (in order to find the sources for dart: source code).

The original problem is about accessing the "dart:" sources. The proposed solution of locating the SDK seems like it's just one way to solve that problem.

If that's the only thing we need the SDK directory for, maybe we should just have an API for accessing "dart:" sources directly?

@sigmundch
Copy link
Member

Lasse - you bring a very good point, but maybe there is an option in the middle, for example, could we provide a way to detect when the executable lives in the default SDK location?

One possible way to address this would be an API as follows:

class Platform {
  ...

  /// The fully resolved path to the Dart executable. Similar to [executable], except
  /// that we provide an absolute path and resolve symlinks to reach the actual binary file.
  String executablePath;

  /// Returns whether [executablePath] is in the default SDK location. This will be true
  /// for the dart binary that is shipped with the SDK, but false if the dart binary
  /// was moved or copied somewhere else.
  bool get isExecutableInDefaultSdkLocation => ...;

  /// The path to the SDK home relative from the default location of the Dart binary in
  /// the SDK. This would be a relative path from [executablePath] when
  /// [isExecutableInDefaultSdkLocation] is true.
  static const String pathToSdkHomeFromDefaultDartLocation = ...;

}

There are actually many combinations of APIs we can come up with, but let me explain what are the issues behind we are trying to address:
 (a) it's hard to determine where the Dart executable is. Today if you have dart in your path, the Platform.executable returns "dart" and not a path to the actual executable. A lot of the adhoc logic when we are figuring out the sdk path is because we can't tell where the Dart binary is. Adding [executablePath] can address this.

 (b) it's adhoc to determine what is the root of the SDK, even when the Dart binary is inside the SDK. Today, we guess it by searching for an internal SDK file lib/_internal/libraries.dart, but that can change with time. The idea behind [pathToSdkHomeFromDefaultDartLocation] is that we shouldn't need adhoc checks to know where the home of the SDK should be.

 (c) we can't control that the vm will be inside the SDK default location. This is basically your concern, I propose that we just provide a way to detect this (isExecutableInDefaultSdkLocation).

Thoughts?

@sgjesse
Copy link
Contributor

sgjesse commented Dec 17, 2014

This is something which will fit nicely in a package. dart:io should already have all the API needed. Running

main() => new File(Platform.executable).resolveSymbolicLinks().then(print);

prints the path to the Dart executable.

@sethladd
Copy link
Contributor

Does this work when that code is served up via a transformer?

@blois blois added Type-Enhancement P1 A high priority bug; for example, a single project is unusable or has many test failures area-pkg Used for miscellaneous pkg/ packages not associated with specific area- teams. labels Dec 17, 2014
@blois blois added this to the 1.11 milestone Dec 17, 2014
nex3 referenced this issue in dart-lang/test Jun 9, 2015
Platform.executable doesn't include the ".exe" extension on Windows,
which causes symlink resolution for it to fail.

Closes #133

R=kevmoo@google.com

Review URL: https://codereview.chromium.org//1165313002.
sgjesse added a commit that referenced this issue Jun 15, 2015
The change to Platform.excutable in
e03ab17 was a breaking change and it
has been reverted.

A new getter Platform.resolvedExecutable has been added to provide the
the fully qualified path of the executable.

BUG=#16994
R=lrn@google.com, kustermann@google.com, len@google.com

Review URL: https://codereview.chromium.org//1180623006.
sgjesse added a commit that referenced this issue Jun 15, 2015
The change to Platform.excutable in
e03ab17 was a breaking change and it
has been reverted.

A new getter Platform.resolvedExecutable has been added to provide the
the fully qualified path of the executable.

BUG=#16994
R=lrn@google.com, kustermann@google.com, len@google.com

Review URL: https://codereview.chromium.org//1180623006.
@sgjesse
Copy link
Contributor

sgjesse commented Jun 16, 2015

After a number of different changes we have decided the following:

  • Platform.executable will not change from its original behaviour
  • A new getter Platform.resolvedExecutable is added. This returns the absolute path, without any symlinks, to the actual executable as resolved by the OS.

This landed on master, c05c8c6, and will go into 1.11, where is now in 1.11.0-dev.5.3, c0e90cb.

@sgjesse sgjesse closed this as completed Jun 16, 2015
@sethladd
Copy link
Contributor

Thanks for the update!

@kevmoo kevmoo added type-enhancement A request for a change that isn't a bug and removed triaged labels Mar 1, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-pkg Used for miscellaneous pkg/ packages not associated with specific area- teams. P1 A high priority bug; for example, a single project is unusable or has many test failures type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

9 participants