Skip to content

Commit

Permalink
Add proper program logic for debuginfo enablement
Browse files Browse the repository at this point in the history
All these years, enabling debuginfo has required distros to hijack the
spec %install section with a macro like this:

    %install %{?_enable_debug_packages:%{?buildsubdir:%{debug_package}}}\
    %%install\
    %{nil}

This for a widely used, longtime upstream supported feature is just
gross, and also very non-obvious, feeble and whatnot. And totally
prevents the new append/prepend options from being used with %install.

Replace the logic parts with actual C code where the logic is more
straightforward to follow, taking advantage of dynamic spec generation
so debuginfo packages are only ever generated during an actual build.

There's a crazy amount of details buried in such a tiny piece, commented
in the code now.

A noteworthy point here is that the presence of the old %install hack now
causes an explicit failure, something like:

    error: line 4: %package debuginfo: package foo-debuginfo already exists
    error: parsing failed

This explicit break is very much intentional as we want distros to remove
those %install hacks from their macro files.

Fixes: #2204
Fixes: #1878
  • Loading branch information
pmatilai committed Apr 15, 2024
1 parent a385821 commit b1fe2d2
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 10 deletions.
58 changes: 55 additions & 3 deletions build/build.c
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,60 @@ static rpmRC doBuildDir(rpmSpec spec, int test, StringBuf *sbp)
return rc;
}

/* Write out a specpart for debuginfo packages if the stars align just so. */
static rpmRC doDebugPackage(rpmSpec spec)
{
rpmRC rc = RPMRC_OK;

/* noarch packages don't get debuginfo. */
if (rstreq(headerGetString(spec->packages->header, RPMTAG_ARCH), "noarch"))
return rc;

/* debuginfo is only enabled if buildsubdir is defined. */
char *t = rpmExpand("%{?buildsubdir:%{macrobody:debug_package}}", NULL);

/*
* Finally, LOTS of things rely on defining %debug_package to %{nil}
* for disabling debug package generation. We're not expanding the
* %debug_package body above to avoid double-expansion side-effects,
* so we need to test for the %{nil} override literally.
*/
if (t && *t && strcmp(t, "%{nil}")) {
char *p = rpmGetPath("%{specpartsdir}/rpm-debuginfo.specpart", NULL);
FILE *f = fopen(p, "w+");
if (f == NULL || fputs(t, f) == EOF)
rc = RPMRC_FAIL;
if (f)
fclose(f);

if (rc) {
rpmlog(RPMLOG_ERR,
_("failed to write debuginfo template: %s: %s\n"),
p, strerror(errno));
} else {
/* debuginfo extraction is further conditioned on __debug_package */
rpmPushMacro(spec->macros, "__debug_package", NULL, "1", RMIL_SPEC);
}
free(p);
}
free(t);
return rc;
}

static rpmRC doInstallScript(rpmSpec spec, int test, StringBuf *sbp)
{
rpmRC rc = RPMRC_OK;
const char *script = rpmSpecGetSection(spec, RPMBUILD_INSTALL);

if (script && rpmExpandNumeric("%{?_enable_debug_packages}"))
rc = doDebugPackage(spec);

if (!rc)
rc = doScript(spec, RPMBUILD_INSTALL, "%install", script, test, sbp);

return rc;
}

static int buildSpec(rpmts ts, BTA_t buildArgs, rpmSpec spec, int what)
{
int rc = RPMRC_OK;
Expand Down Expand Up @@ -464,9 +518,7 @@ static int buildSpec(rpmts ts, BTA_t buildArgs, rpmSpec spec, int what)
goto exit;

if ((what & RPMBUILD_INSTALL) &&
(rc = doScript(spec, RPMBUILD_INSTALL, "%install",
rpmSpecGetSection(spec, RPMBUILD_INSTALL),
test, sbp)))
(rc = doInstallScript(spec, test, sbp)))
goto exit;

if (((what & RPMBUILD_INSTALL) || (what & RPMBUILD_PACKAGEBINARY)) &&
Expand Down
3 changes: 0 additions & 3 deletions macros.in
Original file line number Diff line number Diff line change
Expand Up @@ -195,11 +195,8 @@ package or when debugging this package.\
%{nil}

%debug_package \
%ifnarch noarch\
%global __debug_package 1\
%_debuginfo_template\
%{?_debugsource_packages:%_debugsource_template}\
%endif\
%{nil}

%_langpack_template() \
Expand Down
4 changes: 0 additions & 4 deletions tests/data/macros.debug
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,6 @@
%{__os_install_post}\
%{nil}

%install %{?_enable_debug_packages:%{?buildsubdir:%{debug_package}}}\
%%install\
%{nil}

# Should missing buildids terminate a build?
%_missing_build_ids_terminate_build 1

Expand Down

0 comments on commit b1fe2d2

Please sign in to comment.