Re: [PATCH libinput 1/5] tools: if the execdir is the builddir, add it to the path
On Wed, Jul 04, 2018 at 11:29:26AM +0100, Emil Velikov wrote: > Hi Peter, > > On 28 June 2018 at 00:51, Peter Hutterer wrote: > > When running libinput tools from the builddir, look up the subtools in the > > builddir as well. Otherwise, add the install prefix to the list of lookup > > locations. > > > > This ensures that a) we're running builddir stuff against builddir stuff, > > but > > also b) that we're not running builddir stuff against installed stuff > > because > > that may give us false positives. > > > > Signed-off-by: Peter Hutterer > > --- > > Similar-ish to the git approach Emil suggested in [1] but git is a lot more > > involved here. Since we only need this for debugging tools, we can pick the > > simpler approach here and simply ignore most of the corner cases. > > > I think that git does thing in that particular way is to ensure a > consistent lookup approach. yeah, git cares a lot more about different platforms and features and it works around a relocatable (relative) execdir. And comparing to the prefix is 'safer' because you're not leaking the builddir into the binary. But for this particular use-case ... meh :) > Personally tools_execdir_is_builddir reads like it should return a > bool, sadly no better name comes to mind. indeed. I'll see if I come up with something more telling but given the limited impact I'm happy enough with the current name. Thanks for the review. Cheers, Peter ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput 1/5] tools: if the execdir is the builddir, add it to the path
Hi Peter, On 28 June 2018 at 00:51, Peter Hutterer wrote: > When running libinput tools from the builddir, look up the subtools in the > builddir as well. Otherwise, add the install prefix to the list of lookup > locations. > > This ensures that a) we're running builddir stuff against builddir stuff, but > also b) that we're not running builddir stuff against installed stuff because > that may give us false positives. > > Signed-off-by: Peter Hutterer > --- > Similar-ish to the git approach Emil suggested in [1] but git is a lot more > involved here. Since we only need this for debugging tools, we can pick the > simpler approach here and simply ignore most of the corner cases. > I think that git does thing in that particular way is to ensure a consistent lookup approach. Personally tools_execdir_is_builddir reads like it should return a bool, sadly no better name comes to mind. More of an jfyi, than anything else. HTH Emil ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput 1/5] tools: if the execdir is the builddir, add it to the path
On Thu, 28 Jun 2018 09:51:39 +1000
Peter Hutterer wrote:
> When running libinput tools from the builddir, look up the subtools in the
> builddir as well. Otherwise, add the install prefix to the list of lookup
> locations.
>
> This ensures that a) we're running builddir stuff against builddir stuff, but
> also b) that we're not running builddir stuff against installed stuff because
> that may give us false positives.
>
> Signed-off-by: Peter Hutterer
> ---
> Similar-ish to the git approach Emil suggested in [1] but git is a lot more
> involved here. Since we only need this for debugging tools, we can pick the
> simpler approach here and simply ignore most of the corner cases.
>
> [1] https://lists.freedesktop.org/archives/wayland-devel/2018-June/038658.html
Hi Peter,
this is an interesting idea. My worry is that
tools_execdir_is_builddir() might incorrectly return NULL and you
wouldn't know, so I would propose to add a test that specifically tests
this function to return the correct path when running under the test
suite. Probably quite difficult to test the opposite too?
The code looks correct to me, but I haven't read it in the context of
the surrounding code. I might some day steal this for Weston. :-)
Thanks,
pq
>
> meson.build| 1 +
> tools/shared.c | 31 ++-
> tools/shared.h | 3 +++
> 3 files changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/meson.build b/meson.build
> index 58e909b6..5580086f 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -31,6 +31,7 @@ add_project_arguments(cppflags, language : 'cpp')
>
> config_h = configuration_data()
> config_h.set('_GNU_SOURCE', '1')
> +config_h.set_quoted('MESON_BUILD_ROOT', meson.build_root())
>
> prefix = '''#define _GNU_SOURCE 1
> #include
> diff --git a/tools/shared.c b/tools/shared.c
> index f2ed1fd0..776b3d3e 100644
> --- a/tools/shared.c
> +++ b/tools/shared.c
> @@ -467,18 +467,47 @@ out:
> return is_touchpad;
> }
>
> +/* Try to read the directory we're executing from and if it matches the
> + * builddir, return it as path. Otherwise, return NULL.
> + */
> +char *
> +tools_execdir_is_builddir(void)
> +{
> + char execdir[PATH_MAX];
> + char *pathsep;
> + ssize_t sz;
> +
> + sz = readlink("/proc/self/exe", execdir, sizeof(execdir));
> + if (sz <= 0 || sz == sizeof(execdir))
> + return NULL;
> +
> + pathsep = strrchr(execdir, '/');
> + if (!pathsep)
> + return NULL;
> +
> + *pathsep = '\0';
> + if (!streq(execdir, MESON_BUILD_ROOT))
> + return NULL;
> +
> + return safe_strdup(execdir);
> +}
> +
> static inline void
> setup_path(void)
> {
> const char *path = getenv("PATH");
> char new_path[PATH_MAX];
> + char *builddir;
> +
> + builddir = tools_execdir_is_builddir();
>
> snprintf(new_path,
>sizeof(new_path),
>"%s:%s",
> - LIBINPUT_TOOL_PATH,
> + builddir ? builddir : LIBINPUT_TOOL_PATH,
>path ? path : "");
> setenv("PATH", new_path, 1);
> + free(builddir);
> }
>
> int
> diff --git a/tools/shared.h b/tools/shared.h
> index e30bc39f..364babe6 100644
> --- a/tools/shared.h
> +++ b/tools/shared.h
> @@ -119,4 +119,7 @@ tools_list_device_quirks(struct quirks_context *ctx,
>void (*callback)(void *userdata, const char *str),
>void *userdata);
>
> +char *
> +tools_execdir_is_builddir(void);
> +
> #endif
pgpWrM02dWC_d.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 1/5] tools: if the execdir is the builddir, add it to the path
When running libinput tools from the builddir, look up the subtools in the
builddir as well. Otherwise, add the install prefix to the list of lookup
locations.
This ensures that a) we're running builddir stuff against builddir stuff, but
also b) that we're not running builddir stuff against installed stuff because
that may give us false positives.
Signed-off-by: Peter Hutterer
---
Similar-ish to the git approach Emil suggested in [1] but git is a lot more
involved here. Since we only need this for debugging tools, we can pick the
simpler approach here and simply ignore most of the corner cases.
[1] https://lists.freedesktop.org/archives/wayland-devel/2018-June/038658.html
meson.build| 1 +
tools/shared.c | 31 ++-
tools/shared.h | 3 +++
3 files changed, 34 insertions(+), 1 deletion(-)
diff --git a/meson.build b/meson.build
index 58e909b6..5580086f 100644
--- a/meson.build
+++ b/meson.build
@@ -31,6 +31,7 @@ add_project_arguments(cppflags, language : 'cpp')
config_h = configuration_data()
config_h.set('_GNU_SOURCE', '1')
+config_h.set_quoted('MESON_BUILD_ROOT', meson.build_root())
prefix = '''#define _GNU_SOURCE 1
#include
diff --git a/tools/shared.c b/tools/shared.c
index f2ed1fd0..776b3d3e 100644
--- a/tools/shared.c
+++ b/tools/shared.c
@@ -467,18 +467,47 @@ out:
return is_touchpad;
}
+/* Try to read the directory we're executing from and if it matches the
+ * builddir, return it as path. Otherwise, return NULL.
+ */
+char *
+tools_execdir_is_builddir(void)
+{
+ char execdir[PATH_MAX];
+ char *pathsep;
+ ssize_t sz;
+
+ sz = readlink("/proc/self/exe", execdir, sizeof(execdir));
+ if (sz <= 0 || sz == sizeof(execdir))
+ return NULL;
+
+ pathsep = strrchr(execdir, '/');
+ if (!pathsep)
+ return NULL;
+
+ *pathsep = '\0';
+ if (!streq(execdir, MESON_BUILD_ROOT))
+ return NULL;
+
+ return safe_strdup(execdir);
+}
+
static inline void
setup_path(void)
{
const char *path = getenv("PATH");
char new_path[PATH_MAX];
+ char *builddir;
+
+ builddir = tools_execdir_is_builddir();
snprintf(new_path,
sizeof(new_path),
"%s:%s",
-LIBINPUT_TOOL_PATH,
+builddir ? builddir : LIBINPUT_TOOL_PATH,
path ? path : "");
setenv("PATH", new_path, 1);
+ free(builddir);
}
int
diff --git a/tools/shared.h b/tools/shared.h
index e30bc39f..364babe6 100644
--- a/tools/shared.h
+++ b/tools/shared.h
@@ -119,4 +119,7 @@ tools_list_device_quirks(struct quirks_context *ctx,
void (*callback)(void *userdata, const char *str),
void *userdata);
+char *
+tools_execdir_is_builddir(void);
+
#endif
--
2.17.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
