*** tristan has quit IRC | 06:29 | |
*** shibu has joined #buildstream | 07:14 | |
*** tristan has joined #buildstream | 07:36 | |
*** bochecha has joined #buildstream | 07:48 | |
benschubert | hey tristan, juergbi are you around? I am clear on what API I think is best for _yaml.Node, but that requires lots of changes in the codebase. Would you prefer a one-time sweeping change with all the API done, adding the new API only with minimal uses, or piece by piece (removing 1-2 functions per MR). I'm leaning towards the last one, and have opened a WIP MR !1401 so we can start a discussion. I'm happy to move to a one-time sweep if you prefer | 08:15 |
---|---|---|
gitlab-br-bot | MR !1401: WIP: introduce a Node api, and remove node_get https://gitlab.com/BuildStream/buildstream/merge_requests/1401 | 08:15 |
benschubert | jennis: ^ might itnerest you too | 08:15 |
jennis | I think the initial WIP MR is a good idea so that we can all at least get a taste of/agree on the new API | 08:20 |
juergbi | hi benschubert, I generally prefer the incremental approach as well. we just need to be confident that all planned parts really get done in a reasonable time frame. i.e., I wouldn't want the code base to get stuck in a half-migrated state | 08:20 |
benschubert | juergbi: I could also create a long-living branche new-node-api, and merge smaller changes with code reviews in it if we prefer, rebasing shouldn't be too much of a hassle | 08:21 |
benschubert | I just want to make sure that I do get enough code reviews and that we end up in a better state :) | 08:21 |
juergbi | benschubert: if you indeed don't expect significant rebasing overhead, that might make sense | 08:22 |
benschubert | but I'm confident with where I want the codebase to be | 08:22 |
juergbi | would be equivalent from the reviewing point of view (small MRs) without risking inconsistency in master at all | 08:22 |
benschubert | yep, and only breaking external plugins once :) | 08:22 |
juergbi | indeed | 08:22 |
jennis | I suspect that once we get this first MR right, the rest should be smooth sailing, right? | 08:22 |
benschubert | I'll create a new branch for that and move the MR to it, thanks! | 08:22 |
benschubert | jennis: there is lots of edge cases, and I might need to tweak a few things to keep the provenance entirely correct, but yes :) | 08:23 |
jennis | fair enough, thanks benschubert | 08:23 |
*** ChanServ sets mode: +o tristan | 08:25 | |
tristan | benschubert, agreed... I'd like to minimize the amount of times we need to churn consumers of the API, so an atomic change is desirable there - if you want to do incremental internal changes leading up to that, that would be fine | 08:25 |
benschubert | I'll make a long living branch to which we merge everything from that change | 08:25 |
benschubert | and once we are happy we can do a sweep change :) | 08:25 |
tristan | benschubert, I wonder... (just really pie in the sky pondering here...) if this Node API could somehow meld in with ruamel.yaml one day - having the API this way seems conducive to that | 08:28 |
tristan | i.e. perhaps we could save cycles if we were to subclass some ruamel things in the parser and have them come out as Nodes in the first place ? | 08:29 |
tristan | just a random thought really | 08:29 |
benschubert | tristan: that's almost what is done now :), we basically rewrite the Representer and return our own nodes instead | 08:30 |
tristan | Ahhh | 08:30 |
tristan | Nice | 08:30 |
benschubert | which still have a problem, that ruamel doesn't expose the class that calls the representer as a C extension, only as a python class | 08:30 |
benschubert | so we still have a roundtrip to the intepreter here | 08:30 |
benschubert | but we sould be able in the future to possibly ask / contribute some changes for that | 08:31 |
tristan | I recall when I brute forced the provenance to be stored in the parent dicts thinking "If I had time... I should really be making the parser return something with all of this context" | 08:31 |
benschubert | https://gitlab.com/BuildStream/buildstream/merge_requests/1401 is now targeting the new branch in which I'll put all the API changes, Feel free to give early review. I plan to have it out of WIP today or sometimes next week depending on how much time I get on it | 08:32 |
benschubert | the yaml changes themselves are fairly sset. I'm now moving to remove all the other calls to node_get | 08:32 |
jennis | How do we tell if an element is from a junction? | 08:36 |
jennis | I was hoping to find some __junctioned flag in element or something | 08:37 |
tristan | jennis, who wants to ask ? | 08:38 |
jennis | me! | 08:38 |
tristan | jennis, it makes a difference, I'm not sure there is an answer for a plugin author | 08:39 |
jennis | I guess we can get the element's project, and see if this doesn't match the parent project? | 08:39 |
tristan | but the core is allowed to know for sure | 08:39 |
jennis | It's for the core, specifically, not pushing junctioned elements to a parent project's remote | 08:39 |
tristan | jennis, right, via the project | 08:39 |
tristan | jennis, there are a couple attributes, the project when loaded for instance also indicates the element from whence it was junctioned | 08:40 |
*** raoul has joined #buildstream | 08:40 | |
tristan | so if project = element._get_project(), and `project.junction is None`, then it is in the toplevel project | 08:41 |
tristan | Hmmm | 08:41 |
tristan | "To a parent project's remote" | 08:41 |
jennis | Ok, I should be able to use that | 08:41 |
tristan | jennis, That sounds ambiguous to me a bit, i.e. you might not be interested in whether that is None | 08:42 |
tristan | but you should be able to get the context you need | 08:42 |
jennis | No, rather I'd be interest in whether it's not none | 08:42 |
tristan | jennis, I don't think so | 08:42 |
jennis | then, it's a junctioned element? | 08:42 |
tristan | jennis, I mean "Not pushing junctioned elements to a parent project's remote" as a statement... implies there is an element in context which is to be pushed to a remote | 08:43 |
tristan | and that there is some recursion going on | 08:43 |
tristan | jennis, Assume there are a dozen junctioned projects in the loaded graph, and you don't know for sure if the one which explicitly needs to be pushed is from the toplevel project or not... right ? | 08:44 |
tristan | jennis, But I think that *anyway*, "the remotes to where an element's artifact must be pushed" is resolved on a per project basis anyway | 08:45 |
jennis | Ahh I think I'm getting myself confused | 08:45 |
tristan | At load time | 08:45 |
jennis | I was under the impression that the top level project would have project.junction = None | 08:45 |
tristan | So perhaps if there are issues around that, it would be better to ensure those are loaded correctly at startup, rather than adding any statements in code which say "if self._get_project().junction._get_project() == self._get_project()" | 08:46 |
jennis | Yes, that does seem to be the case...? | 08:46 |
jennis | I've just set up a test case using our junction examples project, the top level element, from the parent (callHello.bst), element._get_project().junction returns None, whereas the others return hello-junction.bst | 08:47 |
tristan | jennis: *Yes* - The toplevel project will have it's junction set to None.... and *No* I don't believe that there is any rule that is specific to the toplevel project | 08:48 |
tristan | jennis, if there is a difference in behavior for a toplevel & subproject... then that same difference in behavior would also apply to a subproject & sub-subproject, right ? | 08:49 |
jennis | oh, gotcha | 08:49 |
tristan | jennis, But also... I really think this should be resolved at load time - configuration of which remotes to push to are done in project.conf and user configuration | 08:50 |
tristan | jennis, in the end, after loading - every Project() object should know where things should be being pushed to | 08:50 |
tristan | we shouldnt need runtime checks there | 08:50 |
jennis | mhmm, but we still need all the remotes to be able to push too | 08:50 |
tristan | as already, each Project() is different | 08:51 |
jennis | We just need to know whether the element should push to the remote | 08:51 |
* tristan doesnt understand the first sentence | 08:51 | |
tristan | each project.conf is loaded separately right ? | 08:52 |
tristan | jennis, Are we basing this on an incorrect assumption that the configuration of where to push artifacts belongs in the ArtifactCache(), rather than on the Project() ? | 08:52 |
jennis | ahh right, well there must be a point where we create a list of *all* remotes, from the top-level project and the junctions | 08:53 |
tristan | I think we may have still some weirdness where we have the ArtifactCache loading data from projects and then hashing them on a per project basis ? | 08:53 |
tristan | That is a bit confusing, misleading, and messy | 08:53 |
jennis | Potentially, I'll have a look | 08:53 |
tristan | But I suppose it is at least still configuration that is loaded on a per project basis (even though it's loaded in a place which naturally causes developers to be very confused) | 08:54 |
jennis | But if we're loading remotes, per project, at some point we must be saying, "give me all the remotes I can push to", and this is what we're looping over in ArtifactCache.push() | 08:54 |
jennis | ahh, interestingly, we loop over self._remotes[project] | 08:55 |
jennis | Ok this should be easy enough to find | 08:55 |
tristan | jennis, It sounds like that question: "give me all the remotes I can push to", is a question that an Element should ask the Project, and then inform the ArtifactCache of the response to that question at push() time | 08:55 |
tristan | but yeah, I'm sure there is a way to fix this issue without refactoring all of that to be more sensible | 08:56 |
jennis | Agreed | 08:56 |
*** shibu has quit IRC | 08:56 | |
*** shibu has joined #buildstream | 08:59 | |
*** jonathanmaw has joined #buildstream | 09:09 | |
*** shibu has quit IRC | 09:18 | |
*** shibu has joined #buildstream | 09:18 | |
*** lachlan has joined #buildstream | 09:47 | |
*** lachlan has quit IRC | 09:56 | |
*** lachlan has joined #buildstream | 10:03 | |
*** lachlan has quit IRC | 10:09 | |
gitlab-br-bot | BenjaminSchubert approved MR !1399 (raoul/unique-error-domains->master: Make ErrorDomain a unique enumeration) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1399 | 10:21 |
gitlab-br-bot | marge-bot123 merged MR !1399 (raoul/unique-error-domains->master: Make ErrorDomain a unique enumeration) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1399 | 10:24 |
*** lachlan has joined #buildstream | 10:24 | |
tlater[m] | tristan: Do you happen to be around? | 10:28 |
gitlab-br-bot | raoul.hidalgocharman opened (was WIP) MR !1400 (raoul/1044-blobs-on-demand->master: CLI options for on demand blob fetching) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1400 | 10:48 |
gitlab-br-bot | BenjaminSchubert approved MR !1400 (raoul/1044-blobs-on-demand->master: CLI options for on demand blob fetching) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1400 | 10:50 |
tristan | tlater[m], I am ! | 10:50 |
tlater[m] | Awesome! | 10:50 |
tlater[m] | I wanted to ask you what your opinion on the whole testing with ostree thing is | 10:51 |
tlater[m] | I've been working on the docs-freedesktop thing | 10:51 |
*** lachlan has quit IRC | 10:51 | |
tlater[m] | And my goal is to build freedesktop as part of the examples, which means that the CI needs to build freedesktop | 10:51 |
tlater[m] | But that's a whole can of worms with us moving ostree out of the standard plugins | 10:52 |
tlater[m] | And it would mean that building the examples is difficult on non-ostree platforms | 10:52 |
tlater[m] | Right now I think it would be alright to override elements in ostree for non-ostree platforms, and to use a cache managed by us to make sure that the actual examples don't take forever to build. | 10:53 |
tlater[m] | s/in ostree/in freedesktop/ | 10:54 |
tlater[m] | But well, it's combining a lot of moving parts, and I want to check if that sounds sensible... | 10:54 |
tristan | Ummm | 10:58 |
tristan | tlater[m], So you want to build freedesktop-sdk as a part of BuildStream CI ? | 10:58 |
tlater[m] | Only the base platform part | 10:58 |
tristan | I mean, we have that but it's mostly for testing master can still do it | 10:58 |
tristan | Mmmmyeah | 10:59 |
tlater[m] | Right, yes, but the point isn't building freedesktop-sdk. The point is having examples that are 100% buildstream powered. | 10:59 |
tristan | tlater[m], I don't think that BuildStream itself needs to be involved in the knowledge base of how to bootstrap an operating system - I think those are distinctly separate topics | 10:59 |
tristan | While it's one of the important things that you can do with BuildStream, BuildStream should not be the one demonstrating that set of knowledge | 11:00 |
tristan | Ok so... having examples that use freedesktop-sdk as a base makes sense, but do *we* have to build/bootstrap it ? | 11:00 |
tristan | I think those are separate topics of discussion too: (A) Lets have examples which run in runtimes that were created by BuildStream... (B) Let's have our CI actually build the runtimes where are examples run | 11:01 |
tristan | Do they need to be intertwined ? | 11:01 |
persia | building it and bootstrapping it aare two different things. While I don't have a firm opinion on buildstream building freedesktop-sdk in CI, I do think it should not be bootstrapped. | 11:02 |
persia | Also, while it might be interesting to have a CI job that builds a runtime as an example, I don't think it makes sense to recurse that: such a path leads to a preferential runtime. | 11:03 |
tlater[m] | Hm, well, the bootstrapping comes naturally from including freedesktop-sdk as a junction, and I think the docs would be more useful if they showed how buildstream would actually be used to build something; currently we have no examples for this at all. | 11:04 |
tlater[m] | Though I suppose if the buildstream docs only look at buildstream as a tool, that is going a bit too far. | 11:04 |
tlater[m] | tristan: So you think it would make more sense to swap a freedesktop-sdk tarball into the place of the current 'doctored' alpine image, and instead have a doc that lives somewhere else to show how you would *really* depend on freedesktop-sdk in a project? | 11:05 |
tristan | persia, tlater[m] ... "building OR bootstrapping freedesktop-sdk" is also *totally* different from just using it's output to base our examples on | 11:09 |
tristan | Do we need to be doing *any* of that ? | 11:09 |
tristan | persia, Right now our "preferential runtime" is an alpine linux tarball, I think it's a good idea to at least have a preferential runtime that was built with BuildStream for our examples | 11:10 |
tlater[m] | tristan: It is a shame that our examples miss out on showing how BuildStream can define everything from the ground up if we don't build our runtime, though. | 11:13 |
tristan | tlater[m], It would make more sense to refer to external documentation about that, though | 11:15 |
tristan | tlater[m], it's not uncommon for projects to list some of their users on their web pages or point to their documentation | 11:15 |
tlater[m] | Yes, I can definitely see that. It makes sense to keep the examples as minimal as possible. | 11:16 |
tlater[m] | If we use a freedesktop-sdk base tarball we can always have a pretty note section that points to a doc that shows this dependency thing in more detail. | 11:17 |
tlater[m] | Yeah, I'm convinced. I suppose freedesktop wouldn't mind this in their docs. | 11:18 |
tristan | Right, also if we want an example of using a junction, we should not use an example of a project junctioning freedesktop-sdk | 11:18 |
tristan | we should have something very small and self contained which provides the minimal amount of information possible to display how a junction works | 11:18 |
tlater[m] | No, the current junction example does the job pretty well. | 11:18 |
tristan | (as you said: it makes sense for examples to be minimal as possible) | 11:18 |
tristan | right | 11:18 |
tristan | It might use an alpine tarball, and it might make sense to swap that for an fdsdk one | 11:19 |
tristan | just for the sake of using something built with buildstream, proving we "stand on our own two feet" so to speak | 11:19 |
tlater[m] | Also so people don't have to trust our "doctoring" - we can just actually use BuildStream to tell them what that means ;p | 11:20 |
*** shibu has quit IRC | 11:26 | |
*** lachlan has joined #buildstream | 11:28 | |
*** lachlan has quit IRC | 11:47 | |
*** lachlan has joined #buildstream | 12:02 | |
*** lachlan has quit IRC | 12:05 | |
*** tristan has quit IRC | 12:35 | |
*** tristan has joined #buildstream | 12:45 | |
*** lachlan has joined #buildstream | 13:28 | |
*** lachlan has quit IRC | 14:32 | |
gitlab-br-bot | jennis approved MR !1394 (aevri/psutil_affinity->master: _platform.get_cpu_count: use psutil instead of os) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1394 | 14:41 |
*** lachlan has joined #buildstream | 14:46 | |
*** shibu has joined #buildstream | 14:50 | |
persia | I became distracted, but my concern is that the ability to "stand on our own two feet" doesn't lead to an inability to work in other contexts. Nice to be able to stand alone, of course, but also absolutely essential to not create a monolithic environment that doesn't play well with others. | 14:55 |
*** lachlan has quit IRC | 14:56 | |
*** lachlan has joined #buildstream | 15:14 | |
*** lachlan has quit IRC | 16:05 | |
*** lantw44 has quit IRC | 16:07 | |
*** lantw44 has joined #buildstream | 16:07 | |
*** bochecha has quit IRC | 16:16 | |
*** shibu has quit IRC | 16:22 | |
*** shibu has joined #buildstream | 16:23 | |
*** raoul has quit IRC | 16:41 | |
tlater[m] | persia: That's a reasonable concern, but I think with tristan's suggestion we also circumvent that issue nicely. The docs will use a runtime that, while it was built with BuildStream originally, is distributed as a tarball, and should therefore basically be the "standard" (well, unless you consider docker the "standard"). | 16:59 |
tlater[m] | We're "standing on our own two feet", but we're not limiting ourselves to that, and we don't send a message of "you can only use buildstream with buildstream" | 17:00 |
tlater[m] | My concern was that we're not showing that you can use only buildstream, but that's resolved by placing that particular doc in an external source. | 17:00 |
*** jonathanmaw has quit IRC | 17:02 | |
*** tpollard has quit IRC | 17:07 | |
*** shibu has quit IRC | 17:43 | |
*** benschubert has quit IRC | 18:58 | |
*** lachlan has joined #buildstream | 19:29 | |
*** lachlan has quit IRC | 19:42 | |
*** kapip has quit IRC | 19:53 | |
*** kapip has joined #buildstream | 20:01 | |
persia | tlater[m]: Fair. I think there is a happy compromise; I mostly just wanted to identify my opinion about "too far". | 20:20 |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!