mupPR # closed: snapd#3734, snapd#3916, snapd#3963, snapd#3992, snapd#3994, snapd#3998, snapd#4013, snapd#4040, snapd#4049, snapd#4059, snapd#4063, snapd#4064, snapd#4068, snapd#4073, snapd#4076, snapd#4078, snapd#4100, snapd#4102, snapd#4103, snapd#4104, snapd#410600:03
mupPR # opened: snapd#3734, snapd#3916, snapd#3963, snapd#3992, snapd#3994, snapd#3998, snapd#4013, snapd#4040, snapd#4049, snapd#4059, snapd#4063, snapd#4064, snapd#4068, snapd#4073, snapd#4076, snapd#4078, snapd#4100, snapd#4102, snapd#4103, snapd#4104, snapd#410600:04
Son_Gokuthat's... a lot of pulls01:13
=== ikey is now known as ikey|zzz
=== JoshStrobl is now known as JoshStrobl|Away
=== JoshStrobl|Away is now known as JoshStrobl
zyga-ubuntugood morning07:28
zyga-ubuntuman, it's a nice and sunny 1C day over here :)07:28
zyga-ubuntuquiet day...08:03
zyga-ubuntu /exit08:19
zyga-solusone IRC is enough today08:19
=== LarreaMikel1 is now known as LarreaMikel
mupPR snapd#4108 opened: repo: ConnectedPlug and ConnectedSlot types <Created by stolowski> <https://github.com/snapcore/snapd/pull/4108>09:01
zyga-soluspstolowski: hey, thank you for 410809:05
zyga-soluspstolowski: do you think you could write some text in the PR description to help reviewers understand what is going on more clearly?09:05
pstolowskizyga-solus, added a few more info to the second paragraph of the description, not sure if there is anything else unclear (the link to the forum topic is also helpful)09:11
zyga-soluspstolowski: "add new data types for connected plugs/slots"?09:12
zyga-solusthis is a bit terse for ~1K diff09:12
pedronispstolowski: hi, what's blocking #4013 ?09:13
mupPR #4013: repo, daemon: use PlugInfo, SlotInfo <Created by stolowski> <https://github.com/snapcore/snapd/pull/4013>09:13
pstolowskizyga-solus, see pt. 5 of the plan09:13
zyga-soluspstolowski: I think that kind of stuff should be in the PR09:13
pedronisit will not be that big in 4013 gets in09:14
pedroniss/in 4013/if 4013/09:14
pstolowskizyga-solus, ah, sorry. i was still referring to the description of 4013. you're talking about 4108.09:16
zyga-soluspstolowski: yes, sorry :)09:16
pstolowskizyga-solus, will expand it, yes09:16
zyga-solusthank you!09:16
pstolowskipedronis, i hope three is nothing blocking 4013 once zyga-solus approves09:16
pedronispstolowski: tests are red though?09:17
zyga-solusI will look at 4013 soon09:17
pstolowskipedronis, linode:ubuntu-core-16-64:tests/main/snap-seccomp failed, 1369 passed, 1 failure. it failed on matching libs with ldd.09:20
pstolowskipedronis, will restart them09:20
pstolowskizyga-solus, and yes, as pedronis said, 4108 will actually be small09:21
pedronisno mvo today?09:37
zyga-soluspedronis: no, he is off today and tomorrow09:38
ogra_pedronis, germany is celebrating nailing of paper to a church door today ...09:41
pedronishttps://en.wikipedia.org/wiki/Reformation_Day  (I think)09:43
ogra_Son_Goku, google martin luther ...09:43
Son_Gokuoh right09:43
Son_Gokuwe don't recognize it in the USA09:44
Son_Gokubut I knew it was a thing09:44
ogra_the northern parts of germany are all protestant (that is why we complain so much i guess)09:46
pedronisChipaca: hi, a review of #4106 would be appreciated when you have time10:16
mupPR #4106: many: lookup and use the URL from a store assertion if one is set for use <Created by pedronis> <https://github.com/snapcore/snapd/pull/4106>10:16
zyga-solusman, logging10:38
zyga-soluson the upside I get to the point where tmpfs is usable10:39
zyga-solusbut I found another place where logging breaks10:39
zyga-solusand what I do now feels like laying a set of extension cords across the stac k10:39
=== LarreaMikel1 is now known as LarreaMikel
* zyga-solus lunch11:26
jdstrandmwhudson: there have been a lot of issues with nvidia (re https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=880174). I'm not sure who has been focusing on those issues lately. historically it's been zyga-solus and mvo iirc12:30
jdstrandI know oSoMoN has been affected of late. I don't know if he has looked at it in depth12:31
=== wgrant_ is now known as wgrant
zyga-solusI think debian needs updates12:31
* sergiusens goes for a walk12:32
oSoMoNjdstrand, I have tried to get feedback from users with nvidia hw (I don't have any myself), but nothing conclusive so far12:46
flexiondotorgom26er: Would you like to move Android Studio to Snapcrafters?12:50
om26ersure @flexiondotorg12:55
flexiondotorgJust reply on the fourm.12:55
flexiondotorgI'm just replying on the forum :-)12:55
flexiondotorg@om26er I've not used ANdroid Studio myself. Do you know if it interfaces with fastboot or adb?12:57
nothalflexiondotorg: No such command!12:57
pedronisChipaca: standup?13:01
Chipacaoh hello 2fa13:08
Chipacano not even 2fa13:08
om26erflexiondotorg: it does ship adb/fastboot with it, yes.13:08
om26erwhen you say "interfaces" is that in the terminologies of snapd ?13:08
om26er(and sorry for the late reply, I was driving)13:08
flexiondotorgom26er: Thanks. I suspected as much.13:08
flexiondotorgYes, in the forum post I am referring to snap interfaces.13:09
=== ikey|zzz is now known as ikey
flexiondotorg@om26er No problem. I just got round to this on my job list :-)13:09
nothalflexiondotorg: No such command!13:09
om26erflexiondotorg: replied.13:12
flexiondotorgCheers. I'll import it when you've had the chance to add the README :-)13:12
zyga-soluscachio: so about debian, as I said I don't see the thing in sysfs but I didn't google or search in the kernel tree to see what makes that sysfs available13:16
cachiozyga-solus, ok, I'll try to see what is missing13:17
Chipacapedronis: AFAICT we are not doing gzip13:18
zyga-soluspstolowski: I'll get a coffee/tea and I'll do one more pass through 401313:18
pedronisChipaca: is it worth it? something to look into?13:18
pedronisat least for this API13:18
pstolowskizyga-solus, thanks!13:18
Chipacapedronis: AFAICT², golang's standard library doesn't do it -- i guess they figure you should just use http2?13:19
zyga-solusjdstrand: I have a tmpfs+bindmount fallback now but I need to address a few shortcomings (files and broken undo)13:19
zyga-solusjdstrand: I ran into a number of odd cases where denials are not logged13:19
zyga-solusjdstrand: what do I need to do to enable that?13:19
pedronisChipaca: fun13:19
Chipacapedronis: there was something about https + gzip == bad, but i don't remember13:19
pedronisChipaca: https://en.wikipedia.org/wiki/BREACH ?13:22
zyga-solus21st century, the day where security exploits are commonplace but at least we give a random one cool name, icon and website13:23
* sergiusens is back13:31
Chipacapedronis: sounds like it13:32
Chipacazyga-solus: we are tribal animals; names and stories are how we learn13:32
pedronisChipaca: anyway not clear the attack is relevant for the content of here, anyway in this case the request doesnt' strictly need to be https, that's just our default13:33
Chipacazyga-solus: also punches to the face, but i'd rather names and stories13:33
zyga-solusChipaca: we need to paint those cave walls with shellcode13:33
zyga-solusChipaca: don't forget chanting and smoking various substances ;)13:33
Chipacazyga-solus: sshhh13:34
om26erflexiondotorg: added the README, we can improve from there: https://github.com/om26er/android-studio-snap13:37
om26erso whats the update story for this ? When a new version arrives would I need to notify snapcrafters ?13:47
sergiusensstgraber do you know if that snapd/systemd nice issue is gone with the latest snapd requiring privileged containers?13:49
zyga-soluspstolowski: about 4013, can you please add docstrings to all the new public things?13:49
pstolowskizyga-solus, ok, doing13:50
zyga-soluspstolowski: I'll provide ideas13:50
zyga-soluspstolowski: if you wait ~20 min13:50
pstolowskizyga-solus, let me push something, just let me know if you have better descriptions13:52
zyga-soluson the upside, I found the missing SNAPD_DEBUG bridge13:54
Facusergiusens, elopio, may I get re-reviews on this? https://github.com/snapcore/snapcraft/pull/1634  thanks!14:01
mupPR snapcraft#1634: Push metadata to the Store <Created by facundobatista> <https://github.com/snapcore/snapcraft/pull/1634>14:01
Facuroadmr, ^14:01
mupPR snapcraft#1644 closed: lxd: fix the push in container builds <bug> <Created by elopio> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/1644>14:01
zyga-soluspstolowski: done14:03
zyga-solusand I fixed a silly bug in my code, swapped mount args :D14:04
jdstrandzyga-solus: if there are explicit deny rules, prepend them with 'audit' to log them (eg: audit deny /path/to/... r,). or disable rate limiting: sudo sysctl -w kernel.printk_ratelimit=014:08
zyga-solusrate limiting fixed it14:09
zyga-solusthank you!14:09
zyga-solusman, I feel like turning the light on14:09
zyga-solusworking in the blind was annoying14:09
zyga-solusjdstrand: so, any reason not to disable rate limiting in spread in general?14:11
jdstrandzyga-solus: I thought we were. istr sending something up that did that14:12
zyga-solusnope, I just got the logs to work when I ran the command myself14:12
zyga-solusI'll include this as a RFC PR to consider14:13
jdstrandzyga-solus: it is commented out of tests/lib/prepare.sh14:13
zyga-solusI see14:13
zyga-solusreading the forum14:13
zyga-solusthank you, I'll check that out after this PR14:14
zyga-solusjdstrand: do you hae time to look at one function today?14:14
zyga-solusjdstrand: not a security review, just a sanity check14:14
jdstrandzyga-solus: I can do that14:15
zyga-solusjdstrand: can you please fetch my remote and look at feature/transparent-tmpfs, look at cmd/snap-update-ns/utils.go createWritableMimic14:17
roadmrFacu: yay thanks!14:17
sergiusensFacu we are trying to wrap up our 2.35 milestone, I've marked that as 2.36 so it might be something for next week14:17
stgrabersergiusens: pretty sure the systemd SRU has been published14:19
Facusergiusens, perfect, thanks!14:20
* Chipaca needs to step out for a while, will bbl14:21
sergiusensstgraber great, I will try and remove our workarounds for travis and see how it goes14:26
zyga-ubuntu"cannot parse mountinfo of the current process14:39
zyga-solusnobody said it would be boring :)14:43
zyga-solusfound a cute mountinfo parsing bug now :)14:50
zyga-solusbut also, kernel is not hepful here :)14:50
kyrofaelopio, how is autopkgtest looking this morning?15:07
kyrofasnappy-m-o, autopkgtest 1583 xenial:amd6415:08
snappy-m-okyrofa: I've just triggered your test.15:08
elopiokyrofa: I really really hope it's good. I'll update the one for adt-lxd, It's so close...15:09
kyrofaGood deal. Everything I know about has been merged, so let's see...15:10
kyrofasergiusens, are you around? Can you create https://github.com/snapcraft-docs/ros2-talker-listener for me?15:11
sergiusenskyrofa sure15:22
sergiusenskyrofa done15:25
kyrofasergiusens, thank you!15:25
zyga-ubuntuit works :D15:35
zyga-ubuntujdstrand: it works :)15:38
zyga-ubuntuI'll break it down and start sending PRs up :)15:38
zyga-ubuntuI also worked aournd an apparent kernel bug15:39
zyga-ubuntujdstrand: I found that mount() can be called with empty source and it will not be quoted in any way, resulting in weird (and harder to parse) mountinfo15:39
zyga-ubuntuI now generate 'none' tmpfs so it's okay and no tools break but I will send a patch anyway15:40
zyga-ubuntuafter the cleanup I'll send my early tmpfs PR and start working on unit tests as there are none for the new code yet15:40
pstolowskiand now 4013 failed with error: cannot perform the following tasks:15:42
pstolowski- Download snap "test-snapd-tools" (6) from channel "stable" (Get https://api.snapcraft.io/api/v1/snaps/download/eFe8BTR5L5V9F7yHeMAPxkEr2NdUXMtw_6.snap: dial tcp i/o timeout15:42
zyga-ubuntupstolowski: store woes15:42
zyga-ubuntuwe had a lot over weekend15:42
jdstrandzyga-ubuntu: ack, cool15:45
=== genii is now known as genii-zombii
zyga-ubuntujdstrand: small one for you https://github.com/snapcore/snapd/pull/410916:03
mupPR #4109: cmd/libsnap: fix parsing of empty mountinfo fields <Created by zyga> <https://github.com/snapcore/snapd/pull/4109>16:03
mupPR snapd#4109 opened: cmd/libsnap: fix parsing of empty mountinfo fields <Created by zyga> <https://github.com/snapcore/snapd/pull/4109>16:04
om26eris there a way to refer the value gathered from `version-script` in snapcraft.yaml ? Think of $SNAPCRAFT_PROJECT_VERSION but for version-script16:14
om26ersergiusens: ^16:14
om26erSo I think that might be a bug, hmm.16:16
zyga-ubuntupstolowski: hey, I also pushed https://github.com/snapcore/snapd/pull/4104 if you want to have a quick look, it's very short and simple16:18
mupPR #4104: cmd/snap-update-ns: add logging to snap-update-ns <Created by zyga> <https://github.com/snapcore/snapd/pull/4104>16:18
pstolowskizyga-solus, yep, looking16:21
zyga-ubuntuthank you :)16:25
zyga-ubuntupstolowski: https://github.com/snapcore/snapd/pull/4109#discussion_r148051564 :-)16:31
mupPR #4109: cmd/libsnap: fix parsing of empty mountinfo fields <Created by zyga> <https://github.com/snapcore/snapd/pull/4109>16:31
pstolowskizyga-solus, thanks! reading16:31
zyga-ubuntupstolowski: if you think something is wrong please ask, maybe I'm confused myself16:38
zyga-ubuntuofftopic, tests seem rapid today16:43
sergiusensom26er no there isn't, it is for setting things, not reading16:44
om26ersergiusens: I have a use case: I want to dynamically read the latest version number of my software from a server url (version-script is fine for that). I then want that same version to be used by my source url in the `dump` plugin.16:46
om26erI would generally expect the value returned to from version-script to be exported as $SNAPCRAFT_PROJECT_VERSION not the fallback `version` that I have in place.16:47
om26ermy snap here could definitely use that: https://github.com/om26er/sublime-text-3-snap/blob/master/snap/snapcraft.yaml16:48
pstolowskizyga-solus, i find it hard to digest it at this late hour tbh ;)16:50
zyga-ubuntupstolowski: no worries, it's not urgent :)16:50
zyga-ubuntuthank you for looking16:50
zyga-ubuntucachio: hey, can you have a 2nd look at 4102 please?16:56
pstolowskizyga-solus, is it possible for 2 adjacent fields to be empty like that?16:56
cachiozyga-ubuntu, sure16:56
zyga-ubuntuthank you :)16:56
pstolowskizyga-ubuntu, , is it possible for 2 adjacent fields to be empty like that?16:59
zyga-ubuntupstolowski: I honestly don't know, this feels like a kernel bug16:59
zyga-ubuntupstolowski: the only case I encountered was tmpfs with "" mount source17:00
zyga-ubuntupstolowski: proc(5) doesn't mention this17:00
pstolowskizyga-ubuntu, yes, that would be unexepected but i'm curious if it deserver a test for the behavior of the parser (i.e. make sure we don't crash)17:00
zyga-ubuntuah, sure, I can add more tests17:00
zyga-ubuntugood idea17:00
om26erare build.snapcraft builders on maintenance of something ? My build haven't started for quite a bit.17:15
kyrofaom26er, things tend to slow down when a new Ubuntu release opens17:24
kyrofaelopio, all our tests use testtools nowadays, right? Is there a reason we're still running them with unittest?17:26
kyrofaI'm digging into this weird BlockingIOError thing. It seems to be coming from unittest, particularly a section of it that has been re-implemented by testtools, but we17:27
kyrofa're not using testtools to run17:27
elopiokyrofa all should be using testtools. Changing the runner sounds worth a try.17:29
kyrofaelopio, alright, I'm trying it17:29
kyrofaJust wanted to make sure I wasn't insane, thanks elopio :)17:29
sergiusensom26er well, version-script is only analyzed after prime so that won't be possible anyways17:30
om26erthat's an implementation detail :) As a developer I expected $SNAPCRAFT_PROJECT_VERSION to behave differently.17:31
sergiusensom26er well, the docs for version-script says it affects only the resulting metadata17:32
sergiusenskyrofa if that's it, nice find17:33
kyrofasergiusens, no guarantees, testing now17:33
* zyga-ubuntu is too tired now17:35
zyga-ubuntuback demands a break17:35
kyrofasergiusens, what I actually think is happening is that the test runners don't handle EAGAIN properly17:36
kyrofaWhich means we have a few options assuming testtools has the same issue: try to stay under whatever threshold we're hitting that's causing the EAGAIN, or fix the runner to handle it properly. It just so happens that one seems like a short-term fix and another seems like a long-term, so I'm looking into both17:41
=== cachio is now known as cachio_afk
sergiusensgot it17:46
stokachuanyone seen this error before "- Run configure hook of "conjure-up" snap if present (run hook "configure": / not root-owned 66094:40918)."17:50
stokachuthis is from a snap install conjure-up --classic on arm6417:50
stokachuwililupy: ^ is the one having the issue17:51
wililupystokachu: yes17:51
kyrofastokachu, yuck. jdstrand any idea what that ^ may be?18:00
jdstrandkyrofa: I have not18:03
* sergiusens is commuting on a bus18:11
wililupykyrofa,jdstrand: I can give you access to the server if that helps?18:17
kyrofawililupy, sure, I'm curious. pubkey: https://pastebin.ubuntu.com/25860353/18:18
kyrofawililupy, what the heck is wrong with this machine :P18:28
kyrofa$ ls -ld /18:28
kyrofadrwxr-xr-x 24 66094 40918 4096 Oct 31 16:23 /18:28
kyrofawililupy, there's a weird mixture of that in / as well18:28
kyrofawililupy, bin looks good, but boot is weird, etc.18:28
genii-zombiiodd UID/GID18:29
kyrofawililupy, not sure what the deal is, but snap-confine barfs at it definitely18:30
kyrofaFix that up and I bet it'll start working18:30
wililupyThats good info. I'll try to re-install the OS and see what happens. I'll keep you posted. Thanks18:31
kyrofaSure thing18:33
kyrofastokachu, FYI ^18:36
jdstrandkyrofa: probably 'owner' in the apparmor rules18:43
kyrofajdstrand, it's an explicit check in snap-confine18:44
kyrofajdstrand, ensuring the bpfpath is owned by root, presumably to ensure tampering would be difficult18:45
jdstrandoh yes18:45
jdstrandkyrofa: I should've remembered that from the error, but for some reason it didn't trigger in my head18:46
mupPR snapd#4102 closed: tests: refactor and expand content interface test <Created by zyga> <Merged by zyga> <https://github.com/snapcore/snapd/pull/4102>18:47
kyrofajdstrand, I imagine you have a few other things vying for space ;)18:47
om26ersergiusens: Hey! Have you considered renaming telegram-sergiusens to telegram ?18:58
* Pharaoh_Atem sighs19:06
* zyga-ubuntu hugs Pharaoh_Atem 19:06
zyga-ubuntuwhat's up?19:06
Pharaoh_AtemI want proper SELinux support in snapd already :(19:06
zyga-ubuntuI don't even know where to begin on that19:07
zyga-ubuntusnap-confine should probably do something but that is a deep-dive sprint19:07
* zyga-ubuntu goes to fix his manual pages 19:09
* zyga-ubuntu tries to learn manual page syntax19:10
=== cachio_afk is now known as cachio
* zyga-ubuntu reads groff source code19:21
nacczyga-ubuntu: good luck :)19:22
zyga-ubuntunacc: man, what is .tmac?19:31
nacczyga-ubuntu: troff macros19:33
zyga-ubuntuI'm trying to understand why I'm getting a warning:19:33
zyga-ubuntumdoc warning: .Lb: no description for library `libkeep' available (#8)19:33
mupPR #8: launchpad.net/snappy -> github.com/ubuntu-core/snappy <Created by chipaca> <Merged by elopio> <https://github.com/snapcore/snapd/pull/8>19:33
naccsilly mup19:34
nacczyga-ubuntu: can you pastebin the source?19:34
zyga-ubuntuof the man page?19:34
nacczyga-ubuntu: yeah19:34
zyga-ubuntuI'm a bit shy, it's my pet project19:34
zyga-ubuntuthe relevant part is19:34
zyga-ubuntu.Sh LIBRARY19:34
zyga-ubuntu.Lb libkeep19:34
nacczyga-ubuntu: does that possibly mean there is no libkeep manpage?19:35
zyga-ubuntuthat depends, it's in my source tree19:36
zyga-ubuntuin section .319:36
naccit looks like it can't find it locally as it's running?19:36
naccie d doc-str-Lb-\*[doc-arg\n[doc-arg-ptr]]19:36
zyga-ubuntuaha, I probably need to install that19:36
zyga-ubuntuI read that file but I didn't undetstand the syntax yet19:37
naccit's rough :)19:37
zyga-ubuntuI found this by grepping, following from man source, looking for --warnings19:37
zyga-ubuntuI use this command to run the "checker"19:37
nacczyga-ubuntu: ah, see `man groff_mdoc` for valid .Lb values19:38
zyga-ubuntuPAGER=cat LANG='en_US.UTF-8' MANWIDTH=80 man --warnings -E UTF-8 -l keep_alloc_free.3 >/dev/null19:38
zyga-ubuntuthank you19:38
nacczyga-ubuntu: you might need a mdoc.local?19:38
naccand need to defien a str-Lb-libkeep, i think19:39
zyga-ubuntuwhere does mdoc.local live?19:39
nacczyga-ubuntu: /etc/groff/mdoc.local apparently19:40
naccso that doesn't help you much :/19:40
zyga-ubuntuwell, man page looks ok, that's one I'm willing to ignore19:40
zyga-ubuntuthank you for teaching me about tihs19:42
nacczyga-ubuntu: np, learninng right alongside you :)19:43
kyrofaHrmm... setuptools masks output19:52
kyrofaEr, I mean testtools19:53
kyrofaToo many tools19:53
wililupykyrofa: I re-installed the OS and I was able to install conjure-up. Good thing too. There were a bunch of updates that system wasn't seeing as well.19:53
kyrofawililupy, good deal, yeah I imagine that would cause all sorts of issues19:53
kyrofawililupy, thanks for getting back to me!19:53
wililupykyrofa: np. thanks for the help.19:54
kyrofaOf course19:54
stokachuwililupy: good to hear!20:04
kyrofasergiusens, elopio testtools.run -h lies about the options supported-- there is no verbose mode :P20:18
kyrofaJust look at this bug, how embarrassing https://bugs.launchpad.net/testtools/+bug/87290620:19
mupBug #872906: python -m testtools.run discover -v doesn't set verbosity <runner> <testtools:Triaged> <https://launchpad.net/bugs/872906>20:19
kyrofaSo we of course timeout20:20
kyrofaThe investigation continues20:23
elopiokyrofa: we can easily make our own runner, printing whatever we want. For example, I combined some things from unittest and testtools to make the concurrent runner.20:23
sergiusensoh well, you know, maybe thomi can give you some ideas on your issue20:23
thomiI have been summoned!20:24
elopiokyrofa also, once we move all the tests into snapcraft/tests, we don't need discover. We can just run the module.20:25
thomikyrofa: yeah, the 'discover' command is a bit of a hack, and ideally shouldn't be used on the command line20:26
kyrofaActually, thomi if you have a sec, could you give me your thoughts on the bug that's sucking the lifeblood out of me?20:27
thomisure thing20:28
mupBug #1717921: CI: BlockingIOError about 50% of the time <Snapcraft:Confirmed> <https://launchpad.net/bugs/1717921>20:28
kyrofathomi, note that the error always seems to come from unittest's runner20:28
* thomi reads20:29
kyrofathomi, I was hoping the testtools runner would fare better, but without verbose mode Travis just times out thinking it died20:30
thomikyrofa: Travis requires some output on stdout or it kills the job?20:31
kyrofathomi, yep20:31
kyrofaFun times20:31
kyrofaThere are ways to hack around it, but without output it's difficult to track progress as well20:32
thomihuh, and the individual tests are longer than the timeout? Even in non-verbose mode, testtools will print a '.' after every test20:32
kyrofaSome, yeah20:32
thomikyrofa: Are you able to point me to the code where you setup the subprocess calls? I think you're correct that this is related, and I suspect what's happening is that the Process stdout or stderr pipes are filling up20:34
kyrofathomi, most subprocess calls go through common.run{_output}, defined here https://github.com/snapcore/snapcraft/blob/master/snapcraft/internal/common.py#L6620:36
kyrofathomi, we're using check_call or check_output though, each of which handle reading off their own stdout/stderr20:38
kyrofa(at least I thought)20:38
thomihmmm, yes, it seems it should20:44
thomiI was pretty sure there was a warning in the python docs about ensuring process stdout pipes didn't get full, but I can't see it now, maybe it was a python2 consideration only20:49
kyrofathomi, that's for setting stdout or stderr=PIPE with check_call20:50
kyrofaIn which case the caller is responsible to emptying them20:50
thomicheck_output simply calls run with stdout=PIPE. Does the same consideration not apply there as well?20:51
kyrofathomi, run is fine assuming it also called communicate afterward, no?20:51
kyrofaYeah here's the warning on check_call: "20:51
kyrofaDo not use stdout=PIPE or stderr=PIPE with this function. The child process will block if it generates enough output to a pipe to fill up the OS pipe buffer as the pipes are not being read from. "20:51
kyrofaHere: https://docs.python.org/3/library/subprocess.html#subprocess.check_call20:51
thomiI see - check_call is implemented by calling `call()`, but `check_output` is implemented in terms of `run` and `Popen`20:52
thomikyrofa: this only happens on travis?20:54
kyrofathomi, yeah20:54
kyrofaSuper weird20:54
kyrofaAlthough I'm not running the entire suite locally, maybe there's something odd that happens when the whole shebang runs20:55
=== JoshStrobl is now known as JoshStrobl|zzz
thomiI'd be tempted at this point to try and create a minimal reproducer in a separate repo. It'd be interesting to see what's required to trigger it consistently20:56
kyrofaYeah that's not a bad idea20:56
kyrofaI wonder if lxc might be involved as well20:56
thomiif it is, that's at least easy to test locally20:57
kyrofathomi, anyway, I don't mean to suck your lifeblood too, but if you could mull it over in the back of your mind I'd be thankful20:57
thomiyeah - I will - let me know if you make any progress, you've got me interested now.20:57
kyrofaWill do! I'll set up a test repo shortly and see if I can manage to get it happening20:58
thomiIf you want an easy way to create lots of tests (assuming for the moment that lots of tests running in parallel is part of what's required to trigger this) look at `testscenarios` - you can programmatically multiply a single test N times.20:58
mupPR snapcraft#1641 closed: lxd: catch CalledProcessError in _container_run <bug> <Created by kalikiana> <Closed by sergiusens> <https://github.com/snapcore/snapcraft/pull/1641>20:59
mupPR snapcraft#1647 opened: lxd: better surfacing of errors <Created by sergiusens> <https://github.com/snapcore/snapcraft/pull/1647>20:59
sergiusenselopio kyrofa ^21:00
jdstrandtyhicks: hey, I noticed in https://github.com/snapcore/snapd/pull/3998 that you are using EPERM. this is consistent with all of our conversations, but I forgot until recently that apparmor and DAC return EACCES. I wonder if it would make sense to use EACCES instead? note that device cgroup uses EPERM21:21
mupPR #3998: snap-confine, snap-seccomp: utilize new seccomp logging features <Created by tyhicks> <https://github.com/snapcore/snapd/pull/3998>21:21
elopiosnappy-m-o autopkgtest 1647 xenial:amd64 xenial:arm64 xenial:armhf21:24
snappy-m-oelopio: I've just triggered your test.21:25
tyhicksjdstrand: I don't really have an opinion on the matter but I'll be happy to talk through it21:33
tyhicksjdstrand: it may require a case-by-case evaluation per syscall21:34
tyhicksjdstrand: the man pages for setuid(), setresuid(), etc., don't mention EACCES as a possible return value but do mention EPERM21:36
jdstrandtyhicks: oh hmm21:36
jdstrandtyhicks: does apparmor retrun !EACCES depending on the access?21:37
tyhicksjdstrand: I don't think it is possible for an LSM to cause EACESS to be returned for those syscalls (I may be wrong about that...)21:37
tyhicksjdstrand: I'm not sure off the top of my head21:37
zyga-ubuntujdstrand: I recall -1321:41
zyga-ubuntuwhich is indeed EACCES21:41
jdstrandzyga-ubuntu: yes, I looked at this recently for open, but I hadn't considered that they might set errno differently depending on syscall21:42
jdstrandtyhicks: well, ultimately it doesn't matter. I was asking for consistency21:42
jdstrandtyhicks: and I did notice that apparmor and device cgroup were different with open21:43
tyhicksjdstrand: I don't necessarily think that they do set errno different depending on the syscall21:43
jdstrandEACCES vs EPERM21:43
tyhicksjdstrand: rather, I think there's a subset of syscalls that don't have LSM hooks21:44
jdstrandtyhicks: so, you're saying that if there is a hook, the LSM can set it to whatever it wants (eg, apparmor uses EACCES)21:46
jdstrandtyhicks: I don't quite understand if there isn't a hook... I mean, if there isn't a hook, there isn't a mediation point for the LSM21:46
tyhicksjust a sec, I'm tracing some apparmor hooks21:48
tyhicksjdstrand: AppArmor return EPERM in a couple instances (capability checks and profile transitions)21:50
jdstrandtyhicks: seccomp is different in that it doesn't use the LSM hooks though. why wouldn't it set errno to something else? I guess I could understand with setuid and friends cause setting it to something else might lead to programs not behaving correctly since they might be checking for EPERM21:50
tyhicksjdstrand: sometimes those EPERMS are passed on and they're ignored in other cases21:50
jdstrandtyhicks: ok, good to know21:51
tyhicksjdstrand: when you say, why wouldn't it set errno to something else?"21:51
tyhicksjdstrand: what errno value are you talking about when you say, "why wouldn't it set errno to something else?"21:51
jjohansentyhicks, jdstrand: yep EACCESS when policy says no, EPERM in a couple of places when its not so much policy as some other violation21:52
jdstrandtyhicks: if we use SECCOMP_RET_ERRNO() and set it to EACCES, why would the kernel return something else?21:53
tyhicksjdstrand: it won't (we must have misunderstood each other somewhere)21:53
jdstrand(that was my question. I may have answered myself with setuid and friends)21:53
jdstrandtyhicks: you said something about not being able to set EACCES for setuid. or did I misunderstand?21:54
tyhicksjdstrand: I was just saying that I don't think an LSM can intervene in a setuid() syscall and, if that's true, no applications are going to expect EACCES to be returned from setuid()21:55
tyhicksmaybe jjohansen can correct me if I'm wrong there21:55
jdstrandtyhicks: I didn't think seccomp was a true LSM in that regard. as for setuid and whether we should set EACCEs for it, that is worth considering for sure21:56
jdstrandI think I can test that real quick21:56
tyhicksjdstrand: I'm not talking about seccomp when I say LSM21:57
tyhicksjdstrand: I'm talking about selinux, apparmor, smack when I say LSM21:57
jdstrandtyhicks: ok. well, I was only talking about your seccomp PR :)21:57
jdstrandso I was a bit confused when you started talking about LSMs :)21:57
tyhickswut? you mentioned AppArmor in your first message on this topic :)21:58
tyhickswe're just talking in circles at this point :)21:58
jdstrandoh pfft21:58
jdstrandI wasn't clear21:58
jdstrand"I wonder if it would make sense to use EACCES instead?"21:59
tyhicksI think we both feel like EPERM is the right thing for snapd's seccomp filter to return, right?21:59
jdstrands/EACCES in your PR instead"21:59
jdstrandmy question was wondering if your PR should be changed to EACCES for consistency with apparmor22:00
jdstrandbut there is a very good argument against doing that with setuid22:00
tyhicksI think EPERM is a good default to go with for now. There may be some specific syscalls where we prefer to return EACCES instead of EPERM once we start widely testing a number a snaps with the new confinement.22:01
jdstrandtyhicks: that's fair. thanks and sorry for the extra circle or two22:04
tyhicksjdstrand: no problem - I'm a bit anxious to see how existing snaps will react to the change22:06
jdstrandI suspect most will eventually die in weird ways with cascading failures22:07
jdstrandI suspect setpriority will be the one that isn't so bad22:07
jdstrandfrom what I've seen, the return code is often not checked for that, but that isn't at all fatal22:08
tyhicksyeah, that'll be nice to not result in a kill22:08
jjohansenjdstrand: an LSM certainly can interfere with the setuid syscall, I think the hook has been removed atm but it could be reintroduced22:08
tyhicks(that was me that said that)22:09
tyhicksjjohansen: I was just going off of the hooks that currently exist22:09
tyhicksjjohansen: how long ago was that?22:09
jjohansentyhicks: hrmm, a couple years ago, when there was some unused hook cleanup, it is one I would like back for apparmor22:10
jjohansenI'm not the only one who wants a couple of the user related hooks to return22:10
jjohansenits hard to argue for them without the code to do something with them though22:10
tyhicksinteresting, thanks22:12

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!