Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot create bottle for tcl-tk with a newer brew than 3.3.11 #12832

Closed
2 tasks done
imresteiner opened this issue Feb 3, 2022 · 24 comments · Fixed by #12864
Closed
2 tasks done

Cannot create bottle for tcl-tk with a newer brew than 3.3.11 #12832

imresteiner opened this issue Feb 3, 2022 · 24 comments · Fixed by #12864
Labels
bug Reproducible Homebrew/brew bug outdated PR was locked due to age

Comments

@imresteiner
Copy link

imresteiner commented Feb 3, 2022

brew config output

> brew config
HOMEBREW_VERSION: 3.3.13-13-g62b82c0
ORIGIN: https://git.balabit/macos/homebrew.git
HEAD: 62b82c0792a3ad21584e6b1070214d2b1beaeded
Last commit: 12 seconds ago
Core tap ORIGIN: https://git.balabit/macos/homebrew-repo.git
Core tap HEAD: 88ac7fce91b3f1785d8d51767045ba5d8aa15253
Core tap last commit: 23 hours ago
Core tap branch: upgrade-formulas
HOMEBREW_PREFIX: /opt/bb-brew
HOMEBREW_CACHE: /opt/bb-brew/Caches
HOMEBREW_CASK_OPTS: []
HOMEBREW_MAKE_JOBS: 4
HOMEBREW_NO_ANALYTICS: set
HOMEBREW_NO_AUTO_UPDATE: set
Homebrew Ruby: 2.6.8 => /opt/bb-brew/Homebrew/Library/Homebrew/vendor/portable-ruby/2.6.8/bin/ruby
CPU: quad-core 64-bit kabylake
Clang: 11.0.3 build 1103
Git: 2.24.3 => /Applications/Xcode.app/Contents/Developer/usr/bin/git
Curl: 7.64.1 => /usr/bin/curl
macOS: 10.15.7-x86_64
CLT: 12.0.0.32.29
Xcode: 11.7

brew doctor output

> brew doctor 
Please note that these warnings are just used to help the Homebrew maintainers
with debugging if you file an issue. If everything you use Homebrew for is
working fine: please don't worry or file an issue; just ignore this. Thanks!

Warning: Some taps are not on the default git origin branch and may not receive
updates. If this is a surprise to you, check out the default branch with:
  git -C $(brew --repo homebrew/core) checkout master

Warning: Your Xcode (11.7) is outdated.
Please update to Xcode 12.4 (or delete it).
Xcode can be updated from the App Store.

Verification

  • I ran brew update and am still able to reproduce my issue.
  • I have resolved all warnings from brew doctor and that did not fix my problem.

What were you trying to do (and why)?

I would like to update the formulas we are using to build our product. To release the binaries another git branch was created which is reseted to the homebrew-core repo's master.

What happened (include all command output)?

brew bottle --json tcl-tk command failed.

output:

