MaxThemeName and MaxSkinName limit

Message ID 514DD4F4.5090907@users.sourceforge.net
State New
Headers

Commit Message

Lucian Muresan March 23, 2013, 4:14 p.m. UTC
  Hi,

thank you Klaus for holding up your release plan for 2.0!
However, I hope this minor patch won't be too much trouble for you, as
it only increases the limit of 16 to theme and skin names, letting them
be NAME_MAX as almost any files, since these names can also be involved
in file names.
Was there a good reason to limit them at 16 only, perhaps the fear that
the name won't fit on the OSD? I don't really think someone would really
make use of as many as 255 characters for this. On the other hand, users
just encountered crashes with plugins which (unfortunately, yet) have
themes of their own, when just adding a new theme with a name longer
than 16 and at the same time the original plugin author relied on
MaxThemeName when allocating the string length.
So, what do you think, easy to adopt?

Regards,
Lucian
  

Comments

Klaus Schmidinger March 23, 2013, 9:50 p.m. UTC | #1
On 23.03.2013 17:14, Lucian Muresan wrote:
> Hi,
>
> thank you Klaus for holding up your release plan for 2.0!
> However, I hope this minor patch won't be too much trouble for you, as
> it only increases the limit of 16 to theme and skin names, letting them
> be NAME_MAX as almost any files, since these names can also be involved
> in file names.
> Was there a good reason to limit them at 16 only, perhaps the fear that
> the name won't fit on the OSD? I don't really think someone would really
> make use of as many as 255 characters for this. On the other hand, users
> just encountered crashes with plugins which (unfortunately, yet) have
> themes of their own, when just adding a new theme with a name longer
> than 16 and at the same time the original plugin author relied on
> MaxThemeName when allocating the string length.

Those are bugs in the plugins and should be fixed there.
Or is this something that can also happen in the core VDR?

> So, what do you think, easy to adopt?

Well, for one, now is definitely not the right time for a change like this!
And furthermore, skin and theme names should be short. What sense does it
make to call a theme something like "This is the theme that implements a range
of colors from 400 nanometers to 700 nanometers", when you could just plain
simple call it "rainbow"? ;-)

So I'd say the limit is there for a reason, and should stay there.

Klaus
  
Lucian Muresan March 24, 2013, 9:49 a.m. UTC | #2
On 23.03.2013 22:50, Klaus Schmidinger wrote:
> On 23.03.2013 17:14, Lucian Muresan wrote:
>> Hi,
>>
>> thank you Klaus for holding up your release plan for 2.0!
>> However, I hope this minor patch won't be too much trouble for you, as
>> it only increases the limit of 16 to theme and skin names, letting them
>> be NAME_MAX as almost any files, since these names can also be involved
>> in file names.
>> Was there a good reason to limit them at 16 only, perhaps the fear that
>> the name won't fit on the OSD? I don't really think someone would really
>> make use of as many as 255 characters for this. On the other hand, users
>> just encountered crashes with plugins which (unfortunately, yet) have
>> themes of their own, when just adding a new theme with a name longer
>> than 16 and at the same time the original plugin author relied on
>> MaxThemeName when allocating the string length.
> 
> Those are bugs in the plugins and should be fixed there.
> Or is this something that can also happen in the core VDR?

So be it, it's quite easy for plugins to ignore MaxThemeName or allocate
such a string length, say 4*MaxThemeName as long as they mange their own
themes...
I did not test how VDR reacts when it is fed with a theme or a skin with
a name longer than 16 (seems those are the only 2 places in VDR where
you use those, on statically allocating the strings holding the theme
and the skin, which you store to and load from setup...

>> So, what do you think, easy to adopt?
> 
> Well, for one, now is definitely not the right time for a change like this!
> And furthermore, skin and theme names should be short. What sense does it
> make to call a theme something like "This is the theme that implements a
> range
> of colors from 400 nanometers to 700 nanometers", when you could just plain
> simple call it "rainbow"? ;-)

Not necessary to exaggerate with such an hilarious example, you're
getting near to sarcastic and missing the point, let's just take your
short example in a more realistic scenario:

rainbow_1920x1080.theme
rainbow_1280x768.theme

just because there still are plugins having to use skins which are
resolution-dependent, so those names aren't at all that uncommonly long
like your example, yet they try to carry some little useful information.
And still, one of them is already violating the limit (if we do not
consider the file extension). Of course, you might say, take out the
'_', or the 'x', I could answer I'd rather take out some letter out of
"rainbow", and so on, you see what I'm trying to point out, it's hitting
a limit difficult to argue...

> So I'd say the limit is there for a reason, and should stay there.

Could you at least please, name it?

Regards,
Lucian
  
