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

[Backport release-1.4.0-alpha2] Deploy flattened POMs and reduce BOM to "public" modules #8856

Merged
6 commits merged into from
Mar 1, 2022

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Mar 1, 2022

Description

Backport of #8833 to release-1.4.0-alpha2.

relates to #8716

npepinpe and others added 6 commits March 1, 2022 09:18
Introduces the `flatten-maven-plugin` in the build process, starting at
the BOM. The BOM is flattened according to the `bom` rules, and the rest
of the projects are flattened according to the `ossrh` rules.

(cherry picked from commit 93e795e)
Reduces the set of modules specified in the BOM to only these modules
which are meant for third-party consumption. This means, only the
modules which are expected and prepared to be used in projects outside
of Zeebe. All other so called internal modules are defined in the
parent.

The goal here is to make it more explicit to third party users which
modules they can safely include in their projects, and which are more
likely to change/disappear/break over time, and thus they should not
rely on. They can still import them, but they must now do so explicitly
and at their own risk.

(cherry picked from commit 93655bb)
Keep the flatten plugin `outputDirectory` configuration to the default,
i.e. the source pom's own directory. This ensures plugins running post
flatten will be able to correctly resolve the project's root directory
from the location of the POM file, making sure resource resolution
works. For example, Spotbugs will resolve configuration files relatively
to the POM's parent directory.

(cherry picked from commit d832da5)
The feel module contains a `spotbugs/spotbugs-exclude.xml` file that
specifies that the scala classes should be ignored. Thus far, it was
unclear how this file was picked-up by the spotless-maven-plugin,
considering that `parent/pom.xml` specifies the file as
`spotbugs/spotbugs-exclude.xml`. Note that this does not include the
`${project.basedir}` and thus refers to the root project, instead of the
feel module's own file.

The only other module that defines its own spotbugs-exclude file is the
expression-language. The `expression-language/pom.xml` file uses the
`${project.basedir}` to refer to its own file and overwrite the base one
(defined by the build-tools module). This makes it additionally strange
that the exclusion file in the feel module could be picked up by
spotbugs. So what is going on?

Running mvn clean install -Pspotbugs -X shows us:
[DEBUG]   Adding Exclude Filter File
[DEBUG] resource is spotbugs/spotbugs-exclude.xml
[DEBUG] artifact is spotbugs-exclude.xml
[DEBUG] The resource 'spotbugs/spotbugs-exclude.xml' was found as
'/Users/korthout/dev/camunda-cloud/zeebe/feel/spotbugs/spotbugs-exclude.xml'
[DEBUG] location of resourceFile file is
/Users/korthout/dev/camunda-cloud/zeebe/feel/target/spotbugs-exclude.xml

So leaving out the `${project.basedir}` works, but why? Well, it seems
that spotbugs resolves the configuration files using the location of the
pom file. Specifically it resolves them relatively to the POM's parent
directory. We ran into a problem with that while adding the flatten
plugin, because it was changing the POM's location for any plugins that
ran after the flatten plugin. A note has been added to that plugin's
configuration to not make that mistake again.

Coming back to our spotbugs configuration, we can now improve things in
multiple ways:
- remove the property in expression-lanaguage that overwrites the
  spotbugs configuration file's location
- add that same property to the feel-integration module
- change the spotbugs configuration by providing a file to each leaf
  project and completely remove the common configuration

I've chosen to remove the property from the expression-language that
overwrites the spotbugs configuration file's location. My main reason is
that it is not necessary to configure this. Since it is not necessary it
can (and has) lead to inconsistent configuration (see feel and
expression-language difference). And this might happen again when we try
to copy such config. By removing the property, the only configuration
that is left, are the actual spotbugs-exclude.xml files. Copying it to
another module, will make that module use it as well.

(cherry picked from commit 8c33dfa)
The BOM inherits from the camunda-release-parent, which sets a bunch of
values that don't make sense for zeebe's bom. This overwrites does
values with better fitting alternatives.

Alternatively, this could have inlines the inheritted parent pom. But
this removes central control over the deployment of maven modules.
Perhaps we'll reconsider this later, but for now, it's easier to simply
overwrite the values.

Note that the description of the BOM has been emptied, because zeebe's
parent-pom inherits from the zeebe-bom. This avoids that any submodules
that do not define a description themselves inherit the
camunda-release-parent's description. In the future we may change this
by properly providing a description to each of the poms.

(cherry picked from commit 764a19c)
The readme still mentioned that the bom contains all zeebe modules. This
is no longer true.

This updates the readme by explaining what it does contain, what it can
be used for and provides an example.

(cherry picked from commit a9ae286)
Copy link
Member

@lenaschoenburg lenaschoenburg left a comment

Choose a reason for hiding this comment

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

bors r+

@ghost ghost merged commit 309a8d6 into release-1.4.0-alpha2 Mar 1, 2022
@ghost ghost deleted the backport-8833-to-release-1.4.0-alpha2 branch March 1, 2022 09:43
This pull request was closed.
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.

3 participants