[ https://issues.apache.org/jira/browse/MNG-7820?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17747452#comment-17747452 ]
ASF GitHub Bot commented on MNG-7820: ------------------------------------- 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:ja...@maven.org">Jason van Zyl </a> + * @author <a href="mailto:vincent.sive...@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.mac...@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.mac...@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.mac...@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? > Remove dependency on plexus-utils > --------------------------------- > > Key: MNG-7820 > URL: https://issues.apache.org/jira/browse/MNG-7820 > Project: Maven > Issue Type: Task > Reporter: Guillaume Nodet > Priority: Major > Fix For: 4.0.0-alpha-8 > > -- This message was sent by Atlassian Jira (v8.20.10#820010)