| *** Prince781 has quit IRC | 01:04 | |
| *** Prince781 has joined #buildstream | 02:07 | |
| *** Prince781 has quit IRC | 02:11 | |
| *** Prince781_ has joined #buildstream | 02:12 | |
| *** cs_shadow has quit IRC | 05:13 | |
| *** ernestask has joined #buildstream | 05:53 | |
| *** tristan has joined #buildstream | 06:27 | |
| *** Prince781_ has quit IRC | 06:37 | |
| paulsherwood | am i alone in wishing that the bst mailing list would add [buildstream] to email titles? | 07:11 |
|---|---|---|
| tristan | paulsherwood, I'm ambivalent and not against subject line munging | 07:28 |
| tristan | lemme see if the UI lets me do that easily | 07:28 |
| tristan | "Prefix for subject line of list postings." | 07:29 |
| tristan | I'm not sure if I should put "buildstream" or "[buildstream]", and it has a bunch of obscure features | 07:32 |
| * tristan tries "[buildstream]" and lets see what happens | 07:32 | |
| tristan | it works :) | 07:37 |
| paulsherwood | that works, thanks | 07:37 |
| paulsherwood | tristan: you've been notably silent on my 'need for backwards compatibility' comments... | 07:37 |
| *** bochecha has joined #buildstream | 07:39 | |
| tristan | paulsherwood, is that on the mailing list ? | 07:47 |
| tristan | I was swamped yesterday trying to open up master for 1.3.x development | 07:48 |
| tristan | Is there an issue with backwards compatibility ? I noticed a comment fly by yesterday which I think valentind clarified at the time | 07:48 |
| *** coldtom has joined #buildstream | 07:53 | |
| *** finn has quit IRC | 07:57 | |
| paulsherwood | on gitlab. i can email the list if that's better | 07:58 |
| paulsherwood | https://gitlab.com/BuildStream/buildstream/issues/474 | 07:58 |
| tristan | Ah that one, about the cache key | 07:59 |
| paulsherwood | to go a bit further, i think for the long term bst needs to be clear about its input versioning (schema of bst files) and output versioning (cache-key algorithm and artifact content rules) | 08:00 |
| tristan | right, I was actually thinking on that yesterday, I'm surprised we dont already have an issue for that | 08:00 |
| valentind | I might have misunderstood yesterday because of the context. | 08:00 |
| tristan | valentind, different topic | 08:00 |
| paulsherwood | and bst as a program needs to support historic 'versions' of both input and output, for as long as is possible | 08:00 |
| tristan | paulsherwood, we did raise this in a meeting last year as a long term goal, it basically requires work | 08:01 |
| paulsherwood | this was one of the things we did not understand properly early in baserock... maybe i *still* don't understand it properly | 08:01 |
| tristan | For input, we focus very strongly on backwards compatibility of the format, yes | 08:02 |
| paulsherwood | tristan: i know it requires work, but it's not harder than what bst already does. ybd makes a reasonable attempt at it... but the code is ugly of course. lots of 'if version > x' | 08:02 |
| tristan | For cache keys, I know we've been over this a few times, I think that it's wiser to wait for the big changes regarding caching of build trees etc but may be wrong on that | 08:03 |
| paulsherwood | if we don't meet these 'contracts' we always risk making it impossible for users to upgrade, so they get left behind | 08:03 |
| jmac | I'd like to understand this problem | 08:03 |
| paulsherwood | jmac: me too :-) | 08:03 |
| jmac | What are we talking about caching? Artifacts? | 08:03 |
| tristan | jmac, at the surface of it... we would want future versions of BuildStream to produce the same artifact cache keys in 10 years time, for instance | 08:04 |
| paulsherwood | this is not just artifacs... it's the question of backwards/forwards compatibility in general | 08:04 |
| tristan | i.e. a version of BuildStream released 10 years later | 08:04 |
| * paulsherwood would like to link to his original comment on this on the baserock wiki, but it seems to be down :/ | 08:05 | |
| tristan | jmac, currently we don't consider the cache key calculations to be stable, and the reasons behind desiring that cache key to be stable across versions are intricate I think | 08:05 |
| jmac | So what's the key calculated on? Is it more than the directory treee produced after assembly? | 08:05 |
| tristan | jmac, one of the bigger arguments which persia raised, was that when you have to build a *lot* of software, upgrading BuildStream means you can no longer re-use your old artifacts... and that any newly created artifacts under a new key... *need to undergo validation again* | 08:06 |
| jmac | Yes, #474 mentions that | 08:07 |
| jmac | From the code it looks like the cache key is calculated on a number of fields from the element, project and context | 08:09 |
| tristan | jmac, currently; the cache key algorithm is preserved as much as possible within BuildStream core and the core elements, adding new features to the project format sometimes changes the key, but this is always done in a backwards compatible way | 08:12 |
| tristan | jmac, i.e. "The key is only effected if the new feature is used" | 08:12 |
| tristan | However, the artifact container itself has supporting data for features that are orthogonal to the format | 08:12 |
| tristan | So, the current procedure for "Caching build trees in the artifact" requires a core artifact version bump | 08:13 |
| tristan | This is so that BuildStream can know that the artifact it's working with is compatible with the featureset | 08:13 |
| tristan | Further than this, there is another gap related... which is that YBD allowed the user to provide the "DEFAULTS" file, for which we need to provide something similar | 08:13 |
| tristan | I.e. if we ever change a default value (in data/projectconfig.yaml or in the element yaml files), this changes how things get built, inadvertently also effecting cache key | 08:14 |
| tristan | The user would need to also be able to specify the defaults on which to build; or support this in some way | 08:15 |
| tristan | The harder problem to solve is how to add data and restructure the artifact internals in order to implement new features without bumping the cache key | 08:16 |
| paulsherwood | to be clear, i'm not saying don't bump the key | 08:20 |
| paulsherwood | just that when we bump, we declare a new version, and distinguish from previous versions, and be willing to operate in previous-version-mode | 08:21 |
| paulsherwood | version bump has to happen for content changes too (e.g. if we change the structure/metadata/content of artifacts) we need to bump the key/version, even if we hash all inputs the same | 08:22 |
| jmac | At the moment, the artifact version is hashed so we can't do that. We'd need to make the artifact key reversible so we can get the version out again. | 08:22 |
| paulsherwood | this led to needing to include the version itself in the hashing | 08:22 |
| paulsherwood | jmac: not sure why? | 08:23 |
| jmac | You're not sure why it's hashed? | 08:23 |
| paulsherwood | why do we need reversibility? | 08:23 |
| jmac | If you have two different cache keys for artifacts, how would you know whether one was from a previous artifact version if both keys are hashes? | 08:24 |
| Kinnison | jmac: Make the key of the form: VERS-HASH-CHECKSUM or somesuch? | 08:25 |
| jmac | Kinnison: That's what I mean by reversible | 08:25 |
| Kinnison | aah :-) | 08:25 |
| * Kinnison thinks paulsherwood meant you wanted a non-trapdoor hashing function | 08:25 | |
| Kinnison | erm, insert some more 'thinking' in there | 08:25 |
| * Kinnison thinks paulsherwood thinks meant you wanted a non-trapdoor hashing function | 08:25 | |
| Kinnison | there we go | 08:25 |
| jmac | That sounds like a contradiction | 08:26 |
| paulsherwood | jmac: not sure why we need that. ybd has elementname.hash where inputs to hash include version | 08:26 |
| jmac | Hence the earlier question about how you'd know whether one was from a previous version | 08:27 |
| paulsherwood | i'm not saying bst needs to work with multiple versions at the same time | 08:27 |
| jmac | Or if it had other changes in the inputs to the hash | 08:27 |
| * paulsherwood suddenly wishes we had a whiteboard | 08:27 | |
| jmac | Right, I've misunderstood something because I thought #474 meant exactly that | 08:28 |
| paulsherwood | in the baserock approach, all of the metadata for inputs lands inside the artifact, so for a given artifact we can find out what we need by unpacking it, if tyhat's the usecase | 08:29 |
| * tristan stepped out for a moment and reads backlog | 08:29 | |
| tristan | right | 08:29 |
| paulsherwood | but we don't need to understand version in the cache-key itself | 08:29 |
| paulsherwood | if ybd generates a given key for a given version, it will find that key. for a different version, it will calculate a different key | 08:30 |
| tristan | So basically it means that whenever any feature which requires support from the artifact structure is introduced, the codebase needs to support the version with and without the feature | 08:30 |
| paulsherwood | so if i want to operate on old artifacts (which have old keys) i need to tell ybd to use the old version | 08:30 |
| paulsherwood | tristan: correct | 08:31 |
| paulsherwood | hopefully, the rate of change/versioning reduces over time, as we iron out all of the corner cases | 08:31 |
| paulsherwood | ybd is at v14 now | 08:31 |
| tristan | essentially doubling the supported modes of operation with each change, which is expensive (i.e. I'm currently trying to view the "build tree caching" feature set as something which we can be sure is always in an artifact, but only optionally downloaded and used, this is already a big concern to me while we still dont support versioned artifacts) | 08:31 |
| paulsherwood | tristan: hence the need for system-level ci that checks old versioning adequately | 08:32 |
| tristan | As long as you don't grow features too much, it's not too expensive; which is also a decent reason for at least delaying this feature | 08:32 |
| jmac | So you'd try to reuse old artifacts by iterating through previous BST_ARTIFACT_VERSION values and generating the potential hashes for all of them? Or would you remove BST_ARTIFACT_VERSION from the key? | 08:32 |
| tristan | jmac, no, you would have the project declare the artifact version it wants to use | 08:33 |
| tristan | pretty much | 08:33 |
| jmac | Oh, OK, that would work | 08:33 |
| tristan | and only enable features based on that, and calculate the cache key based on that version | 08:33 |
| paulsherwood | yup. https://gitlab.com/baserock/ybd/blob/master/ybd/cache.py#L94 shows ybd's ugly implementation | 08:34 |
| tristan | I dont mean to call your baby ugly paulsherwood ... sorry for that :) | 08:34 |
| paulsherwood | and https://gitlab.com/baserock/ybd/blob/master/ybd/config/ybd.conf#L47 shows the actual descriptions of the various bumps | 08:35 |
| paulsherwood | tristan: :-) | 08:35 |
| paulsherwood | and https://gitlab.com/baserock/ybd/blob/master/.gitlab-ci.yml#L30 shows that ybd has only two tests to verify that it still calculates v1 and v6 correctly | 08:36 |
| tristan | with the amount of churn going on this cycle I think that it's much more expensive compared to YBD's featureset, I feel like if we can get through the build tree caching and distributed building ordeals first, and reach something more mature; it would be a more opportune time for the project to implement this versioning feature | 08:37 |
| tristan | of course it's possible to do otherwise, but adds friction to development | 08:37 |
| *** bethw has joined #buildstream | 08:37 | |
| paulsherwood | tristan: i'm ok with that, but i confess i'm a bit disappointed that this wasn't understood earlier | 08:37 |
| paulsherwood | the main thing i'm hoping that we can do is regognise that this is one of the 'contracts' required for super-long term support (afaict) | 08:38 |
| paulsherwood | and hence, while we keepr breaking it, we're not ready for that usecase | 08:39 |
| tristan | Right, we probably need a place, possibly in roadmapping, where we can document and communicate long term goals as well | 08:39 |
| tristan | such that both developers and users are aware of where we are going | 08:40 |
| *** tristan_ has joined #buildstream | 08:43 | |
| *** tristan has quit IRC | 08:43 | |
| paulsherwood | in another context, we recently published this: https://gitlab.com/trustable/distros/overview/blob/master/intents.md#operating-system-definition-and-approach-to-release-and-maintenance | 08:45 |
| *** jennis has quit IRC | 08:45 | |
| paulsherwood | not so much roadmap, but i think there's a need to state in one place what we commitments the project aims to meet for users | 08:46 |
| *** jennis has joined #buildstream | 09:02 | |
| juergbi | tristan_: any opinion on Python 3.5 type hints? https://gitlab.com/BuildStream/buildstream/merge_requests/445/diffs#note_88729487 | 09:06 |
| juergbi | I generally like them but am not sure whether we need to establish a policy first or should just accept them in contributions | 09:07 |
| juergbi | (for consistency) | 09:07 |
| tristan_ | juergbi, I think that if we're going to accept something which changes the current style, we should definitely establish policy | 09:10 |
| tristan_ | juergbi, I think I probably do like the hints too | 09:10 |
| Nexus | erk, buildstream subject prefix, can we not? :/ | 09:11 |
| tiagogomes | ooc are they just hints or also establish some kind of type check? | 09:11 |
| tristan_ | juergbi, that said, establishing policy should probably come with opening an issue as a "goal" to be closed once the codebase has been made uniform again | 09:11 |
| jmac | tiagogomes: They are just documentation at the moment, but they follow a standard which can be used to do type checking in the future. | 09:12 |
| tristan_ | I'm not sure what exactly they are, but I do feel it's more important to have a uniform style throughout the codebase, than to use the feature | 09:12 |
| juergbi | that sounds sensible | 09:12 |
| juergbi | I would like to also use them at some point / check them in CI | 09:12 |
| jmac | The likely result is that I'll remove them from that !445, since that MR is blocking other work | 09:13 |
| juergbi | jmac: as you apparently are already familiar with them, would you mind opening a gitlab issue about this as per above? | 09:14 |
| juergbi | depending on when !445 lands, this would mean that we don't have to remove them from there | 09:14 |
| jmac | Sure. | 09:14 |
| juergbi | thanks | 09:14 |
| jmac | Phil: What's the status of #436? It has the 'review' tag on it but appears to need some answers to questions | 09:17 |
| * tristan_ has dinner appointment and will be back in a few hours... | 09:18 | |
| *** jonathanmaw has joined #buildstream | 09:21 | |
| *** qinusty has joined #buildstream | 09:25 | |
| gitlab-br-bot | buildstream: merge request (franred/catch-exception-when-umounting-devices->master: _sandboxbwrap.py: Catch ENOTCONN when umounting) #539 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/539 | 09:27 |
| gitlab-br-bot | buildstream: merge request (bst_workspace_open_force_does_nothing->master: _stream.py: Added functionality for workspace open -f) #549 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/549 | 09:29 |
| juergbi | tristan_: you mentioned man-page generation fails for bst-artifact-server. do you have any further details? I don't see an issue here, man/bst-artifact-server.1 appears to get generated successfully | 09:29 |
| noisecell | https://gitlab.com/franred/buildstream/pipelines/26016896 -- test-fedora and test-unix failed in the following pipeline https://gitlab.com/franred/buildstream/pipelines/26016896 for unrelated reasons of the patches, have we experienced this before? | 09:29 |
| gitlab-br-bot | buildstream: merge request (franred/catch-exception-when-umounting-devices->master: WIP: _sandboxbwrap.py: Catch ENOTCONN when umounting) #539 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/539 | 09:39 |
| gitlab-br-bot | buildstream: merge request (188-trigger-external-commands-on-certain-events->master: WIP: Resolve "Trigger external commands on certain events") #226 changed state ("closed"): https://gitlab.com/BuildStream/buildstream/merge_requests/226 | 09:39 |
| gitlab-br-bot | buildstream: merge request (image-authoring->master: WIP: Image authoring documentation) #262 changed state ("closed"): https://gitlab.com/BuildStream/buildstream/merge_requests/262 | 09:41 |
| gitlab-br-bot | buildstream: issue #482 ("Add type hints to code") changed state ("opened") https://gitlab.com/BuildStream/buildstream/issues/482 | 09:45 |
| tiagogomes | Shouldn't 482 have the newcomer tag? | 09:46 |
| tiagogomes | s/tag/label/ | 09:46 |
| jmac | I don't think so | 09:48 |
| jmac | Although given the question I think the title should be changed | 09:48 |
| jmac | It's now "Decide whether to add type hints to coding standard" | 09:49 |
| tiagogomes | aah | 09:51 |
| juergbi | converting existing code base could indeed be a newcomer task, though | 09:51 |
| juergbi | once decided | 09:51 |
| qinusty | Does anyone know what version of openssl is used/required by bst-artifact-server? | 09:52 |
| tiagogomes | easy but quite tedious | 09:52 |
| tiagogomes | I have to update my PR for tarball due conflicts on NEWS. To which version should I add an entry there "buildstream 1.2.0" ? | 09:53 |
| jmac | Hmm, interesting point about Sphinx | 09:53 |
| juergbi | tiagogomes: if the MR targets master, it will likely be 1.3.1 | 09:55 |
| tiagogomes | Not 1.3.0 then? As there is a buildstream 1.1.0 | 09:57 |
| juergbi | qinusty: do you mean for the command line tool 'openssl'? no idea. for the library the dependency is exclusively indirect via grpc, afaict | 09:57 |
| juergbi | tiagogomes: the plan is that tristan_ already tags master as 1.3.0 pretty much now to get sensible generated version number, but without actually doing a full release yet (as it would be almost identical to 1.1.4) | 09:58 |
| juergbi | tristan_: btw: master has diverged from the 1.2 branch, so it's safe to tag 1.3.0 now, if we want to | 09:59 |
| qinusty | Ah, I'm currently setting up a bst-artifact-server locally to look into some of the push/pull interactions. I'm getting "Handshake failed with fatal error SSL_ERROR_SSL: error:100000f7:SSL routines:OPENSSL_internal:WRONG_VERSION_NUMBER." coming out on the server every time I try and connect the client. (Both ran locally on my machine) | 09:59 |
| juergbi | qinusty: hm, not sure why that's happening. | 10:00 |
| juergbi | https://github.com/grpc/grpc/issues/9538 grpc forces TLS 1.2 and an error is raised if the other party does not support that | 10:00 |
| Kinnison | qinusty: Odd, that sounds like you've got something compiled against one version using a library of another version | 10:01 |
| juergbi | however, if they both ran on the same machine with the same openssl version, that's odd | 10:01 |
| juergbi | unless the same error message is raised if your openssl version doesn't support TLS 1.2 at all. what openssl version do you have installed? | 10:01 |
| qinusty | Both are ran on my machine and run OpenSSL 1.1.0f (Which seems to be the latest debian has available without extra repositories) | 10:02 |
| gitlab-br-bot | buildstream: merge request (tiagogomes/tarball_checkout->master: Add support for creating artifact tarballs) #546 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/546 | 10:03 |
| juergbi | that's definitely recent enough, assuming grpc supports the OpenSSL 1.1 API | 10:03 |
| juergbi | I'm still on 1.0.2 due to the API break | 10:04 |
| gitlab-br-bot | buildstream: merge request (phil/437-workspaces-tutorial->master: Phil/437 workspaces tutorial) #519 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/519 | 10:20 |
| gitlab-br-bot | buildstream: merge request (phil/437-workspaces-tutorial->master: Phil/437 workspaces tutorial) #519 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/519 | 10:31 |
| gitlab-br-bot | buildstream: merge request (Qinusty/scheduler_docstring_fix->master: WIP: scheduler.py: Correct some anomalies within the docstrings) #551 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/551 | 10:36 |
| qinusty | Can someone review that when they get a minute? Only docstring changes to scheduler.py for some issues I spotted. | 10:37 |
| jmac | I will | 10:38 |
| qinusty | Cheers | 10:39 |
| gitlab-br-bot | buildstream: issue #458 ("Update test suite images") changed state ("closed") https://gitlab.com/BuildStream/buildstream/issues/458 | 10:41 |
| gitlab-br-bot | buildstream: merge request (chandan/use-testsuite-fedora->master: .gitlab-ci.yml: Use testsuite images for running tests) #531 changed state ("merged"): https://gitlab.com/BuildStream/buildstream/merge_requests/531 | 10:41 |
| gitlab-br-bot | buildstream: merge request (Qinusty/scheduler_docstring_fix->master: WIP: scheduler.py: Correct some anomalies within the docstrings) #551 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/551 | 10:41 |
| jmac | qinusty: Yep, all good! | 10:41 |
| jmac | I would merge that but I'm unsure how we do that | 10:42 |
| gitlab-br-bot | buildstream: merge request (Qinusty/scheduler_docstring_fix->master: scheduler.py: Correct some anomalies within the docstrings) #551 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/551 | 10:42 |
| *** bethw has quit IRC | 10:44 | |
| *** bethw has joined #buildstream | 10:45 | |
| *** jonathanmaw has quit IRC | 10:54 | |
| *** jonathanmaw has joined #buildstream | 10:59 | |
| *** jonathanmaw_ has joined #buildstream | 10:59 | |
| *** jonathanmaw has quit IRC | 11:00 | |
| *** jonathanmaw_ has quit IRC | 11:00 | |
| *** bethw has quit IRC | 11:01 | |
| *** jonathanmaw has joined #buildstream | 11:02 | |
| *** jonathanmaw has quit IRC | 11:09 | |
| *** jonathanmaw has joined #buildstream | 11:09 | |
| gitlab-br-bot | buildstream: merge request (sam/pushing-fix->master: WIP: _artifactcache/pushreceive.py: Avoid premature 'done' messages) #532 changed state ("closed"): https://gitlab.com/BuildStream/buildstream/merge_requests/532 | 11:38 |
| *** ernestask has quit IRC | 11:58 | |
| *** ernestask has joined #buildstream | 12:09 | |
| gitlab-br-bot | buildstream: merge request (phil/437-workspaces-tutorial->master: Phil/437 workspaces tutorial) #519 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/519 | 12:12 |
| gitlab-br-bot | buildstream: merge request (valentindavid/331_include->master: Add support for include in project.conf) #471 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/471 | 12:18 |
| tlater | jmac: Since we don't have to wait for tristan / juergbi to merge things anymore, you're allowed to click the button :) | 12:18 |
| gitlab-br-bot | buildstream: issue #483 ("Buildstream should fail if ref: is not within the tag/branch specify in track:") changed state ("opened") https://gitlab.com/BuildStream/buildstream/issues/483 | 12:19 |
| tlater | I'm also still to scared, though. | 12:19 |
| laurence | if it's genuinely tiny / trivial, then don't be scared :) | 12:19 |
| laurence | i'm aware that 'tiny / trivial' is not 100% explicitly defined, though | 12:19 |
| jmac | tlater: Is gitlab set up correctly to merge without merge commits if I do that? | 12:20 |
| tlater | laurence: This one is definitely trivial by any definition | 12:20 |
| tlater | jmac: Let me check what this branch does | 12:20 |
| jmac | tristan always merged outside GitLab, so I thought there might be something special needed | 12:20 |
| tlater | jmac: For this branch, someone will want to `git rebase` | 12:20 |
| laurence | tlater, thought so :) | 12:21 |
| tlater | jmac: But generally, I've not seen gitlab produce a merge commit. Tristan only sometimes merged from outside gitlab. | 12:22 |
| jmac | All good then | 12:22 |
| jmac | qinusty: Please rebase !551 onto master (someone has made commits to master since this morning) and I'll merge | 12:23 |
| paulsherwood | hmmm... is the project setup to include ff with merge commmits? | 12:23 |
| * paulsherwood would prefer that | 12:23 | |
| * tlater ponders what ff means in this context... FireFox? | 12:24 | |
| jmac | Fast-forward | 12:24 |
| noisecell | jonathanmaw, https://gitlab.com/franred/buildstream/pipelines/26016896 | 12:24 |
| tlater | Ah | 12:24 |
| jmac | I also prefer merge commits but tristan_ doesn't, AFAIR | 12:24 |
| tlater | Yep, we don't do merge commits at all - not in any of the other projects either afaik. | 12:25 |
| Kinnison | There're arguments either way. I tend to avoid merge commits where the patch series is clean/neat but will use them to bring something otherwise unwieldy/difficult-to-split in :-) | 12:26 |
| jonathanmaw | noisecell: very odd that whether an element is cached or buildable depends on whether it's debian or fedora | 12:27 |
| *** bethw has joined #buildstream | 12:28 | |
| noisecell | jonathanmaw, yup, it is a very odd behaviour maybe tlater or juergbi know anything about it? | 12:29 |
| *** xjuan has joined #buildstream | 12:31 | |
| tlater | Phew, that's an odd one | 12:32 |
| tlater | noisecell: Have you tried restarting the CI at all? | 12:32 |
| *** jonathanmaw_ has joined #buildstream | 12:33 | |
| noisecell | tlater, gilab CI? | 12:33 |
| *** jonathanmaw has quit IRC | 12:33 | |
| tlater | Yes - there's a chance it's a fluke | 12:33 |
| tlater | Obviously still something we need to debug | 12:33 |
| tlater | But at least that would mean the issue isn't caused by your branch | 12:33 |
| jonathanmaw_ | tlater: this is a failure on the third retry | 12:34 |
| *** jonathanmaw_ is now known as jonathanmaw | 12:34 | |
| tlater | Then it seems like it's somehow caused by the branch | 12:34 |
| * tlater has a peek, looks to be related to expiry at least | 12:34 | |
| * noisecell clears the cache and re-runs the pipeline | 12:34 | |
| tlater | Oh, yeah, expiry... | 12:34 |
| noisecell | tlater, I didn't add any tests, so if it is failing for my branch should be failing for others too? | 12:35 |
| tlater | noisecell: It doesn't, hence my confusion | 12:35 |
| tlater | If it still fails after *that* retry, tell me | 12:36 |
| tlater | I wonder if we're just running out of disk space on the runner | 12:36 |
| tlater | That might cause the expiry tests to go haywire | 12:36 |
| noisecell | ok, I will keep you informed | 12:37 |
| tristan_ | paulsherwood, jmac ... I'm ready to cave in to the semi-linear history with merge commits setting on gitlab, if only to slightly lower the bar for reviews in light of the new policies | 12:50 |
| * tristan_ was also thinking about that this week too | 12:51 | |
| * Kinnison would prefer that each merge *could be* FF | 12:51 | |
| Kinnison | even if it's then merged with a merge commit | 12:51 |
| Kinnison | Since it shows that the MR has been tested in combination with master | 12:51 |
| Kinnison | (unless the CI already does that, in which case I'll shush up :-) ) | 12:52 |
| gitlab-br-bot | buildstream: merge request (Qinusty/scheduler_docstring_fix->master: scheduler.py: Correct some anomalies within the docstrings) #551 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/551 | 12:52 |
| paulsherwood | Kinnison: ci does that | 12:53 |
| tristan_ | Kinnison, the gitlab setting is a bit confusing in calling it "semi-linear" | 12:53 |
| Kinnison | paulsherwood: then I shall shush up :-D | 12:54 |
| tristan_ | but it indeed does that (there is another non-linear setting which I would argue vehemently against) | 12:54 |
| * paulsherwood would also propose set all commits to 2 approvers for now... to reduce the fear factor and increase the incentive for reviews | 12:54 | |
| paulsherwood | s/commits/merge requests/ | 12:54 |
| * Kinnison nods | 12:54 | |
| Kinnison | 2 approvers makes a lot of sense to me | 12:54 |
| tristan_ | I still dislike approvers for exactly that reason | 12:54 |
| paulsherwood | ??? | 12:54 |
| tristan_ | in the end, someone has to hit the merge button | 12:54 |
| paulsherwood | so what? second approver can do it? | 12:55 |
| * paulsherwood has found this approach works reasonably well at scale | 12:55 | |
| tristan_ | having multiple approvers is fine, they dont need to block landing it, and they should not reduce the sense of accountability you take when you hit merge | 12:55 |
| Kinnison | +1 on accountability when hitting merge | 12:55 |
| * paulsherwood doesn't get tristan's point | 12:56 | |
| Kinnison | tristan_: think of it as A writes, B reviews positively, C reviews and hits merge. All three are on-hook | 12:56 |
| paulsherwood | if i propose MR, i'm placing myself on the hook | 12:56 |
| noisecell | tlater, it is still failing: https://gitlab.com/franred/buildstream/pipelines/26087728 -- see test-unix job | 12:56 |
| paulsherwood | i don't propose MRs just so someone else will test/fix my stuff | 12:57 |
| * Kinnison thinks that, at least short term, requiring a minimum of three humans to be involved will mitigate the (relative) inexperience of some of the humans | 12:57 | |
| paulsherwood | +1 | 12:57 |
| Kinnison | the three being the author, and two reviewers | 12:57 |
| noisecell | tlater, do I need to configure or enable anything for the cache expiry to work? ...although are the tests which is what are failing... | 12:57 |
| * Kinnison also doesn't think that it'd be unreasonable for a more experienced author to have their MR merged with only one reviewer if it's urgent | 12:57 | |
| Kinnison | However, I'll bow to tristan_'s decisions on the governance ultimately, obviously :-) | 12:58 |
| paulsherwood | :-) | 12:58 |
| paulsherwood | seen elsewhere "4. YBD seemed a little faster than BuildStream | 12:59 |
| * paulsherwood gives a silent w00T inside :) | 12:59 | |
| tristan_ | paulsherwood, the point is basically from what I've seen, multiple approvers as a requirement seems to share the accountability, or it re-enforces that perception at least | 12:59 |
| paulsherwood | tristan_: i don't think either of us has more than anecdata for this | 13:00 |
| tristan_ | this is what worries me about the approvers feature in general, person A is familiar enough with part of the patch, person B might be familiar enough with the same part | 13:00 |
| tristan_ | two 50%'s dont amount to a 100% | 13:01 |
| tristan_ | Still, this could just as well be communicated in comments on the merge request | 13:01 |
| * paulsherwood doesn't see how 'no approvers, commit at will' fixes this | 13:01 | |
| tristan_ | which can be more effective than a button | 13:01 |
| paulsherwood | also seen elsewhere" "We found that by switching to BST, we gained a number of advantages. | 13:01 |
| tiagogomes | More daylight at evening? | 13:07 |
| paulsherwood | hah | 13:08 |
| *** toscalix has joined #buildstream | 13:17 | |
| gitlab-br-bot | buildstream: merge request (Qinusty/scheduler_docstring_fix->master: scheduler.py: Correct some anomalies within the docstrings) #551 changed state ("merged"): https://gitlab.com/BuildStream/buildstream/merge_requests/551 | 13:22 |
| paulsherwood | some BST user feedback here https://lists.trustable.io/pipermail/trustable-distros/2018-July/000008.html | 13:24 |
| tlater | noisecell: Any news on that test case? | 13:27 |
| noisecell | tlater, it is still failing: https://gitlab.com/franred/buildstream/pipelines/26087728 -- this time in all the systems | 13:28 |
| tlater | Grr | 13:29 |
| * tlater will have a look | 13:29 | |
| noisecell | tlater, do you know if I need to enable anything or something has changed in the project.conf? | 13:29 |
| tiagogomes | noisecell is your branch rebased on master? | 13:31 |
| tlater | noisecell: Not as far as I know | 13:32 |
| tlater | I suspect this is fallout from the artifact cache expiry branch | 13:32 |
| tiagogomes | As the unit tests are passing for #263 | 13:32 |
| tlater | So give me a minute to look at why this is happening :) | 13:32 |
| noisecell | tiagogomes, I think so | 13:32 |
| noisecell | tiagogomes, https://gitlab.com/BuildStream/buildstream/merge_requests/537 -- it doesn't show it needs to be rebased | 13:34 |
| noisecell | tlater, thank you | 13:34 |
| tiagogomes | noisecell the MR comes from your own personal fork. Are you using runners set up by you? | 13:36 |
| noisecell | tiagogomes, no, the runners are the default runners from gitlab | 13:36 |
| ironfoot | I believe that buildstream will let you be a "Developer" so that you can create branches within buildstream/buildstream.git that will be tested by their runners | 13:46 |
| tlater | noisecell: I recommend you use the gitlab runners | 13:56 |
| tlater | Err, the buildstream runners | 13:56 |
| tlater | I'll give you developer access :) | 13:56 |
| noisecell | tlater, ok | 13:57 |
| tlater | Looks like you already have full maintainer access | 13:58 |
| tpollard | Are there any docs for the frontend? | 13:58 |
| tlater | You'll just need to push your branch to the main repository - that should trigger the test runs | 13:58 |
| tlater | noisecell: Even if that fixes it, I'm still concerned about the test failure | 13:59 |
| tlater | But it may well be that your runners are simply not configured to allow everything we need | 13:59 |
| noisecell | tlater, if that is the case, do we have documentation about how to set up our runners? | 14:00 |
| tlater | noisecell: They're codethink-owned machines | 14:00 |
| * tlater isn't sure how they are set up, but presumably there is documentation somewhere. | 14:01 | |
| noisecell | tlater, sure, although if someone externally or even anyone in the team want to re-do the job I assume we would like to follow the same steps we did in the past ;-) | 14:01 |
| tlater | Yep, I'm aware :) I think it should be easy enough to get documentation from codethink for this though | 14:02 |
| tlater | That said, checking out your branch now to test this locally... | 14:02 |
| noisecell | tlater, I've pushed a rebased branch in: https://gitlab.com/BuildStream/buildstream/pipelines/26096200 | 14:08 |
| tlater | Hm, simply running that docker container + your branch doesn't reproduce it | 14:08 |
| tlater | Thanks! | 14:08 |
| noisecell | is there any way to swap branches in MR? | 14:08 |
| jmac | noisecell: No, you have to create a new MR, unfortuntaely | 14:09 |
| noisecell | jmac, ok, thanks | 14:09 |
| *** Prince781 has joined #buildstream | 14:09 | |
| tlater | noisecell: Do you think you could give me write access to your fork so that I can try this out on one of your runners? | 14:13 |
| tlater | May be a bit too much hassle if it works in the main repository. | 14:13 |
| noisecell | tlater, Im fine granting you access to my fork, it is not an issue | 14:14 |
| tlater | Thanks :) | 14:14 |
| noisecell | tlater, done | 14:15 |
| * tlater starts doing some diagnosis on this cache | 14:15 | |
| tlater | noisecell: Your branch on the main repository looks to pass | 14:21 |
| noisecell | tlater, yup... well at least I can re-submit the MR then :) | 14:22 |
| tlater | Yep, that's fine | 14:22 |
| tlater | I suspect that this is related to the CAS cache requiring different network access | 14:22 |
| tlater | Though if so that's a major PITA | 14:22 |
| tlater | And really, I don't have a clue how that would affect expiry ordering | 14:23 |
| tlater | OH! | 14:23 |
| tlater | Of course! | 14:23 |
| tlater | Gitlab runners probably don't store mtime | 14:23 |
| tlater | Haha | 14:23 |
| tlater | Does anyone know if you can set up a file system that wouldn't remember mtimes? | 14:24 |
| gitlab-br-bot | buildstream: merge request (franred/fix-except-argument-in-source-bundle->master: source-bundle: Enable --except option) #552 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/552 | 14:25 |
| gitlab-br-bot | buildstream: merge request (franred/fix-except-argument-in-source-bundle->master: source-bundle: Enable --except option) #537 changed state ("closed"): https://gitlab.com/BuildStream/buildstream/merge_requests/537 | 14:26 |
| noisecell | tlater, jonathanmaw, https://gitlab.com/BuildStream/buildstream/merge_requests/552 | 14:27 |
| tlater | noisecell: Not sure if you have discussed this, but do you think a test case for this would be appropriate? | 14:28 |
| tlater | Seems like that would be fairly easy to write, since you're fixing a pretty specific bug :) | 14:29 |
| noisecell | tlater, when creating the patch I though about this, but I couldn't find a place where you were checking the arguments for the different commands you are using | 14:29 |
| jonathanmaw | tlater: quite possibly, but iiuc it'll be a fair bit of work, since there's currently no tests that source-bundle works | 14:29 |
| *** cs_shadow has joined #buildstream | 14:29 | |
| noisecell | so I didn't found an example where to base the test | 14:29 |
| tlater | Ah, alright, I thought we had source-bundle tests | 14:29 |
| tlater | Right, I won't complain then | 14:30 |
| noisecell | tlater, maybe we should open an issue to create the tests for source-bundle? | 14:30 |
| tlater | noisecell: Yep, though I haven't checked if there is one yet. | 14:31 |
| tiagogomes | I think there's one already | 14:31 |
| tlater | So it looks like gitlab runners do atimes/mtimes right: https://gitlab.com/franred/buildstream/-/jobs/82978708 | 14:32 |
| noisecell | tiagogomes, oh ok | 14:33 |
| tlater | noisecell: I still can't figure out why this happens. My best guess is that gitlab runners don't store the modification times correctly. | 14:40 |
| tlater | Which is a little problematic | 14:40 |
| tlater | Perhaps we should keep an artifact manifest around after all? | 14:40 |
| tlater | juergbi: Thoughts on this? | 14:40 |
| noisecell | does that mean that we have change the configuration on our runners to store the modification times correctly? | 14:41 |
| paulsherwood | why do mtimes matter for bst? | 14:41 |
| tlater | paulsherwood: We figure out which artifacts to remove based on mtimes - essentially just because that was a convenient place to store the last access time. | 14:42 |
| juergbi | tlater: on what specifically? | 14:42 |
| paulsherwood | tlater: ack | 14:42 |
| tlater | juergbi: noisecell just ran into a bug with artifact cache expiry - it seems that on normal gitlab runners, the artifacts will expire in the wrong order | 14:42 |
| tlater | I suspect they don't keep track of mtimes correctly | 14:42 |
| juergbi | that would seem really odd, mtime is crucial in many build systems | 14:43 |
| juergbi | atime can be fragile, but mtime should be reliable | 14:43 |
| tlater | Hrm | 14:43 |
| tlater | I'm not really sure what else could change between our runners and theirs | 14:44 |
| tristan_ | juergbi, note that subsecond precision of mtimes is not entirely widespread | 14:44 |
| juergbi | tlater: is it possible the filesystem handles it properly but the CI cache zip doesn't? | 14:44 |
| tristan_ | I think ext4 | 14:44 |
| juergbi | that's true, we should probably not rely on subsecond mtime | 14:44 |
| juergbi | (although I think ext4 supports this) | 14:44 |
| tlater | Ah, we do write very small artifacts | 14:44 |
| tlater | So perhaps there's a chance their mtimes aren't granular enough? | 14:44 |
| tristan_ | for a test case, I'd expect a few artifacts created in the same second | 14:45 |
| tlater | Yep, that would also explain the slight bit of nondeterminism | 14:45 |
| juergbi | tlater: the CLI tool `stat` should show subsecond precision, maybe use this as test | 14:45 |
| juergbi | tristan_: btw: just a reminder, do you want to tag 1.3.0? | 14:46 |
| tristan_ | sure | 14:46 |
| tristan_ | I'll push one now | 14:46 |
| tlater | Looks suspiciously like non-sub-second mtimes: https://gitlab.com/franred/buildstream/-/jobs/82985577 | 14:48 |
| * tlater will file a bug, this doesn't seem critical enough to need fixing immediately | 14:48 | |
| *** leopi has joined #buildstream | 14:52 | |
| tristan_ | tlater, in practice, it seems that that bug is doesnt really have negative side effects, but if it effects CI it's certainly worth keeping track of | 14:52 |
| tristan_ | if you're going to nuke a least recently used artifact, and have to choose between two that were created in the same second... meh | 14:53 |
| gitlab-br-bot | buildstream: issue #484 ("Artifact cache expiry relies on granular mtimes") changed state ("opened") https://gitlab.com/BuildStream/buildstream/issues/484 | 14:58 |
| tlater | tristan_: I figure it's probably enough to skip that test if we're on a machine that doesn't support sub-second mtimes. | 14:59 |
| tristan_ | tlater, I agree, but would hope that determining that is difficult | 15:00 |
| tristan_ | of course, we're going to hit this again next year when intel ships meltdown free chips that are 1000 times faster than 8th gen | 15:00 |
| tlater | Maybe it's enough to check if the millisecond points in are 0 ;) | 15:00 |
| tristan_ | and all the artifacts are created in the same microsecond | 15:01 |
| tlater | Hehe | 15:01 |
| tlater | I guess we can just sleep for a second between each artifact, too | 15:01 |
| tristan_ | that would still be better to do conditionally, though | 15:01 |
| tristan_ | (sleep 2 seconds, even) | 15:02 |
| *** Prince781 has quit IRC | 15:08 | |
| *** Prince781 has joined #buildstream | 15:11 | |
| gitlab-br-bot | buildstream: merge request (valentindavid/331_include->master: Add support for include in project.conf) #471 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/471 | 15:11 |
| tristan_ | noisecell, tlater, tiagogomes; for reference, it's: https://gitlab.com/BuildStream/buildstream/issues/310 | 15:18 |
| tristan_ | would be great to get coverage there indeed :-/ | 15:18 |
| * tlater even filed that issue | 15:25 | |
| tlater | How did I forget? | 15:25 |
| tiagogomes | Punish yourself by fixing it now :) | 15:26 |
| noisecell | tlater, jonathanmaw, tristan_, happy for me to merge: https://gitlab.com/BuildStream/buildstream/merge_requests/552 ? | 15:38 |
| *** Prince781 has quit IRC | 15:38 | |
| jonathanmaw | I am | 15:39 |
| tlater | I should test it manually first, but I won't block you from merging if jonathanmaw agrees. | 15:40 |
| noisecell | tlater, I can wait for you to test | 15:42 |
| noisecell | it | 15:42 |
| tlater | Alright :) | 15:42 |
| gitlab-br-bot | buildstream: merge request (Qinusty/275->master: Qinusty/275) #553 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/553 | 15:46 |
| tlater | noisecell: Looks good :) | 15:48 |
| tlater | Feel free to merge | 15:49 |
| gitlab-br-bot | buildstream: merge request (Qinusty/275->master: Indicate where artifacts are going to and coming from in the log) #553 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/553 | 15:53 |
| gitlab-br-bot | buildstream: merge request (Qinusty/275->master: Indicate where artifacts are going to and coming from in the log) #553 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/553 | 16:09 |
| rdale | is in possible to build a stage1 gcc with the tiny c compiler in the alpine tarball? | 16:11 |
| gitlab-br-bot | buildstream: merge request (franred/fix-except-argument-in-source-bundle->master: source-bundle: Enable --except option) #552 changed state ("merged"): https://gitlab.com/BuildStream/buildstream/merge_requests/552 | 16:19 |
| *** jonathanmaw has quit IRC | 16:33 | |
| *** Prince781 has joined #buildstream | 16:36 | |
| qinusty | Can someone review https://gitlab.com/BuildStream/buildstream/merge_requests/553/ please? | 16:46 |
| *** bethw has quit IRC | 16:46 | |
| *** qinusty has quit IRC | 16:58 | |
| *** coldtom has quit IRC | 16:59 | |
| *** tpollard has quit IRC | 17:16 | |
| *** toscalix has quit IRC | 17:24 | |
| *** ernestask has quit IRC | 17:27 | |
| *** leopi has quit IRC | 18:09 | |
| alatiera | would it be possible to set it so when bst opens a workspace it uses an ssh url for the git remote instead of http? | 18:26 |
| cs_shadow | one approach would be to use ssh urls in the element instead of https urls | 18:32 |
| cs_shadow | if you can't change that for whatever reason, then a workaround would be to use "insteadOf" config like so: https://stackoverflow.com/a/22027731 | 18:32 |
| *** vraj has joined #buildstream | 18:57 | |
| *** vraj has joined #buildstream | 18:58 | |
| *** vraj has quit IRC | 19:01 | |
| *** tristan_ has quit IRC | 19:13 | |
| gitlab-br-bot | buildstream: issue #468 ("source-bundle: --except argument is ignored") changed state ("closed") https://gitlab.com/BuildStream/buildstream/issues/468 | 19:49 |
| *** Prince781 has quit IRC | 20:05 | |
| *** jsgrant has joined #buildstream | 21:34 | |
| *** xjuan has quit IRC | 21:49 | |
| gitlab-br-bot | buildstream: issue #485 ("RFE: Add "bst fmt" (or similar) to rewrite .bst files to canonical format") changed state ("opened") https://gitlab.com/BuildStream/buildstream/issues/485 | 22:47 |
| gitlab-br-bot | buildstream: merge request (tiagogomes/tarball_checkout->master: Add support for creating artifact tarballs) #546 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/546 | 22:50 |
| *** bethw has joined #buildstream | 22:55 | |
| *** cs_shadow has quit IRC | 23:10 | |
| gitlab-br-bot | buildstream: merge request (franred/fix-except-argument-in-source-bundle->master: source-bundle: Enable --except option) #552 changed state ("merged"): https://gitlab.com/BuildStream/buildstream/merge_requests/552 | 23:41 |
| *** bethw has quit IRC | 23:45 | |
| *** lantw44 has quit IRC | 23:59 | |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!