soteria icon indicating copy to clipboard operation
soteria copied to clipboard

Problems with pom.xml file

Open glassfishrobot opened this issue 8 years ago • 9 comments

The soteria pom.xml file (and/or child poms) have several problems:

  • The when deploying soteria, all test artifacts are pushed to the repository along with the jarfile, sources, and javadoc artifacts.
  • The dependency on the API is brittle. If the <spec_impl_version> is a snapshot, it's not possible to build a release, so that property has to be manually edited and pushed to the repo prior to building a release, and then restored to SNAPSHOT afterwards. SNAPSHOT is permanently removed, it becomes impossible to build and install locally when making interdependent changes to both API and RI (doesn't seem to be able to find API jars in the "promoted" repository). Similarly, it needs to be updated every time the API build revs. Ideally, we could specify something like "latest" in a way that would work for both release and snapshot builds without requiring pom changes.
  • The following warning, likely caused by the spec-version-maven-plugin being in the top level pom. Might be enough to just move it down; not sure if the top level pom has any dependencies on it.
[WARNING] 
[WARNING] Some problems were encountered while building the effective model for org.glassfish.soteria:javax.security.enterprise:bundle:1.0-b11-SNAPSHOT
[WARNING] 'build.plugins.plugin.version' for org.glassfish.build:spec-version-maven-plugin is missing. @ org.glassfish.soteria:javax.security.enterprise:[unknown-version], /code/jsr375/security-soteria/impl/pom.xml, line 38, column 21
[WARNING] 
[WARNING] It is highly recommended to fix these problems because they threaten the stability of your build.
[WARNING] 
[WARNING] For this reason, future Maven versions might no longer support building such malformed projects.
[WARNING] 

There may be other issues -- Arjan mentioned the eclipse plugin not liking the bundle packaging -- but those are the ones I'm aware of at the moment.

glassfishrobot avatar Jul 26 '17 02:07 glassfishrobot

  • Issue Imported From: https://github.com/javaee/security-soteria/issues/131
  • Original Issue Raised By:@wmhopkins
  • Original Issue Assigned To: @wmhopkins

glassfishrobot avatar May 25 '18 05:05 glassfishrobot

@arjantijms Commented

The when deploying soteria, all test artifacts are pushed to the repository along with the jarfile, sources, and javadoc artifacts.

This was originally proposed by Oracle employee Manfred Riem. He clarified that the advantage is that you have a 100% clear record then of which version passed for the deployed binary.

There may be other issues -- Arjan mentioned the eclipse plugin not liking the bundle packaging -- but those are the ones I'm aware of at the moment.

Maybe something changed, or maybe Eclipse was being Eclipse, but it's not complaining anymore now.

glassfishrobot avatar Jul 26 '17 06:07 glassfishrobot

@wmhopkins Commented The following issues are fixed in 2f51c3e:

  • Brittle dependency on API. This is still hard-coded, and will need to be updated when new versions of the API jar are published, but there is now a "release" profile override for the two properties used, <spec_build> and <spec_impl_version>, so that release builds can be done without editing the file. Coding for a dependency on "latest" seems to be an anti-pattern, and is difficult to do using SNAPSHOT builds. In any event, we're very close to final release, so the API version will stop changing very soon.
  • Fixed the spec-version-maven-plugin warning by specifying the version of the plugin to use. Added the Glassfish Promoted repo to repositories, so can build against promoted versions of the API jar.

Issues still outstanding are:

  • Haven't yet found a good way to build just the impl project for release. It does seem I could do a prepare, then script commands to manually check out the tagged version, cd to impl, and run mvn deploy -P release, but scripting all that, plus the required cleanup, doesn't seem worth the trouble, as the artifacts produced don't seem different from what we'd get by building/releasing all the artifacts, then dropping the parent and test artifacts before closing the staging repo.
  • Discovered another potential issue, in that the pom file that gets released with the artifacts is full of references to properties, rather than hard coded values, for dependencies, various spec/impl version numbers, etc. We probably need to push as much of that as possible down into the impl pom.

glassfishrobot avatar Aug 17 '17 04:08 glassfishrobot

@wmhopkins Commented The following stackoverflow thread describes how to filter the modules that are deployed: is-it-possible-to-filter-artifacts-in-maven-release-plugin.

glassfishrobot avatar Aug 17 '17 11:08 glassfishrobot

@arjantijms Commented

Haven't yet found a good way to build just the impl project for release. It does seem I could do a prepare, then script commands to manually check out the tagged version, cd to impl, and run mvn deploy -P release, but scripting all that, plus the required cleanup, doesn't seem worth the trouble, as the artifacts produced don't seem different from what we'd get by building/releasing all the artifacts, then dropping the parent and test artifacts before closing the staging repo.

This is was actually explicitly done this way and modelled after the MVC project. The spec lead at the time (Manfred Riem) brought forward the point of then having an indisputable record of which tests were passed for the binary in the repo (e.g. maven central).

I'd personally not worry about this too much.

Edit: just noticed I had already mentioned the same thing just a few comments up ;)

glassfishrobot avatar Aug 17 '17 14:08 glassfishrobot

@wmhopkins Commented Yes, I'd seen the earlier comment. I understand the POV, and it may make sense to continue publishing tests for snapshots, but I really don't see the need for published and labeled builds to include tests, as the exact set of tests can be obtained by using the corresponding label. Anybody that's running tests should be able to clone the repo and build them from source. Publishing a large (and growing) set of tests every time the binary artifact is published seems like a waste of space, and, while I agree it's not something to be really concerned about, the clutter annoys me. ;)

Maybe I'll ping Manfred to get his POV more fully. Has there been reason, in your experience, to be concerned about people disputing the test results? I.e., has it been important in the past to guarantee an exact correlation between built artifacts and tests?

glassfishrobot avatar Aug 17 '17 15:08 glassfishrobot

@arjantijms Commented Publishing the tests along with the RI makes it very easy for anyone to run the tests without having to worry about building.

So if a challenge comes in and there is a test you can point to the WAR and let them deploy it and run the tests.

glassfishrobot avatar Aug 17 '17 15:08 glassfishrobot

@wmhopkins Commented True enough -- I just wonder how often there is actually a "challenge" for which this would matter. I've emailed Manfred to get his POV. Either way, this isn't critical for this release, so leaving the issue open for now, but deferring out of the RI Code Complete milestone.

glassfishrobot avatar Aug 17 '17 18:08 glassfishrobot

@wmhopkins Commented Added "enhancement" label because some of the issues here were bugs (now fixed) but the remaining open items are essentially enhancements.

glassfishrobot avatar Aug 17 '17 18:08 glassfishrobot