git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Luke Diamand <luke@diamand.org>
To: Elijah Newren <newren@gmail.com>
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Joel Holdsworth" <jholdsworth@nvidia.com>,
	"Git Mailing List" <git@vger.kernel.org>
Subject: Re: [PATCH 0/6] Transition git-p4.py to support Python 3 only
Date: Sun, 12 Dec 2021 08:55:20 +0000	[thread overview]
Message-ID: <CAE5ih7-7eH_ezsvZ6TWjZoHg0PZ2nh7C0rKrWSzCnNe44bR2zw@mail.gmail.com> (raw)
In-Reply-To: <CABPp-BHjc1i4o-Oe2U2fV8_TRgRPfve_mYt=kveTYMy-3BdpCA@mail.gmail.com>

On Sat, 11 Dec 2021 at 21:00, Elijah Newren <newren@gmail.com> wrote:
>
> Hi,
>
> On Fri, Dec 10, 2021 at 10:07 PM Junio C Hamano <gitster@pobox.com> wrote:
> >
> > Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> >
> > > On Thu, Dec 09 2021, Joel Holdsworth wrote:
> > >
> > >> Python 2 was discontinued in 2020, and there is no longer any officially
> > >> supported interpreter. Further development of git-p4.py will require
> > >> would-be developers to test their changes with all supported dialects of
> > >> the language. However, if there is no longer any supported runtime
> > >> environment available, this places an unreasonable burden on the Git
> > >> project to maintain support for an obselete dialect of the language.
> > >
> > > Does it? I can still install Python 2.7 on Debian, presumably other OS's
> > > have similar ways to easily test it.
> >
> > Yes, that is a good point to make against "we cannot maintain the
> > half meant to cater to Python2 of the script".  Developers should be
> > able to keep and test Python2 support, if it is necessary.
>
> I also disagree with the reason Joel gave in the quoted paragraph for
> dropping Python2 support, but I think there are other good reasons to
> drop it.
>
> > So the more important question is if there are end-users that have
> > no choice but sticking to Python2.  Is there distributions and
> > systems that do not offer Python3, on which end-users have happily
> > been using Python2?  If some users with vendor supported Python2 do
> > not have access to Python3, cutting them off may be premature.
>
> These are good questions, though I think there's more to it than this,
> as I'll mention in just a minute...
>
> > As the general direction, I do not mind deprecating support for
> > Python2, and then eventually removing it.  I just do not know if 2
> > years is long enough for the latter (IIRC, the sunset happened at
> > the beginning of 2020, and we are about to end 2021).
>
> Python2 was deprecated by the python project in 2008, with announced
> plans to stop all support (including security fixes) in 2015.  They
> pushed the sunset date back to Jan 1, 2020.  So it has only been
> end-of-life for just under 2 years, but it's been deprecated for over
> 13 years.
>
> In regards to your good questions about Python3 availability on some
> platforms: If such platforms exist, they have had over a decade's
> heads up...so let's ask a few extra questions.  If these platforms
> still haven't made python3 available, would newer versions of Git even
> be available on these platforms?  Even if newer Git versions are
> available, would users on such platforms have any qualms with using an
> older Git version given the platform insistence of only providing an
> old Python version lacking any support (even security fixes)?
>
>
> Some of my personal python2/python3 experience, if it's useful in
> weighing decisions:
>
> * There are python projects for which I still continue to support
> simultaneous python2 and python3 usage, though for projects that are
> smaller then git-p4.py (e.g. 1/2 to 1/3 the size).  Such multi-version
> support is painful, and it causes occasional bugs that hit users that
> wouldn't arise if there was only one supported python version.
>
> * I initially wanted to also do the multi-version support for
> git-filter-repo (which is approximately the same size as git-p4.py,
> and obviously also interfaces with git somewhat deeply).  I gave up on
> it, and didn't consider it justified, especially with the
> then-soon-impending End-Of-Life for python2.  I instead just switched
> from python2 -> python3 (in 2019; yes, I'm a straggler.)  Granted, I
> did benefit from the fact that git-filter-repo is a
> once-in-a-blue-moon usage tool (and only by one member on the team),
> rather than a daily usage tool, but I may have come to the same
> decision anyway even back then.
>
> * (Slight tangent) I tried to use unicode strings everywhere in
> git-filter-repo a few times, but invariably found it to be buggy and
> slow.  It was a mistake, and I eventually switched over to bytestrings
> everywhere, only converting to unicode (when possible) when printing
> messages for the user on the console.  bytestrings are ugly to use
> (IMO), but they're a better data model when dealing with file
> contents, process output, filenames, etc.  I think git-p4's decision
> to attempt to use unicode strings everywhere is a mistake that'll
> probably result in bugs based on that experience of mine; it's not an
> appropriate model of the relevant data.  It'll also make things
> slower.

+1

When using python2, git-p4 just ignores all the encoding issues, and I
think this is actually exactly what we want. This means that in some
ways, the Python2 version is currently more correct than the Python3
one.

With Python3, as you say, it attempts to convert to/from unicode
strings, and probably this is the root of the problems.

Perforce has a mode where it works in unicode internally. This gets
configured in the server, and clients then do The Right Thing.

https://www.perforce.com/manuals/p4sag/Content/P4SAG/superuser.unicode.clients.html

However, many P4 shops don't bother to set this (I know we don't) so
you end up with BIG5 and CP1252 just being dumped raw into Perforce,
and then coming back out again. This causes problems for Perforce
clients just as much as for git-p4 clients.

