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

macOS .pkg installer fails to detect Xcode if it's on another volume #15802

Closed
3 tasks done
misterfifths opened this issue Aug 1, 2023 · 13 comments · Fixed by #15810
Closed
3 tasks done

macOS .pkg installer fails to detect Xcode if it's on another volume #15802

misterfifths opened this issue Aug 1, 2023 · 13 comments · Fixed by #15810
Labels
bug Reproducible Homebrew/brew bug outdated PR was locked due to age

Comments

@misterfifths
Copy link

misterfifths commented Aug 1, 2023

brew doctor output

N/A, this issue is with the new .pkg installer.

Verification

  • My "brew doctor output" above says Your system is ready to brew. and am still able to reproduce my issue.
  • I ran brew update twice and am still able to reproduce my issue.
  • This issue's title and/or description do not reference a single formula e.g. brew install wget. If they do, open an issue at https://github.com/Homebrew/homebrew-core/issues/new/choose instead.

brew config output

N/A

What were you trying to do (and why)?

Install Homebrew on macOS using the .pkg installer. This seems to reproduce on the latest versions of Monterey and Ventura, as well as beta 4 of Sonoma.

What happened (include all command output)?

The fileExistsAtPath call in installation-check script in Distribution.xml fails if the path is on another volume. In my case, /Applications/Xcode.app was a symlink to another volume (and I didn't have the CLTs installed), so the installer was convinced that it couldn't proceed.

This output is generated in /private/var/log/install.log:

Installer[63009]: IFJS: Package Authoring Error: access to path "/Applications/Xcode.app/Contents/Developer/usr/bin/git" requires <options allow-external-scripts='true'>

Adding that option to the Distribution.xml file does indeed fix the issue, but it results in this ominous warning from productbuild:

productbuild: [WARNING]: Specifying [allow-external-scripts='true'] is deprecated.
           Having this in your distribution will break the installability
           of this product in an upcoming release of macOS.
           Apple reserves the right to reject notarization of any product with this option.

I believe that this is an Apple bug. allow-external-scripts as documented has nothing to do with checking file existence, only running arbitrary executables through the JS interface. I filed FB12823044 with Apple.

In the meantime, perhaps adding allow-external-scripts is an option? I'm not sure if Apple is actually rejecting such packages at notarization time. On the other hand, maybe this is too niche of a circumstance to worry about.

What did you expect to happen?

The installer should detect an Xcode or CLT install even if it's a symlink to another volume.

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

  1. Make sure you do not have the CLTs installed, or the installation-check script will detect them first and not encounter this error.
  2. Symlink /Applications/Xcode.app to an app on another volume.
  3. Open the .pkg installer and immediately see a dialog for the fatal error (and the "Package Authoring Error" in install.log).

(Alternatively you could symlink /Library/Developer/CommandLineTools to another volume instead of Xcode.app. Or just the git file that the script is checking. Any path that winds up on another volume seems to trigger this error.)

@misterfifths misterfifths added the bug Reproducible Homebrew/brew bug label Aug 1, 2023
@Bo98
Copy link
Member

Bo98 commented Aug 1, 2023

Yeah I think the error message is a bit confusing as general I/O failures will trigger that.

Ideally this would would fallback to mdfind "kMDItemCFBundleIdentifier == com.apple.dt.Xcode" | grep -v "/Backups.backupdb/" like brew itself does, but I don't know if that works any better for you or not.

@misterfifths
Copy link
Author

Yeah I think the error message is a bit confusing as general I/O failures will trigger that.

Hm, interesting. If it was an IO error, then why would allow-external-scripts fix it?

Ideally this would would fallback to mdfind "kMDItemCFBundleIdentifier == com.apple.dt.Xcode" | grep -v "/Backups.backupdb/" like brew itself does, but I don't know if that works any better for you or not.

Sure, or even xcode-select -p. But running either of those from Distribution.xml would actually require allow-external-scripts.

@MikeMcQuaid
Copy link
Member

In the meantime, perhaps adding allow-external-scripts is an option? I'm not sure if Apple is actually rejecting such packages at notarization time. On the other hand, maybe this is too niche of a circumstance to worry about.

Yeh, I'd really like to avoid adding use of a deprecated method like this for this reason. Given the installer is a new thing: I'd rather the scope is limited and we avoid using deprecated methods.


I'm leaning towards this being a WONTFIX or "the fix is updating the documentation" I'm afraid because of the workarounds of:

  • you could install the CLT
  • you could not symlink Xcode
  • you could use the non-`.pkg installer

@misterfifths
Copy link
Author

I'm leaning towards this being a WONTFIX or "the fix is updating the documentation"

Sure, that's totally reasonable. Maybe Apple will get around to fixing things.

  • you could install the CLT
  • you could not symlink Xcode
  • you could use the non-`.pkg installer

FWIW, my use case here was installing Homebrew in a VM with Xcode shared to it from the host. The .sh installer works fine, but I was trying to avoid installing the CLT to save disk space, and the .pkg doesn't require it.

And this is probably a question for another time, but what's the current situation re: the CLT? I know that there was a switch at some point to basically require it always, even if Xcode is installed (e.g. #11250). But if the .pkg doesn't require it, perhaps that could be revisited?

@Bo98
Copy link
Member

Bo98 commented Aug 2, 2023

On Intel: no development tools are required at all technically, assuming you don't build from source or use --HEAD. On arm64, it is required due to the way we currently do codesigning.

If you do build from source any formula from source (including any third-party taps that don't offer bottles), CLT is actually mandatory if you aren't using the latest major macOS version, and this is enforced pretty much right away when a new major macOS is released since it ships alongside a new Xcode that removes the previous SDK unlike the CLT. The pkg installer should probably at least check for this case if it's going to enforce dev tools.

Certain packages like Python, GCC and LLVM also require CLT even when installed from a bottle because they bake in a SDK path, and we always choose the CLT for that because Xcode is movable and doesn't always contain the SDKs we need. There's however an argument that we could be less restrictive about this for Python given there's many use cases for that that don't use dev tools.

Certain formulae like Swift formulae may require full Xcode when building from source, but if you stay up-to-date with macOS and use the default install prefix then these cases shouldn't be an issue since you'll use the bottle.

@Bo98
Copy link
Member

Bo98 commented Aug 2, 2023

For comparison's sake, the install.sh installer requires the CLT and does not check for Xcode at all.

@misterfifths
Copy link
Author

Certain packages like Python, GCC and LLVM also require CLT even when installed from a bottle because they bake in a SDK path, and we always choose the CLT for that because Xcode is movable and doesn't always contain the SDKs we need.

Just for my edification, where do paths like that end up getting baked in? I've (probably accidentally - I assume I blew it away at some point for some reason and never thought about it again) been running Homebrew without the CLT for ages on this machine with no issues with bottled Python or LLVM. I'm not seeing anything in otool -L for the binaries in Cellar/python or the site-packages.

@Bo98
Copy link
Member

Bo98 commented Aug 2, 2023

For GCC and LLVM it's the default sysroot. So if the CLT isn't present then it won't find any headers without a manual -isysroot.

For Python, it'll be in some config file. Can't remember where, but it's used when building native Python modules. Should be able to be seen from a recursive grep. Won't affect all operations however.

been running Homebrew without the CLT for ages on this machine with no issues with bottled Python or LLVM

Well given the https://github.com/Homebrew/homebrew-core/blob/149182413c1d22cfe92526bbc55067f81d51ba92/Formula/python@3.11.rb#L27 and similar for LLVM, it couldn't have been using the bottles unless you did --force-bottle? Building from source without CLT will bake in the Xcode install instead.

@misterfifths
Copy link
Author

Ah-ha, that explains it. Thanks for the thorough explanation! Sorry if I lead this bug astray. I'll go with the .sh installer, though hopefully this discussion was useful for thinking about bringing the .pkg up to parity.

@MikeMcQuaid
Copy link
Member

Thanks for chipping in @Bo98 and for understanding @misterfifths!

The pkg installer should probably at least check for this case if it's going to enforce dev tools.

@Bo98 if you can figure out how with the (very) limited APIs available: I'm definitely game.

For comparison's sake, the install.sh installer requires the CLT and does not check for Xcode at all.

Yeh, the temptation is to do the same here if we get other complaints.

On arm64, it is required due to the way we currently do codesigning.

@woodruffw did you have some ideas about being able to get rid of this one day?

@woodruffw
Copy link
Member

@woodruffw did you have some ideas about being able to get rid of this one day?

Yeah -- I began to look into this a (long) time ago with Homebrew/ruby-macho#265; I can try and revive that work, although I'm not sure yet if it'll be feasible to have a pure Ruby local codesigning implementation yet 🙂

@Bo98
Copy link
Member

Bo98 commented Aug 2, 2023

if you can figure out how with the (very) limited APIs available: I'm definitely game.

if (system.compareVersions(system.version.ProductVersion, '13') < 0)
  // needs CLT

or something like that

@MikeMcQuaid
Copy link
Member

MikeMcQuaid commented Aug 3, 2023

Yeah -- I began to look into this a (long) time ago with Homebrew/ruby-macho#265; I can try and revive that work, although I'm not sure yet if it'll be feasible to have a pure Ruby local codesigning implementation yet 🙂

@woodruffw Looks good/interesting! I also think this would be a case where if Swift provides access to any macOS APIs for this: it could be a good option. I think it would be wonderful to have a pure-Ruby (or at least: non-CLT or Xcode) implementation if possible.

if (system.compareVersions(system.version.ProductVersion, '13') < 0)
  // needs CLT

or something like that

@Bo98 will give this a go 👍🏻

@github-actions github-actions bot added the outdated PR was locked due to age label Sep 3, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 3, 2023
dduugg pushed a commit to dduugg/brew that referenced this issue Sep 30, 2023
We always require it in `install.sh` and we need it for all but the
latest version of macOS so this seems a simpler compromise.

Fixes Homebrew#15802
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