[virt-tools-list] [PATCH 3/6] Change install script output (tool)

[Fabiano Fidêncio]

On Wed, Jul 25, 2012 at 3:14 PM, Daniel P. Berrange <berrange at redhat.com> wrote:
> On Tue, Jul 24, 2012 at 10:05:15PM +0200, Fabiano Fidêncio wrote:
>> Now, instead of return a string, the osinfo-install-script tool writes
>> that string in a file, passed as argument, that will be into a dir,
>> also passed as argument.
>>
>> If the filename argument is NULL, the output files will be written as
>> set in data/install-scripts/*.xml (in filename attribute from template
>> element):
>>     - Linuxes: fedora.ks
>>     - Windows 2k3r2 and older: windows.sif
>>     - Windows 2k8 and newer: windows.xml
>
> I think we should omit the filename argument entirely on the
> command line, because in the Windows case we're going to need
> to support generation of multiple output files. In such a case
> passing a filename on the ARGV doesn't make sense.
>
> I could see value in having an optional arg to specify a
> filename prefix. eg
>
>    # osinfo-install-script --prefix foo   win2k3r2
>
> would result in a file  'foo-windows.sif' being created.

agreed

>
>> If the dirname argument is NULL, the output files will be written in the
>> current directory.
>
> I think we could probably just make that an parameter flag
> instead eg.
>
>    # osinfo-install-script --output-dir /some/dir  win2k3r2
>

eorry, i didn't understand your point.
could you try to clarify it a bit more for me?

>
>> diff --git a/osinfo/osinfo_install_script.c b/osinfo/osinfo_install_script.c
>> index 1f437a1..b2d7351 100644
>> --- a/osinfo/osinfo_install_script.c
>> +++ b/osinfo/osinfo_install_script.c
>> @@ -56,6 +56,7 @@ enum {
>>      PROP_TEMPLATE_DATA,
>>      PROP_PROFILE,
>>      PROP_PRODUCT_KEY_FORMAT,
>> +    PROP_OUTPUT_FILENAME,
>>  };
>
> Given an an <install-script> will allow multiple <template> args
> each with their own filename, I don't think it really makes sense
> to have an 'output-filename' property.  I'd support having an
> "output-prefix" property to support the usage I desribed earlier

agreed

>
>> @@ -686,6 +737,136 @@ gchar *osinfo_install_script_generate(OsinfoInstallScript *script,
>>      return data.output;
>>  }
>>
>> +void osinfo_install_script_generate_output_async(OsinfoInstallScript *script,
>> +                                                 OsinfoOs *os,
>> +                                                 OsinfoInstallConfig *config,
>> +                                                 GFile *output_dir,
>
> This parameter appears to be ignored in this method which is strange ?

it's not so strange and i can explain why.
GOutputFile is used in the callback. Here, I just call the old
function to generate the script and, in the callback, I'll write the
script in the file. Makes sense? Still not clear?

>
>> +                                                 GCancellable *cancellable,
>> +                                                 GAsyncReadyCallback callback,
>> +                                                 gpointer user_data)
>> +{
>> +    osinfo_install_script_generate_async(script,
>> +                                         os,
>> +                                         config,
>> +                                         cancellable,
>> +                                         callback,
>> +                                         user_data);
>> +}
>
> Daniel
> --
> |: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
> |: http://libvirt.org              -o-             http://virt-manager.org :|
> |: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
> |: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|



-- 
Fabiano Fidêncio