Bug 1479198 - debuginfo generation excludes source files if they are outside the directory created by %setup (%buildsubdir)
debuginfo generation excludes source files if they are outside the directory ...
Status: NEW
Product: Fedora
Classification: Fedora
Component: rpm (Show other bugs)
27
Unspecified Unspecified
unspecified Severity unspecified
: ---
: ---
Assigned To: packaging-team-maint
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2017-08-08 02:38 EDT by Dan Callaghan
Modified: 2017-08-15 02:54 EDT (History)
7 users (show)

See Also:
Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed:
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Dan Callaghan 2017-08-08 02:38:53 EDT
Version-Release number of selected component (if applicable):
rpm-4.13.0.1-39.fc27

How reproducible:
somewhat easily

Steps to Reproduce:
1. Build xmonad-log-applet in rawhide

Actual results:
Build fails.
https://koji.fedoraproject.org/koji/taskinfo?taskID=20850297

[...]
Processing files: xmonad-log-applet-debugsource-2.1.0-12.fc27.x86_64
RPM build errors:
error: Empty %files file /builddir/build/BUILD/xmonad-log-applet-2.1.0/debugsourcefiles.list
    Empty %files file /builddir/build/BUILD/xmonad-log-applet-2.1.0/debugsourcefiles.list

Expected results:
Should produce -debuginfo for each subpackage, and combined -debugsource.

Additional info:
The xmonad-log-applet package builds two different copies of the source code, one for MATE and one for XFCE. It produces two subpackages, xmonad-log-applet-xfce and xmonad-log-applet-mate. In particular, it does *not* produce any binary package called xmonad-log-applet using the semi-well-known trick of not specifying any %files for the main package.

http://pkgs.fedoraproject.org/rpms/xmonad-log-applet/blob/9bace5e89ae6e8b1851ed9b34faf820f44d6ad0b/f/xmonad-log-applet.spec

Before Fedora 27 this correctly produces an xmonad-log-applet-debuginfo package, which contains sources and debug symbols for both of the subpackages:

$ sudo dnf --enablerepo=\*debuginfo repoquery -l xmonad-log-applet-debuginfo
Last metadata expiration check: 0:00:08 ago on Tue Aug  8 16:27:53 2017.
/usr/lib/debug
/usr/lib/debug/.build-id
/usr/lib/debug/.build-id/64
/usr/lib/debug/.build-id/64/79bb45384f81fee994cee226818433f4330cf8
/usr/lib/debug/.build-id/64/79bb45384f81fee994cee226818433f4330cf8.debug
/usr/lib/debug/.build-id/6b
/usr/lib/debug/.build-id/6b/88431bf31b1826504599daa74b040d3fc37177
/usr/lib/debug/.build-id/6b/88431bf31b1826504599daa74b040d3fc37177.debug
/usr/lib/debug/usr
/usr/lib/debug/usr/lib64
/usr/lib/debug/usr/lib64/xfce4
/usr/lib/debug/usr/lib64/xfce4/panel
/usr/lib/debug/usr/lib64/xfce4/panel/plugins
/usr/lib/debug/usr/lib64/xfce4/panel/plugins/xmonad-log-applet.debug
/usr/lib/debug/usr/libexec
/usr/lib/debug/usr/libexec/xmonad-log-applet.debug
/usr/src/debug/mate
/usr/src/debug/mate/main.c
/usr/src/debug/xfce4
/usr/src/debug/xfce4/main.c

I'm guessing that the new -debugsource macros are getting confused because there is no main package xmonad-log-applet, only the two subpackages?
Comment 1 Igor Gnatenko 2017-08-08 02:44:32 EDT
Usually it means that package was compiled without -g, so it is not possible to find source code... Seems like it's not the case.
Comment 2 Igor Gnatenko 2017-08-08 02:56:30 EDT
/usr/src/debug/mate
/usr/src/debug/mate/main.c
/usr/src/debug/xfce4
/usr/src/debug/xfce4/main.c


it seems that because you put sources outside of BUILDDIR...

So, there are 2 things:
- (?) RPM should put sources even if those are not under BUILDDIR
- xmonad-log-applet should not do things like `cp -pr . ../xfce`, it is just wrong..


