Skip to content

Metadata <remove-attr> doesn't work as expected #976

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.

Sign up for GitHub

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

Jump to bottom
Closed
jpobst opened this issue May 2, 2022 · 0 comments · Fixed by #999
Closed

Metadata <remove-attr> doesn't work as expected #976

jpobst opened this issue May 2, 2022 · 0 comments · Fixed by #999
Labels
bug Component does not function as intended generator Issues binding a Java library (generator, class-parse, etc.)
Milestone
7.0.0

Comments

@jpobst
Copy link
Contributor

jpobst commented May 2, 2022

Given an api.xml element like:

<package name='my.package'>
  <class name='MyClass' api-since='29' />
</package>

You would expect to be able to remove the api-since attribute with this metadata:

<remove-attr path='/api/package[@name='my.package']/class[@name='MyClass']' name='api-since' />

However, even though generator reads the name attribute, it then proceeds to ignore it and remove all attributes on the matched node. This seems unintuitive, and I'm not sure how this could ever be successfully used in a binding project, since leaving an empty <class>, etc. node causes generator to crash.

Relevant code:
https://github.com/xamarin/java.interop/blob/0c90cf5cd67c5bd4978fbcf016281d330426989f/src/Java.Interop.Tools.Generator/Metadata/FixupXmlDocument.cs#L149-L167

