*** xjuan has quit IRC | 00:50 | |
*** mcatanzaro has quit IRC | 04:24 | |
*** persia has quit IRC | 06:01 | |
*** waltervargas[m] has quit IRC | 06:01 | |
*** persia has joined #buildstream | 06:03 | |
*** waltervargas[m] has joined #buildstream | 06:06 | |
*** hase has joined #buildstream | 07:06 | |
*** hase has quit IRC | 07:08 | |
*** i0d3l4y has joined #buildstream | 07:08 | |
*** i0d3l4y has quit IRC | 07:33 | |
*** dominic has joined #buildstream | 08:09 | |
*** dominic has joined #buildstream | 08:09 | |
*** aday has joined #buildstream | 09:34 | |
*** tristan has joined #buildstream | 09:47 | |
*** jonathanmaw has joined #buildstream | 09:51 | |
tristan | juergbi, I was able to run totem with access to /dev/dri, without severe tweaks to the core bwrap sandbox | 10:59 |
---|---|---|
juergbi | \o/ | 10:59 |
juergbi | how did you solve the permission issue? | 10:59 |
tristan | Well, I didnt | 11:00 |
juergbi | sudo chmod a+rw? | 11:00 |
tristan | nope | 11:00 |
tristan | The devices show up as nobody:nogroup | 11:00 |
juergbi | ah but the gid check is still done outside the namespace? | 11:00 |
tristan | There is one tweak to the bwrap sandbox though, and I'm thinking; lets just apply it across the board for all mounts | 11:00 |
juergbi | which is? | 11:01 |
tristan | Basically, we are using --bind for all mounts that we setup | 11:01 |
tristan | But, it works if you use --bind-dev (doh!) | 11:01 |
tristan | bwrap --help says: "--bind SRC DEST Bind mount the host path SRC on DEST" | 11:02 |
tristan | and "--dev-bind SRC DEST Bind mount the host path SRC on DEST, allowing device access" | 11:02 |
juergbi | ah, --bind uses nodev | 11:02 |
tristan | Well, in bwrap it will basically do... yeah that | 11:02 |
tristan | So, I feel like, allowing device access across the board, does not effect build sandboxes | 11:03 |
juergbi | as those mount sources anyway can't contain device nodes | 11:03 |
tristan | because it's all stuff we stage there anyway | 11:03 |
tristan | right, the alternative would be dual code paths and special casing | 11:03 |
juergbi | yes, i can't think of an issue out of the top of my head | 11:03 |
tristan | There is something else I needed hehe (but not from core features) | 11:04 |
juergbi | a driver? | 11:04 |
tristan | If I only mount /dev/dri, then I get "MESA-LOADER: failed to retrieve device information" | 11:04 |
tristan | twice, at startup | 11:04 |
tristan | then, if I try to play a video | 11:05 |
juergbi | does it use /sys to get device info? | 11:05 |
tristan | totem just crashes with this message from mesa: | 11:05 |
tristan | intel_do_flush_locked failed: Invalid argument | 11:05 |
juergbi | hm | 11:05 |
tristan | Mounting /sys fixes that, exactly | 11:05 |
juergbi | so it seems it may make sense to support mounting a couple of directories via project conf after all | 11:06 |
tristan | So wonder if it's sane... mount /dev/dri and /sys | 11:06 |
juergbi | not real filesystems but still | 11:06 |
tristan | yeah, right now I'm testing on command line | 11:06 |
juergbi | for the weak sandbox, i don't really see an issue | 11:06 |
juergbi | we don't want those in the build sandbox | 11:07 |
juergbi | but should also not need them | 11:07 |
tristan | (with my added --mount support branch) | 11:07 |
juergbi | does flatpak mount whole /sys as well or do they restrict it somehow? | 11:07 |
tristan | flatpak does similar, I'm not sure what it's doing for /sys though | 11:07 |
tristan | but it will try to mount /dev/dri | 11:07 |
tristan | if access to dri was requested (actually it has a hard coded list, including mali etc, which it tries) | 11:08 |
juergbi | https://github.com/flatpak/flatpak/blob/master/common/flatpak-run.c#L2253 | 11:09 |
tristan | With our approach; we could similarly list those in a project.conf, and it will just mount any of the things which exist, and (currently) warn if they dont exist | 11:09 |
juergbi | mounting a subset | 11:09 |
tristan | ah I see, I didnt get that far | 11:10 |
tristan | pulseaudio looks like a nightmare | 11:10 |
tristan | but lets make dri work first :) | 11:10 |
juergbi | i don't like seeing a warning that mali driver is not loaded on non-mali systems | 11:10 |
tristan | Right, me neither | 11:10 |
juergbi | but we could start with /dev/dri | 11:11 |
tristan | So; currently we already support list-or-dict | 11:11 |
juergbi | and look into that later | 11:11 |
tristan | So, we could have that configurable, whether something is expected or not ? | 11:11 |
juergbi | sure, sounds fine to mark some of those as optional to disable the warnings | 11:11 |
juergbi | or possibly replace the warning with an error if we support that | 11:11 |
juergbi | for the non-optional cases | 11:11 |
tristan | right, there is a big fuzzy area though; I think warning is better in this case | 11:12 |
tristan | i.e. with a warning we inform the user that things might not work as the project expects it to | 11:12 |
tristan | but we dont disallow them a shell environment entirely | 11:12 |
juergbi | right, forcing the user to remove mounts in the project.conf sounds bad | 11:13 |
tristan | s/big/bit | 11:13 |
juergbi | sounds fine to me | 11:13 |
* tristan still has to deal with cleanup of stuff we add by creating directories | 11:14 | |
juergbi | tristan: btw: quick other question, we never store absolute symlinks in artifacts, right? | 11:14 |
tristan | but that should close dri | 11:14 |
tristan | problems | 11:14 |
juergbi | nice | 11:14 |
tristan | Ummm, no that is not true; we store whatever symlinks were in the collection directory as is | 11:15 |
juergbi | hm, but we have this function to create relative symlinks out of absolute ones | 11:15 |
tristan | We are careful to make them relative while staging, though | 11:15 |
juergbi | i thought we already did this before a commit | 11:15 |
tristan | so it happens on the other side, not at artifact collection time | 11:15 |
juergbi | well, we use the same helper function for both | 11:15 |
tristan | Do we ? | 11:16 |
tristan | I think that is something which must have changed at some point then | 11:16 |
juergbi | possible. we hardlink files from the output directory to a temp directory as commit preparation | 11:16 |
tristan | Ah | 11:16 |
juergbi | and that recursive hardlinking uses the same process_list function, afaict | 11:16 |
tristan | That must have implicitly happened when splitting the artifact into files/ logs/ and meta/ | 11:17 |
juergbi | that's possible | 11:17 |
juergbi | i might need to rely on that for remote execution | 11:17 |
juergbi | do you anticipate any issues with such a restriction? | 11:17 |
tristan | "All symlinks must be relative" restriction ? | 11:18 |
tristan | At all times now, basically ? | 11:18 |
juergbi | packages are allowed to create absolute symlinks | 11:18 |
tristan | Well, in the most pedantic sense, we are disallowing users from using absolute symlinks | 11:19 |
juergbi | but artifacts would never contain absolute symlinks | 11:19 |
tristan | Which is a limitation | 11:19 |
juergbi | we already essentially did this for staging, though | 11:19 |
tristan | Packages being allowed to create absolute symlinks I think is besides the point | 11:19 |
tristan | If we create a bunch of rpms which create symlinks in their metadata post-inst or whatnot, that is one thing | 11:20 |
tristan | If we run rpm inside a build sandbox to generate a firmware; absoluteness of those will be lost in the generated artifact | 11:20 |
juergbi | there is currently no way to extract an artifact without going through this processing function, though, right? (assuming you don't poke at ostree directly) | 11:21 |
tristan | It would be good to "aim" for a day where every possible thing which can be expressed for a given platform/filesystem, can be preserved | 11:21 |
juergbi | so the difference is not actually user-visible | 11:21 |
tristan | But we fall short of this now, of course | 11:22 |
tristan | To be honest I dont feel strongly about this today | 11:23 |
tristan | It would be nice to eventually get rid of this relative transformation stuff, which is originally there because of the way paths are expressed | 11:23 |
tristan | (i.e. its marginally possible for buildstream to try to stage stuff onto your host because of symlinks) | 11:24 |
tristan | the transformation is a protective measure against that | 11:24 |
juergbi | yes, this could theoretically be handled while preserving absolute symlinks | 11:25 |
tristan | If staging were to be more bullet proof, maybe we should not need this | 11:25 |
juergbi | but it's a bit cumbersome | 11:25 |
tristan | All I'm saying is ideally we should shoot for "perfect" and admit that we are not | 11:25 |
tristan | How is this a problem for remote execution btw ? | 11:26 |
tristan | I mean... if we eventually get around to solving this, we can solve it there too right ? | 11:26 |
juergbi | yes, i was initially planning to fully support absolute symlinks but for the remote execution API there is a desire for canonical cross-platform support as far as possible | 11:26 |
juergbi | and absolute symlinks don't necessarily make sense cross-platform | 11:27 |
tristan | I'm not sure what canonical means in this sense | 11:27 |
tristan | I mean, does it mean "Support only the common denominator of all possible platforms" ? | 11:28 |
tristan | that would not sound sensible | 11:28 |
juergbi | e.g., using / as path separator everywhere | 11:28 |
tristan | right, that would not work on windows | 11:28 |
juergbi | actually that does | 11:28 |
juergbi | but absolute paths on windows don't start with / (or \) | 11:28 |
tristan | Only if you're not building native windows stuff with native windows buildstream, right ? | 11:28 |
juergbi | windows accepts / as alternative directory separator | 11:29 |
tristan | Oh | 11:29 |
juergbi | (alternatively, the software could easily translate standardized / to \) | 11:29 |
juergbi | the binary protocol would accept any kinds of symlinks | 11:30 |
juergbi | however, the API might officially only allow relative ones | 11:30 |
juergbi | (not decided yet, just a proposal for now) | 11:30 |
juergbi | so if we wanted to use absolute symlinks, we'd either have to get that restriction lifted in the API spec or use a server that allows it anyway | 11:31 |
tristan | Errm, so the driving reasoning behind this is basically, symlinks on platforms which use paths which dont start with a path separator (e.g. volume letter) would break this canonical way of expressing paths ? | 11:32 |
tristan | I can understand a strong desire for wanting to express paths in a canonical way | 11:33 |
tristan | especially since CAS and all of this is all expressing merkle trees and paths | 11:33 |
juergbi | yes, that's the point here, you want the same thing to have the same digest | 11:33 |
tristan | however, I have a thought... | 11:34 |
juergbi | so you also want to reduce differences between platforms | 11:34 |
tristan | If you are building something... even on a platform where paths dont start with path separators, maybe absolute paths on those platforms should *still* be represented with a path separator | 11:34 |
juergbi | like /c/foo? | 11:35 |
juergbi | that's the mingw/cygwin way, i suppose | 11:35 |
tristan | I.e.; the "Stuff you are building" is not.... "The layout of volumes on a target machine", it's just a payload | 11:35 |
tristan | One might build a bunch of stuff, and flash it to a drive somewhere, where it will appear as D:\... | 11:35 |
tristan | or somewhere else as C:\... | 11:35 |
juergbi | i think in practice absolute paths should almost never matter at all for build actions on Windows (symlinks themselves are much rarer on Windows but let's ignore that) | 11:37 |
juergbi | as there aren't really absolute standard directories there | 11:37 |
tristan | Well | 11:38 |
juergbi | it would definitely also be an option to reject absolute symlinks just for Windows but allow them for POSIX | 11:38 |
tristan | I wonder if "build activities" is different from "the output of the build" | 11:38 |
tristan | We are treating this as the same now | 11:38 |
tristan | But making concessions about how things will be staged, and storing the full output data, could eventually become different | 11:39 |
* tristan didnt word that very well | 11:39 | |
tristan | I.e. on windows, lets say a build tries to create a symlink to the absolute path E:\domain\config ... it's sort of at their on risk, during the build; it would be staged as a relative symlink, and ensured to be restored if necessary in the artifact ? | 11:41 |
tristan | cross-volume relative paths would however seem impossible, on windows | 11:42 |
skullman | the last time I had a poke at creating volumes in windows there was the option of it not having a drive letter and being mounted on top of another drive's path | 11:43 |
skullman | I don't know if it would permit a symlink out of it in that form. | 11:43 |
tristan | so possible, but not cross drive-letter | 11:43 |
* tristan also doesnt know about windows rules for symlinks | 11:44 | |
juergbi | they only got introduced in Vista or so, iirc | 11:45 |
juergbi | don't know how much they're actually used | 11:45 |
juergbi | ok, so one way to look at it would be to say that absolute symlinks are allowed if the Action's input root directory is mapped to the filesystem root (of a sandbox as in our case) | 11:46 |
juergbi | as in that scenario, / would still not escape the specified input directory | 11:46 |
juergbi | and this might even work on Windows in the case that you have a virtual drive letter for the sandbox or something like that | 11:46 |
juergbi | (no idea how this is typically handled) | 11:47 |
juergbi | but including symlinks to other drive letters doesn't make sense to me | 11:47 |
tristan | it doesnt make sense to me in the context of a build | 11:47 |
tristan | it makes sense to me in the context of a user | 11:47 |
juergbi | sure, but we're talking about builds here | 11:48 |
tristan | (i.e. I have symlinks from my home dir to a huge volume in /store, that's typical) | 11:48 |
juergbi | build output should pretty much never depend on user config | 11:48 |
tristan | right :) | 11:48 |
juergbi | ok, i'll try to make a case for this to keep the option open for us to preserve absolute symlinks | 11:49 |
juergbi | btw: extended permissions are also not supported but as buildstream anyway doesn't support this, i'm not planning on writing a proposal for this at this point | 11:49 |
tristan | I also think that, we probably never want to stray outside of filesystem data; drive layouting sounds to me to be out of scope for BuildStream | 11:49 |
tristan | When we do know about that at all, we are editing a bootable image, but technically we're just creating a file, which happens to be a partitioned image | 11:50 |
tristan | extended permissions probably need to be carried in an abstract way by CAS, and applied in filesystem/platform specific ways in remote workers | 11:51 |
tristan | not immensely concerned about this right now, though | 11:52 |
juergbi | same here. given the issues we see in #38, i'm tending towards handling extended permissions via public data. either manually (which is beneficial to have clear overview of what packages have setuid root, e.g.) or with a LD_PRELOAD helper that generates the public data | 11:56 |
jmac | What do you mean by public data? | 12:04 |
tristan | jmac, public: in an element | 12:06 |
tristan | http://buildstream.gitlab.io/buildstream/format.html#public | 12:06 |
jmac | Oh, I see | 12:07 |
gitlab-br-bot | buildstream: merge request (tristan/shell-mounts->master: Adding new --mount option to bst shell.) #302 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/302 | 12:57 |
tristan | juergbi, I | 13:00 |
tristan | gah | 13:00 |
tristan | I'm tempted to just allow directories in `host-files`, rather than adding a separate key | 13:01 |
tristan | it's trivial though, any preference ? | 13:01 |
* tristan thinks project.conf files read better without that separation | 13:01 | |
gitlab-br-bot | buildstream: merge request (214-we-need-a-way-to-make-elements-depend-on-a-subset-of-an-element-s-output->master: Resolve "We need a way to make elements depend on a subset of an element's output") #284 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/284 | 13:01 |
tristan | I suppose, since we're in an unstable cycle we *could* change `host-files` for `host-paths`, but the existing name seems ok for directories | 13:03 |
tristan | if you share a directory, it's probably because you want to have access to the files inside it | 13:04 |
*** tristan has quit IRC | 13:12 | |
*** tristan has joined #buildstream | 13:18 | |
juergbi | i don't mind too much either way | 13:22 |
tristan | cool, I'm going to anyway add a note to the docs that one should not specify a location where user's important data is expected to be found | 13:27 |
tristan | like a /home dir | 13:27 |
tristan | in either case, it's an important note | 13:27 |
gitlab-br-bot | buildstream: merge request (214-we-need-a-way-to-make-elements-depend-on-a-subset-of-an-element-s-output->master: Resolve "We need a way to make elements depend on a subset of an element's output") #284 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/284 | 13:28 |
juergbi | yes, that makes sense | 13:32 |
*** xjuan has joined #buildstream | 14:14 | |
*** lantw44 has quit IRC | 14:26 | |
*** lantw44 has joined #buildstream | 14:30 | |
gitlab-br-bot | buildstream: merge request (239-use-pylint-for-linting->master: WIP: Resolve "Use pylint for linting") #283 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/283 | 14:32 |
jennis_ | Would someone be able to review !283? skullman? | 14:33 |
skullman | I'll have a glance through while CI is running | 14:34 |
jennis_ | ok | 14:34 |
jennis_ | thanks | 14:34 |
skullman | jennis_: lint fails from whitespace and "too-many-ancestors" | 14:55 |
jennis_ | yes I expected that to happen | 14:55 |
jennis_ | tlater, has some more "strict" linting in the CI tests so I will work off the log output from that and make those corrections | 14:56 |
*** mcatanzaro has joined #buildstream | 15:07 | |
gitlab-br-bot | buildstream: merge request (jmac/configurable-logging->master: Configurable log line formatting) #282 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/282 | 15:13 |
juergbi | jmac: i think it looks good now. i was just wondering whether we should use elapsed_us or elapsed-us. i don't think we use any underscores in other format variables so far but - is used for workspace-dirs in the element format | 16:14 |
juergbi | is _ instead of - intentional because it's a unit? | 16:14 |
juergbi | if we say we want to use _ for units that's fine with me. but if it was an unintentional difference to existing variables, we should strive for consistency | 16:15 |
jmac | offs. No, not intentional, just my brain is frozen today | 16:16 |
jmac | I'll change it | 16:17 |
*** dominic has quit IRC | 16:18 | |
gitlab-br-bot | buildstream: merge request (jmac/configurable-logging->master: Configurable log line formatting) #282 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/282 | 16:19 |
gitlab-br-bot | buildstream: merge request (239-use-pylint-for-linting->master: WIP: Resolve "Use pylint for linting") #283 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/283 | 16:20 |
jennis_ | skullman ^^ I think that should pass the linting pipeline, let's wait and see! | 16:21 |
gitlab-br-bot | buildstream: merge request (jmac/configurable-logging->master: Configurable log line formatting) #282 changed state ("merged"): https://gitlab.com/BuildStream/buildstream/merge_requests/282 | 17:08 |
jmac | woop | 17:09 |
jennis_ | nice one | 17:09 |
gitlab-br-bot | buildstream: merge request (214-we-need-a-way-to-make-elements-depend-on-a-subset-of-an-element-s-output->master: Resolve "We need a way to make elements depend on a subset of an element's output") #284 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/284 | 17:11 |
jennis_ | my MR failed the pipeline because of 1 space :@ | 17:11 |
gitlab-br-bot | buildstream: issue #277 ("workspaces.yaml has no versioning mechanism") changed state ("opened") https://gitlab.com/BuildStream/buildstream/issues/277 | 17:19 |
gitlab-br-bot | buildstream: merge request (214-we-need-a-way-to-make-elements-depend-on-a-subset-of-an-element-s-output->master: Resolve "We need a way to make elements depend on a subset of an element's output") #284 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/284 | 17:27 |
*** jonathanmaw has quit IRC | 17:28 | |
*** noisecell has quit IRC | 17:28 | |
gitlab-br-bot | buildstream: merge request (239-use-pylint-for-linting->master: WIP: Resolve "Use pylint for linting") #283 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/283 | 17:32 |
gitlab-br-bot | buildstream: issue #214 ("We need a way to make elements depend on a subset of an element's output") changed state ("closed") https://gitlab.com/BuildStream/buildstream/issues/214 | 17:51 |
gitlab-br-bot | buildstream: merge request (214-we-need-a-way-to-make-elements-depend-on-a-subset-of-an-element-s-output->master: Resolve "We need a way to make elements depend on a subset of an element's output") #284 changed state ("merged"): https://gitlab.com/BuildStream/buildstream/merge_requests/284 | 17:51 |
*** Prince781 has joined #buildstream | 17:52 | |
*** Prince781 has quit IRC | 18:06 | |
*** Prince781 has joined #buildstream | 18:07 | |
*** valentind has joined #buildstream | 18:45 | |
*** Prince781 has quit IRC | 18:46 | |
*** Prince781 has joined #buildstream | 18:57 | |
*** Prince781 has quit IRC | 19:55 | |
*** xjuan has quit IRC | 19:56 | |
*** Prince781 has joined #buildstream | 19:56 | |
*** Prince781 has quit IRC | 20:13 | |
gitlab-br-bot | buildstream: merge request (juerg/git-describe->master: git.py: Output version key with git describe output) #291 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/291 | 20:34 |
*** tristan has quit IRC | 20:37 | |
*** Prince781 has joined #buildstream | 21:28 | |
gitlab-br-bot | buildstream: merge request (juerg/git-track-tags->master: git.py: Support tracking annotated tags in a branch) #303 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/303 | 21:30 |
*** Prince781 has quit IRC | 21:35 | |
*** aday has quit IRC | 21:46 | |
*** aday has joined #buildstream | 21:46 | |
*** aday has quit IRC | 21:50 | |
*** slaf has quit IRC | 22:26 | |
*** slaf has joined #buildstream | 22:31 | |
*** xjuan has joined #buildstream | 22:32 | |
*** valentind has quit IRC | 22:48 | |
*** xjuan has quit IRC | 23:06 | |
*** Prince781 has joined #buildstream | 23:19 |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!