GeezerVison

cc. also @ned-deily who has opinions about the build process.

FWIW I have a follow-up change to the module freezing code that is a net -100 lines on the diff which uses a similar approach to reduce make boilerplate and redundancy. That one more clearly demonstrates value in the general pattern than this one. But the justifications are basically the same.

Is this really more portable than the pattern rule approach? It's certainly more cryptic :)

In these particular cases, I'd personally like to work towards getting libmpdec and expat out of our repository entirely (i.e., make --with-system-expat --with-system-libmpdec the default and later only way to build), though that's a bigger project with a longer timeline. I wonder if it would make sense to pull in a bit extra from these packages for now in order to delegate to their own build system to build them instead of shoehorning them into ours, but a cursory look says that's probably more effort than it's worth.

This looks reasonable enough to try it on the buildbots, at least.

🤖 New build scheduled with the buildbot fleet by @zware for commit 44eda25 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

The buildbot failures don't look related to this change.

The buildbot failures don't look related to this change.

@zware, it looks like the FreeBSD buildbot build didn't or hasn't run yet? I think this is the relevant buildrequest.

That worker has apparently been down for about 4 months now, though I can't tell at a glance if the fault lies on the client or server side (@koobs?)

IMO, we should wait for 3.13 to open before landing this.

I didn't realize we were only days away from the end of the 3.12 new features window. I guess I'm so used to Rust and its 6 week release cadence.

Anyway, yes, I'm fine waiting until the 3.13 window to land more invasive build system changes. (Not like I have much choice in the matter.)

@indygreg: How does this fare on BSD Make?

