| mup | PR snapcraft#2158 opened: rust plugin: fix cargo builds and run tests <Created by tismith> <https://github.com/snapcore/snapcraft/pull/2158> | 03:28 |
|---|---|---|
| mborzecki | morning | 05:05 |
| zyga | hey hey :) | 05:14 |
| mborzecki | zyga: hey | 05:15 |
| mup | PR snapd#5273 closed: testutil: add test support for Fstatfs <Simple> <Created by zyga> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/5273> | 05:29 |
| zyga | woot, thank you! | 05:30 |
| mborzecki | so are we doing the `snap list --format=..` thing or not? | 05:38 |
| zyga | I ... don't know | 05:40 |
| zyga | I think not | 05:40 |
| zyga | mborzecki: 5266 is simple and green | 05:42 |
| mvo | mborzecki: it looks like no, lets see if the bugreport is happy about the snap list|awk|tail +2 solution | 05:45 |
| mborzecki | it was a nice feature :( | 05:53 |
| mvo | mborzecki: yeah, I have grown to like it as well | 05:53 |
| mborzecki | there was a panic from spread in #5271 https://paste.ubuntu.com/p/XXqr7TY9KM/ | 05:58 |
| mup | PR #5271: [WIP] cmd: attempt to start the document portal if running with a session bus <Created by jhenstridge> <https://github.com/snapcore/snapd/pull/5271> | 05:58 |
| mborzecki | niemeyer: ^^ | 05:58 |
| zyga | mborzecki: I think we saw a few panics yesterday and gustavo is aware of them | 06:15 |
| zyga | but I'm not sure if those are the same panics or if spread was changed but some panics remained | 06:15 |
| zyga | hey mvo, good morning | 06:15 |
| mup | PR snapd#5266 closed: interfaces/builtin/docker: use commonInterface over specific struct <Created by anonymouse64> <Merged by zyga> <https://github.com/snapcore/snapd/pull/5266> | 06:17 |
| zyga | https://github.com/snapcore/snapd/pull/5230 needs a 2nd review | 06:17 |
| mup | PR #5230: interfaces/udisks2: also implement implicit classic slot <Created by jdstrand> <https://github.com/snapcore/snapd/pull/5230> | 06:17 |
| mvo | zyga: good morning to you as well! | 06:18 |
| zyga | mvo: what's the state of https://github.com/snapcore/snapd/pull/5250 | 06:18 |
| mup | PR #5250: interfaces/udev,misc: only trigger udev events on input subsystem as needed <Created by jdstrand> <https://github.com/snapcore/snapd/pull/5250> | 06:18 |
| zyga | there's a conflict but I see that you took care of the feedback | 06:18 |
| zyga | are you working on this or is the conflict recent | 06:18 |
| zyga | er | 06:18 |
| zyga | sorry | 06:18 |
| zyga | bad PR | 06:18 |
| zyga | https://github.com/snapcore/snapd/pull/5226 | 06:18 |
| mup | PR #5226: data: add systemd user environment generator <Created by mvo5> <https://github.com/snapcore/snapd/pull/5226> | 06:18 |
| zyga | this is what I meant | 06:18 |
| mvo | zyga: the user envirronment generator? iirc it had some packaging issues I need to look at them | 06:24 |
| zyga | ack, | 06:24 |
| zyga | I asked some follow up packaging questions there | 06:24 |
| mvo | ta | 06:24 |
| zyga | +1 on everything if it works :) | 06:24 |
| mvo | ta² | 06:24 |
| zyga | unicode :D | 06:24 |
| Lin-Buo-Ren-alt1 | https://forum.snapcraft.io/t/organize-another-dir-causes-the-items-under-host-root-directory-to-be-copied-in-another-dir/5806 | 06:26 |
| mup | PR snapd#5259 closed: devicestate: support seeding from a base snap instead of core <Blocked> <Created by mvo5> <Closed by mvo5> <https://github.com/snapcore/snapd/pull/5259> | 06:58 |
| === pstolowski|afk is now known as pstolowski | ||
| pstolowski | morning o/ | 07:04 |
| mborzecki | pstolowski: hey | 07:06 |
| mvo | hey pstolowski | 07:06 |
| zyga | hey pawel | 07:26 |
| * zyga -> walk | 07:45 | |
| zyga | or maybe in 15 minutes | 07:46 |
| mup | PR snapd#5275 opened: cmd/snap: use snaptest.MockSnapCurrent in `snap run` tests <Simple> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/5275> | 07:53 |
| mborzecki | trivial pr ^^ | 07:53 |
| mup | PR snapd#5276 opened: devicestate: support seeding from a base snap instead of core <Created by mvo5> <https://github.com/snapcore/snapd/pull/5276> | 08:08 |
| zyga | mborzecki: https://github.com/snapcore/snapd/pull/5277 :) | 08:18 |
| mup | PR #5277: cmd/snap-update-ns: add helper for checking for read-only filesystems <Created by zyga> <https://github.com/snapcore/snapd/pull/5277> | 08:18 |
| zyga | I'll take your PR for snap run | 08:18 |
| mup | PR snapd#5277 opened: cmd/snap-update-ns: add helper for checking for read-only filesystems <Created by zyga> <https://github.com/snapcore/snapd/pull/5277> | 08:18 |
| mborzecki | haha ;) trade PRs | 08:18 |
| zyga | spread still panics | 08:18 |
| zyga | can we revert spread somehow? | 08:18 |
| mborzecki | i'm looking into spread right now | 08:18 |
| zyga | +1 on the PR | 08:19 |
| zyga | and I need to go or my dog will hate me | 08:19 |
| mborzecki | zyga: it won't hate you, it'll just piss on the floor :P | 08:20 |
| zyga | and *then* he will hate me ;) | 08:20 |
| zyga | no, he's too humble for that I'm not that evil | 08:20 |
| * zyga -> walk | 08:20 | |
| mvo | the spread crash is not happening all the time, right? I saw at least one green PR from me this morning | 08:21 |
| mvo | mborzecki: can I restart your failed MockSnapCurrent PR? or do you want to keep it for the error ? | 08:22 |
| mborzecki | mvo: go ahead | 08:22 |
| pedronis | mvo: some comments on #5274 | 08:26 |
| mup | PR #5274: configstate: deny configuration of base snaps and for the "snapd" snap <Created by mvo5> <https://github.com/snapcore/snapd/pull/5274> | 08:26 |
| mborzecki | niemeyer: https://github.com/snapcore/spread/pull/59 | 08:28 |
| mup | PR spread#59: spread: do not panic if error message from google backend is empty <Created by bboozzoo> <https://github.com/snapcore/spread/pull/59> | 08:28 |
| mvo | pedronis: great, thank you! | 08:28 |
| pedronis | mvo: I don't understand the changer about errDoNothing in 5276 | 08:35 |
| pedronis | mvo: I probably read the new code wrong but it's a bit unclear what's the intention of it | 08:40 |
| mvo | pedronis: let me double check | 08:41 |
| pedronis | just avoid calling trivialSeeding twice? | 08:41 |
| pedronis | I mean from two places | 08:41 |
| pedronis | but then the comment in import Assertion is misleading | 08:42 |
| mvo | pedronis: thanks, let me rework this | 08:43 |
| pedronis | mvo: I mean we will not use the model returned by importAssertionsFromSeed in that case | 08:45 |
| pedronis | also it doesn't get set in state anyway so it wouldn't help later code that needs a model in all cases | 08:48 |
| pachulo | Hi! Does anybody knows something the vim snap? https://forum.snapcraft.io/t/vim-snap/5573 | 08:48 |
| mvo | pedronis: yeah, I think this was a misconception my part, these bits can be undone | 08:53 |
| pedronis | mvo: I suppose maybe because at some point you were passing model in trivialSeeding ? | 08:53 |
| mvo | pedronis: exactly | 08:53 |
| mvo | pedronis: using "core" for the config makes all of this much simpler | 08:54 |
| pedronis | I see, anyway as I said return a model that way from importAssertionsFromSeed breaks its invariant so we would needed to do something else anyway | 08:54 |
| * pedronis errands | 08:54 | |
| mvo | pedronis: there will be some small complications in the followups because some code checks for that the snap is installed when getting config | 08:54 |
| mvo | pedronis: thank you, I will update the PRs | 08:55 |
| zyga | re | 09:06 |
| zyga | pachulo: hey | 09:06 |
| zyga | pachulo: since snap crafters are the publisher I would suggest asking popey about it | 09:07 |
| zyga | mborzecki: my PR is green :) | 09:16 |
| zyga | no better chance to review it than now :) | 09:16 |
| zyga | oh I see it's reviewed already | 09:16 |
| * zyga will forever ponder what is the condition under GitHub updates the page without a reload | 09:17 | |
| zyga | mvo: ^ quick 2nd review on 5277 | 09:17 |
| mborzecki | heh, github ui is terrible | 09:17 |
| zyga | I _like_ the way it looks but _hate_ the way it is stale without apparent reason | 09:17 |
| mborzecki | the funniest is when you comment on a hunk that gets changed in another patch, and you forgot to switch to 'show all patches' | 09:17 |
| zyga | thank you for the review on 5272 pawel | 09:19 |
| pachulo | thanks zyga ! | 09:23 |
| zyga | pachulo: note that you also got a response on the foruom | 09:23 |
| zyga | *forum | 09:23 |
| zyga | jamie there is right that there should be a repository under snapcrafters on github | 09:24 |
| zyga | mvo: thank you mvo | 09:26 |
| zyga | mborzecki: to the point about GitHub UX being not too great, I got a comment (approval) from mvo and it was shown as a comment but the comment / approval count below was stale | 09:26 |
| mup | PR snapd#5277 closed: cmd/snap-update-ns: add helper for checking for read-only filesystems <Created by zyga> <Merged by zyga> <https://github.com/snapcore/snapd/pull/5277> | 09:28 |
| mvo | pedronis: thanks again for the review(s), I update the PRs, I added a testcase that loads a snap-setup without a type. interesstingly this does not crash, it only crashes if there is an empty (or invalid) "type":"" in the json | 09:31 |
| mvo | pedronis: but maybe you have something different in midn? | 09:31 |
| mvo | mind | 09:31 |
| mup | PR snapd#5275 closed: cmd/snap: use snaptest.MockSnapCurrent in `snap run` tests <Simple> <Created by bboozzoo> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/5275> | 09:48 |
| popey | pachulo: wassup? | 10:16 |
| zyga | popey: is the vim snap owned by snapcrafters/ | 10:17 |
| zyga | popey: and if so, where is the snapcraft.yaml | 10:17 |
| popey | sergio initially created the snap, sergiusens ^ | 10:17 |
| zyga | mborzecki: one more helper before the main meat of the validation logic | 10:20 |
| zyga | mborzecki: #5278 | 10:20 |
| mup | PR #5278: cmd/snap-update-ns: add IsTrustedTmpfs and tests <Created by zyga> <https://github.com/snapcore/snapd/pull/5278> | 10:20 |
| pedronis | mvo: ah see, no, nothing different, just forgot that behavior of the json umarshaller | 10:20 |
| mup | PR snapd#5278 opened: cmd/snap-update-ns: add IsTrustedTmpfs and tests <Created by zyga> <https://github.com/snapcore/snapd/pull/5278> | 10:20 |
| mvo | pedronis: thank you! I feel much better having a test for this :) | 10:24 |
| pedronis | mvo: I will re-review after lunch | 10:26 |
| Chipaca | Hi all. A notice and reminder that I'm off work today and tomorrow. | 10:26 |
| zyga | Chipaca: ack | 10:29 |
| mvo | pedronis: ta | 10:31 |
| mvo | Chipaca: enjoy! we miss you already | 10:31 |
| Chipaca | mvo: :-) | 10:32 |
| * zyga considers lunch for a change | 10:37 | |
| robert_ancell | Chipaca, after implementing the /v2/snaps?select=...&snaps=... API in snapd-glib, can you think of any reason to ever have a client use the /v2/snaps/[name] API? I'm thinking of deprecating those old methods. | 10:45 |
| robert_ancell | I guess we continue to use it with POST, but not GET | 10:46 |
| Chipaca | robert_ancell: I'd say if there is it's because of a bug | 10:49 |
| robert_ancell | Chipaca, so be clear, /v2/snaps/[name] is really redundant now? | 10:49 |
| Chipaca | robert_ancell: well, it's going to be faster but probably not an issue in practice | 10:50 |
| Chipaca | robert_ancell: but, yeah | 10:50 |
| Chipaca | and it might not even be measurably faster -- i'd have to try to measure | 10:51 |
| ondra | zyga ping | 10:58 |
| zyga | Go | 11:00 |
| robert_ancell | ah, there is a bit of a difference /v2/snaps?snaps=blah returns 200 and /v2/snaps/blah returns 404 | 11:02 |
| ondra | zyga do we treat kernel snap differently when it comes to interfaces used by hooks? | 11:03 |
| zyga | Not that I know of | 11:04 |
| zyga | Kernel should not have interfaces or hooks AFAIR | 11:04 |
| mborzecki | hmm snapshots separate elements using _ in snaphot file names, will need some extra care for parallel installs | 11:08 |
| ondra | zyga I think joc spotted my error here | 11:11 |
| ondra | zyga indeed it's a bit special case I have here, but let's see :) | 11:11 |
| zyga | What is the use case? | 11:11 |
| ondra | zyga pi3 kernel update :) | 11:12 |
| zyga | Oj | 11:12 |
| zyga | Tell me more please | 11:12 |
| ondra | zyga let me proof tested first :) | 11:12 |
| zyga | Ok | 11:12 |
| mvo | zyga: I updated 5263 based on your feedback (thanks for this btw). also thanks to mborzecki | 11:18 |
| zyga | Ack | 11:18 |
| zyga | I will check after lunch | 11:18 |
| sergiusens | zyga, popey: that was my first classic snap ever, we didn't even have a build service yet iirc | 11:24 |
| mborzecki | one huge renames patch coming up, 97 files changed, 875 insertions(+), 702 deletions(-) | 11:34 |
| zyga | What is that | 11:35 |
| zyga | Switching to under_score ;-)) | 11:36 |
| zyga | I’m almost done with lunch | 11:36 |
| zyga | Will be home soon | 11:36 |
| pedronis | mborzecki: why renaming the dir ones ? | 11:42 |
| pedronis | me is probably missing something | 11:43 |
| mborzecki | pedronis: because some paths end up using StoreName() instead of InstanceName(), hence *Dir() were changed to make me go though each place and update it accordingly | 11:44 |
| pedronis | mborzecki: why would a path use StoreName ? | 11:45 |
| pedronis | genuine question | 11:45 |
| mborzecki | pedronis: eg. inside snap-exec after remount, apparmor profiles which refer to the paths inside the mount ns | 11:46 |
| pedronis | mborzecki: but there is no store variant of them, you then do snap.MountDir(snapName, rev) and similar ? | 11:47 |
| mborzecki | pedronis: yes | 11:48 |
| mborzecki | and the Instance*Dir() use the same snap.*Dir() helpers | 11:48 |
| pedronis | mborzecki: I suppose we could mount files at some point, not sure the it easy though | 11:48 |
| pedronis | sorry | 11:48 |
| pedronis | I mean share mount files | 11:48 |
| mborzecki | it'd be nice | 11:49 |
| mborzecki | right now each instance will get own copy | 11:49 |
| pedronis | that's an easier starting point | 11:49 |
| pedronis | there are fun questions also about conflicts | 11:49 |
| mborzecki | given that they can be on different revisions that's also a bit easier to handle | 11:50 |
| pedronis | mborzecki: anyway afaiu snap-confine and snap-exec are the only things that need to deal with dirs without instance-key, right? | 11:50 |
| mborzecki | pedronis: and apparmor backend so far | 11:51 |
| pedronis | mborzecki: I see an StoreName indeed, why ? | 11:52 |
| pedronis | to repelace SNAP_NAME, but unclear SNAP_NAME can still be used in profiles? | 11:53 |
| pedronis | ah, it's because inside vs outside? | 11:54 |
| pedronis | mborzecki: ok, I see, we could really sort of keep the old names, but maybe the new names are clearer | 11:57 |
| pedronis | because of this inside vs outside issue | 11:58 |
| mborzecki | pedronis: yup | 11:58 |
| mborzecki | pedronis: we can roll back the *Dir() renames later on when the store vs instance name thing is sorted out | 11:58 |
| pedronis | mborzecki: anyway it's not tragically big, just a bit, anyway will need a span of quite time to go over it | 12:01 |
| pedronis | mborzecki: did you see test that needed doubling, or did you still not look into that? | 12:03 |
| mborzecki | pedronis: i doubled only few tests, mostly in snap | 12:07 |
| pedronis | mborzecki: most places that need StoreName probably need one, places using InstanceName it depends I suppose | 12:09 |
| zyga | re | 12:10 |
| === pstolowski is now known as pstolowski|lunch | ||
| mvo | zyga: what comment should I add to 5263 | 12:27 |
| zyga | that there's no locking done by specific functions | 12:27 |
| zyga | (so that we don't forget this) | 12:27 |
| mvo | zyga: aha, ok | 12:29 |
| zyga | hmm | 12:29 |
| zyga | spread keeps crashing | 12:29 |
| mvo | zyga: will do | 12:29 |
| mvo | yeah, spread is not happy today | 12:29 |
| zyga | which is odd because some PRs are just ireland-grass-green | 12:30 |
| zyga | while others fail after 1st minute | 12:30 |
| jdstrand | mborzecki: I haven't really been following the conversation (I was conducting and interview-- note to channel: there are open positions on the Ubuntu Security team!) | 12:31 |
| jdstrand | mborzecki: but otoh the things to think about are file paths (both where snaps live, things are mounted, but also apparmor, seccomp, udev, dbus policy files) | 12:32 |
| jdstrand | mborzecki: then the security label ('profile foo' in apparmor policy) and the udev tag (which annoyingly must use underscores) | 12:33 |
| mborzecki | jdstrand: do the underscores have any special meaning there? | 12:34 |
| jdstrand | mborzecki: yes! :) they are the delimiter instead of '.'. udev tags can't have periods | 12:34 |
| jdstrand | mborzecki: eg: SUBSYSTEM=="drm", KERNEL=="card[0-9]*", TAG+="snap_0ad_0ad" | 12:35 |
| jdstrand | mborzecki: that's ok though. we are very strict that it is 'snap_name_cmd' | 12:35 |
| jdstrand | mborzecki: so to date, there will only ever be exactly two underscores | 12:35 |
| mborzecki | jdstrand: right, but then if you have snap_0ad_local_0ad it's fine too right? | 12:36 |
| jdstrand | that would be difficult | 12:36 |
| jdstrand | snap_0ad_0ad_local would be better | 12:36 |
| jdstrand | well | 12:36 |
| jdstrand | you could do it | 12:36 |
| mborzecki | i mean, the snap is named 0ad_local in this case | 12:37 |
| jdstrand | if count(underscores) == 3, then name is 0ad_local | 12:37 |
| jdstrand | it does mean we'll need to adjust that parsing everywhere though | 12:37 |
| jdstrand | and to finish my PSA for security team position: https://boards.greenhouse.io/canonical/jobs/1158266. it's a great team to work for! :) | 12:41 |
| mborzecki | jdstrand: hmm snap-device-helper seems to do some funny stuff with those names | 12:42 |
| mvo | jdstrand: I would totally apply if I hadn't a great team already | 12:43 |
| jdstrand | hehe | 12:43 |
| jdstrand | I'm not trying to poach snapd team members :) | 12:43 |
| jdstrand | there are others in the channel who may see it ;) | 12:43 |
| mvo | jdstrand: I know :) | 12:43 |
| mvo | jdstrand: still you guys have a great team | 12:44 |
| jdstrand | mvo: that said, you would be a wonderful addition, as would any of the snapd devs (but I'm not poaching-- I just love working with all of you. good thing I get to continue doing so :) | 12:44 |
| jdstrand | mvo: thanks! it's true. the security team is awesome | 12:44 |
| mvo | heh :) | 12:44 |
| * mvo blushes | 12:44 | |
| jdstrand | mborzecki: right, it needs to convert back to '.' for the device cgroup name in /sys/fs/cgroup/devices | 12:46 |
| jdstrand | mborzecki: eg: /sys/fs/cgroup/devices/snap.firefox.firefox | 12:46 |
| jdstrand | mborzecki: so it would have to learn to do snap_firefox_local_firefox -> snap.firefox_local.firefox | 12:46 |
| mborzecki | jdstrand: right, so another small update there :/ | 12:47 |
| zyga | mborzecki: btw do you have that huge rename PR ready? | 12:47 |
| jdstrand | mborzecki: we try to use '.' as the delimiter everywhere we can for consistency (I tried to use '_' in the early days, but '.' was deemed prettier (is is)) | 12:47 |
| jdstrand | it* is | 12:47 |
| mborzecki | zyga: yes, #5253 | 12:48 |
| mup | PR #5253: snap: introduce new fields for parallel snap installation <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/5253> | 12:48 |
| zyga | I'll review it after standupo | 12:48 |
| mborzecki | jdstrand: use # and watch as the world burns :P | 12:48 |
| jdstrand | hehe | 12:48 |
| mvo | zyga: I think you made me write locking code in the errtracker "db". its just too embarassing to write a comment that there is none | 12:50 |
| mvo | zyga: smart move ;) | 12:50 |
| zyga | hahaha | 12:50 |
| * zyga mutters "success" ;) | 12:51 | |
| jdstrand | mborzecki: note that in the apparmor profile itself, the security label is set in various apparmor variables at the top: | 12:55 |
| jdstrand | @{SNAP_NAME}="0ad" | 12:55 |
| jdstrand | @{SNAP_REVISION}="18" | 12:55 |
| jdstrand | @{PROFILE_DBUS}="snap_2e0ad_2e0ad" | 12:55 |
| jdstrand | @{INSTALL_DIR}="/{,var/lib/snapd/}snap" | 12:55 |
| jdstrand | profile "snap.0ad.0ad" (attach_disconnected,mediate_deleted) { | 12:55 |
| jdstrand | mborzecki: you probably saw that, but I mention it specifically for PROFILE_DBUS | 12:55 |
| zyga | this is such an old concept it will take a while to change everywhere | 12:56 |
| jdstrand | mborzecki: that will probably just work, but do make sure you get snap_2e0ad_2elocal_2e0ad when have snap.0ad.0ad | 12:57 |
| jdstrand | mborzecki: sorry | 12:57 |
| mborzecki | jdstrand: SNAP_NAME=0ad_local, then PROFILE_DBUS will be snap_2e0ad_local_2e0ad | 12:57 |
| jdstrand | mborzecki: that will probably just work, but do make sure you get snap_2e0ad_2elocal_2e0ad when have snap.0ad_local.0ad | 12:57 |
| mborzecki | jdstrand: and profile "snap.0ad_local.0ad" | 12:57 |
| mborzecki | jdstrand: snap_2e0ad_2elocal_2e0ad instead of snap_2e0ad_local_2e0ad then? | 12:58 |
| jdstrand | snap.0ad_local.0ad translated to dbus is snap_2e0ad_2elocal_2e0ad | 12:58 |
| jdstrand | oh no | 12:58 |
| jdstrand | _2e is '.' | 12:59 |
| jdstrand | gimme a sec | 12:59 |
| pedronis | mborzecki: seems SNAP_NAME is sometimes used in places that really need the instance name in the templates | 12:59 |
| jdstrand | what you use depends on how the mounts are setup | 13:00 |
| pedronis | not all uses are for directories in the namespace | 13:00 |
| zyga | standup time | 13:00 |
| jdstrand | right | 13:00 |
| jdstrand | I don't know what is happening at the filesystem level | 13:00 |
| jdstrand | but you'll the security label to include _local for IPC, etc | 13:01 |
| jdstrand | you'll want | 13:01 |
| jdstrand | mborzecki: it would be easiest if the file accesses looked like /snap/foo_local/... instead of /snap/foo/... | 13:02 |
| mborzecki | jdstrand: hm well, it was requested that we bind mount to /snap/foo | 13:03 |
| jdstrand | mborzecki: but if that isn't possible/desirable, you'll need to introduce another apparmor variable: SNAP_NAME_FILE (or something) | 13:04 |
| jdstrand | eg: | 13:04 |
| jdstrand | @{SNAP_NAME}="0ad_local" | 13:04 |
| jdstrand | @{SNAP_NAME_FILE}="0ad" | 13:04 |
| jdstrand | with file accesses using @{SNAP_NAME_FILE} and everything else @{SNAP_NAME} | 13:04 |
| jdstrand | mborzecki: that is tricky though. $SNAP is easy enough, but SNAP_DATA, SNAP_COMMON and especially SNAP_USER_DATA and SNAP_USER_COMMON are going to be harder, since you'll have to manage all those addition bind mounts | 13:06 |
| jdstrand | it is possible, just more complexity | 13:06 |
| jdstrand | it is only possible because of the recent user mount work btw since a root processes will be mucking around in the user's home for SNAP_USER_DATA and SNAP_USER_COMMON | 13:07 |
| zyga | mvo: https://github.com/snapcore/snapd/pull/5263 is green and has +2 | 13:09 |
| jdstrand | we are setting ourselves up for if there is a bug, then 0ad_local might be able to write to 0ad's user data. from a complexity/secure design perspective, I recommend 0ad_local everywhere | 13:09 |
| mup | PR #5263: errtracker: do not send duplicated reports <Created by mvo5> <https://github.com/snapcore/snapd/pull/5263> | 13:09 |
| jdstrand | mborzecki: ^. that doesn't mean I would block the design. but I think that there needs to be an active decision that the benefit outweighs the complexity/risk | 13:10 |
| jdstrand | mborzecki: I would like to be involved in the PR reviews as they pertain to security | 13:12 |
| mborzecki | jdstrand: sure | 13:12 |
| * jdstrand adds a card to trello so ratliff is aware | 13:12 | |
| jdstrand | ratliff: this'll be a blue item | 13:12 |
| * ratliff reads back to understand the priority | 13:13 | |
| jdstrand | mborzecki: hmm, I'm not sure how parallel installs are going to work with IPC. eg, two network-managers or two dockers installed | 13:15 |
| jdstrand | mborzecki: they necessarily need to have their service in the global namespace, so will conflict with each other... | 13:16 |
| mborzecki | jdstrand: i suppose common sense will have to prevail here, unless the other docker instance is configured to listen on different path things will break | 13:18 |
| mborzecki | jdstrand: maybe postgres usecase is simpler, local snaps on different ports (docker mangles some kernel state so it's probably not the best thing to be installed mulitple times) | 13:19 |
| jdstrand | mborzecki: yes... perhaps we can start be saying "if you implement a slot, you can't do this" | 13:19 |
| jdstrand | mborzecki: that said, gnome apps all need to slot a dbus interface for the well-known name | 13:20 |
| jdstrand | mborzecki: so they are going to break if parallel installed | 13:20 |
| jdstrand | (and it isn't just gnome) | 13:21 |
| mborzecki | jdstrand: this, probably snaps that have socket activation too | 13:21 |
| jdstrand | yeah | 13:21 |
| mborzecki | at least using abstract socket paths | 13:22 |
| jdstrand | mborzecki: can you describe the feature of parallel installs? is it documented somewhere? | 13:22 |
| mborzecki | jdstrand: https://forum.snapcraft.io/t/parallel-snap-installs/5763 | 13:22 |
| jdstrand | mborzecki: well, any path where the client expects to find the server somewhere specific. it could be a dbus well-known name, abstract socket, named socket, pipe, ... all kinds of stuff | 13:23 |
| niemeyer | jdstrand, mborzecki: Can we have a quick sync up call shortly? (after standup, on going) | 13:24 |
| mborzecki | niemeyer: ok | 13:24 |
| jdstrand | niemeyer: maybe after that we can discuss update-alternatives and interface docs | 13:25 |
| niemeyer | jdstrand: Sounds good! | 13:25 |
| jdstrand | ratliff: I created a preliminary card in trello | 13:27 |
| === pstolowski|lunch is now known as pstolowski | ||
| zyga | uhhh | 13:39 |
| zyga | my coffee machine erupted :/ | 13:39 |
| pedronis | niemeyer: pstolowski: this can be re-reviewed now: https://github.com/snapcore/snapd/pull/5221 | 13:39 |
| mup | PR #5221: snap: parse connect instructions in gadget.yaml <Created by pedronis> <https://github.com/snapcore/snapd/pull/5221> | 13:39 |
| pstolowski | ok | 13:39 |
| zyga | it punctured the capsule incorrectly and all the coffee went outside the wrong way | 13:39 |
| genii | zyga: Perhaps it's one of those recalled Keurigs | 13:40 |
| zyga | no, I'm not familiar with those | 13:40 |
| mborzecki | zyga: just get a moka pot ;) | 13:40 |
| zyga | I'll get a mop and a pot ;) | 13:40 |
| pstolowski | guys, if you see econnreset test failure again please let me know, i lost yesterday's log when I re-loaded the tab with travis log today | 13:54 |
| pedronis | pstolowski: should we add some logging to the retry logic to see the error when we don't retry? | 13:56 |
| pstolowski | pedronis: yep, that may help | 13:57 |
| jdstrand | niemeyer, mborzecki: I tried to summarize much of the above in https://forum.snapcraft.io/t/parallel-snap-installs/5763/3 | 14:00 |
| mborzecki | jdstrand: thanks! | 14:03 |
| mvo | zyga: a second review for 5274 would be great, you did a first pass already afaict | 14:08 |
| zyga | ACLU | 14:10 |
| zyga | Ack | 14:10 |
| zyga | I wonder what spellchecker contains ACLU | 14:10 |
| jdstrand | niemeyer, mborzecki: I'm ready whenever you are | 14:10 |
| niemeyer | jdstrand: Just finishing a pre-scheduled meeting I had until the half hour.. should be off in 20 or so | 14:11 |
| jdstrand | ack | 14:11 |
| mup | PR snapd#5279 opened: interfaces/builtin: create socketcan interface <Created by jocave> <https://github.com/snapcore/snapd/pull/5279> | 14:18 |
| jdstrand | fyi, I've added a review of PR 5279 to my list. I have a number of questions. others might want to wait until I do my first review | 14:30 |
| mup | PR #5279: interfaces/builtin: create socketcan interface <Created by jocave> <https://github.com/snapcore/snapd/pull/5279> | 14:30 |
| jdstrand | joc: fyi ^ (and thanks for the PR. I'm stepping into a meeting so it'll be a bit) | 14:31 |
| joc | np, thanks for looking at it jdstrand | 14:31 |
| niemeyer | jdstrand, mborzecki: https://meet.google.com/dnp-muwd-mng | 14:35 |
| mup | PR snapd#5280 opened: httputil: extra debug if an error is not retried <Created by stolowski> <https://github.com/snapcore/snapd/pull/5280> | 14:36 |
| greyback | who can I poke to get a human reviewer for https://launchpad.net/~gerboland/+snap/chromium-mir-kiosk/+build/241717 | 14:41 |
| mup | PR snapcraft#2128 closed: project_loader: stop setting LD_LIBRARY_PATH <Created by kyrofa> <Closed by kyrofa> <https://github.com/snapcore/snapcraft/pull/2128> | 14:41 |
| greyback | ah, there's a button. ignore me | 14:45 |
| didrocks | jdstrand: hey, around | 15:01 |
| jdstrand | didrocks: hey, so we were thinking we were going to have the update-alternatives meeting now, but it'll be in an hour or so | 15:05 |
| jdstrand | didrocks: your attendance isn't stricly required, so if you can't attend that's ok, but if you can that would be great. is that an ok time? | 15:06 |
| didrocks | jdstrand: hum, I can't be around at that time, I have some family duties | 15:06 |
| didrocks | jdstrand: I don't think indeed that I'm required, you know update-alternatives as well as I do if not better :) | 15:06 |
| jdstrand | didrocks: that's fine. I understand the problem and even the specifics of the access you desire | 15:07 |
| didrocks | jdstrand: the only thing to remember is that the alternative isn't on a binary, but on a css file used by gdm via gnome-shell | 15:07 |
| didrocks | gdm3.css | 15:07 |
| jdstrand | yep | 15:07 |
| didrocks | which doesn't match the snap name | 15:07 |
| jdstrand | nope | 15:07 |
| jdstrand | :) | 15:07 |
| didrocks | I guess that's all you need to know :) | 15:07 |
| jdstrand | ok, thanks! | 15:07 |
| didrocks | jdstrand: ah, and you got why I separated that in 2 snaps, one arch:all and the other one? | 15:08 |
| jdstrand | didrocks: I'll summarize the outcome in the forum after the meeting | 15:08 |
| didrocks | perfect! | 15:08 |
| jdstrand | didrocks: actually, I forgot that detail | 15:08 |
| jdstrand | there is gsettings and update alternatives | 15:08 |
| didrocks | jdstrand: basically, I built the theme via Travis CI | 15:08 |
| didrocks | which is using the docker image | 15:08 |
| cachio | niemeyer, PR for gc ready https://github.com/snapcore/spread/pull/60 | 15:08 |
| didrocks | and so, I need to have the theme snap arch: all | 15:08 |
| mup | PR spread#60: Garbage collection for google backend <Created by sergiocazzolato> <https://github.com/snapcore/spread/pull/60> | 15:09 |
| cachio | niemeyer, and tested | 15:09 |
| didrocks | I can't use snapcraft.io because the theme is made of 5 github projects | 15:09 |
| didrocks | and any commit in any of those can trigger a new revisino | 15:09 |
| didrocks | if you are interested into the glory detail, the snapcraft.yaml is: https://github.com/ubuntu/communitheme-snap-helpers/blob/master/snap/snapcraft.yaml | 15:10 |
| didrocks | and the build script is https://github.com/ubuntu/communitheme-snap-helpers/blob/master/build/prepare-build-snap (pulled by the 5 projects in Travis) | 15:10 |
| didrocks | Note: sed -i "s# source: .*$TRAVIS_REPO_SLUG\.git# source: \.#" snap/snapcraft.yaml | 15:10 |
| didrocks | which means "for the current project, take the local branch" (handling PR) | 15:10 |
| didrocks | I guess that's it, more details about what I do for the theme and CI is at https://didrocks.fr/2018/04/10/welcome-to-the-ubuntu-bionic-age-new-wip-ubuntu-theme-as-a-snap/ | 15:11 |
| jdstrand | didrocks: it seems you could instead of pointing at 5 repositories, point at one (yours, has files from all 5), and then pull in git commits from the 5 into your one as desired. then you can hook that up and do arch specific builds | 15:11 |
| didrocks | jdstrand: it won't work easily for proposed changes though | 15:12 |
| jdstrand | didrocks: it still isn't clear why the 5 trees requires arch stuff.. | 15:12 |
| didrocks | jdstrand: see https://github.com/ubuntu/gtk-communitheme/pull/526#issuecomment-395422606 | 15:13 |
| mup | PR ubuntu/gtk-communitheme#526: Made suggested action label button more visible when disabled <Created by clobrano> <https://github.com/ubuntu/gtk-communitheme/pull/526> | 15:13 |
| jdstrand | it would be maintenance overhead, so would have to weigh the options | 15:13 |
| didrocks | thanks to this, on each project, I can detect PR in Travis | 15:13 |
| didrocks | and build a particular snap with that changes on those PR | 15:13 |
| didrocks | that people can switch to, test… | 15:13 |
| pstolowski | #5280 hit that spread panic | 15:13 |
| didrocks | you can't really do that with snapcraft.io, which is only one branch on one project | 15:14 |
| mup | PR #5280: httputil: extra debug if an error is not retried <Simple> <Created by stolowski> <https://github.com/snapcore/snapd/pull/5280> | 15:14 |
| jdstrand | didrocks: perhaps that is more of an ev thing then | 15:14 |
| jdstrand | didrocks: but the two-snaps approach doesn't really change the conversation regarding update-alternatives/gsettings, right? | 15:14 |
| didrocks | jdstrand: got the confirmation that's not planned/doable right now on the store side, and I have a solution which works. All this, I mean, "I have to use Travis CI, so docker image, so one arch build, hence arch: all, hence 2 snaps" | 15:15 |
| didrocks | jdstrand: no, it's just to explain why there are 2 snaps instead of one :) | 15:15 |
| pstolowski | mborzecki: how far is your spread fix from landing? do you need a review? | 15:15 |
| jdstrand | I see. ok, well, if it comes up, I can refer back to this | 15:15 |
| jdstrand | thanks! | 15:15 |
| didrocks | jdstrand: thank you! :) | 15:16 |
| mborzecki | pstolowski: niemeyer said he'll do a slightly different fix, we'll have to wait a bit | 15:17 |
| pstolowski | ok | 15:17 |
| jdstrand | mborzecki: re hard link vs symlink> of course, you can't hard link a dir. since we are only talking about $SNAP, you could symlink from the local ones to /snap/foo/current then adjust the template policy as needed | 15:19 |
| jdstrand | mborzecki: that could probably be made to work | 15:20 |
| mvo | hrm, 5274 consistently fails with a spread panic here :/ | 15:24 |
| * mvo takes a break | 15:24 | |
| mborzecki | jdstrand: afaiu hardlinking was in the context of *.snap (the squashfs images) | 15:24 |
| jdstrand | mborzecki: that would work. it does mean the kernel then has 3 different mount points. I wonder if it will dedupe? | 15:29 |
| pedronis | mborzecki: I left some more input into the PR, some of it is really note about areas that will need attention, might want to add a TODO now or keep a note | 15:31 |
| pedronis | but didn't go to the end of it | 15:31 |
| cachio | niemeyer, did you fixed the spread panic issue ? | 15:34 |
| cachio | niemeyer, I can work on that if you want | 15:35 |
| niemeyer | No, have been in calls all morning, and having lunch now.. sure, if you have a clear view of the fix, please open a PR.. otherwise I can quickly look at it | 15:36 |
| cachio | niemeyer, ok, I'll take a look now | 15:38 |
| Saviq | jdstrand: hey, re: https://forum.snapcraft.io/t/classic-confinement-for-subsurface/5795 - should raw-usb give me access to /dev/ttyUSB0? any idea why I'm not seeing denials even though the app can't open the serial port? | 15:52 |
| mborzecki | cachio: you can take a look at https://github.com/snapcore/spread/pull/59 niemeyer mentioned he wanted something more informative (maybe some logging there as well?) | 16:07 |
| mup | PR spread#59: spread: do not panic if error message from google backend is empty <Created by bboozzoo> <https://github.com/snapcore/spread/pull/59> | 16:07 |
| mborzecki | cachio: don't know if spread is doing direct http calls to the api, if you suspect it's due to 500s the maybe it'd be possible to catch the problem earlier | 16:09 |
| jdstrand | Saviq: if you aren't seeing logged denials, I'm not sure why unless you are non-root or the device didn't end up in the snap's device cgroup | 16:09 |
| jdstrand | Saviq: the raw-usb interface uses var rawusbConnectedPlugUDev = []string{`SUBSYSTEM=="usb"`} | 16:09 |
| jdstrand | Saviq: it does not have /dev/ttyUSB*, but I would expect a denial to be logged | 16:10 |
| jdstrand | Saviq: you can see what is allowed to the snap in the cgroup with: cat /sys/fs/cgroup/devices/snap.name.cmd/devices.list | 16:11 |
| cachio | mborzecki, sure, I'll take a look | 16:12 |
| jdstrand | Saviq: (but you will need to have run the snap once for that to show anything) | 16:12 |
| mborzecki | cachio: this will only prevent the panic, if you can catch the problem earlier that'd be great | 16:12 |
| zyga | jdstrand: hey, can you please enqueue https://github.com/snapcore/snapd/pull/5278/files | 16:14 |
| mup | PR #5278: cmd/snap-update-ns: add IsTrustedTmpfs and tests <Created by zyga> <https://github.com/snapcore/snapd/pull/5278> | 16:14 |
| === pstolowski is now known as pstolowski|afk | ||
| pedronis | mborzecki: btw another thing to consider is that there might be places that assume there is one local snap name for each snap-id, which will no longer be true, UpdateMany and helpers is example (but problably not the only one), they use a stateByID map | 16:17 |
| pedronis | atm | 16:17 |
| jdstrand | ok | 16:20 |
| zyga | thank you | 16:20 |
| rbasak | nacc: o/ | 16:22 |
| rbasak | nacc: oh, I'm wrong | 16:22 |
| rbasak | gpgrt_get_syscall_clamp appears in usr/lib/libgpg-error.so.0.22.0 both the good and bad snaps | 16:22 |
| niemeyer | mborzecki, cachio: No logging at this point.. we already have logging.. it's the creation of the error that is wrong | 16:22 |
| rbasak | That doesn't seem right | 16:23 |
| nacc | rbasak: yeah it's right :) | 16:23 |
| nacc | rbasak: the only thing i've been able to determine is when _pygit2's ldd shows the wrong libgpg-error | 16:24 |
| nacc | where determine == indicates a good or bad snap | 16:24 |
| nacc | rbasak: it's what makes me think our build is fine and it's a bug in snapcraft :) | 16:24 |
| zyga | mvo: did another pass over 5274 | 16:24 |
| cyphermox | ogra_: poke; ubuntu-core-libs, do you want to get that seeded somewhere (tbh, I'm not sure where) so it ends up in main? | 16:25 |
| cyphermox | (bug LP: #1572539) | 16:25 |
| rbasak | nacc: yes, that does look different: https://pastebin.ubuntu.com/p/xtnYDMhYR7/ | 16:25 |
| rbasak | nacc: OK, I think I might be caught up with you now :) | 16:26 |
| mup | Bug #1572539: [MIR] ubuntu-core-libs <ubuntu-core-meta (Ubuntu):Fix Committed> <https://launchpad.net/bugs/1572539> | 16:26 |
| nacc | rbasak: sorry, otp right now; and thit is almost exactly what we hit before, so i had a lot of context in my head | 16:26 |
| nacc | rbasak: and yep, that's exactly the symptom i see | 16:26 |
| nacc | i think it's a bug in the rpath/patchelf usage by snapcraft | 16:26 |
| rbasak | Would you agree it's non-deterministic in snapcraft? And I'm just unlucky that it only happens on Launchpad and not locally? | 16:27 |
| nacc | rbasak: that's my suspicion; did you notice one of my CI runs failed the way your didn't? | 16:27 |
| nacc | and in the same way as LP's build did | 16:27 |
| rbasak | I didn't, but I guess you're just luckier than I am then :) | 16:27 |
| nacc | kyrofa: mentioned that the deterministic ordering stuff is in a PR now | 16:28 |
| rbasak | Even if it's made deterministic, how do we make sure that the one used is the one we want? | 16:28 |
| nacc | like i mentioned in #launchpad, it might make us fully broken or it might fully fix it :) | 16:28 |
| rbasak | Right :) | 16:28 |
| zyga | mborzecki: can you do a quick pass over 5287 | 16:28 |
| zyga | er | 16:28 |
| zyga | 5278 | 16:29 |
| nacc | yeah, it depends on what the bug we're hitting actually is, and for that i think we need kyrofa or sergiusens to look | 16:29 |
| rbasak | I think we also need snapcraft to follow our ignores from the stage directive | 16:29 |
| nacc | rbasak: i think it does, right? do we ever see the wrong libgpg-error in stage? | 16:29 |
| * zyga has some mental thing where sometimes last two characters and almost always last two digits get swapped | 16:29 | |
| nacc | rbasak: *possibly* the rpath needs to respect the eliding | 16:29 |
| rbasak | I don't know what's present in stage in the broken case since I can't reproduce a broken build tree | 16:29 |
| nacc | yeah | 16:30 |
| rbasak | But I don't see anything in stage/lib/ in the successful case, which is where I'd expect that one to go | 16:30 |
| rbasak | So I think the ignore is working, but it's later being unignored. | 16:31 |
| nacc | right, could be | 16:31 |
| nacc | this was a pain to debug, and kyrofa just understood the details a lot better than i did and was able to figure it out faster than i could :) | 16:31 |
| rbasak | So I guess we're broken until this can be addressed in snapcraft. In the meantime, the best I can do is manually build the snap locally and upload after verification that it's OK. | 16:32 |
| rbasak | I wonder if I can hack an actual deletion of the file from parts/ | 16:33 |
| rbasak | And if that would break anything | 16:33 |
| rbasak | Not that I could test that since I can't reproduce :( | 16:34 |
| kyrofa | rbasak, nacc yeah I think the first thing we need is a way to reproduce | 16:35 |
| kyrofa | rbasak, have you tried using edge, by any chance? One (of two) deterministic-fixing things has landed there | 16:35 |
| cachio | mborzecki, niemeyer the PR works well | 16:36 |
| cachio | niemeyer, but there is something else which is causing this 500 | 16:36 |
| rbasak | kyrofa: I could try locally where I can't reproduce, but I'm not sure where that'll get me | 16:36 |
| kyrofa | rbasak, i.e. with edge, you should notice that snapcraft handles parts in the same order between runs | 16:36 |
| kyrofa | rbasak, it may allow you to reproduce every time, or it may fix LP, haha | 16:36 |
| zyga | jdstrand: #5279 is interesting | 16:36 |
| mup | PR #5279: interfaces/builtin: create socketcan interface <Created by jocave> <https://github.com/snapcore/snapd/pull/5279> | 16:37 |
| rbasak | I don't think I have any choice about what LP runs | 16:37 |
| rbasak | But I can see if it reproduces every time, sure :) | 16:37 |
| kyrofa | Indeed, and it won't be fixed there yet anyway | 16:37 |
| kyrofa | Yeah see if changes anything for you locally | 16:37 |
| rbasak | I'm running out of time today. I'll try that tomorrow. | 16:37 |
| niemeyer | cachio: What PR? | 16:37 |
| cachio | niemeyer, https://github.com/snapcore/spread/pull/59 | 16:37 |
| mup | PR spread#59: spread: do not panic if error message from google backend is empty <Created by bboozzoo> <https://github.com/snapcore/spread/pull/59> | 16:37 |
| rbasak | kyrofa, nacc: thank you for your help. | 16:37 |
| jdstrand | zyga: yes I mentioned that earlier | 16:38 |
| kyrofa | Of course. I'll chat with sergiusens and see if we can get the other deterministic thing sorted as well | 16:38 |
| zyga | oh, sorry I missed trhat | 16:38 |
| kyrofa | (library search order) | 16:38 |
| niemeyer | cachio: No it doesn't work well.. an error value that says "error" is quite pointless | 16:38 |
| nacc | rbasak: np, sorry for not documenting what i found better | 16:38 |
| sergiusens | rbasak, nacc: since you are building with jenkins, I would suggest you use the snap instead of the deb. We are waiting for that change to happen for buildd's as well | 16:40 |
| joc | zyga: the question about whether to use network interfaces is indeed interesting, i'll wait to see what you all think! | 16:40 |
| zyga | jdstrand: AFAIK some filesystems are toying with hardlinking directories but it's not mainline yet | 16:40 |
| zyga | joc: I'm +0.8 on making it a socket type and going with existing interfaces | 16:41 |
| mvo | pstolowski|afk: here is a econnereset https://api.travis-ci.org/v3/job/389153950/log.txt for you (you asked earlier) | 16:43 |
| zyga | jdstrand: oh and totally forgot to ask you | 16:43 |
| pstolowski|afk | mvo: great, thanks, i'll save it and inspect tomorrow | 16:43 |
| zyga | jdstrand: the new seccomp deferral to userspace feature that was on LWN looks super juicy | 16:43 |
| zyga | jdstrand: do you think we should eye supporting that? | 16:43 |
| mvo | zyga: 5263 is also ready for another look | 16:44 |
| mvo | pstolowski|afk: yw | 16:44 |
| zyga | mvo: I was reading it already | 16:45 |
| zyga | just approved it again | 16:45 |
| zyga | (with two questions) | 16:45 |
| mup | PR snapd#5272 closed: cmd/snap-update-ns: improve wording in many errors <Created by zyga> <Merged by zyga> <https://github.com/snapcore/snapd/pull/5272> | 16:45 |
| zyga | pstolowski|afk: econreset: https://api.travis-ci.org/v3/job/389153950/log.txt | 16:46 |
| pstolowski|afk | zyga: yep, just got it from mvo above, thanks | 16:47 |
| zyga | do you want me to save the log and restart? | 16:47 |
| zyga | mvo: shall we close https://github.com/snapcore/snapd/pull/5234 ? | 16:49 |
| mup | PR #5234: snap: add `snap list --format=...` option <Created by mvo5> <https://github.com/snapcore/snapd/pull/5234> | 16:49 |
| jdstrand | zyga: I think that is the bit that lxd and other container managers wanted. tyhicks could probably comment further. iirc, that is potentially interesting for prompting, but iirc, jjohansen mentioned that it wasn't sufficient. jjohansen already started apparmor prompting | 16:51 |
| zyga | I was wondering about how fast it is and if it could be used to make changes to seccomp "profiles" instant without process recycling | 16:52 |
| jdstrand | https://lwn.net/Articles/754789/ is what I presume you looked at | 16:52 |
| zyga | _perhaps_ https://lwn.net/Articles/756233/ | 16:52 |
| zyga | jdstrand: this is also potentially super useful for... classic confinement | 16:53 |
| zyga | we could do smart exec interception | 16:53 |
| zyga | execing inside $SNAP would give you the dynamic linker interception with correct flags | 16:54 |
| jjohansen | its interesting, arguments from it are still a problem | 16:54 |
| zyga | execing outside would give you the regular behaviour | 16:54 |
| zyga | (so finally we could ship bash as a classically confined app) | 16:54 |
| zyga | jjohansen: yes, I understand it's still WIP | 16:54 |
| zyga | I'd love if some work to copy the arguments for inspection was applied to seccomp as this would make it x10 more useful | 16:55 |
| cachio | niemeyer, yes, that's true, I am still researching what it is causing that google returns the error | 16:55 |
| jdstrand | hmm, apparently my subscription expired | 16:55 |
| zyga | jjohansen: hey :-) how are you? | 16:55 |
| jjohansen | hey zyga | 16:55 |
| jdstrand | that's weird | 16:55 |
| niemeyer | cachio: I don't think that's critical in this case.. a 500 is an error on Google side.. it's blowing up beyond our control.. what we need is to simply be able to display that, and to retry if needed. | 16:55 |
| niemeyer | cachio: Most backends, and I suspect Google included, already have logic for retrying on such errors | 16:56 |
| zyga | jdstrand: I sent you a subscriber link | 16:56 |
| zyga | niemeyer: spread broke google? :-) | 16:56 |
| zyga | next up, azure! | 16:56 |
| niemeyer | zyga: Apparently.. | 16:56 |
| cachio | hehehe | 16:56 |
| jdstrand | I would have to believe there would be a significant performance hit | 16:56 |
| zyga | jdstrand: people in the thread there say it's next to none | 16:56 |
| jdstrand | zyga: like, we have the old policy in the kernel and ask userspace if it has something new? | 16:57 |
| zyga | I didn't read the patches or anything, it just looks interesting on the outside | 16:57 |
| jjohansen | zyga: the problem with seccomp args is that copying them is TOCTTOU race | 16:57 |
| zyga | jdstrand: I think the point is that there is no policy | 16:57 |
| jjohansen | which is always bad for security | 16:57 |
| zyga | it's a decision that goes to userspace to ack (perform) and return a result / error / fd | 16:57 |
| zyga | jjohansen: I know but I was hinting at a way to copy them out once | 16:57 |
| zyga | jjohansen: and then pass them to other layers already in the kernel | 16:57 |
| cachio | niemeyer, it is nice to this this comment in the code | 16:58 |
| cachio | / Repeat on 500s. Comes from Linode logic, not observed on Google so far. | 16:58 |
| zyga | jjohansen: it's doable, just nobody is doing that apparently | 16:58 |
| zyga | jjohansen: perhaps not everything can be copied but common string / stat buffers should be ok | 16:58 |
| jjohansen | sure, problem is you need to rewrite the whole syscall stack todo that | 16:58 |
| zyga | :D | 16:58 |
| cachio | I should update it too | 16:58 |
| zyga | I understand that's probably the reason | 16:58 |
| zyga | jjohansen: it's the kernel, I don't doubt it will happen, initially people will hate it, then several years later it will be better than sliced bread | 16:59 |
| zyga | just after more drivers in userspace and other microkernel things ;) | 16:59 |
| jjohansen | zyga: uhmmm, probably not ever going to happen | 16:59 |
| zyga | jjohansen: well, isn't that _already_ happening? | 16:59 |
| jjohansen | it might happen on a couple syscalls | 17:00 |
| jjohansen | zyga: no | 17:00 |
| zyga | elf in userspace | 17:00 |
| zyga | ah, the syscall thing | 17:00 |
| zyga | but the syscall data has to be copied out anyway | 17:00 |
| zyga | well, I'm not a kernel hacker, I'm sure it's not trivial and very performance sensitive | 17:00 |
| jjohansen | zyga: they are just copying the 6 register values for the syscall, regardless of whether the syscall actually uses 6 register values | 17:01 |
| zyga | it's just that I don't believe in "no" anymore as the kernel has crazier stuff thrown into it every year | 17:01 |
| niemeyer | cachio: Do you have that debug output which observed the 500 at hand? | 17:01 |
| jjohansen | zyga: eg. ioctl, that syscall has different params values sizes, .. based on which ioctl it is and new ones are always being added | 17:02 |
| cachio | niemeyer, this it what I have https://paste.ubuntu.com/p/HMvvxNMq9G/ | 17:02 |
| zyga | jjohansen: that's a good point | 17:02 |
| jjohansen | its a complete mess, and only the specific ioctl handler deals with it | 17:02 |
| zyga | jjohansen: still, handling open and a few related calls this way would make seccomp incredibly more powerful | 17:02 |
| jjohansen | some syscalls maybe will get the treatment but not all of them | 17:02 |
| zyga | ioctl can stay as is | 17:02 |
| jjohansen | zyga: there are other reasons it problematic for seccomp to access the args | 17:03 |
| jjohansen | like it needs to do the copy from user but its in the syscall assembly code level | 17:03 |
| niemeyer | cachio: Nice, thanks.. let me cook something up quickly | 17:03 |
| jjohansen | its not impossible, but it sure makes things uglier | 17:03 |
| Saviq | jdstrand: so ttyUSB0 seems to be 188:0 AFAICT and devices.list does not have that | 17:11 |
| Saviq | but agreed I don't understand why no DENIED | 17:11 |
| jdstrand | Saviq: DAC is evaluated first, so if it isn't in the device cgroup, then no denial | 17:14 |
| jdstrand | Saviq: what is the output of: udevadm info /dev/ttyUSB0 | 17:14 |
| Saviq | jdstrand: http://paste.ubuntu.com/p/vtxzQHHVWQ/ | 17:14 |
| jdstrand | Saviq: it isn't tagged: E: TAGS=:systemd: | 17:15 |
| zyga | btw, jdstrand I don't know if you are aware of an issue with systemd and the recent introduction of bind/unbind events | 17:16 |
| jdstrand | Saviq: what is the output of: snap interfaces -i raw-usb | 17:16 |
| zyga | it apparently has caused issues with udev tagging | 17:16 |
| * zyga stumbled upon systemd bug report about this | 17:16 | |
| jdstrand | zyga: I'm not on that specific issue. when was it introduced? I needed to redo things some time ago for a change in behavior | 17:17 |
| jdstrand | niemeyer: is now a good time to meet? | 17:18 |
| cachio | niemeyer, I am not getting the panic anymore with this change https://paste.ubuntu.com/p/VhKjqwdnyf/ | 17:20 |
| jdstrand | Saviq: fyi, the device.list with c:188:0 is 'character device' with major 188 and minor 0. ls -l /dev/ttyUSB0 will of course give you major minor. I don't have a USB serial plugged in so didn't confirm 188:0 | 17:20 |
| cachio | niemeyer, basically because the error message is comming empty | 17:21 |
| niemeyer | jdstrand: Just finishing the proposed change and will be with you | 17:21 |
| jdstrand | Saviq: but, clearly the device isn't udev-tagged so I wouldn't expect it in there | 17:21 |
| niemeyer | cachio: I don't know how to say that more clearly.. we discussed this in the meeting, and I just explained above.. of course the error is gone if you kill the error message | 17:21 |
| niemeyer | cachio: We don't want that | 17:21 |
| niemeyer | cachio: We want to fix the error message instead.. please leave it with me.. I'll push a change in a minute | 17:22 |
| zyga | jdstrand: let me find the bug link | 17:22 |
| zyga | jdstrand: it's already deployed to bionic | 17:22 |
| zyga | jdstrand: the summary of the bug is: systemd sees bind/unbind events and treats them as add/remove, dropping tags | 17:23 |
| zyga | https://github.com/systemd/systemd/issues/8221 | 17:23 |
| zyga | the comments indicate that this has caused widespread issues | 17:23 |
| jdstrand | zyga: this seems different from what I saw and that systemd will need a patch | 17:25 |
| mup | PR snapd#5280 closed: httputil: extra debug if an error is not retried <Simple> <Created by stolowski> <Merged by stolowski> <https://github.com/snapcore/snapd/pull/5280> | 17:26 |
| niemeyer | cachio: Please give this a shot and let me know how it goes: https://github.com/snapcore/spread/commit/9aa319e | 17:30 |
| niemeyer | cachio: It's in master | 17:30 |
| niemeyer | mborzecki: ^ | 17:31 |
| cachio | niemeyer, sure | 17:31 |
| niemeyer | Most of the change in the loop is just indenting it in so we can repeat the call from later on | 17:31 |
| niemeyer | jdstrand: I'm ready | 17:32 |
| niemeyer | jdstrand: https://meet.google.com/ipr-very-aqb | 17:32 |
| jdstrand | ok | 17:33 |
| cachio | niemeyer, it is working | 17:35 |
| cachio | spread images was failing 100% of the runs and now it does not fail anymore | 17:35 |
| niemeyer | cachio: Nice, so retrying is working as well | 17:40 |
| cachio | yes | 17:41 |
| Saviq | jdstrand: so devices.list http://paste.ubuntu.com/p/x6h58TrPPk/ does not have 188:0 in it, where would you say the missing piece is? udev rules? | 17:41 |
| mup | PR snapd#5281 opened: snap: reject more layout locations <Simple> <Created by zyga> <https://github.com/snapcore/snapd/pull/5281> | 17:47 |
| * zyga -> teak | 17:57 | |
| zyga | tea | 17:57 |
| zyga | Pharaoh_Atem: https://www.linux.com/learn/intro-to-linux/2018/5/get-started-snap-packages-linux :-) | 18:24 |
| zyga | nice | 18:24 |
| Pharaoh_Atem | well then | 18:26 |
| Pharaoh_Atem | that's surprising | 18:26 |
| zyga | on Fedora :) | 18:26 |
| cachio | niemeyer, with this error on spread I could test the garbage colletion very well | 18:38 |
| cachio | Currently I am running it | 18:38 |
| cachio | niemeyer, I am cleaning more than 400 machines | 18:43 |
| niemeyer | cachio: Wow | 19:01 |
| niemeyer | cachio: How come we have that many leftovers? | 19:02 |
| cachio | niemeyer, all the builds that failed in travis because of the issue left all the machines alive | 19:04 |
| cachio | and the testing I did to reproduce errors too | 19:04 |
| niemeyer | Ah, makes sense | 19:06 |
| * zyga breaks for some book reading | 19:20 | |
| mup | PR snapd#5263 closed: errtracker: do not send duplicated reports <Created by mvo5> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/5263> | 19:42 |
| cachio | Pharaoh_Atem, hey, did you see this one? | 19:51 |
| cachio | https://travis-ci.org/snapcore/snapd/builds/389407059#L1973 | 19:52 |
| Pharaoh_Atem | looks like someone needs to fix the sed command | 19:52 |
| Pharaoh_Atem | I think that was originally added by mvo | 19:52 |
| Pharaoh_Atem | also, bogus date should be fixed too | 19:53 |
| jdstrand | niemeyer: ok, I sketched out a pretty complete design that someone could run with here: https://forum.snapcraft.io/t/classic-confinement-request-communitheme-set-default/5146/24 | 20:00 |
| cachio | Pharaoh_Atem, ok, I'll try | 20:00 |
| jdstrand | niemeyer: please review and ack since I made a couple tweaks and thought about future iterations that might affect the yaml | 20:00 |
| mvo | zyga: did my answers to 5274 look reasonable? I whish there was a "stack this PR on top of the other" feature in GH, I have a nice cleanup pending on top of 5274 | 20:10 |
| jdstrand | Saviq: sorry, I was in a meeting. what is the output of: snap interfaces -i raw-usb? | 20:15 |
| zyga | Mvo: I didn’t check yet. My computer is occupied by the family | 20:25 |
| nacc | rbasak: fwiw, i thought i had a brilliant idea, that maybe we weren't specifying that pygit2 needed to be built/staged after libgpg-error, but afaict, pygit2 comes from git-ubuntu's dependencies, which is after devscripts which is after gnupg2 which is after libgpg-error. | 20:27 |
| nacc | rbasak: but i think that might be the way to debug it, if we can get a failure and success log | 20:27 |
| zyga | jdstrand: will alternatives allow coping of binaries out? | 20:29 |
| jdstrand | zyga: not by a strict definition of what you just said, but yes it allows confinement escape. the post discusses that | 20:29 |
| cachio | niemeyer, did you already updated spread on amazonaws? | 20:30 |
| zyga | Rather than escape I was wondering about just working outside of the namespace | 20:30 |
| cachio | niemeyer, some builds still failing with the spread issue | 20:30 |
| cachio | last error that I saw was 30 minutes ago | 20:31 |
| Saviq | jdstrand: http://paste.ubuntu.com/p/Zpq4mKT6vt/ | 20:31 |
| jdstrand | zyga: the mechanism would allow substituting, say, /usr/bin/vim, for something copied from the snap into /var/lib/snapd/alternatives. the process regulating the use of the interface would not | 20:31 |
| jdstrand | Saviq: ok, and what is in /etc/udev/rules.d/70-snap.subsurface.rules | 20:32 |
| zyga | I don’t follow how a binary copied out of a snap would operate | 20:32 |
| jdstrand | zyga: it wouldn't | 20:33 |
| zyga | Shared libraries, data, etc | 20:33 |
| jdstrand | zyga: we wouldn't allow it | 20:33 |
| zyga | Ah | 20:33 |
| zyga | So what would we allow | 20:33 |
| jdstrand | but the interface is general | 20:33 |
| zyga | I assume the copy is for a specific purpose | 20:33 |
| jdstrand | did you read the topic? | 20:33 |
| jdstrand | :) | 20:33 |
| zyga | On the forum | 20:33 |
| jdstrand | yeah | 20:33 |
| zyga | Yes, maybe misread or missed the point | 20:33 |
| jdstrand | it is prompted to change out the the gdm css theme file | 20:34 |
| zyga | Or probably just sleeping :-) | 20:34 |
| jdstrand | s/the the// | 20:34 |
| zyga | Yeah data would work fine | 20:34 |
| zyga | Btw | 20:34 |
| zyga | Could this be the general exports mechanism? | 20:34 |
| jdstrand | from a mount namespace perspective, yes, not for confinement escape. that is why it is manual | 20:35 |
| zyga | Eg export wallpapers/man pages/themes/other data | 20:35 |
| zyga | With alternatives on top | 20:35 |
| jdstrand | zyga: general exports> no, it is specifically for updating symlinks in the 'update-alternatives' system (see the man page if you are unfamiliar with it) | 20:35 |
| zyga | Right | 20:36 |
| jdstrand | zyga: I mean, maybe | 20:36 |
| zyga | Ok | 20:36 |
| jdstrand | it depends on how it is all put together. it might be good, it might not | 20:36 |
| Saviq | jdstrand: http://paste.ubuntu.com/p/bH6cXrmqFx/ | 20:36 |
| zyga | Btw: very nice write-up | 20:37 |
| jdstrand | zyga: alternatives is about files, not directories, so for a general export mechanism it wouldn't work well. for a handful of files, sure | 20:37 |
| zyga | I was thinking just about the copy part | 20:38 |
| zyga | And perhaps about /var/lib/snapd/e ports | 20:38 |
| jdstrand | Saviq: that looks correct. what happens if in one terminal you do: 'sudo udevadm monitor --subsystem-match=usb' and then you unplug and plug in the device | 20:39 |
| jdstrand | Saviq: then, give my the output of the monitor command and 'sudo udevadm info /dev/ttyUSB0' | 20:39 |
| jdstrand | zyga: the locations and dir structure in /var/lib/snapd/alternatives could change. I had to make that up as I wrote it cause I realized gdm might crash if it starts before the communitheme-set-defaults snap was mounted | 20:40 |
| zyga | Mmm | 20:41 |
| jdstrand | (we initially said that the alternative would point to /snap/name/current/... | 20:41 |
| jdstrand | ) | 20:41 |
| jdstrand | that won't work great for a number of things | 20:41 |
| zyga | I will watch this closely, I can help with the code as well | 20:42 |
| jdstrand | I don't know who will work on it. I just participated in the design. if I am to do it, would need to get it prioritized with stakeholders, etc, etc | 20:43 |
| jdstrand | but I tried to lay it all out so someone could run with it | 20:43 |
| jdstrand | but we need a final approval on the design/write-up | 20:43 |
| zyga | Agreed | 20:45 |
| Saviq | jdstrand: https://pastebin.ubuntu.com/p/34x4k6RjxW/ | 20:47 |
| jdstrand | zyga: I added a note to the writeup that only files are supported | 20:47 |
| zyga | Thanks | 20:47 |
| jdstrand | Saviq: oh, this is on bionic? | 20:47 |
| jdstrand | Saviq: I think you hit zyga's bug: https://github.com/systemd/systemd/issues/8221 | 20:48 |
| jdstrand | Saviq: I suspect this might work on a xenial system (or perhaps bionic with just the xenial kernel). can you file this against systemd in Ubuntu if it works on xenial and not bionic | 20:50 |
| zyga | I really wonder how something this huge went under everyone’s radar since 4.12 | 20:50 |
| jdstrand | zyga: it looks like it is subsystem dependent. eg, the input subsystem has no bind/unbind event if I plugin a joystick | 20:51 |
| zyga | Mm | 20:52 |
| zyga | I see a swarm of bins/unbind messages but I didn’t check the details | 20:52 |
| zyga | It may also explain why my .. battery doesn’t work | 20:52 |
| zyga | I have a battery that ought to charge a ThinkPad via usb-c pd | 20:53 |
| zyga | It supposedly works in windows | 20:53 |
| zyga | But when I plug it I see a bazillion of errors and bind/unbind calls | 20:53 |
| zyga | Oh well | 20:53 |
| zyga | Software | 20:54 |
| jdstrand | seems this would plausibly be it. the bug had a patch you could run locally if you were desperate :) | 20:54 |
| zyga | As long as we don’t have GNU/Linux toilets | 20:54 |
| zyga | Yeah, I think I’ll pass for now. I mostly work on a desktop | 20:54 |
| jdstrand | heh | 20:55 |
| jdstrand | we might be able to work around this in snapd based on https://github.com/freedesktop/ModemManager/commit/c07382a486f53e1b3cf729b41518d2a0ba528f5a | 20:57 |
| Saviq | jdstrand: oh! | 20:58 |
| * Saviq boots xenial up | 20:58 | |
| * jdstrand tries to find his usb serial | 20:59 | |
| mup | PR snapcraft#2143 closed: lifecycle: don't clean priming area if the snap is being tried <Created by kyrofa> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/2143> | 21:03 |
| cachio | niemeyer, https://travis-ci.org/snapcore/snapd/builds/389166279 | 21:12 |
| cachio | niemeyer, it is not happening in all the builds, it is more sporadic now | 21:12 |
| jdstrand | Saviq: actually, I figured it out | 21:23 |
| jdstrand | zyga: you may want to listen | 21:23 |
| zyga | I'm here actually | 21:23 |
| jdstrand | Saviq (cc zyga): snapd isn't affected by that systemd issue because while bind and unbind come through, snap-device-helper ignores them | 21:24 |
| jdstrand | Saviq (cc zyga): the problem is that udevadm info /dev/ttyUSB0 is reporting the subsystem as tty | 21:25 |
| zyga | do we need to update any bundled udev rules that don't involve just tagging via s-d-h ? | 21:25 |
| jdstrand | zyga: probably | 21:25 |
| jdstrand | someone should look at that. modem-manager was specifically affected | 21:25 |
| jdstrand | Saviq (cc zyga); but if I change the rules file in /etc/udev/rules.d/70... to have something like this, it works (on 18.04 and 4.15): | 21:26 |
| jdstrand | SUBSYSTEM=="usb", TAG+="snap_test-policy-app-consumer_raw-usb" | 21:27 |
| jdstrand | SUBSYSTEM=="tty", ENV{ID_BUS}=="usb", TAG+="snap_test-policy-app-consumer_raw-usb" | 21:27 |
| jdstrand | TAG=="snap_test-policy-app-consumer_raw-usb", RUN+="/usr/lib/snapd/snap-device-helper $env{ACTION} snap_test-policy-app-consumer_raw-usb $devpath $major:$minor" | 21:27 |
| jdstrand | Saviq (cc zyga): it is the second rule that I added | 21:27 |
| jdstrand | Saviq: I'll create a PR | 21:27 |
| zyga | hmmmm? | 21:31 |
| zyga | wait | 21:31 |
| zyga | so we append a tag (to a set of tags) | 21:31 |
| zyga | then if tag is .. what we added we run the helper | 21:31 |
| zyga | is this exploiting the fact that bind/unbind reset tags? | 21:31 |
| jdstrand | zyga: huh? | 21:31 |
| jdstrand | no | 21:31 |
| jdstrand | the subsystem doesn't match | 21:31 |
| jdstrand | the first is what we have now | 21:32 |
| zyga | this is what I don't understand https://www.irccloud.com/pastebin/vLHFJYLd/ | 21:32 |
| jdstrand | udevadmin info /dev/ttyUSB0 shows the subsystem as E: SUBSYSTEM=tty | 21:32 |
| jdstrand | therefore the first rule will never match | 21:32 |
| jdstrand | so we add a tty rule that only adds tty devices that are usb | 21:33 |
| jdstrand | zyga: what's the problem? | 21:33 |
| zyga | I think I just need sleep | 21:34 |
| zyga | :) | 21:34 |
| zyga | I will read it with fresh mind tomorrow | 21:34 |
| jdstrand | if subsystem is tty and the property ID_BUS is set to usb, tag the device | 21:34 |
| zyga | I perhaps need to see it on a wider screen | 21:34 |
| zyga | not on IRC | 21:34 |
| zyga | and check where the newlines are | 21:34 |
| jdstrand | well, you'll see it in a PR in a moment | 21:34 |
| zyga | as wrapping and udev using most horrid syntax makes it confusing | 21:34 |
| zyga | systemd guys, really, udev rules are the worst | 21:34 |
| mup | PR snapd#5282 opened: interfaces/raw-usb: also allow usb serial devices <Created by jdstrand> <https://github.com/snapcore/snapd/pull/5282> | 21:45 |
| jdstrand | Saviq, zyga: ^ | 21:45 |
| zyga | jdstrand: can you please review https://github.com/snapcore/snapd/pull/5281 :) | 21:54 |
| mup | PR #5281: snap: reject more layout locations <Simple> <Created by zyga> <https://github.com/snapcore/snapd/pull/5281> | 21:54 |
| zyga | it's just a rescue from a PR I closed | 21:54 |
| mup | PR snapcraft#2159 opened: many: extract lifecycle ordering into own module <Created by kyrofa> <https://github.com/snapcore/snapcraft/pull/2159> | 22:03 |
| niemeyer | cachio: No, haven't updated the Travis binaries.. will do so today still | 22:16 |
| mup | PR snapcraft#2156 closed: snap: use apt from the archive instead of compiling <Created by sergiusens> <Merged by kyrofa> <https://github.com/snapcore/snapcraft/pull/2156> | 22:21 |
| niemeyer | cachio: Updated.. please let me know how it goes | 23:19 |
| niemeyer | Just restarted that build as well | 23:20 |
| cachio | niemeyer, great, thanks | 23:39 |
| Matthew-S | Does anyone know how stage-packages are resolved? Do they refer to regular Ubuntu packages or something else? Thanks | 23:59 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!