[PATCH] perf build fixes for opensuse 10.0

[PATCH] perf build fixes for opensuse 10.0

[Andi Kleen]

Fix the perf Makefile to compile on opensuse 10.0

-Werror in shipping code is usually a bad idea. Remove that.
Also remove warning options that are not supported in older compilers.
And fix a missing include for __always_inline.
    
Signed-off-by: Andi Kleen <ak@linux.intel.com>

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 4f1fa77..1bf4d9c 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -201,13 +201,13 @@ EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wshadow
 EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Winit-self
 EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wpacked
 EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wredundant-decls
-EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wstack-protector
+#EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wstack-protector
 EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wstrict-aliasing=3
 EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wswitch-default
 EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wswitch-enum
 EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wno-system-headers
 EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wundef
-EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wvolatile-register-var
+#EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wvolatile-register-var
 EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wwrite-strings
 EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wbad-function-cast
 EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wmissing-declarations
@@ -224,7 +224,7 @@ ifndef PERF_DEBUG
   CFLAGS_OPTIMIZE = -O6
 endif
 
-CFLAGS = -ggdb3 -Wall -Wextra -std=gnu99 -Werror $(CFLAGS_OPTIMIZE) -D_FORTIFY_SOURCE=2 $(EXTRA_WARNINGS) $(EXTRA_CFLAGS)
+CFLAGS = -ggdb3 -Wall -Wextra -std=gnu99 $(CFLAGS_OPTIMIZE) -D_FORTIFY_SOURCE=2 $(EXTRA_WARNINGS) $(EXTRA_CFLAGS)
 EXTLIBS = -lpthread -lrt -lelf -lm
 ALL_CFLAGS = $(CFLAGS) -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64
 ALL_LDFLAGS = $(LDFLAGS)
diff --git a/tools/perf/util/include/linux/bitops.h b/tools/perf/util/include/linux/bitops.h
index bb4ac2e..305f1c2 100644
--- a/tools/perf/util/include/linux/bitops.h
+++ b/tools/perf/util/include/linux/bitops.h
@@ -2,6 +2,7 @@
 #define _PERF_LINUX_BITOPS_H_
 
 #include <linux/kernel.h>
+#include <linux/compiler.h>
 #include <asm/hweight.h>
 
 #define BITS_PER_LONG __WORDSIZE

Re: [PATCH] perf build fixes for opensuse 10.0

[John Kacur]

On Wed, Sep 22, 2010 at 11:03 AM, Andi Kleen <andi@firstfloor.org> wrote:
> Fix the perf Makefile to compile on opensuse 10.0
>
> -Werror in shipping code is usually a bad idea. Remove that.

Why not remove it then when you ship the code, but leave it in the kernel
where it is useful?

> Also remove warning options that are not supported in older compilers.

I can get on-board with the above though.

> And fix a missing include for __always_inline.
>
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
>
> diff --git a/tools/perf/Makefile b/tools/perf/Makefile
> index 4f1fa77..1bf4d9c 100644
> --- a/tools/perf/Makefile
> +++ b/tools/perf/Makefile
> @@ -201,13 +201,13 @@ EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wshadow
>  EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Winit-self
>  EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wpacked
>  EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wredundant-decls
> -EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wstack-protector
> +#EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wstack-protector
>  EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wstrict-aliasing=3
>  EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wswitch-default
>  EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wswitch-enum
>  EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wno-system-headers
>  EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wundef
> -EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wvolatile-register-var
> +#EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wvolatile-register-var
>  EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wwrite-strings
>  EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wbad-function-cast
>  EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wmissing-declarations
> @@ -224,7 +224,7 @@ ifndef PERF_DEBUG
>   CFLAGS_OPTIMIZE = -O6
>  endif
>
> -CFLAGS = -ggdb3 -Wall -Wextra -std=gnu99 -Werror $(CFLAGS_OPTIMIZE) -D_FORTIFY_SOURCE=2 $(EXTRA_WARNINGS) $(EXTRA_CFLAGS)
> +CFLAGS = -ggdb3 -Wall -Wextra -std=gnu99 $(CFLAGS_OPTIMIZE) -D_FORTIFY_SOURCE=2 $(EXTRA_WARNINGS) $(EXTRA_CFLAGS)
>  EXTLIBS = -lpthread -lrt -lelf -lm
>  ALL_CFLAGS = $(CFLAGS) -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64
>  ALL_LDFLAGS = $(LDFLAGS)
> diff --git a/tools/perf/util/include/linux/bitops.h b/tools/perf/util/include/linux/bitops.h
> index bb4ac2e..305f1c2 100644
> --- a/tools/perf/util/include/linux/bitops.h
> +++ b/tools/perf/util/include/linux/bitops.h
> @@ -2,6 +2,7 @@
>  #define _PERF_LINUX_BITOPS_H_
>
>  #include <linux/kernel.h>
> +#include <linux/compiler.h>
>  #include <asm/hweight.h>
>
>  #define BITS_PER_LONG __WORDSIZE
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

