Skip to content
This repository was archived by the owner on Mar 24, 2026. It is now read-only.

Fix path for Dart executable in server channel#360

Open
debuggerx01 wants to merge 1 commit intoinvertase:mainfrom
debuggerx01:patch-1
Open

Fix path for Dart executable in server channel#360
debuggerx01 wants to merge 1 commit intoinvertase:mainfrom
debuggerx01:patch-1

Conversation

@debuggerx01
Copy link
Copy Markdown

Resolve: #357

@docs-page
Copy link
Copy Markdown

docs-page bot commented Oct 21, 2025

To view this pull requests documentation preview, visit the following URL:

docs.page/invertase/dart_custom_lint~360

Documentation is deployed and generated using docs.page.

@vercel
Copy link
Copy Markdown

vercel bot commented Oct 21, 2025

@debuggerx01 is attempting to deploy a commit to the Invertase Team on Vercel.

A member of the Team first needs to authorize it.

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Oct 21, 2025

CLA assistant check
All committers have signed the CLA.

@ryanhanks-bestow
Copy link
Copy Markdown

How were you running this locally?

@ryanhanks-bestow
Copy link
Copy Markdown

I applied this to the .pub_cache/ version my project is locked to and it worked

Sweet hack / fix!

@tsavo-at-pieces
Copy link
Copy Markdown

Code Quality Review

Changes

In packages/custom_lint/lib/src/v2/server_to_client_channel.dart, replaces the hardcoded 'dart' executable string with Platform.resolvedExecutable.replaceFirst('/dartaotruntime', '/dart') when spawning the custom_lint client process. This addresses issue #357 where the analysis server fails with "No such file or directory" when dart is not on the system PATH (common in FVM/monorepo setups where the SDK is resolved through tooling rather than PATH).

Quality Assessment

The problem is real and well-documented -- issue #357 has 14 thumbs-up and a clear reproduction involving FVM + Dart workspaces where the analysis server's environment does not have dart on PATH.

However, the fix has significant concerns:

  1. Brittle string replacement: Platform.resolvedExecutable.replaceFirst('/dartaotruntime', '/dart') relies on the internal implementation detail that Platform.resolvedExecutable returns a path ending in dartaotruntime when running inside the analysis server (AOT-compiled). This is an undocumented Dart VM behavior that could change between SDK versions without notice.

  2. Platform-specific path separator: The replacement uses /dartaotruntime with a forward slash, which will fail on Windows where paths use \. A cross-platform approach would use Platform.pathSeparator or package:path.

  3. No fallback: If Platform.resolvedExecutable does not contain dartaotruntime (e.g., running in JIT mode during development, or a future SDK restructuring), replaceFirst is a no-op and returns the original path, which may not be the dart binary. This is actually safe as a fallback if the original path is a valid Dart executable, but the semantics are unclear.

  4. Alternative approaches worth considering:

    • Use Platform.resolvedExecutable to find the SDK bin/ directory via dirname(Platform.resolvedExecutable) and then construct the path to dart from there. This would be more robust:
      final sdkBin = dirname(Platform.resolvedExecutable);
      final dartExe = join(sdkBin, 'dart');
    • Or use io.Process.resolvedExecutable patterns from the Dart SDK itself.
  5. User confirmation: A commenter confirmed the fix works by patching their local .pub_cache version, which is encouraging.

Verdict

NEEDS_WORK -- The core idea (use Platform.resolvedExecutable to find the Dart SDK instead of relying on PATH) is correct and solves a real problem. However, the replaceFirst('/dartaotruntime', '/dart') string manipulation is brittle and not cross-platform. Recommend refactoring to derive the dart executable path from the SDK bin/ directory using dirname(Platform.resolvedExecutable) and join(sdkBin, 'dart'), which is both more robust and platform-agnostic.

@tsavo-at-pieces
Copy link
Copy Markdown

Fork Maintainer Review (dart_custom_lint)

Reviewing on behalf of open-runtime/dart_custom_lint fork.

Summary

Fixes the Dart executable path resolution in server_to_client_channel.dart. Changes 'dart' to Platform.resolvedExecutable.replaceFirst('/dartaotruntime', '/dart') when spawning the custom_lint client process. This resolves issue #357 where analysis.setContextRoots fails with "No such file or directory" in certain environments.

Upstream Status

Open on upstream since Oct 21, 2025 (4+ months). Not yet merged. Community contribution by @debuggerx01. References upstream issue #357 which describes failures in monorepo + FVM + Dart workspace setups.

Assessment

The fix addresses a real problem but the approach has concerns:

