-
Notifications
You must be signed in to change notification settings - Fork 564
[Xamarin.Android.Build.Tasks] Run aapt2 compile incrementally.
#4190
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
Conversation
|
@jonathanpeppers should we make this feature optional? So users can turn it on/off. Also should we restrict this only to app projects? |
jonathanpeppers
left a comment
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.
@dellis1972 I think this could be just the new behavior if all existing projects we know about continue to work. If you reopen this PR on a branch on the xamarin org, the QA tests will be able to run (they can't run on a fork 😞 ).
I think it would also be good to write a test checking if the target builds partially: https://github.com/xamarin/xamarin-android/blob/e2e1a846ca6136f0a3bac5ffb48277938ed5c204/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Common/BuildOutput.cs#L65-L76
Yup, this is till a work in progress so I'll get to that too :) |
059f1aa to
38edb12
Compare
|
/azp run |
|
Pull request contains merge conflicts. |
362a131 to
5b0ff79
Compare
|
Windows tests are failing with so it looks like the daemon mode does not work with long paths on windows 🤦♂ |
6851928 to
9182c08
Compare
jonathanpeppers
left a comment
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.
Should the first use of this daemon be behind a feature flag? Or is it definitely going to be better to use by default?
95f8683 to
a7ef6e0
Compare
|
/azp run |
|
Pull request contains merge conflicts. |
5de7e92 to
9e075a9
Compare
8a50c0f to
1b3f666
Compare
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
1b3f666 to
210eb03
Compare
922bd62 to
802f0d3
Compare
|
@jonathanpeppers ok we are down to 1 failure its the profile AOT one. |
802f0d3 to
c01098f
Compare
The `aapt2 compile` command runs in two modes. The one we currently use is the `archive` mode. We calling `aapt2 compile` with the `--dir` argument you end up generating one `flata` archive for all the files. The side effect of this is that even if you only change one file, it will need to regenerate the entire `flata` archive. But it has a second mode. Rather than using `--dir` you just send in a single file. This then writes a single `.flat` file to the output directory. While this does mean you have to call `aapt2 compile` for EVERY file, it does mean we can leverage MSbuilds support for partial targets. This means MSbuild will detect ONLY the files which changed and allow us to call `aapt2 compile` with just THOSE files. One exception to this new system are references which use the `AndroidSkipResourceProcessing` metadata. In those cases the chance of those libraries being updated on a regular basis is quite low. So in that case using a `flata` archive will be better since the files won't be changing much. While this may impact on initial build times, the goal is to make incremental builds quicker. This is especially true for users to use ALLOT of `AndroidResource` items. A note regarding the `aapt2 daemon` mode. In order to write accented characters we need to set the `StandardInput` encoding to UTF8. This is not possible directly in netstandard 2.0. So we have to use `Console.InputEncoding` instead. Also not that we MUST not include a BOM when writting the commands. This is because `aapt2` will try to parse the BOM as command characters. Also the `aapt2 link` command sometimes reports it is "Done" before it has even written the archive for the file. So we can get into a position where we think we are done but the file is not on disk. So we have had to include a nasty wait which will poll for the existence of the expected output file and only return when it exists. The good news is we know at this point if the command failed or not, so we can bypass the check on failure.
c01098f to
ac32ffd
Compare
|
Draft release notes Here is the note I'm thinking of including for this pull request: To suggest something different, feel free to reply in this PR or add a |
|
Is it possible to disable this new feature? We suspect this feature to break the functionality of ResizetizerNT and since it's apparently impossible to downgrade Xamarin.Android after a Visual Studio for Mac upgrade we are not able to build the Android app. Edit: Apparently it's possible to downgrade Xamarin.Android by installing the old pkg. Apparently it is related to this, ResizetizerNT works fine with 10.3 |
|
@acuntex its is not possible to disable this feature, and I can't see how it would effect ResizetizerNT. I would recommend you open an Issue in this repo https://github.com/xamarin/xamarin-android/issues/new/choose . Provide as much detail as possible including diagnostic build logs which show the error you are getting. |
The
aapt2 compilecommand runs in two modes. The one wecurrently use is the
archivemode. We callingaapt2 compilewith the
--dirargument you end up generating oneflataarchive for all the files. The side effect of this is that even
if you only change one file, it will need to regenerate the
entire
flataarchive.But it has a second mode. Rather than using
--diryou just sendin a single file. This then writes a single
.flatfile to theoutput directory. While this does mean you have to call
aapt2 compilefor EVERY file, it does mean we can leverage MSbuilds support for
partial targets. This means MSbuild will detect ONLY the files which
changed and allow us to call
aapt2 compilewith just THOSE files.While this may impact on initial build times, the goal is to make
incremental builds quicker. This is especially true for users to
use ALLOT of
AndroidResourceitems.A note regarding the
aapt2 daemonmode. In order to writeaccented characters we need to set the
StandardInputencoding to UTF8. This is not possible directly
in netstandard 2.0. So we have to use
Console.InputEncodinginstead. Also not that we MUST not include a BOM when writting
the commands. This is because
aapt2will try to parse the BOMas command characters.
Also the
aapt2 linkcommand sometimes reports it is "Done" beforeit has even written the archive for the file. So we can get into a
position where we think we are done but the file is not on disk.
So we have had to include a nasty wait which will poll for the
existence of the expected output file and only return when it
exists. The good news is we know at this point if the command
failed or not, so we can bypass the check on failure.