[01:29] will you guys be trying to get snapcraft 2.39 into bionic RELEASE? [06:00] maaan [06:00] its Friday and we are deep in golang build issues === Guest23352 is now known as ogasawara [07:31] hello === zyga_ is now known as zyga [07:31] * zyga rebuilds systemd due to CVEs, meh [07:31] I made that branch we talked about [07:31] https://github.com/snapcore/snapd/pull/4641 [07:31] PR #4641: many: move mount code to osutil [07:31] and on top of that: https://github.com/snapcore/snapd/pull/4642 [07:31] PR #4642: many: add nfs-home flag to system-key [07:31] zyga: nice, thanks [07:31] both of those are red because of golang [07:32] * mvo looks [07:32] after 4641 we could remove some duplicate code that parsed mountinfo in osutil [07:38] zyga: that sounds good, let me check [07:42] zyga: fwiw, it fails because of undefine FindUID [07:42] it fails in cgo [07:42] zyga: and undefined FindGID [07:42] this is a cgo issue [07:42] zyga: it is? [07:43] it passes in normal go [07:43] (I didn't push this without testing0 [07:43] zyga: did this also break with the latest go update? [07:43] hmm, not sure, I didn't push anything else [07:43] hmm [07:43] but FindUID was always in that spot [07:44] mmm [07:44] ah [07:44] snap-update-ns had some exception to build with cgo, right? [07:45] so [07:45] quick fix [07:45] we don't need username lookup in mountentry.go [07:46] we just need the number [07:46] let me remove that code [07:46] zyga: \o/ [07:46] zyga: thanks [07:46] zyga: this is for the static build of snap-exec [07:46] zyga: so that things work even without a glibc in the core [07:49] ok, almost done [07:50] mvo, while I'm testing [07:50] quick experiment [07:50] run htop or similar in one terminal [07:50] go to root of snapd and run: time go test ./... [07:50] it seems that we have a part that scales well and a part when CPU is not really busy [07:51] for me it totals at around 1 minute 30 seconds [07:52] zyga: let me try with "time ./run-checks --unit" [07:52] no [07:52] that's idfferent [07:52] and faaaaar slower [07:52] it does sequential test for each package [07:52] the command I used does it in parallel [07:52] zyga: aha, go test in the new go skips vendor/ right? iirc 1.6 does not [07:52] (PR updated) [07:53] zyga: thanks for the update [07:53] correct [07:53] if 4641 passes I will push the same patch ot 4642 [07:55] zyga: go test starts with low cpu for me and then speeds up [07:56] zyga: for me real is about 0m58, user 2m9 and sys 0m14 [07:57] hmmm, interesting [07:58] on artful? [07:58] bionic, go1.9 [07:58] morning! [07:58] ah, I'm on 1.8 [07:58] hey pawel [07:58] hey pstolowski [07:58] zyga: that might be it, I can try on an artful machine in a bit [07:58] looking at the output it did run vendor tests here [07:59] but in any case, did you see a drop in CPU usage during that period? [07:59] for me it starts high, then drops to sequential [07:59] for instance cmd is super slow [07:59] (nothing happens there) [07:59] er, sorry, the one *after* cmd [07:59] cmd/snap [07:59] 25 seconds [07:59] client takes 10 seconds [08:00] zyga: yeah, cmd/snap, daemon, overlord are all slow for me, client too [08:00] devicestate and snapstaet too, might be worthwhile to look at the reasons at some point [08:03] http://paste.ubuntu.com/26545336/ [08:03] snap-seccomp [08:03] 85 seconds [08:03] I think those are those tests that run gcc, right? [08:04] I wonder if there's some golang testing trick that would let us spawn those as parallel, indepentend tests [08:04] maybe even using go() itself [08:11] Hello all [08:14] hey [08:14] you're up early :) [08:15] morning! [08:16] Moin! Yeah.. will try to do an early Friday today [08:18] zyga: yeah, snap-seccomp runs a lot of gcc, might be worth looking at too [08:18] hey niemeyer, good moooorining [08:19] and we have the new logo in the forum, looks really nice [08:20] PR snapd#4641 closed: many: move mount code to osutil [08:20] good morning, snappy [08:21] zyga: please update the other PR as well (the system-key one) [08:22] zyga: nevermind, I pushed it [08:23] mvo: Nice, isn't it? Really enjoyed it too [08:23] mvo: The whole design for the new site (not deployed yet) looks really sweet [08:24] mvo, re [08:24] thanks ! [08:24] looking forward to see it :) [08:24] zyga: thank *you* [08:24] zyga: lets hope tests are happy [08:24] zyga: also: great that we have this test, it saved our bacon [08:24] (or tofu) [08:24] indeed :-) [08:24] oh, the new logo! [08:25] I like the subtle separation of snap from craft [08:37] mvo thank you for initializing nfs home :) [08:38] I didn't think about that last night [08:38] mvo can we land 4049? [08:45] zyga: yes, I thnk so [08:46] pstolowski: there is still a comment I made that seem unadressed in #4401 [08:46] PR #4401: snapstate/ifacestate: auto-connect tasks [08:46] pstolowski: I recommented on it [08:47] pedronis, oh,i'm sorry about that, missed that somehow. thanks, will look at this [08:48] np, thanks for working through this [08:57] zyga: hm, a peculiar error in the system-key nfs PR, snap-cofine didn't refuse to run [08:58] oh [08:58] looking [08:58] weird [08:58] just on the three systems? [08:59] zyga: yeah, really strange, I have not tried to reproduce yet [08:59] mvo actually, other systems are disabled there [09:00] hmm [09:00] hmm hmm [09:01] my only idea is that something loads that profile [09:01] and that's probably one of the changes in the PR [09:01] * zyga reviews diff [09:05] mvo maybe we are killing the profile in the helper test scripts [09:05] I mean, system key, not the profile [09:06] mvo actually [09:06] so [09:07] I think I see what's going on [09:07] apparmor initialize starts up [09:07] then we get into regenerate all security profiles [09:07] that doesn't do anything [09:08] hmm, no wait wrong file [09:08] * zyga gets back to square one :/ [09:08] zyga: oh, so we need to keep the current core snap revision in the system-key? [09:08] zyga: to ensure we always re-generate for the snap-confine on the core snap [09:08] zyga: woah, our system key needs a lot of input :/ [09:09] well, that's another perhaps [09:09] I don't know yet [09:09] I wonder why it clearly regenerated the profile [09:09] not why it wouldn't generate a profile [09:09] let me get a coffee and have a deeper look [09:09] I think system key is long overdue and I cannot wait to see it in production :) [09:10] zyga: heh, more thorny than anticipated [09:10] zyga: I will add the core revision and see if that helps [09:10] zyga: its needed anyway [09:10] yeah, ok [09:10] that's actually true anyway because core revision determines apparmor templates [09:10] so +1 on that [09:16] * zyga got some coffee [09:16] niemeyer: go to be-e-e-ed [09:16] niemeyer: (thank you for the review though) [09:16] good morning all [09:16] Chipaca: Moin :) [09:17] Chipaca: Should have another section of it soon [09:17] * kalikiana will copy zyga and also get coffee [09:17] * Chipaca loves the new branding [09:18] niemeyer: about homes vs users, thinking about it some more [09:18] niemeyer: the tars are already encoding the uids/gids [09:18] niemeyer: should i just go with the uids, internally, and the username->uid translation on the fly? [09:19] Chipaca: On which side? I think the API should take the username.. internally we probably need to translate into a home at some point [09:20] Chipaca: It wouldn't hurt to keep the uid as well, though, at least in the metadata [09:20] niemeyer: to be able to alert when things change and restore would mess things up? [09:20] sgtm [09:22] api being username sounds a'ight (although it will mean having to do user lookups, which seems to kill spread on 14.04) [09:50] Chipaca: One more round on your way.. cmd/snap this time [09:50] niemeyer: just seen it [09:50] niemeyer: is the 'internal spacing' about the space in '2m ago', and in 'yyy-mm-dd hh:mm:ss'? [09:51] niemeyer: yes i can make the latter use T instead of space [09:51] niemeyer: for the former, i don't know how to drop the space without it losing meaning [09:51] Chipaca: Yeah, exactly [09:51] zyga: I added the core rev now but the security-setuid-root will not be fixed with this :/ [09:51] Chipaca: 2mins [09:51] (i could cheat and use something that looks like a space but isn't! that'd help machine parsing) [09:51] yeah, I suspect it's something else [09:52] niemeyer: i mean, i can drop the 'ago' if you think it's obvious [09:52] I'm really puzzled, it must be something obvious but I cannot see what would cause this from the diff [09:52] i added it because it seemed wrong without [09:52] Chipaca: Right, I think it's fine.. it's implicit from context [09:52] ok [09:53] Chipaca: Thinking about it now, it's probably nicer either way, even if we didn't care about the spacing [09:53] Chipaca: The "ago" will get boring quickly on a long listing [09:53] true [09:57] oops i forgot to change 'lost to the ages' back :-) [09:57] … and the "is probably *fine*" one! [09:57] heh [10:03] zyga: I have a debug shell now, I see that the apparmor profile for the snap-confine for the core rev is loaded (but not on disk) [10:03] is it possible that [10:03] 1) the profile is gone because of the test scripts and how they restore state [10:04] 2) the profile is in memory because prior test loaded it and we don't unload them in test restore scripts [10:05] * Chipaca waiting for spread to finish so he can fire off another spread, but really should leave for physio [10:06] zyga: yeah, I think its something like this, the details are still a bit blurry to me, I ran the the test standalone and got no error [10:06] zyga: eh, got *an* error [10:06] oh [10:06] well, that's possible too [10:06] the prepare for suite would load snapd [10:06] and that would keep the profile loaded [10:06] I don't think we actually unload anything in tests, right? [10:24] oh drat [10:24] I made the coffee and never picked it up [10:28] zyga: I think I have an idea, testing it now [10:29] mvo yeah? let me know once you have the result, I'm curious :) [10:30] jamesh: Are you around? Or anyone else involved in snappy on the desktop? I would like someone to take a spin with https://github.com/flatpak/xdg-desktop-portal/pull/155 [10:30] PR flatpak/xdg-desktop-portal#155: Support snap [10:30] hey [10:30] hey alexlarsson :) [10:31] hey [10:31] alexlarsson: hi. I'll give it a spin [10:31] oh excellent, jamesh is still up :) [10:31] It depends on https://github.com/flatpak/xdg-desktop-portal/pull/154 [10:31] PR flatpak/xdg-desktop-portal#154: Add application info abstraction [10:31] Which would be good if it could get separate reviews, but all the commits from that is in the other pr too [10:32] jamesh: i don't really have a snappy setup so i kinda coded the apparmor stuff blindly [10:32] jamesh: also, there are some todos that you might want to look at [10:32] oh shiny [10:33] I'm hoping we can release a xdg-desktop-portal with the document portal and snappy support next week [10:33] Then i can do complementary flatpak releases with it removed [10:33] * Chipaca -> physio, finally [10:33] alexlarsson: we don't yet have the portal support ready in master yet, so it isn't exactly trivial to test out at the moment [10:34] jamesh: you can test with gdbus from a snap [10:34] jamesh: In fact, maybe you can just put https://github.com/matthiasclasen/portal-test in a snap? [10:35] jamesh: I guess you need dbus aa rules to grant access to org.freedesktop.portal.* [10:36] alexlarsson: that's what I used when I was working on a prototype for this last year: https://github.com/jhenstridge/portal-test/blob/snap-pkg/snap/snapcraft.yaml [10:36] jamesh: But, at the very least you could add some debug printfs in the portal and see if it gets the right app id [10:36] I probably need to change that packaging a bit though [10:37] jamesh: also, i *think* i got the app id validation right by looking at the glob for snap package names, but please verify that [10:37] gdbus call --session --dest org.freedesktop.portal.Desktop --object-path /org/freedesktop/portal/desktop --method org.freedesktop.portal.OpenURI.OpenURI "" "https://gnome.org" {} [10:37] is a simple thing to test [10:38] Well, that actually doesn't really check the app id, so maybe not the best test... [10:38] But if you combine it with some added printfs to your xdg-desktop-portal you should get some basic validation [10:39] alexlarsson thank you :-) [10:39] np [10:39] I really don't want app developers to have to target multiple apis [10:39] we need to share this shit [10:40] not sure anyone was planning anything different, no..? [10:40] yeah, that's very much true [10:40] alexlarsson: agreed. I don't want to repeat all the work that's been done for GTK and Qt integration. [10:40] ikey: no, that was always the idea, but it need to also get done [10:40] ah gotcha [10:40] set a stable baseline now.. [10:57] zyga: quick question> kernel version as part of system-key: yes/no? [10:57] ok, I think I got the validation right now [10:57] oh [10:57] actually [10:57] version is not of much use [10:57] I'd say no unless you have a clear need [10:58] mvo we could keep bugs: [] [10:58] ;-) [10:58] if kernel bugs are bugging us [10:59] zyga: aha, thats good, yeah, we can create: bugs: [foo] when we need it [10:59] zyga: cool, thank you [11:05] * ogra_ wonder what the new bird logo on the forum is meant to represent [11:23] ogra_: it's the snapcraft logo... it represents snappy :) [11:23] haha [11:26] pretty banging logo though [11:26] it could animate [11:26] also it hidpis on the site favicon [11:26] * ikey is happy [11:27] it's nicely hidpi [11:27] yeah [11:27] lol [11:27] it scales all the way [11:27] "snappy takes flight" ™ [11:27] * ikey also cringes [11:27] wait does this make solus (budgie) and snappy birds of a feather? :P [11:27] twin engine bird [11:27] lol [11:28] hey ... https://docs.ubuntu.com/core/en/reference/interfaces/shows a "network-status" interface but snap interfaces in 2.30 does not ... did we lose something since 2.25 ? [11:28] mvo, ^^^ [11:28] (or zyga ) [11:28] I think I know [11:28] just checking [11:29] yeah [11:29] it's not implicit [11:29] it's just there but you need an snap to carry it [11:29] * ikey unsubs from mir notifications on github [11:29] Trevinho, ^^^ [11:29] just reminds me how little im doing myself.. [11:44] * zyga rethinks that validation [11:44] can be done faster and much more easily than now [11:45] by tracking source and target constraints separately [11:56] pedronis: Reviewed most of #4497 [11:57] PR #4497: many: make rebooting of core on refresh immediate, refactor logic around it [11:57] pedronis: Please have a look and let me know if you like the proposal [11:57] pedronis: Will take a coffee break and will review the server end [12:19] pedronis, do you have a moment? [12:27] so many non-powered-off-servers [12:32] pstolowski: now, I was having lunch [12:33] pedronis, ok, i need to run to school now, will be back in ~10 min [12:34] pedronis, will ping you for a quick HO before the standup if thats ok [12:34] ok [12:36] niemeyer: from quick look proposal seems reasonable, it means the client becomes stateful though, will take me a bit to get back to that PR though [12:44] woot [12:44] it works :) [12:53] pedronis, can you join standup HO? === ikey is now known as ikey|afk [12:59] zyga: fwiw, 4642 is green now [13:00] oh [13:00] oh standup [13:00] mvo so it was the test setup! [13:00] cool [13:01] mvo shall we merge it? [13:01] jdstrand hey [13:01] zyga: maybe we can look over it again just in case [13:02] I think pawel or chipaca could look [13:02] zyga: one independent review would be cool, maybe Chipaca or pstolowski [13:02] zyga: heh [13:02] :D [13:02] sure [13:02] it's funny, no? [13:28] ogra_: oh ok... It was my guess too after re-reading [13:29] Network observer should be enough though [13:29] Trevinho, yeah [13:29] But what we'd need is a connection status one [13:29] With auto connection [13:29] Although qt also needs to be fixed not to require too many infos [13:31] * kalikiana going out for lunch, back later [13:49] zyga: hey [13:49] jdstrand hey, I added the validation we spoke about last night and I will add some more (about things we didn't talk about) [13:49] well, at least not yesterday [13:50] I think we mentioned this long ago but we didn't black-list /proc and other similar places [13:50] I will do that [13:50] zyga: more in that pr or more in a future pr? [13:50] I also merged master into jamesh branch [13:50] jdstrand it can be separate, it's not related to that PR directly [13:51] I will look at one function that was meant to validate if a mount operation worked [13:52] zyga: so, technically /proc isn't a security issue (since they are just putting something they already have read/write access to in there. it isn't going to fakeout the actual kernel), but it is going to be an error [13:52] in the snap if it is doing that === ikey|afk is now known as ikey [13:53] zyga: so +1 on filtering /proc, /sys, /lost+found, and /dev [13:53] I agree but I think it's sensible to be conservative [13:53] oh [13:53] lost and found [13:53] good one [13:53] noted [13:53] /boot and /run probably also make sense [13:53] jdstrand as for 3963, I might look at splitting it to help [13:53] but, eh [13:54] we can always open it up more later, so seems safe to block these weird ones [13:54] yes, that's the principle I think [13:54] phase 1, conservative approach [13:54] * jdstrand nods [13:54] as for 3963, thank you [13:55] it is *so* close [13:55] * zyga looks at `snap/validate.go:276:20: "compatiblity" is a misspelling of "compatibility"` and sighs [13:55] pedronis: Yeah, we'd update it on every request, whether "maintenance" was set or not [13:56] pedronis: I think that reflects well the semantics on the server end [13:57] jdstrand the new validation logic resulting from last night's discussion is: https://github.com/snapcore/snapd/pull/4640/files#diff-b76be54ad28535ed9b0ac36a2851516eR240 [13:57] PR #4640: snap,interfaces: allow using bind-file layouts [13:57] zyga: so, I'm seeing things like this: [13:57] if layout.Bind != "" { [13:58] if layout.BineFile != "" { [13:58] and I wonder why that isn't if/else if [13:58] no reason, but it doesn't change anything [13:58] which makes my wonder if this is not caught: [13:58] zyga, reviewed 4642 [13:58] /foo: [13:58] bind: ... [13:59] bind-file: ... [13:59] this is caught by existing validation that's older than this pr [13:59] you can barely see it ... [13:59] zyga: are there tests for it? [13:59] https://github.com/snapcore/snapd/pull/4640/files#diff-b76be54ad28535ed9b0ac36a2851516eR560 [13:59] PR #4640: snap,interfaces: allow using bind-file layouts [13:59] pedronis, results on staing https://paste.ubuntu.com/26546594/ [13:59] still 6 tests failing [14:00] yes, though not for every possible combination [14:00] I can add more :) [14:00] it's been a learning exercise so far [14:00] https://github.com/snapcore/snapd/pull/4640/files#diff-0c50c794f66e17a6bf61861029a83e21R533 [14:00] actually I did, sorry, [14:00] I just see it in the diff now [14:00] zyga: right, so that isn't going to catch this: [14:00] /foo: [14:00] bind: ... [14:00] symlink: ... [14:00] no, it does [14:00] it catches it [14:01] you must use exactly one of the kind definers [14:01] oh nused [14:01] I was looking at something different [14:01] yeah, number of things used [14:01] if you can add a test for that, it would be great [14:01] yeah, I did for that exact case [14:01] I <3 negative tests [14:02] ok [14:02] https://github.com/snapcore/snapd/pull/4640/files#diff-0c50c794f66e17a6bf61861029a83e21R533 [14:02] PR #4640: snap,interfaces: allow using bind-file layouts [14:02] I'm sure there are some things I didn't cover, I will go over it again [14:02] I'm looking only at a particular commit. I'll look at the whole thing again [14:02] I was thinking how to do the source validation and I was thinking about various options [14:02] I like one test [14:02] I'm sure you will love it [14:02] https://github.com/snapcore/snapd/pull/4640/files#diff-0c50c794f66e17a6bf61861029a83e21R744 [14:03]