Add workflow for automated version increments in pull-requests #2352

HannesWell · 2024-09-16T22:55:57Z

If an Eclipse Plugin project is changed the first time in a development cycle build failures due to missing version bumps are very common, for example like in the following case:

 [ERROR] Failed to execute goal org.eclipse.tycho.extras:tycho-p2-extras-plugin:4.0.8:compare-version-with-baselines (compare-attached-artifacts-with-release) on project org.eclipse.pde.api.tools.tests:
Only qualifier changed for (org.eclipse.pde.api.tools.tests/1.3.600.v20240905-2015). Expected to have bigger x.y.z than what is available in baseline (1.3.600.v20240806-1811) -> [Help 1]

This PR adds a workflow to automatically detect the need for such service version increments whenever a PR is created or updated, using a fast running Maven build that invokes the tycho-p2-extras-plugin:compare-version-with-baselines goal. The required version bumps are then applied by invoking Tycho's tycho-versions-plugin:bump-versions goal. The result is committed with the expected message and pushed to the PR's branch using a configurable Eclipse bot account.
Furthermore the configured bot also adds a comment to the affected PR to inform about what happened, to list the files that were changed and to provide a git-patch of the change.

In order to make this possible a personal access token] (PAT) with scope public_repo:

With this all set up and running, one does not have to apply version bumps due to qualifier differences anymore and we also don't have to explain it to new users because everything just happens automatically.

An example of the current state can be seen in my PDE fork where I configured my own main account to act as bot-account:

Some first change in a release-cycle from a fork HannesWell/eclipse.pde#3

This is the major implementation part of https://gitlab.eclipse.org/eclipse-wg/ide-wg/community/-/issues/56. The other parts are to set up the repositories where the new workflows should be called to use the described functionality.
I'll start with the PDE repository and also use it as a verification that this really works. Afterwards I'll set this up for all repositories that are interested.

The next steps are to ask the EF infra team to set up the corresponding tokens described above.

HannesWell · 2024-09-16T23:04:36Z

eclipse-platform-parent/pom.xml

+          <plugin>
+            <groupId>org.apache.maven.plugins</groupId>
+            <artifactId>maven-antrun-plugin</artifactId>
+            <executions>
+              <execution>
+                <id>resolve-version-qualifier</id>
+                <goals>
+                  <goal>run</goal>
+                </goals>
+                <phase>generate-sources</phase>
+                <configuration>
+                  <exportAntProperties>true</exportAntProperties>
+                  <target>
+                    <!-- See https://stackoverflow.com/a/53227117 and http://ant-contrib.sourceforge.net/tasks/tasks/index.html -->
+                    <taskdef resource="net/sf/antcontrib/antlib.xml"/>
+                    <if>
+                      <or>
+                        <equals arg1="${project.packaging}" arg2="eclipse-plugin"/>
+                        <equals arg1="${project.packaging}" arg2="eclipse-test-plugin"/>
+                      </or>
+                      <then>
+                        <copy file="META-INF/MANIFEST.MF" tofile="${project.build.directory}/MANIFEST.MF"/>
+                        <manifest file="${project.build.directory}/MANIFEST.MF" mode="update">
+                          <attribute name="Bundle-Version" value="${qualifiedVersion}"/>
+                        </manifest>
+                        <property name="version.check.skip" value="false"/>
+                      </then>
+                      <elseif>
+                        <equals arg1="${project.packaging}" arg2="eclipse-feature"/>
+                        <then>
+                          <copy file="feature.xml" tofile="${project.build.directory}/feature.xml"/>
+                          <replace file="${project.build.directory}/feature.xml" 
+                            token='version="${unqualifiedVersion}.qualifier"' value='version="${qualifiedVersion}"'/>
+                          <manifest file="${project.build.directory}/MANIFEST.MF"/>
+                          <property name="version.check.skip" value="false"/>
+                        </then>
+                      </elseif>
+                      <else>
+                        <property name="version.check.skip" value="true"/>
+                        <manifest file="${project.build.directory}/MANIFEST.MF"/>
+                        <echo message="Configure to skip version checks"/>
+                      </else>
+                    </if>
+                  </target>
+                </configuration>
+              </execution>
+            </executions>
+            <dependencies>
+              <dependency>
+                <groupId>ant-contrib</groupId>
+                <artifactId>ant-contrib</artifactId>
+                <version>1.0b3</version>
+                <exclusions>
+                  <exclusion>
+                    <groupId>ant</groupId>
+                    <artifactId>ant</artifactId>
+                  </exclusion>
+                </exclusions>
+              </dependency>
+            </dependencies>
+          </plugin>
+          <plugin>
+            <groupId>org.apache.maven.plugins</groupId>
+            <artifactId>maven-jar-plugin</artifactId>
+            <executions>
+              <execution>
+                <id>pack-pseudo-jar</id>
+                <goals>
+                  <goal>jar</goal>
+                </goals>
+                <phase>generate-sources</phase>
+                <configuration>
+                  <classesDirectory>${project.build.directory}</classesDirectory>
+                  <includes>
+                    <include>feature.xml</include>
+                    <include>MANIFEST.MF</include>
+                  </includes>
+                  <archive>
+                    <manifestFile>${project.build.directory}/MANIFEST.MF</manifestFile>
+                  </archive>
+                </configuration>
+              </execution>
+            </executions>
+          </plugin>


