Create distribution packages for ubuntu (.deb release v0.0.1)#129
Create distribution packages for ubuntu (.deb release v0.0.1)#129nciric merged 9 commits intounicode-org:mainfrom
Conversation
inflection/CMakeLists.txt
Outdated
|
|
||
| set(CPACK_PACKAGE_NAME "unicode-inflection") | ||
| set(CPACK_PACKAGE_VENDOR "Unicode Consortium") | ||
| set(CPACK_PACKAGE_CONTACT "grhoten@gmail.com") |
There was a problem hiding this comment.
I think we need to find another contact. I'm not sure that I'm ready to have my email address here.
There was a problem hiding this comment.
Is that a mandatory field? If we have to have it, we can use the inflection-team@unicode.org one.
There was a problem hiding this comment.
I’d prefer just listing the official GitHub project. The discussion page can be used for communication.
inflection/CMakeLists.txt
Outdated
|
|
||
| # DEB-specific options | ||
| set(CPACK_DEBIAN_PACKAGE_MAINTAINER "Unicode Consortium <grhoten@gmail.com>") | ||
| set(CPACK_DEBIAN_PACKAGE_DEPENDS "libicu-dev (>= 77.1)") |
There was a problem hiding this comment.
The dev part is only needed for building. The non-dev dependency is needed for runtime.
There was a problem hiding this comment.
I'll update the dependency to libicu77 for runtime
nciric
left a comment
There was a problem hiding this comment.
Looks good in general with some comments.
| - 'v*' | ||
| workflow_dispatch: | ||
| pull_request: | ||
| branches: [ main, master ] |
There was a problem hiding this comment.
I don't think we need the master part. It's obsolete and our repo doesn't have it.
| with: | ||
| lfs: true | ||
|
|
||
| - name: Setup Git LFS |
There was a problem hiding this comment.
We can land it as is for now, but fix it after Frank lands 128 (LFS caching).
| sudo apt update | ||
| sudo apt install -y cmake build-essential clang pkg-config | ||
|
|
||
| - name: Install ICU 77.1 |
There was a problem hiding this comment.
ICU caching already landed - we can use it here to save bandwidth.
inflection/CMakeLists.txt
Outdated
|
|
||
| set(CPACK_PACKAGE_NAME "unicode-inflection") | ||
| set(CPACK_PACKAGE_VENDOR "Unicode Consortium") | ||
| set(CPACK_PACKAGE_CONTACT "grhoten@gmail.com") |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
See https://github.com/unicode-org/inflection/pull/128/files for example on how to do it.
There was a problem hiding this comment.
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.
d526bf8 to
dde6c7a
Compare
dde6c7a to
a22316b
Compare
| tags: | ||
| - 'v*' | ||
| workflow_dispatch: | ||
| pull_request: |
There was a problem hiding this comment.
Would this run the flow on each pull request? I don't think we need that.
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
Do we need this for final version?
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
I think you can run test in parallel as with previous step (-j$...)
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
can we extract the ICU version into a variable and use in multiple places? Just the icu4c-77_1-Ubuntu22.04-x64.tgz part
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I’ll rename the step to something generic like Install ICU (Binary)
|
Is it possible to derive the version of the Debian package from resources/share/inflection/config.properties? |
Great suggestion, I'll look into it. |
|
Btw I asked LLM to do something like this, and it came close: CMake Action with ICU caching: |
inflection/CMakeLists.txt
Outdated
| list(GET INFLECTION_VERSION_LIST 2 CPACK_PACKAGE_VERSION_PATCH) | ||
| endif() | ||
|
|
||
|
|
There was a problem hiding this comment.
remove extra new lines (line 141 and here).
| set(CPACK_PACKAGE_NAME "unicode-inflection") | ||
| set(CPACK_PACKAGE_VERSION "${INFLECTION_VERSION}") | ||
|
|
||
|
|
There was a problem hiding this comment.
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) | |||
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Good idea, will do that.
…e-org#129) * Github workflow for Ubuntu Packaging * Github workflow for Ubuntu Packaging. * Github workflow for Ubuntu Packaging! * Refactor packaging workflow based on the feedback * Fix CPack versioning: derive major/minor/patch from INFLECTION_VERSION * Fixing Version Issue * Fixing the Cmake Version * Fix: Cpack Version Handling * Add CPack-based Ubuntu packaging and GitHub Actions release workflow
* Github workflow for Ubuntu Packaging * Github workflow for Ubuntu Packaging. * Github workflow for Ubuntu Packaging! * Refactor packaging workflow based on the feedback * Fix CPack versioning: derive major/minor/patch from INFLECTION_VERSION * Fixing Version Issue * Fixing the Cmake Version * Fix: Cpack Version Handling * Add CPack-based Ubuntu packaging and GitHub Actions release workflow
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
.deband.tar.gzartifacts 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
CMakeLists.txtto:Inflection-0.1.0).deband.tar.gzartifactsGitHub Actions Workflow
.github/workflows/create-ubuntu-distribution-packaging.ymlv0.1.0)make checkcpackOutputs
.debpackage.tar.gzpackage