Ds 1343 - Add parent pom.xml and build.propertiesfiles into the binary release #119

Merged
merged 7 commits into from Nov 1, 2012

Projects

None yet

4 participants

@tdonohue
Member

This is a slightly updated version of Pull #112.

It merges Robin's changes in that Pull Request, along with the following additions:

  • Removes all "testEnvironment" stuff from the 'Binary' packaged release
  • Ensures that Unit Testing is turned off by default (necessary so Binary release doesn't attempt to run it)
  • Ensures that the testEnvironment.zip is only generated for the Source release (and NOT for Binary release)
  • Ensures the 'mvn package -Pdistribution' command (which builds SourceForge zip/tarball) still works properly, though now it needs to be run from [dspace-src]/dspace/ instead of [dspace-src]
@mdiggory
Member

Interesting, the original dspace-parent pom used "if exists" by design previously, but we switched away from this to be able to disable specific modules from being built in maven.

My thought is that if we want to still be able to enable/disable modules, but also have a binary release with a dspace-parent pom, we will need to move all the dspace-xxx modules under a submodule that dspace-parent uses "if-exists" to enable

dspace-parent (profile with activation "if exists")
dspace-parent/modules
dspace-parent/modules/dspace-api
dspace-parent/modules/dspace-xmlui
dspace-parent/modules/dspace-jspui

exclude "modules" directory from release while allowing both binary and source release to use "mvn -P dspace-api,dspace-xmlui"

@tdonohue
Member

Hi Mark,

Even with this pull request, you can still disable specific modules from being built.

For example, to build all of DSpace except for LNI & JSPUI:

mvn package -P!dspace-lni,!dspace-jspui

So, I don't think we're losing anything by moving to having "if exists" in the main (dspace-parent) POM. Obviously, you are correct that enabling only specific modules to build will no longer work (e.g. "mvn package -Pdspace-api,dspace-xmlui"). But, I'm not sure that is much of a loss?

However, I have noticed that this new "if exists" structure is causing a minor annoyance with creating distribution zip/tarballs (in that all of DSpace is re-compiled, when it's not necessary to recompile for zip/tarball generation).

So, I'm working on a fix to possibly move the zip/tarball assembly stuff over into the [src]/dspace/ subproject (alongside the main assembly.xml file). This seems to work well from my testing thus far, and actually helps to consolidate our multiple "assembly" options into one maven project.

@tdonohue
Member

I've just force updated (git push -f) this pull request, as it got messed up after #112 was initially merged & then undone.

This force update does the following:

  • cherry picks three important commits from @robintaylor's #112 which were necessary
  • Adds two of my own commits to disable Unit testing by default, and also ensure the testEnvironment.zip is NOT built for the "binary" release of DSpace.

There's still a few minor things to take care of here, since the "mvn package -Pdistribution" command (which creates the zip/tarball distribution packages) is still acting slightly oddly. Will push those changes up in just a bit.

@helix84
Member
helix84 commented Oct 31, 2012

Tim, would you mind documenting for posterity which particular commit breaks the -P behaviour?

@tdonohue tdonohue Move the zip/tarball creation assemblies (release.xml & src-release.x…
…ml) to [src]/dspace/src/main/assembly/

Also move the DSpace build assembly (assembly.xml) file to that directory as well, to consolidate.
a05f0d0
@tdonohue
Member

@helix84,

It's the 52fb650 and 62007a0 commits (initially from #112). They change all the build "profiles" in the dspace-parent POM from being...

<activeByDefault>true</activeByDefault>

To being...

<file><exists>[module-name]/pom.xml</exists></file>

Those two commits have the side-effect of breaking "mvn package -Pdspace-api,dspace-xmlui" (which used to allow you to select only specific modules to build). The reason why is that all modules whose "pom.xml" file is found will be auto-executed (essentially the -P is ignored in this situation). However, you can still disable individual modules from building by doing something like "mvn package -P!dspace-lni,!dspace-jspui" (this would build everything except for the LNI & JSPUI).

To be clear though, the "file exists" changes are necessary if we wish to include the "dspace-parent" POM in the "binary" release of DSpace. And, at least at this point, we MUST include the "dspace-parent" POM in the "binary" release of DSpace, in order for the new "build.properties" stuff to work for the binary release. (see 0cb2e07 for these changes)

Hopefully that clarifies all the interwoven issues here.

@helix84
Member
helix84 commented Oct 31, 2012

Hi Tim, thanks for the executive summary for us Maven dummies.

You mentioning auto-executing pom.xml files triggered some wheels in my head - this could be used as a method to build self-contained addons without any modifications to the upstream code. Richard and Joao were discussing addons recently on dspace-devel. But I expect I'm not the first one to think of that usage :)

Sorry for side-tracking and feel free to move this discussion to -devel if you want to reply.

@tdonohue tdonohue Move the 'distributions' profile (which creates the zip/tarball distr…
…ibutions) to the [src]/dspace/pom.xml. Minor updates (including enhanced comments) to all three of our Assemblies (assembly.xml, release.xml and src-release.xml), to ensure they all work properly when run from [src]/dspace/src/main/assembly/. Also minor updates to 'dspace-parent' and 'dspace' POM files. All these changes now fix issues with 'mvn package -Pdistributions'.
7e1b535
@tdonohue
Member

Just pushed a few final commits (a05f0d0 and 7e1b535) to this pull request which do the following:

  • Move the src-release.xml and release.xml assemblies (used to build Sourceforge zip/tarballs) to [src]/dspace/src/main/assembly/ (and moved the "assembly.xml" there alongside it).
  • Enhanced the comments in each of these maven assembly docs, to better describe exactly what they are doing.
  • Updated the 'dspace-parent' & 'dspace' POMs to ensure that 'mvn package -Pdistributions' (which calls the src-release.xml and release.xml assemblies) works properly again. It now needs to be run from [src]/dspace/ though.

I've done some very thorough testing of these changes to ensure I didn't accidentally break anything in the build or release process. From my testing all of the following work great:

  • Normal builds (with or without unit tests enabled) from [dspace-src] or [dspace-src]/dspace/
  • Dry Run release works fine
  • Builds from a zip/tarball distribution (either the *-src-release or the *-release packages).

So, I think this Pull Request is ready to be merged!

@tdonohue
Member

If no one has any objections, I'm going to merge this pull request tomorrow in time for RC3, that that we are able to ensure the *-release zip/tarballs can be tested during next week's testathon.

@tdonohue tdonohue merged commit a8cff6d into DSpace:master Nov 1, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment