[11/11] docker: Don't require rebuilding if unnecessary

Message ID 20180624195557.19909-11-stephen@that.guru
State New
Headers show
Series
  • [01/11] REST: Check.user is not read-only
Related show

Commit Message

Stephen Finucane June 24, 2018, 7:55 p.m.
Now that we're pinning versions, we're going to see more frequent
dependency version changes. Requiring a rebuild after every one of these
is tiresome so don't force it and instead display a helpful message
merely suggesting that a rebuild may be necessary.

Signed-off-by: Stephen Finucane <stephen@that.guru>
Cc: Daniel Axtens <dja@axtens.net>
---
This is a modified version of an earlier patch I submitted to the list.
The original was rejected as there were concerns that it would lead to
some confusion. However, I think the warning it prints is explicit
enough that someone can make their own mind up about this.
---
 tools/docker/entrypoint.sh | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Daniel Axtens June 27, 2018, 4:28 a.m. | #1
Stephen Finucane <stephen@that.guru> writes:

> Now that we're pinning versions, we're going to see more frequent
> dependency version changes. Requiring a rebuild after every one of these
> is tiresome so don't force it and instead display a helpful message
> merely suggesting that a rebuild may be necessary.
>
> Signed-off-by: Stephen Finucane <stephen@that.guru>
> Cc: Daniel Axtens <dja@axtens.net>
> ---
> This is a modified version of an earlier patch I submitted to the list.
> The original was rejected as there were concerns that it would lead to
> some confusion. However, I think the warning it prints is explicit
> enough that someone can make their own mind up about this.

OK, fair enough.

Acked-by: Daniel Axtens <dja@axtens.net>

Regards,
Daniel

> ---
>  tools/docker/entrypoint.sh | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/tools/docker/entrypoint.sh b/tools/docker/entrypoint.sh
> index 997b8763..afa85333 100755
> --- a/tools/docker/entrypoint.sh
> +++ b/tools/docker/entrypoint.sh
> @@ -69,9 +69,10 @@ fi
>  for x in /tmp/requirements-*.txt; do
>      if ! cmp $x ~/patchwork/$(basename $x); then
>          echo "A requirements file has changed."
> -        echo "Please rebuild the patchwork image:"
> +        echo "You may need to rebuild the patchwork image:"
>          echo "    docker-compose build web"
> -        exit 1
> +        echo ""
> +        diff -u $x ~/patchwork/$(basename $x)
>      fi
>  done
>  
> -- 
> 2.17.1
Petr Vorel June 27, 2018, 4:38 a.m. | #2
Hi Stephen,

> +++ b/tools/docker/entrypoint.sh
> @@ -69,9 +69,10 @@ fi
>  for x in /tmp/requirements-*.txt; do
>      if ! cmp $x ~/patchwork/$(basename $x); then
>          echo "A requirements file has changed."
> -        echo "Please rebuild the patchwork image:"
> +        echo "You may need to rebuild the patchwork image:"
>          echo "    docker-compose build web"
> -        exit 1
> +        echo ""
> +        diff -u $x ~/patchwork/$(basename $x)
I'd use 'cat << EOF' instead of many echoes.
But that's just a minor nit, unrelated much to this patch (I see that's the
style written in entrypoint.sh).


Kind regards,
Petr

Patch

diff --git a/tools/docker/entrypoint.sh b/tools/docker/entrypoint.sh
index 997b8763..afa85333 100755
--- a/tools/docker/entrypoint.sh
+++ b/tools/docker/entrypoint.sh
@@ -69,9 +69,10 @@  fi
 for x in /tmp/requirements-*.txt; do
     if ! cmp $x ~/patchwork/$(basename $x); then
         echo "A requirements file has changed."
-        echo "Please rebuild the patchwork image:"
+        echo "You may need to rebuild the patchwork image:"
         echo "    docker-compose build web"
-        exit 1
+        echo ""
+        diff -u $x ~/patchwork/$(basename $x)
     fi
 done