On 09/18/2017 06:56 PM, Mark Wielaard wrote:
Debuginfo packages are useful without debugsource files. But it is often
useful to also have the debugsourc files. So make debuginfo packages that
don't contain sources recommend the debugsource package (or the main
debuginfo one if the sources are not in a separate debugsource package).
Rename addPackageRequires to addPackageRequiresRecommends with an argument
to indicate whether the package is required or recommended. Move up so it
can be used with filterDebuginfoPackage. Add Package dbgsrc as argument to
filterDebuginfoPackage so it can be added as recommendation. Add a new
function findDebugsourcePackage. Use it to add a requires to the main
debuginfo file and/or the debuginfo subpackages.
Extend the various rpmbuild.at tests that create debugsource and/or
debuginfo subpackages to check the debugsource (or main debuginfo)
package is recommended.
No objections to the functionality, but some style nits + suggestions
below...
Signed-off-by: Mark Wielaard <m...@klomp.org>
---
build/files.c | 59 ++++++++++++++++++++++++++++++++++++++++---------------
tests/rpmbuild.at | 46 ++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 88 insertions(+), 17 deletions(-)
diff --git a/build/files.c b/build/files.c
index 5e84532..86ec80a 100644
--- a/build/files.c
+++ b/build/files.c
@@ -2773,6 +2773,21 @@ static void patchDebugPackageString(Package dbg, rpmTag
tag, Package pkg, Packag
_free(newsubst);
}
+/* add a requires or recommends for package "to" into package "from". */
+static void addPackageRequiresRecommends(Package from, Package to,
+ bool recommends)
+{
+ const char *name;
+ char *evr, *isaprov;
+ enum rpmTag_e tag = recommends ? RPMTAG_RECOMMENDNAME : RPMTAG_REQUIRENAME;
+ name = headerGetString(to->header, RPMTAG_NAME);
+ evr = headerGetAsString(to->header, RPMTAG_EVR);
+ isaprov = rpmExpand(name, "%{?_isa}", NULL);
+ addReqProv(from, tag, isaprov, evr, RPMSENSE_EQUAL, 0);
+ free(isaprov);
+ free(evr);
+}
Moving an entire function while also changing it is a bit of a no-no,
because it's makes it hard to see what actually changed. Moving things
around also obfuscates git history, which is why I prefer adding a
prototype to the top of the file instead. But if you need to move stuff
around (such as different file entirely), do so in a separate commit
that doesn't change anything and also state this in the commit message.
Also please split refactoring changes like this to separate commits:
one commit that does the necessary enhancements/changes to
addPackageRequires() and updates existing callers, and another commit to
add the new functionality. It just makes things the diffs so much more
obvious and nicer to review.
+
/* create a new debuginfo subpackage for package pkg from the
* main debuginfo package */
static Package cloneDebuginfoPackage(rpmSpec spec, Package pkg, Package
maindbg)
@@ -2805,7 +2820,8 @@ static Package cloneDebuginfoPackage(rpmSpec spec,
Package pkg, Package maindbg)
/* collect the debug files for package pkg and put them into
* a (possibly new) debuginfo subpackage */
static void filterDebuginfoPackage(rpmSpec spec, Package pkg,
- Package maindbg, char *buildroot, char *uniquearch)
+ Package maindbg, Package dbgsrc,
+ char *buildroot, char *uniquearch)
{
rpmfi fi;
ARGV_t files = NULL;
@@ -2914,6 +2930,8 @@ static void filterDebuginfoPackage(rpmSpec spec, Package
pkg,
else {
Package dbg = cloneDebuginfoPackage(spec, pkg, maindbg);
dbg->fileList = files;
+ /* Recommend the debugsource package (or the main debuginfo). */
+ addPackageRequiresRecommends(dbg, dbgsrc ?: maindbg, true);
Omitting middle operand of conditional expressions is a gcc extension,
those are best avoided in general and especially when there's no actual
need to use one.
[...]
@@ -3038,6 +3056,12 @@ rpmRC processBinaryFiles(rpmSpec spec, rpmBuildPkgFlags
pkgFlags,
if (rpmExpandNumeric("%{?_unique_debug_names}"))
uniquearch = rpmExpand("-%{VERSION}-%{RELEASE}.%{_arch}", NULL);
}
+ } else if (dbgsrcpkg != NULL) {
+ /* We have a debugsource package, but no debuginfo subpackages.
+ The main debuginfo package should recommend the debugsource one. */
+ Package dbgpkg = findDebuginfoPackage(spec);
+ if (dbgpkg)
+ addPackageRequiresRecommends(dbgpkg, dbgsrcpkg, true);
}
for (pkg = spec->packages; pkg != NULL; pkg = pkg->next) {
@@ -3055,6 +3079,8 @@ rpmRC processBinaryFiles(rpmSpec spec, rpmBuildPkgFlags
pkgFlags,
deplink = extradbg;
if (addDebugSrc(extradbg, buildroot))
deplink = extradbg;
+ if (dbgsrcpkg != NULL)
+ addPackageRequiresRecommends(extradbg, dbgsrcpkg, true);
maindbg = NULL; /* all normal packages processed */
}
@@ -3071,9 +3097,10 @@ rpmRC processBinaryFiles(rpmSpec spec, rpmBuildPkgFlags pkgFlags,
goto exit;
if (maindbg)
- filterDebuginfoPackage(spec, pkg, maindbg, buildroot, uniquearch);
+ filterDebuginfoPackage(spec, pkg, maindbg, dbgsrcpkg,
+ buildroot, uniquearch);
else if (deplink && pkg != deplink)
- addPackageRequires(pkg, deplink);
+ addPackageRequiresRecommends(pkg, deplink, false);
Hmm, I think this would be both more obvious and generally useful if you
just call the function addPackageDeps() and pass the relevant dependency
tag instead of true/false for require/recommend, eg
addPackageDeps(pkg, deplink, RPMTAG_REQUIRENAME);
addPackageDeps(extradbg, dbgsrcpkg, RPMTAG_RECOMMENDNAME);
vs
addPackageRequiresRecommends(pkg, deplink, false);
addPackageRequiresRecommends(extradbg, dbgsrcpkg, true);
That way we wont end up with
addPackageRequiresRecommendsConflictsObsoltetes() one day :)
- Panu -
_______________________________________________
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
