=== jamesh__ is now known as jamesh
[01:54] <mup> Issue core20#98 closed: add btrfs-progs to core, please? <Created by montvid> <Closed by montvid> <https://github.com/snapcore/core20/issues/98>
[06:13] <mup> PR snapd#9774 closed: o/snapshotstate: don't set auto flag in the snapshot file <Needs Samuele review> <Created by stolowski> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/9774>
[08:01] <pstolowski> morning
[08:08] <zyga> pstolowski, hey, good morning
[08:40] <zyga> mvo, good morning :)
[08:40] <mvo> zyga: good morning to you too!
[08:47] <pstolowski> hey mvo
[08:49] <mvo> hey pstolowski ! good morning
[08:53] <mvo> pstolowski: 9429 looks good, if it needs a force-merge just let me know (looks like tests are still running though)
[08:53] <mvo> pstolowski: thanks for updating the tests :)
[08:54] <pstolowski> mvo: sure, thanks for suggestions
[10:29] <pstolowski> so classic/hotplug issue was apparently an issue with the image that got fixed
[11:10] <mvo> pstolowski: that is good to know
[11:13] <pstolowski> i'm preparing a pr that will improve diagnosing such issue
[11:19] <mvo> pedronis: fun! trying to workaround issue #13 first killed dbus and now crashed my session. "assertion 's' failed on src/core/service.c:3223 in systemd. we are having fun here
[11:19] <mup> PR #13: Bugfix/review tools reenable <Created by mvo5> <Merged by elopio> <https://github.com/snapcore/snapd/pull/13>
[11:38] <pedronis> mvo: I reviewed #9791
[11:38] <mup> PR #9791: devicestate: implement checkFDEFeatures() <Squash-merge> <UC20> <Created by mvo5> <https://github.com/snapcore/snapd/pull/9791>
[11:40] <mvo> pedronis: thank you
[11:51] <pstolowski> hmm gdrive/google acting up
[11:56] <pstolowski> hmm and www.googleapis.com/compute
[11:57] <ogra> yeah, seems all og google services
[11:57] <ogra> *of
[12:01] <pstolowski> thanks for confirmation
[12:02] <degville> google is having problems: https://news.ycombinator.com/item?id=25415989
[12:03] <ogra> at least their search still works (probably fell back to the bing backend)
[12:03] <degville> ahaha!
[12:10] <mup> PR snapd#8620 closed: spdx: add GPL-3.0-or-later license <⛔ Blocked> <Created by prabhu> <Closed by pedronis> <https://github.com/snapcore/snapd/pull/8620>
[12:12] <pedronis> #9778 needs 2nd reviews
[12:12] <mup> PR #9778: asserts/repair.go: add "bases" and "modes" support to the repair assertion <UC20> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/9778>
[12:15] <pedronis> pstolowski: what should we do with https://github.com/snapcore/snapd/pull/8532 ?
[12:15] <mup> PR #8532: tests: install new snapd deb into preseed image <Created by jhenstridge> <https://github.com/snapcore/snapd/pull/8532>
[12:16] <pstolowski> pedronis: i need to think. i'm slightly inclined to close it
[12:18] <pedronis> that's fine if you think we don't need it or doesn't help enough
[12:29] <ijohnson> not a great start to the day, my internet speed is at like kilobits and on top of this google is down
[12:29]  * ijohnson considers going back to bed and trying again later 
[12:30] <cmatsuoka> ijohnson: we'll have a solar eclipse here in a few hours. it's the end of the world
[12:31] <ijohnson> cmatsuoka: haha nice
[12:38] <pstolowski> ijohnson: hey ijohnson . yep, i also saw 500s on github
[12:38] <ijohnson> hey pstolowski
[12:39] <ijohnson> yeah feels like a "week before holiday break" kind of week :-P
[12:41] <zyga> lol
[12:41] <zyga> the end of 2020 is when google deletes all gmail databases
[12:42] <pedronis> ijohnson: thanks for reviewing #9790, it needs a 2nd review now
[12:42] <mup> PR #9790: gadget: move BuildPartitionList to install and make it unexported <Cleanup :broom:> <UC20> <Created by pedronis> <https://github.com/snapcore/snapd/pull/9790>
[12:42] <pedronis> ijohnson: #9789 could also use a review
[12:42] <mup> PR #9789: many: separate consistency/content validation into gadget.Validate|Content <Cleanup :broom:> <Run nested> <UC20> <Created by pedronis> <https://github.com/snapcore/snapd/pull/9789>
[12:42] <ijohnson> pedronis: yes it's in my queue for today
[12:45] <mup> PR snapd#9793 closed: tests: add os query commands for subsystems and architectures <Created by sergiocazzolato> <Merged by sergiocazzolato> <https://github.com/snapcore/snapd/pull/9793>
[12:45] <mup> PR snapd#9794 opened: daemon: start moving implementation to api_snaps.go <Cleanup :broom:> <Created by pedronis> <https://github.com/snapcore/snapd/pull/9794>
[12:53] <slvn_> hello, I've developed a few small games and published them as snap packages, 4yrs ago.
[12:53] <slvn_> now, I am updating them and it needs manual review. Should I just wait or ask for review ?
[12:59] <slvn_> Also, it relies on SDL2 libs, and it isn't fully fonctionnal: since, it can't access libGL with dlopen, there is no hardware rendering,
[12:59] <slvn_> and fallback to software rendering which is slower and looks bad for some points.
[12:59] <slvn_> This was the case before, not sure how it's has improved now ?
[13:15] <pedronis> slvn_: what kind of errors are triggering the manual review? depending on that you might need to change some things or to ask for extra permissions through the process https://forum.snapcraft.io/t/process-for-aliases-auto-connections-and-tracks/455 , in case of doubt the expedient things would be to explain the issues within a forum post
[13:23] <slvn_> pedronis, re-doing the snap-review in console, it says:
[13:23] <slvn_>  - declaration-snap-v2:plugs_installation:1bsyl:unity8
[13:23] <slvn_> 	human review required due to 'allow-installation' constraint (bool)
[13:23] <slvn_> ok, not sure why I have this plug set. I struggle a little bit to recompile the snap
[13:24] <pedronis> it's a legacy plug at this point, a snap shouldn't need it
[13:25] <mup> PR snapd#9795 opened: tests: improve hotplug test setup on classic <Run nested> <Created by stolowski> <https://github.com/snapcore/snapd/pull/9795>
[13:26] <slvn_> I'll remove it and retry then. I think because the former version was published with unity7 plug
[13:26] <slvn_> anyone now if there is a work-around for SDL2 using libGL within snap ?
[13:27] <slvn_> I found this: https://forum.snapcraft.io/t/how-best-to-link-to-sdl2/5721/3
[13:28] <slvn_> I don't understand the last message: "At my end, this works properly with the LD_LIBRARY_PATH set by desktop-launch script from snapcraft-desktop-helpers"
[13:54] <slvn_> pedronis, I removed the unity8 plug and the snap-review passed
[13:55] <slvn_> it makes me rememeber that maybe unity7 plug one was added because sdl2 access mir on ubuntu 16.
[13:56] <slvn_> so I am not even sure that the snap would still work on ubuntu 16, any idea how i could test this ?
[14:36] <pstolowski> pedronis, mvo : i'm  going to address this comment in the existing snapshots PR: "instead of failing we also have the option to change import to return the old ...". anything else missing for the existing PR to land? The "// XXX: deal with import in progress..." would be a followup
[14:39] <mvo> pstolowski: deal with import in progress should be a followup yes, no opinion on the other one
[14:48] <mvo> ijohnson: 9787 is ready for a re-review
[14:48] <ijohnson> great
[14:50] <ijohnson> oooh github has a dark mode now
[14:50] <ijohnson> fancy
[14:51] <ijohnson> mvo: +1
[14:54] <mvo> thank you!
[14:54] <mvo> also 9769 needs reviews :)
[15:10] <ijohnson> added that one to my queue as well :-)
[15:15] <mvo> ijohnson: thank you!
[15:16] <ijohnson> yaw
[15:16] <mvo> pedronis: I replied to 9791, not 100% sure I got what you had in mind
[15:19] <pedronis> mvo: I maybe confused but I looked again and errors from there seems to be eaten and just turn into false, nil one level up?
[15:20] <mvo> pedronis: oh, sorry, let me double check
[15:20] <mvo> pedronis: I think you are right, the code is confusing
[15:21] <pedronis> maybe we should log one level up?
[15:21] <mvo> pedronis: yeah, I think we also not log seccomp errors there :/
[15:22] <mvo> pedronis: so let me add logging and a test
[15:36] <ijohnson> pstolowski: is there any reason in theory we couldn't do `snap debug state --timings file.json` like `snap debug timings` but for local offline state.json ? does debug timings rely on snapd being active ?
[15:38] <mvo> pedronis: what is your sense, should we log the tpm/fde errors as debug or notice? right now I have debug
[15:39] <pedronis> mvo: this is for install right?  I would say notice
[15:39] <mvo> pedronis: yes, install
[15:39] <mvo> pedronis: sounds good to me, thank you!
[15:41] <pstolowski> ijohnson: there is not reason, can be done. it's just historical, timings were implemented long before the idea for offline debug
[15:41] <pstolowski> *no reason
[15:41] <ijohnson> nice
[15:41] <ijohnson> yeah it would be useful to do offline debugging for this very slow system we are debugging right now
[15:41] <ijohnson> maybe I'll have a quick stab at it if I get bored
[15:47] <pstolowski> ijohnson: the only slightly annoying detail is that the api side of debug takes care of combining timings with task doing/undoing, nothing complex about that, but it's slightly more convoluted than just printing timings
[15:47] <ijohnson> hmm I see, perhaps we could push that code into the client for `snap debug state ...` specifically
[15:47] <ijohnson> or share that code
[15:48] <pstolowski> yes sharing would be nice, otherwise a lot needs to be duplicated
[15:48] <pstolowski> and yes, offline timings would be nice imho
[15:51] <mup> PR snapd#9787 closed: boot: tweak resealing with fde-setup hooks <Skip spread> <Squash-merge> <UC20> <Created by mvo5> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/9787>
[16:30] <mup> PR core20#99 opened: hooks: update xdg-settings to support subcommands <Created by seb128> <https://github.com/snapcore/core20/pull/99>
[16:39] <mup> PR core18#175 opened: hooks: update xdg-settings to support subcommands <Created by seb128> <https://github.com/snapcore/core18/pull/175>
[17:05] <slvn_> ... some follow-up, I fix my issue: passed review, access to libGL and audio, and now I am trying my updated snap to an old ubuntu 16 machine
[17:06] <slvn_> and I got this failing message when installing:
[17:06] <mup> PR snapd#9791 closed: devicestate: implement checkFDEFeatures() <Squash-merge> <UC20> <Created by mvo5> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/9791>
[17:06] <slvn_> snap assumes unsupported features: command-chain (try new ubuntu-core)
[17:07] <slvn_> it works on ubuntu 20, but no 16
[17:07] <ijohnson> slvn_: what is `snap version` ?
[17:07] <slvn_> ijohnson, what do  you mean ?
[17:07] <ijohnson> slvn_: on the system that you try to install the snap on, can you run the command `snap version` ?
[17:07] <slvn_> unknow command on ubuntu 16
[17:08] <slvn_> :) so quite old
[17:08] <slvn_> snap --version:
[17:08] <slvn_> 2.16
[17:08] <ijohnson> wow, yeah that's positively ancient
[17:08] <ijohnson> you will need to do `snap install core` I think
[17:08] <slvn_> ok
[17:10] <slvn_> ijohnson, done, but the version of snap hasn't changed
[17:10] <slvn_> and still not working
[17:11] <ijohnson> slvn_: what is `snap list` ?
[17:11] <slvn_> ubuntu-core 16-2
[17:11] <slvn_> and core 16-2
[17:11] <slvn_> core 16-2.48
[17:11] <ijohnson> can you try `sudo apt update && sudo apt install snapd`
[17:11] <pedronis> mvo: did you forget to push to #9791 ?
[17:11] <mup> PR #9791: devicestate: implement checkFDEFeatures() <Squash-merge> <UC20> <Created by mvo5> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/9791>
[17:12] <ijohnson> pedronis: yeah I didn't see the code that was mentioned either
[17:13] <slvn_> ijohnson, snapd is already the newest version 2.16
[17:13] <ijohnson> slvn_: but that's not the newest snapd ... package, what is `cat /etc/os-release` ?
[17:13] <slvn_> ubuntu 16.10
[17:13] <ijohnson> ah
[17:14] <slvn_> I'm try on an old ubuntu my updated snap
[17:14] <ijohnson> you are using an eol distro and so there are no more updates
[17:14] <mvo> pedronis: silly me, I push an updated PR
[17:14] <pedronis> it's ok but bit confusing :)
[17:14] <ijohnson> slvn_: you should really upgrade to 18.04 or an ubuntu series that is still supported, 16.10 is no longer supported at all
[17:14] <slvn_> sorry, I am away
[17:14] <slvn_> ijohnson, I try a regression to test my snap on an older VM
[17:17] <mvo> pedronis: pushed 9796
[17:21] <mup> PR snapd#9796 opened: devicestate: log checkEncryption errors via logger.Noticef <Squash-merge> <UC20> <Created by mvo5> <https://github.com/snapcore/snapd/pull/9796>
[17:27] <pedronis> mvo: commented
[17:28] <ijohnson> pedronis: reviewed #9789, seems we lost a couple tests ?
[17:28] <mup> PR #9789: many: separate consistency/content validation into gadget.Validate|Content <Cleanup :broom:> <Run nested> <UC20> <Created by pedronis> <https://github.com/snapcore/snapd/pull/9789>
[17:29] <pedronis> ijohnson: yes, a couple of tests were removed as they don't apply anymore
[17:30] <ijohnson> so we don't validate the bootloader setting in the gadget.yaml ?
[17:30] <ijohnson> that doesn't seem like an expected chagne
[17:30] <ijohnson> *change
[17:30] <pedronis> ijohnson: we do
[17:30] <pedronis> there's more tests
[17:30] <ijohnson> where ?
[17:30] <pedronis> you need to consider the tests that are there
[17:30] <pedronis> are you looking at the tests or just the diff?
[17:31] <ijohnson> I searched the full file where you deleted that test from and couldn't find one which checks for an invalid bootloader setting in the gadget.yaml
[17:31] <ijohnson> are you saying the existing test is in a different file?
[17:31] <pedronis> ijohnson: notice that per comment &gadget.ValidationConstraints{} and nil are not equivalent
[17:32] <pedronis> ijohnson: gadget_test has tests about that
[17:33] <pedronis> ijohnson: Validate doesn't redo the job of InfoFromGadgetYaml given that now it takes a parsed gadget.Info
[17:39] <ijohnson> pedronis: I responded in the pr with a new comment
[17:39] <ijohnson> I see the tests now
[17:44] <pedronis> ijohnson: I tried to answer
[17:50] <ijohnson> pedronis: I responded with another suggestion, my concern is really that the behavior to a casual reader/reviewer is non-standard and thus subject to bugs where folks don't fully comprehend the change they are making
[17:51] <ijohnson> pedronis: mvo: are there any other pr's that I should review in my afternoon, I think I have reviewed them all except #9780 which I think can wait a bit if not until after the break at least tomorrow/wednesday
[17:51] <mup> PR #9780: many: use ResolvedSource() from gadget content when writing boot assets <UC20> <Created by mvo5> <https://github.com/snapcore/snapd/pull/9780>
[17:54] <mvo> ijohnson: I will push something shortly to workaround the systemd-run issue in initrd
[17:54] <mvo> ijohnson: it will be a bit ugly
[17:55] <ijohnson> mmm ok
[18:00] <pedronis> ijohnson: not from me, my follow up in gadget probably is better submitted after we have landed the rest
[18:01] <ijohnson> ok
=== ijohnson is now known as ijohnson|lunch
[18:05] <mvo> pedronis, ijohnson|lunch I pushed 9797 as a draft but I really hope to land 9769 soon so that the diff is nicer to read
[18:05] <ijohnson|lunch> sounds good to me
[18:06] <mup> PR snapd#9797 opened: secboot: add workaround for snapcore/core-initrd issue #13 <Created by mvo5> <https://github.com/snapcore/snapd/pull/9797>
[18:16] <mup> PR snapd#9769 closed: tests: add nested spread end-to-end test for fde-hooks <Run nested> <Squash-merge> <UC20> <Created by mvo5> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/9769>
[18:21] <mvo> pedronis, ijohnson|lunch 9797 is updated, I will look at 9796 after dinner
=== ijohnson|lunch is now known as ijohnson
[20:11] <mvo> ijohnson: I call it a day now, feel free to apply tweaks/fixes etc to 9797 as needed, it's the last PR for 2.48.2 we really need (same for 9796 but that is slightly less critical)
[20:11] <ijohnson> mvo: ok, sounds good I am reviewing it now
[20:11] <ijohnson> I will push any suggestions I ahve
[20:11] <mvo> \o/
[20:11] <ijohnson> *have
[20:12] <mvo> cool
[20:13] <mvo> thank you!
[20:14] <pedronis> I asked some questions there, in 9797
[20:33] <ijohnson> pedronis: is it okay to address the things you mentioned  in a push to mvo's branch ?
[20:33] <ijohnson> I responded to you points in 9797
[20:35] <pedronis> ijohnson: thanks, yes I think it's ok to try to push forward there
[20:35] <ijohnson> pedronis: ack will push in a little bit
[20:38] <pedronis> ijohnson: preopening the files seems the path forward, I see *Mode options for directories but not for those files
[20:39] <pedronis> at least pre-creating doesn't sound wrong
[20:39] <pedronis> also because by default systemd doesn't truncate them according to docs
[20:58] <ijohnson> right
[21:52] <ijohnson> pedronis: if you're still around I pushed a couple fixes to 9797
[21:52] <ijohnson> spent a bit of time writing an overly paranoid unit test which may have been overkill, but I don't think it hurts
[21:56] <pedronis> ijohnson: thx, it's slightly late for me to do a poper review but I'll try to do it first thing in the morning
[21:56] <pedronis> *proper
[21:56] <ijohnson> sounds good
[21:56] <ijohnson> have a nice evening, ttyl
[21:57] <pedronis> thx, same to you
[23:18] <mup> PR snapd#9790 closed: gadget: move BuildPartitionList to install and make it unexported <Cleanup :broom:> <UC20> <Created by pedronis> <Merged by pedronis> <https://github.com/snapcore/snapd/pull/9790>
[23:58] <mup> PR snapd#9798 opened: o/{device,snap}state: enable devmode snaps with dangerous model assertions <UC20> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/9798>