Patchwork warn the user if flashrom is x months old

login
register
about
Submitter David Hendricks
Date 2011-07-08 18:46:38
Message ID <CAKOoyUfzyMKeQydoUWLbqiRs7jP7t5dbKSqPmJMJL4FQ9E6N8A@mail.gmail.com>
Download mbox | patch
Permalink /patch/3260/
State Not for merge
Headers show

Comments

David Hendricks - 2011-07-08 18:46:38
On Fri, Jul 8, 2011 at 4:35 AM, Stefan Tauner <
stefan.tauner@student.tuwien.ac.at> wrote:

> i had the idea to implement an expiration date for flashrom binaries
> (i.e. a time bomb). in the case distributions ship ancient binaries
> (in the future) this may save a bunch of hardware.
>
> i noted two reactions back then when i proposed this idea on IRC in my
> todo.txt:
>  <twice11> If I validated software to work on some system, I don't want
>  it to suddenly break one year later.
>  <agaran> twice11: but kind notify that version you use is %d days old
>  (and repeated every multiple of time or 100days or so) could be nice
>

Interesting idea, but I have to agree with twice11 here. I do not want a
version of flashrom that works fine on the systems I'm supporting to
suddenly start breaking scripts or throwing superfluous warnings to users
and testers. This will definitely cause headaches for ODMs/OEMs and
commercial Linux system vendors :-(

IMHO it would be more useful to try and get generic distros to a) not
include a flashrom binary by default and b) when a user installs it
manually, fetch and compile sources from SVN.

As for the patch itself, if this idea moves forward then it would be nice to
isolate the code a bit more so it can be easily removed. Rather than placing
it in print_version(), I recommend creating another function which would be
called from cli_classic(). Something like:

where print_expiration() contains the code that prints warning.
Stefan Tauner - 2011-07-09 12:04:06
On Fri, 8 Jul 2011 11:46:38 -0700
David Hendricks <dhendrix@google.com> wrote:

> On Fri, Jul 8, 2011 at 4:35 AM, Stefan Tauner <
> stefan.tauner@student.tuwien.ac.at> wrote:
> 
> > i had the idea to implement an expiration date for flashrom binaries
> > (i.e. a time bomb). in the case distributions ship ancient binaries
> > (in the future) this may save a bunch of hardware.
> >
> > i noted two reactions back then when i proposed this idea on IRC in my
> > todo.txt:
> >  <twice11> If I validated software to work on some system, I don't want
> >  it to suddenly break one year later.
> >  <agaran> twice11: but kind notify that version you use is %d days old
> >  (and repeated every multiple of time or 100days or so) could be nice
> >
> 
> Interesting idea, but I have to agree with twice11 here. I do not want a
> version of flashrom that works fine on the systems I'm supporting to
> suddenly start breaking scripts or throwing superfluous warnings to users
> and testers. This will definitely cause headaches for ODMs/OEMs and
> commercial Linux system vendors :-(

sure. when i said "it is worth to think about something like this", i
meant something like the current patch, that prints a warning only.
OTOH, it may still be useful to provide an option to really bail out
instead of warning only (which of course would be off by default).
the question is if any maintainer or individual may ever use it...
if there is interest for this, i could create an add-on patch for that
so we can decide its inclusion independently from the warning patch.
please say so, if you think i should do that.

> IMHO it would be more useful to try and get generic distros to a) not
> include a flashrom binary by default and b) when a user installs it
> manually, fetch and compile sources from SVN.

providing a script that tries to mimic what the wiki installation guide
tells the user to do, might be worth then?
checking the availability of different package managers is quite easy,
checking for the correct names shouldnt be much harder?

> As for the patch itself, if this idea moves forward then it would be nice to
> isolate the code a bit more so it can be easily removed. Rather than placing
> it in print_version(), I recommend creating another function which would be
> called from cli_classic(). 

jup, certainly a good idea.
"so it can be easily removed" -> do you think anyone would really want
that? does it justify an option in the makefile?
maybe we should "move" the OLD_MONTHS macro to the macro and add an
#ifdef around the code?

carldani: the xml "parsing" is not a problem... parsing the normal svn
info output would be harder. do you see a problem in the way i do it?