==> Determining tcl-tk bottle rebuild...
==> Bottling tcl-tk--8.6.12_1.catalina.bottle.tar.gz...
Error: new ID must be a String
Do not report this issue until you've run `brew update` and tried again.
/opt/bb-brew/Homebrew/Library/Homebrew/vendor/bundle/ruby/2.6.0/gems/ruby-macho-3.0.0/lib/macho/macho_file.rb:318:in `change_dylib_id'
/opt/bb-brew/Homebrew/Library/Homebrew/vendor/bundle/ruby/2.6.0/gems/ruby-macho-3.0.0/lib/macho/tools.rb:26:in `change_dylib_id'
/opt/bb-brew/Homebrew/Library/Homebrew/os/mac/keg.rb:10:in `change_dylib_id'
/opt/bb-brew/Homebrew/Library/Homebrew/extend/os/mac/keg_relocate.rb:26:in `block (2 levels) in relocate_dynamic_linkage'
/opt/bb-brew/Homebrew/Library/Homebrew/extend/pathname.rb:341:in `ensure_writable'
/opt/bb-brew/Homebrew/Library/Homebrew/extend/os/mac/keg_relocate.rb:23:in `block in relocate_dynamic_linkage'
/opt/bb-brew/Homebrew/Library/Homebrew/extend/os/mac/keg_relocate.rb:22:in `each'
/opt/bb-brew/Homebrew/Library/Homebrew/extend/os/mac/keg_relocate.rb:22:in `relocate_dynamic_linkage'
/opt/bb-brew/Homebrew/Library/Homebrew/keg_relocate.rb:130:in `replace_placeholders_with_locations'
/opt/bb-brew/Homebrew/Library/Homebrew/dev-cmd/bottle.rb:501:in `block (2 levels) in bottle_formula'
/opt/bb-brew/Homebrew/Library/Homebrew/utils.rb:399:in `ignore_interrupts'
/opt/bb-brew/Homebrew/Library/Homebrew/dev-cmd/bottle.rb:499:in `ensure in block in bottle_formula'
/opt/bb-brew/Homebrew/Library/Homebrew/dev-cmd/bottle.rb:499:in `block in bottle_formula'
/opt/bb-brew/Homebrew/Library/Homebrew/keg.rb:334:in `block in lock'
/opt/bb-brew/Homebrew/Library/Homebrew/lock_file.rb:35:in `with_lock'
/opt/bb-brew/Homebrew/Library/Homebrew/keg.rb:330:in `lock'
/opt/bb-brew/Homebrew/Library/Homebrew/dev-cmd/bottle.rb:390:in `bottle_formula'
/opt/bb-brew/Homebrew/Library/Homebrew/dev-cmd/bottle.rb:105:in `block in bottle'
/opt/bb-brew/Homebrew/Library/Homebrew/dev-cmd/bottle.rb:104:in `each'
/opt/bb-brew/Homebrew/Library/Homebrew/dev-cmd/bottle.rb:104:in `bottle'
/opt/bb-brew/Homebrew/Library/Homebrew/brew.rb:110:in `<main>'

After a short investigation it seems an empty id is generated for the lib at Library/Homebrew/extend/os/mac/keg_relocate.rb:25
==> FILE: /opt/bb-brew/Cellar/tcl-tk/8.6.12_1/lib/tcltls1.7.22/tcltls.dylib
==> FILE.DYLIB_ID shared-tcltls.dylib
==> ID:

With version 3.3.11 brew can build the bottle successfully. With a newer version I could reproduce the issue.

What did you expect to happen?

The json config is generated to create a bottle afterwards.

Step-by-step reproduction instructions (by running brew commands)

> brew install --build-bottle tcl-tk
> brew bottle --json tcl-tk
> brew bottle --merge tcl-tk*.json --write
@imresteiner imresteiner added the bug Reproducible Homebrew/brew bug label Feb 3, 2022
@MikeMcQuaid MikeMcQuaid transferred this issue from Homebrew/brew Feb 3, 2022
@request-info

This comment was marked as resolved.

@request-info request-info bot added the needs response Needs a response from the issue/PR author label Feb 3, 2022
@carlocab
Copy link
Member

carlocab commented Feb 3, 2022

What's the output of

otool -L /opt/bb-brew/Cellar/tcl-tk/8.6.12_1/lib/tcltls1.7.22/tcltls.dylib

?

@carlocab
Copy link
Member

carlocab commented Feb 3, 2022

I'm going to guess the install name of tclts.dylib is just tclts.dylib. If so, the bug is probably here:

When the install name is just a basename file.dirname is just ., which causes problems later on.

@carlocab
Copy link
Member

carlocab commented Feb 3, 2022

I'm confused about when this problem was introduced though, even supposing I'm correct. Any chance you could do a git bisect to find the problematic commit?

@imresteiner
Copy link
Author

otool -L /opt/bb-brew/Cellar/tcl-tk/8.6.12_1/lib/tcltls1.7.22/tcltls.dylib
/opt/bb-brew/Cellar/tcl-tk/8.6.12_1/lib/tcltls1.7.22/tcltls.dylib:
shared-tcltls.dylib (compatibility version 0.0.0, current version 0.0.0)
/opt/bb-brew/opt/openssl@1.1/lib/libssl.1.1.dylib (compatibility version 1.1.0, current version 1.1.0)
/opt/bb-brew/opt/openssl@1.1/lib/libcrypto.1.1.dylib (compatibility version 1.1.0, current version 1.1.0)
/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1281.100.1)

@carlocab
Copy link
Member

carlocab commented Feb 3, 2022

Well, close enough. Still just a basename: shared-tcltls.dylib

@carlocab
Copy link
Member

carlocab commented Feb 3, 2022

@MikeMcQuaid I think this is a bug in brew, so I'm transferring it back.

@carlocab carlocab transferred this issue from Homebrew/homebrew-core Feb 3, 2022
@request-info

This comment was marked as resolved.

@carlocab carlocab removed the needs response Needs a response from the issue/PR author label Feb 3, 2022
@imresteiner
Copy link
Author

Finally finished with the git bisect.
It seems this commit brought in the issue:

568bc94 is the first bad commit
commit 568bc94
Author: Carlo Cabrera 30379873+carlocab@users.noreply.github.com
Date: Wed Jan 19 00:34:39 2022 +0800

os/mac/keg: use `MachOFile#delete_rpath` instead of `MachO::Tools`

