Refactor Maven, add maven-openjdk8 module #372
Conversation
jmtd
changed the title
|
I've un-WIPPED this. I think it's OK if a bit dramatic. In particular I'd like feedback on the proposed version scheme. TIA! |
We don't need to separate out the API since there is only one implementing module. Simplify things by merging them down to a common jboss.container.maven.s2i module. Signed-off-by: Jonathan Dowland <[email protected]>
Move to a scheme where we encode the RHEL major.minor version into the module version. So, jboss.container.maven.35.35scl provides Maven 3.5 via SCL for RHEL7.0+. The version is thus 7.0.3.5. Rename the four affected modules to a common module name "jboss.container.maven" and change their file paths down accordingly. Update jboss.container.maven.default.* which depend upon the above. Images using these modules will need to update. Signed-off-by: Jonathan Dowland <[email protected]>
The module name jboss.container.maven.default.bash was provided by two modules in order to support Maven version 3.5 and 3.6 (See a6ddba6 for details) The source differences between the two modules were very minor. Adjust jboss.container.maven.default.bash.default to remove explicit references to maven versions, and delete the 36 version. Rename, move and re-version the remaining module as jboss.container.maven.default (no .bash suffix). The new module name means we can side-step the version epoch issue. Dependent images will need to adjust to the new name and remove any explicit module version (instead depend upon the specific Maven required) Signed-off-by: Jonathan Dowland <[email protected]>
Signed-off-by: Jonathan Dowland <[email protected]>
|
why remove the bats tests? |
|
@jmtd I like that there will only be one maven module, and while I don't like the version scheme looking at it initially, I don't really see a way to avoid this (the 8.2.3.6.8 one I had to re-read your comment after looking at the diff to remember what the last .8 .meant). I agree with @spolti though, we use bats quite a lot for testing pull requests to jboss-eap-modules etc, so I'd recommend keeping them if you can (we can set up PR tests for this repo if you like.) |
|
If I’ve deleted the bats tests it’s a mistake and I’ll revert that bit. But I think you’re seeing one copy of them deleted in the diff from the merge of JBoss.container.maven.default.* and the other copy is still there |
Yep just checked, jboss/container/maven/default/bash/default/tests/bats is the new home for them. |
Thanks for your feedback! Yes, the version scheme leaves a bad taste in my mouth. If we could use dashes as well as dots it would look a lot better, but alas PEP440 does not support that. |
This is WIP so you can get an early look at what I'm thinking. I'm still performing extensive testing.
Ultimately this is all in aid of adding a module/version that will install maven-openjdk8 specifically (for use on RHEL 8.2+)
Based on top of #371
A brief summary of what i've done here
jboss.container.maven.s2i.{bash,api}to justjboss.container.maven.s2ijboss.container.maven.??modules to common namejboss.container.mavenwith a four-part version: RHEL major . RHEL minor . Maven major . Maven minor. This is PEP440 compliant so will silence cekit warnings. E.g. version 3.5scl (which implicitly was RHEL 7 only) becomes 7.0.3.5.jboss.container.maven.default.*down to a single modulejboss.container.maven.default.jboss.container.maven8.2.3.6.8 (RHEL 8.2, Maven 3.6, JDK8, yep that's an optional fifth-part version number. Since 8.2.3.6 lacks the fifth part, this will sort higher, so JDK8 will be chosen by default)My starting point was "this is really confusing can't we have a single
mavenmodule name?" and I got as close as I could.jboss.container.maven.defaultcould now potentially be merged intojboss.container.maven.api.