mupPR snapcraft#3014 closed: meta: migrate get_build_base to Snap <Created by sergiusens> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/3014>00:16
mvohey mborzecki05:53
mborzeckimvo: hey hey05:53
mborzeckibrb, new kernel06:01
mborzeckiand back06:04
mupPR 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
mvomborzecki: welcome back!06:07
zygaSleepy a bit06:15
mborzeckizyga: hey06:15
mborzeckimvo: the spread job on 19.10 didn't run here https://github.com/snapcore/snapd/pull/8443 but all the other jobs are green06:22
mupPR #8443: interfaces/many: miscellaneous policy updates xliv <Created by jdstrand> <https://github.com/snapcore/snapd/pull/8443>06:22
mborzeckimvo: 2.44 port of that PR is green, so we could probably merge 844306:22
jameshI also had a CI run pass on everything but 19.10 today: https://github.com/snapcore/snapd/pull/835606:25
mupPR #8356: cmd/snap: Implement a "snap routine file-access" command <Created by jhenstridge> <https://github.com/snapcore/snapd/pull/8356>06:25
pedronismvo: hi, uc20-snap-recovery  has a failure, is searching for a 3G partition size but now we have 1G again06:27
mborzeckiso that 19.10 failure pops up in a number of PRs06:30
pedronismvo: also should we move those main/uc20-* tests to 20.04 ? (they are running on 19.10 atm)06:30
mborzeckipedronis: do you have a log from that failure in uc20-snap-recovery?06:31
pedronismborzecki: yes, but I would expect anything to fail with it at this point06:32
pedronismborzecki: https://github.com/snapcore/snapd/pull/8435/checks?check_run_id=56588416506:33
mupPR #8435: configcore,tests: fix setting watchdog options on UC18/20 <Reviewed> <Created by pedronis> <https://github.com/snapcore/snapd/pull/8435>06:33
mvopedronis: 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
mborzeckipedronis: thanks, looking06:33
mupPR 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
mvopedronis: totally +1 for moving those from 19.10->20.0406:33
mvomborzecki: are you on this? that's great \o/06:34
pedronismvo: it landed06:34
pedronismborzecki: 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/loop206:35
pedronisthat's what's failing afaict06:35
* mvo onds06:37
* mvo nods06:37
mborzeckimvo: pedronis: isn't that because of https://github.com/snapcore/pc-amd64-gadget/commit/a0eb11c6f6945d9395a858aaf396f3f3c0c609cf#diff-3da8b5c9ab650c29257cf39fccfa8be7 ?06:39
mvomborzecki: I think so06:39
mvomborzecki: 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
mborzeckimvo: ack06:40
mborzeckihmm that test should probably also check that the system-data was automatically expanded before it proceeeds to add that 110MB chunk in the next scenario06:43
pedronismborzecki: maybe two PRs ?  one to fix it and one to change/improve/move things?06:49
pedronisthey should also be called uc20-create-partions-* at this point06:50
* pedronis will not ask for a pony as well06:52
* mvo lols and wants a pony too06:55
mvogood morning pstolowski07:01
pedronismvo: did a new pass on #843907:06
mupPR #8439: secboot: import secboot on ubuntu, provide dummy on !ubuntu <UC20> <Created by mvo5> <https://github.com/snapcore/snapd/pull/8439>07:06
mupPR 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
mborzeckimvo: pedronis: this should do it ^^07:09
zygamborzecki: can you review https://github.com/snapcore/snapd/pull/843707:10
mupPR #8437: tests/session-tool: collect information about services on startup <Created by zyga> <https://github.com/snapcore/snapd/pull/8437>07:10
zygait's just extra debug in case that service shows up again07:10
pedronismborzecki: thx, there's another couple of uc20-snap-recovery-* tests that need the renaming, ->20.04 change07:10
pedronismborzecki: maybe in the follow up?07:10
mborzeckipedronis: yeah, i'll add the tweaks to check the auto grow there too07:11
pedronisthank you07:11
pedroniszyga: pstolowski: hi, #8390 needs a 2nd/re-review07:13
mupPR #8390: state: add state.CopyState() helper <UC20> <Created by mvo5> <https://github.com/snapcore/snapd/pull/8390>07:13
zygait's interesting to look at test durations below07:14
zygasome are >> 50minutes07:14
jameshhttps://github.com/snapcore/snapd/actions/runs/72216991 shows a run that was killed by the hard 5 hour timeout07:16
mvothanks mborzecki !07:28
zygamvo: https://github.com/snapcore/snapd/pull/8390#pullrequestreview-388854965 is good to merge07:34
mupPR #8390: state: add state.CopyState() helper <UC20> <Created by mvo5> <https://github.com/snapcore/snapd/pull/8390>07:34
zygamvo: I think it's okay to override the test that failed due to session-tool07:34
pedronismborzecki: I made some comments in #843307:34
mupPR #8433: overlord/devicestate: support for recover and run modes <UC20> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/8433>07:34
mvozyga: thank you07:35
mupPR snapd#8390 closed: state: add state.CopyState() helper <UC20> <Created by mvo5> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/8390>07:36
pedronis#8411 needs 2nd reviews07:38
mupPR #8411: boot: cleanup more things, simplify code <UC20> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/8411>07:38
* zyga looks at 841107:38
pedroniszyga: are you familiar with bootstate20.go ? otherwise you probably should read it first07:41
zygayeah, I need to07:41
zygathere's loads of new things in core 20 for me07:41
zygapedronis: what is modeenv?07:44
pedroniszyga: 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 use07:53
mwhudsonmvo: hey would snapd be ok with having go 1.14 as default in focal?08:04
zygamwhudson: we test with master go08:05
zygamwhudson: so I suspect so08:05
mvomwhudson: pretty sure yes08:07
mvomwhudson: like zyga said, we do run all our unit tests on go-master already08:07
mvomwhudson: might be worthwhile to run a full spread run with 1.14 though, when do you want to change the default?08:08
mwhudsonzyga, mvo: thanks08:08
mwhudsonmvo: well about 3 weeks ago i guess08:08
mwhudsonmvo: before release08:08
mvomwhudson: *cough* sorry, I guess that was a dumb question08:08
mwhudsoni've kind of dropped the ball here really :/08:09
pedronismvo: it's there an easy way to do that, use 1.14 to build snapd in a full run?08:10
mupPR 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:10
=== doko_ is now known as doko
mborzeckipedronis: mvo: arch, sid and fedora 32 tests probably run with 1.14 already08:17
zygayeah, I'm pretty sure we already test this variant for a while08:20
pedroniszyga: have we lost the PR check in the github actions? spread fails on #8446 but not github08:21
mupPR #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
pedronissorry, I mean the PR title check08:22
zygapedronis: let me check08:22
zygapedronis: not removed but it doens't trigger, it loads TRAVIS_PULL_REQUEST for context08:23
zygapedronis: let me see if we can port that08:23
zygaand what else is using TRAVIS_*08:23
mupPR snapd#8447 opened: github: check pull request title <Created by zyga> <https://github.com/snapcore/snapd/pull/8447>08:33
zygapedronis: ^^08:33
zygaoh boy08:35
zygamaybe we should just host that as our own action08:35
zyga(it's not marked as public)08:35
pedroniszyga: also what permission does such an action get?08:36
jameshzyga: Note that you should have access to the event payload via the file $GITHUB_EVENT_PATH08:37
zygapedronis: it runs on the worker08:37
pedroniswhat permissions has the worker?08:37
jameshput that through a json parser, and you can see pull request title, labels, etc08:37
zygapedronis: it gets a token from github to make API requests08:37
zygapedronis: but I don't know if it can make any write requests08:38
jameshand it can make arbitrary changes to the workspace before the rest of the job continues08:38
pedronisI suppose there should be a way to control that, but if we don't want to dig08:38
pedronisbetter not use 3rd party action on our stuff08:38
jameshand access anything left on the system by previous steps08:38
zygayeah, we can fork and host our own copy08:39
zygamaybe that will actually make it work, I'm puzzled by the error08:40
mupPR snapd#8447 closed: github: check pull request title <Created by zyga> <Closed by zyga> <https://github.com/snapcore/snapd/pull/8447>08:42
mupPR 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
mvomborzecki: does not cherry pick cleanly, we will need a 2.44 version as a PR :/08:45
mborzeckimvo:  ok, will do08:45
mupPR 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
jameshzyga: 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 though08:47
zygaI looked at the forks08:47
zygathere's a fork with better code and a status check that can be enforced08:47
zygaI'll park this for now and resume in the evening, I think we need to fork it to our own org and manage it there08:48
jameshjson.load(open(os.environ["GITHUB_EVENT_PATH"], "r")) might be a better option08:48
jameshthat's what github.context.payload in the action is08:50
mborzeckimvo: hmm don't think we need that patch for 2.44 branch tho08:51
mvomborzecki: aha, cool08:52
mborzeckimvo: running the test to make sure, but it looks like the test was never updated there in the first place08:53
mborzecki(i mean updated to check gadget partitions sizes)08:53
mvomborzecki: even better, if so we can just remove the milestone08:53
mvomborzecki: might still be worthwhile to move from 19.10->20.04 in 2.44 so maybe cherrypick the other one08:53
mvomborzecki: I will try to cherry-pick once 8446 landed08:55
mborzeckimvo: c3019489d2 (19.10 -> 20.04) applies cleanly08:55
mvomborzecki: nice! and cherry-picked08:56
mupPR snapd#8448 opened: tests/session-tool: add session-tool --dump <Simple 😃> <Created by zyga> <https://github.com/snapcore/snapd/pull/8448>09:07
=== Wouter01009 is now known as Wouter0100
=== Wouter01009 is now known as Wouter0100
* pstolowski going to the grocery and other supplies, bb in 1h10:15
ijohnsonpedronis: does this make sense now? https://github.com/snapcore/snapd/pull/8442#issuecomment-61029820610:16
mupPR #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:16
ijohnsonpedronis: does this make sense now? https://github.com/snapcore/snapd/pull/8442#issuecomment-61029820610:17
pedronisijohnson: it still seems that there should be something simpler that can be done10:21
ijohnsonpedronis: do you mean a simpler way to mock proc/self/mountinfo all the places we need to?10:23
pedronisfirst of all I notice that LoadMountInfo usage is a bit weird10:24
pedronisit's always used with /proc/self/mountinfo except it takes a path afaict10:25
pedronisfor the benefit of its own tests10:25
ijohnsonpedronis: LoadMountInfo ends up getting used in many places outside of it's own tests10:25
pedronisyes, so it's optimizing for the wrong thing10:26
ijohnsonpedronis: 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 one10:26
pedronisijohnson: 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 afaict10:27
pedronisijohnson: MockProcSelfMountInfo(path, content string) should become just MockMountInfo(content)10:28
pedronisand convince internall LoadMountInfo to use content instead of readin the file10:28
ijohnsonpedronis: 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 one10:28
ijohnson(for the tests only obviously)10:29
ijohnsonthis means that LoadMountInfo needs a way to know the dirs.GlobalRootDir10:29
pedronisijohnson: see my suggestion, I don't want to even bother with files10:29
pedronisunless we have a reason to10:30
pedronisdo we have a reason?10:30
ijohnsonhmm ok I can do that instead I suppose10:30
ijohnsonI 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 too10:31
pedronisijohnson: does it affect a lot tests? the current PR looks very churny10:37
* pedronis needs to have lunch10:37
ijohnsonpedronis: there are many tests that end up triggering reads to /proc/self/mountinfo10:37
ijohnsonand there are also actually quite a few tests that need to mock a specific mountinfo10:37
zygahey ijohnson :)10:50
ijohnsonhey zyga10:50
zygawe should get self-hosted workers from canonistack today so we will see much faster iteration10:51
ijohnsonthat's great news10:51
pedronisijohnson: 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 mocked11:00
ijohnsonhmm good point I can look at that for inspiration11:01
pedronisijohnson: basically var inTesting := strings.HasSuffix(os.Args[0], ".test")11:02
pedronisvery blunt but simple11:02
pedroniswe don't have a lot of places, but we do have a couple that do things like that11:02
* ijohnson waits for many many tests to panic11:05
* zyga is puzzled by something11:07
ijohnson$ go test ./... | grep PANICKED | grep -Po '[0-9]+ PANICKED' | awk '{print $1}' | paste -sd+ | bc11:09
pedroniszyga: 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
mupPR #8435: configcore,tests: fix setting watchdog options on UC18/20 <Reviewed> <Created by pedronis> <https://github.com/snapcore/snapd/pull/8435>11:11
zygado you recall when you merged master into this?11:12
zygapedronis: technically yes but I think since nothing else failed, you can just ask mvo to merge11:12
pedronisthat doesn't scale a lot though11:13
zygaah, this is before the extra stop and debug patches11:13
mvomerging now11:13
mvobut yeah, does not really scale11:13
zygain travis we had to restart 50% of the tests, which was somewhat better11:14
mupPR 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
zygaI will see if there'a programmatic way to do what we want11:14
zygamaybe the GUI is missing but we can just handle that in other ways11:14
zygaone thing we could do today is to just put those into separate workflows11:15
zygathen we can restart each one11:15
zygain any way, point taken, I'll check11:15
mupPR snapd#8443 closed: interfaces/many: miscellaneous policy updates xliv <Created by jdstrand> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/8443>11:35
mvozyga: thank you!11:35
mupPR 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:36
mupPR snapd#8429 closed: github: port CLA check to Github Actions <Created by jhenstridge> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/8429>11:37
ijohnsonpedronis: ok now 8442 is doing as requested11:42
pedronisijohnson: there's a bunch of place that SetRootDir where is a bit unclear if they needed at some stage, or also now.11:50
ijohnsonpedronis: probably leftover from prior commits, let me try to clean that up11:51
ijohnsonpedronis: I added squash merge since this PR is going through quite a bit of churn11:51
pedronisijohnson: it's also trying to do a quite a bit in one go, like the apparmor dirs move11:52
ijohnsonwell since the goalpost for this PR has moved it can probably be split up now, moving apparmor can at least be split out too11:53
ijohnsonI wasn't able to split it up before since the apparmor dirs was causing an import loop11:53
pedronisijohnson: other question, is this related to the cross-checks ? or something else?12:03
ijohnsonpedronis: yes I need to mock mountinfo from the cross-checks PR12:03
ijohnsonand I didn't want to add yet another instance of mocking it in that PR12:04
ijohnsonif this is really too much work I suppose I could it just makes me feel icky inside12:04
pedronisI'm just trying to undetstand the context12:05
pedronisijohnson: I also thought we would use findmnt or something for the cross check12:05
pedronisinstead of doing using our own code12:06
ijohnsonpedronis: 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
pedronisijohnson: findmnt has a json output12:06
pedronisijohnson: I'm ok using our own code if it's obvious12:07
ijohnson... but still if we already have code ?12:07
pedronismy memory is that using mountinfo is not that trivial12:07
pedronisbut maybe I'm misremembering12:07
ijohnsonit's just LoadMountInfo() and then loop over looking for MountDir == the-thing-I-care-about12:07
ijohnsonand 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 pkg12:08
mupPR 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:10
ijohnsonI broke out the apparmor stuff into this ^12:11
pedronisijohnson: there's a bug in that one, probably we don't have test that reveal it12:16
ijohnsonpedronis: you mean a bu in 8449 ?12:17
ijohnsonhmm okay, well now it's independent so we can just leave that open and deal with later when we have more time12:17
pedronisit might fail in spread, I don't know12:22
pedronisijohnson: it's still shows up in the other diff? am I missing something?12:24
ijohnsonpedronis: 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 history12:25
ijohnsonpedronis: I would prefer just to close the big one and re-open at this point if you don't have saved comments there12:25
pedronisno, just global comments12:26
mupPR snapd#8450 opened: selinux: export MockIsEnforcing; systemd: use in tests <Test Robustness> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/8450>12:27
ijohnsonpedronis: so I'm okay to close 8442?12:27
pedronisijohnson: yes12:31
pstolowskipedronis: what is your idea for hooking FilesystemOnlyApply into core16/18 firstboot and preseeding flows? core config hook run twice?12:41
pedronispstolowski: no for those we need to call it from image13:01
mupPR 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
pstolowskipedronis: mind if i do this change?13:03
pedronispstolowski: no, we can chat quickly about it, I'm in another meeting atm13:04
pstolowskipedronis: ok, sure, let me know when you have a moment13:04
mupPR 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:09
pstolowskizyga: was this fixed https://bugs.launchpad.net/snapd/+bug/1870343 ?13:45
mupBug #1870343: Unit test failure on launchpad build <snapd:New> <https://launchpad.net/bugs/1870343>13:46
zygaI don't know, I only reported it13:46
zygalet me look deeper13:46
zygaI doubt it was fixed13:47
zygait was a random failure but it is non the less interesting, that code is not full of timers and sleeps13:47
mvo8325 (recover mode) should be ready for a second review13:52
jdstrandpedronis: hey, whenever you get a chance can you comment on: https://forum.snapcraft.io/t/support-for-daemon-dbus/8855/11 ?13:58
pedronisjdstrand: yes, I'll try to get to it in the next couple of days14:01
jdstrandpedronis: I'm going to add an UPDATE comment for the move away from bus-name, so please read that14:02
jdstrandpedronis: thanks!14:02
ijohnsonmvo: what about 8010? are you going to close that one? iirc that one is blocked waiting for pedronis14:04
mvoijohnson: it's a subset of the other one, I don't mind either way14:09
ijohnsonok, I reviewed 8010 but was waiting to review 8325 until 8010 was merged, if you like I can review 8325 now anyways?14:10
mvoijohnson: let's ask pedronis if we want to review 8010 first or juts do it all in one go in 832514:15
ijohnsonmborzeck1: could I trouble you for a review on https://github.com/snapcore/snapd/pull/8411 ?14:17
mupPR #8411: boot: cleanup more things, simplify code <UC20> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/8411>14:17
ijohnsondegville: 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:19
degvilleijohnson: 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
ijohnsonthanks :-)14:22
mupPR 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:36
pedronismborzeck1: 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 journald14:37
pedronis#8411 still needs 2nd review (I think I said it before)14:41
mupPR #8411: boot: cleanup more things, simplify code <UC20> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/8411>14:41
zygamvo: small review for https://github.com/snapcore/snapd/pull/8325#pullrequestreview-38916207814:41
mupPR #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:41
pedronismvo: I think closing 8010 is ok14:43
pstolowskipedronis: 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:46
mupPR snapcraft#3016 opened: cli: remove experimental config.yaml support <Created by cjp256> <https://github.com/snapcore/snapcraft/pull/3016>14:48
mupPR 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:50
zygaijohnson: https://github.com/snapcore/snapd/pull/8451#pullrequestreview-38919097414:52
mupPR #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:52
ijohnsonzyga: did you see that the apparmor stuff is split out into a seprate PR?14:54
ijohnsonthanks for the review btw14:54
zygaI saw the PR had prerequisites yeah14:54
zygasorry for dumping it all in one spot14:54
ijohnsonzyga: re adding a callback automatically invoking it, yes that seems like a useful thing to do14:54
zygaI think it's okay, no need to change anything here IMO14:54
zygafollow ups are +114:54
zygaI'm very happy to see this14:55
ijohnsonno prob, but if you also quick +1 the smaller PR's contained in the big one that would be awesome :-)14:55
zygamainly because we were going deeper into a world where everything had mocking helpers you had to know about14:55
ijohnsonzyga: yes the more I learned yesterday the more irritated I became that we weren't doing this14:55
zygaor you would see failures in magic places14:55
zygathe mock or panic idea is great14:55
ijohnsonI really like Samuele's suggestion to make it panic if not mocked14:55
zygaa balance of convenience (no passing explicit state everywhere) but sound mind that you didn't miss anything14:55
zygapedronis: really nice idea!14:55
zygaand we should adopt this for more mocking that is public14:56
zygait's just almost always wrong unless you are super careful14:56
zygabut even if you are, it may break tomorrow14:56
zygaso this is great14:56
zygaI'll look at the prereqs now14:56
zygaand taking pressure off dirs is great14:56
ijohnsonI've also thought about making exported Mock*() functions panic when run not in tests14:56
zygait's been such a dumpster of things14:56
ijohnsondirs has gotten really big14:56
zygaeven if we get some duplication it's much easier to reason about it14:56
zygaijohnson: yeah, so they fail closed, not open14:57
pedronispstolowski: I reviewed #841415:11
mupPR #8414: o/configstate: core config handler for persistent journal <Needs Samuele review> <Created by stolowski> <https://github.com/snapcore/snapd/pull/8414>15:11
pstolowskipedronis: thank you, i like the marker idea15:11
pedronispstolowski: it's a bit tricky to implement atomatically, we probable need to use a rename somehow15:13
pedronisheh, atomically15:13
pedronispstolowski: because the naive way we might end up we directory that we made but cannot remove15:14
pedroniss/we directory/with a directory/15:14
zygapstolowski: I commented on that as well15:16
zygaah, wrong part15:16
zygasorry, let me revise15:16
zygacommenting on the conversation page vs the diff15:16
pedronisbit confused by that suggestion, we do want to remove that dir even if it's full15:17
pedronisif it was created by us15:17
zygayes, I revised my comment15:18
pedronisijohnson: 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 propose15:20
pedroniss/that the/but the/15:20
ijohnsonpedronis sure I can just add a TODO there or something, not right now is totally fine15:22
pedronispstolowski: do you want to chat now about image and FsOnlyApply ? or tomorrow?15:35
pstolowskipedronis: yes, today is fine15:36
pedronispstolowski: let's do it now then15:36
zygaijohnson: I think that's all of them15:43
ijohnsonthank you zyga !15:43
mborzeck1ompf, finally done with https://github.com/CanonicalLtd/subiquity/pull/69215:46
mupPR CanonicalLtd/subiquity#692: console_conf: various recover chooser tweaks <Created by bboozzoo> <https://github.com/CanonicalLtd/subiquity/pull/692>15:46
mborzeck1degville: can you take a look at the wording of the user facing messages in the PR ^^15:46
zygamborzeck1: are you pythonic now? ;D15:46
mborzeck1zyga: hopefully it's just temporary15:47
zygamborzeck1: I'm __sure__ it is15:47
zygamborzeck1: what's up with 1?15:47
=== mborzeck1 is now known as mborzecki
mborzeckiprobably network15:47
degvillemborzecki: yep, of course. thanks!15:49
mborzeckidegville: thanks!15:50
the-mentorI wonder if anyone can help me with a snap issue15:57
the-mentori opened this thread to try and help debug an issue with snaps and vulkan with nvidia gpus15:58
zygathe-mentor: hey16:18
zygaI can look tomorrow as I'm about to EOD16:18
zygaI have the hardware and the means to check this16:18
zygabut I suspect it may require more love to fix unless it's something trivial16:18
zygathe-mentor: our nvidia support is not robust and needs changes :/16:20
* zyga EODs16:27
mvozyga: back now, looking at the queue now16:42
zygamvo: ta16:45
zygamvo: I'm in the office but not actively working now16:45
mupPR 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
ijohnsonzyga: do you know if there are maybe some spots in the queue taken up by just _branches_ that are pushed?16:47
ijohnsonI.e. I push a branch that I don't have an open PR for, will the tests run on that?16:48
zygaijohnson: IIRC no, only pull requests should be considered16:48
ijohnsonokay, 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 ago16:48
mupPR 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
zygait's going through a backlog16:49
zygatests for past commits to an open PR will be tested16:49
ijohnsoneven if the PR is closed or merged it will keep running those?16:49
zygayes, actions is more of a "bring your own" system16:50
zygathere's a way to encode the policy16:50
zygabut we didn't do any yet16:50
zygathere's a way to close those automatically16:50
zygawe're just discovering that16:50
zygaI made a remark about this in the internal channel16:50
zygaijohnson: though part of it is really the relative immaturity of the product16:50
ijohnsonah I see it now16:51
mupPR 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
mvocmatsuoka: do you think you could review 8439? afaict it needs a second review16:51
mvozyga, 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 fixed16:52
mvoi.e. if there is no cancel feature yet16:53
zygamvo: there's a way to do that, perhaps I should integrate that ASAP16:53
ijohnsoniiuc zyga said there is, we just have to opt-in to it somehow16:53
mvoijohnson, zyga ok, that sounds ok then, is it much work?16:54
zygalet me check16:55
zygamvo: can you look at https://github.com/styfle/cancel-workflow-action16:55
zygait has to be someone who has repo access16:55
zygathere are two things that need to be done there16:56
zygaif this works we can fork that repo and run that action from snapcore/cancel-workflow-action16:56
zygaso that we don't use 3rd party actions16:56
cmatsuokamvo: checking that...17:02
zygaijohnson, mvo: opened 845217:03
mupPR snapd#8452 opened: github: cancel previous builds <Created by zyga> <https://github.com/snapcore/snapd/pull/8452>17:03
zygathis is also interesting: https://github.com/snapcore/snapd/actions?query=is%3Aqueued17:04
ijohnsoninteresting you can manually cancel jobs too17:04
cmatsuokamvo: done17:04
zygaironically this has more visibility than travis before17:04
zygawe see exactly what's queued and what's running17:05
ijohnsonyes this is much more useful imho17:05
zygaas the queue drains I'd like to consider merging https://github.com/snapcore/snapd/pull/844017:05
zygawe could really ease the pain then17:06
mupPR #8440: github: move spread to self-hosted workers <Created by zyga> <https://github.com/snapcore/snapd/pull/8440>17:06
zygaand if cancel works that might be enough17:06
zygaif this doesn't work we could go back to travis :/17:06
zygathe goal was to improve17:06
zyganot disrupt everything without improvement17:06
ijohnsonpersonally I think we're really close to having a much better workflow than we had on travis17:07
ijohnsonI think to go back to travis would be very sad now17:07
ijohnson(apologies to anyone named traivs)17:07
zygait would only be better if we could re-oreder the queue17:07
ijohnsonthat would be TOO good17:08
zygahttps://github.com/snapcore/snapd/actions?query=workflow%3ATests+actor%3Azyga <- stuff I'm waiting on17:09
zygathat's actually cool, i didn't notice this before17:09
zygaor maybe "blocked" on17:10
zygaas those are queued17:10
ijohnsonhaha I literally was looking at that view for me too17:10
ijohnsonthere were a few jobs from like 2 weeks ago that stuck somehow so I just canceled them now17:10
ijohnsonseems to work good!17:10
zygaI have a feeling that maybe we did jump prematurely17:10
zygabut apart from the initial pain it's the better place to be17:10
zygaI suspect way more innovation is going into actions than travis today17:10
zyga(sorry travis, we really love what you gave the world)17:10
zygamvo: did you set the secret?17:22
zygaso instead of soldering that power on switch17:45
zygaI just enabled wake-on-lan17:45
mvozyga: I did17:45
zygahmm, ok,17:45
zygaI'll check once the queue clear17:45
zygamaybe a typo17:46
zygait failed with missing argument17:46
zygaas if what we provided expanded to nothing17:46
mvozyga: definitely  GH_ACCESS_TOKEN there17:48
zygait could be a non-starter with a PR17:48
zygaPR from a fork17:48
mvozyga: 8452 canceled the unit test it seems17:48
mvozyga: oh no17:49
zygaI cancelled17:49
mvozyga: so this whole cancel thing will not work?17:49
zyganot sure which PR is that17:49
zygaI don't know yet17:49
mvozyga: the cancel pr17:49
zygaah, yeah, I cancelled a few things17:49
zyga(my own)17:49
zygato explore what to do without wasting cycles17:49
mvozyga: pretty sure it won't work if the workflow is part of a fork17:49
mvozyga: as there won't be a token then, that's a bummer17:50
mupPR snapd#8453 opened: [RFC] travis.yml: re-enable travis <Created by mvo5> <https://github.com/snapcore/snapd/pull/8453>18:27
mupPR snapcraft#3016 closed: cli: remove experimental config.yaml support <Created by cjp256> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/3016>18:33
mupPR snapd#8452 closed: github: cancel previous builds <Created by zyga> <Closed by zyga> <https://github.com/snapcore/snapd/pull/8452>18:45
zygastore is wonky now18:52
zyganoise][: the store is indeed wonky now19:01
jdstrandkenvandine: hey, I just noticed these denials in snap-store while looking at an unrelated bug: https://paste.ubuntu.com/p/vhGRmC8hyK/19:06
jdstrandkenvandine: expected?19:06
jdstrandkenvandine: a bit surprised it is looking at mime stuff in hostfs19:07
kenvandinejdstrand:  which channel?19:08
cmatsuokazyga: is there a way to ask github to re-execute a test?19:08
zygacmatsuoka: yes19:08
zygacmatsuoka: click on checks19:09
zygacmatsuoka: then on the button on the right19:09
zyga(starting from a PR page)19:09
jdstrandkenvandine: it was in an unrelated bug report that just showed dmesg as an attachment. I don't have any other details19:09
cmatsuokazyga: the cancel workflow button?19:10
diddledanjdstrand, kenvandine, I confirm that it is looking in those paths on my system, too.. also denied.19:20
diddledansnappy-debug log: https://paste.ubuntu.com/p/h9pbYnWvs5/19:22
kenvandinejdstrand, 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#L51919:22
kenvandinesince the core snap has a /usr/share/applications we have to look via hostfs19:22
kenvandineto find the installed desktop files19:22
kenvandinei wonder if that load_desktop triggers something that tries to find the mime stuff19:23
diddledanthe two flatpak paths in my log are interesting, too19:24
kenvandinethat's related to XDG_DATA_DIRS19:24
kenvandineflatpak modifies those19:24
diddledanaah possibly because I've installed a flatpak then19:26
kenvandinediddledan: yeah19:28
kenvandineit'll do that19:28
zygacmatsuoka: there's a restart / cancel19:39
zygacmatsuoka: if it says cancel then it's still running19:39
* zyga is gone19:40
mupPR snapcraft#3017 opened: pluginhandler: deterministic load depending on plugin and build-base <Created by sergiusens> <https://github.com/snapcore/snapcraft/pull/3017>19:46
zygacmatsuoka: if around: https://github.com/snapcore/snapd/pull/845419:48
mupPR #8454: tests/session-tool: session ordering is non-deterministic <Created by zyga> <https://github.com/snapcore/snapd/pull/8454>19:48
mupPR snapd#8454 opened: tests/session-tool: session ordering is non-deterministic <Created by zyga> <https://github.com/snapcore/snapd/pull/8454>19:48
zygasession-tool is my worst thing ever19:48
zygalike I want to strangle myself now19:48
cmatsuokazyga: 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 anyway19:51
zygaone thing that I find super confusin19:51
zygais that when you restart tests19:51
zygawhat happens is that you get unit tests19:52
zygabut rest is still as-it-was before19:52
zygathen once those run19:52
zygayou get canary19:52
zygathen rest19:52
zygaso it's a wave19:52
zyganot an immediate invalidation19:52
mupPR snapcraft#3018 opened: Add prebuilt-wheel-dir to python plugin <Created by jimmylomro> <https://github.com/snapcore/snapcraft/pull/3018>19:55
mupPR snapcraft#3019 opened: static: consolidate tooling setup to setup.cfg <Created by sergiusens> <https://github.com/snapcore/snapcraft/pull/3019>20:01
mupPR snapcraft#3020 opened: spread tests: default base for local plugin tests <Created by sergiusens> <https://github.com/snapcore/snapcraft/pull/3020>20:01
stgraberzyga: does "snap run" require a running snapd?20:39
stgraberzyga: 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 everything20:40
stgraberzyga: our own shutdown script has logic that should timeout after 9min so this has never made much sense20:40
stgraberzyga: until I finally managed to keep a shell on a system where this is happening20:40
stgraberzyga: that's what I'm seeing there: https://paste.ubuntu.com/p/jFdb9sTghF/20:41
stgraberzyga: removing the snapd socket and manually starting snapd back up seems to fix the situation20:47
stgraberin that the stop operation then unblocks, succeeds and systemd shuts down the system20:47
zygastgraber: tired, just one sentence - yes after some operations like kernel upgrade when system key indicates snapd needs to create new sandbox profiles21:29
zygaSnap run detects this and pauses21:29
stgraberprobably should not do that on a stop operation though when snapd is already gone21:30
mupPR 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:31
SaviqChrisTownsend: ^ FYI21:32
mupPR snapcraft#3021 opened: remote-build: remove artifact sanity check <Created by cjp256> <https://github.com/snapcore/snapcraft/pull/3021>22:07
mupPR 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>22:31

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!