diff mbox

[v3,03/24] docker: rebuild image if 'extra files' checksum does not match

Message ID 20170521032956.27446-4-f4bug@amsat.org
State Superseded, archived
Headers show

Commit Message

Philippe Mathieu-Daudé May 21, 2017, 3:29 a.m. UTC
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 tests/docker/docker.py | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

Comments

Alex Bennée May 22, 2017, 7:55 a.m. UTC | #1
Philippe Mathieu-Daudé <f4bug@amsat.org> writes:

> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  tests/docker/docker.py | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)

Hmm odd what bracnh did you cut this from. git apply-mbox keeps falling
over:

Applying: docker: rebuild image if 'extra files' checksum does not match
error: sha1 information is lacking or useless (tests/docker/docker.py).
error: could not build fake ancestor
Patch failed at 0001 docker: rebuild image if 'extra files' checksum does not match
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


>
> diff --git a/tests/docker/docker.py b/tests/docker/docker.py
> index eef92a54f1..0dd6a3304f 100755
> --- a/tests/docker/docker.py
> +++ b/tests/docker/docker.py
> @@ -38,6 +38,9 @@ def _text_checksum(text):
>      """Calculate a digest string unique to the text content"""
>      return hashlib.sha1(text).hexdigest()
>
> +def _file_checksum(filename):
> +    return _text_checksum(open(filename, 'rb').read())
> +
>  def _guess_docker_command():
>      """ Guess a working docker command or raise exception if not found"""
>      commands = [["docker"], ["sudo", "-n", "docker"]]
> @@ -154,7 +157,7 @@ class Docker(object):
>          return labels.get("com.qemu.dockerfile-checksum", "")
>
>      def build_image(self, tag, docker_dir, dockerfile,
> -                    quiet=True, user=False, argv=None):
> +                    quiet=True, user=False, argv=None, extra_files_cksum=[]):
>          if argv == None:
>              argv = []
>
> @@ -170,7 +173,7 @@ class Docker(object):
>
>          tmp_df.write("\n")
>          tmp_df.write("LABEL com.qemu.dockerfile-checksum=%s" %
> -                     _text_checksum(dockerfile))
> +                     _text_checksum(dockerfile + "-".join(extra_files_cksum)))
>          tmp_df.flush()
>
>          self._do(["build", "-t", tag, "-f", tmp_df.name] + argv + \
> @@ -276,16 +279,22 @@ class BuildCommand(SubCommand):
>                      return 1
>
>              # Include files used by ADD directives found within the Dockerfile.
> +            cksum = []
>              if args.include_executable:
> +                # FIXME: there is no checksum of this executable and the linked
> +                # libraries, once the image built any change of this executable
> +                # or any library won't trigger another build.
>                  _copy_binary_with_libs(args.include_executable, docker_dir)
>              for filename in args.include_files or []:
>                  _copy_with_mkdir(filename, docker_dir)
> +                cksum += [_file_checksum(filename)]
>
>              argv += ["--build-arg=" + k.lower() + "=" + v
>                          for k, v in os.environ.iteritems()
>                          if k.lower() in FILTERED_ENV_NAMES]
>              dkr.build_image(tag, docker_dir, dockerfile,
> -                            quiet=args.quiet, user=args.user, argv=argv)
> +                            quiet=args.quiet, user=args.user, argv=argv,
> +                            extra_files_cksum=cksum)
>
>              rmtree(docker_dir)


--
Alex Bennée
Philippe Mathieu-Daudé May 22, 2017, 1:23 p.m. UTC | #2
Hi Alex,

On 05/22/2017 04:55 AM, Alex Bennée wrote:
>
> Philippe Mathieu-Daudé <f4bug@amsat.org> writes:
>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  tests/docker/docker.py | 15 ++++++++++++---
>>  1 file changed, 12 insertions(+), 3 deletions(-)
>
> Hmm odd what bracnh did you cut this from. git apply-mbox keeps falling
> over:
>
> Applying: docker: rebuild image if 'extra files' checksum does not match
> error: sha1 information is lacking or useless (tests/docker/docker.py).
> error: could not build fake ancestor
> Patch failed at 0001 docker: rebuild image if 'extra files' checksum does not match
> The copy of the patch that failed is found in: .git/rebase-apply/patch
> When you have resolved this problem, run "git am --continue".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".

