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

d8/r8 can give lots of warnings about MANIFEST.MF #3622

Closed
jonathanpeppers opened this issue Sep 12, 2019 · 23 comments · Fixed by #4001
Closed

d8/r8 can give lots of warnings about MANIFEST.MF #3622

jonathanpeppers opened this issue Sep 12, 2019 · 23 comments · Fixed by #4001
Assignees
Labels
Area: App+Library Build Issues when building Library projects or Application projects.

Comments

@jonathanpeppers
Copy link
Member

Context: #2939 (comment)

Steps to Reproduce

  1. In some apps you get multiple warnings such as:
R8 : warning : Resource 'META-INF/MANIFEST.MF' already exists.

I think you might see them with a Xamarin.Forms template even, just enable d8/r8.

Expected Behavior

R8 shouldn't produce warnings by default.

Actual Behavior

We get warnings!

Version Information

Happens with 16.2, and probably all versions.

@jonathanpeppers jonathanpeppers added the Area: App+Library Build Issues when building Library projects or Application projects. label Sep 12, 2019
@jonathanpeppers jonathanpeppers added this to the Under Consideration milestone Sep 12, 2019
@jonathanpeppers jonathanpeppers self-assigned this Sep 12, 2019
@jonathanpeppers jonathanpeppers changed the title d8/r8 can give lots of warnings about d8/r8 can give lots of warnings about MANIFEST.MF Sep 12, 2019
@KennyDizi
Copy link

thank you @jonathanpeppers !

@gmck
Copy link

gmck commented Sep 12, 2019

I also get a number of those and additionally META-INF/MSFTSIG.SF and META-INF/MSFTSIG.RSA. No Forms in my stuff though.

@Afshin1980
Copy link

Same problem!

@jbtdevgit
Copy link

jbtdevgit commented Oct 1, 2019

I also have -keep public class *extends androidx.versionedparcelable.VersionedParcelable encountered when r8 is enabled

@cosminstirbu
Copy link

Same problem

@DevEddy
Copy link

DevEddy commented Oct 8, 2019

Same here

4 similar comments
@geseru
Copy link

geseru commented Oct 10, 2019

Same here

@Bustillox
Copy link

Same here

@andreas-gabriel
Copy link

Same here

@SavikPavel
Copy link

Same here

jonathanpeppers added a commit to jonathanpeppers/xamarin-android that referenced this issue Dec 6, 2019
Fixes: dotnet#3622
Context: https://r8.googlesource.com/r8/

Just building a Xamairn.Forms template with multi-dex enabled produces
the warning:

    R8 : warning : Resource 'META-INF/MANIFEST.MF' already exists.

There are also other reports of:

    R8 : warning : Resource 'META-INF/MSFTSIG.SF' already exists.
    R8 : warning : Resource 'META-INF/MSFTSIG.RSA' already exists.

Reviewing r8's source code, I see no way to prevent it from emitting
these messages.

For now, it seems simple enough to add a `Regex` for this message and
downgrade it to a regular MSBuild message.

I added a test for this scenario as well, however I was also getting
this message from r8:

    Warning: The rule `-keep public class * extends java.lang.annotation.Annotation { *; }` uses extends but actually matches implements.

I don't think we should downgrade this one, yet... I reworked the test
so allow 1 warning and used a property to disable a warning coming
from Xamarin.Build.Download.
jonathanpeppers added a commit to jonathanpeppers/xamarin-android that referenced this issue Dec 9, 2019
Fixes: dotnet#3622
Context: https://r8.googlesource.com/r8/

Just building a Xamairn.Forms template with multi-dex enabled produces
the warning:

    R8 : warning : Resource 'META-INF/MANIFEST.MF' already exists.

There are also other reports of:

    R8 : warning : Resource 'META-INF/MSFTSIG.SF' already exists.
    R8 : warning : Resource 'META-INF/MSFTSIG.RSA' already exists.

Reviewing r8's source code, I see no way to prevent it from emitting
these messages.

For now, it seems simple enough to add a `Regex` for this message and
downgrade it to a regular MSBuild message.

I added a test for this scenario as well, however I was also getting
this message from r8:

    Warning: The rule `-keep public class * extends java.lang.annotation.Annotation { *; }` uses extends but actually matches implements.

