Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Version 0.10.0 - Static Language Support #304

Closed
wants to merge 14 commits into from

4 participants

@benjchristensen

Manual merge of pull #300 from @mattrjacobs

This will make RxJava completely static by removing all Object overloads (see #208 and #204).

I'm submitting this before it being 100% ready so people can review and provide feedback.

Open items to append to this pull request before merging:

1) subscribe with map is not handled yet

The following signature needs to be made static. Right now the lack of this combined with removal of Functions.from dynamic language functionality has broken this.

public Subscription subscribe(final Map<String, Object> callbacks)

2) Core artifact naming convention

Should rxjava-core-x.y.x.jar become rxjava-x.y.z.jar since the concept of core+language no longer applies?

I think I'd prefer this:

  • rxjava-x.y.z.jar
  • rxjava-groovy-x.y.z.jar
  • rxjava-clojure-x.y.z.jar
  • rxjava-scala-x.y.z.jar
  • rxjava-jruby-x.y.z.jar
  • rxjava-kotlin-x.y.z.jar
  • rxjava-dynamic-x.y.z.jar (object overload for any language)
  • rxjava-groovy-clojure-x.y.z.jar (multi-language jar)

Only one of those jars is needed hence the reason why I think the 'core' term is no longer needed as it communicated the fact it was always needed.

Any contrib modules would be: rxjava-contrib-module-name-x.y.x.jar

3) Dependencies from languages to core still exist

The build still will result in Maven Central POM files requires rxjava-core from the language version despite that not being the case. Need to eliminate this dependency in the artifact.


Implementation notes originally posted at #204 (comment):

After implementing and throwing away a few different approaches we have landed on a solution we feel will balance the various competing priorities.

Our goals are:

  • support static typing for Java/Scala/Kotlin etc by removing the Object overloads
  • support any JVM language, static or dynamically typed
  • allow all languages to use the same rx.Observable class so that we don't divide libraries with helpers such as GroovyObservable, ClojureObservable etc that then need to be converted back and forth when doing interop
  • do not require special classloaders or agents to enable runtime bytecode generation
  • do not remove static operators to enable proxying
  • small jars and limited or no dependencies

The solution we have arrived at will work as follows:

  • The rxjava-core source code will delete all Object overload methods and be pure static java.
    • Any language that supports functional interfaces directly (such as Java 8 and XTend) can use the Java core version directly.
  • Languages needing specific lambda/clojure type mapping to the Func/Action types will have language specific Jars created via build-time bytecode generation.
    • Any method with a Func/Action argument will be overloaded with a version supporting the language requirements.

For example:

The default Java version:

public static <T> Observable<T> filter(Observable<T> that, Func1<T, Boolean> predicate)

A Groovy version:

public static <T> Observable<T> filter(Observable<T> that, groovy.lang.Closure predicate)
  • A jar per language will be created as follows:
    • rxjava-x.y.z.jar
    • rxjava-groovy-x.y.z.jar
    • rxjava-clojure-x.y.z.jar
    • rxjava-scala-x.y.z.jar
    • rxjava-jruby-x.y.z.jar
    • rxjava-kotlin-x.y.z.jar

A project will include just the jar that meets their language needs, there will no longer be a "core" jar plus the language adaptor.

The drawback of this is that mixing two of these in a classpath will result in non-deterministic loading (whichever is loaded last wins) and that is the version that will be used. This means if a library depends on rxjava.jar but is using Groovy and needs rxjava-groovy.jar it is up to the developer of that project to make sure they have only the rxjava-groovy.jar version. This is not ideal but is a one-time pain setting up a build and is better than the constant pain of missing static typing or converting to/from different Observable implementations for different languages.

  • At this time we are optimizing for projects using a single language or Java + another language. If there are use cases where people are trying to mix multiple languages in a very polyglot manner we have two options:

    • include an rxjava-dynamic.jar version that re-adds the Object overloads
    • include build configs for common combinations of languages such as rxjava-groovy-clojure.jar
  • Language adaptations (such as clojure which has preferred idioms that necessitate wrapping) will still be possible through the language-adaptor projects and be included in the appropriate language jars.

This should not break any code but will require a slight change to the build dependencies in your project when we release this.

We hope that this enables the RxJava project to better achieve its goal of being polyglot and targeting the JVM in general and not any specific languages without sacrificing interop or idiomatic usage in each of the languages.

mattrjacobs and others added some commits
@mattrjacobs mattrjacobs Make rxjava-core typesafe 7f1a989
@mattrjacobs mattrjacobs Add Code generator that adapts the core Observable to a dynamic langu…
…age's native function support

