diff mbox series

[v3,for,3.0,13/18] docker: add --hint to docker.py check

Message ID 20180717195553.9111-14-alex.bennee@linaro.org
State New
Headers show
Series docker fixes (and one tcg test tweak) | expand

Commit Message

Alex Bennée July 17, 2018, 7:55 p.m. UTC
When a check fails we currently just report why we failed. This is not
totally helpful to people who want to boot-strap a new image. Add a
--hint option which we can pass down to give a bit more information.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

---
 tests/docker/Makefile.include | 3 ++-
 tests/docker/docker.py        | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

-- 
2.17.1

Comments

Fam Zheng July 24, 2018, 7:46 a.m. UTC | #1
On Wed, Jul 18, 2018 at 4:03 AM Alex Bennée <alex.bennee@linaro.org> wrote:
>

> When a check fails we currently just report why we failed. This is not

> totally helpful to people who want to boot-strap a new image. Add a

> --hint option which we can pass down to give a bit more information.

>

> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---

>  tests/docker/Makefile.include | 3 ++-

>  tests/docker/docker.py        | 3 ++-

>  2 files changed, 4 insertions(+), 2 deletions(-)

>

> diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include

> index ec23620153..c9e412f9d0 100644

> --- a/tests/docker/Makefile.include

> +++ b/tests/docker/Makefile.include

> @@ -73,7 +73,8 @@ docker-binfmt-image-debian-%: $(DOCKER_FILES_DIR)/debian-bootstrap.docker

>                         $(if $(EXECUTABLE),--include-executable=$(EXECUTABLE)), \

>                         "BUILD","binfmt debian-$* (debootstrapped)"),           \

>                 $(call quiet-command,                                           \

> -                       $(DOCKER_SCRIPT) check --quiet qemu:debian-$* $<,       \

> +                       $(DOCKER_SCRIPT) check --quiet qemu:debian-$* $<        \

> +                       --hint "you will need to build $(EXECUTABLE)",          \

>                         "CHECK", "debian-$* exists"))

>

>  endif

> diff --git a/tests/docker/docker.py b/tests/docker/docker.py

> index 2f81c6b13b..523f4b95a2 100755

> --- a/tests/docker/docker.py

> +++ b/tests/docker/docker.py

> @@ -475,6 +475,7 @@ class CheckCommand(SubCommand):

>                              default="checksum", help="check type")

>          parser.add_argument("--olderthan", default=60, type=int,

>                              help="number of minutes")

> +        parser.add_argument("--hint", default="", help="hint to user")

>

>      def run(self, args, argv):

>          tag = args.tag

> @@ -487,7 +488,7 @@ class CheckCommand(SubCommand):

>

>          info = dkr.inspect_tag(tag)

>          if info is None:

> -            print("Image does not exist")

> +            print("Image does not exist %s" % (args.hint))


No, please. This is just hacky and makes no sense to me. The Makefile
can do this

  $(DOCKER_SCRIPT) check ... || { echo "you will need to build
$(EXECUTABLE)"; exit 1 }

Fam

>              return 1

>

>          if args.checktype == "checksum":

> --

> 2.17.1

>
Alex Bennée July 24, 2018, 8:52 a.m. UTC | #2
Fam Zheng <famz@redhat.com> writes:

> On Wed, Jul 18, 2018 at 4:03 AM Alex Bennée <alex.bennee@linaro.org> wrote:

>>

>> When a check fails we currently just report why we failed. This is not

>> totally helpful to people who want to boot-strap a new image. Add a

>> --hint option which we can pass down to give a bit more information.

>>

>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

>> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

>> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

>> ---

>>  tests/docker/Makefile.include | 3 ++-

>>  tests/docker/docker.py        | 3 ++-

>>  2 files changed, 4 insertions(+), 2 deletions(-)

>>

>> diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include

>> index ec23620153..c9e412f9d0 100644

>> --- a/tests/docker/Makefile.include

>> +++ b/tests/docker/Makefile.include

>> @@ -73,7 +73,8 @@ docker-binfmt-image-debian-%: $(DOCKER_FILES_DIR)/debian-bootstrap.docker

>>                         $(if $(EXECUTABLE),--include-executable=$(EXECUTABLE)), \

>>                         "BUILD","binfmt debian-$* (debootstrapped)"),           \

>>                 $(call quiet-command,                                           \

>> -                       $(DOCKER_SCRIPT) check --quiet qemu:debian-$* $<,       \

>> +                       $(DOCKER_SCRIPT) check --quiet qemu:debian-$* $<        \

>> +                       --hint "you will need to build $(EXECUTABLE)",          \

>>                         "CHECK", "debian-$* exists"))

>>

>>  endif

>> diff --git a/tests/docker/docker.py b/tests/docker/docker.py

>> index 2f81c6b13b..523f4b95a2 100755

>> --- a/tests/docker/docker.py

>> +++ b/tests/docker/docker.py

>> @@ -475,6 +475,7 @@ class CheckCommand(SubCommand):

>>                              default="checksum", help="check type")

>>          parser.add_argument("--olderthan", default=60, type=int,

>>                              help="number of minutes")

>> +        parser.add_argument("--hint", default="", help="hint to user")

>>

>>      def run(self, args, argv):

>>          tag = args.tag

>> @@ -487,7 +488,7 @@ class CheckCommand(SubCommand):

>>

>>          info = dkr.inspect_tag(tag)

>>          if info is None:

>> -            print("Image does not exist")

>> +            print("Image does not exist %s" % (args.hint))

>

> No, please. This is just hacky and makes no sense to me. The Makefile

> can do this

>

>   $(DOCKER_SCRIPT) check ... || { echo "you will need to build

> $(EXECUTABLE)"; exit 1 }


OK I'll do it in the Makefile.

>

> Fam

>

>>              return 1

>>

>>          if args.checktype == "checksum":

>> --

>> 2.17.1

>>



--
Alex Bennée
diff mbox series

Patch

diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index ec23620153..c9e412f9d0 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -73,7 +73,8 @@  docker-binfmt-image-debian-%: $(DOCKER_FILES_DIR)/debian-bootstrap.docker
 			$(if $(EXECUTABLE),--include-executable=$(EXECUTABLE)), \
 			"BUILD","binfmt debian-$* (debootstrapped)"),		\
 		$(call quiet-command,						\
-			$(DOCKER_SCRIPT) check --quiet qemu:debian-$* $<,	\
+			$(DOCKER_SCRIPT) check --quiet qemu:debian-$* $<	\
+			--hint "you will need to build $(EXECUTABLE)",		\
 			"CHECK", "debian-$* exists"))
 
 endif
diff --git a/tests/docker/docker.py b/tests/docker/docker.py
index 2f81c6b13b..523f4b95a2 100755
--- a/tests/docker/docker.py
+++ b/tests/docker/docker.py
@@ -475,6 +475,7 @@  class CheckCommand(SubCommand):
                             default="checksum", help="check type")
         parser.add_argument("--olderthan", default=60, type=int,
                             help="number of minutes")
+        parser.add_argument("--hint", default="", help="hint to user")
 
     def run(self, args, argv):
         tag = args.tag
@@ -487,7 +488,7 @@  class CheckCommand(SubCommand):
 
         info = dkr.inspect_tag(tag)
         if info is None:
-            print("Image does not exist")
+            print("Image does not exist %s" % (args.hint))
             return 1
 
         if args.checktype == "checksum":