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

Issue:
Use main docker build command by default rather than buildx
Currently `docker.build()` is an alias to `docker.buildx.build()`, but it's not clear to me why this is since `docker build` and `docker buildx build` aren't entirely compatible and the latter still seems to be somewhat experimental?

I have hit two issues with `docker.buildx.build()`: using `FROM sha256:{image.id}` in a Dockerfile is not recognised (it is with `docker build`); and on another host my docker client does not have buildx installed (and I'm very reluctant for this to be a requirement).

Under the assumption that the main reason `docker build` is not implemented is because it's additional effort on top of supporting `docker buildx` commands I've put together a basic implementation that is working for me - the PR is #346, feel free to take it over or let me know how I can get it ready for merge. Unfortunately this is a breaking change, since the `docker.build()` API is slightly different... This does seem a more correct mirror of the CLI, so I wonder if there's a way to get it in with the appropriate changelog notices and version bump?


Discussion / hints:

Fix patch:
diff --git a/docs/template/sub-commands/buildx.md b/docs/template/sub-commands/buildx.md
new file mode 100644
index 00000000..34c91d1d
--- /dev/null
+++ b/docs/template/sub-commands/buildx.md
@@ -0,0 +1,23 @@
+{{autogenerated}}
+
+## Notes about the transition between the legacy builder and buildx
+
+Users are encouraged to use buildx in Python-on-whales through the
+[`docker.build()`](buildx.md#build) function.
+
+Buildx is the next gen Docker builder and a transition is underway to make
+the `docker build` shell command use buildx. Python-on-whales has had an opinionated
+answer on the matter as `docker.build()` will always use buildx. This is because Python-on-whales was created 
+during the transition and doesn't have an existing user codebase to support.
+
+The legacy builder is still available by calling [`docker.legacy_build()`](image.md#legacy_build), but note that
+
+* It won't work if you use Docker 22.06 or above 
+* It won't work if you used `docker.buildx.install()` or `docker buildx install` previously 
+* It won't work if you had set the environment variable `DOCKER_BUILDKIT` to `1`
+
+Some resources on the matter:
+
+* [Discussion about supporting the legacy builder in Python-on-whales](https://github.com/gabrieldemarmiesse/python-on-whales/issues/345)
+* [Proposal: make BuildKit the default builder on Linux](https://github.com/moby/moby/issues/40379)
+* [Deprecated Engine Features: Legacy builder for Linux images](https://github.com/docker/cli/blob/master/docs/deprecated.md#legacy-builder-for-linux-images)
diff --git a/python_on_whales/components/image/cli_wrapper.py b/python_on_whales/components/image/cli_wrapper.py
index 003cea6f..278d34dd 100644
--- a/python_on_whales/components/image/cli_wrapper.py
+++ b/python_on_whales/components/image/cli_wrapper.py
@@ -202,6 +202,80 @@ def __init__(self, client_config: ClientConfig):
             client_config
         ).build
 
+    def legacy_build(
+        self,
+        context_path: ValidPath,
+        add_hosts: Dict[str, str] = {},
+        build_args: Dict[str, str] = {},
+        cache: bool = True,
+        file: Optional[ValidPath] = None,
+        labels: Dict[str, str] = {},
+        network: Optional[str] = None,
+        pull: bool = False,
+        tags: Union[str, List[str]] = [],
+        target: Optional[str] = None,
+    ) -> python_on_whales.components.image.cli_wrapper.Image:
+        """Build a Docker image with the old Docker builder (meaning not using buildx/buildkit)
+
+        As the name implies this is a legacy building method. Users are strongly encouraged to use
+        `docker.build()` instead. The legacy builder will not be available in docker v22.06 and above.
+
+        This function also won't run the legacy builder if the environment variable
+        `DOCKER_BUILDKIT` is set to `1` or if you had run previously `docker buildx install` from bash
+        or `docker.buildx.install()` from Python.
+
+        Some resources on why moving to buildx/buildkit is necessary:
+
+        * [Proposal: make BuildKit the default builder on Linux](https://github.com/moby/moby/issues/40379)
+        * [Deprecated Engine Features: Legacy builder for Linux images](https://github.com/docker/cli/blob/master/docs/deprecated.md#legacy-builder-for-linux-images)
+
+        A `python_on_whales.Image` is returned, even when using multiple tags.
+        That is because it will produce a single image with multiple tags.
+
+        # Arguments
+            context_path: The path of the build context. Defaults to the current working directory
+            add_hosts: Hosts to add. `add_hosts={"my_host1": "192.168.32.35"}`
+            build_args: The build arguments.
+                ex `build_args={"PY_VERSION": "3.7.8", "UBUNTU_VERSION": "20.04"}`.
+            cache: Whether or not to use the cache, defaults to True
+            file: The path of the Dockerfile, defaults to `context_path/Dockerfile`
+            labels: Dict of labels to add to the image.
+                `labels={"very-secure": "1", "needs-gpu": "0"}` for example.
+            network: which network to use when building the Docker image
+            pull: Always attempt to pull a newer version of the image
+            tags: Tag or tags to put on the resulting image.
+            target: Set the target build stage to build.
+
+        # Returns
+            A `python_on_whales.Image`
+        """
+        # to make it easier to write and read tests, the tests of this function
+        # are also grouped with the tests of "docker.build()".
+        tags = to_list(tags)
+        full_cmd = self.docker_cmd + ["build"]
+
+        full_cmd.add_args_list(
+            "--add-host", format_dict_for_cli(add_hosts, separator=":")
+        )
+        full_cmd.add_args_list("--build-arg", format_dict_for_cli(build_args))
+        full_cmd.add_args_list("--label", format_dict_for_cli(labels))
+        full_cmd.add_flag("--pull", pull)
+        full_cmd.add_simple_arg("--file", file)
+        full_cmd.add_simple_arg("--target", target)
+        full_cmd.add_simple_arg("--network", network)
+        full_cmd.add_flag("--no-cache", not cache)
+        full_cmd.add_args_list("--tag", tags)
+
+        docker_image = python_on_whales.components.image.cli_wrapper.ImageCLI(
+            self.client_config
+        )
+        full_cmd.append(context_path)
+        stdout_lines = run(full_cmd).splitlines()
+        for line in stdout_lines:
+            if "Successfully built" in line:
+                image_id = line.split(" ")[-1]
+        return docker_image.inspect(image_id)
+
     def history(self):
         """Not yet implemented"""
         raise NotImplementedError
diff --git a/python_on_whales/docker_client.py b/python_on_whales/docker_client.py
index 9e181faa..8c5caef8 100644
--- a/python_on_whales/docker_client.py
+++ b/python_on_whales/docker_client.py
@@ -154,6 +154,7 @@ def __init__(
         # aliases
         self.attach = None
         self.build = self.buildx.build
+        self.legacy_build = self.image.legacy_build
         self.commit = self.container.commit
         self.copy = self.container.copy
         self.create = self.container.create


Test patch:
diff --git a/tests/python_on_whales/components/buildx/test_buildx_cli_wrapper.py b/tests/python_on_whales/components/buildx/test_buildx_cli_wrapper.py
index 3f54e307..2fcba07f 100644
--- a/tests/python_on_whales/components/buildx/test_buildx_cli_wrapper.py
+++ b/tests/python_on_whales/components/buildx/test_buildx_cli_wrapper.py
@@ -52,6 +52,12 @@ def test_buildx_build_load_docker_driver(tmp_path):
     assert my_image.size > 1000
 
 
+def test_legacy_build_simple_case(tmp_path):
+    (tmp_path / "Dockerfile").write_text(dockerfile_content1)
+    my_image = docker.legacy_build(tmp_path)
+    assert my_image.size > 1000
+
+
 @pytest.mark.usefixtures("with_docker_driver")
 def test_buildx_build_push_registry(tmp_path, docker_registry):
     (tmp_path / "Dockerfile").write_text(dockerfile_content1)
@@ -161,6 +167,12 @@ def test_buildx_build_load_container_driver_tag(tmp_path):
         assert my_image.repo_tags == ["my_tag:1"]
 
 
+def test_legacy_build_tag(tmp_path):
+    (tmp_path / "Dockerfile").write_text(dockerfile_content1)
+    with docker.legacy_build(tmp_path, tags="my_tag:1") as my_image:
+        assert my_image.repo_tags == ["my_tag:1"]
+
+
 @pytest.mark.usefixtures("with_container_driver")
 def test_buildx_build_load_container_driver_tags(tmp_path):
     (tmp_path / "Dockerfile").write_text(dockerfile_content1)
@@ -258,6 +270,12 @@ def test_buildx_build_single_tag(tmp_path):
     assert "hello1:latest" in image.repo_tags
 
 
+def test_legacy_build_single_tag(tmp_path):
+    (tmp_path / "Dockerfile").write_text(dockerfile_content1)
+    image = docker.legacy_build(tmp_path, tags="hello1")
+    assert "hello1:latest" in image.repo_tags
+
+
 @pytest.mark.usefixtures("with_docker_driver")
 def test_buildx_build_multiple_tags(tmp_path):
     (tmp_path / "Dockerfile").write_text(dockerfile_content1)
@@ -266,6 +284,13 @@ def test_buildx_build_multiple_tags(tmp_path):
     assert "hello2:latest" in image.repo_tags
 
 
+def test_legacy_build_multiple_tags(tmp_path):
+    (tmp_path / "Dockerfile").write_text(dockerfile_content1)
+    image = docker.legacy_build(tmp_path, tags=["hello1", "hello2"])
+    assert "hello1:latest" in image.repo_tags
+    assert "hello2:latest" in image.repo_tags
+
+
 @pytest.mark.usefixtures("with_docker_driver")
 def test_cache_invalidity(tmp_path):
 
@@ -302,12 +327,22 @@ def test_buildx_build_network(tmp_path):
     docker.buildx.build(tmp_path, network="host")
 
 
+def test_legacy_build_network(tmp_path):
+    (tmp_path / "Dockerfile").write_text(dockerfile_content1)
+    docker.legacy_build(tmp_path, network="host")
+
+
 @pytest.mark.usefixtures("with_docker_driver")
 def test_buildx_build_file(tmp_path):
     (tmp_path / "Dockerfile111").write_text(dockerfile_content1)
     docker.buildx.build(tmp_path, file=(tmp_path / "Dockerfile111"))
 
 
+def test_legacy_build_file(tmp_path):
+    (tmp_path / "Dockerfile111").write_text(dockerfile_content1)
+    docker.legacy_build(tmp_path, file=(tmp_path / "Dockerfile111"))
+
+
 def test_buildx_create_remove():
     builder = docker.buildx.create()