Make the build's Ruby scripts run on Ruby 2.6 through 4.0 - #85
Conversation
gen_html spawned multimarkdown through Kernel#open with a leading "|", which Ruby 3.3 deprecated (Feature #19630) and Ruby 4.0 removed. With a Homebrew Ruby 4 first on PATH the About and Help pages failed to build. Use IO.popen with an argv array instead. gen_html also gains -o: the page is written next to the target and renamed into place, and the CMake rule uses it instead of a shell redirect. A failed run previously left a truncated page that ninja then treated as up to date, so the broken page shipped in the app. gen_credits.rb requires cgi/escape, the only part of cgi it uses and the only part Ruby 4.0 keeps. gen_test no longer appends to a string literal, which Ruby 4 warns will be frozen. Output is byte-identical for all sixteen generated pages under the system Ruby 2.6.10 and Homebrew Ruby 4.0.7. tests/gen_html_test.rb covers the conversion, -o, and the no-partial-output guarantee. Refs #83
tbates
left a comment
There was a problem hiding this comment.
Code review, all five files:
-
textmate/bin/gen_html:97: open("|…", 'r+') → IO.popen([filter, "--nosmart"], 'r+')does exactly the right job here: Skips the shell and compatible with system 2.6 and future 4.0.- I ran the same IO.popen array + r+ + close_write pattern under system Ruby 2.6.10 with /bin/cat standing in for multimarkdown: round-trip passes.✓
-
-o atomic write (tmp file + File.rename): good fix for the truncated-page problem, and stdout behavior is unchanged when -o is omitted.
- One nit: a failed run orphans the .pid.tmp file; harmless, but a retry never cleans it?
-
textmate/cmake/TextMateHelpers.cmake:206: switching the custom command to -o removes the shell-redirect dependency. Robust under the CMake generator.
-
gen_credits.rb (cgi → cgi/escape): correct, CGI.escapeHTML lives in cgi/escape on both 2.6 and 4.0.
-
gen_test ("" → String.new): harmless on 2.6, future-proofs frozen literals.
-
tests/gen_html_test.rb: well designed, runs under whichever Ruby executes it via RbConfig.ruby, covers conversion, -o, and no-clobber-on-failure.- Note the file is standalone minitest, not wired into ctest (the t_*.cc glob will not pick up a .rb file), so CI must invoke ruby tests/gen_html_test.rb explicitly.
I scanned the other build Ruby scripts (expand_variables, extract_changes, show_log, update_changes, generate_available_bundles.rb) for pipe-open, exists?, Fixnum, URI.escape, untaint, bare cgi. All clean. The only other gen_html caller is release.yml:305, using stdout redirect, which keeps working.
Refs #83.
bin/gen_htmlspawned multimarkdown throughKernel#openwith a leading|. Ruby 3.3 deprecated that (Feature #19630) and Ruby 4.0 removed it, so with a Homebrew Ruby 4 first on PATH every#!/usr/bin/env rubybuild script that reaches gen_html fails, which is the About pages, the Help pages, the CHANGELOG page and the release notes. Nothing in the build hardcodes/usr/bin/ruby; the scripts just assumed an older Ruby.Changes:
gen_htmlusesIO.popenwith an argv array.gen_html -o FILEwrites next to the target and renames into place. The CMake rule uses it instead of a shell redirect. Before, a failed run left a truncated page that ninja then treated as up to date, so the broken page shipped in the app (seen while testing: a 295-byte About.html accepted by the next build).gen_credits.rbrequirescgi/escape, the only part of cgi it uses and the only part Ruby 4.0 keeps. Ruby 4.0.7 still ships a stubcgi.rbthat forwards to it, so this is spelling, not a breakage.gen_testinitialises its buffer withString.new; Ruby 4 warns that the literal will be frozen.tests/gen_html_test.rb(minitest, run standalone likebuild_filetype_index_test.rb) covers the conversion,-o, and that a failed run leaves an existing output file untouched.Verification on macOS 27.0 with the system Ruby 2.6.10 and Homebrew Ruby 4.0.7:
ninja Applications/TextMate/md/About/About.htmlsucceeds with Ruby 4.0.7 first on PATH; on main it fails at gen_html:97.tests/gen_html_test.rb: 3 runs, 13 assertions, 0 failures under each Ruby.gen_testoutput unchanged apart from the#linepath;expand_variablesandextract_changeswere already fine on both.