Add CI to build and publish compiled binaries by robertbastian · Pull Request #6826 · unicode-org/icu4x

robertbastian · 2025-08-08T11:56:35Z

gemini-code-assist · 2025-08-08T11:56:38Z

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.

This reverts commit 4686320.

This reverts commit b4a65e0.

This reverts commit c213e03.

sffc · 2025-08-13T05:52:18Z

/gemini review

gemini-code-assist

Code Review

This pull request refactors the build scripts to use Rust target triples (e.g., aarch64-apple-darwin) for identifying build artifacts, which is a good improvement for clarity and consistency. However, I've identified a couple of critical issues in the auto-generated hashes.dart file where incorrect Rust target strings are used as keys. These errors would break the fetching of pre-built binaries for Android ARM64 and macOS ARM64 platforms. I've also pointed out a minor area for code cleanup. Please address these points to ensure the build process works correctly for all users.

ffi/dart/lib/src/hook_helpers/hashes.dart

ffi/dart/hook/build.dart

.github/workflows/artifacts-build.yml

sffc · 2025-08-13T06:12:09Z

.github/workflows/artifacts-build.yml

+        name: ffi-${{matrix.os}}-${{ matrix.compiletype }}${{ matrix.compiletype == 'static' && matrix.include_data == true && '-with_data' || '' }}-libs
+        path: bin
+
+  release-ffi-libs:


Question: Have you tested this action, since it only runs on tags?

I've run in up to the release step, see the commit history

sffc · 2025-08-13T06:13:25Z

.github/workflows/artifacts-build.yml

+      run: |
+        (cd ffi/dart && dart pub get)
+
+        dart ffi/dart/lib/src/hook_helpers/build_libs.dart --working_directory . --file bin/icu4x-2-${{ matrix.compiletype }}${{ env.FILENAME_SUFFIX }}-armv7-linux-androideabi --target armv7-linux-androideabi --compile_type ${{ matrix.compiletype }} --cargo_features ${{ env.FEATURES }}


Question: Why are you using a Dart script to build these, instead of a cargo make script or something?