Klaus Schmidinger March 24, 2013, 10 a.m. UTC | #3
On 24.03.2013 10:49, Lucian Muresan wrote:
> On 23.03.2013 22:50, Klaus Schmidinger wrote:
>> On 23.03.2013 17:14, Lucian Muresan wrote:
>>> Hi,
>>>
>>> thank you Klaus for holding up your release plan for 2.0!
>>> However, I hope this minor patch won't be too much trouble for you, as
>>> it only increases the limit of 16 to theme and skin names, letting them
>>> be NAME_MAX as almost any files, since these names can also be involved
>>> in file names.
>>> Was there a good reason to limit them at 16 only, perhaps the fear that
>>> the name won't fit on the OSD? I don't really think someone would really
>>> make use of as many as 255 characters for this. On the other hand, users
>>> just encountered crashes with plugins which (unfortunately, yet) have
>>> themes of their own, when just adding a new theme with a name longer
>>> than 16 and at the same time the original plugin author relied on
>>> MaxThemeName when allocating the string length.
>>
>> Those are bugs in the plugins and should be fixed there.
>> Or is this something that can also happen in the core VDR?
>
> So be it, it's quite easy for plugins to ignore MaxThemeName or allocate
> such a string length, say 4*MaxThemeName as long as they mange their own
> themes...
> I did not test how VDR reacts when it is fed with a theme or a skin with
> a name longer than 16 (seems those are the only 2 places in VDR where
> you use those, on statically allocating the strings holding the theme
> and the skin, which you store to and load from setup...
>
>>> So, what do you think, easy to adopt?
>>
>> Well, for one, now is definitely not the right time for a change like this!
>> And furthermore, skin and theme names should be short. What sense does it
>> make to call a theme something like "This is the theme that implements a
>> range
>> of colors from 400 nanometers to 700 nanometers", when you could just plain
>> simple call it "rainbow"? ;-)
>
> Not necessary to exaggerate with such an hilarious example, you're
> getting near to sarcastic and missing the point, let's just take your
> short example in a more realistic scenario:
>
> rainbow_1920x1080.theme
> rainbow_1280x768.theme

Themes are all about colors - why do they even need to mention resolutions?

Besides, skins should react dynamically on the current OSD size.

> just because there still are plugins having to use skins which are
> resolution-dependent, so those names aren't at all that uncommonly long
> like your example, yet they try to carry some little useful information.
> And still, one of them is already violating the limit (if we do not
> consider the file extension). Of course, you might say, take out the
> '_', or the 'x', I could answer I'd rather take out some letter out of
> "rainbow", and so on, you see what I'm trying to point out, it's hitting
> a limit difficult to argue...
>
>> So I'd say the limit is there for a reason, and should stay there.
>
> Could you at least please, name it?

Keeping the names *short* ;-)

This limit has been in there for almost ten years and has apparently
never been a problem (at least not to my knowledge). I can't change that
so close before the release of a major new version - even if it might
look like it won't break anything. You can get back to me with this after
version 2.0.0 is out. Version 2.1.x can pick up such changes again...

Klaus
  
Helmut Auer March 24, 2013, 10:21 a.m. UTC | #4
Am 24.03.2013 11:00, schrieb Klaus Schmidinger:
>
> This limit has been in there for almost ten years and has apparently
> never been a problem (at least not to my knowledge).
 >