This will allow us to avoid keeping track of the number of `RPATH`s
while trying to delete duplicates.

See discussion at #12745.

Library/Homebrew/extend/os/mac/keg_relocate.rb | 10 +---------
Library/Homebrew/os/mac/keg.rb | 2 +-
Library/Homebrew/os/mac/mach.rb | 7 +++++++
3 files changed, 9 insertions(+), 10 deletions(-)

@carlocab
Copy link
Member

carlocab commented Feb 3, 2022

Thanks. That's the only closely related change recently, so it makes sense. On the other hand, I don't see how that affects what currently looks like unrelated code...

@carlocab
Copy link
Member

carlocab commented Feb 3, 2022

Can you show the output of brew bottle --verbose --debug --json tcl-tk?

@imresteiner
Copy link
Author

imresteiner commented Feb 3, 2022

brew bottle --json --verbose --debug tcl-tk
/opt/bb-brew/Homebrew/Library/Homebrew/brew.rb (Formulary::FormulaLoader): loading /opt/bb-brew/Homebrew/Library/Taps/homebrew/homebrew-core/Formula/tcl-tk.rb
==> Determining tcl-tk bottle rebuild...
/opt/bb-brew/Homebrew/Library/Homebrew/brew.rb (Formulary::FormulaContentsLoader): loading /opt/bb-brew/Homebrew/Library/Taps/homebrew/homebrew-core/Formula/tcl-tk.rb
/opt/bb-brew/Homebrew/Library/Homebrew/shims/shared/git --version
==> Bottling tcl-tk--8.6.12_1.catalina.bottle.tar.gz...
==> Changing install name in /opt/bb-brew/Cellar/tcl-tk/8.6.12_1/bin/tclsh8.6
  from /opt/bb-brew/Cellar/tcl-tk/8.6.12_1/lib/libtcl8.6.dylib
    to @@HOMEBREW_CELLAR@@/tcl-tk/8.6.12_1/lib/libtcl8.6.dylib
==> Changing install name in /opt/bb-brew/Cellar/tcl-tk/8.6.12_1/bin/wish8.6
  from /opt/bb-brew/Cellar/tcl-tk/8.6.12_1/lib/libtk8.6.dylib
    to @@HOMEBREW_CELLAR@@/tcl-tk/8.6.12_1/lib/libtk8.6.dylib
==> Changing install name in /opt/bb-brew/Cellar/tcl-tk/8.6.12_1/bin/wish8.6
  from /opt/bb-brew/Cellar/tcl-tk/8.6.12_1/lib/libtcl8.6.dylib
    to @@HOMEBREW_CELLAR@@/tcl-tk/8.6.12_1/lib/libtcl8.6.dylib
==> Changing dylib ID of /opt/bb-brew/Cellar/tcl-tk/8.6.12_1/lib/itcl4.2.2/libitcl4.2.2.dylib
  from /opt/bb-brew/opt/tcl-tk/lib/itcl4.2.2/libitcl4.2.2.dylib
    to @@HOMEBREW_PREFIX@@/opt/tcl-tk/lib/itcl4.2.2/libitcl4.2.2.dylib
