You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2022/05/12 08:06:20 UTC
[GitHub] [maven] gnodet opened a new pull request, #736: [MNG-7476] Display a warning when an aggregator mojo is lock other mojos executions
gnodet opened a new pull request, #736:
URL: https://github.com/apache/maven/pull/736
This needs to be applied to 3.8.x, 3.9.x and master branches
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] gnodet commented on a diff in pull request #736: [3.8.x] [MNG-7476] Display a warning when an aggregator mojo is lock other mojos executions
Posted by GitBox <gi...@apache.org>.
gnodet commented on code in PR #736:
URL: https://github.com/apache/maven/pull/736#discussion_r872493501
##########
maven-core/src/main/java/org/apache/maven/lifecycle/internal/MojoExecutor.java:
##########
@@ -232,7 +237,17 @@ private static class ProjectLock implements AutoCloseable
boolean aggregator = mojoDescriptor.isAggregator();
acquiredAggregatorLock = aggregator ? aggregatorLock.writeLock() : aggregatorLock.readLock();
acquiredProjectLock = getProjectLock( session );
- acquiredAggregatorLock.lock();
+ if ( !acquiredAggregatorLock.tryLock() )
+ {
+ for ( String s : MessageHelper.formatWarning(
+ "An aggregator Mojo is already executing in parallel build, but aggregator "
+ + "Mojos require exclusive access to reactor to prevent race conditions. This "
+ + "mojo execution will be blocked until the aggregator work is done." ) )
+ {
Review Comment:
Right, I'll dig it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] slawekjaranowski commented on a diff in pull request #736: [MNG-7476] Display a warning when an aggregator mojo is lock other mojos executions
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on code in PR #736:
URL: https://github.com/apache/maven/pull/736#discussion_r871367201
##########
maven-core/src/main/java/org/apache/maven/internal/MessageHelper.java:
##########
@@ -0,0 +1,91 @@
+package org.apache.maven.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.util.ArrayList;
+import java.util.List;
+
+/**
+ * Helper class to format warning messages to the console
+ */
+public class MessageHelper
+{
+
+ private static final int DEFAULT_MAX_SIZE = 65;
+ private static final char BOX_CHAR = '*';
+
+ public static String separatorLine()
+ {
+ StringBuilder sb = new StringBuilder( DEFAULT_MAX_SIZE );
+ repeat( sb, '*', DEFAULT_MAX_SIZE );
+ return sb.toString();
+ }
+
+ public static List<String> formatWarning( String... lines )
Review Comment:
proposition:
```
void formatLog( Consumer<String>logger, String... lines )
```
##########
maven-core/src/main/java/org/apache/maven/internal/MessageHelper.java:
##########
@@ -0,0 +1,91 @@
+package org.apache.maven.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.util.ArrayList;
+import java.util.List;
+
+/**
+ * Helper class to format warning messages to the console
+ */
+public class MessageHelper
+{
+
+ private static final int DEFAULT_MAX_SIZE = 65;
+ private static final char BOX_CHAR = '*';
+
+ public static String separatorLine()
+ {
+ StringBuilder sb = new StringBuilder( DEFAULT_MAX_SIZE );
+ repeat( sb, '*', DEFAULT_MAX_SIZE );
+ return sb.toString();
+ }
+
+ public static List<String> formatWarning( String... lines )
+ {
+ int size = DEFAULT_MAX_SIZE;
+ int rem = size - 4;
+ List<String> result = new ArrayList<>();
+ StringBuilder sb = new StringBuilder( size );
+ // first line
+ sb.setLength( 0 );
+ repeat( sb, BOX_CHAR, size );
+ result.add( sb.toString() );
Review Comment:
and than - instead add to result
```
logger.accept( sb.toString() );
```
##########
maven-core/src/main/java/org/apache/maven/lifecycle/internal/MojoExecutor.java:
##########
@@ -232,7 +237,17 @@ private static class ProjectLock implements AutoCloseable
boolean aggregator = mojoDescriptor.isAggregator();
acquiredAggregatorLock = aggregator ? aggregatorLock.writeLock() : aggregatorLock.readLock();
acquiredProjectLock = getProjectLock( session );
- acquiredAggregatorLock.lock();
+ if ( !acquiredAggregatorLock.tryLock() )
+ {
+ for ( String s : MessageHelper.formatWarning(
+ "An aggregator Mojo is already executing in parallel build, but aggregator "
+ + "Mojos require exclusive access to reactor to prevent race conditions. This "
+ + "mojo execution will be blocked until the aggregator work is done." ) )
+ {
+ LOGGER.warn( s );
+ }
Review Comment:
this can be replaced by:
```
MessageHelper. formatLog( LOGGER::warn, "line1", "line2" .... );
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on pull request #736: [3.8.x] [MNG-7476] Display a warning when an aggregator mojo locks other mojos executions
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #736:
URL: https://github.com/apache/maven/pull/736#issuecomment-1140493570
Polished commits, they are now subsequently, standalone testable (tested actually). As with previous releases I have created a JIRA issue which address the problem which in turn solves the regression. Immediately visible in the changelog. This PR works decently with the fixed IT PR. Only one open question needs to be answered by @gnodet. Will align IT as well. Thank your for the effort.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] gnodet commented on a diff in pull request #736: [3.8.x] [MNG-7476] Display a warning when an aggregator mojo is lock other mojos executions
Posted by GitBox <gi...@apache.org>.
gnodet commented on code in PR #736:
URL: https://github.com/apache/maven/pull/736#discussion_r872496150
##########
maven-core/src/main/java/org/apache/maven/internal/MessageHelper.java:
##########
@@ -0,0 +1,91 @@
+package org.apache.maven.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.util.ArrayList;
+import java.util.List;
+
+/**
+ * Helper class to format warning messages to the console
+ */
+public class MessageHelper
+{
+
+ private static final int DEFAULT_MAX_SIZE = 65;
+ private static final char BOX_CHAR = '*';
+
+ public static String separatorLine()
+ {
+ StringBuilder sb = new StringBuilder( DEFAULT_MAX_SIZE );
+ repeat( sb, '*', DEFAULT_MAX_SIZE );
+ return sb.toString();
+ }
+
+ public static List<String> formatWarning( String... lines )
Review Comment:
That's not a package that is used in maven-core. Do we really need to add a dependency on it ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on pull request #736: [3.8.x] [MNG-7476] Display a warning when an aggregator mojo locks other mojos executions
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #736:
URL: https://github.com/apache/maven/pull/736#issuecomment-1140466025
> > > > @gnodet Can the reproducer be turned into an IT?
> > >
> > >
> > > I suppose. The failure would lead to a deadlock. is there a timeout that can be set in the invoker ?
> >
> >
> > @slawekjaranowski Do you know this from top of your head?
>
> There is no implementation for timeouts in Verifier.
I have checked the source code of verifier. The lower level library supports timeouts in seconds, so we just need to wrap this with verifier. The JVM must be forked for this, so if timeout is > 0 and forkMode is auto, then autoforking must happen.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on a diff in pull request #736: [3.8.x] [MNG-7476] Display a warning when an aggregator mojo is lock other mojos executions
Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #736:
URL: https://github.com/apache/maven/pull/736#discussion_r872464396
##########
maven-core/src/main/java/org/apache/maven/internal/MessageHelper.java:
##########
@@ -0,0 +1,91 @@
+package org.apache.maven.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.util.ArrayList;
+import java.util.List;
+
+/**
+ * Helper class to format warning messages to the console
+ */
+public class MessageHelper
+{
+
+ private static final int DEFAULT_MAX_SIZE = 65;
+ private static final char BOX_CHAR = '*';
+
+ public static String separatorLine()
+ {
+ StringBuilder sb = new StringBuilder( DEFAULT_MAX_SIZE );
+ repeat( sb, '*', DEFAULT_MAX_SIZE );
+ return sb.toString();
+ }
+
+ public static List<String> formatWarning( String... lines )
+ {
+ int size = DEFAULT_MAX_SIZE;
+ int rem = size - 4;
Review Comment:
Shouldn't the 4 be a static final? I honestly don't know where this 4 comes from the top of my head.
##########
maven-core/src/main/java/org/apache/maven/internal/MessageHelper.java:
##########
@@ -0,0 +1,91 @@
+package org.apache.maven.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.util.ArrayList;
+import java.util.List;
+
+/**
+ * Helper class to format warning messages to the console
+ */
+public class MessageHelper
+{
+
+ private static final int DEFAULT_MAX_SIZE = 65;
+ private static final char BOX_CHAR = '*';
+
+ public static String separatorLine()
+ {
+ StringBuilder sb = new StringBuilder( DEFAULT_MAX_SIZE );
+ repeat( sb, '*', DEFAULT_MAX_SIZE );
+ return sb.toString();
+ }
+
+ public static List<String> formatWarning( String... lines )
Review Comment:
Isn't this https://commons.apache.org/proper/commons-text/javadocs/api-release/org/apache/commons/text/WordUtils.html#wrap(java.lang.String,int)?
##########
maven-core/src/main/java/org/apache/maven/lifecycle/internal/MojoExecutor.java:
##########
@@ -232,7 +237,17 @@ private static class ProjectLock implements AutoCloseable
boolean aggregator = mojoDescriptor.isAggregator();
acquiredAggregatorLock = aggregator ? aggregatorLock.writeLock() : aggregatorLock.readLock();
acquiredProjectLock = getProjectLock( session );
- acquiredAggregatorLock.lock();
+ if ( !acquiredAggregatorLock.tryLock() )
+ {
+ for ( String s : MessageHelper.formatWarning(
+ "An aggregator Mojo is already executing in parallel build, but aggregator "
+ + "Mojos require exclusive access to reactor to prevent race conditions. This "
+ + "mojo execution will be blocked until the aggregator work is done." ) )
+ {
Review Comment:
I have commented on this already somewhere else. Can you find that up? I can't :-(
##########
maven-core/src/main/java/org/apache/maven/internal/MessageHelper.java:
##########
@@ -0,0 +1,91 @@
+package org.apache.maven.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.util.ArrayList;
+import java.util.List;
+
+/**
+ * Helper class to format warning messages to the console
+ */
+public class MessageHelper
+{
+
+ private static final int DEFAULT_MAX_SIZE = 65;
+ private static final char BOX_CHAR = '*';
+
+ public static String separatorLine()
+ {
+ StringBuilder sb = new StringBuilder( DEFAULT_MAX_SIZE );
+ repeat( sb, '*', DEFAULT_MAX_SIZE );
+ return sb.toString();
+ }
+
+ public static List<String> formatWarning( String... lines )
+ {
+ int size = DEFAULT_MAX_SIZE;
+ int rem = size - 4;
+ List<String> result = new ArrayList<>();
+ StringBuilder sb = new StringBuilder( size );
+ // first line
+ sb.setLength( 0 );
+ repeat( sb, BOX_CHAR, size );
+ result.add( sb.toString() );
+ // lines
+ for ( String line : lines )
+ {
+ sb.setLength( 0 );
+ String[] words = line.split( " " );
+ for ( String word : words )
+ {
+ if ( sb.length() >= rem - word.length() - ( sb.length() > 0 ? 1 : 0 ) )
+ {
+ repeat( sb, ' ', rem - sb.length() );
+ result.add( BOX_CHAR + " " + sb + " " + BOX_CHAR );
+ sb.setLength( 0 );
+ }
+ if ( sb.length() > 0 )
+ {
+ sb.append( ' ' );
+ }
+ sb.append( word );
+ }
+
+ while ( sb.length() < rem )
+ {
+ sb.append( ' ' );
+ }
+ result.add( BOX_CHAR + " " + sb + " " + BOX_CHAR );
+ }
+ // last line
+ sb.setLength( 0 );
+ repeat( sb, BOX_CHAR, size );
+ result.add( sb.toString() );
+ return result;
+ }
+
+ private static void repeat( StringBuilder sb, char c, int nb )
+ {
+ for ( int i = 0; i < nb; i++ )
+ {
+ sb.append( c );
+ }
Review Comment:
Same as https://commons.apache.org/proper/commons-lang/apidocs/org/apache/commons/lang3/StringUtils.html#repeat-char-int-?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on pull request #736: [3.8.x] [MNG-7476] Display a warning when an aggregator mojo is lock other mojos executions
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #736:
URL: https://github.com/apache/maven/pull/736#issuecomment-1128046352
> > @gnodet Can the reproducer be turned into an IT?
>
> I suppose. The failure would lead to a deadlock. is there a timeout that can be set in the invoker ?
@slawekjaranowski Do you know this from top of your head?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on a diff in pull request #736: [3.8.x] [MNG-7476] Display a warning when an aggregator mojo is lock other mojos executions
Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #736:
URL: https://github.com/apache/maven/pull/736#discussion_r874051177
##########
maven-core/src/main/java/org/apache/maven/lifecycle/internal/MojoExecutor.java:
##########
@@ -282,6 +308,32 @@ private Lock getProjectLock( MavenSession session )
}
}
+ static class OwnerReentrantLock extends ReentrantLock
+ {
+ @Override
+ public Thread getOwner()
+ {
+ return super.getOwner();
+ }
+ }
+
+ static class OwnerReentrantReadWriteLock extends ReentrantReadWriteLock
+ {
+ @Override
+ public Thread getOwner()
+ {
+ return super.getOwner();
+ }
+ }
+
+ private static void warn( String msg )
+ {
+ for ( String s : MessageHelper.formatWarning( msg ) )
+ {
+ LOGGER.warn( s );
+ }
Review Comment:
Well, look into /maven-embedder/src/main/java/org/apache/maven/cli/transfer/ConsoleMavenTransferListener.java, I do synchronize there to make console output project. Since the logger is static final you can use it as monitor, no?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] gnodet commented on pull request #736: [3.8.x] [MNG-7476] Display a warning when an aggregator mojo locks other mojos executions
Posted by GitBox <gi...@apache.org>.
gnodet commented on PR #736:
URL: https://github.com/apache/maven/pull/736#issuecomment-1140912782
> > @michael-o I've slightly refactored the `MojoExecutor` to cleanup the access to the `mojos` map.
>
> This looks good and makes sense. Totally scoped in the `ProjectLock`. As far as I can see this commit shall be merged into "[[MNG-7487](https://issues.apache.org/jira/browse/MNG-7487)]" and "[[MNG-7476](https://issues.apache.org/jira/browse/MNG-7476)]" rebased on top of that.
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] gnodet commented on a diff in pull request #736: [3.8.x] [MNG-7476] Display a warning when an aggregator mojo is lock other mojos executions
Posted by GitBox <gi...@apache.org>.
gnodet commented on code in PR #736:
URL: https://github.com/apache/maven/pull/736#discussion_r873515895
##########
maven-core/src/main/java/org/apache/maven/internal/MessageHelper.java:
##########
@@ -0,0 +1,91 @@
+package org.apache.maven.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.util.ArrayList;
+import java.util.List;
+
+/**
+ * Helper class to format warning messages to the console
+ */
+public class MessageHelper
+{
+
+ private static final int DEFAULT_MAX_SIZE = 65;
+ private static final char BOX_CHAR = '*';
+
+ public static String separatorLine()
+ {
+ StringBuilder sb = new StringBuilder( DEFAULT_MAX_SIZE );
+ repeat( sb, '*', DEFAULT_MAX_SIZE );
+ return sb.toString();
+ }
+
+ public static List<String> formatWarning( String... lines )
+ {
+ int size = DEFAULT_MAX_SIZE;
+ int rem = size - 4;
Review Comment:
I've added a comment, I don't think it warrants a constant as it can not be changed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] gnodet commented on pull request #736: [3.8.x] [MNG-7476] Display a warning when an aggregator mojo is lock other mojos executions
Posted by GitBox <gi...@apache.org>.
gnodet commented on PR #736:
URL: https://github.com/apache/maven/pull/736#issuecomment-1127443478
Fwiw, I've reviewed the original MNG-7433 issue and especially the reproducer and found out the issue was just a bug in the original project locking and not simply a missing warning. The lock was acquired too early, so that forked lifecycles could not obtain the lock for the project at all.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] gnodet commented on a diff in pull request #736: [3.8.x] [MNG-7476] Display a warning when an aggregator mojo is lock other mojos executions
Posted by GitBox <gi...@apache.org>.
gnodet commented on code in PR #736:
URL: https://github.com/apache/maven/pull/736#discussion_r872494330
##########
maven-core/src/main/java/org/apache/maven/internal/MessageHelper.java:
##########
@@ -0,0 +1,91 @@
+package org.apache.maven.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.util.ArrayList;
+import java.util.List;
+
+/**
+ * Helper class to format warning messages to the console
+ */
+public class MessageHelper
+{
+
+ private static final int DEFAULT_MAX_SIZE = 65;
+ private static final char BOX_CHAR = '*';
+
+ public static String separatorLine()
+ {
+ StringBuilder sb = new StringBuilder( DEFAULT_MAX_SIZE );
+ repeat( sb, '*', DEFAULT_MAX_SIZE );
+ return sb.toString();
+ }
+
+ public static List<String> formatWarning( String... lines )
+ {
+ int size = DEFAULT_MAX_SIZE;
+ int rem = size - 4;
+ List<String> result = new ArrayList<>();
+ StringBuilder sb = new StringBuilder( size );
+ // first line
+ sb.setLength( 0 );
+ repeat( sb, BOX_CHAR, size );
+ result.add( sb.toString() );
+ // lines
+ for ( String line : lines )
+ {
+ sb.setLength( 0 );
+ String[] words = line.split( " " );
+ for ( String word : words )
+ {
+ if ( sb.length() >= rem - word.length() - ( sb.length() > 0 ? 1 : 0 ) )
+ {
+ repeat( sb, ' ', rem - sb.length() );
+ result.add( BOX_CHAR + " " + sb + " " + BOX_CHAR );
+ sb.setLength( 0 );
+ }
+ if ( sb.length() > 0 )
+ {
+ sb.append( ' ' );
+ }
+ sb.append( word );
+ }
+
+ while ( sb.length() < rem )
+ {
+ sb.append( ' ' );
+ }
+ result.add( BOX_CHAR + " " + sb + " " + BOX_CHAR );
+ }
+ // last line
+ sb.setLength( 0 );
+ repeat( sb, BOX_CHAR, size );
+ result.add( sb.toString() );
+ return result;
+ }
+
+ private static void repeat( StringBuilder sb, char c, int nb )
+ {
+ for ( int i = 0; i < nb; i++ )
+ {
+ sb.append( c );
+ }
Review Comment:
I dislike dependencies on apache commons. External utility libraries are painful if they ever break something. I tend to prefer internal utility classes. I'll see if we have something somewhere else.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on a diff in pull request #736: [3.8.x] [MNG-7476] Display a warning when an aggregator mojo is lock other mojos executions
Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #736:
URL: https://github.com/apache/maven/pull/736#discussion_r872588379
##########
maven-core/src/main/java/org/apache/maven/lifecycle/internal/MojoExecutor.java:
##########
@@ -232,7 +237,17 @@ private static class ProjectLock implements AutoCloseable
boolean aggregator = mojoDescriptor.isAggregator();
acquiredAggregatorLock = aggregator ? aggregatorLock.writeLock() : aggregatorLock.readLock();
acquiredProjectLock = getProjectLock( session );
- acquiredAggregatorLock.lock();
+ if ( !acquiredAggregatorLock.tryLock() )
+ {
+ for ( String s : MessageHelper.formatWarning(
+ "An aggregator Mojo is already executing in parallel build, but aggregator "
+ + "Mojos require exclusive access to reactor to prevent race conditions. This "
+ + "mojo execution will be blocked until the aggregator work is done." ) )
+ {
Review Comment:
Yes, context info would be good.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] gnodet commented on a diff in pull request #736: [3.8.x] [MNG-7476] Display a warning when an aggregator mojo is lock other mojos executions
Posted by GitBox <gi...@apache.org>.
gnodet commented on code in PR #736:
URL: https://github.com/apache/maven/pull/736#discussion_r872525468
##########
maven-core/src/main/java/org/apache/maven/internal/MessageHelper.java:
##########
@@ -0,0 +1,91 @@
+package org.apache.maven.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.util.ArrayList;
+import java.util.List;
+
+/**
+ * Helper class to format warning messages to the console
+ */
+public class MessageHelper
+{
+
+ private static final int DEFAULT_MAX_SIZE = 65;
+ private static final char BOX_CHAR = '*';
+
+ public static String separatorLine()
+ {
+ StringBuilder sb = new StringBuilder( DEFAULT_MAX_SIZE );
+ repeat( sb, '*', DEFAULT_MAX_SIZE );
+ return sb.toString();
+ }
+
+ public static List<String> formatWarning( String... lines )
+ {
+ int size = DEFAULT_MAX_SIZE;
+ int rem = size - 4;
+ List<String> result = new ArrayList<>();
+ StringBuilder sb = new StringBuilder( size );
+ // first line
+ sb.setLength( 0 );
+ repeat( sb, BOX_CHAR, size );
+ result.add( sb.toString() );
+ // lines
+ for ( String line : lines )
+ {
+ sb.setLength( 0 );
+ String[] words = line.split( " " );
+ for ( String word : words )
+ {
+ if ( sb.length() >= rem - word.length() - ( sb.length() > 0 ? 1 : 0 ) )
+ {
+ repeat( sb, ' ', rem - sb.length() );
+ result.add( BOX_CHAR + " " + sb + " " + BOX_CHAR );
+ sb.setLength( 0 );
+ }
+ if ( sb.length() > 0 )
+ {
+ sb.append( ' ' );
+ }
+ sb.append( word );
+ }
+
+ while ( sb.length() < rem )
+ {
+ sb.append( ' ' );
+ }
+ result.add( BOX_CHAR + " " + sb + " " + BOX_CHAR );
+ }
+ // last line
+ sb.setLength( 0 );
+ repeat( sb, BOX_CHAR, size );
+ result.add( sb.toString() );
+ return result;
+ }
+
+ private static void repeat( StringBuilder sb, char c, int nb )
+ {
+ for ( int i = 0; i < nb; i++ )
+ {
+ sb.append( c );
+ }
Review Comment:
And fwiw, the commons-lang3 one would suffer from the same problem. It's a bit inefficient to create a `String` (using a StringBuilder) that will just be appended to the existing `StringBuilder`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] gnodet commented on a diff in pull request #736: [3.8.x] [MNG-7476] Display a warning when an aggregator mojo is lock other mojos executions
Posted by GitBox <gi...@apache.org>.
gnodet commented on code in PR #736:
URL: https://github.com/apache/maven/pull/736#discussion_r872523457
##########
maven-core/src/main/java/org/apache/maven/internal/MessageHelper.java:
##########
@@ -0,0 +1,91 @@
+package org.apache.maven.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.util.ArrayList;
+import java.util.List;
+
+/**
+ * Helper class to format warning messages to the console
+ */
+public class MessageHelper
+{
+
+ private static final int DEFAULT_MAX_SIZE = 65;
+ private static final char BOX_CHAR = '*';
+
+ public static String separatorLine()
+ {
+ StringBuilder sb = new StringBuilder( DEFAULT_MAX_SIZE );
+ repeat( sb, '*', DEFAULT_MAX_SIZE );
+ return sb.toString();
+ }
+
+ public static List<String> formatWarning( String... lines )
+ {
+ int size = DEFAULT_MAX_SIZE;
+ int rem = size - 4;
+ List<String> result = new ArrayList<>();
+ StringBuilder sb = new StringBuilder( size );
+ // first line
+ sb.setLength( 0 );
+ repeat( sb, BOX_CHAR, size );
+ result.add( sb.toString() );
+ // lines
+ for ( String line : lines )
+ {
+ sb.setLength( 0 );
+ String[] words = line.split( " " );
+ for ( String word : words )
+ {
+ if ( sb.length() >= rem - word.length() - ( sb.length() > 0 ? 1 : 0 ) )
+ {
+ repeat( sb, ' ', rem - sb.length() );
+ result.add( BOX_CHAR + " " + sb + " " + BOX_CHAR );
+ sb.setLength( 0 );
+ }
+ if ( sb.length() > 0 )
+ {
+ sb.append( ' ' );
+ }
+ sb.append( word );
+ }
+
+ while ( sb.length() < rem )
+ {
+ sb.append( ' ' );
+ }
+ result.add( BOX_CHAR + " " + sb + " " + BOX_CHAR );
+ }
+ // last line
+ sb.setLength( 0 );
+ repeat( sb, BOX_CHAR, size );
+ result.add( sb.toString() );
+ return result;
+ }
+
+ private static void repeat( StringBuilder sb, char c, int nb )
+ {
+ for ( int i = 0; i < nb; i++ )
+ {
+ sb.append( c );
+ }
Review Comment:
That won't work, we'll need to `StringBuilder` here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on a diff in pull request #736: [MNG-7476] Display a warning when an aggregator mojo is lock other mojos executions
Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #736:
URL: https://github.com/apache/maven/pull/736#discussion_r871389367
##########
maven-core/src/main/java/org/apache/maven/internal/MessageHelper.java:
##########
@@ -0,0 +1,91 @@
+package org.apache.maven.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.util.ArrayList;
+import java.util.List;
+
+/**
+ * Helper class to format warning messages to the console
+ */
+public class MessageHelper
+{
+
+ private static final int DEFAULT_MAX_SIZE = 65;
+ private static final char BOX_CHAR = '*';
+
+ public static String separatorLine()
+ {
+ StringBuilder sb = new StringBuilder( DEFAULT_MAX_SIZE );
+ repeat( sb, '*', DEFAULT_MAX_SIZE );
+ return sb.toString();
+ }
+
+ public static List<String> formatWarning( String... lines )
Review Comment:
No possible, we need this in 3.8.x, Java 7
##########
maven-core/src/main/java/org/apache/maven/internal/MessageHelper.java:
##########
@@ -0,0 +1,91 @@
+package org.apache.maven.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.util.ArrayList;
+import java.util.List;
+
+/**
+ * Helper class to format warning messages to the console
+ */
+public class MessageHelper
+{
+
+ private static final int DEFAULT_MAX_SIZE = 65;
+ private static final char BOX_CHAR = '*';
+
+ public static String separatorLine()
+ {
+ StringBuilder sb = new StringBuilder( DEFAULT_MAX_SIZE );
+ repeat( sb, '*', DEFAULT_MAX_SIZE );
+ return sb.toString();
+ }
+
+ public static List<String> formatWarning( String... lines )
Review Comment:
Not possible, we need this in 3.8.x, Java 7
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] gnodet merged pull request #736: [3.8.x] [MNG-7476] Display a warning when an aggregator mojo locks other mojos executions
Posted by GitBox <gi...@apache.org>.
gnodet merged PR #736:
URL: https://github.com/apache/maven/pull/736
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on a diff in pull request #736: [3.8.x] [MNG-7476] Display a warning when an aggregator mojo is lock other mojos executions
Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #736:
URL: https://github.com/apache/maven/pull/736#discussion_r873995380
##########
maven-core/src/main/java/org/apache/maven/lifecycle/internal/MojoExecutor.java:
##########
@@ -220,20 +223,43 @@ private void execute( MavenSession session, MojoExecution mojoExecution, Project
* TODO: ideally, the builder should take care of the ordering in a smarter way
* TODO: and concurrency issues fixed with MNG-7157
*/
- private static class ProjectLock implements AutoCloseable
+ private class ProjectLock implements AutoCloseable
{
final Lock acquiredAggregatorLock;
- final Lock acquiredProjectLock;
+ final OwnerReentrantLock acquiredProjectLock;
- ProjectLock( MavenSession session, MojoDescriptor mojoDescriptor, ReadWriteLock aggregatorLock )
+ ProjectLock( MavenSession session, MojoDescriptor mojoDescriptor )
{
+ mojos.put( Thread.currentThread(), mojoDescriptor );
if ( session.getRequest().getDegreeOfConcurrency() > 1 )
{
- boolean aggregator = mojoDescriptor.isAggregator();
- acquiredAggregatorLock = aggregator ? aggregatorLock.writeLock() : aggregatorLock.readLock();
+ acquiredAggregatorLock = mojoDescriptor.isAggregator()
+ ? aggregatorLock.writeLock() : aggregatorLock.readLock();
+ if ( !acquiredAggregatorLock.tryLock() )
+ {
+ Thread owner = aggregatorLock.getOwner();
+ MojoDescriptor ownerMojo = owner != null ? mojos.get( owner ) : null;
+ String str = ownerMojo != null ? " The " + ownerMojo.getId() : "An ";
+ String msg = str + " aggregator mojo is already being executed "
+ + "in this parallel build, those kind of mojos require exclusive access to "
+ + "reactor to prevent race conditions. This mojo execution will be blocked "
+ + "until the aggregator mojo is done.";
+ warn( msg );
+ acquiredAggregatorLock.lock();
+ }
acquiredProjectLock = getProjectLock( session );
- acquiredAggregatorLock.lock();
- acquiredProjectLock.lock();
+ if ( !acquiredProjectLock.tryLock() )
+ {
+ Thread owner = acquiredProjectLock.getOwner();
+ MojoDescriptor ownerMojo = owner != null ? mojos.get( owner ) : null;
+ String str = ownerMojo != null ? " The " + ownerMojo.getId() : "An ";
+ String msg = str + " mojo is already being executed "
+ + "on the project " + session.getCurrentProject().toString() + ". "
Review Comment:
toString() is redundant. It will be called by Java automatically.
##########
maven-core/src/main/java/org/apache/maven/lifecycle/internal/MojoExecutor.java:
##########
@@ -282,6 +308,32 @@ private Lock getProjectLock( MavenSession session )
}
}
+ static class OwnerReentrantLock extends ReentrantLock
+ {
+ @Override
+ public Thread getOwner()
+ {
+ return super.getOwner();
+ }
+ }
+
+ static class OwnerReentrantReadWriteLock extends ReentrantReadWriteLock
+ {
+ @Override
+ public Thread getOwner()
+ {
+ return super.getOwner();
+ }
+ }
+
+ private static void warn( String msg )
+ {
+ for ( String s : MessageHelper.formatWarning( msg ) )
+ {
+ LOGGER.warn( s );
+ }
Review Comment:
In a concurrent build may it happen that interleaving threads might break this message as a whole?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on a diff in pull request #736: [3.8.x] [MNG-7476] Display a warning when an aggregator mojo locks other mojos executions
Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #736:
URL: https://github.com/apache/maven/pull/736#discussion_r884276548
##########
maven-core/src/main/java/org/apache/maven/lifecycle/internal/MojoExecutor.java:
##########
@@ -220,20 +223,44 @@ private void execute( MavenSession session, MojoExecution mojoExecution, Project
* TODO: ideally, the builder should take care of the ordering in a smarter way
* TODO: and concurrency issues fixed with MNG-7157
*/
- private static class ProjectLock implements AutoCloseable
+ private class ProjectLock implements AutoCloseable
{
final Lock acquiredAggregatorLock;
- final Lock acquiredProjectLock;
+ final OwnerReentrantLock acquiredProjectLock;
- ProjectLock( MavenSession session, MojoDescriptor mojoDescriptor, ReadWriteLock aggregatorLock )
+ ProjectLock( MavenSession session, MojoDescriptor mojoDescriptor )
{
+ mojos.put( Thread.currentThread(), mojoDescriptor );
Review Comment:
this duplicates https://github.com/apache/maven/pull/736/files#diff-afa12cb60ed6ee8f6ad777fc0a0b4db1726b94306d7bda74f930ee95a27a9195R350. Intended?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on pull request #736: [3.8.x] [MNG-7476] Display a warning when an aggregator mojo locks other mojos executions
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #736:
URL: https://github.com/apache/maven/pull/736#issuecomment-1140458035
Moved message helper to https://github.com/apache/maven/pull/746.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on a diff in pull request #736: [3.8.x] [MNG-7476] Display a warning when an aggregator mojo locks other mojos executions
Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #736:
URL: https://github.com/apache/maven/pull/736#discussion_r884276548
##########
maven-core/src/main/java/org/apache/maven/lifecycle/internal/MojoExecutor.java:
##########
@@ -220,20 +223,44 @@ private void execute( MavenSession session, MojoExecution mojoExecution, Project
* TODO: ideally, the builder should take care of the ordering in a smarter way
* TODO: and concurrency issues fixed with MNG-7157
*/
- private static class ProjectLock implements AutoCloseable
+ private class ProjectLock implements AutoCloseable
{
final Lock acquiredAggregatorLock;
- final Lock acquiredProjectLock;
+ final OwnerReentrantLock acquiredProjectLock;
- ProjectLock( MavenSession session, MojoDescriptor mojoDescriptor, ReadWriteLock aggregatorLock )
+ ProjectLock( MavenSession session, MojoDescriptor mojoDescriptor )
{
+ mojos.put( Thread.currentThread(), mojoDescriptor );
Review Comment:
this duplicates https://github.com/apache/maven/pull/736/files#diff-afa12cb60ed6ee8f6ad777fc0a0b4db1726b94306d7bda74f930ee95a27a9195R350. Intended? I assume since the finally block removes, this must go and the caller's put will remain.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] gnodet commented on pull request #736: [3.8.x] [MNG-7476] Display a warning when an aggregator mojo locks other mojos executions
Posted by GitBox <gi...@apache.org>.
gnodet commented on PR #736:
URL: https://github.com/apache/maven/pull/736#issuecomment-1140833840
@michael-o I've slightly refactored the `MojoExecutor` to cleanup the access to the `mojos` map.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] gnodet commented on a diff in pull request #736: [3.8.x] [MNG-7476] Display a warning when an aggregator mojo is lock other mojos executions
Posted by GitBox <gi...@apache.org>.
gnodet commented on code in PR #736:
URL: https://github.com/apache/maven/pull/736#discussion_r872518135
##########
maven-core/src/main/java/org/apache/maven/internal/MessageHelper.java:
##########
@@ -0,0 +1,91 @@
+package org.apache.maven.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.util.ArrayList;
+import java.util.List;
+
+/**
+ * Helper class to format warning messages to the console
+ */
+public class MessageHelper
+{
+
+ private static final int DEFAULT_MAX_SIZE = 65;
+ private static final char BOX_CHAR = '*';
+
+ public static String separatorLine()
+ {
+ StringBuilder sb = new StringBuilder( DEFAULT_MAX_SIZE );
+ repeat( sb, '*', DEFAULT_MAX_SIZE );
+ return sb.toString();
+ }
+
+ public static List<String> formatWarning( String... lines )
+ {
+ int size = DEFAULT_MAX_SIZE;
+ int rem = size - 4;
+ List<String> result = new ArrayList<>();
+ StringBuilder sb = new StringBuilder( size );
+ // first line
+ sb.setLength( 0 );
+ repeat( sb, BOX_CHAR, size );
+ result.add( sb.toString() );
+ // lines
+ for ( String line : lines )
+ {
+ sb.setLength( 0 );
+ String[] words = line.split( " " );
+ for ( String word : words )
+ {
+ if ( sb.length() >= rem - word.length() - ( sb.length() > 0 ? 1 : 0 ) )
+ {
+ repeat( sb, ' ', rem - sb.length() );
+ result.add( BOX_CHAR + " " + sb + " " + BOX_CHAR );
+ sb.setLength( 0 );
+ }
+ if ( sb.length() > 0 )
+ {
+ sb.append( ' ' );
+ }
+ sb.append( word );
+ }
+
+ while ( sb.length() < rem )
+ {
+ sb.append( ' ' );
+ }
+ result.add( BOX_CHAR + " " + sb + " " + BOX_CHAR );
+ }
+ // last line
+ sb.setLength( 0 );
+ repeat( sb, BOX_CHAR, size );
+ result.add( sb.toString() );
+ return result;
+ }
+
+ private static void repeat( StringBuilder sb, char c, int nb )
+ {
+ for ( int i = 0; i < nb; i++ )
+ {
+ sb.append( c );
+ }
Review Comment:
We do use `org.codehaus.plexus.util.StringUtils` a lot in maven-core, so I could reuse the `String repeat(String, int)` method from that class, not as efficient than a `char` based one, but I doubt it really matters here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] gnodet commented on pull request #736: [3.8.x] [MNG-7476] Display a warning when an aggregator mojo is lock other mojos executions
Posted by GitBox <gi...@apache.org>.
gnodet commented on PR #736:
URL: https://github.com/apache/maven/pull/736#issuecomment-1128791257
> @gnodet Can the reproducer be turned into an IT?
@michael-o https://github.com/apache/maven-integration-testing/pull/162
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] gnodet commented on a diff in pull request #736: [3.8.x] [MNG-7476] Display a warning when an aggregator mojo is lock other mojos executions
Posted by GitBox <gi...@apache.org>.
gnodet commented on code in PR #736:
URL: https://github.com/apache/maven/pull/736#discussion_r874048159
##########
maven-core/src/main/java/org/apache/maven/lifecycle/internal/MojoExecutor.java:
##########
@@ -282,6 +308,32 @@ private Lock getProjectLock( MavenSession session )
}
}
+ static class OwnerReentrantLock extends ReentrantLock
+ {
+ @Override
+ public Thread getOwner()
+ {
+ return super.getOwner();
+ }
+ }
+
+ static class OwnerReentrantReadWriteLock extends ReentrantReadWriteLock
+ {
+ @Override
+ public Thread getOwner()
+ {
+ return super.getOwner();
+ }
+ }
+
+ private static void warn( String msg )
+ {
+ for ( String s : MessageHelper.formatWarning( msg ) )
+ {
+ LOGGER.warn( s );
+ }
Review Comment:
Yes, but I don't think slf4j provide any api to _lock_ the appender. In addition, any build-in new line will break the formatting box. But in a concurrent build, you definitely want to use _mvnd_ which brings back things in order :-)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [3.8.x] [MNG-7476] Display a warning when an aggregator mojo locks other mojos executions [maven]
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on PR #736:
URL: https://github.com/apache/maven/pull/736#issuecomment-2852353956
I found commits, jira updated
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] gnodet commented on a diff in pull request #736: [3.8.x] [MNG-7476] Display a warning when an aggregator mojo locks other mojos executions
Posted by GitBox <gi...@apache.org>.
gnodet commented on code in PR #736:
URL: https://github.com/apache/maven/pull/736#discussion_r884527568
##########
maven-core/src/main/java/org/apache/maven/lifecycle/internal/MojoExecutor.java:
##########
@@ -220,20 +223,44 @@ private void execute( MavenSession session, MojoExecution mojoExecution, Project
* TODO: ideally, the builder should take care of the ordering in a smarter way
* TODO: and concurrency issues fixed with MNG-7157
*/
- private static class ProjectLock implements AutoCloseable
+ private class ProjectLock implements AutoCloseable
{
final Lock acquiredAggregatorLock;
- final Lock acquiredProjectLock;
+ final OwnerReentrantLock acquiredProjectLock;
- ProjectLock( MavenSession session, MojoDescriptor mojoDescriptor, ReadWriteLock aggregatorLock )
+ ProjectLock( MavenSession session, MojoDescriptor mojoDescriptor )
{
+ mojos.put( Thread.currentThread(), mojoDescriptor );
Review Comment:
You're right. I'll remove the access to the `mojos` map in the caller method and will add the corresponding removal from the map in the `close` method of the `ProjectLock`. This way, only the `ProjectLock` class will actually access the map.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] slawekjaranowski commented on pull request #736: [3.8.x] [MNG-7476] Display a warning when an aggregator mojo locks other mojos executions
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on PR #736:
URL: https://github.com/apache/maven/pull/736#issuecomment-1135634739
> > > @gnodet Can the reproducer be turned into an IT?
> >
> >
> > I suppose. The failure would lead to a deadlock. is there a timeout that can be set in the invoker ?
>
> @slawekjaranowski Do you know this from top of your head?
There is no implementation for timeouts in Verifier.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] slawekjaranowski commented on a diff in pull request #736: [MNG-7476] Display a warning when an aggregator mojo is lock other mojos executions
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on code in PR #736:
URL: https://github.com/apache/maven/pull/736#discussion_r871399532
##########
maven-core/src/main/java/org/apache/maven/internal/MessageHelper.java:
##########
@@ -0,0 +1,91 @@
+package org.apache.maven.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.util.ArrayList;
+import java.util.List;
+
+/**
+ * Helper class to format warning messages to the console
+ */
+public class MessageHelper
+{
+
+ private static final int DEFAULT_MAX_SIZE = 65;
+ private static final char BOX_CHAR = '*';
+
+ public static String separatorLine()
+ {
+ StringBuilder sb = new StringBuilder( DEFAULT_MAX_SIZE );
+ repeat( sb, '*', DEFAULT_MAX_SIZE );
+ return sb.toString();
+ }
+
+ public static List<String> formatWarning( String... lines )
Review Comment:
ups ... 😞 I missed that is for 3.8.x also ...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on pull request #736: [3.8.x] [MNG-7476] Display a warning when an aggregator mojo is lock other mojos executions
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #736:
URL: https://github.com/apache/maven/pull/736#issuecomment-1125400966
> > This now interferes/depends on the other PR, do you want to keep this one as-is or use the old style for now until the message formatting is properly defined/discussed?
>
> I'd rather commit that one to 3.8.x and other branches asap and investigate the other PR for 3.9.x and master once this one is committed.
Alright, will retest tomorrow.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on a diff in pull request #736: [3.8.x] [MNG-7476] Display a warning when an aggregator mojo is lock other mojos executions
Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #736:
URL: https://github.com/apache/maven/pull/736#discussion_r872514772
##########
maven-core/src/main/java/org/apache/maven/internal/MessageHelper.java:
##########
@@ -0,0 +1,91 @@
+package org.apache.maven.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.util.ArrayList;
+import java.util.List;
+
+/**
+ * Helper class to format warning messages to the console
+ */
+public class MessageHelper
+{
+
+ private static final int DEFAULT_MAX_SIZE = 65;
+ private static final char BOX_CHAR = '*';
+
+ public static String separatorLine()
+ {
+ StringBuilder sb = new StringBuilder( DEFAULT_MAX_SIZE );
+ repeat( sb, '*', DEFAULT_MAX_SIZE );
+ return sb.toString();
+ }
+
+ public static List<String> formatWarning( String... lines )
Review Comment:
No, just a hint that this has been invented.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] gnodet commented on a diff in pull request #736: [3.8.x] [MNG-7476] Display a warning when an aggregator mojo is lock other mojos executions
Posted by GitBox <gi...@apache.org>.
gnodet commented on code in PR #736:
URL: https://github.com/apache/maven/pull/736#discussion_r872522599
##########
maven-core/src/main/java/org/apache/maven/lifecycle/internal/MojoExecutor.java:
##########
@@ -232,7 +237,17 @@ private static class ProjectLock implements AutoCloseable
boolean aggregator = mojoDescriptor.isAggregator();
acquiredAggregatorLock = aggregator ? aggregatorLock.writeLock() : aggregatorLock.readLock();
acquiredProjectLock = getProjectLock( session );
- acquiredAggregatorLock.lock();
+ if ( !acquiredAggregatorLock.tryLock() )
+ {
+ for ( String s : MessageHelper.formatWarning(
+ "An aggregator Mojo is already executing in parallel build, but aggregator "
+ + "Mojos require exclusive access to reactor to prevent race conditions. This "
+ + "mojo execution will be blocked until the aggregator work is done." ) )
+ {
Review Comment:
Found it. But that makes me wonder if we should display some more detailed infos about the aggregator mojo. It may not be trivial to find it in the whole build. Thoughts ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [3.8.x] [MNG-7476] Display a warning when an aggregator mojo locks other mojos executions [maven]
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on PR #736:
URL: https://github.com/apache/maven/pull/736#issuecomment-2852322063
> This needs to be applied to 3.8.x, 3.9.x and master branches
Do we did it ?
Can you remember issues or PR for it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] gnodet commented on pull request #736: [MNG-7476] Display a warning when an aggregator mojo is lock other mojos executions
Posted by GitBox <gi...@apache.org>.
gnodet commented on PR #736:
URL: https://github.com/apache/maven/pull/736#issuecomment-1124684085
Fwiw, I've raised https://github.com/apache/maven/pull/737 to make the console output adapt to the terminal width.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on a diff in pull request #736: [3.8.x] [MNG-7476] Display a warning when an aggregator mojo is lock other mojos executions
Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #736:
URL: https://github.com/apache/maven/pull/736#discussion_r872514294
##########
maven-core/src/main/java/org/apache/maven/internal/MessageHelper.java:
##########
@@ -0,0 +1,91 @@
+package org.apache.maven.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.util.ArrayList;
+import java.util.List;
+
+/**
+ * Helper class to format warning messages to the console
+ */
+public class MessageHelper
+{
+
+ private static final int DEFAULT_MAX_SIZE = 65;
+ private static final char BOX_CHAR = '*';
+
+ public static String separatorLine()
+ {
+ StringBuilder sb = new StringBuilder( DEFAULT_MAX_SIZE );
+ repeat( sb, '*', DEFAULT_MAX_SIZE );
+ return sb.toString();
+ }
+
+ public static List<String> formatWarning( String... lines )
+ {
+ int size = DEFAULT_MAX_SIZE;
+ int rem = size - 4;
+ List<String> result = new ArrayList<>();
+ StringBuilder sb = new StringBuilder( size );
+ // first line
+ sb.setLength( 0 );
+ repeat( sb, BOX_CHAR, size );
+ result.add( sb.toString() );
+ // lines
+ for ( String line : lines )
+ {
+ sb.setLength( 0 );
+ String[] words = line.split( " " );
+ for ( String word : words )
+ {
+ if ( sb.length() >= rem - word.length() - ( sb.length() > 0 ? 1 : 0 ) )
+ {
+ repeat( sb, ' ', rem - sb.length() );
+ result.add( BOX_CHAR + " " + sb + " " + BOX_CHAR );
+ sb.setLength( 0 );
+ }
+ if ( sb.length() > 0 )
+ {
+ sb.append( ' ' );
+ }
+ sb.append( word );
+ }
+
+ while ( sb.length() < rem )
+ {
+ sb.append( ' ' );
+ }
+ result.add( BOX_CHAR + " " + sb + " " + BOX_CHAR );
+ }
+ // last line
+ sb.setLength( 0 );
+ repeat( sb, BOX_CHAR, size );
+ result.add( sb.toString() );
+ return result;
+ }
+
+ private static void repeat( StringBuilder sb, char c, int nb )
+ {
+ for ( int i = 0; i < nb; i++ )
+ {
+ sb.append( c );
+ }
Review Comment:
I don't object, but wanted to point out that this wheel has already been invented
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] gnodet commented on pull request #736: [MNG-7476] Display a warning when an aggregator mojo is lock other mojos executions
Posted by GitBox <gi...@apache.org>.
gnodet commented on PR #736:
URL: https://github.com/apache/maven/pull/736#issuecomment-1125021926
> This now interferes/depends on the other PR, do you want to keep this one as-is or use the old style for now until the message formatting is properly defined/discussed?
I'd rather commit that one to 3.8.x and other branches asap and investigate the other PR for 3.9.x and master once this one is committed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] pzygielo commented on a diff in pull request #736: [3.8.x] [MNG-7476] Display a warning when an aggregator mojo is lock other mojos executions
Posted by GitBox <gi...@apache.org>.
pzygielo commented on code in PR #736:
URL: https://github.com/apache/maven/pull/736#discussion_r873595696
##########
maven-core/src/main/java/org/apache/maven/internal/MessageHelper.java:
##########
@@ -0,0 +1,91 @@
+package org.apache.maven.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.util.ArrayList;
+import java.util.List;
+
+/**
+ * Helper class to format warning messages to the console
+ */
+public class MessageHelper
+{
+
+ private static final int DEFAULT_MAX_SIZE = 65;
+ private static final char BOX_CHAR = '*';
+
+ public static String separatorLine()
+ {
+ StringBuilder sb = new StringBuilder( DEFAULT_MAX_SIZE );
+ repeat( sb, '*', DEFAULT_MAX_SIZE );
+ return sb.toString();
+ }
+
+ public static List<String> formatWarning( String... lines )
+ {
+ int size = DEFAULT_MAX_SIZE;
+ int rem = size - 4;
Review Comment:
It would, at least, introduce the name for the 4. Now it's magic.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] gnodet commented on pull request #736: [3.8.x] [MNG-7476] Display a warning when an aggregator mojo is lock other mojos executions
Posted by GitBox <gi...@apache.org>.
gnodet commented on PR #736:
URL: https://github.com/apache/maven/pull/736#issuecomment-1128039573
> @gnodet Can the reproducer be turned into an IT?
I suppose. The failure would lead to a deadlock. is there a timeout that can be set in the invoker ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on pull request #736: [3.8.x] [MNG-7476] Display a warning when an aggregator mojo is lock other mojos executions
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #736:
URL: https://github.com/apache/maven/pull/736#issuecomment-1128035343
@gnodet Can the reproducer be turned into an IT?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on pull request #736: [3.8.x] [MNG-7476] Display a warning when an aggregator mojo locks other mojos executions
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #736:
URL: https://github.com/apache/maven/pull/736#issuecomment-1140496544
IT aligned and blessed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on pull request #736: [3.8.x] [MNG-7476] Display a warning when an aggregator mojo locks other mojos executions
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #736:
URL: https://github.com/apache/maven/pull/736#issuecomment-1140855001
> @michael-o I've slightly refactored the `MojoExecutor` to cleanup the access to the `mojos` map.
This looks good and makes sense. Totally scoped in the `ProjectLock`. As far as I can see this commit shall be merged into "[MNG-7487]" and "[MNG-7476]" rebased on top of that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org