[pacman-dev] [PATCH] [makepkg] split debug support

[Andres P]

On Sat, May 22, 2010 at 10:24:39AM +1000, Allan McRae wrote:
> General comments:
> 
> 1) Why do we need a $dbgdir? It is creating a package so just reuse
> pkgdir.   The way I see the layout is:
> 
> $startdir/pkg/pkgname
>              /pkgname-debug
> 
> Split packages already use the subdir in $startdir/pkg approach and
> I see no reason not to extend it for the single package approach.
> In fact, that is basically the only difference between single and
> split packages so it would unify that code.  As part of that
> unification, we would get the backing up of variables before
> entering the package function so we could just override them as
> needed for the debug package creation.
>
If you're proposing that every package gets treated as split; i.e.,
pkgname always becomes an array, then I agree. That is the cleanest
route. Having pkg/built_sources become pkg/pkgname/built_sources for
_all_ packages would mean less complication in makepkg.

Until then, the split dbg is needed since the current model can't make
assumptions about pkg's first level contents when working with non-split
packages.

It would make sense to stall this until this gets implemented.

> 2) I would much prefer having a flag (--dbgpkg?) over the adding of
> an option (perhaps with a default setting in makepkg.conf for those
> that want to always build debug packages).  Adding an option in a
> PKGBUILD "forces" everyone to use it given it overrides makepkg.conf
> settings.
>
I did away with the flag because it happened to be the only option that
had both an entry in OPTIONS and a --flag. It should be easy to place it
again.

You are saying it should be a boolean in makepkg.conf instead of OPTIONS?

I agree with that, but it doesn't make sense to continue allowing strip
to override as you explained, though. It's more invasive than splitdbg.

And some PKGBUILDS even override settings outside of the scope of
options=(), like PKGEXT=.pkg.tar.gz. This is unavoidable with the
current model.

It would be the first option with both a flag and an entry in
makepkg.conf. That means new logic to handle precedence.

> 3) How does this interact with !strip?
>
splitdbg always takes precedence over strip

This is from my patch, line 893, patched source:
    if [[ $(check_option splitdbg) = "y" ]]; then
        ...
    elif [[ $(check_option strip) = "y" ]]; then
        ...
    fi

splitdbg must be disabled explicitely; disabling strip alone won't work.

Also notice how only _one_ of them may run.

Implementing a scheme where OPTIONS=(splitdbg) only makes the debug
symbol and OPTIONS=(strip) takes exclusive care of the stripping would
be riddled with checks, since objcopy needs to link to binaries and it
doesn't make sense for those that haven't been stripped. It'd be slow as
molasses.

> 4) How does this interact with "makepkg -i"
> 
It does not install the -debug packages w/makepkg -i

Once 1) gets addressed and -debug packages get added into the pkgname
array or a similar metric, this could take place.

> 5) Should this only work with packages that are not arch=any? I.e.
> in a split package with some binary and some "any" packages, only
> the binary packages should have debug packages created.  At the
> moment it looks like it will create an empty package.
> 
Good thinking; I'll add an arch check for the next ver. But you're wrong
in assuming it creates an empty package.

line 1108:
	[[ -n $(find "$dbgdir" -type f -name \*.debug -print -quit) &&
		$(check_option splitdbg) = "y" ]] || return 0

If it doesn't find .debug files in startdir/dbg it won't proceed.

This is one of the benefits of having a dedicated dir, although the
search could be altered to -path pkg/*-debug/, or similar.

> 6) I do not understand what secure_dir is needed for?  I do not
> think that this was part of the original patch.
> 
The main complaint about the original patch was repetition.

Notice how my patch did away with tons of lines with:
    mkdir dir
    chmod a-s dir

It's more maintainable. It's related to the patch because ./dbg gets
created this way.
> Allan
> 

-- 
Andres P