From: Zamir SUN (Red Hat) <[email protected]>
This is a set of patch to improve trace-cmd python plugin support.
Patch 1 fixes the detection for swig. With this patch, python-plugin can
be built successfully.
Patch 2 improves the detection for python ldflags. With this patch,
ldflags can be detected with python3 in the future.
Zamir SUN (2):
trace-cmd: Fix the detection for swig
trace-cmd: Change the way of getting python ldflags.
Makefile | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--
2.14.3
From: Zamir SUN <[email protected]>
The current detection for swig will cause output to be
/usr/bin/swig
y
So this will never be equal to y. With this patch, the swig path is
removed from output, so the detection can work as expected.
Fixes 3bf187a43b7e6302592552ecbc294e5820249687
Signed-off-by: Zamir SUN (Red Hat) <[email protected]>
---
Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index a5d2c38..7c0d1a6 100644
--- a/Makefile
+++ b/Makefile
@@ -105,7 +105,7 @@ PYTHON_GUI := ctracecmd.so ctracecmdgui.so
PYTHON_VERS ?= python
# Can build python?
-ifeq ($(shell sh -c "pkg-config --cflags $(PYTHON_VERS) > /dev/null 2>&1 && which swig && echo y"), y)
+ifeq ($(shell sh -c "pkg-config --cflags $(PYTHON_VERS) > /dev/null 2>&1 && which swig > /dev/null && echo y"), y)
PYTHON_PLUGINS := plugin_python.so
BUILD_PYTHON := $(PYTHON) $(PYTHON_PLUGINS)
PYTHON_SO_INSTALL := ctracecmd.install
--
2.14.3
From: Zamir SUN <[email protected]>
Prior than this patch, Makefile detects python ldflags using a hardcoded
python command. It will cause problems if we are building against
python3 in the future when ldflags for python2 and python3 are
different. With this patch, python ldflags are detected by
corresponding python{,3}-config which will detect the right config for
python plugins.
Signed-off-by: Zamir SUN (Red Hat) <[email protected]>
---
Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index 7c0d1a6..f41e399 100644
--- a/Makefile
+++ b/Makefile
@@ -636,7 +636,7 @@ report_noswig: force
PYTHON_INCLUDES = `pkg-config --cflags $(PYTHON_VERS)`
PYTHON_LDFLAGS = `pkg-config --libs $(PYTHON_VERS)` \
- $(shell python2 -c "import distutils.sysconfig; print distutils.sysconfig.get_config_var('LINKFORSHARED')")
+ $(shell $(PYTHON_VERS)-config --ldflags)
PYGTK_CFLAGS = `pkg-config --cflags pygtk-2.0`
ctracecmd.so: $(TCMD_LIB_OBJS) ctracecmd.i
--
2.14.3
Hi Steve,
Do you have any thoughts for this?
Thanks.
On 02/04/2018 11:20 AM, [email protected] wrote:
> From: Zamir SUN (Red Hat) <[email protected]>
>
> This is a set of patch to improve trace-cmd python plugin support.
>
> Patch 1 fixes the detection for swig. With this patch, python-plugin can
> be built successfully.
>
> Patch 2 improves the detection for python ldflags. With this patch,
> ldflags can be detected with python3 in the future.
>
> Zamir SUN (2):
> trace-cmd: Fix the detection for swig
> trace-cmd: Change the way of getting python ldflags.
>
> Makefile | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
--
Zamir SUN
Fedora user
GPG : 1D86 6D4A 49CE 4BBD 72CF FCF5 D856 6E11 F2A0 525E
On Tue, 6 Feb 2018 20:31:54 +0800
Zamir SUN <[email protected]> wrote:
> Hi Steve,
>
> Do you have any thoughts for this?
>
Hi Zamir,
I just got back from traveling and I'm digging through email. I saw
your patches and do plan on looking at them this week.
-- Steve
On Sun, 4 Feb 2018 11:20:13 +0800
[email protected] wrote:
> From: Zamir SUN <[email protected]>
>
> The current detection for swig will cause output to be
> /usr/bin/swig
> y
> So this will never be equal to y. With this patch, the swig path is
> removed from output, so the detection can work as expected.
>
> Fixes 3bf187a43b7e6302592552ecbc294e5820249687
Hi Zamir,
Actually, a fix was already sent to me:
http://lkml.kernel.org/r/[email protected]
I'm working on add that one.
Thanks!
-- Steve
>
> Signed-off-by: Zamir SUN (Red Hat) <[email protected]>
> ---
> Makefile | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index a5d2c38..7c0d1a6 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -105,7 +105,7 @@ PYTHON_GUI := ctracecmd.so ctracecmdgui.so
> PYTHON_VERS ?= python
>
> # Can build python?
> -ifeq ($(shell sh -c "pkg-config --cflags $(PYTHON_VERS) > /dev/null 2>&1 && which swig && echo y"), y)
> +ifeq ($(shell sh -c "pkg-config --cflags $(PYTHON_VERS) > /dev/null 2>&1 && which swig > /dev/null && echo y"), y)
> PYTHON_PLUGINS := plugin_python.so
> BUILD_PYTHON := $(PYTHON) $(PYTHON_PLUGINS)
> PYTHON_SO_INSTALL := ctracecmd.install
On 02/07/2018 07:18 AM, Steven Rostedt wrote:
> On Sun, 4 Feb 2018 11:20:13 +0800
> [email protected] wrote:
>
>> From: Zamir SUN <[email protected]>
>>
>> The current detection for swig will cause output to be
>> /usr/bin/swig
>> y
>> So this will never be equal to y. With this patch, the swig path is
>> removed from output, so the detection can work as expected.
>>
>> Fixes 3bf187a43b7e6302592552ecbc294e5820249687
>
> Hi Zamir,
>
> Actually, a fix was already sent to me:
>
> http://lkml.kernel.org/r/[email protected]
>
> I'm working on add that one.
>
Thanks for the heads-up! I did not see that before.
While just a note for this, in Fedora 27:
$ if command -v swig; then echo 1; else echo 0; fi
/usr/bin/swig
1
Actually this test has the same problem as of `which swig` - When swig
exists, the var SWIG_DEFINED will not be 1 (as shown above).
I see Vladislav is already on this thread. I hope this can be fixed
before merging the patch set.
> Thanks!
>
> -- Steve
>
>
>>
>> Signed-off-by: Zamir SUN (Red Hat) <[email protected]>
>> ---
>> Makefile | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/Makefile b/Makefile
>> index a5d2c38..7c0d1a6 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -105,7 +105,7 @@ PYTHON_GUI := ctracecmd.so ctracecmdgui.so
>> PYTHON_VERS ?= python
>>
>> # Can build python?
>> -ifeq ($(shell sh -c "pkg-config --cflags $(PYTHON_VERS) > /dev/null 2>&1 && which swig && echo y"), y)
>> +ifeq ($(shell sh -c "pkg-config --cflags $(PYTHON_VERS) > /dev/null 2>&1 && which swig > /dev/null && echo y"), y)
>> PYTHON_PLUGINS := plugin_python.so
>> BUILD_PYTHON := $(PYTHON) $(PYTHON_PLUGINS)
>> PYTHON_SO_INSTALL := ctracecmd.install
>
--
Zamir SUN
Fedora user
GPG : 1D86 6D4A 49CE 4BBD 72CF FCF5 D856 6E11 F2A0 525E
On Wed, 7 Feb 2018 22:01:46 +0800
Zamir SUN <[email protected]> wrote:
> Thanks for the heads-up! I did not see that before.
>
> While just a note for this, in Fedora 27:
>
> $ if command -v swig; then echo 1; else echo 0; fi
> /usr/bin/swig
> 1
>
> Actually this test has the same problem as of `which swig` - When swig
> exists, the var SWIG_DEFINED will not be 1 (as shown above).
>
> I see Vladislav is already on this thread. I hope this can be fixed
> before merging the patch set.
>
After doing some more testing, I'm going to push out the changes today.
Would you be able to add patches on top of the changes? We are doing a
restructuring of the directory such that it can grow more.
I'll be backporting these fixes to 2.6 and 2.7 as well.
Thanks!
-- Steve
On 02/07/2018 10:40 PM, Steven Rostedt wrote:
> On Wed, 7 Feb 2018 22:01:46 +0800
> Zamir SUN <[email protected]> wrote:
>
>
>> Thanks for the heads-up! I did not see that before.
>>
>> While just a note for this, in Fedora 27:
>>
>> $ if command -v swig; then echo 1; else echo 0; fi
>> /usr/bin/swig
>> 1
>>
>> Actually this test has the same problem as of `which swig` - When swig
>> exists, the var SWIG_DEFINED will not be 1 (as shown above).
>>
>> I see Vladislav is already on this thread. I hope this can be fixed
>> before merging the patch set.
>>
>
> After doing some more testing, I'm going to push out the changes today.
> Would you be able to add patches on top of the changes? We are doing a
> restructuring of the directory such that it can grow more.
>
> I'll be backporting these fixes to 2.6 and 2.7 as well.
>
Sure, I will rebase after Vladislav's patch set is merged.
Thanks!
--
Zamir SUN
Fedora user
GPG : 1D86 6D4A 49CE 4BBD 72CF FCF5 D856 6E11 F2A0 525E
On Wed, 2018-02-07 at 22:01 +0800, Zamir SUN wrote:
>
> While just a note for this, in Fedora 27:
>
> $ if command -v swig; then echo 1; else echo 0; fi
> /usr/bin/swig
> 1
>
> Actually this test has the same problem as of `which swig` - When swig
> exists, the var SWIG_DEFINED will not be 1 (as shown above).
>
> I see Vladislav is already on this thread. I hope this can be fixed
> before merging the patch set.
>
Hi Zamir,
I'm sorry but I'm unable to reproduce the issue on Fedora 27,
after applying my series of patches.
In my case (fresh install of Fedora27, no customizations):
if command -v swig; then echo 1; else echo 0; fi
works as expected.
SWIG_DEFINED does not have to be '1' in order the Makefile to work.
Because the check in my branch is:
ifeq ($(SWIG_DEFINED), 0)
BUILD_PYTHON := report_noswig
NO_PYTHON = 1
endif
So there should be no problem. Just to be sure, I've tried to compile trace-cmd
with my patches first without 'swig' and later with it (on Fedora).
The result was that, in the first case the message 'no swig installed'
was displayed, while in the other case it was not. Or better, a message
saying that 'python-dev' is missing was shown.
[Yes, both swig and python-dev are necessary].
After installing also 'python-devel' (in Ubuntu is just '-dev'), the
build system was able to build the rest of the python targets as well.
Could you please try by using my fork on GitHub?
https://github.com/vvaltchev/trace-cmd-fork
BRANCH NAME: make8.
Let me know if you still experience the issue.
Thanks,
Vlad
--
Vladislav Valtchev
VMware Open Source Technology Center
On 02/08/2018 01:25 AM, Vladislav Valtchev wrote:
> On Wed, 2018-02-07 at 22:01 +0800, Zamir SUN wrote:
>>
>> While just a note for this, in Fedora 27:
>>
>> $ if command -v swig; then echo 1; else echo 0; fi
>> /usr/bin/swig
>> 1
>>
>> Actually this test has the same problem as of `which swig` - When swig
>> exists, the var SWIG_DEFINED will not be 1 (as shown above).
>>
>> I see Vladislav is already on this thread. I hope this can be fixed
>> before merging the patch set.
>>
>
> Hi Zamir,
> I'm sorry but I'm unable to reproduce the issue on Fedora 27,
> after applying my series of patches.
>
> In my case (fresh install of Fedora27, no customizations):
> if command -v swig; then echo 1; else echo 0; fi
> works as expected.
>
> SWIG_DEFINED does not have to be '1' in order the Makefile to work.
> Because the check in my branch is:
>
> ifeq ($(SWIG_DEFINED), 0)
> BUILD_PYTHON := report_noswig
> NO_PYTHON = 1
> endif
>
> So there should be no problem. Just to be sure, I've tried to compile trace-cmd
> with my patches first without 'swig' and later with it (on Fedora).
> The result was that, in the first case the message 'no swig installed'
> was displayed, while in the other case it was not. Or better, a message
> saying that 'python-dev' is missing was shown.
> [Yes, both swig and python-dev are necessary].
>
> After installing also 'python-devel' (in Ubuntu is just '-dev'), the
> build system was able to build the rest of the python targets as well.
>
>
> Could you please try by using my fork on GitHub?
> https://github.com/vvaltchev/trace-cmd-fork
> BRANCH NAME: make8.
>
> Let me know if you still experience the issue.
>
Hi Vladislav,
Thanks for the URL. I tried to build from your make8 branch, it works
fine. I must made some copy-paste mistake yesterday.
Sorry for the noise.
Thanks.
> Thanks,
> Vlad
>
>
>
--
Zamir SUN
Fedora user
GPG : 1D86 6D4A 49CE 4BBD 72CF FCF5 D856 6E11 F2A0 525E
On Thu, 2018-02-08 at 20:05 +0800, Zamir SUN wrote:
>
> Hi Vladislav,
>
> Thanks for the URL. I tried to build from your make8 branch, it works
> fine. I must made some copy-paste mistake yesterday.
>
> Sorry for the noise.
>
> Thanks.
No problem, Zamir.
I'm happy that it works.
The series has been merged in the master branch by Steven now.
Anyway, I think your patch 2/2 is still necessary: thanks for it!
Vlad
--
Vladislav Valtchev
VMware Open Source Technology Center
On Sun, 4 Feb 2018 11:20:14 +0800
[email protected] wrote:
> From: Zamir SUN <[email protected]>
>
> Prior than this patch, Makefile detects python ldflags using a hardcoded
> python command. It will cause problems if we are building against
> python3 in the future when ldflags for python2 and python3 are
> different. With this patch, python ldflags are detected by
> corresponding python{,3}-config which will detect the right config for
> python plugins.
Thanks, applied.
-- Steve
On Sun, 4 Feb 2018 11:20:14 +0800
[email protected] wrote:
> From: Zamir SUN <[email protected]>
>
> Prior than this patch, Makefile detects python ldflags using a hardcoded
> python command. It will cause problems if we are building against
> python3 in the future when ldflags for python2 and python3 are
> different. With this patch, python ldflags are detected by
> corresponding python{,3}-config which will detect the right config for
> python plugins.
>
> Signed-off-by: Zamir SUN (Red Hat) <[email protected]>
> ---
> Makefile | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index 7c0d1a6..f41e399 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -636,7 +636,7 @@ report_noswig: force
>
> PYTHON_INCLUDES = `pkg-config --cflags $(PYTHON_VERS)`
> PYTHON_LDFLAGS = `pkg-config --libs $(PYTHON_VERS)` \
> - $(shell python2 -c "import distutils.sysconfig; print distutils.sysconfig.get_config_var('LINKFORSHARED')")
> + $(shell $(PYTHON_VERS)-config --ldflags)
> PYGTK_CFLAGS = `pkg-config --cflags pygtk-2.0`
>
> ctracecmd.so: $(TCMD_LIB_OBJS) ctracecmd.i
BTW, I did have this applied, but when testing it caused warnings. I
had to apply this patch to fix it:
-- Steve
From d5579b729114135da20bcee6896a0683ff54f33a Mon Sep 17 00:00:00 2001
From: "Steven Rostedt (VMware)" <[email protected]>
Date: Thu, 22 Mar 2018 18:38:22 -0400
Subject: [PATCH] trace-cmd build: Do not execute python scripts if there is no
python
I was triggering the following build messages when not having swig installed:
make: -config: Command not found
NO_PYTHON forced: swig not installed, not compiling python plugins
make: -config: Command not found
UPDATE trace_python_dir
make: -config: Command not found
This is due to the executing of $(PYTHON_VERS)-config, which would just turn
into "-config". Do not assign the PYTHON_* variables if NO_PYTHON is
defined.
Signed-off-by: Steven Rostedt (VMware) <[email protected]>
---
Makefile | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Makefile b/Makefile
index 2c82301..26fd42b 100644
--- a/Makefile
+++ b/Makefile
@@ -373,10 +373,16 @@ report_nopythondev: force
$(Q)echo " python-dev is not installed, not compiling python plugins"
$(Q)echo
+ifndef NO_PYTHON
PYTHON_INCLUDES = `pkg-config --cflags $(PYTHON_VERS)`
PYTHON_LDFLAGS = `pkg-config --libs $(PYTHON_VERS)` \
$(shell $(PYTHON_VERS)-config --ldflags)
PYGTK_CFLAGS = `pkg-config --cflags pygtk-2.0`
+else
+PYTHON_INCLUDES =
+PYTHON_LDFLAGS =
+PYGTK_CFLAGS =
+endif
export PYTHON_INCLUDES
export PYTHON_LDFLAGS
--
2.13.6
On 03/23/2018 07:13 AM, Steven Rostedt wrote:
> On Sun, 4 Feb 2018 11:20:14 +0800
> [email protected] wrote:
>
>> From: Zamir SUN <[email protected]>
>>
>> Prior than this patch, Makefile detects python ldflags using a hardcoded
>> python command. It will cause problems if we are building against
>> python3 in the future when ldflags for python2 and python3 are
>> different. With this patch, python ldflags are detected by
>> corresponding python{,3}-config which will detect the right config for
>> python plugins.
>>
>> Signed-off-by: Zamir SUN (Red Hat) <[email protected]>
>> ---
>> Makefile | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/Makefile b/Makefile
>> index 7c0d1a6..f41e399 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -636,7 +636,7 @@ report_noswig: force
>>
>> PYTHON_INCLUDES = `pkg-config --cflags $(PYTHON_VERS)`
>> PYTHON_LDFLAGS = `pkg-config --libs $(PYTHON_VERS)` \
>> - $(shell python2 -c "import distutils.sysconfig; print distutils.sysconfig.get_config_var('LINKFORSHARED')")
>> + $(shell $(PYTHON_VERS)-config --ldflags)
>> PYGTK_CFLAGS = `pkg-config --cflags pygtk-2.0`
>>
>> ctracecmd.so: $(TCMD_LIB_OBJS) ctracecmd.i
>
> BTW, I did have this applied, but when testing it caused warnings. I
> had to apply this patch to fix it:
>
> -- Steve
Thanks for the info!
I am always using Fedora and Python is always preloaded so I did not
think of this scenario.
>
> From d5579b729114135da20bcee6896a0683ff54f33a Mon Sep 17 00:00:00 2001
> From: "Steven Rostedt (VMware)" <[email protected]>
> Date: Thu, 22 Mar 2018 18:38:22 -0400
> Subject: [PATCH] trace-cmd build: Do not execute python scripts if there is no
> python
>
> I was triggering the following build messages when not having swig installed:
>
> make: -config: Command not found
>
> NO_PYTHON forced: swig not installed, not compiling python plugins
>
> make: -config: Command not found
> UPDATE trace_python_dir
> make: -config: Command not found
>
> This is due to the executing of $(PYTHON_VERS)-config, which would just turn
> into "-config". Do not assign the PYTHON_* variables if NO_PYTHON is
> defined.
>
> Signed-off-by: Steven Rostedt (VMware) <[email protected]>
> ---
> Makefile | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/Makefile b/Makefile
> index 2c82301..26fd42b 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -373,10 +373,16 @@ report_nopythondev: force
> $(Q)echo " python-dev is not installed, not compiling python plugins"
> $(Q)echo
>
> +ifndef NO_PYTHON
> PYTHON_INCLUDES = `pkg-config --cflags $(PYTHON_VERS)`
> PYTHON_LDFLAGS = `pkg-config --libs $(PYTHON_VERS)` \
> $(shell $(PYTHON_VERS)-config --ldflags)
> PYGTK_CFLAGS = `pkg-config --cflags pygtk-2.0`
> +else
> +PYTHON_INCLUDES =
> +PYTHON_LDFLAGS =
> +PYGTK_CFLAGS =
> +endif
>
> export PYTHON_INCLUDES
> export PYTHON_LDFLAGS
>
--
Zamir SUN
Fedora user
GPG : 1D86 6D4A 49CE 4BBD 72CF FCF5 D856 6E11 F2A0 525E