classification
Title: build system runs when it may merely link
Type: Stage: patch review
Components: Build Versions:
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: eitan.adler, r.david.murray
Priority: normal Keywords: patch

Created on 2018-05-13 18:38 by eitan.adler, last changed 2018-05-15 07:44 by eitan.adler.

Pull Requests
URL Status Linked Edit
PR 6781 open eitan.adler, 2018-05-13 18:41
Messages (5)
msg316471 - (view) Author: Eitan Adler (eitan.adler) * Date: 2018-05-13 18:38
The build system attempts to run certain test code when it can actually just link the code. This is a minor performance optimization but is really a semantic correctness issue.
msg316473 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2018-05-13 18:52
Thanks for wanting to improve Python.

Your PR seems to contain quite a few changes that aren't obviously related to your topic.

As for the topic, I know there are at least some cases where we really do have to *run* the code to prove the function is supported by the platform.  That is, there are platforms that define the function but it doesn't actually work.  I don't know if that applies to the things you are changing, and it will require input from people with more knowledge of the build system than I to confirm that your changes are a good idea.
msg316474 - (view) Author: Eitan Adler (eitan.adler) * Date: 2018-05-13 18:56
The unrelated changes are likely due to the "regen" step. I am using newer tools than previously used. It is unclear to me why we include generated files in the repository. Some projects prefer this to avoid the need to have autotools installed on the build machine.

With regard to "some cases where we really do have to *run* the code to prove the function" you will notice that this is not a complete search and replace: I manually looked through the various uses of AC_RUN_IFELSE and only changed the ones that I thought should be changed. If someone has more knowledge of more arcane platforms and disagrees with my assessment, I will happily revert those changes.
msg316475 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2018-05-13 19:03
Yes, we are one of those projects that wants it to be possible to build python without having the tools that generate build artifacts.  You will note that we also check in build artifacts that are produced by our own python scripts, to avoid the problem of trying to build python when you don't yet have a python.

Please use the same version of autotools as were used to generate the current files.  Upgrading autotools is done in a separate PR that only does the upgrade (and I don't know how we decide when to do that...)

Actually, you could just generate the PR with the changes to the source files, not the build artifacte, for the initial review.
msg316476 - (view) Author: Eitan Adler (eitan.adler) * Date: 2018-05-13 19:06
I believe I have removed the regen step from all my patches. I will separately upgrade the generated files (particularly since it is only a minor version bump).
History
Date User Action Args
2018-05-15 07:44:31eitan.adlersettitle: build system runs when it merely link -> build system runs when it may merely link
2018-05-13 19:06:28eitan.adlersetmessages: + msg316476
2018-05-13 19:03:42r.david.murraysetmessages: + msg316475
2018-05-13 18:56:11eitan.adlersetmessages: + msg316474
2018-05-13 18:52:16r.david.murraysetnosy: + r.david.murray
messages: + msg316473
2018-05-13 18:41:47eitan.adlersetkeywords: + patch
stage: patch review
pull_requests: + pull_request6468
2018-05-13 18:38:36eitan.adlercreate