vapor/toolbox

Make command fails if sudo is not found

clsource opened this issue · 5 comments

Describe the bug

Using swift:5.4-bionic Dockerfile the makefiles fails because Ubuntu 18.04 does not includes sudo by default.

To Reproduce

  • Use cd toolbox && make install inside a container created using the swift:5.4-bionic image

Expected behavior

vapor command line is installed

Environment

  • Vapor Framework version: 4
  • Vapor Toolbox version: 18.3.3
  • OS version: Ubuntu 18.04 inside Docker

Additional context

To solve this I deleted using the sudo command inside the Makefile

DEST := /usr/local/bin/vapor

build:
	swiftc ./scripts/build.swift
	./build
	rm ./build
install: build
	mv .build/release/vapor ${DEST}
	chmod 755 ${DEST}
uninstall:
	rm ${DEST}
clean:
	rm -rf .build

Another alternative is ensuring the sudo command inside the Dockerfile of swift

FROM swift:5.4-bionic

RUN apt update
RUN apt install sudo

WORKDIR /src
0xTim commented

Simply put, the toolbox is not designed to be run in a Docker container and we can't amount for every different environment.

If you really need the toolbox inside a container you should manually install it. However there are very few situations where it should be used, you should use swift build or swift run etc instead

This is till a problem. I am on RHEL 8 and am forced to run a container to run swift, the official swift container images have no sudo in them.

0xTim commented

@Jeffrey-de-Bruijn what are you trying to use the toolbox for?

I am new to Vapor, I am following the Vapor docs. I understand that the toolbox is not required to use Vapor but I want to try to follow the recommendations, unfortunately the makefile will break as soon as the first command with sudo.

Having sudo in the makefile is not ideal, but a solution like this could be implemented

DEST := /usr/local/bin/vapor

build:
	swiftc ./scripts/build.swift
	./build
	rm ./build
install: build
    ifneq ($(shell id -u), 0)
	sudo mv .build/release/vapor ${DEST}
	sudo chmod 755 ${DEST}
    else
	mv .build/release/vapor ${DEST}
	chmod 755 ${DEST}
    endif
uninstall:
    ifneq ($(shell id -u), 0)
	sudo rm ${DEST}
    else
	rm ${DEST}
    endif
clean:
	rm -rf .build

This should run commands without sudo if the user is root, otherwise it should use sudo as currently implemented.

0xTim commented

@Jeffrey-de-Bruijn I'm happy to accept this as a PR if you want to add it 🙌