| Message ID | 20171124151748.2564-2-arkadiusz.hiler@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <arkadiusz.hiler@intel.com> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> 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 <sys/utsname.h> > #include <termios.h> > #include <assert.h> > -#include <linux/limits.h> > + > +#include <proc/readproc.h> > + > #include "drmtest.h" > #include "i915_drm.h" > #include "intel_chipset.h" > @@ -68,10 +70,6 @@ > #include <libgen.h> /* for dirname() */ > #endif > > -#ifdef HAVE_PROCPS > -#include <proc/readproc.h> > -#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
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 <arkadiusz.hiler@intel.com> > > Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > 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? :-)
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 <arkadiusz.hiler@intel.com> > > > > Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > > > 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
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 <sys/utsname.h> #include <termios.h> #include <assert.h> -#include <linux/limits.h> + +#include <proc/readproc.h> + #include "drmtest.h" #include "i915_drm.h" #include "intel_chipset.h" @@ -68,10 +70,6 @@ #include <libgen.h> /* for dirname() */ #endif -#ifdef HAVE_PROCPS -#include <proc/readproc.h> -#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()
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 <arkadiusz.hiler@intel.com> --- configure.ac | 6 +----- lib/igt_aux.c | 35 +++-------------------------------- meson.build | 5 +---- 3 files changed, 5 insertions(+), 41 deletions(-)