-
Notifications
You must be signed in to change notification settings - Fork 3k
[MNG-6869] New flag to verify Maven installation status #995
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 40 commits
904caa4
2dab934
1082a0d
8d46d92
9aaf62f
57c7020
55f35fd
fdcce30
e4ec517
77526f5
fbbcc20
ba9354a
85b0907
12a6b4b
140a63c
7afb18e
ca4cb33
0764fd0
30c9649
d134358
6ba97e6
79338ae
b470aca
8629a8a
7fcc380
2b01442
95c52f2
d0f3b29
58e2f46
f67f7c3
27a7e0a
d2416a2
2a9ce5e
ed191ad
6adddbd
9443a2a
41177b3
fc2e7f7
2114cda
853819c
f39aec2
6c4348d
eaf2421
63006df
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,214 @@ | ||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one | ||
| * or more contributor license agreements. See the NOTICE file | ||
| * distributed with this work for additional information | ||
| * regarding copyright ownership. The ASF licenses this file | ||
| * to you under the Apache License, Version 2.0 (the | ||
| * "License"); you may not use this file except in compliance | ||
| * with the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, | ||
| * software distributed under the License is distributed on an | ||
| * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
| * KIND, either express or implied. See the License for the | ||
| * specific language governing permissions and limitations | ||
| * under the License. | ||
| */ | ||
| package org.apache.maven.cli; | ||
|
|
||
| import java.io.File; | ||
| import java.io.IOException; | ||
| import java.net.URI; | ||
| import java.nio.file.Files; | ||
| import java.nio.file.Path; | ||
| import java.nio.file.Paths; | ||
| import java.util.ArrayList; | ||
| import java.util.Collection; | ||
| import java.util.Collections; | ||
| import java.util.Comparator; | ||
| import java.util.List; | ||
| import java.util.stream.Collectors; | ||
| import java.util.stream.Stream; | ||
|
|
||
| import org.apache.maven.api.ArtifactCoordinate; | ||
| import org.apache.maven.api.Session; | ||
| import org.apache.maven.api.services.ArtifactResolver; | ||
| import org.apache.maven.api.services.ArtifactResolverException; | ||
| import org.apache.maven.api.services.ArtifactResolverResult; | ||
| import org.apache.maven.artifact.repository.ArtifactRepository; | ||
| import org.apache.maven.bridge.MavenRepositorySystem; | ||
| import org.apache.maven.execution.DefaultMavenExecutionResult; | ||
| import org.apache.maven.execution.MavenExecutionRequest; | ||
| import org.apache.maven.execution.MavenExecutionRequestPopulationException; | ||
| import org.apache.maven.execution.MavenExecutionRequestPopulator; | ||
| import org.apache.maven.execution.MavenSession; | ||
| import org.apache.maven.internal.aether.DefaultRepositorySystemSessionFactory; | ||
| import org.apache.maven.internal.impl.DefaultArtifactCoordinate; | ||
| import org.apache.maven.internal.impl.DefaultSession; | ||
| import org.apache.maven.internal.impl.DefaultSessionFactory; | ||
| import org.apache.maven.session.scope.internal.SessionScope; | ||
| import org.codehaus.plexus.PlexusContainer; | ||
| import org.codehaus.plexus.component.repository.exception.ComponentLookupException; | ||
| import org.eclipse.aether.RepositorySystemSession; | ||
| import org.eclipse.aether.artifact.Artifact; | ||
| import org.eclipse.aether.artifact.DefaultArtifact; | ||
| import org.eclipse.aether.resolution.ArtifactResolutionException; | ||
| import org.eclipse.aether.resolution.ArtifactResult; | ||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
|
|
||
| public class MavenStatusCommand { | ||
| private static final Logger LOGGER = LoggerFactory.getLogger(MavenStatusCommand.class); | ||
|
|
||
| /** | ||
| * In order to verify artifacts can be downloaded from the remote repositories we want to resolve an actual | ||
| * artifact. The Apache Maven artifact was chosen as it eventually, be it by proxy, mirror or directly, will be | ||
| * gathered from the central repository. The version is chosen arbitrarily since any listed should work. | ||
| */ | ||
| public static final Artifact APACHE_MAVEN_ARTIFACT = | ||
| new DefaultArtifact("org.apache.maven", "apache-maven", null, "pom", "3.8.6"); | ||
|
|
||
| private final MavenExecutionRequestPopulator mavenExecutionRequestPopulator; | ||
| private final ArtifactResolver artifactResolver; | ||
| private final RemoteRepositoryConnectionVerifier remoteRepositoryConnectionVerifier; | ||
| private final DefaultSessionFactory defaultSessionFactory; | ||
| private final DefaultRepositorySystemSessionFactory repoSession; | ||
| private final MavenRepositorySystem repositorySystem; | ||
| private final PlexusContainer container; | ||
| private final SessionScope sessionScope; | ||
| private Path tempLocalRepository; | ||
|
|
||
| public MavenStatusCommand(final PlexusContainer container) throws ComponentLookupException { | ||
| this.container = container; | ||
| this.remoteRepositoryConnectionVerifier = new RemoteRepositoryConnectionVerifier(container); | ||
| this.mavenExecutionRequestPopulator = container.lookup(MavenExecutionRequestPopulator.class); | ||
| this.artifactResolver = container.lookup(ArtifactResolver.class); | ||
| this.defaultSessionFactory = container.lookup(DefaultSessionFactory.class); | ||
| this.repoSession = container.lookup(DefaultRepositorySystemSessionFactory.class); | ||
| this.sessionScope = container.lookup(SessionScope.class); | ||
| this.repositorySystem = container.lookup(MavenRepositorySystem.class); | ||
| } | ||
|
|
||
| public List<String> verify(final MavenExecutionRequest cliRequest) throws MavenExecutionRequestPopulationException { | ||
| final MavenExecutionRequest mavenExecutionRequest = mavenExecutionRequestPopulator.populateDefaults(cliRequest); | ||
|
|
||
| final ArtifactRepository localRepository = cliRequest.getLocalRepository(); | ||
|
|
||
| final List<String> localRepositoryIssues = | ||
| verifyLocalRepository(Paths.get(URI.create(localRepository.getUrl()))); | ||
|
|
||
| // We overwrite the local repository with a temporary directory to avoid using a cached version of the artifact. | ||
| setTemporaryLocalRepositoryPathOnRequest(cliRequest); | ||
|
|
||
| final List<String> remoteRepositoryIssues = | ||
| verifyRemoteRepositoryConnections(cliRequest.getRemoteRepositories(), mavenExecutionRequest); | ||
| final List<String> artifactResolutionIssues = verifyArtifactResolution(mavenExecutionRequest); | ||
|
|
||
| cleanupTempFiles(); | ||
|
|
||
| // Collect all issues into a single list | ||
| return Stream.of(localRepositoryIssues, remoteRepositoryIssues, artifactResolutionIssues) | ||
| .flatMap(Collection::stream) | ||
| .collect(Collectors.toList()); | ||
| } | ||
|
|
||
| private void cleanupTempFiles() { | ||
| if (tempLocalRepository != null) { | ||
| try (Stream<Path> files = Files.walk(tempLocalRepository)) { | ||
| files.sorted(Comparator.reverseOrder()) // Sort in reverse order so that directories are deleted last | ||
| .map(Path::toFile) | ||
| .forEach(File::delete); | ||
| } catch (IOException ioe) { | ||
| LOGGER.debug("Failed to delete temporary local repository", ioe); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private void setTemporaryLocalRepositoryPathOnRequest(final MavenExecutionRequest request) { | ||
| try { | ||
| tempLocalRepository = Files.createTempDirectory("mvn-status").toAbsolutePath(); | ||
| request.setLocalRepositoryPath(tempLocalRepository.toString()); | ||
| request.setLocalRepository(repositorySystem.createLocalRepository(request, tempLocalRepository.toFile())); | ||
| } catch (Exception ex) { | ||
| LOGGER.debug("Could not create temporary local repository", ex); | ||
| LOGGER.warn("Artifact resolution test is less accurate as it may use earlier resolution results."); | ||
| } | ||
| } | ||
|
|
||
| private List<String> verifyRemoteRepositoryConnections( | ||
| final List<ArtifactRepository> remoteRepositories, final MavenExecutionRequest mavenExecutionRequest) { | ||
| final List<String> issues = new ArrayList<>(); | ||
|
|
||
| for (ArtifactRepository remoteRepository : remoteRepositories) { | ||
| final RepositorySystemSession repositorySession = repoSession.newRepositorySession(mavenExecutionRequest); | ||
| remoteRepositoryConnectionVerifier | ||
| .verifyConnectionToRemoteRepository(repositorySession, remoteRepository) | ||
| .ifPresent(issues::add); | ||
| } | ||
|
|
||
| return issues; | ||
| } | ||
|
|
||
| private List<String> verifyArtifactResolution(final MavenExecutionRequest mavenExecutionRequest) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is 1 step too much. Is this the only way to verify if a remote repository is accessible? If you need to use an explicit file instead of a directory, is it possible to use http HEAD ?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Just to verify: are you referring to the fact that this code actually downloads a file (e.g., it could've been an HTTP HEAD request), or to the fact that the In the case of the former: is a remote repository required to support an HTTP HEAD request? Could we rely on the fact that if HTTP HEAD is OK, HTTP GET will be OK, too?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It depends on what you define as "artifact resolution works". I'm not interested if the implementation is correct. I wish there's a way to only confirm that the connection works. It is about returning questions on StackOverflow that say Maven can't download things, but it is just that it cannot reach a repository. Possible rootcauses are missing proxies (or misconfigured) and misconfigured mirrors. Maybe @michael-o or @cstamas can think of a low-level reliable way to conform this.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From my experience this is not testing a random artifact which works but more enabling the aether logs which help, ultimately logging the request to be able to replay it with curl to do the check. Rest is random tests which can easily be proven false positive/negative due to proxies (correct) configuration.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The goal of this code is not to explain what is wrong and how the user should fix it. I agree with @rmannibucau that the Aether logs are more suitable for that. What this code wants to do is tell the user they have a problem, without even building a project. Ideally, in the case of the StackOverflow questions that @rfscholte refers to, we could say "please share the output of Even though this doesn't tell the user the solution, it points them in a few directions where they could look. It's a lot more compact than the complete Aether logs - which the user could still inspect, in case this message doesn't point them in the right direction already. |
||
| final Session session = this.defaultSessionFactory.getSession(new MavenSession( | ||
| container, | ||
| repoSession.newRepositorySession(mavenExecutionRequest), | ||
| mavenExecutionRequest, | ||
| new DefaultMavenExecutionResult())); | ||
|
|
||
| sessionScope.enter(); | ||
| try { | ||
| sessionScope.seed(DefaultSession.class, (DefaultSession) session); | ||
|
|
||
| ArtifactCoordinate artifactCoordinate = new DefaultArtifactCoordinate(session, APACHE_MAVEN_ARTIFACT); | ||
| ArtifactResolverResult resolverResult = | ||
| artifactResolver.resolve(session, Collections.singleton(artifactCoordinate)); | ||
|
|
||
| resolverResult | ||
| .getArtifacts() | ||
| .forEach((key, value) -> LOGGER.debug("Successfully resolved {} to {}", key, value)); | ||
|
|
||
| return Collections.emptyList(); | ||
| } catch (ArtifactResolverException are) { | ||
| return extractIssuesFromArtifactResolverException(are); | ||
| } finally { | ||
| sessionScope.exit(); | ||
| LOGGER.info("Artifact resolution check completed"); | ||
| } | ||
| } | ||
|
|
||
| private List<String> extractIssuesFromArtifactResolverException(final Exception exception) { | ||
| final boolean isArtifactResolutionException = exception.getCause() instanceof ArtifactResolutionException; | ||
| if (isArtifactResolutionException) { | ||
| final ArtifactResolutionException are = (ArtifactResolutionException) exception.getCause(); | ||
| return are.getResults().stream() | ||
| .map(ArtifactResult::getExceptions) | ||
| .flatMap(List::stream) | ||
| .map(Throwable::getMessage) | ||
| .collect(Collectors.toList()); | ||
| } else { | ||
| return Collections.singletonList(exception.getMessage()); | ||
| } | ||
| } | ||
|
|
||
| private List<String> verifyLocalRepository(final Path localRepositoryPath) { | ||
| final List<String> issues = new ArrayList<>(); | ||
|
|
||
| if (!Files.isDirectory(localRepositoryPath)) { | ||
| issues.add(String.format("Local repository path '%s' is not a directory.", localRepositoryPath)); | ||
| } | ||
|
|
||
| if (!Files.isReadable(localRepositoryPath)) { | ||
| issues.add(String.format("No read permissions on local repository '%s'.", localRepositoryPath)); | ||
| } | ||
|
|
||
| if (!Files.isWritable(localRepositoryPath)) { | ||
| issues.add(String.format("No write permissions on local repository '%s'.", localRepositoryPath)); | ||
| } | ||
|
|
||
|
mthmulders marked this conversation as resolved.
|
||
| LOGGER.info("Local repository setup check completed"); | ||
| return issues; | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this change be removed from this PR ? It's completely unrelated.
Also, if we do change that, other classes such as
DefaultArtifactand maybe others use the same mechanism and should be changed at the same time.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure! I've just pushed a commit to undo this part.