[00:08] kyrofa, about the command, the only way I can make them match is if I explicitly set the string to be "snapcraft, version xxx" in both commands. Is that okay? [00:09] gsilvapt, I'd leave the Click one `prog` [00:09] kyrofa, sounds okay to me but the problem is that when you run the tests it does not print snapcraft. Instead it prints run, version xxx [00:10] Maybe that is related with the tests, I do not know [00:10] gsilvapt, oh that's unfortunate indeed [00:10] gsilvapt, yeah, hardcode it then [00:10] Ok, I will do that, verify again the tests and submit the changes [00:10] kyrofa, thank you! [00:50] Issue snapcraft#1666 closed: Refactor to support elf patching [00:50] PR snapcraft#1744 closed: elf: conversion from libraries [01:50] PR snapcraft#1725 closed: tests: share the cache in ros tests === elijah_ is now known as elijah [05:56] morning guys [06:31] PR snapd#4264 closed: tests: force delete when tests are restore to avoid suite failure [06:39] PR snapd#4240 closed: spread.yaml: increase workers for opensuse to 3 [06:53] PR snapd#4266 opened: snapstate: add new refresh-hints helper and use it [07:01] good morning [07:02] hey zyga-ubuntu , good morning [07:11] :-) [07:11] I'm still in non-{wayland,x} text mode [07:12] using another computer for code reviews [07:12] this is particularly refreshin [07:12] lots of free memory and full-screen, single focus applications [07:13] zyga-ubuntu: haha, nice [07:23] I spent some time yesterday trying to untangle our prepare/restore code [07:23] if not permanantly then at least enough to get good measurements of what is changing [07:23] and I must say that code is complex and confusing [07:24] not just in the actions taken but in their placement and justification [07:24] (or lack or thereof) [07:24] zyga-ubuntu: yeah, its all over the place [07:25] I'll keep at it as it needs to be in proper shape to detect tests that change too much [07:40] PR snapd#4267 opened: snapstate: move catalogRefresh into its own helper [07:46] * kalikiana o/ [07:46] mvo: review on 4266 [07:48] zyga-ubuntu: ta [07:50] I marked it as changes requested but just midlly so, we can merge this quickly [07:59] zyga-ubuntu: thanks, the can-auto-refresh question is interessting as it raises a bigger questions [08:01] pedronis: do you think the new refresh-hints code should also consult "CanAutoRefresh", or should we do the refresh-hints even if the device is still bootstraping? I guess we want to honor CanAutoRefresh but I want to double check. I also think you are right about RefreshOptions:RefreshNop, I think we want this, I will followup on this [08:03] zyga-ubuntu: thanks again for the quick review :) [08:03] :-) [08:26] mvo: yes, I think we want to consult CanAutoRefresh (otherwise will just consider partial information or don't have a device session yet) [08:28] pedronis: +1 - thank you [08:56] * zyga-ubuntu tries something mergable instead [09:00] mvo: trivial merge on 4268 [09:00] PR snapd#4268 opened: spread.yaml: move prepare-each closer to restore-each [09:11] PR snapd#4269 opened: timeutil: new refresh timer parser [09:13] PR snapd#4270 opened: timeutil: remove support to parse weekday schedules [09:16] mborzecki: heh, looks like we need to solve conflicts here ;) either way is fine with me, I can wait for your merge or you can review mine and merge into yours (or wait until master is in) [09:16] mborzecki: should be very simple in any case [09:17] mvo: yup, i'm wiring a comment on gh [09:18] i'm actually using some of the pieces that you remove :) [09:18] mborzecki: yeah, happy to revert those bits [09:19] mborzecki: the key of the PR (I can make it smaller) is that the weekday parsing in v1 is not supported in snapstate so its dead code and caused one confusion in the validation already [09:21] hmm ok, i mean it's not a big problem, if it gets merged before 4269, i'll just bring back the missing pieces [09:22] mborzecki: no worries, I can do that now to make your life easier, its the weekday part of the struct and the weekdayMap that you need, right? [09:23] but i'm guessing 4269 will be a longer review, so I wouldn't want to block you [09:23] mborzecki: yeah, let me rework my PR (was not aware of yours) so that the impact for you is minimal [09:24] mvo: only if you have time for this, otherwise i can fix this myself in my PR [09:25] mvo: would appreciate you taking a look at 4269 BTW :) [09:26] mborzecki: re-added the weekday/weekday map now. I check the other one out once I finished my current PR. I glanced over it and it looked good from a very brief look [09:34] mvo: thanks [09:49] hmm [09:50] so, what does ${GOPATH%%:*} expand to? [09:50] because as far as I can see, to nothing at all [09:51] %% will strip the longest suffix of "*" which is everything [09:51] it'll strip the suffix of ":*" -- not "*" [09:52] so effectively remove everything including and after the first colon [09:53] are you sure? [09:53] ah [09:53] right, I made a typo in my tests [09:53] thanks! [09:55] :-) [09:56] mvo: can you please force-merge 4268 [09:57] I could try again and waste an hour but I think the situation is riddiculus enough not to do that [09:57] and the error is clearly some store flue on refresh deltas [09:59] PR snapd#4268 closed: spread.yaml: move prepare-each closer to restore-each [10:00] thanks! [10:01] mvo: another quick PR to look at 4271 [10:01] yw [10:02] PR snapd#4271 opened: spread.yaml: fix shellcheck issues and trivial refactor [10:17] mborzecki: errors in your schedule PR [10:18] mborzecki: real unit tests failures in overlord/snapstate [10:18] zyga-ubuntu: thx, on it [10:18] mvo: btw, I'm on FF57 now (actually on nightly so 59) and I can use umatrix [10:18] mvo: try again, maybe your extension was fixed now [10:19] mvo: ff 59 with umatrix is the fastest browser on any system I used [10:19] (umatrix cutting the number of crap javascript from ads and tracking) [10:22] zyga-ubuntu: I have a bunch that I missed, but things are mostly good again. umatrix, cookie auto-delete, noscript are my essential ones, I also have a custom one that I need to port in time (based on the passhash extension) [10:22] mvo: things are fast again! [10:23] PR snapd#4272 opened: snapstate: ensure RefreshSchedule() gives accurate results [10:25] mvo: ^ the description there has "git push ..." in the middle, could you fix it [10:25] pedronis: autsch, sorry for that [10:25] not sure if it lost bits or just need the bit removed [10:27] pedronis: apparently a wrongly focused window, the original commit message is correct [10:27] pedronis: most of the PRs this morning are based on what we talked about yesterday, I they capture your suggestions, they should make 4161 simpler === __chip__ is now known as Chipaca [10:33] mvo: review on 4272 [10:33] mborzecki: do you think 4269 can be split? [10:34] mborzecki: I'm often doing small reviews while waiting for local spread run to finish but 1K PR is not in that class [10:34] for sure i can put the first 2 patches in a separate pr [10:34] mborzecki: thank you, I'll gadly review that [10:39] PR snapd#4273 opened: timeutil: introduce helpers for weekdays and TimeOfDay [10:39] zyga-ubuntu: ^^ [10:41] thanks [10:43] mborzecki: doesn't that TimeOfDay Add method mess up on DST change? [10:43] 11pm + 6h isn't always 5am [10:43] Chipaca: I commented on a similar aspect [10:44] mborzecki: commented [10:44] hmm not sure you can do anythign about this TimeOfDay as it's not aware of actual date [10:45] mborzecki: yeah, it depends on how it is used [10:47] zyga-ubuntu: i think we can ignore leap seconds though -- we're explicitly not supporting them as a valid time [10:47] Chipaca: +1 [10:47] PR snapd#4271 closed: spread.yaml: fix shellcheck issues and trivial refactor [10:47] imo, it's the same way as time.Time.Add() is not aware of DST only until you do presentation [10:48] mborzecki: it all depends on how this is then used, though [10:50] Chipaca: trivial 4274 [10:50] PR snapd#4274 opened: spread.yaml: bump delta ref to 2.29 [10:51] that's a review size i can _do_ [10:51] * Chipaca in pain and on pain meds -- not a good combo [10:54] Chipaca: :( [10:54] pedronis: ¯\_(ツ)_/¯ [10:55] PR snapd#4275 opened: spread.yaml,tests: move most of project-wide prepare/restore to separate file [10:55] Chipaca: take care, I'm sorry for your pain :| [10:55] another small refactor for spread.yaml ^ [10:55] 10% executed locally so I assume it's okay, rest will show up in real testing [10:56] zyga-ubuntu: this one is to work with your changes to spread? [10:56] Chipaca: this is all preparation work, I made so many changes trying to untangle and understand this it was impossible to review [10:56] Chipaca: I can propose the spread PR for reference [10:56] Chipaca: the problem is we don't really prepare/restore cleanly [10:57] Chipaca: and I will hopefully changed that by EOD, without introducing any extre time costs [10:57] Chipaca: have a look at spread#47 please [10:57] PR spread#47: Add support for project-wide measure-each stanza [10:57] zyga-ubuntu: I'm getting this again :-( ` open /var/lib/snapd/hostfs/home/cris/snap/lxd/common/snapcraft9xy6a3v1/core_3440.assert: no such file or directory [...] subprocess.CalledProcessError: Command '['lxc', 'file', 'push', '/home/cris/snap/lxd/common/snapcraft9clif0ke/snapcraft_x1.snap', 'helga:snapcraft-multipass/run/snapcraft_x1.snap']' returned non-zero exit status 1.` [10:58] kalikiana: which of the files is missing? any denials? [10:59] Chipaca: in my approach the measure-each code ensures that files on / (and just that filesystem) and outside of /tmp don't change, mount table doesn't change, packages don't change and sysctl doesn't change [10:59] Chipaca: but it changes all the time as prepare/restore is a bit wonky and I need to clean it up before addressing some real problems [11:01] PR snapd#4253 closed: task tunner: extended task runner test [11:01] zyga-ubuntu: not that I can see... [11:02] I'm trying again with `snappy-debug scanlog` open [11:02] zyga-ubuntu: this is btw snapcraft as a snap, with lxd from apt [11:02] mvo: approved 4272 [11:02] zyga-ubuntu: ta [11:02] kalikiana: too many errors recently [11:02] kalikiana: I have LXD issues to address b [11:03] kalikiana: but our integration test suite is so unreliable I took a dive into it [11:03] zyga-ubuntu: thanks again for this review, that made things a lot better (checkRefreshSchedle was pretty terrible) [11:03] zyga-ubuntu: is there anything that changed in lxd recently? I'm quite sure snapcraft changed nothing [11:03] kalikiana: I don't know, sorry :/ [11:04] kalikiana: there was a new stable lxd recently I think, but that is really all I know [11:04] stable channel lxd snap to be precise [11:04] PR snapcraft#1750 closed: kernel plugin: don't assume /lib/firmware is present if !kernel-with-firmware [11:05] kalikiana: yesterday to be precise. and also a new cnadidate a bit later [11:05] mvo: I guess I'll try if the snap suffers from the same issue - what I find super puzzling is that hostfs is in the path at all, and considering it's empty in the host that would easily explain why lxd can't access it [11:07] PR snapcraft#1643 closed: tests: run daily autopkgtest in travis [11:08] mborzecki: approved 4273 [11:08] ta [11:09] PR snapd#4267 closed: snapstate: move catalogRefresh into its own helper [11:09] mvo: trivial comment on 4266 (from pedronis, not me), +2 otherwise and ready to land [11:10] mvo: thank you for updating 4262 :) [11:11] Chipaca: do you feel like reviewing 4254 from mvo (it's about SNAP_DID_REEXEC) [11:11] ? [11:11] PR snapd#4256 closed: snap-confine: add workaround for snap-confine on 4.13/upstream [11:11] PR snapd#4262 closed: store: do not log the http body for catalog updates [11:11] * Chipaca looks [11:17] mvo: we still ship a configure hook in core, is the plan to remove that entirely and switch to internally-handled configuration? [11:27] zyga-ubuntu: the plan is to remove it, we still have more work to do to fix 2.30 to make that possible [11:27] though [11:27] pedronis: I see [11:28] https://forum.snapcraft.io/t/new-configure-snapd-task-and-reverting/2774 [11:28] pedronis: I just noticed we do something odd in prepare code [11:28] we enable refresh on external spread backends [11:28] but only if the configure hook exists [11:31] zyga-ubuntu: concerning denials: none, I'm only seeing unrelated ones eg. Telegram now [11:31] which makes sense if all it does is read an empty folder [11:34] kalikiana: is this locally or in some CI system? [11:34] kalikiana: do you have any changes on the system that is affected? [11:35] zyga-ubuntu: all local, xenial with backports [11:41] mborzecki: is 4270 safe to merge now? [11:42] zyga-ubuntu: yes [11:42] mvo: trivial comment on 4252 [11:42] mvo: and more comments on 4272 [11:42] PR snapd#4270 closed: timeutil: remove support to parse weekday schedules [11:43] 4263 needs a 2nd review [11:43] PR snapd#4254 closed: cmd, errtracker: get rid of SNAP_DID_REEXEC environment [11:44] mborzecki: can you finish your review o 4109, it's small and has been lingering for too long [11:44] PR snapd#4171 closed: tests: adding test to test physical memory observe interface [11:46] mborzecki: conflicts in 4273 [11:49] mborzecki, .. and in 4269 [11:51] cjwatson: Aww.. no more gimme? [11:52] Hellos! [11:52] niemeyer: thank you for the music [11:53] zyga-ubuntu: hi. I was wondering if you have any ideas about how to write a spread test for my user-mounts PR. I haven't included any interface that add user mounts at this stage, so I'm not sure where to start [11:56] pushed an update to 4273, 4269 will be fixed later as currently it will conflict with 4273 [11:59] jamesh: I think it should be a part of the desktop interface (right?) [12:00] jamesh: alternatively just use a test snap, install it, hack the profile and run update-ns [12:00] jamesh: (and, more importantly, hack the profile, discard the ns and run snap-confine again so that it sets up things) [12:00] zyga-ubuntu: the desktop innnterface will use it for portals eventually, but that isn't part of the PR [12:00] jamesh: look at the advanced content interface test for inspiration [12:01] zyga-ubuntu: also, what's the most convenient way to run a single spread test locally? [12:01] jamesh: look at tests/main/interfaces-content-mkdir-writable [12:01] jamesh: the qemu backend, you can iterate very quickly (ish) [12:01] jamesh: I'm running it all the time [12:01] jamesh: I pushed images to spread.zygoon.pl/images [12:02] jamesh: so pick one and look at spread docs on how to use [12:02] jamesh: (just put to ~/spread/qemu) [12:02] thanks [12:02] jamesh: then SPREAD_DEBUG_EACH=0 spread qemu:ubuntu-16.04-64:tests/main/ [12:02] jamesh: then SPREAD_DEBUG_EACH=0 spread -debug -v -reuse qemu:ubuntu-16.04-64:tests/main/ [12:02] (2nd line is better) [12:03] jamesh: the desire for the spread test is to see that this really really works with confinment and what not [12:03] jamesh: and weird kernels and other unexpected stuff [12:04] mvo: small typo in 4266 [12:04] mvo: small tweak needed in 4252 [12:05] * zyga-ubuntu is trying to get the PR page list down to one [12:06] #4158 needs a 2nd review [12:06] PR #4158: snapctl: don't error out on start/stop/restart from configure hook during install or refresh [12:07] looking [12:07] zyga-ubuntu: okay. fwiw, the unit tests in the PR are all passing now: it looks like the run yesterday hit some problem trying to talk to the VMs [12:07] jamesh: yeah, our perpetual problem of not having a batch system [12:16] mvo, hey [12:16] mvo, looking at the issue with "Job for snapd.service canceled" [12:17] mvo, any idea how to deal with it? [12:17] mvo, retrying in case of error could be a solution? === Son_Goku is now known as Conan_Kudo === Conan_Kudo is now known as Son_Goku [12:27] pstolowski: review on 4158 [12:27] hmm, so MATCH is in the top-level spread context, this is a problem if we want to use it in scripts :/ [12:28] PR snapd#4274 closed: spread.yaml: bump delta ref to 2.29 [12:29] hello niemeyer :) [12:29] thank! [12:29] thanks* :) [12:29] zyga-ubuntu: Hey! [12:29] zyga-ubuntu: Man, I found your brother last night [12:30] zyga-ubuntu: https://www.youtube.com/watch?v=cHjf-YmIBCs [12:30] niemeyer: oh? [12:31] hahaha [12:31] yeah, actually he even sounds similar in some way [12:31] I hit the case where spread tarball changes while we are making it, hmmm, [12:31] that is a fun one :( [12:32] maybe once I'm done with refactoring/untangling it will become evident [12:32] I bet something is not done right somewhere [12:32] service lingers or something like that [12:33] zyga-ubuntu: Yeah, the voice tone, the looks, and even the shirt! [12:33] niemeyer: surreal a bit :) [12:33] :P [12:33] niemeyer: my accent is better though (haha, maybe) [12:35] zyga-ubuntu: Well, not better, but less german :-) [12:36] (I don't see accents as bad in general.. just means we speak another language too) [12:39] accents don't necessarily imply another language [12:39] oh god yes [12:39] I watched a review of a game recently [12:39] and geez [12:39] this guy was unberable [12:39] IMO, they tell a story of the person's origin and history [12:40] somewhere from US presumably but _MAN_ [12:40] I've intentionally smoothed out the accent and speech style I grew up with, because it can confuse people [12:41] and even then, I don't have all of it out of my speech [12:41] ok, I'm done watching my weird mirror :) [12:43] Son_Goku: you're like Jack Barrowman :-D [12:43] although arguably his original accent is worse [12:43] well, I'm not *quite* so flamboyant [12:43] but yes [12:43] haha, yeah [12:44] well, for the first half of my childhood was spent in Indiana (US Midwest) and the second half was in Mississippi (US South), so I have bits of both in my speech [12:46] afk, see you at the call [12:48] zyga-ubuntu: the plan is to not ship the configure hook in core, yes [12:50] cachio: hey, thanks for chasing the "job canceled" issue, I have no good answer, a retry is certainly not a bad idea, maybe this gives us even a better idea what exactly is causing this [12:51] zyga-ubuntu: thanks also for the reviews, I chase the open points now :) [12:52] mvo, I'll try it [12:52] mvo: thank you! :) [12:58] * kalikiana coffee break [13:00] mvo: should we ask systemd peeps about that? [13:00] I'll be late 2 min [13:04] hmm [13:04] some issues joining [13:05] hmm :/ [13:17] Chipaca: its only on 14.04 and on our crufty backport [13:38] mwhudson: is the plan to have go 1.10 in B? [13:44] Chipaca: are you rejoining? [13:48] ok, I'll break for lunch and I'll push forward in a moment :) [14:02] zyga-ubuntu: question for you i think: why do osutil.Find[UG]id return a uint64? [14:04] Chipaca: I believe that is what the golang 1.9 code we borrowed did do [14:04] Chipaca: and we just ran along [14:05] hmm [14:05] * Chipaca looks [14:05] offtopic, 3C and rain, I really don't envy commuters [14:05] zyga-ubuntu: golang uses strings for uids and gids [14:05] zyga-ubuntu: in the os/user package [14:05] elsewhere it uses uint32's [14:05] (hilariously) [14:06] Chipaca: then I don't remember, I think there were two layers of this? [14:06] Chipaca: or maybe as simple and naive as what parseint returns [14:07] *uint [14:08] zyga-ubuntu: also: those functions return a 0 on error, which can be bad [14:08] but i don't know if i want to fix the in this pr [14:08] hmm [14:17] ogra_, hey [14:18] ogra_, https://paste.ubuntu.com/26020272/ [14:18] the timexone in ubuntu core is n/a by defaulty [14:18] ogra_, is it a known issue? [14:20] zyga-ubuntu, mvo: in snap-seccomp/main.go's findGid we check a regexp, and then call osutil.FindGid which checks a regexp [14:20] zyga-ubuntu, mvo: those regexps differ [14:20] zyga-ubuntu, mvo: quoth the raven, "WAT" [14:20] zyga-ubuntu, mvo: ignore me, i got that wrong [14:22] cachio, not that i'm aware ... file it ... [14:22] ogra_, ok tx [14:22] cachio, is that x86 or arm ? [14:23] (on arm the fixrtc script checks for n/a and sets it properly, on x86 we do not use the fixrtc script afaik) [14:23] mvo: the one with weirder features is probably correct [14:23] er [14:23] Chipaca: ^ [14:23] Chipaca: can you paste them here please? [14:24] ogra_, all of them [14:24] ogra_, in the pi3 also [14:24] zyga-ubuntu: sorry, i got confused, but let me show you how [14:25] cachio, ah, well ... we should set it to UTC as default at build time i guess [14:25] zyga-ubuntu: in snap-seccomp main: group must match "^[a-z][-a-z0-9_]*$" [14:26] that is probably incorrect (just too restrictive) [14:28] zyga-ubuntu: in snap-update-ns entry: x-snapd.gid must match "(^[0-9]+$)|(^[a-z_][a-z0-9_-]*[$]?$)" [14:28] this one is correct but also matches numbers (as just direct group i) [14:29] so if you want to take one, take the latter one and remove the number part [14:29] Chipaca: the 2nd one is correct, I looked at the spec and, weirdly enough, that's what valid names are [14:31] zyga-ubuntu: group names can end in $ but otherwise have no $ in them? [14:31] what does that $ mean? (i presume it's special somehow) [14:31] (i'm not changing that regexp in this round) [14:31] (next one? sure) [14:32] ogra_, it is set to UTC but the description is n/a [14:32] yes [14:32] Time zone: n/a (UTC, +0000) [14:33] Chipaca: it's special, it means [14:33] ... [14:33] man I should have commented that somewhere [14:33] cachio, yes, i see that ... but if you set it manually the description says UTC as well [14:33] Time zone: UTC (UTC, +0000) [14:33] zyga-ubuntu: yeah :-) a "where does this regexp come from" would be good [14:33] ogra_, yes [14:33]