*** mint has joined #buildstream | 05:31 | |
*** benschubert has joined #buildstream | 07:02 | |
*** bochecha has joined #buildstream | 07:50 | |
*** tristan has quit IRC | 07:55 | |
benschubert | Hey, would someone have time to review https://gitlab.com/BuildStream/buildstream/merge_requests/1472 ? It's already been reviewed multiple times by chunks, but a final look would be appreciated | 08:12 |
---|---|---|
benschubert | !1472 (new node api) | 08:12 |
*** alexandrufazakas has joined #buildstream | 08:15 | |
*** tristan has joined #buildstream | 08:23 | |
*** bochecha has quit IRC | 08:23 | |
*** tiagogomes has joined #buildstream | 08:24 | |
benschubert | tristan: would you have time for a final approval of ^ ? | 08:28 |
*** tiagogomes has quit IRC | 08:31 | |
*** tpollard has joined #buildstream | 08:34 | |
*** tiagogomes has joined #buildstream | 08:36 | |
gitlab-br-bot | juergbi approved MR !1473 (tristan/config-max-jobs->master: Add max-jobs configuration and command line option) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1473 | 09:07 |
*** jonathanmaw has joined #buildstream | 09:08 | |
*** ChanServ sets mode: +o tristan | 09:11 | |
tristan | benschubert, lets take a look... I guess it will be huge ! | 09:11 |
*** phildawson has joined #buildstream | 09:11 | |
benschubert | tristan: that's a pretty safe bet ;) | 09:11 |
*** cs-shadow has joined #buildstream | 09:11 | |
tristan | I'll give it a skim at this point... a fully thorough thing isn't really possible but we've looked at all the bits and pieces so should be pretty safe :) | 09:12 |
benschubert | tristan: yep :) Kinnison and I already did that too | 09:12 |
* tristan hopes that gitlab can cope haha | 09:12 | |
tristan | speaking of which, I think CI is borked | 09:13 |
tristan | someone needs to slap the runners I think | 09:13 |
*** traveltissues has joined #buildstream | 09:13 | |
*** rdale has joined #buildstream | 09:14 | |
Kinnison | valentind: Any chance you can look at the runners? | 09:18 |
tristan | benschubert, This one is a bit concerning: https://gitlab.com/BuildStream/buildstream/merge_requests/1472/diffs#note_191768836 | 09:40 |
benschubert | tristan: I agree we can improve those. However, I don't think 'scalar' is unreasonable (cf; https://en.wikipedia.org/wiki/YAML second paragraph). I think mapping to yaml definitions would be best. What about only using 'scalar', 'dict' and 'list' ? | 09:45 |
tristan | benschubert, That would satisfy me :) | 09:47 |
benschubert | let me fix this then! | 09:47 |
tristan | benschubert, I still think that if get_node() were able to specify multiple specific types of "scalar" instead of just one, it would be a better error message | 09:47 |
tristan | I think we agree on that | 09:48 |
benschubert | tristan: we only have one type of scalar in BuildStream though no? which would be 'string'? | 09:48 |
tristan | But still using mapping/list/scalar is good :) | 09:48 |
tristan | In terms of python types yes | 09:48 |
tristan | benschubert, But whenever we load something, we either want a string or an int | 09:49 |
benschubert | true | 09:49 |
tristan | I'm not sure if we ever want a double | 09:49 |
benschubert | it's not possible currently, but easy to add when we have the need for it | 09:49 |
tristan | yeah, that's not a worry - I've never seen the need for a double either | 09:50 |
tristan | (and a "version string" is definitely not a double :)) | 09:50 |
tristan | oh sorry, "dict/list/scalar" right | 09:50 |
tristan | I said mapping | 09:50 |
tristan | but yeah lets use dict, or even dictionary is better | 09:51 |
tristan | benschubert, For plugin authors, it would anyway be good to make the link in toplevel documentation jargon for the type | 09:52 |
benschubert | tristan: that should already be the case | 09:52 |
tristan | benschubert, i.e. for MappingNode at the toplevel, make the mental link that "This is a YAML dictionary" and for ScalarNode "This is a YAML list" | 09:52 |
tristan | Right | 09:52 |
tristan | Just saying we should make sure those terminologies match and we use the same word :) | 09:53 |
benschubert | e.g: first line of the docstring in SequenceNode: This class represents a Sequence (list) in a YAML document. | 09:53 |
tristan | Yep | 09:53 |
benschubert | tristan: https://gitlab.com/BuildStream/buildstream/merge_requests/1472/diffs?commit_id=a63a361f6e7fe7853b87967bbaefec4acc0e38f6 would that be good for you? | 09:53 |
valentind | I am looking at the builders. It seems docker-machine does not machine to install docker on the spawned machines. | 09:54 |
*** mint has quit IRC | 09:54 | |
tristan | benschubert, works for me yeah :) | 09:55 |
valentind | Well, it seems the builders work now. But I am not sure why. I only removed all docker machines and restarted gitlab-ci. | 10:02 |
benschubert | valentind: thanks! | 10:03 |
benschubert | tristan: let me know when you are done, so that we can hand it to marge :) | 10:03 |
*** jjardon has quit IRC | 10:04 | |
valentind | Ah no, it does not work. I finally get the errors again. | 10:04 |
*** cs-shadow has quit IRC | 10:04 | |
*** cs-shadow has joined #buildstream | 10:04 | |
* tristan 's pipelines are still in "retry in 3s" state :) | 10:06 | |
*** benschubert has quit IRC | 10:06 | |
*** jjardon has joined #buildstream | 10:06 | |
*** ChanServ sets mode: +o jjardon | 10:06 | |
*** benschubert has joined #buildstream | 10:07 | |
*** jjardon has quit IRC | 10:07 | |
*** jjardon has joined #buildstream | 10:08 | |
*** ChanServ sets mode: +o jjardon | 10:08 | |
gitlab-br-bot | tristanvb approved MR !1472 (bschubert/new-node-api->master: Rewrite of the Node API) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1472 | 10:10 |
benschubert | o/ thanks! | 10:12 |
tristan | benschubert, as a minor enhancement, I am still fond of the idea of having ScalarNode do some convenience enum accessors (where the allowed values are a list of strings, possibly defined as the attributes of a python enum) | 10:13 |
tristan | lemme take a look at how context.py loads something like... the on-error value for instance; we could make error reporting for all of those cases consistent and save some LOC | 10:14 |
benschubert | tristan: not sure I understand where you would use that. Can of a 'get_node' on scalars with multiple return types? | 10:14 |
tristan | oh you have _node_get_option_str() ? | 10:15 |
tristan | benschubert, I mean more like _node_get_option_str(), not sure why that is private or why it's not a convenience accessor of ScalarNode | 10:16 |
benschubert | tristan: oh! That's in _context | 10:16 |
benschubert | that probably should not be there | 10:17 |
benschubert | and it uses node_get? | 10:17 |
benschubert | so it's unused? | 10:17 |
tristan | I was thinking something like the python could define a SchedulerErrorAction enum... with the members SchedulerErrorAction.continue = 1, SchedulerErrorAction.quit = 2, SchedulerErrorAction.terminate = 3 | 10:17 |
benschubert | ah no sorry | 10:17 |
benschubert | wrong branch | 10:17 |
tristan | And load it like: action = ScalarNode.get_enum(SchedulerErrorAction) | 10:17 |
tristan | I mean, a list of strings is fine, the enum idea is just sugar coating (but could make coding a *bit* more safe, maybe ?) | 10:18 |
Kinnison | Wow, tristan has found many things where I didn't -- I must have been changeblind :( | 10:18 |
tristan | Right now it uses this internal function _node_get_option_str() | 10:18 |
* Kinnison apologises | 10:18 | |
benschubert | tristan: 'get_from_enum'? :) | 10:18 |
tristan | benschubert, That sounds sexy yeah :) | 10:18 |
tristan | Kinnison, it's because I know what I'm looking for... it's not like I scanned that huge MR and came up with these :) | 10:19 |
benschubert | ok! I'll add this, but in a follow up MR to get this one ok? | 10:19 |
Kinnison | tristan: oh heh | 10:19 |
tristan | benschubert, yeah sure | 10:19 |
benschubert | perfect | 10:19 |
benschubert | tristan: also, we should make this one public right? | 10:24 |
tristan | benschubert, Yeah ! | 10:30 |
valentind | So get.docker.com script probably removed fedora 29. | 10:31 |
tristan | I don't have a plugin off hand in mind which will use it but, when it comes to loading yaml configuration, enums are certainly a handy thing to have ! :) | 10:31 |
benschubert | yep, I'mm for it :D | 10:31 |
tristan | right now in _context.py alone, we have both the scheduler error action, and the cache buildtrees option which are enums | 10:32 |
tristan | in project, we have ref-storage which is an enum | 10:33 |
valentind | I have to build a custom image for digital ocean. It will take me maybe some hours to get it right. | 10:36 |
benschubert | valentind: why do we have fedora 29? Shouldn't we be using 30? | 10:38 |
valentind | It did not work. | 10:38 |
valentind | But now 29 does not work either. | 10:38 |
valentind | We would need to make a custom image that has docker already installed. | 10:38 |
valentind | Fedora 28 only is supported by docker-machine to provision docker on it. | 10:39 |
valentind | And I do not remember why we do not use fedora 28 anymore. But there was also an issue there. | 10:39 |
benschubert | ok | 10:39 |
valentind | I think I might switch to Debian buster. Maybe that works. | 10:39 |
valentind | It is just to run docker. Nothing more. So it should not matter. | 10:40 |
valentind | Seems debian and ubuntu are more supported by docker. | 10:41 |
valentind | Well, that install script is half-assed supporting Buster. | 10:44 |
gitlab-br-bot | traveltissues approved MR !1473 (tristan/config-max-jobs->master: Add max-jobs configuration and command line option) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1473 | 11:05 |
gitlab-br-bot | traveltissues approved MR !1465 (juerg/context->master: Make Context class a Python context manager) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1465 | 11:17 |
valentind | Ah, seems like working! | 11:57 |
valentind | In the end I bumped the builders to use fedora 30 and provided my own docker install script for docker-machine: https://docker-install-script.ams3.digitaloceanspaces.com/install-docker.sh | 12:00 |
valentind | Which just installs docker from fedora and add symbolic links to make it work with systemd files created by docker-machine. | 12:01 |
valentind | People can restart their builds now. | 12:02 |
valentind | Please ping me again if it looks like it is not working. | 12:02 |
traveltissues | thanks valentind | 12:05 |
*** bochecha has joined #buildstream | 12:17 | |
*** phoenix has joined #buildstream | 12:19 | |
traveltissues | juergbi, please will you give me dev access on buildbox-fuse | 12:26 |
*** phoenix has quit IRC | 12:29 | |
*** phoenix has joined #buildstream | 12:34 | |
gitlab-br-bot | beckyella16 opened (was WIP) MR !1476 (becky/artifact_checkout_directory->master: Artifact checks out to default dir if no --tar or --directory) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1476 | 12:37 |
benschubert | thanks a lot valentind | 12:39 |
*** alexandrufazakas has quit IRC | 12:41 | |
*** alexandrufazakas has joined #buildstream | 12:41 | |
*** alexandrufazakas_ has joined #buildstream | 12:52 | |
tristan | traveltissues, Looks like you have two pipelines which are not "latest" for your branch at https://gitlab.com/BuildStream/buildstream/pipelines, mind if I cancel the older jobs ? | 12:56 |
tristan | we have a lot of pipelines in "pending" :) | 12:57 |
traveltissues | np, cancelled them | 12:57 |
tristan | thanks ! | 13:04 |
tristan | valentind, Thanks a lot for spending all that time on the runners ! | 13:05 |
tristan | Damn that was a painful CI-less morning :) | 13:06 |
*** lachlan has joined #buildstream | 13:10 | |
gitlab-br-bot | traveltissues approved MR !1475 (chandan/fix-fuse-check->master: sandbox/_sandboxbwrap.py: Fix fuse import issue) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1475 | 13:13 |
traveltissues | is there any more feedback on https://gitlab.com/BuildStream/buildstream/merge_requests/1468 ? | 13:19 |
alexandrufazakas | Hey, can someone help me understand what this issue might need https://gitlab.com/BuildStream/buildstream/issues/1078 ? I understand we prefer `bst build --deps all` instead of `bst build --all`, but I'm not sure what where `run` comes into play | 13:29 |
alexandrufazakas | And do we have means of figuring out which dependencies are build-only, runtime dependencies etc? | 13:30 |
*** lachlan has quit IRC | 13:40 | |
*** lachlan has joined #buildstream | 13:42 | |
*** phoenix has quit IRC | 13:43 | |
juergbi | traveltissues: added to buildbox group | 13:46 |
traveltissues | thanks | 13:47 |
gitlab-br-bot | traveltissues opened (was WIP) MR !1470 (traveltissues/cache-key-changes-2->master: workspace clobbers sources) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1470 | 13:58 |
gitlab-br-bot | marge-bot123 closed issue #1081 (BuildStream throws an exception on Linux when fuse is unavailable) on buildstream https://gitlab.com/BuildStream/buildstream/issues/1081 | 14:12 |
gitlab-br-bot | marge-bot123 merged MR !1475 (chandan/fix-fuse-check->master: sandbox/_sandboxbwrap.py: Fix fuse import issue) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1475 | 14:12 |
gitlab-br-bot | tristanvb opened MR !1477 (tristan/backport-previous-sources->bst-1: Backport ability to see previous sources in fetch() and track()) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1477 | 14:25 |
*** lachlan has quit IRC | 14:30 | |
*** lachlan has joined #buildstream | 14:35 | |
*** lachlan has quit IRC | 14:52 | |
gitlab-br-bot | marge-bot123 merged MR !1472 (bschubert/new-node-api->master: Rewrite of the Node API) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1472 | 14:58 |
benschubert | o/ | 14:58 |
*** tristan has quit IRC | 15:00 | |
cs-shadow | awesome | 15:02 |
benschubert | next step: fix plugins... | 15:02 |
cs-shadow | i can finally rebase my mypy PR | 15:02 |
*** lachlan has joined #buildstream | 15:03 | |
alexandrufazakas | Anyone familiar with https://gitlab.com/BuildStream/buildstream/issues/178 that can help me understand it a bit? :D | 15:03 |
benschubert | valentind: I tried running the overnight tests for my new changes, and apparently we get the same problems on the overnight runners https://gitlab.com/BuildStream/buildstream/-/jobs/251848334 would you have time to investigate? | 15:06 |
valentind | I did the same change there, so it should work. | 15:06 |
valentind | hostname too long. | 15:07 |
*** tristan has joined #buildstream | 15:07 | |
valentind | Another issue. | 15:07 |
valentind | I will fix that. | 15:07 |
benschubert | oh, thanks! | 15:07 |
valentind | I have to restart gitlab-runner. | 15:08 |
benschubert | tristan, juergbi I'm looking at removing other hot spots in our code. I sometimes need to add helpers in cython for various methods we have. What do you prefer: a etx/ module that contains them, or a "_modulename.pyx" file next to "modulename.py" ? | 15:13 |
valentind | benschubert, that should be working now. | 15:16 |
benschubert | valentind: thanks a lot! | 15:16 |
juergbi | benschubert: I think I'd prefer _modulename.pyx next to modulename.py as it seems a bit odd to group those files together in a subdirectory as those files don't form a separate component, i.e., they don't really belong together | 15:18 |
benschubert | thanks! I do also tend to prefer this one, I'll go for this unless someone disagrees strongly | 15:18 |
*** ChanServ sets mode: +o tristan | 15:25 | |
tristan | benschubert, juergbi agreed | 15:25 |
benschubert | perfect, thanks! | 15:25 |
gitlab-br-bot | traveltissues approved MR !1473 (tristan/config-max-jobs->master: Add max-jobs configuration and command line option) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1473 | 15:27 |
*** bochecha has quit IRC | 15:46 | |
*** phildawson_ has joined #buildstream | 15:51 | |
*** phildawson has quit IRC | 15:52 | |
*** lachlan has quit IRC | 15:53 | |
*** lachlan has joined #buildstream | 16:10 | |
*** alexandrufazakas has quit IRC | 16:31 | |
*** jonathanmaw has quit IRC | 16:48 | |
*** lachlan has quit IRC | 16:55 | |
cs-shadow | hi, can one of the admins of bst-plugins-experimental please share the credentials for twine/pypi with me? I am trying to set up something similar for bst-plugins-container, and I thought perhaps we can use the same creds for all repos under the buildstream org | 16:59 |
*** lachlan has joined #buildstream | 17:00 | |
*** lachlan has quit IRC | 17:07 | |
juergbi | cs-shadow: I should probably have access to that, but I don't see any protected CI variables in that repo. do you know where the credentials are stored? | 17:12 |
cs-shadow | juergbi: thanks for looking. That's weird, I think it ought to be there if the release step (https://gitlab.com/BuildStream/bst-plugins-experimental/blob/master/.gitlab-ci.yml#L206) actually works | 17:22 |
cs-shadow | juergbi: wild guess - it's possible when the repo was moved from bst-external, we forgot to move the credentials | 17:23 |
cs-shadow | can you please check there as well? | 17:23 |
juergbi | cs-shadow: yes, they are there | 17:25 |
juergbi | should probably copy them to bst-plugins-experimental | 17:26 |
juergbi | or is pypi-related CI not updated yet for the new repo anyway? | 17:26 |
cs-shadow | They should be moved at some point, however perhaps it's best to defer it until we plan on releasing the experimental plugins | 17:27 |
*** lachlan has joined #buildstream | 17:34 | |
*** phoenix has joined #buildstream | 17:42 | |
*** phildawson_ has quit IRC | 17:48 | |
*** lachlan has quit IRC | 17:57 | |
*** lachlan has joined #buildstream | 18:03 | |
*** lachlan has quit IRC | 18:12 | |
*** traveltissues has quit IRC | 18:14 | |
*** lachlan has joined #buildstream | 18:15 | |
*** lachlan has quit IRC | 18:27 | |
*** lachlan has joined #buildstream | 18:32 | |
*** lachlan has quit IRC | 18:45 | |
*** lachlan has joined #buildstream | 18:59 | |
benschubert | jjardon: hey, if I want to fix a plugin on freedesktop-sdk for bst2, which branch should I target? | 19:19 |
benschubert | tristan: ^ you might also know actually :) | 19:20 |
jjardon | benschubert: sec | 19:21 |
jjardon | Should be the one pointing to https://gitlab.com/BuildStream/buildstream/blob/master/.gitlab-ci.yml#L226, let me find the branch name | 19:25 |
benschubert | sure, thanks :) | 19:25 |
jjardon | benschubert: https://gitlab.com/freedesktop-sdk/freedesktop-sdk/tree/freedesktop-sdk-18.08.31.1-bst2 | 19:26 |
*** lachlan has quit IRC | 19:26 | |
benschubert | Thanks a lot! I'll try to put a MR tonight | 19:26 |
jjardon | Nice! Thanks | 19:28 |
jjardon | benschubert: is this to fix https://gitlab.com/BuildStream/buildstream/issues/1049 ? | 19:29 |
benschubert | jjardon: no, for the breaking yaml api change I introduced this morning | 19:31 |
*** lachlan has joined #buildstream | 19:36 | |
*** lachlan has quit IRC | 19:40 | |
*** lachlan has joined #buildstream | 19:48 | |
*** lachlan has quit IRC | 19:56 | |
benschubert | juergbi: did 'strip-args' and such got removed from the defaults in the end? I'm getting errors about this missing while trying to fix freedesktop-dsk | 21:37 |
*** dftxbs3e has quit IRC | 22:44 | |
*** dftxbs3e has joined #buildstream | 22:45 | |
*** phoenix has quit IRC | 23:37 |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!