IRC logs for #buildstream for Monday, 2020-05-11

tristanvalentind, it's a breaking change, I don't think anyone had it in mind to backport it04:08
tristanvalentind, it replaces junction name coalescing, and is unfinished since it completely disallows multiple junctions to the same project04:09
tristanI think that you can still achieve whatever you need without this though, just with less error protections04:09
tristani.e. you can still achieve the overrides with junction name coalescing04:10
tristanvalentind, !1913 looks out of the blue to me, fwiw I think that that is not really a problem space that is specific to junctions04:16
tristanif you had everything in a single project, you will still find areas where projects have cyclic dependencies; for example you normally would depend on an early build of busybox, but at later stages you would want to *replace* that with coreutils and bash04:17
tristanso it becomes desirable to swap out the overlapping busybox with later reverse dependencies04:18
*** tristan has quit IRC04:41
*** narispo has quit IRC05:00
*** narispo has joined #buildstream05:00
*** aminbegood has joined #buildstream06:20
*** benschubert has joined #buildstream07:02
*** aminbegood has quit IRC07:10
*** phildawson has joined #buildstream07:45
*** jude has joined #buildstream07:48
*** pointswaves has joined #buildstream07:52
*** pointswaves has quit IRC07:55
*** seanborg has joined #buildstream08:04
*** rdale has joined #buildstream08:17
coldtomthe issue isn't cyclic dependencies aiui, it's any time you want to use something from a junction but modify the build somehow08:18
coldtomsure you can patch the junction, or use an overlay, but these both feel hacky and like you're doing something kinda wrong08:18
*** tpollard has joined #buildstream08:20
*** seanborg has quit IRC08:22
*** seanborg has joined #buildstream08:22
*** seanborg has quit IRC08:23
*** seanborg has joined #buildstream08:23
*** santi has joined #buildstream08:30
juergbicoldtom: I think we need to distinguish the different use cases08:34
benschubertcoldtom: not sure why a `patch` would be hacky there? It's a well known and used practice: Archlinux, Debian, Ubuntu, Fedora, they all use patches on top of their sources, usually using `quilt`08:34
*** aminbegood has joined #buildstream08:36
benschubert(the `patch` has the advantage of being an explicit well recognized mechanism :) )08:36
juergbiI'd rather tend towards git branches08:37
juergbibut that's mostly a matter of personal preference08:37
coldtombenschubert, they patch their sources, but i don't really see junctions as the same as a regular source - it's more a metasource to me, and so it feels very different to patch it08:37
juergbiand what's more convenient depends on how frequently changes are applied upstream/downstream08:37
benschubertjuergbi: true. I tend to prefer `quilt` as I find it easier to grasp the exact changes done08:38
WSalmonbuildstream2 pulls from the bottom of the tree rather than the top of the tree, AFAICT it dose not skip build deps of my build deps and this coupled with bst not ciurciting pushes for artifacts on the server, means that some of my jobs on runners that do not always have a hot cache are taking a hug amount of time, it also means that developrs have to have to down load a lot more than they need, am I missing something? I used to have CI stages that08:40
WSalmonwould `bst build` then `bst checkout` if the cache server was up but the local cache was cold they would download one thing and then check it out, currently they have to download and upload 100s of things..08:40
juergbiWSalmon: bst build should not be pulling build-only dependencies by default, unless they are needed in the current session08:41
juergbiif you see this happening, this sounds like a bug. however, I think we already have this covered by tests08:42
juergbiand bst artifact checkout should only pull the dependencies as per --deps option. default is --deps run, though, not --deps none08:46
juergbibuild-only dependencies should not be pulled unless --deps build or --deps all is specified08:46
WSalmonbut i need to run bst build for the rare cases when the last one or two elements have been expired cos i havent used that branch for a while..08:48
juergbias I mentioned, bst build should not be pulling unneeded build-only dependencies either by default08:48
juergbiif you see this happening, a test case would be ideal08:48
WSalmonok, i dont really understand how it can know what it needs when it pulls from the bottom up08:49
juergbiinternally it starts top down08:50
WSalmoni will investigate the case were it looks like its pulling build deps of build deps08:50
juergbihowever, it determines fairly early which build dependencies are definitely needed08:50
juergbiand it prioritizes those from the bottom of the stack as they are needed first08:50
cphangWSalmon could you link to CI jobs that show this occurring?08:52
*** mohan43u_ has joined #buildstream08:54
juergbiWSalmon: hm, maybe we actually optimize build-only dependencies away only based on the local cache08:55
*** mohan43u has quit IRC08:55
juergbiso if nothing is local yet we would always pull all elements08:55
benschubertjuergbi: I *think* it's the case yes08:55
juergbithat would definitely sound like an area that we could improve08:56
benschubertfixing that is not a trivial change in the pipeline though (need to be able to dynamically add stuff, which it was not really made to do).08:56
benschubert+1 on fixing that and make the pipeline really push-based :)08:56
*** mohan43u_ has quit IRC08:56
*** mohan43u has joined #buildstream08:57
juergbiI thought we already made it more dynamic than that a long time ago but I'm probably mixing something up08:57
WSalmoncphang, im going to double check but i think the bootstrap stuff is build-dep of a build-dep of the targets for https://gitlab.com/freedesktop-sdk/freedesktop-sdk/-/jobs/546340519 but im not 100% and im not 100% that there is enough in the cache so i was going to check. but from what benschubert and juergbi are saying then it sounds like that is whats happening08:58
*** tristan has joined #buildstream08:59
*** chipb_ has joined #buildstream09:00
*** chipb has quit IRC09:00
juergbitristan: junctions mail, last paragraph, I'm a bit confused by "we might have both of these appraoches". do you mean "might need/want"?09:01
*** ChanServ sets mode: +o tristan09:03
tristanjuergbi, yes.09:03
benschuberttristan: let me know when you have a few brain cycles free to discuss cache key tests :)09:03
juergbibenschubert: https://gitlab.com/BuildStream/buildstream/-/merge_requests/1718 removed the set_required logic09:04
juergbididn't this cause the regression noticed by WSalmon?09:04
tristanbenschubert, brain cycles I have, I expect to be interrupted very soon though09:05
benschubertjuergbi: https://gitlab.com/BuildStream/buildstream/-/merge_requests/1718/diffs#11743b796594142e47df22054b99d263d52e28aa_1625_1559 should have been safe. AFAIK we did have this problem for longer than that. Might want to try though if it indeed was a regression09:06
tristanbenschubert, but if we start, I can ponder this in the background and come back in a couple hours...09:06
benschuberttristan: sure :) Great. So for the cache-key tests, I wanted to allow plugin projects to be able to run them without writing the whole logic09:07
tristanbenschubert, from a general perspective, I think that BuildStream should be offering API in buildstream.testing which plugin projects can use to register explicit tests in their own pytest test suite09:07
tristanI'm not particularly fond of the "Here buildstream: This is my plugin, now test it !" approach09:07
tristanBut that's just a general feeling I have about buildstream.testing09:08
benschubert2 ways I can see that:09:08
benschubert1) We add a new method to the `Repo` that returns a project/elements and bst tests on this. Plus: it's very visible, minus: only for sources09:08
benschubert2) Provid a set of "check" methods that plugins can use for testing, which can be much more general afterwards (when we see patterns, we add a helper and tada)09:08
benschubertok I guess the 2. would therefore be the better solution09:08
tristanRight I think so09:09
benschubertSo provide a `check_cache_keys_stability(path_to_project)` method that expects a project with the same layout as we have now in bst for cache keys as a start?09:09
tristancache key tests are fairly simple, but they do require some responsibility and work on the authors part09:09
tristanExporting the function that does the checks would be helpful09:10
benschubertcheers, I'll move that way then09:10
benschubertthanks!09:10
tristanbenschubert, I would be happy with that, I think that since this is an external API we'll want to document that function well09:10
benschubertyep definitely09:11
tristanexplaining how it works09:11
tristanMaybe we want to also provide the update program09:11
tristanwhich consequently also generates the .expected files when you first setup your test09:11
benschubertDo we have a `plugin author` documentation part?09:11
tristanPlugins I suppose we do not have full expectations of API stability or stability of cache keys (except for a subset of plugin projects which decide to offer that)09:11
tristanSo the update program is also useful09:12
tristanbenschubert, unfortunately not really, it's the last piece of docs we don't have a clean story for09:12
tristanwe have some in the Element/Source/Plugin docs but that's it09:12
* tristan has to run now...09:12
benschubertyep good point. I'm not 100% sure how we'd end up exposing this. A new entrypoint that's added with `pip install buildstream[test]` ?09:12
benschuberttristan: ok. Fine with postponing the docs part of it?09:13
benschubertIf someone has time, i'd appreciate reviews on! !1905, !1906, !1907, !1911 and https://gitlab.com/BuildStream/bst-plugins-experimental/-/merge_requests/10809:15
*** Frazer has joined #buildstream10:04
Frazerhi, is it ok to use an example in https://gitlab.com/BuildStream/buildstream/-/tree/master/doc/examples for this project https://github.com/github/linguist/tree/master/samples . this should hopefully add BuildStream as a known language to gitlab?10:10
*** radiofree has joined #buildstream10:42
radiofreeHello!10:44
radiofreei'm trying to get --format ${deps} to work, but it only ever seems to print "%{deps}"10:44
radiofreee.g using the example from the test (cd buildstream/tests/frontend/project)10:44
radiofreebst show --deps all  checkout-deps.bst (shows dependencies)10:45
benschubertradiofree: what version of BuildStream are you on?10:45
radiofreebst show --deps all --format "%{name}: %{deps}" checkout-deps.bst10:45
radiofreethis prints "import-dev.bst: %{deps}"10:45
radiofree1.4.110:46
benschubertI don't think bst 1.4 supports '%{deps}'10:47
benschubertis it listed when you do `bst format --help` ?10:47
radiofree"Error: No such command 'format'."10:48
coldtomradiofree, bst show --help10:48
coldtomapparently it's not there10:48
radiofreeah, no10:49
benschubertah right, show sorry10:49
radiofreethat'll be why then :)10:50
radiofreemust have been looking at the documentation for master10:51
benschuberttristan: https://gitlab.com/BuildStream/buildstream/-/merge_requests/1915/ how about this? It's lacking the script and the documentation, but that might be easier to add later?11:04
jjardonabderrahim[m]: have you seen https://gitlab.com/BuildStream/buildstream/-/merge_requests/1914 ? looks like a clean solution to the overlaps problems we have (or cleaner than the current one at least)11:29
tristanFrazer, The examples are meant to be usable samples, the intent is that anyone can use them12:14
Frazertristan thanks, just making sure im not breaching anything12:19
*** aminbegood has quit IRC12:21
*** aminbegood has joined #buildstream12:21
tristanFrazer, yeah we don't have copyright/license headings in the bst files in BuildStream proper, it would probably make the documentation which those yaml files get literally rendered into very noisy and hard to read12:24
tristanMaybe we need to clarify that somehow12:24
tristanbenschubert, I added a comment there, generally yeah looks good12:25
benschuberttristan: cheers, I'll address the comments, move the update script and such then12:25
WSalmontristan, are they not covered by the main copyright notice and so tecnically lgpl?12:25
tristanbenschubert, my comment is mostly: Lets make sure that everything public gets imported directly from `buildstream` or from `buildstream.testing` respectively, and not add new namespaces12:25
tristanWSalmon, I'm not sure12:25
tristanWSalmon, I think they are technically lgpl or fork-and-do-what-you-want-we-dont-care (even less binding than lgpl), but I can't say with exact certainty that we've expressed that correctly12:27
benschuberttristan: ah good point, I'll move that to `from buildstream.testing`12:27
jjardontristan: would it be possible to release 1.4.3 soonish? I think the CI is fine now?12:44
tristanjjardon, CI is not fine12:47
tristanjjardon, https://gitlab.com/BuildStream/buildstream/-/merge_requests/1917 should make it fine12:47
tristanin theory12:47
* tristan waits to see if this passes again12:48
tristanjjardon, I can backport that to 1.4 branch if we want a new 1.4.x sure12:48
tristanjjardon, there is stuff you want in 1.4 soon then ?12:49
jjardontristan: do we have a 1.4 branch? any new feature in bst-1 ?12:49
jjardontristan: we need something is already in bst-112:49
jjardonwe are using a patched bst-1 version at the moment12:49
tristanjjardon, not exactly yet, but I branched with the intention of landing https://gitlab.com/BuildStream/buildstream/-/merge_requests/1912 on it12:49
tristanand then CI blew up12:50
tristanjjardon, ok we'll roll another 1.4 then, no worries12:50
jjardontristan: ah, ok. Nice thanks a lot!12:50
tristanI wanted to focus on getting bst-2 detection and other stuff into 1.6 but a micro point 1.4 is no trouble12:50
jjardontristan: maybe more for 1.6, but maybe you can take a look  to https://gitlab.com/BuildStream/buildstream/-/merge_requests/1914  as well; that will help junctions a lot12:51
jjardonin our case it will simplify GNOME's junction of fdsdk and it will make it less "hacky"12:51
tristanjjardon, I think there is an ongoing conversation about that12:52
tristanI frankly dislike the idea of anything injected into a junctioned project, we're working on providing more clarity around that and ensuring less of that happens, not more12:52
tristanthis looks like a whole other MR, where is the original one12:53
jjardontristan: in GNOME we need to replace glib, and other libraries; the current solution with filters is not ideal, and quite fragile12:53
benschubertthe other MR is !1913. I would be against merging anything there until we had a ML discussion though12:54
tristanjjardon, to repeat my comment on the original (probably master targetting) MR: We need to work on making it easier for g-b-m to consume the dependencies with glib removed, not work on allowing g-b-m to modify how fdsdk cooks glib12:54
jjardonah sorry, I didnt see the comments at https://gitlab.com/BuildStream/buildstream/-/merge_requests/191312:55
* jjardon reads12:55
tristanbefore we ever started buildstream, I was thinking of a dependency semantic "replaces", where once you get up to a certain part of the stack, you could have new elements replacing what gets staged at lower levels (this was from YBD/morph days)12:57
tristanI'm not sure it's exactly the right solution, and in BuildStream I would implement it differently12:58
tristanHowever, I do think it's interesting to note that this problem does not _only_ present itself because of junctions12:58
tristanjjardon, I think you were present years ago when we discussed how to get coreutils/bash to replace busybox once they are built and busybox is no longer needed12:59
tristanI think this is effectively the same thing (although a different use case)12:59
jjardonyep, I think I remember13:00
tristanSimilar situations arise with cyclic build dependencies too, where for instance you have to build something without support for another thing, in order to build that other thing, so you can come back and build the first thing with support for the other13:02
* tristan seems to recall something like that happening with libxml213:02
juergbivalentind: can we close !1914 at least for now? I think it's only confusing/noisy if there is already a backport MR open for a feature that is still being discussed for master13:18
juergbi(I don't have objections to the branch for testing, of course, but there is no need for the MR right now)13:19
*** cs-shadow has joined #buildstream13:19
tristanAnyone wanna look at !1917 ?13:47
valentindjuergbi, It is WIP.13:51
*** seanborg_ has joined #buildstream13:51
*** seanborg has quit IRC13:52
juergbivalentind: sure but what's the point in already opening an MR? discussing the backport is premature, imo. and it adds noise to the MR list13:52
valentindI will unmark WIP the master one first. Until it is resolved. But before I unmark I want the opinion of other people from g-b-m.13:52
valentindjuergbi, It is to have a link to what has been effectively tested.13:53
tristanI think the point is that so far, there is a lot of doubt around that MR landing in buildstream at all, and we would like to see discussion on the list about the appropriate way to handle this class of problem13:59
tristanif it turns out that we really have to resort to overriding things in junctioned projects, I think that would be a last resort, but lets discuss other options first14:00
tristanbenschubert, since you're working through plugin testing, do you know where the ostree `repo` implementation is located in the `bst-plugins-experimental` repo ?14:02
tristanAh I found it14:03
benschuberttristan: not by heart, let me have a look (we need to reorganize all of this -_-)14:03
tristanbenschubert, there is a `testing` module in src/bst_plugins_experimental14:05
benschubertthat doesn't make sense :/ it should be in tests/14:05
tristanwhich... is a little strange but might have to do with how buildstream itself uses bst_plugins_experimental in order to test it's own regressions against plugins14:06
benschubertno, we should not have to use those14:07
tristanwe don't use the `repo` implementations from plugin packages ?14:09
tristanI'm really fuzzy on this tbh14:10
juergbitristan: !1917 looks reasonable to me but I'm a bit surprised that the gpg stuff requires that many files14:10
tristanI don't know heh14:10
juergbitristan: also, is pubring.kbx~ really needed or was this added accidentally?14:10
tristanjuergbi, I just created a gpg key with the homedir set to that directory, and it generated all that14:10
tristanI found the ~ file strange as well14:10
tristanjuergbi, it's possible that it works without all of the files, hard to know which ones are hard required14:11
benschuberttristan: actually could we just create them on the fly?14:11
benschuberttristan: we do, but from the tests of bst-plugins-experimental normally14:12
benschubertso it could just live there14:12
tristanbenschubert, what are you talking about, the GPG keys ?14:14
benschubertyes sorry for the context switch14:14
tristanHmmm, it's a bit awkward to do, ostree seems to store a key/homedir for this purpose too14:15
tristanit's a bit different than this one14:15
benschubertok14:16
tristanWell, the answer is that we probably could; but it'd take a while ;-)14:16
tristanone thing which is annoying is I had to find out the name of the key with `gpg --list-keys`14:16
tristanprobably there is a way to name a key and avoid all that14:17
benschubertah sure, let's not bother then14:17
tristanI've also got: https://gitlab.com/BuildStream/bst-plugins-experimental/-/merge_requests/10914:18
tristanThat test passes locally, we'll see what it does in CI14:18
benschubertWho's in charge of  buildstream-bastion-overnight  ? I can't get a nightly build to finish: https://gitlab.com/BuildStream/buildstream/-/jobs/54641911814:47
*** aminbegood has quit IRC14:56
*** phoenix has joined #buildstream15:09
*** seanborg_ has quit IRC15:24
*** seanborg_ has joined #buildstream15:24
benschubertjuergbi: ^ do you know by any chance? :)15:45
juergbibenschubert: benbrown, jjardon, valentind ^^15:46
benschubertoh thanks15:48
benschuberttristan: looking at the script to automatically update cache keys. It's tricky:15:51
benschubert- We don't want that in entry_points for BuildStream, as it would pollute installs15:51
benschubert- We can't really have a switch in the check method itself as we might be working in a tmpdir so our change would not persist.15:51
benschubert- Having an ad-hoc script would still mean users need to copy it and won't have nice interface.15:51
benschubertSo... should we provide the function to update and users are left to add it in a python script? Any other idea?15:51
cs-shadowcan't we do something like `python -m `buildstream.testing.update_keys` ?15:54
cs-shadowand with correct quoting - `python -m buildstream.testing.update_keys`15:55
benschubertoh that's a good point. I'll go for this15:55
cs-shadowthis method,could in turn call the underlying method for checking/rewriting the cache keys. The difference compared to tests would be that this will pass the real path to the tests, not a tmpdir15:56
WSalmonjuergbi, i have no idea what https://gitlab.com/BuildStream/buildstream/-/merge_requests/1718/diffs dose it is not code i am familiar with, do you think this patch is way buildstream always seems to pull from the bottom up?16:02
juergbiWSalmon: possibly. if you have time to create a test case for this pull issue, I can try to revert the relevant commit16:04
valentindbenschubert, Sorry, did not see. Do you still have a problem with the CI?16:10
benschubertvalentind: yeah, didn't get a single one ending in 4 tries, that's the nightly build so it takes a very long time16:10
valentindIt looks like the timeout was reached.16:11
valentindTimeout: 20h (from project)16:11
benschubertvalentind: no that's the total timeout. Time was 305min before it was killed16:11
benschubert(Duration: 305 minutes 53 seconds )16:12
benschubertso the runner times out roughly after 5 hours?16:12
WSalmonjuergbi, sounds like a plan16:12
valentindRight. I think it is 6.16:13
valentindbenschubert, I can change that.16:13
valentindThe problem is that the gitlab runner leaves so many dead builders that we have to forcefully clean them up.16:13
benschubertvalentind: thanks! Otherwise we can reduce what we build for the nightly fsdk test, which is also fine by me16:13
valentindI thought it was 12 hours. But maybe it was 6. I have to check.16:13
benschubertsure, keep me updated, thanks16:14
valentindbenschubert, I put the timeout to 12 hours.16:17
benschubertthanks! I'll see if I manage to finish a build now :)16:18
valentindThat might still happen. It would just be nice if gitlab fixed the runner and cleanup correctly builders.16:18
juergbivalentind: I assume that issue is related to the docker-machine digitalocean issue (API limits)16:19
valentindjuergbi, There are 2 bugs I think.16:20
juergbikubernetes is probably the best long term bet. possible alternatives are to reduce the number of DO API calls in the docker-machine backend or moving away from DO16:20
valentindBut yes it is around docker-machine.16:20
valentindSure. Though I will not touch kubernetes. I do not have time for that.16:21
juergbisure, I understand. same here16:21
*** jude has quit IRC16:28
*** santi has quit IRC16:37
*** santi has joined #buildstream16:42
*** pointswaves has joined #buildstream17:30
*** seanborg_ has quit IRC17:34
*** santi has quit IRC17:59
*** chipb_ is now known as chipb18:31
*** tpollard has quit IRC18:41
*** aminbegood has joined #buildstream19:37
*** aminbegood has quit IRC20:07
*** toscalix has joined #buildstream20:22
*** cs-shadow has quit IRC21:09
*** toscalix has quit IRC21:29
*** phoenix has quit IRC22:11
*** pointswaves has quit IRC22:20
*** benschubert has quit IRC23:00
*** rdale has quit IRC23:32

Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!