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

The extra option including a space is splitted to execute the command. #215

Closed
junaruga opened this issue May 24, 2023 · 4 comments
Closed

Comments

@junaruga
Copy link

junaruga commented May 24, 2023

The ruby/openssl is using the rake-compiler. Here is my test on Fedora 37.

$ ruby -v
ruby 3.3.0dev (2023-05-22T03:58:17Z master bd786e7896) [x86_64-linux]
$ git clone https://github.com/ruby/openssl.git

$ cd openssl

Install the gems. The --standalone option is my favorite. :)

$ bundle install --standalone

$ bundle list | grep rake-compiler
  * rake-compiler (1.2.1)

Then the following command with --with-cflags='"-Wundef -Werror"' propagates the -Wundef -Werror to the compiler gcc command. Note that the MAKEFLAGS="V=1" is an option to print the compiler commands.

$ MAKEFLAGS="V=1" bundle exec rake compile -- --enable-debug --with-cflags='"-Wundef -Werror"'
...
/home/jaruga/.local/ruby-bd786e7896/bin/ruby -I. -r.rake-compiler-siteconf.rb ../../../../ext/openssl/extconf.rb -- --enable-debug --with-cflags="-Wundef -Werror"
...
gcc -I. -I/home/jaruga/.local/ruby-bd786e7896/include/ruby-3.3.0+0/x86_64-linux -I/home/jaruga/.local/ruby-bd786e7896/include/ruby-3.3.0+0/ruby/backward -I/home/jaruga/.local/ruby-bd786e7896/include/ruby-3.3.0+0 -I../../../../ext/openssl  -DRUBY_EXTCONF_H=\"extconf.h\"    -fPIC -Wundef -Werror   -o openssl_missing.o -c ../../../../ext/openssl/openssl_missing.c
...

However, the following command with --with-cflags='-Wundef -Werror' (without additional quotes) separates the --with-cflags=-Wundef from the -Werror. And it doesn't propagate the -Werror to the gcc command line.

$ MAKEFLAGS="V=1" bundle exec rake compile -- --enable-debug --with-cflags='-Wundef -Werror'
...
/home/jaruga/.local/ruby-bd786e7896/bin/ruby -I. -r.rake-compiler-siteconf.rb ../../../../ext/openssl/extconf.rb -- --enable-debug --with-cflags=-Wundef -Werror
...
gcc -I. -I/home/jaruga/.local/ruby-bd786e7896/include/ruby-3.3.0+0/x86_64-linux -I/home/jaruga/.local/ruby-bd786e7896/include/ruby-3.3.0+0/ruby/backward -I/home/jaruga/.local/ruby-bd786e7896/include/ruby-3.3.0+0 -I../../../../ext/openssl  -DRUBY_EXTCONF_H=\"extconf.h\"    -fPIC -Wundef   -o openssl_missing.o -c ../../../../ext/openssl/openssl_missing.c
...

Is it an expected behavior? Is it a bug?

Debug

I debugged with debug gem by adding the gem "debug" to the Gemfile of the ruby/opnessl. And set the breakpoint below.

$ diff -u extensiontask.rb.orig extensiontask.rb
--- extensiontask.rb.orig	2023-05-24 16:00:55.649111625 +0200
+++ extensiontask.rb	2023-05-24 16:31:21.646223039 +0200
@@ -214,6 +214,9 @@
           cmd.push(*extra_options)
         end
 
+        require 'debug'
+        binding.break
+
         chdir tmp_path do
           # FIXME: Rake is broken for multiple arguments system() calls.
           # Add current directory to the search path of Ruby
$ MAKEFLAGS="V=1" bundle exec rake compile -- --enable-debug --with-cflags='-Wundef -Werror'

I checked at the breakpoint binding.break.

