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

Allow RUBY_TARGET to be inherited from sudo #513

Merged
merged 1 commit into from
Jan 31, 2025

Conversation

stanhu
Copy link
Contributor

@stanhu stanhu commented Jan 30, 2025

Previously RUBY_TARGET was never passed along because sudo -u was run. Add this variable to the sudoers file to preserve this varaible.

@stanhu
Copy link
Contributor Author

stanhu commented Jan 30, 2025

For some reason, I'm seeing this when I test this locally:

$ bundle exec rb-sys-dock --platform x86_64-linux-gnu --tag test --build
🐳 Using version test of the Docker image
platform is x86_64-linux-gnu
🐳 Running default build command (rake native:x86_64-linux-gnu gem)
Bundle complete! 11 Gemfile dependencies, 23 gems now installed.
Bundled gems are installed into `/tmp/rb-sys-dock/bundle`
rake aborted!
Don't know how to build task 'native:x86_64-linux-gnu' (See the list of available tasks with `rake --tasks`)
/tmp/rb-sys-dock/bundle/ruby/3.4.0/gems/rake-13.2.1/exe/rake:27:in '<top (required)>'
/usr/local/rbenv/versions/3.4.1/bin/bundle:25:in 'Kernel#load'
/usr/local/rbenv/versions/3.4.1/bin/bundle:25:in '<main>'
(See full trace by running task with --trace)
rake aborted!
Don't know how to build task 'native:x86_64-linux-gnu' (See the list of available tasks with `rake --tasks`)

(See full trace by running task with --trace)

@stanhu
Copy link
Contributor Author

stanhu commented Jan 30, 2025

I guess that problem happens even on the tagged versions, so I might be doing something wrong.

UPDATE: Figured it out; I need to run this from the oxi-test dir, not the rb-sys dir. 🤦

@stanhu stanhu marked this pull request as draft January 30, 2025 19:39
@stanhu
Copy link
Contributor Author

stanhu commented Jan 30, 2025

Oh, I see:

  1. The runas script quietly runs /usr/bin/sudo -u "$USER" -H BASH_ENV=/etc/rubybashrc -- "$@". This clears all environment varaiables and uses the rubybashrc script to set them up.
  2. RUBY_TARGET is needed inside ./rb-sys/gem/lib/rb_sys/extensiontask.rb to determine the cross-compile target.

RUBY_TARGET has been a static value, but now we're trying to make it dynamic. This does make me think that it might be easier to just create a separate Dockerfile here.

@stanhu stanhu force-pushed the sh-fix-ruby-target-try3 branch 2 times, most recently from 7d0b23f to 60c59f5 Compare January 30, 2025 20:59
@stanhu stanhu changed the title Avoid relying on RUBY_TARGET env variable Directly pass in RUBY_TARGET to Rake task Jan 30, 2025
@stanhu stanhu marked this pull request as ready for review January 30, 2025 20:59
@stanhu stanhu force-pushed the sh-fix-ruby-target-try3 branch from 60c59f5 to c7b6f47 Compare January 30, 2025 21:01
@@ -310,8 +314,8 @@ def default_command_to_run(input_args)
input_cmd = input_args.empty? ? "true" : input_args.join(" ")

if OPTIONS[:build]
with_bundle = +"test -f Gemfile && bundle install && #{input_cmd} && bundle exec rake native:$RUBY_TARGET gem"
without_bundle = "#{input_cmd} && rake native:$RUBY_TARGET gem"
with_bundle = +"test -f Gemfile && bundle install && #{input_cmd} && bundle exec rake native:#{ruby_target} gem RUBY_TARGET=#{ruby_target}"
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 not quite sure why adding the environment variable at the end works, but why adding it via export RUBY_TARGET=#{ruby_target} && before this line does not.

Copy link
Contributor Author

@stanhu stanhu Jan 30, 2025

Choose a reason for hiding this comment

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

@ianks
Copy link
Collaborator

ianks commented Jan 30, 2025

We

Oh, I see:

  1. The runas script quietly runs /usr/bin/sudo -u "$USER" -H BASH_ENV=/etc/rubybashrc -- "$@". This clears all environment varaiables and uses the rubybashrc script to set them up.
  2. RUBY_TARGET is needed inside ./rb-sys/gem/lib/rb_sys/extensiontask.rb to determine the cross-compile target.

RUBY_TARGET has been a static value, but now we're trying to make it dynamic. This does make me think that it might be easier to just create a separate Dockerfile here.

We may need to add it to sudoers.d?

$ docker run --platform linux/amd64 --rm -it rbsys/aarch64-linux:0.9.109 /bin/bash
root@c4a3fea5b9b9:/# cat /etc/sudoers.d/rb-sys-dock        
Defaults        env_keep += "BUNDLE_PATH RB_SYS_CARGO_TARGET_DIR RAKEOPT"

@ianks
Copy link
Collaborator

ianks commented Jan 30, 2025

We should probably make it include the defaults from rake-compiler-dock too...

root@c4a3fea5b9b9:/# cat /etc/sudoers.d/rake-compiler-dock 
Defaults        env_keep += "http_proxy https_proxy ftp_proxy GEM_PRIVATE_KEY_PASSPHRASE RCD_HOST_RUBY_PLATFORM RCD_HOST_RUBY_VERSION RCD_IMAGE RUBY_CC_VERSION LD_LIBRARY_PATH DEVTOOLSET_ROOTPATH SOURCE_DATE_EPOCH"

@stanhu
Copy link
Contributor Author

stanhu commented Jan 30, 2025

Yeah, I solved it by directly inserting RUBY_TARGET into the Rakefile, which bypasses the need to set this all up in the environment.

@ianks
Copy link
Collaborator

ianks commented Jan 30, 2025

Yeah, I solved it by directly inserting RUBY_TARGET into the Rakefile, which bypasses the need to set this all up in the environment.

The one downside is that things wont work properly without --build, although maybe not a big deal

@stanhu
Copy link
Contributor Author

stanhu commented Jan 31, 2025

Ah, interesting. Let me try this out.

Previously RUBY_TARGET was never passed along because `sudo -u` was
run. Add this variable to the sudoers file to preserve this varaible.
@stanhu stanhu force-pushed the sh-fix-ruby-target-try3 branch from c7b6f47 to 092b887 Compare January 31, 2025 02:12
@stanhu stanhu changed the title Directly pass in RUBY_TARGET to Rake task Allow RUBY_TARGET to be inherited from sudo Jan 31, 2025
@stanhu
Copy link
Contributor Author

stanhu commented Jan 31, 2025

Thanks for that tip. I think this works.

@ianks ianks merged commit c21f978 into oxidize-rb:main Jan 31, 2025
56 checks passed
@stanhu
Copy link
Contributor Author

stanhu commented Jan 31, 2025

@ianks Thanks for your help! Could you tag a new release so I can try upgrading a gem again?

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.

2 participants