search

GetPoplog / Seed

Scripts for getting Poplog onto your Linux machine
MIT License
7 stars 3 forks source link

Fix typo in the definition of the pop_ved environment variable, closing #132 #133

Closed sfkleach closed 1 year ago

sfkleach commented 1 year ago

This corrects a long-standing minor error that affected the poplog command tool, so that the ved command could not be invoked without a mishap.

willprice commented 1 year ago

Would set -u have caught this class of bug?

sfkleach commented 1 year ago

Would set -u have caught this class of bug?

That is a good question. The script makePopEnv.sh does start with set -euo pipefail but the popenv.sh scripts that it auto-generates do not. If we add the -u flag to them then the failure would have happened at build time, during buildInPlace.mk while calling echoEnv.sh.

My only worry is that this might lock us that little bit more into bash? I am not sure which flags the two shells share. The scenario that matters is for a user that does not want to use the new-fangled poplog commander. I have tried to protect their right to do things the old way, in order to protect legacy scripts. Could this impact on that scenario?

sfkleach commented 1 year ago

Looks like set -eu is shared by /bin/sh and /bin/bash. Will raise an issue to enhance makePopEnv.sh appropriately.