When eclipse-tycho/tycho#4279 and eclipse-tycho/tycho#4280 respectively their back-ports are available this can be much simpler and specially can work without explicit file manipulations.

What is the benefit of packing a "pseudo-jar" here? Usually skipping tests (and maybe using multiple threads) makes the packing fast enough, trying to replicate the packing to safe a few seconds does not seems very useful given that we perform non trivial task there.

It was just a work-around because the two tycho changes mentioned above were not yet available.
Now it's just an invocation of two tycho-plugins configured to do the minimally necessary, plus an antrun invocation to set a few properties based on the project's packaging type.

Of course it would be even very nice if just the tycho-p2-extras-plugin:compare-version-with-baselines goal could be invoked without a preceding tycho-p2-plugin and tycho-packaging-plugin, but that seemed to be more work (although it was also more work than expected to strip that all down).

Usually skipping tests (and maybe using multiple threads) makes the packing fast enough, trying to replicate the packing to safe a few seconds does not seems very useful given that we perform non trivial task there.

This not only saves a few seconds. With the current set up a build takes in the PDE example about 45s, while a full build even if executed in parallel and without tests takes about 5min (on my machine). If it is considers that in case of required version bumps at least one more build is required this is already a difference of two vs. 10min response time.
In the PDE example presented above, even three build runs are necessary so the response time that's currently between three/four minutes (the workflow has an overhead) would go up to ~15min. Which is also added to the overall build time.
And I think the response time should be fast.

If this does not work anymore we can still go back to mvn clean verify -DskipTests -T 1C -Dcompare-version-with-baselines.skip=false or make tycho-p2-extras-plugin:compare-version-with-baselines work with only a qualifier being computed before.

HannesWell · 2024-09-16T23:07:46Z

.github/workflows/publishVersionCheckResults.yml

+# TODO: Add an FAQ about version bumps?
+# - Why are the version bumps necessary at all?
+#   According to the rules of semantic versioning at least the service version of a bundle should be increased if anything changes between releases.
+# - Why is the service version bumped by +100
+#   Allows service version increments on maintanance branches that stay semnatically smaller than the next releases version
+# - Why a separate commit?
+#   Easier to identify in Git history and the actual change is easier to revert (version must not be reverted)


I intend to add an folded FAQ to the comment added by the bot so that especially newcommers can understand better why these version bumps are necessary and why they are done this way.

The list is currently a brief summary, but if you think questions are missing, please let me know.

As discussed in #2352 (comment) ff the comment added by this workflow now just links to a new section in the wiki that I have just created:
https://github.com/eclipse-platform/eclipse.platform.releng.aggregator/wiki/Common-Build-Issues#missing-version-increments

HannesWell · 2024-09-16T23:11:17Z

.github/workflows/publishVersionCheckResults.yml

+#   Allows service version increments on maintanance branches that stay semnatically smaller than the next releases version
+# - Why a separate commit?
+#   Easier to identify in Git history and the actual change is easier to revert (version must not be reverted)
+# TODO: make comment updatable?


