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

kie-issues#1506: Use per-package local Maven repositories for dependency declaration correctness on kie-tools #2635

Merged
merged 13 commits into from
Oct 9, 2024

Conversation

tiagobento
Copy link
Contributor

@tiagobento tiagobento commented Oct 2, 2024

Closes apache/incubator-kie-issues#1506


This PR introduces a new way of working with Maven on kie-tools. It takes advantage of the maven.repo.local.tail configuration option to allow not installing all packages to maven.repo.local and risk depending on undeclared packages.

This new structure depends on quarkusio/quarkus#43352, so it can only be merged once kie-tools is using a version of Quarkus that contains this PR.

There are a lot of things to be written about this topic, and I'll get to it once we're closer to being able to merge this.

I made the ""per-package dist"" way of wiring Maven-based packages together be disabled for now, so we can proceed merging this PR and I'll make it the default once quarkusio/quarkus#43352 is available on Quarkus.

In this PR:

  • Created _intellij-project to hold files that allow importing kie-tools on IntelliJ as a single project.
  • Renamed some variables on Webpack and other configuration files standardizing the use of envs. We should reserve the name buildEnv for the tool not the values.
  • Merged maven-config-setup-helper into maven-base, Making maven-base the only requirement for Maven-based packages.
  • Configured -ntp where possible to save space on logs.
  • Stopped copying unnecesasry artifacts inside SonataFlow devmode and builder images.
  • Improved and of top-level POMs on Maven-based packages.
  • Centralized almost all 3rd party dependencies into maven-base's
  • Create and configured include-1st-party-dependencies to allow for running mvn during bootstrap, when dependencies might not have been built yet.
  • Removed a lot of unnecessary configuration on Maven-based packages POMs, which is not centralized on maven-base's POM. (E.g., maven plugins versions)
  • Moved mvnw installation to maven-base, instead of it being a script on package.json. Although I think we could get rid of this entirely.
  • Proper cleanup of m2-repo-via-http-image on dev-deployment-kogito-quarkus-blank-app-image/package.json
  • Created m2-repo-via-http:container:prepare-m2-repo-volume scripts on packages depending on maven-m2-repo-via-http-image to support multiple local Maven repositories, using hard links.
  • Removed unnecessary DEV_DEPLOYMENT_KOGITO_QUARKUS_BLANK_APP_IMAGE__mavenM2RepoViaHttpImage env var.
  • Configured kn-plugin-workflow tests to properly wire local test projects with -Dmaven.repo.local.tail.
  • Renamed $(build-env quarkusPlatform.version) to $(build-env versions.quarkus)
  • Renamed $(build-env kogitoRuntime.version) to $(build-env versions.kogito)
  • Introduced a basic MANUAL.md file containing a little bit of information about the kie-tools monorepo.

Comments and questions are welcome! :)


P.S.: It's expected that a few tests will break on kn-plugin-workflow because we're using Quarkus 3.8.6, which doesn't contain the PR I mentioned above.

@tiagobento tiagobento added pr: DO NOT MERGE Draft PR, not ready for merging area:monorepo labels Oct 2, 2024
@tiagobento tiagobento changed the title [DO NOT MERGE] NO-ISSUE: Improve Maven-based packages [DO NOT MERGE] kie-issues#1506: Use per-package local Maven repositories for dependency declaration correctness on kie-tools Oct 2, 2024
Rename

Fix build reproducibility

v2

Oops

Fix DashBuilder configuration of maven-deploy-plugin

Oops

Fix maven-m2-repo-via-http-image configuration

.

Fix config

removing mvn.tail

Maybe fix build

Rename method

Oops

Merge maven-config-setup-helper with maven-base

Oops

Oops

env and more stuff

Oops. Lockfile

Force not skipping local deploy

Fix

Hm

No transfer progres

Rename versions env

Reverting root files

IntelliJ project configuration

.
@tiagobento tiagobento force-pushed the maven-local-2 branch 5 times, most recently from c973e4d to 4f0f043 Compare October 5, 2024 01:50
@@ -0,0 +1,8 @@
<?xml version="1.0" encoding="UTF-8"?>
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this shouldn't be added right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should. *.iml files are what allow us to have pre-configured default for modules of _intellij-project.

Copy link
Contributor

Choose a reason for hiding this comment

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

then +1

@tiagobento tiagobento removed the pr: DO NOT MERGE Draft PR, not ready for merging label Oct 7, 2024
@tiagobento tiagobento changed the title [DO NOT MERGE] kie-issues#1506: Use per-package local Maven repositories for dependency declaration correctness on kie-tools kie-issues#1506: Use per-package local Maven repositories for dependency declaration correctness on kie-tools Oct 7, 2024
Copy link
Member

@thiagoelg thiagoelg left a comment

Choose a reason for hiding this comment

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

Awesome job! 🎉
This solves most of the issues we had when building Java packages on kie-tools. Thanks @tiagobento!

@yesamer yesamer self-requested a review October 8, 2024 13:47
Copy link
Contributor

@yesamer yesamer left a comment

Choose a reason for hiding this comment

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

So far, just some missing headers.
I guess *.iml file shouldn't have the headers, because autogenerated by IntellliJ. In such a case, please ignore them

<remote-repository>
<option name="id" value="jdt.ls.p2" />
<option name="name" value="jdt.ls.p2" />
<option name="url" value="https://download.eclipse.org/jdtls/milestones/1.30.1/repository/" />
Copy link
Contributor

Choose a reason for hiding this comment

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

@tiagobento This repo is strictly related to the current jdt.ls version used in our code. Said so, I would add a comment here and in the vscode-java-code-completion-extension-plugin/pom.xml to warn the user to update the repo when the version is updated.

<version.maven.jar.plugin>3.4.1</version.maven.jar.plugin>
<version.maven.remote.resources.plugin>3.2.0</version.maven.remote.resources.plugin>
<version.maven.clean.plugin>3.4.0</version.maven.clean.plugin>
<version.codehaus.flatten.plugin>1.6.0</version.codehaus.flatten.plugin>
Copy link
Contributor

Choose a reason for hiding this comment

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

As we discussed in private, as a future improvement I would discuss better centralizing the maven plugin management, declaring them in the maven base pom and inheriting them from the apache pom @tiagobento @pefernan

Copy link
Contributor

@yesamer yesamer left a comment

Choose a reason for hiding this comment

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

Great improvement @tiagobento!

@tiagobento tiagobento merged commit 8756ef6 into apache:main Oct 9, 2024
14 checks passed
@yesamer yesamer deleted the maven-local-2 branch October 10, 2024 06:31
@yesamer yesamer restored the maven-local-2 branch October 10, 2024 06:32
fantonangeli added a commit to fantonangeli/kie-tools that referenced this pull request Oct 25, 2024
rgdoliveira pushed a commit to kiegroup/kie-tools that referenced this pull request Oct 25, 2024
…changes (#57)

* Apply updates from apache#2635

* Updates from
apache#2455

* Updates from apache#2376

* Fixed wrong value for SONATAFLOW_MANAGEMENT_CONSOLE_WEBAPP__sonataflowEnvMode
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.

Use per-package local Maven repositories for dependency declaration correctness on kie-tools
5 participants