[Xamarin.Android.Build.Tasks] more fixes for parallel builds #2063
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Context: dotnet#2043 Another issue that could crop up from dotnet#2043 is: - Two instances of `ResolveLibraryProjectImports` are run in parallel - A file is written to at the same time, causing invalid zip files in `obj/Debug/lp`, such as a `classes.jar` file An example error message would be: error APT0000: ERROR: unable to open 'obj/Debug/lp/7/jl/bin/classes.jar' as a zip file: -78 "classes.jar' as a zip file: -78". Checking the zip file indicates it is corrupted: $ zipinfo bin/TestDebug/temp/BuildInParallel/obj/Debug/lp/7/jl/bin/classes.jar Archive: bin/TestDebug/temp/BuildInParallel/obj/Debug/lp/7/jl/bin/classes.jar [bin/TestDebug/temp/BuildInParallel/obj/Debug/lp/7/jl/bin/classes.jar] End-of-central-directory signature not found. Either this file is not a zipfile, or it constitutes one disk of a multi-part archive. In the latter case the central directory and zipfile comment will be found on the last disk(s) of this archive. zipinfo: cannot find zipfile directory in one of bin/TestDebug/temp/BuildInParallel/obj/Debug/lp/7/jl/bin/classes.jar or bin/TestDebug/temp/BuildInParallel/obj/Debug/lp/7/jl/bin/classes.jar.zip, and cannot find bin/TestDebug/temp/BuildInParallel/obj/Debug/lp/7/jl/bin/classes.jar.ZIP, period. So an improvement I made to `ResolveLibraryProjectImports`: - Extract to a temp file - Use `CopyIfChanged` to the destination - Delete the temp file This is generally *safer*, and a good change, in general. However... When running the `BuildInParallel` test on Mac, it seems to randomly fail in other MSBuild tasks! If I run the test several times, it seems to fail about 1 in 3... So for now, I think we should ignore the `BuildInParallel` test on Mac and come back to this later. It would be *nice* to have a completely robust build that won't break when run in parallel. Maybe should be something we work on in the future?
dellis1972
approved these changes
Aug 14, 2018
Contributor
|
Looks ok to me, once its green it should be ok to merge. |
Contributor
|
The error in the build does not seem related to this PR, so I'm merging. |
jonpryor
pushed a commit
that referenced
this pull request
Aug 15, 2018
Context: #2043 Another issue that could crop up from #2043 is: - Two instances of `ResolveLibraryProjectImports` are run in parallel - A file is written to at the same time, causing invalid zip files in `obj/Debug/lp`, such as a `classes.jar` file An example error message would be: error APT0000: ERROR: unable to open 'obj/Debug/lp/7/jl/bin/classes.jar' as a zip file: -78 "classes.jar' as a zip file: -78". Checking the zip file indicates it is corrupted: $ zipinfo bin/TestDebug/temp/BuildInParallel/obj/Debug/lp/7/jl/bin/classes.jar Archive: bin/TestDebug/temp/BuildInParallel/obj/Debug/lp/7/jl/bin/classes.jar [bin/TestDebug/temp/BuildInParallel/obj/Debug/lp/7/jl/bin/classes.jar] End-of-central-directory signature not found. Either this file is not a zipfile, or it constitutes one disk of a multi-part archive. In the latter case the central directory and zipfile comment will be found on the last disk(s) of this archive. zipinfo: cannot find zipfile directory in one of bin/TestDebug/temp/BuildInParallel/obj/Debug/lp/7/jl/bin/classes.jar or bin/TestDebug/temp/BuildInParallel/obj/Debug/lp/7/jl/bin/classes.jar.zip, and cannot find bin/TestDebug/temp/BuildInParallel/obj/Debug/lp/7/jl/bin/classes.jar.ZIP, period. So an improvement I made to `ResolveLibraryProjectImports`: - Extract to a temp file - Use `CopyIfChanged` to the destination - Delete the temp file This is generally *safer*, and a good change, in general. However... When running the `BuildInParallel` test on Mac, it seems to randomly fail in other MSBuild tasks! If I run the test several times, it seems to fail about 1 in 3... So for now, I think we should ignore the `BuildInParallel` test on Mac and come back to this later. It would be *nice* to have a completely robust build that won't break when run in parallel. Maybe should be something we work on in the future?
jonathanpeppers
added a commit
to jonathanpeppers/xamarin-android
that referenced
this pull request
Aug 23, 2018
…otnet#2063)" This *partially* reverts commit 73f7f47. We want the change to the test, because it was failing on MacOS. However, I noticed a performance regression in `ResolveLibraryProjectImports`. It went from ~1 sec to ~3 sec for me. Since we shouldn't make our build times *worse*, let's revert this change for now. It only *helped* parallel builds on Mac, but did not fully fix them.
jonathanpeppers
added a commit
that referenced
this pull request
Aug 23, 2018
…2063)" (#2098) This *partially* reverts commit 73f7f47. We want the change to the test, because it was failing on MacOS. However, I noticed a performance regression in `ResolveLibraryProjectImports`. It went from ~1 sec to ~3 sec for me. Since we shouldn't make our build times *worse*, let's revert this change for now. It only *helped* parallel builds on Mac, but did not fully fix them.
jonathanpeppers
added a commit
that referenced
this pull request
Aug 23, 2018
…2063)" (#2098) This *partially* reverts commit 73f7f47. We want the change to the test, because it was failing on MacOS. However, I noticed a performance regression in `ResolveLibraryProjectImports`. It went from ~1 sec to ~3 sec for me. Since we shouldn't make our build times *worse*, let's revert this change for now. It only *helped* parallel builds on Mac, but did not fully fix them.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Context: #2043
Another issue that could crop up from #2043 is:
ResolveLibraryProjectImportsare run in parallelobj/Debug/lp, such as aclasses.jarfileAn example error message would be:
Checking the zip file indicates it is corrupted:
So an improvement I made to
ResolveLibraryProjectImports:CopyIfChangedto the destinationThis is generally safer, and a good change, in general.
However...
When running the
BuildInParalleltest on Mac, it seems to randomlyfail in other MSBuild tasks!
If I run the test several times, it seems to fail about 1 in 3...
So for now, I think we should ignore the
BuildInParalleltest on Macand come back to this later.
It would be nice to have a completely robust build that won't break
when run in parallel. Maybe should be something we work on in the
future?