Sorry to hack this thread , but my wound is still wide open :(
There also was a makefile concept for ten years and apparently no one has a problem with it, but 
it was changed and that that has cost me many many hours and days and all the fun I had with VDR 
before ...
  
Manuel Reimer March 24, 2013, 10:30 a.m. UTC | #5
Helmut Auer wrote:
> Sorry to hack this thread , but my wound is still wide open :(
> There also was a makefile concept for ten years and apparently no one has a
> problem with it, but it was changed and that that has cost me many many hours
> and days and all the fun I had with VDR before ...

Then please, finally, name your problems, so someone can help you to find solutions.

The old Makefile system did not work well for packaging and made it
unneccessarily difficult to build plugins from outside the VDR source. It also 
often required manual steps to be done after installing the plugin (copy 
resources or something like that) which can be done in the "install" target, 
now. So in my opinion the new system is a big benefit for 2.0.

Yours

Manuel
  
Lucian Muresan March 24, 2013, 10:31 a.m. UTC | #6
Hi,

On 24.03.2013 11:00, Klaus Schmidinger wrote:
[...]
>> rainbow_1920x1080.theme
>> rainbow_1280x768.theme
> 
> Themes are all about colors - why do they even need to mention resolutions?
> 
> Besides, skins should react dynamically on the current OSD size.

as mentioned right after giving the example, that's the way some of the
themes unfortunately work, but that's due to the technology they use
(they are displaying background images and the like, which are
pre-generated for that exact resolution, possibly a dynamic scaling
won't look right in those cases). On the other hand, I'm completely with
you, they should behave better, but they are not my skins, nor do I
particularly use them, I was only trying to help the users a bit, which
add such skins without touching any code...

>> just because there still are plugins having to use skins which are
>> resolution-dependent, so those names aren't at all that uncommonly long
>> like your example, yet they try to carry some little useful information.
>> And still, one of them is already violating the limit (if we do not
>> consider the file extension). Of course, you might say, take out the
>> '_', or the 'x', I could answer I'd rather take out some letter out of
>> "rainbow", and so on, you see what I'm trying to point out, it's hitting
>> a limit difficult to argue...
>>
>>> So I'd say the limit is there for a reason, and should stay there.
>>
>> Could you at least please, name it?
> 
> Keeping the names *short* ;-)

Seriously ;) ?

> This limit has been in there for almost ten years and has apparently
> never been a problem (at least not to my knowledge). I can't change that
> so close before the release of a major new version - even if it might
> look like it won't break anything. You can get back to me with this after
> version 2.0.0 is out. Version 2.1.x can pick up such changes again...

That's fair enough and also ok with me, thanks.

Regards,
Lucian
  
Helmut Auer March 24, 2013, 10:51 a.m. UTC | #7
> Helmut Auer wrote:
>> Sorry to hack this thread , but my wound is still wide open :(
>> There also was a makefile concept for ten years and apparently no one has a
>> problem with it, but it was changed and that that has cost me many many hours
>> and days and all the fun I had with VDR before ...
>
> Then please, finally, name your problems, so someone can help you to find solutions.
>
There's nothing to find anymore, I had to debug crashes of a plugin (not my own) which were 
caused by migrating to the new makefile, because cflags and c++flags were differnet.
I searched for the segfault in the code but the reason was the make, and that costs me some days.
And also the migration of many plugins costs me a lot of time.

> The old Makefile system did not work well for packaging and made it
> unneccessarily difficult to build plugins from outside the VDR source. It also
> often required manual steps to be done after installing the plugin (copy
> resources or something like that) which can be done in the "install" target,
> now. So in my opinion the new system is a big benefit for 2.0.
>
Just for my point of view, I do not have any benefit of the new makefile concept, only trouble.
This makefile change has lead to the biggest cut in VDR times, imho such a change should be made 
after a major release not (in VDR times) short before a major release.

If I'd be a a vdr user thats no problem, I can stay with vdr 1.7.32. Bu unfortunately I'm a 
distributor and I've promised my users a new version with VDR 2.0.
Now I have the choice between breaking my word or setting up the new system which will cost me 
some more days with a system that I do not like.
  
Christopher Reimer March 24, 2013, 12:01 p.m. UTC | #8
Am 24.03.2013 11:51, schrieb Helmut Auer:
>> Helmut Auer wrote:
>>> Sorry to hack this thread , but my wound is still wide open :(
>>> There also was a makefile concept for ten years and apparently no
>>> one has a
>>> problem with it, but it was changed and that that has cost me many
>>> many hours
>>> and days and all the fun I had with VDR before ...
>> 
>> Then please, finally, name your problems, so someone can help you to
>> find solutions.
>> 
> There's nothing to find anymore, I had to debug crashes of a plugin
> (not my own) which were caused by migrating to the new makefile,
> because cflags and c++flags were differnet.

I can't imagine, that this is caused by a changed Makefile.

Which plugin are you refering to?

> I searched for the segfault in the code but the reason was the make,
> and that costs me some days.
> And also the migration of many plugins costs me a lot of time.

You don't have to migrate them. You can stay with the old Makefiles.
Take a look at the VDR4Arch Project

https://github.com/CReimer/vdr4arch/blob/master/plugins/vdr-filebrowser/PKGBUILD

> 
>> The old Makefile system did not work well for packaging and made it
>> unneccessarily difficult to build plugins from outside the VDR
>> source. It also
>> often required manual steps to be done after installing the plugin
>> (copy
>> resources or something like that) which can be done in the "install"
>> target,
>> now. So in my opinion the new system is a big benefit for 2.0.
>> 
> Just for my point of view, I do not have any benefit of the new
> makefile concept, only trouble.
> This makefile change has lead to the biggest cut in VDR times, imho
> such a change should be made after a major release not (in VDR times)
> short before a major release.
> 
> If I'd be a a vdr user thats no problem, I can stay with vdr 1.7.32.
> Bu unfortunately I'm a distributor and I've promised my users a new
> version with VDR 2.0.
> Now I have the choice between breaking my word or setting up the new
> system which will cost me some more days with a system that I do not
> like.

There's also a problem with your attitude. You want to provide all
plugins regardless of how old or how broken they are.


Christopher Reimer
  
Helmut Auer March 24, 2013, 12:24 p.m. UTC | #9
Hi

>> There's nothing to find anymore, I had to debug crashes of a plugin
>> (not my own) which were caused by migrating to the new makefile,
>> because cflags and c++flags were differnet.
>
> I can't imagine, that this is caused by a changed Makefile.
>
There are lots of things that you can't imagine ;)

> Which plugin are you refering to?
>
softhddevice and live Plugin.
There was a mismatch between c an c++ flags and this was surely caused by the new makefile system :)


>> I searched for the segfault in the code but the reason was the make,
>> and that costs me some days.
>> And also the migration of many plugins costs me a lot of time.
>
> You don't have to migrate them. You can stay with the old Makefiles.
> Take a look at the VDR4Arch Project
>
> https://github.com/CReimer/vdr4arch/blob/master/plugins/vdr-filebrowser/PKGBUILD
>
I already know this, but there were times without the compatibility flag.

>> If I'd be a a vdr user thats no problem, I can stay with vdr 1.7.32.
>> Bu unfortunately I'm a distributor and I've promised my users a new
>> version with VDR 2.0.
>> Now I have the choice between breaking my word or setting up the new
>> system which will cost me some more days with a system that I do not
>> like.
>
> There's also a problem with your attitude. You want to provide all
> plugins regardless of how old or how broken they are.
>
Thats your point of view I just want to build these plugins which are used by Gen2VDR users ...
  
Lucian Muresan March 24, 2013, 12:51 p.m. UTC | #10
Hi,

On 24.03.2013 13:24, Helmut Auer wrote:
[...]
>>> There's nothing to find anymore, I had to debug crashes of a plugin
>>> (not my own) which were caused by migrating to the new makefile,
>>> because cflags and c++flags were differnet.
>>
>> I can't imagine, that this is caused by a changed Makefile.
>>
> There are lots of things that you can't imagine ;)
> 
>> Which plugin are you refering to?
>>
> softhddevice and live Plugin.
> There was a mismatch between c an c++ flags and this was surely caused
> by the new makefile system :)
> 
> 
>>> I searched for the segfault in the code but the reason was the make,
>>> and that costs me some days.

I could give you another example, Christopher, of which Makefile was
migrated by yourself, with exactly the consequence pointed out by
Helmut, I only don't think we're allowed to mention that plugin here, so
just remember, 3 months ago, "Issue #18" of that Plugin on Github...

Regards, Lucian
  
Christopher Reimer March 24, 2013, 1:10 p.m. UTC | #11
Am 24.03.2013 13:51, schrieb Lucian Muresan:
> Hi,
> 
> On 24.03.2013 13:24, Helmut Auer wrote:
> [...]
>>>> There's nothing to find anymore, I had to debug crashes of a plugin
>>>> (not my own) which were caused by migrating to the new makefile,
>>>> because cflags and c++flags were differnet.
>>> 
>>> I can't imagine, that this is caused by a changed Makefile.
>>> 
>> There are lots of things that you can't imagine ;)
>> 
>>> Which plugin are you refering to?
>>> 
>> softhddevice and live Plugin.
>> There was a mismatch between c an c++ flags and this was surely
>> caused
>> by the new makefile system :)
>> 
>> 
>>>> I searched for the segfault in the code but the reason was the
>>>> make,
>>>> and that costs me some days.
> 
> I could give you another example, Christopher, of which Makefile was
> migrated by yourself, with exactly the consequence pointed out by
> Helmut, I only don't think we're allowed to mention that plugin here,
> so
> just remember, 3 months ago, "Issue #18" of that Plugin on Github...
> 
> Regards, Lucian
> 