Mark, any opinion on this?
Comment 3 Dan Callaghan 2017-08-08 03:17:56 EDT
Well it's true that the source tree is being built in a subdirectory named "xfce" or "mate" instead of the conventional "xmonad-log-applet-2.1.0" but I guess there is no real guarantee that the sources are in a particular subdirectory named after %{name}-%{version}. Rpm shouldn't be assuming that, should it?
Comment 4 Dan Callaghan 2017-08-08 03:20:42 EDT
To be clear... The sources are not outside BUILDDIR (not sure which macro that is exactly, but I assume you mean the directory where %prep is invoked). The "cp -pr . ../xfce" command happens after %setup which has already changed into xmonad-log-applet-2.1.0 subdirectory.
Comment 5 Mark Wielaard 2017-08-08 05:44:51 EDT
Yes, this is because the sources are "outside" the directory %setup creates in %prep. The idea is that any such sources might belong to another packages that happened to also be installed in the build root (like include files). And if we did package and install them (which older rpm did) then they might conflict (note that they would end up under /usr/src/debug/mate and /usr/src/debug/xfce which might belong to an [unversioned] mate or xfce package).

So this is indeed a slight change in behaviour. Sorry about that.

Luckily it is easy to resolve:

diff --git a/xmonad-log-applet.spec b/xmonad-log-applet.spec
index 53f85fc..51c7c24 100644
--- a/xmonad-log-applet.spec
+++ b/xmonad-log-applet.spec
@@ -46,27 +46,29 @@ or any other information you send it from your xmonad.hs.
 %patch1 -p1
 cp -pr . ../xfce4
 cp -pr . ../mate
+mv ../xfce4 .
+mv ../mate .
 
 %build
 # The upstream build only allows selecting one desktop environment, but 
-# we would like to package all three. So we run the build three times.
+# we would like to package both. So we run the build two times.
 
