-
Notifications
You must be signed in to change notification settings - Fork 76
[DRAFT] Port to girepository-2.0 #337
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
This is an initial port of lgi to use girepository-2.0 introduced in the GLib 2.80 release. If using 2.85, it will use the new gi_repository_dup_default() function to ensure the repository is using the same repository as applications and libraries such as libpeas. There is still some work to be done, as libpeas tests do not yet pass using this version of lgi.
5ae577d to
4683e39
Compare
Sorry, but I do not thing there is anyone around who can give you that. Personally, I only ever poked at specific bits and pieces when something failed / broke and hoped that Pavouk knew what was going on. And now I do not thing anyone is around who really understands "all the magic". Sorry. |
psychon
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I scrolled through this and commented what I noticed, but... dunno. I have not much clue about any of this.
Out of curiosity, what would happen if LGI just continued using gobject-introspection-1.0 instead of switching to girepository-2.0? What does this switch mean for any downstream users?
Also, you only touched meson.build. Is the Makefile still fine?
| repository = gi_repository_dup_default (); | ||
| #else | ||
| repository = gi_repository_new (); | ||
| #endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A quick google session suggests that these two function do very different things. I am still not sure about the details, but things sound complicated....
| luaL_getmetatable (L, LGI_GI_INFO); | ||
| lua_setmetatable (L, -2); | ||
| } | ||
| g_assert (GI_IS_BASE_INFO (info)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happened to the lua_pushnil() case?
| #if !GLIB_CHECK_VERSION(2, 30, 0) | ||
| /* Workaround for broken g_struct_info_get_size() for GValue, see | ||
| /* Workaround for broken gi_struct_info_get_size() for GValue, see | ||
| https://bugzilla.gnome.org/show_bug.cgi?id=657040 */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure that bug report is about broken gi_struct_info_get_size()? I am not sure about the history / timeline, but this seems weird.
|
|
||
| /* This is workaround method for broken | ||
| g_object_info_get_*_function_pointer() in GI 1.32.0. (see | ||
| gi_object_info_get_*_function_pointer() in GI 1.32.0. (see |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does gi_object_info_get_*_function_pointer() even exist in GI 1.32.0?
GNOME devs are forcing the migration. E.g. gEdit dropped (I hope temporarily) the support of Python plugins because of this. IMHO gir2 support in vital for the project to work together with newer GNOME projects, given that gir1 and gir2 can't coexist in one app. |
Since you asked for feedback, I'm gonna drop a few words. Do not judge it as criticism, but just as my sincere wishes. I see it as a good practice to move forward on dependent library versions. However, for such a stable library as LGI is, I'm not sure whether it is viable or not, it would be really nice to keep compatibility with girepository-1.0. From your edits so far, mostly renaming function calls, it seems like that with proper macros, it could be handled. What I mean is: #ifdef LGI_GIREPOSITORY_1_0
/* legacy code goes here */
#else
/* 2.0 code goes here */
#endifon each replacement, or define most of them on a shared header file and import it when needed. |
You could probably make the functions that now return boolean instead of -1 values do that. But I don't have the time/motivation to do so. I also don't really see the value in it. For the things that need |
I have this mostly fixed for libpeas for the 2.x branch, and will have it fixed for libpeas-1.x for GNOME 49. It does require an ABI break technically but should at least allow things to move forward. |
I would have to drop Lua support from libpeas-1.x and libpeas-2.x starting with GNOME 49 (releases in September).
I didn't touch the Makefile, mostly because I didn't even notice it was there. I want to be clear about my availability to push through on this though. I don't really have any work cycles to do any of this so this was me stealing about a day to work through some things. I can't commit to pushing forward on it any more than this. The side-effect will be, however, that if it doesn't happen, I have to drop Lua support for libpeas soon. |
vtrlx
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stumbled onto this PR and decided to test it as I have a GNOME app using this binding so I'm very interested in ensuring it can continue to be updated for GNOME 49. For the purpose of Adw/Gtk applications, all that's needed here is a single change which I've included in the review.
As an aside, while running an Adw/Gtk application with the aforementioned change, I get an error in the terminal every time I instantiate an object from Adw or Gtk:
(process:248015): GLib-GIRepository-CRITICAL **: 13:12:25.881: gi_base_info_unref: assertion 'GI_IS_BASE_INFO (info)' failed
Made some changes to how gi_base_info_unref is called from the LGI side but it didn't affect this message. Just noting here—but aside from this odd detail, my suggested change puts this PR into a working state for writing apps.
Co-authored-by: Victoria Lacroix <[email protected]>
This is an initial port of lgi to use girepository-2.0 introduced in the GLib 2.80 release.
If using 2.85, it will use the new gi_repository_dup_default() function to ensure the repository is using the same repository as applications and libraries such as libpeas.
There is still some work to be done, as libpeas tests do not yet pass using this version of lgi. But I wanted to get some feedback on things and some extra eyes.
In particular, I'm seeing this when running the libpeas unit tests.
Which has code like:
and presumably the failure is converting the
{}to the flags.Anyway, still trying to dig that out.