| mborzecki | morning | 05:03 |
|---|---|---|
| mborzecki | snap instance user directory mappings are a bummer | 07:25 |
| mvo | mborzecki: is it hard to do? | 07:26 |
| mvo | mborzecki: btw, does 5561 need a sameule/gustavo review or could this go in with two reviews? | 07:27 |
| mborzecki | mvo: the setup is ugly, it already relies on SNAP_USER_DATA being set, now it's even worse | 07:27 |
| mborzecki | mvo: it'd be great if someone did a 2/3rd review perhaps | 07:28 |
| mvo | mborzecki: ok | 07:35 |
| mvo | mborzecki: I hope to find time for reviews today/tomorrow | 07:35 |
| mborzecki | mvo: thanks | 07:35 |
| mborzecki | meanwhile i'll try to find a way to do user mappings in s-c that does not invovlve exploding the number of env vars passed from snap and does not call getpewnt | 07:36 |
| mvo | mborzecki: sounds good | 07:37 |
| mborzecki | mvo: the silly thing about this part is that since it's bind mounting stuff it needs to run with elevated privs, this makes relying too much on env vars is a bit scary | 07:37 |
| Chipaca | moin moin | 07:46 |
| Chipaca | mborzecki: if you could take a look at https://github.com/snapcore/snapd/pull/5591 it'd be great: it's a fix for a bug found in spread tests of https://github.com/snapcore/snapd/pull/5580 | 07:47 |
| mborzecki | Chipaca: looking | 07:47 |
| Chipaca | mborzecki: thanks | 07:48 |
| mborzecki | Chipaca: high order math, +2 < 2 +1 :) | 07:50 |
| Chipaca | mborzecki: IKR | 07:50 |
| Chipaca | the maths was there because the +6 was rather mysterious | 07:51 |
| Chipaca | but, not that much, i dunno | 07:51 |
| Chipaca | i add comments when requested :-) | 07:51 |
| mvo | kyrofa: hey, 5569 probably needs a master merge. I tried to do that for you but it appears that you disabled the ability for maintainers to push to your branches(?) | 08:06 |
| mvo | kyrofa: master merge to make tests great^Wgreen again | 08:07 |
| mvo | some easy wins: 5548 and 5450 (both relatively short) | 08:10 |
| mborzecki | opened https://github.com/snapcore/snapd/pull/5594 for snapenv, i suppose it might be a bit controversial | 08:16 |
| Chipaca | mvo: in looking at #5450, I saw a bit of code that looked suspicious | 08:34 |
| Chipaca | mvo: in Put, if the Chtimes fails with an IsNotExist, we re-link the file | 08:34 |
| Chipaca | mvo: but we don't re-Chtimes the file | 08:35 |
| mvo | Chipaca: aha, let me have a look | 08:44 |
| Chipaca | mvo: a few more comments there | 08:55 |
| Chipaca | mvo: overall looks good though | 08:55 |
| mvo | Chipaca: yeah, nice one on the hardLinkCount, thats indeed a bit silly | 08:57 |
| Chipaca | mvo: that's the one that stopped me from +1'ing it :-) | 08:58 |
| Chipaca | the others are a non-rhetoric question, and a nit | 08:58 |
| mvo | Chipaca: yeah, on it. thank you! | 08:59 |
| Chipaca | mvo: the cache's Count isn't used outside of tests, right? | 09:00 |
| mvo | Chipaca: correct, we can probably unexport it | 09:01 |
| Chipaca | mvo: was the cache size a config? | 09:01 |
| Chipaca | mvo: i was more worried about it being used in a loop somewhere, as it's not particularly efficient if the cache is arbitrarily sized | 09:02 |
| Chipaca | if the cache size is not configurable, it's less of a worry | 09:02 |
| mvo | Chipaca: it is not configurable right now (the cache count) | 09:03 |
| mvo | Chipaca: aha, yes, building a list of the content for the size is silly if the size is large | 09:04 |
| mvo | Chipaca: I can: a) add a FIXME b) rework | 09:04 |
| Chipaca | mvo: (a) is fine -- and it'll only be an issue if the list is big | 09:04 |
| Chipaca | mvo: OTOH | 09:04 |
| Chipaca | mvo: why even count n>1 files in the cache size? | 09:05 |
| mvo | Chipaca: heh, yeah, maybe it should simply be "cacheTooBig() bool" instead of count | 09:05 |
| mvo | (well, proper name of course) | 09:05 |
| Chipaca | maybe we want to invert that: first step, remove n>1 files from fis; _then_ see if it's too big, sort it, etc | 09:06 |
| mvo | Chipaca: hm, not sure I get the idea but let me first address the other comments and then I read it again and ponder about it | 09:08 |
| Chipaca | mvo: i mean https://pastebin.ubuntu.com/p/wnrzqp72Gb/ | 09:16 |
| Chipaca | er, except the last loop should be on filtered :-) | 09:16 |
| Chipaca | of course, as hardLinkCount is just an accessor, it probably makes more sense to do that twice instead of copying all the file infos over | 09:18 |
| Chipaca | lovely morning for bikeshedding, isn't it? | 09:18 |
| mvo | Chipaca: aha, I see what you mean, let me poke at it a bit more | 09:19 |
| mvo | Chipaca: it is lovely! and the cache gets better with every iteration :) | 09:20 |
| Chipaca | mvo: https://pastebin.ubuntu.com/p/H9fwdVrb7d/ | 09:20 |
| mvo | Chipaca: yeah, feel free to pull and commit, this looks nice | 09:21 |
| Chipaca | mvo: and the 'are we ready yet' condition further down would need a tweak also | 09:21 |
| Chipaca | mvo: ah ok, will do | 09:21 |
| mvo | Chipaca: I would keep the comment about the link count and that we get stuff for free if the link count is > 1 | 09:21 |
| mvo | Chipaca: but other than that I think this is nice and clean | 09:21 |
| Chipaca | mvo: yeah, i was trying to explain the idea, not provide a diff :-D | 09:21 |
| Chipaca | mvo: is "numOwned" the right name for that count? | 09:22 |
| mvo | Chipaca: I can take your diff and run with it as well, I did not want to make you work, just had the impression it was "done" | 09:25 |
| mvo | Chipaca: yeah, I think numOwned is good, trying to think of anything more descriptive | 09:26 |
| mvo | Chipaca: I think this is a good name | 09:26 |
| Chipaca | mvo: 550d714b179733cc8e96bd2c91a1890af3613d69 | 09:30 |
| Chipaca | oh wait, it's already there | 09:31 |
| Chipaca | mvo: I pushed it to your repo :-D | 09:31 |
| Chipaca | I thought I could only do that when it was a PR | 09:31 |
| Chipaca | gah, github needs to refresh my gpg keys | 09:32 |
| mvo | Chipaca: tests are unhappy now - not sure if that is a bug in the tests or in the code. off by one it seems | 09:32 |
| Chipaca | mvo: a bug? in code I wrote?! inconceivable! | 09:35 |
| === cjwatson_ is now known as cjwatson | ||
| Chipaca | mvo: ah, not a bug, just tests not updated for the smarter cache :-D | 09:47 |
| mvo | Chipaca: oh? whats the issue with the test? | 09:50 |
| mvo | Chipaca: aha, count needs an update as well? to return just unowned? or what is the best fix? | 09:54 |
| Chipaca | mvo: looking | 09:55 |
| mvo | ok | 09:55 |
| mvo | mborzecki: when you have some spare cycles, 5540 is probably another easy win, just needs a second review | 09:56 |
| mborzecki | mvo: ack | 09:57 |
| Chipaca | mvo: this might actually be a bug somewhere | 09:59 |
| Chipaca | mvo: in TestPutMany, I print the Nlink of every file, and the last one is always 2 | 09:59 |
| Chipaca | mvo: https://pastebin.ubuntu.com/p/rZFjtd39Sw/ | 10:00 |
| Chipaca | mvo: so it doesn't get cleaned up, so Count() > maxItems | 10:00 |
| Chipaca | mvo: but the question is why is nlink>1 | 10:00 |
| Chipaca | is something holding the file open? | 10:01 |
| mvo | Chipaca: hm, let me double check iirc its using iotutil.WriteFile() for the temp file creation | 10:01 |
| Chipaca | actually holding it open doesn't increase nlink | 10:02 |
| Chipaca | hmm | 10:02 |
| mvo | Chipaca: is this maybe simply an ordering problem? the last file is put in and it exists as a test file. now put sees it has a link count of 2 (one the test file and the file in the cache) and skips it | 10:02 |
| mvo | Chipaca: the remove of the test file is after the put | 10:02 |
| mvo | Chipaca: so probably just a confusing test | 10:02 |
| Chipaca | oh drat | 10:03 |
| Chipaca | mvo: i'm counting before the remove | 10:03 |
| * Chipaca is dumb :-) | 10:03 | |
| * Chipaca moves it and tries again | 10:03 | |
| mvo | Chipaca: its complicated | 10:03 |
| mvo | Chipaca: I mean life and computers and all that | 10:03 |
| Chipaca | mvo: relationships also | 10:03 |
| mvo | Chipaca: *cough* wise words! | 10:04 |
| Chipaca | mvo: so, yes, it's that at Put time the count is 2 | 10:04 |
| mvo | Chipaca: so all good, we just need to update the test and add a comment I guess | 10:05 |
| Chipaca | mvo: should I, or will you? | 10:06 |
| mvo | Chipaca: either way is fine, let me do it, I pulled you deep enough into this rabbithole already | 10:06 |
| * Chipaca puts down his "100 ways of cooking rabbit" book | 10:06 | |
| mvo | Chipaca: update pushed, I will merge once tests are green | 10:11 |
| mvo | 5576 is also a trivial win | 10:12 |
| Chipaca | mvo: 'has a link count of 2 because it still exists outside of the cache dir so its is not counted' sounds like you're talking about Count(), when the link count of 2 and it not being counted is at Put time | 10:12 |
| mvo | Chipaca: hm, good point, let me try to correct this | 10:18 |
| mvo | Chipaca: I force pushed an updated version | 10:19 |
| Chipaca | mvo: s/has/had/, s/its/it's/, but only because I'm at nit central and it's nit breeding season | 10:20 |
| Chipaca | ah | 10:21 |
| Chipaca | mvo: s/its is/it's/ | 10:21 |
| Chipaca | :) | 10:21 |
| mvo | Chipaca: heh, force pushed again | 10:22 |
| * Chipaca is busy with a technical issue involving biscuits and coffee in a mug that is too narrow for dunking | 10:23 | |
| mvo | down to 36 PRs and counting! | 10:25 |
| mborzecki | hm i'm missing a bit about apparmor, `aa-complain /snap/core/5183/usr/lib/snapd/snap-confine` should be enough to put it in complain mode for snap-confine right? | 10:56 |
| Chipaca | mvo: 35 | 11:33 |
| cachio | mvo, https://paste.ubuntu.com/p/rWZNvyVXpc/ | 11:36 |
| cachio | I was taking a look to the PR which fix that | 11:38 |
| cachio | mvo, #5440 | 11:38 |
| xnox | 2018-08-01 20:43:25 Failed tasks: 1 | 11:40 |
| xnox | - autopkgtest:ubuntu-18.10-arm64:tests/main/auto-refresh | 11:40 |
| xnox | i hope this is a fluke, retrying cosmic snapd autopkgtest | 11:40 |
| ogra | xnox, yo ... how good is your journald fu ? (i'm looking for someone who has some insight into it) | 11:43 |
| Chipaca | mvo: mborzecki: https://gist.github.com/chipaca/911a8a4905b091c10caa9854bf7ea4b4 | 11:44 |
| ogra | not sure you saw my questions in the backlog in #ubuntu-devel from last night | 11:44 |
| xnox | ogra, nothing beyond what the manpage says..... but you can try | 11:44 |
| ogra | xnox, well, so i always thought journald uses a ringbuffer in ram by default ... but found out that this isnt true ... it actually logs to a file in /run ... and by default has no upper limit set | 11:45 |
| mborzecki | aare expert needed, @{HOME}/snap/*_*/ does not match /home/test/snap/test-snapd-tools_foo/ ? | 11:50 |
| mvo | mborzecki: thats surprising - jdstrand will know why @{HOME}/snap/*_*/ does not match /home/test/snap/test-snapd-tools_foo/ ? | 11:52 |
| mborzecki | i get apparmor="DENIED" operation="mount" info="failed srcname match" error=-13 profile="/snap/core/5183/usr/lib/snapd/snap-confine" name="/home/test/snap/test-snapd-tools/" pid=24324 comm="snap-confine" srcname="/home/test/snap/test-snapd-tools_foo/" flags="rw, rbind" | 11:53 |
| mvo | cachio: is the fix for refresh.timer=managed incomplete? | 11:54 |
| Saviq | hmm refreshing a snap just now, snapd seems stuck at "Copy snap... data" (there's a whole of 512 bytes in $SNAP_DATA...) | 11:57 |
| Saviq | that a known issue? | 11:57 |
| cachio | mvo, well, it is not working | 12:00 |
| cachio | we received an email yesterday about this issue | 12:00 |
| cachio | mvo, sudo snap set system refresh.timer=managed | 12:00 |
| cachio | it is not working neither | 12:01 |
| mborzecki | Saviq: can you do snap change --last=refresh? maybe its waiting for some dependency | 12:01 |
| mvo | cachio: thanks, checking | 12:02 |
| cachio | mvo, thanks | 12:06 |
| mvo | cachio: will send a pr shortly, quite annoying, the validation is in the way | 12:09 |
| cachio | mvo, thanks | 12:11 |
| cachio | mvo, I could make a new spread test for that, or update one | 12:13 |
| mvo | cachio: I'm updating the existing one as part of the fix | 12:13 |
| cachio | mvo, great :) | 12:13 |
| mvo | cachio: but yeah, that was the problem, the previous change lacked a full end-to-end test | 12:13 |
| mvo | cachio: I think I missed the mail from yesterday about this, who send it? | 12:14 |
| Saviq | mborzecki: sorry, missed it, seems to have refreshed after all | 12:14 |
| cachio | mvo, it was a reply for the announcement which I did saying the 2.34.3 was promoted to candidate | 12:15 |
| cachio | mvo, Tony Espy sent it | 12:15 |
| mborzecki | Saviq: can you post the output nonetheless? it's interesting if there was some dep after all | 12:15 |
| Saviq | mborzecki: http://paste.ubuntu.com/p/c8cXZhTRv3/ | 12:16 |
| mborzecki | Saviq: when that happens (i mean a task is waiting for some dep), the prompt is seemingly stuck, probably bad ux on our side | 12:16 |
| Saviq | so it took 16 mins | 12:17 |
| mborzecki | Chipaca: do you remember of 'ready' column is when the task is done? | 12:18 |
| Chipaca | mborzecki: yes | 12:18 |
| Chipaca | spawntime is when the task was created, readytime is when it's finished | 12:19 |
| mborzecki | so maybe it did take that long after all | 12:19 |
| Saviq | http://paste.ubuntu.com/p/DRWwF5hptt/ | 12:20 |
| Chipaca | mborzecki: well, maybe | 12:20 |
| Saviq | 280MB... | 12:20 |
| Chipaca | we're probably missing a 'start time' to know exactly what happened | 12:21 |
| Chipaca | OTOH if you run with SNAPD_DEBUG=1 you'd know more | 12:21 |
| jdstrand | mborzecki: can you show me the full denial and the full rule that doesn't match? | 12:21 |
| jdstrand | (for the '_') | 12:21 |
| Chipaca | Saviq: were you running with SNAPD_DEBUG=1? | 12:22 |
| mborzecki | jdstrand: https://github.com/bboozzoo/snapd/commit/07de54fe9b126923dbc15251a09fbda0a8f84204 and https://paste.ubuntu.com/p/y4kNfM734Y/ | 12:22 |
| mborzecki | jdstrand: the other 2 rules for /snap/*_* and /var/snap/*_* seem to match, i was getting denials before and now they are gone | 12:23 |
| Saviq | Chipaca: I'm afraid no | 12:24 |
| mborzecki | heh weird failure with MATCH https://paste.ubuntu.com/p/M3rXxHD35Y/, obviously works if running in the debug shell | 12:27 |
| jdstrand | mborzecki: you are hitting https://bugs.launchpad.net/ubuntu/+source/apparmor/+bug/1612393 | 12:27 |
| mborzecki | jdstrand: so just /home/*/snap.. ? | 12:28 |
| thresh | neat, gcloud packaged in snap | 12:28 |
| * thresh apt removes it | 12:28 | |
| mvo | cachio: 5595 fixes the refresh.timer issue for real this time | 12:30 |
| jdstrand | mborzecki: by default, /home/*/snap/*_*/ and /root/snap/*_*/. but see /etc/apparmor.d/tunables/home (and home.d/*) if the user updated the tunables for some other location | 12:30 |
| jdstrand | mborzecki: that said, snap-confine itself falls over if people use an alternate location for home (ie, they tried to adjust home.d/something so probably should mention LP 1612393 and also that snap-confine doesn't support other locations for home.d in a code comment | 12:32 |
| cachio | mvo, great, thanks, I'll take a look now | 12:32 |
| jdstrand | mborzecki: and move on | 12:32 |
| mborzecki | jdstrand: had to use 'mount options=(rw rbind) /home/*/snap/*_*/ -> /home/*/snap/*/,' | 12:32 |
| mborzecki | jdstrand: and it sort of works, thanks for the help! :) | 12:33 |
| jdstrand | oh right, you aren't mounting on /home/*/snap/*/ | 12:33 |
| jdstrand | err | 12:33 |
| jdstrand | you aren't mounting on /home/*/snap/*_*/ | 12:33 |
| jdstrand | you are mount from foo_parallel to foo | 12:33 |
| jdstrand | so that makes sense | 12:33 |
| ogra | jdstrand, regarding the device-tree/model access i think it should live in the same interface that allows /proc/cpuinfo (whichever that is) | 12:34 |
| ogra | IMHO they are equivalent | 12:35 |
| jdstrand | ogra: that is allowed by default in the base abstraction because it is used by glibc's sysconf | 12:37 |
| ogra | bah ... damned :) | 12:37 |
| ogra | i thought i found a clever criteria ;) | 12:37 |
| mborzecki | parallel installs integration branch https://github.com/snapcore/snapd/pull/5596 | 12:38 |
| ogra | *criterium | 12:38 |
| Chipaca | mvo: I'm looking at 5592 | 12:38 |
| ogra | *criterion ?? | 12:38 |
| ogra | hmm | 12:38 |
| jdstrand | ogra: I mean, we can add it anywhere we want. there are basically two questions: what fails if it isn't allowed? how sensitive is the information? | 12:38 |
| Chipaca | mvo: without this, would the broken snapd snap install succeed? | 12:38 |
| Chipaca | ogra: critteropolis | 12:39 |
| ogra | jdstrand, nothing fails, thats really optional but allows you to i.e. distinguish on which specific Pi model you run | 12:39 |
| jdstrand | ogra: it seems like very few things fail otherwise we would have heard about it much sooner | 12:39 |
| jdstrand | ok | 12:39 |
| ogra | yeah, nothing fails ... but it would be good to be able to get to that info | 12:40 |
| jdstrand | sure | 12:40 |
| jdstrand | it'll be added | 12:40 |
| ogra | thx | 12:40 |
| jdstrand | $ cat /proc/device-tree/model | 12:40 |
| jdstrand | TI AM335x BeagleBone Black | 12:40 |
| ogra | (i'm working towards a unified Pi image that runs on all Pi's and there it would be come ore important) | 12:40 |
| jdstrand | ok, that isn't very sensitive | 12:40 |
| * jdstrand nods | 12:41 | |
| jdstrand | I saw the thread. sounds neat | 12:41 |
| ogra | its the same as "Hardware:" in /proc/cpuinfo in most cases i think | 12:41 |
| jdstrand | Hardware: Generic AM33XX (Flattened Device Tree) | 12:41 |
| ogra | ah, k ... so it is actually more secific | 12:41 |
| ogra | *specific | 12:42 |
| jdstrand | *barerly* | 12:42 |
| ogra | Hardware: BCM2709 | 12:42 |
| jdstrand | barely* | 12:42 |
| ogra | on a pi3 | 12:42 |
| jdstrand | huh, on pi 2 it is quite different | 12:43 |
| jdstrand | barely*$ grep Hardware /proc/cpuinfo | 12:43 |
| jdstrand | Hardware: BCM2709 | 12:43 |
| jdstrand | meh | 12:43 |
| jdstrand | $ grep Hardware /proc/cpuinfo | 12:43 |
| jdstrand | Hardware: BCM2709 | 12:43 |
| jdstrand | $ cat /proc/device-tree/model | 12:43 |
| jdstrand | Raspberry Pi 2 Model B Rev 1.1 | 12:43 |
| ogra | well, they use the same CPU core ... but differet SoC peripherials | 12:43 |
| jdstrand | but obviously someone can create a map having only BCM2709 | 12:43 |
| ogra | so here the devicetree model is actually helpful | 12:44 |
| jdstrand | ogra: this doesn't sound sensitive at all with the things that we already allow by default | 12:44 |
| ogra | ah, you mean we could allow it without interface | 12:44 |
| ogra | fine with me as well | 12:44 |
| jdstrand | that is what I'm thinking | 12:44 |
| ogra | yeah | 12:44 |
| mvo | Chipaca: yes | 12:44 |
| ogra | if cpuinfo is accessible the model should be too | 12:44 |
| jdstrand | ogra: is it generally useful? like, will a library ever look at that to try to optimize something? | 12:45 |
| mvo | Chipaca: so without 5592 a broken (or empty) snapd snap would lead to snapd stopping but no new snapd in place | 12:45 |
| ogra | dunno, i guess they'd rather use cpabilities from /sys or some such ... | 12:45 |
| jdstrand | ogra: well, I'll do some more investigating | 12:46 |
| ogra | at least on a deb/distro evel you typically dont have per cpu optimization but have the binaries rather look up single capabilities | 12:46 |
| ogra | t is definitely useful info for i.e. wrapper scripts in snaps | 12:46 |
| jdstrand | yep | 12:47 |
| jdstrand | ogra: oh, it is actually already allowed by hardware-observe | 12:48 |
| jdstrand | @{PROC}/device-tree/{,**} r, | 12:48 |
| ogra | oh, wow ... it allows all of it even | 12:48 |
| ogra | perfect | 12:49 |
| jdstrand | (it was added to support 'lshw -quiet') | 12:49 |
| Chipaca | mvo: conflcits in 5592 fwiw | 12:55 |
| mvo | Chipaca: yeah, on it | 12:56 |
| mvo | Chipaca: just testing locally a bit | 12:57 |
| Chipaca | ouf, 2pm and i haven't had lunch | 13:00 |
| mborzecki | https://paste.ubuntu.com/p/M3rXxHD35Y/ | 13:03 |
| jdstrand | mvo: hey, is the forum ok? I got only 2 emails from it last night. nothing after 01:29 UTC until 09:25 UTC. then got one at 09:58 UTC and then nothing since | 13:08 |
| jdstrand | mvo: I'm still looking to see if it is local | 13:09 |
| ogra | there wasnt much ... probably because most people in europe are simly melting and not sitting on computers :) | 13:14 |
| jdstrand | ogra: ok, that confirms it at least isn't local (I couldn't find it to be either) | 13:22 |
| jdstrand | 2 emails over night. that is unprecedented :) | 13:22 |
| ogra | i dont have cnstant emailing set up (only for direct notifications/mentions) but in general there wasnt much traffic tonight | 13:22 |
| jdstrand | I have constant | 13:25 |
| mborzecki | mvo: Chipaca: replaced MATCH with grep -qE in the first line the test failed and it failed on the next line | 13:31 |
| Chipaca | mborzecki: remind me what was the pr? | 13:40 |
| Chipaca | mborzecki: Ran for 1 hr 1 min 13 sec ! | 13:42 |
| * Chipaca runs the test by hand | 13:43 | |
| mborzecki | Chipaca: https://github.com/snapcore/snapd/pull/5594 | 13:47 |
| mborzecki | Chipaca: tests/main/snap-env | 13:47 |
| mvo | mborzecki: hm, maybe a subtle bug in the new MATCH since it got fixed? | 13:54 |
| mvo | mborzecki: are you in a debug shell? if so, what do you get with declare -f MATCH? | 13:55 |
| mvo | mborzecki: the fact that there is no echo at all is a bit strange | 13:56 |
| Chipaca | mvo: running MATCH by hand works | 13:56 |
| Chipaca | that is, it passes | 13:57 |
| mborzecki | mvo: Chipaca: added set -x in MATCH https://paste.ubuntu.com/p/fRbXRQFVKk/ and it's not failing now | 13:57 |
| mborzecki | hmm | 13:57 |
| mborzecki | i'll try an earlier commit in spread | 14:01 |
| mvo | mborzecki: yeah, try the one before "echo | grep" was replaced with <<< | 14:04 |
| mborzecki | mvo: mhm, doing this right now | 14:04 |
| mborzecki | mvo: older revision is also failing | 14:09 |
| mvo | mborzecki: "good" I guess | 14:09 |
| mvo | mborzecki: also WTH? | 14:10 |
| mvo | mborzecki: what exit code do you get when it fails? | 14:10 |
| mvo | mborzecki: I mean right after match failure what is $? ? | 14:10 |
| Chipaca | mborzecki: you say ou added 'set -x' and it started passing? | 14:11 |
| Chipaca | mborzecki: because I overwrote MATCH in task.yaml itself, and it failed because of the wc -l test | 14:12 |
| Chipaca | because snap-vars.txt has 18 lines, now, not 12 | 14:12 |
| mborzecki | Chipaca: yes, in MATCH() https://paste.ubuntu.com/p/fRbXRQFVKk/ | 14:12 |
| mborzecki | Chipaca: yup, so it went throught the MATCH lines that were failing previously | 14:12 |
| Chipaca | ah, yes | 14:12 |
| mborzecki | Chipaca: what do you mean you rewrote it? copy & paste to task.yaml directly? | 14:13 |
| Chipaca | yah | 14:14 |
| Chipaca | but, i can reproduce the failure | 14:16 |
| mvo | local stdin=\"$(cat) <- this failing might make the script fail without any message | 14:16 |
| Chipaca | by which I mean: python3 -c $'import yaml;y=yaml.load(open("task.yaml"));\nfor i in ("execute","prepare","restore"):\n if i in y: open(i+".sh", "w").write(y[i])' | 14:17 |
| Chipaca | and the ( set -eux; source execute.sh ) | 14:17 |
| Chipaca | and i see it fail in the same way | 14:17 |
| mvo | Chipaca: you see it always failing the same way? | 14:18 |
| Chipaca | yes | 14:18 |
| mvo | Chipaca: you say "-x" there, does that yield anything useful? | 14:19 |
| Chipaca | mvo: if I overwrite MATCH with one that doesn't +x, the tests pass | 14:20 |
| Chipaca | mvo: if I add a add -v, the tests pass | 14:20 |
| mvo | Chipaca: can you add strace? | 14:20 |
| mvo | Chipaca: i.e. strace the entire beast with -f -s1024 | 14:21 |
| mvo | Chipaca: and then we can look for execve and wait and sigchild | 14:21 |
| Chipaca | hmmmmmmmmm | 14:21 |
| mborzecki | mvo: your favourite topic https://lwn.net/Articles/760584/ | 14:21 |
| mvo | Chipaca: and hopefully get a clue | 14:21 |
| mvo | mborzecki: *garrr* | 14:21 |
| Chipaca | mvo: if I add "echo HELLO" at the top of MATCH, I see 11 HELLOs, even when it fails | 14:22 |
| * cachio afk | 14:22 | |
| mvo | Chipaca: *cough* wuut? 11? | 14:23 |
| Chipaca | mvo: so the MATCHes are passing | 14:24 |
| Chipaca | mvo: we're just not seeing the output | 14:24 |
| Chipaca | 11 is where the first known-to-fail test is | 14:24 |
| mvo | Chipaca: hm, ok. if you remove the fist 11 that work, I assume nothing changes and the 12th still fails? can that one be straced :) ? | 14:25 |
| * mvo would love to see the strace | 14:25 | |
| Chipaca | mvo: the first one that fails is not a MATCH but the test | 14:25 |
| Chipaca | test $(wc -l < snap-vars.txt) -eq 12 | 14:25 |
| Chipaca | ^ that one | 14:25 |
| mvo | Chipaca: oh, sorry, I misunderstood this then. so match is fine for some reason we just don't see that it is "test $(wc -l)" that is failing? | 14:27 |
| mvo | Chipaca: if you add an "echo" before the test, is that visible in the failure output? | 14:27 |
| mborzecki | heh | 14:27 |
| Chipaca | mvo: before the "test $(wc -l < snap-vars.txt) -eq 12" line? | 14:29 |
| mvo | Chipaca: yeah, just curious if the echo (and possible output) is also eaten up | 14:30 |
| Chipaca | mvo: echo HELLO, and echo HELLO STDERR >&2, both work | 14:30 |
| Chipaca | right before the test line | 14:30 |
| mvo | Chipaca: interessting | 14:31 |
| Chipaca | mvo: things that also work: making the funciton run in a subshell (i.e. MATCH() ( … ) instead of MATCH() { … } | 14:31 |
| kyrofa | mvo, interesting, no, that box is checked | 14:32 |
| mvo | kyrofa: aha, then I was maybe just confused (its very hot here). anyway, I can merge master for you or you can do it, that should fix the spread tests (they were a bit unhappy in the past days) | 14:33 |
| mvo | Chipaca: confusing | 14:33 |
| kyrofa | mvo, happy to do it, the button isn't showing up for me either so I'll do it manually | 14:33 |
| kyrofa | Github might be a little sad right now | 14:33 |
| kyrofa | mvo, done | 14:35 |
| mvo | kyrofa: ta | 14:36 |
| mvo | Chipaca: do you have a reproducer that you could pastebin for lazy people like me to just run? or is it complicated? | 14:36 |
| Chipaca | mvo: working on it | 14:38 |
| mvo | Chipaca: ta! | 14:40 |
| mborzecki | https://pastebin.com/EBFtyBnr 'not seen' is actually seen, but after the first match the second one does not appear | 14:44 |
| Chipaca | mvo: got a reproducer | 14:47 |
| Chipaca | mvo: now working on minimising it a bit | 14:48 |
| mborzecki | Chipaca: https://pastebin.com/raw/4nzDYLRf | 14:49 |
| Chipaca | mborzecki: https://pastebin.ubuntu.com/p/D3nbpBtvMw/ | 14:49 |
| Chipaca | mvo: ^ | 14:50 |
| mvo | Chipaca: nice | 14:51 |
| mvo | mborzecki: nice as well :) | 14:51 |
| mvo | Chipaca: ok, so without "<<<" (replaced that with pipes) things seems to work | 14:53 |
| Chipaca | mvo: https://pastebin.ubuntu.com/p/7kyVQ373hy/ | 14:53 |
| Chipaca | mvo: changing 'gep <<< "$(cat)"' to 'cat | grep' fails in the same way | 14:54 |
| mvo | Chipaca: removing " { | 14:55 |
| mvo | set +ux | 14:55 |
| mvo | } 2> /dev/null" fixes it for me | 14:55 |
| Chipaca | mvo: as does dropping the x from that, or doing a set -v | 14:55 |
| mvo | Chipaca: egon@bod:~/devel/go/src/github.com/snapcore/snapd.mvo$ pastebinit /tmp/lala4.sh http://paste.ubuntu.com/p/GDJZCGz2CY/ | 14:55 |
| Chipaca | mvo: wa | 14:56 |
| Chipaca | haha lel | 14:56 |
| Chipaca | is this a new feature in bash | 14:56 |
| mborzecki | magic | 14:56 |
| Chipaca | mborzecki: you mean maciej | 14:57 |
| mborzecki | shall we fix it in spread then? | 14:58 |
| Chipaca | mborzecki: fix it how? | 14:58 |
| Chipaca | that bit of code is to avoid having debug output from MATCH itself | 14:58 |
| Chipaca | easiest fix is probably to do the subshell thing | 14:58 |
| Chipaca | behaviour-preserving fix i mean | 14:58 |
| mvo | hm, hm, let me check something | 14:59 |
| mborzecki | meanwhile pushed to fix to change 12 to 18, higher order algebra | 15:00 |
| mborzecki | btw. funny quote from the entropy pool article | 15:01 |
| mborzecki | > Laura Abbott was set to test the patch on a Fedora Rawhide kernel, but found that the hang at boot time waiting for random numbers .. | 15:01 |
| mvo | Chipaca: yeah, I was wondering if we need shopt or something but subshell seems least intrusive | 15:01 |
| mvo | mborzecki: yeah, I was reading this too, sad sad | 15:02 |
| Chipaca | mborzecki: wrt your PR, fix the wc -l test and it should pass | 15:11 |
| mborzecki | Chipaca: yup | 15:11 |
| Chipaca | the MATCH bug should not affect successful test results | 15:11 |
| mborzecki | https://i.imgur.com/CB7qWD6.jpg 45C | 15:11 |
| mvo | Chipaca: interesstingly http://paste.ubuntu.com/p/h72s4vVyB4/ works, so the pipe seems to trigger some cleanup in the stderr redirect | 15:12 |
| Chipaca | mvo: I'm pretty sure this counts as a bug in bash, fwiw | 15:13 |
| Chipaca | OTOH given how it behaves the same in 14.04 through 18.04, they might say it's working as intended (somehow) | 15:13 |
| Chipaca | OTO²H it wouldn't be the first antediluvian bug we've found | 15:14 |
| mvo | Chipaca: fun, looks like their bugtracker is a mailinglist | 15:24 |
| mvo | Chipaca: hm, let me quickly run an idea by you: this is our bug: we do "set +ux" in MATCH which is a function so no subshell. we run MATCH foo <<< foo which will also not create a subshell. at this point we did set +x which means we truned of tracing globally. when doing "echo foo|MATCH something" this will run match inside a subshell which means the traceing that is turned off does not affect our main shell. makes sense? | 15:31 |
| mvo | mborzecki: your input on the above is of course appreciated as well | 15:32 |
| mvo | mborzecki: 45°C?!? uhhh | 15:32 |
| Chipaca | mvo: makes sense | 15:33 |
| mvo | Chipaca: which means the fix is more () (you can never have enough): http://paste.ubuntu.com/p/jZJ5ySrCpV/ | 15:34 |
| Chipaca | mvo: I meant: MATCH() ( … ) | 15:34 |
| Chipaca | mvo: instead of MATCH() { … } | 15:35 |
| mvo | Chipaca: aha, yes! | 15:35 |
| mvo | Chipaca: are you on it already? or shall i do a spread fix? | 15:35 |
| Chipaca | mvo: go for it | 15:35 |
| mvo | Chipaca: thanks | 15:35 |
| mvo | Chipaca: I guess this is what you had in mind all along when you said "lets just use a subshell" earlier? | 15:36 |
| Chipaca | <Chipaca> mvo: things that also work: making the funciton run in a subshell (i.e. MATCH() ( … ) instead of MATCH() { … } | 15:36 |
| Chipaca | mvo: ^there? :-) | 15:36 |
| mvo | Chipaca: yes | 15:36 |
| Chipaca | mvo: yes | 15:36 |
| mvo | Chipaca: *cough* I should read more carefully :) thank you, nice that we have yet another bug squashed | 15:37 |
| cachio | mvo, https://paste.ubuntu.com/p/Xn2vN9N7hz/ | 15:42 |
| cachio | here: https://travis-ci.org/snapcore/snapd/builds/411259659?utm_source=github_status&utm_medium=notification | 15:42 |
| mvo | cachio: this looks like an empty reply, no? | 15:43 |
| * mvo needs to be afk for some minutes | 15:43 | |
| cachio | don't know | 15:44 |
| cachio | because mborzecki saw the same in another test | 15:44 |
| cachio | mborzecki, empty reply or we are not managing well in MATCH function | 15:45 |
| cachio | I am trying to reproduce it locally | 15:45 |
| cachio | mvo, I'll keep you updated | 15:45 |
| mvo | cachio: you may want to try my latest spread PR | 16:13 |
| cachio | mvo, sure | 16:23 |
| cachio | mvo, I was wondering if it is already possible to create a core-18 image on edge? | 16:24 |
| cachio | using ubuntu-image | 16:24 |
| kyrofa | noise][, I'm getting a 504 trying to get snap metrics, status seems all green | 16:49 |
| noise][ | kyrofa: what URL? | 16:52 |
| kyrofa | noise][, https://snapcraft.io/account/snaps/nextcloud/metrics | 16:55 |
| noise][ | looks like it's timing out, working on other snaps. Can you please file a bug? | 16:56 |
| kyrofa | noise][, done: https://bugs.launchpad.net/snapstore/+bug/1785094 | 16:58 |
| noise][ | thx, we'll dig in. | 16:59 |
| cachio | mvo, I could reproduce the issue using your spread PR | 17:09 |
| FreeBDSM | snap is too complex | 17:14 |
| FreeBDSM | I use mozilla's script that built firefox esr 60+ | 17:14 |
| FreeBDSM | I've used it to build 52.9.0-esr | 17:14 |
| FreeBDSM | it built fine, it runs and works, but instead of pages I see nothing | 17:14 |
| kyrofa | mvo, yikes: https://api.travis-ci.org/v3/job/411316584/log.txt | 17:44 |
| kyrofa | mvo, that's https://github.com/snapcore/snapd/pull/5569 after merging with master :P | 17:45 |
| === Caelum_ is now known as Caelum | ||
| ads20000 | popey Wimpress is it possible to use custom Wine patches within your tmnationsforever Wine schema? :) | 19:34 |
| ads20000 | (well, I guess it is possible, just wondering if there's an easy way of doing that... presumably I bundle the .patch files in the repo and then put in patch -pi < foo.patch somewhere in the .yaml but I don't know where...) | 19:35 |
| mvo | cachio: sorry for the delay. hm, sad to hear that spread MATCH still has the same issue | 19:48 |
| mvo | cachio: re core18 image, let me mail you some details | 19:48 |
| cachio | mvo, np | 19:48 |
| cachio | mvo, nice, thanks | 19:48 |
| mvo | cachio: see /msg | 19:50 |
| roadmr | mvo: you're still up! can I trouble you a bit? | 19:53 |
| mvo | roadmr: sure | 20:06 |
| roadmr | mvo: two things! 1) I checked that seeds repository seb pointed me to, but can't find where snap:core is included. Is that implicit somewhere if the image has any snap:whatevers? | 20:07 |
| roadmr | and 2) how is the revision of the snap that gets baked in the image determined? is it mentioned anywhere, or a simple snapshot of what was currently on stable when the image was built? | 20:08 |
| mvo | roadmr: I haven't looked at the seed but I'm pretty sure its implicit | 20:08 |
| roadmr | ok... yes, that sounds plausible for core | 20:08 |
| mvo | roadmr: (2) not 100% sure but about 80% that its whatever is in stable at the image building time | 20:08 |
| roadmr | mvo: the reason we want this is: we want to know which revisions of snaps are included in images (isos and maybe cloud images) so we can ensure deltas to go from that version to the latest stable are always available | 20:09 |
| mvo | roadmr: everything else would also be difficult because the builder cannot access snaps by revision | 20:09 |
| roadmr | mvo: right - oh that's a very good point. | 20:09 |
| mvo | roadmr: aha, I see | 20:09 |
| roadmr | then I guess it's what's on stable... which leads to - do you know if there's a build log or something which tells us which version ended up in the image? | 20:09 |
| mvo | roadmr: the images usually have a manifest, let me see if I can find the LP builders | 20:09 |
| roadmr | ah exactly :) thanks!! | 20:09 |
| mvo | roadmr: I need to find it :) but yeah, there is one | 20:10 |
| roadmr | mvo: for example - the download to upgrade an 18.04 image's core was 91 MB; yesterday we built the deltas and download size went down to 27 mb | 20:10 |
| roadmr | anyone booting 18.04 and waiting until the snaps refresh will benefit from this, starting yesterday | 20:10 |
| roadmr | mvo: no rush/problem, if you happen to remember where they are that'd be useful | 20:11 |
| roadmr | mvo: as is, we have to look at our metrics and react when we see lots of misses for a snap. Not intractable but we'd like something more preemptive/authoritative. | 20:11 |
| mvo | roadmr: https://launchpad.net/~ubuntu-cdimage/+livefs/ubuntu/bionic/ubuntu has the builds and the build logs | 20:18 |
| roadmr | mvo: marvelous :) thanks! | 20:18 |
| mvo | roadmr: my pleasure | 20:18 |
| mvo | has anyone seen "- Fetch and check assertions for snap "core" (5183) (cannot get device session from store: store server returned status 400 and body "{\"error_list\":[{\"code\":null,\"message\":\"Nonce is missing or invalid.\"}],\"errors\":[\"Nonce is missing or invalid.\"],\"result\":\"error\"}\n")" | 20:21 |
| roadmr | mvo: hmm is that reproducible? when a device gets a session from the store, it does a handshake that includes a nonce and it has to send that back as part of the dance | 20:24 |
| roadmr | mvo: it usually just works, but if this is happening consistently it might need a look | 20:24 |
| mvo | roadmr: thanks! its the first time I saw it, probably a one-off issue then, I retriggered the test now | 20:26 |
| * mvo crosses finggers | 20:26 | |
| roadmr | mvo: hope it works, but let me know if not. | 20:26 |
| roadmr | (might have been a memcached hiccup; but that hasn't been an issue in ages) | 20:26 |
| robert_ancell | Has anyone talked to Gitlab about making snaps? It would be pretty awesome to 'snap install gitlab-runner' | 23:01 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!
