| Message ID | 4f80724641e17bf0d1937dbad77f987fbf86cd64.1586309211.git.congdanhqx@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ci: replace our Azure Pipeline by GitHub Actions | expand |
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. > Since GitHub Action VM hasn't install our build dependencies, yet. s/install/installed/ I'm not sure what you mean with "yet". > And there're no harm to reinstall them (in Travis) > Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> > --- > ci/install-dependencies.sh | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh > index 497fd32ca8..371902bb75 100755 > --- a/ci/install-dependencies.sh > +++ b/ci/install-dependencies.sh > @@ -7,12 +7,17 @@ > > P4WHENCE=http://filehost.perforce.com/perforce/r$LINUX_P4_VERSION > LFSWHENCE=https://github.com/github/git-lfs/releases/download/v$LINUX_GIT_LFS_VERSION > +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'. > 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. Furthermore, this doesn't install 'make', though in other jobs it is installed explicitly. Note that the StaticAnalysis job requires 'make' as well. Also note that we have a 'linux-gcc-4.8' job as well... > > test -n "$ALREADY_HAVE_ASCIIDOCTOR" || > gem install --version 1.5.8 asciidoctor > ;; > +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. > + ;; > esac > > if type p4d >/dev/null && type p4 >/dev/null > -- > 2.26.0.334.g6536db25bb >
On 2020-04-10 17:53:22+0200, SZEDER Gábor <szeder.dev@gmail.com> 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 Doan <congdanhqx@gmail.com> writes: > On 2020-04-10 17:53:22+0200, SZEDER Gábor <szeder.dev@gmail.com> wrote: > ... >> 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. > ... > No, this patch shouldn't change it. > I will remove apache2 from $UBUNTU_COMMON_PKGS Thanks both for working together to trim excess bits and add back missing bits to polish it.
diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh index 497fd32ca8..371902bb75 100755 --- a/ci/install-dependencies.sh +++ b/ci/install-dependencies.sh @@ -7,12 +7,17 @@ P4WHENCE=http://filehost.perforce.com/perforce/r$LINUX_P4_VERSION LFSWHENCE=https://github.com/github/git-lfs/releases/download/v$LINUX_GIT_LFS_VERSION +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" 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 \ + $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 test -n "$ALREADY_HAVE_ASCIIDOCTOR" || gem install --version 1.5.8 asciidoctor ;; +GETTEXT_POISON) + sudo apt-get -q update + sudo apt-get -q -y install $UBUNTU_COMMON_PKGS + ;; esac if type p4d >/dev/null && type p4 >/dev/null
In a later patch, we will support GitHub Action. Explicitly install all of our build dependencies. Since GitHub Action VM hasn't install our build dependencies, yet. And there're no harm to reinstall them (in Travis) Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> --- ci/install-dependencies.sh | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-)