[Xamarin.Android.Build.Tasks] Rework Aapt log processing. #1153
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
dellis1972
added
Area: xamarin-android Build
jonpryor
reviewed
Jan 4, 2018
| } | ||
|
|
||
| bool RunAapt (string commandLine) | ||
| bool RunAapt (string commandLine, IList<Tuple<string, bool>> output) |
Contributor
There was a problem hiding this comment.
Philosophically... Tuple<string, bool>?! I wonder if we can use ValueTuple, and at least have (string message, bool stderr)?
Otherwise, really not a fan of tuples.
If we can't use (string message, bool stderr), perhaps a new struct would be better?
jonpryor
reviewed
Jan 4, 2018
| var output = new List<Tuple<string, bool>> (); | ||
| var ret = RunAapt (cmd, output); | ||
| foreach (var line in output) { | ||
| if (line.Item2) { |
Contributor
There was a problem hiding this comment.
...and this right here is why I'm not fond of Tuple<string, bool>. line.stderr would be so much more readable.
Not a chance I'm going to remember that Item2 is whether the message came from stdout vs. stderr.
Contributor
|
Aside: I thought one of the related ideas was to "ignore" the |
Context dotnet#1134 Aapt is pretty inconsistent with how it reports errors and warnings. Warnings are written to stderr.. So we are reworking the output processing for aapt to hopefully handle errors are warnings in a more consistent way. Rather than process the output in realtime we will buffer it and process the output once aapt has completed. This will allow us to check to see if the required output file (R.java) was created. If it wasn't then we can assume that the process failed. We can then process the output knowing that we are expecting and error. In the case where R.java does exist, we can assume if we do find anything which looks like an error it is more than likely a warning.
Contributor
|
Re-worked commit message for merging: |
jonpryor
pushed a commit
that referenced
this pull request
Jan 4, 2018
Fixes: #1134 `aapt` is pretty inconsistent with how it reports errors and warnings; for example, warnings are written to stderr. This is a bit of a conundrum: * Messages *may* contain `error` or `warning`, but they might not (no level specified; 2135856). * Messages written to stderr may be warnings, *not* errors. * The `aapt` return value may be non-zero when warnings are written but output files are still generated. In particular, consider Issue #1134: `aapt` writes to stderr: max res 10, skipping values-sw720dp-land-v13 "max res 10, skipping values-sw720dp-land-v13". This was previously being reported as an error, "simply" because it was written to stderr, but the above is *not* an error. Rework `aapt` output processing to hopefully handle errors and warnings in a more consistent way. Instead of processing the output in realtime we will instead buffer it and process the output once `aapt` has completed. This will allow us to check to see if the required output file (`R.java`) was created. If `R.java` *is* created, messages written to stderr without an explicit level will be treated as warnings. If `R.java` is *not* created, messages written to stderr without an explicit level will be treated as *errors*.
jonpryor
pushed a commit
that referenced
this pull request
Jan 28, 2021
Changes: xamarin/monodroid@ad19471...bafd139 * xamarin/monodroid@bafd13939: [tools/msbuild] Fix DateTimeOffset error (#1158) * xamarin/monodroid@b355593ab: [tools/msbuild] Use millisecond resoluton for Fast Deployment (#1157) * xamarin/monodroid@34ae2a311: [tools/msbuild] add %(TargetPath) metadata for <FastDeploy/> (#1156) * xamarin/monodroid@d9fcaa019: [tools/msbuild] Fix Time resoluton for Fast Deployment (#1154) * xamarin/monodroid@390c048ca: [msbuild] Use `$(LZ4PackageVersion)` (#1153)
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 #1134
Aapt is pretty inconsistent with how it reports errors and
warnings. Warnings are written to stderr..
So we are reworking the output processing for aapt to hopefully
handle errors are warnings in a more consistent way. Rather than
process the output in realtime we will buffer it and process the
output once aapt has completed. This will allow us to check to
see if the required output file (R.java) was created. If it
wasn't then we can assume that the process failed.
We can then process the output knowing that we are expecting
and error. In the case where R.java does exist, we can assume
if we do find anything which looks like an error it is more
than likely a warning.