* Only hooked up to rxjava-groovy in this commit
c93ba88
@mattrjacobs mattrjacobs Clean up rxjava-groovy Gradle build bbedef2
@mattrjacobs mattrjacobs Get rxjava-clojure build working in Gradle 698508f
@mattrjacobs mattrjacobs Get rxjava-jruby build working in Gradle 5ac401d
@mattrjacobs mattrjacobs Added rxjava-dynamic language-adaptor
* Adds an Object overload to Observable methods via code generation
d4efc29
@mattrjacobs mattrjacobs Reworked Scala adaptor to use implicits in RxImplicits, rather than c…
…ode generation
4d45c10
@mattrjacobs mattrjacobs Added license to files I touched and moved Gradle config around 5d10dbf
@mattrjacobs mattrjacobs Bugfix to codegeneration - explicitly call a static/instance method i…
…n body
73c8631
@benjchristensen benjchristensen Merge branch 'multi-static' of git://github.com/mattrjacobs/RxJava in…
…to version_0.10.0_multi-static

Conflicts:
	rxjava-core/src/main/java/rx/Observable.java
8beb820
@benjchristensen benjchristensen Remove '? super' generic definitions
I couldn't get these to work once we removed the Object overloads that was apparently being invoked instead of the staticly typed method so removing them.
If someone else can get these generics working then please submit a new pull request.
227c442
@benjchristensen benjchristensen Removing zip FuncN overloads - they conflict with dynamic languages
These methods will need different names as they have the same argument count so break with dynamic overloads once we do static handling of dynamic languages.
b831bf8
@benjchristensen benjchristensen Upgrade to Gradle 1.6
- needed for Scala build to succeed
add859f
@benjchristensen benjchristensen Version 0.10.0 e8373fa
@cloudbees-pull-request-builder

RxJava-pull-requests #174 SUCCESS
This pull request looks good

@mattrjacobs
Collaborator
  1. I don't think I follow your point about making the subscribe(Map<String, Object>) method static. Wouldn't that break all existing scripts that use this method? Could we add a subscribe(Map<String, Closure>) or subscribe(Map<String, IFn>) variant in the dynamic JARs? That would also suggest tightening up the core signature to subscribe(Map<String, Function>).

  2. This naming convention makes sense to me. I will change rxjava-core to rxjava in the artifact name, at least. I think I will leave the directory rxjava-core alone, since it makes more sense there, and IMO, it would be confusing to have a dir like RxJava/rxjava/... I don't plan on bringing rxjava-kotlin along with this pull request, but I can review #292 to help fit that into the 0.10 world.

  3. I think I understand this, though I'm already at the edge of my Gradle ability. If I'm understanding this correctly, we should not change the build steps for a project - we just need to change the generated Maven POM. That makes sense - I'll figure out a way to accomplish that.

@jmhofer

Looks good, all in all. I'll try this out with my Scala sample as soon as possible. Thanks for the awesome work!

@benjchristensen

Thanks @jmhofer for stepping in to test out these changes.

@mattrjacobs Answers to your questions ...

1) subscribe(Map)

If we leave the Map<String, Object> then the Functions.from capability needs to keep working.

The other option is we change that to be Map<String, Function> as you suggest and then auto-generate the versions for each language.

2) Thanks, and agreed on the directory name, but that can change if we want in the future without impacting the artifact name once you set the name.

3) Maven POM

I think the simplest solution is just not have any "compile" dependencies, only "provided". I believe your Gradle scripts just need to adjust to include provided dependencies, not just compile runtime dependencies and then it will work. I would not try messing with POM files directly. If something is specified as "provided" then it is used for compilation but not included in the POM when pushed to Maven Central.

@cloudbees-pull-request-builder

RxJava-pull-requests #175 SUCCESS
This pull request looks good

@cloudbees-pull-request-builder

RxJava-pull-requests #176 FAILURE
Looks like there's a problem with this pull request

@cloudbees-pull-request-builder

RxJava-pull-requests #177 SUCCESS
This pull request looks good

@jmhofer

Currently, if you publish RxJava to your local Maven repo, you'll find that the rxjava-scala and rxjava-swing POMs include a dependency on "rxjava-core", which doesn't exist anymore as a module, as it was renamed to "rxjava".

Unfortunately, I don't know enough about Gradle to be able to fix this. My guess would be that it's not enough to set the baseName of the JAR to just "rxjava". There should be a way to tell gradle that the module is called "rxjava" even though the directory is called "rxjava-core".

@jmhofer

Here's another problem: Excluding "*$UnitTest" from JARs that might be included by Scala will cause the Scala compiler to crash with a Typer error because it expects defined inner classes to exist. This means that at least in rxjava-swing, the exclude can't be done.