==> Changing dylib ID of /opt/bb-brew/Cellar/tcl-tk/8.6.12_1/lib/itk4.1.0/libitk4.1.0.dylib
  from /opt/bb-brew/opt/tcl-tk/lib/itk4.1.0/libitk4.1.0.dylib
    to @@HOMEBREW_PREFIX@@/opt/tcl-tk/lib/itk4.1.0/libitk4.1.0.dylib
==> Changing dylib ID of /opt/bb-brew/Cellar/tcl-tk/8.6.12_1/lib/libtcl8.6.dylib
  from /opt/bb-brew/opt/tcl-tk/lib/libtcl8.6.dylib
    to @@HOMEBREW_PREFIX@@/opt/tcl-tk/lib/libtcl8.6.dylib
==> Changing dylib ID of /opt/bb-brew/Cellar/tcl-tk/8.6.12_1/lib/libtk8.6.dylib
  from /opt/bb-brew/opt/tcl-tk/lib/libtk8.6.dylib
    to @@HOMEBREW_PREFIX@@/opt/tcl-tk/lib/libtk8.6.dylib
==> Changing dylib ID of /opt/bb-brew/Cellar/tcl-tk/8.6.12_1/lib/sqlite3.36.0/libsqlite3.36.0.dylib
  from /opt/bb-brew/opt/tcl-tk/lib/sqlite3.36.0/libsqlite3.36.0.dylib
    to @@HOMEBREW_PREFIX@@/opt/tcl-tk/lib/sqlite3.36.0/libsqlite3.36.0.dylib
==> Changing dylib ID of /opt/bb-brew/Cellar/tcl-tk/8.6.12_1/lib/tcllibc/macosx-x86_64-clang/tcllibc.dylib
  from /opt/bb-brew/opt/tcl-tk/lib/tcllibc/macosx-x86_64-clang/v3118_00000000000000000000000000000154.dylib
    to @@HOMEBREW_PREFIX@@/opt/tcl-tk/lib/tcllibc/macosx-x86_64-clang/v3118_00000000000000000000000000000154.dylib
==> Changing dylib ID of /opt/bb-brew/Cellar/tcl-tk/8.6.12_1/lib/tcltls1.7.22/tcltls.dylib
  from shared-tcltls.dylib
    to 
==> Changing install name in /opt/bb-brew/Cellar/tcl-tk/8.6.12_1/bin/tclsh8.6
  from @@HOMEBREW_CELLAR@@/tcl-tk/8.6.12_1/lib/libtcl8.6.dylib
    to /opt/bb-brew/Cellar/tcl-tk/8.6.12_1/lib/libtcl8.6.dylib
==> Changing install name in /opt/bb-brew/Cellar/tcl-tk/8.6.12_1/bin/wish8.6
  from @@HOMEBREW_CELLAR@@/tcl-tk/8.6.12_1/lib/libtk8.6.dylib
    to /opt/bb-brew/Cellar/tcl-tk/8.6.12_1/lib/libtk8.6.dylib
==> Changing install name in /opt/bb-brew/Cellar/tcl-tk/8.6.12_1/bin/wish8.6
  from @@HOMEBREW_CELLAR@@/tcl-tk/8.6.12_1/lib/libtcl8.6.dylib
    to /opt/bb-brew/Cellar/tcl-tk/8.6.12_1/lib/libtcl8.6.dylib
==> Changing dylib ID of /opt/bb-brew/Cellar/tcl-tk/8.6.12_1/lib/itcl4.2.2/libitcl4.2.2.dylib
  from @@HOMEBREW_PREFIX@@/opt/tcl-tk/lib/itcl4.2.2/libitcl4.2.2.dylib
    to /opt/bb-brew/opt/tcl-tk/lib/itcl4.2.2/libitcl4.2.2.dylib
==> Changing dylib ID of /opt/bb-brew/Cellar/tcl-tk/8.6.12_1/lib/itk4.1.0/libitk4.1.0.dylib
  from @@HOMEBREW_PREFIX@@/opt/tcl-tk/lib/itk4.1.0/libitk4.1.0.dylib
    to /opt/bb-brew/opt/tcl-tk/lib/itk4.1.0/libitk4.1.0.dylib
