[00:16] <mup> PR snapcraft#3014 closed: meta: migrate get_build_base to Snap <Created by sergiusens> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/3014>
[05:52] <mborzecki> morning
[05:53] <mvo> hey mborzecki
[05:53] <mborzecki> mvo: hey hey
[06:01] <mborzecki> brb, new kernel
[06:04] <mborzecki> and back
[06:07] <mup> PR snapd#8413 closed: interfaces: don't use the owner modifier for files shared via document portal <Created by jhenstridge> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/8413>
[06:07] <mvo> mborzecki: welcome back!
[06:15] <zyga> Hey
[06:15] <zyga> Sleepy a bit
[06:15] <mborzecki> zyga: hey
[06:22] <mborzecki> mvo: the spread job on 19.10 didn't run here https://github.com/snapcore/snapd/pull/8443 but all the other jobs are green
[06:22] <mup> PR #8443: interfaces/many: miscellaneous policy updates xliv <Created by jdstrand> <https://github.com/snapcore/snapd/pull/8443>
[06:22] <mborzecki> mvo: 2.44 port of that PR is green, so we could probably merge 8443
[06:25] <jamesh> I also had a CI run pass on everything but 19.10 today: https://github.com/snapcore/snapd/pull/8356
[06:25] <mup> PR #8356: cmd/snap: Implement a "snap routine file-access" command <Created by jhenstridge> <https://github.com/snapcore/snapd/pull/8356>
[06:27] <pedronis> mvo: hi, uc20-snap-recovery  has a failure, is searching for a 3G partition size but now we have 1G again
[06:30] <mborzecki> so that 19.10 failure pops up in a number of PRs
[06:30] <pedronis> mvo: also should we move those main/uc20-* tests to 20.04 ? (they are running on 19.10 atm)
[06:31] <mborzecki> pedronis: do you have a log from that failure in uc20-snap-recovery?
[06:32] <pedronis> mborzecki: yes, but I would expect anything to fail with it at this point
[06:33] <pedronis> mborzecki: https://github.com/snapcore/snapd/pull/8435/checks?check_run_id=565884165
[06:33] <mup> PR #8435: configcore,tests: fix setting watchdog options on UC18/20 <Reviewed> <Created by pedronis> <https://github.com/snapcore/snapd/pull/8435>
[06:33] <mvo> pedronis: I remember that there was a PR to shrink the size of the ubuntu-data partition down to 1G from claudio iirc, probably this is what is causing this failure :(
[06:33] <mborzecki> pedronis: thanks, looking
[06:33] <mup> PR snapd#8438 closed: tests/session-tool: stop anacron.service in prepare <Simple 😃> <Created by zyga> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/8438>
[06:33] <mvo> pedronis: totally +1 for moving those from 19.10->20.04
[06:34] <mvo> mborzecki: are you on this? that's great \o/
[06:34] <pedronis> mvo: it landed
[06:35] <pedronis> mborzecki: nothing deep, but I need to go have breakfast:
[06:35] <pedronis> + MATCH '/dev/loop2p4 .* 3G\s* Linux filesystem'
[06:35] <pedronis> + sfdisk -l /dev/loop2
[06:35] <pedronis> that's what's failing afaict
[06:37]  * mvo onds
[06:37]  * mvo nods
[06:39] <mborzecki> mvo: pedronis: isn't that because of https://github.com/snapcore/pc-amd64-gadget/commit/a0eb11c6f6945d9395a858aaf396f3f3c0c609cf#diff-3da8b5c9ab650c29257cf39fccfa8be7 ?
[06:39] <mvo> mborzecki: I think so
[06:40] <mvo> mborzecki: while you are in those tests, please also move them from 19.10 to 20.04 (sorry if this is redundant information, we mentioned it above already but I wanted to be explicit :)
[06:40] <mborzecki> mvo: ack
[06:41] <mvo> ta
[06:43] <mborzecki> hmm that test should probably also check that the system-data was automatically expanded before it proceeeds to add that 110MB chunk in the next scenario
[06:49] <pedronis> mborzecki: maybe two PRs ?  one to fix it and one to change/improve/move things?
[06:50] <pedronis> they should also be called uc20-create-partions-* at this point
[06:52]  * pedronis will not ask for a pony as well
[06:55]  * mvo lols and wants a pony too
[06:58] <mborzecki> :P
[07:01] <pstolowski> morning
[07:01] <mvo> good morning pstolowski
[07:06] <pedronis> mvo: did a new pass on #8439
[07:06] <mup> PR #8439: secboot: import secboot on ubuntu, provide dummy on !ubuntu <UC20> <Created by mvo5> <https://github.com/snapcore/snapd/pull/8439>
[07:09] <mup> PR snapd#8445 opened: tests/main/uc20-snap-recovery: unbreak, rename to uc20-create-partitions <Simple 😃> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/8445>
[07:09] <mborzecki> mvo: pedronis: this should do it ^^
[07:09] <zyga> o/
[07:10] <zyga> mborzecki: can you review https://github.com/snapcore/snapd/pull/8437
[07:10] <mup> PR #8437: tests/session-tool: collect information about services on startup <Created by zyga> <https://github.com/snapcore/snapd/pull/8437>
[07:10] <zyga> it's just extra debug in case that service shows up again
[07:10] <pedronis> mborzecki: thx, there's another couple of uc20-snap-recovery-* tests that need the renaming, ->20.04 change
[07:10] <pedronis> mborzecki: maybe in the follow up?
[07:11] <mborzecki> pedronis: yeah, i'll add the tweaks to check the auto grow there too
[07:11] <pedronis> thank you
[07:13] <pedronis> zyga: pstolowski: hi, #8390 needs a 2nd/re-review
[07:13] <zyga> sure
[07:13] <zyga> looking
[07:13] <mup> PR #8390: state: add state.CopyState() helper <UC20> <Created by mvo5> <https://github.com/snapcore/snapd/pull/8390>
[07:13] <pstolowski> looking
[07:14] <zyga> it's interesting to look at test durations below
[07:14] <zyga> some are >> 50minutes
[07:16] <jamesh> https://github.com/snapcore/snapd/actions/runs/72216991 shows a run that was killed by the hard 5 hour timeout
[07:22] <zyga> interesting
[07:28] <mvo> thanks mborzecki !
[07:34] <zyga> mvo: https://github.com/snapcore/snapd/pull/8390#pullrequestreview-388854965 is good to merge
[07:34] <mup> PR #8390: state: add state.CopyState() helper <UC20> <Created by mvo5> <https://github.com/snapcore/snapd/pull/8390>
[07:34] <zyga> mvo: I think it's okay to override the test that failed due to session-tool
[07:34] <pedronis> mborzecki: I made some comments in #8433
[07:34] <mup> PR #8433: overlord/devicestate: support for recover and run modes <UC20> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/8433>
[07:34] <mborzecki> thanks
[07:35] <mvo> zyga: thank you
[07:36] <mup> PR snapd#8390 closed: state: add state.CopyState() helper <UC20> <Created by mvo5> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/8390>
[07:38] <pedronis> #8411 needs 2nd reviews
[07:38] <mup> PR #8411: boot: cleanup more things, simplify code <UC20> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/8411>
[07:38]  * zyga looks at 8411
[07:41] <pedronis> zyga: are you familiar with bootstate20.go ? otherwise you probably should read it first
[07:41] <zyga> yeah, I need to
[07:41] <zyga> there's loads of new things in core 20 for me
[07:44] <zyga> pedronis: what is modeenv?
[07:53] <pedronis> zyga: boot related state and settings that are not needed before we open writable, so they can be kept there,  main stuff is really which base to use
[07:53] <zyga> k
[08:04] <mwhudson> mvo: hey would snapd be ok with having go 1.14 as default in focal?
[08:05] <zyga> mwhudson: we test with master go
[08:05] <zyga> mwhudson: so I suspect so
[08:07] <mvo> mwhudson: pretty sure yes
[08:07] <mvo> mwhudson: like zyga said, we do run all our unit tests on go-master already
[08:08] <mvo> mwhudson: might be worthwhile to run a full spread run with 1.14 though, when do you want to change the default?
[08:08] <mwhudson> zyga, mvo: thanks
[08:08] <mwhudson> mvo: well about 3 weeks ago i guess
[08:08] <mwhudson> mvo: before release
[08:08] <mvo> mwhudson: *cough* sorry, I guess that was a dumb question
[08:09] <mwhudson> i've kind of dropped the ball here really :/
[08:09] <zyga> heh
[08:10] <pedronis> mvo: it's there an easy way to do that, use 1.14 to build snapd in a full run?
[08:10] <mup> PR snapd#8446 opened: tests/main/uc20-create-partitions-*: tweaks, renames, switch to 20.04 <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/8446>
[08:17] <mborzecki> pedronis: mvo: arch, sid and fedora 32 tests probably run with 1.14 already
[08:20] <zyga> yeah, I'm pretty sure we already test this variant for a while
[08:21] <pedronis> zyga: have we lost the PR check in the github actions? spread fails on #8446 but not github
[08:22] <mup> PR #8446: tests/main/uc20-create-partitions-*: tweaks, renames, switch to 20.04 <Simple 😃> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/8446>
[08:22] <pedronis> sorry, I mean the PR title check
[08:22] <zyga> pedronis: let me check
[08:23] <zyga> pedronis: not removed but it doens't trigger, it loads TRAVIS_PULL_REQUEST for context
[08:23] <zyga> pedronis: let me see if we can port that
[08:23] <zyga> and what else is using TRAVIS_*
[08:33] <mup> PR snapd#8447 opened: github: check pull request title <Created by zyga> <https://github.com/snapcore/snapd/pull/8447>
[08:33] <zyga> pedronis: ^^
[08:35] <zyga> oh boy
[08:35] <zyga> almost
[08:35] <zyga> maybe we should just host that as our own action
[08:35] <zyga> (it's not marked as public)
[08:36] <pedronis> zyga: also what permission does such an action get?
[08:37] <jamesh> zyga: Note that you should have access to the event payload via the file $GITHUB_EVENT_PATH
[08:37] <zyga> pedronis: it runs on the worker
[08:37] <pedronis> what permissions has the worker?
[08:37] <jamesh> put that through a json parser, and you can see pull request title, labels, etc
[08:37] <zyga> pedronis: it gets a token from github to make API requests
[08:38] <zyga> pedronis: but I don't know if it can make any write requests
[08:38] <jamesh> and it can make arbitrary changes to the workspace before the rest of the job continues
[08:38] <pedronis> I suppose there should be a way to control that, but if we don't want to dig
[08:38] <pedronis> better not use 3rd party action on our stuff
[08:38] <jamesh> and access anything left on the system by previous steps
[08:39] <zyga> yeah, we can fork and host our own copy
[08:40] <zyga> maybe that will actually make it work, I'm puzzled by the error
[08:42] <mup> PR snapd#8447 closed: github: check pull request title <Created by zyga> <Closed by zyga> <https://github.com/snapcore/snapd/pull/8447>
[08:44] <mup> PR snapd#8445 closed: tests/main/uc20-snap-recovery: unbreak, rename to uc20-create-partitions <Simple 😃> <Created by bboozzoo> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/8445>
[08:44] <mborzecki> yay
[08:45] <mvo> mborzecki: does not cherry pick cleanly, we will need a 2.44 version as a PR :/
[08:45] <mborzecki> mvo:  ok, will do
[08:47] <mup> PR snapd#8437 closed: tests/session-tool: collect information about services on startup <Created by zyga> <Merged by zyga> <https://github.com/snapcore/snapd/pull/8437>
[08:47] <jamesh> zyga: I think the release tag you picked is simply busted.  The file line 2 of https://github.com/DylanVann/check-pull-request-title/blob/v0.1.14/dist/index.js doesn't exist: it does in the v0.1.13 tag though
[08:47] <zyga> yeah
[08:47] <zyga> I looked at the forks
[08:47] <zyga> there's a fork with better code and a status check that can be enforced
[08:48] <zyga> I'll park this for now and resume in the evening, I think we need to fork it to our own org and manage it there
[08:48] <jamesh> json.load(open(os.environ["GITHUB_EVENT_PATH"], "r")) might be a better option
[08:50] <zyga> yeah
[08:50] <jamesh> that's what github.context.payload in the action is
[08:51] <mborzecki> mvo: hmm don't think we need that patch for 2.44 branch tho
[08:52] <mvo> mborzecki: aha, cool
[08:53] <mborzecki> mvo: running the test to make sure, but it looks like the test was never updated there in the first place
[08:53] <mborzecki> (i mean updated to check gadget partitions sizes)
[08:53] <mvo> mborzecki: even better, if so we can just remove the milestone
[08:53] <mvo> mborzecki: might still be worthwhile to move from 19.10->20.04 in 2.44 so maybe cherrypick the other one
[08:54] <mborzecki> yeah
[08:55] <mvo> mborzecki: I will try to cherry-pick once 8446 landed
[08:55] <mborzecki> mvo: c3019489d2 (19.10 -> 20.04) applies cleanly
[08:56] <mvo> mborzecki: nice! and cherry-picked
[09:07] <mup> PR snapd#8448 opened: tests/session-tool: add session-tool --dump <Simple 😃> <Created by zyga> <https://github.com/snapcore/snapd/pull/8448>
[10:15]  * pstolowski going to the grocery and other supplies, bb in 1h
[10:16] <ijohnson> pedronis: does this make sense now? https://github.com/snapcore/snapd/pull/8442#issuecomment-610298206
[10:16] <mup> PR #8442: tests/many: stop importing osutil from dirs, mock ProcSelfMountInfo more <Test Robustness> <⛔ Blocked> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/8442>
[10:17] <ijohnson> pedronis: does this make sense now? https://github.com/snapcore/snapd/pull/8442#issuecomment-610298206
[10:21] <pedronis> ijohnson: it still seems that there should be something simpler that can be done
[10:23] <ijohnson> pedronis: do you mean a simpler way to mock proc/self/mountinfo all the places we need to?
[10:24] <pedronis> first of all I notice that LoadMountInfo usage is a bit weird
[10:25] <pedronis> it's always used with /proc/self/mountinfo except it takes a path afaict
[10:25] <pedronis> for the benefit of its own tests
[10:25] <ijohnson> pedronis: LoadMountInfo ends up getting used in many places outside of it's own tests
[10:26] <pedronis> yes, so it's optimizing for the wrong thing
[10:26] <ijohnson> pedronis: additionally, part of the issue I found with our current tests is that during the tests of other things, it will be used but on the go process's mountinfo, instead of a mocked one
[10:27] <pedronis> ijohnson: so my suggestion is like this LoadMountInfo should take nothing or if that's annoying take "" to mean the default location, there should be no visible ProcSelfMountInfo afaict
[10:28] <pedronis> ijohnson: MockProcSelfMountInfo(path, content string) should become just MockMountInfo(content)
[10:28] <pedronis> and convince internall LoadMountInfo to use content instead of readin the file
[10:28] <ijohnson> pedronis: what is most convenient for other tests, including the ones I have not yet proposed, is that all of the bits of the codebase that in effect read /proc/self/mountinfo should actually really be reading <testdir>/proc/self/mountinfo instead of the real one
[10:29] <ijohnson> (for the tests only obviously)
[10:29] <ijohnson> this means that LoadMountInfo needs a way to know the dirs.GlobalRootDir
[10:29] <pedronis> ijohnson: see my suggestion, I don't want to even bother with files
[10:30] <pedronis> unless we have a reason to
[10:30] <pedronis> do we have a reason?
[10:30] <ijohnson> hmm ok I can do that instead I suppose
[10:31] <ijohnson> I don't think we have a reason to use files, other than it was slightly nicer to have a empty mountinfo by default when you just set RootDir to <testdir>, but now we also need to actually set an empty mountinfo too
[10:37] <pedronis> ijohnson: does it affect a lot tests? the current PR looks very churny
[10:37]  * pedronis needs to have lunch
[10:37] <ijohnson> pedronis: there are many tests that end up triggering reads to /proc/self/mountinfo
[10:37] <ijohnson> and there are also actually quite a few tests that need to mock a specific mountinfo
[10:50] <zyga> hey ijohnson :)
[10:50] <ijohnson> hey zyga
[10:51] <zyga> we should get self-hosted workers from canonistack today so we will see much faster iteration
[10:51] <ijohnson> that's great news
[11:00] <pedronis> ijohnson: we are already detecting if we are in testing for snapdUnsafeIO, so maybe we can do something based on that, possibly panic if we are in tests and proc mountinfo is not mocked
[11:01] <ijohnson> hmm good point I can look at that for inspiration
[11:02] <pedronis> ijohnson: basically var inTesting := strings.HasSuffix(os.Args[0], ".test")
[11:02] <pedronis> very blunt but simple
[11:02] <pedronis> we don't have a lot of places, but we do have a couple that do things like that
[11:05]  * ijohnson waits for many many tests to panic
[11:07]  * zyga is puzzled by something
[11:09] <ijohnson> $ go test ./... | grep PANICKED | grep -Po '[0-9]+ PANICKED' | awk '{print $1}' | paste -sd+ | bc
[11:09] <ijohnson> 207
[11:09] <ijohnson> hooray
[11:11] <pedronis> zyga: session-tool failure here:  https://github.com/snapcore/snapd/pull/8435/checks?check_run_id=567093683 on centos ? now I need to re-run all the tests?
[11:11] <mup> PR #8435: configcore,tests: fix setting watchdog options on UC18/20 <Reviewed> <Created by pedronis> <https://github.com/snapcore/snapd/pull/8435>
[11:12] <zyga> looking
[11:12] <zyga> do you recall when you merged master into this?
[11:12] <zyga> pedronis: technically yes but I think since nothing else failed, you can just ask mvo to merge
[11:13] <pedronis> that doesn't scale a lot though
[11:13] <zyga> ah, this is before the extra stop and debug patches
[11:13] <mvo> merging now
[11:13] <mvo> but yeah, does not really scale
[11:14] <zyga> in travis we had to restart 50% of the tests, which was somewhat better
[11:14] <mup> PR snapd#8435 closed: configcore,tests: fix setting watchdog options on UC18/20 <Reviewed> <Created by pedronis> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/8435>
[11:14] <zyga> I will see if there'a programmatic way to do what we want
[11:14] <zyga> maybe the GUI is missing but we can just handle that in other ways
[11:15] <zyga> one thing we could do today is to just put those into separate workflows
[11:15] <zyga> then we can restart each one
[11:15] <zyga> in any way, point taken, I'll check
[11:35] <mup> PR snapd#8443 closed: interfaces/many: miscellaneous policy updates xliv <Created by jdstrand> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/8443>
[11:35] <mvo> zyga: thank you!
[11:36] <mup> PR snapd#8444 closed: interfaces/many: miscellaneous policy updates xliv - 2.44 <Created by jdstrand> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/8444>
[11:37] <mup> PR snapd#8429 closed: github: port CLA check to Github Actions <Created by jhenstridge> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/8429>
[11:42] <ijohnson> pedronis: ok now 8442 is doing as requested
[11:50] <pedronis> ijohnson: there's a bunch of place that SetRootDir where is a bit unclear if they needed at some stage, or also now.
[11:51] <ijohnson> pedronis: probably leftover from prior commits, let me try to clean that up
[11:51] <ijohnson> pedronis: I added squash merge since this PR is going through quite a bit of churn
[11:52] <pedronis> ijohnson: it's also trying to do a quite a bit in one go, like the apparmor dirs move
[11:53] <ijohnson> well since the goalpost for this PR has moved it can probably be split up now, moving apparmor can at least be split out too
[11:53] <ijohnson> I wasn't able to split it up before since the apparmor dirs was causing an import loop
[12:03] <pedronis> ijohnson: other question, is this related to the cross-checks ? or something else?
[12:03] <ijohnson> pedronis: yes I need to mock mountinfo from the cross-checks PR
[12:04] <pedronis> ok
[12:04] <ijohnson> and I didn't want to add yet another instance of mocking it in that PR
[12:04] <ijohnson> if this is really too much work I suppose I could it just makes me feel icky inside
[12:05] <pedronis> I'm just trying to undetstand the context
[12:05] <pedronis> ijohnson: I also thought we would use findmnt or something for the cross check
[12:06] <pedronis> instead of doing using our own code
[12:06] <ijohnson> pedronis: I mean we could, but I thought it would be better to use our own mountinfo parsing code rather than rely on parsing findmnt ?
[12:06] <pedronis> ijohnson: findmnt has a json output
[12:07] <pedronis> ijohnson: I'm ok using our own code if it's obvious
[12:07] <ijohnson> ... but still if we already have code ?
[12:07] <pedronis> my memory is that using mountinfo is not that trivial
[12:07] <pedronis> but maybe I'm misremembering
[12:07] <ijohnson> it's just LoadMountInfo() and then loop over looking for MountDir == the-thing-I-care-about
[12:08] <ijohnson> and then the other place I need it will be to look for the efivars fs that is mounted to incorporate the suggestion from maciej and claudio in the efi vars code in boot pkg
[12:10] <mup> PR snapd#8449 opened: dirs: don't depend on osutil anymore, mv apparmor vars to apparmor pkg <Test Robustness> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/8449>
[12:11] <ijohnson> I broke out the apparmor stuff into this ^
[12:16] <pedronis> ijohnson: there's a bug in that one, probably we don't have test that reveal it
[12:17] <ijohnson> pedronis: you mean a bu in 8449 ?
[12:17] <ijohnson> *bug
[12:17] <pedronis> yes
[12:17] <ijohnson> hmm okay, well now it's independent so we can just leave that open and deal with later when we have more time
[12:22] <pedronis> it might fail in spread, I don't know
[12:24] <pedronis> ijohnson: it's still shows up in the other diff? am I missing something?
[12:25] <ijohnson> pedronis: do you have saved comments on the original big PR? I have broken out another PR now and rebased/rebased so it is simpler to understand the commit history
[12:25] <ijohnson> pedronis: I would prefer just to close the big one and re-open at this point if you don't have saved comments there
[12:26] <pedronis> no, just global comments
[12:26] <pedronis> afaict
[12:27] <mup> PR snapd#8450 opened: selinux: export MockIsEnforcing; systemd: use in tests <Test Robustness> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/8450>
[12:27] <ijohnson> pedronis: so I'm okay to close 8442?
[12:31] <pedronis> ijohnson: yes
[12:41] <pstolowski> pedronis: what is your idea for hooking FilesystemOnlyApply into core16/18 firstboot and preseeding flows? core config hook run twice?
[13:01] <pedronis> pstolowski: no for those we need to call it from image
[13:03] <mup> PR snapd#8442 closed: tests/many: stop importing osutil from dirs, mock ProcSelfMountInfo more <Squash-merge> <Test Robustness> <⛔ Blocked> <Created by anonymouse64> <Closed by anonymouse64> <https://github.com/snapcore/snapd/pull/8442>
[13:03] <pstolowski> pedronis: mind if i do this change?
[13:04] <pedronis> pstolowski: no, we can chat quickly about it, I'm in another meeting atm
[13:04] <pedronis> though
[13:04] <pstolowski> pedronis: ok, sure, let me know when you have a moment
[13:09] <mup> PR snapd#8446 closed: tests/main/uc20-create-partitions: tweaks, renames, switch to 20.04 <Simple 😃> <Created by bboozzoo> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/8446>
[13:45] <pstolowski> zyga: was this fixed https://bugs.launchpad.net/snapd/+bug/1870343 ?
[13:46] <mup> Bug #1870343: Unit test failure on launchpad build <snapd:New> <https://launchpad.net/bugs/1870343>
[13:46] <zyga> checking
[13:46] <zyga> I don't know, I only reported it
[13:46] <zyga> let me look deeper
[13:47] <zyga> I doubt it was fixed
[13:47] <zyga> it was a random failure but it is non the less interesting, that code is not full of timers and sleeps
[13:52] <mvo> 8325 (recover mode) should be ready for a second review
[13:53] <zyga> ack
[13:58] <jdstrand> pedronis: hey, whenever you get a chance can you comment on: https://forum.snapcraft.io/t/support-for-daemon-dbus/8855/11 ?
[14:01] <pedronis> jdstrand: yes, I'll try to get to it in the next couple of days
[14:02] <jdstrand> pedronis: I'm going to add an UPDATE comment for the move away from bus-name, so please read that
[14:02] <jdstrand> pedronis: thanks!
[14:04] <ijohnson> mvo: what about 8010? are you going to close that one? iirc that one is blocked waiting for pedronis
[14:09] <mvo> ijohnson: it's a subset of the other one, I don't mind either way
[14:10] <ijohnson> ok, I reviewed 8010 but was waiting to review 8325 until 8010 was merged, if you like I can review 8325 now anyways?
[14:15] <mvo> ijohnson: let's ask pedronis if we want to review 8010 first or juts do it all in one go in 8325
[14:17] <ijohnson> mborzeck1: could I trouble you for a review on https://github.com/snapcore/snapd/pull/8411 ?
[14:17] <mup> PR #8411: boot: cleanup more things, simplify code <UC20> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/8411>
[14:19] <ijohnson> degville: I know you worked on the hooks docs recently, have you seen this comment: https://forum.snapcraft.io/t/snap-common-looks-different-from-snap-shell/3056/5?u=ijohnson ?
[14:22] <degville> ijohnson: no, I'd not seen that comment - thanks! I totally agree too. I'll try and make the hook docs a little more intuitive..
[14:22] <ijohnson> thanks :-)
[14:36] <mup> PR snapd#8451 opened: osutil: mock proc/self/mountinfo properly everywhere <Needs Samuele review> <Test Robustness> <UC20> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/8451>
[14:37] <pedronis> mborzeck1: pstolowski: there's no way to make journald emit is current config right?  systemd has systemctl show ? but I don't think there is something for journald
[14:41] <pedronis> #8411 still needs 2nd review (I think I said it before)
[14:41] <mup> PR #8411: boot: cleanup more things, simplify code <UC20> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/8411>
[14:41] <zyga> mvo: small review for https://github.com/snapcore/snapd/pull/8325#pullrequestreview-389162078
[14:41] <mup> PR #8325: snap-bootstrap: copy auth data from real ubuntu-data in recovery mode <UC20> <Created by mvo5> <https://github.com/snapcore/snapd/pull/8325>
[14:43] <pedronis> mvo: I think closing 8010 is ok
[14:46] <pstolowski> pedronis: i couldn't spot anyting in the man. only this: "By default, the configuration file in /etc/systemd/ contains commented out entries showing the defaults as a guide to the administrator."
[14:48] <mup> PR snapcraft#3016 opened: cli: remove experimental config.yaml support <Created by cjp256> <https://github.com/snapcore/snapcraft/pull/3016>
[14:50] <mup> PR snapd#8010 closed: snap-bootstrap: add support for "recover" mode <Needs Samuele review> <UC20> <Created by mvo5> <Closed by mvo5> <https://github.com/snapcore/snapd/pull/8010>
[14:52] <zyga> ijohnson: https://github.com/snapcore/snapd/pull/8451#pullrequestreview-389190974
[14:52] <mup> PR #8451: osutil: mock proc/self/mountinfo properly everywhere <Needs Samuele review> <Test Robustness> <UC20> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/8451>
[14:54] <ijohnson> zyga: did you see that the apparmor stuff is split out into a seprate PR?
[14:54] <ijohnson> thanks for the review btw
[14:54] <zyga> I saw the PR had prerequisites yeah
[14:54] <zyga> sorry for dumping it all in one spot
[14:54] <ijohnson> zyga: re adding a callback automatically invoking it, yes that seems like a useful thing to do
[14:54] <zyga> I think it's okay, no need to change anything here IMO
[14:54] <zyga> follow ups are +1
[14:55] <zyga> I'm very happy to see this
[14:55] <ijohnson> no prob, but if you also quick +1 the smaller PR's contained in the big one that would be awesome :-)
[14:55] <zyga> mainly because we were going deeper into a world where everything had mocking helpers you had to know about
[14:55] <ijohnson> zyga: yes the more I learned yesterday the more irritated I became that we weren't doing this
[14:55] <zyga> or you would see failures in magic places
[14:55] <ijohnson> yes
[14:55] <zyga> the mock or panic idea is great
[14:55] <ijohnson> I really like Samuele's suggestion to make it panic if not mocked
[14:55] <zyga> a balance of convenience (no passing explicit state everywhere) but sound mind that you didn't miss anything
[14:55] <zyga> pedronis: really nice idea!
[14:56] <zyga> and we should adopt this for more mocking that is public
[14:56] <ijohnson> yes
[14:56] <zyga> it's just almost always wrong unless you are super careful
[14:56] <zyga> but even if you are, it may break tomorrow
[14:56] <zyga> so this is great
[14:56] <zyga> I'll look at the prereqs now
[14:56] <zyga> and taking pressure off dirs is great
[14:56] <ijohnson> I've also thought about making exported Mock*() functions panic when run not in tests
[14:56] <zyga> it's been such a dumpster of things
[14:56] <ijohnson> dirs has gotten really big
[14:56] <zyga> even if we get some duplication it's much easier to reason about it
[14:57] <zyga> ijohnson: yeah, so they fail closed, not open
[14:57] <zyga> +1
[15:11] <pedronis> pstolowski: I reviewed #8414
[15:11] <mup> PR #8414: o/configstate: core config handler for persistent journal <Needs Samuele review> <Created by stolowski> <https://github.com/snapcore/snapd/pull/8414>
[15:11] <pstolowski> pedronis: thank you, i like the marker idea
[15:13] <pedronis> pstolowski: it's a bit tricky to implement atomatically, we probable need to use a rename somehow
[15:13] <pedronis> heh, atomically
[15:14] <pedronis> pstolowski: because the naive way we might end up we directory that we made but cannot remove
[15:14] <pedronis> s/we directory/with a directory/
[15:16] <zyga> pstolowski: I commented on that as well
[15:16] <pstolowski> ty
[15:16] <zyga> ah, wrong part
[15:16] <zyga> sorry, let me revise
[15:16] <zyga> commenting on the conversation page vs the diff
[15:17] <pedronis> bit confused by that suggestion, we do want to remove that dir even if it's full
[15:17] <pedronis> if it was created by us
[15:18] <zyga> yes, I revised my comment
[15:20] <pedronis> ijohnson: about adding the callback calling it, it seems interesting, that the current code is much more obvious, we would need a good name for the dirs helper and I don't have one to propose
[15:20] <pedronis> s/that the/but the/
[15:22] <ijohnson> pedronis sure I can just add a TODO there or something, not right now is totally fine
[15:35] <pedronis> pstolowski: do you want to chat now about image and FsOnlyApply ? or tomorrow?
[15:36] <pstolowski> pedronis: yes, today is fine
[15:36] <pedronis> pstolowski: let's do it now then
[15:36] <pstolowski> ok
[15:43] <zyga> ijohnson: I think that's all of them
[15:43] <ijohnson> thank you zyga !
[15:46] <mborzeck1> ompf, finally done with https://github.com/CanonicalLtd/subiquity/pull/692
[15:46] <mup> PR CanonicalLtd/subiquity#692: console_conf: various recover chooser tweaks <Created by bboozzoo> <https://github.com/CanonicalLtd/subiquity/pull/692>
[15:46] <mborzeck1> degville: can you take a look at the wording of the user facing messages in the PR ^^
[15:46] <zyga> mborzeck1: are you pythonic now? ;D
[15:47] <mborzeck1> zyga: hopefully it's just temporary
[15:47] <zyga> mborzeck1: I'm __sure__ it is
[15:47] <zyga> mborzeck1: what's up with 1?
[15:47] <zyga> network/power
[15:47] <zyga> ?
[15:47] <mborzecki> probably network
[15:49] <degville> mborzecki: yep, of course. thanks!
[15:50] <mborzecki> degville: thanks!
[15:57] <the-mentor> hellow
[15:57] <the-mentor> I wonder if anyone can help me with a snap issue
[15:57] <the-mentor> https://forum.snapcraft.io/t/vulkan-is-broken-on-snaps-when-using-nvidia-proprietary-drivers/16378
[15:58] <the-mentor> i opened this thread to try and help debug an issue with snaps and vulkan with nvidia gpus
[16:18] <zyga> the-mentor: hey
[16:18] <zyga> I can look tomorrow as I'm about to EOD
[16:18] <zyga> I have the hardware and the means to check this
[16:18] <zyga> but I suspect it may require more love to fix unless it's something trivial
[16:20] <zyga> the-mentor: our nvidia support is not robust and needs changes :/
[16:27]  * zyga EODs
[16:42] <mvo> zyga: back now, looking at the queue now
[16:45] <zyga> mvo: ta
[16:45] <zyga> mvo: I'm in the office but not actively working now
[16:47] <mup> PR snapd#8441 closed: interfaces: add case for rootWritableOverlay + NFS <Simple 😃> <Test Robustness> <Created by anonymouse64> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/8441>
[16:47] <ijohnson> zyga: do you know if there are maybe some spots in the queue taken up by just _branches_ that are pushed?
[16:48] <ijohnson> I.e. I push a branch that I don't have an open PR for, will the tests run on that?
[16:48] <zyga> ijohnson: IIRC no, only pull requests should be considered
[16:48] <ijohnson> okay, cool just a bit odd some of the emails I got as the links they give you are just referencing a branch and not a PR, and the PR that was open for the branch was closed like 4-5 hours ago
[16:49] <mup> PR snapd#8408 closed: snap/naming: add validator for snap security tag <Skip spread> <Created by zyga> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/8408>
[16:49] <zyga> it's going through a backlog
[16:49] <zyga> tests for past commits to an open PR will be tested
[16:49] <ijohnson> even if the PR is closed or merged it will keep running those?
[16:50] <zyga> yes, actions is more of a "bring your own" system
[16:50] <zyga> there's a way to encode the policy
[16:50] <zyga> but we didn't do any yet
[16:50] <zyga> there's a way to close those automatically
[16:50] <zyga> we're just discovering that
[16:50] <zyga> I made a remark about this in the internal channel
[16:50] <zyga> ijohnson: though part of it is really the relative immaturity of the product
[16:51] <ijohnson> ah I see it now
[16:51] <mup> PR snapd#8433 closed: overlord/devicestate: support for recover and run modes <UC20> <Created by bboozzoo> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/8433>
[16:51] <mvo> cmatsuoka: do you think you could review 8439? afaict it needs a second review
[16:52] <mvo> zyga, ijohnson if it really tests all commits even if they are no longer "tip" I'm not sure we should continue using GH actions until this is fixed
[16:53] <mvo> i.e. if there is no cancel feature yet
[16:53] <zyga> mvo: there's a way to do that, perhaps I should integrate that ASAP
[16:53] <ijohnson> iiuc zyga said there is, we just have to opt-in to it somehow
[16:53] <zyga> yeah
[16:54] <mvo> ijohnson, zyga ok, that sounds ok then, is it much work?
[16:55] <zyga> let me check
[16:55] <zyga> mvo: can you look at https://github.com/styfle/cancel-workflow-action
[16:55] <zyga> it has to be someone who has repo access
[16:56] <zyga> there are two things that need to be done there
[16:56] <zyga> if this works we can fork that repo and run that action from snapcore/cancel-workflow-action
[16:56] <zyga> so that we don't use 3rd party actions
[17:02] <cmatsuoka> mvo: checking that...
[17:03] <mvo> ta
[17:03] <zyga> ijohnson, mvo: opened 8452
[17:03] <mup> PR snapd#8452 opened: github: cancel previous builds <Created by zyga> <https://github.com/snapcore/snapd/pull/8452>
[17:04] <zyga> this is also interesting: https://github.com/snapcore/snapd/actions?query=is%3Aqueued
[17:04] <ijohnson> interesting you can manually cancel jobs too
[17:04] <cmatsuoka> mvo: done
[17:04] <zyga> ironically this has more visibility than travis before
[17:05] <zyga> we see exactly what's queued and what's running
[17:05] <ijohnson> yes this is much more useful imho
[17:05] <zyga> as the queue drains I'd like to consider merging https://github.com/snapcore/snapd/pull/8440
[17:06] <zyga> we could really ease the pain then
[17:06] <mup> PR #8440: github: move spread to self-hosted workers <Created by zyga> <https://github.com/snapcore/snapd/pull/8440>
[17:06] <zyga> and if cancel works that might be enough
[17:06] <zyga> if this doesn't work we could go back to travis :/
[17:06] <zyga> the goal was to improve
[17:06] <zyga> not disrupt everything without improvement
[17:07] <ijohnson> personally I think we're really close to having a much better workflow than we had on travis
[17:07] <ijohnson> I think to go back to travis would be very sad now
[17:07] <ijohnson> (apologies to anyone named traivs)
[17:07] <zyga> it would only be better if we could re-oreder the queue
[17:08] <ijohnson> that would be TOO good
[17:09] <zyga> https://github.com/snapcore/snapd/actions?query=workflow%3ATests+actor%3Azyga <- stuff I'm waiting on
[17:09] <zyga> that's actually cool, i didn't notice this before
[17:10] <zyga> or maybe "blocked" on
[17:10] <zyga> as those are queued
[17:10] <ijohnson> haha I literally was looking at that view for me too
[17:10] <ijohnson> there were a few jobs from like 2 weeks ago that stuck somehow so I just canceled them now
[17:10] <ijohnson> seems to work good!
[17:10] <zyga> I have a feeling that maybe we did jump prematurely
[17:10] <zyga> but apart from the initial pain it's the better place to be
[17:10] <zyga> I suspect way more innovation is going into actions than travis today
[17:10] <zyga> (sorry travis, we really love what you gave the world)
[17:22] <zyga> mvo: did you set the secret?
[17:45] <zyga> ha
[17:45] <zyga> so instead of soldering that power on switch
[17:45] <zyga> I just enabled wake-on-lan
[17:45] <zyga> :D
[17:45] <mvo> zyga: I did
[17:45] <zyga> hmm, ok,
[17:45] <zyga> I'll check once the queue clear
[17:45] <zyga> thanks!
[17:46] <zyga> maybe a typo
[17:46] <zyga> it failed with missing argument
[17:46] <zyga> as if what we provided expanded to nothing
[17:48] <mvo> zyga: definitely  GH_ACCESS_TOKEN there
[17:48] <zyga> it could be a non-starter with a PR
[17:48] <zyga> PR from a fork
[17:48] <zyga> :/
[17:48] <mvo> zyga: 8452 canceled the unit test it seems
[17:49] <mvo> zyga: oh no
[17:49] <zyga> I cancelled
[17:49] <mvo> zyga: so this whole cancel thing will not work?
[17:49] <zyga> not sure which PR is that
[17:49] <zyga> I don't know yet
[17:49] <mvo> zyga: the cancel pr
[17:49] <zyga> ah, yeah, I cancelled a few things
[17:49] <zyga> (my own)
[17:49] <zyga> to explore what to do without wasting cycles
[17:49] <mvo> zyga: pretty sure it won't work if the workflow is part of a fork
[17:50] <mvo> zyga: as there won't be a token then, that's a bummer
[18:27] <mup> PR snapd#8453 opened: [RFC] travis.yml: re-enable travis <Created by mvo5> <https://github.com/snapcore/snapd/pull/8453>
[18:33] <mup> PR snapcraft#3016 closed: cli: remove experimental config.yaml support <Created by cjp256> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/3016>
[18:45] <mup> PR snapd#8452 closed: github: cancel previous builds <Created by zyga> <Closed by zyga> <https://github.com/snapcore/snapd/pull/8452>
[18:52] <zyga> store is wonky now
[19:01] <zyga> noise][: the store is indeed wonky now
[19:01] <zyga> https://www.irccloud.com/pastebin/vQoYjUDu/
[19:06] <jdstrand> kenvandine: hey, I just noticed these denials in snap-store while looking at an unrelated bug: https://paste.ubuntu.com/p/vhGRmC8hyK/
[19:06] <jdstrand> kenvandine: expected?
[19:07] <jdstrand> kenvandine: a bit surprised it is looking at mime stuff in hostfs
[19:08] <kenvandine> jdstrand:  which channel?
[19:08] <cmatsuoka> zyga: is there a way to ask github to re-execute a test?
[19:08] <zyga> cmatsuoka: yes
[19:09] <zyga> cmatsuoka: click on checks
[19:09] <zyga> cmatsuoka: then on the button on the right
[19:09] <zyga> (starting from a PR page)
[19:09] <jdstrand> kenvandine: it was in an unrelated bug report that just showed dmesg as an attachment. I don't have any other details
[19:10] <cmatsuoka> zyga: the cancel workflow button?
[19:20] <diddledan> jdstrand, kenvandine, I confirm that it is looking in those paths on my system, too.. also denied.
[19:22] <diddledan> snappy-debug log: https://paste.ubuntu.com/p/h9pbYnWvs5/
[19:22] <kenvandine> jdstrand, diddledan: it might be because of https://gitlab.gnome.org/Community/Ubuntu/gnome-software/-/blob/snap-store-3-36/plugins/core/gs-plugin-appstream.c#L519
[19:22] <kenvandine> since the core snap has a /usr/share/applications we have to look via hostfs
[19:22] <kenvandine> to find the installed desktop files
[19:23] <kenvandine> i wonder if that load_desktop triggers something that tries to find the mime stuff
[19:24] <diddledan> the two flatpak paths in my log are interesting, too
[19:24] <kenvandine> that's related to XDG_DATA_DIRS
[19:24] <kenvandine> probably
[19:24] <kenvandine> flatpak modifies those
[19:26] <diddledan> aah possibly because I've installed a flatpak then
[19:28] <kenvandine> diddledan: yeah
[19:28] <kenvandine> it'll do that
[19:39] <zyga> cmatsuoka: there's a restart / cancel
[19:39] <zyga> cmatsuoka: if it says cancel then it's still running
[19:40]  * zyga is gone
[19:46] <mup> PR snapcraft#3017 opened: pluginhandler: deterministic load depending on plugin and build-base <Created by sergiusens> <https://github.com/snapcore/snapcraft/pull/3017>
[19:48] <zyga> cmatsuoka: if around: https://github.com/snapcore/snapd/pull/8454
[19:48] <mup> PR #8454: tests/session-tool: session ordering is non-deterministic <Created by zyga> <https://github.com/snapcore/snapd/pull/8454>
[19:48] <mup> PR snapd#8454 opened: tests/session-tool: session ordering is non-deterministic <Created by zyga> <https://github.com/snapcore/snapd/pull/8454>
[19:48] <zyga> session-tool is my worst thing ever
[19:48] <zyga> like I want to strangle myself now
[19:49] <cmatsuoka> ick
[19:50] <zyga> ta
[19:51] <cmatsuoka> zyga: so yeah, my PRs were stuck in a point where only the cancel button was available and it didn't do much, but I had to merge master anyway
[19:51] <zyga> one thing that I find super confusin
[19:51] <zyga> *confusing
[19:51] <zyga> is that when you restart tests
[19:52] <zyga> what happens is that you get unit tests
[19:52] <zyga> but rest is still as-it-was before
[19:52] <zyga> then once those run
[19:52] <zyga> you get canary
[19:52] <zyga> then rest
[19:52] <zyga> so it's a wave
[19:52] <zyga> not an immediate invalidation
[19:55] <mup> PR snapcraft#3018 opened: Add prebuilt-wheel-dir to python plugin <Created by jimmylomro> <https://github.com/snapcore/snapcraft/pull/3018>
[20:01] <mup> PR snapcraft#3019 opened: static: consolidate tooling setup to setup.cfg <Created by sergiusens> <https://github.com/snapcore/snapcraft/pull/3019>
[20:01] <mup> PR snapcraft#3020 opened: spread tests: default base for local plugin tests <Created by sergiusens> <https://github.com/snapcore/snapcraft/pull/3020>
[20:39] <stgraber> zyga: does "snap run" require a running snapd?
[20:40] <stgraber> zyga: we've been trying to catch weird shutdown issues for a while now, where the system effectively gets stuck for 10min, until systemd gives up and kills everything
[20:40] <stgraber> zyga: our own shutdown script has logic that should timeout after 9min so this has never made much sense
[20:40] <stgraber> zyga: until I finally managed to keep a shell on a system where this is happening
[20:41] <stgraber> zyga: that's what I'm seeing there: https://paste.ubuntu.com/p/jFdb9sTghF/
[20:47] <stgraber> zyga: removing the snapd socket and manually starting snapd back up seems to fix the situation
[20:47] <stgraber> in that the stop operation then unblocks, succeeds and systemd shuts down the system
[21:29] <zyga> stgraber: tired, just one sentence - yes after some operations like kernel upgrade when system key indicates snapd needs to create new sandbox profiles
[21:29] <zyga> Snap run detects this and pauses
[21:30] <stgraber> probably should not do that on a stop operation though when snapd is already gone
[21:31] <mup> PR snapcraft#2784 closed: [multipass] use stdio to get data in/out of Multipass <Created by Saviq> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/2784>
[21:32] <Saviq> ChrisTownsend: ^ FYI
[22:07] <mup> PR snapcraft#3021 opened: remote-build: remove artifact sanity check <Created by cjp256> <https://github.com/snapcore/snapcraft/pull/3021>
[22:31] <mup> PR snapcraft#3017 closed: pluginhandler: deterministic load depending on plugin and build-base <Created by sergiusens> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/3017>