[05:27] morning [05:38] kernel update, rebooting, brb [05:40] aand back [05:45] Hey :-) [05:45] mborzecki: solved that issue last night [05:45] zyga: hey [05:45] I’ll eat dry and update the pr [05:45] zyga: oh, solved mine too, i can create bootable images now :) [05:46] s/dry/sth/ [05:48] Sounds like a good day indeed [05:48] What was the issue? [05:52] zyga: the raw structure writer was dumping image content at a start offset that was within a complete volume, but since i'm constructing an image of each partition separately, i need to apply a bias to have the structure start at 0 offset [05:54] now i need to figure out where the hell grubenv comes from in ubuntu-image [06:02] mborzecki: the grubenv comes from `snap prepare-image` [06:02] mvo: hi, and thanks for the tip [06:03] mborzecki: yw and hi as well [06:26] PR snapd#6893 opened: gadget: helper for shifting structure start position [06:39] back in the office :) [06:40] mvo: hey, I nailed that bug last night [06:40] I need to make two small modifications to the fix PR and it's going to be green :) [06:40] one is just shell quoting and another is reduction of a test that was really measuring the bug to begin with [06:40] once I realised the test was buggy it was all obvious :) [06:42] mvo: I also got a much nicer approach to defer, but separated out into its own branch [06:42] it is much nicer than the v1 [06:42] I'll get to it now :) [06:42] hey pedronis [06:46] zyga: nice! [06:46] hey pedronis [06:46] zyga: looking forward to it [06:46] mvo: hi === pstolowski|afk is now known as pstolowski [07:01] morning [07:02] pstolowski: pedronis: hey [07:03] is build.snapcraft.io a bit flaky at the moment? I can't seem to add a new repo (it just keeps spinning...) [07:29] pstolowski: hi, what are you working on ATM ? [07:31] pedronis: hi, a small followup for snap debug timings output, then a small bugfix for them, then i'll move onto preventing nulls (with a forum topic at the same time) [07:32] pstolowski: I thought a bit further about the latter, we should chat before we go there, also as we discussed it's probably 2.41 material [07:32] pstolowski: mvo: should we try to get "base: none" support sooner instead [07:33] pedronis: there are two small issues with ensure timings, so perhaps we can discuss them + null at the same time [07:36] pstolowski: we can chat in 30 mins if you want [07:36] ~30 [07:37] pedronis: ok [07:48] hmm shellcheck got an update apparently [07:49] mborzecki: is it breaking our stuff? [07:50] mvo: https://paste.ubuntu.com/p/Ztb7rqnzXP/ [07:51] mvo: hey, I'd love a review of https://github.com/snapcore/snapd/pull/6888 [07:51] PR #6888: tests: add mountinfo-tool [07:51] it's a pre-req for the test for the propagation bug [07:51] mvo: https://github.com/koalaman/shellcheck/wiki/SC2251 [07:52] mborzecki: uh, thats pedantic [07:52] mborzecki: let me read this but I'm not sure I like it [07:53] mborzecki: eh [07:53] mborzecki: if I'm reading this correctly our tests have some big holes?!? [07:53] * mvo looks closer [07:54] mvo: heh, yeah, not sure i understand the issue here, trying it out locally [07:54] mborzecki: yeah [07:54] mborzecki: https://paste.ubuntu.com/p/rtzBmt5Btf/ [07:55] mborzecki: this looks like either ! true or ! false trigger an errexit [07:56] mborzecki: interesting, shell is a valley of landmines! [07:56] zyga: its getting worse and worse [07:56] mvo: fun [07:56] yeah [07:56] zyga: I mean, the more I learn the more I despair about all the corners one need to know [07:56] it's all terrible [07:56] mvo: right? [07:56] mvo: don't trigger errexit you mean, right? [07:56] I wish someone wrote a sane-sh [07:56] zyga: also - I swear I saw this a gazillon of times in various scripts [07:57] yeah [07:57] mvo: we should donate to shellcheck [07:57] so yes, our tests are full of this, fun, not [07:57] mborzecki: yeah, it seems like !true !false !whatever, nothing triggers an exit [07:57] pedronis: /o\ [07:57] mvo: yup :/ ehh [07:57] zyga: totally, at least we need to sent him some "you are a hero" mail or something [07:57] mvo: at least it's good shellcheck flags that [07:58] zyga@yantra:~/go/lnx-src/github.com/snapcore/snapd> git grep '!' -- */task.yaml | wc -l [07:58] 490 [07:58] oh my [07:58] mborzecki: which version of shellcheck is that? [07:58] zyga: snap info shellcheck, look at edge [07:58] zyga@yantra:~/go/lnx-src/github.com/snapcore/snapd> git grep '!' -- '*.sh' | wc -l [07:58] 129 [07:59] btw. it's also published as a snap by the author :P [07:59] mborzecki: yes [07:59] mvo: anyway trying here their correct doesn't work either [07:59] maybe I'm just confused [07:59] zyga: if this guy is on twitter we should sent some hearts, I'm sure email will just get into his spam folder [08:01] pedronis: same here, how strange [08:02] we do ! foo, because we expect foo to fail, so if it doesn't fail, the test should err out i.e. errexit should get triggerred [08:02] perhaps "correct" is just about telling shellcheck that is what you mean [08:02] in all cases ! true doesn't exit [08:02] no, the wiki says, it will errexit [08:02] but doesn't [08:03] maybe a different shell? [08:03] I tried bash and dash [08:04] I tried bash 3.2.57 (macos) and 5.0.7 (linux) [08:04] same lack of result [08:04] hey Chipaca [08:04] hey chihchun [08:05] I meant Chipaca [08:05] Chipaca: great timing, we have shell fun again [08:05] Chipaca: https://github.com/koalaman/shellcheck/wiki/SC2251 [08:05] deep shell fun [08:05] pstolowski: want to chat now? [08:05] pedronis: let me abberviate that to "deep sh" [08:05] pedronis: yep, 1 minute [08:06] mvo: strawman: uses python for tests [08:06] mvo: fwiw ! true without {} doesn't work as expected either [08:06] mvo: with that magic module that puts all shell commands as python functions [08:06] mvo: via import magic [08:06] * zyga looks [08:06] zyga: sounds like you worked with bitbake :P [08:07] mborzecki: mvo: nice [08:07] oooh [08:07] github now supports security patches [08:07] so I've got bugs in my head, and we've got bugs in our code [08:07] zyga: oh? [08:08] Chipaca: so, the trick is finding a short form that actually works, bc one they say is correct does not seem to work [08:08] I think I misread, security *bugs* [08:08] https://twitter.com/github/status/1131470848513249280 [08:08] Chipaca: did you say "bugs on my head" ? [08:08] mborzecki: I have an idea [08:08] mborzecki: give me a sec [08:08] mborzecki: and it ties into a super nice idea (famous last words) that I had in my last branch [08:08] mborzecki: to put new things on $PATH for testing [08:08] mborzecki: we can write "not ..." command [08:08] mvo: in [08:08] that just runs the rest in shell and negates the result [08:08] pstolowski: I'm in the standup [08:09] * zyga does so quickly [08:09] pedronis: ok coming [08:09] zyga: oh, thats an interessting idea [08:09] mvo: right? [08:09] mvo: give me 3 [08:09] hack spread and make it define NOT [08:11] mvo: ( ! true ) [08:11] aha! [08:11] mborzecki: funny, the linked stackoverflow mentions () [08:12] ha! [08:12] mborzecki: so probably worth a bugreport/patch [08:12] should have done the reading [08:12] mborzecki: to the shellcheck guy, least we can do [08:12] nah, reading is overrated [08:15] mvo: fwiw ( ! true ) isn't flagged [08:16] mvo: https://github.com/snapcore/snapd/pull/6894 [08:16] PR #6894: tests: add "not" command as replacement for "!" in tests [08:16] PR snapd#6894 opened: tests: add "not" command as replacement for "!" in tests [08:16] mborzecki: ^ [08:17] um [08:17] zyga: didn't you mean to put bin in the path [08:17] Chipaca: didn't I put it? [08:17] zyga: and the binary in the bin [08:17] zyga: so the binary is in the path [08:17] ? [08:17] doh [08:17] haha [08:17] I see [08:18] zyga: you even wrote a .keep file to keep the empty directory [08:18] zyga: i think you need more coffee [08:18] Chipaca: I'm having some now :D [08:18] I recycled the bin idea from an earlier branch [08:18] fixed now [08:18] have of your neurons are all "whee" and the other half are all "g'waymnsfifCOMFYfmsmwm" [08:19] half* [08:19] Chipaca: I'm tempted to add tests/lib/man to MANPATH and write a man page now [08:19] should I? [08:19] mvo: if you like this idea (about not replacing !) I can adjust tests [08:19] zyga: no [08:20] zyga: (i don't really mind but nobody's gonna read it) [08:20] mvo: perhaps we should invite shellcheck developer to a sprint sometime, he might be a good voice in the haskel community [08:20] Chipaca: I'd read it in --debug [08:21] zyga: have it look out for -h/--help then? [08:21] zyga: but seems silly [08:21] zyga: this is how /bin/true is not an empty file [08:21] or was it /bin/false [08:21] Chipaca: let's fix the tests story first [08:21] anyway :) [08:22] then I can play on this later :) [08:22] zyga: I like the PR, unless pedronis disagrees I think we should use this approach and replace all our "! foo" with "not foo" or "assert_fail" or "assertFail foo" ? [08:22] mvo: I agree [08:22] /bin/true could be just an empty executable file and instead is a 27k dynamic binary, and it's all zyga's fault [08:23] mvo: I'd go with "not foo" first [08:23] not assert anything [08:23] as we really are asserting each line [08:23] because of the set -e [08:23] so quicl poll on the name: "not", "assert_false", "assertFalse" ? [08:23] mvo: we may also see bugs uncovered [08:23] mvo: it should be "not" because of what I said [08:23] zyga: oh yes [08:23] and plays nice with the language, python and readability [08:24] unless it starts to look ugly in tests [08:24] but let's see how it works out [08:24] mvo: I'll start attacking shell tests now [08:25] zyga: sounds good, *if* there is a discussion about the naming we can easily sed the name again [08:25] mvo: sounds good [08:25] mvo: even inside the patch ): [08:25] :-) [08:25] * zyga is on it [08:25] haha, true [08:27] zyga, mborzecki [08:27] have you guys tried the new snapd updates for fedora and el7? [08:28] no, not lately, I just updated my F29 machine to usable status [08:28] Eighth_Doctor: posted karma already, did you push any new updates? [08:28] no [08:28] it's the 2.39 update with the selinux stuff [08:30] mvo: reported https://github.com/koalaman/shellcheck/issues/1588 [08:32] Eighth_Doctor: https://bodhi.fedoraproject.org/updates/FEDORA-2019-57826cb704#comment-944246 btw there's a systemd issue that also works better after a reboot [08:32] zyga: do you think you could try the update in an actual workstation install? [08:33] mborzecki: ta [08:33] mborzecki: yeah, on F30 [08:33] mborzecki: I've marked the update as one that requires a reboot [08:35] zyga: mvo: given the spike and everything not sure zyga should work on this, can we put somebody else on it [08:35] pedronis: it's done now, just running tests [08:35] zyga: done, as in you adjusted all tests? [08:36] yes [08:36] ok, if some fail you probably pass it along to somebody else though [08:37] yeah [08:37] running now, fingers crossed [08:38] I want, at least, the spread-shellcheck test to pass [08:39] pedronis: agreed, we also need to finish the mount bug, iirc there are some loose ends here too. lets give the other parts to someone else [08:39] Eighth_Doctor: other than that the updates seemed to work fine :P now that we also have f30 under spread it should be even better [08:40] Eighth_Doctor: think you might be interested in this one: https://github.com/snapcore/snapd/pull/6874 [08:40] PR #6874: cmd/snap-confine: do not mount over non files/directories [08:48] PR snapd#6895 opened: cmd/snap-confine, data/selinux: cherry pick Fedora 30 fixes to 2.39 [08:49] hm should probably cherry-pick f30 for spread too [08:54] PR snapd#6880 closed: daemon: refactor user ops to api_users [08:57] mvo: how does https://github.com/snapcore/snapd/pull/6894 look? [08:57] PR #6894: tests: add "not" command as replacement for "!" in tests [08:59] mvo: this is now shellcheck clean [08:59] mvo: I'm running a pass of main/regression to see if something explodes [08:59] but I will open the PR for review soon [09:00] zyga: looking [09:03] zyga: yeah, looks sensible [09:03] updated the comment in the PR [09:06] PR snapd#6896 opened: cmd/snap: tweak the output of snap debug timings --ensure= [09:09] tests are looking good so far [09:17] zyga: great, once that is ready please go back to the mount bug, its the last piece missing for 2.39.1 [09:17] mvo: mount bug is ready, it's just blocked by spread here [09:17] but sure [09:17] zyga: blocked by spread in the sense that tests are unstable? [09:18] by spread-shellcheck specifically ;) [09:18] mvo: let's do this: [09:18] actually [09:18] * zyga checks first [09:18] zyga: hm, that seems silly, lets move to shellcheck from stable then for now [09:18] my understanding is that master is red due to the shellcheck issue [09:18] let's fix shellcheck in master [09:18] stable: v0.6.0 2019-01-11 (329) 3MB - [09:18] I think we should (temporarily) switch to shellcheck/stable [09:19] and then move back to edge with the fix [09:19] land the test tool from https://github.com/snapcore/snapd/pull/6888 (I can make it so that it doesn't conflict with the "not" branch) [09:19] PR #6888: tests: add mountinfo-tool [09:19] mvo: and then I can just propose the mount propagation PR and start working on intramfs [09:21] zyga: ok, that sounds like a plan - 1. move to shellcheck stable so that we can land things again, 2 we need reviews for 6888 3. propose the mount fix (even with 6888 not fully reviewed). sounds sensible? [09:21] yep [09:21] +1 [09:22] ta [09:23] PR snapd#6897 opened: tests/unit/spread-shellcheck: temporary workaround for SC2251 [09:24] mvo: zyga: ^^ [09:24] thanks [09:33] mborzecki: can you look at https://github.com/snapcore/snapd/pull/6888 [09:33] PR #6888: tests: add mountinfo-tool [09:33] it's not small but it's a test tool [09:33] zyga: looking [09:33] thx [09:48] PR snapd#6898 opened: make snapstate.Update take *RevisionOptions instead of chan, rev [09:54] PR snapd#6816 closed: daemon, overlord: support for cohort-key in refresh and switch [10:08] some bad news, for perfect support NOT would need to be a function like MATCH [10:08] pipe expressions also need to be adjusted [10:08] from [10:08] ! echo foo bar | grep foo [10:08] zyga: I am only a bot, please don't think I'm intelligent :) [10:09] to echo foo bar | not grep foo [10:09] travis is pretty slow today [10:09] I'll start working on initrd while we wait [10:16] PR core-build#39 opened: tests: fix RuntimeWarning coroutine was never awaited [10:25] zyga: mmh, sounds like it needs more attention [10:25] pedronis: yeah, I've dropped it for now [10:26] pedronis: I will look in the evening, it's not hard, just tedious to look at 16 tests that fail [10:26] pedronis: and to move the not around [10:26] cmatsuoka, mvo: could you please look at https://github.com/snapcore/core-build/pull/39 [10:26] PR core-build#39: tests: fix RuntimeWarning coroutine was never awaited [10:27] mborzecki: shellcheck branch switch failed on portal :/ [10:27] zyga: heh [10:27] and auto refresh retry [10:27] * mborzecki is not surprised [10:30] mborzecki: how about https://github.com/snapcore/snapd/pull/6888 ? [10:30] PR #6888: tests: add mountinfo-tool [10:31] zyga: just clicked send [10:33] mborzecki: repleid [10:33] *replied [10:33] oddly everything is "outdated" in your review [10:33] did you start it a while ago before I git mv'd files [10:33] ? [10:40] zyga: hmm maybe [10:40] zyga: what i tought about with device being tuple: http://paste.ubuntu.com/p/78j96hf84m/ [10:42] mborzecki: mmmm, I see [10:42] mborzecki: I would have used nametuple for that [10:42] mborzecki: but I wanted to align more closely with C where that's a single variable [10:42] zyga: or that, don't know if it's worth the change though [10:43] mvo, hey [10:58] mborzecki: zyga: what was the consensus wrt ! ? [10:58] Chipaca: switched to --stable for now, zyga opened a PR with `not` helper [10:58] mborzecki: is that on master? [10:59] Chipaca: neither is, https://github.com/snapcore/snapd/pull/6897 https://github.com/snapcore/snapd/pull/6894 [10:59] PR #6897: tests/unit/spread-shellcheck: temporary workaround for SC2251 [10:59] PR #6894: tests: add "not" command as replacement for "!" in tests [11:00] ack [11:00] Chipaca: oh and opened an issue https://github.com/koalaman/shellcheck/issues/1588 [11:01] mborzecki: 'temporary workaround fo SCP 2251' (http://www.scp-wiki.net/scp-2251) [11:03] oh god must not get sucked in [11:03] * Chipaca updates his nerdsniping score [11:08] * zyga has a great moment by reading a particular man page!!! [11:18] zyga: tell us more [11:19] mvo: haha, nothing special really, just mkfs.ext4 can copy data into the filesystem in one go [11:19] so no root required :) [11:21] zyga: yeah, ubuntu-image is doing this [11:21] zyga: its pretty cool [11:24] zyga: i think that's what mborzecki was using fakeroot for [11:24] PR snapd#6899 opened: image: make prepare-image recovery-system aware [11:24] yup, otherwise it ends up owned by whoever ran mkfs.* [11:25] fakeroot mkfs.ext4 -d [11:25] wish mkfs.vfat supported that too :/ [11:28] mborzecki: dostools can copy files in and out [11:29] zyga: and that's what i'm using [11:29] zyga: what i meant is to mkfs.vfat have -d to popualte fs with some contents, instead of mcopy -i -s :: [11:30] yeah [11:31] pedronis: re base:none, we have code right now that installs "snapd" in prereq if core/snapd is not installed (for interfaces to work). should we keep doing this when installing a snap with base:none and there is nothing else in the system? [11:31] pstolowski: yes [11:32] k [11:33] mvo, I just updated the #6883 [11:33] PR #6883: tests: fix how strings are matched on auto-refresh-retry test [11:35] mborzecki: I made a pass over #6871, couple of comments [11:35] PR #6871: gadget: raw/bare structure writer and updater [11:36] pedronis: thanks! [11:47] PR snapd#6897 closed: tests/unit/spread-shellcheck: temporary workaround for SC2251 [11:47] * Chipaca lunches [11:52] mvo: can you cherry-pick 6897 to 2.39? [12:04] off to pick up the kids [12:10] mvo: can you review https://github.com/snapcore/snapd/pull/6888 [12:10] PR #6888: tests: add mountinfo-tool [12:28] zyga, about the mountinfo-tool, it is really nice [12:29] zyga, but I think that should be better if the test checks more stuff, I mean using the tool we check mount info on the system [12:29] zyga, perhaps it could be a following PR [12:29] also I can do it for sure [12:32] cachio: do you mean the spread test? [12:32] zyga, yes [12:32] cachio: yeah, it's just a start, I plan to work on it some more [12:32] zyga, nice, thanks [12:45]