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

Ensure that the configuration file is an absolute path in Docker build #306

Merged
merged 3 commits into from
Jul 30, 2019

Conversation

hhromic
Copy link
Contributor

@hhromic hhromic commented Jul 6, 2019

PR #302 broke the Docker build script as previously documented. See below.

The specific problem is in commit 2ddd7c1, where the passed config file (using the -c option) is now mounted inside the container using the --volume src:dest:opt Docker option.

If you pass a relative-path config file (like in the non-working example you gave in the PR description) you get this error after building the Docker image and starting the pi-gen build:

bash ${RPi-Distro/pi-gen/build-docker.sh} -c config
(...)
./build.sh: line 137: source: /config: is a directory

The problem is that Docker requires absolute paths for mounting single files inside the container, otherwise it silently tries to mount a volume name instead as an empty directory. Therefore the Docker build no longer works with the following invocation forms (relative config-paths):

./build-docker.sh -c myconfig
/path/to/build-docker.sh -c myconfig   # also doesn't work

And instead the Docker build must now be invoked using absolute config-paths:

./build-docker.sh -c /path/to/myconfig   # works
./build-docker.sh -c $PWD/myconfig   # alternative
/path/to/build-docker.sh -c /path/to/myconfig   # another alternative

This is not really fixable because is just the way Docker works when using --volume.

Instead, this PR improves the documentation overall and adds how build-docker.sh works currently.

Update: Fixed now using realpath (see commit 4fd20bd) that preserves old behaviour.

Also I'm silencing a new shellcheck warning and slightly fixing a missing && in a Docker pipeline.

@XECDesign
Copy link
Member

A few merge conflicts that need to be resolved

@hhromic
Copy link
Contributor Author

hhromic commented Jul 24, 2019

I have a better idea now to solve the issue addressed by this PR: use realpath to resolve the config file if given as relative. This way it will preserve the expected behaviour. I will be updating the PR in coming days after testing. Please keep it open.
Thanks!

hhromic added 3 commits July 29, 2019 20:05
* In case of error, `&&` does not continue execution
* SC2086: Double quote to prevent globbing and word splitting.
The specific problem is in commit 2ddd7c1, where the passed config file
(using the `-c` option) is now mounted inside the container using the
`--volume src:dest:opt` Docker option.

The problem is that Docker requires absolute paths for mounting single
files inside the container, otherwise it silently tries to mount a volume
name instead as an empty directory. Therefore the Docker build no longer
works with the following invocation forms (relative config-paths):

    ./build-docker.sh -c myconfig
    /path/to/build-docker.sh -c myconfig   # also doesn't work

This commit uses `realpath` (included in coreutils) in the Docker build
script to ensure that the passed configuration file is always an
absolute path before passing it to Docker.
@hhromic hhromic force-pushed the fix-docker-build branch from 34b1d47 to 4fd20bd Compare July 29, 2019 20:00
@hhromic hhromic changed the title Better document Docker build Ensure that the configuration file is an absolute path in Docker build Jul 29, 2019
@hhromic
Copy link
Contributor Author

hhromic commented Jul 29, 2019

@XECDesign I rebased and changed now the PR to use realpath in the Docker build script. In this way, the old behaviour is preserved therefore no extra documentation is necessary. It just works.
I tested this and is working fine again for all cases including relative config file arguments.
Hope you are fine with it otherwise let me know of any changes desired.

@XECDesign XECDesign merged commit 920e22b into RPi-Distro:master Jul 30, 2019
@XECDesign
Copy link
Member

Thanks again, Hugo. Much appreciated.

@hhromic hhromic deleted the fix-docker-build branch August 1, 2019 10:31
fuji246 pushed a commit to lomorage/pi-gen that referenced this pull request Sep 17, 2019
RPi-Distro#306)

* Use `&&` instead of `;` in Docker pipeline

* In case of error, `&&` does not continue execution

* Silence shellcheck warning

* SC2086: Double quote to prevent globbing and word splitting.

* Ensure that the configuration file is an absolute path in Docker build

The specific problem is in commit 2ddd7c1, where the passed config file
(using the `-c` option) is now mounted inside the container using the
`--volume src:dest:opt` Docker option.

The problem is that Docker requires absolute paths for mounting single
files inside the container, otherwise it silently tries to mount a volume
name instead as an empty directory. Therefore the Docker build no longer
works with the following invocation forms (relative config-paths):

    ./build-docker.sh -c myconfig
    /path/to/build-docker.sh -c myconfig   # also doesn't work