because we already have a dart build script. I plan to migrate this in the future (but we'll always need a Dart build script anyway)

sffc · 2025-08-13T06:15:26Z

ffi/dart/tool/regenerate_hashes.dart

+  for (final rustTarget in [
+    'armv7-linux-androideabi',
+    'aarch64-linux-android',
+    'i686-linux-android',
+    'riscv64-linux-android',
+    'x86_64-linux-android',
+    'aarch64-unknown-fuchsia',
+    'x86_64-unknown-fuchsia',
+    'aarch64-apple-ios',
+    'x86_64-apple-ios',
+    'armv7-unknown-linux-gnueabihf',
+    'aarch64-unknown-linux-gnu',
+    'i686-unknown-linux-gnu',
+    'riscv32gc-unknown-linux-gnu',
+    'riscv64gc-unknown-linux-gnu',
+    'x86_64-unknown-linux-gnu',
+    'aarch64-apple-darwin',
+    'x86_64-apple-darwin',
+    'aarch64-pc-windows-msvc',
+    'i686-pc-windows-msvc',
+    'x86_64-pc-windows-msvc',


Observation: You have this list, or things derived from this list, in multiple places. Seems fragile?

the whole hash code needs to be cleaned up but I can only really work on it once we have published artifacts

sffc · 2025-08-13T06:16:30Z

.github/workflows/artifacts-build.yml

+        echo "riscv64-linux-android.linker=\"$ANDROID_NDK_LATEST_HOME/toolchains/llvm/prebuilt/linux-x86_64/bin/riscv64-linux-android35-clang\"" >> .cargo/config.toml
+        echo "aarch64-unknown-linux-gnu.linker=\"aarch64-linux-gnu-gcc\"" >> .cargo/config.toml
+        echo "armv7-unknown-linux-gnueabihf.linker=\"arm-linux-gnueabihf-gcc\"" >> .cargo/config.toml
+        echo "riscv64gc-unknown-linux-gnu.linker=\"riscv64-linux-gnu-gcc\"" >> .cargo/config.toml


Question: Why can't you use lld for everything? Why do you need to install and point to custom linkers for each platform

this is existing code that currently lives in dart-lang/i18n, but previously already lived in ICU4X where you already reviewed it: https://github.com/unicode-org/icu4x/pull/4514/files#diff-d1bb081e6d04ced14e89691280b752ae6124cee2b13d0af35840b7bf3db32a07

sffc · 2025-08-15T04:16:30Z

@robertbastian Can I ship the package on pub.dev as 2.0.0-dev0?
@sffc Yes, I think that is the correct number to use, and I think it's better to have a package out there, even if we decide to tweak it later with more features / slicing / etc.
@Manishearth LGTM. A bit worried about the stability stuff.
@sffc Let's confirm with @Moseum. We're meeting later today.
@robertbastian I think @Moseum is okay with this; he preferred 0.2 but is okay with 2.0.0-dev0.
@sffc I think releasing a prerelease artifact doesn't constitute a Technical Decision and therefore we can approve it here in the Working Group. But, FYI @zbraniecki.

robertbastian force-pushed the artifacts branch 3 times, most recently from 00cab07 to 8f20b88 Compare August 8, 2025 12:58

robertbastian added 3 commits August 11, 2025 14:51

Revert "rm icu4x_artifacts.yml for this PR"

084b099

This reverts commit 4686320.

rust names

7c09a84

merge into artifacts-build

60c1fc2

robertbastian force-pushed the artifacts branch from 8f20b88 to 60c1fc2 Compare August 11, 2025 12:51

robertbastian added 2 commits August 11, 2025 14:53

test

b4a65e0

Revert "test"

c213e03

This reverts commit b4a65e0.

robertbastian requested a review from mosuem August 11, 2025 13:16

robertbastian marked this pull request as ready for review August 11, 2025 13:16

robertbastian requested a review from a team as a code owner August 11, 2025 13:16

robertbastian requested a review from younies August 11, 2025 13:17

robertbastian added 3 commits August 12, 2025 15:31

Reapply "test"

41384ba

This reverts commit c213e03.

release

163c17b

release

b13c9db

robertbastian requested a review from sffc August 12, 2025 15:27

gemini-code-assist bot reviewed Aug 13, 2025

View reviewed changes

ffi/dart/lib/src/hook_helpers/hashes.dart Show resolved Hide resolved

ffi/dart/lib/src/hook_helpers/hashes.dart Show resolved Hide resolved

ffi/dart/hook/build.dart Show resolved Hide resolved

sffc reviewed Aug 13, 2025

View reviewed changes

sffc changed the title ~~Build artifacts~~ Add CI to build and publish compiled binaries Aug 13, 2025

rm submodule

2f0a1e7

robertbastian requested a review from sffc August 13, 2025 11:22

robertbastian added 2 commits August 13, 2025 13:23

nit

7b505ec

use --emit link=

5e0d3e2

sffc approved these changes Aug 13, 2025

View reviewed changes

robertbastian merged commit 0027ccb into unicode-org:main Aug 13, 2025
32 checks passed

robertbastian deleted the artifacts branch August 13, 2025 15:16

Conversation

robertbastian commented Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gemini-code-assist bot commented Aug 8, 2025

Uh oh!

sffc commented Aug 13, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sffc Aug 13, 2025

Choose a reason for hiding this comment

Uh oh!

robertbastian Aug 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sffc Aug 13, 2025

Choose a reason for hiding this comment

Uh oh!

robertbastian Aug 13, 2025

Choose a reason for hiding this comment

Uh oh!

sffc Aug 13, 2025

Choose a reason for hiding this comment

Uh oh!

robertbastian Aug 13, 2025

Choose a reason for hiding this comment

Uh oh!

sffc Aug 13, 2025

Choose a reason for hiding this comment

Uh oh!

robertbastian Aug 13, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sffc commented Aug 15, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

robertbastian commented Aug 8, 2025 •

edited

Loading

robertbastian Aug 13, 2025 •

edited

Loading