==> Changing dylib ID of /opt/bb-brew/Cellar/tcl-tk/8.6.12_1/lib/libtcl8.6.dylib
  from @@HOMEBREW_PREFIX@@/opt/tcl-tk/lib/libtcl8.6.dylib
    to /opt/bb-brew/opt/tcl-tk/lib/libtcl8.6.dylib
==> Changing dylib ID of /opt/bb-brew/Cellar/tcl-tk/8.6.12_1/lib/libtk8.6.dylib
  from @@HOMEBREW_PREFIX@@/opt/tcl-tk/lib/libtk8.6.dylib
    to /opt/bb-brew/opt/tcl-tk/lib/libtk8.6.dylib
==> Changing dylib ID of /opt/bb-brew/Cellar/tcl-tk/8.6.12_1/lib/sqlite3.36.0/libsqlite3.36.0.dylib
  from @@HOMEBREW_PREFIX@@/opt/tcl-tk/lib/sqlite3.36.0/libsqlite3.36.0.dylib
    to /opt/bb-brew/opt/tcl-tk/lib/sqlite3.36.0/libsqlite3.36.0.dylib
==> Changing dylib ID of /opt/bb-brew/Cellar/tcl-tk/8.6.12_1/lib/tcllibc/macosx-x86_64-clang/tcllibc.dylib
  from @@HOMEBREW_PREFIX@@/opt/tcl-tk/lib/tcllibc/macosx-x86_64-clang/v3118_00000000000000000000000000000154.dylib
    to /opt/bb-brew/opt/tcl-tk/lib/tcllibc/macosx-x86_64-clang/v3118_00000000000000000000000000000154.dylib
==> Changing dylib ID of /opt/bb-brew/Cellar/tcl-tk/8.6.12_1/lib/tcltls1.7.22/tcltls.dylib
  from shared-tcltls.dylib
    to 
