Re: [PATCH weston 5/6] libweston: do not add libweston-$version to the Cflags
On Fri, 8 Jul 2016 11:31:42 +0100 Emil Velikov wrote: > On 7 July 2016 at 19:18, Quentin Glidic > wrote: > > On 07/07/2016 18:28, Emil Velikov wrote: > >> > >> On 7 July 2016 at 10:20, Pekka Paalanen wrote: > >>> > >>> [snip] > >>> Therefore a NAK from me too. > >>> > >> As you guys wish. In that case can we drop the pkgincludedir variable > >> ? Most packages don't bother with it (on my local setup only 7 out of > >> 740 do) > > > > > > Is there a strong reason to remove it? Right now I would see it as a noise > > commit with no real value. > > > If 100 to 1 ratio isn't enough, I don't think anything is. Then again, > I'm wondering if the topic hasn't turned into a perfect bikeshedding > example ? Hi Emil, if you really care about that one variable so much to write a patch for it, I shall merge it. I believe nothing should be asking the .pc about a pkgincludedir specifically, right? Thanks, pq pgpmghQTj5wLv.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 5/6] libweston: do not add libweston-$version to the Cflags
On 7 July 2016 at 19:18, Quentin Glidic wrote: > On 07/07/2016 18:28, Emil Velikov wrote: >> >> On 7 July 2016 at 10:20, Pekka Paalanen wrote: >>> >>> [snip] >>> Therefore a NAK from me too. >>> >> As you guys wish. In that case can we drop the pkgincludedir variable >> ? Most packages don't bother with it (on my local setup only 7 out of >> 740 do) > > > Is there a strong reason to remove it? Right now I would see it as a noise > commit with no real value. > If 100 to 1 ratio isn't enough, I don't think anything is. Then again, I'm wondering if the topic hasn't turned into a perfect bikeshedding example ? -Emil ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 5/6] libweston: do not add libweston-$version to the Cflags
On 07/07/2016 18:28, Emil Velikov wrote: On 7 July 2016 at 10:20, Pekka Paalanen wrote: [snip] Therefore a NAK from me too. As you guys wish. In that case can we drop the pkgincludedir variable ? Most packages don't bother with it (on my local setup only 7 out of 740 do) Is there a strong reason to remove it? Right now I would see it as a noise commit with no real value. Cheers, -- Quentin “Sardem FF7” Glidic ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 5/6] libweston: do not add libweston-$version to the Cflags
On 7 July 2016 at 10:20, Pekka Paalanen wrote: > On Mon, 4 Jul 2016 16:02:23 +0100 > Emil Velikov wrote: > >> On 4 July 2016 at 15:32, Quentin Glidic >> wrote: >> > On 04/07/2016 16:23, Emil Velikov wrote: >> >> >> >> From: Emil Velikov >> >> >> >> When managing headers there's normally two ways to handle them >> >> - with or without the subfolder. >> >> >> >> Opting for the latter case here, since it will provide direct feedback, >> >> whether one is using libweston-0 or any other version. >> >> >> >> Which in turn should deter (help prevent) issues like building/linking >> >> against multiple versions of libweston. >> >> >> >> Signed-off-by: Emil Velikov >> > >> > >> > I really prefer not to do that. It means supporting multiple versions of >> > libweston will lead to a really big #ifdef dance at the top of the file to >> > include every single version you might support, instead of a just a few >> > #ifdef around specific new/old functions you use. >> > >> Yes, I agree with you - adding ifdef spaghetti is ugly. Then again, if >> one wants to support multiple versions they will need a bunch of them >> either way. >> >> Keeping things explicit will give (and/or save) you and others a fair >> bit of time when it goes wrong/nasty. Here is an (somewhat silly, or >> you might say sad) example I've seen in the open-source Tizen world... >> or was it Android: >> >> Dev. lacks exact knowledge when function A (libfoo1) and B (libfoo2) >> are introduced. Thus he/she adds both versions of libfoo in the >> configure pkg-config checks. At which point, you will have symbol >> collisions, seemingly random corruption and/or crashes. > > Hi Emil, > > I'm not sure I get that example. > > I read the long IRC discussion between the two of you the other > day, and I have to say I'm on Quentin's side on this, solely because > switching from one libweston version to the next will cause lots of > "unnecessary" +1 for well use of quotation marks :-) > changes if you need to fix the #include directives > everywhere. I also do not think doing that mechanical excercise > will make anyone think about the actual ABI changes. > "Anyone" is an overstatement. I might be a minority, but I would make the effort of checking the API's (I use) in such cases. The whole example and logic steers on the idea that the greater the effort required by the user the larger likelihood that they will stop and think for a second "wait there might be a reason behind all this annoyance". As said, I'm likely a minority here. > If we want to make sure users actually notice libweston ABI > changes, we need to make those as build-breaking API changes too. > That's the only feasible thing I can think of, unless we had someone > writing a checklist of all the ABI changes - a fat chance - and > hoping that downstream devs actually go through it. > Yes, it's impossible to make thing completely bullet proof or even get some (most?) users to read through the changelog. That is unless things blow up in their face :-) > Therefore a NAK from me too. > As you guys wish. In that case can we drop the pkgincludedir variable ? Most packages don't bother with it (on my local setup only 7 out of 740 do) Thanks Emil ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 5/6] libweston: do not add libweston-$version to the Cflags
On Mon, 4 Jul 2016 16:02:23 +0100 Emil Velikov wrote: > On 4 July 2016 at 15:32, Quentin Glidic > wrote: > > On 04/07/2016 16:23, Emil Velikov wrote: > >> > >> From: Emil Velikov > >> > >> When managing headers there's normally two ways to handle them > >> - with or without the subfolder. > >> > >> Opting for the latter case here, since it will provide direct feedback, > >> whether one is using libweston-0 or any other version. > >> > >> Which in turn should deter (help prevent) issues like building/linking > >> against multiple versions of libweston. > >> > >> Signed-off-by: Emil Velikov > > > > > > I really prefer not to do that. It means supporting multiple versions of > > libweston will lead to a really big #ifdef dance at the top of the file to > > include every single version you might support, instead of a just a few > > #ifdef around specific new/old functions you use. > > > Yes, I agree with you - adding ifdef spaghetti is ugly. Then again, if > one wants to support multiple versions they will need a bunch of them > either way. > > Keeping things explicit will give (and/or save) you and others a fair > bit of time when it goes wrong/nasty. Here is an (somewhat silly, or > you might say sad) example I've seen in the open-source Tizen world... > or was it Android: > > Dev. lacks exact knowledge when function A (libfoo1) and B (libfoo2) > are introduced. Thus he/she adds both versions of libfoo in the > configure pkg-config checks. At which point, you will have symbol > collisions, seemingly random corruption and/or crashes. Hi Emil, I'm not sure I get that example. I read the long IRC discussion between the two of you the other day, and I have to say I'm on Quentin's side on this, solely because switching from one libweston version to the next will cause lots of "unnecessary" changes if you need to fix the #include directives everywhere. I also do not think doing that mechanical excercise will make anyone think about the actual ABI changes. If we want to make sure users actually notice libweston ABI changes, we need to make those as build-breaking API changes too. That's the only feasible thing I can think of, unless we had someone writing a checklist of all the ABI changes - a fat chance - and hoping that downstream devs actually go through it. Therefore a NAK from me too. Thanks, pq pgp8YiXFOYxyM.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 5/6] libweston: do not add libweston-$version to the Cflags
On 4 July 2016 at 15:32, Quentin Glidic wrote: > On 04/07/2016 16:23, Emil Velikov wrote: >> >> From: Emil Velikov >> >> When managing headers there's normally two ways to handle them >> - with or without the subfolder. >> >> Opting for the latter case here, since it will provide direct feedback, >> whether one is using libweston-0 or any other version. >> >> Which in turn should deter (help prevent) issues like building/linking >> against multiple versions of libweston. >> >> Signed-off-by: Emil Velikov > > > I really prefer not to do that. It means supporting multiple versions of > libweston will lead to a really big #ifdef dance at the top of the file to > include every single version you might support, instead of a just a few > #ifdef around specific new/old functions you use. > Yes, I agree with you - adding ifdef spaghetti is ugly. Then again, if one wants to support multiple versions they will need a bunch of them either way. Keeping things explicit will give (and/or save) you and others a fair bit of time when it goes wrong/nasty. Here is an (somewhat silly, or you might say sad) example I've seen in the open-source Tizen world... or was it Android: Dev. lacks exact knowledge when function A (libfoo1) and B (libfoo2) are introduced. Thus he/she adds both versions of libfoo in the configure pkg-config checks. At which point, you will have symbol collisions, seemingly random corruption and/or crashes. Cheers, Emil ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 5/6] libweston: do not add libweston-$version to the Cflags
On 04/07/2016 16:23, Emil Velikov wrote: From: Emil Velikov When managing headers there's normally two ways to handle them - with or without the subfolder. Opting for the latter case here, since it will provide direct feedback, whether one is using libweston-0 or any other version. Which in turn should deter (help prevent) issues like building/linking against multiple versions of libweston. Signed-off-by: Emil Velikov I really prefer not to do that. It means supporting multiple versions of libweston will lead to a really big #ifdef dance at the top of the file to include every single version you might support, instead of a just a few #ifdef around specific new/old functions you use. NAK for me. Cheers, --- libweston/libweston.pc.in | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/libweston/libweston.pc.in b/libweston/libweston.pc.in index 24fe813..bb95af9 100644 --- a/libweston/libweston.pc.in +++ b/libweston/libweston.pc.in @@ -2,11 +2,10 @@ prefix=@prefix@ exec_prefix=@exec_prefix@ libdir=@libdir@ includedir=@includedir@ -pkgincludedir=${includedir}/libweston-@LIBWESTON_ABI_VERSION@ Name: libweston API Description: Header files for libweston compositors development Version: @WESTON_VERSION@ Requires.private: wayland-server pixman-1 xkbcommon -Cflags: -I${pkgincludedir} +Cflags: -I${includedir} Libs: -L${libdir} -lweston-@LIBWESTON_ABI_VERSION@ -- Quentin “Sardem FF7” Glidic ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston 5/6] libweston: do not add libweston-$version to the Cflags
From: Emil Velikov When managing headers there's normally two ways to handle them - with or without the subfolder. Opting for the latter case here, since it will provide direct feedback, whether one is using libweston-0 or any other version. Which in turn should deter (help prevent) issues like building/linking against multiple versions of libweston. Signed-off-by: Emil Velikov --- libweston/libweston.pc.in | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/libweston/libweston.pc.in b/libweston/libweston.pc.in index 24fe813..bb95af9 100644 --- a/libweston/libweston.pc.in +++ b/libweston/libweston.pc.in @@ -2,11 +2,10 @@ prefix=@prefix@ exec_prefix=@exec_prefix@ libdir=@libdir@ includedir=@includedir@ -pkgincludedir=${includedir}/libweston-@LIBWESTON_ABI_VERSION@ Name: libweston API Description: Header files for libweston compositors development Version: @WESTON_VERSION@ Requires.private: wayland-server pixman-1 xkbcommon -Cflags: -I${pkgincludedir} +Cflags: -I${includedir} Libs: -L${libdir} -lweston-@LIBWESTON_ABI_VERSION@ -- 2.8.2 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel