This package uses knowledge about internals of the Dart sdk layout, location of snapshots, type of snapshot etc. causing issues

[a-siva]

This package in file build/build_web_compilers/lib/src/sdk_js_compile_builder.dart uses knowledge about the internals of the sdk layout to figure out location of a snapshot and assumes that the snapshot would be a JIT snapshot to execute it. having such a hard coded dependency on the SDK layout is bad, the layout of the SDK could change version to version causing breakages in this packages.

The sdk is in the process of switching snapshots to AOT snapshots (https://github.com/dart-lang/sdk/issues/53576) and the assumption in the above code about the snapshot being a JIT snapshot breaks this package when the snapshot type is changed to an AOT snapshot.

[jakemac53]

If/when we have an alternative entry point to the compilers, we would be happy to use it.

I mentioned in the other PR, but if we also can commit to a command line interface (including arguments), then we could possibly drop the tight SDK constraints that we currently have. We do that just because of how tightly coupled we are with the SDK in terms of these assumptions.

[a-siva]

https://dart-review.googlesource.com/c/sdk/+/392202 adds a command line interface in dart cli to invoke the dart development compiler

[jakemac53]

cc @nshahan are we committing to the current CLI args as well as a part of this?

[jakemac53]

@a-siva note that we also have a dependency on the frontend server path so if we want to fully stop relying on SDK internals we also would need a public entry point to that.

[jakemac53]

Or actually I think its not frontend server but the kernel worker snapshot, https://github.com/dart-lang/build/blob/039aecaab190c70077f69e72f090675c8df04685/build_modules/lib/src/workers.dart#L82

Other packages do rely on frontend server though (specifically, package:test)

[a-siva]

Or actually I think its not frontend server but the kernel worker snapshot,

https://github.com/dart-lang/build/blob/039aecaab190c70077f69e72f090675c8df04685/build_modules/lib/src/workers.dart#L82

Other packages do rely on frontend server though (specifically, package:test)

The frontend server snapshot has already been switched to an aot snapshot in the SDK and the JIT snapshot deleted. I think all references to the JIT snapshot have been modified.

With regards to the kernel worker I have a question about why we have this additional wrapper around CFE, isn;t the frontend server capable of handling the same functionality ?

[jakemac53]

The frontend server snapshot has already been switched to an aot snapshot in the SDK and the JIT snapshot deleted. I think all references to the JIT snapshot have been modified.

Yes, but we are still relying on the specific location of the AOT snapshot.

With regards to the kernel worker I have a question about why we have this additional wrapper around CFE, isn;t the frontend server capable of handling the same functionality ?

This is for modular compilation specifically - this is the same entrypoint used by bazel workers. It implements specifically the bazel worker protocol for long lived workers.

[a-siva]

Yes, but we are still relying on the specific location of the AOT snapshot.

True. @derekxu16 is working on making the resident frontend-server process robust. Once that is done we can switch out these direct invocation of the AOT snapshots to use the command to start the frontend server.

With regards to the kernel worker I have a question about why we have this additional wrapper around CFE, isn;t the frontend server capable of handling the same functionality ?

This is for modular compilation specifically - this is the same entrypoint used by bazel workers. It implements specifically the bazel worker protocol for long lived workers.

Ok, I will add a command for the kernel worker

[nshahan]

are we committing to the current CLI args as well as a part of this?

Nothing has changed in regards to our policy of breaking the support for the command line flags for DDC. They should all be considered as (un)stable as they were before this change. I think of this as decoupling the invocations in tooling outside the SDK from the file structure shipped inside the SDK.

[jakemac53]

They should all be considered as (un)stable as they were before this change.

Ok, we will keep the tight constraints then 👍

[davidmorgan]

Closing as stale.