[00:08] <Chipaca> not finished, will carry on tomorrow
[00:08] <Chipaca> off to zzz :-)
[00:54] <mup> PR snapd#4835 opened: tests: add bionic system to google backend <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/4835>
[05:27] <zyga-ubuntu> Good morning
[06:07] <mborzecki> morning
[07:15] <zyga-ubuntu> re!
[07:15] <zyga-ubuntu> kids are handled, wife at work, dog is sleeping (again) and I can finally get to work :)
[07:19] <mborzecki> zyga-ubuntu: hey
[07:19] <zyga-ubuntu> hey :-)
[07:21] <mborzecki> zyga-ubuntu: so the kernel bug with dangling symlinks is real, quite a lot of these came up in your pr
[07:21] <zyga-ubuntu> yes, it's real
[07:21] <zyga-ubuntu> now it's unclear if it is related to the issues we saw
[07:21] <zyga-ubuntu> or if it is just a harmless issue
[07:22] <mborzecki> zyga-ubuntu: does it also appear in debian sid, or is this part of apparmor ubuntu specific?
[07:22] <zyga-ubuntu> I don't know
[07:23] <zyga-ubuntu> my gut feeling is that it's not specific to ubuntu
[07:25] <zyga-ubuntu> oops, I managed to get an appointment with a doctor at 11
[07:36] <zyga-ubuntu> good morning mvo
[07:36] <mvo> zyga-ubuntu: hey! good morning
[07:39] <mborzecki> mvo: hey
[07:39] <mvo> hey mborzecki
[07:39] <mvo> time for 2.32~rc2!
[07:43] <mvo> zyga-ubuntu: sorry for being a pain, what is the (rough) status of the layout fix we need for 2.32?
[07:44] <zyga-ubuntu> no change since last week, I know what the problem is and I worked on the first of the two problems (symlink problem) but put this on hold earlier this week
[07:44] <zyga-ubuntu> I'm adding unit tests
[07:44] <zyga-ubuntu> though I think I will be changing that code a little as well
[07:44] <mvo> zyga-ubuntu: thanks!
[07:44] <zyga-ubuntu> so the status is: known bugs, in progress
[07:44] <mvo> zyga-ubuntu: could we disable parts of the functionality to get there quicker? or is that a) stupid b) not-needed c) all of the above?
[07:45] <zyga-ubuntu> it's not worth it, we need the experimental feature flag anyway
[07:45] <mvo> zyga-ubuntu: aha, ok
[07:45] <zyga-ubuntu> the symlink bug is "don't make symlinks if one is in place and has good value"
[07:45] <zyga-ubuntu> the writability leak problem is "don't write over things that are not tmpfs"
[07:46] <mborzecki> hm the opensuse kernel that we use has apparmor disabled apparently
[07:46] <mborzecki> uname shows 4.15.7-x86_64-linode102, linode specific kernel?
[07:47] <zyga-ubuntu> yes, apparently
[07:47] <mvo> anyone working on the vet failures on 18.04? if not I will do now
[07:48] <mborzecki> damn, kernel-default-4.4.27-2.1.x86_64 this is installed, but it's running a different kernel, pfff
[07:48] <mborzecki> mvo: i can take a look
[07:49] <mvo> mborzecki: its about the vet failures in the test log of 4835
[07:50] <zyga-ubuntu> there are some people who want to contribute to the gentoo overlay
[07:50] <zyga-ubuntu> https://github.com/zyga/gentoo-snappy/issues
[07:50] <zyga-ubuntu> some PRs and issues
[07:50] <mup> PR snapd#4803 closed: snap-confine, snap-seccomp: utilize new seccomp logging features - 2.32 <Created by jdstrand> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/4803>
[07:50] <mup> PR snapd#4831 closed: tests: force profile re-generation via system-key <Created by mvo5> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/4831>
[07:51] <mup> PR snapd#4828 closed: many: support holding refreshes by setting refresh.hold (2.32) <Critical> <Created by pedronis> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/4828>
[07:52] <mvo> zyga-ubuntu: how safe is 4830?
[07:53] <mvo> zyga-ubuntu: I am inclinded to merge
[07:53] <zyga-ubuntu> I don't know of any issues
[07:53] <mvo> zyga-ubuntu: this runs in edge for some time, right?
[07:53] <zyga-ubuntu> yes
[07:53] <zyga-ubuntu> I think it's safe
[07:53] <zyga-ubuntu> also on reverts
[07:53] <zyga-ubuntu> since reverting brings the reverted snap-confine
[07:53] <zyga-ubuntu> and its own profile and startup behavior
[07:54] <zyga-ubuntu> the only potential downside is if we made a mistake in the apparmor profiles
[07:54] <mvo> ok
[07:54] <zyga-ubuntu> but nobody complained so far
[07:54] <mup> PR snapd#4830 closed: many: generate and use per-snap snap-update-ns profile (2.32) <Created by zyga> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/4830>
[07:55] <zyga-ubuntu> mvo: hmmmm
[07:55] <zyga-ubuntu> https://github.com/snapcore/snapd/pull/4765
[07:55] <mup> PR #4765: interfaces: harden snap-update-ns profile <Created by zyga> <https://github.com/snapcore/snapd/pull/4765>
[07:55] <zyga-ubuntu> actually, I'm drunk
[07:55] <zyga-ubuntu> since that PR you just merged is just the split of the profiles
[07:55] <zyga-ubuntu> this is the hardening
[07:56] <zyga-ubuntu> and this is the part when I meant that maybe we missed something
[07:56] <zyga-ubuntu> the first PR is safe
[07:56] <zyga-ubuntu> mvo: this PR is the interesting one that jamie wanted to see.
[07:56] <zyga-ubuntu> man, I thought we merged it already
[07:57] <zyga-ubuntu> and is full of policy
[07:57] <zyga-ubuntu> mvo: can you review it?
[07:58] <mvo> zyga-ubuntu: yes, in a bit, let me go over the essential bits for 2.32 first
[07:58] <zyga-ubuntu> mvo: I think this is also for 2.32 but I understand if you lack it
[07:58] <zyga-ubuntu> NACK it
[07:58] <zyga-ubuntu> (silly spellchecker on macOS)
[08:00] <kalikiana> moin moin
[08:01] <zyga-ubuntu> hey kalikiana
[08:03] <mup> PR snapd#4836 opened: tests: force profile re-generation via system-key <Created by mvo5> <https://github.com/snapcore/snapd/pull/4836>
[08:06] <pstolowski> morning!
[08:06] <mborzecki> kalikiana: pstolowski: morning guys
[08:07] <mborzecki> woow, we use testable examples in snap/info_test.go, never seen it being used before
[08:07] <mvo> mborzecki: we want 4826 - right?
[08:07] <mborzecki> mvo: for 2.32?
[08:07] <mvo> mborzecki: I mean we want it for 2.32
[08:07] <mvo> mborzecki: yes
[08:07] <mvo> pstolowski: good morning!
[08:08] <mborzecki> mvo: yes, it'd be nice to include it, it's a bugfix :)
[08:08] <mvo> pedronis: 4825 looks almost ready, just one question from gustavo left afaict?
[08:08] <mvo> mborzecki: cool, lets see if we can find a second reviewer
[08:15] <mvo> zyga-ubuntu: do we know how the feature flag will look like? environment key?
[08:16] <zyga-ubuntu> Core setting
[08:16] <zyga-ubuntu> snap set core experimental.layouts true
[08:16] <mborzecki> hmm vet does not handle struct methods defined in export_test.go files
[08:19] <mvo> zyga-ubuntu: do you have code for this or shall I add some? asking because I think with that in place ~rc2 would be ready
[08:20] <mvo> zyga-ubuntu: I can work on it
[08:29] <mborzecki> https://github.com/golang/go/issues/23701
[08:33] <zyga-ubuntu> mvo: I don't have that yet
[08:33] <zyga-ubuntu> so if you want to pick it up, thanks please do
[08:33] <mvo> mborzecki: hm, this reads as if the issue should be fixed? do we just need a golang update in bionic?
[08:34] <mborzecki> go version go1.10 linux/amd64 locally and i can still see this
[08:34] <mvo> zyga-ubuntu: yeah, I want 2.32~rc2 today if layout have bugs thats ok, we need one more upload for 2.32(-final) anyway
[08:34] <zyga-ubuntu> ack
[08:34] <mvo> mborzecki: hm, does that have https://go-review.googlesource.com/c/go/+/92215 ?
[08:36] <mborzecki> mvo: yes, the commit is in 1.10 branch
[08:37] <mvo> :(
[08:37] <mvo> ok
[08:37] <mvo> mborzecki: if you have a fix for the other vet issues, please push
[08:41] <mup> PR snapd#4837 opened: many: go vet cleanups <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/4837>
[08:41] <mborzecki> mvo: ^^
[08:41] <mvo> mborzecki: \o/
[08:42] <mborzecki> as for the bug that's supposed to be resolved in 1.10, i can still reproduce it with the sample that's provided in the original ticket on both 1.10 from golang.org download and 1.10 from arch packages
[08:43] <mvo> mborzecki: ok, so we need to reopen upstream? and/or find a local workaround :/
[08:44] <mvo> and/or disable vet on go1.10 (which would suck)
[08:45] <mborzecki> i have a sort of a workaround but it's labor intensive :)
[08:45] <mborzecki> basically changing the struct methods to simple funcs
[08:46] <mborzecki> and update all call sites
[08:47] <mvo> mborzecki: hm, that sounds slightly nasty
[08:50] <Chipaca> moin moin
[08:51]  * Chipaca realises he still has time for more coffee before official start-of-day
[08:52] <mborzecki> mvo: it isn't that bad https://paste.ubuntu.com/p/ZPgmsKpM6s/
[08:53] <mborzecki> mvo: i can push that too
[08:53] <mvo> mborzecki: yeah, that looks reasonable
[08:53] <mvo> mborzecki: unfortunate but reasonable
[08:53] <mvo> Chipaca: hey! good morning!
[08:54] <mborzecki> Chipaca: morning, more coffee yes, that's a splendid idea :P
[08:54] <om26er> elopio: Hi! I see you did initial snap packaging for keybase client. Are there any blockers on the way, anyway I could help get that finished ?
[08:55]  * Chipaca hugs elopio 
[08:56]  * Chipaca hugs his coffee, also
[08:58] <mborzecki> mvo: pushed the workarounds
[09:02] <mborzecki> need to run an errand, bb in ~1.5h
[09:27]  * zyga-ubuntu -> doctor
[09:27] <zyga-ubuntu> ttyl
[09:40] <mup> PR snapd#4838 opened: snapstate: put layout feature behind feature flag <Created by mvo5> <https://github.com/snapcore/snapd/pull/4838>
[09:57] <zyga-ubuntu> mvo: commented, thank you for writing that
[09:57]  * zyga-ubuntu waits at the doctors
[09:58] <zyga-ubuntu> This will take a while (hopefully not too long)
[10:04] <mvo> zyga-ubuntu: thank you
[10:08] <mvo> zyga-ubuntu: heh, spread tests> probably, I guess the spread run will be very unhappy!
[10:08] <mvo> zyga-ubuntu: good catch
[10:08] <Chipaca> mvo: can you remind me what advise-snap's supposed to do without --command?
[10:09] <mvo> Chipaca: advice on snap name
[10:09] <Chipaca> mvo: what's the apt equivalent of that one?
[10:09] <mvo> Chipaca: so that "apt install spotify" could say something nice like "The spotify snap is available at version 2.0, see snap info spotify for additional versions"
[10:09] <Chipaca> ah
[10:09] <Chipaca> a'ight
[10:09] <mvo> Chipaca: its a bit under speced right now
[10:09] <mvo> Chipaca: we also probably want mispell there
[10:09] <mvo> (which should be trivial)
[10:11] <Chipaca> speling is trivial indeed
[10:17] <Chipaca> we really struggle with 'temporarily'
[10:17] <Chipaca> so far i've found temporarely and temporariy
[10:23] <mvo> Chipaca: *cough* s/we/I/
[10:24] <zyga-ubuntu> mvo: I will miss standup. I need to do an X-ray now :-(
[10:25] <zyga-ubuntu> Not sure how soon I will be back today
[10:25] <zyga-ubuntu> I can work on reviews from my phone
[10:26] <mvo> zyga-ubuntu: uhh, good luck
[10:27] <pstolowski> yay, found a bug in go-udev
[10:27] <zyga-ubuntu> pstolowski: that was quick
[10:30] <pstolowski> zyga-ubuntu: it's a small project and just a few functions really. and fix is trival thankfully
[10:37] <Chipaca> huh, there's a new rpi
[10:38] <zyga-ubuntu> Yep
[10:38] <zyga-ubuntu> And it has a heat spreader on the chip
[10:38] <zyga-ubuntu> And does PoE with a hat
[10:38] <zyga-ubuntu> That latter thing makes it really interesting but there is no information about the availability of the hat yet
[10:39] <Chipaca> I didn't know Poe used a hat
[10:39]  * Chipaca imagines ravens with trilbies
[10:40] <Chipaca> anyway, bbiab
[10:51] <zyga-ubuntu> Chipaca it is a very stylish poe
[10:51]  * zyga-ubuntu waits for X-ray
[10:55] <mup> PR snapd#4839 opened: errtracker: respect the /etc/whoopsie configuration <Created by mvo5> <https://github.com/snapcore/snapd/pull/4839>
[11:02] <zyga-ubuntu> mvo: reviewed
[11:03] <abeato> mvo, it looks like the root reason for the bug we chatted around yesterday is device cgroups: https://forum.snapcraft.io/t/race-condition-between-snapd-and-nm-when-providing-permissions/4451/3
[11:03] <abeato> zyga-ubuntu, ^^
[11:03] <zyga-ubuntu> Hey
[11:03] <abeato> zyga-ubuntu, o/
[11:03] <zyga-ubuntu> I’m AFK so sorry if I don’t respond quickly
[11:04] <zyga-ubuntu> Let me read the forum thread
[11:04] <abeato> thanks
[11:04] <zyga-ubuntu> So a quick question
[11:04] <zyga-ubuntu> What created the additional device nodes
[11:05] <abeato> zyga-ubuntu, a script that needs to set some GPIO to power on the device. That takes a few seconds
[11:05] <zyga-ubuntu> It feels like we are missing an udev rule that adds tags such devices
[11:05] <zyga-ubuntu> Is it udev tagged by any part of network manager interface?
[11:05] <abeato> the udev rule is fine, it tags /dev/tty*
[11:06] <abeato> it is in the ppp interface, used by NM
[11:06] <zyga-ubuntu> Can you please paste the udev profile generates by snapd for this snap
[11:06] <abeato> sure
[11:06] <zyga-ubuntu> Thanks
[11:06]  * zyga-ubuntu only has a phone with him
[11:07] <abeato> zyga-ubuntu, https://pastebin.canonical.com/p/jGBrPdkZZq/
[11:08] <zyga-ubuntu> Aww
[11:08] <zyga-ubuntu> My token is at home
[11:08] <zyga-ubuntu> Is it sensitive?
[11:08] <abeato> zyga-ubuntu, nah, not really: https://paste.ubuntu.com/p/sGWRcj7Ts4/
[11:09] <abeato> who creates the device cgroup, snap-confine?
[11:09] <zyga-ubuntu> Yes
[11:10] <abeato> it would make sense that it adds only the present devices
[11:10] <abeato> and if a new one appears, apparmor rules are fine thanks to udev, but not the device cgroup
[11:10] <abeato> which is not updated
[11:11] <mborzecki> re
[11:11] <abeato> (well, not thanks to udev, it is just that apparmor has a tty*)
[11:13] <zyga-ubuntu> Re
[11:13] <zyga-ubuntu> Well it should be updated
[11:13] <zyga-ubuntu> Look at those rules
[11:13] <zyga-ubuntu> We run a tool to change the device cgroup as devices come and go
[11:14] <zyga-ubuntu> Unless we are hit by a bug
[11:14] <zyga-ubuntu> We renamed that tool now
[11:15] <abeato> hm then a bug must be
[11:15] <abeato> which is that tool?
[11:15] <zyga-ubuntu> Can you confirm it exists in your system with the given path you see in the udev rule
[11:15] <zyga-ubuntu> Read the rule please
[11:15] <zyga-ubuntu> It is raining
[11:15] <zyga-ubuntu> And typing on a phone is not easy
[11:16] <abeato> sure :)
[11:16] <abeato> ok, /usr/lib/snapd/snappy-app-dev
[11:19] <zyga-ubuntu> Yes
[11:19] <abeato> zyga-ubuntu, mvo is it possible that core in candidate has been recently upgraded? now I cannot reproduce the issue anymore, although it is a race so I am not 100% sure
[11:20] <mvo> abeato: candidate got updated on monday
[11:20] <mvo> abeato: from 2.31.1 to 2.31.2
[11:20] <abeato> hm...
[11:20] <mvo> abeato: http://people.canonical.com/~mvo/core-changes/html/candidate/2.31.1r4110_2.31.2r4206.html
[11:21]  * abeato trying to reproduce again
[11:23] <zyga-ubuntu> abeato: do you see this on core or on classic
[11:23] <abeato> core
[11:23] <zyga-ubuntu> I see
[11:23] <zyga-ubuntu> Hmm hmm
[11:23] <zyga-ubuntu> Well
[11:23] <zyga-ubuntu> If it shows up please tell us
[11:23] <abeato> yup
[11:24] <zyga-ubuntu> To the best of our knowledge the system supports devices that are added after startup
[11:24] <zyga-ubuntu> One possible thing to check for would be a race during cgroup construction
[11:24] <zyga-ubuntu> That may indeed be buggy
[11:24] <zyga-ubuntu> I would have to look at the code to confirm this
[11:25] <zyga-ubuntu> Perhaps the tool should be grabbing the per-snap lock
[11:25] <zyga-ubuntu> As that would probably rule out the race
[11:25] <abeato> I can control when the device is created, will tell you if I can reproduce that way in 1min
[11:25] <zyga-ubuntu> So to be clear
[11:26] <zyga-ubuntu> If the udev event fires at the same moment as the first process of that snap
[11:26] <zyga-ubuntu> We may have an issue in that specific case
[11:26] <zyga-ubuntu> Ok
[11:26] <zyga-ubuntu> I have my X-ray but results won’t be available till 4PM, I’m heading home
[11:30] <abeato> zyga-ubuntu, definitely I can reproduce (ping me when you are back again)
[11:31] <zyga-ubuntu> Do can you reproduce that and capture udev logs as things happen?
[11:31] <abeato> yes
[11:31] <zyga-ubuntu> Please add all findings to the forum thread
[11:32] <abeato> will do, thanks
[11:32] <zyga-ubuntu> And can you look at the resulting cgroup and ensure the devices are not there
[11:32] <zyga-ubuntu> I wonder if we can make the small helper tool log something
[11:32] <abeato> that's how I checked that I reproduced it :)
[11:33] <abeato> the new devices are not in devices.list
[11:33] <abeato> (and I got the EPERM too)
[11:33] <zyga-ubuntu> Ok
[11:34] <zyga-ubuntu> Which device is that? The serial port?
[11:34] <abeato>  /dev/ttyACM0 from a modem
[11:35] <mborzecki> abeato: is there any indication in dmesg that the port may go away and appear again?
[11:35] <zyga-ubuntu> The rule looks ol
[11:35] <zyga-ubuntu> Can you check one more thing
[11:35] <zyga-ubuntu> Run the tool as udev would
[11:35] <abeato> mborzecki, it appears and it is kept there
[11:36] <zyga-ubuntu> Pass the right tings (cannot tell you from the top of my head)
[11:36] <zyga-ubuntu> And see if it gets added
[11:36] <abeato> sure, let me try that
[11:39] <abeato> zyga-ubuntu, https://paste.ubuntu.com/p/hZFQ5XS62g/ :/
[11:40] <zyga-ubuntu> Woah
[11:40] <zyga-ubuntu> That looks like a bug
[11:40] <zyga-ubuntu> Is the tool anywhere in the filesystem?
[11:40] <abeato>  /lib/udev/snappy-app-dev
[11:40] <abeato> bad path
[11:40] <jjohansen> zyga-ubuntu: just a quick update before I bail, I have a patch thats is a wip, I should have something for you to test tomorrow (today?) sometime
[11:41] <abeato> zyga-ubuntu, so there is a bad path in the udev rule, that's it
[11:41] <zyga-ubuntu> That is great news jjohansen. Thank you for getting this so quickly! I can test the patch when you have it
[11:42] <zyga-ubuntu> Mvo: we need a small fix for what abeato just found
[11:42] <zyga-ubuntu> abeato: which version is that?
[11:42] <abeato> zyga-ubuntu, 4209
[11:43] <zyga-ubuntu> And in snap —version parlance?
[11:43] <mvo> zyga-ubuntu: oh, what fix is needed?
[11:43] <zyga-ubuntu> jjohansen: is there any indication that the bug may have other side effects?
[11:43] <abeato> zyga-ubuntu, https://paste.ubuntu.com/p/tWkMJRP84Y/
[11:43] <zyga-ubuntu> mvo: udev rules use a wrong path to the helper tool
[11:44] <zyga-ubuntu> Woah
[11:44] <abeato> mvo, /usr/lib/snapd/snappy-app-dev -> /lib/udev/snappy-app-dev
[11:44] <zyga-ubuntu> abeato your snapd is wonky
[11:44] <zyga-ubuntu> Can you reproduced that on vanilla 2.31.2
[11:44] <zyga-ubuntu> I will be home soon
[11:44] <zyga-ubuntu> ~40 minutes maybe
[11:45] <mvo> abeato: this is slightly strange, the snapd version is pretty out of sync with the snap version
[11:45] <zyga-ubuntu> (That not super soon though)
[11:45] <abeato> zyga-ubuntu, mvo well, it is the core from candidate, no hacks I promish
[11:45] <zyga-ubuntu> That is cery ul ujęły
[11:45] <zyga-ubuntu> Unlikely
[11:46] <zyga-ubuntu> Did you push a locally built snapd over?
[11:46] <mvo> abeato: when I grep snappy-app-dev in master I get only a compat symlink hit, I'm a bit puzzled why its looking in /usr/lib/snapd/snappy-app-dev, is that in the udev rule? or where do you see it?
[11:47] <zyga-ubuntu> It is in the udev rule but it looks like the snapd there is old or at least based on an old dirty build
[11:48] <abeato> zyga-ubuntu, hm, wait, I remember I pushed one snapd and mounted it on boot to measure a performance issue
[11:48] <abeato> let me remove that
[11:48] <zyga-ubuntu> Thank you!
[11:56] <mup> PR snapd#4840 opened: many: add `core.problem-reports.disabled` option <Created by mvo5> <https://github.com/snapcore/snapd/pull/4840>
[11:56] <mborzecki> heh, go vet is flaky in 1.10 and even more so in master, this is what I get with master atm: https://paste.ubuntu.com/p/B2vc9F3CGr/ using a test case https://github.com/golang/go/issues/23701#issuecomment-363187224
[11:57] <mvo> mwhudson: hm, do you have an opinion about go vet in bionic? we have some problems with 1.10 -^
[11:57] <mvo> mborzecki: maybe the default of 1.10 is a bit too aggressive :/
[11:58] <mborzecki> there will be an update to 1.10.1 though?
[11:58] <mborzecki> i think i'd prefer a slightly broken version if that helps from getting stuck with 1.9 for the next 5 years
[11:59] <mvo> mborzecki: I guess, worst case is that we disable vet on 1.10 for now
[11:59] <mborzecki> mvo: the workarounds are there, we can revert them once we know it's fixed again
[11:59] <abeato> zyga-ubuntu, mvo after reverting the snapd version I see the right path in /etc/udev/rules.d/70-snap.network-manager.rules, so this was my fault. Sorry for the noise
[12:01] <mvo> mborzecki: aha, ok. I thought you mentioned there are additional errors, am I outdated :) ?
[12:01] <mvo> abeato: no worries, thanks for reporting it, better this way than if we let a bug escape :)
[12:02] <abeato> at least I've learned a bit about device cgroups ;)
[12:02] <mborzecki> mvo: i mean i tried go master, built it and was able to reproduced the problem using a test case posted in the original ticket, got a nice backtrace from go internals :P at least 1.10 did not panic
[12:04] <zyga-ubuntu> abeato: thank you for confirming that!
[12:04] <zyga-ubuntu> mvo: alarm is over :)
[12:05] <abeato> zyga-ubuntu, thank you in fact
[12:08] <zyga-ubuntu> and I'm back
[12:08] <zyga-ubuntu> resuming work on fixes
[12:08] <mvo> mborzecki: ok
[12:08] <mvo> zyga-ubuntu: thank you for handling it!
[12:26] <mup> PR snapcraft#1983 closed: snap: update revision of patchelf to use <Created by sergiusens> <Closed by sergiusens> <https://github.com/snapcore/snapcraft/pull/1983>
[12:54] <mup> PR snapd#4841 opened: snap/pack, cmd/snap: add `snap pack --check-skeleton` <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/4841>
[13:39] <zyga-ubuntu> Chipaca: would you mind if I bother you with a symlinkat review later today?
[13:39] <pstolowski> niemeyer: that's the project I was referring to: https://github.com/pilebones/go-udev
[13:39] <niemeyer> pstolowski: Yeah, that's one I looked into as well, given my browsing history
[13:39] <niemeyer> pstolowski: Looking for others now
[13:40] <Chipaca> zyga-ubuntu: yes, I'd be very upset with myself if you asking me for a review bothered me
[13:43] <Chipaca> niemeyer: wrt the 'halp' pr (#4809) your comments on 'snap pack' seem to imply you want a change of behaviour of snap pack
[13:43] <mup> PR #4809: cmd/snap: tweak and polish help strings <Created by chipaca> <https://github.com/snapcore/snapd/pull/4809>
[13:43] <niemeyer> Chipaca: Hmm.. it wasn't intentional at least
[13:43] <niemeyer> Chipaca: What did I miss?
[13:43] <Chipaca> i won't be working on that pr until this evening so there's no rush, but
[13:43] <Chipaca> niemeyer: 'snap pack' already defaults both arguments to '.'
[13:43] <niemeyer> Chipaca: Huh, ok
[13:44] <niemeyer> Chipaca: So what's the delta with try?
[13:44] <niemeyer> Chipaca: That it doens't look for "prime"?
[13:45] <Chipaca> niemeyer: essentially yes, but also it only sets it to . if there's a meta/snap.yaml in .
[13:45] <Chipaca> and it only sets it to prime if there's a snapcraft.yaml in one of the usual places
[13:45] <niemeyer> Chipaca: Okay, I indeed missed those details.. so +1 on your suggestion
[13:45] <Chipaca> (curiously it doesn't look for meta/snap.yaml in the prime directory, but that's a tweak)
[13:46] <niemeyer> Chipaca: I think we should ignore snapcraft.yaml and just look for meta/snap.yaml as you suggest
[13:46] <Chipaca> niemeyer: it's still slightly awkard that it'll happily drop a snap inside the snap (and there are snaps with snaps in them in the store, so it's a real problem)
[13:46] <niemeyer> (in prime, in .)
[13:46] <niemeyer> Chipaca: Is there a way to make snap pack ignore those?
[13:47]  * niemeyer --help
[13:47] <Chipaca> niemeyer: not currently; snapcraft has an ignore list, but not snap pack
[13:48] <Chipaca> OTOH most people will be using it via snapcraft so it'll be fine :-)
[13:48] <Chipaca> anyway, glad that's cleared up, i'll tweak the text and the behaviour (but possibly in a followup)
[13:48] <Chipaca> this evening though; now to the cache
[13:49] <Chipaca> hm, maybe, maybe target should default to the parent dir of snap-dir
[13:49] <mborzecki> Chipaca: or soem /tmp location
[13:50] <Chipaca> mborzecki: it might not be the best example, but dpkg-buildpackage plops the debs in ..
[13:50] <niemeyer> Chipaca: Yep, -e works
[13:50] <niemeyer> Chipaca: Although it's completely undocumented apparently
[13:50] <niemeyer> Chipaca: -e <filename>
[13:50] <niemeyer> Chipaca: I suggest using that for any *.snap in the root directory
[13:50] <Chipaca> niemeyer: fair 'nuff
[13:51] <niemeyer> Chipaca: I've seen actual snaps shipping the prior revision of their own snaps
[13:51] <Chipaca> niemeyer: IKR
[13:51] <niemeyer> Chipaca: Built with snapcraft
[13:51] <niemeyer> So it looks like a common issue
[13:52] <niemeyer> In the rare circumstances where people do want to ship snaps, they can just put it in a subdir
[13:53] <Chipaca> niemeyer: https://i.imgflip.com/26d5u0.jpg
[13:53] <niemeyer> Yeah :)
[13:53]  * Chipaca forgets the whole conversation and moves to the hash cache stash
[13:57] <cachio> mvo, I have tried the upgrade test issue with the last bionic image in google and I see the same problem
[13:57] <cachio> all the snaps appear as broken
[13:58] <cachio> mvo, any idea?
[13:58] <cachio> I could get a debug session with the error
[14:03] <Chipaca> my / has inode number 2
[14:03] <Chipaca> how
[14:03]  * Chipaca doesn't really care and gets back to work
[14:04] <mvo> cachio: yeah, happy to get into  a debug session
[14:04] <mvo> cachio: what did you run to trigger it?
[14:05] <cachio> mvo, https://github.com/snapcore/snapd/pull/4835
[14:05] <mup> PR #4835: tests: add bionic system to google backend <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/4835>
[14:05] <cachio> this branch
[14:05] <cachio> mvo, but using this image
[14:05] <cachio> ubuntu-os-cloud-devel/daily-ubuntu-1804-bionic-v20180307
[14:05] <cachio> which is the last one
[14:05] <cachio> mvo, nad run spread -debug google:ubuntu-18.04-64:tests/upgrade/basic
[14:31] <mborzecki> mvo: I'll close #4597 if you don't mind
[14:31] <mup> PR #4597: snapstate: allow core config via $core <Blocked> <Created by mvo5> <https://github.com/snapcore/snapd/pull/4597>
[14:39] <mvo> mborzecki: ok, hope some of the code will still be useful for your work in this area
[14:40] <mborzecki> mvo: yes, i'll pull in your commit and work on top of it
[14:42] <mup> PR snapd#4597 closed: snapstate: allow core config via $core <Blocked> <Created by mvo5> <Closed by bboozzoo> <https://github.com/snapcore/snapd/pull/4597>
[14:43] <mvo> mborzecki: \o/
[14:44] <mup> PR snapd#4842 opened: interfaces: make hash of apparmor profile for snap-confine in core part of the system-key <Blocked> <Created by mvo5> <https://github.com/snapcore/snapd/pull/4842>
[14:44] <mvo> ^- this is to test the theory that the system-key needs extending to get rid of the errors we saw with 2.32 and the out-of-sync of the profiles, please ignore until green
[14:44] <zyga-ubuntu> mvo: interesting
[14:45] <mvo> zyga-ubuntu: yeah, it usually this is covered by the core rev but not in our tests
[14:45] <mvo> zyga-ubuntu: lets see if its green or not
[15:02] <kyrofa> Hey Chipaca, if/when you're able, can I get a sanity check review from you on a snapcraft PR? We're adding the ability to call functions from scriptlets which are then handled by the running Snapcraft instance, and we're doing it via fifos: https://github.com/snapcore/snapcraft/pull/2002
[15:03] <mup> PR snapcraft#2002: pluginhandler: add builtin functions to scriptlets <Created by kyrofa> <https://github.com/snapcore/snapcraft/pull/2002>
[15:03] <Chipaca> kyrofa: fifos, as in fifo(7)?
[15:04] <kyrofa> Yessir
[15:05] <kyrofa> Relevant parts of the PR are python (https://github.com/snapcore/snapcraft/pull/2002/files#diff-313e8dc0aa8d02f56e6ec918c25ddd93R74) and shell (https://github.com/snapcore/snapcraft/pull/2002/files#diff-8b247ab4b649deb9ee488e09837eba39R30)
[15:05] <mup> PR snapcraft#2002: pluginhandler: add builtin functions to scriptlets <Created by kyrofa> <https://github.com/snapcore/snapcraft/pull/2002>
[15:19] <kyrofa> Wonder if it would be simpler with a more snapctl-like approach
[15:23] <Chipaca> kyrofa: what does the 'status' variable get used for?
[15:23] <kyrofa> Chipaca, to block
[15:23] <kyrofa> Until the function has been handled
[15:23] <kyrofa> Otherwise once the "call" fifo has been read, the scriptlet goes on its merry way
[15:25] <Chipaca> kyrofa: does this work?
[15:26] <kyrofa> Chipaca, in my testing anyway, yeah. Do you see issues?
[15:26] <Chipaca> kyrofa: only things that seem strange but they might be connected outside of my limited window into the code
[15:26] <Chipaca> like, cwd of the popen is not tempdir
[15:27] <Chipaca> but it finds the fifos
[15:27] <Chipaca> ?
[15:27] <kyrofa> The fifos are an absolute path, handed to the script run by the popen
[15:28] <Chipaca> ah :-)
[15:31] <Chipaca> hmmm
[15:33] <Chipaca> kyrofa: another question: why nonblocking?
[15:34] <kyrofa> Chipaca, on the Python side? So that the scriptlet can actually exit and Snapcraft can move on even if they don't utilize the fifos
[15:34] <kyrofa> Otherwise Snapcraft will block checking for a function call, and the scriptlet will exit in the meantime
[15:41] <Chipaca> kyrofa: i'm having trouble hitting that failure mode
[15:41] <cachio> Pharaoh_Atem, hey
[15:42] <cachio> Pharaoh_Atem, do you haave time to continue with the dbus test?
[15:42] <Pharaoh_Atem> cachio: I will in ~20 minutes
[15:42] <cachio> Pharaoh_Atem, great, thanks
[15:43] <cachio> Pharaoh_Atem, I'll prepare the environment in that case
[15:43] <Chipaca> kyrofa: granted my code is a butchering of yours just to test it (mostly because my experience with fifos is that they are finickity)
[15:45] <kyrofa> Chipaca, that's exactly the kind of info I was hoping you'd have! This approach does not give you warm and fuzzies, then?
[15:45] <Chipaca> kyrofa: it can work, and when it does it's brilliant
[15:45] <Chipaca> kyrofa: and most of my bad experiences have been around things deadlockinig, which the nonblocking thing should avoid? I think
[15:45] <Chipaca> if i understand it correctly at least
[15:46] <kyrofa> Indeed. But your hacking is confusing, let me try removing the non-blocking and see if I can get it to fail like I expect, hold on
[15:46] <Chipaca> but, with it nonblocking you do end up spinning quite a bit
[15:46] <kyrofa> Yeah totally
[15:46] <kyrofa> It's not beautiful
[15:50] <kyrofa> Yeah Chipaca remove ` | os.O_NONBLOCK` in `_NonBlockingFifo` and then run a YAML that looks like this: https://pastebin.ubuntu.com/p/V2tZ7GfzBn/
[15:50] <kyrofa> Whereas, if you add a `snapcraft_build` call to the bottom of the `build` scriptlet, things start working again since you wrote to the fifo to unblock snapcraft
[15:54]  * zyga-ubuntu goes to pick up xray results, ttyl
[15:55]  * Chipaca plays with this
[16:02] <cachio> Pharaoh_Atem, env ready
[16:03] <Chipaca> kyrofa: I'm not seeing it getting blocked
[16:03] <kyrofa> Huh. I can't explain that
[16:03] <Chipaca> kyrofa: https://pastebin.ubuntu.com/p/xpPwjyvKQS/
[16:04] <Chipaca> that's with O_NONBLOCK commented out
[16:06] <Chipaca> kyrofa: but it might not be picking up my changes
[16:06] <kyrofa> Chipaca, how do you have it installed?
[16:06] <kyrofa> venv?
[16:07] <Chipaca> kyrofa: i don't, i'm running it from the git repo + venv
[16:07] <Chipaca> i don't think i installed it to the venv
[16:07]  * Chipaca checks
[16:07] <Chipaca> dangit
[16:07] <Chipaca> yes it's there
[16:07] <kyrofa> And it's installed into the venv with --editable ?
[16:07] <Chipaca> there we go
[16:08] <kyrofa> Oh phew
[16:08] <Chipaca> kyrofa: somehow doing 'pip install -r requirements.txt .' (as per HACKING) also installed snapcraft itself
[16:08] <Chipaca> i guess that's the . there :-)
[16:09] <kyrofa> Indeed. Although below is the guide for actually _developing_ using that method, which requires --editable
[16:09] <kyrofa> Although even that falls on its face sometimes
[16:09] <kyrofa> Every once in a while I need to reinstall it and I never know why
[16:10] <Chipaca> kyrofa: is there a way to tell it to do a clean and then a build?
[16:10] <kyrofa> snapcraft clean && snapcraft build
[16:10] <Chipaca> or to jfd a build?
[16:10] <Chipaca> eh, fair enough
[16:16] <pstolowski> Chipaca: thanks for staying vigiliant with serial-port interface regexp... completely overlooked what you spotted!
[16:17] <Chipaca> pstolowski: no
[16:17] <Chipaca> pstolowski: np*
[16:17] <Chipaca> :-)
[16:17] <Chipaca> kyrofa: i need to go for a bit, but i'll carry on looking when ig et back
[16:17]  * Chipaca afk
[16:18] <pstolowski> Chipaca: it looked very innocent :}
[16:18] <kyrofa> Chipaca, of course, I appreciate your time!
[16:42] <cachio> Pharaoh_Atem, there?
[16:53] <Chipaca> pstolowski: IKR, I wouldn't've noticed if the regexp hadn't been blatently wrong
[16:53] <Chipaca> kyrofa: *blush*
[17:10] <popey> When do we plan to have overlay support landed?
[17:11] <jjohansen> zyga-ubuntu: no, no indication that the bug has other side effects (well it will break LXD checkpoint/restore). The symlink isn't getting properly updated on profile replacement, to point to the new blob.
[17:22] <Chipaca> kyrofa: does this code need to run on non-linux?
[17:23] <kyrofa> Chipaca, I don't believe so, no
[17:29] <Chipaca> kyrofa: my suggestion about O_RDRW is non-portable, you see :-)
[17:29] <kyrofa> Chipaca, are FIFOs portable?
[17:30] <Chipaca> kyrofa: not to cp/m, dos, nor derivatives
[17:30] <Chipaca> :-)
[17:31] <Chipaca> kyrofa: but to unices yes
[17:48]  * kalikiana heading out o/
[18:01] <Chipaca> niemeyer: question about the approach to the cache
[18:03] <Chipaca> niemeyer: currrently asserts.SnapFileSHA3_384 calls osutil.FileDigest to get the digest and then asserts.EncodeDigest to format it (prepend sha3-384: etc)
[18:04] <niemeyer> Chipaca: Ok
[18:04] <Chipaca> niemeyer: writing osutil.GetPathInfo, I can either make it receive a hasher (the caller would then wrap and pass in SnapFileSHA3_384), or i could move SnapFileSHA3_384 and EncodeDigest to osutil, or I could move GetPathInfo to asserts (or elsewhere)
[18:05] <Chipaca> niemeyer: the disadvantage of the hasher is that it makes it awkward to add more 'expensive' stuff, if everything is passed in
[18:05] <Chipaca> but otherwise it seems fine
[18:07] <Chipaca> niemeyer: so the question is, which approach do you think is best?
[18:07] <niemeyer> Chipaca: Yeah, I think it should compute internally to make it convenient.. I'm just wondering if it's a bit of a weak design in that if we introduce further hashes, we'll probably want to read the file only once to do them all, instead of asking N hashing functions to each compute it again
[18:08] <niemeyer> Chipaca: So perhaps neither? Where is the real sha3 algo living?
[18:08] <Chipaca> niemeyer: which is the 'real sha3'
[18:08] <niemeyer> Chipaca: The thing that computes the sha3 digest given bytes
[18:08]  * niemeyer digs in
[18:09] <Chipaca> niemeyer: osutil.FileDigest takes a generic crypto.Hash
[18:09] <niemeyer> Chipaca: That's good.. we can cook a crypto.Hash which is actually multiple hashes easily, I suppose
[18:10] <niemeyer> Chipaca: So the answer to my question above is golang.org/x/crypto/sha3
[18:11] <Chipaca> fwiw I think I need a thing that takes an io.Reader, not bytes
[18:11] <Chipaca> but that's minor
[18:11] <Chipaca> (otoh it's easy to multiplex readers to do multiple hashes)
[18:12] <niemeyer> Chipaca: I suggest leaving asserts alone for now and just doing the low-level thing inside osutil, leveraging crypto/sha3 directly
[18:12] <niemeyer> Chipaca: The cache can keep bytes too
[18:13] <niemeyer> Chipaca: DeriveSideInfo can return the actual bytes too, unencoded
[18:14] <mup> PR snapd#4843 opened: interfaces/builtin: let MM change qmi device attributes <Created by alfonsosanchezbeato> <https://github.com/snapcore/snapd/pull/4843>
[18:14] <niemeyer> Chipaca: FWIW, if we find a need to reuse EncodeDigest later, it should probably live inside strutil
[18:14] <Chipaca> niemeyer: I _think_ i understood what you meant :-)
[18:15] <Chipaca> niemeyer: basically make SnapFileSHA3_384 call osutil.GetPathInfo instead of osutil.FileDigest to get the digest
[18:15] <Chipaca> i think that'll work, because i think everything uses that to get the hash
[18:16] <Chipaca> i'll double check and report back if it's otherwise
[18:16] <niemeyer> Chipaca: I don't think that will work
[18:16] <Chipaca> niemeyer: then i didn't understand what you meant
[18:16] <niemeyer> Chipaca: Well, or maybe it will, if we reuse the StateCache type there as well
[18:17] <Chipaca> ah, because it doesn't have the state :-/
[18:17] <niemeyer> Chipaca: The original suggestion was to *return* the digest, but I guess your solution is cleaner actually
[18:17] <Chipaca> heh
[18:17] <Chipaca> niemeyer: the problem is what is 'the digest'
[18:17] <Chipaca> all these functions return 'the digest'
[18:17] <niemeyer> Chipaca: Right.. but we can pass in the state as a StateCache interface as well
[18:17] <Chipaca> it's just … different digests
[18:17] <Chipaca> :-)
[18:18] <niemeyer> Chipaca: DeriveFooBar doesn't return any digests today
[18:18] <Chipaca> niemeyer: i never mentioned DeriveFooBar
[18:18] <Chipaca> ;-)
[18:18] <niemeyer> Chipaca: I did, both in the call and above
[18:18] <niemeyer> "Chipaca: DeriveSideInfo can return the actual bytes too, unencoded"
[18:19] <niemeyer> Chipaca: This is the actual function that performs the digest inside firstboot
[18:20] <Chipaca> niemeyer: yes, but i'm not sure what the bytes would be useful for
[18:20] <niemeyer> Chipaca: To cache them
[18:20] <niemeyer> Chipaca: But... you're right
[18:21] <Chipaca> i hate being right before understanding the thing
[18:21] <Chipaca> :-)
[18:21] <Chipaca> probably a bit late in the day for me
[18:21] <niemeyer> Chipaca: It's cleaner to pass the StateCache in as an interface, and call GetPathInfo inside it
[18:22] <niemeyer> Chipaca: Perhaps osutil.ComputePathInfo instead
[18:22] <niemeyer> Chipaca: So callers realize this is not a trivial op
[18:22] <Chipaca> niemeyer: HoldOnToYourTrowsersWereGoingToGetPathInfo
[18:22] <Chipaca> :-p
[18:22] <niemeyer> Even better :P
[18:23] <Chipaca> yeah the name was a this-thing-needs-a-name token one, not the actual thing
[18:23] <Chipaca> ComputePathInfo it is
[18:24] <Chipaca> and, eod for a while (probably be back later to play with help strings)
[18:25] <niemeyer> Chipaca: Enjoy
[20:04] <mup> PR snapd#4837 closed: many: go vet cleanups <Created by bboozzoo> <Merged by sergiocazzolato> <https://github.com/snapcore/snapd/pull/4837>
[20:15] <cachio> niemeyer, could you please take a look to this one? https://github.com/snapcore/spread/pull/52
[20:15] <mup> PR spread#52: Add storage option for google <Created by sergiocazzolato> <https://github.com/snapcore/spread/pull/52>
[20:17] <cachio> it is making fail the ubuntu-16.04-32 on google
[21:02] <Chipaca> niemeyer: ping
[21:21] <mup> Issue snapcraft#1659 closed: Implement support for supported bases <Created by sergiusens> <Closed by sergiusens> <https://github.com/snapcore/snapcraft/issue/1659>
[21:21] <mup> PR snapcraft#1993 closed: core: initial support for bases <Created by sergiusens> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/1993>
[21:23] <niemeyer> Chipaca: Hyea
[21:23] <Chipaca> niemeyer: noodling around with this stuff, it looks like I should give PathInfos a Rename
[21:23] <Chipaca> niemeyer: at which point I'm not sure PathInfo is the right name for the beast
[21:24]  * Chipaca goes back to noodlin' and lets niemeyer worry about names
[21:24] <niemeyer> Chipaca: I'm not sure about what other name might be appropriate though.. we're effectively associating a bag of knowledge with a particular path
[21:27] <Chipaca> niemeyer: HashedFile?
[21:27] <Chipaca> eh, probably not or it'll grow Open() and stuff
[21:28] <Chipaca> and I guess Rename is actually a method of the hash cache, not the path info
[21:28] <Chipaca> hrmph
[21:29] <Chipaca> (the rename thing is because in quite a few places (at least two so far :-) ) we hash just before moving the file_
[21:29] <Chipaca> the download with deltas path is really interesting
[21:29] <Chipaca> apply delta hashes the composed file to make sure it's ok
[21:30] <Chipaca> and if so it's treated like a complete partial
[21:30] <Chipaca> so it's hashed to make sure it's ok
[21:30] <mup> PR snapcraft#2003 opened: patches: make the ctypes patch more robust and add armhf arch triplet <Created by sergiusens> <https://github.com/snapcore/snapcraft/pull/2003>
[21:31] <Chipaca> and that's in store, before hashing it to make sure it's ok
[21:31] <Chipaca> :-)
[21:40]  * cachio afk
[21:55] <niemeyer> Chipaca: Renaming the path info doesn't sound too bad
[21:55] <Chipaca> niemeyer: yeah, i was originally thinking of having it do the actual rename
[21:55] <Chipaca> but it'll just rename the cache and be happy
[21:55] <niemeyer> Chipaca: The association is still with the path, and it's not just a hash.. we want mtime, size, etc, as well
[21:55] <niemeyer> Chipaca: Ah, agreed.. it should act just on the cache
[21:57] <mup> PR snapcraft#2004 opened: errros: add a specific error when running commnds from plugins <bug> <Created by sergiusens> <https://github.com/snapcore/snapcraft/pull/2004>
[23:23] <cachio> niemeyer, hey
[23:23] <niemeyer> cachio: Heya
[23:24] <cachio> niemeyer, when you have 5 minutes, could you please take a look to https://github.com/snapcore/spread/pull/52 ?
[23:24] <mup> PR spread#52: Add storage option for google <Created by sergiocazzolato> <https://github.com/snapcore/spread/pull/52>
[23:24] <cachio> niemeyer, I have 1 PR waiting for this change
[23:25] <niemeyer> ACk
[23:25] <cachio> niemeyer, tx
[23:28] <cachio> niemeyer, something else
[23:28] <cachio> niemeyer, I see https://paste.ubuntu.com/p/2Wd5dQX6WS/ after a reboot in debian 9 in google
[23:29] <cachio> it is like spread tries to connect and for some reason pam is closing the session for the root
[23:29] <cachio> the sshd config is the same we have in linode
[23:29] <cachio> did you see that before?
[23:44] <mwhudson> um
[23:44] <mwhudson> subiquity is failing to start in a live cd with "cannot create user data directory"
[23:44] <mwhudson> what can possibly have changed since this worked yesterday
[23:49]  * cachio eod