Re: [Intel-gfx] [PATCH i-g-t 2/2] Revert "lib/igt_aux: Make procps optional"

2017-11-29 Thread Daniel Vetter 
On Wed, Nov 29, 2017 at 03:24:41PM +0200, Arkadiusz Hiler wrote:
> On Wed, Nov 29, 2017 at 12:07:06PM +0100, Daniel Vetter wrote:
> > On Fri, Nov 24, 2017 at 05:17:48PM +0200, Arkadiusz Hiler wrote:
> > > This reverts commit d7d3f4e87b827152f00bdf89a67871736672b492
> > > and gets rid of the config option from the meson.build.
> > > 
> > > It was needed only for the Android support.
> > > 
> > > Signed-off-by: Arkadiusz Hiler 
> > 
> > Acked-by: Daniel Vetter 
> > 
> > on both patches.
> > 
> > I think there's a bunch more things that are only optional because of
> > Android. Stuff like udev and glib iirc. But maybe we want to keep those,
> > to avoid to much pain for the next time around someone wants to implement
> > Android support natively.
> > -Daniel
> 
> Pushed, thanks for Acks and R-bs!
> 
> As of further cleanups, there is one really impending on us
> - the libunwind one.
> 
> We have huge sections of the code wrapped in ifdefs which bit us more
> than once - it's easy to misplace code in there, code that should always
> be compiled.
> 
> This needs a rework, initial ideas is to put all the unwind related mess
> into separate file and compile the whole thing conditionally - for
> clearer separation.
> 
> We would also need "fallback" noop implementation of some of those
> functions.
> 
> Or we may ask ourself, with Android support gone, is this really worth
> it and shouldn't we make libunwind non-optional and just get rid of the
> preprocessor macors? :-)

+1 on simply requiring libunwind. The other option is more work, and can
be done when we have a real ask for it (aka Android, in which case your
suggested split-out is probably what we want).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t 2/2] Revert "lib/igt_aux: Make procps optional"

2017-11-29 Thread Arkadiusz Hiler 
On Wed, Nov 29, 2017 at 12:07:06PM +0100, Daniel Vetter wrote:
> On Fri, Nov 24, 2017 at 05:17:48PM +0200, Arkadiusz Hiler wrote:
> > This reverts commit d7d3f4e87b827152f00bdf89a67871736672b492
> > and gets rid of the config option from the meson.build.
> > 
> > It was needed only for the Android support.
> > 
> > Signed-off-by: Arkadiusz Hiler 
> 
> Acked-by: Daniel Vetter 
> 
> on both patches.
> 
> I think there's a bunch more things that are only optional because of
> Android. Stuff like udev and glib iirc. But maybe we want to keep those,
> to avoid to much pain for the next time around someone wants to implement
> Android support natively.
> -Daniel

Pushed, thanks for Acks and R-bs!

As of further cleanups, there is one really impending on us
- the libunwind one.

We have huge sections of the code wrapped in ifdefs which bit us more
than once - it's easy to misplace code in there, code that should always
be compiled.

This needs a rework, initial ideas is to put all the unwind related mess
into separate file and compile the whole thing conditionally - for
clearer separation.

We would also need "fallback" noop implementation of some of those
functions.

Or we may ask ourself, with Android support gone, is this really worth
it and shouldn't we make libunwind non-optional and just get rid of the
preprocessor macors? :-)

-- 
Cheers,
Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t 2/2] Revert "lib/igt_aux: Make procps optional"

2017-11-29 Thread Daniel Vetter 
On Fri, Nov 24, 2017 at 05:17:48PM +0200, Arkadiusz Hiler wrote:
> This reverts commit d7d3f4e87b827152f00bdf89a67871736672b492
> and gets rid of the config option from the meson.build.
> 
> It was needed only for the Android support.
> 
> Signed-off-by: Arkadiusz Hiler 

Acked-by: Daniel Vetter 

on both patches.

