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

Add a releaseProfile to generate dependency reduced pom #12367

Merged
merged 2 commits into from
Oct 23, 2020

Conversation

maobaolong
Copy link
Contributor

@maobaolong maobaolong commented Oct 22, 2020

#12338

This is a workaround for MSHADE-148, which leads to an infinite loop when building Alluxio.

There are two scenarios.

  • Releasing Alluxio
    Maven build with -Prelease.
    • If using newer than 3.3.x version of maven, you also have to use sequential build, Like mvn clean install -Prelease xxxx.
    • If using lower version maven before 3.3.x, you can use parallel build, e.g. mvn clean install -T 4 -Prelease xxxx
  • Not Releasing Alluxio
    If you are not releasing Alluxio, you can build Alluxio like before, please don't add the -Prelease into you build command line. e.g. mvn clean install -T 4 xxxx

This patch adds a -Prelease. If present, it will set createDependencyReducedPom true. The consequences are:

If you are releasing Alluxio with this profile, you are fine as long as before.
If you are releasing Alluxio without this profile, the alluxio-shaded-xxx-yyy.pom will be the same as the pom.xml under the module directory.
If you are not releasing Alluxio but you are using this profile, you may run into #12338
If you are not releasing Alluxio and you did not include this profile, you are fine.
This is all documented in pom.xml and tested locally with maven 3.6.3.

@alluxio-bot
Copy link
Contributor

Automated checks report:

  • Commits associated with Github account: PASS
  • PR title follows the conventions: FAIL
    • The title of the PR does not pass all the checks. Please fix the following issues:
      • First word must be capitalized

Some checks failed. Please fix the reported issues and reply 'alluxio-bot, check this please' to re-run checks.

@alluxio-ci
Copy link

Can one of the admins verify this patch?

@maobaolong maobaolong changed the title add a releaseProfile to generate dependency reduced pom Add a releaseProfile to generate dependency reduced pom Oct 22, 2020
@alluxio-bot
Copy link
Contributor

Automated checks report:

  • Commits associated with Github account: PASS
  • PR title follows the conventions: PASS

All checks passed!

pom.xml Outdated
@@ -1487,5 +1488,19 @@
</plugins>
</build>
</profile>
<profile>
<id>release-profile</id>
Copy link
Contributor

Choose a reason for hiding this comment

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

shall we add it here too @Xenorith -Prelease-profile

args := []string{"-T", "2C", "-am", "clean", "install", "-DskipTests", "-Dfindbugs.skip", "-Dmaven.javadoc.skip", "-Dcheckstyle.skip", "-Pno-webui-linter"}

Copy link
Contributor

Choose a reason for hiding this comment

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

from my understanding but correct me if i'm wrong, -Prelease-profile is only necessary when building artifacts to be published to maven. in all other cases, we would not want this profile to be enabled

Copy link
Contributor

Choose a reason for hiding this comment

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

@LuQQiu can you confirm that we do not want to append this profile argument to generate-tarball.go because this doesn't get executed when publishing to maven?

Copy link
Contributor

Choose a reason for hiding this comment

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

Not quite sure if we need it when generating the tarball. But publish to maven doesn't have relationships with generate-tarball.go

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@LuQQiu Thanks for your reply, as publish to maven doesn't have relationships with generate-tarball.go, so I think we don't need to add -Prelease here.

Copy link
Contributor

@apc999 apc999 left a comment

Choose a reason for hiding this comment

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

I would just call this release instead of relase-profile.
otherwise LGTM

@calvinjia do you want to review this pr too?

@maobaolong
Copy link
Contributor Author

@apc999 @LuQQiu @Xenorith Thanks for your review and suggestion, I pushed a new commit to rename the release-profile to release, PTAL.

Copy link
Contributor

@Xenorith Xenorith left a comment

Choose a reason for hiding this comment

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

thanks for your efforts to resolve this issue. i verified that building maven without the release profile works. will wait for others to take a look and approve before merging

@maobaolong
Copy link
Contributor Author

@Xenorith Thanks a lot.

@apc999
Copy link
Contributor

apc999 commented Oct 23, 2020

alluxio-bot, merge this please

@alluxio-bot alluxio-bot merged commit 91e1e86 into Alluxio:master Oct 23, 2020
@maobaolong
Copy link
Contributor Author

Thank @apc999 for merging this.

@apc999
Copy link
Contributor

apc999 commented Oct 23, 2020

alluxio-bot, cherry-pick this to branch-2.4 please

@alluxio-bot
Copy link
Contributor

Auto cherry-pick to branch branch-2.4 successfully opened PR: #12377

alluxio-bot pushed a commit that referenced this pull request Oct 23, 2020
#12338

This is a workaround for MSHADE-148, which leads to an infinite loop
when building Alluxio.

There are two scenarios.
- Releasing Alluxio
Maven build with `-Prelease`.
- If using newer than `3.3.x` version of maven, you also have to use
sequential build, Like `mvn clean install -Prelease xxxx`.
- If using lower version maven before `3.3.x`, you can use parallel
build, e.g. `mvn clean install -T 4 -Prelease xxxx`
- Not Releasing Alluxio
If you are not releasing Alluxio, you can build Alluxio like before,
please don't add the `-Prelease` into you build command line. e.g. `mvn
clean install -T 4 xxxx`

This patch adds a -Prelease. If present, it will set
createDependencyReducedPom true. The consequences are:

If you are releasing Alluxio with this profile, you are fine as long as
before.
If you are releasing Alluxio without this profile, the
`alluxio-shaded-xxx-yyy.pom` will be the same as the pom.xml under the
module directory.
If you are not releasing Alluxio but you are using this profile, you may
run into #12338
If you are not releasing Alluxio and you did not include this profile,
you are fine.
This is all documented in pom.xml and tested locally with maven 3.6.3.

pr-link: #12367
change-id: cid-2f26f91c9f3198d9dac11633888de0a9f8af2ce6
alluxio-bot added a commit that referenced this pull request Oct 23, 2020
Cherry-pick of existing commit.
orig-pr: #12367
orig-commit: 91e1e86
orig-commit-author: maobaolong <[email protected]>

pr-link: #12377
change-id: cid-2f26f91c9f3198d9dac11633888de0a9f8af2ce6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants