Skip to content

Commit 1665aca

Browse files
radicalradekdoulik
andauthored
[wasm] Fix a race condition in adding payloads for helix (#65995)
* [wasm] Fix a race condition in adding payloads for helix The issue shows up only in Wasm.Build.Tests for the `TestUsingWorkloads!=true` case, as `clang --version` failing with: ``` /datadisks/disk1/work/A0100914/p/build/emsdk/upstream/bin/clang: error while loading shared libraries: libLLVM-13git.so: cannot open shared object file: No such file or directory (TaskId:231) ``` For this non-workloads testing case, we copy `emscripten` from the system copy to the git checkout. And then use that directory for the payload. The build logs show that the missing file (`libLLVM-13git.so`) does get copied as part of copying `/usr/local/emscripten/emsdk` to `$(RepoRoot)/src/mono/wasm/emsdk`. But the file, and a few others seem to be missing in the final helix payload. We add `emsdk` to the payload for target path `build/emsdk`. But a recent change also added `node` for this case with a target path `build/emsdk/node`, with an overlapping path with `build/emsdk`. I believe this is causing an issue where these directories are being processed in parallel, and cause some files get missed. This commit: 1. Add `node` only when needed (skip WBT for example); 2. Use a non-overlapping path for `node`, `build/emsdk-node`. Fixes #65956 * Change `_HelixLocalNodePath` evaluation order Co-authored-by: Radek Doulik <radekdoulik@gmail.com>
1 parent 02e4667 commit 1665aca

File tree

1 file changed

+31
-16
lines changed

1 file changed

+31
-16
lines changed

src/libraries/sendtohelix-wasm.targets

Lines changed: 31 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,22 @@
2020
<EmSdkDirForHelixPayload>$([MSBuild]::NormalizeDirectory('$(RepoRoot)', 'src', 'mono', 'wasm', 'emsdk'))</EmSdkDirForHelixPayload>
2121

2222
<NeedsWorkload Condition="'$(Scenario)' == 'BuildWasmApps'">true</NeedsWorkload>
23-
<NeedsEMSDK Condition="'$(NeedsToBuildWasmAppsOnHelix)' == 'true' or ('$(Scenario)' == 'BuildWasmApps' and '$(TestUsingWorkloads)' != 'true')">true</NeedsEMSDK>
23+
<NeedsEMSDK Condition="'$(NeedsToBuildWasmAppsOnHelix)' == 'true' or '$(Scenario)' == 'BuildWasmApps'">true</NeedsEMSDK>
2424
<NeedsEMSDKNode Condition="'$(Scenario)' == 'WasmTestOnNodeJs' or '$(Scenario)' == 'BuildWasmApps'">true</NeedsEMSDKNode>
2525
<NeedsToRunOnBrowser Condition="'$(Scenario)' == 'WasmTestOnBrowser' or '$(Scenario)' == 'BuildWasmApps'">true</NeedsToRunOnBrowser>
2626
<NeedsToRunOnBrowser Condition="'$(NeedsToRunOnBrowser)' == '' and '$(IsWasmDebuggerTests)' == 'true'">true</NeedsToRunOnBrowser>
2727

2828
<IncludeXHarnessCli>true</IncludeXHarnessCli>
2929
<EnableXHarnessTelemetry>true</EnableXHarnessTelemetry>
30+
<IncludeNodePayload Condition="'$(NeedsEMSDKNode)' == 'true' and '$(NeedsEMSDK)' != 'true'">true</IncludeNodePayload>
31+
</PropertyGroup>
32+
33+
<PropertyGroup>
34+
<_HelixLocalNodePath Condition="'$(NeedsEMSDKNode)' == 'true' and '$(WindowsShell)' != 'true'">$HELIX_CORRELATION_PAYLOAD/build/emsdk-node</_HelixLocalNodePath>
35+
<_HelixLocalNodePath Condition="'$(NeedsEMSDKNode)' == 'true' and '$(WindowsShell)' == 'true'">%HELIX_CORRELATION_PAYLOAD%\build\emsdk-node</_HelixLocalNodePath>
36+
37+
<_HelixLocalNodePath Condition="'$(NeedsEMSDK)' == 'true' and '$(WindowsShell)' != 'true'">$HELIX_CORRELATION_PAYLOAD/build/emsdk/node</_HelixLocalNodePath>
38+
<_HelixLocalNodePath Condition="'$(NeedsEMSDK)' == 'true' and '$(WindowsShell)' == 'true'">%HELIX_CORRELATION_PAYLOAD%\build\emsdk\node</_HelixLocalNodePath>
3039
</PropertyGroup>
3140

3241
<ItemGroup Condition="'$(WindowsShell)' != 'true'">
@@ -40,15 +49,6 @@
4049

4150
<HelixPreCommand Condition="'$(NeedsToRunOnBrowser)' == 'true'" Include="export PATH=$HELIX_CORRELATION_PAYLOAD/$(ChromeDriverDirName):$PATH" />
4251
<HelixPreCommand Condition="'$(NeedsToRunOnBrowser)' == 'true'" Include="export PATH=$HELIX_CORRELATION_PAYLOAD/$(ChromiumDirName):$PATH" />
43-
44-
<!-- Fix symbolic links that are broken already on build machine and also in the correlation payload -->
45-
<HelixPreCommand Condition="'$(NeedsEMSDKNode)' == 'true'" Include="export HELIX_NODEJS_VERSION=%24(ls $HELIX_CORRELATION_PAYLOAD/build/emsdk/node)" />
46-
<HelixPreCommand Condition="'$(NeedsEMSDKNode)' == 'true'" Include="export HELIX_NODEJS_PATH=$HELIX_CORRELATION_PAYLOAD/build/emsdk/node/$HELIX_NODEJS_VERSION" />
47-
<HelixPreCommand Condition="'$(NeedsEMSDKNode)' == 'true'" Include="export PATH=$HELIX_NODEJS_PATH/bin:$PATH" />
48-
<HelixPreCommand Condition="'$(NeedsEMSDKNode)' == 'true'" Include="rm $HELIX_NODEJS_PATH/bin/npm" />
49-
<HelixPreCommand Condition="'$(NeedsEMSDKNode)' == 'true'" Include="rm $HELIX_NODEJS_PATH/bin/npx" />
50-
<HelixPreCommand Condition="'$(NeedsEMSDKNode)' == 'true'" Include="ln -s ../lib/node_modules/npm/bin/npm-cli.js $HELIX_NODEJS_PATH/bin/npm" />
51-
<HelixPreCommand Condition="'$(NeedsEMSDKNode)' == 'true'" Include="ln -s ../lib/node_modules/npm/bin/npx-cli.js $HELIX_NODEJS_PATH/bin/npx" />
5252
</ItemGroup>
5353

5454
<ItemGroup Condition="'$(WindowsShell)' == 'true'">
@@ -62,12 +62,26 @@
6262

6363
<HelixPreCommand Condition="'$(NeedsToRunOnBrowser)' == 'true'" Include="set PATH=%HELIX_CORRELATION_PAYLOAD%\$(ChromeDriverDirName)%3B%PATH%" />
6464
<HelixPreCommand Condition="'$(NeedsToRunOnBrowser)' == 'true'" Include="set PATH=%HELIX_CORRELATION_PAYLOAD%\$(ChromiumDirName)%3B%PATH%" />
65+
</ItemGroup>
66+
67+
<ItemGroup Condition="'$(NeedsEMSDKNode)' == 'true' and '$(WindowsShell)' != 'true'">
68+
<!-- Fix symbolic links that are broken already on build machine and also in the correlation payload -->
69+
<HelixPreCommand Include="export _HELIX_NODEJS_VERSION=%24(ls $(_HelixLocalNodePath))" />
70+
<HelixPreCommand Include="export _HELIX_NODEJS_PATH=$(_HelixLocalNodePath)/$_HELIX_NODEJS_VERSION" />
71+
<HelixPreCommand Include="export PATH=$_HELIX_NODEJS_PATH/bin:$PATH" />
72+
<HelixPreCommand Include="rm $_HELIX_NODEJS_PATH/bin/npm" />
73+
<HelixPreCommand Include="rm $_HELIX_NODEJS_PATH/bin/npx" />
74+
<HelixPreCommand Include="ln -s ../lib/node_modules/npm/bin/npm-cli.js $_HELIX_NODEJS_PATH/bin/npm" />
75+
<HelixPreCommand Include="ln -s ../lib/node_modules/npm/bin/npx-cli.js $_HELIX_NODEJS_PATH/bin/npx" />
76+
</ItemGroup>
6577

66-
<HelixPreCommand Condition="'$(NeedsEMSDKNode)' == 'true'" Include="for /f %%i in ('dir %HELIX_CORRELATION_PAYLOAD%\build\emsdk\node /b') do set HELIX_NODEJS_VERSION=%%i" />
67-
<HelixPreCommand Condition="'$(NeedsEMSDKNode)' == 'true'" Include="set PATH=%HELIX_CORRELATION_PAYLOAD%\build\emsdk\node\%HELIX_NODEJS_VERSION%\bin%3B%PATH%" />
78+
<ItemGroup Condition="'$(NeedsEMSDKNode)' == 'true' and '$(WindowsShell)' == 'true'">
79+
<HelixPreCommand Include="for /f %%i in ('dir $(_HelixLocalNodePath) /b') do set _HELIX_NODEJS_VERSION=%%i" />
80+
<HelixPreCommand Include="set PATH=$(_HelixLocalNodePath)/%_HELIX_NODEJS_VERSION%/bin%3B%PATH%" />
6881
</ItemGroup>
6982

7083
<PropertyGroup>
84+
7185
<!--
7286
We are hosting the payloads for the WASM/browser on kestrel in the xharness process.
7387
We also run some network tests to this server and so, we are running it on both HTTP and HTTPS.
@@ -123,8 +137,9 @@
123137
<HelixCorrelationPayload Include="$(MonoTargetsTasksDir)" Destination="build/MonoTargetsTasks" />
124138
</ItemGroup>
125139

126-
<ItemGroup Condition="'$(NeedsEMSDKNode)' == 'true'">
127-
<HelixCorrelationPayload Include="$(EmSdkDirForHelixPayload)node" Destination="build/emsdk/node" />
140+
<!-- copy node separately only if EMSDK is not being included -->
141+
<ItemGroup Condition="'$(IncludeNodePayload)' == 'true'">
142+
<HelixCorrelationPayload Include="$(EmSdkDirForHelixPayload)node" Destination="build/emsdk-node" />
128143
</ItemGroup>
129144

130145
<ItemGroup Condition="'$(Scenario)' == '' or '$(Scenario)' == 'normal' or '$(Scenario)' == 'WasmTestOnBrowser' or '$(Scenario)' == 'WasmTestOnNodeJS'">
@@ -154,7 +169,7 @@
154169
</PropertyGroup>
155170

156171
<Message Condition="'$(NeedsEMSDK)' == 'true' or '$(NeedsEMSDKNode)' == 'true'" Importance="High" Text="Using emsdk: $(EmSdkDirForHelixPayload)" />
157-
172+
158173
<ItemGroup Condition="'$(IsRunningLibraryTests)' == 'true'">
159174
<_WasmWorkItem Include="$(TestArchiveRoot)browseronly/**/*.zip" Condition="'$(Scenario)' == 'WasmTestOnBrowser'" />
160175
<_WasmWorkItem Include="$(TestArchiveRoot)browserornodejs/**/*.zip" Condition="'$(Scenario)' == 'WasmTestOnBrowser'" />
@@ -208,7 +223,7 @@
208223
<!-- CI has emscripten provisioned in $(EMSDK_PATH) as `/usr/local/emscripten`. Because helix tasks will
209224
attempt to write a .payload file, we cannot use $(EMSDK_PATH) to package emsdk as a helix correlation
210225
payload. Instead, we copy over the files to a new directory `src/mono/wasm/emsdk` and use that. -->
211-
<Target Name="StageEmSdkForHelix" Condition="('$(NeedsEMSDK)' == 'true' or '$(NeedsEMSDKNode)' == 'true') and !Exists($(EmSdkDirForHelixPayload))">
226+
<Target Name="StageEmSdkForHelix" Condition="('$(NeedsEMSDK)' == 'true' or '$(IncludeNodePayload)' == 'true') and !Exists($(EmSdkDirForHelixPayload))">
212227
<Error Condition="'$(EMSDK_PATH)' == '' or !Exists($(EMSDK_PATH))"
213228
Text="Could not find emscripten sdk in EMSDK_PATH=$(EMSDK_PATH), needed to provision for running tests on helix" />
214229

0 commit comments

Comments
 (0)