IRC logs for #buildstream for Thursday, 2019-02-28

*** nimish has joined #buildstream01:14
*** mohan43u has quit IRC02:44
*** alatiera has quit IRC02:55
*** kapil___ has quit IRC03:02
*** nimish has quit IRC03:18
*** mohan43u has joined #buildstream04:13
*** mohan43u has quit IRC04:39
*** mohan43u has joined #buildstream04:54
gitlab-br-botjuergbi 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/118405:05
*** tristan has joined #buildstream05:40
*** ChanServ sets mode: +o tristan05:40
gitlab-br-botjuergbi 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/119206:04
tristanAppears that we don't collect coverage reports from the wsl tests :-S06:41
tristanI guess the fact that the test is allowed to fail complicates this06:42
juergbitristan: for fedora-update-deps we do collect coverage, though, even though that's also allowed to fail06:45
juergbiif I'm reading it correctly06:45
juergbican we just do the same for WSL or is it more complicated?06:46
tristanjuergbi, we might be *generating* coverage for that one, but I don't think we're collecting/collating it06:47
tristani.e. it needs to be explicitly listed in the dependencies of the coverage job afaiu06:47
juergbiit already is06:48
tristanSo if we have a job which depends on an artifact that is conditionally created... the job will still run ?06:48
tristanThat is nice06:48
juergbiupdate-deps doesn't fail frequently but I think I've seen it fail before without failing the pipeline06:48
juergbimight misremember that, though06:48
tristanjuergbi, I expect it to be more complicated, only inasmuch as we probably need to adjust .coveragerc to list win32 path patterns for collation of results06:49
juergbihm, we're not running on Win3206:49
juergbithe runner runs inside WSL06:49
juergbii.e., it's not a Windows runner that starts WSL06:50
tristanbut you are right about the update-deps thing06:50
tristanit is allow_failure and collected in coverage06:50
tristanAh I see, so since the runner itself runs inside WSL it should have pretty much the same behaviors as on linux, pathwise at least06:51
juergbiyes, it's not based on a Docker base image, but other than that it should be fairly similar in behavior06:52
juergbijonathanmaw will know the details06:52
tristangrrr, so there is something weird about that image :-S06:53
tristanhttps://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 issue06:56
juergbipossible06:58
tristanYup, it was06:58
tristanI am curious though, I have a sneaking suspicion that I messed things up, or fixed them in a bad way06:59
tristanAre we at least guaranteed that the runner state is clean on every run ?07:00
tristanI.e. no side effects will be carried over from run to run except for those stored in the cache ?07:00
tristanRight, 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 test07:07
tristanA 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 pipelines07:08
juergbiwith my latest (unpushed) branch I can run the whole test suite in 250s :)07:36
juergbi(down from about 400s)07:36
tristanjuergbi, How did you manage that ?07:42
juergbitristan: switching to CasBasedDirectory for `import` elements07:43
tristanAhh07:43
juergbithey don't need any sandbox07:43
tristanHah07:43
juergbiand I've optimized CasBasedDirectory a lot this week07:43
tristanYeah I've been watching the branches land07:44
tristan(someones been shaking the tree heh)07:44
juergbiyes, got staging time down by 40% for local sandboxes, and down by >80% for buildbox / remote execution07:45
juergbinow the next bottleneck for the latter (virtual staging) is the overhead of filling result.files_written, which is only needed for overlap warnings07:46
juergbi(to figure out the conflicting element, iirc)07:46
tristanOnce we solve the problem of how to decide on which elements to cache build trees for, master will be very attractive !07:46
juergbioverlap 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
juergbibuild trees: well, we already did land an option for this, the question is mainly whether the default should be changed07:47
tristanIs it ?07:48
juergbiyes, 'cache-buildtrees'07:48
tristanYeah but, are failed builds unconditional ?07:48
juergbiit supports 'cache-buildtrees: failure'07:48
juergbiwhich I use and I think would be a reasonable default07:49
juergbiit also supports 'cache-buildtrees: never' but I don't think this is really useful07:49
tristanI'm not sure that's an API we want long term honestly07:49
juergbiI agree. the tristate was chosen matching pull-buildtrees config07:49
juergbibut we could remove support for that 'never' state07:49
tristanmaybe "when useful" is more interesting than having the user understand when useful is, and explicitly saying "failure"07:49
juergbithat's an odd config value name ;)07:50
tristanI.e. I am already seeing recent issue reports about incremental workspace builds with remote execution07:50
tristanjuergbi, yeah, I'm not really serious about the name, just think that this is information a user shouldnt have to know07:50
juergbiwell, I'd be in favor of making 'failure' the default07:51
tristanas long as those edge cases grow, those edge cases should be a part of the default07:51
juergbiin 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 it07:51
tristanwithout changing the default, I think.07:51
juergbiI don't understand what you mean with that07:51
tristanI mean a setting name which means "cache build trees in the edge cases where I am likely to need them"07:52
tristanSo that when those edge cases increase, the default setting doesnt change; only it covers more cases07:52
juergbii.e., you're expecting further edge cases than 'failure' where we want to cache them by default07:52
tristanHow do you do incremental workspace builds, especially with remote execution, without caching the build trees of previous runs ?07:53
juergbiright, that's required07:53
tristanI also expect we will need it for tests, even though the world is in denial07:53
juergbifor some tests / test setups, it will likely be required07:54
tristanif 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
juergbibut it's definitely possible to run tests in different ways07:54
tristan(like .INTERMEDIATE: in make)07:54
juergbimaybe we should simply call it 'cache-buildtrees: default'07:55
juergbior auto07:55
tristanI 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
juergbiof course07:55
tristanjuergbi, that is kind of my point yeah07:55
tristanauto would be a good word07:56
juergbiinstalled tests are the other option, but that's not commonly supported07:56
tristanI don't think they are mutually exclusive anyway07:56
juergbino07:57
tristanRunning installed tests is also nice to do, of course07:57
juergbitristan: does this mean you'd be happy with a MR renaming 'failure' to 'auto', dropping 'never', and making 'auto' the new default07:57
tristanjuergbi, *I* would definitely yes07:57
tristanI don't really expect objections to that :)07:58
juergbigreat, let's do that :)07:58
*** paulsherwood has joined #buildstream08:16
KinnisonIs project.conf allowed to (@) through a junction element?08:18
*** kapil___ has joined #buildstream08:20
tristanKinnison, Yes it should08:22
tristanKinnison, iirc there is just some fields which cannot be derived from cross junction includes in project.conf08:23
juergbiyes, we have a two stage project load08:23
tristanprobably fields which one would not be interested in cross-junction-include defining anyway08: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
tristanIs there a link/url for the remote execution API itself ?08:26
tristannot necessarily terse documentation (preferably a project website if there is one, for a blog post)08:27
juergbihttps://github.com/bazelbuild/remote-apis/08:27
juergbithat's as close as it gets08:27
tristanAh that is pretty good though08:27
juergbiyes, but I think the API design document is still in Google Docs and not linked08:27
juergbibut at least the .protos with API documentation are maintained there08:28
juergbion github08:28
tristanSure, I was more looking for an introductory "what this is" page anyway :)08:28
tristanand this has a README and seems right08:28
KinnisonCan options be included across junctions?08:31
tristanNo08:34
KinnisonSo if I junction a project in, I have to redeclare all of its options in my project.conf ?08:34
tristanNot either08:34
KinnisonOr are they namespaced so I can set foojunction.bst:someoption to somevalue ?08:34
juergbiyou can set subproject options in the junction itself08:34
juergbihttps://docs.buildstream.build/elements/junction.html#options08:35
tristanright, 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
juergbior maybe better https://docs.buildstream.build/elements/junction.html#overview08:35
tristanBut you are by no means obligated to support all of the options a subproject exposes08:35
juergbiif you want the same options in your projects, you indeed have to redeclare them, though08:36
juergbiafaik08:36
KinnisonAah, 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 phews08:36
juergbiyes08:36
KinnisonOK good08:36
Kinnisonwhen 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 panicked08:36
tristanRight, 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 junction08:37
KinnisonSo includes in project.conf *must* be project-local ?08:37
juergbino, I don't think so08:37
tristanThere *should* be errors if you include something which will be ignored/discarded08:38
tristannot sure that there are off hand08:38
KinnisonAre there cases where you would want to include something into project.conf from across a junction?08:38
juergbiyes, e.g., something like strip rules08:39
tristanKinnison, the original (main) motivation for developing includes, is so that one project can inherit variables, env vars and shell configurations from subprojects08:39
juergbiand similar variables08:39
tristanof course there are other benefits08:39
tristanKinnison, 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 include08: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
tristanfor example, and can do the same for different topics08:41
tristanKinnison, hence the dual pass load08:42
Kinnisontwo passes may be insufficient to settle (if it's astable it'll never settle)08:42
KinnisonOr do you mean something different by two passes than I'm thinking?08:42
juergbiKinnison: the loader always uses the options from the first pass for junctions08:42
juergbiso it should be stable, even though it might disallow some crazy include08:43
tristanhttps://docs.buildstream.build/format_intro.html#include08:43
tristanKinnison, the above explains more about includes and junctions08:43
juergbihttps://gitlab.com/BuildStream/buildstream/blob/master/buildstream/_loader/loader.py#L23508:43
Kinnisonaha, I must have missed that the first time I read through the intro08:43
Kinnisonso... when loading a junction element, no matter what, you use the project config with includes of junctioned stuff entirely ignored?08:44
KinnisonOr is that only true until project.conf is fully loaded?08:44
juergbias per the linked code, it will always use the options from the first pass08:45
Kinnisonvariables, element overrides, source overrides, options, and mirrors from the first pass, or just options?08:46
KinnisonSo there's always two project configurations held around - one first pass, one fully resolved ?08:46
tristanNothing from across a junction can affect the loading of a junction08:46
tristanI don't expect the first pass to be around later on08:47
KinnisonOK08: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
tristanAfter the loader completes we should be free of the first pass, otherwise I guess it would be unnecessarily allocated memory08:48
KinnisonOK08:49
KinnisonBut first pass is held throughout the entire load, even after project.conf is fully resolved.  I see, thanks for the clarity there08:49
*** tristan has quit IRC08:53
*** benschubert has joined #buildstream08:57
*** tristan has joined #buildstream09:02
*** ChanServ sets mode: +o tristan09:02
gitlab-br-botjjardon closed issue #737 (Buildstream local cache doesn't get cleanup automatically when the disk is full) on buildstream https://gitlab.com/BuildStream/buildstream/issues/73709:11
* Kinnison attempts to grok the logic at line 85 of _includes.py09:12
tristanKinnison, 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 dictionary09:16
tristanKinnison, it's complicated because with includes, composition should happen below the including node :)09:16
* Kinnison reads it as:09:16
Kinnison1. take base_node and extract includes from it09:17
tristanor in other words, the including dictionary keys override what is included09:17
*** ikerperez has joined #buildstream09:17
Kinnison2. 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
tristanThe code flow might might run that way, I'm just explaining the why behind it, and the desired behavior09:18
KinnisonI understand the desired behaviour09:18
KinnisonI'm attempting to see how it matches09:18
Kinnison:D09:18
Kinnisonincluding the to_delete stuff etc09:18
KinnisonI *think* I'm slowly grokking this machinery09:18
*** flatmush has quit IRC09:21
*** flatmush has joined #buildstream09:21
KinnisonSo this means that, for example, (=): - whatever in the base_node, will only be verified against the last of the included file dictionaries?09:22
tristanAn (=) 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 node09:28
tristanover the list09:28
tristan... of included fragments09:28
tristanagain though, I am not going from how the code works but how it is supposed to09:30
tristan(sorry)09:30
KinnisonNo, I think we're talking at cross purposes09:30
Kinnison(=) overwrites a list, yes09:30
Kinnisonbut09:30
Kinnisonif the list comes in say the first include in the list09:30
Kinnisonand the second include doesn't have it09:30
Kinnisonthen surely the composite will fail09:30
Kinnisonbecause we're not compositing the base node over the top of the composition of all the includes09:30
Kinnisonwe're compositing in sequence09:30
Kinnisonfrom last include backward09:31
* Kinnison is still very clumsy with all this vocabulary so may be unclear :(09:31
tristanKinnison, So I believe the reason for composition in sequence is because _yaml.composite() is what is going to handle these list composition directives09:34
KinnisonYes09:34
tristanI'm not sure why it is reversed, but valentind would likely remember09:34
tristanI think he is on holiday though09:34
Kinnisonreversed 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 includes09:35
Kinnisonthat's pretty obvious to me09:35
Kinnison*but*09:35
Kinnisonsince 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
Kinnisonsomething: (=): [ stuff, and, things ]09:36
Kinnison(@): [ include1, include2 ]09:36
KinnisonWhere09:36
Kinnisoninclude1 contains a something: []09:36
Kinnisonand include2 does not09:36
*** jonathanmaw has joined #buildstream09:36
Kinnisonas a user, I'd expect include1 and include 2 to be combined, and then for the base node to be applied09:38
Kinnisonbut the code will try and composite include2, base_node09:38
Kinnisonand choke on the (=)09:38
Kinnisonno?09:38
*** jennis has joined #buildstream09:41
*** phildawson has joined #buildstream09:49
*** raoul has joined #buildstream09:56
tristanKinnison, that is an interesting observation10:12
tristanwould seem to be a bug if it is the case10:12
tristanshould be easy enough to throw a test case together in tests/format/include.py10:13
*** sebastian has quit IRC10:14
gitlab-br-botaevri opened issue #935 (Remote Execution: build command failures hard to diagnose (always 'Directory not found')) on buildstream https://gitlab.com/BuildStream/buildstream/issues/93510:15
*** sebastian has joined #buildstream10:15
*** sebastian has quit IRC10:21
gitlab-br-botjuergbi 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/118610:28
gitlab-br-botjuergbi closed issue #799 (Remote Execution: Make TLS and client certificate optional for storage service) on buildstream https://gitlab.com/BuildStream/buildstream/issues/79910:28
gitlab-br-botjuergbi 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/118610:28
*** lachlan has joined #buildstream10:34
adds68juergbi you'll make Marge angry :P10:35
juergbishe was resting ;)10:35
tristanadds68, I think that is the fun part, doesnt she have a dictionary of randomized insults to bark out when she feels abused ?10:36
adds68tristan haha! Someone should submit that as a feature10:36
KinnisonSurely the answer is to just remove merge rights from anyone but marge?10:37
juergbilike https://media0dk-a.akamaihd.net/48/89/c60a361f7436dfbb1c25cbec4c871dc2.jpg10:38
adds68juergbi haha! :)10:39
adds68Kinnison this is how the singularity begins...10:39
Kinnison:+1:10:40
*** lachlan has quit IRC10:41
tristanjonathanmaw, Ummm, you know about the WSL runner apparently ?10:55
tristan(or so I'm told)10:55
jonathanmawtristan: yes10:55
tristanjonathanmaw, Care to just check this comment and confirm whether my suspicions are fact ?10:56
tristanhttps://gitlab.com/BuildStream/buildstream/merge_requests/1189#note_14580959610:56
tristanif so, maybe it is not easy to fix, but probably worth tracking in an issue (maybe tagged with the infrastructure tag)10:57
benschuberttristan: my understanding is, the WSL runner runs in WSL, not in docker, so anything created outside the test directory will live on10:57
benschubertwhereas all other runners are docker's, so we get a fresh runner everytime10:57
tristanbenschubert, 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 it10:58
tristanEver since I heard of WSL, I have been hoping for a Sandbox implementation which uses it to run commands on our provided/staged runtime10:58
tristanBut maybe not a worthwhile effort :)10:59
tristanIn 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 runner11: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 #buildstream11:08
tristanjonathanmaw, Thanks for the comment :)11:08
tristanjonathanmaw, In the meantime, are you able to wrangle the runner manually somehow ? I don't know how it is *supposed* to be upgraded11:09
tristanjonathanmaw, 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-packages11:10
jonathanmawtristan: 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-script11:10
jonathanmaw*created11:10
tristanAh that's good to know that we already have automation to create it11:10
tristanIt would be good to tie that automation into the project somehow11:10
tristanBut I suppose that would be part of a larger effort of documenting how to setup gitlab CI for BuildStream11:11
juergbitpollard: does the test update in !1184 look good to you now?11:15
gitlab-br-botMR !1184: Use virtual artifact directory to stage and extract metadata https://gitlab.com/BuildStream/buildstream/merge_requests/118411:15
*** tristan has quit IRC11:16
*** lachlan has quit IRC11:17
tpollardjuergbi: 1 sec11:18
gitlab-br-bottpollard 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/118411:22
juergbita11:22
*** tristan has joined #buildstream11:33
*** jmac has joined #buildstream11:35
jmacSorry, my shell box got rebooted overnight. Is there anyone around who can help me run the automated tests on bst-external?11:35
juergbijonathanmaw, jennis: ^^11:39
jennisjmac, having a look now11:44
jennis./setup.py test runs the test suite (there is no tox yet), but it looks like this is failing with bst master11:44
jmacYep. I'm using the current master of bst-external and buildstream and it's complaining about obsolete commands11:44
jennisYes, I am seeing this too11:45
jmacBut the CI appears to be working; does it use a fixed version of buildstream?11:45
jennisahh, it's using the buildstream/buildstream-fedora image11:46
jennisWhich is being deprecated IIRC11:47
* jmac notes that this is a problem that can only happen because the useful bits of buildstream are split across two git repositories11:47
jennisOk, I guess this should be using an image with buildstream master installed11:48
jenniscs-shadow, am I right to believe that buildstream/buildstream:dev should be a nightly build of master?11:49
gitlab-br-botmarge-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/118411:49
tpollardwoop11:50
jennisjmac, 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
jmacNo. Is there a particular version of buildstream I should revert to?11:52
jennisWell, I think we need to get this working with BuildStream master, correct? And have the CI run in a container that is also doing this11:53
jennisCurrently, both those things don't happen11:53
jennisDo you agree?11:53
jmacYes, 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
jennisOk, well it looks like until I get this fixed (trying now) the tests are only going to pass on an older version of buildstream11:57
jennis4d92c106, I think11:58
jennisFrom looking at the image defined at the top of .gitlab-ci.yml11:58
jmacTa11:59
*** alatiera has joined #buildstream12:01
*** raoul has quit IRC12:02
*** raoul has joined #buildstream12:12
*** lachlan has joined #buildstream12:15
*** lachlan has quit IRC12:29
*** raoul has quit IRC12:32
cs-shadowjennis: no, it'll be the development snapshot, i.e. 1.3.0 at the moment12:33
cs-shadow`:nightly` is the tag for nightly12:33
jennisOh, that makes sense12:34
cs-shadow"Image Variants" section of https://hub.docker.com/r/buildstream/buildstream talks a bit about it12:34
jenniscs-shadow, my work around was to using test-suite, cloning and then install BuildStream >.<12:34
jennisYeah, just looking at that now12:34
jennisthanks12:34
jennisMany broken links, I'll put in an MR12:35
cs-shadowoh really, where?12:35
jenniscs-shadow In the README of buildstream-docker-images12:42
cs-shadowah, 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 links12:44
*** lachlan has joined #buildstream12:47
jennisgood shout, I'd imagine so12:49
jenniscs-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 IRC12:53
cs-shadowjennis: thanks! I've provided the correct URL as a reply12:54
jenniscs-shadow, whoops forgot to update one of the links12:57
jennissex12:57
jennissec*12:57
jennis!12:57
jenniscs-shadow, should be good now12:59
cs-shadowjennis: seems all good now, thank you very much]13:02
jennisYou're welcome13:02
tpollardjuergbi: is there any more MR's to get landed before the abstract WIP can be pointed back to master?13:04
juergbitpollard: no. I have a couple more in flight and there might be some tiny conflicts, but nothing substantial13:06
tpollardjuergbi: that's the impression I had, thanks!13:10
gitlab-br-botcs-shadow approved MR !1189 (tristan/optional-coverage->master: Make coverage optional) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/118913:18
gitlab-br-botjuergbi closed issue #920 (Use virtual directories for staging dependencies) on buildstream https://gitlab.com/BuildStream/buildstream/issues/92013:28
gitlab-br-botjuergbi closed issue #913 (Extracting buildtree/specific subdirs from the cache should be done directly) on buildstream https://gitlab.com/BuildStream/buildstream/issues/91313:29
gitlab-br-botjuergbi closed issue #921 (Drop extract directories) on buildstream https://gitlab.com/BuildStream/buildstream/issues/92113:30
*** nimish has joined #buildstream13:39
*** raoul has joined #buildstream13:48
gitlab-br-botaevri 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/119313:48
gitlab-br-botjuergbi 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/118813:48
*** phildawson has quit IRC13:58
*** phildawson has joined #buildstream13:58
*** lachlan has joined #buildstream14:11
*** phildawson has quit IRC14:20
*** lachlan has quit IRC14:26
*** phildawson has joined #buildstream14:29
*** lachlan has joined #buildstream14:31
juergbiaevri: haha, talking to Marge now?14:32
juergbi!1188 is now ready for review for anyone familiar with or interested in CasBasedDirectory optimizations14:35
*** lachlan has quit IRC14:35
cs-shadowaevri: missed opportunity to reference https://www.xkcd.com/149/ :)14:40
gitlab-br-botmarge-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/119314:45
jmacIs _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 check14:47
cs-shadowhttps://github.com/warner/python-versioneer#theory-of-operation might be useful14:49
jmacLooks like we might overwrite it if we updated the version of versioneer14:52
*** lachlan has joined #buildstream14:54
gitlab-br-botraoul.hidalgocharman opened (was WIP) MR !1124 (raoul/440-source-cache->master: Source cache) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/112414:54
raoulThink !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 now14:55
juergbigreat, will take a look14:55
cs-shadowjmac: yeah, anytime we do `versioneer install` and its format changes, it might get updated14:58
cs-shadowalthough it's format will likely not change wildly in reality14:59
jennisOur CI uses the buildstream/testsuite-* images, at some tag, do we manually update these tags when system deps change?15:24
cs-shadowyep15:25
jennisahh, ok :/15:25
aevrijuergbi: yup, trying to stay on her good side for the robot uprising. I, for one, welcome our new gitlab-bot overlords.15:29
jenniscs-shadow, in the CI for bst-plugins-container, where is the image with bst installed / where do we even install bst?15:32
jennisI'm failing to see how we can even run bst commands for those tests15:32
jennisjmac, 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_testsuite15:34
jmacAh, super15:35
jennisIt's still failing CI due to some coverage stuff I'm unsure about, but it does work locally for me15:35
cs-shadowJennie: this image doesn’t have it. Check tox.ini15:35
cs-shadowjennis: ^15:35
jenniscs-shadow, so when we run 'tox --whatever' in the container, BuildStream is installed then, in the tox environment?15:37
*** raoul has quit IRC15:56
cs-shadowjennis: exactly15:58
*** raoul has joined #buildstream16:03
*** nimish has quit IRC16:14
*** nimish has joined #buildstream16:18
jennisthanks cs-shadow16:25
*** paulsherwood has quit IRC16:27
*** lachlan has quit IRC16:36
*** lachlan has joined #buildstream16:37
*** lachlan has quit IRC16:40
*** lachlan has joined #buildstream16:44
*** lachlan has quit IRC16:56
*** lachlan has joined #buildstream16:57
*** lachlan has quit IRC17:01
*** lachlan has joined #buildstream17:03
*** lachlan has quit IRC17:11
*** lachlan has joined #buildstream17:26
*** lachlan has quit IRC17:30
*** lachlan has joined #buildstream17:44
gitlab-br-botraoul.hidalgocharman approved MR !1188 (juerg/lazy-directory-digest->master: _casbaseddirectory.py: Calculate directory digest lazily) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/118817:47
jennistristan, 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' key18:00
jennisI've pushed a branch that adds a test for this here: https://gitlab.com/BuildStream/buildstream/commit/c48f7c1d1f87e75c1b9c1ef83b7e2b19967734a918:00
jennisThis test is failing because we *are* managing to overwrite 'test' which doesn't exist in b.yml18:01
gitlab-br-bottpollard opened (was WIP) MR !1175 (tpollard/908->master: Artifact 'abstraction' class) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/117518:02
*** jonathanmaw has quit IRC18:17
*** lachlan has quit IRC18:22
*** raoul has quit IRC18:24
gitlab-br-botBenjaminSchubert opened issue #936 (Tests fail if host tools are not in a standard directory) on buildstream https://gitlab.com/BuildStream/buildstream/issues/93618:28
*** alatiera has quit IRC18:43
gitlab-br-botBenjaminSchubert 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/119418:51
gitlab-br-botcs-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/119418:57
*** nimish has quit IRC19:19
*** nimish has joined #buildstream19:20
*** ChanServ sets mode: +o tristan19:25
tristanjennis, 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 test19:25
tristanjennis, that is probably already 90% of fixing it :)19:25
tristanjmac, 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.219:28
tristanor ensuring that downstream projects which use bst-external with bst 1.2 dont accidentally get bst-external for master and then break19:28
juergbiI think we should find a way for external plugins to support both bst 1.2 and master19:29
juergbiespecially important for project-local plugins but also sounds useful for other external plugins, imo19:29
tristanCertainly 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 model19:30
juergbithat doesn't help for project-local plugins19:31
tristanif 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 well19:31
cs-shadowjuergbi: sorry if i'm bit out of context, but what's preventing plugins from supporting both versions?19:31
tristanfor project local plugins you don't have that problem do you ?19:31
tristanI mean if your project uses bst 1.2, then the local plugins you have in the project are for bst 1.219:31
juergbics-shadow: nothing really, they could use something like `hasattr()` and similar I suppose19:31
tristancs-shadow, lack of guaranteed API stability is preventing that19:32
juergbitristan: well, my goal would be to keep e.g., fdo-sdk supporting both19:32
tristanbut I suppose ugly hacks could be employed instead of sane versioning :-S19:32
tristanjuergbi, That sounds awesome but I am not that hopeful especially after going ahead with plugin migration plans19:32
tristanI 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 master19:33
tristanonce bst diverges to a point where it can not really make sense19:33
tristan(maybe for now it can ?)19:34
juergbifreedesktop-sdk could switch to the external plugin even while they still support 1.2 as well19:34
juergbioverriding bst 1.2 core plugins19:34
juergbias long as the external plugin stays compatible with 1.219:34
juergbiI think there is a big benefit in allowing projects to be compatible with both19:34
tristanI am assuming that we don't really switch without significantly modifying the project.conf format for obtaining plugins in a more reliable way than pip19:35
tristanbut that is still up in the air I guess19:35
juergbitrue, that is a potential issue19:35
tristanYeah, I agree it would be nice, but I think that that dream is mostly incompatible with a lack of guaranteed API stability19:35
tristanIt would *really* be nice though :)19:35
juergbiwith Python it should really be possible to handle this19:36
juergbiI think it's worth the maintenance overhead for external plugins19:36
juergbiand it might not be more painful overall than maintaining two branches19:36
tristanWith that I agree19:36
tristanI 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.019:38
juergbiwe could potentially extract certain helper classes from bst master to avoid too many conditionals19:39
juergbie.g., to use the virtual directory API (only with filesystem backend)19:40
cs-shadowI 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 differently19:40
tristanOr, 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 initializer19:40
juergbics-shadow: my main concern is how to support bst 1.2 and master with a single project branch19:41
tristancs-shadow, core plugins certainly have been receiving master specific updates especially for virtual directories19:41
juergbiI assume he means on 1.219:41
cs-shadowyeah i meant on the 1.2 branch19:41
tristanOh yeah19:41
tristanyeah we're talking about supporting 1.2 & master19:42
juergbics-shadow: if project.conf uses pip for external plugins, the same branch has to work for 1.2 and master19:42
juergbiunless I'm missing something here19:42
juergbimaybe conditional in project.conf, hmm19:42
tristancurrently project.conf does not have any way to impose a specific version of a pip installed plugin package19:43
tristanWe go on the assumption that plugin packages are API stable19:43
cs-shadowjuergbi: I'd have imagined that therre's a requirements.txt or similar in the project root that can achieve this19:43
tristanSo we would need a different package name I guess for both 1.2 vs master19:43
juergbithat could be an option, I suppose, instead of installing via project.conf pip19:44
cs-shadowbuildstream itself will still not be enforcing conditionals etc. but it's concievable to have bst1.2-reqs.txt and bst-mater-reqs.txt19:45
cs-shadowthat's one option anyway19:45
tristancs-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 from19:45
tristanI 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 work19:46
tristanI believe currently the instructions say to "install bst-external", and that means getting the git repo at master and running `pip3 install .` from there19:47
cs-shadowwell, 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 plugins19:49
cs-shadowthat would ensure the `pip install bst-external` gives a version that works with bst-1.219:49
cs-shadowand people who want to try master, can install the pre-release versions19:49
tristanI was thinking that would be a convenient way yeah, or juergbi was suggesting to have those plugins support both versions for the transition period19:49
tristancs-shadow, *however*19:50
tristanbst-external is not parallel installable19:50
tristanSo it will be a pain in the ass to go switch from bst 1.2 to master and back at will19:50
tristanWhile trying out master and reverting to 1.2 at times if things break19:51
juergbiindeed, adding a big hurdle for users trying out 1.9019: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-shadowhow's that different from installing a different bst version?19:52
tristanin 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-shadowi am not sure about the new namespace unless we also plan to have bst-2 as well?19:53
tristancs-shadow, First of all, even if bst 1.90 is not parallel installable with 1.2, it is still a *huge* difference19:53
SotKIs there a reason for not just installing the new (or old) bst-external in a virtualenv?19:54
tristani.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
tristanSotK, That is not something we can impose on users19:54
tristanSotK, 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
tristanFor 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 frustration19:56
gitlab-br-botBenjaminSchubert opened MR !1195 (remove-dead-code->master: Remove dead code) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/119519:56
tristanHonestly, 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 fine19:57
tristanAnd we can do fancy venv stuff from CI just to ensure that master is staying relatively compatible19:57
tristanThen 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
benschuberttristan: 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
benschubertOtherwise we would need to guarantee many other things to be coherent20:31
benschubertLike cache keys20:31
benschubertAnd plugins can restrict their buildstream version compatibility, as is the case for Django/flask/pytest, etx20:32
tristanbenschubert, 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 ambiguous21:33
tristanbenschubert, 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 lookup21:34
*** slaf has quit IRC22:07
*** tristan has quit IRC22:24
*** slaf has joined #buildstream22:34
*** slaf has joined #buildstream22:34
*** slaf has joined #buildstream22:35
*** slaf has joined #buildstream22:35
*** slaf has joined #buildstream22:35
*** slaf has joined #buildstream22:35
*** slaf has joined #buildstream22:36
*** slaf has joined #buildstream22:36
*** slaf has joined #buildstream22:36
*** slaf has joined #buildstream22:36
*** slaf has joined #buildstream22:37
*** slaf has joined #buildstream22:37
*** benschubert has quit IRC22:57
*** slaf has quit IRC23:03
*** slaf has joined #buildstream23:03
*** slaf has joined #buildstream23:03
*** slaf has joined #buildstream23:03
*** slaf has joined #buildstream23:04
*** slaf has joined #buildstream23:04
*** slaf has joined #buildstream23:04

Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!