Re: [PATCH] perf build fixes for opensuse 10.0

[Andi Kleen]

> On Wed, Sep 22, 2010 at 11:03 AM, Andi Kleen <andi@firstfloor.org> wrote:
>> Fix the perf Makefile to compile on opensuse 10.0
>>
>> -Werror in shipping code is usually a bad idea. Remove that.
>
> Why not remove it then when you ship the code, but leave it in the kernel
> where it is useful?

What do you mean with "you ship"?

I downloaded a kernel from kernel.org, it shipped to me and didn't
build.

People can enable -Werror in their private source dirs, but
it shouldn't be in any downloadable tarball or git.

-Andi

Re: [PATCH] perf build fixes for opensuse 10.0

[John Kacur]

On Wed, Sep 22, 2010 at 1:15 PM, Andi Kleen <andi@firstfloor.org> wrote:
>> On Wed, Sep 22, 2010 at 11:03 AM, Andi Kleen <andi@firstfloor.org> wrote:
>>> Fix the perf Makefile to compile on opensuse 10.0
>>>
>>> -Werror in shipping code is usually a bad idea. Remove that.
>>
>> Why not remove it then when you ship the code, but leave it in the kernel
>> where it is useful?
>
> What do you mean with "you ship"?

The subject of your email is "perf build fixes for opensuse 10.0"
So, I inferred from there.

> I downloaded a kernel from kernel.org, it shipped to me and didn't
> build.

Any reason you can't fix the build errors instead?
Btw, I just tried building the lastest perf in tip/master
and I had no problems.

>
> People can enable -Werror in their private source dirs, but
> it shouldn't be in any downloadable tarball or git.
>

Re: [PATCH] perf build fixes for opensuse 10.0

[Andi Kleen]

On Wed, Sep 22, 2010 at 03:10:04PM +0200, John Kacur wrote:
> On Wed, Sep 22, 2010 at 1:15 PM, Andi Kleen <andi@firstfloor.org> wrote:
> >> On Wed, Sep 22, 2010 at 11:03 AM, Andi Kleen <andi@firstfloor.org> wrote:
> >>> Fix the perf Makefile to compile on opensuse 10.0
> >>>
> >>> -Werror in shipping code is usually a bad idea. Remove that.
> >>
> >> Why not remove it then when you ship the code, but leave it in the kernel
> >> where it is useful?
> >
> > What do you mean with "you ship"?
> 
> The subject of your email is "perf build fixes for opensuse 10.0"
> So, I inferred from there.

You inferred wrong.  That was just using it on opensuse 10.0

(opensuse 10.0 has already shipped a very long time ago, it's really
old by now)

> 
> > I downloaded a kernel from kernel.org, it shipped to me and didn't
> > build.
> 
> Any reason you can't fix the build errors instead?

That's what I did -- and then sent the patch.

> Btw, I just tried building the lastest perf in tip/master
> and I had no problems.

Because you're not using opensuse 10.0

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.

Re: [PATCH] perf build fixes for opensuse 10.0

[Ingo Molnar]

* Andi Kleen <andi@firstfloor.org> wrote:

> Fix the perf Makefile to compile on opensuse 10.0
> 
> -Werror in shipping code is usually a bad idea. Remove that. Also 
> remove warning options that are not supported in older compilers. And 
> fix a missing include for __always_inline.

NAK, for obvious reasons.

Please send patches that fix those warnings instead. Yes, some will be 
false positives - but quite a few will point to problems we want to 
address.

Thanks,

	Ingo

Re: [PATCH] perf build fixes for opensuse 10.0

[Ingo Molnar]

* John Kacur <jkacur@redhat.com> wrote:

> On Wed, Sep 22, 2010 at 1:15 PM, Andi Kleen <andi@firstfloor.org> wrote:
> >> On Wed, Sep 22, 2010 at 11:03 AM, Andi Kleen <andi@firstfloor.org> wrote:
> >>> Fix the perf Makefile to compile on opensuse 10.0
> >>>
> >>> -Werror in shipping code is usually a bad idea. Remove that.
> >>
> >> Why not remove it then when you ship the code, but leave it in the kernel
> >> where it is useful?
> >
> > What do you mean with "you ship"?
> 
> The subject of your email is "perf build fixes for opensuse 10.0"
> So, I inferred from there.
> 
> > I downloaded a kernel from kernel.org, it shipped to me and didn't
> > build.
> 
> Any reason you can't fix the build errors instead?

Well, it's Andi - he has a well established track record for picking the 
worst possible solution for pretty much anything and then being 
passive-agressive difficult about it as if there was no tomorrow. It's 
typically quite time-consuming for everyone involved, with little 
productive output. YMMV.

Here he could have done a dozen things to make his posting more useful: 
he could have included the build warnings in the changelog (he didnt), 
he could have tried to address them (he didnt), he could have argued for 
some specific warnings to be disabled on specific, well-established 
grounds, for example demonstrating that it _only_ produces false 
positives (he didnt) - etc. etc. He did none of those things.

Thanks,

	Ingo