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

JDK 11 broke @(JavaDocJar) & related support #5474

Closed
jonpryor opened this issue Jan 7, 2021 · 2 comments · Fixed by #5479
Closed

JDK 11 broke @(JavaDocJar) & related support #5474

jonpryor opened this issue Jan 7, 2021 · 2 comments · Fixed by #5479
Assignees
Labels
Area: Bindings Issues in Java Library Binding projects. bug Component does not function as intended.

Comments

@jonpryor
Copy link
Member

jonpryor commented Jan 7, 2021

Context: #5468 (comment)

Commit 380e95e inadvertently broken @(JavaSourceJar) and @(JavaDocJar) support.

Steps to Reproduce

  1. Try the repro at Issue [Xamarin.Android.Build.Tasks] build error JavaSourceJar xxxx.stamp not exist #2745:
    a. Create a new Binding project.
    b. Download glide-3.7.0.jar, set its Build action to @(EmbeddedJar).
    c. Download glide-3.7.0-javadoc.jar, set its Build action to @(JavaDocJar)
  2. Build the project.

Scratch.Gxa2745.zip

Expected Behavior

The build should use the @(JavaDocJar) file. (Not necessarily "build successfully", but "do something with" the @(JavaDocJar) files.

Actual Behavior

The @(JavaDocJar) file isn't used.

Issue #2745 makes for an interesting comparison point: if the file were used, the build would fail with an MSB3375, unless the underlying bug was fixed (PR #4406?!), which hasn't happened.

Discussion

However, this is a regression introduced by the introduction of JDK 11 support in 380e95e, due to buggy JDK 1.8 detection.

If we build the project, we get binding-related errors:

$ msbuild /v:diag
…
BINDINGSGENERATOR : warning BG8604: top ancestor SupportRequestManagerFragment not found for nested type Com.Bumptech.Glide.Manager.SupportRequestManagerFragment._1.
…
obj/Debug/generated/src/Com.Bumptech.Glide.Load.Resource.Gif.GifResourceEncoder.cs(10,70): error CS0535: 'GifResourceEncoder' does not implement interface member 'IEncoder.
Encode(Object, Stream)' 

Sure, it errors, but at least we got to the point of building the binding!

However, if we "manually work around" the broken JDK 1.8 detection support by setting the $(_JavadocSupported) property to True, we in fact reproduce Issue #2745:

$ msbuild /p:_JavadocSupported=True /v:diag
…
…/Xamarin.Android.Bindings.Documentation.targets(31,5): error MSB3375: The file "obj/Debug/javadocs/glide-3.7.0-javadoc.stamp" does not exist. 

This proves three things:

  1. Commit 380e95e broke "out-of-the-box" $(JavaDocJar) support.
  2. Issue [Xamarin.Android.Build.Tasks] build error JavaSourceJar xxxx.stamp not exist #2745 has not actually been fixed
  3. generator output remains "less-than-ideal"
@jonpryor jonpryor added the Area: Bindings Issues in Java Library Binding projects. label Jan 7, 2021
@jonpryor jonpryor self-assigned this Jan 7, 2021
@jonpryor
Copy link
Member Author

jonpryor commented Jan 7, 2021

What's "funny" is a7413a2 removes the $(_JavadocSupported) property, so the "workaround to force Issue #2745 to occur" no longer works.

And Issue #2745 doesn't appear.

But, again, that's not because it's been fixed. It's because we seem to have broken @(JavaDocJar) support entirely, probably because of a7413a2.

On the plus side, @(JavaSourceJar) support is much better: drop in glide-3.7.0-sources.jar with a Build action of @(JavaSourceJar) in current master, and the resulting binding will contain C# XML Documentation Comments:

// File: obj/Debug/generated/src/Com.Bumptech.Glide.Manager.ILifecycle.cs

namespace Com.Bumptech.Glide.Manager {

	/// <summary>An interface for listening to Activity/Fragment lifecycle events.</summary>
	// Metadata.xml XPath interface reference: path="/api/package[@name='com.bumptech.glide.manager']/interface[@name='Lifecycle']"
	[Register ("com/bumptech/glide/manager/Lifecycle", "", "Com.Bumptech.Glide.Manager.ILifecycleInvoker")]
	public partial interface ILifecycle : IJavaObject, IJavaPeerable {
		// Metadata.xml XPath method reference: path="/api/package[@name='com.bumptech.glide.manager']/interface[@name='Lifecycle']/method[@name='addListener' and count(parameter)=1 and parameter[1][@type='com.bumptech.glide.manager.LifecycleListener']]"
		/// <summary>Adds the given listener to the set of listeners managed by this Lifecycle implementation.</summary>
		[Register ("addListener", "(Lcom/bumptech/glide/manager/LifecycleListener;)V", "GetAddListener_Lcom_bumptech_glide_manager_LifecycleListener_Handler:Com.Bumptech.Glide.Manager.ILifecycleInvoker, Scratch.Gxa2745")]
		void AddListener (global::Com.Bumptech.Glide.Manager.ILifecycleListener listener);

	}
}

which is really cool.

…but doesn't change the fact that @(JavaDocJar) is currently broken. 😞

@jonpryor
Copy link
Member Author

jonpryor commented Jan 7, 2021

I can cause Issue #2745 on 0f2dc44 by building with /p:_UseLegacyJavadocImport=True.

jonpryor added a commit to jonpryor/xamarin-android that referenced this issue Jan 7, 2021
Fixes: dotnet#2745
Fixes: dotnet#5474

Issue dotnet#2745 noted that `@(JavaDocJar)` didn't work; attempting to use
it would result in an MSB3375 error:

	…/Xamarin.Android.Bindings.Documentation.targets(31,5): error MSB3375: The file "obj/Debug/javadocs/glide-3.7.0-javadoc.stamp" does not exist.

This issue was subsequently hidden by 380e95e, which disabled the
`_ExtractJavaDocJars` target unless JDK 1.8 was being used.

There were two problems with 380e95e:

 1. The `_ExtractJavaDocJars` target didn't need to be disabled!
    It just unzips the `.jar`; it does not require JDK 1.8.

 2. The check for JDK 1.8 was bad, and was *never* True.

The bad JDK 1.8 check was later removed in a7413a2, which "fixed"
(2), but a7413a2 continued to disable `_ExtractJavaDocJars`,
preventing `@(JavaDocJar)` from being used.

The only documentation-related functionality that needed to be
disabled because of JDK 11 was `@(JavaSourceJar)`, which got a better
fix in commits a7413a2 and 0e95ec7.

Fix the `_ExtractJavaDocJars` target so that it properly runs when
there are any `@(JavaDocJar)` items, then fix the `_ExtractJavaDocJars`
target so that it doesn't emit an MSB3375 error.

Note: `@(JavaDocJar)` is still problematic: it involves parsing
Javadoc HTML, which contains numerous "dialects" (it's why the JDK 11
commit skipped `@(JavaSourceJar)` support: trying to update our HTML
scrapers was "too fiddly").

We will deprecate `@(JavaDocJar)` support in a future release.
`@(JavaSourceJar)` should be preferred.

Update the `BindingBuildTest.cs` infrastructure so that instead of
having `.jar` files encoded in Base64, we instead use
`@(EmbeddedResource)`s for the `.jar` files.  A new `UpdateResources`
target will update the appropriate `.jar` files.
jonpryor added a commit to jonpryor/xamarin-android that referenced this issue Jan 8, 2021
Fixes: dotnet#2745
Fixes: dotnet#5474

Issue dotnet#2745 noted that `@(JavaDocJar)` didn't work; attempting to use
it would result in an MSB3375 error:

	…/Xamarin.Android.Bindings.Documentation.targets(31,5): error MSB3375: The file "obj/Debug/javadocs/glide-3.7.0-javadoc.stamp" does not exist.

This issue was subsequently hidden by 380e95e, which disabled the
`_ExtractJavaDocJars` target unless JDK 1.8 was being used.

There were two problems with 380e95e:

 1. The `_ExtractJavaDocJars` target didn't need to be disabled!
    It just unzips the `.jar`; it does not require JDK 1.8.

 2. The check for JDK 1.8 was bad, and was *never* True.

The bad JDK 1.8 check was later removed in a7413a2, which "fixed"
(2), but a7413a2 continued to disable `_ExtractJavaDocJars`,
preventing `@(JavaDocJar)` from being used.

The only documentation-related functionality that needed to be
disabled because of JDK 11 was `@(JavaSourceJar)`, which got a better
fix in commits a7413a2 and 0e95ec7.

Fix the `_ExtractJavaDocJars` target so that it properly runs when
there are any `@(JavaDocJar)` items, then fix the `_ExtractJavaDocJars`
target so that it doesn't emit an MSB3375 error.

Note: `@(JavaDocJar)` is still problematic: it involves parsing
Javadoc HTML, which contains numerous "dialects" (it's why the JDK 11
commit skipped `@(JavaSourceJar)` support: trying to update our HTML
scrapers was "too fiddly").

We will deprecate `@(JavaDocJar)` support in a future release.
`@(JavaSourceJar)` should be preferred.

Update the `BindingBuildTest.cs` infrastructure so that instead of
having `.jar` files encoded in Base64, we instead use
`@(EmbeddedResource)`s for the `.jar` files.  A new `UpdateResources`
target will update the appropriate `.jar` files.
@jonpryor jonpryor added this to the d16-9 milestone Jan 8, 2021
jonpryor added a commit to jonpryor/xamarin-android that referenced this issue Jan 8, 2021
Fixes: dotnet#2745
Fixes: dotnet#5474

Issue dotnet#2745 noted that `@(JavaDocJar)` didn't work; attempting to use
it would result in an MSB3375 error:

	…/Xamarin.Android.Bindings.Documentation.targets(31,5): error MSB3375: The file "obj/Debug/javadocs/glide-3.7.0-javadoc.stamp" does not exist.

This issue was subsequently hidden by 380e95e, which disabled the
`_ExtractJavaDocJars` target unless JDK 1.8 was being used.

There were two problems with 380e95e:

 1. The `_ExtractJavaDocJars` target didn't need to be disabled!
    It just unzips the `.jar`; it does not require JDK 1.8.

 2. The check for JDK 1.8 was bad, and was *never* True.

The bad JDK 1.8 check was later removed in a7413a2, which "fixed"
(2), but a7413a2 continued to disable `_ExtractJavaDocJars`,
preventing `@(JavaDocJar)` from being used.

The only documentation-related functionality that needed to be
disabled because of JDK 11 was `@(JavaSourceJar)`, which got a better
fix in commits a7413a2 and 0e95ec7.

Fix the `_ExtractJavaDocJars` target so that it properly runs when
there are any `@(JavaDocJar)` items, then fix the `_ExtractJavaDocJars`
target so that it doesn't emit an MSB3375 error.

Note: `@(JavaDocJar)` is still problematic: it involves parsing
Javadoc HTML, which contains numerous "dialects" (it's why the JDK 11
commit skipped `@(JavaSourceJar)` support: trying to update our HTML
scrapers was "too fiddly").

Deprecate `@(JavaDocJar)`, and remove
`Xamarin.Android.Bindings.Documentation.targets` from the .NET 6
installation packages.  Support for `$(_UseLegacyJavadocImport)`=False
won't exist for .NET 6; `@(JavaSourceJar)` support in .NET t will
*only* use `java-source-utils.jar` (a7413a2).

Update the `BindingBuildTest.cs` infrastructure so that instead of
having `.jar` files encoded in Base64, we instead use
`@(EmbeddedResource)`s for the `.jar` files.  A new `UpdateResources`
target will update the appropriate `.jar` files.
jonpryor added a commit that referenced this issue Jan 9, 2021
Fixes: #2745
Fixes: #5474

Issue #2745 noted that `@(JavaDocJar)` didn't work; attempting to use
it would result in an MSB3375 error:

	…/Xamarin.Android.Bindings.Documentation.targets(31,5): error MSB3375: The file "obj/Debug/javadocs/glide-3.7.0-javadoc.stamp" does not exist.

This issue was subsequently hidden by 380e95e, which disabled the
`_ExtractJavaDocJars` target unless JDK 1.8 was being used.

There were two problems with 380e95e:

 1. The `_ExtractJavaDocJars` target didn't need to be disabled!
    It just unzips the `.jar`; it does not require JDK 1.8.

 2. The check for JDK 1.8 was bad, and was *never* True.

The bad JDK 1.8 check was later removed in a7413a2, which "fixed"
(2), but a7413a2 continued to disable `_ExtractJavaDocJars`,
preventing `@(JavaDocJar)` from being used.

The only documentation-related functionality that needed to be
disabled because of JDK 11 was `@(JavaSourceJar)`, which got a better
fix in commits a7413a2 and 0e95ec7.

Fix the `_ExtractJavaDocJars` target so that it properly runs when
there are any `@(JavaDocJar)` items, then fix the `_ExtractJavaDocJars`
target so that it doesn't emit an MSB3375 error.

Note: `@(JavaDocJar)` is still problematic: it involves parsing
Javadoc HTML, which contains numerous "dialects" (it's why the JDK 11
commit skipped `@(JavaSourceJar)` support: trying to update our HTML
scrapers was "too fiddly").

Deprecate `@(JavaDocJar)`, and remove
`Xamarin.Android.Bindings.Documentation.targets` from the .NET 6
installation packages.  Support for `$(_UseLegacyJavadocImport)`=False
won't exist for .NET 6; `@(JavaSourceJar)` support in .NET t will
*only* use `java-source-utils.jar` (a7413a2).

Update the `BindingBuildTest.cs` infrastructure so that instead of
having `.jar` files encoded in Base64, we instead use
`@(EmbeddedResource)`s for the `.jar` files.  A new `UpdateResources`
target will update the appropriate `.jar` files.
@jonpryor jonpryor modified the milestones: d16-9, 11.3 Preview (d16-10) Apr 13, 2021
@jonpryor jonpryor added the bug Component does not function as intended. label Apr 13, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Jun 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Area: Bindings Issues in Java Library Binding projects. bug Component does not function as intended.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant