[01:36] PR snapd#7411 opened: cmd/model: output tweaks, add'l tests === epod is now known as luk3yx [04:56] good morning [04:56] wow, I'm online _before_ maciek for once :) [05:35] morning [05:43] Hey Maciek :-) [05:43] Start with reviews please [05:43] There is a fix for master from Sergio [05:45] Good morning mvo [05:51] PR snapd#7410 closed: tests: support fastly-global.cdn.snapcraft.io url on proxy-no-core test [06:01] thank you mborzecki [06:01] zyga: hey ;) [06:02] mvo: morning to you as well [06:02] - google:ubuntu-18.04-64:tests/main/desktop-portal-filechooser [06:02] - google:ubuntu-18.04-64:tests/main/proxy-no-core [06:02] two failures of the day :/ [06:02] duh [06:02] good morning mborzecki and zyga [06:02] restarted already _today_ [06:02] oh well [06:02] I'll rebase on the fix and try again [06:03] mborzecki: I extracted https://github.com/snapcore/snapd/pull/7400 from the MS_SHARED bugfix branch [06:03] PR #7400: cmd/snap-update-ns: don't propagate detaching changes [06:03] mborzecki: it's fairly isolated, except for apparmor changes (yuck) [06:03] mborzecki: I'll add a ns test for writable mimic now [06:06] last night while working on this I wrote DiffEquals checker [06:07] it's like Equals but it requires text or error and it produces actual diffs [06:07] I think it's fantastic and would be 99% of the benefit of whatever else we were using at the total line cost of about a dozen [06:18] zyga: hmm i think the tests in snap-test.c using g_test_subprocess() are incorrect [06:18] zyga: looking at this code right now, and we shouldn't be calling g_test_fail() in the subprocess when we expect the actual code-under-test to die :/ [06:19] zyga: fortunately the actual code is correct :P so it dies as expected [06:19] mborzecki: oh, is that because we set the non-fatal failures? [06:21] zyga: loo at the diff, https://paste.ubuntu.com/p/zKhcQ8gWgK/ we shouldn't be calling g_test_Fail() as it would mask the error case when the sc_draop_instance_key doesn't die() when we expect it to [06:21] mmmm [06:21] mborzecki: perhaps time to move away from die to sc_error [06:21] it's so fragile to test that [06:21] and error is so-much-nicer [06:27] mborzecki: speaking of which [06:27] I have a patch for mountinfo to use that [06:27] I'll send it today [06:27] it's been too long, it's half a year old by now [06:28] mvo: I'll start slow, I stopped working past midnight [06:28] mvo: see you at around 9:30 ish [06:31] zyga: ok === pstolowski|afk is now known as pstolowski [06:58] morning [07:17] PR snapd#7412 opened: tests: run dbus-launch inside a systemd unit [07:53] zyga: added some tests for sc_invocation, though will open a PR once #7406 lands [07:53] PR #7406: cmd/snap-confine: keep track of snap instance name and the snap name [07:54] pstolowski: hey [07:54] mborzecki: hi [07:55] pstolowski: can you take another look at https://github.com/snapcore/snapd/pull/7387? [07:55] PR #7387: packaging/fedora, tests/lib/prepare-restore: helper tool for packing sources for RPM [07:55] mborzecki: yep, just need to finish another review [07:55] pstolowski: sure, thanks! [08:04] pedronis: hey, +1 on #7381 with a few minor remarks and two typo fixes [08:04] PR #7381: seed,image,o/devicestate: extract seed loading to seed/seed16.go [08:05] mborzecki: would it make sense to move pack-source one level up (or somewhere else) since it's not fedora only? [08:06] pstolowski: hm wasn't able to find a good place for it, doesn't really fit under reelase-tools [08:06] pstolowski: maybe under packaging? $top/packaging/pack-source? [08:07] pstolowski: thank you [08:12] mborzecki: yes, directly under packaging/ sounds good [08:40] PR snapd#7413 opened: hookstate/ctlcmd: fix snapctl set help message [08:40] ^ a trivial one line change of help string [08:49] mborzecki: ack [08:49] sorry, back now, [08:49] I was sleeping [08:49] feeling better now [08:49] pstolowski: I made some comments again in #7277 [08:49] PR #7277: overlord/snapstate: fix undo on firstboot seeding [08:53] mvo: I'm commenting on https://github.com/snapcore/snapd/pull/7412 [08:53] oh wow [08:53] PR #7412: tests: run dbus-launch inside a systemd unit [08:53] that's quick :D [08:53] mvo: retry-tool is new, usability ideas welcome [08:54] PR snapd#7404 closed: cmd/snap: don't append / to snap name just because a dir exists [08:56] If it wasn't for xenial and centos 7, I'd suggest just spinning up a systemd user instance together with its bus [08:57] zyga: I was tweaking things there anyway :) [08:57] jamesh: yeah, its a bit of pita on these older distros [08:58] pedronis: ty [08:58] jamesh: i noticed serveral seconds of delay when doing "test-snapd-eds.contacs list" etc, this is normal, right? [08:58] jamesh: I checked with both my version and the previous one and it seems to be no regression [08:58] mvo: is that every time or just the first time? [08:59] jamesh: just the first time [08:59] so it might just be "evolution-data-server is slow to start"? [09:00] jamesh: could be, its seconds and strace shows some sort of poll timing out [09:00] jamesh: but its fine, I was just wondering [09:01] zyga: looks like the problem is more common, fresh bug report: https://bugs.launchpad.net/snapd/+bug/1842521 [09:01] Bug #1842521: Deleted snaps showing up in gnome-disks and nautilus sidebar

