Skip to content

Implement trustkit android release GitHub actions - #118

Open
alexanderdibenedetto wants to merge 4 commits into
masterfrom
mts1-542-implement-trustkit-android-release-github-actions
Open

alexanderdibenedetto wants to merge 4 commits into
masterfrom
mts1-542-implement-trustkit-android-release-github-actions

Conversation

@alexanderdibenedetto

@alexanderdibenedetto alexanderdibenedetto commented Nov 2, 2026 •

Copy link
Copy Markdown

Release TrustKit 1.2.0 to Maven Central from GitHub releases

Publishing a GitHub release now verifies the version, builds a signed Maven bundle (AAR, POM, module metadata, sources, javadoc, checksums, signatures), attaches it to the release, and publishes it to Maven Central.

  • Release logic lives in scripts/. Publishing config moved into trustkit/build.gradle.
  • CI runs on every PR. Lint runs first. Then unit tests, script tests, the release bundle, the demo apps and instrumentation tests run in parallel.
  • Fixed instrumentation tests broken by live-site certificate changes. Live tests now use only datatheorem.com, yahoo.com and badssl.com, with the same pins as TrustKit iOS.
  • README: release instructions and a Maven Central badge.

Before the first release: create a maven-central environment with MAVEN_CENTRAL_USERNAME, MAVEN_CENTRAL_PASSWORD, SIGNING_KEY and SIGNING_KEY_PASSWORD, and publish the signing key's public half to a keyserver.

Alexander DiBenedetto added 4 commits November 2, 2026 14:00
CI now runs on every pull request: lint first, then unit tests, the
release bundle, demo apps and instrumentation tests in parallel. A
published GitHub release attaches the signed bundle zip and uploads
the same zip to Maven Central. Build and release logic lives in
scripts/, and publishing config moves into trustkit/build.gradle.
Replace facebook, microsoft and google hosts with yahoo.com and
datatheorem.com equivalents. Stop re-running CI at release time, and
replace the dead Bintray badge with a Maven Central badge.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings marked 🟡 are optional suggestions and need no follow-up push.

