You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "CrazyHZM (via GitHub)" <gi...@apache.org> on 2023/07/20 03:01:10 UTC
[GitHub] [maven] CrazyHZM opened a new pull request, #1208: [MNG-7820] Remove dependency on plexus-utils
CrazyHZM opened a new pull request, #1208:
URL: https://github.com/apache/maven/pull/1208
Following this checklist to help us incorporate your
contribution quickly and easily:
- [ ] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/MNG) filed
for the change (usually before you start working on it). Trivial changes like typos do not
require a JIRA issue. Your pull request should address just this issue, without
pulling in other changes.
- [ ] Each commit in the pull request should have a meaningful subject line and body.
- [ ] Format the pull request title like `[MNG-XXX] SUMMARY`,
where you replace `MNG-XXX` and `SUMMARY` with the appropriate JIRA issue.
- [ ] Also format the first line of the commit message like `[MNG-XXX] SUMMARY`.
Best practice is to use the JIRA issue title in both the pull request title and in the first line of the commit message.
- [ ] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [ ] Run `mvn clean verify` to make sure basic checks pass. A more thorough check will
be performed on your pull request automatically.
- [ ] You have run the [Core IT][core-its] successfully.
If your pull request is about ~20 lines of code you don't need to sign an
[Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf) if you are unsure
please ask on the developers list.
To make clear that you license your contribution under
the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
you have to acknowledge this by using the following check-box.
- [ ] I hereby declare this contribution to be licenced under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
- [ ] In any other case, please file an [Apache Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
[core-its]: https://maven.apache.org/core-its/core-it-suite/
--
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
[GitHub] [maven] gnodet commented on pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #1208:
URL: https://github.com/apache/maven/pull/1208#issuecomment-1718991448
Superseded by #1243 and #1244.
--
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
[GitHub] [maven] gnodet commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1304514826
##########
maven-compat/src/main/java/org/apache/maven/repository/legacy/DefaultWagonManager.java:
##########
@@ -465,6 +469,28 @@ public void getRemoteFile(
}
}
+ private void copyFile(File source, File destination) throws IOException {
Review Comment:
> I wouldn't assume the existing method is correct. Please use the JDK Files.copy method instead
It already uses Files.copy, so I think that's fine. If needed, it can be inlined, but I don't see the benefit.
--
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
[GitHub] [maven] CrazyHZM commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "CrazyHZM (via GitHub)" <gi...@apache.org>.
CrazyHZM commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1283443247
##########
maven-compat/src/main/java/org/apache/maven/repository/legacy/DefaultWagonManager.java:
##########
@@ -465,6 +469,28 @@ public void getRemoteFile(
}
}
+ private void copyFile(File source, File destination) throws IOException {
Review Comment:
Since this is the only dependency, @gnodet suggests inlining the method,
--
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
[GitHub] [maven] gnodet commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1305645105
##########
maven-compat/src/main/java/org/apache/maven/repository/legacy/DefaultWagonManager.java:
##########
@@ -465,6 +469,28 @@ public void getRemoteFile(
}
}
+ private void copyFile(File source, File destination) throws IOException {
+ String message;
+ if (!source.exists()) {
+ message = "File " + source + " does not exist";
+ throw new IOException(message);
+ } else if (!source.getCanonicalPath().equals(destination.getCanonicalPath())) {
+ File parentFile = destination.getParentFile();
+ if (parentFile != null && !parentFile.exists()) {
+ parentFile.mkdirs();
+ }
+ Files.copy(
+ source.toPath(),
+ destination.toPath(),
+ StandardCopyOption.REPLACE_EXISTING,
+ StandardCopyOption.COPY_ATTRIBUTES);
+ if (source.length() != destination.length()) {
Review Comment:
maven-compat is deprecated and should be gone in Maven 4.1, there's no point in enhancing the code there. Quite the opposite, any change may introduce regressions in legacy / untested code. Untested, because all the ITs have now been updated and that code path isn't used anymore, but in very plugins. If you want to modify a code that has been proven to work, please provide a unit test for that then.
Also, the following test simply exhibits that the check has an effect and is not a no-op. The documentation does in no way guarantee that the copy is atomic, it's actually specifically written that it's not. I agree there's no way simple way to ensure that in this code, and I certainly would not want to go in that direction for the reason that code is bound to be removed soon.
```
public static void main(String[] args) throws Exception {
Path dir = Files.createTempDirectory("test-");
Path source = dir.resolve("source.txt");
try (Writer w = Files.newBufferedWriter(source, StandardCharsets.UTF_8)) {
for (int i = 0; i < 1024 * 1024; i++) {
w.append("01234567890\n");
}
}
Path dest = dir.resolve("dest.txt");
for (int t = 0; t < 4; t++) {
new Thread(() -> {
try {
Files.copy(source, dest, StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.COPY_ATTRIBUTES);
if (Files.size(source) != Files.size(dest)) {
throw new IOException("Failed to copy full contents from " + source + " to " + dest);
}
} catch (IOException e) {
throw new IOError(e);
}
}).start();
}
}
```
--
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
[GitHub] [maven] gnodet commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1310368704
##########
maven-compat/src/main/java/org/apache/maven/repository/legacy/DefaultWagonManager.java:
##########
@@ -465,6 +469,28 @@ public void getRemoteFile(
}
}
+ private void copyFile(File source, File destination) throws IOException {
+ String message;
+ if (!source.exists()) {
+ message = "File " + source + " does not exist";
+ throw new IOException(message);
+ } else if (!source.getCanonicalPath().equals(destination.getCanonicalPath())) {
+ File parentFile = destination.getParentFile();
+ if (parentFile != null && !parentFile.exists()) {
+ parentFile.mkdirs();
Review Comment:
The method has been removed.
--
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
[GitHub] [maven] gnodet commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1310366044
##########
maven-compat/src/main/java/org/apache/maven/repository/legacy/DefaultWagonManager.java:
##########
@@ -465,6 +469,28 @@ public void getRemoteFile(
}
}
+ private void copyFile(File source, File destination) throws IOException {
Review Comment:
Method removed
--
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
[GitHub] [maven] elharo commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1324435432
##########
maven-core/src/main/java/org/apache/maven/plugin/ReflectionValueExtractor.java:
##########
@@ -0,0 +1,303 @@
+/*
+ * 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.plugin;
+
+import java.lang.ref.WeakReference;
+import java.lang.reflect.Array;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.util.List;
+import java.util.Map;
+import java.util.WeakHashMap;
+
+/**
+ * Extract values from an Object instance using simple dotted expressions
+ * such as <code>project.build.sourceDirectory</code>
+ * <p>
+ * The implementation supports indexed, nested and mapped properties similar to the JSP way.
+ *
+ * @see <a href=
+ * "http://struts.apache.org/1.x/struts-taglib/indexedprops.html">http://struts.apache.org/1.x/struts-taglib/indexedprops.html</a>
Review Comment:
404
##########
maven-model/src/test/java/org/apache/maven/model/v4/Xpp3DomPerfTest.java:
##########
@@ -18,22 +18,6 @@
*/
package org.apache.maven.model.v4;
-/*
Review Comment:
I checked. This is hard violation of Apache policy. From https://www.apache.org/legal/src-headers.html
Do not modify or remove any copyright notices or licenses within third-party works.
Make sure that every third-party work includes its associated license, even if that requires adding a copy of the license from the third-party download site into the distribution.
Do not add the standard Apache License header to the top of third-party source files.
--
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
[GitHub] [maven] gnodet commented on pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #1208:
URL: https://github.com/apache/maven/pull/1208#issuecomment-1692789865
> > @elharo I don't really see the need to spend much time on rewriting code copied from a utility class. The DefaultWagonManager is part of maven-compat, so deprecated and not really maintained. Any possible cleanup could introduce problems, so I'm more in favour of a blunt copy/paste, even if the code is not the cleanest, to avoid any possible behaviour difference with this legacy code.
> > For information, I've successfully run the whole IT test suite after changing the DefaultWagonManager to throw exceptions instead of actually downloading/uploading anything using a Wagon (and disabling the DefaultWagonManagerTest UT). However, it might still be in use by old plugins, so I would not delete it at this point (we do have warnings when using maven-compat since a few months afaik). But I would not spend too much time on this code either...
> > This is different for the DAG stuff which is used by maven-core...
>
> @gnodet So we should still do some optimization on the copied DAG, right?
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
[GitHub] [maven] elharo commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1274768736
##########
maven-compat/src/main/java/org/apache/maven/profiles/activation/OperatingSystemProfileActivator.java:
##########
@@ -18,17 +18,48 @@
*/
package org.apache.maven.profiles.activation;
+import java.util.Locale;
+
import org.apache.maven.model.Activation;
import org.apache.maven.model.ActivationOS;
import org.apache.maven.model.Profile;
-import org.codehaus.plexus.util.Os;
/**
* OperatingSystemProfileActivator
*/
@Deprecated
public class OperatingSystemProfileActivator implements ProfileActivator {
+ public static final String FAMILY_DOS = "dos";
Review Comment:
why are these public? especially in a deprecated class?
##########
maven-compat/src/main/java/org/apache/maven/repository/legacy/DefaultWagonManager.java:
##########
@@ -465,6 +469,28 @@ public void getRemoteFile(
}
}
+ private void copyFile(File source, File destination) throws IOException {
Review Comment:
These sorts of methods are very tricky with lots of inobvious, platform dependent edge cases that are hard to get right. I'd be much more comfortable if we could use a single method from commons-io like FileUtils.copyFile
##########
maven-core/src/main/java/org/apache/maven/utils/introspection/ReflectionValueExtractor.java:
##########
@@ -0,0 +1,325 @@
+/*
+ * 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.utils.introspection;
+
+import java.lang.ref.WeakReference;
+import java.lang.reflect.Array;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.util.List;
+import java.util.Map;
+import java.util.WeakHashMap;
+
+/**
+ * <p>
+ * Using simple dotted expressions to extract the values from an Object instance, For example we might want to extract a
+ * value like: <code>project.build.sourceDirectory</code>
+ * </p>
+ * <p>
+ * The implementation supports indexed, nested and mapped properties similar to the JSP way.
+ * </p>
+ *
+ * @author <a href="mailto:jason@maven.org">Jason van Zyl </a>
+ * @author <a href="mailto:vincent.siveton@gmail.com">Vincent Siveton</a>
+ *
+ * @see <a href=
+ * "http://struts.apache.org/1.x/struts-taglib/indexedprops.html">http://struts.apache.org/1.x/struts-taglib/indexedprops.html</a>
+ */
+public class ReflectionValueExtractor {
Review Comment:
non-public if possible
##########
maven-core/src/main/java/org/apache/maven/utils/dag/Vertex.java:
##########
@@ -0,0 +1,151 @@
+/*
Review Comment:
These classes seem to have simply been copied over from plexus. I'm no sure you can simply remove copyright like that but IANAL
##########
maven-compat/src/main/java/org/apache/maven/repository/legacy/DefaultWagonManager.java:
##########
@@ -465,6 +469,28 @@ public void getRemoteFile(
}
}
+ private void copyFile(File source, File destination) throws IOException {
+ String message;
Review Comment:
declare whrere used, and don't reuse for two different messages
##########
maven-core/src/main/java/org/apache/maven/utils/dag/CycleDetector.java:
##########
@@ -0,0 +1,136 @@
+/*
+ * 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.utils.dag;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * @author <a href="michal.maczka@dimatics.com">Michal Maczka</a>
+ *
+ */
+public class CycleDetector {
+
+ private static final Integer NOT_VISITED = 0;
+
+ private static final Integer VISITING = 1;
+
+ private static final Integer VISITED = 2;
+
+ public static List<String> hasCycle(final DAG graph) {
+ final List<Vertex> vertices = graph.getVertices();
+
+ final Map<Vertex, Integer> vertexStateMap = new HashMap<>();
+
+ List<String> retValue = null;
+
+ for (Vertex vertex : vertices) {
+ if (isNotVisited(vertex, vertexStateMap)) {
+ retValue = introducesCycle(vertex, vertexStateMap);
+
+ if (retValue != null) {
+ break;
+ }
+ }
+ }
+
+ return retValue;
+ }
+
+ /**
+ * This method will be called when an edge leading to given vertex was added and we want to check if introduction of
Review Comment:
is called
is added
i.e. present tense always in tech writing
##########
maven-core/src/main/java/org/apache/maven/utils/dag/CycleDetectedException.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.utils.dag;
+
+import java.util.Iterator;
+import java.util.List;
+
+public class CycleDetectedException extends Exception {
+ private List<String> cycle;
+
+ public CycleDetectedException(final String message, final List<String> cycle) {
+ super(message);
+
+ this.cycle = cycle;
+ }
+
+ public List<String> getCycle() {
+ return cycle;
+ }
+
+ public String cycleToString() {
+ final StringBuilder buffer = new StringBuilder();
+
+ for (Iterator<String> iterator = cycle.iterator(); iterator.hasNext(); ) {
Review Comment:
StringUtils.join from commons-lang?
##########
maven-embedder/src/main/java/org/apache/maven/cli/CLIReportingUtils.java:
##########
@@ -49,6 +50,36 @@ public final class CLIReportingUtils {
public static final String BUILD_VERSION_PROPERTY = "version";
+ public static final String FAMILY_DOS = "dos";
Review Comment:
non-public
##########
maven-core/src/main/java/org/apache/maven/utils/dag/CycleDetector.java:
##########
@@ -0,0 +1,136 @@
+/*
+ * 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.utils.dag;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * @author <a href="michal.maczka@dimatics.com">Michal Maczka</a>
+ *
+ */
+public class CycleDetector {
+
+ private static final Integer NOT_VISITED = 0;
+
+ private static final Integer VISITING = 1;
+
+ private static final Integer VISITED = 2;
+
+ public static List<String> hasCycle(final DAG graph) {
+ final List<Vertex> vertices = graph.getVertices();
+
+ final Map<Vertex, Integer> vertexStateMap = new HashMap<>();
+
+ List<String> retValue = null;
+
+ for (Vertex vertex : vertices) {
+ if (isNotVisited(vertex, vertexStateMap)) {
+ retValue = introducesCycle(vertex, vertexStateMap);
+
+ if (retValue != null) {
+ break;
+ }
+ }
+ }
+
+ return retValue;
+ }
+
+ /**
+ * This method will be called when an edge leading to given vertex was added and we want to check if introduction of
+ * this edge has not resulted in apparition of cycle in the graph
+ *
+ * @param vertex the vertex
+ * @param vertexStateMap the vertex Map
+ * @return the found cycle
+ */
+ public static List<String> introducesCycle(final Vertex vertex, final Map<Vertex, Integer> vertexStateMap) {
Review Comment:
does this need to be public?
##########
maven-core/src/main/java/org/apache/maven/utils/dag/DAG.java:
##########
@@ -0,0 +1,239 @@
+/*
+ * 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.utils.dag;
+
+import java.io.Serializable;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * DAG = Directed Acyclic Graph
+ *
+ * @author <a href="michal.maczka@dimatics.com">Michal Maczka</a>
+ *
+ * TODO this class should be renamed from DAG to Dag
+ */
+public class DAG implements Cloneable, Serializable {
Review Comment:
does this class really need to be public?
--
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
[GitHub] [maven] gnodet commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1311737481
##########
maven-core/src/main/java/org/apache/maven/utils/dag/Vertex.java:
##########
@@ -0,0 +1,151 @@
+/*
Review Comment:
We can keep the codehaus header below the package I think.
--
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
[GitHub] [maven] elharo commented on pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on PR #1208:
URL: https://github.com/apache/maven/pull/1208#issuecomment-1707483578
Certainly it can be done in another PR. Take this one piece at a time. There's no rule that says you have to fix every class at once. It's a big dependency and it's OK to break it up into chunks. Indeed it's preferable:
https://google.github.io/eng-practices/review/developer/small-cls.html
--
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
[GitHub] [maven] gnodet commented on pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #1208:
URL: https://github.com/apache/maven/pull/1208#issuecomment-1707329632
@elharo we can't refactor the `Os` class in a different PR unless we merge it first, as the PR removes the plexus-utils dependency... I'll refactor the class completely.
--
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
[GitHub] [maven] CrazyHZM commented on pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "CrazyHZM (via GitHub)" <gi...@apache.org>.
CrazyHZM commented on PR #1208:
URL: https://github.com/apache/maven/pull/1208#issuecomment-1707614613
> > @elharo we can't refactor the `Os` class in a different PR unless we merge it first, as the PR removes the plexus-utils dependency... I'll refactor the class completely.
>
> Done. However, the logic should not be changed in this PR. If you want to update the behaviour, we should create a ticket for that and will have to modify the main doc at https://maven.apache.org/enforcer/enforcer-rules/requireOS.html, or rather at https://maven.apache.org/guides/introduction/introduction-to-profiles.html#os
@gnodet @elharo We can create a Jira to record this.
--
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
[GitHub] [maven] gnodet commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1323448246
##########
maven-core/src/main/java/org/apache/maven/utils/introspection/ReflectionValueExtractor.java:
##########
@@ -0,0 +1,325 @@
+/*
Review Comment:
I've explained those earlier. They come from Apache, so I think it's safe to remove the headers on the 3 files from this package. That's what I did in a more recent commit.
--
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
[GitHub] [maven] gnodet commented on pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #1208:
URL: https://github.com/apache/maven/pull/1208#issuecomment-1713389394
@elharo could you take another look ?
--
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
[GitHub] [maven] gnodet commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1323636892
##########
maven-core/src/main/java/org/apache/maven/utils/introspection/ReflectionValueExtractor.java:
##########
@@ -0,0 +1,325 @@
+/*
+ * 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.utils.introspection;
+
+import java.lang.ref.WeakReference;
+import java.lang.reflect.Array;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.util.List;
+import java.util.Map;
+import java.util.WeakHashMap;
+
+/**
+ * <p>
+ * Using simple dotted expressions to extract the values from an Object instance, For example we might want to extract a
+ * value like: <code>project.build.sourceDirectory</code>
+ * </p>
+ * <p>
+ * The implementation supports indexed, nested and mapped properties similar to the JSP way.
+ * </p>
+ *
+ * @author <a href="mailto:jason@maven.org">Jason van Zyl </a>
+ * @author <a href="mailto:vincent.siveton@gmail.com">Vincent Siveton</a>
+ *
+ * @see <a href=
+ * "http://struts.apache.org/1.x/struts-taglib/indexedprops.html">http://struts.apache.org/1.x/struts-taglib/indexedprops.html</a>
+ */
+public class ReflectionValueExtractor {
+ private static final Class<?>[] CLASS_ARGS = new Class[0];
+
+ private static final Object[] OBJECT_ARGS = new Object[0];
+
+ /**
+ * Use a WeakHashMap here, so the keys (Class objects) can be garbage collected. This approach prevents permgen
+ * space overflows due to retention of discarded classloaders.
+ */
+ private static final Map<Class<?>, WeakReference<ClassMap>> CLASS_MAPS = new WeakHashMap<>();
+
+ static final int EOF = -1;
Review Comment:
Fixed
--
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
[GitHub] [maven] gnodet commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1321446465
##########
maven-model-builder/src/main/java/org/apache/maven/utils/Os.java:
##########
@@ -0,0 +1,213 @@
+/*
+ * 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.utils;
+
+import java.util.Locale;
+import java.util.stream.Stream;
+
+/**
+ * OS support
+ */
+public class Os {
+
+ /**
+ * The OS Name.
+ */
+ public static final String OS_NAME = System.getProperty("os.name").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * The OA architecture.
+ */
+ public static final String OS_ARCH = System.getProperty("os.arch").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * The OS version.
+ */
+ public static final String OS_VERSION = System.getProperty("os.version").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * OS Family
+ */
+ public static final String OS_FAMILY;
+
+ public static final boolean IS_WINDOWS;
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_WINDOWS = "windows";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_WIN9X = "win9x";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_NT = "winnt";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_OS2 = "os/2";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_NETWARE = "netware";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_DOS = "dos";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_MAC = "mac";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_TANDEM = "tandem";
Review Comment:
I disagree, even if I have no idea what `tandem` is, I don't want this PR to affect the behaviour.
##########
maven-model-builder/src/main/java/org/apache/maven/utils/Os.java:
##########
@@ -0,0 +1,213 @@
+/*
+ * 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.utils;
+
+import java.util.Locale;
+import java.util.stream.Stream;
+
+/**
+ * OS support
+ */
+public class Os {
+
+ /**
+ * The OS Name.
+ */
+ public static final String OS_NAME = System.getProperty("os.name").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * The OA architecture.
+ */
+ public static final String OS_ARCH = System.getProperty("os.arch").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * The OS version.
+ */
+ public static final String OS_VERSION = System.getProperty("os.version").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * OS Family
+ */
+ public static final String OS_FAMILY;
+
+ public static final boolean IS_WINDOWS;
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_WINDOWS = "windows";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_WIN9X = "win9x";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_NT = "winnt";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_OS2 = "os/2";
Review Comment:
same
--
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
[GitHub] [maven] gnodet commented on pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #1208:
URL: https://github.com/apache/maven/pull/1208#issuecomment-1714387479
@elharo I've rewritten the `dag` related classes with non public classes. The other copyrighted class originate from within the ASF and the last one was written by me.
--
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
[GitHub] [maven] gnodet commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1324600451
##########
maven-model/src/test/java/org/apache/maven/model/v4/Xpp3DomPerfTest.java:
##########
@@ -18,22 +18,6 @@
*/
package org.apache.maven.model.v4;
-/*
Review Comment:
I am the author and owner of this file, so I gave it to Codehaus, but now I'm giving it to the ASF.
So the pointer you gave does not apply 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
[GitHub] [maven] gnodet commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1305645835
##########
maven-compat/src/main/java/org/apache/maven/repository/legacy/DefaultWagonManager.java:
##########
@@ -465,6 +469,28 @@ public void getRemoteFile(
}
}
+ private void copyFile(File source, File destination) throws IOException {
+ String message;
+ if (!source.exists()) {
+ message = "File " + source + " does not exist";
+ throw new IOException(message);
+ } else if (!source.getCanonicalPath().equals(destination.getCanonicalPath())) {
+ File parentFile = destination.getParentFile();
+ if (parentFile != null && !parentFile.exists()) {
+ parentFile.mkdirs();
Review Comment:
Changing the behaviour of legacy / untested code is not a good idea. AND maven-compat is bound to be deleted in 4.1
--
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
[GitHub] [maven] CrazyHZM commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "CrazyHZM (via GitHub)" <gi...@apache.org>.
CrazyHZM commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1305099150
##########
maven-core/src/main/java/org/apache/maven/utils/dag/CycleDetector.java:
##########
@@ -0,0 +1,136 @@
+/*
+ * 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.utils.dag;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * @author <a href="michal.maczka@dimatics.com">Michal Maczka</a>
+ *
+ */
+public class CycleDetector {
+
+ private static final Integer NOT_VISITED = 0;
+
+ private static final Integer VISITING = 1;
+
+ private static final Integer VISITED = 2;
+
+ public static List<String> hasCycle(final DAG graph) {
+ final List<Vertex> vertices = graph.getVertices();
+
+ final Map<Vertex, Integer> vertexStateMap = new HashMap<>();
+
+ List<String> retValue = null;
+
+ for (Vertex vertex : vertices) {
+ if (isNotVisited(vertex, vertexStateMap)) {
+ retValue = introducesCycle(vertex, vertexStateMap);
+
+ if (retValue != null) {
+ break;
+ }
+ }
+ }
+
+ return retValue;
+ }
+
+ /**
+ * This method will be called when an edge leading to given vertex was added and we want to check if introduction of
+ * this edge has not resulted in apparition of cycle in the graph
+ *
+ * @param vertex the vertex
+ * @param vertexStateMap the vertex Map
+ * @return the found cycle
+ */
+ public static List<String> introducesCycle(final Vertex vertex, final Map<Vertex, Integer> vertexStateMap) {
Review Comment:
change to `private`.
--
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
[GitHub] [maven] CrazyHZM commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "CrazyHZM (via GitHub)" <gi...@apache.org>.
CrazyHZM commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1306681408
##########
maven-compat/src/main/java/org/apache/maven/repository/legacy/DefaultWagonManager.java:
##########
@@ -465,6 +469,28 @@ public void getRemoteFile(
}
}
+ private void copyFile(File source, File destination) throws IOException {
+ String message;
+ if (!source.exists()) {
+ message = "File " + source + " does not exist";
+ throw new IOException(message);
+ } else if (!source.getCanonicalPath().equals(destination.getCanonicalPath())) {
+ File parentFile = destination.getParentFile();
+ if (parentFile != null && !parentFile.exists()) {
+ parentFile.mkdirs();
+ }
+ Files.copy(
+ source.toPath(),
+ destination.toPath(),
+ StandardCopyOption.REPLACE_EXISTING,
+ StandardCopyOption.COPY_ATTRIBUTES);
+ if (source.length() != destination.length()) {
Review Comment:
@gnodet
I find the copyFile method of maven-shared-utils deprecated, I think we don't need to use it anymore.
--
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
[GitHub] [maven] elharo commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1307372298
##########
maven-compat/src/main/java/org/apache/maven/repository/legacy/DefaultWagonManager.java:
##########
@@ -465,6 +469,28 @@ public void getRemoteFile(
}
}
+ private void copyFile(File source, File destination) throws IOException {
+ String message;
+ if (!source.exists()) {
+ message = "File " + source + " does not exist";
+ throw new IOException(message);
+ } else if (!source.getCanonicalPath().equals(destination.getCanonicalPath())) {
+ File parentFile = destination.getParentFile();
+ if (parentFile != null && !parentFile.exists()) {
+ parentFile.mkdirs();
Review Comment:
The existing code is buggy and can fail. There is no guarantee mkdirs succeeds here.
##########
maven-compat/src/main/java/org/apache/maven/repository/legacy/DefaultWagonManager.java:
##########
@@ -465,6 +469,28 @@ public void getRemoteFile(
}
}
+ private void copyFile(File source, File destination) throws IOException {
+ String message;
+ if (!source.exists()) {
+ message = "File " + source + " does not exist";
+ throw new IOException(message);
+ } else if (!source.getCanonicalPath().equals(destination.getCanonicalPath())) {
+ File parentFile = destination.getParentFile();
+ if (parentFile != null && !parentFile.exists()) {
+ parentFile.mkdirs();
+ }
+ Files.copy(
+ source.toPath(),
+ destination.toPath(),
+ StandardCopyOption.REPLACE_EXISTING,
+ StandardCopyOption.COPY_ATTRIBUTES);
+ if (source.length() != destination.length()) {
Review Comment:
There's some shared history between all of these, and they have the same failure modes and issues. That's why we need to move away from all of them and use the JDK methods that are available in Java 7+ that did not exist 20 years ago when this buggy code was first written.
--
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
[GitHub] [maven] CrazyHZM commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "CrazyHZM (via GitHub)" <gi...@apache.org>.
CrazyHZM commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1312503644
##########
maven-core/src/main/java/org/apache/maven/utils/dag/Vertex.java:
##########
@@ -0,0 +1,151 @@
+/*
Review Comment:
@gnodet
Done.
--
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
[GitHub] [maven] gnodet commented on pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #1208:
URL: https://github.com/apache/maven/pull/1208#issuecomment-1707052868
@elharo any more concerns ?
--
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
[GitHub] [maven] elharo commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1304137698
##########
maven-compat/src/main/java/org/apache/maven/repository/legacy/DefaultWagonManager.java:
##########
@@ -465,6 +469,28 @@ public void getRemoteFile(
}
}
+ private void copyFile(File source, File destination) throws IOException {
Review Comment:
I wouldn't assume the existing method is correct. Please use the JDK Files.copy method instead
--
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
[GitHub] [maven] gnodet commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1321445365
##########
maven-model-builder/src/main/java/org/apache/maven/utils/Os.java:
##########
@@ -0,0 +1,213 @@
+/*
+ * 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.utils;
+
+import java.util.Locale;
+import java.util.stream.Stream;
+
+/**
+ * OS support
+ */
+public class Os {
+
+ /**
+ * The OS Name.
+ */
+ public static final String OS_NAME = System.getProperty("os.name").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * The OA architecture.
+ */
+ public static final String OS_ARCH = System.getProperty("os.arch").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * The OS version.
+ */
+ public static final String OS_VERSION = System.getProperty("os.version").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * OS Family
+ */
+ public static final String OS_FAMILY;
+
+ public static final boolean IS_WINDOWS;
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_WINDOWS = "windows";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_WIN9X = "win9x";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_NT = "winnt";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_OS2 = "os/2";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_NETWARE = "netware";
Review Comment:
They are documented, this PR is not about changing the current behaviour. Trimming down the list of supported OS should be a separate JIRA issue / 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
[GitHub] [maven] gnodet closed pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet closed pull request #1208: [MNG-7820] Remove dependency on plexus-utils
URL: https://github.com/apache/maven/pull/1208
--
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
[GitHub] [maven] gnodet commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1268959818
##########
src/mdo/reader-ex.vm:
##########
@@ -124,7 +123,7 @@ public class ${className} {
* @return ${root.name}
*/
public ${root.name} read(InputStream in, boolean strict, InputSource source) throws IOException, XmlPullParserException {
- return read(ReaderFactory.newXmlReader(in), strict, source);
+ return read(new XmlStreamReader(in), strict, source);
Review Comment:
Same here
##########
src/mdo/reader-modified.vm:
##########
@@ -135,7 +133,7 @@ public class ${className} {
* @return ${root.name}
*/
public ${root.name} read(InputStream in, boolean strict) throws IOException, XmlPullParserException {
- return read(ReaderFactory.newXmlReader(in), strict);
+ return read(new XmlStreamReader(in), strict);
Review Comment:
...here...
##########
maven-settings-builder/pom.xml:
##########
@@ -51,17 +51,19 @@ under the License.
<groupId>org.codehaus.plexus</groupId>
<artifactId>plexus-interpolation</artifactId>
</dependency>
- <dependency>
- <groupId>org.codehaus.plexus</groupId>
- <artifactId>plexus-utils</artifactId>
- </dependency>
<dependency>
<groupId>org.apache.maven</groupId>
<artifactId>maven-settings</artifactId>
</dependency>
<dependency>
<groupId>org.codehaus.plexus</groupId>
<artifactId>plexus-sec-dispatcher</artifactId>
+ <exclusions>
Review Comment:
The latest code depends on plexus-xml, not plexus-utils.
##########
src/mdo/reader-modified.vm:
##########
@@ -148,7 +146,7 @@ public class ${className} {
* @return ${root.name}
*/
public ${root.name} read(InputStream in) throws IOException, XmlPullParserException {
- return read(ReaderFactory.newXmlReader(in));
+ return read(new XmlStreamReader(in));
Review Comment:
here...
##########
src/mdo/reader-ex.vm:
##########
@@ -57,7 +57,7 @@ import ${packageModelV4}.InputLocation;
#foreach ( $class in $model.allClasses )
import ${packageModelV4}.${class.name};
#end
-import org.codehaus.plexus.util.ReaderFactory;
+import org.codehaus.plexus.util.xml.XmlStreamReader;
Review Comment:
We should avoid direct usage of XmlStreamReader and let the parser to its job, passing either the `InputStream` or `Reader`, but not wrapping the object.
##########
src/mdo/reader.vm:
##########
@@ -131,7 +129,7 @@ public class ${className} {
* @return ${root.name}
*/
public ${root.name} read(InputStream in) throws IOException, XmlPullParserException {
- return read(ReaderFactory.newXmlReader(in));
+ return read(new XmlStreamReader(in));
Review Comment:
here...
##########
maven-compat/src/main/java/org/apache/maven/repository/legacy/DefaultWagonManager.java:
##########
@@ -25,13 +25,18 @@
import java.io.File;
import java.io.IOException;
import java.lang.reflect.Method;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.nio.file.StandardOpenOption;
import java.security.NoSuchAlgorithmException;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Properties;
+import org.apache.commons.io.FileUtils;
Review Comment:
Can we inline the calls to `FileUtils` to not add a dependency to _commons-io_ ?
##########
maven-compat/pom.xml:
##########
@@ -93,6 +93,11 @@ under the License.
<artifactId>wagon-provider-api</artifactId>
</dependency>
+ <dependency>
+ <groupId>commons-io</groupId>
+ <artifactId>commons-io</artifactId>
+ </dependency>
Review Comment:
I think it should be possible to inline the references to commons-io so that we actually trim down the list of dependencies and not add a new one...
##########
src/mdo/reader.vm:
##########
@@ -118,7 +116,7 @@ public class ${className} {
* @return ${root.name}
*/
public ${root.name} read(InputStream in, boolean strict) throws IOException, XmlPullParserException {
- return read(ReaderFactory.newXmlReader(in), strict);
+ return read(new XmlStreamReader(in), strict);
Review Comment:
here...
##########
src/mdo/writer.vm:
##########
@@ -58,7 +58,7 @@ import org.apache.maven.internal.xml.XmlNodeBuilder;
#foreach ( $class in $model.allClasses )
import ${packageModelV4}.${class.name};
#end
-import org.codehaus.plexus.util.ReaderFactory;
+import org.codehaus.plexus.util.xml.XmlStreamReader;
Review Comment:
This import does not seem to be used, to just remove 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
[GitHub] [maven] gnodet commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1283531940
##########
maven-core/src/main/java/org/apache/maven/utils/dag/DAG.java:
##########
@@ -0,0 +1,239 @@
+/*
+ * 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.utils.dag;
+
+import java.io.Serializable;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * DAG = Directed Acyclic Graph
+ *
+ * @author <a href="michal.maczka@dimatics.com">Michal Maczka</a>
+ *
+ * TODO this class should be renamed from DAG to Dag
+ */
+public class DAG implements Cloneable, Serializable {
Review Comment:
Yes, it's used in `org.apache.maven.project.ProjectSorter`
--
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
[GitHub] [maven] CrazyHZM commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "CrazyHZM (via GitHub)" <gi...@apache.org>.
CrazyHZM commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1312485220
##########
maven-core/src/main/java/org/apache/maven/utils/dag/CycleDetectedException.java:
##########
@@ -0,0 +1,40 @@
+/*
+ * 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.utils.dag;
Review Comment:
DAG is required after removing plexus-utils.
--
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
[GitHub] [maven] gnodet commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1306630132
##########
maven-compat/src/main/java/org/apache/maven/repository/legacy/DefaultWagonManager.java:
##########
@@ -465,6 +469,28 @@ public void getRemoteFile(
}
}
+ private void copyFile(File source, File destination) throws IOException {
+ String message;
+ if (!source.exists()) {
+ message = "File " + source + " does not exist";
+ throw new IOException(message);
+ } else if (!source.getCanonicalPath().equals(destination.getCanonicalPath())) {
+ File parentFile = destination.getParentFile();
+ if (parentFile != null && !parentFile.exists()) {
+ parentFile.mkdirs();
+ }
+ Files.copy(
+ source.toPath(),
+ destination.toPath(),
+ StandardCopyOption.REPLACE_EXISTING,
+ StandardCopyOption.COPY_ATTRIBUTES);
+ if (source.length() != destination.length()) {
Review Comment:
Fwiw, maven-shared-utils' version of it also has the same check: https://github.com/apache/maven-shared-utils/blob/master/src/main/java/org/apache/maven/shared/utils/io/FileUtils.java#L760
--
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
[GitHub] [maven] elharo commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1310423407
##########
maven-core/src/main/java/org/apache/maven/utils/dag/DAG.java:
##########
@@ -0,0 +1,239 @@
+/*
+ * 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.utils.dag;
+
+import java.io.Serializable;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * DAG = Directed Acyclic Graph
+ *
+ * @author <a href="michal.maczka@dimatics.com">Michal Maczka</a>
+ *
+ * TODO this class should be renamed from DAG to Dag
+ */
+public class DAG implements Cloneable, Serializable {
+ // ------------------------------------------------------------
+ // Fields
+ // ------------------------------------------------------------
+ /**
+ * Nodes will be kept in two data structures at the same time for faster processing
+ */
+ /**
+ * Maps vertex's label to vertex
+ */
+ private Map<String, Vertex> vertexMap = new HashMap<>();
+
+ /**
+ * Conatin list of all vertices
+ */
+ private List<Vertex> vertexList = new ArrayList<>();
+
+ // ------------------------------------------------------------
+ // Constructors
+ // ------------------------------------------------------------
+
+ /**
+ *
+ */
+ public DAG() {
+ super();
+ }
+
+ // ------------------------------------------------------------
+ // Accessors
+ // ------------------------------------------------------------
+
+ /**
+ * @return the vertices
+ */
+ public List<Vertex> getVertices() {
+ return vertexList;
+ }
+
+ /**
+ * @deprecated instead use {@link #getVertices()}
+ * @return the vertices
+ */
+ @Deprecated
+ public List<Vertex> getVerticies() {
+ return getVertices();
+ }
+
+ public Set<String> getLabels() {
+ return vertexMap.keySet();
+ }
+
+ // ------------------------------------------------------------
+ // Implementation
+ // ------------------------------------------------------------
+
+ /**
+ * Adds vertex to DAG. If vertex of given label already exist in DAG no vertex is added
+ *
+ * @param label The label of the Vertex
+ * @return New vertex if vertex of given label was not present in the DAG or existing vertex if vertex of given
+ * label was already added to DAG
+ */
+ public Vertex addVertex(final String label) {
+ Vertex retValue = null;
+
+ // check if vertex is already in DAG
+ if (vertexMap.containsKey(label)) {
+ retValue = vertexMap.get(label);
+ } else {
+ retValue = new Vertex(label);
+
+ vertexMap.put(label, retValue);
+
+ vertexList.add(retValue);
+ }
+
+ return retValue;
+ }
+
+ public void addEdge(final String from, final String to) throws CycleDetectedException {
+ final Vertex v1 = addVertex(from);
+
+ final Vertex v2 = addVertex(to);
+
+ addEdge(v1, v2);
+ }
+
+ public void addEdge(final Vertex from, final Vertex to) throws CycleDetectedException {
+
+ from.addEdgeTo(to);
+
+ to.addEdgeFrom(from);
+
+ final List<String> cycle = CycleDetector.introducesCycle(to);
+
+ if (cycle != null) {
+ // remove edge which introduced cycle
+
+ removeEdge(from, to);
+
+ final String msg = "Edge between '" + from + "' and '" + to + "' introduces to cycle in the graph";
+
+ throw new CycleDetectedException(msg, cycle);
+ }
+ }
+
+ public void removeEdge(final String from, final String to) {
+ final Vertex v1 = addVertex(from);
+
+ final Vertex v2 = addVertex(to);
+
+ removeEdge(v1, v2);
+ }
+
+ public void removeEdge(final Vertex from, final Vertex to) {
+ from.removeEdgeTo(to);
+
+ to.removeEdgeFrom(from);
+ }
+
+ public Vertex getVertex(final String label) {
+ final Vertex retValue = vertexMap.get(label);
+
+ return retValue;
+ }
+
+ public boolean hasEdge(final String label1, final String label2) {
+ final Vertex v1 = getVertex(label1);
+
+ final Vertex v2 = getVertex(label2);
+
+ final boolean retValue = v1.getChildren().contains(v2);
+
+ return retValue;
+ }
+
+ /**
+ * @param label see name
+ * @return the childs
+ */
+ public List<String> getChildLabels(final String label) {
+ final Vertex vertex = getVertex(label);
+
+ return vertex.getChildLabels();
+ }
+
+ /**
+ * @param label see name
+ * @return the parents
+ */
+ public List<String> getParentLabels(final String label) {
+ final Vertex vertex = getVertex(label);
+
+ return vertex.getParentLabels();
+ }
+
+ /**
+ * @see java.lang.Object#clone()
+ */
+ @Override
+ public Object clone() throws CloneNotSupportedException {
+ // this is what's failing..
+ final Object retValue = super.clone();
+
+ return retValue;
+ }
+
+ /**
+ * Indicates if there is at least one edge leading to or from vertex of given label
+ * @param label the label
+ * @return <code>true</code> if this vertex is connected with other vertex,<code>false</code> otherwise
+ */
+ public boolean isConnected(final String label) {
+ final Vertex vertex = getVertex(label);
+
+ final boolean retValue = vertex.isConnected();
+
+ return retValue;
+ }
+
+ /**
+ * Return the list of labels of successor in order decided by topological sort
+ *
+ * @param label The label of the vertex whose predecessors are searched
+ * @return The list of labels. Returned list contains also the label passed as parameter to this method. This label
Review Comment:
lower case, per Oracle guidelines for javadoc
--
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
[GitHub] [maven] gnodet commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1323636571
##########
maven-core/src/main/java/org/apache/maven/utils/introspection/ReflectionValueExtractor.java:
##########
@@ -0,0 +1,325 @@
+/*
+ * 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.utils.introspection;
+
+import java.lang.ref.WeakReference;
+import java.lang.reflect.Array;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.util.List;
+import java.util.Map;
+import java.util.WeakHashMap;
+
+/**
+ * <p>
+ * Using simple dotted expressions to extract the values from an Object instance, For example we might want to extract a
+ * value like: <code>project.build.sourceDirectory</code>
+ * </p>
+ * <p>
+ * The implementation supports indexed, nested and mapped properties similar to the JSP way.
+ * </p>
+ *
+ * @author <a href="mailto:jason@maven.org">Jason van Zyl </a>
+ * @author <a href="mailto:vincent.siveton@gmail.com">Vincent Siveton</a>
+ *
+ * @see <a href=
+ * "http://struts.apache.org/1.x/struts-taglib/indexedprops.html">http://struts.apache.org/1.x/struts-taglib/indexedprops.html</a>
+ */
+public class ReflectionValueExtractor {
+ private static final Class<?>[] CLASS_ARGS = new Class[0];
+
+ private static final Object[] OBJECT_ARGS = new Object[0];
+
+ /**
+ * Use a WeakHashMap here, so the keys (Class objects) can be garbage collected. This approach prevents permgen
+ * space overflows due to retention of discarded classloaders.
+ */
+ private static final Map<Class<?>, WeakReference<ClassMap>> CLASS_MAPS = new WeakHashMap<>();
+
+ static final int EOF = -1;
+
+ static final char PROPERTY_START = '.';
+
+ static final char INDEXED_START = '[';
+
+ static final char INDEXED_END = ']';
+
+ static final char MAPPED_START = '(';
+
+ static final char MAPPED_END = ')';
+
+ static class Tokenizer {
+ final String expression;
+
+ int idx;
+
+ Tokenizer(String expression) {
+ this.expression = expression;
+ }
+
+ public int peekChar() {
+ return idx < expression.length() ? expression.charAt(idx) : EOF;
+ }
+
+ public int skipChar() {
+ return idx < expression.length() ? expression.charAt(idx++) : EOF;
+ }
+
+ public String nextToken(char delimiter) {
+ int start = idx;
+
+ while (idx < expression.length() && delimiter != expression.charAt(idx)) {
+ idx++;
+ }
+
+ // delimiter MUST be present
+ if (idx <= start || idx >= expression.length()) {
+ return null;
+ }
+
+ return expression.substring(start, idx++);
+ }
+
+ public String nextPropertyName() {
+ final int start = idx;
+
+ while (idx < expression.length() && Character.isJavaIdentifierPart(expression.charAt(idx))) {
+ idx++;
+ }
+
+ // property name does not require delimiter
+ if (idx <= start || idx > expression.length()) {
+ return null;
+ }
+
+ return expression.substring(start, idx);
+ }
+
+ public int getPosition() {
+ return idx < expression.length() ? idx : EOF;
+ }
+
+ // to make tokenizer look pretty in debugger
+ @Override
+ public String toString() {
+ return idx < expression.length() ? expression.substring(idx) : "<EOF>";
+ }
+ }
+
+ private ReflectionValueExtractor() {}
+
+ /**
+ * <p>The implementation supports indexed, nested and mapped properties.</p>
+ *
+ * <ul>
+ * <li>nested properties should be defined by a dot, i.e. "user.address.street"</li>
+ * <li>indexed properties (java.util.List or array instance) should be contains <code>(\\w+)\\[(\\d+)\\]</code>
+ * pattern, i.e. "user.addresses[1].street"</li>
+ * <li>mapped properties should be contains <code>(\\w+)\\((.+)\\)</code> pattern, i.e.
+ * "user.addresses(myAddress).street"</li>
+ * </ul>
+ *
+ * @param expression not null expression
+ * @param root not null object
+ * @return the object defined by the expression
+ */
+ public static Object evaluate(String expression, Object root) {
+ return evaluate(expression, root, true);
+ }
+
+ /**
+ * <p>The implementation supports indexed, nested and mapped properties.</p>
+ *
+ * <ul>
+ * <li>nested properties should be defined by a dot, i.e. "user.address.street"</li>
+ * <li>indexed properties (java.util.List or array instance) should be contains <code>(\\w+)\\[(\\d+)\\]</code>
+ * pattern, i.e. "user.addresses[1].street"</li>
+ * <li>mapped properties should be contains <code>(\\w+)\\((.+)\\)</code> pattern, i.e.
+ * "user.addresses(myAddress).street"</li>
+ * </ul>
+ *
+ * @param expression not null expression
+ * @param root not null object
+ * @param trimRootToken root start
+ * @return the object defined by the expression
+ */
+ public static Object evaluate(String expression, final Object root, final boolean trimRootToken) {
+ Object value = root;
+
+ // ----------------------------------------------------------------------
+ // Walk the dots and retrieve the ultimate value desired from the
+ // MavenProject instance.
+ // ----------------------------------------------------------------------
+
+ if (expression == null || expression.isEmpty() || !Character.isJavaIdentifierStart(expression.charAt(0))) {
+ return null;
+ }
+
+ boolean hasDots = expression.indexOf(PROPERTY_START) >= 0;
+
+ final Tokenizer tokenizer;
+ if (trimRootToken && hasDots) {
+ tokenizer = new Tokenizer(expression);
+ tokenizer.nextPropertyName();
+ if (tokenizer.getPosition() == EOF) {
+ return null;
+ }
+ } else {
+ tokenizer = new Tokenizer("." + expression);
+ }
+
+ int propertyPosition = tokenizer.getPosition();
+ while (value != null && tokenizer.peekChar() != EOF) {
+ switch (tokenizer.skipChar()) {
+ case INDEXED_START:
+ value = getIndexedValue(
+ expression,
+ propertyPosition,
+ tokenizer.getPosition(),
+ value,
+ tokenizer.nextToken(INDEXED_END));
+ break;
+ case MAPPED_START:
+ value = getMappedValue(
+ expression,
+ propertyPosition,
+ tokenizer.getPosition(),
+ value,
+ tokenizer.nextToken(MAPPED_END));
+ break;
+ case PROPERTY_START:
+ propertyPosition = tokenizer.getPosition();
+ value = getPropertyValue(value, tokenizer.nextPropertyName());
+ break;
+ default:
+ // could not parse expression
+ return null;
+ }
+ }
+
+ return value;
+ }
+
+ private static Object getMappedValue(
+ final String expression, final int from, final int to, final Object value, final String key) {
+ if (value == null || key == null) {
+ return null;
+ }
+ if (value instanceof Map) {
+ try {
+ Object[] localParams = new Object[] {key};
+ ClassMap classMap = getClassMap(value.getClass());
+ Method method = classMap.findMethod("get", localParams);
+ return method.invoke(value, localParams);
+ } catch (Exception e) {
+ throw new RuntimeException(e);
+ }
+ } else {
+ final String message = String.format(
+ "The token '%s' at position '%d' refers to a java.util.Map, but the value seems is an instance of '%s'",
+ expression.subSequence(from, to), from, value.getClass());
+ throw new IllegalArgumentException(message);
+ }
+ }
+
+ private static Object getIndexedValue(
+ final String expression, final int from, final int to, final Object value, final String indexStr) {
+ int index;
+ try {
+ index = Integer.parseInt(indexStr);
+
+ if (value.getClass().isArray()) {
+ return Array.get(value, index);
+ }
+
+ } catch (NumberFormatException e) {
+ final String message = String.format("The indexStr '%s' is not a number", indexStr);
+ throw new IllegalArgumentException(message);
+ }
+
+ if (value instanceof List) {
+ try {
+ ClassMap classMap = getClassMap(value.getClass());
+ // use get method on List interface
+ Object[] localParams = new Object[] {index};
+ Method method = classMap.findMethod("get", localParams);
+ return method.invoke(value, localParams);
+ } catch (Exception e) {
+ throw new RuntimeException(e);
Review Comment:
The original code was returning null. I've added tests and reverted this code change.
##########
maven-core/src/main/java/org/apache/maven/utils/introspection/ReflectionValueExtractor.java:
##########
@@ -0,0 +1,325 @@
+/*
+ * 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.utils.introspection;
+
+import java.lang.ref.WeakReference;
+import java.lang.reflect.Array;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.util.List;
+import java.util.Map;
+import java.util.WeakHashMap;
+
+/**
+ * <p>
+ * Using simple dotted expressions to extract the values from an Object instance, For example we might want to extract a
+ * value like: <code>project.build.sourceDirectory</code>
+ * </p>
+ * <p>
+ * The implementation supports indexed, nested and mapped properties similar to the JSP way.
+ * </p>
+ *
+ * @author <a href="mailto:jason@maven.org">Jason van Zyl </a>
+ * @author <a href="mailto:vincent.siveton@gmail.com">Vincent Siveton</a>
+ *
+ * @see <a href=
+ * "http://struts.apache.org/1.x/struts-taglib/indexedprops.html">http://struts.apache.org/1.x/struts-taglib/indexedprops.html</a>
+ */
+public class ReflectionValueExtractor {
+ private static final Class<?>[] CLASS_ARGS = new Class[0];
+
+ private static final Object[] OBJECT_ARGS = new Object[0];
+
+ /**
+ * Use a WeakHashMap here, so the keys (Class objects) can be garbage collected. This approach prevents permgen
+ * space overflows due to retention of discarded classloaders.
+ */
+ private static final Map<Class<?>, WeakReference<ClassMap>> CLASS_MAPS = new WeakHashMap<>();
+
+ static final int EOF = -1;
+
+ static final char PROPERTY_START = '.';
+
+ static final char INDEXED_START = '[';
+
+ static final char INDEXED_END = ']';
+
+ static final char MAPPED_START = '(';
+
+ static final char MAPPED_END = ')';
+
+ static class Tokenizer {
+ final String expression;
+
+ int idx;
+
+ Tokenizer(String expression) {
+ this.expression = expression;
+ }
+
+ public int peekChar() {
+ return idx < expression.length() ? expression.charAt(idx) : EOF;
+ }
+
+ public int skipChar() {
+ return idx < expression.length() ? expression.charAt(idx++) : EOF;
+ }
+
+ public String nextToken(char delimiter) {
+ int start = idx;
+
+ while (idx < expression.length() && delimiter != expression.charAt(idx)) {
+ idx++;
+ }
+
+ // delimiter MUST be present
+ if (idx <= start || idx >= expression.length()) {
+ return null;
+ }
+
+ return expression.substring(start, idx++);
+ }
+
+ public String nextPropertyName() {
+ final int start = idx;
+
+ while (idx < expression.length() && Character.isJavaIdentifierPart(expression.charAt(idx))) {
+ idx++;
+ }
+
+ // property name does not require delimiter
+ if (idx <= start || idx > expression.length()) {
+ return null;
+ }
+
+ return expression.substring(start, idx);
+ }
+
+ public int getPosition() {
+ return idx < expression.length() ? idx : EOF;
+ }
+
+ // to make tokenizer look pretty in debugger
+ @Override
+ public String toString() {
+ return idx < expression.length() ? expression.substring(idx) : "<EOF>";
+ }
+ }
+
+ private ReflectionValueExtractor() {}
+
+ /**
+ * <p>The implementation supports indexed, nested and mapped properties.</p>
+ *
+ * <ul>
+ * <li>nested properties should be defined by a dot, i.e. "user.address.street"</li>
+ * <li>indexed properties (java.util.List or array instance) should be contains <code>(\\w+)\\[(\\d+)\\]</code>
+ * pattern, i.e. "user.addresses[1].street"</li>
+ * <li>mapped properties should be contains <code>(\\w+)\\((.+)\\)</code> pattern, i.e.
+ * "user.addresses(myAddress).street"</li>
+ * </ul>
+ *
+ * @param expression not null expression
+ * @param root not null object
+ * @return the object defined by the expression
+ */
+ public static Object evaluate(String expression, Object root) {
+ return evaluate(expression, root, true);
+ }
+
+ /**
+ * <p>The implementation supports indexed, nested and mapped properties.</p>
+ *
+ * <ul>
+ * <li>nested properties should be defined by a dot, i.e. "user.address.street"</li>
+ * <li>indexed properties (java.util.List or array instance) should be contains <code>(\\w+)\\[(\\d+)\\]</code>
+ * pattern, i.e. "user.addresses[1].street"</li>
+ * <li>mapped properties should be contains <code>(\\w+)\\((.+)\\)</code> pattern, i.e.
+ * "user.addresses(myAddress).street"</li>
+ * </ul>
+ *
+ * @param expression not null expression
+ * @param root not null object
+ * @param trimRootToken root start
+ * @return the object defined by the expression
+ */
+ public static Object evaluate(String expression, final Object root, final boolean trimRootToken) {
+ Object value = root;
+
+ // ----------------------------------------------------------------------
+ // Walk the dots and retrieve the ultimate value desired from the
+ // MavenProject instance.
+ // ----------------------------------------------------------------------
+
+ if (expression == null || expression.isEmpty() || !Character.isJavaIdentifierStart(expression.charAt(0))) {
+ return null;
+ }
+
+ boolean hasDots = expression.indexOf(PROPERTY_START) >= 0;
+
+ final Tokenizer tokenizer;
+ if (trimRootToken && hasDots) {
+ tokenizer = new Tokenizer(expression);
+ tokenizer.nextPropertyName();
+ if (tokenizer.getPosition() == EOF) {
+ return null;
+ }
+ } else {
+ tokenizer = new Tokenizer("." + expression);
+ }
+
+ int propertyPosition = tokenizer.getPosition();
+ while (value != null && tokenizer.peekChar() != EOF) {
+ switch (tokenizer.skipChar()) {
+ case INDEXED_START:
+ value = getIndexedValue(
+ expression,
+ propertyPosition,
+ tokenizer.getPosition(),
+ value,
+ tokenizer.nextToken(INDEXED_END));
+ break;
+ case MAPPED_START:
+ value = getMappedValue(
+ expression,
+ propertyPosition,
+ tokenizer.getPosition(),
+ value,
+ tokenizer.nextToken(MAPPED_END));
+ break;
+ case PROPERTY_START:
+ propertyPosition = tokenizer.getPosition();
+ value = getPropertyValue(value, tokenizer.nextPropertyName());
+ break;
+ default:
+ // could not parse expression
+ return null;
+ }
+ }
+
+ return value;
+ }
+
+ private static Object getMappedValue(
+ final String expression, final int from, final int to, final Object value, final String key) {
+ if (value == null || key == null) {
+ return null;
+ }
+ if (value instanceof Map) {
+ try {
+ Object[] localParams = new Object[] {key};
+ ClassMap classMap = getClassMap(value.getClass());
+ Method method = classMap.findMethod("get", localParams);
+ return method.invoke(value, localParams);
+ } catch (Exception e) {
+ throw new RuntimeException(e);
+ }
+ } else {
+ final String message = String.format(
+ "The token '%s' at position '%d' refers to a java.util.Map, but the value seems is an instance of '%s'",
+ expression.subSequence(from, to), from, value.getClass());
+ throw new IllegalArgumentException(message);
+ }
+ }
+
+ private static Object getIndexedValue(
+ final String expression, final int from, final int to, final Object value, final String indexStr) {
+ int index;
+ try {
+ index = Integer.parseInt(indexStr);
+
+ if (value.getClass().isArray()) {
+ return Array.get(value, index);
+ }
+
+ } catch (NumberFormatException e) {
+ final String message = String.format("The indexStr '%s' is not a number", indexStr);
+ throw new IllegalArgumentException(message);
+ }
+
+ if (value instanceof List) {
+ try {
+ ClassMap classMap = getClassMap(value.getClass());
+ // use get method on List interface
+ Object[] localParams = new Object[] {index};
+ Method method = classMap.findMethod("get", localParams);
+ return method.invoke(value, localParams);
+ } catch (Exception e) {
+ throw new RuntimeException(e);
+ }
+ } else {
+ final String message = String.format(
+ "The token '%s' at position '%d' refers to a java.util.List or an array, but the value seems is an instance of '%s'",
Review Comment:
Removed `seems`.
--
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
[GitHub] [maven] gnodet commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1323636017
##########
maven-core/src/main/java/org/apache/maven/utils/introspection/ReflectionValueExtractor.java:
##########
@@ -0,0 +1,325 @@
+/*
+ * 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.utils.introspection;
+
+import java.lang.ref.WeakReference;
+import java.lang.reflect.Array;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.util.List;
+import java.util.Map;
+import java.util.WeakHashMap;
+
+/**
+ * <p>
+ * Using simple dotted expressions to extract the values from an Object instance, For example we might want to extract a
+ * value like: <code>project.build.sourceDirectory</code>
+ * </p>
+ * <p>
+ * The implementation supports indexed, nested and mapped properties similar to the JSP way.
+ * </p>
+ *
+ * @author <a href="mailto:jason@maven.org">Jason van Zyl </a>
+ * @author <a href="mailto:vincent.siveton@gmail.com">Vincent Siveton</a>
+ *
+ * @see <a href=
+ * "http://struts.apache.org/1.x/struts-taglib/indexedprops.html">http://struts.apache.org/1.x/struts-taglib/indexedprops.html</a>
+ */
+public class ReflectionValueExtractor {
+ private static final Class<?>[] CLASS_ARGS = new Class[0];
+
+ private static final Object[] OBJECT_ARGS = new Object[0];
+
+ /**
+ * Use a WeakHashMap here, so the keys (Class objects) can be garbage collected. This approach prevents permgen
+ * space overflows due to retention of discarded classloaders.
+ */
+ private static final Map<Class<?>, WeakReference<ClassMap>> CLASS_MAPS = new WeakHashMap<>();
+
+ static final int EOF = -1;
+
+ static final char PROPERTY_START = '.';
+
+ static final char INDEXED_START = '[';
+
+ static final char INDEXED_END = ']';
+
+ static final char MAPPED_START = '(';
+
+ static final char MAPPED_END = ')';
+
+ static class Tokenizer {
+ final String expression;
+
+ int idx;
+
+ Tokenizer(String expression) {
+ this.expression = expression;
+ }
+
+ public int peekChar() {
+ return idx < expression.length() ? expression.charAt(idx) : EOF;
+ }
+
+ public int skipChar() {
+ return idx < expression.length() ? expression.charAt(idx++) : EOF;
+ }
+
+ public String nextToken(char delimiter) {
+ int start = idx;
+
+ while (idx < expression.length() && delimiter != expression.charAt(idx)) {
+ idx++;
+ }
+
+ // delimiter MUST be present
+ if (idx <= start || idx >= expression.length()) {
+ return null;
+ }
+
+ return expression.substring(start, idx++);
+ }
+
+ public String nextPropertyName() {
+ final int start = idx;
+
+ while (idx < expression.length() && Character.isJavaIdentifierPart(expression.charAt(idx))) {
+ idx++;
+ }
+
+ // property name does not require delimiter
+ if (idx <= start || idx > expression.length()) {
+ return null;
+ }
+
+ return expression.substring(start, idx);
+ }
+
+ public int getPosition() {
+ return idx < expression.length() ? idx : EOF;
+ }
+
+ // to make tokenizer look pretty in debugger
+ @Override
+ public String toString() {
+ return idx < expression.length() ? expression.substring(idx) : "<EOF>";
+ }
+ }
+
+ private ReflectionValueExtractor() {}
+
+ /**
+ * <p>The implementation supports indexed, nested and mapped properties.</p>
+ *
+ * <ul>
+ * <li>nested properties should be defined by a dot, i.e. "user.address.street"</li>
+ * <li>indexed properties (java.util.List or array instance) should be contains <code>(\\w+)\\[(\\d+)\\]</code>
+ * pattern, i.e. "user.addresses[1].street"</li>
+ * <li>mapped properties should be contains <code>(\\w+)\\((.+)\\)</code> pattern, i.e.
+ * "user.addresses(myAddress).street"</li>
+ * </ul>
+ *
+ * @param expression not null expression
+ * @param root not null object
+ * @return the object defined by the expression
+ */
+ public static Object evaluate(String expression, Object root) {
+ return evaluate(expression, root, true);
+ }
+
+ /**
+ * <p>The implementation supports indexed, nested and mapped properties.</p>
+ *
+ * <ul>
+ * <li>nested properties should be defined by a dot, i.e. "user.address.street"</li>
+ * <li>indexed properties (java.util.List or array instance) should be contains <code>(\\w+)\\[(\\d+)\\]</code>
+ * pattern, i.e. "user.addresses[1].street"</li>
+ * <li>mapped properties should be contains <code>(\\w+)\\((.+)\\)</code> pattern, i.e.
+ * "user.addresses(myAddress).street"</li>
+ * </ul>
+ *
+ * @param expression not null expression
+ * @param root not null object
+ * @param trimRootToken root start
+ * @return the object defined by the expression
+ */
+ public static Object evaluate(String expression, final Object root, final boolean trimRootToken) {
+ Object value = root;
+
+ // ----------------------------------------------------------------------
+ // Walk the dots and retrieve the ultimate value desired from the
+ // MavenProject instance.
+ // ----------------------------------------------------------------------
+
+ if (expression == null || expression.isEmpty() || !Character.isJavaIdentifierStart(expression.charAt(0))) {
+ return null;
+ }
+
+ boolean hasDots = expression.indexOf(PROPERTY_START) >= 0;
+
+ final Tokenizer tokenizer;
+ if (trimRootToken && hasDots) {
+ tokenizer = new Tokenizer(expression);
+ tokenizer.nextPropertyName();
+ if (tokenizer.getPosition() == EOF) {
+ return null;
+ }
+ } else {
+ tokenizer = new Tokenizer("." + expression);
+ }
+
+ int propertyPosition = tokenizer.getPosition();
+ while (value != null && tokenizer.peekChar() != EOF) {
+ switch (tokenizer.skipChar()) {
+ case INDEXED_START:
+ value = getIndexedValue(
+ expression,
+ propertyPosition,
+ tokenizer.getPosition(),
+ value,
+ tokenizer.nextToken(INDEXED_END));
+ break;
+ case MAPPED_START:
+ value = getMappedValue(
+ expression,
+ propertyPosition,
+ tokenizer.getPosition(),
+ value,
+ tokenizer.nextToken(MAPPED_END));
+ break;
+ case PROPERTY_START:
+ propertyPosition = tokenizer.getPosition();
+ value = getPropertyValue(value, tokenizer.nextPropertyName());
+ break;
+ default:
+ // could not parse expression
+ return null;
+ }
+ }
+
+ return value;
+ }
+
+ private static Object getMappedValue(
+ final String expression, final int from, final int to, final Object value, final String key) {
+ if (value == null || key == null) {
+ return null;
+ }
+ if (value instanceof Map) {
+ try {
+ Object[] localParams = new Object[] {key};
+ ClassMap classMap = getClassMap(value.getClass());
+ Method method = classMap.findMethod("get", localParams);
+ return method.invoke(value, localParams);
+ } catch (Exception e) {
+ throw new RuntimeException(e);
+ }
+ } else {
+ final String message = String.format(
+ "The token '%s' at position '%d' refers to a java.util.Map, but the value seems is an instance of '%s'",
Review Comment:
I've removed the `seems` ...
--
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
[GitHub] [maven] elharo commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1321373092
##########
maven-core/src/main/java/org/apache/maven/utils/dag/Dag.java:
##########
@@ -0,0 +1,255 @@
+/*
+ * 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.utils.dag;
+
+/*
+ * Copyright The Codehaus Foundation.
Review Comment:
ditto. Possibly start here: https://incubator.apache.org/ip-clearance/ip-clearance-template.html
##########
maven-core/src/main/java/org/apache/maven/utils/dag/CycleDetector.java:
##########
@@ -0,0 +1,131 @@
+/*
+ * 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.utils.dag;
+
+/*
+ * Copyright The Codehaus Foundation.
Review Comment:
as above
##########
maven-model-builder/src/main/java/org/apache/maven/utils/Os.java:
##########
@@ -0,0 +1,213 @@
+/*
+ * 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.utils;
+
+import java.util.Locale;
+import java.util.stream.Stream;
+
+/**
+ * OS support
+ */
+public class Os {
+
+ /**
+ * The OS Name.
+ */
+ public static final String OS_NAME = System.getProperty("os.name").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * The OA architecture.
+ */
+ public static final String OS_ARCH = System.getProperty("os.arch").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * The OS version.
+ */
+ public static final String OS_VERSION = System.getProperty("os.version").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * OS Family
+ */
+ public static final String OS_FAMILY;
+
+ public static final boolean IS_WINDOWS;
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_WINDOWS = "windows";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_WIN9X = "win9x";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_NT = "winnt";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_OS2 = "os/2";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_NETWARE = "netware";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_DOS = "dos";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_MAC = "mac";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_TANDEM = "tandem";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_UNIX = "unix";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_OPENVMS = "openvms";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_ZOS = "z/os";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_OS400 = "os/400";
+
+ /**
+ * OpenJDK is reported to call MacOS X "Darwin"
+ *
+ * @see <a href="https://issues.apache.org/bugzilla/show_bug.cgi?id=44889">bugzilla issue</a>
+ * @see <a href="https://issues.apache.org/jira/browse/HADOOP-3318">HADOOP-3318</a>
+ */
+ private static final String DARWIN = "darwin";
+
+ /**
+ * The path separator.
+ */
+ private static final String PATH_SEP = System.getProperty("path.separator");
+
+ static {
+ OS_FAMILY = getOsFamily();
+ IS_WINDOWS = isWindowsFamily();
+ }
+
+ private Os() {}
+
+ /**
+ * Determines if the OS on which Maven is executing matches the
+ * given OS family.
+ *
+ * @param family the family to check for
+ * @return true if the OS matches
+ *
+ */
+ public static boolean isFamily(String family) {
Review Comment:
I don't think this needs to be public, though perhaps I missed something. It's definitely the wonkiest part of this code, so if we can make it private we absolutely should.
##########
maven-model-builder/src/main/java/org/apache/maven/utils/Os.java:
##########
@@ -0,0 +1,213 @@
+/*
+ * 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.utils;
+
+import java.util.Locale;
+import java.util.stream.Stream;
+
+/**
+ * OS support
+ */
+public class Os {
+
+ /**
+ * The OS Name.
+ */
+ public static final String OS_NAME = System.getProperty("os.name").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * The OA architecture.
+ */
+ public static final String OS_ARCH = System.getProperty("os.arch").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * The OS version.
+ */
+ public static final String OS_VERSION = System.getProperty("os.version").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * OS Family
+ */
+ public static final String OS_FAMILY;
+
+ public static final boolean IS_WINDOWS;
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_WINDOWS = "windows";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_WIN9X = "win9x";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_NT = "winnt";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_OS2 = "os/2";
Review Comment:
remove or make private
##########
maven-model-builder/src/main/java/org/apache/maven/utils/Os.java:
##########
@@ -0,0 +1,213 @@
+/*
+ * 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.utils;
+
+import java.util.Locale;
+import java.util.stream.Stream;
+
+/**
+ * OS support
+ */
+public class Os {
+
+ /**
+ * The OS Name.
+ */
+ public static final String OS_NAME = System.getProperty("os.name").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * The OA architecture.
+ */
+ public static final String OS_ARCH = System.getProperty("os.arch").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * The OS version.
+ */
+ public static final String OS_VERSION = System.getProperty("os.version").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * OS Family
+ */
+ public static final String OS_FAMILY;
+
+ public static final boolean IS_WINDOWS;
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_WINDOWS = "windows";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_WIN9X = "win9x";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_NT = "winnt";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_OS2 = "os/2";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_NETWARE = "netware";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_DOS = "dos";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_MAC = "mac";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_TANDEM = "tandem";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_UNIX = "unix";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_OPENVMS = "openvms";
Review Comment:
not sure about these,but probably remove or make private
##########
maven-core/src/main/java/org/apache/maven/utils/dag/CycleDetectedException.java:
##########
@@ -0,0 +1,56 @@
+/*
+ * 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.utils.dag;
+
+/*
+ * Copyright The Codehaus Foundation.
Review Comment:
Might need an Apache lawyer here, but I suspect this isn't OK.
Also I really, really think this PR should be split up so easy parts like removing StringUtils don't have to wait on complex issues like the status of Codehaus copyright notices.
##########
maven-model-builder/src/main/java/org/apache/maven/utils/Os.java:
##########
@@ -0,0 +1,213 @@
+/*
+ * 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.utils;
+
+import java.util.Locale;
+import java.util.stream.Stream;
+
+/**
+ * OS support
+ */
+public class Os {
+
+ /**
+ * The OS Name.
+ */
+ public static final String OS_NAME = System.getProperty("os.name").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * The OA architecture.
+ */
+ public static final String OS_ARCH = System.getProperty("os.arch").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * The OS version.
+ */
+ public static final String OS_VERSION = System.getProperty("os.version").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * OS Family
+ */
+ public static final String OS_FAMILY;
+
+ public static final boolean IS_WINDOWS;
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_WINDOWS = "windows";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_WIN9X = "win9x";
Review Comment:
remove or make private
##########
maven-model-builder/src/main/java/org/apache/maven/utils/Os.java:
##########
@@ -0,0 +1,213 @@
+/*
+ * 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.utils;
+
+import java.util.Locale;
+import java.util.stream.Stream;
+
+/**
+ * OS support
+ */
+public class Os {
+
+ /**
+ * The OS Name.
+ */
+ public static final String OS_NAME = System.getProperty("os.name").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * The OA architecture.
+ */
+ public static final String OS_ARCH = System.getProperty("os.arch").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * The OS version.
+ */
+ public static final String OS_VERSION = System.getProperty("os.version").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * OS Family
+ */
+ public static final String OS_FAMILY;
+
+ public static final boolean IS_WINDOWS;
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_WINDOWS = "windows";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_WIN9X = "win9x";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_NT = "winnt";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_OS2 = "os/2";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_NETWARE = "netware";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_DOS = "dos";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_MAC = "mac";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_TANDEM = "tandem";
Review Comment:
remove or make private
##########
maven-model-builder/src/main/java/org/apache/maven/utils/Os.java:
##########
@@ -0,0 +1,213 @@
+/*
+ * 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.utils;
+
+import java.util.Locale;
+import java.util.stream.Stream;
+
+/**
+ * OS support
+ */
+public class Os {
+
+ /**
+ * The OS Name.
+ */
+ public static final String OS_NAME = System.getProperty("os.name").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * The OA architecture.
+ */
+ public static final String OS_ARCH = System.getProperty("os.arch").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * The OS version.
+ */
+ public static final String OS_VERSION = System.getProperty("os.version").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * OS Family
+ */
+ public static final String OS_FAMILY;
+
+ public static final boolean IS_WINDOWS;
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_WINDOWS = "windows";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_WIN9X = "win9x";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_NT = "winnt";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_OS2 = "os/2";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_NETWARE = "netware";
Review Comment:
surely we can get rid of Netware in 2023 and probably some of the others too.
--
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
[GitHub] [maven] gnodet commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1321443872
##########
maven-model-builder/src/main/java/org/apache/maven/utils/Os.java:
##########
@@ -0,0 +1,213 @@
+/*
+ * 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.utils;
+
+import java.util.Locale;
+import java.util.stream.Stream;
+
+/**
+ * OS support
+ */
+public class Os {
+
+ /**
+ * The OS Name.
+ */
+ public static final String OS_NAME = System.getProperty("os.name").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * The OA architecture.
+ */
+ public static final String OS_ARCH = System.getProperty("os.arch").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * The OS version.
+ */
+ public static final String OS_VERSION = System.getProperty("os.version").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * OS Family
+ */
+ public static final String OS_FAMILY;
+
+ public static final boolean IS_WINDOWS;
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_WINDOWS = "windows";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_WIN9X = "win9x";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_NT = "winnt";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_OS2 = "os/2";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_NETWARE = "netware";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_DOS = "dos";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_MAC = "mac";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_TANDEM = "tandem";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_UNIX = "unix";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_OPENVMS = "openvms";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_ZOS = "z/os";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_OS400 = "os/400";
+
+ /**
+ * OpenJDK is reported to call MacOS X "Darwin"
+ *
+ * @see <a href="https://issues.apache.org/bugzilla/show_bug.cgi?id=44889">bugzilla issue</a>
+ * @see <a href="https://issues.apache.org/jira/browse/HADOOP-3318">HADOOP-3318</a>
+ */
+ private static final String DARWIN = "darwin";
+
+ /**
+ * The path separator.
+ */
+ private static final String PATH_SEP = System.getProperty("path.separator");
+
+ static {
+ OS_FAMILY = getOsFamily();
+ IS_WINDOWS = isWindowsFamily();
+ }
+
+ private Os() {}
+
+ /**
+ * Determines if the OS on which Maven is executing matches the
+ * given OS family.
+ *
+ * @param family the family to check for
+ * @return true if the OS matches
+ *
+ */
+ public static boolean isFamily(String family) {
Review Comment:
Yes, I think you missed the fact that it's part of the public Maven 3.x API because it's used by [OS profile activation](https://github.com/apache/maven/blob/3ce0d951c199c9a0e9375f23587dcd7b1b657026/maven-model-builder/src/main/java/org/apache/maven/model/profile/activation/OperatingSystemProfileActivator.java#L140) and the families are documented on the [Maven site](https://maven.apache.org/enforcer/enforcer-rules/requireOS.html), so I don't think it makes much sense making those constants private as they are publicly documented, even if technically, they could be made private.
--
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
[GitHub] [maven] gnodet commented on pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #1208:
URL: https://github.com/apache/maven/pull/1208#issuecomment-1691906581
@elharo I don't really see the need to spend much time on rewriting code copied from a utility class. The DefaultWagonManager is part of maven-compat, so deprecated and not really maintained. Any possible cleanup could introduce problems, so I'm more in favour of a blunt copy/paste, even if the code is not the cleanest, to avoid any possible behaviour difference with this legacy code.
For information, I've successfully run the whole IT test suite after changing the DefaultWagonManager to throw exceptions instead of actually downloading/uploading anything using a Wagon (and disabling the DefaultWagonManagerTest UT). However, it might still be in use by old plugins, so I would not delete it at this point (we do have warnings when using maven-compat since a few months afaik). But I would not spend too much time on this code either...
--
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
[GitHub] [maven] elharo commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1304233854
##########
maven-compat/src/main/java/org/apache/maven/repository/legacy/DefaultWagonManager.java:
##########
@@ -465,6 +469,28 @@ public void getRemoteFile(
}
}
+ private void copyFile(File source, File destination) throws IOException {
+ String message;
+ if (!source.exists()) {
+ message = "File " + source + " does not exist";
+ throw new IOException(message);
+ } else if (!source.getCanonicalPath().equals(destination.getCanonicalPath())) {
+ File parentFile = destination.getParentFile();
+ if (parentFile != null && !parentFile.exists()) {
+ parentFile.mkdirs();
+ }
+ Files.copy(
+ source.toPath(),
+ destination.toPath(),
+ StandardCopyOption.REPLACE_EXISTING,
+ StandardCopyOption.COPY_ATTRIBUTES);
+ if (source.length() != destination.length()) {
Review Comment:
This if block isn't needed. If Files.copy doesn't copy everything, an IOException will be thrown above.
##########
maven-compat/src/main/java/org/apache/maven/repository/legacy/DefaultWagonManager.java:
##########
@@ -465,6 +469,28 @@ public void getRemoteFile(
}
}
+ private void copyFile(File source, File destination) throws IOException {
+ String message;
+ if (!source.exists()) {
+ message = "File " + source + " does not exist";
+ throw new IOException(message);
+ } else if (!source.getCanonicalPath().equals(destination.getCanonicalPath())) {
+ File parentFile = destination.getParentFile();
+ if (parentFile != null && !parentFile.exists()) {
+ parentFile.mkdirs();
Review Comment:
The return value isn't checked here. Use Files.createDirectories instead and drop the exists check.
##########
maven-compat/src/main/java/org/apache/maven/repository/legacy/DefaultWagonManager.java:
##########
@@ -465,6 +469,28 @@ public void getRemoteFile(
}
}
+ private void copyFile(File source, File destination) throws IOException {
Review Comment:
I think this can be static
--
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
[GitHub] [maven] CrazyHZM commented on pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "CrazyHZM (via GitHub)" <gi...@apache.org>.
CrazyHZM commented on PR #1208:
URL: https://github.com/apache/maven/pull/1208#issuecomment-1692879432
> Nice, thx ! I think we can also revert the dependency on `commons-io` which was moved from `test` scope to `runtime`. Afaik, it's not used at runtime anymore with the recent changes.
@gnodet Done.
--
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
[GitHub] [maven] elharo commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1305624838
##########
maven-compat/src/main/java/org/apache/maven/profiles/activation/OperatingSystemProfileActivator.java:
##########
@@ -122,12 +153,54 @@ private boolean determineFamilyMatch(String family) {
test = test.substring(1);
}
- boolean result = Os.isFamily(test);
+ boolean result = isFamily(test);
if (reverse) {
return !result;
} else {
return result;
}
}
+
+ private boolean isFamily(String family) {
Review Comment:
Just moving code from plexus is not kosher, either technically or legally. plexus-utils has iffy ownership and license status due to the Codehaus mess. Nor has it had good maintenance or updates over the years. It should be replaced de novo. Copy paste of the existing code into this project is not OK.
--
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
[GitHub] [maven] CrazyHZM commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "CrazyHZM (via GitHub)" <gi...@apache.org>.
CrazyHZM commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1306615720
##########
maven-compat/src/main/java/org/apache/maven/profiles/activation/OperatingSystemProfileActivator.java:
##########
@@ -122,12 +153,54 @@ private boolean determineFamilyMatch(String family) {
test = test.substring(1);
}
- boolean result = Os.isFamily(test);
+ boolean result = isFamily(test);
if (reverse) {
return !result;
} else {
return result;
}
}
+
+ private boolean isFamily(String family) {
Review Comment:
> > Just moving code from plexus is not kosher, either technically or legally. plexus-utils has iffy ownership and license status due to the Codehaus mess. Nor has it had good maintenance or updates over the years. It should be replaced de novo. Copy paste of the existing code into this project is not OK.
>
> @CrazyHZM for `Os`, we need to use [the one from maven-shared-utils instead](https://github.com/apache/maven-shared-utils/blob/master/src/main/java/org/apache/maven/shared/utils/Os.java)
@gnodet Done.
--
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
[GitHub] [maven] elharo commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1305598002
##########
maven-compat/src/main/java/org/apache/maven/repository/legacy/DefaultWagonManager.java:
##########
@@ -465,6 +469,28 @@ public void getRemoteFile(
}
}
+ private void copyFile(File source, File destination) throws IOException {
+ String message;
+ if (!source.exists()) {
+ message = "File " + source + " does not exist";
+ throw new IOException(message);
+ } else if (!source.getCanonicalPath().equals(destination.getCanonicalPath())) {
+ File parentFile = destination.getParentFile();
+ if (parentFile != null && !parentFile.exists()) {
+ parentFile.mkdirs();
+ }
+ Files.copy(
+ source.toPath(),
+ destination.toPath(),
+ StandardCopyOption.REPLACE_EXISTING,
+ StandardCopyOption.COPY_ATTRIBUTES);
+ if (source.length() != destination.length()) {
Review Comment:
Even with this check it's not safe against concurrent modifications outside this thread or process. I'm willing to assume the JDK method behaves as documented until proven otherwise. And inlining questionable code is no better than writing it from scratch. This block should go.
##########
maven-compat/src/main/java/org/apache/maven/profiles/activation/OperatingSystemProfileActivator.java:
##########
@@ -122,12 +153,54 @@ private boolean determineFamilyMatch(String family) {
test = test.substring(1);
}
- boolean result = Os.isFamily(test);
+ boolean result = isFamily(test);
if (reverse) {
return !result;
} else {
return result;
}
}
+
+ private boolean isFamily(String family) {
Review Comment:
This can likely be made static, though I wonder if there's more work to be done here. Netware? Windows 98? It amazes me that in one thread I'm trying to convince folks not to drop support for the widely used Java 8 while over here in this PR I'm seeing support for OSs that were defunct circa Java 1.4.
--
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
[GitHub] [maven] gnodet commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1304179696
##########
maven-compat/src/main/java/org/apache/maven/repository/legacy/DefaultWagonManager.java:
##########
@@ -465,6 +469,28 @@ public void getRemoteFile(
}
}
+ private void copyFile(File source, File destination) throws IOException {
Review Comment:
It already uses `Files.copy`, so I think that's fine. If needed, it can be inlined, but I don't see the benefit.
--
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
[GitHub] [maven] CrazyHZM commented on pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "CrazyHZM (via GitHub)" <gi...@apache.org>.
CrazyHZM commented on PR #1208:
URL: https://github.com/apache/maven/pull/1208#issuecomment-1700313477
@elharo
Thanks to this careful review, I went over javadoc again and fixed the problems.
--
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
[GitHub] [maven] gnodet commented on pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #1208:
URL: https://github.com/apache/maven/pull/1208#issuecomment-1707713876
> Certainly it can be done in another PR. Take this one piece at a time. There's no rule that says you have to fix every class at once. It's a big dependency and it's OK to break it up into chunks. Indeed it's preferable:
>
> https://google.github.io/eng-practices/review/developer/small-cls.html
What I meant is that it's not just about removing the `Os` switch from this PR. Given the dependency is removed completely, all POMs modifications would have to be reverted too.
Anyway, the class has been rewritten and simplified now.
--
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
[GitHub] [maven] CrazyHZM commented on pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "CrazyHZM (via GitHub)" <gi...@apache.org>.
CrazyHZM commented on PR #1208:
URL: https://github.com/apache/maven/pull/1208#issuecomment-1692812315
@gnodet All issues should have been resolved except for the `DefaultWagonManager` issue dispute, which was controversial.
--
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
[GitHub] [maven] gnodet commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1304513934
##########
maven-compat/src/main/java/org/apache/maven/repository/legacy/DefaultWagonManager.java:
##########
@@ -465,6 +469,28 @@ public void getRemoteFile(
}
}
+ private void copyFile(File source, File destination) throws IOException {
+ String message;
+ if (!source.exists()) {
+ message = "File " + source + " does not exist";
+ throw new IOException(message);
+ } else if (!source.getCanonicalPath().equals(destination.getCanonicalPath())) {
+ File parentFile = destination.getParentFile();
+ if (parentFile != null && !parentFile.exists()) {
+ parentFile.mkdirs();
+ }
+ Files.copy(
+ source.toPath(),
+ destination.toPath(),
+ StandardCopyOption.REPLACE_EXISTING,
+ StandardCopyOption.COPY_ATTRIBUTES);
+ if (source.length() != destination.length()) {
Review Comment:
I have no idea what could happen in case of concurrent access to the repository...
Copy is not necessarily atomic, so I'm not sure that an exception will always be thrown before this check.
--
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
[GitHub] [maven] gnodet commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1306609506
##########
maven-compat/src/main/java/org/apache/maven/profiles/activation/OperatingSystemProfileActivator.java:
##########
@@ -122,12 +153,54 @@ private boolean determineFamilyMatch(String family) {
test = test.substring(1);
}
- boolean result = Os.isFamily(test);
+ boolean result = isFamily(test);
if (reverse) {
return !result;
} else {
return result;
}
}
+
+ private boolean isFamily(String family) {
Review Comment:
> Just moving code from plexus is not kosher, either technically or legally. plexus-utils has iffy ownership and license status due to the Codehaus mess. Nor has it had good maintenance or updates over the years. It should be replaced de novo. Copy paste of the existing code into this project is not OK.
@CrazyHZM for `Os`, we need to use [the one from maven-shared-utils instead](https://github.com/apache/maven-shared-utils/blob/master/src/main/java/org/apache/maven/shared/utils/Os.java)
--
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
[GitHub] [maven] elharo commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1305624838
##########
maven-compat/src/main/java/org/apache/maven/profiles/activation/OperatingSystemProfileActivator.java:
##########
@@ -122,12 +153,54 @@ private boolean determineFamilyMatch(String family) {
test = test.substring(1);
}
- boolean result = Os.isFamily(test);
+ boolean result = isFamily(test);
if (reverse) {
return !result;
} else {
return result;
}
}
+
+ private boolean isFamily(String family) {
Review Comment:
Just moving code from plexus is not kosher, either technically or legally. plexus-utils has iffy ownership and license status due to the Codehaus mess. Nor has it had good maintenance or updates over the years. It should be replaced de novo. Copy paste of the existing code is not OK.
--
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
[GitHub] [maven] CrazyHZM commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "CrazyHZM (via GitHub)" <gi...@apache.org>.
CrazyHZM commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1310415013
##########
maven-core/src/main/java/org/apache/maven/utils/dag/DAG.java:
##########
@@ -0,0 +1,239 @@
+/*
+ * 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.utils.dag;
+
+import java.io.Serializable;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * DAG = Directed Acyclic Graph
+ *
+ * @author <a href="michal.maczka@dimatics.com">Michal Maczka</a>
+ *
+ * TODO this class should be renamed from DAG to Dag
+ */
+public class DAG implements Cloneable, Serializable {
+ // ------------------------------------------------------------
+ // Fields
+ // ------------------------------------------------------------
+ /**
+ * Nodes will be kept in two data structures at the same time for faster processing
+ */
+ /**
+ * Maps vertex's label to vertex
+ */
+ private Map<String, Vertex> vertexMap = new HashMap<>();
+
+ /**
+ * Conatin list of all vertices
+ */
+ private List<Vertex> vertexList = new ArrayList<>();
+
+ // ------------------------------------------------------------
+ // Constructors
+ // ------------------------------------------------------------
+
+ /**
+ *
+ */
+ public DAG() {
+ super();
+ }
+
+ // ------------------------------------------------------------
+ // Accessors
+ // ------------------------------------------------------------
+
+ /**
+ * @return the vertices
+ */
+ public List<Vertex> getVertices() {
+ return vertexList;
+ }
+
+ /**
+ * @deprecated instead use {@link #getVertices()}
+ * @return the vertices
+ */
+ @Deprecated
+ public List<Vertex> getVerticies() {
+ return getVertices();
+ }
+
+ public Set<String> getLabels() {
+ return vertexMap.keySet();
+ }
+
+ // ------------------------------------------------------------
+ // Implementation
+ // ------------------------------------------------------------
+
+ /**
+ * Adds vertex to DAG. If vertex of given label already exist in DAG no vertex is added
+ *
+ * @param label The label of the Vertex
+ * @return New vertex if vertex of given label was not present in the DAG or existing vertex if vertex of given
+ * label was already added to DAG
+ */
+ public Vertex addVertex(final String label) {
+ Vertex retValue = null;
+
+ // check if vertex is already in DAG
+ if (vertexMap.containsKey(label)) {
+ retValue = vertexMap.get(label);
+ } else {
+ retValue = new Vertex(label);
+
+ vertexMap.put(label, retValue);
+
+ vertexList.add(retValue);
+ }
+
+ return retValue;
+ }
+
+ public void addEdge(final String from, final String to) throws CycleDetectedException {
+ final Vertex v1 = addVertex(from);
+
+ final Vertex v2 = addVertex(to);
+
+ addEdge(v1, v2);
+ }
+
+ public void addEdge(final Vertex from, final Vertex to) throws CycleDetectedException {
+
+ from.addEdgeTo(to);
+
+ to.addEdgeFrom(from);
+
+ final List<String> cycle = CycleDetector.introducesCycle(to);
+
+ if (cycle != null) {
+ // remove edge which introduced cycle
+
+ removeEdge(from, to);
+
+ final String msg = "Edge between '" + from + "' and '" + to + "' introduces to cycle in the graph";
+
+ throw new CycleDetectedException(msg, cycle);
+ }
+ }
+
+ public void removeEdge(final String from, final String to) {
+ final Vertex v1 = addVertex(from);
+
+ final Vertex v2 = addVertex(to);
+
+ removeEdge(v1, v2);
+ }
+
+ public void removeEdge(final Vertex from, final Vertex to) {
+ from.removeEdgeTo(to);
+
+ to.removeEdgeFrom(from);
+ }
+
+ public Vertex getVertex(final String label) {
+ final Vertex retValue = vertexMap.get(label);
+
+ return retValue;
+ }
+
+ public boolean hasEdge(final String label1, final String label2) {
+ final Vertex v1 = getVertex(label1);
+
+ final Vertex v2 = getVertex(label2);
+
+ final boolean retValue = v1.getChildren().contains(v2);
+
+ return retValue;
+ }
+
+ /**
+ * @param label see name
+ * @return the childs
+ */
+ public List<String> getChildLabels(final String label) {
+ final Vertex vertex = getVertex(label);
+
+ return vertex.getChildLabels();
+ }
+
+ /**
+ * @param label see name
+ * @return the parents
+ */
+ public List<String> getParentLabels(final String label) {
+ final Vertex vertex = getVertex(label);
+
+ return vertex.getParentLabels();
+ }
+
+ /**
+ * @see java.lang.Object#clone()
+ */
+ @Override
+ public Object clone() throws CloneNotSupportedException {
+ // this is what's failing..
+ final Object retValue = super.clone();
+
+ return retValue;
+ }
+
+ /**
+ * Indicates if there is at least one edge leading to or from vertex of given label
+ * @param label the label
+ * @return <code>true</code> if this vertex is connected with other vertex,<code>false</code> otherwise
+ */
+ public boolean isConnected(final String label) {
+ final Vertex vertex = getVertex(label);
+
+ final boolean retValue = vertex.isConnected();
+
+ return retValue;
+ }
+
+ /**
+ * Return the list of labels of successor in order decided by topological sort
+ *
+ * @param label The label of the vertex whose predecessors are searched
Review Comment:
@elharo
Is the "The" missing in front of the `label`?
--
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
[GitHub] [maven] gnodet commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1283530419
##########
maven-core/src/main/java/org/apache/maven/utils/dag/CycleDetectedException.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.utils.dag;
+
+import java.util.Iterator;
+import java.util.List;
+
+public class CycleDetectedException extends Exception {
+ private List<String> cycle;
+
+ public CycleDetectedException(final String message, final List<String> cycle) {
+ super(message);
+
+ this.cycle = cycle;
+ }
+
+ public List<String> getCycle() {
+ return cycle;
+ }
+
+ public String cycleToString() {
+ final StringBuilder buffer = new StringBuilder();
+
+ for (Iterator<String> iterator = cycle.iterator(); iterator.hasNext(); ) {
Review Comment:
[`String.join(CharSequence, CharSequence...)`](https://docs.oracle.com/javase/8/docs/api/java/lang/String.html#join-java.lang.CharSequence-java.lang.CharSequence...-) has been introduced in JDK 8, so we should rather use that 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
[GitHub] [maven] CrazyHZM commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "CrazyHZM (via GitHub)" <gi...@apache.org>.
CrazyHZM commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1284026832
##########
maven-core/src/main/java/org/apache/maven/utils/dag/CycleDetectedException.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.utils.dag;
+
+import java.util.Iterator;
+import java.util.List;
+
+public class CycleDetectedException extends Exception {
+ private List<String> cycle;
+
+ public CycleDetectedException(final String message, final List<String> cycle) {
+ super(message);
+
+ this.cycle = cycle;
+ }
+
+ public List<String> getCycle() {
+ return cycle;
+ }
+
+ public String cycleToString() {
+ final StringBuilder buffer = new StringBuilder();
+
+ for (Iterator<String> iterator = cycle.iterator(); iterator.hasNext(); ) {
Review Comment:
Done.
--
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
[GitHub] [maven] gnodet commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1323637084
##########
maven-core/src/main/java/org/apache/maven/toolchain/java/JavaToolchainFactory.java:
##########
@@ -86,9 +86,9 @@ public ToolchainPrivate createToolchain(ToolchainModel model) throws Misconfigur
throw new MisconfiguredToolchainException(
"Java toolchain without the " + JavaToolchainImpl.KEY_JAVAHOME + " configuration element.");
}
- File normal = new File(FileUtils.normalize(javahome.getValue()));
+ File normal = Paths.get(javahome.getValue()).normalize().toFile();
Review Comment:
Fixed
##########
maven-core/src/main/java/org/apache/maven/toolchain/java/JavaToolchainImpl.java:
##########
@@ -53,7 +53,7 @@ public String toString() {
}
public String findTool(String toolName) {
- File toRet = findTool(toolName, new File(FileUtils.normalize(getJavaHome())));
+ File toRet = findTool(toolName, Paths.get(getJavaHome()).normalize().toFile());
Review Comment:
Fixed
--
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
[GitHub] [maven] gnodet commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1324600451
##########
maven-model/src/test/java/org/apache/maven/model/v4/Xpp3DomPerfTest.java:
##########
@@ -18,22 +18,6 @@
*/
package org.apache.maven.model.v4;
-/*
Review Comment:
I am the author and owner of this file, so I gave it to Codehaus, but now I'm giving it to the ASF.
So the pointer you gave does not apply here because it's *not* a _third-party source file_.
--
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
[GitHub] [maven] gnodet commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1321446764
##########
maven-model-builder/src/main/java/org/apache/maven/utils/Os.java:
##########
@@ -0,0 +1,213 @@
+/*
+ * 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.utils;
+
+import java.util.Locale;
+import java.util.stream.Stream;
+
+/**
+ * OS support
+ */
+public class Os {
+
+ /**
+ * The OS Name.
+ */
+ public static final String OS_NAME = System.getProperty("os.name").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * The OA architecture.
+ */
+ public static final String OS_ARCH = System.getProperty("os.arch").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * The OS version.
+ */
+ public static final String OS_VERSION = System.getProperty("os.version").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * OS Family
+ */
+ public static final String OS_FAMILY;
+
+ public static final boolean IS_WINDOWS;
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_WINDOWS = "windows";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_WIN9X = "win9x";
Review Comment:
same
##########
maven-model-builder/src/main/java/org/apache/maven/utils/Os.java:
##########
@@ -0,0 +1,213 @@
+/*
+ * 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.utils;
+
+import java.util.Locale;
+import java.util.stream.Stream;
+
+/**
+ * OS support
+ */
+public class Os {
+
+ /**
+ * The OS Name.
+ */
+ public static final String OS_NAME = System.getProperty("os.name").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * The OA architecture.
+ */
+ public static final String OS_ARCH = System.getProperty("os.arch").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * The OS version.
+ */
+ public static final String OS_VERSION = System.getProperty("os.version").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * OS Family
+ */
+ public static final String OS_FAMILY;
+
+ public static final boolean IS_WINDOWS;
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_WINDOWS = "windows";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_WIN9X = "win9x";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_NT = "winnt";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_OS2 = "os/2";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_NETWARE = "netware";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_DOS = "dos";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_MAC = "mac";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_TANDEM = "tandem";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_UNIX = "unix";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_OPENVMS = "openvms";
Review Comment:
same
--
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
[GitHub] [maven] slawekjaranowski commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1321550991
##########
maven-model-builder/src/main/java/org/apache/maven/utils/Os.java:
##########
@@ -0,0 +1,213 @@
+/*
+ * 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.utils;
+
+import java.util.Locale;
+import java.util.stream.Stream;
+
+/**
+ * OS support
+ */
+public class Os {
+
+ /**
+ * The OS Name.
+ */
+ public static final String OS_NAME = System.getProperty("os.name").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * The OA architecture.
+ */
+ public static final String OS_ARCH = System.getProperty("os.arch").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * The OS version.
+ */
+ public static final String OS_VERSION = System.getProperty("os.version").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * OS Family
+ */
+ public static final String OS_FAMILY;
+
+ public static final boolean IS_WINDOWS;
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_WINDOWS = "windows";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_WIN9X = "win9x";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_NT = "winnt";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_OS2 = "os/2";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_NETWARE = "netware";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_DOS = "dos";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_MAC = "mac";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_TANDEM = "tandem";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_UNIX = "unix";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_OPENVMS = "openvms";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_ZOS = "z/os";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_OS400 = "os/400";
+
+ /**
+ * OpenJDK is reported to call MacOS X "Darwin"
+ *
+ * @see <a href="https://issues.apache.org/bugzilla/show_bug.cgi?id=44889">bugzilla issue</a>
+ * @see <a href="https://issues.apache.org/jira/browse/HADOOP-3318">HADOOP-3318</a>
+ */
+ private static final String DARWIN = "darwin";
+
+ /**
+ * The path separator.
+ */
+ private static final String PATH_SEP = System.getProperty("path.separator");
+
+ static {
+ OS_FAMILY = getOsFamily();
+ IS_WINDOWS = isWindowsFamily();
+ }
+
+ private Os() {}
+
+ /**
+ * Determines if the OS on which Maven is executing matches the
+ * given OS family.
+ *
+ * @param family the family to check for
+ * @return true if the OS matches
+ *
+ */
+ public static boolean isFamily(String family) {
Review Comment:
Enforcer rule `requireOS` use `plexus.Os` ... so we need check or improve also enfrcer rule in such cases
--
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
[GitHub] [maven] elharo commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1325782731
##########
maven-model/src/test/java/org/apache/maven/model/v4/Xpp3DomPerfTest.java:
##########
@@ -18,22 +18,6 @@
*/
package org.apache.maven.model.v4;
-/*
Review Comment:
Possibly you can ask at https://issues.apache.org/jira/projects/LEGAL/issues/LEGAL-654?filter=allopenissues
I'm fine with whatever they say is fine.
--
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
[GitHub] [maven] gnodet commented on pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #1208:
URL: https://github.com/apache/maven/pull/1208#issuecomment-1693347546
@CrazyHZM I've seen the `Os` stuff copied multiple times. Could the class be copied into maven-core and reused instead of duplicating this bits 3/4 times ?
--
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
[GitHub] [maven] CrazyHZM commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "CrazyHZM (via GitHub)" <gi...@apache.org>.
CrazyHZM commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1305096986
##########
maven-core/src/main/java/org/apache/maven/utils/introspection/ReflectionValueExtractor.java:
##########
@@ -0,0 +1,325 @@
+/*
+ * 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.utils.introspection;
+
+import java.lang.ref.WeakReference;
+import java.lang.reflect.Array;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.util.List;
+import java.util.Map;
+import java.util.WeakHashMap;
+
+/**
+ * <p>
+ * Using simple dotted expressions to extract the values from an Object instance, For example we might want to extract a
+ * value like: <code>project.build.sourceDirectory</code>
+ * </p>
+ * <p>
+ * The implementation supports indexed, nested and mapped properties similar to the JSP way.
+ * </p>
+ *
+ * @author <a href="mailto:jason@maven.org">Jason van Zyl </a>
+ * @author <a href="mailto:vincent.siveton@gmail.com">Vincent Siveton</a>
+ *
+ * @see <a href=
+ * "http://struts.apache.org/1.x/struts-taglib/indexedprops.html">http://struts.apache.org/1.x/struts-taglib/indexedprops.html</a>
+ */
+public class ReflectionValueExtractor {
Review Comment:
It is used by `org.apache.maven.plugin.PluginParameterExpressionEvaluatorV4` and cannot be made public.
--
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
[GitHub] [maven] CrazyHZM commented on pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "CrazyHZM (via GitHub)" <gi...@apache.org>.
CrazyHZM commented on PR #1208:
URL: https://github.com/apache/maven/pull/1208#issuecomment-1692669455
> @elharo I don't really see the need to spend much time on rewriting code copied from a utility class. The DefaultWagonManager is part of maven-compat, so deprecated and not really maintained. Any possible cleanup could introduce problems, so I'm more in favour of a blunt copy/paste, even if the code is not the cleanest, to avoid any possible behaviour difference with this legacy code.
>
> For information, I've successfully run the whole IT test suite after changing the DefaultWagonManager to throw exceptions instead of actually downloading/uploading anything using a Wagon (and disabling the DefaultWagonManagerTest UT). However, it might still be in use by old plugins, so I would not delete it at this point (we do have warnings when using maven-compat since a few months afaik). But I would not spend too much time on this code either...
>
> This is different for the DAG stuff which is used by maven-core...
@gnodet So we should still do some optimization on the copied DAG, right?
--
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
[GitHub] [maven] elharo commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1310379975
##########
maven-core/src/main/java/org/apache/maven/utils/dag/DAG.java:
##########
@@ -0,0 +1,239 @@
+/*
+ * 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.utils.dag;
+
+import java.io.Serializable;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * DAG = Directed Acyclic Graph
+ *
+ * @author <a href="michal.maczka@dimatics.com">Michal Maczka</a>
+ *
+ * TODO this class should be renamed from DAG to Dag
+ */
+public class DAG implements Cloneable, Serializable {
+ // ------------------------------------------------------------
+ // Fields
+ // ------------------------------------------------------------
+ /**
+ * Nodes will be kept in two data structures at the same time for faster processing
+ */
+ /**
+ * Maps vertex's label to vertex
+ */
+ private Map<String, Vertex> vertexMap = new HashMap<>();
+
+ /**
+ * Conatin list of all vertices
+ */
+ private List<Vertex> vertexList = new ArrayList<>();
+
+ // ------------------------------------------------------------
+ // Constructors
+ // ------------------------------------------------------------
+
+ /**
+ *
+ */
+ public DAG() {
+ super();
+ }
+
+ // ------------------------------------------------------------
+ // Accessors
+ // ------------------------------------------------------------
+
+ /**
+ * @return the vertices
+ */
+ public List<Vertex> getVertices() {
+ return vertexList;
+ }
+
+ /**
+ * @deprecated instead use {@link #getVertices()}
+ * @return the vertices
+ */
+ @Deprecated
+ public List<Vertex> getVerticies() {
+ return getVertices();
+ }
+
+ public Set<String> getLabels() {
+ return vertexMap.keySet();
+ }
+
+ // ------------------------------------------------------------
+ // Implementation
+ // ------------------------------------------------------------
+
+ /**
+ * Adds vertex to DAG. If vertex of given label already exist in DAG no vertex is added
+ *
+ * @param label The label of the Vertex
+ * @return New vertex if vertex of given label was not present in the DAG or existing vertex if vertex of given
+ * label was already added to DAG
+ */
+ public Vertex addVertex(final String label) {
+ Vertex retValue = null;
+
+ // check if vertex is already in DAG
+ if (vertexMap.containsKey(label)) {
+ retValue = vertexMap.get(label);
+ } else {
+ retValue = new Vertex(label);
+
+ vertexMap.put(label, retValue);
+
+ vertexList.add(retValue);
+ }
+
+ return retValue;
+ }
+
+ public void addEdge(final String from, final String to) throws CycleDetectedException {
+ final Vertex v1 = addVertex(from);
+
+ final Vertex v2 = addVertex(to);
+
+ addEdge(v1, v2);
+ }
+
+ public void addEdge(final Vertex from, final Vertex to) throws CycleDetectedException {
+
+ from.addEdgeTo(to);
+
+ to.addEdgeFrom(from);
+
+ final List<String> cycle = CycleDetector.introducesCycle(to);
+
+ if (cycle != null) {
+ // remove edge which introduced cycle
+
+ removeEdge(from, to);
+
+ final String msg = "Edge between '" + from + "' and '" + to + "' introduces to cycle in the graph";
+
+ throw new CycleDetectedException(msg, cycle);
+ }
+ }
+
+ public void removeEdge(final String from, final String to) {
+ final Vertex v1 = addVertex(from);
+
+ final Vertex v2 = addVertex(to);
+
+ removeEdge(v1, v2);
+ }
+
+ public void removeEdge(final Vertex from, final Vertex to) {
+ from.removeEdgeTo(to);
+
+ to.removeEdgeFrom(from);
+ }
+
+ public Vertex getVertex(final String label) {
+ final Vertex retValue = vertexMap.get(label);
+
+ return retValue;
+ }
+
+ public boolean hasEdge(final String label1, final String label2) {
+ final Vertex v1 = getVertex(label1);
+
+ final Vertex v2 = getVertex(label2);
+
+ final boolean retValue = v1.getChildren().contains(v2);
+
+ return retValue;
+ }
+
+ /**
+ * @param label see name
+ * @return the childs
+ */
+ public List<String> getChildLabels(final String label) {
+ final Vertex vertex = getVertex(label);
+
+ return vertex.getChildLabels();
+ }
+
+ /**
+ * @param label see name
+ * @return the parents
+ */
+ public List<String> getParentLabels(final String label) {
+ final Vertex vertex = getVertex(label);
+
+ return vertex.getParentLabels();
+ }
+
+ /**
+ * @see java.lang.Object#clone()
+ */
+ @Override
+ public Object clone() throws CloneNotSupportedException {
+ // this is what's failing..
+ final Object retValue = super.clone();
+
+ return retValue;
+ }
+
+ /**
+ * Indicates if there is at least one edge leading to or from vertex of given label
+ * @param label the label
+ * @return <code>true</code> if this vertex is connected with other vertex,<code>false</code> otherwise
+ */
+ public boolean isConnected(final String label) {
+ final Vertex vertex = getVertex(label);
+
+ final boolean retValue = vertex.isConnected();
+
+ return retValue;
+ }
+
+ /**
+ * Return the list of labels of successor in order decided by topological sort
+ *
+ * @param label The label of the vertex whose predecessors are searched
Review Comment:
nit: the
##########
maven-core/src/main/java/org/apache/maven/plugin/internal/MavenPluginMavenPrerequisiteChecker.java:
##########
@@ -44,7 +43,17 @@ public MavenPluginMavenPrerequisiteChecker(RuntimeInformation runtimeInformation
@Override
public void accept(PluginDescriptor pluginDescriptor) {
String requiredMavenVersion = pluginDescriptor.getRequiredMavenVersion();
- if (StringUtils.isNotBlank(requiredMavenVersion)) {
+
+ boolean isBlankVersion = true;
+ if (requiredMavenVersion != null) {
+ for (int i = 0; i < requiredMavenVersion.length(); i++) {
Review Comment:
just use String.trim
##########
maven-core/src/main/java/org/apache/maven/utils/dag/DAG.java:
##########
@@ -0,0 +1,239 @@
+/*
+ * 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.utils.dag;
+
+import java.io.Serializable;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * DAG = Directed Acyclic Graph
+ *
+ * @author <a href="michal.maczka@dimatics.com">Michal Maczka</a>
+ *
+ * TODO this class should be renamed from DAG to Dag
+ */
+public class DAG implements Cloneable, Serializable {
+ // ------------------------------------------------------------
+ // Fields
+ // ------------------------------------------------------------
+ /**
+ * Nodes will be kept in two data structures at the same time for faster processing
+ */
+ /**
+ * Maps vertex's label to vertex
+ */
+ private Map<String, Vertex> vertexMap = new HashMap<>();
+
+ /**
+ * Conatin list of all vertices
+ */
+ private List<Vertex> vertexList = new ArrayList<>();
+
+ // ------------------------------------------------------------
+ // Constructors
+ // ------------------------------------------------------------
+
+ /**
+ *
+ */
+ public DAG() {
+ super();
+ }
+
+ // ------------------------------------------------------------
+ // Accessors
+ // ------------------------------------------------------------
+
+ /**
+ * @return the vertices
+ */
+ public List<Vertex> getVertices() {
+ return vertexList;
+ }
+
+ /**
+ * @deprecated instead use {@link #getVertices()}
+ * @return the vertices
+ */
+ @Deprecated
+ public List<Vertex> getVerticies() {
+ return getVertices();
+ }
+
+ public Set<String> getLabels() {
+ return vertexMap.keySet();
+ }
+
+ // ------------------------------------------------------------
+ // Implementation
+ // ------------------------------------------------------------
+
+ /**
+ * Adds vertex to DAG. If vertex of given label already exist in DAG no vertex is added
+ *
+ * @param label The label of the Vertex
+ * @return New vertex if vertex of given label was not present in the DAG or existing vertex if vertex of given
+ * label was already added to DAG
+ */
+ public Vertex addVertex(final String label) {
+ Vertex retValue = null;
+
+ // check if vertex is already in DAG
+ if (vertexMap.containsKey(label)) {
+ retValue = vertexMap.get(label);
+ } else {
+ retValue = new Vertex(label);
+
+ vertexMap.put(label, retValue);
+
+ vertexList.add(retValue);
+ }
+
+ return retValue;
+ }
+
+ public void addEdge(final String from, final String to) throws CycleDetectedException {
+ final Vertex v1 = addVertex(from);
+
+ final Vertex v2 = addVertex(to);
+
+ addEdge(v1, v2);
+ }
+
+ public void addEdge(final Vertex from, final Vertex to) throws CycleDetectedException {
+
+ from.addEdgeTo(to);
+
+ to.addEdgeFrom(from);
+
+ final List<String> cycle = CycleDetector.introducesCycle(to);
+
+ if (cycle != null) {
+ // remove edge which introduced cycle
+
+ removeEdge(from, to);
+
+ final String msg = "Edge between '" + from + "' and '" + to + "' introduces to cycle in the graph";
+
+ throw new CycleDetectedException(msg, cycle);
+ }
+ }
+
+ public void removeEdge(final String from, final String to) {
+ final Vertex v1 = addVertex(from);
+
+ final Vertex v2 = addVertex(to);
+
+ removeEdge(v1, v2);
+ }
+
+ public void removeEdge(final Vertex from, final Vertex to) {
+ from.removeEdgeTo(to);
+
+ to.removeEdgeFrom(from);
+ }
+
+ public Vertex getVertex(final String label) {
+ final Vertex retValue = vertexMap.get(label);
+
+ return retValue;
+ }
+
+ public boolean hasEdge(final String label1, final String label2) {
+ final Vertex v1 = getVertex(label1);
+
+ final Vertex v2 = getVertex(label2);
+
+ final boolean retValue = v1.getChildren().contains(v2);
+
+ return retValue;
+ }
+
+ /**
+ * @param label see name
+ * @return the childs
+ */
+ public List<String> getChildLabels(final String label) {
+ final Vertex vertex = getVertex(label);
+
+ return vertex.getChildLabels();
+ }
+
+ /**
+ * @param label see name
+ * @return the parents
+ */
+ public List<String> getParentLabels(final String label) {
+ final Vertex vertex = getVertex(label);
+
+ return vertex.getParentLabels();
+ }
+
+ /**
+ * @see java.lang.Object#clone()
+ */
+ @Override
+ public Object clone() throws CloneNotSupportedException {
+ // this is what's failing..
+ final Object retValue = super.clone();
+
+ return retValue;
+ }
+
+ /**
+ * Indicates if there is at least one edge leading to or from vertex of given label
+ * @param label the label
+ * @return <code>true</code> if this vertex is connected with other vertex,<code>false</code> otherwise
+ */
+ public boolean isConnected(final String label) {
+ final Vertex vertex = getVertex(label);
+
+ final boolean retValue = vertex.isConnected();
+
+ return retValue;
+ }
+
+ /**
+ * Return the list of labels of successor in order decided by topological sort
+ *
+ * @param label The label of the vertex whose predecessors are searched
+ * @return The list of labels. Returned list contains also the label passed as parameter to this method. This label
Review Comment:
nit: the
##########
maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java:
##########
@@ -194,7 +193,17 @@ public PluginDescriptor getPluginDescriptor(
PluginDescriptor descriptor = extractPluginDescriptor(pluginArtifact, plugin);
- if (StringUtils.isBlank(descriptor.getRequiredMavenVersion())) {
+ boolean isBlankVersion = true;
+ if (descriptor.getRequiredMavenVersion() != null) {
+ for (int i = 0; i < descriptor.getRequiredMavenVersion().length(); i++) {
Review Comment:
just call String.trim and see if the result is empty or use SttingUtils.isBlank from Apache Commons Lang
##########
maven-core/src/main/java/org/apache/maven/utils/dag/DAG.java:
##########
@@ -0,0 +1,239 @@
+/*
+ * 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.utils.dag;
+
+import java.io.Serializable;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * DAG = Directed Acyclic Graph
+ *
+ * @author <a href="michal.maczka@dimatics.com">Michal Maczka</a>
+ *
+ * TODO this class should be renamed from DAG to Dag
+ */
+public class DAG implements Cloneable, Serializable {
+ // ------------------------------------------------------------
+ // Fields
+ // ------------------------------------------------------------
+ /**
+ * Nodes will be kept in two data structures at the same time for faster processing
+ */
+ /**
+ * Maps vertex's label to vertex
+ */
+ private Map<String, Vertex> vertexMap = new HashMap<>();
+
+ /**
+ * Conatin list of all vertices
+ */
+ private List<Vertex> vertexList = new ArrayList<>();
+
+ // ------------------------------------------------------------
+ // Constructors
+ // ------------------------------------------------------------
+
+ /**
+ *
+ */
+ public DAG() {
+ super();
+ }
+
+ // ------------------------------------------------------------
+ // Accessors
+ // ------------------------------------------------------------
+
+ /**
+ * @return the vertices
+ */
+ public List<Vertex> getVertices() {
+ return vertexList;
+ }
+
+ /**
+ * @deprecated instead use {@link #getVertices()}
+ * @return the vertices
+ */
+ @Deprecated
+ public List<Vertex> getVerticies() {
+ return getVertices();
+ }
+
+ public Set<String> getLabels() {
+ return vertexMap.keySet();
+ }
+
+ // ------------------------------------------------------------
+ // Implementation
+ // ------------------------------------------------------------
+
+ /**
+ * Adds vertex to DAG. If vertex of given label already exist in DAG no vertex is added
+ *
+ * @param label The label of the Vertex
+ * @return New vertex if vertex of given label was not present in the DAG or existing vertex if vertex of given
+ * label was already added to DAG
+ */
+ public Vertex addVertex(final String label) {
+ Vertex retValue = null;
+
+ // check if vertex is already in DAG
+ if (vertexMap.containsKey(label)) {
+ retValue = vertexMap.get(label);
+ } else {
+ retValue = new Vertex(label);
+
+ vertexMap.put(label, retValue);
+
+ vertexList.add(retValue);
+ }
+
+ return retValue;
+ }
+
+ public void addEdge(final String from, final String to) throws CycleDetectedException {
+ final Vertex v1 = addVertex(from);
+
+ final Vertex v2 = addVertex(to);
+
+ addEdge(v1, v2);
+ }
+
+ public void addEdge(final Vertex from, final Vertex to) throws CycleDetectedException {
+
+ from.addEdgeTo(to);
+
+ to.addEdgeFrom(from);
+
+ final List<String> cycle = CycleDetector.introducesCycle(to);
+
+ if (cycle != null) {
+ // remove edge which introduced cycle
+
+ removeEdge(from, to);
+
+ final String msg = "Edge between '" + from + "' and '" + to + "' introduces to cycle in the graph";
+
+ throw new CycleDetectedException(msg, cycle);
+ }
+ }
+
+ public void removeEdge(final String from, final String to) {
+ final Vertex v1 = addVertex(from);
+
+ final Vertex v2 = addVertex(to);
+
+ removeEdge(v1, v2);
+ }
+
+ public void removeEdge(final Vertex from, final Vertex to) {
+ from.removeEdgeTo(to);
+
+ to.removeEdgeFrom(from);
+ }
+
+ public Vertex getVertex(final String label) {
+ final Vertex retValue = vertexMap.get(label);
+
+ return retValue;
+ }
+
+ public boolean hasEdge(final String label1, final String label2) {
+ final Vertex v1 = getVertex(label1);
+
+ final Vertex v2 = getVertex(label2);
+
+ final boolean retValue = v1.getChildren().contains(v2);
+
+ return retValue;
+ }
+
+ /**
+ * @param label see name
+ * @return the childs
+ */
+ public List<String> getChildLabels(final String label) {
+ final Vertex vertex = getVertex(label);
+
+ return vertex.getChildLabels();
+ }
+
+ /**
+ * @param label see name
+ * @return the parents
+ */
+ public List<String> getParentLabels(final String label) {
+ final Vertex vertex = getVertex(label);
+
+ return vertex.getParentLabels();
+ }
+
+ /**
+ * @see java.lang.Object#clone()
+ */
+ @Override
+ public Object clone() throws CloneNotSupportedException {
+ // this is what's failing..
+ final Object retValue = super.clone();
+
+ return retValue;
+ }
+
+ /**
+ * Indicates if there is at least one edge leading to or from vertex of given label
+ * @param label the label
Review Comment:
nit: blank line
--
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
[GitHub] [maven] CrazyHZM commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "CrazyHZM (via GitHub)" <gi...@apache.org>.
CrazyHZM commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1310415877
##########
maven-core/src/main/java/org/apache/maven/utils/dag/DAG.java:
##########
@@ -0,0 +1,239 @@
+/*
+ * 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.utils.dag;
+
+import java.io.Serializable;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * DAG = Directed Acyclic Graph
+ *
+ * @author <a href="michal.maczka@dimatics.com">Michal Maczka</a>
+ *
+ * TODO this class should be renamed from DAG to Dag
+ */
+public class DAG implements Cloneable, Serializable {
+ // ------------------------------------------------------------
+ // Fields
+ // ------------------------------------------------------------
+ /**
+ * Nodes will be kept in two data structures at the same time for faster processing
+ */
+ /**
+ * Maps vertex's label to vertex
+ */
+ private Map<String, Vertex> vertexMap = new HashMap<>();
+
+ /**
+ * Conatin list of all vertices
+ */
+ private List<Vertex> vertexList = new ArrayList<>();
+
+ // ------------------------------------------------------------
+ // Constructors
+ // ------------------------------------------------------------
+
+ /**
+ *
+ */
+ public DAG() {
+ super();
+ }
+
+ // ------------------------------------------------------------
+ // Accessors
+ // ------------------------------------------------------------
+
+ /**
+ * @return the vertices
+ */
+ public List<Vertex> getVertices() {
+ return vertexList;
+ }
+
+ /**
+ * @deprecated instead use {@link #getVertices()}
+ * @return the vertices
+ */
+ @Deprecated
+ public List<Vertex> getVerticies() {
+ return getVertices();
+ }
+
+ public Set<String> getLabels() {
+ return vertexMap.keySet();
+ }
+
+ // ------------------------------------------------------------
+ // Implementation
+ // ------------------------------------------------------------
+
+ /**
+ * Adds vertex to DAG. If vertex of given label already exist in DAG no vertex is added
+ *
+ * @param label The label of the Vertex
+ * @return New vertex if vertex of given label was not present in the DAG or existing vertex if vertex of given
+ * label was already added to DAG
+ */
+ public Vertex addVertex(final String label) {
+ Vertex retValue = null;
+
+ // check if vertex is already in DAG
+ if (vertexMap.containsKey(label)) {
+ retValue = vertexMap.get(label);
+ } else {
+ retValue = new Vertex(label);
+
+ vertexMap.put(label, retValue);
+
+ vertexList.add(retValue);
+ }
+
+ return retValue;
+ }
+
+ public void addEdge(final String from, final String to) throws CycleDetectedException {
+ final Vertex v1 = addVertex(from);
+
+ final Vertex v2 = addVertex(to);
+
+ addEdge(v1, v2);
+ }
+
+ public void addEdge(final Vertex from, final Vertex to) throws CycleDetectedException {
+
+ from.addEdgeTo(to);
+
+ to.addEdgeFrom(from);
+
+ final List<String> cycle = CycleDetector.introducesCycle(to);
+
+ if (cycle != null) {
+ // remove edge which introduced cycle
+
+ removeEdge(from, to);
+
+ final String msg = "Edge between '" + from + "' and '" + to + "' introduces to cycle in the graph";
+
+ throw new CycleDetectedException(msg, cycle);
+ }
+ }
+
+ public void removeEdge(final String from, final String to) {
+ final Vertex v1 = addVertex(from);
+
+ final Vertex v2 = addVertex(to);
+
+ removeEdge(v1, v2);
+ }
+
+ public void removeEdge(final Vertex from, final Vertex to) {
+ from.removeEdgeTo(to);
+
+ to.removeEdgeFrom(from);
+ }
+
+ public Vertex getVertex(final String label) {
+ final Vertex retValue = vertexMap.get(label);
+
+ return retValue;
+ }
+
+ public boolean hasEdge(final String label1, final String label2) {
+ final Vertex v1 = getVertex(label1);
+
+ final Vertex v2 = getVertex(label2);
+
+ final boolean retValue = v1.getChildren().contains(v2);
+
+ return retValue;
+ }
+
+ /**
+ * @param label see name
+ * @return the childs
+ */
+ public List<String> getChildLabels(final String label) {
+ final Vertex vertex = getVertex(label);
+
+ return vertex.getChildLabels();
+ }
+
+ /**
+ * @param label see name
+ * @return the parents
+ */
+ public List<String> getParentLabels(final String label) {
+ final Vertex vertex = getVertex(label);
+
+ return vertex.getParentLabels();
+ }
+
+ /**
+ * @see java.lang.Object#clone()
+ */
+ @Override
+ public Object clone() throws CloneNotSupportedException {
+ // this is what's failing..
+ final Object retValue = super.clone();
+
+ return retValue;
+ }
+
+ /**
+ * Indicates if there is at least one edge leading to or from vertex of given label
+ * @param label the label
+ * @return <code>true</code> if this vertex is connected with other vertex,<code>false</code> otherwise
+ */
+ public boolean isConnected(final String label) {
+ final Vertex vertex = getVertex(label);
+
+ final boolean retValue = vertex.isConnected();
+
+ return retValue;
+ }
+
+ /**
+ * Return the list of labels of successor in order decided by topological sort
+ *
+ * @param label The label of the vertex whose predecessors are searched
+ * @return The list of labels. Returned list contains also the label passed as parameter to this method. This label
Review Comment:
@elharo
Is the "The" missing in front of the labels?
--
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
[GitHub] [maven] elharo commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1310422373
##########
maven-core/src/main/java/org/apache/maven/utils/dag/Vertex.java:
##########
@@ -0,0 +1,151 @@
+/*
+ * 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.utils.dag;
+
+import java.io.Serializable;
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * @author <a href="michal.maczka@dimatics.com">Michal Maczka</a>
+ *
+ */
+public class Vertex implements Cloneable, Serializable {
+ // ------------------------------------------------------------
+ // Fields
+ // ------------------------------------------------------------
+ private String label = null;
+
+ List<Vertex> children = new ArrayList<>();
+
+ List<Vertex> parents = new ArrayList<>();
+
+ // ------------------------------------------------------------
+ // Constructors
+ // ------------------------------------------------------------
+
+ public Vertex(final String label) {
+ this.label = label;
+ }
+
+ // ------------------------------------------------------------
+ // Accessors
+ // ------------------------------------------------------------
+
+ public String getLabel() {
+ return label;
+ }
+
+ public void addEdgeTo(final Vertex vertex) {
+ children.add(vertex);
+ }
+
+ public void removeEdgeTo(final Vertex vertex) {
+ children.remove(vertex);
+ }
+
+ public void addEdgeFrom(final Vertex vertex) {
+ parents.add(vertex);
+ }
+
+ public void removeEdgeFrom(final Vertex vertex) {
+ parents.remove(vertex);
+ }
+
+ public List<Vertex> getChildren() {
+ return children;
+ }
+
+ /**
+ * Get the labels used by the most direct children.
+ *
+ * @return the labels used by the most direct children.
Review Comment:
no period, per Oracle guidelines
--
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
[GitHub] [maven] elharo commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1316362150
##########
maven-model-builder/src/main/java/org/apache/maven/utils/Os.java:
##########
@@ -0,0 +1,383 @@
+/*
+ * 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.utils;
+
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Locale;
+import java.util.Set;
+
+/*
+ * This class is copied from maven-shared-utils.
+ *
+ * This class was copied over from Apache ANT.
+ * Even the version from plexus-utils was
+ * only an ANT fork!
+ * The last time it got copied was on 2011-08-12</p>
+ * When merging changes please take care of the special
+ * OS_FAMILY handling in this version of Os.java!
+ */
+
+/**
+ * <p>Condition that tests the OS type.</p>
+ *
+ * @author Stefan Bodewig
+ * @author Magesh Umasankar
+ * @author Brian Fox
+ * @author Mark Struberg
+ */
+public class Os {
+ /**
+ * The OS Name.
+ */
+ public static final String OS_NAME = System.getProperty("os.name").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * The OA architecture.
+ */
+ public static final String OS_ARCH = System.getProperty("os.arch").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * The OS version.
+ */
+ public static final String OS_VERSION = System.getProperty("os.version").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * The path separator.
+ */
+ public static final String PATH_SEP = System.getProperty("path.separator");
+
+ /**
+ * system line separator , e.g. "\n" on unixoid systems and "\r\n" on Windows
+ */
+ public static final String LINE_SEP = System.getProperty("line.separator");
+
+ /**
+ * OS Family
+ */
+ public static final String OS_FAMILY = getOsFamily();
+
+ // store the valid families
+ private static final Set<String> VALID_FAMILIES = getValidFamilies();
+
+ /**
+ * OS family to look for
+ */
+ private String family;
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_WINDOWS = "windows";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_WIN9X = "win9x";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_NT = "winnt";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_OS2 = "os/2";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_NETWARE = "netware";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_DOS = "dos";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_MAC = "mac";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_TANDEM = "tandem";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_UNIX = "unix";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_OPENVMS = "openvms";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_ZOS = "z/os";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_OS400 = "os/400";
+
+ /**
+ * OpenJDK is reported to call MacOS X "Darwin"
+ *
+ * @see <a href="https://issues.apache.org/bugzilla/show_bug.cgi?id=44889">bugzilla issue</a>
+ * @see <a href="https://issues.apache.org/jira/browse/HADOOP-3318">HADOOP-3318</a>
+ */
+ private static final String DARWIN = "darwin";
+
+ /**
+ * The set of valid families. This methods initializes the set until
+ * VALID_FAMILIES constant is set.
+ * @return The set of families.
+ */
+ public static Set<String> getValidFamilies() {
+ if (VALID_FAMILIES != null) {
+ return VALID_FAMILIES;
+ }
+
+ Set<String> valid = new HashSet<>();
+ valid.add(FAMILY_DOS);
+ valid.add(FAMILY_MAC);
+ valid.add(FAMILY_NETWARE);
+ valid.add(FAMILY_NT);
+ valid.add(FAMILY_OPENVMS);
+ valid.add(FAMILY_OS2);
+ valid.add(FAMILY_OS400);
+ valid.add(FAMILY_TANDEM);
+ valid.add(FAMILY_UNIX);
+ valid.add(FAMILY_WIN9X);
+ valid.add(FAMILY_WINDOWS);
+ valid.add(FAMILY_ZOS);
+
+ return Collections.unmodifiableSet(valid);
+ }
+
+ /**
+ * Default constructor
+ */
+ public Os() {
+ // default
+ }
+
+ /**
+ * Constructor that sets the family attribute
+ *
+ * @param family a String value
+ */
+ public Os(String family) {
+ setFamily(family);
+ }
+
+ /**
+ * Sets the desired OS family type
+ *
+ * @param f The OS family type desired<br />
+ * Possible values:<br />
+ * <ul>
+ * <li>dos</li>
+ * <li>mac</li>
+ * <li>netware</li>
+ * <li>os/2</li>
+ * <li>tandem</li>
+ * <li>unix</li>
+ * <li>windows</li>
+ * <li>win9x</li>
+ * <li>z/os</li>
+ * <li>os/400</li>
+ * </ul>
+ */
+ private void setFamily(String f) {
Review Comment:
Noticing a bug now. Any family can be set, valid or not. This class really isn't OK. Move to a separate PR where the changes that involve this can get more attention.
##########
maven-model-builder/src/main/java/org/apache/maven/utils/Os.java:
##########
@@ -0,0 +1,383 @@
+/*
Review Comment:
It's public, not just an implementation detail, so I think more care is warranted. If it can be made non-public it can be cleaned up later, but we should be cautious about locking in APIs we don't need and don't want to support long term.
##########
maven-model-builder/src/main/java/org/apache/maven/utils/Os.java:
##########
@@ -0,0 +1,383 @@
+/*
+ * 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.utils;
+
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Locale;
+import java.util.Set;
+
+/*
+ * This class is copied from maven-shared-utils.
+ *
+ * This class was copied over from Apache ANT.
+ * Even the version from plexus-utils was
+ * only an ANT fork!
+ * The last time it got copied was on 2011-08-12</p>
+ * When merging changes please take care of the special
+ * OS_FAMILY handling in this version of Os.java!
+ */
+
+/**
+ * <p>Condition that tests the OS type.</p>
+ *
+ * @author Stefan Bodewig
+ * @author Magesh Umasankar
+ * @author Brian Fox
+ * @author Mark Struberg
+ */
+public class Os {
+ /**
+ * The OS Name.
+ */
+ public static final String OS_NAME = System.getProperty("os.name").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * The OA architecture.
+ */
+ public static final String OS_ARCH = System.getProperty("os.arch").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * The OS version.
+ */
+ public static final String OS_VERSION = System.getProperty("os.version").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * The path separator.
+ */
+ public static final String PATH_SEP = System.getProperty("path.separator");
+
+ /**
+ * system line separator , e.g. "\n" on unixoid systems and "\r\n" on Windows
+ */
+ public static final String LINE_SEP = System.getProperty("line.separator");
+
+ /**
+ * OS Family
+ */
+ public static final String OS_FAMILY = getOsFamily();
+
+ // store the valid families
+ private static final Set<String> VALID_FAMILIES = getValidFamilies();
+
+ /**
+ * OS family to look for
+ */
+ private String family;
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_WINDOWS = "windows";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_WIN9X = "win9x";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_NT = "winnt";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_OS2 = "os/2";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_NETWARE = "netware";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_DOS = "dos";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_MAC = "mac";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_TANDEM = "tandem";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_UNIX = "unix";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_OPENVMS = "openvms";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_ZOS = "z/os";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_OS400 = "os/400";
+
+ /**
+ * OpenJDK is reported to call MacOS X "Darwin"
+ *
+ * @see <a href="https://issues.apache.org/bugzilla/show_bug.cgi?id=44889">bugzilla issue</a>
+ * @see <a href="https://issues.apache.org/jira/browse/HADOOP-3318">HADOOP-3318</a>
+ */
+ private static final String DARWIN = "darwin";
+
+ /**
+ * The set of valid families. This methods initializes the set until
+ * VALID_FAMILIES constant is set.
+ * @return The set of families.
+ */
+ public static Set<String> getValidFamilies() {
+ if (VALID_FAMILIES != null) {
+ return VALID_FAMILIES;
+ }
+
+ Set<String> valid = new HashSet<>();
+ valid.add(FAMILY_DOS);
+ valid.add(FAMILY_MAC);
+ valid.add(FAMILY_NETWARE);
+ valid.add(FAMILY_NT);
+ valid.add(FAMILY_OPENVMS);
+ valid.add(FAMILY_OS2);
+ valid.add(FAMILY_OS400);
+ valid.add(FAMILY_TANDEM);
+ valid.add(FAMILY_UNIX);
+ valid.add(FAMILY_WIN9X);
+ valid.add(FAMILY_WINDOWS);
+ valid.add(FAMILY_ZOS);
+
+ return Collections.unmodifiableSet(valid);
+ }
+
+ /**
+ * Default constructor
+ */
+ public Os() {
+ // default
+ }
+
+ /**
+ * Constructor that sets the family attribute
+ *
+ * @param family a String value
+ */
+ public Os(String family) {
+ setFamily(family);
+ }
+
+ /**
+ * Sets the desired OS family type
+ *
+ * @param f The OS family type desired<br />
+ * Possible values:<br />
+ * <ul>
+ * <li>dos</li>
+ * <li>mac</li>
+ * <li>netware</li>
+ * <li>os/2</li>
+ * <li>tandem</li>
+ * <li>unix</li>
+ * <li>windows</li>
+ * <li>win9x</li>
+ * <li>z/os</li>
+ * <li>os/400</li>
+ * </ul>
+ */
+ private void setFamily(String f) {
+ family = f.toLowerCase(Locale.ENGLISH);
+ }
+
+ /**
+ * Determines if the OS on which Ant is executing matches the type of
Review Comment:
Comment is wrong
##########
maven-model-builder/src/main/java/org/apache/maven/utils/Os.java:
##########
@@ -0,0 +1,383 @@
+/*
+ * 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.utils;
+
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Locale;
+import java.util.Set;
+
+/*
+ * This class is copied from maven-shared-utils.
+ *
+ * This class was copied over from Apache ANT.
+ * Even the version from plexus-utils was
+ * only an ANT fork!
+ * The last time it got copied was on 2011-08-12</p>
+ * When merging changes please take care of the special
+ * OS_FAMILY handling in this version of Os.java!
+ */
+
+/**
+ * <p>Condition that tests the OS type.</p>
+ *
+ * @author Stefan Bodewig
+ * @author Magesh Umasankar
+ * @author Brian Fox
+ * @author Mark Struberg
+ */
+public class Os {
+ /**
+ * The OS Name.
+ */
+ public static final String OS_NAME = System.getProperty("os.name").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * The OA architecture.
+ */
+ public static final String OS_ARCH = System.getProperty("os.arch").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * The OS version.
+ */
+ public static final String OS_VERSION = System.getProperty("os.version").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * The path separator.
+ */
+ public static final String PATH_SEP = System.getProperty("path.separator");
+
+ /**
+ * system line separator , e.g. "\n" on unixoid systems and "\r\n" on Windows
+ */
+ public static final String LINE_SEP = System.getProperty("line.separator");
+
+ /**
+ * OS Family
+ */
+ public static final String OS_FAMILY = getOsFamily();
+
+ // store the valid families
+ private static final Set<String> VALID_FAMILIES = getValidFamilies();
+
+ /**
+ * OS family to look for
+ */
+ private String family;
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_WINDOWS = "windows";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_WIN9X = "win9x";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_NT = "winnt";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_OS2 = "os/2";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_NETWARE = "netware";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_DOS = "dos";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_MAC = "mac";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_TANDEM = "tandem";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_UNIX = "unix";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_OPENVMS = "openvms";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_ZOS = "z/os";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_OS400 = "os/400";
+
+ /**
+ * OpenJDK is reported to call MacOS X "Darwin"
+ *
+ * @see <a href="https://issues.apache.org/bugzilla/show_bug.cgi?id=44889">bugzilla issue</a>
+ * @see <a href="https://issues.apache.org/jira/browse/HADOOP-3318">HADOOP-3318</a>
+ */
+ private static final String DARWIN = "darwin";
+
+ /**
+ * The set of valid families. This methods initializes the set until
+ * VALID_FAMILIES constant is set.
+ * @return The set of families.
+ */
+ public static Set<String> getValidFamilies() {
+ if (VALID_FAMILIES != null) {
+ return VALID_FAMILIES;
+ }
+
+ Set<String> valid = new HashSet<>();
+ valid.add(FAMILY_DOS);
+ valid.add(FAMILY_MAC);
+ valid.add(FAMILY_NETWARE);
+ valid.add(FAMILY_NT);
+ valid.add(FAMILY_OPENVMS);
+ valid.add(FAMILY_OS2);
+ valid.add(FAMILY_OS400);
+ valid.add(FAMILY_TANDEM);
+ valid.add(FAMILY_UNIX);
+ valid.add(FAMILY_WIN9X);
+ valid.add(FAMILY_WINDOWS);
+ valid.add(FAMILY_ZOS);
+
+ return Collections.unmodifiableSet(valid);
+ }
+
+ /**
+ * Default constructor
+ */
+ public Os() {
+ // default
+ }
+
+ /**
+ * Constructor that sets the family attribute
+ *
+ * @param family a String value
+ */
+ public Os(String family) {
+ setFamily(family);
+ }
+
+ /**
+ * Sets the desired OS family type
+ *
+ * @param f The OS family type desired<br />
+ * Possible values:<br />
+ * <ul>
+ * <li>dos</li>
+ * <li>mac</li>
+ * <li>netware</li>
+ * <li>os/2</li>
+ * <li>tandem</li>
+ * <li>unix</li>
+ * <li>windows</li>
+ * <li>win9x</li>
+ * <li>z/os</li>
+ * <li>os/400</li>
+ * </ul>
+ */
+ private void setFamily(String f) {
+ family = f.toLowerCase(Locale.ENGLISH);
+ }
+
+ /**
+ * Determines if the OS on which Ant is executing matches the type of
+ * that set in setFamily.
+ *
+ * @return true if the os matches.
+ * @see Os#setFamily(String)
+ */
+ boolean eval() {
+ return isOs(family, null, null, null);
+ }
+
+ /**
+ * Determines if the OS on which Ant is executing matches the
+ * given OS family.
+ *
+ * @param family the family to check for
+ * @return true if the OS matches
+ *
+ */
+ public static boolean isFamily(String family) {
+ return isOs(family, null, null, null);
+ }
+
+ /**
+ * Determines if the OS on which Ant is executing matches the
+ * given OS name.
+ *
+ * @param name the OS name to check for
+ * @return true if the OS matches
+ *
+ */
+ public static boolean isName(String name) {
+ return isOs(null, name, null, null);
+ }
+
+ /**
+ * Determines if the OS on which Ant is executing matches the
Review Comment:
and yet again
##########
maven-model-builder/src/main/java/org/apache/maven/utils/Os.java:
##########
@@ -0,0 +1,383 @@
+/*
+ * 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.utils;
+
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Locale;
+import java.util.Set;
+
+/*
+ * This class is copied from maven-shared-utils.
+ *
+ * This class was copied over from Apache ANT.
+ * Even the version from plexus-utils was
+ * only an ANT fork!
+ * The last time it got copied was on 2011-08-12</p>
+ * When merging changes please take care of the special
+ * OS_FAMILY handling in this version of Os.java!
+ */
+
+/**
+ * <p>Condition that tests the OS type.</p>
+ *
+ * @author Stefan Bodewig
+ * @author Magesh Umasankar
+ * @author Brian Fox
+ * @author Mark Struberg
+ */
+public class Os {
+ /**
+ * The OS Name.
+ */
+ public static final String OS_NAME = System.getProperty("os.name").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * The OA architecture.
+ */
+ public static final String OS_ARCH = System.getProperty("os.arch").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * The OS version.
+ */
+ public static final String OS_VERSION = System.getProperty("os.version").toLowerCase(Locale.ENGLISH);
+
+ /**
+ * The path separator.
+ */
+ public static final String PATH_SEP = System.getProperty("path.separator");
+
+ /**
+ * system line separator , e.g. "\n" on unixoid systems and "\r\n" on Windows
+ */
+ public static final String LINE_SEP = System.getProperty("line.separator");
+
+ /**
+ * OS Family
+ */
+ public static final String OS_FAMILY = getOsFamily();
+
+ // store the valid families
+ private static final Set<String> VALID_FAMILIES = getValidFamilies();
+
+ /**
+ * OS family to look for
+ */
+ private String family;
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_WINDOWS = "windows";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_WIN9X = "win9x";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_NT = "winnt";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_OS2 = "os/2";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_NETWARE = "netware";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_DOS = "dos";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_MAC = "mac";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_TANDEM = "tandem";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_UNIX = "unix";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_OPENVMS = "openvms";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_ZOS = "z/os";
+
+ /**
+ * OS family that can be tested for. {@value}
+ */
+ public static final String FAMILY_OS400 = "os/400";
+
+ /**
+ * OpenJDK is reported to call MacOS X "Darwin"
+ *
+ * @see <a href="https://issues.apache.org/bugzilla/show_bug.cgi?id=44889">bugzilla issue</a>
+ * @see <a href="https://issues.apache.org/jira/browse/HADOOP-3318">HADOOP-3318</a>
+ */
+ private static final String DARWIN = "darwin";
+
+ /**
+ * The set of valid families. This methods initializes the set until
+ * VALID_FAMILIES constant is set.
+ * @return The set of families.
+ */
+ public static Set<String> getValidFamilies() {
+ if (VALID_FAMILIES != null) {
+ return VALID_FAMILIES;
+ }
+
+ Set<String> valid = new HashSet<>();
+ valid.add(FAMILY_DOS);
+ valid.add(FAMILY_MAC);
+ valid.add(FAMILY_NETWARE);
+ valid.add(FAMILY_NT);
+ valid.add(FAMILY_OPENVMS);
+ valid.add(FAMILY_OS2);
+ valid.add(FAMILY_OS400);
+ valid.add(FAMILY_TANDEM);
+ valid.add(FAMILY_UNIX);
+ valid.add(FAMILY_WIN9X);
+ valid.add(FAMILY_WINDOWS);
+ valid.add(FAMILY_ZOS);
+
+ return Collections.unmodifiableSet(valid);
+ }
+
+ /**
+ * Default constructor
+ */
+ public Os() {
+ // default
+ }
+
+ /**
+ * Constructor that sets the family attribute
+ *
+ * @param family a String value
+ */
+ public Os(String family) {
+ setFamily(family);
+ }
+
+ /**
+ * Sets the desired OS family type
+ *
+ * @param f The OS family type desired<br />
+ * Possible values:<br />
+ * <ul>
+ * <li>dos</li>
+ * <li>mac</li>
+ * <li>netware</li>
+ * <li>os/2</li>
+ * <li>tandem</li>
+ * <li>unix</li>
+ * <li>windows</li>
+ * <li>win9x</li>
+ * <li>z/os</li>
+ * <li>os/400</li>
+ * </ul>
+ */
+ private void setFamily(String f) {
+ family = f.toLowerCase(Locale.ENGLISH);
+ }
+
+ /**
+ * Determines if the OS on which Ant is executing matches the type of
+ * that set in setFamily.
+ *
+ * @return true if the os matches.
+ * @see Os#setFamily(String)
+ */
+ boolean eval() {
+ return isOs(family, null, null, null);
+ }
+
+ /**
+ * Determines if the OS on which Ant is executing matches the
Review Comment:
Ant, again
--
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
[GitHub] [maven] CrazyHZM commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "CrazyHZM (via GitHub)" <gi...@apache.org>.
CrazyHZM commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1305099465
##########
maven-core/src/main/java/org/apache/maven/utils/dag/CycleDetector.java:
##########
@@ -0,0 +1,136 @@
+/*
+ * 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.utils.dag;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * @author <a href="michal.maczka@dimatics.com">Michal Maczka</a>
+ *
+ */
+public class CycleDetector {
+
+ private static final Integer NOT_VISITED = 0;
+
+ private static final Integer VISITING = 1;
+
+ private static final Integer VISITED = 2;
+
+ public static List<String> hasCycle(final DAG graph) {
+ final List<Vertex> vertices = graph.getVertices();
+
+ final Map<Vertex, Integer> vertexStateMap = new HashMap<>();
+
+ List<String> retValue = null;
+
+ for (Vertex vertex : vertices) {
+ if (isNotVisited(vertex, vertexStateMap)) {
+ retValue = introducesCycle(vertex, vertexStateMap);
+
+ if (retValue != null) {
+ break;
+ }
+ }
+ }
+
+ return retValue;
+ }
+
+ /**
+ * This method will be called when an edge leading to given vertex was added and we want to check if introduction of
Review Comment:
Done.
--
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
[GitHub] [maven] elharo commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1274774380
##########
maven-compat/src/main/java/org/apache/maven/repository/legacy/DefaultWagonManager.java:
##########
@@ -465,6 +469,28 @@ public void getRemoteFile(
}
}
+ private void copyFile(File source, File destination) throws IOException {
+ String message;
Review Comment:
declare where used, and don't reuse for two different messages, or just inline the two uses of this variable
--
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
[GitHub] [maven] gnodet commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1305616342
##########
maven-compat/src/main/java/org/apache/maven/profiles/activation/OperatingSystemProfileActivator.java:
##########
@@ -122,12 +153,54 @@ private boolean determineFamilyMatch(String family) {
test = test.substring(1);
}
- boolean result = Os.isFamily(test);
+ boolean result = isFamily(test);
if (reverse) {
return !result;
} else {
return result;
}
}
+
+ private boolean isFamily(String family) {
Review Comment:
Mmh, could the reason be that you're considering this code as new code, whereas I consider it as a code move from plexus-utils ? If it was good to be used there, I assume it's good to be used in maven too as-is...
So which ones do you want to remove support for exactly ?
--
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
[GitHub] [maven] CrazyHZM commented on pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "CrazyHZM (via GitHub)" <gi...@apache.org>.
CrazyHZM commented on PR #1208:
URL: https://github.com/apache/maven/pull/1208#issuecomment-1694547753
>
@gnodet I remigrated the Os 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
[GitHub] [maven] elharo commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1322972137
##########
maven-core/src/main/java/org/apache/maven/toolchain/java/JavaToolchainFactory.java:
##########
@@ -86,9 +86,9 @@ public ToolchainPrivate createToolchain(ToolchainModel model) throws Misconfigur
throw new MisconfiguredToolchainException(
"Java toolchain without the " + JavaToolchainImpl.KEY_JAVAHOME + " configuration element.");
}
- File normal = new File(FileUtils.normalize(javahome.getValue()));
+ File normal = Paths.get(javahome.getValue()).normalize().toFile();
Review Comment:
No need to convert to a File; just use Files.exists(path) below
##########
maven-core/src/main/java/org/apache/maven/utils/introspection/ReflectionValueExtractor.java:
##########
@@ -0,0 +1,325 @@
+/*
+ * 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.utils.introspection;
+
+import java.lang.ref.WeakReference;
+import java.lang.reflect.Array;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.util.List;
+import java.util.Map;
+import java.util.WeakHashMap;
+
+/**
+ * <p>
+ * Using simple dotted expressions to extract the values from an Object instance, For example we might want to extract a
Review Comment:
run on sentence
##########
maven-core/src/main/java/org/apache/maven/utils/introspection/ReflectionValueExtractor.java:
##########
@@ -0,0 +1,325 @@
+/*
+ * 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.utils.introspection;
+
+import java.lang.ref.WeakReference;
+import java.lang.reflect.Array;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.util.List;
+import java.util.Map;
+import java.util.WeakHashMap;
+
+/**
+ * <p>
+ * Using simple dotted expressions to extract the values from an Object instance, For example we might want to extract a
+ * value like: <code>project.build.sourceDirectory</code>
+ * </p>
+ * <p>
+ * The implementation supports indexed, nested and mapped properties similar to the JSP way.
+ * </p>
+ *
+ * @author <a href="mailto:jason@maven.org">Jason van Zyl </a>
+ * @author <a href="mailto:vincent.siveton@gmail.com">Vincent Siveton</a>
+ *
+ * @see <a href=
+ * "http://struts.apache.org/1.x/struts-taglib/indexedprops.html">http://struts.apache.org/1.x/struts-taglib/indexedprops.html</a>
+ */
+public class ReflectionValueExtractor {
+ private static final Class<?>[] CLASS_ARGS = new Class[0];
+
+ private static final Object[] OBJECT_ARGS = new Object[0];
+
+ /**
+ * Use a WeakHashMap here, so the keys (Class objects) can be garbage collected. This approach prevents permgen
+ * space overflows due to retention of discarded classloaders.
+ */
+ private static final Map<Class<?>, WeakReference<ClassMap>> CLASS_MAPS = new WeakHashMap<>();
+
+ static final int EOF = -1;
+
+ static final char PROPERTY_START = '.';
+
+ static final char INDEXED_START = '[';
+
+ static final char INDEXED_END = ']';
+
+ static final char MAPPED_START = '(';
+
+ static final char MAPPED_END = ')';
+
+ static class Tokenizer {
+ final String expression;
+
+ int idx;
+
+ Tokenizer(String expression) {
+ this.expression = expression;
+ }
+
+ public int peekChar() {
+ return idx < expression.length() ? expression.charAt(idx) : EOF;
+ }
+
+ public int skipChar() {
+ return idx < expression.length() ? expression.charAt(idx++) : EOF;
+ }
+
+ public String nextToken(char delimiter) {
+ int start = idx;
+
+ while (idx < expression.length() && delimiter != expression.charAt(idx)) {
+ idx++;
+ }
+
+ // delimiter MUST be present
+ if (idx <= start || idx >= expression.length()) {
+ return null;
+ }
+
+ return expression.substring(start, idx++);
+ }
+
+ public String nextPropertyName() {
+ final int start = idx;
+
+ while (idx < expression.length() && Character.isJavaIdentifierPart(expression.charAt(idx))) {
+ idx++;
+ }
+
+ // property name does not require delimiter
+ if (idx <= start || idx > expression.length()) {
+ return null;
+ }
+
+ return expression.substring(start, idx);
+ }
+
+ public int getPosition() {
+ return idx < expression.length() ? idx : EOF;
+ }
+
+ // to make tokenizer look pretty in debugger
+ @Override
+ public String toString() {
+ return idx < expression.length() ? expression.substring(idx) : "<EOF>";
+ }
+ }
+
+ private ReflectionValueExtractor() {}
+
+ /**
+ * <p>The implementation supports indexed, nested and mapped properties.</p>
+ *
+ * <ul>
+ * <li>nested properties should be defined by a dot, i.e. "user.address.street"</li>
+ * <li>indexed properties (java.util.List or array instance) should be contains <code>(\\w+)\\[(\\d+)\\]</code>
+ * pattern, i.e. "user.addresses[1].street"</li>
+ * <li>mapped properties should be contains <code>(\\w+)\\((.+)\\)</code> pattern, i.e.
+ * "user.addresses(myAddress).street"</li>
+ * </ul>
+ *
+ * @param expression not null expression
+ * @param root not null object
+ * @return the object defined by the expression
+ */
+ public static Object evaluate(String expression, Object root) {
+ return evaluate(expression, root, true);
+ }
+
+ /**
+ * <p>The implementation supports indexed, nested and mapped properties.</p>
+ *
+ * <ul>
+ * <li>nested properties should be defined by a dot, i.e. "user.address.street"</li>
+ * <li>indexed properties (java.util.List or array instance) should be contains <code>(\\w+)\\[(\\d+)\\]</code>
+ * pattern, i.e. "user.addresses[1].street"</li>
+ * <li>mapped properties should be contains <code>(\\w+)\\((.+)\\)</code> pattern, i.e.
+ * "user.addresses(myAddress).street"</li>
+ * </ul>
+ *
+ * @param expression not null expression
+ * @param root not null object
+ * @param trimRootToken root start
+ * @return the object defined by the expression
+ */
+ public static Object evaluate(String expression, final Object root, final boolean trimRootToken) {
+ Object value = root;
+
+ // ----------------------------------------------------------------------
+ // Walk the dots and retrieve the ultimate value desired from the
+ // MavenProject instance.
+ // ----------------------------------------------------------------------
+
+ if (expression == null || expression.isEmpty() || !Character.isJavaIdentifierStart(expression.charAt(0))) {
+ return null;
+ }
+
+ boolean hasDots = expression.indexOf(PROPERTY_START) >= 0;
+
+ final Tokenizer tokenizer;
+ if (trimRootToken && hasDots) {
+ tokenizer = new Tokenizer(expression);
+ tokenizer.nextPropertyName();
+ if (tokenizer.getPosition() == EOF) {
+ return null;
+ }
+ } else {
+ tokenizer = new Tokenizer("." + expression);
+ }
+
+ int propertyPosition = tokenizer.getPosition();
+ while (value != null && tokenizer.peekChar() != EOF) {
+ switch (tokenizer.skipChar()) {
+ case INDEXED_START:
+ value = getIndexedValue(
+ expression,
+ propertyPosition,
+ tokenizer.getPosition(),
+ value,
+ tokenizer.nextToken(INDEXED_END));
+ break;
+ case MAPPED_START:
+ value = getMappedValue(
+ expression,
+ propertyPosition,
+ tokenizer.getPosition(),
+ value,
+ tokenizer.nextToken(MAPPED_END));
+ break;
+ case PROPERTY_START:
+ propertyPosition = tokenizer.getPosition();
+ value = getPropertyValue(value, tokenizer.nextPropertyName());
+ break;
+ default:
+ // could not parse expression
Review Comment:
belongs in javadoc
##########
maven-core/src/main/java/org/apache/maven/utils/introspection/ReflectionValueExtractor.java:
##########
@@ -0,0 +1,325 @@
+/*
+ * 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.utils.introspection;
+
+import java.lang.ref.WeakReference;
+import java.lang.reflect.Array;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.util.List;
+import java.util.Map;
+import java.util.WeakHashMap;
+
+/**
+ * <p>
+ * Using simple dotted expressions to extract the values from an Object instance, For example we might want to extract a
+ * value like: <code>project.build.sourceDirectory</code>
+ * </p>
+ * <p>
+ * The implementation supports indexed, nested and mapped properties similar to the JSP way.
+ * </p>
+ *
+ * @author <a href="mailto:jason@maven.org">Jason van Zyl </a>
+ * @author <a href="mailto:vincent.siveton@gmail.com">Vincent Siveton</a>
+ *
+ * @see <a href=
+ * "http://struts.apache.org/1.x/struts-taglib/indexedprops.html">http://struts.apache.org/1.x/struts-taglib/indexedprops.html</a>
+ */
+public class ReflectionValueExtractor {
+ private static final Class<?>[] CLASS_ARGS = new Class[0];
+
+ private static final Object[] OBJECT_ARGS = new Object[0];
+
+ /**
+ * Use a WeakHashMap here, so the keys (Class objects) can be garbage collected. This approach prevents permgen
+ * space overflows due to retention of discarded classloaders.
+ */
+ private static final Map<Class<?>, WeakReference<ClassMap>> CLASS_MAPS = new WeakHashMap<>();
+
+ static final int EOF = -1;
+
+ static final char PROPERTY_START = '.';
+
+ static final char INDEXED_START = '[';
+
+ static final char INDEXED_END = ']';
+
+ static final char MAPPED_START = '(';
+
+ static final char MAPPED_END = ')';
+
+ static class Tokenizer {
+ final String expression;
+
+ int idx;
+
+ Tokenizer(String expression) {
+ this.expression = expression;
+ }
+
+ public int peekChar() {
+ return idx < expression.length() ? expression.charAt(idx) : EOF;
+ }
+
+ public int skipChar() {
+ return idx < expression.length() ? expression.charAt(idx++) : EOF;
+ }
+
+ public String nextToken(char delimiter) {
+ int start = idx;
+
+ while (idx < expression.length() && delimiter != expression.charAt(idx)) {
+ idx++;
+ }
+
+ // delimiter MUST be present
+ if (idx <= start || idx >= expression.length()) {
+ return null;
+ }
+
+ return expression.substring(start, idx++);
+ }
+
+ public String nextPropertyName() {
+ final int start = idx;
+
+ while (idx < expression.length() && Character.isJavaIdentifierPart(expression.charAt(idx))) {
+ idx++;
+ }
+
+ // property name does not require delimiter
+ if (idx <= start || idx > expression.length()) {
+ return null;
+ }
+
+ return expression.substring(start, idx);
+ }
+
+ public int getPosition() {
+ return idx < expression.length() ? idx : EOF;
+ }
+
+ // to make tokenizer look pretty in debugger
+ @Override
+ public String toString() {
+ return idx < expression.length() ? expression.substring(idx) : "<EOF>";
+ }
+ }
+
+ private ReflectionValueExtractor() {}
+
+ /**
+ * <p>The implementation supports indexed, nested and mapped properties.</p>
+ *
+ * <ul>
+ * <li>nested properties should be defined by a dot, i.e. "user.address.street"</li>
+ * <li>indexed properties (java.util.List or array instance) should be contains <code>(\\w+)\\[(\\d+)\\]</code>
+ * pattern, i.e. "user.addresses[1].street"</li>
+ * <li>mapped properties should be contains <code>(\\w+)\\((.+)\\)</code> pattern, i.e.
+ * "user.addresses(myAddress).street"</li>
+ * </ul>
+ *
+ * @param expression not null expression
+ * @param root not null object
+ * @return the object defined by the expression
+ */
+ public static Object evaluate(String expression, Object root) {
+ return evaluate(expression, root, true);
+ }
+
+ /**
+ * <p>The implementation supports indexed, nested and mapped properties.</p>
+ *
+ * <ul>
+ * <li>nested properties should be defined by a dot, i.e. "user.address.street"</li>
+ * <li>indexed properties (java.util.List or array instance) should be contains <code>(\\w+)\\[(\\d+)\\]</code>
+ * pattern, i.e. "user.addresses[1].street"</li>
+ * <li>mapped properties should be contains <code>(\\w+)\\((.+)\\)</code> pattern, i.e.
+ * "user.addresses(myAddress).street"</li>
+ * </ul>
+ *
+ * @param expression not null expression
+ * @param root not null object
+ * @param trimRootToken root start
+ * @return the object defined by the expression
+ */
+ public static Object evaluate(String expression, final Object root, final boolean trimRootToken) {
+ Object value = root;
+
+ // ----------------------------------------------------------------------
+ // Walk the dots and retrieve the ultimate value desired from the
+ // MavenProject instance.
+ // ----------------------------------------------------------------------
+
+ if (expression == null || expression.isEmpty() || !Character.isJavaIdentifierStart(expression.charAt(0))) {
+ return null;
+ }
+
+ boolean hasDots = expression.indexOf(PROPERTY_START) >= 0;
+
+ final Tokenizer tokenizer;
+ if (trimRootToken && hasDots) {
+ tokenizer = new Tokenizer(expression);
+ tokenizer.nextPropertyName();
+ if (tokenizer.getPosition() == EOF) {
+ return null;
+ }
+ } else {
+ tokenizer = new Tokenizer("." + expression);
+ }
+
+ int propertyPosition = tokenizer.getPosition();
+ while (value != null && tokenizer.peekChar() != EOF) {
+ switch (tokenizer.skipChar()) {
+ case INDEXED_START:
+ value = getIndexedValue(
+ expression,
+ propertyPosition,
+ tokenizer.getPosition(),
+ value,
+ tokenizer.nextToken(INDEXED_END));
+ break;
+ case MAPPED_START:
+ value = getMappedValue(
+ expression,
+ propertyPosition,
+ tokenizer.getPosition(),
+ value,
+ tokenizer.nextToken(MAPPED_END));
+ break;
+ case PROPERTY_START:
+ propertyPosition = tokenizer.getPosition();
+ value = getPropertyValue(value, tokenizer.nextPropertyName());
+ break;
+ default:
+ // could not parse expression
+ return null;
+ }
+ }
+
+ return value;
+ }
+
+ private static Object getMappedValue(
+ final String expression, final int from, final int to, final Object value, final String key) {
+ if (value == null || key == null) {
+ return null;
+ }
+ if (value instanceof Map) {
+ try {
+ Object[] localParams = new Object[] {key};
+ ClassMap classMap = getClassMap(value.getClass());
+ Method method = classMap.findMethod("get", localParams);
+ return method.invoke(value, localParams);
+ } catch (Exception e) {
+ throw new RuntimeException(e);
+ }
+ } else {
+ final String message = String.format(
+ "The token '%s' at position '%d' refers to a java.util.Map, but the value seems is an instance of '%s'",
+ expression.subSequence(from, to), from, value.getClass());
+ throw new IllegalArgumentException(message);
+ }
+ }
+
+ private static Object getIndexedValue(
+ final String expression, final int from, final int to, final Object value, final String indexStr) {
+ int index;
+ try {
+ index = Integer.parseInt(indexStr);
+
+ if (value.getClass().isArray()) {
+ return Array.get(value, index);
+ }
+
+ } catch (NumberFormatException e) {
+ final String message = String.format("The indexStr '%s' is not a number", indexStr);
+ throw new IllegalArgumentException(message);
+ }
+
+ if (value instanceof List) {
+ try {
+ ClassMap classMap = getClassMap(value.getClass());
+ // use get method on List interface
+ Object[] localParams = new Object[] {index};
+ Method method = classMap.findMethod("get", localParams);
+ return method.invoke(value, localParams);
+ } catch (Exception e) {
+ throw new RuntimeException(e);
+ }
+ } else {
+ final String message = String.format(
+ "The token '%s' at position '%d' refers to a java.util.List or an array, but the value seems is an instance of '%s'",
+ expression.subSequence(from, to), from, value.getClass());
+ throw new IllegalArgumentException(message);
+ }
+ }
+
+ private static Object getPropertyValue(Object value, String property) {
+ if (value == null || property == null) {
+ return null;
+ }
+
+ ClassMap classMap = getClassMap(value.getClass());
+ Method method;
Review Comment:
declare in try block
##########
maven-core/src/main/java/org/apache/maven/utils/introspection/ReflectionValueExtractor.java:
##########
@@ -0,0 +1,325 @@
+/*
+ * 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.utils.introspection;
+
+import java.lang.ref.WeakReference;
+import java.lang.reflect.Array;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.util.List;
+import java.util.Map;
+import java.util.WeakHashMap;
+
+/**
+ * <p>
+ * Using simple dotted expressions to extract the values from an Object instance, For example we might want to extract a
+ * value like: <code>project.build.sourceDirectory</code>
+ * </p>
+ * <p>
+ * The implementation supports indexed, nested and mapped properties similar to the JSP way.
+ * </p>
+ *
+ * @author <a href="mailto:jason@maven.org">Jason van Zyl </a>
+ * @author <a href="mailto:vincent.siveton@gmail.com">Vincent Siveton</a>
+ *
+ * @see <a href=
+ * "http://struts.apache.org/1.x/struts-taglib/indexedprops.html">http://struts.apache.org/1.x/struts-taglib/indexedprops.html</a>
+ */
+public class ReflectionValueExtractor {
+ private static final Class<?>[] CLASS_ARGS = new Class[0];
+
+ private static final Object[] OBJECT_ARGS = new Object[0];
+
+ /**
+ * Use a WeakHashMap here, so the keys (Class objects) can be garbage collected. This approach prevents permgen
+ * space overflows due to retention of discarded classloaders.
+ */
+ private static final Map<Class<?>, WeakReference<ClassMap>> CLASS_MAPS = new WeakHashMap<>();
+
+ static final int EOF = -1;
+
+ static final char PROPERTY_START = '.';
+
+ static final char INDEXED_START = '[';
+
+ static final char INDEXED_END = ']';
+
+ static final char MAPPED_START = '(';
+
+ static final char MAPPED_END = ')';
+
+ static class Tokenizer {
+ final String expression;
+
+ int idx;
+
+ Tokenizer(String expression) {
+ this.expression = expression;
+ }
+
+ public int peekChar() {
+ return idx < expression.length() ? expression.charAt(idx) : EOF;
+ }
+
+ public int skipChar() {
+ return idx < expression.length() ? expression.charAt(idx++) : EOF;
+ }
+
+ public String nextToken(char delimiter) {
+ int start = idx;
+
+ while (idx < expression.length() && delimiter != expression.charAt(idx)) {
+ idx++;
+ }
+
+ // delimiter MUST be present
+ if (idx <= start || idx >= expression.length()) {
+ return null;
+ }
+
+ return expression.substring(start, idx++);
+ }
+
+ public String nextPropertyName() {
+ final int start = idx;
+
+ while (idx < expression.length() && Character.isJavaIdentifierPart(expression.charAt(idx))) {
+ idx++;
+ }
+
+ // property name does not require delimiter
+ if (idx <= start || idx > expression.length()) {
+ return null;
+ }
+
+ return expression.substring(start, idx);
+ }
+
+ public int getPosition() {
+ return idx < expression.length() ? idx : EOF;
+ }
+
+ // to make tokenizer look pretty in debugger
+ @Override
+ public String toString() {
+ return idx < expression.length() ? expression.substring(idx) : "<EOF>";
+ }
+ }
+
+ private ReflectionValueExtractor() {}
+
+ /**
+ * <p>The implementation supports indexed, nested and mapped properties.</p>
+ *
+ * <ul>
+ * <li>nested properties should be defined by a dot, i.e. "user.address.street"</li>
+ * <li>indexed properties (java.util.List or array instance) should be contains <code>(\\w+)\\[(\\d+)\\]</code>
+ * pattern, i.e. "user.addresses[1].street"</li>
+ * <li>mapped properties should be contains <code>(\\w+)\\((.+)\\)</code> pattern, i.e.
+ * "user.addresses(myAddress).street"</li>
+ * </ul>
+ *
+ * @param expression not null expression
+ * @param root not null object
+ * @return the object defined by the expression
+ */
+ public static Object evaluate(String expression, Object root) {
+ return evaluate(expression, root, true);
+ }
+
+ /**
+ * <p>The implementation supports indexed, nested and mapped properties.</p>
+ *
+ * <ul>
+ * <li>nested properties should be defined by a dot, i.e. "user.address.street"</li>
+ * <li>indexed properties (java.util.List or array instance) should be contains <code>(\\w+)\\[(\\d+)\\]</code>
+ * pattern, i.e. "user.addresses[1].street"</li>
+ * <li>mapped properties should be contains <code>(\\w+)\\((.+)\\)</code> pattern, i.e.
+ * "user.addresses(myAddress).street"</li>
+ * </ul>
+ *
+ * @param expression not null expression
+ * @param root not null object
+ * @param trimRootToken root start
+ * @return the object defined by the expression
+ */
+ public static Object evaluate(String expression, final Object root, final boolean trimRootToken) {
+ Object value = root;
+
+ // ----------------------------------------------------------------------
+ // Walk the dots and retrieve the ultimate value desired from the
+ // MavenProject instance.
+ // ----------------------------------------------------------------------
+
+ if (expression == null || expression.isEmpty() || !Character.isJavaIdentifierStart(expression.charAt(0))) {
+ return null;
+ }
+
+ boolean hasDots = expression.indexOf(PROPERTY_START) >= 0;
+
+ final Tokenizer tokenizer;
+ if (trimRootToken && hasDots) {
+ tokenizer = new Tokenizer(expression);
+ tokenizer.nextPropertyName();
+ if (tokenizer.getPosition() == EOF) {
+ return null;
+ }
+ } else {
+ tokenizer = new Tokenizer("." + expression);
+ }
+
+ int propertyPosition = tokenizer.getPosition();
+ while (value != null && tokenizer.peekChar() != EOF) {
+ switch (tokenizer.skipChar()) {
+ case INDEXED_START:
+ value = getIndexedValue(
+ expression,
+ propertyPosition,
+ tokenizer.getPosition(),
+ value,
+ tokenizer.nextToken(INDEXED_END));
+ break;
+ case MAPPED_START:
+ value = getMappedValue(
+ expression,
+ propertyPosition,
+ tokenizer.getPosition(),
+ value,
+ tokenizer.nextToken(MAPPED_END));
+ break;
+ case PROPERTY_START:
+ propertyPosition = tokenizer.getPosition();
+ value = getPropertyValue(value, tokenizer.nextPropertyName());
+ break;
+ default:
+ // could not parse expression
+ return null;
+ }
+ }
+
+ return value;
+ }
+
+ private static Object getMappedValue(
+ final String expression, final int from, final int to, final Object value, final String key) {
+ if (value == null || key == null) {
+ return null;
+ }
+ if (value instanceof Map) {
+ try {
+ Object[] localParams = new Object[] {key};
+ ClassMap classMap = getClassMap(value.getClass());
+ Method method = classMap.findMethod("get", localParams);
+ return method.invoke(value, localParams);
+ } catch (Exception e) {
+ throw new RuntimeException(e);
+ }
+ } else {
+ final String message = String.format(
+ "The token '%s' at position '%d' refers to a java.util.Map, but the value seems is an instance of '%s'",
+ expression.subSequence(from, to), from, value.getClass());
+ throw new IllegalArgumentException(message);
+ }
+ }
+
+ private static Object getIndexedValue(
+ final String expression, final int from, final int to, final Object value, final String indexStr) {
+ int index;
+ try {
+ index = Integer.parseInt(indexStr);
+
+ if (value.getClass().isArray()) {
+ return Array.get(value, index);
+ }
+
+ } catch (NumberFormatException e) {
+ final String message = String.format("The indexStr '%s' is not a number", indexStr);
+ throw new IllegalArgumentException(message);
+ }
+
+ if (value instanceof List) {
+ try {
+ ClassMap classMap = getClassMap(value.getClass());
+ // use get method on List interface
+ Object[] localParams = new Object[] {index};
+ Method method = classMap.findMethod("get", localParams);
+ return method.invoke(value, localParams);
+ } catch (Exception e) {
+ throw new RuntimeException(e);
+ }
+ } else {
+ final String message = String.format(
+ "The token '%s' at position '%d' refers to a java.util.List or an array, but the value seems is an instance of '%s'",
Review Comment:
is --> to be
##########
maven-core/src/main/java/org/apache/maven/toolchain/java/JavaToolchainImpl.java:
##########
@@ -53,7 +53,7 @@ public String toString() {
}
public String findTool(String toolName) {
- File toRet = findTool(toolName, new File(FileUtils.normalize(getJavaHome())));
+ File toRet = findTool(toolName, Paths.get(getJavaHome()).normalize().toFile());
Review Comment:
Use Path.toAbsolutePath().toString() below and don't convert to a File here
##########
maven-core/src/main/java/org/apache/maven/utils/introspection/ReflectionValueExtractor.java:
##########
@@ -0,0 +1,325 @@
+/*
+ * 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.utils.introspection;
+
+import java.lang.ref.WeakReference;
+import java.lang.reflect.Array;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.util.List;
+import java.util.Map;
+import java.util.WeakHashMap;
+
+/**
+ * <p>
+ * Using simple dotted expressions to extract the values from an Object instance, For example we might want to extract a
+ * value like: <code>project.build.sourceDirectory</code>
+ * </p>
+ * <p>
+ * The implementation supports indexed, nested and mapped properties similar to the JSP way.
+ * </p>
+ *
+ * @author <a href="mailto:jason@maven.org">Jason van Zyl </a>
+ * @author <a href="mailto:vincent.siveton@gmail.com">Vincent Siveton</a>
+ *
+ * @see <a href=
+ * "http://struts.apache.org/1.x/struts-taglib/indexedprops.html">http://struts.apache.org/1.x/struts-taglib/indexedprops.html</a>
+ */
+public class ReflectionValueExtractor {
+ private static final Class<?>[] CLASS_ARGS = new Class[0];
+
+ private static final Object[] OBJECT_ARGS = new Object[0];
+
+ /**
+ * Use a WeakHashMap here, so the keys (Class objects) can be garbage collected. This approach prevents permgen
+ * space overflows due to retention of discarded classloaders.
+ */
+ private static final Map<Class<?>, WeakReference<ClassMap>> CLASS_MAPS = new WeakHashMap<>();
+
+ static final int EOF = -1;
+
+ static final char PROPERTY_START = '.';
+
+ static final char INDEXED_START = '[';
+
+ static final char INDEXED_END = ']';
+
+ static final char MAPPED_START = '(';
+
+ static final char MAPPED_END = ')';
+
+ static class Tokenizer {
+ final String expression;
+
+ int idx;
+
+ Tokenizer(String expression) {
+ this.expression = expression;
+ }
+
+ public int peekChar() {
Review Comment:
don't need public methods in a non-public class
##########
maven-core/src/main/java/org/apache/maven/utils/introspection/ReflectionValueExtractor.java:
##########
@@ -0,0 +1,325 @@
+/*
+ * 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.utils.introspection;
+
+import java.lang.ref.WeakReference;
+import java.lang.reflect.Array;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.util.List;
+import java.util.Map;
+import java.util.WeakHashMap;
+
+/**
+ * <p>
+ * Using simple dotted expressions to extract the values from an Object instance, For example we might want to extract a
+ * value like: <code>project.build.sourceDirectory</code>
+ * </p>
+ * <p>
+ * The implementation supports indexed, nested and mapped properties similar to the JSP way.
+ * </p>
+ *
+ * @author <a href="mailto:jason@maven.org">Jason van Zyl </a>
+ * @author <a href="mailto:vincent.siveton@gmail.com">Vincent Siveton</a>
+ *
+ * @see <a href=
+ * "http://struts.apache.org/1.x/struts-taglib/indexedprops.html">http://struts.apache.org/1.x/struts-taglib/indexedprops.html</a>
+ */
+public class ReflectionValueExtractor {
+ private static final Class<?>[] CLASS_ARGS = new Class[0];
+
+ private static final Object[] OBJECT_ARGS = new Object[0];
+
+ /**
+ * Use a WeakHashMap here, so the keys (Class objects) can be garbage collected. This approach prevents permgen
+ * space overflows due to retention of discarded classloaders.
+ */
+ private static final Map<Class<?>, WeakReference<ClassMap>> CLASS_MAPS = new WeakHashMap<>();
+
+ static final int EOF = -1;
Review Comment:
Can these all be private?
##########
maven-core/src/main/java/org/apache/maven/utils/introspection/ReflectionValueExtractor.java:
##########
@@ -0,0 +1,325 @@
+/*
+ * 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.utils.introspection;
+
+import java.lang.ref.WeakReference;
+import java.lang.reflect.Array;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.util.List;
+import java.util.Map;
+import java.util.WeakHashMap;
+
+/**
+ * <p>
+ * Using simple dotted expressions to extract the values from an Object instance, For example we might want to extract a
+ * value like: <code>project.build.sourceDirectory</code>
+ * </p>
+ * <p>
+ * The implementation supports indexed, nested and mapped properties similar to the JSP way.
+ * </p>
+ *
+ * @author <a href="mailto:jason@maven.org">Jason van Zyl </a>
+ * @author <a href="mailto:vincent.siveton@gmail.com">Vincent Siveton</a>
+ *
+ * @see <a href=
+ * "http://struts.apache.org/1.x/struts-taglib/indexedprops.html">http://struts.apache.org/1.x/struts-taglib/indexedprops.html</a>
Review Comment:
link is 404
##########
maven-core/src/main/java/org/apache/maven/utils/introspection/ReflectionValueExtractor.java:
##########
@@ -0,0 +1,325 @@
+/*
Review Comment:
same possible copyright issues as the DAG classes
##########
maven-core/src/main/java/org/apache/maven/utils/introspection/ReflectionValueExtractor.java:
##########
@@ -0,0 +1,325 @@
+/*
+ * 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.utils.introspection;
+
+import java.lang.ref.WeakReference;
+import java.lang.reflect.Array;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.util.List;
+import java.util.Map;
+import java.util.WeakHashMap;
+
+/**
+ * <p>
+ * Using simple dotted expressions to extract the values from an Object instance, For example we might want to extract a
+ * value like: <code>project.build.sourceDirectory</code>
+ * </p>
+ * <p>
+ * The implementation supports indexed, nested and mapped properties similar to the JSP way.
+ * </p>
+ *
+ * @author <a href="mailto:jason@maven.org">Jason van Zyl </a>
+ * @author <a href="mailto:vincent.siveton@gmail.com">Vincent Siveton</a>
+ *
+ * @see <a href=
+ * "http://struts.apache.org/1.x/struts-taglib/indexedprops.html">http://struts.apache.org/1.x/struts-taglib/indexedprops.html</a>
+ */
+public class ReflectionValueExtractor {
+ private static final Class<?>[] CLASS_ARGS = new Class[0];
+
+ private static final Object[] OBJECT_ARGS = new Object[0];
+
+ /**
+ * Use a WeakHashMap here, so the keys (Class objects) can be garbage collected. This approach prevents permgen
+ * space overflows due to retention of discarded classloaders.
+ */
+ private static final Map<Class<?>, WeakReference<ClassMap>> CLASS_MAPS = new WeakHashMap<>();
+
+ static final int EOF = -1;
+
+ static final char PROPERTY_START = '.';
+
+ static final char INDEXED_START = '[';
+
+ static final char INDEXED_END = ']';
+
+ static final char MAPPED_START = '(';
+
+ static final char MAPPED_END = ')';
+
+ static class Tokenizer {
+ final String expression;
+
+ int idx;
+
+ Tokenizer(String expression) {
+ this.expression = expression;
+ }
+
+ public int peekChar() {
+ return idx < expression.length() ? expression.charAt(idx) : EOF;
+ }
+
+ public int skipChar() {
+ return idx < expression.length() ? expression.charAt(idx++) : EOF;
+ }
+
+ public String nextToken(char delimiter) {
+ int start = idx;
+
+ while (idx < expression.length() && delimiter != expression.charAt(idx)) {
+ idx++;
+ }
+
+ // delimiter MUST be present
+ if (idx <= start || idx >= expression.length()) {
+ return null;
+ }
+
+ return expression.substring(start, idx++);
+ }
+
+ public String nextPropertyName() {
+ final int start = idx;
+
+ while (idx < expression.length() && Character.isJavaIdentifierPart(expression.charAt(idx))) {
+ idx++;
+ }
+
+ // property name does not require delimiter
+ if (idx <= start || idx > expression.length()) {
+ return null;
+ }
+
+ return expression.substring(start, idx);
+ }
+
+ public int getPosition() {
+ return idx < expression.length() ? idx : EOF;
+ }
+
+ // to make tokenizer look pretty in debugger
+ @Override
+ public String toString() {
+ return idx < expression.length() ? expression.substring(idx) : "<EOF>";
+ }
+ }
+
+ private ReflectionValueExtractor() {}
+
+ /**
+ * <p>The implementation supports indexed, nested and mapped properties.</p>
+ *
+ * <ul>
+ * <li>nested properties should be defined by a dot, i.e. "user.address.street"</li>
+ * <li>indexed properties (java.util.List or array instance) should be contains <code>(\\w+)\\[(\\d+)\\]</code>
+ * pattern, i.e. "user.addresses[1].street"</li>
+ * <li>mapped properties should be contains <code>(\\w+)\\((.+)\\)</code> pattern, i.e.
+ * "user.addresses(myAddress).street"</li>
+ * </ul>
+ *
+ * @param expression not null expression
+ * @param root not null object
+ * @return the object defined by the expression
+ */
+ public static Object evaluate(String expression, Object root) {
+ return evaluate(expression, root, true);
+ }
+
+ /**
+ * <p>The implementation supports indexed, nested and mapped properties.</p>
+ *
+ * <ul>
+ * <li>nested properties should be defined by a dot, i.e. "user.address.street"</li>
+ * <li>indexed properties (java.util.List or array instance) should be contains <code>(\\w+)\\[(\\d+)\\]</code>
+ * pattern, i.e. "user.addresses[1].street"</li>
+ * <li>mapped properties should be contains <code>(\\w+)\\((.+)\\)</code> pattern, i.e.
+ * "user.addresses(myAddress).street"</li>
+ * </ul>
+ *
+ * @param expression not null expression
+ * @param root not null object
+ * @param trimRootToken root start
+ * @return the object defined by the expression
+ */
+ public static Object evaluate(String expression, final Object root, final boolean trimRootToken) {
+ Object value = root;
+
+ // ----------------------------------------------------------------------
+ // Walk the dots and retrieve the ultimate value desired from the
+ // MavenProject instance.
+ // ----------------------------------------------------------------------
+
+ if (expression == null || expression.isEmpty() || !Character.isJavaIdentifierStart(expression.charAt(0))) {
+ return null;
+ }
+
+ boolean hasDots = expression.indexOf(PROPERTY_START) >= 0;
+
+ final Tokenizer tokenizer;
+ if (trimRootToken && hasDots) {
+ tokenizer = new Tokenizer(expression);
+ tokenizer.nextPropertyName();
+ if (tokenizer.getPosition() == EOF) {
+ return null;
+ }
+ } else {
+ tokenizer = new Tokenizer("." + expression);
+ }
+
+ int propertyPosition = tokenizer.getPosition();
+ while (value != null && tokenizer.peekChar() != EOF) {
+ switch (tokenizer.skipChar()) {
+ case INDEXED_START:
+ value = getIndexedValue(
+ expression,
+ propertyPosition,
+ tokenizer.getPosition(),
+ value,
+ tokenizer.nextToken(INDEXED_END));
+ break;
+ case MAPPED_START:
+ value = getMappedValue(
+ expression,
+ propertyPosition,
+ tokenizer.getPosition(),
+ value,
+ tokenizer.nextToken(MAPPED_END));
+ break;
+ case PROPERTY_START:
+ propertyPosition = tokenizer.getPosition();
+ value = getPropertyValue(value, tokenizer.nextPropertyName());
+ break;
+ default:
+ // could not parse expression
+ return null;
+ }
+ }
+
+ return value;
+ }
+
+ private static Object getMappedValue(
+ final String expression, final int from, final int to, final Object value, final String key) {
+ if (value == null || key == null) {
+ return null;
+ }
+ if (value instanceof Map) {
+ try {
+ Object[] localParams = new Object[] {key};
+ ClassMap classMap = getClassMap(value.getClass());
+ Method method = classMap.findMethod("get", localParams);
+ return method.invoke(value, localParams);
+ } catch (Exception e) {
+ throw new RuntimeException(e);
+ }
+ } else {
+ final String message = String.format(
+ "The token '%s' at position '%d' refers to a java.util.Map, but the value seems is an instance of '%s'",
Review Comment:
is --> to be
##########
maven-core/src/main/java/org/apache/maven/utils/introspection/ReflectionValueExtractor.java:
##########
@@ -0,0 +1,325 @@
+/*
+ * 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.utils.introspection;
+
+import java.lang.ref.WeakReference;
+import java.lang.reflect.Array;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.util.List;
+import java.util.Map;
+import java.util.WeakHashMap;
+
+/**
+ * <p>
+ * Using simple dotted expressions to extract the values from an Object instance, For example we might want to extract a
+ * value like: <code>project.build.sourceDirectory</code>
+ * </p>
+ * <p>
+ * The implementation supports indexed, nested and mapped properties similar to the JSP way.
+ * </p>
+ *
+ * @author <a href="mailto:jason@maven.org">Jason van Zyl </a>
+ * @author <a href="mailto:vincent.siveton@gmail.com">Vincent Siveton</a>
+ *
+ * @see <a href=
+ * "http://struts.apache.org/1.x/struts-taglib/indexedprops.html">http://struts.apache.org/1.x/struts-taglib/indexedprops.html</a>
+ */
+public class ReflectionValueExtractor {
+ private static final Class<?>[] CLASS_ARGS = new Class[0];
+
+ private static final Object[] OBJECT_ARGS = new Object[0];
+
+ /**
+ * Use a WeakHashMap here, so the keys (Class objects) can be garbage collected. This approach prevents permgen
+ * space overflows due to retention of discarded classloaders.
+ */
+ private static final Map<Class<?>, WeakReference<ClassMap>> CLASS_MAPS = new WeakHashMap<>();
+
+ static final int EOF = -1;
+
+ static final char PROPERTY_START = '.';
+
+ static final char INDEXED_START = '[';
+
+ static final char INDEXED_END = ']';
+
+ static final char MAPPED_START = '(';
+
+ static final char MAPPED_END = ')';
+
+ static class Tokenizer {
+ final String expression;
+
+ int idx;
+
+ Tokenizer(String expression) {
+ this.expression = expression;
+ }
+
+ public int peekChar() {
+ return idx < expression.length() ? expression.charAt(idx) : EOF;
+ }
+
+ public int skipChar() {
+ return idx < expression.length() ? expression.charAt(idx++) : EOF;
+ }
+
+ public String nextToken(char delimiter) {
+ int start = idx;
+
+ while (idx < expression.length() && delimiter != expression.charAt(idx)) {
+ idx++;
+ }
+
+ // delimiter MUST be present
+ if (idx <= start || idx >= expression.length()) {
+ return null;
+ }
+
+ return expression.substring(start, idx++);
+ }
+
+ public String nextPropertyName() {
+ final int start = idx;
+
+ while (idx < expression.length() && Character.isJavaIdentifierPart(expression.charAt(idx))) {
+ idx++;
+ }
+
+ // property name does not require delimiter
+ if (idx <= start || idx > expression.length()) {
+ return null;
+ }
+
+ return expression.substring(start, idx);
+ }
+
+ public int getPosition() {
+ return idx < expression.length() ? idx : EOF;
+ }
+
+ // to make tokenizer look pretty in debugger
+ @Override
+ public String toString() {
+ return idx < expression.length() ? expression.substring(idx) : "<EOF>";
+ }
+ }
+
+ private ReflectionValueExtractor() {}
+
+ /**
+ * <p>The implementation supports indexed, nested and mapped properties.</p>
+ *
+ * <ul>
+ * <li>nested properties should be defined by a dot, i.e. "user.address.street"</li>
+ * <li>indexed properties (java.util.List or array instance) should be contains <code>(\\w+)\\[(\\d+)\\]</code>
+ * pattern, i.e. "user.addresses[1].street"</li>
+ * <li>mapped properties should be contains <code>(\\w+)\\((.+)\\)</code> pattern, i.e.
+ * "user.addresses(myAddress).street"</li>
+ * </ul>
+ *
+ * @param expression not null expression
+ * @param root not null object
+ * @return the object defined by the expression
+ */
+ public static Object evaluate(String expression, Object root) {
+ return evaluate(expression, root, true);
+ }
+
+ /**
+ * <p>The implementation supports indexed, nested and mapped properties.</p>
+ *
+ * <ul>
+ * <li>nested properties should be defined by a dot, i.e. "user.address.street"</li>
+ * <li>indexed properties (java.util.List or array instance) should be contains <code>(\\w+)\\[(\\d+)\\]</code>
+ * pattern, i.e. "user.addresses[1].street"</li>
+ * <li>mapped properties should be contains <code>(\\w+)\\((.+)\\)</code> pattern, i.e.
+ * "user.addresses(myAddress).street"</li>
+ * </ul>
+ *
+ * @param expression not null expression
+ * @param root not null object
+ * @param trimRootToken root start
+ * @return the object defined by the expression
+ */
+ public static Object evaluate(String expression, final Object root, final boolean trimRootToken) {
+ Object value = root;
+
+ // ----------------------------------------------------------------------
+ // Walk the dots and retrieve the ultimate value desired from the
+ // MavenProject instance.
+ // ----------------------------------------------------------------------
+
+ if (expression == null || expression.isEmpty() || !Character.isJavaIdentifierStart(expression.charAt(0))) {
+ return null;
+ }
+
+ boolean hasDots = expression.indexOf(PROPERTY_START) >= 0;
+
+ final Tokenizer tokenizer;
+ if (trimRootToken && hasDots) {
+ tokenizer = new Tokenizer(expression);
+ tokenizer.nextPropertyName();
+ if (tokenizer.getPosition() == EOF) {
+ return null;
+ }
+ } else {
+ tokenizer = new Tokenizer("." + expression);
+ }
+
+ int propertyPosition = tokenizer.getPosition();
+ while (value != null && tokenizer.peekChar() != EOF) {
+ switch (tokenizer.skipChar()) {
+ case INDEXED_START:
+ value = getIndexedValue(
+ expression,
+ propertyPosition,
+ tokenizer.getPosition(),
+ value,
+ tokenizer.nextToken(INDEXED_END));
+ break;
+ case MAPPED_START:
+ value = getMappedValue(
+ expression,
+ propertyPosition,
+ tokenizer.getPosition(),
+ value,
+ tokenizer.nextToken(MAPPED_END));
+ break;
+ case PROPERTY_START:
+ propertyPosition = tokenizer.getPosition();
+ value = getPropertyValue(value, tokenizer.nextPropertyName());
+ break;
+ default:
+ // could not parse expression
+ return null;
+ }
+ }
+
+ return value;
+ }
+
+ private static Object getMappedValue(
+ final String expression, final int from, final int to, final Object value, final String key) {
+ if (value == null || key == null) {
+ return null;
+ }
+ if (value instanceof Map) {
+ try {
+ Object[] localParams = new Object[] {key};
+ ClassMap classMap = getClassMap(value.getClass());
+ Method method = classMap.findMethod("get", localParams);
+ return method.invoke(value, localParams);
+ } catch (Exception e) {
+ throw new RuntimeException(e);
+ }
+ } else {
+ final String message = String.format(
+ "The token '%s' at position '%d' refers to a java.util.Map, but the value seems is an instance of '%s'",
+ expression.subSequence(from, to), from, value.getClass());
+ throw new IllegalArgumentException(message);
+ }
+ }
+
+ private static Object getIndexedValue(
+ final String expression, final int from, final int to, final Object value, final String indexStr) {
+ int index;
+ try {
+ index = Integer.parseInt(indexStr);
+
+ if (value.getClass().isArray()) {
+ return Array.get(value, index);
+ }
+
+ } catch (NumberFormatException e) {
+ final String message = String.format("The indexStr '%s' is not a number", indexStr);
+ throw new IllegalArgumentException(message);
+ }
+
+ if (value instanceof List) {
+ try {
+ ClassMap classMap = getClassMap(value.getClass());
+ // use get method on List interface
+ Object[] localParams = new Object[] {index};
+ Method method = classMap.findMethod("get", localParams);
+ return method.invoke(value, localParams);
+ } catch (Exception e) {
+ throw new RuntimeException(e);
Review Comment:
we likely shouldn't be throwing a raw runtime exception here. At least use a subclass, but possibly the logic here needs a rethink
##########
maven-model-builder/src/main/java/org/apache/maven/utils/Os.java:
##########
@@ -0,0 +1,383 @@
+/*
Review Comment:
The values might be used but I don't think these constants are. enforcer-rules has its own variant of this class. I'm thinking about all of these but anything we make public here makes it hard to fix or improve any of this later.
--
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
[GitHub] [maven] elharo commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1325779523
##########
maven-model/src/test/java/org/apache/maven/model/v4/Xpp3DomPerfTest.java:
##########
@@ -18,22 +18,6 @@
*/
package org.apache.maven.model.v4;
-/*
Review Comment:
We need a lawyer, or at least official approval. This is complicated and Apache is rigorous about crossing the t's and dotting the I's. IANAL and cannot decide on who owns what or what rules apply where outside the common cases, and this one is getting increasingly wonky.
--
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
[GitHub] [maven] elharo commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1311520008
##########
maven-core/src/main/java/org/apache/maven/utils/dag/DAG.java:
##########
@@ -0,0 +1,240 @@
+/*
+ * 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.utils.dag;
+
+import java.io.Serializable;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * DAG = Directed Acyclic Graph
+ *
+ * @author <a href="michal.maczka@dimatics.com">Michal Maczka</a>
+ *
+ * TODO this class should be renamed from DAG to Dag
Review Comment:
Now is the time to do this
##########
maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuilder.java:
##########
@@ -103,6 +102,10 @@ public class DefaultProjectBuilder implements ProjectBuilder {
private final RootLocator rootLocator;
+ public static final String OS_NAME = System.getProperty("os.name").toLowerCase(Locale.US);
Review Comment:
private
##########
maven-model-builder/src/main/java/org/apache/maven/utils/Os.java:
##########
@@ -0,0 +1,383 @@
+/*
Review Comment:
This can't be copied over as is. It needs to be cleaned up and rethought. It's horribly out of date.
##########
maven-core/src/main/java/org/apache/maven/utils/dag/CycleDetectedException.java:
##########
@@ -0,0 +1,40 @@
+/*
+ * 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.utils.dag;
Review Comment:
This DAG stuff should probably be a separate PR from random StringUtils and IO fixes.
##########
maven-model/src/main/java/org/apache/maven/model/io/xpp3/MavenXpp3ReaderEx.java:
##########
@@ -112,13 +119,13 @@ public Model read(InputStream in, InputSource source) throws IOException, XmlPul
*
* @param parser a parser object.
* @param strict a strict object.
- * @throws IOException IOException if any.
- * @throws XmlPullParserException XmlPullParserException if
- * any.
* @return Model
+ * @throws IOException IOException if any.
Review Comment:
no period, and in general "if any" is not sufficient description. Something like "if an I/O error occurs while reading from the underlying source"
##########
maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuilder.java:
##########
@@ -103,6 +102,10 @@ public class DefaultProjectBuilder implements ProjectBuilder {
private final RootLocator rootLocator;
+ public static final String OS_NAME = System.getProperty("os.name").toLowerCase(Locale.US);
+
+ public static final String FAMILY_WINDOWS = "windows";
Review Comment:
private
In general, avoid introducing new public API
##########
maven-core/src/main/java/org/apache/maven/properties/internal/EnvironmentUtils.java:
##########
@@ -36,6 +34,10 @@ public class EnvironmentUtils {
private static Properties envVars;
+ public static final String OS_NAME = System.getProperty("os.name").toLowerCase(Locale.US);
Review Comment:
private
##########
maven-core/src/main/java/org/apache/maven/utils/introspection/ReflectionValueExtractor.java:
##########
@@ -0,0 +1,325 @@
+/*
+ * 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.utils.introspection;
+
+import java.lang.ref.WeakReference;
+import java.lang.reflect.Array;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.util.List;
+import java.util.Map;
+import java.util.WeakHashMap;
+
+/**
+ * <p>
+ * Using simple dotted expressions to extract the values from an Object instance, For example we might want to extract a
+ * value like: <code>project.build.sourceDirectory</code>
+ * </p>
+ * <p>
+ * The implementation supports indexed, nested and mapped properties similar to the JSP way.
+ * </p>
+ *
+ * @author <a href="mailto:jason@maven.org">Jason van Zyl </a>
+ * @author <a href="mailto:vincent.siveton@gmail.com">Vincent Siveton</a>
+ *
+ * @see <a href=
+ * "http://struts.apache.org/1.x/struts-taglib/indexedprops.html">http://struts.apache.org/1.x/struts-taglib/indexedprops.html</a>
+ */
+public class ReflectionValueExtractor {
+ private static final Class<?>[] CLASS_ARGS = new Class[0];
+
+ private static final Object[] OBJECT_ARGS = new Object[0];
+
+ /**
+ * Use a WeakHashMap here, so the keys (Class objects) can be garbage collected. This approach prevents permgen
+ * space overflows due to retention of discarded classloaders.
+ */
+ private static final Map<Class<?>, WeakReference<ClassMap>> CLASS_MAPS = new WeakHashMap<>();
+
+ static final int EOF = -1;
+
+ static final char PROPERTY_START = '.';
+
+ static final char INDEXED_START = '[';
+
+ static final char INDEXED_END = ']';
+
+ static final char MAPPED_START = '(';
+
+ static final char MAPPED_END = ')';
+
+ static class Tokenizer {
+ final String expression;
+
+ int idx;
+
+ Tokenizer(String expression) {
+ this.expression = expression;
+ }
+
+ public int peekChar() {
+ return idx < expression.length() ? expression.charAt(idx) : EOF;
+ }
+
+ public int skipChar() {
+ return idx < expression.length() ? expression.charAt(idx++) : EOF;
+ }
+
+ public String nextToken(char delimiter) {
+ int start = idx;
+
+ while (idx < expression.length() && delimiter != expression.charAt(idx)) {
+ idx++;
+ }
+
+ // delimiter MUST be present
+ if (idx <= start || idx >= expression.length()) {
+ return null;
+ }
+
+ return expression.substring(start, idx++);
+ }
+
+ public String nextPropertyName() {
+ final int start = idx;
+
+ while (idx < expression.length() && Character.isJavaIdentifierPart(expression.charAt(idx))) {
+ idx++;
+ }
+
+ // property name does not require delimiter
+ if (idx <= start || idx > expression.length()) {
+ return null;
+ }
+
+ return expression.substring(start, idx);
+ }
+
+ public int getPosition() {
+ return idx < expression.length() ? idx : EOF;
+ }
+
+ // to make tokenizer look pretty in debugger
+ @Override
+ public String toString() {
+ return idx < expression.length() ? expression.substring(idx) : "<EOF>";
+ }
+ }
+
+ private ReflectionValueExtractor() {}
+
+ /**
+ * <p>The implementation supports indexed, nested and mapped properties.</p>
+ *
+ * <ul>
+ * <li>nested properties should be defined by a dot, i.e. "user.address.street"</li>
+ * <li>indexed properties (java.util.List or array instance) should be contains <code>(\\w+)\\[(\\d+)\\]</code>
+ * pattern, i.e. "user.addresses[1].street"</li>
+ * <li>mapped properties should be contains <code>(\\w+)\\((.+)\\)</code> pattern, i.e.
+ * "user.addresses(myAddress).street"</li>
+ * </ul>
+ *
+ * @param expression not null expression
+ * @param root not null object
+ * @return the object defined by the expression
+ * @throws Exception if any
+ */
+ public static Object evaluate(String expression, Object root) throws Exception {
+ return evaluate(expression, root, true);
+ }
+
+ /**
+ * <p>The implementation supports indexed, nested and mapped properties.</p>
+ *
+ * <ul>
+ * <li>nested properties should be defined by a dot, i.e. "user.address.street"</li>
+ * <li>indexed properties (java.util.List or array instance) should be contains <code>(\\w+)\\[(\\d+)\\]</code>
+ * pattern, i.e. "user.addresses[1].street"</li>
+ * <li>mapped properties should be contains <code>(\\w+)\\((.+)\\)</code> pattern, i.e.
+ * "user.addresses(myAddress).street"</li>
+ * </ul>
+ *
+ * @param expression not null expression
+ * @param root not null object
+ * @param trimRootToken root start
+ * @return the object defined by the expression
+ * @throws Exception if any
+ */
+ // TODO: don't throw Exception
+ public static Object evaluate(String expression, final Object root, final boolean trimRootToken) throws Exception {
+ Object value = root;
+
+ // ----------------------------------------------------------------------
+ // Walk the dots and retrieve the ultimate value desired from the
+ // MavenProject instance.
+ // ----------------------------------------------------------------------
+
+ if (expression == null || expression.isEmpty() || !Character.isJavaIdentifierStart(expression.charAt(0))) {
+ return null;
+ }
+
+ boolean hasDots = expression.indexOf(PROPERTY_START) >= 0;
+
+ final Tokenizer tokenizer;
+ if (trimRootToken && hasDots) {
+ tokenizer = new Tokenizer(expression);
+ tokenizer.nextPropertyName();
+ if (tokenizer.getPosition() == EOF) {
+ return null;
+ }
+ } else {
+ tokenizer = new Tokenizer("." + expression);
+ }
+
+ int propertyPosition = tokenizer.getPosition();
+ while (value != null && tokenizer.peekChar() != EOF) {
+ switch (tokenizer.skipChar()) {
+ case INDEXED_START:
+ value = getIndexedValue(
+ expression,
+ propertyPosition,
+ tokenizer.getPosition(),
+ value,
+ tokenizer.nextToken(INDEXED_END));
+ break;
+ case MAPPED_START:
+ value = getMappedValue(
+ expression,
+ propertyPosition,
+ tokenizer.getPosition(),
+ value,
+ tokenizer.nextToken(MAPPED_END));
+ break;
+ case PROPERTY_START:
+ propertyPosition = tokenizer.getPosition();
+ value = getPropertyValue(value, tokenizer.nextPropertyName());
+ break;
+ default:
+ // could not parse expression
+ return null;
+ }
+ }
+
+ return value;
+ }
+
+ private static Object getMappedValue(
+ final String expression, final int from, final int to, final Object value, final String key)
+ throws Exception {
+ if (value == null || key == null) {
+ return null;
+ }
+
+ if (value instanceof Map) {
+ Object[] localParams = new Object[] {key};
+ ClassMap classMap = getClassMap(value.getClass());
+ Method method = classMap.findMethod("get", localParams);
+ return method.invoke(value, localParams);
+ }
+
+ final String message = String.format(
+ "The token '%s' at position '%d' refers to a java.util.Map, but the value seems is an instance of '%s'",
+ expression.subSequence(from, to), from, value.getClass());
+
+ throw new Exception(message);
Review Comment:
This should be a more specific exception.
Note: I do not care if this was already done incorrectly somewhere else. It's still wrong here.
##########
maven-model/src/main/java/org/apache/maven/model/io/xpp3/MavenXpp3ReaderEx.java:
##########
@@ -54,56 +55,62 @@ public boolean getAddDefaultEntities() {
/**
* @param reader a reader object.
* @param strict a strict object.
- * @throws IOException IOException if any.
- * @throws XmlPullParserException XmlPullParserException if
- * any.
* @return Model
+ * @throws IOException IOException if any.
+ * @throws XMLStreamException XMLStreamException if
+ * any.
*/
- public Model read(Reader reader, boolean strict, InputSource source) throws IOException, XmlPullParserException {
- XmlPullParser parser =
- addDefaultEntities ? new MXParser(EntityReplacementMap.defaultEntityReplacementMap) : new MXParser();
- parser.setInput(reader);
+ public Model read(Reader reader, boolean strict, InputSource source) throws IOException, XMLStreamException {
+ XMLInputFactory factory = new com.ctc.wstx.stax.WstxInputFactory();
+ factory.setProperty(XMLInputFactory.IS_REPLACING_ENTITY_REFERENCES, false);
+ XMLStreamReader parser = null;
Review Comment:
try block needs to surround everything
##########
maven-core/src/main/java/org/apache/maven/utils/introspection/ReflectionValueExtractor.java:
##########
@@ -0,0 +1,325 @@
+/*
+ * 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.utils.introspection;
+
+import java.lang.ref.WeakReference;
+import java.lang.reflect.Array;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.util.List;
+import java.util.Map;
+import java.util.WeakHashMap;
+
+/**
+ * <p>
+ * Using simple dotted expressions to extract the values from an Object instance, For example we might want to extract a
+ * value like: <code>project.build.sourceDirectory</code>
+ * </p>
+ * <p>
+ * The implementation supports indexed, nested and mapped properties similar to the JSP way.
+ * </p>
+ *
+ * @author <a href="mailto:jason@maven.org">Jason van Zyl </a>
+ * @author <a href="mailto:vincent.siveton@gmail.com">Vincent Siveton</a>
+ *
+ * @see <a href=
+ * "http://struts.apache.org/1.x/struts-taglib/indexedprops.html">http://struts.apache.org/1.x/struts-taglib/indexedprops.html</a>
+ */
+public class ReflectionValueExtractor {
+ private static final Class<?>[] CLASS_ARGS = new Class[0];
+
+ private static final Object[] OBJECT_ARGS = new Object[0];
+
+ /**
+ * Use a WeakHashMap here, so the keys (Class objects) can be garbage collected. This approach prevents permgen
+ * space overflows due to retention of discarded classloaders.
+ */
+ private static final Map<Class<?>, WeakReference<ClassMap>> CLASS_MAPS = new WeakHashMap<>();
+
+ static final int EOF = -1;
+
+ static final char PROPERTY_START = '.';
+
+ static final char INDEXED_START = '[';
+
+ static final char INDEXED_END = ']';
+
+ static final char MAPPED_START = '(';
+
+ static final char MAPPED_END = ')';
+
+ static class Tokenizer {
+ final String expression;
+
+ int idx;
+
+ Tokenizer(String expression) {
+ this.expression = expression;
+ }
+
+ public int peekChar() {
+ return idx < expression.length() ? expression.charAt(idx) : EOF;
+ }
+
+ public int skipChar() {
+ return idx < expression.length() ? expression.charAt(idx++) : EOF;
+ }
+
+ public String nextToken(char delimiter) {
+ int start = idx;
+
+ while (idx < expression.length() && delimiter != expression.charAt(idx)) {
+ idx++;
+ }
+
+ // delimiter MUST be present
+ if (idx <= start || idx >= expression.length()) {
+ return null;
+ }
+
+ return expression.substring(start, idx++);
+ }
+
+ public String nextPropertyName() {
+ final int start = idx;
+
+ while (idx < expression.length() && Character.isJavaIdentifierPart(expression.charAt(idx))) {
+ idx++;
+ }
+
+ // property name does not require delimiter
+ if (idx <= start || idx > expression.length()) {
+ return null;
+ }
+
+ return expression.substring(start, idx);
+ }
+
+ public int getPosition() {
+ return idx < expression.length() ? idx : EOF;
+ }
+
+ // to make tokenizer look pretty in debugger
+ @Override
+ public String toString() {
+ return idx < expression.length() ? expression.substring(idx) : "<EOF>";
+ }
+ }
+
+ private ReflectionValueExtractor() {}
+
+ /**
+ * <p>The implementation supports indexed, nested and mapped properties.</p>
+ *
+ * <ul>
+ * <li>nested properties should be defined by a dot, i.e. "user.address.street"</li>
+ * <li>indexed properties (java.util.List or array instance) should be contains <code>(\\w+)\\[(\\d+)\\]</code>
+ * pattern, i.e. "user.addresses[1].street"</li>
+ * <li>mapped properties should be contains <code>(\\w+)\\((.+)\\)</code> pattern, i.e.
+ * "user.addresses(myAddress).street"</li>
+ * </ul>
+ *
+ * @param expression not null expression
+ * @param root not null object
+ * @return the object defined by the expression
+ * @throws Exception if any
+ */
+ public static Object evaluate(String expression, Object root) throws Exception {
+ return evaluate(expression, root, true);
+ }
+
+ /**
+ * <p>The implementation supports indexed, nested and mapped properties.</p>
+ *
+ * <ul>
+ * <li>nested properties should be defined by a dot, i.e. "user.address.street"</li>
+ * <li>indexed properties (java.util.List or array instance) should be contains <code>(\\w+)\\[(\\d+)\\]</code>
+ * pattern, i.e. "user.addresses[1].street"</li>
+ * <li>mapped properties should be contains <code>(\\w+)\\((.+)\\)</code> pattern, i.e.
+ * "user.addresses(myAddress).street"</li>
+ * </ul>
+ *
+ * @param expression not null expression
+ * @param root not null object
+ * @param trimRootToken root start
+ * @return the object defined by the expression
+ * @throws Exception if any
+ */
+ // TODO: don't throw Exception
+ public static Object evaluate(String expression, final Object root, final boolean trimRootToken) throws Exception {
+ Object value = root;
+
+ // ----------------------------------------------------------------------
+ // Walk the dots and retrieve the ultimate value desired from the
+ // MavenProject instance.
+ // ----------------------------------------------------------------------
+
+ if (expression == null || expression.isEmpty() || !Character.isJavaIdentifierStart(expression.charAt(0))) {
+ return null;
+ }
+
+ boolean hasDots = expression.indexOf(PROPERTY_START) >= 0;
+
+ final Tokenizer tokenizer;
+ if (trimRootToken && hasDots) {
+ tokenizer = new Tokenizer(expression);
+ tokenizer.nextPropertyName();
+ if (tokenizer.getPosition() == EOF) {
+ return null;
+ }
+ } else {
+ tokenizer = new Tokenizer("." + expression);
+ }
+
+ int propertyPosition = tokenizer.getPosition();
+ while (value != null && tokenizer.peekChar() != EOF) {
+ switch (tokenizer.skipChar()) {
+ case INDEXED_START:
+ value = getIndexedValue(
+ expression,
+ propertyPosition,
+ tokenizer.getPosition(),
+ value,
+ tokenizer.nextToken(INDEXED_END));
+ break;
+ case MAPPED_START:
+ value = getMappedValue(
+ expression,
+ propertyPosition,
+ tokenizer.getPosition(),
+ value,
+ tokenizer.nextToken(MAPPED_END));
+ break;
+ case PROPERTY_START:
+ propertyPosition = tokenizer.getPosition();
+ value = getPropertyValue(value, tokenizer.nextPropertyName());
+ break;
+ default:
+ // could not parse expression
+ return null;
+ }
+ }
+
+ return value;
+ }
+
+ private static Object getMappedValue(
+ final String expression, final int from, final int to, final Object value, final String key)
+ throws Exception {
+ if (value == null || key == null) {
+ return null;
+ }
+
+ if (value instanceof Map) {
+ Object[] localParams = new Object[] {key};
+ ClassMap classMap = getClassMap(value.getClass());
+ Method method = classMap.findMethod("get", localParams);
+ return method.invoke(value, localParams);
+ }
+
+ final String message = String.format(
+ "The token '%s' at position '%d' refers to a java.util.Map, but the value seems is an instance of '%s'",
+ expression.subSequence(from, to), from, value.getClass());
+
+ throw new Exception(message);
+ }
+
+ private static Object getIndexedValue(
+ final String expression, final int from, final int to, final Object value, final String indexStr)
+ throws Exception {
+ try {
+ int index = Integer.parseInt(indexStr);
Review Comment:
indexStr should probably be an int
##########
maven-core/src/main/java/org/apache/maven/utils/introspection/ReflectionValueExtractor.java:
##########
@@ -0,0 +1,325 @@
+/*
+ * 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.utils.introspection;
+
+import java.lang.ref.WeakReference;
+import java.lang.reflect.Array;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.util.List;
+import java.util.Map;
+import java.util.WeakHashMap;
+
+/**
+ * <p>
+ * Using simple dotted expressions to extract the values from an Object instance, For example we might want to extract a
+ * value like: <code>project.build.sourceDirectory</code>
+ * </p>
+ * <p>
+ * The implementation supports indexed, nested and mapped properties similar to the JSP way.
+ * </p>
+ *
+ * @author <a href="mailto:jason@maven.org">Jason van Zyl </a>
+ * @author <a href="mailto:vincent.siveton@gmail.com">Vincent Siveton</a>
+ *
+ * @see <a href=
+ * "http://struts.apache.org/1.x/struts-taglib/indexedprops.html">http://struts.apache.org/1.x/struts-taglib/indexedprops.html</a>
+ */
+public class ReflectionValueExtractor {
+ private static final Class<?>[] CLASS_ARGS = new Class[0];
+
+ private static final Object[] OBJECT_ARGS = new Object[0];
+
+ /**
+ * Use a WeakHashMap here, so the keys (Class objects) can be garbage collected. This approach prevents permgen
+ * space overflows due to retention of discarded classloaders.
+ */
+ private static final Map<Class<?>, WeakReference<ClassMap>> CLASS_MAPS = new WeakHashMap<>();
+
+ static final int EOF = -1;
+
+ static final char PROPERTY_START = '.';
+
+ static final char INDEXED_START = '[';
+
+ static final char INDEXED_END = ']';
+
+ static final char MAPPED_START = '(';
+
+ static final char MAPPED_END = ')';
+
+ static class Tokenizer {
+ final String expression;
+
+ int idx;
+
+ Tokenizer(String expression) {
+ this.expression = expression;
+ }
+
+ public int peekChar() {
+ return idx < expression.length() ? expression.charAt(idx) : EOF;
+ }
+
+ public int skipChar() {
+ return idx < expression.length() ? expression.charAt(idx++) : EOF;
+ }
+
+ public String nextToken(char delimiter) {
+ int start = idx;
+
+ while (idx < expression.length() && delimiter != expression.charAt(idx)) {
+ idx++;
+ }
+
+ // delimiter MUST be present
+ if (idx <= start || idx >= expression.length()) {
+ return null;
+ }
+
+ return expression.substring(start, idx++);
+ }
+
+ public String nextPropertyName() {
+ final int start = idx;
+
+ while (idx < expression.length() && Character.isJavaIdentifierPart(expression.charAt(idx))) {
+ idx++;
+ }
+
+ // property name does not require delimiter
+ if (idx <= start || idx > expression.length()) {
+ return null;
+ }
+
+ return expression.substring(start, idx);
+ }
+
+ public int getPosition() {
+ return idx < expression.length() ? idx : EOF;
+ }
+
+ // to make tokenizer look pretty in debugger
+ @Override
+ public String toString() {
+ return idx < expression.length() ? expression.substring(idx) : "<EOF>";
+ }
+ }
+
+ private ReflectionValueExtractor() {}
+
+ /**
+ * <p>The implementation supports indexed, nested and mapped properties.</p>
+ *
+ * <ul>
+ * <li>nested properties should be defined by a dot, i.e. "user.address.street"</li>
+ * <li>indexed properties (java.util.List or array instance) should be contains <code>(\\w+)\\[(\\d+)\\]</code>
+ * pattern, i.e. "user.addresses[1].street"</li>
+ * <li>mapped properties should be contains <code>(\\w+)\\((.+)\\)</code> pattern, i.e.
+ * "user.addresses(myAddress).street"</li>
+ * </ul>
+ *
+ * @param expression not null expression
+ * @param root not null object
+ * @return the object defined by the expression
+ * @throws Exception if any
+ */
+ public static Object evaluate(String expression, Object root) throws Exception {
+ return evaluate(expression, root, true);
+ }
+
+ /**
+ * <p>The implementation supports indexed, nested and mapped properties.</p>
+ *
+ * <ul>
+ * <li>nested properties should be defined by a dot, i.e. "user.address.street"</li>
+ * <li>indexed properties (java.util.List or array instance) should be contains <code>(\\w+)\\[(\\d+)\\]</code>
+ * pattern, i.e. "user.addresses[1].street"</li>
+ * <li>mapped properties should be contains <code>(\\w+)\\((.+)\\)</code> pattern, i.e.
+ * "user.addresses(myAddress).street"</li>
+ * </ul>
+ *
+ * @param expression not null expression
+ * @param root not null object
+ * @param trimRootToken root start
+ * @return the object defined by the expression
+ * @throws Exception if any
+ */
+ // TODO: don't throw Exception
+ public static Object evaluate(String expression, final Object root, final boolean trimRootToken) throws Exception {
+ Object value = root;
+
+ // ----------------------------------------------------------------------
+ // Walk the dots and retrieve the ultimate value desired from the
+ // MavenProject instance.
+ // ----------------------------------------------------------------------
+
+ if (expression == null || expression.isEmpty() || !Character.isJavaIdentifierStart(expression.charAt(0))) {
+ return null;
+ }
+
+ boolean hasDots = expression.indexOf(PROPERTY_START) >= 0;
+
+ final Tokenizer tokenizer;
+ if (trimRootToken && hasDots) {
+ tokenizer = new Tokenizer(expression);
+ tokenizer.nextPropertyName();
+ if (tokenizer.getPosition() == EOF) {
+ return null;
+ }
+ } else {
+ tokenizer = new Tokenizer("." + expression);
+ }
+
+ int propertyPosition = tokenizer.getPosition();
+ while (value != null && tokenizer.peekChar() != EOF) {
+ switch (tokenizer.skipChar()) {
+ case INDEXED_START:
+ value = getIndexedValue(
+ expression,
+ propertyPosition,
+ tokenizer.getPosition(),
+ value,
+ tokenizer.nextToken(INDEXED_END));
+ break;
+ case MAPPED_START:
+ value = getMappedValue(
+ expression,
+ propertyPosition,
+ tokenizer.getPosition(),
+ value,
+ tokenizer.nextToken(MAPPED_END));
+ break;
+ case PROPERTY_START:
+ propertyPosition = tokenizer.getPosition();
+ value = getPropertyValue(value, tokenizer.nextPropertyName());
+ break;
+ default:
+ // could not parse expression
+ return null;
+ }
+ }
+
+ return value;
+ }
+
+ private static Object getMappedValue(
+ final String expression, final int from, final int to, final Object value, final String key)
+ throws Exception {
+ if (value == null || key == null) {
+ return null;
+ }
+
+ if (value instanceof Map) {
+ Object[] localParams = new Object[] {key};
+ ClassMap classMap = getClassMap(value.getClass());
+ Method method = classMap.findMethod("get", localParams);
+ return method.invoke(value, localParams);
+ }
+
+ final String message = String.format(
+ "The token '%s' at position '%d' refers to a java.util.Map, but the value seems is an instance of '%s'",
+ expression.subSequence(from, to), from, value.getClass());
+
+ throw new Exception(message);
+ }
+
+ private static Object getIndexedValue(
+ final String expression, final int from, final int to, final Object value, final String indexStr)
+ throws Exception {
+ try {
+ int index = Integer.parseInt(indexStr);
+
+ if (value.getClass().isArray()) {
+ return Array.get(value, index);
+ }
+
+ if (value instanceof List) {
+ ClassMap classMap = getClassMap(value.getClass());
+ // use get method on List interface
+ Object[] localParams = new Object[] {index};
+ Method method = classMap.findMethod("get", localParams);
+ return method.invoke(value, localParams);
+ }
+ } catch (NumberFormatException e) {
+ return null;
Review Comment:
This seems very suspicious
##########
maven-model/src/main/java/org/apache/maven/model/io/xpp3/MavenXpp3ReaderEx.java:
##########
@@ -112,13 +119,13 @@ public Model read(InputStream in, InputSource source) throws IOException, XmlPul
*
* @param parser a parser object.
* @param strict a strict object.
- * @throws IOException IOException if any.
- * @throws XmlPullParserException XmlPullParserException if
- * any.
* @return Model
+ * @throws IOException IOException if any.
+ * @throws XMLStreamException XMLStreamException if
Review Comment:
definitely needs to be expanded and distinguished from IOException
--
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
[GitHub] [maven] gnodet commented on a diff in pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1316213195
##########
maven-model-builder/src/main/java/org/apache/maven/utils/Os.java:
##########
@@ -0,0 +1,383 @@
+/*
Review Comment:
This class is used in two cases: for profile activation and to check if the OS is windows. The second case is still correct and for the first one, I don't think anything is broken. So yes, we may not be able to detect if we're on OSX Big Sur, Monterey or whatever, but we don't need it. It solves our needs, I think it's fine.
--
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
[GitHub] [maven] gnodet commented on pull request #1208: [MNG-7820] Remove dependency on plexus-utils
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #1208:
URL: https://github.com/apache/maven/pull/1208#issuecomment-1707384017
> @elharo we can't refactor the `Os` class in a different PR unless we merge it first, as the PR removes the plexus-utils dependency... I'll refactor the class completely.
Done. However, the logic should not be changed in this PR. If you want to update the behaviour, we should create a ticket for that and will have to modify the main doc at https://maven.apache.org/enforcer/enforcer-rules/requireOS.html
--
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