*** nimish has joined #buildstream | 01:14 | |
*** mohan43u has quit IRC | 02:44 | |
*** alatiera has quit IRC | 02:55 | |
*** kapil___ has quit IRC | 03:02 | |
*** nimish has quit IRC | 03:18 | |
*** mohan43u has joined #buildstream | 04:13 | |
*** mohan43u has quit IRC | 04:39 | |
*** mohan43u has joined #buildstream | 04:54 | |
gitlab-br-bot | juergbi opened (was WIP) MR !1184 (juerg/virtual-artifact-directory->master: Use virtual artifact directory to stage and extract metadata) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1184 | 05:05 |
---|---|---|
*** tristan has joined #buildstream | 05:40 | |
*** ChanServ sets mode: +o tristan | 05:40 | |
gitlab-br-bot | juergbi opened MR !1192 (juerg/remote-execution-cas->master: _sandboxremote.py: Use empty CasBasedDirectory as initial sandbox root) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1192 | 06:04 |
tristan | Appears that we don't collect coverage reports from the wsl tests :-S | 06:41 |
tristan | I guess the fact that the test is allowed to fail complicates this | 06:42 |
juergbi | tristan: for fedora-update-deps we do collect coverage, though, even though that's also allowed to fail | 06:45 |
juergbi | if I'm reading it correctly | 06:45 |
juergbi | can we just do the same for WSL or is it more complicated? | 06:46 |
tristan | juergbi, we might be *generating* coverage for that one, but I don't think we're collecting/collating it | 06:47 |
tristan | i.e. it needs to be explicitly listed in the dependencies of the coverage job afaiu | 06:47 |
juergbi | it already is | 06:48 |
tristan | So if we have a job which depends on an artifact that is conditionally created... the job will still run ? | 06:48 |
tristan | That is nice | 06:48 |
juergbi | update-deps doesn't fail frequently but I think I've seen it fail before without failing the pipeline | 06:48 |
juergbi | might misremember that, though | 06:48 |
tristan | juergbi, I expect it to be more complicated, only inasmuch as we probably need to adjust .coveragerc to list win32 path patterns for collation of results | 06:49 |
juergbi | hm, we're not running on Win32 | 06:49 |
juergbi | the runner runs inside WSL | 06:49 |
juergbi | i.e., it's not a Windows runner that starts WSL | 06:50 |
tristan | but you are right about the update-deps thing | 06:50 |
tristan | it is allow_failure and collected in coverage | 06:50 |
tristan | Ah I see, so since the runner itself runs inside WSL it should have pretty much the same behaviors as on linux, pathwise at least | 06:51 |
juergbi | yes, it's not based on a Docker base image, but other than that it should be fairly similar in behavior | 06:52 |
juergbi | jonathanmaw will know the details | 06:52 |
tristan | grrr, so there is something weird about that image :-S | 06:53 |
tristan | https://gitlab.com/BuildStream/buildstream/-/jobs/169031047 shows that tox is already up to date (3.7.0) in /root/.local/lib/python3.6/site-packages... and `tox --version` reports ancient "2.5.0 imported from /usr/lib/python3/dist-packages/tox/__init__.py" | 06:55 |
* tristan thinks this might just be a PATH issue | 06:56 | |
juergbi | possible | 06:58 |
tristan | Yup, it was | 06:58 |
tristan | I am curious though, I have a sneaking suspicion that I messed things up, or fixed them in a bad way | 06:59 |
tristan | Are we at least guaranteed that the runner state is clean on every run ? | 07:00 |
tristan | I.e. no side effects will be carried over from run to run except for those stored in the cache ? | 07:00 |
tristan | Right, so there is also a tox in /root/.local/lib/python3.6/site-packages/tox/__init__.py, but that might be the install that I did in a previous test | 07:07 |
tristan | A test I ran with the blind confidence that of course, whatever I do in a single test run could never have any affects on other pipelines | 07:08 |
juergbi | with my latest (unpushed) branch I can run the whole test suite in 250s :) | 07:36 |
juergbi | (down from about 400s) | 07:36 |
tristan | juergbi, How did you manage that ? | 07:42 |
juergbi | tristan: switching to CasBasedDirectory for `import` elements | 07:43 |
tristan | Ahh | 07:43 |
juergbi | they don't need any sandbox | 07:43 |
tristan | Hah | 07:43 |
juergbi | and I've optimized CasBasedDirectory a lot this week | 07:43 |
tristan | Yeah I've been watching the branches land | 07:44 |
tristan | (someones been shaking the tree heh) | 07:44 |
juergbi | yes, got staging time down by 40% for local sandboxes, and down by >80% for buildbox / remote execution | 07:45 |
juergbi | now the next bottleneck for the latter (virtual staging) is the overhead of filling result.files_written, which is only needed for overlap warnings | 07:46 |
juergbi | (to figure out the conflicting element, iirc) | 07:46 |
tristan | Once we solve the problem of how to decide on which elements to cache build trees for, master will be very attractive ! | 07:46 |
juergbi | overlap warnings: maybe we can find another approach for this. however, I might not tackle this right now. virtual staging is already reasonably fast now (with MRs) | 07:47 |
juergbi | build trees: well, we already did land an option for this, the question is mainly whether the default should be changed | 07:47 |
tristan | Is it ? | 07:48 |
juergbi | yes, 'cache-buildtrees' | 07:48 |
tristan | Yeah but, are failed builds unconditional ? | 07:48 |
juergbi | it supports 'cache-buildtrees: failure' | 07:48 |
juergbi | which I use and I think would be a reasonable default | 07:49 |
juergbi | it also supports 'cache-buildtrees: never' but I don't think this is really useful | 07:49 |
tristan | I'm not sure that's an API we want long term honestly | 07:49 |
juergbi | I agree. the tristate was chosen matching pull-buildtrees config | 07:49 |
juergbi | but we could remove support for that 'never' state | 07:49 |
tristan | maybe "when useful" is more interesting than having the user understand when useful is, and explicitly saying "failure" | 07:49 |
juergbi | that's an odd config value name ;) | 07:50 |
tristan | I.e. I am already seeing recent issue reports about incremental workspace builds with remote execution | 07:50 |
tristan | juergbi, yeah, I'm not really serious about the name, just think that this is information a user shouldnt have to know | 07:50 |
juergbi | well, I'd be in favor of making 'failure' the default | 07:51 |
tristan | as long as those edge cases grow, those edge cases should be a part of the default | 07:51 |
juergbi | in which case, the user doesn't really need to know about this and only needs to know about 'always', if they want to always cache it | 07:51 |
tristan | without changing the default, I think. | 07:51 |
juergbi | I don't understand what you mean with that | 07:51 |
tristan | I mean a setting name which means "cache build trees in the edge cases where I am likely to need them" | 07:52 |
tristan | So that when those edge cases increase, the default setting doesnt change; only it covers more cases | 07:52 |
juergbi | i.e., you're expecting further edge cases than 'failure' where we want to cache them by default | 07:52 |
tristan | How do you do incremental workspace builds, especially with remote execution, without caching the build trees of previous runs ? | 07:53 |
juergbi | right, that's required | 07:53 |
tristan | I also expect we will need it for tests, even though the world is in denial | 07:53 |
juergbi | for some tests / test setups, it will likely be required | 07:54 |
tristan | if we ever parallelize reverse dependency builds with make check - we need that (I am thinking something like "intermediate elements" would be a nice solution for that) | 07:54 |
juergbi | but it's definitely possible to run tests in different ways | 07:54 |
tristan | (like .INTERMEDIATE: in make) | 07:54 |
juergbi | maybe we should simply call it 'cache-buildtrees: default' | 07:55 |
juergbi | or auto | 07:55 |
tristan | I mean, if we dont parallelize reverse dependency builds, then it's not even really worth any discussion imo, nothing stops people from running make check after running make right now (no support needed) | 07:55 |
juergbi | of course | 07:55 |
tristan | juergbi, that is kind of my point yeah | 07:55 |
tristan | auto would be a good word | 07:56 |
juergbi | installed tests are the other option, but that's not commonly supported | 07:56 |
tristan | I don't think they are mutually exclusive anyway | 07:56 |
juergbi | no | 07:57 |
tristan | Running installed tests is also nice to do, of course | 07:57 |
juergbi | tristan: does this mean you'd be happy with a MR renaming 'failure' to 'auto', dropping 'never', and making 'auto' the new default | 07:57 |
tristan | juergbi, *I* would definitely yes | 07:57 |
tristan | I don't really expect objections to that :) | 07:58 |
juergbi | great, let's do that :) | 07:58 |
*** paulsherwood has joined #buildstream | 08:16 | |
Kinnison | Is project.conf allowed to (@) through a junction element? | 08:18 |
*** kapil___ has joined #buildstream | 08:20 | |
tristan | Kinnison, Yes it should | 08:22 |
tristan | Kinnison, iirc there is just some fields which cannot be derived from cross junction includes in project.conf | 08:23 |
juergbi | yes, we have a two stage project load | 08:23 |
tristan | probably fields which one would not be interested in cross-junction-include defining anyway | 08:23 |
tristan | (and it was definitely designed with this in mind specifically, as it is particularly useful in project.conf to allow such derivation) | 08:25 |
tristan | Is there a link/url for the remote execution API itself ? | 08:26 |
tristan | not necessarily terse documentation (preferably a project website if there is one, for a blog post) | 08:27 |
juergbi | https://github.com/bazelbuild/remote-apis/ | 08:27 |
juergbi | that's as close as it gets | 08:27 |
tristan | Ah that is pretty good though | 08:27 |
juergbi | yes, but I think the API design document is still in Google Docs and not linked | 08:27 |
juergbi | but at least the .protos with API documentation are maintained there | 08:28 |
juergbi | on github | 08:28 |
tristan | Sure, I was more looking for an introductory "what this is" page anyway :) | 08:28 |
tristan | and this has a README and seems right | 08:28 |
Kinnison | Can options be included across junctions? | 08:31 |
tristan | No | 08:34 |
Kinnison | So if I junction a project in, I have to redeclare all of its options in my project.conf ? | 08:34 |
tristan | Not either | 08:34 |
Kinnison | Or are they namespaced so I can set foojunction.bst:someoption to somevalue ? | 08:34 |
juergbi | you can set subproject options in the junction itself | 08:34 |
juergbi | https://docs.buildstream.build/elements/junction.html#options | 08:35 |
tristan | right, you are allowed to select any options that a subproject provides, and you can set it with a variable (so you can partially automate this if the settings are mirrorable) | 08:35 |
juergbi | or maybe better https://docs.buildstream.build/elements/junction.html#overview | 08:35 |
tristan | But you are by no means obligated to support all of the options a subproject exposes | 08:35 |
juergbi | if you want the same options in your projects, you indeed have to redeclare them, though | 08:36 |
juergbi | afaik | 08:36 |
Kinnison | Aah, so if I want to, I can set the options to non-default values, but if I want my user to be able to do so, I have to mirror/whatever so that they set an option in me, and I then use that to set an option in the junctioned project? | 08:36 |
* Kinnison phews | 08:36 | |
juergbi | yes | 08:36 |
Kinnison | OK good | 08:36 |
Kinnison | when I was re-reading the intro to the project.conf stuff last night I feared a situation of astable option inclusion via junctions and nearly panicked | 08:36 |
tristan | Right, when you declare your option you can export it as a named variable, and you can feed that named variable as an option value to the junction | 08:37 |
Kinnison | So includes in project.conf *must* be project-local ? | 08:37 |
juergbi | no, I don't think so | 08:37 |
tristan | There *should* be errors if you include something which will be ignored/discarded | 08:38 |
tristan | not sure that there are off hand | 08:38 |
Kinnison | Are there cases where you would want to include something into project.conf from across a junction? | 08:38 |
juergbi | yes, e.g., something like strip rules | 08:39 |
tristan | Kinnison, the original (main) motivation for developing includes, is so that one project can inherit variables, env vars and shell configurations from subprojects | 08:39 |
juergbi | and similar variables | 08:39 |
tristan | of course there are other benefits | 08:39 |
tristan | Kinnison, A recommended setup is to have your project.conf include something like a "shell.yaml" file, and then another project which uses your project can *also* have a shell.yaml which extends your shell.yaml via an include | 08:40 |
* Kinnison continues to fear then, since variables can be used to parameterise options which end up being usable to conditionally include / configure other things, which might change the original variable, which changes what gets included/conditionally processed, which changes the variables, which ... no? | 08:41 | |
tristan | for example, and can do the same for different topics | 08:41 |
tristan | Kinnison, hence the dual pass load | 08:42 |
Kinnison | two passes may be insufficient to settle (if it's astable it'll never settle) | 08:42 |
Kinnison | Or do you mean something different by two passes than I'm thinking? | 08:42 |
juergbi | Kinnison: the loader always uses the options from the first pass for junctions | 08:42 |
juergbi | so it should be stable, even though it might disallow some crazy include | 08:43 |
tristan | https://docs.buildstream.build/format_intro.html#include | 08:43 |
tristan | Kinnison, the above explains more about includes and junctions | 08:43 |
juergbi | https://gitlab.com/BuildStream/buildstream/blob/master/buildstream/_loader/loader.py#L235 | 08:43 |
Kinnison | aha, I must have missed that the first time I read through the intro | 08:43 |
Kinnison | so... when loading a junction element, no matter what, you use the project config with includes of junctioned stuff entirely ignored? | 08:44 |
Kinnison | Or is that only true until project.conf is fully loaded? | 08:44 |
juergbi | as per the linked code, it will always use the options from the first pass | 08:45 |
Kinnison | variables, element overrides, source overrides, options, and mirrors from the first pass, or just options? | 08:46 |
Kinnison | So there's always two project configurations held around - one first pass, one fully resolved ? | 08:46 |
tristan | Nothing from across a junction can affect the loading of a junction | 08:46 |
tristan | I don't expect the first pass to be around later on | 08:47 |
Kinnison | OK | 08:47 |
Kinnison | "later on" being for example when loading an element which refers to another through a junction? Or being after the loader completes? | 08:47 |
tristan | After the loader completes we should be free of the first pass, otherwise I guess it would be unnecessarily allocated memory | 08:48 |
Kinnison | OK | 08:49 |
Kinnison | But first pass is held throughout the entire load, even after project.conf is fully resolved. I see, thanks for the clarity there | 08:49 |
*** tristan has quit IRC | 08:53 | |
*** benschubert has joined #buildstream | 08:57 | |
*** tristan has joined #buildstream | 09:02 | |
*** ChanServ sets mode: +o tristan | 09:02 | |
gitlab-br-bot | jjardon closed issue #737 (Buildstream local cache doesn't get cleanup automatically when the disk is full) on buildstream https://gitlab.com/BuildStream/buildstream/issues/737 | 09:11 |
* Kinnison attempts to grok the logic at line 85 of _includes.py | 09:12 | |
tristan | Kinnison, The includes is a list... and each include file overrides the previous include dictionary in the list... *after* all of that composition is done, the *including* dictionary is composited on top of what was included, the result replaces the including dictionary | 09:16 |
tristan | Kinnison, it's complicated because with includes, composition should happen below the including node :) | 09:16 |
* Kinnison reads it as: | 09:16 | |
Kinnison | 1. take base_node and extract includes from it | 09:17 |
tristan | or in other words, the including dictionary keys override what is included | 09:17 |
*** ikerperez has joined #buildstream | 09:17 | |
Kinnison | 2. for each include, in reverse order, composite base_node on top of the included dictionary, then replace the base_node with the content of the resultant composition. | 09:17 |
tristan | The code flow might might run that way, I'm just explaining the why behind it, and the desired behavior | 09:18 |
Kinnison | I understand the desired behaviour | 09:18 |
Kinnison | I'm attempting to see how it matches | 09:18 |
Kinnison | :D | 09:18 |
Kinnison | including the to_delete stuff etc | 09:18 |
Kinnison | I *think* I'm slowly grokking this machinery | 09:18 |
*** flatmush has quit IRC | 09:21 | |
*** flatmush has joined #buildstream | 09:21 | |
Kinnison | So this means that, for example, (=): - whatever in the base_node, will only be verified against the last of the included file dictionaries? | 09:22 |
tristan | An (=) overwrites a list, so if various fragments in the include list overwrite the same list, then the last one wins, but the including fragment still has precedence over that node | 09:28 |
tristan | over the list | 09:28 |
tristan | ... of included fragments | 09:28 |
tristan | again though, I am not going from how the code works but how it is supposed to | 09:30 |
tristan | (sorry) | 09:30 |
Kinnison | No, I think we're talking at cross purposes | 09:30 |
Kinnison | (=) overwrites a list, yes | 09:30 |
Kinnison | but | 09:30 |
Kinnison | if the list comes in say the first include in the list | 09:30 |
Kinnison | and the second include doesn't have it | 09:30 |
Kinnison | then surely the composite will fail | 09:30 |
Kinnison | because we're not compositing the base node over the top of the composition of all the includes | 09:30 |
Kinnison | we're compositing in sequence | 09:30 |
Kinnison | from last include backward | 09:31 |
* Kinnison is still very clumsy with all this vocabulary so may be unclear :( | 09:31 | |
tristan | Kinnison, So I believe the reason for composition in sequence is because _yaml.composite() is what is going to handle these list composition directives | 09:34 |
Kinnison | Yes | 09:34 |
tristan | I'm not sure why it is reversed, but valentind would likely remember | 09:34 |
tristan | I think he is on holiday though | 09:34 |
Kinnison | reversed presumably because each time, the composited result replaces the base_node, and then we work backwards. Doing that means that the original base_node clobbers anything in the includes, and then each include clobbers those which were before it in the lst of includes | 09:35 |
Kinnison | that's pretty obvious to me | 09:35 |
Kinnison | *but* | 09:35 |
Kinnison | since we clobber with the base_node *first* on the *last* include, I fear that there might be some kind of confusion if we had a situation along the lines of: | 09:35 |
Kinnison | something: (=): [ stuff, and, things ] | 09:36 |
Kinnison | (@): [ include1, include2 ] | 09:36 |
Kinnison | Where | 09:36 |
Kinnison | include1 contains a something: [] | 09:36 |
Kinnison | and include2 does not | 09:36 |
*** jonathanmaw has joined #buildstream | 09:36 | |
Kinnison | as a user, I'd expect include1 and include 2 to be combined, and then for the base node to be applied | 09:38 |
Kinnison | but the code will try and composite include2, base_node | 09:38 |
Kinnison | and choke on the (=) | 09:38 |
Kinnison | no? | 09:38 |
*** jennis has joined #buildstream | 09:41 | |
*** phildawson has joined #buildstream | 09:49 | |
*** raoul has joined #buildstream | 09:56 | |
tristan | Kinnison, that is an interesting observation | 10:12 |
tristan | would seem to be a bug if it is the case | 10:12 |
tristan | should be easy enough to throw a test case together in tests/format/include.py | 10:13 |
*** sebastian has quit IRC | 10:14 | |
gitlab-br-bot | aevri opened issue #935 (Remote Execution: build command failures hard to diagnose (always 'Directory not found')) on buildstream https://gitlab.com/BuildStream/buildstream/issues/935 | 10:15 |
*** sebastian has joined #buildstream | 10:15 | |
*** sebastian has quit IRC | 10:21 | |
gitlab-br-bot | juergbi approved MR !1186 (mablanch/799-RE-optional-TLS->master: Optional TLS support for remote-execution storage service) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1186 | 10:28 |
gitlab-br-bot | juergbi closed issue #799 (Remote Execution: Make TLS and client certificate optional for storage service) on buildstream https://gitlab.com/BuildStream/buildstream/issues/799 | 10:28 |
gitlab-br-bot | juergbi merged MR !1186 (mablanch/799-RE-optional-TLS->master: Optional TLS support for remote-execution storage service) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1186 | 10:28 |
*** lachlan has joined #buildstream | 10:34 | |
adds68 | juergbi you'll make Marge angry :P | 10:35 |
juergbi | she was resting ;) | 10:35 |
tristan | adds68, I think that is the fun part, doesnt she have a dictionary of randomized insults to bark out when she feels abused ? | 10:36 |
adds68 | tristan haha! Someone should submit that as a feature | 10:36 |
Kinnison | Surely the answer is to just remove merge rights from anyone but marge? | 10:37 |
juergbi | like https://media0dk-a.akamaihd.net/48/89/c60a361f7436dfbb1c25cbec4c871dc2.jpg | 10:38 |
adds68 | juergbi haha! :) | 10:39 |
adds68 | Kinnison this is how the singularity begins... | 10:39 |
Kinnison | :+1: | 10:40 |
*** lachlan has quit IRC | 10:41 | |
tristan | jonathanmaw, Ummm, you know about the WSL runner apparently ? | 10:55 |
tristan | (or so I'm told) | 10:55 |
jonathanmaw | tristan: yes | 10:55 |
tristan | jonathanmaw, Care to just check this comment and confirm whether my suspicions are fact ? | 10:56 |
tristan | https://gitlab.com/BuildStream/buildstream/merge_requests/1189#note_145809596 | 10:56 |
tristan | if so, maybe it is not easy to fix, but probably worth tracking in an issue (maybe tagged with the infrastructure tag) | 10:57 |
benschubert | tristan: my understanding is, the WSL runner runs in WSL, not in docker, so anything created outside the test directory will live on | 10:57 |
benschubert | whereas all other runners are docker's, so we get a fresh runner everytime | 10:57 |
tristan | benschubert, sure, I don't know the mechanics, my understanding is also that WSL is a subsystem which can probably be handed a fresh presine runtime every time you launch it | 10:58 |
tristan | Ever since I heard of WSL, I have been hoping for a Sandbox implementation which uses it to run commands on our provided/staged runtime | 10:58 |
tristan | But maybe not a worthwhile effort :) | 10:59 |
tristan | In any case; if it is the case that the runner is just a persistent thing... then we are missing the part which reproduces an acceptable runner | 11:01 |
tristan | (not to mention it is error prone to persist state across pipelines, but that might be mitigated somewhat by running as a regular user inside WSL and not as root) | 11:02 |
*** lachlan has joined #buildstream | 11:08 | |
tristan | jonathanmaw, Thanks for the comment :) | 11:08 |
tristan | jonathanmaw, In the meantime, are you able to wrangle the runner manually somehow ? I don't know how it is *supposed* to be upgraded | 11:09 |
tristan | jonathanmaw, it could be good to undo the mess I've created and have the WSL image already have a recent version of `tox` in it's dist-packages | 11:10 |
jonathanmaw | tristan: I have custody over the machine it's running on, but the runner itself is a VM I creted with https://gitlab.com/jonathanmaw/wsl-setup-script | 11:10 |
jonathanmaw | *created | 11:10 |
tristan | Ah that's good to know that we already have automation to create it | 11:10 |
tristan | It would be good to tie that automation into the project somehow | 11:10 |
tristan | But I suppose that would be part of a larger effort of documenting how to setup gitlab CI for BuildStream | 11:11 |
juergbi | tpollard: does the test update in !1184 look good to you now? | 11:15 |
gitlab-br-bot | MR !1184: Use virtual artifact directory to stage and extract metadata https://gitlab.com/BuildStream/buildstream/merge_requests/1184 | 11:15 |
*** tristan has quit IRC | 11:16 | |
*** lachlan has quit IRC | 11:17 | |
tpollard | juergbi: 1 sec | 11:18 |
gitlab-br-bot | tpollard approved MR !1184 (juerg/virtual-artifact-directory->master: Use virtual artifact directory to stage and extract metadata) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1184 | 11:22 |
juergbi | ta | 11:22 |
*** tristan has joined #buildstream | 11:33 | |
*** jmac has joined #buildstream | 11:35 | |
jmac | Sorry, my shell box got rebooted overnight. Is there anyone around who can help me run the automated tests on bst-external? | 11:35 |
juergbi | jonathanmaw, jennis: ^^ | 11:39 |
jennis | jmac, having a look now | 11:44 |
jennis | ./setup.py test runs the test suite (there is no tox yet), but it looks like this is failing with bst master | 11:44 |
jmac | Yep. I'm using the current master of bst-external and buildstream and it's complaining about obsolete commands | 11:44 |
jennis | Yes, I am seeing this too | 11:45 |
jmac | But the CI appears to be working; does it use a fixed version of buildstream? | 11:45 |
jennis | ahh, it's using the buildstream/buildstream-fedora image | 11:46 |
jennis | Which is being deprecated IIRC | 11:47 |
* jmac notes that this is a problem that can only happen because the useful bits of buildstream are split across two git repositories | 11:47 | |
jennis | Ok, I guess this should be using an image with buildstream master installed | 11:48 |
jennis | cs-shadow, am I right to believe that buildstream/buildstream:dev should be a nightly build of master? | 11:49 |
gitlab-br-bot | marge-bot123 merged MR !1184 (juerg/virtual-artifact-directory->master: Use virtual artifact directory to stage and extract metadata) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1184 | 11:49 |
tpollard | woop | 11:50 |
jennis | jmac, locally you're testing against your system install of BuildStream, this is failing because something is now obsolete, have you managed to fix that bit...? | 11:51 |
jmac | No. Is there a particular version of buildstream I should revert to? | 11:52 |
jennis | Well, I think we need to get this working with BuildStream master, correct? And have the CI run in a container that is also doing this | 11:53 |
jennis | Currently, both those things don't happen | 11:53 |
jennis | Do you agree? | 11:53 |
jmac | Yes, I would agree with that, but that's not my immediate goal. I'd like to use the tests to test an MR I have for a different bug. | 11:56 |
jennis | Ok, well it looks like until I get this fixed (trying now) the tests are only going to pass on an older version of buildstream | 11:57 |
jennis | 4d92c106, I think | 11:58 |
jennis | From looking at the image defined at the top of .gitlab-ci.yml | 11:58 |
jmac | Ta | 11:59 |
*** alatiera has joined #buildstream | 12:01 | |
*** raoul has quit IRC | 12:02 | |
*** raoul has joined #buildstream | 12:12 | |
*** lachlan has joined #buildstream | 12:15 | |
*** lachlan has quit IRC | 12:29 | |
*** raoul has quit IRC | 12:32 | |
cs-shadow | jennis: no, it'll be the development snapshot, i.e. 1.3.0 at the moment | 12:33 |
cs-shadow | `:nightly` is the tag for nightly | 12:33 |
jennis | Oh, that makes sense | 12:34 |
cs-shadow | "Image Variants" section of https://hub.docker.com/r/buildstream/buildstream talks a bit about it | 12:34 |
jennis | cs-shadow, my work around was to using test-suite, cloning and then install BuildStream >.< | 12:34 |
jennis | Yeah, just looking at that now | 12:34 |
jennis | thanks | 12:34 |
jennis | Many broken links, I'll put in an MR | 12:35 |
cs-shadow | oh really, where? | 12:35 |
jennis | cs-shadow In the README of buildstream-docker-images | 12:42 |
cs-shadow | ah, right! I can see them now :| | 12:44 |
* cs-shadow wonders if there's an easy way to run a lint job that'll check for broken links | 12:44 | |
*** lachlan has joined #buildstream | 12:47 | |
jennis | good shout, I'd imagine so | 12:49 |
jennis | cs-shadow, I've put up an MR, if you have time could you respond to https://gitlab.com/BuildStream/buildstream-docker-images/merge_requests/112/diffs#note_145970347 ? | 12:51 |
*** lachlan has quit IRC | 12:53 | |
cs-shadow | jennis: thanks! I've provided the correct URL as a reply | 12:54 |
jennis | cs-shadow, whoops forgot to update one of the links | 12:57 |
jennis | sex | 12:57 |
jennis | sec* | 12:57 |
jennis | ! | 12:57 |
jennis | cs-shadow, should be good now | 12:59 |
cs-shadow | jennis: seems all good now, thank you very much] | 13:02 |
jennis | You're welcome | 13:02 |
tpollard | juergbi: is there any more MR's to get landed before the abstract WIP can be pointed back to master? | 13:04 |
juergbi | tpollard: no. I have a couple more in flight and there might be some tiny conflicts, but nothing substantial | 13:06 |
tpollard | juergbi: that's the impression I had, thanks! | 13:10 |
gitlab-br-bot | cs-shadow approved MR !1189 (tristan/optional-coverage->master: Make coverage optional) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1189 | 13:18 |
gitlab-br-bot | juergbi closed issue #920 (Use virtual directories for staging dependencies) on buildstream https://gitlab.com/BuildStream/buildstream/issues/920 | 13:28 |
gitlab-br-bot | juergbi closed issue #913 (Extracting buildtree/specific subdirs from the cache should be done directly) on buildstream https://gitlab.com/BuildStream/buildstream/issues/913 | 13:29 |
gitlab-br-bot | juergbi closed issue #921 (Drop extract directories) on buildstream https://gitlab.com/BuildStream/buildstream/issues/921 | 13:30 |
*** nimish has joined #buildstream | 13:39 | |
*** raoul has joined #buildstream | 13:48 | |
gitlab-br-bot | aevri opened MR !1193 (aevri/dyn_minimum_cache_msg->master: cascache: don't hardcode 2G warning string) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1193 | 13:48 |
gitlab-br-bot | juergbi opened (was WIP) MR !1188 (juerg/lazy-directory-digest->master: _casbaseddirectory.py: Calculate directory digest lazily) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1188 | 13:48 |
*** phildawson has quit IRC | 13:58 | |
*** phildawson has joined #buildstream | 13:58 | |
*** lachlan has joined #buildstream | 14:11 | |
*** phildawson has quit IRC | 14:20 | |
*** lachlan has quit IRC | 14:26 | |
*** phildawson has joined #buildstream | 14:29 | |
*** lachlan has joined #buildstream | 14:31 | |
juergbi | aevri: haha, talking to Marge now? | 14:32 |
juergbi | !1188 is now ready for review for anyone familiar with or interested in CasBasedDirectory optimizations | 14:35 |
*** lachlan has quit IRC | 14:35 | |
cs-shadow | aevri: missed opportunity to reference https://www.xkcd.com/149/ :) | 14:40 |
gitlab-br-bot | marge-bot123 merged MR !1193 (aevri/dyn_minimum_cache_msg->master: cascache: don't hardcode 2G warning string) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1193 | 14:45 |
jmac | Is _version.py something that was generated once by versioneer, or something that might be auto-generated again? I assume the first as it's under version control, but wanted to check | 14:47 |
cs-shadow | https://github.com/warner/python-versioneer#theory-of-operation might be useful | 14:49 |
jmac | Looks like we might overwrite it if we updated the version of versioneer | 14:52 |
*** lachlan has joined #buildstream | 14:54 | |
gitlab-br-bot | raoul.hidalgocharman opened (was WIP) MR !1124 (raoul/440-source-cache->master: Source cache) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1124 | 14:54 |
raoul | Think !1124 is good for review now juergbi, trying to figure out if I can split the commits up a bit more but content seems to be all working now | 14:55 |
juergbi | great, will take a look | 14:55 |
cs-shadow | jmac: yeah, anytime we do `versioneer install` and its format changes, it might get updated | 14:58 |
cs-shadow | although it's format will likely not change wildly in reality | 14:59 |
jennis | Our CI uses the buildstream/testsuite-* images, at some tag, do we manually update these tags when system deps change? | 15:24 |
cs-shadow | yep | 15:25 |
jennis | ahh, ok :/ | 15:25 |
aevri | juergbi: yup, trying to stay on her good side for the robot uprising. I, for one, welcome our new gitlab-bot overlords. | 15:29 |
jennis | cs-shadow, in the CI for bst-plugins-container, where is the image with bst installed / where do we even install bst? | 15:32 |
jennis | I'm failing to see how we can even run bst commands for those tests | 15:32 |
jennis | jmac, I've created a branch on bst-external which is using bst master 'jennis/cleanup_testsuite': https://gitlab.com/BuildStream/bst-external/commits/jennis/cleanup_testsuite | 15:34 |
jmac | Ah, super | 15:35 |
jennis | It's still failing CI due to some coverage stuff I'm unsure about, but it does work locally for me | 15:35 |
cs-shadow | Jennie: this image doesn’t have it. Check tox.ini | 15:35 |
cs-shadow | jennis: ^ | 15:35 |
jennis | cs-shadow, so when we run 'tox --whatever' in the container, BuildStream is installed then, in the tox environment? | 15:37 |
*** raoul has quit IRC | 15:56 | |
cs-shadow | jennis: exactly | 15:58 |
*** raoul has joined #buildstream | 16:03 | |
*** nimish has quit IRC | 16:14 | |
*** nimish has joined #buildstream | 16:18 | |
jennis | thanks cs-shadow | 16:25 |
*** paulsherwood has quit IRC | 16:27 | |
*** lachlan has quit IRC | 16:36 | |
*** lachlan has joined #buildstream | 16:37 | |
*** lachlan has quit IRC | 16:40 | |
*** lachlan has joined #buildstream | 16:44 | |
*** lachlan has quit IRC | 16:56 | |
*** lachlan has joined #buildstream | 16:57 | |
*** lachlan has quit IRC | 17:01 | |
*** lachlan has joined #buildstream | 17:03 | |
*** lachlan has quit IRC | 17:11 | |
*** lachlan has joined #buildstream | 17:26 | |
*** lachlan has quit IRC | 17:30 | |
*** lachlan has joined #buildstream | 17:44 | |
gitlab-br-bot | raoul.hidalgocharman approved MR !1188 (juerg/lazy-directory-digest->master: _casbaseddirectory.py: Calculate directory digest lazily) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1188 | 17:47 |
jennis | tristan, Kinnison, I may have missed it but looking at tests/internals/yaml.py and the various yaml files in tests/internals/yaml/*, it doesn't look like we actually test trying to overwrite a node that doesn't exist in the target. For the test cases in the file I just mentioned, I think we're always overwriting a 'children' key | 18:00 |
jennis | I've pushed a branch that adds a test for this here: https://gitlab.com/BuildStream/buildstream/commit/c48f7c1d1f87e75c1b9c1ef83b7e2b19967734a9 | 18:00 |
jennis | This test is failing because we *are* managing to overwrite 'test' which doesn't exist in b.yml | 18:01 |
gitlab-br-bot | tpollard opened (was WIP) MR !1175 (tpollard/908->master: Artifact 'abstraction' class) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1175 | 18:02 |
*** jonathanmaw has quit IRC | 18:17 | |
*** lachlan has quit IRC | 18:22 | |
*** raoul has quit IRC | 18:24 | |
gitlab-br-bot | BenjaminSchubert opened issue #936 (Tests fail if host tools are not in a standard directory) on buildstream https://gitlab.com/BuildStream/buildstream/issues/936 | 18:28 |
*** alatiera has quit IRC | 18:43 | |
gitlab-br-bot | BenjaminSchubert opened MR !1194 (bschubert/fix-test-paths->master: store full host tools command in sites.py variables check.) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1194 | 18:51 |
gitlab-br-bot | cs-shadow approved MR !1194 (bschubert/fix-test-paths->master: store full host tools command in sites.py variables check.) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1194 | 18:57 |
*** nimish has quit IRC | 19:19 | |
*** nimish has joined #buildstream | 19:20 | |
*** ChanServ sets mode: +o tristan | 19:25 | |
tristan | jennis, Thanks for creating a test case ! it would be good to make it work in a standalone way (without /home/jennis/banana.yml ?) and file an issue/mr with the failing test | 19:25 |
tristan | jennis, that is probably already 90% of fixing it :) | 19:25 |
tristan | jmac, I would strongly caution against porting bst-external to depend on bst master in advance of clarifying how to install bst-external for 1.2 for the projects who are actually using bst-external with 1.2 | 19:28 |
tristan | or ensuring that downstream projects which use bst-external with bst 1.2 dont accidentally get bst-external for master and then break | 19:28 |
juergbi | I think we should find a way for external plugins to support both bst 1.2 and master | 19:29 |
juergbi | especially important for project-local plugins but also sounds useful for other external plugins, imo | 19:29 |
tristan | Certainly there should be, but I think that for now we are distributing with pip, and so we could at least go for a pip production vs pip "pre" release model | 19:30 |
juergbi | that doesn't help for project-local plugins | 19:31 |
tristan | if people are cloning the repo and `pip install .` (which I think this is the case), then I guess for now all we can do is attempt to communicate it well | 19:31 |
cs-shadow | juergbi: sorry if i'm bit out of context, but what's preventing plugins from supporting both versions? | 19:31 |
tristan | for project local plugins you don't have that problem do you ? | 19:31 |
tristan | I mean if your project uses bst 1.2, then the local plugins you have in the project are for bst 1.2 | 19:31 |
juergbi | cs-shadow: nothing really, they could use something like `hasattr()` and similar I suppose | 19:31 |
tristan | cs-shadow, lack of guaranteed API stability is preventing that | 19:32 |
juergbi | tristan: well, my goal would be to keep e.g., fdo-sdk supporting both | 19:32 |
tristan | but I suppose ugly hacks could be employed instead of sane versioning :-S | 19:32 |
tristan | juergbi, That sounds awesome but I am not that hopeful especially after going ahead with plugin migration plans | 19:32 |
tristan | I expect that until there is a 2.0 stable, there will be a set of patches to apply to freedesktop-sdk for the sake of running CI of patched freedesktop-sdk against bst master | 19:33 |
tristan | once bst diverges to a point where it can not really make sense | 19:33 |
tristan | (maybe for now it can ?) | 19:34 |
juergbi | freedesktop-sdk could switch to the external plugin even while they still support 1.2 as well | 19:34 |
juergbi | overriding bst 1.2 core plugins | 19:34 |
juergbi | as long as the external plugin stays compatible with 1.2 | 19:34 |
juergbi | I think there is a big benefit in allowing projects to be compatible with both | 19:34 |
tristan | I am assuming that we don't really switch without significantly modifying the project.conf format for obtaining plugins in a more reliable way than pip | 19:35 |
tristan | but that is still up in the air I guess | 19:35 |
juergbi | true, that is a potential issue | 19:35 |
tristan | Yeah, I agree it would be nice, but I think that that dream is mostly incompatible with a lack of guaranteed API stability | 19:35 |
tristan | It would *really* be nice though :) | 19:35 |
juergbi | with Python it should really be possible to handle this | 19:36 |
juergbi | I think it's worth the maintenance overhead for external plugins | 19:36 |
juergbi | and it might not be more painful overall than maintaining two branches | 19:36 |
tristan | With that I agree | 19:36 |
tristan | I very dislike the idea of having version conditional python in the plugins themselves (there is a huge risk that people follow the pattern of the plugins we maintain, causing the whole thing to smell really bad), but suppose that such dual support can be safely phased out post 2.0 | 19:38 |
juergbi | we could potentially extract certain helper classes from bst master to avoid too many conditionals | 19:39 |
juergbi | e.g., to use the virtual directory API (only with filesystem backend) | 19:40 |
cs-shadow | I am not sure if I agree that external plugins should continue to support bst-1.2 with their new versions. BuildStream Core plugins are themselves not getting any updates, I am not sure why we want external plugins to behave differently | 19:40 |
tristan | Or, sanely split the code (or even duplicate it within the plugin), such that a single plugin.py file has 2 plugin implementations and conditionally returns the plugin of choice from the initializer | 19:40 |
juergbi | cs-shadow: my main concern is how to support bst 1.2 and master with a single project branch | 19:41 |
tristan | cs-shadow, core plugins certainly have been receiving master specific updates especially for virtual directories | 19:41 |
juergbi | I assume he means on 1.2 | 19:41 |
cs-shadow | yeah i meant on the 1.2 branch | 19:41 |
tristan | Oh yeah | 19:41 |
tristan | yeah we're talking about supporting 1.2 & master | 19:42 |
juergbi | cs-shadow: if project.conf uses pip for external plugins, the same branch has to work for 1.2 and master | 19:42 |
juergbi | unless I'm missing something here | 19:42 |
juergbi | maybe conditional in project.conf, hmm | 19:42 |
tristan | currently project.conf does not have any way to impose a specific version of a pip installed plugin package | 19:43 |
tristan | We go on the assumption that plugin packages are API stable | 19:43 |
cs-shadow | juergbi: I'd have imagined that therre's a requirements.txt or similar in the project root that can achieve this | 19:43 |
tristan | So we would need a different package name I guess for both 1.2 vs master | 19:43 |
juergbi | that could be an option, I suppose, instead of installing via project.conf pip | 19:44 |
cs-shadow | buildstream itself will still not be enforcing conditionals etc. but it's concievable to have bst1.2-reqs.txt and bst-mater-reqs.txt | 19:45 |
cs-shadow | that's one option anyway | 19:45 |
tristan | cs-shadow, we don't impose any specific way of using BuildStream, and honestly if BuildStream allows a setup where you can break then it is our fault, not the users for not using a special venv or smth to run BuildStream from | 19:45 |
tristan | I mean for CI sure it is something that we could "do", but that is not a good statement for the user who git clones freedesktop-sdk and runs bst 1.2 on it and rightfully expects things to just work | 19:46 |
tristan | I believe currently the instructions say to "install bst-external", and that means getting the git repo at master and running `pip3 install .` from there | 19:47 |
cs-shadow | well, assuming this is only about the transition period between now and sometime after 2.0, we can follow the policy of only releasing pre-release versions for external plugins | 19:49 |
cs-shadow | that would ensure the `pip install bst-external` gives a version that works with bst-1.2 | 19:49 |
cs-shadow | and people who want to try master, can install the pre-release versions | 19:49 |
tristan | I was thinking that would be a convenient way yeah, or juergbi was suggesting to have those plugins support both versions for the transition period | 19:49 |
tristan | cs-shadow, *however* | 19:50 |
tristan | bst-external is not parallel installable | 19:50 |
tristan | So it will be a pain in the ass to go switch from bst 1.2 to master and back at will | 19:50 |
tristan | While trying out master and reverting to 1.2 at times if things break | 19:51 |
juergbi | indeed, adding a big hurdle for users trying out 1.90 | 19:51 |
tristan | (i.e. switching means you need to change your bst-external installation, unless we have bst-external-2 for bst master instead) | 19:51 |
cs-shadow | how's that different from installing a different bst version? | 19:52 |
tristan | in any case, 1.90/2.0 is a "new api", so using a different namespace is the appropriate thing (but supporting both versions from bst-external as juergbi suggested is also feasible) | 19:52 |
cs-shadow | i am not sure about the new namespace unless we also plan to have bst-2 as well? | 19:53 |
tristan | cs-shadow, First of all, even if bst 1.90 is not parallel installable with 1.2, it is still a *huge* difference | 19:53 |
SotK | Is there a reason for not just installing the new (or old) bst-external in a virtualenv? | 19:54 |
tristan | i.e. the difference between "I want to try master, so I checkout master and install", vs. "I want to try master, so I checkout master and install, and things break horribly, because I had no idea I had to go reach into this separate repository and ALSO reinstall that one" | 19:54 |
tristan | SotK, That is not something we can impose on users | 19:54 |
tristan | SotK, i.e. it's something that we could "do" in CI for projects we control, but most importantly is the first impressions we make on random users who follow sensible rules, and expect things to "just work" | 19:55 |
tristan | For CI of builds of freedesktop-sdk it is not a problem at all, there is only one or two CI's to maintain - but we dont want to spend a year frustrating random users and evaporating all interest in using a tool that only causes frustration | 19:56 |
gitlab-br-bot | BenjaminSchubert opened MR !1195 (remove-dead-code->master: Remove dead code) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1195 | 19:56 |
tristan | Honestly, as long as the default for regular users is to obtain 1.2 compatible plugins from bst-external until there is a 2.0, then that is also rather fine | 19:57 |
tristan | And we can do fancy venv stuff from CI just to ensure that master is staying relatively compatible | 19:57 |
tristan | Then only users who dare to try out the new 2.0 stuff end up with some hassle (but it would be damn *nice* if users could easily try out master in the meantime too) | 19:58 |
benschubert | tristan: plugins and buildstream are installable through pip. We can expect users installing normally to get 1.2 and compatible external plugins and users installing with --prerelease to get 1.9 versions. And people who want master are on their own because that's how it works. Hence we don't need any compatibility between versions. | 20:31 |
benschubert | Otherwise we would need to guarantee many other things to be coherent | 20:31 |
benschubert | Like cache keys | 20:31 |
benschubert | And plugins can restrict their buildstream version compatibility, as is the case for Django/flask/pytest, etx | 20:32 |
tristan | benschubert, Plugins can ("sort of") assert the version if they cheat and expect to be in a pip/venv controlled environment, but it is rather the project.conf which needs to express what plugins they intend to use, without namespacing plugin packages then a project.conf's declaration of what plugins to use is ambiguous | 21:33 |
tristan | benschubert, using --prerelease does not give you parallel installability either in the sense that whatever you happened to install will be what is used by the project.conf lookup | 21:34 |
*** slaf has quit IRC | 22:07 | |
*** tristan has quit IRC | 22:24 | |
*** slaf has joined #buildstream | 22:34 | |
*** slaf has joined #buildstream | 22:34 | |
*** slaf has joined #buildstream | 22:35 | |
*** slaf has joined #buildstream | 22:35 | |
*** slaf has joined #buildstream | 22:35 | |
*** slaf has joined #buildstream | 22:35 | |
*** slaf has joined #buildstream | 22:36 | |
*** slaf has joined #buildstream | 22:36 | |
*** slaf has joined #buildstream | 22:36 | |
*** slaf has joined #buildstream | 22:36 | |
*** slaf has joined #buildstream | 22:37 | |
*** slaf has joined #buildstream | 22:37 | |
*** benschubert has quit IRC | 22:57 | |
*** slaf has quit IRC | 23:03 | |
*** slaf has joined #buildstream | 23:03 | |
*** slaf has joined #buildstream | 23:03 | |
*** slaf has joined #buildstream | 23:03 | |
*** slaf has joined #buildstream | 23:04 | |
*** slaf has joined #buildstream | 23:04 | |
*** slaf has joined #buildstream | 23:04 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!