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

(PA-5594) Enable macOS 13 (ARM) puppet-runtime builds for agent-runtime-7.x #684

Merged
merged 1 commit into from
Jul 11, 2023

Conversation

skyamgarp
Copy link
Contributor

@skyamgarp skyamgarp commented Jun 26, 2023

Hello,
The result of component diff :

`
Platform name: osx-13-arm64
Component 'openssl-1.1.1'
Field: configure[0]
--------------------
- [" ./Configure --prefix=/opt/puppetlabs/puppet --libdir=lib --openssldir=/opt/puppetlabs/puppet/ssl shared no-asm darwin64-x86_64-cc no-camellia no-ec2m no-md2 no-ssl3 no-dtls no-dtls1 no-idea no-seed no-weak-ssl-ciphers -DOPENSSL_NO_HEARTBEATS -I/opt/puppetlabs/puppet/include "]
+ [" ./Configure --prefix=/opt/puppetlabs/puppet --libdir=lib --openssldir=/opt/puppetlabs/puppet/ssl shared no-asm darwin64-arm64-cc no-camellia no-ec2m no-md2 no-ssl3 no-dtls no-dtls1 no-idea no-seed no-weak-ssl-ciphers -DOPENSSL_NO_HEARTBEATS -I/opt/puppetlabs/puppet/include "]

    Field: environment
    --------------------
    - PATH=/opt/pl-build-tools/bin:$(PATH):/usr/local/bin
    + PATH=/opt/homebrew/bin:$(PATH):/usr/local/bin


Component 'ruby-2.7.8'
    Field: configure[0]
    --------------------
    - ["bash configure         --enable-shared         --enable-bundled-libyaml         --disable-install-doc         --disable-install-rdoc                   --prefix=/opt/puppetlabs/puppet --with-opt-dir=/opt/puppetlabs/puppet  --enable-dtrace "]
    + ["bash configure         --enable-shared         --enable-bundled-libyaml         --disable-install-doc         --disable-install-rdoc                   --prefix=/opt/puppetlabs/puppet --with-opt-dir=/opt/puppetlabs/puppet  --with-openssl-dir=/opt/puppetlabs/puppet  --enable-dtrace "]

    Field: environment
    --------------------
    - optflags=-I/opt/puppetlabs/puppet/include CC=clang
    + optflags=-I/opt/puppetlabs/puppet/include CC=clang PATH=/opt/homebrew/bin:$(PATH):/usr/local/bin

    Field: install[12]
    --------------------
    + ["/opt/puppetlabs/puppet/bin/ruby ../rbconfig-update.rb \"{\\\"CC\\\"=>\\\"clang\\\"}\" $$(/opt/puppetlabs/puppet/bin/ruby -e \"puts RbConfig::CONFIG[\\\"topdir\\\"]\")", "cp original_rbconfig.rb /opt/puppetlabs/puppet/share/doc/rbconfig-2.7.8-orig.rb", "cp new_rbconfig.rb $$(/opt/puppetlabs/puppet/bin/ruby -e \"puts RbConfig::CONFIG[\\\"topdir\\\"]\")/rbconfig.rb"]

`

And vanagon generic pipeline

Also, Artifacts

@skyamgarp skyamgarp requested review from a team as code owners June 26, 2023 07:35
@@ -50,7 +50,7 @@
pkg.environment 'optflags', settings[:cflags] + ' -O3'
elsif platform.is_macos?
pkg.environment 'optflags', settings[:cflags]
if platform.is_cross_compiled?
if platform.architecture == 'arm64'
# Pin to an older version of ruby@2.5. This can be removed once we're no longer cross-compiling
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 we should be able to remove this conditional in pinning to ruby 2.5?

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 exclude this change after rebasing, see https://github.com/puppetlabs/puppet-runtime/pull/680/files#r1253820661.

Also @tvpartytonight is right that Ruby 2.5 is no longer needed (it was used in Puppet 6 which is EOL). If there isn't a PA ticket filed already about removing 6.x code from the runtime, can you file one and we can delete that code later?

@joshcooper
Copy link
Contributor

joshcooper commented Jul 6, 2023

Since macOS 13 ARM in puppet7 and 8 share many of the same components, e.g. augeas, ruby, etc, we're having to make the same set of changes in this PR and #680. For example, see my comment about cross-compiling in https://github.com/puppetlabs/puppet-runtime/pull/680/files#r1253820661.

I talked with @shubhamshinde360 yesterday and I think we're close to landing his PR. Let's focus on getting that merged and then rebase this PR against upstream/master. I'll also leave some specific comments inline.

@joshcooper
Copy link
Contributor

joshcooper commented Jul 6, 2023

@skyamgarp I was able to reproduce the build failure. The issue is a combination of brew bumping packages to depend on openssl 3 and the fact that we're doing native compiles on macOS 13 ARM.

