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

Tweaks to docker build script #1688

Merged
merged 3 commits into from Oct 1, 2018

Conversation

SteVwonder
Copy link
Member

The main changes are for better compatibility with Mac OSX, but one of the changes is also for users that don't use /bin/bash as their ${SHELL}.

Changes:

  • Support BSD getopt
  • Add option to skip mounting home directory: --no-home
  • Launch interactive container with /bin/bash regardless of ${SHELL}

Adds a `--no-home` option to the `docker-run-checks.sh` script.

Useful for building on Mac OSX where mounting the home directory within a Linux
container can cause issues down the road.  For example, the $HOME in OSX is
`/Users/$username`, but in linux, it is `/home/$username`.  Some tools, like
ssh-ident and emacs, can create files in your home directory with absolute path
information in them.  Launching these tools within the linux docker container
while the host Mac OSX home directory is mounted can lead to inconsistent state.

Additionally, if you are a hyper-conservative user, this limits the exposure and
risk of running the script.
The bionic-base image only has `/bin/bash` and `/bin/dash` shells.  If
`${SHELL}` is set to anything else (e.g. `/bin/zsh`), the interactive container
fails to run.  Hardcodes the shell command to `/bin/bash` to ensure the
interactive session always works.
@SteVwonder SteVwonder requested a review from grondo October 1, 2018 19:02
@codecov-io
Copy link

Codecov Report

Merging #1688 into master will increase coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##           master   #1688      +/-   ##
=========================================
+ Coverage   79.29%   79.3%   +<.01%     
=========================================
  Files         186     186              
  Lines       35059   35059              
=========================================
+ Hits        27801   27803       +2     
+ Misses       7258    7256       -2
Impacted Files Coverage Δ
src/bindings/lua/lua-hostlist/hostlist.c 58.78% <0%> (-0.23%) ⬇️
src/cmd/flux-job.c 88% <0%> (+0.66%) ⬆️
src/common/libflux/mrpc.c 86.64% <0%> (+1.14%) ⬆️

Copy link
Contributor

@grondo grondo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable to me!

@grondo
Copy link
Contributor

grondo commented Oct 1, 2018

Going to go ahead and merge if that's ok. It will be interesting to see if we hit the same travis-ci errors on master.

@grondo grondo merged commit 70c067e into flux-framework:master Oct 1, 2018
@SteVwonder SteVwonder deleted the docker-build-tweaks branch February 16, 2019 00:22
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

Successfully merging this pull request may close these issues.

None yet

3 participants