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

glib, glib-utils: merge #108307

Closed
wants to merge 9 commits into from
Closed

glib, glib-utils: merge #108307

wants to merge 9 commits into from

Conversation

carlocab
Copy link
Member

  • Have you followed the guidelines for contributing?
  • Have you ensured that your commits follow the commit style guide?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing brew install --build-from-source <formula>)? If this is a new formula, does it pass brew audit --new <formula>?

glib-utils currently leans heavily on the implementation details of Meson. We have no guarantees of stability of that API, so this can break with any Meson update, and we wouldn't even know that it happened until the next time glib-utils is rebuilt.

Let's get rid of it.

We can decouple glib from a specific versioned Python dependency by using uses_from_macos "python" instead. This does rely on our syntax check ignoring dependency conflicts of this type, but that's at least something we have control over (unlike Meson's internal API).

While we're here, let's switch this to using libffi from macOS.

The tap syntax check may fail from a number of formulae currently depending on glib-utils. To avoid having to rebuild them, I may remove those dependencies in a separate syntax-only PR.

Closes #107305.

@BrewTestBot BrewTestBot added the automerge-skip `brew pr-automerge` will skip this pull request label Aug 17, 2022
Formula/glib.rb Outdated
depends_on "pcre"

uses_from_macos "libffi", since: :catalina
uses_from_macos "python"
Copy link
Member

@cho-m cho-m Aug 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does mean utilities won't work out-of-the-box on Mojave and older since they need Python 3, which could cascade into breaking dependent builds.

This is reason we originally didn't choose this approach as uses_from_macos "python", since: :catalina can trigger audit failure

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does mean utilities won't work out-of-the-box on Mojave and older since they need Python 3, which could cascade into breaking dependent builds.

Mojave support is "best-effort" --
I don't think supporting Mojave can justify the hacks in glib-utils.

This is reason we originally didn't choose this approach as uses_from_macos "python", since: :catalina can trigger audit failure

What audit failure was that?

Copy link
Member

