| === popey0 is now known as popey | ||
| mup | PR snapd#9586 closed: update-pot: include file locations in translation template, and extract strings from desktop files <Created by jhenstridge> <Merged by jhenstridge> <https://github.com/snapcore/snapd/pull/9586> | 04:14 |
|---|---|---|
| mup | PR snapd#9592 opened: [wip] many: implement degraded recover mode <Run nested> <UC20> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/9592> | 04:45 |
| zyga | timothy, this is expected, that part of the sandbox is based on apparmor which doesn't exist on fedora | 06:58 |
| mborzecki | morning | 07:24 |
| mborzecki | mvo: hey | 07:38 |
| mvo | good morning mborzecki | 07:38 |
| mborzecki | mvo: looks like some bits of rpm spec we share between fedora/rhel/aamzon do not expand correctly on amazon linux | 07:39 |
| mvo | mborzecki: oh no! is that hard to fix? | 07:40 |
| mborzecki | mvo: not really, it's about the session agent and the session agent socket, though i think it's safe to assume one will not be runnig user sessions on amazon linux | 07:41 |
| mborzecki | (as in a graphical sessions) | 07:41 |
| mvo | mborzecki: aha, ok | 07:42 |
| mvo | mborzecki: that sounds not too bad then | 07:42 |
| mvo | mborzecki: anything I should look at right away? | 07:42 |
| mborzecki | i mean, it's also a bit weird, i thought they were supposed to pull in stuff from rhel, it looks like they did but just once, the relevant macros made their way to rhel7, but not to amzn2 | 07:42 |
| mborzecki | mvo: hm one last look at https://github.com/snapcore/snapd/pull/9577 ? | 07:44 |
| mup | PR #9577: many: seal a fallback object to the recovery boot chain <Run nested> <UC20> <Created by cmatsuoka> <https://github.com/snapcore/snapd/pull/9577> | 07:44 |
| mvo | mborzecki: sure | 07:44 |
| mvo | mborzecki: I think there were just minor test issue but let me look (followup material) | 07:44 |
| mvo | mborzecki: fwiw, the gentoo fix/change is in | 08:00 |
| mborzecki | yay | 08:00 |
| mvo | mborzecki: it seems to take ~24h for the mechanics to propergate a CLA change | 08:00 |
| mup | PR snapd#9588 closed: dirs: add "gentoo" to altDirDistros <Created by zmedico> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/9588> | 08:00 |
| mborzecki | mvo: maybe some cron job that runs once a day | 08:01 |
| mvo | yeah | 08:03 |
| pstolowski | morning | 08:03 |
| mvo | good morning pstolowski ! | 08:03 |
| pstolowski | o/ | 08:05 |
| mborzecki | pstolowski: hey | 08:27 |
| pstolowski | heya | 08:27 |
| mborzecki | mvo: so i did some more digging, and imo we should not fix anything for amzon linux 2, but rather pusht he users to file bugs or request support with amazon, the systemd package they have is outdated with buggy macros | 08:28 |
| sil2100 | Hey guys! Is there any reason why the amd64 UC20 images are 8GB big? Was it only to workaround the ubuntu-image bug with not properly calculating the final image size? | 08:29 |
| sil2100 | (hard-coded to 8GB big) | 08:29 |
| mvo | mborzecki: +1 | 08:35 |
| mvo | sil2100: iirc yes | 08:35 |
| mvo | mborzecki: hrm, hrm, nested core18 in 9577 failed | 08:55 |
| mborzecki | mvo: heh the log is not that useful | 09:14 |
| pstolowski | mvo: interesting thoughts https://ahmet.im/blog/golang-json-decoder-pitfalls/ and related https://github.com/google/go-github/pull/317 | 09:32 |
| mup | PR google/go-github#317: Drain Response.Body to enable TCP/TLS connection reuse (4x speedup) <Created by FiloSottile> <Merged by willnorris> <https://github.com/google/go-github/pull/317> | 09:32 |
| pstolowski | maybe harmless in our cases, otoh we do use Decode on the body a lot (this is unrelated to the issue we talked about yesterday, but might be a separate problem) | 09:33 |
| pstolowski | having extra json data isn't a big deal i think, but maybe not draining the response is | 09:34 |
| mvo | pstolowski: interessting, thanks for this | 09:35 |
| mborzecki | mvo: so i rerun the test a couple of times locally and it is passing | 10:09 |
| mvo | mborzecki: ok - in a meeting right now so can't really reply but it sounds like we could re-run one more time | 10:10 |
| mborzecki | mvo: also tried to have snapd log to journal+console on core18, but for some reason it's no being picked up | 10:10 |
| mup | PR snapd#9591 closed: gadget, gadget/install: move helpers to install package, refactor unit tests <Run nested> <Simple 😃> <UC20> <Created by bboozzoo> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/9591> | 10:26 |
| mup | PR snapd#9593 opened: tests/lib/nested: enable snapd logging to console for core18 <Run nested> <Simple 😃> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/9593> | 10:36 |
| mvo | mborzecki: thanks (still in a meeting) | 10:45 |
| mup | PR snapd#9577 closed: many: seal a fallback object to the recovery boot chain <Run nested> <UC20> <Created by cmatsuoka> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/9577> | 11:47 |
| mup | PR snapd#9594 opened: findpartitions rewrite <Created by xnox> <https://github.com/snapcore/snapd/pull/9594> | 12:02 |
| pstolowski | hmmm | 12:06 |
| pstolowski | mborzecki: do you know why sc_check_rootfs_dir in snap-confine doesn't need any core18/20 logic? | 12:06 |
| mborzecki | pstolowski: afaiu the typical case is that the base snap (which bescomes rootfs exists) which is true for core/core18/core20 | 12:10 |
| mborzecki | the rest is some fallback code for ubuntu-core (?), and core16 which can be used as base, but had no stable release, so the fallback goes to core | 12:11 |
| pstolowski | mborzecki: ah, because it falls into if (access(inv->rootfs_dir, F_OK) == 0, i misread this | 12:11 |
| pstolowski | thanks | 12:11 |
| rogpeppe | pstolowski: hiya. just FYI that raspberry pi has crashed again without recovering (the machine itself is running, but the snap isn't running and /snap/snapd/current has gone again) | 12:17 |
| rogpeppe | zyga: ^ | 12:17 |
| mborzecki | pstolowski: ^^ | 12:18 |
| rogpeppe | pstolowski: if there's anything i can do to help out with logs or anything else, let me know. you still have access to the machine. | 12:18 |
| pstolowski | rogpeppe: hi. i believe the problem is understood and it's a low-level issue outside of snapd that prevents upgrade, zyga menaged to reproduce it, i think it's in the hands of foundation team now. mvo do you know a bug for this? | 12:24 |
| rogpeppe | pstolowski: thanks | 12:25 |
| pstolowski | rogpeppe: also, btw, zyga quit a few days ago (but still shows up here ;)) | 12:25 |
| rogpeppe | pstolowski: do you know of a quick way to fix the issue on my machine, at least temporarily? | 12:25 |
| rogpeppe | pstolowski: i.e. to get the service running again. | 12:25 |
| rogpeppe | pstolowski: oh, that's a shame! zyga: good luck in your future endeavours! | 12:26 |
| pstolowski | rogpeppe: you are already holding refreshes for a month or so right? | 12:28 |
| rogpeppe | pstolowski: what do you mean? | 12:28 |
| pstolowski | rogpeppe: the setting that delays automatic refreshes for a bit | 12:29 |
| rogpeppe | pstolowski: i haven't changed any settings at all | 12:29 |
| pstolowski | rogpeppe: ok | 12:29 |
| mvo | rogpeppe, pstolowski I think part of the problem is/was that the uboot in the pi-gadget is outdated and does not have a fix for sd-card writing. not sure if zyga manually applied the fixed uboot to your device though | 12:29 |
| rogpeppe | mvo: would it help to reinstall everything on a fresh sd card with the latest ubuntu core? | 12:30 |
| pstolowski | rogpeppe: yes, that is what i was going to suggest.. the problem affects some old images & old uboot | 12:31 |
| rogpeppe | pstolowski: ok, i'll do that. but it'll take a few days to send the card by post. is there a way to get the snap services running in the meantime? | 12:31 |
| mvo | rogpeppe: I would wait a tiny bit, the pi gadget is not updated yet | 12:32 |
| mvo | rogpeppe: let me quickly double check the state of this | 12:32 |
| pstolowski | rogpeppe: yes, there is a temporary workaround, i've just logged onto your pi3 to check bash history because I did it once before | 12:34 |
| rogpeppe | pstolowski: thanks! | 12:34 |
| mvo | pstolowski: did zyga apply the fix? | 12:35 |
| mvo | pstolowski: or what exactly does "temporary workaround" mean (sorry for being a bit irgnorant) | 12:35 |
| pstolowski | rogpeppe: /snap/snapd/9169/usr/bin/snap enable snapd | 12:35 |
| pstolowski | rogpeppe: this brings snap command back | 12:35 |
| pstolowski | rogpeppe: i just did it | 12:35 |
| mvo | ta | 12:36 |
| pstolowski | rogpeppe: i havern't done anything about services, you may want to check if they are fine and start if needed | 12:36 |
| pstolowski | mvo: i don't know about any uboot fixes, no idea if zyga applied anything | 12:36 |
| mvo | rogpeppe: I asked "waveform" to join us here, he maintains the pi gadget, will ask him about the status of the uboot SRU that contains the fix | 12:37 |
| mvo | pstolowski: that's fine, was just curious | 12:37 |
| mvo | pstolowski: probably risky anyway before we have the snap | 12:37 |
| pstolowski | mvo: actaully, most likely not because only I have access to rogpeppe's box | 12:37 |
| * mvo nods | 12:38 | |
| rogpeppe | pstolowski: ok, good call. the service was active but not running. now it's running again | 12:40 |
| pstolowski | rogpeppe: sorry for all the trouble. this was a tricky bug related to uboot and the only way to investigate was to have physical access & see the uboot error (remote access wasn't enough) | 12:47 |
| rogpeppe | pstolowski: that's ok. hopefully when i reinstall, this won't happen again for a while at least. | 12:48 |
| pstolowski | rogpeppe: just note what mvo said about availability of new pi gadget | 12:49 |
| rogpeppe | pstolowski: what's the "pi gadget" ? | 12:49 |
| pstolowski | rogpeppe: it's pi3 support package (a snap) that includes files for booting | 12:50 |
| rogpeppe | pstolowski: ack | 12:51 |
| mborzecki | the fallback mount handling in s-b has become quite complex now | 13:00 |
| mvo | rogpeppe: waveform may have more details about when we get a new uboot that has the sd-card writing bug fixed | 13:45 |
| * cachio lunch | 14:59 | |
| pstolowski | cachio: hey, did you see questions under https://github.com/snapcore/snapd/pull/9567 ? | 15:01 |
| mup | PR #9567: tests: using the nested-state tool in nested tests <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/9567> | 15:01 |
| pstolowski | rogpeppe: fyi, i found the bug: https://bugs.launchpad.net/snapd/+bug/1900693 | 15:41 |
| mup | Bug #1900693: snapd cannot refresh on some SD cards due to uboot bug <snapd:New> <u-boot (Ubuntu):New> <https://launchpad.net/bugs/1900693> | 15:41 |
| rogpeppe | pstolowski: thanks! interesting to see the underlying bug too. BTW, which part of the system is this about? https://github.com/u-boot/u-boot/commit/b1125802a524641ad1ac803b4a617756d26f007d | 15:44 |
| rogpeppe | ah, maybe the SD card driver? | 15:45 |
| ogra | not sure how much you guys watch the #ubuntu channel ... | 15:45 |
| ogra | <wonko> home and removable-media | 15:45 |
| ogra | <wonko> I *can't* do ls ~/.ssh but I *can* do ls ~/.ssh/* | 15:45 |
| ogra | <wonko> which seems broken | 15:45 |
| ogra | should a user be able to list content of ~/.ssh when using globbing (while listing without globbing is properly blocked by apparmor) | 15:46 |
| pstolowski | rogpeppe: i'm not sure | 15:48 |
| niemeyer | ogra: I cannot list ~/.ssh/* here | 16:07 |
| niemeyer | ogra: The behavior is not about globbing.. it just means he's got read access to ~/ but has to ~/.ssh/, which sounds wrong | 16:07 |
| ogra | niemeyer, that user is inside a multipass VM running the lsd snap .... it is a bit more than just ls on a normal hist it seems https://gist.github.com/bhechinger/e2fdb1e4e4ab636287c265806ab3dabf | 16:07 |
| ogra | *host | 16:07 |
| pstolowski | Saviq: hey, re https://bugs.launchpad.net/snapd/+bug/1902230, the issue didn't get away and it was stuck for good right? | 16:08 |
| mup | Bug #1902230: Tasks stuck if api.snapcraft.io not found <snapd:New> <https://launchpad.net/bugs/1902230> | 16:08 |
| pstolowski | *didn't go | 16:08 |
| niemeyer | ogra: removable-media is not even connected.. it's just nome | 16:09 |
| niemeyer | home | 16:09 |
| ogra | yep | 16:09 |
| niemeyer | This is a relevant bug if it was just allowing access to .ssh, but my bet is that there's something else fishy in that environment | 16:09 |
| niemeyer | mvo: The restarting state is another area that might benefit from some attention | 16:09 |
| ogra | yes, my suspicion was that multipass bind mounts content from outside the VM there or similar | 16:10 |
| niemeyer | mvo: It's currently distributed over several packages, with internals leaking thorugh | 16:10 |
| ogra | niemeyer, ijohnson is looking deeper into it | 16:10 |
| niemeyer | mvo: We had a pretty small surface in the state package itself, with all responsibility lying in the backend, but now there's quite a bit about that inside the state itself, with the settings being known inside state, in overlord, and all the way to the daemon package | 16:11 |
| * ijohnson got pulled into a meeting | 16:11 | |
| niemeyer | This kind of logic seems to make sense in the overlord, since it's about orchestration.. | 16:13 |
| ogra | ijohnson, niemeyer ... another datapoint ... this multipass instance is running on top of windows (hyper-v) ... | 16:14 |
| ogra | (do we actually expect proper confinement there at all ???) | 16:15 |
| niemeyer | ogra: snap debug confinement | 16:19 |
| zyga | rogpeppe, re | 16:21 |
| ogra | niemeyer, interestingly that seems to returns "strict" ... | 16:23 |
| niemeyer | ogra: Maybe it's just not working properly.. is that a Canonical kernel? | 16:23 |
| ogra | niemeyer, looks like "kernel 5.4.0-52-generic" ... (from https://gist.github.com/bhechinger/e2fdb1e4e4ab636287c265806ab3dabf) | 16:25 |
| ogra | at least if snap version gets the correct info | 16:25 |
| niemeyer | It's clear snapd thinks the environment is strict given the data it has.. the question is whether this is a real kernel/apparmor bundle | 16:26 |
| ogra | yeah ... thats probably a question to Saviq | 16:26 |
| niemeyer | Also, is the snap in devmode? | 16:27 |
| ogra | (i'm not using multipass .... passionate lxd user here) | 16:27 |
| niemeyer | Ah, and that's the *lxd* snap? | 16:27 |
| ogra | no, "lsd" | 16:28 |
| niemeyer | LXD has plenty of access into the system to be able to do all it does | 16:28 |
| cachio | pstolowski, checking | 16:28 |
| pstolowski | cachio: thanks | 16:28 |
| ogra | niemeyer, he is using the "lsd" snap (ls on steroids) inside a multipass VM on a windows machine | 16:28 |
| niemeyer | Ah, ok | 16:29 |
| ogra | no lxd involved (that was just about me not using multipass, sorry for confusing) | 16:29 |
| pstolowski | cachio: i also reviewed your other PR re boot-state, but please take a look at the small suggestion | 16:29 |
| niemeyer | ogra: Can you reproduce the issue on a vm? | 16:29 |
| cachio | pstolowski, nice, I'll take a look to all of them | 16:29 |
| cachio | thanks for the reviews | 16:29 |
| ogra | i can try ... but that takes a bit ... | 16:30 |
| ijohnson | ogra: I can reproduce the issue on my native focal machine | 16:35 |
| * ijohnson is back from meetings now | 16:35 | |
| ijohnson | my guess is that this snap is doing some stat syscall trickery | 16:36 |
| ogra | ijohnson, gah ... now my multipass has finally downloaded that image ! | 16:36 |
| ogra | okay, so seems we have an actual bug | 16:36 |
| ijohnson | ogra: and indeed if I do `snap run --shell lsd` and do a normal `ls ~/.ssh/*` that does not work | 16:36 |
| ogra | thats so strange | 16:37 |
| ogra | ubuntu@primary:~$ lsd ~/.ssh/ | 16:39 |
| ogra | cannot access '/home/ubuntu/.ssh/': Permission denied (os error 13) | 16:39 |
| ogra | ubuntu@primary:~$ lsd ~/.ssh/* | 16:39 |
| ogra |  authorized_keys | 16:39 |
| ogra | yep, same here .... i can repro ... | 16:39 |
| ogra | and i can even reproduce it on my native machine | 16:40 |
| ogra | ogra@acheron:~$ lsd ~/.ssh/ | 16:40 |
| ogra | cannot access '/home/ogra/.ssh/': Permission denied (os error 13) | 16:40 |
| ogra | ogra@acheron:~$ lsd ~/.ssh/* | 16:40 |
| ogra |  config  id_rsa  id_rsa.keystore  id_rsa.pub  known_hosts  known_hosts.old | 16:40 |
| ogra | so yeah, there is a real bug | 16:42 |
| ijohnson | I think it's essentially using stat to get the inode, then incrementing the inode to get what the file name is so it is an information leak, but it's probably not a CVE | 16:42 |
| ogra | yeah | 16:43 |
| Nemesis | Hello everyone, I have fresh Manjaro install and most of my snap installed applications won't open. can someon assist me to fix that issue? | 16:48 |
| ogra | Nemesis, are you using wayland ? | 16:48 |
| ijohnson | ogra: ah I know what it is | 16:48 |
| ijohnson | ogra: try `lsd '~/.config/*'` | 16:49 |
| ijohnson | ogra: or `lsd '~/.ssh/*'` as compared to `lsd ~/.ssh/*` | 16:49 |
| Nemesis | ogra: i am not that good in linux, and this is my first try arch based system | 16:51 |
| Nemesis | usually I mostly use debian based | 16:51 |
| Nemesis | and also I am not familliar with wayland | 16:51 |
| ogra | ijohnson, so a quoting issue you think ? | 16:52 |
| ijohnson | ogra: it's bash giving away all your secrets! | 16:52 |
| ogra | hah ! | 16:52 |
| ijohnson | ogra: when I strace that command, I see this: | 16:52 |
| ijohnson | [pid 44201] 1604422062.123092 execve("/snap/lsd/62/command-lsd.wrapper", ["/snap/lsd/62/command-lsd.wrapper", "/home/ijohnson/.ssh/authorized_keys", "/home/ijohnson/.ssh/config", "/home/ijohnson/.ssh/id_rsa", "/home/ijohnson/.ssh/id_rsa-lxd", "/home/ijohnson/.ssh/id_rsa-lxd.pub", "/home/ijohnson/.ssh/id_rsa.pub", "/home/ijohnson/.ssh/known_hosts", "/home/ijohnson/.ssh/known_hosts.old"], 0xc420178300 /* 95 vars */ | 16:52 |
| ijohnson | <unfinished ...> | 16:52 |
| ijohnson | so bash expands the "*" to all the files there and then those are all passed as args to `snap run`, and lsd then proceeds to work, because it uses stat on those files | 16:53 |
| Nemesis | I have installed Manjaro KDE | 16:53 |
| ijohnson | and stat syscall always works | 16:53 |
| ogra | Nemesis, https://forum.snapcraft.io/t/completely-broken-snapd-environment-in-manjaro-20-1-2/20900 ... | 16:53 |
| ogra | ijohnson, hah lovely | 16:54 |
| Nemesis | i can see that, so that's mean snap is broken and i cannot use it and respectively fix it? | 16:55 |
| ogra | though i think thats not bash specific ... POSI shell would do the same | 16:55 |
| ogra | Nemesis, probably use an older manjaro image (but perhaps also tell the guy in the forum that he is not alone (which he is asking)) | 16:56 |
| ijohnson | ogra: right I think all shells do this | 16:56 |
| ogra | yeah | 16:56 |
| Nemesis | ok, I understand | 16:56 |
| cachio | pstolowski, I answered questions | 17:01 |
| cachio | I hope it is more clear nowe | 17:01 |
| cachio | now | 17:01 |
| pstolowski | checking | 17:03 |
| ijohnson | mvo: so I spoke with xnox about the disk PR, that is now ready to go, there are improvements we want/should do sometime but the pr as-is fixes the issue about parent <-> child device mapping so we should merge that with normal snapd reviews | 17:03 |
| ijohnson | there are lp bugs filed for the improvements I assigned to me that I think we should do sooner rather than later, but aren't necessary for 1.0 I think | 17:03 |
| mvo | ijohnson: cool, I'm happy to merge your PR, it got my +1 already | 17:13 |
| pstolowski | cachio: +1 | 17:13 |
| cachio | pstolowski, thanks | 17:14 |
| ijohnson | mvo: ok, I did slightly adjust the logic so I dismissed your review so if you want to just quickly give it another review then maybe maciej can review it tomorrow morning | 17:14 |
| pstolowski | cachio: btw, did you see https://github.com/snapcore/snapd/pull/9533#issuecomment-719501270 ? | 17:14 |
| mup | PR #9533: tests: new systemd-state tool <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/9533> | 17:14 |
| cachio | pstolowski, yes, I am trying to fix that for trusty, if I cant so I'll skip it in the test | 17:15 |
| pstolowski | ok | 17:16 |
| cachio | pstolowski, I'll let you know when I test it | 17:17 |
| cachio | thanks for the heads up | 17:17 |
| pstolowski | cachio: sure. is there any other PR you need reviewed? | 17:17 |
| cachio | pstolowski, you already reviewed the 3 most important ones | 17:18 |
| cachio | so thanks for that | 17:18 |
| cachio | pstolowski, well I have #9276 | 17:19 |
| mup | PR #9276: tests: new backend for desktop and external classic <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/9276> | 17:19 |
| cachio | it is a pretty old one | 17:20 |
| cachio | but not so important | 17:20 |
| zyga-mbp | hey cachio :) | 17:21 |
| cachio | zyga-mbp, hey zyga | 17:21 |
| cachio | how is it going? | 17:21 |
| zyga-mbp | good, rapid pace for sure | 17:21 |
| pstolowski | cachio: right, i saw that.. could be nice though. i may have a look tomorrow. could you please merge master in it just to have it up-to-date? | 17:21 |
| cachio | pstolowski, sure, thanks | 17:22 |
| pstolowski | ok. cu | 17:22 |
| zyga-mbp | cachio any reviews I can help you with? | 17:22 |
| cachio | zyga-mbp, I think the only one missing is #9533 | 17:24 |
| mup | PR #9533: tests: new systemd-state tool <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/9533> | 17:24 |
| cachio | zyga-mbp, the rest is already covered | 17:24 |
| ijohnson | hey zyga-mbp how are things going? | 17:24 |
| zyga-mbp | ijohnson hey! | 17:24 |
| cachio | zyga-mbp, still need to make f33 work | 17:24 |
| zyga-mbp | ijohnson different :) | 17:24 |
| zyga-mbp | ijohnson but I EOD at 16:30 which is just amazing | 17:24 |
| cachio | zyga-mbp, #9556 | 17:24 |
| mup | PR #9556: tests: testing new fedora 33 image <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/9556> | 17:24 |
| ijohnson | zyga-mbp: nice :-) having an early EOD is nice | 17:25 |
| zyga-mbp | the 31->33 patch looks good | 17:25 |
| zyga-mbp | ijohnson: lots of corpo stuff to figure out | 17:25 |
| zyga-mbp | but really easy to communicate as well, that's surprising to me | 17:25 |
| ijohnson | yeah always happens at new jobs | 17:25 |
| ijohnson | anyways zyga-mbp it was good to "see" you here again, I need to get back into some code :-) | 17:29 |
| zyga-mbp | thank you :-) | 17:33 |
| mvo | ijohnson: is the comment in 9541 still in sync with the code? just asking before I do a proper review | 17:35 |
| * mvo will have dinner first before doing a deeper dive | 17:35 | |
| ijohnson | mvo: which comment? the ones in the code yes, and I responded to all thecomments in the PR so everything should be in sync | 17:35 |
| ijohnson | the comment in LP bug is not editable so you need to look at the other comments further down in the bug report if you are referring to that | 17:36 |
| mvo | ijohnson: these comments https://github.com/snapcore/snapd/pull/9541/files#diff-db994f6207a7cf0f7df752561b677c6816387fe42bec335ede11f6a46d411169R341 I mean but great if they are :) | 17:36 |
| mup | PR #9541: osutil/disks: re-implement partition searching for disk w/ non-adjacent parts <Bug> <Run nested> <UC20> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/9541> | 17:36 |
| ijohnson | mvo: yes those comments are about future improvements, but not needed for the pr | 17:37 |
| ijohnson | mvo: I will look into sorting those out in a followup PR, probably not for 1.0 | 17:37 |
| ijohnson | mvo: I will leave responses to those to make it clear though | 17:37 |
| ijohnson | mvo: ok responded hopefully that makes it more clear | 17:38 |
| zyga-mbp | cachio https://github.com/snapcore/snapd/pull/9533#pullrequestreview-522723935 | 17:44 |
| mup | PR #9533: tests: new systemd-state tool <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/9533> | 17:44 |
| zyga-mbp | cachio I'm somewhat -1 on this, left ideas and some comments but mainly look at the first comment | 17:44 |
| cachio | zyga-mbp, thanks, well the idea of this was to move the helper we already have to a tool | 17:47 |
| cachio | which we can test | 17:47 |
| zyga-mbp | yeah but not everything should be a helper like that | 17:47 |
| zyga-mbp | it's a very shallow helper on top of systemd commands | 17:48 |
| zyga-mbp | with custom syntax and behavior | 17:48 |
| zyga-mbp | it will not replace our use of systemd tools directly | 17:48 |
| zyga-mbp | I would not go there | 17:48 |
| zyga-mbp | as I said in the review, I would keep two commands | 17:48 |
| zyga-mbp | but slim them to work well with repeat | 17:48 |
| zyga-mbp | and really not implement the rest as a utility but instead put it in the few spots that need it | 17:48 |
| zyga-mbp | and each time, consider if you can systemd-run something instead | 17:49 |
| cachio | and the rest should stay in systemd-sh? | 17:49 |
| cachio | systemd.sh? | 17:49 |
| zyga-mbp | no, just move it to the places that use those | 17:49 |
| zyga-mbp | how many tests call those functions? | 17:49 |
| cachio | zyga-mbp, 17 | 17:50 |
| cachio | which are creating systemd units | 17:50 |
| zyga-mbp | I've opened a random test that imports systemd.sh | 17:51 |
| zyga-mbp | and it wasn't using it | 17:51 |
| zyga-mbp | I'd suggest going through the tests and filtering out those first | 17:51 |
| zyga-mbp | it may be a bad sample (I looked at tests/core/remodel-base) | 17:52 |
| cachio | zyga-mbp, I listed the tests which are using the function systemd_create_and_start | 17:52 |
| cachio | there are 17 creating systemd units through this function | 17:52 |
| zyga-mbp | but that doesn't change what my comment said: those helpers are not doing complex things, they are wrapping systemd tools with new syntax one has to learn, I'm not sure that's worth doing | 17:52 |
| zyga-mbp | I see | 17:52 |
| * zyga-mbp looks | 17:53 | |
| zyga-mbp | I see 12 task.yaml files | 17:53 |
| zyga-mbp | I've opened tests/core/snap-set-core-config | 17:54 |
| zyga-mbp | and instead of what that is doing, it's a really one liner: systemd-run --unit rsyslog.service sleep 2h | 17:54 |
| zyga-mbp | and similarly systemctl stop to remove it | 17:54 |
| zyga-mbp | this has some nice advantage, it goes through the dbus api | 17:55 |
| zyga-mbp | and doesn't require reloading systemd | 17:55 |
| zyga-mbp | (reloading is only needed when you change files from under systemd) | 17:55 |
| zyga-mbp | anyway, I cannot -1 it for real anymore but think over my advice | 17:55 |
| zyga-mbp | IMO using systemd-run instead will make most of that obsolete | 17:55 |
| zyga-mbp | and also more readable, as it's a well known command and not our custom helper | 17:56 |
| zyga-mbp | you can also couple that with tests.cleanup defer systemctl stop to make sure it is not leaking in any way, and put that right next to each other | 17:56 |
| cachio | zyga-mbp, systemd-run supports a reboot? | 17:56 |
| zyga-mbp | cachio no, but most tests don't need that | 17:57 |
| zyga-mbp | systemd-run creates units in memory | 17:57 |
| zyga-mbp | those that do can just make the units, it's not like two printf calls are somehow special | 17:57 |
| zyga-mbp | and you don't need to learn new tool and how to use it exactly | 17:57 |
| cachio | zyga-mbp, using systemd-run makes sense to me | 17:58 |
| cachio | I would need up update the tests for that | 17:58 |
| zyga-mbp | yeah, just change a few and see how it feels like | 17:58 |
| cachio | zyga-mbp, I'll try that, just need to make sure it works well in all the supported systemd | 17:59 |
| cachio | version | 17:59 |
| zyga-mbp | cachio systemd-run works on 16.04+ | 17:59 |
| zyga-mbp | 14.04 is not supported since our broken systemd version there is detached from dbus | 17:59 |
| zyga-mbp | I would not sacrifice readability for support over 14.04 | 18:00 |
| cachio | zyga-mbp, agree | 18:00 |
| cachio | zyga-mbp, thanks for the advise | 18:01 |
| zyga-mbp | good luck! | 18:01 |
| === ijohnson is now known as ijohnson|lunch | ||
| === ijohnson|lunch is now known as ijohnson | ||
| mvo | ijohnson: 9541 looks fine, one question/suggestion but needs a second review still :/ | 19:51 |
| ijohnson | mvo: yeah I was thinking maybe maciej could take a look in the AM | 19:52 |
| ijohnson | thanks for the review, I will go look at your question now | 19:52 |
| ijohnson | also sorry I did respond to those comments, but they didn't show up because they were part of a review apparently, but I just switched them to normal comments now | 19:52 |
| mvo | ijohnson: yeah, I will coordinate with him in the morning | 19:52 |
| ijohnson | ok | 19:52 |
| mvo | ijohnson: no worries | 19:53 |
| ijohnson | mvo: also I discussed with maciej after SU how to refactor 9592, I think we have a cleaner way to implement it now, and a way that should make testing easier as well, so I will try to get that updated today | 19:55 |
| mvo | ijohnson: nice! | 19:55 |
| mvo | ijohnson: thanks for this update | 19:55 |
| ijohnson | mvo: also also, we talked about transitioning from run -> recover automatically in the case where we couldn't unlock data partition and all fallbacks fail, I think it's reasonable thing to do and slightly helps us avoid the problem of being unable to trigger the chooser when the recovery key prompt swallows input | 19:56 |
| ijohnson | mvo: this specific change shouldn't be too complex, but before we pull that in I think pedronis needs to have a look so it will have to wait until Thursday anyways | 19:57 |
| mvo | ijohnson: fallabck run->recover> agreed | 19:58 |
| ijohnson | ok, I will maybe open that today, maybe tomorrow | 20:00 |
| mup | PR snapcraft#3347 opened: Create an extra 'build' directory for plugins <Created by artivis> <https://github.com/snapcore/snapcraft/pull/3347> | 21:17 |
| mup | PR snapcraft#3348 opened: ROS plugins use plugins' build directory <Created by artivis> <https://github.com/snapcore/snapcraft/pull/3348> | 21:27 |
| mup | PR snapd#9595 opened: interfaces/greengrass-support: minimize to enable 1.11 no-container lambdas <Needs security review> <â›” Blocked> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/9595> | 21:39 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!