*** tristan has quit IRC | 05:48 | |
*** tristan has joined #buildstream | 07:36 | |
*** ChanServ sets mode: +o tristan | 07:36 | |
*** tristan has quit IRC | 07:40 | |
*** tristan has joined #buildstream | 08:37 | |
*** ChanServ sets mode: +o tristan | 08:37 | |
juergbi | tristan: regarding #1340 I've recently been thinking about changing the algorithm for staging / overlap handling | 09:00 |
---|---|---|
juergbi | an idea I had was to first create CasBasedDirectory objects for each dependency (take care of split domains as part of this step, where necessary) | 09:01 |
juergbi | and then merge these directories in a single pass by recursively iterating over all these sorted dependency directories in parallel | 09:02 |
juergbi | this would directly provide conflict information as part of the merge without having to generate/store/check file lists for each dependency | 09:03 |
juergbi | and e.g. directories (or even whole directory trees) that are exclusive to a single dependency wouldn't even need to be iterated | 09:04 |
juergbi | I'd expect that to generally improve staging performance and also allow simplification of the Directory API (no filelistresult, no conflict resolution), at the cost of having to write this, hopefully not too complicated, merge algorithm | 09:05 |
juergbi | don't know whether the performance impact would be significant, though (will obviously depend on the directory structure) | 09:06 |
tristan | juergbi, sounds like an optimization, I do think that the current implementation requiring a search() is bogus, that doesn't detract from search() in itself being completely broken without support for full element paths though | 09:13 |
tristan | This is ridiculously easy to fix, it's hard to believe it's stood for so long | 09:14 |
juergbi | agreed | 09:14 |
tristan | Anyway, I'm adding tests using script element via the integration route, I'll try to add overlap tests for this case too, it wont hurt :) | 09:15 |
tristan | juergbi, The one thing I'm not entirely sure of, is whether we should expose Element.full_name | 09:15 |
tristan | (currently _get_full_name()) | 09:15 |
tristan | Seems like we should allow that in public API, but as there is no immediate need for it, I'll exclude that from the MR | 09:16 |
juergbi | tristan: search() is relative to the Element's project, right? | 09:19 |
juergbi | but get_full_name() wouldn't be, hm | 09:20 |
tristan | juergbi, After the following MRs, it won't make a difference afaict | 09:25 |
tristan | Currently with junction name coalescing things are weird indeed | 09:25 |
tristan | oh wait no | 09:26 |
tristan | it would still differ | 09:26 |
tristan | Oh strange | 09:26 |
tristan | Oh no, I was right the first time | 09:27 |
tristan | Or not, jaysus, no it's not | 09:27 |
*** benschubert has joined #buildstream | 09:27 | |
tristan | ok, so this is indeed trickier than I thought | 09:27 |
tristan | A full path is not the same as a project relative name, and Element.search() needs a project relative name, not a full path | 09:28 |
tristan | Not a lot tricker than I thought, but I didn't notice this :) | 09:28 |
tristan | juergbi, thanks for pointing that out haha | 09:28 |
tristan | Grrr, anyone run integration tests locally lately ? | 09:32 |
tristan | "The FUSE stager child process unexpectedly died with exit code 1" | 09:32 |
tristan | Another marvelously descriptive error message | 09:32 |
tristan | juergbi, I'm using buildbox-x86_64-linux-0.0.8-1e3b6f8e.tar.xz, should I have something newer than that ? | 09:34 |
juergbi | tristan: fusermount3 (possibly fuse3 package) not installed? | 09:35 |
juergbi | yes, definitely need to forward the error message from buildbox-fuse | 09:35 |
tristan | Can we get an error message about requiring fuse3 ? | 09:36 |
juergbi | yes, we need to fix that in buildbox-casd | 09:37 |
tristan | That fixes things it seems | 09:37 |
juergbi | https://gitlab.com/BuildGrid/buildbox/buildbox-casd/-/issues/72 | 09:43 |
tristan | Under the new world order of removed junction coalescing, I guess it would be correct to say that: "An element relative name of a dependency is the dependency name, with the element's project's junction's name prefix stripped away" | 10:04 |
tristan | I.e. you cannot have a dependency on an element which does not share the same prefix | 10:04 |
tristan | So strip away self._get_project.junction._get_full_name() from the dependency._get_full_name(), plus one ":", and there you have the element relative name of any dependency | 10:06 |
tristan | Rather: self._get_project().junction._get_full_name() | 10:06 |
tristan | Unfortunately in this case, I need to delay the Element.search() fix to a later commit after nuking junction name coalescing | 10:17 |
tristan | I was otherwise hoping to include it in https://gitlab.com/BuildStream/buildstream/-/merge_requests/1956 | 10:18 |
juergbi | yes, this sounds correct to me | 10:27 |
* tristan is adding yet another test but it seems to be correct | 10:30 | |
juergbi | tristan: I've replied to the junction mail, so we can hopefully conclude this asap | 10:52 |
tristan | Great | 10:55 |
tristan | I've made some progress, now I have the full pathing which is rather thorough (did I miss any surfaces where we support element paths ?), and now the Element.search() thing which must come after !1901 | 10:55 |
tristan | Element.search() fix is at https://gitlab.com/BuildStream/buildstream/-/merge_requests/1957 now fwiw | 10:56 |
tristan | It doesn't yet fix overlaps, but I think overlaps need to be fixed differently now that I consider element relative searching | 10:56 |
tristan | maybe not, but I think we can just use the plugin_id for the overlaps problem | 10:56 |
tristan | (only the last two commits on !1957 are Element.search()) | 10:57 |
tristan | not in my inbox yet but I have it on the list | 10:59 |
tristan | juergbi, So maybe we should require that every duplicated project be listed, rather than relying on one unlisted project remaining ? | 11:00 |
tristan | While my mind has been a bit too tightly tied to the implementation, this appears to be a better user facing API | 11:00 |
tristan | duplicates: { projectname: [ "junction1.bst", "junction2.bst" ] }, the list of junctions should always cover *ALL* junctions to a duplicated project, leaving none out (after being merged with superprojects) | 11:02 |
tristan | So the first time a project is duplicated, the list must have 2, but a superproject might extend that with an additional single-item list for the same project | 11:03 |
tristan | juergbi, replied: one problem I can see with this plan I think | 11:12 |
*** tristan has quit IRC | 11:17 | |
*** hasebastian has joined #buildstream | 15:22 | |
*** hasebastian has quit IRC | 18:09 | |
*** benschubert has quit IRC | 19:41 |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!