lang/python*: Install GDB debugging scripts

[cem]

Summary

Commit log message:

lang/python*: Install GDB debugging scripts

Users with a GDB that supports Python extensions [1] will automatically
load the extra debugging extensions when debugging programs that are linked
with libpythonX.Y.so.foo.

This enables extensions like 'py-bt' and 'py-frame' as described in
the Fedora Wiki Article: Easier Python Debugging [2], which can be
very useful for debugging Python program state from crashes in
C extensions, for example.

[1] PYTHON option enabled in devel/gdb
[2] https://fedoraproject.org/wiki/Features/EasierPythonDebugging 

PR: 203021
Reviewed_by: mat, koobs (Python)
Approved by: koobs (Python)
Differential_Revision: D10398

Test Plan

• testport: Pending (poudriere)

[rfyu28uyeg_snkmail.com]

I've seen other -gdb.py files installed to share/gdb/auto-load (such as /usr/local/share/gdb/auto-load/usr/local/lib/libglib-2.0.so.0.5000.2-gdb.py from the devel/glib20 port). Should the python -gdb.py file be installed there, too?

lang/python27/Makefile

128 ↗

(On Diff #27439)

Use ${INSTALL_DATA} instead of 'install -m444' (same for python3* versions)

[cem]

In D10398#215437, @rfyu28uyeg_snkmail.com wrote:

I've seen other -gdb.py files installed to share/gdb/auto-load (such as /usr/local/share/gdb/auto-load/usr/local/lib/libglib-2.0.so.0.5000.2-gdb.py from the devel/glib20 port). Should the python -gdb.py file be installed there, too?

Does that mean the port would need a runtime dependency on gdb for the /usr/local/share/gdb directory? If not, seems like a good idea to me.

[mat]

lang/python27/Makefile

128 ↗

(On Diff #27439)

No, INSTALL_DATA means 644. (But, yeah, it must use INSTALL_DATA, not install direclty.

[rfyu28uyeg_snkmail.com]

In D10398#215440, @cem wrote:

Does that mean the port would need a runtime dependency on gdb for the /usr/local/share/gdb directory? If not, seems like a good idea to me.

No a dependency on gdb should not be required.

I don't know if share/gdb/auto-load/ is the desired standard place. FWIW, linux (e.g., Fedora) puts some -gdb.py files there:

$ locate gdb.py
/usr/bin/mono-gdb.py
/usr/bin/mono-sgen-gdb.py
/usr/share/gdb/auto-load/usr/lib/libstdc++.so.6.0.21-gdb.py
/usr/share/gdb/auto-load/usr/lib/libstdc++.so.6.0.21-gdb.pyc
/usr/share/gdb/auto-load/usr/lib/libstdc++.so.6.0.21-gdb.pyo
/usr/share/gdb/auto-load/usr/lib64/libisl.so.13.1.0-gdb.py
/usr/share/gdb/auto-load/usr/lib64/libstdc++.so.6.0.21-gdb.py
/usr/share/gdb/auto-load/usr/lib64/libstdc++.so.6.0.21-gdb.pyc
/usr/share/gdb/auto-load/usr/lib64/libstdc++.so.6.0.21-gdb.pyo

[rfyu28uyeg_snkmail.com]

lang/python27/Makefile

128 ↗

(On Diff #27439)

Indeed. Although if for some reason one really wanted 0444, you should add to pkg-plist anyway. In this case 444 is probably right (not really a file to be edited), but not worth the trouble to override the default 644 IMO.

[cem]

In D10398#215449, @rfyu28uyeg_snkmail.com wrote:

In D10398#215440, @cem wrote:

Does that mean the port would need a runtime dependency on gdb for the /usr/local/share/gdb directory? If not, seems like a good idea to me.

No a dependency on gdb should not be required.

Ok.

I don't know if share/gdb/auto-load/ is the desired standard place. FWIW, linux (e.g., Fedora) puts some -gdb.py files there:

$ locate gdb.py
/usr/bin/mono-gdb.py
/usr/bin/mono-sgen-gdb.py

Right — they can either be co-located with the relevant binaries, or in a couple automatically loaded places.

/usr/share/gdb/auto-load/usr/lib/libstdc++.so.6.0.21-gdb.py
/usr/share/gdb/auto-load/usr/lib/libstdc++.so.6.0.21-gdb.pyc
/usr/share/gdb/auto-load/usr/lib/libstdc++.so.6.0.21-gdb.pyo
/usr/share/gdb/auto-load/usr/lib64/libisl.so.13.1.0-gdb.py
/usr/share/gdb/auto-load/usr/lib64/libstdc++.so.6.0.21-gdb.py
/usr/share/gdb/auto-load/usr/lib64/libstdc++.so.6.0.21-gdb.pyc
/usr/share/gdb/auto-load/usr/lib64/libstdc++.so.6.0.21-gdb.pyo

Fedora puts the Python -gdb.py stuff here:

/usr/lib/debug/usr/lib64/libpython2.7.so.1.0.debug-gdb.py
/usr/lib/debug/usr/lib64/libpython2.7_d.so.1.0.debug-gdb.py

But /usr/lib/debug is still somewhat immature in FreeBSD, I'd prefer either co-located or in the /usr/share/gdb directory. (By the way, is it ok for a port to install into such a path outside of LOCALBASE?)

The reason Fedora gives for not co-locating the script with the shared library is that it "generates noise in ldconfig:" https://bugzilla.redhat.com/show_bug.cgi?id=562980 . We could instead teach FreeBSD ldconfig to ignore such scripts, if it whines about them.

[cem]

Use ${INSTALL_DATA} instead of manual install.
Leave installed location alone while it is under discussion.

[rfyu28uyeg_snkmail.com]

Installing under ${PREFIX} (typically LOCALBASE) is generally preferred. The most common things installed outside of PREFIX/LOCALBASE are things under /var. Many kernel module ports install to /boot/modules.

FreeBSD's ldconfig doesn't whine for non-shlib files. It has to work a wee bit more to ignore them, but that's pretty minimal.

Sounds like there's no clear standard location. I think co-located with the python lib is probably fine for now.

[cem]

In D10398#215886, @rfyu28uyeg_snkmail.com wrote:

Installing under ${PREFIX} (typically LOCALBASE) is generally preferred. The most common things installed outside of PREFIX/LOCALBASE are things under /var. Many kernel module ports install to /boot/modules.

Yeah, that makes sense.

FreeBSD's ldconfig doesn't whine for non-shlib files. It has to work a wee bit more to ignore them, but that's pretty minimal.

Ok.

Sounds like there's no clear standard location. I think co-located with the python lib is probably fine for now.

If you think it's okay as-is, can you mark the review as "Accepted?" Thanks!

[cem]

Ping? Can I commit this?

[mat]

In D10398#218054, @cem wrote:

Ping? Can I commit this?

As soon as someone from python says yes. I added the blocking reviewer for a reason, so that you know when it is ok to commit this. Removing it does not change that fact.

[cem]

Nevertheless, it's been two weeks and no one has actually reviewed it... so it's a de facto veto. The ports team desperately needs to grow and this is not an effective way to grow it.

[koobs]

@cem The Python port changes look fine, though I can't comment on the correct location for debug/gdb bits in the short or long-term for FreeBSD in general.

If you'd like to commit this change, please:

• Update the SUMMARY field to reflect what the exact proposed commit log message would be and include all the relevant Metadata: fields such as Reviewed_by:, Approved by: Differential Revision:, et al.
• Confirm these changes pass QA (poudriere in particular). Add these details to the TEST PLAN section of the review

If you don't have an adequate QA environment setup now, just let me know and I'll take care of the commit, but a complete commit log message including rationale/explanation as well as describing the use-case/benefit for users would be great so I can just copy pasta.

[cem]

In D10398#218436, @koobs wrote:

@cem The Python port changes look fine, though I can't comment on the correct location for debug/gdb bits in the short or long-term for FreeBSD in general.

I think it's an ok location for now. If we change our minds later, we can always fix it then.

If you'd like to commit this change, please:

I don't care who commits it, I just want FreeBSD to be a little bit better and I'm trying to share knowledge of the awesome Python GDB integration work that's been done.

• Update the SUMMARY field to reflect what the exact proposed commit log message

It's already that. (The first line is in the title, though.)

would be and include all the relevant Metadata: fields such as Reviewed_by:, Approved by: Differential Revision:, et al.

Well, so far, all of those are empty. I fill them in when I commit based on the results of the review.

• Confirm these changes pass QA (poudriere in particular). Add these details to the TEST PLAN section of the review

If you don't have an adequate QA environment setup now, just let me know

I don't have a pourdriere environment set up. My FreeBSD machine has 4.5 GB of free disk space and I can't just install a bunch of jails. Are there any cluster resources for poudriere testing?

Obviously, this change should have no functional impact on dependent ports. It's just dropping an extra file that shouldn't conflict with anything.

and I'll take care of the commit,

I would appreciate it.

but a complete commit log message including rationale/explanation as well as describing the use-case/benefit for users would be great so I can just copy pasta.

That's already present in the review title/description. Python developers can more easily debug their programs from GDB. This is most useful when you have a crash in a C extensions, or are debugging Python internals. But you could also gcore a running Python and inspect it with GDB with these extensions. Really, check out the URL from the description, it's quite neat.

[koobs]

In D10398#218437, @cem wrote:

In D10398#218436, @koobs wrote:

@cem The Python port changes look fine, though I can't comment on the correct location for debug/gdb bits in the short or long-term for FreeBSD in general.

I think it's an ok location for now. If we change our minds later, we can always fix it then.

If you'd like to commit this change, please:

I don't care who commits it, I just want FreeBSD to be a little bit better and I'm trying to share knowledge of the awesome Python GDB integration work that's been done.

• Update the SUMMARY field to reflect what the exact proposed commit log message

It's already that. (The first line is in the title, though.)

would be and include all the relevant Metadata: fields such as Reviewed_by:, Approved by: Differential Revision:, et al.

Well, so far, all of those are empty. I fill them in when I commit based on the results of the review.

• Confirm these changes pass QA (poudriere in particular). Add these details to the TEST PLAN section of the review

If you don't have an adequate QA environment setup now, just let me know

I don't have a pourdriere environment set up. My FreeBSD machine has 4.5 GB of free disk space and I can't just install a bunch of jails. Are there any cluster resources for poudriere testing?

Obviously, this change should have no functional impact on dependent ports. It's just dropping an extra file that shouldn't conflict with anything.

and I'll take care of the commit,

I would appreciate it.

but a complete commit log message including rationale/explanation as well as describing the use-case/benefit for users would be great so I can just copy pasta.

That's already present in the review title/description. Python developers can more easily debug their programs from GDB. This is most useful when you have a crash in a C extensions, or are debugging Python internals. But you could also gcore a running Python and inspect it with GDB with these extensions. Really, check out the URL from the description, it's quite neat.

Reviews are for commit metadata as well so they ought to be as verbatim as possible, particularly for non-port committers requiring extra approval or where the committer is not the maintainer. I've updated the SUMMARY as an example for future reviews, can you please re-read it and confirm that it still has correct/accurate intent and semantics.

I'll take care of the QA and commit from there once you confirm the messaging is OK.

[cem]

Reviewed_by: mat, koobs (Python)

Mat didn't actually review it, did he?

Approved by: koobs (Python)
Differential_Revision: D10398

What's with the extra underscores in the metadata?

[koobs]

@cem also take the differential again (undo the abandoning) so the changeset/review remains attributed to you

[koobs]

@cem The status of @mat's review appears to request/require further changes. I'm not sure if this is still the case, and he'll need to update the status

[cem]

In D10398#218441, @koobs wrote:

Reviews are for commit metadata as well so they ought to be as verbatim as possible, particularly for non-port committers requiring extra approval or where the committer is not the maintainer. I've updated the SUMMARY as an example for future reviews, can you please re-read it and confirm that it still has correct/accurate intent and semantics.

Looks good to me (modulo earlier nits).

I'll take care of the QA and commit from there once you confirm the messaging is OK.

Thanks!

[koobs]

In D10398#218442, @cem wrote:

Reviewed_by: mat, koobs (Python)

Mat didn't actually review it, did he?

I'm going by the review status says requires changes (x) by mat to mean it was reviewed (changes requested implies it was)

Approved by: koobs (Python)
Differential_Revision: D10398

What's with the extra underscores in the metadata?

Reviewed by appears to be a reserved word/phrase in Phabricator when they at the beginning of a line, even within code blocks and Phabricator wont allow you to save updates to revision without changing those property names. Differential Revision is also reserved.

[cem]

In D10398#218444, @koobs wrote:

@cem also take the differential again (undo the abandoning) so the changeset/review remains attributed to you

Meh. I've had a horrible experience trying to improve ports without a commit bit. In the future, I'll just maintain a ports overlay and stop submitting my improvements back upstream. It just isn't worth it when portmgr is so hostile to outsiders.

@cem The status of @mat's review appears to request/require further changes. I'm not sure if this is still the case, and he'll need to update the status

So far all he's done is sandbag the review. If Python team approves and he has no concrete objections, I'm not sure why the review needs to be blocked on him.

[koobs]

In D10398#218447, @cem wrote:

In D10398#218444, @koobs wrote:

@cem also take the differential again (undo the abandoning) so the changeset/review remains attributed to you

Meh. I've had a horrible experience trying to improve ports without a commit bit. In the future, I'll just maintain a ports overlay and stop submitting my improvements back upstream. It just isn't worth it when portmgr is so hostile to outsiders.

Please don't do that, the vast majority of ports committers (including myself) want to make it as easy as possible for people to submit changes, ideally ending up with more src people getting ports bits after mentorship (unofficial or otherwise). For all diffs I review, I treat them as if they are from a mentee (full review), to encourage continual improvement, even from existing committers. This should not be seen as hoops to jump through or barriers to entry.

@cem The status of @mat's review appears to request/require further changes. I'm not sure if this is still the case, and he'll need to update the status

So far all he's done is sandbag the review. If Python team approves and he has no concrete objections, I'm not sure why the review needs to be blocked on him.

Mat has subscribed himself to all diffs that fall under the ports tree (as anyone can do), and giving him the benefit of the doubt, its so that if a new diff has been created with no 'reviewers' by the submitter, that the changeset can receive some (not zero) attention and not fall through the cracks or demotivate contributors. Whether the execution matches the goal is a separate issue.

I see that the INSTALL_DATA change requested (by @mat) has been made in the diff, and only the state of the review needs to change, but the commit is not (administratively) blocked on that.

[mat]

In D10398#218447, @cem wrote:

In D10398#218444, @koobs wrote:

@cem The status of @mat's review appears to request/require further changes. I'm not sure if this is still the case, and he'll need to update the status

So far all he's done is sandbag the review. If Python team approves and he has no concrete objections, I'm not sure why the review needs to be blocked on him.

What I did was:

1. review the change, and comment on it
2. add the appropriate group to the review so that it can be reviewed by the people who have a clue.

This tool, phabricator, here, is a code review tool, which means it is there so that changes can be reviewed so that the submitter improves his ports skills (or src, or doc skills).

If you did not mean to ask for review of your code and wanted to open a bug report, so that the appropriate maintainer could work on it and commit it after it met with the ports standards, you should have done so on our bug tracking software, which is at https://bugs.freebsd.org/submit/.

[cem]

In D10398#219056, @mat wrote:

1. add the appropriate group to the review so that it can be reviewed by the people who have a clue.

It's a stupidly simple change that any ports committer could review for sanity. Hell, a dozen would review it immediately if I just committed it. It's either fine, or not fine. No one reviewed it for weeks.

This tool, phabricator, here, is a code review tool, which means it is there so that changes can be reviewed so that the submitter improves his ports skills (or src, or doc skills).

Don't be an ass, mat. I've used phabricator dozens of times for my own reviews and reviewed hundreds of others' changes. I'm familiar with its use.

If you did not mean to ask for review of your code and wanted to open a bug report, so that the appropriate maintainer could work on it and commit it after it met with the ports standards, you should have done so on our bug tracking software, which is at https://bugs.freebsd.org/submit/.

Don't be an ass, mat. I requested a review and never got one. I'm familiar with bugzilla and you know that. This is just another example of your hostility to non-committers.

[koobs]

Found some issue in QA, will add comments here where appropriate. No changes necessary, I've fixed them in my working copy. Will land them once QA is complete.

[koobs]

lang/python33/Makefile

121 ↗

(On Diff #27444)

Doesn't match pkg-plist entry, missing ${ABIFLAGS} suffix of filename (libpython3.3m.so.1-gdb.py)

lang/python34/Makefile

124 ↗

(On Diff #27444)

Doesn't match pkg-plist entry, missing ${ABIFLAGS} suffix of filename (libpython3.4m.so.1-gdb.py)

lang/python35/Makefile

132 ↗

(On Diff #27444)

Using package plist (pkg-plist) %%substitution%% syntax, should be the following instead:

${STAGEDIR}${PREFIX}/lib/libpython${PYTHON_VER}${ABIFLAGS}.so.1.0-gdb.py

lang/python36/Makefile

128 ↗

(On Diff #27444)

Using package plist (pkg-plist) %%substitution%% syntax, should be the following instead:

${STAGEDIR}${PREFIX}/lib/libpython${PYTHON_VER}${ABIFLAGS}.so.1.0-gdb.py

[koobs]

Package changes (new file), bump PORTREVISION accordingly