From dca417bde7fae2a5969134eb57be271b7638c0c5 Mon Sep 17 00:00:00 2001 From: Nobuyoshi Nakada Date: Wed, 22 Jul 2026 20:19:30 +0900 Subject: [PATCH] [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 | 2 +- common.mk | 8 +- configure.ac | 2 +- defs/gmake.mk | 2 +- template/configure-ext.mk.tmpl | 4 +- tool/make-snapshot | 2 +- tool/mkdepend.rb | 118 ++++++++++++++++----- tool/test/test_mkdepend.rb | 128 +++++++++++++++++++---- win32/setup.mak | 2 +- 9 files changed, 210 insertions(+), 58 deletions(-) diff --git a/.github/workflows/check_dependencies.yml b/.github/workflows/check_dependencies.yml index a79b23dee69..14b010658ba 100644 --- a/.github/workflows/check_dependencies.yml +++ b/.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: diff --git a/common.mk b/common.mk index 61d25ec9efa..6997b98dbe8 100644 --- a/common.mk +++ b/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) diff --git a/configure.ac b/configure.ac index 32932c0b997..f636d19e3f1 100644 --- a/configure.ac +++ b/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)' diff --git a/defs/gmake.mk b/defs/gmake.mk index a519a004aa5..6142cfc9e2c 100644 --- a/defs/gmake.mk +++ b/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) diff --git a/template/configure-ext.mk.tmpl b/template/configure-ext.mk.tmpl index 04ac717fb1b..1a959dfe6b5 100644 --- a/template/configure-ext.mk.tmpl +++ b/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| diff --git a/tool/make-snapshot b/tool/make-snapshot index 76a36db5823..43abd664bdc 100755 --- a/tool/make-snapshot +++ b/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 diff --git a/tool/mkdepend.rb b/tool/mkdepend.rb index dcf12fa825f..2a87d32a8fa 100644 --- a/tool/mkdepend.rb +++ b/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 diff --git a/tool/test/test_mkdepend.rb b/tool/test/test_mkdepend.rb index 8f85a4fc7b9..34c03bdc5a4 100644 --- a/tool/test/test_mkdepend.rb +++ b/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 diff --git a/win32/setup.mak b/win32/setup.mak index 90a503baca5..7bb2c65784d 100644 --- a/win32/setup.mak +++ b/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-: