[
https://issues.apache.org/jira/browse/MBUILDCACHE-32?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17682135#comment-17682135
]
ASF GitHub Bot commented on MBUILDCACHE-32:
-------------------------------------------
gnodet commented on code in PR #33:
URL:
https://github.com/apache/maven-build-cache-extension/pull/33#discussion_r1090695427
##########
src/main/java/org/apache/maven/buildcache/RemoteCacheRepositoryImpl.java:
##########
@@ -165,9 +168,28 @@ public Optional<byte[]> getResourceContent( String url )
throws IOException
transporter.get( task );
return Optional.of( task.getDataBytes() );
}
+ catch ( ResourceDoesNotExistException e )
+ {
+ if ( mavenSession.getRequest().isShowErrors() )
+ {
+ LOGGER.info( "Resource not found: {}", getFullUrl( url ), e );
+ }
+ else
+ {
+ LOGGER.info( "Resource not found: {}", getFullUrl( url ) );
+ }
+ return Optional.empty();
+ }
catch ( Exception e )
{
- LOGGER.info( "Cannot download {}", getFullUrl( url ), e );
+ if ( mavenSession.getRequest().isShowErrors() )
+ {
+ LOGGER.error( "Cannot download {}", getFullUrl( url ), e );
Review Comment:
Afaik, printing an error usually means the build should fail, or the user
should really look at the problem. If the cache isn't available, I don't think
this warrant an error, as the build should continue locally instead of
downloading the already built artifacts. I would tone it down to `warn` at
least.
##########
src/main/java/org/apache/maven/buildcache/RemoteCacheRepositoryImpl.java:
##########
@@ -165,9 +168,28 @@ public Optional<byte[]> getResourceContent( String url )
throws IOException
transporter.get( task );
return Optional.of( task.getDataBytes() );
}
+ catch ( ResourceDoesNotExistException e )
+ {
+ if ( mavenSession.getRequest().isShowErrors() )
Review Comment:
I think it would be better to use `LOGGER.isDebugEnabled()` rather than the
request parameter.
##########
src/main/java/org/apache/maven/buildcache/RemoteCacheRepositoryImpl.java:
##########
@@ -64,6 +65,7 @@ public class RemoteCacheRepositoryImpl implements
RemoteCacheRepository, Closeab
private final XmlService xmlService;
private final CacheConfig cacheConfig;
+ private final MavenSession mavenSession;
Review Comment:
This should not be needed anymore (see below).
##########
src/site/markdown/getting-started.md:
##########
@@ -17,7 +17,7 @@
## Getting Started
-To on-board incremental Maven you need to complete several steps:
+To on-board incremental Maven need to complete several steps:
Review Comment:
From a dev perspective, having atomic commits is better imho. The doc
should be split from the code change are those are completely unrelated.
##########
src/site/markdown/getting-started.md:
##########
@@ -27,48 +27,55 @@ To on-board incremental Maven you need to complete several
steps:
### Declaring build cache extension
```xml
+
<extension>
<groupId>org.apache.maven.extensions</groupId>
<artifactId>maven-build-cache-extension</artifactId>
- <version>1.0.0-SNAPSHOT</version>
+ <version>1.0.0</version>
</extension>
```
-either in `pom.xml`'s `<project>/<build>/<extensions>` or in
`.mvn/extensions.xml`'s `<extensions>`
+either in `pom.xml`'s `<project>/<build>/<extensions>` or in
`.mvn/extensions.xml`'s `<extensions>`. Using core
+extension model (`.mvn/extensions.xml` file) is preferable as it allows better
access to maven apis and could allow
+more sophisticated optimizations in the future.
Review Comment:
There are very few differences imho. Do you foresee any change on that part
?
##########
src/site/markdown/getting-started.md:
##########
@@ -27,48 +27,55 @@ To on-board incremental Maven you need to complete several
steps:
### Declaring build cache extension
```xml
+
<extension>
<groupId>org.apache.maven.extensions</groupId>
<artifactId>maven-build-cache-extension</artifactId>
- <version>1.0.0-SNAPSHOT</version>
+ <version>1.0.0</version>
</extension>
```
-either in `pom.xml`'s `<project>/<build>/<extensions>` or in
`.mvn/extensions.xml`'s `<extensions>`
+either in `pom.xml`'s `<project>/<build>/<extensions>` or in
`.mvn/extensions.xml`'s `<extensions>`. Using core
+extension model (`.mvn/extensions.xml` file) is preferable as it allows better
access to maven apis and could allow
+more sophisticated optimizations in the future.
### Adding build cache config
Copy template config
[`maven-build-cache-config.xml`](../resources/maven-build-cache-config.xml)
to [`.mvn/`](https://maven.apache.org/configure.html) directory of your
project.
-To get overall understanding of build cache machinery, it is recommended to
review the config and read comments. In typical
-scenario you need to:
+To get overall understanding of build cache machinery, it is recommended to
review the config and read comments. In a
Review Comment:
Missing articles. `To get a comprehensive understanding of the build cache
machinery`
##########
src/site/markdown/getting-started.md:
##########
@@ -27,48 +27,55 @@ To on-board incremental Maven you need to complete several
steps:
### Declaring build cache extension
```xml
+
<extension>
<groupId>org.apache.maven.extensions</groupId>
<artifactId>maven-build-cache-extension</artifactId>
- <version>1.0.0-SNAPSHOT</version>
+ <version>1.0.0</version>
</extension>
```
-either in `pom.xml`'s `<project>/<build>/<extensions>` or in
`.mvn/extensions.xml`'s `<extensions>`
+either in `pom.xml`'s `<project>/<build>/<extensions>` or in
`.mvn/extensions.xml`'s `<extensions>`. Using core
+extension model (`.mvn/extensions.xml` file) is preferable as it allows better
access to maven apis and could allow
+more sophisticated optimizations in the future.
### Adding build cache config
Copy template config
[`maven-build-cache-config.xml`](../resources/maven-build-cache-config.xml)
to [`.mvn/`](https://maven.apache.org/configure.html) directory of your
project.
-To get overall understanding of build cache machinery, it is recommended to
review the config and read comments. In typical
-scenario you need to:
+To get overall understanding of build cache machinery, it is recommended to
review the config and read comments. In a
+typical scenario need to:
* Exclude unstable, temporary files or environment specific files
-* Add plugins reconciliation rules – add critical plugins parameters to
reconciliation
-* Configure precise source code files selectors. Though source code locations
discovered automatically from project and plugins config,
- there might be edge cases.
-* Add remote cache location (if remote cache is used)
+* Add critical plugins parameters to runtime reconciliation
+* Configure precise source code files selectors. Though source code locations
discovered automatically from project and
Review Comment:
Missing verb. `Although source code locations are discovered automatically
from the project configuration and plugins, there may be edge cases.`
##########
src/site/markdown/getting-started.md:
##########
@@ -27,48 +27,55 @@ To on-board incremental Maven you need to complete several
steps:
### Declaring build cache extension
```xml
+
<extension>
<groupId>org.apache.maven.extensions</groupId>
<artifactId>maven-build-cache-extension</artifactId>
- <version>1.0.0-SNAPSHOT</version>
+ <version>1.0.0</version>
</extension>
```
-either in `pom.xml`'s `<project>/<build>/<extensions>` or in
`.mvn/extensions.xml`'s `<extensions>`
+either in `pom.xml`'s `<project>/<build>/<extensions>` or in
`.mvn/extensions.xml`'s `<extensions>`. Using core
+extension model (`.mvn/extensions.xml` file) is preferable as it allows better
access to maven apis and could allow
+more sophisticated optimizations in the future.
### Adding build cache config
Copy template config
[`maven-build-cache-config.xml`](../resources/maven-build-cache-config.xml)
to [`.mvn/`](https://maven.apache.org/configure.html) directory of your
project.
-To get overall understanding of build cache machinery, it is recommended to
review the config and read comments. In typical
-scenario you need to:
+To get overall understanding of build cache machinery, it is recommended to
review the config and read comments. In a
+typical scenario need to:
* Exclude unstable, temporary files or environment specific files
-* Add plugins reconciliation rules – add critical plugins parameters to
reconciliation
-* Configure precise source code files selectors. Though source code locations
discovered automatically from project and plugins config,
- there might be edge cases.
-* Add remote cache location (if remote cache is used)
+* Add critical plugins parameters to runtime reconciliation
+* Configure precise source code files selectors. Though source code locations
discovered automatically from project and
+ plugins config, there might be edge cases.
+* Configure remote cache (if remote cache is used)
### Adjusting build cache config
-Having extension run usual command, like `mvn package`. Verify the caching
engine is activated:
+Having extension configured run usual command, like `mvn package`. Verify the
caching engine is activated:
Review Comment:
Missing verb. Not even sure what is meant here with `Having extension
configured run usual command` ...
##########
src/site/markdown/remote-cache.md:
##########
@@ -29,12 +29,11 @@ Before you start, please keep in mind basic principles:
source code file, every dependency and effective pom (including plugin
parameters). Every element's hash contributes
to the key. In order to produce the same key there engine must consume
exactly the same hashes.
* There is no built-in normalization of line endings in this implementation,
file hash calculation is raw bytes
- based. The most obvious implication could be illustrated by a simple Git
checkout. By default, git will check out
- source code with CRLF line endings on win and LF on Linux. Because of that
builds over same commit on a Linux agent
- and local build on Windows workstation will yield different hashes.
+ based. Common pitfall is a Git checkout with CRLF on Win and LF on Linux.
Same commit with different line endings will
Review Comment:
`The same commit`
##########
src/site/markdown/getting-started.md:
##########
@@ -27,48 +27,55 @@ To on-board incremental Maven you need to complete several
steps:
### Declaring build cache extension
```xml
+
<extension>
<groupId>org.apache.maven.extensions</groupId>
<artifactId>maven-build-cache-extension</artifactId>
- <version>1.0.0-SNAPSHOT</version>
+ <version>1.0.0</version>
</extension>
```
-either in `pom.xml`'s `<project>/<build>/<extensions>` or in
`.mvn/extensions.xml`'s `<extensions>`
+either in `pom.xml`'s `<project>/<build>/<extensions>` or in
`.mvn/extensions.xml`'s `<extensions>`. Using core
+extension model (`.mvn/extensions.xml` file) is preferable as it allows better
access to maven apis and could allow
+more sophisticated optimizations in the future.
### Adding build cache config
Copy template config
[`maven-build-cache-config.xml`](../resources/maven-build-cache-config.xml)
to [`.mvn/`](https://maven.apache.org/configure.html) directory of your
project.
-To get overall understanding of build cache machinery, it is recommended to
review the config and read comments. In typical
-scenario you need to:
+To get overall understanding of build cache machinery, it is recommended to
review the config and read comments. In a
+typical scenario need to:
* Exclude unstable, temporary files or environment specific files
-* Add plugins reconciliation rules – add critical plugins parameters to
reconciliation
-* Configure precise source code files selectors. Though source code locations
discovered automatically from project and plugins config,
- there might be edge cases.
-* Add remote cache location (if remote cache is used)
+* Add critical plugins parameters to runtime reconciliation
+* Configure precise source code files selectors. Though source code locations
discovered automatically from project and
+ plugins config, there might be edge cases.
+* Configure remote cache (if remote cache is used)
### Adjusting build cache config
-Having extension run usual command, like `mvn package`. Verify the caching
engine is activated:
+Having extension configured run usual command, like `mvn package`. Verify the
caching engine is activated:
-* Check log output - there should be cache related output or initialization
error message:
+* Check log output - there should be the cache related output or
initialization error message:
```
[INFO] Loading cache configuration from <project
dir>/.mvn/maven-build-cache-config.xml
```
* Navigate to your local repo directory - there should be a sibling directory
`cache` next to the usual
local `repository`.
* Find `buildinfo.xml` in the cache repository for typical module and review
it. Ensure that
- * expected source code files are present in the build info
- * Review all plugings used in the build and add their critical parameters to
reconciliation
+ * Expected source code files are present in the build info
Review Comment:
lowercase is preferred I think, as it's the same sentence. Missing article ?
`the expected source code`
##########
src/site/markdown/remote-cache.md:
##########
@@ -88,13 +99,12 @@ Allow writes in remote cache add jvm property to designated
CI builds.
Run the build, review log and ensure that artifacts are uploaded to remote
cache. Now, rerun build and ensure that it
completes almost instantly because it is fully cached.
-### Remote cache relocation to local builds
+### Make remote cache portable
-As practice shows, developers often don't realize that builds they run in
local and CI environments are different. So
-straightforward attempt to reuse remote cache in local build usually results
in cache misses because of difference in
-plugins, parameters, profiles, environment, etc. In order to reuse results you
might need to change poms, cache config,
-CI jobs and the project itself. This part is usually most challenging and
time-consuming. Follow steps below to
-iteratively achieve working configuration.
+As practice shows, builds which run on local workstations and CI environments
are different. Straightforward attempt to
+reuse cache produced in different environment usually results in cache misses.
As is, caches are rarely portable because
+of differences in CI jobs, environment specifics and even project itself.
Making cache portable is usually the most
Review Comment:
`even the project`
`Making caches portable`
##########
src/site/markdown/getting-started.md:
##########
@@ -27,48 +27,55 @@ To on-board incremental Maven you need to complete several
steps:
### Declaring build cache extension
```xml
+
<extension>
<groupId>org.apache.maven.extensions</groupId>
<artifactId>maven-build-cache-extension</artifactId>
- <version>1.0.0-SNAPSHOT</version>
+ <version>1.0.0</version>
</extension>
```
-either in `pom.xml`'s `<project>/<build>/<extensions>` or in
`.mvn/extensions.xml`'s `<extensions>`
+either in `pom.xml`'s `<project>/<build>/<extensions>` or in
`.mvn/extensions.xml`'s `<extensions>`. Using core
+extension model (`.mvn/extensions.xml` file) is preferable as it allows better
access to maven apis and could allow
+more sophisticated optimizations in the future.
### Adding build cache config
Copy template config
[`maven-build-cache-config.xml`](../resources/maven-build-cache-config.xml)
to [`.mvn/`](https://maven.apache.org/configure.html) directory of your
project.
-To get overall understanding of build cache machinery, it is recommended to
review the config and read comments. In typical
-scenario you need to:
+To get overall understanding of build cache machinery, it is recommended to
review the config and read comments. In a
+typical scenario need to:
Review Comment:
Missing pronoun.
##########
src/site/markdown/remote-cache.md:
##########
@@ -112,7 +122,7 @@ Copy the link to a `build-cache-report.xml` and provide it
to your local build a
mvn verify -Dmaven.build.cache.failFast=true
-Dmaven.build.cache.baselineUrl=https://your-cache-url/<...>/915296a3-4596-4eb5-bf37-f6e13ebe087e/build-cache-report.xml
```
-Once discrepancy between remote and local builds detected cache will fail with
diagnostic info in
+Once discrepancy between remote and local builds detected cache will fail and
spool diagnostic info in
Review Comment:
`Once the discrepancy between remote and local versions is detected, the
cache will fail and diagnostic information will be stored in`
##########
src/main/java/org/apache/maven/buildcache/RemoteCacheRepositoryImpl.java:
##########
@@ -165,9 +168,28 @@ public Optional<byte[]> getResourceContent( String url )
throws IOException
transporter.get( task );
return Optional.of( task.getDataBytes() );
}
+ catch ( ResourceDoesNotExistException e )
+ {
+ if ( mavenSession.getRequest().isShowErrors() )
+ {
+ LOGGER.info( "Resource not found: {}", getFullUrl( url ), e );
+ }
+ else
+ {
+ LOGGER.info( "Resource not found: {}", getFullUrl( url ) );
+ }
+ return Optional.empty();
+ }
catch ( Exception e )
{
- LOGGER.info( "Cannot download {}", getFullUrl( url ), e );
+ if ( mavenSession.getRequest().isShowErrors() )
+ {
+ LOGGER.error( "Cannot download {}", getFullUrl( url ), e );
Review Comment:
Use `LOGGER.isDebugEnabled()` rather than
`mavenSession.getRequest().isShowErrors()`.
##########
src/site/markdown/getting-started.md:
##########
@@ -17,7 +17,7 @@
## Getting Started
-To on-board incremental Maven you need to complete several steps:
+To on-board incremental Maven need to complete several steps:
Review Comment:
There's no pronoun anymore...
##########
src/site/markdown/getting-started.md:
##########
@@ -27,48 +27,55 @@ To on-board incremental Maven you need to complete several
steps:
### Declaring build cache extension
```xml
+
<extension>
<groupId>org.apache.maven.extensions</groupId>
<artifactId>maven-build-cache-extension</artifactId>
- <version>1.0.0-SNAPSHOT</version>
+ <version>1.0.0</version>
</extension>
```
-either in `pom.xml`'s `<project>/<build>/<extensions>` or in
`.mvn/extensions.xml`'s `<extensions>`
+either in `pom.xml`'s `<project>/<build>/<extensions>` or in
`.mvn/extensions.xml`'s `<extensions>`. Using core
+extension model (`.mvn/extensions.xml` file) is preferable as it allows better
access to maven apis and could allow
+more sophisticated optimizations in the future.
### Adding build cache config
Copy template config
[`maven-build-cache-config.xml`](../resources/maven-build-cache-config.xml)
to [`.mvn/`](https://maven.apache.org/configure.html) directory of your
project.
-To get overall understanding of build cache machinery, it is recommended to
review the config and read comments. In typical
-scenario you need to:
+To get overall understanding of build cache machinery, it is recommended to
review the config and read comments. In a
+typical scenario need to:
* Exclude unstable, temporary files or environment specific files
-* Add plugins reconciliation rules – add critical plugins parameters to
reconciliation
-* Configure precise source code files selectors. Though source code locations
discovered automatically from project and plugins config,
- there might be edge cases.
-* Add remote cache location (if remote cache is used)
+* Add critical plugins parameters to runtime reconciliation
+* Configure precise source code files selectors. Though source code locations
discovered automatically from project and
+ plugins config, there might be edge cases.
+* Configure remote cache (if remote cache is used)
Review Comment:
May be simplify to `Configure remote cache if needed`
##########
src/site/markdown/getting-started.md:
##########
@@ -27,48 +27,55 @@ To on-board incremental Maven you need to complete several
steps:
### Declaring build cache extension
```xml
+
<extension>
<groupId>org.apache.maven.extensions</groupId>
<artifactId>maven-build-cache-extension</artifactId>
- <version>1.0.0-SNAPSHOT</version>
+ <version>1.0.0</version>
</extension>
```
-either in `pom.xml`'s `<project>/<build>/<extensions>` or in
`.mvn/extensions.xml`'s `<extensions>`
+either in `pom.xml`'s `<project>/<build>/<extensions>` or in
`.mvn/extensions.xml`'s `<extensions>`. Using core
+extension model (`.mvn/extensions.xml` file) is preferable as it allows better
access to maven apis and could allow
+more sophisticated optimizations in the future.
### Adding build cache config
Copy template config
[`maven-build-cache-config.xml`](../resources/maven-build-cache-config.xml)
to [`.mvn/`](https://maven.apache.org/configure.html) directory of your
project.
-To get overall understanding of build cache machinery, it is recommended to
review the config and read comments. In typical
-scenario you need to:
+To get overall understanding of build cache machinery, it is recommended to
review the config and read comments. In a
+typical scenario need to:
* Exclude unstable, temporary files or environment specific files
-* Add plugins reconciliation rules – add critical plugins parameters to
reconciliation
-* Configure precise source code files selectors. Though source code locations
discovered automatically from project and plugins config,
- there might be edge cases.
-* Add remote cache location (if remote cache is used)
+* Add critical plugins parameters to runtime reconciliation
+* Configure precise source code files selectors. Though source code locations
discovered automatically from project and
+ plugins config, there might be edge cases.
+* Configure remote cache (if remote cache is used)
### Adjusting build cache config
-Having extension run usual command, like `mvn package`. Verify the caching
engine is activated:
+Having extension configured run usual command, like `mvn package`. Verify the
caching engine is activated:
-* Check log output - there should be cache related output or initialization
error message:
+* Check log output - there should be the cache related output or
initialization error message:
Review Comment:
`a` rather than `the` ?
same for `or an initialization error message` ?
##########
src/site/markdown/remote-cache.md:
##########
@@ -57,19 +56,31 @@ supports http PUT/GET/HEAD operations will suffice (Nginx,
Apache or similar). A
change `remote@enabled` to true:
```xml
-<remote enabled="true">
+<!--Default @id is 'cache'-->
+<remote enabled="true" id="my-cache">
<url>http://your-buildcache-url</url>
</remote>
```
If proxy or authentication is required to access remote cache, add server
record to settings.xml as described
in [Servers](https://maven.apache.org/settings.html#Servers). The server
should be referenced from cache config:
-```
-TBD
+```xml
+
+<servers>
+ <server>
+ <!
> Do not print exception when probing builds in remote repo
> ---------------------------------------------------------
>
> Key: MBUILDCACHE-32
> URL: https://issues.apache.org/jira/browse/MBUILDCACHE-32
> Project: Maven Build Cache Extension
> Issue Type: Bug
> Reporter: Alexander Ashitkin
> Priority: Major
> Labels: pull-request-available
> Original Estimate: 24h
> Remaining Estimate: 24h
>
> When cache engine tries to discover existing cache by checksum, it sends get
> request.
> This request is normally getting 404s, because cache is not guaranteed to
> exist.
> It's a normal situation and exception should not be printed in such case as
> it meaninglessly pollutes logs:
> {code:java}
> org.apache.maven.wagon.ResourceDoesNotExistException: resource missing at
> https://my-cache/.../buildinfo.xml, status: 404 Not Found
> at
> org.apache.maven.wagon.shared.http.AbstractHttpClientWagon.fillInputData
> (AbstractHttpClientWagon.java:1191)
> at
> org.apache.maven.wagon.shared.http.AbstractHttpClientWagon.fillInputData
> (AbstractHttpClientWagon.java:1140)
> at org.apache.maven.wagon.StreamWagon.getInputStream
> (StreamWagon.java:126)
> at org.apache.maven.wagon.StreamWagon.getIfNewerToStream
> (StreamWagon.java:226)
> at org.apache.maven.wagon.StreamWagon.getToStream (StreamWagon.java:262)
> at org.eclipse.aether.transport.wagon.WagonTransporter$GetTaskRunner.run
> (WagonTransporter.java:533)
> at org.eclipse.aether.transport.wagon.WagonTransporter.execute
> (WagonTransporter.java:425)
> at org.eclipse.aether.transport.wagon.WagonTransporter.get
> (WagonTransporter.java:400)
> at
> org.apache.maven.buildcache.RemoteCacheRepositoryImpl.getResourceContent
> (RemoteCacheRepositoryImpl.java:165)
> at org.apache.maven.buildcache.RemoteCacheRepositoryImpl.findBuild
> (RemoteCacheRepositoryImpl.java:114)
> at org.apache.maven.buildcache.LocalCacheRepositoryImpl.findBuild
> (LocalCacheRepositoryImpl.java:183)
> at org.apache.maven.buildcache.CacheControllerImpl.findCachedBuild
> (CacheControllerImpl.java:212)
> at org.apache.maven.buildcache.CacheControllerImpl.findCachedBuild
> (CacheControllerImpl.java:179)
> at org.apache.maven.buildcache.BuildCacheMojosExecutionStrategy.execute
> (BuildCacheMojosExecutionStrategy.java:114)
> at org.apache.maven.lifecycle.internal.MojoExecutor.execute
> (MojoExecutor.java:179)
> {code}
> {{Need to create method similar to
> RemoteCacheRepositoryImpl#getResourceContent, but }}{{getResourceContentQuiet
> and use it when probing buildinfo.xml. the method should not log exceptions}}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