what do you think could "cause problems for users of live CDs and
FreeDOS distributions" in the current implementation?
Carl-Daniel Hailfinger - 2011-07-09 16:32:05
Am 09.07.2011 14:04 schrieb Stefan Tauner:
> On Fri, 8 Jul 2011 11:46:38 -0700
> David Hendricks <dhendrix@google.com> wrote:
>   
>> On Fri, Jul 8, 2011 at 4:35 AM, Stefan Tauner 
>> <stefan.tauner@student.tuwien.ac.at> wrote:
>>     
>>> i had the idea to implement an expiration date for flashrom binaries
>>> (i.e. a time bomb). in the case distributions ship ancient binaries
>>> (in the future) this may save a bunch of hardware.
>>>
>>> i noted two reactions back then when i proposed this idea on IRC in my
>>> todo.txt:
>>>  <twice11> If I validated software to work on some system, I don't want
>>>  it to suddenly break one year later.
>>>  <agaran> twice11: but kind notify that version you use is %d days old
>>>  (and repeated every multiple of time or 100days or so) could be nice
>>>       
>> Interesting idea, but I have to agree with twice11 here. I do not want a
>> version of flashrom that works fine on the systems I'm supporting to
>> suddenly start breaking scripts or throwing superfluous warnings to users
>> and testers. This will definitely cause headaches for ODMs/OEMs and
>> commercial Linux system vendors :-(
>>     
> sure. when i said "it is worth to think about something like this", i
> meant something like the current patch, that prints a warning only.
> OTOH, it may still be useful to provide an option to really bail out
> instead of warning only (which of course would be off by default).
> the question is if any maintainer or individual may ever use it...
> if there is interest for this, i could create an add-on patch for that
> so we can decide its inclusion independently from the warning patch.
> please say so, if you think i should do that.
>   

The issue of suddenly appearing warnings which change the user
experience is not to be taken lightly.


>> IMHO it would be more useful to try and get generic distros to a) not
>> include a flashrom binary by default and b) when a user installs it
>> manually, fetch and compile sources from SVN.
>>     
> providing a script that tries to mimic what the wiki installation guide
> tells the user to do, might be worth then?
> checking the availability of different package managers is quite easy,
> checking for the correct names shouldnt be much harder?
>   

If anything, we should automatically build a static flashrom version for
all supported architectures and platforms on each checkin, and offer
those for download. That static flashrom binary should include libpci.


>> As for the patch itself, if this idea moves forward then it would be nice to
>> isolate the code a bit more so it can be easily removed. Rather than placing
>> it in print_version(), I recommend creating another function which would be
>> called from cli_classic(). 
>>     
> jup, certainly a good idea.
> "so it can be easily removed" -> do you think anyone would really want
> that? does it justify an option in the makefile?
> maybe we should "move" the OLD_MONTHS macro to the macro and add an
> #ifdef around the code?
>   

What about printing the sentence only if something is marked as
untested? OTOH, we already tell people to recheck with latest flashrom
in that case.


> carldani: the xml "parsing" is not a problem... parsing the normal svn
> info output would be harder. do you see a problem in the way i do it?
>   

Would that really be harder? Normal svn output is pretty constant from a
formatting POV, but with XML they can insert linebreaks wherever they
want. Do you handle that correctly?


> what do you think could "cause problems for users of live CDs and
> FreeDOS distributions" in the current implementation?
>   

I think the current stable FreeDOS 1.0 distibution is ~4 years old. We
should consider the case where people want to ship flashrom in a
distribution without users bugging them about a more current flashrom
release. I fear adverse adoption effects.

To be honest, I think we should postpone the decision until 0.9.4 is out
(maybe even after 0.9.5). The dicussion itself is helpful, and we can
hopefully build a rough consensus how the feature has to work _if_ it
gets included.


Regards,
Carl-Daniel
Stefan Tauner - 2011-07-09 20:56:29
On Sat, 09 Jul 2011 18:32:05 +0200
Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net> wrote:

