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

[SUREFIRE-1972] Use current version of surefire-shared-utils #426

Closed
wants to merge 2 commits into from

Conversation

Tibor17
Copy link
Contributor

@Tibor17 Tibor17 commented Jan 2, 2022

This helps us fixing JDK 18 issue (see the PR #422 and #424) regarding deprecated SecurityManager.

Additionally, the project will reference own versions only.

Following this checklist to help us incorporate your
contribution quickly and easily:

  • Make sure there is a JIRA issue filed
    for the change (usually before you start working on it). Trivial changes like typos do not
    require a JIRA issue. Your pull request should address just this issue, without
    pulling in other changes.
  • Each commit in the pull request should have a meaningful subject line and body.
  • Format the pull request title like [SUREFIRE-XXX] - Fixes bug in ApproximateQuantiles,
    where you replace SUREFIRE-XXX with the appropriate JIRA issue. Best practice
    is to use the JIRA issue title in the pull request title and in the first line of the
    commit message.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Run mvn clean install to make sure basic checks pass. A more thorough check will
    be performed on your pull request automatically.
  • You have run the integration tests successfully (mvn -Prun-its clean install).

If your pull request is about ~20 lines of code you don't need to sign an
Individual Contributor License Agreement if you are unsure
please ask on the developers list.

To make clear that you license your contribution under
the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.

Copy link
Member

@slawekjaranowski slawekjaranowski left a comment

Choose a reason for hiding this comment

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

Does attach artifact with next classifier will not have the same effect?

It will be easier - only one additional plugin will be needed.

@Tibor17
Copy link
Contributor Author

Tibor17 commented Jan 5, 2022

@slawekjaranowski
This might be even possible with a classifier. Let me check it out.

@Tibor17
Copy link
Contributor Author

Tibor17 commented Jan 6, 2022

@slawekjaranowski
This looks much better. Please have a look at the second commit. Can I squash it, rebase and merge?
f38a485

@slawekjaranowski
Copy link
Member

looks very good .. go ahead

@Tibor17
Copy link
Contributor Author

Tibor17 commented Jan 6, 2022

@slawekjaranowski
Open this branch in IDEA and you will see red lines. The IDEA does not understand the classifiers. Additionally surefire-booter hangs, I did not have time to have a look yet.

@Tibor17
Copy link
Contributor Author

Tibor17 commented Jan 6, 2022

@slawekjaranowski

I was able to avoid build-helper-maven-plugin and used the trick with setting properties (distRepoId, distRepoUrl) for snapshots by default and for release versions as well. The other two plugins could not be avoided. This works fine with IntelliJ IDEA/ Eclipse. The solution previously used with a classifier did not work with IDEA - red lines in the import section and hanging tests in the module surefire-booter.

@slawekjaranowski
Copy link
Member

Next issue what i see:
install:install-file creates and install empty pom for surefire-shared-utils..jar it is done in install phase ... so probably additional pom will be not signed by gpg during release

I don't know if will be possible release to Central repository without signatures.

@slawekjaranowski
Copy link
Member

We are trying many complicated way to build this artifact ...
I start thinking about remove it and use shade-plugin in final modules - it will be more clear.
We can use shade-plugin with minimizeJar

Copy link
Member

@slawekjaranowski slawekjaranowski left a comment

Choose a reason for hiding this comment

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

@Tibor17 open questions - does artefact managed by install:install-file will be signed during release?

@Tibor17
Copy link
Contributor Author

Tibor17 commented Jan 8, 2022

@slawekjaranowski Well it should be signed, we can check it out but the snapshot deployments worked.
I need to go offline for an hour but then I would try this...

@slawekjaranowski
Copy link
Member

It is not signed ....

 mvn clean install -pl surefire-shared -P apache-release

in local repo I have:

$ ls -l ~/.m2/repository/org/apache/maven/surefire/surefire-shared-utils/3.0.0-M6-SNAPSHOT/
total 3040
...      232 Jan  8 18:37 _remote.repositories
...      741 Jan  8 18:37 maven-metadata-local.xml
...  1543867 Jan  8 18:37 surefire-shared-utils-3.0.0-M6-SNAPSHOT.jar
...      503 Jan  8 18:37 surefire-shared-utils-3.0.0-M6-SNAPSHOT.pom

$ ls -l ~/.m2/repository-prv/org/apache/maven/surefire/surefire-shared/3.0.0-M6-SNAPSHOT/
total 3088
...      408 Jan  8 18:37 _remote.repositories
...     1497 Jan  8 18:37 maven-metadata-local.xml
...     7127 Jan  8 18:37 surefire-shared-3.0.0-M6-SNAPSHOT-sources.jar
...      866 Jan  8 18:37 surefire-shared-3.0.0-M6-SNAPSHOT-sources.jar.asc
...  1543867 Jan  8 18:37 surefire-shared-3.0.0-M6-SNAPSHOT.jar
...      866 Jan  8 18:37 surefire-shared-3.0.0-M6-SNAPSHOT.jar.asc
...     5386 Jan  8 18:37 surefire-shared-3.0.0-M6-SNAPSHOT.pom
...      866 Jan  8 18:37 surefire-shared-3.0.0-M6-SNAPSHOT.pom.asc

@Tibor17
Copy link
Contributor Author

Tibor17 commented Jan 8, 2022

I made it working ;-)
But the profile apache-release is not dedicated for SNAPSHOTS. Only for releases.
Yeah, I had to turn distributionManagement.repository.id to distributionManagement.snapshotRepository.id but it is done to confirm that the GnuPG plugin works as well.
See the maven-release-plugin in the ASF parent. The plugin activates the apache-release profile with gpg, javadoc, sources plugins, etc.

@slawekjaranowski
Copy link
Member

We will not have signature for additional artifact.

gpg is executet in verify phase
install-jar is bind by you to install phase

I don't know what restrictions are for AFS releases ... but for oss by Sonatype each artifact must have signature

Profile apache-release can be used for test, we see what will happen during release

@Tibor17
Copy link
Contributor Author

Tibor17 commented Jan 13, 2022

@slawekjaranowski
The profile apache-release cannot be used together with tests by default because the contributors will see crashed maven-gpg-plugin. I guess all you wanted to say was that the checksums SHA-512 are needed, right?

@michael-o
Copy link
Member

Will check this in the next couple of days. This is maybe 50 shades of Maven.

@Tibor17
Copy link
Contributor Author

Tibor17 commented Jan 13, 2022

As I spoke with Michael. I guess the m-exec-p would help us solve all these issues, the Maven project would build the far jar as it should and the IntelliJ IDEA would see the shaded artifact. It is my hypothesis, we can confirm it in a new commit, let;s see...

@Tibor17
Copy link
Contributor Author

Tibor17 commented Jan 14, 2022

Both alternatives (last two commits) have some disadvantages.
646b38f = has no notion of maven-parent POM.
0b2bed9 = red lines in the code
As I spoke with Karl, he told me some long time ago, that referencing some version like nowadays 3.0.0-M5 is fine. Next time we have to thing in advance and prepare the upgrades for the next version.

@Tibor17
Copy link
Contributor Author

Tibor17 commented Jan 14, 2022

@slawekjaranowski
@michael-o
I found a good way with classifiers and I worked around the IDEA issue with a small trick using profile. Meanwhile see this bug in Jetbrains. Closing in favor of #443, I think you would be very happy.

@Tibor17 Tibor17 closed this Jan 14, 2022
@Tibor17 Tibor17 deleted the jira-1972 branch January 18, 2022 10:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants