Reduce image size, upgrade to Clang 10 #2
Conversation
GCC 6, 7, and Clang 5, 6, 7 are no longer needed since https://code.wireshark.org/review/36281 ("gitlab-ci: remove unnecessary jobs, upgrade versions"). Install Clang 10 as it is about to reach stable very soon. This will require another Wireshark .gitlab-ci.yaml change once the image is published. This reduces the image size from 2.76G to 1.75G. Replace apt by apt-get since 'apt' does not have a stable interface.
Reduces the Docker image size from 1.75G to 1.37G.
Requires an updated wireshark/wireshark-ubuntu-dev image: wireshark/wireshark-ubuntu-dev-docker#2 Change-Id: Iee869a44cede6152a2f7927735a74b0bb89bef60 Reviewed-on: https://code.wireshark.org/review/36283 Reviewed-by: Anders Broman <a.broman58@gmail.com>
|
Do not merge yet, I plan to make two more changes which still need to be tested:
Ideally the image is built with a snapshot of the final Clang 10 release as opposed to rc3. The final Clang 10 release hopefully happens this or next week. |
|
Before merging, I'd consider using tags in the docker hub repo. With a simple versioning scheme (1, 2, 3...) we could be retro-compatible, by releasing a new tag. |
|
I've tagged the current master, and configured docker hub to create automatically tags according to a tagging scheme such as vX. If we want to take this way, we should update the gitlab-ci file to use v1, then merge this (will create a new "latest" build), then create the tag "v2" (will create the tag v2 on docker hub) then update gitlab-ci to use the new tag. |
|
Unfortunately the docker image failed to complete. Investigation needed. |
|
The docker image most likely failed to build because apt.llvm.org requires HTTPS support now, something that was also fixed in this PR. Instead of tag such as v1, what about yyyy.mm.dd? That makes it quite clear what version the images are based on. Clang 10.0.0-rc4 was just tagged and will become the final 10 release if no problems are reported. This version has already been published in the apt repo: https://apt.llvm.org/bionic/pool/main/l/llvm-toolchain-10/ |
Master has been fixed in pr #3 .
That does't really sound clearer to me. A tag such as yyyy.mm.dd sounds weird, instead. |
|
Looks like I forgot to push the https fix, I just did that together with upgrading GCC. This was tested a few days ago. A date-based format is not that uncommon. We use yyyy.mm.r in our company where r is the revision number, allowing multiple updates a day. You can also see that for the official Docker image for example: https://hub.docker.com/_/docker |
|
I've reconfigured docker hub to automatically create tagged images for any tag name used here. Feel free to use the tagging scheme you prefer the most. |
|
Based on the description, it seems that a tag and branch would have to be pushed at once to ensure that "latest" corresponds to the same image as the tagged version, is that right?
Based on this, I would guess that Should we continue using tags, or drop it and use image digests instead? |
|
Ubuntu 18.04 ships with gcc-8 by default, the Fedora image already tests with GCC 9.2.1. I am tempted to drop the additional PPA dependency. |
Lekensteyn
force-pushed
the
pwu/upgrade-clang
branch
from
3eb958d
to
c07da6c
Compare
GCC 5 is no longer needed since Wireshark v3.3.0rc0-716-g0ec5ca3ecf. gcc-7 is included with build-essential on Ubuntu 18.04, save 110M by not installing GCC 8. Current compiler versions included with this image: * gcc-7 7.5.0-3ubuntu1~18.04 (via build-essential), * clang-10 1:10~++20200323042644+d32170dbd5b-1~exp1~20200323154014.129 (from the apt.llvm.org repo, this is the final LLVM 10.0.0 tag). Based on debian-setup.sh from wireshark v3.3.0rc0-842-g8baf0fd295, the local image size is 1.06G.
Lekensteyn
force-pushed
the
pwu/upgrade-clang
branch
from
c07da6c
to
23a8892
Compare
|
Squashed the last two patches, the newly image built from this change will have Clang 10.0.0 (final) which was tagged yesterday. |
Requires an updated wireshark/wireshark-ubuntu-dev image: wireshark/wireshark-ubuntu-dev-docker#2 Remove -Wframe-larger-than while at it. The default size in our CMake config is 32k. Clang should not significantly go over it. If so, then it has to be solved there, and not in the Gitlab config. Change-Id: I3891fcbd9dec8e5a4597404aa8131f28a1755a02 Reviewed-on: https://code.wireshark.org/review/36369 Reviewed-by: Peter Wu <peter@lekensteyn.nl>
Requires an updated wireshark/wireshark-ubuntu-dev image: wireshark/wireshark-ubuntu-dev-docker#2 Remove -Wframe-larger-than while at it. The default size in our CMake config is 32k. Clang should not significantly go over it. If so, then it has to be solved there, and not in the Gitlab config. Change-Id: I3891fcbd9dec8e5a4597404aa8131f28a1755a02 Reviewed-on: https://code.wireshark.org/review/36369 Reviewed-by: Peter Wu <peter@lekensteyn.nl> (cherry picked from commit 10b88d0d4474ab7b0006939d12fa7ea199d47ff5) Reviewed-on: https://code.wireshark.org/review/36569
Requires an updated wireshark/wireshark-ubuntu-dev image: wireshark/wireshark-ubuntu-dev-docker#2 Remove -Wframe-larger-than while at it. The default size in our CMake config is 32k. Clang should not significantly go over it. If so, then it has to be solved there, and not in the Gitlab config. Change-Id: I3891fcbd9dec8e5a4597404aa8131f28a1755a02 Reviewed-on: https://code.wireshark.org/review/36369 Reviewed-by: Peter Wu <peter@lekensteyn.nl> (cherry picked from commit 7579d98) Reviewed-on: https://code.wireshark.org/review/36570
|
The latest image is 349.55MB when compressed I cannot find info for the previous image, but it was 700M or 800M compressed if I remember correctly. |
Changes:
Remove old compilers, add Clang 10
GCC 6, 7, and Clang 5, 6, 7 are no longer needed since
https://code.wireshark.org/review/36281
("gitlab-ci: remove unnecessary jobs, upgrade versions").
Install Clang 10 as it is about to reach stable very soon. This will
require another Wireshark .gitlab-ci.yaml change once the image is
published.
This reduces the image size from 2.76G to 1.75G.
Replace apt by apt-get since 'apt' does not have a stable interface.
Do not install recommended packages
Reduces the Docker image size from 1.75G to 1.37G.
Note: I tested this image locally and was able to build a Debian package and run pytest.