mupPR snapcraft#2568 closed: docs: consolidate on a simple HACKING.md <Created by sergiusens> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/2568>00:07
=== jamesh_ is now known as jamesh
zygaHey mborzecki06:00
mborzeckizyga: hey06:00
mborzeckizyga: how's your day off coming? :)06:00
zygaI was just wondering the same thing06:00
zygaShould I take today off for real06:00
zygaOr work some more to get stuff done06:00
zygaSo many things to work on06:01
zygamborzecki: I sent a small patch for spread-shellcheck06:05
zygaAnd had an idea on how to fix not MATCH06:05
zygaWe can save MATCH to lib/bin on startup06:07
zygaLike we do now to match.sh include file06:07
zygaSince it is on path it will be more flexible06:07
zygaThen MATCH is no longer a function06:08
zygaThen we can put โ€œnotโ€ on path and fix all tests06:08
mborzeckizyga: in other news, got an image built by snap-image (strawman) to boot up to a point where i got console conf :P06:15
zygaThat is great06:16
zygaIt still feels that landing your changes will take a month06:17
mborzeckiyeah, and tests and all that06:17
mborzeckizyga: if you guys have a system-rescue role that you'd like me to add or something, this coudl be done really easily06:18
zygaRole as in partition type?06:19
zygaI think we should talk about how the recovery system is designed, so far, to keep you in the loop06:19
mborzeckizyga: role as role in gadget.yaml terms, and whatever that means practically, eg. like system-boot is a bit special06:19
mborzeckizyga: and so is system-data06:19
zygaI think some things there will change06:20
zygaThough I think that is best for next week06:20
zygare, back in the office06:37
zygamborzecki: can you do a quick review of https://github.com/snapcore/snapd/pull/690606:38
zygajust a variable rename06:38
mupPR #6906: cmd/snap-update-ns: rename ctx to upCtx <Simple ๐Ÿ˜ƒ> <Created by zyga> <https://github.com/snapcore/snapd/pull/6906>06:38
zygathough more importantly I need a review for https://github.com/snapcore/snapd/pull/6903 to get 2.39.1 fix in place06:38
mupPR #6903: spread-shellcheck: add support for variants and environment <Created by zyga> <https://github.com/snapcore/snapd/pull/6903>06:38
zygathe shellcheck change is not perfect06:38
zygabut it's also not totally incorrect06:38
zygaand allows my tests to not get stuck on that06:38
* zyga quick coffee07:01
zygamborzecki: if you can review the spread-shellcheck change I will be able to propose the propagation fixes07:01
zygaI will do some branch gardening soon07:03
zygabut first coffee07:03
=== pstolowski|afk is now known as pstolowski
zygaHey :-)07:09
zygaInteresting https://forum.snapcraft.io/t/snapshots-can-expose-sensitive-data/1149307:19
pstolowskizyga: hmm interesting indeed07:27
mupPR snapd#6906 closed: cmd/snap-update-ns: rename ctx to upCtx <Simple ๐Ÿ˜ƒ> <Created by zyga> <Merged by zyga> <https://github.com/snapcore/snapd/pull/6906>07:27
zygamakes me feel that snapshots should be saved back to $HOME07:27
zygaas anything else is not correct wrt NFS and encryption07:27
zygapstolowski: quick trivial https://github.com/snapcore/snapd/pull/690707:28
mupPR #6907: cmd/snap-update-ns: add several TODO comments <Created by zyga> <https://github.com/snapcore/snapd/pull/6907>07:28
zygajust comments07:28
zygabut that's *all* of the refactoring :D07:28
mupPR snapd#6907 opened: cmd/snap-update-ns: add several TODO comments <Created by zyga> <https://github.com/snapcore/snapd/pull/6907>07:28
mborzeckipstolowski: hey07:30
mupPR snapd#6894 closed: tests: add "not" command as replacement for "!" in tests <Created by zyga> <Closed by zyga> <https://github.com/snapcore/snapd/pull/6894>07:40
mupPR snapd#6908 opened: tests: add "not" command <Simple ๐Ÿ˜ƒ> <Created by zyga> <https://github.com/snapcore/snapd/pull/6908>07:44
zygamborzecki, pstolowski: how do you feel about ^ not08:05
zygaI want to build this on top  https://github.com/snapcore/snapd/pull/690908:05
mupPR #6909: spread.yaml,tests: change MATCH and REBOOT to cmds <Created by zyga> <https://github.com/snapcore/snapd/pull/6909>08:05
zygaand then fix our scripts to use not rather than !08:05
mupPR snapd#6909 opened: spread.yaml,tests: change MATCH and REBOOT to cmds <Created by zyga> <https://github.com/snapcore/snapd/pull/6909>08:06
pstolowskizyga: interesting; don't we still need to make it a compound expressionm e.g. { ! "$@" } ?08:11
zygano,  why?08:11
zyga! functions correctly08:12
zyganote that this executable is not sourced, it's a program that runs without set -e08:12
pstolowskizyga: 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/3958201208:12
=== icey_ is now known as icey
zygaI 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:22
zygamborzecki: https://github.com/google/pytype08:40
zygamborzecki: from that video you linkedto08:40
zygamborzecki: updated https://github.com/snapcore/snapd/pull/690308:51
mupPR #6903: spread-shellcheck: add support for variants and environment <Created by zyga> <https://github.com/snapcore/snapd/pull/6903>08:51
mborzeckizyga: nhm, lgtm08:55
zygamborzecki: something for you :) https://github.com/snapcore/snapd/pull/691009:00
mupPR #6910: spread.yaml: use "snap connections" in debug <Simple ๐Ÿ˜ƒ> <Created by zyga> <https://github.com/snapcore/snapd/pull/6910>09:00
mupPR snapd#6910 opened: spread.yaml: use "snap connections" in debug <Simple ๐Ÿ˜ƒ> <Created by zyga> <https://github.com/snapcore/snapd/pull/6910>09:00
* zyga really gets that coffee now09:03
zygaafter the break I will try to land "not" and "MATCH" as commands and see if I can fix ! tree-wide09:03
zygaand revert shellcheck patch so that we track edge again09:03
zygaback now :)09:17
zygaaaaand, "not" is red09:17
mupPR snapd#6907 closed: cmd/snap-update-ns: add several TODO comments <Created by zyga> <Merged by zyga> <https://github.com/snapcore/snapd/pull/6907>09:20
pstolowskiand i just had google:ubuntu-14.04-64:tests/main/parallel-install-aliases failure on one of my PRs09:35
pstolowskinever seen it failing09:35
zygapstolowski: how did it fail?09:38
pstolowskizyga: 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
pstolowskizyga: i wonder it snap change --last=alias check there isn't racy09:40
zygahuh, I saw that fail once as well09:41
pstolowskibecause just above we test happy execution09:41
pstolowskizyga: i'll kick it again on travis and in the meantime execute and investigate it locally09:41
pstolowskiand if that's it, will fix it09:42
zygapstolowski: can you look at https://github.com/snapcore/snapd/pull/690309:42
mupPR #6903: spread-shellcheck: add support for variants and environment <Created by zyga> <https://github.com/snapcore/snapd/pull/6903>09:42
zygaI need a 2nd review to open the fix for the mount propagation PR09:42
zygamborzecki: curious, permissions package seems to be updated with some snapd bits09:52
zygaas a simple build in obs failed on that09:52
* zyga looks09:52
zygasettle not converging09:53
* zyga loves when unit tests fail09:53
zygapstolowski: does this sound familiar?09:54
zygaah, this is device registration, not hotplug09:55
zygathis happens each time09:56
* zyga debugs09:56
mupPR snapd#6893 closed: gadget: helper for shifting structure start position <Gadget update> <Simple ๐Ÿ˜ƒ> <Created by bboozzoo> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/6893>10:01
zygago test passes on my tree with 2.3910:01
zygafails in the tarball10:01
zygaI disabled unit tests and installed the resulting package10:14
zygalet's see how it operates10:14
zygathen I can see why they fail again10:14
zygain x11 mode I don't have /snap/bin on path10:20
zygain wayland mode some apps just crash10:20
zygaafter reboot things seem to be have10:23
zygabehave even10:23
pstolowskizyga: 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
zygado you remember https://github.com/snapcore/snapd/pull/6360 <- I just merged master into it :)10:35
mupPR #6360: cmd/snap-update-ns: refactor of profile application <Per-user mount ns  ๐ŸŽ> <Created by zyga> <https://github.com/snapcore/snapd/pull/6360>10:35
zygapstolowski: yeah10:35
zygabut only in packaging build10:35
zyganot in a checkout10:35
zygaI didn't look deeper yet10:35
zyga"not" failed again10:36
pstolowskizyga: on which test?10:37
zygaservice watchdog10:37
pstolowskizyga: +1 on 6903 with a question10:51
pstolowskiof course parallel-install-aliases is passing when run locally... will keep it running during lunch10:55
zygapstolowski: replied there10:56
* pstolowski lunch11:00
mupPR snapd#6903 closed: spread-shellcheck: add support for variants and environment <Created by zyga> <Merged by zyga> <https://github.com/snapcore/snapd/pull/6903>11:03
zygaI've opened https://github.com/snapcore/snapd/pull/6891 for real now11:04
mupPR #6891: many: make per-snap mount namespace MS_SHARED <Created by zyga> <https://github.com/snapcore/snapd/pull/6891>11:04
zygatests *should* pass now11:04
zygaso fingers crossed11:04
zygaI think I need a break11:05
zygabut it's moving along11:05
zygawhat do you guys think about https://github.com/snapcore/snapd/pull/6909 ?11:05
mupPR #6909: spread.yaml,tests: change MATCH and REBOOT to cmds <Created by zyga> <https://github.com/snapcore/snapd/pull/6909>11:05
zygaI will work on mountinfo-tool to support earlier versions of python so that we can use it in 16.0411:05
mupPR snapd#6911 opened: cmd/snap-update-ns: rename leftover ctx to upCtx <Simple ๐Ÿ˜ƒ> <Created by zyga> <https://github.com/snapcore/snapd/pull/6911>11:09
* zyga breaks11:11
mupPR snapd#6912 opened: gadget: keep track of the index where structure content was defined <Gadget update> <Simple ๐Ÿ˜ƒ> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/6912>11:14
mupPR snapd#6883 closed: tests: fix how strings are matched on auto-refresh-retry test <Created by sergiocazzolato> <Merged by sergiocazzolato> <https://github.com/snapcore/snapd/pull/6883>11:15
mupPR snapd#6910 closed: spread.yaml: use "snap connections" in debug <Simple ๐Ÿ˜ƒ> <Created by zyga> <Merged by zyga> <https://github.com/snapcore/snapd/pull/6910>11:15
zygamborzecki: is https://github.com/snapcore/snapd/pull/6890 really for review?11:17
mupPR #6890: gadget: mounted filesystem writer & updater <Gadget update> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/6890>11:17
mborzeckizyga: heh, yes11:17
mborzeckizyga: want to dive in? :)11:17
zygathey told me to split a 1K diff11:18
zygatook several moths to review11:18
zyga(i'd be faster with moths :D11:18
mborzeckizyga: 1.6k is tests :P11:18
zygafor me 80% was tests11:18
zygagood luck :-)11:18
zygano way to chunk that?11:18
mborzeckiidk, maybe i could sample, method by method :/11:19
zygaat the end of the day I can review and approve but you need extra reviews from samuele11:19
mborzeckihm maybe i could pull the writer thing apart, that's ~10% of the thing11:19
zygaok, really time to break now11:20
zygaI'll merge "not" when green11:20
zygaand if you guys can review https://github.com/snapcore/snapd/pull/6909 we could fix all of ! foo bugs11:20
mupPR #6909: spread.yaml,tests: change MATCH and REBOOT to cmds <Created by zyga> <https://github.com/snapcore/snapd/pull/6909>11:20
mupPR snapd#6913 opened: tests: force snap-reexec test to fail to reproduce the error on ubuntu 14.04 <โ›” Blocked> <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/6913>11:20
mborzeckizyga: should we have NOT instead of lowercase not to match MATCH and REBOOT?11:29
popey_chipaca not working today?11:36
zygamborzecki: MATCH and REBOOT come from spread11:46
zygapopey_: I think he's off today11:46
zygamborzecki: I think lowercase "not" looks nicer11:46
zygamborzecki: but to answer your question, I kept REBOOT and MATCH uppercase only to match spread names and avoid code changes11:47
zyganot failed again :D11:47
zygalanding a trivial unused file11:47
mborzeckipstolowski: a look at https://github.com/snapcore/snapd/pull/6912 ?11:47
mupPR #6912: gadget: keep track of the index where structure content was defined <Gadget update> <Simple ๐Ÿ˜ƒ> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/6912>11:47
zygathis time on refresh-delta-from-core11:48
zygaI'll grab some food11:48
=== ricab is now known as ricab|lunch
=== alan_g is now known as alan_g|lunch
zygatests still churning ...12:19
mborzeckizyga: can you do another pass of #6750? you did one there already, and I pushed some changes in the meantime12:35
mupPR #6750: overlord/devicestate: update-gadget task handler with stubbed gadget callbacks <Gadget update> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/6750>12:35
mupPR snapd#6908 closed: tests: add "not" command <Simple ๐Ÿ˜ƒ> <Created by zyga> <Merged by zyga> <https://github.com/snapcore/snapd/pull/6908>12:35
zygamborzecki: can you discuss that portal test change PR with cachio please?12:36
zygamborzecki: https://github.com/snapcore/snapd/pull/6912 needs formatting fixes12:37
mupPR #6912: gadget: keep track of the index where structure content was defined <Gadget update> <Simple ๐Ÿ˜ƒ> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/6912>12:37
cachiozyga, sure12:37
zygaprobably need to reformat with ancient go12:37
zygasettle not converging (unit tests) https://www.irccloud.com/pastebin/KgUHJ07T/12:38
cachiomborzecki, did you see the comments in the PR?12:38
zygaloads of those12:38
mborzeckiomg, i hate they made the formatting change in 1.1112:42
mborzeckisuper annoying12:42
jdstrandzyga: 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-49560984612:51
mupPR #6891: many: make per-snap mount namespace MS_SHARED <Created by zyga> <https://github.com/snapcore/snapd/pull/6891>12:51
zygaI noticed it failed for real in one specific test12:51
zygalooking now12:51
zygapstolowski: standup13:01
WimpressSnapcraft Live is just starting - https://www.youtube.com/watch?v=yAnRM4m63MY13:01
zygaWimpress: nice!13:02
mborzeckiWimpress: nice, shirt, tie and shorts right? :)13:02
zygamborzecki: *hopefully* there are shorts13:03
mborzeckithe upsides of working remotely :)13:03
mupPR snapd#6911 closed: cmd/snap-update-ns: rename leftover ctx to upCtx <Simple ๐Ÿ˜ƒ> <Created by zyga> <Merged by stolowski> <https://github.com/snapcore/snapd/pull/6911>13:09
=== alan_g|lunch is now known as alan_g
=== ricab|lunch is now known as ricab
zygajdstrand: I consistently get a denial for a permission I hold13:39
zygaperhaps something quirky with mount options and 14.04 (4.4) kernel13:39
jdstrandzyga: does it go away with a bare mount rule?13:39
zygajdstrand: just "mount," ?13:39
zygajdstrand: curiously, it does not13:40
zygaI'm a tool13:40
zygareexec bit me13:41
jdstrandheh. it always does at least once ;)13:41
zygajdstrand: bare mount rule is ok13:42
zygalet's see if without it it also works :D13:42
zygaso it works but I don't know why ... let's restart and see13:43
zygajdstrand: I think something is still fishy but I cannot point to anything apart from the failure in spread13:44
zygajdstrand: I tweaked things a little os that they read:13:46
zygarelevant fragment of apparmor permissions https://www.irccloud.com/pastebin/v267dsbt/13:46
pstolowskizyga: do you have anything needing 2nd review that i can take a look at?13:46
zygathe denial I got is associated with permission on line 613:46
zygapstolowski: https://github.com/snapcore/snapd/pull/689113:46
zygait's not 2nd but it needs review13:47
mupPR #6891: many: make per-snap mount namespace MS_SHARED <Created by zyga> <https://github.com/snapcore/snapd/pull/6891>13:47
zygathat's the 2.39.1 patch13:47
jdstrandzyga: so, you are saying there is no apparmor denial, but an eperm, but with a bare mount rule, it all works?13:48
zygathere's a denial13:48
zygabare mount fixes it13:48
jdstrandwhat is the denial?13:48
zygathe denial is13:49
zygaMay 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
jdstrandyou're missing rw13:49
zygabut wait, rw I had13:49
zygaI removed it13:49
zyga(also rw makes no sense for propagation changes)13:50
jdstrandI can only comment on what you have shown me13:50
zygayeah I know :)13:50
jdstrandzyga: 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 shared13:51
jdstrandseeing the strace of that might help...13:51
roadmrjdstrand: 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 branch13:53
jdstrandzyga: I mean, looking at the strace I have here, the rbinds don't specify rw, but there it is on line 4 of your paste13:53
jdstrandroadmr: yay!!13:53
jdstrand \o\13:54
jdstrand /o/13:54
jdstrand \o\13:54
jdstrand \o/13:54
* jdstrand hugs roadmr :)13:54
roadmr๐Ÿ™Œ  hehe13:54
jdstrandzyga: I suspect if you add the rw and account for reexec, you will be on your way13:55
zygathere's rw there already13:55
zygait's not that13:55
zygaI'm debugging13:55
* jdstrand is confused13:56
zygajdstrand: there *was* rw, I removed it thinking it is the issue13:57
jdstrandzyga: but reexec?13:59
zygaI changed the source and re-ran spread13:59
zygagah, I'm tired14:00
zygajdstrand: I merged master into the ancient huge PRs14:00
zygajdstrand: https://github.com/snapcore/snapd/pull/6341 is interesting (diff wise small)14:01
mupPR #6341: WIP: persist per-user mount namespace profile <Created by zyga> <https://github.com/snapcore/snapd/pull/6341>14:01
mupBug #1830382 opened: Have a progress indicator for snap pack (like with snapcraft pack) <Snappy:New> <https://launchpad.net/bugs/1830382>14:02
zygajdstrand: with that branch I should be able to answer your questions about the set of required capabilities14:03
jdstrandthese PRs are like rabbits14:05
mupBug #1830382 changed: Have a progress indicator for snap pack (like with snapcraft pack) <snapd:Triaged> <https://launchpad.net/bugs/1830382>14:05
jdstrandI review a bunch and then end up with more than I started with14:05
sparkiegeekmvo, zyga: btw, shouldn't https://bugs.launchpad.net/snapd/+bug/1819318 be at least Fix Committed? and arguably Fix Released?14:07
mupBug #1819318: no interfaces when installing only core18 <snapd:In Progress> <https://launchpad.net/bugs/1819318>14:07
zygasparkiegeek: in a call14:08
zygamvo worked on that14:08
zygajdstrand: https://github.com/snapcore/snapd/pull/6891#issuecomment-495643768 I'm giving up now14:15
mupPR #6891: many: make per-snap mount namespace MS_SHARED <Created by zyga> <https://github.com/snapcore/snapd/pull/6891>14:15
zyganeed to take a walk and think14:15
pstolowskia few hours later and... parallel-install-aliases doesn't want to fail14:22
mupPR snapd#6912 closed: gadget: keep track of the index where structure content was defined <Gadget update> <Simple ๐Ÿ˜ƒ> <Created by bboozzoo> <Merged by stolowski> <https://github.com/snapcore/snapd/pull/6912>14:22
mupPR snapd#6914 opened: tests: change strace parameters on snap-run test to avoid the test gets stuck <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/6914>15:34
* cachio lunch15:57
=== pstolowski is now known as pstolowski|Afk
* zyga feels sick somehow, EODing now16:55
mupBug #1830412 opened: QT5 classic snaps segmentation fault <Snappy:New> <https://launchpad.net/bugs/1830412>17:10
* cachio afk18:59
mupPR pc-amd64-gadget#17 closed: add .travis.yml to enable basic smoke testing (20) <Created by mvo5> <Merged by vorlonofportland> <https://github.com/snapcore/pc-amd64-gadget/pull/17>20:53
mupPR pc-amd64-gadget#16 closed: add .travis.yml to enable basic smoke testing (18) <Created by mvo5> <Merged by vorlonofportland> <https://github.com/snapcore/pc-amd64-gadget/pull/16>20:55
mupPR pc-amd64-gadget#15 closed: add .travis.yml to enable basic smoke testing <Created by mvo5> <Merged by vorlonofportland> <https://github.com/snapcore/pc-amd64-gadget/pull/15>21:00

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!