* [PATCH 0/1] package.bbclass: Allow overriding of debugedit starting path @ 2012-07-11 16:00 Peter Seebach 2012-07-11 16:00 ` [PATCH 1/1] " Peter Seebach 0 siblings, 1 reply; 10+ messages in thread From: Peter Seebach @ 2012-07-11 16:00 UTC (permalink / raw) To: openembedded-core We use a prebuilt toolchain which has all debug symbols start out looking like /opt/windriver/wrlinux/ia32/i686-wrs-linux-gnu/src/... and we'd rather not start populating /opt on targets when we have all this lovely logic for debug sources. This patch provides a generic hook for overriding the starting path that debugedit will replace with the canonical debugsrc path. This strikes me as a pretty low-impact alternative to, say, encouraging such recipes to call debugedit directly. I am open to the idea of allowing a replacement to be specified, too, but I didn't need that and don't immediately see why it'd matter. And since I switch between scripting languages really often, and this is a source of confusion: Yes, in Python, ("" or x) yields x. The following changes since commit 20405bdab18b7a2b101f818e72863d289d5945e4: Bruce Ashfield (1): linux-yocto/3.0: perf scripting support are available in the git repository at: git://git.yoctoproject.org/poky-contrib seebs/debugsrc http://git.yoctoproject.org/cgit.cgi/poky-contrib/log/?h=seebs/debugsrc Peter Seebach (1): package.bbclass: Allow overriding of debugedit starting path meta/classes/package.bbclass | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/1] package.bbclass: Allow overriding of debugedit starting path 2012-07-11 16:00 [PATCH 0/1] package.bbclass: Allow overriding of debugedit starting path Peter Seebach @ 2012-07-11 16:00 ` Peter Seebach 2012-07-11 16:12 ` Richard Purdie 0 siblings, 1 reply; 10+ messages in thread From: Peter Seebach @ 2012-07-11 16:00 UTC (permalink / raw) To: openembedded-core In the usual case, the computed path used for debugedit to fix up path references for the target filesystem is correct. However, prebuilt binary components, such as prebuilt toolchains, can have debug paths that do not reflect paths within the local build directory. Providing an override lets us continue to use the standard debugedit logic in package.bbclass. Signed-off-by: Peter Seebach <peter.seebach@windriver.com> --- meta/classes/package.bbclass | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass index e6856cd..1a8c7f8 100644 --- a/meta/classes/package.bbclass +++ b/meta/classes/package.bbclass @@ -191,6 +191,7 @@ def splitfile(file, debugfile, debugsrcdir, d): debugedit = d.expand("${STAGING_LIBDIR_NATIVE}/rpm/bin/debugedit") workdir = d.getVar("WORKDIR", True) workparentdir = os.path.dirname(workdir) + overridedir = d.getVar("DEBUGSRC_OVERRIDE_PATH", True) or "" sourcefile = d.expand("${WORKDIR}/debugsources.list") # We ignore kernel modules, we don't generate debug info files. @@ -205,7 +206,7 @@ def splitfile(file, debugfile, debugsrcdir, d): # We need to extract the debug src information here... if debugsrcdir: - subprocess.call("%s'%s' -b '%s' -d '%s' -i -l '%s' '%s'" % (pathprefix, debugedit, workparentdir, debugsrcdir, sourcefile, file), shell=True) + subprocess.call("%s'%s' -b '%s' -d '%s' -i -l '%s' '%s'" % (pathprefix, debugedit, overridedir or workparentdir, debugsrcdir, sourcefile, file), shell=True) bb.mkdirhier(os.path.dirname(debugfile)) -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/1] package.bbclass: Allow overriding of debugedit starting path 2012-07-11 16:00 ` [PATCH 1/1] " Peter Seebach @ 2012-07-11 16:12 ` Richard Purdie 2012-07-11 16:33 ` Peter Seebach 0 siblings, 1 reply; 10+ messages in thread From: Richard Purdie @ 2012-07-11 16:12 UTC (permalink / raw) To: Patches and discussions about the oe-core layer On Wed, 2012-07-11 at 11:00 -0500, Peter Seebach wrote: > In the usual case, the computed path used for debugedit to fix > up path references for the target filesystem is correct. However, > prebuilt binary components, such as prebuilt toolchains, can > have debug paths that do not reflect paths within the local > build directory. Providing an override lets us continue to use > the standard debugedit logic in package.bbclass. > > Signed-off-by: Peter Seebach <peter.seebach@windriver.com> > --- > meta/classes/package.bbclass | 3 ++- > 1 files changed, 2 insertions(+), 1 deletions(-) > > diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass > index e6856cd..1a8c7f8 100644 > --- a/meta/classes/package.bbclass > +++ b/meta/classes/package.bbclass > @@ -191,6 +191,7 @@ def splitfile(file, debugfile, debugsrcdir, d): > debugedit = d.expand("${STAGING_LIBDIR_NATIVE}/rpm/bin/debugedit") > workdir = d.getVar("WORKDIR", True) > workparentdir = os.path.dirname(workdir) > + overridedir = d.getVar("DEBUGSRC_OVERRIDE_PATH", True) or "" > sourcefile = d.expand("${WORKDIR}/debugsources.list") I think I at least would find this slightly less confusing as: workparentdir = d.getVar("DEBUGSRC_OVERRIDE_PATH", True) or os.path.dirname(workdir) Cheers, Richard ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/1] package.bbclass: Allow overriding of debugedit starting path 2012-07-11 16:12 ` Richard Purdie @ 2012-07-11 16:33 ` Peter Seebach 2012-07-11 17:36 ` Richard Purdie 0 siblings, 1 reply; 10+ messages in thread From: Peter Seebach @ 2012-07-11 16:33 UTC (permalink / raw) To: Patches and discussions about the oe-core layer On Wed, 11 Jul 2012 17:12:29 +0100 Richard Purdie <richard.purdie@linuxfoundation.org> wrote: > I think I at least would find this slightly less confusing as: > > workparentdir = d.getVar("DEBUGSRC_OVERRIDE_PATH", True) or > os.path.dirname(workdir) Wait, LESS confusing? I appear to have tragically misunderstood the design goals of package.bbclass. :P But yes, that's a good improvement. Applied locally. Speaking of confusing: If purely hypothetically I wanted to submit a patch which standardized the indentation in package.bbclass, would anyone be interested in that? I ask only because while I can accept either 8-space or 4-space indentation, I find it comforting when any given file full of Python source uses one or the other. And while there's currently only a couple of blocks of 4-space indentation in the file, we *normally* use 4-space, that's the quasi-official Python community norm, and a LOT of the "too long" lines in that file would be much more readable at 4 spaces. (This would be a totally separate patch, and I'm not super happy about the idea of a patch which updates half or more of the lines in the file, but it's not as though it'll be less painful to fix later.) -s -- Listen, get this. Nobody with a good compiler needs to be justified. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/1] package.bbclass: Allow overriding of debugedit starting path 2012-07-11 16:33 ` Peter Seebach @ 2012-07-11 17:36 ` Richard Purdie 2012-07-11 17:56 ` Ross Burton 2012-07-11 18:54 ` Peter Seebach 0 siblings, 2 replies; 10+ messages in thread From: Richard Purdie @ 2012-07-11 17:36 UTC (permalink / raw) To: Peter Seebach; +Cc: Patches and discussions about the oe-core layer On Wed, 2012-07-11 at 11:33 -0500, Peter Seebach wrote: > On Wed, 11 Jul 2012 17:12:29 +0100 > Richard Purdie <richard.purdie@linuxfoundation.org> wrote: > > > I think I at least would find this slightly less confusing as: > > > > workparentdir = d.getVar("DEBUGSRC_OVERRIDE_PATH", True) or > > os.path.dirname(workdir) > > Wait, LESS confusing? > > I appear to have tragically misunderstood the design goals of > package.bbclass. :P Well, we are trying over time... :) > But yes, that's a good improvement. Applied locally. > > Speaking of confusing: If purely hypothetically I wanted to > submit a patch which standardized the indentation in package.bbclass, > would anyone be interested in that? I ask only because while I can > accept either 8-space or 4-space indentation, I find it comforting when > any given file full of Python source uses one or the other. It should all use 4 space for python functions. There is however a twist which is due to the way we handle _prepend and _append. Those prepends and appends have whitespace too and I seem to remember issues with whitespace matching. Yes, this is horrible. This is why that file hasn't been touched for whitespace though. > And while there's currently only a couple of blocks of 4-space > indentation in the file, we *normally* use 4-space, that's the > quasi-official Python community norm, and a LOT of the "too long" lines > in that file would be much more readable at 4 spaces. > > (This would be a totally separate patch, and I'm not super happy about > the idea of a patch which updates half or more of the lines in the > file, but it's not as though it'll be less painful to fix later.) I'd be interested to see how much breakage we get from changing it. In fact I just tried it, the result: http://git.yoctoproject.org/cgit.cgi/poky-contrib/commit/?h=rpurdie/t14&id=3db917bfb2455715a3a3a542ea831d05ca1cf9f7 the particularly nasty bit: python populate_packages () { # Whitespace is deliberately a tab here due to the number of packages which # prepend this fuction :( populate_packages_core(d) } other than that it does seem to be working as long as I tweaked the busybox recipe and update-alternatives too. We could go through and change all the populate_packages_prepend functions but its the ones outside OE-Core I worry about. I also worry there are some _append functions now silently failing though :(. Cheers, Richard ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/1] package.bbclass: Allow overriding of debugedit starting path 2012-07-11 17:36 ` Richard Purdie @ 2012-07-11 17:56 ` Ross Burton 2012-07-11 18:54 ` Peter Seebach 1 sibling, 0 replies; 10+ messages in thread From: Ross Burton @ 2012-07-11 17:56 UTC (permalink / raw) To: Patches and discussions about the oe-core layer [-- Attachment #1: Type: text/plain, Size: 3274 bytes --] Incredibly sorry for top-posting, but a build history diff should show any delta and assuming none will give a lot more confidence in the changes being complete. In theory a simple change of indentation shouldn't result in any changes to the image, right? Ross -- Ross Burton Sent with Sparrow (http://www.sparrowmailapp.com/?sig) On Wednesday, 11 July 2012 at 18:36, Richard Purdie wrote: > On Wed, 2012-07-11 at 11:33 -0500, Peter Seebach wrote: > > On Wed, 11 Jul 2012 17:12:29 +0100 > > Richard Purdie <richard.purdie@linuxfoundation.org> wrote: > > > > > I think I at least would find this slightly less confusing as: > > > > > > workparentdir = d.getVar("DEBUGSRC_OVERRIDE_PATH", True) or > > > os.path.dirname(workdir) > > > > > > > > > Wait, LESS confusing? > > > > I appear to have tragically misunderstood the design goals of > > package.bbclass. :P > > > > > Well, we are trying over time... :) > > > But yes, that's a good improvement. Applied locally. > > > > Speaking of confusing: If purely hypothetically I wanted to > > submit a patch which standardized the indentation in package.bbclass, > > would anyone be interested in that? I ask only because while I can > > accept either 8-space or 4-space indentation, I find it comforting when > > any given file full of Python source uses one or the other. > > > > > It should all use 4 space for python functions. There is however a twist > which is due to the way we handle _prepend and _append. Those prepends > and appends have whitespace too and I seem to remember issues with > whitespace matching. > > Yes, this is horrible. This is why that file hasn't been touched for > whitespace though. > > > And while there's currently only a couple of blocks of 4-space > > indentation in the file, we *normally* use 4-space, that's the > > quasi-official Python community norm, and a LOT of the "too long" lines > > in that file would be much more readable at 4 spaces. > > > > (This would be a totally separate patch, and I'm not super happy about > > the idea of a patch which updates half or more of the lines in the > > file, but it's not as though it'll be less painful to fix later.) > > > > > I'd be interested to see how much breakage we get from changing it. In > fact I just tried it, the result: > > http://git.yoctoproject.org/cgit.cgi/poky-contrib/commit/?h=rpurdie/t14&id=3db917bfb2455715a3a3a542ea831d05ca1cf9f7 > > the particularly nasty bit: > > python populate_packages () { > # Whitespace is deliberately a tab here due to the number of packages which > # prepend this fuction :( > populate_packages_core(d) > } > > other than that it does seem to be working as long as I tweaked the > busybox recipe and update-alternatives too. We could go through and > change all the populate_packages_prepend functions but its the ones > outside OE-Core I worry about. I also worry there are some _append > functions now silently failing though :(. > > Cheers, > > Richard > > > _______________________________________________ > Openembedded-core mailing list > Openembedded-core@lists.openembedded.org > http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core > > [-- Attachment #2: Type: text/html, Size: 4616 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/1] package.bbclass: Allow overriding of debugedit starting path 2012-07-11 17:36 ` Richard Purdie 2012-07-11 17:56 ` Ross Burton @ 2012-07-11 18:54 ` Peter Seebach 2012-07-11 19:01 ` Chris Larson 1 sibling, 1 reply; 10+ messages in thread From: Peter Seebach @ 2012-07-11 18:54 UTC (permalink / raw) To: Richard Purdie; +Cc: Patches and discussions about the oe-core layer On Wed, 11 Jul 2012 18:36:50 +0100 Richard Purdie <richard.purdie@linuxfoundation.org> wrote: > It should all use 4 space for python functions. There is however a > twist which is due to the way we handle _prepend and _append. Those > prepends and appends have whitespace too and I seem to remember > issues with whitespace matching. > > Yes, this is horrible. This is why that file hasn't been touched for > whitespace though. noooooooo That hurts so much. Okay, crazy thought. Imagine that we were to make a modified parser, with the following behavior: Whenever reading a python function, count the indentation of the first line (N), then replace every N spaces of indentation with 4 spaces. If this worked, it would make it possible to start fixing stuff. DISCLAIMER: Due to Reasons, I have slept about 15 minutes out of the last day, also I'm sick. When I say "crazy thought", I do not kid. Anyway: Anyone have other thoughts/comments on the possible starting path fixup? I am pretty sure that it is sufficient for all real cases, with the one exception being prebuilts where the debug path names are unreasonably short. (I theorize that a more advanced debugedit could be written which could handle those by rewriting files. I nominate someone else to try to actually do it.) My initial theory that I'd need to be able to specify the destination path evaporates with the realization that we can always move source files to whatever path we want them in; it's just that it's nicer for everyone if the debug source files are always in the standard locations, give or take, no matter how they were built. -s -- Listen, get this. Nobody with a good compiler needs to be justified. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/1] package.bbclass: Allow overriding of debugedit starting path 2012-07-11 18:54 ` Peter Seebach @ 2012-07-11 19:01 ` Chris Larson 2012-07-11 19:54 ` Burton, Ross 0 siblings, 1 reply; 10+ messages in thread From: Chris Larson @ 2012-07-11 19:01 UTC (permalink / raw) To: Patches and discussions about the oe-core layer On Wed, Jul 11, 2012 at 11:54 AM, Peter Seebach <peter.seebach@windriver.com> wrote: > Okay, crazy thought. Imagine that we were to make a modified parser, > with the following behavior: > > Whenever reading a python function, count the indentation of the first > line (N), then replace every N spaces of indentation with 4 spaces. > > If this worked, it would make it possible to start fixing stuff. That's one possibility. The other would be to treat _append/_prepended functions as independent entities. Rather than concatenating functions, it'd add them to the prefuncs/postfuncs or equivalent. Without concatenation, there'd be no issues with mismatches. -- Christopher Larson ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/1] package.bbclass: Allow overriding of debugedit starting path 2012-07-11 19:01 ` Chris Larson @ 2012-07-11 19:54 ` Burton, Ross 2012-07-11 20:20 ` Richard Purdie 0 siblings, 1 reply; 10+ messages in thread From: Burton, Ross @ 2012-07-11 19:54 UTC (permalink / raw) To: Patches and discussions about the oe-core layer On 11 July 2012 20:01, Chris Larson <clarson@kergoth.com> wrote: > That's one possibility. The other would be to treat _append/_prepended > functions as independent entities. Rather than concatenating > functions, it'd add them to the prefuncs/postfuncs or equivalent. > Without concatenation, there'd be no issues with mismatches. I always thought it would be best to give each append/prepend chunk a unique name and then call them sequentially instead of gluing them together. Ross ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/1] package.bbclass: Allow overriding of debugedit starting path 2012-07-11 19:54 ` Burton, Ross @ 2012-07-11 20:20 ` Richard Purdie 0 siblings, 0 replies; 10+ messages in thread From: Richard Purdie @ 2012-07-11 20:20 UTC (permalink / raw) To: Patches and discussions about the oe-core layer On Wed, 2012-07-11 at 20:54 +0100, Burton, Ross wrote: > On 11 July 2012 20:01, Chris Larson <clarson@kergoth.com> wrote: > > That's one possibility. The other would be to treat _append/_prepended > > functions as independent entities. Rather than concatenating > > functions, it'd add them to the prefuncs/postfuncs or equivalent. > > Without concatenation, there'd be no issues with mismatches. > > I always thought it would be best to give each append/prepend chunk a > unique name and then call them sequentially instead of gluing them > together. Some of them even rely on variables set in other fragments. I think I killed the one example I saw but there still could be something potentially relying on this :( I agree we need to fix this. I'm tempted to have the python parser just error on finding tab indentation. Cheers, Richard ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2012-07-11 20:32 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-07-11 16:00 [PATCH 0/1] package.bbclass: Allow overriding of debugedit starting path Peter Seebach 2012-07-11 16:00 ` [PATCH 1/1] " Peter Seebach 2012-07-11 16:12 ` Richard Purdie 2012-07-11 16:33 ` Peter Seebach 2012-07-11 17:36 ` Richard Purdie 2012-07-11 17:56 ` Ross Burton 2012-07-11 18:54 ` Peter Seebach 2012-07-11 19:01 ` Chris Larson 2012-07-11 19:54 ` Burton, Ross 2012-07-11 20:20 ` Richard Purdie
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.