8000 Create distribution packages for ubuntu (.deb release v0.0.1) by dockerh · Pull Request #129 · unicode-org/inflection · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Create distribution packages for ubuntu (.deb release v0.0.1) #129

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

dockerh
Copy link
Collaborator
@dockerh dockerh commented Jun 11, 2025

Ubuntu Packaging with CPack & GitHub Actions

This PR introduces a GitHub Actions workflow for building and packaging the Unicode Inflection library on Ubuntu using CPack. It generates .deb and .tar.gz artifacts and uploads them to GitHub Releases when a Git tag is pushed. ICU 77.1 is installed from a prebuilt binary release.


Key Changes

CMake / CPack Configuration

  • Updates CMakeLists.txt to:
    • Dynamically parse version from Git tags (e.g. Inflection-0.1.0)
    • Configure CPack to generate .deb and .tar.gz artifacts

GitHub Actions Workflow

  • Workflow file: .github/workflows/create-ubuntu-distribution-packaging.yml
  • Triggered on tag push (e.g., v0.1.0)
  • Key steps:
    • Installs ICU 77.1 from a prebuilt Ubuntu binary
    • Builds using Clang with CMake
    • Runs tests with make check
    • Packages using cpack
    • Uploads artifacts to GitHub Release

Outputs

  • Ubuntu .deb package
  • Source .tar.gz package


set(CPACK_PACKAGE_NAME "unicode-inflection")
set(CPACK_PACKAGE_VENDOR "Unicode Consortium")
set(CPACK_PACKAGE_CONTACT "grhoten@gmail.com")
Copy link
Member

Choose a reason for hiding this comment

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

I think we need to find another contact. I'm not sure that I'm ready to have my email address here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is that a mandatory field? If we have to have it, we can use the inflection-team@unicode.org one.

Copy link
Member

Choose a reason for hiding this comment

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

I’d prefer just listing the official GitHub project. The discussion page can be used for communication.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Noted


# DEB-specific options
set(CPACK_DEBIAN_PACKAGE_MAINTAINER "Unicode Consortium <grhoten@gmail.com>")
set(CPACK_DEBIAN_PACKAGE_DEPENDS "libicu-dev (>= 77.1)")
Copy link
Member

Choose a reason for hiding this comment

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

The dev part is only needed for building. The non-dev dependency is needed for runtime.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll update the dependency to libicu77 for runtime

Copy link
Contributor
@nciric nciric left a comment

Choose a reason for hiding this comment

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

Looks good in general with some comments.

- 'v*'
workflow_dispatch:
pull_request:
branches: [ main, master ]
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we need the master part. It's obsolete and our repo doesn't have it.

with:
lfs: true
10000
- name: Setup Git LFS
Copy link
Contributor

Choose a reason for hiding this comment

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

We can land it as is for now, but fix it after Frank lands 128 (LFS caching).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sounds good.

sudo apt update
sudo apt install -y cmake build-essential clang pkg-config

- name: Install ICU 77.1
Copy link
Contributor

Choose a reason for hiding this comment

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

ICU caching already landed - we can use it here to save bandwidth.


set(CPACK_PACKAGE_NAME "unicode-inflection")
set(CPACK_PACKAGE_VENDOR "Unicode Consortium")
set(CPACK_PACKAGE_CONTACT "grhoten@gmail.com")
Copy link
Contributor

Choose a reason for hiding this comment

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

Is that a mandatory field? If we have to have it, we can use the inflection-team@unicode.org one.

- name: Install ICU 77.1
run: |
cd /tmp
wget https://github.com/unicode-org/icu/releases/download/release-77-1/icu4c-77_1-src.tgz
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we download and compile sources? Why not just download and install binary release for Ubuntu? It takes time to compile ICU.

One needs to only copy the $download_location/usr tree to /usr/local/ folder

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for the feedback, I’ll update the workflow to use the binary release and copy the $download_location/usr tree to /usr/local instead of building from source.

@CLAassistant
Copy link
CLAassistant commented Jul 4, 2025

CLA assistant check
All committers have signed the CLA.

@dockerh dockerh force-pushed the release/ubuntu-packaging-v0.0.1 branch 2 times, most recently from d526bf8 to dde6c7a Compare July 5, 2025 08:12
@dockerh dockerh force-pushed the release/ubuntu-packaging-v0.0.1 branch from dde6c7a to a22316b Compare July 5, 2025 19:38
@dockerh dockerh requested review from grhoten and nciric July 6, 2025 08:50
tags:
- 'v*'
workflow_dispatch:
pull_request:
Copy link
Contributor

Choose a reason for hiding this comment

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

Would this run the flow on each pull request? I don't think we need that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I’ll remove the pull_request trigger so the workflow runs only on tag pushes and manual dispatch.

run: |
sudo ldconfig
- name: Debug ICU version
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this for final version?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was helpful for initial testing, but I agree, it can be removed from the final version. I’ll delete the debug step.

