podman: Change hardcoded libexec path #90379

afbjorklund · 2021-12-03T17:44:10Z

Podman had hardcoded paths for brew, which means it doesn't work for user installations.

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>?

Closes:

podman machine start cannot find gvproxy when brew is not installed in the standard directory containers/podman#12161

afbjorklund · 2021-12-03T17:47:12Z

This was the error, on Linux: (/home/linuxbrew)

Error: unable to start host networking: "could not find \"gvproxy\" in one of [/usr/local/libexec/podman /usr/local/lib/podman /usr/libexec/podman /usr/lib/podman]"

carlocab

Thanks for your PR, @afbjorklund.

Please use the preferred commit-message style for homebrew/core. We put the name of the formula first in commit-message headings. Here's a quick summary from the style guide:

For new formulae:

At Homebrew, we like to put the name of the formula up front like so: foobar 7.3 (new formula).

For existing formulae:

The preferred commit message format for simple version updates is foobar 7.3 and for fixes is foobar: fix flibble matrix..

Refer to the commit style guide for more details. Also, when making further changes to your pull request, use the following guidelines to make sure that @BrewTestBot can merge your commits:

One formula per commit; one commit per formula.

Keep merge commits out of the pull request.

carlocab · 2021-12-03T17:58:29Z

Formula/podman.rb

+  # Remove hardcoded paths
+  patch :DATA


Has this patch been upstreamed?

Nope, upstream says it should be fixed by downstream...

i.e. they hardcoded some common brew paths

The patch isn't strictly required, we can loop through all the unused paths first if preferred

Also the change is in a dependency (separate project), which is statically linked

https://github.com/containers/common/blob/main/pkg/config/config_darwin.go

Would the inreplace suffice without the patch? We don't carry patches that won't get accepted upstream. (With a handful of exceptions for patches that were made before this was made a policy, but even those could face the chopping block at some point.)

It also seems a bit weird to me for upstream to add Homebrew-specific paths that we would then remove here. Does a similar issue not affect other, non-Homebrew, users, who don't install into /usr or /usr/local?

See containers/podman#12161 (comment)

I did see that, but didn't we just establish above that this issue is not Homebrew-specific? Or was the conclusion above that this affects non-Homebrew users who install into non-standard locations incorrect?

The right thing would be for "catatonit" and "gvproxy" to be dependencies, not bundled.

See #87116 (comment). Though I'm not sure how having gvproxy be a dependency fixes the issue about hard-coded paths.

I don't think they support Homebrew outside of standard prefix, or running on Linux (linuxbrew)
So all other Linux distributions have to patch the search paths in a similar way, or add symlinks.

