| *** mcatanzaro has quit IRC | 00:34 | |
| *** persia has joined #buildstream | 00:49 | |
| *** tristan has joined #buildstream | 04:41 | |
| *** noisecell has joined #buildstream | 07:29 | |
| gitlab-br-bot | buildstream: merge request (175-refactor-integration-tests->master: WIP: Resolve "Refactor integration tests") #233 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/233 | 08:27 |
|---|---|---|
| tristan | juergbi, see my last comment on incremental builds: https://gitlab.com/BuildStream/buildstream/merge_requests/126#note_55052953 | 08:39 |
| juergbi | ok, reading | 08:40 |
| tristan | juergbi, I think that MR is ready to land with a one liner warning and a rebased/fixed history; I'd like to hand off the final review if you can take it from there | 08:40 |
| tristan | note that it adds an integration test, I feel like better to get that in in advance of the test refactoring; and take care of the rebase on our side | 08:41 |
| juergbi | hm, recalculate and stages | 08:43 |
| tristan | recalculate is back ? | 08:45 |
| juergbi | for workspace sources | 08:46 |
| juergbi | that's a tricky aspect, not sure whether there is a better solution | 08:46 |
| tristan | right - Well - another approach would beeeeee.... not have source.py responsible for caching it ? | 08:46 |
| juergbi | possibly | 08:46 |
| juergbi | need to take a closer look | 08:47 |
| tristan | juergbi, the comment I made on stages stands; he had it the other way, I said I wanted to think about it; at this point it looks trivial enough that I dont feel like burdening the contributor with that | 08:47 |
| tristan | I dont feel like I have a stronger opinion on it than you do at the moment fwiw, so I'm happy to let you have that one :) | 08:48 |
| tristan | recalculate coming back is something not awesome though | 08:49 |
| juergbi | tristan: i.e., if i can think of a clean modification of the patch without stages, either variant is fine with you? | 08:50 |
| juergbi | yes, i'll give that some thoughts | 08:50 |
| tristan | juergbi, maybe just an explicit wipe method would be not bad, assuming it's always accessed via Element._update_state() | 08:50 |
| tristan | juergbi, yeah either variant with stages or without, you have a stronger opinion than I do on that front | 08:51 |
| juergbi | invalidate_workspace_key or something | 08:51 |
| tristan | juergbi, but since he already swapped it around I felt it kinder to not ask him to swap it back | 08:51 |
| juergbi | probably better an explicit name than generic recalculate | 08:51 |
| tristan | indeed | 08:52 |
| juergbi | yes, that makes sense | 08:52 |
| *** aiden has joined #buildstream | 09:06 | |
| tristan | oh this is interesting | 09:08 |
| tristan | juergbi, ... | 09:09 |
| tristan | A.) Clone gnome-modulesets from gnome7 | 09:09 |
| tristan | B.) bst build --track-all code-deps/evolution-data-server.bst | 09:09 |
| tristan | C.) Consistently reproduce: https://gitlab.com/BuildStream/buildstream/blob/master/buildstream/_scheduler/fetchqueue.py#L63 | 09:10 |
| tristan | that is you ! | 09:10 |
| tristan | hehe | 09:10 |
| tristan | Sorry (B) (until jmac fixes it! \o/)... should actually be: bst build --track-all code-deps/evolution-data-server.bst | cat | 09:11 |
| *** aidenhjj has joined #buildstream | 09:12 | |
| juergbi | oh, will take a look | 09:12 |
| tristan | lemme see maybe it's a specific element | 09:14 |
| tristan | cause I'm only getting it once | 09:14 |
| *** aiden has quit IRC | 09:14 | |
| tristan | yep | 09:15 |
| tristan | juergbi, it's a local source | 09:15 |
| tristan | juergbi, maybe you reversed the original assumption that a source is cached until it's not :) | 09:15 |
| tlater | tristan: I think the integration tests are finally ready to merge | 09:16 |
| juergbi | linker-priority? | 09:16 |
| tristan | yep | 09:16 |
| juergbi | i don't think i have | 09:16 |
| * tlater did not write a custom stdout-tee thing because it is actually quite difficult, may be worth an issue | 09:16 | |
| tlater | But otherwise I addressed ssam2's comments | 09:16 |
| tristan | tlater, I have a desire to land cs_shadow's branch first if that can happen quickly... | 09:17 |
| gitlab-br-bot | buildstream: merge request (175-refactor-integration-tests->master: Resolve "Refactor integration tests") #233 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/233 | 09:17 |
| tlater | Yeah, that's alright :) | 09:17 |
| *** aidenhjj has quit IRC | 09:18 | |
| tristan | juergbi, originally we had it None initially to recognize the uninitialized state | 09:19 |
| juergbi | ah i know what it is | 09:20 |
| juergbi | tracking a non-trackable source | 09:20 |
| juergbi | issue with state variable ;) | 09:20 |
| tristan | yes | 09:20 |
| juergbi | we mark it as being tracked but because there is never a set_ref with a local source, it will never be unmarked as being tracked | 09:20 |
| tristan | I wonder why our tests dont handle that... hmmmm | 09:20 |
| tristan | I guess I only tried exercising the trackable ones in the generic tests | 09:21 |
| juergbi | is this urgently blocking anything? i'll fix it today anyway but wanted to finish other things first | 09:22 |
| tristan | Errr myeaaah okay | 09:22 |
| tristan | Ummm, I guess I can work with 1.0 :) | 09:22 |
| * tristan checks out 1.0 | 09:22 | |
| juergbi | ok, i guess i'll do this next anyway | 09:22 |
| tristan | it's certainly on the Critical level | 09:23 |
| tristan | burning fire | 09:23 |
| tristan | widening window that it might happen to a user | 09:23 |
| gitlab-br-bot | buildstream: merge request (175-refactor-integration-tests->master: Resolve "Refactor integration tests") #233 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/233 | 09:24 |
| juergbi | ok, should be easy to fix but i'll have to add an Element/Source method corresponding to the inverse of schedule_tracking that i can call in the TrackQueue | 09:25 |
| juergbi | a possibly better solution would be to recognize sources that can't be tracked | 09:26 |
| tristan | juergbi, Why is it not enough that Source.get_consistency() *always* returns Consistency.CACHED ? | 09:28 |
| juergbi | but that information is not currently available for source plugins | 09:28 |
| juergbi | well, how do i know that it always returns that and not just with the current value? | 09:28 |
| tristan | Why is that important ? | 09:28 |
| tristan | Oh wait, you mean track | 09:28 |
| tristan | juergbi, the assertion is in fetch, btw | 09:29 |
| tristan | not track | 09:29 |
| juergbi | yes but it's due to a track issue | 09:29 |
| tristan | I dont know how it properly escaped track's evil clutches | 09:29 |
| tristan | Ahhhhh | 09:29 |
| tristan | You assume a source needs to be fetched after tracking, *even* if it reports Consistency.CACHED ? | 09:30 |
| tristan | That's the bug. | 09:30 |
| juergbi | no, i didn't | 09:30 |
| juergbi | i wrongly assumed set_ref would be called in track done for every source | 09:30 |
| tristan | Well I dont see what's wrong here at least in theory, then | 09:30 |
| juergbi | and marked Source as no longer being tracker in set_ref() | 09:31 |
| tristan | I'm quite certain that there is no need for a Source to report anything more | 09:31 |
| juergbi | for local sources, track() returns None, which means that set_ref is never called | 09:31 |
| juergbi | because it's not returned in the list to the main process | 09:31 |
| juergbi | we could theoretically add all sources there | 09:31 |
| tristan | sounds like internal churn yes | 09:31 |
| * tristan was mostly around to defend the point that Sources need *not* be marked with some additional sauce | 09:32 | |
| *** valentind has joined #buildstream | 09:42 | |
| *** noisecell has quit IRC | 09:43 | |
| *** jonathanmaw has joined #buildstream | 09:43 | |
| *** aiden has joined #buildstream | 09:43 | |
| gitlab-br-bot | buildstream: issue #198 ("Cross-platform testing is not truly cross-platform") changed state ("opened") https://gitlab.com/BuildStream/buildstream/issues/198 | 09:49 |
| *** ssam2 has joined #buildstream | 10:07 | |
| gitlab-br-bot | buildstream: issue #199 ("Test suite buildstream imports may be ambiguous") changed state ("opened") https://gitlab.com/BuildStream/buildstream/issues/199 | 10:37 |
| tristan | tlater, thanks for doing the secretary boring stuff :) | 10:38 |
| tlater | ;) | 10:38 |
| gitlab-br-bot | buildstream: issue #200 ("bst workspace open creates the directory and then fails.") changed state ("opened") https://gitlab.com/BuildStream/buildstream/issues/200 | 10:44 |
| gitlab-br-bot | buildstream: merge request (issue-182_Closing_non-existing_workspace->master: Fix for issue 182 Closing non-existing workspace) #227 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/227 | 10:48 |
| * tristan uses workspaces to test a patch from Milan to fix the EDS build | 10:49 | |
| *** valentind has quit IRC | 10:49 | |
| *** ernestask has joined #buildstream | 11:14 | |
| gitlab-br-bot | buildstream: issue #201 ("bst track reports strange output for untrackable elements") changed state ("opened") https://gitlab.com/BuildStream/buildstream/issues/201 | 11:14 |
| gitlab-br-bot | buildstream: merge request (juerg/tracking->master: Fix --track with local sources) #244 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/244 | 11:22 |
| juergbi | ah, i actually fixed #201 in that as well | 11:23 |
| tristan | juergbi, I filed a bug in mid-fix ? | 11:23 |
| tristan | heh | 11:23 |
| juergbi | yes, adding the number to the commit/MR | 11:24 |
| tristan | ah cool, so I dont have to file --track with local sources | 11:24 |
| gitlab-br-bot | buildstream: merge request (juerg/tracking->master: Fix --track with local sources) #244 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/244 | 11:25 |
| gitlab-br-bot | buildstream: merge request (juerg/tracking->master: Fix --track with local sources) #244 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/244 | 11:25 |
| juergbi | no, both should be fixed by !244 | 11:25 |
| juergbi | i haven't added new test cases, though. might not have time for that today | 11:26 |
| tristan | juergbi, no worry; at least it's fixed | 11:30 |
| tristan | But, it *would* be great to add one too - seems like a good thing to cover | 11:30 |
| gitlab-br-bot | buildstream: merge request (sam/integration-tests-alpine->master: WIP: Use a custom base sysroot for the integration tests) #243 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/243 | 11:44 |
| *** tristan has quit IRC | 11:49 | |
| gitlab-br-bot | buildstream: issue #201 ("bst track reports strange output for untrackable elements") changed state ("closed") https://gitlab.com/BuildStream/buildstream/issues/201 | 12:01 |
| gitlab-br-bot | buildstream: merge request (juerg/tracking->master: Fix --track with local sources) #244 changed state ("merged"): https://gitlab.com/BuildStream/buildstream/merge_requests/244 | 12:01 |
| ssam2 | writing integration tests on top of !233 is infinitely nicer than with the old setup | 12:47 |
| ssam2 | i say infinitely because previously one of the tests i needed was basically impossible :-) | 12:47 |
| tlater | \o/ | 12:50 |
| gitlab-br-bot | buildstream: merge request (incremental-build->master: WIP: Add support for doing incremental builds) #126 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/126 | 14:06 |
| *** mcatanzaro has joined #buildstream | 14:06 | |
| gitlab-br-bot | buildstream: merge request (incremental-build->master: WIP: Add support for doing incremental builds) #126 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/126 | 14:09 |
| gitlab-br-bot | buildstream: merge request (incremental-build->master: Add support for doing incremental builds) #126 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/126 | 14:29 |
| *** tlater has left #buildstream | 14:31 | |
| *** tlater has joined #buildstream | 14:31 | |
| * tlater just noticed something about a 'debugging widget' - what is that? | 15:04 | |
| ssam2 | where ? | 15:04 |
| tlater | In widget.py - presumably it controls some form of output | 15:04 |
| tlater | I just have no idea how I would run that | 15:05 |
| tlater | Ah, I guess it's what shows --debug input | 15:06 |
| * tlater wasn't aware of that flag, but notes it for future debugging | 15:06 | |
| *** benzea has joined #buildstream | 15:24 | |
| cs_shadow | Hi folks, is it a known issue that unix integration tests are slower than linux integration tests or do I have a really bad luck with those tests? I've noticed this pattern multiple times, for instance https://gitlab.com/bloomberg/bst/buildstream/-/jobs/48526803 finished in ~16 mins whereas https://gitlab.com/bloomberg/bst/buildstream/-/jobs/48526805 took over 50 mins | 15:47 |
| persia | cs_shadow: I think it is slower for non-linux, as some of the linux-only features are worked around clunkily, but I'm not authoritative. | 15:51 |
| tlater | cs_shadow: That is a known issue - I think it's mostly been solved by the integration refactoring | 15:51 |
| tlater | Or at least, it's a lot less extreme now | 15:51 |
| cs_shadow | good to know, thanks! | 15:52 |
| *** jude has quit IRC | 17:04 | |
| *** jude has joined #buildstream | 17:12 | |
| gitlab-br-bot | buildstream: merge request (164-minimise-overlaps-by-having-overlaps-raise-exceptions-unless-configured-not-to->master: Resolve "Minimise overlaps by having overlaps raise exceptions unless configured not to") #181 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/181 | 17:53 |
| *** ssam3 has joined #buildstream | 18:14 | |
| *** ssam2 has quit IRC | 18:14 | |
| gitlab-br-bot | buildstream: merge request (issue-166_yaml_removing_underscores->master: WIP: Issue 166 yaml removing underscores) #245 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/245 | 18:45 |
| persia | Stylistic note: do we care about spurious whitespace changes as part of merge requests? | 18:48 |
| *** jonathanmaw has quit IRC | 18:50 | |
| juergbi | we generally do, yes | 18:55 |
| juergbi | should be fixed | 18:55 |
| juergbi | nexus: also, gitlab issues are referenced with # not ! (the latter is for MRs) | 18:55 |
| tlater | About #166, do we know why the ref changes? | 19:05 |
| tlater | I thought this was because it successfully tracks and writes back the new ref, but while doing so also writes a modified 'track' target because of the ruamel bug. | 19:06 |
| persia | I thought it was because BuildStream had internal storage as an int, so we were dependent on how the int is formatted to have it be the ref string or some other string. | 19:06 |
| persia | If BuildStream successfully tracks internally, that's a happy accident of how the call to git is made. | 19:06 |
| persia | In that the method invoked for int->string when checking out a branch happens to recreate the same sequence of characters as is found in the input. | 19:07 |
| tlater | Right, I see | 19:07 |
| tlater | I thought only the 'track' field was affected | 19:07 |
| persia | That's just how we found the bug. The internal variable representation is the issue (IIUC) | 19:08 |
| *** ssam3 has quit IRC | 19:08 | |
| tlater | I *thought* ruamel's representation was the issue, but that makes sense | 19:08 |
| juergbi | my suspicion is that track is interpreted as integer as 1231 | 19:09 |
| * tlater wrote that issue with a bit of speculation | 19:09 | |
| juergbi | and git then resolves this to the commit with that prefix | 19:09 |
| persia | Yes. ruamel's representation was the issue, and BuildStream imports part of ruamel to parse yaml, so BuildStream inherits ruamel's represtation. | 19:09 |
| persia | +en | 19:09 |
| tlater | Yup, that makes sense, ta for the clarification | 19:10 |
| juergbi | tlater: or to be specific, 1231 points to a blob, not to a commit | 19:10 |
| juergbi | that's why you then get the error | 19:11 |
| tlater | Yeah, I was never sure why that happened, and too lazy to actually check if I got the correct commit. This finally explains the whole issue \o/ | 19:11 |
| juergbi | thinking about that, maybe we could also change the git plugin to only ever resolve to commits | 19:12 |
| persia | Which makes !245 more useful to consider | 19:12 |
| persia | juergbi: Would it also be able to resolve branch names and tag names to commits? | 19:12 |
| juergbi | and maybe even disallow abbreviated commit ids or at the very least require more than 4 digits | 19:12 |
| juergbi | yes, we definitely need this for track | 19:12 |
| juergbi | we just don't want to resolve 'track' to a blob | 19:13 |
| persia | I think disallowing abbreviations is important: it isn't hard to get the full SHA1, and anything smaller should be a branch or tag name. | 19:13 |
| juergbi | i'd also tend in that direction | 19:13 |
| persia | There's just too much chance that an abbreviation is reused by a future commit, without the bst file author being aware. | 19:13 |
| persia | (especially for things that bst early, where the abbreviation may only be four or five characters) | 19:13 |
| * tlater had a long look through our test coverage, and also thinks that "interactive" testing would improve things a bit | 19:40 | |
| tlater | Lots of exception handling that isn't tested, but only comes up when the user interacts with buildstream | 19:40 |
| tlater | Probably food for discussion tomorrow though :) | 19:41 |
| persia | tlater: Would something like behave be able to drive the interaction you seek? | 19:41 |
| tlater | I'm talking about tests that would simulate terminal handling a bit better - i.e. hitting ^C at inopportune moments | 19:43 |
| * tlater can't see what behave does from a cursory glance, and hadn't heard of it | 19:44 | |
| tlater | tristan mentioned these kinds of tests a few days ago, in relation to a few ^C bugs | 19:45 |
| persia | behave is just a framework for given...when...then... structured tests, which are intended to call into the program to verify behaviour. The ^C test is perhaps a bit harder. | 19:59 |
| *** valentind has joined #buildstream | 20:06 | |
| *** ernestask has quit IRC | 20:37 | |
| *** tristan has joined #buildstream | 21:04 | |
| *** valentind has quit IRC | 22:51 | |
| *** tristan has quit IRC | 23:01 | |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!