| Message ID | 20220727163632.59806-2-lucas.araujo@eldorado.org.br |
|---|---|
| State | New |
| Headers | show |
| Series | Patch series to set up a ppc64le CI | expand |
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
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,
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
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 --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 \
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(+)