I think there's a bunch more things that are only optional because of
Android. Stuff like udev and glib iirc. But maybe we want to keep those,
to avoid to much pain for the next time around someone wants to implement
Android support natively.
-Daniel
> ---
>  configure.ac  |  6 +-
>  lib/igt_aux.c | 35 +++
>  meson.build   |  5 +
>  3 files changed, 5 insertions(+), 41 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 1ac2e8e8..84c6e646 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -123,11 +123,7 @@ AC_SUBST(ASSEMBLER_WARN_CFLAGS)
>  PKG_CHECK_MODULES(DRM, [libdrm >= 2.4.82])
>  PKG_CHECK_MODULES(PCIACCESS, [pciaccess >= 0.10])
>  PKG_CHECK_MODULES(KMOD, [libkmod])
> -PKG_CHECK_MODULES(PROCPS, [libprocps], [procps=yes], [procps=no])
> -AM_CONDITIONAL(HAVE_PROCPS, [test "x$procps" = xyes])
> -if test x"$procps" = xyes; then
> - AC_DEFINE(HAVE_PROCPS,1,[Enable process managment without shelling out])
> -fi
> +PKG_CHECK_MODULES(PROCPS, [libprocps])
>  PKG_CHECK_MODULES(VALGRIND, [valgrind], [have_valgrind=yes], 
> [have_valgrind=no])
>  
>  if test x$have_valgrind = xyes; then
> diff --git a/lib/igt_aux.c b/lib/igt_aux.c
> index a41ae2f1..e2424109 100644
> --- a/lib/igt_aux.c
> +++ b/lib/igt_aux.c
> @@ -48,7 +48,9 @@
>  #include 
>  #include 
>  #include 
> -#include 
> +
> +#include 
> +
>  #include "drmtest.h"
>  #include "i915_drm.h"
>  #include "intel_chipset.h"
> @@ -68,10 +70,6 @@
>  #include/* for dirname() */
>  #endif
>  
> -#ifdef HAVE_PROCPS
> -#include 
> -#endif
> -
>  /**
>   * SECTION:igt_aux
>   * @short_description: Auxiliary libraries and support functions
> @@ -1296,7 +1294,6 @@ void igt_set_module_param_int(const char *name, int val)
>   * This function sends the signal @sig for a process found in process table
>   * with name @comm.
>   */
> -#ifdef HAVE_PROCPS
>  int igt_terminate_process(int sig, const char *comm)
>  {
>   PROCTAB *proc;
> @@ -1321,19 +1318,7 @@ int igt_terminate_process(int sig, const char *comm)
>   closeproc(proc);
>   return err;
>  }
> -#else
> -#warning "No procps, using naive implementation of igt_terminate_process"
>  
> -int igt_terminate_process(int sig, const char *comm)
> -{
> - char pkill_cmd[NAME_MAX];
> -
> - snprintf(pkill_cmd, sizeof(pkill_cmd), "pkill -x -%d %s", sig, comm);
> - return system(pkill_cmd);
> -}
> -#endif
> -
> -#ifdef HAVE_PROCPS
>  struct pinfo {
>   pid_t pid;
>   const char *comm;
> @@ -1515,7 +1500,6 @@ __igt_lsof(const char *dir)
>  
>   closeproc(proc);
>  }
> -#endif
>  
>  /**
>   * igt_lsof: Lists information about files opened by processes.
> @@ -1524,7 +1508,6 @@ __igt_lsof(const char *dir)
>   * This function mimics (a restrictive form of) lsof(8), but also shows
>   * information about opened fds.
>   */
> -#ifdef HAVE_PROCPS
>  void
>  igt_lsof(const char *dpath)
>  {
> @@ -1549,18 +1532,6 @@ igt_lsof(const char *dpath)
>  
>   free(sanitized);
>  }
> -#else
> -#warning "No procps, using naive implementation of igt_lsof"
> -
> -void
> -igt_lsof(const char *dpath)
> -{
> - char lsof_cmd[NAME_MAX];
> -
> - snprintf(lsof_cmd, sizeof(lsof_cmd), "lsof +d %s", dpath);
> - system(lsof_cmd);
> -}
> -#endif
>  
>  static struct igt_siglatency {
>   timer_t timer;
> diff --git a/meson.build b/meson.build
> index 2361866b..8e01b05d 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -37,10 +37,7 @@ libdrm_amdgpu = dependency('libdrm_amdgpu', required : 
> false)
>  
>  pciaccess = dependency('pciaccess', version : '>=0.10')
>  libkmod = dependency('libkmod')
> -libprocps = dependency('libprocps', required : false)
> -if libprocps.found()
> - config.set('HAVE_PROCPS', 1)
> -endif
> +libprocps = dependency('libprocps', required : true)
>  
>  valgrind = dependency('valgrind', required : false)
>  if valgrind.found()
> -- 
> 2.13.6
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH i-g-t 2/2] Revert "lib/igt_aux: Make procps optional"

2017-11-24 Thread Arkadiusz Hiler 
This reverts commit d7d3f4e87b827152f00bdf89a67871736672b492
and gets rid of the config option from the meson.build.

It was needed only for the Android support.

Signed-off-by: Arkadiusz Hiler 
---
 configure.ac  |  6 +-
 lib/igt_aux.c | 35 +++
 meson.build   |  5 +
 3 files changed, 5 insertions(+), 41 deletions(-)

