icu icon indicating copy to clipboard operation
icu copied to clipboard

ICU-21691 add semantic versioning support for OSGI

Open buuhuu opened this issue 4 years ago • 5 comments

This pull request introduces semantic versioning to icu4j and replaces the previously hard coded Export-Package headers of the icu4j.jar MANIFEST.MF.

A compile time dependency to org.osgi:osgi.annotations:8.0.0 was introduced and package-info.java files with the according @Version annotations were added to already exported packages. I chose version 1.0.0 for all of them, succeeding the currently released, implicit version 0.0.0.

The build process was extended with bnd tools, where the bnd command replaces the former jar ant task. This command uses the new icu4j.bnd file to package the bundle (jar) and generate the manifest. The icu4j.bnd file is basically the former manifest.stub with some bnd specific refactorings/instructions added.

Last, after the bundle (jar) was built, a baseline check gets executed, comparing the the current build against the latest release of icu4j. This is implemented in the build-tools subproject as the bnd tools baseline command does not allow to fail the build. When the baseline check fails, a report is generated that gives information about which API change requires a package version change and forces the user to update the package version along any breaking changes. Such a report will look similar to

     [java] Baseline mismatch for package com.ibm.icu.lang, MINOR change. Current is 1.0.0, repo is 1.0.0, suggest 1.1.0 or -
     [java] MINOR                PACKAGE    com.ibm.icu.lang
     [java]   MINOR              CLASS      com.ibm.icu.lang.CharSequences
     [java]     ADDED            METHOD     foobar()
     [java]       ADDED          ACCESS     static
     [java]       ADDED          RETURN     int
     [java] The baselining check failed when checking icu4j.jar against lib/icu4j-latest.jar.
     [java] BaselineCheck failed

Unlikely but also possible is an insufficient increment of the impl version, for example when the jar content or its manifest changes substantially. For this the detailed report will not be provided (simply because it may be huge). However a suggestion for the correct version will be made.

Dealing with API drafts

According to ICU API compatibility new APIs are introduced as drafts and are subject to change until they become stable. This may interfere with the baseline check and would cause many major package version increments when many binary breaking changes are introduced in the draft-lifetime of an API. To prevent this draft APIs should be marked BaselineIgnore from now on, until the API gets promoted to stable.

  • [x] Update the documentation accordingly
Breaking(?) Changes

So far icu4j had no dependencies. With this PR the dependency to org.osgi:osgi.annotations was introduced. These annotations all have a RetentionPolicy.CLASS set. However it introduces a dependency of the default task jar to ivy in order to retrieve the compile time dependency/-ies. This may be considered a breaking change as it requires the build instance to have an internet connection, which was so far only necessary to run tests.

  • [X] For CI, check if it makes sense to cache the ivy cache to improve build performance.

Apparently it only is a manner of seconds.

2021-07-29T08:39:04.1626124Z retrieve-dependencies:
2021-07-29T08:39:**04.2947160Z** [ivy:retrieve] :: Apache Ivy 2.5.0 - 20191020104435 :: https://ant.apache.org/ivy/ ::
2021-07-29T08:39:04.3029443Z [ivy:retrieve] :: loading settings :: url = jar:file:/home/vsts/.ant/lib/ivy.jar!/org/apache/ivy/core/settings/ivysettings.xml
2021-07-29T08:39:**19.4213678Z** 
2021-07-29T08:39:19.4214551Z init:
Transition

The change will not interfere with the next release in any way. This is, because the exported version of the packages of the last icu4j release is 0.0.0 which is treated as development version by bnd. Only once a release with a version range [1,) was released the baseline check will start reporting violations to semantic versioning.

Checklist
  • [X] Required: Issue filed: https://unicode-org.atlassian.net/browse/ICU-21691
  • [X] Required: The PR title must be prefixed with a JIRA Issue number.
  • [X] Required: The PR description must include the link to the Jira Issue, for example by completing the URL in the first checklist item
  • [X] Required: Each commit message must be prefixed with a JIRA Issue number.
  • [ ] Issue accepted (done by Technical Committee after discussion)
  • [ ] Tests included, if applicable
  • [x] API docs and/or User Guide docs changed or added, if applicable

buuhuu avatar Jul 28 '21 20:07 buuhuu

Hooray! The files in the branch are the same across the force-push. 😃

~ Your Friendly Jira-GitHub PR Checker Bot

Hooray! The files in the branch are the same across the force-push. 😃

~ Your Friendly Jira-GitHub PR Checker Bot

@Buuhuu Thank you for this PR. I read through your explanation and I understand the goals. This enhancement sounds good in general. I have one minor concern about package versioning. You suggested to start with 1.0.0 using the current ICU code as the baseline, and increment each package version when we add/remove/change APIs. Is my understanding correct? Practically, what is the benefit of managing API level at package level? In other words, if package export version uses the ICU4J API version (70 for next release) for all packages, I understand it might create redundant version numbers, but are there any issues with this. My concern is mostly extra task for release, and confusion which might come from these version numbers.

yumaoka avatar Sep 17 '21 20:09 yumaoka

Thanks @yumaoka for your feedback and please apologise my late reply (I have been on vacation last week).

Your understanding is correct. For a detailed explanation about semantic versioning in OSGI please have a look at [1].

The biggest advantage of semantic versioning comes for API consumers, which are implementations that use but do not implement the APIs from the icu4j bundle. As long as the (stable) APIs - lets say in com.ibm.icu.text - do not change in an incompatible way, the major version of the exported package stays the same and matches the consumers import range. That means the consumer can use an updated bundle version with minor changes to the exported package at runtime and does not need to be rebuild against this version.

Versioning all exported packages individually makes sense to reduce the probability of an incompatible change that would require a major package version update. That means that if com.ibm.icu.text gets a major version update due to an incompatible change, consumers of com.ibm.icu.math are not affected by that and don't need to be rebuild.

I am not an expert in the development process of icu4j but I think you don't have to expect any issues/additional steps with the release. As said in the initial description, the baseline check guarantees the build passes only when the exported packages are exported in semantical correct versions which requires to update the versions already with a PR. Additionally the baseline check makes a recommendation which version to choose. However, I understand that drafting APIs in icu4j is an established process which may interfere with semantic versioning and so should be ignored. Managing the @BaselineIgnore annotations is indeed additional effort, but again not related to the release process as far as I understand it.

I will add an update to the documentation to the PR as I just realised it is part of the same repo.

  1. https://docs.osgi.org/whitepaper/semantic-versioning/Semantic-Versioning-20190110.pdf

buuhuu avatar Sep 28 '21 08:09 buuhuu

I updated the documentation.

I also annotated all internal symbols (excl. classes in impl) with @BaselineIgnore("999.99.9") and all draft symbols (again excl. impl) with @BaselineIgnore("9.9.9"). The both versions mean far far far in the future (999.99.9) and far in the future (9.9.9). They have to be set because @BaselineIngore ignores the symbol only as long as it is compared to a version smaller then the specified one (it is actually meant to be used to ignore API changes temporarily).

While writing the docs I realised that changes to the package versions will happen rarely, meaning only when a draft API becomes stable and its @BaselineIgnore annotation is removed. However it looks like the evolution of different APIs is not linear. There are some that are still in draft since quite a while already while others got stable after a minimal period of time. That's the reason why we cannot, unfortunately just take next minor package version for @BaselineIngore.

Regarding

In other words, if package export version uses the ICU4J API version (70 for next release) for all packages, I understand it might create redundant version numbers, but are there any issues with this.

Yes, as a simplified solution, that would work as well. However, in OSGI it would mean that with each spec version increment (69=>70) all exported packages contain binary incompatible changes for consumers (with an import range like [69.0.0,70)). This has the consequence that all consuming bundles have to be rebuild (eventually adjusted) against the new icu4j build. If the above statement about incompatible changes is correct, then we can go for the simple approach. If not it would solve the issue of exporting packages always with the same version 0.0.0 (as it is now) but it would still require consumers to rebuild against each release. Disclaimer: in OSGI based applications it is common to update individual dependencies only at runtime, without rebuilding all bundles of the application.

If you prefer the simple approach as a compromise between established process and compatibility to OSGI, we can close this PR and I will open a new one?

buuhuu avatar Sep 29 '21 20:09 buuhuu