All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] perf: Don't hardcode host include path for libslang
@ 2019-06-14 18:39 Florian Fainelli
  2019-06-16  9:46 ` Jiri Olsa
  2019-06-22  6:49 ` [tip:perf/core] perf tools: " tip-bot for Florian Fainelli
  0 siblings, 2 replies; 6+ messages in thread
From: Florian Fainelli @ 2019-06-14 18:39 UTC (permalink / raw)
  To: linux-kernel
  Cc: bcm-kernel-feedback-list, Florian Fainelli, Peter Zijlstra,
	Ingo Molnar, Arnaldo Carvalho de Melo, Alexander Shishkin,
	Jiri Olsa, Namhyung Kim, Alexey Budankov, Quentin Monnet,
	Stanislav Fomichev, Jakub Kicinski

Hardcoding /usr/include/slang is fundamentally incompatible with cross
compilation and will lead to the inability for a cross-compiled
environment to properly detect whether slang is available or not.

If /usr/include/slang is necessary that is a distribution specific
knowledge that could be solved with either a standard pkg-config .pc
file (which slang has) or simply overriding CFLAGS accordingly, but the
default perf Makefile should be clean of all of that.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 tools/build/feature/Makefile | 2 +-
 tools/perf/Makefile.config   | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
index 4b8244ee65ce..f9432d21eff9 100644
--- a/tools/build/feature/Makefile
+++ b/tools/build/feature/Makefile
@@ -181,7 +181,7 @@ $(OUTPUT)test-libaudit.bin:
 	$(BUILD) -laudit
 
 $(OUTPUT)test-libslang.bin:
-	$(BUILD) -I/usr/include/slang -lslang
+	$(BUILD) -lslang
 
 $(OUTPUT)test-libcrypto.bin:
 	$(BUILD) -lcrypto
diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index 85fbcd265351..b11134fdf59f 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -641,7 +641,6 @@ ifndef NO_SLANG
     NO_SLANG := 1
   else
     # Fedora has /usr/include/slang/slang.h, but ubuntu /usr/include/slang.h
-    CFLAGS += -I/usr/include/slang
     CFLAGS += -DHAVE_SLANG_SUPPORT
     EXTLIBS += -lslang
     $(call detected,CONFIG_SLANG)
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] perf: Don't hardcode host include path for libslang
  2019-06-14 18:39 [PATCH] perf: Don't hardcode host include path for libslang Florian Fainelli
@ 2019-06-16  9:46 ` Jiri Olsa
  2019-06-17 17:37   ` Florian Fainelli
  2019-06-17 18:11   ` Arnaldo Carvalho de Melo
  2019-06-22  6:49 ` [tip:perf/core] perf tools: " tip-bot for Florian Fainelli
  1 sibling, 2 replies; 6+ messages in thread
From: Jiri Olsa @ 2019-06-16  9:46 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: linux-kernel, bcm-kernel-feedback-list, Peter Zijlstra,
	Ingo Molnar, Arnaldo Carvalho de Melo, Alexander Shishkin,
	Namhyung Kim, Alexey Budankov, Quentin Monnet, Stanislav Fomichev,
	Jakub Kicinski

On Fri, Jun 14, 2019 at 11:39:47AM -0700, Florian Fainelli wrote:
> Hardcoding /usr/include/slang is fundamentally incompatible with cross
> compilation and will lead to the inability for a cross-compiled
> environment to properly detect whether slang is available or not.
> 
> If /usr/include/slang is necessary that is a distribution specific
> knowledge that could be solved with either a standard pkg-config .pc
> file (which slang has) or simply overriding CFLAGS accordingly, but the
> default perf Makefile should be clean of all of that.

fedora 30 is ok with this, I guess acme's distro test will
tell us about the rest ;-)

jirka

> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  tools/build/feature/Makefile | 2 +-
>  tools/perf/Makefile.config   | 1 -
>  2 files changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
> index 4b8244ee65ce..f9432d21eff9 100644
> --- a/tools/build/feature/Makefile
> +++ b/tools/build/feature/Makefile
> @@ -181,7 +181,7 @@ $(OUTPUT)test-libaudit.bin:
>  	$(BUILD) -laudit
>  
>  $(OUTPUT)test-libslang.bin:
> -	$(BUILD) -I/usr/include/slang -lslang
> +	$(BUILD) -lslang
>  
>  $(OUTPUT)test-libcrypto.bin:
>  	$(BUILD) -lcrypto
> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> index 85fbcd265351..b11134fdf59f 100644
> --- a/tools/perf/Makefile.config
> +++ b/tools/perf/Makefile.config
> @@ -641,7 +641,6 @@ ifndef NO_SLANG
>      NO_SLANG := 1
>    else
>      # Fedora has /usr/include/slang/slang.h, but ubuntu /usr/include/slang.h
> -    CFLAGS += -I/usr/include/slang
>      CFLAGS += -DHAVE_SLANG_SUPPORT
>      EXTLIBS += -lslang
>      $(call detected,CONFIG_SLANG)
> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] perf: Don't hardcode host include path for libslang
  2019-06-16  9:46 ` Jiri Olsa
@ 2019-06-17 17:37   ` Florian Fainelli
  2019-06-17 18:11   ` Arnaldo Carvalho de Melo
  1 sibling, 0 replies; 6+ messages in thread
From: Florian Fainelli @ 2019-06-17 17:37 UTC (permalink / raw)
  To: Jiri Olsa, Florian Fainelli
  Cc: linux-kernel, bcm-kernel-feedback-list, Peter Zijlstra,
	Ingo Molnar, Arnaldo Carvalho de Melo, Alexander Shishkin,
	Namhyung Kim, Alexey Budankov, Quentin Monnet, Stanislav Fomichev,
	Jakub Kicinski

On 6/16/19 2:46 AM, Jiri Olsa wrote:
> On Fri, Jun 14, 2019 at 11:39:47AM -0700, Florian Fainelli wrote:
>> Hardcoding /usr/include/slang is fundamentally incompatible with cross
>> compilation and will lead to the inability for a cross-compiled
>> environment to properly detect whether slang is available or not.
>>
>> If /usr/include/slang is necessary that is a distribution specific
>> knowledge that could be solved with either a standard pkg-config .pc
>> file (which slang has) or simply overriding CFLAGS accordingly, but the
>> default perf Makefile should be clean of all of that.
> 
> fedora 30 is ok with this, I guess acme's distro test will
> tell us about the rest ;-)

If this patch somehow breaks a particular distribution (I could test on
everything), the following might be more acceptable:

diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
index 4b8244ee65ce..eefeaa9aa208 100644
--- a/tools/build/feature/Makefile
+++ b/tools/build/feature/Makefile
@@ -71,6 +71,7 @@ CC ?= $(CROSS_COMPILE)gcc
 CXX ?= $(CROSS_COMPILE)g++
 PKG_CONFIG ?= $(CROSS_COMPILE)pkg-config
 LLVM_CONFIG ?= llvm-config
+LIBSLANG_CFLAGS ?= -I/usr/include/slang

 all: $(FILES)

@@ -181,7 +182,7 @@ $(OUTPUT)test-libaudit.bin:
        $(BUILD) -laudit

 $(OUTPUT)test-libslang.bin:
-       $(BUILD) -I/usr/include/slang -lslang
+       $(BUILD) $(LIBSLANG_CFLAGS) -lslang

 $(OUTPUT)test-libcrypto.bin:
        $(BUILD) -lcrypto
diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index 85fbcd265351..df3022b213b3 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -636,12 +636,13 @@ ifdef NO_NEWT
 endif

 ifndef NO_SLANG
+  LIBSLANG_CFLAGS?=-I/usr/include/slang
   ifneq ($(feature-libslang), 1)
     msg := $(warning slang not found, disables TUI support. Please
install slang-devel, libslang-dev or libslang2-dev);
     NO_SLANG := 1
   else
     # Fedora has /usr/include/slang/slang.h, but ubuntu
/usr/include/slang.h
-    CFLAGS += -I/usr/include/slang
+    CFLAGS += $(LIBSLANG_CFLAGS)
     CFLAGS += -DHAVE_SLANG_SUPPORT
     EXTLIBS += -lslang
     $(call detected,CONFIG_SLANG)

> 
> jirka
> 
>>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>>  tools/build/feature/Makefile | 2 +-
>>  tools/perf/Makefile.config   | 1 -
>>  2 files changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
>> index 4b8244ee65ce..f9432d21eff9 100644
>> --- a/tools/build/feature/Makefile
>> +++ b/tools/build/feature/Makefile
>> @@ -181,7 +181,7 @@ $(OUTPUT)test-libaudit.bin:
>>  	$(BUILD) -laudit
>>  
>>  $(OUTPUT)test-libslang.bin:
>> -	$(BUILD) -I/usr/include/slang -lslang
>> +	$(BUILD) -lslang
>>  
>>  $(OUTPUT)test-libcrypto.bin:
>>  	$(BUILD) -lcrypto
>> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
>> index 85fbcd265351..b11134fdf59f 100644
>> --- a/tools/perf/Makefile.config
>> +++ b/tools/perf/Makefile.config
>> @@ -641,7 +641,6 @@ ifndef NO_SLANG
>>      NO_SLANG := 1
>>    else
>>      # Fedora has /usr/include/slang/slang.h, but ubuntu /usr/include/slang.h
>> -    CFLAGS += -I/usr/include/slang
>>      CFLAGS += -DHAVE_SLANG_SUPPORT
>>      EXTLIBS += -lslang
>>      $(call detected,CONFIG_SLANG)
>> -- 
>> 2.17.1
>>


-- 
Florian

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] perf: Don't hardcode host include path for libslang
  2019-06-16  9:46 ` Jiri Olsa
  2019-06-17 17:37   ` Florian Fainelli
@ 2019-06-17 18:11   ` Arnaldo Carvalho de Melo
  2019-06-17 18:16     ` Florian Fainelli
  1 sibling, 1 reply; 6+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-06-17 18:11 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Florian Fainelli, linux-kernel, bcm-kernel-feedback-list,
	Peter Zijlstra, Ingo Molnar, Alexander Shishkin, Namhyung Kim,
	Alexey Budankov, Quentin Monnet, Stanislav Fomichev,
	Jakub Kicinski

Em Sun, Jun 16, 2019 at 11:46:05AM +0200, Jiri Olsa escreveu:
> On Fri, Jun 14, 2019 at 11:39:47AM -0700, Florian Fainelli wrote:
> > Hardcoding /usr/include/slang is fundamentally incompatible with cross
> > compilation and will lead to the inability for a cross-compiled
> > environment to properly detect whether slang is available or not.
> > 
> > If /usr/include/slang is necessary that is a distribution specific
> > knowledge that could be solved with either a standard pkg-config .pc
> > file (which slang has) or simply overriding CFLAGS accordingly, but the
> > default perf Makefile should be clean of all of that.
> 
> fedora 30 is ok with this, I guess acme's distro test will
> tell us about the rest ;-)

Seems to be just needless old cruft:

[perfbuilder@7143ebde35eb /]$ cat /etc/redhat-release
Fedora release 20 (Heisenbug)
[perfbuilder@7143ebde35eb /]$
[perfbuilder@7143ebde35eb /]$ ls -la /usr/include/slang.h 
-rw-r--r--. 1 root root 87562 Apr 11  2011 /usr/include/slang.h
[perfbuilder@7143ebde35eb /]$ ls -la /usr/include/slang/slang.h 
lrwxrwxrwx. 1 root root 10 Jun 17 16:41 /usr/include/slang/slang.h -> ../slang.h
[perfbuilder@7143ebde35eb /]$ 

So I'm removing that comment:

> >      # Fedora has /usr/include/slang/slang.h, but ubuntu /usr/include/slang.h

And adding a:

Fixes: ef7b93a11904 ("perf report: Librarize the annotation code and use it in the newt browser")

Will do a build with all the containers and check that the output for
all the ones with the slang devel package installed have slang
successfully detected.

Thanks,

- Arnaldo
 
> jirka
> 
> > 
> > Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> > ---
> >  tools/build/feature/Makefile | 2 +-
> >  tools/perf/Makefile.config   | 1 -
2> >  2 files changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
> > index 4b8244ee65ce..f9432d21eff9 100644
> > --- a/tools/build/feature/Makefile
> > +++ b/tools/build/feature/Makefile
> > @@ -181,7 +181,7 @@ $(OUTPUT)test-libaudit.bin:
> >  	$(BUILD) -laudit
> >  
> >  $(OUTPUT)test-libslang.bin:
> > -	$(BUILD) -I/usr/include/slang -lslang
> > +	$(BUILD) -lslang
> >  
> >  $(OUTPUT)test-libcrypto.bin:
> >  	$(BUILD) -lcrypto
> > diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> > index 85fbcd265351..b11134fdf59f 100644
> > --- a/tools/perf/Makefile.config
> > +++ b/tools/perf/Makefile.config
> > @@ -641,7 +641,6 @@ ifndef NO_SLANG
> >      NO_SLANG := 1
> >    else
> >      # Fedora has /usr/include/slang/slang.h, but ubuntu /usr/include/slang.h
> > -    CFLAGS += -I/usr/include/slang
> >      CFLAGS += -DHAVE_SLANG_SUPPORT
> >      EXTLIBS += -lslang
> >      $(call detected,CONFIG_SLANG)
> > -- 
> > 2.17.1
> > 

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] perf: Don't hardcode host include path for libslang
  2019-06-17 18:11   ` Arnaldo Carvalho de Melo
