[06:05] morning [06:13] 'morning! [06:14] good morning mardy [06:31] mardy: mvo: hello [06:31] mborzecki: good morning! [06:31] mvo: i've tweaked https://forum.snapcraft.io/t/extra-kernel-commandline-arguments-on-uc20/24370 a bit [06:37] mborzecki: awesome, thanks. if you feel it's ready I can publish [06:39] mvo: yeah, i think we can publish it, graham will still be able to tweak it as needed [06:46] mborzecki: excellent, I will do a final read and then publish [06:49] mvo: thanks! and thanks for starting with the doc [06:49] mborzecki: my pleasure [06:59] mborzecki: looks perfect, thanks for your edits, much clearer this way. I listed it now [06:59] mvo: yay, thanks! [07:08] morning [07:14] good morning pstolowski [07:14] pstolowski: hey [07:25] PR snapd#10260 closed: secboot: switch encryption key size to 32 byte (thanks to Chris) [07:35] PR snapd#10263 opened: interfaces: fix linter issues [08:14] mborzecki: hi, how should I install golangci-lint to try it locally? [08:28] pedronis: you can grab it from here: https://github.com/golangci/golangci-lint/releases [08:28] or go get, whichever is more convenient [08:43] mborzecki: I'm probably using it wrong but I'm quite confused by its output, there's a lot of typecheck errors which is not even a linter we list [08:48] pedronis: have you checked the PR? https://github.com/snapcore/snapd/pull/10082 [08:48] PR #10082: github: try out golangci-lint [08:49] mborzecki: yes, I'm running it with ~/go/bin/golangci-lint run -c .golangci.yml [08:49] from that PR [08:49] maybe I don't even need the -c [08:50] anyway the output doesn't seem useful, I expected testpackage problems but I don't see any, but I get typecheck problems [08:50] hm it shoudl pick up the config from he repo [08:50] I get the same with -c or without [08:50] pedronis: which packages it complains about? [08:50] (typecheck specifically) [08:51] which version were you using? [08:53] mborzecki: this is the output I'm getting, https://paste.ubuntu.com/p/bY6cgdbqrx/ it's rather confusing/useless [08:53] pedronis: i have 1.39 built from source directly, let my try the latest master [08:54] this is 1.40 binary I think [08:54] golangci-lint has version 1.40.0 built from 5c6adb63 on 2021-05-10T10:45:21Z [08:55] did they change the config ? [08:56] hm 1.40 seems to be working fine here [08:57] anyway if I can't make it work I'm kind of -1 on it [08:59] mborzecki: I'm on focal fwiw [08:59] pedronis: hm this all i see it complaining about with current master: https://paste.ubuntu.com/p/sm6BdD9SB9/ [08:59] I get completely different errors [09:00] did you use go get? [09:00] as I said I grabbed the binary [09:00] pedronis: hm i think mardy is on focal too, but i don't expect it to be a factor here [09:00] pedronis: no it's a tarball from the releases page [09:00] I think I got the same [09:01] anyway if it's giving different outputs depending on the phase of the moon is not making me very happy [09:02] the way i run it is i'm inside the snapd source tree, and then just call `golangci-lint run`, the config file should be picked up automatically [09:02] yea, I do that [09:02] I get those other errors [09:02] I get those errors also if I pass the config explicitly [09:02] can you run `golangci-lint run --verbose` ? [09:03] pedronis: this is what i get with --verbose: https://paste.ubuntu.com/p/jsfMJhNdRx/ [09:05] I get the same list of linters, but analyzers has output for example === rZr is now known as RzR [09:06] mborzecki: I get Issues before processing: 4033012, after processing: 53 [09:07] ogra, hi i am back with my logs "the-tool[207]: - assertion is signed with expired public key" , I need an RTC module [09:07] mborzecki: very different: https://paste.ubuntu.com/p/qhwgqZ8Rg2/ [09:08] mborzecki: ah, maybe I know what I'm doing wrong [09:08] pedronis: hm what is it? [09:08] I might have the gopath set wrong, maybe [09:09] also got G111MODULE=off, but not sure that changes anything either [09:10] fwiw i was overriding GOPATH, but the tool worked fine with that too [09:11] (though the go-pls version i used was unhappy about changing gopath) [09:12] pedronis: did you get it to work? [09:20] RzR, i think ijohnson added a fix for that, try the edge channel for image builds [09:21] ogra, I tried edge see my versions [09:21] https://forum.snapcraft.io/t/built-uc20-rasperry-pi-image-hangs-on-boot/23891/22?u=rzr [09:22] ah, sorry, havent checked the forum in 1h or so 🙂 [09:23] let me see if I have a RTC module if not I'll rebuild some snaps to update timestamps [09:23] mborzecki: it worked, I'm not sure what to think about gosimple, it's both right and to naggy, also it's suggesting things that might be wrong if it gets is type analysis wrong [09:25] pedronis: the PR sets up the linter to only complain about new things, and the action adds notes rather than review comments, so not a hard fail [09:28] mborzecki: that sounds annoying in its own ways [09:28] I mean the added notes [09:28] I struggle already sometimes with preexisting comments when doing reviews [09:28] pedronis, mborzecki: since running the linter takes ages, I always run it on one package at a time (for example, `golangci-lint run ./interfaces/`) [09:29] mborzecki: we should probably run it without gosimple and testpackage, at least for a while, especially until we haven't switched to go1.13 everywhere [09:29] mardy: results should be cached between runs afaik [09:34] mborzecki: fwiw it feels slow here too [09:35] pedronis: is it slower than runnning each linter separately though? :) [09:36] it probably takes a bit longer if you run it on the whole tree, i usually invoke it on the package(s) i modify in the branch [09:36] mborzecki: anyway I left some comments in the PR [09:37] anyways, it's ~15s on the whole tree, vs ~3s after there's some cached data [09:37] pedronis: thanks, i'll take a look [10:35] ogra, maybe it also need https://github.com/snapcore/snapd/pull/10085 [10:35] PR #10085: cmd/snap-bootstrap/initramfs-mounts: move time forward using assertion times (2.49) [10:35] mvo, ^ [10:36] well not sure i am using 2.50 [10:37] sorry for noise, I need to dig deeper [11:10] pedronis: hi, i've updated the two refresh-control PRs you commented on yesterday [11:10] pstolowski: I'll see if I can get back to them today [11:10] that I also need 2nd reviews though [11:10] thx [11:13] pedronis: i've also updated #10182 and set as ready to review although it probably makes no sense until phase1 is merged [11:13] PR #10182: o/snapstate: autorefresh phase1 for refresh-control [11:13] Bug #10182: Can not logout of gnome when xcompmgr is running [11:21] PR snapd#10252 closed: boot: reseal given keys when the respective boot chain has changed [11:34] pedronis: regarding https://github.com/snapcore/snapd/pull/10253#discussion_r630320902 i'm not sure we should also blindly add a system to current list, maybe it's better to error out in such case? [11:34] PR #10253: boot: helpers for manipulating current and good recovery systems list <⛔ Blocked> [11:35] (also pushed master there, so the diff is smaller) [11:42] mborzecki: well, we need the various pieces we have to fit together, the issue that we need to be careful with is not to accumulate either tried systems at the end of the list [11:44] mborzecki: should we chat on this after the standup? [11:45] pedronis: hm can we try 1430 maybe? i need to leave at 4 and drop the kids off at school for their training [11:45] mborzecki: I have another meeting at 14:30 [11:46] pedronis: ah, ok, let's stay after standup then, we usually make it in half an hour so there should be enough time after [11:46] thx [11:50] I noticed that we are not setting the NoNewPrivileges flag in snap-confine, because (as it's written in the comments) it breaks some snaps [11:50] do we have some bugs in launchpad to track this? [11:50] mardy no, because there is no point in no new privs there [11:50] snap-confine is a launcher [11:51] IIRC having that would block the launched program from doing what it may genuinely want to do [11:51] PR snapd#10227 opened: test read the file from spread [11:51] zyga: mmm... maybe that would not be the right place, but I grepped for NO_NEW_PRIV in the snapd source tree, and there's no place where it's set [11:52] right but why do you want to have it? [11:53] zyga: to make sure that a process running in a snap cannot break outside of its confinement [11:53] I just wrote a comment about it in https://github.com/snapcore/snapd/pull/8926, please let me know if I'm not getting the story right :-) [11:53] PR #8926: interfaces: add microstack-support interface [12:08] mardy your assumption is wrong, basically [12:08] mardy I'm away from snapd development so I won't give you a full explanation now [12:08] but the assumption that permissions are only reduced is incorrect [12:08] there's a bounding box [12:08] but transitions are possible within it [12:09] zyga: but that's a bug, right? [12:09] no [12:10] I mean, in apparmor [12:10] if one sets the NO_NEW_PRIVILEGES flag, one should expect that any child process won't have more permissions that the process that this flag was applied to [12:21] PR snapd#10227 closed: test read the file from spread [12:30] ehh mocking is fun `revision 0 is already the current revision` [12:36] mardy I'm really not able to answer in detail beyond "that is not the goal", snap-confine is the entry point to the sandbox that is defined by snapd, an it can be as broad as required [12:36] mardy including not confined at all [12:36] it all plays with the fact that some interfaces are privileged and you cannot just use them [12:36] (at will that is) [12:44] mardy and zyga (see amurray): hey, I can't get into this now, but I added a comment to the PR. summary> nnp and apparmor haven't historically played well together, but that's ok wrt to snap interface policy since we have different types of interfaces that can be mediated via snap declarations (again, see the comment) [14:01] cachio: hey does this error message mean that core-initrd needs to change the spread URL it is using for spread tests ? https://travis-ci.org/github/snapcore/core-initrd/builds/770716802 [14:02] yes [14:02] ijohnson, we should use the one I pasted in the notes [14:02] cachio: do you have time to propose a PR to core-initrd fixing the problem ? [14:02] ijohnson, sure [14:02] cachio: awesome thank you [14:03] the repo is https://github.com/snapcore/core-initrd/blob/main/spread.yaml [14:06] PR snapd#10248 closed: tests: adding support for debian 10 on gce [14:10] ijohnson, there is a problem [14:10] we need to move to github actions [14:10] cachio: what's the problem [14:10] cachio: yeah I have known about that, do we need to do it in order to fix their spread tests though ? [14:11] otherwise no way to use the gce sa key [14:12] cachio can you work with xnox to move them to GitHub actions then? [14:12] Or sil2100 if xnox is not working on core-initrd anymore [14:39] jdstrand, zyga: thanks, it's way more clear now :-) [14:46] PR snapd#10264 opened: config: add "virtual" config via config.RegisterVirtualConfig [14:54] cachio: ijohnson: move to github actions would be welcomed. I thought at the time it was easier to do travis, hence that's what was done then. [14:54] yeah at the time it was easier to do travis but times have changed [14:55] sorry in a meeting right now, but will setup a private chat about migration in a little bit [15:34] hi I've found a minor mistake in [15:34] https://github.com/kubiko/roseapple-pi-ubuntuCore-build/pull/1 [15:34] PR kubiko/roseapple-pi-ubuntuCore-build#1: docs: Append gcc to PATH env var [16:07] * cachio lunch [16:45] mborzecki: I left some comments/questions in the tasks PR [17:30] pedronis: thanks!