Weird...

I still have the command in my history, I used:
   $ git format-patch master --cover-letter -v 3 -n -s -o 
outgoing/shippable_v3 --to qemu-devel@nongnu.org --to 'Alex Bennée 
<alex.bennee@linaro.org>' --to 'Fam Zheng <famz@redhat.com>'

Having:
   $ git rev-parse master
56821559f0ba682fe6b367815572e6f974d329ab

You can also fetch this branch here:
   https://github.com/philmd/qemu/tree/docker_shippable_v3
Shippable output:
   https://app.shippable.com/github/philmd/qemu/status/dashboard

Regards,

Phil.

>
>
>>
>> diff --git a/tests/docker/docker.py b/tests/docker/docker.py
>> index eef92a54f1..0dd6a3304f 100755
>> --- a/tests/docker/docker.py
>> +++ b/tests/docker/docker.py
>> @@ -38,6 +38,9 @@ def _text_checksum(text):
>>      """Calculate a digest string unique to the text content"""
>>      return hashlib.sha1(text).hexdigest()
>>
>> +def _file_checksum(filename):
>> +    return _text_checksum(open(filename, 'rb').read())
>> +
>>  def _guess_docker_command():
>>      """ Guess a working docker command or raise exception if not found"""
>>      commands = [["docker"], ["sudo", "-n", "docker"]]
>> @@ -154,7 +157,7 @@ class Docker(object):
>>          return labels.get("com.qemu.dockerfile-checksum", "")
>>
>>      def build_image(self, tag, docker_dir, dockerfile,
>> -                    quiet=True, user=False, argv=None):
>> +                    quiet=True, user=False, argv=None, extra_files_cksum=[]):
>>          if argv == None:
>>              argv = []
>>
>> @@ -170,7 +173,7 @@ class Docker(object):
>>
>>          tmp_df.write("\n")
>>          tmp_df.write("LABEL com.qemu.dockerfile-checksum=%s" %
>> -                     _text_checksum(dockerfile))
>> +                     _text_checksum(dockerfile + "-".join(extra_files_cksum)))
>>          tmp_df.flush()
>>
>>          self._do(["build", "-t", tag, "-f", tmp_df.name] + argv + \
>> @@ -276,16 +279,22 @@ class BuildCommand(SubCommand):
>>                      return 1
>>
>>              # Include files used by ADD directives found within the Dockerfile.
>> +            cksum = []
>>              if args.include_executable:
>> +                # FIXME: there is no checksum of this executable and the linked
>> +                # libraries, once the image built any change of this executable
>> +                # or any library won't trigger another build.
>>                  _copy_binary_with_libs(args.include_executable, docker_dir)
>>              for filename in args.include_files or []:
>>                  _copy_with_mkdir(filename, docker_dir)
>> +                cksum += [_file_checksum(filename)]
>>
>>              argv += ["--build-arg=" + k.lower() + "=" + v
>>                          for k, v in os.environ.iteritems()
>>                          if k.lower() in FILTERED_ENV_NAMES]
>>              dkr.build_image(tag, docker_dir, dockerfile,
>> -                            quiet=args.quiet, user=args.user, argv=argv)
>> +                            quiet=args.quiet, user=args.user, argv=argv,
>> +                            extra_files_cksum=cksum)
>>
>>              rmtree(docker_dir)
>
>
> --
> Alex Bennée
>
Alex Bennée May 22, 2017, 1:39 p.m. UTC | #3
Philippe Mathieu-Daudé <f4bug@amsat.org> writes:

