From e1ea9da5072db332084aa12866a06952a1c15851 Mon Sep 17 00:00:00 2001 From: Lukas Oberhuber Date: Tue, 26 Jul 2022 00:00:45 +0100 Subject: [PATCH 01/42] Conceptual draft of dsym support for macos --- Library/Homebrew/extend/os/mac/keg.rb | 17 +++++++++++++++++ Library/Homebrew/formula_installer.rb | 15 +++++++++++++++ Library/Homebrew/keg.rb | 4 ++++ Library/Homebrew/shims/super/cc | 6 ++++++ 4 files changed, 42 insertions(+) diff --git a/Library/Homebrew/extend/os/mac/keg.rb b/Library/Homebrew/extend/os/mac/keg.rb index 67abdc6ab3..cfe0b4f648 100644 --- a/Library/Homebrew/extend/os/mac/keg.rb +++ b/Library/Homebrew/extend/os/mac/keg.rb @@ -62,4 +62,21 @@ class Keg #{result.stderr} EOS end + + def dsymutil + binary_executable_or_library_files.each { |file| dsymutil_binary(file) } + end + + def dsymutil_binary(file) + odebug "Extracting symbols #{file}" + + result = system_command("dsymutil", args: [file]) + return if result.success? + + # If it fails again, error out + onoe <<~EOS + Failed to extract symbols from #{file}: + #{result.stderr} + EOS + end end diff --git a/Library/Homebrew/formula_installer.rb b/Library/Homebrew/formula_installer.rb index 03486bb730..611ec46643 100644 --- a/Library/Homebrew/formula_installer.rb +++ b/Library/Homebrew/formula_installer.rb @@ -39,6 +39,7 @@ class FormulaInstaller attr_predicate :installed_as_dependency?, :installed_on_request? attr_predicate :show_summary_heading?, :show_header? attr_predicate :force_bottle?, :ignore_deps?, :only_deps?, :interactive?, :git?, :force?, :overwrite?, :keep_tmp? + attr_predicate :debug_symbols? attr_predicate :verbose?, :debug?, :quiet? def initialize( @@ -71,6 +72,8 @@ class FormulaInstaller @force = force @overwrite = overwrite @keep_tmp = keep_tmp + # Just for this proof of concept + @debug_symbols = keep_tmp @link_keg = !formula.keg_only? || link_keg @show_header = show_header @ignore_deps = ignore_deps @@ -802,6 +805,8 @@ class FormulaInstaller post_install end + dsymutil(keg) if debug_symbols? + # Updates the cache for a particular formula after doing an install CacheStoreDatabase.use(:linkage) do |db| break unless db.created? @@ -1326,4 +1331,14 @@ class FormulaInstaller #{SPDX.license_expression_to_string formula.license}. EOS end + + sig { params(keg: Keg).void } + def dsymutil(keg) + keg.dsymutil + # TODO + # rescue Keg::DsymError => e + rescue RuntimeError => e + ofail "Failed to extract debugging symbols for #{formula.full_name}" + puts e + end end diff --git a/Library/Homebrew/keg.rb b/Library/Homebrew/keg.rb index c0ef58fc89..7c7defe7e0 100644 --- a/Library/Homebrew/keg.rb +++ b/Library/Homebrew/keg.rb @@ -483,6 +483,8 @@ class Keg ObserverPathnameExtension.n end + def dsymutil; end + def remove_oldname_opt_record return unless oldname_opt_record return if oldname_opt_record.resolved_path != path @@ -531,6 +533,8 @@ class Keg def codesign_patched_binary(file); end + def dsymutil_binary(file); end + private def resolve_any_conflicts(dst, dry_run: false, verbose: false, overwrite: false) diff --git a/Library/Homebrew/shims/super/cc b/Library/Homebrew/shims/super/cc index 7d68f9fc5f..aed92991f9 100755 --- a/Library/Homebrew/shims/super/cc +++ b/Library/Homebrew/shims/super/cc @@ -292,6 +292,7 @@ class Cmd args.concat(optflags) unless runtime_cpu_detection? args.concat(archflags) args << "-std=#{@arg0}" if /c[89]9/.match?(@arg0) + args << "-g" if debug_symbols? args end @@ -399,6 +400,11 @@ class Cmd config.include?("w") end + def debug_symbols? + mac? + # && config.include?("D") + end + def canonical_path(path) path = Pathname.new(path) path = path.realpath if path.exist? From 2d4c792b0b6e74c06b9bc58ec93dbce50e89b67e Mon Sep 17 00:00:00 2001 From: Lukas Oberhuber Date: Tue, 26 Jul 2022 10:00:05 +0100 Subject: [PATCH 02/42] Make one function --- Library/Homebrew/extend/os/mac/keg.rb | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/Library/Homebrew/extend/os/mac/keg.rb b/Library/Homebrew/extend/os/mac/keg.rb index cfe0b4f648..a6c03b82d7 100644 --- a/Library/Homebrew/extend/os/mac/keg.rb +++ b/Library/Homebrew/extend/os/mac/keg.rb @@ -64,19 +64,17 @@ class Keg end def dsymutil - binary_executable_or_library_files.each { |file| dsymutil_binary(file) } - end + binary_executable_or_library_files.each do |file| + odebug "Extracting symbols #{file}" - def dsymutil_binary(file) - odebug "Extracting symbols #{file}" + result = system_command("dsymutil", args: [file]) + next if result.success? - result = system_command("dsymutil", args: [file]) - return if result.success? - - # If it fails again, error out - onoe <<~EOS - Failed to extract symbols from #{file}: - #{result.stderr} - EOS + # If it fails again, error out + onoe <<~EOS + Failed to extract symbols from #{file}: + #{result.stderr} + EOS + end end end From f4cb9a40a6d5f14659b337beedfd6b537fb14e31 Mon Sep 17 00:00:00 2001 From: Lukas Oberhuber Date: Tue, 26 Jul 2022 11:26:03 +0100 Subject: [PATCH 03/42] remove macos specific dummy call Co-authored-by: Carlo Cabrera <30379873+carlocab@users.noreply.github.com> --- Library/Homebrew/keg.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/Library/Homebrew/keg.rb b/Library/Homebrew/keg.rb index 7c7defe7e0..f28c6da545 100644 --- a/Library/Homebrew/keg.rb +++ b/Library/Homebrew/keg.rb @@ -533,8 +533,6 @@ class Keg def codesign_patched_binary(file); end - def dsymutil_binary(file); end - private def resolve_any_conflicts(dst, dry_run: false, verbose: false, overwrite: false) From 74dd365a56917ea5d948ae05586bcfe1a420b577 Mon Sep 17 00:00:00 2001 From: Lukas Oberhuber Date: Tue, 26 Jul 2022 12:13:38 +0100 Subject: [PATCH 04/42] Hardcoded symbol production Needs to be toggled by the `--debug-symbols` flag instead of hard coded --- Library/Homebrew/extend/ENV/super.rb | 11 ++++++++++- Library/Homebrew/shims/super/cc | 3 +-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/Library/Homebrew/extend/ENV/super.rb b/Library/Homebrew/extend/ENV/super.rb index c89a4604c5..c199714866 100644 --- a/Library/Homebrew/extend/ENV/super.rb +++ b/Library/Homebrew/extend/ENV/super.rb @@ -100,6 +100,7 @@ module Superenv # d - Don't strip -march=. Use only in formulae that # have runtime detection of CPU features. # w - Pass -no_weak_imports to the linker + # D - Generate debugging information # # These flags will also be present: # a - apply fix for apr-1-config path @@ -282,7 +283,9 @@ module Superenv sig { returns(String) } def determine_cccfg - "" + # TODO: temporarily hard code symbol generation + # "" + "D" end public @@ -331,6 +334,12 @@ module Superenv append_to_cccfg "g" if compiler == :clang end + sig { void } + def debug_symbols + # TODO: how do I get this method called? + append_to_cccfg "D" if OS.mac? + end + # @private sig { void } def refurbish_args diff --git a/Library/Homebrew/shims/super/cc b/Library/Homebrew/shims/super/cc index aed92991f9..830f569ea5 100755 --- a/Library/Homebrew/shims/super/cc +++ b/Library/Homebrew/shims/super/cc @@ -401,8 +401,7 @@ class Cmd end def debug_symbols? - mac? - # && config.include?("D") + config.include?("D") end def canonical_path(path) From d195f225224031ee556aa84513506d3840ad0efb Mon Sep 17 00:00:00 2001 From: Lukas Oberhuber Date: Tue, 26 Jul 2022 12:15:53 +0100 Subject: [PATCH 05/42] Connecting up `--debug-symbols` flag This connects the calling of dsymutil and the retention of temporary files. Still need to connect compilation to flag. --- Library/Homebrew/build.rb | 7 ++++--- Library/Homebrew/cli/args.rbi | 3 +++ Library/Homebrew/cmd/install.rb | 7 +++++++ Library/Homebrew/cmd/reinstall.rb | 7 +++++++ Library/Homebrew/cmd/upgrade.rb | 7 +++++++ Library/Homebrew/extend/ENV/shared.rb | 3 +++ Library/Homebrew/extend/os/mac/keg.rb | 2 +- Library/Homebrew/formula.rb | 4 ++-- Library/Homebrew/formula_installer.rb | 9 +++++++-- Library/Homebrew/install.rb | 2 ++ Library/Homebrew/reinstall.rb | 2 ++ Library/Homebrew/upgrade.rb | 7 +++++++ 12 files changed, 52 insertions(+), 8 deletions(-) diff --git a/Library/Homebrew/build.rb b/Library/Homebrew/build.rb index b432fdca82..5ee369389a 100644 --- a/Library/Homebrew/build.rb +++ b/Library/Homebrew/build.rb @@ -127,9 +127,10 @@ class Build formula.update_head_version formula.brew( - fetch: false, - keep_tmp: args.keep_tmp?, - interactive: args.interactive?, + fetch: false, + keep_tmp: args.keep_tmp?, + debug_symbols: args.debug_symbols?, + interactive: args.interactive?, ) do with_env( # For head builds, HOMEBREW_FORMULA_PREFIX should include the commit, diff --git a/Library/Homebrew/cli/args.rbi b/Library/Homebrew/cli/args.rbi index 32b1cd8e6e..af6802f6ac 100644 --- a/Library/Homebrew/cli/args.rbi +++ b/Library/Homebrew/cli/args.rbi @@ -87,6 +87,9 @@ module Homebrew sig { returns(T::Boolean) } def keep_tmp?; end + sig { returns(T::Boolean) } + def debug_symbols?; end + sig { returns(T::Boolean) } def overwrite?; end diff --git a/Library/Homebrew/cmd/install.rb b/Library/Homebrew/cmd/install.rb index cff794979e..6848e982b5 100644 --- a/Library/Homebrew/cmd/install.rb +++ b/Library/Homebrew/cmd/install.rb @@ -91,6 +91,11 @@ module Homebrew [:switch, "--keep-tmp", { description: "Retain the temporary files created during installation.", }], + [:switch, "--debug-symbols", { + depends_on: "--build-from-source", + description: "Generates debugging symbols during build. Source will be in temporary directory " \ + "which is retained. (see --keep-tmp)", + }], [:switch, "--build-bottle", { description: "Prepare the formula for eventual bottling during installation, skipping any " \ "post-install steps.", @@ -232,6 +237,7 @@ module Homebrew git: args.git?, interactive: args.interactive?, keep_tmp: args.keep_tmp?, + debug_symbols: args.debug_symbols?, force: args.force?, overwrite: args.overwrite?, debug: args.debug?, @@ -247,6 +253,7 @@ module Homebrew build_from_source_formulae: args.build_from_source_formulae, interactive: args.interactive?, keep_tmp: args.keep_tmp?, + debug_symbols: args.debug_symbols?, force: args.force?, debug: args.debug?, quiet: args.quiet?, diff --git a/Library/Homebrew/cmd/reinstall.rb b/Library/Homebrew/cmd/reinstall.rb index bc6bca5053..bb8b4844cc 100644 --- a/Library/Homebrew/cmd/reinstall.rb +++ b/Library/Homebrew/cmd/reinstall.rb @@ -57,6 +57,11 @@ module Homebrew [:switch, "--keep-tmp", { description: "Retain the temporary files created during installation.", }], + [:switch, "--debug-symbols", { + depends_on: "--build-from-source", + description: "Generates debugging symbols during build. Source will be in temporary directory " \ + "which is retained. (see --keep-tmp)", + }], [:switch, "--display-times", { env: :display_install_times, description: "Print install times for each formula at the end of the run.", @@ -115,6 +120,7 @@ module Homebrew build_from_source_formulae: args.build_from_source_formulae, interactive: args.interactive?, keep_tmp: args.keep_tmp?, + debug_symbols: args.debug_symbols?, force: args.force?, debug: args.debug?, quiet: args.quiet?, @@ -132,6 +138,7 @@ module Homebrew build_from_source_formulae: args.build_from_source_formulae, interactive: args.interactive?, keep_tmp: args.keep_tmp?, + debug_symbols: args.debug_symbols?, force: args.force?, debug: args.debug?, quiet: args.quiet?, diff --git a/Library/Homebrew/cmd/upgrade.rb b/Library/Homebrew/cmd/upgrade.rb index ec7520a7c3..3496c7274b 100644 --- a/Library/Homebrew/cmd/upgrade.rb +++ b/Library/Homebrew/cmd/upgrade.rb @@ -68,6 +68,11 @@ module Homebrew [:switch, "--keep-tmp", { description: "Retain the temporary files created during installation.", }], + [:switch, "--debug-symbols", { + depends_on: "--build-from-source", + description: "Generates debugging symbols during build. Source will be in temporary directory " \ + "which is retained. (see --keep-tmp)", + }], [:switch, "--display-times", { env: :display_install_times, description: "Print install times for each package at the end of the run.", @@ -185,6 +190,7 @@ module Homebrew build_from_source_formulae: args.build_from_source_formulae, interactive: args.interactive?, keep_tmp: args.keep_tmp?, + debug_symbols: args.debug_symbols?, force: args.force?, debug: args.debug?, quiet: args.quiet?, @@ -200,6 +206,7 @@ module Homebrew build_from_source_formulae: args.build_from_source_formulae, interactive: args.interactive?, keep_tmp: args.keep_tmp?, + debug_symbols: args.debug_symbols?, force: args.force?, debug: args.debug?, quiet: args.quiet?, diff --git a/Library/Homebrew/extend/ENV/shared.rb b/Library/Homebrew/extend/ENV/shared.rb index 381ceccfdb..bccd15aaf5 100644 --- a/Library/Homebrew/extend/ENV/shared.rb +++ b/Library/Homebrew/extend/ENV/shared.rb @@ -315,6 +315,9 @@ module SharedEnvExtension sig { void } def permit_arch_flags; end + sig { void } + def debug_symbols; end + # @private sig { params(cc: T.any(Symbol, String)).returns(T::Boolean) } def compiler_any_clang?(cc = compiler) diff --git a/Library/Homebrew/extend/os/mac/keg.rb b/Library/Homebrew/extend/os/mac/keg.rb index a6c03b82d7..a1fde91b14 100644 --- a/Library/Homebrew/extend/os/mac/keg.rb +++ b/Library/Homebrew/extend/os/mac/keg.rb @@ -67,7 +67,7 @@ class Keg binary_executable_or_library_files.each do |file| odebug "Extracting symbols #{file}" - result = system_command("dsymutil", args: [file]) + result = system_command("dsymutil", args: [file], print_stderr: false) next if result.success? # If it fails again, error out diff --git a/Library/Homebrew/formula.rb b/Library/Homebrew/formula.rb index 17488e0ed1..87084de97d 100644 --- a/Library/Homebrew/formula.rb +++ b/Library/Homebrew/formula.rb @@ -1273,11 +1273,11 @@ class Formula # Yields |self,staging| with current working directory set to the uncompressed tarball # where staging is a {Mktemp} staging context. # @private - def brew(fetch: true, keep_tmp: false, interactive: false) + def brew(fetch: true, keep_tmp: false, debug_symbols: false, interactive: false) @prefix_returns_versioned_prefix = true active_spec.fetch if fetch stage(interactive: interactive) do |staging| - staging.retain! if keep_tmp + staging.retain! if keep_tmp || debug_symbols prepare_patches fetch_patches if fetch diff --git a/Library/Homebrew/formula_installer.rb b/Library/Homebrew/formula_installer.rb index 611ec46643..52e1e9881c 100644 --- a/Library/Homebrew/formula_installer.rb +++ b/Library/Homebrew/formula_installer.rb @@ -59,6 +59,7 @@ class FormulaInstaller git: false, interactive: false, keep_tmp: false, + debug_symbols: false, cc: nil, options: Options.new, force: false, @@ -72,8 +73,7 @@ class FormulaInstaller @force = force @overwrite = overwrite @keep_tmp = keep_tmp - # Just for this proof of concept - @debug_symbols = keep_tmp + @debug_symbols = debug_symbols @link_keg = !formula.keg_only? || link_keg @show_header = show_header @ignore_deps = ignore_deps @@ -691,6 +691,7 @@ class FormulaInstaller include_test_formulae: @include_test_formulae, build_from_source_formulae: @build_from_source_formulae, keep_tmp: keep_tmp?, + debug_symbols: debug_symbols?, force: force?, debug: debug?, quiet: quiet?, @@ -744,6 +745,7 @@ class FormulaInstaller include_test_formulae: @include_test_formulae, build_from_source_formulae: @build_from_source_formulae, keep_tmp: keep_tmp?, + debug_symbols: debug_symbols?, force: force?, debug: debug?, quiet: quiet?, @@ -877,6 +879,9 @@ class FormulaInstaller args << "--debug" if debug? args << "--cc=#{@cc}" if @cc args << "--keep-tmp" if keep_tmp? + args << "--debug-symbols" if debug_symbols? + # Avoids dependecy error on flag + args << "--build-from-source" if build_from_source? && debug_symbols? if @env.present? args << "--env=#{@env}" diff --git a/Library/Homebrew/install.rb b/Library/Homebrew/install.rb index 9ab565d431..3d838dac3e 100644 --- a/Library/Homebrew/install.rb +++ b/Library/Homebrew/install.rb @@ -269,6 +269,7 @@ module Homebrew git: false, interactive: false, keep_tmp: false, + debug_symbols: false, force: false, overwrite: false, debug: false, @@ -293,6 +294,7 @@ module Homebrew git: git, interactive: interactive, keep_tmp: keep_tmp, + debug_symbols: debug_symbols, force: force, overwrite: overwrite, debug: debug, diff --git a/Library/Homebrew/reinstall.rb b/Library/Homebrew/reinstall.rb index 2c9c0ee430..f260d041b7 100644 --- a/Library/Homebrew/reinstall.rb +++ b/Library/Homebrew/reinstall.rb @@ -16,6 +16,7 @@ module Homebrew build_from_source_formulae: [], interactive: false, keep_tmp: false, + debug_symbols: false, force: false, debug: false, quiet: false, @@ -48,6 +49,7 @@ module Homebrew git: git, interactive: interactive, keep_tmp: keep_tmp, + debug_symbols: debug_symbols, force: force, debug: debug, quiet: quiet, diff --git a/Library/Homebrew/upgrade.rb b/Library/Homebrew/upgrade.rb index c9f0acc576..54e3223c18 100644 --- a/Library/Homebrew/upgrade.rb +++ b/Library/Homebrew/upgrade.rb @@ -24,6 +24,7 @@ module Homebrew build_from_source_formulae: [], interactive: false, keep_tmp: false, + debug_symbols: false, force: false, debug: false, quiet: false, @@ -61,6 +62,7 @@ module Homebrew build_from_source_formulae: build_from_source_formulae, interactive: interactive, keep_tmp: keep_tmp, + debug_symbols: debug_symbols, force: force, debug: debug, quiet: quiet, @@ -128,6 +130,7 @@ module Homebrew build_from_source_formulae: [], interactive: false, keep_tmp: false, + debug_symbols: false, force: false, debug: false, quiet: false, @@ -161,6 +164,7 @@ module Homebrew build_from_source_formulae: build_from_source_formulae, interactive: interactive, keep_tmp: keep_tmp, + debug_symbols: debug_symbols, force: force, debug: debug, quiet: quiet, @@ -254,6 +258,7 @@ module Homebrew build_from_source_formulae: [], interactive: false, keep_tmp: false, + debug_symbols: false, force: false, debug: false, quiet: false, @@ -339,6 +344,7 @@ module Homebrew build_from_source_formulae: build_from_source_formulae, interactive: interactive, keep_tmp: keep_tmp, + debug_symbols: debug_symbols, force: force, debug: debug, quiet: quiet, @@ -407,6 +413,7 @@ module Homebrew build_from_source_formulae: build_from_source_formulae + [formula.full_name], interactive: interactive, keep_tmp: keep_tmp, + debug_symbols: debug_symbols, force: force, debug: debug, quiet: quiet, From 91065b9ddd24bcc3c279b1b120ec34a2b3810f8b Mon Sep 17 00:00:00 2001 From: Lukas Oberhuber Date: Tue, 26 Jul 2022 16:15:26 +0100 Subject: [PATCH 06/42] Improve flag passing for debug-symbols --- Library/Homebrew/formula_installer.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/Library/Homebrew/formula_installer.rb b/Library/Homebrew/formula_installer.rb index 52e1e9881c..318c513b28 100644 --- a/Library/Homebrew/formula_installer.rb +++ b/Library/Homebrew/formula_installer.rb @@ -879,9 +879,11 @@ class FormulaInstaller args << "--debug" if debug? args << "--cc=#{@cc}" if @cc args << "--keep-tmp" if keep_tmp? - args << "--debug-symbols" if debug_symbols? - # Avoids dependecy error on flag - args << "--build-from-source" if build_from_source? && debug_symbols? + + if debug_symbols? + args << "--debug-symbols" + args << "--build-from-source" + end if @env.present? args << "--env=#{@env}" From c2a95f077fe7bb7b89ca1e4b5cc9c3a2d0cf93cd Mon Sep 17 00:00:00 2001 From: Lukas Oberhuber Date: Tue, 26 Jul 2022 17:18:01 +0100 Subject: [PATCH 07/42] Compiler `-g` flag set based on `--debug-symbols` --- Library/Homebrew/build.rb | 10 ++++++---- Library/Homebrew/extend/ENV.rb | 14 ++++++++++++-- Library/Homebrew/extend/ENV/shared.rb | 14 ++++++++++---- Library/Homebrew/extend/ENV/std.rb | 9 ++++++++- Library/Homebrew/extend/ENV/super.rb | 18 ++++++++++++------ .../Homebrew/extend/os/linux/extend/ENV/std.rb | 12 ++++++++++-- .../extend/os/linux/extend/ENV/super.rb | 12 ++++++++++-- .../extend/os/mac/extend/ENV/shared.rb | 12 ++++++++++-- .../Homebrew/extend/os/mac/extend/ENV/std.rb | 12 ++++++++++-- .../Homebrew/extend/os/mac/extend/ENV/super.rb | 12 ++++++++++-- 10 files changed, 98 insertions(+), 27 deletions(-) diff --git a/Library/Homebrew/build.rb b/Library/Homebrew/build.rb index 5ee369389a..aef1cc3df4 100644 --- a/Library/Homebrew/build.rb +++ b/Library/Homebrew/build.rb @@ -79,10 +79,11 @@ class Build ENV.deps = formula_deps ENV.run_time_deps = run_time_deps ENV.setup_build_environment( - formula: formula, - cc: args.cc, - build_bottle: args.build_bottle?, - bottle_arch: args.bottle_arch, + formula: formula, + cc: args.cc, + build_bottle: args.build_bottle?, + bottle_arch: args.bottle_arch, + debug_symbols: args.debug_symbols?, ) reqs.each do |req| req.modify_build_environment( @@ -96,6 +97,7 @@ class Build cc: args.cc, build_bottle: args.build_bottle?, bottle_arch: args.bottle_arch, + debug_symbols: args.debug_symbols?, ) reqs.each do |req| req.modify_build_environment( diff --git a/Library/Homebrew/extend/ENV.rb b/Library/Homebrew/extend/ENV.rb index 12ef1af60d..40bb592a87 100644 --- a/Library/Homebrew/extend/ENV.rb +++ b/Library/Homebrew/extend/ENV.rb @@ -40,12 +40,22 @@ module EnvActivation _block: T.proc.returns(T.untyped), ).returns(T.untyped) } - def with_build_environment(env: nil, cc: nil, build_bottle: false, bottle_arch: nil, &_block) + def with_build_environment( + env: nil, + cc: nil, + build_bottle: false, + bottle_arch: nil, + debug_symbols: false, + &_block + ) old_env = to_hash.dup tmp_env = to_hash.dup.extend(EnvActivation) T.cast(tmp_env, EnvActivation).activate_extensions!(env: env) T.cast(tmp_env, T.any(Superenv, Stdenv)) - .setup_build_environment(cc: cc, build_bottle: build_bottle, bottle_arch: bottle_arch) + .setup_build_environment( + cc: cc, build_bottle: build_bottle, bottle_arch: bottle_arch, + debug_symbols: debug_symbols, + ) replace(tmp_env) begin diff --git a/Library/Homebrew/extend/ENV/shared.rb b/Library/Homebrew/extend/ENV/shared.rb index bccd15aaf5..f31400733c 100644 --- a/Library/Homebrew/extend/ENV/shared.rb +++ b/Library/Homebrew/extend/ENV/shared.rb @@ -40,13 +40,22 @@ module SharedEnvExtension build_bottle: T.nilable(T::Boolean), bottle_arch: T.nilable(String), testing_formula: T::Boolean, + debug_symbols: T.nilable(T::Boolean), ).void } - def setup_build_environment(formula: nil, cc: nil, build_bottle: false, bottle_arch: nil, testing_formula: false) + def setup_build_environment( + formula: nil, + cc: nil, + build_bottle: false, + bottle_arch: nil, + testing_formula: false, + debug_symbols: false + ) @formula = formula @cc = cc @build_bottle = build_bottle @bottle_arch = bottle_arch + @debug_symbols = debug_symbols reset end private :setup_build_environment @@ -315,9 +324,6 @@ module SharedEnvExtension sig { void } def permit_arch_flags; end - sig { void } - def debug_symbols; end - # @private sig { params(cc: T.any(Symbol, String)).returns(T::Boolean) } def compiler_any_clang?(cc = compiler) diff --git a/Library/Homebrew/extend/ENV/std.rb b/Library/Homebrew/extend/ENV/std.rb index ba4caa1a80..fb3cc6d570 100644 --- a/Library/Homebrew/extend/ENV/std.rb +++ b/Library/Homebrew/extend/ENV/std.rb @@ -23,7 +23,14 @@ module Stdenv testing_formula: T::Boolean, ).void } - def setup_build_environment(formula: nil, cc: nil, build_bottle: false, bottle_arch: nil, testing_formula: false) + def setup_build_environment( + formula: nil, + cc: nil, + build_bottle: false, + bottle_arch: nil, + testing_formula: false, + debug_symbols: false + ) super self["HOMEBREW_ENV"] = "std" diff --git a/Library/Homebrew/extend/ENV/super.rb b/Library/Homebrew/extend/ENV/super.rb index c199714866..3ddaaad7e6 100644 --- a/Library/Homebrew/extend/ENV/super.rb +++ b/Library/Homebrew/extend/ENV/super.rb @@ -57,7 +57,14 @@ module Superenv testing_formula: T::Boolean, ).void } - def setup_build_environment(formula: nil, cc: nil, build_bottle: false, bottle_arch: nil, testing_formula: false) + def setup_build_environment( + formula: nil, + cc: nil, + build_bottle: false, + bottle_arch: nil, + testing_formula: false, + debug_symbols: false + ) super send(compiler) @@ -87,6 +94,8 @@ module Superenv self["HOMEBREW_DEPENDENCIES"] = determine_dependencies self["HOMEBREW_FORMULA_PREFIX"] = @formula.prefix unless @formula.nil? + set_debug_symbols if debug_symbols + # The HOMEBREW_CCCFG ENV variable is used by the ENV/cc tool to control # compiler flag stripping. It consists of a string of characters which act # as flags. Some of these flags are mutually exclusive. @@ -283,9 +292,7 @@ module Superenv sig { returns(String) } def determine_cccfg - # TODO: temporarily hard code symbol generation - # "" - "D" + "" end public @@ -335,8 +342,7 @@ module Superenv end sig { void } - def debug_symbols - # TODO: how do I get this method called? + def set_debug_symbols append_to_cccfg "D" if OS.mac? end diff --git a/Library/Homebrew/extend/os/linux/extend/ENV/std.rb b/Library/Homebrew/extend/os/linux/extend/ENV/std.rb index 76d564cd59..73bc4e2a91 100644 --- a/Library/Homebrew/extend/os/linux/extend/ENV/std.rb +++ b/Library/Homebrew/extend/os/linux/extend/ENV/std.rb @@ -2,10 +2,18 @@ # frozen_string_literal: true module Stdenv - def setup_build_environment(formula: nil, cc: nil, build_bottle: false, bottle_arch: nil, testing_formula: false) + def setup_build_environment( + formula: nil, + cc: nil, + build_bottle: false, + bottle_arch: nil, + testing_formula: false, + debug_symbols: false + ) generic_setup_build_environment( formula: formula, cc: cc, build_bottle: build_bottle, - bottle_arch: bottle_arch, testing_formula: testing_formula + bottle_arch: bottle_arch, testing_formula: testing_formula, + debug_symbols: debug_symbols, ) prepend_path "CPATH", HOMEBREW_PREFIX/"include" diff --git a/Library/Homebrew/extend/os/linux/extend/ENV/super.rb b/Library/Homebrew/extend/os/linux/extend/ENV/super.rb index bbfc418214..074f842099 100644 --- a/Library/Homebrew/extend/os/linux/extend/ENV/super.rb +++ b/Library/Homebrew/extend/os/linux/extend/ENV/super.rb @@ -15,10 +15,18 @@ module Superenv end # @private - def setup_build_environment(formula: nil, cc: nil, build_bottle: false, bottle_arch: nil, testing_formula: false) + def setup_build_environment( + formula: nil, + cc: nil, + build_bottle: false, + bottle_arch: nil, + testing_formula: false, + debug_symbols: false + ) generic_setup_build_environment( formula: formula, cc: cc, build_bottle: build_bottle, - bottle_arch: bottle_arch, testing_formula: testing_formula + bottle_arch: bottle_arch, testing_formula: testing_formula, + debug_symbols: debug_symbols, ) self["HOMEBREW_OPTIMIZATION_LEVEL"] = "O2" self["HOMEBREW_DYNAMIC_LINKER"] = determine_dynamic_linker_path diff --git a/Library/Homebrew/extend/os/mac/extend/ENV/shared.rb b/Library/Homebrew/extend/os/mac/extend/ENV/shared.rb index a690791acc..7db7a5f8fd 100644 --- a/Library/Homebrew/extend/os/mac/extend/ENV/shared.rb +++ b/Library/Homebrew/extend/os/mac/extend/ENV/shared.rb @@ -4,10 +4,18 @@ module SharedEnvExtension extend T::Sig - def setup_build_environment(formula: nil, cc: nil, build_bottle: false, bottle_arch: nil, testing_formula: false) + def setup_build_environment( + formula: nil, + cc: nil, + build_bottle: false, + bottle_arch: nil, + testing_formula: false, + debug_symbols: false + ) generic_shared_setup_build_environment( formula: formula, cc: cc, build_bottle: build_bottle, - bottle_arch: bottle_arch, testing_formula: testing_formula + bottle_arch: bottle_arch, testing_formula: testing_formula, + debug_symbols: debug_symbols, ) # Normalise the system Perl version used, where multiple may be available diff --git a/Library/Homebrew/extend/os/mac/extend/ENV/std.rb b/Library/Homebrew/extend/os/mac/extend/ENV/std.rb index d40fcec500..7368ec4696 100644 --- a/Library/Homebrew/extend/os/mac/extend/ENV/std.rb +++ b/Library/Homebrew/extend/os/mac/extend/ENV/std.rb @@ -10,10 +10,18 @@ module Stdenv ["#{HOMEBREW_LIBRARY}/Homebrew/os/mac/pkgconfig/#{MacOS.version}"] end - def setup_build_environment(formula: nil, cc: nil, build_bottle: false, bottle_arch: nil, testing_formula: false) + def setup_build_environment( + formula: nil, + cc: nil, + build_bottle: false, + bottle_arch: nil, + testing_formula: false, + debug_symbols: false + ) generic_setup_build_environment( formula: formula, cc: cc, build_bottle: build_bottle, - bottle_arch: bottle_arch, testing_formula: testing_formula + bottle_arch: bottle_arch, testing_formula: testing_formula, + debug_symbols: debug_symbols, ) append "LDFLAGS", "-Wl,-headerpad_max_install_names" diff --git a/Library/Homebrew/extend/os/mac/extend/ENV/super.rb b/Library/Homebrew/extend/os/mac/extend/ENV/super.rb index c05535a800..258970240e 100644 --- a/Library/Homebrew/extend/os/mac/extend/ENV/super.rb +++ b/Library/Homebrew/extend/os/mac/extend/ENV/super.rb @@ -85,7 +85,14 @@ module Superenv end # @private - def setup_build_environment(formula: nil, cc: nil, build_bottle: false, bottle_arch: nil, testing_formula: false) + def setup_build_environment( + formula: nil, + cc: nil, + build_bottle: false, + bottle_arch: nil, + testing_formula: false, + debug_symbols: false + ) sdk = formula ? MacOS.sdk_for_formula(formula) : MacOS.sdk is_xcode_sdk = sdk&.source == :xcode @@ -102,7 +109,8 @@ module Superenv generic_setup_build_environment( formula: formula, cc: cc, build_bottle: build_bottle, - bottle_arch: bottle_arch, testing_formula: testing_formula + bottle_arch: bottle_arch, testing_formula: testing_formula, + debug_symbols: debug_symbols, ) # Filter out symbols known not to be defined since GNU Autotools can't From 6551b25b07a82758209a75152ac4b27494f81e5c Mon Sep 17 00:00:00 2001 From: Lukas Oberhuber Date: Tue, 26 Jul 2022 17:56:11 +0100 Subject: [PATCH 08/42] Compile with symbols when requested on all plats --- Library/Homebrew/extend/ENV/super.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Library/Homebrew/extend/ENV/super.rb b/Library/Homebrew/extend/ENV/super.rb index 3ddaaad7e6..e95f0d2997 100644 --- a/Library/Homebrew/extend/ENV/super.rb +++ b/Library/Homebrew/extend/ENV/super.rb @@ -343,7 +343,7 @@ module Superenv sig { void } def set_debug_symbols - append_to_cccfg "D" if OS.mac? + append_to_cccfg "D" end # @private From a578068dd55caa4e9ec6544dc93fa216d47dcf93 Mon Sep 17 00:00:00 2001 From: Lukas Oberhuber Date: Tue, 26 Jul 2022 18:01:22 +0100 Subject: [PATCH 09/42] Unlikely but ensures reasonable error --- Library/Homebrew/formula_installer.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/Library/Homebrew/formula_installer.rb b/Library/Homebrew/formula_installer.rb index 318c513b28..18c8e94334 100644 --- a/Library/Homebrew/formula_installer.rb +++ b/Library/Homebrew/formula_installer.rb @@ -1342,8 +1342,6 @@ class FormulaInstaller sig { params(keg: Keg).void } def dsymutil(keg) keg.dsymutil - # TODO - # rescue Keg::DsymError => e rescue RuntimeError => e ofail "Failed to extract debugging symbols for #{formula.full_name}" puts e From c4422503041ffb13423e95ddadcc99c735b282e9 Mon Sep 17 00:00:00 2001 From: Lukas Oberhuber Date: Tue, 26 Jul 2022 19:27:23 +0100 Subject: [PATCH 10/42] Increase Metrics/BlockLength limit for `install_args` Instead of disabling the cop for that block --- Library/Homebrew/.rubocop.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Library/Homebrew/.rubocop.yml b/Library/Homebrew/.rubocop.yml index 07b613ecb3..f9590b7800 100644 --- a/Library/Homebrew/.rubocop.yml +++ b/Library/Homebrew/.rubocop.yml @@ -16,7 +16,7 @@ Lint/NestedMethodDefinition: Metrics/AbcSize: Max: 280 Metrics/BlockLength: - Max: 106 + Max: 111 Exclude: # TODO: extract more of the bottling logic - "dev-cmd/bottle.rb" From 215e54566074d95771925579d15c20764e8b288c Mon Sep 17 00:00:00 2001 From: Lukas Oberhuber Date: Tue, 26 Jul 2022 19:28:30 +0100 Subject: [PATCH 11/42] `brew style` --- Library/Homebrew/build.rb | 8 ++++---- Library/Homebrew/cmd/install.rb | 4 ++-- Library/Homebrew/cmd/reinstall.rb | 4 ++-- Library/Homebrew/cmd/upgrade.rb | 4 ++-- Library/Homebrew/extend/ENV.rb | 2 +- Library/Homebrew/extend/os/linux/extend/ENV/std.rb | 2 +- Library/Homebrew/extend/os/linux/extend/ENV/super.rb | 2 +- Library/Homebrew/extend/os/mac/extend/ENV/shared.rb | 2 +- Library/Homebrew/extend/os/mac/extend/ENV/std.rb | 2 +- Library/Homebrew/extend/os/mac/extend/ENV/super.rb | 2 +- 10 files changed, 16 insertions(+), 16 deletions(-) diff --git a/Library/Homebrew/build.rb b/Library/Homebrew/build.rb index aef1cc3df4..ec430e04ba 100644 --- a/Library/Homebrew/build.rb +++ b/Library/Homebrew/build.rb @@ -93,10 +93,10 @@ class Build deps.each(&:modify_build_environment) else ENV.setup_build_environment( - formula: formula, - cc: args.cc, - build_bottle: args.build_bottle?, - bottle_arch: args.bottle_arch, + formula: formula, + cc: args.cc, + build_bottle: args.build_bottle?, + bottle_arch: args.bottle_arch, debug_symbols: args.debug_symbols?, ) reqs.each do |req| diff --git a/Library/Homebrew/cmd/install.rb b/Library/Homebrew/cmd/install.rb index 6848e982b5..6844ef124b 100644 --- a/Library/Homebrew/cmd/install.rb +++ b/Library/Homebrew/cmd/install.rb @@ -93,8 +93,8 @@ module Homebrew }], [:switch, "--debug-symbols", { depends_on: "--build-from-source", - description: "Generates debugging symbols during build. Source will be in temporary directory " \ - "which is retained. (see --keep-tmp)", + description: "Generate debug symbols on build. Source will be retained in a temporary directory. " \ + "(see --keep-tmp)", }], [:switch, "--build-bottle", { description: "Prepare the formula for eventual bottling during installation, skipping any " \ diff --git a/Library/Homebrew/cmd/reinstall.rb b/Library/Homebrew/cmd/reinstall.rb index bb8b4844cc..608eee3212 100644 --- a/Library/Homebrew/cmd/reinstall.rb +++ b/Library/Homebrew/cmd/reinstall.rb @@ -59,8 +59,8 @@ module Homebrew }], [:switch, "--debug-symbols", { depends_on: "--build-from-source", - description: "Generates debugging symbols during build. Source will be in temporary directory " \ - "which is retained. (see --keep-tmp)", + description: "Generate debug symbols on build. Source will be retained in a temporary directory. " \ + "(see --keep-tmp)", }], [:switch, "--display-times", { env: :display_install_times, diff --git a/Library/Homebrew/cmd/upgrade.rb b/Library/Homebrew/cmd/upgrade.rb index 3496c7274b..49d5a783f2 100644 --- a/Library/Homebrew/cmd/upgrade.rb +++ b/Library/Homebrew/cmd/upgrade.rb @@ -70,8 +70,8 @@ module Homebrew }], [:switch, "--debug-symbols", { depends_on: "--build-from-source", - description: "Generates debugging symbols during build. Source will be in temporary directory " \ - "which is retained. (see --keep-tmp)", + description: "Generate debug symbols on build. Source will be retained in a temporary directory. " \ + "(see --keep-tmp)", }], [:switch, "--display-times", { env: :display_install_times, diff --git a/Library/Homebrew/extend/ENV.rb b/Library/Homebrew/extend/ENV.rb index 40bb592a87..2b1182f71d 100644 --- a/Library/Homebrew/extend/ENV.rb +++ b/Library/Homebrew/extend/ENV.rb @@ -54,7 +54,7 @@ module EnvActivation T.cast(tmp_env, T.any(Superenv, Stdenv)) .setup_build_environment( cc: cc, build_bottle: build_bottle, bottle_arch: bottle_arch, - debug_symbols: debug_symbols, + debug_symbols: debug_symbols ) replace(tmp_env) diff --git a/Library/Homebrew/extend/os/linux/extend/ENV/std.rb b/Library/Homebrew/extend/os/linux/extend/ENV/std.rb index 73bc4e2a91..2b0c0b172a 100644 --- a/Library/Homebrew/extend/os/linux/extend/ENV/std.rb +++ b/Library/Homebrew/extend/os/linux/extend/ENV/std.rb @@ -13,7 +13,7 @@ module Stdenv generic_setup_build_environment( formula: formula, cc: cc, build_bottle: build_bottle, bottle_arch: bottle_arch, testing_formula: testing_formula, - debug_symbols: debug_symbols, + debug_symbols: debug_symbols ) prepend_path "CPATH", HOMEBREW_PREFIX/"include" diff --git a/Library/Homebrew/extend/os/linux/extend/ENV/super.rb b/Library/Homebrew/extend/os/linux/extend/ENV/super.rb index 074f842099..8358986170 100644 --- a/Library/Homebrew/extend/os/linux/extend/ENV/super.rb +++ b/Library/Homebrew/extend/os/linux/extend/ENV/super.rb @@ -26,7 +26,7 @@ module Superenv generic_setup_build_environment( formula: formula, cc: cc, build_bottle: build_bottle, bottle_arch: bottle_arch, testing_formula: testing_formula, - debug_symbols: debug_symbols, + debug_symbols: debug_symbols ) self["HOMEBREW_OPTIMIZATION_LEVEL"] = "O2" self["HOMEBREW_DYNAMIC_LINKER"] = determine_dynamic_linker_path diff --git a/Library/Homebrew/extend/os/mac/extend/ENV/shared.rb b/Library/Homebrew/extend/os/mac/extend/ENV/shared.rb index 7db7a5f8fd..268932734f 100644 --- a/Library/Homebrew/extend/os/mac/extend/ENV/shared.rb +++ b/Library/Homebrew/extend/os/mac/extend/ENV/shared.rb @@ -15,7 +15,7 @@ module SharedEnvExtension generic_shared_setup_build_environment( formula: formula, cc: cc, build_bottle: build_bottle, bottle_arch: bottle_arch, testing_formula: testing_formula, - debug_symbols: debug_symbols, + debug_symbols: debug_symbols ) # Normalise the system Perl version used, where multiple may be available diff --git a/Library/Homebrew/extend/os/mac/extend/ENV/std.rb b/Library/Homebrew/extend/os/mac/extend/ENV/std.rb index 7368ec4696..699658bc8d 100644 --- a/Library/Homebrew/extend/os/mac/extend/ENV/std.rb +++ b/Library/Homebrew/extend/os/mac/extend/ENV/std.rb @@ -21,7 +21,7 @@ module Stdenv generic_setup_build_environment( formula: formula, cc: cc, build_bottle: build_bottle, bottle_arch: bottle_arch, testing_formula: testing_formula, - debug_symbols: debug_symbols, + debug_symbols: debug_symbols ) append "LDFLAGS", "-Wl,-headerpad_max_install_names" diff --git a/Library/Homebrew/extend/os/mac/extend/ENV/super.rb b/Library/Homebrew/extend/os/mac/extend/ENV/super.rb index 258970240e..c17240444f 100644 --- a/Library/Homebrew/extend/os/mac/extend/ENV/super.rb +++ b/Library/Homebrew/extend/os/mac/extend/ENV/super.rb @@ -110,7 +110,7 @@ module Superenv generic_setup_build_environment( formula: formula, cc: cc, build_bottle: build_bottle, bottle_arch: bottle_arch, testing_formula: testing_formula, - debug_symbols: debug_symbols, + debug_symbols: debug_symbols ) # Filter out symbols known not to be defined since GNU Autotools can't From 22b1b61b73f742f9e4e149b8a34419a4522cdb5f Mon Sep 17 00:00:00 2001 From: Lukas Oberhuber Date: Tue, 26 Jul 2022 19:36:43 +0100 Subject: [PATCH 12/42] Fix `brew typecheck` --- Library/Homebrew/extend/ENV.rb | 11 ++++++----- Library/Homebrew/extend/ENV/std.rb | 1 + Library/Homebrew/extend/ENV/super.rb | 1 + 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/Library/Homebrew/extend/ENV.rb b/Library/Homebrew/extend/ENV.rb index 2b1182f71d..dd3145954a 100644 --- a/Library/Homebrew/extend/ENV.rb +++ b/Library/Homebrew/extend/ENV.rb @@ -33,11 +33,12 @@ module EnvActivation sig { params( - env: T.nilable(String), - cc: T.nilable(String), - build_bottle: T::Boolean, - bottle_arch: T.nilable(String), - _block: T.proc.returns(T.untyped), + env: T.nilable(String), + cc: T.nilable(String), + build_bottle: T::Boolean, + bottle_arch: T.nilable(String), + debug_symbols: T.nilable(T::Boolean), + _block: T.proc.returns(T.untyped), ).returns(T.untyped) } def with_build_environment( diff --git a/Library/Homebrew/extend/ENV/std.rb b/Library/Homebrew/extend/ENV/std.rb index fb3cc6d570..a1ee227f2f 100644 --- a/Library/Homebrew/extend/ENV/std.rb +++ b/Library/Homebrew/extend/ENV/std.rb @@ -21,6 +21,7 @@ module Stdenv build_bottle: T.nilable(T::Boolean), bottle_arch: T.nilable(String), testing_formula: T::Boolean, + debug_symbols: T.nilable(T::Boolean), ).void } def setup_build_environment( diff --git a/Library/Homebrew/extend/ENV/super.rb b/Library/Homebrew/extend/ENV/super.rb index e95f0d2997..619f6bb02f 100644 --- a/Library/Homebrew/extend/ENV/super.rb +++ b/Library/Homebrew/extend/ENV/super.rb @@ -55,6 +55,7 @@ module Superenv build_bottle: T.nilable(T::Boolean), bottle_arch: T.nilable(String), testing_formula: T::Boolean, + debug_symbols: T.nilable(T::Boolean), ).void } def setup_build_environment( From e46a61e181f8e324d1bc4222bedf692c11ad84fa Mon Sep 17 00:00:00 2001 From: Lukas Oberhuber Date: Sat, 30 Jul 2022 11:08:52 +0100 Subject: [PATCH 13/42] rename & inline dsymutil to prepare_debug_symbols --- Library/Homebrew/extend/os/mac/keg.rb | 2 +- Library/Homebrew/formula_installer.rb | 10 +--------- Library/Homebrew/keg.rb | 2 +- 3 files changed, 3 insertions(+), 11 deletions(-) diff --git a/Library/Homebrew/extend/os/mac/keg.rb b/Library/Homebrew/extend/os/mac/keg.rb index a1fde91b14..7337586456 100644 --- a/Library/Homebrew/extend/os/mac/keg.rb +++ b/Library/Homebrew/extend/os/mac/keg.rb @@ -63,7 +63,7 @@ class Keg EOS end - def dsymutil + def prepare_debug_symbols binary_executable_or_library_files.each do |file| odebug "Extracting symbols #{file}" diff --git a/Library/Homebrew/formula_installer.rb b/Library/Homebrew/formula_installer.rb index 18c8e94334..e895c09215 100644 --- a/Library/Homebrew/formula_installer.rb +++ b/Library/Homebrew/formula_installer.rb @@ -807,7 +807,7 @@ class FormulaInstaller post_install end - dsymutil(keg) if debug_symbols? + keg.prepare_debug_symbols if debug_symbols? # Updates the cache for a particular formula after doing an install CacheStoreDatabase.use(:linkage) do |db| @@ -1338,12 +1338,4 @@ class FormulaInstaller #{SPDX.license_expression_to_string formula.license}. EOS end - - sig { params(keg: Keg).void } - def dsymutil(keg) - keg.dsymutil - rescue RuntimeError => e - ofail "Failed to extract debugging symbols for #{formula.full_name}" - puts e - end end diff --git a/Library/Homebrew/keg.rb b/Library/Homebrew/keg.rb index f28c6da545..a12e8de395 100644 --- a/Library/Homebrew/keg.rb +++ b/Library/Homebrew/keg.rb @@ -483,7 +483,7 @@ class Keg ObserverPathnameExtension.n end - def dsymutil; end + def prepare_debug_symbols; end def remove_oldname_opt_record return unless oldname_opt_record From cd9fe97c551d4e9b5f0a702a1382a6ccbe494773 Mon Sep 17 00:00:00 2001 From: Lukas Oberhuber Date: Sat, 30 Jul 2022 11:10:26 +0100 Subject: [PATCH 14/42] Improve style --- Library/Homebrew/build.rb | 18 ++++-------------- Library/Homebrew/extend/ENV.rb | 15 +++------------ Library/Homebrew/extend/ENV/shared.rb | 10 ++-------- Library/Homebrew/extend/ENV/std.rb | 10 ++-------- Library/Homebrew/extend/ENV/super.rb | 10 ++-------- .../Homebrew/extend/os/linux/extend/ENV/std.rb | 17 ++++------------- .../extend/os/linux/extend/ENV/super.rb | 17 ++++------------- .../extend/os/mac/extend/ENV/shared.rb | 5 ++--- .../Homebrew/extend/os/mac/extend/ENV/std.rb | 7 ++----- .../Homebrew/extend/os/mac/extend/ENV/super.rb | 15 ++++----------- 10 files changed, 29 insertions(+), 95 deletions(-) diff --git a/Library/Homebrew/build.rb b/Library/Homebrew/build.rb index ec430e04ba..0722cb060c 100644 --- a/Library/Homebrew/build.rb +++ b/Library/Homebrew/build.rb @@ -78,13 +78,8 @@ class Build ENV.keg_only_deps = keg_only_deps ENV.deps = formula_deps ENV.run_time_deps = run_time_deps - ENV.setup_build_environment( - formula: formula, - cc: args.cc, - build_bottle: args.build_bottle?, - bottle_arch: args.bottle_arch, - debug_symbols: args.debug_symbols?, - ) + ENV.setup_build_environment(formula: formula, cc: args.cc, build_bottle: args.build_bottle?, + bottle_arch: args.bottle_arch, debug_symbols: args.debug_symbols?) reqs.each do |req| req.modify_build_environment( env: args.env, cc: args.cc, build_bottle: args.build_bottle?, bottle_arch: args.bottle_arch, @@ -92,13 +87,8 @@ class Build end deps.each(&:modify_build_environment) else - ENV.setup_build_environment( - formula: formula, - cc: args.cc, - build_bottle: args.build_bottle?, - bottle_arch: args.bottle_arch, - debug_symbols: args.debug_symbols?, - ) + ENV.setup_build_environment(formula: formula, cc: args.cc, build_bottle: args.build_bottle?, + bottle_arch: args.bottle_arch, debug_symbols: args.debug_symbols?) reqs.each do |req| req.modify_build_environment( env: args.env, cc: args.cc, build_bottle: args.build_bottle?, bottle_arch: args.bottle_arch, diff --git a/Library/Homebrew/extend/ENV.rb b/Library/Homebrew/extend/ENV.rb index dd3145954a..54b293ee87 100644 --- a/Library/Homebrew/extend/ENV.rb +++ b/Library/Homebrew/extend/ENV.rb @@ -41,22 +41,13 @@ module EnvActivation _block: T.proc.returns(T.untyped), ).returns(T.untyped) } - def with_build_environment( - env: nil, - cc: nil, - build_bottle: false, - bottle_arch: nil, - debug_symbols: false, - &_block - ) + def with_build_environment(env: nil, cc: nil, build_bottle: false, bottle_arch: nil, debug_symbols: false, &_block) old_env = to_hash.dup tmp_env = to_hash.dup.extend(EnvActivation) T.cast(tmp_env, EnvActivation).activate_extensions!(env: env) T.cast(tmp_env, T.any(Superenv, Stdenv)) - .setup_build_environment( - cc: cc, build_bottle: build_bottle, bottle_arch: bottle_arch, - debug_symbols: debug_symbols - ) + .setup_build_environment(cc: cc, build_bottle: build_bottle, bottle_arch: bottle_arch, + debug_symbols: debug_symbols) replace(tmp_env) begin diff --git a/Library/Homebrew/extend/ENV/shared.rb b/Library/Homebrew/extend/ENV/shared.rb index f31400733c..95bede7d4a 100644 --- a/Library/Homebrew/extend/ENV/shared.rb +++ b/Library/Homebrew/extend/ENV/shared.rb @@ -43,14 +43,8 @@ module SharedEnvExtension debug_symbols: T.nilable(T::Boolean), ).void } - def setup_build_environment( - formula: nil, - cc: nil, - build_bottle: false, - bottle_arch: nil, - testing_formula: false, - debug_symbols: false - ) + def setup_build_environment(formula: nil, cc: nil, build_bottle: false, bottle_arch: nil, testing_formula: false, + debug_symbols: false) @formula = formula @cc = cc @build_bottle = build_bottle diff --git a/Library/Homebrew/extend/ENV/std.rb b/Library/Homebrew/extend/ENV/std.rb index a1ee227f2f..37d4c7779f 100644 --- a/Library/Homebrew/extend/ENV/std.rb +++ b/Library/Homebrew/extend/ENV/std.rb @@ -24,14 +24,8 @@ module Stdenv debug_symbols: T.nilable(T::Boolean), ).void } - def setup_build_environment( - formula: nil, - cc: nil, - build_bottle: false, - bottle_arch: nil, - testing_formula: false, - debug_symbols: false - ) + def setup_build_environment(formula: nil, cc: nil, build_bottle: false, bottle_arch: nil, testing_formula: false, + debug_symbols: false) super self["HOMEBREW_ENV"] = "std" diff --git a/Library/Homebrew/extend/ENV/super.rb b/Library/Homebrew/extend/ENV/super.rb index 619f6bb02f..8f9a8330c9 100644 --- a/Library/Homebrew/extend/ENV/super.rb +++ b/Library/Homebrew/extend/ENV/super.rb @@ -58,14 +58,8 @@ module Superenv debug_symbols: T.nilable(T::Boolean), ).void } - def setup_build_environment( - formula: nil, - cc: nil, - build_bottle: false, - bottle_arch: nil, - testing_formula: false, - debug_symbols: false - ) + def setup_build_environment(formula: nil, cc: nil, build_bottle: false, bottle_arch: nil, testing_formula: false, + debug_symbols: false) super send(compiler) diff --git a/Library/Homebrew/extend/os/linux/extend/ENV/std.rb b/Library/Homebrew/extend/os/linux/extend/ENV/std.rb index 2b0c0b172a..58c741e1d4 100644 --- a/Library/Homebrew/extend/os/linux/extend/ENV/std.rb +++ b/Library/Homebrew/extend/os/linux/extend/ENV/std.rb @@ -2,19 +2,10 @@ # frozen_string_literal: true module Stdenv - def setup_build_environment( - formula: nil, - cc: nil, - build_bottle: false, - bottle_arch: nil, - testing_formula: false, - debug_symbols: false - ) - generic_setup_build_environment( - formula: formula, cc: cc, build_bottle: build_bottle, - bottle_arch: bottle_arch, testing_formula: testing_formula, - debug_symbols: debug_symbols - ) + def setup_build_environment(formula: nil, cc: nil, build_bottle: false, bottle_arch: nil, testing_formula: false, + debug_symbols: false) + generic_setup_build_environment(formula: formula, cc: cc, build_bottle: build_bottle, bottle_arch: bottle_arch, + testing_formula: testing_formula, debug_symbols: debug_symbols) prepend_path "CPATH", HOMEBREW_PREFIX/"include" prepend_path "LIBRARY_PATH", HOMEBREW_PREFIX/"lib" diff --git a/Library/Homebrew/extend/os/linux/extend/ENV/super.rb b/Library/Homebrew/extend/os/linux/extend/ENV/super.rb index 8358986170..9dbac81d0e 100644 --- a/Library/Homebrew/extend/os/linux/extend/ENV/super.rb +++ b/Library/Homebrew/extend/os/linux/extend/ENV/super.rb @@ -15,19 +15,10 @@ module Superenv end # @private - def setup_build_environment( - formula: nil, - cc: nil, - build_bottle: false, - bottle_arch: nil, - testing_formula: false, - debug_symbols: false - ) - generic_setup_build_environment( - formula: formula, cc: cc, build_bottle: build_bottle, - bottle_arch: bottle_arch, testing_formula: testing_formula, - debug_symbols: debug_symbols - ) + def setup_build_environment(formula: nil, cc: nil, build_bottle: false, bottle_arch: nil, testing_formula: false, + debug_symbols: false) + generic_setup_build_environment(formula: formula, cc: cc, build_bottle: build_bottle, bottle_arch: bottle_arch, + testing_formula: testing_formula, debug_symbols: debug_symbols) self["HOMEBREW_OPTIMIZATION_LEVEL"] = "O2" self["HOMEBREW_DYNAMIC_LINKER"] = determine_dynamic_linker_path self["HOMEBREW_RPATH_PATHS"] = determine_rpath_paths(@formula) diff --git a/Library/Homebrew/extend/os/mac/extend/ENV/shared.rb b/Library/Homebrew/extend/os/mac/extend/ENV/shared.rb index 268932734f..019c3091f7 100644 --- a/Library/Homebrew/extend/os/mac/extend/ENV/shared.rb +++ b/Library/Homebrew/extend/os/mac/extend/ENV/shared.rb @@ -13,9 +13,8 @@ module SharedEnvExtension debug_symbols: false ) generic_shared_setup_build_environment( - formula: formula, cc: cc, build_bottle: build_bottle, - bottle_arch: bottle_arch, testing_formula: testing_formula, - debug_symbols: debug_symbols + formula: formula, cc: cc, build_bottle: build_bottle, bottle_arch: bottle_arch, + testing_formula: testing_formula, debug_symbols: debug_symbols ) # Normalise the system Perl version used, where multiple may be available diff --git a/Library/Homebrew/extend/os/mac/extend/ENV/std.rb b/Library/Homebrew/extend/os/mac/extend/ENV/std.rb index 699658bc8d..1c255a151e 100644 --- a/Library/Homebrew/extend/os/mac/extend/ENV/std.rb +++ b/Library/Homebrew/extend/os/mac/extend/ENV/std.rb @@ -18,11 +18,8 @@ module Stdenv testing_formula: false, debug_symbols: false ) - generic_setup_build_environment( - formula: formula, cc: cc, build_bottle: build_bottle, - bottle_arch: bottle_arch, testing_formula: testing_formula, - debug_symbols: debug_symbols - ) + generic_setup_build_environment(formula: formula, cc: cc, build_bottle: build_bottle, bottle_arch: bottle_arch, + testing_formula: testing_formula, debug_symbols: debug_symbols) append "LDFLAGS", "-Wl,-headerpad_max_install_names" diff --git a/Library/Homebrew/extend/os/mac/extend/ENV/super.rb b/Library/Homebrew/extend/os/mac/extend/ENV/super.rb index c17240444f..4c7d767fbb 100644 --- a/Library/Homebrew/extend/os/mac/extend/ENV/super.rb +++ b/Library/Homebrew/extend/os/mac/extend/ENV/super.rb @@ -85,14 +85,8 @@ module Superenv end # @private - def setup_build_environment( - formula: nil, - cc: nil, - build_bottle: false, - bottle_arch: nil, - testing_formula: false, - debug_symbols: false - ) + def setup_build_environment(formula: nil, cc: nil, build_bottle: false, bottle_arch: nil, testing_formula: false, + debug_symbols: false) sdk = formula ? MacOS.sdk_for_formula(formula) : MacOS.sdk is_xcode_sdk = sdk&.source == :xcode @@ -108,9 +102,8 @@ module Superenv end generic_setup_build_environment( - formula: formula, cc: cc, build_bottle: build_bottle, - bottle_arch: bottle_arch, testing_formula: testing_formula, - debug_symbols: debug_symbols + formula: formula, cc: cc, build_bottle: build_bottle, bottle_arch: bottle_arch, + testing_formula: testing_formula, debug_symbols: debug_symbols ) # Filter out symbols known not to be defined since GNU Autotools can't From 15f1ac8775f19908b3f484ccdb21a0dcfda0099f Mon Sep 17 00:00:00 2001 From: Lukas Oberhuber Date: Sat, 30 Jul 2022 11:23:40 +0100 Subject: [PATCH 15/42] Integration test for `--debug-symbols` But only checks that the command works. Not that `dsymutil` is called or that `-g` is added to the compile args. (Not sure how to do either in an integration test.) --- Library/Homebrew/test/cmd/install_spec.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Library/Homebrew/test/cmd/install_spec.rb b/Library/Homebrew/test/cmd/install_spec.rb index 560bc3393c..590ee68506 100644 --- a/Library/Homebrew/test/cmd/install_spec.rb +++ b/Library/Homebrew/test/cmd/install_spec.rb @@ -72,4 +72,14 @@ describe "brew install" do .and be_a_success expect(HOMEBREW_CELLAR/"testball1/HEAD-d5eb689/foo/test").not_to be_a_file end + + it "installs formulae with debug symbols", :integration_test do + setup_test_formula "testball1" + + expect { brew "install", "testball1", "--debug-symbols", "-s" } + .to output(%r{#{HOMEBREW_CELLAR}/testball1/0\.1}o).to_stdout + .and not_to_output.to_stderr + .and be_a_success + expect(HOMEBREW_CELLAR/"testball1/0.1/foo/test").not_to be_a_file + end end From 4eea117e849a68b9364242808050316696b2d9dd Mon Sep 17 00:00:00 2001 From: Lukas Oberhuber Date: Sat, 30 Jul 2022 11:41:05 +0100 Subject: [PATCH 16/42] `ofail` if debug-symbols cannot be extracted. --- Library/Homebrew/extend/os/mac/keg.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Library/Homebrew/extend/os/mac/keg.rb b/Library/Homebrew/extend/os/mac/keg.rb index 7337586456..7b9b29cc8e 100644 --- a/Library/Homebrew/extend/os/mac/keg.rb +++ b/Library/Homebrew/extend/os/mac/keg.rb @@ -71,7 +71,7 @@ class Keg next if result.success? # If it fails again, error out - onoe <<~EOS + ofail <<~EOS Failed to extract symbols from #{file}: #{result.stderr} EOS From 4b0d52ef62c9e964b2a570b9e9459f454f311255 Mon Sep 17 00:00:00 2001 From: Lukas Oberhuber Date: Sun, 31 Jul 2022 19:59:25 +0100 Subject: [PATCH 17/42] debug_symbols passed down to soure dir creator The flag is now passed down to resource which creates the directory for unpacking the source. --- Library/Homebrew/formula.rb | 6 +-- Library/Homebrew/mksource.rb | 84 ++++++++++++++++++++++++++++++++++++ Library/Homebrew/resource.rb | 13 +++--- 3 files changed, 94 insertions(+), 9 deletions(-) create mode 100644 Library/Homebrew/mksource.rb diff --git a/Library/Homebrew/formula.rb b/Library/Homebrew/formula.rb index 87084de97d..e5b3dc8c15 100644 --- a/Library/Homebrew/formula.rb +++ b/Library/Homebrew/formula.rb @@ -1276,7 +1276,7 @@ class Formula def brew(fetch: true, keep_tmp: false, debug_symbols: false, interactive: false) @prefix_returns_versioned_prefix = true active_spec.fetch if fetch - stage(interactive: interactive) do |staging| + stage(interactive: interactive, debug_symbols: debug_symbols) do |staging| staging.retain! if keep_tmp || debug_symbols prepare_patches @@ -2437,8 +2437,8 @@ class Formula } end - def stage(interactive: false) - active_spec.stage do |staging| + def stage(interactive: false, debug_symbols: false) + active_spec.stage(debug_symbols: debug_symbols) do |staging| @source_modified_time = active_spec.source_modified_time @buildpath = Pathname.pwd env_home = buildpath/".brew_home" diff --git a/Library/Homebrew/mksource.rb b/Library/Homebrew/mksource.rb new file mode 100644 index 0000000000..8dc93af6cc --- /dev/null +++ b/Library/Homebrew/mksource.rb @@ -0,0 +1,84 @@ +# typed: false +# frozen_string_literal: true + +# Performs {Formula#mktemp}'s functionality, and tracks the results. +# Each instance is only intended to be used once. +class Mksource + extend T::Sig + + include FileUtils + + # Path to the tmpdir used in this run, as a {Pathname}. + attr_reader :tmpdir + + def initialize(prefix, opts = {}) + @prefix = prefix + @retain = opts[:retain] + @quiet = false + end + + # Instructs this {Mksource} to retain the staged files. + sig { void } + def retain! + @retain = true + end + + # True if the staged temporary files should be retained. + def retain? + @retain + end + + # Instructs this Mksource to not emit messages when retention is triggered. + sig { void } + def quiet! + @quiet = true + end + + sig { returns(String) } + def to_s + "[Mksource: #{tmpdir} retain=#{@retain} quiet=#{@quiet}]" + end + + def run + @tmpdir = Pathname.new(Dir.mktmpdir("#{@prefix.tr "@", "AT"}-", HOMEBREW_TEMP)) + + # Make sure files inside the temporary directory have the same group as the + # brew instance. + # + # Reference from `man 2 open` + # > When a new file is created, it is given the group of the directory which + # contains it. + group_id = if HOMEBREW_BREW_FILE.grpowned? + HOMEBREW_BREW_FILE.stat.gid + else + Process.gid + end + begin + chown(nil, group_id, tmpdir) + rescue Errno::EPERM + opoo "Failed setting group \"#{Etc.getgrgid(group_id).name}\" on #{tmpdir}" + end + + begin + Dir.chdir(tmpdir) { yield self } + ensure + ignore_interrupts { chmod_rm_rf(tmpdir) } unless retain? + end + ensure + ohai "Temporary files retained at:", @tmpdir.to_s if retain? && !@tmpdir.nil? && !@quiet + end + + private + + def chmod_rm_rf(path) + if path.directory? && !path.symlink? + chmod("u+rw", path) if path.owned? # Need permissions in order to see the contents + path.children.each { |child| chmod_rm_rf(child) } + rmdir(path) + else + rm_f(path) + end + rescue + nil # Just skip this directory. + end +end diff --git a/Library/Homebrew/resource.rb b/Library/Homebrew/resource.rb index a8e5f712c3..834b30ea0d 100644 --- a/Library/Homebrew/resource.rb +++ b/Library/Homebrew/resource.rb @@ -5,6 +5,7 @@ require "download_strategy" require "checksum" require "version" require "mktemp" +require "mksource" require "livecheck" require "extend/on_system" @@ -89,14 +90,14 @@ class Resource # dir using {Mktemp} so that works with all subtypes. # # @api public - def stage(target = nil, &block) + def stage(target = nil, debug_symbols: false, &block) raise ArgumentError, "target directory or block is required" if !target && block.blank? prepare_patches fetch_patches(skip_downloaded: true) fetch unless downloaded? - unpack(target, &block) + unpack(target, debug_symbols: debug_symbols, &block) end def prepare_patches @@ -120,9 +121,9 @@ class Resource # If block is given, yield to that block with `|stage|`, where stage # is a {ResourceStageContext}. # A target or a block must be given, but not both. - def unpack(target = nil) + def unpack(target = nil, debug_symbols: false) current_working_directory = Pathname.pwd - mktemp(download_name) do |staging| + stage_resource(download_name, debug_symbols: debug_symbols) do |staging| downloader.stage do @source_modified_time = downloader.source_modified_time apply_patches @@ -235,8 +236,8 @@ class Resource protected - def mktemp(prefix, &block) - Mktemp.new(prefix).run(&block) + def stage_resource(prefix, debug_symbols: false, &block) + debug_symbols ? Mksource.new(prefix).run(&block) : Mktemp.new(prefix).run(&block) end private From 8b1eb32e99efa05d263c8508becc187b44c9d33b Mon Sep 17 00:00:00 2001 From: Lukas Oberhuber Date: Sun, 31 Jul 2022 20:33:25 +0100 Subject: [PATCH 18/42] Sources now retained in Caches/Homebrew/Sources Full path is ~/Library/Caches/Homebrew/Sources This creates a brand new directory for each build, but leaves previous. --- Library/Homebrew/mksource.rb | 84 ------------------------------------ Library/Homebrew/mktemp.rb | 11 ++++- Library/Homebrew/resource.rb | 3 +- 3 files changed, 10 insertions(+), 88 deletions(-) delete mode 100644 Library/Homebrew/mksource.rb diff --git a/Library/Homebrew/mksource.rb b/Library/Homebrew/mksource.rb deleted file mode 100644 index 8dc93af6cc..0000000000 --- a/Library/Homebrew/mksource.rb +++ /dev/null @@ -1,84 +0,0 @@ -# typed: false -# frozen_string_literal: true - -# Performs {Formula#mktemp}'s functionality, and tracks the results. -# Each instance is only intended to be used once. -class Mksource - extend T::Sig - - include FileUtils - - # Path to the tmpdir used in this run, as a {Pathname}. - attr_reader :tmpdir - - def initialize(prefix, opts = {}) - @prefix = prefix - @retain = opts[:retain] - @quiet = false - end - - # Instructs this {Mksource} to retain the staged files. - sig { void } - def retain! - @retain = true - end - - # True if the staged temporary files should be retained. - def retain? - @retain - end - - # Instructs this Mksource to not emit messages when retention is triggered. - sig { void } - def quiet! - @quiet = true - end - - sig { returns(String) } - def to_s - "[Mksource: #{tmpdir} retain=#{@retain} quiet=#{@quiet}]" - end - - def run - @tmpdir = Pathname.new(Dir.mktmpdir("#{@prefix.tr "@", "AT"}-", HOMEBREW_TEMP)) - - # Make sure files inside the temporary directory have the same group as the - # brew instance. - # - # Reference from `man 2 open` - # > When a new file is created, it is given the group of the directory which - # contains it. - group_id = if HOMEBREW_BREW_FILE.grpowned? - HOMEBREW_BREW_FILE.stat.gid - else - Process.gid - end - begin - chown(nil, group_id, tmpdir) - rescue Errno::EPERM - opoo "Failed setting group \"#{Etc.getgrgid(group_id).name}\" on #{tmpdir}" - end - - begin - Dir.chdir(tmpdir) { yield self } - ensure - ignore_interrupts { chmod_rm_rf(tmpdir) } unless retain? - end - ensure - ohai "Temporary files retained at:", @tmpdir.to_s if retain? && !@tmpdir.nil? && !@quiet - end - - private - - def chmod_rm_rf(path) - if path.directory? && !path.symlink? - chmod("u+rw", path) if path.owned? # Need permissions in order to see the contents - path.children.each { |child| chmod_rm_rf(child) } - rmdir(path) - else - rm_f(path) - end - rescue - nil # Just skip this directory. - end -end diff --git a/Library/Homebrew/mktemp.rb b/Library/Homebrew/mktemp.rb index c95f6241bd..16aa64a0d5 100644 --- a/Library/Homebrew/mktemp.rb +++ b/Library/Homebrew/mktemp.rb @@ -13,7 +13,8 @@ class Mktemp def initialize(prefix, opts = {}) @prefix = prefix - @retain = opts[:retain] + @retain_in_sources = opts[:retain_in_sources] + @retain = opts[:retain] || @retain_in_sources @quiet = false end @@ -40,7 +41,13 @@ class Mktemp end def run - @tmpdir = Pathname.new(Dir.mktmpdir("#{@prefix.tr "@", "AT"}-", HOMEBREW_TEMP)) + if @retain_in_sources + root = "#{HOMEBREW_CACHE}/Sources" + FileUtils.mkdir_p root + else + root = HOMEBREW_TEMP + end + @tmpdir = Pathname.new(Dir.mktmpdir("#{@prefix.tr "@", "AT"}-", root)) # Make sure files inside the temporary directory have the same group as the # brew instance. diff --git a/Library/Homebrew/resource.rb b/Library/Homebrew/resource.rb index 834b30ea0d..2246ae42e4 100644 --- a/Library/Homebrew/resource.rb +++ b/Library/Homebrew/resource.rb @@ -5,7 +5,6 @@ require "download_strategy" require "checksum" require "version" require "mktemp" -require "mksource" require "livecheck" require "extend/on_system" @@ -237,7 +236,7 @@ class Resource protected def stage_resource(prefix, debug_symbols: false, &block) - debug_symbols ? Mksource.new(prefix).run(&block) : Mktemp.new(prefix).run(&block) + Mktemp.new(prefix, retain_in_sources: debug_symbols).run(&block) end private From 93132c6876a6484bae0436bdb3d93c6717bcf828 Mon Sep 17 00:00:00 2001 From: Lukas Oberhuber Date: Sun, 31 Jul 2022 20:54:14 +0100 Subject: [PATCH 19/42] Always put source files in the same directory There's no point in saving old ones because the debug symbols will only for the newest bulid anyway. Currently blows away what was there before, which isn't ideal for a dev workflow. Maybe that should be changed, given a tar file should be a tar file, so shouldn't change. But there are many different types of files. --- Library/Homebrew/mktemp.rb | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/Library/Homebrew/mktemp.rb b/Library/Homebrew/mktemp.rb index 16aa64a0d5..7a9302e419 100644 --- a/Library/Homebrew/mktemp.rb +++ b/Library/Homebrew/mktemp.rb @@ -42,12 +42,13 @@ class Mktemp def run if @retain_in_sources - root = "#{HOMEBREW_CACHE}/Sources" - FileUtils.mkdir_p root + source_dir = "#{HOMEBREW_CACHE}/Sources/#{@prefix.tr "@", "AT"}" + chmod_rm_rf(source_dir) # clear out previous (otherwise not sure what happens) + FileUtils.mkdir_p(source_dir) + @tmpdir = Pathname.new(source_dir) else - root = HOMEBREW_TEMP + @tmpdir = Pathname.new(Dir.mktmpdir("#{@prefix.tr "@", "AT"}-", HOMEBREW_TEMP)) end - @tmpdir = Pathname.new(Dir.mktmpdir("#{@prefix.tr "@", "AT"}-", root)) # Make sure files inside the temporary directory have the same group as the # brew instance. From 41a526546661f400011d0ea56f7daa60cec458af Mon Sep 17 00:00:00 2001 From: Lukas Oberhuber Date: Sun, 31 Jul 2022 21:06:33 +0100 Subject: [PATCH 20/42] Improve messaging of debug source location --- Library/Homebrew/mktemp.rb | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/Library/Homebrew/mktemp.rb b/Library/Homebrew/mktemp.rb index 7a9302e419..92718d36f4 100644 --- a/Library/Homebrew/mktemp.rb +++ b/Library/Homebrew/mktemp.rb @@ -29,6 +29,11 @@ class Mktemp @retain end + # True if the source files should be retained. + def retain_in_sources? + @retain_in_sources + end + # Instructs this Mktemp to not emit messages when retention is triggered. sig { void } def quiet! @@ -73,7 +78,10 @@ class Mktemp ignore_interrupts { chmod_rm_rf(tmpdir) } unless retain? end ensure - ohai "Temporary files retained at:", @tmpdir.to_s if retain? && !@tmpdir.nil? && !@quiet + if retain? && !@tmpdir.nil? && !@quiet + message = retain_in_sources? ? "Source files for debugging available at:" : "Temporary files retained at:" + ohai message, @tmpdir.to_s + end end private From bee353109049781e6321977ce6b134e8709d102d Mon Sep 17 00:00:00 2001 From: Lukas Oberhuber Date: Mon, 1 Aug 2022 15:25:34 -0700 Subject: [PATCH 21/42] Improve style --- Library/Homebrew/extend/os/mac/extend/ENV/shared.rb | 10 ++-------- Library/Homebrew/extend/os/mac/extend/ENV/std.rb | 10 ++-------- Library/Homebrew/test/cmd/install_spec.rb | 2 +- 3 files changed, 5 insertions(+), 17 deletions(-) diff --git a/Library/Homebrew/extend/os/mac/extend/ENV/shared.rb b/Library/Homebrew/extend/os/mac/extend/ENV/shared.rb index 019c3091f7..6ee43c0bb8 100644 --- a/Library/Homebrew/extend/os/mac/extend/ENV/shared.rb +++ b/Library/Homebrew/extend/os/mac/extend/ENV/shared.rb @@ -4,14 +4,8 @@ module SharedEnvExtension extend T::Sig - def setup_build_environment( - formula: nil, - cc: nil, - build_bottle: false, - bottle_arch: nil, - testing_formula: false, - debug_symbols: false - ) + def setup_build_environment(formula: nil, cc: nil, build_bottle: false, bottle_arch: nil, testing_formula: false, + debug_symbols: false) generic_shared_setup_build_environment( formula: formula, cc: cc, build_bottle: build_bottle, bottle_arch: bottle_arch, testing_formula: testing_formula, debug_symbols: debug_symbols diff --git a/Library/Homebrew/extend/os/mac/extend/ENV/std.rb b/Library/Homebrew/extend/os/mac/extend/ENV/std.rb index 1c255a151e..d17b35c64d 100644 --- a/Library/Homebrew/extend/os/mac/extend/ENV/std.rb +++ b/Library/Homebrew/extend/os/mac/extend/ENV/std.rb @@ -10,14 +10,8 @@ module Stdenv ["#{HOMEBREW_LIBRARY}/Homebrew/os/mac/pkgconfig/#{MacOS.version}"] end - def setup_build_environment( - formula: nil, - cc: nil, - build_bottle: false, - bottle_arch: nil, - testing_formula: false, - debug_symbols: false - ) + def setup_build_environment(formula: nil, cc: nil, build_bottle: false, bottle_arch: nil, testing_formula: false, + debug_symbols: false) generic_setup_build_environment(formula: formula, cc: cc, build_bottle: build_bottle, bottle_arch: bottle_arch, testing_formula: testing_formula, debug_symbols: debug_symbols) diff --git a/Library/Homebrew/test/cmd/install_spec.rb b/Library/Homebrew/test/cmd/install_spec.rb index 590ee68506..cffebfc524 100644 --- a/Library/Homebrew/test/cmd/install_spec.rb +++ b/Library/Homebrew/test/cmd/install_spec.rb @@ -76,7 +76,7 @@ describe "brew install" do it "installs formulae with debug symbols", :integration_test do setup_test_formula "testball1" - expect { brew "install", "testball1", "--debug-symbols", "-s" } + expect { brew "install", "testball1", "--debug-symbols", "--build-from-source" } .to output(%r{#{HOMEBREW_CELLAR}/testball1/0\.1}o).to_stdout .and not_to_output.to_stderr .and be_a_success From 5b1724ef33fe88c62e485646d45d0b33699d7720 Mon Sep 17 00:00:00 2001 From: Lukas Oberhuber Date: Mon, 1 Aug 2022 15:27:17 -0700 Subject: [PATCH 22/42] Fix rubocop warning by ignoring for `install_args` --- Library/Homebrew/.rubocop.yml | 2 +- Library/Homebrew/cmd/install.rb | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/Library/Homebrew/.rubocop.yml b/Library/Homebrew/.rubocop.yml index f9590b7800..07b613ecb3 100644 --- a/Library/Homebrew/.rubocop.yml +++ b/Library/Homebrew/.rubocop.yml @@ -16,7 +16,7 @@ Lint/NestedMethodDefinition: Metrics/AbcSize: Max: 280 Metrics/BlockLength: - Max: 111 + Max: 106 Exclude: # TODO: extract more of the bottling logic - "dev-cmd/bottle.rb" diff --git a/Library/Homebrew/cmd/install.rb b/Library/Homebrew/cmd/install.rb index 6844ef124b..18f8034488 100644 --- a/Library/Homebrew/cmd/install.rb +++ b/Library/Homebrew/cmd/install.rb @@ -20,6 +20,7 @@ module Homebrew module_function + # rubocop:disable Metrics/BlockLength sig { returns(CLI::Parser) } def install_args Homebrew::CLI::Parser.new do @@ -141,6 +142,7 @@ module Homebrew named_args [:formula, :cask], min: 1 end end + # rubocop:enable Metrics/BlockLength def install args = install_args.parse From fdf17f06b1c9a35e544653ed4c46ea0b9866e610 Mon Sep 17 00:00:00 2001 From: Lukas Oberhuber Date: Mon, 1 Aug 2022 16:30:00 -0700 Subject: [PATCH 23/42] Test for dSYM directory on Mac --- Library/Homebrew/test/cmd/install_spec.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Library/Homebrew/test/cmd/install_spec.rb b/Library/Homebrew/test/cmd/install_spec.rb index cffebfc524..3840f00f1e 100644 --- a/Library/Homebrew/test/cmd/install_spec.rb +++ b/Library/Homebrew/test/cmd/install_spec.rb @@ -80,6 +80,7 @@ describe "brew install" do .to output(%r{#{HOMEBREW_CELLAR}/testball1/0\.1}o).to_stdout .and not_to_output.to_stderr .and be_a_success - expect(HOMEBREW_CELLAR/"testball1/0.1/foo/test").not_to be_a_file + expect(HOMEBREW_CELLAR/"testball1/0.1/bin/test").to be_a_file + expect(HOMEBREW_CELLAR/"testball1/0.1/bin/test.dSYM").to be_a_directory if OS.mac? end end From 676e3d4923c3c92c46f479c54e4cbf2ac0fb71c0 Mon Sep 17 00:00:00 2001 From: Lukas Oberhuber Date: Mon, 1 Aug 2022 18:30:14 -0700 Subject: [PATCH 24/42] Change name of option on mktemp From retain_in_sources to retain_in_cache --- Library/Homebrew/mktemp.rb | 12 ++++++------ Library/Homebrew/resource.rb | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Library/Homebrew/mktemp.rb b/Library/Homebrew/mktemp.rb index 92718d36f4..1e958159f2 100644 --- a/Library/Homebrew/mktemp.rb +++ b/Library/Homebrew/mktemp.rb @@ -13,8 +13,8 @@ class Mktemp def initialize(prefix, opts = {}) @prefix = prefix - @retain_in_sources = opts[:retain_in_sources] - @retain = opts[:retain] || @retain_in_sources + @retain_in_cache = opts[:retain_in_cache] + @retain = opts[:retain] || @retain_in_cache @quiet = false end @@ -30,8 +30,8 @@ class Mktemp end # True if the source files should be retained. - def retain_in_sources? - @retain_in_sources + def retain_in_cache? + @retain_in_cache end # Instructs this Mktemp to not emit messages when retention is triggered. @@ -46,7 +46,7 @@ class Mktemp end def run - if @retain_in_sources + if @retain_in_cache source_dir = "#{HOMEBREW_CACHE}/Sources/#{@prefix.tr "@", "AT"}" chmod_rm_rf(source_dir) # clear out previous (otherwise not sure what happens) FileUtils.mkdir_p(source_dir) @@ -79,7 +79,7 @@ class Mktemp end ensure if retain? && !@tmpdir.nil? && !@quiet - message = retain_in_sources? ? "Source files for debugging available at:" : "Temporary files retained at:" + message = retain_in_cache? ? "Source files for debugging available at:" : "Temporary files retained at:" ohai message, @tmpdir.to_s end end diff --git a/Library/Homebrew/resource.rb b/Library/Homebrew/resource.rb index 2246ae42e4..b3a127bc59 100644 --- a/Library/Homebrew/resource.rb +++ b/Library/Homebrew/resource.rb @@ -236,7 +236,7 @@ class Resource protected def stage_resource(prefix, debug_symbols: false, &block) - Mktemp.new(prefix, retain_in_sources: debug_symbols).run(&block) + Mktemp.new(prefix, retain_in_cache: debug_symbols).run(&block) end private From 07e299760a0315d5af6c13dab172c2dba97d07c2 Mon Sep 17 00:00:00 2001 From: Lukas Oberhuber Date: Tue, 2 Aug 2022 08:33:40 -0700 Subject: [PATCH 25/42] mktemp: use "present?" instead of '!nil?" Co-authored-by: Mike McQuaid --- Library/Homebrew/mktemp.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Library/Homebrew/mktemp.rb b/Library/Homebrew/mktemp.rb index 1e958159f2..b0a394f319 100644 --- a/Library/Homebrew/mktemp.rb +++ b/Library/Homebrew/mktemp.rb @@ -78,7 +78,7 @@ class Mktemp ignore_interrupts { chmod_rm_rf(tmpdir) } unless retain? end ensure - if retain? && !@tmpdir.nil? && !@quiet + if retain? && @tmpdir.present? && !@quiet message = retain_in_cache? ? "Source files for debugging available at:" : "Temporary files retained at:" ohai message, @tmpdir.to_s end From d35f2e76a77f53cf2c6f75a1beb36b92e54d595b Mon Sep 17 00:00:00 2001 From: Lukas Oberhuber Date: Tue, 2 Aug 2022 08:39:02 -0700 Subject: [PATCH 26/42] Move Metrics/Blocklength disable to rubocop.yml --- Library/Homebrew/.rubocop.yml | 1 + Library/Homebrew/cmd/install.rb | 2 -- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/Library/Homebrew/.rubocop.yml b/Library/Homebrew/.rubocop.yml index 07b613ecb3..b5ba4ec572 100644 --- a/Library/Homebrew/.rubocop.yml +++ b/Library/Homebrew/.rubocop.yml @@ -21,6 +21,7 @@ Metrics/BlockLength: # TODO: extract more of the bottling logic - "dev-cmd/bottle.rb" - "test/**/*" + - "cmd/install.rb" Metrics/BlockNesting: Max: 5 Metrics/ClassLength: diff --git a/Library/Homebrew/cmd/install.rb b/Library/Homebrew/cmd/install.rb index 18f8034488..6844ef124b 100644 --- a/Library/Homebrew/cmd/install.rb +++ b/Library/Homebrew/cmd/install.rb @@ -20,7 +20,6 @@ module Homebrew module_function - # rubocop:disable Metrics/BlockLength sig { returns(CLI::Parser) } def install_args Homebrew::CLI::Parser.new do @@ -142,7 +141,6 @@ module Homebrew named_args [:formula, :cask], min: 1 end end - # rubocop:enable Metrics/BlockLength def install args = install_args.parse From 88a69b3de20e0faa12f8dee2c2298617d76b55ec Mon Sep 17 00:00:00 2001 From: Lukas Oberhuber Date: Tue, 2 Aug 2022 08:47:19 -0700 Subject: [PATCH 27/42] Restore previous style --- Library/Homebrew/build.rb | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/Library/Homebrew/build.rb b/Library/Homebrew/build.rb index 0722cb060c..ec430e04ba 100644 --- a/Library/Homebrew/build.rb +++ b/Library/Homebrew/build.rb @@ -78,8 +78,13 @@ class Build ENV.keg_only_deps = keg_only_deps ENV.deps = formula_deps ENV.run_time_deps = run_time_deps - ENV.setup_build_environment(formula: formula, cc: args.cc, build_bottle: args.build_bottle?, - bottle_arch: args.bottle_arch, debug_symbols: args.debug_symbols?) + ENV.setup_build_environment( + formula: formula, + cc: args.cc, + build_bottle: args.build_bottle?, + bottle_arch: args.bottle_arch, + debug_symbols: args.debug_symbols?, + ) reqs.each do |req| req.modify_build_environment( env: args.env, cc: args.cc, build_bottle: args.build_bottle?, bottle_arch: args.bottle_arch, @@ -87,8 +92,13 @@ class Build end deps.each(&:modify_build_environment) else - ENV.setup_build_environment(formula: formula, cc: args.cc, build_bottle: args.build_bottle?, - bottle_arch: args.bottle_arch, debug_symbols: args.debug_symbols?) + ENV.setup_build_environment( + formula: formula, + cc: args.cc, + build_bottle: args.build_bottle?, + bottle_arch: args.bottle_arch, + debug_symbols: args.debug_symbols?, + ) reqs.each do |req| req.modify_build_environment( env: args.env, cc: args.cc, build_bottle: args.build_bottle?, bottle_arch: args.bottle_arch, From 60831da3b82feb4f44e9b540e74eb929bb7b68c3 Mon Sep 17 00:00:00 2001 From: Lukas Oberhuber Date: Tue, 2 Aug 2022 08:50:37 -0700 Subject: [PATCH 28/42] DRY up formula prefix --- Library/Homebrew/mktemp.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Library/Homebrew/mktemp.rb b/Library/Homebrew/mktemp.rb index b0a394f319..e7b50a9740 100644 --- a/Library/Homebrew/mktemp.rb +++ b/Library/Homebrew/mktemp.rb @@ -46,13 +46,14 @@ class Mktemp end def run + prefix_name = @prefix.tr "@", "AT" if @retain_in_cache - source_dir = "#{HOMEBREW_CACHE}/Sources/#{@prefix.tr "@", "AT"}" + source_dir = "#{HOMEBREW_CACHE}/Sources/#{prefix_name}" chmod_rm_rf(source_dir) # clear out previous (otherwise not sure what happens) FileUtils.mkdir_p(source_dir) @tmpdir = Pathname.new(source_dir) else - @tmpdir = Pathname.new(Dir.mktmpdir("#{@prefix.tr "@", "AT"}-", HOMEBREW_TEMP)) + @tmpdir = Pathname.new(Dir.mktmpdir("#{prefix_name}-", HOMEBREW_TEMP)) end # Make sure files inside the temporary directory have the same group as the From 89c1d6812dac1c2c11d7c653cf5ec66302d80dd6 Mon Sep 17 00:00:00 2001 From: Lukas Oberhuber Date: Tue, 2 Aug 2022 08:53:15 -0700 Subject: [PATCH 29/42] Test sources are dropped in HOMEBREW_CACHE/Sources --- Library/Homebrew/test/cmd/install_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/Library/Homebrew/test/cmd/install_spec.rb b/Library/Homebrew/test/cmd/install_spec.rb index 3840f00f1e..dbf9d6a358 100644 --- a/Library/Homebrew/test/cmd/install_spec.rb +++ b/Library/Homebrew/test/cmd/install_spec.rb @@ -82,5 +82,6 @@ describe "brew install" do .and be_a_success expect(HOMEBREW_CELLAR/"testball1/0.1/bin/test").to be_a_file expect(HOMEBREW_CELLAR/"testball1/0.1/bin/test.dSYM").to be_a_directory if OS.mac? + expect(HOMEBREW_CACHE/"Sources/testball1").to be_a_directory end end From 2ce58f9fcba5a9c214d9f56c6101b1c5bab3a304 Mon Sep 17 00:00:00 2001 From: Lukas Oberhuber Date: Thu, 4 Aug 2022 08:52:49 -0700 Subject: [PATCH 30/42] fix removing of previous source --- Library/Homebrew/mktemp.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Library/Homebrew/mktemp.rb b/Library/Homebrew/mktemp.rb index e7b50a9740..0127327189 100644 --- a/Library/Homebrew/mktemp.rb +++ b/Library/Homebrew/mktemp.rb @@ -47,11 +47,11 @@ class Mktemp def run prefix_name = @prefix.tr "@", "AT" - if @retain_in_cache + if retain_in_cache? source_dir = "#{HOMEBREW_CACHE}/Sources/#{prefix_name}" - chmod_rm_rf(source_dir) # clear out previous (otherwise not sure what happens) - FileUtils.mkdir_p(source_dir) @tmpdir = Pathname.new(source_dir) + chmod_rm_rf(@tmpdir) # clear out previous (otherwise not sure what happens) + FileUtils.mkdir_p(source_dir) else @tmpdir = Pathname.new(Dir.mktmpdir("#{prefix_name}-", HOMEBREW_TEMP)) end From 0b554a41387e164b8aae668bc9eb2e459106d5b4 Mon Sep 17 00:00:00 2001 From: Lukas Oberhuber Date: Thu, 4 Aug 2022 08:53:07 -0700 Subject: [PATCH 31/42] Style --- Library/Homebrew/extend/os/mac/extend/ENV/shared.rb | 7 +++---- Library/Homebrew/extend/os/mac/extend/ENV/super.rb | 6 ++---- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/Library/Homebrew/extend/os/mac/extend/ENV/shared.rb b/Library/Homebrew/extend/os/mac/extend/ENV/shared.rb index 6ee43c0bb8..e2d47c2e76 100644 --- a/Library/Homebrew/extend/os/mac/extend/ENV/shared.rb +++ b/Library/Homebrew/extend/os/mac/extend/ENV/shared.rb @@ -6,10 +6,9 @@ module SharedEnvExtension def setup_build_environment(formula: nil, cc: nil, build_bottle: false, bottle_arch: nil, testing_formula: false, debug_symbols: false) - generic_shared_setup_build_environment( - formula: formula, cc: cc, build_bottle: build_bottle, bottle_arch: bottle_arch, - testing_formula: testing_formula, debug_symbols: debug_symbols - ) + generic_shared_setup_build_environment(formula: formula, cc: cc, build_bottle: build_bottle, + bottle_arch: bottle_arch, testing_formula: testing_formula, + debug_symbols: debug_symbols) # Normalise the system Perl version used, where multiple may be available self["VERSIONER_PERL_VERSION"] = MacOS.preferred_perl_version diff --git a/Library/Homebrew/extend/os/mac/extend/ENV/super.rb b/Library/Homebrew/extend/os/mac/extend/ENV/super.rb index 4c7d767fbb..af0482205c 100644 --- a/Library/Homebrew/extend/os/mac/extend/ENV/super.rb +++ b/Library/Homebrew/extend/os/mac/extend/ENV/super.rb @@ -101,10 +101,8 @@ module Superenv MacOS::CLT::PKG_PATH end - generic_setup_build_environment( - formula: formula, cc: cc, build_bottle: build_bottle, bottle_arch: bottle_arch, - testing_formula: testing_formula, debug_symbols: debug_symbols - ) + generic_setup_build_environment(formula: formula, cc: cc, build_bottle: build_bottle, bottle_arch: bottle_arch, + testing_formula: testing_formula, debug_symbols: debug_symbols) # Filter out symbols known not to be defined since GNU Autotools can't # reliably figure this out with Xcode 8 and above. From 6fbadb35e71fa9e37e11457b5efe3954163fcf5d Mon Sep 17 00:00:00 2001 From: Lukas Oberhuber Date: Fri, 5 Aug 2022 16:06:22 -0700 Subject: [PATCH 32/42] Style fix Co-authored-by: Mike McQuaid --- Library/Homebrew/extend/ENV.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Library/Homebrew/extend/ENV.rb b/Library/Homebrew/extend/ENV.rb index 54b293ee87..391a241305 100644 --- a/Library/Homebrew/extend/ENV.rb +++ b/Library/Homebrew/extend/ENV.rb @@ -46,7 +46,7 @@ module EnvActivation tmp_env = to_hash.dup.extend(EnvActivation) T.cast(tmp_env, EnvActivation).activate_extensions!(env: env) T.cast(tmp_env, T.any(Superenv, Stdenv)) - .setup_build_environment(cc: cc, build_bottle: build_bottle, bottle_arch: bottle_arch, + .setup_build_environment(cc: cc, build_bottle: build_bottle, bottle_arch: bottle_arch, debug_symbols: debug_symbols) replace(tmp_env) From 936d363c44be481bc6be207decac04778844d31a Mon Sep 17 00:00:00 2001 From: Lukas Oberhuber Date: Fri, 5 Aug 2022 16:12:39 -0700 Subject: [PATCH 33/42] Style Co-authored-by: Mike McQuaid --- Library/Homebrew/mktemp.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Library/Homebrew/mktemp.rb b/Library/Homebrew/mktemp.rb index 0127327189..ea3e767bdd 100644 --- a/Library/Homebrew/mktemp.rb +++ b/Library/Homebrew/mktemp.rb @@ -47,13 +47,13 @@ class Mktemp def run prefix_name = @prefix.tr "@", "AT" - if retain_in_cache? - source_dir = "#{HOMEBREW_CACHE}/Sources/#{prefix_name}" - @tmpdir = Pathname.new(source_dir) - chmod_rm_rf(@tmpdir) # clear out previous (otherwise not sure what happens) - FileUtils.mkdir_p(source_dir) + @tmpdir = if retain_in_cache? + tmpdir = HOMEBREW_CACHE/"Sources/#{prefix_name}" + chmod_rm_rf(tmpdir) # clear out previous staging directory + tmpdir.parent.mkpath + tmpdir else - @tmpdir = Pathname.new(Dir.mktmpdir("#{prefix_name}-", HOMEBREW_TEMP)) + Pathname.new(Dir.mktmpdir("#{prefix_name}-", HOMEBREW_TEMP)) end # Make sure files inside the temporary directory have the same group as the From 45d8f3789f3710e31a3bf2777a282f1671b02213 Mon Sep 17 00:00:00 2001 From: Lukas Oberhuber Date: Thu, 4 Aug 2022 18:11:35 -0700 Subject: [PATCH 34/42] Test of set_debug_symbols --- Library/Homebrew/test/ENV_spec.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Library/Homebrew/test/ENV_spec.rb b/Library/Homebrew/test/ENV_spec.rb index c09a50bca6..af0b71261f 100644 --- a/Library/Homebrew/test/ENV_spec.rb +++ b/Library/Homebrew/test/ENV_spec.rb @@ -205,5 +205,12 @@ describe "ENV" do expect(env["HOMEBREW_CCCFG"]).to include("g") end end + + describe "#set_debug_symbols" do + it "sets the debug symbols flag" do + env.set_debug_symbols + expect(env["HOMEBREW_CCCFG"]).to include("D") + end + end end end From 47bf7b8a8a08bbaf9d77d5238de961dce14b02af Mon Sep 17 00:00:00 2001 From: Lukas Oberhuber Date: Fri, 5 Aug 2022 16:18:20 -0700 Subject: [PATCH 35/42] Fix source cache directory creation --- Library/Homebrew/mktemp.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Library/Homebrew/mktemp.rb b/Library/Homebrew/mktemp.rb index ea3e767bdd..fcc0301e6e 100644 --- a/Library/Homebrew/mktemp.rb +++ b/Library/Homebrew/mktemp.rb @@ -50,7 +50,7 @@ class Mktemp @tmpdir = if retain_in_cache? tmpdir = HOMEBREW_CACHE/"Sources/#{prefix_name}" chmod_rm_rf(tmpdir) # clear out previous staging directory - tmpdir.parent.mkpath + tmpdir.mkpath tmpdir else Pathname.new(Dir.mktmpdir("#{prefix_name}-", HOMEBREW_TEMP)) From 683bcd92b04f8b13b75570c23bf1f6904db12f16 Mon Sep 17 00:00:00 2001 From: Lukas Oberhuber Date: Fri, 5 Aug 2022 16:54:32 -0700 Subject: [PATCH 36/42] Attempted linux test --- Library/Homebrew/test/cmd/install_spec.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Library/Homebrew/test/cmd/install_spec.rb b/Library/Homebrew/test/cmd/install_spec.rb index dbf9d6a358..e9b9949af0 100644 --- a/Library/Homebrew/test/cmd/install_spec.rb +++ b/Library/Homebrew/test/cmd/install_spec.rb @@ -82,6 +82,10 @@ describe "brew install" do .and be_a_success expect(HOMEBREW_CELLAR/"testball1/0.1/bin/test").to be_a_file expect(HOMEBREW_CELLAR/"testball1/0.1/bin/test.dSYM").to be_a_directory if OS.mac? + if OS.linux? + expect { system_command("objdump", "-h", "${HOMEBREW_CELLAR}/testball1/0.1/bin/test") } + .to output(%r{\.debug}).to_stdout + end expect(HOMEBREW_CACHE/"Sources/testball1").to be_a_directory end end From d6067418c1e2e2c4872d0318acfc8305f9eb5884 Mon Sep 17 00:00:00 2001 From: Lukas Oberhuber Date: Fri, 5 Aug 2022 17:00:54 -0700 Subject: [PATCH 37/42] Fix tmpdir take 2 --- Library/Homebrew/mktemp.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Library/Homebrew/mktemp.rb b/Library/Homebrew/mktemp.rb index fcc0301e6e..a691fcecc0 100644 --- a/Library/Homebrew/mktemp.rb +++ b/Library/Homebrew/mktemp.rb @@ -48,10 +48,10 @@ class Mktemp def run prefix_name = @prefix.tr "@", "AT" @tmpdir = if retain_in_cache? - tmpdir = HOMEBREW_CACHE/"Sources/#{prefix_name}" + tmp_dir = HOMEBREW_CACHE/"Sources/#{prefix_name}" chmod_rm_rf(tmpdir) # clear out previous staging directory - tmpdir.mkpath - tmpdir + tmp_dir.mkpath + tmp_dir else Pathname.new(Dir.mktmpdir("#{prefix_name}-", HOMEBREW_TEMP)) end @@ -68,15 +68,15 @@ class Mktemp Process.gid end begin - chown(nil, group_id, tmpdir) + chown(nil, group_id, @tmpdir) rescue Errno::EPERM - opoo "Failed setting group \"#{Etc.getgrgid(group_id).name}\" on #{tmpdir}" + opoo "Failed setting group \"#{Etc.getgrgid(group_id).name}\" on #{@tmpdir}" end begin Dir.chdir(tmpdir) { yield self } ensure - ignore_interrupts { chmod_rm_rf(tmpdir) } unless retain? + ignore_interrupts { chmod_rm_rf(@tmpdir) } unless retain? end ensure if retain? && @tmpdir.present? && !@quiet From f930dd58a04c82a510ed8942cef9129c8933fe1d Mon Sep 17 00:00:00 2001 From: Lukas Oberhuber Date: Fri, 5 Aug 2022 17:02:10 -0700 Subject: [PATCH 38/42] Fix style --- Library/Homebrew/test/cmd/install_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Library/Homebrew/test/cmd/install_spec.rb b/Library/Homebrew/test/cmd/install_spec.rb index e9b9949af0..2a13c187bd 100644 --- a/Library/Homebrew/test/cmd/install_spec.rb +++ b/Library/Homebrew/test/cmd/install_spec.rb @@ -84,7 +84,7 @@ describe "brew install" do expect(HOMEBREW_CELLAR/"testball1/0.1/bin/test.dSYM").to be_a_directory if OS.mac? if OS.linux? expect { system_command("objdump", "-h", "${HOMEBREW_CELLAR}/testball1/0.1/bin/test") } - .to output(%r{\.debug}).to_stdout + .to output(/\.debug/).to_stdout end expect(HOMEBREW_CACHE/"Sources/testball1").to be_a_directory end From 19a66e75c445decd66bf16f1778e9e880fad1460 Mon Sep 17 00:00:00 2001 From: Lukas Oberhuber Date: Sat, 6 Aug 2022 10:06:32 -0700 Subject: [PATCH 39/42] Second try at objdump call for linux --- Library/Homebrew/test/cmd/install_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Library/Homebrew/test/cmd/install_spec.rb b/Library/Homebrew/test/cmd/install_spec.rb index 2a13c187bd..82cdcddacc 100644 --- a/Library/Homebrew/test/cmd/install_spec.rb +++ b/Library/Homebrew/test/cmd/install_spec.rb @@ -83,7 +83,7 @@ describe "brew install" do expect(HOMEBREW_CELLAR/"testball1/0.1/bin/test").to be_a_file expect(HOMEBREW_CELLAR/"testball1/0.1/bin/test.dSYM").to be_a_directory if OS.mac? if OS.linux? - expect { system_command("objdump", "-h", "${HOMEBREW_CELLAR}/testball1/0.1/bin/test") } + expect { system_command("objdump", args: ["-h", "${HOMEBREW_CELLAR}/testball1/0.1/bin/test"]) } .to output(/\.debug/).to_stdout end expect(HOMEBREW_CACHE/"Sources/testball1").to be_a_directory From aa2682a098fd84c9edbbe54350f0e9a796a312b3 Mon Sep 17 00:00:00 2001 From: Lukas Oberhuber Date: Wed, 10 Aug 2022 16:55:19 -1000 Subject: [PATCH 40/42] These tests don't test anything On MacOS, the compile flags `-g` are not set, and I can't figure out how to set them here. `dsymutil` runs successfully regardless of if there are debug symbols or not. Same on linux therefore the test cannot succeed. --- Library/Homebrew/test/cmd/install_spec.rb | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/Library/Homebrew/test/cmd/install_spec.rb b/Library/Homebrew/test/cmd/install_spec.rb index 82cdcddacc..2eb96de51d 100644 --- a/Library/Homebrew/test/cmd/install_spec.rb +++ b/Library/Homebrew/test/cmd/install_spec.rb @@ -74,17 +74,26 @@ describe "brew install" do end it "installs formulae with debug symbols", :integration_test do - setup_test_formula "testball1" + setup_test_formula "testball1", <<~RUBY + def install + prefix.install Dir["*"] + (buildpath/"test.c").write \ + "#include \\nint main(){printf(\\"test\\");return 0;}" + bin.mkpath + system ENV.cc, "test.c", "-o", bin/"test" + end + RUBY expect { brew "install", "testball1", "--debug-symbols", "--build-from-source" } .to output(%r{#{HOMEBREW_CELLAR}/testball1/0\.1}o).to_stdout .and not_to_output.to_stderr .and be_a_success expect(HOMEBREW_CELLAR/"testball1/0.1/bin/test").to be_a_file - expect(HOMEBREW_CELLAR/"testball1/0.1/bin/test.dSYM").to be_a_directory if OS.mac? + expect(HOMEBREW_CELLAR/"testball1/0.1/bin/test.dSYM/Contents/Resources/DWARF/test").to be_a_file if OS.mac? if OS.linux? - expect { system_command("objdump", args: ["-h", "${HOMEBREW_CELLAR}/testball1/0.1/bin/test"]) } - .to output(/\.debug/).to_stdout + # raise system_command("ls", args: ["-lasR", HOMEBREW_CELLAR/"testball1/0.1"]).merged_output + # expect { system_command("objdump", args: ["-h", HOMEBREW_CELLAR/"testball1/0.1/bin/test"]) } + # .to output(/\.debug/).to_stdout end expect(HOMEBREW_CACHE/"Sources/testball1").to be_a_directory end From 2c829380b5ea18ed32595b89560b46b23887ce65 Mon Sep 17 00:00:00 2001 From: Lukas Oberhuber Date: Thu, 11 Aug 2022 10:54:18 -1000 Subject: [PATCH 41/42] Test that --debug-symbols succeeds Due to limitations of the test framework, this only tests that the command with the --debug-symbols flag succeeds and that on MacOS the `dsymutil` is run. --- Library/Homebrew/test/cmd/install_spec.rb | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/Library/Homebrew/test/cmd/install_spec.rb b/Library/Homebrew/test/cmd/install_spec.rb index 2eb96de51d..c7a4bba552 100644 --- a/Library/Homebrew/test/cmd/install_spec.rb +++ b/Library/Homebrew/test/cmd/install_spec.rb @@ -74,15 +74,7 @@ describe "brew install" do end it "installs formulae with debug symbols", :integration_test do - setup_test_formula "testball1", <<~RUBY - def install - prefix.install Dir["*"] - (buildpath/"test.c").write \ - "#include \\nint main(){printf(\\"test\\");return 0;}" - bin.mkpath - system ENV.cc, "test.c", "-o", bin/"test" - end - RUBY + setup_test_formula "testball1" expect { brew "install", "testball1", "--debug-symbols", "--build-from-source" } .to output(%r{#{HOMEBREW_CELLAR}/testball1/0\.1}o).to_stdout @@ -90,11 +82,6 @@ describe "brew install" do .and be_a_success expect(HOMEBREW_CELLAR/"testball1/0.1/bin/test").to be_a_file expect(HOMEBREW_CELLAR/"testball1/0.1/bin/test.dSYM/Contents/Resources/DWARF/test").to be_a_file if OS.mac? - if OS.linux? - # raise system_command("ls", args: ["-lasR", HOMEBREW_CELLAR/"testball1/0.1"]).merged_output - # expect { system_command("objdump", args: ["-h", HOMEBREW_CELLAR/"testball1/0.1/bin/test"]) } - # .to output(/\.debug/).to_stdout - end expect(HOMEBREW_CACHE/"Sources/testball1").to be_a_directory end end From 04ff6a18f4e4fa4cdd5e4da39dca1127eaf443ef Mon Sep 17 00:00:00 2001 From: Mike McQuaid Date: Fri, 12 Aug 2022 09:34:51 +0100 Subject: [PATCH 42/42] Tweak --debug-symbols description. --- Library/Homebrew/cmd/install.rb | 3 +-- Library/Homebrew/cmd/reinstall.rb | 3 +-- Library/Homebrew/cmd/upgrade.rb | 3 +-- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/Library/Homebrew/cmd/install.rb b/Library/Homebrew/cmd/install.rb index e4d6ab4b47..a312190440 100644 --- a/Library/Homebrew/cmd/install.rb +++ b/Library/Homebrew/cmd/install.rb @@ -93,8 +93,7 @@ module Homebrew }], [:switch, "--debug-symbols", { depends_on: "--build-from-source", - description: "Generate debug symbols on build. Source will be retained in a temporary directory. " \ - "(see --keep-tmp)", + description: "Generate debug symbols on build. Source will be retained in a cache directory. ", }], [:switch, "--build-bottle", { description: "Prepare the formula for eventual bottling during installation, skipping any " \ diff --git a/Library/Homebrew/cmd/reinstall.rb b/Library/Homebrew/cmd/reinstall.rb index e7d8331dcc..62f4777e0d 100644 --- a/Library/Homebrew/cmd/reinstall.rb +++ b/Library/Homebrew/cmd/reinstall.rb @@ -59,8 +59,7 @@ module Homebrew }], [:switch, "--debug-symbols", { depends_on: "--build-from-source", - description: "Generate debug symbols on build. Source will be retained in a temporary directory. " \ - "(see --keep-tmp)", + description: "Generate debug symbols on build. Source will be retained in a cache directory. ", }], [:switch, "--display-times", { env: :display_install_times, diff --git a/Library/Homebrew/cmd/upgrade.rb b/Library/Homebrew/cmd/upgrade.rb index 3da6d1232e..f176a26a5b 100644 --- a/Library/Homebrew/cmd/upgrade.rb +++ b/Library/Homebrew/cmd/upgrade.rb @@ -70,8 +70,7 @@ module Homebrew }], [:switch, "--debug-symbols", { depends_on: "--build-from-source", - description: "Generate debug symbols on build. Source will be retained in a temporary directory. " \ - "(see --keep-tmp)", + description: "Generate debug symbols on build. Source will be retained in a cache directory. ", }], [:switch, "--display-times", { env: :display_install_times,