appengine-java-vm-runtime icon indicating copy to clipboard operation
appengine-java-vm-runtime copied to clipboard

Is there a reason for using a repacked version of guava

Open aozarov opened this issue 8 years ago • 3 comments

see this list:

Targets
    Occurrences of 'repackage' in Project with mask '*.java'
Found Occurrences  (25 usages found)
    AppEngineWebXml.java  (3 usages found)
        19import com.google.appengine.repackaged.com.google.common.base.CharMatcher;
        20import com.google.appengine.repackaged.com.google.common.collect.Lists;
        21import com.google.appengine.repackaged.com.google.common.collect.Maps;
    FakeableVmApiProxyDelegate.java  (1 usage found)
        18import com.google.appengine.repackaged.com.google.protobuf.MessageLite;
    MultipartMimeUtils.java  (1 usage found)
        18import com.google.appengine.repackaged.com.google.common.io.ByteStreams;
    RemoteApiSharedTests.java  (4 usages found)
        27import com.google.appengine.repackaged.com.google.common.base.Supplier;
        28import com.google.appengine.repackaged.com.google.common.collect.LinkedListMultimap;
        29import com.google.appengine.repackaged.com.google.common.collect.Lists;
        30import com.google.appengine.repackaged.com.google.common.collect.Multimap;
    Restricted.java  (2 usages found)
        19 * We repackage this class into com.google.appengine.repackaged so that
        19 * We repackage this class into com.google.appengine.repackaged so that
    SessionManager.java  (1 usage found)
        18import static com.google.appengine.repackaged.com.google.common.io.BaseEncoding.base64Url;
    TestRepackagedAccess.java  (4 usages found)
        19 * {@link Restricted} gets repackaged into <code>com.google.appengine.repackaged</code>
        19 * {@link Restricted} gets repackaged into <code>com.google.appengine.repackaged</code>
        24public class TestRepackagedAccess implements Runnable {
        28"Class " + TestRepackagedAccess.class.getName() + "$%d loaded from";
    VmApiProxyDelegate.java  (1 usage found)
        30import com.google.appengine.repackaged.com.google.common.collect.Lists;
    VmApiProxyDelegateTest.java  (1 usage found)
        26import com.google.appengine.repackaged.com.google.common.collect.ImmutableMap;
    VmAppLogsWriter.java  (1 usage found)
        18import com.google.appengine.repackaged.com.google.common.base.Stopwatch;
    VmRequestThreadFactory.java  (4 usages found)
        18import static com.google.appengine.repackaged.com.google.common.base.Preconditions.checkArgument;
        19import static com.google.appengine.repackaged.com.google.common.base.Preconditions.checkState;
        21import com.google.appengine.repackaged.com.google.common.collect.ImmutableList;
        22import com.google.appengine.repackaged.com.google.common.collect.Lists;
    VmRuntimeUtils.java  (1 usage found)
        18import static com.google.appengine.repackaged.com.google.common.base.MoreObjects.firstNonNull;
    XmlUtils.java  (1 usage found)
        18import com.google.appengine.repackaged.com.google.common.io.Files;

We already depend on many other third-party libraries including apache httpcore, httpclient, commons-logging, commons-code and also google json. Any reason for treating guava differently?

aozarov avatar May 24 '16 22:05 aozarov

Also, I would expect that even though the runtime may depend on guava (or any other library) the application would be able to use their own independent version as they run in a separate/clean class-loader? /cc @gregw

aozarov avatar May 24 '16 23:05 aozarov

By default, any extra classes put on the jetty server class path are exposed to the webapplication. But it is an easy thing to configure in jetty and we have the concepts of:

  • server-classes: packages and/or classes that are used to implement the server and are hidden from the webapp, so they cannot be loaded by the application.
  • system-classes: packages and/or classes that are on the server classpath, are exposed to the webapplication, but cannot be replaced by the web application. eg java.lang.* is a system class pattern. Some deployments will make things like CDI system classes, others allow it to be an application component.
  • parent-loader-priority: The servlet spec breaks the java classloader contract in that it allows the application classloader to be used before trying the parent classloader (modulo in jetty the system/server classes above). We are able to turn that off if need be and run a strict java compliant classloader hierarchy if needed.

So I very much prefer to use all dependencies directly and not to rebundle and/or rename them.

We probably need to do a bit of work to improve the configuration of server-classes so that such classes are hidden (either at their normal names or even any renaming we do).

gregw avatar May 25 '16 01:05 gregw

So the repackaging is already done in the appengine-api-1.0-sdk-1.9.38.jar that we consume from maven repository:

com/google/appengine/api/
com/google/appengine/repackaged/com/fasterxml/jackson/
com/google/appengine/repackaged/com/google/api/client/
com/google/appengine/repackaged/com/google/api/client/util/store/
com/google/appengine/repackaged/com/google/appengine/api/search/
com/google/appengine/repackaged/com/google/common/
com/google/appengine/repackaged/com/google/datastore/
com/google/appengine/repackaged/com/google/gson/
com/google/appengine/repackaged/com/google/io/
com/google/appengine/repackaged/com/google/net/
com/google/appengine/repackaged/com/google/protobuf/
com/google/appengine/repackaged/com/google/protos
com/google/appengine/repackaged/com/google/rpc/
com/google/appengine/repackaged/com/google/storage/
com/google/appengine/repackaged/com/google/type/
com/google/appengine/repackaged/org/antlr/
com/google/appengine/repackaged/org/apache/commons/codec/
com/google/appengine/repackaged/org/codehaus/jackson/
com/google/appengine/repackaged/org/joda/time/
com/google/appengine/spi/
com/google/appengine/tools/
com/google/apphosting/api/
com/google/apphosting/base/
com/google/apphosting/client/datastoreservice/
com/google/apphosting/client/searchservice/
com/google/apphosting/client/serviceapp/
com/google/apphosting/datastore/
com/google/apphosting/utils/remoteapi/
com/google/apphosting/utils/servlet/
com/google/cloud/sql/jdbc/
com/google/cloudsearch/
com/google/protos/cloud/sql/
com/google/storage/onestore/
javax/activation/
javax/cache/
javax/mail/
javax/mail/event/
javax/mail/internet/
javax/mail/search/
javax/mail/util/
org/apache/geronimo/mail/

Note that it appears very inconsistent with what is or is not repackaged within that jar as some com.google packages are just bundled and not repackaged.

However, I'm guessing there is not much that we can do about this, unless there is a version of that artefact available in maven that does not bundle/repackage its dependencies? I've had a quick look and could not find one.

Long term it would be best to avoid bundling dependencies and impls within an API jar

Mid term it would be good to ask for a pur api jar artefact without bundled dependencies, so we could at least pull in and manage dependencies ourselves.

Short term we can probably not do much to avoid the repackaging, unless we are prepared to strip the jar and rewrite classes back to using the unrepackaged versions. So I will put this on hold for now and work on #259 to at least hide the dependencies from the webapp

gregw avatar Jun 02 '16 00:06 gregw