| Message ID | 20180307193930.GW3701@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 2018-03-07 14:39, Arnaldo Carvalho de Melo wrote: > Em Wed, Mar 07, 2018 at 04:05:43PM -0300, Arnaldo Carvalho de Melo > escreveu: >> Em Wed, Mar 07, 2018 at 04:05:09PM -0300, Arnaldo Carvalho de Melo >> escreveu: >> > Em Wed, Mar 07, 2018 at 10:54:15AM -0800, Andi Kleen escreveu: >> > > > Sorry about that. That's probably because FNM_EXTMATCH is a GNU extension, >> > > > not POSIX, and the Alpine and Android runtimes likely don't implement >> > > > that... >> > > > I'll send a fix reverting back to the strncmp to ignore the uncore_ prefix, >> > > > and dropping that extension. >> > > >> > > Just don't set it? Even the basic glob patterns are useful. >> > >> > Or use: >> > >> > #ifndef FNM_EXTMATCH >> > #define FNM_EXTMATCH 0 >> > #endif >> > >> > So on systems without it, its not used, while on GNU systems, we have >> > that functionality (pretty fancy, someone may need that... ;-)) >> >> I'll try this route, btw, no need to send more patches for now. > > So, with the patch at the end of this message, it works now in those > systems: > > [root@jouet ~]# dm > 1 alpine:3.4 : Ok gcc (Alpine 5.3.0) 5.3.0 > 2 alpine:3.5 : Ok gcc (Alpine 6.2.1) 6.2.1 > 20160822 > 3 alpine:3.6 : Ok gcc (Alpine 6.3.0) 6.3.0 > 4 alpine:edge : Ok gcc (Alpine 6.4.0) 6.4.0 > 5 amazonlinux:1 : Ok gcc (GCC) 4.8.5 20150623 > (Red Hat 4.8.5-11) > 6 amazonlinux:2 : Ok gcc (GCC) 7.2.1 20170915 > (Red Hat 7.2.1-2) > 7 android-ndk:r12b-arm : Ok arm-linux-androideabi-gcc > (GCC) 4.9.x 20150123 (prerelease) > 8 android-ndk:r15c-arm : Ok arm-linux-androideabi-gcc > (GCC) 4.9.x 20150123 (prerelease) > 9 centos:5 : Ok gcc (GCC) 4.1.2 20080704 > (Red Hat 4.1.2-55) > 10 centos:6 : Ok gcc (GCC) 4.4.7 20120313 > (Red Hat 4.4.7-18) > > I stopped the test at this point to process some more patches, will > restart the tests with those extra patches and if all goes well with > the > other 46 build environments, push upstream, thanks. > > - Arnaldo > > diff --git a/tools/perf/util/parse-events.y > b/tools/perf/util/parse-events.y > index 316ac073aa78..18473be7d787 100644 > --- a/tools/perf/util/parse-events.y > +++ b/tools/perf/util/parse-events.y > @@ -9,6 +9,13 @@ > #define YYDEBUG 1 > > #include <fnmatch.h> > +/* > + * GNU extension, so better define it to 0 for systems such > + * as Android and Alpine Linux. > + */ > +#ifndef FNM_EXTMATCH > +#define FNM_EXTMATCH 0 > +#endif > #include <linux/compiler.h> > #include <linux/list.h> > #include <linux/types.h> Hey Jiri, The downside is that, while the compilation now works on those systems, the pattern will not work as intended in them :o( Let me cook something and send it ASAP. Thanks, Agustín
Em Wed, Mar 07, 2018 at 02:49:50PM -0500, Agustin Vega-Frias escreveu: > On 2018-03-07 14:39, Arnaldo Carvalho de Melo wrote: > > Em Wed, Mar 07, 2018 at 04:05:43PM -0300, Arnaldo Carvalho de Melo > > escreveu: > > > Em Wed, Mar 07, 2018 at 04:05:09PM -0300, Arnaldo Carvalho de Melo > > > escreveu: > > > > Em Wed, Mar 07, 2018 at 10:54:15AM -0800, Andi Kleen escreveu: > > > > > > Sorry about that. That's probably because FNM_EXTMATCH is a GNU extension, > > > > > > not POSIX, and the Alpine and Android runtimes likely don't implement > > > > > > that... > > > > > > I'll send a fix reverting back to the strncmp to ignore the uncore_ prefix, > > > > > > and dropping that extension. > > > > > > > > > > Just don't set it? Even the basic glob patterns are useful. > > > > > > > > Or use: > > > > > > > > #ifndef FNM_EXTMATCH > > > > #define FNM_EXTMATCH 0 > > > > #endif > > > > > > > > So on systems without it, its not used, while on GNU systems, we have > > > > that functionality (pretty fancy, someone may need that... ;-)) > > > > > > I'll try this route, btw, no need to send more patches for now. > > > > So, with the patch at the end of this message, it works now in those > > systems: > > > > [root@jouet ~]# dm > > 1 alpine:3.4 : Ok gcc (Alpine 5.3.0) 5.3.0 > > 2 alpine:3.5 : Ok gcc (Alpine 6.2.1) 6.2.1 > > 20160822 > > 3 alpine:3.6 : Ok gcc (Alpine 6.3.0) 6.3.0 > > 4 alpine:edge : Ok gcc (Alpine 6.4.0) 6.4.0 > > 5 amazonlinux:1 : Ok gcc (GCC) 4.8.5 20150623 > > (Red Hat 4.8.5-11) > > 6 amazonlinux:2 : Ok gcc (GCC) 7.2.1 20170915 > > (Red Hat 7.2.1-2) > > 7 android-ndk:r12b-arm : Ok arm-linux-androideabi-gcc > > (GCC) 4.9.x 20150123 (prerelease) > > 8 android-ndk:r15c-arm : Ok arm-linux-androideabi-gcc > > (GCC) 4.9.x 20150123 (prerelease) > > 9 centos:5 : Ok gcc (GCC) 4.1.2 20080704 > > (Red Hat 4.1.2-55) > > 10 centos:6 : Ok gcc (GCC) 4.4.7 20120313 > > (Red Hat 4.4.7-18) > > > > I stopped the test at this point to process some more patches, will > > restart the tests with those extra patches and if all goes well with the > > other 46 build environments, push upstream, thanks. > > > > - Arnaldo > > > > diff --git a/tools/perf/util/parse-events.y > > b/tools/perf/util/parse-events.y > > index 316ac073aa78..18473be7d787 100644 > > --- a/tools/perf/util/parse-events.y > > +++ b/tools/perf/util/parse-events.y > > @@ -9,6 +9,13 @@ > > #define YYDEBUG 1 > > > > #include <fnmatch.h> > > +/* > > + * GNU extension, so better define it to 0 for systems such > > + * as Android and Alpine Linux. > > + */ > > +#ifndef FNM_EXTMATCH > > +#define FNM_EXTMATCH 0 > > +#endif > > #include <linux/compiler.h> > > #include <linux/list.h> > > #include <linux/types.h> > > Hey Jiri, > > The downside is that, while the compilation now works on those > systems, the pattern will not work as intended in them :o( > Let me cook something and send it ASAP. Well, do you think this is really a big problem? Even if we add nice docs? I haven't tested something that works with this syntax on a capable system and then on one that doesn't, to see how it would behave, probably it would say something about a syntax error? - Arnaldo
On 2018-03-07 14:58, Arnaldo Carvalho de Melo wrote: > Em Wed, Mar 07, 2018 at 02:49:50PM -0500, Agustin Vega-Frias escreveu: >> On 2018-03-07 14:39, Arnaldo Carvalho de Melo wrote: >> > Em Wed, Mar 07, 2018 at 04:05:43PM -0300, Arnaldo Carvalho de Melo >> > escreveu: >> > > Em Wed, Mar 07, 2018 at 04:05:09PM -0300, Arnaldo Carvalho de Melo >> > > escreveu: >> > > > Em Wed, Mar 07, 2018 at 10:54:15AM -0800, Andi Kleen escreveu: >> > > > > > Sorry about that. That's probably because FNM_EXTMATCH is a GNU extension, >> > > > > > not POSIX, and the Alpine and Android runtimes likely don't implement >> > > > > > that... >> > > > > > I'll send a fix reverting back to the strncmp to ignore the uncore_ prefix, >> > > > > > and dropping that extension. >> > > > > >> > > > > Just don't set it? Even the basic glob patterns are useful. >> > > > >> > > > Or use: >> > > > >> > > > #ifndef FNM_EXTMATCH >> > > > #define FNM_EXTMATCH 0 >> > > > #endif >> > > > >> > > > So on systems without it, its not used, while on GNU systems, we have >> > > > that functionality (pretty fancy, someone may need that... ;-)) >> > > >> > > I'll try this route, btw, no need to send more patches for now. >> > >> > So, with the patch at the end of this message, it works now in those >> > systems: >> > >> > [root@jouet ~]# dm >> > 1 alpine:3.4 : Ok gcc (Alpine 5.3.0) 5.3.0 >> > 2 alpine:3.5 : Ok gcc (Alpine 6.2.1) 6.2.1 >> > 20160822 >> > 3 alpine:3.6 : Ok gcc (Alpine 6.3.0) 6.3.0 >> > 4 alpine:edge : Ok gcc (Alpine 6.4.0) 6.4.0 >> > 5 amazonlinux:1 : Ok gcc (GCC) 4.8.5 20150623 >> > (Red Hat 4.8.5-11) >> > 6 amazonlinux:2 : Ok gcc (GCC) 7.2.1 20170915 >> > (Red Hat 7.2.1-2) >> > 7 android-ndk:r12b-arm : Ok arm-linux-androideabi-gcc >> > (GCC) 4.9.x 20150123 (prerelease) >> > 8 android-ndk:r15c-arm : Ok arm-linux-androideabi-gcc >> > (GCC) 4.9.x 20150123 (prerelease) >> > 9 centos:5 : Ok gcc (GCC) 4.1.2 20080704 >> > (Red Hat 4.1.2-55) >> > 10 centos:6 : Ok gcc (GCC) 4.4.7 20120313 >> > (Red Hat 4.4.7-18) >> > >> > I stopped the test at this point to process some more patches, will >> > restart the tests with those extra patches and if all goes well with the >> > other 46 build environments, push upstream, thanks. >> > >> > - Arnaldo >> > >> > diff --git a/tools/perf/util/parse-events.y >> > b/tools/perf/util/parse-events.y >> > index 316ac073aa78..18473be7d787 100644 >> > --- a/tools/perf/util/parse-events.y >> > +++ b/tools/perf/util/parse-events.y >> > @@ -9,6 +9,13 @@ >> > #define YYDEBUG 1 >> > >> > #include <fnmatch.h> >> > +/* >> > + * GNU extension, so better define it to 0 for systems such >> > + * as Android and Alpine Linux. >> > + */ >> > +#ifndef FNM_EXTMATCH >> > +#define FNM_EXTMATCH 0 >> > +#endif >> > #include <linux/compiler.h> >> > #include <linux/list.h> >> > #include <linux/types.h> >> >> Hey Jiri, >> >> The downside is that, while the compilation now works on those >> systems, the pattern will not work as intended in them :o( >> Let me cook something and send it ASAP. > > Well, do you think this is really a big problem? Even if we add nice > docs? > > I haven't tested something that works with this syntax on a capable > system and then on one that doesn't, to see how it would behave, > probably it would say something about a syntax error? > Most likely, yes, it would be flagged as a syntax error in the event name. I'd prefer we fix this. I have the new patch ready, do you want me to just send you that patch instead of the series? Thanks, Agustín
On Wed, Mar 07, 2018 at 03:09:08PM -0500, Agustin Vega-Frias wrote: SNIP > > > > #include <fnmatch.h> > > > > +/* > > > > + * GNU extension, so better define it to 0 for systems such > > > > + * as Android and Alpine Linux. > > > > + */ > > > > +#ifndef FNM_EXTMATCH > > > > +#define FNM_EXTMATCH 0 > > > > +#endif > > > > #include <linux/compiler.h> > > > > #include <linux/list.h> > > > > #include <linux/types.h> > > > > > > Hey Jiri, > > > > > > The downside is that, while the compilation now works on those > > > systems, the pattern will not work as intended in them :o( > > > Let me cook something and send it ASAP. > > > > Well, do you think this is really a big problem? Even if we add nice > > docs? > > > > I haven't tested something that works with this syntax on a capable > > system and then on one that doesn't, to see how it would behave, > > probably it would say something about a syntax error? > > > > Most likely, yes, it would be flagged as a syntax error in the event name. > I'd prefer we fix this. I have the new patch ready, do you want me to just > send you that patch instead of the series? we could also detect the extension via build features and have HAVE_FEATURE_ macro defined for it, so we'd have full functionality on system with its support but maybe that's little too much for feature this size, or we can add it later.. now I'd be ok with the limited functionality as well jirka
Em Wed, Mar 07, 2018 at 03:09:08PM -0500, Agustin Vega-Frias escreveu: > On 2018-03-07 14:58, Arnaldo Carvalho de Melo wrote: > > Em Wed, Mar 07, 2018 at 02:49:50PM -0500, Agustin Vega-Frias escreveu: > > > On 2018-03-07 14:39, Arnaldo Carvalho de Melo wrote: > > > > Em Wed, Mar 07, 2018 at 04:05:43PM -0300, Arnaldo Carvalho de Melo > > > > escreveu: > > > > > Em Wed, Mar 07, 2018 at 04:05:09PM -0300, Arnaldo Carvalho de Melo > > > > > escreveu: > > > > > > Em Wed, Mar 07, 2018 at 10:54:15AM -0800, Andi Kleen escreveu: > > > > > > > > Sorry about that. That's probably because FNM_EXTMATCH is a GNU extension, > > > > > > > > not POSIX, and the Alpine and Android runtimes likely don't implement > > > > > > > > that... > > > > > > > > I'll send a fix reverting back to the strncmp to ignore the uncore_ prefix, > > > > > > > > and dropping that extension. > > > > > > > > > > > > > > Just don't set it? Even the basic glob patterns are useful. > > > > > > > > > > > > Or use: > > > > > > > > > > > > #ifndef FNM_EXTMATCH > > > > > > #define FNM_EXTMATCH 0 > > > > > > #endif > > > > > > > > > > > > So on systems without it, its not used, while on GNU systems, we have > > > > > > that functionality (pretty fancy, someone may need that... ;-)) > > > > > > > > > > I'll try this route, btw, no need to send more patches for now. > > > > > > > > So, with the patch at the end of this message, it works now in those > > > > systems: > > > > > > > > [root@jouet ~]# dm > > > > 1 alpine:3.4 : Ok gcc (Alpine 5.3.0) 5.3.0 > > > > 2 alpine:3.5 : Ok gcc (Alpine 6.2.1) 6.2.1 > > > > 20160822 > > > > 3 alpine:3.6 : Ok gcc (Alpine 6.3.0) 6.3.0 > > > > 4 alpine:edge : Ok gcc (Alpine 6.4.0) 6.4.0 > > > > 5 amazonlinux:1 : Ok gcc (GCC) 4.8.5 20150623 > > > > (Red Hat 4.8.5-11) > > > > 6 amazonlinux:2 : Ok gcc (GCC) 7.2.1 20170915 > > > > (Red Hat 7.2.1-2) > > > > 7 android-ndk:r12b-arm : Ok arm-linux-androideabi-gcc > > > > (GCC) 4.9.x 20150123 (prerelease) > > > > 8 android-ndk:r15c-arm : Ok arm-linux-androideabi-gcc > > > > (GCC) 4.9.x 20150123 (prerelease) > > > > 9 centos:5 : Ok gcc (GCC) 4.1.2 20080704 > > > > (Red Hat 4.1.2-55) > > > > 10 centos:6 : Ok gcc (GCC) 4.4.7 20120313 > > > > (Red Hat 4.4.7-18) > > > > > > > > I stopped the test at this point to process some more patches, will > > > > restart the tests with those extra patches and if all goes well with the > > > > other 46 build environments, push upstream, thanks. > > > > > > > > - Arnaldo > > > > > > > > diff --git a/tools/perf/util/parse-events.y > > > > b/tools/perf/util/parse-events.y > > > > index 316ac073aa78..18473be7d787 100644 > > > > --- a/tools/perf/util/parse-events.y > > > > +++ b/tools/perf/util/parse-events.y > > > > @@ -9,6 +9,13 @@ > > > > #define YYDEBUG 1 > > > > > > > > #include <fnmatch.h> > > > > +/* > > > > + * GNU extension, so better define it to 0 for systems such > > > > + * as Android and Alpine Linux. > > > > + */ > > > > +#ifndef FNM_EXTMATCH > > > > +#define FNM_EXTMATCH 0 > > > > +#endif > > > > #include <linux/compiler.h> > > > > #include <linux/list.h> > > > > #include <linux/types.h> > > > > > > Hey Jiri, > > > > > > The downside is that, while the compilation now works on those > > > systems, the pattern will not work as intended in them :o( > > > Let me cook something and send it ASAP. > > > > Well, do you think this is really a big problem? Even if we add nice > > docs? > > > > I haven't tested something that works with this syntax on a capable > > system and then on one that doesn't, to see how it would behave, > > probably it would say something about a syntax error? > > > > Most likely, yes, it would be flagged as a syntax error in the event name. > I'd prefer we fix this. I have the new patch ready, do you want me to just > send you that patch instead of the series? Just that patch is ok. - Arnaldo
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y index 316ac073aa78..18473be7d787 100644 --- a/tools/perf/util/parse-events.y +++ b/tools/perf/util/parse-events.y @@ -9,6 +9,13 @@ #define YYDEBUG 1 #include <fnmatch.h> +/* + * GNU extension, so better define it to 0 for systems such + * as Android and Alpine Linux. + */ +#ifndef FNM_EXTMATCH +#define FNM_EXTMATCH 0 +#endif #include <linux/compiler.h> #include <linux/list.h> #include <linux/types.h>