Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix/ladwlo thread safety fix #22

Open
wants to merge 2 commits into
base: gmaven-1.x
Choose a base branch
from

Conversation

ladwlo
Copy link

@ladwlo ladwlo commented Sep 22, 2020

Fix for issue #21

<module>gmaven-runtime-1.6</module>
<module>gmaven-runtime-1.7</module>
<module>gmaven-runtime-1.8</module>
<module>gmaven-runtime-2.0</module>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

profiles turn these on, any reason why these were added here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I just didn't realize there were profiles (and plain 'mvn install' complained about missing snapshot of gmaven-runtime-1.6). I removed that change - thanks.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe the profiles aren't kicking on properly as they should, will have a look when I merge this. thx.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it's because the 1.x branch is old and unaware of jdk's >1.7. Since I built it with Java 8, no profile was matched to the JDK. Maybe I should have added a section for java8, but when I simply used -Pjava7 everything built fine.

@ladwlo ladwlo force-pushed the bugfix/ladwlo-thread-safety-fix branch from ce69d29 to bb54417 Compare September 23, 2020 05:44
@jdillon
Copy link
Member

jdillon commented Sep 24, 2020

FTR its unfortunate that older maven-plugins that are not marked as thread-safe are not adapted in a MT maven-execution to be run by a single thread; I recall there is a deeper problem in the integration that makes it not thread-safe but I don't remember what that is ATM.

And sadly I never use the MT maven because many plugins don't play well with it, so I never run into these problems, but maybe the general ecosystem has improved wrt to the use of --threads.

@ladwlo
Copy link
Author

ladwlo commented Sep 24, 2020

I fully agree. I did observe some suspicious behavior of liquibase plugin and groovy-eclipse-compiler in MT mode as well. Luckily, it seems that the set of maven plugins used in our project works quite well with threads - at least I haven't seen failures of our build process since making the gmaven plugin safe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants