Bug #16668: Syntax error in tool/vcs.rb

• Status: Closed
• Assignee: -
• Target version: -
• ruby -v: ruby 2.6.5p114 (2019-10-01 revision 67812) [i686-linux]
• Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN

[humptydumpty (Jens With)]

./tool/file2lastrev.rb:10:in `require': /home/imptst/ruby/ruby-2.6.5/tool/vcs.rb:127: syntax error (SyntaxError)

  def self.detect(path, uplevel_limit: 0)

Should be:

def self.detect(path, uplevel_limit= 0)

[Hanmac (Hans Mackowiak)]

something might be wrong with your ruby version, because def self.detect(path, uplevel_limit: 0) is valid ruby code

[nobu (Nobuyoshi Nakada)]

• Description updated (diff)
• Status changed from Open to Feedback

Probably you use ruby 1.9 or earlier as BASERUBY, right?

[humptydumpty (Jens With)]

Yes, ruby -v:
ruby 1.8.1 (2003-12-25) [i386-linux-gnu]

which ruby:
/usr/bin/ruby

Extract from my log file:

	BASERUBY = /usr/bin/ruby
	CC = gcc
	LD = ld
	LDSHARED = gcc -shared
	CFLAGS = -O3 -ggdb3 -Wall -Wdeclaration-after-statement -Wdeprecated-declarations -Wpointer-arith -Wwrite-strings -Wimplicit-fallthrough=0 -Wmissing-noreturn -Wno-long-long -Wno-packed-bitfield-compat -Wno-parentheses-equality -Wno-unused-parameter -Wno-unused-value -Wunused-variable -std=gnu99 
	XCFLAGS = -D_FORTIFY_SOURCE=2 -DRUBY_EXPORT -fPIE -DCANONICALIZATION_FOR_MATHN
	CPPFLAGS =   -I. -I.ext/include/i686-linux -I./include -I. -I./enc/unicode/12.1.0 
	DLDFLAGS = -pie  
	SOLIBS = -lpthread -lrt -lrt -lrt -lgmp -ldl -lcrypt -lm 
	LANG = C
	LC_ALL = C
	LC_CTYPE = 
gcc (GCC) 3.4.6 20060404 (Red Hat 3.4.6-11)
Copyright (C) 2006 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

./tool/file2lastrev.rb:10:in `require': /home/imptst/ruby/ruby-2.6.5/tool/vcs.rb:127: syntax error (SyntaxError)
  def self.detect(path, uplevel_limit: 0)
                                      ^
/home/imptst/ruby/ruby-2.6.5/tool/vcs.rb:153: dynamic constant assignment
  NullDevice = defined?(IO::NULL) ? IO::NULL :
              ^
/home/imptst/ruby/ruby-2.6.5/tool/vcs.rb:223: class definition in method body
/home/imptst/ruby/ruby-2.6.5/tool/vcs.rb:225: dynamic constant assignment
    COMMAND = ENV['SVN'] || 'svn'
             ^
/home/imptst/ruby/ruby-2.6.5/tool/vcs.rb:355: class definition in method body
/home/imptst/ruby/ruby-2.6.5/tool/vcs.rb:357: dynamic constant assignment
    COMMAND = ENV["GIT"] || 'git'
             ^`
/home/imptst/ruby/ruby-2.6.5/tool/vcs.rb:418: dynamic constant assignment
    Branch = Struct.new(:to_str)
            ^
/home/imptst/ruby/ruby-2.6.5/tool/vcs.rb:534: syntax error	from ./tool/file2lastrev.rb:10
make: [.revision.time] Error 1 (ignored)
./revision.h unchanged

[nobu (Nobuyoshi Nakada)]

Build from a tarball, or configure with --with-baseruby=no option.

[humptydumpty (Jens With)]

Thanks for the info, though I am wondering, why configure doesn't take care of that automatically.

Isn't this a tarball what I get by downloading via https://cache.ruby-lang.org/pub/ruby/2.6/ruby-2.6.5.tar.gz?

[jeremyevans0 (Jeremy Evans)]

I was able to reproduce this by building from the 2.6.5 tarball, and making ruby be ruby 1.8.7, without any use of --with-baseruby. The tarball automatically sets BASERUBY based on ruby even though it isn't needed in most tarball builds.

Note that this is not a build error. Ruby 2.6.5 continues to build after the ./revision.h unchanged at the end, and completes building successfully in my environment.

detect in this case was switched back from keyword arguments to an option hash in 6b5e712361cca8559ed66d5c1106e888c5971d39. That's back when we planned to support BASERUBY being Ruby before 2.0. However, 46acd0075d80c2f886498f089fde1e9d795d50c4 requires BASERUBY to be at least Ruby 2.2 (see eaa011ffdb55a315a6b35a52c3636c673f9ea836).

It would be nice to check for BASERUBY only if it was needed. However, even when using the tarball, there are cases where it is needed (cross compiling), so you can't use a tarball build as simple indicator of whether BASERUBY is required. Restructuring configure.ac to only check/set BASERUBY when it is needed may not be a simple task.

I do think it would make sense to automatically fail if older ruby versions that are not usable as BASERUBY are explicitly specified or implicitly picked up. Additionally, there is no reason to check for whether BASERUBY supports --disable=gems, since all supported versions of BASERUBY should.

tool/lib/vcs.rb has code to explicitly support Ruby before 2.2, and I'm guessing that can be removed now that we require BASERUBY to be at least Ruby 2.2.

I've added a pull request with the changes I think should be made: https://github.com/ruby/ruby/pull/2947

[humptydumpty (Jens With)]

Thanks for your response!

I realised it didn't affect the build, but for me a proper configure is showcasing the quality of a package.
Whenever I am confronted with sloppy configuration I am doubting the quality of everything else.

[ko1 (Koichi Sasada)]

• Related to Misc #16671: BASERUBY version policy added

• Status changed from Feedback to Closed

Applied in changeset git|9a3371be8f5274d1dca64b93c7b9a379e32ea2f5.

---

Check that BASERUBY is at least Ruby 2.2 in configure

BASERUBY needs to be at least Ruby 2.2 since
46acd0075d80c2f886498f089fde1e9d795d50c4.

I think it's better to explicitly fail early as soon as BASERUBY
is used in this case, versus trying to debug later failures.

This modifies things to check both implicitly use of ruby in the
PATH as BASERUBY, and explicitly specified older versions of ruby
when using --with-baseruby.

Fixes [Bug #16668]