[05:50] morning [06:38] PR snapd#4726 opened: [2.31] systemd, wrappers: start all snap services in one systemctl call [06:39] mvo: morning [06:39] hey mborzecki, good morning [06:39] mborzecki: thanks for opening this pr! [06:40] mvo: do we have any idication that we'll be doing another point release? [06:41] good morning [06:42] hey zyga, good morning [06:42] zyga:hey [06:42] hey guys [06:42] mvo did the release happen [06:42] there were some issues last night [06:43] zyga: yeah, it happened but according to bret they use phased updates to push it out [06:43] mvo +1 to merge 4724 [06:44] mvo ah, good, I stopped following at around midnight [06:46] btw. i was browsing archlinxu forum the other day, and noticed this: https://bbs.archlinux.org/viewtopic.php?id=234301 apparently thre's a problem installing snaps when behind a firewall [06:47] i think we could set HTTP_PROXY in /etc/environment (or an equivalent) but that would be inconvenient if you change networks, eg. there's a need for a proxy on one network but not on another [06:48] mborzecki: hm, yeah, so I think one fix would be that snap (the client) passes the clients proxy env to snapd and snapd uses that. should be simple [06:49] mvo I think the way this is handled in real networks is different [06:49] mborzecki: iirc I talked to gustavo about it but he did not like it, but I don't remember why [06:49] mvo there's a function to call for each URL that gives you proxy data [06:49] zyga: thats how libproxy will do it [06:49] (or information not to proxy) [06:49] exactly [06:49] so depending on how far we are willing to go with it [06:49] tricky for us to do because of the snapd daemon restriction [06:50] I think it's still a bit simplistic as in corporations the auth is integrated but at least it is better than all-or-nothing [06:50] looked through stdlib source code, and it seems that HTTP_PROXY is supported [06:51] mborzecki: yes, but snapd will only read it from /etc/environment [06:51] there is a forum discussion but unfortunately inconclusive [06:51] uhh right, i meant i used that as indication that net/http can do proxy [06:51] aha, yes :) [06:52] https://forum.snapcraft.io/t/improve-proxy-configuration-for-snapd/942 fwiw [06:54] heh, all this proxy stuff is actually a bit awkward to me, i'm used to this being handled transparently by whatever trickery cisco or netgear employed [07:09] can #4719 be merged? [07:09] PR #4719: timeutil: account for 24h wrap when flattening clock spans [07:12] mborzecki: how is the autostart going? anything interesting new here? [07:13] PR snapd#4719 closed: timeutil: account for 24h wrap when flattening clock spans [07:13] mvo: trying to finish with timer services first, maybe i'll look into autostart a bit later today [07:14] mborzecki: yeah, timers are more important. thank you [07:19] linode:ubuntu-14.04-64:tests/main/refresh-all-undo failed again https://paste.ubuntu.com/p/wGwqr8MDSp/ === Beret- is now known as Beret [07:59] heyas! [07:59] hey pstolowski ! good morning [07:59] hey :) [08:00] pstolowski: thanks for your review, I love the attrer stuff, much nicer than the naked map access [08:00] mvo, cool! [08:01] pstolowski: hey [08:05] #4716 anyone? it's trivial [08:05] PR #4716: tests: make sure snapd is running before attempting to remove leftover snaps [08:11] pstolowski: looking [08:12] mvo, ty! [08:12] PR snapd#4716 closed: tests: make sure snapd is running before attempting to remove leftover snaps [08:18] mvo can I land 4724 [08:18] it's just a trivial preparation for upcoming cleanup [08:19] good morning [08:19] hey kalikiana [08:21] PR core#81 closed: 14-set-motd.chroot: updated based on the suggestions from Mark [08:23] PR snapd#4726 closed: [2.31] systemd, wrappers: start all snap services in one systemctl call [08:24] thanks mvo! [08:24] PR snapd#4724 closed: osutil: aggregate mockable symbols [08:25] hmm I think chipaca already mentioned this before [08:25] but I'm getting this [08:25] advisor/backend.go:199: literal copies lock value from *db: github.com/snapcore/snapd/vendor/github.com/snapcore/bolt.DB contains sync.Pool contains sync.noCopy [08:30] pstolowski: do you think we can get the limited reader into 2.32? would be nice to have that fix [08:30] pstolowski: is the status that it just needs a re-review? [08:36] mvo, yes, it needs re-review from Gustavo, but I applied the snippet he suggested (with only minor change), so perhaps you can make a call [08:36] pstolowski: thanks, this now looks very nice, I think its uncontroversial [08:36] PR snapd#4584 closed: hooks/strutil: limit the number of data read from the hooks to avoid oom [08:37] \o/ [08:37] mvo, shall I cherry-pick it and prepare a PR? [08:37] PR snapd#4727 opened: many: simplify mocking of home-on-NFS [08:37] mvo this is a mostly red follow up I was talking about [08:38] this should also help with one of jdstrand's branches that adds overlayfs data to system key [08:38] zyga: my understanding is that 4140 just needs the rename and then things are ok? if so, sounds like something worth doing for 2.32 - wdyt? [08:39] zyga: it has +1 from jamie and you already and gustavo only objected the name afaict [08:39] yes, I agree [08:39] I can handle that if you want [08:44] ok, I'll carry on with mount business [08:45] zyga: either way is fine [08:54] mvo: hi, how are things? when will we cut 2.32 ? [08:56] pedronis: once tests are green for the default-provider PR, looks like the store is a bit unhappy currently, but maybe it fixed itself [08:56] pedronis: anything you want in there? [08:58] mvo: no, mostly wondering about my reorg/refactor PR that are green, whether I should hold them for post 2.32 [08:58] or not [08:58] mvo: you can look, #4715 #4722 [08:58] PR #4715: store: reorg auth refresh [08:58] PR #4722: store: cleanup test naming, dropping remoteRepo and UbuntuStore(Repository)? references [08:59] pedronis: holding it for a little bit would be great [09:05] PR snapd#4728 opened: store: move infoFromRemote into details.go close to snapDetails [09:08] mo'in peeps [09:09] hey Chipaca ! good morning [09:12] Chipaca: hi, if I understood the discussion yesterday, this would make refreshed/InstalledDate report a value closer to the intention: https://pastebin.ubuntu.com/p/4G8TDSQFsr/ [09:13] ? [09:13] as you said with that change no test fails, at least in daemon [09:17] :/ [09:19] jdstrand: I was renaming the gnome-online-accounts-service to accounts-services as requested by gustavo. it looks like the store (basedeclartion) and review tools need an update (store is rejecting my test snap right now) [09:19] zyga: do you know if it is safe to just override this via manual review -^ [09:21] mvo I don't know if it is safe but I suspect that's what manual review is for [09:24] pedronis: I've got a branch already on spread for this [09:24] pedronis: and, well, close [09:24] pedronis: that'd work for squashes [09:24] pedronis: for it to work for everything you'd want it to be Lstat [09:25] my branch does that, but … then it takes a stand :-) [09:25] any known issue with lxd today? got 2 failures https://api.travis-ci.org/v3/job/345155633/log.txt [09:25] Chipaca: anyway probably time to move this to methods on snap.Info I think [09:25] whatever the stand [09:26] Chipaca: I'm mostly interested in installedDate for snaps from the store [09:26] fwiw [09:27] pedronis: I think you'll like my PR [09:27] pstolowski look at this [09:27] E: Type 'curity' is not known on line 50 in source list /etc/apt/sources.list [09:27] mborzecki: why do you sometimes ask 'merge?' on PRs that are green and have two +1s? [09:27] "security" became "curity" [09:29] Chipaca: ahh, cause there's +1 with some comments, so just double checking that the changes i pushed are ok for those who reviewed [09:29] zyga, missed that, thanks. interesting [09:29] mborzecki: for myself, if I've +1'ed it with nits, it's because (a) i won't block the landing even if you don't fix them, and (b) i trust you to fix them; also (c) i'll shout if you mess it up and (d) git reset --hard @^^^^^ [09:29] hahah ok :) [09:31] zyga, that's inside the container, i don't think we generate sources.list do we? [09:31] I don't know [09:33] PR snapd#4103 closed: snapstate: auto install default-providers for content snaps [09:34] PR snapd#4677 closed: cmd/snap: introduce `snap run --timer` [09:35] PR snapd#4680 closed: snap: pass full timer spec in `snap run --timer` [09:39] PR snapd#4729 opened: many: drop snaps' InstallDate, introduce Updated [09:39] pedronis: ^_^ [09:40] zyga: pstolowski: can you take another look at #4695 ? [09:40] PR #4695: wrappers: generator for systemd OnCalendar schedules [09:40] Chipaca: I have bad news for you [09:41] pedronis: is it about potato fungus [09:43] Chipaca: you expect last updated to be the creation time of the revision? [09:43] on the store [09:44] pedronis: it … seemed to be; isn't it? [09:47] I don't know [09:47] pedronis: bah, I guess it might be the creation time of the last revision, which might not be the one being pointed at? [09:47] bu that's what the new api has [09:47] pedronis: that == creation time of the revision? [09:47] that's perfect :-) [09:47] yes, but for the new api [09:47] right [09:47] the old I don't know [09:48] I can check [09:48] mborzecki, sure [09:48] just that this value might be a bit confusing for remote for a bit [09:48] pedronis: please check (no rush); depending on what it is, the confusion might not be terrible [09:49] for example if it's the timestamp of the latest revision, i don't mind [09:49] if it's the timestamp of the last time you changed something via the web, i do mind :-) [09:51] Chipaca: no, it's the timestamp of the creation of the revision under consideration also for the old api [09:52] at least as server by modern infra [09:52] * Chipaca dances [09:52] (it might have been something else at some point in the past) [09:52] Chipaca: your PR and my last PR will conflict :/ [09:52] pedronis: I don't mind deconflicting [09:52] pedronis: hurry up and land it :-D [09:53] mine is small (if spread out) [09:53] mvo told me to wait [09:53] anyway I need reviews, but is trivial: #4728 [09:53] PR #4728: store: move infoFromRemote into details.go close to snapDetails [09:53] psh, who listens to mvo [09:53] pedronis: conflict! :-) [09:53] not on that PR [09:54] I suppose on the based one [09:54] or maybe yes [09:54] ah, conflict with mvo stuff [09:54] I'll have fun for a bit I fear [09:55] mmh, not, it's really only this one [09:55] I'll just wait at this point [09:56] I could stack mine on yours [09:56] that'd make it a fun review for people [09:56] I hear they really like wading through huge changes [09:57] Chipaca: my in progress new api stuff is +-3000 lines (I will split it, but a chunk will still be +1000 mostly tests) [09:58] I shall endeavour to nitpick it to death in detail [10:06] prereqSuite.TestDoPrereqRetryWhenBaseInFlight failed on master https://paste.ubuntu.com/p/qwbkSCGm3K/ [10:07] i've restarted the travis build, see if reproduces again [10:08] I saw a few failures related to store hicckups todaty [10:08] *today [10:08] but nothing major [10:08] (just annoying) [10:10] fatal: unable to access 'https://go.googlesource.com/crypto/': The requested URL returned error: 502 [10:10] if that's any consolation, I've got a failure related to google hiccup [10:12] Chipaca: couple of initial small comments [10:12] pedronis: I'm not sure my 'open question' is clear: I thought of having it show "updated" (or even "last-updated") for remotes, but that leaves locals with "refreshed" which IMHO isn't ideal either [10:12] update is strange for remotes [10:13] let me think [10:13] * Chipaca lets pedronis think, and goes off to physio [10:13] Chipaca: I don't think we want to print something for remotes [10:13] we would want dates in the channel [10:13] s [10:14] we don't print a top revision either [10:14] for remote infos [10:14] it's a bit strange to put a date there without the revision [10:16] Chipaca: are you seeing what I'm referring to? [10:17] I think we do want dates in the map, yes [10:17] I'm saying if you put a date at top-level [10:17] the confusing munge of revisioned and revisionless info in the store results is confusing, yes [10:17] is very unclear what it refers to [10:18] unless you mention the channel [10:18] Chipaca on your way, can you look at the idea behind https://github.com/snapcore/snapd/pull/4727 [10:18] PR #4727: many: simplify mocking of home-on-NFS [10:18] but I do think having it is valuable, as long as it refers to what the user gets when they just ask for the snap without qualifying it [10:18] (which is, i think, what it does) [10:18] just mocking the function instead of what the function depends on [10:18] Chipaca: last-stable-updated ? [10:19] I just fear whatever we do right now, we will regret/have to change it again [10:20] Chipaca: or you could think how you want dates in the channels, and start putting in only the one for stable? [10:21] that'd work rather well actually [10:21] bah [10:21] sorta-kinda [10:21] but also I don't know [10:21] how to add a date [10:21] there [10:21] without thinking as breaking format [10:21] pedronis: note I try to use the exact same layout for 'installed:' as I do for channels [10:21] but it'd be weird for that to change when you installed the snap [10:22] hmmmm [10:22] yes, it's a bit the problem I mentioned, it is strange to use one field for a value that is available remotely [10:22] but turns into a different value locally [10:23] pedronis: how about, for local snaps just leave it as 'refreshed', and for remotes just add the one for the latest stable as a comment to channels [10:23] might be ok [10:23] ie: channels: # latest stable from YYYY-mm-ddTHH:MM:SSZZZ [10:24] ah [10:24] like that [10:24] * pedronis was confusing notes and comments [10:24] pedronis: do you think 'updated' is ok in the api? [10:24] it's getting a baroque [10:24] * Chipaca prefers the term 'neo-rococo' [10:25] :) [10:26] Chipaca: my question is whether we want different fields for local vs remote [10:27] that's annoying in different ways though [10:28] I mean, if we're going for full clarity we'd go with "last-stable-revision-created-on" and "snap-downloaded-on" and "try-created-on" [10:28] ¯\_(ツ)_/¯ [10:31] 4th failure on trivial PR :/ [10:31] oh, this time racy unit tests [10:31] FAIL: handlers_prereq_test.go:155: prereqSuite.TestDoPrereqRetryWhenBaseInFlight [10:32] handlers_prereq_test.go:186: [10:32] // check that t is not done yet, it must wait for core [10:32] c.Check(t.Status(), Equals, state.DoingStatus) [10:32] ... obtained state.Status = 4 ("Done") [10:32] ... expected state.Status = 3 ("Doing") [10:32] ok, so it's only me seeing this [10:34] reproducible locally [10:43] mvo, pedronis https://forum.snapcraft.io/t/using-content-for-a-role-system-data-partition-makes-it-not-be-system-data-anymore [10:46] sounds a ubuntu-image issue [10:46] I don't think prepare-image deals at level [10:47] mborzecki: uh, that race looks like I caused it. sorry for this [10:47] *at that level [10:53] mvo: are you looking into it or should i? [10:53] mborzecki: I am not looking right now, need to prepare lunch in a wee bit :( [10:53] mvo: ok, i'll take a look then [10:54] mborzecki I'm not looking either, sorry [11:01] mborzecki: thank you! if you get stuck let me know [11:19] PR snapd#4730 opened: userd/tests: Test kdialog calls and mock kdialog too to make tests work in KDE [11:19] mvo, can you take a look at #4730 ^ ? [11:19] PR #4730: userd/tests: Test kdialog calls and mock kdialog too to make tests work in KDE [11:19] we fail on KDE currently :} [11:31] sigh [11:31] (not about kde) [11:38] gaaah [11:39] mvo, the prereq unit test seems to be flaky [11:39] mvo, https://pastebin.ubuntu.com/p/k5MpksjvrN/ [11:39] so [11:39] mvo, (this is from master) [11:39] I now feel I need to implement a little bit of path traversing, mount table traversing logic in userspace [11:40] mvo, also got this failure in travis a moment ago on my pr - https://api.travis-ci.org/v3/job/345185500/log.txt [11:40] the simple check grew a little bit that I started to write prose comment that barely fits on my 27" screen :/ [11:41] pstolowski mborzecki ran into it too [11:41] (and me too) [11:41] zyga, ah indeed, just looked at the backlog [11:41] pstolowski: i'm looking into it atm [11:41] mborzecki, great, thanks [11:42] jdstrand I'm a bit depressed again :) [11:42] about that mount verification [11:42] it's one notch harder than I thought yesterday [11:42] and that's with constraints I don't know if are reasonable (not a generic solution for sure) [11:43] this is all because there's no frelling MS_NOFOLLOW in mount [11:44] * cachio_ afk [11:56] ok, so here's a small problem with that test, it assumes that tasks are run in the runner in a specific order, however the runner takes the list of tasks to run by calling State.Tasks(), interlly State.tasks is a map, so when you range the order is not guaranteed, and so it happens that sometimes one task runs before the other [11:56] mborzecki, is there a reason we don't translate mon-fri to mon..fri, but expand all the days? not biggie at all, just curious; i guess it's just more straightforward to always expand? [11:57] pstolowski: yes, easier to expand [11:57] mborzecki, in certain cases we force the order by task.WaitFor(another task) [11:58] mborzecki, ack, that's fine, thanks [11:58] pstolowski: i think the idea in this case is not to use waitfor, but rather detect that a change we need is in flight and retry later [11:59] mborzecki, right, i see that now [12:01] mborzecki, perhaps we need a dummy task that holds link-snap task, then we mark it done in a controlled way, and that unblocks prereq task [12:01] mborzecki nice analysis [12:08] * pstolowski lunch [12:23] zyga, ondra: bug#1750059 might be of interest to you [12:23] for different reasons [12:25] you know that thing where you plan to replace a bit of hardware because it's getting old, and it immediately starts showing even more signs of old age, as if resenting its replacement? my notebook now chirps when i adjust the brightness [12:25] Chipaca :) [12:25] Chipaca thanKs for bug info! [12:26] ondra: integration-y thing with a workaround, thought you'd like to have it on the radar [12:29] mborzecki: pstolowski: I think s.snapmgr.AddAdhocTaskHandler can probably be abused to get a controllable link-snap [12:29] for that test [12:31] Chipaca ack, queued [12:31] pedronis: and return state.Retry{} in the handler? [12:31] mborzecki: or wait on a channel before returning [12:32] pedronis: right, i've started adding some mocks to taskrunner, but your suggestion may be better [12:32] Chipaca interesting [12:33] I wonder if there's a way to tell systemd "this service uses that path" [12:34] mborzecki: is just a way to get to call AddTaskHandler again, that is already there... the latter seems not to worry about overriding. is used in a very controlled way so seems fine as is [12:34] sorry AddHandler [12:35] Chipaca: one of the BindsTo or one of it's friends perhaps? [12:35] pedronis, so looking through the sources i'm not that sure it is an ubuntu-image issue ... prepare-image unpacks the gadget but does not move the content to the "image" dir ... https://paste.ubuntu.com/p/6MbNxJ6574/ [12:36] mborzecki: MagicallyFrobnicatesInto [12:37] mborzecki: somebody should write https://git-man-page-generator.lokaltog.net/ but for systemd directives [12:40] jdstrand around? [12:42] ogra_: afaik the only bit that prepare-image is the bootloader conf, and cloud bits [12:42] *prepare-image copies [12:42] pedronis, right thats the issue i guess [12:43] if i'm allowed to define "content:" in the yaml for the writable partition it should copy that too ... [12:43] it does even look at Volumes [12:43] ... and if i'm not allowed it should error out and not silently swallow the files [12:43] it's a ubuntu-image problem [12:43] as far as I understand [12:44] content: is definitely a problem of ubuntu-image [12:44] we don't even read the gadget.yaml afaict [12:44] anyway, mvo may be a better person to discuss this with [12:44] k [12:48] anyone interested in reading a bit of prose about an algorithm I'm writing [12:48] to spot any logic holes? [12:49] zyga: o/ [12:50] pstolowski: thanks, I will look at the prereq unit test, mborzecki did some analyisis as well [12:50] ogra_: I have a look at the forum post in a little bit [12:53] Chipaca https://pastebin.ubuntu.com/p/Jnw6KSHsrj/ [12:54] mvo, thx [12:54] Chipaca this will be in a PR sometime after standup, I'm still working on the logic below the comment [12:54] s/ant/and/ [12:54] (in the text) [12:54] btw .. for everyones friday entertainment ... https://github.com/npm/npm/issues/19883 [12:55] ogra_: sudo thing? [12:55] mborzecki, well, npm thing :) [12:56] (randomly changing permissions if it can ... if sudo is used it changes them to the uid of the calling user, not to root ... if you do it as root user everything becomes 777 root:root ) [12:58] wow [12:59] havoc [13:00] zyga: is the second column in mountinfo the minor? [13:00] no [13:00] parent mount id [13:00] zyga: ah, mount id, parent, major:minor? [13:00] correct [13:01] zyga: I followed that explanation [13:01] Chipaca standup :) [13:01] zyga: as an intro it's nice [13:01] zyga: missing chapter 2, the attack, and chapter 3, the narrow escape [13:02] Sorry, running late for the standup [13:02] Will be there in ~2min [13:02] Chipaca and I should name characters better [13:05] zyga, did you get a chance to fix the new error from gcc8? [13:09] Son_Goku no, sorry, I will task switch to that as soon as the standup is over [13:09] I'm sorry about that, I'll ensure it builds on f27 [13:19] mvo: yes, they would. let me look [13:19] jdstrand hey :) [13:19] jdstrand I will have some things to discuss with you today [13:19] one more important than other, let me know when it would be a good time for you [13:19] zyga_: hey, saw your pings [13:20]