No need to parse JavaDoc in class-parse anymore.

[atsushieno]

Use pre-generated parameter names description instead.

They can be generated at external/xamarin-android-docimporter-ng/

Soon we will be able to delete src/Mono.Android/Profiles/api*.xml.in.

So far, build-tools/api-xml-adjuster is updated to take these params.txt
and generate api-*.xml.in. It is now done within a few minutes.

[atsushieno]

Marking DO-NOT-MERGE until we merge dotnet/java-interop#200

[jonpryor]

Shouldn't xamarin-android-docimporter-ng be merged with Java.Interop? I see no reason for Xamarin.Android.ApiTools.ParameterNameExtractor to live in a separate repo.

[atsushieno]

Not only because it is about ANDROID API and not generic Java related, but also it has no relationship with the build process.

Xamarin organization should fork the repo under its own though (but whoever can do that never actually did importing repos not matter how important it is and I asked so often).

[jonpryor]

Not only because it is about ANDROID API and not generic Java related.

OK, so it shouldn't be in Java.Interop.

but also it has no relationship with the build process.

I assume this means that you believe xamarin-android-docimporter-ng shouldn't be part of xamarin-android either?

I don't understand the rationale. Cross-module bumps are annoying; I don't feel a need to potentially make things worse.

[atsushieno]

I removed do-not-merge and am re-adding it again because I'd rather switch xamarin-android-docimporter-ng reference to the one in dotnet/java-interop#208.

[atsushieno]

It's all good now.

[jonpryor]

Should this use --parameter-names?

[atsushieno]

It turned out that your hack to detect documentation type is buggy. I'm going to create another PR to fix it.

[atsushieno]

dotnet/java-interop#212

[jonpryor]

src/Mono.Android/Profiles/api-.params.txt is a weird file to add. Shouldn't there be an integer in there?

[atsushieno]

indeed! Removed it.

[atsushieno]

API regressions should be fixed by dotnet/java-interop#215. Technically it is not mandatory (as I pushed regenerated api-*.xml. in with the fixed class-parse) but it's nice to have the PR merged first and we bump java.interop within this change.

[atsushieno]

HUH? Why does it have conflicts??

[atsushieno]

build

[jonpryor]

I'm assuming that we don't need to care about these "parameter name 'regressions'" because the api-25.params.txt has the correct parameter name?

[jonpryor]

Yup, api-25.params.txt contains:

  class FileBackupHelperBase
    writeNewStateDescription(android.os.ParcelFileDescriptor fd)

[jonpryor]

I don't understand how this all fits together. :-(

(Which is problematic when I'm attempting to rewrite the commit message to state how this all fits together...)

For starters, this is about class-parse-generated XML and the parameter names contained within. We care because we want to eventually "delete src/Mono.Android/Profiles/api*.xml.in", and instead generate those files at build-time.

That part I understand.

Additionally, the new api-*.params.txt files appear to only be used by class-parse.

What I don't understand is how this PR works, specifically around the altered src/Mono.Android/Profiles/api*.xml.in files and parameter names. Given changes such as this, I'd naively think that the resulting assembly would also contain those changes. It doesn't -- yay! (we'd probably break API if it did change, because of event) -- but I don't understand how those parameter names don't mean anything.

Thus, how is it that this PR can change parameter names in e.g. src/Mono.Android/Profiles/api-24.xml.in, yet those changes don't impact anything?

...and I think I have an answer: src/Mono.Android/Profiles/api-27.xml.in is unchanged. api-24.xml.in, meanwhile, is not built by the default PR system.

Meaning I don't see the breakage I fear I'll see because we're not building enough to see it.

[jonpryor]

build

[jonpryor]

I added the full-mono-integration-build label and rebuilt, so that all API levels would be built. This should allow us to see if the changes to src/Mono.Android/Profiles/api-24.xml.in are meaningful.

...and if they're not meaningful, why aren't they meaningful? (I suspect they are meaningful.)

[atsushieno]

