On 14 October 2016 at 23:06, Greg Kurz <gr...@kaod.org> wrote:
> On Fri, 14 Oct 2016 16:31:01 -0500
> Eric Blake <ebl...@redhat.com> wrote:
>> On 10/14/2016 04:26 PM, Greg Kurz wrote:
>> > Since commit "80dd5c4918ab trace: introduce a formal group name for trace
>> > events", tracetool generates C variable names and macro definitions out
>> > of the path to the trace-events-all file.
>> > diff --git a/scripts/tracetool.py b/scripts/tracetool.py
>> > index 629b2593c846..b81b834db924 100755
>> > --- a/scripts/tracetool.py
>> > +++ b/scripts/tracetool.py
>> > @@ -70,7 +70,7 @@ def make_group_name(filename):
>> > if dirname == "":
>> > return "common"
>> > - return re.sub(r"/|-", "_", dirname)
>> > + return "_" + re.sub(r"[^\w]", "_", dirname)
>> This STILL doesn't solve the complaint that the build is now dependent
>> on the location. Why can't we STRIP off any prefix prior to the in-tree
>> portion of the naming that we know is sane, instead of munging the
>> prefix but in the process creating source code that generates with
>> different lengths?
> Heh, because the complaint was about the build break :)
>> Ideally, compiling twice, once in directory 'a', and the second time in
>> directory 'aaaaaaaaaaaaaaaaaaaaaaaaaaaa', should not make a noticeable
>> difference in the final size of the executable due to the difference in
>> lengths of the debugging symbols used to record the longer name of the
>> second directory being encoded into lots of macro names.
> You're right. I'm okay to look for a way to fix the symbol variable size
> issue, but I think this patch still makes sense as it makes tracetool
> less fragile in case we add a directory with an illegal character to
> the QEMU tree... and it fixes annoying build breaks (Paolo also hit
> this and asked to revert 80dd5c4918ab in another mail).
I agree with Eric that we should just not be putting
the build directory name in these variables -- this
looks like it's simply a bug to me, and we should fix it.
I think the chances of us adding a directory to the QEMU
tree itself with a silly name are quite low (not least because
if you do that then you get a handy build failure to tell
you not to do that ;-))