From 15993834293f91fe27033b31edd520fc8eabdd5c Mon Sep 17 00:00:00 2001 From: joncrall Date: Sun, 26 Jun 2022 17:10:02 -0400 Subject: [PATCH] Cleanup --- cibuildwheel/docker_container.py | 23 ++++--------------- unit_test/docker_container_test.py | 37 ++++++++++++++++-------------- 2 files changed, 24 insertions(+), 36 deletions(-) diff --git a/cibuildwheel/docker_container.py b/cibuildwheel/docker_container.py index 0d010aee..cad594d8 100644 --- a/cibuildwheel/docker_container.py +++ b/cibuildwheel/docker_container.py @@ -55,7 +55,6 @@ class DockerContainer: simulate_32_bit: bool = False, cwd: Optional[PathOrStr] = None, container_engine: str = "docker", - env: Optional[Dict[str, str]] = None, ): if not docker_image: raise ValueError("Must have a non-empty docker image to run.") @@ -80,20 +79,6 @@ class DockerContainer: shell_args = ["linux32", "/bin/bash"] if self.simulate_32_bit else ["/bin/bash"] - # volume args are ignored on CircleCI - # For a discussion of if :Z should be included or not: - # https://github.com/pypa/cibuildwheel/pull/966#discussion_r906707824 - # https://stackoverflow.com/questions/35218194/what-is-z-flag-in-docker-containers-volumes-from-option/35222815#35222815 - # https://github.com/moby/moby/issues/30934 - # The Z option indicates that the bind mount content is private and - # unshared. Use extreme caution with these options. Bind-mounting a - # system directory such as /home or /usr with the Z option renders your - # host machine inoperable and you may need to relabel the host machine - # files by hand. - volume_args = ['--volume=/:/host'] - # volume_args = ['--volume=/:/host:Z'] - # volume_args = [] - subprocess.run( [ self.container_engine, @@ -101,8 +86,8 @@ class DockerContainer: "--env=CIBUILDWHEEL", f"--name={self.name}", "--interactive", + "--volume=/:/host", # ignored on CircleCI *network_args, - *volume_args, self.docker_image, *shell_args, ], @@ -126,13 +111,13 @@ class DockerContainer: self.bash_stdout = self.process.stdout # run a noop command to block until the container is responding - self.call(["/bin/true"], cwd="/") + self.call(["/bin/true"]) if self.cwd: # Although `docker create -w` does create the working dir if it # does not exist, podman does not. There does not seem to be a way # to setup a workdir for a container running in podman. - self.call(["mkdir", "-p", str(self.cwd)], cwd="/") + self.call(["mkdir", "-p", os.fspath(self.cwd)]) return self @@ -389,4 +374,4 @@ class DockerContainer: def shell_quote(path: PurePath) -> str: - return shlex.quote(str(path)) + return shlex.quote(os.fspath(path)) diff --git a/unit_test/docker_container_test.py b/unit_test/docker_container_test.py index 52603b5a..fac3dfdf 100644 --- a/unit_test/docker_container_test.py +++ b/unit_test/docker_container_test.py @@ -36,7 +36,7 @@ _STATE = { # @atexit.register -def _cleanup_tempdir(): +def _cleanup_podman_vfs_tempdir(): """ Cleans up any configuration written by :func:`basis_container_kwargs`. @@ -58,10 +58,6 @@ def _cleanup_tempdir(): # unless you fake a UID of 0. The package rootlesskit helps with that. if _STATE['using_podman']: subprocess.call(['podman', 'unshare', 'rm', '-rf', temp_test_dir.name]) - try: - temp_test_dir.cleanup() - except Exception as ex: - print(f"Issue cleaning up ex = {ex!r}") _STATE['temp_test_dir'] = None @@ -82,24 +78,31 @@ def basis_container_kwargs(): parameterized test. """ - if _STATE['temp_test_dir'] is None: - # Only setup the temp directory once for all tests - _STATE['temp_test_dir'] = tempfile.TemporaryDirectory(prefix="cibw_test_") - # Register the special cleanup hook after the temp directory is created - # to ensure that it runs before the temp directory logic runs (which - # will not handle cases where there is a fake root UID). - atexit.register(_cleanup_tempdir) - - temp_test_dir = _STATE['temp_test_dir'] - + # TODO: Pytest should be aware of if we are trying to test docker / podman + # or not HAVE_DOCKER = bool(shutil.which("docker")) HAVE_PODMAN = bool(shutil.which("podman")) - if HAVE_DOCKER: + REQUESTED_DOCKER = HAVE_DOCKER + REQUESTED_PODMAN = HAVE_PODMAN + + if _STATE['temp_test_dir'] is None: + # Only setup the temp directory once for all tests + _STATE['temp_test_dir'] = tempfile.TemporaryDirectory(prefix="cibw_test_") + if REQUESTED_PODMAN: + # Register the special cleanup hook after the temp directory is + # created to ensure that it runs before the temp directory logic + # runs (which will not handle cases where there is a fake root + # UID). + atexit.register(_cleanup_podman_vfs_tempdir) + + temp_test_dir = _STATE['temp_test_dir'] + + if REQUESTED_DOCKER: # Basic podman configuration yield {"container_engine": "docker", "docker_image": DEFAULT_IMAGE} - if HAVE_PODMAN: + if REQUESTED_PODMAN: # Basic podman usage _STATE['using_podman'] = True yield {"container_engine": "podman", "docker_image": DEFAULT_IMAGE}