When git-p4 is used with python2, provided your client has the same
character encoding as the author of the code you are looking at, it
will appear to work. Otherwise you get character encoding errors.

To fix this with python3, we need to make a choice:

- either work out the character encoding that Perforce is sending us
which it doesn't know and can't tell us and then convert that to/from
unicode. There was a patch series a while ago which tried to let you
configure a default encoding.
- or to just preserve everything that Perforce sends us and let the
client figure it out.

The current patch series is (I think) tending towards the first of
those options. But we're trying to recreate information that just
doesn't exist.

Here's the patch that Andrew Oakley sent a while back which attempts
to get git-p4 to just preserve what it gets:

https://lore.kernel.org/git/20210412085251.51475-1-andrew@adoakley.name/

Luke


>
> [I actually think the unicode vs. bytestring thing might be more
> important for bug fixing than limiting to python3.  Though I think
> both are worthwhile.]

+1

  reply	other threads:[~2021-12-12  8:55 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-09 20:10 [PATCH 0/6] Transition git-p4.py to support Python 3 only Joel Holdsworth
2021-12-09 20:10 ` [PATCH 1/6] git-p4: Always pass cmd arguments to subprocess as a python lists Joel Holdsworth
2021-12-09 22:42   ` Junio C Hamano
2021-12-09 20:10 ` [PATCH 2/6] git-p4: Don't print shell commands as " Joel Holdsworth
2021-12-09 20:10 ` [PATCH 3/6] git-p4: Removed support for Python 2 Joel Holdsworth
2021-12-09 22:44   ` Junio C Hamano
2021-12-09 23:07     ` rsbecker
2021-12-10  3:25   ` David Aguilar
2021-12-10 10:44     ` Joel Holdsworth
2021-12-09 20:10 ` [PATCH 4/6] git-p4: Decode byte strings before printing Joel Holdsworth
2021-12-09 22:47   ` Junio C Hamano
2021-12-10  8:40     ` Fabian Stelzer
2021-12-10 10:48       ` Joel Holdsworth
2021-12-10 10:41     ` Joel Holdsworth
2021-12-09 20:10 ` [PATCH 5/6] git-p4: Eliminate decode_stream and encode_stream Joel Holdsworth
2021-12-09 20:10 ` [PATCH 6/6] git-p4: Resolve RCS keywords in binary Joel Holdsworth
2021-12-10  7:57   ` Luke Diamand
2021-12-10 10:51     ` Joel Holdsworth
2021-12-10  0:48 ` [PATCH 0/6] Transition git-p4.py to support Python 3 only Ævar Arnfjörð Bjarmason
2021-12-10 10:37   ` Joel Holdsworth
2021-12-10 11:30     ` Ævar Arnfjörð Bjarmason
2021-12-10 21:34   ` Junio C Hamano
2021-12-10 21:53     ` rsbecker
2021-12-11 21:00     ` Elijah Newren
2021-12-12  8:55       ` Luke Diamand [this message]
2021-12-10  7:53 ` Luke Diamand
2021-12-10 10:54   ` Joel Holdsworth
2021-12-11  9:58     ` Luke Diamand
2021-12-13 13:47       ` Joel Holdsworth
2021-12-13 19:29         ` Junio C Hamano
2021-12-13 19:58           ` Joel Holdsworth

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=CAE5ih7-7eH_ezsvZ6TWjZoHg0PZ2nh7C0rKrWSzCnNe44bR2zw@mail.gmail.com \
    --to=luke@diamand.org \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jholdsworth@nvidia.com \
    --cc=newren@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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).