[generator] .projitems shouldn't contain duplicate filenames #46
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Building [Mono.Android][0] with `msbuild` results in error MSB3105:
error MSB3105: The item
".../xamarin-android/src/Mono.Android/obj/Debug/android-23/Android.Annotation.SuppressLintAttribute.cs"
was specified more than once in the "Sources" parameter. Duplicate
items are not supported by the "Sources" parameter.
`Android.Annotation.SuppressLintAttribute.cs` is in the `Sources`
paameter twice becuase it's listed twice within
`Mono.Android.projitems`:
<Compile Include="$(MSBuildThisFileDirectory)\Android.Annotation.SuppressLintAttribute.cs" />
<Compile Include="$(MSBuildThisFileDirectory)\Android.Annotation.SuppressLintAttribute.cs" />
The question is *why* is this file -- and other files! -- repeated?
The answer is twofold:
1. Because `android.annotation.SuppressLint` is present *twice* within
the API XML description, once as the "real" `<interface/>`, and
once [manually added via `metadata`][1].
2. Because `ClassGen.cs` emits annotations.
(1) is done for backward compatibility reasons: long back in the mists
of time, Annotations were originally bound as "normal" classes instead
of as interface+custom attribute pairs, so if there are any ancient
binary assemblies referencing the class, that class needs to remain.
Consequently, removing (1) isn't an option.
That leaves (2): *both* `InterfaceGen.cs` and `ClassGen.cs` call
`GenerateAnnotationAttribute()`. `InterfaceGen.cs` -- respopnsible for
emitting `Android.Annotation.ISuppressLint.cs` in this example -- will
also emit `Android.Annotation.SuppressLintAttribute.cs`.
`ClassGen.cs` *also* emits
`Android.Annotation.SuppressLintAttribute.cs`.
The overall goal is to stop emitting duplicate files, as that will
allow `msbuild` to work properly. There are two plausible fixes:
1. `GenerationInfo.GenerateLibraryProjectFile()` could write out all
unique filenames, thus "ignoring" the duplicate file generation.
2. Fix `ClassGen.cs` to *not emit annotations*.
(1) is a hack. It works, but I consider it a hack.
I assert that (2) is the proper fix. Java annotations are interfaces,
so I see no plausible reason for `ClassGen.cs` to be calling
`GenerateAnnotationAttribute()` in the first place.
[0]: https://github.com/xamarin/xamarin-android/tree/b89dc15/src/Mono.Android
[1]: https://github.com/xamarin/xamarin-android/blob/b89dc15d/src/Mono.Android/metadata#L583-L588
Contributor
Author
|
With this fix in place, |
jonpryor
added a commit
to jonpryor/xamarin-android
that referenced
this pull request
Jun 11, 2016
DO NOT MERGE!!! TODO: Need the Java.Interop bump for the generator fix! dotnet/java-interop#46 Fix the solution and project files so that `msbuild` may be used to build the solution instead of requiring `xbuild`. There were a few issues that `msbuild` didn't like: 1. MSBuild doesn't like the "extra" configuration mappings in Xamarin.Android.sln. 2. MSBuild doesn't like the presence of `.dll` within `@(Reference)` entries. `<Reference Include="System.dll" />` is Bad™, so Don't Do That™.™. 3. Turning `$(AndroidSupportedAbis)` into an item group is...broken. (1) and (2) are straightforward fixes. (3) requires some explanation. `src/monodroid` needs to *only* build `libmonodroid.so` for the non-"host" ABIs within `$(AndroidSupportedAbis)`. It needs this restriction because non-selected ABIs may not be configured in `$(AndroidNdkDirectory)`, and thus can't be built. This *could* be done by following `build-tools/mono-runtimes/mono-runtimes.projitems` and doing lots of `Condition`s on `$(AndroidSupportedAbisForConditionalChecks)`: <_MonoRuntime Include="armeabi-v7a" Condition="$(AndroidSupportedAbisForConditionalChecks.Contains(':armeabi-v7a:'))" /> ... However, that's kinda ugly when *all* we need is the ABI name, so `monodroid.projitems` was "cute": <PropertyGroup> <_SupportedAbis>$(AndroidSupportedAbis.Replace(':', ';'))</_SupportedAbis> </PropertyGroup> <ItemGroup> <_MonoRuntime Include="$(_SupportedAbis)" Exclude="@(HostOSName)" /> </ItemGroup> <!-- @(_MonoRuntime) is `armeabi-v7a` by default --> This works...on xbuild, but *not* `msbuild`. Doh! (`msbuild` is "smart" and doesn't treat the `;` as an item separator, so if `$(AndroidSupportedAbis)` is `host-Darwin;armeabi-v7a` then MSBuild treats the `;` as part of the filename -- NOT a filename separator -- and `@(_MonoRuntime)` contains *one* item with an `%(Identity)` of `host-Darwin;armeabi-v7a`. On the one hand, this is kinda awesome and answers the question "how can you have a filename that contains `;`?", but on the other hand it broke my project!) The only fix I could think of was to use `.Split(':')`: <_MonoRuntime Include="$(AndroidSupportedAbis.Split(':'))" Exclude="@(HostOSName)" /> That provides desired behavior with `msbuild`, but `xbuild` doesn't support it and appears to either *ignore* it, or treat it literally. Other attempts to fix things so that `msbuild` behaves as desired would cause `xbuild` to break; they appear to be *mutually incompatible* here. To resolve this, add a new `<ValueToItems/>` task to `Xamarin.Android.Tools.BootstrapTasks.dll`. This new task takes a value, splits on a given separator, and turns it into an `<ItemGroup/>`, allowing particular items to be excluded: <ValueToItems Value="$(AndroidSupportedAbis)" Split=":" Exclude="@(HostOSName)"> <Output TaskParameter="Items" ItemName="_MonoRuntime" /> </ValueToItems> This allows for `@(_MonoRuntime)` to have the desired value on both `msbuild` and `xbuild`, allowing `src/monodroid` to build as desired.
jonpryor
added a commit
to jonpryor/xamarin-android
that referenced
this pull request
Jun 11, 2016
DO NOT MERGE!!! TODO: Need the Java.Interop bump for the generator fix! dotnet/java-interop#46 Fix the solution and project files so that `msbuild` may be used to build the solution instead of requiring `xbuild`. There were a few issues that `msbuild` didn't like: 1. MSBuild doesn't like the "extra" configuration mappings in Xamarin.Android.sln. 2. MSBuild doesn't like the presence of `.dll` within `@(Reference)` entries. `<Reference Include="System.dll" />` is Bad™, so Don't Do That™.™. 3. Turning `$(AndroidSupportedAbis)` into an item group is...broken. (1) and (2) are straightforward fixes. (3) requires some explanation. `src/monodroid` needs to *only* build `libmonodroid.so` for the non-"host" ABIs within `$(AndroidSupportedAbis)`. It needs this restriction because non-selected ABIs may not be configured in `$(AndroidNdkDirectory)`, and thus can't be built. This *could* be done by following `build-tools/mono-runtimes/mono-runtimes.projitems` and doing lots of `Condition`s on `$(AndroidSupportedAbisForConditionalChecks)`: <_MonoRuntime Include="armeabi-v7a" Condition="$(AndroidSupportedAbisForConditionalChecks.Contains(':armeabi-v7a:'))" /> ... However, that's kinda ugly when *all* we need is the ABI name, so `monodroid.projitems` was "cute": <PropertyGroup> <_SupportedAbis>$(AndroidSupportedAbis.Replace(':', ';'))</_SupportedAbis> </PropertyGroup> <ItemGroup> <_MonoRuntime Include="$(_SupportedAbis)" Exclude="@(HostOSName)" /> </ItemGroup> <!-- @(_MonoRuntime) is `armeabi-v7a` by default --> This works...on xbuild, but *not* `msbuild`. Doh! (`msbuild` is "smart" and doesn't treat the `;` as an item separator, so if `$(AndroidSupportedAbis)` is `host-Darwin;armeabi-v7a` then MSBuild treats the `;` as part of the filename -- NOT a filename separator -- and `@(_MonoRuntime)` contains *one* item with an `%(Identity)` of `host-Darwin;armeabi-v7a`. On the one hand, this is kinda awesome and answers the question "how can you have a filename that contains `;`?", but on the other hand it broke my project!) The only fix I could think of was to use `.Split(':')`: <_MonoRuntime Include="$(AndroidSupportedAbis.Split(':'))" Exclude="@(HostOSName)" /> That provides desired behavior with `msbuild`, but `xbuild` doesn't support it and appears to either *ignore* it, or treat it literally. Other attempts to fix things so that `msbuild` behaves as desired would cause `xbuild` to break; they appear to be *mutually incompatible* here. To resolve this, add a new `<ValueToItems/>` task to `Xamarin.Android.Tools.BootstrapTasks.dll`. This new task takes a value, splits on a given separator, and turns it into an `<ItemGroup/>`, allowing particular items to be excluded: <ValueToItems Value="$(AndroidSupportedAbis)" Separator=":" Exclude="@(HostOSName)"> <Output TaskParameter="Items" ItemName="_MonoRuntime" /> </ValueToItems> This allows for `@(_MonoRuntime)` to have the desired value on both `msbuild` and `xbuild`, allowing `src/monodroid` to build as desired.
Contributor
|
The change looks a bit scary, but I verified that there is no class that implements java.lang.annotation.Annotation in android.jar too. Normal Java annotations should be interfaces and this assumption should be valid. |
radekdoulik
added a commit
to radekdoulik/java.interop
that referenced
this pull request
Aug 27, 2018
Changes in xamarin-android-tools between 75530565b6aa903b3a0e52b61df4dd94475a19fc and 9e78d6ee586b498d0ea082b3bc00432c23583dd1: 9e78d6e (HEAD, origin/master, origin/HEAD, master) [tests] fix test failures on Windows (dotnet#47) bdf0158 Better support no installed JDKs on macOS (dotnet#48) 6353659 Log what is happening during path selection (dotnet#46) 3ef860b Take BUILD_NUMBER into consideration for Version sorting (dotnet#45) d3de054 Allow an optional locator to be provided to JdkInfo (dotnet#43) 917d3b3 Don't require quotes around `release` values (dotnet#41) 7427692 [tests] Unit tests for finding NDK location based on $PATH (dotnet#40) dbc517b Merge pull request dotnet#38 from jonpryor/jonp-ndk-via-path 511d580 Allow finding NDK location based on $PATH b42c217 [tests] Fix DetectAndSetPreferredJavaSdkPathToLatest() test (dotnet#37) a4aad18 Add AndroidSdkInfo.DetectAndSetPreferredJavaSdkPathToLatest() (dotnet#35) fae7e0a [tests] Remove temporary directories (dotnet#36) 07c4c2b [Xamarin.Android.Tools.AndroidSdk] Revert JDK validation (dotnet#34)
radekdoulik
added a commit
that referenced
this pull request
Aug 27, 2018
Changes in xamarin-android-tools between 75530565b6aa903b3a0e52b61df4dd94475a19fc and 9e78d6ee586b498d0ea082b3bc00432c23583dd1: 9e78d6e (HEAD, origin/master, origin/HEAD, master) [tests] fix test failures on Windows (#47) bdf0158 Better support no installed JDKs on macOS (#48) 6353659 Log what is happening during path selection (#46) 3ef860b Take BUILD_NUMBER into consideration for Version sorting (#45) d3de054 Allow an optional locator to be provided to JdkInfo (#43) 917d3b3 Don't require quotes around `release` values (#41) 7427692 [tests] Unit tests for finding NDK location based on $PATH (#40) dbc517b Merge pull request #38 from jonpryor/jonp-ndk-via-path 511d580 Allow finding NDK location based on $PATH b42c217 [tests] Fix DetectAndSetPreferredJavaSdkPathToLatest() test (#37) a4aad18 Add AndroidSdkInfo.DetectAndSetPreferredJavaSdkPathToLatest() (#35) fae7e0a [tests] Remove temporary directories (#36) 07c4c2b [Xamarin.Android.Tools.AndroidSdk] Revert JDK validation (#34)
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Building Mono.Android with
msbuildresults in error MSB3105:Android.Annotation.SuppressLintAttribute.csis in theSourcespaameter twice becuase it's listed twice within
Mono.Android.projitems:The question is why is this file -- and other files! -- repeated?
The answer is twofold:
android.annotation.SuppressLintis present twice withinthe API XML description, once as the "real"
<interface/>, andonce manually added via
metadata.ClassGen.csemits annotations.(1) is done for backward compatibility reasons: long back in the mists
of time, Annotations were originally bound as "normal" classes instead
of as interface+custom attribute pairs, so if there are any ancient
binary assemblies referencing the class, that class needs to remain.
Consequently, removing (1) isn't an option.
That leaves (2): both
InterfaceGen.csandClassGen.cscallGenerateAnnotationAttribute().InterfaceGen.cs-- respopnsible foremitting
Android.Annotation.ISuppressLint.csin this example -- willalso emit
Android.Annotation.SuppressLintAttribute.cs.ClassGen.csalso emitsAndroid.Annotation.SuppressLintAttribute.cs.The overall goal is to stop emitting duplicate files, as that will
allow
msbuildto work properly. There are two plausible fixes:GenerationInfo.GenerateLibraryProjectFile()could write out allunique filenames, thus "ignoring" the duplicate file generation.
ClassGen.csto not emit annotations.(1) is a hack. It works, but I consider it a hack.
I assert that (2) is the proper fix. Java annotations are interfaces,
so I see no plausible reason for
ClassGen.csto be callingGenerateAnnotationAttribute()in the first place.