| === chihchun_afk is now known as chihchun | ||
| mborzecki | morning | 05:17 |
|---|---|---|
| zyga | Hey :-) | 05:25 |
| mborzecki | zyga: hey hey | 05:32 |
| === chihchun is now known as chihchun_afk | ||
| zyga | Hey mvo :-) | 06:09 |
| mvo | hey zyga | 06:09 |
| mvo | it looks like we have failures in the testsuite in fedora / centos right now | 06:09 |
| mvo | or is it just my PR? | 06:09 |
| zyga | My stuff was ok last night | 06:10 |
| zyga | Iβm getting breakfast still | 06:10 |
| === chihchun_afk is now known as chihchun | ||
| mborzecki | mvo: hey, more selinux crap? | 06:18 |
| mborzecki | btw. https://bugs.launchpad.net/ubuntu/+source/snapd/+bug/1826460 is the same story as https://forum.snapcraft.io/t/selinux-warning-when-running-lxc/11100/ | 06:19 |
| mborzecki | i seriously don't know what to think of it, oh and 2.39 breaks totally there | 06:19 |
| mborzecki | i reproduced the problem with disco cloud image and installed that weird kernel from the kernel-ppa | 06:20 |
| mvo | mborzecki: aha, is that the stacked LSM kernel? | 06:20 |
| mborzecki | mvo: i don't think so, the kernel seems to have AA disabled, and SELinux enabled and default | 06:26 |
| mborzecki | mvo: oh, and once you install the kernel, none of the selinux utils are pulled in | 06:28 |
| mvo | mborzecki: woah | 06:33 |
| zyga | Stacking? | 06:36 |
| mvo | zyga: there is this work to allow selinux and apparmor in parallel, I was wondering if its that kernel | 06:38 |
| zyga | mvo: disco has it | 06:38 |
| zyga | It landed at the same time as another lxd related bug with shiftfs | 06:39 |
| mborzecki | shitfs? | 06:43 |
| mborzecki | ah, that's for uid/gid on fs | 06:43 |
| mborzecki | anyways, i don't think it's this kernel | 06:44 |
| === pstolowski|afk is now known as pstolowski | ||
| pstolowski | heyas | 07:01 |
| mborzecki | pstolowski: hey | 07:03 |
| mborzecki | left a note under https://bugs.launchpad.net/ubuntu/+source/snapd/+bug/1826460/comments/3 | 07:04 |
| zyga | hey PaweΕ :) | 07:04 |
| mborzecki | idk, maybe we could somehow guess the system is using a default policy, meaning, nothing probably works an then stop any attempt to use selinux | 07:05 |
| zyga | pstolowski: https://github.com/snapcore/snapd/pull/6781 <- 2nd version of static attr fix | 07:07 |
| pstolowski | zyga: yes, i saw it, thank you, will get to it shortly | 07:07 |
| zyga | pstolowski: tha main difference is that the changes are made in reloadConnections, not in setup profiles, because this catches system key regeneration | 07:07 |
| zyga | thanks! | 07:08 |
| mborzecki | still don't get why someone who's not a kernel developer or test would install some kernel package from a random PPA | 07:11 |
| zyga | mborzecki: I know why | 07:28 |
| zyga | mborzecki: sergiusens does it because it makes his surface work | 07:28 |
| zyga | mborzecki: AFAIK people do it to get modern vega working on older releases | 07:28 |
| mborzecki | zyga: well, but sergiusens is more than capable of making it work | 07:28 |
| zyga | mborzecki: some people do it to follow vanilla kernel development | 07:28 |
| zyga | mborzecki: but not all people are sergiusens :) | 07:29 |
| mborzecki | zyga: also, if you provide a kernel package for ubuntu, one would expect that it's soemwhat tailored for the distro :P | 07:31 |
| zyga | mborzecki: there's an official vanila kernel ppa | 07:31 |
| zyga | mborzecki: not tailored at all | 07:31 |
| mborzecki | zyga: it's like someone provided a kernel package from COPR for fedora, which defaults to AppArmor, that'd be fun | 07:31 |
| zyga | but what do I know :) | 07:31 |
| * zyga could use a 2nd review for https://github.com/snapcore/snapd/pull/6758 | 08:05 | |
| zyga | promise not to push more today ;) | 08:06 |
| * zyga does more reviews | 08:12 | |
| pstolowski | zyga: reviewed static attrs PR | 08:30 |
| zyga | pstolowski: thank you! | 08:37 |
| Chipaca | mvo: morning sir. Working with sergiusens for a bit yesterday we were both surprised that #6532, which landed in master on 7/3, wasn't in 2.38 which was cut 21/3. My memory of (and, granted, attention to) the release is very poor, so I was hoping you'd know why this was this way | 08:40 |
| Chipaca | https://github.com/snapcore/snapd/pull/6743 could use a second review if anybody is hankerin' to do one | 08:41 |
| mvo | Chipaca: uh, that sounds fishy! | 08:42 |
| mvo | Chipaca: aha, I see now. 2.38 was actually branched earlier, 2019-03-05 and at this point we only cherry-picked things targeted for 2.38 :/ | 08:44 |
| Chipaca | mvo: i suspected something like that | 08:45 |
| Chipaca | mvo: but didn't have dates :-) | 08:45 |
| Chipaca | a shame, but ok | 08:45 |
| Chipaca | (sergio was all ready to use that feature :) ) | 08:45 |
| mvo | Chipaca: it wil be in 2.39 which is not that far away | 08:47 |
| Chipaca | yep | 08:47 |
| * pstolowski running quick errand, afk for a while | 08:55 | |
| zyga | heh | 08:58 |
| zyga | Chipaca: I wanted to review https://github.com/snapcore/snapd/pull/6743 but I noticed I already did | 08:58 |
| * Chipaca hugs zyga | 08:58 | |
| Chipaca | zyga: thank you | 08:58 |
| zyga | Chipaca: wanna do a quick 2nd +1 for https://github.com/snapcore/snapd/pull/6758 ? | 09:02 |
| zyga | a part of an ongoing refactoring | 09:02 |
| Chipaca | zyga: where is NewUserProfileUpdateContext used for, that it's public? | 09:12 |
| zyga | it's used from main.go | 09:13 |
| * zyga checks | 09:13 | |
| zyga | probably in subsequent patches | 09:14 |
| zyga | Chipaca: in 2 patches from this one: https://github.com/zyga/snapd/commit/1ffaae4478d680566b78ead86f3911c5784893ea | 09:14 |
| zyga | Chipaca: so not yet, but soon :) | 09:14 |
| zyga | Chipaca: https://github.com/zyga/snapd/commits/feature/user-mount-ns-20.9-split-rebased - | 09:15 |
| Chipaca | zyga: that's in the same package, isn't it? | 09:15 |
| zyga | yes | 09:15 |
| zyga | but we may need to put it into a library soon-ish so I'm slowly building towards that | 09:16 |
| zyga | (for a new binary) | 09:16 |
| zyga | snap-update-ns and snap-bootstrap-ns | 09:16 |
| zyga | would share most of the code but would behave differently and bootstrap would do some new things too | 09:16 |
| zyga | (bootstrap would also not need the horrid C bootstrap logic) | 09:17 |
| zyga | (as in bootstrap.c, the name overlap is unfortunate) | 09:17 |
| Chipaca | zyga: reviewed, mostly. Almost a +1. | 09:21 |
| zyga | reading | 09:21 |
| zyga | Chipaca: replied | 09:23 |
| zyga | if it seems I'm relucant to change it it's partially true and because of the changes stacked on top; I can rework locking if you feel strongly about it but even there I would prefer to append a patch to this series (that was tested and "polished" rather than rebase it, mostly to avoid the extra cost | 09:25 |
| pstolowski | re | 09:28 |
| zyga | pstolowski: hey | 09:29 |
| zyga | question to you sir: | 09:29 |
| zyga | is this what you expected? https://www.irccloud.com/pastebin/CoMWt6SB/ | 09:29 |
| zyga | I would have to add the pair of unrelated snaps for this test to preserve that connection | 09:29 |
| Chipaca | zyga: yes I think the lock/unlock pair would be better, but also yes no problem for it to be done further down the series | 09:31 |
| pstolowski | zyga: yes, that's what i expected. do you need to add those snaps because something is unhappy about them missing? | 09:31 |
| zyga | Chipaca: in that model, the unlock state would be simply stored in the context, right? | 09:31 |
| Chipaca | zyga: yes | 09:31 |
| zyga | pstolowski: not unhappy, as the comment states, they are removed on manager startup | 09:31 |
| zyga | pstolowski: note that the order of manager startup vs adding snaps is essential | 09:31 |
| Chipaca | zyga: or you could just embed the locker | 09:32 |
| pstolowski | zyga: you could add them to the state after instantiating the manager and before carrying on with the test, no? | 09:34 |
| zyga | No | 09:35 |
| zyga | Because manager removes unused connections on startup | 09:36 |
| zyga | I would have to add the unrelated snaps before starting the manager | 09:36 |
| zyga | I can do that, just asking if that is what you expected | 09:37 |
| pstolowski | zyga: yes, but that happens in Manager - initialize, when you instantiate the manager in the test. you can alter the state afterwards? | 09:37 |
| pstolowski | zyga: ah i see | 09:39 |
| pstolowski | zyga: perhaps the second test is better to do this | 09:39 |
| pstolowski | zyga: the one that creates a task for setup-profiles | 09:39 |
| zyga | Mhm | 09:39 |
| pstolowski | zyga: sorry, you're right about the one i commented on | 09:40 |
| pstolowski | zyga: btw https://github.com/snapcore/snapd/pull/6751 is pending some rework right? | 09:44 |
| zyga | pstolowski: looking | 09:44 |
| zyga | pstolowski: well, it needs tests | 09:44 |
| zyga | pstolowski: but I think the place is right | 09:44 |
| pstolowski | ok | 09:45 |
| mborzecki | pstolowski: some quick fixes for the policy https://github.com/snapcore/snapd/pull/6783 that were caught in 6778 | 09:47 |
| zyga | mborzecki: man, selinux is comple | 09:53 |
| zyga | complex* | 09:53 |
| mborzecki | zyga: or just different conceptually | 09:56 |
| Chipaca | zyga: πππππ£π€π₯ππ₯πππππ₯ πΈπππ£π₯ | 09:57 |
| * mvo hugs zyga for "the computer says NO" | 09:58 | |
| zyga | lol :) | 10:03 |
| === Girtablulu is now known as Girt|Vacation | ||
| zyga | pstolowski: updated https://github.com/snapcore/snapd/pull/6781 | 10:12 |
| zyga | the new thing is https://github.com/snapcore/snapd/pull/6781/commits/dd739571c24e659d0449c0c0ca6314719f301777 | 10:15 |
| * Chipaca goes out for a bit | 10:19 | |
| zyga | mvo: I think you should review https://github.com/snapcore/snapd/pull/6781 | 10:22 |
| mvo | zyga: full of meetings today but will do | 10:35 |
| mvo | zyga: once I get the chance | 10:35 |
| pstolowski | zyga: ty, will look in a bit | 10:37 |
| zyga | Chipaca: something like this? https://github.com/zyga/snapd/commit/328b4b471e22609d60b4b4e430a4aa648963c714 | 10:48 |
| zyga | mvo: thank you! | 10:48 |
| zyga | mvo: about https://github.com/snapcore/snapd/pull/6756 - should I rework it to return an error or leave as-is? | 10:50 |
| zyga | Chipaca: https://bet365techblog.com/open-sourcing-jingo-a-faster-json-encoder-for-go <- maybe worth prototyping a switch to see what we'd save? | 10:57 |
| * zyga quick lunch | 11:16 | |
| === chihchun is now known as chihchun_afk | ||
| zyga | my body demands coffee | 11:38 |
| zyga | my mind demands coffee | 11:39 |
| zyga | I shall make coffee | 11:39 |
| zyga | though going outside for a bike ride seems lovely too, it's so warm already | 11:39 |
| zyga | pstolowski, mborzecki: I have 28C outside, you? | 11:39 |
| mborzecki | 28-29 | 11:39 |
| pstolowski | zyga: 27 outside. 28 inside.. /o\ | 11:40 |
| zyga | desktop-portal-filechooser fails | 11:43 |
| zyga | la la la | 11:43 |
| mborzecki | out to drop the kids off at school for some extra classes, back in 30 | 11:47 |
| jdstrand | mborzecki: hey, can you look at https://github.com/snapcore/snapd/pull/6780#discussion_r278914233 | 11:47 |
| jdstrand | mborzecki: in 30 :) | 11:47 |
| === chihchun_afk is now known as chihchun | ||
| zyga | jdstrand: I pushed some things to https://github.com/snapcore/snapd/pull/6714 | 11:57 |
| zyga | hey :) | 11:58 |
| zyga | jdstrand: (also replied to all the comments there) | 11:58 |
| zyga | jdstrand: the /tmp thing has left one question in my mind | 11:59 |
| zyga | jdstrand: on regular systems with classic packages /tmp is not private | 11:59 |
| zyga | jdstrand: so if an app invoked by another user on this system drops a file that just happens to be readable by my user, well, tough luck, it'd better not contain secrets | 12:00 |
| jdstrand | that's just normal dac | 12:00 |
| zyga | jdstrand: the question is: why is snap /tmp different? could we just make /tmp/snap.$SNAP_NAME/ with 01777 | 12:01 |
| jdstrand | the /tmp should still be 1777 root:root | 12:01 |
| zyga | jdstrand: note, in my question I eliminate the presence of intermediate root:root 0700 base directory | 12:01 |
| jdstrand | it doesn't really need to be different. iirc, you are doing 0700 at that point only to bind mount on it and then later chmod 1777 | 12:02 |
| zyga | hmmm, we don't bind mount on it AFAIK | 12:02 |
| zyga | or perhaps we do but that's something that we could change | 12:02 |
| jdstrand | you are otherwise it wouldn't be known as /tmp in the mount namesapce... ? | 12:03 |
| zyga | that is, we could mkdir 01777 and then bind mount that over /tmp (so not over /tmp/$RANDOM/tmp but over constructed-snap-rootfs/tmp | 12:03 |
| zyga | yeah, but we construct the root filesystem in a different way now (on the side) and the very old code that manages /tmp could be adapted to that style too | 12:03 |
| zyga | we could even mount a tmpfs there and call it a day (no visibility from initial mount namespace) but I think that was ruled out because of space constraints | 12:04 |
| jdstrand | I'm not opposed to that changing. the main thing is, the result needs to be root:root 1777 and the getting there needs to be race-free | 12:04 |
| zyga | jdstrand: can you expand on the race-free aspect? | 12:04 |
| jdstrand | zyga: I don't think we want to mount a tmpfs there | 12:04 |
| zyga | jdstrand: (agreed on tmpfs) | 12:05 |
| jdstrand | zyga: we are actively not doing that because a tmpfs can take up to 50% of ram and those cause low memory | 12:05 |
| jdstrand | thus* | 12:05 |
| jdstrand | zyga: race free> yeah, however we get there, there can't be a possibility of an unprivileged user (or a snap at all) to to symlink attacks, etc | 12:06 |
| zyga | mhm | 12:06 |
| zyga | we protect against some of the attacks on mount with our special mount code in snap-update-ns but we don't have a C equivalent AFAIK | 12:07 |
| zyga | jdstrand: as far as I'm aware only mount is really susceptible to a race attack, out of the primitives we use | 12:08 |
| mborzecki | re | 12:08 |
| zyga | jdstrand: I'm trying to find a solution that would be devoid of edge cases, correct, not insecure (see what I did there :), and usable for people | 12:08 |
| jdstrand | zyga: I think you are not taking my point as intended | 12:08 |
| jdstrand | zyga: we have a property now where we are race free. with the rename to a fixed dir or the rewrite you are sugggesting, we want the same property | 12:09 |
| zyga | jdstrand: no disagreement there | 12:10 |
| * jdstrand takes deep breath | 12:13 | |
| jdstrand | why is sid failing to build all of a sudden: https://paste.ubuntu.com/p/k9nYZkrSgP/ | 12:14 |
| zyga | jdstrand: where are you seeing this? | 12:15 |
| zyga | in the sbuild test? | 12:15 |
| jdstrand | I can't get out from under constant problems with this PR... /o\ | 12:15 |
| jdstrand | zyga: I saw it didn't build here: https://travis-ci.org/snapcore/snapd/jobs/524707744 | 12:16 |
| zyga | jdstrand: hey do you remember user mounts? :D | 12:16 |
| * zyga looks | 12:16 | |
| jdstrand | zyga: it did not have great logs, so I tried in a local qemu | 12:16 |
| zyga | jdstrand: it's going to be all right, it's just one of those moments where you improve things for many people, not just snap users, by fixing bugs blocking your work | 12:16 |
| jdstrand | use -debug with spread and then rewound and did: | 12:16 |
| jdstrand | cd /home/gopath/src/github.com/snapcore/snapd | 12:17 |
| jdstrand | cd _build && go install -gcflags=all=\"-trimpath=/home/gopath/src/github.com/snapcore/snapd/_build/src\" -asmflags=all=\"... | 12:17 |
| jdstrand | as shown by the -debug output | 12:17 |
| jdstrand | zyga: yes, that's true. it is just been nuts | 12:17 |
| jdstrand | thanks | 12:17 |
| jdstrand | in this case, I don't know what bolt is. I do know what libseccomp-golang is, and I'm puzzled why sid is not working with the distro golang-seccomp | 12:19 |
| * jdstrand shakes fist | 12:19 | |
| jdstrand | it seems to not be installed. why? | 12:19 |
| zyga | how we ignore errors in some places? https://www.irccloud.com/pastebin/4jf6qJKw/ | 12:19 |
| jdstrand | </rhetorical> | 12:19 |
| * jdstrand looks | 12:19 | |
| zyga | jdstrand: I'm trying to make sense of that but I see more magic | 12:20 |
| * jdstrand is puzzled. these packages are in the build-depends | 12:20 | |
| zyga | src/github.com/snapcore/snapd/cmd/snap-seccomp/main.go:655:10: undefined: seccomp.ActLog <- this seems to say that golang-seccomp is older? | 12:21 |
| zyga | jdstrand: build-depends vs vendorized | 12:21 |
| jdstrand | I'm wrong, it is installed | 12:22 |
| jdstrand | zyga: ah. ok. yes | 12:22 |
| zyga | package from debian took priority? | 12:23 |
| jdstrand | zyga: man I skipped right over that | 12:23 |
| jdstrand | ok, I see what to do | 12:23 |
| jdstrand | mborzecki: do you think if I just added the 4th field to system-key it wouldn't be a big deal? | 12:24 |
| zyga | jdstrand: but does that mean we won't be able to build de-vendorized in sid? | 12:24 |
| zyga | jdstrand: the sbuild test should show that btw, if that breaks next we need to update golang-seccomp in debian | 12:24 |
| jdstrand | zyga: I can make it work. it worked before | 12:24 |
| zyga | jdstrand: system key is flexible, just check how costly it is to compute because snap run does it | 12:24 |
| jdstrand | I made a small changge to tyhicks initial patch which apparently broke it | 12:24 |
| jdstrand | zyga: adding the 4th field couldn't be cheaper. it is that string match | 12:25 |
| mvo | zyga: sorry for the delay - 6756 is afaict no longer needed, the change that landed recently checks for nil and returns an error now | 12:25 |
| zyga | mvo: oh, where was that? | 12:26 |
| zyga | mvo: please comment in the PR and feel free to close it | 12:26 |
| mvo | zyga: thanks, will do | 12:26 |
| jdstrand | mborzecki: nm, I'm going for it | 12:26 |
| mborzecki | jdstrand: i wouldn't mind, it's an implementation aspect, not user facing, but nothing i would block on | 12:27 |
| zyga | jdstrand: I will be much happer once, one day, I will replace all of snap-seccomp with pure go code | 12:27 |
| mborzecki | jdstrand: though once it's collected & parsed in backend init, you could do whatever later on in the code, no need to call snap-seccomp once again | 12:27 |
| mborzecki | (unless for compilation obviously)( | 12:27 |
| zyga | jdstrand: and can remove that from system key for that extra oomph speed for snap startup :) | 12:27 |
| zyga | mborzecki: note that snap-run does this every time AFIAK | 12:28 |
| mborzecki | zyga: yes, it also picks up build ids and so on | 12:28 |
| zyga | so-many-reviews | 12:29 |
| jdstrand | zyga: well, I think you cannot do that without removing libseccomp cause libseccomp is C code and CGO_ENABLED=0, but I don't want to go there | 12:29 |
| zyga | mborzecki: does it mean snap-run will run snap-seccomp with some query argument each time on every run? | 12:29 |
| zyga | jdstrand: yes, that's what I meant | 12:29 |
| zyga | but don't worry, I'm sure we will slowly get there, with enough checks to be confident we are producing the same bpf as before | 12:29 |
| mborzecki | zyga: it's part of system-key, so yes | 12:30 |
| zyga | mborzecki: would be good to time that | 12:30 |
| zyga | (on a pi for example) | 12:31 |
| jdstrand | zyga: I can't say how worried I am about that any more than I have. we should *not* reimplement libseccomp. the world has standardized on it and it is tricky code. we will trade one set of problems that everyone faces and has a desire to fix for another that only you could fix and no one else cares | 12:32 |
| mborzecki | zyga: afaik we call apparmor_parser too, woulnd't expect snap-seccomp to be slower than that | 12:32 |
| jdstrand | but I don't have time to debate this | 12:33 |
| Chipaca | showoffs | 12:34 |
| Chipaca | oops, wrong place | 12:34 |
| mborzecki | offshows :) | 12:34 |
| Chipaca | that was a comment about you and zyga having ~28C | 12:34 |
| Chipaca | didn't notice i was scrolled way up | 12:35 |
| zyga | haha | 12:35 |
| Chipaca | zyga: looking at your locking code now | 12:35 |
| zyga | Chipaca: I pushed a 2nd branch that fixes a typo in the branch name but it's otherwise identical | 12:35 |
| Chipaca | zyga: calling Unlock when it's not locked would usually be a panic() | 12:36 |
| zyga | Chipaca: +1, easy change | 12:36 |
| Chipaca | zyga: either implicit or explicit I don't mind :-) | 12:36 |
| mborzecki | hmm a bunch of snaps mounted locally, somehow udisks insists on showing 2 of them as normal disks, wtf? | 12:37 |
| zyga | mborzecki: that's a local patch to udisks or you lost your mtab state | 12:37 |
| Chipaca | mborzecki: that seems to happen when something is using and old core | 12:38 |
| jdstrand | mborzecki: jdstrand: though once it's collected & parsed in backend init, you could do whatever later on in the code, no need to call snap-seccomp once again | 12:38 |
| mborzecki | Chipaca: yes, that would be it | 12:38 |
| jdstrand | mborzecki: I recall looking at that and had some trouble. what are you suggesting? | 12:39 |
| jdstrand | mborzecki: reading it off the disk? | 12:39 |
| Chipaca | my typo-ing rate today is quite high | 12:39 |
| Chipaca | seem to be typing phonetically a lot | 12:39 |
| mborzecki | jdstrand: iirc version info is kept in seccomp backend struct | 12:39 |
| jdstrand | mborzecki: (this was the bit where I said 'there were multiple ways of doing this' in my PR | 12:39 |
| jdstrand | mborzecki: that isn't available way out in overlord/snapstate | 12:40 |
| jdstrand | mborzecki: I was hoping to extract it from somewhere via interfaces/system_key.go but resorted to a callout to snap-seccomp | 12:41 |
| Chipaca | augh. There's a Jesse Lenton somewhere in Winnipeg that's receiving some documents via a secure online thing, but they've given my email for it β¦ so if anybody wants to find out some dirt about some employees of some random corp in canada, hit me up | 12:42 |
| jdstrand | mborzecki: afaict, snapd doesn't maintain system-key in memory. it has it on disk and at strategic times, recalculates and compares to waht is on disk | 12:43 |
| mborzecki | jdstrand: hm that's probably easier, the cache info is well hidden under interfaces :/ | 12:43 |
| jdstrand | yeah | 12:43 |
| jdstrand | mborzecki: but I could read it from disk | 12:43 |
| jdstrand | mborzecki: I was somewhat hesitant of that though | 12:44 |
| jdstrand | I mean, it *should* be ok | 12:44 |
| mborzecki | jdstrand: imo calling the compiler again sounds reasonable | 12:44 |
| jdstrand | mborzecki: ok, that is where I landed. I'll add the fourth field then I just have the one callout | 12:45 |
| mborzecki | jdstrand: unless, there would be like an instance of seccomp.Compiler() set up to use snap-seccomp kept somewhere | 12:45 |
| mborzecki | jdstrand: hm otoh, that microoptimization is probably not worth it, so a second call to snap-seccomp sounds ok to me | 12:46 |
| jdstrand | mborzecki: again, where I landed with 'picked something that made sense to me' :) | 12:47 |
| jdstrand | mborzecki: thank you for discussing this here | 12:47 |
| * Chipaca wanders off to make tea | 12:55 | |
| === chihchun is now known as chihchun_afk | ||
| zyga | Chipaca: thanks for sharing that error talk! | 13:33 |
| zyga | I'm very interested in error handling in both Go and C | 13:33 |
| Chipaca | zyga: there's also a very good one about constants I think you'd enjoy | 13:33 |
| Chipaca | zyga: https://www.youtube.com/watch?v=pN_lm6QqHcw | 13:34 |
| zyga | Chipaca: opened in another tab | 13:34 |
| zyga | https://github.com/snapcore/snapd/pull/6785 is green | 13:35 |
| * zyga uses social engineering tricks to get reviews | 13:35 | |
| * Chipaca hires a russian bot farm to -1 zyga's PRs and thwart his social engineering campaign | 13:36 | |
| * Chipaca buys some sweets with the change | 13:36 | |
| zyga | Chipaca: we could move code review to twitter and facebook :D | 13:36 |
| zyga | Chipaca: so can we start to stop using fmt.Erorrf :) | 13:37 |
| Chipaca | zyga: well we shouldn't :-) | 13:37 |
| Chipaca | because they hacked fmt.Errorf :-D | 13:37 |
| Chipaca | (continue the talk) | 13:37 |
| zyga | drat, that's like we watch a movie together but you know the ending :D | 13:37 |
| * zyga watches | 13:37 | |
| Chipaca | as long as it's fmt.Errorf("yadda yadda %v", err), or %s err, it'll DTRT | 13:37 |
| zyga | oh | 13:38 |
| zyga | tricky | 13:38 |
| zyga | it groks the argument is the wrapped error? | 13:38 |
| Chipaca | zyga: also the error crawls up its butt and explodes it from the inside | 13:38 |
| zyga | and stores it as well? | 13:38 |
| Chipaca | something like that? i haven't tried it :-) | 13:38 |
| zyga | juju/errors :) | 13:39 |
| zyga | nice | 13:39 |
| zyga | Chipaca: so will golang 2.0 switch to spaces and ban tabs? | 13:39 |
| zyga | or will it be something we can reasonably be on in the next 12 months after it ships? | 13:39 |
| Chipaca | all those big 2.0 features are getting worked into 1.x | 13:40 |
| Chipaca | that was one of the things that came out of the 2.0 discussions | 13:40 |
| zyga | so what will 2.0 bring? | 13:40 |
| zyga | that's not in 1.x? | 13:40 |
| zyga | interesting | 13:41 |
| Chipaca | zyga: actual incompatible things left over? | 13:41 |
| zyga | using interfaces is so natural in go | 13:41 |
| Chipaca | zyga: basically they don't want to be python | 13:41 |
| zyga | that's a neat idea | 13:41 |
| zyga | Chipaca: a bit of what I'm hearing starts to feel like objc | 13:42 |
| Chipaca | zyga: i haven't used objc in anger, so i don't have a strong opinion of it either way | 13:43 |
| Chipaca | zyga: but i also don't know what you mean :-) | 13:43 |
| zyga | Chipaca: the highly dynamic aspect | 13:43 |
| Chipaca | i think the only time i've used objc it was actually c with some weird bits (rather like how i've used c++) | 13:44 |
| zyga | the errors.As thing is odd, why on earth is it returning a boolean? | 13:45 |
| mborzecki | zyga: sounds like you really want that pkg/errors package :) | 13:47 |
| zyga | mborzecki: not sure yet | 13:47 |
| zyga | I see their point about how those are weak | 13:47 |
| Chipaca | zyga: errors.Is returns a boolean | 13:49 |
| zyga | Chipaca: yes, but As ? | 13:50 |
| zyga | Chipaca: I was expecting some form of casting but perhaps I misunderstood that | 13:50 |
| Chipaca | https://tip.golang.org/pkg/errors/#As | 13:51 |
| Chipaca | zyga: so you tell it the type by passing it a thing, so it sets that thing to the matching one and tells you that it succeeded via the bool | 13:51 |
| zyga | ah, so it writes to the target | 13:51 |
| zyga | I thought it just uses that for type information | 13:52 |
| zyga | that makes sense now | 13:52 |
| Chipaca | ye | 13:52 |
| zyga | though ugly because golang and generics but that's all right | 13:52 |
| Chipaca | zyga: (?i) makes the (containing sub)regexp case-insensitive from there to end or (?-i) | 13:56 |
| zyga | yeah | 13:57 |
| zyga | I read the replies from Maciej | 13:57 |
| zyga | mborzecki: +1 | 13:57 |
| Chipaca | zyga: you can also do (?i:regexp) and have that subregexp be case-insensitive | 13:57 |
| * zyga liked the RE_I flag or whatever that was called in other systems | 13:57 | |
| Chipaca | zyga: other systems also support these flags | 13:58 |
| Chipaca | i mean, this is taken from perl regexp syntax | 13:58 |
| Chipaca | perl, python, and pcre all also support making it case insensitive outside of the regexp though | 13:58 |
| Chipaca | and yes that is clearer, to me | 13:58 |
| Chipaca | but Β―\_(γ)_/Β― go's purportedly-perl-compatible regexps (which aren't) have that nice predictable non-pathological time characteristics, so that's nice instead | 13:59 |
| Chipaca | btw perl's regexp support for unicode is _still_ unmatched | 13:59 |
| * Chipaca looks into outsourcing reviews | 14:02 | |
| zyga | Chipaca: those russian trolls, do they do reviews too? :) | 14:09 |
| Chipaca | zyga: i'm sure they do | 14:10 |
| zyga | CHA-CHING! | 14:10 |
| cachio | mvo, could you please include this pr #6694 to next 2.39 | 14:10 |
| cachio | mvo, you merge all the PRs with milestone 2.39 right? | 14:11 |
| cachio | mvo, otherwise I can provide you a list of PRs which I need on 2.39 | 14:13 |
| zyga | Chipaca: watched both now | 14:15 |
| cachio | jamesh, hi | 14:22 |
| cachio | jamesh, I am testing snapd on 19.04 and 1 test is failing https://paste.ubuntu.com/p/YN44WYGCM5/ | 14:24 |
| cachio | doing xdg-open snap://package | 14:25 |
| cachio | jamesh, I was researching and found that it fails when the package gnome-settings-daemon is preinstalled | 14:25 |
| zyga | mvo: can we merge https://github.com/snapcore/snapd/pull/6717 | 14:26 |
| cachio | jamesh, were you aware of this problem? | 14:27 |
| Chipaca | hmmmmm | 14:38 |
| mvo | zyga: looking | 14:38 |
| mvo | zyga: scopedTracker is a strange name | 14:38 |
| zyga | mvo: perdronis changed it | 14:38 |
| Chipaca | mvo: do you know why we allow installing ubuntu-core still? | 14:38 |
| mvo | zyga: yeah | 14:38 |
| zyga | mvo: so I guess it is his informal +1 | 14:38 |
| mvo | Chipaca: wuuut? we do? | 14:38 |
| zyga | mvo: wrt to name | 14:38 |
| mvo | zyga: yeah | 14:39 |
| zyga | (insert chipaca emoji) | 14:39 |
| mvo | zyga: hahaha | 14:39 |
| zyga | I don't care, it's important to fix the bug more than to have a fun name | 14:39 |
| mvo | zyga: I'm with you on that | 14:39 |
| Chipaca | mvo: yes we do. Needs to be part of a multi-snap install; we block it for single snaps | 14:39 |
| zyga | mvo: shall I merge it then? | 14:40 |
| mvo | zyga: yes, looks sane | 14:41 |
| Chipaca | zyga: there is as yet no chipaca emoji. But we've got a mate emoji, so the chipaca can't be far behind! | 14:41 |
| mvo | Chipaca: aha, I see | 14:41 |
| Chipaca | mvo: i'll fix | 14:41 |
| zyga | mvo: merging, thanks! | 14:41 |
| === chihchun_afk is now known as chihchun | ||
| mvo | Chipaca: thank you | 14:44 |
| Chipaca | mvo: https://github.com/snapcore/snapd/pull/6786 fwiw | 14:57 |
| mvo | thanks Chipaca | 15:06 |
| zyga | Chipaca: s/also// | 15:06 |
| zyga | Chipaca: +1 though a spread test would be nice | 15:07 |
| Chipaca | zyga: why no also? | 15:07 |
| zyga | it reads oddly, not sure ;) | 15:07 |
| zyga | saying | 15:07 |
| Chipaca | ah ok | 15:07 |
| zyga | saying "daemon: verify snap instructions for multi-snap requests" reads better | 15:07 |
| Chipaca | boom | 15:07 |
| zyga | thanks! | 15:08 |
| Chipaca | i also removed the extra space your regexp left behind | 15:08 |
| zyga | darn, I was thinking about that | 15:09 |
| zyga | :D | 15:09 |
| zyga | ok | 15:09 |
| zyga | last thing of the day: spread test for the mount bug | 15:09 |
| Odd_Bloke | jdstrand: I'd love your thoughts on https://forum.snapcraft.io/t/potential-additions-to-account-control-interface/11123/1 | 15:35 |
| Chipaca | Odd_Bloke: is the snap you're building only for use on ubuntu core? | 15:37 |
| Odd_Bloke | Chipaca: At the moment, I'm just doing exploratory work to see what would be required to strictly confine cloud-init; this was one of the things that I found I had to add a `system-files` interface for, despite us having an interface that sounds like it would include them. | 15:39 |
| === chihchun is now known as chihchun_afk | ||
| Chipaca | Odd_Bloke: account-control is all about core tho | 15:40 |
| Odd_Bloke | There is nothing in the docs that indicates that to be the case, AFAICT? | 15:40 |
| Chipaca | the docs might need updating; it's an old interface | 15:41 |
| Chipaca | # Only allow modifying the non-system extrausers database | 15:41 |
| Chipaca | ^ that's from the code | 15:41 |
| Odd_Bloke | The docs say "account-control allows managing non-system user accounts." | 15:41 |
| Chipaca | yes i can read | 15:41 |
| Chipaca | sigh | 15:41 |
| Chipaca | Odd_Bloke: sorry, not sure where that came from | 15:41 |
| Chipaca | Odd_Bloke: yes the docs might need updating, if I've got this right | 15:42 |
| Chipaca | i'm not an expert on this bit of the code, jdstrand was the right person to ask | 15:42 |
| Chipaca | but, from what's in the code, this will only work in core | 15:42 |
| jdstrand | I responded | 15:45 |
| Odd_Bloke | Thanks! | 15:46 |
| * cachio lunch | 15:47 | |
| jdstrand | ok, rewrote the actlog PR: https://github.com/snapcore/snapd/pull/6787 | 15:49 |
| jdstrand | it was a more involved pr, but cleaner in the end, so I thing that is a good thing | 15:49 |
| Chipaca | zyga: hostname-control might need a similar warning? | 15:55 |
| Chipaca | zyga: OTOH if they're only for core, why have them in classic at all? | 15:55 |
| zyga | Chipaca: TIF | 15:55 |
| Chipaca | ! | 15:55 |
| * Chipaca goes | 15:55 | |
| Odd_Bloke | I think hostname-control is useful on classic? | 15:56 |
| Odd_Bloke | It gives write access to /etc/hostname and exec on hostnamectl. | 15:56 |
| jdstrand | Odd_Bloke: sure. it has no restrictions like that (just rememeber to snap connect it) | 15:58 |
| jdstrand | (it is manually connected by default, which can be adjusted with a snap declaration) | 15:58 |
| Odd_Bloke | I also had to add a few system-files rules to allow me to manage networking from cloud-init (on classic); is network-control expected to be a core-only thing, or is that worth opening up a topic for? | 15:59 |
| jdstrand | Odd_Bloke: it is for anyone. please open a topic | 16:03 |
| Odd_Bloke | Will do. | 16:03 |
| jdstrand | Odd_Bloke: I'll be doing some profile bug fixes for 2.39 and will add that to the list | 16:03 |
| Odd_Bloke | Oh, actually, I hadn't seen network-setup-control until I typed in my topic title, let me see if that makes a difference. | 16:06 |
| Odd_Bloke | OK, that's simple enough to just read. | 16:07 |
| zyga | jdstrand: does https://forum.snapcraft.io/t/snapping-a-font/11122/2?u=zyga sound sensibly? | 16:23 |
| jdstrand | zyga: it would help. with fonts it probably isn't bad. with arbitrary content like images, sounds, movies it would be rather heavyweight | 16:27 |
| Odd_Bloke | jdstrand: https://forum.snapcraft.io/t/network-control-network-setup-control-doesnt-enable-running-netplan-generate-on-classic/11126 | 16:33 |
| zyga | jdstrand: fonts contain full VMs nowadays, you know? | 16:36 |
| Odd_Bloke | ISTR there were font cache versioning problems recently, too? | 16:37 |
| Odd_Bloke | That might make fonts a little trickier? | 16:37 |
| Odd_Bloke | (I recall none of the details.) | 16:37 |
| zyga | Odd_Bloke: yes, at runtime, but not for distributing the font by itself I think | 16:37 |
| zyga | but yeah | 16:37 |
| zyga | it compounds the problem | 16:37 |
| Chipaca | zyga: wrt fonts etc, couldn't we sanity-check them server-side? | 16:38 |
| zyga | Chipaca: that's an interesting idea, perhaps we could | 16:38 |
| zyga | Chipaca: but it would be tricky for one reason | 16:39 |
| zyga | Chipaca: when you find that you were not checking for something that is evil and you said it was fine before you have to revoke that decision | 16:39 |
| zyga | Chipaca: in a filter approach you can provide a refresh to the filter package and re-generate content | 16:39 |
| Chipaca | zyga: yeah | 16:39 |
| zyga | Chipaca: thereby neutering the invalid blob | 16:39 |
| zyga | Chipaca: I think it's all a scale of paranoia | 16:40 |
| zyga | one could say that some type of content is plausibly ok to ship without a filter | 16:40 |
| Chipaca | CM fonts | 16:40 |
| Chipaca | \o/ | 16:40 |
| zyga | Chipaca: some things can be chameleons and can be interpreted as many things at once | 16:40 |
| zyga | it's complex but for some stuff I'd love to have it | 16:40 |
| zyga | wallpapers and fonts are a good candidate | 16:40 |
| Chipaca | fonts should be shipped only as metafont instructions, launchpad builds it into a snap that's gtg | 16:40 |
| zyga | jdstrand: https://github.com/snapcore/snapd/pull/6787#pullrequestreview-231254474 | 16:47 |
| Chipaca | i now have two (2! which is also 2) PRs that are green and just need another +1 to land | 16:48 |
| * Chipaca also has more prs than just those, but these two are Special | 16:49 | |
| * Chipaca puts the PRs in the short bus | 16:49 | |
| zyga | Chipaca: wanna read https://github.com/snapcore/snapd/pull/6785 ? | 16:54 |
| zyga | I could bug mvo for +2 and then open the next chunk :) | 16:54 |
| zyga | (only about 5 left) | 16:55 |
| zyga | this is a +12 β13 PR so pretty painless | 16:55 |
| ijohnson | jdstrand: any chance you'll get a chance to review 6697 (about daemon-notify) soonish? if not that's fine I can workaround it but it would be better to know ahead of time if that workaround is likely to be necessary | 16:55 |
| Chipaca | zyga: why is 1000 the right thing instead of os.Getuid() | 16:59 |
| Chipaca | zyga: ? | 16:59 |
| jdstrand | ijohnson: the workaround is likely necessary. possibly next week | 17:01 |
| zyga | Chipaca: because the test uses 1000 a line above :) | 17:14 |
| zyga | it's a fixed value | 17:14 |
| zyga | Chipaca: read the 2nd commit plase | 17:14 |
| zyga | *please | 17:14 |
| zyga | Chipaca: sorry for the lag, I went upstairs to stretch and found pizza lefttovers :D | 17:14 |
| zyga | *leftovers | 17:15 |
| zyga | mvo: pretty please a 2nd review on https://github.com/snapcore/snapd/pull/6785 | 17:16 |
| zyga | Chipaca: thank you! | 17:16 |
| zyga | Chipaca: for on this line https://github.com/snapcore/snapd/pull/6785/commits/0917f2460ae1d8dbe19823c32d79773add5addb1#diff-fca81d6060b39fa4cf7450a9d05bd004R384 | 17:17 |
| * zyga EODs | 17:28 | |
| ijohnson | jdstrand: ack thanks | 18:21 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!