gnodet commented on code in PR #1640:
URL: https://github.com/apache/maven/pull/1640#discussion_r1713056325
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Artifact.java:
##########
@@ -23,76 +23,83 @@
import org.apache.maven.api.annotations.Nonnull;
/**
- * An artifact points to a resource such as a jar file or war application.
+ * Pointer to a resolved resource such as a <abbr>JAR</abbr> file or
<abbr>WAE</abbr> application.
Review Comment:
We can't use `resolved` here imho. An `Artifact` may not be resolved /
downloaded.
For example, the result of the `DependencyResolver#collect()` method will
return a graph of `Node`, each one containing a `Dependency` (which extends
`Artifact`), without having downloaded the related files.
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Dependency.java:
##########
@@ -23,30 +23,45 @@
import org.apache.maven.api.annotations.Nonnull;
/**
+ * Result of resolving a {@code DependencyCoordinate}.
+ * Resolving is the process that clarifies the obligation (optional or
mandatory status),
+ * selects a particular version and downloads the artifact in the local
repository.
*
* @since 4.0.0
*/
@Experimental
@Immutable
public interface Dependency extends Artifact {
-
/**
- * The dependency type.
+ * {@return the type of the dependency}. A dependency can be a
<abbr>JAR</abbr> file,
+ * a modular-<abbr>JAR</abbr> if it is intended to be placed on the
module-path,
+ * a <abbr>JAR</abbr> containing test classes, <i>etc.</i>
*
- * @return the dependency type, never {@code null}
+ * @see DependencyCoordinate#getType()
*/
@Nonnull
Type getType();
+ /**
+ * {@return the time at which the dependency will be used}.
+ * If may be, for example, at compile time only, at run time or at test
time.
+ *
+ * @see DependencyCoordinate#getScope()
+ */
@Nonnull
DependencyScope getScope();
+ /**
+ * Returns whether the dependency is optional or mandatory.
+ * Contrarily to {@link DependencyCoordinate}, the obligation of a
dependency cannot be unspecified.
Review Comment:
It's not specified at all, it's computed. The collection phase will compute
the scope and optionality of each dependency.
##########
api/maven-api-core/src/main/java/org/apache/maven/api/ArtifactCoordinate.java:
##########
@@ -23,64 +23,68 @@
import org.apache.maven.api.annotations.Nonnull;
/**
- * The {@code Coordinate} object is used to point to an {@link Artifact}
- * but the version may be specified as a range instead of an exact version.
+ * Point to an {@link Artifact} but with the version specified as a range
instead of an exact version.
+ * Each {@code ArtifactCoordinate} instance is basically a pointer to a file
in the Maven repository,
+ * except that the version may not be defined precisely.
Review Comment:
Not sure I like the wording, since the actual pointer is the `Artifact`.
`ArtifactCoordinate` is used to create those pointers by resolving the version.
I think relocations do not come into play for artifacts, but only for
dependencies, at least that's what the [resolver ArtifactResolver's doc seems
to
indicate](https://github.com/apache/maven-resolver/blob/97dfd1c2b9deb15734d5e401807e55cd0498332a/maven-resolver-impl/src/main/java/org/eclipse/aether/impl/ArtifactResolver.java#L41-L43)
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Artifact.java:
##########
@@ -23,76 +23,83 @@
import org.apache.maven.api.annotations.Nonnull;
/**
- * An artifact points to a resource such as a jar file or war application.
+ * Pointer to a resolved resource such as a <abbr>JAR</abbr> file or
<abbr>WAE</abbr> application.
+ * {@code Artifact} instances are created when <dfn>resolving</dfn> {@link
Artifact} instances.
+ * Resolving is the process that selects a {@linkplain #getVersion()
particular version}
+ * and downloads the artifact in the local repository.
+ * The download may be deferred to the first time that the file is needed.
*
* @since 4.0.0
*/
@Experimental
@Immutable
public interface Artifact {
-
/**
- * Returns a unique identifier for this artifact.
+ * {@return a unique identifier for this artifact}.
* The identifier is composed of groupId, artifactId, extension,
classifier, and version.
*
- * @return the unique identifier
+ * @see ArtifactCoordinate#getId()
*/
@Nonnull
default String key() {
+ String c = getClassifier();
return getGroupId()
+ ':'
+ getArtifactId()
+ ':'
+ getExtension()
- + (getClassifier().isEmpty() ? "" : ":" + getClassifier())
+ + (c.isEmpty() ? "" : ":" + c)
+ ':'
+ getVersion();
}
/**
- * The groupId of the artifact.
+ * {@return the group identifier of the artifact}.
*
- * @return the groupId
+ * @see ArtifactCoordinate#getGroupId()
*/
@Nonnull
String getGroupId();
/**
- * The artifactId of the artifact.
+ * {@return the identifier of the artifact}.
*
- * @return the artifactId
+ * @see ArtifactCoordinate#getArtifactId()
*/
@Nonnull
String getArtifactId();
/**
- * The version of the artifact.
+ * {@return the version of the artifact}. Contrarily to {@link
ArtifactCoordinate},
+ * each {@code Artifact} is associated to a specific version instead of a
range of versions.
Review Comment:
range or meta version (`LATEST`, `RELEASE`, `SNAPSHOT`)
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Artifact.java:
##########
@@ -23,76 +23,83 @@
import org.apache.maven.api.annotations.Nonnull;
/**
- * An artifact points to a resource such as a jar file or war application.
+ * Pointer to a resolved resource such as a <abbr>JAR</abbr> file or
<abbr>WAE</abbr> application.
+ * {@code Artifact} instances are created when <dfn>resolving</dfn> {@link
Artifact} instances.
+ * Resolving is the process that selects a {@linkplain #getVersion()
particular version}
+ * and downloads the artifact in the local repository.
+ * The download may be deferred to the first time that the file is needed.
*
* @since 4.0.0
*/
@Experimental
@Immutable
public interface Artifact {
-
/**
- * Returns a unique identifier for this artifact.
+ * {@return a unique identifier for this artifact}.
* The identifier is composed of groupId, artifactId, extension,
classifier, and version.
*
- * @return the unique identifier
+ * @see ArtifactCoordinate#getId()
*/
@Nonnull
default String key() {
+ String c = getClassifier();
return getGroupId()
+ ':'
+ getArtifactId()
+ ':'
+ getExtension()
- + (getClassifier().isEmpty() ? "" : ":" + getClassifier())
+ + (c.isEmpty() ? "" : ":" + c)
+ ':'
+ getVersion();
}
/**
- * The groupId of the artifact.
+ * {@return the group identifier of the artifact}.
*
- * @return the groupId
+ * @see ArtifactCoordinate#getGroupId()
*/
@Nonnull
String getGroupId();
/**
- * The artifactId of the artifact.
+ * {@return the identifier of the artifact}.
*
- * @return the artifactId
+ * @see ArtifactCoordinate#getArtifactId()
*/
@Nonnull
String getArtifactId();
/**
- * The version of the artifact.
+ * {@return the version of the artifact}. Contrarily to {@link
ArtifactCoordinate},
+ * each {@code Artifact} is associated to a specific version instead of a
range of versions.
*
- * @return the version
+ * @see ArtifactCoordinate#getVersionConstraint()
*/
@Nonnull
Version getVersion();
/**
- * The base version of the artifact.
- *
- * @return the version
+ * {@return the base version of the artifact}.
+ * TODO: this javadoc is not helpful.
Review Comment:
Agreed !
So the base version is the resolved range version or the meta version (i.e.
it can be `4.0.0-SNAPSHOT`) while the `getVersion()` should return
`4.0.0-20240809.104932-36`).
I'm actually not quite sure if the original `VersionConstraint` was a
range... @cstamas ?
##########
api/maven-api-core/src/main/java/org/apache/maven/api/ArtifactCoordinate.java:
##########
@@ -23,64 +23,68 @@
import org.apache.maven.api.annotations.Nonnull;
/**
- * The {@code Coordinate} object is used to point to an {@link Artifact}
- * but the version may be specified as a range instead of an exact version.
+ * Point to an {@link Artifact} but with the version specified as a range
instead of an exact version.
+ * Each {@code ArtifactCoordinate} instance is basically a pointer to a file
in the Maven repository,
+ * except that the version may not be defined precisely.
*
* @since 4.0.0
*/
@Experimental
@Immutable
public interface ArtifactCoordinate {
-
/**
- * The groupId of the artifact.
- *
- * @return the groupId
+ * {@return the group identifier of the artifact}.
*/
@Nonnull
String getGroupId();
/**
- * The artifactId of the artifact.
- *
- * @return the artifactId
+ * {@return the identifier of the artifact}.
*/
@Nonnull
String getArtifactId();
/**
- * The classifier of the artifact.
+ * Returns the classifier of the artifact.
*
* @return the classifier or an empty string if none, never {@code null}
*/
@Nonnull
String getClassifier();
/**
- * The version of the artifact.
- *
- * @return the version
+ * {@return the range of versions of the artifact}.
Review Comment:
Specific version, range, or meta version.
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Artifact.java:
##########
@@ -23,76 +23,83 @@
import org.apache.maven.api.annotations.Nonnull;
/**
- * An artifact points to a resource such as a jar file or war application.
+ * Pointer to a resolved resource such as a <abbr>JAR</abbr> file or
<abbr>WAE</abbr> application.
+ * {@code Artifact} instances are created when <dfn>resolving</dfn> {@link
Artifact} instances.
Review Comment:
We do resolve `ArtifactCoordinate`s
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Dependency.java:
##########
@@ -23,30 +23,45 @@
import org.apache.maven.api.annotations.Nonnull;
/**
+ * Result of resolving a {@code DependencyCoordinate}.
+ * Resolving is the process that clarifies the obligation (optional or
mandatory status),
+ * selects a particular version and downloads the artifact in the local
repository.
Review Comment:
It's a bit more complicated as explained earlier. `Dependency` is the
output of the `collection` process, which builds the graph of dependencies. It
then needs to be `flattened` and `resolved`. The version selection is done for
each dependency during the collection phase, the flatten phase will keep only a
single version per GA (groupId+artifactId), the resolution will actually
download the dependencies (or artifacts) that have been computed.
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Artifact.java:
##########
@@ -23,76 +23,83 @@
import org.apache.maven.api.annotations.Nonnull;
/**
- * An artifact points to a resource such as a jar file or war application.
+ * Pointer to a resolved resource such as a <abbr>JAR</abbr> file or
<abbr>WAE</abbr> application.
+ * {@code Artifact} instances are created when <dfn>resolving</dfn> {@link
Artifact} instances.
+ * Resolving is the process that selects a {@linkplain #getVersion()
particular version}
+ * and downloads the artifact in the local repository.
Review Comment:
So things are a bit blurry:
* artifact resolution is: resolve version (see
`VersionResolver#resolveVersion()`), and then resolve (i.e. download) the file
* dependency resolution is: collect dependencies, flatten the graph,
resolve (i.e. download) the file
During dependency collection, versions are resolved (@cstamas right?). The
flattening phase removes branches of the graph so that one artifact per GA is
present. The resolution phase will then download the result artifacts.
We don't have any way to download an artifact for which the version has been
resolved though (that may be a missing bit). So if we introduce
`ResolvedArtifact` and `ResolvedDependency`, it may help a bit:
* version resolution: ArtifactCoordinate -> Artifact
* artifact resolution: Artifact -> ResolvedArtifact
* dependency collection -> Node + Dependency
* dependency resolution: list of Dependency -> list of ResolvedDependency
##########
api/maven-api-core/src/main/java/org/apache/maven/api/DependencyScope.java:
##########
@@ -28,9 +28,9 @@
import org.apache.maven.api.annotations.Nonnull;
/**
- * Dependency scope.
- * This represents at which time the dependency will be used, for example, at
compile time only,
- * at run time or at test time. For a given dependency, the scope is directly
derived from the
+ * Represents at which time the dependency will be used.
+ * If may be, for example, at compile time only, at run time or at test time.
Review Comment:
`Indicates when the dependency will be used. For example, it may be at
compile time only, at runtime, or at test time.`
--
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-unsubscr...@maven.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