> Am 09.07.2011 14:04 schrieb Stefan Tauner:
> > On Fri, 8 Jul 2011 11:46:38 -0700
> > David Hendricks <dhendrix@google.com> wrote:
> >   
> >> On Fri, Jul 8, 2011 at 4:35 AM, Stefan Tauner 
> >> <stefan.tauner@student.tuwien.ac.at> wrote:
> >>     
> >>> i had the idea to implement an expiration date for flashrom binaries
> >>> (i.e. a time bomb). in the case distributions ship ancient binaries
> >>> (in the future) this may save a bunch of hardware.
> >>>
> >>> i noted two reactions back then when i proposed this idea on IRC in my
> >>> todo.txt:
> >>>  <twice11> If I validated software to work on some system, I don't want
> >>>  it to suddenly break one year later.
> >>>  <agaran> twice11: but kind notify that version you use is %d days old
> >>>  (and repeated every multiple of time or 100days or so) could be nice
> >>>       
> >> Interesting idea, but I have to agree with twice11 here. I do not want a
> >> version of flashrom that works fine on the systems I'm supporting to
> >> suddenly start breaking scripts or throwing superfluous warnings to users
> >> and testers. This will definitely cause headaches for ODMs/OEMs and
> >> commercial Linux system vendors :-(
> >>     
> > sure. when i said "it is worth to think about something like this", i
> > meant something like the current patch, that prints a warning only.
> > OTOH, it may still be useful to provide an option to really bail out
> > instead of warning only (which of course would be off by default).
> > the question is if any maintainer or individual may ever use it...
> > if there is interest for this, i could create an add-on patch for that
> > so we can decide its inclusion independently from the warning patch.
> > please say so, if you think i should do that.
> >   
> 
> The issue of suddenly appearing warnings which change the user
> experience is not to be taken lightly.

if it changes from "oh damn i have bricked my board because of that
unsupported hardware! why did no one tell me how old that version of
flashrom was" to "aha outdated, maybe i should investigate a bit more
before trying it", i am all for it ;)
but seriously, changing the user experience is the whole idea here.

> >> IMHO it would be more useful to try and get generic distros to a) not
> >> include a flashrom binary by default and b) when a user installs it
> >> manually, fetch and compile sources from SVN.
> >>     
> > providing a script that tries to mimic what the wiki installation guide
> > tells the user to do, might be worth then?
> > checking the availability of different package managers is quite easy,
> > checking for the correct names shouldnt be much harder?
> >   
> 
> If anything, we should automatically build a static flashrom version for
> all supported architectures and platforms on each checkin, and offer
> those for download. That static flashrom binary should include libpci.

covering all possible architectures/OSes might not be practicable(?)

having a script handling package installations and source code checkout
would also ease disaster recovery when a user needs to apply patches
(think: board enable for little french girls... ;)

> >> As for the patch itself, if this idea moves forward then it would be nice to
> >> isolate the code a bit more so it can be easily removed. Rather than placing
> >> it in print_version(), I recommend creating another function which would be
> >> called from cli_classic(). 
> >>     
> > jup, certainly a good idea.
> > "so it can be easily removed" -> do you think anyone would really want
> > that? does it justify an option in the makefile?
> > maybe we should "move" the OLD_MONTHS macro to the macro and add an
> > #ifdef around the code?
> >   
> 
> What about printing the sentence only if something is marked as
> untested? OTOH, we already tell people to recheck with latest flashrom
> in that case.

the problem i try to mitigate here is not "implemented, but untested
features" but unforeseen bugs/unhandled configurations like the locked
regions on ich/pch.

> > carldani: the xml "parsing" is not a problem... parsing the normal svn
> > info output would be harder. do you see a problem in the way i do it?
> >   
> 
> Would that really be harder? Normal svn output is pretty constant from a
> formatting POV, but with XML they can insert linebreaks wherever they
> want. Do you handle that correctly?

the normal svn format of the needed information is like:
Last Changed Date: 2010-07-21 21:41:58 +0000 (Wed, 21 Jul 2010)
TZ=UTC is honored, LC_TIME is too, but i am not sure to what extend...
looks to me as only the language is changed and the format stays the
same. the xml format seems to be always the same, which is to be
expected in any sane implementation imho.

my patch does not handle line breaks inside the date tags, but this
could easily be changed by dropping line breaks before grepping for
example. from what i have experienced while testing it, it is a very
simple (line-oriented) xml output routine. so i doubt this would ever
be a problem anyway, but it is surely a good idea to avoid the problem.

> 
> > what do you think could "cause problems for users of live CDs and
> > FreeDOS distributions" in the current implementation?
> >   
> 
> I think the current stable FreeDOS 1.0 distibution is ~4 years old. We
> should consider the case where people want to ship flashrom in a
> distribution without users bugging them about a more current flashrom
> release. I fear adverse adoption effects.

ok, so you are seeing problems for the developers of those
distributions and not their users. users bugging distros/maintainers
about outdated flashrom binaries is a good thing from my POV. :)
but adding an ifdef and moving the configuration into the makefile to
disable the whole thing as suggested earlier, would solve that?
if the maintainers dont even notice this change and leave the default
on, then i really think it is in the interest of the users to get
notified...

> To be honest, I think we should postpone the decision until 0.9.4 is out
> (maybe even after 0.9.5). The dicussion itself is helpful, and we can
> hopefully build a rough consensus how the feature has to work _if_ it
> gets included.

i don't see much reason for this, but ok...

Patch

--- cli_classic.c       (revision 1367)
+++ cli_classic.c       (working copy)
@@ -146,6 +146,7 @@ 

        print_version();
        print_banner();
+       print_expiration();

        if (selfcheck())
                exit(1);