| *** tristan has joined #buildstream | 00:26 | |
| *** mlalkaka has joined #buildstream | 00:33 | |
| *** mlalkaka has quit IRC | 00:35 | |
| *** tristan has quit IRC | 01:05 | |
| *** jared has joined #buildstream | 01:06 | |
| *** nimish has joined #buildstream | 02:17 | |
| *** nimish has quit IRC | 03:05 | |
| *** mohan43u has joined #buildstream | 05:15 | |
| *** tristan has joined #buildstream | 08:42 | |
| *** toscalix has joined #buildstream | 08:42 | |
| *** ChanServ sets mode: +o tristan | 09:14 | |
| tristan | juergbi, I'm having a hard time thinking of a patch which fixes #919 that is "less noisy" than !1177 (short of tearing _update_state() apart completely and spending weeks refactoring)... Any ideas ? | 09:14 |
|---|---|---|
| gitlab-br-bot | Issue #919: 'bst build <elem>' does not assemble all required elements in some circumstances https://gitlab.com/BuildStream/buildstream/issues/919 | 09:14 |
| gitlab-br-bot | MR !1177: WIP: tests/frontend/workspace.py: Test that all elements build with workspace in play https://gitlab.com/BuildStream/buildstream/merge_requests/1177 | 09:15 |
| juergbi | will take a look | 09:15 |
| tristan | At least in that patch I could avoid listing build dependencies twice, it wasn't meant as a final patch | 09:15 |
| * Kinnison is all for tearing _update_state() apart and refactoring | 09:15 | |
| *** benschubert has joined #buildstream | 09:15 | |
| tristan | Kinnison, Indeed... is that what the idea of an "artifact abstraction class" is for ? | 09:16 |
| Kinnison | It'll help with that a bit, yes. But its primary purpose is to move any code which deals with the "physical" structure of an artifact out of Element | 09:16 |
| tristan | But for right now, it's pretty bad; any time you workspace something behind a compose element (like when you're building a system image and tweaking things)... then bst build never completes in one go | 09:16 |
| tristan | Ah yeah that is a good thing | 09:17 |
| juergbi | I anyway want to get rid of this strange workspace key handling, as discussed at the Autumn gathering | 09:17 |
| tristan | I still don't remember how you intend to address that | 09:18 |
| tristan | Without causing needless/redundant workspaced element builds of course; I think the current handling is quite correct | 09:18 |
| juergbi | store buildtree in CAS, separate from actual workspace | 09:18 |
| tristan | Oh so the initial copy instead | 09:19 |
| juergbi | that is anyway required if we want to support remote builds of workspaces | 09:19 |
| juergbi | and with buildbox-fuse, it should be pretty cheap | 09:19 |
| juergbi | it might be more expensive without fuse, though | 09:19 |
| tristan | Ummm | 09:19 |
| juergbi | have to look into the details again once all prerequisites are there | 09:20 |
| tristan | I'm not sure that works because incremental builds right ? | 09:20 |
| juergbi | the idea is to do it a bit like git | 09:20 |
| tristan | I mean, you need the state of the build tree at the end of the build, to check whether something has changed | 09:20 |
| tristan | Oh no because of the copy again, sorry yeah of course | 09:20 |
| juergbi | create a diff between the source dir of the last build and the current version (should be reasonably cheap with CAS) | 09:21 |
| juergbi | and then apply the changes to the previous buildtree | 09:21 |
| juergbi | i.e., like a git checkout | 09:21 |
| tristan | right, yeah because of the copy, there is an opportunity to diff | 09:21 |
| tristan | (or check timestamps or something cheap) | 09:21 |
| juergbi | still have to decide on how to handle timestamps via buildbox-fuse/cas, though | 09:21 |
| juergbi | so there are some open questions, but that's the general direction I intend to go for | 09:22 |
| juergbi | anyway, not the next step | 09:22 |
| tristan | Right, but anyway there is a bug now that cannot realistically wait for a big refactor | 09:23 |
| juergbi | tristan: btw: I have a branch that replaces file lists (copy_files and co.) with a filter callback | 09:23 |
| juergbi | this provides some nice speedups for CasBasedDirectory import operations | 09:23 |
| juergbi | (as we can do it with much simpler recursive code instead of reparsing each path in the file list) | 09:23 |
| tristan | So for now, we should fix it, and making sure the regression tests are in place should make the refactors safer | 09:23 |
| juergbi | yes, let me take a look now | 09:24 |
| *** jonathanmaw has joined #buildstream | 09:26 | |
| juergbi | tristan: can you please elaborate a bit on your concern with your patch? | 09:26 |
| juergbi | is the extra loop/check expensive? | 09:27 |
| tristan | juergbi, My concern is that Kinnison will look at me with evil eyes | 09:27 |
| tristan | Heh | 09:27 |
| juergbi | ;) | 09:27 |
| tristan | Well, that's what I'm wondering yes, but basically looking at it, it will only happen when actually calculating a key | 09:27 |
| Kinnison | Hey! It's not my fault that my eyes are inherently evil | 09:27 |
| juergbi | I think it would only be expensive if there are many cases where update_state() of the dependency is called and it's still None afterwards | 09:28 |
| tristan | It will kind of double recurse, a bit more than needed | 09:28 |
| juergbi | don't know otoh whether that will be frequent | 09:28 |
| Kinnison | So long as the general non-workspaced case isn't significantly impacted, we'll be okay for now | 09:28 |
| juergbi | it may impact all cases :/ | 09:29 |
| Kinnison | I guess if it's needed short-term, we take the hit | 09:29 |
| Kinnison | benschubert: ^^ do you agree? | 09:29 |
| tristan | I think that ensuring correctness before optimization is something that should not be up for discussion | 09:30 |
| juergbi | we should probably at least do a quick performance comparison | 09:30 |
| tristan | I'm just wondering how to do it most gently for now :) | 09:30 |
| juergbi | yes, of course | 09:30 |
| Kinnison | tristan: Oh indeed if there's a glaring wrongness it needs fixing even if it impacts performance | 09:30 |
| Kinnison | tristan: I'm just checking that evil eyes will be appropriately averted for the proceedings | 09:31 |
| tristan | :) | 09:31 |
| juergbi | tristan: might an alternative be to have the scheduler notice when the cache key gets calculated of an element and in that case, do at least one more round, i.e., don't quit yet just because there was no ready change | 09:32 |
| tristan | What I don't like about that loop mostly, is that calling _update_state() does more than just "constructing a cache key if possible" | 09:32 |
| juergbi | yes, it might end up checking the artifact cache additional times | 09:32 |
| benschubert | what PR are you talking about? I don't have it in my history | 09:33 |
| juergbi | #919 / !1177 | 09:33 |
| gitlab-br-bot | Issue #919: 'bst build <elem>' does not assemble all required elements in some circumstances https://gitlab.com/BuildStream/buildstream/issues/919 | 09:33 |
| tristan | benschubert, !1177 | 09:33 |
| gitlab-br-bot | MR !1177: WIP: tests/frontend/workspace.py: Test that all elements build with workspace in play https://gitlab.com/BuildStream/buildstream/merge_requests/1177 | 09:33 |
| tristan | yeah | 09:33 |
| tristan | (oops) | 09:34 |
| tristan | juergbi, I'm not sure, it certainly sounds like something we should not want to keep | 09:35 |
| benschubert | tristan, juergbi: how can we not be sure about that? I mean, after loading, we compute the cache key of every element | 09:35 |
| tristan | juergbi, I.e. if the scheduler has to poke the beast a few times, that is kind of slippery slope territory where we are compensating elsewhere for something that should be the responsibility of the data model to just get right | 09:35 |
| juergbi | benschubert: workspace cache keys are special, they have to be recalculated after the build | 09:36 |
| tristan | benschubert, Not a workspaced element which has not been built yet, we can only know it's key after the build (right) | 09:36 |
| juergbi | tristan: I'm not saying it should be necessary long term. however, in a way it's a bug in the scheduler's 'making progress' check | 09:37 |
| tristan | I suspect there are also cases where the key changes, or we are not sure of the strong key, until a dependency has passed through the pull queue right ? | 09:37 |
| juergbi | as calculating cache keys is some sort of progress | 09:37 |
| juergbi | yes, for non-strict builds, we might have similar issues | 09:37 |
| benschubert | juergbi: which they areisn't https://gitlab.com/BuildStream/buildstream/blob/master/buildstream/element.py#L1548 enough? | 09:38 |
| benschubert | *isn't this | 09:38 |
| juergbi | benschubert: it doesn't update reverse dependencies, though | 09:38 |
| benschubert | but those will be updated by https://gitlab.com/BuildStream/buildstream/blob/master/buildstream/_scheduler/queues/buildqueue.py#L74 | 09:39 |
| juergbi | so unless all reverse dependencies are in topological order in the queues, it may take multiple loops | 09:39 |
| benschubert | they are in topological order in the queues | 09:40 |
| juergbi | based on the issue, this doesn't seem to be always the case | 09:40 |
| benschubert | or at least should be | 09:40 |
| benschubert | then that is another problem I would say :/ | 09:40 |
| benschubert | but that doesn't seem like the right fix to me | 09:40 |
| tristan | benschubert, that is not clear, whether they should be or not | 09:40 |
| tristan | or whether they should even be lists anymore | 09:41 |
| juergbi | build-only dependencies are inserted dynamically, as needed | 09:41 |
| tristan | benschubert, I.e. build-only dependencies are better off at the end of the queue, due to the uncertainty of ever needing to process them ? | 09:41 |
| benschubert | tristan: if they are not in topological order, then that means we are building an element and its dependencies at the same time. And that is not correct | 09:41 |
| tristan | benschubert, yeah with runtime dependency chains you always have the order you would expect | 09:41 |
| tristan | They *might* even be always in topological order, but be missing out on cache key resolution due to not being "required" at the time _update_state() is called on them ? | 09:43 |
| benschubert | what I don't get is, why do we need to update the state now, shouldn't it get updated when l9ooking for ready stuff, then update here a new time after the pass? | 09:43 |
| tristan | In the test cases, what happens without the patch is that the target is stuck in the fetch queue | 09:44 |
| benschubert | I mean, the dependencies will get updated (even later), and we won't be able to update ours at the first pass. A second pass will fix it though | 09:44 |
| tristan | Not sure a second pass will fix it honestly | 09:44 |
| benschubert | The second pass will run after the first element is resolved no? | 09:45 |
| tristan | While debugging, I was printing elements in queues, which of course changes the behavior (printing the element causes the key to be guessed) | 09:45 |
| tristan | Depending on what chains of build-of-build dependencies might be present in a session, I don't really suspect a second pass will be reliable | 09:45 |
| tristan | While I agree that N-th pass will certainly fix it | 09:46 |
| Kinnison | surely something whose deps have yet to pass through the queue won't be in the queue anyway? | 09:46 |
| tristan | (so debugging was interesting because we had a few elements and depending on how many times they were printed, more keys are resolved in the final summary) | 09:46 |
| tristan | Kinnison, build-only deps | 09:47 |
| tristan | I don't recall honestly | 09:47 |
| tristan | Kinnison, I.e. originally we started the session by determining what could be downloaded or not | 09:47 |
| tristan | This told us what build-only deps would have to build | 09:48 |
| tristan | Then we have our depth sort thing, which I suppose now assumes "build-only deps will never be needed" | 09:48 |
| tristan | And then we only ever get that far down the queue once something has said "hey I'm missing my build dep" | 09:48 |
| tristan | So I'm not sure right now if we place build-only deps interspersed, or always /after/ | 09:49 |
| * Kinnison is very confused. We start with some targets to build, and the graph of elements necessary to make that a reality. | 09:49 | |
| tristan | In any case, the order of the elements in the lists are not supposed to be important | 09:49 |
| Kinnison | We should then put the stuff into the queues which *can start* at that point | 09:49 |
| Kinnison | and any completed job should annotate the graph, allowing things further up to be added to queues where they *can start* only when they can | 09:49 |
| tristan | They are only supposed to optimize the thing so that we try to process deeper things first, which allow more parallelism | 09:49 |
| Kinnison | is that not how it works? | 09:49 |
| tristan | Adding some kind of significance to the order of elements in the queues seems to be a delicate affair | 09:50 |
| tristan | Kinnison, We put everything into the first queue and we are not selective about that | 09:51 |
| tristan | Kinnison, Then the queues interrogate the data model to find out what is ready | 09:52 |
| Kinnison | That seems like we're doing the wrong thing to me | 09:52 |
| Kinnison | unless the 'first queue' is a "decide what to do with X" queue | 09:52 |
| Kinnison | where a valid decision is "drop it out of the queues, it'll come back later when it can run somewhere" | 09:52 |
| tristan | Ok there is an exception, we place only things selected for tracking in the tracking queue | 09:53 |
| tristan | No no... every element goes through the queues in succession unless they are stuck in wait state | 09:53 |
| tristan | The only thing that does not get through all of the queues are build-of-build dependencies which did not require processing | 09:53 |
| tristan | afaics | 09:53 |
| tristan | dropping out of the queues isnt an option, each queue assumes that "when I'm done with this, let the next guy try to do something with this" | 09:54 |
| tristan | A queue doesnt know what queue is coming next nor should it | 09:54 |
| Kinnison | This sounds like we're over-working in order to be more generic | 09:54 |
| *** ikerperez has joined #buildstream | 09:55 | |
| Kinnison | However we're vastly far away from the original discussion about getting a fix in place | 09:55 |
| Kinnison | We can worry about improving performance when it's already right | 09:56 |
| tristan | sorry had a quick call... | 10:01 |
| tristan | Right so... I think the question is whether to fix this in the data model or try to patch it from the scheduler | 10:02 |
| *** tpollard has joined #buildstream | 10:02 | |
| tristan | I can say right away that I believe the responsibility to be in the hands of the data model to always report correct things | 10:02 |
| tristan | I rather hope this to always be true regardless of any refactor | 10:03 |
| *** raoul has joined #buildstream | 10:09 | |
| *** lachlan has joined #buildstream | 10:31 | |
| *** cs-shadow has joined #buildstream | 10:37 | |
| *** jonathanmaw has quit IRC | 10:46 | |
| *** jonathanmaw has joined #buildstream | 10:46 | |
| *** alatiera has joined #buildstream | 10:50 | |
| benschubert | tristan: !1070 does not have the bug, I tried merging your branch in mine, and remove your element.py patch and it works. I'm tidying up loose ends on this one. Would that be an acceptable patch for you instead? (I'm all for adding your test afterwards though, we should definitely not loose the test) | 11:07 |
| gitlab-br-bot | MR !1070: WIP: Refactor _update_state() to be called only when needed https://gitlab.com/BuildStream/buildstream/merge_requests/1070 | 11:07 |
| tristan | benschubert, Oh that is interesting news :) | 11:07 |
| * tristan takes a look | 11:08 | |
| tristan | Ah this is the reverse dependencies | 11:08 |
| tristan | benschubert, it was also an approach I had in mind but thought it was "refactor territory" rather than "bugfix territory" | 11:09 |
| benschubert | that's true | 11:09 |
| cs-shadow | tristan: hey! if you find time, please have a look at !998 (hopefully for the last time :) ) | 11:14 |
| gitlab-br-bot | MR !998: loader: Allow dependencies to use ":" to refer to junctioned elements https://gitlab.com/BuildStream/buildstream/merge_requests/998 | 11:14 |
| tristan | benschubert, I like !1070 :) | 11:17 |
| benschubert | great! that's a neater fix. Let's find out about the performance improvements and merge if it gets some then :D | 11:18 |
| juergbi | tpollard: fyi, I've pushed https://gitlab.com/BuildStream/buildstream/commits/juerg/virtual-artifact-directory which eliminates extract directories and probably affects your refactoring a bit | 11:19 |
| juergbi | it's still a bit WIP but tests should pass | 11:20 |
| juergbi | don't know whether it would make sense to already rebase on top of that | 11:20 |
| juergbi | I'm planning to finish this up and get it merged this week, and hopefully without causing further rebase pain, but can't guarantee that yet | 11:21 |
| tpollard | juergbi: thanks for the headsup, I've pushed a WIP for my work so far, it needs review so might need more reworking anyway :) | 11:26 |
| juergbi | tpollard: ok. shall I take a look at your branch already or is maybe Kinnison already reviewing that? | 11:27 |
| gitlab-br-bot | BenjaminSchubert opened MR !1179 (bschubert/allow-no-cov->master: Make code coverage reporting optional) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1179 | 11:32 |
| gitlab-br-bot | cs-shadow approved MR !1179 (bschubert/allow-no-cov->master: Make code coverage reporting optional) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1179 | 11:33 |
| * Kinnison is not currently, so would be very grateful if juergbi could look soon | 11:34 | |
| juergbi | ok, will take a look | 11:35 |
| Kinnison | thank you | 11:35 |
| tristan | cs-shadow, Added a review to that... I think it's quite ready to merge | 11:35 |
| tristan | cs-shadow, We really should go the extra mile and support the notation through Element.search() as well, but we could equally do that separately | 11:36 |
| tpollard | cheers juergbi | 11:44 |
| cs-shadow | tristan: that's a neat idea. I hadn't thought of that use-case but it can be interesting. Will look at fixing up your other comment and create a separate MR about the search. | 11:45 |
| cs-shadow | thanks for the quick turnaround :) | 11:45 |
| tristan | Thanks for bringing it up ! | 11:50 |
| tristan | Yeah, I happen to be doing the x86image thing trying to get a GNOME image up | 11:50 |
| tristan | And a "problem" I had was that I could not use directly the "deploy tools" from freedesktop-sdk as a sysroot for generating the image | 11:51 |
| tristan | So that would help in that case :) | 11:51 |
| *** lachlan has quit IRC | 12:25 | |
| gitlab-br-bot | marge-bot123 merged MR !1179 (bschubert/allow-no-cov->master: Make code coverage reporting optional) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1179 | 12:32 |
| *** lachlan has joined #buildstream | 12:36 | |
| tristan | This optionality of coverage is really useful... for package maintainers who want to run tests but don't necessarily have exactly the version of coverage we want | 12:42 |
| tristan | e.g. #916 | 12:42 |
| gitlab-br-bot | Issue #916: loosen dev dependencies https://gitlab.com/BuildStream/buildstream/issues/916 | 12:42 |
| tristan | Would be nice if running tests without coverage, also did not have the coverage dependency | 12:43 |
| *** lachlan has quit IRC | 12:57 | |
| *** raoul has quit IRC | 13:03 | |
| *** tristan has quit IRC | 13:07 | |
| *** lachlan has joined #buildstream | 13:08 | |
| *** lachlan has quit IRC | 13:11 | |
| *** tristan has joined #buildstream | 13:11 | |
| gitlab-br-bot | martinblanchard closed issue #626 (Cancel pending remote execution requests on shutdown) on buildstream https://gitlab.com/BuildStream/buildstream/issues/626 | 13:13 |
| gitlab-br-bot | martinblanchard closed issue #823 (Remote Execution: Support instance names in action-cache-service) on buildstream https://gitlab.com/BuildStream/buildstream/issues/823 | 13:13 |
| *** nimish has joined #buildstream | 13:26 | |
| benschubert | tristan: this would be harder to implement sadly :/ | 13:31 |
| *** ChanServ sets mode: +o tristan | 13:32 | |
| tristan | benschubert, I guess it means having separate envs for tests vs tests with coverage | 13:32 |
| tristan | and splitting up the requirements a bit more | 13:32 |
| benschubert | that would be a solution indeed | 13:33 |
| tristan | with tox I think we can easily reuse the commands of one env inside another (so it wouldnt be too wordy in the tox.ini) | 13:33 |
| tristan | I'll take a look anyway | 13:33 |
| * tristan goes to obtain pizza | 13:33 | |
| benschubert | tristan: please ping me on the MR if you get something | 13:35 |
| tristan | benschubert, will do :) | 13:38 |
| *** tristan has quit IRC | 13:41 | |
| *** raoul has joined #buildstream | 13:50 | |
| raoul | Anyone got any ideas on how I may have managed to break source determinism with the source cache? It currently stages the sources together during the fetch stage and then exports it later during `_stage_sources_at`, still doing the set deterministic methods, but permissions on the files for umask tests no longer match and I can't figure out why. | 13:54 |
| raoul | juergbi, I imagine you may have some idea? | 13:54 |
| Kinnison | We certainly do some umaskery in the CASCache | 13:55 |
| juergbi | raoul: CAS doesn't store any permissions (except for the executable bit) | 13:55 |
| juergbi | so the hardlinked files will always be either 0644 or 0755 | 13:55 |
| juergbi | if the test expects permissions other than those to be preserved, this will definitely fail | 13:55 |
| juergbi | however, it should still be deterministic | 13:55 |
| raoul | it's the test that checks whether the permissions are the same if it's run with two different umasks | 13:56 |
| raoul | so I think it is an issue | 13:56 |
| raoul | As far as I can tell as it imports the files to the virtual directory, which for checkouts will be file based, and then calls the deterministic methods on that, so I'm struggling to figure out what's causing the difference | 13:57 |
| juergbi | raoul: have you already checked what exact mode differences the test sees? | 13:58 |
| raoul | yeah, the umask with 0o077 has executable set compared to the one run with 0o022 | 14:00 |
| juergbi | raoul: it might also worth testing on top of my latest branch if you haven't already | 14:00 |
| raoul | I can have a go | 14:00 |
| *** phildawson has quit IRC | 14:03 | |
| *** raoul_ has joined #buildstream | 14:03 | |
| *** raoul has quit IRC | 14:03 | |
| *** tristan has joined #buildstream | 14:03 | |
| gitlab-br-bot | matthew-yates opened MR !1180 (rename-errno-fix->bst-1.2: plugins/sources/git.py: Cope with rename returning error EEXIST) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1180 | 14:05 |
| *** nimish has quit IRC | 14:07 | |
| *** lachlan has joined #buildstream | 14:09 | |
| *** raoul_ has quit IRC | 14:13 | |
| *** lachlan has quit IRC | 14:16 | |
| *** nimish has joined #buildstream | 14:16 | |
| *** lachlan has joined #buildstream | 14:20 | |
| *** phildawson has joined #buildstream | 14:59 | |
| *** raoul_ has joined #buildstream | 15:02 | |
| *** phildawson has quit IRC | 15:03 | |
| *** raoul_ has quit IRC | 15:04 | |
| *** phildawson has joined #buildstream | 15:05 | |
| *** raoul has joined #buildstream | 15:06 | |
| tpollard | thanks for the comment juergbi | 15:08 |
| juergbi | np, sorry for the rebase pain | 15:08 |
| juergbi | jonathanmaw: have you ever seen 'Failed to move cloned git repository [...] Permission denied' as spurious test failure on WSL? | 15:31 |
| juergbi | not sure yet whether it's spurious or my branch breaks that one test only on WSL for some reason | 15:31 |
| juergbi | tests/frontend/workspace.py::test_external_close_other | 15:32 |
| jonathanmaw | juergbi: hmm, I haven't seen it in WSL CI before | 15:32 |
| juergbi | ok, I'll check whether it's still failing on the next update | 15:33 |
| raoul | unfortunately rebasing on top of your branch juergbi has not magically fixed the source determinism :( | 15:46 |
| juergbi | was just a small chance anyway | 15:47 |
| raoul | it was fortunately a surprisingly painless rebase though | 15:48 |
| juergbi | probably more painful for tpollard, unfortunately | 15:48 |
| juergbi | raoul: can you be more specific in the mode difference? i.e., what are the full mode values? | 15:48 |
| raoul | I'll throw the results in a pastebin | 15:49 |
| *** toscalix has quit IRC | 15:50 | |
| raoul | https://pastebin.com/Yvcu9F2S | 15:52 |
| raoul | that's for tests/integration/source-determinism.py::test_deterministic_source_umask[tar] | 15:52 |
| *** lachlan has quit IRC | 15:53 | |
| juergbi | ah, for directories | 16:10 |
| juergbi | raoul: we probably should explicitly call chmod() on created directories | 16:11 |
| juergbi | when extracting from CAS | 16:11 |
| juergbi | hm, I think that requires separate syscall :/ | 16:12 |
| juergbi | utils._ensure_real_directory() and FileBasedDirectory.descend() would likely be affected | 16:13 |
| juergbi | and possibly CASCache.checkout() for consistency | 16:14 |
| juergbi | wondering whether that is really the best approach, though | 16:14 |
| juergbi | one possible alternative would be to explicitly change the umask to 0o22, i.e., ignoring the user's environment | 16:15 |
| juergbi | we can't completely respect the user setting anyway in the name of determinism | 16:15 |
| juergbi | Kinnison might have an opinion on the above | 16:16 |
| *** lachlan has joined #buildstream | 16:17 | |
| Kinnison | wrt. anything where the permissions are our responsibility, I think it's reasonable to not honour the user's umask | 16:17 |
| Kinnison | Where it's the user's responsibility (e.g. when writing out a tracked YAML) we should honour it | 16:17 |
| juergbi | that makes sense, it's just somewhat painful having to support both | 16:18 |
| juergbi | as in extra syscalls. although, I don't know how big the impact is. it seems generally the Python userspace overhead is a lot higher than the total syscall overhead | 16:18 |
| *** lachlan has quit IRC | 16:21 | |
| raoul | juergbi, as in change the umask setting in the source-determinism test? Or ignoring the user environment when running builds? | 16:25 |
| raoul | (it's entirely possible I don't quite get umask cause I've only looked at it today...) | 16:25 |
| juergbi | the alternative would be the latter, i.e., set umask in buildstream itself | 16:25 |
| juergbi | actually, maybe we should set the umask at the beginning of those CAS export methods | 16:26 |
| juergbi | and restore the previous value at the end | 16:26 |
| juergbi | a context manager could be suitable for this | 16:26 |
| juergbi | that way everything outside CAS export still honors the user's umask | 16:26 |
| juergbi | but we don't have to manually chmod everything during CAS export | 16:26 |
| raoul | so something similar to what's being done in the source determinism test, but a context manager used for CAS exports? | 16:27 |
| juergbi | yes | 16:27 |
| juergbi | and always using 0o022 | 16:27 |
| juergbi | deterministic_umask context manager, or something like this | 16:28 |
| *** lachlan has joined #buildstream | 16:37 | |
| *** lachlan has quit IRC | 16:43 | |
| jennis | Does anyone have any ideas on how to get tox working in situations where we introduce another python dependency? | 16:44 |
| jennis | I thought that appending to the requirements.in file would do the trick, but the import is still failing | 16:44 |
| cs-shadow | jennis: tox only looks at .txt files | 16:44 |
| jennis | I've also appended to the .txt file | 16:45 |
| juergbi | tox --recreate | 16:45 |
| cs-shadow | jennis: try with --recreate flag | 16:45 |
| cs-shadow | ^ :) | 16:45 |
| jennis | cs-shadow, juergbi, thanks | 16:46 |
| gitlab-br-bot | juergbi opened MR !1181 (juerg/directory->master: Virtual directory improvements) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1181 | 16:48 |
| laurence | does anyone recall which gnome sysadmin helped us set-up the buildstream-notifications list ? | 16:50 |
| laurence | maybe it was jjardon ? | 16:50 |
| tpollard | juergbi: might be a placebo and I don't have metrics yet, but basing juerg/virtual-artifact-directory seems to have given me a visible decrease in test suite time | 16:57 |
| juergbi | some I/O reduction is indeed expected | 16:58 |
| juergbi | staging time is also reduced by about 35% in my test | 16:59 |
| juergbi | (even excluding the time to create extract directories, which is not done anymore) | 16:59 |
| *** lachlan has joined #buildstream | 17:03 | |
| *** lachlan has quit IRC | 17:06 | |
| *** lachlan has joined #buildstream | 17:21 | |
| *** nimish has quit IRC | 17:21 | |
| *** lachlan has quit IRC | 17:27 | |
| *** lachlan has joined #buildstream | 17:28 | |
| *** lachlan has quit IRC | 17:35 | |
| juergbi | anyone in the review mood for directory operations: !1181, !1182, !1183, !1184 (MR series with dependencies) | 17:37 |
| gitlab-br-bot | MR !1182: WIP: Replace file lists with filter callback for file import https://gitlab.com/BuildStream/buildstream/merge_requests/1182 | 17:37 |
| gitlab-br-bot | MR !1183: WIP: Directory.import_files() improvements https://gitlab.com/BuildStream/buildstream/merge_requests/1183 | 17:37 |
| gitlab-br-bot | MR !1184: WIP: Use virtual artifact directory to stage and extract metadata https://gitlab.com/BuildStream/buildstream/merge_requests/1184 | 17:37 |
| tpollard | I'll definitely give them a look over tomorrow | 17:38 |
| juergbi | ta | 17:39 |
| tpollard | I need to grok the filter callback stuff, as I'm still attacking the compute splits etc | 17:39 |
| juergbi | tpollard: would probably be good to pay special attention to anything buildtree-related (e.g., test changes) | 17:39 |
| tpollard | yup! | 17:40 |
| juergbi | I hope it's more or less comprehensible, the commit splitting for this change should help | 17:40 |
| juergbi | otherwise let me know | 17:40 |
| *** toscalix has joined #buildstream | 17:44 | |
| *** tristan has quit IRC | 17:47 | |
| *** lachlan has joined #buildstream | 17:49 | |
| *** jonathanmaw has quit IRC | 17:54 | |
| *** lachlan has quit IRC | 17:56 | |
| *** phildawson has quit IRC | 18:00 | |
| *** lachlan has joined #buildstream | 18:12 | |
| gitlab-br-bot | raoul.hidalgocharman approved MR !1181 (juerg/directory->master: Virtual directory improvements) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1181 | 18:26 |
| *** raoul has quit IRC | 18:29 | |
| *** tristan has joined #buildstream | 18:30 | |
| *** lachlan has quit IRC | 18:35 | |
| *** toscalix has quit IRC | 18:36 | |
| *** lachlan has joined #buildstream | 18:40 | |
| *** lachlan has quit IRC | 19:00 | |
| *** lachlan has joined #buildstream | 19:22 | |
| *** alatiera has quit IRC | 19:35 | |
| *** alatiera has joined #buildstream | 20:33 | |
| *** lachlan has quit IRC | 20:57 | |
| gitlab-br-bot | cs-shadow opened issue #931 (Support cross-junction elements in `Element.search()`) on buildstream https://gitlab.com/BuildStream/buildstream/issues/931 | 21:45 |
| gitlab-br-bot | marge-bot123 closed issue #809 (Allow cross-junction dependencies to be listed as strings) on buildstream https://gitlab.com/BuildStream/buildstream/issues/809 | 22:16 |
| gitlab-br-bot | marge-bot123 merged MR !998 (chandan/junction-dependency-format->master: loader: Allow dependencies to use ":" to refer to junctioned elements) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/998 | 22:16 |
| *** tristan has quit IRC | 23:07 | |
| jjardon | laurence: probably "av" in #sysadmin | 23:40 |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!