diff --git a/configure.ac b/configure.ac
index 1ac2e8e8..84c6e646 100644
--- a/configure.ac
+++ b/configure.ac
@@ -123,11 +123,7 @@ AC_SUBST(ASSEMBLER_WARN_CFLAGS)
 PKG_CHECK_MODULES(DRM, [libdrm >= 2.4.82])
 PKG_CHECK_MODULES(PCIACCESS, [pciaccess >= 0.10])
 PKG_CHECK_MODULES(KMOD, [libkmod])
-PKG_CHECK_MODULES(PROCPS, [libprocps], [procps=yes], [procps=no])
-AM_CONDITIONAL(HAVE_PROCPS, [test "x$procps" = xyes])
-if test x"$procps" = xyes; then
-   AC_DEFINE(HAVE_PROCPS,1,[Enable process managment without shelling out])
-fi
+PKG_CHECK_MODULES(PROCPS, [libprocps])
 PKG_CHECK_MODULES(VALGRIND, [valgrind], [have_valgrind=yes], 
[have_valgrind=no])
 
 if test x$have_valgrind = xyes; then
diff --git a/lib/igt_aux.c b/lib/igt_aux.c
index a41ae2f1..e2424109 100644
--- a/lib/igt_aux.c
+++ b/lib/igt_aux.c
@@ -48,7 +48,9 @@
 #include 
 #include 
 #include 
-#include 
+
+#include 
+
 #include "drmtest.h"
 #include "i915_drm.h"
 #include "intel_chipset.h"
@@ -68,10 +70,6 @@
 #include/* for dirname() */
 #endif
 
-#ifdef HAVE_PROCPS
-#include 
-#endif
-
 /**
  * SECTION:igt_aux
  * @short_description: Auxiliary libraries and support functions
@@ -1296,7 +1294,6 @@ void igt_set_module_param_int(const char *name, int val)
  * This function sends the signal @sig for a process found in process table
  * with name @comm.
  */
-#ifdef HAVE_PROCPS
 int igt_terminate_process(int sig, const char *comm)
 {
PROCTAB *proc;
@@ -1321,19 +1318,7 @@ int igt_terminate_process(int sig, const char *comm)
closeproc(proc);
return err;
 }
-#else
-#warning "No procps, using naive implementation of igt_terminate_process"
 
-int igt_terminate_process(int sig, const char *comm)
-{
-   char pkill_cmd[NAME_MAX];
-
-   snprintf(pkill_cmd, sizeof(pkill_cmd), "pkill -x -%d %s", sig, comm);
-   return system(pkill_cmd);
-}
-#endif
-
-#ifdef HAVE_PROCPS
 struct pinfo {
pid_t pid;
const char *comm;
@@ -1515,7 +1500,6 @@ __igt_lsof(const char *dir)
 
closeproc(proc);
 }
-#endif
 
 /**
  * igt_lsof: Lists information about files opened by processes.
@@ -1524,7 +1508,6 @@ __igt_lsof(const char *dir)
  * This function mimics (a restrictive form of) lsof(8), but also shows
  * information about opened fds.
  */
-#ifdef HAVE_PROCPS
 void
 igt_lsof(const char *dpath)
 {
@@ -1549,18 +1532,6 @@ igt_lsof(const char *dpath)
 
free(sanitized);
 }
-#else
-#warning "No procps, using naive implementation of igt_lsof"
-
-void
-igt_lsof(const char *dpath)
-{
-   char lsof_cmd[NAME_MAX];
-
-   snprintf(lsof_cmd, sizeof(lsof_cmd), "lsof +d %s", dpath);
-   system(lsof_cmd);
-}
-#endif
 
 static struct igt_siglatency {
timer_t timer;
diff --git a/meson.build b/meson.build
index 2361866b..8e01b05d 100644
--- a/meson.build
+++ b/meson.build
@@ -37,10 +37,7 @@ libdrm_amdgpu = dependency('libdrm_amdgpu', required : false)
 
 pciaccess = dependency('pciaccess', version : '>=0.10')
 libkmod = dependency('libkmod')
-libprocps = dependency('libprocps', required : false)
-if libprocps.found()
-   config.set('HAVE_PROCPS', 1)
-endif
+libprocps = dependency('libprocps', required : true)
 
 valgrind = dependency('valgrind', required : false)
 if valgrind.found()
-- 
2.13.6

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH i-g-t 2/2] Revert "lib/igt_aux: Make procps optional"

2017-09-13 Thread Arkadiusz Hiler 
This reverts commit d7d3f4e87b827152f00bdf89a67871736672b492
and gets rid of the config option from the meson.build.

