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@xxxxxxxxxx> >>>> >>>> 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@xxxxxxxxxxxxxxxxxxxxxxx >> To unsubscribe send an email to kernel-leave@xxxxxxxxxxxxxxxxxxxxxxx _______________________________________________ kernel mailing list -- kernel@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to kernel-leave@xxxxxxxxxxxxxxxxxxxxxxx
