[Gluster-devel] Python3 build process

[Shyam Ranganathan]

On 09/28/2018 09:11 AM, Niels de Vos wrote:
> On Fri, Sep 28, 2018 at 08:57:06AM -0400, Shyam Ranganathan wrote:
>> On 09/28/2018 06:12 AM, Niels de Vos wrote:
>>> On Thu, Sep 27, 2018 at 08:40:54AM -0400, Shyam Ranganathan wrote:
>>>> On 09/27/2018 08:07 AM, Kaleb S. KEITHLEY wrote:
>>>>>> The thought is,
>>>>>> - Add a configure option "--enable-py-version-correction" to configure,
>>>>>> that is disabled by default
>>>>> "correction" implies there's something that's incorrect. How about
>>>>> "conversion" or perhaps just --enable-python2
>>>>>
>>>> I would not like to go with --enable-python2 as that implies it is an
>>>> conscious choice with the understanding that py2 is on the box. Given
>>>> the current ability to detect and hence correct the python shebangs, I
>>>> would think we should retain it as a more detect and modify the shebangs
>>>> option name. (I am looking at this more as an option that does the right
>>>> thing implicitly than someone/tool using this checking explicitly, which
>>>> can mean different things to different people, if that makes sense)
>>>>
>>>> Now "correction" seems like an overkill, maybe "conversion"?
>>> Is it really needed to have this as an option? Instead of an option in
>>> configure.ac, can it not be a post-install task in a Makefile.am? The
>>> number of executable python scripts that get installed are minimal, so I
>>> do not expect that a lot of changes are needed for this.
>>
>> Here is how I summarize this proposal,
>> - Perform the shebang "correction" for py2 in the post install
>>   - Keeps the git clone clean
>> - shebang correction occurs based on a configure time option
>>   - It is not implicit but an explicit choice to correct the shebangs to
>> py2, hence we need an option either way
>> - The configure option would be "--enable-python2"
>>   - Developers that need py2, can configure it as such
>>   - Regression jobs that need py2, either because of the platform they
>> test against, or for py2 compliance in the future, use the said option
>>   - Package builds are agnostic to these changes (currently) as they
>> decide at build time based on the platform what needs to be done.
> 
> I do not think such a ./configure option is needed. configure.ac can
> find out the version that is available, and pick python3 if it has both.
> 
> Tests should just run with "$PYTHON run-the-test.py" instead of
> ./run-the-test.py with a #!/usr/bin/python shebang. The testing
> framework can also find out what version of python is available.

If we back up a bit here, if all shebangs are cleared, then we do not
need anything. That is not the situation at the moment, and neither do I
know if that state can be reached.

We also need to ensure we work against py2 and py3 for the near future,
which entails being specific in some regression job at least on the
python choice, does that correct the shebangs really depends on the
above conclusion.

> 
> 
>>> There do seem quite some Python files that have a shebang, but do not
>>> need it (__init__.py, not executable, no __main__-like functions). This
>>> should probably get reviewed as well. When those scripts get their
>>> shebang removed, even fewer files need to be 'fixed-up'.
>>
>> I propose maintainers/component-owner take this cleanup.
> 
> That would be ideal!
> 
> 
>>> Is there a BZ or GitHub Issue that I can use to send some fixes?
>>
>> See: https://github.com/gluster/glusterfs/issues/411
> 
> Thanks,
> Niels
>