On 04/07/2017 08:07 AM, Don Zickus wrote:
On Thu, Apr 06, 2017 at 12:53:10PM -0700, Laura Abbott wrote:
On 04/05/2017 07:52 AM, Don Zickus wrote:
On Tue, Apr 04, 2017 at 02:36:48PM -0700, Laura Abbott wrote:
From: Laura Abbott labbott@redhat.com
Once upon a time, the kernel needed a lot of special handling to generate proper debuginfo as the kernel was ahead in technology. These days, rpm has improved debuginfo support. The kernel has not kept up with this and it's forward looking calls are now out of date. Switch to more standard invocations of debuginfo calls.
Bringing this out from the previous thread. This drops the special invocations of find-debuginfo.sh and debugedit. The results seem to be reasonable as far as I can tell.
Looking at the /usr/lib/rpm/macros and the find_debuginfo stuff in particular, those pieces of your patch makes sense to drop and replace with the find_debuginfo_opts[1].
I am curious about the AFTER_LINK patch. What is the reason to drop it? The patch probably needs to be dropped as either stale or integrated differently upstream, just thought it would be nice to have it documented.
The purpose of AFTER_LINK was to run the debugedit command. If we're running the standard find-debuginfo.sh, it calls debugedit already so there should be no reason to need the command at all.
Ok, sounds good. Thanks!
Can I assume your testing was install the debuginfo package and have an application like gdb or crash read in the symbols to verify the contents?
Yes, that's roughly what I did.
I think this patch is a good direction forward, just a little nitpick in [1].
Cheers, Don
[1] - Adding the VERSION and RELEASE stuff to find_debuginfo_opts must have been painful. It also makes it hard to read. I was curious if a macro would work there, where we pass a list of files and the macro spits out the files with the VERSION, RELEASE, _arch, _debug junk appended to it?
This might make it easier to add to later and maintain?
It's really not pretty, I spent at least a full day figuring out the regexes because I had no idea how they work. I'm sorely tempted to put some ascii art warning of the horrors within. More macros might help things, I'll see what I can do. What I'd really like is a find-debuginfo.sh flag to add the buildid automatically for the filtering. This might turn into a feature request or a patch if I get around to it.
That was my fear, it was so painful that attempt at future changes, scares folks off. :-(
So I just spent some time trying to explore your changes a little bit. I thought the VERSION and RELEASE stuff was for the kernel but in fact it really is only for the tools (perf and kernel-tools).
Looking at current kernel-tools-debuginfo doesn't show any VERSION and RELEASE stuff. Your patch seems to add that. I guess I am curious why? If you can only have one version of the tools installed at one time, what does VERSION and RELEASE provide us?
My thinking is why don't all the other userspace applications need similar naming?
Sorry for being picky, just trying to understand those particular changes.
Switching over to the default find-debuginfo.sh calls adds the unique build-ids and unique debug names for the kernel. This gets added for all files, even the userspace components. It seems like this is the direction find-debuginfo wants to go in so I figured it was worth the effort to move to it. I went by what needed the ids although maybe I should take a closer look.
Some of this comes back to the problem that the kernel repo doesn't just have the kernel, it has a bunch of other userspace tools that should really be a separate repo. As a result we have to keep a bunch of stuff in the kernel.spec and treat it as a homogeneous project. Nobody has put in the work to fight for moving stuff out of the kernel repo so alas we deal with what's there.
The rest of the patch looks fine to me.
Thanks, I really appreciate getting a review on this.
Cheers, Don
Thanks, Laura _______________________________________________ kernel mailing list -- kernel@lists.fedoraproject.org To unsubscribe send an email to kernel-leave@lists.fedoraproject.org