@jmhofer

I've created a mini-PR against the unit test problem here: benjchristensen#1

@jmhofer

I got my little sample (https://github.com/jmhofer/rxjava-samples) working with this PR. The code itself was very easy to adapt. The implicits seem to be working fine.

One small thing I noticed: The very long names of the implicits are a bit distracting, imho. Should I want to use one of these functions explicitly, I'd prefer shorter names like simply func1 instead of scalaFunction1ToRxFunc1, for example.

@mattrjacobs
Collaborator

Responses to @jmhofer:

  1. POM references to rxjava-core: In the Scala case, I prefer including all of the core Rx classes directly in the rxjava-scala.jar, so that there is no dependency at all. This avoids the problem entirely. For rxjava-swing, I'll work on the Gradle side to make the reference correct. This might take the form of changing the reference or actually changing the name of rxjava-core to rxjava - we'll see how friendly Gradle is.

  2. Excluding inner classes: I've also come across the Scala compiler error on Classfile parsing with excluded inner classes. At the moment, unit tests exist in rxjava-core.jar and rxjava-scala.jar only. Going forward:

    • rxjava-core: These unit tests only exist so that rxjava-scala can be built. I will exclude them, add a new unfilteredJar Gradle task, and build rxjava-scala off of unfilteredJar.
    • rxjava-scala: I will continue to include UnitTests, since it's pretty likely that Scala projects are consuming this
    • rxjava-swing: I hadn't considered the case of Scala consuming this, but you're obviously right. For now, I'll err on the side of caution and unilaterally add the UnitTests back to the jar. At some later point, we might want to re-examine (rxjava-swing-scala.jar/rxjava-swing-filtered.jar). But adding them back is the safe thing to do. Your PR is exactly what I will do, thanks for that.
  3. Implicit names: In my experience, it's rare that I call implicit conversions by name, but I can see your point. My only concern is ambiguous namings. Would changing 'scalaFunction1ToRxFunc1' to 'toRxFunc1' (and other similar naming changes) be satisfying?

@jmhofer

@mattrjacobs: All your suggestions seem sensible to me.

Concerning implicit naming: It may be relatively rare, however using the implicits often leads to your having to specify the parameter types of the closures being implicitely converted, which can be more verbose and confusing than making the conversion explicit and then being able to shorten the closure syntax.

Names like toRxFunc1 are ok with me, not too long and still unambiguous, sounds like a good compromise.

@cloudbees-pull-request-builder

RxJava-pull-requests #181 SUCCESS
This pull request looks good

@cloudbees-pull-request-builder

RxJava-pull-requests #182 SUCCESS
This pull request looks good

@cloudbees-pull-request-builder

RxJava-pull-requests #183 SUCCESS
This pull request looks good

@cloudbees-pull-request-builder

RxJava-pull-requests #188 FAILURE
Looks like there's a problem with this pull request

@benjchristensen

Replaced by #319

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jul 4, 2013
  1. @mattrjacobs

    Make rxjava-core typesafe

    mattrjacobs authored
  2. @mattrjacobs

    Add Code generator that adapts the core Observable to a dynamic langu…

    mattrjacobs authored
    …age's native function support
    
    * Only hooked up to rxjava-groovy in this commit
  3. @mattrjacobs
  4. @mattrjacobs
  5. @mattrjacobs
  6. @mattrjacobs

    Added rxjava-dynamic language-adaptor

    mattrjacobs authored
    * Adds an Object overload to Observable methods via code generation
  7. @mattrjacobs
  8. @mattrjacobs
  9. @mattrjacobs
Commits on Jul 5, 2013
  1. @benjchristensen

    Merge branch 'multi-static' of git://github.com/mattrjacobs/RxJava in…

    benjchristensen authored
    …to version_0.10.0_multi-static
    
    Conflicts:
    	rxjava-core/src/main/java/rx/Observable.java
  2. @benjchristensen

    Remove '? super' generic definitions

    benjchristensen authored
    I couldn't get these to work once we removed the Object overloads that was apparently being invoked instead of the staticly typed method so removing them.
    If someone else can get these generics working then please submit a new pull request.
  3. @benjchristensen

    Removing zip FuncN overloads - they conflict with dynamic languages

    benjchristensen authored
    These methods will need different names as they have the same argument count so break with dynamic overloads once we do static handling of dynamic languages.
  4. @benjchristensen

    Upgrade to Gradle 1.6

    benjchristensen authored
    - needed for Scala build to succeed
  5. @benjchristensen

    Version 0.10.0

    benjchristensen authored
Something went wrong with that request. Please try again.