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

Fix reserve stack segmentation fault when building on RHEL5 or below #523

Merged
merged 2 commits into from Dec 4, 2015

Conversation

robbkidd
Copy link
Contributor

@robbkidd robbkidd commented Dec 2, 2015

Currently only affects 2.1.7 and 2.2.3. This patch taken from the fix
in Ruby trunk and expected to be included in future point releases.

https://redmine.ruby-lang.org/issues/11602

Currently only affects 2.1.7 and 2.2.3. This patch taken from the fix
in Ruby trunk and expected to be included in future point releases.

https://redmine.ruby-lang.org/issues/11602
@robbkidd
Copy link
Contributor Author

robbkidd commented Dec 2, 2015

smacks forehead

@scotthain
Copy link

Patch itself looks good, once you clean up the assignment :)

@curiositycasualty
Copy link

👍 pendng 👌 from Travis

@robbkidd
Copy link
Contributor Author

robbkidd commented Dec 2, 2015

It takes sooo long to re-run the test builds. 😢

@robbkidd
Copy link
Contributor Author

robbkidd commented Dec 3, 2015

Only patched 2.1.7 and 2.2.3 on CentOS 5. Did not apply patch on CentOS 6, 7 or Ubuntu.

[vagrant@default-centos-511]$ find /var/cache/omnibus/src/ruby-* -name thread_pthread.c
/var/cache/omnibus/src/ruby-2.1.6/thread_pthread.c
/var/cache/omnibus/src/ruby-2.1.7/thread_pthread.c
/var/cache/omnibus/src/ruby-2.2.2/thread_pthread.c
/var/cache/omnibus/src/ruby-2.2.3/thread_pthread.c
[vagrant@default-centos-511]$ grep "ensure alloca" /var/cache/omnibus/src/ruby-*/thread_pthread.c
/var/cache/omnibus/src/ruby-2.1.7/thread_pthread.c:     limit[0] = 0; /* ensure alloca is called */
/var/cache/omnibus/src/ruby-2.2.3/thread_pthread.c:     limit[0] = 0; /* ensure alloca is called */

@scotthain
Copy link

LGTM 👍
@chef/omnibus-maintainers

@yzl
Copy link
Contributor

yzl commented Dec 3, 2015

👍

# Currently only affects 2.1.7 and 2.2.3. This patch taken from the fix
# in Ruby trunk and expected to be included in future point releases.
# https://redmine.ruby-lang.org/issues/11602
if ohai['platform_family'] == 'rhel' &&
Copy link
Contributor

Choose a reason for hiding this comment

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

chef-sugar's DSL is fully exposed so you could do:

if rhel? && 

Additionally you could compare using a true constraint with:

if rhel? && Chef::Sugar::Version(ohai['platform_version']).satisfies?('> 6')

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm getting a variety of NameErrors trying to use sugar within omnibus-supermarket which is pinned to a pretty old version of omnibus. I'm inclined to keep the longer, but working direct calls to ohai.

@lamont-granquist
Copy link
Contributor

👍 although also 👍 on sethc's comment on better using sugar.

@lamont-granquist
Copy link
Contributor

based on omnibus-supermarket needing a yak shave to get it up to current standards lets just merge this one to unblock the supermarket team, without worrying about the sugar syntax fixes...

@robbkidd
Copy link
Contributor Author

robbkidd commented Dec 4, 2015

please

@schisamo
Copy link
Contributor

schisamo commented Dec 4, 2015

@robbkidd just :shipit:

@robbkidd
Copy link
Contributor Author

robbkidd commented Dec 4, 2015

I'm still not used to merging my own PRs.

robbkidd added a commit that referenced this pull request Dec 4, 2015
Fix reserve stack segmentation fault when building on RHEL5 or below
@robbkidd robbkidd merged commit 4fdb4e1 into master Dec 4, 2015
@robbkidd robbkidd deleted the robb/patch-ruby-thread branch December 4, 2015 20:40
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.

None yet

6 participants