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

Bump to xamarin/Java.Interop/master@56c92c70 #4376

Merged
merged 6 commits into from
Mar 27, 2020

Conversation

jonpryor
Copy link
Member

DO NOT MERGE

Context: dotnet/java-interop#597

Just testing to see if dotnet/java-interop#597 works...

@jonpryor jonpryor added the do-not-merge PR should not be merged. label Mar 10, 2020
@jonpryor
Copy link
Member Author

Doh, forgot to make this a Draft!

@jonpryor jonpryor force-pushed the jonp-try-ji-cecil-removal branch 2 times, most recently from d9e3261 to 71c17f8 Compare March 10, 2020 16:19
@jonpryor jonpryor force-pushed the jonp-try-ji-cecil-removal branch from 71c17f8 to 4c0580e Compare March 18, 2020 19:15
@jonpryor
Copy link
Member Author

Alas, it doesn't work; it fails to build:

  Java.Interop.Tools.Diagnostics/Diagnostic.cs(151,39): error CS0433: The type 'SequencePoint' exists in both 'Mono.Cecil, Version=0.11.2.0, Culture=neutral, PublicKeyToken=50cebf1cceb9d05e' and 'Xamarin.Android.Cecil, Version=0.11.1.0, Culture=neutral, PublicKeyToken=0738eb9f132ed756' [/Users/builder/azdo/_work/3/s/xamarin-android/external/Java.Interop/src/Java.Interop.Tools.Diagnostics/Java.Interop.Tools.Diagnostics.csproj]
  Java.Interop.Tools.Diagnostics/XamarinAndroidException.cs(38,10): error CS0433: The type 'SequencePoint' exists in both 'Mono.Cecil, Version=0.11.2.0, Culture=neutral, PublicKeyToken=50cebf1cceb9d05e' and 'Xamarin.Android.Cecil, Version=0.11.1.0, Culture=neutral, PublicKeyToken=0738eb9f132ed756' [/Users/builder/azdo/_work/3/s/xamarin-android/external/Java.Interop/src/Java.Interop.Tools.Diagnostics/Java.Interop.Tools.Diagnostics.csproj]

Current conjecture is that external/Java.Interop/src/Java.Interop.Tools.Diagnostics/obj contains various *.cache files which mention both Mono.Cecil.dll and Xamarin.Android.Cecil.dll:

 % grep -ril Cecil --include=\*.cache obj | xargs strings | grep Cecil
}…/Java.Interop/$/../packages/mono.cecil/0.11.2/lib/netstandard2.0/Mono.Cecil.dll
…/Java.Interop/$/../packages/mono.cecil/0.11.2/lib/netstandard2.0/Mono.Cecil.Mdb.dll
…/Java.Interop/../../bin/Debug/lib/xamarin.android/xbuild/Xamarin/Android/Xamarin.Android.Cecil.dll
…/Java.Interop/$/../packages/mono.cecil/0.11.2/lib/netstandard2.0/Mono.Cecil.Rocks.dll
…/Java.Interop/$/../packages/mono.cecil/0.11.2/lib/netstandard2.0/Mono.Cecil.Pdb.dll
Mono.Cecil
NMono.Cecil, Version=0.11.2.0, Culture=neutral, PublicKeyToken=50cebf1cceb9d05e
Mono.Cecil.Mdb
RMono.Cecil.Mdb, Version=0.11.2.0, Culture=neutral, PublicKeyToken=50cebf1cceb9d05e
Xamarin.Android.Cecil
YXamarin.Android.Cecil, Version=0.11.1.0, Culture=neutral, PublicKeyToken=0738eb9f132ed756
Mono.Cecil.Rocks
TMono.Cecil.Rocks, Version=0.11.2.0, Culture=neutral, PublicKeyToken=50cebf1cceb9d05e
Mono.Cecil.Pdb
RMono.Cecil.Pdb, Version=0.11.2.0, Culture=neutral, PublicKeyToken=50cebf1cceb9d05e
…/Java.Interop/%24/../packages/mono.cecil/0.11.2/lib/netstandard2.0/Mono.Cecil.Mdb.dll
…/Java.Interop/%24/../packages/mono.cecil/0.11.2/lib/netstandard2.0/Mono.Cecil.Pdb.dll
…/Java.Interop/%24/../packages/mono.cecil/0.11.2/lib/netstandard2.0/Mono.Cecil.Rocks.dll
…/Java.Interop/%24/../packages/mono.cecil/0.11.2/lib/netstandard2.0/Mono.Cecil.dll
Mono.Cecil
…/Java.Interop/%24/../packages/mono.cecil/0.11.2/lib/netstandard2.0/Mono.Cecil.Mdb.dll
…/Java.Interop/%24/../packages/mono.cecil/0.11.2/lib/netstandard2.0/Mono.Cecil.Pdb.dll
…/Java.Interop/%24/../packages/mono.cecil/0.11.2/lib/netstandard2.0/Mono.Cecil.Rocks.dll
…/Java.Interop/%24/../packages/mono.cecil/0.11.2/lib/netstandard2.0/Mono.Cecil.dll
Mono.Cecil
PathInPackage%lib/netstandard2.0/Mono.Cecil.Mdb.dll%lib/netstandard2.0/Mono.Cecil.Pdb.dll'lib/netstandard2.0/Mono.Cecil.Rocks.dll!lib/netstandard2.0/Mono.Cecil.dll
Mono.Cecil.Mdb.dll
Mono.Cecil.Pdb.dll
Mono.Cecil.Rocks.dll
Mono.Cecil.dll

