[00:09] jdstrand: thanks! [06:14] morning [06:22] brb, new kernel [06:25] and back, 5.4.1 [06:25] PR snapd#7834 closed: tests: move the watchdog timeout to 2s to make the tests work in rpi [06:27] PR snapd#7833 closed: HACKING.md: add nvidia options to configure example [06:44] not sure what the purpose of tests/main/install-store-laaaarge is [06:55] PR snapd#7743 closed: snap-bootstrap: force partition table operations [07:06] mborzecki: that we don’t buffer snaps in memory perhaps? [07:06] Good morning [07:06] Still sleepy [07:07] Fought mould in the office late las night [07:12] zyga: ayy, not good, make sure to remove all of it [07:30] good morning mvo [07:31] hey zyga [07:36] mvo: hey [07:37] mborzecki: good morning! [07:42] ok, off to the bank, back ~12 hopefully [07:43] * zyga jumps into https://github.com/snapcore/snapd/pull/7825 [07:43] PR #7825: many: use transient scope for tracking apps and hooks [08:02] morning [09:03] Hello, [09:03] hey [09:03] :-) [09:03] on 19.04, when i run the test-suit, i get: [09:03] so, snap run --explain, there's a small twist in I/O sync across all the interacting programs [09:03] - {"", time.Time{}}, // zero time default [09:03] + {"", time.Time{}}, // zero time default [09:03] need to think how to make that all work cleanly [09:03] in the formattting test [09:04] sdhd-sascha: ignore it, it's busted [09:04] sdhd-sascha: we format with a fixed gofmt version [09:04] sdhd-sascha: unless you happen to have that it will always differ [09:04] well, on 18.04 it works and go's on... [09:04] is there a workaround [09:06] format with that version [09:06] On 18.04, it ./run-checks stops, with "Crushing failure and despair." [09:06] you can install it as a snap :) [09:06] yeah, it's silly [09:06] mvo: ^ [09:06] i installed go as snap [09:07] sdhd-sascha: just run the unit tests? ./run-checks --unit [09:08] :-) i will [09:08] What gofmt version i should install ? [09:11] brb, I'm starving [09:20] sdhd-sascha: try SKIP_GOFMT=1 ./run-checks [09:21] sdhd-sascha: i think it's the gofmt from go 1.10 but i might be wrong on this [09:21] sdhd-sascha: if you're running go from a snap, snap refresh --channel=1.10 go [09:22] Chipaca: SKIP_GOFMT works. I need this, because in the night i have to shutdown my noisy server ;-) thanks [09:27] noisy stuff in the home needs to die [09:39] Chipaca: right [09:45] mvo: what UC20 bits need reviewing? [09:45] What is best pratice? Build and install master branch? Is master mostly compatible with snapcraft store, or test this seperatly ? [09:46] sdhd-sascha: master is compatible with the store [09:46] sdhd-sascha: we take backwards compatibility seriously [09:47] * zyga breaks branch iteration to review stuff for 2.34 [09:47] 43 even [09:47] pedronis: 7831 should be ready, 7832 too, I'm just adding a spread test [09:56] mvo: 7831 has two +1 [10:00] PR snapd#7831 closed: gadget: extract and export new DiskFromPartition() helper [10:10] pedronis: thanks, merged and I updated 7832 against master plus it has a spread test now [10:10] pedronis: that should unblock 7762 :) [10:36] pedronis: reviewed https://github.com/snapcore/snapd/pull/7228#pullrequestreview-326719099 [10:36] PR #7228: interfaces: add audio-playback/record and pulseaudio spread tests [10:36] pstolowski: about your work on the label bug [10:37] pstolowski: do you think we can chop it into parts so that the several bugs this uncovered can be addressed in isolation [10:37] mvo: it's Ubuntu Core not Ubuntucore, so UC not Uc [10:37] zyga: i think so yes, nb i think i see the issue, just trying a tentative fix [10:37] oh, while are are at naming, why do we have Grubenv and not GrubEnv [10:37] it's rubbing salt into my eyes [10:37] pedronis: thank you, I keep this in mind [10:37] pstolowski: super, thank you for taking this [10:38] mvo: thanks, not the most important thing in the world but well [10:38] pedronis: yeah, we should be consistent! [10:41] mvo: is that right that cmd/snap-bootstrap/bootstrap is mostly tested via spread? [10:52] they are shooting an episode next door again [10:54] mvo: https://github.com/snapcore/snapd/pull/7832#pullrequestreview-326757177 [10:54] PR #7832: snap-bootstrap: support auto-detect device in create-partitions [11:05] zyga: thank you, looking [11:05] (was in a meeting) [11:05] no worries [11:08] zyga: ok, wrt to the label issue.. another problem confirmed and fix works, it's implicit stuff biting again [11:09] zyga: we bind implicit hooks when reading the yaml, but implicit slots arrive later to the picture :/ [11:10] we should probably tear all that apart and redo at some point, but this needs a good plan [11:22] pstolowski: as expected the, no disagreement there [11:23] short coffee break [11:24] it's not so cold today somehow [11:40] back [11:44] PR snapd#7835 opened: gadget: add missing test for duplicate detection of roles [11:55] mvo, hey [11:55] hey cachio [11:56] did you fix the test uc20-snap-recovery ? [11:56] in the last PR merged? [11:59] mvo, I see the tests passed during last execution on travis [11:59] but I see it is failing on master when I execute it [11:59] PR snapd#7836 opened: o/ifacestate: fix binding of implicit hooks to implicit slots on core snap [12:01] zyga: ^ [12:01] nnn [12:02] cachio: it seems racy [12:02] cachio: it's strange, I wonder what broke it [12:03] cachio: do you know when it started failing? [12:03] mvo, yesterday the bionic image was updated [12:03] perhaps something new is breaking it [12:04] I ran the tests yesterday for the new image and that test passed [12:04] cachio: there are also changes in the sfdisk code [12:05] cachio: 7743 landed [12:05] cachio: and after that I see an error on master and I also seen an error in my own stuff - but not in qemu so far (but only ran twice). runing on google now [12:06] mvo, nice [12:06] looks like we also need to improve the error, i.e. what exactly is not compatible [12:06] mvo, hehe, I'll try to reproduce the error using the previous image [12:06] so if I'm able to do it the problem should be in the code [12:07] * Chipaca needs to lie down for a while [12:09] pstolowski: reviewed [12:10] zyga: thank you; please also see my comment under 1st PR [12:10] ack [12:11] cachio: thanks! [12:12] zyga: as touched briefly yesterday, remaining 3rd issue is peer=snap.core.configure now generated for e.g. modem-manager plug on classic [12:14] hey, can i ask a off-topic question? i want to forward a unix-domain socket via ssh & netcat. With "ssh -- nc -q0 -U " i got a connection. [12:15] But now my application needs a socket-filename [12:15] i want with emacsclient securly connect to server (tramp plugin didn't work like expected...) [12:16] :-) [12:23] pstolowski: do we really need to bind hooks fore core, given that the only hook there is hijacked anyway? [12:25] cmatsuoka: hey, good morning [12:25] pstolowski: I'm basically wondering whether we can cut/simplify somehow differently this area [12:25] mvo, the test fails with the old image as well [12:26] cmatsuoka: "fun" - right now master is unhappy, it looks like 7743 sometimes works and sometimes does not. [12:26] cmatsuoka: it looks like adding "blockdev --rereadpt" fixes it [12:26] cmatsuoka: well, "fixes" [12:26] cmatsuoka: I wonder if we need our own code afterall :/ [12:26] pedronis: if we don't then label generation would need some other logic / special casing for core i think (for the reference - #7830) [12:26] PR #7830: interfaces: include hooks in plug/slot apparmor label [12:27] mvo: is it failing on the gadget vs device consistency checking? [12:27] pstolowski: I would like us to explore that [12:27] pstolowski: it's a bit silly to generate profiles for something we never run [12:27] core is anyway special cases in various ways [12:28] but I might be missing something [12:29] PR snapd#7837 opened: devicestate: implement creating partitions in "install" mode [12:29] cmatsuoka: yeah [12:29] cmatsuoka: exactly the issue you describe in the gdoc [12:29] cmatsuoka: seems to break master right now :( [12:30] cmatsuoka: do you think you could try just adding "blockdev --rereadpt" as a workaround? [12:30] cmatsuoka: need to get lunch in a wee bit but would love to have someone look at this while I'm away :) [12:30] mvo: yes, at which point you tried this rereadpt? [12:31] mvo: the error pattern is very odd, last night I couldn't reproduce it anymore after ~10PM UTC [12:32] pedronis: okay, i need to take a step back, and clear my mind. but lunch first [12:33] cmatsuoka: fun! I can reproduce it in gce [12:33] pstolowski: ok, we ignored the silly work because it didn't have consequences so far, but if it starts to give trouble we need to consider [12:33] cmatsuoka: but not in qemu [12:33] cmatsuoka: I see it in my PR everytime in gce and also when running manually I hit it right away [12:34] cmatsuoka: I did run "blockdev --rereadpt" after the failure manually i nthe shell and suddently things went away [12:34] cmatsuoka: thanks for looking into this, if you have further question please use tg, I will go to lunch now [12:34] cmatsuoka: but happy to help as good as I can [12:35] mvo: where did you insert the blockdev that fixes the problem? (I'm afraid it's more like it's disturbing the system enough to make a racy situation work again) [12:35] cmatsuoka: after the test failed I just ran it manually in a shelll [12:36] cmatsuoka: which is obviously not that great of an answer [12:36] * mvo really is away now [12:39] * cmatsuoka earns the "breaking master" badge 😱 [12:40] PR snapcraft#2829 closed: store cli: push title and license on push-metadata [12:53] PR snapd#7838 opened: cmd/snap-bootstrap: stub out snap.SanitizePlugsSlots for real [12:54] * Chipaca _really_ needs to lie down for a while, now [13:08] PR snapd#7839 opened: tests: prevent partitioning test errors [13:24] mvo: issued a quick workaround atempt, but also working on a more definitive solution that drops lsblk entirely and uses blkid instead [13:27] mvo: I removed the 2.43 milestone from snap run --explain, seems a bit unlikely to make it [13:37] pedronis: I just pushed an update there [13:37] with spread tests and more things that it does [13:37] but it can be added in later if you feel like it's not ready [13:38] zyga: for one it needs a jdstrand review, 2nd it needs careful review of each output line [13:39] pedronis: https://github.com/snapcore/snapd/pull/7728/files#diff-771e057220c48a44a34166289f5339f4R1 is the starting point [13:39] PR #7728: cmd: implement snap run --explain [13:41] we can tweak it all the way we want but I think, since it's not meant to be parsed by machines, it should go in after jdstrand's review [13:42] zyga: ? [13:42] I'm saying we should not go over the output with a fine comb and not ship it [13:42] zyga: machine parsing is not my worry here [13:42] we should ship it and improve it after people use it [13:44] zyga: we should start from a good place, atm the are open questions and the output formatting not very consistent [13:44] pedronis: open questions? [13:44] pedronis: what is not consistent? [13:45] zyga: the usage of colons for once [13:45] pedronis: what about it? [13:45] honestly, that's the last thing anyone using this for real will care about [13:46] but I don't think it's inconsistent now, maybe you are looking at the older version [13:46] zyga: maybe, it needs review either way [13:46] that's true [13:46] from me [13:47] I'd just hate to blow it out of proportion as to what it needs to be - it's a tool, not a designer hat; it's good to ship it if it's 80% there and can benefit someone [13:49] not more than any other snapd output [13:49] anyway jdstrand review is still the main blocker atm [13:51] zyga: the open questions is the remark Maciej made about separator, and also how that matches or not the original sketch plan [13:52] pedronis: the separator probably cannot be used if we want wrappers and other layer to handle explain-like output [13:53] pedronis: because while there's a clear start, there is no clear end [13:53] zyga: I agree but the last output had still missing bits vs the original sketch [13:53] not sure if your last pass fixed this or not [13:53] pedronis: I didn't check it against the sketch but I think the sketch is just that, there are TODOs to add more things based on what snap run really does [13:54] pedronis: I'll double check what's going on in the code and if there's something to add based on the original sketch [13:54] zyga: yes, but the sketch has a separator just before the start of the app command itself [13:54] pedronis: but my point is that it's meant to be realistic, not idealistic [13:54] which partly addressed Maciej worry [13:54] addresses [13:55] anyway my point stand that still need a proper review after jdstrand one [13:55] s/after// but yeah [13:55] I think we don't need to wait for jamie [13:55] you touch snap-confine [13:56] I'd like to review at least those bits [13:56] zyga: well I plan to wait, given I have also other things [13:56] not wait for the other review === hunterk_ is now known as hunterk [13:58] zyga: I suppose I could to a quick pass over the output in the spread test when I have a moment [13:58] thanks! [13:58] I can quickly adjust it after initial output review [14:00] is the output up-to-date ATM in tha test? [14:00] yes [14:00] ok, thx [14:02] cannot join standup [14:02] sorry [14:02] I'll join when I xan === ricab is now known as ricab|lunch [14:26] oh my [14:26] sorry guys [14:26] chrome love [14:30] mvo: there is, snap find mumble [14:30] heh [14:30] jdstrand: hey, question about modem-manager interface [14:31] ok [14:31] mvo: ijohnson: can we do it 30 mins before standup tomorrow? [14:31] that interface is pretty ancient [14:31] pedronis: mvo: yes that's fine for me [14:33] jdstrand: is this intended that AppArmorConnectedPlug always adds modemManagerConnectedPlugAppArmor snippet, and on classic in addition appends modemManagerConnectedPlugAppArmorClassic ? should there be if classic - else core? [14:36] pstolowski: I'm looking at it [14:38] jdstrand: that extra snippet gets label=snap.core.* on classic in the generated profile [14:40] pstolowski: so, like I said, this interface is ancient and doesn't benefit from the moderm thinking around app snaps, classic, etc. looking at it, if you install the snap on a classic system, you get a weird peer=(label="snap.core.*") rule [14:42] pstolowski: iirc, on or the other was tacked on later with the understanding that "you're not expected to install the modem-manager snap on a core system" [14:42] but, we've handled this better lately [14:42] pstolowski: eg, in avahi-control [14:44] pstolowski: PermanentSlot and ConnectedSlot could also be updated [14:45] pstolowski: basically, it and most likely bluez and network-manager would likely benefit from being updated to model after avahi-control. though, still, "you're not expected to install the * snap on a core system" with all of these :) [14:46] jdstrand: but you are definitely expected to install the network-manager and bluez snap on UC? [14:46] erf [14:46] pedronis, ijohnson sure, let me schedule something [14:46] and modem-manager snap too actually [14:46] s/core/slassic/ [14:46] classic* [14:46] ahhh yes that makes much more sense +1 [14:46] ijohnson: I totally typoed that :) [14:46] pstolowski: ^ [14:47] it's still early :-) [14:47] pstolowski: is this something you are thinking of fixing or should I add it to the list for the next batch of updates? [14:47] ijohnson: not sure you saw, but nice job on the lib caching forum topic :) [14:48] I think I did see, but thanks again :-) [14:48] :) [14:49] jdstrand: no, at least not now; it just popped because of this weird label (in one of the spread tests), which becomes even more weird with my #7830 [14:49] PR #7830: interfaces: include hooks in plug/slot apparmor label [14:50] pstolowski: ack, I'll add it to my list then [14:51] jdstrand: i though a simple if-classic-else-core in modem manager would do it, but sounds like you envison to streamline it more [14:51] *thought [14:51] jdstrand: thanks, and thanks for explaining [14:52] np [14:52] yeah, they just need updating. they're a bit crufty [14:56] mvo: thx, I see it in your cal but not mine [14:58] pedronis: yeah, sorry, was wondering if we should just use the 4:30 slot I have with ian tomorrow anyway - this would mean that ian does not have to get quite so early [14:58] pedronis: or the 5pm slot [14:58] PR snapd#7733 closed: tests: disable nova from install-snaps test [14:58] pedronis: after the certbot meeting [14:59] mvo: well the first slot overlaps with the certbot one, after the cerbot is fine with me if it works for you [15:00] pedronis: I would have to skip/move one meeting [15:00] mvo: it's ok, it's not too early [15:00] mvo: no strong preference, except I think I need to be at the certbot meeting [15:01] brb, lunch [15:01] (late) [15:18] ijohnson: it could either be 30min before the meeting or 2h later [15:18] mvo: whatever is most convenient for you, I don't have any other meetings except our 1:1 and the SU and it's only 7:30 so that's doable for me [15:28] Lunch over but I’m looking after Lucy now [15:29] I will return in about an hour [15:39] * cachio lunch === ricab|lunch is now known as ricab [15:58] re [16:03] PR snapd#7838 closed: cmd/snap-bootstrap: stub out snap.SanitizePlugsSlots for real <⚠ Critical> [16:04] xnox: ^ [16:18] ijohnson: thanks for the review [16:18] mvo, pedronis: hey, fyi, zyga gave his approval on PR 7228 and I incorporated his feedback. He said to feel free to merge, but I wasn't sure if that meant after another +1 or right away. I think there is a question in my mind since this PRs only adds spread tests and doesn't touch code, and not sure if 2 approvals is required for that [16:18] pedronis: np [16:18] PR #7228: interfaces: add audio-playback/record and pulseaudio spread tests [16:19] ijohnson: about the confusing code, it copied and just slightly modified from some code where the error can happen or not [16:19] (in any case, obviously I'll what for the tests to pass) [16:19] but I agree is confusing if the error is expected [16:19] jdstrand: let me look [16:20] pedronis: yeah that's kinda what I thought but still seems like either a comment explaining that or not doing it that way would be good, because I could see myself needing to write a similar test and taking this one as a template and then being very confused about why it was written this way [16:21] jdstrand: you probably want a 2nd review from some of the people that looked at it already, doesn't need to be me tough [16:22] pedronis: ok [16:23] I actually didn't look at it, just did a meta-comment at some point [16:24] jdstrand: I meant "from my pov it is ok" but I don't think I have the power to merge with one review [16:25] that's fine [16:25] * zyga fights snap security tag hydra [16:25] I didn't want to assign anyone to review it, so I asked in a comment if Ian or Maciej wants to do it [16:29] jdstrand: I can take a look today probably [16:30] ijohnson: thanks. it is only as urgent as cleaning out old PRs is [16:30] ack [16:32] jdstrand: merge master into it if you haven't done that recently please [16:34] mvo: the empty filesystem bug seems to be a race involving node creation and the udev event queue, I'll test a queue flush after partitioning and before creating the filesystems [16:35] zyga: yep, already done [16:35] I learned from last time [16:35] super, just wanted to double check after recent old-branch-post-merge-fixups [16:35] :) [16:36] I think we all learned, it's a shame CI is too costly to run on each master change [16:39] PR snapd#7826 closed: tests: use test-snapd-sh snap instead of test-snapd-tools - Part 1 [16:41] PR snapd#7840 opened: tests: use test-snapd-sh snap instead of test-snapd-tools - Part 2 [16:46] cmatsuoka: cool, thanks for digging into this [17:22] ijohnson and bloodearnest: fyi, https://github.com/sergiusens/snapcraft-preload/pull/39 [17:22] PR sergiusens/snapcraft-preload#39: preload.cpp: use setgroups()/initgroups() in sandbox-compliant manner [17:22] \o/ yay thanks jdstrand [17:32] I feel cold getting me [17:32] Need a break [17:32] Sleepy [17:56] PR snapcraft#2830 opened: elf: properly handle corrupted ELF files [18:29] PR snapcraft#2828 closed: Appstream [18:44] PR snapcraft#2831 opened: appstream extractor: add support for code [18:45] cold meds are working so I'm back [18:45] wondering how to proceed with snap security tag validation [18:45] security tag are super diverse [18:45] and it feels like something to make tidy now [18:49] PR snapd#7839 closed: tests: prevent partitioning test errors [19:39] PR snapd#7841 opened: tests: fix partitioning test debug message [20:02] cachio: PR #7841 fixes a silly shell error in the previous PR [20:02] PR #7841: tests: fix partitioning test debug message [20:04] cmatsuoka, taking a look [20:05] cachio: when the udev fstype error happens, grep fails and the test was aborted [20:05] cmatsuoka, ah, ok [20:05] thanks for the explanation [20:05] +1 [20:07] cachio: I was expecting ID_FS_TYPE to be there with an empty value when it fails, but in fact the key is not listed [20:08]