Even before Issue 18 was fixed it worked flawlessly on Archlinux.

live still uses the old makefile and softhddevice works great here.


Christopher
  
Tobias Grimm March 24, 2013, 1:15 p.m. UTC | #12
On 24.03.2013 13:51, Lucian Muresan wrote:

>>>> I searched for the segfault in the code but the reason was the make,
>>>> and that costs me some days.

There's not very much which go wrong. As long as the plugins are compiled 
with -fPIC and -D_FILE_OFFSET_BITS=64 there should be no ABI 
incompatibilities.

I can understand your frustration - changing a myriad of packages is an 
annoying task (been there - done that! :-)

But after all I think the new Makefile is much better and for the first 
time works out of the box with the standard Debian packaging tools (which 
require *FLAGS, DESTDIR, PREFIX and so on).

Packaging is much easier right now as you basically only have to do:

make all PREFIX=/ ...
make install DESTDIR=...

And for plugins that haven't migrated to the new Makefile the only change 
I had to do was:

export CXXFLAGS += $(shell pkg-config vdr --variable=cxxflags)

The only thing I'm missing is a CPPFLAG in the Makefiles, which I hope 
gets added in 2.0+X.

But I'm talking from the Debian/Ubuntu perspective only - things might be 
slighty more complicated in Gentoo.

Tobias
  
VDRU VDRU March 24, 2013, 2:48 p.m. UTC | #13
On Sun, Mar 24, 2013 at 5:24 AM, Helmut Auer <vdr@helmutauer.de> wrote:
>> Which plugin are you refering to?
>>
> softhddevice and live Plugin.
> There was a mismatch between c an c++ flags and this was surely caused by
> the new makefile system :)

When using the new style Makefile supplied with softhddevice git, I
was getting crashed. Johns acknowledged the plugin had issues with the
new Makefile and recommended to continue using the provided old-style
Makefile. So are you saying that you've actually fixed softhddevice so
it works correctly and no longer crashes when using the new-style
Makefile? If so, would you mind submitting that patch to Johns so he
can merge it?
  