First, the build fails because it can't find OpenSSL >= 1.0.1, < 3.0.0, note the "strictly less than 3"

openssl:
	Could not be configured. It will not be installed.
	/private/var/folders/zz/zyxvpxvq6csfxvn_n0000000000000/T/tmp.n60vh4wY/ruby-2.7.8/ext/openssl/extconf.rb:111: OpenSSL >= 1.0.1, < 3.0.0 or LibreSSL >= 2.5.0 is required
	Check ext/openssl/mkmf.log for more details.

Next, we can see brew installed OpenSSL 3:

pix-arm64-macos13-1:tmp.n60vh4wY root# cd /Users/test/
pix-arm64-macos13-1:test root# su test /opt/homebrew/bin/brew --prefix openssl
/opt/homebrew/opt/openssl@3
pix-arm64-macos13-1:test root# /opt/homebrew/opt/openssl@3/bin/openssl version
OpenSSL 3.1.1 30 May 2023 (Library: OpenSSL 3.1.1 30 May 2023)

This can be fixed by passing --with-openssl-dir=/opt/puppetlabs/puppet in the ruby configure script, so that ruby uses the openssl we just built earlier on in the build (as opposed to the one brew installed):

cd ruby-2.7.8 && \
        bash configure --enable-shared --enable-bundled-libyaml --disable-install-doc --disable-install-rdoc --prefix=/opt/puppetlabs/puppet --with-opt-dir=/opt/puppetlabs/puppet --with-baseruby=/opt/puppetlabs/puppet/bin/ruby --with-coroutine=arm64 --enable-dtrace --with-openssl-dir=/opt/puppetlabs/puppet

This doesn't break the agent-runtime-main build on macOS 13 ARM, because we're building openssl 3.2.2, which is "close enough" to the version brew installed.

And I think agent-runtime-7.x on macOS 12 Intel doesn't have this issue, because we're getting lucky. We don't have a build_requires dependency on perl so we just use openssl 1.1.x.

Long story short, I think we should be adding --with-openssl-dir=/opt/puppetlabs/puppet on all ruby native compilation builds. I'd suggest adding to just the ruby-2.7.8 component on macOS 13 ARM and then later add it to other native macOS builds for ruby-2.7.8 and ruby-3.2.2.

Also for posterity, homebrew seems to require ruby >= 2.6.10, because it only installs it on macOS 11 and 12, but not 13. And those have different preinstalled ruby versions:

$ for mac in alma-decency.delivery.puppetlabs.net skimpy-framer.delivery.puppetlabs.net reflective-cure.vmpooler-prod.puppet.net; do echo && echo $mac && ssh $mac 'sw_vers && ruby --version'; done

alma-decency.delivery.puppetlabs.net
ProductName:	macOS
ProductVersion:	11.2.1
BuildVersion:	20D75
ruby 2.6.3p62 (2019-04-16 revision 67580) [universal.x86_64-darwin20]

skimpy-framer.delivery.puppetlabs.net
ProductName:	macOS
ProductVersion:	12.2.1
BuildVersion:	21D62
ruby 2.6.8p205 (2021-07-07 revision 67951) [universal.x86_64-darwin21]

reflective-cure.vmpooler-prod.puppet.net
ProductName:		macOS
ProductVersion:		13.3.1
ProductVersionExtra:	(a)
BuildVersion:		22E772610a
ruby 2.6.10p210 (2022-04-12 revision 67958) [universal.x86_64-darwin22]

elsif platform.is_cross_compiled? && platform.is_macos?
elsif platform.is_macos? && platform.architecture == 'arm64'
if platform.os_version.to_i >= 13
pkg.environment 'PATH', '/opt/homebrew/bin:$(PATH):/usr/local/bin'
Copy link
Contributor

@joshcooper joshcooper Jul 6, 2023

Choose a reason for hiding this comment

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

Suggest adding this for macOS 13+ ARM only for now:

      special_flags += " --with-openssl-dir=#{settings[:prefix]} "

And then file a ticket to investigate adding to all native builds later (in both ruby-2.7.8.rb and ruby-3.2.2.rb)

special_flags = " --prefix=#{ruby_dir} --with-opt-dir=#{settings[:prefix]} "
special_flags += "--with-openssl-dir=#{settings[:prefix]} " unless platform.is_cross_compiled?

@skyamgarp skyamgarp force-pushed the PA-5594 branch 3 times, most recently from 37e4f84 to 664fc39 Compare July 11, 2023 07:41
@skyamgarp skyamgarp changed the title (PA_5594) Enable macOS 13 (ARM) puppet-runtime builds for agent-runtime-7.x (PA-5594) Enable macOS 13 (ARM) puppet-runtime builds for agent-runtime-7.x Jul 11, 2023
@joshcooper joshcooper merged commit ae55412 into puppetlabs:master Jul 11, 2023
2 checks passed
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.

3 participants