It was needed only for the Android support.

Signed-off-by: Arkadiusz Hiler 
---
 configure.ac  |  6 +-
 lib/igt_aux.c | 35 +++
 meson.build   |  5 +
 3 files changed, 5 insertions(+), 41 deletions(-)

diff --git a/configure.ac b/configure.ac
index c78f5194..2dc03eb2 100644
--- a/configure.ac
+++ b/configure.ac
@@ -123,11 +123,7 @@ AC_SUBST(ASSEMBLER_WARN_CFLAGS)
 PKG_CHECK_MODULES(DRM, [libdrm >= 2.4.82])
 PKG_CHECK_MODULES(PCIACCESS, [pciaccess >= 0.10])
 PKG_CHECK_MODULES(KMOD, [libkmod])
-PKG_CHECK_MODULES(PROCPS, [libprocps], [procps=yes], [procps=no])
-AM_CONDITIONAL(HAVE_PROCPS, [test "x$procps" = xyes])
-if test x"$procps" = xyes; then
-   AC_DEFINE(HAVE_PROCPS,1,[Enable process managment without shelling out])
-fi
+PKG_CHECK_MODULES(PROCPS, [libprocps])
 PKG_CHECK_MODULES(VALGRIND, [valgrind], [have_valgrind=yes], 
[have_valgrind=no])
 
 if test x$have_valgrind = xyes; then
diff --git a/lib/igt_aux.c b/lib/igt_aux.c
index 295e435c..76b2e52a 100644
--- a/lib/igt_aux.c
+++ b/lib/igt_aux.c
@@ -48,7 +48,9 @@
 #include 
 #include 
 #include 
-#include 
+
+#include 
+
 #include "drmtest.h"
 #include "i915_drm.h"
 #include "intel_chipset.h"
@@ -68,10 +70,6 @@
 #include/* for dirname() */
 #endif
 
-#ifdef HAVE_PROCPS
-#include 
-#endif
-
 /**
  * SECTION:igt_aux
  * @short_description: Auxiliary libraries and support functions
@@ -1242,7 +1240,6 @@ void igt_set_module_param_int(const char *name, int val)
  * This function sends the signal @sig for a process found in process table
  * with name @comm.
  */
-#ifdef HAVE_PROCPS
 int igt_terminate_process(int sig, const char *comm)
 {
PROCTAB *proc;
@@ -1267,19 +1264,7 @@ int igt_terminate_process(int sig, const char *comm)
closeproc(proc);
return err;
 }
-#else
-#warning "No procps, using naive implementation of igt_terminate_process"
 
-int igt_terminate_process(int sig, const char *comm)
-{
-   char pkill_cmd[NAME_MAX];
-
-   snprintf(pkill_cmd, sizeof(pkill_cmd), "pkill -x -%d %s", sig, comm);
-   return system(pkill_cmd);
-}
-#endif
-
-#ifdef HAVE_PROCPS
 struct pinfo {
pid_t pid;
const char *comm;
@@ -1461,7 +1446,6 @@ __igt_lsof(const char *dir)
 
closeproc(proc);
 }
-#endif
 
 /**
  * igt_lsof: Lists information about files opened by processes.
@@ -1470,7 +1454,6 @@ __igt_lsof(const char *dir)
  * This function mimics (a restrictive form of) lsof(8), but also shows
  * information about opened fds.
  */
-#ifdef HAVE_PROCPS
 void
 igt_lsof(const char *dpath)
 {
@@ -1495,18 +1478,6 @@ igt_lsof(const char *dpath)
 
free(sanitized);
 }
-#else
-#warning "No procps, using naive implementation of igt_lsof"
-
-void
-igt_lsof(const char *dpath)
-{
-   char lsof_cmd[NAME_MAX];
-
-   snprintf(lsof_cmd, sizeof(lsof_cmd), "lsof +d %s", dpath);
-   system(lsof_cmd);
-}
-#endif
 
 static struct igt_siglatency {
timer_t timer;
diff --git a/meson.build b/meson.build
index 0858354f..8f077602 100644
--- a/meson.build
+++ b/meson.build
@@ -38,10 +38,7 @@ libdrm_amdgpu = dependency('libdrm_amdgpu', required : false)
 
 pciaccess = dependency('pciaccess', version : '>=0.10')
 libkmod = dependency('libkmod')
-libprocps = dependency('libprocps', required : false)
-if libprocps.found()
-   config_h.set('HAVE_PROCPS', 1)
-endif
+libprocps = dependency('libprocps', required : true)
 
 valgrind = dependency('valgrind', required : false)
 if valgrind.found()
-- 
2.13.5

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx