=== chihchun is now known as chihchun_afk
=== chihchun_afk is now known as chihchun
=== chihchun is now known as chihchun_afk
=== chihchun_afk is now known as chihchun
=== chihchun is now known as chihchun_afk
zygaGood morning (perhaps)05:10
zygamborzecki: hey05:21
zygaHow was Easter?05:22
mborzeckizyga: too short :)05:22
zygaToday will be full of stuff :/05:23
zygaIn the bad sense I suspect05:23
mborzeckizyga: hmm https://forum.snapcraft.io/t/disabling-seccomp-sandbox-where-a-buggy-golang-seccomp-is-used/1105405:24
zygaYeah, that is one05:24
mborzeckizyga: do you know if the version in 29 is ok?05:25
zygaI didn’t check yet05:26
zygaI will be in the office at around 805:27
mborzeckibrb, got to reboot, new kernel05:29
mborzeckizyga: has the mystery of broken seed.yaml been explained?05:35
mborzeckizyga: 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 nils05:45
=== chihchun_afk is now known as chihchun
zygaNo, we don’t know anything more yet06:07
=== chihchun is now known as chihchun_afk
mborzeckizyga: i did a clean install with the alpha iso, no issues06:11
zygaWe need to talk to foundations to ask who can help with the installer06:12
zygaWe don’t know how the seed is made06:12
mborzeckizyga: definitely the format is slightly awkward, so for instance this variant causes a nil https://paste.ubuntu.com/p/9N7CXZRtwv/06:15
zygaThis is regular yaml06:17
zygaNothing special06:17
mborzeckizyga: fwiw, we could have used SeedSnap instead of a pointer06:17
zygaNot sure why we did06:18
=== chihchun_afk is now known as chihchun
mborzeckii'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 commit06:21
mborzeckididn't know you could embed lua in rpm macros06:23
zygaI will look at a content bug06:24
mborzeckizyga: something new?06:24
zygaReported in response to a theme bug06:30
zygamborzecki: ok, in the office now06:35
zygaobviously kids didn't want to go with the dog so I had to06:35
zygamborzecki: nothing in recent memory has made them more lazy than the teacher's strike06:36
zygamborzecki: is mvo working today?06:39
=== chihchun is now known as chihchun_afk
mborzeckizyga: idk06:42
mborzeckizyga: fedora's updated libseccomp-gois exactly 0.9 release, so the commit with the fix is not included06:43
mborzeckizyga: filed a bug in fedora https://bugzilla.redhat.com/show_bug.cgi?id=170216906:58
mborzeckion centos we used vendored packages06:58
mborzeckizyga: 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 all07:03
zygamborzecki: jdstrand will join our standup today07:04
zygamborzecki: perhaps discuss with him then07:04
mborzeckizyga: 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
zygamborzecki: eh :)07:04
zygathat's one rabbit hole IMOI07:04
mborzeckizyga: 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 same07:06
zygayes, I was thinking about this earlier, all the effort we did07:06
zygait is really insufficient07:06
mborzeckizyga: or we could get a pass from fesco or other body and just bundle that dep? :)07:07
zygamborzecki: good luck to get that approved in the next 60 days ;)07:07
mborzeckianyways, 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
zygamborzecki: it cannot catch up07:08
zygamborzecki: but that's a separate conversation07:08
=== pstolowski|afk is now known as pstolowski
om26erHi! 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:10
mborzeckizyga: better had with a glass of beer :P (we should make a list of beer-topics fwiw)07:11
mborzeckipstolowski: hey07:11
zygamborzecki: haha, a trello lane :)07:11
zygapstolowski: good morning, how are you doing?07:11
zygapstolowski: had enough mazurki over weekend? :-)07:11
zygaom26er: yes07:12
pstolowskizyga: i'm fine, and satiated :)07:12
pstolowskihow are you guys?07:13
zygapstolowski: depressed a little07:13
zygapstolowski: floodgates of bugs07:13
zygapstolowski: I want holidays that last 4 days :)07:13
pstolowskioh... i need to catch up07:14
pstolowskimborzecki: can you take a look at https://github.com/snapcore/snapd/pull/6728? that could land07:15
mborzeckipstolowski: sure07:19
zygabugs, bugs everywhere07:22
zygathere is something wrong with slot attrs07:22
* zyga wants snap plugs and snap slots07:29
zygait's infuriating to debug07:29
mborzeckizyga: shouldn't mei rule be /dev/mei[0-9]* ?07:35
zygamborzecki: why?07:35
mborzeckizyga: because i don't see + in the manpage :)07:35
zygamborzecki: ?07:35
zygamborzecki: in apparmor re?07:35
mborzeckizyga: yeah, the manpage is not too useful, but i don't see anything that uses +07:36
zygamborzecki: grep for + in interfaces07:36
zygaperhaps it's a common bug07:36
zygaapparmor RE sucks a little for being custom07:36
zygazyga@fyke:~/Documents/go/src/github.com/snapcore/snapd/cmd/snapd$ sudo ./snapd07:37
zygaAppArmor status: apparmor is enabled and all features are available07:37
zygacannot run daemon: cannot obtain snap-seccomp version information: error: unsupported argument "version-info"07:37
zygamborzecki: how do I run snapd now?07:37
mborzeckizyga: build snap-seccomp and put it in the same dir07:37
zygaa bit meh07:37
zyganot a great day07:40
zygapstolowski: we have a serious bug in interfaces07:40
pstolowskizyga: uh, what is it?07:41
zygapstolowski: hold on, let me prepare everything first07:41
mborzeckizyga: 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
zygamborzecki: I disagree07:41
zygamborzecki: zyga@fyke:~/Documents/go/src/github.com/snapcore/snapd/interfaces/builtin$ git grep ']+'07:41
zygafor me it shows much more than one item07:42
zygaah, sorry07:42
zyga we use go REs as well07:42
mborzeckithose are from go07:42
zygamborzecki: sanity check, apparmor, use of + is a bug?07:42
mborzeckizyga: for all we know the parser does not complain :)07:43
zygamborzecki: the parser sucks07:43
mborzeckizyga: fwiw, all other patterns use *07:43
mborzeckii'll open a PR07:43
zygamborzecki: thanks, assign the bug to yourself too07:43
mborzeckizyga: ^^07:52
mborzeckiback to pstolowski's review07:52
zygapstolowski: can you HO?07:52
zygapstolowski: need your eyes on something07:52
zygaI'm in standup07:53
zygamborzecki: wanna see?07:53
=== chihchun_afk is now known as chihchun
cjwatsondiddledan: 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 YAML07:59
Chipacamborzecki: 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
zygaChipaca: -108:29
zygaChipaca: hey08:29
mborzeckiChipaca: 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:32
zygaChipaca: fun day continues https://bugs.launchpad.net/snapd/+bug/1825883/comments/408:33
mborzeckiChipaca: unforunately apparmor.d manpage is most useless explaining AARE :/08:33
diddledanthanks cjwatson08:34
zygaChipaca, mborzecki: my personal experience is that the documentation is only partial there08:34
zygathe only way to learn more is to look at what the kernel reads08:34
zygawhich is non-trivial08:34
mborzeckiChipaca: for reference, we allow `/dev/tty[0-9]* rw,` for x11 and other rules also follow [0-9]* patterns08:34
Chipacamborzecki: /dev/tty is a thing tho08:35
zygaI 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 is08:35
* zyga wrote a partial apparmor decompiler at one point in the past08:35
mborzeckiChipaca: fair enough, /dev/ttyUSB is not :) /dev/tty{USB,ACM}[0-9]*08:36
zygawindy dat08:36
* zyga wants a set of debug commands that just display the data from the state 08:38
mborzeckizyga: hm maybe we'll have libseccomp-golang updated in fedora sooner than expected -> https://bugzilla.redhat.com/show_bug.cgi?id=170216908:38
mborzeckiwhich would be great imo :)08:39
zygamborzecki: report a debian bug too please08:41
zygapstolowski: so08:44
zygapstolowski: the problem is that snap connect is storing information that is never updated after the fact08:44
mborzeckisore & forget08:45
mborzeckistore & forget08:45
pstolowskizyga: 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
pstolowskithat's bad08:49
zygayes :)08:49
zygalooking at what may be the best way forward now08:49
zygabut first I want to add a system08:49
zygathat records a trace of revision in the repo08:49
zygaso repo will know what the revision of each snap is08:50
zygaand will panic if those disagree08:50
pstolowskizyga: we need to think, perhaps we should re-run all the interface hooks, that means disconnecting first and then re-connecting08:52
zygapstolowski: I think we need to take a step back and write some invariants08:53
zygapstolowski: hooks are both niche and complex08:53
zygapstolowski: I'd like to support them but going through the hook door first might just confuse us more08:53
pstolowskizyga: maybe we should take step back and reconsider storing static attrs in state, and only use that for hotplug slots08:57
zygapstolowski: maybe, I will add consistency checks to the repo and see what we get08:58
pstolowskizyga: 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:11
pstolowskizyga: if _, err := m.repo.Connect(connRef, conn.StaticPlugAttrs, conn.DynamicPlugAttrs, conn.StaticSlotAttrs, conn.DynamicSlotAttrs, nil); err != nil { ... in reloadConnections09:13
zygapstolowski: I agree this is where the error is triggered09:14
zygapstolowski: I'm not yet sure what is the best way to fix it09:14
popeyThis was reported to me by multiple people over the weekend09:14
popeyFeels somewhat critical (if it's not already known09:14
zygapopey: this is a duplicate of another bug09:14
popeyit's biting a lot of people because 19.04 just came out and lots of people are installing systems with no snaps pre-installed09:14
zygamarked as such now09:15
zygapopey: I agree09:15
zygapopey: just ETOOMANYBUGS09:15
diddledanbugs.reduce((status) => status.ignore());09:16
diddledanthat should be a map really09:17
diddledanI suck at jokes09:17
popeynerd jokes are best jokes09:18
popey(for nerds)09:18
zygapstolowski: hmm10:05
zyganever mind10:06
=== chihchun is now known as chihchun_afk
zygapstolowski: question, open interface manager's handlers.go and go to doConnect10:15
zygapstolowski: inside that function look at the call to repo.Connect10:16
zygapstolowski: it takes plugDynamicAttrs and slotDynamicAttrs, not taking static attrs (not sure why)10:16
pstolowskizyga: looking10:17
zygapstolowski: 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 Connect10:17
zygawhy is that?10:17
* zyga is dumb10:18
zygasorry, this is correct10:18
zygawe save what the connection gave us10:18
zygapstolowski: sorry for wasting your time10:18
pstolowskizyga: no worries. NewConnectedPlug and NewConnectedSlot are special-cased for nils and take static attrs from plug/slot info10:18
pstolowskizyga: 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 know10:22
zygapstolowski: 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
pstolowskizyga: 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 problem10:25
zygapstolowski: no worries :)10:25
zygapstolowski: the repo is not the problem10:25
zygapstolowski: I made that but id doesn't pick up the problem (obviously)10:26
zygapstolowski: I'm making connState rev aware10:26
zygapstolowski: we run the refresh hooks right?10:46
zygapstolowski: but on snap refresh we don't run interface hooks, do we?10:46
pstolowskizyga: correct, except for autoconnects, those can run hooks10:48
zygahow do you mean?10:48
zygapstolowski: so, I think we have two bugs now10:50
pstolowskizyga: as part of install/refresh we run "autoconnect" to connect new interfaces (if applicable); it will run hooks10:50
zygaI see10:50
zygathe first bug is that the static attributes are stale, that's plain and obvious10:50
zygathe 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 observe10:51
zygaif 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
zygapstolowski: I will break now, take a walk and think about this10:51
zygapstolowski: my gut feeling is that we need to rethink refresh and interfaces10:52
zygapstolowski: we can "hack" to fix this issue10:52
zygapstolowski: but the core issue is more complex and I think what we do today is simply inconsistent with itself10:52
zygapstolowski: we need to find something that is correct and consistent10:52
zygapstolowski: and also not annoying or difficult to use for users10:52
zygapstolowski: (e.g. do you see a disconnect hook and a connect hook when your peer refreshes?)10:53
zygapstolowski: it's extra tricky for content because the snap will synchronously see the old and the new state and nothing in beteween10:53
zygapstolowski: so the hook is really "hey buddy, it changed" without a way to see the prior state10:53
zygapstolowski: I changed stuff enough in my branch to detect when the connection state is inconsistent with the repository10:54
pstolowskizyga: 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-snap10:56
pstolowskizyga: that's in ifacestate.connect(..) that creates interface hook tasks10:56
zygapstolowski: forget autoconnect10:57
zygapstolowski: when you refresh a snap with an existing connection, the peers of that snap *never know about the refresh*10:57
zygapstolowski: and attributes encode the revision10:57
zygapstolowski: so if you use hooks to update internal snap config you are SOL10:57
zygapstolowski: I pushed some patches to https://github.com/snapcore/snapd/compare/master...zyga:fix/lp-1825883?expand=111:00
pstolowskizyga: i'll show you something11:00
pstolowskizyga: https://github.com/snapcore/snapd/pull/5200/11:00
pstolowskizyga: almost one year ago ;)11:00
pstolowskizyga: see // on refresh re-connect existing connections and run their interface hooks (if applicable)11:00
pstolowskizyga: there is even a spread test11:01
zygapstolowski: that's good in itself11:01
zygapstolowski: does this update connection state as well?11:01
zygapstolowski: https://github.com/snapcore/snapd/commit/fe3498a40ac38857142525f781ea808447b19c5e this errors whenever we would take a decision based on old data11:02
zygapstolowski: I'm happy to chat over TG but I need a walk now11:02
pstolowskizyga: yes that old branch updates connstate because it disconnectes and then reconnects, so it runs disconnect hooks and then connect hooks11:04
pstolowskizyga: it was parked back then :(11:04
zygapstolowski: I’ll run my branch on all current tests to see if something is already affected11:06
zygapstolowski: https://twitter.com/ismonkeyuser/status/112056869916813312011:20
zygamborzecki: can you edit https://github.com/snapcore/snapd/pull/6762 to reference the bug report please11:49
mborzeckizyga: was there one?11:50
zygamborzecki: I think so, no? was that just on the forum>11:50
zygamborzecki: then at least a Forum: ... entry next to signed-off-by11:51
mborzeckizyga: i've only seen the forum post11:51
mborzeckizyga: so you mean the commit message?11:51
zygamborzecki: yeah, or the PR merge message11:54
zygapstolowski: I patched my branch to only report an error when the static attributes actually differ11:57
zygapstolowski: we still have tests failing :/11:57
zygapstolowski: so we currently may depend on incorrect behivior11:58
pstolowskii'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 back12:04
* pstolowski is kicking himself for not realizing significance of landing or not landing that old PR12:06
jdstrandmborzecki: fyi, apparmor.d *does* have AARE: "AARE = ?*[]{}^"\nSee below for meanings"12:08
mborzeckijdstrand: right, but the 'see below for meanings' is so informative12:09
jdstrandmborzecki: the bit under Globbing?12:11
mborzeckijdstrand: right, that one12:12
jdstrandhuh. I find that clear...12:12
jdstrandhow would you like to see it changed? I can fix it upstream12:12
Chipacajdstrand: in particular, it says «*   can substitute for any number of characters, excepting '/'», but doesn't say whether "any number" includes 012:13
Chipacajdstrand: i.e. does /dev/foo[0-9]* match /dev/foo12:13
jdstrandit's a glob12:13
jdstrandso, 'no'12:14
jdstrandbut 'yes', I see what you mean12:14
mborzeckijdstrand: also it says globbing, vs me expecting something mentioning AARE (because that's how it's named above)12:15
jdstrandI 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 be12:15
jdstrandmborzecki: yes. up above it says See below without referencing Globbing, and down below it talks about globbing but not AARE12:15
jdstrandmborzecki: you probably saw that I approved and merged the pr12:17
jdstrandmborzecki: thanks. I don't know how I missed that :\12:17
mborzeckijdstrand: mhm, thanks!12:17
zygajdstrand: hey12:21
zygajdstrand: fun day12:21
=== jospoortvliet_ is now known as jospoortvliet
=== chihchun_afk is now known as chihchun
Chipacajdstrand: eoan pronuciation guide: https://www.youtube.com/watch?v=iQXMdR0XDT813:16
jdstrandChipaca: hahaha13:18
Son_Gokumborzecki, how close are we to 2.39 release?13:45
mborzeckiSon_Goku: idk, i'd say not far, zyga do you know?13:45
zygaSon_Goku: hard to say13:45
zygaSon_Goku: I'd say we will postpone because of bugs13:45
zygabut mvo is out so I cannot say with certainty13:46
zygaSon_Goku: it's  been a week of bugs13:46
zygaroadmr: lol :)13:50
* zyga runs an errand, see you later13:50
* cachio afk14:03
om26erHi! 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:26
om26ercase in point https://forum.snapcraft.io/t/please-allow-auto-connect-of-display-control-interface-for-deskconn/1083115:27
Chipacaom26er: https://forum.snapcraft.io/t/process-for-aliases-auto-connections-and-tracks/45515:31
=== om26er89 is now known as om26er
om26erthanks Chipaca15:33
Chipacaom26er: i've pinged @reviwers on that one as it's high time they looked at it15:33
Chipacaom26er: but that topic explains # of votes, timeline, etc15:33
om26erChipaca cool, there is already a new +1 on that one ;-)15:38
ChipacaI wonder if today's xkcd is about libseccomp15:55
* zyga looks at xkcd16:34
=== pstolowski is now known as pstolowski|afk
zygaI guess  I should EOD17:26
=== chihchun is now known as chihchun_afk

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!