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

[Mono.Android] Add UpdateExternalDocumentation target #5485

Merged
merged 4 commits into from
Sep 29, 2021

Conversation

jonpryor
Copy link
Member

Fixes: #5200

Context: a7413a2
Context: xamarin/android-api-docs#23

Add a new UpdateExternalDocumentation target which:

  1. Rebuilds src/Mono.Android with
    $(IncludeAndroidJavadoc)=True. This produces a
    Mono.Anroid.xml file containing imported API-30 Javadoc.

  2. Run mdoc update --import Mono.Android.xml, updating the
    mdoc(5) documentation within
    external/android-api-docs/docs/Mono.Android/en

Usage:

msbuild /t:UpdateExternalDocumentation src/Mono.Android/Mono.Android.csproj

This process takes ~65 minutes on my machine.

Note: This uses the mdoc NuGet package.

jonpryor added a commit to xamarin/android-api-docs that referenced this pull request Jan 12, 2021
Context: dotnet/android#5485
Context: dotnet/android@a7413a2

Update the `Mono.Android.dll` documentation, importing the
Google-provided [API-30 Javadoc documentation][0].

At this time, only the following documentation is imported:

  * `<exception/>`: The Javadoc `@throws` block tag.
  * `<param/>`: The Javadoc `@param` block tag.
  * `<returns/>`: The Javadoc `@returns` block tag.
  * `<summary/>`: First sentence in Javadoc comment.

Additionally, the following information is generated:

  * Documentation Copyright notification
  * HTML Link to the Android documentation for the member.

Note that the link to Android documentation may not work; there are
known issues around the use of generic types.  (There are *lots*
of issues here, e.g. `<see/>` will be emitted which references the
Java type names, not the managed type names, and thus is invalid.)

The perfect is the enemy of the good (enough)!  It's been 4+ years
since the last documentation import.  Let's get *something* newer.

[0]: http://dl.google.com/android/repository/sources-30_r01.zip
@jonpryor jonpryor force-pushed the jonp-update-api-docs branch 2 times, most recently from 28491a7 to 846ed4d Compare January 13, 2021 01:20
@jonpryor
Copy link
Member Author

Fixes: dotnet#5200

Context: a7413a2
Context: xamarin/android-api-docs#23
Context: https://review.docs.microsoft.com/en-us/engineering/projects/reference/dotnet/mdoc

Add a new `UpdateExternalDocumentation` target which:

 1. *Rebuilds* `src/Mono.Android` with
    `$(IncludeAndroidJavadoc)`=True.  This produces a
    `Mono.Anroid.xml` file containing imported API-30 Javadoc.

    This *also* produces the log file
    `src/Mono.Android/UpdateExternalDocumentation-{TIME}.binlog`,
    which contains the build output for the rebuild.

 2. Runs `mdoc update --import Mono.Android.xml --use-docid`,
    updating the [**mdoc**(5) documentation][0] within
    `external/android-api-docs/docs/Mono.Android/en`

    The `--use-docid` flag is needed for integration with the
    documentation infrastructure.

Usage:

	msbuild /t:UpdateExternalDocumentation src/Mono.Android/Mono.Android.csproj

This process takes ~60 minutes on my machine.

Note: This uses the [mdoc NuGet package][1].

[0]: http://docs.go-mono.com/?link=man%3amdoc(5)
[1]: https://www.nuget.org/packages/mdoc/
@jonpryor jonpryor force-pushed the jonp-update-api-docs branch from 846ed4d to 63f78b7 Compare January 14, 2021 02:03
src/Mono.Android/Mono.Android.targets Outdated Show resolved Hide resolved
Comment on lines 286 to 290
<Exec
Command="msbuild /restore /p:IncludeAndroidJavadoc=True /bl:$(_Binlog)"
IgnoreStandardErrorWarningFormat="True"
WorkingDirectory="$(MSBuildThisFileDirectory)"
/>
Copy link
Member

Choose a reason for hiding this comment

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

Could this use <MSBuild/>? or does it actually need to run in a new process?

Copy link
Member

@pjcollins pjcollins May 12, 2021

Choose a reason for hiding this comment

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

I think the intent here was to make sure we always capture a unique binlog from the output. I've kept this for non windows platforms, but updated this target to use <MSBuild/> on Windows so that it can be run there if desired.

Base automatically changed from master to main March 5, 2021 23:08
pjcollins added a commit to dotnet/java-interop that referenced this pull request May 11, 2021
Context: dotnet/android#5485

