=== Wouter01004 is now known as Wouter0100 [06:39] morning [07:02] mborzecki: hey :) [07:03] zyga: hey hey [07:03] how was your weekend? [07:03] mborzecki: easy landing https://github.com/snapcore/snapd/pull/7867 [07:04] PR #7867: tests: fix fwupd version regular expression <⚠ Critical> [07:05] mborzecki: https://github.com/snapcore/snapd/pull/7864 needs love [07:05] PR #7864: cmd/snap-mgmt, packaging/postrm: stop and remove socket units when purging [07:06] PR snapd#7867 closed: tests: fix fwupd version regular expression <⚠ Critical> [07:07] zyga: do you know the backstory of https://github.com/snapcore/snapd/pull/7865 ? [07:07] PR #7865: travis-ci: add go import path [07:08] mborzecki: no idea but probably allows one to travis a fork [07:09] zyga: https://golang.org/cmd/go/#hdr-Remote_import_paths [07:12] hm, why didn't that fail on < 19.04 [07:25] PR snapd#7854 closed: snap-confine: raise egid before calling setup_private_mount() [07:25] hello :-) [07:37] hey sdhd-sascha [07:37] PR snapd#7840 closed: tests: use test-snapd-sh snap instead of test-snapd-tools - Part 2 [07:42] zyga: I'm not sure if it made sense to split the travis jobs. Maybe they were summarized because of the total build-time. And it is difficult to get a free build slot on the online travis-platform... [07:43] sdhd-sascha: yeah, it's a weird limitation; I don't know if your PR should be merged or not yet. I think sergio should review it [07:44] mborzecki: can you please review https://github.com/snapcore/snapd/pull/7129 [07:44] PR #7129: userd: allow setting default-url-scheme-handler [07:44] zyga: but the gopath import would be nice. Because with it, it's possible to build on own github/travis account, before PR [07:44] it's pretty old and I think nobody looks at it [07:52] mborzecki: thinking about https://github.com/snapcore/snapd/pull/7205 [07:52] PR #7205: rfc: introduce confinement options failsafe flag [07:52] what can we do to make it landable? [07:54] zyga: hmm so the snap-mgm test fails bc there's lxd socket unit, turns out the socket units are there base image [07:55] ok, let's filter those out then [08:00] morning [08:00] hey :) [08:18] PR snapd#7866 closed: travis-ci: split integration tests into parts (>4MB log limit) [08:19] PR snapd#7868 opened: Revert "travis-ci: split integration tests into parts (>4MB log limit)" [08:21] pstolowski: mvo: hey [08:25] hey mborzecki [08:26] PR snapd#7868 closed: Revert "travis-ci: split integration tests into parts (>4MB log limit)" [08:27] PR snapd#7862 closed: release: 2.42.5 [08:33] zyga: heh, updated #7864 [08:33] mmm, looking [08:33] PR #7864: cmd/snap-mgmt, packaging/postrm: stop and remove socket units when purging [08:33] aha [08:33] smart [08:33] nice [08:35] mborzecki: going to review your seccomp branch this morning [08:35] pstolowski: cool, thank you! [08:35] pstolowski: can you do a pass over https://github.com/snapcore/snapd/pull/7863 [08:35] PR #7863: interfaces/builtin: add uio interface [08:36] zyga: yes, sure [08:36] sdhd-sascha: hey, re 7866 - I accidently merged it but it was not reviewed yet so I reverted it again. please just reopen (or I can reopen). sorry for this! fwiw, we only hit the 4mb limit under certain circumstances of failing tests. not sure we want to split generally - the downside of a general split is that we only have a fixed amount of travis slots aiui so it may slow things down [08:36] ha, wireguard has landed in mainline [08:38] coffee and 1-2-1 prep [08:38] ttyl [08:39] mvo: no problem. [08:39] sdhd-sascha: thank you! [08:40] mvo: could you test the PR with the go import path? with them it was possible to build snapd on my own repo and travis [08:41] sdhd-sascha: could you please merge it on master or rebase it on master - there was a bug in master that caused the integration tests to fail which got fixed this morning. this should make the test green. [08:42] mvo: so anybody can test building before any PR. [08:42] sdhd-sascha: also could you please elaborate a little bit on this works? it sounds very cool! [08:42] mvo: what should i merge? [08:42] sdhd-sascha: just fetch the lastest upstream from snapd and merge master into your PR. I'm happy to do this for you if you prefer [08:43] mvo: sure, but i'm also new on travis ;-) i'm just googling for a solution ... [08:43] sdhd-sascha: I see :) [08:43] mvo: ok [08:44] sdhd-sascha: so with 7865, what would I have to do to run my forked branch? what steps to follow etc :) ? (does my question make sense?) [08:50] mvo: as i understand, you'll be able to run the travis job on your private fork [08:50] mvo: with 7865. just add the go_import_path, and add your repo under "MyRepositories" on travis. Then manually build on travis or wait on the next git push [08:50] mborzecki: right [08:53] re [08:55] I will now merge master on my pull-request. Can somebody please cancel my travis-jobs ? I wait until it works my private repo first. [08:55] sdhd-sascha: jobs auto-cancel on push [08:55] ...on my private repo [08:55] zyga: ? [08:55] no, on your private repo you can cancel them [08:55] I was talking about the snapd repo [08:56] zyga: I'm too. I won't waste your build time [08:56] don't want [08:59] mborzecki, zyga : do either of you know what's going on here? https://bugzilla.redhat.com/show_bug.cgi?id=1780712 [08:59] it seems like snapd doesn't know how to read the path properly... [09:00] Eighth_Doctor: it's sudo dropping env [09:00] Eighth_Doctor: the keyword is sudo [09:00] blech [09:00] I was afraid of that [09:00] try sudo env [09:00] I need to jump to a meeting [09:00] Eighth_Doctor: https://forum.snapcraft.io/t/centos-7-error-system-does-not-fully-support-snapd-cannot-read-the-value-of-fs-may-detach-mounts-kernel-parameter/14392/5 [09:02] would "sudo -E" help ? [09:02] """-E, --preserve-env preserve user environment when running command""" [09:03] sdhd-sascha: iirc the security policy will still drop your current $PATH [09:03] sudo -i should work though [09:32] mvo: is there a reopen button on github for 7866, or should i opened a new pr [09:36] re [09:36] sdhd-sascha: I think you need to open a new PR [09:37] sdhd-sascha, mborzecki thanks for explaining how 7865 works [09:48] Eighth_Doctor: do you think the proposed solution in https://bugzilla.redhat.com/show_bug.cgi?id=1691996 is workable? [09:48] Eighth_Doctor: btw, I need to thank you for excellent maintenance of snapd in Fedora. I've been using Fedora for a few weeks on my daily driver and it's nothing short of excellent [09:49] Eighth_Doctor: it really helps me to develop cgroupv2 features [09:50] PR snapd#7869 opened: travis-ci: split integration tests into parts (>4MB log limit) [09:51] mvo: i had to reapply the commit, because of the revert [09:52] sdhd-sascha: yeah, again, sorry for the trouble :( won't happen again [09:54] PR snapd#7853 closed: [RFC] boot,bootloader: setup the snapd_recovery_kernel grubenv [09:56] zyga: will you be pushing fixes to 7129? [09:56] checking [09:57] what kind of fixes? [09:57] zyga: just added a bunch of comments, but i can push the fixes too if you're busy with somehting else [09:57] mborzecki: you forgot to click send [09:57] there are no new comments [09:57] zyga: try refreshing [09:57] mborzecki: did [09:58] again [09:58] zyga: https://github.com/snapcore/snapd/pull/7129#pullrequestreview-328794160 [09:58] PR #7129: userd: allow setting default-url-scheme-handler [09:58] wow, now I see them [09:58] haha ;) [09:58] man, eventually consistent my ass [09:58] yeah, go for it [09:58] thank you for looking [09:58] ok [09:58] trivials anyway ;) [09:58] feels like something close [09:58] I'll jump back into https://github.com/snapcore/snapd/pull/7625 [09:59] to get it unblocked [09:59] PR #7625: cmd/snap-confine: stop being setgid root [09:59] but really, the desktop stuff is like duct tape and glue all the way [09:59] mborzecki: notice that 7129 is blocked on jdstrand review [09:59] mborzecki: yeah [10:00] pedronis: right, i can ping jdstrand_ in case he missed it (though i'm sure it's in his queue) [10:00] it's in his queue [10:13] PR snapd#7768 closed: interfaces: add raw-volume interface for access to partitions <⛔ Blocked> [10:14] PR snapd#7768 opened: interfaces: add raw-volume interface for access to partitions [10:37] morning folks [10:38] hey ijohnson [10:38] hey zyga [10:39] hey ijohnson [10:39] mvo: did you see my comment on the SU doc? I didn't make it very far in updating the grub.cfg.normal for the UC20 pc gadget, also I had it in my notes that grub.cfg.recovery was the ubuntu-seed partition but reading through it, I think grub.cfg.recovery is for ubuntu-boot no? [10:40] reading the grub.cfg specifically [10:42] ijohnson: recovery goes to ubuntu-seed and *not* recovery goes to ubuntu-boot [10:43] pedronis: Hmm that's not how it's written now though, the grub.cfg.recovery is currently doing the chainloading to the recovery system grub IIUC [10:43] ijohnson: sorry, we are talking different things [10:44] sure, it is like 4 am here so not surprised I have something confused :-) [10:44] ijohnson: that should happen only for mode != run [10:45] not saying that is what happens right now, neither are definitive [10:45] or close to that [10:45] ijohnson: I have not yet, will look at it next, sorry! [10:45] which is part of the problem [10:45] ijohnson: I mean in run mode we should chain from ubuntu-seed recovery grub into ubuntu-boot grub [10:46] mvo: no problem just wanted to make sure you were aware if you needed someone else to work on this before Wednesday [10:46] ijohnson: yeah, that's fine, I figured :) [10:46] the recovery systems should really play a role only if mode != run [10:46] ijohnson: hope the trip was fine? [10:46] pedronis: yes I understand that part, I'm just a bit confused which grub file in the repo corresponds to which partition [10:47] ijohnson: recovery should correspond to seed, and the other to boot [10:47] if it's the other way around right, things are more messed up than I thought [10:48] mvo: actually I found a flight this morning so I'm at the airport waiting for my flight (hence being up at 4 am)... Hopefully the trip is fine it's currently snowing and we're supposed to get like 10 cm+ of snow this morning [10:49] ijohnson: uh! good luck! [10:49] pedronis: it's possible I am wrong about what it's currently doing but the recovery grub seems to be doing the logic to look for a recovery system to boot into [10:49] ijohnson: also - 4am sounds brual [10:49] ijohnson: brutal :) [10:49] ijohnson: was wonder about the early time :) [10:49] ijohnson: that's exactly what I would expect for seed [10:50] seed == recovery [10:50] pedronis: right that's why I was confused as well [10:50] Err wait sorry I misread [10:51] it looks correct, maybe there is no run mode logic there yet [10:51] I don't know [10:51] it's all part of the problem [10:52] I think there is no run mode logic in the recovery grub yet [10:52] pedronis: why would the seed recovery system grub need to look for all the other recovery systems? Is that by design that you can switch between recovery systems using grub? I guess that kinda makes sense now thinking about it [10:52] mvo: yeah it's a _bit_ early :-) [10:54] mvo: ah [10:55] pedronis: it's something to discuss with xnox today [11:26] brb, cold, going to make tea [11:34] pstolowski: thanks for the comments, updated #7821 [11:34] PR #7821: interfaces/seccomp: parallelize seccomp backend setup [11:34] PR snapd#7864 closed: cmd/snap-mgmt, packaging/postrm: stop and remove socket units when purging [11:36] yw, thanks for the PR! [11:37] re [11:39] PR snapd#7870 opened: boot,bootlaoder: setup the snap recovery system bootenv [11:47] pstolowski: good catch, mborzecki https://github.com/snapcore/snapd/pull/7821#discussion_r355405488 [11:47] PR #7821: interfaces/seccomp: parallelize seccomp backend setup [12:00] sdhd-sascha: https://github.com/snapcore/snapd/pull/7865#pullrequestreview-328870384 [12:00] PR #7865: travis-ci: add go import path [12:09] zyga: already been addressed [12:09] mborzecki: thanks [12:09] zyga, pedronis did you want jdstrand_ 's review on https://github.com/snapcore/snapd/pull/7830 or can it land? [12:09] PR #7830: interfaces: include hooks in plug/slot apparmor label [12:10] not really but I think it's short and would be good to keep him in the loop [12:11] zyga: sure [12:19] zyga: just started a new build on my repo [12:19] sdhd-sascha: you don't need to use repo builds you know [12:19] zyga: could you see, this build from me ? : https://travis-ci.org/sd-hd/snapd/builds/621989511 [12:19] there are faster ways to iterate [12:19] you can just use spread on a local system [12:19] zyga: kitchen was on my plan [12:19] zyga: kitchen is local travis [12:20] zyga: the build above, from weekend. has this patch and only debian sid is failed. all others are green [12:21] PR snapd#7850 closed: apparmor: allow 'r' /sys/kernel/mm/transparent_hugepage/hpage_pmd_size [12:23] zyga: any blockers to merge #7228 ? [12:23] PR #7228: interfaces: add audio-playback/record and pulseaudio spread tests [12:23] pedronis: checking [12:25] zyga: i do more tests, i will report [12:26] pedronis: I think it's good [12:27] PR snapd#7871 opened: tests: add spread test for hook permissions [12:28] PR snapd#7228 closed: interfaces: add audio-playback/record and pulseaudio spread tests [12:33] zyga: this possibly will be in linux 5.6 https://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git/commit/?h=work.openat2&id=b767b87044c4b15c4f222daf6948a7a43f802f7e [12:33] yeah, I noticed on twitter yesterday [12:33] along with the new mount APIs it's great to see those [12:34] I don't have a clear vision of how to adopt such new APIs in snapd [12:34] as we will likely support the old APIs for foreseeable future [12:41] zyga: I pushed something to #7768, let me know whether it is what you expected [12:41] PR #7768: interfaces: add raw-volume interface for access to partitions [12:41] looking [12:43] pedronis: looks great, thanks [12:45] zyga: I am a bit confused by https://github.com/snapcore/snapd/pull/7768/files#r355413333, that's a go regexp, not something we feed apparmor [12:45] PR #7768: interfaces: add raw-volume interface for access to partitions [12:45] oh, did I misunderstand that [12:45] uff [12:45] sorry [12:46] I didn't notice that [12:46] I just noticed composition and was worried [12:46] commented [12:47] given that hotplug is for follow up (and we don't have an immediate use case), I think it can land if it gets green [12:47] I agree [12:47] hotplug is should be more careful, it's not as easy as uio as it is fairly dynamic in nature [12:48] and covers a wide variety of cases [12:48] but I think it's a natural case to attempt [13:04] * zyga small break [13:08] * Chipaca lunch [13:10] PR snapd#7872 opened: doc: HACKING.md change autopkgtest-trusty-amd64.img name [13:11] Can anybody stop my Travis-Build for modifying "HACKING.md" ? [13:19] pstolowski: what's the status of this: https://bugs.launchpad.net/snapd/+bug/1849564 [13:20] Bug #1849564: snap connections doesn't work as expected when interface attributes change === ricab is now known as ricab|lunch [13:30] mvo: zyga: I think we have finally fixed this: https://bugs.launchpad.net/ubuntu/+source/snapd/+bug/1814141 ? [13:31] Bug #1814141: fail to run any snap after snapd refresh, reinstalling snapd from the archive is a temporary fix [13:32] mvo: I suppose this should be closed somehow at this point: https://bugs.launchpad.net/ubuntu/+source/snapd/+bug/1739494 [13:32] Bug #1739494: snapd 2.29.4.2 is not installable on powerpc [13:34] pedronis: totally [13:41] pedronis: it's probably another case of https://bugs.launchpad.net/bugs/1848516 (as suggested in the report) but i'd need to double check if it's fixed too. note that in the bug report the plug name was changed, that's why it's the same problem (i think). if it was just attributes change, then i think we would hit the (known) problem of stale attributes on refresh that we only fixed for content interface [13:41] Bug #1848516: snap connections/interfaces shows dropped interfaces as connected after refresh [13:51] PR snapd#7873 opened: image: set recovery system label when creating the image [14:12] mvo: is it won't fix or fix released? [14:30] mvo: how are things going with getting 2.42.5 rolled out? [14:37] stgraber: looking good, we are waiting for the cert team right now to give us a +1 for moving to candidate, cachio has the details on where we stand here [14:38] pedronis: please don't land 7768 yet. some of the changes that went in I want to change === jdstrand_ is now known as jdstrand [14:39] jdstrand: you'll have to tell me more [14:39] jdstrand: which of the changes? [14:39] jdstrand: the test changes or the path changes [14:40] pedronis: in ijohnson's comment, there needed to be a change, but not to rip out the mmc and vda1 [14:40] jdstrand: they aren't used === ricab|lunch is now known as ricab [14:40] pedronis: I know. that is what needed to change, to use them, not to remove them [14:40] jdstrand: the code doesn't care where the slot come from [14:40] so not sure what difference it would make [14:41] jdstrand: I mean, I follow up sounds ok, I don't think anything fondumental is untested at this point [14:41] I picked those 3 paths specifically so make sure it worked in the normal case, the requested mmc case and the i2o case [14:42] the tests aren't done yet so I'd like to add it back [14:42] plus, I don't understand the s#vda1#vda1/# change [14:43] I only am just looking at it, but I want to understand why that was changed before committing [14:43] jdstrand: that tests the Clean [14:43] it wasn't tested [14:43] and zyga asked about it [14:44] we are actually likely missing tests about that for other interfaces that also have a Clean fwiw [14:47] jdstrand: anyway I don't see any fundamental blockers, but of course more testing is alway appreciated [14:48] pedronis: now I'm rethinking that clean is there at all. the regex is very precise [14:48] that clean should be there* [14:48] jdstrand: as I said to zyga all the other interfaces like this one do that [14:48] the Clean [14:48] so it would be incosistent not to have it at this point [14:49] jdstrand: see serial-port or spi [14:49] etc [14:49] I don't know what it was decided to do it that way [14:49] s/what/why/ [14:50] pedronis: yeah, but maybe that is wrong. why would we allow /dev/././././vda1//////// [14:50] jdstrand: we cannot simply change it, you would need to start with warning in review-tools [14:50] for the old ones [14:50] pedronis: well, I'm talking about this one [14:51] it's a well defined process, clean and then check [14:51] and consistency is important [14:51] but consitently wrong? [14:51] I didn't make the initial decision [14:52] it is not a bad pattern in general [14:52] just conceptually [14:52] but seeing /dev/vda1/ - that is not what we want to allow [14:52] jdstrand: but we do allow it [14:52] there are things, like i2o, that are a subdir [14:52] /dev/i2o/ [14:53] that's the code you wrote [14:53] I just refactored it [14:53] pedronis: yes, and I am rethinking that after seeing the test for /dev/vda1/ [14:54] jdstrand: ok, but then you need to give a list of other interfaces you think we ideally should remove the Clean [14:54] of course, /dev/i2o/ going to /dev/i2o doesn't match the regex, but still [14:54] because is everywhere atm [14:54] which is probably why you also put it in the first place [14:55] it is, I copied an existing over as a template [14:55] nice 500s from the forum [14:55] jdstrand: what I'm saying, you either convince me it's exactly a problem only for the new one, or which it is, and a path forward [14:55] pedronis: we should've been doing if path != filepath.Clean(path) err [14:56] just changing it for this one feels arbitrary [14:56] well, bugs happen. sometimes you only notice after a while [14:56] I just noticed this [14:57] I'm not sure consistency is a reason to add another one [14:57] that's ok, but we still need a path forward for everything [14:57] also because otherwise we'll get there again [14:58] jdstrand: 1 thing doing it one way, and 5 another is not a pattern [14:59] pedronis: I'm not saying that none of them can or can't change, but my point of 5 historical and everything else going forward as correct is a pattern. the 5 historical can be documented in the code [14:59] what's the problem with 'path != filepath.Clean(path)'? [15:00]