OSDir

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Python-Dev] Changes to configure.ac, auto-detection and related build issues


On 15 May 2018 at 05:54, Ned Deily <nad at python.org> wrote:
> On May 15, 2018, at 01:58, Eitan Adler <lists at eitanadler.com> wrote:
>> On Monday, 14 May 2018, Victor Stinner <vstinner at redhat.com> wrote:
>> Hi Eitan,
>>
>> 2018-05-15 0:01 GMT-04:00 Eitan Adler <lists at eitanadler.com>:
>> > I am working on updating, fixing, or otherwise changing python's
>> > configure.ac. This work is complex, (...)
>>
>> Is your work public? Is there an open issue on bugs.python.org or an
>> open pull request?
>>
>> I'm opening bugs and PRs as I Go. Some examples are:
>>
>> https://github.com/python/cpython/commit/98929b545e86e7c7296c912d8f34e8e8d3fd6439
>> https://github.com/python/cpython/pull/6845
>> https://github.com/python/cpython/pull/6848
>> https://github.com/python/cpython/pull/6849
>> https://bugs.python.org/issue33485
>>
>> And so on
>>
>>
>> If not, would you mind to at least describe the changes that you plan to do?
>>
>> > Please feel free to tag me in
>> > related PRs or bugs or emails over the next few weeks.
>>
>> Hopefully, we only rarely need to modify configure.ac
>>
>> I'm primarily worried about breaking arcane platforms I don't have direct access to.
>
>
> Hi, Eitan!
>
> As you recognize, it is always a bit dangerous to modify configure.ac and friends as we do support so many platforms and configuration and downstream users try combinations that we don't claim to test or support.  So, we try to be conservative about making changes there and do so only with good reason.
>
> So far, it's somewhat difficult for me to understand what you are trying to accomplish with the changes you've noted so far other than various cosmetic cleanups.

This all started when I found a bug in the C code of python. I wanted
to submit a PR and test my change, but found that it was painful to
compile Python on many platforms. In particular I needed to use
"clang" but configure.ac was looking for a compiler called "gcc
-pthread". As I started to fix this, I realized there is a lot of
unneeded complexity in configure.ac and am now trying to clean that
up.

>   It is also difficult to properly review a bunch of small PRs that modify the same configuration files and especially without an overall tracking issue.

I wanted to keep the reviews small to be reviewable, revertable, and
bisectable. Is there a nicer way of handling that? Maybe a single
large review with a series of small commits?

> For most of this to move forward, I think you should create or adapt at least one b.p.o issue to describe what changes you are suggesting and why and how they apply to our various platforms and then consolidate PRs under that PR.  Don't be surprised if the PRs don't get much attention right away as we're busy at the moment trying to get 3.7.0 out the door.

Understood. There are lot of PRs and a lot of work. I've been pretty
happy with the traction so far.

>  And it would be best to avoid including generated files (like configure vs configure.ac) in new PRs as that will only add to the likelihood of merge conflicts and review complexity.

I've gotten three different pieces of advice about this:
(a) always include them, so its easier to review
(b) never include, so its easier to review and and avoid merge conflicts
(c) only include if your tool version matches what was used originally

I don't care much but its clear there isn't agreement.

-- 
Eitan Adler