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 779891 - work around recent vala breakages
work around recent vala breakages
Status: RESOLVED FIXED
Product: gnome-builder
Classification: Other
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: GNOME Builder Maintainers
GNOME Builder Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-03-11 10:04 UTC by Christian Hergert
Modified: 2017-03-11 10:12 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
libide: relax context property requirements (5.34 KB, patch)
2017-03-11 10:05 UTC, Christian Hergert
committed Details | Review
vala: work around Vala language breaks (9.03 KB, patch)
2017-03-11 10:05 UTC, Christian Hergert
committed Details | Review
object: add vfuncs for getter/setter and use them (3.14 KB, patch)
2017-03-11 10:05 UTC, Christian Hergert
committed Details | Review
unsaved-files: chain up to parent (911 bytes, patch)
2017-03-11 10:05 UTC, Christian Hergert
committed Details | Review
plugins: add GParameter helper (8.09 KB, patch)
2017-03-11 10:05 UTC, Christian Hergert
committed Details | Review
plugins: use ide_extension_new and ide_extension_set_new (3.59 KB, patch)
2017-03-11 10:05 UTC, Christian Hergert
committed Details | Review
service: drop context property from IdeService (3.77 KB, patch)
2017-03-11 10:05 UTC, Christian Hergert
committed Details | Review
context: use ide_extension_set_new() for creating services (1.66 KB, patch)
2017-03-11 10:05 UTC, Christian Hergert
committed Details | Review
device-manager: use ide_extension_set_new() (1.52 KB, patch)
2017-03-11 10:06 UTC, Christian Hergert
committed Details | Review
rename-provider: drop :context property (2.03 KB, patch)
2017-03-11 10:06 UTC, Christian Hergert
committed Details | Review
indenter: drop :context property (2.36 KB, patch)
2017-03-11 10:06 UTC, Christian Hergert
committed Details | Review
symbol-resolver: drop :context from the symbol resolver (4.81 KB, patch)
2017-03-11 10:06 UTC, Christian Hergert
committed Details | Review
diagnostics: drop :context property (5.77 KB, patch)
2017-03-11 10:06 UTC, Christian Hergert
committed Details | Review
vala: indenter no longer requires context (1.24 KB, patch)
2017-03-11 10:06 UTC, Christian Hergert
committed Details | Review
completion: drop use of :context and add load vfunc (4.60 KB, patch)
2017-03-11 10:06 UTC, Christian Hergert
committed Details | Review
pipeline-addin: drop :context property (4.89 KB, patch)
2017-03-11 10:06 UTC, Christian Hergert
committed Details | Review
ctags: use load vfunc to initialize context (4.42 KB, patch)
2017-03-11 10:06 UTC, Christian Hergert
committed Details | Review
search: drop use of :context property (2.79 KB, patch)
2017-03-11 10:06 UTC, Christian Hergert
committed Details | Review

Description Christian Hergert 2017-03-11 10:04:34 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.
Comment 1 Christian Hergert 2017-03-11 10:05:31 UTC
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.
Comment 2 Christian Hergert 2017-03-11 10:05:35 UTC
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.
Comment 3 Christian Hergert 2017-03-11 10:05:38 UTC
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.
Comment 4 Christian Hergert 2017-03-11 10:05:42 UTC
Created attachment 347684 [details] [review]
unsaved-files: chain up to parent

Now that IdeObject changed semantics, this needs to chain up.
Comment 5 Christian Hergert 2017-03-11 10:05:46 UTC
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.
Comment 6 Christian Hergert 2017-03-11 10:05:49 UTC
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.
Comment 7 Christian Hergert 2017-03-11 10:05:53 UTC
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.
Comment 8 Christian Hergert 2017-03-11 10:05:56 UTC
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.
Comment 9 Christian Hergert 2017-03-11 10:06:00 UTC
Created attachment 347689 [details] [review]
device-manager: use ide_extension_set_new()

This allows locating :context from IdeObject when instantiating plugins.
Comment 10 Christian Hergert 2017-03-11 10:06:04 UTC
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.
Comment 11 Christian Hergert 2017-03-11 10:06:08 UTC
Created attachment 347691 [details] [review]
indenter: drop :context property

We can get :context from the IdeObject class now.
Comment 12 Christian Hergert 2017-03-11 10:06:11 UTC
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.
Comment 13 Christian Hergert 2017-03-11 10:06:15 UTC
Created attachment 347693 [details] [review]
diagnostics: drop :context property

We can drop this now as the context can be found from the base object.
Comment 14 Christian Hergert 2017-03-11 10:06:18 UTC
Created attachment 347694 [details] [review]
vala: indenter no longer requires context
Comment 15 Christian Hergert 2017-03-11 10:06:23 UTC
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.
Comment 16 Christian Hergert 2017-03-11 10:06:27 UTC
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.
Comment 17 Christian Hergert 2017-03-11 10:06:31 UTC
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.
Comment 18 Christian Hergert 2017-03-11 10:06:35 UTC
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.
Comment 19 Christian Hergert 2017-03-11 10:11:34 UTC
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