@cho-m cho-m Aug 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ones seen when mixing node (which uses alias for Python3) and emscripten (#95866)

Similarly seen with node + jupyterlab (#103972, #104097)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should adjust the audit then.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I had to guess, HOMEBREW_SIMULATE_MACOS_ON_LINUX is taking code paths of older version of macOS.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about making this the oldest supported release of macOS?

Off the top of my head, I think that would be fine. Looks like there wasn't really much discussion about it when it was added, so probably fine.

CC: @Bo98

Copy link
Member

@Bo98 Bo98 Aug 18, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The philosophy behind choosing the oldest macOS is that it includes the most dependencies (99.9% of the time). For auditing reasons this is usually a good thing.

We would have the same issue on Linux if the conflict audit wasn't disabled there. I would not rely on this remaining the case - I've been wanting to change that for a long time now.

The conflict audit failure here is really a special case. I'd say the way forward is to special case the python alias to not trigger the conflict audit, since those formulae are typically not bound to a particular version of Python. That's the real issue I feel.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've merged Homebrew/brew#13714. Now we only need #108343.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
uses_from_macos "python"
uses_from_macos "python", since: :catalina

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. This needs another run for the self-hosted runner and to rebuild some dependents against system libffi anyway.

carlocab added a commit to carlocab/brew that referenced this pull request Aug 18, 2022
We have an audit that checks each formula's dependency tree for multiple
versions of the same software. We have an allowlist that allows us to
ignore this audit, but this allowlist requires each formula with a
conflict in its dependency tree to be listed there.

Here, I propose the reverse: if formula `foo` appears in the
`versioned_formula_dependent_conflicts_allowlist`, then all its
dependents will not fail the versioned dependencies conflict because of
a conflict with formula `foo`.

I'd like to do this in the case of `python`, where I think the versioned
dependencies conflict check hurts us more than helps us. Versioned
dependency conflicts are most problematic in the case of libraries with
the same install name but incompatible ABIs. This is almost never a
problem with Python: almost no formulae link with the Python framework
on macOS (in part due to one of our audits that disallows Python
framework linkage in Python modules). Moreover, the various Python
frameworks that we ship have the version in the install name.

The above _might_ be a problem on Linux, since we allow unrestricted
linkage with `libpython`. However, we don't even check versioned
conflicts on Linux, so we aren't as concerned about this in the first
place.

This is also a lot more convenient than adding the dependents of some
Python formula one by one as they acquire conflicts due to changes in
other formulae.

I've also amended `tap_auditor` to allow the use of formula aliases in
an allowlist, to allow us to add `python` to this allowlist instead of
each individual versioned Python formula.

See also discussion at Homebrew/homebrew-core#108307.
BrewTestBot pushed a commit that referenced this pull request Aug 24, 2022
BrewTestBot pushed a commit that referenced this pull request Aug 24, 2022
BrewTestBot pushed a commit that referenced this pull request Aug 24, 2022
BrewTestBot pushed a commit that referenced this pull request Aug 24, 2022
BrewTestBot pushed a commit that referenced this pull request Aug 24, 2022
BrewTestBot pushed a commit that referenced this pull request Aug 24, 2022
BrewTestBot pushed a commit that referenced this pull request Aug 24, 2022
BrewTestBot pushed a commit that referenced this pull request Aug 24, 2022
BrewTestBot pushed a commit that referenced this pull request Aug 24, 2022
BrewTestBot pushed a commit that referenced this pull request Aug 24, 2022
BrewTestBot pushed a commit that referenced this pull request Aug 24, 2022
BrewTestBot pushed a commit that referenced this pull request Aug 24, 2022
BrewTestBot pushed a commit that referenced this pull request Aug 24, 2022
BrewTestBot pushed a commit that referenced this pull request Aug 24, 2022
BrewTestBot pushed a commit that referenced this pull request Aug 24, 2022
Needed after #108307.

Closes #108497.

Signed-off-by: Sean Molenaar <1484494+SMillerDev@users.noreply.github.com>
Signed-off-by: BrewTestBot <1589480+BrewTestBot@users.noreply.github.com>
carlocab added a commit to carlocab/brew that referenced this pull request Aug 24, 2022
The necessary PRs in Homebrew/homebrew-core have been merged. See:
- Homebrew/homebrew-core#108307
- Homebrew/homebrew-core#108497

This reverts commit 6ca02b2.
carlocab added a commit to carlocab/homebrew-core that referenced this pull request Aug 24, 2022
@carlocab carlocab deleted the glib-hacks branch August 24, 2022 02:54
@carlocab
Copy link
Member Author

pygobject3 rebottled on Linux at 54ac8d1.

trezor-agent rev bumped at #108756.

That should settle everything. If there's any remaining breakage, feel free to ping me.

BrewTestBot pushed a commit that referenced this pull request Aug 24, 2022
Needed after #108307.

Closes #108756.

Signed-off-by: BrewTestBot <1589480+BrewTestBot@users.noreply.github.com>
kleisauke added a commit to kleisauke/libvips that referenced this pull request Sep 3, 2022
The GLib utilities, which was split into its own `glib-utils`
formula has been merged back into `glib`, see:
Homebrew/homebrew-core#108307

This reverts commit a846de0.
jcupitt pushed a commit to libvips/libvips that referenced this pull request Sep 3, 2022
The GLib utilities, which was split into its own `glib-utils`
formula has been merged back into `glib`, see:
Homebrew/homebrew-core#108307

This reverts commit a846de0.
carlocab added a commit to carlocab/homebrew-core that referenced this pull request Sep 18, 2022
danielnachun pushed a commit that referenced this pull request Sep 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge-skip `brew pr-automerge` will skip this pull request CI-linux-self-hosted Build on Linux self-hosted runner CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. long build Set a long timeout for formula testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

glib package's pkgconfig references binary not included
5 participants