@ 2019-06-17 18:16     ` Florian Fainelli
  0 siblings, 0 replies; 6+ messages in thread
From: Florian Fainelli @ 2019-06-17 18:16 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Jiri Olsa
  Cc: linux-kernel, bcm-kernel-feedback-list, Peter Zijlstra,
	Ingo Molnar, Alexander Shishkin, Namhyung Kim, Alexey Budankov,
	Quentin Monnet, Stanislav Fomichev, Jakub Kicinski

On 6/17/19 11:11 AM, Arnaldo Carvalho de Melo wrote:
> Em Sun, Jun 16, 2019 at 11:46:05AM +0200, Jiri Olsa escreveu:
>> On Fri, Jun 14, 2019 at 11:39:47AM -0700, Florian Fainelli wrote:
>>> Hardcoding /usr/include/slang is fundamentally incompatible with cross
>>> compilation and will lead to the inability for a cross-compiled
>>> environment to properly detect whether slang is available or not.
>>>
>>> If /usr/include/slang is necessary that is a distribution specific
>>> knowledge that could be solved with either a standard pkg-config .pc
>>> file (which slang has) or simply overriding CFLAGS accordingly, but the
>>> default perf Makefile should be clean of all of that.
>>
>> fedora 30 is ok with this, I guess acme's distro test will
>> tell us about the rest ;-)
> 
> Seems to be just needless old cruft:
> 
> [perfbuilder@7143ebde35eb /]$ cat /etc/redhat-release
> Fedora release 20 (Heisenbug)
> [perfbuilder@7143ebde35eb /]$
> [perfbuilder@7143ebde35eb /]$ ls -la /usr/include/slang.h 
> -rw-r--r--. 1 root root 87562 Apr 11  2011 /usr/include/slang.h
> [perfbuilder@7143ebde35eb /]$ ls -la /usr/include/slang/slang.h 
> lrwxrwxrwx. 1 root root 10 Jun 17 16:41 /usr/include/slang/slang.h -> ../slang.h
> [perfbuilder@7143ebde35eb /]$ 
> 
> So I'm removing that comment:
> 
>>>      # Fedora has /usr/include/slang/slang.h, but ubuntu /usr/include/slang.h
> 
> And adding a:
> 
> Fixes: ef7b93a11904 ("perf report: Librarize the annotation code and use it in the newt browser")

OK, I was not sure if you would consider that a worthy fix or not, but
that works for me.

> 
> Will do a build with all the containers and check that the output for
> all the ones with the slang devel package installed have slang
> successfully detected.

Thanks, let me know if that does not work somehow.
-- 
Florian

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [tip:perf/core] perf tools: Don't hardcode host include path for libslang
  2019-06-14 18:39 [PATCH] perf: Don't hardcode host include path for libslang Florian Fainelli
  2019-06-16  9:46 ` Jiri Olsa