I don't think we should downgrade this one, yet... I reworked the test
so allow 1 warning and used a property to disable a warning coming
from Xamarin.Build.Download.
jonathanpeppers added a commit that referenced this issue Dec 9, 2019
…ges (#4001)

Fixes: #3622
Context: https://r8.googlesource.com/r8/

Just building a Xamairn.Forms template with multi-dex enabled produces
the warning:

    R8 : warning : Resource 'META-INF/MANIFEST.MF' already exists.

There are also other reports of:

    R8 : warning : Resource 'META-INF/MSFTSIG.SF' already exists.
    R8 : warning : Resource 'META-INF/MSFTSIG.RSA' already exists.

Reviewing r8's source code, I see no way to prevent it from emitting
these messages.

For now, it seems simple enough to add a `Regex` for this message and
downgrade it to a regular MSBuild message.

I added a test for this scenario as well, however I was also getting
this message from r8:

    Warning: The rule `-keep public class * extends java.lang.annotation.Annotation { *; }` uses extends but actually matches implements.

I don't think we should downgrade this one, yet... I reworked the test
so allow 1 warning and used a property to disable a warning coming
from Xamarin.Build.Download.
jonpryor pushed a commit that referenced this issue Dec 12, 2019
…ges (#4001)

Fixes: #3622
Context: https://r8.googlesource.com/r8/

Just building a Xamairn.Forms template with multi-dex enabled produces
the warning:

    R8 : warning : Resource 'META-INF/MANIFEST.MF' already exists.

There are also other reports of:

    R8 : warning : Resource 'META-INF/MSFTSIG.SF' already exists.
    R8 : warning : Resource 'META-INF/MSFTSIG.RSA' already exists.

Reviewing r8's source code, I see no way to prevent it from emitting
these messages.

For now, it seems simple enough to add a `Regex` for this message and
downgrade it to a regular MSBuild message.

I added a test for this scenario as well, however I was also getting
this message from r8:

    Warning: The rule `-keep public class * extends java.lang.annotation.Annotation { *; }` uses extends but actually matches implements.

I don't think we should downgrade this one, yet... I reworked the test
so allow 1 warning and used a property to disable a warning coming
from Xamarin.Build.Download.
@tranb3r
Copy link

tranb3r commented Jan 10, 2020

@jonathanpeppers
Is this issue supposed to be fixed in xamarin.android 10.2.0.16 ?
I'm still having these warnings with vs 2019 16.5.0-pre1.0.

In your commit, I can see that the message used in the regex is "Warning: Resource.+already exists", while the one being shown during build is actually "warning : Resource.+already exists".
Notice the missing space between "warning" and ":". Is this a mistake ?

Thanks

@jonathanpeppers
Copy link
Member Author

@tranb3r can you post your build log?

https://docs.microsoft.com/en-us/xamarin/android/troubleshooting/troubleshooting#diagnostic-msbuild-output

I modified a test so it got the same warning, and the warning went away. But it's possible something is different for you?

@tranb3r
Copy link

tranb3r commented Jan 10, 2020

Sure. Here it is (only the r8 task):
build_r8.log

@jonathanpeppers
Copy link
Member Author

@tranb3r are these showing up as warnings in the IDE's error pad? Have a screenshot? These will still get logged, just as messages though.

@tranb3r
Copy link

tranb3r commented Jan 11, 2020

@jonathanpeppers
No, these lines are not showing up as warnings in the IDE. But they are showing up in the build log.
Is this what this issue is about, removing the warning from the IDE, even if they still show up in the build log ?
I was hoping that the root cause for those warnings in R8 would be fixed...

@jonathanpeppers
Copy link
Member Author

I was hoping that the root cause for those warnings in R8 would be fixed...

R8 appears to warn if the same file is encountered in multiple jar files:

Resource 'META-INF/MANIFEST.MF' already exists.

I don't see the need for R8 to warn about this, but that is what it does.

I reviewed R8's source code and could not find a way for us to suppress the warning:

Feel free to review and let us know if you see something we could do differently (or open a PR!). Thanks.

@tranb3r
Copy link

tranb3r commented Jan 14, 2020

I've added the "--no-data-resources" r8 argument and the "resource already exists" warnings are gone (and the output classes.dex is exactly the same).
@jonathanpeppers Do you know what is the impact of adding this argument ? Could it be the default for xamarin.android ?

@jonathanpeppers
Copy link
Member Author

It doesn't seem to be documented very well as to what it does:

> java -jar .\bin\Debug\lib\xamarin.android\xbuild\Xamarin\Android\r8.jar --help
Usage: r8 [options] <input-files>
 where <input-files> are any combination of dex, class, zip, jar, or apk files
 and options are:
...
  --no-data-resources      # Ignore all data resources.

Thanks, Google!

Reading the source code, I found a comment that gives slightly more info:

/**
 * Set the output path-and-mode and control if data resources are included.
 *
 * <p>In addition to setting the output path-and-mode (see {@link #setOutput(Path, OutputMode)})
 * this can control if data resources should be included or not.
 *
 * <p>Data resources are non Java classfile items in the input.
 *
 * <p>If data resources are not included they are ignored in the input and will not produce
 * anything in the output. If data resources are included they are processed according to the
 * configuration and written to the output.
 *
 * @param outputPath Path to write the output to. Must be an archive or and existing directory.
 * @param outputMode Mode in which to write the output.
 * @param includeDataResources If data resources from the input should be included in the
 *     output.
 */

The default value of includeDataResources is true and --no-data-resources sets it to false.

R8 has an option to output java output instead of dex (Android-specific). It seems like that option would only apply to java output. I don't think you can put arbitrary files in a dex. Our build system puts these extra files in the APK later on:

https://github.com/xamarin/xamarin-android/blob/e22be4912e498c705e89acc4e2fc3af8a0a9a8c4/src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs#L175-L201

Let me do some other research on this.

@KennyDizi
Copy link

thanks @jonathanpeppers

@brendanzagaeski
Copy link
Contributor

brendanzagaeski commented Jan 23, 2020

Release status update

A new Preview version has now been published that includes the change to make this warning an informational message so that it will no longer appear in the Error List. That change is not yet included in a Release version. I will update this item again when a Release version is available that includes that change in behavior.

Separate from that first change, investigation will continue on possible further improvements.

Change included in Xamarin.Android 10.2.0.84.

Change included on Windows in Visual Studio 2019 version 16.5 Preview 2. To try the Preview version that includes the change, check for the latest updates in Visual Studio Preview.

Change included on macOS in Visual Studio 2019 for Mac version 8.5 Preview 1. To try the Preview version that includes the change, check for the latest updates on the Preview updater channel.

@brendanzagaeski
Copy link
Contributor

Release status update

A new Release version has now been published on Windows that includes the change to make this warning an informational message so that it will no longer appear in the Error List. That change is not yet published in a Release version on macOS. I will update this item again when a Release version is available on macOS that change in behavior.

Separate from that first change, investigation will continue on possible further improvements.

Change included in Xamarin.Android 10.2.0.100.

Change included on Windows in Visual Studio 2019 version 16.5. To get the new version that includes the change, check for the latest updates or install the latest version from https://visualstudio.microsoft.com/downloads/.

(Change also included on macOS in Visual Studio 2019 for Mac version 8.5 Preview 1 and higher. To try the Preview version that includes the change, check for the latest updates on the Preview updater channel.)

@brendanzagaeski
Copy link
Contributor

Release status update

A new Release version has now been published on macOS that includes the change to make this warning an informational message so that it will no longer appear in the Error List.

Separate from that first change, investigation will continue on possible further improvements.

Change included in Xamarin.Android 10.2.0.100.

Change included on macOS in Visual Studio 2019 for Mac version 8.5. To get the new version that includes the change, check for the latest updates on the Stable updater channel.

(Change also included on Windows in Visual Studio 2019 version 16.5 and higher. To get the new version that includes the change, check for the latest updates or install the latest version from https://visualstudio.microsoft.com/downloads/.)

@KennyDizi
Copy link

Kudos

@ghost ghost locked as resolved and limited conversation to collaborators Jun 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Area: App+Library Build Issues when building Library projects or Application projects.
Projects
None yet