All of lore.kernel.org
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: karthik nayak <karthik.188@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 6/8] t: fix out-of-tree tests for some git-p4 tests
Date: Fri, 13 Dec 2024 06:25:10 +0100	[thread overview]
Message-ID: <Z1vFNpcaRIFBODZk@pks.im> (raw)
In-Reply-To: <CAOLa=ZTgQLgWWsgOCufvkaUDD1RwP+cs8eSAK0XuLXXHiZa_4Q@mail.gmail.com>

On Thu, Dec 12, 2024 at 02:53:47AM -0800, karthik nayak wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> > diff --git a/t/t9835-git-p4-metadata-encoding-python2.sh b/t/t9835-git-p4-metadata-encoding-python2.sh
> > index 036bf79c6674f6f1f0d667c7270674168428ffee..02f9ec09053890a4d41b7dc95644066d6481bbb6 100755
> > --- a/t/t9835-git-p4-metadata-encoding-python2.sh
> > +++ b/t/t9835-git-p4-metadata-encoding-python2.sh
> > @@ -14,23 +14,25 @@ python_target_version='2'
> >  ## SECTION REPEATED IN t9836 ##
> >  ###############################
> >
> > -# Please note: this test calls "git-p4.py" rather than "git-p4", because the
> > -# latter references a specific path so we can't easily force it to run under
> > -# the python version we need to.
> > -
> > -python_major_version=$(python -V 2>&1 | cut -c  8)
> > -python_target_binary=$(which python$python_target_version)
> > -if ! test "$python_major_version" = "$python_target_version" && test "$python_target_binary"
> > +# These tests are specific to Python 2. Write a custom script that executes
> > +# git-p4 directly with the Python 2 interpreter to ensure that we use that
> > +# version even if Git was compiled with Python 3.
> > +python_target_binary=$(which python2)
> > +if test -n "$python_target_binary"
> >  then
> >  	mkdir temp_python
> > -	PATH="$(pwd)/temp_python:$PATH" && export PATH
> > -	ln -s $python_target_binary temp_python/python
> > +	PATH="$(pwd)/temp_python:$PATH"
> > +	export PATH
> > +
> > +	write_script temp_python/git-p4-python2 <<-EOF
> > +	exec "$python_target_binary" "$(git --exec-path)/git-p4" "\$@"
> > +	EOF
> >  fi
> >
> 
> So if the python version (2 here), is available, we create a temp script
> which will use that version. That script is then used in all the
> commands below. Makes sense.
> 
> This is similarly replicated in `t9836` but with Python 3.

Yeah. This whole setup is just plain awkward from my point of view.
Another option would be to accept reality and stop supporting Python 2
altogether. But doing that as part of this patch series did not feel
like a good idea to me.

Thanks for your review!

Patrick

  reply	other threads:[~2024-12-13  5:25 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-11 10:52 [PATCH 0/8] ci: wire up support for Meson Patrick Steinhardt
2024-12-11 10:52 ` [PATCH 1/8] ci/lib: support custom output directories when creating test artifacts Patrick Steinhardt
2024-12-12 10:16   ` karthik nayak
2024-12-13  5:24     ` Patrick Steinhardt
2024-12-11 10:52 ` [PATCH 2/8] Makefile: drop -DSUPPRESS_ANNOTATED_LEAKS Patrick Steinhardt
2024-12-11 10:52 ` [PATCH 3/8] t/unit-tests: rename clar-based unit tests to have a common prefix Patrick Steinhardt
2024-12-12 10:42   ` karthik nayak
2024-12-13  5:25     ` Patrick Steinhardt
2024-12-11 10:52 ` [PATCH 4/8] meson: detect missing tests at configure time Patrick Steinhardt
2024-12-13  9:58   ` Toon Claes
2024-12-13 10:40     ` Patrick Steinhardt
2024-12-11 10:52 ` [PATCH 5/8] Makefile: detect missing Meson tests Patrick Steinhardt
2024-12-11 10:52 ` [PATCH 6/8] t: fix out-of-tree tests for some git-p4 tests Patrick Steinhardt
2024-12-12 10:53   ` karthik nayak
2024-12-13  5:25     ` Patrick Steinhardt [this message]
2024-12-13 10:00   ` Toon Claes
2024-12-13 10:40     ` Patrick Steinhardt
2024-12-11 10:52 ` [PATCH 7/8] t: introduce compatibility options to clar-based tests Patrick Steinhardt
2024-12-13 10:00   ` Toon Claes
2024-12-11 10:52 ` [PATCH 8/8] ci: wire up Meson builds Patrick Steinhardt
2024-12-13 10:01   ` Toon Claes
2024-12-13 10:40     ` Patrick Steinhardt
2024-12-13 10:41 ` [PATCH v2 0/8] ci: wire up support for Meson Patrick Steinhardt
2024-12-13 10:41   ` [PATCH v2 1/8] ci/lib: support custom output directories when creating test artifacts Patrick Steinhardt
2024-12-13 10:41   ` [PATCH v2 2/8] Makefile: drop -DSUPPRESS_ANNOTATED_LEAKS Patrick Steinhardt
2024-12-13 10:41   ` [PATCH v2 3/8] t/unit-tests: rename clar-based unit tests to have a common prefix Patrick Steinhardt
2024-12-13 10:41   ` [PATCH v2 4/8] meson: detect missing tests at configure time Patrick Steinhardt
2024-12-13 10:41   ` [PATCH v2 5/8] Makefile: detect missing Meson tests Patrick Steinhardt
2024-12-13 10:41   ` [PATCH v2 6/8] t: fix out-of-tree tests for some git-p4 tests Patrick Steinhardt
2024-12-13 10:41   ` [PATCH v2 7/8] t: introduce compatibility options to clar-based tests Patrick Steinhardt
2024-12-13 15:56     ` Junio C Hamano
2024-12-13 10:41   ` [PATCH v2 8/8] ci: wire up Meson builds Patrick Steinhardt
2025-07-16 21:07     ` [PATCH] ci: allow github-actions print test failures again Junio C Hamano
2025-07-17  4:43       ` Patrick Steinhardt
2024-12-16  9:32   ` [PATCH v2 0/8] ci: wire up support for Meson Toon Claes
2024-12-16 16:27     ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Z1vFNpcaRIFBODZk@pks.im \
    --to=ps@pks.im \
    --cc=git@vger.kernel.org \
    --cc=karthik.188@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.