IRC logs for #buildstream for Monday, 2018-11-19

*** mohan43u has joined #buildstream05:23
*** jesopo has joined #buildstream05:24
*** jit10 has joined #buildstream06:19
gitlab-br-botjuergbi opened (was WIP) MR !915 (juerg/command-batching->master: Command batching) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/91507:26
*** tristan has joined #buildstream08:00
juergbihi tristan, in case you're up for some (API) reviewing, I've updated my command batching MR: https://gitlab.com/BuildStream/buildstream/merge_requests/91508:25
*** toscalix has joined #buildstream08:39
*** benschubert has joined #buildstream09:13
*** scadoosh has joined #buildstream09:28
*** rdale has joined #buildstream09:38
benschubertskullman: would you have time to look at https://gitlab.com/BuildStream/buildstream/merge_requests/895#note_116963234 ? Thanks!09:46
gitlab-br-bottoscalix opened issue #769 (Improve the documentation related with the disk space quota used by default by BuildStream.) on buildstream https://gitlab.com/BuildStream/buildstream/issues/76909:47
skullmanbenschubert: looking at it now09:47
*** tiagogomes has quit IRC09:49
toscalixthat #769 ticket was in my todo as consequence of a discussion a couple of weeks ago of the #733 ticket, openned by jjardon09:50
gitlab-br-botIssue #733: quota config parameter should be the maximum amount of cache allowed, not the minimum https://gitlab.com/BuildStream/buildstream/issues/73309:50
*** tiagogomes has joined #buildstream09:50
skullmanbenschubert: replied09:54
benschubertthanks09:54
*** jonathanmaw has joined #buildstream09:59
*** phildawson_ has quit IRC10:07
*** phildawson has joined #buildstream10:07
phildawsoncan anyone explain this CI failure to me? https://gitlab.com/BuildStream/buildstream/commit/f067397bd2560ae62be99254cdde8315c3e636ac/pipelines?ref=phil%2Fsource-checkout-options10:07
Kinnisonwow, that looks broken10:08
KinnisonHave you hit retry?10:09
Kinnisondoes it repeat?10:09
phildawsonYup10:09
Kinnisonhrm10:09
phildawsonMy thought exactly10:09
KinnisonDid you re-push between the pipeline running?10:09
phildawsonI did not10:09
Kinnisonis the listed SHA a treeish in your repo?10:09
Nexusoh i've had that before10:09
* Nexus thinks10:09
Nexusi think that's when i accidentally rebased to a pre-master commit10:10
benschubertjuergbi: I'm not sure i understand everything about your comment in https://gitlab.com/BuildStream/buildstream/merge_requests/895#note_118194846 would you mind explaining it a bit more to me?10:11
juergbibenschubert: !915 introduces (optional) command batching where the sandbox.run() calls will be added to a sandbox-internal batch object and the commands execution is deferred until the batch context ends10:12
gitlab-br-botMR !915: Command batching https://gitlab.com/BuildStream/buildstream/merge_requests/91510:12
juergbian effect of this is that the sandbox will be responsible for raising errors for command failures if they were executed as part of a command batch10:13
*** raoul has joined #buildstream10:13
juergbiin the MR I raise SandboxError as that's no longer directly in the Element class hierarchy10:13
phildawsonKinnison, it looks like the SHA gitlab is using for the pipeline is different to the head I thought I pushed. I'll try again and let you know how it goes.10:13
Kinnisongood luck :-)10:14
juergbibenschubert: however, with regards to caching build failures it shouldn't be treated differently from non-batch command failures even though it's a SandboxError10:14
phildawsonnope, git thinks everything is up to date.10:14
benschubertOh and then since I don't cache them anymore in my MR this would break I see. Would it be cleaner to propagate a "BatchCommandError" Instead, like a new exception instead of the sandbox?10:14
phildawsonKinnison, This branch is based on something which may have gone a while without a rebase. Has there been any changes that might be causing this by their absence10:15
toscalixI have been doing cleaning up in tickets this morning10:15
Kinnisonphildawson: It's possible that docker containers have changed, rebasing so that gitlab-ci.yml is up to date may be sensible10:16
juergbibenschubert: could be an option, or maybe rather a generic CommandError that could also be raised by plugins. not sure about best practice with regards to exception class hierarchies10:18
phildawsonThanks Kinnison10:18
gitlab-br-botBenjaminSchubert closed issue #758 (the pip source plugin incorrectly search for a valid version) on buildstream https://gitlab.com/BuildStream/buildstream/issues/75810:19
gitlab-br-botBenjaminSchubert merged MR !942 (bschubert/fix-pip-python->master: plugins/sources/pip.py: also look for python version named "python") on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/94210:19
*** lachlan has joined #buildstream10:19
*** ctolentino has joined #buildstream10:20
benschubertMmh true10:20
benschubertjuergbi: It seems to me that not reusing exceptions too much would be good and keeping sandbox related errors in the sandbox, and commands related ones separately would be better since we would be more sure about what we are catching. However adding the "artifactErrors" too makes it also a bit much in term of complexity (however it could be as simple as "except (SandboxError, ArtifactError): raise; except BstError: setup caching10:23
benschubertelements" which is still acceptable for my PR (don't know how much work it would be on your?) What would you prefer?10:23
juergbibenschubert: shouldn't we rather inverse it? explicitly save artifacts for ElementError and SandboxCommandError?10:24
juergbibecause e.g. for buildstream core or plugin bugs (non-BstError exceptions) we don't want to save it, right?10:25
juergbidue to this I would default to not saving as per my comment10:25
benschubertOh right that makes lots of sense!10:28
benschubertThe SandboxCommandError doesn't exist yet right? Is it okl if I let you add it and depending on which changes lands first we update this part?10:28
juergbiyes, I think that makes sense10:29
benschubertOk I'll update this!10:30
juergbita10:30
benschubertAnd what about the shelling in the sandbox, do we still want that for every BstError?10:32
benschubertor should we just do it for ElementError?10:32
juergbiI'd tend towards using the same check as for artifact saving. skullman: does this make sense to you as well?10:37
skullmanaye, makes sense to me10:38
benschubertjuergbi: skullman does the new approach for !895 seems good to you?10:43
gitlab-br-botMR !895: Don't cache sandbox failures https://gitlab.com/BuildStream/buildstream/merge_requests/89510:43
skullmanlooking at the changes now10:43
*** alatiera_ has joined #buildstream10:44
tiagogomesSo the new buildstream website looks already dead. There's no reference whatsoever to the gathering there10:47
juergbibenschubert: hm, aren't we now missing caching on success? or am I misreading the code?10:47
benschubertthe last line in the "try" should do right?10:47
benschubertWould you rather have a boolean check and the caching in the finally?10:48
benschubertI found it  more readable like that but can revert :)10:48
juergbiah, missed that line in the diff10:48
skullmanbenschubert: makes sense to me, looks neater. Haven't tried the changes out so I may have missed something.10:51
mablanchlachlan: I can't approve from BuildStream/benchmarks, but !10 looks good, I think you can merge that fix.10:51
gitlab-br-botMR !10: Sphinx to not assume python3 is the default python version https://gitlab.com/BuildStream/buildstream/merge_requests/1010:51
juergbibenschubert: I agree, structure looks good to me10:52
benschubertOk, so if the tests pass I can merge?10:52
benschubertor is there something else on this one that would require more work?10:52
*** alatiera_ has joined #buildstream10:54
gitlab-br-botjjardon closed issue #769 (Improve the documentation related with the disk space quota used by default by BuildStream.) on buildstream https://gitlab.com/BuildStream/buildstream/issues/76910:56
juergbibenschubert: I commented with a couple of nits but looks good to merge to me otherwise10:57
benschubertThanks a lot! I will address them and merge if I don't get any more questions :)10:57
juergbita10:58
benschubertand maybe if you had time for !938, it's currently preventing me from running buildstream tests on a more powerfull machine10:59
gitlab-br-botMR !938: Fix os.rename in git source element to correctly handle error codes https://gitlab.com/BuildStream/buildstream/merge_requests/93810:59
juergbiskullman: I don't know whether you saw this already but I've included a commit (unmodified) from !920 in !953. I assume that's fine with you (in case my MR gets merged earlier), otherwise let me know11:02
gitlab-br-botMR !920: Add artifact log command https://gitlab.com/BuildStream/buildstream/merge_requests/92011:02
gitlab-br-botMR !953: _context.py: Drop duplicated default values for user configuration https://gitlab.com/BuildStream/buildstream/merge_requests/95311:02
skullmanjuergbi: I saw and yes it's fine.11:02
juergbita11:04
lachlanThanks: mablanch11:23
gitlab-br-botrichardmaw-codethink approved MR !938 (bschubert/fix-atomic-move-git-repo->master: Fix os.rename in git source element to correctly handle error codes) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/93811:39
gitlab-br-botjuergbi approved MR !895 (bschubert/dont-cache-errors-from-host-tools->master: Don't cache sandbox failures) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/89511:41
*** alatiera_ has quit IRC11:52
gitlab-br-bottlater approved MR !938 (bschubert/fix-atomic-move-git-repo->master: Fix os.rename in git source element to correctly handle error codes) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/93812:05
*** lachlan has quit IRC12:07
*** smkelly has joined #buildstream12:08
jennisI'm importing a tarball (which I can't change) that contains a broken symlink: /bin/foo -> /something/else/bar, now /something/else/bar doesn't actually exist, and I want to relink /bin/foo to /another/thing/baz12:11
jennisWhat would be the best way to do this? I'm thinking some kind of script element that sits on top of the import element and then bundling both together with a stack element12:12
jennisBut I wonder if there is a nicer way to do this?12:13
jennisOr if there are any potential flaws in my proposed method12:13
jmacCan you just import another element on top which just contains /another/thing/baz and the symlink to it?12:15
juergbijennis: I think this would work but you could potentially also combine the two. i.e., just use the import source as source in a script element and create combined output12:15
juergbior maybe even import with a second source which is a local symlink. not sure whether that would work12:17
jennisI will try these options12:22
gitlab-br-botBenjaminSchubert merged MR !938 (bschubert/fix-atomic-move-git-repo->master: Fix os.rename in git source element to correctly handle error codes) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/93812:22
jennisBit worried that I'm not actually going to have the permission to relink this12:22
jennisthanks12:22
*** lachlan has joined #buildstream12:23
jennisjuergbi, sources are forbidden in a script element btw12:31
juergbihm, didn't realize that12:31
juergbiI guess it would kind of conflict with the layouting12:32
cs-shadowjuergbi: hi. Before I land source-checkout command, I wanted to ensure that we are on the same page regarding https://gitlab.com/BuildStream/buildstream/merge_requests/820#note_113915907 so please have a look at it when you get a chance.12:33
juergbilgtm12:34
cs-shadowace, thanks!12:34
*** adds68 has joined #buildstream12:35
adds68Hi, in reference to: https://gitlab.com/BuildStream/buildstream/issues/76412:38
adds68Could anyone give me any idea on what "provenance" data is?12:38
jonathanmawadds68: provenance data is an object attached to a dict which says which line in which source file the data came from12:39
* jonathanmaw checks the MR12:39
jonathanmawyeah, that's the kind of provenance data there12:41
adds68jonathanmaw, so from that error, a function is somehow ending up in the provenance data?12:43
jonathanmawadds68: that'd be my guess12:44
adds68hmmm, is not sure where to even look to debug :(12:45
jonathanmawadds68: I'd suggest looking at collect_manifest.py, line 145, and check whether the manifest being put in the public data contains only objects that ought to be in a yaml dict12:46
jonathanmawjudicious use of print statements is usually my first port of call12:48
adds68yea good point, it seems to work on out bootstrap project, but not in the platform project12:51
juergbibenschubert: a bit late, but I've added a few post-merge comments to !938. will you keep track of those or shall I move them to an issue?12:53
gitlab-br-botMR !938: Fix os.rename in git source element to correctly handle error codes https://gitlab.com/BuildStream/buildstream/merge_requests/93812:53
jennisjmac, your suggestion has worked well for me, thanks!12:54
benschubertjuergbi: oups sorry about merging it too soon. I will create a following mr after lunch12:56
juergbita12:56
gitlab-br-botcs-shadow merged MR !820 (chandan/source-checkout->master: Add `bst source-checkout` command) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/82013:03
*** Jackneill has joined #buildstream13:36
*** lachlan has quit IRC13:57
*** lachlan has joined #buildstream14:13
*** phildawson has quit IRC14:22
jennisDo we have optional configuration to *not* remotely cache stuff?14:42
jennisFor example, if I have a cache with push access and build an element from gnome-build-meta, this will depend on elements from freedesktop, which means I can pull freedesktop elements from their cache, but these will also get pushed to my cache, as well as the gnome stuff. So effectively, the freedesktop artifacts are cached in two remotes14:44
jennisBasically, I don't want to have the junctioned dependencies in my cache14:44
jmacWe have no per-element configuration for artifact caches14:52
*** phildawson has joined #buildstream14:53
juergbijennis: jmac is right with regards to pushing, however, with pulling buildtrees disabled by default now, a side effect is that we will actually no longer push pulled elements14:54
juergbi(unless you enable pullin buildtrees)14:54
juergbimight change again, though14:55
gitlab-br-botvalentindavid approved MR !947 (abderrahim/cmake-variable-types->master: plugins/elements/cmake.yaml: always specify variable types) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/94714:55
jennisOh, that's a great side affect14:57
jenniseffect14:57
jennisMhmm, because we have the functionality to just push one element/artifact without it's dependencies14:57
jennisIt's just, by default, when we bst build, dependencies are pushed14:58
jennisSo I'm asking for something like `bst build --push-deps none` foo.bst14:59
jenniswhere the default of --push-deps is all14:59
jenniswhich is what we essentially have now15:00
tiagogomesjennis why don't you use bst push for that15:02
valentindadds68, I am closing #235, because we have an alternative approach now.15:06
gitlab-br-botIssue #235: Add the ability to generate manifest files https://gitlab.com/BuildStream/buildstream/issues/23515:06
gitlab-br-botvalentindavid closed issue #235 (Add the ability to generate manifest files) on buildstream https://gitlab.com/BuildStream/buildstream/issues/23515:07
jennistiagogomes, because I don't want my build to push dependencies from the junction15:12
jennisso the only way I can do it now is build locally without declaring a remote cache, declare a remote cache and then `bst push` those elements15:13
juergbics-shadow: can you please add a NEWS entry for source-checkout?15:14
benschubertjennis: would "--pushers 0" be a solution when doing bst build?15:17
benschubertthat would avoids the need of editing the file every time15:18
NexusCan someone link me to a good description of what junctions are and how they work? :)15:19
jennisbenschubert, I've not tried that, that's a nice idea though, but I still need to do a separate build and then push job, but I like it15:19
jennisNexus, they are elements that represent another buildstream project15:19
jenniselements in that project can be addressed with junction-name.bst:element-in-junction.bst15:20
jennisI would recommend the tutorial in our documentation15:20
jennishttps://docs.buildstream.build/advanced-features/junction-elements.html15:20
Nexusty15:21
benschubertNexus: would you have time to reply to https://gitlab.com/BuildStream/buildstream/merge_requests/949#note_118191077 ? :)15:21
Nexusbenschubert: sure gimme a sec15:22
tiagogomesbenschubert unless there was some recent change, that means infinite number of fetchers15:22
benschuberttiagogomes: ooups, got mistaken with another project then, sorry jennis15:23
Nexusbenschubert: done15:24
benschubertta!15:24
Nexusjennis: Any idea where in the codebase junctions are interpreted?15:25
gitlab-br-botvalentindavid closed issue #761 (cmake: CMAKE_INSTALL_LIBDIR isn't properly set) on buildstream https://gitlab.com/BuildStream/buildstream/issues/76115:31
gitlab-br-botvalentindavid merged MR !947 (abderrahim/cmake-variable-types->master: plugins/elements/cmake.yaml: always specify variable types) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/94715:31
jennisbenschubert , tiagogomes `--pushers 0` still pushes15:32
jennisNexus, not off the top of my head, I'd just be grep'ing15:33
Nexuskk :)15:34
tiagogomesjennis yeah, what you need is perhaps https://gitlab.com/BuildStream/buildstream/merge_requests/95015:34
jennistiagogomes, this is partially what I need yes15:35
jennisthanks15:35
cs-shadowjuergbi: sure, was just about to do that15:46
abderrahim[m]Hi, how do I backport a MR to 1.2? Is the cherry-pick button on the MR the right thing?15:48
gitlab-br-botBenjaminSchubert opened MR !958 (bschubert/mr938-comments->master: Followup on MR 938, addr4essing additional comments) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/95815:51
benschubertjuergbi: could you validate that !958 adresses your points correctly? thanks!15:51
*** [TwistedJail] has joined #buildstream15:57
gitlab-br-botcs-shadow opened issue #771 (Support multiple elements in checkout and source-checkout) on buildstream https://gitlab.com/BuildStream/buildstream/issues/77116:05
toscalixjonathanmaw: , you and Chandan have a point in "repo per plugin" idea. I will evaluate if we can specify the policy in the line of what I propose it removing the hard link between repo and plugin. I cannot today but I will try tomorrow16:21
jonathanmawta toscalix16:22
gitlab-br-botBenjaminSchubert closed issue #727 (Don't cache host tools failures) on buildstream https://gitlab.com/BuildStream/buildstream/issues/72716:29
gitlab-br-botBenjaminSchubert merged MR !895 (bschubert/dont-cache-errors-from-host-tools->master: Don't cache sandbox failures) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/89516:29
benschubertNexus: I would rather not add the marker per file to keep it consistent with before, would that seem fair to you?16:29
Nexusbenschubert: yeah thats fine16:30
benschubertthanks!16:31
benschubertI'll rebase and merge then16:31
jonathanmawhrm, I'm trying to make it optional to specify the element when running buildstream commands, and I'm not sure if it's possible to do it in a non-confusing way. I've marked the 'element' argument with required=False, but there doesn't seem to be a way to actually separate optional args16:37
jonathanmawI gather that's why skullman ended up adding "-e" to add extra elements in !90916:38
gitlab-br-botMR !909: Allow staging multiple elements in `bst shell` https://gitlab.com/BuildStream/buildstream/merge_requests/90916:38
skullmanjonathanmaw: yep16:38
jonathanmawhrm16:39
jonathanmawIf people expect normal things (like '--' separating out the variable-length args) then I can't really make it optional16:40
gitlab-br-botphildawson opened MR !959 (phil/source-checkout-options->master: Phil/source checkout options) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/95916:43
jonathanmawon the other hand, there is *some* functionality, if you don't plan to specify a command to be run, it works.16:44
gitlab-br-botcs-shadow opened MR !960 (chandan/source-checkout-news->master: NEWS: Add entry for the new source-checkout command) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/96016:44
adds68jonathanmaw, i've managed to get the manifest json data to print a few times from the plugin, however now it seems to have stopped? I've deleted my local artifacts cache, but it still will not rebuild, any idea what could cause this?16:46
cs-shadowcan someone please do a quick sanity check of https://gitlab.com/BuildStream/buildstream/merge_requests/96016:48
gitlab-br-botjmacarthur approved MR !920 (richardmaw/artifact-log->master: Add artifact log command) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/92016:50
jonathanmawcs-shadow: looks fine to me16:51
gitlab-br-botphildawson approved MR !960 (chandan/source-checkout-news->master: NEWS: Add entry for the new source-checkout command) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/96016:51
cs-shadowjonathanmaw: thanks!16:51
cs-shadowphildawson: thanks :)16:52
jonathanmawadds68: hmm, I occasionally see print statements getting overwritten/swallowed by other output. How about if you replace the print(text) with sys.stderr.write(text+'\n')?16:53
adds68jonathanmaw, still nothing, even with --debug engabled16:55
*** lachlan has quit IRC16:56
jonathanmawadds68: I suppose in assemble you should be able to use self.info() to print messages17:05
gitlab-br-botadds68 opened issue #772 (Buildstream plugin output being suppressed) on buildstream https://gitlab.com/BuildStream/buildstream/issues/77217:12
*** lachlan has joined #buildstream17:12
gitlab-br-botcs-shadow merged MR !960 (chandan/source-checkout-news->master: NEWS: Add entry for the new source-checkout command) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/96017:15
gitlab-br-botrichardmaw-codethink opened issue #773 (The command-line should provide access to artifacts) on buildstream https://gitlab.com/BuildStream/buildstream/issues/77317:21
toscalixfosdem lightning talks CfP deadline is this coming Saturday https://fosdem.org/2019/news/2018-08-10-call-for-participation/17:30
*** phildawson82 has joined #buildstream17:33
*** abderrah1 has joined #buildstream17:34
*** abderrahim has quit IRC17:36
adds68jonathanmaw, seems anything inside the configure method will print, it's quite strange though why other parts of the code are suppressed17:47
jonathanmawadds68: I think the specific reason is that configure is called in the main process, and during assemble, it's being called in various subprocesses17:48
jonathanmawand the only reliable way to get the messages out in a sensible order is to pass them back to the parent process to print17:48
jonathanmawtbh I think this confusion would end if we could call the likes of self.info in configure17:48
adds68jonathanmaw, o opened https://gitlab.com/BuildStream/buildstream/issues/772 to see if this could be documented anywhere to make it  bit clearer17:50
jonathanmawadds68: ok, I'll add my response there so it doesn't get lost to history.17:56
adds68jonathanmaw, awesome thanks :)17:56
adds68jonathanmaw, when you say pass them back to the parent process, could you elaborate on that a little bit also?17:56
jonathanmawadds68: ok, I'll do some code diving for context.17:59
*** lachlan has quit IRC18:00
adds68jonathanmaw, thanks!18:01
jonathanmawadds68: interestingly, self.info() should work in the main process, so if it's not working in configure(), then there's a bug there.18:11
*** jonathanmaw has quit IRC18:12
adds68jennis, yes self.info does seem to log as expected! :)18:19
adds68oops jonathanwaw***18:19
*** lachlan has joined #buildstream18:25
*** lachlan has quit IRC18:29
*** toscalix has quit IRC18:29
*** raoul has quit IRC18:56
*** lachlan has joined #buildstream19:03
*** lachlan has quit IRC19:42
*** rdale has quit IRC19:51
*** phildawson has quit IRC20:01
gitlab-br-botjuergbi merged MR !953 (juerg/context-default-values->master: _context.py: Drop duplicated default values for user configuration) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/95320:36
*** jonathanmaw has joined #buildstream20:54
*** jonathanmaw has quit IRC20:56
*** tristan has quit IRC22:33
*** dabukalam has quit IRC22:57
*** dabukalam has joined #buildstream22:58
*** dabukalam has left #buildstream22:58

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