Repository: gabrieldemarmiesse/python-on-whales (language: python)

Issue:
docker buildx command always fails in install check on Windows
The `subprocess.call` overrides all environment variables instead of updating them to add the `DOCKER_CLI_EXPERIMENTAL` variable. This has the effect of the docker buildx binary being downloaded every time a build step is run.

https://github.com/gabrieldemarmiesse/python-on-whales/blob/2f542756e15a7aa99cb2e28f4c52fdd700aac912/python_on_whales/utils.py#L179-L187

It looks like this is handled properly where this function is called, so perhaps that env dictionary should be passed in with the docker binary argument:

https://github.com/gabrieldemarmiesse/python-on-whales/blob/2f542756e15a7aa99cb2e28f4c52fdd700aac912/python_on_whales/utils.py#L106-L110


Discussion / hints:
Thank you for the bug report, It seems we had a similar problem a while ago and it was fixed. Does the bug still appear when doing `pip install --upgrade python-on-whales`?
Actually, after reading the docs, it seems that docker buildx is already installed on most docker distributions. We can now say that this convenience check is not necessary and it's on the user to make sure that docker buildx is installed and working. I'll make a pull request for that.

Fix patch:
diff --git a/python_on_whales/download_binaries.py b/python_on_whales/download_binaries.py
index 7700a057..92382275 100644
--- a/python_on_whales/download_binaries.py
+++ b/python_on_whales/download_binaries.py
@@ -1,7 +1,5 @@
-import os
 import platform
 import shutil
-import stat
 import tempfile
 import warnings
 from pathlib import Path
@@ -18,11 +16,9 @@
     "https://download.docker.com/{os}/static/stable/{arch}/docker-{version}.tgz"
 )
 WINDOWS_CLI_URL = "https://github.com/StefanScherer/docker-cli-builder/releases/download/{version}/docker.exe"
-TEMPLATE_BUILDX = "https://github.com/docker/buildx/releases/download/v{version}/buildx-v{version}.{os}-{arch}"
 
 
 DOCKER_BINARY_PATH = CACHE_DIR / "docker-cli" / DOCKER_VERSION / "docker"
-BUILDX_BINARY_PATH = Path.home() / ".docker" / "cli-plugins" / "docker-buildx"
 
 
 def get_docker_cli_url():
@@ -33,29 +29,6 @@ def get_docker_cli_url():
     return TEMPLATE_CLI.format(os=user_os, arch=arch, version=DOCKER_VERSION)
 
 
-def get_buildx_url():
-    user_os = platform.system().lower()
-    arch = get_arch_for_buildx_url()
-    url = TEMPLATE_BUILDX.format(version=BUILDX_VERSION, os=user_os, arch=arch)
-    if user_os == "windows":
-        url += ".exe"
-    return url
-
-
-def download_buildx():
-    BUILDX_BINARY_PATH.parent.mkdir(exist_ok=True, parents=True)
-    download_from_url(get_buildx_url(), BUILDX_BINARY_PATH)
-
-    st = os.stat(BUILDX_BINARY_PATH)
-    os.chmod(BUILDX_BINARY_PATH, st.st_mode | stat.S_IEXEC)
-    warnings.warn(
-        f"The docker buildx binary file {BUILDX_VERSION} was downloaded and put "
-        f"in `{BUILDX_BINARY_PATH.absolute()}`. \n"
-        f"You can feel free to remove it if you wish, Python on whales will download "
-        f"it again if needed."
-    )
-
-
 def download_docker_cli():
     file_to_download = get_docker_cli_url()
     with tempfile.TemporaryDirectory() as tmp_dir:
@@ -145,32 +118,3 @@ def get_arch_for_docker_cli_url():
             f"and make sure to copy past this error message. \n"
             f"In the meantime, install Docker manually on your system."
         )