-( cd ../xfce4
+( cd xfce4
 %configure --with-panel=xfce4
 make %{?_smp_mflags}
 )
 
-( cd ../mate
+( cd mate
 %configure --with-panel=mate
 make %{?_smp_mflags}
 )
 
 %install
-( cd ../xfce4
+( cd xfce4
 make DESTDIR=%{buildroot} install
 )
 
-( cd ../mate
+( cd mate
 make DESTDIR=%{buildroot} install
 )
 

$ rpm -qp -l /opt/local/src/fedora/xmonad-log-applet/results_xmonad-log-applet/2.1.0/13.fc27/xmonad-log-applet-debugsource-2.1.0-13.fc27.x86_64.rpm 
/usr/src/debug/xmonad-log-applet-2.1.0-13.fc27.x86_64
/usr/src/debug/xmonad-log-applet-2.1.0-13.fc27.x86_64/mate
/usr/src/debug/xmonad-log-applet-2.1.0-13.fc27.x86_64/mate/main.c
/usr/src/debug/xmonad-log-applet-2.1.0-13.fc27.x86_64/xfce4
/usr/src/debug/xmonad-log-applet-2.1.0-13.fc27.x86_64/xfce4/main.c
Comment 6 Panu Matilainen 2017-08-08 06:13:51 EDT
I think catching such hacks is only a good thing, build is and always was supposed to happen inside the directory created by %setup. 

BTW, an alternative approach that avoids unpacking things outside the build directory in the first place:

%prep
%setup -q -c
%patch1 -p0
cp -pr %{name}-%{version} xfce4
cp -pr %{name}-%{version} mate

%build
(cd xfce4
 %configure --with-panel=xfce4
 make %{?_smp_mflags}
 )
[...]
Comment 7 Igor Gnatenko 2017-08-09 03:11:59 EDT
Meantime I've fixed it in better way in fedora package: https://src.fedoraproject.org/rpms/xmonad-log-applet/c/a2f3e6800f3fdff4af97fb10598295398736b418

Wanted to send PR, but it takes forever to fork repository (waited 15 mins).

But for RPM I think we want to show some warnings if it finds sources not under proper builddir. Or even raise some error.
Comment 8 Dan Callaghan 2017-08-09 03:26:02 EDT
Hmm okay. I think it's a bit surprising that the debuginfo generation is assuming you always use %setup and that you never put any sources outside the directory created by %setup.

I assume this is going to break for any package which extracts a tarball "by hand" instead of using %setup too, right?

There are no Fedora guidelines requiring that %setup be used. Nor is there any guideline forbidding copying files outside that directory, as far as I can find.

I think this is a more common case than you might think. Here is another example that I stumbled across today totally by coincidence. mod_wsgi uses %setup to unpack into a mod_wsgi-4.4.8 directory, and then makes a copy of the source beside that in a directory named python3-%{name}-%{version} and does the build twice. In fact *all* Python packages used to be like this before the introduction of %py3_build (which is still optional) and there are probably plenty like this still.

As far as I can see the mod_wsgi debugsources package is now incomplete because the python3 subdirectory is missing. Compare mod_wsgi-debuginfo-4.5.15-1.fc25.x86_64 which has:

[...]
/usr/src/debug/mod_wsgi-4.5.15
/usr/src/debug/mod_wsgi-4.5.15/src
/usr/src/debug/mod_wsgi-4.5.15/src/server
[...]
/usr/src/debug/python3-mod_wsgi-4.5.15-1.fc25
/usr/src/debug/python3-mod_wsgi-4.5.15-1.fc25/src
/usr/src/debug/python3-mod_wsgi-4.5.15-1.fc25/src/server
[...]

whereas built for rawhide mod_wsgi-debugsource-4.5.15-5.fc27.x86_64 only has:

/usr/src/debug/mod_wsgi-4.5.15-5.fc27.x86_64
/usr/src/debug/mod_wsgi-4.5.15-5.fc27.x86_64/src
/usr/src/debug/mod_wsgi-4.5.15-5.fc27.x86_64/src/server
[...]

without the python3- subdirectory. I didn't try loading the rawhide debugsources in gdb but I assume they will fail because they are silently being excluded from the debugsource package. In some ways, it's even *worse* than xmonad-log-applet since at least I noticed the problem because the build failed...
Comment 9 Igor Gnatenko 2017-08-09 03:37:02 EDT
> then makes a copy of the source beside that in a directory named python3-%{name}-%{version} and does the build twice

Yes, this is terribly wrong.


--

Mark, Panu, Florian, do you think we can introduce some warning / error when packages try to go outside of their builddirs?
Comment 10 Mark Wielaard 2017-08-09 07:16:30 EDT
(In reply to Igor Gnatenko from comment #9)
> Mark, Panu, Florian, do you think we can introduce some warning / error when
> packages try to go outside of their builddirs?

In theory, if using unique_debug_src_base and debugsourcefiles, we have all the information to do that, but we still use a very crude/hacky way to actually collect the source files.

Traditionally any sources referenced outside the builddirs couldn't really be used (they were included more or less by accident). Historically debugedit could only shrink source file paths. So it depended on the "depth" of the path used, moving some sources "up" from the rpm setup builddir might result in debugedit just giving up. These days debugedit supports fully rewriting source paths.

debugedit will spit out a list of (canonicalized) files found in the DWARF (line table and compile unit directories) to the file given with -l for each file. These are traditionally filtered by find-debuginfo.sh then cat together in find-debuginfo.sh and then we use this cpio hack to put those files under /usr/debug/src/...:
  (cd "${debug_base_name}"; cpio -pd0mL "${RPM_BUILD_ROOT}${debug_dest_name}")
We then use the following in find-debuginfo.sh to package these up:
    (cd "${RPM_BUILD_ROOT}/usr"
     find src/debug -mindepth 1 -maxdepth 1
    ) | sed 's,^,/usr/,' >> "$srcout"
(There are now two variants of the above, one that puts them in $LISTFILE when the sources go into the debuginfo package, the other puts them in $srcout for when generating a debugsources package.)

So currently we don't really use the filelist generated by debugedit directly and filter it by the cpio/find tricks/hacks which implicitly make it so that only source files under the $RPM_BUILD_DIR (or $BUILDDIR) are packaged up.

We could make this explicit. We then could grep through the debugsource.list that debugedit generated (which are canonical file paths) and check all those files are under $BUILDIR. But we have to have a mechanism to know which files come from other packages and are legitimately included (/usr/include/xxx is the most common example). And some (upstream) builds do some tricks where source files are generated in /tmp or /var/tmp (yes, really...). We currently don't include those, but could (we would have to rewrite those paths).

P.S. I am currently traveling and won't be able to easily come up with a prototype.
Comment 11 Dan Callaghan 2017-08-09 23:52:46 EDT
(In reply to Igor Gnatenko from comment #9)
> Yes, this is terribly wrong.

Yeah you keep saying that. But I am telling you, I suspect there are *many* packages which do this today. And there is nothing that has ever indicated to packagers before now that this is bad or problematic at all. It has worked fine with debuginfo generation up until now, so this is basically a regression in F27 rpm for those packages.

If you do want to start forbidding this practice (of building sources from directories not created by %setup) you will need to do more than just write comments in this bz about how terrible it is.
Comment 12 Dan Callaghan 2017-08-10 01:31:21 EDT
Also I just wanted to clarify a few things...

You mentioned both $RPM_BUILD_DIR and $BUILDDIR. I wasn't sure what either of those are, since normally a packager doesn't need to refer to them. So I looked it up. Please correct me if I have understood it wrongly...

$RPM_BUILD_DIR = %{_builddir} = the directory where you start from in %prep and %build and %install.

This is generally ~/rpmbuild/BUILD, inside mock it's /builddir/build/BUILD.

%{buildsubdir} = the subdirectory where %setup unpacked the sources to.

Often this is %{name}-%{version} but could be basically anything, including empty(?), depending how %setup was invoked.
Seems to be only set in the C-level implementation of the %setup macro.
If %setup is used multiple times each invocation overwrites %{buildsubdir}.

$BUILDDIR as far as I can find is not defined or used anywhere except internally within find-debuginfo.sh as the base directory it is working from.

It was mentioned several times above that xmonad-log-applet was building sources outside $RPM_BUILD_DIR or $BUILDDIR. I agree that *would* be a bad idea, but I will repeat again that the package was *not* doing that. Nor is mod_wsgi doing that. They are building from sources which *are* underneath $RPM_BUILD_DIR but *not* in the directory unpacked by %setup.

So it seems like the problem here is that the sources are being excluded if they are outside of %{buildsubdir}. Nothing to do with $RPM_BUILD_DIR.
Comment 13 Dan Callaghan 2017-08-10 01:58:21 EDT
Btw I checked xmonad-log-applet on F25 and gdb will happily find and load sources from /usr/src/debug/mate or /usr/src/debug/xfce in spite of their odd name. Same with python3-mod_wsgi, gdb loads sources from /usr/src/debug/python3-mod_wsgi-4.5.15-1.fc25. So the fact that these source directories are now silently being dropped from the -debugsource packages is basically a regression for any package in this situation.
Comment 14 Dan Callaghan 2017-08-10 02:31:10 EDT
To illustrate the regression, this is testing the same thing on rawhide using python3-mod_wsgi-4.5.15-2.fc27.x86_64, mod_wsgi-debuginfo-4.5.15-2.fc27.x86_64:

$ pidof httpd
19530 19528 19527 19526 19525
$ sudo gdb -p 19525
[...]
(gdb) list wsgi_python_bucket_setaside
107	src/server/wsgi_buckets.c: No such file or directory.

Here is a quick hack I came up with which seems to restore the previous behaviour. It will grab sources from anywhere in %{_builddir}, not just in %{buildsubdir}. This at least builds mod_wsgi successfully and fixes the missing sources as above. I'll leave it up to you whether this patch would be a bad idea or not...


diff --git a/macros.in b/macros.in
index b651b00..495a8c6 100644
--- a/macros.in
+++ b/macros.in
@@ -188,5 +188,5 @@
     %{?_find_debuginfo_opts} \\\
     %{?_debugsource_packages:-S debugsourcefiles.list} \\\
-    "%{_builddir}/%{?buildsubdir}"\
+    "%{_builddir}"\
 %{nil}
 
@@ -202,5 +202,5 @@
 Debug information is useful when developing applications that use this\
 package or when debugging this package.\
-%files debuginfo -f debugfiles.list\
+%files debuginfo -f %{_builddir}/debugfiles.list\
 %{nil}
 
@@ -214,5 +214,5 @@
 Debug sources are useful when developing applications that use this\
 package or when debugging this package.\
-%files debugsource -f debugsourcefiles.list\
+%files debugsource -f %{_builddir}/debugsourcefiles.list\
 %{nil}
Comment 15 Panu Matilainen 2017-08-10 03:21:08 EDT
Yeah we've been lax about the terminology, but %{buildsubdir} is the one everybody really means, that's the one specifically set up by %setup and what debuginfo also cares about.

When there's a designated directory set up and even cd'ed into automatically, one might think of it as a fairly strong indication that this is where you're supposed to do your work, even if it's not written down in million policies that nobody actually reads. You know - spirit of things if not exact letter?

Not that there's anything new in people abusing every single loophole in rpm(build) they can possibly find whether they actually need it or not. Once the shock wears off (after a few years) it's just tiresome. And mind you this is not even a particularly bad abuse, and nothing personal, no need to go defensive.

Everybody's going to agree that silently excluding sources is bad and a bug, it needs a warning at least. And on a related note, maybe it's time %setup is made mandatory for real but that's out of the scope for this bug.
Comment 16 Jan Kurik 2017-08-15 02:54:13 EDT
This bug appears to have been reported against 'rawhide' during the Fedora 27 development cycle.
Changing version to '27'.

Note You need to log in before you can comment on or make changes to this bug.