Skip to content
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

[MNG-6869] New flag to verify Maven installation status #995

Open
wants to merge 44 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
44 commits
Select commit Hold shift click to select a range
904caa4
[MNG-6869] Initial idea
Giovds Dec 7, 2022
2dab934
[MNG-6869] Further brainstorming
mthmulders Dec 7, 2022
1082a0d
[MNG-6869] Refactor local variable name
Giovds Dec 7, 2022
8d46d92
[MNG-6969] WIP commit
Giovds Dec 22, 2022
9aaf62f
[MNG-6869] Correct way to find local repo path
mthmulders Dec 22, 2022
57c7020
[MNG-6869] Use java.nio instead of java.io
Giovds Dec 22, 2022
55f35fd
[MNG-6869] Use java.nio instead of java.io
mthmulders Dec 22, 2022
fdcce30
[MNG-6869] Implement verification of artifact resolution (WIP)
mthmulders Dec 22, 2022
e4ec517
[MNG-6869] Test artifact resolution with temporary local repository
mthmulders Jan 12, 2023
77526f5
[MNG-6869] Setup checks for remote connections
Giovds Jan 12, 2023
fbbcc20
[MNG-6869] Formatting
mthmulders Jan 12, 2023
ba9354a
[MNG-6869] Refactor: rename method
mthmulders Jan 12, 2023
85b0907
[MNG-6869] Formatting
mthmulders Jan 16, 2023
12a6b4b
[MNG-6869] Verify connection to remote repositories
mthmulders Jan 26, 2023
140a63c
[MNG-6869] Reformat with Spotless
mthmulders Jan 26, 2023
7afb18e
[MNG-6869] Add todo to fix local repository issue
Giovds Jan 26, 2023
ca4cb33
[MNG-6869] Narrow declaration of thrown exception
mthmulders Jan 26, 2023
0764fd0
[MNG-6869] Refine detection of possible connection issues
mthmulders Jan 26, 2023
30c9649
[MNG-6869] Use local repository correctly
Giovds Feb 9, 2023
d134358
[MNG-6869] Clean up temp file after usage
Giovds Feb 9, 2023
6ba97e6
[MNG-6869] Include local repository path in issue text
mthmulders Feb 9, 2023
79338ae
[MNG-6869] Mention local repo check has been completed
mthmulders Feb 9, 2023
b470aca
[MNG-6869] Output detected issues prefixed with a number
mthmulders Feb 9, 2023
8629a8a
[MNG-6869] Properly close streeam
mthmulders Feb 9, 2023
7fcc380
[MNG-6869] Make logger a static constant
mthmulders Feb 9, 2023
2b01442
[MNG-6869] Make it explicit we're populating instance variables
mthmulders Feb 9, 2023
95c52f2
[MNG-6869] Reorder instance variables
mthmulders Feb 9, 2023
d0f3b29
[MNG-6869] Mark method arguments as final
mthmulders Feb 9, 2023
58e2f46
[MNG-6869] Store temporary local repo as Path
mthmulders Feb 9, 2023
f67f7c3
[MNG-6869] Rename logger to LOGGER
mthmulders Feb 9, 2023
27a7e0a
[MNG-6869] Checkstyle: redundant modifier
mthmulders Feb 9, 2023
d2416a2
[MNG-6869] Minor review points
mthmulders Feb 9, 2023
2a9ce5e
[MNG-6869] Remove commented code
Giovds Feb 23, 2023
ed191ad
[MNG-6869] Use exception classification of Transporter
Giovds Feb 23, 2023
6adddbd
[MNG-6869] Make formatException signature non-optional
Giovds Feb 23, 2023
9443a2a
[MNG-6869] Remove unnecessary comment
Giovds Feb 23, 2023
41177b3
[MNG-6869] Remove unused DI fields
Giovds Feb 23, 2023
fc2e7f7
[MNG-6869] Make no assumptions about the existence of Maven Central
mthmulders Feb 23, 2023
2114cda
[MNG-6869] Simplify
mthmulders Feb 23, 2023
853819c
[MNG-6869] Prevent possible ClassCastException
mthmulders Feb 23, 2023
f39aec2
[MNG-6869] Revert unrelated change
mthmulders May 10, 2023
6c4348d
Merge branch 'master' into mng-6869-new-flag-to-verify-maven-status
Giovds Apr 26, 2024
eaf2421
[MNG-6869] Make it work with recent changes
Giovds Apr 26, 2024
63006df
[MNG-6869] Make use of try with resources
Giovds Apr 26, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Merge branch 'master' into mng-6869-new-flag-to-verify-maven-status
  • Loading branch information
