Re: [PATCH weston] build: Define wayland prereq version
On 20 May 2016 at 21:35, Bryce Harrington wrote: > On Thu, May 12, 2016 at 09:48:17AM +0100, Emil Velikov wrote: >> On 12 May 2016 at 09:13, Pekka Paalanen wrote: >> > On Thu, 12 May 2016 11:12:28 +1000 >> > Peter Hutterer wrote: >> > >> >> On Wed, May 11, 2016 at 01:18:59PM -0700, Bryce Harrington wrote: >> >> > Establishes a single variable for defining the libwayland version >> >> > requirements. Enforces the same version dependency between >> >> > libwayland-client and libwayland-server, as recommended by pq in the >> >> > 1.11 release discussions. >> >> > >> >> > Signed-off-by: Bryce Harrington >> >> > --- >> >> > configure.ac | 12 +++- >> >> > 1 file changed, 7 insertions(+), 5 deletions(-) >> >> > >> >> > diff --git a/configure.ac b/configure.ac >> >> > index 2ca1f4e..0b23fc4 100644 >> >> > --- a/configure.ac >> >> > +++ b/configure.ac >> >> > @@ -4,6 +4,8 @@ m4_define([weston_micro_version], [91]) >> >> > m4_define([weston_version], >> >> > >> >> > [weston_major_version.weston_minor_version.weston_micro_version]) >> >> > >> >> > +m4_define([WAYLAND_PREREQ_VERSION], "1.10.0") >> >> >> >> how comes the line above uses [] and here you use ""? is that intentional? >> >> (I keep forgetting whether there's a difference between the two in m4) >> > >> > Yeah, I'm not that sure about using a m4 define. It is one way to do >> > it, but the quotes do look suspicious. >> > >> > FWIW, Mesa uses a big list of common dependency variables too, maybe >> > copy that approach? >> > >> > CC'ing Quentin and Emil, they probably know what's good. >> > >> In all honesty I don't know which one is better, so any >> info/references will be appreciated. For the time being (personally) >> I'd stick with the following as it reads a bit easier ;-) >> >> WAYLAND_PREREQ_VERSION="1.10.0" > > Alright, so I've tested several different variations. I've tested both > using version 1.10.0 (which must pass), and 1.99.0 (which must fail) > >N=10 N=99 > m4_define([WAYLAND_PREREQ_VERSION], "1.N.0") PASS FAIL --> Okay > m4_define([WAYLAND_PREREQ_VERSION], [1.N.0]) PASS FAIL --> Okay > m4_define([WAYLAND_PREREQ_VERSION], 1.N.0) PASS FAIL --> Okay > > WAYLAND_PREREQ_VERSION="1.N.0" PASS PASS --> Incorrect > WAYLAND_PREREQ_VERSION=1.N.0 PASS PASS --> Incorrect > WAYLAND_PREREQ_VERSION=[1.N.0] PASS PASS --> Incorrect > > In all cases, I've referenced the variable as just > WAYLAND_PREREQ_VERSION in the code. If I reference it as > $WAYLAND_PREREQ_VERSION then autogen.sh errors indicating a blank string > was substituted. E.g.: > > configure: error: Package requirements (wayland-server >= pixman-1 >= > 0.25.2 xkbcommon >= 0.3.0) were not met: > No package '>=' found > No package '0.25.2' found > Hmm that doesn't sounds right. The following simplified example works like a charm - change FOO_REQ (or BAR version) to 0.20 and observe the result (expected failure). And yes, the approach does work in for more complex configure.ac - just tested xserver, mesa and libdrm. $ cat Makefile.am $ cat configure.ac AC_PREREQ([2.63]) AC_INIT([libfoo], [0.0.1]) AM_INIT_AUTOMAKE([1.10 foreign]) LT_PREREQ([2.2]) LT_INIT([disable-static]) FOO_REQ=0.10 PKG_CHECK_MODULES(PCIACCESS, [pciaccess >= $FOO_REQ]) BAR="pciaccess >= 0.20" dnl You can put the square brackets around $BAR. Personally I'm not a fan PKG_CHECK_MODULES(PCIACCESS, $BAR) AC_CONFIG_FILES([Makefile]) > I'd tested a number of other variations prior to settling on the > m4_define() syntax, which is why I'm leaning that direction - I just > couldn't get anything else to work. So if anyone feels m4_define() to > be the wrong way to do it, I'm happy to try another way but will need > more specific direction. > Do you have the patch somewhere ? Without it no one can tell you what's going wrong. I'm leaning that there's a trivial mistake in it somewhere. Or perhaps there's something fishy in the existing configure.ac ? -Emil ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] build: Define wayland prereq version
On Thu, May 12, 2016 at 09:48:17AM +0100, Emil Velikov wrote: > On 12 May 2016 at 09:13, Pekka Paalanen wrote: > > On Thu, 12 May 2016 11:12:28 +1000 > > Peter Hutterer wrote: > > > >> On Wed, May 11, 2016 at 01:18:59PM -0700, Bryce Harrington wrote: > >> > Establishes a single variable for defining the libwayland version > >> > requirements. Enforces the same version dependency between > >> > libwayland-client and libwayland-server, as recommended by pq in the > >> > 1.11 release discussions. > >> > > >> > Signed-off-by: Bryce Harrington > >> > --- > >> > configure.ac | 12 +++- > >> > 1 file changed, 7 insertions(+), 5 deletions(-) > >> > > >> > diff --git a/configure.ac b/configure.ac > >> > index 2ca1f4e..0b23fc4 100644 > >> > --- a/configure.ac > >> > +++ b/configure.ac > >> > @@ -4,6 +4,8 @@ m4_define([weston_micro_version], [91]) > >> > m4_define([weston_version], > >> > > >> > [weston_major_version.weston_minor_version.weston_micro_version]) > >> > > >> > +m4_define([WAYLAND_PREREQ_VERSION], "1.10.0") > >> > >> how comes the line above uses [] and here you use ""? is that intentional? > >> (I keep forgetting whether there's a difference between the two in m4) > > > > Yeah, I'm not that sure about using a m4 define. It is one way to do > > it, but the quotes do look suspicious. > > > > FWIW, Mesa uses a big list of common dependency variables too, maybe > > copy that approach? > > > > CC'ing Quentin and Emil, they probably know what's good. > > > In all honesty I don't know which one is better, so any > info/references will be appreciated. For the time being (personally) > I'd stick with the following as it reads a bit easier ;-) > > WAYLAND_PREREQ_VERSION="1.10.0" Alright, so I've tested several different variations. I've tested both using version 1.10.0 (which must pass), and 1.99.0 (which must fail) N=10 N=99 m4_define([WAYLAND_PREREQ_VERSION], "1.N.0") PASS FAIL --> Okay m4_define([WAYLAND_PREREQ_VERSION], [1.N.0]) PASS FAIL --> Okay m4_define([WAYLAND_PREREQ_VERSION], 1.N.0) PASS FAIL --> Okay WAYLAND_PREREQ_VERSION="1.N.0" PASS PASS --> Incorrect WAYLAND_PREREQ_VERSION=1.N.0 PASS PASS --> Incorrect WAYLAND_PREREQ_VERSION=[1.N.0] PASS PASS --> Incorrect In all cases, I've referenced the variable as just WAYLAND_PREREQ_VERSION in the code. If I reference it as $WAYLAND_PREREQ_VERSION then autogen.sh errors indicating a blank string was substituted. E.g.: configure: error: Package requirements (wayland-server >= pixman-1 >= 0.25.2 xkbcommon >= 0.3.0) were not met: No package '>=' found No package '0.25.2' found I'd tested a number of other variations prior to settling on the m4_define() syntax, which is why I'm leaning that direction - I just couldn't get anything else to work. So if anyone feels m4_define() to be the wrong way to do it, I'm happy to try another way but will need more specific direction. Regarding the quoting, it doesn't appear to matter what form to use. I'll go ahead and resubmit the patch with the bracketed form since that looks like it would be more consistent with the rest of the code, and sounds like it would be more standard. Bryce ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] build: Define wayland prereq version
On Thu, 12 May 2016 11:05:08 +0200 Quentin Glidic wrote: > On 12/05/2016 10:48, Emil Velikov wrote: > > On 12 May 2016 at 09:13, Pekka Paalanen wrote: > >> On Thu, 12 May 2016 11:12:28 +1000 > >> Peter Hutterer wrote: > >> > >>> On Wed, May 11, 2016 at 01:18:59PM -0700, Bryce Harrington wrote: > Establishes a single variable for defining the libwayland version > requirements. Enforces the same version dependency between > libwayland-client and libwayland-server, as recommended by pq in the > 1.11 release discussions. > > Signed-off-by: Bryce Harrington > --- > configure.ac | 12 +++- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/configure.ac b/configure.ac > index 2ca1f4e..0b23fc4 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -4,6 +4,8 @@ m4_define([weston_micro_version], [91]) > m4_define([weston_version], > > [weston_major_version.weston_minor_version.weston_micro_version]) > > +m4_define([WAYLAND_PREREQ_VERSION], "1.10.0") > >>> > >>> how comes the line above uses [] and here you use ""? is that intentional? > >>> (I keep forgetting whether there's a difference between the two in m4) > >> > >> Yeah, I'm not that sure about using a m4 define. It is one way to do > >> it, but the quotes do look suspicious. > >> > >> FWIW, Mesa uses a big list of common dependency variables too, maybe > >> copy that approach? > >> > >> CC'ing Quentin and Emil, they probably know what's good. > >> > > In all honesty I don't know which one is better, so any > > info/references will be appreciated. For the time being (personally) > > I'd stick with the following as it reads a bit easier ;-) > > > > WAYLAND_PREREQ_VERSION="1.10.0" > > This form ↑ (shell variable) is the most common one I know. > > AFAICT, Autoconf is not using pure m4 quotation, so the " are part of > the macro definition, thus the pkg-config call will be: > pkg-config --cflags wayland-server >= "3" > It works because pkg-config handles the extra quotes. > > > Unrelated to quoting: shouldn’t we keep the client/server split? In the > current state, we require an higher server version, so it doesn’t > matter, but if clients rely on a newer feature, does it make sense to > force that same server version even though we can disable clients? Who actually builds and tests Weston without either libwayland-client or -server? I never do, and I doubt there are many (any?) people that do and would also benefit from keeping the reqs separate. However, if we do keep the reqs separate while no-one tests them separately, one of them is going to be broken. So IMO merging the reqs into just one version is much better: just one version to bump as needed, less hidden failure modes. This is the whole point of this patch. Less surprises, less combinations for testing. Thanks, pq pgpocQ_FyhKSv.pgp Description: OpenPGP digital signature ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] build: Define wayland prereq version
On 12/05/2016 10:48, Emil Velikov wrote: On 12 May 2016 at 09:13, Pekka Paalanen wrote: On Thu, 12 May 2016 11:12:28 +1000 Peter Hutterer wrote: On Wed, May 11, 2016 at 01:18:59PM -0700, Bryce Harrington wrote: Establishes a single variable for defining the libwayland version requirements. Enforces the same version dependency between libwayland-client and libwayland-server, as recommended by pq in the 1.11 release discussions. Signed-off-by: Bryce Harrington --- configure.ac | 12 +++- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/configure.ac b/configure.ac index 2ca1f4e..0b23fc4 100644 --- a/configure.ac +++ b/configure.ac @@ -4,6 +4,8 @@ m4_define([weston_micro_version], [91]) m4_define([weston_version], [weston_major_version.weston_minor_version.weston_micro_version]) +m4_define([WAYLAND_PREREQ_VERSION], "1.10.0") how comes the line above uses [] and here you use ""? is that intentional? (I keep forgetting whether there's a difference between the two in m4) Yeah, I'm not that sure about using a m4 define. It is one way to do it, but the quotes do look suspicious. FWIW, Mesa uses a big list of common dependency variables too, maybe copy that approach? CC'ing Quentin and Emil, they probably know what's good. In all honesty I don't know which one is better, so any info/references will be appreciated. For the time being (personally) I'd stick with the following as it reads a bit easier ;-) WAYLAND_PREREQ_VERSION="1.10.0" This form ↑ (shell variable) is the most common one I know. AFAICT, Autoconf is not using pure m4 quotation, so the " are part of the macro definition, thus the pkg-config call will be: pkg-config --cflags wayland-server >= "3" It works because pkg-config handles the extra quotes. Unrelated to quoting: shouldn’t we keep the client/server split? In the current state, we require an higher server version, so it doesn’t matter, but if clients rely on a newer feature, does it make sense to force that same server version even though we can disable clients? -- Quentin “Sardem FF7” Glidic ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] build: Define wayland prereq version
On 12 May 2016 at 09:13, Pekka Paalanen wrote: > On Thu, 12 May 2016 11:12:28 +1000 > Peter Hutterer wrote: > >> On Wed, May 11, 2016 at 01:18:59PM -0700, Bryce Harrington wrote: >> > Establishes a single variable for defining the libwayland version >> > requirements. Enforces the same version dependency between >> > libwayland-client and libwayland-server, as recommended by pq in the >> > 1.11 release discussions. >> > >> > Signed-off-by: Bryce Harrington >> > --- >> > configure.ac | 12 +++- >> > 1 file changed, 7 insertions(+), 5 deletions(-) >> > >> > diff --git a/configure.ac b/configure.ac >> > index 2ca1f4e..0b23fc4 100644 >> > --- a/configure.ac >> > +++ b/configure.ac >> > @@ -4,6 +4,8 @@ m4_define([weston_micro_version], [91]) >> > m4_define([weston_version], >> > >> > [weston_major_version.weston_minor_version.weston_micro_version]) >> > >> > +m4_define([WAYLAND_PREREQ_VERSION], "1.10.0") >> >> how comes the line above uses [] and here you use ""? is that intentional? >> (I keep forgetting whether there's a difference between the two in m4) > > Yeah, I'm not that sure about using a m4 define. It is one way to do > it, but the quotes do look suspicious. > > FWIW, Mesa uses a big list of common dependency variables too, maybe > copy that approach? > > CC'ing Quentin and Emil, they probably know what's good. > In all honesty I don't know which one is better, so any info/references will be appreciated. For the time being (personally) I'd stick with the following as it reads a bit easier ;-) WAYLAND_PREREQ_VERSION="1.10.0" Thanks Emil ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] build: Define wayland prereq version
On Thu, 12 May 2016 11:12:28 +1000 Peter Hutterer wrote: > On Wed, May 11, 2016 at 01:18:59PM -0700, Bryce Harrington wrote: > > Establishes a single variable for defining the libwayland version > > requirements. Enforces the same version dependency between > > libwayland-client and libwayland-server, as recommended by pq in the > > 1.11 release discussions. > > > > Signed-off-by: Bryce Harrington > > --- > > configure.ac | 12 +++- > > 1 file changed, 7 insertions(+), 5 deletions(-) > > > > diff --git a/configure.ac b/configure.ac > > index 2ca1f4e..0b23fc4 100644 > > --- a/configure.ac > > +++ b/configure.ac > > @@ -4,6 +4,8 @@ m4_define([weston_micro_version], [91]) > > m4_define([weston_version], > >[weston_major_version.weston_minor_version.weston_micro_version]) > > > > +m4_define([WAYLAND_PREREQ_VERSION], "1.10.0") > > how comes the line above uses [] and here you use ""? is that intentional? > (I keep forgetting whether there's a difference between the two in m4) Yeah, I'm not that sure about using a m4 define. It is one way to do it, but the quotes do look suspicious. FWIW, Mesa uses a big list of common dependency variables too, maybe copy that approach? CC'ing Quentin and Emil, they probably know what's good. Thanks, pq > > + > > AC_PREREQ([2.64]) > > AC_INIT([weston], > > [weston_version], > > @@ -60,7 +62,7 @@ AC_CHECK_HEADERS([execinfo.h]) > > > > AC_CHECK_FUNCS([mkostemp strchrnul initgroups posix_fallocate]) > > > > -COMPOSITOR_MODULES="wayland-server >= 1.10.0 pixman-1 >= 0.25.2" > > +COMPOSITOR_MODULES="wayland-server >= WAYLAND_PREREQ_VERSION pixman-1 >= > > 0.25.2" > > > > AC_CONFIG_FILES([doc/doxygen/tools.doxygen doc/doxygen/tooldev.doxygen]) > > > > @@ -193,7 +195,7 @@ AM_CONDITIONAL(ENABLE_WAYLAND_COMPOSITOR, > > if test x$enable_wayland_compositor = xyes -a x$enable_egl = xyes; then > >AC_DEFINE([BUILD_WAYLAND_COMPOSITOR], [1], > > [Build the Wayland (nested) compositor]) > > - PKG_CHECK_MODULES(WAYLAND_COMPOSITOR, [wayland-client >= 1.5.91 > > wayland-egl wayland-cursor]) > > + PKG_CHECK_MODULES(WAYLAND_COMPOSITOR, [wayland-client >= > > WAYLAND_PREREQ_VERSION wayland-egl wayland-cursor]) > > fi > > > > > > @@ -332,7 +334,7 @@ AM_CONDITIONAL(ENABLE_VAAPI_RECORDER, test > > "x$have_libva" = xyes) > > > > PKG_CHECK_MODULES(CAIRO, [cairo]) > > > > -PKG_CHECK_MODULES(TEST_CLIENT, [wayland-client >= 1.10.0]) > > +PKG_CHECK_MODULES(TEST_CLIENT, [wayland-client >= WAYLAND_PREREQ_VERSION]) > > > > AC_ARG_ENABLE(simple-clients, > >AS_HELP_STRING([--disable-simple-clients], > > @@ -386,9 +388,9 @@ AM_CONDITIONAL(BUILD_CLIENTS, test x$enable_clients = > > xyes) > > if test x$enable_clients = xyes; then > >AC_DEFINE([BUILD_CLIENTS], [1], [Build the Wayland clients]) > > > > - PKG_CHECK_MODULES(CLIENT, [wayland-client >= 1.5.91 cairo >= 1.10.0 > > xkbcommon wayland-cursor]) > > + PKG_CHECK_MODULES(CLIENT, [wayland-client >= WAYLAND_PREREQ_VERSION > > cairo >= 1.10.0 xkbcommon wayland-cursor]) > >PKG_CHECK_MODULES(SERVER, [wayland-server]) > > - PKG_CHECK_MODULES(WESTON_INFO, [wayland-client >= 1.5.91]) > > + PKG_CHECK_MODULES(WESTON_INFO, [wayland-client >= > > WAYLAND_PREREQ_VERSION]) > > > ># Only check for cairo-egl if a GL or GLES renderer requested > >AS_IF([test "x$cairo_modules" = "xcairo-gl" -o "x$cairo_modules" = > > "xcairo-glesv2"], [ > > -- > > 1.9.1 > > pgpOfPgqk1rXB.pgp Description: OpenPGP digital signature ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] build: Define wayland prereq version
On Wed, May 11, 2016 at 01:18:59PM -0700, Bryce Harrington wrote: > Establishes a single variable for defining the libwayland version > requirements. Enforces the same version dependency between > libwayland-client and libwayland-server, as recommended by pq in the > 1.11 release discussions. > > Signed-off-by: Bryce Harrington > --- > configure.ac | 12 +++- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/configure.ac b/configure.ac > index 2ca1f4e..0b23fc4 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -4,6 +4,8 @@ m4_define([weston_micro_version], [91]) > m4_define([weston_version], >[weston_major_version.weston_minor_version.weston_micro_version]) > > +m4_define([WAYLAND_PREREQ_VERSION], "1.10.0") how comes the line above uses [] and here you use ""? is that intentional? (I keep forgetting whether there's a difference between the two in m4) Cheers, Peter > + > AC_PREREQ([2.64]) > AC_INIT([weston], > [weston_version], > @@ -60,7 +62,7 @@ AC_CHECK_HEADERS([execinfo.h]) > > AC_CHECK_FUNCS([mkostemp strchrnul initgroups posix_fallocate]) > > -COMPOSITOR_MODULES="wayland-server >= 1.10.0 pixman-1 >= 0.25.2" > +COMPOSITOR_MODULES="wayland-server >= WAYLAND_PREREQ_VERSION pixman-1 >= > 0.25.2" > > AC_CONFIG_FILES([doc/doxygen/tools.doxygen doc/doxygen/tooldev.doxygen]) > > @@ -193,7 +195,7 @@ AM_CONDITIONAL(ENABLE_WAYLAND_COMPOSITOR, > if test x$enable_wayland_compositor = xyes -a x$enable_egl = xyes; then >AC_DEFINE([BUILD_WAYLAND_COMPOSITOR], [1], > [Build the Wayland (nested) compositor]) > - PKG_CHECK_MODULES(WAYLAND_COMPOSITOR, [wayland-client >= 1.5.91 > wayland-egl wayland-cursor]) > + PKG_CHECK_MODULES(WAYLAND_COMPOSITOR, [wayland-client >= > WAYLAND_PREREQ_VERSION wayland-egl wayland-cursor]) > fi > > > @@ -332,7 +334,7 @@ AM_CONDITIONAL(ENABLE_VAAPI_RECORDER, test "x$have_libva" > = xyes) > > PKG_CHECK_MODULES(CAIRO, [cairo]) > > -PKG_CHECK_MODULES(TEST_CLIENT, [wayland-client >= 1.10.0]) > +PKG_CHECK_MODULES(TEST_CLIENT, [wayland-client >= WAYLAND_PREREQ_VERSION]) > > AC_ARG_ENABLE(simple-clients, >AS_HELP_STRING([--disable-simple-clients], > @@ -386,9 +388,9 @@ AM_CONDITIONAL(BUILD_CLIENTS, test x$enable_clients = > xyes) > if test x$enable_clients = xyes; then >AC_DEFINE([BUILD_CLIENTS], [1], [Build the Wayland clients]) > > - PKG_CHECK_MODULES(CLIENT, [wayland-client >= 1.5.91 cairo >= 1.10.0 > xkbcommon wayland-cursor]) > + PKG_CHECK_MODULES(CLIENT, [wayland-client >= WAYLAND_PREREQ_VERSION cairo > >= 1.10.0 xkbcommon wayland-cursor]) >PKG_CHECK_MODULES(SERVER, [wayland-server]) > - PKG_CHECK_MODULES(WESTON_INFO, [wayland-client >= 1.5.91]) > + PKG_CHECK_MODULES(WESTON_INFO, [wayland-client >= WAYLAND_PREREQ_VERSION]) > ># Only check for cairo-egl if a GL or GLES renderer requested >AS_IF([test "x$cairo_modules" = "xcairo-gl" -o "x$cairo_modules" = > "xcairo-glesv2"], [ > -- > 1.9.1 > > ___ > wayland-devel mailing list > wayland-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/wayland-devel > ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
