Features/debuginfo.mk: Automatically generate a subpackage with debugging info.

[arrowd]

Summary

I'm completely open to the bikeshedding process. The two concerns I'm aware of are

• How to name the subpackage? I'm proposing debuginfo to underline the fact that the package installs only a debugging info files, not a debug flavor of the software itself.

My shell skills are lacking, so there probably also a room for improvement.

Test Plan

Tested with a CMake port sysutils/plasma5-powerdevil and a
simple port sysutils/44bsd-more. Also tested WITH_DEBUGINFO+WITH_DEBUG
combination.

[bapt]

I don t see anything about cflags in the review am I missing something?

[flo]

Mk/Features/debuginfo.mk

8

Typo.
WITH_DEBUGthe

Mk/bsd.port.mk

2653

Something similar needs to be added to Scripts/create-manifest.sh line 89, or rather we need a global pkg-descr for the debuginfo subpackages I think.

Found this while testing with asterisk18

'===> Checking if asterisk18 is already installed
cp: /usr/home/flo/dev/checkouts/portscommit/net/asterisk18/pkg-descr.debuginfo: No such file or directory

[arrowd]

In D43515#992207, @bapt wrote:

I don t see anything about cflags in the review am I missing something?

I hideosly included Features/debug.mk, see the comment.

[mat]

Mk/bsd.port.mk

2653

I'd say DESCR.debuginfo should be defined in the feature, and auto-generated during the staging process, and then contain Debug informations for ${PKGNAME} or something like that.

[arrowd]

Mk/bsd.port.mk

2653

Well, this check happens long before stage stage. So the file would still be non-existant.

... but maybe we keel this special-case check but still define DESCR.debuginfo to an autogenerated file and then it should probably work by the package stage?

[mat]

Mk/bsd.port.mk

2653

Well, the create the file in before patch or something, write it in WRKDIR somewhere.

[arrowd]

• Create DESCR.debuginfo on the fly.
• Fix typo.

[mat]

Mk/Features/debuginfo.mk

30

I thought you said stage was too late for creating that file.

[arrowd]

Mk/Features/debuginfo.mk

30

Turns out it works! But we still need .if !exists(${DESCR.${sp}}) with additional && ${sp} != debuginfo check in bsd.port.mk, otherwise it gets overriden with just category/port/pkg-descr.

[emaste]

The current approach creates multiple .debug directories, while @bapt suggested to use ${LOCALBASE}/lib/debug to mirror /usr/lib/debug.

Note that debuggers will not search in ${LOCALBASE}/lib/debug by default, so although I think that's the "nicer" option I'd say using .debug to start is better.

[emaste]

Mk/Features/debuginfo.mk

6

[arrowd]

• Reuse existing script for generating external debugging symbols.
• devel/gdb: Point to LOCALBASE/lib/debug dir for the debugging symbols.

[arrowd]

• devel/llvm15: Patch lldb to search for debug symbols in LOCALBASE/lib/debug.

[arrowd]

@brooks You might be interested in the LLDB change.

[mat]

In D43515#994013, @emaste wrote:

The current approach creates multiple .debug directories, while @bapt suggested to use ${LOCALBASE}/lib/debug to mirror /usr/lib/debug.

Note that debuggers will not search in ${LOCALBASE}/lib/debug/ by default, so although I think that's the "nicer" option I'd say using .debug to start is better.

Well, debuggers should be taught to go look there. I'm not sure why we can't put files there because nobody looks there, of course nobody looks there, it does not exist yet ;-)
I have no idea how debuggers work, but they probably have some kind of PATH equivalent for looking for debug info, we just need to add /usr/local/lib/debug to it.

[arrowd]

The debuginfo is installed into ${LOCALBASE}/lib/debug/ now and I taught both GDB and LLDB to look into there.

[brooks]

devel/llvm15/files/patch-lldb_source_Symbol_LocateSymbolFile.cpp

25–27 ↗

(On Diff #133462)

This is meaningful different from what is done with gdb. In the gdb case you hardcode the pkg set's LOCALBASE. Here you look up the system wide default. They aren't necessarily the same thing. (For example on CheriBSD we build pure-capability (memory safe) packages with LOCALBASE=/usr/local and a separate package set for ordinary 64-bit binaries with LOCALBASE=/usr/local64 and don't have a per-ABI user.localbase).

Given the lldb is a cross debugger I'm not sure this difference matters much so maybe we would just upstream this patch and not worry about it further.

[arrowd]

devel/llvm15/files/patch-lldb_source_Symbol_LocateSymbolFile.cpp

25–27 ↗

(On Diff #133462)

This is because I want to put the same patch into our base LLDB.

Given the lldb is a cross debugger I'm not sure this difference matters much so maybe we would just upstream this patch and not worry about it further.

Yes, I also wanted to upstream this, if you think it is OK.

[flo]

My issues have been resolved. LGTM. I was able to successfully test this.

[emaste]

Well, debuggers should be taught to go look there.

Indeed, we should probably do that for lldb by implementing the equivalent of GDB's configure-time --with-additional-debug-dirs (that I just learned about from this review).

devel/llvm15/files/patch-lldb_source_Symbol_LocateSymbolFile.cpp

25–27 ↗

(On Diff #133462)

For this case we probably want to search in both paths?

[emaste]

Mk/Features/debuginfo.mk

7

I assume this is fixed in your local tree?

[brooks]

devel/llvm15/files/patch-lldb_source_Symbol_LocateSymbolFile.cpp

25–27 ↗

(On Diff #133462)

For this case we probably want to search in both paths?

Yeah, I think so.

[arrowd]

• Fix typo.

[arrowd]

devel/llvm15/files/patch-lldb_source_Symbol_LocateSymbolFile.cpp

25–27 ↗

(On Diff #133462)

Sorry, I didn't get it, what "both paths"?

[brooks]

devel/llvm15/files/patch-lldb_source_Symbol_LocateSymbolFile.cpp

25–27 ↗

(On Diff #133462)

user.localbase and the value of LOCALBASE for a given package set aren't necessarily the same thing. Ideally lldb should look in both if they differ.

[arrowd]

devel/llvm15/files/patch-lldb_source_Symbol_LocateSymbolFile.cpp

25–27 ↗

(On Diff #133462)

This is that LOCALBASE exist only when building from Ports. When LLDB is also built from base, we don't have this variable at the compile time.

[emaste]

devel/llvm15/files/patch-lldb_source_Symbol_LocateSymbolFile.cpp

25–27 ↗

(On Diff #133462)

That said this issue does not need to be resolved in advance of this change going in, we can iterate on lldb (and gdb) separately

[tcberner]

Assuming it works lgtm

[emaste]

looks good, I'm very happy to see this happening

still an extract -> extracted to update

the lldb change should be split out and handled separately, and we need to address this in several places (upstream lldb, in the base system, multiple ports)