-
-
-def get_arch_for_buildx_url():
-    arch = platform.architecture()[0]
-
-    # I don't know the exact list of possible architectures,
-    # so if a user reports a NotImplementedError, we can easily add
-    # his/her platform here.
-    arch_mapping = {
-        "NotImplementedError": "arm-v6",
-        "NotImplementedError2": "arm-v7",
-        "NotImplementedError3": "arm64",
-        "NotImplementedError4": "ppc64le",
-        "NotImplementedError5": "s390x",
-        "64bit": "amd64",
-    }
-
-    try:
-        return arch_mapping[arch]
-    except KeyError:
-        raise NotImplementedError(
-            f"The architecture detected on your system is `{arch}`, the list of "
-            f"available architectures is {list(arch_mapping.values())}. \n"
-            f"Please open an issue at \n"
-            f"https://github.com/gabrieldemarmiesse/python-on-whales/issues "
-            f"and make sure to copy past this error message. \n"
-            f"In the meantime, install Docker buildx manually on your system: \n"
-            f"https://github.com/docker/buildx"
-        )
diff --git a/python_on_whales/utils.py b/python_on_whales/utils.py
index 63f46cd2..6a66c804 100644
--- a/python_on_whales/utils.py
+++ b/python_on_whales/utils.py
@@ -1,6 +1,5 @@
 import os
 import subprocess
-import warnings
 from pathlib import Path
 from queue import Queue
 from subprocess import PIPE, Popen
@@ -9,8 +8,6 @@
 
 import pydantic
 
-from python_on_whales.download_binaries import download_buildx
-
 PROJECT_ROOT = Path(__file__).parents[1]
 
 
@@ -106,7 +103,6 @@ def run(
     subprocess_env = dict(os.environ)
     subprocess_env.update(env)
     if args[1] == "buildx":
-        install_buildx_if_needed(args[0])
         subprocess_env["DOCKER_CLI_EXPERIMENTAL"] = "enabled"
     if capture_stdout:
         stdout_dest = subprocess.PIPE
@@ -176,33 +172,6 @@ def removeprefix(string: str, prefix: str) -> str:
         return string
 
 
-def install_buildx_if_needed(docker_binary: str):
-    completed_process = subprocess.run(
-        [docker_binary, "buildx"],
-        stdout=subprocess.PIPE,
-        stderr=subprocess.PIPE,
-        env={"DOCKER_CLI_EXPERIMENTAL": "enabled"},
-    )
-    if completed_process.returncode == 0:
-        return
-
-    stderr = completed_process.stderr.decode()
-    if "is not a docker command" in stderr:
-        warnings.warn(
-            "It seems that docker buildx is not installed on your system. \n"
-            "It's going to be downloaded for you. It's only a one time thing."
-            "The next calls to the buildx command won't trigger the "
-            "download again."
-        )
-        download_buildx()
-    else:
-        raise RuntimeError(
-            f"It seems buildx is not properly installed. When running "
-            f"'docker buildx', here is the result:\n"
-            f"{stderr}"
-        )
-
-
 def reader(pipe, pipe_name, queue):
     try:
         with pipe:


Test patch:
diff --git a/tests/Dockerfile b/tests/Dockerfile
index 2657fcee..daa8b266 100644
--- a/tests/Dockerfile
+++ b/tests/Dockerfile
@@ -39,31 +39,8 @@ CMD pytest -v --durations=10 ./tests
 
 #-----------------------------------------------------------------------------
 FROM python_dependencies as tests_without_any_binary
-WORKDIR /python-on-whales
-COPY . .
 
-RUN pip install -e .
-
-CMD pytest -v --durations=10 ./tests/python_on_whales/components/buildx
-
-
-#-----------------------------------------------------------------------------
-FROM python_dependencies as tests_ubuntu_install_without_buildx
-
-RUN apt-get update && \
-    apt-get install -y \
-      apt-transport-https \
-      ca-certificates \
-      curl \
-      gnupg-agent \
-      software-properties-common
-RUN curl -fsSL https://download.docker.com/linux/ubuntu/gpg | apt-key add -
-RUN add-apt-repository \
-   "deb [arch=amd64] https://download.docker.com/linux/debian \
-   $(lsb_release -cs) \
-   stable"
-RUN  apt-get update && \
-     apt-get install -y docker-ce-cli
+COPY --from=get_buildx /docker-buildx /root/.docker/cli-plugins/
 
 WORKDIR /python-on-whales
 COPY . .
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index 6816745c..250dcec6 100644
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -10,7 +10,3 @@ docker run -v /var/run/docker.sock:/var/run/docker.sock test-image-python-on-wha
 
 docker buildx build --load -f tests/Dockerfile --target tests_without_any_binary -t test-image-python-on-whales .
 docker run -v /var/run/docker.sock:/var/run/docker.sock test-image-python-on-whales
-
-
-docker buildx build --load -f tests/Dockerfile --target tests_ubuntu_install_without_buildx -t test-image-python-on-whales .
-docker run -v /var/run/docker.sock:/var/run/docker.sock test-image-python-on-whales