I'm contemplating to make the comment added by the bot updatable, to avoid new comments being added after each (force-)push, if the version bump isn't fetched and pushed as well.
But this also means that updates are less visible.
It can for example also happen that due to an update no version bumps are necessary anymore (but the workflow doesn't do anything in this case anyways).

What's your preference?

I think it must not be configurable, but most confess it a bit unfortunate that we optimize for backports (happen very seldom) and reverts (should actually never happen), anyways a link to the FAQ would be suitable enough for me we don't need a summary replicated on each PR.

Was this intended for #2352 (comment)?

But sure providing the FAQ in the wiki and linking it is of course also a good solution. It would be folded, just like the git-patch, but with a link one can always read the latest version.
I can implement it that way.

but most confess it a bit unfortunate that we optimize for backports (happen very seldom) and reverts (should actually never happen)

I wouldn't say we optimize it, but with the workflow it also just technically simpler. And just as a side-note: since these things happen seldomly it's actually better to have less things to consider because otherwise mistakes are more likely.

.github/workflows/prepareRelease.yml

.github/workflows/checkVersions.yml

laeubi · 2024-09-17T04:58:13Z

Instead of having an own build I think one should adopt to what other workflows do (e.g. the junit-publishing workflow), where they work on the result of a previous workflow. e.g if one looks here:

there is an upload event file step already that contains all the meta-data of the pull request.

Later on, unit test results are uploaded:

now the Publish test results workflow can read them and act upon these.

The only thing needed is then that we probably upload additional information e.g required version bumps can be written to a file in whatever format suitable, maybe even as a patch file by Tycho and don't need to trigger additional builds at all and we make sure we always reuse what Tycho already offers, this could also probably require less permissions and make the workflow easier to understand.

.github/workflows/publishVersionCheckResults.yml

.github/workflows/updateRelease.yml

laeubi · 2024-09-17T05:10:00Z

eclipse-platform-parent/pom.xml

@@ -768,33 +780,6 @@
       </repository>
     </repositories>
   </profile>
-   <profile>


Why is this removed?

With this new workflow to apply and push required version increments automatically to a PR, this seems not to be necessary anymore since the new workflow will handle this.
So removing this and the version-bump part in the updateRelease.yml reduce amount of configuration and avoids that duplicated configuration goes out of sync (e.g. if we want other version-bump deltas in the future).

But I moved all removals to separate commit in this PR to make this more clear.

As one can use the VB profile also locally I don't thing it is really redundant

That's right. But is this really used (by many)?

Anyway I moved this change to #2387, so we can later discuss there if it should be removed or retained.

HannesWell · 2024-09-18T00:12:53Z

Instead of having an own build I think one should adopt to what other workflows do (e.g. the junit-publishing workflow), where they work on the result of a previous workflow. e.g if one looks here:
[...]
The only thing needed is then that we probably upload additional information e.g required version bumps can be written to a file in whatever format suitable, maybe even as a patch file by Tycho and don't need to trigger additional builds at all and we make sure we always reuse what Tycho already offers, this could also probably require less permissions and make the workflow easier to understand.

First of all I started out simple and wanted to keep it simple, but unfortunately it turned out that if you want to push to a PR-branch in a fork from a GitHub workflow, things become complicated. I tried out a lot and tried to keep it as simple as possible.
The main problems are that first the GITHUB_TOKEN has no permission to push to forks, even if I grant write-all permissions, so a PAT is necessary for pushing. And second, workflows triggered by the pull_request event from a fork of a non-committer (i.e. an account without write permissions) don't have access to the secret store of a repo.
Therefore a second workflow is necessary that runs upon completion of the pull_request even triggered workflow, similar to the workflow to publish test results (although it's a bit different there because that workflow only adds a comment, but that's also not possible for a workflow triggered by a PR from a fork).

It would be possible to have the version bumps applied as part of the regular verification builds, but that would only save us one extra git-checkout step plus the configuration of a JDK and Maven as well as that we have a separate file. But all other steps and inputs etc. done in the first workflow still would have to be done and the second workflow would still be necessary at it is.
This moderate simplification of the configuration would come with the draw-back that each iteration would take the full time of the verification build. In the PDE example this would mean ~30min per iteration instead of 45s. In the linked example this would mean a total wait-time of roughly 1.5h until the PR is completely built instead of ~3-4min extra time (see #2352 (comment)).
For me this justifies the current setup.

Having separated builds also helps to separate concerns and avoids that we collect everything in the mavenBuild.yml.

HannesWell · 2024-09-21T14:52:21Z

I updated the workflow to implement the missing TODOs, which besides others, include the addition of a link to a new wiki entry about version-bumps and making the workflow update the comment it added in a previous run, instead of adding a new comment for each run (if e.g. a new change is pushed but the auther overwriting/reverting the applied version bump).

I have also done further testing with

Some first change in a release-cycle from a fork HannesWell/eclipse.pde#3

and everything looks perfectly well.
I also hope that all concerns were addressed.

With that for me this workflow is ready for submission.

To really use it (e.g. like set up in eclipse-pde/eclipse.pde#1408), we have to await the addition of the corresponding tokens by the EF infra team (https://gitlab.eclipse.org/eclipsefdn/helpdesk/-/issues/5020).

jukzi · 2024-09-21T21:44:55Z

I like the idea, but i have doubts about the benefit of this change as long as the API tooling is flaky eclipse-pde/eclipse.pde#1193.
Recent example:
https://github.com/eclipse-platform/eclipse.platform/actions/runs/10961494829/job/30438556401?pr=1562
false positive:

IPasswordStore has been removed from org.eclipse.jsch.core_1.5.500
META-INF/MANIFEST.MF:5 The major version should be incremented in version 1.5.500, since API breakage occurred since version 1.5.500

As far as i understand this workflow would just add a wrong version bump that would have to be manually reverted.

merks · 2024-09-22T06:34:59Z

I would hope the tool is configurable enough that it wouldn't suggest major increments because we basically never to do that the platform's projects, and the IDE would have complained already. I expect the +100 bumps are the things the IDE (API tools) never notice...

HannesWell · 2024-09-22T07:53:11Z

This workflow does not use API-Tools at all, it just uses the tycho-p2-extras:compare-version-with-baselines goal and therefore just detects if anything has changed compared to the baseline (should always be the latest release) and it only suggests the +100 service/micro version-segment bump.
This works quite reliably and I have never seen a false positive in that.

It would be possible to automatically perform the version increments required by API-tooling too, in fact Tycho already supports this and it would just have to be enabled. But as you said API-Tools are not as reliable and it would also increase the runtime of the check significantly.
But first of all I think version increments due to API changes should be a decision made deliberately and in full awareness plus it's already checked and indicated in the IDE. On the other hand, the service-version increments are something that has to be done on any first change in a release cycle and something one does not really get around and the IDE doesn't check that (yet).

HannesWell · 2024-09-24T22:12:45Z

After some more testing with other repos of the SDK revealed that the extra profile for an expedited version check does not work on all SDK projects I have now changed the workflow to just execute an ordinary verification build with tests execution disabled (and no extra api-checks etc. enabled) that runs in parallel.
I will later work on a Tycho mojo geared for this use-case that should make the workflow faster.

With that this is now ready for submission since also the infrastructure of PDE is ready for a first usage and all final testing looks good.
Thank you all for the discussion.

Set up for this repository the workflow for automated version increments in pull-requests added via eclipse-platform/eclipse.platform.releng.aggregator#2352

iloveeclipse · 2024-09-25T18:04:28Z

@HannesWell : also please for JDT.

HannesWell · 2024-09-25T18:10:50Z

Great!

Just created for Platform:

Set up workflow for automated version increments in pull-requests eclipse.platform#1568
Set up workflow for automated version increments in pull-requests eclipse.platform.ui#2326
Set up workflow for automated version increments in pull-requests eclipse.platform.swt#1491

an for Equinox:

Set up workflow for automated version increments in pull-requests eclipse-equinox/equinox#678
Set up workflow for automated version increments in pull-requests eclipse-equinox/p2#549

We have to await the addition of the PATs which was requested in https://gitlab.eclipse.org/eclipsefdn/helpdesk/-/issues/5020#note_2825941 ff.

@HannesWell : also please for JDT.

Yes. Just requested the PAT as well and will follow-up with corresponding PRs for JDT tomorrow.

Set up for this repository the workflow for automated version increments in pull-requests added via eclipse-platform/eclipse.platform.releng.aggregator#2352

HannesWell · 2024-09-26T20:29:56Z

@HannesWell : also please for JDT.

@iloveeclipse please see the following PRs for JDT:

Set up workflow for automated version increments in pull-requests eclipse-jdt/eclipse.jdt.core#3021
Set up workflow for automated version increments in pull-requests eclipse-jdt/eclipse.jdt.debug#516
Set up workflow for automated version increments in pull-requests eclipse-jdt/eclipse.jdt.ui#1680

Set up for this repository the workflow for automated version increments in pull-requests added via eclipse-platform/eclipse.platform.releng.aggregator#2352

HannesWell changed the title ~~Add workflow for automated version increments for pull-requests~~ Add workflow for automated version increments in pull-requests Sep 16, 2024

HannesWell mentioned this pull request Sep 16, 2024

Set up workflow for automated version increments in pull-requests eclipse-pde/eclipse.pde#1408

Merged

HannesWell commented Sep 16, 2024

View reviewed changes