Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[xaprepare] Fix dotnet-install script size check #8311

Merged
merged 1 commit into from
Aug 30, 2023
Merged

[xaprepare] Fix dotnet-install script size check #8311

merged 1 commit into from
Aug 30, 2023

Conversation

pjcollins
Copy link
Member

CI builds have been failing frequently and quietly:

Downloading dotnet-install...
  Error: Installation of dotnet SDK 8.0.100-rc.2.23422.31 failed.

Step Xamarin.Android.Prepare.Step_InstallDotNetPreview failed
System.InvalidOperationException: Step Xamarin.Android.Prepare.Step_InstallDotNetPreview failed

We were returning early when attempts to calculate the size of the
dotnet-install script failed, and a cached install script file was not
already on disk. This has been fixed by ensuring that we still attempt
to download the script if the size calculation fails.

The size check failure has also been fixed by using a get request, and
logging around this prepare step has been improved.

@pjcollins
[xaprepare] Fix dotnet-install script size check
0408ced 
CI builds have been failing frequently and quietly:

    Downloading dotnet-install...
      Error: Installation of dotnet SDK 8.0.100-rc.2.23422.31 failed.

    Step Xamarin.Android.Prepare.Step_InstallDotNetPreview failed
    System.InvalidOperationException: Step Xamarin.Android.Prepare.Step_InstallDotNetPreview failed

We were returning early when attempts to calculate the size of the
`dotnet-install` script failed, and a cached install script file was not
already on disk.  This has been fixed by ensuring that we still attempt
to download the script if the size calculation fails.

The size check failure has also been fixed by using a get request, and
logging around this prepare step has been improved.
jonathanpeppers
jonathanpeppers reviewed Aug 30, 2023
View reviewed changes
build-tools/xaprepare/xaprepare/Steps/Step_InstallDotNetPreview.cs
Comment on lines -88 to +89
return ReportAndCheckCached (message, quietOnError: true);
if (ReportAndCheckCached (message, quietOnError: true))
return true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I deleted the quietOnError parameter entirely and just made it log every time.

I don't think you'd see message above in the build logs without doing something like:

image

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like we may want to skip the cache file error logging initially, as long as we actually proceed to the download step which we weren't doing before. It could be confusing to ~always have that error message in our CI runs?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way it is currently, it looks like we'd never see this log message on error:

Failed to obtain dotnet-install script size from URL '{dotnetScriptUrl}'. HTTP status code:

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or we could simplify this to just warn in both places, as an error about failing to download the script will happen later either way...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way it is currently, it looks like we'd never see this log message on error:

Failed to obtain dotnet-install script size from URL '{dotnetScriptUrl}'. HTTP status code:

Ah right, while this shouldn't be fatal, we still wouldn't know that the size check failed. Let me update the warning/error messaging again.

@jonathanpeppers jonathanpeppers mentioned this pull request Aug 30, 2023
Closed
jonathanpeppers
jonathanpeppers approved these changes Aug 30, 2023
View reviewed changes
Copy link
Member

@jonathanpeppers jonathanpeppers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's merge this to get CI green, it looks like it got past the point that was broken before.

@jonathanpeppers jonathanpeppers merged commit 6862b52 into dotnet:main Aug 30, 2023
2 of 21 checks passed
pjcollins added a commit to pjcollins/android that referenced this pull request Aug 30, 2023
@pjcollins
[xaprepare] Improve dotnet-install script logging
360fd6c 
Context: dotnet#8311 (comment)

Improves the logging around attempts to download or calculate the size
of the dotnet-install script.  Messages about cached install script use
will also now be more explicit.
pjcollins added a commit to pjcollins/android that referenced this pull request Aug 30, 2023
@pjcollins
[xaprepare] Improve dotnet-install script logging
d42431d 
Context: dotnet#8311 (comment)

Improves the logging around attempts to download or calculate the size
of the dotnet-install script.  Messages about cached install script use
will be more explicit, and readability of failure cases should also be
improved.
pjcollins added a commit to pjcollins/android that referenced this pull request Aug 30, 2023
@pjcollins
[xaprepare] Improve dotnet-install script logging
060550c 
Context: dotnet#8311 (comment)