Error: new ID must be a String
/opt/bb-brew/Homebrew/Library/Homebrew/vendor/bundle/ruby/2.6.0/gems/ruby-macho-3.0.0/lib/macho/macho_file.rb:318:in `change_dylib_id'
/opt/bb-brew/Homebrew/Library/Homebrew/vendor/bundle/ruby/2.6.0/gems/ruby-macho-3.0.0/lib/macho/tools.rb:26:in `change_dylib_id'
/opt/bb-brew/Homebrew/Library/Homebrew/os/mac/keg.rb:10:in `change_dylib_id'
/opt/bb-brew/Homebrew/Library/Homebrew/extend/os/mac/keg_relocate.rb:26:in `block (2 levels) in relocate_dynamic_linkage'
/opt/bb-brew/Homebrew/Library/Homebrew/extend/pathname.rb:341:in `ensure_writable'
/opt/bb-brew/Homebrew/Library/Homebrew/extend/os/mac/keg_relocate.rb:23:in `block in relocate_dynamic_linkage'
/opt/bb-brew/Homebrew/Library/Homebrew/extend/os/mac/keg_relocate.rb:22:in `each'
/opt/bb-brew/Homebrew/Library/Homebrew/extend/os/mac/keg_relocate.rb:22:in `relocate_dynamic_linkage'
/opt/bb-brew/Homebrew/Library/Homebrew/keg_relocate.rb:130:in `replace_placeholders_with_locations'
/opt/bb-brew/Homebrew/Library/Homebrew/dev-cmd/bottle.rb:501:in `block (2 levels) in bottle_formula'
/opt/bb-brew/Homebrew/Library/Homebrew/utils.rb:399:in `ignore_interrupts'
/opt/bb-brew/Homebrew/Library/Homebrew/dev-cmd/bottle.rb:499:in `ensure in block in bottle_formula'
/opt/bb-brew/Homebrew/Library/Homebrew/dev-cmd/bottle.rb:499:in `block in bottle_formula'
/opt/bb-brew/Homebrew/Library/Homebrew/keg.rb:334:in `block in lock'
/opt/bb-brew/Homebrew/Library/Homebrew/lock_file.rb:35:in `with_lock'
/opt/bb-brew/Homebrew/Library/Homebrew/keg.rb:330:in `lock'
/opt/bb-brew/Homebrew/Library/Homebrew/dev-cmd/bottle.rb:390:in `bottle_formula'
/opt/bb-brew/Homebrew/Library/Homebrew/dev-cmd/bottle.rb:105:in `block in bottle'
/opt/bb-brew/Homebrew/Library/Homebrew/dev-cmd/bottle.rb:104:in `each'
/opt/bb-brew/Homebrew/Library/Homebrew/dev-cmd/bottle.rb:104:in `bottle'
/opt/bb-brew/Homebrew/Library/Homebrew/brew.rb:110:in `<main>'

@carlocab
Copy link
Member

carlocab commented Feb 3, 2022

I can reproduce this, but I'm still not quite sure why it's happening. Just to double-check: what is the output of brew install --debug tcl-tk?

@imresteiner
Copy link
Author

console_log.log

@gromgit
Copy link
Member

gromgit commented Feb 7, 2022

FWIW, I'm seeing the same issue when bottling grpc, failing on both Intel Mojave and ARM64 Monterey. Here are the Monterey debug-verbose logs, in case it helps troubleshoot the issue:

grpc.build-bottle.log
grpc-bottle.log

@carlocab
Copy link
Member

carlocab commented Feb 7, 2022

My last theory was that this is a regression in ruby-macho, but I, unfortunately, have not been able to reproduce the problematic behaviour using just ruby-macho.

The problem is that tcltls.dylib has an install name of shared-tcltls.dylib (i.e. basename-only). Normally, after the install, fix_dynamic_linkage will turn this into an absolute path (i.e. something like /usr/local/opt/tcl-tk/lib/tcltls1.7.22/tcltls.dylib).

For some reason, fix_dynamic_linkage fails to do this, but doesn't complain about it. This only proves to be a problem when we try to bottle tcl-tk, and the call to change_dylib_id here chokes because the new ID is for some reason an empty string.

The right fix is probably to make sure fix_dynamic_linkage gets the dylib ID change right the first time (this is probably dylib_id_for).

But that still leaves open the question of how this even got broken in the first place. You could try reverting my commit from the git bisect above, but I would be very surprised if it helped. (Maybe there's some edge case I'm not seeing.)

@Pencab

This comment was marked as off-topic.

@gromgit
Copy link
Member

gromgit commented Feb 11, 2022

@carlocab, I can confirm that grpc correctly bottles after reverting 568bc94. Here's the brew install --build-bottle and otool logs before reverting:

grpc.build-bottle.v1.log
grpc.otool.v1.log

and after reverting:

grpc.build-bottle.v2.log
grpc.otool.v2.log

Note that pre-revert, even though the log says:

==> Changing dylib ID of /opt/homebrew/Cellar/grpc/1.43.2/lib/libgrpc++_test_config.1.43.2.dylib
  from @rpath/libgrpc++_test_config.1.43.dylib
    to /opt/homebrew/opt/grpc/lib/libgrpc++_test_config.1.43.dylib

the change doesn't seem to be applied:

/opt/homebrew/opt/grpc/lib/libgrpc++_test_config.1.43.2.dylib:
	@rpath/libgrpc++_test_config.1.43.dylib (compatibility version 1.43.0, current version 1.43.2)

@carlocab
Copy link
Member

One thing I am confused about: 568bc94 only affects the rewriting of RPATHs, but the problem in brew is about the rewriting of dylib IDs. Why does the implementation of RPATH rewriting affect dylib ID rewriting?

@gromgit
Copy link
Member

gromgit commented Feb 11, 2022

If I'm reading the commit correctly, it replaces an open-edit-write sequence on a dylib filename (MachO::Tools.delete_rpath) with an edit-write sequence on an already-open dylib (Pathname.delete_rpath). The former should be robust, since it has to read the file's contents before editing, but perhaps there's a cache conflict or some similar issue in the latter logic?

@carlocab
Copy link
Member

I still don't see how that's related to our reading of dylib IDs (and subsequent re-reading and replacing) from the cache, though.

@gromgit
Copy link
Member

gromgit commented Feb 11, 2022

I'm not sure either, and I'm definitely not familiar with the codebase, but since that's the only effective logic difference, that suggests a failure to commit existing changes somewhere, or overwritten changes.

Some years back, I wrote some code that ending up doing something like the following:

  • read file F into buffer A
  • make change in A
  • read file F into buffer B
  • make change B
  • flush A to file
  • flush B to file

Needless to say, the changes in A disappeared without a trace.

@carlocab
Copy link
Member

carlocab commented Feb 11, 2022

Ah, yes, I see. Here's what's happening:

  1. read file F into buffer A
  2. make change in F
  3. make change in A
  4. flush A to F

Thus our changes from 2 disappear. Do you mind testing this patch? I'll open a PR for it shortly.

diff --git a/Library/Homebrew/os/mac/keg.rb b/Library/Homebrew/os/mac/keg.rb
index ef4395bb6..ead4ad6d7 100644
--- a/Library/Homebrew/os/mac/keg.rb
+++ b/Library/Homebrew/os/mac/keg.rb
@@ -7,7 +7,7 @@ class Keg
 
     @require_relocation = true
     odebug "Changing dylib ID of #{file}\n  from #{file.dylib_id}\n    to #{id}"
-    MachO::Tools.change_dylib_id(file, id, strict: false)
+    file.change_dylib_id(id, strict: false)
     apply_ad_hoc_signature(file)
   rescue MachO::MachOError
     onoe <<~EOS
@@ -23,7 +23,7 @@ class Keg
 
     @require_relocation = true
     odebug "Changing install name in #{file}\n  from #{old}\n    to #{new}"
-    MachO::Tools.change_install_name(file, old, new, strict: false)
+    file.change_install_name(old, new, strict: false)
     apply_ad_hoc_signature(file)
   rescue MachO::MachOError
     onoe <<~EOS
@@ -39,7 +39,7 @@ class Keg
 
     @require_relocation = true
     odebug "Changing rpath in #{file}\n  from #{old}\n    to #{new}"
-    MachO::Tools.change_rpath(file, old, new, strict: false)
+    file.change_rpath(old, new, strict: false)
     apply_ad_hoc_signature(file)
   rescue MachO::MachOError
     onoe <<~EOS
diff --git a/Library/Homebrew/os/mac/mach.rb b/Library/Homebrew/os/mac/mach.rb
index bd636ffba..abadcf8fd 100644
--- a/Library/Homebrew/os/mac/mach.rb
+++ b/Library/Homebrew/os/mac/mach.rb
@@ -64,6 +64,21 @@ module MachOShim
     macho.write!
   end
 
+  def change_rpath(old, new, **options)
+    macho.change_rpath(old, new, options)
+    macho.write!
+  end
+
+  def change_dylib_id(id, **options)
+    macho.change_dylib_id(id, options)
+    macho.write!
+  end
+
+  def change_install_name(old, new, **options)
+    macho.change_install_name(old, new, options)
+    macho.write!
+  end
+
   def dynamically_linked_libraries(except: :none)
     lcs = macho.dylib_load_commands.reject { |lc| lc.type == except } 

carlocab added a commit to carlocab/brew that referenced this issue Feb 11, 2022
We were rewriting dylib IDs and install names using `MachO::Tools`,
which doesn't update the state of the file in memory. This leads to
those changes being undone when we call `change_rpath`.

We fix this by making sure the state of the file in memory always
matches the state of file on disk.

Closes Homebrew#12832.
@carlocab
Copy link
Member

This was surprisingly subtle. (Though maybe I should know better than to be surprised.)

Thanks for the help figuring this out, @gromgit, and thanks for helping track down the offending commit, @imresteiner.

Should be closed by #12864.

carlocab added a commit to carlocab/brew that referenced this issue Feb 11, 2022
We were rewriting dylib IDs and install names using `MachO::Tools`,
which doesn't update the state of the file in memory. This leads to
those changes being undone when we call `delete_rpath`.

We fix this by making sure the state of the file in memory always
matches the state of file on disk.

Closes Homebrew#12832.
@github-actions github-actions bot added the outdated PR was locked due to age label Mar 14, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Reproducible Homebrew/brew bug outdated PR was locked due to age
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants