mup | PR snapcraft#2878 closed: extensions: fix merging of build-environment <Created by cjp256> <Closed by cjp256> <https://github.com/snapcore/snapcraft/pull/2878> | 00:25 |
---|---|---|
zylop | everyday less channels interesting here | 00:25 |
zylop | after an uodate from 28.8bps modem to a 5Mbps fiber t1 Line | 00:25 |
zylop | its sad | 00:25 |
zylop | at least #snappy doesnt have trolls thank god | 00:26 |
zylop | well too late for chat today, maybe i'll create a project called aspies/will/be/ddosed | 00:27 |
zylop | ciao and thanks | 00:27 |
cjp256 | ijohnson: yes, thanks! https://github.com/travis-ci/dpl/pull/1154 | 00:45 |
mup | PR travis-ci/dpl#1154: Update snapd <Created by BanzaiMan> <https://github.com/travis-ci/dpl/pull/1154> | 00:45 |
mborzecki | morning | 06:41 |
=== Girtablulu_ is now known as Girtablulu | ||
zyga | almost te | 08:14 |
zyga | re | 08:14 |
zyga | outside is lovely today | 08:14 |
zyga | pstolowski: how does it look like outside for you? | 08:14 |
pstolowski | zyga: hey, chilly (-1C) but sunny | 08:18 |
mborzecki | zyga: pstolowski: -1 here too, mornig frost everwhere, yesteday evening was -3 and super foggy (no surprise though, +10 during the day) | 08:29 |
zyga | re | 08:36 |
zyga | -2 here when I woke up | 08:36 |
zyga | and fog thick like cream | 08:36 |
zyga | https://usercontent.irccloud-cdn.com/file/Phm3QnMj/image.png | 08:36 |
zyga | when I finally got out to take Bit for a walk it was clearing | 08:37 |
zyga | but still foggy | 08:37 |
zyga | last night I read a little about table layout problem | 08:37 |
zyga | and I decided not to :) | 08:37 |
zyga | at least not during work hours | 08:37 |
zyga | it's a significant chunk of work | 08:37 |
zyga | today I'll start with selinux | 08:37 |
sdhd-sascha | good morning :) | 08:39 |
sdhd-sascha | what is table layout problem ? | 08:39 |
zyga | sdhd-sascha: you have a table comprised of cells | 08:39 |
pstolowski | zyga: what about selinux? | 08:39 |
zyga | sdhd-sascha: some cells may span several rows or columns | 08:39 |
zyga | sdhd-sascha: each cell has text inside | 08:39 |
zyga | sdhd-sascha: text can wrapped according to typical rules (e.g. break on spaces) | 08:40 |
zyga | sdhd-sascha: the problem is | 08:40 |
zyga | sdhd-sascha: given set width | 08:40 |
zyga | sdhd-sascha: compute the minimum height of the table | 08:40 |
zyga | sdhd-sascha: this is NP complete | 08:40 |
zyga | sdhd-sascha: then you have more constraints, e.g. to make text look good according to some optimization function | 08:40 |
zyga | sdhd-sascha: there are some heuristics to follow (that all the implementations use) | 08:40 |
sdhd-sascha | ah, ok, cool | 08:41 |
zyga | sdhd-sascha: there are perfect solutions using very expensive algorithms, mainly to measure the speed and quality of a given heuristic | 08:41 |
sdhd-sascha | sounds like perfect for functional programming and recursion | 08:41 |
zyga | sdhd-sascha: but they all involve heavy math and optimization problems | 08:41 |
zyga | sdhd-sascha: linear algebra, dynamic programming | 08:41 |
zyga | sdhd-sascha: not "I'll write this after reading a paper" in one evening | 08:42 |
zyga | sdhd-sascha: the problem of figuring out how to size a cell is called cell configuration | 08:42 |
zyga | sdhd-sascha: and each cell has a finite set of configurations | 08:43 |
zyga | sdhd-sascha: typically one is best but it's quadratic or O(N^2/3) at best with tricks to find cell configuration that's good | 08:43 |
zyga | sdhd-sascha: that's per cell with N being word legth | 08:43 |
zyga | *count | 08:43 |
zyga | sdhd-sascha: anyway | 08:43 |
zyga | sdhd-sascha: it's fun but not for work | 08:43 |
sdhd-sascha | zyga: maybe, find the cell with the least possibilities. Then call the next, with least possibilities... Could be a starting point | 08:44 |
sdhd-sascha | zyga: yes, that's really fun :-) And true, it needs time | 08:45 |
zyga | This problem has been researched since 1960s | 08:45 |
zyga | There are lots of good algorithms but they are all complex to implement | 08:45 |
sdhd-sascha | 1960s - hmm, sounds like functional programming | 08:45 |
sdhd-sascha | it's sure a compination of bread first and deep first searches | 08:46 |
sdhd-sascha | combination - (sorry) | 08:46 |
zyga | https://people.eng.unimelb.edu.au/gkgange/pubs/table_11.pdf | 08:47 |
sdhd-sascha | and, sorry not "bread" - i mean breadth-first | 08:48 |
sdhd-sascha | cool - thank you :-) | 08:48 |
sdhd-sascha | zyga: hmm - because of the symlink not possible problem in snap. I will give the user at first a setup.sh script like alan it does. | 08:50 |
sdhd-sascha | Maybe i can set an alias after the first start from setup-script to real-app. | 08:50 |
zyga | we should discuss the problem | 08:54 |
zyga | and come up with a strategy to fix it | 08:54 |
zyga | pstolowski: I have a selinux denial to fix in my pR | 08:55 |
pstolowski | ic | 08:57 |
jamesh | zyga: while updating one of my PRs related to portal support, I noticed that a test I'd added was failing on Fedora 31 due to it using cgroups v2. Is that something that is likely to be fixable? | 09:07 |
zyga | jamesh: can you be more specific please? | 09:08 |
zyga | jamesh: I have this for cgroupv2 https://github.com/snapcore/snapd/pull/7825 | 09:08 |
mup | PR #7825: many: use transient scope for tracking apps and hooks <Created by zyga> <https://github.com/snapcore/snapd/pull/7825> | 09:08 |
zyga | jamesh: but it needs another review | 09:08 |
zyga | it's open since november | 09:08 |
jamesh | zyga: it was code using this snapFromPid() code: https://github.com/snapcore/snapd/blob/master/usersession/userd/helpers.go#L76 | 09:08 |
zyga | jamesh: ah, yes | 09:08 |
jamesh | which has a "if cgroup.IsUnified()" error case at the top | 09:09 |
zyga | jamesh: I need to look but I think can be fixed | 09:09 |
zyga | jamesh: we have some generic code in sandbox/cgroup | 09:09 |
zyga | jamesh: so I think the answer is "yes" | 09:09 |
zyga | jamesh: it literally needs reviews and nothing else I think | 09:10 |
jamesh | zyga: I'm mainly concerned because I'd like to reimplement a quick "is this a snap?" check for xdg-desktop-portal itself (to replace the one relying on AppArmor) | 09:10 |
zyga | yeah I remebmer that | 09:10 |
zyga | I think mvo is trying to get reviews to work better | 09:10 |
zyga | specifically security reviews | 09:10 |
jamesh | and if the freezer cgroup thing can't be relied on in the snapd code, then I probably shouldn't be writing more code that uses that method | 09:11 |
zyga | it cannot be relied on | 09:11 |
jamesh | I remember at one point you'd talked about having some kind of marker file in the snap's mount namespace | 09:11 |
zyga | jamesh: yes, we explored many ideas | 09:12 |
zyga | jamesh: right now I cannot promise you the inode idea gets implemented | 09:12 |
zyga | jamesh: but the cgroup idea is very close and seems to be entirely generic as well | 09:12 |
zyga | jamesh: and all you need is one read() call to see which snap a process belongs to | 09:12 |
jamesh | but it'd be a different check compared to what that function is currently doing? | 09:14 |
zyga | jamesh: yes | 09:14 |
zyga | jamesh: the PR I referenced changes most (need to see if it changes userd code, I wasn't aware it does this) | 09:15 |
jamesh | zyga: I was looking at moving that utility function out of userd, so it could be used by the "snap routine portal-info" command | 09:16 |
jamesh | but it is still essentially the same code | 09:16 |
zyga | yeah I think it should move to sandbox | 09:16 |
zyga | mainly because it's one implementation over many | 09:16 |
zyga | jamesh: FYI I will land https://github.com/snapcore/snapd/pull/7238 | 09:21 |
mup | PR #7238: usersession: add systemd user instance service control to user session agent <Needs Samuele review> <Security-High> <Created by jhenstridge> <https://github.com/snapcore/snapd/pull/7238> | 09:21 |
zyga | jamesh: as soon as it goes green | 09:21 |
zyga | jamesh: oh, and I have a question for you | 09:21 |
jamesh | zyga: thanks | 09:21 |
zyga | jamesh: is https://github.com/snapcore/snapd/pull/7589 ok? | 09:22 |
zyga | jamesh: Samuele asked you to changed the PR description | 09:22 |
mup | PR #7589: cmd/snap: add ability to register "snap routine" commands <Created by jhenstridge> <https://github.com/snapcore/snapd/pull/7589> | 09:22 |
zyga | but I wasn't following the history and not sure if you did or not | 09:22 |
zyga | jamesh: can you comment there please | 09:22 |
jamesh | zyga: yep. The initial comment was edited as requested. | 09:23 |
zyga | excellent | 09:23 |
zyga | I'll merge it as well | 09:24 |
zyga | I'll just merge master into it to ensure it's green still | 09:24 |
jamesh | zyga: out of interest, where in the sandbox package do you think that snapFromPid() function would best belong? | 09:25 |
zyga | jamesh: sandbox/cgroup as it models the source of that information | 09:26 |
jamesh | okay. I was just wondering since the API is not particularly cgroup specific | 09:27 |
jamesh | it currently uses cgroups, but that seems like an implementation detail | 09:28 |
zyga | jamesh: yeah but that's how the sandbox package is modeled now, if it changes we can just adjust the calling code; I think it's more important to have one implementation than an abstract entry point | 09:33 |
zyga | (we can have both eventually) | 09:33 |
jamesh | fair enough | 09:33 |
zyga | mborzecki: I cannot understand that selinux denial at all, looking at selinux policy now | 09:51 |
mborzecki | zyga: got the avc log? | 09:51 |
zyga | mborzecki: same as before but that's not the point | 09:51 |
zyga | mborzecki: the denial is new | 09:51 |
zyga | mborzecki: the denial comes from snap-discard-ns | 09:51 |
zyga | mborzecki: using setegid | 09:51 |
zyga | mborzecki: which it starts to in this PR | 09:51 |
zyga | mborzecki: the point is - how can this be denied when it runs as root and snap-confine can setegid all it wants | 09:52 |
zyga | and it calls setegid just prior to the fork/exec call | 09:52 |
zyga | mborzecki: I must be missing something :/ | 09:52 |
mborzecki | zyga: can you paste that denial? con't find it in history | 09:53 |
zyga | sure, one sec | 09:53 |
mup | PR snapcraft#2882 closed: extensions: change extension merge-strategy to fix build-environment <Created by cjp256> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/2882> | 09:53 |
zyga | type=AVC msg=audit(01/16/20 18:15:29.361:1557) : avc: denied { setgid } for pid=13356 comm=snap-discard-ns capability=setgid scontext=system_u:system_r:snappy_mount_t:s0 tcontext=system_u:system_r:snappy_mount_t:s0 tclass=capability permissive=1 | 09:53 |
* Chipaca prepping for the trip | 09:54 | |
pedronis | mborzecki: mvo: hi, #7992 needs 2nd reviews and some tweaks (see my own review comments) | 09:55 |
mup | PR #7992: bootloader: add ExtractedRunKernelImageBootloader interface, implement in grub <UC20> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/7992> | 09:55 |
mborzecki | pedronis: i can take care of the tweaks | 09:55 |
pedronis | mborzecki: thx, please double check that my suggestions make sense | 09:56 |
mborzecki | pedronis: ok | 09:56 |
mup | PR snapcraft#2847 closed: Catkin plugin: consider only 'local' workspaces <Created by artivis> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/2847> | 09:59 |
mborzecki | zyga: hm does s-d-n explicitly setgid? | 10:00 |
zyga | mborzecki: yes, but it runs as root at that time | 10:00 |
mborzecki | zyga: that's the point of selinux, even though it runs as root, the policy is in effect | 10:01 |
zyga | mborzecki: snap-confine doesn't run with egid=0 | 10:01 |
zyga | so it switches to egid=0 and back all the time | 10:01 |
zyga | ? | 10:01 |
zyga | sure | 10:01 |
zyga | but | 10:01 |
pedronis | mvo: open PRs have grown quite a bit again :/ | 10:01 |
zyga | why nothing for snap-confine | 10:02 |
zyga | is it a separate policy? | 10:02 |
mvo | pedronis: yeah, today is a busy day for me but I will try to do some more reviews | 10:02 |
mborzecki | zyga: s-c already has setgid | 10:02 |
zyga | ahhh | 10:02 |
mborzecki | zyga: so selinux policy can actually confine root ;) | 10:02 |
zyga | sure | 10:03 |
zyga | I assumed that snap-confine policy would apply to snap-update-ns invoked from snap-confine | 10:03 |
zyga | not that it would have a standalone policy | 10:03 |
pedronis | zyga: my first impulse feeling on #8008 is that is too big | 10:03 |
mup | PR #8008: render: add the render package and basic widgets <Created by zyga> <https://github.com/snapcore/snapd/pull/8008> | 10:03 |
mborzecki | zyga: no, s-u-n and s-d-n are run with snappy_mount_t context which has a separate policy | 10:03 |
Wouter0100 | Hey guys! Is there any timeline/progress on RPi4 support for Ubuntu Core? :D Curious, as we don't want to be stuck on RPi3 if I start building using Core now | 10:05 |
mborzecki | zyga: i see there's an entry with `allow snappy_mount_t self:capability {..}`, add setgid there | 10:06 |
Chipaca | pedronis: looking at how o/auth handles user addition and removal, don't we need a graveyard for users? | 10:07 |
Chipaca | pedronis: if you don't know offhand i can dig myself :) | 10:07 |
zyga | mborzecki: I see snappy_mount_exec_t | 10:07 |
zyga | is that the type? | 10:07 |
pedronis | Chipaca: I'm not even sure I understand the question tbh | 10:08 |
zyga | mborzecki: I don't see where it gets any permissions yet | 10:08 |
Chipaca | pedronis: you know how we do refreshes bunched by user id | 10:08 |
mborzecki | zyga: nope, snappy_mount_t, around like 426 in snappy.te | 10:08 |
pedronis | Chipaca: yes | 10:08 |
Chipaca | pedronis: when a user logs out, we delete the user completely | 10:08 |
Chipaca | pedronis: if they then log back in they get a new id | 10:08 |
zyga | mborzecki: how is snappy_mount_t related to snap-discard-ns? | 10:09 |
mborzecki | zyga: https://paste.ubuntu.com/p/P4q7VYpMh7/ | 10:09 |
Chipaca | unless i'm missing something, that means the refreshes will be wrong? | 10:09 |
Chipaca | as in, they'll be done as root from there on | 10:09 |
pedronis | Chipaca: aren't the ids always growing | 10:09 |
Chipaca | pedronis: yes | 10:09 |
pedronis | we don't recycle ids, or do we | 10:09 |
Chipaca | pedronis: no we don't | 10:09 |
pedronis | Chipaca: well, the refreshes will already be not working once they log otu | 10:10 |
Chipaca | pedronis: but shouldn't they work again once they log back in | 10:10 |
zyga | mborzecki: but how is that related? | 10:10 |
zyga | *related | 10:10 |
pedronis | Chipaca: maybe yes | 10:10 |
Chipaca | pedronis: i know this sounds related to my current work but it's actually because i resucitated an old laptop and the macaroon had expired so i logged out and back in and that got me thinking | 10:11 |
mborzecki | zyga: both do stuff with mounts, so instead of making it even more fine grained both get the snappy_mount_t policy | 10:11 |
pedronis | Chipaca: you basically are saying that we should reuse the id if matching email | 10:11 |
pedronis | or something | 10:11 |
zyga | I mean how is snap-discard-ns associated with snappy_mount_t | 10:11 |
Chipaca | pedronis: meaning we should keep a graveyard of sorts to remember those things | 10:11 |
mborzecki | zyga: how does selinux figure that out? | 10:11 |
pedronis | Chipaca: what happens if you don't logout but login again? | 10:12 |
zyga | my point is that I cannot trace anything from snap-discard-ns | 10:12 |
pedronis | do we error? | 10:12 |
zyga | to that place where you say to patch things | 10:12 |
pedronis | or do something else? | 10:12 |
pedronis | or it's a nop | 10:12 |
pedronis | (I don't remember myself) | 10:12 |
mborzecki | zyga: quick HO? | 10:13 |
zyga | mborzecki: how are those entries connected | 10:13 |
zyga | mborzecki: sure | 10:13 |
Chipaca | pedronis: looks like you get new macaroni | 10:14 |
Chipaca | pedronis: it works | 10:14 |
Chipaca | pedronis: (so yes i should've done that) | 10:14 |
pedronis | Chipaca: ok, so we have an issue but is not as bad as it could be | 10:14 |
Chipaca | pedronis: yep | 10:14 |
Chipaca | well, the error could be better | 10:14 |
Chipaca | you just get an opaque 'invalid credentials' | 10:15 |
Chipaca | but not clear we can improve on that easily | 10:15 |
pedronis | Chipaca: we need to think a bit, can you create a bug or something | 10:15 |
pedronis | to remember this | 10:15 |
Chipaca | yes | 10:16 |
pedronis | thx | 10:16 |
Chipaca | pedronis: https://bugs.launchpad.net/snapd/+bug/1860102 | 10:23 |
mup | Bug #1860102: snap logout and snap login with same account creates new user <snapd:New> <https://launchpad.net/bugs/1860102> | 10:23 |
pedronis | Chipaca: thanks, tracking as mine for now | 10:23 |
Chipaca | pedronis: 👍 | 10:23 |
* Chipaca goes back to laptop resuscitation | 10:24 | |
mup | PR snapd#7238 closed: usersession: add systemd user instance service control to user session agent <Needs Samuele review> <Security-High> <Created by jhenstridge> <Merged by pedronis> <https://github.com/snapcore/snapd/pull/7238> | 10:27 |
=== pedronis_ is now known as pedronis | ||
pedronis | jamesh: hi, could you add the comment jdstrand asked for in #7555, then it could be merged | 10:27 |
mup | PR #7555: tests: add a test demonstrating that snaps can't access the session agent socket <Created by jhenstridge> <https://github.com/snapcore/snapd/pull/7555> | 10:27 |
zyga | good luck Chipaca | 10:29 |
mborzecki | https://github.com/snapcore/snapd/pull/8013 needs 2nd review | 10:31 |
mup | PR #8013: cmd/snap-bootstrap: check device size before boostrapping and produce a meaningful error <Simple 😃> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/8013> | 10:31 |
* zyga looks | 10:31 | |
zyga | mborzecki: is https://github.com/snapcore/snapd/pull/8013/files#diff-01caa2f676929e8131f31685ae61203cR216 a separate error that is fixed in this branch? | 10:33 |
mup | PR #8013: cmd/snap-bootstrap: check device size before boostrapping and produce a meaningful error <Simple 😃> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/8013> | 10:33 |
mborzecki | zyga: yes, separate patch in that branch | 10:34 |
zyga | mborzecki: I have a small comment on the size code | 10:34 |
mup | PR snapd#7366 closed: interfaces/gpg-keys: Allow access to gpg-agent and creation of lockfiles <Created by ppd1990> <Closed by pedronis> <https://github.com/snapcore/snapd/pull/7366> | 10:37 |
mup | PR snapcraft#2883 opened: docker: add core18 snap that snapcraft now uses as a base <Created by sergiusens> <https://github.com/snapcore/snapcraft/pull/2883> | 10:44 |
sdhd-sascha | Can i anything do, for #7927 ? I still need a patched `snapd` to start sway on classic ubuntu. | 10:46 |
mup | PR #7927: interfaces: x11 allow apparmor on classic <Created by sd-hd> <https://github.com/snapcore/snapd/pull/7927> | 10:46 |
pedronis | sdhd-sascha: is not green atm, and needs a jdstrand review (he is off this week) | 10:47 |
sdhd-sascha | pedronis: ok, i rebase it to current master | 10:48 |
zyga | mborzecki: I opened https://github.com/snapcore/snapd/pull/7980 for review now - the SELinux denial is gone | 10:49 |
mup | PR #7980: packaging,snap-confine: stop being setgid root <Created by zyga> <https://github.com/snapcore/snapd/pull/7980> | 10:49 |
* Chipaca brb | 11:17 | |
mup | PR snapd#8017 closed: tests: add ubuntu 20.04 to the tests execution and remove tumbleweed from unstable <Created by sergiocazzolato> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/8017> | 11:19 |
sdhd-sascha | Hi, as you know, i never worked with golang before. Is there a way to reset the source to a clean working state ? I deleted snapd/cmd/VERSION. And it's in `.gitignore`. So i wonder, how to reset without `go get ...` | 11:21 |
mup | PR snapd#8012 closed: o/devicestate: do not create perfTimings if not needed inside ensureSeed/Operational <Simple 😃> <Created by stolowski> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/8012> | 11:24 |
pstolowski | sdhd-sascha: reset the git working dir of a project? 'git clean -xfd', beware it will forcefully remove any uncommited stuff in the working tree without asking | 11:24 |
mup | PR snapd#8006 closed: tests: skip interfaces-network-manager on arm devices <Simple 😃> <Created by sergiocazzolato> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/8006> | 11:26 |
sdhd-sascha | pstolowski: thank you. I found mkversion.sh. But it's confusing if you change the branch and then the version remains nearly the same. | 11:27 |
sdhd-sascha | I build 2.43 and wonder, why it still reports 2.42 ... | 11:27 |
zyga | mborzecki: :) https://github.com/snapcore/snapd/pull/8016 | 11:28 |
mup | PR #8016: gitignore: ignore snap files <Simple 😃> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/8016> | 11:28 |
mup | PR snapd#8016 closed: gitignore: ignore snap files <Simple 😃> <Created by anonymouse64> <Merged by zyga> <https://github.com/snapcore/snapd/pull/8016> | 11:30 |
zyga | pedronis__: does your blocked label mean that no progress can be done on the render branch for the next week? | 11:41 |
* zyga jumps into https://github.com/snapcore/snapd/pull/7614 | 11:42 | |
mup | PR #7614: cmd/snap-confine: implement snap-device-helper internally <Created by zyga> <https://github.com/snapcore/snapd/pull/7614> | 11:42 |
sdhd-sascha | Hey, i read a lot of MAAS snap here. Is it in the store ? Or did i need to build it myself? | 11:47 |
Chipaca | sdhd-sascha: I think it's unlisted until default tracks work, as upgrades from latest to 2.7 are painful | 11:48 |
Chipaca | sdhd-sascha: but if you find sparkiegeek they might know more | 11:49 |
Chipaca | (default tracks should be working on edge! woop woop) | 11:49 |
sdhd-sascha | Chipaca: thank you. I'm already on 2.7 on local installation and want to migrate to snap. | 11:53 |
Chipaca | sdhd-sascha: note the versions in 'snap info maas' | 11:54 |
sdhd-sascha | hmm, i found it. thank you :) | 11:55 |
sdhd-sascha | Chipaca: did you know if there is a maas-rack and/or maas-region snap ? Or is it all-in-one ? If you didn't know. Then i will figure it out. No stress ;-) | 11:56 |
Chipaca | sdhd-sascha: I don't know either :) I know next to nothing about maas in fact | 11:56 |
sdhd-sascha | Chipaca: I am also at the experimental level. would like to use maas with lxd and juju. | 11:58 |
Chipaca | sdhd-sascha: #maas, maybe? (dunno) | 11:59 |
=== pedronis__ is now known as pedronis | ||
pedronis | Chipaca: I think #7589 could land once green, can you quickly double check it again | 12:08 |
mup | PR #7589: cmd/snap: add ability to register "snap routine" commands <Created by jhenstridge> <https://github.com/snapcore/snapd/pull/7589> | 12:08 |
* zyga looks at another PR | 12:33 | |
mup | PR snapd#7623 closed: tests: check world-writable and test-owned files <Created by zyga> <Closed by zyga> <https://github.com/snapcore/snapd/pull/7623> | 12:35 |
cachio | mborzecki, hello | 12:35 |
cachio | I am reviewing a failing test | 12:35 |
cachio | selinux-clear which is failing on centos | 12:35 |
cachio | mborzecki, this is the error https://paste.ubuntu.com/p/5DxBHnD93d/ | 12:36 |
cachio | seems to be real | 12:36 |
mborzecki | cachio: hah, yeah, known problem, i'll update the policy, keeping centos in unstable systems doesn't help with catching issues like this one early | 12:38 |
cachio | mborzecki, nice, thanks for the confirmation | 12:38 |
pstolowski | cachio: updated #7871, thanks for pointing out pkgdb | 12:49 |
mup | PR #7871: tests: add spread test for hook permissions <Created by stolowski> <https://github.com/snapcore/snapd/pull/7871> | 12:49 |
cachio | pstolowski, yaw | 12:49 |
mup | PR snapd#7875 closed: interfaces: refactor path() from raw-volume into utils with comments for old <Created by jdstrand> <Merged by zyga> <https://github.com/snapcore/snapd/pull/7875> | 12:54 |
pedronis | mborzecki: am I misreading or #7845 should be closed ? | 12:57 |
mup | PR #7845: cmd: add prefix to SYSTEMD_SYSTEM_GENERATOR_DIR <Simple 😃> <Created by sd-hd> <https://github.com/snapcore/snapd/pull/7845> | 12:57 |
mborzecki | pedronis: IMO yes, the problem should be addressed in systemd (if ever) | 12:58 |
pedronis | ok | 12:59 |
mup | PR snapd#7845 closed: cmd: add prefix to SYSTEMD_SYSTEM_GENERATOR_DIR <Created by sd-hd> <Closed by pedronis> <https://github.com/snapcore/snapd/pull/7845> | 13:00 |
mborzecki | added a note to 7845 with some suggestions on hot to approach this sdhd-sascha, it'd be nice to hear what upstream systemd suggest to use there | 13:03 |
* sdhd-sascha I'm back later. And look | 13:06 | |
Chipaca | pedronis: agreed, 7589 can land once green | 13:10 |
pstolowski | pedronis: can we land #7934, and potentially address ijohnson's suggestion re test refactor in a followup? | 13:11 |
mup | PR #7934: o/ifacestate,o/devicestatate: merge gadget-connect logic into auto-connect <Created by pedronis> <https://github.com/snapcore/snapd/pull/7934> | 13:11 |
pedronis | pstolowski: yes, I suspect it would be good to fix the misleading comment though | 13:13 |
pedronis | pstolowski: I'm taking care of that | 13:15 |
pstolowski | pedronis: ty | 13:15 |
pedronis | pstolowski: done, let's see if it gets green again | 13:18 |
Chipaca | pedronis: btw, did you see https://forum.snapcraft.io/t/snapctl-changing-channel-ability/14900?u=chipaca ? | 13:20 |
pedronis | Chipaca: yes, I don't think is something we want to let snap do | 13:20 |
pedronis | in general | 13:20 |
Chipaca | pedronis: right, not like that, but then they ask for more nuanced things in https://forum.snapcraft.io/t/snapctl-changing-channel-ability/14900/3?u=chipaca | 13:20 |
Chipaca | some of them we already wanted (were already implementing?) | 13:21 |
Chipaca | i mean, the 'are there updates in the current channel' one i thought was roadmapped even | 13:21 |
pedronis | yes | 13:21 |
pedronis | even refresh it's probably doable with some user confirmation | 13:22 |
Chipaca | pedronis: 'reading current channel' seems a'ight, also | 13:22 |
pedronis | changing channel likely not | 13:22 |
zyga | brb | 13:22 |
Chipaca | agreed, not without some way of asking the user | 13:22 |
mborzecki | https://github.com/snapcore/snapd/pull/8005 needs a 2nd review | 13:29 |
mup | PR #8005: api: don't return connections referring to non-existing plugs/slots <Bug> <Created by stolowski> <https://github.com/snapcore/snapd/pull/8005> | 13:29 |
zyga | looking | 13:29 |
pedronis | Chipaca: answered or tried to | 13:31 |
Chipaca | pedronis: some from → some form | 13:31 |
Chipaca | :) | 13:31 |
pedronis | Chipaca: yea, fixed | 13:31 |
pedronis | already | 13:31 |
Chipaca | pedronis: having to check version info to know channel breaks QAing a snap along channels though | 13:32 |
pedronis | Chipaca: I don't know, there is a list of features there, not use cases | 13:33 |
Chipaca | pedronis: I've been tempted to make snapd always log verbosely when in edge or beta, for example | 13:33 |
pedronis | I would like to understand the use case more | 13:33 |
Chipaca | pedronis: yep yep | 13:33 |
zyga | hmm | 13:34 |
zyga | pstolowski: do you remember the discussions from Malta about how we want to show connections | 13:34 |
pedronis | Chipaca: is it realy about risks or is it about tracks? | 13:34 |
pedronis | etc | 13:34 |
zyga | pstolowski: when the thing is not present (e.g. dead hotplug connection) | 13:34 |
Chipaca | pedronis: k | 13:34 |
* zyga wonders where the notes from Malta might be | 13:34 | |
zyga | I recall gustavo recorded those in a web tool of some sort | 13:34 |
* zyga looks | 13:34 | |
pedronis | afaik the agreement was then copied into: https://forum.snapcraft.io/t/snap-connections-command/4296/20 | 13:38 |
pedronis | and is what was implemented | 13:38 |
mborzecki | hmm something still not right in 7992 | 13:42 |
pstolowski | zyga: i remember the topic and the general idea, but not details, i don't think we had a concrete plan | 13:44 |
zyga | pstolowski: I'm looking into this | 13:44 |
pstolowski | zyga: is this wrt my PR? | 13:44 |
zyga | yep | 13:44 |
zyga | give me a moment to go through what we planned to do | 13:45 |
pstolowski | zyga: my PR is not closing any doors, but atm user experience is confusing. i made a comment in the code about what needs tweaking when we have an egreement | 13:45 |
ijohnson | morning folks | 13:46 |
pstolowski | ijohnson: hi! | 13:46 |
ijohnson | thanks mborzecki and pedronis for the reviews/commits to the kernel things | 13:46 |
ijohnson | hi pstolowski | 13:46 |
mborzecki | ijohnson: hi, i'll push one more patch if you don't mind | 13:46 |
ijohnson | mborzecki: go for it | 13:46 |
mborzecki | ijohnson: ok, pushed now ;) | 13:57 |
ijohnson | thanks! | 13:57 |
zyga | pstolowski: reviewed | 13:57 |
zyga | https://github.com/snapcore/snapd/pull/8005#pullrequestreview-344593087 | 13:57 |
zyga | oh | 13:57 |
zyga | standup | 13:57 |
mup | PR #8005: api: don't return connections referring to non-existing plugs/slots <Bug> <Created by stolowski> <https://github.com/snapcore/snapd/pull/8005> | 13:57 |
pstolowski | zyga: ty! | 13:58 |
=== ricab is now known as ricab|lunch | ||
mup | PR snapd#8005 closed: api: don't return connections referring to non-existing plugs/slots <Bug> <Created by stolowski> <Merged by stolowski> <https://github.com/snapcore/snapd/pull/8005> | 14:15 |
cjp256 | zyga: sergiusens suggested you might know... when I have a slot defined `slot: mpris: {interface: mpris, name: foo}` , review-tools fails with `"human review required due to 'deny-connection' constraint (interface attributes)"` | 14:16 |
cjp256 | review-tools specifically has a test case for this: https://git.launchpad.net/review-tools/tree/reviewtools/tests/test_sr_declaration.py#n5195 | 14:17 |
cjp256 | but for some reason, if we drop the optional "interface" key, it passes: `slot: mpris: { name: foo }` | 14:18 |
zyga | cjp256: I don't know the review tools code, I cannot explain why without jumping into it from scratch | 14:23 |
zyga | brb, need to check up on kids | 14:37 |
pedronis | ijohnson: if you are happy with the changes 7992 can land once it is green again | 14:56 |
* Chipaca goes to have his head seen to | 14:58 | |
* Chipaca notes it's for a haircut | 14:58 | |
ijohnson | pedronis: ack I will try to get it in and rebase the other one for shorter diff | 14:58 |
mup | PR snapd#7555 closed: tests: add a test demonstrating that snaps can't access the session agent socket <Created by jhenstridge> <Merged by pedronis> <https://github.com/snapcore/snapd/pull/7555> | 15:10 |
* cachio lunch | 15:16 | |
=== ricab|lunch is now known as ricab | ||
mup | PR snapd#7589 closed: cmd/snap: add ability to register "snap routine" commands <Created by jhenstridge> <Merged by pedronis> <https://github.com/snapcore/snapd/pull/7589> | 16:00 |
pedronis | mborzecki: does #8013 need a review from cmatsuoka ? | 16:02 |
mup | PR #8013: cmd/snap-bootstrap: check device size before boostrapping and produce a meaningful error <Simple 😃> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/8013> | 16:02 |
zyga | re | 16:07 |
ijohnson | pedronis: shall I merge #7934 if it goes green today in my PM? | 16:10 |
mup | PR #7934: o/ifacestate,o/devicestatate: merge gadget-connect logic into auto-connect <Created by pedronis> <https://github.com/snapcore/snapd/pull/7934> | 16:10 |
pedronis | ijohnson: if it's green, yes, I made a note to myself about the test refactor | 16:10 |
ijohnson | pedronis: ack | 16:10 |
ijohnson | pedronis: mborzecki: reviewing the patches to 7992, I notice that now DisableTryKernel returns nil if the symlink is _missing_, I thought that we wanted it to return an error if the symlink is missing, but if it's a dangling symlink we don't return error | 16:28 |
ijohnson | pedronis: mborzecki: the use case I was thinking of for that is that if we have a poorgramming error where we try to disable a try kernel when there isn't even a try-kernel.efi we still want to error | 16:29 |
ijohnson | specifically https://github.com/snapcore/snapd/pull/7992/commits/ac52ca80c27aa3a796177628701b07c95e01ab28#diff-637bfac32ed88896fef8c212d836bf11L217-R217 | 16:30 |
mup | PR #7992: bootloader: add ExtractedRunKernelImageBootloader interface, implement in grub <UC20> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/7992> | 16:30 |
mup | PR snapd#8013 closed: cmd/snap-bootstrap: check device size before boostrapping and produce a meaningful error <Simple 😃> <Created by bboozzoo> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/8013> | 16:33 |
mup | PR snapd#7871 closed: tests: add spread test for hook permissions <Created by stolowski> <Merged by stolowski> <https://github.com/snapcore/snapd/pull/7871> | 16:40 |
pstolowski | huh, finally, it took ages | 16:40 |
cjp256 | zyga: just out of curiousity, would you expect behavior changes if "interface" is defined? The docs say it's optional if the plug/slot name matches the interface? | 17:25 |
mup | PR snapd#8018 opened: spread.yaml: fix ubuntu 19.10 and 20.04 names <Simple 😃> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/8018> | 17:48 |
zyga | cjp256: snapd doesn't consider it any different, both result in identical slot | 17:56 |
zyga | cjp256: it feels like bug in the review tools to me | 17:56 |
cjp256 | zyga: ok thanks, that's what I figured. :) have a good weekend! | 17:58 |
zyga | thank you :) | 17:58 |
mup | PR snapd#7992 closed: bootloader: add ExtractedRunKernelImageBootloader interface, implement in grub <UC20> <Created by anonymouse64> <Merged by anonymouse64> <https://github.com/snapcore/snapd/pull/7992> | 18:44 |
pedronis | ijohnson: in general given our use case we don't want to fail even if we get something wrong, we should fail if a kernel that we want to enable is not there, but if a symlink is not there that we are about too remove, well too bad | 18:46 |
ijohnson | pedronis: hmm ok | 18:47 |
pedronis | ijohnson: robustness and correctness is not always the same thing | 18:47 |
ijohnson | pedronis: I guess I was looking at it from the standpoint of having more certainy about what happened | 18:48 |
ijohnson | pedronis: less of a correctness thing and more of a logging thing, i.e. the fact that the symlink wasn't there when we delete it might be masking some other error that happened before | 18:48 |
pedronis | ijohnson: I understand but we don't devices stuck unless there is a strong argument why we can't move forward | 18:49 |
ijohnson | pedronis: sure I guess in that instance it would be a higher level thing and that the caller of DisableTryKernel would see "hey this symlink wasn't there" and still move on, but maybe could log a message | 18:50 |
ijohnson | (IMHO) | 18:50 |
pedronis | ijohnson: I see but it gets too subtle, locally it seems a reasonable enough call | 18:50 |
pedronis | also people don't look at logs of things at appear to work | 18:51 |
pedronis | s/at/that/ | 18:51 |
ijohnson | pedronis: well I'll defer to your judgment about the design of this, but I do disagree that folks don't look at logs when things are working, I think looking at logs is often times a measurement of whether things are working, i.e. when you have a thousand devices you don't want to only monitor the ones that failed | 18:52 |
pedronis | ijohnson: being demanding of what we expect our code to do is a task for our tests, avoiding uncessary failure is something we do want at runtime | 18:53 |
pedronis | because the cost of failure is possibly very high | 18:53 |
ijohnson | pedronis: sure | 18:53 |
ijohnson | I agree with that | 18:53 |
pedronis | ijohnson: anyway we you think we should log that condintion then bootloader itself should do that | 18:56 |
pedronis | there's no point making it the task of one level up | 18:56 |
ijohnson | pedronis: that's fair I might do that then | 18:56 |
mup | PR snapd#7934 closed: o/ifacestate,o/devicestatate: merge gadget-connect logic into auto-connect <Created by pedronis> <Merged by pedronis> <https://github.com/snapcore/snapd/pull/7934> | 18:59 |
mup | PR snapcraft#2884 opened: plugs/slots: match output format read in snapcraft.yaml <Created by cjp256> <https://github.com/snapcore/snapcraft/pull/2884> | 19:54 |
* cachio afk | 20:01 | |
mup | PR snapd#8019 opened: [RFC] interfaces/apparmor: don't omit deny rules in devmode confinement <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/8019> | 21:01 |
mup | PR snapd#8020 opened: interfaces/apparmor: fix doc-comments, unnecessary code <Simple 😃> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/8020> | 21:07 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!