Skip to content

[Xamarin.Android.Build.Tasks] Support %(JavaSourceJar.DocRootUrl) #6662

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
Merged
merged 1 commit into from
Jan 28, 2022
Merged

[Xamarin.Android.Build.Tasks] Support %(JavaSourceJar.DocRootUrl) #6662

merged 1 commit into from
Jan 28, 2022

Conversation

@pjcollins
Copy link
Member

@pjcollins pjcollins commented Jan 25, 2022 •
edited
Loading

Context: dotnet/java-interop@13def0e

Bump to dotnet/java-interop@32635fd
Changes: dotnet/java-interop@d3f0c5c...32635fd

Adds %(DocRootUrl) to the set of metadata that is supported by
@(JavaSourceJar) items. Fixes processing of %(JavadocUrlPrefix) and
%(JavadocUrlStyle) metadata values.

The <_BuildAndroidJavadocXml/> target has also been updated to use the
new --doc-root-url parameter supported by java-source-utils.jar.

@pjcollins pjcollins mentioned this pull request Jan 25, 2022
Merged
jonpryor pushed a commit to dotnet/java-interop that referenced this pull request Jan 25, 2022
@pjcollins
[Xamarin.Android.Tools.Bytecode] Extend api xml doc type detection (#943
32635fd 
)

Context: dotnet/android#6662

The [`@(JavaSourceJar)`][0] Build action supports the following
item metadata:

	<JavaSourceJar Include="javasourcejartest-sources.jar" >
	  <CopyrightFile>$(MSBuildThisFileDirectory)javadoc-copyright.xml</CopyrightFile>
	  <UrlPrefix>https://developer.android.com/reference</UrlPrefix>
	  <UrlStyle>developer.android.com/reference@2020-Nov</UrlStyle>
	  <DocRootUrl>https://developer.android.com</DocRootUrl>
	</JavaSourceJar>

`@(JavaSourceJar)` is used by the Xamarin.Android `<JavaSourceUtils/>`
task, which uses all of the item metadata as various option values to
`java-source-utils.jar --output-javadoc` (7574f16), resulting in
"Javadoc XML", which is subsequently provided to the `<ClassParse/>`
MSBuild task, which passes the "Javadoc XML" as a
`class-parse --docspath=PATH` option value.

`class-parse.exe --docspath=PATH` (eventually) uses
`JavaMethodParameterNameProvider.GetDocletType()` to determine the
file format of `PATH`, and `GetDocletType()` reads the first 500
bytes of the file to determine if `PATH` is `JavaDocletType._ApiXml`
(`<api/>` XML, as produced by `class-parse.exe` & others) or
`JavaDocletType.JavaApiParameterNamesXml` (which *isn't* xml, and is
produced by `java-source-utils.jar --output-params`).

If a `%(JavaSourceJar.CopyrightFile)` file is "large" (> 500 bytes),
then `GetDocletType()` won't detect the "Javadoc XML" file as
`JavaDocletType._ApiXml`, but instead as `JavaDocletType.DroidDoc`
(the default!), which results in (bizarre!) build errors:

	Task "ClassParse"
	  Task Parameter:ToolPath=C:\Users\cloudtest\android-toolchain\dotnet\packs\Microsoft.Android.Sdk.Windows\31.0.200-ci.pr.gh6662.38\tools
	  Task Parameter:OutputFile=obj\Debug\api.xml.class-parse
	  Task Parameter:SourceJars=javasourcejartest.jar
	  Task Parameter:
	      DocumentationPaths=
	          obj\Debug\javadoc-javasourcejartest-sources-C93909CC4CF9CB39.xml
	                  CopyrightFile=…\TestRelease\01-25_05.25.10\temp\JavaSourceJar\javadoc-copyright.xml
	                  DocRootUrl=https://developer.android.com
	                  Hash=C93909CC4CF9CB39
	                  OriginalItemSpec=javasourcejartest-sources.jar
	                  UrlPrefix=https://developer.android.com/reference
	                  UrlStyle=developer.android.com/reference@2020-Nov
	  Using: dotnet C:\Users\cloudtest\android-toolchain\dotnet\packs\Microsoft.Android.Sdk.Windows\31.0.200-ci.pr.gh6662.38\tools\class-parse.dll
	  [class-parse] response file: obj\Debug\class-parse.rsp
	  --o="obj\Debug\api.xml.class-parse"
	  --docspath="obj\Debug\javadoc-javasourcejartest-sources-C93909CC4CF9CB39.xml"
	  "javasourcejartest.jar"
	  dotnet C:\Users\cloudtest\android-toolchain\dotnet\packs\Microsoft.Android.Sdk.Windows\31.0.200-ci.pr.gh6662.38\tools\class-parse.dll @obj\Debug\class-parse.rsp
	  Unhandled exception. System.Exception: Directory 'obj\Debug\javadoc-javasourcejartest-sources-C93909CC4CF9CB39.xml' does not exist
	     at Xamarin.Android.Tools.Bytecode.AndroidDocScraper..ctor(String dir, String patternHead, String resetPatternHead, String parameterPairSplitter, Boolean continuousParamLines, String openMethod, String paramSep, String closeMethod, String postCloseMethodParens)
	     at Xamarin.Android.Tools.Bytecode.AndroidDocScraper..ctor(String dir, String patternHead, String resetPatternHead, String parameterPairSplitter, Boolean continuousParamLines)
	     at Xamarin.Android.Tools.Bytecode.DroidDocScraper..ctor(String dir)
	     at Xamarin.Android.Tools.Bytecode.ClassPath.CreateDocScraper(String src)
	     at Xamarin.Android.Tools.Bytecode.ClassPath.FixupParametersFromDocs(XElement api, String path)
	     at Xamarin.Android.Tools.Bytecode.ClassPath.FixupParametersFromDocs(XElement api)
	     at Xamarin.Android.Tools.Bytecode.ClassPath.ToXElement()
	     at Xamarin.Android.Tools.Bytecode.ClassPath.SaveXmlDescription(TextWriter textWriter)
	     at Xamarin.Android.Tools.App.Main(String[] args)