Lucian Muresan March 24, 2013, 4:36 p.m. UTC | #14
On 24.03.2013 14:10, Christopher Reimer wrote:
> Am 24.03.2013 13:51, schrieb Lucian Muresan:
>> Hi,
>>
>> On 24.03.2013 13:24, Helmut Auer wrote:
>> [...]
>>>>> There's nothing to find anymore, I had to debug crashes of a plugin
>>>>> (not my own) which were caused by migrating to the new makefile,
>>>>> because cflags and c++flags were differnet.
>>>>
>>>> I can't imagine, that this is caused by a changed Makefile.
>>>>
>>> There are lots of things that you can't imagine ;)
>>>
>>>> Which plugin are you refering to?
>>>>
>>> softhddevice and live Plugin.
>>> There was a mismatch between c an c++ flags and this was surely
>>> caused
>>> by the new makefile system :)
>>>
>>>
>>>>> I searched for the segfault in the code but the reason was the
>>>>> make,
>>>>> and that costs me some days.
>>
>> I could give you another example, Christopher, of which Makefile was
>> migrated by yourself, with exactly the consequence pointed out by
>> Helmut, I only don't think we're allowed to mention that plugin here,
>> so
>> just remember, 3 months ago, "Issue #18" of that Plugin on Github...
>>
>> Regards, Lucian
>>
> 
> Even before Issue 18 was fixed it worked flawlessly on Archlinux.

Well, and therefore you concluded it ought to work on any system, but
that's not necessarily true.
I wonder, do you use vanilla VDR or a patched VDR on Archlinux? Gentoo
uses the extended patch, it's just the way it is, users are just glad
that this is possible for them, and when doing so it is absolutely
necessary that all of the plugins are built with the exact same DEFINES
introduced by the patches, and that happens to well, happen or not, in
the plugin Makefile.
Ignoring them, just because of thinking "plugin A does not use any of
the patches X, Y or Z, so I don't need the DEFINES" is likely to give
you a working plugin only if you are _very_ lucky, otherwise unexpected
crashes will bite you.
To take this out of the fortune domain one would have to either analyze
with some true coverage techniques that absolutely no patched code is
called in a specific plugin (and by that I mean, VDR internal code is
also calling itself, so that's why an analysis is not trivial), or to
just make sure, give the DEFINES to all of the plugins. So it turns out
that the new Makefiles are just adopted in a different way by people,
and also there was no clear directive where to store the DEFINES in case
of patches, in /usr/include/vdr/Make.conf (or how it should be called
lately), or right into the cxxflags and cxflags in vdr.pc...

So you see, the whole process caused also a lot of confusion, also
because each Makefile contains the whole logic, I would have placed the
logic in a central, pseudo-Makefile shipped with the vdr sources, and
generated some kind of Makefile stubs for the plugins, including the
central one for the logic, and providing plugin-specifics (even custom,
additional targets) through a well defined interface. That way, once the
interface would have been established, the central logic could have been
adapted fromtime to time without having to ever touch plugin Makefiles
again. Unfortunatley, for me it just wasn't the right time to interfere
in the makefile discussions when they were elaborated. I might come back
with a drop-in proposal by the the time of the next developent series,
quite possible that having only a few lines makefiles would be
interesting for someone...

Regards, Lucian
  
Lucian Muresan March 24, 2013, 4:39 p.m. UTC | #15
On 24.03.2013 14:15, Tobi wrote:
> On 24.03.2013 13:51, Lucian Muresan wrote:
> 
>>>>> I searched for the segfault in the code but the reason was the make,
>>>>> and that costs me some days.
> 
> There's not very much which go wrong. As long as the plugins are
> compiled with -fPIC and -D_FILE_OFFSET_BITS=64 there should be no ABI
> incompatibilities.
> 
> I can understand your frustration - changing a myriad of packages is an
> annoying task (been there - done that! :-)
> 
> But after all I think the new Makefile is much better and for the first
> time works out of the box with the standard Debian packaging tools
> (which require *FLAGS, DESTDIR, PREFIX and so on).
> 
> Packaging is much easier right now as you basically only have to do:
> 
> make all PREFIX=/ ...
> make install DESTDIR=...
> 
> And for plugins that haven't migrated to the new Makefile the only
> change I had to do was:
> 
> export CXXFLAGS += $(shell pkg-config vdr --variable=cxxflags)
> 
> The only thing I'm missing is a CPPFLAG in the Makefiles, which I hope
> gets added in 2.0+X.
> 
> But I'm talking from the Debian/Ubuntu perspective only - things might
> be slighty more complicated in Gentoo.

Well, see my other answer to Christopher, where I explained what can go
wrong (which of course, doesn't have to happen if using plain vanilla VDR).

Regards,
Lucian
  
Dominic Evans March 24, 2013, 5:29 p.m. UTC | #16
On 24 March 2013 16:36, Lucian Muresan <lucianm@users.sourceforge.net> wrote:
>> Even before Issue 18 was fixed it worked flawlessly on Archlinux.
>
> Well, and therefore you concluded it ought to work on any system, but
> that's not necessarily true.
>
> *SNIP*
>

tbh VDR is quite unusual that it provides this 'template' system for
which plugins can base their Makefiles upon. Consider that most other
apps that provide extension points for plugins to implements only
provide pkg-config flags, and nothing else :)

Klaus is already doing people a massive favour by maintaining this
Make system and continuing to support it. The changes in 2.0 were well
agreed, refined and implemented. Any plugins that don't trivially
build within the new way should be left up to their maintainer to fix,
or interested parties to submit patches.
  
fnu March 24, 2013, 5:41 p.m. UTC | #17
> Klaus is already doing people a massive favour by maintaining this Make
> system and continuing to support it.

FullAck, I guess most people appreciate this way, it makes live not that
bad, some minor changes needed to keep any plugin. There's more work for
many plugins regarding other changes in VDR, despite which Makefile system.
This was IMHO a great job, to keep 99% of the old system with the view into
the future.

With post 2.0 Klaus can now uncompromising cut everything old and
unnecessary, which will probably sort out 80% of all plugins and make
distributors live even easier ... ^^

Thanks for this great work up to now Klaus!

===
Kind regards
fnu
  
VDRU VDRU March 24, 2013, 6:15 p.m. UTC | #18
On Sun, Mar 24, 2013 at 9:36 AM, Lucian Muresan
<lucianm@users.sourceforge.net> wrote:
>> Even before Issue 18 was fixed it worked flawlessly on Archlinux.
>
> Well, and therefore you concluded it ought to work on any system, but
> that's not necessarily true.
> I wonder, do you use vanilla VDR or a patched VDR on Archlinux? Gentoo
> uses the extended patch, it's just the way it is, users are just glad
> that this is possible for them, and when doing so it is absolutely
> necessary that all of the plugins are built with the exact same DEFINES
> introduced by the patches, and that happens to well, happen or not, in
> the plugin Makefile.
> Ignoring them, just because of thinking "plugin A does not use any of
> the patches X, Y or Z, so I don't need the DEFINES" is likely to give
> you a working plugin only if you are _very_ lucky, otherwise unexpected
> crashes will bite you.

I've seen enough coder complaints to know it's an unwritten rule of
thumb that when you patch things and it changes/breaks vanilla
behavior, it's your responsibility to maintain and fix anything that
gets lamed. Rather than expecting Klaus to cater to such cases, you
should appreciate he's willing to consider them.
  
Helmut Auer March 24, 2013, 10:55 p.m. UTC | #19
>>>
>> softhddevice and live Plugin.
>> There was a mismatch between c an c++ flags and this was surely caused by
>> the new makefile system :)
>
> When using the new style Makefile supplied with softhddevice git, I
> was getting crashed. Johns acknowledged the plugin had issues with the
> new Makefile and recommended to continue using the provided old-style
> Makefile. So are you saying that you've actually fixed softhddevice so
> it works correctly and no longer crashes when using the new-style
> Makefile? If so, would you mind submitting that patch to Johns so he
> can merge it?
>
Sorry to say - I haven't fixed that, after I recognized that this ***** makefile stuff was the 
reason I switched back to vdr.1.7.32 and stopped every development for newer versions.
  
