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

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

Open
joshuangfraedom opened this issue May 19, 2021 · 14 comments

Comments

@joshuangfraedom
Copy link

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
Copy link
Member

moljac commented May 20, 2021

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
Copy link
Member

moljac commented May 31, 2021

closing this one.

@moljac moljac closed this as completed May 31, 2021
@joshuangfraedom
Copy link
Author

joshuangfraedom commented May 31, 2021

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
Copy link
Author

joshuangfraedom commented May 31, 2021

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
Copy link
Member

moljac commented Jun 2, 2021

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:

  • to be consistent with "Framework Design Guidelines" (though I have problems with camelCase)
  • to make potenitonal transfer easier (Xamarin.* to Microsoft.* for example)

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.

@moljac moljac reopened this Jun 2, 2021
@jpobst
Copy link
Collaborator

jpobst commented Jun 2, 2021

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
Copy link
Author

joshuangfraedom commented Jun 2, 2021

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
Copy link
Collaborator

jpobst commented Jun 2, 2021

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.)

This was referenced Jun 7, 2021
@jonpryor
Copy link
Member

jonpryor commented Jun 8, 2021

@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
Copy link
Author

@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
Copy link
Member

jonpryor commented Jun 9, 2021

@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
Copy link

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
Copy link

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

@Susko3
Copy link

Susko3 commented May 7, 2024

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>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants