*** Prince781 has joined #buildstream | 00:22 | |
*** Prince781_ has joined #buildstream | 01:44 | |
*** Prince781 has quit IRC | 01:46 | |
*** Prince781_ is now known as Prince781 | 01:46 | |
*** Prince781 has quit IRC | 06:12 | |
gitlab-br-bot | buildstream: merge request (juerg/tar-tracking->master: WIP: _downloadablefilesource.py: Support version tracking) #293 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/293 | 06:23 |
---|---|---|
*** tristan has joined #buildstream | 08:28 | |
*** jonathanmaw has joined #buildstream | 08:45 | |
*** lantw44 has quit IRC | 09:36 | |
*** lantw44 has joined #buildstream | 09:41 | |
gitlab-br-bot | buildstream: merge request (jmac/microsecond-timing->master: Optional microsecond timing for log messages) #275 changed state ("closed"): https://gitlab.com/BuildStream/buildstream/merge_requests/275 | 09:45 |
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 | 09:45 |
*** ssam2 has joined #buildstream | 10:01 | |
gitlab-br-bot | buildstream: issue #62 ("Extend the `tar` and `zip` source plugins to include `etag` support") changed state ("closed") https://gitlab.com/BuildStream/buildstream/issues/62 | 10:06 |
gitlab-br-bot | buildstream: merge request (juerg/etag->master: _downloadablefilesource.py: Add ETag support) #292 changed state ("merged"): https://gitlab.com/BuildStream/buildstream/merge_requests/292 | 10:06 |
jonathanmaw | tristan: do you have any thoughts on https://gitlab.com/BuildStream/buildstream/issues/214 ? i.e. using workspace commands on filter elements. The simplest way to implement that is to add a special case for when the element is a filter. More complicated is restructuring the workspacing logic so that elements implement the logic, and the filter element can override it. | 10:08 |
tristan | Errm | 10:08 |
tristan | jonathanmaw, we dont support workspaces on all elements, is there a reason to want to support it on filter elements ? | 10:09 |
tristan | Seems to me that filter elements would also not even have sources | 10:09 |
jonathanmaw | tristan: filter elements won't have sources, but it's a bit of a nuisance to have to look up the filter element to find out which element to actually open a workspace for | 10:10 |
jonathanmaw | so that you can make changes to the source code to see what happens | 10:10 |
tristan | I'm not sure why you would have to look it up | 10:10 |
tristan | Rather, you would already know that if you had designed a pipeline with a filter element | 10:11 |
juergbi | maybe returning a good error message (this element doesn't have any sources, here is a list of dependencies you might want to workspace instead) | 10:11 |
juergbi | would be sufficient | 10:11 |
juergbi | i don't like the idea of implicit redirection | 10:11 |
persia | Does a filter element support multiple build-dependencies? | 10:12 |
tristan | Ne neither, in the case of a filter though it might make sense to give a helpful error | 10:12 |
tristan | I.e. unlike compose elements; a filter is related to a very specific dependency | 10:12 |
tristan | (which is also an answer to persia's question) | 10:13 |
juergbi | persia: no but the error message would be in core buildstream | 10:13 |
persia | Ah. | 10:14 |
juergbi | and there are other element types without sources | 10:14 |
juergbi | well, maybe the error message could even be in the filter element. but i don't think it has to | 10:14 |
tristan | juergbi, ohhh, I had read your suggestion differently | 10:15 |
tristan | juergbi, I think that there is an opportunity for the filter element itself to print a more helpful message when it's asked to open a workspace | 10:15 |
juergbi | if that's necessary, sure. i was thinking this is potentially a filter-independent improvement | 10:16 |
tristan | Seems the message has an opportunity to be more helpful there | 10:16 |
*** dominic has joined #buildstream | 10:16 | |
tristan | While other source-less elements just dont make much sense to ask for a workspace for at all (like compose, or stack) | 10:17 |
*** tiago has joined #buildstream | 10:17 | |
tristan | juergbi, that said, there's nothing wrong with enhancing the general error message too | 10:17 |
persia | I can imagine wanting to `bst shell` into a compose element: dunno if that is related in any way. | 10:24 |
tristan | Not related | 10:26 |
tristan | But also equally unsupported | 10:26 |
tristan | Support in that case could be added or tried | 10:27 |
tristan | There is much less guarantees though that a compose element before composition (bst shell --build) or after composition (bst shell) would have a usable runtime in `/` | 10:27 |
jonathanmaw | okay, sounds like the solution is to have decent error reporting when you try to workspace an element without sources, then | 10:44 |
jonathanmaw | I'll make a note in the issue and get started | 10:44 |
jonathanmaw | s/workspace an element/open a workspace for an element/ | 10:44 |
gitlab-br-bot | buildstream: merge request (sam/233-push-after-pull->master: Avoid pushing things we just pulled) #276 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/276 | 10:48 |
*** aday has joined #buildstream | 10:49 | |
*** toscalix has joined #buildstream | 11:33 | |
jmac | So in order to demonstrate this UID problem, I'm going to try dbus - has anyone tried converting that to BuildStream yet? | 11:34 |
skullman | is it not built in the baserock conversion? | 11:34 |
juergbi | https://gitlab.com/freedesktop-sdk/freedesktop-sdk/blob/master/sdk/elements/desktop/dbus.bst | 11:35 |
juergbi | baserock probably has it as well, yes | 11:35 |
*** toscalix has quit IRC | 11:35 | |
jmac | Yes, found it | 11:40 |
*** toscalix has joined #buildstream | 11:44 | |
*** toscalix has quit IRC | 11:46 | |
*** toscalix has joined #buildstream | 11:47 | |
*** toscalix has quit IRC | 11:48 | |
*** toscalix has joined #buildstream | 11:48 | |
*** toscalix has quit IRC | 11:50 | |
*** toscalix has joined #buildstream | 11:51 | |
*** toscalix has quit IRC | 12:07 | |
*** ernestask has joined #buildstream | 12:09 | |
*** toscalix has joined #buildstream | 12:10 | |
*** toscalix has quit IRC | 12:20 | |
*** xjuan has joined #buildstream | 12:27 | |
*** toscalix has joined #buildstream | 12:33 | |
*** toscalix_ has joined #buildstream | 12:34 | |
tristan | juergbi, I noticed that we neglected to bump BST_FORMAT_VERSION with junctions | 12:35 |
tristan | juergbi, not a huge deal, but lets try to remember to do this, with e.g. the uid change | 12:36 |
*** toscalix has quit IRC | 12:36 | |
tristan | mentioning it directly to you since you are reviewing patches | 12:38 |
tristan | come to think of it; we should at least document the junctions stuff as "since format version 1" | 12:38 |
tristan | Since I recently bumped it | 12:38 |
tristan | Currently I'm doing this with a ".. note::" with a link to the format version section, like this: https://buildstream.gitlab.io/buildstream/projectconf.html#project-shell | 12:39 |
* tristan just pushed a small fix to document the `junction` dependency attribute as since version 1 | 12:47 | |
gitlab-br-bot | buildstream: merge request (sam/267-integration-cache->master: Make integration-tests work by default, and tweak how INTEGRATION_CACHE is interpreted) #294 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/294 | 12:52 |
gitlab-br-bot | buildstream: merge request (sam/267-integration-cache->master: Make integration-tests work by default, and tweak how INTEGRATION_CACHE is interpreted) #294 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/294 | 12:57 |
gitlab-br-bot | buildstream: merge request (sam/267-integration-cache->master: Make integration-tests work by default, and tweak how INTEGRATION_CACHE is interpreted) #294 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/294 | 12:59 |
jmac | When I run `bst shell --build <element>` I appear to be used 1000. How does that come about? | 12:59 |
jmac | s/used/user/ | 12:59 |
tristan | jmac, since last week, whether you want an isolated shell or not is no longer assumed by `--build` | 13:03 |
tristan | jmac, instead, use `--isolate` if you want an isolated sandbox (whether you pass --build or not) | 13:03 |
jmac | Oh yes, forgotten I'd updated to that | 13:06 |
*** tristan has quit IRC | 13:35 | |
*** mcatanzaro has joined #buildstream | 13:48 | |
*** jennis has quit IRC | 13:51 | |
*** tristan has joined #buildstream | 13:59 | |
*** jennis_ has joined #buildstream | 14:02 | |
*** tristan has quit IRC | 14:04 | |
*** tristan has joined #buildstream | 14:04 | |
ssam2 | juergbi, i'm a bit confused about https://gitlab.com/BuildStream/buildstream/merge_requests/276#note_60578035 | 14:07 |
ssam2 | we want to always use strong cache key here ... | 14:07 |
ssam2 | so surely _get_strict_cache_key() is correct? | 14:08 |
ssam2 | or do I need to do _get_cache_key(strength=_KeyStrength.STRONG) ? | 14:08 |
ssam2 | i suppose strength=_KeyStrength.STRONG is the default, so _get_cache_key() does the same thing | 14:10 |
ssam2 | nice to be explicit though | 14:10 |
gitlab-br-bot | buildstream: merge request (sam/233-push-after-pull->master: Avoid pushing things we just pulled) #276 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/276 | 14:14 |
juergbi | ssam2: being explicit is fine, of course. however, _get_strict_cache_key() would really be wrong in non-strict build plans | 14:45 |
ssam2 | wrong in what way ? | 14:46 |
juergbi | in strict build plans _get_strict_cache_key() is identical to _get_cache_key(strength=_KeyStrength.STRONG) | 14:46 |
ssam2 | but it's different in weak build plans ? | 14:47 |
juergbi | _get_strict_cache_key() remains the same across strict and non-strict build plans | 14:47 |
ssam2 | surely that's what we want in this case? | 14:47 |
ssam2 | regardless of build plan, we only want to look in the cache for the artifact under its strict cache key ? | 14:47 |
juergbi | i.e., it's as if all build dependencies had been built using the strict build plan | 14:47 |
juergbi | _get_cache_key(strength=_KeyStrength.STRONG) is based on the actual dependencies | 14:47 |
juergbi | the preferred cache lookup is the strict cache key | 14:48 |
ssam2 | sorry, i'm still a bit confused | 14:48 |
juergbi | however, we can't push an artifact with the strict cache key if we didn't build it | 14:48 |
juergbi | example :) | 14:48 |
juergbi | three packages: let's call them glibc, glib, and app | 14:48 |
juergbi | app depends on both libraries, glib depends on glibc | 14:49 |
juergbi | initial state is everything is freshly built with strict build plan, all fine | 14:49 |
juergbi | then we update glibc and rebuild only that (non-strict build plan) | 14:49 |
juergbi | then we update app and rebuild that as well but still in non-strict build plan, so we keep the old glib | 14:50 |
juergbi | that app build has a strong cache key based on the fresh strong cache key of glibc but the old strong cache key of glib | 14:50 |
juergbi | as the app build could theoretically be different than an app build with rebuilt glib | 14:51 |
ssam2 | right | 14:52 |
ssam2 | what would the strict cache key be in that case? | 14:52 |
ssam2 | or the app? | 14:52 |
ssam2 | *of the app | 14:52 |
juergbi | that would be what the app's strong cache key would look like in a strict build plan | 14:53 |
juergbi | i.e., based on strict cache keys of both dependencies | 14:53 |
juergbi | when we try to pull before building, we still prefer that one | 14:53 |
juergbi | but we can't build it without also rebuilding glib | 14:53 |
juergbi | which we skip because of non-strict build mode | 14:53 |
juergbi | it's confusing, we should really have better terms | 14:54 |
juergbi | does it make sense now? | 14:54 |
juergbi | anyway, i think that MR looks good now | 14:54 |
ssam2 | i think I get it | 14:55 |
ssam2 | so in strict mode, strong cache key == strict cache key | 14:55 |
juergbi | yes | 14:55 |
ssam2 | in no-strict mode, strong cache key == ... something hard to summarize in a sentence :-) | 14:56 |
juergbi | hehe | 14:56 |
ssam2 | and weak cache key is the same in whatever mode, I guess | 14:56 |
ssam2 | since deps aren't involved | 14:56 |
juergbi | when changing the cache key handling recently, i actually tried to get rid of that strict cache key | 14:56 |
juergbi | weak: yes | 14:56 |
juergbi | to avoid having two different strong cache keys for a single element in a single session | 14:57 |
juergbi | however, for this preferred cache lookup it was still necessary | 14:57 |
juergbi | maybe we could add an assertion or so to make sure that we don't use the strict cache key at the wrong point in time | 14:57 |
juergbi | i don't think we ever need _get_strict_cache_key() when the actual strong cache key is available | 14:58 |
juergbi | so we could potentially make them mutually exclusive | 14:58 |
ssam2 | right | 14:59 |
juergbi | at a given point in time in a session | 14:59 |
ssam2 | improving the documentation for that function would help too | 14:59 |
ssam2 | even if we just said, "you probably want _get_cache_key(strength=_KeyStrength.STRING)" | 14:59 |
juergbi | right, i should have done that | 14:59 |
juergbi | i guess the confusing thing was also that the artifact cache uses strict cache key a lot | 15:00 |
juergbi | but it's pretty much the only place that does, because of the lookups | 15:00 |
ssam2 | yeah | 15:00 |
juergbi | let me merge the branch and then i look at improving this | 15:01 |
gitlab-br-bot | buildstream: merge request (sam/233-push-after-pull->master: Avoid pushing things we just pulled) #276 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/276 | 15:04 |
juergbi | ah, we calculate the strong cache key as soon as we can, of course, and in strict build mode that's earlier than in non-strict build mode. so we can't just generally disallow strict cache key access when the strong cache key is available | 15:10 |
juergbi | and we don't want to delay it in strict build mode as we want to avoid ???????? keys in the frontend when we already know the strong cache key | 15:13 |
ssam2 | I think a comment would be fine | 15:14 |
ssam2 | it would be good if we could make _get_strict_cache_key() more private than _get_cache_key() | 15:14 |
ssam2 | since the latter is preferred in most cases | 15:15 |
ssam2 | but not sure how to do that, unless we make _get_cache_key() public | 15:15 |
juergbi | yes and it wouldn't have helper in the artifact cache case either | 15:17 |
juergbi | as that indeed needs access to it | 15:17 |
juergbi | maybe that part of the logic should actually be moved to Element | 15:17 |
juergbi | i.e., Element provides which cache key to check | 15:17 |
juergbi | not sure | 15:17 |
juergbi | it's definitely not ostree-specific logic | 15:17 |
juergbi | and then we could possibly make it __get_strict_cache_key() | 15:18 |
juergbi | (or rather, directly access the internal attribute) | 15:18 |
ssam2 | oh, so we'd have a _get_cache_key_for_caching_purposes() method instead ? | 15:18 |
juergbi | i was thinking of directly passing in the right cache key as argument | 15:20 |
juergbi | but maybe that doesn't work | 15:20 |
juergbi | i.e., make the ostreecache a bit dumber | 15:21 |
juergbi | just using whatever cache key is given | 15:21 |
juergbi | have you seen that before? https://gitlab.com/BuildStream/buildstream/-/jobs/54382697 | 15:21 |
juergbi | i assume it's a spurious failure, happened after rebase | 15:22 |
juergbi | but i don't remember seeing that before | 15:22 |
juergbi | oh, that looks like a python bug | 15:23 |
ssam2 | i saw that; weird | 15:23 |
juergbi | self.mounts['/'] = Mount(sandbox, '/', not root_readonly) | 15:23 |
juergbi | File "/builds/BuildStream/buildstream/dist/buildstream/buildstream/sandbox/_mount.py", line 57, in __init__ | 15:23 |
juergbi | os.makedirs(self.mount_source, exist_ok=True) | 15:23 |
juergbi | File "/usr/lib64/python3.6/os.py", line 220, in makedirs | 15:23 |
juergbi | mkdir(name, mode) | 15:23 |
juergbi | FileExistsError: [Errno 17] File exists: '/builds/BuildStream/buildstream/dist/buildstream/tmp/test_stack0/cache/build/stack-hi-2q7t0ph1/scratch/_/mount' | 15:23 |
juergbi | it calls makedirs with exist_ok=True but then it failes with File exists | 15:23 |
juergbi | either the python impl. has a race condition or 'mount' is a non-directory | 15:23 |
ssam2 | i imagine the latter | 15:25 |
juergbi | someone created a file called mount? | 15:26 |
juergbi | in a spurious failure? | 15:26 |
gitlab-br-bot | buildstream: issue #233 ("Do not push artifact that have been downloaded from the cache") changed state ("closed") https://gitlab.com/BuildStream/buildstream/issues/233 | 15:27 |
gitlab-br-bot | buildstream: merge request (sam/233-push-after-pull->master: Avoid pushing things we just pulled) #276 changed state ("merged"): https://gitlab.com/BuildStream/buildstream/merge_requests/276 | 15:27 |
ssam2 | not sure how it could happen, to be fair | 15:27 |
juergbi | based on the python code this seems indeed to be the case | 15:28 |
jmac | I don't think dbus is a good demonstration for user ID specification; the ultimate resulting artifact is the same whether you build as uid 0 or another uid, as far as I can tell | 15:59 |
*** Prince781 has joined #buildstream | 15:59 | |
juergbi | jmac: this doesn't behave as expected? https://cgit.freedesktop.org/dbus/dbus/tree/bus/Makefile.am#n273 | 16:05 |
juergbi | i would expect that to fail if DBUS_USER is not root | 16:05 |
juergbi | and the 4750 permission will currently be ignored by buildstream | 16:06 |
jmac | Yes, it takes a different branch there if userid is not 0 | 16:07 |
jmac | But the chmod cannot succeed in either case, and the setuid bit vanishes | 16:08 |
juergbi | i would expect it to succeed but it will be ignored when committing to the ostree artifact cache | 16:08 |
jmac | There is one difference though, writing 0750 would overwrite the 'other' bits | 16:08 |
juergbi | however, i would expect chown to fail, wondering why it doesn't | 16:08 |
jmac | Oops | 16:09 |
jmac | I meant chown fails, chmod succeeds. sorry. | 16:09 |
jmac | So it's just the 'other' bits in the permissions which survive | 16:09 |
jmac | It's a small difference, I suppose | 16:09 |
juergbi | btw: #38 is not about building as non-0 uid but about capturing those ownership/mode changes | 16:11 |
juergbi | so i still think it's a usable example | 16:11 |
jmac | For issue #38, yes, not for the explicit building-as-non-root change | 16:12 |
jmac | Well, it could be used for the latter case too but it's not very impressive | 16:13 |
juergbi | right | 16:14 |
juergbi | example for #242 is tar | 16:14 |
juergbi | it requires FORCE_UNSAFE_CONFIGURE=1 to ./configure with uid 0 | 16:15 |
gitlab-br-bot | buildstream: issue #274 ("Passing files across a `bst shell` sandbox boundry") changed state ("opened") https://gitlab.com/BuildStream/buildstream/issues/274 | 16:22 |
jjardon[m] | Hi! I noticed https://gitlab.com/BuildStream/buildstream/merge_requests/276 gor merged already (thanks ssam2 ! ) any chance to create a pre-release tag after the merge so we can use it easier in other projects? | 16:38 |
*** Prince781 has quit IRC | 17:15 | |
gitlab-br-bot | buildstream: merge request (sam/compose-symlinks-issue->master: Avoid compose element dropping files that are staged inside directory symlinks) #295 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/295 | 17:42 |
gitlab-br-bot | buildstream: merge request (sam/compose-symlinks-issue->master: Avoid compose element dropping files that are staged inside directory symlinks) #295 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/295 | 17:43 |
gitlab-br-bot | buildstream: merge request (sam/compose-symlinks-issue->master: Avoid compose element dropping files that are staged inside directory symlinks) #295 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/295 | 17:45 |
*** ssam2 has quit IRC | 17:49 | |
tristan | jjardon[m], I plan to roll out another release after fixing #274 and implementing the bind mounts discussed in the comment there (https://gitlab.com/BuildStream/buildstream/issues/274#note_60651068); possibly by the end of the week. | 17:52 |
gitlab-br-bot | buildstream: merge request (214-we-need-a-way-to-make-elements-depend-on-a-subset-of-an-element-s-output->master: WIP: 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:54 |
*** toscalix has joined #buildstream | 17:59 | |
*** jonathanmaw has quit IRC | 18:04 | |
jjardon[m] | tristan: cool, thanks | 18:10 |
*** toscalix has quit IRC | 18:10 | |
*** toscalix has joined #buildstream | 18:11 | |
*** dominic has quit IRC | 18:14 | |
*** toscalix has quit IRC | 18:27 | |
*** valentind has joined #buildstream | 19:27 | |
*** ernestask has quit IRC | 20:25 | |
*** mcatanzaro has quit IRC | 20:49 | |
*** xjuan has quit IRC | 21:06 | |
*** aday has quit IRC | 22:11 | |
*** tristan has quit IRC | 22:52 | |
*** valentind has quit IRC | 23:06 | |
*** valentind has joined #buildstream | 23:06 | |
*** valentind has quit IRC | 23:09 | |
*** valentind has joined #buildstream | 23:10 | |
*** slaf has quit IRC | 23:23 | |
*** slaf has joined #buildstream | 23:26 | |
*** valentind has quit IRC | 23:46 |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!