In an attempt to generate updated documentation for API 30 I've noticed
a minor issue in generator.  When a `<javadoc>` element contains
multiple @return values, we will generate multiple `<returns>` elements
in our C# documentation.  This results in invalid C# documentation with
partially missing return information, and the second `<returns>` element
value is not displayed in IDE intellisense.  This also causes an issue
when the xml is processed by `mdoc`.  These extra return lines will
continue to be appended to the `mdoc` output every time the tool is ran
against the invalid `Mono.Android.xml`.
jonpryor pushed a commit to dotnet/java-interop that referenced this pull request May 12, 2021
Context: dotnet/android#5485

In an attempt to generate updated documentation for API-30 I noticed
a minor issue in generator Javadoc import (7574f16): when a
`<javadoc/>` element contains multiple `@return` values:

	<class name="Example" …>
	  <method name="getP" …>
	    <javadoc>
	      <![CDATA[
	        …
	        @return some docs
	        @return additional docs
	      ]]>
	    </javadoc>
	  </method>
	</class>

We would generate multiple `<returns/>` elements, one for each
`@return`:

	partial class Example {
	    /// <returns>some docs</returns>
	    /// <returns>additional docs</returns>
	    public int P {
	        [Register("getP", …)] get => …
	    }
	}

This is "fine", as far as the C# compiler is concerned, which results
in a Documentation XML file containing both `<returns/>`:

	<doc>
	  <members>
	    <member name="P:Example.P">
	      <returns>some docs</returns>
	      <returns>additional docs</returns>
	    </member>
	  </members>
	</doc>

The problem is when we later try to *import* this documentation via
`mdoc update --import`, as dotnet/android#5485 does;
if it's a *method* which contains multiple `<returns/>` elements and
mdoc 5.7.4.9 is used (included with macOS Mono 6.12):

	$ mdoc update -o docs assembly.dll --import assembly.xml

Then the resulting imported [**mdoc**(5)][0] documentation only
contains the *first* element:

	<Docs>
	  <summary>To be added.</summary>
	  <returns>some docs</returns>
	  <remarks>To be added.</remarks>
	</Docs>

Using [mdoc 5.8.0][1] will preserve both `<returns/>` elements, but
"higher level" tooling such as HTML rendering or IntelliSense may not
properly support multiple `<returns/>` elements.

*However*, if it's a *property* which contains multiple `<returns/>`
elements, then `mdoc update` will convert each `<returns/>` element
into a `<value/>` element:

	$ mdoc update -o docs assembly.dll --import assembly.xml
	…
	<Docs>
	  <summary>To be added.</summary>
	  <value>some docs</value>
	  <value>additional docs</value>
	  <remarks>To be added.</remarks>
	</Docs>

This is "odd", but it gets worse: on subsequent `mdoc update`
invocations, *additional* `<value/>` elements are created!

	$ mdoc update -o docs assembly.dll --import assembly.xml
	…
	<Docs>
	  <summary>To be added.</summary>
	  <value>additional docs</value>
	  <value>some docs</value>
	  <value>additional docs</value>
	  <remarks>To be added.</remarks>
	</Docs>

	$ mdoc update -o docs assembly.dll --import assembly.xml
	…
	<Docs>
	  <summary>To be added.</summary>
	  <value>some docs</value>
	  <value>additional docs</value>
	  <value>some docs</value>
	  <value>additional docs</value>
	  <remarks>To be added.</remarks>
	</Docs>

While an `mdoc` bug, we can prevent this from happening by updating
the Javadoc importer so that multiple `@returns` blocks are *merged*
into a single `<returns/>` element:

	partial class Example {
	    /// <returns>some docs additional docs</returns>
	    public int P {…}
	}

[0]: http://docs.go-mono.com/?link=man%3amdoc(5)
[1]: https://www.nuget.org/packages/mdoc/5.8.0
<PropertyGroup>
<_Binlog>UpdateExternalDocumentation-$([System.DateTime]::Now.ToString ("yyyyMMddTHHmmss")).binlog</_Binlog>
</PropertyGroup>
<Exec
Copy link
Member Author

Choose a reason for hiding this comment

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

@pjcollins: any reason to treat macOS & Windows differently here? Why not use <MSBuild/> for both?

Copy link
Member

Choose a reason for hiding this comment

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

I mentioned this briefly in the comment above -- using Exec over MSBuild allows us to automatically capture a binlog for each Mono.Android build, I can move it to use MSBuild for both if we want though

@jonpryor jonpryor merged commit cee5e72 into dotnet:main Sep 29, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Jan 26, 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.

API docs are out of date
4 participants