Furthermore, if I delete the external/Java.Interop/src/Java.Interop.Tools.Diagnostics/obj directory, it builds:

$ rm -Rf obj
$ msbuild /restore /v:diag > b.txt
$ echo $?
0

@jonpryor
Copy link
Member Author

It looks like using msbuild /restore everywhere allows Java.Interop.Tools.Diagnostics to build even when obj contains cache files mentioning the "wrong" assembly...

jonpryor added a commit to jonpryor/xamarin-android that referenced this pull request Mar 19, 2020
***DO NOT MERGE***

Context: dotnet/java-interop#597

Just testing to see if dotnet/java-interop#597 works...

Need to use `msbuild /restore` for all builds to allow
`Java.Interop.Tools.Cecil` to build; see:

dotnet#4376 (comment)
@jonpryor jonpryor force-pushed the jonp-try-ji-cecil-removal branch from 4c0580e to a5a2a5d Compare March 19, 2020 15:27
jonpryor added a commit to jonpryor/xamarin-android that referenced this pull request Mar 19, 2020
***DO NOT MERGE***

Context: dotnet/java-interop#597

Just testing to see if dotnet/java-interop#597 works...

Need to use `msbuild /restore` for all builds to allow
`Java.Interop.Tools.Cecil` to build; see:

dotnet#4376 (comment)
@jonpryor jonpryor force-pushed the jonp-try-ji-cecil-removal branch from a5a2a5d to b70579f Compare March 19, 2020 19:47
jonpryor added a commit to jonpryor/xamarin-android that referenced this pull request Mar 19, 2020
***DO NOT MERGE***

Context: dotnet/java-interop#597

Just testing to see if dotnet/java-interop#597 works...

Need to use `msbuild /restore` for all builds to allow
`Java.Interop.Tools.Cecil` to build; see:

dotnet#4376 (comment)

Note: `$(TargetFrameworks)` throws a wrench into things.  If two
`$(TargetFramework)` builds share the same output directory, the
`IncrementalClean` target will *remove files created by previous
builds*, e.g. when e.g. `generator.csproj` builds the `netcoreapp3.1`
framework, it will *delete* the `generator.exe` built by the `net472`
framework, which results in subsequent build breaks.

The only path to sanity is to *ensure* that different
`$(TargetFramework)` builds have *completely separate* `$(OutputPath)`
values.

The "normal" approach to doing this is for `$(OutputPath)` to end with
`$(TargetFramework)`.

The problem here is that xamarin-android is setup so that we mirror
the installation directory, with e.g. `bin/Debug` being an
"installation root", and that installation root *won't* (normally?)
have *both* net472 and netcoreapp3.1 outputs for the "same" project.

The "solution" is to use the new `$(UtilityOutputFullPathCoreApps)`
property within Java.Interop to ensure that Java.Interop "utility"
apps -- normally placed into
`bin/Debug/lib/xamarin.android/xbuild/Xamarin/Android` -- are
*instead* placed into `bin/Debug-netcoreapp3.1` for netcoreapp3.1
builds.

This ensure that things *actually build*.  Locally.  YMMV.
@jonpryor jonpryor force-pushed the jonp-try-ji-cecil-removal branch from b70579f to b39a585 Compare March 19, 2020 22:46
jonpryor added a commit to jonpryor/xamarin-android that referenced this pull request Mar 20, 2020
***DO NOT MERGE***

Context: dotnet/java-interop#597

Just testing to see if dotnet/java-interop#597 works...

Need to use `msbuild /restore` for all builds to allow
`Java.Interop.Tools.Cecil` to build; see:

