Re: [Tails-dev] Please review bugfix/fix_background_readahea…

Delete this message

Reply to this message
Author: bertagaz
Date:  
To: The Tails public development discussion list
New-Topics: [Tails-dev] When should we fix regressions?
Subject: Re: [Tails-dev] Please review bugfix/fix_background_readahead
On Fri, Aug 31, 2012 at 11:15:41AM +0000, Ague Mill wrote:
> On Mon, Aug 27, 2012 at 12:51:04AM +0200, intrigeri wrote:
> > As you know, I tend to think the problem fixed by this patch is not
> > worth the risks involved at this stage of the release process, but my
> > extra-carefulness does not extend to vetoing if there is a general
> > agreement with applying this patch before rc2.
> >
> > > +     start-stop-daemon \
> > > +        --start --background --pid /var/run/background-readahead.pid --startas /bin/sh -- \
> > > +        -c "$BG_FILES | xargs cat >/dev/null 2>&1")

> >
> > I assume you wanted to write --pidfile, as --pid does not exist in my
> > copy of start-stop-daemon. Beware before s/pid/pidfile/, though:
> > given --pidfile presence/absence changes quite drastically the
> > behavior of start-stop-daemon, if the proposed patch works, then this
> > option is probably not needed, is it?
>
> Actually, this works due to GNU getopt magic:
>
>     $ /sbin/start-stop-daemon --start --background --pidfile /tmp/test.pid \
>           --startas /bin/sh -- -c 'date > /tmp/test'
>     $ /sbin/start-stop-daemon --start --background --pid /tmp/test.pid \
>           --startas /bin/sh -- -c 'date > /tmp/test'
>     $ /sbin/start-stop-daemon --start --background --pi /tmp/test.pid \
>           --startas /bin/sh -- -c 'date > /tmp/test'

>
> This fails:
>
>     $ /sbin/start-stop-daemon --start --background --p /tmp/test.pid \
>           --startas /bin/sh -- -c 'date > /tmp/test' 
>     /sbin/start-stop-daemon: option '--p' is ambiguous

>
> Getting rid of `--pidfile` also fails:
>
>     $ /sbin/start-stop-daemon --start --background \
>           --startas /bin/sh -- -c 'date > /tmp/test'
>     /sbin/start-stop-daemon: need at least one of --exec, --pidfile,
>     --user or --name

>
> What this made me realize, though, is that `--pidfile` is useless
> without `--make-pidfile` in this context.
>
> So I have updated the branch to fully spell `--pidfile` and to also
> create that pid file. The broken sed invocation was also fixed, and as
> added bonus, the foreground progress bar now goes to 100%.
>
> I still would like to see this reach 0.13~rc2.


Given we're supposed to be freezed, I'm not sure this is a good idea,
unless there is has been tested a lot before being merged, and there is a
strong commitment to test this in the 0.13~rc2. Is the goodness of this
patch worth the effort or risk to include this so lately in the release
process?

Also I haven't found a corresponding ticket in the wiki, appart from the
old todo/improve_boot_time_on_cd, which is marked as done.

bert.