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] deletebinobj fix for resources #2199

Merged
merged 1 commit into from
Sep 20, 2018

Conversation

jonathanpeppers
Copy link
Member

@jonathanpeppers jonathanpeppers commented Sep 19, 2018

Fixes: #2193

I have been able to reproduce a #deletebinobj bug with the following
steps:

  1. File | New Project | Android App (Xamarin) | Tabbed App, set
    Minimum Android Version to Android 5.0 (Lollipop)
  2. Build
  3. Add a new TextView to Resources\layout\activity_main.axml, with
    an id of textView1
  4. Build and Deploy

Get a crash at runtime:

Android.Views.InflateException: Binary XML file line #1: Error inflating class android.support.design.widget.BottomNavigationView occurred

A Rebuild fixes the problem, and the app starts correctly again.

After comparing obj directories from before and after, I noticed
obj\Debug\android\src\android\support\compat\R.java was missing the
field for R.id.textView1!

public static int textView1=0x7f080091;

This doesn't match the error message we are getting here at all... But
this file is updated by the _GenerateJavaDesignerForComponent
MSBuild target. Further research showed that this target was getting
skipped at step no. 4 above, because it was found to be up to date
according to its inputs and outputs.

To verify this was the case, I could delete
obj\Debug\Component.R.cs.flag which would also resolve the runtime
exception (instead of a full Rebuild).

First, I created a new test:

  • CustomViewAddResourceId builds a project with
    <android.support.design.widget.BottomNavigationView />
  • Adds a textView1 layout, builds again
  • Verifies that obj\Debug\android\src\android\support\compat\R.java
    contains textView1

To fix the problem, I did the following:

  • Added $(_AndroidResgenFlagFile) as an input to
    _GenerateJavaDesignerForComponent, so it will run again when
    _UpdateAndroidResgen runs
  • Removed
    AndroidComponentResgenFlagFile="$(_AndroidComponentResgenFlagFile)"
    from the call to the <Aapt /> MSBuild task, so it will re-run
    aapt and generate R.java.

However, things were breaking when $(AndroidUseAapt2) was
enabled.

For aapt2 support I had to:

  • Remove AndroidComponentResgenFlagFile from the <Aapt2Link />
    call
  • Set an extra property that the other <Aapt2Link /> calls are
    already doing:
    CompiledResourceFlatArchive="$(IntermediateOutputPath)\compiled.flata"

@jonathanpeppers jonathanpeppers added the do-not-merge PR should not be merged. label Sep 19, 2018
@dellis1972
Copy link
Contributor

dellis1972 commented Sep 19, 2018

For aapt2 we will probably need to re-run _CompileAndroidLibraryResources to re-genreate the .flata

Fixes: dotnet#2193

I have been able to reproduce a #deletebinobj bug with the following
steps:
1. `File | New Project | Android App (Xamarin) | Tabbed App`, set
   `Minimum Android Version` to `Android 5.0 (Lollipop)`
2. Build
3. Add a new `TextView` to `Resources\layout\activity_main.axml`, with
   an id of `textView1`
4. Build and Deploy

Get a crash at runtime:

    Android.Views.InflateException: Binary XML file line #1: Error inflating class android.support.design.widget.BottomNavigationView occurred

A `Rebuild` fixes the problem, and the app starts correctly again.

After comparing `obj` directories from before and after, I noticed
`obj\Debug\android\src\android\support\compat\R.java` was missing the
field for `R.id.textView1`!

    public static int textView1=0x7f080091;

This doesn't match the error message we are getting here at all... But
this file is updated by the `_GenerateJavaDesignerForComponent`
MSBuild target. Further research showed that this target was getting
skipped at step no. 4 above, because it was found to be up to date
according to its inputs and outputs.

To verify this was the case, I could delete
`obj\Debug\Component.R.cs.flag` which would also resolve the runtime
exception (instead of a full `Rebuild`).

First, I created a new test:
- `CustomViewAddResourceId` builds a project with
  `<android.support.design.widget.BottomNavigationView />`
- Adds a `textView1` layout, builds again
- Verifies that `obj\Debug\android\src\android\support\compat\R.java`
  contains `textView1`

To fix the problem, I did the following:
- Added `$(_AndroidResgenFlagFile)` as an input to
  `_GenerateJavaDesignerForComponent`, so it will run again when
  `_UpdateAndroidResgen` runs
- Removed
  `AndroidComponentResgenFlagFile="$(_AndroidComponentResgenFlagFile)"`
  from the call to the `<Aapt />` MSBuild task, so it will re-run
  `aapt` and generate `R.java`.

However, things were breaking when `$(AndroidUseAapt2)` was
enabled.

For `aapt2` support I had to:
- Remove `AndroidComponentResgenFlagFile` from the `<Aapt2Link />`
  call
- Set an extra property that the other `<Aapt2Link />` calls are
  already doing:
  `CompiledResourceFlatArchive="$(IntermediateOutputPath)\compiled.flata"`
@jonathanpeppers jonathanpeppers removed the do-not-merge PR should not be merged. label Sep 20, 2018
@jonathanpeppers
Copy link
Member Author

Test failure is that random one we've been seeing:

AndroidClientHandlerTests.Mono.Android_Tests, Xamarin.Android.NetTests.AndroidClientHandlerTests.Sanity_Tls_1_2_Url_WithMonoClientHandlerFails / Debug
Error Message
SHOULD NOT BE REACHED: BTLS is present, TLS 1.2 should work.
Stacktrace
  at Xamarin.Android.NetTests.AndroidClientHandlerTests.Sanity_Tls_1_2_Url_WithMonoClientHandlerFails () [0x0006a] in <84ed5a1e46be43709455c56786f89e5d>:0 
  at (wrapper managed-to-native) System.Reflection.MonoMethod.InternalInvoke(System.Reflection.MonoMethod,object,object[],System.Exception&)
  at System.Reflection.MonoMethod.Invoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) [0x0003b] in <20ca3e4ca03b4176b79fc767df166383>:0 

