| gitlab-br-bot | buildstream: merge request (jjardon/doc_arch_releases->master: source/install_linux_distro.rst: Make clearer ArchLinux packages available) #613 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/613 | 00:02 |
|---|---|---|
| *** christophegx has joined #buildstream | 01:51 | |
| *** bochecha has quit IRC | 02:17 | |
| *** tristan has joined #buildstream | 04:24 | |
| gitlab-br-bot | buildstream: merge request (edbaunton/executable-remote-source->master: remote.py: Add support for marking downloaded files executable) #581 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/581 | 05:04 |
| *** tristan has quit IRC | 06:11 | |
| gitlab-br-bot | buildstream: merge request (edbaunton/executable-remote-source->master: remote.py: Add support for marking downloaded files executable) #581 changed state ("merged"): https://gitlab.com/BuildStream/buildstream/merge_requests/581 | 06:13 |
| *** Connection2 has joined #buildstream | 07:00 | |
| *** tristan has joined #buildstream | 07:26 | |
| gitlab-br-bot | buildstream: merge request (jjardon/doc_arch_releases->master: source/install_linux_distro.rst: Make clearer ArchLinux packages available) #613 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/613 | 07:29 |
| *** leopi has joined #buildstream | 07:36 | |
| *** gitlab-br-bot has quit IRC | 07:42 | |
| *** gitlab-br-bot has joined #buildstream | 07:42 | |
| *** gitlab-br-bot has joined #buildstream | 07:42 | |
| *** finn has joined #buildstream | 07:45 | |
| *** finn_ has quit IRC | 07:45 | |
| *** gitlab-br-bot has quit IRC | 07:48 | |
| *** gitlab-br-bot has joined #buildstream | 07:48 | |
| *** gitlab-br-bot has quit IRC | 07:49 | |
| *** gitlab-br-bot has joined #buildstream | 07:49 | |
| *** gitlab-br-bot has quit IRC | 07:51 | |
| *** gitlab-br-bot has joined #buildstream | 07:51 | |
| *** toscalix has joined #buildstream | 07:56 | |
| gitlab-br-bot | buildstream: merge request (jjardon/doc_arch_releases->master: source/install_linux_distro.rst: Make clearer ArchLinux packages available) #613 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/613 | 08:05 |
| *** lkoranda has joined #buildstream | 08:05 | |
| *** bumbar19 has joined #buildstream | 08:10 | |
| WSalmon | phildawson, are you happy with https://gitlab.com/BuildStream/buildstream/merge_requests/580 now i have updated the bit you mentioned? | 08:15 |
| *** gitlab-br-bot has quit IRC | 08:15 | |
| *** gitlab-br-bot has joined #buildstream | 08:15 | |
| phildawson | WSalmon, I'm happy now :) | 08:19 |
| WSalmon | Woop :) | 08:19 |
| gitlab-br-bot | buildstream: merge request (jjardon/doc_arch_releases->master: source/install_linux_distro.rst: Make clearer what ArchLinux packages are available) #613 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/613 | 08:19 |
| jjardon | test seems to fail randomly? fedora job failed in a MR that only changed some docs: https://gitlab.com/BuildStream/buildstream/-/jobs/87321473 known issue? | 08:25 |
| phildawson | jjardon, I think #503 off the top of my head :) | 08:26 |
| phildawson | I expect the test will pass if you rerun it | 08:26 |
| phildawson | though it is somewhat irritating. | 08:27 |
| WSalmon | I have run https://gitlab.com/BuildStream/buildstream/merge_requests/595 through a spell checker but there are a lot of words so could some one have a quick look at the spelling etc and could tiagogomes comment if this solution is expectable? I can do something more complex but it doesn't seem worth it for a few seconds of compute time per CI... but i am happy to be wrong. | 08:27 |
| gitlab-br-bot | buildstream: issue #550 ("pulling from the cache doesn't honor the fetchers setting") changed state ("opened") https://gitlab.com/BuildStream/buildstream/issues/550 | 08:27 |
| WSalmon | *acceptable | 08:27 |
| phildawson | WSalmon, I'll take a look | 08:27 |
| WSalmon | Thanks :) | 08:27 |
| jjardon | phildawson: https://gitlab.com/BuildStream/buildstream/issues/503 seems to be closed, so maybe is another issue? | 08:27 |
| qinusty | What version are you using jjardon? | 08:28 |
| phildawson | jjardon, in that case I'm not sure. Out of curiosity which test(s) was failing? | 08:29 |
| qinusty | test_push_after_pull | 08:29 |
| jjardon | qinusty: this is a MR to master, so latest | 08:29 |
| jjardon | I will open an issue | 08:29 |
| qinusty | Alright, perhaps raise it as an issue and then we'll keep an eye on it | 08:29 |
| phildawson | ^+1 | 08:29 |
| *** moved has joined #buildstream | 08:30 | |
| gitlab-br-bot | buildstream: issue #559 ("Spurious error when running fedora job (the change in the MR was only on documentation)") changed state ("opened") https://gitlab.com/BuildStream/buildstream/issues/559 | 08:30 |
| jjardon | thanks qinusty phildawson | 08:32 |
| gitlab-br-bot | buildstream: merge request (willsalmon/trackWarning->master: Add warning to git track if track and ref are not present) #580 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/580 | 08:45 |
| gitlab-br-bot | buildstream: merge request (willsalmon/trackWarning->master: Add warning to git track if track and ref are not present) #580 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/580 | 08:46 |
| gitlab-br-bot | buildstream: merge request (adamjones/systemd-cas->master: Include systemd file examples for google-cas cache) #524 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/524 | 08:47 |
| *** gitlab-br-bot has quit IRC | 08:49 | |
| *** gitlab-br-bot has joined #buildstream | 08:49 | |
| tristan | jjardon, that spurious failure is https://gitlab.com/BuildStream/buildstream/issues/520 | 08:54 |
| tristan | qinusty, phildawson ^^ | 08:54 |
| jjardon | tristan: ok, let me mark as duplicate | 08:54 |
| gitlab-br-bot | buildstream: issue #559 ("Spurious error when running fedora job (the change in the MR was only on documentation)") changed state ("closed") https://gitlab.com/BuildStream/buildstream/issues/559 | 08:55 |
| tristan | jjardon, thanks | 08:55 |
| jjardon | btw, in case you dont know: comment in the issue with /duplicate #<issue number> to mark issues as dup. I've just discovered that the other day | 08:56 |
| tristan | Oh that's new | 08:58 |
| tristan | jjardon, is there any way to do that through the UI ? | 08:58 |
| jjardon | tristan: no as far as I know | 08:58 |
| tristan | I think that nature of an issue's relationship is new, cause I wanted it a long time ago :) | 08:58 |
| phildawson | Ah, I hadn't spotted that one. Thanks for pointing that out tristan. | 08:59 |
| jjardon | I guess they have the "backend" ready, but they are still working in the UI part | 08:59 |
| tristan | phildawson, it's a pretty hot topic yeah, tiagogomes is working on it already | 08:59 |
| * tiagogomes nods | 09:00 | |
| tristan | jjardon, better they fix the mobile UI first (merge from handphone anyone ?), and making the JS payload a bit less heavy duty (nice if firefox doesnt need to ask you if you want to stop the script), first | 09:01 |
| tristan | at least, I hope they are prioritizing the recent regressions :) | 09:01 |
| gitlab-br-bot | buildstream: issue #560 ("BUG: Message handling out of sync") changed state ("opened") https://gitlab.com/BuildStream/buildstream/issues/560 | 09:01 |
| *** gitlab-br-bot has quit IRC | 09:02 | |
| *** gitlab-br-bot has joined #buildstream | 09:02 | |
| jjardon | tristan: buy a new phone :P | 09:02 |
| tristan | jjardon, gitlab freezes my *laptop* | 09:02 |
| tristan | jjardon, on the phone, I just get this sidebar when I visit the mobile UI, the collapse button doesnt work anymore | 09:03 |
| jjardon | mine is pretty old and I've never seen that popup to stop the script | 09:03 |
| jjardon | tristan: yeah, same here; sometimes that happen, I do not know why | 09:03 |
| jjardon | but yeah, agree that some optimization would be great | 09:04 |
| tristan | I've never had that on my phone, and it only happens on my laptop when: (A) reviewing a non-trivial branch... (B)... adding plenty of comments | 09:04 |
| tristan | that is a regression, not an optimization; last month this wasn't happening afair | 09:04 |
| tristan | The more you add comments, the slower it gets | 09:05 |
| tristan | Commenting on an existing comment, doesnt effect performance | 09:05 |
| *** tpollard has joined #buildstream | 09:05 | |
| tristan | (of course the whole patch takes a looong time to load, but that's not really as bad as the UI becoming unusable) | 09:05 |
| tristan | anyway, /endrant :) | 09:06 |
| qinusty | tristan, I'm getting reordering of nodes within .bst files :/ | 09:06 |
| qinusty | The regression might be worse than we thought, or it's a separate issue. Who knows | 09:07 |
| tristan | qinusty, Do you get reordering with raw ruamel.yaml ? | 09:09 |
| *** gitlab-br-bot has quit IRC | 09:09 | |
| *** gitlab-br-bot has joined #buildstream | 09:09 | |
| tristan | Given say, the same sample bst file as input ? | 09:09 |
| *** gitlab-br-bot has quit IRC | 09:10 | |
| *** gitlab-br-bot has joined #buildstream | 09:10 | |
| * qinusty shrugs, I'm just trying to build something with buildstream and it reordered my file. It doesn't seem to be doing it every time | 09:10 | |
| qinusty | hmmm, should've split that | 09:10 |
| qinusty | you get the point | 09:10 |
| tristan | qinusty, I have a feeling we might be seeing a regression in ruamel.yaml itself, coupled with a regression that we're saving files for refs which didnt change | 09:10 |
| tristan | qinusty, or wait, I might be confusing people... there is a lot going on in parallel, so I am no longer sure if you are the person who is working on that specific regression | 09:11 |
| qinusty | Track reoorders | 09:11 |
| qinusty | I just reproduced it | 09:11 |
| qinusty | I'm not /working/ on it, but I have previously investigated it | 09:12 |
| tristan | Ok | 09:12 |
| qinusty | I'll try comments 1sec | 09:12 |
| gitlab-br-bot | buildstream: merge request (jjardon/doc_arch_releases->master: source/install_linux_distro.rst: Make clearer what ArchLinux packages are available) #613 changed state ("merged"): https://gitlab.com/BuildStream/buildstream/merge_requests/613 | 09:12 |
| qinusty | Okay | 09:13 |
| qinusty | You were right in the meeting, it seems it's all screwed | 09:13 |
| tristan | yeah, if the comments disappear, then BuildStream has regressed into not using the round tripping anymore (which I was doubtful of in our last meeting, but it's possible) | 09:13 |
| qinusty | Comments gone, reordering is out the window | 09:13 |
| tristan | Aha | 09:13 |
| tristan | Ok, do you remember the issue number off hand ? lets update that... we need to know the story :) | 09:14 |
| qinusty | I'll find it | 09:14 |
| tristan | qinusty, actually, can you paste the offending .bst file before and after also ? | 09:14 |
| tristan | that will be very helpful :) | 09:14 |
| gitlab-br-bot | buildstream: issue #560 ("BUG: Message handling out of sync") changed state ("closed") https://gitlab.com/BuildStream/buildstream/issues/560 | 09:18 |
| *** jonathanmaw has joined #buildstream | 09:20 | |
| qinusty | Okay, so getting the "Inconsistent pipeline 'exact versions are missing'..." message. What sort of fix am I looking for? It hints at using bst track to resolve the issue but this doesn't exactly help if I'm not too sure what is supposed to happen :P | 09:22 |
| * qinusty is actually using buildstream for once rather than just example projects | 09:22 | |
| qinusty | `no reference ??????????????????? <element.bst>` is the problem obviously | 09:23 |
| gitlab-br-bot | buildstream: merge request (willsalmon/CacheExpiryTest->master: Trying to mitigate a file system issue) #595 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/595 | 09:24 |
| gitlab-br-bot | buildstream: merge request (adamjones/systemd-cas->master: Include systemd file examples for google-cas cache) #524 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/524 | 09:25 |
| tristan | qinusty, what are you trying to build ? | 09:25 |
| tristan | qinusty, and with what command ? | 09:25 |
| valentind | I see that in utils._get_dir_size we do not keep inodes to avoid calculating several times the same file. Should not we? | 09:25 |
| qinusty | I'm trying to build curl for the sake of understanding how to use buildstream :D | 09:26 |
| qinusty | I've got a manual element with a git source | 09:26 |
| tristan | valentind, I doubt it; seeing as the CAS cache will deduplicate matching file objects, but I might be wrong | 09:26 |
| *** bochecha has joined #buildstream | 09:26 | |
| valentind | tiagogomes, Do you manage to reproduce #520? I can. | 09:26 |
| tristan | qinusty, can I see the project somewhere ? and can you tell me what command you ran, and what you expected to happen ? | 09:27 |
| qinusty | suuuure, give me a few minutes to get it somewhere | 09:27 |
| tristan | qinusty, without that information I can say that: `bst build` by itself expects the project to be consistent, and have all the refs | 09:27 |
| tristan | qinusty, otherwise it will suggest that you track, in order to update the refs | 09:28 |
| qinusty | And refs are gathered from.... | 09:28 |
| tristan | qinusty, `bst build --track-all` will automatically do that | 09:28 |
| phildawson | jjardon, you marked #559 as duplicate of #420 rather than #520. I've added the correct mark but don't know how to remove the old one :) | 09:29 |
| tristan | qinusty, the refs, for a git source, is the commit sha you want to build. Either you have specified it manually, or `bst track` will automatically update it to the latest commit on the branch you've indicated with the `track` parameter | 09:29 |
| tristan | qinusty, in the abstract sense, the 'ref' is a pointer to something exact, while the track is something you optionally provide for the sake of `bst track` to guess the ref for you | 09:30 |
| qinusty | Okay | 09:30 |
| qinusty | https://gitlab.com/Qinusty/bst-build-curl is where I've stuck the files, from what I can understand it might be failing to guess the ref? | 09:31 |
| jjardon | phildawson: ooops thanks | 09:31 |
| qinusty | There are almost certainly issues with my build instructions, but that's besides the point at the minute :P | 09:32 |
| * qinusty probably needs track or ref | 09:33 | |
| tristan | qinusty, I'm a bit confused about your having an 'etag' there: https://gitlab.com/Qinusty/bst-build-curl/blob/master/elements/base/alpine.bst | 09:33 |
| qinusty | the alpine.bst comes straight from an example project | 09:33 |
| * qinusty shrugs | 09:33 | |
| tristan | the 'etag' format was removed a while back within the dev cycle afair | 09:33 |
| tristan | but lets ignore that... I don't think that is causing you an error | 09:33 |
| *** gitlab-br-bot has quit IRC | 09:34 | |
| *** gitlab-br-bot has joined #buildstream | 09:34 | |
| tristan | qinusty, right, so if you want to use track, you need to specify a tracking branch | 09:34 |
| tristan | qinusty, do you want to build curl master branch ? | 09:34 |
| *** gitlab-br-bot has quit IRC | 09:34 | |
| *** gitlab-br-bot has joined #buildstream | 09:34 | |
| qinusty | I guess so, not too fussed. I've added that and we're getting further it seems | 09:34 |
| *** gitlab-br-bot has joined #buildstream | 09:34 | |
| tristan | qinusty, if so, then beside 'url: blabla', specify 'track: master' | 09:34 |
| *** gitlab-br-bot has quit IRC | 09:34 | |
| *** gitlab-br-bot has joined #buildstream | 09:34 | |
| *** gitlab-br-bot has joined #buildstream | 09:35 | |
| qinusty | Is there an issue for this? I remember seeing something about adding a warning to a lack of ref | 09:35 |
| valentind | tristan, to test determinism in source plugins, I am thinking of making an integration test that runs GNU tar on different source plugin, with different umask and make sure the hash is always the same. Do you see a test that would be simpler? | 09:35 |
| tristan | qinusty, there is, I believe the issue is that the error suggests only that you track, even for an untrackable source which does not specify the optional 'track' parameter | 09:36 |
| qinusty | ah | 09:36 |
| qinusty | For me, coming in with no idea of creating a bst project from scratch. It was strange to be told to track and then track tell me ????????????????????? | 09:37 |
| tristan | qinusty, what it comes down to, is that (A) ref is required to build... (B) users can use the tracking productivity feature, but tracking is not required | 09:37 |
| tristan | qinusty, and that part... is because BuildStream is unable at that stage to guess it's cache key, because when you track, you are asking for a new ref; so the cache key for that element will be discovered during the session | 09:38 |
| tristan | qinusty, at the end of a successful tracking session, BuildStream will print a summary of all the cache keys which were discovered during the session | 09:38 |
| *** gitlab-br-bot has quit IRC | 09:38 | |
| *** gitlab-br-bot has joined #buildstream | 09:38 | |
| qinusty | I guess my worry is that at no point in our examples does anything go wrong | 09:39 |
| tristan | I think that we need to improve the learning curve around ref/track, certainly | 09:39 |
| qinusty | Because now you mention it, yes it makes sense that I have to provide one of the two | 09:40 |
| tristan | Also, it's not a bad idea at all to include failures in the examples | 09:40 |
| *** gitlab-br-bot has quit IRC | 09:40 | |
| *** gitlab-br-bot has joined #buildstream | 09:40 | |
| tristan | qinusty, also here's a tip; it looks to me like curl does ./configure && make, and is more like an autotools element which needs a custom configure step | 09:45 |
| tristan | qinusty, here's a suggestion: https://bpaste.net/show/60c4f182efae | 09:45 |
| tristan | So you can always observe the defaults of what a build element is going to do, like the autotools element: http://buildstream.gitlab.io/buildstream/elements/autotools.html | 09:46 |
| tristan | and you can override stuff there | 09:47 |
| tristan | qinusty, you're curl build for example, will produce an empty artifact, because it doesnt install anything into %{install-root} | 09:47 |
| noisecell | tiagogomes, jjardon, toscalix, I think https://gitlab.com/BuildStream/buildstream/issues/116 can be closed, am I missing something? | 09:48 |
| toscalix | checking | 09:50 |
| phildawson | qinusty, there was an issue for printing a more helpful warning when track hadn't been specified. A fix will be landing imminently in !580 :) | 09:50 |
| tpollard | are there any examples / docs on using the include directive? | 09:51 |
| tristan | toscalix, noisecell, it would be good if we could close that with certainty; a statement that we "probably need fusermount" for every platform would solve it | 09:51 |
| toscalix | tpollard: if not, please open a ticket. There should be | 09:51 |
| tristan | as I recall, whether the low level fuse library requires a setuid fusermount program to unmount or not, can be dependent on system configuration and distro choices | 09:52 |
| toscalix | noisecell: tristan added ToDo until we are certain | 09:52 |
| noisecell | tristan, I see what you mean. I though adding the fuse requirement for the every distro was good enough | 09:53 |
| gitlab-br-bot | buildstream: merge request (relative_workspaces->master: Patch for issue #191 support relative workspaces) #504 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/504 | 09:53 |
| jjardon | noisecell: also "We should also document that we require the host kernel to have fuse support (we recently encountered an error where fuse was present as a module but not loaded, and kernel cannot load it because the build was running in a VM)" is not documented yet | 09:54 |
| gitlab-br-bot | buildstream: merge request (relative_workspaces->master: Patch for issue #191 support relative workspaces) #504 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/504 | 09:54 |
| jjardon | from https://gitlab.com/BuildStream/buildstream/issues/116#note_53731672 | 09:55 |
| qinusty | I thought I'd seen it phildawson, nice! | 09:55 |
| tristan | noisecell, I think we can close it if we mention it for each distro indeed, it's the safe bet | 09:57 |
| tristan | noisecell, if we verify that that is done, I certainly think we can close it | 09:57 |
| noisecell | tristan, jjardon, ok | 09:57 |
| valentind | tristan, is running multiple times buildstream in parallel supported? | 10:04 |
| valentind | It has always worked for me. But I wonder if we make sure it does work and we do not break it. | 10:05 |
| tristan | valentind, it is supported yes | 10:06 |
| tristan | valentind, my comment in the issue points out that while we should make the fallback more robust, it shouldnt be happening in the first place | 10:07 |
| tristan | valentind, it's not really high on the priority list though | 10:07 |
| tristan | valentind, i.e. the cache cleanup cannot provide the right guarantees with multiple instances running | 10:07 |
| valentind | OK | 10:08 |
| juergbi | jonathanmaw: I have an old patch for build-depends. maybe too old to be useful but just in case I pushed it to the branch juerg/build-depends | 10:09 |
| gitlab-br-bot | buildstream: merge request (tiagogomes/issue-520->master: Fix race condition when calculating disk usage) #600 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/600 | 10:09 |
| jonathanmaw | tvm juergbi | 10:09 |
| *** gitlab-br-bot has quit IRC | 10:13 | |
| *** gitlab-br-bot has joined #buildstream | 10:13 | |
| gitlab-br-bot | buildstream: issue #419 ("Distribute a systemd `.service` file with the new CAS server") changed state ("closed") https://gitlab.com/BuildStream/buildstream/issues/419 | 10:29 |
| gitlab-br-bot | buildstream: merge request (adamjones/systemd-cas->master: Include systemd file examples for google-cas cache) #524 changed state ("merged"): https://gitlab.com/BuildStream/buildstream/merge_requests/524 | 10:29 |
| gitlab-br-bot | buildstream: merge request (tiagogomes/issue-520->master: Fix race condition when calculating disk usage) #600 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/600 | 10:36 |
| gitlab-br-bot | buildstream: merge request (jennis/change_arch_install_instructions->master: Change install instructions for python-arpy package on Arch) #614 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/614 | 10:47 |
| gitlab-br-bot | buildstream: merge request (relative_workspaces->master: Patch for issue #191 support relative workspaces) #504 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/504 | 10:48 |
| gitlab-br-bot | buildstream: merge request (jennis/change_arch_install_instructions->master: Change install instructions for python-arpy package on Arch) #614 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/614 | 10:50 |
| gitlab-br-bot | buildstream: merge request (jennis/change_arch_install_instructions->master: Change install instructions for python-arpy package on Arch) #614 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/614 | 10:51 |
| gitlab-br-bot | buildstream: merge request (jennis/change_arch_install_instructions->master: Change install instructions for python-arpy package on Arch) #614 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/614 | 10:54 |
| gitlab-br-bot | buildstream: merge request (jennis/change_arch_install_instructions->master: Change install instructions for python-arpy package on Arch) #614 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/614 | 10:54 |
| gitlab-br-bot | buildstream: merge request (tiagogomes/issue-520->master: WIP Fix race condition when calculating disk usage) #600 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/600 | 10:56 |
| cs_shadow | tristan: hi, do you have a few minutes to talk about publishing to pypi? | 11:01 |
| *** gitlab-br-bot has quit IRC | 11:04 | |
| *** gitlab-br-bot has joined #buildstream | 11:04 | |
| gitlab-br-bot | buildstream: merge request (jennis/change_arch_install_instructions->master: Change install instructions for python-arpy package on Arch) #614 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/614 | 11:09 |
| *** gitlab-br-bot has quit IRC | 11:10 | |
| *** gitlab-br-bot has joined #buildstream | 11:10 | |
| *** jennis has joined #buildstream | 11:16 | |
| tristan | cs_shadow, was just swimming in your source transform MR... gimme a sec, and yes :) | 11:20 |
| cs_shadow | tristan: sure, i figured so by looking at gitlab messages :) no rush | 11:21 |
| gitlab-br-bot | buildstream: merge request (chandan/sourcetransform->master: Allow source plugins to access previous sources) #568 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/568 | 11:24 |
| tristan | cs_shadow, ok while we're both here lets chat :) | 11:28 |
| tristan | cs_shadow, so, do you have any idea of the impliciations of installing both the distro package and the pip one ? | 11:29 |
| tristan | e.g. buildstream in both dist-packages and site-packages ? | 11:29 |
| tristan | will some exclusivity be guaranteed ? or will one take precedence over the other ? | 11:29 |
| cs_shadow | tristan: I think the answer to that depends on the distro although I think most python installtions bundled with distros would prefer dist-packages | 11:32 |
| tristan | Ok, so that's probably not a huge issue | 11:33 |
| cs_shadow | also, IIRC dist-packages is a debian thing so won't be there on fedora-based systems | 11:33 |
| tristan | cs_shadow, what about the question I asked yesterday ? How does the installation fail ? | 11:34 |
| tristan | And how do we inform the user when the pip install fails ? | 11:34 |
| tristan | Is the UX nice in that regard ? | 11:34 |
| tristan | Or, *can* we make the UX nice ? | 11:34 |
| cs_shadow | tristan: there are a few options here. The simplest option is to upload a source distribtion to pypi, in which case pip instlal BuildStream will fail in exactly the same way as it would fail from running from installing it from source (which is what it actually is). You can actually test it now using `pip3 install --user --extra-index-url https://test.pypi.org/simple BuildStream` | 11:35 |
| cs_shadow | Don't mind the versions, test pypi is running in future :) | 11:36 |
| cs_shadow | The second option is to use some kind of binary distribution additionally - wheels. But binary distributions are essentially just copied over at install time, there is no easy way to fail the install using wheels. So, we should probably stick to source distributions if we want to be able to fail the install based on system dependencies | 11:39 |
| tristan | yeah I was not familiar at all with how that works, so basically it's running the setup.py on the host, allowing us to abort when certain things are not found ? | 11:39 |
| tristan | cs_shadow, that does sound sane, and I think avoiding these wheels is also a good idea | 11:40 |
| cs_shadow | tristan: in the case of installing from source distribution (sdist), that's exactly what's happening | 11:40 |
| cs_shadow | if BuildStream ever becomes a pure-python package with no hard dependencies then we can reconsider but until then sdist seems like the safest bet | 11:41 |
| tristan | So, there is a weird case where we are depending on psutil, and indirectly the install of BuildStream fails due to psutil requiring gcc to install | 11:41 |
| tristan | cs_shadow, rather the opposite is more likely; and I think even if it were to be pure python (unlikely), we would still want better integration with the host OS than what can be provided inside the pip bubble | 11:42 |
| tristan | in any case, PyPi is probably always going to be a stop-gap solution | 11:44 |
| tristan | cs_shadow, is it setup to ensure we explicitly fail on non-linux platforms ? or is that our responsibility in the setup.py ? | 11:44 |
| cs_shadow | tristan: reg. psutil, i suppose that fails in the same way when installing from git too? If they had wheels, that may have helped :) | 11:45 |
| cs_shadow | tristan: that's up to us if we are uploading sdist | 11:45 |
| cs_shadow | do we not want to support the choot-based unix backend? | 11:46 |
| tristan | cs_shadow, sure; I'm more worried about people starting to whine that `pip install` didnt work properly on windows, or osx | 11:47 |
| cs_shadow | tristan: ah right! Maybe we can add a check for non-linux case too enforcing that they have things that we require, like `chroot` | 11:48 |
| tristan | cs_shadow, that sounds about right; but it's also true that hardening our checks in setup.py is probably orthogonal | 11:49 |
| persia | It should be safe to assume all of POSIX: platforms that can't supply that are increasingly rare. | 11:49 |
| tristan | so; we should file issues about setup.py failing gracefully on unsupported platforms and track that separately | 11:49 |
| cs_shadow | that makes sense to me ' | 11:50 |
| tristan | cs_shadow, in general; I think this is going to help a lot with the releases, and echo the sentiment of others yesterday that we should have a 'buildstream' account which we can share the keys for | 11:51 |
| tristan | cs_shadow, another question; do you have any need for pip install of dev snapshots ? | 11:51 |
| tristan | cs_shadow, to me: one of the main wins here is that it provides a quick solution to communicating the recommended release version at all times | 11:52 |
| tristan | cs_shadow, which means; I personally do not want any dev snapshots through pip | 11:52 |
| tiagogomes | Why does bst did not choose to use threads for parallelism? | 11:52 |
| tristan | tiagogomes, originally, I thought it was safer this way; juergbi and I occasionally throw toasters across the room about this topic | 11:53 |
| tiagogomes | xD | 11:53 |
| tristan | tiagogomes, one of the reasons I made that original design choice, is that right now, a third party not-so-great plugin can only cause a BUG message and BuildStream should be able to fail gracefully | 11:54 |
| tristan | A bad actor in a pluggable environment... + threads... means third party plugins can lockup BuildStream | 11:54 |
| tristan | but... there are good arguments to change the model also | 11:54 |
| cs_shadow | tristan: installing dev snapshots isn't that big of an issue for us currently. pypi isn't great at handling these either from recollection | 11:55 |
| gitlab-br-bot | buildstream: merge request (jennis/change_arch_install_instructions->master: Change install instructions for python-arpy package on Arch) #614 changed state ("merged"): https://gitlab.com/BuildStream/buildstream/merge_requests/614 | 11:56 |
| tiagogomes | Well the plugin still run in the main process no? It is the pull/build/push steps than run in a different process | 11:57 |
| gitlab-br-bot | buildstream: merge request (jmac/cas_virtual_directory->master: WIP: CAS-backed virtual directory implementation) #481 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/481 | 11:57 |
| tristan | tiagogomes, indeed some parts of it run in the main context | 11:57 |
| cs_shadow | tristan: sorry, I’m running off to a meeting. Will catch up in ~30 mins | 11:57 |
| tristan | cs_shadow, thinking of heading to the gym... | 11:58 |
| tristan | cs_shadow, anyway I think we have agreement that it's going to be useful at least in the short term | 11:58 |
| tiagogomes | ~/.cache/buildstream/artifacts/extract/ never gets clean. Result: I've 50GiB laying around on my disk | 11:58 |
| tristan | wat ?! | 11:58 |
| tristan | tiagogomes, I think tlater's cleanup stuff takes care of that, or *must* | 11:59 |
| tiagogomes | I think it cleans the artifacts in the cas cache, not extracted ones | 12:00 |
| tristan | tiagogomes, however; there is another possible cause of this - it's possible that you might have extracted artifacts lying around from before the OSTree -> CAS switch ? | 12:00 |
| tristan | tiagogomes, if you can confirm that, please file an issue :) | 12:00 |
| tiagogomes | That's true as well, but looking at the code that is never cleaned up | 12:01 |
| tiagogomes | also artifacts/cas/tmp could do with some cleanup at start | 12:01 |
| tristan | does it ever leave anything behind ? if so, that is a bug | 12:01 |
| tristan | tiagogomes, I'd rather fix it at the source of the leaked tmp directory | 12:02 |
| gitlab-br-bot | buildstream: merge request (jmac/cas_virtual_directory->master: WIP: CAS-backed virtual directory implementation) #481 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/481 | 12:02 |
| tiagogomes | It uses tempfile.Namedfile. But that is not enough if something unexpected happens | 12:02 |
| tristan | keeping a candle burning to retain support for concurrent BuildStream instances | 12:02 |
| tristan | Well, if something unexpected happens, it's a bug; right ? | 12:03 |
| tristan | One which results in leaked data | 12:03 |
| *** gitlab-br-bot has quit IRC | 12:03 | |
| *** gitlab-br-bot has joined #buildstream | 12:03 | |
| tiagogomes | Yes probably, but would be a good idea to clean at start as well for robustness | 12:03 |
| *** gitlab-br-bot has quit IRC | 12:04 | |
| *** gitlab-br-bot has joined #buildstream | 12:04 | |
| tiagogomes | Are concurrent BuildStream instances something to support? | 12:04 |
| tristan | At the cost of removing data that might be in use by a concurrent BuildStream instance | 12:04 |
| *** gitlab-br-bot has quit IRC | 12:04 | |
| *** gitlab-br-bot has joined #buildstream | 12:04 | |
| tristan | tiagogomes, the cache cleanup thing is the only thing which regresses that, indeed | 12:04 |
| tristan | we've always been very careful about that; but cache cleanup was too difficult to keep it working | 12:04 |
| tristan | well, you will in theory experience problems if you cache is not large enough for the total artifact results of both builds | 12:05 |
| tristan | but not before | 12:05 |
| tristan | s/if you/if your/ | 12:05 |
| tiagogomes | well, for that you would have to use a file for locking. There's no code for that. And I was thinking in addressing #520 not thinking in that | 12:05 |
| *** gitlab-br-bot has quit IRC | 12:06 | |
| *** gitlab-br-bot has joined #buildstream | 12:06 | |
| tristan | Yeah I think we need to still make that cache size calculation fault tolerant, as you did originally | 12:06 |
| tristan | While at the same time, it shouldnt be needed when there is only one instance | 12:06 |
| tristan | We've been avoiding file locks so far, but cache cleanup is so disruptive that I think we might no longer be able to avoid it | 12:07 |
| *** gitlab-br-bot has quit IRC | 12:07 | |
| *** gitlab-br-bot has joined #buildstream | 12:07 | |
| valentind | tristan, I have done some testing, it seems tar and ostree source plugin do not suffer of the umask issue (though tar still has the root vs. user, which I have not tested yet). git, bzr and zip however do. | 12:09 |
| tiagogomes | For multiple instances you won't be able to dodge it | 12:09 |
| tristan | tiagogomes, we were able to dodge it before cleanup, but indeed | 12:13 |
| tristan | tiagogomes, anyway I would not prioritize additional work at this time towards making it perfect; but I don't want to start allowing new code to get in which makes the situation worse | 12:14 |
| tristan | valentind, I don't think I marked ostree as an issue | 12:14 |
| tristan | valentind, and indeed, for tar it should only be the root-ness which effects the output | 12:14 |
| tristan | valentind, fwiw I've been wanting to say; it would be good to batch in as many fixed source plugins into the same branch/MR | 12:15 |
| valentind | tristan, I do everything in the same branch. | 12:16 |
| tristan | valentind, that way we can include a single commit at the end which bumps the artifact version, and avoid bumping it too many times | 12:16 |
| tristan | nice :) | 12:16 |
| valentind | tristan, Something I wonder if I need to do. Should I spawn a process, umask, then execve to the command we run? Or is it OK to just umask around the subprocess.Popen? | 12:16 |
| valentind | tristan, I know that bst does not use threading, but multiprocess, so it should be OK. But I am not completely sure about it. | 12:17 |
| tristan | valentind, you mean in the plugins ? | 12:18 |
| tristan | hrmmmm | 12:18 |
| tristan | valentind, or you mean in tests ? | 12:18 |
| valentind | tristan, from the plugins. | 12:18 |
| tristan | in tests, I worry that the parallelized tests which juergbi likes to use, might break ? | 12:18 |
| tristan | for the plugins... I hadn't considered actually setting the umask as a solution | 12:19 |
| valentind | tristan, ok. for the tests, I will see test parallelized tests. | 12:19 |
| tristan | although it does sound like a logical / easy approach | 12:19 |
| *** gitlab-br-bot has quit IRC | 12:19 | |
| *** gitlab-br-bot has joined #buildstream | 12:19 | |
| tristan | valentind, ok here is another aspect of the problem you should consider... "What attributes do we support in BuildStream projects" | 12:20 |
| tristan | valentind, my feeling is that we need to draw the line and require only the minimum there | 12:20 |
| tristan | valentind, for instance, anything from a `local` source is either 0644/0755 or a symbolic link | 12:21 |
| tristan | valentind, this is because we cannot trust that the method which the project maintainers used to store/revision their project can convey anything more than that | 12:21 |
| tristan | like, git for instance | 12:22 |
| valentind | tristan, for local it does make sense. But it is different from other cases. | 12:22 |
| tristan | yes I understand | 12:22 |
| valentind | So for local I have 0755 or 0644 only. | 12:22 |
| tristan | yes that makes sense for local; we should probably mention this in the documention of the local source itself | 12:22 |
| tristan | valentind, this also makes sense for git/bzr | 12:23 |
| tristan | valentind, probably we could benefit from a utility which implements this commonality | 12:23 |
| valentind | Yes. Something we call at the end of .stage(). | 12:24 |
| tristan | that leaves deb/tar/zip afaics | 12:24 |
| tristan | and patch | 12:25 |
| valentind | deb and tar work. But zip and patch need to be fixed. | 12:25 |
| valentind | tar does not obey umask. | 12:25 |
| valentind | (though it might if we have a tar with missing file rights, I do not know if it is possible). | 12:26 |
| tristan | right, but it will do something different depending on rootness | 12:26 |
| valentind | Yes. | 12:26 |
| tristan | separate problem, but a problem nonetheless | 12:26 |
| valentind | tristan, for tar I just go through the members and put them all to 0/0 root/root before passing to extractall. | 12:27 |
| valentind | I have not tested it. But I think it should do the trick. | 12:27 |
| valentind | I will add a test later. | 12:27 |
| valentind | Though I am not sure how to test it if we need to be root. | 12:28 |
| tristan | valentind, the approaches where it is tempting to set the umask are interesting | 12:28 |
| tristan | valentind, one thing that might be a good idea regardless... is to go ahead and add a preexec_fn argument to Plugin.__call()'s invocation of subprocess.Popen() | 12:29 |
| tristan | and set a deterministic umask for every call to a host tool | 12:29 |
| valentind | tristan, is that an undocumented feature of Python? | 12:30 |
| tristan | valentind, although it raises the question that, if the umask can have different results; should the Source implementor (zip or patch) be allowed to specify the umask it wants to be used ? | 12:30 |
| tristan | valentind, nope | 12:31 |
| tristan | https://docs.python.org/3/library/subprocess.html#popen-constructor | 12:31 |
| valentind | Ah I see, I never saw it. | 12:31 |
| tristan | valentind, all the subprocess things come back to the args provided to Popen(), they all wrap Popen() in some way | 12:31 |
| tristan | yeah, not obvious | 12:31 |
| valentind | Cool. Then it is easy. | 12:31 |
| tristan | I think we need not provide any configurability to umask, that sounds overkill, unless one day it's asked for | 12:32 |
| tristan | just use a sensible one :) | 12:32 |
| *** gitlab-br-bot has quit IRC | 12:32 | |
| tristan | valentind, does `deb` fall into the same category as `tar` ? | 12:33 |
| tristan | Anyway... I leave this in your capable hands... and go for a run :) | 12:33 |
| valentind | OK | 12:33 |
| tristan | cs_shadow, I have only reviewed the feature enabling parts, and will circle back tomorrow for the pip stuff | 12:34 |
| tristan | cs_shadow, I will be looking specifically for whether it conforms to the rules we discussed in the email thread, whether the tests assert that well, and whether we control the serialization of the "ref" in such a way that different versions of host pip could never cause the "ref" to be different due to whitespace etc | 12:36 |
| *** gitlab-br-bot has joined #buildstream | 12:39 | |
| *** gitlab-br-bot has joined #buildstream | 12:39 | |
| *** tristan has quit IRC | 12:41 | |
| *** gitlab-br-bot has quit IRC | 12:41 | |
| qinusty | Upon checking again, it appears that https://gitlab.com/BuildStream/buildstream/issues/470 was caused by the include changes valentind. It's quite a large commit but the regression appears from this commit onwards https://gitlab.com/BuildStream/buildstream/commit/171e803f5dab2644c7bcd2e22acecef64880e1ce | 13:06 |
| toscalix | qinusty: please assign it to valentind so he evaluate it. He is busy with other stuff and not neccessarily paying attention to IRC | 13:09 |
| qinusty | Will do. | 13:09 |
| toscalix | thanks | 13:09 |
| valentind | qinusty, thanks. | 13:09 |
| qinusty | If I can track it down I'll update the issue appropraitely | 13:09 |
| qinusty | s/appropraitely/appropriately | 13:09 |
| *** tristan has joined #buildstream | 13:12 | |
| valentind | qinusty, I can see how this could be a problem when an element uses (@). Not sure why it happens on other elements. We have to consider that some yaml are extended with included files and some that are not. | 13:13 |
| tiagogomes | Can I disable fail-on-overlaps through a CI option | 13:21 |
| tiagogomes | No. | 13:23 |
| *** leopi has quit IRC | 13:30 | |
| persia | tiagogomes: It's a config option. | 13:52 |
| tpollard | https://buildstream.gitlab.io/buildstream/format_intro.html#include 'The including YAML fragment has priority over the files it includes, and overrides any values introduced by the includes.' I'm not finding this line the easiest to parse | 13:53 |
| tpollard | is it saying the file that is appending the include has priority? | 13:53 |
| tpollard | i.e if project.conf has an include, any values in the project.conf have priority? | 13:54 |
| persia | When file A includes file B, if file A defines anything that is defined in file B, the value from file A overrides the value from file B. | 13:54 |
| tpollard | I thought so, thanks persia | 13:54 |
| *** jonathanmaw_ has joined #buildstream | 14:34 | |
| *** CTtpollard has joined #buildstream | 14:34 | |
| *** jonathanmaw has quit IRC | 14:36 | |
| *** tpollard has quit IRC | 14:37 | |
| cs_shadow | tristan: thanks for the review. The pip plugin has a few other pain points but its ref should be more robust | 14:44 |
| cs_shadow | I'll start the process on creating the "buildstream" pypi account, and capture necessary changes to code in issues/MR | 14:45 |
| tristan | cs_shadow, regarding pip, the install guide will need a new section for it too :) | 14:55 |
| tristan | cs_shadow, just passing comment as I noticed a bunch of emails fly by: A copyright statement in a source file always has a date, copyrights are not infinite, they expire; which is why it's important to keep updating that date with new changes | 14:56 |
| tristan | cs_shadow, if there are other dateless instances, they should probably be corrected | 14:56 |
| persia | s/updating/extending | 14:56 |
| tristan | right, IANAL, but that much I think I've understood well enough :) | 14:57 |
| persia | So, if previously "Copyright 1987", change to "Copyright 1987, 2018" if adding code. Alternately, if "Copyright 2017", change to "Copyright 2017-2018" if adding code. Only change copyright lines that represent an entity to whom you are assigning copyright whilst you write the change. | 14:57 |
| tristan | persia, point of interest, I've been asked by clients before to never do "Copyright 2017-2018 ..." | 14:58 |
| persia | (and in some countries, copyright no longer expires, but that's potentially a temporary condition). | 14:58 |
| persia | tristan: Interesting. What is the recommendation? "2017, 2018"? What about when three years apply: does that make it "2016-2018"? | 14:58 |
| tristan | persia, as it arguably looks like one made a range statement, like one is opting out of copyright after the said range is complete | 14:59 |
| * persia has never received any guidance on that particular point, so only has an opinion based on potentially incorrect examples | 14:59 | |
| tristan | One could technically add a new line per modification year, but I've been asked to just change the date, since it's the only relevant part for the file (last time a given holder has updated it) | 14:59 |
| cs_shadow | okay, i can ask around about copyright internally and update the files accordingly | 14:59 |
| persia | tristan: Was that a qualified opinion? I didn't think there was a way to opt out of copyright unless one qualified as a westphalian sovereign. | 15:00 |
| persia | cs_shadow: Excellent strategy :) | 15:00 |
| persia | tristan: Oh, I have received opinion that the first year of publication is relevant for certain classes of argument, although that might only be in certain jurisdictions. | 15:03 |
| persia | I suppose the answer is, as usual, "ask your lawyer" :( | 15:03 |
| tristan | yes indeed | 15:03 |
| tristan | persia, while I personally think that; with the maintainer hat on; I will accept any sensible format so long as the contributing party is the one who may be responsible for defending it in a court... | 15:04 |
| tristan | persia, I also think it's not a good idea to accept "Copyright Foo Inc." without a date, which appears like an overreaching statement of permanent copyright | 15:05 |
| tristan | i.e. "Its your problem" is the right attitude, but I think "I own this forever" is just not a very nice statement | 15:06 |
| persia | I don't think the dates mean that. | 15:09 |
| persia | But I agree that Copyright should include a date. | 15:09 |
| cs_shadow | i think one argument for not having dates written down is that the VCS system already has all this information available in a much nicer way | 15:12 |
| persia | http://www.wipo.int/wipolex/en/treaties/text.jsp?file_id=283693 doesn't seem to contain any indication of the appropriate format. | 15:13 |
| persia | It does indicate that a) term of protection is the *shortest* term in any jurisdiction of all the signatories (suggesting later dates), and that governing law is affected by the country of first publication (suggesting earlier dates). | 15:14 |
| persia | VCS might have everything, but I'm not sure that VCS history is considered authoritative, and there have been cases where things were modified (to comply with copyright) wherein the VCS history includes the modification, suggesting that VCS history may not matter. | 15:15 |
| persia | (but, as with any of these things, one can't be sure until one is in court about one's particular issue, which complicates determining appropriate best practice) | 15:15 |
| *** brackets has joined #buildstream | 15:37 | |
| *** leopi has joined #buildstream | 15:51 | |
| jjardon | Hi, when I define max-jobs in buildstream, is this definine the maximum or actually the exact number of jobs allowed? | 16:06 |
| jjardon | seems it's used as "make -j%{max-jobs}", and AFAIK that define the exact number, not the maximum | 16:06 |
| persia | Make's -j only defines the maximum parallelism. You can test this by compiling a simple one-file c program with make -j99. | 16:11 |
| persia | For any reasonable project, the parallelism approaches the maximum (assuming the dependency tree permits). | 16:11 |
| * persia doesn't know precisely how the buildstream implementation works, but strongly suspects it to suffer from the same limitation, in that exact number of processes is limited by the work that can be done, regardless of how high a value is used for "max-jobs" | 16:12 | |
| jjardon | persia: mmm, readin the make manual I think it actually specify the exact number: "-j [jobs] Specifies the number of jobs (commands) to run simultaneously" | 16:13 |
| persia | jjardon: Yes. That's arguably a documentation bug. If there is only one file to compile, make only runs gcc once, regardless of the job count. | 16:13 |
| jmac | We could make buildstream run 99 copies of the same job, and compare the outputs, if you like | 16:14 |
| persia | As an experiment (doesn't require a makefile), create a .c file in a directory, then run `make ${basename}.o` in that directory. Make's implicit rules shoud cause it to compile, but -j99 won't make it compile 99 times. | 16:14 |
| persia | jmac: I think that is an interesting potential option, but I think it deserves to be something other than "max-jobs". | 16:14 |
| jjardon | persia: I would not expect make to do that, because that's only a single command | 16:16 |
| persia | Yes. When make cannot (usefully) reach the number of jobs, it runs fewer jobs, hence -j not specifying the exact number. I believe buildstream to behave similarly. | 16:16 |
| persia | I also think this behaviour is the least surprising, for all that it makes documentation hard to write precisely. | 16:17 |
| jjardon | persia: ok thanks | 16:20 |
| jjardon | ah, ok: buildstream/_project.py: output.base_variables['max-jobs'] = str(multiprocessing.cpu_count()) | 16:20 |
| jjardon | problem I have here is that I'm in a quite powerfull machine, and several bst and running in docker images. Because they do not know they share the same machine, they all run with -j96, making everything slower | 16:21 |
| persia | Oh, heh. That's where the make server function comes in. | 16:23 |
| * persia looks up the relevant issue | 16:23 | |
| Kinnison | jjardon: You get to specify max-jobs for those images I guess :-) | 16:24 |
| Kinnison | persia: It's nearly impossible to do that safely without risking livelock over time | 16:24 |
| persia | Kinnison: That might be why I can't find a relevant open issue :) | 16:25 |
| Kinnison | Possibly | 16:25 |
| jjardon | I was thinking on putting max-jobs to 12 in my bst project, but then I do not want other runners (with only 4 cores) to _try_ to generate 12 threads | 16:26 |
| jjardon | I think paulsherwood did some research some time ago and concluded that at some point the number of threads doesn't matter, even if you have a lot of cores | 16:27 |
| Kinnison | jjardon: Hmm, yeah, you want something which is min(cpu-cores, 12) don't you? | 16:27 |
| * jjardon needs to search his email archive | 16:27 | |
| jjardon | Kinnison: yup | 16:27 |
| * Kinnison is not sure if you can do that statically or if you'll need something to calculate that and pass it into bst build | 16:28 | |
| * Kinnison is also unsure if -o does that | 16:28 | |
| * Kinnison is still new here | 16:28 | |
| persia | jjardon: the research results are machine-dependent. Essentially, builds end up memory or IO bound fairly easily on machines with lots of cores. | 16:29 |
| persia | My recommendation is to pick a sensible number: if the source being built works well with 8 threads, then make it 8, or pick a smaller number. | 16:29 |
| persia | The only time you really want to run more threads on the lots-of-cores box is when a) you know there is little contention, and b) that machine also has very high-spec memory/IO. | 16:30 |
| jjardon | ah, here it is: https://gitlab.com/baserock/ybd/commit/580b7da0e93bf5717148ace075f6224b76d65d4e https://gitlab.com/baserock/ybd/commit/54fc05733277e914b57150b716df86ae19c4597a | 16:32 |
| jjardon | persia: yeah, so probably the default to max-jobs == cpucores should be changed/improved in buildstream | 16:32 |
| persia | jjardon: That only works for fairly small machines. Unfortunately, it happens to work for >99% of machines developers use on a daily basis, which tends to keep it popular for folk who compile on laptops or similar. | 16:33 |
| Kinnison | jjardon: that seems to be suggesting that beyond 10 cores, you want more builders (--builders N) rather than increasing the -j | 16:34 |
| jjardon | yeah | 16:34 |
| Kinnison | Typically that'll be because most software doesn't usefully parallelise beyond about -j10 | 16:34 |
| Kinnison | probably because of interdependency choke points (e.g. on .a files) | 16:34 |
| jjardon | persia sure, this is to improve the max-jobs for very big machines only | 16:34 |
| persia | Or very small machines. | 16:35 |
| persia | Kinnison: My personal experience with falsely constructed compilation environments that supported *very* wide parallelism was that gcc deals with iowait particularly terribly well before reaching the parallelism available in the project. | 16:36 |
| jjardon | buildstream doesn't a concept for "several instances / builders" right ? | 16:36 |
| jjardon | tristan: ^ | 16:36 |
| Kinnison | jjardon: bst has a --builders | 16:36 |
| persia | Mind you, that was a 96-core machine with a single 5400rpm drive, so perhaps a special case :) | 16:36 |
| Kinnison | jjardon: bst is inherently parallel internally | 16:36 |
| Kinnison | persia: eww | 16:37 |
| jjardon | Kinnison: oh, that make things even more interesting | 16:37 |
| persia | jjardon: I think your application pushes the edges of the remote execution model. | 16:37 |
| WSalmon | i was talking to persia about overlay file sytems and i cant seen any reference to them but i may not have looked hard enough, bst isnt using them is it? | 16:37 |
| jjardon | persia: we are not using remote execution, these are local build | 16:37 |
| jjardon | builds* | 16:38 |
| jjardon | persia: or are you saying we should use/implement remote execution model, wherever that is? :) | 16:38 |
| persia | jjardon: Yes. I suspect your needs (multiple parallel runs of buildstream on real servers) are likely better met by remote execution. Or at least I hope remote execution is designed to handle more than laptops. | 16:39 |
| tristan | jjardon, passing comment hit and run... --builders defines the maximum number of build tasks (parallel builds)... while -j is currently defaulting to the number of available cpus... which often works well because the kernel's scheduler should be good enough, and many parallel jobs are I/O bound when a cpu intensive job is active (I can build many modules while webkit builds, they are mostly doing ./configure most of the time) | 16:40 |
| jjardon | persia: ok; this is a single server, with 98 cores, how remote execution will help here? what version of buildstream supports that? | 16:40 |
| persia | tristan: The kernel's scheduler is totally incapable of dealing with builds for more than about 12 cores. | 16:40 |
| tristan | jjardon, that said... we need to look into a job server to share the available tasks to do it better; problem with that is we'd want one job server, to talk to ninja, make, and any other build system in parallel | 16:40 |
| tristan | persia, still; so far as far as I can see, it's always better to have 4 build *ask* for all the available CPUs, since most of the time they will be running ./configure and be I/O bound, than the alternative, which would be to use less than the available CPUs for a build of webkit, and let other stuff idle | 16:42 |
| tristan | having a single job server which talks to every different kind of BuildElement, would still be the ideal | 16:42 |
| persia | tristan: Try the experiment on a machine with >40 cores, and you'll see surprising results. | 16:43 |
| persia | My guess is that cache eviction due to "fair scheduling" ends up causing signifcant performance degradation, or something like that. Can't really be sure. | 16:43 |
| tristan | persia, not sure I'll be surprised, it sounds like if you want to allocate that many jobs, you will eventually bottleneck on I/O anyway | 16:44 |
| persia | No, point being that it's worse than just IO bottleneck. | 16:44 |
| tristan | How many computers which are likely to run buildstream, have 40 cores ? | 16:45 |
| persia | Once one reaches IO bottleneck, one wants to stop increasing parallelism. Further parallelism beyond that ends up causing additional problems due to scheduling, etc. | 16:45 |
| persia | All the servers, basically. | 16:45 |
| tristan | Anyway; no point really... it's late; if we want to hard code a user config option for it, that's plausible | 16:45 |
| persia | It's hard to purhcase a server with less than 24 "cores" these days, and >40 is fairly normal. | 16:45 |
| tristan | But a job server that talks every build system language (not *just* make), is the better solution | 16:46 |
| persia | Yes, ideally one that also tracks IO load, and can balance things sensibly :) | 16:46 |
| tristan | persia, even if a build server has that many cores; and even if a build server runs many more builds than developers do; those build servers are still outnumbered, I think | 16:46 |
| jjardon | tristan: I'm managing 3 :) | 16:47 |
| persia | Possibly, but if performance is terrible on a build server, it's easier to tell folk to use a different tool that works on servers. | 16:47 |
| tristan | by the developer laptops doing builds, and we need to also optimize for those; because that's where first time users will get an impression | 16:47 |
| tristan | Again, user config option is a possibility, job server is ideal; the current default is not that horrible, though | 16:47 |
| persia | tristan: No, we need to *support* both use cases. There's no point in saying "we optimise for this vs. that". We just choose a metric that happens to work for everyone. | 16:48 |
| persia | Number of cores is a terrible metric: it's increasingly easy to buy desktop workstations with 24 or 32 "cores", which end up behaving badly with that metric. | 16:48 |
| tristan | persia, for the third time before I finish this hit-and-run conversation and get back outside of this house... a config option is a possibility | 16:48 |
| persia | (and 16 or 20 "core" laptops are in the market) | 16:48 |
| tristan | It probably takes less time than talking about this on IRC | 16:48 |
| jjardon | tristan: and you have access to all of them, btw :) I will try to send a patch (only to change the default fo high number of cores) so we can discuss there pros and cons | 16:49 |
| persia | tristan: My preference would be to fix the code to have a good metric, but yeah, we could use a config option if we can't think of one. | 16:49 |
| tristan | persia, still, the *default* in the absence of a config option, should be a default chosen to optimize for the developer laptop (config is per installation/computor) | 16:49 |
| persia | For now, we have max-jobs, which covers the basic case. | 16:49 |
| persia | My point is that within a year or two, numcpus will be terrible also for developer laptops. | 16:49 |
| persia | *handheld* devices are often sold with 10-12 user-accessible cores. Add hyperthreading, and it all falls down. | 16:50 |
| tristan | patches welcome :D | 16:50 |
| * tristan hits the road | 16:50 | |
| persia | Have a good night :) | 16:51 |
| persia | jjardon: You probably want to use max-jobs (with a low value) for now, and maybe open an issue describing the problem you're having in enough depth that it can be thought about properly? | 16:52 |
| *** jonathanmaw_ has quit IRC | 17:07 | |
| *** xjuan has joined #buildstream | 17:07 | |
| *** GigabytePro has joined #buildstream | 17:16 | |
| *** toscalix has quit IRC | 18:12 | |
| *** leopi has quit IRC | 18:46 | |
| tristan | I think the point I was trying to get across, is that you *cannot* set max-jobs, i.e.; someone just has to write a patch to offer a config option that sets it | 18:55 |
| tristan | If you can set the variable in project.conf, and it actually "just works", I'd be surprised | 18:56 |
| *** cs_shadow has quit IRC | 19:21 | |
| *** tristan has quit IRC | 20:40 | |
| *** xjuan has quit IRC | 22:33 | |
| persia | tristan: Indeed. If one needs to build on a wide range of hosts, one can only set to a ridiculously low value. Someone has to write a patch tha t does something sensible. I'm not convinced it should be a new/different/special config option: it may be that the current default can just change in a sensible way. | 22:43 |
| *** bochecha has quit IRC | 23:18 | |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!