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

scala-collection-compat_2.12-0.1.0.jar does not contains scala.collection.BuildFrom due to osgi setting #30

Closed
xuwei-k opened this issue May 16, 2018 · 10 comments

Comments

@xuwei-k
Copy link
Contributor

xuwei-k commented May 16, 2018

// TODO: what should the setting be? This library also adds classes to other (existing) packages.
OsgiKeys.exportPackage := Seq(s"scala.collection.compat.*;version=${version.value}"),

@lrytz
Copy link
Member

lrytz commented May 16, 2018

Oh, it threw out everything that's not in the compat package... Too bad I didn't notice when looking at the jar.

@lrytz
Copy link
Member

lrytz commented May 16, 2018

Thinking how to go forward on this issue, I think it would be best to get rid of the split packages. Instead, the scala-collection-compat library should only expose types in the scala.collection.compat package.

This means we'd put the Factory and BuildFrom and ArraySeq (maybe collection.compat.immutable.ArraySeq?) classes into the compat package. In the 2.13 version, they would be type aliases.

Code that cross-compiles would use collection.compat.Factory. Once the library moves to 2.13 only, the code can change to collection.Factory.

@julienrf
Copy link
Contributor

What is the purpose of this osgi setting?

@lrytz
Copy link
Member

lrytz commented May 16, 2018

To use our jars with osgi they need to define metadata. Split packages are problematic, search on google. I personally have no experience with osgi, few people around me do. But the metadata is improtant for those that want to use it. Our working mode is to continue emitting the metadata, rely on the community to report and fix issues.

In any case, split packages will also cause problems with the java module system.

@julienrf
Copy link
Contributor

Ok, I think your solution would work well.

@lrytz
Copy link
Member

lrytz commented May 16, 2018

Note: the idea of putting ArraySeq into the collection.compat.immutable package is not a good one. Having two immutable packages has too much potential to cause conflicts and confusion.

@julienrf
Copy link
Contributor

julienrf commented May 16, 2018

The only problematic case I can think of is :

import collection.immutable
import collection.compat._

immutable.Something

Because in the last line immutable confusingly refers to the one in the compat object.

We can document this wart. I don't expect people to often import collection.immutable (in contrast with collection.mutable).

@szeiger
Copy link
Contributor

szeiger commented May 16, 2018

I don't know how much of a problem this really is for OSGi. The library is coupled tightly to the standard library anyway so a split package might be just fine. AFAIK Java 9 modules don't support split packages though, so we may have to change it for that reason alone (i.e. if we want to support modules).

@lrytz
Copy link
Member

lrytz commented May 17, 2018

I'm really not sure we should reuse immutable. I think we cannot just assume that noone imports collection.immutable.

scala> :pa -raw
// Entering paste mode (ctrl-D to finish)

package a { package imm { class C } }
package b { package imm { class D } }

// Exiting paste mode, now interpreting.


scala> import a._, b._
import a._
import b._

scala> new imm.C
res4: a.imm.C = a.imm.C@7761e342

scala> new imm.D
               ^
       error: type D is not a member of package a.imm

@som-snytt
Copy link

@lrytz Not sure what's on your previous lines. Normally, it's ambiguous, but REPL nests the imports. You don't need -raw for packages.

scala 2.12.6> :pa
// Entering paste mode (ctrl-D to finish)

package a { package imm { class C } }
package b { package imm { class D } }

// Exiting paste mode, now interpreting.


scala 2.12.6> import a._, b._
import a._
import b._

scala 2.12.6> new imm.C
<console>:18: error: type C is not a member of package b.imm
              new imm.C
                      ^

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

5 participants