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

'Which' not in dependencies #8900

Closed
johandroz opened this issue Oct 2, 2017 · 4 comments
Closed

'Which' not in dependencies #8900

johandroz opened this issue Oct 2, 2017 · 4 comments
Assignees

Comments

@johandroz
Copy link

johandroz commented Oct 2, 2017

Bug report

System info: InfluxDB 1.3.6

Steps to reproduce:

I was creating a container with influxdb with a fedora 26 base. It doesn't come with many "standard utilities" such as which.
Thus this part of the post install script will fail :
if [[ -f /etc/redhat-release ]]; then
# RHEL-variant logic
which systemctl &>/dev/null
if [[ $? -eq 0 ]]; then
install_systemd
else
# Assuming sysv
install_init
install_chkconfig
fi

Additional info: We can resolve this by installing which before installing influx.

@johandroz johandroz changed the title Which command not in dependencies 'Which' not in dependencies Oct 2, 2017
@phemmer
Copy link
Contributor

phemmer commented Oct 2, 2017

Alternatively, since this is obviously a bash script, you could do command -v systemctl, since command is a bash built-in. Or systemctl --version.

And since we're on the subject, the use of $? is unnecessary.

if command -v systemctl >/dev/null; then
  install_systemd
else
 ...

@CMajeri
Copy link

CMajeri commented Oct 2, 2017

@phemmer You're right, but the issue is that this is from the postinstall script of the RPM package. This isn't a homebrew install script :)
It assumes which is installed, but it might not be in small environments i.e. a container.

Assuming I run a fedora container that comes without which, I'd expect a dnf install to install the systemd service file (yes, running systemd in a container is a peculiar use-case, but it's a valid one nonetheless :).

I admit that environments without which that use systemd are very rare, which might be why this has gone unnoticed for so long.

@phemmer
Copy link
Contributor

phemmer commented Oct 2, 2017

@CMajeri

This isn't a homebrew install script

Actually it is: https://github.com/influxdata/influxdb/blob/master/scripts/post-install.sh#L48

@CMajeri
Copy link

CMajeri commented Oct 2, 2017

I meant it's from influx, @johandroz didn't write it himself!

@mark-rushakoff mark-rushakoff self-assigned this Oct 2, 2017
@ghost ghost removed the proposed label Oct 3, 2017
mark-rushakoff added a commit that referenced this issue Oct 4, 2017
Some systems don't have `which` installed.

Also fix some whitespace issues.

Fixes #8900.
benbjohnson pushed a commit to benbjohnson/influxdb that referenced this issue Oct 4, 2017
Some systems don't have `which` installed.

Also fix some whitespace issues.

Fixes influxdata#8900.
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

No branches or pull requests

5 participants