| mup | PR snapd#7411 opened: cmd/model: output tweaks, add'l tests <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/7411> | 01:36 |
|---|---|---|
| === epod is now known as luk3yx | ||
| zyga | good morning | 04:56 |
| zyga | wow, I'm online _before_ maciek for once :) | 04:56 |
| mborzecki | morning | 05:35 |
| zyga | Hey Maciek :-) | 05:43 |
| zyga | Start with reviews please | 05:43 |
| zyga | There is a fix for master from Sergio | 05:43 |
| zyga | Good morning mvo | 05:45 |
| mup | PR snapd#7410 closed: tests: support fastly-global.cdn.snapcraft.io url on proxy-no-core test <Simple 😃> <Created by sergiocazzolato> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/7410> | 05:51 |
| zyga | thank you mborzecki | 06:01 |
| mborzecki | zyga: hey ;) | 06:01 |
| mborzecki | mvo: morning to you as well | 06:02 |
| zyga | - google:ubuntu-18.04-64:tests/main/desktop-portal-filechooser | 06:02 |
| zyga | - google:ubuntu-18.04-64:tests/main/proxy-no-core | 06:02 |
| zyga | two failures of the day :/ | 06:02 |
| mborzecki | duh | 06:02 |
| mvo | good morning mborzecki and zyga | 06:02 |
| zyga | restarted already _today_ | 06:02 |
| zyga | oh well | 06:02 |
| zyga | I'll rebase on the fix and try again | 06:02 |
| zyga | mborzecki: I extracted https://github.com/snapcore/snapd/pull/7400 from the MS_SHARED bugfix branch | 06:03 |
| mup | PR #7400: cmd/snap-update-ns: don't propagate detaching changes <Bug> <Created by zyga> <https://github.com/snapcore/snapd/pull/7400> | 06:03 |
| zyga | mborzecki: it's fairly isolated, except for apparmor changes (yuck) | 06:03 |
| zyga | mborzecki: I'll add a ns test for writable mimic now | 06:03 |
| zyga | last night while working on this I wrote DiffEquals checker | 06:06 |
| zyga | it's like Equals but it requires text or error and it produces actual diffs | 06:07 |
| zyga | I think it's fantastic and would be 99% of the benefit of whatever else we were using at the total line cost of about a dozen | 06:07 |
| mborzecki | zyga: hmm i think the tests in snap-test.c using g_test_subprocess() are incorrect | 06:18 |
| mborzecki | zyga: looking at this code right now, and we shouldn't be calling g_test_fail() in the subprocess when we expect the actual code-under-test to die :/ | 06:18 |
| mborzecki | zyga: fortunately the actual code is correct :P so it dies as expected | 06:19 |
| zyga | mborzecki: oh, is that because we set the non-fatal failures? | 06:19 |
| mborzecki | zyga: loo at the diff, https://paste.ubuntu.com/p/zKhcQ8gWgK/ we shouldn't be calling g_test_Fail() as it would mask the error case when the sc_draop_instance_key doesn't die() when we expect it to | 06:21 |
| zyga | mmmm | 06:21 |
| zyga | mborzecki: perhaps time to move away from die to sc_error | 06:21 |
| zyga | it's so fragile to test that | 06:21 |
| zyga | and error is so-much-nicer | 06:21 |
| zyga | mborzecki: speaking of which | 06:27 |
| zyga | I have a patch for mountinfo to use that | 06:27 |
| zyga | I'll send it today | 06:27 |
| zyga | it's been too long, it's half a year old by now | 06:27 |
| zyga | mvo: I'll start slow, I stopped working past midnight | 06:28 |
| zyga | mvo: see you at around 9:30 ish | 06:28 |
| mvo | zyga: ok | 06:31 |
| === pstolowski|afk is now known as pstolowski | ||
| pstolowski | morning | 06:58 |
| mup | PR snapd#7412 opened: tests: run dbus-launch inside a systemd unit <Test Robustness> <Created by mvo5> <https://github.com/snapcore/snapd/pull/7412> | 07:17 |
| mborzecki | zyga: added some tests for sc_invocation, though will open a PR once #7406 lands | 07:53 |
| mup | PR #7406: cmd/snap-confine: keep track of snap instance name and the snap name <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/7406> | 07:53 |
| mborzecki | pstolowski: hey | 07:54 |
| pstolowski | mborzecki: hi | 07:54 |
| mborzecki | pstolowski: can you take another look at https://github.com/snapcore/snapd/pull/7387? | 07:55 |
| mup | PR #7387: packaging/fedora, tests/lib/prepare-restore: helper tool for packing sources for RPM <Simple 😃> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/7387> | 07:55 |
| pstolowski | mborzecki: yep, just need to finish another review | 07:55 |
| mborzecki | pstolowski: sure, thanks! | 07:55 |
| pstolowski | pedronis: hey, +1 on #7381 with a few minor remarks and two typo fixes | 08:04 |
| mup | PR #7381: seed,image,o/devicestate: extract seed loading to seed/seed16.go <Created by pedronis> <https://github.com/snapcore/snapd/pull/7381> | 08:04 |
| pstolowski | mborzecki: would it make sense to move pack-source one level up (or somewhere else) since it's not fedora only? | 08:05 |
| mborzecki | pstolowski: hm wasn't able to find a good place for it, doesn't really fit under reelase-tools | 08:06 |
| mborzecki | pstolowski: maybe under packaging? $top/packaging/pack-source? | 08:06 |
| pedronis | pstolowski: thank you | 08:07 |
| pstolowski | mborzecki: yes, directly under packaging/ sounds good | 08:12 |
| mup | PR snapd#7413 opened: hookstate/ctlcmd: fix snapctl set help message <Simple 😃> <Created by stolowski> <https://github.com/snapcore/snapd/pull/7413> | 08:40 |
| pstolowski | ^ a trivial one line change of help string | 08:40 |
| zyga | mborzecki: ack | 08:49 |
| zyga | sorry, back now, | 08:49 |
| zyga | I was sleeping | 08:49 |
| zyga | feeling better now | 08:49 |
| pedronis | pstolowski: I made some comments again in #7277 | 08:49 |
| mup | PR #7277: overlord/snapstate: fix undo on firstboot seeding <Created by stolowski> <https://github.com/snapcore/snapd/pull/7277> | 08:49 |
| zyga | mvo: I'm commenting on https://github.com/snapcore/snapd/pull/7412 | 08:53 |
| zyga | oh wow | 08:53 |
| mup | PR #7412: tests: run dbus-launch inside a systemd unit <Test Robustness> <Created by mvo5> <https://github.com/snapcore/snapd/pull/7412> | 08:53 |
| zyga | that's quick :D | 08:53 |
| zyga | mvo: retry-tool is new, usability ideas welcome | 08:53 |
| mup | PR snapd#7404 closed: cmd/snap: don't append / to snap name just because a dir exists <Simple 😃> <Created by chipaca> <Merged by zyga> <https://github.com/snapcore/snapd/pull/7404> | 08:54 |
| jamesh | If it wasn't for xenial and centos 7, I'd suggest just spinning up a systemd user instance together with its bus | 08:56 |
| mvo | zyga: I was tweaking things there anyway :) | 08:57 |
| mvo | jamesh: yeah, its a bit of pita on these older distros | 08:57 |
| pstolowski | pedronis: ty | 08:58 |
| mvo | jamesh: i noticed serveral seconds of delay when doing "test-snapd-eds.contacs list" etc, this is normal, right? | 08:58 |
| mvo | jamesh: I checked with both my version and the previous one and it seems to be no regression | 08:58 |
| jamesh | mvo: is that every time or just the first time? | 08:58 |
| mvo | jamesh: just the first time | 08:59 |
| jamesh | so it might just be "evolution-data-server is slow to start"? | 08:59 |
| mvo | jamesh: could be, its seconds and strace shows some sort of poll timing out | 09:00 |
| mvo | jamesh: but its fine, I was just wondering | 09:00 |
| pstolowski | zyga: looks like the problem is more common, fresh bug report: https://bugs.launchpad.net/snapd/+bug/1842521 | 09:01 |
| mup | Bug #1842521: Deleted snaps showing up in gnome-disks and nautilus sidebar <snapd:New> <gnome-disk-utility (Ubuntu):New> <https://launchpad.net/bugs/1842521> | 09:01 |
| zyga | pstolowski: that's interesting | 09:02 |
| zyga | it's an app snap that leaks then | 09:02 |
| jamesh | mvo: I don't know specifically, but EDS is a bit over complicated | 09:02 |
| mvo | jamesh: yeah, its fine | 09:02 |
| jamesh | It's the kind of thing that could probably be a lot smaller if redesigned today, but there is no one to do it | 09:03 |
| pstolowski | Chipaca: hey, can you take a look at https://bugs.launchpad.net/snapd/+bug/1838788? you're probably the best person to assess the validity and if it can be fixed | 09:13 |
| mup | Bug #1838788: Pagination information not returned in /v2/find <snapd:New> <https://launchpad.net/bugs/1838788> | 09:13 |
| pstolowski | (or if should be) | 09:13 |
| pstolowski | zyga: app leaks? you mean app is stil running and holding the mountpoint? | 09:14 |
| zyga | pstolowski: it's not a mount point | 09:15 |
| zyga | it's a loop device | 09:15 |
| zyga | it doesn't even have to be mounted | 09:15 |
| zyga | just attached to a file (now deleted) on disk | 09:15 |
| zyga | it's a different level of leak IMO | 09:15 |
| pstolowski | zyga: i see | 09:15 |
| zyga | I'll spend some time debugging that later, I'm adding tests for writable mimic now | 09:15 |
| zyga | it's clear that it _is_ leaking | 09:15 |
| pstolowski | ack | 09:16 |
| pstolowski | slightly annoying it pollutes file manager like this | 09:16 |
| zyga | no disagreement there | 09:19 |
| pstolowski | uhm getting repetitive travis failures on desktop-portal-filechooser (i know this one has a PR from sergio), and two other tests failed to prepare on core16 and core18, it seems like snapd was not listening when it should be there already during prepare | 09:31 |
| zyga | pstolowski: remember that bug where snapd starts before cores are mounted? | 09:31 |
| zyga | maybe that happened? | 09:31 |
| Chipaca | that happens every time in the core18 spread test (but it recovers afaict) | 09:32 |
| Chipaca | if you run it manually you'll see snapd coming and failing to find core18's snap.yaml | 09:33 |
| zyga | what's not great | 09:33 |
| zyga | *that's* | 09:33 |
| zyga | I need coffee, sorry guys, I'm very sleepy still | 09:33 |
| pstolowski | Chipaca: thanks for updating the bug | 09:34 |
| mborzecki | pstolowski: about that LP bug, that's what i mentioned seeing last time we discsused the problem | 09:35 |
| Chipaca | pstolowski: I don't know how you manage to do triage erryday | 09:36 |
| Chipaca | pstolowski: I'd be jumping out windows at this point | 09:37 |
| zyga | Chipaca: if you were any closer I'd grab you for a bike ride together :) | 09:37 |
| pstolowski | Chipaca: haha. not every day though.. and today i only looked at maybe 3 bugs | 09:37 |
| zyga | Chipaca: a bit of a shame we live so far away | 09:37 |
| zyga | Chipaca: I think the secret is that pawel's window is on the ground floor so he comes back rattled a little and carries on :D | 09:37 |
| Chipaca | zyga: opening it before starting triage probably helps | 09:38 |
| Chipaca | pstolowski: ssh, let me worship you without knowing the nitty gritty | 09:38 |
| pstolowski | Chipaca: however.. it seems to me that just spending 15-30 minutes on it every day we can keep up the pace with any new bugs, so... | 09:38 |
| zyga | *that's the sprit* | 09:39 |
| pstolowski | :) | 09:39 |
| pstolowski | Chipaca: one more related for you (and then i'll shut up): https://bugs.launchpad.net/snapd/+bug/1838786 | 09:45 |
| mup | Bug #1838786: Categories not returned in /v2/find results <snapd:New> <https://launchpad.net/bugs/1838786> | 09:45 |
| pstolowski | sorry, two ;) https://bugs.launchpad.net/snapd/+bug/1838787 | 09:45 |
| mup | Bug #1838787: Website not returned in /v2/find results <snapd:New> <https://launchpad.net/bugs/1838787> | 09:45 |
| Chipaca | i have a pr for the first one, some where | 09:47 |
| pstolowski | Chipaca: didn't you also work on https://bugs.launchpad.net/snapd/+bug/1832767 at some point? or was it only some fixes for restricted characters? | 09:51 |
| mup | Bug #1832767: snap info doesn't strip Markdown <snapd:New> <https://launchpad.net/bugs/1832767> | 09:52 |
| Chipaca | pstolowski: i fixed it from the other end | 09:52 |
| Chipaca | pstolowski: i'll update the bug | 09:53 |
| pstolowski | thanks! | 09:53 |
| pstolowski | mborzecki: i think we just need better help message to steer users | 09:55 |
| mborzecki | pstolowski: in snap connections? | 09:55 |
| pstolowski | mborzecki: i didn't remember the details of our discussions from one of the previous sprint. i had literally to play around with it to re-discover this ;) | 09:56 |
| pstolowski | mborzecki: yes | 09:56 |
| Chipaca | pstolowski: by "from the other end" i mean, the issue i addressed was _ugly_ markdown coming from the store, which we addressed by … not sending ugly markdown :-) | 09:56 |
| Chipaca | it can still be fairly ugly, but not full on eugh ugly | 09:56 |
| pstolowski | mborzecki: so i can only imagine it's more confusing for users | 09:56 |
| pstolowski | Chipaca: i see :). i thought i you meant you fixed it server side so it never sends or accepts it | 09:57 |
| pstolowski | s/sends/returns/ | 09:58 |
| Chipaca | pstolowski: 'snap info firefox' has plenty of markdown | 09:59 |
| Chipaca | it's just not too ugly | 09:59 |
| pedronis | mvo: hi, is #6404 ready for re-review or not yet ? | 10:20 |
| mup | PR #6404: snapstate: auto transition on experimental.snapd-snap=true <Created by mvo5> <https://github.com/snapcore/snapd/pull/6404> | 10:21 |
| * zyga fixed one small bug and moves forward | 10:55 | |
| mup | PR snapd#7073 closed: interfaces: OpenGL/Vulkan drivers provided by a base should be usable <⛔ Blocked> <Created by jhenstridge> <Closed by jhenstridge> <https://github.com/snapcore/snapd/pull/7073> | 11:00 |
| mup | PR snapd#7414 opened: tests: keep track of installed packages and restore the state after the test <Test Robustness> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/7414> | 11:07 |
| zyga | mborzecki: some feedback there | 11:12 |
| zyga | mborzecki: I experimented with something similar, have a variant of that for dpkg that's somewhat weaker than yours | 11:13 |
| zyga | mborzecki: I think we could join forces and craft something together | 11:13 |
| mborzecki | zyga: sgtm, let me look at the apt-tool and your draft pr | 11:13 |
| zyga | mborzecki: thank you | 11:14 |
| zyga | mborzecki: if you want we can spin up a blank testbed-tool PR and land that quickly | 11:14 |
| zyga | and add various measurement parts on top | 11:14 |
| zyga | note that my PR does one essential change | 11:14 |
| zyga | it fixes where we restore | 11:14 |
| zyga | it's no longer causing the world to fall apart | 11:14 |
| ogra | zyga, i see some weird behaviour using layuouts ... when using an install or configure hook to put a file into $SNAP_DATA and using a layout to map that file to a subdir in /etc i somehow end up with an empty file in $SNAP_DATA ... in what order do the hooks run in relation to layouts ? | 11:15 |
| zyga | I think we should split that out as a small PR and land separately after careful review | 11:15 |
| ogra | (looks like the copied file is overlayed by an empty one somehow, if i take the layout out of snapcraft.yaml i get a proper file as expected (just not mapped to /etc) | 11:16 |
| ogra | ) | 11:16 |
| zyga | ogra: to answer your question, hooks cannot ever observe pre-layout filesystem | 11:16 |
| zyga | ogra: can you file a bug, ideally with a small reproducer, I'll take a look | 11:16 |
| ogra | so i need a wrapper and actually write into the file then ... k | 11:16 |
| zyga | ogra: some bugs got fixed recently, 2.41 will be much better on that than before | 11:16 |
| zyga | ogra: but perhaps there's another one | 11:17 |
| zyga | https://github.com/snapcore/snapd/pull/7168/files#diff-d2dd0567bc81c6005b7c0f2e727f9b57R531 | 11:17 |
| zyga | er | 11:17 |
| zyga | sorry, cannot copy paste in this wayland business | 11:17 |
| zyga | anyway | 11:17 |
| mup | PR #7168: tests: measure testbed for leaking mountinfo entries <Created by zyga> <https://github.com/snapcore/snapd/pull/7168> | 11:17 |
| zyga | mborzecki: I need to fix the die situation to release 2.41 | 11:17 |
| zyga | for opensuse | 11:17 |
| ogra | well, i can switch my machine to beta, perhaps it works there ... | 11:17 |
| zyga | mborzecki: mvo said he doesn't have time to review 7362 | 11:17 |
| * ogra gives it a try with 2.41 | 11:18 | |
| zyga | pstolowski: ^ can you look instead, mvo said another reviewer is fine | 11:18 |
| zyga | I can then cherry pick that into a distro patch | 11:18 |
| mvo | pedronis: 6404 is ready yes | 11:27 |
| pedronis | yea, added it to my queue | 11:28 |
| pstolowski | zyga: 7362? ok, looking | 11:31 |
| zyga | yes, thanks! | 11:32 |
| pstolowski | allright, i remember that other PR from contributor.. | 11:34 |
| mup | PR snapd#7413 closed: hookstate/ctlcmd: fix snapctl set help message <Simple 😃> <Created by stolowski> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/7413> | 11:34 |
| === ricab is now known as ricab|lunch | ||
| zyga | mborzecki: https://github.com/snapcore/snapd/pull/7400 is green, I think you are the one to review it | 11:44 |
| mup | PR #7400: cmd/snap-update-ns: don't propagate detaching changes <Bug> <Created by zyga> <https://github.com/snapcore/snapd/pull/7400> | 11:44 |
| * Chipaca ponders lunch | 11:46 | |
| zyga | more progress, more passing tests | 11:56 |
| zyga | feels like a good day now | 11:56 |
| pstolowski | zyga: +1 with 2 minors | 12:02 |
| zyga | thanks, looking | 12:02 |
| zyga | pstolowski: replied on both, can you check the first answer please | 12:04 |
| pstolowski | yep, done, got it | 12:08 |
| zyga | super, thank you | 12:11 |
| mup | PR snapcraft#2700 closed: schema: build-base support for the kernel type <Created by sergiusens> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/2700> | 12:18 |
| * dot-tobias says hi | 12:34 | |
| doko | mvo, sergiusens: snapcraft autopkg test failure, triggered by binutils in eoan | 12:34 |
| ogra | fix binutls then :P | 12:39 |
| Chipaca | pstolowski: did you resolve the core18 test prepare failures? getting them too | 12:40 |
| Chipaca | https://travis-ci.org/snapcore/snapd/builds/581107491?utm_source=github_status&utm_medium=notification | 12:40 |
| cachio | Chipaca, is it new? | 12:49 |
| Chipaca | cachio: i think so | 12:49 |
| cachio | Chipaca, I'll take a look if nobody else is already on it | 12:50 |
| Chipaca | cachio: thanks! | 12:50 |
| cachio | np | 12:50 |
| === ricab|lunch is now known as ricab | ||
| cjp256 | Is there an "official" recommendation/doc stating how to detect if running in a snap? I've seen a couple of apps doing it slightly differently (e.g. checking for "SNAP" or "SNAP_NAME" in environment). Someone in #snapcraft was asking. sergiusens pointed out that snapcraft itself uses SNAP_NAME, the preferred route to detect if being invoked by another classic snap. | 12:57 |
| pstolowski | Chipaca: not yet, i was about to look at this. got them a couple of times since yesterday | 12:58 |
| Chipaca | cachio: standup? | 13:01 |
| ijohnson | cjp256: usually most folks just check for $SNAP being defined | 13:03 |
| sergiusens | ijohnson: in the past lxd had a problem as snapcraft was calling out to it (the deb) and the in-code logic assumed it was a snap, so we check to see if the SNAP_NAME matches what we want | 13:06 |
| mup | PR snapcraft#2701 opened: spread tests: update gnome extension tests <Created by sergiusens> <https://github.com/snapcore/snapcraft/pull/2701> | 13:06 |
| ijohnson | sergiusens: fair enough, I haven't written many classic snaps so I haven't run into that kind of issue | 13:07 |
| sergiusens | assumed it was a snap as it was only checking for the existence of the export and not the value it carried... SNAP_NAME is an easier value to check against as it is fixed to the snap | 13:07 |
| zyga | jdstrand: hey, can you please look at https://github.com/snapcore/snapd/pull/7400 | 13:14 |
| mup | PR #7400: cmd/snap-update-ns: don't propagate detaching changes <Bug> <Created by zyga> <https://github.com/snapcore/snapd/pull/7400> | 13:14 |
| zyga | it's super small in non-test code, just interested in your opinion | 13:14 |
| Chipaca | zyga: it might be related to the generator, as there is a target (local-fs.target) which is part of snapd.service (and .socket) critical path | 13:14 |
| Chipaca | zyga: in 16.04, which doesn't use the generator | 13:15 |
| Chipaca | afaik | 13:15 |
| cjp256 | It does seem like something that should be formalized one way or another and added to the docs. degville: Where do you think would be the most appropriate place? | 13:15 |
| zyga | Chipaca: in the bug report it happened on core18 system | 13:16 |
| zyga | Chipaca: but in either case, we need to think about what happens when snapd doesn't have snaps mounted | 13:16 |
| zyga | Chipaca: I think only bad things will happen | 13:16 |
| Chipaca | zyga: ?? | 13:16 |
| zyga | Chipaca: because no plugs or no slots | 13:16 |
| zyga | Chipaca: e.g. on reboot with new kernel, new system key | 13:16 |
| Chipaca | zyga: why would a new system key mean no snaps mounted? | 13:17 |
| zyga | Chipaca: new kernel -> you reboot -> you race with mounting snaps -> you lose -> you create profiles without any interfaces for apps | 13:17 |
| Chipaca | zyga: why are you racing with mounting snaps? | 13:17 |
| zyga | Chipaca: because snapd.service is not synchronized with .mount units | 13:18 |
| Chipaca | zyga: it is | 13:18 |
| Chipaca | zyga: I just said so | 13:18 |
| zyga | how is that done? | 13:18 |
| zyga | ah, I misunderstood that | 13:18 |
| zyga | can you re-state please | 13:18 |
| Chipaca | at least on 16.04, i haven't checked 18.04 but it's easy to check | 13:18 |
| Chipaca | zyga: there's a local-fs.target | 13:18 |
| Chipaca | zyga: mounts are done before that | 13:18 |
| Chipaca | zyga: snapd runs after that | 13:18 |
| Chipaca | at least, that's how it _should_ work :-) | 13:19 |
| zyga | mmm, I see | 13:20 |
| Chipaca | zyga: easy to check | 13:20 |
| Chipaca | zyga: in core18, systemctl show snap-core-*.mount | 13:20 |
| Chipaca | systemctl show snap-core-*.mount | grep Before | 13:20 |
| Chipaca | should list local-fs.target | 13:21 |
| zyga | they have before=snapd.service now | 13:21 |
| zyga | hmmm | 13:21 |
| zyga | isn't that automatic somehow? | 13:21 |
| Chipaca | $ systemctl show snap-core18-*.mount | grep Before | 13:21 |
| zyga | the local-fs | 13:21 |
| Chipaca | Before=local-fs.target snapd.service multi-user.target umount.target | 13:21 |
| zyga | something to check | 13:21 |
| Chipaca | ^ that's 16.04, 18.04 should have the same | 13:21 |
| zyga | oh | 13:21 |
| zyga | show vs cat | 13:21 |
| zyga | thanks, I didn't know about this | 13:21 |
| Chipaca | and, snapd should have After | 13:21 |
| Chipaca | in 16.04 that After actually includes every .mount | 13:22 |
| Chipaca | note the mount has a before on snapd.service :-) | 13:22 |
| Chipaca | so it *should* be doubly-sure that it won't happen | 13:22 |
| Chipaca | there is explicit ordering | 13:22 |
| Chipaca | now, does it work? i don't know | 13:22 |
| Chipaca | if it doens't there's clearly a bug somewhere | 13:22 |
| zyga | mhm | 13:22 |
| zyga | yeah, it'd be interesting to see if we can attempt to reproduce this somehow and run it in a loop | 13:22 |
| zyga | to gather some fresh data | 13:23 |
| Chipaca | it might just be in the weird snapd in our core18 spread environ | 13:23 |
| zyga | or it might be in our weird snapd-from-snap service | 13:23 |
| Chipaca | zyga: if you can get a shell on a system showing this, systemctl analyze plot would be useful | 13:23 |
| zyga | mmm, indeed | 13:24 |
| mborzecki | cmatsuoka: ping me if you have questions about snap-image | 13:31 |
| Chipaca | zyga: as last words "the treasure is buried in the" is hard to beat | 13:31 |
| cmatsuoka | mborzecki: thanks, I'll have a good look and probably questions will appear | 13:32 |
| zyga | Chipaca: :) | 13:35 |
| zyga | pstolowski: remember that TV crew? they are coming back for another episode | 13:35 |
| pstolowski | zyga: aha, what tv show is that? | 13:36 |
| zyga | pstolowski: gliniarze or something like that | 13:36 |
| degville | cjp256: I think documenting this is a good idea too. There currently isn't an ideal place - I do have a doc describing the running environment on my todo list, but for now it may be most useful in the snap hooks page: https://forum.snapcraft.io/t/supported-snap-hooks/3795 | 13:38 |
| mup | PR snapd#7383 closed: cmd/snap: improve help and error msg for snapshot commands <Created by galgalesh> <Merged by chipaca> <https://github.com/snapcore/snapd/pull/7383> | 13:46 |
| mborzecki | off to pick up the kids and do groceries | 13:59 |
| pedronis | mvo: https://github.com/snapcore/snapd/pull/6705#discussion_r321285831 | 14:07 |
| mup | PR #6705: bootloader: little kernel support <Needs Samuele review> <Created by kubiko> <https://github.com/snapcore/snapd/pull/6705> | 14:07 |
| pedronis | mvo: I'll work on that today after I pushed my current state of the image refactoring | 14:08 |
| === vicamo_ is now known as vicamo | ||
| Chipaca | pedronis: I'd rather we didn't print "lk" in error messages as it can be hard to tell from "1k" | 14:12 |
| pedronis | Chipaca: so LK ? | 14:12 |
| pedronis | I don't mind a ton | 14:12 |
| pedronis | as long as it is consistent | 14:12 |
| Chipaca | and "cannot open 1k environment file" is not impossible, as error messages go | 14:12 |
| * ogra proposes "hackish-android-bootloader" | 14:12 | |
| Chipaca | :) | 14:12 |
| Chipaca | ogra: it's like swedish chef was fired from the kitchen so they picked up bootloader hacking instead | 14:13 |
| ogra | haha | 14:13 |
| Chipaca | ogra: also, https://www.youtube.com/watch?v=B7UmUX68KtE | 14:13 |
| mvo | pedronis: thank you | 14:14 |
| ogra | LOL | 14:14 |
| ogra | Pöpcørn | 14:14 |
| mvo | pedronis: yeah, thats nice | 14:14 |
| ijohnson | pedronis: I'm sorry the explanation for the changes are complicated... I don't think we can do it during start-snap-services because that's too late I think, we need to disable the services before the post-refresh hook | 14:22 |
| ijohnson | I need to think more about stop-snap-services, that might be okay because we run that after the post-refresh hook IIRC | 14:23 |
| ijohnson | err s/post-refresh/pre-refresh/ | 14:23 |
| pedronis | ijohnson: anyway you are probably victim of having put code near something that seems wrong | 14:23 |
| pedronis | I don't understand anymore | 14:23 |
| pedronis | ijohnson: anyway my main worry about where we unlock remains | 14:24 |
| ijohnson | I can look into where we unlock, I mainly just put it there because that's what mborzecki commented on, I will try to find more examples in the codebase of how that is done other places | 14:24 |
| ijohnson | pedronis: the main thing I am trying to do is capture the state as late as possible from systemd during unlink-snap, and then restore it as early as possible in link-snap (similar for the undo handlers) | 14:26 |
| pedronis | I don't late is a useful concept here | 14:26 |
| pedronis | this a relatively fast operation holding the lock | 14:26 |
| ijohnson | I realize it's not very similar to other things in the codebase since it's both very short lived and also potentially ver long lived | 14:26 |
| pedronis | as they are on trunk | 14:26 |
| pedronis | yes, my worry there | 14:26 |
| pedronis | we are changing some property of this things | 14:26 |
| pedronis | pstolowski: can you remind why of this line: https://github.com/snapcore/snapd/blob/master/overlord/snapstate/handlers.go#L1311 it doesn't make sense to me atm, why would we restore a config while we are unlinking a snap | 14:28 |
| pedronis | I would imagine we do that when linking | 14:28 |
| pedronis | the other unlink has a save instead | 14:28 |
| pedronis | ijohnson: anyway that's why your code confused me ^, I don't understand the lines it has close | 14:29 |
| pedronis | now | 14:30 |
| ijohnson | ok | 14:30 |
| pedronis | ijohnson: anyway as I said we need either need to move those interaction with systemd close to end of the handlers, to to the stop/start ones | 14:31 |
| pedronis | s/to to/or to/ | 14:31 |
| pstolowski | pedronis: in undoLink, afair we restore config of previous revision, because config might have already been updated e.g. by configure hook | 14:31 |
| pedronis | pstolowski: it's part of the undo case? | 14:31 |
| pedronis | but we do a relink at the very end | 14:32 |
| pedronis | pstolowski: if it's right, it needs a comment, if it isn't it needs moving/going away. atm it's very confusing | 14:33 |
| pedronis | ijohnson: also s/end/start or end/ | 14:34 |
| pedronis | sorry | 14:34 |
| ijohnson | what other tasks do we run after a configure hook? the only one I see is the health-check | 14:35 |
| ijohnson | could the health-check fail and trigger things to be undone? | 14:36 |
| ijohnson | if the health-check could fail the entire change, then I think pstolowski's code is correct as is, because the configure hook could have successfully finished and committed changes to the state that we need to restore | 14:37 |
| ijohnson | s/health-check/any other random task that runs after the configure snap task/ | 14:37 |
| pedronis | but by definition we should run anything on an unlinked snap | 14:38 |
| pedronis | shouldn't run | 14:38 |
| pedronis | that's the bit I don't get | 14:38 |
| pedronis | I'm not saying we shouldn't restore | 14:38 |
| pedronis | I'm questioning the where | 14:38 |
| ijohnson | hmm | 14:38 |
| pedronis | when of it | 14:38 |
| ijohnson | I see your point now | 14:39 |
| ijohnson | for my case, with specifically undoLinkSnap, we could save the state of the services very early in the function rather than right before the unlink | 14:39 |
| ijohnson | would that make more sense? | 14:39 |
| ijohnson | because we can't save it after the unlink | 14:40 |
| pstolowski | i need to refresh my memory re the details of config snapshots; the general idea is to switch to respective version of config whenever we switch between revisions, whether it's undo or succesfull op | 14:42 |
| pedronis | ijohnson: yes, doing this slow things that don't want the lock either first or last seems best | 14:43 |
| pedronis | sorry, do want the lock | 14:43 |
| pstolowski | i'll check that code and add comments or fix if needed | 14:43 |
| pedronis | pstolowski: thank you | 14:43 |
| zyga | re | 14:43 |
| pedronis | pstolowski: it got me slightly confused in a different review | 14:44 |
| ijohnson | pedronis: ack I will make changes to that effect, do you still want me to pursue using the task for storing the list short-term and then commit it later for long term storage? | 14:44 |
| pedronis | ijohnson: yes, think about that and see if it's better or worse | 14:45 |
| ijohnson | I'm still not convinced we can entirely push it out to the start/stop-snap-services tasks I need to think on that a bit more | 14:45 |
| ijohnson | okay I will see what I can do | 14:45 |
| ijohnson | thanks for the review | 14:45 |
| pedronis | ijohnson: keeping unlink/link fast is attractive | 14:45 |
| pedronis | in terms of think we can reason about | 14:45 |
| ijohnson | also did you see my questions about the new `snap model` output on your forum post? | 14:45 |
| ijohnson | (and also my PR for that too) | 14:45 |
| pedronis | ijohnson: yes, but didn't get to look at it yet, I saw the question | 14:46 |
| ijohnson | okay, thanks | 14:46 |
| pedronis | my original thought was not do that, but now not sure | 14:46 |
| pedronis | I'm thinking about it | 14:46 |
| ijohnson | okay, well whenever you get a chance either on the PR or the forum post is fine and I'll update the PR with whatever else you need | 14:46 |
| ijohnson | oh one last thing, do you think I should split out the cli.StoreAccount change to a different PR? | 14:47 |
| ijohnson | it seemed like a small change so I kept it but I could also see the case for making that a different PR | 14:47 |
| pedronis | ijohnson: the PR at least at moment doesn't look too big, and we have nothing pressing that would need it, so seems ok | 14:48 |
| pedronis | as is | 14:48 |
| ijohnson | pedronis: ack, thanks | 14:48 |
| jdstrand | zyga: I plan to (7400) | 14:50 |
| zyga | thank you | 14:52 |
| diddledan | ooh, 2.41 is in candidate! | 15:00 |
| diddledan | \o/ | 15:00 |
| * diddledan reboots to loonicks to try it | 15:00 | |
| pstolowski | zyga, Chipaca annoyingly, I cannot reproduce that spread test issue we talked about on standup when running individual tests on gc | 15:13 |
| zyga | pstolowski: it's always like that | 15:14 |
| zyga | the magic compounds :/ | 15:14 |
| diddledan | dang. my changes in 2.41 aren't enough for makemkv to find the optical drive :-( | 15:35 |
| diddledan | looks like it still needs another permission granted: `/sys/bus/scsi/devices r,` | 15:36 |
| diddledan | I would expect that to be in hardware-observe. but we _could_ add a new scsi-observe if we want it more fine-grained? | 15:36 |
| diddledan | @jdstrand ^^^^ | 15:37 |
| mup | PR snapcraft#2701 closed: spread tests: update gnome extension tests <Created by sergiusens> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/2701> | 15:39 |
| diddledan | how do I double-check the device cgroup again to make sure it added the sg0 device into it? | 15:40 |
| mup | PR snapd#7415 opened: tests: fix mountinfo-tool filtering when used with rewriting <Created by zyga> <https://github.com/snapcore/snapd/pull/7415> | 15:45 |
| mup | PR snapd#7416 opened: seed/seedwriter: start of Writer and internal policy16/tree16 <Created by pedronis> <https://github.com/snapcore/snapd/pull/7416> | 15:45 |
| pedronis | mvo: next steps ^ but it's huge instead of just big because I need to land the seed16 one | 15:46 |
| pedronis | first | 15:47 |
| pedronis | doing the small bootloader Find change now | 15:48 |
| diddledan | ok, it looks like the cgroup has it assigned (actually sg2, not sg0) `c 21:2 rwm` | 15:50 |
| diddledan | so my changes are working correctly, they're just not sufficient :-) | 15:50 |
| mup | PR snapd#7387 closed: packaging/fedora, tests/lib/prepare-restore: helper tool for packing sources for RPM <Simple 😃> <Created by bboozzoo> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/7387> | 15:51 |
| mup | PR snapcraft#2702 opened: Release changelog for 3.8 <Created by sergiusens> <https://github.com/snapcore/snapcraft/pull/2702> | 15:54 |
| * cachio lunch | 16:00 | |
| mvo | pedronis: cool, I think I will have a look at this pr tomorrow | 16:00 |
| * Chipaca → run | 16:01 | |
| mvo | pedronis: uh, 3k diff :/ | 16:01 |
| * mvo runs as well | 16:01 | |
| zyga | mvo: haha | 16:01 |
| zyga | mvo: now you promised ;) | 16:01 |
| mvo | zyga: heh - its actually not that huge, it contains the seed loading one (7381) | 16:02 |
| Chipaca | mvo: *actual* run? | 16:02 |
| mvo | Chipaca: nah, not today :( | 16:02 |
| Chipaca | mvo: aw, had my hopes up | 16:03 |
| Chipaca | anyway, 'm off. take care and see some of you around later. Otherwise, have a great weekend! | 16:03 |
| zyga | Chipaca: o/ | 16:03 |
| mup | PR snapd#7417 opened: interfaces/wayland: On classic systems only consider the OS slot for auto-connect <Created by AlanGriffiths> <https://github.com/snapcore/snapd/pull/7417> | 16:07 |
| pedronis | mvo: it contains the previous one as I said | 16:21 |
| pedronis | but yes, it will be 1400 on its own | 16:21 |
| mvo | pedronis: I'm experimenting with bases remodel right now, when testing this for real I get "invalid request: unexpected data after serial-request and optional model" | 16:46 |
| mvo | pedronis: is this something known, am I doing it wrong? | 16:46 |
| pedronis | mvo: experimenting in which sense? | 16:47 |
| mvo | pedronis: doing a uc16->uc18 remodel | 16:47 |
| pedronis | mvo: with the what kind of model? | 16:47 |
| pedronis | in the tests? | 16:47 |
| pedronis | for real? | 16:47 |
| mvo | pedronis: the real models | 16:47 |
| mvo | pedronis: should I use test models for this? | 16:47 |
| pedronis | mvo: the issue is that the serial vault knows about remodeling, but the store doesn't | 16:48 |
| pedronis | the real model use the store as serial vault | 16:48 |
| pedronis | the store needs change for that to work | 16:48 |
| mvo | pedronis: aha, ok. that makes sense then, I will setup appropriate test models and retry :) | 16:49 |
| mvo | pedronis: thats fine, thanks for the explaination | 16:49 |
| * mvo needs to be afk for a few minutes | 16:49 | |
| pedronis | mvo: the store is not setup to do remodels, but I suppose we need to teach about this kind of remodel | 16:49 |
| pedronis | if we want to do it | 16:49 |
| pedronis | a bit unclear though | 16:49 |
| mup | PR snapd#7159 closed: tests: add functions to make an abstraction for the snaps <Simple 😃> <Created by sergiocazzolato> <Merged by sergiocazzolato> <https://github.com/snapcore/snapd/pull/7159> | 16:51 |
| * zyga is on child watch duty | 16:51 | |
| zyga | though there's one more bugfix PR coming after | 16:51 |
| zyga | making some more progress | 17:23 |
| zyga | about to fix duplication on core16 | 17:23 |
| zyga | need to explore because unique to 16 somehow | 17:23 |
| Saviq | ughies https://status.snapcraft.io/ | 17:23 |
| zyga | https://xkcd.com/908/ | 17:24 |
| mup | PR snapd#7418 opened: many: pass the rootdir and options to bootloader.Find <Created by pedronis> <https://github.com/snapcore/snapd/pull/7418> | 17:35 |
| mup | PR snapd#7362 closed: cmd: unify die() across C programs <Created by zyga> <Merged by zyga> <https://github.com/snapcore/snapd/pull/7362> | 17:44 |
| mup | PR snapd#7358 closed: cmd/system-shutdown: Rename die() -> die_reboot() to fix build failures with LTO enabled <⛔ Blocked> <Created by dcermak> <Closed by zyga> <https://github.com/snapcore/snapd/pull/7358> | 17:45 |
| ardaguclu_ | TCP connection gets lost after handshake completed for snapcraft.io | 17:48 |
| zyga | ardaguclu_: status.snapcraft.io | 17:48 |
| ardaguclu_ | zyga: yeah, after checking there I tried myself | 17:49 |
| mborzecki | https://paste.ubuntu.com/p/dpSXHrZtQc/ funny test failure | 17:52 |
| jdstrand | diddledan: hardware-observe should handle that | 18:09 |
| jdstrand | diddledan: as for the device cgroup, look in /sys/fs/cgroup/devices/snap.name.cmd/devices.list for the major:minor of /dev/sg0. can also do: udevadm info /dev/sg0 to see if the TAGS has your snap | 18:12 |
| diddledan | Thanks, jdstrand . The device is correctly in the cgroup. | 18:14 |
| jdstrand | \o/ | 18:14 |
| mup | PR snapd#7406 closed: cmd/snap-confine: keep track of snap instance name and the snap name <Created by bboozzoo> <Merged by zyga> <https://github.com/snapcore/snapd/pull/7406> | 18:27 |
| mvo | hrm, hrm, so commit 92a096278e040b452125260b00742ae6e62caf2c (zyg@xenial-server) breaks the cla checker in my 6404 pr - I wonder how to get out of this missery | 18:30 |
| zyga | oops | 18:34 |
| zyga | ah, 2016 | 18:34 |
| zyga | uff :) | 18:35 |
| zyga | mvo: I can offer a rebase solution ;) | 18:35 |
| zyga | (just kidding) | 18:35 |
| zyga | can you whitelist zyga@xenial-server and | 18:35 |
| zyga | just call it a day? | 18:35 |
| mvo | zyga: I have no idea why it breaks :/ | 18:35 |
| mvo | zyga: I think that works - or I could rebase my patches | 18:35 |
| zyga | I mean the rebase was a silly suggestion | 18:35 |
| zyga | you'd have to rebase 3 years | 18:35 |
| mvo | zyga: I wonder why it hits my PR :( | 18:36 |
| zyga | mvo: git history caught it? | 18:36 |
| zyga | mvo: AFAIK we take only shallow history | 18:36 |
| zyga | so something close is related pulling it in? | 18:36 |
| zyga | not sure | 18:36 |
| * mvo has no idea | 18:37 | |
| jdstrand | zyga: I review PR 7400. curious, how much more is there wrt 'mount namespace robustness'? | 19:29 |
| mup | PR #7400: cmd/snap-update-ns: don't propagate detaching changes <Bug> <Created by zyga> <https://github.com/snapcore/snapd/pull/7400> | 19:29 |
| jdstrand | reviewed* | 19:29 |
| zyga | jdstrand: MS_SHARED for sure | 19:29 |
| zyga | jdstrand: some more tests | 19:29 |
| zyga | jdstrand: like mount-ns test for more constructs, specifically the mimic | 19:29 |
| zyga | jdstrand: after that I think we can return to wrapping up persistence | 19:29 |
| jdstrand | zyga: are there open PRs for these or are those all the ones you'll reopen? | 19:29 |
| zyga | jdstrand: I'll open those, I'm trying to keep it under six | 19:30 |
| zyga | jdstrand: some are not fully ready | 19:30 |
| * jdstrand nods | 19:30 | |
| zyga | jdstrand: note that MS_SHARED you had reviewed before | 19:30 |
| zyga | jdstrand: but it uncovered more issues | 19:30 |
| zyga | jdstrand: there's something wrong with loopback devices | 19:30 |
| zyga | jdstrand: they definitely leak | 19:30 |
| zyga | jdstrand: apps, bases, all alike | 19:30 |
| zyga | jdstrand: so more research into that area | 19:30 |
| zyga | jdstrand: is there some specific concern you have? | 19:31 |
| jdstrand | zyga: just trying to understand the scope and timing of work. this will be a blue item for us | 19:32 |
| zyga | jdstrand: we have an item for it, under robustness | 19:32 |
| zyga | jdstrand: you can refer to that one from your roadmap perhaps, not sure if that's useful | 19:33 |
| zyga | jdstrand: note that most of the work involves digging and fixing tests | 19:33 |
| zyga | jdstrand: there are relatively few patches still, compared to the amount of time | 19:33 |
| jdstrand | zyga: yes. and we will have a similar one that is blue for me to perform the reviews to help you achieve it :) | 19:33 |
| zyga | jdstrand: thank you :) | 19:33 |
| zyga | jdstrand: so far most of the effort was to stabilize the test suite | 19:33 |
| zyga | jdstrand: so that we actually get to write new tests | 19:33 |
| jdstrand | yeah. it is all really good stuff | 19:34 |
| zyga | I'll produce updates and answers for your questions tomorrow | 19:34 |
| jdstrand | anyway, don't mean to dtract from your evening or change any priorities, just a simple question to help me plan | 19:34 |
| zyga | jdstrand: no worries, it's always fine :) | 19:34 |
| === msalvatore_ is now known as msalvatore | ||
| === guiverc2 is now known as guiverc | ||
| * Chipaca EOWs | 21:00 | |
| diddledan | gui snaps work in WSL2: https://usercontent.irccloud-cdn.com/file/HyrFk7TC/image.png | 23:23 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!
