search

lucee / lucee-dockerfiles

Official Lucee Dockerfiles for Docker Hub build images
https://hub.docker.com/u/lucee/
MIT License
85 stars 50 forks source link

Wip/micha/remove config #86

Closed michaeloffner closed 3 months ago

michaeloffner commented 3 months ago

I completely remove the Lucee config, Lucee will create the correct config for itself and this is faster than adding a content with no real benefit.

michaeloffner commented 3 months ago

i moved the tomcat config in a separate folder, we don't need a different tomcat config for the different versions of Lucee

justincarter commented 3 months ago

The way the builds are triggered the master branch is used for all builds regardless of the version of Lucee (old supported versions, or new versions that we want to support).

The minor version folders were necessary because different versions of Lucee sometimes supported different versions of extensions, and so this was the only way to maintain backwards compatibility if we needed to do a build for older versions of Lucee. The hope was also that it would support future builds, without having a separate Dockerfile -- the addition of the .cfconfig.json means the Dockerfile would need to change though (even if we simply don't copy that file in, we'd be removing the XML file copy).

At the moment, I think we need to keep the existing folder structure to continue supporting Lucee 5.4, and if we remove Lucee 5.0/5.1/5.2/5.3 from the build process then we can remove those folders.

I think we should close this PR and then I can do the tidy up elsewhere?

michaeloffner commented 3 months ago

@justincarter we can keep the Lucee 5 folders, but in my opinion they bring no benefit for Lucee 5, on contrary. Extension definition in the xml config files of Lucee 5 are just informative (like a log) and do NOT trigger any action.

In addition every Lucee version (5 and 6) comes with instructions for required extension build in https://github.com/lucee/Lucee/blob/6.1/core/src/main/java/META-INF/MANIFEST.MF#L364

adding this files just mess around with that and should be avoided.

I have analysed all the config files and they have not benefit over the build in config Lucee comes with, they simply mess around and slow down the start up because they unnecessarily customize Lucee.

michaeloffner commented 3 months ago

i have triggered an action building with this config https://github.com/lucee/lucee-dockerfiles/actions/runs/9363640823

michaeloffner commented 3 months ago

@justincarter i have added the config again for Lucee 5, even i think that is obsolete, but keep the status quo on this is fine. But for Lucee 6, it really makes no sense. It simply messes it up, by forcing an import and forcing Lucee 6 into multi mode.

justincarter commented 3 months ago

But for Lucee 6, it really makes no sense. It simply messes it up, by forcing an import and forcing Lucee 6 into multi mode.

Agreed, I'd rather not ship any XML config for Lucee 6 and default to single mode, so if letting Lucee generate a fresh JSON config is the best way to do that then I'm completely on board :)

Thinking towards the future with Tomcat, I think we might need separate Tomcat 9.x/10.x/11.x config directories. Previously we had no distinction between Tomcat 8.x/8.5.x/9.x and they seemed to work fine, but I'm not sure if that will be the case with the Tomcat 10.x package name changes? This repo still needs to support builds against multiple versions, so that might become necessary.