search

CompEvol / beast2

Bayesian Evolutionary Analysis by Sampling Trees
www.beast2.org
GNU Lesser General Public License v2.1
240 stars 84 forks source link

Later Java versions complain about split packages #883

Open Anaphory opened 5 years ago

Anaphory commented 5 years ago

I just tried to run beast2 and BEASTLabs in Eclipse without restricting it to an old compatibility level of Java version, but I can't. The error is

The package beast.app.tools conflicts with a package accessible from another module: beast2

and so on for other split packages. Apparently, the Java modules interface (which you start to discuss in #834) really doesn't like packages with the same name in different modules, and all the different Beast add-ons are naturally different modules.

Do you have any intention of fixing this is the future? If so, how? It may seriously mess up the structure of existing add-ons and analyses, with all need their core packages renamed, but it sounds like a necessary thing for future-proofing BEAST2.

Anaphory commented 5 years ago

I mean, the easy fix is to make sure that I use classpaths only and not modulepaths, but nonetheless I wanted to bring this up. (And leave this note for posterity, in case someone else gets confused.)

alexeid commented 5 years ago

My opinion has been for a long time that BEAST2 add-ons should not share Java package name space with the BEAST2 core. However a lot of packages do share Java package name space with the core, partially because some of the core classes don’t have the right API design so that add-ons need to have package-protected access to methods or instance variables. This could be systematically fixed but it will take quite a bit of refactoring in a number of packages.

On 20/11/2019, at 5:45 AM, Gereon Kaiping notifications@github.com wrote:

I just tried to run beast2 and BEASTLabs in Java without restricting it to an old compatibility level of Java version, but I can't. The error is

The package beast.app.tools conflicts with a package accessible from another module: beast2 and so on for other split packages. Apparently, the Java modules interface (which you start to discuss in #834 https://github.com/CompEvol/beast2/issues/834) really doesn't like packages with the same name in different modules, and all the different Beast add-ons are naturally different modules.

Do you have any intention of fixing this is the future? If so, how? It may seriously mess up the structure of existing add-ons and analyses, with all need their core packages renamed, but it sounds like a necessary thing for future-proofing BEAST2.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/CompEvol/beast2/issues/883?email_source=notifications&email_token=AAG5MSNRGSXZUTYCWU23WPLQUQJ3RA5CNFSM4JPGGCM2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4H2M3PXA, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAG5MSJRJKCPHLW64V5XJ33QUQJ3RANCNFSM4JPGGCMQ.

rbouckaert commented 5 years ago

To deal with XML compatibility, the XMLParser can be made aware of how to map v2.6.x XML to equivalent classes in modules, which should not be too hard to implement. Each package can have its own configuration file with such mappings.

The bigger issue is that of refactoring from packages to modules. Perhaps a workable strategy is to identify packages that are easy to modularise (class A packages) and those that require significant work (class B). For v2.7.0, we can create a new core that contains the old core plus class B packages. This has the benefit of being able to modularise class B packages when time to do so is available, but of course hampers quick release cycles for class B packages which then need to be in sync with BEAST releases.

rbouckaert commented 4 years ago

With cd11f09 it becomes possible for each package to define map elements in the version.xml file that comes with the package. These have the <map from="xxx" to ="yyy"/> format, for instance

<map from='beast.evolution.ClassName' 
     to='mypackage.evolution.NewClassName'/>

and allow the XMLParser to recognise spec="beast.evolution.ClassName" from current XML to be mapped to class mypackage.evolution.NewClassName. It takes the class path in account: if beast.evolution is in the class path defined in the XML, it also maps spec="ClassName" to mypackage.evolution.NewClassName.