[09:02] pstolowski: that's interesting [09:02] it's an app snap that leaks then [09:02] mvo: I don't know specifically, but EDS is a bit over complicated [09:02] jamesh: yeah, its fine [09:03] It's the kind of thing that could probably be a lot smaller if redesigned today, but there is no one to do it [09:13] Chipaca: hey, can you take a look at https://bugs.launchpad.net/snapd/+bug/1838788? you're probably the best person to assess the validity and if it can be fixed [09:13] Bug #1838788: Pagination information not returned in /v2/find [09:13] (or if should be) [09:14] zyga: app leaks? you mean app is stil running and holding the mountpoint? [09:15] pstolowski: it's not a mount point [09:15] it's a loop device [09:15] it doesn't even have to be mounted [09:15] just attached to a file (now deleted) on disk [09:15] it's a different level of leak IMO [09:15] zyga: i see [09:15] I'll spend some time debugging that later, I'm adding tests for writable mimic now [09:15] it's clear that it _is_ leaking [09:16] ack [09:16] slightly annoying it pollutes file manager like this [09:19] no disagreement there [09:31] uhm getting repetitive travis failures on desktop-portal-filechooser (i know this one has a PR from sergio), and two other tests failed to prepare on core16 and core18, it seems like snapd was not listening when it should be there already during prepare [09:31] pstolowski: remember that bug where snapd starts before cores are mounted? [09:31] maybe that happened? [09:32] that happens every time in the core18 spread test (but it recovers afaict) [09:33] if you run it manually you'll see snapd coming and failing to find core18's snap.yaml [09:33] what's not great [09:33] *that's* [09:33] I need coffee, sorry guys, I'm very sleepy still [09:34] Chipaca: thanks for updating the bug [09:35] pstolowski: about that LP bug, that's what i mentioned seeing last time we discsused the problem [09:36] pstolowski: I don't know how you manage to do triage erryday [09:37] pstolowski: I'd be jumping out windows at this point [09:37] Chipaca: if you were any closer I'd grab you for a bike ride together :) [09:37] Chipaca: haha. not every day though.. and today i only looked at maybe 3 bugs [09:37] Chipaca: a bit of a shame we live so far away [09:37] Chipaca: I think the secret is that pawel's window is on the ground floor so he comes back rattled a little and carries on :D [09:38] zyga: opening it before starting triage probably helps [09:38] pstolowski: ssh, let me worship you without knowing the nitty gritty [09:38] Chipaca: however.. it seems to me that just spending 15-30 minutes on it every day we can keep up the pace with any new bugs, so... [09:39] *that's the sprit* [09:39] :) [09:45] Chipaca: one more related for you (and then i'll shut up): https://bugs.launchpad.net/snapd/+bug/1838786 [09:45] Bug #1838786: Categories not returned in /v2/find results [09:45] sorry, two ;) https://bugs.launchpad.net/snapd/+bug/1838787 [09:45] Bug #1838787: Website not returned in /v2/find results [09:47] i have a pr for the first one, some where [09:51] Chipaca: didn't you also work on https://bugs.launchpad.net/snapd/+bug/1832767 at some point? or was it only some fixes for restricted characters? [09:52] Bug #1832767: snap info doesn't strip Markdown [09:52] pstolowski: i fixed it from the other end [09:53] pstolowski: i'll update the bug [09:53] thanks! [09:55] mborzecki: i think we just need better help message to steer users [09:55] pstolowski: in snap connections? [09:56] mborzecki: i didn't remember the details of our discussions from one of the previous sprint. i had literally to play around with it to re-discover this ;) [09:56] mborzecki: yes [09:56] pstolowski: by "from the other end" i mean, the issue i addressed was _ugly_ markdown coming from the store, which we addressed by … not sending ugly markdown :-) [09:56] it can still be fairly ugly, but not full on eugh ugly [09:56] mborzecki: so i can only imagine it's more confusing for users [09:57] Chipaca: i see :). i thought i you meant you fixed it server side so it never sends or accepts it [09:58] s/sends/returns/ [09:59] pstolowski: 'snap info firefox' has plenty of markdown [09:59] it's just not too ugly [10:20] mvo: hi, is #6404 ready for re-review or not yet ? [10:21] PR #6404: snapstate: auto transition on experimental.snapd-snap=true [10:55] * zyga fixed one small bug and moves forward [11:00] PR snapd#7073 closed: interfaces: OpenGL/Vulkan drivers provided by a base should be usable <⛔ Blocked> [11:07] PR snapd#7414 opened: tests: keep track of installed packages and restore the state after the test [11:12] mborzecki: some feedback there [11:13] mborzecki: I experimented with something similar, have a variant of that for dpkg that's somewhat weaker than yours [11:13] mborzecki: I think we could join forces and craft something together [11:13] zyga: sgtm, let me look at the apt-tool and your draft pr [11:14] mborzecki: thank you [11:14] mborzecki: if you want we can spin up a blank testbed-tool PR and land that quickly [11:14] and add various measurement parts on top [11:14] note that my PR does one essential change [11:14] it fixes where we restore [11:14] it's no longer causing the world to fall apart [11:15] zyga, i see some weird behaviour using layuouts ... when using an install or configure hook to put a file into $SNAP_DATA and using a layout to map that file to a subdir in /etc i somehow end up with an empty file in $SNAP_DATA ... in what order do the hooks run in relation to layouts ? [11:15] I think we should split that out as a small PR and land separately after careful review [11:16] (looks like the copied file is overlayed by an empty one somehow, if i take the layout out of snapcraft.yaml i get a proper file as expected (just not mapped to /etc) [11:16] ) [11:16] ogra: to answer your question, hooks cannot ever observe pre-layout filesystem [11:16] ogra: can you file a bug, ideally with a small reproducer, I'll take a look [11:16] so i need a wrapper and actually write into the file then ... k [11:16] ogra: some bugs got fixed recently, 2.41 will be much better on that than before [11:17] ogra: but perhaps there's another one [11:17] https://github.com/snapcore/snapd/pull/7168/files#diff-d2dd0567bc81c6005b7c0f2e727f9b57R531 [11:17] er [11:17] sorry, cannot copy paste in this wayland business [11:17] anyway [11:17] PR #7168: tests: measure testbed for leaking mountinfo entries [11:17] mborzecki: I need to fix the die situation to release 2.41 [11:17] for opensuse [11:17] well, i can switch my machine to beta, perhaps it works there ... [11:17] mborzecki: mvo said he doesn't have time to review 7362 [11:18] * ogra gives it a try with 2.41 [11:18] pstolowski: ^ can you look instead, mvo said another reviewer is fine [11:18] I can then cherry pick that into a distro patch [11:27] pedronis: 6404 is ready yes [11:28] yea, added it to my queue [11:31] zyga: 7362? ok, looking [11:32] yes, thanks! [11:34] allright, i remember that other PR from contributor.. [11:34] PR snapd#7413 closed: hookstate/ctlcmd: fix snapctl set help message === ricab is now known as ricab|lunch [11:44] mborzecki: https://github.com/snapcore/snapd/pull/7400 is green, I think you are the one to review it [11:44] PR #7400: cmd/snap-update-ns: don't propagate detaching changes [11:46] * Chipaca ponders lunch [11:56] more progress, more passing tests [11:56] feels like a good day now [12:02] zyga: +1 with 2 minors [12:02] thanks, looking [12:04] pstolowski: replied on both, can you check the first answer please [12:08] yep, done, got it [12:11] super, thank you [12:18] PR snapcraft#2700 closed: schema: build-base support for the kernel type [12:34] * dot-tobias says hi [12:34] mvo, sergiusens: snapcraft autopkg test failure, triggered by binutils in eoan [12:39] fix binutls then :P [12:40] pstolowski: did you resolve the core18 test prepare failures? getting them too [12:40] https://travis-ci.org/snapcore/snapd/builds/581107491?utm_source=github_status&utm_medium=notification [12:49] Chipaca, is it new? [12:49] cachio: i think so [12:50] Chipaca, I'll take a look if nobody else is already on it [12:50] cachio: thanks! [12:50] np === ricab|lunch is now known as ricab [12:57] Is there an "official" recommendation/doc stating how to detect if running in a snap? I've seen a couple of apps doing it slightly differently (e.g. checking for "SNAP" or "SNAP_NAME" in environment). Someone in #snapcraft was asking. sergiusens pointed out that snapcraft itself uses SNAP_NAME, the preferred route to detect if being invoked by another classic snap. [12:58] Chipaca: not yet, i was about to look at this. got them a couple of times since yesterday [13:01] cachio: standup? [13:03] cjp256: usually most folks just check for $SNAP being defined [13:06] ijohnson: in the past lxd had a problem as snapcraft was calling out to it (the deb) and the in-code logic assumed it was a snap, so we check to see if the SNAP_NAME matches what we want [13:06] PR snapcraft#2701 opened: spread tests: update gnome extension tests [13:07] sergiusens: fair enough, I haven't written many classic snaps so I haven't run into that kind of issue [13:07] assumed it was a snap as it was only checking for the existence of the export and not the value it carried... SNAP_NAME is an easier value to check against as it is fixed to the snap [13:14] jdstrand: hey, can you please look at https://github.com/snapcore/snapd/pull/7400 [13:14] PR #7400: cmd/snap-update-ns: don't propagate detaching changes [13:14] it's super small in non-test code, just interested in your opinion [13:14] zyga: it might be related to the generator, as there is a target (local-fs.target) which is part of snapd.service (and .socket) critical path [13:15] zyga: in 16.04, which doesn't use the generator [13:15] afaik [13:15] It does seem like something that should be formalized one way or another and added to the docs. degville: Where do you think would be the most appropriate place? [13:16] Chipaca: in the bug report it happened on core18 system [13:16] Chipaca: but in either case, we need to think about what happens when snapd doesn't have snaps mounted [13:16] Chipaca: I think only bad things will happen [13:16] zyga: ?? [13:16] Chipaca: because no plugs or no slots [13:16] Chipaca: e.g. on reboot with new kernel, new system key [13:17] zyga: why would a new system key mean no snaps mounted? [13:17] Chipaca: new kernel -> you reboot -> you race with mounting snaps -> you lose -> you create profiles without any interfaces for apps [13:17] zyga: why are you racing with mounting snaps? [13:18] Chipaca: because snapd.service is not synchronized with .mount units [13:18] zyga: it is [13:18] zyga: I just said so [13:18] how is that done? [13:18] ah, I misunderstood that [13:18] can you re-state please [13:18] at least on 16.04, i haven't checked 18.04 but it's easy to check [13:18] zyga: there's a local-fs.target [13:18] zyga: mounts are done before that [13:18] zyga: snapd runs after that [13:19] at least, that's how it _should_ work :-) [13:20] mmm, I see [13:20] zyga: easy to check [13:20] zyga: in core18, systemctl show snap-core-*.mount [13:20] systemctl show snap-core-*.mount | grep Before [13:21] should list local-fs.target [13:21] they have before=snapd.service now [13:21] hmmm [13:21] isn't that automatic somehow? [13:21] $ systemctl show snap-core18-*.mount | grep Before [13:21] the local-fs [13:21] Before=local-fs.target snapd.service multi-user.target umount.target [13:21] something to check [13:21] ^ that's 16.04, 18.04 should have the same [13:21] oh [13:21] show vs cat [13:21] thanks, I didn't know about this [13:21] and, snapd should have After [13:22] in 16.04 that After actually includes every .mount [13:22] note the mount has a before on snapd.service :-) [13:22] so it *should* be doubly-sure that it won't happen [13:22] there is explicit ordering [13:22] now, does it work? i don't know [13:22] if it doens't there's clearly a bug somewhere [13:22] mhm [13:22] yeah, it'd be interesting to see if we can attempt to reproduce this somehow and run it in a loop [13:23] to gather some fresh data [13:23] it might just be in the weird snapd in our core18 spread environ [13:23] or it might be in our weird snapd-from-snap service [13:23] zyga: if you can get a shell on a system showing this, systemctl analyze plot would be useful [13:24] mmm, indeed [13:31] cmatsuoka: ping me if you have questions about snap-image [13:31] zyga: as last words "the treasure is buried in the" is hard to beat [13:32] mborzecki: thanks, I'll have a good look and probably questions will appear [13:35] Chipaca: :) [13:35] pstolowski: remember that TV crew? they are coming back for another episode [13:36] zyga: aha, what tv show is that? [13:36] pstolowski: gliniarze or something like that [13:38] cjp256: I think documenting this is a good idea too. There currently isn't an ideal place - I do have a doc describing the running environment on my todo list, but for now it may be most useful in the snap hooks page: https://forum.snapcraft.io/t/supported-snap-hooks/3795 [13:46] PR snapd#7383 closed: cmd/snap: improve help and error msg for snapshot commands [13:59] off to pick up the kids and do groceries [14:07] mvo: https://github.com/snapcore/snapd/pull/6705#discussion_r321285831 [14:07] PR #6705: bootloader: little kernel support [14:08] mvo: I'll work on that today after I pushed my current state of the image refactoring === vicamo_ is now known as vicamo [14:12] pedronis: I'd rather we didn't print "lk" in error messages as it can be hard to tell from "1k" [14:12] Chipaca: so LK ? [14:12] I don't mind a ton [14:12] as long as it is consistent [14:12] and "cannot open 1k environment file" is not impossible, as error messages go [14:12] * ogra proposes "hackish-android-bootloader" [14:12] :) [14:13] ogra: it's like swedish chef was fired from the kitchen so they picked up bootloader hacking instead [14:13] haha [14:13] ogra: also, https://www.youtube.com/watch?v=B7UmUX68KtE [14:14] pedronis: thank you [14:14] LOL [14:14] Pöpcørn [14:14] pedronis: yeah, thats nice [14:22] pedronis: I'm sorry the explanation for the changes are complicated... I don't think we can do it during start-snap-services because that's too late I think, we need to disable the services before the post-refresh hook [14:23] I need to think more about stop-snap-services, that might be okay because we run that after the post-refresh hook IIRC [14:23] err s/post-refresh/pre-refresh/ [14:23] ijohnson: anyway you are probably victim of having put code near something that seems wrong [14:23] I don't understand anymore [14:24] ijohnson: anyway my main worry about where we unlock remains [14:24] I can look into where we unlock, I mainly just put it there because that's what mborzecki commented on, I will try to find more examples in the codebase of how that is done other places [14:26] pedronis: the main thing I am trying to do is capture the state as late as possible from systemd during unlink-snap, and then restore it as early as possible in link-snap (similar for the undo handlers) [14:26] I don't late is a useful concept here [14:26] this a relatively fast operation holding the lock [14:26] I realize it's not very similar to other things in the codebase since it's both very short lived and also potentially ver long lived [14:26] as they are on trunk [14:26] yes, my worry there [14:26] we are changing some property of this things [14:28] pstolowski: can you remind why of this line: https://github.com/snapcore/snapd/blob/master/overlord/snapstate/handlers.go#L1311 it doesn't make sense to me atm, why would we restore a config while we are unlinking a snap [14:28] I would imagine we do that when linking [14:28] the other unlink has a save instead [14:29] ijohnson: anyway that's why your code confused me ^, I don't understand the lines it has close [14:30] now [14:30] ok [14:31] ijohnson: anyway as I said we need either need to move those interaction with systemd close to end of the handlers, to to the stop/start ones [14:31] s/to to/or to/ [14:31] pedronis: in undoLink, afair we restore config of previous revision, because config might have already been updated e.g. by configure hook [14:31] pstolowski: it's part of the undo case? [14:32] but we do a relink at the very end [14:33] pstolowski: if it's right, it needs a comment, if it isn't it needs moving/going away. atm it's very confusing [14:34] ijohnson: also s/end/start or end/ [14:34] sorry [14:35]