Skip to content

keg_relocate: check for uses_from_macos when replacing Perl prefix #20043

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

botantony
Copy link
Contributor

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew style with your changes locally?
  • Have you successfully run brew typecheck with your changes locally?
  • Have you successfully run brew tests with your changes locally?

Closes #20023

@Bo98
Copy link
Member

Bo98 commented Jun 3, 2025

Am sort of OK with this if others are but cautiously as it's a bit weird to have the uses_from_macos marker only on Linux because Linux happens to be the one picked. Linux itself shouldn't have to care what macOS system dependencies are.

Do we even know that deterministically Linux is always picked with all bottles? It might just be alphabetical and break if a name like Yosemite returns.

@botantony
Copy link
Contributor Author

I am not sure what this is. Did you mean that Linux runners do not check bounds (and keg will use the system's prefix even if since: :<higher_macos_version> is set)?

@Bo98
Copy link
Member

Bo98 commented Jun 3, 2025

I wasn't talking about since: but good point as this will be incorrect for that.

all bottles are produced by effectively picking one of the OS bottles and tabs and using that. Which one is used is not defined but probably happens to be x86_64_linux. runtime_deps_hash is also only called with actual runtime dependencies - so any uses_from_macos is omitted on platforms that use the system:

self.class.new(*__getobj__.reject { |dep| dep.uses_from_macos? && dep.use_macos_install? })

Here's what the tab would output with this:

depends_on "perl"

All JSON files:

{
  ...
  "runtime_dependencies": [
    {
      "full_name": "perl",
      ...
      "declared_directly": true,
      "uses_from_macos": false,
    }
  ],
  ...
}

uses_from_macos "perl"

sequoia.json and sonoma.json:

{
  ...
  "runtime_dependencies": [],
  ...
}

x86_64_linux.json

{
  ...
  "runtime_dependencies": [
    {
      "full_name": "perl",
      ...
      "declared_directly": true,
      "uses_from_macos": true,
    }
  ],
  ...
}

all.json: seems to happen to use x86_64_linux.json


uses_from_macos "perl", since: :sequoia

sequoia.json:

{
  ...
  "runtime_dependencies": [],
  ...
}

sonoma.json and x86_64_linux.json

{
  ...
  "runtime_dependencies": [
    {
      "full_name": "perl",
      ...
      "declared_directly": true,
      "uses_from_macos": true,
    }
  ],
  ...
}

all.json: seems to happen to use x86_64_linux.json


The last case definitely being incorrect for <= Sonoma if we are checking for uses_from_macos.

In the end it's a fundamental problem with using runtime_dependencies in all bottles. The exact same could happen with on_macos or on_linux blocks. We do have code that works around it but it applies too late for this particular case:

tab.runtime_dependencies = Tab.runtime_deps_hash(formula, f_runtime_deps)

@MikeMcQuaid
Copy link
Member

@botantony please hold off opening any more PRs until the existing ones are merged, thanks

@MikeMcQuaid
Copy link
Member

Am sort of OK with this if others are but cautiously as it's a bit weird to have the uses_from_macos marker only on Linux because Linux happens to be the one picked. Linux itself shouldn't have to care what macOS system dependencies are.

Agreed. It's also unclear to me how/whether this is perl specific.

In the end it's a fundamental problem with using runtime_dependencies in all bottles.

@Bo98 maybe we should just throw it away at either bottle and/or installation time and regenerate it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Keg relocation doesn't account for uses_from_macos when replacing #!@@HOMEBREW_<language>@@ shebangs
3 participants