[04:54] morning mvo, there's some weirdness going on with some of the spread tests where it just dies immediately before running spread [04:55] The SU doc for me has an example build to look at, I couldn't figure it out, almost seems like spread itself is dying [04:56] Anyways time for me to log off, good luck [04:56] ijohnson: thank you, I noticed this right before going to bed too, I had hoped it was a fluke [04:56] ijohnson: thanks, get some rest! [04:56] ijohnson: I have a look [05:04] PR snapd#7642 opened: travis.yml: add 'sh -x' to run-checks [05:15] good morning [05:16] hey mvo [05:18] morning [05:18] hey zyga and mborzecki [05:19] hey mborzecki [05:19] looks like travis has some problems sometimes [05:19] mvo: zyga: hey [05:19] fog, fog as far as I can see :) [05:19] and of course my debug PR works just fine :( [05:19] hahaha [05:19] mvo: merge everything into it and land ;) [05:20] some store woes yesterday [05:20] error: cannot search: got unexpected HTTP status code 403 via GET to [05:20] [05:20] "https://api.snapcraft.io/api/v1/snaps/search?confinement=strict%2Cclassic&fields=anon_download_url%2Carchitecture%2Cchannel%2Cdownload_sha3_384%2Csummary%2Cdescription%2Cbinary_filesize%2Cdownload_url%2Clast_updated%2Cpackage_name%2Cprices%2Cpublisher%2Cratings_average%2Crevision%2Csnap_id%2Clicense%2Cbase%2Cmedia%2Csupport_url%2Ccontact%2Ctitle%2Ccontent%2Cversion%2Corigin%2Cdeveloper_id%2Cdeveloper_name%2Cdeveloper_vali [05:20] dation%2Cprivate%2Cconfinement%2Ccommon_ids&q=pc&scope=wide" [05:20] some 503 as well [05:37] hello niemeyer [05:37] niemeyer: when you are around today could you please look at sergio's fix for spread spread tests: https://github.com/snapcore/spread/pull/91 [05:37] PR spread#91: tests: Fix adhoc test [05:42] mborzecki: https://github.com/snapcore/snapd/pull/7614 could use a review [05:42] PR #7614: cmd/snap-confine: implement snap-device-helper internally [05:42] and it's green (shocking) [06:00] mvo: do you think you could look at https://github.com/snapcore/snapd/pull/7547 [06:00] PR #7547: many: use a dedicated named cgroup hierarchy for tracking [06:00] at least conceptually [06:01] it is already being looked at by ian and jamie but I wanted to make sure you understand what is going on there as well [06:01] zyga: sure, was wondering if samuele needs to look but I'm happy to do it [06:01] mvo: I think we all should look because it's such a fundamental part [06:01] zyga: sounds good [06:18] PR snapd#7634 closed: tests: ignore directories for go modules [06:19] * zyga needs to garden his PRs [06:35] zyga: left some comments under your PRs [06:35] zyga: btw. what's the problem with ijohnson's named hierarchy use? [06:36] none? [06:36] can you clarify please [06:41] zyga: https://github.com/snapcore/snapd/pull/7547#pullrequestreview-305622718 but i see it was nothing weird ;P [06:41] PR #7547: many: use a dedicated named cgroup hierarchy for tracking [06:42] mborzecki: did you see my reply on https://github.com/snapcore/snapd/pull/7547#issuecomment-545280953 [06:44] PR snapd#7629 closed: snap-recovery: create filesystems as defined in the gadget [06:51] mvo: https://github.com/snapcore/snapd/pull/7642 is green [06:51] PR #7642: travis.yml: add 'sh -x' to run-checks [06:53] zyga: yeah, its strange [06:53] zyga: another of my PRs also went green [06:53] let's merge it and see [06:53] zyga: maybe it was a fluke [06:54] question is what was it, your PR might answer that [06:54] zyga: yeah, we can always revert === pstolowski|afk is now known as pstolowski [07:04] morning [07:04] hey pawel [07:04] pstolowski: sorry to jump into this straight away but I'm hunting for reviews for the emit() branch again :) [07:05] mborzecki: I debugged why I saw different numbers in spread and on my system, without the memory fix patch [07:05] mborzecki: turns out my desktop had more rules because of the document portal [07:05] hah [07:05] mborzecki: so the test is reliable [07:05] mborzecki: it just measures a smaller set of rules [07:06] mborzecki: but it's still clear as day and night that it is using small amount of memory vs huge before [07:06] zyga: will get to it in a moment [07:06] pstolowski: I rebased the original PR on the emit branch now [07:07] pstolowski: and it shows exactly how little has to change, on top of emit(), to fix the memory issue [07:07] pstolowski: this is the only remaining patch there: https://github.com/snapcore/snapd/pull/7632/commits/03ef1bd9b36c368c64c717879098f7fbe533c389 [07:07] PR #7632: interfaces: de-duplicate emitted update-ns profiles [07:24] brb [07:24] so cold and foggy today [07:24] hot tea anyone? [07:42] re [07:52] pstolowski: thank you, replied to both [07:56] kyrofa: hey, replied to https://github.com/nextcloud/nextcloud-snap/issues/913 [08:14] zyga: Did you review the PR? [08:15] zyga: Good morning [08:15] niemeyer: good morning [08:15] niemeyer: I had a look at it, is there something wrong? [08:15] Feels flimsy [08:15] It broke because we're using awk to parse yaml [08:15] I'm happy to rework it, let me look again [08:16] The PR puts more shell to make more guesses about what the document happens to be [08:16] Trivial to break it again [08:16] sure, I'll make it less reliant on shell [08:25] zyga: The proper fix is to not use awk or head to parse yaml [08:43] * zyga had to help with the baby, back now [08:43] niemeyer: yeah, I have an idea how to do that now [08:44] Python would do [08:44] Single command line.. python3 -c "import yaml" [08:50] hmmm when a test suite is embedded in another test suite, and the first one has Test* funcs, shared tests are doubled when both suites are run [08:50] mborzecki: yes [08:50] kind of expected when one things about it, but still a bit meh when doing a larger split [08:51] you really need to create a Base [08:51] of some kind [08:51] without tests [08:51] s/things/thinks/ [08:51] for the share fixture stuff [08:51] there are some examples [08:51] in the code base [08:51] yeah, my concern is actually untangling devicemgr test suite [08:52] tried to be 'smart' about it :/ [08:52] mborzecki: if it's too onoreous you can always do it in two steps [08:52] just split tests across files but keep one suite [08:52] and do the suite splitting when it's less blocking [08:52] pedronis: mhm, that's what i have now [08:53] i suppose is good enough for now, at least the test file is no longer 4k+ lines [08:53] yes, we have some packages in that state [08:53] still better than too big test files [08:53] and can be improved [08:54] 4k lines? them's rookie numbers! [08:54] 8 files changed, 4699 insertions(+), 4516 deletions(-) [08:54] heh [08:54] * Chipaca looks at snapstate_test [08:55] PR snapd#7643 opened: overlord/devicestate: refactor and split into per-functionality files, drop dead code [08:55] hmmm [08:55] niemeyer: yep, fixed locally, just running a quick check before pushing [08:55] haha github decided it's not going to show diffs ;P [08:55] are you seeing that too? [08:56] for the past few days I had issues with diff pane [08:56] it was just empty, apart from the typical github UI [08:56] zyga: nah, the diffs are just too large in this PR [08:56] #7643 [08:56] PR #7643: overlord/devicestate: refactor and split into per-functionality files, drop dead code [08:56] \o/ [08:58] mborzecki: 7640 is ready for a review [09:02] niemeyer: updated now [09:03] oops [09:03] no permission to push [09:05] niemeyer: opened https://github.com/snapcore/spread/pull/92 [09:05] PR spread#92: tests: fix ad-hoc test [09:09] pushed a really shallow suite split with common suite [09:18] mborzecki: 8K diff [09:18] mborzecki: very appropriate, 5K is passe [09:19] zyga: Thanks! Let's see if it passes [09:27] heh, go.mod broke gorename? [09:31] niemeyer: it's green :) [09:32] ... and it's in [09:32] super! thank you [09:33] niemeyer: this was a prelude to a change that does more than just fixes tests: https://github.com/snapcore/spread/pull/90 [09:33] PR spread#90: runner: remove filterDir, default include to "." [09:37] Reviewed [09:38] niemeyer: the motivation behind this change is to eventually have "tar" invoked with project path and not the individual files and directories [09:39] niemeyer: because that in turns allows us to say tar --exclude-vcs-ignore, which will automatically handle .gitignore [09:39] niemeyer: there's an older branch that builds towards handling .gitignore: https://github.com/snapcore/spread/pull/89 [09:39] PR spread#89: runner: pass --exclude-vcs-ignores to tar [09:39] zyga: There are two different issues here: [09:40] The first one is that spread is meant to send whatever files the project needs it to send, and also exclude whatever files it wants to to drop [09:40] We cannot make assumptions about what the person using spread is actually trying to do with it [09:40] But you as the spread user should be able to tune that list to your desires [09:41] I agree, and therefore the idea is to allow users to configure spread to respect .gitignore-like files if they are present [09:41] The second thing is that filterDir has a specific purpose. The PR dropped that purpose silently, and said nothing about it. That's usually a bad sign. [09:41] zyga: The fact you want files to be ignored by git is not an indication that you want spread not to ship them [09:41] niemeyer: did I miss the prupose? it is duplicated by how tar is invoked [09:42] zyga: What is being done by filterDIr? [09:42] niemeyer: we filter out spread-reuse files [09:42] zyga: Maybe I'm the one missing it [09:42] niemeyer: but this is redundant because the way spread invokes tar does that already [09:42] niemeyer: it's not in the diff here because that part did not have to change [09:43] zyga: I see now, sorry and thanks [09:43] zyga: Let me play with it to see how it diverges if it all [09:43] if at all [09:44] niemeyer: thanks! [09:44] niemeyer: I think this part is safe, it really is about working around a bug in tar [09:44] niemeyer: tar needs to be shown the root of the project for it to consider .gitignore there (along with the required option to read it) [09:45] niemeyer: separarely https://github.com/snapcore/spread/pull/91 can be closed now because it was a part of the branch you just merged [09:45] PR spread#91: tests: Fix adhoc test [09:47] zyga: So, the outcome is different indeed: [09:47] oh? [09:48] https://www.irccloud.com/pastebin/kb82VgAp/ [09:49] Weird.. [09:49] The pastebin corrupted line breaks [09:49] is that different on the receiving end? [09:49] Yeah, I suspect these paths are actually stored into the file [09:49] Double checking [09:50] https://www.irccloud.com/pastebin/bXQa7OJX/ [09:58] PR snapd#7644 opened: gadget: rename existing and add new helpers for checking filesystem/partition presence [10:04] mborzecki: https://github.com/snapcore/spread/pull/85 needs master merge [10:04] PR spread#85: spread/client: workaround bash 4.3 subshell errexit issues [10:56] brbr, need tea [10:58] we have a confusing spread system naming pattern [10:59] imagine in 2064 [10:59] spread -debug -v google:ubuntu-core-64-64: [11:02] :) [11:07] it's non-problem until 2028, I mean we might want to clean it up, but I don't think we should do that now particularly [11:07] ubuntu-core-64-128 \o/ [11:08] I mean ubuntu-core-30-32 start to get confusing, otoh there might not be a 32 anymore at that point [11:11] ;D [11:11] I was totally joking btw [11:11] Iza is back, made her some tea as well [11:26] hmm [11:26] mvo: did we do any changes to core/core16 recently? [11:27] mvo: I'm seeing /system-data/etc/systemd/user added to host mount table [11:27] or is the test flawed [11:27] * zyga runs a small experiment [11:29] PR snapd#7636 closed: gadget, overlord/devicestate: add support for customized update policy, add remodel policy [11:29] listening to Parliament today, we should change our job titles to "due of snapd" [11:29] "duke*" [11:31] ah, I think I know why [11:31] but it needs changes anyway [11:31] * zyga small break [11:42] PR snapd#7645 opened: client: add xerrors and wrap errors coming from "client" [11:43] zyga: this got added iirc because of the user session service that was added recently [11:46] mvo: that's excellent, thank you for confirming this [11:50] mvo: reviewed https://github.com/snapcore/snapd/pull/7645#pullrequestreview-305827995 [11:50] PR #7645: client: add xerrors and wrap errors coming from "client" [11:51] mborzecki: conflicts on https://github.com/snapcore/snapd/pull/7643 [11:51] PR #7643: overlord/devicestate: refactor and split into per-functionality files, drop dead code [11:52] ah damn, i expected the other PR to be in conflict [11:53] ijohnson, mborzecki: can you please finish your review on https://github.com/snapcore/snapd/pull/7639 [11:53] PR #7639: interfaces: emit update-ns snippets to function [11:53] it will help to review the bugfix PR in the evening [11:58] Chipaca, are you able to add categories in the forum ? i tend to think we should have a "multipass" one [12:07] what the .... ?!?! [12:07] * zyga looks closer [12:08] zyga sure thing, I'll get on that right after breakfast and then SU [12:08] thanks! [12:10] WHAT THE ?! [12:10] mvo: something changed drastically in core16 [12:10] mvo: do you have a sec? [12:11] * Chipaca takes a break [12:15] mborzecki: you won't believe what's in this commit message [12:15] PR snapd#7646 opened: tests: update mount-ns after addition of /etc/systemd/user [12:15] mborzecki: please have a look, I think we need to understand what is going on [12:15] note that core18 is okay [12:16] it was not affected by whatever caused this [12:16] zyga: one in 7646? [12:16] I'm looking at anything merged to snap-confine [12:16] yes [12:16] mborzecki: for the purpose of staying honest I can split this into two patches [12:16] but please read it first [12:18] mborzecki: I strongly suspect it is a fallout from 4b4cf41fd17951157221f406e6f7a5bb5f2f6fff [12:19] it probably was me not noticing the test didn't run [12:19] I'll double check in a moment [12:20] but I don't understand how this affects everything outside of /snap yet [12:21] hmm, but I did change the data set there [12:21] it cannot be that [12:24] I cannot find anything that would explain this in changes since the 6th of September [12:32] hmmmm [12:32] maybe a false alarm [12:32] I think I confused multiple runs together, I cannot reproduce this now, it must have been a run from the branch that enables sharing [12:32] mborzecki: I think crisis averted [12:37] PR snapd#7644 closed: gadget: rename existing and add new helpers for checking filesystem/partition presence [12:38] do we intend to land #7642? [12:38] PR #7642: travis.yml: add 'sh -x' to run-checks === ricab is now known as ricab|lunch [12:49] mborzecki: please look at the last comment on https://github.com/snapcore/snapd/pull/7643 [12:49] PR #7643: overlord/devicestate: refactor and split into per-functionality files, drop dead code [12:50] hi guys, all my snaps stopped working this morning, I get this message: [12:51] snap-confine has elevated permissions and is not confined but should be. Refusing to continue to avoid permission escalation attacks [12:51] ahasenack: hello [12:51] ahasenack: can you please provide the output of "snap version" [12:51] zyga: hi, https://pastebin.ubuntu.com/p/fNNY5Fbk6t/ [12:52] ahasenack: can you please share the output of systemctl status apparmor.service [12:52] zyga: https://pastebin.ubuntu.com/p/dcWgC4K2CV/ [12:52] sounds like I have an ordering problem in systemd services [12:53] let me delete the zfskey one [12:53] something is off [12:53] can you paste the output of journalctl -u apparmor.service [12:54] it's the same, but I now started it explicitly, and it worked [12:54] zfskey-nsnx@nsnx.service/start is a systemd service I added recently, I'm guessing it has impossible targets/ordering [12:54] snaps are working after I started apparmor [12:56] thanks! [12:56] good luck :) [12:57] zyga: sorry, was at lunch [12:57] mvo: no more crisis :) [12:58] Saviq: fix, but... [12:58] sergiusens (cc Saviq): hey, is snapcraft running the review-tools in travis/LP now? [12:58] o/ jdstrand [12:59] PR snapd#7641 closed: tests: moving ubuntu-19.10-64 from google-unstable to google backend [12:59] jdstrand: not yet, that is my next work item after I get some core20 stuff out of the way [12:59] sergiusens: Saviq mentioned https://travis-ci.org/CanonicalLtd/multipass/jobs/601308279#L4750, which I was surprised to see [13:00] jdstrand: line 55 and 59 from https://travis-ci.org/CanonicalLtd/multipass/jobs/601308279/config [13:00] sergiusens: since there are probably some things we want to auto-block on, but others we do not (eg, it would be ok to let the store handle personal-files, for example) [13:01] PR snapd#7647 opened: gadget: skip structures with no partition table entry during remodel [13:01] sergiusens: ah, ok. note ^ before enabling everywhere :) [13:01] jdstrand: this was the contentious topic we did not reach an agreement on, we have no way to know what was whitelisted store side [13:02] jdstrand: hello! :) [13:02] sergiusens: right. so, you can give snap-review '--json' then read that and decide what to and what not to block on [13:03] sergiusens: eg, you could choose to entirely skip declaration-snap-v2 [13:03] s/skip/ignore/ [13:03] zyga: hi :) [13:03] jdstrand: I will have some things for you after the standup [13:07] jdstrand: should we also --allow-classic if the snap is classic? Do the review-tools issue some output when that flag is used? [13:08] jdstrand: we're running it ourselves explicitly [13:09] but it's likely we're using it wrong :) [13:12] Saviq: yeah, see that and now, you are fine, though you may want to set SNAP_ENFORCE_RESQUASHFS=0 for faster reviews [13:12] sergiusens: you will probably want that ^ too, or at the very least a way to opt in/out of it [13:13] jdstrand: I just wonder if there's something we can do to make it pass with the new plugs? Does that talk to the store for assertions or? [13:13] zyga: fyi, I will be looking at https://github.com/snapcore/snapd/pulls/review-requested/jdstrand this week [13:13] Saviq: I issued the snap decl already. I said 'fix', I meant, 'I fixed it' [13:14] Ah! :) [13:14] ChrisTownsend: ^^ [13:14] Saviq: actually, I only fixed part of it [13:14] jdstrand: thank you :) [13:14] Saviq, ChrisTownsend: when it is uploaded to the store, it will now pass [13:15] Saviq, ChrisTownsend: but not on your invocation of snap-review. gimme a sec for an updated invocation [13:15] * ChrisTownsend reads backlog [13:16] jdstrand: I prepared the apparmor memory fix in a way I believe can land after your reviews and some follow-ups for things like function names and typical iteration elements [13:16] zyga / jdstrand: Re the NetworkManager D-Bus Introspectable issue from yesterday (https://bugs.launchpad.net/snapd/+bug/1849291) → sorry for being impatient, but I need a rough timeframe when this will be available in core to decide if I should postpone the refactor in my snap until Introspectable is allowed in the network-manager interface. Let me know if I can be of any help! [13:16] Bug #1849291: Interface network-manager should grant access to org.freedesktop.DBus.Introspectable [13:17] jdstrand: I included a memory usage test as well [13:17] dot-tobias: I didn't manage to do it yesterday but I can propose a fix today [13:17] dot-tobias: please ask mvo about release schedule for core / snapd [13:18] zyga: That would be marvelous, thank you! Will do [13:18] dot-tobias: is there a PR for this issue yet? [13:18] dot-tobias: (in a meeting so I may a bit slow replying) [13:19] mvo: no, not yet, it's a small change to allow dbus introspection method on network manager that was missing before [13:24] ChrisTownsend (cc Saviq): https://paste.ubuntu.com/p/RChQs4dkTQ/ [13:25] jdstrand: ack, thanks! [13:26] jdstrand: cool, thanks! [13:27] jdstrand: in theory we could also download the snap declaration from the store, right? So that we can flag any changes necessary early? [13:28] ijohnson: https://github.com/snapcore/snapd/pull/7547#issuecomment-545280953 [13:28] zyga: after reading your response that all makes sense [13:28] PR #7547: many: use a dedicated named cgroup hierarchy for tracking [13:28] cool, excellent :) [13:28] pstolowski: about that kind of code (loadSeed), you don't need to create the db either, you can pass nils to LoadAssertions, it's created there because the test wants to look at it [13:30] zyga: re-approved [13:30] ijohnson: woot, thanks! [13:31] pedronis: ah, good, this just answered another dillema that i'd have, good [13:32] pstolowski: go doc Seed in seed should be helpful (it says this e.g.) hopefully [13:33] mvo: pstolowski: I'm touching that area again, how strongly would you like me to change the places we use cstr(s) for constraint(s) expanding them to the full word, I vaguely remember you commented on this at some point [13:36] mborzecki: can you look at https://github.com/snapcore/snapd/pull/7646 [13:36] PR #7646: tests: update mount-ns after addition of /etc/systemd/user [13:36] degville, hey [13:36] degville, I am free, just ping me when you have time [13:36]