dotnet#4376 (comment)

Note: `$(TargetFrameworks)` throws a wrench into things.  If two
`$(TargetFramework)` builds share the same output directory, the
`IncrementalClean` target will *remove files created by previous
builds*, e.g. when e.g. `generator.csproj` builds the `netcoreapp3.1`
framework, it will *delete* the `generator.exe` built by the `net472`
framework, which results in subsequent build breaks.

The only path to sanity is to *ensure* that different
`$(TargetFramework)` builds have *completely separate* `$(OutputPath)`
values.

The "normal" approach to doing this is for `$(OutputPath)` to end with
`$(TargetFramework)`.

The problem here is that xamarin-android is setup so that we mirror
the installation directory, with e.g. `bin/Debug` being an
"installation root", and that installation root *won't* (normally?)
have *both* net472 and netcoreapp3.1 outputs for the "same" project.

The "solution" is to use the new `$(UtilityOutputFullPathCoreApps)`
property within Java.Interop to ensure that Java.Interop "utility"
apps -- normally placed into
`bin/Debug/lib/xamarin.android/xbuild/Xamarin/Android` -- are
*instead* placed into `bin/Debug-netcoreapp3.1` for netcoreapp3.1
builds.

This ensure that things *actually build*.  Locally.  YMMV.
@jonpryor jonpryor force-pushed the jonp-try-ji-cecil-removal branch from b39a585 to a08e84b Compare March 20, 2020 18:54
jonpryor added a commit to jonpryor/xamarin-android that referenced this pull request Mar 21, 2020
***DO NOT MERGE***

Context: dotnet/java-interop#597

Just testing to see if dotnet/java-interop#597 works...

Need to use `msbuild /restore` for all builds to allow
`Java.Interop.Tools.Cecil` to build; see:

dotnet#4376 (comment)

Note: `$(TargetFrameworks)` throws a wrench into things.  If two
`$(TargetFramework)` builds share the same output directory, the
`IncrementalClean` target will *remove files created by previous
builds*, e.g. when e.g. `generator.csproj` builds the `netcoreapp3.1`
framework, it will *delete* the `generator.exe` built by the `net472`
framework, which results in subsequent build breaks.

The only path to sanity is to *ensure* that different
`$(TargetFramework)` builds have *completely separate* `$(OutputPath)`
values.

The "normal" approach to doing this is for `$(OutputPath)` to end with
`$(TargetFramework)`.

The problem here is that xamarin-android is setup so that we mirror
the installation directory, with e.g. `bin/Debug` being an
"installation root", and that installation root *won't* (normally?)
have *both* net472 and netcoreapp3.1 outputs for the "same" project.

The "solution" is to use the new `$(UtilityOutputFullPathCoreApps)`
property within Java.Interop to ensure that Java.Interop "utility"
apps -- normally placed into
`bin/Debug/lib/xamarin.android/xbuild/Xamarin/Android` -- are
*instead* placed into `bin/Debug-netcoreapp3.1` for netcoreapp3.1
builds.

This ensure that things *actually build*.  Locally.  YMMV.
@jonpryor jonpryor force-pushed the jonp-try-ji-cecil-removal branch from a08e84b to b5adb86 Compare March 21, 2020 03:27
jonpryor added a commit to jonpryor/xamarin-android that referenced this pull request Mar 25, 2020
***DO NOT MERGE***

Context: dotnet/java-interop#597

Just testing to see if dotnet/java-interop#597 works...

Need to use `msbuild /restore` for all builds to allow
`Java.Interop.Tools.Cecil` to build; see:

dotnet#4376 (comment)

Note: `$(TargetFrameworks)` throws a wrench into things.  If two
`$(TargetFramework)` builds share the same output directory, the
`IncrementalClean` target will *remove files created by previous
builds*, e.g. when e.g. `generator.csproj` builds the `netcoreapp3.1`
framework, it will *delete* the `generator.exe` built by the `net472`
framework, which results in subsequent build breaks.

The only path to sanity is to *ensure* that different
`$(TargetFramework)` builds have *completely separate* `$(OutputPath)`
values.

The "normal" approach to doing this is for `$(OutputPath)` to end with
`$(TargetFramework)`.

The problem here is that xamarin-android is setup so that we mirror
the installation directory, with e.g. `bin/Debug` being an
"installation root", and that installation root *won't* (normally?)
have *both* net472 and netcoreapp3.1 outputs for the "same" project.

