=== chihchun_afk is now known as chihchun
[06:06] <mborzecki> morning
[07:57] <mborzecki> zyga: pushed some fixes to #6264
[07:57] <mup> PR #6264: spread, tests: add Fedora 29 <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/6264>
[07:57] <mborzecki> that was one annoying distro upgrade, hopefully the changes make the whole test suite better
=== pstolowski|afk is now known as pstolowski
[08:07] <pstolowski> mornings
[08:13] <zyga> Hey ho
[08:14] <zyga> Just woke up
[08:14] <zyga> I’ll be around shortly
[08:15] <mborzecki> pstolowski: zyga: hey
[08:25] <zyga> Hi :-)
[08:25] <zyga> Feels like winter
[08:27] <jamesh> zyga: thanks for the help with that test snap yesterday.  It build successfully, but I'm guessing it got trapped by automated review.  I'm not sure who other than mvo has access to that snap.
[08:29] <zyga> I think jdstrand can check the status
[08:29] <zyga> I will ping mvo about the snap too just in case he has some time
[08:31] <jamesh> Running review-tools manually, it seems to be complaining about the new attribute on the dbus interface, and the fact I used "daemon: dbus" for one of the apps
[08:31] <jamesh> something snapd supports but snapcraft and review-tools don't seem to like
[08:33] <zyga> Hmmmm
[08:33] <vidal72[m]> mborzecki: this may affect snapd in arch: https://git.archlinux.org/svntogit/packages.git/commit/trunk?h=packages/filesystem&id=6fa932aa685da87e78e0f845bd890645607d302e , https://bugs.archlinux.org/task/58499
[08:33] <zyga> Maybe that is something for roadmr as well
[08:34] <zyga> Can you write a forum post about it please? It will be easier to as for help this way
[08:35] <mborzecki> vidal72[m]: thanks, i'm subscribed already, they actually changed it to ID=archlinux https://git.archlinux.org/svntogit/packages.git/commit/trunk?h=packages/filesystem&id=6fa932aa685da87e78e0f845bd890645607d302e
[08:36] <mborzecki> wonder if snapd is the only project that checks that at all
[08:38] <vidal72[m]> breaking something is the best way to know is someone used it :)
[08:43] <mborzecki> updated golang got pushed to EPEL stable, hopefully we'll be able to switch centos image to 7.6 tomorrow
[08:59] <zyga> :-)
[09:01] <mborzecki> hm hm actually switched the image temporarily and was able to build snapd package
[09:03] <mborzecki> a unit test failed failed in fedora 29 pr https://paste.ubuntu.com/p/r8WGW9DHNW/ a race of sorts?
[09:12] <pstolowski> looking
[09:13] <pstolowski> mborzecki: hmm close to the area of system key computation that was touched recently
[09:14] <pstolowski> zyga: ^
[09:27] <pstolowski> running this test in a loop locally, works fine, i suspect it's because system key is in place (do we miss mocking in the test?)
[09:31] <zyga> mmm
[09:31] <zyga> looking
[09:31] <zyga> feels unrelated
[09:31] <zyga> like missing mocking
[09:31] <zyga> "foo failed" was expectef
[09:32] <zyga> but logs had something else
[09:34] <mup> PR snapd#6232 closed: overlord/snapstate: support for pre-remove hook <⛔ Blocked> <Created by stolowski> <Closed by stolowski> <https://github.com/snapcore/snapd/pull/6232>
[09:34] <pstolowski> zyga, mborzecki #6268 is green now and needs reviews
[09:34] <mup> PR #6268: overlord/ifacestate: helpers for serializing hotplug changes <Hotplug 🔌> <Created by stolowski> <https://github.com/snapcore/snapd/pull/6268>
[09:34] <zyga> looking
[09:39] <pstolowski> zyga, mborzecki do you have anything that needs a review?
[09:40] <zyga> I mainly have https://github.com/snapcore/snapd/pull/6190
[09:40] <mup> PR #6190: overlord/configstate,features: expose features to snapd tools <⛔ Blocked> <Created by zyga> <https://github.com/snapcore/snapd/pull/6190>
[09:40] <mborzecki> pstolowski: #6264
[09:40] <mup> PR #6264: spread, tests: add Fedora 29 <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/6264>
[09:40] <pstolowski> ok
[09:42] <pstolowski> mborzecki: oh wow... that looks like whack-a-mole (not your fault)
[09:44] <mborzecki> pstolowski: yeah, loads of fun
[09:57] <zyga> mborzecki: did you get to the bottom of why those tests with swapped su username passed before?
[10:10] <mborzecki> zyga: yes, the project path created by spread is subject to default umask, that has changed in fedora 29, but was 022 before, so the directories along the path got 755, then the snapd source tree is owned by test:test, even though /home/gopath/src/github.com/snapcore/snapd is owned by root:root
[10:10] <pstolowski> mborzecki: would it be possible to set umask anywhere in prepare.sh to have sane defaults for all tests (if not already there)?
[10:10] <pstolowski> (i suppose it's not that easy, but curious)
[10:12] <mborzecki> pstolowski: idk whether spread makes a single huge script of each prepare, prepare-each, execute block, but it'd probably need to be set by spread to be effective
[10:12] <mborzecki> maybe it makes sense to extend spread to support umask: 022 in spread.yaml
[10:12] <pstolowski> mborzecki: uhm, right, i see
[10:13] <mborzecki> zyga: oh, and some of the tests that i fixed never executed on fedora (because of systems: [..]), or only ran for strict confinement
[10:18] <zyga> mborzecki: spread uses shell, cat to write files and execute them
[10:18] <zyga> mborzecki: I spent some time looking at it
[10:19] <mborzecki> zyga: hm so spread level thing probably makes most sense
[10:19] <zyga> mborzecki: I have some patches to use sftp to send files back and forth
[10:19] <zyga> but probably unlikely to land them
[10:20] <mborzecki> anyways, the fixes are imo useful either way :)
[10:26] <mborzecki> damn, hate travis ui
[10:27] <mborzecki> open travis job, 'raw log' button appears only once the log was downloaded a chopped into folds by js
[10:29] <zyga> maybe we should kill the folds
[10:29] <zyga> they are -1 useful
[10:30] <zyga> travis is slow like hell anyway
[10:30] <zyga> I only ever use the raw log
[10:30] <zyga> and often it is too long
[10:30] <zyga> folds don't help in the raw log
[10:46] <mup> PR snapd#6269 opened: cmd, dirs, interfaces/apparmor: update distro identification to support ID="archlinux" <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/6269>
[10:47] <mborzecki> zyga: pstolowski: ^^ simple one
[10:47] <zyga> I will probably do only reviews in the morning
[10:47] <pstolowski> k
[10:47] <zyga> thanks maciek!
[10:48] <mborzecki> we'll need to cherry-pick that to 2.36
[10:48] <mborzecki> and in the meantime, i'll ship a patch with the package
[10:49] <zyga> mborzecki: commented inline but +1
[10:49] <mborzecki> thx
[10:49] <zyga> I think we should do 2.37 next week
[10:50] <zyga> backporting sucks
[10:56] <mborzecki> zyga: btw. i think there was a subtle bug in the code you pointed
[10:56] <zyga> oh? where?
[10:57] <mborzecki> defers are executed LIFO, so the defer dirs.SetRootDir() would execute before restore of mocking os release
[10:58] <zyga> mmm
[11:03] <pstolowski> mborzecki: there is one more defer setrootdir in this test
[11:04] <pstolowski> (in the old code)
[11:18] <Xceed> is there a way to do the following with getting a validation error...
[11:18] <Xceed> text=$(cat <<EOF
[11:18] <Xceed> branch = master
[11:18] <Xceed> EOF
[11:18] <Xceed> )
[11:18] <Xceed> within override-prime:
[11:18] <Xceed> as a scriptlet
[11:18] <Xceed> s/with/without
[11:26]  * zyga is not well
[11:27] <pstolowski> Xceed: you may want to ask on the forum where more snapcrafters can help
[11:36] <zyga> mborzecki, pstolowski: it's funny how we use go
[11:36] <zyga> but our task/change system is really a bunch of python-like key=value store
[11:38] <zyga> pstolowski: sorry for the lag, I feel really so-so
[11:38] <zyga> reviewed
[11:38] <pstolowski> true. i think that's common for task-based solutions like ours.. i saw that in other projects
[11:38] <pstolowski> you end up with flexible datatypes, such as variant to stuff metadata on tasks
[11:39] <pstolowski> zyga: no worries, get some rest
[11:39] <zyga> I'm happy to do more reviews
[11:40] <zyga> anyone?
[11:41] <Xceed> i used separate lines of echos instead, crude, but it works and ive spent too much time on this
[11:41] <zyga> xnox: is the problem with yaml?
[11:41] <zyga> er
[11:41] <zyga> Xceed: ^
[11:41] <zyga> (sorry xnox, bad tab completion again)
[11:42] <pstolowski> zyga: https://github.com/snapcore/snapd/pull/6114 is the closes hotplug PR to land, but i think we want mvo's review anyway
[11:42] <mup> PR #6114: overlord/ifacestate: handler for hotplug-disconnect task <Hotplug 🔌> <Created by stolowski> <https://github.com/snapcore/snapd/pull/6114>
[11:42] <Xceed> yes, i copied pasted a multiline cat file <<EOF type that works in the script to the yaml and it failed
[11:42] <pstolowski> zyga: otherwise nothing immediate from me, thanks for the review of hotplug seq
[11:42] <zyga> Xceed: can you show me the yaml please?
[11:42] <Xceed> i did, see above
[11:43] <zyga> I mean a larger fragment
[11:43] <zyga> that shows the key-value in yaml
[11:43] <zyga> unless I missed that, sorry
[11:44] <zyga> my daughter has cold and is sick this week, I guess I'm becoming affected
[11:47] <zyga> mborzecki: do you feel we could review and land the snapd.mk branch?
[11:48] <Xceed> zyga, https://vgy.me/OT7tii.png
[11:48] <zyga> aba
[11:48] <zyga> that's not valid yaml
[11:48] <zyga> not the way you expect at least
[11:48] <zyga> parse that with any yaml parser to see what the structure is
[11:49] <zyga> indent lines 69-71
[11:49] <zyga> to be flush with the indent of cat
[11:49] <mborzecki> Xceed: indent to match `cat`
[11:49] <zyga> and re-parse
[11:49] <zyga> yep
[11:49] <Xceed> its a scriptlet, i expect all inside EOT to be taken as a raw output though
[11:49] <Xceed> otherise any identation will be added to the output file
[11:50] <zyga> it's yaml
[11:50] <zyga> not shell script
[11:50] <zyga> all the stuff in this document is yaml
[11:50] <mborzecki> Xceed: but it must be a valid yaml in the first place, this one is not
[11:50] <zyga> the values may be strings
[11:50] <zyga> the strings may have any content
[11:50] <zyga> but you must make it valid yaml first
[11:50] <Xceed> k, thanks guys
[11:55] <mup> PR snapd#6267 closed: overlord: move Conf interface to configstate/config to avoid cyclic imports <Simple 😃> <Created by zyga> <Closed by zyga> <https://github.com/snapcore/snapd/pull/6267>
[11:57] <zyga> mborzecki: https://github.com/snapcore/snapd/pull/6264 is green
[11:57] <mup> PR #6264: spread, tests: add Fedora 29 <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/6264>
[11:58] <mborzecki> yay
[12:00] <mup> PR snapd#6264 closed: spread, tests: add Fedora 29 <Created by bboozzoo> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/6264>
[12:02] <zyga> jamesh: relayed to mvo
[12:03] <zyga> I'll also chase with jdstrand later today
[12:04] <zyga> mborzecki: looking at https://github.com/kubernetes-sigs/kind
[12:04] <zyga> I like the pkg/ tree
[12:04] <zyga> I mean, putting various source code under pkg/
[12:04] <zyga> and not in random spots in the top level
[12:07] <mborzecki> hm can we replace the nice 'crushing failure and despair' or the pony with a useful piece of text?
[12:08] <mborzecki> ./run-checks --short-unit output is ok locally, but quite useless on travis
[12:09] <mborzecki> can't figure out why unit tests (the separate job) fail: https://api.travis-ci.org/v3/job/464278797/log.txt
[12:09] <mborzecki> the log ends with:
[12:09] <mborzecki> The command "./run-checks --short-unit" exited with 0
[12:09] <mborzecki> Done. Your build exited with 1.
[12:09] <mborzecki> wtf?
[12:15] <jamesh> zyga: thanks.  I've written up the question over using "daemon: dbus" here: https://forum.snapcraft.io/t/support-for-daemon-dbus/8855
[12:36] <zyga> jamesh: thank you, I will use this for talking with jdstrand later todat
[12:36] <zyga> *today
[12:36] <zyga> mborzecki: yes
[12:36] <zyga> mborzecki: it sucks
[12:36] <zyga> mborzecki: I had a PR for this, one sec
[12:37] <zyga> https://github.com/snapcore/snapd/pull/6209
[12:37] <mup> PR #6209: run-checks: discard test good/bad banner <Created by zyga> <Closed by zyga> <https://github.com/snapcore/snapd/pull/6209>
[12:37] <mup> PR snapd#6209 opened: run-checks: discard test good/bad banner <Created by zyga> <https://github.com/snapcore/snapd/pull/6209>
[12:37] <mborzecki> nice
[12:38] <mborzecki> wonder if mvo prefers to see the banner on his terminal or on travis
[12:38] <zyga> one thing to fix
[12:38] <mborzecki> the html log shows the banner correctly iirc
[12:38] <zyga> is to fail fastt
[12:38] <mborzecki> yup
[12:38] <zyga> not go on doing useless extra tests if something failed
[12:39] <zyga> mborzecki: I would honestly prefer a short summary
[12:39] <zyga> saying stuff like:
[12:39] <zyga> unit tests: pass
[12:39] <zyga> format: fail
[12:39] <zyga> vet: pass
[12:39] <zyga> etc
[12:39] <zyga> the banner is childlish
[12:40] <zyga> and I haven't seen it displayed properly in the last year
[12:40] <mborzecki> heh
[12:40] <mborzecki> idk maybe some isatty check in the shell would do
[12:44] <zyga> IMO the banner is pointless but that's just me
[12:49] <pstolowski> +1
[12:49] <pstolowski> we could easily find a single utf emoji to replace it ;)
[13:02] <Xceed> ${SNAP_COMMON} used in the scope of the apps.name.command: contains <apps.name> i.e. /var/snap/<apps.name>/common but in the scope of override-prime does not i.e. /var/snap/snapcraft/common (so 'snapcraft' instead). So, I append '/../../<apps.name>/common' in order to make ${SNAP_COMMON} path in the scope of prime to be consistent with that at the command runtime - a little kludgy but it worked.
[13:10] <pstolowski> zyga: some comments to #6190
[13:10] <mup> PR #6190: overlord/configstate,features: expose features to snapd tools <⛔ Blocked> <Created by zyga> <https://github.com/snapcore/snapd/pull/6190>
[13:11] <zyga> thank you
[13:11] <pstolowski> zyga: sorry if they are naive
[13:11] <pstolowski> i'm likely missing a detail or some agreement
[13:15] <zyga> pstolowski: thank you, replied to most
[13:16] <zyga> ask questions about anything you want, I realise some of the choices may be more obscure than usual
[13:18] <pstolowski> zyga: thanks
[13:23] <zyga> thank you for looking :)
[13:33] <jdstrand> jamesh, zyga: there is some history with 'daemon: dbus'. I'll have to research but will comment in the forum
[13:33]  * zyga nods
[13:34] <pstolowski> mborzecki: i'm still puzzled about multiple dirs.SetRootDir("/") in TestPathsArch in https://github.com/snapcore/snapd/pull/6269
[13:34] <mup> PR #6269: cmd, dirs, interfaces/apparmor: update distro identification to support ID="archlinux" <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/6269>
[13:35] <pstolowski> if you can address that then i'm happy to approve
[13:39] <mborzecki> pstolowski: we update the paths only when dirs.SetRootDir() is called, that's why it has to be called again after changing the mocked os-reelase info
[13:39] <mborzecki> pstolowski: does that answer your question?
[13:39] <pstolowski> mborzecki: ah, i see, yes, thanks!
[13:41] <cachio> zyga, hey
[13:41] <cachio> about this spread: detect "signal: terminated" in journal logs
[13:41] <cachio> it is breaking all the tests on borads
[13:41] <cachio> :(
[13:42] <zyga> can you show some example please?
[13:42] <cachio> I see lines like this one
[13:42] <cachio> Dec 06 05:12:04 localhost.localdomain snapd[1998]: udevmon.go:119: udev enumeration error: cannot read udevadm output: signal: terminated
[13:42] <cachio> which make the test suite fail
[13:42] <zyga> cachio: can you please show systemctl cat snapd.service
[13:43] <cachio> zyga, https://paste.ubuntu.com/p/64hkcch6FT/
[13:43] <zyga> cachio: can you tell me how those tests are being used
[13:44] <zyga> it seems like tests are out of sync with the running code
[13:44] <zyga> https://github.com/snapcore/snapd/pull/6250
[13:44] <mup> PR #6250: data: set KillMode=process for snapd <Created by mvo5> <Merged by zyga> <https://github.com/snapcore/snapd/pull/6250>
[13:44] <zyga> this PR introduced the check for "signal: terminated"
[13:44] <zyga> as well as a change to the systemd unit for snapd service
[13:44] <cachio> the ones on edge: I flash stable image, then I refresh to edge
[13:44] <cachio> and run the tests using external backend
[13:44] <zyga> the only way tests would pass is you always tested (whichever channel) with a matching test tree
[13:45] <zyga> so we need to either update snapd/core to match tetsts
[13:45] <zyga> or downgrade tests to match snapd/core
[13:46] <cachio> zyga, on edge tests I am doing that
[13:47] <cachio> using core from edge and using the last commit pushed to snapd-vender and used to build the core snap
[13:48] <zyga> and the failure you saw, was that following the process?
[13:48] <zyga> that you just outlined?
[13:50] <mborzecki> pstolowski: can i have your +1 on https://github.com/snapcore/snapd/pull/6269 ? :)
[13:50] <mup> PR #6269: cmd, dirs, interfaces/apparmor: update distro identification to support ID="archlinux" <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/6269>
[13:50] <zyga> cachio: ?
[13:51] <cachio> zyga, I ran the lxd test
[13:51] <pstolowski> mborzecki: absolutely
[13:51] <zyga> cachio: not sure how that answers my question
[13:51] <zyga> cachio: were the tests and the code in sync when that failure was reported?
[13:51] <cachio> zyga, yes
[13:51] <mborzecki> pstolowski: ta
[13:51] <cachio> core from edge
[13:51] <zyga> cachio: can you look at the PR I referenced above please
[13:51] <zyga> it adds the new check
[13:52] <zyga> and adds a KillMode=process entry to the systemd service
[13:52] <zyga> right?
[13:52] <zyga> how can we explain that the test was present but the service was not changed?
[13:54] <cachio> zyga, checking logs to see traceability
[13:54] <zyga> cachio: my explanation is that the service is not changed because the test system was out of date with tests
[13:54] <zyga> would be great to figure out if that's true and if so, what needs changing
[13:55] <cachio> zyga, yes, got it, I am trying to see the traceability between the tests we are using and the changes included in edge
[13:55] <cachio> to determine if those are really on sync
[13:57] <zyga> mborzecki, pstolowski: I'm in bed, will skip standup
[13:57] <mborzecki> zyga: ack
[13:57] <zyga> my update: not feeling well, doing some simpler tasks,
[13:58] <pstolowski> ack, shall we skipp standup then and do quick one on irc?
[13:58] <mborzecki> pstolowski: we can do the regular one
[13:58] <pstolowski> ok
[14:34] <mup> PR snapd#6269 closed: cmd, dirs, interfaces/apparmor: update distro identification to support ID="archlinux" <Created by bboozzoo> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/6269>
[14:37] <mup> PR snapd#6270 opened: data/selinux, tests: refactor SELinux policy, add minimal tests <SELinux> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/6270>
[14:39] <mborzecki> off to pick up the kids
[14:57] <kravietz> hi all, my snapcraft has just upgraded to 3 and it tries to build snaps with multipass, but multipass won't work on a build server that is a VM itself - any ideas?
[14:58] <roadmr> kravietz: if the VM is throwaway, you can use --destructive-mode
[14:58] <roadmr> but it is destructive :)
[14:58] <kravietz> in what sense?
[14:58] <kravietz> is is the same as in snapcraft 2?
[14:58] <kravietz> for now I have just reverted to the latest 2.x
[15:02] <zyga> jdstrand: gentle ping about https://github.com/snapcore/snapd/pull/6244
[15:02] <mup> PR #6244: release: detect too old apparmor_parser <Created by zyga> <https://github.com/snapcore/snapd/pull/6244>
[15:07] <niemeyer> sergiusens: Was just running snapcraft now and got surprised by how slow it is to even start
[15:07] <niemeyer> sergiusens: Looks like a regression
[15:49]  * cachio lunch
=== chihchun is now known as chihchun_afk
[16:08] <kjackal> hi snappy people, I have this LP builder that was supposed to push a snap to the store an hour ago, but I do not see it https://launchpad.net/~microk8s-dev/+snap/microk8s-1.13
[16:08] <kjackal> We have already builders for 1.12, 1.11, 1.10 and latest snap tracks and they work with no issues
=== pstolowski is now known as pstolowski|afk
[18:57]  * cachio afk
[22:57] <zyga> jdstrand: thank you!
[22:57] <zyga> jdstrand: I pushed the fixes too btw
[22:57] <zyga> woot, that was one spicy branch to land :-)
[23:20] <jdstrand> zyga: thanks!
[23:20] <jdstrand> hehe, yes
[23:21] <zyga> I'm happy 2.36 got the "short" version of that branch
[23:21] <zyga> with 0 changes to release/apparmor.go
[23:21] <zyga> but I'm also happy this made it into master, it's much nicer now