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

Support android 4.4 #881

Closed
wants to merge 5 commits into from
Closed

Support android 4.4 #881

wants to merge 5 commits into from

Conversation

erd
Copy link
Member

@erd erd commented Dec 15, 2013

No description provided.

@erd
Copy link
Member Author

erd commented Dec 15, 2013

The original PR somehow got detached from my repo, so I've recreated it. See #829 for previous discussion.

@erd
Copy link
Member Author

erd commented Dec 18, 2013

I've released the 4.4 android-all jar on Sonatype, so Travis should be able to build this branch.

@xian
Copy link
Member

xian commented Dec 19, 2013

Re Java 7: seems like we should be able to remap any references to AutoCloseable onto some other class, e.g. org.robolectric.AutoCloseable using Setup#classNameTranslations() -- I must have missed something in the AsmInstrumentingClassLoader. It'd be nice to not force an upgrade on our users.

@@ -113,7 +102,7 @@ public Implementation(Config baseConfig, Config overlayConfig) {
return overlayValue.equals(nullValue) ? baseValue : overlayValue;
}

@Override public int emulateSdk() {
@Override public int sdk() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we do this, it needs a backwards-incompatible-change release note.

@erd
Copy link
Member Author

erd commented Dec 19, 2013

Re Java 7: seems like we should be able to remap any references to AutoCloseable onto some other class, e.g. org.robolectric.AutoCloseable using Setup#classNameTranslations() -- I must have missed something in the AsmInstrumentingClassLoader. It'd be nice to not force an upgrade on our users.

I agree about Java 7. I wasn't sure how to do this, which is why I went down the Java 7 rabbit hole. I realized that we couldn't load AutoClosable from AOSP (since it's in java.lang), but I like the idea of loading it into a different package and rewriting the class name references.

@erd
Copy link
Member Author

erd commented Dec 19, 2013

I've removed the changes to the Config annotation and reverted the logging changes.

I'll see if I can figure out how to remap the AutoClosable class name, so we don't have to switch to Java 7.

@ChristianKatzmann
Copy link
Contributor

I'd love to see this branch merged into master. Any progress in the AutoClosable remapping so far?

@ry4n1m3
Copy link
Member

ry4n1m3 commented Jan 10, 2014

+1 love to see this merged. How goes Java 7 circumventing?

@erd
Copy link
Member Author

erd commented Jan 10, 2014

It is a giant pain in the ass.

Actually, that's not entirely true. I did manage to get the Robolectric tests passing against API 19, but run into problems running that build of Robolectric against an app built with API 18. I'm starting to worry that any changes we make to be compatible with KitKat will break compatibility with (recent) older versions.

@JurgenCruz
Copy link
Contributor

I lol'd

@erd
Copy link
Member Author

erd commented Feb 4, 2014

@xian I took another stab at not requiring Java 7 to build Robolectric with KitKat. I immediately ran into the problem that org.robolectric.res.builder.XmlFileBuilder extends XmlResourceParser in Android which extends AutoClosable. Since that's not in the classpath, Robolectric won't compile. This assumes that as part of supporting KitKit, we want to compile Robolectric against android-all-4.4 rather than the SDK.

@xian
Copy link
Member

xian commented Feb 10, 2014

@erd narf. I suppose we could do something horrible like change XmlResourceParserImpl's parent class at runtime... Is there a reason not to compile against an older version of the SDK?

@erd erd added the kitkat label Mar 20, 2014
@JaKXz
Copy link

JaKXz commented Apr 9, 2014

Just to be explicit, support for API 19 is not in the Sonatype 2.3-SNAPSHOT jar (as of April 9, 2014) correct?

@ChristianKatzmann
Copy link
Contributor

Unfortunately, the current snapshot does not contain support for API 19.

@JaKXz
Copy link

JaKXz commented Apr 9, 2014

Thanks @ChristianBecker. Is there a planned/estimated timeline?

@dodgex
Copy link
Contributor

dodgex commented May 14, 2014

is there a rough ETA when robolectric will officially support 4.4?

@jongerrish
Copy link
Contributor

Is the only issue blocking this PR just backwards compatibility? How important is it to continue to support users running tests using Java 6?

@JakeWharton
Copy link
Member

In my opinion, zero importance. But I'm not the only stakeholder.

@jongerrish
Copy link
Contributor

I tend to agree, I think its more important to get HEAD working with KK. I'd argue its OK to break compatibility. If you're eager enough for the latest Robolectric build you're probably eager enough to start building with Java 7. Its probably OK to stop support for Java 6 at Robolectric 2.3 and declare 2.4 Java 7 only. What do others think?

@mpost
Copy link

mpost commented Jul 11, 2014

I would agree. Drop java6 in order to support kitkat.

@tonycosentini
Copy link
Contributor

+1 This would be very convenient.

@JakeWharton
Copy link
Member

We should release 2.4 before making this change. 2.5 (or a 3.0) can be Java
7 only.
On Jul 10, 2014 8:49 PM, "Tony Cosentini" notifications@github.com wrote:

+1 This would be very convenient.


Reply to this email directly or view it on GitHub
#881 (comment)
.

@jongerrish
Copy link
Contributor

How about just cutting HEAD as 2.3.1 now, there are not many changes since 2.3 so maybe it doesn't warrant a major version number. I'd propose to submit this PR right after that and then you might want to release that also as 2.4, that way users who would rather take prebuilt versions rather than snapshot builds have access to KK.

So release:-

2.3.1 Latest current version without KK support.
2.4 Same as 2.3.1 + this PR.

Thoughts?

@chalup
Copy link
Contributor

chalup commented Jul 11, 2014

+1

@JakeWharton
Copy link
Member

Java 7 is breaking change. It should be a 3.0.

I don't care what the next version number is. It makes no difference and we've never placed weight or logic behind how we determine what the next one is.

@jongerrish
Copy link
Contributor

SGTM
On Jul 11, 2014 12:42 PM, "Jake Wharton" notifications@github.com wrote:

Java 7 is breaking change. It should be a 3.0.

I don't care what the next version number is. It makes no difference and
we've never placed weight or logic behind how we determine what the next
one is.


Reply to this email directly or view it on GitHub
#881 (comment)
.

@erd
Copy link
Member Author

erd commented Jul 14, 2014

The next Robolectric release will be 2.4. It's been about two months, so we should look at releasing in the next month or so. If we end up requiring Java 7 to make this PR work, we'll release it as version 3.0.

@erd
Copy link
Member Author

erd commented Jul 14, 2014

Is the only issue blocking this PR just backwards compatibility? How important is it to continue to support users running tests using Java 6?

It's not just a Java 6 -> Java 7 change. In making some of our shadows compatible with API 19, they no longer work when you run against API 18. Since we do want to support older Android SDKs, we might have to look into supporting version specific shadows. That might have some interesting ramifications.

@fpoyer
Copy link

fpoyer commented Aug 7, 2014

Until you actually find a way to do all that (support android 4.4/jdk 7 while still being compatible with previous SDKs), what is the current workaround to support JDK 7? Is there a configuration/parameter to set in one of the gradle build files? I'm using Android Studio 0.8.2 and compiling/running with openjdk-7-jdk and right now I'm getting the "java.lang.VerifyError: Expecting a stackmap frame" error as soon as I try to create an Activity containing a com.google.android.gms.maps.SupportMapFragment in my test...

@awrichar
Copy link
Contributor

Any updates on this? Anything I can do to help? Our project is moving to KitKat soon, and Robolectric has been an excellent unit test tool for us so far - would like to help get official KitKat support if we can.

@erd
Copy link
Member Author

erd commented Sep 25, 2014

I'm going to get a chance to work on this issue full time at work starting at the end of next month. We're planning on cutting the 2.4 release sometime soon, and will do another release when the KitKat work is finished. Trust me - I really want to get this out the door.

@seadowg
Copy link
Collaborator

seadowg commented Sep 29, 2014

@erd Is the plan to move to a 3.0 that targets a min of 19 or to keep supporting 16,17 and 18. It feels like the former might be simpler?

@xialin
Copy link

xialin commented Oct 1, 2014

👍 can't wait for it out!

@awrichar
Copy link
Contributor

awrichar commented Oct 7, 2014

Just to voice another opinion on the 1.6 -> 1.7 switch...for building the KitKat source, the required JDK version is 1.6. For L it's 1.7 (see http://source.android.com/source/initializing.html#installing-the-jdk). From my perspective, it would be good to line up with this if possible.

My personal stake is that I'm building a custom KitKat image on the same machine where I'm running these tests. Having to use Java 6 for the SDK build and Java 7 for the unit tests is cumbersome. I'm probably a rare case, but just wanted to point it out.

@fpoyer
Copy link

fpoyer commented Oct 8, 2014

@awrichar : this is slightly off topic, but as far as I can tell, JDK 1.6 being required means that this is the minimal version to be used, but you should be able to use any version after that, provided you set the source/target compatibility modes correctly (either through your pom.xml, build.gradle, javac flags, Eclipse project settings or whatever other mean used to compile your custom SDK).
I'm currently building pretty much everthing using the oracle 1.8 JDK, never had a problem so far.

@awrichar
Copy link
Contributor

awrichar commented Oct 8, 2014

@fpoyer Generally true, but with the Android system build, they literally run "java -version" from the makefile and parse the output to ensure it's Sun JDK 1.6. So from that side it's pretty locked in :(

I'm mainly wondering what the need is to raise the minimum to 1.7 for a KitKat release, since the KitKat code itself only needs 1.6. Did one of the build tools like aapt make a move that requires 1.7? If it's helpful for the majority of users, it obviously makes sense to do - just want to make sure the minimum requirements aren't raised unnecessarily.

@awrichar
Copy link
Contributor

Think I answered my own question - it looks like Google added usages of AutoCloseable in KitKat, despite the fact that they still require you to build with the Java 6 JDK. They include java.lang.AutoCloseable in their "core" package (basically their custom JRE). Of course, Robolectric chokes against Java 6 with either NoClassDefFoundError (if AutoCloseable isn't found) or SecurityException (when I tried to manually add java.lang.AutoCloseable to android-all).

So I'm on board with moving the minimum to 1.7 :) the AOSP source is slightly broken in this aspect; usage of AutoCloseable should require building with Java 7.

@erd erd closed this Nov 25, 2014
@crankycoder
Copy link

@erd is this now superseded by #1398 ?

@erd
Copy link
Member Author

erd commented Nov 25, 2014

Yes, see #1398

@erd erd deleted the support-android-4.4 branch December 2, 2014 03:59
@z2x-code
Copy link

z2x-code commented Nov 5, 2015

where to download the jar file with dependencies ?

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

Successfully merging this pull request may close these issues.

None yet