diff mbox series

[1/8] tests/docker: Fix alpine dockerfile

Message ID 20220727163632.59806-2-lucas.araujo@eldorado.org.br (mailing list archive)
State New, archived
Headers show
Series Patch series to set up a ppc64le CI | expand

Commit Message

Lucas Mateus Martins Araujo e Castro July 27, 2022, 4:36 p.m. UTC
Currently the run script uses 'readlink -e' but the image only has the
busybox readlink, this commit add the coreutils package which
contains the readlink with the '-e' option.

Signed-off-by: Lucas Mateus Castro(alqotel) <lucas.araujo@eldorado.org.br>
---
 tests/docker/dockerfiles/alpine.docker | 1 +
 1 file changed, 1 insertion(+)

Comments

Daniel P. Berrangé July 27, 2022, 5:09 p.m. UTC | #1
On Wed, Jul 27, 2022 at 01:36:25PM -0300, Lucas Mateus Castro(alqotel) wrote:
> Currently the run script uses 'readlink -e' but the image only has the
> busybox readlink, this commit add the coreutils package which
> contains the readlink with the '-e' option.

Use of 'readlink' is discouraged in favour of 'realpath'. AFAICT, we
can just do that change and not need the '-e' flag anyway.

> 
> Signed-off-by: Lucas Mateus Castro(alqotel) <lucas.araujo@eldorado.org.br>
> ---
>  tests/docker/dockerfiles/alpine.docker | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tests/docker/dockerfiles/alpine.docker b/tests/docker/dockerfiles/alpine.docker
> index 3f4c0f95cb..2943a99730 100644
> --- a/tests/docker/dockerfiles/alpine.docker
> +++ b/tests/docker/dockerfiles/alpine.docker
> @@ -21,6 +21,7 @@ RUN apk update && \
>          cdrkit \
>          ceph-dev \
>          clang \
> +        coreutils \
>          ctags \
>          curl-dev \
>          cyrus-sasl-dev \

This file contents is autogenerated, so editting it manually is
wrong and changes will be lost.


With regards,
Daniel
Lucas Mateus Martins Araujo e Castro July 27, 2022, 5:23 p.m. UTC | #2
On 27/07/2022 14:09, Daniel P. Berrangé wrote:

> On Wed, Jul 27, 2022 at 01:36:25PM -0300, Lucas Mateus Castro(alqotel) wrote:
>> Currently the run script uses 'readlink -e' but the image only has the
>> busybox readlink, this commit add the coreutils package which
>> contains the readlink with the '-e' option.
> Use of 'readlink' is discouraged in favour of 'realpath'. AFAICT, we
> can just do that change and not need the '-e' flag anyway.
So a patch just changing
-BASE="$(dirname $(readlink -e $0))"
+BASE="$(dirname $(realpath $0))"

Ok, I'll send it with v2.

>
>> Signed-off-by: Lucas Mateus Castro(alqotel)<lucas.araujo@eldorado.org.br>
>> ---
>>   tests/docker/dockerfiles/alpine.docker | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/tests/docker/dockerfiles/alpine.docker b/tests/docker/dockerfiles/alpine.docker
>> index 3f4c0f95cb..2943a99730 100644
>> --- a/tests/docker/dockerfiles/alpine.docker
>> +++ b/tests/docker/dockerfiles/alpine.docker
>> @@ -21,6 +21,7 @@ RUN apk update && \
>>           cdrkit \
>>           ceph-dev \
>>           clang \
>> +        coreutils \
>>           ctags \
>>           curl-dev \
>>           cyrus-sasl-dev \
> This file contents is autogenerated, so editting it manually is
> wrong and changes will be lost.

True, that was one of the reasons I had problems with patch 8 (it 
changes the dockerfiles directly) but forgot that it'd apply here as well.

Thanks,
Thomas Huth July 28, 2022, 6:52 a.m. UTC | #3
On 27/07/2022 18.36, Lucas Mateus Castro(alqotel) wrote:
> Currently the run script uses 'readlink -e' but the image only has the
> busybox readlink, this commit add the coreutils package which
> contains the readlink with the '-e' option.
> 
> Signed-off-by: Lucas Mateus Castro(alqotel) <lucas.araujo@eldorado.org.br>
> ---
>   tests/docker/dockerfiles/alpine.docker | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/tests/docker/dockerfiles/alpine.docker b/tests/docker/dockerfiles/alpine.docker
> index 3f4c0f95cb..2943a99730 100644
> --- a/tests/docker/dockerfiles/alpine.docker
> +++ b/tests/docker/dockerfiles/alpine.docker
> @@ -21,6 +21,7 @@ RUN apk update && \
>           cdrkit \
>           ceph-dev \
>           clang \
> +        coreutils \
>           ctags \
>           curl-dev \
>           cyrus-sasl-dev \

Not a good idea. If you look at the top of the file, you can see:

# THIS FILE WAS AUTO-GENERATED

So your modifications will be overwritten the next time someone runs the 
lcitool.

I guess you'd need to modify tests/lcitool/projects/qemu.yml instead? Daniel?

Anyway, it might be better to fix the part that uses "readlink -e" ... Seems 
like busybox' readlink supports -f at least, so maybe it's enough to switch 
to -f instead of -e ?

  Thomas
Thomas Huth July 28, 2022, 7:06 a.m. UTC | #4
On 28/07/2022 08.52, Thomas Huth wrote:
> On 27/07/2022 18.36, Lucas Mateus Castro(alqotel) wrote:
>> Currently the run script uses 'readlink -e' but the image only has the
>> busybox readlink, this commit add the coreutils package which
>> contains the readlink with the '-e' option.
>>
>> Signed-off-by: Lucas Mateus Castro(alqotel) <lucas.araujo@eldorado.org.br>
>> ---
>>   tests/docker/dockerfiles/alpine.docker | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/tests/docker/dockerfiles/alpine.docker 
>> b/tests/docker/dockerfiles/alpine.docker
>> index 3f4c0f95cb..2943a99730 100644
>> --- a/tests/docker/dockerfiles/alpine.docker
>> +++ b/tests/docker/dockerfiles/alpine.docker
>> @@ -21,6 +21,7 @@ RUN apk update && \
>>           cdrkit \
>>           ceph-dev \
>>           clang \
>> +        coreutils \
>>           ctags \
>>           curl-dev \
>>           cyrus-sasl-dev \
> 
> Not a good idea. If you look at the top of the file, you can see:
> 
> # THIS FILE WAS AUTO-GENERATED
> 
> So your modifications will be overwritten the next time someone runs the 
> lcitool.
> 
> I guess you'd need to modify tests/lcitool/projects/qemu.yml instead? Daniel?

Never mind, my e-mail program messed up the threading again, so I did not 
see that Daniel already replied.

  Thomas
diff mbox series

Patch

diff --git a/tests/docker/dockerfiles/alpine.docker b/tests/docker/dockerfiles/alpine.docker
index 3f4c0f95cb..2943a99730 100644
--- a/tests/docker/dockerfiles/alpine.docker
+++ b/tests/docker/dockerfiles/alpine.docker
@@ -21,6 +21,7 @@  RUN apk update && \
         cdrkit \
         ceph-dev \
         clang \
+        coreutils \
         ctags \
         curl-dev \
         cyrus-sasl-dev \