1 2 Previous Next 16 Replies Latest reply on Nov 6, 2012 9:10 PM by Andrew Rubinger

    SWR API Incompatible with JDK7 Compiler

    Andrew Rubinger Master

      Michal Matloka today noticed this issue with building SWR with JDK7:

       

        https://issues.jboss.org/browse/SHRINKRES-78

       

      I believe we have 3 options.

       

      1) Change the API so this isn't a problem

      2) Put some post-compilation step in place to fix up the bytecode (it'll work fine in the runtime, but the compiler is being overly aggressive IMO)

      3) Enforce that we build with JDK6

       

      Drawbacks to these:

       

      1) But the API we have is so nice!

      2) Complexity / feasibility.  If this can be worked through, it's my favorite option

      3) SWR in its current state will never be able to move to JDK7+ libraries or features.

       

      Again, we're in Alpha, so if we need to make API changes we *can*.  Opinions sought.  Go.

        • 1. Re: SWR API Incompatible with JDK7 Compiler
          Andrew Rubinger Master

          I *also* wonder if it's just the Oracle JDK7 compiler implementation which is introducing this restriction.  I'm not opposed to mandating we build the project with another vendor's compiler.

          • 2. Re: SWR API Incompatible with JDK7 Compiler
            Andrew Rubinger Master

            Fails with:

             

            IBM JDK: Java version: 1.7.0, vendor: IBM Corporation

            Oracle JDK: Java version: 1.7.0_03, vendor: Oracle Corporation

            OpenJDK: Java version: 1.7.0_147-icedtea, vendor: Oracle Corporation

            • 3. Re: SWR API Incompatible with JDK7 Compiler
              Andrew Rubinger Master

              This is legal compilation under JDK6:

               

                // access flags 0x401
                // signature (Ljava/lang/Class<Ljava/io/File;>;)[Ljava/io/File;
                // declaration: java.io.File[] as(java.lang.Class<java.io.File>)
                public abstract as(Ljava/lang/Class;)[Ljava/io/File; throws java/lang/IllegalArgumentException

               

                // access flags 0x401
                // signature (Ljava/lang/Class<Ljava/io/InputStream;>;)[Ljava/io/InputStream;
                // declaration: java.io.InputStream[] as(java.lang.Class<java.io.InputStream>)
                public abstract as(Ljava/lang/Class;)[Ljava/io/InputStream; throws java/lang/IllegalArgumentException

              • 4. Re: SWR API Incompatible with JDK7 Compiler
                Andrew Rubinger Master

                API Changes for debate:

                 

                  https://github.com/ALRubinger/resolver/commit/1da03cb29b39278e2404a76c625f0f0310d7507a

                 

                This renames:

                 

                as(File.class) > asFile();

                asSingle(File.class) > asSingleFile();

                 

                ...etc

                • 5. Re: SWR API Incompatible with JDK7 Compiler
                  Karel Piwko Master

                  I was thinking of making as() an SPI point. So methods would be T[] as(Class<T> clazz) and T asSingle(Class<T> clazz). T would be any type,

                  there would be a FormatterFactory, which will delegate to an impl which is able to handle T or fail with an exception.

                   

                  That way we:

                   

                  + 1/ Keep the API simpler and nicer

                  + 2/ Allow users to wrap results as whatever type they want

                  + 3/ It would force us to wrap ArtifactResult object into proper SPI

                   

                  - 1/ We loose type safe API in sense that user would be able to retype to JavaArchive.class even if that formatter is not on classpath, failing in runtime instead of compile time.

                   

                   

                  I have to do a prototype impl to see if that works first though.

                  • 6. Re: SWR API Incompatible with JDK7 Compiler
                    Andrew Rubinger Master

                    I believe 2) and 3) are still covered by my patch, which continues to provide:

                     

                    <RETURNTYPE> RETURNTYPE[] as(Class<RETURNTYPE> type, FormatProcessor<RETURNTYPE> processor)
                            throws IllegalArgumentException;
                    <RETURNTYPE> RETURNTYPE asSingle(Class<RETURNTYPE> type, FormatProcessor<RETURNTYPE> processor)
                            throws IllegalArgumentException, NonUniqueResultException, NoResolvedResultException;
                    

                     

                    In this way we still accept an extensible FormatProcessor without losing type safety (and IDE autocomplete makes it clear to the user what operations are permitted:

                     

                    asFile()

                    asSingleFile()

                    asInputStream()

                    asSingleInputStream()

                    asResolvedArtifactMetadata()

                    asSingleResolvedArtifactMetadata()

                    as(MyClass.class,new MyClassFormatter())

                    asSingle(MyClass.class,new MyClassFormatter())

                     

                    ...or am I missing some other benefit to your FormatterFactory approach?  If you put up a commit/patch for review we can debate the path to move us forward.

                    • 7. Re: SWR API Incompatible with JDK7 Compiler
                      Karel Piwko Master

                      Seen that, but currently occupied with other stuff. I'll definitely provide an implementation to discuss "soonish". The only benefit of my approach is that it avoids method explosion.

                      • 8. Re: SWR API Incompatible with JDK7 Compiler
                        Andrew Rubinger Master

                        Cool.  My preference will probably go for whatever's most intuitive, looking forward to seeing it.

                        • 9. Re: SWR API Incompatible with JDK7 Compiler
                          Karel Piwko Master

                          I finally managed to do the prototype impl. It is stored here:

                           

                          https://github.com/kpiwko/resolver/tree/SHRINKRES-78

                           

                          Changes done:

                          1/ An SPI service to replace existing one allowing to register multiple services per interface

                          2/ Changed FormatProcessor API

                          3/ Changed MavenFormatStageImpl to dynamically load FormatProcessors

                          4/ Changed FormatStage to allow any RETURTYPE type, this means we do not longer fail compile-time

                          5/ Aligned ResolvedArtifactInfo

                           

                          Side effect fixes:

                          1/ [not yet reported] Former SPI does not close input stream

                          2/ [SHRINKRES-79] MavenDependencyImpl exclusion ordering

                           

                          Changes to be done if this implementation is preferred:

                          1/ Create a proper SPI module

                          2/ Propagate registered services, e.g. search classpath only once

                          3/ Remove all other FormatStages and reduce number of entry points, simply generics in the project

                           

                          Karel

                          • 10. Re: SWR API Incompatible with JDK7 Compiler
                            Andrew Rubinger Master

                            Karel and I reviewed this in IRC, and iniitally I really liked this proposal.  On further inspection, though, I've found a problem:

                             

                            code_suggestion.png

                             

                            By registering the supported FormatProcessors at runtime, we lose the ability to give the users a hint as to what's supported at compile-time.  They know they're supposed to put in Class<SOMETHING>, but how are they to know that File.class, InputStream.class, etc are supported?

                             

                            For this reason and this reason alone, I'd prefer to go with the method-overload approach in my patch.  It's not as pretty, but IMO it does make for a more intuitive experience.

                             

                            Thoughts?

                            • 11. Re: SWR API Incompatible with JDK7 Compiler
                              Andrew Rubinger Master

                              Another hitch: it's impossible to resolve a ResolvedArtifactInfo from the FormatProcessor SPI - the contract doesn't provide enough info.

                              • 12. Re: SWR API Incompatible with JDK7 Compiler
                                Karel Piwko Master

                                As for FormatProcessor, its API is poorly designed...we need a specific object to be passed between Strategy and Formatter. Then we can have a contract which will make ResolvedArtifactInfo processor possible.

                                 

                                I'm thinking of a compromise. Having asFile() and as(Class<?> returnType). People need mostly files, right? More advanced types we can document.

                                • 13. Re: SWR API Incompatible with JDK7 Compiler
                                  Karel Piwko Master

                                  I've updated SHRINKRES-78 branch at https://github.com/kpiwko/resolver/tree/SHRINKRES-78 with following changes:

                                   

                                  1/ Refactored ResolvedArtifactInfo into ResolvedArtifact, which is now primary object passed from StrategyStage to FormatStage

                                  2/ Modified FormatProcessors to handle ResolvedArtifact (or its subtype) instead of File

                                  3/ Modified FormatStage to have following methods (e.g. used almost your approach while removing direct injection of FormatProcessors and adding asResolvedArtifact()):

                                  • as(Class<RETURNTYPE), asFile(), asInputStream(), asResolvedArtifact()
                                  • asSingle variants of above

                                  4/ ServiceRegistry which acts as an app-scoped container for registered services, solving ClassLoader propagation

                                  5/ Updated MavenResolvedArtifact to have the same API as FormatStage

                                   

                                  The only call where as(X) is needed is with as(Class<Archive<?>>). With SPI support, we still have possibility to trim number of interfaces and remove ShrinkWrapMaven altogether, as discussed on irc.

                                   

                                  swr-api-remake.png

                                  • 14. Re: SWR API Incompatible with JDK7 Compiler
                                    Andrew Rubinger Master

                                    Great hybrid approach, I think.  Well done.

                                     

                                    Currently in upstream/master:

                                     

                                     

                                    So there's no longer IDE autocomplete for as(JavaArchive.class), but I think that's livable, especially since we got to remove that pesky second ShrinkWrapMaven entry point (which served only to close the generic context in a way that extended the API).

                                     

                                    All of this in 2.0.0-alpha-5.  Not a bad release, I must say.  Thanks to all.

                                     

                                    S,

                                    ALR

                                    1 2 Previous Next