xamarin / XamarinComponents

Plugins for Xamarin
MIT License
1.99k stars 692 forks source link

AndroidX.JetBrains.Annotations – Package namespace conflict when binding native kotlin libraries #1176

Open joshuangfraedom opened 3 years ago

joshuangfraedom commented 3 years ago

When including the Kotlin standard library (jdk 7 version 1.5.0.1) I have a namespace conflict with JetBrains.Annotations which we leverage heavily.

For the time being I've worked around it with a target but would it be possible to remove this dependency?

moljac commented 3 years ago

Thanks for the feedback.

So, let me see if I understand this correctly.

This is kotlin stdlib pom.xml

https://repo1.maven.org/maven2/org/jetbrains/kotlin/kotlin-stdlib/1.5.0/kotlin-stdlib-1.5.0.pom

<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>
<groupId>org.jetbrains.kotlin</groupId>
<artifactId>kotlin-stdlib</artifactId>
<version>1.5.0</version>
<name>Kotlin Stdlib</name>
<description>Kotlin Standard Library for JVM</description>
<url>https://kotlinlang.org/</url>
<licenses>
<license>
<name>The Apache License, Version 2.0</name>
<url>http://www.apache.org/licenses/LICENSE-2.0.txt</url>
</license>
</licenses>
<developers>
<developer>
<name>Kotlin Team</name>
<organization>JetBrains</organization>
<organizationUrl>https://www.jetbrains.com</organizationUrl>
</developer>
</developers>
<scm>
<connection>scm:git:https://github.com/JetBrains/kotlin.git</connection>
<developerConnection>scm:git:https://github.com/JetBrains/kotlin.git</developerConnection>
<url>https://github.com/JetBrains/kotlin</url>
</scm>
<dependencies>
<dependency>
<groupId>org.jetbrains</groupId>
<artifactId>annotations</artifactId>
<version>13.0</version>
<scope>compile</scope>
</dependency>
<dependency>
<groupId>org.jetbrains.kotlin</groupId>
<artifactId>kotlin-stdlib-common</artifactId>
<version>1.5.0</version>
<scope>compile</scope>
</dependency>
</dependencies>
</project>

Removing nuget dependency Xamarin.Jetbrains.Annotations from Xamarin.Kotlin.StdLib would break Xamarin.Kotlin.StdLib and other libraries and users.

There are 2 workaround that could work:

  1. using fully qualified names in problematic files
  2. using aliases (using JetBrainsAnnotations=Jetbrains.Something; and using XamarinAnnotations=Xamarin.Something;)
moljac commented 3 years ago

closing this one.

joshuangfraedom commented 3 years ago

Sorry, missed the earlier message, Fully qualified namespaces don't work because the Xamarin.Jetbrains.Annotations package is in the JetBrains.Annotations namespace, thus you get a conflict no matter what you try aside from creating an alias at the assembly import level.

The 'fix' would be to use the Jetbrains.Annotations package instead of the Xamarin.Jetbrains.Annotations one, which only seems to exist for the sake of the kotlin stdlib packages.

Then you'd include the org.jetbrains.annotations jar and not expose it through C# so you don't get any namespace conflicts.

The alternative would be to re-namespace the Xamarin.Jetbrains.Annotations package to use the Xamarin.Jetbrains.Annotations namespace rather than sharing the Jetbrains.Annotations namespace

joshuangfraedom commented 3 years ago

That is to say, change the import here: https://github.com/xamarin/XamarinComponents/blob/589b5fb2befe500e1497e106e66a555fbf0eaf3a/Android/Kotlin/source/Xamarin.Jetbrains.Annotations/Transforms/Metadata.xml From:

<attr path="/api/package[@name='org.jetbrains.annotations']" name="managedName">JetBrains.Annotations<attr/>

To:

<attr path="/api/package[@name='org.jetbrains.annotations']" name="managedName">Xamarin.JetBrains.Annotations<attr/>

@moljac sorry not sure if Github will notify you with this now being closed.

moljac commented 3 years ago

If you check my work - I try to use <CompanyName>.<Namespace>.<Class> for nugets, namespaces and assemblynames, where CompanyName in most cases is Xamarin.

This is because of several reasons:

Seems I'll have to add these to readme.

Then you'd include the org.jetbrains.annotations jar and not expose it through C# so you don't get any namespace conflicts.

