[05:10] <zyga> Good morning (perhaps)
[05:15] <mborzecki> morning
[05:21] <zyga> mborzecki: hey
[05:22] <zyga> How was Easter?
[05:22] <mborzecki> zyga: too short :)
[05:23] <zyga> Today will be full of stuff :/
[05:23] <zyga> In the bad sense I suspect
[05:24] <mborzecki> zyga: hmm https://forum.snapcraft.io/t/disabling-seccomp-sandbox-where-a-buggy-golang-seccomp-is-used/11054
[05:24] <zyga> Yeah, that is one
[05:25] <mborzecki> zyga: do you know if the version in 29 is ok?
[05:26] <zyga> I didn’t check yet
[05:27] <zyga> I will be in the office at around 8
[05:29] <mborzecki> brb, got to reboot, new kernel
[05:35] <mborzecki> zyga: has the mystery of broken seed.yaml been explained?
[05:45] <mborzecki> zyga: seed.yaml from that alpha image has a slightly weird format like so https://paste.ubuntu.com/p/vM2MZjZN2t/ but the test i added to check that in the code seems to work fine, no nils
[06:07] <zyga> No, we don’t know anything more yet
[06:11] <mborzecki> zyga: i did a clean install with the alpha iso, no issues
[06:12] <zyga> We need to talk to foundations to ask who can help with the installer
[06:12] <zyga> We don’t know how the seed is made
[06:15] <mborzecki> zyga: definitely the format is slightly awkward, so for instance this variant causes a nil https://paste.ubuntu.com/p/9N7CXZRtwv/
[06:17] <zyga> Yeah
[06:17] <zyga> This is regular yaml
[06:17] <zyga> Nothing special
[06:17] <mborzecki> zyga: fwiw, we could have used SeedSnap instead of a pointer
[06:17] <zyga> Yes
[06:18] <zyga> Not sure why we did
[06:21] <mborzecki> i'll check f29 golang seccomp next, according to bodhi it was updated recent'y, but i'm not sure whether that's 0.9 tag exactly or some other commit
[06:23] <mborzecki> didn't know you could embed lua in rpm macros
[06:24] <zyga> Thanks
[06:24] <zyga> I will look at a content bug
[06:24] <mborzecki> zyga: something new?
[06:30] <zyga> Yes
[06:30] <zyga> Reported in response to a theme bug
[06:35] <zyga> mborzecki: ok, in the office now
[06:35] <zyga> obviously kids didn't want to go with the dog so I had to
[06:36] <zyga> mborzecki: nothing in recent memory has made them more lazy than the teacher's strike
[06:39] <zyga> mborzecki: is mvo working today?
[06:42] <mborzecki> zyga: idk
[06:43] <mborzecki> zyga: fedora's updated libseccomp-gois exactly 0.9 release, so the commit with the fix is not included
[06:58] <mborzecki> zyga: filed a bug in fedora https://bugzilla.redhat.com/show_bug.cgi?id=1702169
[06:58] <zyga> ack
[06:58] <mborzecki> on centos we used vendored packages
[07:03] <mborzecki> zyga: so i'm thinking, fedora & debian are the only 2 distros that do not bundle golang deps, once they are fixed, we may not need to disable seccomp dynamically at all
[07:04] <zyga> mborzecki: jdstrand will join our standup today
[07:04] <zyga> mborzecki: perhaps discuss with him then
[07:04] <mborzecki> zyga: and a silly thought, since we already do the snap-seccomp version-info magic, maybe we could extend this to cover bpf output geenrated by the lib too?
[07:04] <zyga> mborzecki: eh :)
[07:04] <zyga> that's one rabbit hole IMOI
[07:06] <mborzecki> zyga: i mean the problem is that even if they update snap-seccomp bindings, we may not renerate the profiles, because lib version and supported syscalls could stay the same
[07:06] <zyga> yes, I was thinking about this earlier, all the effort we did
[07:06] <zyga> it is really insufficient
[07:07] <mborzecki> zyga: or we could get a pass from fesco or other body and just bundle that dep? :)
[07:07] <zyga> mborzecki: good luck to get that approved in the next 60 days ;)
[07:08] <mborzecki> anyways, golang deps are still arguably silly and don't fit the disto world (or iow the distro world did not catch up with golang/rust/node/python package handling yet)
[07:08] <zyga> mborzecki: it cannot catch up
[07:08] <zyga> mborzecki: but that's a separate conversation
[07:10] <pstolowski> mornings!
[07:10] <om26er> Hi! I opened that one https://forum.snapcraft.io/t/auto-connection-request-for-gpio-interface/11013 -- is that under the current store rules possible ?
[07:11] <mborzecki> zyga: better had with a glass of beer :P (we should make a list of beer-topics fwiw)
[07:11] <mborzecki> pstolowski: hey
[07:11] <zyga> mborzecki: haha, a trello lane :)
[07:11] <zyga> pstolowski: good morning, how are you doing?
[07:11] <zyga> pstolowski: had enough mazurki over weekend? :-)
[07:12] <zyga> om26er: yes
[07:12] <pstolowski> zyga: i'm fine, and satiated :)
[07:13] <pstolowski> how are you guys?
[07:13] <zyga> pstolowski: depressed a little
[07:13] <zyga> pstolowski: floodgates of bugs
[07:13] <zyga> pstolowski: I want holidays that last 4 days :)
[07:14] <pstolowski> oh... i need to catch up
[07:15] <pstolowski> mborzecki: can you take a look at https://github.com/snapcore/snapd/pull/6728? that could land
[07:19] <mborzecki> pstolowski: sure
[07:22] <zyga> mmmmm
[07:22] <zyga> hmmm
[07:22] <zyga> bugs, bugs everywhere
[07:22] <zyga> there is something wrong with slot attrs
[07:29]  * zyga wants snap plugs and snap slots
[07:29] <zyga> it's infuriating to debug
[07:35] <mborzecki> zyga: shouldn't mei rule be /dev/mei[0-9]* ?
[07:35] <zyga> mborzecki: why?
[07:35] <mborzecki> zyga: because i don't see + in the manpage :)
[07:35] <zyga> mborzecki: ?
[07:35] <zyga> mborzecki: in apparmor re?
[07:36] <mborzecki> zyga: yeah, the manpage is not too useful, but i don't see anything that uses +
[07:36] <zyga> mborzecki: grep for + in interfaces
[07:36] <zyga> perhaps it's a common bug
[07:36] <zyga> apparmor RE sucks a little for being custom
[07:37] <zyga> zyga@fyke:~/Documents/go/src/github.com/snapcore/snapd/cmd/snapd$ sudo ./snapd
[07:37] <zyga> AppArmor status: apparmor is enabled and all features are available
[07:37] <zyga> cannot run daemon: cannot obtain snap-seccomp version information: error: unsupported argument "version-info"
[07:37] <zyga> mborzecki: how do I run snapd now?
[07:37] <mborzecki> zyga: build snap-seccomp and put it in the same dir
[07:37] <zyga> ok
[07:37] <zyga> a bit meh
[07:39] <zyga> eh
[07:40] <zyga> not a great day
[07:40] <zyga> pstolowski: we have a serious bug in interfaces
[07:41] <pstolowski> zyga: uh, what is it?
[07:41] <zyga> pstolowski: hold on, let me prepare everything first
[07:41] <mborzecki> zyga: heh, looks like a bug in patterns, we have /dev/sr[0-9]* /dev/rtc[0-9]* /dev/input/event[0-9]* but only /dev/mei[0-9]+
[07:41] <zyga> mborzecki: I disagree
[07:41] <zyga> mborzecki: zyga@fyke:~/Documents/go/src/github.com/snapcore/snapd/interfaces/builtin$ git grep ']+'
[07:42] <zyga> for me it shows much more than one item
[07:42] <zyga> ah, sorry
[07:42] <zyga>  we use go REs as well
[07:42] <mborzecki> those are from go
[07:42] <zyga> mborzecki: sanity check, apparmor, use of + is a bug?
[07:43] <mborzecki> zyga: for all we know the parser does not complain :)
[07:43] <zyga> mborzecki: the parser sucks
[07:43] <mborzecki> zyga: fwiw, all other patterns use *
[07:43] <mborzecki> i'll open a PR
[07:43] <mborzecki> meh
[07:43] <zyga> mborzecki: thanks, assign the bug to yourself too
[07:51] <mborzecki> https://github.com/snapcore/snapd/pull/6762
[07:52] <mborzecki> zyga: ^^
[07:52] <mborzecki> back to pstolowski's review
[07:52] <zyga> pstolowski: can you HO?
[07:52] <zyga> pstolowski: need your eyes on something
[07:53] <zyga> I'm in standup
[07:53] <zyga> mborzecki: wanna see?
[07:59] <cjwatson> diddledan: in case you didn't notice, https://github.com/diddlesnaps/gog-galaxy-wine/blob/master/snap/snapcraft.yaml has conflict markers in it and is thus failing to parse as YAML
[08:28] <Chipaca> mborzecki: I don't know AARE; does * there mean "0 or more"? (the manpage is not clear to me -- it says "any number" which might include 0 or not)
[08:28] <zyga> https://github.com/snapcore/snapd/compare/master...zyga:fix/lp-1825883?expand=1
[08:29] <zyga> Chipaca: -1
[08:29] <zyga> ;)
[08:29] <zyga> Chipaca: hey
[08:29] <Chipaca> 'sup
[08:32] <mborzecki> Chipaca: i think it's more like glob for file paths, so dev /dev/mei[0-9] matches mei0-mei9 followed by whatever that's not /
[08:33] <zyga> Chipaca: fun day continues https://bugs.launchpad.net/snapd/+bug/1825883/comments/4
[08:33] <mborzecki> Chipaca: unforunately apparmor.d manpage is most useless explaining AARE :/
[08:34] <diddledan> thanks cjwatson
[08:34] <zyga> Chipaca, mborzecki: my personal experience is that the documentation is only partial there
[08:34] <zyga> the only way to learn more is to look at what the kernel reads
[08:34] <zyga> which is non-trivial
[08:34] <mborzecki> Chipaca: for reference, we allow `/dev/tty[0-9]* rw,` for x11 and other rules also follow [0-9]* patterns
[08:35] <Chipaca> mborzecki: /dev/tty is a thing tho
[08:35] <zyga> I failed mid way on that because of the representation of AARE is only documented in the C implementation as imperative code, there's no comment as to what the representation is
[08:35]  * zyga wrote a partial apparmor decompiler at one point in the past
[08:36] <mborzecki> Chipaca: fair enough, /dev/ttyUSB is not :) /dev/tty{USB,ACM}[0-9]*
[08:36] <zyga> windy dat
[08:36] <zyga> *day
[08:38]  * zyga wants a set of debug commands that just display the data from the state 
[08:38] <mborzecki> zyga: hm maybe we'll have libseccomp-golang updated in fedora sooner than expected -> https://bugzilla.redhat.com/show_bug.cgi?id=1702169
[08:39] <mborzecki> which would be great imo :)
[08:41] <zyga> mborzecki: report a debian bug too please
[08:44] <zyga> pstolowski: so
[08:44] <zyga> pstolowski: the problem is that snap connect is storing information that is never updated after the fact
[08:45] <mborzecki> sore & forget
[08:45] <mborzecki> store & forget
[08:49] <pstolowski> zyga: right. we never call doConnect again, and with the new logic of storing static attrs in the state we don't refresh them, in the old implementation setup-profiles would pick the new attributes right?
[08:49] <zyga> yes
[08:49] <pstolowski> damn
[08:49] <pstolowski> that's bad
[08:49] <zyga> yes :)
[08:49] <zyga> looking at what may be the best way forward now
[08:49] <zyga> but first I want to add a system
[08:49] <zyga> that records a trace of revision in the repo
[08:50] <zyga> so repo will know what the revision of each snap is
[08:50] <zyga> and will panic if those disagree
[08:52] <pstolowski> zyga: we need to think, perhaps we should re-run all the interface hooks, that means disconnecting first and then re-connecting
[08:53] <zyga> pstolowski: I think we need to take a step back and write some invariants
[08:53] <zyga> pstolowski: hooks are both niche and complex
[08:53] <zyga> pstolowski: I'd like to support them but going through the hook door first might just confuse us more
[08:57] <pstolowski> zyga: maybe we should take step back and reconsider storing static attrs in state, and only use that for hotplug slots
[08:58] <zyga> pstolowski: maybe, I will add consistency checks to the repo and see what we get
[09:11] <pstolowski> zyga: i think setupProfilesForSnap needs fixing. it already does repo.Remove(snap) and Re-add(snap), and then reloadConnection which picks outdated attrs from state.
[09:13] <pstolowski> zyga: if _, err := m.repo.Connect(connRef, conn.StaticPlugAttrs, conn.DynamicPlugAttrs, conn.StaticSlotAttrs, conn.DynamicSlotAttrs, nil); err != nil { ... in reloadConnections
[09:14] <zyga> pstolowski: I agree this is where the error is triggered
[09:14] <zyga> pstolowski: I'm not yet sure what is the best way to fix it
[09:14] <popey> https://bugs.launchpad.net/snapd/+bug/1825956
[09:14] <popey> This was reported to me by multiple people over the weekend
[09:14] <popey> Feels somewhat critical (if it's not already known
[09:14] <zyga> popey: this is a duplicate of another bug
[09:14] <popey> ok
[09:14] <popey> it's biting a lot of people because 19.04 just came out and lots of people are installing systems with no snaps pre-installed
[09:15] <zyga> https://bugs.launchpad.net/snapd/+bug/1819318
[09:15] <zyga> marked as such now
[09:15] <zyga> popey: I agree
[09:15] <zyga> popey: just ETOOMANYBUGS
[09:16] <diddledan> bugs.reduce((status) => status.ignore());
[09:16] <diddledan> :-p
[09:17] <diddledan> that should be a map really
[09:17] <diddledan> I suck at jokes
[09:18] <popey> nerd jokes are best jokes
[09:18] <popey> (for nerds)
[09:18] <diddledan> :-)
[10:05] <zyga> pstolowski: hmm
[10:06] <zyga> never mind
[10:15] <zyga> pstolowski: question, open interface manager's handlers.go and go to doConnect
[10:16] <zyga> pstolowski: inside that function look at the call to repo.Connect
[10:16] <zyga> pstolowski: it takes plugDynamicAttrs and slotDynamicAttrs, not taking static attrs (not sure why)
[10:17] <pstolowski> zyga: looking
[10:17] <zyga> pstolowski: now the questsion is, a few lines below that, we create connState and store plug and slot static and dynamic attrs using conn.{Plug,Slot}.{Dynamic,Static}Attrs() instead of what we passed to Connect
[10:17] <zyga> why is that?
[10:18] <zyga> aah
[10:18]  * zyga is dumb
[10:18] <zyga> sorry, this is correct
[10:18] <zyga> we save what the connection gave us
[10:18] <zyga> pstolowski: sorry for wasting your time
[10:18] <pstolowski> zyga: no worries. NewConnectedPlug and NewConnectedSlot are special-cased for nils and take static attrs from plug/slot info
[10:22] <pstolowski> zyga: i'm thinking a fix i suggested above for setupprofiles/reload connections is a potential solution, i'm happy to work on this, just let me know
[10:23] <zyga> pstolowski: I'm still working on this, I'm not convinced that is enough (it would fix it but I want to be sure we've fixed all cases of this issue)
[10:23] <pstolowski> ok
[10:25] <pstolowski> zyga: if you're making repo rev-aware, then i think it would be great to keep it separate from actual bugfix (if possible), i think it's a bit orthogonal problem
[10:25] <zyga> pstolowski: no worries :)
[10:25] <zyga> pstolowski: the repo is not the problem
[10:26] <zyga> pstolowski: I made that but id doesn't pick up the problem (obviously)
[10:26] <zyga> pstolowski: I'm making connState rev aware
[10:46] <zyga> pstolowski: we run the refresh hooks right?
[10:46] <zyga> pstolowski: but on snap refresh we don't run interface hooks, do we?
[10:48] <pstolowski> zyga: correct, except for autoconnects, those can run hooks
[10:48] <zyga> hmm?
[10:48] <zyga> how do you mean?
[10:50] <zyga> pstolowski: so, I think we have two bugs now
[10:50] <pstolowski> zyga: as part of install/refresh we run "autoconnect" to connect new interfaces (if applicable); it will run hooks
[10:50] <zyga> I see
[10:50] <zyga> the first bug is that the static attributes are stale, that's plain and obvious
[10:51] <zyga> the dynamic attributes are equally out of date, if you setup a content connection hook you will most likely be able to see the revision in the path you observe
[10:51] <zyga> if you refresh stuff those will be stale equally (so snaps will not see the accurate paths if they use hooks to update them)
[10:51] <zyga> pstolowski: I will break now, take a walk and think about this
[10:52] <zyga> pstolowski: my gut feeling is that we need to rethink refresh and interfaces
[10:52] <zyga> pstolowski: we can "hack" to fix this issue
[10:52] <zyga> pstolowski: but the core issue is more complex and I think what we do today is simply inconsistent with itself
[10:52] <zyga> pstolowski: we need to find something that is correct and consistent
[10:52] <zyga> pstolowski: and also not annoying or difficult to use for users
[10:53] <zyga> pstolowski: (e.g. do you see a disconnect hook and a connect hook when your peer refreshes?)
[10:53] <zyga> pstolowski: it's extra tricky for content because the snap will synchronously see the old and the new state and nothing in beteween
[10:53] <zyga> pstolowski: so the hook is really "hey buddy, it changed" without a way to see the prior state
[10:54] <zyga> pstolowski: I changed stuff enough in my branch to detect when the connection state is inconsistent with the repository
[10:56] <pstolowski> zyga: for autoconnect we do plugStatic, slotStatic, err := initialConnectAttributes(st, plugSnapInfo, plugSnap, plugName, slotSnapInfo, slotSnap, slotName), that's what the hooks see, and that is from CurrentInfo() after link-snap
[10:56] <pstolowski> zyga: that's in ifacestate.connect(..) that creates interface hook tasks
[10:57] <zyga> pstolowski: forget autoconnect
[10:57] <zyga> pstolowski: when you refresh a snap with an existing connection, the peers of that snap *never know about the refresh*
[10:57] <zyga> pstolowski: and attributes encode the revision
[10:57] <zyga> pstolowski: so if you use hooks to update internal snap config you are SOL
[11:00] <zyga> pstolowski: I pushed some patches to https://github.com/snapcore/snapd/compare/master...zyga:fix/lp-1825883?expand=1
[11:00] <pstolowski> zyga: i'll show you something
[11:00] <pstolowski> zyga: https://github.com/snapcore/snapd/pull/5200/
[11:00] <pstolowski> zyga: almost one year ago ;)
[11:00] <pstolowski> zyga: see // on refresh re-connect existing connections and run their interface hooks (if applicable)
[11:01] <pstolowski> zyga: there is even a spread test
[11:01] <zyga> pstolowski: that's good in itself
[11:01] <zyga> pstolowski: does this update connection state as well?
[11:02] <zyga> pstolowski: https://github.com/snapcore/snapd/commit/fe3498a40ac38857142525f781ea808447b19c5e this errors whenever we would take a decision based on old data
[11:02] <zyga> pstolowski: I'm happy to chat over TG but I need a walk now
[11:04] <pstolowski> zyga: yes that old branch updates connstate because it disconnectes and then reconnects, so it runs disconnect hooks and then connect hooks
[11:04] <pstolowski> zyga: it was parked back then :(
[11:06] <zyga> pstolowski: I’ll run my branch on all current tests to see if something is already affected
[11:20] <zyga> pstolowski: https://twitter.com/ismonkeyuser/status/1120568699168133120
[11:21] <zyga> ;-)
[11:21] <pstolowski> :D
[11:49] <zyga> mborzecki: can you edit https://github.com/snapcore/snapd/pull/6762 to reference the bug report please
[11:50] <mborzecki> zyga: was there one?
[11:50] <zyga> mborzecki: I think so, no? was that just on the forum>
[11:51] <zyga> mborzecki: then at least a Forum: ... entry next to signed-off-by
[11:51] <mborzecki> zyga: i've only seen the forum post
[11:51] <mborzecki> zyga: so you mean the commit message?
[11:54] <zyga> mborzecki: yeah, or the PR merge message
[11:57] <zyga> pstolowski: I patched my branch to only report an error when the static attributes actually differ
[11:57] <zyga> pstolowski: we still have tests failing :/
[11:58] <zyga> pstolowski: so we currently may depend on incorrect behivior
[11:58] <zyga> *behaviour
[12:01] <pstolowski> uhm
[12:04] <pstolowski> i'm afraid the only correct way of dealing with this is to really re-run (reconnect) everything (like in my old PR, modulo comments from pedronis - namely the comment re hooks not knowning if it's initial connect vs reconnect). we need to discuss when he is back
[12:06]  * pstolowski is kicking himself for not realizing significance of landing or not landing that old PR
[12:08] <jdstrand> mborzecki: fyi, apparmor.d *does* have AARE: "AARE = ?*[]{}^"\nSee below for meanings"
[12:09] <mborzecki> jdstrand: right, but the 'see below for meanings' is so informative
[12:11] <jdstrand> mborzecki: the bit under Globbing?
[12:12] <mborzecki> jdstrand: right, that one
[12:12] <jdstrand> huh. I find that clear...
[12:12] <jdstrand> how would you like to see it changed? I can fix it upstream
[12:13] <Chipaca> jdstrand: in particular, it says «*   can substitute for any number of characters, excepting '/'», but doesn't say whether "any number" includes 0
[12:13] <Chipaca> jdstrand: i.e. does /dev/foo[0-9]* match /dev/foo
[12:13] <jdstrand> it's a glob
[12:14] <jdstrand> so, 'no'
[12:14] <jdstrand> but 'yes', I see what you mean
[12:14] <Chipaca> :-)
[12:15] <mborzecki> jdstrand: also it says globbing, vs me expecting something mentioning AARE (because that's how it's named above)
[12:15] <jdstrand> I mean it does say 'File resources may be specified with a globbing syntax similar to that used by popular shells, such as csh(1), bash(1), zsh(1)." but it isn't as clear as it be
[12:15] <jdstrand> mborzecki: yes. up above it says See below without referencing Globbing, and down below it talks about globbing but not AARE
[12:17] <jdstrand> mborzecki: you probably saw that I approved and merged the pr
[12:17] <jdstrand> mborzecki: thanks. I don't know how I missed that :\
[12:17] <mborzecki> jdstrand: mhm, thanks!
[12:21] <zyga> jdstrand: hey
[12:21] <zyga> jdstrand: fun day
[13:16] <Chipaca> jdstrand: eoan pronuciation guide: https://www.youtube.com/watch?v=iQXMdR0XDT8
[13:18] <jdstrand> Chipaca: hahaha
[13:45] <Son_Goku> mborzecki, how close are we to 2.39 release?
[13:45] <mborzecki> Son_Goku: idk, i'd say not far, zyga do you know?
[13:45] <zyga> Son_Goku: hard to say
[13:45] <zyga> Son_Goku: I'd say we will postpone because of bugs
[13:46] <zyga> but mvo is out so I cannot say with certainty
[13:46] <Son_Goku> okay
[13:46] <zyga> Son_Goku: it's  been a week of bugs
[13:48] <roadmr> 🐞🐜🐛
[13:50] <zyga> roadmr: lol :)
[13:50]  * zyga runs an errand, see you later
[14:03]  * cachio afk
[15:26] <om26er> Hi! Regarding requests for auto-connection of snap interfaces, how long does it generally take and also what number of +1's does it take for a request to go live ?
[15:27] <om26er> case in point https://forum.snapcraft.io/t/please-allow-auto-connect-of-display-control-interface-for-deskconn/10831
[15:31] <Chipaca> om26er: https://forum.snapcraft.io/t/process-for-aliases-auto-connections-and-tracks/455
[15:33] <om26er> thanks Chipaca
[15:33] <Chipaca> om26er: i've pinged @reviwers on that one as it's high time they looked at it
[15:33] <Chipaca> om26er: but that topic explains # of votes, timeline, etc
[15:38] <om26er> Chipaca cool, there is already a new +1 on that one ;-)
[15:55] <Chipaca> I wonder if today's xkcd is about libseccomp
[15:56] <jdstrand> heh
[16:34] <zyga> Re
[16:34]  * zyga looks at xkcd
[17:26] <zyga> I guess  I should EOD