| === chihchun_afk is now known as chihchun | ||
| mborzecki | morning | 05:15 |
|---|---|---|
| zyga | Good morning | 06:50 |
| zyga | mborzecki: did you see my second pass review | 06:50 |
| mborzecki | zyga: hey | 06:50 |
| mborzecki | zyga: yes, i pushed an update with comments | 06:51 |
| zyga | Thanks, I’ll look soon | 06:51 |
| zyga | Lovely day | 06:56 |
| === pstolowski|afk is now known as pstolowski | ||
| pstolowski | morning | 07:08 |
| 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:09 |
| zyga | pedronis: changed it as well, it looks very good imo | 07:10 |
| zyga | Look at patch history please | 07:10 |
| pstolowski | ack | 07:11 |
| zyga | https://github.com/snapcore/snapd/pull/6740 is short and could help me move snap-update-ns forward | 07:44 |
| 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:47 |
| pstolowski | zyga: he is off from today | 07:50 |
| zyga | eh | 07:50 |
| zyga | not great | 07:50 |
| zyga | ok | 07:50 |
| mborzecki | pstolowski: can you take a look at https://github.com/snapcore/snapd/pull/6748 ? | 08:41 |
| pstolowski | yep | 08:43 |
| zyga | pstolowski: hey, can you please look at https://github.com/snapcore/snapd/pull/6740 next | 08:49 |
| zyga | pstolowski: and please tell us what to review | 08:52 |
| zyga | I think that's all of us today | 08:52 |
| 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:55 |
| zyga | pstolowski: in the PR commit message please | 08:56 |
| pstolowski | k | 08:56 |
| zyga | thanks! | 08:56 |
| === chihchun is now known as chihchun_afk | ||
| zyga | jdstrand: hey, is https://github.com/snapcore/snapd/pull/6681 ready for review? | 12:47 |
| === chihchun_afk is now known as chihchun | ||
| 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:49 |
| 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:50 |
| 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:51 |
| 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:52 |
| 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:53 |
| 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:54 |
| 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:55 |
| zyga | jdstrand: then perhaps you just need to use the simple functions and not the osutil ones? | 13:56 |
| jdstrand | well, jeez | 13:57 |
| 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:58 |
| 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 | 13:59 |
| 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:00 | |
| 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:01 |
| 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:02 |
| 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:03 |
| 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:04 |
| 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:05 |
| 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:06 |
| jdstrand | mborzecki: so are you saying that FindUid also needs cgo even though it is using user.Lookup? | 14:07 |
| zyga | jdstrand: it's one file | 14:08 |
| 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:09 |
| 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:10 |
| * zyga has a new flower pot :) | 14:11 | |
| jdstrand | I mean, I can just try this out I guess | 14:11 |
| zyga | :D | 14:11 |
| 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:12 |
| 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:13 |
| 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:14 |
| 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:15 |
| 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:16 |
| 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:17 |
| 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:18 |
| jdstrand | since we don't need extrausers for this now | 14:19 |
| jdstrand | but we will later | 14:19 |
| 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:20 |
| 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:22 |
| jdstrand | mborzecki: I'm sorry, if I'm using only builtins, what is the problem? | 14:23 |
| 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:25 |
| 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:26 |
| mborzecki | jdstrand: which we eventually dropped anyway | 14:27 |
| * zyga breaks for lunch | 14:32 | |
| jdstrand | zyga: fyi https://github.com/snapcore/snapd/pull/6759 | 14:49 |
| zyga | Back in 10 | 14:54 |
| zyga | back now | 15:07 |
| zyga | jdstrand: the error on https://travis-ci.org/snapcore/snapd/jobs/522179183 is puzzling! | 15:08 |
| 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:09 |
| 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:11 |
| Son_Goku | zyga, any idea from the snap side for this? https://github.com/livecd-tools/livecd-tools/issues/125 | 15:17 |
| 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:18 |
| 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:19 |
| 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:20 | |
| Son_Goku | I use basically as "simply speaking" | 15:21 |
| Son_Goku | sorry | 15:21 |
| zyga | no worries :) | 15:21 |
| zyga | I'm looking for seeding docs | 15:22 |
| zyga | Son_Goku: I answered in the github issue now | 15:23 |
| Son_Goku | thanks | 15:25 |
| zyga | jdstrand: I commented on https://github.com/snapcore/snapd/pull/6759 | 15:35 |
| zyga | I'm back to hacking on groups | 15:36 |
| zyga | I solved my conundrum | 15:36 |
| === pstolowski is now known as pstolowski|afk | ||
| * zyga begs for review on https://github.com/snapcore/snapd/pull/6714 | 16:54 | |
| zyga | jdstrand: ping | 17:45 |
| zyga | jdstrand: my last patch of the day | 17:48 |
| 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:49 |
| zyga | jdstrand: https://github.com/snapcore/snapd/pull/6760 | 17:52 |
| 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 | 17:53 | |
| 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 | 19:29 |
| === chihchun is now known as chihchun_afk | ||
| === chihchun_afk is now known as chihchun | ||
| 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:00 |
| zyga | jdstrand: thank you for both reviews | 21:15 |
| * zyga started another print and goes to sleep | 21:16 | |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!