Subject: Re: [PATCH 6/8] t: fix out-of-tree tests for some git-p4 tests

From: Patrick Steinhardt <[email-masked]>
Date: Fri, 13 Dec 2024 06:25:10 +0100

On Thu, Dec 12, 2024 at 02:53:47AM -0800, karthik nayak wrote:
> Patrick Steinhardt <[email-masked]> 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