<remove-attr> seems to be undocumented: https://docs.microsoft.com/en-us/xamarin/android/platform/binding-java-library/customizing-bindings/java-bindings-metadata
@jpobst jpobst added bug Component does not function as intended generator Issues binding a Java library (generator, class-parse, etc.) labels May 2, 2022
@jpobst jpobst changed the title Metadata remove-attr doesn't work as expected Metadata <remove-attr> doesn't work as expected May 2, 2022
@jpobst jpobst added this to the 7.0.0 milestone May 12, 2022
@jpobst jpobst mentioned this issue Jun 28, 2022
Merged
jonpryor pushed a commit that referenced this issue Jun 30, 2022
@jpobst
[generator] Fix <remove-attr/> metadata (#999)
7f1d2d7 
Fixes: #976

Given an `api.xml` element like:

	<package name='my.package'>
	  <class name='MyClass' api-since='29' />
	</package>

One would expect to be able to remove the `class/@api-since` attribute
with this metadata:

	<remove-attr path='/api/package[@name='my.package']/class[@name='MyClass']' name='api-since' />

However, even though `generator` reads the `name` attribute, it then
proceeds to ignore it and remove *all* attributes on the matched node,
resulting in:

	<package name='my.package'>
	  <class />
	</package>

This seems unintuitive, and I'm not sure how this could ever be
successfully used in a binding project, since leaving an empty
`<class/>`/etc. node causes `generator` to crash.

Update `<remove-attr/>` to work as expected, removing the named
attribute.
jonpryor added a commit to dotnet/android that referenced this issue Jul 8, 2022
@jonpryor
Bump to xamarin/java.interop/main@c942ab68 (#7157)
59352f8 
Fixes: dotnet/java-interop#335
Fixes: dotnet/java-interop#954
Fixes: dotnet/java-interop#976
Fixes: dotnet/java-interop#981
Fixes: dotnet/java-interop#992

Changes: dotnet/java-interop@7716ae5...c942ab6

  * dotnet/java-interop@c942ab68: [java-source-utils] Build one `$(TargetFramework)` (#1007)
  * dotnet/java-interop@b7caa788: [Byecode] Hide `internal` Kotlin fields marked with `@JvmField` (#1004)
  * dotnet/java-interop@22d5687b: [generator] Add `@managedOverride` values `none` and `reabstract`. (#1000)
  * dotnet/java-interop@7f1d2d7a: [generator] Fix <remove-attr/> metadata (#999)
  * dotnet/java-interop@265ad762: [Java.Interop.Tools.JavaSource] Fix tag parsing errors (#997)
  * dotnet/java-interop@99c68a86: [Java.Interop] JniTypeSignature & CultureInfo, empty strings (#1002)
  * dotnet/java-interop@920ea648: [Java.Interop] optimize JniTypeManager.AssertSimpleReference() (#1001)
  * dotnet/java-interop@4b4fedd3: [generator] Process Javadoc for nested types (#996)

Ever since commit 2197a45, CI builds for the `main` branch have been
incredibly flakey: 918613d through 27647d2 all failed to complete
the **Mac** stage -- which *must* pass in order for most unit tests
to run -- then dbadf13 built, then f149c25 failed.

Nine commits in the past week have failed to adequately build.
(PR builds, meanwhile, were largely fine.)

Most of these failures are from `make all-tests`:

	_ExtractJavadocsFromJavaSourceJars:
	  /Users/runner/android-toolchain/jdk-11/bin/java -jar /Users/runner/work/1/s/xamarin-android/bin/Release/lib/xamarin.android/xbuild/Xamarin/Android/java-source-utils.jar @/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/tmp1cf0947e.tmp 
	JAVASOURCEUTILS : warning : Invalid or corrupt jarfile /Users/runner/work/1/s/xamarin-android/bin/Release/lib/xamarin.android/xbuild/Xamarin/Android/java-source-utils.jar [/Users/runner/work/1/s/xamarin-android/tests/CodeGen-Binding/Xamarin.Android.McwGen-Tests/Xamarin.Android.McwGen-Tests.csproj]
	…
	BINDINGSGENERATOR : warning BG8600: Invalid XML file '/Users/runner/work/1/s/xamarin-android/tests/CodeGen-Binding/Xamarin.Android.McwGen-Tests/obj/Release/javadoc-xamarin-test-sources-F6F5170BF7A8BC71.xml': Could not find file "/Users/runner/work/1/s/xamarin-android/tests/CodeGen-Binding/Xamarin.Android.McwGen-Tests/obj/Release/javadoc-xamarin-test-sources-F6F5170BF7A8BC71.xml".  For details, see verbose output. [/Users/runner/work/1/s/xamarin-android/tests/CodeGen-Binding/Xamarin.Android.McwGen-Tests/Xamarin.Android.McwGen-Tests.csproj]
	  System.IO.FileNotFoundException: Could not find file "/Users/runner/work/1/s/xamarin-android/tests/CodeGen-Binding/Xamarin.Android.McwGen-Tests/obj/Release/javadoc-xamarin-test-sources-F6F5170BF7A8BC71.xml"
	…
	"/Users/runner/work/1/s/xamarin-android/Xamarin.Android-Tests.sln" (default target) (1:2) ->
	"/Users/runner/work/1/s/xamarin-android/tests/CodeGen-Binding/Xamarin.Android.JcwGen-Tests/Xamarin.Android.JcwGen-Tests.csproj" (default target) (12:6) ->
	(CoreCompile target) -> 
	  /Users/runner/work/1/s/xamarin-android/tests/CodeGen-Binding/Xamarin.Android.JcwGen-Tests/BindingTests.cs(78,37): error CS1061: 'DataEventArgs' does not contain a definition for 'FromNode' and no accessible extension method 'FromNode' accepting a first argument of type 'DataEventArgs' could be found (are you missing a using directive or an assembly reference?) [/Users/runner/work/1/s/xamarin-android/tests/CodeGen-Binding/Xamarin.Android.JcwGen-Tests/Xamarin.Android.JcwGen-Tests.csproj]
	  /Users/runner/work/1/s/xamarin-android/tests/CodeGen-Binding/Xamarin.Android.JcwGen-Tests/BindingTests.cs(79,40): error CS1061: 'DataEventArgs' does not contain a definition for 'FromChannel' and no accessible extension method 'FromChannel' accepting a first argument of type 'DataEventArgs' could be found (are you missing a using directive or an assembly reference?) [/Users/runner/work/1/s/xamarin-android/tests/CodeGen-Binding/Xamarin.Android.JcwGen-Tests/Xamarin.Android.JcwGen-Tests.csproj]
	  /Users/runner/work/1/s/xamarin-android/tests/CodeGen-Binding/Xamarin.Android.JcwGen-Tests/BindingTests.cs(80,40): error CS1061: 'DataEventArgs' does not contain a definition for 'PayloadType' and no accessible extension method 'PayloadType' accepting a first argument of type 'DataEventArgs' could be found (are you missing a using directive or an assembly reference?) [/Users/runner/work/1/s/xamarin-android/tests/CodeGen-Binding/Xamarin.Android.JcwGen-Tests/Xamarin.Android.JcwGen-Tests.csproj]
	  /Users/runner/work/1/s/xamarin-android/tests/CodeGen-Binding/Xamarin.Android.JcwGen-Tests/BindingTests.cs(81,28): error CS1061: 'DataEventArgs' does not contain a definition for 'Payload' and no accessible extension method 'Payload' accepting a first argument of type 'DataEventArgs' could be found (are you missing a using directive or an assembly reference?) [/Users/runner/work/1/s/xamarin-android/tests/CodeGen-Binding/Xamarin.Android.JcwGen-Tests/Xamarin.Android.JcwGen-Tests.csproj]
	  /Users/runner/work/1/s/xamarin-android/tests/CodeGen-Binding/Xamarin.Android.JcwGen-Tests/BindingTests.cs(82,29): error CS1061: 'DataEventArgs' does not contain a definition for 'Payload' and no accessible extension method 'Payload' accepting a first argument of type 'DataEventArgs' could be found (are you missing a using directive or an assembly reference?) [/Users/runner/work/1/s/xamarin-android/tests/CodeGen-Binding/Xamarin.Android.JcwGen-Tests/Xamarin.Android.JcwGen-Tests.csproj]
	  /Users/runner/work/1/s/xamarin-android/tests/CodeGen-Binding/Xamarin.Android.JcwGen-Tests/BindingTests.cs(84,51): error CS1061: 'DataEventArgs' does not contain a definition for 'Payload' and no accessible extension method 'Payload' accepting a first argument of type 'DataEventArgs' could be found (are you missing a using directive or an assembly reference?) [/Users/runner/work/1/s/xamarin-android/tests/CodeGen-Binding/Xamarin.Android.JcwGen-Tests/Xamarin.Android.JcwGen-Tests.csproj]
	  /Users/runner/work/1/s/xamarin-android/tests/CodeGen-Binding/Xamarin.Android.JcwGen-Tests/BindingTests.cs(85,10): error CS1061: 'DataEventArgs' does not contain a definition for 'Payload' and no accessible extension method 'Payload' accepting a first argument of type 'DataEventArgs' could be found (are you missing a using directive or an assembly reference?) [/Users/runner/work/1/s/xamarin-android/tests/CodeGen-Binding/Xamarin.Android.JcwGen-Tests/Xamarin.Android.JcwGen-Tests.csproj]

`java-source-utils.jar` is (apparently) corrupt, so
`javadoc-xamarin-test-sources-F6F5170BF7A8BC71.xml` is never created,
which means no parameter name information is present, which means the
generated `DataEventArgs` type doesn't have the appropriate property
names, as the property names come from parameter names, and since we
don't have parameter names we instead have property names like `P0`,
`P1`, `P2`, etc.

Why, then, is `java-source-utils.jar` corrupt?  The current guess is
that it is being built *concurrently*; from a
`msbuild-20220706T175333-leeroy-all.binlog` build log file from one
of the offending builds, we see:

	17:53:44.526 59:11>Target "_BuildJava: (TargetId:490)" in file "/Users/runner/work/1/s/xamarin-android/external/Java.Interop/tools/java-source-utils/java-source-utils.targets" from project "/Users/runner/work/1/s/xamarin-android/external/Java.Interop/tools/java-source-utils/java-source-utils.csproj" (entry point):
	17:53:44.526 59:11>Building target "_BuildJava" completely.
	  Output file "/Users/runner/work/1/s/xamarin-android/bin/Release/lib/packs/Microsoft.Android.Sdk.Darwin/33.0.0/tools/java-source-utils.jar" does not exist.
	…
	17:54:19.564 59:12>Target "DispatchToInnerBuilds: (TargetId:1637)" in file "/Users/runner/work/1/s/xamarin-android/bin/Release/dotnet/sdk/7.0.100-preview.7.22354.2/Microsoft.Common.CrossTargeting.targets" from project "/Users/runner/work/1/s/xamarin-android/external/Java.Interop/tools/java-source-utils/java-source-utils.csproj" (target "Build" depends on it):
	  Task "MSBuild" (TaskId:1099)
	    Task Parameter:BuildInParallel=True (TaskId:1099)
	    Task Parameter:Targets=Build (TaskId:1099)
	    Task Parameter:
	        Projects=
	            java-source-utils.csproj
	                    AdditionalProperties=TargetFramework=net7.0 (TaskId:1099)
	    Additional Properties for project "java-source-utils.csproj": (TaskId:1099)
	      TargetFramework=net7.0 (TaskId:1099)
	…
	17:54:19.592 59:12>Target "_BuildJava: (TargetId:1640)" in file "/Users/runner/work/1/s/xamarin-android/external/Java.Interop/tools/java-source-utils/java-source-utils.targets" from project "/Users/runner/work/1/s/xamarin-android/external/Java.Interop/tools/java-source-utils/java-source-utils.csproj" (entry point):
	  Building target "_BuildJava" completely.
	  Output file "/Users/runner/work/1/s/xamarin-android/external/Java.Interop/../../bin/Release/lib/xamarin.android/xbuild/Xamarin/Android/java-source-utils.jar" does not exist.
	…
	17:54:41.034 59:11>Done building target "_BuildJava" in project "java-source-utils.csproj".: (TargetId:490)
 
What appears to be happening -- and there is lots of conjecture here,
as the build log is enormous and it's difficult to piece everything
together -- is that `java-source-utils.csproj` is built *twice*:
Once via `Xamarin.Android.sln`, and once via `apksigner.csproj`, as
it appears that setting `%(PackageReference.AdditionalProperties)`
triggers a *nested build*; note the `DispatchToInnerBuilds` target
which invokes the `<MSBuild/>` Task with the specified
`AdditionalProperties`.

However, that's not the only potential source of concurrent builds;
`java-source-utils.csproj` used `$(TargetFrameworks)` (plural), which
can *also* result in concurrent builds between the "outer" and "inner"
builds used as part of `$(TargetFrameworks)`.

Fix this problem by bumping to dotnet/java-interop@c942ab68, which
updates `java-source-utils.csproj` to use the singular
`$(TargetFramework)` property -- avoiding "outer" and "inner" build
problems -- and "for good measure" update `apksigner.csproj` to
remove `%(ProjectReference.AdditionalProperties)` on
`java-source-utils.csproj`.  This should ensure that
`java-source-utils.csproj` is only built *once*, which in turn should
ensure that `java-source-utils.jar` isn't corrupted, and our CI
builds become more reliable
@github-actions github-actions bot locked and limited conversation to collaborators Apr 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
bug Component does not function as intended generator Issues binding a Java library (generator, class-parse, etc.)
Projects
None yet
Milestone
7.0.0
Development

Successfully merging a pull request may close this issue.

[generator] Fix <remove-attr> metadata. dotnet/java-interop
1 participant
@jpobst