The "solution" is to use the new `$(UtilityOutputFullPathCoreApps)`
property within Java.Interop to ensure that Java.Interop "utility"
apps -- normally placed into
`bin/Debug/lib/xamarin.android/xbuild/Xamarin/Android` -- are
*instead* placed into `bin/Debug-netcoreapp3.1` for netcoreapp3.1
builds.

This ensure that things *actually build*.  Locally.  YMMV.
@jonpryor jonpryor force-pushed the jonp-try-ji-cecil-removal branch from 7b08516 to 16d0019 Compare March 25, 2020 17:52
***DO NOT MERGE***

Context: dotnet/java-interop#597

Just testing to see if dotnet/java-interop#597 works...

Need to use `msbuild /restore` for all builds to allow
`Java.Interop.Tools.Cecil` to build; see:

dotnet#4376 (comment)

Note: `$(TargetFrameworks)` throws a wrench into things.  If two
`$(TargetFramework)` builds share the same output directory, the
`IncrementalClean` target will *remove files created by previous
builds*, e.g. when e.g. `generator.csproj` builds the `netcoreapp3.1`
framework, it will *delete* the `generator.exe` built by the `net472`
framework, which results in subsequent build breaks.

The only path to sanity is to *ensure* that different
`$(TargetFramework)` builds have *completely separate* `$(OutputPath)`
values.

The "normal" approach to doing this is for `$(OutputPath)` to end with
`$(TargetFramework)`.

The problem here is that xamarin-android is setup so that we mirror
the installation directory, with e.g. `bin/Debug` being an
"installation root", and that installation root *won't* (normally?)
have *both* net472 and netcoreapp3.1 outputs for the "same" project.

The "solution" is to use the new `$(UtilityOutputFullPathCoreApps)`
property within Java.Interop to ensure that Java.Interop "utility"
apps -- normally placed into
`bin/Debug/lib/xamarin.android/xbuild/Xamarin/Android` -- are
*instead* placed into `bin/Debug-netcoreapp3.1` for netcoreapp3.1
builds.

This ensure that things *actually build*.  Locally.  YMMV.
@jonpryor jonpryor force-pushed the jonp-try-ji-cecil-removal branch from 16d0019 to c11909e Compare March 26, 2020 20:09
@jonpryor
Copy link
Member Author

...and, related: dotnet/msbuild#3000 (comment)

dotnet/msbuild#2811

