| *** traveltissues has joined #buildstream | 08:23 | |
| coldtom | anyone against merging !1759? | 08:25 |
|---|---|---|
| gitlab-br-bot | MR !1759: _project.py: Allow junctions to use parent remote when `ignore-junction-remotes` set https://gitlab.com/BuildStream/buildstream/merge_requests/1759 | 08:25 |
| gitlab-br-bot | traveltissues approved MR !1759 (coldtom/fix-junction-remotes->master: _project.py: Allow junctions to use parent remote when `ignore-junction-remotes` set) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1759 | 08:31 |
| *** tme5 has joined #buildstream | 08:57 | |
| gitlab-br-bot | juergbi approved MR !1759 (coldtom/fix-junction-remotes->master: _project.py: Allow junctions to use parent remote when `ignore-junction-remotes` set) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1759 | 09:17 |
| juergbi | coldtom: no, makes sense to me | 09:17 |
| coldtom | great, is marge working now? or will someone need to manually merge? | 09:17 |
| juergbi | I think marge still has issues due to the failing remote execution test | 09:18 |
| juergbi | !1758 was just rebased, possibly to be merged, so maybe we should wait for that and then rebase | 09:20 |
| gitlab-br-bot | MR !1758: Drop support for `setup.py test` https://gitlab.com/BuildStream/buildstream/merge_requests/1758 | 09:20 |
| juergbi | but I can merge it afterwards if you can't | 09:20 |
| *** santi has joined #buildstream | 09:36 | |
| *** jonathanmaw has joined #buildstream | 10:23 | |
| juergbi | tlater[m]: can you please take another look at !1738? | 10:25 |
| gitlab-br-bot | MR !1738: Add buildbox-run sandboxing backend https://gitlab.com/BuildStream/buildstream/merge_requests/1738 | 10:25 |
| *** lachlan has joined #buildstream | 10:34 | |
| tlater[m] | juergbi: sure | 10:37 |
| gitlab-br-bot | tlater approved MR !1738 (juerg/buildbox-run->master: Add buildbox-run sandboxing backend) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1738 | 10:42 |
| gitlab-br-bot | juergbi merged MR !1759 (coldtom/fix-junction-remotes->master: _project.py: Allow junctions to use parent remote when `ignore-junction-remotes` set) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1759 | 11:42 |
| *** lachlan has quit IRC | 11:56 | |
| *** lachlan has joined #buildstream | 12:10 | |
| *** lachlan has quit IRC | 12:13 | |
| *** lachlan has joined #buildstream | 12:17 | |
| *** lachlan has quit IRC | 12:21 | |
| *** lachlan has joined #buildstream | 12:32 | |
| gitlab-br-bot | juergbi merged MR !1738 (juerg/buildbox-run->master: Add buildbox-run sandboxing backend) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1738 | 12:34 |
| gitlab-br-bot | juergbi approved MR !1763 (tlater/tar-test-security->master: tests/sources/tar.py: `utils._force_rmtree` instead of giving 777 permissions) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1763 | 12:39 |
| tme5 | does anyone know how to run the tests in _sourcetests/mirror.py locally? (the ones which fail here https://gitlab.com/BuildStream/buildstream/-/jobs/374732737) | 12:42 |
| tme5 | I've tried running via tox on that file but it just skips them | 12:42 |
| juergbi | tme5: if it doesn't work by file, maybe it works by pattern: e.g., tox -- -k test_mirror_fetch | 12:43 |
| tpollard | tox -e sourcetests -- $nameoftest | 12:45 |
| tpollard | https://gitlab.com/BuildStream/buildstream/blob/master/tox.ini#L204 | 12:45 |
| tme5 | both of those give me errors hahah | 12:46 |
| tme5 | it appears I don't have pexpect in tox | 12:46 |
| tlater[m] | tme5: What errors? | 12:47 |
| tlater[m] | Ah | 12:48 |
| tlater[m] | tme5: Between when you last ran the test suite and now someone added interactive tests | 12:48 |
| tlater[m] | You'll need to run tox -re -- -k test_mirror_fetch | 12:49 |
| tlater[m] | Ot actually, just tox -r | 12:49 |
| tlater[m] | (With whatever other things you want to make the run more specific) | 12:49 |
| tlater[m] | `tox -r` will re-create the virtualenv, and add the new stuff :) | 12:49 |
| tme5 | ah thanks that's worked | 12:51 |
| *** santi has quit IRC | 13:13 | |
| gitlab-br-bot | tlater merged MR !1763 (tlater/tar-test-security->master: tests/sources/tar.py: `utils._force_rmtree` instead of giving 777 permissions) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1763 | 13:22 |
| *** santi has joined #buildstream | 13:27 | |
| *** santi has quit IRC | 13:43 | |
| tme5 | Could anyone help me out? I'm trying to understand what test_mirror_fetch does. It seems to create two repos, one is the other but with an extra commit, and then it sets up an element to source from the repo with the missing commit (via an alias, which I don't understand the purpose of) | 13:44 |
| tme5 | i believe the intent is to test a cache updating with new commits from upstream? but it's not clear to me how the "upstream_repo" is set as the upstream of the "mirror_repo" | 13:51 |
| *** toscalix has joined #buildstream | 14:00 | |
| tlater[m] | benschubert: You might know more about ^, or was that cs-shadow? | 14:01 |
| benschubert | ah I *think* I have touched that recently-ish | 14:02 |
| benschubert | let me look at it | 14:02 |
| benschubert | I can tell that this needs either more explicit comments or rewriting | 14:04 |
| benschubert | I *think* the aim is to ensure we can fetch from a different mirror if we setup an alias correctly | 14:04 |
| benschubert | wait no | 14:05 |
| tlater[m] | I concur, btw | 14:05 |
| tlater[m] | Definitely at least needs some comments explaining what's going on :D | 14:05 |
| benschubert | I think what's happening is: | 14:07 |
| benschubert | 1) We create a repo with a mirror, the mirror is missing a ref that upstream has | 14:07 |
| benschubert | 2) We source fetch, and the mirror should be taken first, but it doesn't have the ref, so it gracefully should fallback to upstream | 14:07 |
| benschubert | Would that be coherent with your understanding tlater[m], tme5 ? | 14:07 |
| benschubert | If that's the case, yeah a rewriting.... | 14:07 |
| tlater[m] | benschubert: Roughly. I confused upstream_repo, mirror_repo and upstream_ref, but on second read, that seems to make sense. | 14:08 |
| benschubert | tlater[m]: btw is https://gitlab.com/BuildStream/buildstream/merge_requests/1760 fine with you now? :) | 14:11 |
| tlater[m] | benschubert: Hmm | 14:12 |
| tlater[m] | Is 8 extra minutes in CI that bad | 14:12 |
| tlater[m] | I suppose it isn't nice for those who like testing through CI | 14:12 |
| tlater[m] | And the randomization will be flaky anyway | 14:12 |
| tlater[m] | Yeah, lgtm | 14:12 |
| gitlab-br-bot | tlater approved MR !1760 (bschubert/add-randomized-order-tests->master: tox.ini: Add ability to run tests in a randomized order) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1760 | 14:12 |
| benschubert | Yeah, unless we have it run reliably for a time, I would rather not have it right now :) | 14:13 |
| benschubert | And same, I've been working on having our tests run in a separate process each, to ensure they are completely isolated relative to the code, once those work I'll also add them as overnight :) | 14:14 |
| *** santi has joined #buildstream | 14:21 | |
| *** traveltissues has quit IRC | 14:22 | |
| *** traveltissues has joined #buildstream | 14:23 | |
| gitlab-br-bot | BenjaminSchubert approved MR !1762 (jjardon/fdsdk-190805->master: .gitlab-ci.yml: Use latest freedesktop-sdk 19.08.5) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1762 | 14:27 |
| gitlab-br-bot | BenjaminSchubert merged MR !1760 (bschubert/add-randomized-order-tests->master: tox.ini: Add ability to run tests in a randomized order) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1760 | 14:28 |
| *** toscalix has quit IRC | 14:45 | |
| *** toscalix has joined #buildstream | 14:46 | |
| tme5 | benschubert, i think i understand now, thank you | 14:47 |
| benschubert | tme5: happy to help. :) | 14:48 |
| tme5 | i broke the test because of a seemingly odd asymmetry between sources which implement fetch and those which implement get_source_fetchers | 14:48 |
| tme5 | if i want to mark something as needing further fixing etc. should i use XXX in a comment? | 14:50 |
| benschubert | further fixing as the tests don't pass? I would use pytest.mark.xfail(reason="THE REASON IF FAILS") and add a TODO or FIXME | 14:51 |
| tlater[m] | Also, an issue is often appropriate. | 14:51 |
| benschubert | tlater[m]: unless it's done in the same MR :) | 14:52 |
| tme5 | the test passes but basically i'm having to rely on undocumented implementation detail | 14:52 |
| tme5 | of a public API function | 14:52 |
| tme5 | nope that is indeed a complete lie and I can't read | 14:53 |
| tme5 | never mind! | 14:53 |
| tme5 | :) | 14:53 |
| *** traveltissues_ has joined #buildstream | 14:58 | |
| *** traveltissues has quit IRC | 14:58 | |
| gitlab-br-bot | BenjaminSchubert opened issue #1236 (Follow-up from "WIP: Optimize consistency and state handling") on buildstream https://gitlab.com/BuildStream/buildstream/issues/1236 | 15:00 |
| benschubert | tlater[m]: if you have time for another look at !1739. I believe it should be good for benchmarking and proposal on the ML :) | 15:06 |
| gitlab-br-bot | MR !1739: WIP: Optimize consistency and state handling https://gitlab.com/BuildStream/buildstream/merge_requests/1739 | 15:06 |
| benschubert | (will still need to update the NEWS) | 15:07 |
| tlater[m] | benschubert: Sure | 15:08 |
| *** lachlan has quit IRC | 15:19 | |
| tlater[m] | benschubert: Looks pretty much good to me | 15:21 |
| tlater[m] | Obviosuly those comments will still need cleaning, but it's good enough for a proposal :) | 15:22 |
| *** lachlan has joined #buildstream | 15:22 | |
| gitlab-br-bot | juergbi closed issue #1226 (Add CI job for buildbox-run-bubblewrap with buildbox-fuse) on buildstream https://gitlab.com/BuildStream/buildstream/issues/1226 | 15:24 |
| gitlab-br-bot | juergbi merged MR !1752 (juerg/buildbox-run-bubblewrap->master: Add CI job to test buildbox-run-bubblewrap and buildbox-fuse) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1752 | 15:24 |
| benschubert | yep :) I'll prepare a ML post then | 15:28 |
| gitlab-br-bot | tpollard opened MR !1764 (tpollard/showjunction->master: _frontend/widget.py: show_pipeline() Don't show buildable for junction) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1764 | 15:40 |
| *** narispo has quit IRC | 15:42 | |
| *** narispo has joined #buildstream | 15:42 | |
| *** qinusty has quit IRC | 15:51 | |
| *** ikerperez has quit IRC | 15:51 | |
| *** adds68 has quit IRC | 15:51 | |
| *** lachlan has quit IRC | 15:58 | |
| *** lachlan has joined #buildstream | 15:59 | |
| tme5 | what's your policy of scopes of MRs? | 16:05 |
| tlater[m] | tme5: There's no particular policy. Preferably an MR covers one topic/solves one problem, but things often blur depending on what level of abstraction you're looking at. | 16:05 |
| tlater[m] | If an MR gets very big because you chunk lots of things into it, someone might suggest you split it to make it easier to work on/review/merge | 16:06 |
| tme5 | my branch mostly affects one file, can my branch do minor clean-up/commenting/refactoring in functions i'm using? | 16:06 |
| tme5 | said changes have their own commits | 16:06 |
| tlater[m] | Yes, that's not uncommon, though you should preferably keep refactoring to separate commits | 16:07 |
| *** traveltissues_ has quit IRC | 16:07 | |
| tlater[m] | tme5: No problem at all with that :) | 16:07 |
| tme5 | ok nice, thanks | 16:07 |
| juergbi | tlater[m]: the tar change breaks userchroot: https://gitlab.com/BuildStream/buildstream/-/jobs/375033442 | 16:18 |
| juergbi | need to fix up utils._force_rmtree, it shouldn't fail in this case | 16:19 |
| tlater[m] | :( | 16:19 |
| tlater[m] | Does that test normally not run? | 16:19 |
| tlater[m] | Oh, userchroot | 16:20 |
| tlater[m] | Yeah | 16:20 |
| tlater[m] | juergbi: That'll be because we don't have group permissions, right? | 16:20 |
| tlater[m] | It should become 775, but only for that branch | 16:20 |
| juergbi | tlater[m]: no, the immediate issue is that chmod() throws an exception because the file owner is casd, not bst | 16:21 |
| juergbi | (EPERM) | 16:21 |
| juergbi | it would likely work just fine without the chmod | 16:21 |
| tlater[m] | Hm, strange, surely that should have happened with the old implementation too? | 16:22 |
| juergbi | no, the old implementation only called chmod on failure | 16:22 |
| juergbi | force_rmtree always calls it | 16:22 |
| tlater[m] | We could probably do that here too, to be fair | 16:22 |
| tlater[m] | This would fit in the general permissions fix MR, right? | 16:23 |
| tlater[m] | Is that one still open or should I start a new one? | 16:23 |
| juergbi | that was already merged a while ago | 16:24 |
| juergbi | so either a new one or we'll add it to the userchroot one | 16:24 |
| tlater[m] | Probably doesn't make sense without the userchroot change, to be fair | 16:24 |
| juergbi | not sure whether it makes more sense to switch to onerror or ignore EPERM on chmod | 16:26 |
| tlater[m] | juergbi: We could get EPERM for other reasons, no? | 16:27 |
| juergbi | EPERM on chmod can only happen if you're not the file owner, afaict | 16:27 |
| tlater[m] | Hm, if that's guaranteed we can probably ignore it | 16:28 |
| tlater[m] | But I'm not sure I like that. It makes more sense to only change permissions if we realize we need to in my mind, even if it's slightly racey | 16:29 |
| juergbi | it's what the POSIX manual says. on Linux EPERM can also happen if the file is marked immutable or append-only (file attributes that are not part of POSIX) | 16:29 |
| juergbi | I'd be fine with switching to onerror | 16:30 |
| tlater[m] | I think I prefer that. | 16:30 |
| *** adds68 has joined #buildstream | 16:34 | |
| *** ikerperez has joined #buildstream | 16:36 | |
| gitlab-br-bot | tmewett opened MR !1765 (tmewett/recursive-git->master: Make Git source plugin clone submodules recursively) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1765 | 16:40 |
| *** phildawson has joined #buildstream | 16:41 | |
| tme5 | ^ ready for review :) | 16:43 |
| *** santi has quit IRC | 16:44 | |
| *** santi has joined #buildstream | 17:00 | |
| *** phildawson_ has joined #buildstream | 17:06 | |
| *** phildawson_ has quit IRC | 17:15 | |
| *** phildawson_ has joined #buildstream | 17:16 | |
| *** toscalix has quit IRC | 17:19 | |
| *** toscalix has joined #buildstream | 17:20 | |
| tpollard | interrupt handling seems much more consistent for me on the debian target since !1755 btw benschubert | 17:25 |
| gitlab-br-bot | MR !1755: scheduler.py: Optimize scheduling by not calling it unnecessarily https://gitlab.com/BuildStream/buildstream/merge_requests/1755 | 17:25 |
| tpollard | before it could often take quite a delay to propagate the interrupt | 17:26 |
| tpollard | I guess due to the signal overloading | 17:28 |
| tpollard | or it blocking from writing the interrupt dialogue to the UI whilst it processed the warning messages | 17:29 |
| gitlab-br-bot | juergbi opened issue #1237 (Add CI job for buildbox-run-userchroot) on buildstream https://gitlab.com/BuildStream/buildstream/issues/1237 | 17:31 |
| gitlab-br-bot | juergbi closed issue #1177 (Support buildbox-run as sandboxing backend) on buildstream https://gitlab.com/BuildStream/buildstream/issues/1177 | 17:34 |
| *** toscalix has quit IRC | 17:35 | |
| *** toscalix has joined #buildstream | 17:36 | |
| juergbi | tpollard: yes, that makes sense. SIGINT is ignored whenever the asyncio socket buffer is full, which was frequently the case in that test repo | 17:37 |
| juergbi | tpollard: do you need new WSL measurements with the rebased branch? | 17:37 |
| *** tme5 has quit IRC | 17:39 | |
| benschubert | Happy to hear that :) | 17:41 |
| benschubert | And I believe we'll never hit the warning again, unless we have a huge amount of builders stopping at the exact same time :) | 17:44 |
| *** toscalix has quit IRC | 17:49 | |
| *** toscalix has joined #buildstream | 17:58 | |
| *** toscalix has quit IRC | 18:00 | |
| tlater[m] | tme5: (whenever you're around) URIs aren't allowed to have non-absolute paths: https://tools.ietf.org/html/rfc3986#section-3 | 18:00 |
| *** lachlan has quit IRC | 18:00 | |
| *** toscalix has joined #buildstream | 18:01 | |
| *** toscalix has quit IRC | 18:02 | |
| tlater[m] | Oh, hang on, path-rootless... | 18:02 |
| tlater[m] | Nevermind, I might have misinterpreted that | 18:03 |
| *** santi has quit IRC | 18:03 | |
| *** jonathanmaw has quit IRC | 18:04 | |
| juergbi | tlater[m]: yes, URIs can definitely be relative. however, they aren't allowed to contain arbitrary characters (without escaping) | 18:05 |
| juergbi | so I still think we need a separate path field | 18:06 |
| *** tiagogomes has quit IRC | 18:06 | |
| *** lachlan has joined #buildstream | 18:06 | |
| benschubert | I dislike this idea, that will make simple elements harder to write and remove some guarantees (cached state, etc) that we currently have, I'll have to write an answer to the ML | 18:08 |
| benschubert | I'd rather go for the project-path variable and treat them as normal remote sources | 18:09 |
| benschubert | But thoses thoughts will go on the ML tomorrow | 18:09 |
| benschubert | Afaik, we should already be supporting any uri scheme without change, "just" need to document/add the variable and we would have a solid system | 18:11 |
| *** lachlan has quit IRC | 18:17 | |
| *** lachlan has joined #buildstream | 18:30 | |
| *** rdale has quit IRC | 18:33 | |
| Kinnison | Does anyone know why master has issues with 3.8? | 19:13 |
| Kinnison | coldtom: ^^? | 19:14 |
| *** lachlan has quit IRC | 19:19 | |
| *** phildawson_ has quit IRC | 19:35 | |
| *** benschubert has quit IRC | 20:30 | |
| *** benschubert has joined #buildstream | 21:49 | |
| benschubert | Kinnison: the problem is between coverage, the asyncio loop, its signals and python 3.8 | 21:49 |
| benschubert | I've tried looking at it, but still haven't found out the root cause. Also coverage 5.0 should be out by end of week or next week, and it's not working either with our code so I wonder if that might be easier to fix first | 21:51 |
| benschubert | And there's been quite a few change on asyncio in 3.8 :( | 21:52 |
| benschubert | https://gitlab.com/BuildStream/buildstream/tree/bschubert/python38 is where I am at currently | 21:59 |
| benschubert | Afaik, my branch should work when using BuildStream | 22:01 |
| *** phildawson_ has joined #buildstream | 23:03 | |
| *** phildawson_ has quit IRC | 23:06 | |
| Kinnison | ta benschubert | 23:18 |
| *** phildawson_ has joined #buildstream | 23:33 | |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!