Plymouth progress patch

[Ray Strode]

Hi,

> 1: I have committed a patch to get the progress to retain between splash
> plugins.
Awesome work!

> Attached is the second half of the patch which I wanted you to OK
> before I commit. It changes the boot-duration to contain the arrival times
> (as a percentage of the whole boot time) of the messages during the previous
> boot.
> e.g:
> 0.381:Applying Intel CPU microcode update:                 [  OK  ]
> 0.408:ip6tables: Applying firewall rules:                  [  OK  ]
> 0.438:iptables: Applying firewall rules:                   [  OK  ]
> 0.502:Bringing up loopback interface:                      [  OK  ]
> 0.578:Starting auditd:                                     [  OK  ]
> 0.618:Starting restorecond:                                [  OK  ]
> 0.763:Starting setroubleshootd:                            [  OK  ]
> 0.810:Starting sshd:                                       [  OK  ]
>
> This would allow the live CD version to come with such a file and the
> progress would calibrate itself to the boot speed of the current machine.
Livecd boot is a real problem that I didn't really know how I was
going to deal with.

This seems like a good plan.

> I also made the progress smooth to remove any backward steps and slowdowns.
> http://www.cs.man.ac.uk/~brejc8/temp/1.svg shows the time in seconds vs the
> progress reported % of several boots.
>
> 2: I was going to propose indirecting the control plugins from rather than
> directly drawing to the shadow framebuffer, to draw to images. This would
> allow a level of flexibility by allowing text to be placed with opacity,
> rotated, etc.
That might be useful for third party (specialized or "fun") plugins.
I think we want to avoid text in the stock plugin as much as possible,
though.  We don't want to show untranslated messages and we don't want
to have to ship a lot of fonts and translations in the initial ram
disk.

> Additionally, I have most of a fixed font control plugin in  place to allow text before pango is loaded.
For the same reasons, I think we want to avoid using this in most
cases.  It's sort of "fail" to show text in the initrd, because the
text will be inherently wrong and broken for a lot of languages.

Comments about patch, below:

> diff --git a/src/libply/ply-progress.c b/src/libply/ply-progress.c
> index 955fe28..dd97eaa 100644
> --- a/src/libply/ply-progress.c
> +++ b/src/libply/ply-progress.c
This should probably be in libplybootsplash or just straight in src/.
libply has sort of grown to be "stuff that's in a runtime library" and
libplybootsplash is more "stuff that's very plymouth specific" but not
specific to any given particular splash plugin.

...
> +static ply_progress_message_t*
> +ply_progress_message_search (ply_list_t *message_list, char* string)
> +{
> +  ply_list_node_t *node;
> +  node = ply_list_get_first_node (message_list);
> +
> +  while (node)
> +    {
> +      ply_progress_message_t *message = ply_list_node_get_data (node);
> +      if (strcmp(string, message->string)==0)
> +          return message;
> +      node = ply_list_get_next_node (message_list, node);
> +    }
> +  return NULL;
> +}
> +

We may want to add a hash table to libply instead of doing the linear
search here
...
>  void
>  ply_progress_pause (ply_progress_t* progress)
>  {
> -  progress->wait_time = ply_get_timestamp ();
> +  progress->pause_time = ply_get_timestamp ();
> +  progress->paused=1;
>    return;
>  }
Minor nit: should use stdbool "true" instead of 1, and should have
spaces around operators (here and elsewhere).

...
> @@ -140,11 +237,53 @@ ply_progress_session_output (ply_progress_t* progress,
>                               const char *output,
>                               size_t      size)
>  {
Instead of ply_progress_session_output it might be better to have
ply_progress_update that is keyed off of the main on_update handler.

Since session text can change from boot to boot (if you change locale,
or if things fail because the network is offline or whatever), we have
init tell us about each service in boot up using plymouth
--update="some-unique-id".  Those ids are what ply_progress should be
fed I think.

That should trim out a lot of parsing code, too, I think.
...
> +  int i=0;
> +
> +  while (i<size)
> +    {
> +      if (output[i] == '\n')
> +        {
> +          ply_progress_message_t* message;
> +          progress->current_message = realloc (progress->current_message, progress->current_message_size+i+1);
> +          memcpy(&progress->current_message[progress->current_message_size], output, i);
> +          progress->current_message_size += i;
> +          progress->current_message[progress->current_message_size]='\0';
This kind of thing could be replaced with a ply-buffer (although this
particular thing should go away completely by hooking into on_update
instead of on_session_output)

Also, make sure you give your copyright to the stuff you write.

--Ray