| mwhudson | er does cmd/snap progress reporting essentially assume that any task where total > 1 is downloading something? | 00:03 |
|---|---|---|
| mwhudson | that sounds like a chipaca question | 00:03 |
| mwhudson | behold my snazzy progress bar! https://asciinema.org/a/aTbeNl5BnF2jtbWlXZT8ed7eE | 00:54 |
| === chihchun_afk is now known as chihchun | ||
| zyga | Hello | 05:35 |
| mborzecki | morning | 06:07 |
| zyga | hey mborzecki :-) | 06:08 |
| zyga | how are you doing>? | 06:08 |
| mborzecki | zyga: hey | 06:08 |
| mborzecki | zyga: fine, 'connections' landed yesterday :) | 06:08 |
| zyga | I saw, nice work :) | 06:09 |
| mborzecki | zyga: hm need to figure out why the spread test fails in #6485 | 06:10 |
| mup | PR #6485: interfaces/seccomp: regenerate changed profiles only <β Blocked> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/6485> | 06:10 |
| mborzecki | zyga: the small s-c patches are waiting for perdronis right? | 06:11 |
| zyga | mborzecki: and jdstrand | 06:11 |
| zyga | mborzecki: I'm making breakfast for kids now | 06:11 |
| zyga | re | 06:37 |
| zyga | mborzecki: looks l like core18 + snapd not working | 06:37 |
| zyga | mborzecki: did you try to get a debug shell? | 06:38 |
| zyga | though given that this is prepare it may be broken too | 06:38 |
| mborzecki | zyga: which pr? | 06:38 |
| zyga | perhaps we need to adjust that code thttps://github.com/snapcore/snapd/pull/6485 | 06:38 |
| mup | PR #6485: interfaces/seccomp: regenerate changed profiles only <β Blocked> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/6485> | 06:38 |
| zyga | so that we bail out early if snap is not on path | 06:38 |
| mborzecki | zyga: stuck looking at some review now | 06:39 |
| zyga | ok | 06:39 |
| mborzecki | zyga: but that's interesting | 06:39 |
| === pstolowski|afk is now known as pstolowski | ||
| pstolowski | mornings | 08:03 |
| mvo | good morning pstolowski | 08:05 |
| zyga | Hey PaweΕ, Michael | 08:15 |
| mborzecki | pstolowski, mvo: morning guys | 08:18 |
| mvo | hey zyga and mborzecki ! | 08:19 |
| mvo | 6492 needs a second review (its on the 2.38 critical path) | 08:24 |
| mvo | pedronis: 6356 looks good to me, unless you want to do something with it I will merge it | 08:25 |
| mup | PR snapd#6560 closed: cmd/snap-confine: drop uid from random /tmp name <Simple π> <Created by zyga> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/6560> | 08:26 |
| mup | PR snapd#6561 closed: cmd/snap-confine: chown private /tmp to root.root <Simple π> <Created by zyga> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/6561> | 08:26 |
| pedronis | mvo: it's fine to merge for me, but make sure people are aware you would like follow ups | 08:32 |
| mvo | pedronis: I will wait for john - followups are not strictly needed, its mostly ideas/suggestions nothing strictly required | 08:34 |
| pedronis | ok | 08:34 |
| mborzecki | 5.0 kernel is already in testing repo in arch, hope we don't make any assumptions about kernel versions in our code | 08:37 |
| pedronis | mborzecki: we compare it in a couple of places to see if it's higher than some version | 08:41 |
| pedronis | but usually there's a distro check as well | 08:42 |
| pedronis | mborzecki: I see code like in apparmor backend.go seccomp backend.go and sanity/version.go | 08:42 |
| pedronis | they should be fine afaict | 08:42 |
| mborzecki | yeah | 08:42 |
| mvo | mborzecki: when we compare we use the semantic versions compare that follows the debian rules which the kernel honors currently so we should be good | 08:44 |
| pedronis | mvo: you left some optional comments also in #6322 for pstolowski | 08:44 |
| mup | PR #6322: overlord/hookstate: apply pending transaction changes onto temporary configuration for snapctl get <Complex> <Created by stolowski> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/6322> | 08:45 |
| pstolowski | pedronis: i noted these comments and will address in a followup, these were cosmetics | 08:47 |
| zyga | Hey, with dog at vet, Iβll start around 10 today | 08:48 |
| zyga | Thanks for merging mvo | 08:49 |
| doko | how is mvo merged? | 08:51 |
| zyga | Mvo is very dedicated, one could say merged with is duties ;-) | 08:52 |
| mvo | haha | 08:54 |
| mvo | pedronis: I addressed the feedback in 6381, the one question is if I should remove "setDeviceFromModelAssertion" again from that PR and move it into a later one | 09:31 |
| mup | PR snapd#6539 closed: cmd, daemon: split out the common bits of mapLocal and mapRemote <Created by chipaca> <Merged by chipaca> <https://github.com/snapcore/snapd/pull/6539> | 09:36 |
| mvo | 6492 needs a second review | 09:47 |
| mvo | Chipaca: I will merge 6356, there are some nitpicks/ideas in my review but feel free to ignore or do in a followup | 09:48 |
| Chipaca | hmm | 09:48 |
| * Chipaca looks | 09:48 | |
| Chipaca | mvo: ok :-) | 09:48 |
| mwhudson | Chipaca: for some reason i think you might like this https://asciinema.org/a/aTbeNl5BnF2jtbWlXZT8ed7eE | 09:48 |
| mvo | zyga: you mentioned you want to re-review 6549, could you please make it a priority? I would like to finalize 2.38~pre1 this morning | 09:48 |
| Chipaca | mwhudson: nice! | 09:49 |
| Chipaca | mwhudson: how much would that break if the snap progress bar took up more lines? | 09:50 |
| mwhudson | Chipaca: not at all, it's pinging v2/changes/$id | 09:50 |
| Chipaca | mwhudson: ah, it's doing its own thing? ok | 09:50 |
| joedborg | jdstrand: I'm unable to pause them due to this issue https://github.com/canonical-websites/build.snapcraft.io/issues/623 | 09:50 |
| Chipaca | mwhudson: using ansimeter? | 09:51 |
| mwhudson | Chipaca: no, urwid has it's own process bar thingy | 09:51 |
| zyga | mvo: ack | 09:51 |
| zyga | mvo: let me look now | 09:51 |
| zyga | wow, it's windy today | 09:51 |
| mwhudson | Chipaca: you can tell it's not ansimeter becuase it's not reporting time remaining :) | 09:51 |
| Chipaca | ah and it says MiB | 09:51 |
| Chipaca | ok | 09:51 |
| mvo | mwhudson: nice indeed | 09:51 |
| Chipaca | mwhudson: basically, you can have more than 1 thing in Doing | 09:52 |
| mwhudson | Chipaca: wait.go / ansimeter seems to assume that anything that has total > 1 is downloading something | 09:52 |
| mup | PR snapd#6356 closed: overlord/snapstate: during refresh, re-refresh on epoch bump <Created by chipaca> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/6356> | 09:52 |
| Chipaca | mwhudson: and currently we just report the first one | 09:52 |
| Chipaca | which is 'fun' | 09:52 |
| mwhudson | Chipaca: ah! | 09:52 |
| mvo | did I mention that 6492 needs a second review ? *please* ? | 09:52 |
| mwhudson | i guess it wouldn't be so hard to cope with that in my code | 09:52 |
| mwhudson | Chipaca: is this something that happens in practice? | 09:52 |
| Chipaca | mwhudson: that's why if you install something that needs to download a content snap, it'll stay in 'connecting' for ages | 09:53 |
| mwhudson | ah ha | 09:53 |
| Chipaca | mvo: i'll do it | 09:53 |
| mwhudson | so if i was refreshing to a version of subiquity that depended on core18, for example... | 09:53 |
| mvo | Chipaca: thank you! | 09:53 |
| zyga | mvo: I can look as well after apparmor review | 09:54 |
| mvo | zyga: appreicated, but if john looks that is enough, its relatively small and already has a review :) thanks still! | 09:54 |
| zyga | ok | 09:54 |
| Chipaca | mvo: in 6492 you say you "added the check for .Active", but I don't see it | 09:57 |
| Chipaca | mvo: ah, i see the last commit now, ok | 09:57 |
| Chipaca | mvo: we don't block disabling snapd though | 09:58 |
| Chipaca | mvo: so you could have snapd installed and disabled | 09:59 |
| Chipaca | mvo: (add TypeSnapd to snapstate's canDisable) | 09:59 |
| Chipaca | mvo: why am i not writing this on github | 10:00 |
| mup | PR snapd#6562 opened: timings: base API for recording timings in state <Created by stolowski> <https://github.com/snapcore/snapd/pull/6562> | 10:05 |
| pedronis | Chipaca: lots of things break, we don't check for Active core either | 10:05 |
| Chipaca | pedronis: core _is_ in the list that can't be disabled | 10:05 |
| pedronis | Chipaca: is it? | 10:05 |
| Chipaca | []snap.Type{snap.TypeGadget, snap.TypeKernel, snap.TypeOS} | 10:05 |
| pedronis | it wasn't for a long time | 10:06 |
| Chipaca | core is TypeOS | 10:06 |
| Chipaca | we don't check TypeBase | 10:06 |
| pedronis | anyway sounds a different problem | 10:06 |
| pedronis | we do need to switch to a type | 10:06 |
| pedronis | for snapd | 10:06 |
| Chipaca | pedronis: ? | 10:06 |
| Chipaca | snapd is TypeSnapd already | 10:06 |
| pedronis | Chipaca: it isn't | 10:07 |
| pedronis | Chipaca: look at snapcraft.yaml | 10:07 |
| Chipaca | dang | 10:07 |
| Chipaca | we _have_ TypeSnapd | 10:07 |
| pedronis | or do snap info snapd | 10:07 |
| pedronis | Chipaca: we do | 10:07 |
| pedronis | but the store and snapcraft weren't on the plan | 10:08 |
| Chipaca | are we dum | 10:08 |
| Chipaca | :-( | 10:08 |
| Chipaca | we r dum | 10:08 |
| pedronis | sounds we need to push | 10:08 |
| pedronis | Chipaca: I created a card for mvo | 10:10 |
| === cpaelzer_ is now known as cpaelzer | ||
| Chipaca | oh no! i have only one PR up! | 10:13 |
| * Chipaca strives to fix this | 10:13 | |
| pedronis | Chipaca: you can write docs instead | 10:13 |
| Chipaca | pedronis: :-) | 10:13 |
| Chipaca | pedronis: I also have a followup to the snapinfo pr that just landed, that i'm running spread on locally now that it's rebased | 10:14 |
| mup | PR snapd#6492 closed: snapstate: restart into the snapd snap on classic <Created by mvo5> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/6492> | 10:16 |
| mborzecki | wonder whether we should really fail to initialize seccomp backend when snap-seccomp version-info does nto match the patter or is otherwise invalid | 10:30 |
| pedronis | mborzecki: we control both sides, no? | 10:31 |
| mborzecki | pedronis: yes, so it's unlinkely to happen | 10:31 |
| mborzecki | pedronis: however unlikely, should this fail, backend will not initialize and snapd won't start | 10:32 |
| pedronis | mborzecki: we have many (slightly unlikely) things that will make snapd not start | 10:33 |
| mborzecki | ok, so maybe not a problem after all | 10:33 |
| mborzecki | especially since both ends are in sync | 10:33 |
| pedronis | I mean, if we didn't control the other side, I would agree | 10:34 |
| pedronis | that being robust is more important | 10:34 |
| pedronis | but if we do, something is off anyway | 10:34 |
| mborzecki | agreed | 10:34 |
| mup | PR snapd#6563 opened: ctlcmd/tests: tests tweaks (followup to #6322) <Simple π> <Created by stolowski> <https://github.com/snapcore/snapd/pull/6563> | 10:48 |
| pstolowski | mvo: ^ trivial per your comment to 6322 | 10:50 |
| mvo | pstolowski: thanks, I have a look in a wee bit | 10:55 |
| zyga | jdstrand: reviewed https://github.com/snapcore/snapd/pull/6549#pullrequestreview-210608273 | 11:10 |
| mup | PR #6549: apparmor: support AppArmor 2.13 <Created by jdstrand> <https://github.com/snapcore/snapd/pull/6549> | 11:10 |
| mvo | zyga: I answered some questions, please let me know if its sufficient for merge or if you would prefer more info (it seems the open issues can be done in a followup maybe?) | 11:14 |
| zyga | looking | 11:14 |
| zyga | mvo: replied | 11:15 |
| zyga | mvo: can we add the snapd part and then merge it? | 11:15 |
| zyga | mvo: unless you think my questions about profileIsRemovableOnCoreSetup warrant more time | 11:16 |
| mvo | zyga: good question, I'm not super happy with the function but I played around and all my attempts to change it just made it different but not "better" | 11:17 |
| zyga | mvo: I'd split it to spearate checks per line | 11:17 |
| zyga | with comment that explains what is being matched | 11:17 |
| zyga | unlike one very long line | 11:18 |
| mvo | zyga: if you have an idea, could you please do it and add a comment what it would look like? | 11:18 |
| zyga | sure | 11:18 |
| zyga | doing that now | 11:18 |
| mvo | ta | 11:20 |
| zyga | mvo: I tried and none of my ideas were any better | 11:28 |
| zyga | mvo: I'll defer to you in case you want to land and release | 11:28 |
| === sparkieg` is now known as sparkiegeek | ||
| mvo | zyga: thank you, I had the same experience, might be worthwhile to add a note in the PR (or a FIXME in the code) that it would be nice to find something nicer looking | 11:34 |
| mvo | zyga: but otherwise I think we are good to merge and can polish in a followup :) (sorry, a bit in OCD mode to get a first 2.38~pre1 out) | 11:35 |
| zyga | mvo: there's always .1 | 11:35 |
| zyga | ;-) | 11:35 |
| zyga | (right?) | 11:35 |
| mvo | zyga: *cough* | 11:37 |
| mvo | zyga: in this case a ~pre2 ,) | 11:37 |
| zyga | I, for one, embrace the trigger happy merge because we need to get faster, not slower | 11:38 |
| mborzecki | damn spread snap-seccomp spread test, it's passing on arch nowm but failing on ubuntu still for some reason | 11:42 |
| zyga | mborzecki: hmm? | 11:43 |
| mborzecki | and debian sid doesn't build because we're patching the source code | 11:43 |
| mborzecki | zyga: tests/regression/lp-1805838 is failing with the seccomp branch, even with the fixes i added | 11:43 |
| zyga | oh | 11:44 |
| zyga | that's serious | 11:44 |
| pedronis | yes, we do need to patch the patches | 11:53 |
| pedronis | at least now they are in the same tree | 11:53 |
| mvo | mborzecki: what PR is that? | 11:54 |
| mborzecki | mvo: #6485 | 11:54 |
| mup | PR #6485: interfaces/seccomp: regenerate changed profiles only <β Blocked> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/6485> | 11:54 |
| mborzecki | mvo: no worries, i'll update those, but it took a while to figure out what's happening there | 11:55 |
| mvo | mborzecki: thank you! | 11:55 |
| mup | PR snapd#6549 closed: apparmor: support AppArmor 2.13 <Created by jdstrand> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/6549> | 11:55 |
| mborzecki | btw. fedora packaging does sed to place that libseccomp-go import, we should probably use the same patch as for debian there instead | 11:56 |
| pedronis | mvo: do we need to keep the configure hook in core in sync with the code in snapd? (it's mostly not used on classic), and if you get a new core you get also a new snapd | 11:58 |
| pedronis | mvo: Q is about this: https://github.com/snapcore/core/pull/102 | 11:58 |
| mup | PR core#102: hooks: add force_turbo to PI_CONFIG_KEYS <Created by zyga> <https://github.com/snapcore/core/pull/102> | 11:58 |
| mvo | pedronis: I think we don't, we do it all internally now, don't we? | 12:00 |
| pedronis | mvo: yes, that's why the question, I wonder if we can drop it now completely? or make it empty? | 12:00 |
| pedronis | mvo: we needed for a while for reverts, during the transition | 12:01 |
| mvo | pedronis: yeah, I think making it empty is the right move | 12:02 |
| mvo | pedronis: lets add a card, should be trivial | 12:02 |
| pstolowski | dammit, power outage here | 12:02 |
| pedronis | mvo: https://trello.com/c/IakOtfSS/183-replace-hooks-configure-in-core-with-an-empty-one | 12:12 |
| zyga | brb | 12:12 |
| mvo | pedronis: ta | 12:13 |
| mvo | pedronis: https://github.com/snapcore/snapd/blob/release/2.38/packaging/ubuntu-16.04/changelog has the 2.38~pre1 changelog | 12:14 |
| pedronis | mvo: thx, will look at in a little bit | 12:15 |
| mvo | pedronis: no rush, its a ~pre1 | 12:15 |
| mup | PR snapd#6564 opened: cmd/snap, tests: refactor info to unify handling of 'direct' snaps <Created by chipaca> <https://github.com/snapcore/snapd/pull/6564> | 12:29 |
| pedronis | mvo: I looked at the new things since the last list, I was aware of most of them, I also double checked a couple of snap-confine stuff | 12:32 |
| mborzecki | pstolowski: any storms cooming your way today? | 12:33 |
| pstolowski | mborzecki: nope | 12:34 |
| mborzecki | mvo: zyga: i've updated the seccomp patch, please take a look if that's fine by you or should i add a new one on top of the current series | 12:34 |
| zyga | mborzecki: sure, give me a sec to look | 12:34 |
| mborzecki | pstolowski: well, the season has already started :P | 12:34 |
| === ricab is now known as ricab|lunch | ||
| pstolowski | mborzecki: in that case it's a maintenance in the nearby area | 12:46 |
| zyga | mborzecki: hmmm | 12:58 |
| zyga | mborzecki: snap-seccomp version must go into system key, otherwise snapd update with new snap-seccomp won't trigger a regeneration | 12:59 |
| mborzecki | zyga: build id will change | 13:00 |
| mborzecki | oh, w8 | 13:00 |
| mborzecki | hm | 13:00 |
| zyga | oh, perhaps that is sufficient? | 13:00 |
| mborzecki | nvm, need to look at it again | 13:01 |
| pedronis | mborzecki: I mentioned something related in the PR | 13:01 |
| mborzecki | off to pick up the kids | 13:03 |
| zyga | mborzecki: review sent | 13:08 |
| mup | PR snapd#6324 closed: Fixing socket permissions on 4.15 kernels <Created by kubiko> <Merged by jdstrand> <https://github.com/snapcore/snapd/pull/6324> | 13:29 |
| jdstrand | zyga: I think I answered your questions in PR 6549 | 13:37 |
| mup | PR #6549: apparmor: support AppArmor 2.13 <Created by jdstrand> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/6549> | 13:37 |
| jdstrand | mvo, zyga: fwiw, I rewrote that function a bunch of times too. see my comment on why I landed on what I did | 13:40 |
| jdstrand | joedborg: ok, can you at least make a fix to it and say that it 'architectures: [ all ]' (it doesn't ship any binaries). that will make it easier to deal with reviews, but you should also request classic confinement in the forum since nothing passes review atm | 13:45 |
| joedborg | okay jdstrand | 13:46 |
| pedronis | jdstrand: I asked a question there | 13:46 |
| === ricab|lunch is now known as ricab | ||
| zyga | jdstrand: thank you, looking | 13:55 |
| jdstrand | pedronis: answered | 13:56 |
| zyga | jdstrand: thank you for the answers | 13:57 |
| zyga | jdstrand: note that some of my questions and the rfc branch was trigged by my unfamiliarity with 2.13 | 13:57 |
| jdstrand | I rewrote this a bunch of ways and like mvo couldn't come up with something better (in my mind). I picked one; classic bikeshed | 13:57 |
| jdstrand | happy to do a followup PR if there is something clearly better | 13:57 |
| zyga | jdstrand: tumbleweed is using 2.13 now so I'm learing about it | 13:58 |
| jdstrand | zyga: it really isn't that different | 13:58 |
| zyga | .features file is fun | 13:58 |
| zyga | lisp but with braces instead of parentheses ;) | 13:58 |
| jdstrand | zyga: the only thing that 2.13 is different about is it creates a hash of the features and kernel abi as a subdir of the cache dir and puts everything in there | 13:59 |
| zyga | jdstrand: oh, interesting, so 2.13 already has cache populated with snapd stuff | 13:59 |
| jdstrand | zyga: the unified directory is not dictated by 2.13. that is just something Debian did, but anyone could have | 13:59 |
| zyga | jdstrand: unified directory? | 14:00 |
| zyga | do you mean /var/cache/apparmor? | 14:00 |
| jdstrand | zyga: if tumbleweed has 2.13 and is generating snap policy, I think you'll find that you cannot remove snaps on tumbleweed right now | 14:00 |
| zyga | jdstrand: on TW /var/cache/apparmor/$cache.0/ has stuff like snap-confine.core.1234 | 14:00 |
| zyga | oh! | 14:00 |
| zyga | jdstrand: fun | 14:00 |
| * zyga tries | 14:00 | |
| jdstrand | zyga: yes, /var/cache/apparmor | 14:00 |
| jdstrand | zyga: the cache location is still chosen by the distro. in Debian and soon to be ubuntu 19.04, it is /var/cache/apparmor. it might be that elsewhere. | 14:02 |
| zyga | cannot remove vlc on TW https://www.irccloud.com/pastebin/7HWJOWvZ/ | 14:02 |
| zyga | standup time | 14:02 |
| zyga | but I will dig in a moment | 14:02 |
| jdstrand | zyga: the choosing of the cache location is separate from the forest behavior is all I'm saying. the fact the Debian combined /etc/apparmor.d/cache and /var/cache/apparmor into on unified dir is separate from 2.13 | 14:02 |
| zyga | Chipaca: ^ is this expected? | 14:03 |
| zyga | jdstrand: ah, I see now | 14:03 |
| zyga | jdstrand: thank you for this | 14:03 |
| jdstrand | np | 14:03 |
| zyga | jdstrand: I am iterating on responses and patches based on your feedback, thank you for that as well | 14:03 |
| Chipaca | zyga: no, that's not expected | 14:03 |
| jdstrand | zyga: I maintain that it is an isolated changed in just Unload that breaking out a bunch of code and added machinery is probably not worth it | 14:04 |
| jdstrand | zyga: but I'll let others decide that | 14:04 |
| jdstrand | adding* | 14:04 |
| Chipaca | zyga: what does βfind /var/snap/vlc -lsβ print out | 14:05 |
| jdstrand | zyga: note that the vls removal is a different error than what I saw with 2.13 apparmor on Ubuntu. It seems you are hitting a different error earlier than when snapd tries to remove the cache files but can't | 14:06 |
| jdstrand | vlc* | 14:06 |
| zyga | zyga@yantra:~> sudo find /var/snap/vlc -ls | 14:06 |
| zyga | 2234362 4 drwxr-xr-x 3 root root 4096 mar 5 15:01 /var/snap/vlc | 14:06 |
| zyga | 2367702 4 drwxr-xr-x 2 root root 4096 lis 6 22:06 /var/snap/vlc/555 | 14:06 |
| zyga | ack | 14:07 |
| Chipaca | zyga: so the question is why did os.RemoveAll fail to remove that | 14:07 |
| zyga | Chipaca: note that this is reproducible! | 14:08 |
| zyga | undo works | 14:08 |
| zyga | so ... good? | 14:08 |
| zyga | https://www.irccloud.com/pastebin/qdZMzJ6Q/ | 14:08 |
| Chipaca | zyga: undo of remove does not work | 14:08 |
| zyga | it's interesting that we snap disconnect | 14:08 |
| zyga | why do we do that? CC pstolowski | 14:08 |
| pstolowski | zyga: because of hooks that may run on the other end of connection | 14:09 |
| zyga | aha | 14:09 |
| pstolowski | (interface hooks) | 14:10 |
| zyga | we should be able to optimize away the ones that don't have any | 14:10 |
| pstolowski | zyga: +1 | 14:10 |
| Chipaca | zyga: so the question is why do you have revision 555 there | 14:10 |
| zyga | dunno | 14:10 |
| Chipaca | (also maybe we should be more resilient about this) | 14:10 |
| zyga | why cannot snapd remove it? | 14:10 |
| Chipaca | zyga: because it doesn't try to | 14:10 |
| Chipaca | it doesn't expect that to be there | 14:11 |
| zyga | aaah | 14:11 |
| pstolowski | zyga: actually we probably don't create hook tasks if not needed already | 14:11 |
| zyga | I see | 14:11 |
| Chipaca | zyga: it removes all the revisions, and then the parent | 14:11 |
| zyga | pstolowski: I'm pretty sure disconnecting stuff from vlc is not needed when you remove the snap | 14:11 |
| zyga | and all the connections go to core | 14:11 |
| Chipaca | zyga: vlc has mpris which doesn't go to core, fwiw | 14:11 |
| zyga | Chipaca: yes but mpris is not connected | 14:12 |
| zyga | https://www.irccloud.com/pastebin/tJtbkHRy/ | 14:15 |
| zyga | Chipaca: it's from october | 14:15 |
| zyga | but this is my dev machine so anything is possible | 14:15 |
| Chipaca | zyga: snap list --all ? | 14:15 |
| zyga | tried it, not there | 14:16 |
| Chipaca | I can't find an obvious way of repro'ing it | 14:22 |
| Chipaca | maybe there's a cleanup missing | 14:22 |
| Chipaca | ? | 14:22 |
| zyga | no worries, we know it may happen | 14:22 |
| zyga | shall I report it for tracking: | 14:22 |
| Chipaca | that is, a refresh or install that fails and doesn't un-mkdir? | 14:22 |
| zyga | ? | 14:22 |
| Chipaca | hah! | 14:28 |
| Chipaca | zyga: yes indeed we do! | 14:28 |
| Chipaca | zyga: o/s/b/link.go, updateCurrentSymlinks | 14:28 |
| Chipaca | zyga: happily does a MkdirAll(snap.DataDir()) | 14:28 |
| Chipaca | zyga: and then more stuff | 14:28 |
| Chipaca | zyga: and that more stuff can fail | 14:28 |
| Chipaca | zyga: and there is no clean up of the mkdir on fail | 14:29 |
| zyga | good | 14:29 |
| zyga | I'm happy we're happy now :) | 14:29 |
| * Chipaca looks around | 14:29 | |
| Chipaca | we are? | 14:29 |
| zyga | shallow eyeballs all that stuff | 14:29 |
| mup | PR snapd#6565 opened: snap: remove obsolete license-* fields in the yaml <Created by mvo5> <https://github.com/snapcore/snapd/pull/6565> | 14:39 |
| Chipaca | zyga: i'll push a pr in a bit | 14:40 |
| Chipaca | first i need to go move my car | 14:40 |
| Chipaca | oh! mvo: tomorrow I need to go to do an anual paperwork thing for kid #2 (kid #1's is in a couple of weeks), probably won't make it to the standup even | 14:40 |
| Chipaca | i'll file for a half-day later | 14:41 |
| pedronis | jdstrand: hi, could you adjust the summaries in #5644 , then it could be landed as per agreement | 14:42 |
| mup | PR #5644: interfaces: add audio-playback/audio-record and make pulseaudio manually connect <Created by jdstrand> <https://github.com/snapcore/snapd/pull/5644> | 14:42 |
| pedronis | assuming the tests follow-up | 14:42 |
| jdstrand | pedronis: so, I was wanting to ask you about that. I did make some changes before your last comment ot make changes | 14:43 |
| jdstrand | pedronis: what is it you are looking for exactly? | 14:43 |
| jdstrand | (and yes, when tests can be written, I'll write them) | 14:43 |
| pedronis | jdstrand: https://github.com/snapcore/snapd/pull/5644/files#r212674777 | 14:44 |
| mup | PR #5644: interfaces: add audio-playback/audio-record and make pulseaudio manually connect <Created by jdstrand> <https://github.com/snapcore/snapd/pull/5644> | 14:44 |
| jdstrand | pedronis: oh, that change. ack. I thought this was something else | 14:45 |
| mvo | Chipaca: thanks | 14:45 |
| mvo | Chipaca: no worries | 14:45 |
| pedronis | jdstrand: to be fair, I see we use your spelling quite a bit | 14:46 |
| pedronis | well, not quite a bit | 14:47 |
| pedronis | but in a few cases | 14:47 |
| pedronis | degville: interestingly there are a few things that are document for snapcraft.yaml but not snap.yaml :) | 14:49 |
| pedronis | degville: https://forum.snapcraft.io/t/snapcraft-app-and-service-metadata/8335 | 14:49 |
| degville | pedronis: yeah, I've been pinged about updates to snapcraft.yaml but didn't think to push them back to the snap.yaml doc. | 14:50 |
| zyga | Lunch | 14:50 |
| zyga | Chipaca: thank you! :-) | 14:50 |
| pedronis | jdstrand: pulseaudio is interesting because the summary in the code talks about operating as the service, but the doc doesn't https://forum.snapcraft.io/t/the-pulseaudio-interface/7906 | 14:51 |
| pedronis | jdstrand: while I would, sort of, expect the reverse | 14:52 |
| pedronis | jdstrand: I mean the simple summary to explain the plug/consumer POV, the "extended" docs to explain that the slot can be implemented by apps | 14:54 |
| mborzecki | zyga: pedronis: under the assumption that snapd and snap-seccomp are in sync, i think we don't need to include snap-seccomp version in version-info, just the library version and the hash are enough | 14:55 |
| zyga | mborzecki: what about system-key? | 14:55 |
| zyga | mborzecki: I agree with your statement | 14:55 |
| zyga | mborzecki: but system-key correctness is essential for correctness | 14:55 |
| mborzecki | zyga: we still need that, i.e. put version-info in system-key | 14:55 |
| pedronis | ? | 14:56 |
| mborzecki | zyga: what i mean we don't necessarily need to include the compiler version in the output | 14:56 |
| pedronis | I'm missing something | 14:56 |
| mborzecki | (snap-seccomp being the compiler) | 14:56 |
| zyga | pedronis: I think mborzecki is working with the assumption that we have snapd build-id in system-key | 14:56 |
| zyga | though | 14:56 |
| zyga | to be fair | 14:57 |
| zyga | I think it's not correct | 14:57 |
| mborzecki | which we have | 14:57 |
| zyga | once we get reproducible builds | 14:57 |
| zyga | snapd can remain unchanged | 14:57 |
| zyga | while snap-seccomp can change | 14:57 |
| pedronis | yes | 14:57 |
| zyga | so in that sense it's not fully sufficient | 14:57 |
| pedronis | that's the part that is delicate | 14:57 |
| pedronis | I sort of think we are trying to avoid adding a number | 14:57 |
| pedronis | at the expense of subtle bugs | 14:57 |
| mborzecki | hm maybe we should then use the build-id of snap-seccomp too | 14:58 |
| zyga | pedronis: I agree | 14:58 |
| zyga | pedronis: let's do it cleanly | 14:58 |
| zyga | mborzecki: that would be okay | 14:58 |
| mborzecki | so version-info would be: <build-id> <library-version> <hash> | 14:58 |
| zyga | and I prefer the build-id over a version | 14:58 |
| zyga | versions are easy to misuse | 14:58 |
| pedronis | that's fine, in the end this an optimisation over always regen | 14:59 |
| mborzecki | yup | 14:59 |
| mborzecki | ok, build-id it is | 14:59 |
| zyga | pstolowski: https://github.com/snapcore/snapd/pull/6563 | 15:00 |
| mup | PR #6563: ctlcmd/tests: tests tweaks (followup to #6322) <Simple π> <Created by stolowski> <https://github.com/snapcore/snapd/pull/6563> | 15:00 |
| pstolowski | zyga: k, merging, thx | 15:02 |
| mup | PR snapd#6563 closed: ctlcmd/tests: tests tweaks (followup to #6322) <Simple π> <Created by stolowski> <Merged by stolowski> <https://github.com/snapcore/snapd/pull/6563> | 15:02 |
| jdstrand | pedronis: ok, https://github.com/snapcore/snapd/pull/5644 updated. note, it is for master/2.38 and *not* for 2.38 (I need time to do the other supporting work in electron, snapcraft, etc, etc) | 15:05 |
| jdstrand | meh | 15:05 |
| mup | PR #5644: interfaces: add audio-playback/audio-record and make pulseaudio manually connect <Created by jdstrand> <https://github.com/snapcore/snapd/pull/5644> | 15:05 |
| jdstrand | master/2.39* | 15:05 |
| jdstrand | this is 2.39 material, not 2.38 (so many typos this morning :) | 15:05 |
| jdstrand | but not in the PR! ;) | 15:06 |
| zyga | jdstrand: I nuked stdio from https://github.com/snapcore/snapd/pull/6498 | 15:08 |
| mup | PR #6498: cmd/libsnap: add cgroup-pids-support module <Created by zyga> <https://github.com/snapcore/snapd/pull/6498> | 15:08 |
| jdstrand | ack, thanks | 15:08 |
| === chihchun is now known as chihchun_afk | ||
| mvo | sil2100: https://github.com/snapcore/core/pull/103 is probably something for you :) | 15:16 |
| mup | PR core#103: hooks: apt warning should return an error <Created by townsend2010> <https://github.com/snapcore/core/pull/103> | 15:16 |
| sil2100 | mvo: hey! Oh, it's for core? | 15:17 |
| mvo | sil2100: yeah | 15:17 |
| mvo | sil2100: oh, sorry | 15:17 |
| sil2100 | But yeah, it makes sense :) | 15:17 |
| sil2100 | +1 | 15:17 |
| mvo | sil2100: yeah, its for core, I guess that means still our turf | 15:17 |
| mvo | sil2100: sorry, ENEEDMORETEA :) | 15:18 |
| mup | PR core#103 closed: hooks: apt warning should return an error <Created by townsend2010> <Merged by mvo5> <https://github.com/snapcore/core/pull/103> | 15:18 |
| sil2100 | mvo: hm, do we actually have a warning wrapper like this in core18? Would have to check | 15:19 |
| * sil2100 is too deep in point releases in the last weeks | 15:19 | |
| mvo | sil2100: oh, indeed - are they all done now? or one more to go? | 15:21 |
| ChrisTownsend | mvo: Hey, thanks for merging that. It will help us on Multipass and our support for core. | 15:21 |
| ChrisTownsend | I didn't see an analogous script in the core18 code though. | 15:21 |
| sil2100 | One in progress, hopefully the last one ;) | 15:22 |
| mvo | ChrisTownsend: thank you! we may need to add one for core18 too, I will sync with sil2100 on that (but no worries, will also exit 1) | 15:27 |
| ChrisTownsend | mvo: Sure, glad to contribute:) | 15:27 |
| mup | PR snapd#6566 opened: interface: avahi-observe: Fixing socket permissions on 4.15 kernels wβ¦ <Simple π> <Created by jdstrand> <https://github.com/snapcore/snapd/pull/6566> | 15:30 |
| jdstrand | mvo: hey, just as a heads up, I don't have any policy updates for 2.38 other than getting ^ in for ondra | 15:36 |
| ondra | jdstrand thank you :) | 15:38 |
| jdstrand | ondra: you're welcome. I owed it to you for the delay in the PR review ;) | 15:39 |
| ondra | jdstrand no worries, it only came back when ogra started to use 4.15 kernel on some of his new boards | 15:40 |
| jdstrand | cool | 15:40 |
| jdstrand | glad it hasn't been a persistent pain point | 15:40 |
| ogra | yeah, i have a bunch of new rockchip boards (and images) | 15:40 |
| ogra | using ubuntu-bionic.git (and -disco) for the kernels | 15:40 |
| jdstrand | neat | 15:41 |
| Chipaca | mvo: you around? | 15:48 |
| Chipaca | zyga: v | 15:50 |
| zyga | v? | 15:51 |
| zyga | ah | 15:51 |
| zyga | incoming :) | 15:51 |
| * Chipaca waits for mup | 15:51 | |
| mup | PR snapd#6567 opened: overlord/snapstate/backend: make LinkSnap clean up more <Created by chipaca> <https://github.com/snapcore/snapd/pull/6567> | 15:51 |
| Chipaca | zyga: ^ | 15:51 |
| zyga | heh :) | 15:51 |
| Chipaca | zyga: there's still a bug there, as pointed out in the pr | 15:52 |
| pedronis | ondra: hi, btw you still a couple of PRs open where jdstrand left feedback asking changes | 15:52 |
| Chipaca | and it might be something that's impacting people | 15:52 |
| Chipaca | because, it aligns with some funny "why is it thinking it needs to boot this" errors | 15:52 |
| ondra | pedronis yep, on my list. I was on hols and then tradeshow and only back in the office this week | 15:52 |
| pedronis | ondra: ok | 15:52 |
| zyga | break, time to take the dog out again | 16:00 |
| mvo | Chipaca: in meetings | 16:01 |
| * Chipaca hugs mvo | 16:01 | |
| mvo | Chipaca: how can I help you? | 16:01 |
| Chipaca | mvo: wondering if there's an easy way to 'undo' boot.SetNextBoot(info) | 16:01 |
| Chipaca | mvo: in https://github.com/snapcore/snapd/pull/6567/files#diff-1fdad998bbd61f4345491c0b17b6585eR115 | 16:02 |
| mup | PR #6567: overlord/snapstate/backend: make LinkSnap clean up more <Created by chipaca> <https://github.com/snapcore/snapd/pull/6567> | 16:02 |
| mvo | Chipaca: I think so, just unsetting snap_try_core should be enough | 16:03 |
| mvo | Chipaca: still in a meeting so a bit terse | 16:03 |
| Chipaca | mvo: no worries :-) | 16:03 |
| mup | PR snapd#6568 opened: overlord/snapstate: fix restoring of "old-current" revision config in undoLinkSnap <Created by stolowski> <https://github.com/snapcore/snapd/pull/6568> | 16:14 |
| kenvandine | mborzecki: hey, i'm playing around with parallel installs and ran into an issue. mvo said i should raise it here with you | 16:24 |
| kenvandine | i installed gedit from edge as gedit_master | 16:24 |
| kenvandine | the gtk-common-themes content interfaces all connected fine | 16:24 |
| kenvandine | but the gnome-3-28-1804 content interface didn't | 16:25 |
| kenvandine | i manually connected it and it seems to have failed | 16:25 |
| Chipaca | kenvandine: what's 'snap tasks' of the failed change? (maybe --last=connect) | 16:32 |
| kenvandine | Done today at 11:21 EST today at 11:21 EST Connect gedit_master:gnome-3-28-1804 to gnome-3-28-1804:gnome-3-28-1804 | 16:32 |
| kenvandine | that was my manual connect | 16:32 |
| kenvandine | but the content is not mounted in the snap | 16:33 |
| Chipaca | ah (?) | 16:33 |
| kenvandine | i installed another parallel snap and that connected everything just fine | 16:48 |
| mborzecki | kenvandine: let me try that locally | 16:53 |
| kenvandine | i'm guessing if i remove gedit_master and install again it might work | 16:54 |
| kenvandine | since other snaps are working | 16:54 |
| kenvandine | but i'll leave it broken in case i can collect any useful info for you | 16:54 |
| mborzecki | kenvandine: it ought to be deterministic :P | 16:54 |
| kenvandine | *ought* to be | 16:55 |
| mborzecki | kenvandine: can you upload `snap change` of that install to pastebin? | 16:56 |
| kenvandine | mborzecki: you mean the line from snap changes? | 16:57 |
| mborzecki | kenvandine: the output of `snap change <that-change-id>` | 16:58 |
| kenvandine | oh | 16:58 |
| kenvandine | never done that before :) | 16:58 |
| mborzecki | installed here just fine | 16:58 |
| kenvandine | http://paste.ubuntu.com/p/sGxcwknFRp/ | 16:58 |
| kenvandine | yeah, i've since installed several other gnome snaps fine | 16:58 |
| kenvandine | looking at the change, it doesn't even list gnome-3-28-1804 | 16:59 |
| mborzecki | kenvandine: ok, can you run `snap changes` locate the manual connect that failed and paste snap changes <that-id> ? | 17:02 |
| Chipaca | mborzecki: snap tasks <that id> | 17:03 |
| Chipaca | or snap change <that id> | 17:03 |
| kenvandine | mborzecki: Done today at 11:20 EST today at 11:20 EST Connect gedit_master:gnome-3-28-1804 to gnome-3-28-1804:gnome-3-28-1804 | 17:03 |
| kenvandine | it didn't think it failed | 17:03 |
| Chipaca | kenvandine: anything interesting in jouranlctl -u snapd ? | 17:03 |
| Chipaca | or some other command that rhymes with that | 17:03 |
| kenvandine | Mar 05 11:20:03 x230 snapd[1778]: api.go:1077: Installing snap "gedit_master" revision unset | 17:04 |
| kenvandine | only thing around the same time | 17:04 |
| kenvandine | the interface is currently connected, but the mount point is empty | 17:05 |
| mborzecki | ah ok, so the content interface is conencted but the data is nto visible? | 17:05 |
| kenvandine | right | 17:05 |
| kenvandine | mborzecki: although it didn't connect at install | 17:05 |
| kenvandine | like it should have | 17:06 |
| kenvandine | i manually connected it | 17:06 |
| mborzecki | kenvandine: ok, maybe try this, sudo nsenter -m/run/snapd/ns/gedit_master.mnt /bin/findmnt and paste the output | 17:11 |
| kenvandine | mborzecki: http://paste.ubuntu.com/p/xHmnP6mSgT/ | 17:15 |
| mborzecki | kenvandine: nothing stands out, can you also paste the contents of /var/lib/snapd/mount/snap.gedit_master.fstab ? | 17:27 |
| kenvandine | mborzecki: http://paste.ubuntu.com/p/5m5JpD2pgw/ | 17:29 |
| mborzecki | kenvandine: the content mount points of gtk-common-themes and gnome-* seem to be populated here just fine, i mean mounted and there's data inside | 17:29 |
| kenvandine | yeah | 17:30 |
| kenvandine | same here for all the other snaps | 17:30 |
| kenvandine | that i installed since | 17:30 |
| kenvandine | so i suspect if i remove and reinstall it will be fine | 17:30 |
| kenvandine | this was the first snap i installed after enabling parallel installs on core | 17:31 |
| mborzecki | kenvandine: ok, let's see if we can reproduce it at least, can you stop gedit, run sudo /usr/lib/snapd/snap-discard-ns gedit_master and then run `SNAPD_DEBUG=1 SNAP_CONFINE_DEBUG=1 snap run gedit_master` and paste the output | 17:35 |
| kenvandine | mborzecki: https://pastebin.ubuntu.com/p/XCKtXgRSYx/ | 17:38 |
| kenvandine | mborzecki: it actually ran fine | 17:38 |
| mborzecki | kenvandine: idk, things seem to look fine in the outputs, probably be good if zyga could take a look too | 17:44 |
| kenvandine | now it all works fine | 17:44 |
| kenvandine | so something there cleared it up | 17:44 |
| kenvandine | i guess the snap-discard-ns ? | 17:44 |
| mborzecki | kenvandine: yes, that tool removes the mount namespace and it will be recreated on the next snap run | 17:45 |
| zyga | kenvandine: yes, you can use it to get a clean slate | 17:55 |
| zyga | pedronis: hey, do you want to have a look at https://github.com/snapcore/snapd/pull/6498 before it lands? | 17:56 |
| mup | PR #6498: cmd/libsnap: add cgroup-pids-support module <Created by zyga> <https://github.com/snapcore/snapd/pull/6498> | 17:56 |
| pedronis | zyga: I think it was alright, do we have something using it yet? or is that coming? | 17:56 |
| zyga | pedronis: that's upcoming in another branch | 17:57 |
| zyga | pedronis: just separated for different review people usually | 17:57 |
| zyga | pedronis: (and shorter) | 17:57 |
| zyga | pedronis: though it's already used | 17:57 |
| zyga | pedronis: because of freezer code now beign based on this | 17:57 |
| pedronis | yes, that I understand | 17:57 |
| pedronis | ok | 17:58 |
| zyga | one question, I forgot to add copyright headers | 17:58 |
| zyga | s/question/change/ | 17:58 |
| zyga | ok, pushed | 18:06 |
| * zyga EODs | 18:31 | |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!