-
Notifications
You must be signed in to change notification settings - Fork 5.2k
[wasm] Fix perf pipeline errors #75406
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -38,6 +38,7 @@ iosmono=false | |
| iosllvmbuild="" | ||
| maui_version="" | ||
| only_sanity=false | ||
| dotnet_versions="" | ||
|
|
||
| while (($# > 0)); do | ||
| lowerI="$(echo $1 | tr "[:upper:]" "[:lower:]")" | ||
|
|
@@ -148,6 +149,10 @@ while (($# > 0)); do | |
| use_latest_dotnet=true | ||
| shift 1 | ||
| ;; | ||
| --dotnetversions) | ||
| dotnet_versions="$2" | ||
| shift 2 | ||
| ;; | ||
| --iosmono) | ||
| iosmono=true | ||
| shift 1 | ||
|
|
@@ -196,12 +201,13 @@ while (($# > 0)); do | |
| echo " --wasmbundle Path to the wasm bundle containing the dotnet, and data needed for helix payload" | ||
| echo " --wasmaot Indicate wasm aot" | ||
| echo " --latestdotnet --dotnet-versions will not be specified. --dotnet-versions defaults to LKG version in global.json " | ||
| echo " --dotnetversions Passed as '--dotnet-versions <value>' to the setup script" | ||
| echo " --alpine Set for runs on Alpine" | ||
| echo " --iosmono Set for ios Mono/Maui runs" | ||
| echo " --iosllvmbuild Set LLVM for iOS Mono/Maui runs" | ||
| echo " --mauiversion Set the maui version for Mono/Maui runs" | ||
| echo "" | ||
| exit 0 | ||
| exit 1 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why did we change this to 1?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you pass an unknown argument to the script then it will show help, and exit. But the build will keep going, even though it is missing crucial setup configuration. With
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That makes sense, this should be a good change. |
||
| ;; | ||
| esac | ||
| done | ||
|
|
@@ -354,6 +360,10 @@ if [[ -n "$mono_dotnet" && "$monoaot" == "false" ]]; then | |
| mv $mono_dotnet $mono_dotnet_path | ||
| fi | ||
|
|
||
| if [[ -n "$dotnet_versions" ]]; then | ||
| setup_arguments="$setup_arguments --dotnet-versions $dotnet_versions" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a corresponding change to ci_setup in the perf repo to use this setup argument? I didn't see anything already implemented.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see, I missed the dotnet parser args being added to the ci_setup parser. |
||
| fi | ||
|
|
||
| if [[ "$monoaot" == "true" ]]; then | ||
| monoaot_dotnet_path=$payload_directory/monoaot | ||
| mv $monoaot_path $monoaot_dotnet_path | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is mentioned that the additionalSetupParameters is passed to performance-setup, although is being passed to ci_setup here. Along with the changes to performance-setup, it looks like this should be added to the lines above this one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For
run-perfomance-jobthe comments inperf-wasm-jobs.ymlsay:additionalSetupParameters: '--dotnet-versions 8.0.0' # passed to ci_setup.pyFor
run-scenarios-jobthe comment says:additionalSetupParameters: '--dotnetversions 8.0.0' # passed to performance-setup.shThat matches what the ymls are doing. Please correct me if I'm missing something.