[05:07] <mborzecki> morning
[05:08] <zyga> Hey hey
[05:09] <zyga> Will you have a moment to look at my reasoning today?
[05:09] <zyga> As soon as the kids go to school we could have a quick call
[05:10] <mborzecki> zyga: hey
[05:10] <mborzecki> zyga: sure, ping me
[05:10] <zyga> Also https://github.com/snapcore/snapd/pull/6796 is simple and I have more later :/
[05:10] <mup> PR #6796: cmd/snap-update-ns: add no-op load/save current user profile logic <Created by zyga> <https://github.com/snapcore/snapd/pull/6796>
[05:51] <zyga> ok, both kids are handled now
[05:52] <zyga> mborzecki: https://meet.google.com/rug-kzid-ewz?authuser=0
[05:57] <mborzecki> zyga: ok, let me get some coffee
[05:58] <zyga> sure, thank you
[06:46] <mup> PR snapd#6829 closed: devicestate: use deviceCtx in checkGadgetOrKernel <Remodel :train:> <Created by pedronis> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/6829>
[06:47] <zyga> https://github.com/zyga/hello-cuda
[06:53] <pedronis> mvo: hi, #6833 is the next one in the series, is actually the refactoring about creating models in tests I started Friday (that derailed me a bit)
[06:53] <mup> PR #6833: many: introduce assertstest.SigningAccounts and AddMany test helpers <Remodel :train:> <Created by pedronis> <https://github.com/snapcore/snapd/pull/6833>
[06:58] <pedronis> mvo: I'm going to merge 6776, it will be turned into calls to remodel ctx methods soon
[06:59] <mvo> pedronis: ok, I will check 6833 hopefully this morning
[06:59] <pstolowski> morning
[06:59] <mup> PR snapd#6776 closed: devicestate: set "new-model" on the remodel change <Remodel :train:> <Simple 😃> <Created by mvo5> <Merged by pedronis> <https://github.com/snapcore/snapd/pull/6776>
[07:00] <mvo> pedronis: about 6776> sounds good, will you work on the rmeodel ctx method transition or shall I?
[07:00] <mvo> pedronis: having it will make the kernel-remodel PR much smaller, so thats nice
[07:02] <zyga> mvo: we fixed it
[07:02] <zyga> mvo: wooooot
[07:02] <zyga> the bug is real, it's easy to fix and it's well understood now
[07:02] <mvo> zyga: \o/
[07:02] <mvo> zyga: is it a kernel bug?
[07:02] <zyga> no, my bug :)
[07:03] <zyga> but very elusive, you'll see soon
[07:03]  * zyga is euphoric 
[07:04] <zyga> I'll chop the fixes up and start sending it right away
[07:04] <zyga> kenvandine: I've fixed a massive bug relating to layouts, content and magic-it-is-broken-bugs
[07:04] <zyga> that explains a lot of things not working :D
[07:04] <mvo> zyga: is this a 2.39.1?
[07:04] <zyga> mvo: yes, we should
[07:04] <zyga> it will help everyone on the desktop
[07:04] <zyga> mvo: the fix is small
[07:04] <zyga> so should be fine, I'll start preparing branches
[07:04] <zyga> mvo: it's really this:
[07:05] <zyga> when we mount tmpfs we don't set propagation so it stays private
[07:05] <zyga> that's all :)
[07:05] <zyga> this applies to layout tmpfs and mimic tmpfs
[07:05] <zyga> mborzecki and me just spent last hour debugging it in a live pair programming session
[07:05] <zyga> mborzecki thank you so much for helping :)
[07:05] <mborzecki> mvo: hey
[07:06] <mborzecki> zyga: not sure i helped much though :)
[07:06] <zyga> mvo: so the nvidia state is not only great but also working reliably now :)
[07:06] <mborzecki> pstolowski: hey to you too :)
[07:06] <pstolowski> o/
[07:08] <mvo> zyga: cool
[07:08] <mvo> hey mborzecki and pstolowski
[07:08] <mvo> zyga: can we spread test this somehow?
[07:09] <zyga> Yes
[07:10] <mvo> zyga: excellent
[07:10] <pedronis> mvo: sorry, I will work on the remodel methods translation, my next task is actuall implementing brand store switch for real
[07:10] <mvo> pedronis: excellent, thanks
[07:10] <mvo> pedronis: just wanted to double check if I can help you in this specific task
[07:10] <zyga> mvo: though with some loopholes because of per user mount namespaces
[07:10] <zyga> mvo: but yes; we can test this
[07:11] <mvo> pstolowski: I reviewed 6793 - do you think a unit test cmd_debug_timings_test.go for a followup would be hard? just something simple?
[07:11] <mvo> zyga: cool!
[07:12] <pedronis> mvo: we need a follow up anyway because of the discussion about having an artificial line to make the ensure bits more like the change bits
[07:12] <pstolowski> mvo: yes, i was thinking about that, some rudimentgary unit tests would be good to have, will do in a followup
[07:13] <pedronis> pstolowski: but with the branch we can gather data on the pis which would be great
[07:14] <pstolowski> yes
[07:19] <mvo> pstolowski: great! I +1 as not having this test is not a regression and we can fix in a followup :)
[07:19] <pstolowski> ty
[07:22] <zyga> Saviq: hey
[07:22] <zyga> Saviq: I think I also fixed the bug that was affecting mir
[07:22] <zyga> the one you asked me about a few weeks ago
[07:23] <zyga> this is the bug of the month for me :)
[07:23]  * zyga reports bugs to begin working on regression tests to begin chopping fixes into patches
[07:23] <Saviq> zyga: got a bug#? :)
[07:23] <zyga> Saviq: I'm reporting it now
[07:24] <zyga> Saviq: but this is when you wanted to share mir backend drivers with mir and then share mir stack + backend driver to apps
[07:24] <zyga> I know why this is broken now
[07:24] <Saviq> Oh
[07:24] <zyga> yeah :)
[07:24] <zyga> it's getting fixed today
[07:24] <Saviq> That'd be awesome :)
[07:24] <zyga> this single bug explains LOADS of mysteriously incorrect behavior
[07:26] <zyga> kenvandine: ^ this is the bug that was breaking themes and gnome platform snap and all kinds of "connected but not connected" bugs
[07:26] <zyga> mvo: ^ you can see why I'm so happy now
[07:28] <mvo> zyga: \o/
[07:35] <pedronis> zyga: great, is it easy to add tests about it?
[07:36] <zyga> pedronis: not every easy but 100% doable, the bug is extremely elusive and two of the components of the bug would make a simple spread test not catch it
[07:36] <zyga> pedronis: I'm describing the bug in detail
[07:36] <zyga> pedronis: and I know how to write a spread test for it
[07:36] <zyga> pedronis: none of our current tests doing content or layout would capture it
[07:36] <pedronis> ok, just wondering because we didn't catch it so far
[07:36] <zyga> it's a tricky bastard :)
[07:36] <pedronis> but good if we can have a spread test
[07:36] <zyga> yes, that's 2nd step after reporting the bug :)
[07:36] <zyga> write a test and show it fail :)
[07:37] <pedronis> mvo: I'm going to cleanup managers_test.go a little bit as a drive-by, mostly ms -> s, and using testutil AddCleanup
[07:38] <mvo> pedronis: \o/
[07:40] <mup> PR snapd#6844 opened: interfaces: special-case "snapd" in sanitizeSlotReservedForOS* helpers <Simple 😃> <⚠ Critical> <Created by stolowski> <https://github.com/snapcore/snapd/pull/6844>
[07:41] <pstolowski> mvo, pedronis ^ so this fixes hotplug on core18+snapd, tested manually on pi3
[07:41] <pstolowski> and i'll prep proper followup (touching 80+ files) as discussed yesterday
[07:42] <mvo> pstolowski: nice
[07:42] <mvo> pstolowski: this is also 2.39.1 I suppose?
[07:42] <pstolowski> mvo: yes
[07:42] <mvo> ta
[07:44] <zyga> mborzecki, mvo, pedronis, Saviq: https://bugs.launchpad.net/snapd/+bug/1828354
[07:44] <mup> Bug #1828354: mount event propagation on snapd-mounted tmpfs is incorrect <snapd:In Progress by zyga> <https://launchpad.net/bugs/1828354>
[07:44] <zyga> I will start writing the spread test
[07:44] <zyga> in case I get hit by a bus mborzecki knows how to fix it too :)
[07:44] <mborzecki> hahah
[07:45] <zyga> let me know if the bug description is unclear
[07:46] <pstolowski> i'll also think about spread test for core18+snapd & hotplug, need to talk to Sergio about that
[07:54] <Saviq> zyga: any news on https://bugs.launchpad.net/snapd/+bug/1819875 ?
[07:54] <mup> Bug #1819875: base core16->core18 migration <snapd:In Progress by zyga> <https://launchpad.net/bugs/1819875>
[07:56] <zyga> Saviq: it's stalled, I know what to do, will attack it after Lyon
[07:56] <zyga> Saviq: the initial patch was ANACked, I started working on v2 and have close-but-not-quite code for that
[07:56] <zyga> mborzecki, mvo: I also reported https://bugs.launchpad.net/snapd/+bug/1828357
[07:56] <mup> Bug #1828357: snap-update-ns incorrectly sorts mount profile entries <snapd:New> <https://launchpad.net/bugs/1828357>
[07:57] <zyga> Saviq: there are several more bugs in core->core18 migration
[07:57] <zyga> Saviq: one is that snapd tools go stale
[07:58] <zyga> Saviq: there are several other bugs affecting core18, I don't think they are all reported
[07:58] <Saviq> so "not yet" :)
[07:59] <zyga> no, apart from Lyon-driven work it is close to the top of my queue
[07:59] <zyga> (no as in: "no, not yet")
[08:02] <pstolowski> mborzecki: replied to your comment in https://github.com/snapcore/snapd/pull/6793 ; can you re-review?
[08:02] <mup> PR #6793: cmd/snap: support for --ensure argument for snap debug timings <Created by stolowski> <https://github.com/snapcore/snapd/pull/6793>
[08:03] <mborzecki> pstolowski: thanks, it was more like an open question, nothing to be done in that PR anyway
[08:06] <mvo> pedronis: should I add switching gadgets (with the caveat that we cannot do asset upadates yet) or do you think thats premature?
[08:07] <pedronis> mvo: that is more delicate, we really need to do the right thing with GadgetInfo
[08:07] <pedronis> it's used much more than KernelInfo
[08:09]  * mvo nods
[08:12] <mvo> a second review of 6835 would be great - its small
[08:13] <pedronis> mborzecki: I tried to answer your question, bit confused by the 2nd because that's the central point of the PR, though it might be sutble
[08:15] <mvo> 6751 has two +1 but I added the unit test - so a second review from someone who is not me would be great
[08:15] <mvo> its pretty small and should be simple to review
[08:16] <pedronis> mborzecki: I changed the description of 6822, made that should makes thing clear. It also go outdated at some point.
[08:17] <pedronis> s/made/maybe/
[08:17] <pedronis> mvo: I can look in a little bit
[08:20] <mborzecki> pedronis: what i meant in the 2nd question is cosmetic mostly, whether to embed *DeviceManger or have it as a field like `manager *DeviceManger`
[08:21] <pedronis> mborzecki: ah, I didn't understand that way
[08:21] <mborzecki> pedronis: sorry, should have made it more clear in the comment
[08:23] <pedronis> mborzecki: no, you are right, the wrapper implements all the interface methods
[08:23] <pedronis> so it could be a field
[08:23] <pedronis> this changed a bit over the history of the branch
[08:24] <pedronis> or I could remove .DeviceManager
[08:24] <pedronis> and keep it this way
[08:24] <pedronis> but as it is is all a bit silly
[08:25] <mborzecki> pedronis: i mean it's not a problem, we could clean it up later on, or keep it like this, either way the code works
[08:26] <Chipaca> pstolowski: o/
[08:26] <pedronis> mborzecki: yea, for context,  at some point DeviceManager had Device
[08:26] <pedronis> but now that is hidden
[08:26] <pstolowski> hey Chipaca
[08:27] <Chipaca> pstolowski: hiya. I've got a question about hooks :-)
[08:27]  * pstolowski is hiding
[08:27] <pstolowski> shoot
[08:28] <Chipaca> pstolowski: I'm getting 'cannot find hook' when trying to install (of a hook i've added), from snap-exec, but in snap-exec I print info.Hooks and the hook is right there, but the snap-exec does not seem to be the one from /usr/lib/snapd … is it always from core or sth?
[08:29] <pstolowski> Chipaca: what hook?
[08:29] <Chipaca> pstolowski: I've just answered myself by doing a bind-mount, and yes it's the one from core
[08:29] <Chipaca> (i got a different panic now \o/ )
[08:30] <Chipaca> pstolowski: check-health
[08:32] <pstolowski> Chipaca: you'll need to whitelist a new hook in supportedHooks (and possibly in one other place, but not sure from top of my head). but you probably already found that out
[08:33] <Chipaca> pstolowski: yep :-)
[08:33] <Chipaca> pstolowski: I just got the whole thing working \o/
[08:34] <pstolowski> nice! \o/
[08:37] <mup> PR snapd#6796 closed: cmd/snap-update-ns: add no-op load/save current user profile logic <Created by zyga> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/6796>
[08:38] <Chipaca> pedronis: can we talk about storage for a bit?
[08:39] <Chipaca> pedronis: actually, i'll just write out my question and you answer when you have time
[08:41] <Chipaca> pedronis: some decisions need to be done on 'edge', ie when transitioning from good to bad or vice versa, and I wonder if what we actually want is a log of all the runs (or the last N)
[08:41] <mup> PR snapd#6845 opened: cmd/snap-update-ns: move apply{Profile,{User,System}Fstab} to same file <Created by zyga> <https://github.com/snapcore/snapd/pull/6845>
[08:42] <Chipaca> pedronis: so the question is: should I make it an in-state thing that keeps the current result and the previous one, or the current result and the previous status, or should it be on disk with a log of everything?
[08:42] <zyga> pstolowski: asked a question in https://github.com/snapcore/snapd/pull/6844
[08:42] <mup> PR #6844: interfaces: special-case "snapd" in sanitizeSlotReservedForOS* helpers <Simple 😃> <⚠ Critical> <Created by stolowski> <https://github.com/snapcore/snapd/pull/6844>
[08:43] <Chipaca> pedronis: are actions going to be taken in-hook-handler, looking at current and previous result only, or are they going to be run from outside and look at all history, or...?
[08:44] <pedronis> Chipaca: which decisions need to be made on edge ? I don't remeber that
[08:44] <Chipaca> pedronis: when it was ok but suddenly isn't
[08:45] <Chipaca> pedronis: warning the user? alerting the backend?
[08:45] <Chipaca> not sure (it's not part of this round of work)
[08:45] <Chipaca> but there was some action we wanted to do when a snap had been fine and then started being broken
[08:46] <pedronis> we want to check more often when is broken
[08:46] <pedronis> but that itsn't about the edge
[08:46] <pedronis> we do need to know what the status was before a refresh and after
[08:46] <pedronis> Chipaca: atm I would expect to take "actions" from check-rerefresh
[08:47] <pedronis> Chipaca: I mean, we always discussed needing to run the hook both at the start and during a refresh
[08:48] <Chipaca> pedronis: and then take actions based on the difference, right?
[08:48] <pedronis> possibly yes
[08:48] <Chipaca> ie bad->good vs good->bad
[08:48] <pedronis> but that info could be kept on the change/task in the change
[08:49] <pedronis> anyway if you think we should keep the last two different values
[08:49] <pedronis> that's fine
[08:49] <pedronis> I would put this in the state
[08:49] <Chipaca> ok
[08:50] <pedronis> Chipaca: we need to remember for which revision they are though? though the idea is that health itself is not for a revision
[08:50] <zyga> brb, coffee
[08:52] <pedronis> Chipaca: I mean, is not clear to me whether we should just stick the last value in the change , or we keep always two, but we haven't dug deep into all the things we might want to do with this
[08:54] <Chipaca> pedronis: I'll make it a map[instance]HealthState; we can always have HealthState have a Previous HealthState
[08:55] <Chipaca> pedronis: I'll add Revision to HealthState: like the timestamp, it's not that the health is 'for' a timestamp or revision, but that when the health was this, these were the timestamp and revision
[08:55] <Chipaca> will help with debugging / auditing / developing at the least
[08:56] <pedronis> yes
[08:56] <Chipaca> pedronis: initial PR will just run the hook at the end of install
[08:56] <Chipaca> fwiw
[08:56] <pedronis> yea, that's fine
[08:57] <pedronis> Chipaca: you mean install/refresh, or only install?
[08:57] <Chipaca> pedronis: doInstall :-) install/refresh/try/revert
[08:57] <pedronis> ok
[08:58] <pstolowski> zyga: replied
[08:58] <zyga> pstolowski: thanks, looking
[08:59] <zyga> pstolowski: +1
[09:00] <pstolowski> zyga: ty
[09:00] <pstolowski> zyga: i'm preparing a followup to remove these helpers from all interfaces. that means reservedForOS in common.go will go away
[09:01] <zyga> pstolowski: why?
[09:01] <pstolowski> zyga: because base declaration do the same
[09:01] <zyga> pstolowski: but base declaration is *not* considered when doing snap install --dangerous
[09:01] <zyga> or am I mistaken?
[09:01] <pstolowski> zyga: oh
[09:01] <zyga> it will give people false impressions when developing snaps
[09:02] <zyga> perhaps we should run the base policy checker but just print a warning on install when --dangerous is used
[09:02] <pstolowski> zyga: fair point if this is the case
[09:02] <zyga> then I think this is sensible
[09:02] <zyga> and far less code too
[09:12] <mvo> pedronis: do you want to tweak 6822 further or can it go in ? it has two +1 but there is this discussion about initialRegistrationContext
[09:13] <mup> PR snapd#6846 opened: cmd/snap-confine: unshare per-user mount ns once <Created by zyga> <https://github.com/snapcore/snapd/pull/6846>
[09:13] <zyga> mborzecki: https://github.com/snapcore/snapd/pull/6846
[09:13] <mup> PR #6846: cmd/snap-confine: unshare per-user mount ns once <Created by zyga> <https://github.com/snapcore/snapd/pull/6846>
[09:14] <pedronis> mvo: I can change it a bit in a follow up
[09:14] <pedronis> it can go in
[09:15] <mup> PR snapd#6822 closed: overlord/devicestate: introduce registrationContext <Remodel :train:> <Created by pedronis> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/6822>
[09:15] <Chipaca> pstolowski: why is hookstate.Context's SnapRevision "unset", when the revision is something like -4?
[09:15] <mvo> pedronis: it can I think - I'm inclined to merge 6833 as well or do you think this needs further reviews (all tests/test-helpers)
[09:16] <Chipaca> ooohhh hooksetup
[09:16]  * Chipaca looks into it
[09:16] <pedronis> we never set the revision I think
[09:16] <pedronis> actually
[09:16] <pedronis> there was a lot of back and forth on that
[09:17] <pstolowski> Chipaca: this predates the time i started working on snapd.. i think Gustavo and Kyle worked on this, i don't know the background
[09:18] <pedronis> mvo: I don't know about 6833 , it's all tests but it's big
[09:19] <pedronis> also maybe people have opinions on names
[09:19] <pedronis> I don't know
[09:21] <pedronis> pstolowski: I answered your question in 6822
[09:22] <pstolowski> ty
[09:23] <mvo> pedronis: I think its a thing of beauty so maybe someone more reviews are actually good :)
[09:24] <pstolowski> mvo: updated #6844
[09:24] <mup> PR #6844: interfaces: special-case "snapd" in sanitizeSlotReservedForOS* helpers <Simple 😃> <⚠ Critical> <Created by stolowski> <https://github.com/snapcore/snapd/pull/6844>
[09:24] <mvo> pstolowski: \o/
[09:24] <zyga> mvo: replied on that unshare bug
[09:25] <zyga> let me know if you want me to pursue the test, I personally think it's not worth it
[09:25] <mup> Bug #1828374 opened: Unable to install qabro <Snappy:New> <https://launchpad.net/bugs/1828374>
[09:26] <mvo> zyga: thank you
[09:26] <zyga> mvo: we could write a test that would use trace but it would be super tricky because this code only runs when *not* root but we'd need to run snap-confine via sudo + strace and then somehow drop back to user while running under strace already
[09:27] <zyga> CE has reported a bug related to seeding
[09:27] <zyga> https://bugs.launchpad.net/snappy/+bug/1828374
[09:27] <mup> Bug #1828374: Unable to install qabro <Snappy:New> <https://launchpad.net/bugs/1828374>
[09:27] <zyga> there's one change, seeding, it is stuck for two days
[09:27] <zyga> the Doing task is Doing 2 days ago, at 04:16 EDT - Ensure prerequisites for "gnome-calculator" are available
[09:27] <zyga> is this the ordering bug
[09:27] <mvo> zyga: hm,hm and there is nothing else with propagation (or lakc thereof) that is easy(ish) to observe?
[09:27] <zyga> where bases must be before snaps?
[09:27] <zyga> mvo: correct
[09:28] <mvo> zyga: is this 2.39 material?
[09:28] <zyga> mvo: yes, but only because I would like to test this together with the 2nd bug fix and not without
[09:28] <zyga> mvo: and if it passes and nobody spots any hole in reviews it is a clear bug to get rid of
[09:29] <mvo> zyga: ok, we should probably have a quick call on it if it goes into 2.39 without some sort of test
[09:29] <zyga> mvo: what would we discuss?
[09:29] <mvo> zyga: a bit later though, I will wait for the second PR
[09:29] <zyga> mvo: ok
[09:29] <zyga> mvo: I can write the allocation test
[09:29] <mvo> zyga: not sure thats worth it
[09:29] <zyga> it'd be a bit fugly but if you strognly one one
[09:29] <mvo> zyga: maybe I'm too cautious
[09:29] <pedronis> zyga: yes, that seems and ordering bug
[09:29] <pedronis> or missing base
[09:29] <zyga> pedronis: is the ordering documented somewhere that I can point CE to?
[09:30] <mvo> zyga: anyway, please focus on the second PR and then we can talk a bit more
[09:30] <zyga> mvo: ok
[09:30] <zyga> doing that now
[09:30] <mvo> zyga: thank you!
[09:30] <pedronis> zyga: we really shouldn't have ordering bugs actually, but missing stuff is an issue
[09:31] <zyga> pedronis: I agree, I would add that snapd should bail if it detects a missing base and not just stall for dayts
[09:32] <pedronis> the more focuses we are on the big things the more is likely we can do something about the small ones
[09:34] <zyga> pedronis: yeah, agreed
[09:34] <niemeyer> Mornings
[09:34] <niemeyer> mvo, pedronis_: Do you have a moment for a quick call?
[09:35] <mvo> niemeyer: yes, can be there in 1min
[09:36] <niemeyer> Sweet
[09:36] <niemeyer> Let's use the standup link
[09:45] <Chipaca> pstolowski: question about something I'm finding hard to follow: I'm getting my hook run even for snaps that don't have the hook. Where does the decision not to run hooks that aren't present happen? the (to me) obvious place, hookmgr's runHook, knows there isn't a hook and carries on
[09:46] <Chipaca> bah
[09:46] <Chipaca> it's not the (non-existent) hook that's run, it's the handler
[09:46] <Chipaca> the Done() thing
[09:47] <Chipaca> pstolowski: so i change my question: how can the Done handler know whether the hook was run?
[09:54] <Chipaca> pstolowski: or, why doesn't runHook just return nil when it knows there isn't a hook to run?
[09:55] <pstolowski> Chipaca: it seems it's always called even if hook doesn't exist. and the only case when it's not run is when error occurs and IgnoreError is not set
[09:56] <zyga> 	Failed for "golang.org/x/crypto/cast5" (failed to clone repo): exit status 128
[09:56] <pstolowski> Chipaca: i think we have never relied on Done() so far
[09:56] <mborzecki> hmm ubuntu-image is full of surprises
[09:56] <mborzecki> calculated = ceil(farthest_offset / 1024 + 17) * 1024
[09:56] <Chipaca> pstolowski: grmbl
[09:56] <mborzecki> that's how image size is calculated befire it's actually built
[09:56] <zyga> there's more from godbus :/
[09:56] <Chipaca> pedronis: looking at blame maybe i should've asked you the above :-) or maybe kyrofa
[09:56] <pstolowski> Chipaca: again.. this is old code, almost untouched since initial impl
[09:57] <pstolowski> Chipaca: what you're saying sounds reasonable to do imho
[09:57] <mborzecki> and a TODO note too: # TODO: Hard-codes last 34 512-byte sectors for backup GPT, empirically derived from sgdisk behavior.
[10:27]  * pstolowski lunch
[10:27]  * zyga is hungry too
[10:35] <kenvandine> zyga: great!
[10:36]  * kenvandine high fives zyga 
[10:50] <zyga> kenvandine: I'm totally crazy happy about this bug finally coming out of the shadows :)
[10:51] <zyga> kenvandine: this is https://bugs.launchpad.net/snapd/+bug/1828354 if you want to track
[10:51] <mup> Bug #1828354: mount event propagation on snapd-mounted tmpfs is incorrect <snapd:In Progress by zyga> <https://launchpad.net/bugs/1828354>
[10:51] <zyga> I'll fix it today
[10:51] <zyga> but first, I need to take the dog out, he's looking at me and asks for it :)
[11:08] <mup> PR snapd#6845 closed: cmd/snap-update-ns: move apply{Profile,{User,System}Fstab} to same file <Simple 😃> <Created by zyga> <Merged by zyga> <https://github.com/snapcore/snapd/pull/6845>
[11:16] <mup> PR snapd#6847 opened: cmd/snap-update-ns: make apply{User,System}Fstab identical <Created by zyga> <https://github.com/snapcore/snapd/pull/6847>
[11:25] <mup> PR snapd#6848 opened: First pass at healthstate (no tests yet) <Created by chipaca> <https://github.com/snapcore/snapd/pull/6848>
[11:25] <Chipaca> pedronis: ^^ draft PR (working on adding tests)
[11:25] <Chipaca> pedronis: will probably drop the IsCtrl thing until later, as it's not essential and clutters the pr
[11:25] <pedronis> Chipaca: thx
[11:26] <Chipaca> pedronis: but thought you might want an early look
[11:30] <mborzecki> do we have some code already that escapes runes into \x.. hex encoding like systemd-escape?
[11:30] <pedronis> Chipaca: is SnapRevision actually set? doesn't need to be set in CheckHealth ?
[11:30] <Chipaca> mborzecki: systemd.Escape ?
[11:31] <Chipaca> mborzecki: systemd.EscapeUnitNamePath
[11:31] <Chipaca> pedronis: it is set
[11:31] <mborzecki> Chipaca: thanks!
[11:31] <Chipaca> pedronis: it's always "unset" for local revisions
[11:31] <Chipaca> pedronis: it is set for store revisions
[11:31] <Chipaca> pedronis: so that's probably good enough
[11:31] <pedronis> Chipaca: ok, bit puzzled by that
[11:31] <Chipaca> me three
[11:31] <Chipaca> :)
[11:32] <pedronis> I don't remember it being that way
[11:32] <mvo> woah, this "draft" feature is cool
[11:32] <Chipaca> pedronis: you can try it locally, becaues Done is always called so health is always set to something
[11:32]  * mvo hugs Chipaca for learning something new
[11:32] <pedronis> Chipaca: the other question, not immediate, if it will always want to write directly to the health state, or sometimes we'll need to keep the health pending somewhere
[11:32]  * Chipaca hugs mvo for the attitude
[11:32] <pedronis> but we should have ways to add that indirection if needed
[11:33] <Chipaca> pedronis: yep
[11:41] <pedronis> mvo: 6751 looks good but I have a comment about lack of comments :)
[11:42] <mvo> pedronis: thanks, will fix
[11:58] <ogra> Download snap "motion-ogra" (16) from channel "edge"                                                                                       0% 17.2kB/s 63.3m
[11:58] <ogra> hmpf !
[12:06] <mborzecki> off to pick up the kids
[12:13] <zyga> back from lunch and walk
[12:16] <zyga> mvo: can you revise your review on https://github.com/snapcore/snapd/pull/6751
[12:16] <mup> PR #6751: overlord/snapstate: perform hard refresh check <Created by zyga> <https://github.com/snapcore/snapd/pull/6751>
[12:23] <cwayne> ogra: heya! Youve got a pi 3b+ yeah?
[12:26] <mvo> zyga: sure, checking
[12:26] <zyga> mvo: I think the PR needs a comment
[12:26] <zyga> but perhaps one can be added via github directly
[12:26] <ogra> cwayne, who hasnt ? (its the default you get since 1.5y if you order one :P )
[12:28] <mvo> zyga: yeah, I have a meeting now will see when I can add it
[12:28] <zyga> mvo: I can add the comment, just need your review change
[12:28] <mvo> zyga: +1
[12:28] <zyga> thanks!
[12:35] <cwayne> ogra: :) was just curious if you've seen any wifi issues with the latest core/snapd
[12:36] <ogra> cwayne, not on core 16 and 18 is still too immature for daily use IMHO
[12:47] <cwayne> ogra: with every snap on stable? Just curious as we're seeing wifi issues on ours in the lab but only one of them
[12:48] <ogra> cwayne, with core and kernel on stable (i mostly use my own gadgets for my in-house images that run sensible stuff)
[12:49] <cwayne> Hm ok so it's not a kernel issue then, that's good
[12:49] <cwayne> I think our pi may just be flaky
[12:50] <ogra> or there is some freequency distortionn/noise ... the wlan isnt very good through walls either
[12:55] <cwayne> Doesn't go through any walls iirc, but could be lots of things
[12:55] <cwayne> Just weird that it happened so suddenly
[13:31] <mup> PR snapd#6798 closed: gadget: introduce checkers for sanitizing structure updates <Gadget update> <Created by bboozzoo> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/6798>
[13:44] <mup> PR snapd#6849 opened: many: introduce a gadget helper for locating device matching given structure <Gadget update> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/6849>
[13:55] <mup> PR snapd#6850 opened: gadget: add volume level update checks <Gadget update> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/6850>
[13:58] <pedronis> a 2nd review for 6833 , it's a tests refactor
[13:58] <pedronis> would be great
[14:08] <popey_> where should I file a crasher bug against the snap command line tool?
[14:09] <Chipaca> popey_: oooh
[14:09] <Chipaca> popey_: tell me more
[14:09] <popey_> https://paste.ubuntu.com/p/zZwqpZ2N2w/
[14:09] <popey_> snap info qtencodegui
[14:09] <popey_> run that
[14:10] <Chipaca> popey_: https://bugs.launchpad.net/snapd/+filebug?field.title=snapd+went+boom
[14:11] <popey_> :)
[14:11] <Chipaca> s/snapd\+/snap+/ but that
[14:11] <Chipaca> fffantastic
[14:12] <popey_> https://bugs.launchpad.net/snapd/+bug/1828425
[14:12] <mup> Bug #1828425: snap went boom <snapd:New> <https://launchpad.net/bugs/1828425>
[14:27] <Chipaca> WTheck
[14:29] <Chipaca> grahrrr
[14:29]  * Chipaca hates self
[14:30] <roadmr> great bug description :)
[14:31] <zyga> dinner
[14:38] <Chipaca> mvo: we've got a snap crasher, and a fix for it coming up, what should i target it to?
[14:39] <Chipaca> mvo: (this is not a regression)
[14:39] <Chipaca> mvo: (this is not a drill either)
[14:39] <mvo> Chipaca: 2.39
[14:39] <mvo> Chipaca: we need to do  a 2.39.1
[14:39] <Chipaca> mvo: tagging is enough?
[14:39] <mvo> Chipaca: yes
[14:39] <Chipaca> k
[14:39] <Chipaca> mvo: thanks
[14:40] <mup> PR snapd#6851 opened: cmd/snap: mangle descriptions that have indent > terminal width <Simple 😃> <⚠ Critical> <Created by chipaca> <https://github.com/snapcore/snapd/pull/6851>
[14:40] <Chipaca> mvo: popey_: ^^ tadaa
[14:41] <Chipaca> popey_: and before you go "i set my terminal to 1000 columns wide and it still crashes", snap caps the terminal width to ~100 so paragraphs are legible
[14:54] <kenvandine> jdstrand: if i want USN notifications to be sent to a list for a bunch of snaps, do i have to add the list as a collaborator?
[14:55]  * kenvandine hopes not
[14:55] <jdstrand> kenvandine: no. just tell me and I'll adjust
[14:55] <jdstrand> kenvandine: it needs to be an open list
[14:56] <kenvandine> cool
[14:56] <jdstrand> otherwise you'll need to moderate the emails
[15:07] <mup> PR snapd#6847 closed: cmd/snap-update-ns: make apply{User,System}Fstab identical <Created by zyga> <Merged by zyga> <https://github.com/snapcore/snapd/pull/6847>
[15:09] <cachio> Chipaca, niemeyer hey, when you have time, could you please take a look to those PRs on spread project https://github.com/snapcore/spread/pull/70 and https://github.com/snapcore/spread/pull/75 ?
[15:09] <cachio> thanks
[15:09] <mup> PR spread#70: Close ssh connection when reboot is stuck <Created by sergiocazzolato> <https://github.com/snapcore/spread/pull/70>
[15:09] <mup> PR spread#75: Make spread tests for spread project run on google backend <Created by sergiocazzolato> <https://github.com/snapcore/spread/pull/75>
[15:09] <mup> PR snapd#6852 opened: cmd/snap-update-ns: merge apply functions <Created by zyga> <https://github.com/snapcore/snapd/pull/6852>
[15:10] <Chipaca> cachio: yes
[15:12] <mup> PR snapd#6844 closed: interfaces: special-case "snapd" in sanitizeSlotReservedForOS* helpers <Simple 😃> <Squash-merge> <⚠ Critical> <Created by stolowski> <Merged by stolowski> <https://github.com/snapcore/snapd/pull/6844>
[15:18] <mup> PR snapd#6853 opened: interfaces: special-case "snapd" in sanitizeSlotReservedForOS* helper… <⚠ Critical> <Created by stolowski> <https://github.com/snapcore/snapd/pull/6853>
[15:28] <pedronis> mvo: going to merge 6833 at this point, happy to get later feedback
[15:28] <zyga> thanks for all the review guys! that's the best day for this feature for a looooong time
[15:28] <mup> PR snapd#6833 closed: many: introduce assertstest.SigningAccounts and AddMany test helpers <Remodel :train:> <Created by pedronis> <Merged by pedronis> <https://github.com/snapcore/snapd/pull/6833>
[15:30] <mvo> pedronis: +1
[15:40] <pedronis> now the next one is real code and it has turned out bigger than I expected (mostly because of unit tests), I might have to split it, #6838
[15:40] <mup> PR #6838: overlord/devicestate: introduce remodel kinds and contexts <Remodel :train:> <⛔ Blocked> <Created by pedronis> <https://github.com/snapcore/snapd/pull/6838>
[15:48] <zyga> mvo: I need to break and run an errand but I experimented a bit more and I have good feeling about the propagation bug tests
[15:49] <zyga> mvo: perhaps tomorrow we can attempt to do .1 and put it into beta?
[15:50] <zyga> mvo: though not sure if that's super required
[15:56]  * cachio lunch
[15:56] <Chipaca> cachio: what about spread#73?
[15:56] <mvo> zyga: I will see what I can do
[15:56] <mup> PR spread#73: Run tests on travis <Created by sergiocazzolato> <https://github.com/snapcore/spread/pull/73>
[15:56] <popey_> Chipaca: i actually was running snap info in a terminal, so no idea what it thinks the terminal width is
[15:56] <zyga> mvo: let's talk tomorrow
[15:56] <zyga> mvo: thank you for everything today!
[15:56] <popey_> s/terminal/script/
[15:57] <mvo> zyga: thank you!
[15:58] <Chipaca> popey_: https://imgflip.com/i/30jols
[15:59] <popey_> that'll do :)
[15:59] <zyga> Chipaca: buhahaha, that's too funny :)
[16:01] <Chipaca> zyga: :)
[16:01] <Chipaca> zyga: "A bug reference, if one exists, would be nice to add as a comment." i'm not sure what you mean, given the link in the description
[16:01] <zyga> I mean the comment :)
[16:01] <zyga> the // giving up one
[16:02] <Chipaca> zyga: ohhh
[16:02] <Chipaca> zyga: I'll add it if I need to bump spread
[16:02] <Chipaca> :)
[16:02] <zyga> +1
[16:02] <Chipaca> OTOH it's only just started
[16:02] <Chipaca> i could cancel the whole thing
[16:02]  * Chipaca goes for it
[16:04] <mup> PR pc-amd64-gadget#10 closed: Add mmx64.efi (MokManager) to support mokutil <Created by tsunghanliu> <https://github.com/snapcore/pc-amd64-gadget/pull/10>
[16:04] <mup> PR pc-amd64-gadget#11 closed: Add mmx64.efi (MokManager) to support mokutil <Created by tsunghanliu> <https://github.com/snapcore/pc-amd64-gadget/pull/11>
[16:05] <mup> PR pc-amd64-gadget#10 opened: Add mmx64.efi (MokManager) to support mokutil <Created by tsunghanliu> <https://github.com/snapcore/pc-amd64-gadget/pull/10>
[16:05] <mup> PR pc-amd64-gadget#11 opened: Add mmx64.efi (MokManager) to support mokutil <Created by tsunghanliu> <https://github.com/snapcore/pc-amd64-gadget/pull/11>
[16:06] <Chipaca> zyga: like so?
[16:06] <zyga> very nice
[16:06] <zyga> thank you!
[16:06] <Chipaca> huzzah
[16:09] <kenvandine> Chipaca: some snaps with contact urls set aren't showing them in the output of snap info
[16:09] <kenvandine> Chipaca: any idea why?
[16:10] <Chipaca> kenvandine: impossible :-p
[16:10] <kenvandine> :)
[16:10] <kenvandine> Chipaca: look at libreoffice and gnome-system-monitor
[16:10] <kenvandine> the latter doesn't have contact at all
[16:10] <Chipaca> kenvandine: it does if you don't have it installed
[16:11] <kenvandine> oh
[16:11] <kenvandine> that seems like a bug :)
[16:11] <Chipaca> contact:   https://bugs.launchpad.net/ubuntu/+source/gnome-system-monitor/+bugs?field.tag=snap
[16:11] <Chipaca> ^ that's what i get
[16:11] <kenvandine> ok, that's what it should be
[16:11] <kenvandine> weird you don't have it installed, that's a seeded snap :)
[16:11] <Chipaca> kenvandine: so, the snap does _not_ have contact info
[16:11] <Chipaca> kenvandine: I'm on 16.04
[16:11] <kenvandine> ah
[16:12] <Chipaca> kenvandine: some fields, if the store has edited versions, get pulled down
[16:12] <Chipaca> kenvandine: contact is not one of those fields
[16:12] <Chipaca> kenvandine: that is
[16:12] <Chipaca> kenvandine: some fields are per snap, some fields are per revision
[16:13] <Chipaca> kenvandine: if they're per snap, they should be fetched from the store on install, etc, like description does
[16:13] <Chipaca> today only title, summary, and description get this treatment
[16:14] <kenvandine> i think contact would be very interesting info for installed snaps
[16:14] <kenvandine> makes it easier to file bugs for snaps you have installed
[16:15]  * kenvandine files bug
[16:15] <Chipaca> kenvandine: if the snap has contact info it will be shown
[16:16] <Chipaca> hmmm
[16:16] <Chipaca> OTOH sideinfo has contact
[16:16] <Chipaca> hmm
[16:16] <Chipaca> something's wrong
[16:17] <Chipaca> kenvandine: maybe i lied, let me test this
[16:17] <popey_> :)
[16:17] <kenvandine> :)
[16:19] <Chipaca> sudo jq '.data.snaps."gnome-system-monitor".sequence[0].contact' /var/lib/snapd/state.json
[16:19] <Chipaca> kenvandine: ^ what does that say?
[16:19] <kenvandine> Chipaca: null
[16:19] <kenvandine> could it be that when i installed the snap there wasn't contact info?
[16:20] <Chipaca> kenvandine: and if you change the [0] to []| ?
[16:20] <kenvandine> null null null
[16:21] <Chipaca> kenvandine: so, i was wrong, and we do fetch the contact on install / refresh
[16:21] <Chipaca> kenvandine: (i was confused because i'm easily confused)
[16:21] <kenvandine> :)
[16:22] <Chipaca> kenvandine: if you were to remove it and reinstall it you should get your contact info (in both info and in that jq)
[16:22]  * Chipaca tries to remember if doing it with a cached snap will still ping the store, and hopes it does
[16:24] <kenvandine> Chipaca: indeed that works
[16:24] <Chipaca> k
[16:24] <kenvandine> should it work after refresh too?
[16:24] <Chipaca> kenvandine: after a refresh that fetches a new snap, yes
[16:24] <kenvandine> ok
[16:24] <Chipaca> kenvandine: it should also work after a refresh that doesn't refresh the snap, but that's unplanned work
[16:25] <Chipaca> (the store does actually tell us all this info just to say "no refreshes dude")
[16:25] <kenvandine> so basically this won't really help users until i publish updates for all the snaps
[16:25] <Chipaca> (granted it tells us because we ask for it)
[16:25] <kenvandine> although, i've refreshed most of my snaps since updating the contact fields
[16:26] <kenvandine> refreshed == rebuilt
[16:26] <kenvandine> so shouldn't be too bad
[16:26] <Chipaca> kenvandine: yes (and a bug about refreshing the fields on refresh would be useful)
[16:27] <Chipaca> not that there's any hope of slipping this in before 2040
[16:27] <Chipaca> or is that 20.4
[16:27] <Chipaca> :)
[16:27] <kenvandine> 21 years, not bad :)
[16:27]  * Chipaca meant 20.04 but ¯\_(ツ)_/¯ 
[16:28] <Chipaca> kenvandine: I might even have saved up enough for a deposit on a house by then! woo
[16:28] <kenvandine> :)
[16:29] <Chipaca> anyway, HTH, sorry it couldn't be entirely good news
[16:29] <kenvandine> no worries
[16:29] <Chipaca> kenvandine: could you add "double-check contact fields" to the getting-snaps-ready-for-seeding checklist?
[16:30] <kenvandine> in a bug report?
[16:30] <Chipaca> kenvandine: I have no idea -- this was a mythical checklist i imagined was used to vet snaps before they landed on the iso
[16:31] <kenvandine> lol
[16:31] <kenvandine> ok
[16:31] <kenvandine> i see what you mena
[16:31] <kenvandine> mean
[16:31] <kenvandine> this time i was confused
[16:31] <Chipaca> oh no
[16:31] <Chipaca> it's contagious
[16:31] <Chipaca> run
[16:31] <kenvandine> :)
[16:37] <zyga> oh
[16:37] <zyga> pedronis: is this expected? https://www.irccloud.com/pastebin/KcYvl1zT/
[16:38] <zyga> there's some more odd stuff
[16:38] <zyga> - Download snap "snapd" (3028) from channel "stable" (Get https://api.snapcraft.io/api/v1/snaps/download/PMrrV4ml8uWuEUDBT8dSGnKUYbevVhc4_3028.snap: x509: certificate signed by unknown authority (possibly because of "crypto/rsa: verification error" while trying to verify candidate authority certificate "Let's Encrypt Authority X3"))
[16:39] <pedronis> that's SSL issues
[16:39] <pedronis> worth poking the store people if it persists
[16:39] <zyga> OK
[16:42] <zyga> Pharaoh_Atem: hey, did you recently fix this https://bugzilla.redhat.com/show_bug.cgi?id=1707422 ?
[16:55] <Chipaca> cachio: is snapd#6541 still a thing we want?
[16:56] <mup> PR #6541: tests: change how xdg-desktop-portal is prepared and restored for desktop-portal-* tests <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/6541>
[17:14] <cachio> Chipaca, yes, I am testing a fic for the document-portal-activation test
[17:15] <cachio> because it is failing since I am installing the xdg-desktop-portal package on the prepare
[17:16] <cachio> I'll push a fix today
[17:27] <Chipaca> cachio: k
[19:26] <mup> PR snapd#6852 closed: cmd/snap-update-ns: merge apply functions <Created by zyga> <Merged by zyga> <https://github.com/snapcore/snapd/pull/6852>
[19:29] <mup> PR snapd#6854 opened: cmd/snap-update-ns: rename applyFstab to executeMountProfileUpdate <Simple 😃> <Created by zyga> <https://github.com/snapcore/snapd/pull/6854>
[19:37] <mup> PR snapd#6842 closed: tests: fix how the base snap are deleted when there are multiple to deleted on reset <Simple 😃> <Created by sergiocazzolato> <Merged by sergiocazzolato> <https://github.com/snapcore/snapd/pull/6842>
[19:54] <mup> PR snapcraft#2561 opened: Promote branches fix <Created by sergiusens> <https://github.com/snapcore/snapcraft/pull/2561>
[19:58] <zyga> jdstrand: hey, can you please look at (3 line) https://github.com/snapcore/snapd/pull/6846
[19:58] <mup> PR #6846: cmd/snap-confine: unshare per-user mount ns once <Bug> <Simple 😃> <Created by zyga> <https://github.com/snapcore/snapd/pull/6846>
[19:59] <zyga> jdstrand: oh, and perhaps if you are curious, we've solved the issue I showed to you that other evening
[19:59] <zyga> jdstrand: it's all described here https://bugs.launchpad.net/snapd/+bug/1828354
[19:59] <zyga> jdstrand: I'm working on fixing that with tests for all the variants now
[19:59] <mup> Bug #1828354: mount event propagation on snapd-mounted tmpfs is incorrect <snapd:In Progress by zyga> <https://launchpad.net/bugs/1828354>
[20:25] <jdstrand> zyga: nice
[20:28] <mup> PR snapd#6855 opened: overlord: implement store switch remodeling  <Created by pedronis> <https://github.com/snapcore/snapd/pull/6855>