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. > } > > > @@ -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. > }; > > osinfo_install_script_generate_async(script, > @@ -686,6 +773,121 @@ gchar *osinfo_install_script_generate(OsinfoInstallScript *script, > return data.output; > } > > +static void osinfo_install_script_generate_output_write_file(GObject *src, > + GAsyncResult *res, > + gpointer user_data) > +{ > + struct OsinfoInstallScriptGenerateOutputData *data = user_data; > + > + if (data->stream == NULL) > + data->stream = g_file_replace_finish(G_FILE (src), res, &data->error); > + else > + data->output_pos += g_output_stream_write_finish(G_OUTPUT_STREAM(data->stream), > + res, > + &data->error); arguments preferably need to be aligned with the first. > + > + if (data->output_pos < data->output_len) { > + g_output_stream_write_async(G_OUTPUT_STREAM (data->stream), > + data->output + data->output_pos, > + data->output_len - data->output_pos, > + G_PRIORITY_DEFAULT, > + data->cancellable, > + osinfo_install_script_generate_output_write_file, > + data); Same with these arguments. > + > + } else { > + g_output_stream_close_async(G_OUTPUT_STREAM (data->stream), > + G_PRIORITY_DEFAULT, > + data->cancellable, > + osinfo_install_script_generate_output_close_file, > + data); And here. Finding and correcting the rest of unaligned arguments is left as home work. :) > + } > +} > + > +void osinfo_install_script_generate_output_async(OsinfoInstallScript *script, > + OsinfoOs *os, > + OsinfoInstallConfig *config, > + GFile *output_dir, > + GCancellable *cancellable, > + GAsyncReadyCallback callback, > + gpointer user_data) > +{ > + const gchar *filename; > + const gchar *prefix; > + struct OsinfoInstallScriptGenerateOutputData *data = > + g_new0(struct OsinfoInstallScriptGenerateOutputData, 1); I suggest declaring OsinfoInstallScriptGenerateOutputData as typedef so you don't have to write 'struct' everywhere: typdef struct _OsinfoInstallScriptGenerateOutputData struct OsinfoInstallScriptGenerateOutputData; struct _struct OsinfoInstallScriptGenerateOutputData { .. } > + struct OsinfoInstallScriptGenerateSync *data_sync = user_data; > + > + data->res = g_simple_async_result_new(G_OBJECT(script), > + callback, > + user_data, > + osinfo_install_script_generate_output_async); > + > + data->cancellable = cancellable; > + data->error = data_sync->error; > + data->output = osinfo_install_script_generate(script, > + os, > + config, > + cancellable, > + &data->error); > + data->output_pos = 0; > + data->output_len = strlen(data->output); > + > + prefix = > + osinfo_install_script_get_output_prefix(script); > + filename = > + osinfo_install_script_get_output_filename(script); > + > + if (prefix) { > + gchar *output_filename = g_strdup_printf("%s-%s", prefix, filename); > + data->file = g_file_get_child(output_dir, output_filename); > + g_free(output_filename); > + } else { > + data->file = g_file_get_child(output_dir, filename); > + } > + > + g_file_replace_async(data->file, > + NULL, > + TRUE, > + G_FILE_CREATE_NONE, > + G_PRIORITY_DEFAULT, > + cancellable, > + osinfo_install_script_generate_output_write_file, > + data); > +} > + > +GFile *osinfo_install_script_generate_output(OsinfoInstallScript *script, > + OsinfoOs *os, > + OsinfoInstallConfig *config, > + GFile *output_dir, > + GCancellable *cancellable, > + GError **error) > +{ > + GMainLoop *loop = g_main_loop_new(g_main_context_get_thread_default(), > + TRUE); > + struct OsinfoInstallScriptGenerateSync data = { > + loop, NULL, NULL, NULL > + }; Same here (no stuct needed). > + osinfo_install_script_generate_output_async(script, > + os, > + config, > + output_dir, > + cancellable, > + osinfo_install_script_generate_output_done, > + &data); > + > + if (g_main_loop_is_running(loop)) > + g_main_loop_run(loop); > + > + if (data.error) > + g_propagate_error(error, data.error); > + > + g_main_loop_unref(loop); > + > + return data.file; > +} > > /* > * Local variables: > diff --git a/osinfo/osinfo_install_script.h b/osinfo/osinfo_install_script.h > index 00740c2..440fd7e 100644 > --- a/osinfo/osinfo_install_script.h > +++ b/osinfo/osinfo_install_script.h > @@ -49,6 +49,8 @@ typedef struct _OsinfoInstallScriptPrivate OsinfoInstallScriptPrivate; > #define OSINFO_INSTALL_SCRIPT_PROP_TEMPLATE_DATA "template-data" > #define OSINFO_INSTALL_SCRIPT_PROP_PROFILE "profile" > #define OSINFO_INSTALL_SCRIPT_PROP_PRODUCT_KEY_FORMAT "product-key-format" > +#define OSINFO_INSTALL_SCRIPT_PROP_OUTPUT_FILENAME "output-filename" > + > > /* object */ > struct _OsinfoInstallScript > @@ -85,6 +87,10 @@ const gchar *osinfo_install_script_get_profile(OsinfoInstallScript *script); > > const gchar *osinfo_install_script_get_product_key_format(OsinfoInstallScript *script); > > +void osinfo_install_script_set_output_prefix(OsinfoInstallScript *script, const gchar *prefix); > + > +const gchar *osinfo_install_script_get_output_prefix(OsinfoInstallScript *script); > + > void osinfo_install_script_generate_async(OsinfoInstallScript *script, > OsinfoOs *os, > OsinfoInstallConfig *config, > @@ -102,6 +108,24 @@ gchar *osinfo_install_script_generate(OsinfoInstallScript *script, > GCancellable *cancellable, > GError **error); > > +void osinfo_install_script_generate_output_async(OsinfoInstallScript *script, > + OsinfoOs *os, > + OsinfoInstallConfig *config, > + GFile *output_dir, > + GCancellable *cancellable, > + GAsyncReadyCallback callback, > + gpointer user_data); > + > +GFile *osinfo_install_script_generate_output_finish(OsinfoInstallScript *script, > + GAsyncResult *res, > + GError **error); > + > +GFile *osinfo_install_script_generate_output(OsinfoInstallScript *script, > + OsinfoOs *os, > + OsinfoInstallConfig *config, > + GFile *output_dir, > + GCancellable *cancellable, > + GError **error); > > #endif /* __OSINFO_INSTALL_SCRIPT_H__ */ > /* > diff --git a/osinfo/osinfo_loader.c b/osinfo/osinfo_loader.c > index f89dee0..31e442e 100644 > --- a/osinfo/osinfo_loader.c > +++ b/osinfo/osinfo_loader.c > @@ -585,6 +585,15 @@ static void osinfo_loader_install_script(OsinfoLoader *loader, > value); > g_free(value); > > + value = osinfo_loader_string("string(./template/@filename)", ctxt, err); > + if (error_is_set(err)) > + goto error; > + if (value) > + osinfo_entity_set_param(OSINFO_ENTITY(installScript), > + OSINFO_INSTALL_SCRIPT_PROP_OUTPUT_FILENAME, > + value); > + g_free(value); > + > osinfo_db_add_install_script(loader->priv->db, installScript); > > return; > diff --git a/tools/osinfo-install-script.c b/tools/osinfo-install-script.c > index c6aeae6..3860887 100644 > --- a/tools/osinfo-install-script.c > +++ b/tools/osinfo-install-script.c > @@ -26,7 +26,9 @@ > #include <osinfo/osinfo.h> > #include <string.h> > > -static gchar *profile; > +static const gchar *profile; > +static const gchar *output_dir; > +static const gchar *prefix; > > static OsinfoInstallConfig *config; > > @@ -60,6 +62,10 @@ static GOptionEntry entries[] = > { > { "profile", 'p', 0, G_OPTION_ARG_STRING, (void*)&profile, > "Install script profile", NULL, }, > + { "output-dir", 'd', 0, G_OPTION_ARG_STRING, (void*)&output_dir, > + "Install script output directory", NULL, }, > + { "prefix", 'P', 0, G_OPTION_ARG_STRING, (void*)&prefix, > + "The output filename prefix", NULL, }, > { "config", 'c', 0, G_OPTION_ARG_CALLBACK, > handle_config, > "Set configuration parameter", "key=value" }, > @@ -103,48 +109,49 @@ static OsinfoOs *find_os(OsinfoDb *db, > static gboolean generate_script(OsinfoOs *os) > { > OsinfoInstallScriptList *scripts = osinfo_os_get_install_script_list(os); > - OsinfoInstallScriptList *jeosScripts; > + OsinfoInstallScriptList *profileScripts; > OsinfoFilter *filter; > OsinfoInstallScript *script; > gboolean ret = FALSE; > GError *error = NULL; > - gchar *data; > + GFile *dir = g_file_new_for_commandline_arg(output_dir ? output_dir : "."); > > filter = osinfo_filter_new(); > osinfo_filter_add_constraint(filter, > OSINFO_INSTALL_SCRIPT_PROP_PROFILE, > profile ? profile : > OSINFO_INSTALL_SCRIPT_PROFILE_JEOS); > + profileScripts = osinfo_install_scriptlist_new_filtered(scripts, > + filter); We don't camelCase variable names. profileScripts -> profile_scripts. > - jeosScripts = osinfo_install_scriptlist_new_filtered(scripts, > - filter); > - if (osinfo_list_get_length(OSINFO_LIST(jeosScripts)) != 1) { > + if (osinfo_list_get_length(OSINFO_LIST(profileScripts)) != 1) { > g_printerr("Cannot find any install script for profile '%s'\n", > profile ? profile : > OSINFO_INSTALL_SCRIPT_PROFILE_JEOS); > goto cleanup; > } > > - script = OSINFO_INSTALL_SCRIPT(osinfo_list_get_nth(OSINFO_LIST(jeosScripts), 0)); > - if (!(data = osinfo_install_script_generate(script, > - os, > - config, > - NULL, > - &error))) { > + script = OSINFO_INSTALL_SCRIPT(osinfo_list_get_nth(OSINFO_LIST(profileScripts), 0)); > + > + if (prefix) > + osinfo_install_script_set_output_prefix(script, prefix); > + > + if (osinfo_install_script_generate_output(script, > + os, > + config, > + dir, > + NULL, > + &error) == NULL) { > g_printerr("Unable to generate install script: %s\n", > error ? error->message : "unknown"); > goto cleanup; > } > > - g_print("%s\n", data); > - Pretty sure it was some previous patch of yours that added this so you want to squash this into that patch. > ret = TRUE; > > cleanup: > - g_free(data); > g_object_unref(scripts); > - g_object_unref(jeosScripts); > - g_object_unref(filter); > + g_object_unref(profileScripts); > return ret; > } > > @@ -197,7 +204,6 @@ gint main(gint argc, gchar **argv) > goto EXIT; > } > > - > if (!generate_script(os)) { > ret = -5; > goto EXIT; > -- > 1.7.11.2 > > _______________________________________________ > virt-tools-list mailing list > virt-tools-list@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/virt-tools-list -- Regards, Zeeshan Ali (Khattak) FSF member#5124