=== jamesh_ is now known as jamesh
[07:00] <mborzecki> morning
[07:57] <mup> PR snapd#10074 opened: daemon: fix signing key validity timestamp in unit tests <Simple 😃> <Skip spread> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/10074>
[08:02] <mborzecki> mvo: hey
[08:02] <mvo> good morning mborzecki
[08:03] <mborzecki> mvo: i think that https://github.com/snapcore/snapd/pull/10074 should fix the model timestamp issue taht we occasionally see in the tests
[08:03] <mup> PR #10074: daemon: fix signing key validity timestamp in unit tests <Simple 😃> <Skip spread> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/10074>
[08:04] <mvo> mborzecki: niiiiiiiice!
[08:05] <mvo> mborzecki: haha, nice catch
[08:06] <mborzecki> afaiu the model gets signed by the key with some timestamp, but we generate user assertions and as a dependency set up the account key only in the next call
[08:09] <mvo> yeah, it akes sense
[08:11] <mborzecki> if we used RFC3339Nano for the timestamps it would come up sooner ;)
[08:14] <pstolowski> morning
[08:25] <mvo> good morning pstolowski
[08:25] <mvo> mborzecki: haha, indeed
[08:26] <mborzecki> pstolowski: hey
[08:28] <mborzecki> mvo: i've restarted spread test job in https://github.com/snapcore/snapd/pull/10071 should we pull in the change that added 21.04 to spread.yaml too? (maybe in a separate pr?)
[08:28] <mup> PR #10071:  packaging: drop dh-systemd from build-depends on ubuntu-16.04+ (2.49) <⚠ Critical> <Created by mvo5> <https://github.com/snapcore/snapd/pull/10071>
[08:30] <mvo> mborzecki: a good question, probably
[08:30] <mvo> mborzecki: yeah, makes sense, gives us more confidence for the unit tests this way
[08:33] <mup> PR snapd#10074 closed: daemon: fix signing key validity timestamp in unit tests <Simple 😃> <Skip spread> <Created by bboozzoo> <Merged by stolowski> <https://github.com/snapcore/snapd/pull/10074>
[08:43] <pedronis> is prepare failing with: 2021-03-24T07:31:03.3664323Z rm: cannot remove '/var/lib/snapd': Directory not empty
[08:43] <pedronis> 2021-03-24T07:31:03.3665576Z dpkg: error processing package snapd (--purge):
[08:43] <pedronis> 2021-03-24T07:31:03.3666687Z  installed snapd package post-removal script subprocess returned error exit status 1
[08:43] <pedronis> 2021-03-24T07:31:03.3667583Z Errors were encountered while processing:
[08:43] <pedronis> something new?
[08:43] <pedronis> I got it here: https://github.com/snapcore/snapd/pull/10040/checks?check_run_id=2182138789
[08:43] <mup> PR #10040: daemon: switch api_test.go to daemon_test and various other cleanups <Cleanup :broom:> <Created by pedronis> <https://github.com/snapcore/snapd/pull/10040>
[08:44] <mborzecki> pstolowski: can you take a look https://github.com/snapcore/snapd/pull/10059 ?
[08:44] <mup> PR #10059: cmd/snap: use less aggressive client timeouts in unit tests <Simple 😃> <Skip spread> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/10059>
[08:44] <pstolowski> mborzecki: looking
[08:45] <mborzecki> thanks!
[08:50] <mvo> thank you guys, you rock
[08:51] <pedronis> mborzecki: hi, #10067 looks good, there are mostly question whether we are missing some tests perhaps, #10054 we still need to think what to do with the check
[08:51] <mup> Bug #10067: jigit: new changes from Debian require merging <jigit (Ubuntu):Fix Released by pitti> <https://launchpad.net/bugs/10067>
[08:51] <mup> PR #10067: overlord/snapstate: make sure that snapd current symlink is not removed during refresh <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/10067>
[08:51] <mup> Bug #10054: gnome-gpg: new changes from Debian require merging <Ubuntu:Fix Released by cjwatson> <https://launchpad.net/bugs/10054>
[08:51] <mup> PR #10054: snapdtool, wrappers: add dependency on usr-lib-snapd.mount for services on core with snapd snap <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/10054>
[08:52] <pedronis> #10068 needs 2nd reviews
[08:52] <mup> Bug #10068: kbd-chooser: new changes from Debian require merging <kbd-chooser (Ubuntu):Fix Released by cjwatson> <https://launchpad.net/bugs/10068>
[08:52] <mup> PR #10068: o/configstate: don't pass --root=/ when masking/unmasking/enabling/disabling services <Run nested> <Created by stolowski> <https://github.com/snapcore/snapd/pull/10068>
[08:53] <mup> PR snapd#10059 closed: cmd/snap: use less aggressive client timeouts in unit tests <Simple 😃> <Skip spread> <Created by bboozzoo> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/10059>
[08:54] <mborzecki> pedronis: i'm taking a look at the questions in 10067, and i think we should land it before 10054
[08:54] <pedronis> ok
[08:55] <pedronis> #10070 also needs a 2nd review, it's trivial
[08:55] <mup> Bug #10070: kdegraphics: new changes from Debian require merging <Ubuntu:Fix Released by cjwatson> <https://launchpad.net/bugs/10070>
[08:55] <mup> PR #10070: tests/core/fsck-on-boot: unmount /run/mnt/snapd directly on uc20 <Simple 😃> <Test Robustness> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/10070>
[08:56] <mvo> looking
[08:58] <mup> PR snapd#10037 closed: boot: export helper for clearing tried system state, add tests <Simple 😃> <Created by bboozzoo> <Merged by pedronis> <https://github.com/snapcore/snapd/pull/10037>
[08:58] <pedronis> mborzecki: ^ I merged that
[08:58] <mborzecki> pedronis: thanks!
[08:59] <pedronis> mvo: #10035 is kind of ready to land, otoh maybe it should have run with nested ?
[08:59] <mup> PR #10035: o/devicestate: split off ensuring next boot goes to run mode into new task <Simple 😃> <UC20> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/10035>
[08:59] <mup> Bug #10035: camlimages: new changes from Debian require merging <Ubuntu:Fix Released by cjwatson> <https://launchpad.net/bugs/10035>
[09:02] <mvo> pedronis: let me look. what is your opinion on 10068 for 2.49? it seems safe enough but it also changes how we disable rsyslog
[09:02] <mvo> pedronis: we could also pull it into 2.50 that would be okay still timeline wise (but 2.49 would be nicer from this POV)
[09:03] <pedronis> mvo: we have spread tests for that I think, it doesn't seem very risky too me, but maybe pstolowski has input on this
[09:03] <mvo> pedronis: I will run 10035 here nested locally
[09:03] <mvo> pedronis: yeah, my feeling too
[09:04]  * pedronis quick errands
[09:11] <pstolowski> mvo: yes i think 10068 should be fine for 2.49
[09:11] <pstolowski> assuming spread nested is happy
[09:13] <mup> PR snapd#10069 closed: tests: fix cgroup-tracking test <Created by sergiocazzolato> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/10069>
[09:15] <mvo> pstolowski: thanks! can I squash merge 10048 ? or would you rather have the two commits? (would make backports easier)
[09:16] <pstolowski> mvo: it's fine to squash
[09:28] <pstolowski> mvo: nb i might have a fix for https://bugs.launchpad.net/snapd/+bug/1920773 today
[09:28] <mup> Bug #1920773: Snapd crashes and fails to start when snap config dict and dict elememnts unset at the same time <snapd:Confirmed for stolowski> <https://launchpad.net/bugs/1920773>
[09:28] <jamesh> alan_g: I think https://github.com/MirServer/snapd/pull/6 should resolve the remaining issues with the desktop-launch PR. There were a lot of unclosed review threads.
[09:28] <mup> PR MirServer/snapd#6: More fixes based on review feedback <Created by jhenstridge> <https://github.com/MirServer/snapd/pull/6>
[09:30] <mvo> pstolowski: \o/ thank you
[09:36] <alan_g> jamesh, thanks. Will take a look this PM (my time)
[09:36] <jamesh> alan_g: thanks.  No merges from master to confuse things this time
[09:50] <mborzecki> pedronis: i've updated #10067
[09:50] <mup> Bug #10067: jigit: new changes from Debian require merging <jigit (Ubuntu):Fix Released by pitti> <https://launchpad.net/bugs/10067>
[09:50] <mup> PR #10067: overlord/snapstate: make sure that snapd current symlink is not removed during refresh <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/10067>
[10:13] <pedronis> pstolowski: I reviewed 100062 with a suggestion
[10:14] <pstolowski> pedronis: thanks! i didn't know that
[10:25] <pstolowski> oh well, i've a tentative fix for panic on nil map when unsetting the config, but when adding new tests i found another funny scenario where unsets followed by sets over same path don't give correct result at the end :(
[10:27] <pedronis> pstolowski: sounds we have deeper problems?
[10:27] <pedronis> or we broke something recently?
[10:28] <pstolowski> pedronis: don't think we broke, must be something that dates back to introduction of unset/nils
[10:29] <pedronis> :/
[10:32] <pstolowski> really an edge case i think, e.g. doc.x.a=1; commit; doc.x=nil doc.x.a=nil doc.x.a=1 -> doesn't set doc.x.a back, config remains nil (that's with my tentative fix as otherwise it would panic in the middle)
[10:32] <pstolowski> also, this works fine if commited in between, so only an issue when unsetting and then setting over same path in same transaction
[10:33] <pstolowski> and it's hard to comprehend what's going wrong :/
[10:35] <pedronis> pstolowski: sounds like Transaction.changes is not quite the right representation or we are holding it wrong
[10:35] <pstolowski> pedronis: or PatchConfig logic gets something wrong
[10:37] <pedronis> yea, but at some if the code is hard to get right maybe the data structure is wrong
[10:37] <pedronis> *some point
[10:50] <pstolowski> pedronis: should we unhide snap validate command now?
[10:53] <mborzecki> is the forum down?
[10:57] <pedronis> not here, a bit slow
[11:00] <pstolowski> mborzecki: do you have a moment  to take a look at https://github.com/snapcore/snapd/pull/10062 ?
[11:00] <mup> PR #10062: tests: validation sets spread test <validation-sets :white_check_mark:> <Created by stolowski> <https://github.com/snapcore/snapd/pull/10062>
[11:00] <mborzecki> pstolowski: sure
[11:03] <pedronis> pstolowski: for 2.50? yes, enforce is hidden right?
[11:03] <mborzecki> pedronis: been thinking about https://github.com/snapcore/snapd/pull/10054#pullrequestreview-616530057 a bit, and what zyga proposed is probably the only way that works correctly with run-from-snapd-snap
[11:03] <mup> PR #10054: snapdtool, wrappers: add dependency on usr-lib-snapd.mount for services on core with snapd snap <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/10054>
[11:04] <pedronis> mborzecki: I fear that but it feels a bit strange
[11:06] <mborzecki> pedronis: it would probably involve stat() grab major:minor of the device, inpect /sys/dev/block/<maj:min>/loop/backing_file, not too hard but not simple either
[11:07] <pedronis> mborzecki: we need this only for core18/20 right, though? not classic?
[11:08] <pstolowski> pedronis: yes, for 2.50. enforce is hidden but also entire command is hidden
[11:08] <mborzecki> yes, it's for 10054 which is relevant to core only
[11:08] <pedronis> I mean the helper where is is now would need to be general, but our use case is core only
[11:08] <pedronis> mborzecki: we can get the info from the model on core
[11:09] <pedronis> we just need to pass it down somethow
[11:10] <mborzecki> pedronis: ah, so you mean rather than inspecting where snapd comes from, check whether the curernt model indicates core (or otherwise the need to usr-lib-snapd.mount)?
[11:12] <mborzecki> hm the services are written when linking the snap, so we could probably pass something down in LinkContext
[11:25] <pedronis> mborzecki: another approach would be to check if usr-lib-snapd.mount exists
[11:25] <pedronis> perhaps?
[11:25] <mborzecki> let me get you the diff, it's actually quite clean
[11:28] <mborzecki> pedronis: https://paste.centos.org/view/0c605d86 it's missing tests in snapstate
[11:28] <mborzecki> oh, and pastebinit does not work anymore (?), i'm getting `Failed to contact the server: HTTP Error 405: Method Not Allowed`
[11:43] <pedronis> mborzecki: yes, it looks reasonable
[11:45] <pedronis> mborzecki: we can discuss final naming in the PR
[11:45] <mborzecki> yup
[12:13] <mborzecki> pedronis: i've updated https://github.com/snapcore/snapd/pull/10054
[12:13] <mup> PR #10054: overlord/snapstate, wrappers: add dependency on usr-lib-snapd.mount for services on core with snapd snap <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/10054>
[12:14] <mvo> pedronis: I updated 9981 as discussed
[12:14] <mvo> pedronis: I could also chop it into smaller chunks (or try to), it grew over time :/
[12:16] <pedronis> mvo: I'll look in a bit
[12:16] <pedronis> mborzecki: thanks, I'll look in a bit
[12:17] <mborzecki> back to that review i promised to pstolowski
[12:18] <mvo> pedronis: thanks!
[13:10] <pedronis> mvo: I reviewed it,  I think it needs re-reviews from the others too
[13:53] <ijohnson> morning
[13:58] <mvo> pedronis: great, thanks
[15:40] <jdstrand> mvo: hi! I updated snappy-debug today for 2.49.1, but it is failing to upload: https://launchpad.net/~jdstrand/+snap/snappy-debug/+build/1347999 ('Cannot upload new revisions for name=snappy-debug'). I reauthorized store uploads, but it didn't work. I don't have any visibility into this. can you (or someone else) help/advise?
[15:42] <jdstrand> mvo: also https://launchpad.net/~jdstrand/+snap/snappy-debug/+build/1348000 (this is amd64, the other is i386, the others haven't built yet)
[15:43] <zyga> hey jdstrand :)
[15:43] <zyga> it's so nice to see You here
[15:43] <jdstrand> hey zyga :)
[15:43] <jdstrand> nice to see you too :)
[15:43] <zyga> how do you find time to work on Snappy bits? I'm swamped with work myself?
[15:44] <jdstrand> zyga: I don't really. I am attending a virtual conference today and it didn't start til 10am local so I did something bite size :)
[15:45] <zyga> linaro connect by any chance?
[15:45] <zyga> no, that would be much earlier
[15:45] <jdstrand> no, it is a secure coding thing
[15:45] <zyga> oh, neat
[15:46] <jdstrand> I may find have some work time for some interface work to support some snaps at some point though
[15:46] <jdstrand> s/find/find I/
[15:46] <zyga> I want to get back into _some_ snapd work that's actually aligned with my new day job
[15:47]  * jdstrand nods
[16:13] <pedronis> mborzecki: I reviewed #9940, thanks
[16:13] <mup> PR #9940:  boot: cmd/snap-bootstrap: handle a candidate recovery system v2 <Run nested> <UC20> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/9940>
[16:15] <mvo> pedronis: fwiw, 9981 is updated again
[16:18] <pedronis> mvo: re-reviewed
[16:18] <mvo> pedronis: \o/ thank you
[16:21] <mvo> maybe ijohnson or mborzecki can do a second review of 9981 of the last few commits, great that it's finally ready :)
[16:29] <mup> PR snapcraft#3487 opened: cli: warn about migration if using core <Created by cjp256> <https://github.com/snapcore/snapcraft/pull/3487>
[16:47] <ijohnson> mvo: sure I will add it to my queue
[16:47] <mvo> ta
=== ijohnson is now known as ijohnson|lunch
[18:24] <mup> PR snapcraft#3488 opened: cli, repo: add support for UA tokens <Created by cjp256> <https://github.com/snapcore/snapcraft/pull/3488>
=== ijohnson|lunch is now known as ijohnson
[20:40] <mup> PR snapd#10075 opened: udisks2 2.8.4 needs to also lock /run/mount/utab <Needs Samuele review> <Created by knitzsche> <https://github.com/snapcore/snapd/pull/10075>