When we bind dependencies and transitive deps, we usually do not know whether API will be used from managed code (C#) or not. In most cases I am not API expert, so "the best bet" is to bind it with C# API surfaced and this is also due to the fact that we have no assurance that JetBrains will release their library like in this case.

Bottom line: will discuss it and see what can we/I do.

jpobst commented 3 years ago

Unfortunately, changing the namespace will break every application and NuGet package that currently uses these annotations, as .NET does not have a way to type forward to a different namespace.

If we just use the JetBrains version of JetBrains.Annotations, it will not contain the metadata needed to tie it back to the bound Java version (ie: the [Register] attributes):

namespace JetBrains.Annotations
{
    [Register("org/jetbrains/annotations/Contract", "", "JetBrains.Annotations.IContractInvoker")]
    public interface IContract : IAnnotation, IJavaObject, IDisposable, IJavaPeerable
    {
        [Register("pure", "()Z", "GetPureHandler:JetBrains.Annotations.IContractInvoker, Xamarin.Jetbrains.Annotations")]
        bool Pure();

        [Register("value", "()Ljava/lang/String;", "GetValueHandler:JetBrains.Annotations.IContractInvoker, Xamarin.Jetbrains.Annotations")]
        string Value();
    }
}

The only "safe" way to "rename" the namespace I can think of would be to create a second Xamarin.Jetbrains.Annotations2 package and have future versions of Kotlin stdlib depend on that. (That could still lead to scenarios where you need both packages, but I guess the types won't conflict since they'll be in different namespaces.)

joshuangfraedom commented 3 years ago

Ah of course that makes sense, the Jetbrains.Annotations package isn't a java binding.

~You've probably considered this, but there is also the possibility of dropping the dependency entirely, and including the jetbrains annotations jar as a reference jar which will avoid the binding of the library entirely so there'll be no possibility of conflicts.~

Just saw you posted twice, guess the options are pretty limited, I don't imagine a breaking change like this would be that bad to users. They'd need to replace the JetBrains.Annotations import with Xamarin.JetBrains.Annotations but I can't image there's any other issue?

jpobst commented 3 years ago

If it was simply an issue of a compile time error then it wouldn't be too big of an issue.

The bigger issue is already compiled libraries. That is, any already published NuGet referencing the existing library types would contain type references to ex: JetBrains.Annotations.IContract. If a user updates to the new Annotations package and there isn't a corresponding update to the other Nuget, their app will crash at runtime with something like Type JetBrains.Annotations.IContract could not be found. (Since the type would now be Xamarin.JetBrains.Annotations.IContract.)

jonpryor commented 3 years ago

@joshuangfraedom wrote:

Fully qualified namespaces don't work because the Xamarin.Jetbrains.Annotations package is in the JetBrains.Annotations namespace, thus you get a conflict no matter what you try aside from creating an alias at the assembly import level.

This is what the C# namespace alias qualifier is for. You can use global::JetBrains.Annotations.Type1 to explicitly use Type1 from the the JetBrains.Annotations namespace, even if your current namespace is nested within Xamarin.JetBrains.Annotations:

namespace JetBrains.Annotations {
    class AnotherExample {}
}

namespace Xamarin.JetBrains.Annotations {
    class Example {}

    class App {
        public static void Main ()
        {
            // CS0234 if `global::` isn't present on following line
            global::JetBrains.Annotations.AnotherExample e = null;
        }
    }
}
joshuangfraedom commented 3 years ago

@joshuangfraedom wrote:

Fully qualified namespaces don't work because the Xamarin.Jetbrains.Annotations package is in the JetBrains.Annotations namespace, thus you get a conflict no matter what you try aside from creating an alias at the assembly import level.

This is what the C# namespace alias qualifier is for. You can use global::JetBrains.Annotations.Type1 to explicitly use Type1 from the the JetBrains.Annotations namespace, even if your current namespace is nested within Xamarin.JetBrains.Annotations:

namespace JetBrains.Annotations {
    class AnotherExample {}
}

namespace Xamarin.JetBrains.Annotations {
    class Example {}

    class App {
        public static void Main ()
        {
            // CS0234 if `global::` isn't present on following line
            global::JetBrains.Annotations.AnotherExample e = null;
        }
    }
}

Hey @jonpryor For clarity, the problem is that both packages expose the same namespace, so you can't qualify it that way as for example the type is JetBrains.Annotations.NotNull in both assemblies. global::JetBrains.Annotations.NotNull will match the same type in both packages.

jonpryor commented 3 years ago

@joshuangfraedom wrote

For clarity, the problem is that both packages expose the same namespace, so you can't qualify it that way as for example the type is JetBrains.Annotations.NotNull in both assemblies.

That is what the C# namespace alias qualifier alongside extern aliases is for.

In your C# source code, you could do:

// At global scope
extern alias JetBrains;

Within your .csproj, add %(Aliases) metadata which uses the same value, e.g.

<ItemGroup>
  <PackageReference Include="JetBrains.Annotations" Version="…" Aliases="JetBrains" />
</ItemGroup>

%(Aliases) is a comma-separated or semicolon-separated list of values; one of the values should match an extern alias declaration. When these match, you can then use the C# namespace alias qualifier to reference types within the specified assembly:

extern alias JetBrains;

partial class Example {
    JetBrains::JetBrains.Annotations.NotNull notNull = null;
}

Note that when using %(Aliases), all types from the "aliased" assembly are not visible unless explicitly qualified. global::JetBrains.Anotations.NotNull will not resolve to JetBrains.Anotations.NotNull, JetBrains.Annotations; it will instead resolve to JetBrains.Anotations.NotNull, Xamarin.JetBrains.Annotations. (You can of course "reverse" this and make the Xamarin assembly use %(Aliases), in which case the JetBrains assembly could use global::.)

gtbuchanan commented 1 year ago

I ran into this problem when I upgraded Xamarin.AndroidX.AppCompat to 1.5.x because it now references Xamarin.Kotlin.StdLib. This workaround seems to work without having to specify a PackageReference for Xamarin.Jetbrains.Annotations:

<Target Name="HideXamarinJetBrains" BeforeTargets="FindReferenceAssembliesForReferences;ResolveReferences">
  <ItemGroup>
    <ReferencePath Condition="'%(FileName)' == 'Xamarin.Jetbrains.Annotations'">
      <Aliases>DoNotUse</Aliases>
    </ReferencePath>
  </ItemGroup>
</Target>
kp-xcess commented 1 year ago

@gtbuchanan Thanks! This solved the issue for me (using Xamarin Forms) whilst building the Android project.

Susko3 commented 5 months ago

In our specific use case, the namespace/type conflict was causing downstream issues. So the fix was to add PrivateAssets="compile" to the offending PackageReference.

This won't fix the issue in the base project, but any project that references it will be unaffected.

   <ItemGroup>
-    <PackageReference Include="Xamarin.AndroidX.Window" Version="1.2.0.1" />
+    <PackageReference Include="Xamarin.AndroidX.Window" Version="1.2.0.1" PrivateAssets="compile" />
   </ItemGroup>