| *** bochecha has joined #buildstream | 07:38 | |
| *** bochecha has quit IRC | 07:51 | |
| *** alexandrufazakas has joined #buildstream | 07:58 | |
| alexandrufazakas | Is the CI still broken? | 08:03 |
|---|---|---|
| *** tristan has quit IRC | 08:16 | |
| *** tiagogomes has joined #buildstream | 08:21 | |
| *** tiagogomes has quit IRC | 08:30 | |
| *** tiagogomes has joined #buildstream | 08:33 | |
| WSalmon | valentind, i think i got a bit confused by the gnome issue, but something about the filesystem seems to have changed in the standard x86_64 runners, if you look at the mtimes in the CI failure info they are all floats but integer values, If i recall correctly on a normal file system they would not be rounded to the closest second | 08:45 |
| valentind | WSalmon, probably depends on the filesystem. | 08:46 |
| *** bochecha has joined #buildstream | 08:46 | |
| WSalmon | yep, but i think something about the file system that the x86_64 runners used changed last friday | 08:47 |
| valentind | Yep, I moved to coreos, like the nightly build. For the host os of the builders. | 08:48 |
| valentind | But this is because fedora stopped working. | 08:48 |
| *** tristan has joined #buildstream | 08:50 | |
| valentind | Probably coreos uses overlay2 for docker backend. | 08:50 |
| valentind | Actually no. It is probably using XFS. | 08:53 |
| valentind | You need ext4 to get better than seconds. So I would suppose overlayfs on ext4 works. | 08:54 |
| juergbi | xfs doesn't support sub-second file times? | 08:54 |
| *** benschubert has joined #buildstream | 08:54 | |
| juergbi | afaict, it does, although it might be optional when creating the filesystem | 08:55 |
| *** jonathanmaw has joined #buildstream | 08:55 | |
| valentind | I will spin up a coreos machine manually on DO and see how it is configured. | 08:59 |
| *** lachlan has joined #buildstream | 09:01 | |
| *** tpollard has joined #buildstream | 09:04 | |
| WSalmon | brill, thanks valentind | 09:04 |
| valentind | I think though the tests should not depend on the filesystem. | 09:06 |
| WSalmon | juergbi, tristan I think we want to fix this asap. but also do we want to have a test stage (maybe nightly or on master) that has the 1sec file system? we should be able to suport this kind of file system also | 09:07 |
| valentind | If there is no other way to write the test, maybe you can try adding a conditional skip for those tests when it detects the filesystem does not support sub-second. | 09:07 |
| WSalmon | valentind, agree | 09:07 |
| *** phildawson_ has joined #buildstream | 09:08 | |
| juergbi | we don't want to skip tests because of that in CI, though | 09:08 |
| juergbi | subsecond timestamps should not be asking too much these days | 09:08 |
| juergbi | should be supported by all modern filesystems | 09:09 |
| WSalmon | given how many os's we test, could we test more than one file system? | 09:09 |
| *** lachlan has quit IRC | 09:11 | |
| WSalmon | i think the short term fix is to get back to a 1sec file system asap i think valentind is trying, if you get blocked and you let us know so we can help :D but i think testing sub 1sec file systems regully and making bst resiliant should be a longer turm goal, | 09:14 |
| WSalmon | im gona be away for a bit if valentind gets blocked is juergbi a good person for him to reach out to? | 09:15 |
| *** tiagogomes has quit IRC | 09:15 | |
| valentind | I think Lachlan was supposed to help with infra. | 09:16 |
| *** ChanServ sets mode: +o tristan | 09:16 | |
| tristan | WSalmon, fwiw, the fact that the test fails doesn't really mean we don't support subsecond precision mtimes | 09:16 |
| tristan | you may have already knew that... /me reads full backlog :) | 09:17 |
| juergbi | we could add sleeps to test in environments where subsecond precision is not available. however, I wouldn't generally recommend such environments for production | 09:17 |
| *** tiagogomes has joined #buildstream | 09:18 | |
| juergbi | buildstream itself should also not fall over with low precision but actual builds might (and things like blob expiry will be less precise) | 09:18 |
| tristan | juergbi, right... the way we rely on it is not such that it will fail us | 09:19 |
| tristan | Well, I guess it depends on your build scripts if they are going to fail, but rather also unlikely | 09:19 |
| WSalmon | tristan, well given the way some of those tests fail, not all of our featchers work on these file systems | 09:19 |
| WSalmon | juergbi, please see https://gitlab.com/BuildStream/buildstream/merge_requests/1439 | 09:20 |
| *** lachlan has joined #buildstream | 09:20 | |
| WSalmon | we had added sleeps | 09:20 |
| WSalmon | and nice functions | 09:20 |
| tristan | the vast majority of the linux lower level stack existed before ext4, I can vaguely recall some corner cases where typing `make` would do nothing :) | 09:20 |
| WSalmon | but people were not adding them | 09:20 |
| WSalmon | but when i added all the sleeps in were they should be it fixed the simple issues | 09:20 |
| WSalmon | but we have 2 tests that are not fixed by adding sleeps | 09:21 |
| juergbi | WSalmon: that should mostly be race conditions in the tests themselves | 09:21 |
| WSalmon | i have to go now but have a look at the MR | 09:21 |
| WSalmon | i think they are nasty | 09:21 |
| tristan | I recall the original artifact server may not have been waiting for a reply... hence my second comment, anyway it rings a bell | 09:21 |
| tristan | `bst build` may be returning while the remote is still finishing it's work | 09:22 |
| juergbi | that would surprise me | 09:23 |
| tristan | juergbi, it has evolved quite a bit since I originally did the tar streaming patches | 09:24 |
| juergbi | with gRPC we generally wait for response | 09:24 |
| tristan | juergbi, but at that way long ago time, I remember having trouble synchronizing the end of the transactions | 09:24 |
| tristan | sure, just pointing it out as it was a way long ago issue :) | 09:25 |
| juergbi | yes, but we no longer use a custom protocol. there could still be a bug somewhere, of course, but I don't think we intentionally send off a gRPC request anywhere without waiting for the response | 09:25 |
| tristan | for the record: I would much prefer testing how BuildStream behaves with real file systems, possibly adjusting our expectations for minor deviations like mtime precision... than mocking the environment in test cases and modifying BuildStream to fool it | 09:33 |
| tristan | For instance, I still think it would be much better to call `mkfs` and fuse mount an actual filesystem of a size we control, and remove the calls to unittest.mock() in the artifact expiry tests | 09:34 |
| juergbi | mounting e.g. an ext4 filesystem image via fuse is not completely trivial (can't use the ext4 implementation of the running kernel) | 09:38 |
| tristan | juergbi, Then require root privileges for that specific test and loopback mount it ? | 09:39 |
| tristan | we do have controlled runner environments which let us setup real environments under which we can test | 09:39 |
| juergbi | that would be a possibility but replacing the current test with this means, it will no longer be tested in (typical) local developer environments | 09:39 |
| juergbi | so I'm not sure whether it would be a net improvement | 09:40 |
| juergbi | we could have both but not sure whether that would make sense | 09:40 |
| tristan | I think its rather fine if not every single thing can be tested as regular user on developer laptop... surely there are also windows specific tests incoming which I cannot run in linux | 09:40 |
| tristan | I just think it's more correct to test the real thing than to add local hacks and pretend | 09:41 |
| juergbi | sure but I think we should also avoid such test gaps where we can | 09:41 |
| juergbi | I generally agree but we have to keep all factors in mind | 09:42 |
| tlater[m] | This would also mean that those tests don't work anywhere you don't have fuse | 09:42 |
| tristan | tlater[m], I think the topic changed from fuse -> root | 09:43 |
| tlater[m] | Ah, right | 09:43 |
| tristan | tlater[m], as juergbi pointed out that fuse is not going to use the same OS pathways as a real mount would | 09:43 |
| tlater[m] | Still won't work under windows and such, which is a bit annoying for tests like these that should really be a bit more generic | 09:43 |
| juergbi | something like a quota fuse, emulating limited disk space, might be the best compromise | 09:44 |
| tristan | tlater[m], I don't really understand why that would be true, on windows we would have an ntfs and we would want to have that tested as well | 09:44 |
| juergbi | no mocking in Python and no need for root | 09:44 |
| tristan | I mean, WSL windows aside... each case is a valid use case | 09:45 |
| tristan | I suppose WSL windows is also different than ext4 on linux (as WSL stops at the syscall table, the FS drivers must be ntfs) | 09:46 |
| tristan | Anyway... no need to jump at the opportunity to test every single operating condition, but perhaps something to consider in an idealistic future :) | 09:48 |
| juergbi | btw: WSL2 will use real ext4 (via image file in NTFS), afaik. current WSL is indeed different from the filesystem perspective | 09:53 |
| *** bochecha has quit IRC | 09:54 | |
| tristan | Interesting | 09:59 |
| tristan | I wonder when they will port MS Office to linux :) | 09:59 |
| tlater[m] | tristan: *Technically* they sort of already have, given office365 | 10:01 |
| tpollard | seeing microsoft put a linux kernel on a public vcs is still making me feel weird https://github.com/microsoft/WSL2-Linux-Kernel | 10:01 |
| *** lachlan has quit IRC | 10:01 | |
| tristan | Anyway... we can *ahem* always circulate MS Office flatpaks based on wine runtimes... | 10:04 |
| *** cs-shadow has joined #buildstream | 10:06 | |
| tlater[m] | https://winepak.org/ use disappointingly little buildstream :| | 10:07 |
| *** swick is now known as admin | 10:10 | |
| valentind | Is it still using Freedesktop SDK 1.6? | 10:19 |
| tlater[m] | valentind: Some of it still was last I looked. The maintainer has gone silent afaics | 10:22 |
| valentind | tlater[m], maybe you could build it with BuildStream. | 10:26 |
| valentind | That would be nice to test the cross-compilers we have. | 10:26 |
| valentind | For 32 bits | 10:26 |
| tlater[m] | Yeah, it's a thought I've had :) | 10:27 |
| jonathanmaw | tristan: Following from your comments on TaskGroup / FrontendTaskGroup, do I gather that an appropriate solution would be for the subscribers to the task_group_added callbacks to just store the passed group, instead of registering to receive notifications when the TaskGroup changes? | 10:27 |
| tristan | jonathanmaw, I'm not 100% clear on the intended structure... and I think it is fine if we land an initial structure and tweak it slightly later | 10:29 |
| tristan | jonathanmaw, the main gist is that the core manages and drives everything it advertises to the frontend and the frontend widgets only register some notifications | 10:30 |
| tristan | jonathanmaw, So... it could be "task group added / task group removed"... it could be "task groups changed"... | 10:30 |
| valentind | So it seems that the root filesystem on coreos is using ext4. On top of that, overlayfs is used. So it should work. But the root filesystem does not have nanoseconds. | 10:31 |
| tristan | jonathanmaw, Also, once the frontend has access to the main `State` object, it can always access `state.task_groups` in any notification it receives | 10:31 |
| tristan | jonathanmaw, So I think there would be no need to even store them separately | 10:31 |
| tristan | jonathanmaw, there could be a single `State` level notification for "task group details/statistics changed" (i.e. the data on the task groups and element counts have changed) | 10:32 |
| tristan | Or there could be per task group notifications | 10:32 |
| jonathanmaw | so, frontend widgets are able to read State.task_groups, but also receive notifications that State.task_groups has changed? | 10:34 |
| tristan | jonathanmaw, My gut feeling about structure is this: We initially land something that is properly MVC style... and later we may remodel the notifications such as to optimize for the frontend (we may end up with global notifications and we may deliver them less frequently... in order to minimize UI updates) | 10:35 |
| valentind | inode size is 128. That should explain why it is not using nanoseconds. | 10:36 |
| tristan | jonathanmaw, Yes exactly what you said... and moreover... *how* those notifications are delivered is what I mean might change and that is rather okay | 10:36 |
| jonathanmaw | tristan: ah, so the notifications that something has changed are so that the frontend knows if it needs to recalculate anything, not as a way to keep data synchronised with the frontend | 10:37 |
| jonathanmaw | gotcha | 10:37 |
| tristan | jonathanmaw, Maybe we want a single notification that the task groups have changed in some way, perhaps distinguishing between "the list of task groups have changed" and "the values held by the task groups have changed" | 10:37 |
| tristan | right, the actual data is always on State | 10:37 |
| tristan | notifications that they changed means the view needs to reflect it | 10:37 |
| valentind | I will restart gitlab-runner to try a different image. So all the current job will fail. | 10:44 |
| tristan | jonathanmaw, I think one of my comments didnt make it to _state.py | 10:47 |
| tristan | jonathanmaw, or maybe the diff changed... did you get my request to add some comment blocks separating the frontend facing notification API from the core notification driving API ? | 10:47 |
| tlater[m] | valentind: About a 1:3 chance they will anyway ;p | 10:47 |
| tristan | Aha | 10:48 |
| tristan | in my browser cache !!! | 10:48 |
| tristan | sneaky gitlab :-/ | 10:48 |
| valentind | I think we should complain to digital ocean that the coreos image that we are supposed to use with gitlab ci are broken because it uses 128 inode size for the root system. | 10:48 |
| tristan | jonathanmaw, I was just going to ask... now that we've had this conversation on IRC... does this comment make more sense ? https://gitlab.com/BuildStream/buildstream/merge_requests/1409/diffs#note_187015362 | 10:48 |
| jonathanmaw | yeah | 10:51 |
| tristan | :) | 10:51 |
| tristan | jonathanmaw, In a later iteration we'll want to look into frontend UI update throttling and such | 10:53 |
| alexandrufazakas | tlater: Hey, please have a look at !1430 whenever you've some time :) | 10:53 |
| tristan | so we can decide then whether the throttling needs to happen at the core State level or in the frontend | 10:53 |
| gitlab-br-bot | MR !1430: Add bst init argument https://gitlab.com/BuildStream/buildstream/merge_requests/1430 | 10:53 |
| tlater[m] | ooh, the br-bot is smarter these days, huh | 10:54 |
| tpollard | ++ to UI throttling | 10:54 |
| alexandrufazakas | tlater: I mentioned in the MR as well, that -C and directory test fails due to some unhandled_exception apparently :( | 10:54 |
| tristan | jonathanmaw, In the mean time... I really think that we prefer that notifications are not *too* granular | 10:58 |
| tristan | jonathanmaw, in the long term if we consider UI update throttling, it means that the frontend receives less callbacks and updates the UI less | 10:59 |
| valentind | OK, so unfortunately fedora images still fail like before. | 11:00 |
| tristan | jonathanmaw, actually you know... forget about my opinion on that... how the UI get's updated depends on a number of factors | 11:01 |
| tristan | so whatever really... for now lets just get the separation done :) | 11:01 |
| valentind | Trying with fedora atomic... | 11:05 |
| tlater[m] | alexandrufazakas: I think some git rebase mixups happened - your patch literally contains the same tests repeated once or twice | 11:12 |
| tlater[m] | I also think a handful of them are basically duplicates of older tests now that we've changed the semantics of `bst init`, and changed the old tests accordingly. | 11:13 |
| tlater[m] | Besides that, the patch looks good now | 11:13 |
| alexandrufazakas | tlater: I think so too, yeah. Sorry about that, I'll fix them | 11:14 |
| valentind | Fedora atomic does not work either. | 11:15 |
| alexandrufazakas | And yes, you're right the functionalities are similar, didn't realize the others are testing the argument behaviour too now haha | 11:15 |
| valentind | I will have to make an image for coreos with inodes of size 256 | 11:15 |
| alexandrufazakas | tlater: Would it be okay to include that small patch regarding str(None) in this MR and close the other one? | 11:17 |
| *** lachlan has joined #buildstream | 11:19 | |
| tlater[m] | alexandrufazakas: Absolutely :) | 11:26 |
| alexandrufazakas | Sweet, cheers :D | 11:27 |
| *** bochecha has joined #buildstream | 12:25 | |
| gitlab-br-bot | AlexFazakas closed MR !1437 (AlexFazakas/str-none-runcli->master: runcli: Don't set project to 'None' on no input) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1437 | 12:42 |
| alexandrufazakas | tlater: I think I fixed the issues with !1430, but I still have some issues with a couple of tests. For some reason, the max recursion ones keep failing. And the `bst init -C` one. If you could have a look at those when you've some spare time, I'd appreciate it :) | 12:52 |
| gitlab-br-bot | MR !1430: Add bst init argument https://gitlab.com/BuildStream/buildstream/merge_requests/1430 | 12:52 |
| tlater[m] | alexandrufazakas: I can't spot any recursion, do you have anything to go off debug wise? | 12:57 |
| alexandrufazakas | tlater: this is what I get running the max depth tests https://pastebin.com/sX98YBKe apparently the issue is in runcli at line 394 for some reason | 13:01 |
| tlater[m] | alexandrufazakas: Don't ask me *why*, but it's obvious that that would infinitely recurse | 13:02 |
| tlater[m] | setup_test() calls itself unconditionally | 13:02 |
| tpollard | alexandrufazakas: you can't join() None | 13:02 |
| tlater[m] | Oh, nevermind, maybe not, I misread that... | 13:03 |
| alexandrufazakas | tpollard: Ah, is it line 324 in app.py then? | 13:04 |
| alexandrufazakas | detail_dir = os.path.join(os.path.abspath(self._main_options['directory']), target_directory) | 13:04 |
| alexandrufazakas | This one | 13:04 |
| tlater[m] | Yup, that's it | 13:04 |
| tlater[m] | `target_directory` is not set | 13:05 |
| tlater[m] | Something called `bst -C <something> init` | 13:05 |
| tlater[m] | alexandrufazakas: That means you'll need to check whether target_directory is None before joining | 13:05 |
| alexandrufazakas | Or we could just set the default to os.getcwd() | 13:05 |
| alexandrufazakas | Since that's what `bst init` should do, right? | 13:06 |
| alexandrufazakas | Oh | 13:06 |
| alexandrufazakas | Nevermind | 13:06 |
| alexandrufazakas | Actually, it would probably fix this | 13:06 |
| tlater[m] | Hm, no, I don't think that's quite right | 13:06 |
| alexandrufazakas | But I'm not sure if we want to do that? | 13:06 |
| tlater[m] | While we *do* set it to `self.main_options['directory']`, which should only ever be `os.getcwd()`, this might change in the future | 13:07 |
| tlater[m] | If you set `target_directory`'s default to `os.getcwd()`, you're duplicating code | 13:07 |
| tlater[m] | Also, I'm not 100% certain that joining `/foo/bar` with `/foo/bar` would result in anything sensible here | 13:08 |
| alexandrufazakas | tlater: I think it results in /foo/bar. Join starts a new path if it encounters an absolute one | 13:08 |
| tlater[m] | alexandrufazakas: I'd suggest that you do the check *after* we resolve `directory` | 13:09 |
| tlater[m] | And simply raise the error with `directory` instaed | 13:09 |
| tlater[m] | alexandrufazakas: Yep, that's what should happen, but I'm not sure if that's the right thing to do in all cases. | 13:09 |
| alexandrufazakas | Yeah, that makes sense | 13:09 |
| alexandrufazakas | tpollard, tlater: Thanks for the help | 13:10 |
| *** lachlan has quit IRC | 13:28 | |
| valentind | WSalmon, builders migt be working correctly now. | 13:30 |
| valentind | Though I see a lot of the time builders do not manage to start properly. Probably because updating fedora and installing docker takes a lot of time and it fails with time out. | 13:31 |
| tlater[m] | Thanks valentind! | 13:33 |
| *** lachlan has joined #buildstream | 13:33 | |
| alexandrufazakas | Anyone have any idea what that assert fails? :( | 13:41 |
| alexandrufazakas | https://pastebin.com/EqAM32du | 13:41 |
| * tpollard stares longingly at gitlab | 13:43 | |
| tlater[m] | alexandrufazakas: Not 100% on this, but I suspect that this is raised *before* "main" errors can be raised | 13:45 |
| tlater[m] | In other words, we expect assert_main_error to fail | 13:45 |
| tlater[m] | There should be another assertion somewhere that does the right thing here | 13:46 |
| tlater[m] | If not, I'm horribly wrong, and I apologize | 13:46 |
| tlater[m] | But I think it'll be something like assert_app_error? | 13:46 |
| alexandrufazakas | tlater: `result.assert_main_error(ErrorDomain.APP, 'init-with-set-directory')` is what I'm using | 13:46 |
| alexandrufazakas | so just use _app_error instead? | 13:47 |
| alexandrufazakas | Oh, apparently dont just use app_error instead | 13:47 |
| tlater[m] | I have no idea if that actually exists, skim through runcli? | 13:47 |
| tlater[m] | I just believe that that's how these exceptions work | 13:47 |
| alexandrufazakas | Don't think it exists | 13:48 |
| alexandrufazakas | No other test has it | 13:48 |
| alexandrufazakas | But I'll have a look through the exceptions sure | 13:48 |
| tlater[m] | alexandrufazakas: Reading through runcli.py, you'll notice that `self.unhandled_exception` is asserted to be true iff the exception is a `SystemExit`. | 13:50 |
| tlater[m] | There's a comment right above that that will tell you CLI errors should always look like that. | 13:51 |
| tlater[m] | Why do we get a SystemExit here? | 13:51 |
| alexandrufazakas | Uhh | 13:52 |
| alexandrufazakas | Not sure | 13:52 |
| *** lachlan has quit IRC | 13:52 | |
| tlater[m] | Let me read a little into it then... | 13:52 |
| tlater[m] | If you scroll about 10 lines down from your code, you'll notice that we catch our own AppErrors and call `self.error_exit(e)` | 13:53 |
| tlater[m] | You should perform your check as part of that try block | 13:54 |
| tlater[m] | alexandrufazakas ^ | 13:54 |
| alexandrufazakas | Ah, that makes sense, yeah. I was thinking we might need a try-catch block but I wasn't sure where/how. Thanks tlater :) | 13:56 |
| tlater[m] | (Note that AppError is a BstError) | 13:56 |
| alexandrufazakas | tlater: Yep, that does it | 13:57 |
| *** tristan has quit IRC | 14:08 | |
| *** tristan has joined #buildstream | 14:15 | |
| *** lachlan has joined #buildstream | 14:24 | |
| tlater[m] | phildawson_: It turns out that cython files have headers, and whoever wrote that function did not expose it in the header - which meant that you *could* access it from python, but not cython. | 14:27 |
| * tlater[m] kept wondering if there was a weird thing going on like you can only access cpdef stuff from cpdef stuff | 14:28 | |
| *** lachlan has quit IRC | 14:29 | |
| *** lachlan has joined #buildstream | 14:30 | |
| *** lachlan has quit IRC | 14:34 | |
| *** tiagogomes has quit IRC | 14:38 | |
| *** lachlan has joined #buildstream | 14:39 | |
| *** tiagogomes has joined #buildstream | 14:41 | |
| tlater[m] | alexandrufazakas: One thing I keep forgetting to mention is that you'll need to write a NEWS entry | 14:41 |
| alexandrufazakas | Ah | 14:41 |
| tlater[m] | Since we're breaking backwards compatibility | 14:42 |
| alexandrufazakas | tlater: Tbh, you did mention it, I just didn't realize I was the one to do it | 14:42 |
| alexandrufazakas | I'll do that in a second | 14:42 |
| tlater[m] | I also think there's at least one piece of documentation that you'll need to amend | 14:42 |
| tlater[m] | To you the honors :) | 14:42 |
| alexandrufazakas | haha | 14:42 |
| *** tiagogomes has quit IRC | 14:42 | |
| alexandrufazakas | tlater: Should probably update something in man/ ? | 14:43 |
| tlater[m] | alexandrufazakas: man/ is generated automatically using click | 14:44 |
| tlater[m] | You'll want to update something in doc/ | 14:44 |
| alexandrufazakas | Ah | 14:44 |
| alexandrufazakas | Makes sense | 14:44 |
| *** ChanServ sets mode: +o tristan | 14:45 | |
| tristan | alexandrufazakas, tlater[m] probably the first entry in the tutorial | 14:45 |
| tristan | should be using_examples.rst I suspect | 14:45 |
| tristan | and perhaps any related session files in doc/sessions | 14:45 |
| alexandrufazakas | tristan: I'll look into that. Thank you :) | 14:46 |
| tlater[m] | ta tristan, I rely too much on grep :) | 14:46 |
| alexandrufazakas | Is the whole man/ directory generated from click? | 14:47 |
| *** lachlan has quit IRC | 14:47 | |
| alexandrufazakas | 'cus I was wondering why it's included in the source repository in this case | 14:47 |
| tristan | tlater[m], nah, grep is the best | 14:48 |
| tristan | tlater[m], probably more reliable than my memory, but I think that is the right place | 14:48 |
| tristan | at least the things in Using are using_*.rst files | 14:48 |
| alexandrufazakas | Uhh, should I be updating the 1.3.1 section? | 14:48 |
| tristan | alexandrufazakas, possibly if that is the huge section at the top yeah | 14:48 |
| alexandrufazakas | It indeed is the huge section at the top :) | 14:49 |
| tristan | alexandrufazakas, once we release a shapshot, we'll probably just change that to 1.91.0 | 14:49 |
| tlater[m] | alexandrufazakas: regarding man/, I'd just say read the makefile in there (or whatever other build construct lives there) | 14:49 |
| tristan | oh right | 14:49 |
| tlater[m] | iirc it was some hairy stuff tristan hacked together to get around the click author not wanting to take his patch | 14:50 |
| tristan | tlater[m], CONTRIBUTING.rst has instructions for man regeneration | 14:50 |
| tristan | But did someone integrate man pages into the build scripts anyway recently ? | 14:50 |
| tristan | benschubert maybe ? | 14:50 |
| * tristan vaguely recalls that being discussed | 14:50 | |
| tlater[m] | I'm fairly sure tox runs that now, yeah :) | 14:50 |
| tristan | so if the files are not statically added in man/, no need to do anything | 14:50 |
| tristan | I think the idea was to have them generated in dist tarballs but not in git | 14:51 |
| benschubert | I don't remember doing that | 14:51 |
| tristan | Ok | 14:51 |
| tristan | So I think it was not done, all I remember was that it was *discussed* :) | 14:51 |
| tlater[m] | testenv:man | 14:51 |
| tristan | ah | 14:51 |
| tristan | tlater[m], So if that is there... hopefully CONTRIBUTING.rst was updated to instruct us to use tox to generate them | 14:52 |
| tristan | fingers crossed :) | 14:52 |
| tristan | alexandrufazakas, probably what that means is run `tox -e man` and commit the result of that | 14:53 |
| tlater[m] | tristan: It was actually updated! | 14:53 |
| tlater[m] | tox -e man is apparently enough to just regenerate them, but it does need to be run manually | 14:53 |
| tristan | yay \o/ | 14:53 |
| alexandrufazakas | Oh, that's awesome | 14:53 |
| tlater[m] | Really, IMO CI should do it | 14:54 |
| *** lachlan has joined #buildstream | 14:54 | |
| tristan | tlater[m], it could but that has never been known to fail | 14:54 |
| tlater[m] | But I suppose you'd need to deploy it as part of a package then, because it can't commit the actual pages | 14:54 |
| tristan | tlater[m], the important part is that `setup.py install` installs up to date ones... ideally we shouldn't have them statically in git | 14:54 |
| tlater[m] | Yeah, that would make a lot more sense | 14:55 |
| tristan | tlater[m], that means going through the weird awkward process of integrating strange things in distutils | 14:55 |
| benschubert | right, sdist/bdist should be generating them ideally | 14:55 |
| benschubert | like we have for cython stuff | 14:55 |
| tristan | iirc, it *should* be trivially enough doable, just that the click-man plugin which does it was not very condusive to that integration | 14:57 |
| tristan | Or, I just lost patience with it at some point | 14:57 |
| *** lachlan has quit IRC | 14:58 | |
| *** lachlan has joined #buildstream | 15:00 | |
| *** tiagogomes has joined #buildstream | 15:02 | |
| *** lachlan has quit IRC | 15:06 | |
| *** lachlan has joined #buildstream | 15:09 | |
| *** lachlan has quit IRC | 15:19 | |
| *** phil has joined #buildstream | 15:32 | |
| *** phildawson_ has quit IRC | 15:33 | |
| *** lachlan has joined #buildstream | 15:39 | |
| *** bochecha has quit IRC | 15:43 | |
| benschubert | tristan: https://gitlab.com/BuildStream/buildstream/merge_requests/1434#note_187161126 I think that's the correct behavior, but wasn't in the original discussion about that. Can you confirm? | 16:41 |
| gitlab-br-bot | BenjaminSchubert merged MR !1434 (bschubert/node-find-target->bschubert/new-node-api: Replace 'node_find_target' by 'MappingNode.find') on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1434 | 16:41 |
| *** raoul has quit IRC | 16:48 | |
| *** alexandrufazakas has quit IRC | 16:48 | |
| *** tiagogomes has quit IRC | 16:54 | |
| tristan | benschubert, commented... what we should be getting is a warning (not an error), about refs which *are* present but will be ignored (not about refs which are missing) | 17:13 |
| benschubert | tristan: that was on master, not my branch that I ran this | 17:13 |
| tristan | benschubert, I suspect that that is the behavior that you saw but just mistyped it in the comment (I don't think we *ever* had errors about missing refs at `bst show` time, that would not be an error) | 17:13 |
| tristan | or even a warning... it would just be elements with `inconsistent` state | 17:14 |
| benschubert | right, I might have gone too quickly | 17:16 |
| benschubert | so behavior is correct right? | 17:16 |
| tristan | benschubert, note that that warning... in master might be an error (if that is configurable as a fatal warning in master, I didnt check) | 17:16 |
| tristan | benschubert, Yeah :) | 17:16 |
| tristan | I think the behavior is correct | 17:16 |
| benschubert | thanks! | 17:20 |
| benschubert | tristan: also, quesiton about workspaces, do you know what the 'running-files' is? How can I populate this? | 17:20 |
| tristan | Hahahahaha | 17:21 |
| tristan | Ask tlater[m] ! | 17:21 |
| benschubert | tlater[m]: ^ please? :P | 17:21 |
| tlater[m] | Oof, that's a long time ago | 17:21 |
| tristan | benschubert, I believe it is "The files in the workspace which differed since the last successful build" | 17:21 |
| tlater[m] | tristan is right | 17:21 |
| tristan | or rather, the files in the last successful build, which differ from the original unworkspaced thing ? | 17:21 |
| tristan | Oh no, it's the former | 17:22 |
| benschubert | oh I see, thanks! | 17:22 |
| benschubert | gah I need to be able to build then | 17:22 |
| tlater[m] | benschubert: There is a big long comment about it somewhere | 17:22 |
| tristan | story about making sure modified files are "newer" in the sandbox, for incremental builds | 17:22 |
| benschubert | I see, thanks a lot | 17:24 |
| benschubert | I'm checking that my node_sanitize rework doesn't break anything, and I was wondering how to populate this | 17:24 |
| tlater[m] | benschubert: Actually, it's just files created by *dependencies* since the last successful build - see element.py:768 | 17:24 |
| tlater[m] | frontend/workspace.py has tests that ensure these things don't break | 17:25 |
| * tristan always gets confused about that... indeed, dependencies can change in between workspace builds | 17:25 | |
| tlater[m] | It's one of those test cases you only really "get" when you actually look at how it's reproduced | 17:27 |
| tristan | or when you read the elaborate book tlater[m] is going to publish on the subject ;-) | 17:27 |
| benschubert | tlater[m]: frontend/workspaces.py: would that get an error where I would be modifying the format without paying attention? | 17:27 |
| tlater[m] | benschubert: It would certainly break if the running files aren't populated correctly anymore | 17:29 |
| tlater[m] | And it should also break if you change running_files to foo_bar or something | 17:30 |
| tlater[m] | What kind of modifications do you mean? | 17:30 |
| benschubert | https://gitlab.com/BuildStream/buildstream/merge_requests/1438 :) removing the 'node_sanitize' | 17:30 |
| *** toscalix has joined #buildstream | 17:30 | |
| tristan | which is to say... if they are written out and read back in a different character encoding (silly example)... then the tests would still pass, but it would warrant a version bump in .bst/workspaces.yml | 17:30 |
| tristan | I really expect this to not impact running files in a bad way | 17:31 |
| tlater[m] | Yep, I agree with that | 17:31 |
| tlater[m] | It won't affect the actual behavior if the tests pass, and considering nobody should even consider modifying this stuff manually that should be enough. | 17:32 |
| tristan | Just... "If you upgrade to a new version of BuildStream and have open workspaces... that should not cause things to break" | 17:33 |
| tristan | as such the workspaces.yml is versioned and automatically upgrades itself | 17:34 |
| tlater[m] | Unless you have a running instance of buildstream, install a new version, and somehow get the two versions of buildstream to try and synch up on what running files they have while the two are running. | 17:34 |
| tristan | That in itself is not tested afaik - it could be interesting to add a test with a committed workspaces.yml to ensure that it works when modifying code around that | 17:35 |
| tristan | (and add a new test with a committed workspaces.yml in every case that we modify/bump the workspaces.yml format version) | 17:36 |
| tristan | but meh, it will probably never change, or change once or twice in the next 10 years | 17:36 |
| benschubert | tristan: https://gitlab.com/BuildStream/buildstream/merge_requests/1438#note_187177300 that's my current tests. If you seem something that I should test more let me know! tlater[m] actually your input would be good too | 17:36 |
| *** lachlan has quit IRC | 17:50 | |
| *** phil has quit IRC | 17:58 | |
| *** lachlan has joined #buildstream | 18:24 | |
| *** lachlan has quit IRC | 19:11 | |
| *** lachlan has joined #buildstream | 19:14 | |
| *** lachlan has quit IRC | 19:25 | |
| *** lachlan has joined #buildstream | 19:26 | |
| *** lachlan has quit IRC | 19:30 | |
| *** lachlan has joined #buildstream | 19:34 | |
| *** lachlan has quit IRC | 19:46 | |
| *** toscalix has quit IRC | 20:05 | |
| *** jonathanmaw has quit IRC | 20:21 | |
| *** benschubert has quit IRC | 22:18 | |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!