Single source of truth for the DiffDetective version #135
Conversation
default.nix
Outdated
| pname = "DiffDetective"; | ||
| version = "2.1.0"; | ||
| version = pkgs.lib.removeSuffix "\n" (pkgs.lib.readFile (pkgs.runCommandLocal "DiffDetective-version" {} "${pkgs.xq-xml}/bin/xq -x '/project/version' ${./pom.xml} > $out")); |
There was a problem hiding this comment.
Maybe add a comment here what this complicated expression does?
| @@ -32,7 +32,7 @@ WORKDIR /home/sherlock | |||
| # Copy the compiled JAR file from the first stage into the second stage | |||
| # Syntax: COPY --from=STAGE_ID SOURCE_PATH TARGET_PATH | |||
| WORKDIR /home/sherlock/holmes | |||
| COPY --from=0 /home/user/target/diffdetective-2.1.0-jar-with-dependencies.jar ./DiffDetective.jar | |||
| COPY --from=0 /home/user/target/diffdetective-*-jar-with-dependencies.jar ./DiffDetective.jar | |||
There was a problem hiding this comment.
I wonder if simplifying our workspace is worth introducing potential bugs here. As you mentioned, this will break in case there are multiple built versions, right? Maybe we could have two (instead of one) version files: the pom.xml, and a version.txt (or so) which is a file that contains just the version number as plain text. We can then use this version file in all the shell scripts. Just an idea.
There was a problem hiding this comment.
The Dockerfiles are not at risk for this problem because they build DiffDetective with a fresh target folder hence there will never be a second jar. The .sh scripts I changed are problematic.
An alternative would be to create a script which "parses" the pom.xml and outputs the version. Then we could use that script in the other scripts. Note that this would be a very lax parser because I want to avoid introducing a dependency on something like xq for developer machines.
# extracts the first version tag in pom.xml
sed -n '/<version/ {s/.*version>\(.*\)<\/version.*/\1/; p; q}' pom.xmlAdding a second "version" file kind of defeats the purpose in my opinion.
I also tested the Dockerfile right now to verify my claim: It doesn't actually build! The most relevant errors show that there is some Java version mismatch:
54.79 [ERROR] /home/user/src/main/java/org/variantsync/diffdetective/variation/diff/Projection.java:[10,39] cannot access org.variantsync.functjonal.list.FilteredMappedListView
54.79 [ERROR] bad class file: /root/.m2/repository/org/variantsync/functjonal/1.0-SNAPSHOT/functjonal-1.0-SNAPSHOT.jar(/org/variantsync/functjonal/list/FilteredMappedListView.class)
54.79 [ERROR] class file has wrong version 63.0, should be 61.0
54.79 [ERROR] Please remove or make sure it appears in the correct subdirectory of the classpath.
The last Functjonal update was in 3ceefb4. What motivated this commit?
There was a problem hiding this comment.
I guess the most important thing here is to end with a robust solution that does not easily break. If we have to overcomplicate things and make everything more fragile, I am fine with just having a bit of extra work when incrementing the version number: put the burden on the developer, not the user.
Apart from that, your sed solution looks good. It should be robust enough if reused within the other scripts.
It's bad news that the Docker file breaks. We should fix this asap on main because that is what FSE reviewers see. Could you have a look at it? Should we have the docker file build functjonal by itself? It seems I messed up with rebuilding the new version of functjonal. It is probably enough to build functjonal anew with an older version of Java. (The motivation for 3ceefb4 was exactly to fix this error. Previously, I had rebuild functjonal to introduce some new feature I needed somewhere but the jar was build with an even newer version.)
There was a problem hiding this comment.
I could repeat this error with the Dockerfile in replication/esecfse22.
ibbem
force-pushed
the
single-source-of-truth-version
branch
from
a032d73 to
b8b6bd0
Compare
ibbem
force-pushed
the
single-source-of-truth-version
branch
from
b8b6bd0 to
ba621c5
Compare
As discussed in #134, we want to be able to edit the version number of DiffDetective in one single place: the
pom.xml. There is no alternative to usingpom.xml(e.g., usingdefault.nix) becausepom.xmldoesn't support inclusion of other files (except XML files). Fortunately, the shell and Nix are both flexible enough to support this (although it's uncommon to parse XML in Nix, hence this complicated expression).However, the shell scripts are more brittle now because they will fail if multiple versions of DiffDetective jars exist. Do we even need these scripts anymore? If yes, it would make sense to give them a little love (e.g., they always print DONE, even in case of errors, etc.).
The only place where the version number is still duplicated is the
README.md. However, there is a note where to find the actual current version so this should be fine.