Changes: dotnet/java-interop@1a086ff...56c92c7

  * dotnet/java-interop@56c92c7: [build] Remove cecil submodule (dotnet#597)
  * dotnet/java-interop@3091274: [build] Provide a default $(Configuration) value (dotnet#612)
  * dotnet/java-interop@cf3e7c2: [generator] Don't process duplicate reference assemblies (dotnet#611)
  * dotnet/java-interop@f5fa462: [jnienv-gen] Convert to SDK-style (dotnet#608)
@jonpryor jonpryor changed the title Bump to jonpryor/Java.Interop@4f55ace3 Bump to xamarin/Java.Interop/master@56c92c70 Mar 27, 2020
@jonpryor
Copy link
Member Author

Now that it builds, dotnet/java-interop#597 has been merged. Now to turn this PR into a "real" Java.Interop bump

@jonpryor
Copy link
Member Author

Squash-and-merge summary:

Bump to xamarin/Java.Interop/master@56c92c70 (#4376)

Commit body:

Changes: https://github.com/xamarin/Java.Interop/compare/1a086ffd51436ec3bb78467a150e4f9121d57419...56c92c70887c58275917752450126c8c39988aa2

  * xamarin/Java.Interop@56c92c7: [build] Remove cecil submodule (#597)
  * xamarin/Java.Interop@3091274: [build] Provide a default $(Configuration) value (#612)
  * xamarin/Java.Interop@cf3e7c2: [generator] Don't process duplicate reference assemblies (#611)
  * xamarin/Java.Interop@f5fa462: [jnienv-gen] Convert to SDK-style (#608)

Of particular note is [xamarin/Java.Interop@56c92c7][0], which
replaces the `mono/cecil` submodule within Java.Interop with the
[`Mono.Cecil` NuGet package][1] in an effort to simplify the
Java.Interop build system.

This simplifies the Java.Interop repo, and we *thought* that since
xamarin-android *doesn't even use* Java.Interop's cecil submodule-built
`Mono.Cecil.dll` -- instead the `Mono.Cecil.dll` from the
"mono archive" is "renamed" to `Xamarin.Android.Cecil.dll` during
`make prepare` (0c9f83b7) -- surely this would be a simple change.

The removal of the cecil submodule also required changing
`ThirdPartyNotice.txt` generation so that the LICENSE for Cecil was
obtained from the mono archive instead of from Java.Interop.

Unfortunately, the integration was a tad more complicated than
anticipated.  With the ongoing adoption of MSBuild multi-targeting
and builds against the `netcoreapp3.1` target framework -- commit
e2854ee7 and numerous commits in Java.Interop -- we encountered a
problem with MSBuild semantics: If two `$(TargetFramework)` builds
share the same output directory, the `IncrementalClean` target will
*remove files created by previous builds*, e.g. when e.g.
`Java.Interop/tools/generator.csproj` builds the `netcoreapp3.1`
framework, it will *delete* the `generator.exe` built by the `net472`
framework, which results in subsequent build breaks.

The only path to sanity is to *ensure* that different
`$(TargetFramework)` builds have *completely separate* `$(OutputPath)`
values.  The "normal" approach to doing this is for `$(OutputPath)`
to end with `$(TargetFramework)`, which is the case when
`$(AppendTargetFrameworkToOutputPath)`=True (the default).

Unfortunately in xamarin-android we don't want `$(OutputPath)` to end
with `$(TargetFramework)`; we want the build tree structure to mirror
the installation directory structure, which -- at present -- doesn't
mention `$(TargetFramework)` at all.

The solution here is to use "non-overlapping" directories.  For
example, in e2854ee7 there are "two" `$(OutputPath)` values:

  * `MonoAndroid10.0`: `bin/Debug/lib/xamarin.android/xbuild-frameworks/MonoAndroid/v10.0/Mono.Android.dll`
  *   `netcoreapp3.1`: `bin/Debug/lib/xamarin.android/xbuild-frameworks/Xamarin.Android.App/netcoreapp3.1/Mono.Android.dll`

The same "non-overlapping directories" strategy needs to be extended
to *all* multi-targeted projects from Java.Interop, *including*
dependencies.  Dependencies such as `Xamarin.Android.Cecil.dll`.

Define a new `$(UtilityOutputFullPathCoreApps)` MSBuild property so
that Java.Interop "utility" project builds, when building for the
`netcoreapp3.1` framework, use a *different* `Xamarin.Android.Cecil.dll`
than is used with the `net472`-related builds.

Update `xaprepare` to *create* this new `netcoreapp3.1`-correlated
`Xamarin.Android.Cecil.dll`.  It's the same file, just in a different
directory, to prevent "accidental" deletes by `IncrementalClean`.

Even with all that, MSBuild still had other ideas.  In particular,
MSBuild wasn't particularly happy about our attempt to use the
`$(UtilityOutputFullPath)` property to "switch" between using a
`@(PackageReference)` to the Mono.Cecil NuGet package vs. using a
`@(Reference)` to the `Xamarin.Android.Cecil.dll` assembly, because
MSBuild *caches* this information somewhere within `obj` directories.

To get MSBuild to re-evaluate it's assembly reference choices, we must
instead replace `msbuild` with `msbuild /restore`.

Which still isn't enough, because some of our MSBuild invocations are
via the `<MSBuild/>` task, within `msbuild`.  To get *that* working,
we need to explicitly invoke the `Restore` target through a *separate*
`<MSBuild/>` task invocation.  You ***CANNOT*** use
`<MSBuild Targets="Restore;Build" />`, as "obvious" as that may be,
because it [doesn't work reliably][2].  ([Yet.][3])

[0]: https://github.com/xamarin/Java.Interop/commit/56c92c70887c58275917752450126c8c39988aa2
[1]: https://www.nuget.org/packages/Mono.Cecil/0.11.2
[2]: https://github.com/microsoft/msbuild/issues/3000#issuecomment-417675215
[3]: https://github.com/microsoft/msbuild/issues/2811

@jonpryor jonpryor removed the do-not-merge PR should not be merged. label Mar 27, 2020
Now it's `jnienv-gen` breaking us. :-/

  /Users/runner/hostedtoolcache/dotnet/sdk/3.1.100/Sdks/Microsoft.NET.Sdk/targets/Microsoft.PackageDependencyResolution.targets(234,5): error NETSDK1004: Assets file '/Users/runner/runners/2.165.2/work/1/s/external/Java.Interop/build-tools/jnienv-gen/obj/project.assets.json' not found. Run a NuGet package restore to generate this file. [/Users/runner/runners/2.165.2/work/1/s/external/Java.Interop/build-tools/jnienv-gen/jnienv-gen.csproj]
@jonpryor jonpryor merged commit 295aff2 into dotnet:master Mar 27, 2020
@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.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants