[06:10] morning [06:53] Bug #1700560 changed: “snap find” returns confusing results [07:29] hello [07:29] sorry for being late [07:29] mborzecki how are you doing? :-) [07:29] zyga: hey [07:30] it's snowing outside (again) [07:30] zyga: have i missed anything interesting on friday? [07:31] hey zyga and mborzecki [07:31] haha, same here :-) snow snow snow [07:31] good morning mvo [07:31] here as well! [07:31] snow! [07:31] mvo: morning [07:31] well, I had a great Friday EOD except for one test that made me think all weekend about it [07:33] zyga: what was the test? [07:33] https://github.com/snapcore/snapd/pull/4644 [07:33] PR #4644: tests: add a spread test for layouts [07:33] look at the diff first [07:33] it passes everywhere but on core [07:34] jamesh hello [07:36] btw. this is what mikołajki looks like this time of the year: https://imgur.com/a/1Np8S [07:36] oh, we have a bug report hidden on github: https://github.com/snapcore/snapd/pull/3889#issuecomment-364637980 [07:36] PR #3889: interfaces: mount host system fonts in desktop interface [07:37] mborzecki woah, nice! [07:37] warsaw is full of smog because of inefficient combustion heaters and people burning trash :/ [07:38] sounds like every major city during the winter :P [07:39] there's an area in lodz, south-west, towards pabianice, mostly older housing, the air quality meters are usually going all red during the winter [07:39] yeah, it's all around poland [07:39] I miss spain, nobody burned trash, it wasn't that cold, air was clean [07:39] it wasn't that cold <-- that's your solution there :) [07:40] yeah but also everything was heated by gas, not coal/wood/garbage [07:40] and also a bit more money on average [07:43] zyga: that test failing, `cannot stat /var/lib/snapd/seccomp: No such file or directory` ? another thing, was there no /etc in the snap rootfs? [07:43] there is /etc in the core snap, we also have writable paths that change how that operates [07:45] zyga: there's a separate path for all this in snap-confine too? [07:45] nope [07:45] well, not for layouts [07:45] I have a suspicion what the problem might be... stay tuned :) [07:46] zyga: right, not for layouts, but for bootstrapping the mount namespace [07:47] yes, we do have different setup there [07:47] * zyga looks at http://aqicn.org/city/poland/mazowieckie/warszawa/ursynow/ and at http://aqicn.org/city/spain/catalunya/barcelona-eixample/ [07:55] zyga: hi [07:55] hey! [07:55] I merged master into your PR [07:56] and resolved some conflicts due to a recent big code move [07:59] jamesh I may also work on that branch further this week, mainly by chopping it down and landing parts while the last validation problem gets solved [07:59] thanks. I've been thinking a bit more about the checks jdstrand wanted. If we can't work out a way to reliably reconstruct the mount source from the mountinfo file, I wonder if it would be enough to ensure it is on the right device? [08:00] jamesh I don't know yet, I need to think about that and focus (just barely woke up) [08:01] i.e. if we expect the mount source to be in the user's /run/user/$UID tmpfs, we should be able to reliably check that it is still from that file system after the mount [08:02] (e.g. hasn't been replaced by a symlink to /etc on a different device) [08:02] yes, that's doable [08:02] one thing we can do is to check that after the mount op finishes there are no more symlinks than before [08:02] but I need to think about what we might be racing with here [08:02] of course, for the xdg-document-portal case, the mount point is in a fuse file system under $XDG_RUNTIME_DIR, so we can't just check the one device number [08:04] I think we could do a simple walk backwards through mountinfo before the mount to find out the expected device for the source, and then check the details of the new mountinfo entry afterwards to confirm [08:05] yeah, that's doable, I think we need to walk both the mount table and the path components and see where we land at each stage (if we follow any symlinks along the way) [08:06] jamesh btw, I was also wondering about one more thing, if we can simplify the problem by 50% by doing "mount -t stuff /dev/stuff ." [08:06] for user mounts, I think it'd be okay to consider any cross-device symlinks to be an error [08:06] if we mount in the current directory, if that's even doable, we have smaller attack surface [08:06] not sure if we're going to mount more than fuse filesystems now [08:07] mvo: could you upload a .vendor.tar.gz release files to github releases page for 2.31? [08:07] zyga: what do you mean by "mount -t stuff" here? [08:07] jamesh: pushd && cd /tmp && mount -t tmpfs tmpfs . && popd [08:08] jamesh no way to attack mount target directory (mount point) [08:08] and after we cd we can take all our time to investigate where we've landed [08:08] (we can also write a secure chdir) [08:09] mborzecki: sure, let me do this [08:09] zyga: is that more secure than doing "cd dest; mount --bind /source ." ? [08:09] mvo: thanks [08:09] no, that's exactly what I meant [08:09] the idea is to avoid mount /one/path /another/path [08:10] and to reduce that to /one/path [08:12] just wanted to check what would happen when mounting over a symlink: the command line tool dereferences the symlink itself, so that doesn't help much [08:12] jamesh yeah, that happens at syscall level too, that's why mount(2) sucks so much :/ [08:13] so who wants to propose a mountat() system call? :) [08:13] there were some proposals like that [08:14] but I think it's a bit complex for reasons I don't fully understand [08:14] there is some reason why it's path based more than just FD based [08:14] the last thing I remember was ... [08:17] * zyga still searches [08:18] https://lwn.net/Articles/718638/ [08:20] * zyga breaks for breakfast [08:23] jamesh: hey, did you look at the document portal? [08:26] hey alexlarsson [08:26] hey hey [08:27] You can't get any simple mount() changes in [08:27] because al viro hates the mount syscall and wants to completely redo it [08:28] Thus blocking minor improvements [08:28] Not that i disagree with him... [08:32] well, a no-follow flag would do wonders [08:32] but yeah [08:32] I'd love to see a new API too [08:32] alexlarsson: I haven't done a full test of your branch yet. I'll get that done and add some feedback to the PR. [08:32] (don't really mind if both happen) [08:33] there is a fantasy linux mount API described here: https://lwn.net/Articles/718638/ [08:33] What i would love is a bind mount + make readonly in a single call [08:34] I have some horrendous complicated code for recursive read-only bind mounts :/ [08:35] https://github.com/projectatomic/bubblewrap/blob/master/bind-mount.c [08:35] this is total bullshit [08:35] and should just be MS_BIND | MS_REC | MS_RDONLY [08:37] all software evolves till it parses mountinfo ;-) [08:39] I asked al, and he agreed it was bullshit. But... [08:39] but nack to small improvements? [08:40] yeah [08:40] is he working on the new big API or is it just fossilisation stage and nothing happens? [08:41] unclear [08:43] If you fixed mount() so that flags other than MS_REC were respected when performing a bind mount, how would the app even know? [08:44] jamesh I think it'd require a MOUNT_EXTRA_FLAGS flag or something similar to indicate desire to use a richer interface [08:44] or an entirely new syscall [08:45] zyga: the syscall is currently documented as ignoring other flags, and the call is likely to succeed on an old kernel anyway [08:45] so you'd need to check after the fact to see if they were applied [08:46] too messy, the idea is to make userspace easier, not just hard but different [08:46] given what you said a new syscall is really needed [08:47] jamesh: child of MS_MGC_VAL ? [08:48] If they'd instead made it an error to specify the flags that it currently ignores, there'd be something user space could check for [08:48] yeah [08:48] but that would make sense! [08:48] haha [08:48] all the good choices we never did [08:50] and even once the new interface is here it will be many many cycles before it can be used as a dependency without cutting a significant chunk of users [09:31] morning all [09:36] hey Chipaca, good morning [09:36] mvo: hiya [09:38] I suck at naming things. What would *you* call this Thing, and what would you call the arguments to New after the first? https://gist.github.com/chipaca/21222a4fa23ae5d8629c84cc016f23ea [09:38] ^ that's what i've been playing with this weekend [09:38] * Chipaca now gets back to work [09:39] Chipaca: hm, maybe Formater? [09:40] mvo: it's not strictly work, and it's got bugs still, but if you want to play with it, https://people.canonical.com/~john/snap-find should work on amd64 [09:40] (needs a running snapd) [09:41] Chipaca: nice [09:42] Chipaca: Columnizer , ColumnFormater, SheetFormater for some more ideas [09:42] oooh, Columnizr and be all web 2.0 [09:43] wrt the bugs, one is go's, in that i need to add code to handle double-wide characters by hand (search for 'hello' and look at hello-bluet) [09:43] another is that in long listings it seems to be doing something wrong and leaving too much space for things it's not going to print yet, so there's a cutoff missing its spot [09:43] but anyway,lots of fun rune-twiddling to be had [09:44] Chipaca: :) looks/sounds like it indeed! === ikey|afk is now known as ikey [10:44] * Chipaca -> physio break [10:59] * Chipaca -> *really* physio break [11:06] PR snapd#4648 opened: debian, snap: only static link libseccomp in snap-seccomp on ubuntu [11:12] PR snapd#4645 closed: snapctl: allow `snapctl get` from any uid [12:01] PR snapd#4649 opened: many: record if snap was installed with --dangerous, snow relevant annotation in `snap info` and `snap list` [13:00] PR snapd#4650 opened: daemon: allow `snapctl get` from any uid [13:01] hmmm, hangouts seems rather dark right now [13:02] * Chipaca turns it off and back on again [13:02] oh god they're trying to use opengl [13:50] PR snapd#4629 closed: ifacestate: only rewrite security profiles if the system-key changes [13:53] man, go-flags is terrible at formatting this stuff [13:53] :-( [13:54] https://pastebin.ubuntu.com/=dS8vNTVhrM/ [13:54] like, wtf is going on with the option alignment [13:54] --hello [13:54] . --yes-im-over-here [13:54] . --WAT [13:58] PR snapd#4651 opened: cmd/snap: add help for service commands (oops) [13:58] * Chipaca hugs mup [14:02] today is a day where I curse Go for making it so that build flags are comments in Go files [14:03] Son_Goku: CGO_LDFLAGS_ALLOW is your friend [14:03] * Son_Goku sighs [14:04] Son_Goku: they did forget to add a similar for pkg-config though :) [14:04] * Son_Goku mournfully looks over at the nice stuff in Rust with Cargo... [14:17] off to pick up the kids [14:19] zyga: 4643 has a trivial suggestion for a comment fix [14:19] ack [14:20] PR snapd#4638 closed: devicestate: fix autopkgtest failure in TestDoRequestSerialErrorsOnNoHost [14:25] PR snapd#4651 closed: cmd/snap: add help for service commands (oops) [14:25] wooooo [14:25] that's got to be a record [14:25] 23 minutes from PR to landed with 2 +1s and green spread [14:29] * mvo hugs Chipaca [14:37] PR snapd#4646 closed: snap-seccomp: fix cgo pkg-config and LDFLAGS [15:11] the classic confinement test segfaults on bionic :( /me wants the `snap run --gdb` branch to be merged to debug this [15:11] jdstrand: you around? [15:12] PR snapd#4643 closed: snap: disallow layouts in various special directories [15:34] Chipaca: hey, yes [15:34] jdstrand: I was looking through the review tools wondering about version validation [15:34] jdstrand: are they still from lp:click-reviewer-tools ? [15:35] Chipaca: for the moment [15:35] jdstrand: and if so, is it _verify_pkgversion in common.py [15:35] jdstrand: and if so, maybe update it so it matches snapcraft's validator? [15:35] jdstrand: or, sync with snapcraft about it [15:36] there are things that one considers invalid that the other doesn't, in both directions [15:36] Chipaca: I can do that. it matched at one point... [15:36] Chipaca: is there a particular issue you are looking at? [15:37] jdstrand: I'm adding version validation to snapd [15:37] and I'd like to write that it matches what the review tools and snapcraft do [15:37] but currently i cannot :-) [15:37] then, yes, I'll adjust the review tools to match snapcraft [15:38] Chipaca: since you've looked at snapcraft, where is the validation happening there? [15:38] jdstrand: ^[a-zA-Z0-9.+~-]{1,32}$ is what snapcraft is checking fwiw [15:39] so '~' is a valid version? [15:39] jdstrand: schema/snapcraft.yaml [15:39] jdstrand: yes [15:39] jdstrand: also README.~1~ [15:40] Chipaca: let me rephrase. '~', '.', '+' and '-' are all supposed to be valid versions? [15:40] jdstrand: in snapd, _anything_ is a valid version :-) [15:40] but yes in snapcraft those things are valid versions [15:40] Chipaca: the store has its own checks separate from the review-tools aiui, and the review tools reflect that [15:40] Chipaca: I know it is in snapcraft :P is snapcraft *wrong*? [15:40] this is turning into a Meeting, isn't it [15:41] *gasp* [15:41] surely not [15:41] sergiusens: you on national holiday today, n'eh? [15:42] kyrofa: kalikiana: can we talk about versions for a bit? [15:42] or maybe i should open a bug [15:42] Chipaca, sure! [15:42] Chipaca: honestly, I think we need the store team. iirc there are checks there [15:42] kyrofa: am I right in thinking that snapcraft uses the schema/snapcraft.yaml thing to validate versions? [15:42] Oh, looks like I have some reading to do [15:43] Chipaca, yes, if it's a string it's validated with that regex [15:43] jdstrand: ok [15:43] I'll change the review tools to whatever is agreed to. the version check was more important when we had the version as part of the security label [15:43] noise][: who knows about the version validation the store does? [15:44] noise][: that is, validation of the version string in a snap [15:44] Chipaca, yeah we have no horse in this race either-- you just tell us what you want it to be and it shall be done [15:44] kyrofa: basically the regexp there is suspect, but we have apparently three different ones already :-) so, this [15:45] kyrofa: version must match ^fantastic-.*$ [15:45] \o/ [15:45] I *think* what I was remembering was that we were doing what the store does, not what snapcraft does, since people can run the review-tools on a snap prior to upload (no one does that now, but when this was written, that was the thinking) [15:45] Yeah seems reasonable [15:45] :P [15:46] jdstrand: right, but I'm not sure the store enforces the "version must be debianish" rule any more [15:46] it's possible now that click and snapv1 are gone, the version check in the store was removed [15:46] right. me either [15:46] hence you asking them :) [15:46] jdstrand: Chipaca: I don't see a version check in the store [15:47] hmmm [15:47] there's just a max length [15:47] it had to do with the upload [15:47] afaict [15:47] I forget what it was called... [15:47] maybe I should just try to upload something with a ridiculous version and see what happens [15:49] I don't know if the output of the test is: 'Is a valid package' or 'Stated package version matches snap.yaml' [15:49] those two tests are not from the review tools [15:51] pedronis: does what you looked at have those checks? ^ [15:52] - malformed 'version': 'this is some silly potatoes' [15:53] no spaces? [15:53] Chipaca: that seems review tools tough [15:53] not the store itself [15:53] I don't see a pattern check for version in the store code [15:54] jdstrand: note review tool's [A-Za-z0-9.+:~-]+? means those things are also valid versions for this, fwiw [15:55] so, it's just snapcraft and review tools, beyond the length limit [15:56] Chipaca: yes [15:57] there's a 32 chars max-length [15:57] jdstrand: and i just pushed a thing with a version of ~ just to make sure [15:57] in the sotre [15:57] store [15:57] alright, well, if there are no checks in the store, I'll just update the review tools to match snapcraft [15:57] jdstrand: I'm double checking with other people about the store [15:57] kyrofa: is there any reason snapcraft explicitly disallows epoch-style versions things? [15:58] kyrofa: (there's no ":" in there) [15:58] Chipaca, not that I know of. This regex has been there as long as I can remember [15:58] pedronis: it was some code that westby wrote. I can't remember the name of it. it might be gone now [15:59] Chipaca, we don't use the version for anything other than plopping into the snap.yaml and putting it into the filename of the resulting snap [15:59] kyrofa: it probably should include ':'. people like to snapcraft a stage-package and just use the deb version [16:00] jdstrand, yeah easy peasy. Sounds like we want some other changes too though, perhaps to not start or end or have multiple [.+~-:] [16:00] would ^[a-zA-Z0-9]([a-zA-Z0-9:.+~-]{,30}[a-zA-Z0-9+])?$ be any better? [16:00] oh, oh, _ also [16:00] jdstrand: other store people seems to confirm there's no check (atm) [16:00] ^[a-zA-Z0-9]([a-zA-Z0-9:.+~_-]{,30}[a-zA-Z0-9+])?$ [16:00] *seem [16:00] Yeah seems fine to me Chipaca. You want it, you got it [16:01] mvo: you also know something about versions, could you eyeball it? ^ [16:01] Chipaca: in a meeting right now, will look soon [16:02] mvo: ack [16:03] Chipaca: I would suggest: ^[a-zA-Z0-9]([a-zA-Z0-9:.+~_-]{,30}[a-zA-Z0-9+~])?$ [16:03] Chipaca: (allow ~ at the end) [16:05] jdstrand: i don't mind; what version are you imagining that needs it? [16:05] 1.2.3-2~ [16:05] I left the + in there because I imagined somebody wanting a 3.2+ version [16:05] I've seen that before [16:05] ok, fair [16:05] yeah, 1.0~ will be common [16:05] (ish) [16:06] its a common pattern for e.g. backports [16:06] also if [+~_-] isn't an emoticon it should be [16:06] Chipaca: alright, adding to trello ^[a-zA-Z0-9]([a-zA-Z0-9:.+~_-]{,30}[a-zA-Z0-9+~])?$. ping me if changing [16:06] jdstrand: will do, thank you! [16:07] I'm also surprised that the review tools allow what they do... they didn't always. I remember the discussions. anyway, who cares, it is changing now :) [16:07] kyrofa: running with this one ^ will shout if it changes [16:07] also may i suggest you python people use (?: instead of plain ( :-P [16:10] kyrofa: actually i'm not sure what the schema validation uses for regexps; if {,30} trips it up, you can use a * and the max-length of 32 will catch it [16:11] * Chipaca breathes in, lets people deal with their problems, and gets back to his own [16:15] Chipaca, sounds good [16:16] Sorry, made the mistake of launching chrome. Had to reboot [16:22] written forum post ahead of PR, because why not: https://forum.snapcraft.io/t/snapd-enforcinig-snap-versions/3974 [16:22] (currently not listed so people aren't confused :-) ) [16:23] Chipaca, waiit.. how do you do unlisted posts? [16:24] jdstrand, what kind of timeline do you anticipate for that change? We should coordinate [16:25] kyrofa: magic [16:25] * kyrofa hears "special permissions you don't have" [16:25] kyrofa: (tap the gear, it should be there) [16:26] * Chipaca publishes a snap with a version of 'unpredictable-counterrevolutions' [16:26] I see hide details and poll [16:26] kyrofa: not terribly soon unless someone wants it *right now* [16:26] kyrofa: you pleb [16:26] jdstrand, no rush from me. I suppose snapcraft could make the change before you folks and it would be okay. It'd be bad the other way around [16:27] * kyrofa puts together a PR [16:27] Chipaca: the version of "base-18" is "very-unstable", but that should still validate right? [16:27] mvo: unpredictable-counterrevolutions validates :-) [16:28] Chipaca: heh [16:29] kyrofa: what is in the review-tools is pretty close to what we agreed to [16:29] Good deal [16:29] kyrofa: I guess just commit to snapcraft soon and these can just roll in later [16:30] jdstrand, sounds good [16:30] the only problematic one is '_' in that you'll start allowing '_' but the review tools won't [16:30] but, you'd have to know that all of a sudden you could use it [16:31] the other bits show the review tools more strict already, and people haven't been complaining [16:31] or we haven't heard them :-) [16:32] (i think it's the sort of thing that people will hit, complain, tweak the string, and move on [16:32] ) [16:32] s/complain/grumble/ perhaps [16:32] point is, status quo if we aren't in perfect sync [16:32] when we are in perfect sync, no more grumbling [16:36] hi jdstrand [16:37] jdstrand: so nothing will change with the review tools r1000 unless I set that resquashfs env vraiable, right? [16:38] roadmr: not wrt resquash, no [16:39] jdstrand: ok - right. I have r1000 on staging right now and it was happy with my usual, trivial test snap [16:39] * jdstrand nods [16:39] roadmr: you may have noticed all the snaps in tests/ in the bzr tree [16:39] jdstrand: oh I did not! awesome! [16:40] roadmr: that was part of all the testsuite updates I did [16:40] jdstrand: kyrofa: you've probably realised by now that it's {0,30} not {,30} in the regexp [16:40] Chipaca: yes [16:40] the {,30} might be a perlism, or it might be a bug in my head :-) [16:40] jdstrand: fantastic! I usually sanity-check locally against some bogus snap I have, to avoid pushing a bogus revno, but this should make all that moot! [16:40] moot-woot! [16:41] roadmr: if you want to send me that snap, I can add it to the tools [16:41] or anything else you like to test [16:41] jdstrand: I don't think it's needed, it's an empty snap with classic confinement and only meant to protect against that case we had once... [16:41] jdstrand: where you had an undeclared dependency so the thing worked for you but failed when pushed to the store [16:41] I may already have that use case in there [16:42] jdstrand: to be honest I think I'll switch my local testing from my crap snap to the list you have here, though again it's for sanity, if you ran that test suite at merge time that looks quite thorough [16:42] roadmr: note I said 'wrt resquash'. that is because there are new and updated checks, but that is all normal stuff [16:43] jdstrand: so does resquash modify the snap-under-test? or just verifies that when resquashing, the result is identical to what was uploaded? [16:44] jdstrand: (that's why I zeroed in on that feature - was worried it might somehow alter the uploaded snap) [16:44] in which case many components would be unhappy [16:44] roadmr: there is a big matrix for testing that I use. 14.04 was tested. running 'make functest' will run the tools on tests/, but the output is different on 14.04 and 16.04 on some click stuff due to testsuite mocking [16:44] roadmr: that will go away when I drop click/snapv1 [16:45] that'll be awesome [16:45] roadmr: the resquash will *not* modify the original squash. it unsquashes then resquashes and makes sure they are the same [16:45] s/they are/it is the same as the orig/ [16:45] jdstrand: ah, nice! [16:46] yeah, we totally don't want to touch the original [16:46] roadmr: also note, that the resquashfs have always run [16:46] ohh [16:46] roadmr: they are even running in r1000. I didn't change that [16:47] roadmr: the env vars say whether the check is enforced or not, and whether or not to use fakeroot [16:47] they haven't been enforced for a long time, and r1000 doesn't change that [16:47] once snapcraft 2.39 is is in -updates on all the releases, I'll turn enforcement back on [16:48] jdstrand: I see. So if it's enforced it'll actually return an error if they don't match? [16:48] roadmr: yes [16:48] but the snap needs to be squash with -no-fragments, otherwise false positives [16:48] and snapcraft 2.39 does that [16:48] jdstrand: ok. I was thinking how to set that variable but from this I get we shouldn't, until you ask for a tools update which enforces this, right? [16:49] roadmr: yes. you don't need to do anything. I was mentioning it in case you wanted to test it yourself or do something on staging, etc, but prod doesn't need to do anything [16:50] jdstrand: ok. Sounds like we'll test that once you enable it (at which point it'll make it to staging anyway) [16:50] roadmr: when I turn on enforcement, it'll be by default and the tools will honor SNAP_RESQHASHFS_ENFORCE=0 to turn it off [16:50] jdstrand: much fiddlier for me to add a feature flag to set the env var [16:50] yes [16:50] jdstrand: right, we can totally add the feature flag if we see a lot of false positives and we want to be able to switch that off quickly [16:50] the env var was mostly for us in debugging the enforcement feature [16:51] I never wanted you to have to do anything [16:53] thanks :) [16:53] we're clear on that then. r1000 should land in prod sometime this week [16:53] \o/ [16:53] thanks :) [16:53] jdstrand: I'll be afk from tomorrow until Feb 21st [16:54] roadmr: ack. while I have you, you adjusted the debs to install already? [16:54] jdstrand: yes, I did that first on staging to ensure none of the deps wrecked the world, then updated the tools, which are running fine too [16:54] perfect [16:54] thanks again! :) [16:55] most of those deps you probably had. a few are if you want to run parts of the testsuite there [16:56] jdstrand: yes, we did tweak the set of deps a bit (removed python3-all since we already install python3) but there were only 5 new ones [16:56] * jdstrand nods [17:10] why, oh why, doesn't the go test runner tell me exactly what file it's running? There are eleventyone backend_test.go and at least five have a TestUpdatingSnapToOneWithFewerHooks :-/ [17:32] PR snapd#4652 opened: tests: fix spread test failures on 18.04 [17:39] Hello everyone I am having problems removing a snap [17:40] I keep getting an error I have googled and seems like I must be the only one [17:40] the error is error: cannot remove "chromium": snap "chromium" has changes in progress [17:41] I did a reboot and did not open chromium and I am still getting this message [17:41]