| *** nimish has joined #buildstream | 00:54 | |
| *** nimish has quit IRC | 01:05 | |
| *** swick has quit IRC | 03:04 | |
| *** tristan has joined #buildstream | 03:46 | |
| *** phil has quit IRC | 03:47 | |
| *** tpollard has quit IRC | 04:07 | |
| *** tristan has quit IRC | 04:43 | |
| *** tristan has joined #buildstream | 04:49 | |
| *** tristan has quit IRC | 05:02 | |
| *** tristan has joined #buildstream | 05:30 | |
| *** mohan43u has joined #buildstream | 05:50 | |
| *** toscalix has joined #buildstream | 06:31 | |
| *** toscalix has quit IRC | 06:44 | |
| *** toscalix has joined #buildstream | 06:45 | |
| *** tpollard has joined #buildstream | 08:08 | |
| *** toscalix has quit IRC | 08:22 | |
| *** ChanServ sets mode: +o tristan | 08:45 | |
| tristan | juergbi, Think it would be reasonable to have BuildStream make an attempt to configure the terminal settings in early startup ? | 08:45 |
|---|---|---|
| tristan | juergbi, i.e. see latest comment: https://gitlab.com/BuildStream/buildstream/issues/929#note_148137339 | 08:46 |
| juergbi | tristan: could make sense. maybe check whether ncurses apps typically do this and do the same? | 08:51 |
| tristan | Yeah, I think it's either do that or error out early | 08:52 |
| *** ikerperez has joined #buildstream | 09:06 | |
| *** jennis has joined #buildstream | 09:11 | |
| gitlab-br-bot | jennis opened MR !1209 (jennis/remove_node_chain_stuff->master: Rip out ChainMap(), node_chain_copy(), node_list_copy()) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1209 | 09:26 |
| *** toscalix has joined #buildstream | 09:31 | |
| *** toscalix has quit IRC | 09:33 | |
| gitlab-br-bot | danielsilverstone-ct approved MR !1209 (jennis/remove_node_chain_stuff->master: Rip out ChainMap(), node_chain_copy(), node_list_copy()) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1209 | 09:34 |
| *** toscalix has joined #buildstream | 09:34 | |
| gitlab-br-bot | BenjaminSchubert opened (was WIP) MR !1070 (bschubert/pipeline->master: Refactor _update_state() to be called only when needed) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1070 | 09:39 |
| benschubert | tristan: https://gitlab.com/BuildStream/buildstream/merge_requests/1070/ I've updated the PR to it would also take runtime deps of build deps in account. Could you have a last look ? I also added my latest benchmarking information. | 09:40 |
| *** phildawson has joined #buildstream | 09:42 | |
| tristan | benschubert, of course, I'm waiting to backport that to 1.2 haha | 09:42 |
| tristan | benschubert, I think you already squashed it as per jonathanmaw's comment (so that we don't have history of approaches which didnt land) | 09:44 |
| tristan | is that right ? | 09:45 |
| benschubert | tristan: I didn't yet no | 09:45 |
| benschubert | was planning to do that after the review | 09:45 |
| gitlab-br-bot | phildawson opened issue #949 (`integration` pytest mark we use in our test suite is a misnomer) on buildstream https://gitlab.com/BuildStream/buildstream/issues/949 | 09:52 |
| benschubert | tristan: should I merge all commits together or would you keep some separate? | 09:52 |
| tristan | benschubert, I like commits to be separate and granular, so long as: A) We keep the tests passing for every commit... B.) We don't include stuff that's not going in | 09:54 |
| benschubert | tristan: then let's keep it like that and just rewrite the one commit message you noticed? | 09:54 |
| tristan | I think so, your branch doesnt appear to contain fixes of previous commits or pivots in approaches, that's the kind of thing we try to avoid | 09:55 |
| benschubert | agreed | 09:55 |
| benschubert | tristan: one question to make sure about the recursive stuff. dependencies() with scope=Scope.BUILD, recurse=False, will give only direct build dependencies AND their runtime dependencies? IS that correct? or only the direct build dependencies? | 10:00 |
| tristan | crap did I discard the comment which took me a while to write ? | 10:01 |
| benschubert | Oh the previosu one, no it's still there | 10:01 |
| benschubert | https://gitlab.com/BuildStream/buildstream/merge_requests/1070#note_148176406 you mean? | 10:01 |
| benschubert | or which one? | 10:02 |
| tristan | Ah no, I didnt, https://gitlab.com/BuildStream/buildstream/merge_requests/1070/diffs#note_148176406 indeed showed up :) | 10:03 |
| tristan | gitlab UI slow | 10:03 |
| tristan | benschubert, to answer your question, recursive=False will only yield direct dependencies, and furthermore, it will never yield the element itself | 10:06 |
| tristan | it is a rather special case for examining only one level | 10:06 |
| benschubert | ok! I will fix that, great, it will make performance even better :D | 10:07 |
| tristan | benschubert, regarding https://gitlab.com/BuildStream/buildstream/merge_requests/1070/diffs#note_148173233... | 10:07 |
| tristan | I don't think making recurse=False in that call fixes things, or I doubt it | 10:07 |
| tristan | unconditionally building a list of reverse dependencies would, as I understand you want to be selective and reduce that list | 10:08 |
| tristan | if we do, then that list should probably be a set() ? | 10:08 |
| benschubert | Yeah that's true, I don't need recurse :) | 10:10 |
| benschubert | and the set is not needed because the .dependencies function only returns every element once | 10:11 |
| tristan | Uh oh | 10:12 |
| tristan | benschubert, we have a bug I believe | 10:12 |
| tristan | benschubert, you are doing this from _new_from_meta() | 10:13 |
| benschubert | yes | 10:13 |
| tristan | benschubert, which has an early return for a good reason, elements will be hit multiple times from different dependency paths | 10:13 |
| tristan | I believe that you are only capturing the reverse dependencies from one of the paths, even though the element can be reached by many | 10:14 |
| tristan | I think that it will be much simpler to unconditionally collect all reverse dependencies, and avoid calling Element.dependencies() at all for this | 10:15 |
| benschubert | tristan: I am registering when I complete an element on all its dependencies | 10:15 |
| benschubert | tristan: "I think that it will be much simpler to unconditionally collect all reverse dependencies, and avoid calling Element.dependencies() at all for this" => we will be adding too much, the real thing we need is the build dependencies | 10:15 |
| benschubert | so I'm calling once the element is complete | 10:16 |
| benschubert | I don't see what the bug would be? | 10:16 |
| tristan | benschubert, consider the case where something *else* depends on that element, and it is already constructed | 10:17 |
| tristan | i.e. we iterate from top down, from a target into a bunch of orthogonal dependencies which all depend on the same base | 10:17 |
| tristan | The first iteration constructs the base, and updates this reverse dependency list | 10:17 |
| benschubert | the link is only created as we move up | 10:17 |
| benschubert | No | 10:18 |
| benschubert | yes | 10:18 |
| tristan | The next iterations (in parent stack frames), construct orthogonal reverse dependencies and then early return in the common, already constructed base dependency | 10:19 |
| benschubert | yes | 10:19 |
| tristan | So those orthogonal elements which depend on the base, will not get added to the reverse dependency list, as this list is only constructed for the first time _new_from_meta() is called | 10:20 |
| tristan | benschubert, understand ? | 10:20 |
| benschubert | No, let me try: | 10:20 |
| tristan | "if meta in cls.__instantiated_elements:" | 10:20 |
| benschubert | 1) You construct your base. Then go through all its dependencies, and register the base as one of their rdep. There is none. | 10:21 |
| benschubert | 2) You construct all the elements (orthogonal) that have a dependency to your base, and for each element you register them in the base as a rdep | 10:21 |
| tristan | benschubert, except you dont do (2) | 10:22 |
| tristan | not in your branch | 10:22 |
| benschubert | line 981 and 982 exactly do that | 10:22 |
| benschubert | note that they are working on my dependencies, not on my element | 10:22 |
| benschubert | when an element is built, all its dependencies are already built | 10:23 |
| benschubert | so I can safely register my element on them | 10:23 |
| benschubert | as a rdep | 10:23 |
| tristan | Hmmm | 10:23 |
| benschubert | (brb) | 10:23 |
| * tristan will not however, have a dinner appointment | 10:24 | |
| tristan | benschubert, I'll take a closer look, but I think you are right | 10:24 |
| *** jonathanmaw has joined #buildstream | 10:24 | |
| benschubert | tristan: I'll address your comments already | 10:25 |
| benschubert | Would be good if we can merge this soon | 10:25 |
| tristan | yeah, it fixes a bad bug (I'm not really concerned much about the performance gains it might bring, and I expect a different round of refactoring and splitting up the activities of _update_state() to be much more effective for optimizing purposes) | 10:27 |
| tristan | it's also nice because it makes more sense to have reverse dependencies notified, the algorithm is more reliable | 10:28 |
| *** lachlan_ has joined #buildstream | 10:38 | |
| *** swick has joined #buildstream | 10:46 | |
| benschubert | tristan: I updated all your comments! | 10:51 |
| benschubert | So waiting for the last greenlight before sending it to marge | 10:52 |
| benschubert | if anybody else want to have a look, now is the time | 10:52 |
| * Kinnison doesn't think he has anything to add | 10:52 | |
| *** jonathanmaw_ has joined #buildstream | 10:53 | |
| *** jonathanmaw has quit IRC | 10:53 | |
| *** cs-shadow has quit IRC | 10:53 | |
| *** cs-shadow has joined #buildstream | 10:53 | |
| *** pro[m] has quit IRC | 10:53 | |
| *** cgmcintyre[m] has quit IRC | 10:53 | |
| *** mattiasb has quit IRC | 10:53 | |
| *** Demos[m] has quit IRC | 10:53 | |
| *** skullone[m] has quit IRC | 10:53 | |
| *** dbuch has quit IRC | 10:53 | |
| *** jjardon[m] has quit IRC | 10:53 | |
| benschubert | jonathanmaw_: ? | 10:53 |
| * jonathanmaw_ reads irclogs | 10:54 | |
| benschubert | jonathanmaw_: ah I meant do you want a last look at !1070? | 10:59 |
| gitlab-br-bot | MR !1070: Refactor _update_state() to be called only when needed https://gitlab.com/BuildStream/buildstream/merge_requests/1070 | 10:59 |
| * jonathanmaw_ will have a look | 10:59 | |
| *** jonathanmaw_ is now known as jonathanmaw | 10:59 | |
| *** pro[m] has joined #buildstream | 11:01 | |
| *** cgmcintyre[m] has joined #buildstream | 11:01 | |
| *** mattiasb has joined #buildstream | 11:02 | |
| *** Demos[m] has joined #buildstream | 11:12 | |
| *** lachlan_ has quit IRC | 11:14 | |
| *** lachlan_ has joined #buildstream | 11:31 | |
| *** lachlan_ has quit IRC | 11:36 | |
| *** skullone[m] has joined #buildstream | 11:48 | |
| *** lachlan has joined #buildstream | 11:51 | |
| gitlab-br-bot | jennis opened MR !1211 (jennis/move_node_get_project_path->master: Cleanup: Move _yaml.node_get_project_path to Project._get_path_from_node()) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1211 | 12:00 |
| *** raoul_ has joined #buildstream | 12:01 | |
| jennis | juergbi, regarding our recent discussion, I'd appreciate if you could spare a couple of minutes for ^ (!1211), no worries if not | 12:02 |
| *** dbuch has joined #buildstream | 12:20 | |
| *** jjardon[m] has joined #buildstream | 12:22 | |
| *** raoul_ has quit IRC | 12:28 | |
| *** jonathanmaw has quit IRC | 12:43 | |
| *** jonathanmaw has joined #buildstream | 12:45 | |
| coldtom | is anybody from bst-external around to take a look at https://gitlab.com/BuildStream/bst-external/merge_requests/70 ? | 12:50 |
| *** raoul_ has joined #buildstream | 13:23 | |
| benschubert | tristan: I don't see why we need "Scope.ALL" non recursive in !1070, I answered your comment | 13:24 |
| *** nimish has joined #buildstream | 13:37 | |
| *** gokcennurlu has joined #buildstream | 13:53 | |
| *** toscalix has quit IRC | 14:04 | |
| *** mablanch_ is now known as mablanch | 14:16 | |
| *** toscalix has joined #buildstream | 14:36 | |
| gitlab-br-bot | gokcennurlu opened issue #950 (Things to think about when redesigning YAML) on buildstream https://gitlab.com/BuildStream/buildstream/issues/950 | 15:47 |
| gitlab-br-bot | marge-bot123 merged MR !1022 (chandan/base-git-mirror->master: Expose _GitMirror as part of plugin author facing API) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1022 | 15:48 |
| *** nimish has quit IRC | 16:41 | |
| *** nimish has joined #buildstream | 16:45 | |
| *** phildawson has quit IRC | 16:59 | |
| *** toscalix has quit IRC | 17:00 | |
| *** nimish has quit IRC | 17:03 | |
| *** jonathanmaw has quit IRC | 17:05 | |
| *** nimish has joined #buildstream | 17:14 | |
| *** phildawson has joined #buildstream | 17:40 | |
| *** tpollard has quit IRC | 18:00 | |
| *** lachlan has quit IRC | 18:24 | |
| *** phildawson has quit IRC | 18:29 | |
| *** nimish has quit IRC | 18:54 | |
| tristan | benschubert, just getting home in a comfortably numb state... I dont see how you handle the case of indirect runtime only dependencies of build-only dependencies in reverse dependency invalidations while considereing only build-only dependencies | 19:16 |
| tristan | but I will be happy to provide a test case to prove it tomorrow :) | 19:16 |
| benschubert | tristan: ok! I'll be awaiting the test case :D | 19:18 |
| tristan | benschubert, it will be along the lines of the existing test case which you have cherry picked from #919 already | 19:19 |
| gitlab-br-bot | Issue #919: 'bst build <elem>' does not assemble all required elements in some circumstances https://gitlab.com/BuildStream/buildstream/issues/919 | 19:19 |
| benschubert | tristan: great, let's add more testcases! | 19:20 |
| tristan | except it will additionally contain a case where there is a runtime-only dependency of the build-only dependency | 19:20 |
| tristan | lets drink more soju ! | 20:17 |
| *** tristan has quit IRC | 21:34 | |
| *** cs-shadow has quit IRC | 22:22 | |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!