[00:07] PR snapcraft#2568 closed: docs: consolidate on a simple HACKING.md === jamesh_ is now known as jamesh [05:06] morning [06:00] Hey mborzecki [06:00] zyga: hey [06:00] zyga: how's your day off coming? :) [06:00] Haha [06:00] I was just wondering the same thing [06:00] Should I take today off for real [06:00] Or work some more to get stuff done [06:01] So many things to work on [06:05] mborzecki: I sent a small patch for spread-shellcheck [06:05] And had an idea on how to fix not MATCH [06:07] We can save MATCH to lib/bin on startup [06:07] Like we do now to match.sh include file [06:07] Since it is on path it will be more flexible [06:08] Then MATCH is no longer a function [06:08] Then we can put “not” on path and fix all tests [06:15] zyga: in other news, got an image built by snap-image (strawman) to boot up to a point where i got console conf :P [06:16] Wooooot [06:16] That is great [06:17] It still feels that landing your changes will take a month [06:17] yeah, and tests and all that [06:18] zyga: if you guys have a system-rescue role that you'd like me to add or something, this coudl be done really easily [06:19] Role as in partition type? [06:19] I think we should talk about how the recovery system is designed, so far, to keep you in the loop [06:19] zyga: role as role in gadget.yaml terms, and whatever that means practically, eg. like system-boot is a bit special [06:19] zyga: and so is system-data [06:20] I think some things there will change [06:20] Though I think that is best for next week [06:37] re, back in the office [06:38] mborzecki: can you do a quick review of https://github.com/snapcore/snapd/pull/6906 [06:38] just a variable rename [06:38] PR #6906: cmd/snap-update-ns: rename ctx to upCtx [06:38] though more importantly I need a review for https://github.com/snapcore/snapd/pull/6903 to get 2.39.1 fix in place [06:38] PR #6903: spread-shellcheck: add support for variants and environment [06:38] the shellcheck change is not perfect [06:38] but it's also not totally incorrect [06:38] and allows my tests to not get stuck on that [07:01] * zyga quick coffee [07:01] mborzecki: if you can review the spread-shellcheck change I will be able to propose the propagation fixes [07:03] I will do some branch gardening soon [07:03] but first coffee [07:04] morning === pstolowski|afk is now known as pstolowski [07:09] Hey :-) [07:19] Interesting https://forum.snapcraft.io/t/snapshots-can-expose-sensitive-data/11493 [07:27] zyga: hmm interesting indeed [07:27] PR snapd#6906 closed: cmd/snap-update-ns: rename ctx to upCtx [07:27] makes me feel that snapshots should be saved back to $HOME [07:27] as anything else is not correct wrt NFS and encryption [07:28] pstolowski: quick trivial https://github.com/snapcore/snapd/pull/6907 [07:28] PR #6907: cmd/snap-update-ns: add several TODO comments [07:28] just comments [07:28] but that's *all* of the refactoring :D [07:28] wooooot [07:28] PR snapd#6907 opened: cmd/snap-update-ns: add several TODO comments [07:29] +1§ [07:30] pstolowski: hey [07:30] o/ [07:40] PR snapd#6894 closed: tests: add "not" command as replacement for "!" in tests [07:44] PR snapd#6908 opened: tests: add "not" command [08:05] mborzecki, pstolowski: how do you feel about ^ not [08:05] I want to build this on top https://github.com/snapcore/snapd/pull/6909 [08:05] PR #6909: spread.yaml,tests: change MATCH and REBOOT to cmds [08:05] and then fix our scripts to use not rather than ! [08:06] PR snapd#6909 opened: spread.yaml,tests: change MATCH and REBOOT to cmds [08:11] zyga: interesting; don't we still need to make it a compound expressionm e.g. { ! "$@" } ? [08:11] no, why? [08:12] ! functions correctly [08:12] note that this executable is not sourced, it's a program that runs without set -e [08:12] zyga: ok, so -e is the key here. i was reading also https://stackoverflow.com/questions/39581150/why-do-i-need-parenthesis-in-bash-set-e-and-negated-return-code/39582012 === icey_ is now known as icey [08:22] I wonder if snapd is going to get any sponsors: https://techcrunch.com/2019/05/23/github-launches-sponsors-lets-you-pay-your-favorite-open-source-contributors/ [08:40] mborzecki: https://github.com/google/pytype [08:40] mborzecki: from that video you linkedto [08:51] mborzecki: updated https://github.com/snapcore/snapd/pull/6903 [08:51] PR #6903: spread-shellcheck: add support for variants and environment [08:55] zyga: nhm, lgtm [08:56] nhm? [09:00] mborzecki: something for you :) https://github.com/snapcore/snapd/pull/6910 [09:00] PR #6910: spread.yaml: use "snap connections" in debug [09:00] PR snapd#6910 opened: spread.yaml: use "snap connections" in debug [09:03] * zyga really gets that coffee now [09:03] after the break I will try to land "not" and "MATCH" as commands and see if I can fix ! tree-wide [09:03] and revert shellcheck patch so that we track edge again [09:17] back now :) [09:17] aaaand, "not" is red [09:20] PR snapd#6907 closed: cmd/snap-update-ns: add several TODO comments [09:35] and i just had google:ubuntu-14.04-64:tests/main/parallel-install-aliases failure on one of my PRs [09:35] never seen it failing [09:38] pstolowski: how did it fail? [09:40] zyga: it was expecting the change to fail with 'cannot enable alias "alias1" for "aliases_foo", already enabled for "aliases"', but instead got 'Done today at 15:27 UTC today at 15:27 UTC Setup manual alias "alias1" => "cmd1" for snap "aliases"' [09:40] zyga: i wonder it snap change --last=alias check there isn't racy [09:41] huh, I saw that fail once as well [09:41] yesterday [09:41] because just above we test happy execution [09:41] zyga: i'll kick it again on travis and in the meantime execute and investigate it locally [09:41] k [09:42] and if that's it, will fix it [09:42] pstolowski: can you look at https://github.com/snapcore/snapd/pull/6903 [09:42] PR #6903: spread-shellcheck: add support for variants and environment [09:42] k [09:42] I need a 2nd review to open the fix for the mount propagation PR [09:52] mborzecki: curious, permissions package seems to be updated with some snapd bits [09:52] as a simple build in obs failed on that [09:52] * zyga looks [09:53] settle not converging [09:53] * zyga loves when unit tests fail [09:54] pstolowski: does this sound familiar? [09:54] https://www.irccloud.com/pastebin/02IEvvjO/ [09:55] ah, this is device registration, not hotplug [09:55] sorry [09:55] bah [09:56] this happens each time [09:56] * zyga debugs [10:01] PR snapd#6893 closed: gadget: helper for shifting structure start position [10:01] hrm [10:01] go test passes on my tree with 2.39 [10:01] fails in the tarball [10:01] whhyyyyy [10:05] hmmm [10:14] I disabled unit tests and installed the resulting package [10:14] let's see how it operates [10:14] then I can see why they fail again [10:20] eh [10:20] in x11 mode I don't have /snap/bin on path [10:20] in wayland mode some apps just crash [10:20] lovely [10:23] after reboot things seem to be have [10:23] behave even [10:35] zyga: hmm somewhat, seen failures like this from time to time while working on these tests, usually was a race. you're saying it fails each time like this? [10:35] do you remember https://github.com/snapcore/snapd/pull/6360 <- I just merged master into it :) [10:35] PR #6360: cmd/snap-update-ns: refactor of profile application [10:35] pstolowski: yeah [10:35] but only in packaging build [10:35] not in a checkout [10:35] I didn't look deeper yet [10:36] "not" failed again [10:37] zyga: on which test? [10:37] service watchdog [10:51] zyga: +1 on 6903 with a question [10:51] looking [10:55] of course parallel-install-aliases is passing when run locally... will keep it running during lunch [10:56] pstolowski: replied there [11:00] ty [11:00] * pstolowski lunch [11:03] PR snapd#6903 closed: spread-shellcheck: add support for variants and environment [11:04] I've opened https://github.com/snapcore/snapd/pull/6891 for real now [11:04] PR #6891: many: make per-snap mount namespace MS_SHARED [11:04] tests *should* pass now [11:04] so fingers crossed [11:05] I think I need a break [11:05] but it's moving along [11:05] what do you guys think about https://github.com/snapcore/snapd/pull/6909 ? [11:05] PR #6909: spread.yaml,tests: change MATCH and REBOOT to cmds [11:05] I will work on mountinfo-tool to support earlier versions of python so that we can use it in 16.04 [11:09] PR snapd#6911 opened: cmd/snap-update-ns: rename leftover ctx to upCtx [11:11] * zyga breaks [11:14] PR snapd#6912 opened: gadget: keep track of the index where structure content was defined [11:15] PR snapd#6883 closed: tests: fix how strings are matched on auto-refresh-retry test [11:15] PR snapd#6910 closed: spread.yaml: use "snap connections" in debug [11:17] mborzecki: is https://github.com/snapcore/snapd/pull/6890 really for review? [11:17] PR #6890: gadget: mounted filesystem writer & updater [11:17] zyga: heh, yes [11:17] *gulp* [11:17] zyga: want to dive in? :) [11:18] they told me to split a 1K diff [11:18] took several moths to review [11:18] *months [11:18] (i'd be faster with moths :D [11:18] zyga: 1.6k is tests :P [11:18] for me 80% was tests [11:18] good luck :-) [11:18] no way to chunk that? [11:19] idk, maybe i could sample, method by method :/ [11:19] at the end of the day I can review and approve but you need extra reviews from samuele [11:19] hm maybe i could pull the writer thing apart, that's ~10% of the thing [11:20] ok, really time to break now [11:20] I'll merge "not" when green [11:20] and if you guys can review https://github.com/snapcore/snapd/pull/6909 we could fix all of ! foo bugs [11:20] PR #6909: spread.yaml,tests: change MATCH and REBOOT to cmds [11:20] PR snapd#6913 opened: tests: force snap-reexec test to fail to reproduce the error on ubuntu 14.04 <⛔ Blocked> [11:29] zyga: should we have NOT instead of lowercase not to match MATCH and REBOOT? [11:36] chipaca not working today? [11:46] mborzecki: MATCH and REBOOT come from spread [11:46] popey_: I think he's off today [11:46] mborzecki: I think lowercase "not" looks nicer [11:46] ta [11:47] mborzecki: but to answer your question, I kept REBOOT and MATCH uppercase only to match spread names and avoid code changes [11:47] not failed again :D [11:47] heh [11:47] landing a trivial unused file [11:47] pstolowski: a look at https://github.com/snapcore/snapd/pull/6912 ? [11:47] PR #6912: gadget: keep track of the index where structure content was defined [11:48] this time on refresh-delta-from-core [11:48] meh [11:48] I'll grab some food [11:48] ttyl === ricab is now known as ricab|lunch === alan_g is now known as alan_g|lunch [12:19] tests still churning ... [12:35] zyga: can you do another pass of #6750? you did one there already, and I pushed some changes in the meantime [12:35] PR #6750: overlord/devicestate: update-gadget task handler with stubbed gadget callbacks [12:35] yep [12:35] PR snapd#6908 closed: tests: add "not" command [12:35] nice! [12:36] mborzecki: can you discuss that portal test change PR with cachio please? [12:37] mborzecki: https://github.com/snapcore/snapd/pull/6912 needs formatting fixes [12:37] PR #6912: gadget: keep track of the index where structure content was defined [12:37] zyga, sure [12:37] probably need to reformat with ancient go [12:37] (yuck) [12:38] settle not converging (unit tests) https://www.irccloud.com/pastebin/KgUHJ07T/ [12:38] mborzecki, did you see the comments in the PR? [12:38] loads of those [12:42] omg, i hate they made the formatting change in 1.11 [12:42] super annoying [12:51] zyga: fyi, I added a sprinking of emojis, a comment on your response to /etc and then also https://github.com/snapcore/snapd/pull/6891#issuecomment-495609846 [12:51] PR #6891: many: make per-snap mount namespace MS_SHARED [12:51] :-) [12:51] thanks! [12:51] I noticed it failed for real in one specific test [12:51] looking now [13:01] pstolowski: standup [13:01] yep [13:01] Snapcraft Live is just starting - https://www.youtube.com/watch?v=yAnRM4m63MY [13:02] Wimpress: nice! [13:02] Wimpress: nice, shirt, tie and shorts right? :) [13:03] mborzecki: *hopefully* there are shorts [13:03] the upsides of working remotely :) [13:09] PR snapd#6911 closed: cmd/snap-update-ns: rename leftover ctx to upCtx === alan_g|lunch is now known as alan_g === ricab|lunch is now known as ricab [13:39] jdstrand: I consistently get a denial for a permission I hold [13:39] perhaps something quirky with mount options and 14.04 (4.4) kernel [13:39] zyga: does it go away with a bare mount rule? [13:39] jdstrand: just "mount," ? [13:40] yes [13:40] jdstrand: curiously, it does not [13:40] ahhh [13:40] I'm a tool [13:41] sorryt [13:41] reexec bit me [13:41] heh. it always does at least once ;) [13:42] jdstrand: bare mount rule is ok [13:42] let's see if without it it also works :D [13:43] so it works but I don't know why ... let's restart and see [13:44] jdstrand: I think something is still fishy but I cannot point to anything apart from the failure in spread [13:46] jdstrand: I tweaked things a little os that they read: [13:46] relevant fragment of apparmor permissions https://www.irccloud.com/pastebin/v267dsbt/ [13:46] zyga: do you have anything needing 2nd review that i can take a look at? [13:46] the denial I got is associated with permission on line 6 [13:46] pstolowski: https://github.com/snapcore/snapd/pull/6891 [13:47] it's not 2nd but it needs review [13:47] PR #6891: many: make per-snap mount namespace MS_SHARED [13:47] that's the 2.39.1 patch [13:48] zyga: so, you are saying there is no apparmor denial, but an eperm, but with a bare mount rule, it all works? [13:48] there's a denial [13:48] bare mount fixes it [13:48] what is the denial? [13:49] the denial is [13:49] May 24 13:41:11 may241325-532686 kernel: audit: type=1400 audit(1558705271.095:74): apparmor="DENIED" operation="mount" info="failed flags match" error=-13 profile="/snap/snapd/x1/usr/lib/snapd/snap-confine" name="/tmp/snap.rootfs_dPNxms/" pid=18844 comm="snap-confine" flags="rw, rshared" [13:49] you're missing rw [13:49] but wait, rw I had [13:49] I removed it [13:50] (also rw makes no sense for propagation changes) [13:50] I can only comment on what you have shown me [13:50] :) [13:50] yeah I know :) [13:51] zyga: I'm not sure why rw would pop out. that may be a bug or it may be some subtlety in the kernel that needs it cause it is shared [13:51] seeing the strace of that might help... [13:51] yeah [13:53] jdstrand: hey! tools r... wait... checking the actual version... "20190522-1231UTC click-reviewers-tools version " is now in production \o/ I think you can now get rid of your bzr branch [13:53] zyga: I mean, looking at the strace I have here, the rbinds don't specify rw, but there it is on line 4 of your paste [13:53] roadmr: yay!! [13:54] \o\ [13:54] /o/ [13:54] \o\ [13:54] \o/ [13:54] * jdstrand hugs roadmr :) [13:54] 🙌 hehe [13:55] zyga: I suspect if you add the rw and account for reexec, you will be on your way [13:55] there's rw there already [13:55] it's not that [13:55] I'm debugging [13:56] * jdstrand is confused [13:57] jdstrand: there *was* rw, I removed it thinking it is the issue [13:59] zyga: but reexec? [13:59] I changed the source and re-ran spread [14:00] gah, I'm tired [14:00] jdstrand: I merged master into the ancient huge PRs [14:01] jdstrand: https://github.com/snapcore/snapd/pull/6341 is interesting (diff wise small) [14:01] PR #6341: WIP: persist per-user mount namespace profile [14:02] Bug #1830382 opened: Have a progress indicator for snap pack (like with snapcraft pack) [14:03] jdstrand: with that branch I should be able to answer your questions about the set of required capabilities [14:05] these PRs are like rabbits [14:05] Bug #1830382 changed: Have a progress indicator for snap pack (like with snapcraft pack) [14:05] I review a bunch and then end up with more than I started with [14:05] yeah [14:07] mvo, zyga: btw, shouldn't https://bugs.launchpad.net/snapd/+bug/1819318 be at least Fix Committed? and arguably Fix Released? [14:07] Bug #1819318: no interfaces when installing only core18 [14:08] sparkiegeek: in a call [14:08] mvo worked on that [14:15] jdstrand: https://github.com/snapcore/snapd/pull/6891#issuecomment-495643768 I'm giving up now [14:15] PR #6891: many: make per-snap mount namespace MS_SHARED [14:15] need to take a walk and think [14:22] a few hours later and... parallel-install-aliases doesn't want to fail [14:22]