[05:15] <mborzecki> morning
[06:50] <zyga> Good morning
[06:50] <zyga> mborzecki: did you see my second pass review
[06:50] <mborzecki> zyga: hey
[06:51] <mborzecki> zyga: yes, i pushed an update with comments
[06:51] <zyga> Thanks, I’ll look soon
[06:56] <zyga> Lovely day
[07:08] <pstolowski> morning
[07:09] <pstolowski> hey zyga, i'm planning to review your implicit plugs/slots stuff today
[07:09] <mborzecki> pstolowski: hey
[07:09] <zyga> pstolowski: thank you
[07:10] <zyga> pedronis: changed it as well, it looks very good imo
[07:10] <zyga> Look at patch history please
[07:11] <pstolowski> ack
[07:44] <zyga> https://github.com/snapcore/snapd/pull/6740 is short and could help me move snap-update-ns forward
[07:47] <zyga> pedronis: I noticed that defer is blocked, is there something specific you want me to focus on? I would love to be able to move it forward next week while you are away
[07:50] <pstolowski> zyga: he is off from today
[07:50] <zyga> eh
[07:50] <zyga> not great
[07:50] <zyga> ok
[08:41] <mborzecki> pstolowski: can you take a look at https://github.com/snapcore/snapd/pull/6748 ?
[08:43] <pstolowski> yep
[08:49] <zyga> pstolowski: hey, can you please look at https://github.com/snapcore/snapd/pull/6740 next
[08:52] <zyga> pstolowski: and please tell us what to review
[08:52] <zyga> I think that's all of us today
[08:55] <zyga> pstolowski: in https://github.com/snapcore/snapd/pull/6755 could you add a Fixes: ... line that references the bug number?
[08:55] <pstolowski> zyga: it's a private bug
[08:55] <zyga> that's fine
[08:55] <zyga> it's only private for now
[08:55] <pstolowski> ok
[08:55] <zyga> it will become public once the security aspect expires
[08:55] <pstolowski> zyga: shall i add it in the code, or PR description only?
[08:56] <zyga> pstolowski: in the PR commit message please
[08:56] <pstolowski> k
[08:56] <zyga>  thanks!
[12:47] <zyga> jdstrand: hey, is https://github.com/snapcore/snapd/pull/6681 ready for review?
[13:49] <jdstrand> zyga (cc pedronis and mvo): yes, I made some adjustments to the PR for the investigation yesterday. what remains is a) trying to figure out why spread is failing in:
[13:49] <jdstrand> 2019-04-19 13:02:27 Error executing google:ubuntu-16.04-32:tests/main/static :
[13:49] <jdstrand> -----
[13:49] <jdstrand> + CGO_ENABLED=0
[13:49] <jdstrand> + go build -o snapd.static github.com/snapcore/snapd/cmd/snapd
[13:49] <jdstrand> # github.com/snapcore/snapd/overlord/snapstate
[13:49] <jdstrand> ../../../overlord/snapstate/check_snap.go:433:16: undefined: osutil.FindUid
[13:49] <jdstrand> ../../../overlord/snapstate/check_snap.go:434:16: undefined: osutil.FindGid
[13:49] <jdstrand> b) comment updates from pedronis
[13:49] <zyga> jdstrand: perhaps those are failing because FindUid requires cgo?
[13:49] <zyga> jdstrand: FYI, I'm off on Monday
[13:50] <jdstrand> c) more spread tests to verify functionality of the bpf
[13:50] <zyga> jdstrand: if you have a moment please review 6714
[13:50] <zyga> jdstrand: did you get to the bottom of the getuid issue?
[13:50] <zyga> is it a bug in libseccomp or a glibc wrapper confusing the actual syscall?
[13:50] <zyga> jdstrand: perdonis is off today and next week
[13:51] <jdstrand> zyga: please see the comment in the PR (I put it all in there)
[13:51] <jdstrand> zy
[13:51] <zyga> jdstrand: percfect, I surely will :)
[13:51] <jdstrand> zyga: so, FindUid is not new and this only started failing within the last couple days
[13:51] <zyga> hmm
[13:52] <zyga> odd, perhaps it was not build with CGO_ENABLED=0
[13:52] <jdstrand> I don't know...
[13:52] <zyga> but I didn't look, my comment was just a hunch
[13:52] <jdstrand> this is the function that snap-seccomp uses to resolve u:root
[13:52] <zyga> jdstrand: snap-seccomp is built with cgo
[13:52] <zyga> jdstrand: snapd is not
[13:52] <zyga> jdstrand: you cannot use this function from snapd
[13:53] <jdstrand> zyga: I literally have to
[13:53] <zyga> we cannot :/
[13:53] <zyga> we need a helper to resolve it then
[13:53] <zyga> a process you can call to give you the value
[13:53]  * jdstrand sighs
[13:53]  * zyga is happy at least that part is not mysterious anymore
[13:53]  * zyga hugs jdstrand 
[13:53] <zyga> sorry, I know this is painful
[13:54] <jdstrand> I can call out to getent I guess
[13:54] <zyga> probably yeah
[13:54] <jdstrand> or parse /etc/passwd and /etc/group myself
[13:54] <zyga> jdstrand: that doesn't work
[13:55] <zyga> jdstrand: you can use go functions for that anyway
[13:55] <zyga> jdstrand: the real problem is nsswitch.conf
[13:55] <zyga> and ldap like systems
[13:55] <jdstrand> the go functions are what are doing this
[13:55] <zyga> jdstrand: that's why you need to use cgo to use that
[13:55] <zyga> jdstrand: there's a line where pure go can do simple things
[13:55] <zyga> like read passwd files
[13:55] <zyga> and cgo can call the glibc functions that do everything
[13:55] <jdstrand> zyga: I know that. though for this particular thing, it should really be on the system
[13:55] <zyga> jdstrand: we chose not to cross that line in snapd
[13:56] <zyga> jdstrand: then perhaps you just need to use the simple functions and not the osutil ones?
[13:57] <jdstrand> well, jeez
[13:58] <jdstrand> the check is to make sure the user/group is available and bail early (an implied 'assumes')
[13:58] <jdstrand> but it should really behave exactly like what snap-seccomp is doing
[13:59] <jdstrand> snap-seccomp could move to parsing /etc/passwd and /etc/group
[13:59] <zyga> jdstrand: (silly Friday idea): call snap-seccomp with setgid g:daemon and parse the word out of the filter ;)
[13:59] <jdstrand> but that is a bigger change than I want to make
[13:59]  * jdstrand goes with getent
[13:59] <jdstrand> heh
[14:00] <zyga> jdstrand: I think there are builtin go functions that give you that answer
[14:00] <jdstrand> I think getent will be much cheaper :)
[14:00] <zyga> *think*
[14:00] <zyga> not sure
[14:00] <zyga> you don't need to shell out to getent
[14:00] <jdstrand> I just googling-- people seem to have packages for it but not a part of the language
[14:00]  * zyga looks
[14:01] <jdstrand> hmm, actually, this starting failing when I added the implied assumes most likely. that makes more sense
[14:01] <zyga> jdstrand: os/user.LookupGroup
[14:01] <jdstrand> zyga: yes, that is what we are doing
[14:01] <jdstrand> well
[14:01] <zyga> then get group.GID if it didn't fail
[14:01] <jdstrand> we are using LookupUser
[14:01] <zyga> hold on? are we?
[14:01] <jdstrand> yes
[14:02] <zyga> wait, and that doesn't compile?
[14:02]  * zyga is sceptical now
[14:02] <jdstrand> os.FindUid uses LookupUser
[14:02] <zyga> can you do a tiny program that calls LookupGroup?
[14:02] <zyga> yes
[14:02] <zyga> LookupUser is old
[14:02] <jdstrand> LookupUser has always existed in golang
[14:02] <zyga> LookupGroup is new
[14:02] <zyga> *new enough for us though
[14:02] <jdstrand> the test is failing on FindUid
[14:02] <jdstrand> yes, we stole LookupGroup from newer golang
[14:03] <jdstrand> and could remove that code
[14:03] <zyga> FindUid is different
[14:03] <jdstrand> I was actually planning on doing that in another PR since we now require 1.10
[14:03] <zyga> I mean
[14:03] <roadmr> hey folks, what happens if I have a snap named "foo" and then I install a "bar" snap that has an autoalias bar.foo -> foo  ?
[14:03] <jdstrand> but why is FindUid failing?
[14:03] <zyga> jdstrand: we can probably drop lookupGroup
[14:03] <zyga> jdstrand: and just use the plain versions
[14:03] <jdstrand> zyga: yes, I just said that :)
[14:03] <zyga> jdstrand: and that will get you out of the issue
[14:04] <zyga> jdstrand: that is probably the only thing that requires C
[14:04] <zyga> jdstrand: cool, ping me and mborzecki for review please
[14:04] <jdstrand> it will get me out of the issue for FindGid, but not FindUid, no?
[14:04] <zyga> jdstrand: no, why?
[14:04] <zyga> jdstrand: it's just the "import C" that breaks you
[14:04] <zyga> drop that from osutil and you are good
[14:05] <jdstrand> because of what I jus wrote?
[14:05] <jdstrand> findUid does user.Lookup
[14:05] <mborzecki> hm had a branch somewhere that drops all C dependant bits from osutil
[14:05] <zyga> jdstrand: I don't get it then, sorry, can you reprhase?
[14:05] <zyga> jdstrand: calling user.Lookup is not the problem
[14:05] <zyga> jdstrand: osutil *does not build*
[14:05] <jdstrand> the spread test fail because of FindGid *and* FindUid
[14:05] <zyga> jdstrand: when you disable C
[14:05] <zyga> at least not that part :)
[14:05] <jdstrand> ok, not that makes more sense
[14:06] <jdstrand> now*
[14:06] <jdstrand> nothing else used group.gp presumably
[14:06] <mborzecki> jdstrand: https://github.com/bboozzoo/snapd/commit/66e761e11bdb4f29ba279113b20c55d000035fda ?
[14:06] <jdstrand> let me do a quick PR for that
[14:07] <jdstrand> mborzecki: so are you saying that FindUid also needs cgo even though it is using user.Lookup?
[14:08] <zyga> jdstrand: it's one file
[14:09] <zyga> the whole file gets removed from the build process
[14:09] <zyga> that's all
[14:09] <jdstrand> zyga: right, I understand your point
[14:09] <jdstrand> that makes sense
[14:09] <jdstrand> but then mborzecki pointed me at https://github.com/bboozzoo/snapd/commit/66e761e11bdb4f29ba279113b20c55d000035fda which stubs FindUid
[14:09] <jdstrand> when it seems it may only need to stub FindGid
[14:10] <jdstrand> so I wanted a clarification on that
[14:10] <jdstrand> since if FindUid also needs cgo, then the LookupGroup fix is valid on its own, but doesn't help me
[14:11]  * zyga has a new flower pot :)
[14:11] <jdstrand> I mean, I can just try this out I guess
[14:11] <zyga> :D
[14:12] <jdstrand> zyga: anyway, so there a few things to tie up with my PR and I'll be adding more tests, but it is ready for review. you could wait til Tuesday I guess (all my bits should be done then)
[14:12] <jdstrand> and yes, I plan to look at PRs today
[14:12] <zyga> jdstrand: cool,
[14:13] <zyga> jdstrand: I have a few piles of changes for snap-confine pending but I don't want to throw this all at you
[14:13] <zyga> jdstrand: one thing that was bugging me a while ago was stdlib's IO functions that we cannot use because they apparently suck on error reporting
[14:13] <zyga> jdstrand: so I wrote my own proof of concept with tests to show what happens in all error cases
[14:14] <mborzecki> jdstrand: if you want to have cgo enabled, but use non-cgo implementation of os/user bits you'd have to pass -tags osuser (and wit that patch i linked -tags osutil too)
[14:14]  * zyga is starving
[14:14] <zyga> time to eat
[14:14] <jdstrand> mborzecki: that is interesting, but I think a different answer to my question
[14:14] <jdstrand> mborzecki: I am going to do a PR to use user.LookupGroup
[14:15] <jdstrand> mborzecki: and remove all the cgo stuff in osutil
[14:15] <jdstrand> mborzecki: I thought that would fix stuff, but then your commit stubbed osutil.FindUid, that already used user.LookupUser, so I didn't know why you did that one since it was already using the bultin
[14:16] <mborzecki> jdstrand: that's tricky i think, we'll loose /etc/nss* and probably extrausers too
[14:16] <jdstrand> I see
[14:16] <jdstrand> so the builtins don't use nss at all
[14:17] <jdstrand> we need extra users
[14:17] <mborzecki> yup, let me point you to the source code
[14:17] <jdstrand> ok, I think instead of using builtins, I can convert FindUid and FindGid to use getent
[14:18] <mborzecki> jdstrand: https://github.com/golang/go/tree/master/src/os/user look at cgo_lookup_unix.go and lookup_unix.go
[14:18] <jdstrand> actually, for this PR, I will probably use LookupGroup. then add a code comment that when we need extrausers, we will need to use getent
[14:19] <jdstrand> since we don't need extrausers for this now
[14:19] <jdstrand> but we will later
[14:20] <jdstrand> passwd:         compat extrausers
[14:20] <jdstrand> from nsswitch.conf
[14:20] <jdstrand> ok, this is all clear now
[14:20] <jdstrand> zyga, mborzecki: thanks
[14:22] <mborzecki> jdstrand: btw. that also makes calling the os/user bits from snap-exec snap-update-ns bit problematic, as those are built statically via -extldflags=-ldflags=-static, but will still attempt to dlopen the nss bits
[14:23] <jdstrand> mborzecki: I'm sorry, if I'm using only builtins, what is the problem?
[14:25] <jdstrand> mborzecki: nothing except snap-seccomp and my PR use FindUid and FindGid
[14:25] <mborzecki> jdstrand: because builtins only know about /etc/groups & /etc/passwd
[14:25] <jdstrand> mborzecki: I know. but snap-exec and snap-update-ns don't do lookups... ?
[14:26] <jdstrand> mborzecki: at least not via these functions. I feel like I am missing your point
[14:26] <mborzecki> jdstrand: they don't, i stumbled upon this problem when trying to do the mapping for $HOME/snap_<instance> to $HOME/snap with parallel installs
[14:27] <mborzecki> jdstrand: which we eventually dropped anyway
[14:32]  * zyga breaks for lunch 
[14:49] <jdstrand> zyga: fyi https://github.com/snapcore/snapd/pull/6759
[14:54] <zyga> Back in 10
[15:07] <zyga> back now
[15:08] <zyga> jdstrand: the error on https://travis-ci.org/snapcore/snapd/jobs/522179183 is puzzling!
[15:09] <zyga> jdstrand: as a nitpick, could you please expand the commit message, it's nicer to know _why_ we are doing that not what is being done
[15:11] <zyga> jdstrand: the error talks about build ID, I would suspect it is caused by lack of cgo now
[15:11] <zyga> jdstrand: and default build mode that don't inject build-id
[15:17] <Son_Goku> zyga, any idea from the snap side for this? https://github.com/livecd-tools/livecd-tools/issues/125
[15:18] <zyga> looking
[15:18] <zyga> hmm, no idea if we are using livecd-tools anymore
[15:18] <zyga> it's a distro question
[15:18] <zyga> for snapd side I can say this
[15:18] <zyga> you can put seed data on a filesystem
[15:19] <zyga> then on 1st boot snapd spots the seed and installs the snaps listed there
[15:19] <Son_Goku> I don't know anything about those things
[15:19] <zyga> this is done for ubuntu live CD for instance
[15:19] <Son_Goku> so it'd be handy if you could detail that for this
[15:19] <zyga> but I don't know anything about livecd-tools
[15:19] <zyga> I don't know much about this honestly, this is really mvo/chipaca area
[15:20] <zyga> I can try to ask them after easter
[15:20] <Son_Goku> livecd-tools just basically uses a kickstart file as input to create live media
[15:20] <Son_Goku> it's how Fedora and other derivatives of Fedora make media
[15:20] <zyga> *basically* is what most people would not use :)
[15:20] <zyga> I don't know how distros do that today
[15:20]  * zyga (I mean the word basically assumes you know this)
[15:21] <Son_Goku> I use basically as "simply speaking"
[15:21] <Son_Goku> sorry
[15:21] <zyga> no worries :)
[15:22] <zyga> I'm looking for seeding docs
[15:23] <zyga> Son_Goku: I answered in the github issue now
[15:25] <Son_Goku> thanks
[15:35] <zyga> jdstrand: I commented on https://github.com/snapcore/snapd/pull/6759
[15:36] <zyga> I'm back to hacking on groups
[15:36] <zyga> I solved my conundrum
[16:54]  * zyga begs for review on https://github.com/snapcore/snapd/pull/6714
[17:45] <zyga> jdstrand: ping
[17:48] <zyga> jdstrand: my last patch of the day
[17:49] <zyga> jdstrand: in case you are around I wanted to explain some of the things since you will be probably the only one that _has to_ review it
[17:52] <zyga> jdstrand: https://github.com/snapcore/snapd/pull/6760
[17:53] <zyga> jdstrand: I'm EODing and EOWing now, please if you have some time left today have a look at  https://github.com/snapcore/snapd/pull/6714 which is the follow-up from the /tmp security issue
[17:53]  * zyga waves
[19:29] <sergiusens> zyga: hey, if you are around, mind giving some insight on https://github.com/ChristianPauly/fluffychat/issues/117#issuecomment-484959454 ... I suspect this is a host lacking many services, but have not dug into it and you might know more about it
[21:00] <zyga> sergiusens: someone has staged real xdg-open and it took priority
[21:00] <zyga> sergiusens: our xdg-open doesn't use dbus-launch and does not print that output
[21:15] <zyga> jdstrand: thank you for both reviews
[21:16]  * zyga started another print and goes to sleep