[01:36] <mup> PR snapd#7411 opened: cmd/model: output tweaks, add'l tests <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/7411>
[04:56] <zyga> good morning
[04:56] <zyga> wow, I'm online _before_ maciek for once :)
[05:35] <mborzecki> morning
[05:43] <zyga> Hey Maciek :-)
[05:43] <zyga> Start with reviews please
[05:43] <zyga> There is a fix for master from Sergio
[05:45] <zyga> Good morning mvo
[05:51] <mup> PR snapd#7410 closed: tests: support fastly-global.cdn.snapcraft.io url on proxy-no-core test <Simple 😃> <Created by sergiocazzolato> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/7410>
[06:01] <zyga> thank you mborzecki
[06:01] <mborzecki> zyga: hey ;)
[06:02] <mborzecki> mvo: morning to you as well
[06:02] <zyga>     - google:ubuntu-18.04-64:tests/main/desktop-portal-filechooser
[06:02] <zyga>     - google:ubuntu-18.04-64:tests/main/proxy-no-core
[06:02] <zyga> two failures of the day :/
[06:02] <mborzecki> duh
[06:02] <mvo> good morning mborzecki and zyga
[06:02] <zyga> restarted already _today_
[06:02] <zyga> oh well
[06:02] <zyga> I'll rebase on the fix and try again
[06:03] <zyga> mborzecki: I extracted https://github.com/snapcore/snapd/pull/7400 from the MS_SHARED bugfix branch
[06:03] <mup> PR #7400: cmd/snap-update-ns: don't propagate detaching changes <Bug> <Created by zyga> <https://github.com/snapcore/snapd/pull/7400>
[06:03] <zyga> mborzecki: it's fairly isolated, except for apparmor changes (yuck)
[06:03] <zyga> mborzecki: I'll add a ns test for writable mimic now
[06:06] <zyga> last night while working on this I wrote DiffEquals checker
[06:07] <zyga> it's like Equals but it requires text or error and it produces actual diffs
[06:07] <zyga> 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] <mborzecki> zyga: hmm i think the tests in snap-test.c using g_test_subprocess() are incorrect
[06:18] <mborzecki> 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] <mborzecki> zyga: fortunately the actual code is correct :P so it dies as expected
[06:19] <zyga> mborzecki: oh, is that because we set the non-fatal failures?
[06:21] <mborzecki> 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] <zyga> mmmm
[06:21] <zyga> mborzecki: perhaps time to move away from die to sc_error
[06:21] <zyga> it's so fragile to test that
[06:21] <zyga> and error is so-much-nicer
[06:27] <zyga> mborzecki: speaking of which
[06:27] <zyga> I have a patch for mountinfo to use that
[06:27] <zyga> I'll send it today
[06:27] <zyga> it's been too long, it's half a year old by now
[06:28] <zyga> mvo: I'll start slow, I stopped working past midnight
[06:28] <zyga> mvo: see you at around 9:30 ish
[06:31] <mvo> zyga: ok
[06:58] <pstolowski> morning
[07:17] <mup> PR snapd#7412 opened: tests: run dbus-launch inside a systemd unit <Test Robustness> <Created by mvo5> <https://github.com/snapcore/snapd/pull/7412>
[07:53] <mborzecki> zyga: added some tests for sc_invocation, though will open a PR once #7406 lands
[07:53] <mup> PR #7406: cmd/snap-confine: keep track of snap instance name and the snap name <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/7406>
[07:54] <mborzecki> pstolowski: hey
[07:54] <pstolowski> mborzecki: hi
[07:55] <mborzecki> pstolowski:  can you take another look at https://github.com/snapcore/snapd/pull/7387?
[07:55] <mup> PR #7387: packaging/fedora, tests/lib/prepare-restore: helper tool for packing sources for RPM <Simple 😃> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/7387>
[07:55] <pstolowski> mborzecki: yep, just need to finish another review
[07:55] <mborzecki> pstolowski: sure, thanks!
[08:04] <pstolowski> pedronis: hey, +1 on #7381 with a few minor remarks and two typo fixes
[08:04] <mup> PR #7381: seed,image,o/devicestate: extract seed loading to seed/seed16.go <Created by pedronis> <https://github.com/snapcore/snapd/pull/7381>
[08:05] <pstolowski> mborzecki: would it make sense to move pack-source one level up (or somewhere else) since it's not fedora only?
[08:06] <mborzecki> pstolowski: hm wasn't able to find a good place for it, doesn't really fit under reelase-tools
[08:06] <mborzecki> pstolowski: maybe under packaging? $top/packaging/pack-source?
[08:07] <pedronis> pstolowski: thank you
[08:12] <pstolowski> mborzecki: yes, directly under packaging/ sounds good
[08:40] <mup> PR snapd#7413 opened: hookstate/ctlcmd: fix snapctl set help message <Simple 😃> <Created by stolowski> <https://github.com/snapcore/snapd/pull/7413>
[08:40] <pstolowski> ^ a trivial one line change of help string
[08:49] <zyga> mborzecki: ack
[08:49] <zyga> sorry, back now,
[08:49] <zyga> I was sleeping
[08:49] <zyga> feeling better now
[08:49] <pedronis> pstolowski: I made some comments again in #7277
[08:49] <mup> PR #7277: overlord/snapstate: fix undo on firstboot seeding <Created by stolowski> <https://github.com/snapcore/snapd/pull/7277>
[08:53] <zyga> mvo: I'm commenting on https://github.com/snapcore/snapd/pull/7412
[08:53] <zyga> oh wow
[08:53] <mup> PR #7412: tests: run dbus-launch inside a systemd unit <Test Robustness> <Created by mvo5> <https://github.com/snapcore/snapd/pull/7412>
[08:53] <zyga> that's quick :D
[08:53] <zyga> mvo: retry-tool is new, usability ideas welcome
[08:54] <mup> PR snapd#7404 closed: cmd/snap: don't append / to snap name just because a dir exists <Simple 😃> <Created by chipaca> <Merged by zyga> <https://github.com/snapcore/snapd/pull/7404>
[08:56] <jamesh> 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] <mvo> zyga: I was tweaking things there anyway :)
[08:57] <mvo> jamesh: yeah, its a bit of pita on these older distros
[08:58] <pstolowski> pedronis: ty
[08:58] <mvo> jamesh: i noticed serveral seconds of delay when doing "test-snapd-eds.contacs list" etc, this is normal, right?
[08:58] <mvo> jamesh: I checked with both my version and the previous one and it seems to be no regression
[08:58] <jamesh> mvo: is that every time or just the first time?
[08:59] <mvo> jamesh: just the first time
[08:59] <jamesh> so it might just be "evolution-data-server is slow to start"?
[09:00] <mvo> jamesh: could be, its seconds and strace shows some sort of poll timing out
[09:00] <mvo> jamesh: but its fine, I was just wondering
[09:01] <pstolowski> zyga: looks like the problem is more common, fresh bug report: https://bugs.launchpad.net/snapd/+bug/1842521
[09:01] <mup> Bug #1842521: Deleted snaps showing up in gnome-disks and nautilus sidebar <snapd:New> <gnome-disk-utility (Ubuntu):New> <https://launchpad.net/bugs/1842521>
[09:02] <zyga> pstolowski: that's interesting
[09:02] <zyga> it's an app snap that leaks then
[09:02] <jamesh> mvo: I don't know specifically, but EDS is a bit over complicated
[09:02] <mvo> jamesh: yeah, its fine
[09:03] <jamesh> 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] <pstolowski> 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] <mup> Bug #1838788: Pagination information not returned in /v2/find <snapd:New> <https://launchpad.net/bugs/1838788>
[09:13] <pstolowski> (or if should be)
[09:14] <pstolowski> zyga: app leaks? you mean app is stil running and holding the mountpoint?
[09:15] <zyga> pstolowski: it's not a mount point
[09:15] <zyga> it's a loop device
[09:15] <zyga> it doesn't even have to be mounted
[09:15] <zyga> just attached to a file (now deleted) on disk
[09:15] <zyga> it's a different level of leak IMO
[09:15] <pstolowski> zyga: i see
[09:15] <zyga> I'll spend some time debugging that later, I'm adding tests for writable mimic now
[09:15] <zyga> it's clear that it _is_ leaking
[09:16] <pstolowski> ack
[09:16] <pstolowski> slightly annoying it pollutes file manager like this
[09:19] <zyga> no disagreement there
[09:31] <pstolowski> 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] <zyga> pstolowski: remember that bug where snapd starts before cores are mounted?
[09:31] <zyga> maybe that happened?
[09:32] <Chipaca> that happens every time in the core18 spread test (but it recovers afaict)
[09:33] <Chipaca> if you run it manually you'll see snapd coming and failing to find core18's snap.yaml
[09:33] <zyga> what's not great
[09:33] <zyga> *that's*
[09:33] <zyga> I need coffee, sorry guys, I'm very sleepy still
[09:34] <pstolowski> Chipaca: thanks for updating the bug
[09:35] <mborzecki> pstolowski: about that LP bug, that's what i mentioned seeing last time we discsused the problem
[09:36] <Chipaca> pstolowski: I don't know how you manage to do triage erryday
[09:37] <Chipaca> pstolowski: I'd be jumping out windows at this point
[09:37] <zyga> Chipaca: if you were any closer I'd grab you for a bike ride together :)
[09:37] <pstolowski> Chipaca: haha. not every day though.. and today i only looked at maybe 3 bugs
[09:37] <zyga> Chipaca: a bit of a shame we live so far away
[09:37] <zyga> 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] <Chipaca> zyga: opening it before starting triage probably helps
[09:38] <Chipaca> pstolowski: ssh, let me worship you without knowing the nitty gritty
[09:38] <pstolowski> 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] <zyga> *that's the sprit*
[09:39] <pstolowski> :)
[09:45] <pstolowski> Chipaca: one more related for you (and then i'll shut up): https://bugs.launchpad.net/snapd/+bug/1838786
[09:45] <mup> Bug #1838786: Categories not returned in /v2/find results <snapd:New> <https://launchpad.net/bugs/1838786>
[09:45] <pstolowski> sorry, two ;) https://bugs.launchpad.net/snapd/+bug/1838787
[09:45] <mup> Bug #1838787: Website not returned in /v2/find results <snapd:New> <https://launchpad.net/bugs/1838787>
[09:47] <Chipaca> i have a pr for the first one, some where
[09:51] <pstolowski> 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] <mup> Bug #1832767: snap info doesn't strip Markdown <snapd:New> <https://launchpad.net/bugs/1832767>
[09:52] <Chipaca> pstolowski: i fixed it from the other end
[09:53] <Chipaca> pstolowski: i'll update the bug
[09:53] <pstolowski> thanks!
[09:55] <pstolowski> mborzecki: i think we just need better help message to steer users
[09:55] <mborzecki> pstolowski: in snap connections?
[09:56] <pstolowski> 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] <pstolowski> mborzecki: yes
[09:56] <Chipaca> 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] <Chipaca> it can still be fairly ugly, but not full on eugh ugly
[09:56] <pstolowski> mborzecki: so i can only imagine it's more confusing for users
[09:57] <pstolowski> Chipaca: i see :). i thought i you meant you fixed it server side so it never sends or accepts it
[09:58] <pstolowski> s/sends/returns/
[09:59] <Chipaca> pstolowski: 'snap info firefox' has plenty of markdown
[09:59] <Chipaca> it's just not too ugly
[10:20] <pedronis> mvo: hi, is #6404 ready for re-review or not yet ?
[10:21] <mup> PR #6404: snapstate: auto transition on experimental.snapd-snap=true <Created by mvo5> <https://github.com/snapcore/snapd/pull/6404>
[10:55]  * zyga fixed one small bug and moves forward
[11:00] <mup> PR snapd#7073 closed: interfaces: OpenGL/Vulkan drivers provided by a base should be usable <⛔ Blocked> <Created by jhenstridge> <Closed by jhenstridge> <https://github.com/snapcore/snapd/pull/7073>
[11:07] <mup> PR snapd#7414 opened: tests: keep track of installed packages and restore the state after the test <Test Robustness> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/7414>
[11:12] <zyga> mborzecki: some feedback there
[11:13] <zyga> mborzecki: I experimented with something similar, have a variant of that for dpkg that's somewhat weaker than yours
[11:13] <zyga> mborzecki: I think we could join forces and craft something together
[11:13] <mborzecki> zyga: sgtm, let me look at the apt-tool and your draft pr
[11:14] <zyga> mborzecki: thank you
[11:14] <zyga> mborzecki: if you want we can spin up a blank testbed-tool PR and land that quickly
[11:14] <zyga> and add various measurement parts on top
[11:14] <zyga> note that my PR does one essential change
[11:14] <zyga> it fixes where we restore
[11:14] <zyga> it's no longer causing the world to fall apart
[11:15] <ogra> 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] <zyga> I think we should split that out as a small PR and land separately after careful review
[11:16] <ogra> (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] <ogra> )
[11:16] <zyga> ogra: to answer your question, hooks cannot ever observe pre-layout filesystem
[11:16] <zyga> ogra: can you file a bug, ideally with a small reproducer, I'll take a look
[11:16] <ogra> so i need a wrapper and actually write into the file then ... k
[11:16] <zyga> ogra: some bugs got fixed recently, 2.41 will be much better on that than before
[11:17] <zyga> ogra: but perhaps there's another one
[11:17] <zyga> https://github.com/snapcore/snapd/pull/7168/files#diff-d2dd0567bc81c6005b7c0f2e727f9b57R531
[11:17] <zyga> er
[11:17] <zyga> sorry, cannot copy paste in this wayland business
[11:17] <zyga> anyway
[11:17] <mup> PR #7168: tests: measure testbed for leaking mountinfo entries <Created by zyga> <https://github.com/snapcore/snapd/pull/7168>
[11:17] <zyga> mborzecki: I need to fix the die situation to release 2.41
[11:17] <zyga> for opensuse
[11:17] <ogra> well, i can switch my machine to beta, perhaps it works there ...
[11:17] <zyga> mborzecki: mvo said he doesn't have time to review 7362
[11:18]  * ogra gives it a try with 2.41
[11:18] <zyga> pstolowski: ^ can you look instead, mvo said another reviewer is fine
[11:18] <zyga> I can then cherry pick that into a distro patch
[11:27] <mvo> pedronis: 6404 is ready yes
[11:28] <pedronis> yea, added it to my queue
[11:31] <pstolowski> zyga: 7362? ok, looking
[11:32] <zyga> yes, thanks!
[11:34] <pstolowski> allright, i remember that other PR from contributor..
[11:34] <mup> PR snapd#7413 closed: hookstate/ctlcmd: fix snapctl set help message <Simple 😃> <Created by stolowski> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/7413>
[11:44] <zyga> mborzecki: https://github.com/snapcore/snapd/pull/7400 is green, I think you are the one to review it
[11:44] <mup> PR #7400: cmd/snap-update-ns: don't propagate detaching changes <Bug> <Created by zyga> <https://github.com/snapcore/snapd/pull/7400>
[11:46]  * Chipaca ponders lunch
[11:56] <zyga> more progress, more passing tests
[11:56] <zyga> feels like a good day now
[12:02] <pstolowski> zyga: +1 with 2 minors
[12:02] <zyga> thanks, looking
[12:04] <zyga> pstolowski: replied on both, can you check the first answer please
[12:08] <pstolowski> yep, done, got it
[12:11] <zyga> super, thank you
[12:18] <mup> PR snapcraft#2700 closed: schema: build-base support for the kernel type <Created by sergiusens> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/2700>
[12:34]  * dot-tobias says hi
[12:34] <doko> mvo, sergiusens: snapcraft autopkg test failure, triggered by binutils in eoan
[12:39] <ogra> fix binutls then :P
[12:40] <Chipaca> pstolowski: did you resolve the core18 test prepare failures? getting them too
[12:40] <Chipaca> https://travis-ci.org/snapcore/snapd/builds/581107491?utm_source=github_status&utm_medium=notification
[12:49] <cachio> Chipaca, is it new?
[12:49] <Chipaca> cachio: i think so
[12:50] <cachio> Chipaca, I'll take a look if nobody else is already on it
[12:50] <Chipaca> cachio: thanks!
[12:50] <cachio> np
[12:57] <cjp256> 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] <pstolowski> Chipaca: not yet, i was about to look at this. got them a couple of times since yesterday
[13:01] <Chipaca> cachio: standup?
[13:03] <ijohnson> cjp256: usually most folks just check for $SNAP being defined
[13:06] <sergiusens> 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] <mup> PR snapcraft#2701 opened: spread tests: update gnome extension tests <Created by sergiusens> <https://github.com/snapcore/snapcraft/pull/2701>
[13:07] <ijohnson> sergiusens: fair enough, I haven't written many classic snaps so I haven't run into that kind of issue
[13:07] <sergiusens> 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] <zyga> jdstrand: hey, can you please look at https://github.com/snapcore/snapd/pull/7400
[13:14] <mup> PR #7400: cmd/snap-update-ns: don't propagate detaching changes <Bug> <Created by zyga> <https://github.com/snapcore/snapd/pull/7400>
[13:14] <zyga> it's super small in non-test code, just interested in your opinion
[13:14] <Chipaca> 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] <Chipaca> zyga: in 16.04, which doesn't use the generator
[13:15] <Chipaca> afaik
[13:15] <cjp256> 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] <zyga> Chipaca: in the bug report it happened on core18 system
[13:16] <zyga> Chipaca: but in either case, we need to think about what happens when snapd doesn't have snaps mounted
[13:16] <zyga> Chipaca: I think only bad things will happen
[13:16] <Chipaca> zyga: ??
[13:16] <zyga> Chipaca: because no plugs or no slots
[13:16] <zyga> Chipaca: e.g. on reboot with new kernel,  new system key
[13:17] <Chipaca> zyga: why would a new system key mean no snaps mounted?
[13:17] <zyga> Chipaca: new kernel -> you reboot -> you race with mounting snaps -> you lose -> you create profiles without any interfaces for apps
[13:17] <Chipaca> zyga: why are you racing with mounting snaps?
[13:18] <zyga> Chipaca: because snapd.service is not synchronized with .mount units
[13:18] <Chipaca> zyga: it is
[13:18] <Chipaca> zyga: I just said so
[13:18] <zyga> how is that done?
[13:18] <zyga> ah, I misunderstood that
[13:18] <zyga> can you re-state please
[13:18] <Chipaca> at least on 16.04, i haven't checked 18.04 but it's easy to check
[13:18] <Chipaca> zyga: there's a local-fs.target
[13:18] <Chipaca> zyga: mounts are done before that
[13:18] <Chipaca> zyga: snapd runs after that
[13:19] <Chipaca> at least, that's how it _should_ work :-)
[13:20] <zyga> mmm, I see
[13:20] <Chipaca> zyga: easy to check
[13:20] <Chipaca> zyga: in core18, systemctl show snap-core-*.mount
[13:20] <Chipaca> systemctl show snap-core-*.mount | grep Before
[13:21] <Chipaca> should list local-fs.target
[13:21] <zyga> they have before=snapd.service now
[13:21] <zyga> hmmm
[13:21] <zyga> isn't that automatic somehow?
[13:21] <Chipaca> $ systemctl show snap-core18-*.mount | grep Before
[13:21] <zyga> the local-fs
[13:21] <Chipaca> Before=local-fs.target snapd.service multi-user.target umount.target
[13:21] <zyga> something to check
[13:21] <Chipaca> ^ that's 16.04, 18.04 should have the same
[13:21] <zyga> oh
[13:21] <zyga> show vs cat
[13:21] <zyga> thanks, I didn't know about this
[13:21] <Chipaca> and, snapd should have After
[13:22] <Chipaca> in 16.04 that After actually includes every .mount
[13:22] <Chipaca> note the mount has a before on snapd.service :-)
[13:22] <Chipaca> so it *should* be doubly-sure that it won't happen
[13:22] <Chipaca> there is explicit ordering
[13:22] <Chipaca> now, does it work? i don't know
[13:22] <Chipaca> if it doens't there's clearly a bug somewhere
[13:22] <zyga> mhm
[13:22] <zyga> yeah, it'd be interesting to see if we can attempt to reproduce this somehow and run it in a loop
[13:23] <zyga> to gather some fresh data
[13:23] <Chipaca> it might just be in the weird snapd in our core18 spread environ
[13:23] <zyga> or it might be in our weird snapd-from-snap service
[13:23] <Chipaca> zyga: if you can get a shell on a system showing this, systemctl analyze plot would be useful
[13:24] <zyga> mmm, indeed
[13:31] <mborzecki> cmatsuoka: ping me if you have questions about snap-image
[13:31] <Chipaca> zyga: as last words "the treasure is buried in the" is hard to beat
[13:32] <cmatsuoka> mborzecki: thanks, I'll have a good look and probably questions will appear
[13:35] <zyga> Chipaca: :)
[13:35] <zyga> pstolowski: remember that TV crew? they are coming back for another episode
[13:36] <pstolowski> zyga: aha, what tv show is that?
[13:36] <zyga> pstolowski: gliniarze or something like that
[13:38] <degville> 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] <mup> PR snapd#7383 closed: cmd/snap: improve help and error msg for snapshot commands <Created by galgalesh> <Merged by chipaca> <https://github.com/snapcore/snapd/pull/7383>
[13:59] <mborzecki> off to pick up the kids and do groceries
[14:07] <pedronis> mvo: https://github.com/snapcore/snapd/pull/6705#discussion_r321285831
[14:07] <mup> PR #6705: bootloader: little kernel support <Needs Samuele review> <Created by kubiko> <https://github.com/snapcore/snapd/pull/6705>
[14:08] <pedronis> mvo: I'll work on that today after I pushed my current state of the image refactoring
[14:12] <Chipaca> pedronis: I'd rather we didn't print "lk" in error messages as it can be hard to tell from "1k"
[14:12] <pedronis> Chipaca: so LK ?
[14:12] <pedronis> I don't mind a ton
[14:12] <pedronis> as long as it is consistent
[14:12] <Chipaca> and "cannot open 1k environment file" is not impossible, as error messages go
[14:12]  * ogra proposes "hackish-android-bootloader" 
[14:12] <Chipaca> :)
[14:13] <Chipaca> ogra: it's like swedish chef was fired from the kitchen so they picked up bootloader hacking instead
[14:13] <ogra> haha
[14:13] <Chipaca> ogra: also, https://www.youtube.com/watch?v=B7UmUX68KtE
[14:14] <mvo> pedronis: thank you
[14:14] <ogra> LOL
[14:14] <ogra> Pöpcørn
[14:14] <mvo> pedronis: yeah, thats nice
[14:22] <ijohnson> 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] <ijohnson> 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] <ijohnson> err s/post-refresh/pre-refresh/
[14:23] <pedronis> ijohnson: anyway you are probably victim of having put code near something that seems wrong
[14:23] <pedronis> I don't understand anymore
[14:24] <pedronis> ijohnson: anyway my main worry about where we unlock remains
[14:24] <ijohnson> 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] <ijohnson> 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] <pedronis> I don't late is a useful concept here
[14:26] <pedronis> this a relatively fast operation holding the lock
[14:26] <ijohnson> 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] <pedronis> as they are on trunk
[14:26] <pedronis> yes, my worry there
[14:26] <pedronis> we are changing some property of this things
[14:28] <pedronis> 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] <pedronis> I would imagine we do that when linking
[14:28] <pedronis> the other unlink has a save instead
[14:29] <pedronis> ijohnson: anyway that's why your code confused me ^, I don't understand the lines it has close
[14:30] <pedronis> now
[14:30] <ijohnson> ok
[14:31] <pedronis> 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] <pedronis> s/to to/or to/
[14:31] <pstolowski> pedronis: in undoLink, afair we restore config of previous revision, because config might have already been updated e.g. by configure hook
[14:31] <pedronis> pstolowski: it's part of the undo case?
[14:32] <pedronis> but we do a relink at the very end
[14:33] <pedronis> 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] <pedronis> ijohnson: also  s/end/start or end/
[14:34] <pedronis> sorry
[14:35] <ijohnson> what other tasks do we run after a configure hook? the only one I see is the health-check
[14:36] <ijohnson> could the health-check fail and trigger things to be undone?
[14:37] <ijohnson> if the health-check could fail the entire change, then I think pstolowski's code is correct as is, because the configure hook could have successfully finished and committed changes to the state that we need to restore
[14:37] <ijohnson> s/health-check/any other random task that runs after the configure snap task/
[14:38] <pedronis> but by definition we should run anything on an unlinked snap
[14:38] <pedronis> shouldn't run
[14:38] <pedronis> that's the bit I don't get
[14:38] <pedronis> I'm not saying we shouldn't restore
[14:38] <pedronis> I'm questioning the where
[14:38] <ijohnson> hmm
[14:38] <pedronis> when of it
[14:39] <ijohnson> I see your point now
[14:39] <ijohnson> for my case, with specifically undoLinkSnap, we could save the state of the services very early in the function rather than right before the unlink
[14:39] <ijohnson> would that make more sense?
[14:40] <ijohnson> because we can't save it after the unlink
[14:42] <pstolowski> i need to refresh my memory re the details of config snapshots; the general idea is to switch to respective version of config whenever we switch between revisions, whether it's undo or succesfull op
[14:43] <pedronis> ijohnson: yes, doing this slow things that don't want the lock either first or last seems best
[14:43] <pedronis> sorry, do want the lock
[14:43] <pstolowski> i'll check that code and add comments or fix if needed
[14:43] <pedronis> pstolowski: thank you
[14:43] <zyga> re
[14:44] <pedronis> pstolowski: it got me slightly confused in a different review
[14:44] <ijohnson> pedronis: ack I will make changes to that effect, do you still want me to pursue using the task for storing the list short-term and then commit it later for long term storage?
[14:45] <pedronis> ijohnson: yes, think about that and see if it's better or worse
[14:45] <ijohnson> I'm still not convinced we can entirely push it out to the start/stop-snap-services tasks I need to think on that a bit more
[14:45] <ijohnson> okay I will see what I can do
[14:45] <ijohnson> thanks for the review
[14:45] <pedronis> ijohnson: keeping unlink/link fast is attractive
[14:45] <pedronis> in terms of think we can reason about
[14:45] <ijohnson> also did you see my questions about the new `snap model` output on your forum post?
[14:45] <ijohnson> (and also my PR for that too)
[14:46] <pedronis> ijohnson: yes, but didn't get to look at it yet, I saw the question
[14:46] <ijohnson> okay, thanks
[14:46] <pedronis> my original thought was not do that, but now not sure
[14:46] <pedronis> I'm thinking about it
[14:46] <ijohnson> okay, well whenever you get a chance either on the PR or the forum post is fine and I'll update the PR with whatever else you need
[14:47] <ijohnson> oh one last thing, do you think I should split out the cli.StoreAccount change to a different PR?
[14:47] <ijohnson> it seemed like a small change so I kept it but I could also see the case for making that a different PR
[14:48] <pedronis> ijohnson: the PR at least at moment doesn't look too big, and we have nothing pressing that would need it, so seems ok
[14:48] <pedronis> as is
[14:48] <ijohnson> pedronis: ack, thanks
[14:50] <jdstrand> zyga: I plan to (7400)
[14:52] <zyga> thank you
[15:00] <diddledan> ooh, 2.41 is in candidate!
[15:00] <diddledan> \o/
[15:00]  * diddledan reboots to loonicks to try it
[15:13] <pstolowski> zyga, Chipaca annoyingly, I cannot reproduce that spread test issue we talked about on standup when running individual tests on gc
[15:14] <zyga> pstolowski: it's always like that
[15:14] <zyga> the magic compounds :/
[15:35] <diddledan> dang. my changes in 2.41 aren't enough for makemkv to find the optical drive :-(
[15:36] <diddledan> looks like it still needs another permission granted: `/sys/bus/scsi/devices r,`
[15:36] <diddledan> I would expect that to be in hardware-observe. but we _could_ add a new scsi-observe if we want it more fine-grained?
[15:37] <diddledan> @jdstrand ^^^^
[15:39] <mup> PR snapcraft#2701 closed: spread tests: update gnome extension tests <Created by sergiusens> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/2701>
[15:40] <diddledan> how do I double-check the device cgroup again to make sure it added the sg0 device into it?
[15:45] <mup> PR snapd#7415 opened: tests: fix mountinfo-tool filtering when used with rewriting <Created by zyga> <https://github.com/snapcore/snapd/pull/7415>
[15:45] <mup> PR snapd#7416 opened: seed/seedwriter: start of Writer and internal policy16/tree16 <Created by pedronis> <https://github.com/snapcore/snapd/pull/7416>
[15:46] <pedronis> mvo: next steps ^ but it's huge instead of just big because I need to land the seed16 one
[15:47] <pedronis> first
[15:48] <pedronis> doing the small bootloader Find change now
[15:50] <diddledan> ok, it looks like the cgroup has it assigned (actually sg2, not sg0) `c 21:2 rwm`
[15:50] <diddledan> so my changes are working correctly, they're just not sufficient :-)
[15:51] <mup> PR snapd#7387 closed: packaging/fedora, tests/lib/prepare-restore: helper tool for packing sources for RPM <Simple 😃> <Created by bboozzoo> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/7387>
[15:54] <mup> PR snapcraft#2702 opened: Release changelog for 3.8 <Created by sergiusens> <https://github.com/snapcore/snapcraft/pull/2702>
[16:00]  * cachio lunch
[16:00] <mvo> pedronis: cool, I think I will have a look at this pr tomorrow
[16:01]  * Chipaca → run
[16:01] <mvo> pedronis: uh, 3k diff :/
[16:01]  * mvo runs as well
[16:01] <zyga> mvo: haha
[16:01] <zyga> mvo: now you promised ;)
[16:02] <mvo> zyga: heh - its actually not that huge, it contains the seed loading one (7381)
[16:02] <Chipaca> mvo: *actual* run?
[16:02] <mvo> Chipaca: nah, not today :(
[16:03] <Chipaca> mvo: aw, had my hopes up
[16:03] <Chipaca> anyway, 'm off. take care and see some of you around later. Otherwise, have a great weekend!
[16:03] <zyga> Chipaca: o/
[16:07] <mup> PR snapd#7417 opened: interfaces/wayland: On classic systems only consider the OS slot for auto-connect <Created by AlanGriffiths> <https://github.com/snapcore/snapd/pull/7417>
[16:21] <pedronis> mvo: it contains the previous one as I said
[16:21] <pedronis> but yes, it will be 1400 on its own
[16:46] <mvo> pedronis: I'm experimenting with bases remodel right now, when testing this for real I get "invalid request: unexpected data after serial-request and optional model"
[16:46] <mvo> pedronis: is this something known, am I doing it wrong?
[16:47] <pedronis> mvo: experimenting in which sense?
[16:47] <mvo> pedronis: doing a uc16->uc18 remodel
[16:47] <pedronis> mvo: with the what kind of model?
[16:47] <pedronis> in the tests?
[16:47] <pedronis> for real?
[16:47] <mvo> pedronis: the real models
[16:47] <mvo> pedronis: should I use test models for this?
[16:48] <pedronis> mvo: the issue is that the serial vault knows about remodeling, but the store doesn't
[16:48] <pedronis> the real model use the store as serial vault
[16:48] <pedronis> the store needs  change for that to work
[16:49] <mvo> pedronis: aha, ok. that makes sense then, I will setup appropriate test models and retry :)
[16:49] <mvo> pedronis: thats fine, thanks for the explaination
[16:49]  * mvo needs to be afk for a few minutes
[16:49] <pedronis> mvo: the store is not setup to do remodels, but I suppose we need to teach about this  kind of remodel
[16:49] <pedronis> if we want to do it
[16:49] <pedronis> a bit unclear though
[16:51] <mup> PR snapd#7159 closed: tests: add functions to make an abstraction for the snaps <Simple 😃> <Created by sergiocazzolato> <Merged by sergiocazzolato> <https://github.com/snapcore/snapd/pull/7159>
[16:51]  * zyga is on child watch duty
[16:51] <zyga> though there's one more bugfix PR coming after
[17:23] <zyga> making some more progress
[17:23] <zyga> about to fix duplication on core16
[17:23] <zyga> need to explore because unique to 16 somehow
[17:23] <Saviq> ughies https://status.snapcraft.io/
[17:24] <zyga> https://xkcd.com/908/
[17:35] <mup> PR snapd#7418 opened: many: pass the rootdir and options to bootloader.Find <Created by pedronis> <https://github.com/snapcore/snapd/pull/7418>
[17:44] <mup> PR snapd#7362 closed: cmd: unify die() across C programs <Created by zyga> <Merged by zyga> <https://github.com/snapcore/snapd/pull/7362>
[17:45] <mup> PR snapd#7358 closed: cmd/system-shutdown: Rename die() -> die_reboot() to fix build failures with LTO enabled <⛔ Blocked> <Created by dcermak> <Closed by zyga> <https://github.com/snapcore/snapd/pull/7358>
[17:48] <ardaguclu_> TCP connection gets lost after handshake completed for snapcraft.io
[17:48] <zyga> ardaguclu_: status.snapcraft.io
[17:49] <ardaguclu_> zyga: yeah, after checking there I tried myself
[17:52] <mborzecki> https://paste.ubuntu.com/p/dpSXHrZtQc/ funny test failure
[18:09] <jdstrand> diddledan: hardware-observe should handle that
[18:12] <jdstrand> diddledan: as for the device cgroup, look in /sys/fs/cgroup/devices/snap.name.cmd/devices.list for the major:minor of /dev/sg0. can also do: udevadm info /dev/sg0 to see if the TAGS has your snap
[18:14] <diddledan> Thanks, jdstrand . The device is correctly in the cgroup.
[18:14] <jdstrand> \o/
[18:27] <mup> PR snapd#7406 closed: cmd/snap-confine: keep track of snap instance name and the snap name <Created by bboozzoo> <Merged by zyga> <https://github.com/snapcore/snapd/pull/7406>
[18:30] <mvo> hrm, hrm, so commit 92a096278e040b452125260b00742ae6e62caf2c (zyg@xenial-server) breaks the cla checker in my 6404 pr - I wonder how to get out of this missery
[18:34] <zyga> oops
[18:34] <zyga> ah, 2016
[18:35] <zyga> uff :)
[18:35] <zyga> mvo: I can offer a rebase solution ;)
[18:35] <zyga> (just kidding)
[18:35] <zyga> can you whitelist zyga@xenial-server and
[18:35] <zyga> just call it a day?
[18:35] <mvo> zyga: I have no idea why it breaks :/
[18:35] <mvo> zyga: I think that works - or I could rebase my patches
[18:35] <zyga> I mean the rebase was a silly suggestion
[18:35] <zyga> you'd have to rebase 3 years
[18:36] <mvo> zyga: I wonder why it hits my PR :(
[18:36] <zyga> mvo: git history caught it?
[18:36] <zyga> mvo: AFAIK we take only shallow history
[18:36] <zyga> so something close is related pulling it in?
[18:36] <zyga> not sure
[18:37]  * mvo has no idea
[19:29] <jdstrand> zyga: I review PR 7400. curious, how much more is there wrt 'mount namespace robustness'?
[19:29] <mup> PR #7400: cmd/snap-update-ns: don't propagate detaching changes <Bug> <Created by zyga> <https://github.com/snapcore/snapd/pull/7400>
[19:29] <jdstrand> reviewed*
[19:29] <zyga> jdstrand: MS_SHARED for sure
[19:29] <zyga> jdstrand: some more tests
[19:29] <zyga> jdstrand: like mount-ns test for more constructs, specifically the mimic
[19:29] <zyga> jdstrand: after that I think we can return to wrapping up persistence
[19:29] <jdstrand> zyga: are there open PRs for these or are those all the ones you'll reopen?
[19:30] <zyga> jdstrand: I'll open those, I'm trying to keep it under six
[19:30] <zyga> jdstrand: some are not fully ready
[19:30]  * jdstrand nods
[19:30] <zyga> jdstrand: note that MS_SHARED you had reviewed before
[19:30] <zyga> jdstrand: but it uncovered more issues
[19:30] <zyga> jdstrand: there's something wrong with loopback devices
[19:30] <zyga> jdstrand: they definitely leak
[19:30] <zyga> jdstrand: apps, bases, all alike
[19:30] <zyga> jdstrand: so more research into that area
[19:31] <zyga> jdstrand: is there some specific concern you have?
[19:32] <jdstrand> zyga: just trying to understand the scope and timing of work. this will be a blue item for us
[19:32] <zyga> jdstrand: we have an item for it, under robustness
[19:33] <zyga> jdstrand: you can refer to that one from your roadmap perhaps, not sure if that's useful
[19:33] <zyga> jdstrand: note that most of the work involves digging and fixing tests
[19:33] <zyga> jdstrand: there are relatively few patches still, compared to the amount of time
[19:33] <jdstrand> zyga: yes. and we will have a similar one that is blue for me to perform the reviews to help you achieve it :)
[19:33] <zyga> jdstrand: thank you :)
[19:33] <zyga> jdstrand: so far most of the effort was to stabilize the test suite
[19:33] <zyga> jdstrand: so that we actually get to write new tests
[19:34] <jdstrand> yeah. it is all really good stuff
[19:34] <zyga> I'll produce updates and answers for your questions tomorrow
[19:34] <jdstrand> anyway, don't mean to dtract from your evening or change any priorities, just a simple question to help me plan
[19:34] <zyga> jdstrand: no worries, it's always fine :)
[21:00]  * Chipaca EOWs
[23:23] <diddledan> gui snaps work in WSL2: https://usercontent.irccloud-cdn.com/file/HyrFk7TC/image.png