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

pkgconf: drop-in replacement for pkg-config #194885

Merged
merged 3 commits into from
Nov 12, 2024
Merged

Conversation

cho-m
Copy link
Member

@cho-m cho-m commented Oct 18, 2024

  • 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 HOMEBREW_NO_INSTALL_FROM_API=1 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 HOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source <formula>)? If this is a new formula, does it pass brew audit --new <formula>?

Copy link
Member

@carlocab carlocab left a comment

Choose a reason for hiding this comment

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

I suggest retiring pkg-config already too. i.e. Let's delete it and add a formula rename for pkg-config -> pkgconf.

@cho-m
Copy link
Member Author

cho-m commented Oct 18, 2024

I suggest retiring pkg-config already too. i.e. Let's delete it and add a formula rename for pkg-config -> pkgconf.

That's a bit complicated as audit doesn't like usage of old names / aliases.

Assuming audit is not changed, either need to:

  • Do a giant syntax-only migration of all pkg-config usage to pkgconf.
  • Make pkg-config into a transitional formula that just installs pkgconf and then do the migration at a more leisurely pace. At the end, can squash pkg-config formula to either be a rename/alias of pkgconf.

@carlocab
Copy link
Member

We can just add a temporary exception for pkg-config and do the migration gradually.

@Bo98
Copy link
Member

Bo98 commented Oct 18, 2024

Am OK with this PR given pkgconf and pkg-config are already conflicting.

In terms of switching our uses to pkgconf, the requirements are:

  • pkgconf cannot conflict with pkg-config
  • brew install pkg-config must continue to work (I'd probably go as far as saying it should continue to do so without warnings, i.e. as an alias)

Merging via renames and merging via alias however works completely differently. One uses the migrator mechanism and one doesn't. So will need to carefully test it.

I expect anything we do here to be "special", which may indeed mean adjusting things in brew to allow depends_on "pkg-config" (potentially for a long time/forever as it would affect so many third-party formulae).

@github-actions github-actions bot added the automerge-skip `brew pr-automerge` will skip this pull request label Oct 30, 2024
@cho-m
Copy link
Member Author

cho-m commented Oct 30, 2024

Merging via renames and merging via alias however works completely differently. One uses the migrator mechanism and one doesn't. So will need to carefully test it.

At least pkgconf is using a higher version number so can be seen as an upgrade.

brew outdated pkg-config
pkgconf (0.29.2_3) < 2.3.0_1

Though that also means the old version may remain for users who avoid upgrades.

@cho-m
Copy link
Member Author

cho-m commented Oct 30, 2024

For alias option, probably need Homebrew/brew#18675 in stable tag before continuing.

@cho-m
Copy link
Member Author

cho-m commented Nov 1, 2024

I guess one possible issue is if we update shim as a user that still has old pkg-config may not end up with pkgconf path:

https://github.com/Homebrew/brew/blob/master/Library/Homebrew/shims/mac/super/pkg-config#L5


Though, perhaps we shouldn't be using the explicit path. It does cause pkg-config to be silently added as a dependency as long as it is locally installed.

Maybe using the PATH lookup helper would align closer to our other shims.

@cho-m cho-m force-pushed the pkgconf-symlink branch 3 times, most recently from fab6337 to b14c9b1 Compare November 3, 2024 16:39
@cho-m cho-m added the maintainer feedback Additional maintainers' opinions may be needed label Nov 4, 2024
@cho-m cho-m marked this pull request as ready for review November 4, 2024 14:24
@cho-m
Copy link
Member Author

cho-m commented Nov 4, 2024

Alias support available in 4.4.4. I think brew should just see this as an upgrade and handle like other version bumps.

Only bug I see is brew may not clean up the old Cellar. EDIT: A brew cleanup does handle leftover installation so easy way for users to deal with.

@carlocab
Copy link
Member

carlocab commented Nov 4, 2024

LGTM. CC @Homebrew/core for additional feedback.

@MikeMcQuaid
Copy link
Member

Let's delete it and add a formula rename for pkg-config -> pkgconf.

I don't think we should do this. It's not a formula rename, it's another formula that we're migrating to.

I feel vaguely uneasy but OK about the pkg-config alias but the above is definitely a step too far.

Alias support available in 4.4.4. I think brew should just see this as an upgrade and handle like other version bumps.

@cho-m can you make sure you've tested this locally? Want to ensure that this is robust before this is merged.

@p-linnane p-linnane requested a review from a team November 4, 2024 20:18
Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

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

Fine with me when @cho-m is 👍🏻 and has tested upgrade path locally.

@cho-m
Copy link
Member Author

cho-m commented Nov 8, 2024

At least the basic usage is all fine from tap. Testing without revision on pkgconf to check bottle usage.

  • brew upgrade will install pkgconf and link it, e.g.

    $ brew outdated
    pkgconf (0.29.2_3) < 2.3.0
    
    $ brew upgrade
    ==> Upgrading 1 outdated package:
    pkg-config 2.3.0
    ==> Downloading https://ghcr.io/v2/homebrew/core/pkgconf/manifests/2.3.0
    Already downloaded: /Users/cho-m/Library/Caches/Homebrew/downloads/2f1a3719bf003aa9443a2e2e6af8e6441aa287088a24771231f191a9bcf36884--pkgconf-2.3.0.bottle_manifest.json
    ==> Fetching pkgconf
    ==> Downloading https://ghcr.io/v2/homebrew/core/pkgconf/blobs/sha256:9b44fe313d296fa10617d6b58ea1eab78bf217b6f00478ad98b01e0375475472
    Already downloaded: /Users/cho-m/Library/Caches/Homebrew/downloads/eed8a0a3462be8896edfce1cf6d726e4ebfd489cafecfb485f1b1b669a5d89af--pkgconf--2.3.0.sequoia.bottle.tar.gz
    ==> Upgrading pkg-config
      -> 2.3.0
    ==> Pouring pkgconf--2.3.0.sequoia.bottle.tar.gz
    🍺  /usr/local/Cellar/pkgconf/2.3.0: 25 files, 327.9KB
    ==> Running `brew cleanup pkgconf`...
    Disable this behaviour by setting HOMEBREW_NO_INSTALL_CLEANUP.
    Hide these hints with HOMEBREW_NO_ENV_HINTS (see `man brew`).
    Removing: /usr/local/Cellar/pkg-config/0.29.2_3... (12 files, 631.4KB)
    
    $ brew info pkg-config
    ==> pkgconf: stable 2.3.0 (bottled), HEAD
    Package compiler and linker metadata toolkit
    https://github.com/pkgconf/pkgconf
    Installed
    /usr/local/Cellar/pkgconf/2.3.0 (25 files, 327.9KB) *
      Poured from bottle on 2024-11-08 at 13:13:07
    From: https://github.com/Homebrew/homebrew-core/blob/HEAD/Formula/p/pkgconf.rb
    License: ISC
    ==> Options
    --HEAD
        Install HEAD version
    
    $ jq '.source' /usr/local/opt/pkg-config/INSTALL_RECEIPT.json
    {
      "spec": "stable",
      "versions": {
        "stable": "2.3.0",
        "head": null,
        "version_scheme": 0
      },
      "path": "/usr/local/Homebrew/Library/Taps/homebrew/homebrew-core/Aliases/pkg-config",
      "tap_git_head": "afeea04d898316ef1ea37d87b80de9c9829550d7",
      "tap": "homebrew/core"
    }
  • brew install <formula-with-pkg-config-dep> will automatically upgrade/install pkgconf and record it in receipt:

    $ brew install pkg-config
    ==> Downloading https://ghcr.io/v2/homebrew/core/pkg-config/manifests/0.29.2_3
    Already downloaded: /Users/cho-m/Library/Caches/Homebrew/downloads/ac691fc7ab8ecffba32a837e7197101d271474a3a84cfddcc30c9fd6763ab3c6--pkg-config-0.29.2_3.bottle_manifest.json
    ==> Fetching pkg-config
    ==> Downloading https://ghcr.io/v2/homebrew/core/pkg-config/blobs/sha256:55f4268c3d6f320932ce736e8ad862a98b27048a30481f85c0679d8c655fb0f6
    Already downloaded: /Users/cho-m/Library/Caches/Homebrew/downloads/25f2808beac7c455c7101ea7fb7f9ae61f5019163147d7e822e30a76ceb11263--pkg-config--0.29.2_3.sequoia.bottle.tar.gz
    ==> Pouring pkg-config--0.29.2_3.sequoia.bottle.tar.gz
    🍺  /usr/local/Cellar/pkg-config/0.29.2_3: 12 files, 631.5KB
    ==> Running `brew cleanup pkg-config`...
    Disable this behaviour by setting HOMEBREW_NO_INSTALL_CLEANUP.
    Hide these hints with HOMEBREW_NO_ENV_HINTS (see `man brew`).
    
    $ # update branch and revision
    
    $ brew install libpthread-stubs
    ==> Downloading https://ghcr.io/v2/homebrew/core/libpthread-stubs/manifests/0.5-1
    Already downloaded: /Users/cho-m/Library/Caches/Homebrew/downloads/daf0585704083a365a05b49b2a0078c97907e4a8b527f2a432b59a77805cf3f4--libpthread-stubs-0.5-1.bottle_manifest.json
    ==> Fetching dependencies for libpthread-stubs: pkgconf
    ==> Downloading https://ghcr.io/v2/homebrew/core/pkgconf/manifests/2.3.0
    Already downloaded: /Users/cho-m/Library/Caches/Homebrew/downloads/2f1a3719bf003aa9443a2e2e6af8e6441aa287088a24771231f191a9bcf36884--pkgconf-2.3.0.bottle_manifest.json
    ==> Fetching pkgconf
    ==> Downloading https://ghcr.io/v2/homebrew/core/pkgconf/blobs/sha256:9b44fe313d296fa10617d6b58ea1eab78bf217b6f00478ad98b01e0375475472
    Already downloaded: /Users/cho-m/Library/Caches/Homebrew/downloads/eed8a0a3462be8896edfce1cf6d726e4ebfd489cafecfb485f1b1b669a5d89af--pkgconf--2.3.0.sequoia.bottle.tar.gz
    ==> Fetching libpthread-stubs
    ==> Downloading https://ghcr.io/v2/homebrew/core/libpthread-stubs/blobs/sha256:303b8c21fc1b9322b6bd8f24e75a4e53a1c331d09b4f6271f75eba743d119819
    Already downloaded: /Users/cho-m/Library/Caches/Homebrew/downloads/623815c812e848149470976d73b58273ffb90c3873fe1ed3ec11f6d4a88c877f--libpthread-stubs--0.5.all.bottle.1.tar.gz
    ==> Installing dependencies for libpthread-stubs: pkgconf
    ==> Installing libpthread-stubs dependency: pkgconf
    ==> Downloading https://ghcr.io/v2/homebrew/core/pkgconf/manifests/2.3.0
    Already downloaded: /Users/cho-m/Library/Caches/Homebrew/downloads/2f1a3719bf003aa9443a2e2e6af8e6441aa287088a24771231f191a9bcf36884--pkgconf-2.3.0.bottle_manifest.json
    ==> Pouring pkgconf--2.3.0.sequoia.bottle.tar.gz
    🍺  /usr/local/Cellar/pkgconf/2.3.0: 25 files, 327.9KB
    ==> Installing libpthread-stubs
    ==> Pouring libpthread-stubs--0.5.all.bottle.1.tar.gz
    🍺  /usr/local/Cellar/libpthread-stubs/0.5: 6 files, 10.6KB
    ==> Running `brew cleanup libpthread-stubs`...
    Disable this behaviour by setting HOMEBREW_NO_INSTALL_CLEANUP.
    Hide these hints with HOMEBREW_NO_ENV_HINTS (see `man brew`).
    
    $ jq '.runtime_dependencies' /usr/local/Cellar/libpthread-stubs/0.5/INSTALL_RECEIPT.json
    [
      {
        "full_name": "pkgconf",
        "version": "2.3.0",
        "revision": 0,
        "pkg_version": "2.3.0",
        "declared_directly": false
      }
    ]

@MikeMcQuaid
Copy link
Member

@cho-m sounds good, thanks!

@cho-m
Copy link
Member Author

cho-m commented Nov 12, 2024

Will rebase due to merging my Homebrew/brew PR which will probably cause syntax failure. Then can merge.

@cho-m cho-m removed the automerge-skip `brew pr-automerge` will skip this pull request label Nov 12, 2024
Copy link
Contributor

🤖 An automated task has requested bottles to be published to this PR.

@github-actions github-actions bot added the CI-published-bottle-commits The commits for the built bottles have been pushed to the PR branch. label Nov 12, 2024
@BrewTestBot BrewTestBot added this pull request to the merge queue Nov 12, 2024
Merged via the queue into master with commit 0c35794 Nov 12, 2024
15 checks passed
@BrewTestBot BrewTestBot deleted the pkgconf-symlink branch November 12, 2024 03:46
@deecewan
Copy link

is there any context as to why this was done? this seems to be the PR that deleted pkg-config formula, but it doesn't mention at all why it happened.

@carlocab
Copy link
Member

See discussion at #182175

@Bo98
Copy link
Member

Bo98 commented Dec 10, 2024

The change shouldn't have any notable effect for the vast majority of users (except one incident with GitHub Actions that was caused by GitHub themselves which they have since fixed). We added aliases so brew install pkg-config still works and pkg-config still exists as an executable. Existing pkg-config installs install pkgconf automatically on brew upgrade, retaining full compatibility with any existing hardcoded paths (e.g. /opt/homebrew/opt/pkg-config/bin/pkg-config references).

If you face any regressions let us know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI-published-bottle-commits The commits for the built bottles have been pushed to the PR branch. maintainer feedback Additional maintainers' opinions may be needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants