Skip to content
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

CI: add release on macos-14 #2950

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

CI: add release on macos-14 #2950

wants to merge 10 commits into from

Conversation

LeoHsiao1
Copy link
Contributor

Good morning!
In 2020, apple sells a new generation of mac computers, changing the CPU chips from Intel to M1, and the CPU architecture from amd64 to arm64.
In #2173, some users want to use exiv2 on MacOS M1 computers. I would like to help them, although I don't have a MacOS computer.

In 2024, actions/runner-images#9255, GitHub Actions added the macos-14 runner. It only uses M1 chips, not Intel chips.
So I modified the exiv2 workflow script to compile on macos-14. It looks like it compiled successfully, but not yet tested.

There is another problem, we need to change the filename format of the release packages, to differentiate between amd64 and arm64.
https://github.com/Exiv2/exiv2/blob/77915ad17b103793017e2b3b5ffd6c8a3ab1e916/.github/workflows/release.yml#L224-227

Copy link

codecov bot commented Apr 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 64.62%. Comparing base (77915ad) to head (eeaa0aa).
Report is 11 commits behind head on main.

❗ Current head eeaa0aa differs from pull request most recent head 930f40a. Consider uploading reports for the commit 930f40a to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2950   +/-   ##
=======================================
  Coverage   64.62%   64.62%           
=======================================
  Files         104      104           
  Lines       22239    22239           
  Branches    10911    10911           
=======================================
  Hits        14371    14371           
  Misses       5626     5626           
  Partials     2242     2242           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kmilos
Copy link
Collaborator

kmilos commented Apr 7, 2024

Thanks for the initiative! For the arch and archive names, you can check what we did for darktable recently: https://github.com/darktable-org/darktable/blob/master/.github/workflows/nightly.yml

@LeoHsiao1
Copy link
Contributor Author

Thanks for the darktable example.
In exiv2, .github/workflows/release.yml now generates 4 zip files:

exiv2-linux64.zip
exiv2-win.zip
exiv2-macos-12-x64.zip    # it is compatible with macos ≥12 , amd64
exiv2-macos-14-arm64.zip  # it is compatible with macos ≥14 , arm64

However, if you unzip exiv2-macos-12-x64.zip or exiv2-macos-14-arm64.zip, you will get a file named exiv2-1.00.0.9-Darwin.tar.gz.
This name is determined by cmake.
Honestly, I don't know how to configure cmake.

@kmilos kmilos added CI Issues related with CI jobs help wanted labels Apr 12, 2024
fail-fast: false
matrix:
runner:
- { os: macos-12, arch: X64 }
Copy link
Collaborator

Choose a reason for hiding this comment

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

These were now updated.

Use `64bit` or `32bit` on Windows hosts (including MinGW/Cygwin/MSYS),
the value of `${CMAKE_HOST_SYSTEM_ARCHITECTURE}` on all others.
Always insert into package filename directly after version,
separated from preceding version and following flags with hyphens.

So, e.g.
* `exiv2-1.0.9-Linux64.tar.gz`
* `exiv2-1.0.9-Darwin.tar.gz`
* `exiv2-1.0.9-2019msvc64.zip`
become:
* `exiv2-1.0.9-x86_64-Linux.tar.gz`
* `exiv2-1.0.9-arm64-Darwin.tar.gz`
* `exiv2-1.0.9-64bit-2019msvc.zip`
respectively.
@ferdnyc
Copy link
Contributor

ferdnyc commented May 10, 2024

@LeoHsiao1 I've used one of my favorite tricks, and opened a PR against your PR.

PR LeoHsiao1#1 will, if you merge it, incorporate my change to the packaging configs into this PR. Here's my commit message explaining how the package-naming changes:

CMake: Add architecture to package filename

Use 64bit or 32bit on Windows hosts (including MinGW/Cygwin/MSYS), the value of ${CMAKE_HOST_SYSTEM_ARCHITECTURE} on all others. Always insert into package filename directly after version, separated from preceding version and following flags with hyphens.

So, e.g.

  • exiv2-1.0.9-Linux64.tar.gz
  • exiv2-1.0.9-Darwin.tar.gz
  • exiv2-1.0.9-2019msvc64.zip

become:

  • exiv2-1.0.9-x86_64-Linux.tar.gz
  • exiv2-1.0.9-arm64-Darwin.tar.gz
  • exiv2-1.0.9-64bit-2019msvc.zip

respectively.

(Committers here can then still review that change as part of this PR.)

CMake: Add architecture to package filename
@LeoHsiao1
Copy link
Contributor Author

Thanks @ferdnyc , I merged your PR.
I run .github/workflows/release.yml again. Now it generate 4 files:

exiv2-linux64.zip         -> unzip ->    exiv2-1.00.0.9-x86_64-Linux.tar.gz
exiv2-macos-12-x64.zip    -> unzip ->    exiv2-1.00.0.9-x86_64-Darwin.tar.gz
exiv2-macos-14-arm64.zip  -> unzip ->    exiv2-1.00.0.9-arm64-Darwin.tar.gz
exiv2-win.zip             -> unzip ->    exiv2-1.00.0.9-64bit-2019msvc.zip

@kmilos
Copy link
Collaborator

kmilos commented May 11, 2024

Nice @ferdnyc

However, I suggest using real arch for Windows builds as well, as WoA adoption seems to be around the corner.

@ferdnyc
Copy link
Contributor

ferdnyc commented May 12, 2024

@kmilos

However, I suggest using real arch for Windows builds as well, as WoA adoption seems to be around the corner.

That's a good point, I'm just not sure how to do it for Windows. The CMake script uses the size of a void pointer in the current compiler, to determine 32- vs. 64-bit builds, and I'm pretty sure that even when sizeof(void_p) == 4, the ${CMAKE_HOST_SYSTEM_PROCESSOR} will be whatever 64-bit CPU the build is actually running on.

I suppose I could just hard-code it to i686 for 32-bit builds, since that's the only possibility, and use the real ${CMAKE_HOST_SYSTEM_PROCESSOR} value for 64-bit builds. How do people feel about that?

Copy link
Contributor

@ferdnyc ferdnyc left a comment

Choose a reason for hiding this comment

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

Three suggested changes that, applied together, would implement the "force i686 for 32-bit builds but use the real processor for all others" idea I mentioned in the comments.

(I had to split it up like this because of GitHub's annoying "no suggestions on ranges which include deleted lines" nonsense.)

cmake/packaging.cmake Outdated Show resolved Hide resolved
cmake/packaging.cmake Outdated Show resolved Hide resolved
cmake/packaging.cmake Outdated Show resolved Hide resolved
@LeoHsiao1
Copy link
Contributor Author

Fine, now .github/workflows/release.yml generate these files:

exiv2-arm64-macos-14.zip    -> unzip ->   exiv2-1.00.0.9-arm64-Darwin.tar.gz
exiv2-x64-macos-12.zip      -> unzip ->   exiv2-1.00.0.9-x86_64-Darwin.tar.gz
exiv2-x64-ubuntu-22.04.zip  -> unzip ->   exiv2-1.00.0.9-x86_64-Linux.tar.gz
exiv2-x64-windows-2022.zip  -> unzip ->   exiv2-1.00.0.9-AMD64-2019msvc.zip

The filename on the left side, indicates which machine exiv2 was built from.
Github Action may offer linux/arm64 and windows/arm64 in the future.

@neheb
Copy link
Collaborator

neheb commented Oct 2, 2024

Needs rebasing I think.

@LeoHsiao1
Copy link
Contributor Author

This PR only changes two files, which can be merged or rebased.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Issues related with CI jobs help wanted
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants