After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 780628 - autotools: Mark chained stage as completed (or not)
autotools: Mark chained stage as completed (or not)
Status: RESOLVED FIXED
Product: gnome-builder
Classification: Other
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: GNOME Builder Maintainers
GNOME Builder Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-03-28 00:27 UTC by Matthew Leeds
Modified: 2017-03-29 18:41 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
autotools: Mark chained stage as completed (or not) (1.07 KB, patch)
2017-03-28 00:27 UTC, Matthew Leeds
needs-work Details | Review
pipeline: Mark chained stage as completed (or not) (3.68 KB, patch)
2017-03-29 04:39 UTC, Matthew Leeds
needs-work Details | Review

Description Matthew Leeds 2017-03-28 00:27:38 UTC
I'm not sure why this can't be done at a higher level in
ide_build_pipeline_try_chain() but maybe there's some situation
I'm not considering.
Comment 1 Matthew Leeds 2017-03-28 00:27:42 UTC
Created attachment 348849 [details] [review]
autotools: Mark chained stage as completed (or not)

This commit ensures that when two make stages are chained, the chained
stage gets marked as completed when the combined stage completes (and
back to not completed if it's invalidated).
Comment 2 Christian Hergert 2017-03-28 00:36:01 UTC
Review of attachment 348849 [details] [review]:

This isn't really safe to do unless you unbind after the operation. A plugin could insert a stage in-between if it is enabled after pipeline setup.
Comment 3 Matthew Leeds 2017-03-29 04:39:27 UTC
Created attachment 348914 [details] [review]
pipeline: Mark chained stage as completed (or not)

This commit ensures that when two make stages are chained, the chained
stage gets marked as completed when the combined stage completes (and
back to not completed if it's invalidated).
Comment 4 Christian Hergert 2017-03-29 17:35:14 UTC
Review of attachment 348914 [details] [review]:

A couple quick fixes and then this LGTM

::: libide/buildsystem/ide-build-pipeline.c
@@ +1109,3 @@
   g_array_set_clear_func (self->errfmts, clear_error_format);
 
+  self->chained_bindings = g_ptr_array_new_with_free_func (g_object_unref);

Instead of relying on "auto-unbinding" of the GBinding, instead use a custom destroy notify that does:

 g_binding_unbind (binding);
 g_object_unref (binding);

@@ -1176,3 +1191,3 @@
 
-      /*
-       * NOTE: We do not mark the chained stage as completed as that is left
+      chained_binding = g_object_bind_property (stage, "completed", entry->stage, "completed", 0);
+      g_ptr_array_add (self->chained_bindings, chained_binding);

The return value from g_object_bind_property() is not owned, so you need to g_object_ref(chained_binding).
Comment 5 Matthew Leeds 2017-03-29 18:41:21 UTC
Pushed as commit dd207ab91e893193d463fc89ebc29ade2db6db1f