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

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

Merged
merged 1 commit into from
Jul 19, 2024

Conversation

stweil
Copy link
Member

@stweil stweil commented Jul 17, 2024

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
Copy link
Member Author

stweil commented Jul 17, 2024

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

@stweil stweil requested a review from solth July 17, 2024 18:16
This makes it easier to update the version and allows running Maven
for example like this: `mvn -Drevision=3.7.0-test clean install`.

Signed-off-by: Stefan Weil <[email protected]>
@solth solth merged commit 144b93f into kitodo:master Jul 19, 2024
5 checks passed
@stweil stweil deleted the revision branch July 19, 2024 11:03
@henning-gerhardt
Copy link
Collaborator

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
Copy link
Member Author

stweil commented Jul 30, 2024

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
Copy link
Collaborator

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
Copy link
Member Author

stweil commented Jul 30, 2024

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

@henning-gerhardt
Copy link
Collaborator

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
Copy link
Member Author

stweil commented Jul 30, 2024

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
Copy link
Collaborator

henning-gerhardt commented Jul 30, 2024

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
Copy link
Collaborator

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
Copy link
Member Author

stweil commented Jul 30, 2024

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
Copy link
Collaborator

henning-gerhardt commented Jul 30, 2024

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
Copy link
Member Author

stweil commented Jul 30, 2024

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
Copy link
Collaborator

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.

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