twitter-archive/commons

com.twitter.common#jar-tool 0.1.8 & 0.1.9 are java 1.7 classfile jars

jsirois opened this issue · 9 comments

This breaks twitter/commons itself which tries to CI against 1.6 and 1.7 and blocks upgrading the twitter/commons jar-tool from 0.1.7. Either an 0.1.10 needs to be published using -source 6 -target 6 or else the twitter/commons travis-ci needs to be restricted to 1.7.

CI example here: https://travis-ci.org/twitter/commons/jobs/54192995
Important bit of the log:

[== Running python tests ==]
...
03:17:33 00:01   [test]
03:17:33 00:01     [run_prep_command]
03:17:33 00:01     [test]
03:17:33 00:01     [pytest]
03:17:35 00:03       [run]
...
                     ============== test session starts ===============
                     platform linux2 -- Python 2.7.3 -- py-1.4.26 -- pytest-2.6.4
                     plugins: cov, timeout
                     collected 1 items 

                     tests/python/twitter/common/zookeeper/kazoo_client_test.py F

                     ==================== FAILURES ====================
                     __________________ test_metrics __________________

                         def test_metrics():
                     >     with ZookeeperServer() as server:
                             event = threading.Event()

                     tests/python/twitter/common/zookeeper/kazoo_client_test.py:28: 
                     _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
                     /tmp/tmp27vSyP/twitter/common/zookeeper/test_server.py:101: in __init__
                         self.angrybird = self.setup_thrift()
                     _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

                     self = <twitter.common.zookeeper.test_server.ZookeeperServer object at 0x365cb90>

                         def setup_thrift(self):
                           if self._service is None:
                             time.sleep(self.INITIAL_BACKOFF)
                             for _ in range(self.CONNECT_RETRIES):
                               try:
                                 self._service = make_client(ZooKeeperThriftServer, 'localhost', self.thrift_port,
                                   protocol=TFinagleProtocol)
                                 break
                               except TTransportException:
                                 time.sleep(self.CONNECT_BACKOFF_SECS)
                                 continue
                             else:
                     >         raise self.NotStarted('Could not start Zookeeper cluster!')
                     E         NotStarted: Could not start Zookeeper cluster!

                     /tmp/tmp27vSyP/twitter/common/zookeeper/test_server.py:130: NotStarted
                     -------------- Captured stdout call --------------

                     03:18:12 00:00 [main]
                                    (To run a reporting server: ./pants server)
...
                     03:18:37 00:25   [binary]
                     03:18:37 00:25     [python-binary-create]
                     03:18:37 00:25     [binary]
                                        creating dist/angrybird.jar
                     03:18:37 00:25       [create-monolithic-jar]
                     03:18:37 00:25         [add-internal-classes]
                     03:18:37 00:25         [add-dependency-jars]
                     03:18:37 00:25         [bootstrap-jar-tool]
                     03:19:01 00:49         [jar-tool]
                                            ==== stderr ====
                                            java.lang.UnsupportedClassVersionError: com/twitter/common/jar/tool/Main : Unsupported major.minor version 51.0
                                                at java.lang.ClassLoader.defineClass1(Native Method)
                                                at java.lang.ClassLoader.defineClass(ClassLoader.java:643)
                                                at java.security.SecureClassLoader.defineClass(SecureClassLoader.java:142)
                                                at java.net.URLClassLoader.defineClass(URLClassLoader.java:277)
                                                at java.net.URLClassLoader.access$000(URLClassLoader.java:73)
                                                at java.net.URLClassLoader$1.run(URLClassLoader.java:212)
                                                at java.security.AccessController.doPrivileged(Native Method)
                                                at java.net.URLClassLoader.findClass(URLClassLoader.java:205)
                                                at java.lang.ClassLoader.loadClass(ClassLoader.java:323)
                                                at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:294)
                                                at java.lang.ClassLoader.loadClass(ClassLoader.java:268)
                                                at java.lang.Class.forName0(Native Method)
                                                at java.lang.Class.forName(Class.java:191)
                                                at com.martiansoftware.nailgun.NGSession.run(NGSession.java:242)
...

May I ask what would be the consequences if we restrict CI to 1.7? Do we know any client of these artifacts require 1.6?

There may be none. The customers are the transitive jvm customers of pants as a build tool so it would be best to query panst-devel. I think Square uses all of 6, 7 & 8 in their codebases, but I'm not sure if they use 1.6 as a runtime - they may use 1.7 & 1.8 or just 1.8 for the runtime. But @ericzundel will know for sure. In general I'd like to maintain compatibility with 1.6 in pants since we've always supported it and this tool just uses 1.6 language features. At some point pants really needs to fork its commons tools and own them over in pantsbuild so if Twitter needs commons publications to be 1.7 this might be a good time to fork jar-tool, compiler & specs runner.

Square's internal repos are using mostly 1.8 with some 1.7 usage. We're now rid of our 1.6 dependencies (in our main java repo anyway). A agree its good to keep a low level build tool like this at Java 1.6 unless we have a compelling reason to use more recent language features.

Erics response made me think more and I think its actually not viable for Twitter to start publishing the commons java 1.6 codebase (zookeeper, args, application, etc) in 1.7. That should break folks. I know AirBnB, Compass and Aurora all use bits of the commons code. There is no telling if they or their customers are forced to stay on 1.6 runtimes.

0.1.10 with java6 compatible bits is now published at:

http://maven.twttr.com/com/twitter/common/jar-tool/0.1.10/

Thanks @jinfeng!

OK - the transitive closure (or some subset) of the jar-tool's deps actually needed to be re-published:

22:28:54 00:33         [jar-tool]
                                            ==== stderr ====
                                            java.lang.UnsupportedClassVersionError: com/twitter/common/args/Arg : Unsupported major.minor version 51.0
                                                at java.lang.ClassLoader.defineClass1(Native Method)
                                                at java.lang.ClassLoader.defineClass(ClassLoader.java:643)
                                                at java.security.SecureClassLoader.defineClass(SecureClassLoader.java:142)
                                                at java.net.URLClassLoader.defineClass(URLClassLoader.java:277)
                                                at java.net.URLClassLoader.access$000(URLClassLoader.java:73)
                                                at java.net.URLClassLoader$1.run(URLClassLoader.java:212)
                                                at java.security.AccessController.doPrivileged(Native Method)
                                                at java.net.URLClassLoader.findClass(URLClassLoader.java:205)
                                                at java.lang.ClassLoader.loadClass(ClassLoader.java:323)
                                                at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:294)
                                                at java.lang.ClassLoader.loadClass(ClassLoader.java:268)
                                                at com.twitter.common.logging.RootLogConfig.<clinit>(RootLogConfig.java:49)
                                                at com.twitter.common.jar.tool.Main.main(Main.java:388)
                                                at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
                                                at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
                                                at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
                                                at java.lang.reflect.Method.invoke(Method.java:622)
                                                at com.martiansoftware.nailgun.NGSession.run(NGSession.java:280)

So jar-tool is now good, but it still depends on an args that java-7 only.

Before going further I'll ask on pants-devel about Twitter, twitter/commons and java 6 compat for all tools. If it turns out Twitter is now routinely only publishing to a java 7 minimum we'll continually get breaks like this and the time for pantsbuild to fork / move this code into its own repo is now.

Upgraded to org.pantsbuild maven artifacts in da94c93