> Hi Alex,
>
> On 05/22/2017 04:55 AM, Alex Bennée wrote:
>>
>> Philippe Mathieu-Daudé <f4bug@amsat.org> writes:
>>
>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> ---
>>>  tests/docker/docker.py | 15 ++++++++++++---
>>>  1 file changed, 12 insertions(+), 3 deletions(-)
>>
>> Hmm odd what bracnh did you cut this from. git apply-mbox keeps falling
>> over:
>>
>> Applying: docker: rebuild image if 'extra files' checksum does not match
>> error: sha1 information is lacking or useless (tests/docker/docker.py).
>> error: could not build fake ancestor
>> Patch failed at 0001 docker: rebuild image if 'extra files' checksum does not match
>> The copy of the patch that failed is found in: .git/rebase-apply/patch
>> When you have resolved this problem, run "git am --continue".
>> If you prefer to skip this patch, run "git am --skip" instead.
>> To restore the original branch and stop patching, run "git am --abort".
>
> Weird...
>
> I still have the command in my history, I used:
>   $ git format-patch master --cover-letter -v 3 -n -s -o
> outgoing/shippable_v3 --to qemu-devel@nongnu.org --to 'Alex Bennée
> <alex.bennee@linaro.org>' --to 'Fam Zheng <famz@redhat.com>'
>
> Having:
>   $ git rev-parse master
> 56821559f0ba682fe6b367815572e6f974d329ab
>
> You can also fetch this branch here:
>   https://github.com/philmd/qemu/tree/docker_shippable_v3

Weird indeed. However my manually applied tree matches yours so I
haven't missed anything, *phew* ;-)

> Shippable output:
>   https://app.shippable.com/github/philmd/qemu/status/dashboard
>
> Regards,
>
> Phil.
>
>>
>>
>>>
>>> diff --git a/tests/docker/docker.py b/tests/docker/docker.py
>>> index eef92a54f1..0dd6a3304f 100755
>>> --- a/tests/docker/docker.py
>>> +++ b/tests/docker/docker.py
>>> @@ -38,6 +38,9 @@ def _text_checksum(text):
>>>      """Calculate a digest string unique to the text content"""
>>>      return hashlib.sha1(text).hexdigest()
>>>
>>> +def _file_checksum(filename):
>>> +    return _text_checksum(open(filename, 'rb').read())
>>> +
>>>  def _guess_docker_command():
>>>      """ Guess a working docker command or raise exception if not found"""
>>>      commands = [["docker"], ["sudo", "-n", "docker"]]
>>> @@ -154,7 +157,7 @@ class Docker(object):
>>>          return labels.get("com.qemu.dockerfile-checksum", "")
>>>
>>>      def build_image(self, tag, docker_dir, dockerfile,
>>> -                    quiet=True, user=False, argv=None):
>>> +                    quiet=True, user=False, argv=None, extra_files_cksum=[]):
>>>          if argv == None:
>>>              argv = []
>>>
>>> @@ -170,7 +173,7 @@ class Docker(object):
>>>
>>>          tmp_df.write("\n")
>>>          tmp_df.write("LABEL com.qemu.dockerfile-checksum=%s" %
>>> -                     _text_checksum(dockerfile))
>>> +                     _text_checksum(dockerfile + "-".join(extra_files_cksum)))
>>>          tmp_df.flush()
>>>
>>>          self._do(["build", "-t", tag, "-f", tmp_df.name] + argv + \
>>> @@ -276,16 +279,22 @@ class BuildCommand(SubCommand):
>>>                      return 1
>>>
>>>              # Include files used by ADD directives found within the Dockerfile.
>>> +            cksum = []
>>>              if args.include_executable:
>>> +                # FIXME: there is no checksum of this executable and the linked
>>> +                # libraries, once the image built any change of this executable
>>> +                # or any library won't trigger another build.
>>>                  _copy_binary_with_libs(args.include_executable, docker_dir)
>>>              for filename in args.include_files or []:
>>>                  _copy_with_mkdir(filename, docker_dir)
>>> +                cksum += [_file_checksum(filename)]
>>>
>>>              argv += ["--build-arg=" + k.lower() + "=" + v
>>>                          for k, v in os.environ.iteritems()
>>>                          if k.lower() in FILTERED_ENV_NAMES]
>>>              dkr.build_image(tag, docker_dir, dockerfile,
>>> -                            quiet=args.quiet, user=args.user, argv=argv)
>>> +                            quiet=args.quiet, user=args.user, argv=argv,
>>> +                            extra_files_cksum=cksum)
>>>
>>>              rmtree(docker_dir)
>>
>>
>> --
>> Alex Bennée
>>


