From 914b841481b29d326ccd534ad7f9e635a0d61ea3 Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Wed, 8 Jun 2016 23:48:51 -0400 Subject: [PATCH 1/6] module: Be a bit more robust When we call builder_module_cleanup_collect on a module that has no changes, we shouldn't crash. --- builder/builder-module.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/builder/builder-module.c b/builder/builder-module.c index 4c676b10..f4ef4710 100644 --- a/builder/builder-module.c +++ b/builder/builder-module.c @@ -1464,6 +1464,9 @@ builder_module_cleanup_collect (BuilderModule *self, const char **global_patterns; const char **local_patterns; + if (!self->changes) + return; + if (platform) { global_patterns = builder_context_get_global_cleanup_platform (context); From 9a8eef8597b8f0223d70be1b2571e65b70de641e Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Wed, 8 Jun 2016 23:53:03 -0400 Subject: [PATCH 2/6] manifest: Skip source-less modules when building Building such modules won't produce anything. And future commits will make source-less modules useful to support. --- builder/builder-manifest.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/builder/builder-manifest.c b/builder/builder-manifest.c index 34dd4cb2..ae82b714 100644 --- a/builder/builder-manifest.c +++ b/builder/builder-manifest.c @@ -1105,6 +1105,12 @@ builder_manifest_build (BuilderManifest *self, g_autofree char *stage = g_strdup_printf ("build-%s", builder_module_get_name (m)); + if (!builder_module_get_sources (m)) + { + g_print ("Skipping module %s (no sources)\n", builder_module_get_name (m)); + continue; + } + builder_module_checksum (m, cache, context); if (!builder_cache_lookup (cache, stage)) From 2757c63fea4c3ec66a53ebea110210af94613bc1 Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Wed, 8 Jun 2016 23:38:33 -0400 Subject: [PATCH 3/6] Add a modules property to BuilderModule This will let us load modules recursively. --- builder/builder-module.c | 94 +++++++++++++++++++++++++++++++++++++++- builder/builder-module.h | 1 + 2 files changed, 93 insertions(+), 2 deletions(-) diff --git a/builder/builder-module.c b/builder/builder-module.c index f4ef4710..b18e412f 100644 --- a/builder/builder-module.c +++ b/builder/builder-module.c @@ -57,6 +57,7 @@ struct BuilderModule char **cleanup; char **cleanup_platform; GList *sources; + GList *modules; }; typedef struct @@ -88,6 +89,7 @@ enum { PROP_CLEANUP, PROP_CLEANUP_PLATFORM, PROP_POST_INSTALL, + PROP_MODULES, LAST_PROP }; @@ -107,6 +109,7 @@ builder_module_finalize (GObject *object) g_list_free_full (self->sources, g_object_unref); g_strfreev (self->cleanup); g_strfreev (self->cleanup_platform); + g_list_free_full (self->modules, g_object_unref); if (self->changes) g_ptr_array_unref (self->changes); @@ -192,6 +195,10 @@ builder_module_get_property (GObject *object, g_value_set_boxed (value, self->cleanup_platform); break; + case PROP_MODULES: + g_value_set_pointer (value, self->modules); + break; + default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); } @@ -296,6 +303,12 @@ builder_module_set_property (GObject *object, g_strfreev (tmp); break; + case PROP_MODULES: + g_list_free_full (self->modules, g_object_unref); + /* NOTE: This takes ownership of the list! */ + self->modules = g_value_get_pointer (value); + break; + default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); } @@ -428,6 +441,12 @@ builder_module_class_init (BuilderModuleClass *klass) "", G_TYPE_STRV, G_PARAM_READWRITE)); + g_object_class_install_property (object_class, + PROP_MODULES, + g_param_spec_pointer ("modules", + "", + "", + G_PARAM_READWRITE)); } static void @@ -441,7 +460,31 @@ builder_module_serialize_property (JsonSerializable *serializable, const GValue *value, GParamSpec *pspec) { - if (strcmp (property_name, "sources") == 0) + if (strcmp (property_name, "modules") == 0) + { + BuilderModule *self = BUILDER_MODULE (serializable); + JsonNode *retval = NULL; + GList *l; + + if (self->modules) + { + JsonArray *array; + + array = json_array_sized_new (g_list_length (self->modules)); + + for (l = self->modules; l != NULL; l = l->next) + { + JsonNode *child = json_gobject_serialize (l->data); + json_array_add_element (array, child); + } + + retval = json_node_init_array (json_node_alloc (), array); + json_array_unref (array); + } + + return retval; + } + else if (strcmp (property_name, "sources") == 0) { BuilderModule *self = BUILDER_MODULE (serializable); JsonNode *retval = NULL; @@ -481,7 +524,48 @@ builder_module_deserialize_property (JsonSerializable *serializable, GParamSpec *pspec, JsonNode *property_node) { - if (strcmp (property_name, "sources") == 0) + if (strcmp (property_name, "modules") == 0) + { + if (JSON_NODE_TYPE (property_node) == JSON_NODE_NULL) + { + g_value_set_pointer (value, NULL); + return TRUE; + } + else if (JSON_NODE_TYPE (property_node) == JSON_NODE_ARRAY) + { + JsonArray *array = json_node_get_array (property_node); + guint i, array_len = json_array_get_length (array); + GList *modules = NULL; + GObject *module; + + for (i = 0; i < array_len; i++) + { + JsonNode *element_node = json_array_get_element (array, i); + + if (JSON_NODE_TYPE (element_node) != JSON_NODE_OBJECT) + { + g_list_free_full (modules, g_object_unref); + return FALSE; + } + + module = json_gobject_deserialize (BUILDER_TYPE_MODULE, element_node); + if (module == NULL) + { + g_list_free_full (modules, g_object_unref); + return FALSE; + } + + modules = g_list_prepend (modules, module); + } + + g_value_set_pointer (value, g_list_reverse (modules)); + + return TRUE; + } + + return FALSE; + } + else if (strcmp (property_name, "sources") == 0) { if (JSON_NODE_TYPE (property_node) == JSON_NODE_NULL) { @@ -556,6 +640,12 @@ builder_module_get_sources (BuilderModule *self) return self->sources; } +GList * +builder_module_get_modules (BuilderModule *self) +{ + return self->modules; +} + gboolean builder_module_download_sources (BuilderModule *self, gboolean update_vcs, diff --git a/builder/builder-module.h b/builder/builder-module.h index 0adc3fb3..2c6a10b8 100644 --- a/builder/builder-module.h +++ b/builder/builder-module.h @@ -42,6 +42,7 @@ GType builder_module_get_type (void); const char * builder_module_get_name (BuilderModule *self); gboolean builder_module_get_disabled (BuilderModule *self); GList * builder_module_get_sources (BuilderModule *self); +GList * builder_module_get_modules (BuilderModule *self); GPtrArray * builder_module_get_changes (BuilderModule *self); void builder_module_set_changes (BuilderModule *self, GPtrArray *changes); From e25e379a2a2e199c15e0229a9b0f55257f47df03 Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Wed, 8 Jun 2016 23:39:20 -0400 Subject: [PATCH 4/6] manifest: expand module list Linearize the tree of modules and submodules when the modules are set on the manifest, while filtering out disabled modules at the same time. Skip source-less modules when building; this makes it possible to have modules that only contain submodules. With this approach, we use the tree structure of modules for serializing and deserializing to and from json, while using a linear list of modules for building. --- builder/builder-manifest.c | 51 +++++++++++++++++++++++--------------- 1 file changed, 31 insertions(+), 20 deletions(-) diff --git a/builder/builder-manifest.c b/builder/builder-manifest.c index ae82b714..31da49fe 100644 --- a/builder/builder-manifest.c +++ b/builder/builder-manifest.c @@ -70,6 +70,7 @@ struct BuilderManifest char *command; BuilderOptions *build_options; GList *modules; + GList *expanded_modules; }; typedef struct @@ -134,6 +135,7 @@ builder_manifest_finalize (GObject *object) g_free (self->command); g_clear_object (&self->build_options); g_list_free_full (self->modules, g_object_unref); + g_list_free (self->expanded_modules); g_strfreev (self->cleanup); g_strfreev (self->cleanup_commands); g_strfreev (self->cleanup_platform); @@ -148,6 +150,28 @@ builder_manifest_finalize (GObject *object) G_OBJECT_CLASS (builder_manifest_parent_class)->finalize (object); } +static GList * +expand_modules (GList *modules) +{ + GList *l; + GList *expanded = NULL; + + for (l = modules; l; l = l->next) + { + BuilderModule *m = l->data; + GList *submodules; + + if (builder_module_get_disabled (m)) + continue; + + submodules = expand_modules (builder_module_get_modules (m)); + expanded = g_list_concat (expanded, submodules); + expanded = g_list_append (expanded, m); + } + + return expanded; +} + static void builder_manifest_get_property (GObject *object, guint prop_id, @@ -357,6 +381,8 @@ builder_manifest_set_property (GObject *object, g_list_free_full (self->modules, g_object_unref); /* NOTE: This takes ownership of the list! */ self->modules = g_value_get_pointer (value); + g_list_free (self->expanded_modules); + self->expanded_modules = expand_modules (self->modules); break; case PROP_CLEANUP: @@ -995,7 +1021,7 @@ builder_manifest_checksum_for_cleanup (BuilderManifest *self, builder_cache_checksum_str (cache, self->desktop_file_name_suffix); builder_cache_checksum_boolean (cache, self->appstream_compose); - for (l = self->modules; l != NULL; l = l->next) + for (l = self->expanded_modules; l != NULL; l = l->next) { BuilderModule *m = l->data; builder_module_checksum_for_cleanup (m, cache, context); @@ -1063,13 +1089,10 @@ builder_manifest_download (BuilderManifest *self, GList *l; g_print ("Downloading sources\n"); - for (l = self->modules; l != NULL; l = l->next) + for (l = self->expanded_modules; l != NULL; l = l->next) { BuilderModule *m = l->data; - if (builder_module_get_disabled (m)) - continue; - if (!builder_module_download_sources (m, update_vcs, context, error)) return FALSE; } @@ -1092,17 +1115,11 @@ builder_manifest_build (BuilderManifest *self, builder_context_set_separate_locales (context, self->separate_locales); g_print ("Starting build of %s\n", self->id ? self->id : "app"); - for (l = self->modules; l != NULL; l = l->next) + for (l = self->expanded_modules; l != NULL; l = l->next) { BuilderModule *m = l->data; g_autoptr(GPtrArray) changes = NULL; - if (builder_module_get_disabled (m)) - { - g_print ("Skipping disabled module %s\n", builder_module_get_name (m)); - continue; - } - g_autofree char *stage = g_strdup_printf ("build-%s", builder_module_get_name (m)); if (!builder_module_get_sources (m)) @@ -1392,13 +1409,10 @@ builder_manifest_cleanup (BuilderManifest *self, } } - for (l = self->modules; l != NULL; l = l->next) + for (l = self->expanded_modules; l != NULL; l = l->next) { BuilderModule *m = l->data; - if (builder_module_get_disabled (m)) - continue; - builder_module_cleanup_collect (m, FALSE, context, to_remove_ht); } @@ -1870,13 +1884,10 @@ builder_manifest_create_platform (BuilderManifest *self, return FALSE; } - for (l = self->modules; l != NULL; l = l->next) + for (l = self->expanded_modules; l != NULL; l = l->next) { BuilderModule *m = l->data; - if (builder_module_get_disabled (m)) - continue; - builder_module_cleanup_collect (m, TRUE, context, to_remove_ht); } From 5225e7b1d0e70433ef27179222278cc13a437c12 Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Thu, 9 Jun 2016 00:29:02 -0400 Subject: [PATCH 5/6] manifest: Prepare expand_modules to report errors Move the expand_modules call from the modules setter to builder_manifest_start, where we have a chance to report errors. --- builder/builder-manifest.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/builder/builder-manifest.c b/builder/builder-manifest.c index 31da49fe..02d29064 100644 --- a/builder/builder-manifest.c +++ b/builder/builder-manifest.c @@ -150,26 +150,27 @@ builder_manifest_finalize (GObject *object) G_OBJECT_CLASS (builder_manifest_parent_class)->finalize (object); } -static GList * -expand_modules (GList *modules) +static gboolean +expand_modules (GList *modules, GList **expanded, GError **error) { GList *l; - GList *expanded = NULL; for (l = modules; l; l = l->next) { BuilderModule *m = l->data; - GList *submodules; + GList *submodules = NULL; if (builder_module_get_disabled (m)) continue; - submodules = expand_modules (builder_module_get_modules (m)); - expanded = g_list_concat (expanded, submodules); - expanded = g_list_append (expanded, m); + if (!expand_modules (builder_module_get_modules (m), &submodules, error)) + return FALSE; + + *expanded = g_list_concat (*expanded, submodules); + *expanded = g_list_append (*expanded, m); } - return expanded; + return TRUE; } static void @@ -381,8 +382,6 @@ builder_manifest_set_property (GObject *object, g_list_free_full (self->modules, g_object_unref); /* NOTE: This takes ownership of the list! */ self->modules = g_value_get_pointer (value); - g_list_free (self->expanded_modules); - self->expanded_modules = expand_modules (self->modules); break; case PROP_CLEANUP: @@ -893,6 +892,9 @@ builder_manifest_start (BuilderManifest *self, self->runtime, builder_manifest_get_runtime_version (self)); + if (!expand_modules (self->modules, &self->expanded_modules, error)) + return FALSE; + return TRUE; } From 99e4f83e332ffec056c9704b8c9f67648cba4f1f Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Thu, 9 Jun 2016 01:01:37 -0400 Subject: [PATCH 6/6] Allow inclusions in the module list When we see a string in the modules array, parse it as a json file and use the resulting BuilderModule object. --- builder/builder-manifest.c | 20 ++++++++++++++------ builder/builder-module.c | 20 ++++++++++++++------ 2 files changed, 28 insertions(+), 12 deletions(-) diff --git a/builder/builder-manifest.c b/builder/builder-manifest.c index 02d29064..afb11a66 100644 --- a/builder/builder-manifest.c +++ b/builder/builder-manifest.c @@ -767,13 +767,21 @@ builder_manifest_deserialize_property (JsonSerializable *serializable, { JsonNode *element_node = json_array_get_element (array, i); - if (JSON_NODE_TYPE (element_node) != JSON_NODE_OBJECT) - { - g_list_free_full (modules, g_object_unref); - return FALSE; - } + module = NULL; + + if (JSON_NODE_HOLDS_VALUE (element_node) && + json_node_get_value_type (element_node) == G_TYPE_STRING) + { + const char *module_path = json_node_get_string (element_node); + g_autofree char *json = NULL; + + if (g_file_get_contents (module_path, &json, NULL, NULL)) + module = json_gobject_from_data (BUILDER_TYPE_MODULE, + json, -1, NULL); + } + else if (JSON_NODE_HOLDS_OBJECT (element_node)) + module = json_gobject_deserialize (BUILDER_TYPE_MODULE, element_node); - module = json_gobject_deserialize (BUILDER_TYPE_MODULE, element_node); if (module == NULL) { g_list_free_full (modules, g_object_unref); diff --git a/builder/builder-module.c b/builder/builder-module.c index b18e412f..11e03b86 100644 --- a/builder/builder-module.c +++ b/builder/builder-module.c @@ -542,13 +542,21 @@ builder_module_deserialize_property (JsonSerializable *serializable, { JsonNode *element_node = json_array_get_element (array, i); - if (JSON_NODE_TYPE (element_node) != JSON_NODE_OBJECT) - { - g_list_free_full (modules, g_object_unref); - return FALSE; - } + module = NULL; + + if (JSON_NODE_HOLDS_VALUE (element_node) && + json_node_get_value_type (element_node) == G_TYPE_STRING) + { + const char *module_path = json_node_get_string (element_node); + g_autofree char *json = NULL; + + if (g_file_get_contents (module_path, &json, NULL, NULL)) + module = json_gobject_from_data (BUILDER_TYPE_MODULE, + json, -1, NULL); + } + else if (JSON_NODE_HOLDS_OBJECT (element_node)) + module = json_gobject_deserialize (BUILDER_TYPE_MODULE, element_node); - module = json_gobject_deserialize (BUILDER_TYPE_MODULE, element_node); if (module == NULL) { g_list_free_full (modules, g_object_unref);