build_runner_run_in_isolation_test failing on pkg-win-release-arm64

[rmacnak-google]

The tests

native_assets_builder/test/build_runner/build_runner_run_in_isolation_test RuntimeError (expected Pass)

are failing on configurations

unittest-asserts-release-win-arm64

Initial results for a new builder.
Builder
Log

Seems related to setting up the MSVC toolchain.

cc @dcharkes @pbo-linaro

[pbo-linaro]

When running it on my windows-arm64 machine, it works well with dart included with latest flutter version:

$ C:/work/flutter/bin/dart --enable_asserts --sound-null-safety -Dtest_runner.configuration=unittest-asserts-release-win-arm64  test/build_runner/build_runner_run_in_isolation_test.dart
$ echo $?
0
$ C:/work/flutter/bin/cache/dart-sdk/bin/dart.exe --version
Dart SDK version: 3.3.0-87.0.dev (dev) (Wed Nov 1 05:03:58 2023 -0700) on "windows_arm64"

By looking at the log file you attached, error seems to be:

FINE: 2023-11-02 14:46:53.690765: native_add.c
FINE: 2023-11-02 14:46:53.784512: /out:native_add.exe 
FINE: 2023-11-02 14:46:53.784512: /DLL 
FINE: 2023-11-02 14:46:53.784512: /out:C:\opt\s\w\ir\x\t\539c2b8\native_add\.dart_tool\native_assets_builder\72eff2cbb7cd7305e1b9c568ff72ec9b\out\native_add.dll 
FINE: 2023-11-02 14:46:53.784512: native_add.obj 
FINE: 2023-11-02 14:46:53.800136: LINK : fatal error LNK1104: cannot open file 'C:\Windows\lnk{D7295B7D-6881-453E-9E15-4F5CD3ABFD3F}.tmp'

I would suggest to try on another machine than the one that reported this failure.
I'm a bit surprised to see a temp file reported in C:\Windows.

[dcharkes]

That is a weird temp location indeed.

vs_files\1023ce2e82\VC\Tools\MSVC\14.29.30133\bin\Hostx64\arm64\cl.exe, what version are you using locally @pbo-linaro ?

I don't have a Windows arm64 machine, so I cannot be of much help here unfortunately.

[pbo-linaro]

cl.exe on PATH is latest version (19.37.32824).
After looking closely at the test, I noticed it was not running anything on my side, due to this if guard.

I'm not sure to understand what to add as option, or env variable to really trigger the test. Any advice to reproduce this test @dcharkes?

[dcharkes]

The test is testing that the compilers run correctly when none of the environment vars is passed in.

In order to test this, it looks for C_COMPILER_CC and alike in the environment, and passes these in manually to the test.

This test doesn't run on the GitHub CI or locally, because we don't control the environment variables.
This test does run on the Dart CI or locally, because we do set the compilers manually.

The fact that we don't skip the test with the if guard, means the C_COMPILER_CC variable is set correctly in https://github.com/dart-lang/sdk/blob/dc68ce5c6839123f6116f57896810437a40ba8b6/pkg/test_runner/lib/src/configuration.dart#L296-L331.

Maybe the MSVC in depot_tools is tool old and doesn't contain working arm64 tools? Could you give 14.29.30133 a spin @pbo-linaro?

[pbo-linaro]

I spent 2 hours trying to reproduce this, without a direct success.

The envscript needed in test is setenv.cmd, which does not come with the Windows SDK I installed (tried on two different machines). I'm not even sure it's present on latest Windows versions to be honest.
In replacement, I tried to use vcvarsall.bat. For a reason I don't understand, some libraries are not found (kernel32.lib), when a "normal" console finds it correctly. Probably some missing env var are preventing this from working.

Using this run.bat script:

set version=14.29.30133
set vcvars=C:/Program Files/Microsoft Visual Studio/2022/Community/VC/Auxiliary/Build/vcvarsall.bat
set root=c:/Program Files/Microsoft Visual Studio/2022/Community/VC/Tools/MSVC/%version%/bin/HostX64/arm64/

set C_COMPILER__CC=%root%/cl.exe
set C_COMPILER__AR=%root%/lib.exe
set C_COMPILER__LD=%root%/link.exe
set C_COMPILER__ENV_SCRIPT=%vcvars%
set C_COMPILER__ENV_SCRIPT_ARGUMENTS=amd64_arm64

call C:/work/flutter/bin/dart.bat test/build_runner/build_runner_run_in_isolation_test.dart

It fails with LINK : fatal error LNK1104: cannot open file 'kernel32.lib'.

---

Instead, I dumped a full working environment in a .bat, and set all env variables.
In this case, the test works with 14.29 (vs2019) and 14.37.

It's hard for me to debug google CI machines remotely, but what @rmacnak-google could do to progress is dump all env variables when the build step is run. Please check the TEMP and TMP variable is set correctly as it's still my intuition for the original failure.

[dcharkes]

setenv.cmd is from depot_tools I believe.

Please check the TEMP variable is set correctly as it's still my intuition for the original failure.

The test is explicitly unsetting all environment variables. Maybe unsetting TEMP is too agressive.

We already have an exception for SYSTEMROOT, possibly TEMP should also have that exception:

native/pkgs/native_assets_builder/lib/src/utils/run_process.dart

Lines 28 to 37 in c72ed16

	if (Platform.isWindows && !includeParentEnvironment) {
	const winEnvKeys = [
	'SYSTEMROOT',
	];
	environment = {
	for (final winEnvKey in winEnvKeys)
	winEnvKey: Platform.environment[winEnvKey]!,
	...?environment,
	};
	}

[pbo-linaro]

The specific VS files (vs_files\1023ce2e82\VC\Tools\MSVC\14.29.30133\bin\Hostx64\arm64\cl.exe) are not redistributed outside of Google (would be illegal), which is the problem for me to reproduce this.
It's a good idea to give a try at what you describe @dcharkes (TMP and TEMP should both be preserved to prevent another issue one of those days).

[dcharkes]

Could you make a PR, I'll hit the merge button.

[dcharkes]

P.S. Thanks for bringing this up. I think we'd like to clamp down the environment variables to avoid having external variables influencing builds (#32), but we need to know which env vars we need to keep.

[pbo-linaro]

If that's the right problem, I would still not understand why it works with x64 version of the test.
Let's see the result.

[pbo-linaro]

It seems like dart CI is back to green for windows-arm64. Is that correct?
(I'm not sure how dart-native is pulled from sdk).

[dcharkes]

It was green because the failure was approved.

We had a roll of your commit to the SDK: https://dart-review.googlesource.com/c/sdk/+/334083
And that made the test pass: https://dart-ci.web.app/#showLatestFailures=false&test=native_assets_builder/test/build_runner/build_runner_run_in_isolation_test

The current results are green: https://dart-current-results.web.app/#/filter=native_assets_builder/test/build_runner/build_runner_run_in_isolation_test&showAll

Thanks @pbo-linaro for the trouble shooting! 🚀
Thanks @rmacnak-google for setting up the pkg bot! 🎸

[pbo-linaro]

Good to see this.
Regarding the TEMP issue, and the fact it works on x64, maybe the user running tests is more privileged on x64, which allows it to read/write in C:\Windows. I know it won't be investigated now, but I mention it in case we have another funny failure to debug specific to windows-arm64 platform.

Thanks @dcharkes for your help on reproduction and details in code and commits involved, that's really appreciated 👍.