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.

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

[build] Improve build error handling #1223

Merged
merged 1 commit into from
Jan 24, 2018

Conversation

jonpryor
Copy link
Member

Context: #1211 (comment)
Context: #1211 (comment)

What should happen when a portion of a build fails?
That's not entirely rhetorical: the entire build should fail.

What does happen when a portion of a build fails?
Unfortunately that's also not rhetorical: the build continues!

(Say what?!)

Case in point: PR #1211 build 2373, which experienced a
failure in the mono build:

    MONO_PATH="./../../class/lib/build:$MONO_PATH" ... -R ../../../class/lib/monodroid/nunitlite.dll ...
    error CS0009: Metadata file '/Users/builder/jenkins/workspace/xamarin-android-pr-builder/xamarin-android/external/mono/mcs/class/lib/monodroid/nunitlite.dll' could not be opened -- PE image doesn't contain managed metadata.

But the error was ignored, and the build continued, resulting in the
"final" errors (among others)

    System.Net/NetworkChangeTest.cs(5,7): error CS0246: The type or namespace name `NUnit' could not be found. Are you missing an assembly reference?

This is madness. The first error should have stopped the
build. The build wasn't stopped, meaning that it continued -- with
an "unstable" tree state -- resulting in "bizarre" build errors
later. In particular, Xamarin.Android.NUniteLite.dll wasn't built
in the Debug configuration because the mono build failed for the
Debug configuration, but everything "worked" in Release.

The fundamental cause of this madness? Makefile rules such as
make leeroy-all, which effectively are:

    tools/scripts/xabuild Xamarin.Android.sln /p:Configuration=Debug ; \
    tools/scripts/xabuild Xamarin.Android.sln /p:Configuration=Release; \

Because of the ; separating the commands, any errors from the
first command are ignored, resulting in the tearing out of my hair
when everything goes "weird".

Review the Makefile targets in Makefile and
build-tools/scripts/BuildEverything.mk and replace ; with
|| exit 1 when the command should have fatal errors. This should
cause the build to fail the first time an error is encountered,
instead of continuing on its merry way to die horribly later.

Context: dotnet#1211 (comment)
Context: dotnet#1211 (comment)

What *should* happen when a portion of a build fails?
That's not entirely rhetorical: the *entire* build should fail.

What *does* happen when a portion of a build fails?
Unfortunately that's also not rhetorical: the build continues!

(Say what?!)

Case in point: [PR dotnet#1211 build 2373][pr-2373], which experienced a
failure in the mono build:

[pr-2373]: https://jenkins.mono-project.com/view/Xamarin.Android/job/xamarin-android-pr-builder/2373/

        MONO_PATH="./../../class/lib/build:$MONO_PATH" ... -R ../../../class/lib/monodroid/nunitlite.dll ...
        error CS0009: Metadata file '/Users/builder/jenkins/workspace/xamarin-android-pr-builder/xamarin-android/external/mono/mcs/class/lib/monodroid/nunitlite.dll' could not be opened -- PE image doesn't contain managed metadata.

*But the error was ignored*, and the build continued, resulting in the
"final" errors (among others)

        System.Net/NetworkChangeTest.cs(5,7): error CS0246: The type or namespace name `NUnit' could not be found. Are you missing an assembly reference?

**This is *madness*.** The first error *should* have stopped the
build. The build *wasn't* stopped, meaning that it continued -- with
an "unstable" tree state -- resulting in "bizarre" build errors
*later*. In particular, `Xamarin.Android.NUniteLite.dll` wasn't built
in the *Debug* configuration because the *mono* build failed for the
Debug configuration, but everything "worked" in Release.

The fundamental cause of this madness? Makefile rules such as
`make leeroy-all`, which *effectively* are:

        tools/scripts/xabuild Xamarin.Android.sln /p:Configuration=Debug ; \
        tools/scripts/xabuild Xamarin.Android.sln /p:Configuration=Release; \

*Because* of the `;` separating the commands, any errors from the
first command are *ignored*, resulting in the tearing out of my hair
when everything goes "weird".

Review the Makefile targets in `Makefile` and
`build-tools/scripts/BuildEverything.mk` and replace `;` with
`|| exit 1` when the command should have fatal errors. This *should*
cause the build to fail the *first* time an error is encountered,
instead of continuing on its merry way to die horribly later.
@grendello
Copy link
Contributor

build

@jonpryor jonpryor merged commit 84b6f66 into dotnet:master Jan 24, 2018
jonpryor added a commit to jonpryor/xamarin-android that referenced this pull request Aug 26, 2021
Changes: xamarin/monodroid@fb0d502...5676b84

  * xamarin/monodroid@5676b84b4: Bump to xamarin/androidtools@0abc0d7c (dotnet#1223)
  * xamarin/monodroid@daf1aa909: [optimization] Replace 'new T[0]' with 'Array.Empty<T> ()' to reduce allocations. (dotnet#1221)
  * xamarin/monodroid@fb848118b: [tools/msbuild] Check `device.Properties.BuildVersionSdk` for `-1` (dotnet#1222)
  * xamarin/monodroid@489a389d1: [tools/msbuild] Check device additional output to see if its the same device. (dotnet#1218)
  * xamarin/monodroid@209a7c352: [tests/AndroidMSBuildTests] Remove Unused Unit Tests (dotnet#1215)
jonpryor added a commit that referenced this pull request Aug 27, 2021
Changes: xamarin/monodroid@fb0d502...5676b84

  * xamarin/monodroid@5676b84b4: Bump to xamarin/androidtools@0abc0d7c (#1223)
  * xamarin/monodroid@daf1aa909: [optimization] Replace 'new T[0]' with 'Array.Empty<T> ()' to reduce allocations. (#1221)
  * xamarin/monodroid@fb848118b: [tools/msbuild] Check `device.Properties.BuildVersionSdk` for `-1` (#1222)
  * xamarin/monodroid@489a389d1: [tools/msbuild] Check device additional output to see if its the same device. (#1218)
  * xamarin/monodroid@209a7c352: [tests/AndroidMSBuildTests] Remove Unused Unit Tests (#1215)
@github-actions github-actions bot locked and limited conversation to collaborators Feb 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants