IRC logs for #buildstream for Friday, 2018-04-13

jjardonhi, im trying to build the GNOME 3.28.1 release and Im getting this when trying to build:00:33
jjardon"[--:--:--][][] WARNING Ignoring redundant source references00:33
jjardonand then the build stops; any idea what is this about?00:33
jjardonusing latest buildstream00:33
jjardonfull log here: https://paste.gnome.org/pcssrif5d00:36
jjardonseems its beacuse the new ref-storage option00:45
jjardonconversion-to-tarballs changes the ref inline00:46
* jjardon uses an older commit00:48
*** tristan_ has joined #buildstream04:59
gitlab-br-botbuildstream: merge request (jjardon/ci_fedora_27->master: Run test in Fedora27, apart from Debian 8) #378 changed state ("merged"): https://gitlab.com/BuildStream/buildstream/merge_requests/37805:57
gitlab-br-botbuildstream: issue #356 ("Need to manually specify coverage dependency artifacts") changed state ("opened") https://gitlab.com/BuildStream/buildstream/issues/35606:00
tristan_Who is good a regexes ?06:04
*** tristan_ is now known as tristan06:05
tristanI want to add something like this to our .gitlab-ci.yml: https://docs.gitlab.com/ce/ci/yaml/#coverage06:05
tristane.g. https://docs.gitlab.com/ce/user/project/pipelines/settings.html#test-coverage-parsing06:05
tristanthat would take our already done combined coverage job and bring it into the gitlab UI, would be pretty cool06:06
tristanthe job looks like: https://gitlab.com/BuildStream/buildstream/-/jobs/6271673706:06
tristanand the important line with the overall result looks like "TOTAL   9193   1449    84.24%"06:06
* tristan thinks https://docs.gitlab.com/ce/ci/yaml/#coverage is wildly underdocumented06:07
tristans/good a regexes/good at regexes/06:07
* tristan personally despises having to write a regex, every, time06:08
juergbihm, yes, it doesn't really say how it uses the regex06:08
juergbibased on the 'Test coverage parsing' it essentially uses \1, afaict06:09
juergbitristan: maybe something like this? TOTAL +\d+ +\d+ \(\d+\.\d+%\)06:11
juergbimissing a +06:12
juergbiTOTAL +\d+ +\d+ +\(\d+\.\d+%\)06:12
juergbiah, no, match groups in ruby are (), not \(\)06:14
juergbibut then the example doesn't make sense06:14
tristanthe example doesnt make huge sense; is it ruby specific even ?06:16
* tristan has no clue06:16
tristanjuergbi, does the percent belong inside the group ?06:17
juergbican only guess06:17
* tristan is gonna try a test06:17
tristanwhich will take a long time haha06:17
juergbi    @project.update_attribute(:build_coverage_regex, /Coverage (\d+)%/)06:18
juergbithat's from the gitlab sources06:18
juergbibased on this I would try06:18
juergbi /TOTAL +\d+ +\d+ +(\d+\.\d+)%/06:19
* tristan tries06:20
tristanoops06:21
tristanhttps://gitlab.com/BuildStream/buildstream/pipelines/2043307406:22
tristanI'm sort of hoping that it will end up in analytics eventually :)06:23
tristannot just "displayed on the pipeline"06:23
tristanwould be cool to see a graph of how coverage has improved/regressed over time06:24
tristanjuergbi, in light of https://gitlab.com/BuildStream/buildstream/issues/294#note_68188866 ... do you think that https://gitlab.com/BuildStream/buildstream/blob/master/buildstream/_artifactcache/ostreecache.py#L105 should be turned into an assert ?06:27
tristanI think that is a bug06:27
tristannot a legitimate error06:27
juergbias it's used right now, I think so06:28
juergbiwith artifact expiry it's no longer so clear cut, though06:28
tristanI think it's still a bug with artifact expiry06:29
tristanthen again parallel builds on the same machine could cause it06:29
juergbiexactly06:30
tristanjuergbi, with this parallel builds + expiry thing, I'm thinking a good strategy would still be to call it a bug, and hope that it never happens06:30
tristani.e. once we reach that critical mass where it actually happens, it's justified to actually *fix* it06:31
tristanbefore that, I'm weary of adding lock files06:31
juergbiyes, as long as the quota is large enough for the working set, it shouldn't happen06:31
tristanwell, more than that; the expiry should never expire things from the working set; that is a different bug06:32
tristanit should error out if the working set does not fit on disk, rather than expiring06:32
tristan(or does not fit in the quota, rather)06:33
juergbiI meant working set also across parallel sessions06:33
tristanAh right06:33
tristanexactly, so; I know it's not *perfect*, but I feel that making it that robust is going too far at this point, so long as we can report decent errors06:34
tristanI guess this would still be a bug, though, hmmm that raises a good question06:35
tristanat this point; it *happened*06:35
tristanwhich indicates a bug06:35
tristanif it happens because of parallel builds, it might be a worthwhile case to report a human readable error; but it would be nicer if we could really distinguish that between an actual bug06:36
tristan*between that and an actual bug06:36
tristanInterestingly, it seems that `last_successful` already ignores the possibility of a user running `rm -rf ~/.cache/buildstream/artifacts`06:38
juergbiah, right, that should be handled gracefully06:39
juergbiby considering all files of dependencies changed06:39
juergbiforcing a full rebuild06:39
tristanright, we thought about it for incremental builds06:40
tristanI pointed that out to tlater ... so I suppose the case of `rm -rf` would be handled at the same time06:40
tristanAnything to notice about https://gitlab.com/BuildStream/buildstream/commit/69b0b87508455330f7d74de2864439507664fae4 ?06:41
tristanI dont see any change in the pipeline06:41
tristanhttps://gitlab.com/BuildStream/buildstream/settings/ci_cd shows an example for pytest-cov06:42
tristan\d+\%\s*$06:43
tristanI think it's wrong though06:45
tristanit'll risk capturing too much06:46
juergbithese examples are also inconsistent, sometimes % in the matching group, sometimes not06:46
juergbihuge mess06:46
* tristan finds it weird that you can set it in the page as well as in the .gitlab-ci.yml06:46
juergbiyes, was wondering that too06:46
tristanI guess the webpage makes some kind of assumption like, "who would possibly run more than one job !"06:47
tristanbut, that's just silly06:47
tristanpipeline status badges06:49
tristanah, that part is for your README.md06:51
tristanok, enough time wasted on silly coverage thing, we can still click on the coverage job to get the report, which I often do06:59
*** dominic has joined #buildstream07:43
*** jonathanmaw has joined #buildstream07:52
*** valentind has quit IRC08:05
*** toscalix has joined #buildstream08:11
*** dominic has quit IRC08:12
tristantlater, for that last comment, I'm going to fix the reference docs for both Element and BuildElement, I'll see if I can get that done in an hour or so08:13
tristantlater, other than the comments I already made... I think it's looking good for a merge08:14
tristanjuergbi, namewise btw, one thing that is regrettable is that the base `BuildElement` defines `configure-commands`; which is quite autotools specific jargon; and we have `Plugin.configure()` which does something else08:15
tristanwe're adding `Element.prepare()` to split up the preparation stage from the build stage (where `configure-commands` now land), and Workspace.prepared local state08:16
tristanIt would be desirable to deprecate `configure-commands` for `prepare-commands` I think for consistent jargon08:16
juergbiyes, it's not ideal08:16
tristanbut, it doesnt seem possible :-S08:16
tristanI wonder if we could alias two variable names08:17
juergbiit's also a trade-off with having duplicated names, which could be confusing08:17
*** slaf- has joined #buildstream08:17
juergbinot sure whether it would really be better in the end08:17
*** slaf- has joined #buildstream08:17
tristanWell, it would certainly be better had we started out with `prepare-commands`08:17
tristanbut a change would not be all that great, indeed08:18
tristanjuergbi, I'm going to enhance the reference docs such that BuildElement explains what it's doing in the various Element.foo() methods08:18
juergbisounds good08:18
tristanwhich should help at least express that configure-commands are run as a part of the prepare stage08:18
*** slaf has quit IRC08:19
*** slaf- is now known as slaf08:19
juergbitristan: btw: separate prepare() might prevent future command batching between prepare and actual build08:19
tristanand I think another heading at the beginning of `Element()` should be good08:19
tristanjuergbi, indeed08:19
juergbinot per se an issue but ideally we can support batching for this08:19
tristanjuergbi, well; remote execution is "supposed to be efficient" as you said08:19
juergbithere will always be an overhead08:20
juergbivery difficult to estimate it right now08:20
juergbiwill depend on how the workers will be connected to the CAS08:20
tristanmaybe we wont have to do the grouping in the end... launching a bwrap sandbox process locally should be a negligible overhead compared to what is being run; but yes, an overhead nonetheless08:20
juergbithat would be ideal, yes08:21
tristanstill, if we get incremental builds (workspace or otherwise), the benefit of separating it should outweigh the small invocation overheads08:21
tlaterta tristan, I'll clean out the other problems asap08:21
*** noisecell has joined #buildstream08:42
gitlab-br-botbuildstream: merge request (jmac/artifact-receive-profiling->master: Add artifact cache receive profiling domain and instructions.) #383 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/38308:53
*** dominic has joined #buildstream08:55
*** aday has joined #buildstream08:59
gitlab-br-botbuildstream: merge request (issue-21_Caching_build_trees->master: WIP: Issue 21 caching build trees) #372 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/37209:25
*** aday has quit IRC09:34
*** tiago has quit IRC09:37
*** tiago has joined #buildstream09:37
*** aday has joined #buildstream09:43
gitlab-br-botbuildstream: merge request (element-prepare->master: Prevent configure commands from running more than once in incremental workspace builds) #386 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/38609:53
tristantlater, almost finished an overhaul of plugin author facing documentation...10:06
tristanactually BuildElement was tip of iceburg, we need something consistent; so I've got Plugin/Element/Source all doing the same thing at the top, and just need to fixup BuildElement10:07
tristanthat should pave the way for your new prepare() method to have a good place to live in docs10:07
tlaterty :)10:10
tlaterHm, is it worth it to try and reset this: https://gitlab.com/BuildStream/buildstream/merge_requests/386#note_68233724 to Element.prepare(self, sandbox)?10:14
tlaterI feel that's almost more confusing10:14
tlaterAlthough if we ever do change the default...10:14
gitlab-br-botbuildstream: merge request (element-prepare->master: Prevent configure commands from running more than once in incremental workspace builds) #386 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/38610:18
gitlab-br-botbuildstream: merge request (element-prepare->master: Prevent configure commands from running more than once in incremental workspace builds) #386 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/38610:18
jjardonHi, seems generation of new docker testsuite images are failing; can I have a quick review of https://gitlab.com/BuildStream/buildstream-docker-images/merge_requests/34 , please?10:26
gitlab-br-botbuildstream: merge request (tristan/plugin-authoring-docs->master: enhanced plugin authoring docs) #387 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/38710:37
gitlab-br-botbuildstream: merge request (135-expire-artifacts-in-local-cache->master: WIP: Resolve "Expire artifacts in local cache") #347 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/34710:39
gitlab-br-botbuildstream: merge request (Move-_list_dir_contents-to-runcli->master: WIP: Move _list_dir_contents  to runcli) #388 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/38810:40
jmacI'm seeing quite a lot of errors from gitlab ci but I don't think they're docker-related10:42
jjardonjmac: https://gitlab.com/BuildStream/buildstream-docker-images/-/jobs/6279536310:45
jjardonjmac: or you meant in buildstream pipelines?10:46
gitlab-br-botbuildstream: merge request (issue-21_Caching_build_trees->master: WIP: Issue 21 caching build trees) #372 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/37210:46
jmacjjardon: In the buildstream pipelines10:47
jmacYeah, completely different thing10:47
jjardonjmac: oh, sorry then; do you have a link so I can take a look?10:48
tlaterjmac: Do you mean the caching issues? I see those all the time as well.10:48
gitlab-br-botbuildstream: merge request (issue-21_Caching_build_trees->master: WIP: Issue 21 caching build trees) #372 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/37210:48
jmactlater: They seemed particularly common this morning10:49
tlaterThis can be entirely down to gitlab infrastructure, though it's been much more common since we switched to the new runner... Probably not a bad idea for jjardon to have a look :)10:50
jjardontlater: did you switch to a new runner?10:51
gitlab-br-botbuildstream: merge request (tristan/plugin-authoring-docs->master: enhanced plugin authoring docs) #387 changed state ("merged"): https://gitlab.com/BuildStream/buildstream/merge_requests/38710:51
tlaterThis happened a while ago, I think you and ssssam[m] were involved in the discussions around that, but it's old memory at this point10:51
tristanjjardon, so I got into a fist fight with gitlab early this afternoon, and even nobody in #gitlab is able to help10:51
tristanjjardon, maybe you have some idea of how we can enable this cool feature that it seems they didnt care about documenting or explaining very well at all10:52
tristanjjardon, check this out: https://docs.gitlab.com/ce/ci/yaml/#coverage10:52
tristanand https://gitlab.com/help/user/project/pipelines/settings#test-coverage-parsing10:52
tristantlater, rebase and have your docs story straight :)10:53
tlater\o/10:53
jjardontlater: jmac  please give me a log and I will try to take a look10:53
gitlab-br-botbuildstream: merge request (issue-21_Caching_build_trees->master: WIP: Issue 21 caching build trees) #372 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/37210:53
jjardontristan: I will check10:53
jmacI'm not asking you to look at it10:53
tristantlater, I'll be leaving momentarily... but assuming you've solved those discussions and amended related docs, I'll be ok to merge from handphone on the go10:54
tlaterjjardon: I don't think it's anything important, tbh, but you can probably pick out any pipeline. This one, for example: https://gitlab.com/BuildStream/buildstream/-/jobs/6279693110:55
tlaterIt will have failures to upload a cache at the end, and a failure to download one at the start10:55
jjardontlater: cheers, maybe the cache is full, I will check10:56
tristantlater, before I go, a couple of notes...11:03
tristantlater, element.py now has a listing of the abstract methods to implement, yours goes under the "build phase", which we now refer to...11:03
tristanfor instance Element.get_public_data() most only be called in the "build phase", instead of the "exhaustive list of linked methods we'll absolutely forget to maintain"11:04
tristantlater, it currently documents them as "must be implemented", you probably want to change that to follow the pattern I layed out in source.py11:04
tristani.e. "Unless explicitly mentioned, these methods are mandatory to implement."11:05
tristansee Source.init_workspace() in that list... **Optional**: bla bla bla about not implementing it11:05
tristanthe wording of everything in the docs is probably not perfect as of yet; that can be improved over time11:06
tristanbut getting consistency in *how* we document things is important to keep inline as we move forward11:06
* tlater will stick to that, tyvm11:07
tlaterJust started reading into your changes to see exactly what was wrong...11:07
tristanthere was even some misinformation :-S11:07
* tlater pays far too little heed to documentation, should make sure I don't ignore it11:08
tristanwe were encouraging people to call Element.get_public_data() in the docs for Plugin.preflight(), originally thinking that we could use that to trigger errors at preflight time11:08
tristanand then later telling people you can only call it at assemble time11:08
tristantlater, I'm also trying to go with a consistent linking pattern to methods11:09
tristani.e. :func:`Source.stage() <buildstream.source.Source.stage>`11:09
tristanie better than :func:`~buildstream.source.Source.stage`11:09
tristanwhich will link to the right thing, but render as an annoyingly ambiguous "stage()" in the HTML11:10
tristanI wasnt able to go through every single file and fix it, though11:10
tlaterAh, that's neat, didn't know we could do that11:11
tristanideally when jennis is finished with the docs initiative, all of the guidelines are at least layed out11:11
tristanevery link can be `Text <link target>`, instead of just `link target`11:11
tristanwe use that a lot11:11
tristanfor some things it's less important, like say :class:`.SourceError`11:12
tristanin that case it will render exactly the same if we specified :class:`SourceError <buildstream.source.SourceError>`11:12
tristanbut when linking methods of classes, it sucks to be ambiguous, you have to actually click the link to know what it's talking about11:13
gitlab-br-botbuildstream: merge request (element-prepare->master: Prevent configure commands from running more than once in incremental workspace builds) #386 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/38611:14
*** tristan has quit IRC11:17
gitlab-br-botbuildstream: merge request (issue-21_Caching_build_trees->master: WIP: Issue 21 caching build trees) #372 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/37211:34
*** tristan has joined #buildstream11:41
gitlab-br-botbuildstream: merge request (issue-21_Caching_build_trees->master: WIP: Issue 21 caching build trees) #372 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/37211:44
gitlab-br-botbuildstream: merge request (element-prepare->master: Prevent configure commands from running more than once in incremental workspace builds) #386 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/38611:51
gitlab-br-botbuildstream: merge request (element-prepare->master: Prevent configure commands from running more than once in incremental workspace builds) #386 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/38611:54
gitlab-br-botbuildstream: merge request (issue-21_Caching_build_trees->master: WIP: Issue 21 caching build trees) #372 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/37211:56
gitlab-br-botbuildstream: merge request (element-prepare->master: Prevent configure commands from running more than once in incremental workspace builds) #386 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/38612:03
tlatertristan: That should conclude the changes, unless you want me to change some wording/squash the documentation commits12:03
*** aday has quit IRC12:05
*** aday has joined #buildstream12:07
*** toscalix has quit IRC12:12
gitlab-br-botbuildstream: merge request (135-expire-artifacts-in-local-cache->master: WIP: Resolve "Expire artifacts in local cache") #347 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/34712:52
Nexusi'm trying to come up with a way of dealing with cached build trees being empty but still being staged into the workspace. An option i thought of was to check if the directory is empty and then send a warning to the user explaining that it is empty and then staging the sources as normal12:53
*** valentind has joined #buildstream13:24
*** jsgrant__ has quit IRC13:37
jmacDo the image-tools or artifact-cache docker images have any automated testing set up?14:32
*** toscalix has joined #buildstream14:33
gitlab-br-botbuildstream: merge request (issue-21_Caching_build_trees->master: WIP: Issue 21 caching build trees) #372 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/37214:40
gitlab-br-botbuildstream: issue #357 ("Cannot build certain modules after deleting ~/.cache/buildstream") changed state ("opened") https://gitlab.com/BuildStream/buildstream/issues/35714:52
tlaterjmac: Currently they don't14:54
tlaterI don't think any of our docker images do, frankly14:54
tlaterThe testsuite ones are obviously tested in buildstream CI, but not per-se if for whatever reason we don't run buildstream tests.14:55
jmacYep. Thought so.14:57
jmacI'm teaching myself docker at the moment so maybe I can add some.14:57
tlaterjmac: There's also the fedora image, it would be nice to have something that asserts bst-here works properly; we have quite a lot of few and no tests for it.15:03
tlaters/few/users/15:03
*** noisecell has quit IRC15:28
gitlab-br-botbuildstream: issue #358 ("Documentation: Element's dependencies order") changed state ("opened") https://gitlab.com/BuildStream/buildstream/issues/35815:47
gitlab-br-botbuildstream: merge request (richardmaw/key-summary->master: WIP: List resolved element keys in job summary) #389 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/38916:22
gitlab-br-botbuildstream: merge request (issue-21_Caching_build_trees->master: WIP: Issue 21 caching build trees) #372 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/37216:37
*** jonathanmaw has quit IRC16:37
*** dominic has quit IRC17:01
gitlab-br-botbuildstream: merge request (jjardon/order_elements->master: source/format.rst: Order of components in "elements:" doesn't matter) #390 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/39017:55
gitlab-br-botbuildstream: merge request (jjardon/order_elements->master: source/format.rst: Order of components in "elements:" doesn't matter) #390 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/39017:57
*** toscalix has quit IRC18:44
*** aday has quit IRC19:01
*** aday has joined #buildstream19:01
*** aday has quit IRC19:05
*** aday has joined #buildstream19:06
valentindIt would be nice to import  _downloadablefilesource.py in custom plugins. But it does not seem easy to import because of the plugin system. Anybody has any trick to import it?19:16
*** aday has quit IRC19:21
*** aday has joined #buildstream19:22
*** aday has quit IRC19:22
*** jsgrant has quit IRC19:23
*** jsgrant has joined #buildstream19:24
*** jsgrant has quit IRC19:28
*** jsgrant has joined #buildstream19:28
*** Prince781 has joined #buildstream21:37
*** tristan has quit IRC22:22

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