Use unpack to support use of schema plugins as artifacts

[jodygarnett]

Transition from use of maven-resources-plugin:copy-resources (directory based) to maven-dependency-plugin:unpack (artifact based). This is an orthogonal but complemetary change to changing maven repositories to manage artifacts. See proposal Host Geonetwork artifacts on repo.osgeo.org for details.

To try this out:

• https://github.com/jodygarnett/core-geonetwork/tree/artifact_unpack

See proposal Use unpack to support use of schema plugins as artifacts and Bolsena 2020 slides for further discussion and decisions.

Addresses issue: #4724

Reviewers are encouraged to start by reading schemas/README.md and web/pom.xml for context.

Web pom.xml use of copy-resources for copy-schemas execution

The web/pom.xml use of copy-resources can be converted to unpack:

• iso19139
• iso19110
• dublin-core
• csw-record
• iso19115-3.2018 (included from switch to 3.11.0-SNAPSHOT metadata101/iso19115-3.2018#74)
• docs/manuals

This is of the form:

<resource>
  <directory>${project.basedir}/../schemas/iso19139/src/main/plugin</directory>
  <targetPath>${basedir}/src/main/webapp/WEB-INF/data/config/schema_plugins
  </targetPath>
</resource>

You can see this stepping outside the current build directory using a relative path reference.

Feedback

• @josegar74 updated add_schema.sh to generate additional metadata101 profiles as documented in README.md
• use of maven-antrun-plugin to replace adding schema plugins one by one to copy_schemas using add_schema.sh
• Provided a link on using settings.xml to manage activeProfiles (just a link not a recommendation)
• Double check profile id can be the same for same between schemas/pom.xml and web/pom.xml so -Pschema-iso19115-3.2018
• Use - when naming profiles rather than '.' or '_'
• Rename to schemas-copy and schemas-unpack so options appear next to each other in IntelliJ
• Change -Dcopy to -DschemaCopy for consistency with profile names
• Test add-schema.sh with a metadata101 example

Decisions from Bolsena Sprint

This activity was covered by the Bolsena 2020 code sprint, see Bolsena 2020 Online slides for discussion and decisions.

• Decision on version numbering:

  • geonetwork: major/minor/revision format
  • “3.10.4-SNAPSHOT”

• Decision on schema-plugin version:

  • Change: Use geonetwork version above
  • Resolves transitive instability

• Decision on schema-plugin packaging:

  • publish both jar and zip to osgeo repository

• Decision on metadata101/iso19115-3.2018

  • Fold into core-geonetwork

Discussion

Q: @josegar74 asks if this approach can be used while geonetwork is running in jetty?

This approach occurs during the maven process-resources phase and has functionally the same result (updating the same web folder used by jetty).

Q: @josegar74 asks if the HNAP or the dutch schema plugins are still built as submodules?

There is no reason to do so:

• The artifacts are fetched from the local maven repository (if you have a checkout of the HNAP plugin you can build it independently).
• If needed the artifacts are fetched from repo.osgeo.org (as deployed by another developer or jenkins build server).

Q: @fxprunayre notes that at some point we may drop the old way?

The use of -Dcopy saves one step (mvn install) and is worthwhile if we can manage it.

The maintenance of web/pom.xml with add_schema.sh is a lot of overhead; and forces each team to fork or branch core-geonetwork.

To resolve this it may be possible to use maven-antrun-plugin to copy any content in schemas/<plugin>/src/main/plugin to the appropriate location resulting a more maintainable solution that does not require team branch.

Q: @ianwallen notes generally artifacts point to released coded, not work in progress code.

That is a mistake, switching to use of SNAPSHOTs to facilitate development.

Q: @josegar74 and @ianwallen both note the requirement to update add-schema.sh

Sorry for mis-judging the importance of this:

• Jose assures me this is required to assist with working on non metadata101 plugins.
• Ian indicates that forking geonetwork is often recommended or required

Ease of transition using profiles

I had a bit of inspiration from the GeoTools codebase:

• isolating both approaches into individual profiles
• Enable unpack profile by default
• Use a -Dcopy flag to activate copy-resources (while disabling the default unpack)

[jodygarnett]

Thanks for the details review @ianwallen I am going to implement as much as I can.

1. I would like to talk through the use of profiles and see if the steps I have taken allivate the need for you to fork the codebase and use add_schema.sh

2. The interaction with the IntelliJ Maven tool window is something we may need to work out over time.

[jodygarnett]

A quick test with IntelliJ (where my jetty:run configuration calls mvn process-resources first) shows:

Failed to execute goal org.apache.maven.plugins:maven-dependency-plugin:3.1.2:unpack (iso19115-3.2018-resources) on project web-app: Artifact has not been packaged yet. When used on reactor artifact, unpack should be executed after packaging: see MDEP-98.

Solution was to build the project once with mvn install, and then my run configuration works as expected.

aside: Looking at one of the workarounds on MDEP-98 shows exactly our solution: a profile for "maven install" using unpack, and a profile for IDE using "copy resources" (not sure I am impressed with this)

[ianwallen]

Shouldn't the copy resource for schema-iso19115-3.2018 be here? And iso19139.xyz can be removed in this case as iso19139.xyz is not supplied as a sample in all the other locations.

[jodygarnett]

The execution copy-schemas-ant above handles all the contents in the schemas folder.

This example is provided if folks need to copy in something that is not in the schemas folder.

[jodygarnett]

Ian / Jose your changes to add-schema.sh work as expected (thank you).

Ian your change to add-schema.sh fill in this section, while it does no harm it is no longer required (and only useful if you are working with a schema plugin that is not in the schemas folder.

[ianwallen]

I'm approving the changes even though there is a known issue with the iso19115-3.2018 failing on the build because the pull request #66 needs to be approved before this build will work.

[jodygarnett]

It is not a known issue, so much as instruction at the top of the page that both need to be merged at the same time :)

[jodygarnett]

I have updated the build to remove the submodule iso19115-3 and include directly in the PR.

In testing -DschemasCopy=true produces:

-rw-r--r--  1 jgarnett  staff  222200066  8 Jul 12:43 target/geonetwork.war

An using `-DschemasCopy=false produces:

-rw-r--r--  1 jgarnett  staff  222200034  8 Jul 12:43 target/geonetwork.war

May need to diff the results to check for any significant change (beyond build timestamp).

[ianwallen]

Regarding "Remove submodule and Include iso19115-3 directly … 0bdcb07" - how is that related to the work on this PR? - This seems like a lot of changes which should be done in a separate PR.

I have also identified an issue with the add-schema.sh and submitted a PR
#4835. This may cause conflicts with your code however you should be able to simply tack these changes to the end of the file.

[jodygarnett]

Regarding "Remove submodule and Include iso19115-3 directly … 0bdcb07" - how is that related to the work on this PR? - This seems like a lot of changes which should be done in a separate PR.

Yes, I should of been more strict in saying no to scope creep as each reviewer asked for more functionality. This is in part why I made proposal to fix war rather than watch this PR keep being delayed.

I have also identified an issue with the add-schema.sh and submitted a PR
#4835. This may cause conflicts with your code however you should be able to simply tack these changes to the end of the file.

I see, should we try rebasing? Currently github shows no conflicts.

[ianwallen]

I see, should we try rebasing? Currently github shows no conflicts.

My PR #4835 has not been approved yet -- who ever gets merged first may cause the other to get conflicts. Also comment was just to let you know that I had added a new dependency for unit test related to schema plugin and that you should review it as it may affect this PR but I don't think it will.

[jodygarnett]

I see @josegar74 merged a few moments ago and it did indeed produce conflicts, I will try and resolve now. Thanks for the notice @ianwallen

[jodygarnett]

@fxprunayre with Jose on vacation what else do I need to do get this merged? Summer holidays are a good opportunity adjust build system while folks are away

[fxprunayre]

@fxprunayre with Jose on vacation what else do I need to do get this merged? Summer holidays are a good opportunity adjust build system while folks are away

No objection but as it will require a couple of hours/days to adapt (and my list is already quite big for august), I would prefer that someone from geocat take care of this one and provide minimum support if issues encountered after merge. @juanluisrp maybe?
When folks are back, we have to check if this should go to 3.10 or 3.12 and who is willing to maintain those branches.

[jodygarnett]

No objection but as it will require a couple of hours/days to adapt (and my list is already quite big for august), I would prefer that someone from geocat take care of this one and provide minimum support if issues encountered after merge. @juanluisrp maybe?

I will ask @juanluisrp he has also looked at the next proposal after this one.

I was going to ask for commit status to help with care and feeding of the build and help folks adapt as needed. But the last nomination did not go smoothly so I am hanging back to see what is needed...

When folks are back, we have to check if this should go to 3.10 or 3.12 and who is willing to maintain those branches.

I understood from the sprint that you wanted to make a 3.12 for this change. To me it is a non functional change so 3.10 backport would be fine.

[juanluisrp]

I needed to remove schemas/iso19115-3.2008 folder from my local clone before running git pull origin master because it was failing with this error:

From github.com:geonetwork/core-geonetwork
 * branch                  master     -> FETCH_HEAD
 = [up to date]            master     -> upstream/master
error: The following untracked working tree files would be overwritten by merge:
    schemas/iso19115-3.2018/ISO19115-3-2014-to-ISO19115-3-2018.xsl
    schemas/iso19115-3.2018/ISO19139-to-ISO19115-3-2018.xsl

...

Aborting

[fxprunayre]

This looks quite similar to the one above no? Can we get rid of it?

[jodygarnett]

Yes, not sure why it was duplicated.

[fxprunayre]

@jodygarnett, questions you may have hints on.

• when a file is removed from a plugin, it is not removed from the data dir. I tried overWrite true without success. BTW overwrite helps when a file is removed from the datadir (which should not happen often) but when switching from one branch to another, you may end up with the wrong version? Maybe there is a clean first option ?
• We used to be able to do mvn process-resources to update the plugin in the datadir - how do you do know? We have to build the plugin first and then process-resource for unpack or do you have a one step command?

Thanks

[jodygarnett]

removed files:

• I agree it would be smart to run "mvn clean" when switching branches.
• Reading overright rules is interesting but shows no way to have files removed

Correct two steps are required using this approach, but:

• You can still use mvn process-resources -DschemasCopy=true if you want a one step command
• See next proposal[(https://github.com/geonetwork/core-geonetwork/wiki/Resolve-competing-web-module-war-definitions) for a long term solution...

(I corrected the above text to use the correct -D property name)

[fxprunayre]

• You can still use mvn process-resources -PschemaCopy=true if you want a one step command

Looks to not be working - maybe I'm doing something wrong? Ended up using #4946 which is faster.

[jodygarnett]

I have a proposal to make things easier long term for your consideration: https://github.com/geonetwork/core-geonetwork/wiki/Resolve-competing-web-module-war-definitions

Can I ask what "looks not to be working" means, do you have a concrete example to test? The existing setup has schema plugins spliced into the war in multiple locations (and directly watched also). Were you able to see your change copied into the correct location? Was it still not recognized?

[josegar74]

Test case:

• With GeoNetwork running with mvn jetty:run, in the schemas project, change a file, for example: SOURCES/schemas/iso19139/src/main/plugin/iso19139/layout/layout.xsl

Adding for example this following line <xsl:message>TEST</xsl:message>, below:

https://github.com/geonetwork/core-geonetwork/blob/master/schemas/iso19139/src/main/plugin/iso19139/layout/layout.xsl, adding a message in

core-geonetwork/schemas/iso19139/src/main/plugin/iso19139/layout/layout.xsl

Lines 65 to 67 in 5706648

	<xsl:template mode="mode-iso19139" match="gn:child" priority="2000">
	<xsl:param name="schema" select="$schema" required="no"/>
	<xsl:param name="labels" select="$labels" required="no"/>

• From the web module, execute `mvn process-resources -PschemaCopy=true``

• Check the file deployed: SOURCES/web/src/main/webapp/WEB-INF/data/config/schema_plugins/iso19139/layout/layout.xsl, doesn't contain the previous change.

[jodygarnett]

Please use -DschemasCopy=true command line option, you can also run with -X and should be able to see it copy in the logs.

[jodygarnett]

I tested locally by changing the following preservelastmodified="true" verbose="true":

                  <target description="copy from schemas to schema_plugins folder">
                    <copy todir="${schema-plugins.dir}" preservelastmodified="true" verbose="true">
                      <fileset dir="${basedir}/../schemas/">
                        <include name="**/src/main/plugin/**"/>
                      </fileset>
                      <regexpmapper handledirsep="yes"
                        from="^[-_\.a-zA-Z0-9]+/src/main/plugin/(.*)"
                          to="\1" />
                    </copy>

And using mvn process-resources -DschemasCopy=true, it shows:

[INFO] --- maven-antrun-plugin:3.0.0:run (copy-schemas-ant) @ web-app ---
[INFO] Executing tasks
[INFO]      [copy] Copying 1 file to /Users/jgarnett/java/geonetwork/core-geonetwork/web/src/main/webapp/WEB-INF/data/config/schema_plugins
[INFO]      [copy] Copying /Users/jgarnett/java/geonetwork/core-geonetwork/schemas/iso19139/src/main/plugin/iso19139/layout/layout.xsl to /Users/jgarnett/java/geonetwork/core-geonetwork/web/src/main/webapp/WEB-INF/data/config/schema_plugins/iso19139/layout/layout.xsl
[INFO] Executed tasks

Thanks for the test case @josegar74, is it copying into the correct location?

[jodygarnett]

The documentation appears correct in web/README.md, I am sorry it appears I had the -D example incorrect in this thread earlier and confused everyone. I am much more focused on making sure the docs are correct.

[jodygarnett]

@josegar74 I like the use of preservelastmodified="true" above, perhaps you can add that to master, it uses the original last modified time for the copied files making it easier to confirm exactly what is being run.

[jodygarnett]

Here is a small PR for that #4948

[jodygarnett]

I needed to remove schemas/iso19115-3.2008 folder from my local clone before running git pull origin master because it was failing with this error:

From github.com:geonetwork/core-geonetwork
 * branch                  master     -> FETCH_HEAD
 = [up to date]            master     -> upstream/master
error: The following untracked working tree files would be overwritten by merge:

I sent instructions to the email list to resolve:

cd schemas
rm -r iso19115-3.2018
git pull
git checkout .