On 2020-04-10 17:53:22+0200, SZEDER Gábor <szeder.dev@xxxxxxxxx> wrote: > On Wed, Apr 08, 2020 at 11:05:36AM +0700, Đoàn Trần Công Danh wrote: > > In a later patch, we will support GitHub Action. > > > > Explicitly install all of our build dependencies. > > ... on Linux. This patch doesn't touch the parts installing > dependencies in the osx jobs, but we do rely on some packages being > installed by default in the osx images we use. This is worth > clarifying in the commit message, and in its subject line. Fair enough. > > Since GitHub Action VM hasn't install our build dependencies, yet. > > s/install/installed/ > > +UBUNTU_COMMON_PKGS="make libssl-dev libcurl4-openssl-dev libexpat-dev > > + perl-modules liberror-perl tcl tk gettext zlib1g-dev apache2 > > + libauthen-sasl-perl libemail-valid-perl libio-socket-ssl-perl > > + libnet-smtp-ssl-perl" > > I note that this list includes 'make' and 'apache2'. I'll remove apache2. > > case "$jobname" in > > linux-clang|linux-gcc) > > sudo apt-add-repository -y "ppa:ubuntu-toolchain-r/test" > > sudo apt-get -q update > > - sudo apt-get -q -y install language-pack-is libsvn-perl apache2 > > + sudo apt-get -q -y install language-pack-is libsvn-perl apache2 \ > > 'apache2' is listed here again. > > > + $UBUNTU_COMMON_PKGS > > case "$jobname" in > > linux-gcc) > > sudo apt-get -q -y install gcc-8 > > @@ -63,11 +68,16 @@ StaticAnalysis) > > ;; > > Documentation) > > sudo apt-get -q update > > - sudo apt-get -q -y install asciidoc xmlto docbook-xsl-ns > > + sudo apt-get -q -y install asciidoc xmlto docbook-xsl-ns \ > > + libcurl4-openssl-dev > > Does the Documentation job really need the 'libcurl4-openssl-dev' > package? FWIW, I just removed this package from my system, and 'make > doc' still succeeded. At the time of writing this series. pu requires `curl-config` for Documentation jobs. Skimming over the mail archive, Peff has sent a patch to fix it. I haven't checked again, though. > Furthermore, this doesn't install 'make', though in other jobs it is > installed explicitly. Note that the StaticAnalysis job requires > 'make' as well. I copied them from Azure Pipelines declaration. I think it's better to list make explicitly in all jobs. > Also note that we have a 'linux-gcc-4.8' job as well... Will do in the re-roll. > > +GETTEXT_POISON) > > + sudo apt-get -q update > > + sudo apt-get -q -y install $UBUNTU_COMMON_PKGS > > The GETTEXT_POISON job currently doesn't install 'apache2', but with > this change it will. If this change is intentional, then please > justify it in the commit message. But I think that we shouldn't > include 'apache2' in $UBUNTU_COMMON_PKGS. No, this patch shouldn't change it. I will remove apache2 from $UBUNTU_COMMON_PKGS -- Danh
