IRC logs for #buildstream for Monday, 2018-02-26

*** Prince781 has joined #buildstream00:22
*** Prince781_ has joined #buildstream01:44
*** Prince781 has quit IRC01:46
*** Prince781_ is now known as Prince78101:46
*** Prince781 has quit IRC06:12
gitlab-br-botbuildstream: merge request (juerg/tar-tracking->master: WIP: _downloadablefilesource.py: Support version tracking) #293 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/29306:23
*** tristan has joined #buildstream08:28
*** jonathanmaw has joined #buildstream08:45
*** lantw44 has quit IRC09:36
*** lantw44 has joined #buildstream09:41
gitlab-br-botbuildstream: merge request (jmac/microsecond-timing->master: Optional microsecond timing for log messages) #275 changed state ("closed"): https://gitlab.com/BuildStream/buildstream/merge_requests/27509:45
gitlab-br-botbuildstream: merge request (jmac/configurable-logging->master: Configurable log line formatting) #282 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/28209:45
*** ssam2 has joined #buildstream10:01
gitlab-br-botbuildstream: issue #62 ("Extend the `tar` and `zip` source plugins to include `etag` support") changed state ("closed") https://gitlab.com/BuildStream/buildstream/issues/6210:06
gitlab-br-botbuildstream: merge request (juerg/etag->master: _downloadablefilesource.py: Add ETag support) #292 changed state ("merged"): https://gitlab.com/BuildStream/buildstream/merge_requests/29210:06
jonathanmawtristan: 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
tristanErrm10:08
tristanjonathanmaw, we dont support workspaces on all elements, is there a reason to want to support it on filter elements ?10:09
tristanSeems to me that filter elements would also not even have sources10:09
jonathanmawtristan: 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 for10:10
jonathanmawso that you can make changes to the source code to see what happens10:10
tristanI'm not sure why you would have to look it up10:10
tristanRather, you would already know that if you had designed a pipeline with a filter element10:11
juergbimaybe 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
juergbiwould be sufficient10:11
juergbii don't like the idea of implicit redirection10:11
persiaDoes a filter element support multiple build-dependencies?10:12
tristanNe neither, in the case of a filter though it might make sense to give a helpful error10:12
tristanI.e. unlike compose elements; a filter is related to a very specific dependency10:12
tristan(which is also an answer to persia's question)10:13
juergbipersia: no but the error message would be in core buildstream10:13
persiaAh.10:14
juergbiand there are other element types without sources10:14
juergbiwell, maybe the error message could even be in the filter element. but i don't think it has to10:14
tristanjuergbi, ohhh, I had read your suggestion differently10:15
tristanjuergbi, 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 workspace10:15
juergbiif that's necessary, sure. i was thinking this is potentially a filter-independent improvement10:16
tristanSeems the message has an opportunity to be more helpful there10:16
*** dominic has joined #buildstream10:16
tristanWhile 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 #buildstream10:17
tristanjuergbi, that said, there's nothing wrong with enhancing the general error message too10:17
persiaI can imagine wanting to `bst shell` into a compose element: dunno if that is related in any way.10:24
tristanNot related10:26
tristanBut also equally unsupported10:26
tristanSupport in that case could be added or tried10:27
tristanThere 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
jonathanmawokay, sounds like the solution is to have decent error reporting when you try to workspace an element without sources, then10:44
jonathanmawI'll make a note in the issue and get started10:44
jonathanmaws/workspace an element/open a workspace for an element/10:44
gitlab-br-botbuildstream: 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/27610:48
*** aday has joined #buildstream10:49
*** toscalix has joined #buildstream11:33
jmacSo in order to demonstrate this UID problem, I'm going to try dbus - has anyone tried converting that to BuildStream yet?11:34
skullmanis it not built in the baserock conversion?11:34
juergbihttps://gitlab.com/freedesktop-sdk/freedesktop-sdk/blob/master/sdk/elements/desktop/dbus.bst11:35
juergbibaserock probably has it as well, yes11:35
*** toscalix has quit IRC11:35
jmacYes, found it11:40
*** toscalix has joined #buildstream11:44
*** toscalix has quit IRC11:46
*** toscalix has joined #buildstream11:47
*** toscalix has quit IRC11:48
*** toscalix has joined #buildstream11:48
*** toscalix has quit IRC11:50
*** toscalix has joined #buildstream11:51
*** toscalix has quit IRC12:07
*** ernestask has joined #buildstream12:09
*** toscalix has joined #buildstream12:10
*** toscalix has quit IRC12:20
*** xjuan has joined #buildstream12:27
*** toscalix has joined #buildstream12:33
*** toscalix_ has joined #buildstream12:34
tristanjuergbi, I noticed that we neglected to bump BST_FORMAT_VERSION with junctions12:35
tristanjuergbi, not a huge deal, but lets try to remember to do this, with e.g. the uid change12:36
*** toscalix has quit IRC12:36
tristanmentioning it directly to you since you are reviewing patches12:38
tristancome to think of it; we should at least document the junctions stuff as "since format version 1"12:38
tristanSince I recently bumped it12:38
tristanCurrently 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-shell12:39
* tristan just pushed a small fix to document the `junction` dependency attribute as since version 112:47
gitlab-br-botbuildstream: 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/29412:52
gitlab-br-botbuildstream: 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/29412:57
gitlab-br-botbuildstream: 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/29412:59
jmacWhen I run `bst shell --build <element>` I appear to be used 1000. How does that come about?12:59
jmacs/used/user/12:59
tristanjmac, since last week, whether you want an isolated shell or not is no longer assumed by `--build`13:03
tristanjmac, instead, use `--isolate` if you want an isolated sandbox (whether you pass --build or not)13:03
jmacOh yes, forgotten I'd updated to that13:06
*** tristan has quit IRC13:35
*** mcatanzaro has joined #buildstream13:48
*** jennis has quit IRC13:51
*** tristan has joined #buildstream13:59
*** jennis_ has joined #buildstream14:02
*** tristan has quit IRC14:04
*** tristan has joined #buildstream14:04
ssam2juergbi, i'm a bit confused about https://gitlab.com/BuildStream/buildstream/merge_requests/276#note_6057803514:07
ssam2we want to always use strong cache key here ...14:07
ssam2so surely _get_strict_cache_key() is correct?14:08
ssam2or do I need to do _get_cache_key(strength=_KeyStrength.STRONG) ?14:08
ssam2i suppose strength=_KeyStrength.STRONG is the default, so _get_cache_key() does the same thing14:10
ssam2nice to be explicit though14:10
gitlab-br-botbuildstream: 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/27614:14
juergbissam2: being explicit is fine, of course. however, _get_strict_cache_key() would really be wrong in non-strict build plans14:45
ssam2wrong in what way ?14:46
juergbiin strict build plans _get_strict_cache_key() is identical to _get_cache_key(strength=_KeyStrength.STRONG)14:46
ssam2but it's different in weak build plans ?14:47
juergbi_get_strict_cache_key() remains the same across strict and non-strict build plans14:47
ssam2surely that's what we want in this case?14:47
ssam2regardless of build plan, we only want to look in the cache for the artifact under its strict cache key ?14:47
juergbii.e., it's as if all build dependencies had been built using the strict build plan14:47
juergbi_get_cache_key(strength=_KeyStrength.STRONG) is based on the actual dependencies14:47
juergbithe preferred cache lookup is the strict cache key14:48
ssam2sorry, i'm still a bit confused14:48
juergbihowever, we can't push an artifact with the strict cache key if we didn't build it14:48
juergbiexample :)14:48
juergbithree packages: let's call them glibc, glib, and app14:48
juergbiapp depends on both libraries, glib depends on glibc14:49
juergbiinitial state is everything is freshly built with strict build plan, all fine14:49
juergbithen we update glibc and rebuild only that (non-strict build plan)14:49
juergbithen we update app and rebuild that as well but still in non-strict build plan, so we keep the old glib14:50
juergbithat app build has a strong cache key based on the fresh strong cache key of glibc but the old strong cache key of glib14:50
juergbias the app build could theoretically be different than an app build with rebuilt glib14:51
ssam2right14:52
ssam2what would the strict cache key be in that case?14:52
ssam2or the app?14:52
ssam2*of the app14:52
juergbithat would be what the app's strong cache key would look like in a strict build plan14:53
juergbii.e., based on strict cache keys of both dependencies14:53
juergbiwhen we try to pull before building, we still prefer that one14:53
juergbibut we can't build it without also rebuilding glib14:53
juergbiwhich we skip because of non-strict build mode14:53
juergbiit's confusing, we should really have better terms14:54
juergbidoes it make sense now?14:54
juergbianyway, i think that MR looks good now14:54
ssam2i think I get it14:55
ssam2so in strict mode, strong cache key == strict cache key14:55
juergbiyes14:55
ssam2in no-strict mode, strong cache key == ... something hard to summarize in a sentence :-)14:56
juergbihehe14:56
ssam2and weak cache key is the same in whatever mode, I guess14:56
ssam2since deps aren't involved14:56
juergbiwhen changing the cache key handling recently, i actually tried to get rid of that strict cache key14:56
juergbiweak: yes14:56
juergbito avoid having two different strong cache keys for a single element in a single session14:57
juergbihowever, for this preferred cache lookup it was still necessary14:57
juergbimaybe we could add an assertion or so to make sure that we don't use the strict cache key at the wrong point in time14:57
juergbii don't think we ever need _get_strict_cache_key() when the actual strong cache key is available14:58
juergbiso we could potentially make them mutually exclusive14:58
ssam2right14:59
juergbiat a given point in time in a session14:59
ssam2improving the documentation for that function would help too14:59
ssam2even if we just said, "you probably want _get_cache_key(strength=_KeyStrength.STRING)"14:59
juergbiright, i should have done that14:59
juergbii guess the confusing thing was also that the artifact cache uses strict cache key a lot15:00
juergbibut it's pretty much the only place that does, because of the lookups15:00
ssam2yeah15:00
juergbilet me merge the branch and then i look at improving this15:01
gitlab-br-botbuildstream: 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/27615:04
juergbiah, 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 available15:10
juergbiand 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 key15:13
ssam2I think a comment would be fine15:14
ssam2it would be good if we could make _get_strict_cache_key() more private than _get_cache_key()15:14
ssam2since the latter is preferred in most cases15:15
ssam2but not sure how to do that, unless we make _get_cache_key() public15:15
juergbiyes and it wouldn't have helper in the artifact cache case either15:17
juergbias that indeed needs access to it15:17
juergbimaybe that part of the logic should actually be moved to Element15:17
juergbii.e., Element provides which cache key to check15:17
juergbinot sure15:17
juergbiit's definitely not ostree-specific logic15:17
juergbiand then we could possibly make it __get_strict_cache_key()15:18
juergbi(or rather, directly access the internal attribute)15:18
ssam2oh, so we'd have a _get_cache_key_for_caching_purposes() method instead ?15:18
juergbii was thinking of directly passing in the right cache key as argument15:20
juergbibut maybe that doesn't work15:20
juergbii.e., make the ostreecache a bit dumber15:21
juergbijust using whatever cache key is given15:21
juergbihave you seen that before? https://gitlab.com/BuildStream/buildstream/-/jobs/5438269715:21
juergbii assume it's a spurious failure, happened after rebase15:22
juergbibut i don't remember seeing that before15:22
juergbioh, that looks like a python bug15:23
ssam2i saw that; weird15: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 makedirs15: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
juergbiit calls makedirs with exist_ok=True but then it failes with File exists15:23
juergbieither the python impl. has a race condition or 'mount' is a non-directory15:23
ssam2i imagine the latter15:25
juergbisomeone created a file called mount?15:26
juergbiin a spurious failure?15:26
gitlab-br-botbuildstream: issue #233 ("Do not push artifact that have been downloaded from the cache") changed state ("closed") https://gitlab.com/BuildStream/buildstream/issues/23315:27
gitlab-br-botbuildstream: 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/27615:27
ssam2not sure how it could happen, to be fair15:27
juergbibased on the python code this seems indeed to be the case15:28
jmacI 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 tell15:59
*** Prince781 has joined #buildstream15:59
juergbijmac: this doesn't behave as expected? https://cgit.freedesktop.org/dbus/dbus/tree/bus/Makefile.am#n27316:05
juergbii would expect that to fail if DBUS_USER is not root16:05
juergbiand the 4750 permission will currently be ignored by buildstream16:06
jmacYes, it takes a different branch there if userid is not 016:07
jmacBut the chmod cannot succeed in either case, and the setuid bit vanishes16:08
juergbii would expect it to succeed but it will be ignored when committing to the ostree artifact cache16:08
jmacThere is one difference though, writing 0750 would overwrite the 'other' bits16:08
juergbihowever, i would expect chown to fail, wondering why it doesn't16:08
jmacOops16:09
jmacI meant chown fails, chmod succeeds. sorry.16:09
jmacSo it's just the 'other' bits in the permissions which survive16:09
jmacIt's a small difference, I suppose16:09
juergbibtw: #38 is not about building as non-0 uid but about capturing those ownership/mode changes16:11
juergbiso i still think it's a usable example16:11
jmacFor issue #38, yes, not for the explicit building-as-non-root change16:12
jmacWell, it could be used for the latter case too but it's not very impressive16:13
juergbiright16:14
juergbiexample for #242 is tar16:14
juergbiit requires FORCE_UNSAFE_CONFIGURE=1 to ./configure with uid 016:15
gitlab-br-botbuildstream: issue #274 ("Passing files across a `bst shell` sandbox boundry") changed state ("opened") https://gitlab.com/BuildStream/buildstream/issues/27416: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 IRC17:15
gitlab-br-botbuildstream: 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/29517:42
gitlab-br-botbuildstream: 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/29517:43
gitlab-br-botbuildstream: 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/29517:45
*** ssam2 has quit IRC17:49
tristanjjardon[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-botbuildstream: 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/28417:54
*** toscalix has joined #buildstream17:59
*** jonathanmaw has quit IRC18:04
jjardon[m]tristan: cool, thanks18:10
*** toscalix has quit IRC18:10
*** toscalix has joined #buildstream18:11
*** dominic has quit IRC18:14
*** toscalix has quit IRC18:27
*** valentind has joined #buildstream19:27
*** ernestask has quit IRC20:25
*** mcatanzaro has quit IRC20:49
*** xjuan has quit IRC21:06
*** aday has quit IRC22:11
*** tristan has quit IRC22:52
*** valentind has quit IRC23:06
*** valentind has joined #buildstream23:06
*** valentind has quit IRC23:09
*** valentind has joined #buildstream23:10
*** slaf has quit IRC23:23
*** slaf has joined #buildstream23:26
*** valentind has quit IRC23:46

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