Extension - renewed #1275
Extension - renewed #1275
Conversation
# Conflicts: # LICENSE # native/python/include/pyjp.h # native/python/pyjp_class.cpp # native/python/pyjp_module.cpp
# Conflicts: # native/common/include/jpype.h # native/python/pyjp_object.cpp
# Conflicts: # .bumpversion.cfg # jpype/__init__.py # native/java/org/jpype/JPypeContext.java # native/python/pyjp_module.cpp # pyproject.toml
|
Looks very interesting, but given that is uses asm should we make it optionally triggered behavior like jpype.imports. Perhaps jpype.extends or similar? That would allow android users who can't use asm to continue properly. |
|
With regard to asm, was it nessary to include asm in org.jpype as opposed to using it. We already have asm in the ivy dependency pull. Or do we need to modify it to get some behavior that isn't already available? |
|
Does this PR actually remove any current public behavior or could the still be considered source compatible with old code? I am trying to decide if the 2.0 is required. |
|
We will need to plan the order of merging. This is a very large PR and I see 3 such merges in queue.
I would recommend test clean up first, flatten and rebase this PR, then flatten rebase reverse. |
I'm not sure, I just left the vendored copy you had in place. |
It does not. There are a few changes that may have changed behavior in unseen ways but iirc all the tests passed so everything should be fine. I probably just made it 2.0 so I could tell which one had the extension support while working on it as jpype versions moved up. |
|
If you don't mind me doing some trim work I can see if I can get it running with the external copy. May need to add some interfaces so that the jar loads cleanly if the asm piece is present or not. |
I don't mind at all. I have my hands tied for a while because I volunteer myself to do too many things and can only hold on to interest for so long. The only risk taken with an external copy is conflicting versions in the classpath. Out of curiosity, what is the issue with Android and ASM? AFAIK the jdk itself has an internal vendored copy of ASM. Half of the motivating factor of the jdk24 class file API was to be able to remove the internal ASM copy. |
|
With regard to Android, my understanding is they use DEX not Java ASM on their side. Thus if we generate an ASM we would have to find some way to DEX it prior to loading. There is another DEX API that is similar to ASM, but I am not sure if we can afford to do both. When I did testing on Android I was able to get everything to run from JPype, except the ASM experiments. Now it may be been something wrong on my part as I am by no means a Android expert. But that is why at least for the core of JPype I have been avoiding doing things that require ASM.
|
If that is the case then it sounds like the simplest solution would just be a platform check in the python code when an extension class is being created. There shouldn't be any issues with the code being there since the reason isn't something like some android security thing seeing ASM and not letting it run. |
|
It is only a concern when creating dynamic classes. We can use it all we want at compile time, write a jar or class file then dex it. I have testing that path before. The problem happens if you want to create a new class in memory and load it. The Android class loader doesn’t take a normal class file.
|
Thrameos
added
the
enhancement
|
Because of the nature of this PR. I am going to rebase on to the foundational changes then split this into a few PRs before accepting. |
No problem. |
|
I will be rebasing this onto the current master. I will likely need to make it a new PR after the rebase. |
3 participants
Ignore the formatting changes for now. They will likely be easier to deal with, with a proper formatting setup. This way I can update to before the formatter was applied, pull in the changes, overwrite with everything I had and rerun the formatter to resolve everything.
There was an unrelated bug or two fixed here as well.
I don't do well with expressing myself or explaining things so documentation may be lacking but I did the best I could.
I haven't touched this since December so it's a bit behind and there are probably conflicts. I'll deal with them later as I'm just opening this to try and get my ass moving on it again.
supersedes #890