| mborzecki | morning | 05:04 |
|---|---|---|
| mup | PR snapd#7323 closed: features, overlord: make parallel-installs exported, export flags on startup <Created by bboozzoo> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/7323> | 05:10 |
| zyga | School morning | 05:33 |
| mborzecki | zyga: hey, yeah, school morning | 05:33 |
| zyga | I will be around shortly | 05:42 |
| zyga | Walking back from school | 05:42 |
| mborzecki | mvo: hey | 05:45 |
| mvo | hey mborzecki ! welcome back | 05:46 |
| mup | PR snapd#7384 opened: HACKING.md: clarify where "make fmt" is needed <Simple 😃> <Created by mvo5> <https://github.com/snapcore/snapd/pull/7384> | 06:05 |
| mup | PR snapd#7382 closed: osutil: make flock test more robust <Test Robustness> <Created by zyga> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/7382> | 06:08 |
| zyga | hey mvo, thank you for merging that | 06:10 |
| zyga | mvo: btw, can you look at the corresponding non-test code | 06:10 |
| mvo | zyga: good morning! thanks for creating it :) | 06:10 |
| zyga | mvo: I was puzzled by how that can ever happen | 06:10 |
| zyga | mvo: it looks like a bug in the compiler or something in our understanding | 06:11 |
| zyga | mvo: as that function cannot return nil, nil, can it? | 06:11 |
| zyga | mborzecki: 1st day of school and temperature drops by 10 degrees | 06:11 |
| mvo | zyga: NewFileLock, no, that looks impossible, we are explicitly about the "nil" there. | 06:11 |
| zyga | mvo: right? I was worried when I saw that because it feels we are missing something important | 06:12 |
| mvo | zyga: yeah, its very confusing | 06:16 |
| zyga | perhaps go gc bug? | 06:16 |
| mborzecki | arch package updated | 06:23 |
| mup | PR snapd#7385 opened: tests/cross/go-build: use go list rather than shell trickery <Simple 😃> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/7385> | 06:25 |
| mborzecki | super simple PR ^^ | 06:25 |
| mvo | zyga: I looked at this flock issue again, I think you fixed it. the issue is that "err = lock.TryLock()" may return nil - if there is lock. but then err, Equals, osutil.ErrALreadyLocked will trigger a err->Error I think | 06:29 |
| zyga | oh | 06:29 |
| zyga | actually, yeah | 06:29 |
| mvo | zyga: which causes the crash, your code now first assigns err and then checks for non-nil so this error goes away | 06:29 |
| zyga | actually | 06:29 |
| zyga | no wait | 06:29 |
| zyga | hmm | 06:29 |
| zyga | so are you saying that due to the raceiness | 06:30 |
| zyga | the error can be nil | 06:30 |
| mvo | zyga: correct | 06:30 |
| zyga | and then Equals would panic? | 06:30 |
| mvo | zyga: yes, I think so, equal will try to make it an error | 06:30 |
| mvo | zyga: I have not looked at the details but I think so | 06:30 |
| zyga | that's certainly plausible and more realistic that a gc bug :) | 06:30 |
| mvo | zyga: I think "c.Assert(nil, Equals, osutil.Err..") should behave the same | 06:30 |
| zyga | nice, you made my mind more at ease | 06:30 |
| zyga | oh, good idea | 06:31 |
| zyga | I'll chcek | 06:31 |
| mvo | zyga: yeah, thanks for fixing this one! it is really an interessting one :) | 06:31 |
| zyga | indeed | 06:31 |
| zyga | https://www.irccloud.com/pastebin/z0RHTYJO/ | 06:31 |
| zyga | you are spot on | 06:31 |
| mvo | yay | 06:31 |
| zyga | that's so unexpected btw | 06:31 |
| mvo | zyga: agreed | 06:31 |
| mvo | zyga: ErrorMatches will DTRT - except it matches instead of equals :/ | 06:32 |
| mborzecki | off to the school with kids, back in ~1.5h | 06:35 |
| zyga | mborzecki: good luck :) | 06:36 |
| zyga | hey sil2100 :) | 06:36 |
| mvo | zyga: oh no, I think this error happens in the "diff" code we added a while ago :( so gopkg.in seems to got broken by this | 06:37 |
| mvo | zyga: that bug is on me I think :( | 06:37 |
| zyga | ahhh | 06:38 |
| zyga | I recall this | 06:38 |
| zyga | well, one less but two more :D | 06:38 |
| mvo | zyga: yeah, there is also the circular bug in github.com/kr/pretty - makes me wonder if we should simply revert that diff code | 06:38 |
| zyga | the diff on error is somewhat useful, I wonder how hard would it be to fix it instead | 06:40 |
| zyga | but also worried if we use old version or if upstream is dead | 06:40 |
| mvo | zyga: I fix it, thats trivial | 06:40 |
| mvo | zyga: I'm just a bit sad that its "1 added, 2 new bugs introduced" | 06:41 |
| zyga | yeah, no disagreement | 06:41 |
| zyga | "what's under this rock over here, let me turn it over" | 06:41 |
| mvo | niemeyer: hey, good morning! if you have some spare cycles, I have a very simple gocheck PR (https://github.com/go-check/check/pull/114) that would be great to get merged. I feel bad because its an issue introduced with a PR that I originally proposed :( | 06:51 |
| mup | PR go-check/check#114: checkers: fix misleading error for c.Check(nil, Equals, "foo") <Created by mvo5> <https://github.com/go-check/check/pull/114> | 06:51 |
| mvo | mborzecki: can you please check if github.com/sanity.io/litter is available in fedora as a package? I wonder if we can switch away from kr/pretty as its unmaintained (and there is this cyclic references bug that can cause hangs) | 07:00 |
| mup | PR snapd#7384 closed: HACKING.md: clarify where "make fmt" is needed <Simple 😃> <Created by mvo5> <Merged by zyga> <https://github.com/snapcore/snapd/pull/7384> | 07:01 |
| === pstolowski|afk is now known as pstolowski | ||
| pstolowski | mornings | 07:02 |
| zyga | hello pawel | 07:03 |
| zyga | pstolowski: feels good to send kids to school again, no? :) | 07:03 |
| jamesh | hi zyga | 07:04 |
| zyga | hey, good morning | 07:05 |
| jamesh | zyga: at GUADEC, there was a request for a snapd feature that might be quite difficult to implement | 07:06 |
| zyga | tell us! | 07:06 |
| jamesh | zyga: essentially they want to move the org.freedesktop.portal.Flatpak.Spawn portal interface from Flatpak to xdg-desktop-portal, and have it do something sensible for snaps | 07:07 |
| pstolowski | zyga: haha, indeed, i just got back from there | 07:07 |
| jamesh | this is essentially a way for a confined process to spawn a second process with even stricter confinement: https://github.com/flatpak/flatpak/blob/master/data/org.freedesktop.portal.Flatpak.xml | 07:07 |
| jamesh | the idea being to allow applications that confine portions of their code to continue doing so when put in Flatpak/snap confinement (e.g. web browsers) | 07:09 |
| zyga | jamesh: hmm | 07:15 |
| zyga | what is the interface like? | 07:16 |
| jamesh | the XML file I linked to has documentation for the interface | 07:16 |
| jamesh | the main features are (a) pass file descriptors to spawned process, (b) optionally block network or file system access, (c) expose a restricted set of files to the spawned subprocess | 07:17 |
| zyga | yeah, reading that now | 07:18 |
| jamesh | It's basically something to replace the fork/unshare/exec code you might find in multi-process apps | 07:18 |
| zyga | hmmm | 07:18 |
| zyga | some of the concepts don't map cleanly | 07:18 |
| zyga | what is a "sandbox" in flatpak term? | 07:18 |
| zyga | as in flag value 4 | 07:19 |
| zyga | spawn in a sandbox | 07:19 |
| jamesh | I need to look that up. | 07:19 |
| zyga | is there a demo app for this or a production app that would use it? | 07:19 |
| jamesh | I think the big one at the moment is webkit-gtk | 07:19 |
| zyga | I think it's doable but not small or on or our roadmap for this cycle so it's hard for me to say if we can do it | 07:19 |
| zyga | jamesh: some things there look vert flatpack specific, like ~/.var/app/$APP_ID/sandbox | 07:20 |
| zyga | I'm sure it's all doable but we'd have to think about how to construct that | 07:21 |
| zyga | jamesh: that dbus api is for the user session? | 07:21 |
| jamesh | zyga: right. I think the idea is to get something that could provide equivalent functionality | 07:21 |
| zyga | jamesh: so you spawn it as yourself essentially, right? | 07:21 |
| jamesh | the xdg-desktop-portal version wouldn't necessarily be identical | 07:21 |
| zyga | I see | 07:21 |
| jamesh | yes. This is a user session API, same as the other portal services | 07:21 |
| zyga | well, I'm open to this for sure | 07:21 |
| zyga | it would be nice to explore this if we have agreement / time to do it | 07:22 |
| jamesh | zyga: so I think "fully sandboxed" means only access to the runtime, app data and files specifically exposed in ~/.var/app/$APP_ID/sandbox | 07:27 |
| jamesh | so snap-wise, that would be no home interface, and only access to a small subset of ~/snap/$snap_name | 07:28 |
| zyga | almost like "all interfaces disconnected" | 07:28 |
| jamesh | possibly, yeah. | 07:29 |
| jamesh | zyga: If we could get this all working and it was adopted by upstreams, it would probably remove the need for the allow-sandbox flag on our browser-support interface | 07:31 |
| zyga | jamesh: indeed, that's pretty interesting as an evolution of the concept "we package stuff as-is" to "we provide a new layer to apps" | 07:32 |
| jamesh | zyga: in the current release of GNOME, the file manager makes use of bubblewrap to confine thumbnailer subprocesses. So a big part of this is to allow this model to continue in the new world | 07:35 |
| jamesh | (since you can't run bubblewrap in Flatpak's bubblewrap based sandbox either) | 07:35 |
| zyga | mborzecki: hey | 07:59 |
| zyga | mborzecki: I updated https://github.com/snapcore/snapd/pull/7218/files | 07:59 |
| zyga | it's just a new spread test | 07:59 |
| mup | PR #7218: tests: measure behavior of the device cgroup <Created by zyga> <https://github.com/snapcore/snapd/pull/7218> | 07:59 |
| zyga | could you please have a look, make sure to read both commit messages | 07:59 |
| niemeyer | mvo: Thanks, will have a look | 08:01 |
| mvo | niemeyer: should be super trivial, let me know if I can help in any way | 08:01 |
| niemeyer | mvo: It's in, thanks again! | 08:05 |
| mvo | niemeyer: \o/ | 08:05 |
| mup | Bug #1842282 opened: can't add channel information as install downloaded snap <Snappy:New> <https://launchpad.net/bugs/1842282> | 08:19 |
| mborzecki | re | 08:32 |
| mborzecki | mvo: there's apackage of github.com/sanity.io/liter in fedora, it's version 1.1.0 if that makes any difference | 08:35 |
| mvo | mborzecki: thank you | 08:36 |
| mborzecki | mvo: hmm https://github.com/sanity.io/litter is a 404 for me? | 08:37 |
| mvo | mborzecki: https://github.com/sanity-io/litter | 08:38 |
| mborzecki | duh | 08:41 |
| mborzecki | ok, works for me :P | 08:41 |
| mborzecki | mvo: do you remember that other lib? https://github.com/davecgh/go-spew | 08:42 |
| mborzecki | mvo: looks like litter is packaged for debian too, so we can switch to either one | 08:43 |
| mvo | mborzecki: spew is available for centos/fedora too? cool | 08:54 |
| mborzecki | mvo: for centos/amzn we use a bundled pacakge | 08:55 |
| mborzecki | s/use/build/ | 08:55 |
| niemeyer | mvo: I have an on going diff sitting on my disk for a while around that area of go-check, btw.. | 08:57 |
| niemeyer | mvo: It improves a bit the way the diff is displayed in some cases | 08:58 |
| niemeyer | mvo: But I've hit an issue with kr's diff lib.. it broke down in some nested data | 08:58 |
| mvo | niemeyer: yes, we have a fix for this in kr/pretty | 08:59 |
| niemeyer | mvo: Oh? | 08:59 |
| mvo | niemeyer: it breaks on cyclic data | 08:59 |
| niemeyer | mvo: Was that submitted upstream? | 08:59 |
| mvo | niemeyer: https://github.com/kr/pretty/pull/58 | 08:59 |
| mup | PR kr/pretty#58: Fix infinite recursion when diffing structs with cycles <Created by stolowski> <https://github.com/kr/pretty/pull/58> | 08:59 |
| mvo | niemeyer: also https://github.com/kr/pretty/pull/44 (which looks similar) | 09:00 |
| mup | PR kr/pretty#44: Detect cyclic references in Fdiff() <Created by mbergin> <https://github.com/kr/pretty/pull/44> | 09:00 |
| niemeyer | Oh, nice | 09:00 |
| mvo | niemeyer: I can't reach him though :/ maybe he replies to twitter pings :) | 09:00 |
| mvo | this is open for some time | 09:00 |
| niemeyer | I can understand him :) | 09:02 |
| niemeyer | We should just fork temporarily while this is sorted | 09:02 |
| mvo | niemeyer: works for me | 09:04 |
| geodb27 | People : hi ! I run a lxd cluster on three virtual machines under ubuntu-18.04 lts. As you might know, lxd is shipped via snap. There seem to be a problem with the way updates are run. Indeed, the three machines are all the same considering installed os version and hardware. They, of course, have all their time maintained via ntp. Error occur when all three decide that it is time to update lxd : since the processes are ran at the same time, this | 09:32 |
| geodb27 | results in a crash of the overall cluster and require intervention to have all three systems in a production state. Question is : how does snap decide it is time to update what is installed ? If it is done internally, wouldn't it be fine to add some "salt" to the decided time so that everything does not start at the same time everywhere ? | 09:32 |
| Chipaca | geodb27: updates shouldn't run at the same time, as there is a random element to the time | 09:39 |
| Chipaca | geodb27: additionally, you can schedule when updates happen, so if you have a requirement for them not to happen simultaneously that is something you can configure | 09:40 |
| Chipaca | geodb27: if you have debug logs on snapd will log when it will update next, but debug logs are very chatty so most people don't | 09:41 |
| Chipaca | geodb27: 'snap refresh --time' will print when the next refresh will happen (mostly) | 09:41 |
| Chipaca | geodb27: 'mostly' because right after snapd starts, and until the first time it checks, the next refresh time isn't set | 09:42 |
| geodb27 | Thanks for you answer Chipaca. I'll check how it is done on the different machines. | 09:42 |
| Chipaca | geodb27: as to how to set the refresh timer, https://snapcraft.io/docs/keeping-snaps-up-to-date | 09:42 |
| geodb27 | Well, the problem seems somewhere else. I'll have to wait for the next lxd's snap to check what goes wrong. | 09:44 |
| * zyga finishes calls and jumps into the fray :) | 09:44 | |
| geodb27 | What is sure is that a lxd cluster is in an "undefined" state after a snap update while a single one runs fine. | 09:45 |
| Chipaca | geodb27: i'm not sure what that means :-| maybe stgraber can shed some light? | 09:47 |
| ppd1990 | zyga: Thanks for reviewing #7366 - I've found a (I believe) satisfactory solution to only exposing appropriate gpg-agent functionality. I'd be quite glad if you could give it another look in the near future | 09:47 |
| mup | PR #7366: interfaces/gpg-keys: Allow access to gpg-agent and creation of lockfiles <Created by ppd1990> <https://github.com/snapcore/snapd/pull/7366> | 09:47 |
| zyga | ppd1990: sure, thank you for iterating | 09:48 |
| zyga | I'll look now :) | 09:48 |
| zyga | ppd1990: you mean the S.gpg-agent.extra socket? | 09:49 |
| ppd1990 | zyga: Right | 09:49 |
| zyga | ppd1990: that sounds very much better, I will defer the final review to jdstrand but I think that's no longer -0.5 on my end | 09:49 |
| zyga | ppd1990: please update the PR with that | 09:49 |
| zyga | make sure to update the summary of the interface as well, it's just text but it matters for review clarity | 09:50 |
| geodb27 | Indeed Chipaca. I'll look forward to fill in an issue on https://discuss.linuxcontainers.org/ when I face the same problem. Thanks for your kind answers anyway :-) | 09:50 |
| ppd1990 | zyga: Will do. Any opinion on it being necessary to "bend" the GPG binary in the snap to this socket? Is is somehow possible to "layout" this socket into the snap namespace instead of S.gpg-agent? | 09:50 |
| zyga | ppd1990: not yet, not for anything in /shome | 09:51 |
| zyga | (I meant /home/) | 09:51 |
| zyga | ppd1990: I think if there's a workable way to point gpg there that's enough | 09:51 |
| ppd1990 | zyga: Fair enough. If it works, it works :) Shall we update the interface description to include "signing and encrypting"? | 09:53 |
| zyga | mhm | 09:53 |
| zyga | plas | 09:53 |
| zyga | *please | 09:53 |
| zyga | gosh, I cannot type today | 09:53 |
| mup | PR snapd#7386 opened: many: refactor boot/boottest and move to bootloader/mockbootloader <Created by chipaca> <https://github.com/snapcore/snapd/pull/7386> | 09:53 |
| * Chipaca gives zyga a keyboard that does auto-correct on the fly | 09:54 | |
| zyga | Chipaca: thakns ;) | 09:54 |
| zyga | (that was deliberate) | 09:55 |
| * Chipaca pushes an update to the auto-correct snap to mess with zyga | 09:55 | |
| zyga | mborzecki: https://github.com/snapcore/snapd/pull/7218 should now pass | 09:56 |
| mup | PR #7218: tests: measure behavior of the device cgroup <Created by zyga> <https://github.com/snapcore/snapd/pull/7218> | 09:56 |
| zyga | Chipaca: brexit news is surprising | 09:57 |
| zyga | Chipaca: at this rate I would not be surprised in anything | 09:57 |
| Chipaca | zyga: which part of it? | 09:57 |
| zyga | Chipaca: the whole suspension of parliament | 09:58 |
| Chipaca | zyga: and the "if you vote this way you'll be fired"? | 09:58 |
| Chipaca | tyranny is fun | 09:58 |
| zyga | game of thrones, except some people are living in it and are on the frontline | 09:59 |
| Chipaca | "this was not meant as a tutorial!?!" | 10:00 |
| mborzecki | zyga: about https://github.com/snapcore/snapd/pull/7385#discussion_r319827158 i did the change and it looks a bit awkward, since it's a single go build -o /dev/null for all 5 binaries (in theory all binaries are written to /dev/null) | 10:04 |
| mup | PR #7385: tests/cross/go-build: use go list rather than shell trickery <Simple 😃> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/7385> | 10:05 |
| zyga | aw, I see | 10:05 |
| zyga | I forgot the binary aspect of it | 10:05 |
| zyga | normally I was just "go building" everything | 10:05 |
| zyga | but I didn't mind the binaries | 10:05 |
| zyga | so what is this doing? testing that we can build? | 10:06 |
| mborzecki | fwiw i think go build should erorr out there | 10:06 |
| mborzecki | zyga: yes, it's cross compiling those binaries | 10:06 |
| zyga | yeah, +1 | 10:07 |
| mborzecki | zyga: can you leave a +1 in the PR? it's green so we'll land it ;P | 10:07 |
| zyga | sure, done! | 10:08 |
| mborzecki | thanks! | 10:08 |
| mup | PR snapd#7385 closed: tests/cross/go-build: use go list rather than shell trickery <Simple 😃> <Created by bboozzoo> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/7385> | 10:08 |
| pedronis | Chipaca: I commented on #7386, we don't have any mock* package atm | 10:23 |
| mup | PR #7386: many: refactor boot/boottest and move to bootloader/mockbootloader <Created by chipaca> <https://github.com/snapcore/snapd/pull/7386> | 10:23 |
| Chipaca | fair | 10:23 |
| * Chipaca adds more mock packages to fix that issue | 10:23 | |
| pedronis | Chipaca: silly you :) | 10:23 |
| Chipaca | :-) | 10:24 |
| mborzecki | zyga: updated #7302 | 10:25 |
| mup | PR #7302: cmd/snap-confine: add support for parallel instances of classic snaps <â›” Blocked> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/7302> | 10:25 |
| zyga | mborzecki: thanks, I'll look shortly | 10:37 |
| zyga | mborzecki: 19.04 ha some more cgroup wonkiness | 10:37 |
| zyga | my test is failing there, I think you were right, the /proc/PID/cgroup format changerd | 10:37 |
| zyga | *changed | 10:37 |
| zyga | I'll finish and check that out soon | 10:37 |
| pedronis | Chipaca: +1, with comments on something related but prexisting | 11:08 |
| mup | PR snapd#7387 opened: packaging/fedora, tests/lib/prepare-restore: helper tool for packing sources for RPM <Simple 😃> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/7387> | 11:23 |
| mborzecki | zyga: support script for packit ^^ | 11:24 |
| zyga | looking | 11:25 |
| zyga | https://github.com/snapcore/snapd/pull/7387#pullrequestreview-282543875 | 11:29 |
| 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> | 11:29 |
| * zyga goes for a walk with the dog | 11:32 | |
| zyga | mborzecki: I'll review mount ns for classic snaps PR when I'm back | 11:33 |
| ppd1990 | zyga: I updated #7366. Thanks again for the guidance | 11:48 |
| mup | PR #7366: interfaces/gpg-keys: Allow access to gpg-agent and creation of lockfiles <Created by ppd1990> <https://github.com/snapcore/snapd/pull/7366> | 11:48 |
| === ricab is now known as ricab|lunch | ||
| Chipaca | pedronis: thanks! I'll fix those in a followup | 12:25 |
| mup | PR snapd#7386 closed: many: refactor boot/boottest and move to bootloader/bootloadertest <Created by chipaca> <Merged by chipaca> <https://github.com/snapcore/snapd/pull/7386> | 12:26 |
| cachio | zyga, did you see this one? https://paste.ubuntu.com/p/MngRDC5nQm/ | 12:27 |
| zyga | re | 13:00 |
| zyga | cachio: what about it? | 13:00 |
| Chipaca | mvo: needing to re-login to chrome, i'm omw | 13:01 |
| zyga | 2fa | 13:03 |
| mup | PR snapd#7388 opened: boot, bootloader, o/snapstate: boot env manip goes in bootloader <Created by chipaca> <https://github.com/snapcore/snapd/pull/7388> | 13:34 |
| Chipaca | pedronis, zyga, ^ includes (as separate commits) the cleanups you'd requested from previous PRs | 13:36 |
| pedronis | Chipaca: we need to talk | 13:38 |
| Chipaca | pedronis: ok | 13:38 |
| === ricab|lunch is now known as ricab | ||
| cmatsuoka | mvo: about the world clocks, you'll also need an extension called "panel world clock (lite)" | 13:48 |
| mup | PR snapd#7388 closed: boot, bootloader, o/snapstate: boot env manip goes in bootloader <Created by chipaca> <Closed by chipaca> <https://github.com/snapcore/snapd/pull/7388> | 13:49 |
| cachio | jamesh, hey | 13:50 |
| cachio | jacekn, https://github.com/snapcore/snapd/pull/7361#issuecomment-526330249 | 13:51 |
| mup | PR #7361: tests: enabling ubuntu 19.10-64 on spread.yaml <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/7361> | 13:51 |
| mvo | cmatsuoka: ta | 13:51 |
| cachio | jamesh, https://github.com/snapcore/snapd/pull/7361#issuecomment-526330249 | 13:51 |
| cachio | jamesh, any idea about this problem on eoan? | 13:51 |
| mup | PR snapd#7389 opened: cmd/libsnap: make feature flag enum 1<<N style <Simple 😃> <Created by zyga> <https://github.com/snapcore/snapd/pull/7389> | 13:55 |
| Chipaca | pedronis: do we have a convetion for constants vs variables? (i think no, but thought to check) | 14:04 |
| Chipaca | pedronis: nm | 14:05 |
| pedronis | Chipaca: in which sense? | 14:08 |
| Chipaca | pedronis: there was a constant defined for "snap_mode", and I wondered if we wanted it to be obvious it was a constant in usage | 14:09 |
| Chipaca | pedronis: but then i realised it was not actually used anywhere | 14:09 |
| pedronis | Chipaca: go doesn't have special naming convention for constants afaik | 14:10 |
| Chipaca | pedronis: so the problem disappeared with a nice round "pop!" | 14:10 |
| Chipaca | pedronis: :) | 14:10 |
| mup | PR snapd#7390 opened: tests: unit test for a refresh failing on configure hook <Created by stolowski> <https://github.com/snapcore/snapd/pull/7390> | 14:13 |
| mup | PR snapcraft#2696 opened: shell_utils: Add SNAPCRAFT_DEBUG_SCRIPTS env variable <Created by mkg20001> <https://github.com/snapcore/snapcraft/pull/2696> | 14:22 |
| mup | PR snapd#7391 opened: boot, bootloader, o/devicestate: boot env manip goes in boot <Created by chipaca> <https://github.com/snapcore/snapd/pull/7391> | 14:40 |
| Chipaca | pedronis: ^^^ | 14:40 |
| cachio | mvo, hey, this is the error on pi2 https://paste.ubuntu.com/p/zzQ8Q73SY6/ | 14:49 |
| cachio | when we try to create a user | 14:50 |
| Chipaca | cachio_: that error looks like /var/lib/extrausers isn't writable | 15:00 |
| mup | PR snapd#7389 closed: cmd/libsnap: make feature flag enum 1<<N style <Simple 😃> <Created by zyga> <Merged by zyga> <https://github.com/snapcore/snapd/pull/7389> | 15:02 |
| mvo | cachio_: what Chipaca said, can you ssh into this machine and touc a file in /var/lib/extrausers ? maybe something "funny" is going on with the writable magic | 15:05 |
| mvo | Chipaca: so I keep poking at ondras LK pr - I played with your idea to split the types and have a "lk" and "lkPrepareImage" and that makes the code easier but it feels a bit strange. I was wondering if we should have a "func PrepareImage(gadgetDir)" on the bootloader interface (and kill ConfigFile there which does not make sense in a world of partitions). wdyt? | 15:08 |
| Chipaca | mvo: would the PrepareImage(gadgetDir) on bootloader avoid all the ifs? | 15:09 |
| mvo | Chipaca: I am exploring this, the way image.go works probably also needs tweaking | 15:10 |
| mvo | Chipaca: anyway, if its not something you looked at (yet) I keep poking, was just wondering if you had thoughts :) | 15:10 |
| zyga | Chipaca: https://github.com/snapcore/snapd/pull/7320 needs some attention | 15:11 |
| mup | PR #7320: snap/pack, cmd_pack: 'snap pack --check-skeleton' checks interfaces <Created by chipaca> <https://github.com/snapcore/snapd/pull/7320> | 15:11 |
| Chipaca | mvo: if it avoids the ifs i'm fine with it :-) i'd check with pedronis wrt core20 but i think bootloader will be growing something similar anyway? not sure | 15:11 |
| Chipaca | zyga: ack | 15:11 |
| mvo | Chipaca: it seems conceptually clean so I think its worth exploring, I look at the image bits now and then I will chat with pedronis if this makes sense | 15:12 |
| Chipaca | mvo: if not, if the bootloader really needs different behaviour depending on whether it's in prepare or regular mode, then it's fine for it to be two separate things. Alternatively it might just be parametrised if the ifs are just to determine a string | 15:13 |
| Chipaca | or, you know, subclassed if it's somewhere in the middle :-) | 15:13 |
| zyga | and so it rains | 15:14 |
| zyga | welcome to september kids | 15:14 |
| mvo | Chipaca: yeah, I have sometihng without ifs now but I'm still not happy, it feels conceptually not great | 15:14 |
| zyga | mvo: 7286 is close but I left a comment there | 15:14 |
| pedronis | mvo: what does PrepareImage do? | 15:14 |
| mvo | Chipaca: https://github.com/snapcore/snapd/compare/master...mvo5:lk-2?expand=1 fwiw - based on the earlier ideas | 15:14 |
| zyga | cachio: can you look at 7218 again please | 15:15 |
| cachio | sure | 15:15 |
| mvo | pedronis: it would do the job of the current InstallBootConfig and setting the boot vars. its mostly there because with LK runtime and image-building time is conceptually different. the image-build needs to write to a file and the runtime to a partition. so I was thinking it would be nice to have a clear interface for this | 15:15 |
| zyga | cachio: https://github.com/snapcore/snapd/pull/7361 seems to be more complex, do you think we can close it and open dedicated PRs for address book interface and test issue, go fmt test issue and then the remaining 19.10 mechanics | 15:16 |
| mup | PR #7361: tests: enabling ubuntu 19.10-64 on spread.yaml <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/7361> | 15:16 |
| zyga | cachio: especially since the interface test may take more iteration | 15:17 |
| pedronis | mvo: ok, it will need some mediation for Core 20 though, because is not all a booloader problem | 15:17 |
| mvo | pedronis: yeah, let me experiment a little bit and then I can write something up. I don't want to waste your time with something that may not be great in the end | 15:18 |
| mvo | (s/great/helpful/) | 15:18 |
| cachio | zyga, ok, what about moving 19.10 to manual | 15:18 |
| zyga | cachio: that's okay too, if that helps you out | 15:19 |
| pedronis | mvo: I am also touching all that code, | 15:19 |
| zyga | cachio: or remove the tests from 19.10 | 15:19 |
| cachio | then we have the system in the spread yaml nad aI can create both changes | 15:19 |
| pedronis | well touching soon | 15:19 |
| zyga | cachio: anything that is focused on one task and can land fast is good in my book | 15:19 |
| cachio | ok, in that case I'll remove those failing tests from 19.10 | 15:19 |
| cachio | so we can go forward | 15:20 |
| cachio | zyga, thanks | 15:20 |
| zyga | that sounds good, thank you | 15:20 |
| pedronis | mvo: my worry is that not all the bootvars will be a task of the bootloader anymore | 15:23 |
| pedronis | so we cannot move the task completely there | 15:23 |
| pedronis | mvo: so the right interface in image is probably something like boot.PrepareImage() but exactly what it needs to take is not super clear atm | 15:25 |
| pedronis | of course internally it would call bootloader bits as needed | 15:25 |
| mvo | pedronis: right, I can add it to boot.PrepareImage, we probably still need something on the bootloader interface to setup a subset of the bootvars | 15:26 |
| Chipaca | pedronis: what's your opinon on using a separate bootloader instead? | 15:26 |
| pedronis | ? | 15:27 |
| Chipaca | ah, missing context | 15:27 |
| Chipaca | pedronis: <mvo> Chipaca: so I keep poking at ondras LK pr - I played with your idea to split the types and have a "lk" and "lkPrepareImage" and that makes the code easier but it feels a bit strange. I was wondering if we should have a "func PrepareImage(gadgetDir)" on the bootloader interface (and kill ConfigFile there which does not make sense in a world of partitions). wdyt? | 15:27 |
| pedronis | have bootloader that we use only in prepare-image ? | 15:27 |
| pedronis | doesnt' sound a great idea to me | 15:28 |
| Chipaca | yeah as mvo says it feels weird :-) | 15:28 |
| mvo | it makes the code more organized though but still feels weird | 15:28 |
| Chipaca | pedronis: it started because currently almost everything in lk is "if inPrepareImage { one thing } else { some other thing }" | 15:28 |
| pedronis | well | 15:29 |
| pedronis | I think that sounds more like | 15:29 |
| pedronis | there should be an optional interface | 15:29 |
| pedronis | and boot.PrepareImage knows about it | 15:29 |
| pedronis | and then the ifs prepare-image bit go under that | 15:29 |
| pedronis | interface | 15:29 |
| mvo | pedronis: interessting, let me ponder about this a bit | 15:30 |
| pedronis | if I understand for most booloaders runtime interface works for prepare-image | 15:30 |
| pedronis | this is not the case | 15:30 |
| pedronis | for lk | 15:31 |
| mvo | pedronis: correct | 15:31 |
| * cachio lunch | 15:31 | |
| pedronis | so have a couple short cut methods under some PrepareImageInstaller interface | 15:31 |
| pedronis | or something | 15:31 |
| pedronis | boot.PrepareImage default code would use the runtime methods | 15:32 |
| pedronis | but if it detects that interface | 15:32 |
| pedronis | it will use the shortcuts/methods there | 15:32 |
| mvo | pedronis: sounds good, I will work on this | 15:32 |
| pedronis | mvo: done this way, with boot has a hub is anyway a good step for Core 20 too | 15:34 |
| pedronis | mvo: it might be good to propse the image boot refactoring separately though | 15:34 |
| pedronis | if you find it works | 15:34 |
| mvo | pedronis: cool, I explore and then will push something | 15:34 |
| pstolowski | Chipaca: was https://bugs.launchpad.net/snapd/+bug/1823680 fixed? | 16:06 |
| mup | Bug #1823680: snap okay error unhelpful <snapd:New for chipaca> <https://launchpad.net/bugs/1823680> | 16:07 |
| zyga | pstolowski, cachio: I made some improvements to retry command, should I push it to 7256 or open a new PR and let 7256 focus on application of that command to the two tests? | 16:29 |
| zyga | I was about to push but based on pawel's comment I'm inclined to open a new PR instead | 16:29 |
| cachio | zyga, is it ok for me if you want to push over the same PR | 16:29 |
| mup | PR snapd#7379 closed: tests: add version-tool for comparing versions <Created by zyga> <Merged by zyga> <https://github.com/snapcore/snapd/pull/7379> | 16:30 |
| zyga | cachio: I'll open a new PR then, | 16:31 |
| zyga | cachio: could you look at 7370 | 16:32 |
| zyga | cachio: the prerequisite has just landed and I think this is good to go | 16:32 |
| zyga | it will fix one random failure on master | 16:32 |
| pstolowski | zyga: i'm ok with same PR but i'd keep that particular test intact, per my comment | 16:33 |
| zyga | pstolowski: I think it's cleaner, the original PR focused on changing the test | 16:33 |
| pstolowski | ok | 16:35 |
| mup | PR snapd#7392 opened: tests: rewrite "retry" command as retry-tool <Test Robustness> <Created by zyga> <https://github.com/snapcore/snapd/pull/7392> | 16:35 |
| zyga | cachio, pstolowski: https://github.com/snapcore/snapd/pull/7256#issuecomment-527200990 | 16:35 |
| cachio | zyga, sure | 16:35 |
| mup | PR #7256: tests: adding retry command and use it to delete $XDG directory <Simple 😃> <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/7256> | 16:36 |
| zyga | cachio: I already found some use for the retry-tool! | 16:38 |
| zyga | it's really nice, it can be used for very powerful one liners | 16:38 |
| zyga | check this out: | 16:40 |
| zyga | cachio: retry-tool in action https://www.irccloud.com/pastebin/LXXgoT84/ | 16:40 |
| pstolowski | nice! | 16:40 |
| pstolowski | i was always annoyned by writing these retry loop | 16:40 |
| pstolowski | *loops | 16:40 |
| zyga | yeah | 16:40 |
| zyga | let's review it :) | 16:40 |
| cachio | zyga, nice | 16:42 |
| cachio | retry tools are always usefull testing :) | 16:43 |
| pstolowski | i'll take a look tomorrow morning | 16:43 |
| pstolowski | ty | 16:43 |
| === pstolowski is now known as pstolowski|afk | ||
| zyga | thanks pawel! | 16:45 |
| zyga | ok, wrapping up one PR and calling it a day | 16:47 |
| mup | PR snapd#7370 closed: tests: fix ephemeral mount table in left over by prepare <Test Robustness> <Created by zyga> <Merged by sergiocazzolato> <https://github.com/snapcore/snapd/pull/7370> | 17:17 |
| zyga | thank you cachio! | 17:21 |
| cachio | zyga, yaw | 17:21 |
| cachio | zyga, is #7361 ok for merge now? | 17:58 |
| mup | PR #7361: tests: enabling ubuntu 19.10-64 on spread.yaml <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/7361> | 17:58 |
| Chipaca | pstolowski|afk: $ snap okay | 18:26 |
| Chipaca | error: you must have looked at the warnings before acknowledging them. Try 'snap warnings'. | 18:26 |
| Chipaca | pstolowski|afk: as per #6919 referenced in the bug | 18:26 |
| mup | PR #6919: cmd/okay: Remove err message when warning file not exist <Created by ardaguclu> <Merged by chipaca> <https://github.com/snapcore/snapd/pull/6919> | 18:26 |
| Chipaca | pstolowski|afk: and then I realised what you were asking me to do was to update the bug's status :-D done | 18:27 |
| Chipaca | post-run brain is not best brain | 18:27 |
| Chipaca | despite it thinking it is | 18:27 |
| mup | PR snapd#7192 closed: tests: initial change to retry on external files download <â›” Blocked> <Created by sergiocazzolato> <Closed by sergiocazzolato> <https://github.com/snapcore/snapd/pull/7192> | 18:48 |
| mup | PR snapd#7361 closed: tests: enabling ubuntu 19.10-64 on spread.yaml <Created by sergiocazzolato> <Merged by sergiocazzolato> <https://github.com/snapcore/snapd/pull/7361> | 20:44 |
| mup | PR snapd#7391 closed: boot, bootloader, o/devicestate: boot env manip goes in boot <Created by chipaca> <Merged by chipaca> <https://github.com/snapcore/snapd/pull/7391> | 21:43 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!