| *** valentind has left #buildstream | 00:04 | |
| *** laurenceurhegyi has quit IRC | 00:09 | |
| *** paulsherwood has quit IRC | 00:10 | |
| *** benbrown has quit IRC | 00:10 | |
| *** laurenceurhegyi has joined #buildstream | 00:11 | |
| *** paulsherwood has joined #buildstream | 00:15 | |
| *** benbrown has joined #buildstream | 00:15 | |
| *** jude has joined #buildstream | 00:25 | |
| *** tristan has joined #buildstream | 06:16 | |
| *** jude has quit IRC | 06:35 | |
| gitlab-br-bot | buildstream: issue #133 ("Bash completions dont complete `--except` arguments") changed state ("closed") https://gitlab.com/BuildStream/buildstream/issues/133 | 07:59 |
|---|---|---|
| gitlab-br-bot | push on buildstream@master (by Tristan Van Berkom): 1 commit (last: _frontend/main.py: Fix #133 - make completions work for --except arguments) https://gitlab.com/BuildStream/buildstream/commit/2780cdbde866a65108d10d030fe7d40015df1b27 | 07:59 |
| gitlab-br-bot | buildstream: merge request (sam/overwrite-list-with-empty-list->master: Allow overwriting a list with an empty list using (=) operator) #141 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/141 | 08:03 |
| gitlab-br-bot | push on buildstream@sam/overwrite-list-with-empty-list (by Tristan Van Berkom): 2 commits (last: _frontend/main.py: Fix #133 - make completions work for --except arguments) https://gitlab.com/BuildStream/buildstream/commit/2780cdbde866a65108d10d030fe7d40015df1b27 | 08:03 |
| *** jude has joined #buildstream | 08:03 | |
| gitlab-br-bot | buildstream: merge request (sam/overwrite-list-with-empty-list->master: Allow overwriting a list with an empty list using (=) operator) #141 changed state ("merged"): https://gitlab.com/BuildStream/buildstream/merge_requests/141 | 08:39 |
| gitlab-br-bot | push on buildstream@master (by Tristan Van Berkom): 1 commit (last: Allow overwriting a list with an empty list using (=) operator) https://gitlab.com/BuildStream/buildstream/commit/34ba445fd1963acada0733c196483c98a57fd753 | 08:39 |
| gitlab-br-bot | buildstream: Sam Thursfield deleted branch sam/overwrite-list-with-empty-list | 08:39 |
| gitlab-br-bot | buildstream: issue #147 ("Compose element loses symlink modifications from integration commands") changed state ("opened") https://gitlab.com/BuildStream/buildstream/issues/147 | 08:57 |
| gitlab-br-bot | buildstream: merge request (compose-integration-delete->master: Handle removed files from integration in compose plugin) #142 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/142 | 08:58 |
| *** adds68 has joined #buildstream | 09:37 | |
| *** tiagogomes has joined #buildstream | 09:53 | |
| *** valentind has joined #buildstream | 09:53 | |
| *** ssam2 has joined #buildstream | 10:02 | |
| *** givascu has joined #buildstream | 10:14 | |
| *** givascu has quit IRC | 10:18 | |
| gitlab-br-bot | buildstream: issue #148 ("Leaked directories when failing to push") changed state ("opened") https://gitlab.com/BuildStream/buildstream/issues/148 | 10:23 |
| *** givascu has joined #buildstream | 10:23 | |
| *** ssam2 has quit IRC | 10:33 | |
| *** ssam2 has joined #buildstream | 10:33 | |
| tlater | tristan: Do you mind mixing old-style tests with new-style tests in the same module? | 10:42 |
| tristan | tlater, Nah I dont mind, they should all become new (I suspect what you mean by module, is in the same test directory) | 10:44 |
| tlater | tristan: Same file, rather than directory | 10:44 |
| tristan | tlater, I migrated the source tests like this; one by one until the last one was gone, and then removed the fixture | 10:44 |
| tristan | tlater, I kind of mind that | 10:45 |
| tristan | yuck :-/ | 10:45 |
| tristan | which are we talking here ? | 10:45 |
| tlater | tristan: pipeline loading | 10:45 |
| tlater | I think the except stuff groups well with that | 10:45 |
| tlater | Though I suppose I can make a separate file for that | 10:45 |
| tlater | tests/pipeline/load.py, to be exact | 10:46 |
| tristan | weird | 10:47 |
| tristan | I wonder why those tests have skipif not HAVE_ROOT ?! | 10:47 |
| tristan | Why would we need root to load a pipeline and check the order ? | 10:47 |
| tlater | tristan: That is pretty weird | 10:48 |
| tristan | tlater, on the other hand, how about pitching in with the technical debt and just convert the whole tests/pipeline/load.py case ? | 10:48 |
| tristan | It looks ridiculously easy to do with `bst show` output and keeping the same test data around | 10:49 |
| tlater | Well, I was going to suggest that, but thought you'd consider merging the except stuff higher priority. | 10:49 |
| tlater | So yeah, happily :) | 10:49 |
| tristan | great, thats better | 10:49 |
| * tlater will also try and see why on earth this has HAVE_ROOT | 10:49 | |
| tristan | yeah that should go, I have no clue why | 10:49 |
| tristan | juergbi, since you're taking over secretary this month, can you just set a wee bit of time aside to figure out how to use GNOME's "Services" bot to do it's meetbot features and use that to log the meeting ? | 11:06 |
| juergbi | will take a look | 11:07 |
| tristan | it should hypothetically make life easier, anyway | 11:07 |
| juergbi | sure | 11:07 |
| gitlab-br-bot | buildstream: merge request (git-element-origin->master: Change origin to point at source repo) #143 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/143 | 11:16 |
| *** nexus has joined #buildstream | 11:39 | |
| gitlab-br-bot | buildstream: Sam Thursfield deleted branch sam/manifests | 11:53 |
| *** valentind has quit IRC | 11:53 | |
| *** jonathanmaw has joined #buildstream | 11:59 | |
| tristan | jonathanmaw, btw, regarding the pep 3102 issue, your API surface is shrinking once again | 12:02 |
| tristan | jonathanmaw, I'm obliterating Context from the public API at the moment | 12:02 |
| jonathanmaw | tristan: good to hear! I couldn't see anything that needed changing, but that's a lot less uncertainty for me to deal with | 12:03 |
| tristan | yeah so the map is basically http://buildstream.gitlab.io/buildstream/pluginauthoring.html#core-framework | 12:04 |
| tristan | but Context will disappear very soon :) | 12:04 |
| * tristan also spotted an inconsistency, or something I find rather weird | 12:05 | |
| tristan | we have --message-lines (amount of lines to print in large messages, typically file overlap warnings for instance) | 12:06 |
| tristan | And we have --error-lines (amount of lines to read from the end of the build log and display when an error occurs) | 12:06 |
| tristan | This seems too much no ? | 12:07 |
| tristan | I mean, couldnt we just have *one* config option / CLI param for these, instead of two ? | 12:07 |
| tristan | Any opinions about this ? and what would be a good name for it ? | 12:08 |
| * tristan cant think of a good reason to really need to configure one without the other | 12:09 | |
| ssam2 | i had never really thought about either | 12:19 |
| ssam2 | the default is obviously pretty OK | 12:20 |
| ssam2 | so yeah, condensing into one option makes sense for me | 12:20 |
| tristan | yeah, I'm in pre 1.0 API cleanup mode | 12:20 |
| tristan | I'm thinking... maybe --context-lines ? | 12:20 |
| tristan | as a name ? | 12:20 |
| tristan | or --max-context-lines | 12:21 |
| tristan | or maybe not context... | 12:21 |
| ssam2 | --log-preview-lines ... ? | 12:24 |
| tlater | Is there any way to get the 'kind' of an element using `bst show`? I have a feeling I won't manage a 100% conversion... | 12:27 |
| gitlab-br-bot | push on buildstream@refactor-private-context (by Tristan Van Berkom): 1 commit (last: Refactor: Move context.py -> _context.py) https://gitlab.com/BuildStream/buildstream/commit/213d9072b684d2dff78d8b4f1c7cfa9d6335b0d0 | 12:27 |
| gitlab-br-bot | buildstream: merge request (refactor-private-context->master: Refactor: Move context.py -> _context.py) #144 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/144 | 12:28 |
| tristan | hrmmmm | 12:35 |
| tristan | preview sounds not bad either | 12:35 |
| tristan | maybe it's a winner with preview vs context | 12:35 |
| tristan | what other logging related options do we have, maybe we can improve consistency | 12:36 |
| tristan | --verbose, --debug, --log-file | 12:37 |
| tristan | ssam2, or just --log-lines maybe ? | 12:38 |
| tristan | --log-max-lines ? | 12:38 |
| ssam2 | those are ok, but the name kind of implies that i might lose some log content altogether if i set it wrong | 12:39 |
| ssam2 | log-preview-lines is too long, but it makes it clear that all the log gets written to disk no matter what value you set it too | 12:39 |
| tristan | Also... there is --colors/--no-colors | 12:41 |
| ssam2 | could reasonably be an env var, since it's only really useful in CI ? | 12:43 |
| tristan | Eh, I rather not have env vars in API at all | 12:44 |
| tristan | Right now we do observe, I think only one | 12:44 |
| tristan | and it's internal to test cases | 12:44 |
| tristan | while running CI, is a use case | 12:45 |
| tristan | (i.e. running CI of a users project) | 12:45 |
| tristan | ssam2, I dont think we should care about that length of cli option name fwiw, bash completions are there for this (and it's not that horribly long either) | 12:48 |
| ssam2 | true | 12:48 |
| tristan | also it's a matter of consistency, seldom used option but user should expect to have similarly named option in CLI as in user config | 12:49 |
| tristan | not dramatically important, although I do find myself using that option | 12:49 |
| tristan | when building full builds of GNOME with `--on-error continue` and `--log-file build.log`; I like to crank that number up | 12:50 |
| * tristan still doesnt know if he likes the word 'preview' that much, though - but takes the point that it's more descriptive | 12:50 | |
| tristan | sigh, names | 12:51 |
| tristan | (/me is jumping back and forth and doing a huge build in the background at the same time, at least) | 12:51 |
| *** paulsherwood has quit IRC | 13:00 | |
| *** benbrown has quit IRC | 13:00 | |
| *** nexus has quit IRC | 13:00 | |
| *** benbrown has joined #buildstream | 13:03 | |
| *** nexus has joined #buildstream | 13:03 | |
| gitlab-br-bot | buildstream: merge request (refactor-private-context->master: Refactor: Move context.py -> _context.py) #144 changed state ("merged"): https://gitlab.com/BuildStream/buildstream/merge_requests/144 | 13:08 |
| *** paulsherwood has joined #buildstream | 13:08 | |
| *** anahuelamo has quit IRC | 13:10 | |
| *** nexus has quit IRC | 13:10 | |
| *** nexus has joined #buildstream | 13:10 | |
| *** anahuelamo has joined #buildstream | 13:10 | |
| gitlab-br-bot | push on buildstream@master (by Tristan Van Berkom): 1 commit (last: Refactor: Move context.py -> _context.py) https://gitlab.com/BuildStream/buildstream/commit/213d9072b684d2dff78d8b4f1c7cfa9d6335b0d0 | 13:13 |
| gitlab-br-bot | buildstream: Tristan Van Berkom deleted branch refactor-private-context | 13:13 |
| *** nexus has quit IRC | 13:14 | |
| *** nexus has joined #buildstream | 13:14 | |
| gitlab-br-bot | push on buildstream@sam/compose-log-splits (by Sam Thursfield): 4 commits (last: _frontend/main.py: Fix #133 - make completions work for --except arguments) https://gitlab.com/BuildStream/buildstream/commit/2780cdbde866a65108d10d030fe7d40015df1b27 | 13:24 |
| gitlab-br-bot | buildstream: merge request (sam/compose-log-splits->master: Log details of artifact splitting when building 'compose' elements) #140 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/140 | 13:24 |
| *** givascu has quit IRC | 13:30 | |
| *** tristan has quit IRC | 13:37 | |
| tlater | Should this be Scope.RUN? https://gitlab.com/BuildStream/buildstream/blob/master/buildstream/element.py#L219 | 13:47 |
| tlater | I get that runtime dependencies of build dependencies are required to build build dependencies | 13:47 |
| tlater | But if that's what this is for then that should be Scope.ALL | 13:47 |
| tlater | And if not it should be Scope.BUILD | 13:48 |
| tlater | ^ line 221 for that | 13:48 |
| * tlater assumes it's correct for now and doesn't touch the related test -- `bst show --scope run` unfortunately shows the behavior of Scope.BUILD there. | 13:56 | |
| *** tristan has joined #buildstream | 14:12 | |
| ssam2 | i have a buildstream where ctrl+c won't make it quit | 14:23 |
| ssam2 | the ticker has also stopped, so that might be related | 14:24 |
| ssam2 | and it's running lots of track and fetch tasks | 14:24 |
| ssam2 | and is also at 100% cpu, and has been there my whole lunch break | 14:24 |
| ssam2 | strace shows it continuously stat()ing the files/ subdir of the project repo (which is gnome-modulesets.git) | 14:27 |
| tlater | ssam2: That's #128 | 14:27 |
| tristan | tlater, what's the issue number for that ^^^^^^^^^^^^^^^ ? | 14:27 |
| tristan | :) | 14:27 |
| tristan | we might promote that to critical I think | 14:27 |
| * tlater hasn't run strace on it yet because I'm stupid, ta ssam2 :) | 14:28 | |
| paulsherwood | tristan: did you mention an alternative to sloccount recently? | 14:29 |
| tristan | paulsherwood, I dont know sloccount, but I think it was radon | 14:30 |
| tristan | it does some interesting features besides SLOC/LLOC counts | 14:30 |
| tristan | ssam2, did you setup a git view http thing on gnome7 ? | 14:31 |
| tristan | ssam2, I forget how to access that | 14:31 |
| ssam2 | yeah | 14:31 |
| ssam2 | https://gnome7.codethink.co.uk/cgit/ | 14:31 |
| tristan | nice ! | 14:31 |
| tristan | thanks :D | 14:31 |
| ssam2 | i guess the continual stat() calls are caused by calling LocalSource.get_unique_key() | 14:32 |
| *** tiagogomes has quit IRC | 14:35 | |
| * tlater has to learn the strace magic | 14:35 | |
| *** tiagogomes has joined #buildstream | 14:35 | |
| ssam2 | `strace -p <PID>` | 14:35 |
| ssam2 | and then you just kinda guess what's going on from first principles | 14:35 |
| tristan | I also learned about python3-dbg package | 14:35 |
| * ssam2 upgrading that right now :-) although i find it sometimes causes GDB to lock up | 14:36 | |
| tristan | on debian anyway, if you install that, and reinstall buildstream with that | 14:36 |
| tristan | yeah, then gdb reports more interesting things | 14:36 |
| tristan | but, you wont be able to infer from an already started process which was not started with the debug python | 14:36 |
| ssam2 | oh, i think it works differently in Fedora | 14:37 |
| tristan | I tried pdb but, I wasnt even able to get off the ground | 14:37 |
| ssam2 | pdb can't attach to running processes AFAIK | 14:37 |
| tristan | lost patience, went for real debugger | 14:37 |
| tlater | That one is a little tricky with an installed package unfortunately - I still haven't figured out how to use pdb sensibly outside of tests. | 14:37 |
| ssam2 | buildstream seems to break pdb, actually | 14:37 |
| ssam2 | i think we must do something to stdin that causes it to exit as soon as it starts | 14:38 |
| tristan | quite possibly, we do system level stuff and fork() without execve() | 14:38 |
| ssam2 | the debug integration seems better in Fedora 26 actually :-) | 14:39 |
| ssam2 | no more lock ups when doing `bt` | 14:39 |
| ssam2 | so, here's my problem: https://paste.baserock.org/johigihoxe :-) | 14:42 |
| tlater | Eww | 14:43 |
| tlater | tristan: Could you explain why we want Scope.RUN here: https://gitlab.com/BuildStream/buildstream/blob/master/buildstream/element.py#L221 ?f | 14:45 |
| * tlater feels like that should probably be Scope.BUILD, but then it has been in the code base for 7+ months | 14:45 | |
| paulsherwood | tristan: radon looks like just the thing, thanks! | 14:48 |
| tpollard | is their any active/documented work in terms of reproducible with buildstream? | 15:01 |
| tpollard | *there | 15:04 |
| ssam2 | all the bits should be there to do bit-for-bit reproducible builds, but i don't know of anyone rigorously doing them | 15:07 |
| ssam2 | "all the bits", hah | 15:07 |
| ssam2 | in particular, SOURCE_DATE_EPOCH is supported: https://gitlab.com/BuildStream/buildstream/blob/master/buildstream/data/projectconfig.yaml#L89 | 15:07 |
| gitlab-br-bot | push on buildstream@except_intersections (by Tristan Maat): 17 commits (last: _pipeline.py: Adjust Planner.plan to multiple targets) https://gitlab.com/BuildStream/buildstream/commit/c05dff5457833afeb9887eee3177daeebc3528bc | 15:10 |
| * ssam2 continues digging into the hang | 15:31 | |
| ssam2 | seems we have been waiting for pid 20970 | 15:32 |
| ssam2 | which was a track of apps/evolution.bst | 15:32 |
| ssam2 | which succeeded | 15:32 |
| ssam2 | there seems to be a callback running in the sigchld handler that is recalculating cache keys | 15:33 |
| tpollard | cheers ssam2 | 15:35 |
| gitlab-br-bot | push on buildstream@remove_dummy_dependency (by Tristan Maat): 1 commit (last: _pipeline.py: Remove DummyElement hack) https://gitlab.com/BuildStream/buildstream/commit/1599d13838d24f214719f7b290ea6e93786d9c9c | 15:39 |
| ssam2 | this make sense so far; the track job completes, and then we recalculate all cache keys | 15:41 |
| tristan | tlater, was having very late supper sorry I missed your question :-/ | 15:41 |
| tlater | tristan: nw :) | 15:41 |
| ssam2 | however, if i `continue`, wait a while and `break`, it looks like we are still always recalculating the cache keys for apps/evolution.bst | 15:42 |
| tristan | tlater, thats an easy answer, it's a recursive function; what does the docs say about build time dependencies again ? | 15:42 |
| tlater | tristan: I know that they include their RUN dependencies for builds, but don't they need their BUILD dependencies? | 15:43 |
| tristan | Absolutely not | 15:43 |
| tristan | tlater, to build an element, you need the build time dependencies of that element only, to run | 15:43 |
| tristan | i.e. the build time dependencies of the element need to run | 15:44 |
| tristan | tlater, following build-of-build ends up with... all; then you completely lose the concept of "I only needed this to build something, so it goes out of scope when I need to run that something" | 15:45 |
| tlater | Ah, alright. The odd thing is then that `bst show --scope build` *does* include build dependencies, but not run dependencies | 15:45 |
| gitlab-br-bot | push on buildstream@remove_dummy_dependency (by Tristan Maat): 4 commits (last: _frontend/main.py: Fix #133 - make completions work for --except arguments) https://gitlab.com/BuildStream/buildstream/commit/2780cdbde866a65108d10d030fe7d40015df1b27 | 15:45 |
| tristan | tlater, it includes build dependencies *of the element* | 15:46 |
| tristan | tlater, not build dependencies of build dependencies of the element | 15:46 |
| tristan | Right ? | 15:46 |
| tlater | tristan: `bst show --deps all --scope build` includes build dependencies of build dependencies of the element - but maybe that's expected? | 15:46 |
| tristan | Unless of course, they are *also* runtime dependencies of build time dependencies (which is the default) | 15:46 |
| tristan | what the hell | 15:48 |
| gitlab-br-bot | push on buildstream@except_intersections (by Tristan Maat): 1 commit (last: load.py: Migrate to new test style (WIP)) https://gitlab.com/BuildStream/buildstream/commit/1e16eada580ee3aa941f0b6de08792eba20b4104 | 15:48 |
| tristan | tlater, there is no `--scope` argument to `bst show` | 15:48 |
| tristan | there is no --scope argument, anywhere afaict | 15:49 |
| tristan | that might have existed months ago, before I implemented proper symmetry for the --deps argument | 15:49 |
| tlater | tristan: ... For some reason I was convinced I was calling that, but I actually meant `--deps` | 15:49 |
| tristan | tlater, if there is a bug there is a bug, the way you are explaining it; I'm not convinced there is a bug | 15:50 |
| * tlater isn't either, but I'm at least sure the bug isn't where I thought it may be :) | 15:50 | |
| jonathanmaw | tristan: hrm, looking at the methods we expose, I can't find Plugin.node_provenance() being used anywhere | 15:52 |
| tristan | jonathanmaw, not sure if that is worth removing | 15:53 |
| tristan | the semantics of node_get() only assert that the loaded stuff is of the expected type | 15:54 |
| tristan | so if you need to print a load time error because of something else that is wrong with the data, you need that | 15:54 |
| tristan | maybe if it's not there, we might end up raising an ElementError or SourceError at Plugin.configure() time while neglecting to use the readily available node provenance | 15:55 |
| ssam2 | so the cache key recalculation that keeps happening seems to be driven by PullQueue.skip() | 15:57 |
| ssam2 | the situation my process is in seems to be... evolution.bst tracked successfully, so it's added to the pull queue, so we check if it's cached, so we calculate its cache key | 15:57 |
| tristan | ssam2, that happens for various reasons | 15:57 |
| ssam2 | i've followed the backtrace up from pipeline.build() to here | 15:58 |
| tristan | ssam2, we definitely need to improve the code for reasoning around when a cache key cannot be calculated, and when it becomes ready to be calculated | 15:58 |
| ssam2 | ok | 15:58 |
| tristan | ssam2, I also noticed that when you are lacking a cache and sources - and you run `bst anything`, thats when you hit that regression where you load...... then at the end you have <unexplained pause>, then show the pipeline summary | 15:59 |
| tristan | that is after calculating keys the first time, and before showing the summary; certainly before any queues are in play | 16:00 |
| gitlab-br-bot | push on buildstream@except_intersections (by Tristan Maat): 1 commit (last: load.py: Migrate to new test style (WIP)) https://gitlab.com/BuildStream/buildstream/commit/27b9790058d4f5e43de681a5b29c1c471dab6437 | 16:00 |
| tlater | tristan: Gah, looks like it's a regression from the multiple targets branch, because I added an intermediate dummy element... I already have a branch that fixes that because I realized there was a better way to do that. | 16:00 |
| tristan | tlater, then that just became the priority | 16:00 |
| tristan | backgammon | 16:00 |
| tristan | tlater, lets use this upgrade of the test case to fix that :) | 16:01 |
| tlater | tristan: Include it in the `--except` branch then? | 16:01 |
| tristan | tlater, I would prefer: take that upgraded test case, and the fix; rebase your --except stuff on top of that, which is right now the fire in the building | 16:02 |
| ssam2 | ok yes, I think the infinite loop we're in is that Element._get_cache_key() never returns | 16:11 |
| ssam2 | so makes sense that the bug is to do with knowing when we can actually calculate a cache key | 16:12 |
| ssam2 | i think i have enough info to file at least a semi-useful bug report now | 16:12 |
| tlater | \o/ | 16:13 |
| tlater | Perhaps making an entire new one is worth it; the other bug report was somehow conflated with 2 others. | 16:13 |
| tristan | ssam2, great one sec... I have a note for you... | 16:16 |
| ssam2 | the other bug report seems actually unrelated | 16:17 |
| tristan | ssam2, https://gitlab.com/BuildStream/buildstream/merge_requests/126#note_46286435 | 16:17 |
| tristan | with incremental builds, this is going to get *even more tricky* | 16:17 |
| ssam2 | hooray! | 16:17 |
| tristan | we have a /somewhat/ reasonable functionality... but I dont think I feel comfortable with that complexity hiding in a private function in element.py | 16:18 |
| tristan | we need to rethink how we cache and reason about this state, that comment suggests a bit more boiler plate and care around that | 16:18 |
| ssam2 | right | 16:19 |
| tristan | I dont know how or what, maybe it's a delegate object, maybe not | 16:19 |
| tristan | the pieces of the puzzle are there, but we've been a bit too nonchalant about state | 16:19 |
| tristan | just a tad ;-) | 16:19 |
| gitlab-br-bot | push on buildstream@except_intersections (by Tristan Maat): 2 commits (last: load.py: Migrate to new test style (WIP)) https://gitlab.com/BuildStream/buildstream/commit/df4e905c1eef7ea2b2752a57356342f77ce2480e | 16:21 |
| gitlab-br-bot | push on buildstream@except_intersections (by Tristan Maat): 20 commits (last: doc/source/formatintro.rst: Fixed duplicate link anchor) https://gitlab.com/BuildStream/buildstream/commit/0363cd99497e2c620d75f4a60f6ca39ad37fac56 | 16:23 |
| jonathanmaw | hrm, looking at Plugin.node_get_member, we're using its default_value arg as a positional and a keyword arg in the codebase | 16:36 |
| gitlab-br-bot | buildstream: issue #149 ("Busyloop while calculating cache keys in `bst build --track`") changed state ("opened") https://gitlab.com/BuildStream/buildstream/issues/149 | 16:36 |
| jonathanmaw | and we use it positionally more than as a keyword | 16:36 |
| tristan | jonathanmaw, something to think about: How do the docs get rendered for these changes ? | 16:38 |
| tristan | jonathanmaw, can we also improve the doc strings to that it is perfectly clear what is A.) A positional argument... B.) An optional positional argument or kwarg... C.) A kwarg only ? | 16:39 |
| tristan | with sphinx | 16:39 |
| jonathanmaw | hrm, I'll have to have a look | 16:39 |
| tristan | jonathanmaw, I feel like that one in particular, deserves to be an optional arg | 16:39 |
| tristan | I think I made an example of exactly that method in the bug report or MR comments | 16:39 |
| tristan | jonathanmaw, I think it's also worth renaming that, perhaps s/default_value/default | 16:40 |
| gitlab-br-bot | push on buildstream@migrate_pipeline_load (by Tristan Maat): 2 commits (last: _pipeline.py: Remove DummyElement hack) https://gitlab.com/BuildStream/buildstream/commit/d9d84097d71e6fe582c23f9eba1099f9ca171916 | 16:43 |
| tristan | I think the only time I've been writing out default_value="foo" in Plugin.node_get_member(), is because of how many times I've been bitten by calling _yaml.node_get() with a positional default value, and it being interpreted as the 'indices' param instead | 16:44 |
| gitlab-br-bot | buildstream: merge request (migrate_pipeline_load->master: Migrate `tests/pipeline/load.py`) #145 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/145 | 16:45 |
| gitlab-br-bot | push on buildstream@migrate_pipeline_load (by Tristan Maat): 1 commit (last: load.py: Migrate to new test style) https://gitlab.com/BuildStream/buildstream/commit/9921b72dd7d517fcd43561a79985b9c656594737 | 16:45 |
| gitlab-br-bot | buildstream: merge request (migrate_pipeline_load->master: Migrate `tests/pipeline/load.py`) #145 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/145 | 16:45 |
| tristan | tlater, there is a problem with that | 16:48 |
| tristan | tlater, look at this line: https://gitlab.com/BuildStream/buildstream/merge_requests/145/diffs#029d46ba7e1b2197fd4337e31e3d8422a390dad5_216_200 | 16:48 |
| tristan | which you just changed to track 'visited' outside of Element.dependencies() | 16:49 |
| tristan | tlater, then look at this line: https://gitlab.com/BuildStream/buildstream/blob/master/buildstream/element.py#L200 | 16:49 |
| tristan | tlater, which presumes that the function is recursing because visited was set | 16:49 |
| tlater | Ack | 16:49 |
| tristan | the visited parameter (which is hidden from the docs) is really a private detail of that function, however it doesnt mean there is no way to internally cooperate with that | 16:50 |
| tristan | you are probably on the right track | 16:50 |
| tristan | but need to detect recursion separately | 16:50 |
| tristan | sorry for hovering over your shoulders from the other side of the globe ;-) | 16:51 |
| * tristan backs off now ;-P | 16:51 | |
| tlater | tristan: Heh, thanks :) | 16:51 |
| tristan | if we have better tests which ensure this is really correct as a result of this exercise, that will be really great :) | 16:52 |
| gitlab-br-bot | buildstream: Tristan Maat created branch except_intersections | 16:53 |
| jonathanmaw | tristan: looks like "*" appears in the list of args that gets generated. It's in good company with the python docs, though | 16:54 |
| jonathanmaw | for example, https://docs.python.org/3/library/multiprocessing.html#process-and-exceptions | 16:54 |
| jonathanmaw | or rather, https://docs.python.org/3/library/multiprocessing.html#multiprocessing.Process | 16:54 |
| tristan | jonathanmaw, thanks for checking it out :) | 17:04 |
| tristan | looks good enough anyway | 17:04 |
| tlater | It looks like this case is going to be hard to test, since it only results in issues if recurse=False is set | 17:10 |
| tlater | The frontend can't do that atm | 17:10 |
| tlater | tristan: How do you feel about a `--deps direct` flag? | 17:10 |
| gitlab-br-bot | push on buildstream@migrate_pipeline_load (by Tristan Maat): 2 commits (last: Remove DummyElement hack) https://gitlab.com/BuildStream/buildstream/commit/8b3831b68bef9537d4b41158d1265c1848414667 | 17:20 |
| gitlab-br-bot | buildstream: merge request (migrate_pipeline_load->master: Migrate `tests/pipeline/load.py`) #145 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/145 | 17:20 |
| tristan | tlater, what does "direct" mean ? | 17:55 |
| tristan | tlater, oh you mean, just one level of dependencies | 17:55 |
| tlater | tristan: Only exactly the dependencies of the target element, but not their dependencies | 17:55 |
| tristan | that would kinda be weird wouldnt it | 17:55 |
| tristan | this is to test the specific edge case I raised ? | 17:55 |
| tlater | tristan: Yup, basically. I don't see another way to do it | 17:56 |
| tristan | are you really sure its only an issue when recurse is false ? | 17:56 |
| tlater | tristan: Yes, because in all other cases recurse just overrides anything did_recurse tries to say. | 17:56 |
| tristan | I was rather thinking of when you wanted build dependencies of two elements, one which build depends on the other | 17:57 |
| tristan | not sure but that was my intuition of where it might break | 17:57 |
| tristan | tlater, to be honest I dont care about testing that edge case so much - it looks like its impossible to hit if your findings are correct | 17:58 |
| tlater | tristan: It can potentially be hit in the planner | 17:59 |
| tlater | But I can't figure out a pipeline that does it :/ | 17:59 |
| tristan | from a real world standpoint, if the frontend cannot reach a given case, then that case cannot present a bug, so it's not testable | 17:59 |
| tristan | tlater, `bst show --deps plan` ? | 18:00 |
| tlater | tristan: Yup, that's what I'm trying to use now | 18:00 |
| tristan | tlater, it depends on which of the targets get fed to Element.dependencies() first | 18:01 |
| tristan | tlater, I think that if you have 2 targets A and B, where A depends on B... and you want the build dependencies of both A and B... | 18:02 |
| tristan | then one of them would suffer | 18:02 |
| *** valentind has joined #buildstream | 18:02 | |
| tristan | tlater, in which case, a test case which does `bst show --deps build a.bst b.bst` AND `bst show --deps b.bst a.bst` (assuming that effects the order here)... should ensure that bad things wont happen | 18:03 |
| *** jonathanmaw has quit IRC | 18:03 | |
| tristan | i.e. they should both produce the same result | 18:03 |
| tlater | tristan: I am fairly sure in that case did_recurse is not even evaluated | 18:03 |
| tristan | tlater, and probable both A and B need to have individual build-only dependencies to hit it | 18:03 |
| tristan | Well, it's not like I've been sitting here thinking about it | 18:04 |
| tlater | https://gitlab.com/BuildStream/buildstream/blob/master/buildstream/element.py#L210 and https://gitlab.com/BuildStream/buildstream/blob/master/buildstream/element.py#L229 are the only spots where that variable is hit | 18:04 |
| tlater | tristan: Yeah, it's alright, I think I can figure it out a pipeline that hits this with a bit more thought. | 18:05 |
| gitlab-br-bot | buildstream: merge request (compose-integration-delete->master: Handle removed files from integration in compose plugin) #142 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/142 | 18:05 |
| tristan | tlater, it's possibly not worth the effort, if you fixed the regression and are testing the dependency order much better than we ever did before; that's already great | 18:06 |
| tristan | of course, thorough testing of these core things make us much safer in the long run, though | 18:06 |
| tristan | maybe I have a hope in hell of ever passing on the maintainer hat to someone else, if things are really well tested :D | 18:07 |
| tlater | One day... ;P | 18:08 |
| * tlater will probably have to think this through again tomorrow... Brain is a little fried by now | 18:08 | |
| gitlab-br-bot | push on buildstream@migrate_pipeline_load (by Tristan Maat): 2 commits (last: load.py: Migrate to new test style) https://gitlab.com/BuildStream/buildstream/commit/d9a876eb2565497b463f0df876a6d48bf18e95e6 | 18:11 |
| gitlab-br-bot | buildstream: merge request (migrate_pipeline_load->master: Migrate `tests/pipeline/load.py`) #145 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/145 | 18:11 |
| tristan | tlater, really though, maybe it's not the best approach to testing | 18:12 |
| tristan | tlater, I mean, I pointed out a piece of code that could potentially cause things to be incorrect; that you probably fixed by now by treating 'visited' and 'did_recurse' separately | 18:13 |
| tristan | that's pretty much a no brainer | 18:13 |
| tristan | tlater, going ahead and trying to guess at "what exactly might that have broken" and testing for that case, is not immensely productive | 18:13 |
| tristan | i.e. its better to start with just knowing that pipelines testing various features produce the results we expect, on a wide variety of combinations | 18:15 |
| tristan | a @pytest.parameterize() case with (target.bst, expected_ordered_list) tuples which just does a ton of interesting things is a much better use of effort | 18:16 |
| tristan | I'm very happy with what I did here for instance: https://gitlab.com/BuildStream/buildstream/blob/master/tests/yaml/yaml.py#L257 | 18:17 |
| tristan | if we had a test battery for each --deps option of `bst show`, using single and multiple targets; thoroughly testing the surface; that makes us damn safe in general | 18:18 |
| tlater | Yeah, I suppose having something like that and then just adding freak cases when they happen is probably better. | 18:18 |
| tlater | In any case, ta for the help tristan :) | 18:22 |
| * tlater will have to go | 18:22 | |
| tlater | o/ | 18:22 |
| tristan | later o/ | 18:23 |
| gitlab-br-bot | push on buildstream@sam/git-fetch-prune (by Sam Thursfield): 2 commits (last: Allow overwriting a list with an empty list using (=) operator) https://gitlab.com/BuildStream/buildstream/commit/cb6df173ef00b28bdc5c0055fbd6f111d23217e9 | 18:55 |
| gitlab-br-bot | push on buildstream@sam/git-fetch-prune (by Sam Thursfield): 4 commits (last: _frontend/main.py: Fix #133 - make completions work for --except arguments) https://gitlab.com/BuildStream/buildstream/commit/2780cdbde866a65108d10d030fe7d40015df1b27 | 18:59 |
| gitlab-br-bot | buildstream: merge request (sam/git-fetch-prune->master: git.py source plugin: Prune remote-tracking branches when fetching) #146 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/146 | 18:59 |
| *** tristan has quit IRC | 19:23 | |
| *** laurenceurhegyi has quit IRC | 19:23 | |
| *** tlater has quit IRC | 19:23 | |
| *** tristan has joined #buildstream | 19:23 | |
| tristan | valentind, sorry I didnt make a full review of that MR today as the test was failing, will do tomorrow | 19:30 |
| tristan | valentind, I did open a related bug report though: https://gitlab.com/BuildStream/buildstream/issues/147 | 19:31 |
| tristan | and tried to detail the issue | 19:31 |
| valentind | That is fine. My fault not checking "git status" before pushing. | 19:32 |
| valentind | tristan, would integration commands the use install-root variable? | 19:33 |
| valentind | I am not sure how a EEXIST would be triggered. | 19:34 |
| tristan | valentind, I'm not sure if EEXIST is actually triggered, when I mention that, I'm referring to when you call utils.move_files() in the compose plugin, but the file has already been moved (the source of it is then missing) | 19:35 |
| tristan | I suppose it would be an EEXIST | 19:35 |
| *** laurenceurhegyi has joined #buildstream | 19:36 | |
| valentind | I scan for missing files before calling move_files. | 19:37 |
| tristan | valentind, that is likely to happen when say, two artifacts contain the same file (overlaps), or when integration commands modified a file (so it changed, and was moved once by virtue of split rules, and then moved again by virtue of being a file modified by integration commands) | 19:37 |
| tristan | Meh, in that case I guess it's unneeded - it might be simpler to ignore the error, though | 19:37 |
| tristan | or, maybe you compose a list of everything you wanna move and just call move_files once | 19:38 |
| valentind | Ah, I use a set. So paths given to move_files are unique. | 19:38 |
| tristan | I did take a brief look, I'm not sure I want to expose that splits API in that form | 19:38 |
| tristan | maybe just a rename or maybe some change for public API... I'll give it some thought when it's not 4:30am :D | 19:39 |
| valentind | OK | 19:39 |
| *** tlater has joined #buildstream | 19:43 | |
| *** ssam2 has quit IRC | 20:04 | |
| *** valentind has quit IRC | 23:00 | |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!