GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — bpo-45573: Introduce extension module flags in Makefile (GH-29594) by tiran · Pull Request #29594 · python/cpython

Conversation

tiran commented Nov 17, 2021 •

edited by bedevere-bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Member

configure now uses a standardized format to forward state, compiler
flags, and linker flags to Makefile, setup.py, and
Modules/Setup. makesetup use the new variables by default if a
module line does not contain any compiler or linker flags. setup.py
has a new function addext().

For a module egg, configure adds:

MODULE_EGG with value yes, missing, disabled, or n/a
MODULE_EGG_CFLAGS
MODULE_EGG_LDFLAGS

Makefile.pre.in may also provide MODULE_EGG_DEPS that lists
dependencies such as header files and static libs.

Signed-off-by: Christian Heimes [email protected]

https://bugs.python.org/issue45573

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

tiran requested a review from erlend-aasland

November 17, 2021 14:44

the-knights-who-say-ni added the CLA signed label

Nov 17, 2021

bedevere-bot added the awaiting core review label

Nov 17, 2021


          bpo-45573: Introduce extension module flags in Makefile

23986fc

``configure`` now uses a standardized format to forward state, compiler
flags, and linker flags to ``Makefile``, ``setup.py``, and
``Modules/Setup``. ``makesetup`` use the new variables by default if a
module line does not contain any compiler or linker flags. ``setup.py``
has a new function ``addext()``.

For a module ``egg``, configure adds:

* ``MODULE_EGG`` with value yes, missing, disabled, or n/a
* ``MODULE_EGG_CFLAGS``
* ``MODULE_EGG_LDFLAGS``

``Makefile.pre.in`` may also provide ``MODULE_EGG_DEPS`` that lists
dependencies such as header files and static libs.

Signed-off-by: Christian Heimes <[email protected]>

tiran marked this pull request as ready for review

November 17, 2021 18:06

tiran force-pushed the bpo-45573-configure-mod branch from 6eb4638 to 23986fc Compare

November 17, 2021 18:06

erlend-aasland approved these changes

Nov 17, 2021

View reviewed changes

erlend-aasland left a comment

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! LG

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

setup.py

		# not available on current platform
		pass
		else:
		# not migrated to MODULE_{name} yet.

erlend-aasland Nov 17, 2021

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps print an info message?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

tiran Nov 18, 2021

Copy link

Copy Markdown

Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good thinking, I plan to add an error message after we have ported all stdlib modules to new system.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

erlend-aasland reacted with thumbs up emoji

All reactions

👍 1 reaction

Comment thread

setup.py

Comment on lines +417 to +418

		if cflags:
		for token in shlex.split(cflags):

erlend-aasland Nov 17, 2021

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, it would have been convenient to be able write for token in shlex.split(cflags) if cflags: :)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

ambv Nov 17, 2021

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can for token in shlex.split(cflags) if cflags else (): if you really want to but I find the boring vanilla form less clever which is a Good Thing.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

erlend-aasland reacted with laugh emoji

tiran reacted with heart emoji

All reactions

😄 1 reaction
❤️ 1 reaction

tiran changed the title ~~bpo-45573: Introduce extension module flags in Makefile~~ bpo-45573: Introduce extension module flags in Makefile (GH-29594)

Nov 18, 2021

tiran merged commit 25ecc04 into python:main

Nov 18, 2021

tiran deleted the bpo-45573-configure-mod branch

November 18, 2021 08:18

bedevere-bot removed the awaiting core review label

Nov 18, 2021

remykarem pushed a commit to remykarem/cpython that referenced this pull request

Dec 7, 2021


          bpo-45573: Introduce extension module flags in Makefile (pythonGH-29594)

74e5e06

``configure`` now uses a standardized format to forward state, compiler
flags, and linker flags to ``Makefile``, ``setup.py``, and
``Modules/Setup``. ``makesetup`` use the new variables by default if a
module line does not contain any compiler or linker flags. ``setup.py``
has a new function ``addext()``.

For a module ``egg``, configure adds:

* ``MODULE_EGG`` with value yes, missing, disabled, or n/a
* ``MODULE_EGG_CFLAGS``
* ``MODULE_EGG_LDFLAGS``

``Makefile.pre.in`` may also provide ``MODULE_EGG_DEPS`` that lists
dependencies such as header files and static libs.

Signed-off-by: Christian Heimes <[email protected]>

tiran mentioned this pull request

Jan 22, 2023

Use pkg-config autoconf macros to detect flags for Modules/Setup #89736

Closed

Reviewers

ambv

ambv left review comments

erlend-aasland

erlend-aasland approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

tiran commented Nov 17, 2021 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

erlend-aasland left a comment

Choose a reason for hiding this comment

Uh oh!

erlend-aasland Nov 17, 2021

Choose a reason for hiding this comment

Uh oh!

tiran Nov 18, 2021

Choose a reason for hiding this comment

Uh oh!

erlend-aasland Nov 17, 2021

Choose a reason for hiding this comment

Uh oh!

ambv Nov 17, 2021

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

tiran commented Nov 17, 2021 •

edited by bedevere-bot

Loading

Uh oh!

There was an error while loading. Please reload this page.