@ 2019-06-22  6:49 ` tip-bot for Florian Fainelli
  1 sibling, 0 replies; 6+ messages in thread
From: tip-bot for Florian Fainelli @ 2019-06-22  6:49 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: alexander.shishkin, jakub.kicinski, f.fainelli, linux-kernel,
	jolsa, peterz, namhyung, hpa, acme, mingo, sdf, quentin.monnet,
	alexey.budankov, tglx

Commit-ID:  1955c8cf5e26b1f70d674190ff9984dbfd531ee9
Gitweb:     https://git.kernel.org/tip/1955c8cf5e26b1f70d674190ff9984dbfd531ee9
Author:     Florian Fainelli <f.fainelli@gmail.com>
AuthorDate: Fri, 14 Jun 2019 11:39:47 -0700
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 17 Jun 2019 15:57:20 -0300

perf tools: Don't hardcode host include path for libslang

Hardcoding /usr/include/slang is fundamentally incompatible with cross
compilation and will lead to the inability for a cross-compiled
environment to properly detect whether slang is available or not.

If /usr/include/slang is necessary that is a distribution specific
knowledge that could be solved with either a standard pkg-config .pc
file (which slang has) or simply overriding CFLAGS accordingly, but the
default perf Makefile should be clean of all of that.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Alexey Budankov <alexey.budankov@linux.intel.com>
Cc: bcm-kernel-feedback-list@broadcom.com
Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Quentin Monnet <quentin.monnet@netronome.com>
Cc: Stanislav Fomichev <sdf@google.com>
Fixes: ef7b93a11904 ("perf report: Librarize the annotation code and use it in the newt browser")
Link: http://lkml.kernel.org/r/20190614183949.5588-1-f.fainelli@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/build/feature/Makefile | 2 +-
 tools/perf/Makefile.config   | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
index 523ee42db0c8..7ef7cf04a292 100644
--- a/tools/build/feature/Makefile
+++ b/tools/build/feature/Makefile
@@ -182,7 +182,7 @@ $(OUTPUT)test-libaudit.bin:
 	$(BUILD) -laudit
 
 $(OUTPUT)test-libslang.bin:
-	$(BUILD) -I/usr/include/slang -lslang
+	$(BUILD) -lslang
 
 $(OUTPUT)test-libcrypto.bin:
 	$(BUILD) -lcrypto
diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index 5f16a20cae86..e04b7a81d221 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -648,7 +648,6 @@ ifndef NO_SLANG
     NO_SLANG := 1
   else
     # Fedora has /usr/include/slang/slang.h, but ubuntu /usr/include/slang.h
-    CFLAGS += -I/usr/include/slang
     CFLAGS += -DHAVE_SLANG_SUPPORT
     EXTLIBS += -lslang
     $(call detected,CONFIG_SLANG)

^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-06-22  6:49 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-06-14 18:39 [PATCH] perf: Don't hardcode host include path for libslang Florian Fainelli
2019-06-16  9:46 ` Jiri Olsa
2019-06-17 17:37   ` Florian Fainelli
2019-06-17 18:11   ` Arnaldo Carvalho de Melo
2019-06-17 18:16     ` Florian Fainelli
2019-06-22  6:49 ` [tip:perf/core] perf tools: " tip-bot for Florian Fainelli

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.