[Qemu-devel] [PATCH] Fix build for less common build directories names

[Qemu-devel] [PATCH] Fix build for less common build directories names

[Stefan Weil]

scripts/tracetool generates a C preprocessor macro from the name of the
build directory. Any characters which are possible in a directory name
but not allowed in a macro name must be substituted, otherwise builds
will fail.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
---

I had problems with a build directory of the form "host,variant".
Is this fix needed for stable, too?

Regards
Stefan W.

 scripts/tracetool.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/tracetool.py b/scripts/tracetool.py
index 629b259..fe9c9e9 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"[^A-Za-z0-9]", "_", dirname)
 
 def main(args):
     global _SCRIPT
-- 
2.1.4

Re: [Qemu-devel] [PATCH] Fix build for less common build directories names

[Peter Maydell]

On 13 October 2016 at 19:29, Stefan Weil <sw@weilnetz.de> wrote:
> scripts/tracetool generates a C preprocessor macro from the name of the
> build directory. Any characters which are possible in a directory name
> but not allowed in a macro name must be substituted, otherwise builds
> will fail.
>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
>
> I had problems with a build directory of the form "host,variant".
> Is this fix needed for stable, too?

Why does it need to care about the build directory name at all?
Ideally builds should be entirely deterministically reproducible
whatever the path to the source or build directory names is...

thanks
-- PMM

Re: [Qemu-devel] [PATCH] Fix build for less common build directories names

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1183 bytes --]

On Thu, Oct 13, 2016 at 07:36:07PM +0100, Peter Maydell wrote:
> On 13 October 2016 at 19:29, Stefan Weil <sw@weilnetz.de> wrote:
> > scripts/tracetool generates a C preprocessor macro from the name of the
> > build directory. Any characters which are possible in a directory name
> > but not allowed in a macro name must be substituted, otherwise builds
> > will fail.
> >
> > Signed-off-by: Stefan Weil <sw@weilnetz.de>
> > ---
> >
> > I had problems with a build directory of the form "host,variant".
> > Is this fix needed for stable, too?
> 
> Why does it need to care about the build directory name at all?
> Ideally builds should be entirely deterministically reproducible
> whatever the path to the source or build directory names is...

It's trying to construct the relative path from the source directory to
a trace-events file (inside the source directory).  This is used so that
block/trace-event macros have BLOCK in their name while net/trace-events
have NET, etc.

It does not care about the build directory or source directory path per
se.  Therefore deterministic builds shouldn't be a problem if the code
is working correctly ;).

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [Qemu-devel] [PATCH] Fix build for less common build directories names

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1254 bytes --]

On Thu, Oct 13, 2016 at 08:29:30PM +0200, Stefan Weil wrote:
> scripts/tracetool generates a C preprocessor macro from the name of the
> build directory. Any characters which are possible in a directory name
> but not allowed in a macro name must be substituted, otherwise builds
> will fail.
> 
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
> 
> I had problems with a build directory of the form "host,variant".
> Is this fix needed for stable, too?
> 
> Regards
> Stefan W.
> 
>  scripts/tracetool.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/tracetool.py b/scripts/tracetool.py
> index 629b259..fe9c9e9 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"[^A-Za-z0-9]", "_", dirname)

dirname should only contain QEMU source tree subdirectory paths (e.g.
hw/net).  How did you get a comma in there?

I tried an out-of-tree build in a directory called 'a,b' but it doesn't
trigger the issue.

I'd like to understand how to reproduce the problem in case I missed
something.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [Qemu-devel] [PATCH] Fix build for less common build directories names

[Paolo Bonzini]

On 14/10/2016 11:53, Stefan Hajnoczi wrote:
> > Why does it need to care about the build directory name at all?
> > Ideally builds should be entirely deterministically reproducible
> > whatever the path to the source or build directory names is...
>
> It's trying to construct the relative path from the source directory to
> a trace-events file (inside the source directory).  This is used so that
> block/trace-event macros have BLOCK in their name while net/trace-events
> have NET, etc.

But Stefan (Weil) is right, it's actually including the path to the
build directory if you're building with objdir!=srcdir.  It should not
need that.

Paolo

Re: [Qemu-devel] [PATCH] Fix build for less common build directories names

[Stefan Weil]

On 10/14/16 12:01, Stefan Hajnoczi wrote:
> dirname should only contain QEMU source tree subdirectory paths (e.g.
> hw/net).  How did you get a comma in there?
>
> I tried an out-of-tree build in a directory called 'a,b' but it doesn't
> trigger the issue.
>
> I'd like to understand how to reproduce the problem in case I missed
> something.
>
> Stefan

I can reproduce it like this from the QEMU base directory:

$ mkdir a,b
$ cd a,b
$ ../configure
[...]
$ make
[...]
make: *** [trace/generated-tracers.o] Error 1
$ head trace/generated-tracers.h
/* This file is autogenerated by tracetool, do not edit. */

#ifndef TRACE_A,B_GENERATED_TRACERS_H
#define TRACE_A,B_GENERATED_TRACERS_H

Stefan

Re: [Qemu-devel] [PATCH] Fix build for less common build directories names

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 931 bytes --]

