kitodo / kitodo-production

Kitodo.Production is a workflow management tool for mass digitization and is part of the Kitodo Digital Library Suite.
http://www.kitodo.org/software/kitodoproduction/
GNU General Public License v3.0
63 stars 63 forks source link

Use special property `revision` to set the version only once #6127

Closed stweil closed 2 months ago

stweil commented 2 months ago

This makes it easier to update the version and allows running Maven for example like this: mvn -Drevision=3.7.0-test clean install.

stweil commented 2 months ago

See related documentation: https://maven.apache.org/maven-ci-friendly.html.

henning-gerhardt commented 2 months ago

I looked into this changes now after I discovered some issues with it (see #6153):

Using the revision may work in general but the stored pom files inside the local .m2 directory of every Kitodo.Production module looks wrong.

For version 3.6.3 the pom file for the Kitodo API module contain an entry like this:

    <parent>
        <artifactId>kitodo-production</artifactId>
        <groupId>org.kitodo</groupId>
        <version>3.6.3</version>
    </parent>

If you run a normal mvn clean installon the current master branch with this changes the file to

    <parent>
        <artifactId>kitodo-production</artifactId>
        <groupId>org.kitodo</groupId>
        <version>${revision}</version>
    </parent>

Even running the command with mvn -Drevision=3.7.0-SNAPSHOT clean install is not replacing the placeholder ${revision}.

As the version in the pom file is not defined or better defined as ${revision} every build now try to get the build artifact with an non existing or not valid version information.

stweil commented 2 months ago

On macOS, I don't have local .m2 directories, but a single one $HOME/.m2.

The file .m2/repository/org/kitodo/kitodo-api/3.7.0-SNAPSHOT/kitodo-api-3.7.0-SNAPSHOT.pom has <version>${revision}</version>. This is resolved by the parent pom .m2/repository/org/kitodo/kitodo-production/3.7.0-SNAPSHOT/kitodo-production-3.7.0-SNAPSHOT.pom which has the necessary property <revision>3.7.0-SNAPSHOT</revision>.

I run all mvn commands from the source root directory. For some plugins (for example flyway) this requires an entry in the root pom.xml. Supporting mvn from root and from module directories at the same time is difficult (maybe even impossible) because the relative paths to configuration files are different (with or without "..").

henning-gerhardt commented 2 months ago

On macOS, I don't have local .m2 directories, but a single one $HOME/.m2.

I did this mean as I wrote that the files inside the local .m2 directory. I have even only a single .m2 directory and so far as I know no one has a .m2 directory inside the git checkout.

As not only in this pull request even on others: which version of mvn did you have on macOS? I think that is maybe related to this kind of issues.

stweil commented 2 months ago

I have Maven 3.9.8 on macOS and 3.8.7 on Linux (default versions from Homebrew / Debian).

henning-gerhardt commented 2 months ago

Good to know, I'm running

$ mvn --version
Apache Maven 3.8.7
Maven home: /usr/share/maven
Java version: 11.0.20, vendor: Debian, runtime: /usr/lib/jvm/java-11-openjdk-amd64
Default locale: en_US, platform encoding: UTF-8
OS name: "linux", version: "6.1.0-23-amd64", arch: "amd64", family: "unix"

So the issues may come from the newer maven version on MacOS or they are not related to the maven version itself.

I can only write that this changes makes me more trouble than other changes since a long time.

stweil commented 2 months ago

I try to help and think that there is an easy solution if all mvn commands are run from the root. Would this be an acceptable solution (as I wrote above mixing support for root and for subdirectories might always cause problems)?

henning-gerhardt commented 2 months ago

I try to help and think that there is an easy solution if all mvn commands are run from the root. Would this be an acceptable solution (as I wrote above mixing support for root and for subdirectories might always cause problems)?

If this solution works for everyone on every current used operating system and in all usage cases why not. // Edit: this solution must work inside with different IDEs (IDEA, Eclipse, ...) too and not only on console.

henning-gerhardt commented 2 months ago

In all generated pom files with this changes they contain

    <parent>
        <artifactId>kitodo-production</artifactId>
        <groupId>org.kitodo</groupId>
        <version>${revision}</version>
    </parent>

If I look into the ~/.m2/repository/org/kitodo/kitodo-production directory I see something like this

$ ls -1
'${revision}'
3.6.0
3.6.0-SNAPSHOT
3.6.2
3.6.3
3.7.0-SNAPSHOT
maven-metadata-local.xml

The '${revision}' subdirectory contains itself only

$ ls -1
'kitodo-production-${revision}.pom.lastUpdated'

The content subdirectory '${revision}' did not look right or at least it is missing important files.

The subdirectory 3.7.0-SNAPSHOT contains a kitodo-production-3.7.0-SNAPSHOT.pom file with content

    <groupId>org.kitodo</groupId>
    <artifactId>kitodo-production</artifactId>
    <version>${revision}</version>
    <packaging>pom</packaging>

Even in this file the placeholder ${revision} is not replaced - which is in my opinion wrong.

stweil commented 2 months ago

I have similar files / content (with less old revisions), so this seems to be okay.

I now tried these commands on Linux based on my updated PR #6133 with config-local/kitodo_config.properties for the default development setup and a newly created kitodo database (like in the CI GitHub action), everything started from the source root directory:

# Removing anything from previous builds.
rm -r ~/.m2
git clean -fxd Kitodo*
# Create database.
sudo mariadb -e 'DROP DATABASE IF EXISTS kitodo;'
sudo mariadb -e 'CREATE DATABASE kitodo;'
sudo mariadb -e "CREATE USER IF NOT EXISTS 'kitodo'@'localhost' IDENTIFIED BY 'kitodo';"
sudo mariadb -e "GRANT ALL ON kitodo.* TO 'kitodo'@'localhost';"
sudo mariadb kitodo < Kitodo/setup/schema.sql
sudo mariadb kitodo < Kitodo/setup/default.sql
# Run initial build and test several commands.
mvn clean install
mvn flyway:baseline
mvn flyway:validate
mvn flyway:migrate

Everything seems to work fine.

henning-gerhardt commented 2 months ago

I have similar files / content (with less old revisions), so this seems to be okay.

This is not okay, this are wrong created files which I never see in any other repository of other projects. May this is working with newer versions of Maven tool it is breaking older versions in their usage.

Everything seems to work fine.

This may the case as on your system was never any issue with your changes.

stweil commented 2 months ago

The content subdirectory '${revision}' did not look right or at least it is missing important files.

I get this directory only when I run mvn commands from a module directory, but not if I run such commands only from the root directory. So you are correct, those files are not okay, but they can be avoided. Just don't run mvn from a module directory. Use #6133 and run from the root.

henning-gerhardt commented 2 months ago

The content subdirectory '${revision}' did not look right or at least it is missing important files.

I get this directory only when I run mvn commands from a module directory, but not if I run such commands only from the root directory. So you are correct, those files are not okay, but they can be avoided. Just don't run mvn from a module directory. Use #6133 and run from the root.

I'm not running any build / install mvn command from a sub directory. Why are you deny / forbid to run any mvn command from a sub directory? If this is not working then the pom configuration is wrong!

I'm can not run #6133 from root source directory as my current installation is broken through this changes here!

@solth: Please revert this changes and accept them back if they are not causing any trouble for any one on any system.