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 2023/11/08 22:00:05 UTC
[PR] [MNG-7924] Better control over and better integration with Resolver [maven]
cstamas opened a new pull request, #1299:
URL: https://github.com/apache/maven/pull/1299
Integrate better and obtain better control over Resolver. These changes did stem from "JPMS module experiment" and are considered improvement but does not implement any functionality related to JPMS module support.
Changes:
* Maven4 should stop "disconnected coexistence" of two type systems (ArtifactHandlers and Resolver ArtifactTypeRegistry), it should unify them.
* Maven4 Core should provide generic and extensible means to introduce new artifact types (fully in extension, and extension should get extended data via "roundtrip" in core/resolver)
--
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-7924] Better control over and better integration with Resolver [maven]
Posted by "rmannibucau (via GitHub)" <gi...@apache.org>.
rmannibucau commented on PR #1299:
URL: https://github.com/apache/maven/pull/1299#issuecomment-1803303626
Is it planned to make the properties injected from the pom "mvn 4 version" and therefore makes the type dictionnary useless?
If so +1 (means the toggles will be exposed to users and type can stick to extension and management in the build handled by end user, best of all worlds), else I'm mixed since it does not change much things if it stays internal 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-7924] Better control over and better integration with Resolver [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #1299:
URL: https://github.com/apache/maven/pull/1299#issuecomment-1803773056
And a counter example: consider maven-resolver artifacts (!):
* uses plain packaging "jar", but
* the produced JARs have Sisu index (Sisu DI, JSR330)
* the produced JARs have OSGi header (OSGi)
* the produced JARs have "Automatic-Module-Name" header (JPMS)
It is up to "consumer" how he consumes these things, which (orthogonal) way he makes use of resolver:
* on classpath as plain JAR (using RepositorySystemSupplier)
* on classpath with Sisu DI (as in Maven)
* in OSGi container as bundles
* in JPMS app (unsure is it possible, but "metadata" is there)
So type is about "consuming", and consumed thing does NOT need to be rebuilt, to consume it as this or that.
--
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-7924] Better control over and better integration with Resolver [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas merged PR #1299:
URL: https://github.com/apache/maven/pull/1299
--
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-7924] Better control over and better integration with Resolver [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #1299:
URL: https://github.com/apache/maven/pull/1299#discussion_r1387548574
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Type.java:
##########
@@ -51,14 +51,21 @@ public interface Type {
* Returns the dependency type id.
* The id uniquely identifies this <i>dependency type</i>.
*
- * @return the id of this type
+ * @return the id of this type, never {@code null}.
*/
String getId();
+ /**
+ * Returns the dependency type language.
+ *
+ * @return the language of this type, never {@code null}.
+ */
+ String getLanguage();
+
/**
* Get the file extension of artifacts of this type.
*
- * @return the file extension
+ * @return the file extension, never {@code null}.
Review Comment:
Add `@Nonnull` to the return type.
Is this actually never null ? The ones coming from maven can be null afaik.
--
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-7924] Better control over and better integration with Resolver [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #1299:
URL: https://github.com/apache/maven/pull/1299#discussion_r1387813713
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Type.java:
##########
@@ -51,14 +51,21 @@ public interface Type {
* Returns the dependency type id.
* The id uniquely identifies this <i>dependency type</i>.
*
- * @return the id of this type
+ * @return the id of this type, never {@code null}.
*/
String getId();
+ /**
+ * Returns the dependency type language.
+ *
+ * @return the language of this type, never {@code null}.
+ */
+ String getLanguage();
Review Comment:
I may remove this, as it is used ONLY in conversion of Type to AH, but am unsure is it used anywhere at all...
We can always return it using DependencyProperties where Map<String, String> can not only hold "flags" (string values of "true" and "false") but something else 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-7924] Better control over and better integration with Resolver [maven]
Posted by "rmannibucau (via GitHub)" <gi...@apache.org>.
rmannibucau commented on PR #1299:
URL: https://github.com/apache/maven/pull/1299#issuecomment-1803806891
@cstamas one reason I really dislike type vs flags is type is not composable without coding (our limitation of being descriptive in our descriptor/pom - which is a good thing by itself but implies that limitation) whereas it has to be at some point in your examples.
Once again, technically we agreed a long time ago but still, users don't understand that, you seems to think they have to and I'd like to avoid them to understand all that. Ultimately it can be a semantic issue (type is not a "common" _type_) but it is a hurting thing in user land and I don't see with your PR which enables to not keep this ambiguity, why we would have to keep using it.
Ultimately your PR even enable to define custom "types" in the pom (predefined flags) so user can use it in build pom and then it is published properly with type=extension and flattened flags if desired.
--
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-7924] Better control over and better integration with Resolver [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #1299:
URL: https://github.com/apache/maven/pull/1299#issuecomment-1803688709
Another important thing people forget is that `type != packaging`, and due this bias/assumption they come up with wrong conclusions...
Type is NOT packaging, but they CAN be equal. Think about "producer" (project producing artifact) and "consumer" (project that consumes produced artifact). And let's stick with "jar artifact" for simplicity.
Producer can produce jar in several ways:
* packaging=jar the most ordinary way
* packaging=takari-jar very common
* packaging=my-supercool-jar (ie. I come up with some my own flavour of packaging for JARs)
* etc
As you see, technically "endless" options for producing JARs (technically, while in reality few people will care to introduce new packaging, but is not uncommon).
Consumer can consume these jars above in several ways:
* dependency type=jar the most ordinary way
* dependency type=takari-jar very uncommon, but here you must have takari-lifecycle extension present. In fact, this is semantically wrong, and shows my pet-peeve, even sonatype does it wrong
* dependency type=my-supercool-jar ditto, you need extension
So as you see, not that packaging != type but they are actually `n:m` or unsure, how to express it...
Sonatype does it wrong:
https://search.maven.org/artifact/io.takari.maven.plugins/takari-lifecycle-plugin/2.1.1/takari-maven-plugin
They recommend to "consume" this plugin as:
```
<dependency>
<groupId>io.takari.maven.plugins</groupId>
<artifactId>takari-lifecycle-plugin</artifactId>
<version>2.1.1</version>
<type>takari-maven-plugin</type>
</dependency>
```
Which is wrong, as to make Maven understand `takari-maven-plugin` you need takari-lifecycle-plugin :smile:
Simply put, Sonatype search suffers from [this issue](https://issues.apache.org/jira/browse/MNG-7373?focusedCommentId=17769743&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17769743)
--
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-7924] Better control over and better integration with Resolver [maven]
Posted by "rmannibucau (via GitHub)" <gi...@apache.org>.
rmannibucau commented on PR #1299:
URL: https://github.com/apache/maven/pull/1299#issuecomment-1803682292
I fully understand that and this is exactly why I'd like we stop doing that.
> We could add "flags" to dependencies, but that would make Maven very very "low level", and would trigger a huge cognitive load on end users IMHO. Rather move "baby steps" with improvements, and we will see where it gets us.
IMHO it is where we are today, almost no user or plugin writer fully understand the implications of a type and it is often misused.
The design of this PR opens the flags which are exactly the proper design and solution to handle the "context" you need and avoid to workaround the lack of flags with an existing marker, aka type today.
See it as it is, would you use `org.superbiz-classpath` in `groupid` to say that `org.superbiz` is in the classpath? Probably not but it is what we do in `type` cause of the lack of alternative.
With your PR we can assume it and move it to flags and I think it is what would serve the best users and us.
The only challenge will be to solve this in pom v4.0.0 (mvn 3) but ultimately since it is build flags it would just be a matter of sticking to type=extension rule so I think we would be good and help our community rather than promoting more a workaround.
--
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-7924] Better control over and better integration with Resolver [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #1299:
URL: https://github.com/apache/maven/pull/1299#discussion_r1389639253
##########
maven-core/src/main/java/org/apache/maven/internal/impl/DefaultDependencyProperties.java:
##########
@@ -0,0 +1,65 @@
+/*
+ * 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.internal.impl;
+
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+
+import org.apache.maven.api.DependencyProperties;
+import org.apache.maven.api.annotations.Nonnull;
+
+import static org.apache.maven.internal.impl.Utils.nonNull;
+
+/**
+ * Default implementation of artifact properties.
+ */
+public class DefaultDependencyProperties implements DependencyProperties {
+ private final Map<String, String> properties;
+
+ public DefaultDependencyProperties(String... flags) {
+ this(Arrays.asList(flags));
Review Comment:
Object... never can be null, it will be `[]` (empty array) at most
--
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-7924] Better control over and better integration with Resolver [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #1299:
URL: https://github.com/apache/maven/pull/1299#issuecomment-1803632036
> Is it planned to make the properties injected from the pom "mvn 4 version" and therefore makes the type dictionnary useless? If so +1 (means the toggles will be exposed to users and type can stick to extension and management in the build handled by end user, best of all worlds), else I'm mixed since it does not change much things if it stays internal only.
A bit fuzzy thought but let me try:
* "Is it planned to make the properties injected from the pom "mvn 4 version" NO
* "therefore makes the type dictionnary useless?" NO
* "means the toggles will be exposed to users" YES
* "type can stick to extension and management in the build handled by end user" um, is like that even today
* "if it stays internal only" um, Type is part of public Maven API, so it cannot be internal only per definitionem
--
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-7924] Better control over and better integration with Resolver [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #1299:
URL: https://github.com/apache/maven/pull/1299#issuecomment-1803653059
Type is part of Maven DNA, and let me try to explain why:
When you are on "resolver level" (so no Maven in vicinity, like MIMA or Resolver "demos" runs), you are dealing with "artifacts", that are really just "named files" or "files with coordinates", and coordinates are needed to lay these files out in some "space" (layout). There is NO whatsoever assumption about "what those files are". At this level resolver merely cares about getting you these files and publishing these files for you.
When you hop abstraction level higher, at Maven level, you do not deal (never!) with "artifacts" anymore, but you attribute the usage (or role) to artifact, so in Maven you deal with:
* artifact that is the project
* artifact that is the parent POM
* artifact that is maven-plugin
* artifact that is a dependency
* etc
(so, "artifact" is superset of ALL things Maven uses, but in Maven universe you KNOW and TELL (implicitly or explicitly) what you want that artifact for).
And while you have "implied types" (parent POM, maven-plugin, extension, etc are "implied types"), for dependencies you need something "extra" to express "what it is" (or "what I want with it"), and the type serves that purpose.
We could add "flags" to dependencies, but that would make Maven very very "low level", and would trigger a huge cognitive load on end users IMHO. Rather move "baby steps" with improvements, and we will see where it gets us.
--
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-7924] Better control over and better integration with Resolver [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #1299:
URL: https://github.com/apache/maven/pull/1299#discussion_r1387809160
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Type.java:
##########
@@ -51,14 +51,21 @@ public interface Type {
* Returns the dependency type id.
* The id uniquely identifies this <i>dependency type</i>.
*
- * @return the id of this type
+ * @return the id of this type, never {@code null}.
*/
String getId();
+ /**
+ * Returns the dependency type language.
+ *
+ * @return the language of this type, never {@code null}.
+ */
+ String getLanguage();
+
/**
* Get the file extension of artifacts of this type.
*
- * @return the file extension
+ * @return the file extension, never {@code null}.
Review Comment:
nope, they are never null for types. In maven, when ext=null, then it implies ext=typeId
--
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-7924] Better control over and better integration with Resolver [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #1299:
URL: https://github.com/apache/maven/pull/1299#discussion_r1387786371
##########
maven-core/src/main/java/org/apache/maven/artifact/handler/manager/LegacyArtifactHandlerManager.java:
##########
@@ -0,0 +1,72 @@
+/*
+ * 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.artifact.handler.manager;
+
+import javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+
+import org.apache.maven.artifact.handler.ArtifactHandler;
+import org.apache.maven.artifact.handler.DefaultArtifactHandler;
+
+/**
+ */
+@Named
+@Singleton
+public class LegacyArtifactHandlerManager implements ArtifactHandlerManager {
Review Comment:
This is a simple trick:
* legacyAHM is what "default" was so far (it have legacy AHs injected), is new, but nobody uses it as it is new thing (just new code "knows" about it)
* defaultAHM changed to delegate calls to typeRegistry
* typeRegistry uses as a fallback legacyAHM
This completes the circle:
* legacy code will refer to defaultAHM that goes to typeRegistry that will refer legacyAHM (as fallback)
* new code will refer to typeRegistry that will refer legacyAHM (as fallback)
This way new code and legacy code both "see" the same things: whether it is new AH (legacy) or Type (new) being added by extension for example
--
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-7924] Better control over and better integration with Resolver [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #1299:
URL: https://github.com/apache/maven/pull/1299#discussion_r1387543172
##########
api/maven-api-core/src/main/java/org/apache/maven/api/DependencyProperties.java:
##########
@@ -0,0 +1,54 @@
+/*
+ * 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.Map;
+
+import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.annotations.Immutable;
+
+/**
+ * Dependency properties supported by Maven Core.
+ *
+ * @since 4.0.0
+ */
+@Experimental
+@Immutable
+public interface DependencyProperties {
+ /**
+ * Boolean flag telling that dependency contains all of its dependencies.
+ * Note: this flag must be kept in sync with resolver!
+ */
+ String FLAG_INCLUDES_DEPENDENCIES = "includesDependencies";
+
+ /**
+ * Boolean flag telling that dependency is meant to be placed on class path.
+ */
+ String FLAG_CLASS_PATH_CONSTITUENT = "classPathConstituent";
+
+ /**
+ * Returns immutable "map view" of all the properties.
+ */
+ Map<String, String> asMap();
Review Comment:
Add `@Nonnull` to the return type.
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Type.java:
##########
@@ -51,14 +51,21 @@ public interface Type {
* Returns the dependency type id.
* The id uniquely identifies this <i>dependency type</i>.
*
- * @return the id of this type
+ * @return the id of this type, never {@code null}.
*/
String getId();
+ /**
+ * Returns the dependency type language.
Review Comment:
Is that natural language or code language ?
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Type.java:
##########
@@ -51,14 +51,21 @@ public interface Type {
* Returns the dependency type id.
* The id uniquely identifies this <i>dependency type</i>.
*
- * @return the id of this type
+ * @return the id of this type, never {@code null}.
*/
String getId();
+ /**
+ * Returns the dependency type language.
+ *
+ * @return the language of this type, never {@code null}.
+ */
+ String getLanguage();
+
/**
* Get the file extension of artifacts of this type.
*
- * @return the file extension
+ * @return the file extension, never {@code null}.
Review Comment:
Add @NonNull to the return type.
Is this actually never null ? The ones coming from maven can be null afaik.
##########
maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/MavenRepositorySystemUtils.java:
##########
@@ -59,8 +62,37 @@ private MavenRepositorySystemUtils() {
* the session with authentication, mirror, proxy and other information required for your environment.
*
* @return The new repository system session, never {@code null}.
+ * @deprecated This method is deprecated.
*/
+ @Deprecated
public static DefaultRepositorySystemSession newSession() {
+ DefaultArtifactTypeRegistry stereotypes = new DefaultArtifactTypeRegistry();
+ stereotypes.add(new DefaultArtifactType("pom"));
Review Comment:
Should we use `DefaultTypeRegistry` and the various providers here instead ?
##########
api/maven-api-core/src/main/java/org/apache/maven/api/DependencyProperties.java:
##########
@@ -0,0 +1,54 @@
+/*
+ * 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.Map;
+
+import org.apache.maven.api.annotations.Experimental;
+import org.apache.maven.api.annotations.Immutable;
+
+/**
+ * Dependency properties supported by Maven Core.
+ *
+ * @since 4.0.0
+ */
+@Experimental
+@Immutable
+public interface DependencyProperties {
+ /**
+ * Boolean flag telling that dependency contains all of its dependencies.
+ * Note: this flag must be kept in sync with resolver!
+ */
+ String FLAG_INCLUDES_DEPENDENCIES = "includesDependencies";
+
+ /**
+ * Boolean flag telling that dependency is meant to be placed on class path.
+ */
+ String FLAG_CLASS_PATH_CONSTITUENT = "classPathConstituent";
+
+ /**
+ * Returns immutable "map view" of all the properties.
+ */
+ Map<String, String> asMap();
+
+ /**
+ * Returns {@code true} if given flag is {@code true}.
+ */
+ boolean checkFlag(String flag);
Review Comment:
Add either `@Nonnull` or `@Nullable` to `flag`.
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Type.java:
##########
@@ -67,17 +74,19 @@ public interface Type {
* The default classifier can be overridden when specifying
* the {@link Dependency#getClassifier()}.
*
- * @return the default classifier
+ * @return the default classifier, or {@code null}.
*/
Review Comment:
Add `@Nullable` to the return type.
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Type.java:
##########
@@ -51,14 +51,21 @@ public interface Type {
* Returns the dependency type id.
* The id uniquely identifies this <i>dependency type</i>.
*
- * @return the id of this type
+ * @return the id of this type, never {@code null}.
*/
String getId();
Review Comment:
Add `@Nonnull` to the return type.
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Type.java:
##########
@@ -51,14 +51,21 @@ public interface Type {
* Returns the dependency type id.
* The id uniquely identifies this <i>dependency type</i>.
*
- * @return the id of this type
+ * @return the id of this type, never {@code null}.
*/
String getId();
+ /**
+ * Returns the dependency type language.
+ *
+ * @return the language of this type, never {@code null}.
+ */
+ String getLanguage();
Review Comment:
Add `@Nonnull` to the return type.
Has this ever been in use anywhere ?
##########
maven-core/src/main/java/org/apache/maven/artifact/handler/manager/DefaultArtifactHandlerManager.java:
##########
@@ -22,51 +22,55 @@
import javax.inject.Named;
import javax.inject.Singleton;
+import java.util.Collections;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
+import org.apache.maven.api.Type;
+import org.apache.maven.api.services.TypeRegistry;
import org.apache.maven.artifact.handler.ArtifactHandler;
import org.apache.maven.artifact.handler.DefaultArtifactHandler;
+import static java.util.Objects.requireNonNull;
+
/**
*/
@Named
@Singleton
public class DefaultArtifactHandlerManager implements ArtifactHandlerManager {
+ private final TypeRegistry typeRegistry;
- private final Map<String, ArtifactHandler> artifactHandlers;
-
- private final Map<String, ArtifactHandler> allHandlers = new ConcurrentHashMap<>();
+ private final ConcurrentHashMap<String, ArtifactHandler> handlers;
@Inject
- public DefaultArtifactHandlerManager(Map<String, ArtifactHandler> artifactHandlers) {
- this.artifactHandlers = artifactHandlers;
+ public DefaultArtifactHandlerManager(TypeRegistry typeRegistry) {
+ this.typeRegistry = requireNonNull(typeRegistry, "null typeRegistry");
+ this.handlers = new ConcurrentHashMap<>();
}
- public ArtifactHandler getArtifactHandler(String type) {
- ArtifactHandler handler = allHandlers.get(type);
-
- if (handler == null) {
- handler = artifactHandlers.get(type);
-
- if (handler == null) {
- handler = new DefaultArtifactHandler(type);
- } else {
- allHandlers.put(type, handler);
- }
- }
-
- return handler;
+ public ArtifactHandler getArtifactHandler(String id) {
+ return handlers.computeIfAbsent(id, k -> {
+ Type type = typeRegistry.getType(id);
+ return new DefaultArtifactHandler(
+ id,
+ type.getExtension(),
+ type.getClassifier(),
+ null,
+ null,
+ type.isIncludesDependencies(),
+ type.getLanguage(),
+ type.isAddedToClassPath()); // TODO: watch out for module path
+ });
}
public void addHandlers(Map<String, ArtifactHandler> handlers) {
// legacy support for maven-gpg-plugin:1.0
- allHandlers.putAll(handlers);
+ throw new IllegalArgumentException("you cannot do this anymore");
Review Comment:
`throw new UnsupportedOperationException("Adding handlers is not supported anymore")` ?
##########
maven-core/src/main/java/org/apache/maven/artifact/handler/manager/DefaultArtifactHandlerManager.java:
##########
@@ -22,51 +22,55 @@
import javax.inject.Named;
import javax.inject.Singleton;
+import java.util.Collections;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
+import org.apache.maven.api.Type;
+import org.apache.maven.api.services.TypeRegistry;
import org.apache.maven.artifact.handler.ArtifactHandler;
import org.apache.maven.artifact.handler.DefaultArtifactHandler;
+import static java.util.Objects.requireNonNull;
+
/**
*/
@Named
@Singleton
public class DefaultArtifactHandlerManager implements ArtifactHandlerManager {
+ private final TypeRegistry typeRegistry;
- private final Map<String, ArtifactHandler> artifactHandlers;
-
- private final Map<String, ArtifactHandler> allHandlers = new ConcurrentHashMap<>();
+ private final ConcurrentHashMap<String, ArtifactHandler> handlers;
@Inject
- public DefaultArtifactHandlerManager(Map<String, ArtifactHandler> artifactHandlers) {
- this.artifactHandlers = artifactHandlers;
+ public DefaultArtifactHandlerManager(TypeRegistry typeRegistry) {
+ this.typeRegistry = requireNonNull(typeRegistry, "null typeRegistry");
+ this.handlers = new ConcurrentHashMap<>();
}
- public ArtifactHandler getArtifactHandler(String type) {
- ArtifactHandler handler = allHandlers.get(type);
-
- if (handler == null) {
- handler = artifactHandlers.get(type);
-
- if (handler == null) {
- handler = new DefaultArtifactHandler(type);
- } else {
- allHandlers.put(type, handler);
- }
- }
-
- return handler;
+ public ArtifactHandler getArtifactHandler(String id) {
+ return handlers.computeIfAbsent(id, k -> {
+ Type type = typeRegistry.getType(id);
+ return new DefaultArtifactHandler(
+ id,
+ type.getExtension(),
+ type.getClassifier(),
+ null,
+ null,
+ type.isIncludesDependencies(),
+ type.getLanguage(),
+ type.isAddedToClassPath()); // TODO: watch out for module path
+ });
}
public void addHandlers(Map<String, ArtifactHandler> handlers) {
// legacy support for maven-gpg-plugin:1.0
- allHandlers.putAll(handlers);
+ throw new IllegalArgumentException("you cannot do this anymore");
}
@Deprecated
public Set<String> getHandlerTypes() {
- return artifactHandlers.keySet();
+ return Collections.emptySet();
Review Comment:
That looks wrong. Should we either delegate to `TypeRegistry` or throw an exception ?
##########
maven-core/src/main/java/org/apache/maven/artifact/handler/manager/LegacyArtifactHandlerManager.java:
##########
@@ -0,0 +1,72 @@
+/*
+ * 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.artifact.handler.manager;
+
+import javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+
+import org.apache.maven.artifact.handler.ArtifactHandler;
+import org.apache.maven.artifact.handler.DefaultArtifactHandler;
+
+/**
+ */
+@Named
+@Singleton
+public class LegacyArtifactHandlerManager implements ArtifactHandlerManager {
Review Comment:
Why do we need the `LegacyArtifactHandlerManager` _and_ the `DefaultArtifactHandlerManager` at the same time ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MNG-7924] Better control over and better integration with Resolver [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #1299:
URL: https://github.com/apache/maven/pull/1299#discussion_r1387788298
##########
maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/MavenRepositorySystemUtils.java:
##########
@@ -59,8 +62,37 @@ private MavenRepositorySystemUtils() {
* the session with authentication, mirror, proxy and other information required for your environment.
*
* @return The new repository system session, never {@code null}.
+ * @deprecated This method is deprecated.
*/
+ @Deprecated
public static DefaultRepositorySystemSession newSession() {
+ DefaultArtifactTypeRegistry stereotypes = new DefaultArtifactTypeRegistry();
+ stereotypes.add(new DefaultArtifactType("pom"));
Review Comment:
This method is deprecated and meant NOT to be used in Maven, only outside (like MIMA). Hence, there is no Maven API present either, "only" resolver stuff, so this is good as is 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-7924] Better control over and better integration with Resolver [maven]
Posted by "rmannibucau (via GitHub)" <gi...@apache.org>.
rmannibucau commented on PR #1299:
URL: https://github.com/apache/maven/pull/1299#issuecomment-1803703826
@cstamas I guess current state is clear but since it is not fully embraced by the community and your PR enables to align both worlds I really think we shouldnt miss this opportunity.
If even sonatype gets it wrong it is likely not very welcomed by the community IMHO.
Side note: never said you were wrong or it was not working, just that there is so much misunderstanding that making it more obvious would be welcomed.
In terms of impl it is mainly your PR + pom adjustments and be it but it makes the user facing interactions (our "ui") way more obvious I think:
* You can to change how an artifact is consumed -> change the related flag.
* You want to resolve a particular extension -> change the extension
vs
* You want the `.jar` to ignore transitive deps? Change its `type` ("Wait, I need to rebuild the dependency?")
--
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-7924] Better control over and better integration with Resolver [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #1299:
URL: https://github.com/apache/maven/pull/1299#issuecomment-1803725216
Just a thought experiment (that takari somewhat tried to deliver): Look at the `takari-maven-component` packaging. It's goal is: producing jar artifact meant to be used within Maven.
This is so expressive... what are you building? "maven-component"s or "jar with sisu index to be used in Maven".
Basically, we could distinguish even "jar with sisu index", and if user depends on that, it could (by some plugin, surefire extension example) trigger Sisu container creation that uses INDEX to pick up components from classpath, and so on and so on... These little information crumbs can serve the purpose to make things even more smarter (without being "smart smart", but simply to behave according things our user describes to us).
Also, they express intent very very clearly.
--
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-7924] Better control over and better integration with Resolver [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #1299:
URL: https://github.com/apache/maven/pull/1299#discussion_r1387827010
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Type.java:
##########
@@ -51,14 +51,21 @@ public interface Type {
* Returns the dependency type id.
* The id uniquely identifies this <i>dependency type</i>.
*
- * @return the id of this type
+ * @return the id of this type, never {@code null}.
*/
String getId();
+ /**
+ * Returns the dependency type language.
Review Comment:
https://github.com/apache/maven-resolver/blob/master/maven-resolver-api/src/main/java/org/eclipse/aether/artifact/ArtifactProperties.java#L35-L38
Will try to remove this (but Resolver ArtifactProperties could be still used to set/get this property)
--
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-7924] Better control over and better integration with Resolver [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #1299:
URL: https://github.com/apache/maven/pull/1299#discussion_r1387849231
##########
maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/MavenRepositorySystemUtils.java:
##########
@@ -59,8 +62,37 @@ private MavenRepositorySystemUtils() {
* the session with authentication, mirror, proxy and other information required for your environment.
*
* @return The new repository system session, never {@code null}.
+ * @deprecated This method is deprecated.
*/
+ @Deprecated
public static DefaultRepositorySystemSession newSession() {
+ DefaultArtifactTypeRegistry stereotypes = new DefaultArtifactTypeRegistry();
+ stereotypes.add(new DefaultArtifactType("pom"));
Review Comment:
Deprecation is probably wrong.... is "deprecated" from Maven POV as it wants to inject own TypeRegistry, but again, code integrating Maven Resolver (like MIMA) does not have new Maven API on classpath, and it will want to stick to "pure" resolver 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-7924] Better control over and better integration with Resolver [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #1299:
URL: https://github.com/apache/maven/pull/1299#discussion_r1387816705
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Type.java:
##########
@@ -51,14 +51,21 @@ public interface Type {
* Returns the dependency type id.
* The id uniquely identifies this <i>dependency type</i>.
*
- * @return the id of this type
+ * @return the id of this type, never {@code null}.
*/
String getId();
+ /**
+ * Returns the dependency type language.
Review Comment:
I think it is more like "universe", as for example the "war" case: means WebArchive in J2EE that is "Java universe". Unsure what the intent of this AH property was originally. Also, POMs are language="none" (so basically "any", as POM can be used even to produce native code as well, or think polyglot, so is not "language bound").
--
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-7924] Better control over and better integration with Resolver [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #1299:
URL: https://github.com/apache/maven/pull/1299#discussion_r1389597488
##########
maven-core/src/main/java/org/apache/maven/internal/impl/DefaultDependencyProperties.java:
##########
@@ -0,0 +1,65 @@
+/*
+ * 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.internal.impl;
+
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+
+import org.apache.maven.api.DependencyProperties;
+import org.apache.maven.api.annotations.Nonnull;
+
+import static org.apache.maven.internal.impl.Utils.nonNull;
+
+/**
+ * Default implementation of artifact properties.
+ */
+public class DefaultDependencyProperties implements DependencyProperties {
+ private final Map<String, String> properties;
+
+ public DefaultDependencyProperties(String... flags) {
+ this(Arrays.asList(flags));
Review Comment:
`nonNull(flags)` or add an `if (flags != null)` ?
##########
maven-core/src/main/java/org/apache/maven/internal/impl/DefaultDependencyProperties.java:
##########
@@ -0,0 +1,65 @@
+/*
+ * 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.internal.impl;
+
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+
+import org.apache.maven.api.DependencyProperties;
+import org.apache.maven.api.annotations.Nonnull;
+
+import static org.apache.maven.internal.impl.Utils.nonNull;
+
+/**
+ * Default implementation of artifact properties.
+ */
+public class DefaultDependencyProperties implements DependencyProperties {
+ private final Map<String, String> properties;
+
+ public DefaultDependencyProperties(String... flags) {
+ this(Arrays.asList(flags));
+ }
+
+ public DefaultDependencyProperties(Collection<String> flags) {
+ HashMap<String, String> map = new HashMap<>();
+ for (String flag : flags) {
Review Comment:
`nonNull(flags)` or add an `if (flags != null)` ?
##########
maven-core/src/main/java/org/apache/maven/internal/impl/DefaultDependencyProperties.java:
##########
@@ -0,0 +1,65 @@
+/*
+ * 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.internal.impl;
+
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+
+import org.apache.maven.api.DependencyProperties;
+import org.apache.maven.api.annotations.Nonnull;
+
+import static org.apache.maven.internal.impl.Utils.nonNull;
+
+/**
+ * Default implementation of artifact properties.
+ */
+public class DefaultDependencyProperties implements DependencyProperties {
+ private final Map<String, String> properties;
+
+ public DefaultDependencyProperties(String... flags) {
+ this(Arrays.asList(flags));
+ }
+
+ public DefaultDependencyProperties(Collection<String> flags) {
+ HashMap<String, String> map = new HashMap<>();
+ for (String flag : flags) {
+ map.put(flag, Boolean.TRUE.toString());
+ }
+ this.properties = Collections.unmodifiableMap(map);
+ }
+
+ public DefaultDependencyProperties(@Nonnull Map<String, String> properties) {
+ this.properties = nonNull(properties, "properties can not be null");
Review Comment:
`nonNull(properties, ...)`- > `Collections.unmodifiableMap(nonNull(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-7924] Better control over and better integration with Resolver [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #1299:
URL: https://github.com/apache/maven/pull/1299#discussion_r1387786371
##########
maven-core/src/main/java/org/apache/maven/artifact/handler/manager/LegacyArtifactHandlerManager.java:
##########
@@ -0,0 +1,72 @@
+/*
+ * 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.artifact.handler.manager;
+
+import javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+
+import org.apache.maven.artifact.handler.ArtifactHandler;
+import org.apache.maven.artifact.handler.DefaultArtifactHandler;
+
+/**
+ */
+@Named
+@Singleton
+public class LegacyArtifactHandlerManager implements ArtifactHandlerManager {
Review Comment:
This is a simple trick:
* legacyAHM is what "default" was so far (it have legacy AHs injected), is new, but nobody uses it as it is new thing (just new code "knows" about it)
* defaultAHM changed to delegate calls to typeRegistry
* typeRegistry uses as a fallback legacyAHM
This completes the circle:
* legacy code will refer to defaultAHM that goes to typeRegistry that will refer legacyAHM (as fallback)
* new code will refer to typeRegistry
This way new and legacy code both "see" the same thing: whether it is new AH (legacy) or Type (new) being added by extension for example
--
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-7924] Better control over and better integration with Resolver [maven]
Posted by "rmannibucau (via GitHub)" <gi...@apache.org>.
rmannibucau commented on PR #1299:
URL: https://github.com/apache/maven/pull/1299#issuecomment-1803635618
> um, is like that even today
Exactly, and I hope the refactoring enables to stop doing that, it is very misleading for end users and technically no more needed with your PR.
--
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-7924] Better control over and better integration with Resolver [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #1299:
URL: https://github.com/apache/maven/pull/1299#discussion_r1387841807
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Type.java:
##########
@@ -51,14 +51,21 @@ public interface Type {
* Returns the dependency type id.
* The id uniquely identifies this <i>dependency type</i>.
*
- * @return the id of this type
+ * @return the id of this type, never {@code null}.
*/
String getId();
+ /**
+ * Returns the dependency type language.
+ *
+ * @return the language of this type, never {@code null}.
+ */
+ String getLanguage();
Review Comment:
Removed language, keep type simplest as possible.
--
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-7924] Better control over and better integration with Resolver [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #1299:
URL: https://github.com/apache/maven/pull/1299#issuecomment-1803707342
Along those lines, yes:
Packaging provides flexibility for producers, to build new (or "old", like packaging "jar" vs "takari-jar") things.
Type provides flexibility for consumers, for example if producer builds a "bundle" (w/ required extension to make sure he is producing valid OSGi bundle), a consumer still may choose to consume it as "jar", or "bundle" (if he is also in OSGi realm), 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