GNOME Bugzilla – Bug 779891
work around recent vala breakages
Last modified: 2017-03-11 10:12:28 UTC
While not breaking the syntax, vala's type-check pass broke nearly all of our vala code that implemented libide plugin types. We can work around this if we drop all the "context" property definitions in our plugin interfaces, switch them to using IdeObject as a prerequisite, and then work around libpeas by doing our own GParameter conversion.
Created attachment 347681 [details] [review] libide: relax context property requirements We don't need to require IdeObject for a bunch of things, we can just rely on GObject + the context property. This will let us work around some language breaking checks in Vala.
Created attachment 347682 [details] [review] vala: work around Vala language breaks Commit e874bb7902cc06f9f6d4427d99ec33e3757304e4 broke a bunch of inheritance in vala (that wasn't very good to begin with, but at least it worked). In particular, Vala has a very hard time determining if an object properly meets the interface requirements. It does not handle relaxed write checks vs the parent very well (and has strange requirements for vfuncs to be available that are completely unnecessary). However, we can stop inheriting from Ide.Object and implement the context property ourselves to work around the issue. Incredibly annoying, but good enough to fix the build a week before release.
Created attachment 347683 [details] [review] object: add vfuncs for getter/setter and use them Chain up here instead of doing our funky only chain-up after setting phase. This should make things more predictable for subclasses that need to do anything funny, and Vala.
Created attachment 347684 [details] [review] unsaved-files: chain up to parent Now that IdeObject changed semantics, this needs to chain up.
Created attachment 347685 [details] [review] plugins: add GParameter helper Libpeas is not sufficient to extract GParameters that we need because we also care about the base objects in the prerequisites, not just interfaces. This is a sort of barebones implementation of that. Scan the interfaces chain, and then the objects that are pre-reqs. If we find the property, all good.
Created attachment 347686 [details] [review] plugins: use ide_extension_new and ide_extension_set_new Instead of using peas for creating the extension sets, use our wrapper so that we can handle the case where the property comes from the pre-requisite class instead of the interface.
Created attachment 347687 [details] [review] service: drop context property from IdeService Now that IdeService can have :context fulfilled from IdeObject when creating plugins, we can simplify this code.
Created attachment 347688 [details] [review] context: use ide_extension_set_new() for creating services This allows us to locate :context from the IdeObject class when instantiating plugins.
Created attachment 347689 [details] [review] device-manager: use ide_extension_set_new() This allows locating :context from IdeObject when instantiating plugins.
Created attachment 347690 [details] [review] rename-provider: drop :context property We get this from IdeObject and now plugins can use that instead of requiring it in the interface.
Created attachment 347691 [details] [review] indenter: drop :context property We can get :context from the IdeObject class now.
Created attachment 347692 [details] [review] symbol-resolver: drop :context from the symbol resolver We get this from IdeObject now, so we don't need to specify it in the interface as long as we keep using ide_extension_new()/etc.
Created attachment 347693 [details] [review] diagnostics: drop :context property We can drop this now as the context can be found from the base object.
Created attachment 347694 [details] [review] vala: indenter no longer requires context
Created attachment 347695 [details] [review] completion: drop use of :context and add load vfunc Instead of requiring the :context property (which causes weird conflicts with vala), use a load vfunc and ensure it gets used.
Created attachment 347696 [details] [review] pipeline-addin: drop :context property We get this from IdeObject, so we don't need to enforce the property directly.
Created attachment 347697 [details] [review] ctags: use load vfunc to initialize context Now that we cannot guarantee that :context will be set before constructed we need to use the load vfunc to attach things. This is much better/safer anyway.
Created attachment 347698 [details] [review] search: drop use of :context property We can inherit this from IdeObject now and still have it attached to plugins at creation time.
This works around the issue as the summary describes. We reduce our use of peas_extension_set_new() and peas_engine_create_extension() in favor of using our own wrapper to build GParameter lists which cna resolve properties from pre-requisite types in addition to the interfaces. Now we can drop all those (*set_context) hacks that were in place for Vala and avoid vala bug 779038 which also broke git master (without a way to workaround because the workaround caused symbol name conflicts with multiple functions in generated C code to have the same name). This needs lots of testing being that we are a week away from release. Attachment 347681 [details] pushed as 1e97f09 - libide: relax context property requirements Attachment 347682 [details] pushed as 47f1172 - vala: work around Vala language breaks Attachment 347683 [details] pushed as 6153940 - object: add vfuncs for getter/setter and use them Attachment 347684 [details] pushed as 7480f87 - unsaved-files: chain up to parent Attachment 347685 [details] pushed as 08310b6 - plugins: add GParameter helper Attachment 347686 [details] pushed as 48f0e18 - plugins: use ide_extension_new and ide_extension_set_new Attachment 347687 [details] pushed as eb7bc79 - service: drop context property from IdeService Attachment 347688 [details] pushed as f20f3e4 - context: use ide_extension_set_new() for creating services Attachment 347689 [details] pushed as f09c365 - device-manager: use ide_extension_set_new() Attachment 347690 [details] pushed as be18668 - rename-provider: drop :context property Attachment 347691 [details] pushed as 3bb3e44 - indenter: drop :context property Attachment 347692 [details] pushed as b319b5d - symbol-resolver: drop :context from the symbol resolver Attachment 347693 [details] pushed as 0cef1bb - diagnostics: drop :context property Attachment 347694 [details] pushed as 252a71f - vala: indenter no longer requires context Attachment 347695 [details] pushed as 7cd2a99 - completion: drop use of :context and add load vfunc Attachment 347696 [details] pushed as 6f80f47 - pipeline-addin: drop :context property Attachment 347697 [details] pushed as 22d2c01 - ctags: use load vfunc to initialize context Attachment 347698 [details] pushed as 644df22 - search: drop use of :context property