You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "cstamas (via GitHub)" <gi...@apache.org> on 2024/01/24 11:34:07 UTC
[PR] [MNG-8026] Maven drives regarding scopes [maven]
cstamas opened a new pull request, #1391:
URL: https://github.com/apache/maven/pull/1391
This is step one: basically moved (copied, as resolver bits are deprecated) all the classes dealing with scopes (and their interpretation) to Maven.
---
https://issues.apache.org/jira/browse/MNG-8026
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "rmannibucau (via GitHub)" <gi...@apache.org>.
rmannibucau commented on PR #1391:
URL: https://github.com/apache/maven/pull/1391#issuecomment-1909427969
@cstamas does it mean we should fix the naming for mvn 4 and align it on what it is? Resolverpaths or alike since it is unrelated to any actual build and becomes even more ambiguous with jpms?
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #1391:
URL: https://github.com/apache/maven/pull/1391#discussion_r1465541701
##########
maven-core/src/main/java/org/apache/maven/internal/impl/DefaultArtifact.java:
##########
@@ -95,7 +97,12 @@ public String getClassifier() {
@Override
public boolean isSnapshot() {
- return DefaultModelVersionParser.checkSnapshot(artifact.getVersion());
+ return artifact.isSnapshot();
Review Comment:
Agreed.
My point is to have one "source of truth". So if we change this once, we change that one spot. Currently, resolver Artifact.isSnapshot should be it IMHO. But agree, let's move it out from this PR, does not belong here.
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #1391:
URL: https://github.com/apache/maven/pull/1391#issuecomment-1920108675
@cstamas I've removed `BuildPath` which will be provided by #1378 (inside `DependencyResolverResult`) and renamed `BuildPathScope` to `PathScope` to align with the other PR which defines `PathType`.
Let me know what you think about the current state.
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #1391:
URL: https://github.com/apache/maven/pull/1391#issuecomment-1908946500
It was always "build path" but Maven internally called it wrongly "classpath" :wink:
Basically what we do here is laying down new API blocks for dep scopes and build path scopes.
The other PR is about "filling in the properties".
And ultimately, you as for "main-compile" or "test-compile" build path, Maven calls into Resolver and it resolves those for you, and you end up with a bunch of artifacts (that are coming via project, so are dependency, hence, had type).
Then, those collections are "split", filtered by various properties present in their artifact.properties that makes them "fall" into proper bucket, like classpath, modulepath, etc
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "desruisseaux (via GitHub)" <gi...@apache.org>.
desruisseaux commented on code in PR #1391:
URL: https://github.com/apache/maven/pull/1391#discussion_r1477063891
##########
api/maven-api-core/src/main/java/org/apache/maven/api/DependencyScope.java:
##########
@@ -0,0 +1,125 @@
+/*
+ * 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.api;
+
+import java.util.Collections;
+import java.util.Map;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.annotations.Immutable;
+import org.apache.maven.api.annotations.Nonnull;
+
+/**
+ * Dependency scope.
+ * <p>
+ * Implementation must have {@code equals()} and {@code hashCode()} implemented, so implementations of this interface
+ * can be used as keys.
+ *
+ * @since 4.0.0
+ */
+@Experimental
+@Immutable
+public enum DependencyScope {
+
+ /**
+ * None. Allows you to declare dependencies (for example to alter reactor build order) but in reality dependencies
+ * in this scope are not part of any build path scope.
+ */
+ NONE("none", false),
+
+ /**
+ * Empty scope.
+ */
+ EMPTY("", false),
+
+ /**
+ * Compile only.
+ */
+ COMPILE_ONLY("compile-only", false),
+
+ /**
+ * Compile.
Review Comment:
I think it would be worth to edit the javadoc accordingly (same for `TEST`).
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #1391:
URL: https://github.com/apache/maven/pull/1391#discussion_r1466167884
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Artifact.java:
##########
@@ -105,6 +105,14 @@ default String key() {
*/
boolean isSnapshot();
+ /**
+ * The artifact properties.
+ *
+ * @return the artifact properties, never {@code null}
+ */
+ @Nonnull
+ ArtifactProperties getArtifactProperties();
Review Comment:
> Or in other words: see this PR [apache/maven-resolver#411](https://github.com/apache/maven-resolver/pull/411) Basically Resolver can be now asked to:
>
> * create Artifact as `new Artifact("g:a:v", artifactTypeRegistry.get("annotation-processor"))`.
> * the to resolve it to have it's full transitive hull
> * and you again end up with "bunch of Artifacts"
>
> No dependency in scope, but you are STILL interested in Artifact properties....
I don't think that's right. You're looking at the resolver API, not the Maven API here.
> Or similarly for ANY artifact, as if artifact you are resolving may have type="modular-jar", then the needed info will be still present in artifact.properties. Again, no dependencies involved, etc.
There's no `Type` on the `Artifact` ...
> Am pretty much confident that this change is correct.
I think it maps to the resolver api and model, but definitely not to the Maven 4 API, which as `Artifact`, `ArtifactCoordinate`, `Dependency` and `DependencyCoordinate`, whereas resolver as `Artifact` only...
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #1391:
URL: https://github.com/apache/maven/pull/1391#discussion_r1466221747
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Artifact.java:
##########
@@ -105,6 +105,14 @@ default String key() {
*/
boolean isSnapshot();
+ /**
+ * The artifact properties.
+ *
+ * @return the artifact properties, never {@code null}
+ */
+ @Nonnull
+ ArtifactProperties getArtifactProperties();
Review Comment:
> ...we already have some properties (is added to classpath etc) no?
@rmannibucau
That method is one of the main sources of confusion :smile: This is what happens:
* Maven2 had `ArtifactHandler#isAddedToClasspath()` which meant exactly that: "is artifact to be added to classpath" as Maven2 was aware only of classpath, nothing else.
* in Maven3 resolver came, and introduced this flag: https://github.com/apache/maven-resolver/blob/master/maven-resolver-api/src/main/java/org/eclipse/aether/artifact/ArtifactProperties.java#L54-L62 (note the javadoc nor any string literal **does not mention classpath or anything**).
* In development of Maven3 early, at one point these two were "shortcut into one": https://github.com/apache/maven/blob/master/maven-core/src/main/java/org/apache/maven/RepositoryUtils.java#L247-L268
* so essentially the "classpath" notion in core coming from Maven2 and "buildpath" abstraction, intentionally defined in Resolver as such (new in Maven3), was blurred to mean "same thing", which was obviously not the intent.
Typical example: the POM packaging, see definition here:
https://github.com/apache/maven/blob/master/maven-core/src/main/java/org/apache/maven/internal/impl/types/PomTypeProvider.java
Long time ago, before BOMs were a thing, people used it to "group dependencies" (see for example this blog: https://blog.sonatype.com/2009/10/maven-tips-and-tricks-grouping-dependencies/).
In essence, you create a POM that has your grouped dependencies and deploy that. So you have:
```
G:A:pom:V
+- D1
+- D2
+- D3
```
On consuming side, when you declare in your project dependency (yes, a dependency, not `import` as today you do with BOMs) like `G:A:pom:V`, hence as:
```
<dependency>
<groupId>G</groupId>
<artifactId>A</artifactId>
<version>V</version>
<type>pom</type>
</dependency>
```
On resolving (for @gnodet collect+resolve) this project "compile" build path scope, assuming is the only dependency you have, you would end up with this list of artifacts: [D1, D2, D3] despite resolution started as `P` (root) where P had 1 dependency `G:A:pom:V`.
Simply put, the Type definition (lack of "constitutes build path") causes that intermediate "grouping POM" be completely omitted (left out) from the results, as it is not part of the build path.
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #1391:
URL: https://github.com/apache/maven/pull/1391#discussion_r1464865561
##########
maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/scopes/MavenDependencyScopes.java:
##########
@@ -0,0 +1,53 @@
+/*
+ * 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.repository.internal.scopes;
+
+/**
+ * The dependency scopes used for Java dependencies in Maven. This class defines labels only, that are doing pass-thru
+ * over Resolver.
+ *
+ * @since 4.0.0
+ */
+public final class MavenDependencyScopes {
Review Comment:
Yes, when the followup PR ultimate goal is reached, this class MavenDependencyScopes should be gone
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #1391:
URL: https://github.com/apache/maven/pull/1391#discussion_r1477061460
##########
api/maven-api-core/src/main/java/org/apache/maven/api/DependencyScope.java:
##########
@@ -0,0 +1,125 @@
+/*
+ * 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.api;
+
+import java.util.Collections;
+import java.util.Map;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.annotations.Immutable;
+import org.apache.maven.api.annotations.Nonnull;
+
+/**
+ * Dependency scope.
+ * <p>
+ * Implementation must have {@code equals()} and {@code hashCode()} implemented, so implementations of this interface
+ * can be used as keys.
+ *
+ * @since 4.0.0
+ */
+@Experimental
+@Immutable
+public enum DependencyScope {
+
+ /**
+ * None. Allows you to declare dependencies (for example to alter reactor build order) but in reality dependencies
+ * in this scope are not part of any build path scope.
+ */
+ NONE("none", false),
+
+ /**
+ * Empty scope.
+ */
+ EMPTY("", false),
+
+ /**
+ * Compile only.
+ */
+ COMPILE_ONLY("compile-only", false),
+
+ /**
+ * Compile.
Review Comment:
Yes
##########
api/maven-api-core/src/main/java/org/apache/maven/api/DependencyScope.java:
##########
@@ -0,0 +1,125 @@
+/*
+ * 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.api;
+
+import java.util.Collections;
+import java.util.Map;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.annotations.Immutable;
+import org.apache.maven.api.annotations.Nonnull;
+
+/**
+ * Dependency scope.
+ * <p>
+ * Implementation must have {@code equals()} and {@code hashCode()} implemented, so implementations of this interface
+ * can be used as keys.
+ *
+ * @since 4.0.0
+ */
+@Experimental
+@Immutable
+public enum DependencyScope {
+
+ /**
+ * None. Allows you to declare dependencies (for example to alter reactor build order) but in reality dependencies
+ * in this scope are not part of any build path scope.
+ */
+ NONE("none", false),
+
+ /**
+ * Empty scope.
+ */
+ EMPTY("", false),
+
+ /**
+ * Compile only.
+ */
+ COMPILE_ONLY("compile-only", false),
+
+ /**
+ * Compile.
+ */
+ COMPILE("compile", true),
+
+ /**
+ * Runtime.
+ */
+ RUNTIME("runtime", true),
+
+ /**
+ * Provided.
+ */
+ PROVIDED("provided", false),
+
+ /**
+ * Test compile only.
+ */
+ TEST_ONLY("test-only", false),
+
+ /**
+ * Test.
Review Comment:
Yes
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "desruisseaux (via GitHub)" <gi...@apache.org>.
desruisseaux commented on code in PR #1391:
URL: https://github.com/apache/maven/pull/1391#discussion_r1477061283
##########
api/maven-api-core/src/main/java/org/apache/maven/api/DependencyScope.java:
##########
@@ -0,0 +1,125 @@
+/*
+ * 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.api;
+
+import java.util.Collections;
+import java.util.Map;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.annotations.Immutable;
+import org.apache.maven.api.annotations.Nonnull;
+
+/**
+ * Dependency scope.
+ * <p>
+ * Implementation must have {@code equals()} and {@code hashCode()} implemented, so implementations of this interface
+ * can be used as keys.
+ *
+ * @since 4.0.0
+ */
+@Experimental
+@Immutable
+public enum DependencyScope {
+
+ /**
+ * None. Allows you to declare dependencies (for example to alter reactor build order) but in reality dependencies
+ * in this scope are not part of any build path scope.
+ */
+ NONE("none", false),
+
+ /**
+ * Empty scope.
+ */
+ EMPTY("", false),
+
+ /**
+ * Compile only.
+ */
+ COMPILE_ONLY("compile-only", false),
+
+ /**
+ * Compile.
+ */
+ COMPILE("compile", true),
+
+ /**
+ * Runtime.
+ */
+ RUNTIME("runtime", true),
+
+ /**
+ * Provided.
+ */
+ PROVIDED("provided", false),
+
+ /**
+ * Test compile only.
+ */
+ TEST_ONLY("test-only", false),
+
+ /**
+ * Test.
Review Comment:
I presume that we mean "test compilation and execution"?
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #1391:
URL: https://github.com/apache/maven/pull/1391#issuecomment-1926632787
Looks good! Sorry to be late to party, but we can improve all this "on the go" anyway...
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "rmannibucau (via GitHub)" <gi...@apache.org>.
rmannibucau commented on code in PR #1391:
URL: https://github.com/apache/maven/pull/1391#discussion_r1466206736
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Artifact.java:
##########
@@ -105,6 +105,14 @@ default String key() {
*/
boolean isSnapshot();
+ /**
+ * The artifact properties.
+ *
+ * @return the artifact properties, never {@code null}
+ */
+ @Nonnull
+ ArtifactProperties getArtifactProperties();
Review Comment:
We need open properties (my 2cts would be it is `Map<String, Object>`, no need to type it stronger) to enable a (custom) type - I include the "module-jar" kind of types, ie all but extensions types - to complete the meta of a dependency - the old artifact handler somehow which is propagated to properties.
Having or not a wrapper is a style choice but we already have some properties (is added to classpath etc) no?
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "desruisseaux (via GitHub)" <gi...@apache.org>.
desruisseaux commented on code in PR #1391:
URL: https://github.com/apache/maven/pull/1391#discussion_r1477061174
##########
api/maven-api-core/src/main/java/org/apache/maven/api/DependencyScope.java:
##########
@@ -0,0 +1,125 @@
+/*
+ * 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.api;
+
+import java.util.Collections;
+import java.util.Map;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.annotations.Immutable;
+import org.apache.maven.api.annotations.Nonnull;
+
+/**
+ * Dependency scope.
+ * <p>
+ * Implementation must have {@code equals()} and {@code hashCode()} implemented, so implementations of this interface
+ * can be used as keys.
+ *
+ * @since 4.0.0
+ */
+@Experimental
+@Immutable
+public enum DependencyScope {
+
+ /**
+ * None. Allows you to declare dependencies (for example to alter reactor build order) but in reality dependencies
+ * in this scope are not part of any build path scope.
+ */
+ NONE("none", false),
+
+ /**
+ * Empty scope.
+ */
+ EMPTY("", false),
+
+ /**
+ * Compile only.
+ */
+ COMPILE_ONLY("compile-only", false),
+
+ /**
+ * Compile.
Review Comment:
I presume that we mean "compile, test and runtime"? (for clarifying the difference with `COMPILE_ONLY`).
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #1391:
URL: https://github.com/apache/maven/pull/1391#discussion_r1464864632
##########
maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/MavenRepositorySystemUtils.java:
##########
@@ -49,7 +49,9 @@
* assist those clients that employ the repository system outside of an IoC container, Maven plugins should instead
* always use regular dependency injection to acquire the repository system.
*
+ * @deprecated See {@link MavenSessionBuilderSupplier}
*/
+@Deprecated
public final class MavenRepositorySystemUtils {
Review Comment:
These are all static methods, IMHO nope.
This class would be used only by some third party integration, but given current Maven4 is still alpha, I am more for dropping this class and use MavenSessionBuilderSupplier instead, in those apps that integrate Maven4. For those apps, the "price" for moving from Resolver1/Maven3 to Resolver2/Maven4 would be this change. Or they should just use MIMA :smile:
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "rmannibucau (via GitHub)" <gi...@apache.org>.
rmannibucau commented on PR #1391:
URL: https://github.com/apache/maven/pull/1391#issuecomment-1909909961
I'm more than fine doing it by "blocks" - in particular the last part Tamas explained - but how can we ensure we don't get this PR without next one in terms of release to avoid to expose - even in an alpha - something we know we'll not keep like that and we'll break?
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #1391:
URL: https://github.com/apache/maven/pull/1391#discussion_r1465540157
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Artifact.java:
##########
@@ -105,6 +105,14 @@ default String key() {
*/
boolean isSnapshot();
+ /**
+ * The artifact properties.
+ *
+ * @return the artifact properties, never {@code null}
+ */
+ @Nonnull
+ ArtifactProperties getArtifactProperties();
Review Comment:
Exactly, happens similarly as in Resolver:
* IF "ad-hoc" resolution happened (i.e. plugin used resolve service) it is _plugin that knows why it did that_
* IF artifact entered via project (as dependency) then you have dependency -> type -> artifact.
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #1391:
URL: https://github.com/apache/maven/pull/1391#issuecomment-1910193302
First "block" or "step" is here https://github.com/apache/maven/pull/1392
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #1391:
URL: https://github.com/apache/maven/pull/1391#issuecomment-1909864873
> @gnodet and this is why I think that "step 1" should have been merged. This discussion will take some, and this PR will just potentially drift away from master (unless we freeze master, which I don't want).
Maybe, but there's no point in the first PR if we don't go further...
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #1391:
URL: https://github.com/apache/maven/pull/1391#discussion_r1466200346
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Artifact.java:
##########
@@ -105,6 +105,14 @@ default String key() {
*/
boolean isSnapshot();
+ /**
+ * The artifact properties.
+ *
+ * @return the artifact properties, never {@code null}
+ */
+ @Nonnull
+ ArtifactProperties getArtifactProperties();
Review Comment:
> No, `Type` is a property of `Dependency`, no Artifact. What happens is that is just an indirection of `Type` -> `properties`. If you think about, EVERY Artifact (sans root) have type (as they enter scope as dependencies of root).
That's wrong. Artifacts can be resolved without any Type / Dependency at all using the `ArtifactResolver`:
https://github.com/apache/maven/blob/2788849c37ed4c425eb9532c5e20b17f85cbe396/api/maven-api-core/src/main/java/org/apache/maven/api/services/ArtifactResolver.java
That's the whole point. ArtifactCoordinate is used to obtain an Artifact. A DependencyCoordinate is used to obtain a Dependency. `DependencyCoordinate` contains `Type` and `Scope`.
If we need some properties on `Artifact`, that's fine, we can define `ArtifactProperties` and `DependencyProperties` which extends `ArtifactProperties`. The artifact properties should contain only properties that are related to an artifact, not its usage (type, scope, etc...). But my understanding is that there's no properties at the moment. The only one in the resolver is the path of the file, which is obtained from the `ArtifactManager`.
> And yes, am looking at Resolver API as it is done well and works. IMO we should really just pull a thin layer on top of it for start, that we can refine later. But in general, this is how things should work, really.
Well, it works, but it's really confusing around this part. Things are blurred and difficult to understand imho. The goal was to split things more clearly, and adding dependency properties on the artifact goes in the wrong way imho.
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "desruisseaux (via GitHub)" <gi...@apache.org>.
desruisseaux commented on code in PR #1391:
URL: https://github.com/apache/maven/pull/1391#discussion_r1477061030
##########
api/maven-api-core/src/main/java/org/apache/maven/api/DependencyScope.java:
##########
@@ -0,0 +1,125 @@
+/*
+ * 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.api;
+
+import java.util.Collections;
+import java.util.Map;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.annotations.Immutable;
+import org.apache.maven.api.annotations.Nonnull;
+
+/**
+ * Dependency scope.
+ * <p>
+ * Implementation must have {@code equals()} and {@code hashCode()} implemented, so implementations of this interface
+ * can be used as keys.
+ *
+ * @since 4.0.0
+ */
+@Experimental
+@Immutable
+public enum DependencyScope {
+
+ /**
+ * None. Allows you to declare dependencies (for example to alter reactor build order) but in reality dependencies
+ * in this scope are not part of any build path scope.
+ */
+ NONE("none", false),
+
+ /**
+ * Empty scope.
Review Comment:
Maybe explain the different between "empty" and "none"?
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #1391:
URL: https://github.com/apache/maven/pull/1391#discussion_r1466200346
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Artifact.java:
##########
@@ -105,6 +105,14 @@ default String key() {
*/
boolean isSnapshot();
+ /**
+ * The artifact properties.
+ *
+ * @return the artifact properties, never {@code null}
+ */
+ @Nonnull
+ ArtifactProperties getArtifactProperties();
Review Comment:
> No, `Type` is a property of `Dependency`, no Artifact. What happens is that is just an indirection of `Type` -> `properties`. If you think about, EVERY Artifact (sans root) have type (as they enter scope as dependencies of root).
That's wrong. Artifacts can be resolved without any Type / Dependency at all using the `ArtifactResolver`:
https://github.com/apache/maven/blob/2788849c37ed4c425eb9532c5e20b17f85cbe396/api/maven-api-core/src/main/java/org/apache/maven/api/services/ArtifactResolver.java
That's the whole point. ArtifactCoordinate is used to obtain an Artifact. A DependencyCoordinate is used to obtain a Dependency. `DependencyCoordinate` contains `Type` and `Scope`.
If we need some properties on `Artifact`, that's fine, we can define `ArtifactProperties` and `DependencyProperties` which extends `ArtifactProperties`. The artifact properties should contain only properties that are related to an artifact, not its usage (type, scope, etc...). But my understanding is that there's no properties at the moment. The only one in the resolver is the path of the file, which is obtained from the `ArtifactManager`.
> And yes, am looking at Resolver API as it is done well and works. IMO we should really just pull a thin layer on top of it for start, that we can refine later. But in general, this is how things should work, really.
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #1391:
URL: https://github.com/apache/maven/pull/1391#discussion_r1466207334
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Artifact.java:
##########
@@ -105,6 +105,14 @@ default String key() {
*/
boolean isSnapshot();
+ /**
+ * The artifact properties.
+ *
+ * @return the artifact properties, never {@code null}
+ */
+ @Nonnull
+ ArtifactProperties getArtifactProperties();
Review Comment:
Sorry, used wrong lingo. You are right "artifact resolve" is really "just download that single file for that artifact, no deps involved". I was talking about `collect+resolve` that is the main use case of Resolver, as there, all the artifacts you end up with (resolved) are dependencies except the root, but that one as well may have properties if you used `ArtifactType` to construct it.
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #1391:
URL: https://github.com/apache/maven/pull/1391#discussion_r1466288875
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Artifact.java:
##########
@@ -105,6 +105,14 @@ default String key() {
*/
boolean isSnapshot();
+ /**
+ * The artifact properties.
+ *
+ * @return the artifact properties, never {@code null}
+ */
+ @Nonnull
+ ArtifactProperties getArtifactProperties();
Review Comment:
> Side note: the long time ago pattern of doing pom for deps is still accurate today and bom created more mess than helped there - but unrelated to the PR, just wanted to highlight this is still used for goods and not legacy.
>
> Ok but it means we need a dependency handler as we have an artifact handler (think it is a bit complex). Ultimately, do we need dependency outside the model (literally ) or can it not just be artifacts? I understand what you mean but it seems like a lot of work for not much gain, no? Can't we merge it somehow in new api?
There's no ArtifactHandler in the v4 api. It's called `Type` and it represents the usage of an Artifact as a Dependency. There's nothing to handle afaik on an Artifact: it's just a pointer to a file in a repository. How it's used needs to be taken care of and that's done in the `Dependency` which is associated to a `Type` and `Scope` and `DependencyProperties`. If we move things to the Artifact or merge Artifact and Dependency, I think that's a source of confusion.
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #1391:
URL: https://github.com/apache/maven/pull/1391#discussion_r1464830997
##########
maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/scopes/MavenDependencyScopes.java:
##########
@@ -0,0 +1,53 @@
+/*
+ * 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.repository.internal.scopes;
+
+/**
+ * The dependency scopes used for Java dependencies in Maven. This class defines labels only, that are doing pass-thru
+ * over Resolver.
+ *
+ * @since 4.0.0
+ */
+public final class MavenDependencyScopes {
Review Comment:
Should that be part of the Maven 4 API ?
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #1391:
URL: https://github.com/apache/maven/pull/1391#discussion_r1466149072
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Artifact.java:
##########
@@ -105,6 +105,14 @@ default String key() {
*/
boolean isSnapshot();
+ /**
+ * The artifact properties.
+ *
+ * @return the artifact properties, never {@code null}
+ */
+ @Nonnull
+ ArtifactProperties getArtifactProperties();
Review Comment:
I disagree here: Dependency is what you have in POMs (either project or transitive deps). So, when you resolve, you end up with bunch of `Artifact`s, NOT `Dependency`ies. Basically, that is why this is needed on `Artifact` and not `Dependency` (where is also needed, as properties from type should and could be still augmented).
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #1391:
URL: https://github.com/apache/maven/pull/1391#issuecomment-1910189997
Just a general remark: Am fine if we "redefine" here everything, from ground up, but IMHO "safest" would be to go "resolver way" (adhere to its abstractions closest as possible) as it is proven and works. And then improve it from there (consider "resolver way" as starting point), instead to try to come up with something completely new and different, and risk the problem of something (some use case, edge case) being forgotten, and then go thru pain of "fixing it" in upcoming API versions...
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #1391:
URL: https://github.com/apache/maven/pull/1391#discussion_r1466313211
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Artifact.java:
##########
@@ -105,6 +105,14 @@ default String key() {
*/
boolean isSnapshot();
+ /**
+ * The artifact properties.
+ *
+ * @return the artifact properties, never {@code null}
+ */
+ @Nonnull
+ ArtifactProperties getArtifactProperties();
Review Comment:
Resolver also encodes "type" in Artifact#properties. It also exposed since eons ctors for DefaultArtifact where you could pass in ArtifactType. The type you declare (i.e. if in project, you have packaging -> type) or if "ad-hoc" by root dependency node type (where callar knows what it does).
Or in short:
* artifact is a file decorated with coordinates and properties
* dependency is an artifact having extras like scope, type, optionality using which you can "derive" an artifact as above
* true, in Resolver "dependency" is ONLY a thing that comes from POM (ArtifactDescriptor) but it is being distilled to artifact (as in first bullet).
Basically "everything is an artifact", and "dependency is an artifact that enters the game as dependency". This is how Resolver abstracts.
Doing collect+resolve operation and ending up with list of sole Dependencies is what you say?
When you collect+resolve, you are interested in all, so even root (again, this is for "ad-hoc" operations, like javadoc may tell: "resolve me this taglet and its dependencies", in that case, javadoc plugin KNOWS root is taglet, but will still need to handle the rest of transitive hull, and place them or use that as needed).
---
@gnodet if I understand you correctly, sans "root", you consider _everything else_ (the transitive hull) as "dependency"? Which kinda makes sense, as they DID enter the result collection as "dependencies of root"?
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #1391:
URL: https://github.com/apache/maven/pull/1391#discussion_r1466386770
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Artifact.java:
##########
@@ -105,6 +105,14 @@ default String key() {
*/
boolean isSnapshot();
+ /**
+ * The artifact properties.
+ *
+ * @return the artifact properties, never {@code null}
+ */
+ @Nonnull
+ ArtifactProperties getArtifactProperties();
Review Comment:
> Resolver also encodes "type" in Artifact#properties. It also exposed since eons ctors for DefaultArtifact where you could pass in ArtifactType. The type you declare (i.e. if in project, you have packaging -> type) or if "ad-hoc" by root dependency node type (where callar knows what it does).
>
> Or in short:
>
> * artifact is a file decorated with coordinates and properties
> * dependency is an artifact having extras like scope, type, optionality using which you can "derive" an artifact as above
> * true, in Resolver "dependency" is ONLY a thing that comes from POM (ArtifactDescriptor) but it is being distilled to artifact (as in second bullet -> first bullet).
>
> Basically "everything is an artifact", and "dependency is an artifact that enters the game as dependency". This is how Resolver abstracts.
Yes, and I think that's where some confusion comes from. Everything is an artifact means that when you have an Artifact object, you don't really know what it is: it can be 5 different things:
* a pointer to a file (with a version range): ArtifactCoordinate
* a resolved artifact in a repository (a definite version): Artifact
* a pointer to a dependency (version range + scope + type)
* a resolved dependency: the resolver actually has `org.eclipse.aether.graph.Dependency` and that's what the v4 api wraps
* an artifact generated by the project
> Doing collect+resolve operation and ending up with list of sole Dependencies is what you say?
This is also a confusing part. It's conceptually a 3 step process: collect + flatten + resolve. The collect grabs the input and computes the graph of Node. Flattens will output a *list* of Node. Then the resolution step becomes the same as the _artifact resolution_ because each node can be turned into an artifact coordinate, and we then just download the actual artifacts.
In the v4 api, we have `ArtifactResolver` which gives an `Artifact`. The `DependencyCollector` returns the graph with `Node`s. The `DependencyResolver` performs both `flatten` which gives an ordered list of `Node`s, and `resolve` which returns an ordered `Map<Dependency, Path>` which I think is what is needed to compute the build paths.
> When you collect+resolve, you are interested in all, so even root (again, this is for "ad-hoc" operations, like javadoc may tell: "resolve me this taglet and its dependencies", in that case, javadoc plugin KNOWS root is taglet, but will still need to handle the rest of transitive hull, and place them or use that as needed).
>
> @gnodet if I understand you correctly, sans "root", you consider _everything else_ (the transitive hull) as "dependency"? Which kinda makes sense, as they DID enter the result collection as "dependencies of root"?
Yes, everything is a dependency. It's only the root which can be a project or artifact, but when you collect the dependencies for a project or artifact, you actually resolve all its *dependencies*.
I think there may be a small piece missing, which is the ability to access the graph from a dependency, i.e. adding a `Node getNode()` on the `org.apache.maven.api.Dependency` to get back to the graph.
At some point, I did envision to call it `ResolvedDependency` which would avoid the confusion with the _model dependency_, which is is translated into a `DependencyCoordinate`, the collected / flattened / resolved.
To sum up: the v3 api lacks a bunch of things, that's why plugins have to use the resolver to get fine grain access to the resolution output. The v4 api aims to expose those bits, but those aren't really new, the `org.apache.maven.api.Dependency` simply wraps `org.eclipse.aether.graph.Dependency`. That was clearly a missing piece in the v3 api.
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #1391:
URL: https://github.com/apache/maven/pull/1391#issuecomment-1909793144
@gnodet and this is why I think that "step 1" should have been merged. This discussion will take some, and this PR will just potentially drift away from master (unless we freeze master, which I don't want).
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #1391:
URL: https://github.com/apache/maven/pull/1391#issuecomment-1909785670
> I also find the term `build path` somewhat misleading...
Am completely fine to keep "resolution path" as well, but did not want to disturb existing classes...
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #1391:
URL: https://github.com/apache/maven/pull/1391#discussion_r1466156908
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Artifact.java:
##########
@@ -105,6 +105,14 @@ default String key() {
*/
boolean isSnapshot();
+ /**
+ * The artifact properties.
+ *
+ * @return the artifact properties, never {@code null}
+ */
+ @Nonnull
+ ArtifactProperties getArtifactProperties();
Review Comment:
Or in other words: see this PR https://github.com/apache/maven-resolver/pull/411
Basically Resolver can be now asked to:
* create Artifact as `new Artifact("g:a:v", artifactTypeRegistry.get("annotation-processor"))`.
* the to resolve it to have it's full transitive hull
* and you again end up with "bunch of Artifacts"
No dependency in scope, but you are STILL interested in Artifact properties....
Or similarly for ANY artifact, as if artifact you are resolving may have type="modular-jar", then the needed info will be still present in artifact.properties. Again, no dependencies involved, etc.
Am pretty much confident that this change is correct.
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #1391:
URL: https://github.com/apache/maven/pull/1391#issuecomment-1909896797
> Maybe, but there's no point in the first PR if we don't go further...
Um, the point of first commit is to move off from all the deprecated classes in resolver (see https://github.com/apache/maven-resolver/pull/412 and related JIRA), as currently, boundary of "scope interpretation and transformation" is completely blurred and doubled between Resolver and Maven Core. Things like adding new scopes is not trivial at all, as it requires changes in both projects, whereis Resolver should be really oblivious about scopes.
The first commit is really not related to 2nd step, it just does the right thing (moves all scope related logic to Maven Core), that in turn also effectively clears grounds for step2.
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "desruisseaux (via GitHub)" <gi...@apache.org>.
desruisseaux commented on code in PR #1391:
URL: https://github.com/apache/maven/pull/1391#discussion_r1477061773
##########
api/maven-api-core/src/main/java/org/apache/maven/api/ExtensibleEnum.java:
##########
@@ -0,0 +1,37 @@
+/*
+ * 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.api;
+
+import org.apache.maven.api.annotations.Nonnull;
+
+/**
+ * Implementation must have {@code equals()} and {@code hashCode()} implemented, so implementations of this interface
+ * can be used as keys.
+ */
+public interface ExtensibleEnum {
Review Comment:
Just for information: the International Organization for Standardization (ISO) uses the `CodeList` term for extensible enumerations, at least in the ISO 19xxx series of international standards (geospatial information). But I did not verified if `CodeList` was consistently used in all other standards as well.
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet merged PR #1391:
URL: https://github.com/apache/maven/pull/1391
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "rmannibucau (via GitHub)" <gi...@apache.org>.
rmannibucau commented on code in PR #1391:
URL: https://github.com/apache/maven/pull/1391#discussion_r1466226764
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Artifact.java:
##########
@@ -105,6 +105,14 @@ default String key() {
*/
boolean isSnapshot();
+ /**
+ * The artifact properties.
+ *
+ * @return the artifact properties, never {@code null}
+ */
+ @Nonnull
+ ArtifactProperties getArtifactProperties();
Review Comment:
Side note: the long time ago pattern of doing pom for deps is still accurate today and bom created more mess than helped there - but unrelated to the PR, just wanted to highlight this is still used for goods and not legacy.
Ok but it means we need a dependency handler as we have an artifact handler (think it is a bit complex).
Ultimately, do we need dependency outside the model (literally <dependency>) or can it not just be artifacts?
I understand what you mean but it seems like a lot of work for not much gain, no? Can't we merge it somehow in new api?
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #1391:
URL: https://github.com/apache/maven/pull/1391#discussion_r1466313211
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Artifact.java:
##########
@@ -105,6 +105,14 @@ default String key() {
*/
boolean isSnapshot();
+ /**
+ * The artifact properties.
+ *
+ * @return the artifact properties, never {@code null}
+ */
+ @Nonnull
+ ArtifactProperties getArtifactProperties();
Review Comment:
Resolver also encodes "type" in Artifact#properties. It also exposed since eons ctors for DefaultArtifact where you could pass in ArtifactType. The type you declare (i.e. if in project, you have packaging -> type) or if "ad-hoc" by root dependency node type (where callar knows what it does).
Or in short:
* artifact is a file decorated with coordinates and properties
* dependency is an artifact having extras like scope, type, optionality using which you can "derive" an artifact as above
* true, in Resolver "dependency" is ONLY a thing that comes from POM (ArtifactDescriptor) but it is being distilled to artifact (as in second bullet -> first bullet).
Basically "everything is an artifact", and "dependency is an artifact that enters the game as dependency". This is how Resolver abstracts.
Doing collect+resolve operation and ending up with list of sole Dependencies is what you say?
When you collect+resolve, you are interested in all, so even root (again, this is for "ad-hoc" operations, like javadoc may tell: "resolve me this taglet and its dependencies", in that case, javadoc plugin KNOWS root is taglet, but will still need to handle the rest of transitive hull, and place them or use that as needed).
---
@gnodet if I understand you correctly, sans "root", you consider _everything else_ (the transitive hull) as "dependency"? Which kinda makes sense, as they DID enter the result collection as "dependencies of root"?
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #1391:
URL: https://github.com/apache/maven/pull/1391#discussion_r1464865561
##########
maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/scopes/MavenDependencyScopes.java:
##########
@@ -0,0 +1,53 @@
+/*
+ * 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.repository.internal.scopes;
+
+/**
+ * The dependency scopes used for Java dependencies in Maven. This class defines labels only, that are doing pass-thru
+ * over Resolver.
+ *
+ * @since 4.0.0
+ */
+public final class MavenDependencyScopes {
Review Comment:
Yes, when the followup PR ultimate goal is reached (and it will tackle Maven API as well), this class MavenDependencyScopes should be gone
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #1391:
URL: https://github.com/apache/maven/pull/1391#discussion_r1465554808
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Artifact.java:
##########
@@ -105,6 +105,14 @@ default String key() {
*/
boolean isSnapshot();
+ /**
+ * The artifact properties.
+ *
+ * @return the artifact properties, never {@code null}
+ */
+ @Nonnull
+ ArtifactProperties getArtifactProperties();
Review Comment:
well, then they should be kept on the `Dependency`
##########
api/maven-api-core/src/main/java/org/apache/maven/api/BuildPath.java:
##########
@@ -0,0 +1,48 @@
+/*
+ * 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.api;
+
+import java.util.Collection;
+
+import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.annotations.Immutable;
+import org.apache.maven.api.annotations.Nonnull;
+
+/**
+ * Build path. Build path is calculated for given {@link Project} and {@link BuildPathScope}.
Review Comment:
It should be noted what the build path is and what's the purpose. It's not a common java notion, unlike class path and module path.
Especially the "build" term could lead to think it's role is build time, but that's not the case and it can be used at build time or run time.
##########
api/maven-api-core/src/main/java/org/apache/maven/api/BuildPath.java:
##########
@@ -0,0 +1,48 @@
+/*
+ * 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.api;
+
+import java.util.Collection;
+
+import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.annotations.Immutable;
+import org.apache.maven.api.annotations.Nonnull;
+
+/**
+ * Build path. Build path is calculated for given {@link Project} and {@link BuildPathScope}.
+ *
+ * @since 4.0.0
+ */
+@Experimental
+@Immutable
+public interface BuildPath {
+ @Nonnull
+ Project project();
+
+ @Nonnull
+ BuildPathScope buildPathScope();
+
+ @Nonnull
+ default Language language() {
+ return project().getLanguage();
+ }
+
+ @Nonnull
+ Collection<Artifact> getArtifacts();
Review Comment:
A `Node` would be required.
From that one, we can derive a `List<Dependency>` and eventually a `List<Artifact>`. But that's related to my remark around artifact properties which I think are still dependency properties.
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #1391:
URL: https://github.com/apache/maven/pull/1391#discussion_r1466175527
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Artifact.java:
##########
@@ -105,6 +105,14 @@ default String key() {
*/
boolean isSnapshot();
+ /**
+ * The artifact properties.
+ *
+ * @return the artifact properties, never {@code null}
+ */
+ @Nonnull
+ ArtifactProperties getArtifactProperties();
Review Comment:
No, `Type` is a property of `Dependency`, no Artifact. What happens is that is just an indirection of `Type` -> `properties`. If you think about, EVERY Artifact (sans root) have type (as they enter scope as dependencies of root).
And yes, am looking at Resolver API as it is done well and works. IMO we should really just pull a thin layer on top of it for start, that we can refine later. But in general, this is how things should work, really.
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #1391:
URL: https://github.com/apache/maven/pull/1391#discussion_r1465533187
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Packaging.java:
##########
@@ -0,0 +1,43 @@
+/*
+ * 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.api;
+
+import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.annotations.Immutable;
+import org.apache.maven.api.annotations.Nonnull;
+
+/**
+ * Interface representing a Maven project packaging.
+ *
+ * @since 4.0.0
+ */
+@Experimental
+@Immutable
+public interface Packaging {
Review Comment:
Yes, see below `Project`
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #1391:
URL: https://github.com/apache/maven/pull/1391#issuecomment-1908920207
There is one maven-core UT failing as DefaultPackagingManager seems not set somewhere.
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #1391:
URL: https://github.com/apache/maven/pull/1391#discussion_r1466156908
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Artifact.java:
##########
@@ -105,6 +105,14 @@ default String key() {
*/
boolean isSnapshot();
+ /**
+ * The artifact properties.
+ *
+ * @return the artifact properties, never {@code null}
+ */
+ @Nonnull
+ ArtifactProperties getArtifactProperties();
Review Comment:
Or in other words: see this PR https://github.com/apache/maven-resolver/pull/411
Basically Resolver can be now asked to:
* create Artifact as `new Artifact("g:a:v", artifactTypeRegistry.get("annotation-processor"))`. (and this ctor is not new, in a way, you could do the same even with existing ctors, this was just a shorthand to make possible to use type with string "coords").
* the to resolve it to have it's full transitive hull
* and you again end up with "bunch of Artifacts", but you still want to sift thru them, filter them by properties
No dependency in scope, but you are STILL interested in Artifact properties....
Or similarly for ANY artifact, as if artifact you are resolving may have type="modular-jar", then the needed info will be still present in artifact.properties. Again, no dependencies involved, etc.
Am pretty much confident that this change is correct.
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "rmannibucau (via GitHub)" <gi...@apache.org>.
rmannibucau commented on PR #1391:
URL: https://github.com/apache/maven/pull/1391#issuecomment-1908933445
@cstamas I see the 1-1 with what exists but I have some hard time - blame the hour maybe ;) - to see how we can make it consistent with the type work which is pending, typically build path will not be a build path but more a resolved artifact set totally unrelated to any path now, no? So for me scope becomes just a resolution stage more than anything else. Am I missing something?
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #1391:
URL: https://github.com/apache/maven/pull/1391#discussion_r1464831692
##########
maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/scopes/MavenDependencyScopes.java:
##########
@@ -0,0 +1,53 @@
+/*
+ * 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.repository.internal.scopes;
+
+/**
+ * The dependency scopes used for Java dependencies in Maven. This class defines labels only, that are doing pass-thru
+ * over Resolver.
+ *
+ * @since 4.0.0
+ */
+public final class MavenDependencyScopes {
Review Comment:
Or is that for a follow-up PR, maybe it needs other adjustements...
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #1391:
URL: https://github.com/apache/maven/pull/1391#issuecomment-1908578674
The most important: `project + buildPathScope == buildPath` but `buildPath` is not done fully (just there as placeholder).
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #1391:
URL: https://github.com/apache/maven/pull/1391#discussion_r1477061342
##########
api/maven-api-core/src/main/java/org/apache/maven/api/DependencyScope.java:
##########
@@ -0,0 +1,125 @@
+/*
+ * 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.api;
+
+import java.util.Collections;
+import java.util.Map;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.annotations.Immutable;
+import org.apache.maven.api.annotations.Nonnull;
+
+/**
+ * Dependency scope.
+ * <p>
+ * Implementation must have {@code equals()} and {@code hashCode()} implemented, so implementations of this interface
+ * can be used as keys.
+ *
+ * @since 4.0.0
+ */
+@Experimental
+@Immutable
+public enum DependencyScope {
+
+ /**
+ * None. Allows you to declare dependencies (for example to alter reactor build order) but in reality dependencies
+ * in this scope are not part of any build path scope.
+ */
+ NONE("none", false),
+
+ /**
+ * Empty scope.
Review Comment:
We saw that "empty" is present in old codebase (3.x), maybe will go away, but we left it here just to be on safe side until we clear up things.
The "none" on the other hand is clearly defined IMHO: merely added just to affect the reactor sorting.
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #1391:
URL: https://github.com/apache/maven/pull/1391#discussion_r1464863514
##########
maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/scopes/MavenDependencyScopes.java:
##########
@@ -0,0 +1,53 @@
+/*
+ * 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.repository.internal.scopes;
+
+/**
+ * The dependency scopes used for Java dependencies in Maven. This class defines labels only, that are doing pass-thru
+ * over Resolver.
+ *
+ * @since 4.0.0
+ */
+public final class MavenDependencyScopes {
Review Comment:
Yes, this PR merely "sets the baseline": all is as before, but code is now part of Maven codebase (not blurred between Resolver and Maven).
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #1391:
URL: https://github.com/apache/maven/pull/1391#issuecomment-1909448765
> @cstamas does it mean we should fix the naming for mvn 4 and align it on what it is? Resolverpaths or alike since it is unrelated to any actual build and becomes even more ambiguous with jpms?
I also find the term `build path` somewhat misleading...
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #1391:
URL: https://github.com/apache/maven/pull/1391#discussion_r1464861777
##########
maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/MavenRepositorySystemUtils.java:
##########
@@ -49,7 +49,9 @@
* assist those clients that employ the repository system outside of an IoC container, Maven plugins should instead
* always use regular dependency injection to acquire the repository system.
*
+ * @deprecated See {@link MavenSessionBuilderSupplier}
*/
+@Deprecated
public final class MavenRepositorySystemUtils {
Review Comment:
Shouldn't this class be re-implemented on top of MavenSessionBuilderSupplier, so that any change in the new one can be somewhat reflected in the old one ?
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #1391:
URL: https://github.com/apache/maven/pull/1391#discussion_r1465532360
##########
maven-core/src/main/java/org/apache/maven/internal/impl/DefaultArtifact.java:
##########
@@ -95,7 +97,12 @@ public String getClassifier() {
@Override
public boolean isSnapshot() {
- return DefaultModelVersionParser.checkSnapshot(artifact.getVersion());
+ return artifact.isSnapshot();
Review Comment:
We had this discussion weeks ago, I think we agreed the snapshot knowledge was not on the artifact. We now have a pluggable `VersionScheme`, I think it belongs there.
Anyway, I think those changes related to snapshots are unrelated and would be better in a separate PR to ease the discussion.
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Artifact.java:
##########
@@ -105,6 +105,14 @@ default String key() {
*/
boolean isSnapshot();
+ /**
+ * The artifact properties.
+ *
+ * @return the artifact properties, never {@code null}
+ */
+ @Nonnull
+ ArtifactProperties getArtifactProperties();
Review Comment:
I don't get this change. An `Artifact` is obtained from coordinates and as no knowledge on how it will be used. The same pointer could point to the same artifact, but with different `Type`, hence different properties. I think those do belong to the resolved. `Dependency`
##########
api/maven-api-core/src/main/java/org/apache/maven/api/BuildPathScope.java:
##########
@@ -0,0 +1,46 @@
+/*
+ * 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.api;
+
+import java.util.Collection;
+
+import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.annotations.Immutable;
+import org.apache.maven.api.annotations.Nonnull;
+
+/**
+ * Build path scope.
+ * <p>
+ * Implementation must have {@code equals()} and {@code hashCode()} implemented, so implementations of this interface
+ * can be used as keys.
+ *
+ * @since 4.0.0
+ */
+@Experimental
+@Immutable
+public interface BuildPathScope {
+ @Nonnull
+ String id();
+
+ @Nonnull
+ Language language();
+
+ @Nonnull
+ Collection<DependencyScope> getDependencyScopes();
Review Comment:
`Set<DependencyScope>` ?
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Packaging.java:
##########
@@ -0,0 +1,43 @@
+/*
+ * 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.api;
+
+import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.annotations.Immutable;
+import org.apache.maven.api.annotations.Nonnull;
+
+/**
+ * Interface representing a Maven project packaging.
+ *
+ * @since 4.0.0
+ */
+@Experimental
+@Immutable
+public interface Packaging {
Review Comment:
I don't see this `Packaging` used. Is the goal to put it on the `Project` ?
##########
api/maven-api-core/src/main/java/org/apache/maven/api/services/LanguageManager.java:
##########
@@ -0,0 +1,34 @@
+/*
+ * 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.api.services;
+
+import java.util.Optional;
+
+import org.apache.maven.api.Language;
+import org.apache.maven.api.Service;
+import org.apache.maven.api.annotations.Nonnull;
+
+public interface LanguageManager extends Service {
+ @Nonnull
+ Optional<Language> lookupLanguageFamily(String id);
+
+ default Language requireLanguageFamily(String id) {
+ return lookupLanguageFamily(id).orElseThrow(() -> new IllegalArgumentException("Unknown language"));
Review Comment:
The requested language should be in the exception message.
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #1391:
URL: https://github.com/apache/maven/pull/1391#discussion_r1465540157
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Artifact.java:
##########
@@ -105,6 +105,14 @@ default String key() {
*/
boolean isSnapshot();
+ /**
+ * The artifact properties.
+ *
+ * @return the artifact properties, never {@code null}
+ */
+ @Nonnull
+ ArtifactProperties getArtifactProperties();
Review Comment:
Exactly, happens similarly as in Resolver:
* IF "ad-hoc" resolution happened (i.e. plugin used resolve service) it is _plugin that knows why it did that_
* IF artifact entered via project (as dependency) then you have dependency -> type -> artifact so properties are set.
We may need something more for this, still refining.
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #1391:
URL: https://github.com/apache/maven/pull/1391#discussion_r1466175527
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Artifact.java:
##########
@@ -105,6 +105,14 @@ default String key() {
*/
boolean isSnapshot();
+ /**
+ * The artifact properties.
+ *
+ * @return the artifact properties, never {@code null}
+ */
+ @Nonnull
+ ArtifactProperties getArtifactProperties();
Review Comment:
No, `Type` is a property of `Dependency`. What happens is that is just an indirection of `Type` -> `properties`. If you think about, EVERY Artifact (sans root) have type (as they enter scope as dependencies of root).
And yes, am looking at Resolver API as it is done well and works. IMO we should really just pull a thin layer on top of it for start, that we can refine later. But in general, this is how things should work, really.
--
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
Re: [PR] [MNG-8026] Maven drives regarding scopes [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #1391:
URL: https://github.com/apache/maven/pull/1391#discussion_r1466213690
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Artifact.java:
##########
@@ -105,6 +105,14 @@ default String key() {
*/
boolean isSnapshot();
+ /**
+ * The artifact properties.
+ *
+ * @return the artifact properties, never {@code null}
+ */
+ @Nonnull
+ ArtifactProperties getArtifactProperties();
Review Comment:
> Sorry, used wrong lingo. You are right "artifact resolve" is really "just download that single file for that artifact, no deps involved". I was talking about `collect+resolve` that is the main use case of Resolver, as there, all the artifacts you end up with (resolved) are dependencies except the root, but that one as well may have properties if you used `ArtifactType` to construct it.
That's exactly why I think we should keep things split more clearly between Artifact and Dependency. We have artifacts which are related to repositories and dependencies for... dependencies. Different words for different meanings. We should make that more clear rather than blur things...
--
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