You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "oehme (via GitHub)" <gi...@apache.org> on 2023/02/26 17:16:01 UTC
[GitHub] [maven-mvnd] oehme opened a new pull request, #797: Add property to disable model caching
oehme opened a new pull request, #797:
URL: https://github.com/apache/maven-mvnd/pull/797
This is mostly for the integration tests of the Gradle Enterprise Maven extension, which heavily test dependency resolution and reuse the same GAVs in many test, e.g. a:b:1.0. It would be a lot of effort to rewrite all those tests and a shame to restart the daemon every time. So I wanted to ask if you'd consider this flag to disable the caching altogether.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-mvnd] oehme commented on a diff in pull request #797: Add property to disable model caching
Posted by "oehme (via GitHub)" <gi...@apache.org>.
oehme commented on code in PR #797:
URL: https://github.com/apache/maven-mvnd/pull/797#discussion_r1123498117
##########
common/src/main/java/org/mvndaemon/mvnd/common/Environment.java:
##########
@@ -163,6 +163,12 @@ public enum Environment {
* non-native clients and is useful mostly for debugging.
*/
MVND_NO_DAEMON("mvnd.noDaemon", "MVND_NO_DAEMON", Boolean.FALSE, OptionType.BOOLEAN, Flags.DISCRIMINATING),
+
+ /**
+ * If <code>true</code>, the daemon will not use its in-memory metadata cache and instead re-read the
+ * metadata from the pom.xml files in the local repository. This is mostly useful for testing purposes.
+ */
+ MVND_NO_MODEL_CACHE("mvnd.noModelCache", null, Boolean.FALSE, OptionType.BOOLEAN, Flags.NONE),
Review Comment:
The `createCache` method is called every build, so it doesn't have to be discriminating. It doesn't hurt either, I don't mind :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-mvnd] oehme commented on a diff in pull request #797: Add property to disable model caching
Posted by "oehme (via GitHub)" <gi...@apache.org>.
oehme commented on code in PR #797:
URL: https://github.com/apache/maven-mvnd/pull/797#discussion_r1122150631
##########
daemon/src/main/java/org/apache/maven/project/SnapshotModelCacheFactory.java:
##########
@@ -45,6 +45,9 @@ public SnapshotModelCacheFactory(DefaultModelCacheFactory factory) {
@Override
public ModelCache createCache(RepositorySystemSession session) {
- return new SnapshotModelCache(globalCache, factory.createCache(session));
+ boolean enableModelCache = Boolean.parseBoolean(System.getProperty("mvnd.modelCache", "true"));
Review Comment:
Sure, I'll do that
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-mvnd] ppalaga merged pull request #797: Add property to disable model caching
Posted by "ppalaga (via GitHub)" <gi...@apache.org>.
ppalaga merged PR #797:
URL: https://github.com/apache/maven-mvnd/pull/797
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-mvnd] ppalaga commented on pull request #797: Add property to disable model caching
Posted by "ppalaga (via GitHub)" <gi...@apache.org>.
ppalaga commented on PR #797:
URL: https://github.com/apache/maven-mvnd/pull/797#issuecomment-1450418416
@gnodet didn't we have a property for disabling caching for specific artifacts?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-mvnd] guylabs commented on pull request #797: Add property to disable model caching
Posted by "guylabs (via GitHub)" <gi...@apache.org>.
guylabs commented on PR #797:
URL: https://github.com/apache/maven-mvnd/pull/797#issuecomment-1450242173
@ppalaga Would that be something that you can add for us?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-mvnd] ppalaga commented on a diff in pull request #797: Add property to disable model caching
Posted by "ppalaga (via GitHub)" <gi...@apache.org>.
ppalaga commented on code in PR #797:
URL: https://github.com/apache/maven-mvnd/pull/797#discussion_r1123526900
##########
common/src/main/java/org/mvndaemon/mvnd/common/Environment.java:
##########
@@ -163,6 +163,12 @@ public enum Environment {
* non-native clients and is useful mostly for debugging.
*/
MVND_NO_DAEMON("mvnd.noDaemon", "MVND_NO_DAEMON", Boolean.FALSE, OptionType.BOOLEAN, Flags.DISCRIMINATING),
+
+ /**
+ * If <code>true</code>, the daemon will not use its in-memory metadata cache and instead re-read the
+ * metadata from the pom.xml files in the local repository. This is mostly useful for testing purposes.
+ */
+ MVND_NO_MODEL_CACHE("mvnd.noModelCache", null, Boolean.FALSE, OptionType.BOOLEAN, Flags.NONE),
Review Comment:
Looking at the CI failures, we may need `Flags.OPTIONAL`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-mvnd] ppalaga commented on a diff in pull request #797: Add property to disable model caching
Posted by "ppalaga (via GitHub)" <gi...@apache.org>.
ppalaga commented on code in PR #797:
URL: https://github.com/apache/maven-mvnd/pull/797#discussion_r1122109132
##########
daemon/src/main/java/org/apache/maven/project/SnapshotModelCacheFactory.java:
##########
@@ -45,6 +45,9 @@ public SnapshotModelCacheFactory(DefaultModelCacheFactory factory) {
@Override
public ModelCache createCache(RepositorySystemSession session) {
- return new SnapshotModelCache(globalCache, factory.createCache(session));
+ boolean enableModelCache = Boolean.parseBoolean(System.getProperty("mvnd.modelCache", "true"));
Review Comment:
Could you please add the new property to `org.mvndaemon.mvnd.common.Environment` and add a bit of JavaDoc describing what it is doing?
Naming: Looking at other boolean properties, we have in `org.mvndaemon.mvnd.common.Environment`, perhaps the `mvnd.noModelCache` style would suit best here. I am not especially happy about negating names, but we already have a couple of those, and they are easier to use (given the default is with model cache enabled) from command line: `-Dmvnd.noModelCache` vs. `-Dmvnd.modelCache=false`. It is less typing. What do others think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-mvnd] ppalaga commented on a diff in pull request #797: Add property to disable model caching
Posted by "ppalaga (via GitHub)" <gi...@apache.org>.
ppalaga commented on code in PR #797:
URL: https://github.com/apache/maven-mvnd/pull/797#discussion_r1123521781
##########
common/src/main/java/org/mvndaemon/mvnd/common/Environment.java:
##########
@@ -163,6 +163,12 @@ public enum Environment {
* non-native clients and is useful mostly for debugging.
*/
MVND_NO_DAEMON("mvnd.noDaemon", "MVND_NO_DAEMON", Boolean.FALSE, OptionType.BOOLEAN, Flags.DISCRIMINATING),
+
+ /**
+ * If <code>true</code>, the daemon will not use its in-memory metadata cache and instead re-read the
+ * metadata from the pom.xml files in the local repository. This is mostly useful for testing purposes.
+ */
+ MVND_NO_MODEL_CACHE("mvnd.noModelCache", null, Boolean.FALSE, OptionType.BOOLEAN, Flags.NONE),
Review Comment:
If it is called every build, then no need to change anything. Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-mvnd] oehme commented on pull request #797: Add property to disable model caching
Posted by "oehme (via GitHub)" <gi...@apache.org>.
oehme commented on PR #797:
URL: https://github.com/apache/maven-mvnd/pull/797#issuecomment-1450502370
@ppalaga you have a property for[ reloading certain class realms](https://github.com/apache/maven-mvnd/blob/b97f585c0915c6ec5525f752b575c40e09ce297c/common/src/main/java/org/mvndaemon/mvnd/common/Environment.java#L288) (e.g. for plugins that hold static state). But nothing for the model cache.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-mvnd] ppalaga commented on pull request #797: Add property to disable model caching
Posted by "ppalaga (via GitHub)" <gi...@apache.org>.
ppalaga commented on PR #797:
URL: https://github.com/apache/maven-mvnd/pull/797#issuecomment-1450557202
> you have a property for reloading certain class realms (e.g. for plugins that hold static state). But nothing for the model cache.
Thanks for explaining @oehme! That's exactly, what my foggy recollection was about.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-mvnd] ppalaga commented on a diff in pull request #797: Add property to disable model caching
Posted by "ppalaga (via GitHub)" <gi...@apache.org>.
ppalaga commented on code in PR #797:
URL: https://github.com/apache/maven-mvnd/pull/797#discussion_r1123228141
##########
common/src/main/java/org/mvndaemon/mvnd/common/Environment.java:
##########
@@ -163,6 +163,12 @@ public enum Environment {
* non-native clients and is useful mostly for debugging.
*/
MVND_NO_DAEMON("mvnd.noDaemon", "MVND_NO_DAEMON", Boolean.FALSE, OptionType.BOOLEAN, Flags.DISCRIMINATING),
+
+ /**
+ * If <code>true</code>, the daemon will not use its in-memory metadata cache and instead re-read the
+ * metadata from the pom.xml files in the local repository. This is mostly useful for testing purposes.
+ */
+ MVND_NO_MODEL_CACHE("mvnd.noModelCache", null, Boolean.FALSE, OptionType.BOOLEAN, Flags.NONE),
Review Comment:
I wonder whether we should use `Flags.DISCRIMINATING`? Is the `createCache()` method called once for a given daemon instance or once per Maven build?
If it's once for daemon, I'd say it should be `Flags.DISCRIMINATING`. Otherwise it could happen that the user starts the daemon with `mvnd -Dmvnd.noModelCache` and all subsequent `mvnd` invocations with *or without* `-Dmvnd.noModelCache` will be served by the same daemon instance that has model cache off. That would not be correct, would it?
`Flags.DISCRIMINATING` will make all calls with `-Dmvnd.noModelCache` be served by a cacheless instance while all calls without `-Dmvnd.noModelCache` will be served by some other daemon instance that has the cache enabled.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org