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.

Sign up for GitHub

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

Jump to bottom

Investigate Mac Builds for 'x64' + 'arm64' or 'universal' #468 #469

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Investigate Mac Builds for 'x64' + 'arm64' or 'universal' #468 #469

wants to merge 1 commit into from

Conversation

jfaltermeier
Copy link
Contributor

@jfaltermeier jfaltermeier commented Feb 19, 2025 •
edited
Loading

What it does

closes #468

How to test

I guess we need to test builds via the preview channel

Review checklist

Reminder for reviewers

@jfaltermeier jfaltermeier force-pushed the jf/mac-arm branch 7 times, most recently from c4f7afa to 6a71c50 Compare March 3, 2025 09:14
@jfaltermeier jfaltermeier force-pushed the jf/mac-arm branch 23 times, most recently from 847a63c to 1391d3f Compare March 3, 2025 16:12
@jfaltermeier jfaltermeier force-pushed the jf/mac-arm branch 2 times, most recently from 4e1391d to ddb2298 Compare March 3, 2025 19:22
@jfaltermeier jfaltermeier force-pushed the jf/mac-arm branch 12 times, most recently from 831dcd3 to a113a6b Compare March 7, 2025 11:11
jfaltermeier
jfaltermeier commented Mar 7, 2025
View reviewed changes
@jfaltermeier jfaltermeier changed the title [WIP] Investigate Mac Builds for 'x64' + 'arm64' or 'universal' #468 Investigate Mac Builds for 'x64' + 'arm64' or 'universal' #468 Mar 10, 2025
@jfaltermeier jfaltermeier marked this pull request as ready for review March 10, 2025 12:01
@jfaltermeier jfaltermeier requested a review from sgraband March 10, 2025 12:02
sgraband
sgraband reviewed Mar 12, 2025
View reviewed changes
Copy link
Contributor

@sgraband sgraband left a comment

Choose a reason for hiding this comment

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

Thanks for the changes! Looks mostly good to me. I have two comments below and two questions:

  • Do you want to merge this with the debug statements?
  • Are you planning on squashing on merge? Then it will be harder to remove the debug statements later on, right?

PUBLISHING.md Outdated
Comment on lines 53 to 57
After completing these steps, open a PR with your changes.
The PR will trigger a verification build that will have two zip files with mac artifacts attached.
Download these zips and replace them on this pre-release: <https://github.com/eclipse-theia/theia-ide/releases/tag/pre-release>.
The unsigned dmgs will be used as the input the Jenkins Build.
Merging the PR automatically triggers a preview release.
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like we could just continue the numbered list for these points. WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, I'll adjust this.

@jfaltermeier
Copy link
Contributor Author

I would like to keep the remaining debug outputs. I already cleaned up unnecessary logs, but the ones left are actually useful in case of errors or if new binaries that need signing are added to the tool. The output from the mac notarization service might not be too helpful.

Since these logs are only in a build-time script, I think this is fine.

I would still prefer to squash, but if you disagree, I can force-push the change in two commits, keeping the debug logs in a separate one. Let me know what works best for you.

@sgraband
Copy link
Contributor

No its fine then. Then i am fine with merging, when the Publishing is adapted

sgraband
sgraband approved these changes Mar 12, 2025
View reviewed changes
Copy link
Contributor

@sgraband sgraband left a comment

Choose a reason for hiding this comment

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

LGTM 👍 I leave it up to you whether or not we should include the macos14 tests

@jfaltermeier
Copy link
Contributor Author

Reason for the test failure was that the application is created on a different path on arm-macs.
I'll rebase this on #496 once merged and then squash and merge this one

@jfaltermeier
Mac Builds for 'x64' + 'arm64'
19c496f 
* since we only have x64 node in Eclipse CI we will build for x64 and
arm64 on Github runners
* Zipped dist directories will be made available to the Jenkins build
via a Github pre-release
* Zips will be produced by verification Job for now
* adjusts the Jenkinsbuild to get the zips from the pre-release, unpack
the dmg, sign the files, repack, and continue build as before
* add a new location on download.eclipse.org for mac arm builds (the
latest file is called the same for both mac archs, so we need two
locations for updates)
* enable tests for arm
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sgraband sgraband sgraband approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Mac Builds for 'x64' + 'arm64' or 'universal'
2 participants
@jfaltermeier @sgraband