[Misc #22206] Parse mkdepend options with OptionParser

Replace `-s` globals with explicit command options and standard long
option names. Group source selection under `--scope` and represent
output behavior as a single mode so command-line precedence is clear.

Keep command-line parsing and option dispatch inside `Mkdepend`, and
return command success directly from `Mkdepend.main`.

.github/workflows/check_dependencies.yml

@@ -33,7 +33,7 @@ jobs:
           ruby-version: '3.1'
           bundler: none
 
-      - run: ruby tool/mkdepend.rb -all -sources -check
+      - run: ruby tool/mkdepend.rb --scope=all --sources --check
 
       - uses: ./.github/actions/slack
         with:

common.mk

@@ -2006,14 +2006,14 @@ rewindable:
 
 HELP_EXTRA_TASKS = ""
 
-MKDEPEND_FILES = -all
-MKDEPEND_OPTIONS = -sources
+MKDEPEND_FILES = --scope=all
+MKDEPEND_OPTIONS = --sources
 
 fix-depends: PHONY
-	$(BASERUBY) -C $(srcdir) tool/mkdepend.rb $(MKDEPEND_FILES) $(MKDEPEND_OPTIONS) -inplace
+	$(BASERUBY) -C $(srcdir) tool/mkdepend.rb $(MKDEPEND_FILES) $(MKDEPEND_OPTIONS) --inplace
 
 check-depends: PHONY
-	$(BASERUBY) -C $(srcdir) tool/mkdepend.rb -all -sources -check
+	$(BASERUBY) -C $(srcdir) tool/mkdepend.rb --scope=all --sources --check
 
 gc/Makefile:
 	$(MAKEDIRS) $(@D)

configure.ac

@@ -4756,7 +4756,7 @@ AS_IF([test "x$HAVE_BASERUBY" = xyes], [
     rm -rf "$ac_pwd/.deps"
     AS_MKDIR_P([.deps])
     RUBYOPT=- $BASERUBY "$srcdir/tool/mkdepend.rb" \
-        -root="$srcdir" -core -output="$ac_pwd/.deps" ||
+        --root="$srcdir" --scope=core --output="$ac_pwd/.deps" ||
         AC_MSG_ERROR([failed to generate build dependencies])
 ], [
     X_DEPENDENCIES_DIR='$(srcdir)'

defs/gmake.mk

@@ -501,7 +501,7 @@ update-deps:
 	$(eval deps_dir := $(shell mktemp -d)/$(update_deps))
 	$(eval GIT_DIR := $(shell $(GIT_IN_SRC) rev-parse --absolute-git-dir))
 	$(GIT) --git-dir=$(GIT_DIR) worktree add $(deps_dir)
-	$(BASERUBY) -C $(deps_dir) tool/mkdepend.rb -all -sources -inplace
+	$(BASERUBY) -C $(deps_dir) tool/mkdepend.rb --scope=all --sources --inplace
 	$(GIT) -C $(deps_dir) diff --no-ext-diff --ignore-submodules --exit-code || \
 	    $(GIT) -C $(deps_dir) commit --all --message='Update dependencies'
 	$(GIT) --git-dir=$(GIT_DIR) worktree remove $(deps_dir)

template/configure-ext.mk.tmpl

@@ -60,8 +60,8 @@ exts: dependencies
 gems:
 
 dependencies:
-	$(Q)$(MINIRUBY) $(srcdir)/tool/mkdepend.rb -root=$(srcdir) \
-		-extensions -thread_model=$(THREAD_MODEL)<%= " -nmake" if nmake %> -output=.deps
+	$(Q)$(MINIRUBY) $(srcdir)/tool/mkdepend.rb --root=$(srcdir) \
+		--scope=extensions --thread-model=$(THREAD_MODEL)<%= " --nmake" if nmake %> --output=.deps
 
 % exts.each do |t, (o, dirs)|
 %   dirs.each do |d|

tool/make-snapshot

@@ -475,7 +475,7 @@ def package(vcs, rev, destdir, tmp = nil)
       vars.delete("UNICODE_FILES") # for stable branches
       vars["UNICODE_VERSION"] = $unicode_version if $unicode_version
       args = vars.dup
-      args["MKDEPEND_FILES"] = "-core"
+      args["MKDEPEND_FILES"] = "--scope=core"
       args["MKDEPEND_OPTIONS"] = ""
       mk.gsub!(/@([A-Za-z_]\w*)@/) {args.delete($1); vars[$1] || ENV[$1]}
       commonmk.gsub!(/^!(?:include \$\(srcdir\)\/(.*))?/) do

tool/mkdepend.rb

@@ -1,11 +1,11 @@
-#!ruby -s
+#!ruby
 
 # Update dependencies without a configured build or a C compiler:
-#   ruby tool/mkdepend.rb -all -sources -inplace
+#   ruby tool/mkdepend.rb --scope=all --sources --inplace
 # Expand source mappings into full dependencies:
-#   ruby tool/mkdepend.rb -all -inplace
+#   ruby tool/mkdepend.rb --scope=all --inplace
 # Check committed source mappings without modifying files:
-#   ruby tool/mkdepend.rb -all -sources -check
+#   ruby tool/mkdepend.rb --scope=all --sources --check
 # The command can be run from either the source or a build directory.
 #
 # Dependency files can declare inputs that do not exist in the source tree:
@@ -35,6 +35,7 @@
 require 'set'
 require 'tempfile'
 require 'fileutils'
+require 'optparse'
 
 TOP_SRCDIR = File.expand_path("..", __dir__)
 
@@ -199,6 +200,72 @@ end
 class Mkdepend
   attr_reader :root
 
+  def self.main(argv = ARGV, err: $stderr)
+    options, inputs = parse_options(argv)
+  rescue OptionParser::ParseError => error
+    err.puts "#{File.basename($0)}: #{error.message}"
+    err.puts "Try '#{File.basename($0)} --help' for more information."
+    false
+  else
+    execute(inputs, **options)
+  end
+
+  def self.parse_options(argv)
+    options = {}
+    output = nil
+    select_mode = proc do |mode|
+      options[:mode] = mode
+    end
+    parser = OptionParser.new do |opts|
+      opts.banner = "Usage: #{File.basename($0)} [options] [files]"
+      opts.separator ""
+      opts.separator "Input selection:"
+      opts.on("--root=DIR", "source tree root") {|value| options[:root] = value}
+      opts.on("--thread-model=MODEL", "thread model") do |value|
+        options[:thread_model] = value
+      end
+      opts.on(
+        "--scope=SCOPE", [:all, :core, :extensions],
+        "dependency files to process (all, core, extensions)",
+      ) do |value|
+        options[:scope] = value
+      end
+
+      opts.separator ""
+      opts.separator "Update mode (last one wins; default: stdout):"
+      opts.on("--output=DIR", "write dependencies under DIR") do |value|
+        select_mode[:output]
+        output = value
+      end
+      opts.on("--inplace", "update dependency files in place") do
+        select_mode[:inplace]
+      end
+      opts.on("--check", "check dependency files without updating") do
+        select_mode[:check]
+      end
+
+      opts.separator ""
+      opts.separator "Output options:"
+      opts.on("--nmake", "emit NMake dependency rules") do
+        options[:nmake] = true
+      end
+      opts.on("--sources", "emit source mappings") do
+        options[:sources] = true
+      end
+      opts.on("-v", "--verbose", "show scanned sources") do
+        options[:verbose] = true
+      end
+    end
+    inputs = parser.parse(argv)
+    options[:output] = output if options[:mode] == :output
+    [options, inputs]
+  end
+
+  def self.execute(inputs, root: TOP_SRCDIR, thread_model: nil, **options)
+    new(root: root, thread_model: thread_model).run(inputs, **options)
+  end
+  private_class_method :execute
+
   def initialize(root: TOP_SRCDIR, thread_model: nil)
     @root = File.expand_path(root)
     @thread_model = thread_model
@@ -629,7 +696,7 @@ class Mkdepend
     sources.values.map(&:last).uniq.sort.join
   end
 
-  def update_deps(rules, input, group: true)
+  def update_deps(rules, input, group: true, verbose: false)
     sources = dependency_source_map(rules, input)
     targets = rules.scan(%r[^([-.\w/]+)\.(?:\$\(OBJEXT\)|o):]).flatten.uniq
     unless (missing = targets - sources.keys).empty?
@@ -638,7 +705,7 @@ class Mkdepend
     generated = []
     sources.each do |target, src|
       src = resolve_dependency_source(src, input)
-      warn "dependencies for #{src}:" if $verbose
+      warn "dependencies for #{src}:" if verbose
       makedepend(src, generated, target: target, input: input)
     end
     compact_dependencies(generated.join, group: group)
@@ -676,12 +743,17 @@ class Mkdepend
     expected_lines.each {|line| err.puts "    #{line.chomp}"}
   end
 
-  def run(inputs = ARGV, out: $stdout, err: $stderr, output: $output,
-          nmake: $nmake, sources: $sources, inplace: $inplace,
-          check: $check,
-          scope: ($core ? :core :
-                  $extensions ? :extensions :
-                  $all ? :all : nil))
+  def run(inputs = ARGV, out: $stdout, err: $stderr, mode: :stdout,
+          output: nil, nmake: false, sources: false, scope: nil,
+          verbose: false)
+    case mode
+    when :output
+      raise ArgumentError, "output directory is missing" unless output
+    when :stdout, :inplace, :check
+      raise ArgumentError, "output requires output mode" if output
+    else
+      raise ArgumentError, "unknown update mode: #{mode.inspect}"
+    end
     if scope
       inputs = dependency_files(scope).map {|file| File.join(@root, file)}
     end
@@ -702,18 +774,18 @@ class Mkdepend
         expected = if sources
           minimize_deps(current_rules, input)
         else
-          update_deps(current_rules, input, group: !nmake)
+          update_deps(current_rules, input, group: !nmake, verbose: verbose)
         end
         updated = match.pre_match + expected + match.post_match
-        if output
+        if mode == :output
           updated = normalize_dependency_rules(updated) unless nmake
           replace_file(File.join(output, relative_source(input)), updated)
         elsif same_dependency_rules?(current_rules, expected)
           next
-        elsif inplace
+        elsif mode == :inplace
           replace_file(input, updated)
           changed = true
-        elsif check
+        elsif mode == :check
           report_outdated_dependencies(input, current_rules, expected, err: err)
           changed = true
         else
@@ -722,19 +794,15 @@ class Mkdepend
         end
       end
     end
-    if check && changed
-      options = " -sources" if sources
+    if mode == :check && changed
+      options = " --sources" if sources
       err.puts "\nupdate with:"
-      err.puts "  ruby tool/mkdepend.rb -all#{options} -inplace"
+      err.puts "  ruby tool/mkdepend.rb --scope=all#{options} --inplace"
     end
-    !changed | output
+    mode != :check || !changed
   end
 end
 
 if __FILE__ == $0
-  success = Mkdepend.new(
-    root: $root || TOP_SRCDIR,
-    thread_model: $thread_model,
-  ).run
-  exit(false) if $check && !success
+  exit(Mkdepend.main)
 end

tool/test/test_mkdepend.rb

@@ -11,6 +11,80 @@ class TestMkdepend < Test::Unit::TestCase
     @mkdepend ||= Mkdepend.new
   end
 
+  def test_parse_options
+    options, inputs = Mkdepend.parse_options(%w[
+      --root=src --thread-model=win32 --output=.deps
+      --scope=extensions --nmake --sources --verbose depend
+    ])
+
+    assert_equal(
+      {
+        root: "src",
+        thread_model: "win32",
+        output: ".deps",
+        mode: :output,
+        scope: :extensions,
+        nmake: true,
+        sources: true,
+        verbose: true,
+      },
+      options,
+    )
+    assert_equal(["depend"], inputs)
+  end
+
+  def test_parse_options_rejects_unknown_options
+    assert_raise(OptionParser::InvalidOption) do
+      Mkdepend.parse_options(["--unknown"])
+    end
+  end
+
+  def test_parse_options_rejects_unknown_scope
+    assert_raise(OptionParser::InvalidArgument) do
+      Mkdepend.parse_options(["--scope=unknown"])
+    end
+  end
+
+  def test_parse_options_selects_update_mode
+    assert_equal(
+      {mode: :inplace},
+      Mkdepend.parse_options(["--inplace"]).first,
+    )
+    assert_equal(
+      {mode: :check},
+      Mkdepend.parse_options(["--check"]).first,
+    )
+  end
+
+  def test_parse_options_uses_last_update_mode
+    options, = Mkdepend.parse_options(["--output=.deps", "--check"])
+    assert_equal({mode: :check}, options)
+
+    options, = Mkdepend.parse_options(["--inplace", "--output=.deps"])
+    assert_equal({mode: :output, output: ".deps"}, options)
+  end
+
+  def test_help_explains_update_mode_precedence
+    stdout, $stdout = $stdout, StringIO.new
+    exit_status = assert_raise(SystemExit) do
+      Mkdepend.parse_options(["--help"])
+    end
+    assert_true(exit_status.success?)
+    help = $stdout.string
+
+    assert_include(
+      help,
+      "Update mode (last one wins; default: stdout):",
+    )
+    output = help.index("--output=DIR")
+    inplace = help.index("--inplace")
+    check = help.index("--check")
+    assert_operator(output, :<, inplace)
+    assert_operator(inplace, :<, check)
+  ensure
+    $stdout = stdout
+  end
+
   def test_scanner_follows_all_possible_project_headers
     Dir.mktmpdir('mkdepend-scanner') do |dir|
       File.write(File.join(dir, 'main.c'), <<~C)
@@ -251,7 +325,9 @@ class TestMkdepend < Test::Unit::TestCase
 
       output = File.join(dir, '.deps')
       mkdepend = Mkdepend.new(root: dir)
-      assert_true(mkdepend.run([input], output: output, nmake: true))
+      assert_true(
+        mkdepend.run([input], mode: :output, output: output, nmake: true),
+      )
       generated = File.read(File.join(output, 'ext/example/depend'))
       assert_include(generated, 'generated.o: generated.c')
       assert_include(generated, 'generated.o: generated.h')
@@ -370,7 +446,7 @@ class TestMkdepend < Test::Unit::TestCase
     end
   end
 
-  def test_run_selects_extension_files_from_command_line
+  def test_run_selects_extension_files
     Dir.mktmpdir('mkdepend-files') do |root|
       %w[depend ext/example/depend].each do |file|
         path = File.join(root, file)
@@ -379,12 +455,13 @@ class TestMkdepend < Test::Unit::TestCase
       end
 
       Dir.mktmpdir('mkdepend-output') do |output|
-        extensions, $extensions = $extensions, true
-        Mkdepend.new(root: root).run([], output: output)
+        assert_true(
+          Mkdepend.main([
+            "--root=#{root}", "--scope=extensions", "--output=#{output}",
+          ]),
+        )
         assert_false(File.exist?(File.join(output, 'depend')))
         assert_true(File.exist?(File.join(output, 'ext/example/depend')))
-      ensure
-        $extensions = extensions
       end
     end
   end
@@ -630,7 +707,12 @@ class TestMkdepend < Test::Unit::TestCase
       path = File.join(dir, 'depend')
       File.write(path, content)
 
-      assert_false(mkdepend.run([path], sources: true, inplace: true))
+      assert_false(
+        mkdepend.run(
+          [path], sources: true, mode: :check, err: StringIO.new,
+        ),
+      )
+      assert_true(mkdepend.run([path], sources: true, mode: :inplace))
       assert_equal(<<~DEPEND, File.read(path))
         manual: rule
         #{MARK_START}
@@ -660,7 +742,7 @@ class TestMkdepend < Test::Unit::TestCase
       File.chmod(0644, path)
       content = File.read(path)
 
-      assert_false(mkdepend.run([path], inplace: true))
+      assert_true(mkdepend.run([path], mode: :inplace))
 
       assert_include(File.read(path), 'array.$(OBJEXT): $(hdrdir)/ruby/ruby.h')
       assert_equal(0644, File.stat(path).mode & 0777)
@@ -674,7 +756,7 @@ class TestMkdepend < Test::Unit::TestCase
       output = File.join(dir, 'build-deps')
       original = File.read(source)
 
-      assert_true(mkdepend.run([source], output: output))
+      assert_true(mkdepend.run([source], mode: :output, output: output))
       generated = File.read(File.join(output, 'ext/date/depend'))
       assert_include(generated, 'date_core.o: date_core.c')
       assert_match(/date_core\.o.*: \$\(hdrdir\)\/ruby\/ruby\.h/, generated)
@@ -687,7 +769,9 @@ class TestMkdepend < Test::Unit::TestCase
     Dir.mktmpdir('mkdepend-output') do |dir|
       source = File.join(TOP_SRCDIR, 'ext/-test-/thread/id/depend')
 
-      assert_true(mkdepend.run([source], output: dir, nmake: true))
+      assert_true(
+        mkdepend.run([source], mode: :output, output: dir, nmake: true),
+      )
       generated = File.read(File.join(dir, 'ext/-test-/thread/id/depend'))
       assert_include(generated, '{$(VPATH)}id.c')
     end
@@ -697,7 +781,7 @@ class TestMkdepend < Test::Unit::TestCase
     Dir.mktmpdir('mkdepend-output') do |dir|
       source = File.join(TOP_SRCDIR, 'ext/-test-/thread/id/depend')
 
-      assert_true(mkdepend.run([source], output: dir))
+      assert_true(mkdepend.run([source], mode: :output, output: dir))
       generated = File.read(File.join(dir, 'ext/-test-/thread/id/depend'))
       assert_include(generated, 'id.o: id.c')
       assert_not_include(generated, '{$(VPATH)}')
@@ -714,7 +798,7 @@ class TestMkdepend < Test::Unit::TestCase
         File.write(input, "#{MARK_START}\n#{rules}#{MARK_END}\n")
         destination = File.join(output, 'depend')
 
-        assert_true(mkdepend.run([input], output: output))
+        assert_true(mkdepend.run([input], mode: :output, output: output))
         assert_equal(
           mkdepend.normalize_dependency_rules(File.read(input)),
           File.read(destination),
@@ -740,20 +824,20 @@ class TestMkdepend < Test::Unit::TestCase
     assert_include(configure, 'AC_SUBST(X_DEPENDENCIES_DIR)')
     assert_include(configure, "X_DEPENDENCIES_DIR='\$X_DEPENDENCIES_DIR'")
     assert_not_include(configure, 'AC_SUBST(DEPENDENCIES_DIR)')
-    assert_include(configure, '-root="$srcdir"')
-    assert_include(configure, '-core')
+    assert_include(configure, '--root="$srcdir"')
+    assert_include(configure, '--scope=core')
     assert_include(makefile, 'DEPENDENCIES_DIR = @X_DEPENDENCIES_DIR@')
     assert_include(prereq, 's,@X_DEPENDENCIES_DIR@,$(srcdir),g')
     assert_include(gnumakefile, 'include $(DEPENDENCIES_DIR)/depend')
     assert_match(/filter-out .*DEPENDENCIES_DIR.*common_mk_includes/, gnumakefile)
     assert_include(win32, 'DEPENDENCIES_DIR = .deps')
     assert_include(win32, 'DEPENDENCIES_DIR = $(srcdir)')
-    assert_include(setup, '-core')
+    assert_include(setup, '--scope=core')
     assert_not_include(mkmf, 'Mkdepend.new')
-    assert_include(configure_ext, '-extensions')
-    assert_include(configure_ext, '-thread_model=$(THREAD_MODEL)')
+    assert_include(configure_ext, '--scope=extensions')
+    assert_include(configure_ext, '--thread-model=$(THREAD_MODEL)')
     assert_match(
-      %r{tool/mkdepend\.rb.*\n.*-output=\.deps},
+      %r{tool/mkdepend\.rb.*\n.*--output=\.deps},
       configure_ext,
     )
   end
@@ -770,12 +854,12 @@ class TestMkdepend < Test::Unit::TestCase
     assert_not_match(/^fix-depends:/, gmake)
     assert_not_match(/^check-depends:/, gmake)
     assert_match(/rm\.bat -f -r \.deps/, setup)
-    assert_include(setup, '-root=$(srcdir)')
+    assert_include(setup, '--root=$(srcdir)')
     assert_match(
-      %r{tool[\\/]mkdepend\.rb -root=\$\(srcdir\) -core -nmake -output=\.deps},
+      %r{tool[\\/]mkdepend\.rb --root=\$\(srcdir\) --scope=core --nmake --output=\.deps},
       setup,
     )
-    assert_include(snapshot, 'args["MKDEPEND_FILES"] = "-core"')
+    assert_include(snapshot, 'args["MKDEPEND_FILES"] = "--scope=core"')
     assert_include(snapshot, 'args["MKDEPEND_OPTIONS"] = ""')
     assert_include(snapshot, 'make.run("fix-depends")')
   end
@@ -786,7 +870,7 @@ class TestMkdepend < Test::Unit::TestCase
       content = "array.$(OBJEXT): {$(VPATH)}array.c\n"
       File.write(path, content)
 
-      assert_true(mkdepend.run([path], inplace: true))
+      assert_true(mkdepend.run([path], mode: :inplace))
       assert_equal(content, File.read(path))
     end
   end

win32/setup.mak

@@ -58,7 +58,7 @@ prefix = $(prefix:\=/)
 -dependencies-: -baseruby-
 !if "$(HAVE_BASERUBY)" != "no"
 	@$(WIN32DIR:/=\)\rm.bat -f -r .deps
-	@$(BASERUBY:/=\) $(srcdir)/tool/mkdepend.rb -root=$(srcdir) -core -nmake -output=.deps
+	@$(BASERUBY:/=\) $(srcdir)/tool/mkdepend.rb --root=$(srcdir) --scope=core --nmake --output=.deps
 !endif
 
 -gmp-: