realm / realm-swift

Realm is a mobile database: a replacement for Core Data & SQLite
https://realm.io
Apache License 2.0
16.27k stars 2.14k forks source link

Realm SPM exports targets with names that are likely to conflict #7289

Closed patron-ghigley closed 3 years ago

patron-ghigley commented 3 years ago

The latest version of Realm exports products like Core and QueryParser via Swift Package Manager. These names are so general that they are likely to conflict with products exported from other packages. (In fact, this has already happened in the case of Core, which is why I'm here.) I propose renaming them RealmCore, RealmQueryParser, etc. instead.

Naming a product Core is probably fine for private code, but for public code used in a wide variety of projects, it is far too general. The temptation to call something Core will be very strong in another packages, make them fail to build with Realm.

My own opinion is that no one creating a public package should call a product Core or any other very general name.

dianaafanador3 commented 3 years ago

Hi @patron-ghigley As you can see in the package file https://github.com/realm/realm-core/blob/v11.0.3/Package.swift from our Core library, we do expose our products as RealmCore and RealmQueryParser, so there should be no conflict within your project.

patron-ghigley commented 3 years ago

Oddly enough, there is. Since our project is proprietary, I can't show you the code, but this evening I'll whip up a public pseudo-project that will show you the problem.

patron-ghigley commented 3 years ago

I think it's actually not a product conflict, but a target conflict.

dianaafanador3 commented 3 years ago

Hello again @patron-ghigley, True, targets on realm-core are named Core and QueryParser, but these are not exposed and should not be used outside of our SDK. I created a sample App creating my own module called Core and there was no conflict at all. So, If you can send us a code with a reproduction of this issue that will be very helpful.

patron-ghigley commented 3 years ago

Thanks @dianaafanador3! I admit it's a little mysterious, but we were able to fix it by pinning Realm to the previous version, 10.7.7.

I think the problem occurs when you have two SPM dependencies with targets of the same name, but I'm only guessing. So you'd need:

And then import them both into a sample app using SPM. That's what I'm going to try this evening.

patron-ghigley commented 3 years ago

@dianaafanador3 Yes indeed that is the problem. I created a package with a target (and product) named Core at https://github.com/patron-ghigley/Nada. (Feel free to use this one in your own test if you wish.) Then I created App and imported Core from Nada.

When I attempted to add Realm, this is the result:

Oops

Apparently, target names must be unique across packages. As a result, I suggest that "Core" is too general a name for an SPM target.

dianaafanador3 commented 3 years ago

@patron-ghigley Yeap there are a lot of changes introduced to the Package.swift file in Core from v10.7.7 to v10.8.0 including a change to a target name from Storage to Core that may be why it works for 10.7.7. We're introducing new datatypes (UUID, Dictionary, Set and Mixed) to the latest version something that you should not miss, staying at 10.7.0. One last question, Are these conflicts caused because of another third party library?, on the meantime I will inform core team about this issue.

patron-ghigley commented 3 years ago

@dianaafanador3 It's not a third party library, but an internal one. We've already got a ticket on our end to change that. However, the temptation to call a target "Core" or some other simple name is very strong. It should probably be avoided to prevent collisions like this.

In any case, thanks for your help with this.

dianaafanador3 commented 3 years ago

Closing this issue and passing this to the core team https://github.com/realm/realm-core/issues/4763