[06:19] good morning [06:20] arnatious: hey, can you tell me how you created the container and where is it running? [06:20] arnatious: host info, lxd version, container info, etc [06:21] hey zyga [06:21] :) [06:21] how are you doing? [06:22] mvo: back to fighting that user test? [06:22] mvo: meanwhile: https://github.com/snapcore/snapd/pull/7337 is simple and green [06:22] PR #7337: tests: re-enable mount-ns test on classic [06:23] if you agree it is simple, that is :) [06:23] zyga: yeah, that and one more flaky one [06:23] Where is my config stored when using irssi as a snap? [06:24] I had a look at the two leaking lets that are on core [06:24] Aavar: most likely in ~/snap/irssi/current/.irssi [06:24] zyga: looking at the prs now [06:24] mvo: and one of them was very puzzling, the one I pasted last night [06:24] but I'll re-read that with fresh minnd [06:24] *mind [06:24] the pastebin is https://paste.ubuntu.com/p/Pdx9qmqbdH/ [06:26] mvo: btw, one surprise from yesterday is ordering of prepare calls [06:26] mvo: prepare-project-each is before prepare-suite-each [06:26] I was somewhat assuming that prepare-project-each is like prepare-task-each which doesn't exist [06:27] mvo: in that pastebin I linked to the part I don't understand is where did the revision 1001 and 1002 come from [06:27] where normally the revision map shows 36 and similar numbers [06:31] zyga: from our fake store maybe? [06:31] zyga: just guessing at this point, we setup a fake store for some tests [06:33] Ahhh, interesting. That must be it! Thank you [06:48] hey pedronis [06:48] I'm just looking at unit test failure in one of your PRs [06:48] https://www.irccloud.com/pastebin/CKcptuEn/ [06:48] I've seen this a few times recently [06:50] weird error, I wonder why is not deterministic [06:50] anyway I'm working in that area [06:50] thanks :) [06:50] so don't think you should spend time on it [06:50] ok [06:50] I'll focus on review [06:51] pedronis: https://github.com/snapcore/snapd/pull/7329 can land, I think [06:51] PR #7329: snap: explicitly forbid trying to parallel install from seed [06:51] PR snapd#7329 closed: snap: explicitly forbid trying to parallel install from seed === pstolowski|afk is now known as pstolowski [06:59] hello [07:00] good morning :) [07:00] hey pstolowski - goooood morning :) [07:09] PR snapd#7340 closed: tests: adding support for arm devices on ubuntu-core-device-reg test [07:14] zyga: 7338 has conflicts [07:14] PR snapd#7339 closed: tests: don't look for lxcfs in mountinfo [07:16] mvo: on it [07:16] done [07:17] PR snapd#7335 closed: tests: add check for snap_daemon user/group [07:19] pedronis: https://github.com/snapcore/snapd/pull/7323 is something you should review I think [07:19] PR #7323: features, overlord: make parallel-installs exported, export flags on startup [07:19] it deals with feature flags and startup behavior [07:20] added to my list [07:20] zyga: that failing test is weird [07:21] it's broken by design [07:21] but a bit unsure why it fails only rarely [07:21] those kinds are most interesting :) [07:25] pstolowski: did you add the MissingBase test to first boot stuff? [07:27] pedronis: checking [07:30] pedronis: yes, TestPopulateFromSeedMissingBase [07:30] pstolowski: something is odd with it [07:30] zyga: 7336 should be an easy review [07:30] pstolowski: unasserted goes into seed.yaml, not the snap [07:30] but is working anyway, most of the time [07:30] but sometimes it fails [07:30] mvo: mhm [07:30] trying to understand why [07:31] pstolowski: anyway, mostly pointing out that unasserted: true is in the wrong place [07:31] mvo: +1 [07:31] * zyga reviews https://github.com/snapcore/snapd/pull/7313 [07:31] PR #7313: many: add the start of Core 20 extensions support to the model assertion [07:34] pedronis: hmm, sorry about that, shall i investigate? [07:35] pstolowski: no, I'm looking into it [07:40] ok, thanks [07:43] thank you zyga ;) [07:47] Aavar: for what? :) [07:51] pstolowski: here's the cleanup/fix, fyi: https://github.com/snapcore/snapd/pull/7341/commits/709e0bd10c78fc74334d7660c7549647d04d5232 [07:51] PR #7341: many: introduce package seed and seedtest [07:56] pstolowski: it was creating variant of local both unasserted and asserted (under the foo* stuff), and sometimes they diverged (not sure why) [07:57] pstolowski: anyway, a bit of confusion all around there :) [07:58] pedronis: aaah i see [08:12] PR snapcraft#2688 closed: Build base types [08:13] pedronis: https://github.com/snapcore/snapd/pull/7313#pullrequestreview-280011112 [08:13] PR #7313: many: add the start of Core 20 extensions support to the model assertion [08:15] mvo: hey [08:15] mvo: look at this: [08:15] + getent passwd snap_daemon [08:15] snap_daemon:x:584788:584788::/nonexistent:/usr/bin/false [08:15] mvo: full log https://api.travis-ci.org/v3/job/576917244/log.txt [08:15] 2019-08-27 07:47:28 Error restoring google:arch-linux-64:tests/main/system-usernames-illegal : [08:16] also suspicious there [08:16] + snap remove test-snapd-illegal-system-username [08:16] snap "test-snapd-illegal-system-username" is not installed [08:16] typo? [08:16] mvo: this was in https://github.com/snapcore/snapd/pull/7316 [08:17] PR #7316: tests: add unit tests for cmd_whoami [08:17] * zyga reviews https://github.com/snapcore/snapd/pull/7341 [08:17] PR #7341: many: introduce package seed and seedtest [08:21] PR snapd#7344 opened: snapstate: use strutil.VersionCompare() in checkVersion() [08:22] PR snapd#7333 closed: tests: fix system version check on listing test for external backend [08:24] PR snapcraft#2689 opened: schema: kernel and snapd types do not use bases [08:24] zyga: nice, thanks, let me check [08:24] zyga: so the catcher of the test thing did find something [08:24] indeed [08:24] look at the log in detail, I'm sure there's something there that's useful [08:25] zyga: yeah, look [08:25] xnox: mvo does `type: kernel` always require a base or will any kernel snap work with any base? [08:27] mvo: pedronis and from last week, do we want the `type: snapd` to NOT specify a base or do we want it to specify a base (even if none or bare)? (unrelated to using a build-base) [08:29] sergiusens: I think we do not need a base for snapd, its self contained [08:29] sergiusens: the kernel is tricky, if it uses hooks it needs a base so that we know how to run the hooks [08:30] hey Chipaca, good morning! [08:30] mvo: 👋 ! [08:30] Chipaca: can I pester you with a question already? I got: [08:30] $ snap switch test-snapd-assumes --edge [08:30] "test-snapd-assumes" switched to the "edge" channel [08:30] Channel edge for test-snapd-assumes is closed; temporarily forwarding to candidate. [08:30] Chipaca: today which is a bit odd because the edge channel is open (and was open at the time of this message). does that ring any bells? [08:31] mvo: was this in a spread test, or somewhere you have debug logs? [08:32] huh [08:32] Chipaca: I ran this manually, was doing some exploring about "assumes:" this morning [08:32] it's reproducible [08:32] cool! [08:33] zyga: this is a fun issue, the logs show that the snap_daemon user is added *before* test-snapd-illegal-system-username is POSTed to the daemon. very puzzling [08:34] POSTed? [08:34] as in installed? [08:34] whaat? [08:34] :D [08:34] mvo: I'll be back soon, let me make some tea [08:35] zyga: yeah, at least if the journald logs are in order [08:35] zyga: yeah, no worries, just wanted to share my puzzlement [08:36] mvo: is 381b45e27773eff87bca4f8ed9b96d027bfb8ee5 part of 2.41~pre1 ? [08:36] mvo: i suspect that's the issue [08:38] zyga: found the issue, its "fun" [08:38] Chipaca: let me check [08:39] Chipaca: it is part of 2.41 it seems :/ [08:39] mvo: maybe we should back that out until we have time to fix it [08:39] Chipaca: could we just revert it in 2.41? [08:40] Chipaca: yeah, I think that makes sense [08:40] mvo: oh [08:40] mvo: tell me :) [08:42] zyga: pr will be up shortly but its easy once spotted [08:42] zyga: I tried to answer to some of your model assertion comments [08:42] pedronis: thanks, let me look [08:44] mvo: so for kernel we will require a base for now (it can be bare) [08:44] pedronis: I replied to some but it all makes sense [08:45] pedronis: LGTM and iterate or iterate and LGTM, as you prefer :) [08:50] PR snapd#7345 opened: overlord/devicestate,seed: small step introduce seed.LoadAssertions and use it from firstboot [08:51] PR snapd#7346 opened: snapstate: validate all system-usernames before creating them === Greyztar- is now known as Greyztar [08:51] zyga: ^- thats the one [08:51] /o\ [08:53] mvo: did we ship this? [08:53] zyga: yeah, also kind of annoying that we overlooked this in the code review [08:53] zyga: well [08:53] :E [08:53] zyga: its in beta [08:54] mvo: should such usernames be made at the time the snap is linked? [08:54] mvo: I'm looking at this now and it's unclear when this runs [08:55] it runs early but this usernames are shared [08:55] so they will end up staying around either way [08:55] in many cases [08:55] pedronis: that's okay, I worry that the sequence now is [check-foo, check-users, check-bar] [08:56] and that check-bar fails but we already acted a little on the system, which feels wrong [08:56] as I said, for the current one support user is ok [08:56] but when we add more we need to revisit this [08:56] it probably merits a TODO in that PR though < mvo [08:56] mvo: how is the patch making things better tough [08:56] it's unclear to me [08:57] we either create all users or none [08:57] we might still fail to install the snap [08:57] later [08:57] ah, indeed [08:57] but given that the one support user [08:57] is shared [08:58] it's not too bad [08:58] but it might be worth revisiting at some point [08:58] pedronis: indeed, let me add this todo [09:17] PR snapd#7347 opened: gadget: do not error on gadget refreshes with multiple volumes [09:19] mvo: multi volume as in multi partition? [09:19] mvo: or multiple disks [09:20] zyga: multiple disks [09:20] mvo: do we have customers using that? [09:20] or is this just a precaution? [09:20] zyga: we have customers using this and this is breaking ondra [09:20] +1 [09:20] thanks for explaining [09:22] mvo: left a suggestion, +1 anyway === vicamo_ is now known as vicamo [09:44] zyga: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=923500#10 [09:44] PR #10: Update README.md [09:44] popey_: looking [09:44] is that comment still accurate? [09:44] ah that one [09:45] it didn't move forward, we discussed accepting partial confinement with actual apparmor enabled but it was nacked for existing distributions [09:46] as we don't have the time to deal with regressions if that broke people [09:46] that's what the state of this is now [09:46] Worth updating the bug? [09:46] it's not a great message but yeah [09:46] if only updating bugs in debian didn't involve email [09:51] mvo: approved #7346 with a comment [09:51] PR #7346: snapstate: validate all system-usernames before creating them [09:54] pedronis: thanks, looking in a wee bit === pedronis_ is now known as pedronis [10:05] * Chipaca takes a break [10:05] zyga: Container created on Ubuntu 18.04, LXD stable/3.16 snap, container config https://paste.ubuntu.com/p/JrcC89rwN2/ [10:16] PR snapd#7348 opened: snapstate,store: check assumes early before downloading the snap [10:25] Chipaca: bad news, it seems like reverting 7204 still gives the same error message, I can bisect [10:26] mvo: rats [10:26] mvo: is it actually a regression? [10:26] Chipaca: not sure, let me try [10:27] Chipaca: aha, I think I'm a muppet, hold on, let me retest this [10:29] mvo: Rowlf? [10:29] Chipaca: that sounds about right [10:30] Chipaca: maybe "beaker" just needs to be renamed to "breaker" [10:33] PR snapd#7349 opened: snap: revert PR#7204 as breaks `snap switch` output [10:33] pstolowski: your input here would be great :) [10:35] mvo: https://www.youtube.com/watch?v=VnT7pT6zCcA [10:37] mvo: oh :( interesting, saying 'stable' instead will be confusing as well [10:45] pstolowski: aha, this is latest/stale vs stable? the new default-channel stuff? [10:45] Chipaca: lol - this is great - his expression matches my mood perfectly [10:46] mvo: yes... and with this revert we'll be showing *wrong* (imprecise/misleading) information when the snapd-side logic of keeping the track kicks in [10:47] zyga, hey, when you have time could you please take a look to #7110 [10:47] PR #7110: tests: new test to check the output after refreshing/reverting core [10:47] zyga, hope last one [10:48] pstolowski: meh, ok [10:48] pstolowski: so we need something else I guess :/ [10:48] pstolowski: there is a spread test in there that might be useful [10:48] pstolowski: the other parts can probably be removed then === alan_g_ is now known as alan_g [10:51] mvo: thanks, +1 and i'll work on this soon [10:55] pedronis: I updated 7346 with a unit test, thanks for the suggestion! [10:55] cachio: in an hour [10:55] zyga, sure, thanks [11:08] pedronis: would love your opinion on 7348 too :) but no rush [11:09] mvo: finishing lunch break [11:10] pedronis: yeah, no rush, I will soon have lunch too - I was just surprised how small it was [11:17] mvo: commented [11:19] PR snapd#7350 opened: tests: clean snap_daemon user and group on system-usernames-illegal test [11:47] PR snapd#7351 opened: tests: retry checking until the written file on desktop-portal-filechooser [11:52] sergiusens: at the moment, `type: kernel` works with any bases, but which base is used matters. As initramfs.img is build with binaries from the base. Thus one should be specified, and it does matter if it's core16, core18, or core20 cause then the initramfs.img has libc6 and etc. from those matching releases xenial / bionic / $devel [11:58] re [12:07] xnox: the plan is that kernels should use/specify build-base: not a base: [12:09] pedronis: yes, that is good. initramfs.img is self contained / can be used by anyone...... but it matters what it is built out of. More like "Built-Using: bionic" in "dpkg speak", despite having no "Depends:". [12:20] PR snapd#7346 closed: snapstate: validate all system-usernames before creating them === ricab is now known as ricab|lunch [12:37] PR snapd#7352 opened: tests: always say 'restore: |' [12:38] are draft branches visible by default? [12:39] I meant draft PRs [12:44] https://github.com/snapcore/snapd/pull/7337 needs a 2nd review [12:44] PR #7337: tests: re-enable mount-ns test on classic [12:45] jdstrand: hey, one correction to what i said yesterday: i was wrong when i said 'snap connect' could also be optimized for multiple connections at once like i did with automatic connections, i misremembered the code that resolves plug/slot names for connect (was thinking we can resolve multiple plugs/slots if you omit connect arguments, but we only resolve a single plug/slot if you omit them) [12:52] has anyone seen debian-sid-64 fail to prepare like this: https://pastebin.ubuntu.com/p/GQjg4jK4rw/ ? [12:53] I've seen it fail to prepare on numerous builds now and it doesn't look like a transient error [12:53] PR snapd#7353 opened: tests: simplify interfaces-account-control test [12:55] PR snapd#7354 opened: tests: rename fuse_support to fuse-support [13:00] PR snapd#7355 opened: [RFC] interfaces/apparmor: load multiple profiles in a single batch [13:01] augh, 2fa [13:04] PR snapd#7110 closed: tests: new test to check the output after refreshing/reverting core === alan_g_ is now known as alan_g === ricab|lunch is now known as ricab [13:55] pstolowski: thanks. it sounded like an exciting feature that I missed :) [14:11] jdstrand: :) [14:14] PR snapd#7352 closed: tests: always say 'restore: |' [14:17] why does it have to be so hot [14:27] PR snapcraft#2674 closed: file_utils: introduce link_or_copy_files [14:46] PR snapd#7203 closed: i18n, vendor, packaging: drop github.com/ojii/gettext.go, use github.com/snapcore/go-gettext [14:48] Chipaca: i'm working on a fix for the confusing snap switch message ("Channel %s for %s is closed; temporarily forwarding to..) caused by my #7204; i don't understand the logic of this temporary forward meesage but i assume it's fine to simply skip it for op = "switch" in showDone(), this will basically make "switch" behave as before, modulo the new track/risk behavior that will be shown correctly (thanks to [14:48] showDone) [14:48] PR #7204: cmd/snap: use showDone helper with 'snap switch' [14:48] Chipaca: sounds sensible? [14:48] pstolowski: sounds like the quickest way forwards yes [14:48] the logic probably assumes 'refresh' === ricab is now known as ricab|brb [14:51] Chipaca: ok, thamks [14:51] *thanks [14:53] PR snapd#7356 opened: i18n, vendor, packaging: drop github.com/ojii/gettext.go, use github.com/snapcore/go-gettext (2.41) [14:57] pstolowski: could you please do a pass over the three simple PRs I have [14:57] they are all tiny and I'd love to merge them [15:01] zyga: sure [15:01] thanks [15:01] there's one -0,+0 PR :D [15:01] PR snapd#7357 opened: Fix snap switch message [15:03] zyga: which one? [15:04] pstolowski: 7354 [15:06] PR snapd#7358 opened: Rename die() -> die_reboot() to fix build failures with LTO enabled [15:10] Chipaca: can you look at that, it looks good-ish but I'm worried about it slightly [15:11] zyga: the switch one? [15:11] no, the die one above [15:12] zyga: did two others, not sure if these were the ones you wanted first. let me know if you need any more [15:12] pstolowski: that's great, thank you [15:18] zyga: the intention was to use the same die from the lib, wasn't it? [15:25] Chipaca: original intention or one in this PR [15:25] zyga, when you have time, could you please atke a look to #7159 [15:25] Chipaca: the original die is not flexible enough [15:25] PR #7159: tests: add functions to make an abstraction for the snaps [15:25] Chipaca: I'm looking at fixing that === Luke_ is now known as Guest27501 [15:26] zyga, and this one also #7256 please [15:26] PR #7256: tests: adding retry command and use it to delete $XDG directory