Re: [PATCH weston 5/6] libweston: do not add libweston-$version to the Cflags

[Pekka Paalanen]

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

[Emil Velikov]

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

[Quentin Glidic]

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

[Emil Velikov]

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

[Pekka Paalanen]

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

[Emil Velikov]

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

[Quentin Glidic]

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