Page MenuHomeFreeBSD

lang/python310: Introduce DTRACE option
Needs RevisionPublic

Authored by 0mp on Aug 10 2021, 9:33 AM.
Tags
None
Referenced Files
Unknown Object (File)
Feb 16 2024, 12:26 PM
Unknown Object (File)
Jan 13 2024, 11:53 AM
Unknown Object (File)
Jan 10 2024, 3:24 PM
Unknown Object (File)
Dec 22 2023, 11:20 PM
Unknown Object (File)
Dec 12 2023, 11:33 PM
Unknown Object (File)
Oct 30 2023, 8:10 AM
Unknown Object (File)
Oct 28 2023, 5:28 PM
Unknown Object (File)
Sep 13 2023, 4:16 AM

Details

Reviewers
koobs
Group Reviewers
Python
Summary
lang/python310: Introduce DTRACE option

- Let users compile Python with DTrace support. The option is off by
  default for now.

- Set USES=gmake. It turns out that the Python makefiles contain syntax
  specific to GNU make, which is interpreted by bmake in an unexptected way (in
  particular, $< is not expanded as desired). In order to fix that, let's just
  use GNU make for now.

- Patch the configure script in order to force building of the object files
  required for DTrace support. Here's the problem with the detection mechanism:

  $ dtrace -G -s ./conftest.d -xnolibs -o /tmp/conftest.o
  ld: error: /usr/lib/dtrace/drti.o is incompatible with /tmp/conftest.o.0DHcqI
  dtrace: failed to link script ./conftest.d: failed to link /tmp/conftest.o: ld exited with status 1


Related Issues

  - https://bugs.python.org/issue29077

Reviewed_by:
Approved by:
Differential_Revision: D31489
Test Plan
  • The port builds fine in poudriere.
  • dtrace -ln 'python*:::' shows some available probes when python3.10 is executing.

Diff Detail

Repository
rP FreeBSD ports repository
Lint
No Lint Coverage
Unit
No Test Coverage
Build Status
Buildable 40960
Build 37849: arc lint + arc unit

Event Timeline

0mp requested review of this revision.Aug 10 2021, 9:33 AM
lang/python310/Makefile
18

Mmm, surprised by this. CPython I don't believe has ever required or had GNU make specific features. Can you identify the introduction of this? Can we fix this with a patch for portability, such that we can submit it upstream? Not upstreamed local changes has been a source of many issues for us in the distant past that we have only recently come out of

74

Empty line before this. Was DTRACE_WITH=dtrace tested? Did it work, not work? WITH|ENABLE are preferred over ON (particularly without an OFF), as it can cause issues with systems detecting features and building with them, even when the OPTION has been disabled

lang/python310/files/patch-configure
7

Is removing the test the correct/complete resolution for upstream, or does this need a freebsd specific call, or a !freebsd specific condition?

0mp edited the summary of this revision. (Show Details)
0mp added inline comments.
lang/python310/Makefile
18

It's a tricky one. bmake seems to misunderstand the desired meaning of $< in the Python makefiles. GNU make does the right thing. In a way, we could fix it but the problem is that you'll never be sure if you actually fixed all the issues.

I can prepare a patch that could be submitted upstream, but from my limited perspective it seems to be an unnecessary uphill battle. But then, I'm not a Python maintainer and you surely understand the dynamics better.

74

I didn't add a newline here, because DEBUG was already in one block with IPV6. I'll add a newline. Should I add one after as well?

Also, WITH should work. I'll give it a try.

lang/python310/files/patch-configure
7

The removal works for FreeBSD. If we want to upstream the patch, we must fix the test instead.

From what I understand, the whole point of this test is to check if the dtrace binary supports -G. I'm not sure why the current test is failing to do the right thing on FreeBSD. I'm also not sure what an elegant fix would be.

Forgot one other thing in previous review. dtrace has been an option in Python for a (long) while, and we would want to add this option across all relevent Python ports

lang/python310/Makefile
18

Not to say this isn't a gmake'ism, or not an issue, but we would expect other failures of this kind across the board and in the past due to this issue, and I can't recall any of the same kind (I may be wrong, or forgetting).

Could of additional things for this issue:

  • If gmake is (currently) needed, we should do it across the board for all ports, unless this is a (recent) version+ specific issue (need confirmation).
  • Was there an actual failure/impact due to the mentioned syntax when not using gmake? Is it scoped to a/the dtrace option?
  • What might a patch to this look like, to obviate the need to switch to gmake completely, that we may be able to send up stream. Upstream is very approachable and we have a good relationship
74

Unless you'd like to isolate that (non functional, logically different) changeset into a separate one, which would be preferable, particularly if it affects all/other python port versions. If so, feel free to not worry about it here.

lang/python310/files/patch-configure
7

I see an identical error in pkg-fallout (13arm64)

https://www.mail-archive.com/freebsd-pkg-fallout@freebsd.org/msg1771010.html

What was the version/arch you saw this linker error on?

Can you see if/where else this fails in the same way for {11,12,13,i386,amd64} at least

koobs requested changes to this revision.Oct 8 2021, 7:43 AM
koobs edited the summary of this revision. (Show Details)
This revision now requires changes to proceed.Oct 8 2021, 7:44 AM
lang/python310/files/patch-configure
7

We stop getting the dtro test error if you make the dtrace call with -64 or -32 flags (it doesn't matter which). Happily this solution also works on linux, so there is a high chance of it being accepted by upstream.

I am happy to make an upstream contribution so that the fix flows in organically, stay tuned.