--
Alex Bennée
Fam Zheng May 24, 2017, 5:24 a.m. UTC | #4
On Sun, 05/21 00:29, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  tests/docker/docker.py | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/docker/docker.py b/tests/docker/docker.py
> index eef92a54f1..0dd6a3304f 100755
> --- a/tests/docker/docker.py
> +++ b/tests/docker/docker.py
> @@ -38,6 +38,9 @@ def _text_checksum(text):
>      """Calculate a digest string unique to the text content"""
>      return hashlib.sha1(text).hexdigest()
>  
> +def _file_checksum(filename):
> +    return _text_checksum(open(filename, 'rb').read())
> +
>  def _guess_docker_command():
>      """ Guess a working docker command or raise exception if not found"""
>      commands = [["docker"], ["sudo", "-n", "docker"]]
> @@ -154,7 +157,7 @@ class Docker(object):
>          return labels.get("com.qemu.dockerfile-checksum", "")
>  
>      def build_image(self, tag, docker_dir, dockerfile,
> -                    quiet=True, user=False, argv=None):
> +                    quiet=True, user=False, argv=None, extra_files_cksum=[]):
>          if argv == None:
>              argv = []
>  
> @@ -170,7 +173,7 @@ class Docker(object):
>  
>          tmp_df.write("\n")
>          tmp_df.write("LABEL com.qemu.dockerfile-checksum=%s" %
> -                     _text_checksum(dockerfile))
> +                     _text_checksum(dockerfile + "-".join(extra_files_cksum)))

Since @dockerfile is the content of the the Dockerfile, how about concatenating
with "\n":

                        _text_checksum("\n".join([dockerfile] + extra_files_cksum))

?


>          tmp_df.flush()
>  
>          self._do(["build", "-t", tag, "-f", tmp_df.name] + argv + \
> @@ -276,16 +279,22 @@ class BuildCommand(SubCommand):
>                      return 1
>  
>              # Include files used by ADD directives found within the Dockerfile.
> +            cksum = []
>              if args.include_executable:
> +                # FIXME: there is no checksum of this executable and the linked
> +                # libraries, once the image built any change of this executable
> +                # or any library won't trigger another build.
>                  _copy_binary_with_libs(args.include_executable, docker_dir)
>              for filename in args.include_files or []:
>                  _copy_with_mkdir(filename, docker_dir)
> +                cksum += [_file_checksum(filename)]
>  
>              argv += ["--build-arg=" + k.lower() + "=" + v
>                          for k, v in os.environ.iteritems()
>                          if k.lower() in FILTERED_ENV_NAMES]
>              dkr.build_image(tag, docker_dir, dockerfile,
> -                            quiet=args.quiet, user=args.user, argv=argv)
> +                            quiet=args.quiet, user=args.user, argv=argv,
> +                            extra_files_cksum=cksum)
>  
>              rmtree(docker_dir)
>  
> -- 
> 2.11.0
>
Philippe Mathieu-Daudé May 26, 2017, 11:17 p.m. UTC | #5
On 05/24/2017 02:24 AM, Fam Zheng wrote:
> On Sun, 05/21 00:29, Philippe Mathieu-Daudé wrote:
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  tests/docker/docker.py | 15 ++++++++++++---
>>  1 file changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/tests/docker/docker.py b/tests/docker/docker.py
>> index eef92a54f1..0dd6a3304f 100755
>> --- a/tests/docker/docker.py
>> +++ b/tests/docker/docker.py
>> @@ -38,6 +38,9 @@ def _text_checksum(text):
>>      """Calculate a digest string unique to the text content"""
>>      return hashlib.sha1(text).hexdigest()
>>
>> +def _file_checksum(filename):
>> +    return _text_checksum(open(filename, 'rb').read())
>> +
>>  def _guess_docker_command():
>>      """ Guess a working docker command or raise exception if not found"""
>>      commands = [["docker"], ["sudo", "-n", "docker"]]
>> @@ -154,7 +157,7 @@ class Docker(object):
>>          return labels.get("com.qemu.dockerfile-checksum", "")
>>
>>      def build_image(self, tag, docker_dir, dockerfile,
>> -                    quiet=True, user=False, argv=None):
>> +                    quiet=True, user=False, argv=None, extra_files_cksum=[]):
>>          if argv == None:
>>              argv = []
>>
>> @@ -170,7 +173,7 @@ class Docker(object):
>>
>>          tmp_df.write("\n")
>>          tmp_df.write("LABEL com.qemu.dockerfile-checksum=%s" %
>> -                     _text_checksum(dockerfile))
>> +                     _text_checksum(dockerfile + "-".join(extra_files_cksum)))
>
> Since @dockerfile is the content of the the Dockerfile, how about concatenating
> with "\n":
>
>                         _text_checksum("\n".join([dockerfile] + extra_files_cksum))
>
> ?

