mktany2k / funcito

Automatically exported from code.google.com/p/funcito
0 stars 0 forks source link

choosing CGLib versus Javassist #24

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
StubFactory is somewhat brittle. 

I suggest using a system property such as funcito.codegen.lib to specify the 
library. Values are 'CGLIB' and 'JAVASSIST' with the former being the default.

Original issue reported on code.google.com by codeto...@gmail.com on 26 Nov 2011 at 11:07

GoogleCodeExporter commented 9 years ago
I just wanted a place to document what the problems were with CGLib somewhere, 
so I chose this issue.  Anyway, for reference see:
* http://code.google.com/p/mockito/issues/detail?id=217#c7
* http://code.google.com/p/mockito/issues/detail?id=212
* http://in.relation.to/Bloggers/DeprecatedCGLIBSupport

I think some of the Mockito issues were worked around in CGLibImposterizer 
(originally ClassImposterizer in the Mockito project).  I don't know yet what 
they are referring to in Java 7 in the Mockito Issue 217 -- I haven't explored 
that yet.

Original comment by kandpwel...@gmail.com on 29 Nov 2011 at 6:57

GoogleCodeExporter commented 9 years ago
So can you unpack how this would work in practice?  Would the default value be 
hidden internal to code, or deployed in a resource file?  If deployed, are we 
now deploying in a zip rather than a simple jar, so that user can replace or 
update the resource file, etc...?

Original comment by kandpwel...@gmail.com on 2 Dec 2011 at 11:05

GoogleCodeExporter commented 9 years ago
FYI, "brittle" != defect.  Reclassifying.  I agree with the premise, but I 
still want to see your proposal unpacked some more.

Original comment by kandpwel...@gmail.com on 7 Dec 2011 at 5:28

GoogleCodeExporter commented 9 years ago
I'm actually very interested in this -- which probably doesn't come thru in my 
previous comments.  So I am raising the priority.  I would like to see this 
resolved before the first "real" release, which I am very unofficially hoping 
to have before the New Year.

Original comment by kandpwel...@gmail.com on 12 Dec 2011 at 7:09

GoogleCodeExporter commented 9 years ago
Here is one algorithm:

- look for a system property called 'funcito.codegen.lib'. If found, use the 
one specified
- Otherwise, search the classpath for both CGLib and Javassist
- if only one is present, then clearly: use that one
- if no property and both are on the classpath, use CGLib but write a warning 
message

This should scale fairly well if there are 3rd or 4th alternatives in the 
future.

Original comment by codeto...@gmail.com on 12 Dec 2011 at 8:59

GoogleCodeExporter commented 9 years ago
Sounds good.  Take it.

Also, as part of the "choosing the codegen lib" task, I want to be able to test 
that Javassist will be chosen if cglib is not present.  This would require a 
different classpath for the particular test.  I was looking for examples on how 
to do this, and so far all I could find was the idea of a custom test runner:

http://marc.info/?l=jakarta-commons-dev&m=112004597501874&w=2

I don't know how other projects handle testing for multiple version of library 
compatibility, but we could possibly use the above technique to test some major 
releases of cglib and javassist.  This to me is more important than testing 
versions of the functional libraries, since the codegen libs seems to be more 
brittle.

Original comment by kandpwel...@gmail.com on 12 Dec 2011 at 10:05

GoogleCodeExporter commented 9 years ago
90% complete. Need to write a warning and possibly some tests.

The current unit testing is pretty stout, as system-level predicates are pushed 
to their own classes, which are mocked.

I have verified the behaviour manually, using the command-line and a suite of 
CLASSPATH settings. I don't know how to automate this (re: your link).

I think we should probably have integration tests that are executed from 
Gradle, rather than JUnit.

Original comment by codeto...@gmail.com on 13 Dec 2011 at 11:38

GoogleCodeExporter commented 9 years ago
I just saw this last night and added some code review comments to the 
changelist. 

Original comment by kandpwel...@gmail.com on 15 Dec 2011 at 4:04

GoogleCodeExporter commented 9 years ago
adding milestone

Original comment by kandpwel...@gmail.com on 16 Dec 2011 at 6:06

GoogleCodeExporter commented 9 years ago
Review comments have been accepted and incorporated. Also, we write a warning 
to System.err when there are 2 codegen jars on the classpath but neither is 
explicitly specified (in which case we default to CgLib).

Original comment by codeto...@gmail.com on 16 Dec 2011 at 11:56

GoogleCodeExporter commented 9 years ago

Original comment by kandpwel...@gmail.com on 17 Dec 2011 at 4:02