Manuel Reimer March 25, 2013, 4:36 p.m. UTC | #20
Lucian Muresan wrote:
> I wonder, do you use vanilla VDR or a patched VDR on Archlinux?

https://github.com/CReimer/vdr4arch/blob/master/vdr/PKGBUILD#L31

One patch (MainMenuHooks) which doesn't change anything on the plugin API.

> Gentoo
> uses the extended patch, it's just the way it is, users are just glad
> that this is possible for them, and when doing so it is absolutely
> necessary that all of the plugins are built with the exact same DEFINES
> introduced by the patches, and that happens to well, happen or not, in
> the plugin Makefile.

The best way to resolve this would be to switch to separate patches as soon as 
possible. The extension patch is broken by design, as it causes the VDR to place 
header files with conditional compiling expressions. So the API changes based on 
the DEFINES even after installation!

> Ignoring them, just because of thinking "plugin A does not use any of
> the patches X, Y or Z, so I don't need the DEFINES" is likely to give
> you a working plugin only if you are _very_ lucky, otherwise unexpected
> crashes will bite you.

The extension patch already patches *many* files in the VDR source, so as it 
requires the DEFINES to be exported, it should also patch the Makefile to export 
them in the cflags variable in vdr.pc.

Yours

Manuel
  
Lucian Muresan March 25, 2013, 7:09 p.m. UTC | #21
On 25.03.2013 17:36, Manuel Reimer wrote:
> The best way to resolve this would be to switch to separate patches as
> soon as possible. The extension patch is broken by design, as it causes
> the VDR to place header files with conditional compiling expressions. So
> the API changes based on the DEFINES even after installation!

Quite possible that it is broken by design, but if used carefully (i.E.
giving all of these DEFINES to all of the plugins built against that
specific patched VDR version, should give all of them the same API) it
works.

> The extension patch already patches *many* files in the VDR source, so
> as it requires the DEFINES to be exported, it should also patch the
> Makefile to export them in the cflags variable in vdr.pc.

So it does on Gentoo, but the problem was with various plugin makefiles
actually, which claimed to be "new style" but for whatever reason the
persons who adapted them to the new style removed including $(PLGCFG) or
circumvented it by mistake. That is also due to some acrobatic
conditional constructs in some desperate tries to keep the same
makefiles also backwards compatible with a whole lot of older VDR
series, instead of just providing the old makefile under another name
just as it was.

