Closed alop closed 7 years ago
@alop great catch...you're completely right. if you want to just set up a PR to point out that homebrew was used for GNU versions of the tools...that would probably be the best approach. if you don't have time for it, we can assign it out. completely your call.
My opinion is that the script should support a line which basically checks to see if the GNU Utils are installed, and throws an error if they aren't. Probably then also have supporting documentation in the docs mentioning that the GNU versions of the tools are required and not the BSD for OSX users. Although, we can probably just get away with a README update, since the script halts anyway if the GNU version isn't installed.
For the sake of cleanliness, I like the approach of checking to see if GNU versions are installed, and throwing an error if they aren't.. Let us know if this is something you want to work on @alop or what your thoughts are regarding this.
I would think that making the script more POSIX would be best, albeit not the easiest, since that requires having bash parse the options in a different manner. The sed lines are easy to fix as they're just missing the command and close slash. Quick fix would be an update to README.
Installing the brew versions of getopt and sed don't seem ideal. I would need to force link these which then could trickle down potentially other issues.
Obviously we cannot account for every system running setup-halcyon.sh
, but we probably want something a little more cross platform.
[jodewey:~/git/halcyon-vagrant-kubernetes] gnu-utils(+3/-0) 1 ± brew install gnu-getopt
Warning: gnu-getopt is a keg-only and another version is linked to opt.
Use `brew install --force` if you want to install this version
[jodewey:~/git/halcyon-vagrant-kubernetes] gnu-utils(+3/-0) ± brew install --force gnu-getopt
Warning: gnu-getopt-1.1.6 already installed, it's just not linked.
[jodewey:~/git/halcyon-vagrant-kubernetes] gnu-utils(+3/-0) ± brew link gnu-getopt
Warning: gnu-getopt is keg-only and must be linked with --force
Note that doing so can interfere with building software.
FWIW - without this PR
./setup-halcyon.sh --k8s-config kolla --k8s-version v$KUBERNETES_VERSION --guest-os centos
getopt: illegal option -- g
getopt: illegal option -- i
Setting up halcyon for: ubuntu with kubernetes 1.5.1 (default)
sed: 1: "./config.rb": invalid command code .
-e Running cleanup code and exiting
@alop thank you for the changes! reviewed #55 and this is a nice enhancement. @aric49 want to weight in?
updated my input in the PR.
The doc has instructions on how to get started on OSX, but the script assumes that things like
sed
andgetopt
are GNU varients. OSX ships with BSD versions of these utils and bombs out in the script.Should I spend a few cycles making it more posix, or should the docs be updated to say something like "OS X users should
brew install sed
andbrew install gnu-getopts
" ?