[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2020-06-16 Thread GitBox


rfscholte commented on a change in pull request #286:
URL: https://github.com/apache/maven/pull/286#discussion_r441095444



##
File path: 
maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuilder.java
##
@@ -406,15 +444,17 @@ private ModelSource createStubModelSource( Artifact 
artifact )
 @SuppressWarnings( "checkstyle:parameternumber" )
 private boolean build( List results, 
List interimResults,
Map projectIndex, List 
pomFiles, Set aggregatorFiles,
-   boolean isRoot, boolean recursive, InternalConfig 
config )
+   boolean isRoot, boolean recursive, InternalConfig 
config,

Review comment:
   Not my change, but I don't mind to adjust 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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2020-06-16 Thread GitBox


rfscholte commented on a change in pull request #286:
URL: https://github.com/apache/maven/pull/286#discussion_r441095313



##
File path: 
maven-core/src/main/java/org/apache/maven/internal/aether/ConsumerModelSourceTransformer.java
##
@@ -0,0 +1,111 @@
+package org.apache.maven.internal.aether;
+
+/*
+ * 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.
+ */
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+
+import javax.xml.parsers.ParserConfigurationException;
+import javax.xml.stream.XMLInputFactory;
+import javax.xml.stream.XMLStreamException;
+import javax.xml.stream.XMLStreamReader;
+import javax.xml.transform.OutputKeys;
+import javax.xml.transform.Transformer;
+import javax.xml.transform.TransformerConfigurationException;
+import javax.xml.transform.sax.SAXTransformerFactory;
+import javax.xml.transform.sax.TransformerHandler;
+
+import org.apache.maven.model.building.AbstractModelSourceTransformer;
+import org.apache.maven.model.building.DefaultBuildPomXMLFilterFactory;
+import org.apache.maven.model.building.TransformerContext;
+import org.apache.maven.xml.Factories;
+import org.apache.maven.xml.internal.DefaultConsumerPomXMLFilterFactory;
+import org.apache.maven.xml.sax.filter.AbstractSAXFilter;
+import org.xml.sax.SAXException;
+
+class ConsumerModelSourceTransformer extends AbstractModelSourceTransformer
+{
+@Override
+protected AbstractSAXFilter getSAXFilter( Path pomFile, TransformerContext 
context )
+throws TransformerConfigurationException, SAXException, 
ParserConfigurationException
+{
+return new DefaultConsumerPomXMLFilterFactory( new 
DefaultBuildPomXMLFilterFactory( context ) ).get( pomFile );
+}
+
+/**
+ * This transformer will ensure that encoding and version are kept.
+ * However, it cannot prevent:
+ * 
+ *   line-endings will be unix-style(LF)
+ *   attributes will be on one line
+ *   Unnecessary whitespace before the rootelement will be remove 
+ * 
+ */
+@Override
+protected TransformerHandler getTransformerHandler( Path pomFile )
+throws IOException, 
org.apache.maven.model.building.TransformerException
+{
+final TransformerHandler transformerHandler;
+
+final SAXTransformerFactory transformerFactory =
+(SAXTransformerFactory) 
Factories.newTransformerFactory();
+
+// Keep same encoding+version
+try ( InputStream input = Files.newInputStream( pomFile ) )
+{
+XMLStreamReader streamReader =
+XMLInputFactory.newFactory().createXMLStreamReader( input );
+
+transformerHandler = transformerFactory.newTransformerHandler();
+
+final String encoding = streamReader.getCharacterEncodingScheme();
+final String version = streamReader.getVersion();
+
+Transformer transformer = transformerHandler.getTransformer();
+transformer.setOutputProperty( OutputKeys.METHOD, "xml" );
+if ( encoding == null && version == null )
+{
+transformer.setOutputProperty( 
OutputKeys.OMIT_XML_DECLARATION, "yes" );
+}
+else
+{
+transformer.setOutputProperty( 
OutputKeys.OMIT_XML_DECLARATION, "no" );
+
+if ( encoding != null )
+{
+transformer.setOutputProperty( OutputKeys.ENCODING, 
encoding );

Review comment:
   There are integration tests with different encodings. They broke when I 
ignored the encoding, so it is very important to keep them.





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2020-06-16 Thread GitBox


rfscholte commented on a change in pull request #286:
URL: https://github.com/apache/maven/pull/286#discussion_r441093443



##
File path: 
maven-core/src/main/java/org/apache/maven/project/ReactorModelPool.java
##
@@ -19,53 +19,127 @@
  * under the License.
  */
 
-import java.io.File;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Collections;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Map;
+import java.util.NoSuchElementException;
+import java.util.Objects;
+import java.util.Set;
+
+import org.apache.maven.model.Model;
 
 /**
- * Holds all POM files that are known to the reactor. This allows the project 
builder to resolve imported POMs from the
+ * Holds all Models that are known to the reactor. This allows the project 
builder to resolve imported Models from the
  * reactor when building another project's effective model.
  *
  * @author Benjamin Bentmann
+ * @author Robert Scholte
  */
 class ReactorModelPool
 {
+private final Map> modelsByGa = new HashMap<>();
+
+private final Map modelsByPath = new HashMap<>();
+
+/**
+ * Get the model by its GAV or (since 3.7.0) by its GA if there is only 
one.

Review comment:
   This method already existed before. The change is that `version` now can 
be null.





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2020-06-16 Thread GitBox


rfscholte commented on a change in pull request #286:
URL: https://github.com/apache/maven/pull/286#discussion_r441092529



##
File path: 
maven-model-builder/src/main/java/org/apache/maven/model/building/AbstractModelSourceTransformer.java
##
@@ -0,0 +1,196 @@
+package org.apache.maven.model.building;
+
+/*
+ * 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.
+ */
+
+import java.io.FileInputStream;
+import java.io.FilterInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.PipedInputStream;
+import java.io.PipedOutputStream;
+import java.nio.file.Path;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import javax.xml.parsers.ParserConfigurationException;
+import javax.xml.transform.TransformerConfigurationException;
+import javax.xml.transform.TransformerException;
+import javax.xml.transform.TransformerFactory;
+import javax.xml.transform.sax.SAXResult;
+import javax.xml.transform.sax.SAXSource;
+import javax.xml.transform.sax.TransformerHandler;
+import javax.xml.transform.stream.StreamResult;
+
+import org.apache.maven.xml.Factories;
+import org.apache.maven.xml.sax.ext.CommentRenormalizer;
+import org.apache.maven.xml.sax.filter.AbstractSAXFilter;
+import org.xml.sax.SAXException;
+
+/**
+ * 
+ * @author Robert Scholte
+ * @since 3.7.0
+ */
+public abstract class AbstractModelSourceTransformer
+implements ModelSourceTransformer
+{
+private static final AtomicInteger TRANSFORM_THREAD_COUNTER = new 
AtomicInteger();
+
+private final TransformerFactory transformerFactory = 
Factories.newTransformerFactory();
+
+protected abstract AbstractSAXFilter getSAXFilter( Path pomFile, 
TransformerContext context )
+throws TransformerConfigurationException, SAXException, 
ParserConfigurationException;
+
+protected OutputStream filterOutputStream( OutputStream outputStream, Path 
pomFile )
+{
+return outputStream;
+}
+
+protected TransformerHandler getTransformerHandler( Path pomFile )
+throws IOException, 
org.apache.maven.model.building.TransformerException
+{
+return null;
+}
+
+@Override
+public final InputStream transform( Path pomFile, TransformerContext 
context )
+throws IOException, 
org.apache.maven.model.building.TransformerException
+{
+final TransformerHandler transformerHandler = getTransformerHandler( 
pomFile );
+
+final AbstractSAXFilter filter;
+try
+{
+filter = getSAXFilter( pomFile, context );
+filter.setLexicalHandler( transformerHandler );
+}
+catch ( TransformerConfigurationException | SAXException | 
ParserConfigurationException e )
+{
+throw new org.apache.maven.model.building.TransformerException( e 
);
+}
+
+final SAXSource transformSource =
+new SAXSource( filter,
+   new org.xml.sax.InputSource( new FileInputStream( 
pomFile.toFile() ) ) );
+
+final PipedOutputStream pout = new PipedOutputStream();
+final PipedInputStream pipedInputStream = new PipedInputStream( pout );
+
+OutputStream out = filterOutputStream( pout, pomFile );
+
+final javax.xml.transform.Result result; 
+if ( transformerHandler == null )
+{
+result = new StreamResult( out );
+}
+else
+{
+result = new SAXResult( transformerHandler );
+( (SAXResult) result ).setLexicalHandler( new CommentRenormalizer( 
transformerHandler ) );
+transformerHandler.setResult( new StreamResult( out ) );
+}
+
+IOExceptionHandler eh = new IOExceptionHandler();
+
+Thread transformThread = new Thread( () -> 
+{
+try ( PipedOutputStream pos = pout )
+{
+transformerFactory.newTransformer().transform( 
transformSource, result );
+}
+catch ( TransformerException | IOException e )
+{
+eh.uncaughtException( Thread.currentThread(), e );
+}
+}, "TransformThread-" + 

[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2020-06-14 Thread GitBox


rfscholte commented on a change in pull request #286:
URL: https://github.com/apache/maven/pull/286#discussion_r439869758



##
File path: 
maven-xml/src/main/java/org/apache/maven/xml/sax/filter/AbstractEventXMLFilter.java
##
@@ -0,0 +1,289 @@
+package org.apache.maven.xml.sax.filter;
+
+/*
+ * 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.
+ */
+
+import java.util.ArrayDeque;
+import java.util.Queue;
+
+import org.apache.maven.xml.sax.SAXEvent;
+import org.apache.maven.xml.sax.SAXEventFactory;
+import org.xml.sax.Attributes;
+import org.xml.sax.Locator;
+import org.xml.sax.SAXException;
+import org.xml.sax.XMLReader;
+import org.xml.sax.ext.LexicalHandler;
+
+/**
+ * Builds up a list of SAXEvents, which will be executed with {@link 
#executeEvents()}
+ * 
+ * @author Robert Scholte
+ * @since 3.7.0
+ */
+abstract class AbstractEventXMLFilter extends AbstractSAXFilter
+{
+private Queue saxEvents = new ArrayDeque<>();
+
+private SAXEventFactory eventFactory;
+
+// characters BEFORE startElement must get state of startingElement
+// this way removing based on state keeps correct formatting
+private SAXEvent characters;
+
+private boolean lockCharacters = false;
+
+protected abstract boolean isParsing();
+
+protected abstract String getState();
+
+protected boolean acceptEvent( String state )
+{
+return true;
+}
+
+AbstractEventXMLFilter()
+{
+super();

Review comment:
   This is all to prevent confusion. This is not just the default 
constructor, there's a second one. Empty constructor often suggest generated 
and redundant, hence I'll keep the `super`-calls.





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2020-06-14 Thread GitBox


rfscholte commented on a change in pull request #286:
URL: https://github.com/apache/maven/pull/286#discussion_r439869255



##
File path: 
maven-core/src/main/java/org/apache/maven/internal/aether/DefaultRepositorySystemSessionFactory.java
##
@@ -266,5 +279,41 @@ private String getMavenVersion()
 
 return props.getProperty( "version", "unknown-version" );
 }
+
+private Collection getTransformersForArtifact( final 
org.eclipse.aether.artifact.Artifact artifact,
+final 
SessionData sessionData )
+{
+TransformerContext context = (TransformerContext) sessionData.get( 
TransformerContext.KEY );
+Collection transformers = new ArrayList<>();
+
+// In case of install:install-file there's no transformer context, as 
the goal is unrelated to the lifecycle. 
+if ( "pom".equals( artifact.getExtension() ) && context != null )
+{
+transformers.add( new FileTransformer()
+{
+@Override
+public InputStream transformData( File pomFile )
+throws IOException, TransformException
+{
+try
+{
+return new ConsumerModelSourceTransformer().transform( 
pomFile.toPath(), context );
+}
+catch ( 
org.apache.maven.model.building.TransformerException e )

Review comment:
   yes, now it can, it used to have collissions with 
`javax.xml.transform.TransformerException` and 
`org.apache.maven.artifact.Artifact`





This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2020-06-14 Thread GitBox


rfscholte commented on a change in pull request #286:
URL: https://github.com/apache/maven/pull/286#discussion_r439858810



##
File path: 
maven-xml/src/test/java/org/apache/maven/xml/sax/LexicalHandlerVerifier.java
##
@@ -0,0 +1,277 @@
+package org.apache.maven.xml.sax;
+
+/*
+ * 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.
+ */
+
+import static org.xmlunit.assertj.XmlAssert.assertThat;
+
+import java.io.StringReader;
+import java.io.StringWriter;
+import java.io.Writer;
+import java.util.Arrays;
+
+import javax.xml.transform.OutputKeys;
+import javax.xml.transform.Source;
+import javax.xml.transform.Transformer;
+import javax.xml.transform.sax.SAXResult;
+import javax.xml.transform.sax.SAXSource;
+import javax.xml.transform.sax.SAXTransformerFactory;
+import javax.xml.transform.sax.TransformerHandler;
+import javax.xml.transform.stream.StreamResult;
+import javax.xml.transform.stream.StreamSource;
+
+import org.apache.maven.xml.Factories;
+import org.apache.maven.xml.sax.filter.AbstractSAXFilter;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.xml.sax.InputSource;
+import org.xml.sax.SAXException;
+import org.xml.sax.XMLFilter;
+import org.xml.sax.XMLReader;
+import org.xml.sax.ext.LexicalHandler;
+import org.xml.sax.helpers.XMLFilterImpl;
+
+/**
+ * Some tests to help understand the chain of events regarding XMLFilters, 
XMLReaders, LexicalHandlers and Transformers
+ * 
+ * @author Robert Scholte
+ *
+ */
+public class LexicalHandlerVerifier

Review comment:
   This class is not a unittest, but was written to better understand how 
chaining XML filters works. None of the Maven code is targeted.
   Now with all XMLFilters in place, I won't need this class anymore. To 
prevent similar questions in the future, let's remove this class.





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2020-06-14 Thread GitBox


rfscholte commented on a change in pull request #286:
URL: https://github.com/apache/maven/pull/286#discussion_r439854934



##
File path: 
maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuilder.java
##
@@ -269,7 +273,7 @@ private DependencyResolutionResult resolveDependencies( 
MavenProject project, Re
 return ids;
 }
 
-private ModelBuildingRequest getModelBuildingRequest( InternalConfig 
config )
+private ModelBuildingRequest getModelBuildingRequest( InternalConfig 
config, File pomFile )

Review comment:
   Is now solved with the TransformerContext, can be 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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2020-06-14 Thread GitBox


rfscholte commented on a change in pull request #286:
URL: https://github.com/apache/maven/pull/286#discussion_r439853954



##
File path: 
maven-core/src/main/java/org/apache/maven/project/ProjectModelResolver.java
##
@@ -285,6 +275,16 @@ public ModelSource resolveModel( final Dependency 
dependency )
 }
 
 dependency.setVersion( 
versionRangeResult.getHighestVersion().toString() );
+
+if ( modelPool != null )
+{
+Model model = Optional.ofNullable( modelPool.get( 
dependency.getGroupId(), dependency.getArtifactId(),

Review comment:
   Good catch, this line of code has moved a lot, including refactoring. 
Indeed no need for the optional 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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2020-06-14 Thread GitBox


rfscholte commented on a change in pull request #286:
URL: https://github.com/apache/maven/pull/286#discussion_r439853720



##
File path: 
maven-model-builder/src/main/java/org/apache/maven/model/building/AbstractModelSourceTransformer.java
##
@@ -0,0 +1,196 @@
+package org.apache.maven.model.building;
+
+/*
+ * 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.
+ */
+
+import java.io.FileInputStream;
+import java.io.FilterInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.PipedInputStream;
+import java.io.PipedOutputStream;
+import java.nio.file.Path;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import javax.xml.parsers.ParserConfigurationException;
+import javax.xml.transform.TransformerConfigurationException;
+import javax.xml.transform.TransformerException;
+import javax.xml.transform.TransformerFactory;
+import javax.xml.transform.sax.SAXResult;
+import javax.xml.transform.sax.SAXSource;
+import javax.xml.transform.sax.TransformerHandler;
+import javax.xml.transform.stream.StreamResult;
+
+import org.apache.maven.xml.Factories;
+import org.apache.maven.xml.sax.ext.CommentRenormalizer;
+import org.apache.maven.xml.sax.filter.AbstractSAXFilter;
+import org.xml.sax.SAXException;
+
+/**
+ * 

Review comment:
   Removing the Javadoc will fail the build. All public classes must have 
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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2020-06-14 Thread GitBox


rfscholte commented on a change in pull request #286:
URL: https://github.com/apache/maven/pull/286#discussion_r439853622



##
File path: 
maven-model-builder/src/main/java/org/apache/maven/model/building/AbstractModelSourceTransformer.java
##
@@ -0,0 +1,196 @@
+package org.apache.maven.model.building;
+
+/*
+ * 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.
+ */
+
+import java.io.FileInputStream;
+import java.io.FilterInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.PipedInputStream;
+import java.io.PipedOutputStream;
+import java.nio.file.Path;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import javax.xml.parsers.ParserConfigurationException;
+import javax.xml.transform.TransformerConfigurationException;
+import javax.xml.transform.TransformerException;
+import javax.xml.transform.TransformerFactory;
+import javax.xml.transform.sax.SAXResult;
+import javax.xml.transform.sax.SAXSource;
+import javax.xml.transform.sax.TransformerHandler;
+import javax.xml.transform.stream.StreamResult;
+
+import org.apache.maven.xml.Factories;
+import org.apache.maven.xml.sax.ext.CommentRenormalizer;
+import org.apache.maven.xml.sax.filter.AbstractSAXFilter;
+import org.xml.sax.SAXException;
+
+/**
+ * 
+ * @author Robert Scholte
+ * @since 3.7.0
+ */
+public abstract class AbstractModelSourceTransformer
+implements ModelSourceTransformer
+{
+private static final AtomicInteger TRANSFORM_THREAD_COUNTER = new 
AtomicInteger();
+
+private final TransformerFactory transformerFactory = 
Factories.newTransformerFactory();
+
+protected abstract AbstractSAXFilter getSAXFilter( Path pomFile, 
TransformerContext context )
+throws TransformerConfigurationException, SAXException, 
ParserConfigurationException;
+
+protected OutputStream filterOutputStream( OutputStream outputStream, Path 
pomFile )
+{
+return outputStream;
+}
+
+protected TransformerHandler getTransformerHandler( Path pomFile )
+throws IOException, 
org.apache.maven.model.building.TransformerException
+{
+return null;
+}
+
+@Override
+public final InputStream transform( Path pomFile, TransformerContext 
context )
+throws IOException, 
org.apache.maven.model.building.TransformerException
+{
+final TransformerHandler transformerHandler = getTransformerHandler( 
pomFile );
+
+final AbstractSAXFilter filter;
+try
+{
+filter = getSAXFilter( pomFile, context );
+filter.setLexicalHandler( transformerHandler );
+}
+catch ( TransformerConfigurationException | SAXException | 
ParserConfigurationException e )
+{
+throw new org.apache.maven.model.building.TransformerException( e 
);
+}
+
+final SAXSource transformSource =
+new SAXSource( filter,
+   new org.xml.sax.InputSource( new FileInputStream( 
pomFile.toFile() ) ) );
+
+final PipedOutputStream pout = new PipedOutputStream();
+final PipedInputStream pipedInputStream = new PipedInputStream( pout );
+
+OutputStream out = filterOutputStream( pout, pomFile );
+
+final javax.xml.transform.Result result; 
+if ( transformerHandler == null )
+{
+result = new StreamResult( out );
+}
+else
+{
+result = new SAXResult( transformerHandler );
+( (SAXResult) result ).setLexicalHandler( new CommentRenormalizer( 
transformerHandler ) );
+transformerHandler.setResult( new StreamResult( out ) );
+}
+
+IOExceptionHandler eh = new IOExceptionHandler();
+
+Thread transformThread = new Thread( () -> 
+{
+try ( PipedOutputStream pos = pout )
+{
+transformerFactory.newTransformer().transform( 
transformSource, result );
+}
+catch ( TransformerException | IOException e )
+{
+eh.uncaughtException( Thread.currentThread(), e );
+}
+}, "TransformThread-" + 

[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2020-06-14 Thread GitBox


rfscholte commented on a change in pull request #286:
URL: https://github.com/apache/maven/pull/286#discussion_r439853476



##
File path: 
maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java
##
@@ -646,8 +657,33 @@ else if ( modelSource instanceof FileModelSource )
 {
 model.setPomFile( ( (FileModelSource) modelSource ).getFile() );
 }
-
 problems.setSource( model );
+
+modelValidator.validateFileModel( model, request, problems );
+request.setFileModel( model );
+
+if ( Features.buildConsumer().isActive() && pomFile != null )
+{
+try

Review comment:
   Building the model is matter of a lot of steps. I gave it a try, but it 
actually made the code harder to understand, because some clear if/else or 
foreach statements make more sense within this context.
   One might give it a try, but not as part of this ticket.





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2020-06-14 Thread GitBox


rfscholte commented on a change in pull request #286:
URL: https://github.com/apache/maven/pull/286#discussion_r439853141



##
File path: 
maven-core/src/main/java/org/apache/maven/internal/aether/ConsumerModelSourceTransformer.java
##
@@ -0,0 +1,111 @@
+package org.apache.maven.internal.aether;
+
+/*
+ * 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.
+ */
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+
+import javax.xml.parsers.ParserConfigurationException;
+import javax.xml.stream.XMLInputFactory;
+import javax.xml.stream.XMLStreamException;
+import javax.xml.stream.XMLStreamReader;
+import javax.xml.transform.OutputKeys;
+import javax.xml.transform.Transformer;
+import javax.xml.transform.TransformerConfigurationException;
+import javax.xml.transform.sax.SAXTransformerFactory;
+import javax.xml.transform.sax.TransformerHandler;
+
+import org.apache.maven.model.building.AbstractModelSourceTransformer;
+import org.apache.maven.model.building.DefaultBuildPomXMLFilterFactory;
+import org.apache.maven.model.building.TransformerContext;
+import org.apache.maven.xml.Factories;
+import org.apache.maven.xml.internal.DefaultConsumerPomXMLFilterFactory;
+import org.apache.maven.xml.sax.filter.AbstractSAXFilter;
+import org.xml.sax.SAXException;
+
+class ConsumerModelSourceTransformer extends AbstractModelSourceTransformer
+{
+@Override
+protected AbstractSAXFilter getSAXFilter( Path pomFile, TransformerContext 
context )
+throws TransformerConfigurationException, SAXException, 
ParserConfigurationException
+{
+return new DefaultConsumerPomXMLFilterFactory( new 
DefaultBuildPomXMLFilterFactory( context ) ).get( pomFile );
+}
+
+/**
+ * This transformer will ensure that encoding and version are kept.
+ * However, it cannot prevent:
+ * 
+ *   line-endings will be unix-style(LF)
+ *   attributes will be on one line
+ *   Unnecessary whitespace before the rootelement will be remove 

Review comment:
   And first bullet is not true anymore, it will be the OS specific 
line-endings





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2020-03-14 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r392589127
 
 

 ##
 File path: 
maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelSourceTransformer.java
 ##
 @@ -0,0 +1,115 @@
+package org.apache.maven.model.building;
+
+/*
+ * 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.
+ */
+
+import java.io.FileInputStream;
+import java.io.FilterOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.PipedInputStream;
+import java.io.PipedOutputStream;
+import java.nio.file.Path;
+
+import javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Singleton;
+import javax.xml.parsers.ParserConfigurationException;
+import javax.xml.transform.TransformerConfigurationException;
+import javax.xml.transform.TransformerException;
+import javax.xml.transform.TransformerFactory;
+import javax.xml.transform.sax.SAXSource;
+import javax.xml.transform.stream.StreamResult;
+
+import org.apache.maven.xml.Factories;
+import org.apache.maven.xml.sax.filter.BuildPomXMLFilterFactory;
+import org.apache.maven.xml.sax.filter.BuildPomXMLFilterListener;
+import org.eclipse.sisu.Nullable;
+import org.xml.sax.SAXException;
+
+/**
+ * 
+ * @author Robert Scholte
+ * @since 3.7.0
+ */
+@Named
+@Singleton
+public class DefaultModelSourceTransformer implements ModelSourceTransformer
+{
+@Inject
+@Nullable
+private BuildPomXMLFilterListener xmlFilterListener;
+
+@Override
+public final InputStream transform( Path pomFile, TransformerContext 
context )
+throws IOException, TransformerConfigurationException, SAXException, 
ParserConfigurationException
+{
+final BuildPomXMLFilterFactory buildPomXMLFilterFactory = new 
DefaultBuildPomXMLFilterFactory( context );
+final TransformerFactory transformerFactory = 
Factories.newTransformerFactory() ;
+
+final PipedOutputStream pipedOutputStream  = new PipedOutputStream();
+final PipedInputStream pipedInputStream  = new PipedInputStream( 
pipedOutputStream );
+
+final SAXSource transformSource =
+new SAXSource( buildPomXMLFilterFactory.get( pomFile ),
+   new org.xml.sax.InputSource( new FileInputStream( 
pomFile.toFile() ) ) );
+
+OutputStream out;
+if ( xmlFilterListener != null )
+{
+out = new FilterOutputStream( pipedOutputStream )
+{
+@Override
+public void write( byte[] b, int off, int len )
+throws IOException
+{
+super.write( b, off, len );
+xmlFilterListener.write( pomFile, b, off, len );
+}  
+};
+}
+else
+{
+out = pipedOutputStream;
+}
+
+final StreamResult result = new StreamResult( out );
+
+final Runnable runnable = new Runnable()
+{
+@Override
+public void run()
+{
+try ( PipedOutputStream out = pipedOutputStream )
+{
+transformerFactory.newTransformer().transform( 
transformSource, result );
+}
+catch ( TransformerException | IOException e )
+{
+throw new RuntimeException( e );
 
 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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2020-03-14 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r392586609
 
 

 ##
 File path: 
maven-model-builder/src/main/java/org/apache/maven/model/io/DefaultModelReader.java
 ##
 @@ -48,14 +54,44 @@
 public class DefaultModelReader
 implements ModelReader
 {
+@Inject
+private ModelSourceTransformer transformer;
 
+public void setTransformer( ModelSourceTransformer transformer )
+{
+this.transformer = transformer;
+}
+
 @Override
 public Model read( File input, Map options )
 throws IOException
 {
 Objects.requireNonNull( input, "input cannot be null" );
 
-Model model = read( new FileInputStream( input ), options );
+TransformerContext context = null;
+if ( options != null )
+{
+context = (TransformerContext) options.get( "transformerContext" );
+}
+
+final InputStream is;
+if ( context == null )
+{
+is = new FileInputStream( input );
+}
+else
+{
+try
+{
+is = transformer.transform( input.toPath(), context );
 
 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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2020-03-14 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r392582965
 
 

 ##
 File path: 
maven-core/src/main/java/org/apache/maven/project/ReactorModelPool.java
 ##
 @@ -19,53 +19,120 @@
  * under the License.
  */
 
-import java.io.File;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Collections;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Map;
+import java.util.NoSuchElementException;
+import java.util.Objects;
+import java.util.Set;
+
+import org.apache.maven.model.Model;
 
 /**
- * Holds all POM files that are known to the reactor. This allows the project 
builder to resolve imported POMs from the
+ * Holds all Models that are known to the reactor. This allows the project 
builder to resolve imported Models from the
  * reactor when building another project's effective model.
  *
  * @author Benjamin Bentmann
+ * @Robert Scholte
  */
 class ReactorModelPool
 {
+private final Map> modelsByGa = new HashMap<>();
+
+private final Map modelsByPath = new HashMap<>();
+
+/**
+ * This used to be the only method, which  
+ *  
+ * @param groupId, never {@code null}
+ * @param artifactId, never {@code null}
+ * @param version, can be {@code null}
+ * @return the matching model
+ * @throws IllegalStateException if version was null and multiple modules 
share the same groupId + artifactId
+ * @throws NoSuchElementException if model could not be found
+ */
+public Model get( String groupId, String artifactId, String version )
+throws IllegalStateException, NoSuchElementException
+{
+return modelsByGa.getOrDefault( new GAKey( groupId, artifactId ), 
Collections.emptySet() ).stream()
+.filter( m -> version == null || version.equals( 
getVersion( m ) ) )
+.reduce( ( a, b ) -> 
+{
+throw new IllegalStateException( "Multiple modules 
with key "
++ a.getGroupId() + ':' + a.getArtifactId() );
+} ).orElse( null );
+}
 
-private final Map pomFiles = new HashMap<>();
-
-public File get( String groupId, String artifactId, String version )
+public Model get( Path path )
 {
-return pomFiles.get( new CacheKey( groupId, artifactId, version ) );
+final Path pomFile;
+if ( Files.isDirectory( path ) )
+{
+pomFile = path.resolve( "pom.xml" );
+}
+else
+{
+pomFile = path;
+}
+return modelsByPath.get( pomFile );
+}
+
+private String getVersion( Model model )
+{
+String version = model.getVersion();
+if ( version == null && model.getParent() != null )
+{
+version = model.getParent().getVersion();
+}
+return version;
 }
 
-public void put( String groupId, String artifactId, String version, File 
pomFile )
+static class Builder
 {
-pomFiles.put( new CacheKey( groupId, artifactId, version ), pomFile );
+private ReactorModelPool pool = new ReactorModelPool();
+
+Builder put( Path pomFile, Model model )
+{
+pool.modelsByPath.put( pomFile, model );
+pool.modelsByGa.computeIfAbsent( new GAKey( getGroupId( model ), 
model.getArtifactId() ),
+ k -> new HashSet() ).add( 
model );
+return this;
+}
+
+ReactorModelPool build() 
+{
+return pool;
+}
+
+private static String getGroupId( Model model )
+{
+String groupId = model.getGroupId();
+if ( groupId == null && model.getParent() != null )
+{
+groupId = model.getParent().getGroupId();
+}
+return groupId;
+}
 }
 
-private static final class CacheKey
+private static final class GAKey
 {
 
 private final String groupId;
 
 private final String artifactId;
 
-private final String version;
-
 private final int hashCode;
 
-CacheKey( String groupId, String artifactId, String version )
+GAKey( String groupId, String artifactId )
 
 Review comment:
   It will be stored without version into `private final Map>`, so you can still get it including the version. This makes it 
possible to get the dependency (without version) from the reactor if and only 
if there's one version.  During build Maven ensures you're picking up reactor 
files, while the uploaded pom will inject this version.


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 

[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2020-03-14 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r392578080
 
 

 ##
 File path: 
maven-xml/src/test/java/org/apache/maven/xml/sax/LexicalHandlerVerifier.java
 ##
 @@ -0,0 +1,277 @@
+package org.apache.maven.xml.sax;
+
+/*
+ * 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.
+ */
+
+import static org.xmlunit.assertj.XmlAssert.assertThat;
+
+import java.io.StringReader;
+import java.io.StringWriter;
+import java.io.Writer;
+import java.util.Arrays;
+
+import javax.xml.transform.OutputKeys;
+import javax.xml.transform.Source;
+import javax.xml.transform.Transformer;
+import javax.xml.transform.sax.SAXResult;
+import javax.xml.transform.sax.SAXSource;
+import javax.xml.transform.sax.SAXTransformerFactory;
+import javax.xml.transform.sax.TransformerHandler;
+import javax.xml.transform.stream.StreamResult;
+import javax.xml.transform.stream.StreamSource;
+
+import org.apache.maven.xml.Factories;
+import org.apache.maven.xml.sax.filter.AbstractSAXFilter;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.xml.sax.InputSource;
+import org.xml.sax.SAXException;
+import org.xml.sax.XMLFilter;
+import org.xml.sax.XMLReader;
+import org.xml.sax.ext.LexicalHandler;
+import org.xml.sax.helpers.XMLFilterImpl;
+
+/**
+ * Some tests to help understand the chain of events regarding XMLFilters, 
XMLReaders, LexicalHandlers and Transformers
+ * 
+ * @author Robert Scholte
+ *
+ */
+public class LexicalHandlerVerifier
+{
+private static final String SAX_PROPERTIES_LEXICAL_HANDLER = 
"http://xml.org/sax/properties/lexical-handler;;
+
+@Rule
+public ExpectedException expectedException = ExpectedException.none();
+
+@Test
+public void parseXmlReader() throws Exception
+{
+expectedException.expect( UnsupportedOperationException.class );
+expectedException.expectMessage( "LexicalHandlerVerifier" );
+
+XMLReader reader = Factories.newXMLReader();
+reader.setProperty( SAX_PROPERTIES_LEXICAL_HANDLER, new 
UnsupportedOperationExceptionLexicalHandler() );
+
+InputSource inputSource = new InputSource( new StringReader( 
"" ) ); 
+reader.parse( inputSource );
+}
+
+@Test
+public void parseXmlFilter() throws Exception
+{
+expectedException.expect( UnsupportedOperationException.class );
+expectedException.expectMessage( "LexicalHandlerVerifier" );
+
+XMLReader reader = Factories.newXMLReader();
+reader.setProperty( SAX_PROPERTIES_LEXICAL_HANDLER, new 
UnsupportedOperationExceptionLexicalHandler() );
+
+XMLFilter filter = new XMLFilterImpl( reader );
+
+InputSource inputSource = new InputSource( new StringReader( 
"" ) ); 
+filter.parse( inputSource );
+}
+
+@Test
+public void transformXmlReader() throws Exception
+{
+Writer writer = new StringWriter();
+StreamResult result = new StreamResult( writer );
+
+SAXTransformerFactory transformerFactory = (SAXTransformerFactory) 
Factories.newTransformerFactory();
+TransformerHandler transformerHandler = 
transformerFactory.newTransformerHandler();
+transformerHandler.setResult( result );
+Transformer transformer = transformerFactory.newTransformer();
+transformer.setOutputProperty( OutputKeys.OMIT_XML_DECLARATION, "yes" 
);
+
+Source xmlSource = new StreamSource( new StringReader( 
"" ) );
+
+SAXResult transformResult = new SAXResult( transformerHandler );
+transformResult.setLexicalHandler( new SortCommentLexicalHandler( 
transformerHandler ) );
+transformer.transform( xmlSource, transformResult );
+
+assertThat( writer.toString() ).and( ""
 
 Review comment:
   Yes, `MavenITmng2254PomEncodingTest` in maven-integration-test that is 
specifically written for encoding issues. 
   Without 
github.com/apache/maven/pull/286/files#diff-9037bac1b0c7ac509f2f375f84d60f8dR1191-R1217
 these would fail.


This is an 

[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2020-03-14 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r392577192
 
 

 ##
 File path: 
maven-model-builder/src/main/java/org/apache/maven/feature/Features.java
 ##
 @@ -0,0 +1,63 @@
+package org.apache.maven.feature;
+
+/*
+ * 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.
+ */
+
+/**
+ * Centralized class for feature information
+ * 
+ * @author Robert Scholte
+ * @since 3.7.0
+ */
+public final class Features
+{
+private Features() 
+{
+}
+
+private static final Feature BUILDCONSUMER = new Feature( 
"maven.experimental.buildconsumer", "true" );
+
+public static Feature buildConsumer()
+{
+return BUILDCONSUMER;
+}
+
+/**
+ * Represents some feature
+ * 
+ * @author Robert Scholte
+ * @since 3.7.0
+ */
+public static class Feature
+{
+private final boolean active;
+
+Feature( String name, String defaultValue )
+{
+active = "true".equals( System.getProperty( name, defaultValue ) );
 
 Review comment:
   `Boolean#getBoolean` doesn't support a default value


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2020-03-02 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r386618866
 
 

 ##
 File path: 
maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuilder.java
 ##
 @@ -1043,6 +1115,120 @@ private String findProfilesXml( ModelBuildingResult 
result, Map p
 
 return null;
 }
+
+private Collection getTransformersForArtifact( final 
org.eclipse.aether.artifact.Artifact artifact,
+
TransformerContext context )
+{
+Collection transformers = new ArrayList<>();
+if ( "pom".equals( artifact.getExtension() ) )
+{
+final SAXTransformerFactory transformerFactory =
+(SAXTransformerFactory) Factories.newTransformerFactory();
+
+transformers.add( new FileTransformer()
+{
+@Override
+public InputStream transformData( File file )
+throws IOException, TransformException
+{
+final PipedOutputStream pipedOutputStream  = new 
PipedOutputStream();
+final PipedInputStream pipedInputStream  = new 
PipedInputStream( pipedOutputStream );
+
+final TransformerHandler transformerHandler =
+getTransformerHandler( transformerFactory, file );
+
+BuildPomXMLFilterFactory buildPomXmlFactory = new 
DefaultBuildPomXMLFilterFactory( context );
+
+final SAXSource transformSource;
+try
+{
+AbstractSAXFilter filter =
+new ConsumerPomXMLFilterFactory( 
buildPomXmlFactory ).get( file.toPath() );
+filter.setLexicalHandler( transformerHandler );
+
+transformSource =
+new SAXSource( filter, new InputSource( new 
FileInputStream( file ) ) );
+}
+catch ( SAXException | ParserConfigurationException | 
TransformerConfigurationException e )
+{   
+throw new TransformException( "Failed to create a 
consumerPomXMLFilter", e );
+}
+
+transformerHandler.setResult( new StreamResult( 
pipedOutputStream ) );
+
+SAXResult transformResult = new SAXResult( 
transformerHandler );
+
+ExecutorService executorService = 
Executors.newSingleThreadExecutor();
+executorService.execute( () -> 
 
 Review comment:
   Good catch, I should use the thread implementation 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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2020-03-02 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r386618453
 
 

 ##
 File path: 
maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuilder.java
 ##
 @@ -375,9 +454,7 @@ private ModelSource createStubModelSource( Artifact 
artifact )
 
 boolean noErrors =
 build( results, interimResults, projectIndex, pomFiles, new 
LinkedHashSet<>(), true, recursive,
-   config );
-
-populateReactorModelPool( modelPool, interimResults );
+   config, poolBuilder );
 
 Review comment:
   The reactormodelpool was always there and holds all models of the reactor 
during one project build. In case Maven is being reused, the reactormodelpool 
is always refreshed. Having such pool is required for building the graph. This 
pool holds exactly the information required to get versions by path or 
groupId+artifactId.


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2020-03-02 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r386614058
 
 

 ##
 File path: 
maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuilder.java
 ##
 @@ -266,16 +302,49 @@ private DependencyResolutionResult resolveDependencies( 
MavenProject project, Re
 return ids;
 }
 
-private ModelBuildingRequest getModelBuildingRequest( InternalConfig 
config )
+private ModelBuildingRequest getModelBuildingRequest( InternalConfig 
config, File pomFile )
 {
 ProjectBuildingRequest configuration = config.request;
 
 ModelBuildingRequest request = new DefaultModelBuildingRequest();
 
 RequestTrace trace = RequestTrace.newChild( null, configuration 
).newChild( request );
 
+RepositorySystemSession repoSession;
+if ( Features.buildConsumer().isActive() )
+{
+TransformerContext context = new TransformerContext()
+{
+@Override
+public String getUserProperty( String key )
+{
+return config.session.getUserProperties().get( key );
+}
+
+@Override
+public Model getRawModel( Path p )
+{
+return config.modelPool.get( p );
+}
+
+@Override
+public Model getRawModel( String groupId, String artifactId )
+{
+return config.modelPool.get( groupId, artifactId, null );
 
 Review comment:
   It has never been, see 
https://maven.apache.org/ref/3.6.3/maven-core/xref/org/apache/maven/project/ReactorModelPool.html
   We should be safe because making the buildplan is done in a single thread. 
Once the buildplan is ready, the builder kicks in and can execute with multiple 
threads.


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2020-03-02 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r386607205
 
 

 ##
 File path: 
maven-xml/src/test/java/org/apache/maven/xml/sax/filter/ReactorDependencyXMLFilterTest.java
 ##
 @@ -0,0 +1,145 @@
+package org.apache.maven.xml.sax.filter;
+
+/*
+ * 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.
+ */
+import static org.xmlunit.assertj.XmlAssert.assertThat;
+
+import javax.xml.parsers.ParserConfigurationException;
+import javax.xml.transform.TransformerException;
+
+import org.junit.Test;
+import org.xml.sax.SAXException;
+
+public class ReactorDependencyXMLFilterTest extends AbstractXMLFilterTests
+{
+@Override
+protected ReactorDependencyXMLFilter getFilter()
+throws TransformerException, SAXException, ParserConfigurationException
+{
+return new ReactorDependencyXMLFilter( (g, a) -> "1.0.0" );
+}
+
+@Test
+public void testDefaultDependency() throws Exception
+{
+String input = ""
++ "GROUPID"
++ "ARTIFACTID"
++ "VERSION"
++ "";
+String expected = input;
+
+String actual = transform( input );
+
+assertThat( actual ).isEqualTo( expected );
+}
+
+@Test
+public void testManagedDependency() throws Exception
+{
+ReactorDependencyXMLFilter filter = new ReactorDependencyXMLFilter( 
(g, a) -> null );
+
+String input = ""
++ "GROUPID"
++ "ARTIFACTID"
++ "";
+String expected = input;
+
+String actual = transform( input, filter );
+
+assertThat( actual ).isEqualTo( expected );
+}
+
+@Test
+public void testReactorDependency() throws Exception
+{
+String input = ""
++ "GROUPID"
++ "ARTIFACTID"
++ "";
+String expected = ""
++ "GROUPID"
++ "ARTIFACTID"
++ "1.0.0"
++ "";
+
+String actual = transform( input );
+
+assertThat( actual ).isEqualTo( expected );
+}
+
+@Test
+public void testReactorDependencyLF() throws Exception
 
 Review comment:
   This test is about adding the version in the expected position, respecting 
proper line ending and indents. Checking if `1.0` was added 
was already done in the previous test.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2020-03-02 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r386599766
 
 

 ##
 File path: 
maven-core/src/main/java/org/apache/maven/internal/aether/DefaultRepositorySystemSessionFactory.java
 ##
 @@ -238,9 +263,83 @@ else if ( request.isUpdateSnapshots() )
 mavenRepositorySystem.injectProxy( session, 
request.getPluginArtifactRepositories() );
 mavenRepositorySystem.injectAuthentication( session, 
request.getPluginArtifactRepositories() );
 
+if ( Boolean.getBoolean( "maven.experimental.buildconsumer" ) )
+{
+session.setFileTransformerManager( newFileTransformerManager() );
+}
 return session;
 }
 
+private FileTransformerManager newFileTransformerManager()
+{
+return new FileTransformerManager()
+{
+@Override
+public Collection getTransformersForArtifact( 
final Artifact artifact )
+{
+Collection transformers = new ArrayList<>();
+if ( "pom".equals( artifact.getExtension() ) )
+{
+final TransformerFactory transformerFactory = 
TransformerFactory.newInstance();
+
+transformers.add( new FileTransformer()
+{
+@Override
+public InputStream transformData( File file )
+throws IOException, TransformException
+{
+final PipedOutputStream pipedOutputStream  = new 
PipedOutputStream();
+final PipedInputStream pipedInputStream  = new 
PipedInputStream( pipedOutputStream );
+
+final SAXSource transformSource;
+try
+{
+transformSource =
+new SAXSource( 
consumerPomXMLFilterFactory.get().get( file.toPath() ),
+   new InputSource( new 
FileReader( file ) ) );
+}
+catch ( SAXException | 
ParserConfigurationException e )
+{   
+e.printStackTrace();
+throw new TransformException( "Failed to 
create a consumerPomXMLFilter", e );
+}
+
+final StreamResult result = new StreamResult( 
pipedOutputStream );
+
+final Runnable runnable = new Runnable()
+{
+@Override
+public void run()
+{
+try ( PipedOutputStream out = 
pipedOutputStream )
+{
+
transformerFactory.newTransformer().transform( transformSource, result );
+}
+catch ( TransformerException | IOException 
e )
+{
+e.printStackTrace();
+throw new RuntimeException( e );
+}
+}
+};
+
+new Thread( runnable ).start();
 
 Review comment:
   The advices I got:
   > 1. I would not use ExecutorService. Instead, I would start a single 
   > thread and give it a unique name, set it to Daemon status.
   > 
   > 2. The exceptions are not managed well. If an IOException occurs in your 
   > solution, you wouldn't know about it.
   I'd suggest to remember this as improvements once somebody hits an issue. So 
far I haven't seen the need to change 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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2020-03-02 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r386592065
 
 

 ##
 File path: 
maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java
 ##
 @@ -733,12 +758,81 @@ private void checkPluginVersions( List 
lineage, ModelBuildingRequest
 private void assembleInheritance( List lineage, 
ModelBuildingRequest request,
   ModelProblemCollector problems )
 {
-for ( int i = lineage.size() - 2; i >= 0; i-- )
+for ( int i = lineage.size() - 2; i >= 1; i-- )
 {
 Model parent = lineage.get( i + 1 ).getModel();
 Model child = lineage.get( i ).getModel();
 inheritanceAssembler.assembleModelInheritance( child, parent, 
request, problems );
 }
+
+// re-read model from file
+if ( Boolean.getBoolean( "maven.experimental.buildconsumer" ) && 
request.isTransformPom() )
+{
+try
+{
+// TODO: parent might be part of reactor... better read all 
lineage items like this?
+Model parent = lineage.get( 1 ).getModel();
+
+Model child = modelProcessor.read( transformData( lineage.get( 
0 ) ), null );
+inheritanceAssembler.assembleModelInheritance( child, parent, 
request, problems );
+
+// sync pomfile, is transient
+child.setPomFile( lineage.get( 0 ).getModel().getPomFile() );
+// overwrite child
+lineage.get( 0 ).setModel( child );
+}
+catch ( IOException | TransformException | SAXException | 
ParserConfigurationException e )
+{
+// this is second read, should not happen
+e.printStackTrace();
+}
+}
+else
+{
+Model parent = lineage.get( 1 ).getModel();
+Model child = lineage.get( 0 ).getModel();
+inheritanceAssembler.assembleModelInheritance( child, parent, 
request, problems );
+}
+}
+
+private InputStream transformData( ModelData modelData )
+throws IOException, TransformException, SAXException, 
ParserConfigurationException
+{
+final TransformerFactory transformerFactory = 
TransformerFactory.newInstance();
+
+final PipedOutputStream pipedOutputStream  = new PipedOutputStream();
+final PipedInputStream pipedInputStream  = new PipedInputStream( 
pipedOutputStream );
+
+// Should always be FileSource for reactor poms
+FileSource source = (FileSource) modelData.getSource();
+
+System.out.println( "transforming " + source.getFile() );
+
+final SAXSource transformSource =
+new SAXSource( buildPomXMLFilterFactory.get().get( 
source.getFile().toPath() ),
+   new org.xml.sax.InputSource( 
modelData.getSource().getInputStream() ) );
+
+final StreamResult result = new StreamResult( pipedOutputStream );
+
+final Runnable runnable = new Runnable()
+{
+@Override
+public void run()
+{
+try ( PipedOutputStream out = pipedOutputStream )
+{
+transformerFactory.newTransformer().transform( 
transformSource, result );
+}
+catch ( TransformerException | IOException e )
+{
+throw new RuntimeException( e );
+}
+}
+};
+
+new Thread( runnable ).start();
 
 Review comment:
   This is not blocking. And I've asked advice on using threads by an expert, 
he would solve it with plain threads.  Somebody can open another issue if 
he/she can show there's a more effective solution.


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2020-03-02 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r386589451
 
 

 ##
 File path: 
maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelCacheManager.java
 ##
 @@ -0,0 +1,74 @@
+package org.apache.maven.model.building;
+
+/*
+ * 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.
+ */
+
+import java.nio.file.Path;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import org.apache.maven.model.Model;
+import org.apache.maven.xml.filter.DependencyKey;
+
+/**
+ * 
+ * @author Robert Scholte
+ * @since 3.7.0
+ */
+@Named
+@Singleton
+public class DefaultModelCacheManager implements ModelCacheManager
+{
+private static final Map MODELCACHE = 
Collections.synchronizedMap( new HashMap() );
 
 Review comment:
   ModelCacheManager has been replaced with existing ReactorModelPool


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2020-02-29 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r386045987
 
 

 ##
 File path: 
maven-core/src/test/java/org/apache/maven/project/ProjectBuilderTest.java
 ##
 @@ -332,8 +320,8 @@ public void testBuildProperties()
 ProjectBuildingRequest configuration = new 
DefaultProjectBuildingRequest();
 configuration.setRepositorySession( 
mavenSession.getRepositorySession() );
 configuration.setResolveDependencies( true );
-List result = projectBuilder.build( 
Collections.singletonList( file ), true, configuration );
-MavenProject project = result.get( 0 ).getProject();
+List result = projectBuilder.build( 
Collections.singletonList(file), true, configuration );
+MavenProject project = result.get(0).getProject();
 
 Review comment:
   Same here, I restored the file, will fix it in a separate 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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2020-02-29 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r386045615
 
 

 ##
 File path: 
maven-core/src/test/java/org/apache/maven/project/AbstractMavenProjectTestCase.java
 ##
 @@ -133,7 +119,11 @@ protected static File getFileForClasspathResource( String 
resource )
 protected ArtifactRepository getLocalRepository()
 throws Exception
 {
-return repositorySystem.createLocalRepository( 
getLocalRepositoryPath() );
+ArtifactRepositoryLayout repoLayout = lookup( 
ArtifactRepositoryLayout.class, "legacy" );
+
+ArtifactRepository r = repositorySystem.createArtifactRepository( 
"local", "file://" + getLocalRepositoryPath().getAbsolutePath(), repoLayout, 
null, null );
 
 Review comment:
   Same response :) fix it separately


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2020-02-29 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r386045562
 
 

 ##
 File path: 
maven-compat/src/test/java/org/apache/maven/project/AbstractMavenProjectTestCase.java
 ##
 @@ -140,7 +121,11 @@ protected static File getFileForClasspathResource( String 
resource )
 protected ArtifactRepository getLocalRepository()
 throws Exception
 {
-return repositorySystem.createLocalRepository( 
getLocalRepositoryPath() );
+ArtifactRepositoryLayout repoLayout = lookup( 
ArtifactRepositoryLayout.class, "legacy" );
+
+ArtifactRepository r = repositorySystem.createArtifactRepository( 
"local", "file://" + getLocalRepositoryPath().getAbsolutePath(), repoLayout, 
null, null );
 
 Review comment:
   I restored the unittest, so now it is back in its original state with the 
bad createArtifactRepository.
   Let's fix that in a separate issue, this one is already too big.


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2020-02-09 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r376787962
 
 

 ##
 File path: 
maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java
 ##
 @@ -816,6 +894,81 @@ private void assembleInheritance( List 
lineage, ModelBuildingRequest
 inheritanceAssembler.assembleModelInheritance( child, parent, 
request, problems );
 }
 }
+
+private InputStream transformData( final Path pomFile )
+throws IOException, TransformerException, SAXException, 
ParserConfigurationException
+{
+final TransformerFactory transformerFactory = 
Factories.newTransformerFactory() ;
+
+final PipedOutputStream pipedOutputStream  = new PipedOutputStream();
+final PipedInputStream pipedInputStream  = new PipedInputStream( 
pipedOutputStream );
+
+// Should always be FileSource for reactor poms
+// System.out.println( "transforming " + pomFile );
+
+final SAXSource transformSource =
+new SAXSource( buildPomXMLFilterFactory.get().get( pomFile ),
+   new org.xml.sax.InputSource( new FileInputStream( 
pomFile.toFile() ) ) );
+
+OutputStream out;
+if ( xmlFilterListener != null )
+{
+out = new FilterOutputStream( pipedOutputStream )
+{
+@Override
+public void write( byte[] b, int off, int len )
+throws IOException
+{
+super.write( b, off, len );
+xmlFilterListener.write( pomFile, b, off, len );
+}  
+};
+}
+else
+{
+out = pipedOutputStream;
+}
+
+final StreamResult result = new StreamResult( out );
+
+final Callable callable = () ->
 
 Review comment:
   Now with the improved caching in place, I should be able to make use of it.
   thanks for the reminder of these comments, I need to improve the exception 
handling here 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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-16 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r335708221
 
 

 ##
 File path: maven-xml/src/main/java/org/apache/maven/xml/SAXEventUtils.java
 ##
 @@ -0,0 +1,38 @@
+package org.apache.maven.xml;
+
+/*
+ * 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.
+ */
+
+/**
+ * Utility class for SAXEvents
+ * 
+ * @author Robert Scholte
+ * @since 4.0.0
+ */
+public final class SAXEventUtils
+{
+private SAXEventUtils()
+{
+}
+
+public static String renameQName( String oldQName, String newLocalName )
+{
+return oldQName.replaceFirst( "[^:]+$", newLocalName );
 
 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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-15 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r335150201
 
 

 ##
 File path: 
maven-xml/src/test/java/org/apache/maven/xml/filter/AbstractXMLFilterTests.java
 ##
 @@ -0,0 +1,95 @@
+package org.apache.maven.xml.filter;
+
+/*
+ * 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.
+ */
+
+import java.io.Reader;
+import java.io.StringReader;
+import java.io.StringWriter;
+import java.io.Writer;
+
+import javax.xml.parsers.ParserConfigurationException;
+import javax.xml.parsers.SAXParserFactory;
+import javax.xml.transform.OutputKeys;
+import javax.xml.transform.Transformer;
+import javax.xml.transform.TransformerException;
+import javax.xml.transform.TransformerFactory;
+import javax.xml.transform.sax.SAXSource;
+import javax.xml.transform.stream.StreamResult;
+
+import org.xml.sax.InputSource;
+import org.xml.sax.SAXException;
+import org.xml.sax.XMLFilter;
+
+public abstract class AbstractXMLFilterTests
+{
+public AbstractXMLFilterTests()
+{
+super();
+}
+
+protected abstract XMLFilter getFilter() throws TransformerException, 
SAXException, ParserConfigurationException;
+
+private void setParent( XMLFilter filter ) throws SAXException, 
ParserConfigurationException
+{
+if( filter.getParent() == null )
+{
+filter.setParent( 
SAXParserFactory.newInstance().newSAXParser().getXMLReader() );
+filter.setFeature( "http://xml.org/sax/features/namespaces;, true 
);
+}
+}
+
+protected String transform( String input )
+throws TransformerException, SAXException, ParserConfigurationException
+{
+return transform( new StringReader( input ) );
+}
+
+protected String transform( Reader input ) throws TransformerException, 
SAXException, ParserConfigurationException
+{
+XMLFilter filter = getFilter();
+setParent( filter );
+return transform( input, filter );
+}
+
+protected String transform( String input, XMLFilter filter ) 
+throws TransformerException, SAXException, ParserConfigurationException
+{
+setParent( filter );
+return transform( new StringReader( input ), filter );
+}
+
+protected String transform( Reader input, XMLFilter filter )
+throws TransformerException, SAXException, ParserConfigurationException
+{
+
+Writer writer = new StringWriter();
+StreamResult result = new StreamResult( writer );
+
+TransformerFactory transformerFactory = 
TransformerFactory.newInstance();
 
 Review comment:
   See `org.apache.maven.xml.Factories`


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-15 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r335149892
 
 

 ##
 File path: maven-xml/src/main/java/org/apache/maven/xml/SAXEventFactory.java
 ##
 @@ -0,0 +1,111 @@
+package org.apache.maven.xml;
+
+/*
+ * 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.
+ */
+
+import org.xml.sax.Attributes;
+import org.xml.sax.ContentHandler;
+import org.xml.sax.Locator;
+
+/**
+ * Factory for SAXEvents
+ * 
+ * @author Robert Scholte
+ * @since 4.0.0
 
 Review comment:
   Version 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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-15 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r335149716
 
 

 ##
 File path: maven-xml/src/main/java/org/apache/maven/xml/SAXEventUtils.java
 ##
 @@ -0,0 +1,38 @@
+package org.apache.maven.xml;
+
+/*
+ * 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.
+ */
+
+/**
+ * Utility class for SAXEvents
+ * 
+ * @author Robert Scholte
+ * @since 4.0.0
 
 Review comment:
   Fixed version


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-15 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r335149584
 
 

 ##
 File path: 
maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java
 ##
 @@ -733,12 +758,81 @@ private void checkPluginVersions( List 
lineage, ModelBuildingRequest
 private void assembleInheritance( List lineage, 
ModelBuildingRequest request,
   ModelProblemCollector problems )
 {
-for ( int i = lineage.size() - 2; i >= 0; i-- )
+for ( int i = lineage.size() - 2; i >= 1; i-- )
 {
 Model parent = lineage.get( i + 1 ).getModel();
 Model child = lineage.get( i ).getModel();
 inheritanceAssembler.assembleModelInheritance( child, parent, 
request, problems );
 }
+
+// re-read model from file
+if ( Boolean.getBoolean( "maven.experimental.buildconsumer" ) && 
request.isTransformPom() )
+{
+try
+{
+// TODO: parent might be part of reactor... better read all 
lineage items like this?
+Model parent = lineage.get( 1 ).getModel();
+
+Model child = modelProcessor.read( transformData( lineage.get( 
0 ) ), null );
+inheritanceAssembler.assembleModelInheritance( child, parent, 
request, problems );
+
+// sync pomfile, is transient
+child.setPomFile( lineage.get( 0 ).getModel().getPomFile() );
+// overwrite child
+lineage.get( 0 ).setModel( child );
+}
+catch ( IOException | TransformException | SAXException | 
ParserConfigurationException e )
+{
+// this is second read, should not happen
+e.printStackTrace();
 
 Review comment:
   This has been replaced. I noticed that the ModelBuilder doesn't contain a 
dependency on a logging framework. Assuming the modelbuilder artifact is used 
by other tools too, I can imagine that logging here is not preferred. Instead 
the ProblemCollector should be used.


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-15 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r335147909
 
 

 ##
 File path: 
maven-core/src/main/java/org/apache/maven/xml/internal/DefaultBuildPomXMLFilterFactory.java
 ##
 @@ -0,0 +1,117 @@
+package org.apache.maven.xml.internal;
+
+/*
+ * 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.
+ */
+
+
+import java.nio.file.Path;
+import java.util.Optional;
+import java.util.function.Function;
+
+import javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import org.apache.maven.execution.MavenSession;
+import org.apache.maven.model.Model;
+import org.apache.maven.model.building.ModelCacheManager;
+import org.apache.maven.xml.filter.BuildPomXMLFilterFactory;
+import org.apache.maven.xml.filter.DependencyKey;
+import org.apache.maven.xml.filter.RelativeProject;
+
+/**
+ * 
+ * @author Robert Scholte
+ * @since 3.7.0
+ */
+@Named
+@Singleton
+public class DefaultBuildPomXMLFilterFactory extends BuildPomXMLFilterFactory
+{
+private MavenSession session;
+
+@Inject
+private ModelCacheManager rawModelCache; 
+
+@Inject
+public DefaultBuildPomXMLFilterFactory( MavenSession session )
+{
+this.session = session;
+}
+
+@Override
+protected Optional getChangelist()
+{
+return Optional.ofNullable( session.getUserProperties().getProperty( 
"changelist" ) );
+}
+
+@Override
+protected Optional getRevision()
+{
+return Optional.ofNullable( session.getUserProperties().getProperty( 
"revision" ) );
+}
+
+@Override
+protected Optional getSha1()
+{
+return Optional.ofNullable( session.getUserProperties().getProperty( 
"sha1" ) );
+}
+
+@Override
+protected Function> getRelativePathMapper()
+{
+return p -> Optional.ofNullable( rawModelCache.get( p ) ).map( m -> 
toRelativeProject( m ) );
+}
+
+@Override
+protected Function getDependencyKeyToVersionMapper()
+{
+return k -> Optional.ofNullable( rawModelCache.get( k ) )
+.map( m -> toVersion( m ) )
+.orElse( null );
+}
+
+private RelativeProject toRelativeProject( final Model m )
 
 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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-15 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r335147791
 
 

 ##
 File path: 
maven-core/src/main/java/org/apache/maven/xml/internal/DefaultBuildPomXMLFilterFactory.java
 ##
 @@ -0,0 +1,117 @@
+package org.apache.maven.xml.internal;
+
+/*
+ * 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.
+ */
+
+
+import java.nio.file.Path;
+import java.util.Optional;
+import java.util.function.Function;
+
+import javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import org.apache.maven.execution.MavenSession;
+import org.apache.maven.model.Model;
+import org.apache.maven.model.building.ModelCacheManager;
+import org.apache.maven.xml.filter.BuildPomXMLFilterFactory;
+import org.apache.maven.xml.filter.DependencyKey;
+import org.apache.maven.xml.filter.RelativeProject;
+
+/**
+ * 
+ * @author Robert Scholte
+ * @since 3.7.0
+ */
+@Named
+@Singleton
+public class DefaultBuildPomXMLFilterFactory extends BuildPomXMLFilterFactory
+{
+private MavenSession session;
+
+@Inject
+private ModelCacheManager rawModelCache; 
+
+@Inject
+public DefaultBuildPomXMLFilterFactory( MavenSession session )
+{
+this.session = session;
+}
+
+@Override
+protected Optional getChangelist()
+{
+return Optional.ofNullable( session.getUserProperties().getProperty( 
"changelist" ) );
+}
+
+@Override
+protected Optional getRevision()
+{
+return Optional.ofNullable( session.getUserProperties().getProperty( 
"revision" ) );
+}
+
+@Override
+protected Optional getSha1()
+{
+return Optional.ofNullable( session.getUserProperties().getProperty( 
"sha1" ) );
+}
+
+@Override
+protected Function> getRelativePathMapper()
+{
+return p -> Optional.ofNullable( rawModelCache.get( p ) ).map( m -> 
toRelativeProject( m ) );
+}
+
+@Override
+protected Function getDependencyKeyToVersionMapper()
+{
+return k -> Optional.ofNullable( rawModelCache.get( k ) )
+.map( m -> toVersion( m ) )
+.orElse( null );
+}
+
+private RelativeProject toRelativeProject( final Model m )
+{
+String groupId = m.getGroupId();
+if ( groupId == null && m.getParent() != null )
+{
+groupId = m.getParent().getGroupId();
+}
+
+String version = m.getVersion();
+if ( version == null && m.getParent() != null )
+{
+version = m.getParent().getVersion();
+}
+
+return new RelativeProject( groupId, m.getArtifactId(), version );
+}
+
+private String toVersion( final Model m )
 
 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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-15 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r335147360
 
 

 ##
 File path: 
maven-core/src/main/java/org/apache/maven/xml/filter/FastForwardFilter.java
 ##
 @@ -0,0 +1,128 @@
+package org.apache.maven.xml.filter;
+
+/*
+ * 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.
+ */
+
+import java.util.ArrayDeque;
+import java.util.Deque;
+
+import org.xml.sax.Attributes;
+import org.xml.sax.ContentHandler;
+import org.xml.sax.SAXException;
+import org.xml.sax.XMLFilter;
+import org.xml.sax.XMLReader;
+import org.xml.sax.helpers.XMLFilterImpl;
+
+/**
+ * This filter will skip all following filters and write directly to the 
output.
+ * Should be used in case of a DOM that should not be effected by other 
filters, even though the elements match 
+ * 
+ * @author Robert Scholte
+ * @since 4.0.0
 
 Review comment:
   Fixed version


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-15 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r335147203
 
 

 ##
 File path: 
maven-core/src/main/java/org/apache/maven/xml/filter/ConsumerPomXMLFilter.java
 ##
 @@ -0,0 +1,73 @@
+package org.apache.maven.xml.filter;
+
+/*
+ * 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.
+ */
+
+import javax.xml.parsers.ParserConfigurationException;
+import javax.xml.parsers.SAXParserFactory;
+
+import org.xml.sax.SAXException;
+import org.xml.sax.XMLFilter;
+import org.xml.sax.XMLReader;
+
+import org.xml.sax.helpers.XMLFilterImpl;
+
+/**
+ * XML Filter to transform pom.xml to consumer pom.
+ * This often means stripping of build-specific information.
+ * 
+ * This filter is used at 2 locations:
+ * - {@link 
org.apache.maven.internal.aether.DefaultRepositorySystemSessionFactory} when 
publishing pom files.
+ * - TODO ???Class when a reactor module is used as dependency. This ensures 
consistency of dependency handling
 
 Review comment:
   Fixed 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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-15 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r335146663
 
 

 ##
 File path: 
maven-core/src/main/java/org/apache/maven/internal/aether/DefaultRepositorySystemSessionFactory.java
 ##
 @@ -238,9 +263,83 @@ else if ( request.isUpdateSnapshots() )
 mavenRepositorySystem.injectProxy( session, 
request.getPluginArtifactRepositories() );
 mavenRepositorySystem.injectAuthentication( session, 
request.getPluginArtifactRepositories() );
 
+if ( Boolean.getBoolean( "maven.experimental.buildconsumer" ) )
+{
+session.setFileTransformerManager( newFileTransformerManager() );
+}
 return session;
 }
 
+private FileTransformerManager newFileTransformerManager()
+{
+return new FileTransformerManager()
+{
+@Override
+public Collection getTransformersForArtifact( 
final Artifact artifact )
+{
+Collection transformers = new ArrayList<>();
+if ( "pom".equals( artifact.getExtension() ) )
+{
+final TransformerFactory transformerFactory = 
TransformerFactory.newInstance();
 
 Review comment:
   Introduced the class `org.apache.maven.xml.Factories`, which contain methods 
to create new instances with preferred properties.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-15 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r334773548
 
 

 ##
 File path: 
maven-core/src/main/java/org/apache/maven/internal/aether/DefaultRepositorySystemSessionFactory.java
 ##
 @@ -238,9 +263,83 @@ else if ( request.isUpdateSnapshots() )
 mavenRepositorySystem.injectProxy( session, 
request.getPluginArtifactRepositories() );
 mavenRepositorySystem.injectAuthentication( session, 
request.getPluginArtifactRepositories() );
 
+if ( Boolean.getBoolean( "maven.experimental.buildconsumer" ) )
 
 Review comment:
   The number of locations went beyond 4, so something had to be improved. 
Introduced a Features class, where you can easily switch the default value.


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-15 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r334773151
 
 

 ##
 File path: 
maven-xml/src/main/java/org/apache/maven/xml/filter/DependencyKey.java
 ##
 @@ -0,0 +1,88 @@
+package org.apache.maven.xml.filter;
+
+/*
+ * 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.
+ */
+
+import java.util.Objects;
+
+/**
+ * 
+ * @author Robert Scholte
+ * @since 3.7.0
+ */
+public class DependencyKey
+{
+private final String groupId;
+
+private final String artifactId;
+
+public DependencyKey( String groupId, String artifactId )
+{
+this.groupId = groupId;
+this.artifactId = artifactId;
 
 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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-15 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r334772817
 
 

 ##
 File path: 
maven-xml/src/test/java/org/apache/maven/xml/filter/ConsumerPomXMLFilterTest.java
 ##
 @@ -0,0 +1,168 @@
+package org.apache.maven.xml.filter;
+
+/*
+ * 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.
+ */
+
+import static org.xmlunit.assertj.XmlAssert.assertThat;
+
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Optional;
+import java.util.function.Function;
+
+import javax.inject.Provider;
+import javax.xml.parsers.ParserConfigurationException;
+
+import org.junit.Test;
+import org.xml.sax.SAXException;
+import org.xml.sax.XMLFilter;
+
+public class ConsumerPomXMLFilterTest extends AbstractXMLFilterTests
+{
+@Override
+protected XMLFilter getFilter() throws SAXException, 
ParserConfigurationException
+{
+final BuildPomXMLFilterFactory buildPomXMLFilterFactory = new 
BuildPomXMLFilterFactory()
+{
+@Override
+protected Optional getSha1()
+{
+return Optional.empty();
+}
+
+@Override
+protected Optional getRevision()
+{
+return Optional.empty();
+}
+
+@Override
+protected Optional getChangelist()
+{
+return Optional.of( "CL" );
+}
+
+@Override
+protected Function> 
getRelativePathMapper()
+{
+return null;
+}
+
+@Override
+protected Function 
getDependencyKeyToVersionMapper()
+{
+return null;
+}
+};
+
+Provider provider = new 
Provider()
+{
+
+@Override
+public BuildPomXMLFilterFactory get()
+{
+return buildPomXMLFilterFactory;
+}
+};
+
+XMLFilter filter = new ConsumerPomXMLFilterFactory( provider )
+{
+}.get( Paths.get( "pom.xml" ) );
+filter.setFeature( "http://xml.org/sax/features/namespaces;, true );
+return filter;
+}
+
+@Test
+public void testAllFilters() throws Exception {
+String input = "\n"
+ + "  \n"
+ + "GROUPID\n"
+ + "PARENT\n"
+ + "VERSION\n"
+ + "../pom.xml\n"
+ + "  \n"
+ + "  PROJECT\n"
+ + "  \n"
+ + "ab\n"
+ + "../cd\n"
+ + "  \n"
+ + "";
+String expected = "\n"
++ "  \n"
++ "GROUPID\n"
++ "PARENT\n"
++ "VERSION\n"
++ "\n"
++ "  \n"
++ "  PROJECT\n"
++ "";
+String actual = transform( input );
+assertThat( actual ).and( expected ).ignoreWhitespace().areIdentical();
+}
+
+@Test
+public void testMe() throws Exception {
+String input = "\r\n" + 
+"http://maven.apache.org/POM/4.0.0\; \r\n" + 
+" 
xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\"\r\n; + 
+" xsi:schemaLocation=\"http://maven.apache.org/POM/4.0.0 
\r\n" + 
+" 
http://maven.apache.org/maven-v4_0_0.xsd\;>\r\n" + 
 
 Review comment:
   I've created https://issues.apache.org/jira/browse/MNG-6778 to fix that in 
the official poms.


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-15 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r334772348
 
 

 ##
 File path: 
maven-core/src/test/java/org/apache/maven/xml/filter/ConsumerPomXMLFilterTest.java
 ##
 @@ -0,0 +1,64 @@
+package org.apache.maven.xml.filter;
+
+/*
+ * 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.
+ */
+
+import static org.xmlunit.assertj.XmlAssert.assertThat;
+
+import org.junit.Before;
+import org.junit.Test;
+
+public class ConsumerPomXMLFilterTest extends AbstractXMLFilterTests
+{
+private ConsumerPomXMLFilter filter;
+
+@Before
+public void setup() throws Exception {
+filter = new ConsumerPomXMLFilter();
+}
+
+@Test
+public void testAllFilters() throws Exception {
+String input = "\n"
+ + "  \n"
+ + "GROUPID\n"
+ + "PARENT\n"
+ + "VERSION\n"
+ + "../pom.xml\n"
+ + "  \n"
+ + "  PROJECT\n"
+ + "  \n"
+ + "ab\n"
+ + "../cd\n"
+ + "  \n"
+ + "";
+String expected = "\n"
++ "  \n"
++ "GROUPID\n"
++ "PARENT\n"
++ "VERSION\n"
++ "\n"
 
 Review comment:
   Okay, let's drop relativePath, so the XML looks as expected. We can always 
decide to optimize the ModelBuilders for 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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-12 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r334255176
 
 

 ##
 File path: 
maven-core/src/main/java/org/apache/maven/internal/aether/DefaultRepositorySystemSessionFactory.java
 ##
 @@ -238,9 +268,82 @@ else if ( request.isUpdateSnapshots() )
 mavenRepositorySystem.injectProxy( session, 
request.getPluginArtifactRepositories() );
 mavenRepositorySystem.injectAuthentication( session, 
request.getPluginArtifactRepositories() );
 
+if ( Boolean.getBoolean( "maven.experimental.buildconsumer" ) )
+{
+session.setFileTransformerManager( newFileTransformerManager() );
+}
 return session;
 }
 
+private FileTransformerManager newFileTransformerManager()
+{
+return new FileTransformerManager()
+{
+@Override
+public Collection getTransformersForArtifact( 
final Artifact artifact )
+{
+Collection transformers = new ArrayList<>();
+if ( "pom".equals( artifact.getExtension() ) )
+{
+final TransformerFactory transformerFactory = 
Factories.newTransformerFactory();
+
+transformers.add( new FileTransformer()
+{
+@Override
+public InputStream transformData( File file )
+throws IOException, TransformException
+{
+final PipedOutputStream pipedOutputStream  = new 
PipedOutputStream();
+final PipedInputStream pipedInputStream  = new 
PipedInputStream( pipedOutputStream );
+
+final SAXSource transformSource;
+try
+{
+transformSource =
+new SAXSource( 
consumerPomXMLFilterFactory.get().get( file.toPath() ),
+   new InputSource( new 
FileReader( file ) ) );
+}
+catch ( SAXException | 
ParserConfigurationException | TransformerConfigurationException e )
+{   
+throw new TransformException( "Failed to 
create a consumerPomXMLFilter", e );
+}
+
+final StreamResult result = new StreamResult( 
pipedOutputStream );
+
+final Callable callable = () ->
+{
+try ( PipedOutputStream out = 
pipedOutputStream )
+{
+
transformerFactory.newTransformer().transform( transformSource, result );
+}
+return null;
+};
+
+ExecutorService executorService = 
Executors.newSingleThreadExecutor();
 
 Review comment:
   Yes, please read 
https://docs.oracle.com/en/java/javase/13/docs/api/java.base/java/io/PipedInputStream.html.
   I don't want to buffer the streams, that can consume too much memory. And it 
is quite easy to get the deadlock: fork, switch to this branch, change default 
value in Features to `true`  and run `mvn test`.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-12 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r334242784
 
 

 ##
 File path: 
maven-core/src/main/java/org/apache/maven/internal/aether/DefaultRepositorySystemSessionFactory.java
 ##
 @@ -238,9 +268,82 @@ else if ( request.isUpdateSnapshots() )
 mavenRepositorySystem.injectProxy( session, 
request.getPluginArtifactRepositories() );
 mavenRepositorySystem.injectAuthentication( session, 
request.getPluginArtifactRepositories() );
 
+if ( Boolean.getBoolean( "maven.experimental.buildconsumer" ) )
+{
+session.setFileTransformerManager( newFileTransformerManager() );
+}
 return session;
 }
 
+private FileTransformerManager newFileTransformerManager()
+{
+return new FileTransformerManager()
+{
+@Override
+public Collection getTransformersForArtifact( 
final Artifact artifact )
+{
+Collection transformers = new ArrayList<>();
+if ( "pom".equals( artifact.getExtension() ) )
+{
+final TransformerFactory transformerFactory = 
Factories.newTransformerFactory();
+
+transformers.add( new FileTransformer()
+{
+@Override
+public InputStream transformData( File file )
+throws IOException, TransformException
+{
+final PipedOutputStream pipedOutputStream  = new 
PipedOutputStream();
+final PipedInputStream pipedInputStream  = new 
PipedInputStream( pipedOutputStream );
+
+final SAXSource transformSource;
+try
+{
+transformSource =
+new SAXSource( 
consumerPomXMLFilterFactory.get().get( file.toPath() ),
+   new InputSource( new 
FileReader( file ) ) );
+}
+catch ( SAXException | 
ParserConfigurationException | TransformerConfigurationException e )
+{   
+throw new TransformException( "Failed to 
create a consumerPomXMLFilter", e );
+}
+
+final StreamResult result = new StreamResult( 
pipedOutputStream );
+
+final Callable callable = () ->
+{
+try ( PipedOutputStream out = 
pipedOutputStream )
+{
+
transformerFactory.newTransformer().transform( transformSource, result );
+}
+return null;
+};
+
+ExecutorService executorService = 
Executors.newSingleThreadExecutor();
 
 Review comment:
   @rmannibucau it seems like this implementation can cause deadlocks (as 
mentioned in the PipedInputStream/PipedOutputStream Javadocs). I see this 
happening when trying to active this feature by default, some unittests start 
to hang. Anyone knows how to solve 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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-03 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r331180213
 
 

 ##
 File path: 
maven-core/src/main/java/org/apache/maven/internal/aether/DefaultRepositorySystemSessionFactory.java
 ##
 @@ -238,9 +263,83 @@ else if ( request.isUpdateSnapshots() )
 mavenRepositorySystem.injectProxy( session, 
request.getPluginArtifactRepositories() );
 mavenRepositorySystem.injectAuthentication( session, 
request.getPluginArtifactRepositories() );
 
+if ( Boolean.getBoolean( "maven.experimental.buildconsumer" ) )
+{
+session.setFileTransformerManager( newFileTransformerManager() );
+}
 return session;
 }
 
+private FileTransformerManager newFileTransformerManager()
+{
+return new FileTransformerManager()
+{
+@Override
+public Collection getTransformersForArtifact( 
final Artifact artifact )
+{
+Collection transformers = new ArrayList<>();
+if ( "pom".equals( artifact.getExtension() ) )
+{
+final TransformerFactory transformerFactory = 
TransformerFactory.newInstance();
+
+transformers.add( new FileTransformer()
+{
+@Override
+public InputStream transformData( File file )
+throws IOException, TransformException
+{
+final PipedOutputStream pipedOutputStream  = new 
PipedOutputStream();
+final PipedInputStream pipedInputStream  = new 
PipedInputStream( pipedOutputStream );
+
+final SAXSource transformSource;
+try
+{
+transformSource =
+new SAXSource( 
consumerPomXMLFilterFactory.get().get( file.toPath() ),
+   new InputSource( new 
FileReader( file ) ) );
+}
+catch ( SAXException | 
ParserConfigurationException e )
+{   
+e.printStackTrace();
+throw new TransformException( "Failed to 
create a consumerPomXMLFilter", e );
+}
+
+final StreamResult result = new StreamResult( 
pipedOutputStream );
+
+final Runnable runnable = new Runnable()
+{
+@Override
+public void run()
+{
+try ( PipedOutputStream out = 
pipedOutputStream )
+{
+
transformerFactory.newTransformer().transform( transformSource, result );
+}
+catch ( TransformerException | IOException 
e )
+{
+e.printStackTrace();
 
 Review comment:
   Now that it is a `Callable`, it is possible to just throw the exceptions. 
https://github.com/apache/maven/commit/b19b05cbf2f30705dffb8c1a541d65096d967d11#diff-f15584934a6ddfdbd298acb2d72bf8f3R312-R319


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-03 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r331179612
 
 

 ##
 File path: 
maven-core/src/main/java/org/apache/maven/internal/aether/DefaultRepositorySystemSessionFactory.java
 ##
 @@ -238,9 +263,83 @@ else if ( request.isUpdateSnapshots() )
 mavenRepositorySystem.injectProxy( session, 
request.getPluginArtifactRepositories() );
 mavenRepositorySystem.injectAuthentication( session, 
request.getPluginArtifactRepositories() );
 
+if ( Boolean.getBoolean( "maven.experimental.buildconsumer" ) )
+{
+session.setFileTransformerManager( newFileTransformerManager() );
+}
 return session;
 }
 
+private FileTransformerManager newFileTransformerManager()
+{
+return new FileTransformerManager()
+{
+@Override
+public Collection getTransformersForArtifact( 
final Artifact artifact )
+{
+Collection transformers = new ArrayList<>();
+if ( "pom".equals( artifact.getExtension() ) )
+{
+final TransformerFactory transformerFactory = 
TransformerFactory.newInstance();
+
+transformers.add( new FileTransformer()
+{
+@Override
+public InputStream transformData( File file )
+throws IOException, TransformException
+{
+final PipedOutputStream pipedOutputStream  = new 
PipedOutputStream();
+final PipedInputStream pipedInputStream  = new 
PipedInputStream( pipedOutputStream );
+
+final SAXSource transformSource;
+try
+{
+transformSource =
+new SAXSource( 
consumerPomXMLFilterFactory.get().get( file.toPath() ),
+   new InputSource( new 
FileReader( file ) ) );
+}
+catch ( SAXException | 
ParserConfigurationException e )
+{   
+e.printStackTrace();
 
 Review comment:
   `e.printStackTrace()` removed 
https://github.com/apache/maven/commit/b19b05cbf2f30705dffb8c1a541d65096d967d11#diff-f15584934a6ddfdbd298acb2d72bf8f3R306-R307


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-03 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r331179612
 
 

 ##
 File path: 
maven-core/src/main/java/org/apache/maven/internal/aether/DefaultRepositorySystemSessionFactory.java
 ##
 @@ -238,9 +263,83 @@ else if ( request.isUpdateSnapshots() )
 mavenRepositorySystem.injectProxy( session, 
request.getPluginArtifactRepositories() );
 mavenRepositorySystem.injectAuthentication( session, 
request.getPluginArtifactRepositories() );
 
+if ( Boolean.getBoolean( "maven.experimental.buildconsumer" ) )
+{
+session.setFileTransformerManager( newFileTransformerManager() );
+}
 return session;
 }
 
+private FileTransformerManager newFileTransformerManager()
+{
+return new FileTransformerManager()
+{
+@Override
+public Collection getTransformersForArtifact( 
final Artifact artifact )
+{
+Collection transformers = new ArrayList<>();
+if ( "pom".equals( artifact.getExtension() ) )
+{
+final TransformerFactory transformerFactory = 
TransformerFactory.newInstance();
+
+transformers.add( new FileTransformer()
+{
+@Override
+public InputStream transformData( File file )
+throws IOException, TransformException
+{
+final PipedOutputStream pipedOutputStream  = new 
PipedOutputStream();
+final PipedInputStream pipedInputStream  = new 
PipedInputStream( pipedOutputStream );
+
+final SAXSource transformSource;
+try
+{
+transformSource =
+new SAXSource( 
consumerPomXMLFilterFactory.get().get( file.toPath() ),
+   new InputSource( new 
FileReader( file ) ) );
+}
+catch ( SAXException | 
ParserConfigurationException e )
+{   
+e.printStackTrace();
 
 Review comment:
   `e.printStackTrace()` 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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-02 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r330389392
 
 

 ##
 File path: 
maven-xml/src/test/java/org/apache/maven/xml/filter/AbstractXMLFilterTests.java
 ##
 @@ -0,0 +1,95 @@
+package org.apache.maven.xml.filter;
+
+/*
+ * 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.
+ */
+
+import java.io.Reader;
+import java.io.StringReader;
+import java.io.StringWriter;
+import java.io.Writer;
+
+import javax.xml.parsers.ParserConfigurationException;
+import javax.xml.parsers.SAXParserFactory;
+import javax.xml.transform.OutputKeys;
+import javax.xml.transform.Transformer;
+import javax.xml.transform.TransformerException;
+import javax.xml.transform.TransformerFactory;
+import javax.xml.transform.sax.SAXSource;
+import javax.xml.transform.stream.StreamResult;
+
+import org.xml.sax.InputSource;
+import org.xml.sax.SAXException;
+import org.xml.sax.XMLFilter;
+
+public abstract class AbstractXMLFilterTests
+{
+public AbstractXMLFilterTests()
+{
+super();
+}
+
+protected abstract XMLFilter getFilter() throws TransformerException, 
SAXException, ParserConfigurationException;
+
+private void setParent( XMLFilter filter ) throws SAXException, 
ParserConfigurationException
+{
+if( filter.getParent() == null )
+{
+filter.setParent( 
SAXParserFactory.newInstance().newSAXParser().getXMLReader() );
+filter.setFeature( "http://xml.org/sax/features/namespaces;, true 
);
+}
+}
+
+protected String transform( String input )
+throws TransformerException, SAXException, ParserConfigurationException
+{
+return transform( new StringReader( input ) );
+}
+
+protected String transform( Reader input ) throws TransformerException, 
SAXException, ParserConfigurationException
+{
+XMLFilter filter = getFilter();
+setParent( filter );
+return transform( input, filter );
+}
+
+protected String transform( String input, XMLFilter filter ) 
+throws TransformerException, SAXException, ParserConfigurationException
+{
+setParent( filter );
+return transform( new StringReader( input ), filter );
+}
+
+protected String transform( Reader input, XMLFilter filter )
+throws TransformerException, SAXException, ParserConfigurationException
+{
+
+Writer writer = new StringWriter();
+StreamResult result = new StreamResult( writer );
+
+TransformerFactory transformerFactory = 
TransformerFactory.newInstance();
 
 Review comment:
   This is all JDK code. I'll leave it up to you to refactor.


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-02 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r330388940
 
 

 ##
 File path: 
maven-core/src/main/java/org/apache/maven/xml/filter/FastForwardFilter.java
 ##
 @@ -0,0 +1,128 @@
+package org.apache.maven.xml.filter;
+
+/*
+ * 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.
+ */
+
+import java.util.ArrayDeque;
+import java.util.Deque;
+
+import org.xml.sax.Attributes;
+import org.xml.sax.ContentHandler;
+import org.xml.sax.SAXException;
+import org.xml.sax.XMLFilter;
+import org.xml.sax.XMLReader;
+import org.xml.sax.helpers.XMLFilterImpl;
+
+/**
+ * This filter will skip all following filters and write directly to the 
output.
+ * Should be used in case of a DOM that should not be effected by other 
filters, even though the elements match 
+ * 
+ * @author Robert Scholte
+ * @since 4.0.0
+ */
+class FastForwardFilter extends XMLFilterImpl
+{
+/**
+ * DOM elements of pom
+ * 
+ * 
+ *  execution.configuration
+ *  plugin.configuration
+ *  plugin.goals
+ *  profile.reports
+ *  project.reports
+ *  reportSet.configuration
+ * 
+ */
+private final Deque state = new ArrayDeque<>();
+
+private int domDepth = 0;
+
+private ContentHandler originalHandler;
+
+FastForwardFilter()
+{
+super();
+}
+
+FastForwardFilter( XMLReader parent )
+{
+super( parent );
+}
+
+@Override
+public void startElement( String uri, String localName, String qName, 
Attributes atts )
+throws SAXException
+{
+super.startElement( uri, localName, qName, atts );
+if ( domDepth > 0 )
 
 Review comment:
   If the XML is invalid, it'll already fail during the first phase. This is 
part of the second phase. This is the best lightwight solution I came up with, 
and there are tests that cover 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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-02 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r330388340
 
 

 ##
 File path: 
maven-core/src/main/java/org/apache/maven/xml/filter/ConsumerPomXMLFilter.java
 ##
 @@ -0,0 +1,73 @@
+package org.apache.maven.xml.filter;
+
+/*
+ * 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.
+ */
+
+import javax.xml.parsers.ParserConfigurationException;
+import javax.xml.parsers.SAXParserFactory;
+
+import org.xml.sax.SAXException;
+import org.xml.sax.XMLFilter;
+import org.xml.sax.XMLReader;
+
+import org.xml.sax.helpers.XMLFilterImpl;
+
+/**
+ * XML Filter to transform pom.xml to consumer pom.
+ * This often means stripping of build-specific information.
+ * 
+ * This filter is used at 2 locations:
+ * - {@link 
org.apache.maven.internal.aether.DefaultRepositorySystemSessionFactory} when 
publishing pom files.
+ * - TODO ???Class when a reactor module is used as dependency. This ensures 
consistency of dependency handling
 
 Review comment:
   No, TODO needs to be resolved. It doesn not apply on the ConsumerFilter, but 
is already solved in the BuildFilter


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-02 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r330387741
 
 

 ##
 File path: 
maven-core/src/main/java/org/apache/maven/internal/aether/DefaultRepositorySystemSessionFactory.java
 ##
 @@ -238,9 +263,83 @@ else if ( request.isUpdateSnapshots() )
 mavenRepositorySystem.injectProxy( session, 
request.getPluginArtifactRepositories() );
 mavenRepositorySystem.injectAuthentication( session, 
request.getPluginArtifactRepositories() );
 
+if ( Boolean.getBoolean( "maven.experimental.buildconsumer" ) )
+{
+session.setFileTransformerManager( newFileTransformerManager() );
+}
 return session;
 }
 
+private FileTransformerManager newFileTransformerManager()
+{
+return new FileTransformerManager()
+{
+@Override
+public Collection getTransformersForArtifact( 
final Artifact artifact )
+{
+Collection transformers = new ArrayList<>();
+if ( "pom".equals( artifact.getExtension() ) )
+{
+final TransformerFactory transformerFactory = 
TransformerFactory.newInstance();
+
+transformers.add( new FileTransformer()
+{
+@Override
+public InputStream transformData( File file )
+throws IOException, TransformException
+{
+final PipedOutputStream pipedOutputStream  = new 
PipedOutputStream();
+final PipedInputStream pipedInputStream  = new 
PipedInputStream( pipedOutputStream );
+
+final SAXSource transformSource;
+try
+{
+transformSource =
+new SAXSource( 
consumerPomXMLFilterFactory.get().get( file.toPath() ),
+   new InputSource( new 
FileReader( file ) ) );
+}
+catch ( SAXException | 
ParserConfigurationException e )
+{   
+e.printStackTrace();
+throw new TransformException( "Failed to 
create a consumerPomXMLFilter", e );
+}
+
+final StreamResult result = new StreamResult( 
pipedOutputStream );
+
+final Runnable runnable = new Runnable()
+{
+@Override
+public void run()
+{
+try ( PipedOutputStream out = 
pipedOutputStream )
+{
+
transformerFactory.newTransformer().transform( transformSource, result );
+}
+catch ( TransformerException | IOException 
e )
+{
+e.printStackTrace();
+throw new RuntimeException( e );
+}
+}
+};
+
+new Thread( runnable ).start();
 
 Review comment:
   To close this discussion: Code is based on examples for PipedStreams 
(requirement for me to lower memoryconsumption), but they still use Threads. 
I'll rewrite them to Callables


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-02 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r330386987
 
 

 ##
 File path: 
maven-core/src/main/java/org/apache/maven/internal/aether/DefaultRepositorySystemSessionFactory.java
 ##
 @@ -238,9 +263,83 @@ else if ( request.isUpdateSnapshots() )
 mavenRepositorySystem.injectProxy( session, 
request.getPluginArtifactRepositories() );
 mavenRepositorySystem.injectAuthentication( session, 
request.getPluginArtifactRepositories() );
 
+if ( Boolean.getBoolean( "maven.experimental.buildconsumer" ) )
+{
+session.setFileTransformerManager( newFileTransformerManager() );
+}
 return session;
 }
 
+private FileTransformerManager newFileTransformerManager()
+{
+return new FileTransformerManager()
+{
+@Override
+public Collection getTransformersForArtifact( 
final Artifact artifact )
+{
+Collection transformers = new ArrayList<>();
+if ( "pom".equals( artifact.getExtension() ) )
+{
+final TransformerFactory transformerFactory = 
TransformerFactory.newInstance();
 
 Review comment:
   Wasn't aware of this sheet. I think it is good to apply them. Only thing I 
want to add: the way I constructed the classes is in a way that nobody else can 
manipulate it, Maven stays in full control. We shouldn't be more vulnerable, 
unless we take classloading order into account.


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-02 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r330386216
 
 

 ##
 File path: 
maven-core/src/main/java/org/apache/maven/internal/aether/DefaultRepositorySystemSessionFactory.java
 ##
 @@ -238,9 +263,83 @@ else if ( request.isUpdateSnapshots() )
 mavenRepositorySystem.injectProxy( session, 
request.getPluginArtifactRepositories() );
 mavenRepositorySystem.injectAuthentication( session, 
request.getPluginArtifactRepositories() );
 
+if ( Boolean.getBoolean( "maven.experimental.buildconsumer" ) )
 
 Review comment:
   Do you have a class in mind? It is being used in model-builder and core.


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-01 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r330274591
 
 

 ##
 File path: 
maven-xml/src/main/java/org/apache/maven/xml/filter/BuildPomXMLFilterFactory.java
 ##
 @@ -0,0 +1,95 @@
+package org.apache.maven.xml.filter;
+
+/*
+ * 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.
+ */
+
+import java.nio.file.Path;
+import java.util.Optional;
+import java.util.function.Function;
+
+import javax.xml.parsers.ParserConfigurationException;
+import javax.xml.parsers.SAXParserFactory;
+
+import org.xml.sax.SAXException;
+import org.xml.sax.XMLReader;
+
+/**
+ * 
+ * @author Robert Scholte
+ * 
+ * @since 3.7.0
+ */
+public abstract class BuildPomXMLFilterFactory
+{
+public final BuildPomXMLFilter get( Path projectFile )
+throws SAXException, ParserConfigurationException
+{
+XMLReader parent = getParent();
+
+if ( getDependencyKeyToVersionMapper() != null )
+{
+ReactorDependencyXMLFilter reactorDependencyXMLFilter =
+new ReactorDependencyXMLFilter( 
getDependencyKeyToVersionMapper() );
+reactorDependencyXMLFilter.setParent( parent );
+parent = reactorDependencyXMLFilter;
+}
+
+if ( getRelativePathMapper() != null )
+{
+ParentXMLFilter parentFilter = new ParentXMLFilter( 
getRelativePathMapper() );
+parentFilter.setProjectPath( projectFile.getParent() );
+parentFilter.setParent( parent );
+parent = parentFilter;
+}
+
+CiFriendlyXMLFilter ciFriendlyFilter = new CiFriendlyXMLFilter();
+getChangelist().ifPresent( ciFriendlyFilter::setChangelist  );
+getRevision().ifPresent( ciFriendlyFilter::setRevision );
+getSha1().ifPresent( ciFriendlyFilter::setSha1 );
+
+if ( ciFriendlyFilter.isSet() )
+{
+ciFriendlyFilter.setParent( parent );
+parent = ciFriendlyFilter;
+}
+
+return new BuildPomXMLFilter( parent );
+}
+
+protected XMLReader getParent() throws SAXException, 
ParserConfigurationException 
+{
+XMLReader xmlReader = 
SAXParserFactory.newInstance().newSAXParser().getXMLReader();
+xmlReader.setFeature( "http://xml.org/sax/features/namespaces;, true );
 
 Review comment:
   
https://github.com/AdoptOpenJDK/openjdk-jdk11/blob/master/src/java.xml/share/classes/com/sun/org/apache/xerces/internal/impl/Constants.java
   
   Maybe this explains 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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-01 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r330265590
 
 

 ##
 File path: 
maven-xml/src/main/java/org/apache/maven/xml/filter/BuildPomXMLFilterFactory.java
 ##
 @@ -0,0 +1,95 @@
+package org.apache.maven.xml.filter;
+
+/*
+ * 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.
+ */
+
+import java.nio.file.Path;
+import java.util.Optional;
+import java.util.function.Function;
+
+import javax.xml.parsers.ParserConfigurationException;
+import javax.xml.parsers.SAXParserFactory;
+
+import org.xml.sax.SAXException;
+import org.xml.sax.XMLReader;
+
+/**
+ * 
+ * @author Robert Scholte
+ * 
+ * @since 3.7.0
+ */
+public abstract class BuildPomXMLFilterFactory
+{
+public final BuildPomXMLFilter get( Path projectFile )
+throws SAXException, ParserConfigurationException
+{
+XMLReader parent = getParent();
+
+if ( getDependencyKeyToVersionMapper() != null )
+{
+ReactorDependencyXMLFilter reactorDependencyXMLFilter =
+new ReactorDependencyXMLFilter( 
getDependencyKeyToVersionMapper() );
+reactorDependencyXMLFilter.setParent( parent );
+parent = reactorDependencyXMLFilter;
+}
+
+if ( getRelativePathMapper() != null )
+{
+ParentXMLFilter parentFilter = new ParentXMLFilter( 
getRelativePathMapper() );
+parentFilter.setProjectPath( projectFile.getParent() );
+parentFilter.setParent( parent );
+parent = parentFilter;
+}
+
+CiFriendlyXMLFilter ciFriendlyFilter = new CiFriendlyXMLFilter();
+getChangelist().ifPresent( ciFriendlyFilter::setChangelist  );
+getRevision().ifPresent( ciFriendlyFilter::setRevision );
+getSha1().ifPresent( ciFriendlyFilter::setSha1 );
+
+if ( ciFriendlyFilter.isSet() )
+{
+ciFriendlyFilter.setParent( parent );
+parent = ciFriendlyFilter;
+}
+
+return new BuildPomXMLFilter( parent );
+}
+
+protected XMLReader getParent() throws SAXException, 
ParserConfigurationException 
+{
+XMLReader xmlReader = 
SAXParserFactory.newInstance().newSAXParser().getXMLReader();
+xmlReader.setFeature( "http://xml.org/sax/features/namespaces;, true );
 
 Review comment:
   Ah, I see. The problem is: even though it looks like a URL, it is just the 
key of this feature. Not including it will mean namespaces will be ignored. 
Some unittests will show you the issue.


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-01 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r330237064
 
 

 ##
 File path: 
maven-xml/src/main/java/org/apache/maven/xml/filter/DependencyKey.java
 ##
 @@ -0,0 +1,88 @@
+package org.apache.maven.xml.filter;
+
+/*
+ * 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.
+ */
+
+import java.util.Objects;
+
+/**
+ * 
+ * @author Robert Scholte
+ * @since 3.7.0
+ */
+public class DependencyKey
+{
+private final String groupId;
+
+private final String artifactId;
+
+public DependencyKey( String groupId, String artifactId )
+{
+this.groupId = groupId;
+this.artifactId = artifactId;
 
 Review comment:
   good catch


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-01 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r330236983
 
 

 ##
 File path: maven-xml/src/main/java/org/apache/maven/xml/SAXEventUtils.java
 ##
 @@ -0,0 +1,38 @@
+package org.apache.maven.xml;
+
+/*
+ * 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.
+ */
+
+/**
+ * Utility class for SAXEvents
+ * 
+ * @author Robert Scholte
+ * @since 4.0.0
+ */
+public final class SAXEventUtils
+{
+private SAXEventUtils()
+{
+}
+
+public static String renameQName( String oldQName, String newLocalName )
+{
+return oldQName.replaceFirst( "[^:]+$", newLocalName );
 
 Review comment:
   Sure, that's a simple change


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-01 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r330235852
 
 

 ##
 File path: 
maven-xml/src/main/java/org/apache/maven/xml/filter/BuildPomXMLFilterFactory.java
 ##
 @@ -0,0 +1,95 @@
+package org.apache.maven.xml.filter;
+
+/*
+ * 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.
+ */
+
+import java.nio.file.Path;
+import java.util.Optional;
+import java.util.function.Function;
+
+import javax.xml.parsers.ParserConfigurationException;
+import javax.xml.parsers.SAXParserFactory;
+
+import org.xml.sax.SAXException;
+import org.xml.sax.XMLReader;
+
+/**
+ * 
+ * @author Robert Scholte
+ * 
+ * @since 3.7.0
+ */
+public abstract class BuildPomXMLFilterFactory
+{
+public final BuildPomXMLFilter get( Path projectFile )
+throws SAXException, ParserConfigurationException
+{
+XMLReader parent = getParent();
+
+if ( getDependencyKeyToVersionMapper() != null )
+{
+ReactorDependencyXMLFilter reactorDependencyXMLFilter =
+new ReactorDependencyXMLFilter( 
getDependencyKeyToVersionMapper() );
+reactorDependencyXMLFilter.setParent( parent );
+parent = reactorDependencyXMLFilter;
+}
+
+if ( getRelativePathMapper() != null )
+{
+ParentXMLFilter parentFilter = new ParentXMLFilter( 
getRelativePathMapper() );
+parentFilter.setProjectPath( projectFile.getParent() );
+parentFilter.setParent( parent );
+parent = parentFilter;
+}
+
+CiFriendlyXMLFilter ciFriendlyFilter = new CiFriendlyXMLFilter();
+getChangelist().ifPresent( ciFriendlyFilter::setChangelist  );
+getRevision().ifPresent( ciFriendlyFilter::setRevision );
+getSha1().ifPresent( ciFriendlyFilter::setSha1 );
+
+if ( ciFriendlyFilter.isSet() )
+{
+ciFriendlyFilter.setParent( parent );
+parent = ciFriendlyFilter;
+}
+
+return new BuildPomXMLFilter( parent );
+}
+
+protected XMLReader getParent() throws SAXException, 
ParserConfigurationException 
+{
+XMLReader xmlReader = 
SAXParserFactory.newInstance().newSAXParser().getXMLReader();
+xmlReader.setFeature( "http://xml.org/sax/features/namespaces;, true );
 
 Review comment:
   Not sure what you're pointing at. You can try to remove it and you'll see 
tests start to fail.


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-01 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r330235426
 
 

 ##
 File path: 
maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java
 ##
 @@ -733,12 +758,81 @@ private void checkPluginVersions( List 
lineage, ModelBuildingRequest
 private void assembleInheritance( List lineage, 
ModelBuildingRequest request,
   ModelProblemCollector problems )
 {
-for ( int i = lineage.size() - 2; i >= 0; i-- )
+for ( int i = lineage.size() - 2; i >= 1; i-- )
 {
 Model parent = lineage.get( i + 1 ).getModel();
 Model child = lineage.get( i ).getModel();
 inheritanceAssembler.assembleModelInheritance( child, parent, 
request, problems );
 }
+
+// re-read model from file
+if ( Boolean.getBoolean( "maven.experimental.buildconsumer" ) && 
request.isTransformPom() )
+{
+try
+{
+// TODO: parent might be part of reactor... better read all 
lineage items like this?
+Model parent = lineage.get( 1 ).getModel();
+
+Model child = modelProcessor.read( transformData( lineage.get( 
0 ) ), null );
+inheritanceAssembler.assembleModelInheritance( child, parent, 
request, problems );
+
+// sync pomfile, is transient
+child.setPomFile( lineage.get( 0 ).getModel().getPomFile() );
+// overwrite child
+lineage.get( 0 ).setModel( child );
+}
+catch ( IOException | TransformException | SAXException | 
ParserConfigurationException e )
+{
+// this is second read, should not happen
+e.printStackTrace();
+}
+}
+else
+{
+Model parent = lineage.get( 1 ).getModel();
+Model child = lineage.get( 0 ).getModel();
+inheritanceAssembler.assembleModelInheritance( child, parent, 
request, problems );
+}
+}
+
+private InputStream transformData( ModelData modelData )
+throws IOException, TransformException, SAXException, 
ParserConfigurationException
+{
+final TransformerFactory transformerFactory = 
TransformerFactory.newInstance();
+
+final PipedOutputStream pipedOutputStream  = new PipedOutputStream();
+final PipedInputStream pipedInputStream  = new PipedInputStream( 
pipedOutputStream );
+
+// Should always be FileSource for reactor poms
+FileSource source = (FileSource) modelData.getSource();
+
+System.out.println( "transforming " + source.getFile() );
+
+final SAXSource transformSource =
+new SAXSource( buildPomXMLFilterFactory.get().get( 
source.getFile().toPath() ),
+   new org.xml.sax.InputSource( 
modelData.getSource().getInputStream() ) );
+
+final StreamResult result = new StreamResult( pipedOutputStream );
+
+final Runnable runnable = new Runnable()
+{
+@Override
+public void run()
+{
+try ( PipedOutputStream out = pipedOutputStream )
+{
+transformerFactory.newTransformer().transform( 
transformSource, result );
+}
+catch ( TransformerException | IOException e )
+{
+throw new RuntimeException( e );
+}
+}
+};
+
+new Thread( runnable ).start();
 
 Review comment:
   If you think it can be improved, go ahead. I just started with this PoC, 
would be good if others could join and improve.


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-01 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r330234983
 
 

 ##
 File path: 
maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelCacheManager.java
 ##
 @@ -0,0 +1,74 @@
+package org.apache.maven.model.building;
+
+/*
+ * 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.
+ */
+
+import java.nio.file.Path;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import org.apache.maven.model.Model;
+import org.apache.maven.xml.filter.DependencyKey;
+
+/**
+ * 
+ * @author Robert Scholte
+ * @since 3.7.0
+ */
+@Named
+@Singleton
+public class DefaultModelCacheManager implements ModelCacheManager
+{
+private static final Map MODELCACHE = 
Collections.synchronizedMap( new HashMap() );
+
+private static final Map DEPKEYMODELCACHE =
 
 Review comment:
   I've tried to find an instance that holds the cache, but IIRC it was out of 
reach of the maven-model-builder. That might be an improvement for 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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-01 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r330234343
 
 

 ##
 File path: 
maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelCacheManager.java
 ##
 @@ -0,0 +1,74 @@
+package org.apache.maven.model.building;
+
+/*
+ * 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.
+ */
+
+import java.nio.file.Path;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import org.apache.maven.model.Model;
+import org.apache.maven.xml.filter.DependencyKey;
+
+/**
+ * 
+ * @author Robert Scholte
+ * @since 3.7.0
+ */
+@Named
+@Singleton
+public class DefaultModelCacheManager implements ModelCacheManager
+{
+private static final Map MODELCACHE = 
Collections.synchronizedMap( new HashMap() );
 
 Review comment:
   I agree on the static versus singleton, but people can still create it with 
a constructor. As long as the caches lives for 1 Maven run, I don't see any 
reason for invalidation. Also, disappearing models will cause issues later on, 
otherwise you need to re-read them again, which is more resource consuming


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-01 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r330232022
 
 

 ##
 File path: 
maven-xml/src/test/java/org/apache/maven/xml/filter/ConsumerPomXMLFilterTest.java
 ##
 @@ -0,0 +1,168 @@
+package org.apache.maven.xml.filter;
+
+/*
+ * 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.
+ */
+
+import static org.xmlunit.assertj.XmlAssert.assertThat;
+
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Optional;
+import java.util.function.Function;
+
+import javax.inject.Provider;
+import javax.xml.parsers.ParserConfigurationException;
+
+import org.junit.Test;
+import org.xml.sax.SAXException;
+import org.xml.sax.XMLFilter;
+
+public class ConsumerPomXMLFilterTest extends AbstractXMLFilterTests
+{
+@Override
+protected XMLFilter getFilter() throws SAXException, 
ParserConfigurationException
+{
+final BuildPomXMLFilterFactory buildPomXMLFilterFactory = new 
BuildPomXMLFilterFactory()
+{
+@Override
+protected Optional getSha1()
+{
+return Optional.empty();
+}
+
+@Override
+protected Optional getRevision()
+{
+return Optional.empty();
+}
+
+@Override
+protected Optional getChangelist()
+{
+return Optional.of( "CL" );
+}
+
+@Override
+protected Function> 
getRelativePathMapper()
+{
+return null;
+}
+
+@Override
+protected Function 
getDependencyKeyToVersionMapper()
+{
+return null;
+}
+};
+
+Provider provider = new 
Provider()
+{
+
+@Override
+public BuildPomXMLFilterFactory get()
+{
+return buildPomXMLFilterFactory;
+}
+};
+
+XMLFilter filter = new ConsumerPomXMLFilterFactory( provider )
+{
+}.get( Paths.get( "pom.xml" ) );
+filter.setFeature( "http://xml.org/sax/features/namespaces;, true );
+return filter;
+}
+
+@Test
+public void testAllFilters() throws Exception {
+String input = "\n"
+ + "  \n"
+ + "GROUPID\n"
+ + "PARENT\n"
+ + "VERSION\n"
+ + "../pom.xml\n"
+ + "  \n"
+ + "  PROJECT\n"
+ + "  \n"
+ + "ab\n"
+ + "../cd\n"
+ + "  \n"
+ + "";
+String expected = "\n"
++ "  \n"
++ "GROUPID\n"
++ "PARENT\n"
++ "VERSION\n"
++ "\n"
++ "  \n"
++ "  PROJECT\n"
++ "";
+String actual = transform( input );
+assertThat( actual ).and( expected ).ignoreWhitespace().areIdentical();
+}
+
+@Test
+public void testMe() throws Exception {
+String input = "\r\n" + 
+"http://maven.apache.org/POM/4.0.0\; \r\n" + 
+" 
xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\"\r\n; + 
+" xsi:schemaLocation=\"http://maven.apache.org/POM/4.0.0 
\r\n" + 
+" 
http://maven.apache.org/maven-v4_0_0.xsd\;>\r\n" + 
 
 Review comment:
   A test, but it is fine to update


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-01 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r330204588
 
 

 ##
 File path: 
maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java
 ##
 @@ -733,12 +758,81 @@ private void checkPluginVersions( List 
lineage, ModelBuildingRequest
 private void assembleInheritance( List lineage, 
ModelBuildingRequest request,
   ModelProblemCollector problems )
 {
-for ( int i = lineage.size() - 2; i >= 0; i-- )
+for ( int i = lineage.size() - 2; i >= 1; i-- )
 {
 Model parent = lineage.get( i + 1 ).getModel();
 Model child = lineage.get( i ).getModel();
 inheritanceAssembler.assembleModelInheritance( child, parent, 
request, problems );
 }
+
+// re-read model from file
+if ( Boolean.getBoolean( "maven.experimental.buildconsumer" ) && 
request.isTransformPom() )
+{
+try
+{
+// TODO: parent might be part of reactor... better read all 
lineage items like this?
+Model parent = lineage.get( 1 ).getModel();
+
+Model child = modelProcessor.read( transformData( lineage.get( 
0 ) ), null );
+inheritanceAssembler.assembleModelInheritance( child, parent, 
request, problems );
+
+// sync pomfile, is transient
+child.setPomFile( lineage.get( 0 ).getModel().getPomFile() );
+// overwrite child
+lineage.get( 0 ).setModel( child );
+}
+catch ( IOException | TransformException | SAXException | 
ParserConfigurationException e )
+{
+// this is second read, should not happen
+e.printStackTrace();
+}
+}
+else
+{
+Model parent = lineage.get( 1 ).getModel();
+Model child = lineage.get( 0 ).getModel();
+inheritanceAssembler.assembleModelInheritance( child, parent, 
request, problems );
+}
+}
+
+private InputStream transformData( ModelData modelData )
+throws IOException, TransformException, SAXException, 
ParserConfigurationException
+{
+final TransformerFactory transformerFactory = 
TransformerFactory.newInstance();
+
+final PipedOutputStream pipedOutputStream  = new PipedOutputStream();
+final PipedInputStream pipedInputStream  = new PipedInputStream( 
pipedOutputStream );
+
+// Should always be FileSource for reactor poms
+FileSource source = (FileSource) modelData.getSource();
+
+System.out.println( "transforming " + source.getFile() );
 
 Review comment:
   Should be removed, but it exposed an interesting bug: 
https://issues.apache.org/jira/browse/MNG-6753


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-01 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r330204200
 
 

 ##
 File path: 
maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java
 ##
 @@ -733,12 +758,81 @@ private void checkPluginVersions( List 
lineage, ModelBuildingRequest
 private void assembleInheritance( List lineage, 
ModelBuildingRequest request,
   ModelProblemCollector problems )
 {
-for ( int i = lineage.size() - 2; i >= 0; i-- )
+for ( int i = lineage.size() - 2; i >= 1; i-- )
 
 Review comment:
   The next fragment in crucial. `lineage` contains the list of (inherited) 
parents poms + current pom, where a parent is already transformed (when it was 
a child). Based on the flag it decides if the child/current pom should be 
transformed or not.


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-01 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r330201980
 
 

 ##
 File path: 
maven-core/src/test/java/org/apache/maven/xml/filter/ConsumerPomXMLFilterTest.java
 ##
 @@ -0,0 +1,64 @@
+package org.apache.maven.xml.filter;
+
+/*
+ * 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.
+ */
+
+import static org.xmlunit.assertj.XmlAssert.assertThat;
+
+import org.junit.Before;
+import org.junit.Test;
+
+public class ConsumerPomXMLFilterTest extends AbstractXMLFilterTests
+{
+private ConsumerPomXMLFilter filter;
+
+@Before
+public void setup() throws Exception {
+filter = new ConsumerPomXMLFilter();
+}
+
+@Test
+public void testAllFilters() throws Exception {
+String input = "\n"
+ + "  \n"
+ + "GROUPID\n"
+ + "PARENT\n"
+ + "VERSION\n"
+ + "../pom.xml\n"
+ + "  \n"
+ + "  PROJECT\n"
+ + "  \n"
+ + "ab\n"
+ + "../cd\n"
+ + "  \n"
+ + "";
+String expected = "\n"
++ "  \n"
++ "GROUPID\n"
++ "PARENT\n"
++ "VERSION\n"
++ "\n"
 
 Review comment:
   This is a good point of discussion: during build it no relativePath means it 
gets the default value, i.e. `..\pom.xml`, whereas now it will be `null`.
   There might be a good reason to have 2 ModelBuilders, 1 for local project 
poms, other for remote poms, where relativePath and possible other elements 
will be ignored.


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-01 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r330198943
 
 

 ##
 File path: 
maven-core/src/main/java/org/apache/maven/xml/internal/DefaultBuildPomXMLFilterFactory.java
 ##
 @@ -0,0 +1,117 @@
+package org.apache.maven.xml.internal;
+
+/*
+ * 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.
+ */
+
+
+import java.nio.file.Path;
+import java.util.Optional;
+import java.util.function.Function;
+
+import javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import org.apache.maven.execution.MavenSession;
+import org.apache.maven.model.Model;
+import org.apache.maven.model.building.ModelCacheManager;
+import org.apache.maven.xml.filter.BuildPomXMLFilterFactory;
+import org.apache.maven.xml.filter.DependencyKey;
+import org.apache.maven.xml.filter.RelativeProject;
+
+/**
+ * 
+ * @author Robert Scholte
+ * @since 3.7.0
+ */
+@Named
+@Singleton
+public class DefaultBuildPomXMLFilterFactory extends BuildPomXMLFilterFactory
+{
+private MavenSession session;
+
+@Inject
+private ModelCacheManager rawModelCache; 
+
+@Inject
+public DefaultBuildPomXMLFilterFactory( MavenSession session )
+{
+this.session = session;
+}
+
+@Override
+protected Optional getChangelist()
+{
+return Optional.ofNullable( session.getUserProperties().getProperty( 
"changelist" ) );
+}
+
+@Override
+protected Optional getRevision()
+{
+return Optional.ofNullable( session.getUserProperties().getProperty( 
"revision" ) );
+}
+
+@Override
+protected Optional getSha1()
 
 Review comment:
   See https://maven.apache.org/maven-ci-friendly.html , right now there are 3 
special placeholders, getters reflect them


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-01 Thread GitBox
rfscholte commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r330185735
 
 

 ##
 File path: pom.xml
 ##
 @@ -47,8 +47,8 @@ under the License.
 
   
 3.0.5
-1.7
-1.7
+1.8
+1.8
 
 Review comment:
   Well, actually https://issues.apache.org/jira/browse/MNG-6399 should do 
that, but I needed it 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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services