So it takes two to tango, plugin Makefiles also have to use those
DEFINES, it's not enough that they are provided in some place everyone
has agreed upon. That was my point actually, while Klaus is doing a
great job, some of the plugin authors failed to do so, which is also
only human, I only would have given them less opportunity to fail...

Regards,
Lucian
  
L. Hanisch March 25, 2013, 7:42 p.m. UTC | #22
Hi,

Am 25.03.2013 20:09, schrieb Lucian Muresan:
> That is also due to some acrobatic
> conditional constructs in some desperate tries to keep the same
> makefiles also backwards compatible with a whole lot of older VDR
> series, instead of just providing the old makefile under another name
> just as it was.

 I think the best way to get compatible with old vdr versions is to manually create a vdr.pc file. Then plugins with new
Makefiles will just build. This should be up to the distributors. Just copy one from a recent vdr and edit the API
version and paths... Have done it with the current stable-vdr 1.7.27 in yaVDR.

 For plugins with old Makefiles: just get the needed variables from vdr.pc in your build-script (pkg-build, debian/rules
etc.) and you're done.

 I like the new Makefile... :)

 I don't like defines in headers, it's just a source of error noone should be forced to manage. Wrapping multiple
patches into each other can only be maintainable if you don't make them selectable. The distributor decides which
patches he/she likes to distribute and maintaining a patch combination is awfull enough... Done that (and doing it) and
I don't really like that either. :)
 But I will do it until there are no more patches *I* need... :)

 Why argue about the new Makefile? It's done, get used to it...

Peace,
Lars.
  
Manuel Reimer March 26, 2013, 4:19 p.m. UTC | #23
Lucian Muresan wrote:
> So it does on Gentoo, but the problem was with various plugin makefiles
> actually, which claimed to be "new style" but for whatever reason the
> persons who adapted them to the new style removed including $(PLGCFG) or
> circumvented it by mistake.

*Wrong way* to fix this! If the "extension patch" requires plugins to get the 
DEFINES, then it should patch the VDR Makefile to forcefully add them to vdr.pc, 
so Plugin-Makefiles get them even if no "PLGCFG" is used.

Yours

Manuel
  
Lucian Muresan March 27, 2013, 5:40 p.m. UTC | #24
On 26.03.2013 17:19, Manuel Reimer wrote:
> Lucian Muresan wrote:
>> So it does on Gentoo, but the problem was with various plugin makefiles
>> actually, which claimed to be "new style" but for whatever reason the
>> persons who adapted them to the new style removed including $(PLGCFG) or
>> circumvented it by mistake.
> 
> *Wrong way* to fix this! If the "extension patch" requires plugins to
> get the DEFINES, then it should patch the VDR Makefile to forcefully add
> them to vdr.pc, so Plugin-Makefiles get them even if no "PLGCFG" is used.

The Gentoo Gentoo VDR maintainer eventually fixed it in _some_ way
achieving this (don't think there is only *your* solution to the
problem, by fixing the makefile), namely have the DEFINES listed in the
flags in vdr.pc.

Your definition of *wrong way* holds only in the unfortunately now
already established scenario that everyone is free to use the template
plugin makefile generated by the newplugin script solely as an
orientation, and omit anything which doesn't *seem* to break things in
his own use case, and of course, the use case "make LCLBLD=1". So while
the new makefile system as a whole (with vdr.pc and so) _is indeed
better_ than the old one, it just still allows too many possibilities
for the plugin developer to break things for others [1], or even to get
himself into some trouble [2], or just be lazy and not provide complete
install targets [3].

So I just want to fight this fake belief which some people (I do not
mean Klaus by that, who did not make this up by himself, but responded
to and consulted people) try to establish, that the new makefile system
is near to perfect and covers everything. I'm also not with those people
who denigrate it entirely, just wanted to say it has some flaws, it
should be allowed to do so without being slapped by every second reply,
and can only re-iterate that I'm sorry I did not have the time to tell
my opinion earlier.

Best regards, looking forward for the 2.1 series...
Lucian


[1] the already discussed DEFINES situation;

[2] for example, an empty LIBS variable, also used in the correct order
(after OBJS) in the final link statement, just for the case that a
plugin have use them, it could have spared some authors of putting it in
wrong order and then wonder about unresolved symbols;

[3] completely missing usage of RESDIR which also can be read out of
vdr.pc and any sample commented out target using it, that way many
plugin authors continue just to ignore it even if their plugin actually
comes with lots of non-program files we all call resources, which have
to be installed under a certain pattern, only modifiable by the RESDIR
variable. Could continue with CACHEDIR, but that applies to less
plugins... You will say this is up to packagers, but actually since
there are these modifiable variables, and also some files have to comply
to a certain directory pattern, this can be well prepared by the plugin
author, it's only about his plugin, and the packagers just have to
divert those variables specific to their distribution and the plugin
would still work without further modifications, and not fight with doing
missing targets for so many plugins...
  
L. Hanisch March 27, 2013, 10:07 p.m. UTC | #25
Hi,