Probably unrelated.

@dellis1972 dellis1972 merged commit 4deec52 into dotnet:master Sep 20, 2018
@jonathanpeppers jonathanpeppers deleted the deletebinobj branch September 20, 2018 16:10
jonathanpeppers added a commit to jonathanpeppers/xamarin-android that referenced this pull request Sep 21, 2018
Context: dotnet#2088
Context: dotnet#2129
Context: dotnet#2199

In 4deec52, I fixed the #deletebinobj problem we discovered.

However...

I introduced a regression to incremental builds, I noticed that the
`_CompileJava` target is now running on a second build with no
changes. Third build? oddly it gets skipped...

It seems to be due to our use of flag files:
1. `_UpdateAndroidResgen` updates `R.cs.flag` and uses the file as an
   output
2. `_GenerateJavaDesignerForComponent` now uses `R.cs.flag` as an
   input
3. `_GenerateJavaStubs` *also* updates the timestamp on `R.cs.flag`.
   This was added in 970da9e, as a workaround for our two instances
   of `ConvertResourcesCases`.
4. `_GenerateJavaDesignerForComponent` will now run again on the next
   build.

Since 1886e6f eliminated the second call to `ConvertResourcesCases`,
we don't need to update `R.cs.flag` in no. 3 any longer.

Removing the call to `<Touch />` `R.cs.flag` in `_GenerateJavaStubs`
fixed the issue, and I added some assertions in relevant tests to
check that the `_CompileJava` and `_GenerateJavaDesignerForComponent`
targets aren't running on incremental builds.
dellis1972 pushed a commit that referenced this pull request Sep 21, 2018
Context: #2088
Context: #2129
Context: #2199

In 4deec52, I fixed the #deletebinobj problem we discovered.

However...

I introduced a regression to incremental builds, I noticed that the
`_CompileJava` target is now running on a second build with no
changes. Third build? oddly it gets skipped...

It seems to be due to our use of flag files:
1. `_UpdateAndroidResgen` updates `R.cs.flag` and uses the file as an
   output
2. `_GenerateJavaDesignerForComponent` now uses `R.cs.flag` as an
   input
3. `_GenerateJavaStubs` *also* updates the timestamp on `R.cs.flag`.
   This was added in 970da9e, as a workaround for our two instances
   of `ConvertResourcesCases`.
4. `_GenerateJavaDesignerForComponent` will now run again on the next
   build.

Since 1886e6f eliminated the second call to `ConvertResourcesCases`,
we don't need to update `R.cs.flag` in no. 3 any longer.

Removing the call to `<Touch />` `R.cs.flag` in `_GenerateJavaStubs`
fixed the issue, and I added some assertions in relevant tests to
check that the `_CompileJava` and `_GenerateJavaDesignerForComponent`
targets aren't running on incremental builds.
dellis1972 pushed a commit that referenced this pull request Oct 3, 2018
Fixes: #2193

I have been able to reproduce a #deletebinobj bug with the following
steps:
1. `File | New Project | Android App (Xamarin) | Tabbed App`, set
   `Minimum Android Version` to `Android 5.0 (Lollipop)`
2. Build
3. Add a new `TextView` to `Resources\layout\activity_main.axml`, with
   an id of `textView1`
4. Build and Deploy

Get a crash at runtime:

    Android.Views.InflateException: Binary XML file line #1: Error inflating class android.support.design.widget.BottomNavigationView occurred

A `Rebuild` fixes the problem, and the app starts correctly again.

After comparing `obj` directories from before and after, I noticed
`obj\Debug\android\src\android\support\compat\R.java` was missing the
field for `R.id.textView1`!

    public static int textView1=0x7f080091;

This doesn't match the error message we are getting here at all... But
this file is updated by the `_GenerateJavaDesignerForComponent`
MSBuild target. Further research showed that this target was getting
skipped at step no. 4 above, because it was found to be up to date
according to its inputs and outputs.

To verify this was the case, I could delete
`obj\Debug\Component.R.cs.flag` which would also resolve the runtime
exception (instead of a full `Rebuild`).

First, I created a new test:
- `CustomViewAddResourceId` builds a project with
  `<android.support.design.widget.BottomNavigationView />`
- Adds a `textView1` layout, builds again
- Verifies that `obj\Debug\android\src\android\support\compat\R.java`
  contains `textView1`

To fix the problem, I did the following:
- Added `$(_AndroidResgenFlagFile)` as an input to
  `_GenerateJavaDesignerForComponent`, so it will run again when
  `_UpdateAndroidResgen` runs
- Removed
  `AndroidComponentResgenFlagFile="$(_AndroidComponentResgenFlagFile)"`
  from the call to the `<Aapt />` MSBuild task, so it will re-run
  `aapt` and generate `R.java`.

However, things were breaking when `$(AndroidUseAapt2)` was
enabled.

For `aapt2` support I had to:
- Remove `AndroidComponentResgenFlagFile` from the `<Aapt2Link />`
  call
- Set an extra property that the other `<Aapt2Link />` calls are
  already doing:
  `CompiledResourceFlatArchive="$(IntermediateOutputPath)\compiled.flata"`
jonathanpeppers added a commit to jonathanpeppers/xamarin-android that referenced this pull request Oct 10, 2018
@github-actions github-actions bot locked and limited conversation to collaborators Feb 2, 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.

DeleteBinObj bug with AndroidResource
2 participants