This commit uses `realpath` (included in coreutils) in the Docker build
script to ensure that the passed configuration file is always an
absolute path before passing it to Docker.
PeterJohnson pushed a commit to PeterJohnson/WPILibPi that referenced this pull request Dec 7, 2019
RPi-Distro#306)

* Use `&&` instead of `;` in Docker pipeline

* In case of error, `&&` does not continue execution

* Silence shellcheck warning

* SC2086: Double quote to prevent globbing and word splitting.

* Ensure that the configuration file is an absolute path in Docker build

The specific problem is in commit 2ddd7c1, where the passed config file
(using the `-c` option) is now mounted inside the container using the
`--volume src:dest:opt` Docker option.

The problem is that Docker requires absolute paths for mounting single
files inside the container, otherwise it silently tries to mount a volume
name instead as an empty directory. Therefore the Docker build no longer
works with the following invocation forms (relative config-paths):

    ./build-docker.sh -c myconfig
    /path/to/build-docker.sh -c myconfig   # also doesn't work

This commit uses `realpath` (included in coreutils) in the Docker build
script to ensure that the passed configuration file is always an
absolute path before passing it to Docker.
general-wedge pushed a commit to HQapp/hq-os that referenced this pull request Jan 5, 2020
RPi-Distro#306)

* Use `&&` instead of `;` in Docker pipeline

* In case of error, `&&` does not continue execution

* Silence shellcheck warning

* SC2086: Double quote to prevent globbing and word splitting.

* Ensure that the configuration file is an absolute path in Docker build

The specific problem is in commit 2ddd7c1, where the passed config file
(using the `-c` option) is now mounted inside the container using the
`--volume src:dest:opt` Docker option.

The problem is that Docker requires absolute paths for mounting single
files inside the container, otherwise it silently tries to mount a volume
name instead as an empty directory. Therefore the Docker build no longer
works with the following invocation forms (relative config-paths):

    ./build-docker.sh -c myconfig
    /path/to/build-docker.sh -c myconfig   # also doesn't work

This commit uses `realpath` (included in coreutils) in the Docker build
script to ensure that the passed configuration file is always an
absolute path before passing it to Docker.
alexgg pushed a commit to balena-os/pi-gen that referenced this pull request Jul 12, 2021
RPi-Distro#306)

* Use `&&` instead of `;` in Docker pipeline

* In case of error, `&&` does not continue execution

* Silence shellcheck warning

* SC2086: Double quote to prevent globbing and word splitting.

* Ensure that the configuration file is an absolute path in Docker build

The specific problem is in commit 2ddd7c1, where the passed config file
(using the `-c` option) is now mounted inside the container using the
`--volume src:dest:opt` Docker option.

The problem is that Docker requires absolute paths for mounting single
files inside the container, otherwise it silently tries to mount a volume
name instead as an empty directory. Therefore the Docker build no longer
works with the following invocation forms (relative config-paths):

    ./build-docker.sh -c myconfig
    /path/to/build-docker.sh -c myconfig   # also doesn't work

This commit uses `realpath` (included in coreutils) in the Docker build
script to ensure that the passed configuration file is always an
absolute path before passing it to Docker.
UmeshMohan-Dozee pushed a commit to DozeeRnD/pi-gen that referenced this pull request Sep 18, 2024
RPi-Distro#306)

* Use `&&` instead of `;` in Docker pipeline

* In case of error, `&&` does not continue execution

* Silence shellcheck warning

* SC2086: Double quote to prevent globbing and word splitting.

* Ensure that the configuration file is an absolute path in Docker build

The specific problem is in commit 2ddd7c1, where the passed config file
(using the `-c` option) is now mounted inside the container using the
`--volume src:dest:opt` Docker option.

The problem is that Docker requires absolute paths for mounting single
files inside the container, otherwise it silently tries to mount a volume
name instead as an empty directory. Therefore the Docker build no longer
works with the following invocation forms (relative config-paths):

    ./build-docker.sh -c myconfig
    /path/to/build-docker.sh -c myconfig   # also doesn't work

This commit uses `realpath` (included in coreutils) in the Docker build
script to ensure that the passed configuration file is always an
absolute path before passing it to Docker.
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.

2 participants