On Wed, Aug 15, 2012 at 5:21 AM, Fabiano Fidêncio <fabiano@xxxxxxxxxxxx> wrote:
> On Mon, Aug 13, 2012 at 2:54 PM, Zeeshan Ali (Khattak)
> <zeeshanak@xxxxxxxxx> wrote:
>> On Mon, Aug 13, 2012 at 10:37 AM, Fabiano Fidêncio <fabiano@xxxxxxxxxxxx> wrote:
>>> Some functions were added to provide support for, instead of return a
>>> string, the osinfo-install-script tool writes that string in a file,
>>> that will be into a dir, passed as argument.
>>
>> Not really a native English speaker and commit a lot of mistakes
>> myself but the para above isnt very comprehensible. Would this be a
>> correct equivalent?
>>
>> Add variant of osinfo_install_script_generate() that outputs the
>> script into a file in the given directory. Also add API to specify a
>> custom prefix for generated file.
>>
>>> These functions are:
>>> - osinfo_install_script_generate_output:
>>> returns a GFile * instance containing the install script, itself.
>>> This GFile * instance must be unref'd by the caller.
>>>
>>> - osinfo_install_script_generate_output_async:
>>> async version of the previous functions.
>>>
>>> - osinfo_install_script_generate_output_finish:
>>> used when the user uses async method.
>>>
>>> - osinfo_install_script_{set,get}_output_prefix:
>>> used to set/get a prefix that will be prepended in the output
>>> filename. The string set is g_strdup'd and g_free'd internally,
>>> avoiding that user cares about that.
>>
>> If you take the above description I provided than there is no need to
>> mention these details.
>>
>>> Moreover, the "filename" attribute was added in the "template" element
>>> in the install script data, and it will be used as the output filename
>>> for each script and will be more detailed below. Also, support to this
>>> field was provided in the XML schema and in the osinfo_loader as well.
>>>
>>> About the osinfo-install-script tool:
>>> If the prefix 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
>>> Otherwise, the prefix will be prepended in the filename as:
>>> <prefix>-<filename>
>>>
>>> If the dirname argument is NULL, the output files will be written in
>>> the current directory.
>>>
>>> It will be used to create, easily, multiple scripts, as used in:
>>> http://bugzilla-attachments.gnome.org/attachment.cgi?id=214681
>>> ---
>>> data/install-scripts/fedora.xml | 2 +-
>>> data/install-scripts/windows-sif.xml | 2 +-
>>> data/install-scripts/windows-unattend.xml | 2 +-
>>> data/schemas/libosinfo.rng | 1 +
>>> osinfo/libosinfo.syms | 3 +
>>> osinfo/osinfo_install_script.c | 222 ++++++++++++++++++++++++++++--
>>> osinfo/osinfo_install_script.h | 24 ++++
>>> osinfo/osinfo_loader.c | 9 ++
>>> tools/osinfo-install-script.c | 42 +++---
>>> 9 files changed, 276 insertions(+), 31 deletions(-)
>>>
>>> diff --git a/data/install-scripts/fedora.xml b/data/install-scripts/fedora.xml
>>> index b4ad72d..338a570 100644
>>> --- a/data/install-scripts/fedora.xml
>>> +++ b/data/install-scripts/fedora.xml
>>> @@ -1,7 +1,7 @@
>>> <libosinfo version="0.0.1">
>>> <install-script id='http://fedoraproject.org/scripts/fedora/jeos'>
>>> <profile>jeos</profile>
>>> - <template>
>>> + <template filename="fedora.ks">
>>> <xsl:stylesheet
>>> xmlns:xsl="http://www.w3.org/1999/XSL/Transform";
>>> version="1.0">
>>> diff --git a/data/install-scripts/windows-sif.xml b/data/install-scripts/windows-sif.xml
>>> index 29a0eae..46b24ae 100644
>>> --- a/data/install-scripts/windows-sif.xml
>>> +++ b/data/install-scripts/windows-sif.xml
>>> @@ -1,7 +1,7 @@
>>> <libosinfo version="0.0.1">
>>> <install-script id='http://microsoft.com/windows/sif'>
>>> <profile>jeos</profile>
>>> - <template>
>>> + <template filename="windows.sif">
>>> <xsl:stylesheet
>>> xmlns:xsl="http://www.w3.org/1999/XSL/Transform";
>>> version="1.0">
>>> diff --git a/data/install-scripts/windows-unattend.xml b/data/install-scripts/windows-unattend.xml
>>> index 9828b34..20e3e23 100644
>>> --- a/data/install-scripts/windows-unattend.xml
>>> +++ b/data/install-scripts/windows-unattend.xml
>>> @@ -1,7 +1,7 @@
>>> <libosinfo version="0.0.1">
>>> <install-script id='http://microsoft.com/windows/unattend'>
>>> <profile>jeos</profile>
>>> - <template>
>>> + <template filename="windows.xml">
>>> <xsl:stylesheet
>>> xmlns:xsl="http://www.w3.org/1999/XSL/Transform";
>>> version="1.0">
>>> diff --git a/data/schemas/libosinfo.rng b/data/schemas/libosinfo.rng
>>> index a5f527d..7c8d7f7 100644
>>> --- a/data/schemas/libosinfo.rng
>>> +++ b/data/schemas/libosinfo.rng
>>> @@ -414,6 +414,7 @@
>>> <text/>
>>> </element>
>>> <element name='template'>
>>> + <attribute name="filename"/>
>>> <choice>
>>> <group>
>>> <attribute name="uri"/>
>>> diff --git a/osinfo/libosinfo.syms b/osinfo/libosinfo.syms
>>> index 2f90183..ddf736e 100644
>>> --- a/osinfo/libosinfo.syms
>>> +++ b/osinfo/libosinfo.syms
>>> @@ -264,6 +264,7 @@ LIBOSINFO_0.2.0 {
>>> osinfo_install_config_set_user_administrator;
>>> osinfo_install_config_set_user_autologin;
>>> osinfo_install_config_set_hostname;
>>> + osinfo_install_script_set_output_prefix;
>>> osinfo_install_script_get_type;
>>> osinfo_install_script_new;
>>> osinfo_install_script_new_data;
>>> @@ -271,6 +272,8 @@ LIBOSINFO_0.2.0 {
>>> osinfo_install_script_generate;
>>> osinfo_install_script_generate_async;
>>> osinfo_install_script_generate_finish;
>>> + osinfo_install_script_generate_output;
>>> + osinfo_install_script_generate_output_async;
>>> osinfo_install_script_get_profile;
>>> osinfo_install_script_get_uri;
>>> osinfo_install_scriptlist_new;
>>> diff --git a/osinfo/osinfo_install_script.c b/osinfo/osinfo_install_script.c
>>> index 1f437a1..2d0b440 100644
>>> --- a/osinfo/osinfo_install_script.c
>>> +++ b/osinfo/osinfo_install_script.c
>>> @@ -46,7 +46,7 @@ G_DEFINE_TYPE (OsinfoInstallScript, osinfo_install_script, OSINFO_TYPE_ENTITY);
>>>
>>> struct _OsinfoInstallScriptPrivate
>>> {
>>> - gboolean unused;
>>> + gchar *output_prefix;
>>> };
>>>
>>> enum {
>>> @@ -141,6 +141,9 @@ osinfo_os_get_property(GObject *object,
>>> static void
>>> osinfo_install_script_finalize (GObject *object)
>>> {
>>> + OsinfoInstallScript *script = OSINFO_INSTALL_SCRIPT (object);
>>> + g_free(script->priv->output_prefix);
>>> +
>>> /* Chain up to the parent class */
>>> G_OBJECT_CLASS (osinfo_install_script_parent_class)->finalize (object);
>>> }
>>> @@ -219,6 +222,7 @@ osinfo_install_script_init (OsinfoInstallScript *list)
>>> OsinfoInstallScriptPrivate *priv;
>>> list->priv = priv = OSINFO_INSTALL_SCRIPT_GET_PRIVATE(list);
>>>
>>> + list->priv->output_prefix = NULL;
>>
>> This is redundant. This struct is zeroed for you.
>
> It's not an error, it's just be careful in excess. :)
> Where am I zeroing the struct? Should I remove this? Is not preferable
> to avoid an error if the user doesn't zero the struct?
There is no error possiblities here. All structs allocated by glib for
you are also zeroed out for you.
>>> }
>>>
>>>
>>> @@ -291,7 +295,25 @@ const gchar *osinfo_install_script_get_product_key_format(OsinfoInstallScript *s
>>> OSINFO_INSTALL_SCRIPT_PROP_PRODUCT_KEY_FORMAT);
>>> }
>>>
>>> -struct OsinfoInstallScriptGenerate {
>>> +void osinfo_install_script_set_output_prefix(OsinfoInstallScript *script,
>>> + const gchar *prefix)
>>> +{
>>> + g_free(script->priv->output_prefix);
>>> + script->priv->output_prefix = g_strdup(prefix);
>>> +}
>>> +
>>> +const gchar *osinfo_install_script_get_output_prefix(OsinfoInstallScript *script)
>>> +{
>>> + return script->priv->output_prefix;
>>> +}
>>> +
>>> +static const gchar *osinfo_install_script_get_output_filename(OsinfoInstallScript *script)
>>> +{
>>> + return osinfo_entity_get_param_value(OSINFO_ENTITY(script),
>>> + OSINFO_INSTALL_SCRIPT_PROP_OUTPUT_FILENAME);
>>> +}
>>> +
>>> +struct OsinfoInstallScriptGenerateData {
>>> GSimpleAsyncResult *res;
>>> OsinfoOs *os;
>>> OsinfoInstallConfig *config;
>>> @@ -299,7 +321,7 @@ struct OsinfoInstallScriptGenerate {
>>> };
>>>
>>>
>>> -static void osinfo_install_script_generate_free(struct OsinfoInstallScriptGenerate *data)
>>> +static void osinfo_install_script_generate_free(struct OsinfoInstallScriptGenerateData *data)
>>
>> * This should be a separate change as it has nothing to do with this
>> patch's goal (and before this patch). Same goes for all other hunks
>> for this renaming..
>> * the free function needs to be renamed too:
>> osinfo_install_script_generate_data_free
>>
>>> {
>>> g_object_unref(data->os);
>>> g_object_unref(data->config);
>>> @@ -308,6 +330,24 @@ static void osinfo_install_script_generate_free(struct OsinfoInstallScriptGenera
>>> g_free(data);
>>> }
>>>
>>> +struct OsinfoInstallScriptGenerateOutputData {
>>> + GSimpleAsyncResult *res;
>>> + GCancellable *cancellable;
>>> + GError *error;
>>> + GFile *file;
>>> + GFileOutputStream *stream;
>>> + gchar *output;
>>> + gssize output_len;
>>> + gssize output_pos;
>>> +};
>>> +
>>> +static void osinfo_install_script_generate_output_free(struct OsinfoInstallScriptGenerateOutputData *data)
>>> +{
>>> + g_object_unref(data->stream);
>>> + g_object_unref(data->res);
>>> + g_free(data);
>>> +}
>>> +
>>>
>>> static xsltStylesheetPtr osinfo_install_script_load_template(const gchar *uri,
>>> const gchar *template,
>>> @@ -530,7 +570,7 @@ static void osinfo_install_script_template_loaded(GObject *src,
>>> GAsyncResult *res,
>>> gpointer user_data)
>>> {
>>> - struct OsinfoInstallScriptGenerate *data = user_data;
>>> + struct OsinfoInstallScriptGenerateData *data = user_data;
>>> GError *error = NULL;
>>> gchar *input = NULL;
>>> gchar *output = NULL;
>>> @@ -579,7 +619,7 @@ void osinfo_install_script_generate_async(OsinfoInstallScript *script,
>>> GAsyncReadyCallback callback,
>>> gpointer user_data)
>>> {
>>> - struct OsinfoInstallScriptGenerate *data = g_new0(struct OsinfoInstallScriptGenerate, 1);
>>> + struct OsinfoInstallScriptGenerateData *data = g_new0(struct OsinfoInstallScriptGenerateData, 1);
>>> const gchar *templateData = osinfo_install_script_get_template_data(script);
>>> const gchar *templateUri = osinfo_install_script_get_template_uri(script);
>>>
>>> @@ -621,9 +661,9 @@ void osinfo_install_script_generate_async(OsinfoInstallScript *script,
>>> }
>>> }
>>>
>>> -gchar *osinfo_install_script_generate_finish(OsinfoInstallScript *script,
>>> - GAsyncResult *res,
>>> - GError **error)
>>> +static gpointer osinfo_install_script_generate_finish_common(OsinfoInstallScript *script,
>>> + GAsyncResult *res,
>>> + GError **error)
>>> {
>>> GSimpleAsyncResult *simple = G_SIMPLE_ASYNC_RESULT(res);
>>>
>>> @@ -635,13 +675,61 @@ gchar *osinfo_install_script_generate_finish(OsinfoInstallScript *script,
>>> return g_simple_async_result_get_op_res_gpointer(simple);
>>> }
>>>
>>> +gchar *osinfo_install_script_generate_finish(OsinfoInstallScript *script,
>>> + GAsyncResult *res,
>>> + GError **error)
>>> +{
>>> + return osinfo_install_script_generate_finish_common(script,
>>> + res,
>>> + error);
>>> +}
>>> +
>>> +GFile *osinfo_install_script_generate_output_finish(OsinfoInstallScript *script,
>>> + GAsyncResult *res,
>>> + GError **error)
>>> +{
>>> + return osinfo_install_script_generate_finish_common(script,
>>> + res,
>>> + error);
>>> +}
>>>
>>> struct OsinfoInstallScriptGenerateSync {
>>> GMainLoop *loop;
>>> GError *error;
>>> gchar *output;
>>> + GFile *file;
>>> };
>>>
>>> +static void osinfo_install_script_generate_output_done(GObject *src,
>>> + GAsyncResult *res,
>>> + gpointer user_data)
>>> +{
>>> + struct OsinfoInstallScriptGenerateSync *data = user_data;
>>> +
>>> + data->file =
>>> + osinfo_install_script_generate_output_finish(OSINFO_INSTALL_SCRIPT(src),
>>> + res,
>>> + &data->error);
>>> + g_main_loop_quit(data->loop);
>>> +}
>>> +
>>> +static void osinfo_install_script_generate_output_close_file(GObject *src,
>>> + GAsyncResult *res,
>>> + gpointer user_data)
>>> +{
>>> + struct OsinfoInstallScriptGenerateOutputData *data = user_data;
>>> +
>>> + g_output_stream_close_finish(G_OUTPUT_STREAM(src),
>>> + res,
>>> + &data->error);
>>> +
>>> + g_simple_async_result_set_op_res_gpointer(data->res,
>>> + data->file, NULL);
>>> + g_simple_async_result_complete_in_idle(data->res);
>>> +
>>> + osinfo_install_script_generate_output_free(data);
>>> +}
>>> +
>>> static void osinfo_install_script_generate_done(GObject *src,
>>> GAsyncResult *res,
>>> gpointer user_data)
>>> @@ -655,7 +743,6 @@ static void osinfo_install_script_generate_done(GObject *src,
>>> g_main_loop_quit(data->loop);
>>> }
>>>
>>> -
>>> gchar *osinfo_install_script_generate(OsinfoInstallScript *script,
>>> OsinfoOs *os,
>>> OsinfoInstallConfig *config,
>>> @@ -665,7 +752,7 @@ gchar *osinfo_install_script_generate(OsinfoInstallScript *script,
>>> GMainLoop *loop = g_main_loop_new(g_main_context_get_thread_default(),
>>> TRUE);
>>> struct OsinfoInstallScriptGenerateSync data = {
>>> - loop, NULL, NULL
>>> + loop, NULL, NULL, NULL
>>
>> If all you need to do is to pass the main loop, you should pass that
>> as the data pointer and no need to have a separate struct.
>
> We are keeping the result (the GFile * or the gchar * that contains
> the script) in this struct as well.
> Should I remove?
Yeah I don't see any need as you get the result when you call the
finalize in this same function.
--
Regards,
Zeeshan Ali (Khattak)
FSF member#5124