On my FreeBSD, I can no longer build Python :-(

$ make
make: "/usr/home/vstinner/python/main/Makefile" line 1047: Invalid line type
make: "/usr/home/vstinner/python/main/Makefile" line 1050: Invalid line type
make: "/usr/home/vstinner/python/main/Makefile" line 1052: Invalid line type
make: "/usr/home/vstinner/python/main/Makefile" line 1062: Invalid line type
make: "/usr/home/vstinner/python/main/Makefile" line 1065: Invalid line type
make: "/usr/home/vstinner/python/main/Makefile" line 1067: Invalid line type
make: Fatal errors encountered -- cannot continue
make: stopped in /usr/home/vstinner/python/main

I have;

$ make -V MAKE_VERSION
20220208

On my FreeBSD, I can no longer build Python :-(

Could you build it on FreeBSD before this PR?

Could you build it on FreeBSD before this PR?

Sorry. I mean: currently, on the main branch of Python (without this PR), I can no longer build Python :-(

Could you build it on FreeBSD before this PR?

Sorry. I mean: currently, on the main branch of Python (without this PR), I can no longer build Python :-(

Aha, so this PR does not change the status quo! I suspected that. I guess we should clarify if was want to restore BSD Make compatibility, or carry on with GNU Make only.

Please fix building Python on FreeBSD before making the situation worse.

Sadly, the maintainer of our two FreBSD decided to remove them. Right now, there is no FreeBSD CI. We should consider CirrusCI.

I tested FreeBSD again, and I'm sorry, I didn't test make correctly previously.

• Building Python in the main branch on FreeBSD is fine!
• Building Python with this PR on FreeBSD no longer works

This PR introduces these issues in Makefile:

$ make
make: "/usr/home/vstinner/python/main/Makefile" line 1047: Invalid line type
make: "/usr/home/vstinner/python/main/Makefile" line 1050: Invalid line type
make: "/usr/home/vstinner/python/main/Makefile" line 1052: Invalid line type
make: "/usr/home/vstinner/python/main/Makefile" line 1062: Invalid line type
make: "/usr/home/vstinner/python/main/Makefile" line 1065: Invalid line type
make: "/usr/home/vstinner/python/main/Makefile" line 1067: Invalid line type
make: Fatal errors encountered -- cannot continue
make: stopped in /usr/home/vstinner/python/main

Surprise, surprise, it's the "non portable" part of this PR which introduces the issue. Like:

# "%.o: %c" is not portable
define libmpdec_compile
$(1): $$(srcdir)/$$(patsubst %.o,%.c,$(1)) $$(LIBMPDEC_HEADERS) $$(PYTHON_HEADERS)
        $$(CC) -c $$(LIBMPDEC_CFLAGS) -o $$@ $$<
endef

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

And if you don't make the requested changes, you will be put in the comfy chair!

@erlend-aasland: Is this change worth it if it breaks FreeBSD support? Should we just close it?

🤖 New build scheduled with the buildbot fleet by @erlend-aasland for commit 444a7c1 🤖

The command will test the builders whose names match following regular expression: .*freebsd.*

The builders matched are:

• AMD64 FreeBSD PR

@erlend-aasland: Is this change worth it if it breaks FreeBSD support? Should we just close it?

If it breaks FreeBSD support it is not worth it. There might be a more portable approach, though. cc. @indygreg

If it breaks FreeBSD support it is not worth it.

Do you expect that FreeBSD buildbot will behave differently than my previous manual test?
#104574 (comment)

The PR didn't change in the meanwhile.

There might be a more portable approach, though. cc. @indygreg

I'm fine with any syntax as soon as it works on all platforms supported by Python ;-)

Do you expect that FreeBSD buildbot will behave differently than my previous manual test? #104574 (comment)

The PR didn't change in the meanwhile.

Oh, I forgot you already tested this!

buildbot/AMD64 FreeBSD PR — Build done.

Fail to build:

 using PTY: False
make: "/buildbot/buildarea/pull_request.ware-freebsd/build/Makefile" line 1049: Invalid line type
make: "/buildbot/buildarea/pull_request.ware-freebsd/build/Makefile" line 1052: Invalid line type
make: "/buildbot/buildarea/pull_request.ware-freebsd/build/Makefile" line 1054: Invalid line type
make: "/buildbot/buildarea/pull_request.ware-freebsd/build/Makefile" line 1064: Invalid line type
make: "/buildbot/buildarea/pull_request.ware-freebsd/build/Makefile" line 1067: Invalid line type
make: "/buildbot/buildarea/pull_request.ware-freebsd/build/Makefile" line 1069: Invalid line type
make: stopped in /buildbot/buildarea/pull_request.ware-freebsd/build
make: Fatal errors encountered -- cannot continue

What’s wrong with requiring people to use gmake?

It makes building Python on FreBSD more annoying and complicated.

If someone wants to use more advanced Makefile rule, I suggest to rewrite the whole Python build system with CMake, Meson or anything else.

This PR is stale because it has been open for 30 days with no activity.

unstale

This change breaks support for BSD make on FreeBSD. I don't think that it's worth it, so I prefer to close it.

@zware:

In these particular cases, I'd personally like to work towards getting libmpdec and expat out of our repository entirely

For libmpdec, there is a concrete plan for that: #115119. This PR is mostly about libmpdec, it only changes 6 lines about expat.

Building Python with this PR on FreeBSD no longer works
(...)
While it's appealing to use cool GNU make features to make the Makefile shorter, I don't think that it's worth it if in exchange we break FreeBSD support.

If someone disagrees, please open a discussion on discuss.python.org suggesting to drop support for BSD make and require GNU make on FreeBSD.

There are also generic discussions about changing the whole Python build system, such as What do you want to see in tomorrow’s CPython build system?

@indygreg:

I think this is sufficiently portable. If not, there are alternatives to $(patsubst) that can be leveraged.

If someone wants to propose a PR using $(patsubst), please go ahead. I can test it on FreeBSD.

Another approach would be to generate the code for libmpdec, as we already do to build stdlib extension modules. (I'm not sure which code is involved, maybe Modules/makesetup.)