Am 27.03.2013 18:40, schrieb Lucian Muresan:
> [2] for example, an empty LIBS variable, also used in the correct order
> (after OBJS) in the final link statement, just for the case that a
> plugin have use them, it could have spared some authors of putting it in
> wrong order and then wonder about unresolved symbols;

 +1

 That should be included in the next developer cycle in the plugins' Makefile. Have stumbled upon this two or three
times. I do know it now but the next one will hit it for sure. :)

> [3] completely missing usage of RESDIR which also can be read out of
> vdr.pc and any sample commented out target using it, that way many
> plugin authors continue just to ignore it even if their plugin actually
> comes with lots of non-program files we all call resources, which have
> to be installed under a certain pattern, only modifiable by the RESDIR
> variable.

 It would be nice, if the plugin's install target would install its resources to its well defined resource directory
(it's $RESDIR/plugins/$PLUGIN, isn't it?). That would make packaging even more easy.

> Could continue with CACHEDIR, but that applies to less
> plugins...

 I understand CACHEDIR as a location for runtime generated data which doesn't necessarily survive reboots of the vdr. I
don't think it should be the target of any installation files. Or am I wrong?

> You will say this is up to packagers, but actually since
> there are these modifiable variables, and also some files have to comply
> to a certain directory pattern, this can be well prepared by the plugin
> author, it's only about his plugin, and the packagers just have to
> divert those variables specific to their distribution and the plugin
> would still work without further modifications, and not fight with doing
> missing targets for so many plugins...

Lars.
  
Lucian Muresan March 27, 2013, 11:20 p.m. UTC | #26
On 27.03.2013 23:07, Lars Hanisch wrote:
>> Could continue with CACHEDIR, but that applies to less
>> > plugins...
>  I understand CACHEDIR as a location for runtime generated data which doesn't necessarily survive reboots of the vdr. I
> don't think it should be the target of any installation files. Or am I wrong?

You are not wrong at all, but I actually mean less installing files into
CACHEDIR but rather creating the subdirectory structure there, as it is
expected by that plugin (sometimes it involves the plugin name,
sometimes it's more generic, like for example several plugins sharing
so-called epg-images which are fetched by some script and can be used
for displaying by different plugins, so they would rather use a
directory name not necessarily bound to some plugin name...). But then,
again, I can agree it's arguable if that's not better a runtime option
anyway...

Regards,
Lucian
  
VDRU VDRU March 27, 2013, 11:29 p.m. UTC | #27
I'm just curious..  Is there any other software anyone can think of
that had so much controversy surrounding a Makefile? I'm not aware of
any myself but that doesn't mean much. Surely there's enough packaged
software out there that methods for these problems, if that's what
they are, have been previously resolved.

All I hope is that when the dust settles from the Makefile madness,
VDR hasn't been turned into a mess.
  
L. Hanisch March 27, 2013, 11:36 p.m. UTC | #28
Am 28.03.2013 00:20, schrieb Lucian Muresan:
> On 27.03.2013 23:07, Lars Hanisch wrote:
>>> Could continue with CACHEDIR, but that applies to less
>>>> plugins...
>>  I understand CACHEDIR as a location for runtime generated data which doesn't necessarily survive reboots of the vdr. I
>> don't think it should be the target of any installation files. Or am I wrong?
> 
> You are not wrong at all, but I actually mean less installing files into
> CACHEDIR but rather creating the subdirectory structure there, as it is
> expected by that plugin (sometimes it involves the plugin name,
> sometimes it's more generic, like for example several plugins sharing
> so-called epg-images which are fetched by some script and can be used
> for displaying by different plugins, so they would rather use a
> directory name not necessarily bound to some plugin name...). But then,
> again, I can agree it's arguable if that's not better a runtime option
> anyway...

 Since the cache directory is writeable by the vdr I think the plugin should create its files and folder at runtime.

 Example: osdteletext, which caches several pages from TTX. It should create its folder $CACHE_DIR/plugins/osdteletext
on startup and then create and delete files if needed.

Lars.

> 
> Regards,
> Lucian
> 
> 
> _______________________________________________
> vdr mailing list
> vdr@linuxtv.org
> http://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr
>
  

Patch

diff -Naur vdr-1.7.42_orig/config.h vdr-1.7.42/config.h
--- vdr-1.7.42_orig/config.h	2013-03-16 16:12:14.000000000 +0100
+++ vdr-1.7.42/config.h	2013-03-23 16:57:48.378701000 +0100
@@ -53,8 +53,8 @@ 
 #define MAXOSDHEIGHT 1200
 
 #define MaxFileName NAME_MAX // obsolete - use NAME_MAX directly instead!
-#define MaxSkinName 16
-#define MaxThemeName 16
+#define MaxSkinName NAME_MAX // obsolete - use NAME_MAX directly instead!
+#define MaxThemeName NAME_MAX // obsolete - use NAME_MAX directly instead!
 
 // Basically VDR works according to the DVB standard, but there are countries/providers
 // that use other standards, which in some details deviate from the DVB standard.