Skip to content

Fixes installation instruction: sudo make install#360

Merged
Luap99 merged 1 commit intocontainers:mainfrom
flowejam:flowejam-patch-2
Jan 23, 2025
Merged

Fixes installation instruction: sudo make install#360
Luap99 merged 1 commit intocontainers:mainfrom
flowejam:flowejam-patch-2

Conversation

@flowejam
Copy link

@flowejam flowejam commented Jan 9, 2025

Fixes a small issue which might cause a bit of friction for newcomers: containers/podman#24913

@rhatdan
Copy link
Member

rhatdan commented Jan 22, 2025

Thanks @flowejam
Please sign your PR
@Luap99 PTAL

Comment on lines +504 to +505
make BUILDTAGS="selinux seccomp" PREFIX=/usr
sudo make install PREFIX=/usr
sudo env PATH=$PATH:/usr/local/go/bin make install PREFIX=/usr
Copy link
Member

Choose a reason for hiding this comment

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

I am bit confused, where go is depends how you installed it so this is just assuming how you in particular did install it. It must be in $PATH for both the make commands so showing it only on the sudo make install seems confusing.

Would it not be better to just write above that go must be in $PATH for all make commands?

Copy link
Contributor

@hdub-tech hdub-tech Jan 22, 2025

Choose a reason for hiding this comment

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

(Not the OP, but someone who also ran into this issue and have been following along.) The installation instructions do call out that Go needs to be on the PATH. However, when using sudo, secure_path is utilized (as OP mentions in containers/podman#24913) , and not the user's (or even root user's) PATH. I agree with OP that it is worth mentioning at least one possible solution, particularly since it is possible (likely?) the user needed to install Go from source in order to have a Podman compatible version.

That said, I think this could be simplified so there is no assumption about how or where Go is installed, while also avoiding the error that multiple people are running into.

sudo env PATH=$PATH make install PREFIX=/usr

Instructions on the page already call out export PATH=$GOPATH/bin:$PATH, so this covers the install Go from source use case. And if a compatible version of Go was installed from a package manager, it will already be on the path. Demo from a "Go installed from source" environment:

$ which go
/usr/local/go/bin/go

$ sudo which go

$ sudo env PATH=$PATH which go
/usr/local/go/bin/go

Copy link
Author

@flowejam flowejam Jan 23, 2025

Choose a reason for hiding this comment

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

@hdub-tech described the issue perfectly. I've incorporated your suggested changes @hdub-tech - thanks!

Since the environment isn't preserved with `sudo make install`, `GO ?= go` in
the podman Makefile (commit: d3cd5098f0abb54279ce4abe594d309e2bc027b0)
will not have a value, causing the command to fail.

Fixes: containers/podman#24913
Signed-off-by: James Flowers <bold.zone2373@fastmail.com>
Helped-by: H Dub <14808878+hdub-tech@users.noreply.github.com>
@flowejam flowejam requested a review from Luap99 January 23, 2025 05:14
Copy link
Member

@Luap99 Luap99 left a comment

Choose a reason for hiding this comment

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

LGTM

@Luap99 Luap99 merged commit c66ef43 into containers:main Jan 23, 2025
3 checks passed
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.

4 participants