mup | PR snapd#10139 opened: cmd/snap-bootstrap/initramfs-mounts: mount ubuntu-data nosuid <Bug> <Needs security review> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/10139> | 00:14 |
---|---|---|
=== jamesh_ is now known as jamesh | ||
mborzecki | morning | 05:46 |
zyga | good morning | 06:29 |
mborzecki | zyga: hey | 06:47 |
zyga | o/ | 06:47 |
zyga | that was one long review: https://github.com/snapcore/snapd/pull/10124#pullrequestreview-634232174 | 06:58 |
mup | PR #10124: tests: update layout for tests - part 1 <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/10124> | 06:58 |
zyga | good morning mvo | 06:58 |
mvo | good morning zyga and mborzecki | 07:01 |
mvo | zyga: oh, woah, thanks for this review! | 07:01 |
mborzecki | mvo: hey, after you've had your morning tea, can you take a look at https://github.com/snapcore/snapd/pull/10130 ? | 07:04 |
mup | PR #10130: boot: load bits of kernel command line from gadget snaps <Run nested> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/10130> | 07:04 |
mvo | mborzecki: sure | 07:05 |
zyga | https://github.com/snapcore/snapd/pull/10125#pullrequestreview-634235979 and I'm back to my work | 07:07 |
mup | PR #10125: tests: update layout for tests - part 2 <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/10125> | 07:07 |
mup | PR snapd#10140 opened: boot, overlord/devicestate: consider gadget command lines when updating boot config <Run nested> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/10140> | 08:35 |
=== popey1 is now known as popey | ||
mup | PR snapd#10124 closed: tests: update layout for tests - part 1 <Created by sergiocazzolato> <Merged by sergiocazzolato> <https://github.com/snapcore/snapd/pull/10124> | 10:11 |
mup | PR snapd#10125 closed: tests: update layout for tests - part 2 <Created by sergiocazzolato> <Merged by sergiocazzolato> <https://github.com/snapcore/snapd/pull/10125> | 10:11 |
mup | PR snapd#10141 opened: tests: enable help test for all the systems <Simple 😃> <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/10141> | 10:16 |
mup | PR snapd#10142 opened: Use $PWD path to store the uc18 image using prepare image <Simple 😃> <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/10142> | 10:26 |
pedronis | https://github.com/snapcore/snapd/pull/10127 needs 2nd reviews | 10:53 |
mup | PR #10127: daemon: add new accessChecker implementations <Created by jhenstridge> <https://github.com/snapcore/snapd/pull/10127> | 10:53 |
mborzecki | let me take a quick look | 11:00 |
mborzecki | btw. we may need to bump the workers count of the nested suite, the tests on 20.04 are taking ~100minutes | 11:01 |
=== ijohnson[m][m] is now known as ijohnson | ||
ijohnson | good morning folks | 12:15 |
ijohnson | pedronis: I responded on https://github.com/snapcore/snapd/pull/10133 | 12:22 |
mup | PR #10133: wrappers/services.go: introduce EnsureSnapServices() <UC20> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/10133> | 12:23 |
pedronis | ijohnson: it doesn't really change my main point which is that RequireMountedSnapdSnap is a global property | 12:25 |
pedronis | ijohnson: as I tried to say in my initial comment we should probably change some other signatures too and then things will be less confusing | 12:26 |
ijohnson | pedronis: mmm so do you think I should do the || of the two options or what do you propose I should do about the fact we have the same option existing in multiple places | 12:27 |
pedronis | ijohnson: but is also not worth blocking this on this much longer | 12:27 |
pedronis | ijohnson: there are various solutions to this problem | 12:27 |
ijohnson | right | 12:28 |
ijohnson | so what should I do with this pr for now then | 12:28 |
pedronis | ijohnson: I think the biggest issues is really the reuse of AddSnapServicesOptions in the map | 12:29 |
pedronis | that's where the confusion starts I think | 12:29 |
ijohnson | pedronis Should the map just have VitaliyRank on its own as the value in the map instead? | 12:30 |
pedronis | probably something like SnapServiceOptions | 12:30 |
pedronis | that is only VitalyRank for now | 12:30 |
pedronis | then generateSnapFile takes both the snap specific and the global one | 12:30 |
ijohnson | pedronis ok | 12:32 |
ijohnson | Will push after SU then | 12:32 |
pedronis | ijohnson: I commented in the PR as well: https://github.com/snapcore/snapd/pull/10133#discussion_r612404544 | 12:35 |
mup | PR #10133: wrappers/services.go: introduce EnsureSnapServices() <UC20> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/10133> | 12:35 |
ijohnson | Thanks for that, makes sense | 12:36 |
pedronis | ijohnson: also as I remarked it seems a couple of my previous commentes were acked but not applied unless I missed something | 12:38 |
ijohnson | @pedronis which other comments? | 12:41 |
pedronis | ijohnson: https://github.com/snapcore/snapd/pull/10133#discussion_r611839404 and https://github.com/snapcore/snapd/pull/10133#discussion_r611835186 | 12:43 |
mup | PR #10133: wrappers/services.go: introduce EnsureSnapServices() <UC20> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/10133> | 12:43 |
ijohnson | pedronis: I addressed both of those things, ptal again | 12:48 |
pedronis | ijohnson: I missed the commit about the options had those bits in it | 12:52 |
pedronis | I suppose my brain expected them to be separate commits | 12:52 |
mborzecki | mvo: can you land https://github.com/snapcore/snapd/pull/10130, the failures seem unrelated really | 13:00 |
mup | PR #10130: boot: load bits of kernel command line from gadget snaps <Run nested> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/10130> | 13:00 |
mborzecki | and it would unblock the other PRs | 13:00 |
mup | PR snapd#10130 closed: boot: load bits of kernel command line from gadget snaps <Run nested> <Created by bboozzoo> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/10130> | 13:46 |
mup | PR snapcraft#3502 opened: snaps: do not validate snaps before install (Fixes LP#1901733) <Created by Saviq> <https://github.com/snapcore/snapcraft/pull/3502> | 14:20 |
mup | PR snapd#10143 opened: boot: handle updating of components that contribute to kernel command line <Skip spread> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/10143> | 14:42 |
pedronis | mborzecki: what's the relation of https://github.com/snapcore/snapd/pull/10140 and https://github.com/snapcore/snapd/pull/10143, I am a bit surprised there's a change in boot after a change in devicestate | 15:13 |
mup | PR #10140: boot, overlord/devicestate: consider gadget command lines when updating boot config <Run nested> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/10140> | 15:13 |
mup | PR #10143: boot: handle updating of components that contribute to kernel command line <Skip spread> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/10143> | 15:13 |
mborzecki | pedronis: the other is a separate call for the gadget update path | 15:16 |
mup | PR snapd#10134 closed: boot: set extra command line arguments when preparing run mode <Run nested> <Created by bboozzoo> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/10134> | 15:17 |
mborzecki | pedronis: the main difference is that the boot config update picks up the candidate command line, while the other pr uses the current one, both end up in the same place to handle this though | 15:18 |
mborzecki | (and there's a change in bootenv in the other call too) | 15:18 |
mup | PR snapd#10141 closed: tests: enable help test for all the systems <Simple 😃> <Created by sergiocazzolato> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/10141> | 15:27 |
pedronis | mborzecki: how many more PR do we need? one wrapping the new function in a task? | 15:31 |
pedronis | spread tests? | 15:32 |
mup | PR snapd#10138 closed: tests: fix gadget-kernel-refs-update-pc test on arm and when $TRUST_TEST_KEY is false <Simple 😃> <Created by sergiocazzolato> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/10138> | 15:32 |
mup | PR snapd#10142 closed: tests: fix prepare-image-grub-core18 for arm devices <Simple 😃> <Created by sergiocazzolato> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/10142> | 15:32 |
mborzecki | pedronis: yeah, and some spread test updates (tweak the existing boot config update test and extend the customized kernel command line one) | 15:34 |
mborzecki | pedronis: let's chat tomorrow morning maybe and we can discuss what's left? | 15:35 |
pedronis | ok | 15:35 |
pedronis | mborzecki: I reviewed on 40 and commented on the name in 43 | 15:40 |
mborzecki | pedronis: thanks | 15:41 |
mup | PR snapd#10144 opened: tests: use op.paths tools instead of dirs.sh helper - part 1 <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/10144> | 15:57 |
mup | PR snapd#10145 opened: snapstate: add "kernel-assets" to featureSet (2.50) <â›” Blocked> <Created by mvo5> <https://github.com/snapcore/snapd/pull/10145> | 15:57 |
ijohnson | pedronis_: sorry took a bit longer than I planned, but I added more tests around how the options work and refactored to match what we discussed | 17:05 |
ijohnson | #10133 is ready for a re-review now | 17:05 |
mup | Bug #10133: sqlrelay: new changes from Debian require merging <Ubuntu:Fix Released by cjwatson> <https://launchpad.net/bugs/10133> | 17:05 |
mup | PR #10133: wrappers/services.go: introduce EnsureSnapServices() <UC20> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/10133> | 17:05 |
mup | PR snapd#10146 opened: tests: add new command to snaps-state to get current core, kernel and gadget <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/10146> | 20:28 |
=== not_phunyguy is now known as phunyguy | ||
mup | PR snapd#10147 opened: browser-support: Allow firefox to run correctly under wayland <Created by alexmurray> <https://github.com/snapcore/snapd/pull/10147> | 21:38 |
mup | PR snapcraft#3501 closed: store: use whoami dashboard endpoint for cli <Created by sergiusens> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/3501> | 22:41 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!