[virt-tools-list] [PATCH 3/4] install script: add install_script_avatar entity

[Fabiano Fidêncio]

Firstly, sorry for a too late reply. :(

On Tue, Aug 28, 2012 at 11:03 AM, Zeeshan Ali (Khattak)
<zeeshanak at gnome.org> wrote:
> On Tue, Aug 28, 2012 at 5:43 AM, Fabiano Fidêncio <fabiano at fidencio.org> wrote:
>> On Mon, Aug 27, 2012 at 8:14 PM, Zeeshan Ali (Khattak)
>> <zeeshanak at gnome.org> wrote:
>>> On Tue, Aug 28, 2012 at 1:54 AM, Fabiano Fidêncio <fabiano at fidencio.org> wrote:
>>>> On Mon, Aug 27, 2012 at 7:43 PM, Zeeshan Ali (Khattak)
>>>> <zeeshanak at gnome.org> wrote:
>>>>> On Tue, Aug 28, 2012 at 12:57 AM, Fabiano Fidêncio <fabiano at fidencio.org> wrote:
>>>>>> On Mon, Aug 27, 2012 at 6:03 PM, Zeeshan Ali (Khattak)
>>>>>> <zeeshanak at gnome.org> wrote:
>>>>>>> On Sun, Aug 26, 2012 at 11:50 AM, Fabiano Fidêncio <fabiano at fidencio.org> wrote:
>>>>>>>> Add a new entity called OsinfoInstallScriptAvatar to help the creation
>>>>>>>> of an Avatar file for an install script. If avatar entity needs to be
>>>>>>>> set, just add, in the .xml's script file:
>>>>>>>> <avatar>
>>>>>>>>     <image-type>...</image-type>
>>>>>>>>     <extension>...</extension>
>>>>>>>>     <alpha>...</alpha>
>>>>>>>>     <width>...</width>
>>>>>>>>     <height>...</height>
>>>>>>>> </avatar>
>>>>>>>
>>>>>>> I don't quit follow one major thing here: What is the direction of the
>>>>>>> OsinfoInstallScriptAvatar? Is app setting it on libosinfo or app
>>>>>>> getting it from libosinfo? How would a typical usage look like?
>>>>>>
>>>>>> App is getting it from libosinfo. With this entity, we can drop
>>>>>> AvatarFormat boxes' class and get this info from install-script.
>>>>>
>>>>> Ah ok so its all about script telling app that if it needs to add a
>>>>> user avatar, which format it needs to be in? If so, how does app know
>>>>> which filename to use with avatar?
>>>>
>>>> How is it done in gnome-boxes?
>>>
>>> You are writing the Boxes patch so you tell me. :) If you are asking
>>> about existing code, it simply uses the username + extention as the
>>> filename. We'll need a way to create/get avatar filename for/to the
>>> application. My idea would be to treat this similar to how we deal
>>> with windows XP's .cmd and .reg files. They are not scripts either but
>>> we treat them as such.
>>>
>>> BTW, how does this patch work with your avatar releted code in commit 1d9822ae ?
>>
>> Hmmm. Looks like we need to improve OsinfoInstallScriptAvatar to
>> support, beyond what is supported:
>> - mount point
>
> Thats very much specific to boxes code.
>
>> - source path
>> - destination path

So, we will keep only these info:
- filename
- disk
- image-type
- extension (optional, default value is "")
- alpha
- width (optional, default value is -1)
- height (optional, default value is -1)

>
> if app is supposed to provide the avatar, we don't want these. I
> propose the following changes:
>
> * Rename Avatar to AvatarInfo and install_script_get/set_avatar accordingly.
done!

> * Document install_script_get_avatar_info a bit so the flow of
> information is clear
done!

> * Dont assume in the desktop profile that app provided the value of
> avatar-disk (its currently done for fedora at least)
we should do an if to get the value from app, if it's not set, we
should use or current template?

> * Keep install_script_set_avatar_info and avatar_info_new internal
> (apps are not supposed to use those so they better not be exposed to
> them)
done!

> * Add
>   * the missing method to set a value for avatar disk config
done!

>   * config parameter to specify avatar filename/path. In fedora.xml,
> you can then replace:
filename *and* path? should be just filename, right?

>
> cp /mnt/unattended-media/<xsl:value-of select="config/user-login"/>
> /var/lib/AccountsService/icons/
>
> by
>
> cp /mnt/unattended-media/<xsl:value-of
> select="config/avatar-filename"/>
> /var/lib/AccountsService/icons/<xsl:value-of
> select="config/user-login"/>
ack!

>
> Similarly for other OSs.
>
> --
> Regards,
>
> Zeeshan Ali (Khattak)
> FSF member#5124


Best Regards,
-- 
Fabiano Fidêncio