Fix the `JavaDocletType._ApiXml` doc type detection in such cases by
also checking for the `<javadoc-metadata>` element that is created by
`java-source-utils.jar --output-javadoc`.

[0]: https://docs.microsoft.com/en-us/xamarin/android/deploy-test/building-apps/build-items#javasourcejar
@pjcollins pjcollins force-pushed the docroot-fix branch 2 times, most recently from 26168cf to 645d4dc Compare January 26, 2022 00:48
@pjcollins pjcollins marked this pull request as ready for review January 26, 2022 15:43
jonathanpeppers
jonathanpeppers approved these changes Jan 26, 2022
View reviewed changes
Copy link
Member

@jonathanpeppers jonathanpeppers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

jonpryor
jonpryor reviewed Jan 27, 2022
View reviewed changes
jonpryor
jonpryor reviewed Jan 27, 2022
View reviewed changes
@pjcollins pjcollins force-pushed the docroot-fix branch 2 times, most recently from 8b9dcbc to 6ca8d73 Compare January 27, 2022 18:55
@jonpryor
Copy link
Contributor

jonpryor commented Jan 27, 2022

Why does this add src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Resources/javasourcejartest-javadoc.jar and …/javasourcejartest.jar and …/javasourcejartest-sources.jar?

These are built and included by: https://github.com/xamarin/xamarin-android/blob/main/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Directory.Build.targets

@pjcollins
Copy link
Member Author

pjcollins commented Jan 28, 2022

Why does this add src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Resources/javasourcejartest-javadoc.jar and …/javasourcejartest.jar and …/javasourcejartest-sources.jar?

I ran the UpdateResources target locally to update these files after changing the java source, I had assumed that these resource updates were done manually as needed. I don't see anything else that calls the UpdateResources target in that file, but maybe I am missing something?

@jonpryor
Copy link
Contributor

jonpryor commented Jan 28, 2022

I don't understand why the DotNetInstallAndRun(True,False,null) test is failing :-(

Running: /Users/runner/Library/Android/dotnet/dotnet run --project "/Users/runner/work/1/a/TestRelease/01-27_20.04.58/temp/DotNetInstallAndRunTrueFalsenull/UnnamedProject.csproj" --no-build /bl:"/Users/runner/work/1/a/TestRelease/01-27_20.04.58/temp/DotNetInstallAndRunTrueFalsenull/run.binlog"
Exit Code: 1

Additionally, run.binlog isn't an attachment. Could we update the unit test infrastructure so that run.binlog is included?

@jonpryor
Copy link
Contributor

jonpryor commented Jan 28, 2022 •
edited
Loading

The Mono.Android.NET_Tests-Interpreter crash is also concerning. :-(

It's doubly concerning that I don't see anything mentioning "Interpreter" in the Artifacts > Test Results - APKs .NET - macOS section.

@jonpryor
Copy link
Contributor

jonpryor commented Jan 28, 2022

@pjcollins wrote:

I ran the UpdateResources target locally to update these files after changing the java source, I had assumed that these resource updates were done manually as needed.

My mistake; I thought these .jar files were being added, but they were instead updates. Apologies. See also commit 16e0aa2:

A new UpdateResources target will update the appropriate .jar files.

@pjcollins
Copy link
Member Author

pjcollins commented Jan 28, 2022

I'm going to try to address some of the test failure reporting limitations mentioned here with #6681. I think this should be good to go otherwise.

@jonpryor jonpryor merged commit d582b80 into dotnet:main Jan 28, 2022
@pjcollins pjcollins deleted the docroot-fix branch January 31, 2022 15:30
@github-actions github-actions bot locked and limited conversation to collaborators Jan 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jonathanpeppers jonathanpeppers jonathanpeppers approved these changes

@dellis1972 dellis1972 Awaiting requested review from dellis1972

+1 more reviewer

@jonpryor jonpryor jonpryor left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@pjcollins @jonpryor @jonathanpeppers