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

Initial implementation #1

Merged
merged 1 commit into from
Oct 23, 2024
Merged

Initial implementation #1

merged 1 commit into from
Oct 23, 2024

Conversation

wojtekmach
Copy link
Collaborator

@wojtekmach wojtekmach commented Sep 12, 2024

This is the implementation of the OTP macOS builds proposal.

Updated README.md: https://github.com/erlef/otp_builds/blob/wm-initial/README.md

As you can see in commits list, the GitHub job automatically commits builds/*.txt changes.

Builds can be manually triggered like this:

ref=OTP-27.0.1;
gh workflow run build.yaml \
  -R erlef/otp_builds \
  --ref wm-initial \
  -f otp-ref-name=$ref \
  -f otp-ref=$(gh api repos/erlang/otp/commits/$ref --jq .sha)

Example

$ curl --fail -LO https://github.com/erlef/otp_builds/releases/download/OTP-27.0.1/OTP-27.0.1-macos-arm64.tar.gz
$ mkdir /tmp/otp ; tar xzf OTP-27.0.1-macos-arm64.tar.gz --cd /tmp/otp
$ /tmp/otp/bin/erl
Erlang/OTP 27 [erts-15.0.1] [source] [64-bit] [smp:8:8] [ds:8:8:10] [async-threads:1] [jit]

Eshell V15.0.1 (press Ctrl+G to abort, type help(). for help)
1>

michallepicki added a commit to michallepicki/asdf-erlang-prebuilt-macos that referenced this pull request Sep 13, 2024
to be reverted when erlef/otp_builds#1 is merged
michallepicki added a commit to michallepicki/asdf-erlang-prebuilt-macos that referenced this pull request Sep 13, 2024
to be reverted when erlef/otp_builds#1 is merged
michallepicki added a commit to michallepicki/asdf-erlang-prebuilt-macos that referenced this pull request Sep 13, 2024
to be reverted when erlef/otp_builds#1 is merged
README.md Outdated Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

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

For clarity, should we mention how OpenSSL and wxWidgets are build and/or statically linked?

And what about "this is our strategy for updating OpenSSL if a vulnerability is found" (?)

README.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@paulo-ferraz-oliveira paulo-ferraz-oliveira left a comment

Choose a reason for hiding this comment

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

Left a few comments but nothing that can't be turned into GitHub issues and done later. Approving...

@wojtekmach
Copy link
Collaborator Author

@paulo-ferraz-oliveira @starbelly thank you for the feedback. Regarding https://github.com/jelly-beam/otp-macos, it is quite comprehensive and it's a great starting point. Now that you can see how I tried implementing this, we can consider the next steps.

Would you rather:

  1. merge changes from this PR to jelly-beam/otp-macos, rename it to erlef/otp-macos
  2. merge changes from jelly-beam/otp-macos here
  3. ignore this PR, merge jelly-beam/otp-macos as is here

?

@paulo-ferraz-oliveira
Copy link
Collaborator

Would you rather:

"merge changes from jelly-beam/otp-macos here", for me 👍

Some stuff can be adapted, I guess, but only if there's a need for it; as long as we have the builds here there's only a few details I'd update, but I don't think they'll change the base behaviour:

  • (later?) I'd start the builds from 25.1 since I think it's feasible Think about building from 25.1 on #4
  • I'd like to have a mechanism for rebuilding a build (e.g. you update a file, then it gets picked up and replaces the build in .txt - or maybe the dispatch already does it?): important in case we want to update e.g. OpenSSL without disruption
  • I'd prefer to have a .sha256.txt of the result next to it, for security Consider attaching a .sha256.txt file to build results #25

This said, I'd go with your current implementation; it's a good starting point and I can see a couple of advantages:

  • compiling OpenSSL (instead of brewing it),
  • doing all stuff by hand (instead of kerling it),
  • using wxWidgets (I didn't test that as a default for kerl so it's possible it's not working, but it's better to have it, too)

I'm also Ok with using otp-macos but it'd need some issues taken care of (remove brew and kerl, for example; add wxWigets), so it's less relevant here.

@starbelly
Copy link
Member

This said, I'd go with your current implementation; it's a good starting point and I can see a couple of advantages:

* compiling OpenSSL (instead of `brew`ing it),

* doing all stuff by hand (instead of `kerl`ing it),

* using wxWidgets (I didn't test that as a default for `kerl` so it's possible it's not working, but it's better to have it, too)

I'm also Ok with using otp-macos but it'd need some issues taken care of (remove brew and kerl, for example; add wxWigets), so it's less relevant here.

☝️ That. Let's go with this PR, then roll in changes that were done in otp-macos, collaboratively.

@wojtekmach
Copy link
Collaborator Author

Sounds good, I’ll send some updates here soon!

@starbelly
Copy link
Member

@wojtekmach Just to confirm, you are ready for this to be merged?

@wojtekmach
Copy link
Collaborator Author

Not quite yet, I'll merge this myself if that's OK.

README.md Outdated Show resolved Hide resolved
@wojtekmach wojtekmach force-pushed the wm-initial branch 2 times, most recently from 7ef84cc to bb007d8 Compare October 23, 2024 21:06
@wojtekmach
Copy link
Collaborator Author

I have made the following changes:

  1. I added a disclaimer at the top of the README saying this is a work in progress. I'd like to go over all issues and PRs and built all releases all the way back to OTP 25.0. Then I'd consider this ready.
  2. I changed naming from macos-{amd64,arm64} to {x86_64,aarch64}-apple-darwin in filenames. The rationale was we'd match erlang:system_info(system_architecture). I'm going to keep build artifacts using old names for a while since beamup was already using this project.
  3. I changed builds/*.txt to builds/*.csv and added openssl and wxwidgets versions there.

I'm going to merge this as is to unblock others tasks but please feel free to keep the discussion going here or in other issues.

Thanks everyone for reviewing this!

@wojtekmach wojtekmach merged commit c132729 into main Oct 23, 2024
@wojtekmach wojtekmach deleted the wm-initial branch October 23, 2024 21:08
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