Improves the logging around attempts to download or calculate the size
of the dotnet-install script.  Messages about cached install script use
will be more explicit, and readability of failure cases should also be
improved.
@pjcollins pjcollins mentioned this pull request Aug 30, 2023
Merged
jonathanpeppers pushed a commit that referenced this pull request Aug 30, 2023
@pjcollins
[xaprepare] Improve dotnet-install script logging (#8312)
c31dcae 
Context: #8311 (comment)

Improves the logging around attempts to download or calculate the size
of the dotnet-install script.  Messages about cached install script use
will be more explicit, and readability of failure cases should also be
improved.
jonathanpeppers pushed a commit that referenced this pull request Aug 31, 2023
@pjcollins @jonathanpeppers
[xaprepare] Fix dotnet-install script size check (#8311)
a75db6d 
CI builds have been failing frequently and quietly:

    Downloading dotnet-install...
      Error: Installation of dotnet SDK 8.0.100-rc.2.23422.31 failed.

    Step Xamarin.Android.Prepare.Step_InstallDotNetPreview failed
    System.InvalidOperationException: Step Xamarin.Android.Prepare.Step_InstallDotNetPreview failed

We were returning early when attempts to calculate the size of the
`dotnet-install` script failed, and a cached install script file was not
already on disk.  This has been fixed by ensuring that we still attempt
to download the script if the size calculation fails.

The size check failure has also been fixed by using a get request, and
logging around this prepare step has been improved.
jonathanpeppers pushed a commit that referenced this pull request Aug 31, 2023
@pjcollins @jonathanpeppers
[xaprepare] Improve dotnet-install script logging (#8312)
9f4e071 
Context: #8311 (comment)

Improves the logging around attempts to download or calculate the size
of the dotnet-install script.  Messages about cached install script use
will be more explicit, and readability of failure cases should also be
improved.
grendello added a commit to grendello/xamarin-android that referenced this pull request Sep 4, 2023
@grendello
Merge branch 'main' into bump-ndk
67bdcad 
* main: (57 commits)
  Bump to dotnet/installer@2809943e7a 8.0.100-rc.2.23431.5 (dotnet#8317)
  [build] Use Microsoft OpenJDK 17.0.8 (dotnet#8309)
  [Mono.Android] Add missing `[Flags]` attribute for generated enums. (dotnet#8310)
  Bump to dotnet/installer@c5e45fd659 8.0.100-rc.2.23427.4 (dotnet#8305)
  [xaprepare] Improve dotnet-install script logging (dotnet#8312)
  [xaprepare] Fix dotnet-install script size check (dotnet#8311)
  [Xamarin.Android.Build.Tasks] improve net6.0 "out of support" message (dotnet#8307)
  [monodroid] Fix the EnableNativeAnalyzers build (dotnet#8293)
  Bump to dotnet/installer@56d8c6497c 8.0.100-rc.2.23422.31 (dotnet#8291)
  [Xamarin.Android.Build.Tasks] Fix APT2264 error on Windows. (dotnet#8289)
  [Mono.Android] Marshal .NET stack trace to Throwable.getStackTrace() (dotnet#8185)
  [tests] Skip sign check when installing MAUI (dotnet#8288)
  Bump to xamarin/monodroid@057b17fe (dotnet#8286)
  [Xamarin.Android.Build.Tasks] add $(AndroidStripILAfterAOT) (dotnet#8172)
  Bump to dotnet/installer@ec2c1ec1b1 8.0.100-rc.2.23420.6 (dotnet#8281)
  Bump to dotnet/installer@001d8e4465 8.0.100-rc.2.23417.14 (dotnet#8276)
  [Mono.Android] [IntentFilter] pathSuffix & pathAdvancedPattern  (dotnet#8261)
  $(AndroidPackVersionSuffix)=rc.2; net8 is 34.0.0-rc.2 (dotnet#8278)
  Bump to xamarin/xamarin-android-tools/main@52f0866 (dotnet#8241)
  [build] set file extension for common `ToolExe` values (dotnet#8267)
  ...
grendello added a commit to grendello/xamarin-android that referenced this pull request Sep 4, 2023
@grendello
Merge branch 'main' into xaprepare-version
1303d50 
* main: (102 commits)
  Bump to dotnet/installer@2809943e7a 8.0.100-rc.2.23431.5 (dotnet#8317)
  [build] Use Microsoft OpenJDK 17.0.8 (dotnet#8309)
  [Mono.Android] Add missing `[Flags]` attribute for generated enums. (dotnet#8310)
  Bump to dotnet/installer@c5e45fd659 8.0.100-rc.2.23427.4 (dotnet#8305)
  [xaprepare] Improve dotnet-install script logging (dotnet#8312)
  [xaprepare] Fix dotnet-install script size check (dotnet#8311)
  [Xamarin.Android.Build.Tasks] improve net6.0 "out of support" message (dotnet#8307)
  [monodroid] Fix the EnableNativeAnalyzers build (dotnet#8293)
  Bump to dotnet/installer@56d8c6497c 8.0.100-rc.2.23422.31 (dotnet#8291)
  [Xamarin.Android.Build.Tasks] Fix APT2264 error on Windows. (dotnet#8289)
  [Mono.Android] Marshal .NET stack trace to Throwable.getStackTrace() (dotnet#8185)
  [tests] Skip sign check when installing MAUI (dotnet#8288)
  Bump to xamarin/monodroid@057b17fe (dotnet#8286)
  [Xamarin.Android.Build.Tasks] add $(AndroidStripILAfterAOT) (dotnet#8172)
  Bump to dotnet/installer@ec2c1ec1b1 8.0.100-rc.2.23420.6 (dotnet#8281)
  Bump to dotnet/installer@001d8e4465 8.0.100-rc.2.23417.14 (dotnet#8276)
  [Mono.Android] [IntentFilter] pathSuffix & pathAdvancedPattern  (dotnet#8261)
  $(AndroidPackVersionSuffix)=rc.2; net8 is 34.0.0-rc.2 (dotnet#8278)
  Bump to xamarin/xamarin-android-tools/main@52f0866 (dotnet#8241)
  [build] set file extension for common `ToolExe` values (dotnet#8267)
  ...
grendello added a commit to grendello/xamarin-android that referenced this pull request Sep 4, 2023
@grendello
Merge branch 'main' into rid-attribute
1f137da 
* main: (68 commits)
  Bump to dotnet/installer@2809943e7a 8.0.100-rc.2.23431.5 (dotnet#8317)
  [build] Use Microsoft OpenJDK 17.0.8 (dotnet#8309)
  [Mono.Android] Add missing `[Flags]` attribute for generated enums. (dotnet#8310)
  Bump to dotnet/installer@c5e45fd659 8.0.100-rc.2.23427.4 (dotnet#8305)
  [xaprepare] Improve dotnet-install script logging (dotnet#8312)
  [xaprepare] Fix dotnet-install script size check (dotnet#8311)
  [Xamarin.Android.Build.Tasks] improve net6.0 "out of support" message (dotnet#8307)
  [monodroid] Fix the EnableNativeAnalyzers build (dotnet#8293)
  Bump to dotnet/installer@56d8c6497c 8.0.100-rc.2.23422.31 (dotnet#8291)
  [Xamarin.Android.Build.Tasks] Fix APT2264 error on Windows. (dotnet#8289)
  [Mono.Android] Marshal .NET stack trace to Throwable.getStackTrace() (dotnet#8185)
  [tests] Skip sign check when installing MAUI (dotnet#8288)
  Bump to xamarin/monodroid@057b17fe (dotnet#8286)
  [Xamarin.Android.Build.Tasks] add $(AndroidStripILAfterAOT) (dotnet#8172)
  Bump to dotnet/installer@ec2c1ec1b1 8.0.100-rc.2.23420.6 (dotnet#8281)
  Bump to dotnet/installer@001d8e4465 8.0.100-rc.2.23417.14 (dotnet#8276)
  [Mono.Android] [IntentFilter] pathSuffix & pathAdvancedPattern  (dotnet#8261)
  $(AndroidPackVersionSuffix)=rc.2; net8 is 34.0.0-rc.2 (dotnet#8278)
  Bump to xamarin/xamarin-android-tools/main@52f0866 (dotnet#8241)
  [build] set file extension for common `ToolExe` values (dotnet#8267)
  ...
grendello added a commit to grendello/xamarin-android that referenced this pull request Sep 4, 2023
@grendello
Merge branch 'main' into all-assemblies-per-rid
f1fb7e8 
* main: (53 commits)
  Bump to dotnet/installer@2809943e7a 8.0.100-rc.2.23431.5 (dotnet#8317)
  [build] Use Microsoft OpenJDK 17.0.8 (dotnet#8309)
  [Mono.Android] Add missing `[Flags]` attribute for generated enums. (dotnet#8310)
  Bump to dotnet/installer@c5e45fd659 8.0.100-rc.2.23427.4 (dotnet#8305)
  [xaprepare] Improve dotnet-install script logging (dotnet#8312)
  [xaprepare] Fix dotnet-install script size check (dotnet#8311)
  [Xamarin.Android.Build.Tasks] improve net6.0 "out of support" message (dotnet#8307)
  [monodroid] Fix the EnableNativeAnalyzers build (dotnet#8293)
  Bump to dotnet/installer@56d8c6497c 8.0.100-rc.2.23422.31 (dotnet#8291)
  [Xamarin.Android.Build.Tasks] Fix APT2264 error on Windows. (dotnet#8289)
  [Mono.Android] Marshal .NET stack trace to Throwable.getStackTrace() (dotnet#8185)
  [tests] Skip sign check when installing MAUI (dotnet#8288)
  Bump to xamarin/monodroid@057b17fe (dotnet#8286)
  [Xamarin.Android.Build.Tasks] add $(AndroidStripILAfterAOT) (dotnet#8172)
  Bump to dotnet/installer@ec2c1ec1b1 8.0.100-rc.2.23420.6 (dotnet#8281)
  Bump to dotnet/installer@001d8e4465 8.0.100-rc.2.23417.14 (dotnet#8276)
  [Mono.Android] [IntentFilter] pathSuffix & pathAdvancedPattern  (dotnet#8261)
  $(AndroidPackVersionSuffix)=rc.2; net8 is 34.0.0-rc.2 (dotnet#8278)
  Bump to xamarin/xamarin-android-tools/main@52f0866 (dotnet#8241)
  [build] set file extension for common `ToolExe` values (dotnet#8267)
  ...
grendello added a commit to grendello/xamarin-android that referenced this pull request Sep 4, 2023
@grendello
Merge branch 'main' into extended-timing
1ecc140 
* main: (25 commits)
  Bump to dotnet/installer@2809943e7a 8.0.100-rc.2.23431.5 (dotnet#8317)
  [build] Use Microsoft OpenJDK 17.0.8 (dotnet#8309)
  [Mono.Android] Add missing `[Flags]` attribute for generated enums. (dotnet#8310)
  Bump to dotnet/installer@c5e45fd659 8.0.100-rc.2.23427.4 (dotnet#8305)
  [xaprepare] Improve dotnet-install script logging (dotnet#8312)
  [xaprepare] Fix dotnet-install script size check (dotnet#8311)
  [Xamarin.Android.Build.Tasks] improve net6.0 "out of support" message (dotnet#8307)
  [monodroid] Fix the EnableNativeAnalyzers build (dotnet#8293)
  Bump to dotnet/installer@56d8c6497c 8.0.100-rc.2.23422.31 (dotnet#8291)
  [Xamarin.Android.Build.Tasks] Fix APT2264 error on Windows. (dotnet#8289)
  [Mono.Android] Marshal .NET stack trace to Throwable.getStackTrace() (dotnet#8185)
  [tests] Skip sign check when installing MAUI (dotnet#8288)
  Bump to xamarin/monodroid@057b17fe (dotnet#8286)
  [Xamarin.Android.Build.Tasks] add $(AndroidStripILAfterAOT) (dotnet#8172)
  Bump to dotnet/installer@ec2c1ec1b1 8.0.100-rc.2.23420.6 (dotnet#8281)
  Bump to dotnet/installer@001d8e4465 8.0.100-rc.2.23417.14 (dotnet#8276)
  [Mono.Android] [IntentFilter] pathSuffix & pathAdvancedPattern  (dotnet#8261)
  $(AndroidPackVersionSuffix)=rc.2; net8 is 34.0.0-rc.2 (dotnet#8278)
  Bump to xamarin/xamarin-android-tools/main@52f0866 (dotnet#8241)
  [build] set file extension for common `ToolExe` values (dotnet#8267)
  ...
grendello added a commit to grendello/xamarin-android that referenced this pull request Sep 4, 2023
@grendello
Merge branch 'main' into maui-perf
cffb1a3 
* main: (38 commits)
  Bump to dotnet/installer@2809943e7a 8.0.100-rc.2.23431.5 (dotnet#8317)
  [build] Use Microsoft OpenJDK 17.0.8 (dotnet#8309)
  [Mono.Android] Add missing `[Flags]` attribute for generated enums. (dotnet#8310)
  Bump to dotnet/installer@c5e45fd659 8.0.100-rc.2.23427.4 (dotnet#8305)
  [xaprepare] Improve dotnet-install script logging (dotnet#8312)
  [xaprepare] Fix dotnet-install script size check (dotnet#8311)
  [Xamarin.Android.Build.Tasks] improve net6.0 "out of support" message (dotnet#8307)
  [monodroid] Fix the EnableNativeAnalyzers build (dotnet#8293)
  Bump to dotnet/installer@56d8c6497c 8.0.100-rc.2.23422.31 (dotnet#8291)
  [Xamarin.Android.Build.Tasks] Fix APT2264 error on Windows. (dotnet#8289)
  [Mono.Android] Marshal .NET stack trace to Throwable.getStackTrace() (dotnet#8185)
  [tests] Skip sign check when installing MAUI (dotnet#8288)
  Bump to xamarin/monodroid@057b17fe (dotnet#8286)
  [Xamarin.Android.Build.Tasks] add $(AndroidStripILAfterAOT) (dotnet#8172)
  Bump to dotnet/installer@ec2c1ec1b1 8.0.100-rc.2.23420.6 (dotnet#8281)
  Bump to dotnet/installer@001d8e4465 8.0.100-rc.2.23417.14 (dotnet#8276)
  [Mono.Android] [IntentFilter] pathSuffix & pathAdvancedPattern  (dotnet#8261)
  $(AndroidPackVersionSuffix)=rc.2; net8 is 34.0.0-rc.2 (dotnet#8278)
  Bump to xamarin/xamarin-android-tools/main@52f0866 (dotnet#8241)
  [build] set file extension for common `ToolExe` values (dotnet#8267)
  ...
jonathanpeppers pushed a commit that referenced this pull request Sep 15, 2023
@pjcollins @jonathanpeppers
[xaprepare] Fix dotnet-install script size check (#8311)
551dfe8 
CI builds have been failing frequently and quietly:

    Downloading dotnet-install...
      Error: Installation of dotnet SDK 8.0.100-rc.2.23422.31 failed.

    Step Xamarin.Android.Prepare.Step_InstallDotNetPreview failed
    System.InvalidOperationException: Step Xamarin.Android.Prepare.Step_InstallDotNetPreview failed

We were returning early when attempts to calculate the size of the
`dotnet-install` script failed, and a cached install script file was not
already on disk.  This has been fixed by ensuring that we still attempt
to download the script if the size calculation fails.

The size check failure has also been fixed by using a get request, and
logging around this prepare step has been improved.
jonathanpeppers pushed a commit that referenced this pull request Sep 15, 2023
@pjcollins @jonathanpeppers
[xaprepare] Improve dotnet-install script logging (#8312)
2ebeb80 
Context: #8311 (comment)

Improves the logging around attempts to download or calculate the size
of the dotnet-install script.  Messages about cached install script use
will be more explicit, and readability of failure cases should also be
improved.
@github-actions github-actions bot locked and limited conversation to collaborators Jan 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jonathanpeppers jonathanpeppers jonathanpeppers approved these changes

@grendello grendello Awaiting requested review from grendello grendello is a code owner

@jpobst jpobst Awaiting requested review from jpobst

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@pjcollins @jonathanpeppers