Sigh. Why should api-27.xml.in even matter while it is never part of build? It is due to buggy MSBuild targets design:

		Target _GenerateApiDescription:
			Executing: mono ../../bin/BuildDebug/api-merge.exe -o "obj/Debug/android-26/mcw/api.xml" -s 'Profiles/api-*.xml.in' Profiles/api-10.xml.in Profiles/api-15.xml.in Profiles/api-16.xml.in Profiles/api-17.xml.in Profiles/api-18.xml.in Profiles/api-19.xml.in Profiles/api-20.xml.in Profiles/api-21.xml.in Profiles/api-22.xml.in Profiles/api-23.xml.in Profiles/api-24.xml.in Profiles/api-25.xml.in Profiles/api-26.xml.in Profiles/api-27.xml.in --last-description=Profiles/api-26.xml.in

[atsushieno]

Anyways api-27.xml.in must not matter because it is actually generated with api-27.params.txt, not from classic DroidDoc parser. That's why there was no change.

[jonpryor]

I'm looking at api-25.xml.in, and indeed, I'm confused. No answers, just confusion.

1. api-25.xml.in changes a writeNewStateDescription() method parameter name from fd to p0.

2. The resulting build artifacts from PR build 2071 contains a v7.1/Mono.Android.dll, which should use api-25.xml.in.

3. The resulting v7.1/Mono.Android.dll contains:

   $ monodis --method Mono.Android.dll | grep riteNewStateDescription
   77264: default class [mscorlib]System.Delegate GetWriteNewStateDescription_Landroid_os_ParcelFileDescriptor_Handler ()  (param: 89493 impl_flags: cil managed )
   77265: default void n_WriteNewStateDescription_Landroid_os_ParcelFileDescriptor_ (native int jnienv, native int native__this, native int native_fd)  (param: 89493 impl_flags: cil managed )
   77266: instance default void WriteNewStateDescription (class Android.OS.ParcelFileDescriptor fd)  (param: 89496 impl_flags: cil managed )
   77273: default class [mscorlib]System.Delegate GetWriteNewStateDescription_Landroid_os_ParcelFileDescriptor_Handler ()  (param: 89499 impl_flags: cil managed )
   77274: default void n_WriteNewStateDescription_Landroid_os_ParcelFileDescriptor_ (native int jnienv, native int native__this, native int native_fd)  (param: 89499 impl_flags: cil managed )
   77275: instance default void WriteNewStateDescription (class Android.OS.ParcelFileDescriptor fd)  (param: 89502 impl_flags: cil managed )
   77285: instance default void WriteNewStateDescription (class Android.OS.ParcelFileDescriptor newState)  (param: 89509 impl_flags: cil managed )
   77301: default class [mscorlib]System.Delegate GetWriteNewStateDescription_Landroid_os_ParcelFileDescriptor_Handler ()  (param: 89532 impl_flags: cil managed )
   77302: default void n_WriteNewStateDescription_Landroid_os_ParcelFileDescriptor_ (native int jnienv, native int native__this, native int native_newState)  (param: 89532 impl_flags: cil managed )
   77303: instance default void WriteNewStateDescription (class Android.OS.ParcelFileDescriptor newState)  (param: 89535 impl_flags: cil managed )
   77337: default class [mscorlib]System.Delegate GetWriteNewStateDescription_Landroid_os_ParcelFileDescriptor_Handler ()  (param: 89570 impl_flags: cil managed )
   77338: default void n_WriteNewStateDescription_Landroid_os_ParcelFileDescriptor_ (native int jnienv, native int native__this, native int native_fd)  (param: 89570 impl_flags: cil managed )
   77339: instance default void WriteNewStateDescription (class Android.OS.ParcelFileDescriptor fd)  (param: 89573 impl_flags: cil managed )
   

4. None of the WriteNewStateDescription() methods have a parameter name of p0.

What magic is this? Mono.Android.dll is produced via generator.exe, which processes the XML file resulting from api-merge.exe against Profiles/api-*.xml.in. I believe that we use the parameter names from the last API level, not the first, meaning the parameter names when building v7.1/Mono.Android.dll should be coming from Profiles/api-25.xml.in.

Yet...they're not?

Where are the parameter names coming from?

[atsushieno]

We do ignore those p0, p1... names. https://github.com/xamarin/xamarin-android/blob/master/build-tools/api-merge/ApiDescription.cs#L173

[atsushieno]

I think I explained why those differences occur: parameter name metadata from bytecode contains parameter names from method definitions from the corresponding definitions from non-public base class. In Java sources the derived classes of course don't contain those methods declared in the non-public base classes and therefore no parameter names are given.

[jonpryor]

We do ignore those p0, p1... names

Ah! I'd forgotten that!