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

[Xamarin.Android.Build.Tasks] Parse AAPT errors so every build error isn't APT0000 #3577

Merged
merged 1 commit into from
Sep 6, 2019

Conversation

jpobst
Copy link
Contributor

@jpobst jpobst commented Sep 3, 2019

Error messages taken from https://gist.github.com/grendello/72c1ce5f6e8cdd3aefd393b983576d9a.

Created by grepping the aapt source for error.

}

static readonly List<Tuple<string, string>> error_codes = new List<Tuple<string, string>> () {
Tuple.Create ("AndroidManifest.xml is corrupt", "APT1100"),
Copy link
Member

Choose a reason for hiding this comment

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

For ease of human parsing, could the error number be first in the tuple, instead of second? That would allow all of the error messages and codes to line up vertically and be easier (for me) to read:

Tuple.Create ("APT1100", "AndroidManifest.xml is corrupt"),
Tuple.Create ("APT1001", "can't use '-u' with add"),
Tuple.Create ("APT1002", "dump failed because assets could not be loaded"),
// ...

Copy link
Member

Choose a reason for hiding this comment

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

Commit message/PR description should also provide where/how you got these strings. Grepping the aapt source (with what grep command)? Something else?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.

@jpobst jpobst marked this pull request as ready for review September 4, 2019 14:35
@jonpryor
Copy link
Member

jonpryor commented Sep 4, 2019

Unit test expected output needs to be updated so that unit tests still pass.

Consider: Xamarin.Android.Build.Tests.ManifestTest.Bug12935(False):

String did not contain 'APT0000: '!

That's because the new output is:

 .../Bug12935_False/obj/Release/android/manifest/AndroidManifest.xml(6): error APT1134: String types not allowed (at 'screenOrientation' with value 'sensorPortait')

@dellis1972
Copy link
Contributor

ok another slight problem... currently we document all of our error codes here. Do we need to auto generate a bunch of documents for these new error codes? I believe they use our docs to generate the docs on the VS site which the IDE links to if you want more help on an error.

@jpobst jpobst force-pushed the aapt-error-codes branch 2 times, most recently from ef39bcd to bfbf509 Compare September 5, 2019 15:29
@jonpryor jonpryor merged commit 2d400aa into master Sep 6, 2019
jonpryor pushed a commit that referenced this pull request Sep 6, 2019
Context: https://devdiv.visualstudio.com/DefaultCollection/DevDiv/_workitems/edit/735263
Context: https://android.googlesource.com/platform/frameworks/base/+/563abce4ed0a0d9f08f8839a8f15b548b3dcd4d1/tools/aapt
Context: https://gist.github.com/grendello/72c1ce5f6e8cdd3aefd393b983576d9a

Errors and warning *codes* for errors and warnings reported
*within Visual Studio* from developers participating in the
[Visual Studio Customer Experience Improvement Program][0] are
reported "telemetry" events, but *only* the error and warning codes
are reported.  The warning and error message contents are not sent.

Unfortunately all `aapt`-related errors are reported as APT0000,
which reduces the utility of telemetry, making it more difficult to
better understand where our build system could be improved.

@grendello created a `grep` pipeline for the `aapt` sources to try to
extract plausible error messages that we can compare against.  The
exact pipeline has been lost, but this is an in-progress pipeline
for finding error messages from the `aapt2` sources:

	rgrep -Pzoh '(?s)(->|\.)Error.*?;\n' * | \
	  tr -d '\n' | tr ';' '\n'| \
	  sed -e 's/^[ \t]*//g' -e 's/^.*Error([ \t]*DiagMessage[ \t]*(.*)[ \t]*<<//g' -e 's/^.*Error(.*)//g;' -e 's/^;$//g' | \
	  sort | uniq

Improve the `<Aapt/>` task to check against this list of "known"
error strings and associate them with unique `APTxxxx` codes.
This will allow for more meaningful documentation to be written
in the future, and will allow our telemetry to be more meaningful
and permit improved focusing of future efforts.

[0]: https://docs.microsoft.com/en-us/visualstudio/ide/visual-studio-experience-improvement-program?view=vs-2019
@jpobst jpobst deleted the aapt-error-codes branch September 6, 2019 19:46
@github-actions github-actions bot locked and limited conversation to collaborators Jan 29, 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.

5 participants