Install bats as root

[xordspar0]

Installing bats to /usr/local requires root privileges. Without this, make install.tools fails. However, if I do sudo make install.tools, then all of the other dependencies and git clones in the current directory end up owned by root. This limits root privileges to the part that needs it.

I'm not sure this is a very good solution. I wanted to open discussion about this issue because currently it's difficult to get a working test environment without digging into the Makefile and install scripts to see what's going on. Otherwise make install.tools fails like this:

HEAD is now at c706d14 Bats 1.1.0
install: cannot create directory ‘/usr/local/libexec’: Permission denied
install: cannot change permissions of ‘/usr/local/share/man/man1’: No such file or directory
install: cannot change permissions of ‘/usr/local/share/man/man7’: No such file or directory
make: *** [Makefile:640: .install.bats] Error 1

Another possible solution is installing bats to _output like the Go dependencies and invoking bats as $GOBIN/bats. Any other ideas?

[TomSweeneyRedHat]

@edsantiago thoughts?

[mheon]

I would actually be fine requiring make install.tools to be run with sudo - though maybe a warning that it requires that could be added?

[xordspar0]

make localunit has a check like that already:

podman/Makefile

Lines 308 to 311 in dc23ef1

	localunit: test/goecho/goecho varlink_generate
	hack/check_root.sh make localunit
	rm -rf ${COVERAGE_PATH} && mkdir -p ${COVERAGE_PATH}
	$(GOBIN)/ginkgo \

However, it would be hard to add the same thing to install.tools because it's a prerequisite that needs root privileges. If I added a check in the same place as localunit, it would run too late:

podman/Makefile

Lines 615 to 616 in dc23ef1

	install.tools: .install.gitvalidation .install.md2man .install.ginkgo .install.golangci-lint .install.bats ## Install needed tools

What about putting the check on .install.bats, like this?

 .PHONY: .install.bats
 .install.bats: .gopathok
+       hack/check_root.sh make .install.bats
        VERSION=v1.1.0 ./hack/install_bats.sh

Or we could just document that make install.tools requires root in test/README.md?

[xordspar0]

Oh, or we could add a new prerequisite that checks for root:

install.tools: .root-privileges .install.gitvalidation .install.md2man .install.ginkgo .install.golangci-lint .install.bats ## Install needed tools

.root-privileges:
	hack/check_root.sh something

[edsantiago]

I'm pretty strongly opposed to sudo in any Makefile steps, for long-held philosophical reasons.

Can you just make a small tweak to hack/install_bats like

if [[ "$(type -t bats)" != "" ]]; then
    exit 0
fi

[xordspar0]

That's fair.

Can you just make a small tweak to hack/install_bats

So if bats is already installed, skip installation. What about when it isn't installed yet?

[edsantiago]

Can sudo be invoked from within install_bats? Preferably with a warning beforehand explaining what will happen and why this is needed? (eg "sudo needed in order to install BATS into /usr/local") ?

[xordspar0]

Sure. I added the "skip if already installed" check and the sudo warning.

[xordspar0]

I'm starting to like this better because now you know that it's trying to install to /usr/local when it asks for you password, and you can install bats yourself if you want to – the script will accept it as long as it's on your PATH.

[jwhonce]

LGTM

[TomSweeneyRedHat]

LGTM
THX @xordspar0 !

[rhatdan]

/approve
/lgtm
Thanks @xordspar0

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rhatdan, xordspar0

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [rhatdan]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[xordspar0]

🎉