Remove `CCompiler.{envScript,envScriptArgs}` and replace by other means

[mkustermann]

Currently we have the following class:

abstract final class CCompilerConfig {
  /// Path to a C compiler.
  Uri? get compiler;

  /// Path to a native linker.
  Uri? get linker;

  /// Path to a native archiver.
  Uri? get archiver;

  /// Path to script that sets environment variables for [compiler], [linker],
  /// and [archiver].
  Uri? get envScript;

  /// Arguments for [envScript].
  List<String>? get envScriptArgs;

  factory CCompilerConfig({
    Uri? archiver,
    Uri? compiler,
    Uri? linker,
    Uri? envScript,
    List<String>? envScriptArgs,
  }) = CCompilerConfigImpl;
}

It is very weird to have envScript and envScriptArgs here. It seems that windows-specific details are being exposed by this API.

We should consider removing these two fields and instead do that differently:

• Either add a class CCompilerConfig { final Map<String, String> environmentVariables; } that encapsulate the additional environment variables needed when invoking CCompilerConfig.{compiler,linker,archiver}
• Or have no replacement: The caller can figure out the additional environment, make wrapper-shell scripts for compiler/linker/archiver. Those wrapper shell scripts can set the needed environment and call the actual target compiler/linker/archiver

[mkustermann]

/cc @dcharkes For discussion

[dcharkes]

The way that the Windows commandline compiler works is that you're supposed to launch a different shell, that has all these env-vars set (including where to find standard libraries.)

• Or have no replacement:

Every user will have to do that, that sounds horrible. Without the env vars basically no compilation can succeed.

• Either add a class CCompilerConfig { final Map<String, String> environmentVariables; } that encapsulate the additional environment variables needed when invoking CCompilerConfig.{compiler,linker,archiver}

That could work, but that requires us to do this in multiple places:

1. flutter tools (or native_assets_builder, but we don't use it in Dartdev because we don't have a predefined compiler there.)
2. Dart CI

So the hacky code that runs the bash script to extract the vars would live in multiple places instead of one.

Not sure if that's cleaner or worse than the current solution.

[mkustermann]

That could work, but that requires us to do this in multiple places:

I could imagine having some helper function somewhere that invokes this envScript with envScriptArgs, sees what environment needs to be set and returns a Map<String, String>.

That shared helper can be used by all bundling tools (of which there's very few) when determining the CCompilerConfig.

To me this seems better than exposing CCompilerConfig.{envScript,envScriptArgs} in the build/hook.dart APIs (which will be seen by many package authors and - similar to me - some may be very confused what those two getters mean, when they have to be used, ...)

[dcharkes]

Another consideration is doing unneeded work if no code assets are built. If we do this inside flutter_tools, we'll need to always run it.

If we always invoke the script we should consider just putting the environment variables into the environment the hook is invoked with. Then we can completely omit a CompilerConfig.environment that users might forget to pass into Process.run.

[dcharkes]

If we always invoke the script we should consider just putting the environment variables into the environment the hook is invoked with. Then we can completely omit a CompilerConfig.environment that users might forget to pass into Process.run.

It feels a bit weird though that we would add env vars to be able to run executables, but we don't also simply prepend the directory with the executables to the path. E.g. the environment we decide to pass in becomes part of the contract for config.code if config.buildCodeAssets == true, but the environment is something like a top-level thing. (Both the JSON config and the environment are toplevels in and of themselves and are both part of the protocol.)

It seems that windows-specific details are being exposed by this API.

Either way, we have windows-specific details in the API, either in the CCompilierConfig or in the (somewhat hidden) environment variables.

Completely removing them is also ugly. We'd be providing half the information needed to run the C compiler. We should provide all or none. And none doesn't work either because we'd like the C code to be compiled with the same C compiler as the Flutter native app is compiled.

[mkustermann]

To me all options on the table seem better then status quo with envScript / envScriptArgs:

• make wrapper shell scripts that populate the environment and call real compiler
• make CompilerConfig.environment
• include env vars in hook invocation

Maybe we can do some research how others solve this issue?

Another consideration is doing unneeded work if no code assets are built. If we do this inside flutter_tools, we'll need to always run it.

Well flutter tools always runs % which clang and other things already. And it's going to have to do that only once per flutter X invocation (compared to N invocations of hooks)

[dcharkes]

Maybe we can do some research how others solve this issue?

CMake seems to require vcvars.bat to have been called already, so that the environment variables are already set. And people try to jump through all kinds of hoops to try to make their CMake still work if it wasn't set.

• https://discourse.cmake.org/t/is-there-a-way-to-integrate-the-call-to-vcvarsall-bat-into-the-cmakepresets-json-file/3100
• https://izzys.casa/2023/09/finding-msvc-with-cmake/

In the Dart SDK GN build files we jump through those kind of hoops to extract the vcvars into the environment in build/toolchain/win/BUILD.gn.

We have two cases:

1. The C Compiler is provided by the embedder/launcher (flutter_tools/Dart SDK).
2. No C Compiler is provided by the embedder/launcher, so package:native_toolchain_c scans the host machine to find one.

Case (2) is similar to what we do in the SDK, nothing is provided from the outside, just figure it out yourself. Of course this means config.cCompiler is simply omitted.

Case (1) is most similar to XCode and VS calling cl.exe with the right environment. So, include env vars in hook invocation. The hook invoker provides a direct path to cl.exe so the environment it provides to the hook should have everything to run cl.exe.

[mkustermann]

Thanks for the investigation, @dcharkes !

So it sounds like nobody else is doing what we currently do (i.e. passing along the cc/... as well as envScript and envScriptArgs)? So either one passes cc with right environment vars already set or lets the thing figure it out. That's somewhat aligned with what I was also thinking.