| Message ID | 20221124090545.4790-4-worldhello.net@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Use fixed github-actions runner image | expand |
On Thu, Nov 24 2022, Jiang Xin wrote: > From: Jiang Xin <zhiyou.jx@alibaba-inc.com> > > Python is missing from the default ubuntu-22.04 runner image, which > prevent git-p4 from working. To install python on ubuntu, we need to > provide correct package name: > > * On Ubuntu 18.04 (bionic), "/usr/bin/python2" is provided by the > "python" package, and "/usr/bin/python3" is provided by the "python3" > package. > > * On Ubuntu 20.04 (focal) and above, "/usr/bin/python2" is provided by > the "python2" package which has a different name from bionic, and > "/usr/bin/python3" is provided by "python3". > > Since the "ubuntu-latest" runner image has a higher version, so its safe > to use "python2" or "python3" package name. > > Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com> > --- > ci/install-dependencies.sh | 2 +- > ci/lib.sh | 2 ++ > 2 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh > index 291e49bdde..e28d93a154 100755 > --- a/ci/install-dependencies.sh > +++ b/ci/install-dependencies.sh > @@ -15,7 +15,7 @@ case "$runs_on_os" in > ubuntu) > sudo apt-get -q update > sudo apt-get -q -y install language-pack-is libsvn-perl apache2 \ > - $UBUNTU_COMMON_PKGS $CC_PACKAGE > + $UBUNTU_COMMON_PKGS $CC_PACKAGE $PYTHON_PACKAGE > mkdir --parents "$P4_PATH" > pushd "$P4_PATH" > wget --quiet "$P4WHENCE/bin.linux26x86_64/p4d" > diff --git a/ci/lib.sh b/ci/lib.sh > index a618d66b81..ebe702e0ea 100755 > --- a/ci/lib.sh > +++ b/ci/lib.sh > @@ -235,8 +235,10 @@ ubuntu) > if [ "$jobname" = linux-gcc ] > then > MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/python3" > + PYTHON_PACKAGE=python3 > else > MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/python2" > + PYTHON_PACKAGE=python2 > fi Let's not copy/paste and repeat ourselves here for no reason. Part of this is pre-existing, but if you just re-arrange these variable decls you can do this instead: PYTHON_PACKAGE=python2 if test "$jobname" = linux-gcc then PYTHON_PACKAGE=python3 fi MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/${PYTHON_PACKAGE}" Even if you don't factor out the "else" like that (which I think would be OK to do while-we'er-at-it) this should be changed so that the "PYTHON_PACKAGE" comes before "MAKEFLAGS" in the two if/else branches here, so we can then use the variable.
On Thu, Nov 24, 2022 at 7:06 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > > > On Thu, Nov 24 2022, Jiang Xin wrote: > > > From: Jiang Xin <zhiyou.jx@alibaba-inc.com> > > > > Python is missing from the default ubuntu-22.04 runner image, which > > prevent git-p4 from working. To install python on ubuntu, we need to > > provide correct package name: > > > > * On Ubuntu 18.04 (bionic), "/usr/bin/python2" is provided by the > > "python" package, and "/usr/bin/python3" is provided by the "python3" > > package. > > > > * On Ubuntu 20.04 (focal) and above, "/usr/bin/python2" is provided by > > the "python2" package which has a different name from bionic, and > > "/usr/bin/python3" is provided by "python3". > > > > Since the "ubuntu-latest" runner image has a higher version, so its safe > > to use "python2" or "python3" package name. > > > > Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com> > > --- > > ci/install-dependencies.sh | 2 +- > > ci/lib.sh | 2 ++ > > 2 files changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh > > index 291e49bdde..e28d93a154 100755 > > --- a/ci/install-dependencies.sh > > +++ b/ci/install-dependencies.sh > > @@ -15,7 +15,7 @@ case "$runs_on_os" in > > ubuntu) > > sudo apt-get -q update > > sudo apt-get -q -y install language-pack-is libsvn-perl apache2 \ > > - $UBUNTU_COMMON_PKGS $CC_PACKAGE > > + $UBUNTU_COMMON_PKGS $CC_PACKAGE $PYTHON_PACKAGE > > mkdir --parents "$P4_PATH" > > pushd "$P4_PATH" > > wget --quiet "$P4WHENCE/bin.linux26x86_64/p4d" > > diff --git a/ci/lib.sh b/ci/lib.sh > > index a618d66b81..ebe702e0ea 100755 > > --- a/ci/lib.sh > > +++ b/ci/lib.sh > > @@ -235,8 +235,10 @@ ubuntu) > > if [ "$jobname" = linux-gcc ] > > then > > MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/python3" > > + PYTHON_PACKAGE=python3 > > else > > MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/python2" > > + PYTHON_PACKAGE=python2 > > fi > > Let's not copy/paste and repeat ourselves here for no reason. Part of > this is pre-existing, but if you just re-arrange these variable decls > you can do this instead: > > PYTHON_PACKAGE=python2 > if test "$jobname" = linux-gcc > then > PYTHON_PACKAGE=python3 > fi > MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/${PYTHON_PACKAGE}" That was exactly my first edition, but I thought it was weird to write as "/usr/bin/${PYTHON_PACKAGE}". But if use two variables like PYTHON_BINARY and PYTHON_PACKAGE, looks even more silly. So I choose current solution. -- Jiang Xin
On Thu, Nov 24 2022, Jiang Xin wrote: > On Thu, Nov 24, 2022 at 7:06 PM Ævar Arnfjörð Bjarmason > <avarab@gmail.com> wrote: >> >> >> On Thu, Nov 24 2022, Jiang Xin wrote: >> >> > From: Jiang Xin <zhiyou.jx@alibaba-inc.com> >> > >> > Python is missing from the default ubuntu-22.04 runner image, which >> > prevent git-p4 from working. To install python on ubuntu, we need to >> > provide correct package name: >> > >> > * On Ubuntu 18.04 (bionic), "/usr/bin/python2" is provided by the >> > "python" package, and "/usr/bin/python3" is provided by the "python3" >> > package. >> > >> > * On Ubuntu 20.04 (focal) and above, "/usr/bin/python2" is provided by >> > the "python2" package which has a different name from bionic, and >> > "/usr/bin/python3" is provided by "python3". >> > >> > Since the "ubuntu-latest" runner image has a higher version, so its safe >> > to use "python2" or "python3" package name. >> > >> > Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com> >> > --- >> > ci/install-dependencies.sh | 2 +- >> > ci/lib.sh | 2 ++ >> > 2 files changed, 3 insertions(+), 1 deletion(-) >> > >> > diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh >> > index 291e49bdde..e28d93a154 100755 >> > --- a/ci/install-dependencies.sh >> > +++ b/ci/install-dependencies.sh >> > @@ -15,7 +15,7 @@ case "$runs_on_os" in >> > ubuntu) >> > sudo apt-get -q update >> > sudo apt-get -q -y install language-pack-is libsvn-perl apache2 \ >> > - $UBUNTU_COMMON_PKGS $CC_PACKAGE >> > + $UBUNTU_COMMON_PKGS $CC_PACKAGE $PYTHON_PACKAGE >> > mkdir --parents "$P4_PATH" >> > pushd "$P4_PATH" >> > wget --quiet "$P4WHENCE/bin.linux26x86_64/p4d" >> > diff --git a/ci/lib.sh b/ci/lib.sh >> > index a618d66b81..ebe702e0ea 100755 >> > --- a/ci/lib.sh >> > +++ b/ci/lib.sh >> > @@ -235,8 +235,10 @@ ubuntu) >> > if [ "$jobname" = linux-gcc ] >> > then >> > MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/python3" >> > + PYTHON_PACKAGE=python3 >> > else >> > MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/python2" >> > + PYTHON_PACKAGE=python2 >> > fi >> >> Let's not copy/paste and repeat ourselves here for no reason. Part of >> this is pre-existing, but if you just re-arrange these variable decls >> you can do this instead: >> >> PYTHON_PACKAGE=python2 >> if test "$jobname" = linux-gcc >> then >> PYTHON_PACKAGE=python3 >> fi >> MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/${PYTHON_PACKAGE}" > > That was exactly my first edition, but I thought it was weird to write > as "/usr/bin/${PYTHON_PACKAGE}". But if use two variables like > PYTHON_BINARY and PYTHON_PACKAGE, looks even more silly. So I choose > current solution. I don't mind if you go for your inital version, it's not much duplication, but why does it look silly? I don't think we need to worry that the <package-name> on Ubuntu (and Debian) won't have a 1=1 mapping to the /usr/bin/<package-name>. So defining the path in terms of the package name seems like an obvious thing to do.
diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh index 291e49bdde..e28d93a154 100755 --- a/ci/install-dependencies.sh +++ b/ci/install-dependencies.sh @@ -15,7 +15,7 @@ case "$runs_on_os" in ubuntu) sudo apt-get -q update sudo apt-get -q -y install language-pack-is libsvn-perl apache2 \ - $UBUNTU_COMMON_PKGS $CC_PACKAGE + $UBUNTU_COMMON_PKGS $CC_PACKAGE $PYTHON_PACKAGE mkdir --parents "$P4_PATH" pushd "$P4_PATH" wget --quiet "$P4WHENCE/bin.linux26x86_64/p4d" diff --git a/ci/lib.sh b/ci/lib.sh index a618d66b81..ebe702e0ea 100755 --- a/ci/lib.sh +++ b/ci/lib.sh @@ -235,8 +235,10 @@ ubuntu) if [ "$jobname" = linux-gcc ] then MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/python3" + PYTHON_PACKAGE=python3 else MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/python2" + PYTHON_PACKAGE=python2 fi export GIT_TEST_HTTPD=true