| *** kapil___ has quit IRC | 01:42 | |
| *** nimish has quit IRC | 03:22 | |
| *** kapil___ has joined #buildstream | 05:00 | |
| *** mohan43u has quit IRC | 05:39 | |
| *** mohan43u has joined #buildstream | 05:42 | |
| gitlab-br-bot | marge-bot123 merged MR !1188 (juerg/lazy-directory-digest->master: _casbaseddirectory.py: Calculate directory digest lazily) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1188 | 05:56 |
|---|---|---|
| *** tristan has joined #buildstream | 07:12 | |
| juergbi | tristan: in case you have a (different) opinion on error type hierarchy: https://gitlab.com/BuildStream/buildstream/merge_requests/1175#note_146216523 | 07:21 |
| *** alatiera has joined #buildstream | 07:47 | |
| *** ChanServ sets mode: +o tristan | 07:59 | |
| tristan | I'm having a hard time answering that | 07:59 |
| *** paulsherwood has joined #buildstream | 08:12 | |
| *** paulsherwood has joined #buildstream | 08:12 | |
| *** sebastian has joined #buildstream | 08:40 | |
| *** sebastian has quit IRC | 08:48 | |
| *** benschubert has joined #buildstream | 08:57 | |
| benschubert | Does anyone has any opinion about https://gitlab.com/BuildStream/buildstream/merge_requests/1194 ? Otherwise I'll merge it soon | 08:57 |
| juergbi | no objections from me | 09:00 |
| juergbi | maybe prefix the commit message with 'tests: ' | 09:01 |
| tristan | benschubert, Seems to me we now have a mixture of variables with HAVE_ and have removed others which represent the presence of an existing CLI | 09:01 |
| benschubert | juergbi: sure! | 09:01 |
| tristan | Do we need to remove all the "HAVE_" at the same time as adding resolved host CLI paths ? | 09:01 |
| tristan | Just looks generally less consistent and readable | 09:02 |
| tristan | also using GIT instead of _site.GIT looks less readable to me | 09:02 |
| gitlab-br-bot | aevri opened MR !1196 (aevri/nonecach2->master: cascache: limit 'infinite' cache to volume size) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1196 | 09:03 |
| tristan | benschubert, aside from stilistic opinionated comments, we should really prefix that commit message subject line with "tests: ...." or such | 09:04 |
| tristan | looking at the subject line of the patch leads me to think this is about _site.py in BuildStream, not related to tests | 09:04 |
| benschubert | tristan: there are two checks we are doing: do we have the command, or do we have a library/a recent enough command. For me this is two different things. We can have GIT and HAVE_GIT, but what's the point? if GIT is None, then I don't have git. I'm happy to use _site.GIT, but then that would mean doing the same for everything coming from sites, to be consistent, and I'm not sure this is nice | 09:04 |
| tristan | benschubert, that is how we use _site.py in BuildStream, and is generally in the coding guidelines | 09:05 |
| benschubert | tristan: then moving every HAVE_* to _site.HAVE_* ? I'm happy to do that, in a separate commit in the same MR if yes | 09:05 |
| tristan | benschubert, i.e. we dont `import function from utils` and use the function, we always call utils.function() so that it is clear when reading the code where this comes from and what it is | 09:06 |
| benschubert | tristan: so adding a second commit, which renames everything we access from _site to use _site.* would be good with you? | 09:08 |
| tristan | benschubert, I would personally like to keep the HAVE_s and only add the tools, so that I can blindly trust that "If there is a thing, there is always a HAVE_, which I should use in the skipif() statements for clarity" | 09:11 |
| tristan | If we change everything to use `_site.*` everywhere, that will also make sense to conform to the overall style. I think it was less important when everything we imported from there was a HAVE_SOMETHING (i.e. it was obvious by looking at that where it comes from) | 09:12 |
| gitlab-br-bot | aevri 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 | 09:14 |
| benschubert | tristan: I would say we have a different version of obvious, since HAVE_GIT was True and the tests wouldn't find my git | 09:14 |
| benschubert | but anyways | 09:15 |
| benschubert | tristan: So export the variables, and use them from _site.X I won't bother renaming all of this more than that then. Would that be good? | 09:15 |
| tristan | Why were your tests not finding your git to begin with ? | 09:16 |
| tristan | Because the individual tests failed to use the API to get a host tool ? | 09:16 |
| tristan | <benschubert> tristan: So export the variables, and use them from _site.X I won't bother renaming all of this more than that then. Would that be good? | 09:17 |
| tristan | Regarding that, if I understand, your patch would only *add* the new shortly named all caps host tools, and those additions would be accessed via _site.FOO, causing the whole patch to be much smaller and not changing the HAVE_FOO stuff at all | 09:18 |
| benschubert | tristan: because they were checking HAVE_GIT and then using "git" directly, while redefining the PATH. This is indeed a bug, bug a bug that would be obvious if we didn't have both | 09:18 |
| tristan | That would be fine with me yeah | 09:18 |
| benschubert | tristan: ok I'll refactor it like that then | 09:18 |
| benschubert | thanks | 09:18 |
| tristan | Right, using "git" directly off the bat seems to be the underlying bug, but resolving them once from _site.py is a micro optimization and should help prevent those bugs from occurring | 09:19 |
| tristan | can agree with that | 09:19 |
| benschubert | On another note, we have ïmport cli"in many of our test files, in order to have the fixture available. Would it be fine with people if I move this to a conftest.py? | 09:20 |
| tristan | I would be very happy if we can remove conftest.py completely | 09:20 |
| benschubert | why? | 09:20 |
| benschubert | I mean it's the canonical way of doing it with pytest | 09:21 |
| tristan | I spent the greater part of a year wondering what that file was and how it integrates with tests and what invokes it | 09:21 |
| tristan | until I finally had to touch it and added a big fat comment at the top | 09:21 |
| tristan | it looks like black magic for anyone who doenst happen to be really close friends with pytest | 09:21 |
| benschubert | tristan: it's in the docs about fixtures on pytest, I don't see how this is black magic. We can expect people coming to the project to at least read the documentation about pytest if they are not familiar with it | 09:22 |
| tristan | exactly, its hidden all the way in documentation | 09:28 |
| *** raoul has joined #buildstream | 09:29 | |
| tristan | tests are just programs which exercise the codebase, one should be able to open up a test file and understand what is going on from there | 09:29 |
| tristan | benschubert, the situation would be *greatly* improved if pytest had chosen a sensible name for it | 09:30 |
| tristan | like say, pytest_globals.py or something with pytest at least in the name | 09:30 |
| benschubert | tristan: I mean, wait I don't have a TestCase class, how does that work? It's the same thing, people need to know the basics of pytest to be able to use pytest | 09:30 |
| tristan | I have never needed to know about the magic conftest.py, until making tests parallelizable with detox | 09:31 |
| tristan | If I look at a test, I can tell from the imports it is using what fixtures come from | 09:32 |
| tristan | it is a bit messy that there is a bunch of pytest module stuff "automatically in context", but even that is less frustrating than conftest.py | 09:32 |
| benschubert | tristan: And then we realize we have two differents paths for cli, and 2 different cli, named cli, cli2 and cli_integration. I'd rather have a coherent name in conftest.py | 09:33 |
| tristan | Anyway, we have a comment in there now so at least people can see what it is | 09:33 |
| tristan | I consider that a bug honestly, we should kill cli_integration | 09:33 |
| tristan | There is really not much special about cli_integration except for a hack that it does on the project.conf, which really should not work differently for integration tests and regular tests | 09:34 |
| tristan | That and the configuration (shared cached sources and artifacts), but that could be handled by virtue of marking the tests as integration tests, without having a separate fixture | 09:35 |
| *** jonathanmaw has joined #buildstream | 09:37 | |
| benschubert | tristan: ok, at least I can have a look to kill this. | 09:37 |
| benschubert | also, would a PR adding linting to the tests (slightly relaxed) would have a chance to pass? | 09:37 |
| tristan | I don't know :) | 09:38 |
| benschubert | ok I'll try then | 09:38 |
| tristan | Would be good to add linting of tests, don't know how many violations we have :) | 09:38 |
| benschubert | too many but I have a WIP PR to clean part of that already | 09:39 |
| *** sebastian has joined #buildstream | 09:48 | |
| jennis | tristan, heh, that /home/jennis/banana.yml thing was just to show what was composited, I'll remove it and file an issue | 09:55 |
| benschubert | tristan: https://gitlab.com/BuildStream/buildstream/merge_requests/1194/ does this suites you better? (assuming all tests pass) | 10:02 |
| tristan | benschubert, Sure ! | 10:05 |
| tristan | jennis, thanks ! it will make it easier to solve :) | 10:05 |
| benschubert | tristan: great I'll assign it to marge then | 10:06 |
| gitlab-br-bot | aevri opened MR !1197 (aevri/unused_commandname->master: buildelement: rm unused 'cmd_name' argument) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1197 | 10:18 |
| laurence | I need to amend some of the security around marge | 10:25 |
| laurence | at the moment, anyone can assign an MR to the bot and get it merged: even if, for example, it's not been approved or they don't have permissions to merge. | 10:25 |
| laurence | I'm looking into it now | 10:25 |
| tristan | laurence, interesting oversight - not particularly worried about unwelcome commits landing (it's not like we really have bad actors to worry about I think) | 10:35 |
| tristan | but good to clear up in general | 10:35 |
| *** lachlan has joined #buildstream | 10:35 | |
| gitlab-br-bot | marge-bot123 closed issue #936 (Tests fail if host tools are not in a standard directory) on buildstream https://gitlab.com/BuildStream/buildstream/issues/936 | 10:41 |
| gitlab-br-bot | marge-bot123 merged 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 | 10:41 |
| *** kapil___ has quit IRC | 10:54 | |
| gitlab-br-bot | aevri opened MR !1198 (aevri/unused_color->master: runcli: rm unused 'color' and '**extra' params) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1198 | 10:58 |
| *** lachlan has quit IRC | 11:10 | |
| gitlab-br-bot | BenjaminSchubert opened MR !1199 (bschubert/no-subprocess-decode->master: refactor: remove the need to decode output from subprocess) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1199 | 11:17 |
| *** lachlan has joined #buildstream | 11:21 | |
| *** lachlan has quit IRC | 11:31 | |
| gitlab-br-bot | LaurenceUrhegyi opened issue #937 (Plug-ins Story: tasks) on buildstream https://gitlab.com/BuildStream/buildstream/issues/937 | 11:35 |
| *** deon has joined #buildstream | 11:37 | |
| *** deon has quit IRC | 11:40 | |
| *** lachlan has joined #buildstream | 11:43 | |
| gitlab-br-bot | jennis opened issue #938 (Overwrite does not raise error when we try to overwrite a non-existent node) on buildstream https://gitlab.com/BuildStream/buildstream/issues/938 | 11:47 |
| jennis | tristan, are you around? | 11:47 |
| tristan | jennis, ummm if you can gimme 10min... just proof reading this draft... | 11:47 |
| jennis | Yeah no worries, just ping me when you're free | 11:48 |
| *** lachlan has quit IRC | 11:49 | |
| *** lachlan has joined #buildstream | 11:52 | |
| gitlab-br-bot | cs-shadow approved MR !1197 (aevri/unused_commandname->master: buildelement: rm unused 'cmd_name' argument) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1197 | 11:54 |
| *** lachlan has quit IRC | 11:57 | |
| *** lachlan has joined #buildstream | 11:59 | |
| *** lachlan has quit IRC | 12:04 | |
| tristan | jennis, ... what's up ? | 12:06 |
| jennis | Hey tristan, I just wanted to discuss with you how we think the composition should work with directives | 12:07 |
| jennis | I've raised an issue and added a test | 12:07 |
| jennis | https://gitlab.com/BuildStream/buildstream/issues/938 | 12:07 |
| tristan | Ah | 12:07 |
| tristan | jennis, you mean it is not clear what the expected behavior should be ? | 12:08 |
| jennis | Kind of | 12:08 |
| jennis | First point is that the code isn't doing what the documentation says | 12:08 |
| jennis | and I think you and Kinnison agreed the other day that if we try to overwrite a list that doesn't exist in the source, when we composite, we should get an error, yes? | 12:08 |
| tristan | The documentation is almost certainly more correct in this case (while sometimes it is not, in this case I'm pretty sure it is) | 12:08 |
| gitlab-br-bot | cs-shadow opened issue #941 (Enable pylint for tests) on buildstream https://gitlab.com/BuildStream/buildstream/issues/941 | 12:09 |
| tristan | That is expected yes, when one uses (=), that is expected | 12:09 |
| jennis | ahh ok, so the tests I've added show that this doesn't happen and the fix for list overwrite specifically probably won't be too difficult | 12:09 |
| jennis | But, I wondered whether this should also happen for (<) and (>) when we try to do this to a non-existent node in the source | 12:10 |
| tristan | I.e. when the thing you are compositing on top of lacks the list you are overwriting, then using (=) to overwrite triggers an error | 12:10 |
| jennis | ? | 12:10 |
| jennis | Yes, I agree, so we should definitely implement that | 12:10 |
| jennis | but what about when we try to prepend/append and the source lacks the list? | 12:10 |
| tristan | Yes, (<) and (>) cause an error when appending/prepending to a non-existent list | 12:11 |
| tristan | if this does not happen with includes, I expect is it only with include lists where this fails to happen | 12:11 |
| jennis | Right, ok, have you seen my test? With random: {(=): [foo]}? | 12:11 |
| tristan | The documentation is more explicit about this for (=), that is because it is the only real difference from specifying a list | 12:12 |
| tristan | I.e. specifying a list without a directive will also overwrite the underlying list | 12:12 |
| tristan | But will not trigger any error | 12:12 |
| jennis | Or it will *create* the list...? | 12:12 |
| tristan | jennis, Right, but that is pretty much the same thing - if a list happens to exist there already, it's not an error | 12:13 |
| tristan | it will be created on top of whatever was there | 12:13 |
| tristan | in the same way that a creating a string will replace whatever string might have been there before (if one was there) | 12:13 |
| tristan | I can't say that I can completely decompiled your test case into my understanding :) | 12:14 |
| tristan | lemme take another look | 12:14 |
| jennis | Yeah ok I understand that | 12:14 |
| jennis | So one thing that I'm unsure about | 12:14 |
| jennis | in my test case, we have a 'base' dict and an 'overlay' dict which we try to compose onto base | 12:15 |
| jennis | For example, before compose base['foo'] = [1,2], if I have overlay['foo'] = {(>): [3,4] }, we get base['foo'] = [1, 2, 3, 4], right...? | 12:16 |
| tristan | Kind of mixing notations a bit but yes that looks correct | 12:17 |
| jennis | Now, if we try: before compose overlay['random'] = {(>): [3,4] }, after composition, we get base['random'] = {(>): [3,4] } | 12:17 |
| jennis | Is that what we want...? Or do we want it to also throw an error in this case? | 12:17 |
| tristan | I dont think I follow | 12:18 |
| jennis | So, when we tried to compose a key that existed in base (the source), the composition worked and we got a nice list out | 12:18 |
| tristan | ok I think I understand | 12:19 |
| jennis | Then, when we try to compose a key that *does not* exist in base (the source), the composition creates an item in the source that has this directive in it | 12:19 |
| tristan | jennis, I believe you are missing out on an important part of composition, lemme check the code | 12:19 |
| jennis | Probably x) | 12:20 |
| tristan | jennis, remember that a pitfall of testing the _yaml API directly means that you have not yet exercised the complete load process | 12:20 |
| tristan | I think that a step is missing but lemme find it | 12:20 |
| gitlab-br-bot | marge-bot123 merged MR !1197 (aevri/unused_commandname->master: buildelement: rm unused 'cmd_name' argument) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1197 | 12:20 |
| tristan | jennis, in real life _yaml.node_final_assertions() gets run | 12:23 |
| jennis | I'm getting the impression that all that needs to happen is that: if we have base['foo'] = [1,2] (and no other items), and try to compose overlay['random'] = {'(=)': ['cow'] }, we should raise a LoadError (and *not* base['random'] = {'(=)': ['cow']}, which is what currently happens | 12:24 |
| tristan | jennis, this is probably missing test coverage for the errors you are expecting | 12:24 |
| jennis | and the rest is working machinery | 12:24 |
| tristan | actually here: tests/format/listdirectiveerrors.py | 12:25 |
| tristan | that is what tests it so far | 12:25 |
| tristan | But maybe we need similar in tests/format/includes.py ? | 12:25 |
| jennis | well yeah, I added one to format/include_composition.py, and one to internals/yaml.py that test this | 12:26 |
| jennis | but perhaps thats a better place | 12:26 |
| tristan | jennis, to elaborate... We call _yaml.composite() in many places, and after compositing two dictionaries together, we cannot know if the result is intended to be composited on top of something else | 12:27 |
| tristan | And so we don't know if everything is resolved yet | 12:27 |
| jennis | Ahh, so it's ok to have those dicts with the directives in them lying around | 12:27 |
| tristan | That is why we have to run a recursive check at the end to ensure that all is finished | 12:27 |
| jennis | as chances are they'll get picked up along the way | 12:27 |
| jennis | if not, we error at the very end... | 12:27 |
| jennis | ? | 12:27 |
| tristan | Right | 12:27 |
| jennis | gotcha, thanks for making that clear | 12:28 |
| jennis | I'll add a test and attempt a fix for the include directive | 12:28 |
| jennis | *add a test to the right place | 12:28 |
| *** lachlan has joined #buildstream | 12:28 | |
| jennis | tristan, that is, if you think the test I added to tests/internals/yaml.py is in the wrong place? | 12:29 |
| tristan | jennis, Nice, thanks for looking into this ! | 12:29 |
| tristan | jennis, I think that the actual issue we are seeing has to do with includes, and am more interested in a tests/format/ test case | 12:30 |
| tristan | For example, a test in _yaml.py which runs _yaml.node_final_assertions() explicitly, does not guarantee that the core is calling _yaml.node_final_assertions() at the right times | 12:30 |
| tristan | while a test for cases in tests/format/includes.py would | 12:31 |
| *** raoul has quit IRC | 12:32 | |
| tristan | Added tests in tests/internals/yaml.py are still useful of course, and it might be good to have a variation of test_list_composition() which checks for errors and uses _yaml.node_final_assertions() | 12:32 |
| gitlab-br-bot | BenjaminSchubert opened MR !1200 (bschubert/more-pythonic-list-concat->master: refactor: Use [a, b, *c] instead of [a, b] + c when building list) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1200 | 12:32 |
| tristan | But personally I am more interested in the other, and wouldnt block a patch which lacks the _yaml level test enhancement | 12:33 |
| tristan | jennis, fwiw, my expectation is that a['foo']: { '(>)': [2, 3] } + b['foo']: { '(>)': [2, 3] } = c['foo']: { '(>)': [2, 3, 2, 3] } | 12:37 |
| tristan | I.e. appends are accumulative until the base list is discovered, and node_final_assertions() will catch cases where no base list was ever discovered | 12:38 |
| jennis | ahh I see, with the expectation that somewhere along the line that then gets appended onto something['foo'], if not, load error | 12:39 |
| jennis | I think I understand it now, thanks | 12:39 |
| *** alatiera has quit IRC | 12:48 | |
| *** nimish has joined #buildstream | 13:08 | |
| gitlab-br-bot | BenjaminSchubert opened (was WIP) MR !1195 (remove-dead-code->master: Remove dead code) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1195 | 13:16 |
| *** nimish has quit IRC | 13:23 | |
| *** nimish has joined #buildstream | 13:27 | |
| *** tristan has quit IRC | 13:44 | |
| *** raoul has joined #buildstream | 13:46 | |
| gitlab-br-bot | aevri approved MR !1200 (bschubert/more-pythonic-list-concat->master: refactor: Use [a, b, *c] instead of [a, b] + c when building list) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1200 | 14:03 |
| *** tristan has joined #buildstream | 14:05 | |
| *** ChanServ sets mode: +o tristan | 14:05 | |
| gitlab-br-bot | tpollard approved MR !1198 (aevri/unused_color->master: runcli: rm unused 'color' and '**extra' params) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1198 | 14:07 |
| gitlab-br-bot | raoul.hidalgocharman approved MR !1196 (aevri/nonecach2->master: cascache: limit 'infinite' cache to volume size) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1196 | 14:23 |
| jennis | tristan, I feel like I'm starting to see why this use-case wasn't caught before | 14:27 |
| jennis | Most of the time, if we define a list we want to explicitly overwrite, the name of the list will fail on the valid keys check, it seems | 14:27 |
| jennis | for example, it may make sense to explicitly overwrite build-commands: for example, which is always going to be there, as it's an expected key | 14:28 |
| jennis | if I try to overwrite 'foo-commands', it'll fail because 'foo-commands' is an unexpected key | 14:29 |
| jennis | Are there places where we can define lists that don't need to be 'expected'? For example, we can define a random 'variable' e.g. {variable: {foo: 'my variable'}}, but I'm not sure where we can do this for lists | 14:30 |
| jennis | ? | 14:30 |
| tristan | Ummm, if you try to overwrite 'foo-commands' with (=) it will fail | 14:32 |
| jennis | Yes, but that fails because 'foo-commands' is an unexpected key | 14:33 |
| tristan | asides from lists, there is no way to distinguish between the intention of overwriting something that exists, or simply setting a value | 14:33 |
| jennis | So also fails without the '(=)' | 14:33 |
| tristan | right | 14:33 |
| Kinnison | jennis: public data | 14:37 |
| tristan | Right, ummm: Are there places where we can define lists that don't need to be 'expected' is the question ? | 14:39 |
| jennis | aha yes it was | 14:39 |
| jennis | thanks Kinnison | 14:39 |
| tristan | I would say there are many places | 14:40 |
| tristan | For instance, the initial declaration of build-commands is one of them | 14:40 |
| tristan | it is not expected where it is declared | 14:40 |
| tristan | Any plugin can expose configuration in the form of a list | 14:40 |
| tristan | public data is a bit higher up | 14:41 |
| tristan | and indeed I guess a more likely place, but at the same time one could have a use case for using (=) in an override of public data | 14:41 |
| tristan | so they get an error if ever they remove that public data | 14:41 |
| tristan | interestingly, variables and environment which have deeper composition chains dont happen to have lists | 14:42 |
| tristan | but composition logic should not really make any assumptions about that | 14:42 |
| * tristan perhaps overlooked a probable motivation behind the question: How can I create a real world test case to assert this ? | 14:44 | |
| jennis | yeah that's what I'm going for, or something that reflects what someone might sanely do | 14:45 |
| *** alatiera has joined #buildstream | 14:51 | |
| *** kapil___ has joined #buildstream | 14:56 | |
| *** tpollard has quit IRC | 14:59 | |
| gitlab-br-bot | marge-bot123 closed issue #906 (The logic to calculate the disk quota seems to be wrong) on buildstream https://gitlab.com/BuildStream/buildstream/issues/906 | 15:18 |
| *** nimish has quit IRC | 15:18 | |
| gitlab-br-bot | marge-bot123 merged MR !1196 (aevri/nonecach2->master: cascache: limit 'infinite' cache to volume size) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1196 | 15:18 |
| *** nimish has joined #buildstream | 15:23 | |
| aevri | Ruh roh, 500s from gitlab | 15:41 |
| aevri | Phew, it's back. | 15:43 |
| *** phildawson has quit IRC | 15:48 | |
| *** phildawson has joined #buildstream | 15:51 | |
| adds68 | Who can actually modify gitlab, wonders if we can get: https://gitlab.com/BuildStream/buildstream/issues/925, ticked off? :) | 15:52 |
| *** lachlan has quit IRC | 15:54 | |
| * tlater[m] can adds68 :) | 15:55 | |
| adds68 | \o/ | 15:55 |
| gitlab-br-bot | tlater closed issue #925 (Set GitLab logo to use the new BuildStream logo/emblem) on buildstream https://gitlab.com/BuildStream/buildstream/issues/925 | 15:55 |
| gitlab-br-bot | tlater reopened issue #925 (Set GitLab logo to use the new BuildStream logo/emblem) on buildstream https://gitlab.com/BuildStream/buildstream/issues/925 | 15:56 |
| tlater[m] | Oh, nevermind | 15:56 |
| * tlater[m] misread the issue | 15:56 | |
| tlater[m] | Might still have relevant permissions, let me check | 15:56 |
| juergbi | done | 15:57 |
| juergbi | ah, only for group, also need for project | 15:57 |
| adds68 | \o/ nearly there :) | 15:58 |
| tlater[m] | ta juergbi | 15:58 |
| *** lachlan has joined #buildstream | 15:58 | |
| juergbi | done | 15:58 |
| * tlater[m] will miss the beaver :[ | 15:58 | |
| adds68 | woohoo! | 15:58 |
| adds68 | tlater[m] he is still the mascot, we could add him to the about section? | 15:58 |
| tlater[m] | He does deserve to be somewhere! | 15:59 |
| tlater[m] | Have we ever named him? | 16:00 |
| tlater[m] | I think tristan referred to him as tristan at some point | 16:00 |
| tlater[m] | Which was cute | 16:00 |
| tlater[m] | But it does feel a little to egocentric. | 16:00 |
| juergbi | tlater[m]: he named him after you, of course ;) | 16:02 |
| tlater[m] | That's my point x) | 16:02 |
| adds68 | tlater[m] https://gitlab.com/BuildStream/buildstream/wikis/Mascot-Source-Files | 16:02 |
| *** raoul has quit IRC | 16:03 | |
| gitlab-br-bot | juergbi closed issue #925 (Set GitLab logo to use the new BuildStream logo/emblem) on buildstream https://gitlab.com/BuildStream/buildstream/issues/925 | 16:03 |
| *** raoul has joined #buildstream | 16:06 | |
| tlater[m] | ta adds68 | 16:06 |
| * tlater[m] wonders how adds68 ended up becoming buildstream's PR manager | 16:06 | |
| adds68 | haha! i think someone hit me on the head | 16:07 |
| * tlater[m] wonders if a junction element can be "transferred" | 16:12 | |
| tlater[m] | I.e., create a local element that directly pulls in a junction element | 16:12 |
| tlater[m] | So that you don't need to use the wordy filename: x junction: y thing to refer to the base platform all the time | 16:12 |
| tlater[m] | I suppose it's possible | 16:12 |
| tlater[m] | I just see nobody doing that | 16:13 |
| tlater[m] | Do we want to promote this? | 16:13 |
| juergbi | tlater[m]: you've seen !998 ? | 16:13 |
| gitlab-br-bot | MR !998: loader: Allow dependencies to use ":" to refer to junctioned elements https://gitlab.com/BuildStream/buildstream/merge_requests/998 | 16:13 |
| tlater[m] | Not yet... | 16:13 |
| tristan | I didnt name the beaver ! | 16:15 |
| tristan | But I made a wise crack at a talk, saying that we affectionately call tlater[m] "tristan" because he's so bright... and also because it happens to be his name | 16:15 |
| tlater[m] | Ah, right | 16:15 |
| tristan | Which might have also been a bit egocentric heh | 16:15 |
| * tlater[m] will need to start a ML thread to name the beaver | 16:16 | |
| tristan | one day we could have cool beaver hats, maybe with aggressive looking beaver teeth | 16:17 |
| tlater[m] | I just want a giant wrench | 16:18 |
| tristan | then we could compete with vlc guys ! | 16:18 |
| tristan | at conferences | 16:18 |
| tlater[m] | Hehe | 16:18 |
| *** deon has joined #buildstream | 16:19 | |
| tlater[m] | Is cs-shadow around? | 16:19 |
| cs-shadow | tlater[m]: heya! | 16:19 |
| tlater[m] | He is! | 16:19 |
| tlater[m] | I just saw !998 - the change that makes junction elements accessible using <junction>:<element> | 16:20 |
| tlater[m] | Do you think that that should be how we always use junctions? | 16:20 |
| tlater[m] | I'm thinking through how we'd use something like freedesktop-sdk on a new project | 16:20 |
| tlater[m] | Aimed at really new users | 16:20 |
| tlater[m] | And I feel like even that syntax is a little confusing | 16:21 |
| tlater[m] | Any thoughts regarding that? | 16:21 |
| tristan | The colon is confusing ? | 16:21 |
| tlater[m] | Well, I suppose it's already confusing enough to depend on an element you can't inspect too easily | 16:21 |
| cs-shadow | do you have a different syntax in mind? | 16:21 |
| tlater[m] | cs-shadow: I was going to hide it behind a separate element | 16:22 |
| tristan | My thoughts are that the dictionary form allows for extensibility and we should keep the dictionary expressions of dependencies valid | 16:22 |
| tlater[m] | I.e., import the base element in a base.bst file and depend on that. | 16:22 |
| tristan | But that the colon notation is also convenient | 16:22 |
| * tlater[m] likes the notation | 16:22 | |
| tristan | I.e. one thing that we still need to add to dependency declarations is "always rebuild" | 16:22 |
| tlater[m] | It's just that it's a bit magic for an intro project | 16:22 |
| tristan | which was initially added only to element types (like compose for instance), but technically should be a semantic of a dependency | 16:23 |
| tristan | tlater[m], I think that it is consistent to use colon notation, as it appears in many places | 16:23 |
| cs-shadow | Agree with tristan that dictionary version should continue to work. I noticed that in our projects, we had many cross-junction dependencies. Having a separate element wasn't reasonable for that many elements, and the bst files were getting rather verbose and repetitive otherwise | 16:23 |
| tristan | I think that before cs-shadow's patch, we already supported it on the command line no ? | 16:24 |
| cs-shadow | yes | 16:24 |
| tristan | i.e. I can open cross-junction workspaces like this | 16:24 |
| cs-shadow | bst show etc already support this | 16:24 |
| tristan | and it is also used to display cross-junction element names in the UI | 16:24 |
| tlater[m] | Ok, I'll use that then, just wanted to get some other thoughts on it, thanks! | 16:24 |
| tristan | right, in bst show and in any bst command which runs a session | 16:24 |
| tristan | cross junction tab-completion would be sweet too, but tricky and and sometimes impossible (before the junction is fetched) | 16:27 |
| tlater[m] | Ooh, that does sound rather cute | 16:27 |
| tlater[m] | It would make explaining junctions so much easier too | 16:27 |
| tristan | probably not the most valuable use of developer hours at this point, but would definitely be nice to have | 16:28 |
| cs-shadow | it will be nice to have bash completions working at all when installed with `--user` | 16:30 |
| gitlab-br-bot | marge-bot123 merged MR !1198 (aevri/unused_color->master: runcli: rm unused 'color' and '**extra' params) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1198 | 16:39 |
| *** deon has quit IRC | 17:01 | |
| *** deon has joined #buildstream | 17:01 | |
| *** deon has quit IRC | 17:04 | |
| benschubert | I'll assign !1195 to marge soon if nobody else wants to review it. let me know :D | 17:34 |
| gitlab-br-bot | MR !1195: Remove dead code https://gitlab.com/BuildStream/buildstream/merge_requests/1195 | 17:34 |
| gitlab-br-bot | aevri approved MR !1195 (remove-dead-code->master: Remove dead code) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1195 | 17:34 |
| *** phildawson has quit IRC | 17:35 | |
| benschubert | idem for !1199 (as soon as I have aevri's approval) | 17:37 |
| gitlab-br-bot | MR !1199: refactor: remove the need to decode output from subprocess https://gitlab.com/BuildStream/buildstream/merge_requests/1199 | 17:37 |
| gitlab-br-bot | cs-shadow opened MR !1201 (chandan/remove-unused-pytestmark->master: tests: Remove unused `pytestmark` variable) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1201 | 17:48 |
| gitlab-br-bot | aevri approved MR !1199 (bschubert/no-subprocess-decode->master: refactor: remove the need to decode output from subprocess) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1199 | 17:48 |
| *** lachlan has quit IRC | 18:05 | |
| *** raoul has quit IRC | 18:18 | |
| *** jonathanmaw has quit IRC | 18:22 | |
| gitlab-br-bot | cs-shadow opened MR !1202 (chandan/remove-redundant-pytest-mark->master: tests: Remove redundant integration pytest markers) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1202 | 18:41 |
| gitlab-br-bot | cs-shadow closed MR !1201 (chandan/remove-unused-pytestmark->master: tests: Remove unused `pytestmark` variable) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1201 | 18:43 |
| gitlab-br-bot | BenjaminSchubert approved MR !1202 (chandan/remove-redundant-pytest-mark->master: tests: Remove redundant integration pytest markers) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1202 | 18:45 |
| gitlab-br-bot | aevri approved MR !1202 (chandan/remove-redundant-pytest-mark->master: tests: Remove redundant integration pytest markers) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1202 | 18:45 |
| gitlab-br-bot | marge-bot123 merged MR !1200 (bschubert/more-pythonic-list-concat->master: refactor: Use [a, b, *c] instead of [a, b] + c when building list) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1200 | 19:04 |
| tristan | cs-shadow, I don't think that is possible (completions with --user installation, i.e. no system integration), but with some tox.ini hackery, we might get it to work in a shell launched with `tox -e venv -- sh` | 19:36 |
| tristan | could even do some fancy things like make the tox env modify $PS1 and make the shell easy to distinguish, and disable the part where tox installs a package for that env - seems an overall nicer developer experience than `pip3 install --user .` | 19:38 |
| tristan | iiuc, not doing the packaging thing would have the same effect | 19:38 |
| gitlab-br-bot | marge-bot123 merged MR !1195 (remove-dead-code->master: Remove dead code) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1195 | 19:49 |
| cs-shadow | tristan: yeah, that's a bit sad. I am also not sure how practical is it to recommend tox to end-users (which is what I had in mind initally) | 20:24 |
| * cs-shadow needs to go home now | 20:24 | |
| gitlab-br-bot | marge-bot123 merged MR !1202 (chandan/remove-redundant-pytest-mark->master: tests: Remove redundant integration pytest markers) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1202 | 20:31 |
| tristan | cs-shadow, no but ultimately `pip3 install --user .` is not either | 21:23 |
| *** benschubert has quit IRC | 21:38 | |
| gitlab-br-bot | marge-bot123 merged MR !1199 (bschubert/no-subprocess-decode->master: refactor: remove the need to decode output from subprocess) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1199 | 21:40 |
| *** tristan has quit IRC | 22:02 | |
| *** kapil___ has quit IRC | 22:25 | |
| *** benschubert has joined #buildstream | 22:58 | |
| *** Kinnison has quit IRC | 23:05 | |
| *** Kinnison has joined #buildstream | 23:05 | |
| doras[m] | Fancy new logo, I see. | 23:35 |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!