Skip to content

Commit

Permalink
[Docker] Refactor/clean-up of docker/bash.sh (#8670)
Browse files Browse the repository at this point in the history
* [Docker] Refactor/clean-up of docker/bash.sh

- Added detailed help message, displayed using `-h` or `--help`.

- Optional flags handled using `getopt`, can now occur in any order.

- `--mount` flag may occur more than once.

- Switched from short arguments to docker-run to long arguments
  (e.g. `--volume` instead of `-v`).  Short arguments are good
  shortcuts for interactive work, but can be more difficult to read in
  longer scripts.

- Mount the `.tvm_test_data` folder, to avoid re-downloading test data
  already available in the host environment.

* [Docker] docker/bash.sh CI fix

Dash-prefixed arguments as part of the command now require prefixing with
-- to separate them from arguments intended for docker/bash.sh

* [Docker] docker/bash.sh, consistent quoting

* [Docker] Added --repo-mount-point for docker/bash.sh

* [Docker] Updated command-line parsing of docker/bash.sh

- Maintained previous behavior, any unrecognized flags after the
  docker/bash.sh are part of the command, no -- is
  needed. (e.g. docker/bash.sh ci_gpu make -j2)

- Reverted changes to Jenskinsfile to add a --, no longer needed.

* [Docker] Fixed multi-argument commands

* [Docker] docker/bash.sh check permissions before mounting ~/.tvm_test_data

* [Docker] Consistent workplace directory in docker/bash.sh for Jenkins

Some locations in the CI perform build commands outside of the build
steps (e.g. tests/scripts/task_ci_setup.sh#L38), and cmake doesn't
like it if the build directory changes.  These should probably be
moved into the build steps of the CI, and be packed in tvm_multilib in
the Jenkinsfile, but for the meantime maintaining a consistent
/workspace directory on all CI nodes allows cmake to run.

* [Docker] Updated bash.sh for MacOS compatibility

MacOS has an older version of bash that handles arrays slightly
differently.  All instances of array expansion `"${ARRAY[@]}"` should
instead be written as `${ARRAY[@]+"${ARRAY[@]}"}`.  Otherwise, `set -u`
will erroneously complain about an undefined variable. See
https://stackoverflow.com/a/61551944 for details.

Even though this is an older version of bash (observed in version
3.2.57), this is the last major version available under GPLv2 and is
therefore the default version on MacOSX.  At some point, the
`docker/bash.sh` could be migrated to python for ease of
maintenance/testing.
  • Loading branch information
Lunderberg authored Aug 11, 2021
1 parent e88fe77 commit 722efc5
Show file tree
Hide file tree
Showing 3 changed files with 313 additions and 105 deletions.
Empty file modified Jenkinsfile
100644 → 100755
Empty file.
9 changes: 5 additions & 4 deletions docker/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ interactive bash session with a given image_name.

The script does the following things:

- Mount current directory to /workspace and set it as home
- Mount current directory to the same location in the docker container, and set it as home
- Switch user to be the same user that calls the bash.sh
- Use the host-side network

Expand Down Expand Up @@ -102,9 +102,10 @@ The command ``./docker/build.sh image_name COMMANDS`` is almost equivelant to
``./docker/bash.sh image_name COMMANDS`` but in the case of ``bash.sh``
a build attempt is not done.

The build command will map the tvm root to /workspace/ inside the container
with the same user as the user invoking the docker command.
Here are some common use examples to perform CI tasks.
The build command will map the tvm root to the corresponding location
inside the container with the same user as the user invoking the
docker command. Here are some common use examples to perform CI
tasks.

- lint the python codes

Expand Down
Loading

0 comments on commit 722efc5

Please sign in to comment.