Presumably if gvproxy was a system dependency, it would be able to find it in some path...
Currently it is a separate package on Ubuntu - with a hardcoded path leading to Podman. :-(

Actually it seems the standalone gvproxy path was already tried (and rejected), so "libexec" it is.

podman-gvproxy 0.1.0 (new formula) #83449

I don't think they support Homebrew outside of standard prefix, or running on Linux (linuxbrew)

Those are not separate projects anymore so they really just support half of Homebrew.

It would be "fun" to see FreeBSDBrew, just like there was some stuff done for DarwinPorts

Basically we tried to run the basic system without the Apple bits, to keep it portable...
And without hardcoding "if apple else gnu". It was surprisingly difficult to do, though.

afbjorklund · 2021-12-03T18:02:53Z

I changed the commit, but the PR headline wasn't updated

carlocab · 2021-12-03T18:20:53Z

Formula/podman.rb

+  # Remove hardcoded paths
+  patch :DATA


Would the inreplace suffice without the patch? We don't carry patches that won't get accepted upstream. (With a handful of exceptions for patches that were made before this was made a policy, but even those could face the chopping block at some point.)

It also seems a bit weird to me for upstream to add Homebrew-specific paths that we would then remove here. Does a similar issue not affect other, non-Homebrew, users, who don't install into /usr or /usr/local?

Formula/podman.rb

The path to helper binaries were hardcoded, so it didn't work for home directory and other relocated installations. Also the "gvproxy" program has been moved from a subdirectory to the top libexec directory in Homebrew, so set the new path. Signed-off-by: Anders F Björklund <[email protected]>

afbjorklund · 2021-12-03T18:51:01Z

Removed all the patches, so now it just changes the hardcoded prefix to the one in the cellar.

anders@ubuntu:/home/linuxbrew$ ./.linuxbrew/bin/podman-remote machine start
INFO[0000] waiting for clients...                       
INFO[0000] listening tcp://0.0.0.0:7777                 
INFO[0000] new connection from @ to /run/user/1000/podman/qemu_podman-machine-default.sock 
Waiting for VM ...
Machine "podman-machine-default" started successfully

SMillerDev · 2021-12-03T20:01:57Z

Is this also submitted as a patch upstream?

afbjorklund · 2021-12-05T15:17:39Z

Is this also submitted as a patch upstream?

Patches were removed, upstream has all the "usual" homebrew search paths hardcoded.

It's not possible to patch the dynamic part upstream, so it has to be edited downstream...

      system "make", "gvproxy"
      libexec.install "bin/gvproxy"

    inreplace "vendor/github.com/containers/common/pkg/config/config_linux.go", "/usr/local/libexec/podman", libexec
    inreplace "vendor/github.com/containers/common/pkg/config/config_darwin.go", "/usr/local/libexec/podman", libexec

Upstream says that it is homebrew's problem, that it allows users to relocate the prefix:

var defaultHelperBinariesDir = []string{
	// Homebrew install paths
	"/usr/local/opt/podman/libexec",
	"/opt/homebrew/bin",
	"/opt/homebrew/opt/podman/libexec",
	"/usr/local/bin",
	// default paths
	"/usr/local/libexec/podman",
	"/usr/local/lib/podman",
	"/usr/libexec/podman",
	"/usr/lib/podman",
}

It doesn't support linuxbrew, user will have to add some configuration before it starts.

SMillerDev · 2021-12-05T15:32:10Z

Upstream says that it is homebrew's problem, that it allows users to relocate the prefix

What about people who want to install their software outside of homebrew? This seems like a really shortsighted idea to me.

afbjorklund · 2021-12-05T16:03:03Z

Upstream says that it is homebrew's problem, that it allows users to relocate the prefix

What about people who want to install their software outside of homebrew? This seems like a really shortsighted idea to me.

Currently you have to either patch it, or supply default configuration...

Basically it supports both prefixes, /usr and /usr/local. (Like Bob)

Any other location of the helper binaries, is left to the packager's discretion:

podman 3.4.0 breaks support for MacPorts' gvproxy location containers/podman#11960

https://github.com/macports/macports-ports/blob/master/sysutils/podman/files/patch-defaultHelperBinariesDir-for-MacPorts.diff

afbjorklund · 2021-12-06T17:02:31Z

Preferrably, "gvproxy" should be relocated to the default path (i.e. under the prefix)

--- a/Formula/podman.rb
+++ b/Formula/podman.rb
@@ -38,7 +38,7 @@ class Podman < Formula
     ENV["CGO_ENABLED"] = "1"
     os = OS.kernel_name.downcase
 
-    inreplace "vendor/github.com/containers/common/pkg/config/config_#{os}.go", "/usr/local/libexec/podman", libexec
+    inreplace "vendor/github.com/containers/common/pkg/config/config_#{os}.go", "/usr/local/libexec/podman", libexec/"podman"
 
     system "make", "podman-remote-#{os}"
     if OS.mac?
@@ -50,7 +50,7 @@ class Podman < Formula
 
     resource("gvproxy").stage do
       system "make", "gvproxy"
-      libexec.install "bin/gvproxy"
+      (libexec/"podman").install "bin/gvproxy"
     end
 
     if build.head?

That would make it more similar to other platforms and to the default /usr config.

Also makes the top directory less cluttered, when the other helpers are added too ?

/usr/libexec/podman/catatonit
/usr/libexec/podman/conmon
/usr/libexec/podman/gvproxy

github-actions · 2021-12-28T00:12:23Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

Applies the patch suggested in Homebrew#90379 (comment). Refs: Homebrew#90379

Applies the patch suggested in #90379 (comment). Refs: #90379 Closes #93794. Signed-off-by: Carlo Cabrera <[email protected]> Signed-off-by: BrewTestBot <[email protected]>

BrewTestBot added the go Go use is a significant feature of the PR or issue label Dec 3, 2021

afbjorklund mentioned this pull request Dec 3, 2021

podman machine start cannot find gvproxy when brew is not installed in the standard directory containers/podman#12161

Closed

carlocab changed the title ~~Remove hardcoded paths from podman installation~~ podman: remove hardcoded paths Dec 3, 2021

carlocab reviewed Dec 3, 2021

View reviewed changes

afbjorklund force-pushed the podman-paths branch from 4e12dfb to a9d5b7a Compare December 3, 2021 18:39

afbjorklund changed the title ~~podman: remove hardcoded paths~~ podman: Change hardcoded libexec path Dec 3, 2021

afbjorklund mentioned this pull request Dec 6, 2021

podman: add Linux support #84276

Closed

github-actions bot added the stale No recent activity label Dec 28, 2021

github-actions bot closed this Jan 4, 2022

gibfahn mentioned this pull request Jan 25, 2022

podman: make formula relocatable #93794

Closed

6 tasks

carlocab pushed a commit to gibfahn/homebrew-core that referenced this pull request Jan 27, 2022

podman: fix installs in non-/usr/local or /opt/homebrew prefixes

1496e25

Applies the patch suggested in Homebrew#90379 (comment). Refs: Homebrew#90379

github-actions bot added the outdated PR was locked due to age label Feb 4, 2022

github-actions bot locked as resolved and limited conversation to collaborators Feb 4, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

podman: Change hardcoded libexec path #90379

podman: Change hardcoded libexec path #90379

afbjorklund commented Dec 3, 2021 •

edited

Loading

afbjorklund commented Dec 3, 2021

carlocab left a comment

carlocab Dec 3, 2021

afbjorklund Dec 3, 2021

afbjorklund Dec 3, 2021 •

edited

Loading

afbjorklund Dec 3, 2021

carlocab Dec 3, 2021

carlocab Dec 3, 2021

afbjorklund Dec 5, 2021

afbjorklund Dec 6, 2021

SMillerDev Dec 6, 2021

afbjorklund Dec 6, 2021

afbjorklund commented Dec 3, 2021

carlocab Dec 3, 2021

afbjorklund commented Dec 3, 2021 •

edited

Loading

SMillerDev commented Dec 3, 2021

afbjorklund commented Dec 5, 2021 •

edited

Loading

SMillerDev commented Dec 5, 2021

afbjorklund commented Dec 5, 2021 •

edited

Loading

afbjorklund commented Dec 6, 2021 •

edited

Loading

github-actions bot commented Dec 28, 2021

podman: Change hardcoded libexec path #90379

podman: Change hardcoded libexec path #90379

Conversation

afbjorklund commented Dec 3, 2021 • edited Loading

afbjorklund commented Dec 3, 2021

carlocab left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

afbjorklund Dec 3, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

afbjorklund commented Dec 3, 2021

Choose a reason for hiding this comment

afbjorklund commented Dec 3, 2021 • edited Loading

SMillerDev commented Dec 3, 2021

afbjorklund commented Dec 5, 2021 • edited Loading

SMillerDev commented Dec 5, 2021

afbjorklund commented Dec 5, 2021 • edited Loading

afbjorklund commented Dec 6, 2021 • edited Loading

github-actions bot commented Dec 28, 2021

afbjorklund commented Dec 3, 2021 •

edited

Loading

afbjorklund Dec 3, 2021 •

edited

Loading

afbjorklund commented Dec 3, 2021 •

edited

Loading

afbjorklund commented Dec 5, 2021 •

edited

Loading

afbjorklund commented Dec 5, 2021 •

edited

Loading

afbjorklund commented Dec 6, 2021 •

edited

Loading