Giovds committed Apr 26, 2024
commit 6c4348d40130c6f04b30778bcb78266a9287acef
27 changes: 27 additions & 0 deletions maven-embedder/src/main/java/org/apache/maven/cli/CLIManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 117,12 @@ public class CLIManager {

public static final String COLOR = "color";

public static final String CACHE_ARTIFACT_NOT_FOUND = "canf";

public static final String STRICT_ARTIFACT_DESCRIPTOR_POLICY = "sadp";

public static final String IGNORE_TRANSITIVE_REPOSITORIES = "itr";

public static final String INSTALLATION_STATUS = "status";

/** This option is deprecated and may be repurposed as Java debug in a future version.
Expand Down Expand Up @@ -310,6 316,27 @@ public CLIManager() {
.optionalArg(true)
.desc("Defines the color mode of the output. Supported are 'auto', 'always', 'never'.")
.build());
options.addOption(Option.builder(CACHE_ARTIFACT_NOT_FOUND)
.longOpt("cache-artifact-not-found")
.hasArg()
.desc(
"Defines caching behaviour for 'not found' artifacts. Supported values are 'true' (default), 'false'.")
.build());
options.addOption(Option.builder(STRICT_ARTIFACT_DESCRIPTOR_POLICY)
.longOpt("strict-artifact-descriptor-policy")
.hasArg()
.desc("Defines 'strict' artifact descriptor policy. Supported values are 'true', 'false' (default).")
.build());
options.addOption(Option.builder(IGNORE_TRANSITIVE_REPOSITORIES)
.longOpt("ignore-transitive-repositories")
.desc("If set, Maven will ignore remote repositories introduced by transitive dependencies.")
.build());

// Adding this back to make Maven fail if used
options.addOption(Option.builder("llr")
.longOpt("legacy-local-repository")
.desc("UNSUPPORTED: Use of this option will make Maven invocation fail.")
.build());
options.addOption(Option.builder()
.longOpt(INSTALLATION_STATUS)
.desc("Display Maven installation status")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 178,8 @@ public class MavenCli {

private CLIManager cliManager;

private MessageBuilderFactory messageBuilderFactory;

private PlexusContainer plexusContainer;

private static final Pattern NEXT_LINE = Pattern.compile("\r?\n");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 152,7 @@ private List<String> verifyRemoteRepositoryConnections(
}

private List<String> verifyArtifactResolution(final MavenExecutionRequest mavenExecutionRequest) {
Copy link
Contributor

Choose a reason for hiding this comment

The 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 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[...] 1 step too much [...]

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 --status verifies if artifact resolution works?

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?

Copy link
Contributor

Choose a reason for hiding this comment

The 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.

Copy link
Contributor

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 mvn --status". Their output might look like this for example:

[INFO] Local repository setup check completed
[INFO] Connection check for repository 'corporate-repo' at 'https://pkgs.dev.azure.com/big-corporate/project/_packaging/whatever-repository/maven/v1' completed
Downloading from corporate-repo: https://pkgs.dev.azure.com/big-corporate/project/_packaging/whatever-repository/maven/v1/org/apache/maven/apache-maven/3.8.6/apache-maven-3.8.6.pom
Downloading from misconfigured-mirror: https://non-existing.example.com/maven2/org/apache/maven/apache-maven/3.8.6/apache-maven-3.8.6.pom
[INFO] Artifact resolution check completed
[INFO]
[ERROR] The following issues where found
[ERROR] 1.  Connection to misconfigured-mirror [https://non-existing.example.com/maven2/] not possible. Cause: non-existing.example.com: nodename nor servname provided, or not known
[ERROR] 2.  Could not find artifact org.apache.maven:apache-maven:pom:3.8.6 in corporate-repo (https://pkgs.dev.azure.com/big-corporate/project/_packaging/whatever-repository/maven/v1)
[ERROR] 3.  Could not transfer artifact org.apache.maven:apache-maven:pom:3.8.6 from/to misconfigured-mirror (https://non-existing.example.com/maven2/): non-existing.example.com: nodename nor servname provided, or not known

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(
final Session session = this.defaultSessionFactory.newSession(new MavenSession(
container,
repoSession.newRepositorySession(mavenExecutionRequest),
mavenExecutionRequest,
Expand Down
You are viewing a condensed version of this merge commit. You can view the full changes here.