| mborzecki | morning | 06:16 |
|---|---|---|
| zyga | good morning | 08:02 |
| zyga | mborzecki: a lower priority thing I sent last week: https://github.com/snapcore/snapd/pull/6251 - the refactoring you asked for | 08:02 |
| mup | PR #6251: cmd/snap-confine: refactor calling snapd tools into helper module <Created by zyga> <https://github.com/snapcore/snapd/pull/6251> | 08:02 |
| zyga | mborzecki: I'd like to mainly land the 2.36 branches: https://github.com/snapcore/snapd/pull/6245 and https://github.com/snapcore/snapd/pull/6235 | 08:03 |
| mup | PR #6245: interfaces/backends: detect too old apparmor_parser (2.36) <Created by zyga> <https://github.com/snapcore/snapd/pull/6245> | 08:03 |
| mup | PR #6235: overlord,apparmor: new syskey behaviour + non-ignored snap-confine profile errors (2.36) <Created by zyga> <https://github.com/snapcore/snapd/pull/6235> | 08:03 |
| zyga | I will focus on https://github.com/snapcore/snapd/pull/6190, sorting out my travel | 08:03 |
| mup | PR #6190: overlord/configstate,features: expose features to snapd tools <⛔ Blocked> <Created by zyga> <https://github.com/snapcore/snapd/pull/6190> | 08:03 |
| zyga | I can assist in any reviews | 08:03 |
| === pstolowski|afk is now known as pstolowski | ||
| pstolowski | mornings | 08:05 |
| zyga | hey pawel | 08:07 |
| zyga | I guess this morning is just us three, no? | 08:07 |
| pstolowski | i think so, yes | 08:15 |
| mborzecki | zyga: pstolowski: morning guys | 08:18 |
| zyga | :-) | 08:18 |
| mborzecki | the week of short standups | 08:18 |
| pstolowski | hey mborzecki | 08:19 |
| mborzecki | zyga: https://github.com/systemd/systemd/issues/10872#issuecomment-443504757 i'm building it right now | 08:19 |
| zyga | looking :) | 08:19 |
| pstolowski | yeah, i'll likely skip today's, my daughter has a dentist apointment at 15:20 | 08:19 |
| zyga | hmmm | 08:20 |
| zyga | pstolowski: ok | 08:20 |
| zyga | pstolowski: I'll bug you for some reviews | 08:20 |
| zyga | since it's just the three of us | 08:20 |
| pstolowski | sure | 08:21 |
| dot-tobias | good morning everyone | 08:40 |
| zyga | hey dot-tobias | 08:46 |
| pstolowski | zyga: thanks for the review of #6180 ; i'm not sure what to do about public SnapGlobal/Explicit tbh | 09:05 |
| mup | PR #6180: snap/info: bind global plugs/slots to implicit hooks <Created by stolowski> <https://github.com/snapcore/snapd/pull/6180> | 09:05 |
| zyga | pstolowski: I tried myself | 09:06 |
| zyga | and I found two ways out | 09:06 |
| zyga | maybe three if we rewrite tests :) | 09:06 |
| zyga | number one is to write a new DeepEquals that ignores private fields | 09:06 |
| zyga | I think it is interesting in principle | 09:06 |
| zyga | the second idea is to keep this as is | 09:06 |
| zyga | I think that's what we should do | 09:06 |
| zyga | I would change the name of the variables a little, I wasn't sure what to call them | 09:07 |
| zyga | ideas welcome | 09:07 |
| pstolowski | zyga: i don't think DeepEquals should ever ignore private feels, it feels like it's no longer deep equals ;) | 09:08 |
| pstolowski | *private fields | 09:08 |
| zyga | it would be some new comparator, sure | 09:08 |
| zyga | but it would help in cases like that | 09:09 |
| zyga | PublicEquals | 09:09 |
| zyga | or something ish | 09:09 |
| zyga | degville: another thing I found valuable today https://www.youtube.com/watch?v=vtIzMaLkCaM :-) | 09:11 |
| pstolowski | zyga: i'd keep it as is and continue in a followup | 09:11 |
| zyga | ^ recommend watching that | 09:11 |
| zyga | pstolowski: yes but please before landing, let's rename the new variables | 09:11 |
| zyga | they don't feel good (sorry for being vague) | 09:11 |
| pstolowski | zyga: i mean yes, sure, changing names is fine | 09:12 |
| zyga | maybe a 3 minute brainstorm with mborzecki could help | 09:12 |
| mborzecki | hm? | 09:12 |
| zyga | mborzecki: we need two variable names | 09:12 |
| zyga | mborzecki: one will tell you that a hook was explicitly defined in yaml, rather than being found in a specific directory on disk | 09:12 |
| zyga | mborzecki: another will tell you that a specific plug or slot affects all apps and hooks in a snap, rather than being associated with just a subset of them | 09:13 |
| zyga | mborzecki: how would you name those two? | 09:13 |
| dot-tobias | Is it possible to detect within my snap if a required interface is already connected? A service in my snap errors out right after the snap is installed, because I have to manually connect the interface. | 09:13 |
| mborzecki | zyga: is that in 6180? | 09:14 |
| pstolowski | mborzecki: yes | 09:15 |
| zyga | aha | 09:15 |
| zyga | dot-tobias: I beileve so | 09:17 |
| zyga | dot-tobias: but only in a hook | 09:17 |
| zyga | dot-tobias: I don't believe this is possible from a snap in general, that is, go and ask if something is connected yet | 09:17 |
| dot-tobias | zyga: Ok, I'll test the hook route. Thought about parsing snapctl interfaces -i <required-interface> my-snap-name from inside the service, but that seemed overkill. | 09:21 |
| zyga | dot-tobias: please ask pstolowski as well | 09:21 |
| zyga | I think this is useful to have | 09:21 |
| mborzecki | pstolowski: what exactly SnapGlobal means there? | 09:22 |
| pstolowski | mborzecki: it means that given plug/slot is defined at the top-level in the snap yaml | 09:24 |
| pstolowski | hmm perhaps TopLevel would be a better name? | 09:26 |
| zyga | I was thinking about scope but that doesn't work as well, I like TopLevel | 09:26 |
| mborzecki | TopLevel sounds ok, but I'm looking at the original code and it looks a bit confusing | 09:32 |
| mborzecki | pstolowski: do i read it right, if you have top level slots in the snap, and say have, one of the slots listed under an app too, then it's no longer automatically bound to all other apps/hooks? | 09:33 |
| zyga | mborzecki: yes, that's the semantics | 09:34 |
| zyga | mborzecki: plugs and slots are defined implictly by mentioning them in the abbreviated format | 09:34 |
| pstolowski | yep | 09:34 |
| zyga | mborzecki: such plugs and slots are bound to the apps and hooks that mention them | 09:34 |
| zyga | mborzecki: interfaces may be also defined or expanded in the top level | 09:34 |
| zyga | unless they are associated with a specific app or plug they become global/toplevel and apply to everything | 09:35 |
| zyga | maybe we should call it what it is | 09:35 |
| zyga | privilege spearation | 09:35 |
| zyga | privilege separation: true, applies to subset | 09:35 |
| zyga | no, applies to all executable code | 09:35 |
| pstolowski | zyga: what do you think about TopLevel name? | 09:52 |
| zyga | yeah, let's go with top level for now | 09:53 |
| zyga | Ideally we'd find a name that makes it useful and not just an implementation detail | 09:53 |
| zyga | but let's not make perfect the enemy of the good | 09:53 |
| mborzecki | enough to move forward | 09:53 |
| zyga | https://github.com/snapcore/snapd/pull/6253 needs a 2nd review | 10:03 |
| mup | PR #6253: Members of canonical LP group should pass CLA check <Created by kenvandine> <https://github.com/snapcore/snapd/pull/6253> | 10:03 |
| pstolowski | zyga: thanks | 10:23 |
| zyga | guys, quick question | 10:31 |
| zyga | I'm working on https://github.com/snapcore/snapd/pull/6190/files | 10:32 |
| mup | PR #6190: overlord/configstate,features: expose features to snapd tools <⛔ Blocked> <Created by zyga> <https://github.com/snapcore/snapd/pull/6190> | 10:32 |
| zyga | there are some low hanging fruit inside | 10:32 |
| zyga | for instance stuff like https://github.com/snapcore/snapd/pull/6190/commits/f8f2f3b389b920f299f8994a7e4fb96a02c14a19 | 10:32 |
| zyga | shall I pop that out to a new branch? | 10:32 |
| mborzecki | zyga: yeah, looks useful | 10:40 |
| zyga | mborzecki: in that case -> https://github.com/snapcore/snapd/pull/6255 | 10:57 |
| mup | PR #6255: testutil: add File{Present,Absent} checkers <Simple 😃> <Created by zyga> <https://github.com/snapcore/snapd/pull/6255> | 10:57 |
| mup | PR snapd#6255 opened: testutil: add File{Present,Absent} checkers <Simple 😃> <Created by zyga> <https://github.com/snapcore/snapd/pull/6255> | 10:58 |
| mborzecki | zyga: i think the fontconfig fix is insufficient for fedora | 11:02 |
| zyga | oh? | 11:02 |
| zyga | tell more please | 11:02 |
| zyga | it made positive effect on all suse versions I tried | 11:02 |
| mborzecki | zyga: they use --with-cache-dir=/usr/lib/fontconfig/cache | 11:02 |
| zyga | (42.3, 15 and tw) | 11:02 |
| zyga | heh | 11:02 |
| zyga | is that when we tell mvo on monday ;) | 11:03 |
| zyga | why /usr/lib?!? | 11:03 |
| mborzecki | zyga: beats me | 11:03 |
| zyga | file a bug please | 11:03 |
| zyga | not on fedora | 11:03 |
| zyga | on snapd for now | 11:03 |
| mborzecki | i was chasing down that denial, and couldn't trigger it on f29 cloud image, so went on digging and found this :/ arch uses /var/cache/fontconfig fwik | 11:04 |
| zyga | brb, let me make coffee | 11:04 |
| zyga | mborzecki: desktop is such a fractured thing | 11:04 |
| zyga | so wait | 11:04 |
| zyga | on fedora /usr/lib/fonconfig/cache | 11:04 |
| zyga | are files there written at runtime | 11:04 |
| zyga | or shipped via packages? | 11:04 |
| zyga | brb | 11:04 |
| mborzecki | written when fc-cache is invoked | 11:05 |
| mborzecki | also, fc-cache appears to be tricky for i686 and x86_64, on fedora fc-cache is a script that calls fc-cache-32 and fc-cache-64 | 11:06 |
| mborzecki | i recall someone mentioning that the cache files are actually a memory dump of a struct or somesuch | 11:06 |
| mborzecki | yeah, fontconfig.i686 ships fc-cache-32 | 11:09 |
| mborzecki | maybe we need a helper to run these tools after all | 11:10 |
| zyga | yes, they are | 11:10 |
| zyga | I think | 11:10 |
| zyga | instead of shipping our own builds of fc-cache | 11:10 |
| zyga | we should instead run the cache from the distro | 11:10 |
| zyga | and only do this for classic confined snap | 11:10 |
| zyga | for strictly confined snaps we should do what we did now | 11:10 |
| zyga | that is, provide our own cache | 11:10 |
| zyga | but the approach for strict and classic must differ | 11:10 |
| mborzecki | zyga: i think the trouble is that the distro may not ship fc-cache for particular fontconfig version in the core snap | 11:18 |
| zyga | mborzecki: but that version is rarely used in practice | 11:18 |
| zyga | otherwise you would see an improvement | 11:19 |
| zyga | right? | 11:19 |
| zyga | you are really seeing a snap using your fontconfig | 11:19 |
| zyga | straight from the distro | 11:19 |
| === geodb27_ is now known as geodb27 | ||
| mborzecki | zyga: my cache, but not my lib version (unless someone tweaked how the snap is built) | 11:29 |
| zyga | so wait | 11:29 |
| zyga | how can both be true | 11:29 |
| zyga | if a program uses fontconfig | 11:29 |
| zyga | it is either from core | 11:29 |
| zyga | from the snap itself (equivalent) | 11:29 |
| zyga | or from the native host libs | 11:30 |
| mborzecki | zyga: yeah, so unless tweaked, the snap ends up with whatever libfontconfig was in 16.04, right? that's for both confined and classic | 11:31 |
| zyga | of 18.04 | 11:31 |
| zyga | yeah | 11:31 |
| zyga | but classic snaps can be hand made | 11:31 |
| zyga | can have any layout inside | 11:31 |
| mborzecki | yes, that's what i mean by tweaking the build | 11:31 |
| zyga | including vanilla linker | 11:31 |
| zyga | so | 11:32 |
| zyga | my point is that there's either libs from the host used | 11:32 |
| zyga | which imply *cache* from the host | 11:32 |
| zyga | or libs from the snap/core | 11:32 |
| zyga | which imply cache compatible with that | 11:32 |
| zyga | there's no third cache | 11:32 |
| mborzecki | aah, i see what you mean, the core lib was rebuilt with cache in /var/cache/fontconfig | 11:32 |
| zyga | mborzecki, pstolowski: could you please review https://github.com/snapcore/snapd/pull/6255 | 11:51 |
| mup | PR #6255: testutil: add File{Present,Absent} checkers <Simple 😃> <Created by zyga> <https://github.com/snapcore/snapd/pull/6255> | 11:51 |
| pstolowski | zyga: nice! that will be pretty useful, looking | 11:51 |
| zyga | pstolowski: and if you can, I need 2nd review on https://github.com/snapcore/snapd/pull/6244 | 11:52 |
| mup | PR #6244: release: detect too old apparmor_parser <Created by zyga> <https://github.com/snapcore/snapd/pull/6244> | 11:52 |
| dot-tobias | pstolowski: Is it possible to detect within my snap's application code if a required interface is already connected? snapctl only supports config and service related commands | 12:05 |
| pstolowski | dot-tobias: hmm, not really, you could store this fact somewhere in interface hooks as connections happen (but then you also need to do the opposite on disconnect) | 12:09 |
| pstolowski | degville: can you remind me where was the doc for interface hooks? | 12:11 |
| degville | pstolowski: https://forum.snapcraft.io/t/interface-hooks/8214 | 12:12 |
| pstolowski | dot-tobias: ^ | 12:12 |
| dot-tobias | pstolowski: Thanks! Sounds like a good solution for now. I just want to prevent log bloat since my snap's services try to run various interface-dependent commands on app start, but adding exception handling just for this one moment where the snap's plugs are not yet connected is a bit much. | 12:13 |
| pstolowski | degville: thanks! is this going to be merged with the main docs? | 12:13 |
| pstolowski | dot-tobias: obviously, if you could simply probe and it's cheap, than it's an option too | 12:13 |
| degville | pstolowski: it's currently linked to from the general hooks page (https://docs.snapcraft.io/supported-snap-hooks/3795) | 12:14 |
| pstolowski | ack, thanks | 12:17 |
| zyga | re | 12:35 |
| zyga | back to coding | 12:35 |
| mborzecki | pstolowski: can you take a look at https://github.com/snapcore/snapd/pull/6246 ? | 12:58 |
| mup | PR #6246: spread: show AVC audits when debugging, start auditd on Fedora <Simple 😃> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/6246> | 12:58 |
| mborzecki | off to pick up the kids | 13:10 |
| zyga | mborzecki, pstolowski: can you please review the 2.36 PRs https://github.com/snapcore/snapd/milestone/21 | 13:15 |
| mup | PR snapd#6253 closed: Members of canonical LP group should pass CLA check <Created by kenvandine> <Merged by zyga> <https://github.com/snapcore/snapd/pull/6253> | 13:16 |
| zyga | brb | 13:43 |
| pstolowski | zyga, mborzecki i'm gonna miss the standup | 13:55 |
| mborzecki | ack | 13:56 |
| mborzecki | zyga: about cgroups v1 https://github.com/systemd/systemd/issues/10969#issuecomment-442357207 ;) | 14:18 |
| zyga | see ;-) | 14:23 |
| mborzecki | zyga: https://github.com/snapcore/snapd/pull/6185 has 2 +1s, i'm thinking squash merge? | 14:23 |
| mup | PR #6185: snap: add new `snap run --trace-exec` call <Performance 🚀> <Created by mvo5> <https://github.com/snapcore/snapd/pull/6185> | 14:23 |
| zyga | looking now | 14:23 |
| zyga | yes | 14:24 |
| zyga | + on squashing it | 14:24 |
| mup | PR snapd#6185 closed: snap: add new `snap run --trace-exec` call <Performance 🚀> <Created by mvo5> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/6185> | 14:27 |
| mborzecki | zyga: i'll open a 2.36 PR | 14:28 |
| zyga | wait | 14:29 |
| zyga | please merge my two PRs | 14:29 |
| zyga | otherwise 2.36 is red | 14:29 |
| mup | PR snapd#6256 opened: snap: add new `snap run --trace-exec` call (2.36) <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/6256> | 14:30 |
| mborzecki | aah ok, looking | 14:30 |
| mborzecki | zyga: #6235 has conflicts now | 14:32 |
| mup | PR #6235: overlord,apparmor: new syskey behaviour + non-ignored snap-confine profile errors (2.36) <Created by zyga> <https://github.com/snapcore/snapd/pull/6235> | 14:32 |
| mup | PR snapd#6245 closed: interfaces/backends: detect too old apparmor_parser (2.36) <Created by zyga> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/6245> | 14:33 |
| zyga | grumpy about random failures | 14:45 |
| zyga | https://github.com/snapcore/snapd/pull/6255 red for 3rd time in a row | 14:46 |
| mup | PR #6255: testutil: add File{Present,Absent} checkers <Simple 😃> <Created by zyga> <https://github.com/snapcore/snapd/pull/6255> | 14:46 |
| mborzecki | ok, left a note under https://github.com/systemd/systemd/issues/10872 | 14:51 |
| zyga | mmm | 14:52 |
| zyga | hmm | 14:52 |
| zyga | so does it work? | 14:52 |
| zyga | mborzecki: btw, I need to show you that part in libmount where I think the bug is as well, there's a lock missing | 14:52 |
| zyga | maybe we are seeing a pair of bugs | 14:52 |
| zyga | mborzecki: will you be around today or are you wrapping up now? | 14:53 |
| mborzecki | zyga: you think bugs are like sith? | 14:53 |
| zyga | mborzecki: like binary star systems ;) | 14:53 |
| zyga | they always come in piarr | 14:53 |
| zyga | *pairs | 14:53 |
| zyga | mborzecki: I need to break for lunch now | 14:53 |
| zyga | mborzecki: will you be here in an hour/ | 14:53 |
| mborzecki | i'll probably be around later too, need to take kids to the scouts ~5pm | 14:54 |
| zyga | ok, ttyl | 14:55 |
| kenvandine | zyga: now that my cla check PR was merged the other PR failed cla-check with a KeyError | 14:59 |
| kenvandine | but the cla_check PR passed CI... is the LP API just flaky? | 14:59 |
| zyga | kenvandine: dunno | 15:14 |
| zyga | kenvandine: can you run the check locally? | 15:14 |
| zyga | or handle the api key | 15:14 |
| zyga | er | 15:14 |
| zyga | or handle the key error | 15:14 |
| zyga | or maybe there's a cache a | 15:14 |
| kenvandine | zyga: it works locally :/ | 15:29 |
| * cachio lunch | 15:33 | |
| zyga | hmm | 15:33 |
| zyga | eh | 15:56 |
| zyga | more failures | 15:56 |
| zyga | need coffee | 16:03 |
| zyga | going to make coffee :) | 16:09 |
| Son_Goku | mborzecki: https://twitter.com/Arrfab/status/1069623805520355329 | 16:20 |
| cachio | mborzecki, zyga should I create a new gce image for centos? | 16:24 |
| cachio | or we are ok | 16:24 |
| mborzecki | Son_Goku: yay! | 16:26 |
| mborzecki | cachio: yes, please | 16:26 |
| cachio | mborzecki, ok | 16:28 |
| mborzecki | cachio: i think it's best if you create a new image under a separate name and only switch if the whole suite passes | 16:28 |
| mborzecki | cachio: if there are issues with the spread run, i can take a look tomorrow and fix stuff :) | 16:29 |
| zyga | re | 16:46 |
| zyga | sorry, had to do homework with kids | 16:46 |
| zyga | back now | 16:46 |
| zyga | cachio: I think a new image will save us time on upgrades so yeah | 16:46 |
| zyga | I agrew with what mborzecki said :) | 16:46 |
| zyga | FUUUK | 16:47 |
| zyga | why does 6255 fail all the time | 16:47 |
| cachio | zyga, mborzecki ok, I'll create the new image and make some runs to validate it | 16:49 |
| zyga | thanks | 16:49 |
| roadmr | zyga: because... if you decompose the 2 as 1 + 1, and do 6(5+1)(5+1) you get 666!!!!!!! evil! | 16:49 |
| roadmr | that made no sense at all, sorry | 16:49 |
| zyga | roadmr: ha, I wound not be suprirsed by now :) | 16:50 |
| zyga | I hate that our test suite has some flaky components | 16:50 |
| zyga | and as we add tests and distributions to run against | 16:50 |
| zyga | the chance of landing a trivial branch is very low | 16:50 |
| zyga | and any attempt takes an hour | 16:50 |
| zyga | cannot have velocity with roadblocks like that :/ | 16:50 |
| zyga | https://people.neilon.software/ :D | 16:54 |
| roadmr | zyga: can you find yourself there? I did find myself :/ | 16:56 |
| zyga | I'd be the extreme underestimator | 16:56 |
| roadmr | hehe :) the icons are fantastic btw | 16:57 |
| sergiusens | zyga: you should have written better tests! :-P | 17:01 |
| zyga | sergiusens: our tests are notoriously flaky :/ | 17:02 |
| zyga | many are racy | 17:02 |
| zyga | some leak stuff but we have no way to tell yet | 17:02 |
| zyga | but at this scale | 17:02 |
| zyga | it's not even that | 17:02 |
| sergiusens | we bit the bullet and stopped development for a while to get all that sorted | 17:02 |
| zyga | simple things like archive issues | 17:02 |
| sergiusens | yes, leaking and test cross contamination is hard | 17:02 |
| zyga | sergiusens: we wanted to do that for a few months but it is unreasonable to do | 17:03 |
| sergiusens | at least, if you want speed | 17:03 |
| zyga | sergiusens: I want speed, I get asked to focus on a feature I need to do | 17:03 |
| zyga | so I do what I'm told | 17:03 |
| zyga | and restart those nasty tests | 17:03 |
| zyga | sergiusens: I don't disagree | 17:03 |
| zyga | I just don't get that choice | 17:03 |
| sergiusens | we added it as a roadmap item, "migrate to spread", so it was accounted for, we still have a list of things that did not make it, but we are in a better place now | 17:04 |
| zyga | sergiusens: I hope next cycle we can do that | 17:05 |
| zyga | spread doesn't work if it must be 100% green across ~5K tests that can almsot each fail on network | 17:05 |
| zyga | we either need a smarter UI / runner that can retrigger a single test | 17:05 |
| zyga | or change how we land things | 17:05 |
| sergiusens | zyga: you should do it during the year crossing cycle, it is shorter and it is nicer to to these sort of things as the end of year holidays come closer | 17:05 |
| zyga | last two weeks was "red in 2.36" mode | 17:06 |
| zyga | where nothing in a release branch could land | 17:06 |
| zyga | sergiusens: this cycle is packed | 17:06 |
| zyga | won't queeze more | 17:06 |
| zyga | we can do some perf work on snapd though | 17:06 |
| zyga | that's valuable | 17:06 |
| zyga | but equally frustrating to land :) | 17:06 |
| sergiusens | zyga: next year... | 17:06 |
| zyga | unless I hack on spread and snapd during weekends and holidays | 17:06 |
| zyga | next year, yeah | 17:06 |
| zyga | I mean, we always fix the test suite | 17:07 |
| zyga | but it's not really done with the effort required | 17:07 |
| zyga | mainly about making the failure causes fixed | 17:07 |
| zyga | (leaking tests) | 17:07 |
| zyga | and racy tests | 17:07 |
| zyga | we have so many racy tests that it's not fun | 17:07 |
| zyga | this with the requirement to get two reviews means that stuff lingers | 17:08 |
| zyga | first until green | 17:08 |
| zyga | then on iteration | 17:08 |
| zyga | then on making that green | 17:08 |
| zyga | then on more reviews | 17:08 |
| jdstrand | roadmr: hi! when convenient, can you pull in r1165? (not urgent) | 17:15 |
| zyga | mborzecki: if still around, could you review https://github.com/snapcore/snapd/pull/6251 | 17:16 |
| mup | PR #6251: cmd/snap-confine: refactor calling snapd tools into helper module <Created by zyga> <https://github.com/snapcore/snapd/pull/6251> | 17:16 |
| roadmr | jdstrand: sure thing! because $REASONS we're on a merging moratorium until tomorrow but I'll merge that tomorrow | 17:16 |
| jdstrand | roadmr: np, thanks again | 17:16 |
| roadmr | happy to help :) | 17:16 |
| zyga | pstolowski: if still around, can you please review https://github.com/snapcore/snapd/pull/6235 | 17:18 |
| mup | PR #6235: overlord,apparmor: new syskey behaviour + non-ignored snap-confine profile errors (2.36) <Created by zyga> <https://github.com/snapcore/snapd/pull/6235> | 17:18 |
| pstolowski | zyga: looking | 17:18 |
| zyga | thank you! | 17:18 |
| zyga | pstolowski: FYI: the master version in https://github.com/snapcore/snapd/pull/6233 has more tests | 17:19 |
| mup | PR #6233: overlord: don't write system key if security setup fails <Created by zyga> <Merged by zyga> <https://github.com/snapcore/snapd/pull/6233> | 17:20 |
| zyga | jdstrand: hey, do you have time to review https://github.com/snapcore/snapd/pull/6244 | 17:24 |
| mup | PR #6244: release: detect too old apparmor_parser <Created by zyga> <https://github.com/snapcore/snapd/pull/6244> | 17:24 |
| zyga | we're very short on reviewers this week | 17:24 |
| jdstrand | seb128: fyi, I think you might find this helpful: https://forum.snapcraft.io/t/notifications-for-out-of-date-stage-packages/5161/7 | 17:35 |
| pstolowski | zyga: i'm slightly confused by the delta between #6233 and #6235 - e.g. writeSystemKey vs shouldWriteSystemKey, an extensive comment missing in 6235> | 17:35 |
| mup | PR #6233: overlord: don't write system key if security setup fails <Created by zyga> <Merged by zyga> <https://github.com/snapcore/snapd/pull/6233> | 17:35 |
| mup | PR #6235: overlord,apparmor: new syskey behaviour + non-ignored snap-confine profile errors (2.36) <Created by zyga> <https://github.com/snapcore/snapd/pull/6235> | 17:35 |
| mup | PR snapd#6255 closed: testutil: add File{Present,Absent} checkers <Simple 😃> <Created by zyga> <Merged by zyga> <https://github.com/snapcore/snapd/pull/6255> | 17:35 |
| zyga | pstolowski: yeah, some things changed as I enabled testing | 17:36 |
| zyga | I can backport the whole lot, just the 2.36 branch is the raw essence of the thing | 17:36 |
| zyga | and the master branch has more stuff to run unit and spread tests | 17:36 |
| pstolowski | zyga: i see, ok | 17:37 |
| pstolowski | i thought you forgot to cherry pick something | 17:37 |
| zyga | since it is just tests I could cherry pick more | 17:37 |
| zyga | but I didn't get reviews :) | 17:38 |
| zyga | pstolowski: not sure if you want to but https://github.com/snapcore/snapd/pull/6257 is technically simple | 17:49 |
| mup | PR #6257: testutils: split checkers, tweak tests <Created by zyga> <https://github.com/snapcore/snapd/pull/6257> | 17:49 |
| zyga | but I didn't mark it as such snice it's a +1332,-994 change | 17:49 |
| mup | PR snapd#6257 opened: testutils: split checkers, tweak tests <Created by zyga> <https://github.com/snapcore/snapd/pull/6257> | 17:49 |
| pstolowski | zyga: will check it tomorrow; 1 question to the earlier PR | 17:50 |
| zyga | looking | 17:50 |
| zyga | thanks, replied | 17:51 |
| zyga | pstolowski: running interfaces-many test on my laptop makes me want to optimize security setup | 17:53 |
| zyga | all that fan noise :) | 17:53 |
| === pstolowski is now known as pstolowski|afk | ||
| * cachio afk | 18:47 | |
| mup | PR snapcraft#2397 closed: cmake plugin: use native primitives <Created by sergiusens> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/2397> | 18:56 |
| seb128 | jdstrand, thx | 19:12 |
| kyrofa | popey, jdstrand: can someone remind me why we require desktop files if using the x11 interface? | 20:11 |
| kyrofa | None of the ROS GUI tools have desktop files, or even icons of which I'm aware, and essentially never make sense to run standalone | 20:12 |
| kyrofa | They're typically brought up from the CLI | 20:13 |
| kyrofa | I'm getting emails from people saying "Dude, I don't even know what a desktop file is, how am I supposed to include one?" | 20:16 |
| jdstrand | kyrofa: there is a mechanism to whitelist that, but the reason why we don't by default is because with (at least) unity7, not shipping a desktop file means the user looks in the dash for the application, then launches it from the dash. now it is in the launcher and the application pins to the launcher and unity7 rewrites the desktop file without running through snap run, therefore unconfined | 20:16 |
| jdstrand | s/and the application/and the user/ | 20:16 |
| jdstrand | kyrofa: https://bugs.launchpad.net/snappy/+bug/1643910 | 20:20 |
| mup | Bug #1643910: BAMF_DESKTOP_FILE_HINT not set in correct place for unity7 <Snappy:Triaged> <bamf (Ubuntu):Triaged by 3v1n0> <https://launchpad.net/bugs/1643910> | 20:20 |
| kyrofa | jdstrand, wait... if one doesn't ship a desktop file, it _doesn't_ show up in the dash, right? | 20:21 |
| jdstrand | kyrofa: actually, I think I referred to the wrong issue as to why we do it (though I'm glad I remembered that one so I could ping in the bug) | 20:23 |
| jdstrand | gimme a sec | 20:23 |
| kyrofa | Alright | 20:28 |
| jdstrand | kyrofa: ok, right, this *is* the bug, but there are two issues in that bug but I only remembered the one, which wasn't the one for the having the check | 20:34 |
| jdstrand | kyrofa: so, forget the dash, cause, yes, you need a desktop file for that | 20:34 |
| jdstrand | kyrofa: *but* if you launch a program that uses X that doesn't ship a desktop file, BMAF (BAMF Application Matching Framework) tries to be smart and find the application that is running | 20:35 |
| jdstrand | kyrofa: that allows it to have something in the launcher, which can then be pinned | 20:36 |
| jdstrand | kyrofa: which then ends up with the wrong entry | 20:36 |
| kyrofa | jdstrand, that seems like it warrants a warning, not an error, no? | 20:36 |
| jdstrand | kyrofa: eg, xmessage does not have a desktop file. if you launch it under unity7, it shows up in the launcher. if you pin that, the desktop file gets written out to .local/... | 20:37 |
| kyrofa | I doubt just pinning what I assume is a direct path to the binary is going to work given the required environment | 20:37 |
| jdstrand | kyrofa: if this thing is a snap, it gets writeen out to .local/... with the wrong Exec= line that makes it run unconfined | 20:37 |
| jdstrand | kyrofa: it is a warning | 20:37 |
| kyrofa | jdstrand, it isn't an error popping it into manual review? | 20:38 |
| jdstrand | kyrofa: but in practice, in makes no difference because warnings block manual reviews | 20:38 |
| kyrofa | Ah | 20:38 |
| jdstrand | err | 20:38 |
| jdstrand | cause manual review | 20:38 |
| kyrofa | Is that intended? | 20:38 |
| jdstrand | the warning bit? well, yes, it is intended but it is long known things could be better. the problem is, if it doesn't block at all, no one will see it | 20:39 |
| jdstrand | there is a mechanism to whitelist things | 20:39 |
| kyrofa | jdstrand, well, the way it is today I have someone who has given up on snaps because it was too hard to even get something into the store | 20:39 |
| kyrofa | There must be some middle ground | 20:39 |
| jdstrand | they stopped using snaps because of *this* issue? | 20:40 |
| jdstrand | there is an easy workaround. provide a desktop file. there is an easy way to get whitelisted-- respond via the store emails or bring it up in the forum | 20:40 |
| kyrofa | This one and a similar issue with using usb-raw making it impossible to get to stable | 20:41 |
| jdstrand | kyrofa: alternatively, unity7 could be fixed, then the check can go away | 20:41 |
| jdstrand | kyrofa: no one communicated to me that someone was flailing due to this issue. I guess I can update the review text to mention bringing it up in the forum | 20:42 |
| jdstrand | kyrofa: as for usb-raw, well, that is a hotplug question and done on a case by case basis, but pstolowski|afk is actively working on that | 20:43 |
| jdstrand | kyrofa: I suggest commenting in https://bugs.launchpad.net/snappy/+bug/1643910 that this needs to be fixed and escalating through the desktop team | 20:43 |
| mup | Bug #1643910: BAMF_DESKTOP_FILE_HINT not set in correct place for unity7 <Snappy:Triaged> <bamf (Ubuntu):Triaged by 3v1n0> <https://launchpad.net/bugs/1643910> | 20:43 |
| kyrofa | Can you explain why usb-raw isn't just not autoconnected? | 20:44 |
| jdstrand | kyrofa: because it gives access to all usb devices on the system. that is rarely what an application requires | 20:44 |
| kyrofa | Well, sure, but that seems like a reason just to deny autoconnection, no? | 20:44 |
| jdstrand | it need a ttyUSB, or a mouse, or something. not everything | 20:44 |
| jdstrand | kyrofa: ok, we are talking about different things | 20:45 |
| jdstrand | kyrofa: $ snap debug get-base-declaration very clearly shows it is only denying auto-connection | 20:46 |
| jdstrand | raw-usb: | 20:46 |
| jdstrand | allow-installation: | 20:46 |
| jdstrand | slot-snap-type: | 20:46 |
| jdstrand | - core | 20:46 |
| jdstrand | deny-auto-connection: true | 20:46 |
| jdstrand | kyrofa: ie, that ^ does what you are asking and has been that way for since forever | 20:46 |
| kyrofa | jdstrand, interesting, my apologies, indeed, that does indeed do what I'm asking. This email says he couldn't move it to a stable channel, which I just tested is not the case. Sounds like a grade issue instead | 20:49 |
| jdstrand | kyrofa: I'll adjust the message for the desktop file, but if this is a stumbling block or you feel it should be escalated, please comment in the bug | 20:51 |
| jdstrand | I'd loave to see this fixed in bamf | 20:51 |
| kyrofa | jdstrand, that would be great, thanks. Any idea if we have docs for how to properly write/integrate a desktop file that we could also link to? People who hit this may have no idea what a desktop file is, how to write one, or how to get it properly in a snap | 20:52 |
| jdstrand | kyrofa: it is in the description that is part of the review message: If using snapcraft, please see https://docs.snapcraft.io/snapcraft-app-and-service-metadata/8335#fixed-assets. Otherwise, please provide a desktop file in meta/gui/*.desktop (it should reference one of the 'apps' from your snapcraft/snap.yaml). | 20:55 |
| kyrofa | Ah, okay | 21:02 |
| kyrofa | jdstrand, speaking of links, there was something else I wanted to talk to you about | 21:02 |
| kyrofa | jdstrand, have you ever used shellcheck before? | 21:02 |
| jdstrand | kyrofa: yes | 21:03 |
| kyrofa | jdstrand, you know how it provides an error code for every issue which has a wiki entry associated with it? | 21:04 |
| kyrofa | jdstrand, think it would be useful for snappy-debug to do something similar? | 21:04 |
| jdstrand | yeah. snappy-debug needs a lot of resources put on it | 21:05 |
| kyrofa | Oh don't get me wrong, it's super useful | 21:06 |
| jdstrand | as it stands, it has had no formal design or resources put on it | 21:06 |
| kyrofa | Yeah fair enough | 21:06 |
| jdstrand | I have cards and work items for it, but it is all way down the list after approved stuff and things for the snapd, et all | 21:06 |
| jdstrand | al* | 21:06 |
| kyrofa | Makes sense | 21:06 |
| jdstrand | kyrofa: no, I know what you mean. it's handy. I just want you to know that I know it needs love :) | 21:07 |
| jdstrand | I try to make sure it continues to be handy | 21:07 |
| kyrofa | That's appreciated. Do the review tools have design docs and time assigned to them? | 21:08 |
| jdstrand | hopefully I can get more time to look at it. patches welcome if you or anyone else wants to work on it (though, you can see how many resources are put on it-- it was pretty hastily thrown together) | 21:08 |
| kyrofa | Yeah my next suggestion was going to be: do they have their own issue trackers? We all benefit from these tools, I see no reason we shouldn't all be contributing to them | 21:09 |
| jdstrand | kyrofa: the review-tools do not have design docs. there is understood maintenance time on them | 21:10 |
| jdstrand | kyrofa: they have the ability to trump other work though since the world can burn if they don't get updated :) | 21:11 |
| kyrofa | Ha! Yes indeed | 21:11 |
| jdstrand | kyrofa: they are both proper rojects in LP | 21:12 |
| jdstrand | projects even | 21:12 |
| jdstrand | https://launchpad.net/review-tools | 21:12 |
| jdstrand | https://launchpad.net/~snappy-dev/snappy-hub/snappy-debug | 21:13 |
| jdstrand | I guess snappy-debug doesn't have a bug tracker (I thought the parent project did) | 21:14 |
| kyrofa | jdstrand, yeah I was just about to ask about that. What is snappy-hub? Historical? | 21:14 |
| jdstrand | if I can ever get time to update it, I would do a rewrite | 21:14 |
| jdstrand | kyrofa: it comes from the 15.04 days | 21:14 |
| diddledan | who do I have to kick to get /dev/shm to be allowed as a layouts target (I want to mount my own tmpfs because the strict naming requirements are impossible to work around for some things) | 21:26 |
| jdstrand | diddledan: zyga, but note that it would be /dev/shm or /run/shm depending on the system | 21:28 |
| zyga | hmm | 21:28 |
| jdstrand | diddledan: I also have a todo to look at LD_PRELOAD for that | 21:28 |
| zyga | layouts cannot work with /dev | 21:28 |
| zyga | layouts create a read-only snapshot | 21:28 |
| zyga | well | 21:28 |
| zyga | I should check some things | 21:28 |
| zyga | but unless we mount /dev ourselves (I don't think we do) | 21:29 |
| zyga | and we actually share /dev/ from host | 21:29 |
| jdstrand | zyga: recall /dev/shm is a directory (just for your investigation) | 21:29 |
| zyga | this will not be easy (or doable using layoutts) | 21:29 |
| zyga | aha | 21:29 |
| zyga | sorry, that's important (it's late) | 21:29 |
| zyga | it does simplifiy things significantly | 21:29 |
| * jdstrand nods | 21:29 | |
| zyga | but does making /dev/shm private via layouts break IPC with host services? | 21:29 |
| zyga | in any case | 21:29 |
| zyga | diddledan: let's find a bug about this | 21:30 |
| kyrofa | Depends on how the IPC is implemented, but that would be the snap's fault anyway, no? | 21:30 |
| zyga | and ping me with a number | 21:30 |
| zyga | kyrofa: ish, it depends if we can practically do it or not | 21:30 |
| jdstrand | I think for some things it would work and others not | 21:31 |
| zyga | indeed | 21:31 |
| zyga | but that is something that requires investigation | 21:31 |
| zyga | since it's past 10PM I will ack the issue, happily work on it but defer till morning :) | 21:31 |
| diddledan | :-) | 21:32 |
| diddledan | I'll see if I can find a bug | 21:32 |
| diddledan | it's a known issue affecting python-multiprocessing for what it's worth | 21:33 |
| diddledan | specifically the python multiprocessing module hasn't been tamed as yet by the snapcraft-preload so anything that needs the module can't run as a snap | 21:34 |
| diddledan | as a strictly confined snap* | 21:34 |
| jdstrand | diddledan: that was what I was going to look at | 21:36 |
| jdstrand | it's python2 only, iirc (ie, there is some way to make python3 work) | 21:37 |
| diddledan | oh? if there's a way to get python3 working that might unblock mycroft | 21:37 |
| jdstrand | there's the forum topic. let me find it | 21:38 |
| jdstrand | this is the topic: https://forum.snapcraft.io/t/python-multiprocessing-sem-open-blocked-in-strict-mode/962 | 21:39 |
| diddledan | yeah, there's a reply that says the workaround didn't work | 21:39 |
| * jdstrand notes this is an issue with other app stores | 21:39 | |
| jdstrand | https://bugs.python.org/issue19478 | 21:39 |
| jdstrand | diddledan: see that ^ | 21:40 |
| * diddledan clicky | 21:40 | |
| jdstrand | "Although it is undocumented, in python 3.4 you can control the prefix used by doing | 21:40 |
| jdstrand | multiprocessing.current_process()._config['semprefix'] = 'myprefix'" | 21:40 |
| jdstrand | I'll take a note to add that to snappy-debug | 21:40 |
| diddledan | ok, I've set a mycroft build going to see if that can fix me up | 21:54 |
| jdstrand | kyrofa: actually, I looked into this and bamf received an update via a different bug | 22:06 |
| jdstrand | kyrofa: I can now make the test an info | 22:06 |
| jdstrand | roadmr: hey, can you make that r1167 instead? | 22:45 |
| roadmr | jdstrand: sure thing! | 22:45 |
| jdstrand | thanks :) | 22:45 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!