From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=kk7RxWGv; spf=pass (domain: linaro.org, ip: 209.85.128.66, mailfrom: leif.lindholm@linaro.org) Received: from mail-wm1-f66.google.com (mail-wm1-f66.google.com [209.85.128.66]) by groups.io with SMTP; Tue, 16 Jul 2019 15:04:53 -0700 Received: by mail-wm1-f66.google.com with SMTP id s15so20126215wmj.3 for ; Tue, 16 Jul 2019 15:04:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=Sgm3WmLbPlIzQmnKw/sW6lv59Ntv3Prs6YOR64/AT7g=; b=kk7RxWGvKVBuaPPiaipGA9A/ukF4rfeaz3HkaGbwReH0K37GDCumtCVPEmJpgXCVf+ +WReXcdsWIOKKTYBYRNbWeut7+TLiZsF5Xr7qMJzugzsPY96lEa+cUhhQ6cECXhxxabY 7XWxEwT7uYF8qrctpzQOIaIH3zubkBRZsCOV6nxBCTYUY4H5MxxNt6zB+oqcJKeWBmu/ 8pvn5aVmqVFeBJjZztmFHhRXhxH5dKkdFgryIbIfqcAt7zN4p/k4vUOtkOibG1A7qt85 mWCRn6EMR+3aPKLqvOtE5b0L/MYTZ/5e1ZwHnU/l6WAhrI4fAZEKuNsDS3+TV2vDeTbE T6Lg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=Sgm3WmLbPlIzQmnKw/sW6lv59Ntv3Prs6YOR64/AT7g=; b=lg+sCM/fijfFcOgaCk4wmrr73oAV+9TjzX9c6gQ0veU76k5iN5je5oImYl6ZY9n6Fv u+JYoHqVBa9IzHawn5XLa8kSPNPIIinUHMDYA/ahp2KA5mGj5vZQYHFi5fAWyUDz6pDz 68M9dG07zw9cGpt3IgQXG35hQ7bOrVh1wDd+ov8ITn/ND9g91cIjlTQRaDlrm0LnH8nO EyIc/jPTNfhaZuym8NFDRBKRnzP6IVam2rz+kEucRiTIKwB/GypTLzrmF9HLt81emHd3 qed9PntB7FphkQ9jBy6JiY2v2nsWHS3e2PDaCzX7stw5ibwfwfKDc004v/OLCBw8IR5H nu6w== X-Gm-Message-State: APjAAAUs15Gh2Ed2220kF9n7CwPXtwLRTsH/MJC+F59l8AVvWDCsfNZa OR+8ggakUzCtFyLZZ3n4+GdWzg== X-Google-Smtp-Source: APXvYqwqOhRvykEhFoOpx2n4gdrsghQRnOmd4kPniZGDP1WfsAMXqHywyBaF1rRE4jIIhg/ezIlagA== X-Received: by 2002:a05:600c:23d2:: with SMTP id p18mr30425555wmb.160.1563314692050; Tue, 16 Jul 2019 15:04:52 -0700 (PDT) Return-Path: Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id d10sm15797707wrx.34.2019.07.16.15.04.51 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 16 Jul 2019 15:04:51 -0700 (PDT) Date: Tue, 16 Jul 2019 23:04:49 +0100 From: "Leif Lindholm" To: Laszlo Ersek Cc: devel@edk2.groups.io, Rebecca Cran , bob.c.feng@intel.com, liming.gao@intel.com, michael.d.kinney@intel.com, afish@apple.com Subject: Re: [PATCH 1/1] edksetup.sh: rework python executable scanning Message-ID: <20190716220449.r7kfozr7yaasi64k@bivouac.eciton.net> References: <20190716190754.25412-1-leif.lindholm@linaro.org> <20579d07-778d-ef9f-9226-25e9629fa2d5@redhat.com> MIME-Version: 1.0 In-Reply-To: <20579d07-778d-ef9f-9226-25e9629fa2d5@redhat.com> User-Agent: NeoMutt/20170113 (1.7.2) Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Tue, Jul 16, 2019 at 10:49:03PM +0200, Laszlo Ersek wrote: > Hi Leif, > > On 07/16/19 21:07, Leif Lindholm wrote: > > If PYTHON_COMMAND is set, use that. > > If PYTHON_COMMAND is not set, use first working of "python", "python3", "python2". > > If none of those work, search the path for python*[0-9], using the highest version > > number across x.y.z format. > > > > Finally, set PYTHON3_ENABLE if selected python is python 3. > > > > Signed-off-by: Leif Lindholm > > --- > > > > This is my somewhat overkill proposal as an alternative to Rebecca's 5/6. > > It is certainly more complex than that patch, and arguably as complex as > > the current upstream implementation, but the semantics and flow is more > > clear (to me, at least). > > > > An alternative version to *this* patch would be one that drops the > > FindHighestVersionedExecutable() function, and the if-statement that > > calls it. This would still leave us with a solution that would use (in order): > > * PYTHON_COMMAND > > * python > > * python3 > > * python2 > > and fail with an error if $PYTHON_COMMAND (if set externally) or none of the > > others could execute $PYTHON_COMMAND --version. > > I think I'd be fine, personally, with this change. It's still a > behavioral change in two aspects, and so I'd like the BaseTools > maintainers to comment on those: Likewise. > (1) If I understand correctly, the proposed patch would favor "python" > (no version number) over "python3". That diverges from current practice. Yes. And in fact this is one of the things I found problematic (but not seriously so) with the functionality that is currently in the tree. > I think this is a relevant question because the BaseTools maintainers > prefer python3 over python2, if a system offers both, even if the system > default "python" points to "python2". > > I deduce this claim from > : > > "we have no enough resource to fully verify Python2 and Python3 both. We > will focus on Python3 validation. If anyone can help verify Python2, it > will be great. And, if you meet with the issue on Python2, please file > BZ. We still fix them." > > So the primary target seems to be python3; considered "more thoroughly > validated" at all times (my words). Yes, but likewise, a system may have more properly validated (and certainly more likely to have common required modules installed for) the python installed as "python". This applies even more so with CI builders running docker images, since whoever configures those is more likely than an end-user to change distribution default. Important point here being that overriding the default behaviour is as easy as PYTHON_COMMAND=python3. > (2) The original proposal (see it included e.g. in > ) > gave some significance to the PYTHON3_ENABLE variable (coming from the > user's environment). Doesn't this patch erase that? Oh, was that was that was for? I couldn't really figure it out (especially w.r.t. relationship with the same in build.py). > ( > > Looking back at that specification -- I basically don't understand what > PYTHON3_ENABLE=TRUE was supposed to stand for, coming from the user's env. You and me both :) > I do understand the other two cases: PYTHON3_ENABLE unset, plus > PYTHON_COMMAND either set or unset. This last variation is actually what > my question (1) concerns. > > ) > > > Furthermore, > > (3) after looking at FindHighestVersionedExecutable(), I think I would > prefer the variant of this patch -- assuming the behavioral change is OK > in the first place -- that does not have > FindHighestVersionedExecutable(). It's not that > FindHighestVersionedExecutable() is too complex -- the problem is that > the function is complex *and* a good chunk of it will never be used in > practice. I doubt we have to prepare for python4 or python5 at this > point. Similarly for patch levels. So, first of all - I am entirely happy with dropping the function. But I wanted to have it written anyway in case someone came up with a really good explanation for why we needed to completely cover the same pattern as the code currently in tree: and the situation does not relate to python4 or python5 - it relates to picking the latest of 2.7, 2.7.6, 3.5, 3.5.2, 3.5,4, 3.7 (which is my interpretation of the behaviour of current HEAD). So basically - I think we need to reach an agreement (with BaseTools maintainers, and existing users) about what the behaviour should be. - What does PYTHON3_ENABLE mean? Is it for probing only, or are we setting it for later use by BaseTools? - What should the priority order be when looking for python executables? - Can we use simply 'python' as the default? - Do we need functionality for more than selecting between python2/python3? If we don't _need_ to support anything beyond python2 and python3 (other than overriding with PYTHON_COMMAND), then including FindHighestVersionedExecutable() would be outright silly. > I believe one thing I like about Rebecca's 5/6 is that, while it does > check for some surprisingly high (and arbitrary) minor releases -- such > as python3.15, python2.10 --, the loops are really small and simple. No > extra complexity is needed for covering all practically relevant > releases. In case we ever exceeded "python3.15", it would take a > one-liner to bump the limit (and it would take a simple patch to factor > out the loop to a function, and check for python4.[0..15] even). I dislike arbitrary limits, and planned maintenance overhead (no matter how trivial). If we only need to worry about picking python2 or python3, then we can both be happy. Best Regards, Leif