This is only the text to hash, I don't think it will make a difference 
since it isn't useful even debugging the script, but it may be cleaner 
so I'll do it (nonetheless python ends up weirdly indented).

>
>
>>          tmp_df.flush()
>>
>>          self._do(["build", "-t", tag, "-f", tmp_df.name] + argv + \
>> @@ -276,16 +279,22 @@ class BuildCommand(SubCommand):
>>                      return 1
>>
>>              # Include files used by ADD directives found within the Dockerfile.
>> +            cksum = []
>>              if args.include_executable:
>> +                # FIXME: there is no checksum of this executable and the linked
>> +                # libraries, once the image built any change of this executable
>> +                # or any library won't trigger another build.
>>                  _copy_binary_with_libs(args.include_executable, docker_dir)
>>              for filename in args.include_files or []:
>>                  _copy_with_mkdir(filename, docker_dir)
>> +                cksum += [_file_checksum(filename)]
>>
>>              argv += ["--build-arg=" + k.lower() + "=" + v
>>                          for k, v in os.environ.iteritems()
>>                          if k.lower() in FILTERED_ENV_NAMES]
>>              dkr.build_image(tag, docker_dir, dockerfile,
>> -                            quiet=args.quiet, user=args.user, argv=argv)
>> +                            quiet=args.quiet, user=args.user, argv=argv,
>> +                            extra_files_cksum=cksum)
>>
>>              rmtree(docker_dir)
>>
>> --
>> 2.11.0
>>
diff mbox

Patch

diff --git a/tests/docker/docker.py b/tests/docker/docker.py
index eef92a54f1..0dd6a3304f 100755
--- a/tests/docker/docker.py
+++ b/tests/docker/docker.py
@@ -38,6 +38,9 @@  def _text_checksum(text):
     """Calculate a digest string unique to the text content"""
     return hashlib.sha1(text).hexdigest()
 
+def _file_checksum(filename):
+    return _text_checksum(open(filename, 'rb').read())
+
 def _guess_docker_command():
     """ Guess a working docker command or raise exception if not found"""
     commands = [["docker"], ["sudo", "-n", "docker"]]
@@ -154,7 +157,7 @@  class Docker(object):
         return labels.get("com.qemu.dockerfile-checksum", "")
 
     def build_image(self, tag, docker_dir, dockerfile,
-                    quiet=True, user=False, argv=None):
+                    quiet=True, user=False, argv=None, extra_files_cksum=[]):
         if argv == None:
             argv = []
 
@@ -170,7 +173,7 @@  class Docker(object):
 
         tmp_df.write("\n")
         tmp_df.write("LABEL com.qemu.dockerfile-checksum=%s" %
-                     _text_checksum(dockerfile))
+                     _text_checksum(dockerfile + "-".join(extra_files_cksum)))
         tmp_df.flush()
 
         self._do(["build", "-t", tag, "-f", tmp_df.name] + argv + \
@@ -276,16 +279,22 @@  class BuildCommand(SubCommand):
                     return 1
 
             # Include files used by ADD directives found within the Dockerfile.
+            cksum = []
             if args.include_executable:
+                # FIXME: there is no checksum of this executable and the linked
+                # libraries, once the image built any change of this executable
+                # or any library won't trigger another build.
                 _copy_binary_with_libs(args.include_executable, docker_dir)
             for filename in args.include_files or []:
                 _copy_with_mkdir(filename, docker_dir)
+                cksum += [_file_checksum(filename)]
 
             argv += ["--build-arg=" + k.lower() + "=" + v
                         for k, v in os.environ.iteritems()
                         if k.lower() in FILTERED_ENV_NAMES]
             dkr.build_image(tag, docker_dir, dockerfile,
-                            quiet=args.quiet, user=args.user, argv=argv)
+                            quiet=args.quiet, user=args.user, argv=argv,
+                            extra_files_cksum=cksum)
 
             rmtree(docker_dir)