Problem it solves: When the analyzer plugin runs, it uses 'dart' as the executable name, relying on PATH resolution. In environments with FVM, version managers, or non-standard Dart installations, dart may not be on PATH or may resolve to the wrong version. Platform.resolvedExecutable gives the absolute path to the currently running Dart binary.

Concerns:

  1. replaceFirst('/dartaotruntime', '/dart') is fragile. It assumes a specific binary layout where dartaotruntime and dart are siblings in the same directory. This is true for standard Dart SDK installations but could break with custom layouts.
  2. No fallback. If Platform.resolvedExecutable does not contain /dartaotruntime, the replaceFirst is a no-op and the full resolved path is used, which should be fine. But the string replacement approach is still brittle.
  3. This is a 1-line change with no tests. The underlying issue (analysis.setContextRoots failed with the following error: No such file or directory #357) has 6 comments and multiple affected users, suggesting this is a real pain point.

Alternative approaches seen in other projects:

  • Using Platform.resolvedExecutable directly (when the executable IS dart)
  • Using dart:io's Process.resolvedExecutable with a lookup table
  • Finding the SDK root from Platform.resolvedExecutable and constructing the path

Despite the fragility concern, this pattern (resolvedExecutable.replaceFirst('/dartaotruntime', '/dart')) is actually used in several Dart ecosystem tools and is the pragmatic solution.

Recommendation

SYNC_FROM_UPSTREAM (once merged) -- This fixes a real issue for FVM and non-standard Dart installations. The approach is pragmatic and commonly used in the Dart ecosystem. Wait for upstream to merge. If upstream remains stale, consider cherry-picking since our fork targets Dart 3.9+ environments where this path resolution matters.

@tsavo-at-pieces
Copy link
Copy Markdown

Compatibility Review (Monorepo Impact)

Reviewer context: We maintain the open-runtime/dart_custom_lint fork, consumed as a workspace member in our Dart monorepo (aot_monorepo). Our fork currently has 'dart' as the executable in server_to_client_channel.dart (line 156).

Monorepo Impact

MEDIUM RISK -- NEEDS CAREFUL EVALUATION. This PR changes the Dart executable resolution from the bare 'dart' string (PATH lookup) to Platform.resolvedExecutable.replaceFirst('/dartaotruntime', '/dart').

The problem it solves is real: When custom_lint runs as an analyzer plugin inside an IDE (IntelliJ, VS Code), the process may be spawned via dartaotruntime (the AOT-compiled Dart runtime) rather than the full dart CLI. Since dartaotruntime is not the dart CLI, calling Process.start('dart', ...) can fail with "No such file or directory" if dart is not on the PATH in that context. Issue #357 documents this clearly.

Concerns for our monorepo:

  1. Platform dependency: Platform.resolvedExecutable returns the path of the currently running Dart executable. In our CI/CD pipelines, this is typically the dart binary from the Dart SDK. The replaceFirst('/dartaotruntime', '/dart') substitution is a heuristic -- it assumes the dart CLI binary is always a sibling of dartaotruntime in the same directory. This is true for standard Dart SDK installations but could break with non-standard SDK layouts (e.g., FVM, custom SDK builds, or containerized environments).

  2. Path separator: The PR uses a hardcoded forward-slash /dartaotruntime and /dart. On Windows, Platform.resolvedExecutable uses backslashes. This means the replaceFirst would fail to match on Windows, falling back to the original Platform.resolvedExecutable (which would be dartaotruntime.exe), potentially making things worse on Windows than the current bare 'dart' approach.

  3. Our usage context: In our monorepo, we run custom_lint via dart run custom_lint from the command line and via IDE integration. The IDE integration case is exactly where this fix matters. The command-line case should be unaffected since Platform.resolvedExecutable would already be the dart binary.

  4. Alternative approaches: A more robust solution might be to use Platform.resolvedExecutable and walk up to find the SDK bin/ directory, or use the cli_util package's getSdkPath() which is already a dependency.

Dependency Concerns

None. No new dependencies. Single line change in server_to_client_channel.dart. Uses only dart:io (Platform).

Recommendation

MERGE WITH CAVEAT -- The fix addresses a real and documented issue (#357) that affects IDE integration. While the replaceFirst heuristic is somewhat fragile (especially on Windows), it is strictly better than the current bare 'dart' approach for the primary failure case (IDE spawning via dartaotruntime). The fallback behavior when the replace does not match is using the full resolved executable path, which is reasonable.

For our fork, we should consider applying this fix. However, a follow-up PR should investigate a more robust approach using cli_util's getSdkPath() to locate the SDK bin/dart reliably across all platforms and SDK installation methods.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

analysis.setContextRoots failed with the following error: No such file or directory

4 participants