-
Notifications
You must be signed in to change notification settings - Fork 35
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
HDDS-11630. Add Build from Maven guide #100
HDDS-11630. Add Build from Maven guide #100
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this @ptlrs. There's a fair amount of maven options hidden in Ozone that would be good to document here too. We can probably collect input from other devs on what they are using. Some that I have saved:
- Skip pnpm build of Recon front-end:
-Dskip.npx=true
- Skip shading Hadoop client jars:
-DskipShade
These options greatly speed up the build if you are a developer and know you do not need either of these components.
|
||
## Prerequisites | ||
|
||
**TODO** : [HDDS-11625](https://issues.apache.org/jira/browse/HDDS-11625) Finalize the version numbers of prerequisite packages |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's see if we can finalize this as part of this change as well. The only thing I'm not sure about is the maven version. Really Ozone should define this in its pom probably using something like the Maven enforcer plugin. Maybe we should raise a PR to add this to the main Ozone repo and then update the docs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good point we should take advantage of the enforcer plugin to have reproducible builds.
However, what should be considered the minimum version for Maven and Git? We depend on Hadoop and I can see if they have a minimum version specified for Maven.
On that note, it is also not clear what the minimum version of Java should be. We do build with versions 8, 11, 17. Some of the pom files specify the compiler source and target versions to Java 8. Some of the tests fail when run from the IDE with a newer JDK and need extra JVM parameters to allow Java lang and util modules and the security manager.
See also some updates to the way we skip Recon frontend build in apache/ozone#7454 |
Co-authored-by: Siyao Meng <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates @ptlrs. I left a few more comments but overall flow and content looks pretty good.
Co-authored-by: Ethan Rose <[email protected]>
Co-authored-by: Ethan Rose <[email protected]>
Co-authored-by: Ethan Rose <[email protected]>
Co-authored-by: Ethan Rose <[email protected]>
Co-authored-by: Ethan Rose <[email protected]>
Co-authored-by: Ethan Rose <[email protected]>
Co-authored-by: Ethan Rose <[email protected]>
…S-11630-build-from-maven-guide
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates, just a few comments left
Co-authored-by: Ethan Rose <[email protected]>
Co-authored-by: Ethan Rose <[email protected]>
Co-authored-by: Ethan Rose <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thanks for the updates! Just a few minor comments
Co-authored-by: Ethan Rose <[email protected]>
Co-authored-by: Ethan Rose <[email protected]>
Co-authored-by: Ethan Rose <[email protected]>
Thank you @errose28 @adoroszlai @jojochuang @smengcl for the reviews.I have made the requested changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the update.
BTW, you don't need to update the PR description with link to new CI runs from your fork. That link is only important initially, to show that you have a clean run before creating the PR. Once created, the PR triggers its own CI run every time you push new changes.
Co-authored-by: Doroszlai, Attila <[email protected]>
Co-authored-by: Doroszlai, Attila <[email protected]>
Thanks @adoroszlai. Good to know. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the continuous updates @ptlrs LGTM
Thanks @ptlrs for the doc, @errose28, @jojochuang, @smengcl for the reviews. |
Thank you for the detailed reviews @adoroszlai, @errose28, @jojochuang and @smengcl. |
What changes were proposed in this pull request?
Added content for the developer guide to build ozone sources using maven
Please describe your PR in detail:
What is the link to the Apache Jira?
https://issues.apache.org/jira/browse/HDDS-11630
How was this patch tested?
CI:
https://github.com/ptlrs/ozone-site/actions/runs/11606635045https://github.com/ptlrs/ozone-site/actions/runs/11811331336https://github.com/ptlrs/ozone-site/actions/runs/11823796643https://github.com/ptlrs/ozone-site/actions/runs/11842696659https://github.com/ptlrs/ozone-site/actions/runs/11842732645https://github.com/ptlrs/ozone-site/actions/runs/12627619493https://github.com/ptlrs/ozone-site/actions/runs/12703203099https://github.com/ptlrs/ozone-site/actions/runs/12757136988
Ran the website locally via docker-compose:
