On Thu, Nov 24 2022, Jiang Xin wrote:
> On Thu, Nov 24, 2022 at 7:06 PM Ævar Arnfjörð Bjarmason
> <avarab@xxxxxxxxx> wrote:
>>
>>
>> On Thu, Nov 24 2022, Jiang Xin wrote:
>>
>> > From: Jiang Xin <zhiyou.jx@xxxxxxxxxxxxxxx>
>> >
>> > 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@xxxxxxxxxxxxxxx>
>> > ---
>> > 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.
