[00:03] <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:54] <mwhudson> behold my snazzy progress bar! https://asciinema.org/a/aTbeNl5BnF2jtbWlXZT8ed7eE
[05:35] <zyga> Hello
[06:07] <mborzecki> morning
[06:08] <zyga> hey mborzecki  :-)
[06:08] <zyga> how are you doing>?
[06:08] <mborzecki> zyga: hey
[06:08] <mborzecki> zyga: fine, 'connections' landed yesterday :)
[06:09] <zyga> I saw, nice work  :)
[06:10] <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:11] <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:37] <zyga> re
[06:37] <zyga> mborzecki: looks l like core18 + snapd not working
[06:38] <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:39] <mborzecki> zyga: stuck looking at some review now
[06:39] <zyga> ok
[06:39] <mborzecki> zyga: but that's interesting
[08:03] <pstolowski> mornings
[08:05] <mvo> good morning pstolowski
[08:15] <zyga> Hey Paweł, Michael
[08:18] <mborzecki> pstolowski, mvo: morning guys
[08:19] <mvo> hey zyga and mborzecki !
[08:24] <mvo> 6492 needs a second review (its on the 2.38 critical path)
[08:25] <mvo> pedronis: 6356 looks good to me, unless you want to do something with it I will merge it
[08:26] <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:32] <pedronis> mvo: it's fine to merge for me,  but make sure people are aware you would like follow ups
[08:34] <mvo> pedronis: I will wait for john - followups are not strictly needed, its mostly ideas/suggestions nothing strictly required
[08:34] <pedronis> ok
[08:37] <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:41] <pedronis> mborzecki: we compare it in a couple of places to see if it's higher than some version
[08:42] <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:44] <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:45] <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:47] <pstolowski> pedronis: i noted these comments and will address in a followup, these were cosmetics
[08:48] <zyga> Hey, with dog at vet, I’ll start around 10 today
[08:49] <zyga> Thanks for merging mvo
[08:51] <doko> how is mvo merged?
[08:52] <zyga> Mvo is very dedicated, one could say merged with is duties ;-)
[08:54] <mvo> haha
[09:31] <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:36] <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:47] <mvo> 6492 needs a second review
[09:48] <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:49] <Chipaca> mwhudson: nice!
[09:50] <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:51] <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:52] <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:53] <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:54] <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:57] <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:58] <Chipaca> mvo: we don't block disabling snapd though
[09:59] <Chipaca> mvo: so you could have snapd installed and disabled
[09:59] <Chipaca> mvo: (add TypeSnapd to snapstate's canDisable)
[10:00] <Chipaca> mvo: why am i not writing this on github
[10:05] <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:06] <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:07] <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:08] <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:10] <pedronis> Chipaca: I created a card for mvo
[10:13] <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:14] <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:16] <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:30] <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:31] <pedronis> mborzecki: we control both sides, no?
[10:31] <mborzecki> pedronis: yes, so it's unlinkely to happen
[10:32] <mborzecki> pedronis: however unlikely, should this fail, backend will not initialize and snapd won't start
[10:33] <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:34] <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:48] <mup> PR snapd#6563 opened: ctlcmd/tests: tests tweaks (followup to #6322) <Simple 😃> <Created by stolowski> <https://github.com/snapcore/snapd/pull/6563>
[10:50] <pstolowski> mvo: ^ trivial per your comment to 6322
[10:55] <mvo> pstolowski: thanks, I have a look in a wee bit
[11:10] <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:14] <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:15] <zyga> mvo: replied
[11:15] <zyga> mvo: can we add the snapd part and then merge it?
[11:16] <zyga> mvo: unless you think my questions about profileIsRemovableOnCoreSetup warrant more time
[11:17] <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:18] <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:20] <mvo> ta
[11:28] <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:34] <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:35] <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:37] <mvo> zyga: *cough*
[11:37] <mvo> zyga: in this case a ~pre2 ,)
[11:38] <zyga> I, for one, embrace the trigger happy merge because we need to get faster, not slower
[11:42] <mborzecki> damn spread snap-seccomp spread test, it's passing on arch nowm but failing on ubuntu still for some reason
[11:43] <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:44] <zyga> oh
[11:44] <zyga> that's serious
[11:53] <pedronis> yes, we do need to patch the patches
[11:53] <pedronis> at least now they are in the same tree
[11:54] <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:55] <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:56] <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:58] <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>
[12:00] <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:01] <pedronis> mvo: we needed for a while for reverts, during the transition
[12:02] <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:12] <pedronis> mvo: https://trello.com/c/IakOtfSS/183-replace-hooks-configure-in-core-with-an-empty-one
[12:12] <zyga> brb
[12:13] <mvo> pedronis: ta
[12:14] <mvo> pedronis: https://github.com/snapcore/snapd/blob/release/2.38/packaging/ubuntu-16.04/changelog has the 2.38~pre1 changelog
[12:15] <pedronis> mvo: thx, will look at in a little bit
[12:15] <mvo> pedronis: no rush, its a ~pre1
[12:29] <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:32] <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:33] <mborzecki> pstolowski: any storms cooming your way today?
[12:34] <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:46] <pstolowski> mborzecki: in that case it's a maintenance in the nearby area
[12:58] <zyga> mborzecki: hmmm
[12:59] <zyga> mborzecki: snap-seccomp version must go into system key, otherwise snapd update with new snap-seccomp won't trigger a regeneration
[13:00] <mborzecki> zyga: build id will change
[13:00] <mborzecki> oh, w8
[13:00] <mborzecki> hm
[13:00] <zyga> oh, perhaps that is sufficient?
[13:01] <mborzecki> nvm, need to look at it again
[13:01] <pedronis> mborzecki: I mentioned something related in the PR
[13:03] <mborzecki> off to pick up the kids
[13:08] <zyga> mborzecki: review sent
[13:29] <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:37] <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:40] <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:45] <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:46] <joedborg> okay jdstrand
[13:46] <pedronis> jdstrand: I asked a question there
[13:55] <zyga> jdstrand: thank you, looking
[13:56] <jdstrand> pedronis: answered
[13:57] <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:58] <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:59] <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
[14:00] <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:02] <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:03] <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:04] <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:05] <Chipaca> zyga: what does ‘find /var/snap/vlc -ls’ print out
[14:06] <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:07] <zyga> ack
[14:07] <Chipaca> zyga: so the question is why did os.RemoveAll fail to remove that
[14:08] <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:09] <pstolowski> zyga: because of hooks that may run on the other end of connection
[14:09] <zyga> aha
[14:10] <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:11] <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:12] <zyga> Chipaca: yes but mpris is not connected
[14:15] <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:16] <zyga> tried it, not there
[14:22] <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:28] <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:29] <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:39] <mup> PR snapd#6565 opened: snap: remove obsolete license-* fields in the yaml <Created by mvo5> <https://github.com/snapcore/snapd/pull/6565>
[14:40] <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:41] <Chipaca> i'll file for a half-day later
[14:42] <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:43] <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:44] <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:45] <jdstrand> pedronis: oh, that change. ack. I thought this was something else
[14:45] <mvo> Chipaca: thanks
[14:45] <mvo> Chipaca: no worries
[14:46] <pedronis> jdstrand: to be fair, I see we use your spelling quite a bit
[14:47] <pedronis> well, not quite a bit
[14:47] <pedronis> but in a few cases
[14:49] <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:50] <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:51] <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:52] <pedronis> jdstrand: while I would, sort of, expect the reverse
[14:54] <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:55] <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:56] <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:57] <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:58] <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:59] <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
[15:00] <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:02] <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:05] <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:06] <jdstrand> but not in the PR! ;)
[15:08] <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:16] <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:17] <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:18] <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:19] <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:21] <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:22] <sil2100> One in progress, hopefully the last one ;)
[15:27] <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:30] <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:36] <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:38] <ondra> jdstrand thank you :)
[15:39] <jdstrand> ondra: you're welcome. I owed it to you for the delay in the PR review ;)
[15:40] <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:41] <jdstrand> neat
[15:48] <Chipaca> mvo: you around?
[15:50] <Chipaca> zyga: v
[15:51] <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:52] <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
[16:00] <zyga> break, time to take the dog out again
[16:01] <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:02] <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:03] <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:14] <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:24] <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:25] <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:32] <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:33] <kenvandine> but the content is not mounted in the snap
[16:33] <Chipaca> ah (?)
[16:48] <kenvandine> i installed another parallel snap and that connected everything just fine
[16:53] <mborzecki> kenvandine: let me try that locally
[16:54] <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:55] <kenvandine> *ought* to be
[16:56] <mborzecki> kenvandine: can you upload `snap change` of that install to pastebin?
[16:57] <kenvandine> mborzecki: you mean the line from snap changes?
[16:58] <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:59] <kenvandine> looking at the change, it doesn't even list gnome-3-28-1804
[17:02] <mborzecki> kenvandine: ok, can you run `snap changes` locate the manual connect that failed and paste snap changes <that-id> ?
[17:03] <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:04] <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:05] <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:06] <kenvandine> like it should have
[17:06] <kenvandine> i manually connected it
[17:11] <mborzecki> kenvandine: ok, maybe try this, sudo nsenter -m/run/snapd/ns/gedit_master.mnt /bin/findmnt and paste the output
[17:15] <kenvandine> mborzecki: http://paste.ubuntu.com/p/xHmnP6mSgT/
[17:27] <mborzecki> kenvandine: nothing stands out, can you also paste the contents of /var/lib/snapd/mount/snap.gedit_master.fstab ?
[17:29] <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:30] <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:31] <kenvandine> this was the first snap i installed after enabling parallel installs on core
[17:35] <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:38] <kenvandine> mborzecki: https://pastebin.ubuntu.com/p/XCKtXgRSYx/
[17:38] <kenvandine> mborzecki: it actually ran fine
[17:44] <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:45] <mborzecki> kenvandine: yes, that tool removes the mount namespace and it will be recreated on the next snap run
[17:55] <zyga> kenvandine: yes, you can use it to get a clean slate
[17:56] <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:57] <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:58] <pedronis> ok
[17:58] <zyga> one question, I forgot to add copyright headers
[17:58] <zyga> s/question/change/
[18:06] <zyga> ok, pushed
[18:31]  * zyga EODs