- name: Run tests
run: |
cd inflection/build
make check
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can run test in parallel as with previous step (-j$...)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes,
I’ll update the make check step to use -j$(nproc) for parallel test execution.

if: steps.cache-icu.outputs.cache-hit != 'true'
run: |
cd /tmp
wget https://github.com/unicode-org/icu/releases/download/release-77-1/icu4c-77_1-Ubuntu22.04-x64.tgz
Copy link
Contributor

Choose a reason for hiding this comment

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

can we extract the ICU version into a variable and use in multiple places? Just the icu4c-77_1-Ubuntu22.04-x64.tgz part

Copy link
Member

Choose a reason for hiding this comment

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

What about ARM64?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Let me look into it, Currently, this workflow targets x64. I’ll explore adding runs-on: ubuntu-latest matrix support for arm64.

path: /usr/local
key: icu-77.1-ubuntu-${{ runner.os }}

- name: Install ICU 77.1 (Binary)
Copy link
Contributor

Choose a reason for hiding this comment

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

Pls remove the version from the name - this step will stay the same in the future, we'll just have to update the numbers below.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I’ll rename the step to something generic like Install ICU (Binary)

@grhoten
Copy link
Member
grhoten commented Jul 7, 2025

Is it possible to derive the version of the Debian package from resources/share/inflection/config.properties?

@dockerh
Copy link
Collaborator Author
dockerh commented Jul 7, 2025

Is it possible to derive the version of the Debian package from resources/share/inflection/config.properties?

Great suggestion, I'll look into it.

@nciric
Copy link
Contributor
nciric commented Jul 7, 2025

Btw I asked LLM to do something like this, and it came close:

CMake

# Install the library and header files
install(TARGETS inflection DESTINATION lib)
install(DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}/include/" DESTINATION include)

# --- CPack configuration ---
set(CPACK_PACKAGE_NAME "inflection")
set(CPACK_PACKAGE_VERSION ${PROJECT_VERSION})
set(CPACK_PACKAGE_DESCRIPTION_SUMMARY "A C++ library for handling grammatical inflection")
set(CPACK_PACKAGE_VENDOR "Unicode")
set(CPACK_PACKAGE_FILE_NAME "${CPACK_PACKAGE_NAME}-${CPACK_PACKAGE_VERSION}-${CMAKE_SYSTEM_NAME}")
set(CPACK_GENERATOR "TGZ;ZIP")

include(CPack)

Action with ICU caching:

name: Create Binary Release

on:
  push:
    tags:
      - 'v*'

jobs:
  build_and_release:
    name: Build and Release
    runs-on: ${{ matrix.os }}
    strategy:
      matrix:
        os: [ubuntu-latest, windows-latest, macos-latest]

    steps:
    - name: Checkout code
      uses: actions/checkout@v4

    - name: Configure CMake
      run: cmake -B build

    - name: Build project
      run: cmake --build build --config Release

    - name: Package with CPack
      run: |
        cd build
        cpack -C Release

    - name: Get package name
      id: get_package_name
      run: |
        cd build
        PACKAGE_FILE=$(find . -name "inflection-*.zip" -o -name "inflection-*.tar.gz" | head -n 1)
        echo "package_file=$PACKAGE_FILE" >> $GITHUB_ENV
        echo "asset_name=$(basename $PACKAGE_FILE)" >> $GITHUB_ENV

    - name: Upload Release Asset
      uses: softprops/action-gh-release@v1
      with:
        files: ${{ env.package_file }}
        name: Release ${{ github.ref_name }}
        tag_name: ${{ github.ref }}
        body: "Binary release for version ${{ github.ref_name }}"
      env:
        GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

@dockerh dockerh requested a review from nciric July 10, 2025 20:51
list(GET INFLECTION_VERSION_LIST 2 CPACK_PACKAGE_VERSION_PATCH)
endif()


Copy link
Contributor

Choose a reason for hiding this comment

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

remove extra new lines (line 141 and here).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Okay

set(CPACK_PACKAGE_NAME "unicode-inflection")
set(CPACK_PACKAGE_VERSION "${INFLECTION_VERSION}")


Copy link
Contributor
@nciric nciric Jul 10, 2025

Choose a reason for hiding this comment

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

I would put a comment here:

# Apply the current tagged Inflection version to the CPack version.
# CPack inherits the version from ??? so we need to reset before the assignment.

@@ -0,0 +1,94 @@
name: Build & Package (Ubuntu)
Copy link
Contributor
@nciric nciric Jul 10, 2025

Choose a reason for hiding this comment

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

We should have a comment on top (or a separate document) that tells us how to update ICU dependancy:

# To update ICU version:
# 1. Update ICU_VERSION variable
# 2. Check package names, e.g. is it still Ubuntu22.04-
# 3. Update inflection/CMakeLists.txt where we check CPACK_DEBIAN_PACKAGE_DEPENDS "libicu77 (>= 77.1)"

Anything else I missed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good idea, will do that.

@dockerh dockerh requested a review from nciric July 12, 2025 19:46
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.

4 participants
0