=== sergiusens_ is now known as sergiusens [06:41] good morning [06:42] good morning didrocks [06:43] salut jibel [06:50] didrocks, ça va ? === pstolowski|afk is now known as pstolowski [07:15] good morning desktopers [07:30] salut seb128, ça va jibel, et toi ? [07:30] lut didrocks, en forme ? [07:30] didrocks, cf #u-r [07:33] seb128: ça va, je reboot rapidement pour tester un snap (j'ai lu #u-r, urgh, mais bon, au moins, on sait comment gardinenner ce code) [07:33] gardienner? [07:33] * didrocks brb [07:34] at least, my snap fix works :) [07:34] seb128: need a hand for ubiquity? [07:35] so repeating here, since it's more of a desktop issue [07:35] the code does [07:35] if gnome_session: [07:35] manager = session.get_object('org.gnome.SessionManager', [07:35] '/org/gnome/SessionManager') [07:35] manager.RequestReboot() [07:35] so it's using dbus to talk directly to gnome-session [07:35] which gives [07:35] No such interface '(null)' on object at path /org/gnome/SessionManager [07:35] didrocks, any idea how that could lead to a (null) interface error? [07:36] I guess RequestReboot() is a method of the manager dbus object [07:37] k, I've the iso, booting to the live session, let's see [07:37] running d-feet currently [07:37] while updating the iso [07:38] I especially don't understand how that can be a new/ubiquity issue [07:38] if anything I would have say a gnome-session issue [07:38] but that component didn't change [07:38] yeah, not related to anything we changed… [07:38] session.get_object -> there is no iface parameter, correct? [07:39] only name and object path [07:40] `bus_name` : str [07:40] `object_path` : str [07:41] I can confirm that the code is buggy in a python3 interpreter on the live CD [07:41] as an user [07:41] not only on the live [07:41] on my session as well [07:41] so not ubiquity, sudio, weird env or anything [07:41] right [07:41] why did that start today? [07:41] jibel, when did you see that dialog working last? [07:41] hey seb128 didrocks [07:42] I wonder if it really started today [07:42] hey willcooke [07:42] hey willcooke [07:42] the puzzling part is that you normally say on which interface you have the method attached [07:42] I'm heading in to the office in a mo with my test laptop for x_nox to fix the hang on reboot of the live session [07:42] and here, we never did [07:42] great [07:46] didrocks, [07:46] $ gdbus call --session --dest org.gnome.SessionManager --object-path /org/gnome/SessionManager --method org.gnome.SessionManager.RequestReboot [07:46] gives a method not implemented on interface org.gnome.SessionManager [07:47] seb128: it works for me in a ipython3 [07:47] like: [07:47] I guess I'm doing something wrong :p [07:47] manager = session.get_object('org.gnome.SessionManager', [07:47] '/org/gnome/SessionManager') [07:47] iface = dbus.Interface(manager, "org.gnome.SessionManager") [07:47] iface.IsSessionRunning() [07:47] for instance [07:48] doesn't work with RequestReboot though??? [07:49] why IsSessionRunning, which is on the same object and interface works (while it doesn't work on "manager" along without an interface specified) and not RequestReboot… [07:51] CanShutdown() works as well, for instance (once you execute it against the interface) [07:51] isn't there some polkit thing which would hide the method? [07:52] nothing in /usr/share/polkit-1 [07:52] just to confirm, the same test passes with previous image. [07:52] it doesn't make any sense [07:52] /o\ [07:52] didrocks, same in d-feet, I can't trigger RequestReboot [07:52] while it's listed [07:52] but same on xenial [07:52] 1. I don't understand how this code could have worked *ever* [07:52] same [07:52] as you don't specify an interface [07:53] 2. I don't understand why specifying the interface, you can access some methods, and not others… [07:53] while* [07:53] I wonder if it always failed to use the session codepath for some reason that we might have fixed [07:53] if gnome_session being False, you mean? [07:54] I know [07:54] Laney broke it [07:54] ? [07:54] -Exec=sudo sh -c 'ubiquity gtk_ui' [07:54] +Exec=sudo --preserve-env=DBUS_SESSION_BUS_ADDRESS,XDG_RUNTIME_DIR sh -c 'ubiquity gtk_ui' [07:54] without the DBUS ADDRESS env it wasn't finding the session object [07:54] so it never used that code [07:54] ohhhhhhh [07:54] so fixing the dbus addressing, we trigger that issue :p [07:55] right [07:55] "fun fun fun" [07:55] which was not needed in the session [07:55] that's the .desktop change [07:55] yep [07:55] still doesn't explain 1. and 2. [07:55] that Laney did because it's the right thing to do [07:55] it explains 1 [07:55] well, maybe this code was never triggered ever [07:55] right [07:55] I thought people implementing it would have tried it :p [07:55] that's what I think [07:55] but why 2. ? [07:55] that's a good question [07:55] ok, now we understand the problem, let's fix it [07:56] it behaves the same on xenial [07:56] yep [07:56] so not a new problem in gnome-session or co [07:56] right [07:56] good to know how it started :) [07:56] so, even if we do the correct fix with using the iface [07:56] we need to understand 2. [07:56] it's not even "you can't execute", it's really "doesn't exist"… [07:57] easy fix is to delete that code [07:57] wait [07:57] it has always been using the else case [07:57] maybe it's really not implemented [07:57] we could keep doing so [07:57] on gnome-session side [07:57] and return this error [07:57] seb128: agreed [07:57] those APIs were distro patch added [07:58] 95_dbus_request_shutdown.patch [07:58] in the debian dir [07:59] I wonder if something changed in gnome-session since those were added which make the implementation needing an update [07:59] yeah, like it's declared but not wired up to the function [07:59] the patch itself isn't enough to see where it's wired up [08:00] yep [08:00] there is a registration [08:01] on the g_dbus_interface_skeleton_export [08:01] and we don't do that for own methods [08:01] our* [08:01] so that explains 2. [08:01] :q [08:02] seb128: gnome-session/gsm-manager.c, line 3208 and so on [08:02] anyway, not going to change that, I think we should fallback to the current behavior [08:02] (current == until today) [08:03] seb128: I would just ditch the broken code in ubiquity [08:04] +1 [08:04] do you want to mp it? [08:04] I can, sure [08:04] thx [08:04] that's one a funny very well intended side-effect :p [08:04] unintended* [08:04] can't type today… [08:04] teaching us for changing more than needed :p [08:04] but I guess I can press del! [08:04] yeah ;) [08:05] there are others import dbus… [08:05] I hope we won't have similar issues that we don't notice at first [08:06] (most of them are using the single SystemBus, so, should be ok) [08:06] should I do the minimal part? [08:07] delete the code or just force gnome_session = False? [08:07] (so that we fix it later) [08:07] depends, if we want to remove the distro-patch anyway… hum [08:07] wdyt? [08:07] I would fix the distro patch [08:07] ok, so let's keep the code [08:07] according to codesearch.debian.net some projects do call to that API [08:07] and just force gnome_session=False? [08:08] +1 [08:08] with a FIXME [08:08] with a code comment [08:08] doing [08:08] sure :p [08:08] thx [08:08] "# FIXME: please ping seb128" [08:08] :) [08:09] lol [08:09] PING [08:13] jibel: seb128: testing with http://paste.ubuntu.com/p/zcgbM59pZQ/ [08:16] didrocks, looks good to me from a reading-the-diff perspective [08:16] yeah, let's get it executed [08:16] didrocks, maybe include a reference to the lp bug? [08:16] also ./tests/run-pep8 works :) [08:17] I did it in the changelog, can do on the FIXME as well [08:18] need a full install to test [08:18] * seb128 starts one [08:18] seb128: can you do in ubiquity-dm? [08:18] testing on live right now [08:19] k, though it shouldn't have an impact in -dm [08:19] shouldn't [08:19] but best to test, we had enough of uuupso cases this week [08:19] but we didn't think about those kind of side-effect :) [08:19] yeah [08:19] :) [08:19] like the oem config key was THERE on the oem install [08:19] and it's checked multiple times in the code :p [08:19] moin [08:19] (install in progress) [08:20] Hey Laney! Today is Wednesday and as everyday, we have an ubiquity upload to do! [08:20] fun [08:21] didrocks, thx, let me test it too [08:21] thanks jibel! [08:21] hey Laney [08:21] hey [08:22] morning [08:22] or well. late late morning. [08:23] had to take morphine, so feel asleep again for an hour or so. [08:23] hey Nafallo [08:26] * willcooke -> train station [08:26] jibel: seb128: Laney: no crash clicking on reboot [08:26] do you want me to perform another test? [08:26] * didrocks MP meanwhile [08:26] beefy machine you have :) [08:26] it never had a 32 bits system on it! :) [08:27] * seb128 does get used as "power off" being used in the gnome-shell menu to "disconnect" eth [08:27] lol [08:27] huh, kdb selection broken again?! [08:27] doesn't* [08:27] to be confirmed ... [08:27] oh, didn't notice [08:27] how so? [08:28] it picked azerty for me [08:28] but I didn't try to change it [08:28] yeah, same here [08:28] maybe I did something weird [08:28] I selected my language in syslinux though [08:28] same [08:28] which was the case which was always working [08:28] what wasn't was to change in the UI [08:28] let me try it in -dm without doing anything at boot [08:31] works here [08:31] in -dm, picked French [08:31] default sectrion in Français/Français (variante obselete) [08:31] and typing is azerty [08:31] changing selection and back to French layout works [08:32] hum, when you don't have an Internet connexion (seems it failed to detect it), the checkbox is grey [08:32] for updates [08:32] but selected [08:32] which is puzzling [08:34] we should probably uncheck it before setting it unsensitive [08:34] should be easy to fix [08:34] * seb128 adds to the list of cleanups [08:34] do you want to do more changes? :) [08:34] that week poking at ubiquity made me want to clean some things at least while I'm looking at it [08:35] not for release :p [08:35] but for +1 or .10 [08:35] .1 or .10 [08:35] yeah ;) [08:35] https://code.launchpad.net/~didrocks/ubiquity/fix-crash-on-reboot/+merge/344215 [08:35] please provide your testing feedback here ^ [08:36] installation is still ongoing but almost there [08:37] ah what does not work is to select english in ubiquity-dm then boot to the live session and select french [08:37] "doesn't work" is arguable [08:38] ah… [08:38] anywayssssssssssss [08:38] this RequestReboot thing got broken basically? [08:38] was never working rather :p [08:39] 1. the code was never executed as lacking correct DBUS_ address [08:39] 2. now that it executes, the code is broken in ubiquity: you don't call a method on a dbus proxy, but need using the interface [08:39] 3. the RequestReboot() and RequestShutdown() are broken (not wired up) in gnome-session [08:39] unsure since then those methods don't work [08:40] ok, well these days you can use logind for that anyway [08:40] so don't need to fix 3 [08:41] (and that's on the system bus ^_^) [08:41] Laney, we should, codesearch.debian shows that there are a bunch of users of that API [08:42] and there might be third party code/admin scripts [08:43] we should fix/get those commited upstream imho [08:43] how can things be using it in debian? [08:45] Laney, https://sources.debian.org/src/italc/1:3.0.3+dfsg1-3/ica/src/LocalSystemIca.cpp/?hl=193#L193 [08:46] the patch isn't there [08:46] Laney, I'm unsure what type of answer you want from me? [08:46] right [08:46] but it exists in Ubuntu and maybe it's enough for some upstream to have started using it? [08:46] well I don't know [08:46] ah whatever, if you want to fix it just do it [08:48] it's probably not important, i'm trying to see if upstream maybe wants it [08:48] I would feel better not removing an API that existed if it's easy to keep for compatibility reasons [08:48] but no big deal either way [08:51] didrocks: any reason to keep the code rather than just adding an XXX: this should use logind comment? [08:52] doesn't *really* matter, but we could avoid a bit of dbus traffic [08:53] Laney: we decided to restore it, but we can later rather switch to logind [08:54] Laney: I don't think that matters, whoever is going to change it afterwards will pick the best decision IMHO [08:54] but now we're doing a GetNameOwner that we ignore the result of [08:54] we have a mp ready/up for merging/tested in live and -dm mode [08:54] so depends how much you want to delay landing/respin I guess [08:55] yeah [08:55] :/ [08:55] either way it doesn't matter much [08:55] this isn't a nice way to approach reviews [08:55] ? [08:55] that was stating the situation [08:55] we can tweak/restest which delays a bit [08:56] or we can merge than, land and line up another mp to be merged directly on friday [08:56] it's up to you as a r-t member to tell us what is best from those who handle the isos respsins perspective [08:56] I don't think anyone is being unreasonable? [08:57] ok it sounded like you were saying "ship this thing, making us change it is busy work" [08:57] no, sorry it sounded like that [08:57] I'm fine doing the change, now or post respin/release is your call [08:58] I don't mind retesting if you change it now. It'd delay the merge by 1h or so [08:59] right, same [08:59] time to write the change and retest should burn 1 or 2 hours [08:59] so r-t call [09:00] I'm moving location, back in 7-8 minutes [09:02] didrocks, can you update your mp to drop the broken code completely? I don't think 1h or 2 impacts the release much [09:03] ok, we decided against it to revive it later on, but fine… [09:03] I don't see what it gives us TBH [09:03] but meh [09:05] ok, removing the dbus import completely thus [09:07] Laney: jibel: pushed to lp:~didrocks/ubiquity/fix-crash-on-reboot, need retesting [09:08] thanks, retesting [09:08] doing it on live [09:08] Laney: helping as well retesting? [09:09] (back) [09:10] seb128: so, pushed code removal, need retesting, jibel is doing it, I'm doing on the live and ask Laney for helping as well [09:11] didrocks, sounds good, thanks [09:11] that way everyone is happy :) [09:12] thx [09:13] and ubiquity crashed… [09:14] one change too much? [09:14] well, I removed the unused now dbus import [09:14] just delete the [09:14] try: [09:14] session = dbus.Bus.get_session() [09:14] gnome_session = session.name_has_owner('org.gnome.SessionManager') [09:14] because I'm sure then someone will come and say "we import something not used in that file" [09:15] maybe the mainloop binding was only done by that file [09:15] from dbus.mainloop.glib import DBusGMainLoop is the mainloop one [09:15] what's the error? [09:15] I don't have a traceback [09:15] just a regular segfault [09:15] journal? [09:16] so, let's readd the mainloop [09:16] didrocks, when did it crahs? [09:16] jibel: after partionning [09:16] oh hey guys [09:16] not for you? [09:17] hey Trevinho, how are you? [09:17] be patient, the iso is still building ... [09:17] jibel: well, I can reliably crash it [09:17] anyway, I'm readding the mainloop, if someone then wants to do that in the correct file calling dbus, he can, ensuring there is no impact on flavors [09:17] nothing in the journal? [09:18] avril 25 11:16:55 ubuntu kernel: traps: ubiquity[8130] trap int3 ip:7f30e9c67c41 sp:7fff937d7f20 error [09:18] right, let's stick to the minimum [09:18] delete try: snippet [09:18] the minimum would be to not remove the call [09:18] to remove the get_session [09:18] but meh [09:18] well, removing that call makes L_aney happy [09:18] and shouldn't create an issue? [09:18] will see [09:19] what did you delete exactly in the buggy version? [09:19] just the import dbus? [09:19] or also the DBusGMainLoop bits? [09:19] well, you need to delete as it's from the dbus import [09:19] I guess that binding shouldbe done where dbus is called [09:20] which it isn't anymore on that file [09:20] k, I guess that's needed [09:20] too much change indeed [09:20] but as said, that's enough for me, let's readd it [09:20] let's stick to the minimal bits [09:20] right [09:20] we can do better cleanups next cycle [09:20] if r-t wants to argue that's an uneeded import, they will figure out where adding it back [09:20] "we" being whoever works on the installer [09:20] well [09:20] I don't think it's going to be an issue [09:20] let's just do that [09:20] there are 23 FIXME in the installer [09:21] seems it's a middle ground that makes everyone happy [09:22] pushed a new rev [09:22] please test [09:22] * didrocks does as well [09:23] don't see it [09:24] to be clear, I didn't say that having a FIXME is bad, in fact I said "rather than just adding an XXX: this should use logind comment" [09:24] Laney: refresh [09:25] at least, we won't have divergent code path anymore, just one [09:25] thx [09:27] seb128: good... Sorry, speaking with Jonas, as we were discussing about one crash fix, and we just landed it :) [09:28] seb128: so far I think the shell reports are basically reduced to low volumes on e.u.c [09:28] what? another ubiquity crash… [09:28] with http://paste.ubuntu.com/p/D7VnTnTqHM/ [09:30] nice, unpacking and apport-unpack crashes [09:30] Trevinho, great work Marco! [09:30] :) [09:33] didrocks, code diff looks fine, I'm unsure [09:33] I don't know as well [09:33] jibel, Laney, can you test didrocks change and see if it creates issues for you as well? [09:33] ok, at least, there is the previous rev which was tested and working [09:33] not sure about that one, I'll let you test as it's obviously crashing here [09:34] I need to step out again for early lunch today, I'm back around 1pm and can help again then [09:34] me too [09:34] k, so let's call that an early french lunch [09:34] hopefully Laney is able to test by the time we are back [09:34] bbiab [09:34] yeah ;) [09:34] bbl! ++ [09:36] yes I'm checking it, no worries [09:37] train wifi = 15 kbps [09:38] seb128, I'm seeing g-i-s segfault when I use the quit menu suddenly. Could be related to slow network [09:38] Will test more in the office [09:48] ah, here's where the fix is happening [10:01] seb128, on it. I removed the dbus import to make pep8 happy [10:02] jibel: nein [10:02] they were worried that's required for some reason [10:02] I'll add an assert to shut pyflakes up [10:11] ok [10:16] ok we just did a live and a DM install, no crash [10:16] going to merge/upload [10:22] same here [10:22] no crash [10:24] ♥ [10:24] adam's approving it [10:49] didrocks: https://twitter.com/itsfoss2/status/989076064734457857 [10:49] is this known? [10:58] Works here [11:08] ah, suggestion on that tweet that it's people upgrading from 16.04 [11:08]