Comment thread .github/workflows/ci.yml
@@ -111,23 +108,3 @@ jobs:
trustkit/build/outputs/managed_device_android_test_additional_output/**
if-no-files-found: ignore
retention-days: 7

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 If branch protection requires the CI checks status, every PR stays blocked after merge, because no job reports that check any more. The diff deletes the ci-checks aggregator job (if: ${{ always() }}, needs build + instrumentation-tests). Its job name was the single status that summed up all CI jobs. Fix: keep an always() aggregator named CI checks that needs lint, unit-tests, script-tests, release-bundle, demo and instrumentation-tests and fails unless every result is success. Otherwise, update branch protection in the same rollout.

Why this was flagged

This applies only if the repository's branch protection lists CI checks as a required status, which this checkout cannot show. The aggregator pattern deleted at .github/workflows/ci.yml (old lines 115-133) exists for exactly that setup. After merge no workflow emits a check named CI checks, so GitHub shows it as "Expected — Waiting for status" on every PR and blocks merging. Admins could instead require the new per-job checks. But GitHub counts a skipped required job as passing. Dependents skip when lint fails, so a protection list that leaves out Lint would let a failing PR merge. On the base branch one required check covered every job. The grep for ci-checks and CI checks finds nothing left in the repo.

Verification: The trigger is that the repository's branch protection or ruleset lists CI checks as a required status check. On the base commit, .github/workflows/ci.yml defines ci-checks: with name: CI checks and if: ${{ always() }}. The diff deletes that whole job (the removed hunk at old lines ~115-133). If protection requires it, every PR waits forever for a CI checks status that never arrives.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can remove it if you've setup branch protection to require all other checks instead.

# Maven Central releases cannot be undone. Never cancel or overlap them.
concurrency:
group: maven-central-release
cancel-in-progress: false

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 (optional) A maintainer who publishes several releases in a row can find a middle release silently never published to Maven Central, despite the "Never cancel" comment. With cancel-in-progress: false, GitHub still cancels a pending run when a newer run joins the same concurrency group. So if three releases are published while the first is still running, the second run is cancelled before it starts. Fix: make every release run eventually, for example with a per-tag group maven-central-release-${{ github.ref_name }}. Alternatively, document that a cancelled pending run must be re-run, and correct the comment.

Why this was flagged

Trigger: three GitHub releases are published, e.g. 1.2.1, 1.1.6, 1.2.2, while the first Release run is still in progress. That run takes a Gradle build plus up to 40 minutes of polling in scripts/publish_maven_central.sh. The workflow-level concurrency: group: maven-central-release at .github/workflows/release.yml:11-13 queues the second run as pending. When the third arrives, GitHub replaces the pending run with the new one and cancels it, and cancel-in-progress: false does not prevent that. The cancelled release keeps its GitHub release page but gets no zip and no Maven Central publication until someone notices and re-runs it. The comment at line 10 promises "Never cancel or overlap them". There was no release workflow on the base branch.

Verification: Lines 11-13 of .github/workflows/release.yml put every run in one workflow-level group: concurrency: group: maven-central-release with cancel-in-progress: false. When a new run queues, any existing pending run in that group is cancelled. So the middle release's run ends as cancelled and never reaches Maven Central. This is a new workflow, so nothing on the base regresses.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using maven-central-release-${{ github.ref_name }} here sounds sensible. We don't want to be able to release the same version twice but we should be able to release 2 different versions simultaneously.

Comment thread trustkit/build.gradle
Comment on lines +64 to +68
publishing {
publications {
release(MavenPublication) {
from components.release
artifactId = 'trustkit'

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 (optional) Apps that compiled against androidx.preference or androidx.annotation only because TrustKit 1.1.5 pulled them in can fail to compile after upgrading to 1.2.0. The deleted publish-mavencentral.gradle copied every implementation dependency into the POM with no <scope>, which Maven treats as compile scope. from components.release publishes them as runtime-only, in both the POM and the new .module metadata. Fix: either keep that exposure (declare androidx.preference and androidx.annotation as api) or call out the dropped transitive compile dependencies in the 1.2.0 release notes.

Why this was flagged

This only matters for downstream apps that use androidx.preference or androidx.annotation classes without declaring those libraries themselves; this checkout cannot list who they are. On the base branch, the deleted deploymentScripts/publish-mavencentral.gradle (its withXml block) added a <dependency> with groupId, artifactId and version, but no scope, for each entry in project.configurations.implementation.allDependencies. Its publication had no Gradle module metadata, so consumers resolved from the POM and got both libraries at compile scope. After the change, trustkit/build.gradle:64-68 uses from components.release. AGP maps the implementation libs.androidx.annotation and implementation libs.androidx.preference lines in trustkit/build.gradle to runtime scope in the POM and leaves them out of the API variant in the new .module file. A consumer that bumps from 1.1.5 to 1.2.0 then gets unresolved-symbol compile errors for those classes. They still resolve at runtime. Nothing in the diff, README or release flow mentions this change in what the artifact exposes.

Verification: In the base, deploymentScripts/publish-mavencentral.gradle added a <dependency> for each implementation entry with no <scope>, so Maven treats them as compile scope. At HEAD, trustkit/build.gradle:68 uses from components.release, and build.gradle:45-46 keep them as implementation. The POM gives them <scope>runtime</scope>, and the .module lists them only in the runtime variant.

@marcantoineg marcantoineg left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 After comments

Comment thread .github/workflows/ci.yml
@@ -111,23 +108,3 @@ jobs:
trustkit/build/outputs/managed_device_android_test_additional_output/**
if-no-files-found: ignore
retention-days: 7

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can remove it if you've setup branch protection to require all other checks instead.

# Maven Central releases cannot be undone. Never cancel or overlap them.
concurrency:
group: maven-central-release
cancel-in-progress: false

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using maven-central-release-${{ github.ref_name }} here sounds sensible. We don't want to be able to release the same version twice but we should be able to release 2 different versions simultaneously.

timeout-minutes: 5
steps:
- uses: actions/checkout@v6
- run: ./scripts/verify_release_version.sh "$GITHUB_REF_NAME"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should probably use ${{ github.event.release.tag_name }} here instead:
https://github.com/orgs/community/discussions/64528#discussioncomment-12978855


# Only the version folder: Central does not want the artifact-level maven-metadata.xml.
(cd "$REPOSITORY" && zip -qr "$BUNDLE" "com/datatheorem/android/trustkit/trustkit/${VERSION}")
unzip -l "$BUNDLE"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we might as well do a zip -r (no -q) and remove the redundant unzip here.

Comment thread trustkit/build.gradle
Comment on lines +80 to +89
developer {
id = 'nabla-c0d3'
name = 'Alban Diquet'
email = 'ad@datatheorem.io'
}
developer {
id = 'jobot0'
name = 'Jordan Bouellat'
email = 'jb@datatheorem.com'
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these still valid?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey Jordan here! First thanks for your work to update the lib! No you can override at least my contact infos with the ones from the new maintainers.

This branch has not been deployed

No deployments
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.

6 participants