On Fri, Oct 14, 2016 at 10:05:18PM +0200, Stefan Weil wrote:
> On 10/14/16 12:01, Stefan Hajnoczi wrote:
> > dirname should only contain QEMU source tree subdirectory paths (e.g.
> > hw/net).  How did you get a comma in there?
> > 
> > I tried an out-of-tree build in a directory called 'a,b' but it doesn't
> > trigger the issue.
> > 
> > I'd like to understand how to reproduce the problem in case I missed
> > something.
> > 
> > Stefan
> 
> I can reproduce it like this from the QEMU base directory:
> 
> $ mkdir a,b
> $ cd a,b
> $ ../configure
> [...]
> $ make
> [...]
> make: *** [trace/generated-tracers.o] Error 1
> $ head trace/generated-tracers.h
> /* This file is autogenerated by tracetool, do not edit. */
> 
> #ifndef TRACE_A,B_GENERATED_TRACERS_H
> #define TRACE_A,B_GENERATED_TRACERS_H

Interesting!  I missed it because my a,b directory was not inside the
source directory.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [Qemu-devel] [PATCH] Fix build for less common build directories names

[Michael Tokarev]

13.10.2016 21:29, Stefan Weil wrote:
> scripts/tracetool generates a C preprocessor macro from the name of the
> build directory. Any characters which are possible in a directory name
> but not allowed in a macro name must be substituted, otherwise builds
> will fail.

Applied to -trivial, thank you!

/mjt

Re: [Qemu-devel] [PATCH] Fix build for less common build directories names

[Peter Maydell]

On 16 October 2016 at 15:31, Michael Tokarev <mjt@tls.msk.ru> wrote:
> 13.10.2016 21:29, Stefan Weil wrote:
>> scripts/tracetool generates a C preprocessor macro from the name of the
>> build directory. Any characters which are possible in a directory name
>> but not allowed in a macro name must be substituted, otherwise builds
>> will fail.
>
> Applied to -trivial, thank you!

Consensus on this and the other thread seemed to be
that we should just fix the bug where tracetool is
putting the build directory name into symbols
rather than working around that...

thanks
-- PMM

Re: [Qemu-devel] [PATCH] Fix build for less common build directories names

[Stefan Weil]

Am 16.10.2016 um 21:04 schrieb Peter Maydell:
> On 16 October 2016 at 15:31, Michael Tokarev <mjt@tls.msk.ru> wrote:
>> 13.10.2016 21:29, Stefan Weil wrote:
>>> scripts/tracetool generates a C preprocessor macro from the name of the
>>> build directory. Any characters which are possible in a directory name
>>> but not allowed in a macro name must be substituted, otherwise builds
>>> will fail.
>>
>> Applied to -trivial, thank you!
>
> Consensus on this and the other thread seemed to be
> that we should just fix the bug where tracetool is
> putting the build directory name into symbols
> rather than working around that...
>
> thanks
> -- PMM
>

IMHO the patch can be applied nevertheless, as it uses a clearer
regular expression for characters allowed in macro names
(not one that is specific for the current QEMU code).

Stefan

Re: [Qemu-devel] [PATCH] Fix build for less common build directories names

[Michael Tokarev]

17.10.2016 07:43, Stefan Weil wrote:
> Am 16.10.2016 um 21:04 schrieb Peter Maydell:
>> On 16 October 2016 at 15:31, Michael Tokarev <mjt@tls.msk.ru> wrote:
>>> 13.10.2016 21:29, Stefan Weil wrote:
>>>> scripts/tracetool generates a C preprocessor macro from the name of the
>>>> build directory. Any characters which are possible in a directory name
>>>> but not allowed in a macro name must be substituted, otherwise builds
>>>> will fail.
>>>
>>> Applied to -trivial, thank you!
>>
>> Consensus on this and the other thread seemed to be
>> that we should just fix the bug where tracetool is
>> putting the build directory name into symbols
>> rather than working around that...
>>
>> thanks
>> -- PMM
>
> IMHO the patch can be applied nevertheless, as it uses a clearer
> regular expression for characters allowed in macro names
> (not one that is specific for the current QEMU code).

So, what's the thing here?

Once again I hit this bug today, when my usual build environment doesn't
work:

./trace/generated-tracers.h:3:15: error: extra tokens at end of
#ifndef directive [-Werror]
 #ifndef TRACE_.B64_GENERATED_TRACERS_H
               ^
and this patch fixes the issue for me.

Thanks,

/mjt

Re: [Qemu-devel] [PATCH] Fix build for less common build directories names

[Peter Maydell]

On 28 October 2016 at 15:57, Michael Tokarev <mjt@tls.msk.ru> wrote:
> 17.10.2016 07:43, Stefan Weil wrote:
>> Am 16.10.2016 um 21:04 schrieb Peter Maydell:
>>> On 16 October 2016 at 15:31, Michael Tokarev <mjt@tls.msk.ru> wrote:
>>>> 13.10.2016 21:29, Stefan Weil wrote:
>>>>> scripts/tracetool generates a C preprocessor macro from the name of the
>>>>> build directory. Any characters which are possible in a directory name
>>>>> but not allowed in a macro name must be substituted, otherwise builds
>>>>> will fail.
>>>>
>>>> Applied to -trivial, thank you!
>>>
>>> Consensus on this and the other thread seemed to be
>>> that we should just fix the bug where tracetool is
>>> putting the build directory name into symbols
>>> rather than working around that...
>>>
>>> thanks
>>> -- PMM
>>
>> IMHO the patch can be applied nevertheless, as it uses a clearer
>> regular expression for characters allowed in macro names
>> (not one that is specific for the current QEMU code).
>
> So, what's the thing here?

I think we concluded that the underlying problem was a bit
tricky to fix and we should apply this workaround in the
meantime.

thanks
-- PMM