Fixed recordedBuildConfigFile absolute path resolution #46872
Conversation
quarkus-bot
bot
added
area/devtools
Status for workflow
|
| : Path.of(this.recordedBuildConfigFile); | ||
| if (previousBuildConfigDump == null) { | ||
| return recordedBuildConfigDirectory.toPath() | ||
| .resolve("quarkus-" + launchMode.getDefaultProfile() + "-config-dump"); |
There was a problem hiding this comment.
I was having a look at the original issue and is it expected we are not taking into account quarkus.config-tracking.file-suffix?
There was a problem hiding this comment.
We could. We'd need to refactor the current implementation further to do that.
It will come at a cost of always resolving the the build time config and bootstrap the app even on the first build (when the config dump from the previous build does not exist, since we will not know about that).
There was a problem hiding this comment.
Does it make sense to have a config property for it if we can't honor it?
To clarify, I wonder if we should just drop it if it can't be honored.
There was a problem hiding this comment.
No, it's used but by the process dumping the used config options during the build. This mojo is another client that reads that file. So all the options are actually used.
I think you had a good point though, we could potentially re-use the same option used by the build process in this mojo.
Perhaps we could keep the existing one but use it only if a user configured values explicitly.
There was a problem hiding this comment.
No, it's used but by the process dumping the used config options during the build.
Yeah, I had that in mind but does it make sense to make it configurable if it's hard to propagate this config everywhere?
What's the value of allowing people to adjust the suffix?
There was a problem hiding this comment.
Apparently, that's what @jprinet was experimenting with, generating config dumps for different profiles: local vs CI, afaiu.
There was a problem hiding this comment.
Exactly, when using the quarkus-build-caching-extension to make the Quarkus build goal cacheable, a recommendation is to check in the dump config in the Git repository.
You may have several versions of this dump config depending on several factors (os, arch...)
|
I'd say it fixes the issue taking the current design into account. We could look into config re-use as a follow up enhancement. |
4 participants
Fixes #46726