(rdbg) f    # frame command
=> 218|         binding.break
=>#0	block {|t=<Rake::FileTask tmp/x86_64-linux/openssl/...|} in define_compile_tasks at ~/var/git/ruby/openssl/bundle/ruby/3.3.0+0/gems/rake-compiler-1.2.1/lib/rake/extensiontask.rb:218
(rdbg) p extra_options    # command
=> ["--", "--enable-debug", "--with-cflags=-Wundef -Werror"]
(rdbg) p cmd    # command
=> ["/home/jaruga/.local/ruby-bd786e7896/bin/ruby", "-I.", "-r.rake-compiler-siteconf.rb", #<Pathname:../../../../ext/openssl/extconf.rb>, "--", "--enable-debug", "--with-cflags=-Wundef -Werror"]
(rdbg) p cmd.join(' ')    # command
=> "/home/jaruga/.local/ruby-bd786e7896/bin/ruby -I. -r.rake-compiler-siteconf.rb ../../../../ext/openssl/extconf.rb -- --enable-debug --with-cflags=-Wundef -Werror"

So, it seems that cmd.join(' ') causes this issue.

sh cmd.join(' ')

@junaruga junaruga changed the title The extra_option including a space is splited to execute the command. The extra_option including a space is splitted to execute the command. May 24, 2023
@kou kou closed this as completed in 8a830fa May 25, 2023
@kou
Copy link
Member

kou commented May 25, 2023

Could you try 1.2.2?

@junaruga junaruga changed the title The extra_option including a space is splitted to execute the command. The extra option including a space is splitted to execute the command. May 25, 2023
@junaruga
Copy link
Author

Thank you for fixing this issue and releasing the new version 1.2.2!

$ bundle list | grep rake-compiler
  * rake-compiler (1.2.2)

I confirmed that the following command propagates the -Wundef -Werror properly to the gcc command lines.

$ MAKEFLAGS="V=1" bundle exec rake compile -- --enable-debug --with-cflags='-Wundef -Werror'
...
/home/jaruga/.local/ruby-bd786e7896/bin/ruby -I. -r.rake-compiler-siteconf.rb ../../../../ext/openssl/extconf.rb -- --enable-debug --with-cflags=-Wundef -Werror
...
gcc -I. -I/home/jaruga/.local/ruby-bd786e7896/include/ruby-3.3.0+0/x86_64-linux -I/home/jaruga/.local/ruby-bd786e7896/include/ruby-3.3.0+0/ruby/backward -I/home/jaruga/.local/ruby-bd786e7896/include/ruby-3.3.0+0 -I../../../../ext/openssl  -DRUBY_EXTCONF_H=\"extconf.h\"    -fPIC -Wundef -Werror   -o openssl_missing.o -c ../../../../ext/openssl/openssl_missing.c
...

Note that the --with-cflags=-Wundef -Werror in the printed command line below doesn't have quotes. So, it may confuse users.

/home/jaruga/.local/ruby-bd786e7896/bin/ruby -I. -r.rake-compiler-siteconf.rb ../../../../ext/openssl/extconf.rb -- --enable-debug --with-cflags=-Wundef -Werror

And the following command with the additional quotes that succeeded with the rake-compiler 1.2.1 failed in the rake-compiler 1.2.2.

$ MAKEFLAGS="V=1" bundle exec rake compile -- --enable-debug --with-cflags='"-Wundef -Werror"'
mkdir -p tmp/x86_64-linux/openssl/3.3.0
cd tmp/x86_64-linux/openssl/3.3.0
/home/jaruga/.local/ruby-bd786e7896/bin/ruby -I. -r.rake-compiler-siteconf.rb ../../../../ext/openssl/extconf.rb -- --enable-debug --with-cflags="-Wundef -Werror"
checking for rb_io_maybe_wait(0, Qnil, Qnil, Qnil) in ruby/io.h... *** ../../../../ext/openssl/extconf.rb failed ***
Could not create Makefile due to some reason, probably lack of necessary
libraries and/or headers.  Check the mkmf.log file for more details.  You may
need configuration options.

Provided configuration options:
	--with-opt-dir
	--without-opt-dir
	--with-opt-include
	--without-opt-include=${opt-dir}/include
	--with-opt-lib
	--without-opt-lib=${opt-dir}/lib
	--with-make-prog
	--without-make-prog
	--srcdir=../../../../ext/openssl
	--curdir
	--ruby=/home/jaruga/.local/ruby-bd786e7896/bin/$(RUBY_BASE_NAME)
	--with-openssl-dir
	--without-openssl-dir
	--with-openssl-include
	--without-openssl-include=${openssl-dir}/include
	--with-openssl-lib
	--without-openssl-lib=${openssl-dir}/lib
	--with-kerberos-dir
	--without-kerberos-dir
	--with-kerberos-include
	--without-kerberos-include=${kerberos-dir}/include
	--with-kerberos-lib
	--without-kerberos-lib=${kerberos-dir}/lib
	--with-debug
	--without-debug
	--enable-debug
/home/jaruga/.local/ruby-bd786e7896/lib/ruby/3.3.0+0/mkmf.rb:480:in `try_do': The compiler failed to generate an executable file. (RuntimeError)
You have to install development tools first.

	from /home/jaruga/.local/ruby-bd786e7896/lib/ruby/3.3.0+0/mkmf.rb:573:in `try_link0'
	from /home/jaruga/.local/ruby-bd786e7896/lib/ruby/3.3.0+0/mkmf.rb:591:in `try_link'
	from /home/jaruga/.local/ruby-bd786e7896/lib/ruby/3.3.0+0/mkmf.rb:809:in `try_func'
	from /home/jaruga/.local/ruby-bd786e7896/lib/ruby/3.3.0+0/mkmf.rb:1110:in `block in have_func'
	from /home/jaruga/.local/ruby-bd786e7896/lib/ruby/3.3.0+0/mkmf.rb:983:in `block in checking_for'
	from /home/jaruga/.local/ruby-bd786e7896/lib/ruby/3.3.0+0/mkmf.rb:344:in `block (2 levels) in postpone'
	from /home/jaruga/.local/ruby-bd786e7896/lib/ruby/3.3.0+0/mkmf.rb:314:in `open'
	from /home/jaruga/.local/ruby-bd786e7896/lib/ruby/3.3.0+0/mkmf.rb:344:in `block in postpone'
	from /home/jaruga/.local/ruby-bd786e7896/lib/ruby/3.3.0+0/mkmf.rb:314:in `open'
	from /home/jaruga/.local/ruby-bd786e7896/lib/ruby/3.3.0+0/mkmf.rb:340:in `postpone'
	from /home/jaruga/.local/ruby-bd786e7896/lib/ruby/3.3.0+0/mkmf.rb:982:in `checking_for'
	from /home/jaruga/.local/ruby-bd786e7896/lib/ruby/3.3.0+0/mkmf.rb:1109:in `have_func'
	from ../../../../ext/openssl/extconf.rb:30:in `<main>'
rake aborted!
Command failed with status (1): [/home/jaruga/.local/ruby-bd786e7896/bin/ru...]
/home/jaruga/var/git/ruby/openssl/bundle/ruby/3.3.0+0/gems/rake-compiler-1.2.2/lib/rake/extensiontask.rb:218:in `block (2 levels) in define_compile_tasks'
/home/jaruga/var/git/ruby/openssl/bundle/ruby/3.3.0+0/gems/rake-compiler-1.2.2/lib/rake/extensiontask.rb:217:in `block in define_compile_tasks'
/home/jaruga/var/git/ruby/openssl/bundle/ruby/3.3.0+0/gems/rake-13.0.6/exe/rake:27:in `<top (required)>'
/home/jaruga/.local/ruby-bd786e7896/bin/bundle:25:in `load'
/home/jaruga/.local/ruby-bd786e7896/bin/bundle:25:in `<main>'
Tasks: TOP => compile => compile:x86_64-linux => compile:openssl:x86_64-linux => copy:openssl:x86_64-linux:3.3.0 => tmp/x86_64-linux/openssl/3.3.0/openssl.so => tmp/x86_64-linux/openssl/3.3.0/Makefile
(See full trace by running task with --trace)

I debugged with debug gem again. The mkmf.rb receives the "\"-Wundef -Werror\"". And I think this is a correct behavior.

@@ -2601,6 +2601,10 @@
     if (w = rbconfig['CC_WRAPPER']) and !w.empty? and !File.executable?(w)
       rbconfig['CC_WRAPPER'] = config['CC_WRAPPER'] = ''
     end
+
+    require 'debug'
+    binding.break
+
     $CFLAGS = with_config("cflags", arg_config("CFLAGS", config["CFLAGS"])).dup
     $CXXFLAGS = (with_config("cxxflags", arg_config("CXXFLAGS", config["CXXFLAGS"]))||'').dup
     $ARCH_FLAG = with_config("arch_flag", arg_config("ARCH_FLAG", config["ARCH_FLAG"])).dup
(rdbg) f    # frame command
=>2606|     binding.break
=>#0	MakeMakefile#init_mkmf(config={"DESTDIR"=>"", "MAJOR"=>"3", "MINOR"=>"3..., rbconfig={"DESTDIR"=>"", "MAJOR"=>"3", "MINOR"=>"3...) at ~/.local/ruby-bd786e7896/lib/ruby/3.3.0+0/mkmf.rb:2606
(rdbg) p with_config("cflags", arg_config("CFLAGS", config["CFLAGS"])).dup    # command
=> "\"-Wundef -Werror\""

@kou
Copy link
Member

kou commented May 25, 2023

Note that the --with-cflags=-Wundef -Werror in the printed command line below doesn't have quotes. So, it may confuse users.

It's done by Rake: https://github.com/ruby/rake/blob/1d3579b01484a59bdf1a3e542662ea4ab7e79f7c/lib/rake/file_utils.rb#L80

If you think that we should improve this, please report it to Rake.

I think this is a correct behavior.

Right.

@junaruga
Copy link
Author

If you think that we should improve this, please report it to Rake.

Thank you for checking the part causing the issue. I reported the issue on ruby/rake#501 at the rake project.

flavorjones added a commit to flavorjones/rake-compiler that referenced this issue May 29, 2023
flavorjones added a commit to flavorjones/rake-compiler that referenced this issue May 30, 2023
kou pushed a commit that referenced this issue May 30, 2023
__Context__

In rake-compiler 1.2.1 and earlier, if an extension did something like
this, rake-compiler handled it gracefully:

```ruby
  Rake::ExtensionTask.new("gem", spec) do |ext|
    ext.config_options << ENV["EXTOPTS"] # this is probably going to be nil
  end
```

It is OK to do this because in 1.2.1 and earlier, the command array was
executed as:

```
  sh cmd.join(' ')
```

However, this was changed in 1.2.2 to

```
  sh *cmd
```

which of course is preferable. However, this breaks extensions [like
nokogiri that do something silly like
above](https://github.com/sparklemotion/nokogiri/actions/runs/5109347425/jobs/9184057367).

__Details__

I've fixed Nokogiri (see
sparklemotion/nokogiri#2894) but I do think that
there may be other gems impacted by this change.

This PR does a few things:

- extract a method `make_makefile_cmd` so that we can unit test it
- backfill basic test coverage
- backfill test for #215
- write a test for this `nil` case, and fix it by returning the result
of `cmd.compact`

This is a big PR because of the extraction and test coverage. Please let
me know if you would prefer this to be done a different way.
konsolebox added a commit to konsolebox/digest-kangarootwelve-ruby that referenced this issue Jun 24, 2023
konsolebox added a commit to konsolebox/digest-xxhash-ruby that referenced this issue Jul 21, 2023
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

No branches or pull requests

2 participants