[KARAF-5810] No config file installed in custom distribution. (On master)#553
[KARAF-5810] No config file installed in custom distribution. (On master)#553vanSabben wants to merge 3 commits intoapache:masterfrom
Conversation
Signed-off-by: van Sabben <4246302+vanSabben@users.noreply.github.com>
- Remove unused parameters. Signed-off-by: van Sabben <4246302+vanSabben@users.noreply.github.com>
vanSabben
changed the title
| } | ||
|
|
||
| new ConfigInstaller(etcDirectory, pidsToExtract) | ||
| new ConfigInstaller(homeDirectory, etcDirectory, pidsToExtract) |
There was a problem hiding this comment.
Why do we need the homeDirectory here ? Config should go in etc. And it seems to be the baseDirectory (not home).
There was a problem hiding this comment.
The documentation says it is relative.
If not specified, the location is relative from the KARAF_BASE variable. It’s also possible to use variable like ${karaf.home}, ${karaf.base}, ${karaf.etc}, or even system properties.
Each location can occur or it is relative to KARAF_BASE. KARAF_HOME is indeed not good. But is home not always equal to base in the AssemblyMojo? Here are no other instances possible? The builder name it home, but it is the custom distribution directory in the maven target directory. What do you think is best?
| public ConfigInstaller(Path etcDirectory, List<String> pidsToExtract) { | ||
|
|
||
| public ConfigInstaller(Path homeDirectory, Path etcDirectory, List<String> pidsToExtract) { | ||
| this.homeDirectory = homeDirectory; |
There was a problem hiding this comment.
Same, I don't think homeDirectory is required here.
There was a problem hiding this comment.
True, but baseDirectory is required.
| } | ||
| } | ||
|
|
||
| private void installConfig(Downloader downloader, ConfigFile pConfigFile) throws Exception { |
There was a problem hiding this comment.
The method should be named installConfigFile to avoid confusion.
| path = path.substring(1); | ||
| } | ||
|
|
||
| Path configFileTarget = homeDirectory.resolve(substFinalName(path)); |
There was a problem hiding this comment.
I don't think it's a good idea to do this here. We should just copy the config file in the etc directory "raw".
There was a problem hiding this comment.
What do you mean by 'raw'? Do not use variables and the like? See comment / documentation above. It can be absolute or relative and use variables.
| }); | ||
| } | ||
|
|
||
| private String substFinalName(String finalname) { |
There was a problem hiding this comment.
Useless (at least not in the target of this class).
There was a problem hiding this comment.
See comment / documentation above. It can be absolute or relative and use variables.
- Changes based on review comment. Signed-off-by: van Sabben <4246302+vanSabben@users.noreply.github.com>
|
See my comment in Jira. |
[ENTESB-18335] Teach karaf-maven-plugin how to make use of the CVE me…
2 participants
Now config files also installed for custom distributions.