*** Prince781 has quit IRC | 02:24 | |
*** leopi has joined #buildstream | 02:37 | |
*** leopi has quit IRC | 02:46 | |
*** leopi has joined #buildstream | 03:35 | |
*** leopi has quit IRC | 04:32 | |
*** cs_shadow has quit IRC | 04:43 | |
*** leopi has joined #buildstream | 05:22 | |
gitlab-br-bot | buildstream: merge request (juerg/googlecas->master: WIP: Remote Execution CAS-based artifact cache) #337 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/337 | 05:58 |
---|---|---|
*** Prince781 has joined #buildstream | 06:22 | |
*** Prince781 has quit IRC | 06:23 | |
*** tristan has joined #buildstream | 06:32 | |
*** ernestask has joined #buildstream | 06:36 | |
tristan | juergbi, lets land this baby ? | 07:10 |
*** Phil has joined #buildstream | 07:12 | |
gitlab-br-bot | buildstream: merge request (juerg/googlecas->master: Remote Execution CAS-based artifact cache) #337 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/337 | 07:24 |
juergbi | the fedora 27 test is still pending for some reason but I've now removed the WIP status | 07:24 |
tristan | tlater, I built epiphany over night and seems to work well... also the builds on gitlab are passing except for the weird GPGME spurious error | 07:24 |
gitlab-br-bot | buildstream: issue #457 ("Filter elements can't be checked-out because it checks out the depended-upon element instead") changed state ("closed") https://gitlab.com/BuildStream/buildstream/issues/457 | 07:27 |
* juergbi retries the fedora 27 test job | 07:28 | |
tristan | tlater, I'm pretty convinced that we're hitting this: https://lists.gnupg.org/pipermail/gnupg-announce/2018q2/000424.html | 07:36 |
tristan | tlater, i.e. filed under "not our problem" | 07:36 |
*** leopi has quit IRC | 07:40 | |
*** coldtom has joined #buildstream | 07:51 | |
gitlab-br-bot | buildstream: issue #134 ("Only push the objects that are not in the remote artifact share") changed state ("closed") https://gitlab.com/BuildStream/buildstream/issues/134 | 07:59 |
gitlab-br-bot | buildstream: issue #138 ("aborting bst push command causes stack trace") changed state ("closed") https://gitlab.com/BuildStream/buildstream/issues/138 | 07:59 |
gitlab-br-bot | buildstream: issue #148 ("Leaked directories when failing to push") changed state ("closed") https://gitlab.com/BuildStream/buildstream/issues/148 | 07:59 |
gitlab-br-bot | buildstream: issue #387 ("Remote Execution CAS-based artifact cache") changed state ("closed") https://gitlab.com/BuildStream/buildstream/issues/387 | 07:59 |
gitlab-br-bot | buildstream: issue #217 ("Updating project.conf with a new url does not cause an update of the ostree config") changed state ("closed") https://gitlab.com/BuildStream/buildstream/issues/217 | 07:59 |
gitlab-br-bot | buildstream: issue #148 ("Leaked directories when failing to push") changed state ("closed") https://gitlab.com/BuildStream/buildstream/issues/148 | 07:59 |
gitlab-br-bot | buildstream: issue #268 ("Stack trace when ostree operates in filesystem not supporting extended attributes") changed state ("closed") https://gitlab.com/BuildStream/buildstream/issues/268 | 07:59 |
gitlab-br-bot | buildstream: issue #217 ("Updating project.conf with a new url does not cause an update of the ostree config") changed state ("closed") https://gitlab.com/BuildStream/buildstream/issues/217 | 07:59 |
gitlab-br-bot | buildstream: issue #268 ("Stack trace when ostree operates in filesystem not supporting extended attributes") changed state ("closed") https://gitlab.com/BuildStream/buildstream/issues/268 | 08:00 |
gitlab-br-bot | buildstream: issue #276 ("Downloading times out; doesn't resume: "[28] Timeout was reached"") changed state ("closed") https://gitlab.com/BuildStream/buildstream/issues/276 | 08:00 |
gitlab-br-bot | buildstream: issue #276 ("Downloading times out; doesn't resume: "[28] Timeout was reached"") changed state ("closed") https://gitlab.com/BuildStream/buildstream/issues/276 | 08:00 |
gitlab-br-bot | buildstream: issue #443 ("Incorrect license at buildstream/_artifactcache/pushreceive.py ?") changed state ("closed") https://gitlab.com/BuildStream/buildstream/issues/443 | 08:00 |
gitlab-br-bot | buildstream: issue #443 ("Incorrect license at buildstream/_artifactcache/pushreceive.py ?") changed state ("closed") https://gitlab.com/BuildStream/buildstream/issues/443 | 08:00 |
gitlab-br-bot | buildstream: issue #460 ("Unexplained errors pushing to OSTree artifact cache") changed state ("closed") https://gitlab.com/BuildStream/buildstream/issues/460 | 08:00 |
gitlab-br-bot | buildstream: issue #387 ("Remote Execution CAS-based artifact cache") changed state ("closed") https://gitlab.com/BuildStream/buildstream/issues/387 | 08:00 |
gitlab-br-bot | buildstream: issue #460 ("Unexplained errors pushing to OSTree artifact cache") changed state ("closed") https://gitlab.com/BuildStream/buildstream/issues/460 | 08:00 |
gitlab-br-bot | buildstream: merge request (juerg/googlecas->master: Remote Execution CAS-based artifact cache) #337 changed state ("merged"): https://gitlab.com/BuildStream/buildstream/merge_requests/337 | 08:00 |
* juergbi goes into hiding in case anything explodes | 08:00 | |
noisecell | heh | 08:01 |
juergbi | tristan: I assume the tiny MR https://gitlab.com/BuildStream/buildstream/merge_requests/543 is fine with you | 08:01 |
gitlab-br-bot | buildstream: merge request (juerg/project-cachekey->master: _project.py: Include fail-on-overlap setting in cache key) #543 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/543 | 08:05 |
gitlab-br-bot | buildstream: merge request (phil/203-BuildStream-crashes-when-dependency-tree-too-deep->master: Phil/203 build stream crashes when dependency tree too deep) #512 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/512 | 08:08 |
tristan | juergbi, yeah that's fine | 08:09 |
tristan | lemme see the code one sec... | 08:10 |
tristan | meh, yeah that's alright | 08:10 |
*** Prince781 has joined #buildstream | 08:10 | |
tristan | technically it need not effect the cache keys for elements which have no dependencies | 08:10 |
tristan | juergbi, is it worth making that distinction ? | 08:11 |
*** finn has joined #buildstream | 08:11 | |
tristan | juergbi, we could save the user from processing a GB of base runtime when turning on fail-on-overlaps | 08:11 |
tristan | if we made that distinction | 08:11 |
gitlab-br-bot | buildstream: merge request (phil/203-BuildStream-crashes-when-dependency-tree-too-deep->master: Phil/203 build stream crashes when dependency tree too deep) #512 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/512 | 08:11 |
juergbi | hm, it would be a lot less straight forward, though | 08:12 |
juergbi | given that I expect CI to handle this and push it to the cache, I'm not sure whether it makes a big difference in practice | 08:13 |
* tristan commented on issue | 08:14 | |
tristan | juergbi, I dont think it's right to assume CI or not CI... rather I would expect the opposite at least the first time around | 08:15 |
tristan | i.e... organization has been building stuff with overlap warnings... and a developer is tasked to fix the overlaps and enable the errors | 08:15 |
tristan | the developer has to do that... and push an MR to the project repository which fixes it | 08:15 |
juergbi | yes, that does sound like a common possibility | 08:21 |
tristan | I think on our side, it's an additional if in the construction of the actual key, which is all centralized in a single function in element.py | 08:22 |
juergbi | I don't consider one-time base runtime build by a single maintainer/developer of the project to be a huge issue, but we could indeed avoid it | 08:22 |
juergbi | not quite | 08:22 |
juergbi | we can't put it in the project cache key in that case | 08:22 |
juergbi | as the project cache key might be used for other things in the future | 08:22 |
tristan | Certainly | 08:22 |
juergbi | but yes, we can just directly include it in element.py I suppose | 08:22 |
tristan | I.e. yes I agree... it's an element augmenting it's cache key based on *its* project's settings | 08:23 |
*** bochecha has joined #buildstream | 08:24 | |
juergbi | ok, I'll change it | 08:25 |
tristan | I think it's better... the time it takes us to change this and consider it this way... is already less than the time a single developer would have to waste downloading a base runtime and reconstructing the base artifact locally | 08:26 |
tristan | (depending on the runtime and the internet connection) | 08:26 |
tristan | Well, assuming they already have a cached version, which is often a one off | 08:27 |
*** leopi has joined #buildstream | 08:30 | |
*** Prince781 has quit IRC | 08:37 | |
* tristan goes to eat, back in ~1hrs | 08:41 | |
gitlab-br-bot | buildstream: merge request (juerg/project-cachekey->master: _project.py: Include fail-on-overlap setting in cache key) #543 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/543 | 08:41 |
*** jonathanmaw has joined #buildstream | 08:42 | |
*** tristan has quit IRC | 08:47 | |
*** bethw has joined #buildstream | 08:56 | |
laurence | Just seen the CAS cache was merged - woo! https://gitlab.com/BuildStream/buildstream/merge_requests/337 | 09:03 |
*** tiago has quit IRC | 09:04 | |
*** tiagogomes has joined #buildstream | 09:04 | |
jmac | I could really do with a review of https://gitlab.com/BuildStream/buildstream/merge_requests/445 | 09:06 |
*** leopi has quit IRC | 09:09 | |
juergbi | that's on my list now | 09:09 |
gitlab-br-bot | buildstream: issue #473 ("project.conf configuration parameters should be part of the cache calculation") changed state ("closed") https://gitlab.com/BuildStream/buildstream/issues/473 | 09:10 |
gitlab-br-bot | buildstream: merge request (juerg/project-cachekey->master: _project.py: Include fail-on-overlap setting in cache key) #543 changed state ("merged"): https://gitlab.com/BuildStream/buildstream/merge_requests/543 | 09:10 |
gitlab-br-bot | buildstream: merge request (valentindavid/331_include->master: WIP: Add support for include in project.conf) #471 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/471 | 09:17 |
*** leopi has joined #buildstream | 09:25 | |
*** tristan has joined #buildstream | 09:35 | |
tlater | tristan: Nice, this is looking good :) | 09:36 |
tlater | I failed my local build, but mostly because I forgot which branch of buildstream I had installed. | 09:37 |
tlater | Do you think you hit the actual expiry at any point during those builds? | 09:37 |
tristan | tlater, no :'( | 09:47 |
tristan | I wanna trigger it | 09:47 |
tristan | tlater, I searched for 'cleanup' in the log, which appears to be logged by _scheduler/jobs/cleanupjob.py | 09:48 |
jonathanmaw | Hi Sam, I'm looking at https://gitlab.com/BuildStream/bst-external/issues/10 and I don't think I understand your problems well enough to know how to make a solution *possible*: | 10:00 |
jonathanmaw | 1. Buildstream's integration-commands don't work for you because they're run | 10:00 |
jonathanmaw | at checkout time, and fail because there isn't a complete sysroot. | 10:00 |
jonathanmaw | 2. Running those commands with a script element is suboptimal because it means | 10:00 |
jonathanmaw | staging the entire gnome sdk. | 10:00 |
jonathanmaw | how do flatpaks normally have these commands run? | 10:00 |
laurence | Did everyone else get a mail this morning re the irc team meeting agenda next week | 10:01 |
laurence | ? | 10:01 |
jonathanmaw | I got it | 10:01 |
gitlab-br-bot | buildstream: merge request (tiagogomes/tarball_checkout->master: WIP Add support for dumping a tarball stream) #546 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/546 | 10:01 |
laurence | thanks j | 10:01 |
laurence | jonathanmaw, | 10:02 |
Phil | laurence, yes | 10:02 |
tlater | tristan: You should probably set a slightly lower cache quota than full disk space ;) | 10:03 |
tristan | yeah | 10:04 |
tristan | tlater, ok so you are in the middle of rebasing against CAS right ? | 10:04 |
tristan | tlater, I'm going to run through the actual code and give a light review of it in the meantime | 10:04 |
gitlab-br-bot | buildstream: merge request (valentindavid/331_include->master: WIP: Add support for include in project.conf) #471 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/471 | 10:05 |
tristan | tlater, do you think we'll be able to land this today ? that's my goal | 10:05 |
*** tiagogomes has quit IRC | 10:05 | |
*** tiagogomes has joined #buildstream | 10:06 | |
tlater | tristan: I think it's possible | 10:06 |
tlater | I'm currently discussing something else, but I'll get right back on track in a minute or 10 | 10:07 |
*** jennis has quit IRC | 10:39 | |
*** jennis has joined #buildstream | 10:39 | |
*** jennis has quit IRC | 10:42 | |
*** jennis has joined #buildstream | 10:42 | |
*** ernestask has quit IRC | 10:48 | |
ssssam[m] | jonathanmaw: I don't know | 11:02 |
ssssam[m] | jonathanmaw: I think flatpak-builder is hardcoded to run them | 11:02 |
ssssam[m] | by the way, to highlight me I think you need to type ssssam[m], not just 'sam' :-) | 11:03 |
tpollard | is there docs for installing bst-external? | 11:13 |
*** jennis2 has joined #buildstream | 11:23 | |
*** jennis has quit IRC | 11:24 | |
jonathanmaw | ssssam[m]: ah, yes. I was going through the rubber duck process of typing my question out into a text document first | 11:37 |
tiagogomes | tpollard, I did ` pip3 install --user -e .` | 11:38 |
jonathanmaw | ssssam[m]: okay, so normally when a flatpak is built it uses host tools to run those commands when that flatpak is being built? | 11:40 |
jonathanmaw | rather than being run when the flatpak is installed? | 11:42 |
jonathanmaw | ssssam[m]: because if it's the first, it sounds like flatpak integration commands should be run during artifact assembly, so the problem is that those commands should be built-in to the flatpak-image plugin, and ideally have a solution to it taking so long to stage gnome-sdk | 11:44 |
*** ernestask has joined #buildstream | 12:00 | |
jjardon | tpollard: tiagogomes probably http://buildstream.gitlab.io/bst-external/ needs some improvements | 12:05 |
*** gnurlu1 has joined #buildstream | 12:17 | |
ssssam[m] | jonathanmaw: i'd hope it uses tools from the sdk rather than from the host | 12:18 |
ssssam[m] | but yes, i don't think the commands are being run when the flatpak is installed | 12:18 |
tpollard | ty | 12:18 |
ssssam[m] | if they were, we'd be able to reuse that feature | 12:18 |
ssssam[m] | i think the correct solution would be to copy whatever flatpak-builder is doing in the flatpak_image plugin | 12:18 |
tlater | o/ gnurlu1 | 12:19 |
gnurlu1 | thank you tlater! | 12:20 |
tlater | Maybe we should have a channel on freenode that just links people here ;) | 12:20 |
jonathanmaw | ssssam[m]: okay, so running those commands as part of flatpak_image will help there. | 12:21 |
gitlab-br-bot | buildstream: merge request (add_flag_to_checkout->master: Add a `--deps` flag to `bst checkout`) #545 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/545 | 12:21 |
jonathanmaw | and if I can stage the GNOME SDK, plus just the flatpak app, that'll be an improvement on staging times compared to what you're doing now | 12:21 |
ssssam[m] | it'll mean removing the current flatpak-integration.bst element from the pipeline altogether, which will be a nice speedup | 12:24 |
*** leopi has quit IRC | 12:28 | |
tristan | looks like gitlab has changed in such a way that instead of the server becoming unresponsive, the whole web browser becomes unresponsive :-S | 12:33 |
tlater | Yep, I think it's their syntax highlighting | 12:36 |
* tlater wonders if there's a gitlab MR interface for emacs | 12:36 | |
ssssam[m] | once upon a time gitlab used to cause webkit based browsers to crash completely | 12:37 |
ssssam[m] | there must be a parallel universe somewhere in which that bug didn't happen, and Baserock switched to GitLab 5 years ago and became an enourmous success | 12:37 |
ssssam[m] | (whereas in this universe GitLab was rejected due to that rather critical bug) | 12:38 |
tlater | git really should have a built-in merge request thing, so that these websites can just use that as a common standard | 12:39 |
*** lantw44 has quit IRC | 12:39 | |
tlater | (I am aware of git's mail integration, it just doesn't support commenting and such) | 12:39 |
*** lantw44 has joined #buildstream | 12:40 | |
jmac | Various approaches to that have been suggested | 12:40 |
jmac | It's my view, for example, that git does support commenting, but most people sound horrified when I suggest it | 12:40 |
tlater | I'm with you - though arguably there's a question where version control ends. | 12:42 |
*** lantw44 has quit IRC | 12:42 | |
*** lantw44 has joined #buildstream | 12:42 | |
gitlab-br-bot | buildstream: merge request (135-expire-artifacts-in-local-cache->master: WIP: Resolve "Expire artifacts in local cache") #347 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/347 | 12:46 |
gitlab-br-bot | buildstream: merge request (135-expire-artifacts-in-local-cache->master: WIP: Resolve "Expire artifacts in local cache") #347 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/347 | 12:47 |
skullman | tlater: notedb was meant to be that | 12:48 |
persia | Still is that, isn't it? Although I haven't seen any recent discussion about landing the spec in git-core. | 12:49 |
gitlab-br-bot | buildstream: merge request (add_flag_to_checkout->master: Add a `--deps` flag to `bst checkout`) #545 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/545 | 12:54 |
juergbi | symlink support for the remote execution API has been merged upstream \o/ and buildstream master already has that exact version | 12:54 |
tristan | tlater, soooo, I'm having a very hard time to post comments and review with the new sluggish client side JS deployed by gitlab :-S | 12:55 |
tristan | tlater, it looks to me that logging is not streamlined as we had planned it to be, which is disappointing :-S | 12:55 |
tristan | I'm trying to figure on if I can reasonably refactor tomorrow before branching and releasing | 12:56 |
*** leopi has joined #buildstream | 13:11 | |
tristan | tlater, ok seeing as it's already passed 10pm... I don't think we'll finish everything today | 13:14 |
tristan | tlater, do you think you can on your side finish rebasing against CAS, and take care of the various nitpick comments I've put up on the MR ? | 13:14 |
tristan | Then I can try to get an early start tomorrow and see if I can streamline the logging code so there is no duplication or knowledge of "how to log" leaking into Job subclasses | 13:15 |
jmac | Do we document the expected results of overwriting (files/symlinks/directories) with (files/symlinks/directories) with the same name when staging? | 13:18 |
jmac | At the moment everything gets replaced except when you try to replace a file or symlink with a directory, and I'm wondering if that is by design | 13:18 |
tristan | jmac, what I can certainly say is intentional, is that we need to support symlinked directories; and this usually works by having something low enough in the stack which you depend on which creates the symlink | 13:21 |
tlater | tristan: Yup, I can definitely do that | 13:22 |
tristan | jmac, i.e. consider a system with a bunch of stuff that is normally using the "/opt/org" prefix, if all of that stuff depends on an element which creates a symlink "/opt/org -> /usr/local/org", then each artifact produced will not have the symlink | 13:23 |
tristan | jmac, but when staging those artifacts against the base one which created the symlink, the files will go into the right place in /usr/local/org and the symlink is preserved | 13:24 |
gitlab-br-bot | buildstream: merge request (135-expire-artifacts-in-local-cache->master: WIP: Resolve "Expire artifacts in local cache") #347 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/347 | 13:24 |
gitlab-br-bot | buildstream: issue #425 ("Add a `--deps` flag to `bst checkout`") changed state ("closed") https://gitlab.com/BuildStream/buildstream/issues/425 | 13:25 |
gitlab-br-bot | buildstream: merge request (add_flag_to_checkout->master: Add a `--deps` flag to `bst checkout`) #545 changed state ("merged"): https://gitlab.com/BuildStream/buildstream/merge_requests/545 | 13:25 |
tristan | tlater, what I think is mostly missing, is the migration of the Element._logfile() related context managers into the base Job class | 13:25 |
gitlab-br-bot | buildstream: merge request (phil/203-BuildStream-crashes-when-dependency-tree-too-deep->master: Phil/203 build stream crashes when dependency tree too deep) #512 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/512 | 13:26 |
tristan | this seems to be at the root of job classes trying to special case logging differently | 13:26 |
jmac | tristan: I think that's a separate issue | 13:27 |
tlater | Hm, iirc that was mostly because normal element jobs want to handle logging differently | 13:27 |
gitlab-br-bot | buildstream: merge request (tiagogomes/tarball_checkout->master: WIP Add support for dumping a tarball stream) #546 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/546 | 13:27 |
tlater | But I figure there's probably a way to make that nicer, thinking about it. | 13:28 |
gitlab-br-bot | buildstream: merge request (phil/add-ubuntu-ci-job->master: .gitlab-ci-yml: Add ubuntu 18 test) #523 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/523 | 13:28 |
tlater | I also think some of the frontend bindings are a bit annoying | 13:28 |
*** tintou_ has quit IRC | 13:28 | |
tlater | The frontend really shouldn't be tied into handling finishing jobs that tightly... | 13:29 |
*** tintou has joined #buildstream | 13:29 | |
gitlab-br-bot | buildstream: merge request (phil/add-ubuntu-ci-job->master: .gitlab-ci-yml: Add ubuntu 18 test) #523 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/523 | 13:29 |
tristan | tlater, the element jobs dont want to do logging differently, they need only inform the base class of whatever particulars the base class needs to know in order to implement the Context.message() APIs | 13:30 |
tristan | tlater, the frontend handling of messages for failures is definitely something that also needs fixing, yes; we should be synchronizing the delivery of the failure details to the frontend | 13:31 |
tristan | instead of looking back at failed messages, which is really gross | 13:31 |
tristan | the frontend also ideally should never see a Job or a Queue, but that is less severe | 13:32 |
tristan | tlater, anyway I'm out for now... if you do feel like attempting a cleanup of that, please keep it separate just in case hehe :) | 13:35 |
tlater | Will do :) | 13:35 |
tlater | o/ tristan | 13:35 |
tristan | \o | 13:35 |
*** tristan has quit IRC | 13:38 | |
tlater | juergbi: You look to have changed some of the testutils API for artifact shares | 13:43 |
tlater | I think your push.py tests make use of that, but in a commit before those changes are introduced - any idea what commit the API changes are in? | 13:43 |
juergbi | oh, hm, let me check | 13:43 |
juergbi | tests/testutils/artifactshare.py: Use CAS artifact server | 13:44 |
juergbi | right, I moved the artifact expiry tests around a bit too much | 13:45 |
tlater | Thanks - just want to make sure it's not my branch breaking those tests :) | 13:45 |
*** leopi has quit IRC | 13:48 | |
juergbi | tlater: they should all pass in master, though | 13:54 |
juergbi | hm, there is an additional early commit, though: tests/testutils/artifactshare.py: Add support for statvfs mocking | 13:55 |
juergbi | which was supposed to work with that early push commit, but I didn't run tests all the time on every single commit. it's definitely possible it's slightly broken mid-history | 13:55 |
gitlab-br-bot | buildstream: issue #476 ("Specify the behaviour when overwriting files, symlinks or directories") changed state ("opened") https://gitlab.com/BuildStream/buildstream/issues/476 | 13:57 |
juergbi | the push.py tests pass for me at this commit: tests/frontend/push.py: Use ArtifactShare statvfs mocking | 13:57 |
tlater | juergbi: Yep, that's alright - I was going through them commit-by-commit | 13:57 |
tlater | Because I was slowly rebasing my changes on top of yours, doing sanity checks :) | 13:57 |
*** tristan has joined #buildstream | 14:00 | |
gitlab-br-bot | buildstream: merge request (tiagogomes/tarball_checkout->master: WIP Add support for dumping a tarball stream) #546 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/546 | 14:01 |
tlater | juergbi: Hrm, I think I'm going to have to bother you a bit about CAS details | 14:03 |
gitlab-br-bot | buildstream: merge request (willsalmon/documentation_link->master: Adding a helpful link to the example) #533 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/533 | 14:04 |
gitlab-br-bot | buildstream: merge request (willsalmon/documentation_link->master: Adding a helpful link to the example) #533 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/533 | 14:05 |
tlater | Ah, you've already implemented some of my API :) | 14:05 |
tlater | Very nice! | 14:05 |
gitlab-br-bot | buildstream: merge request (willsalmon/documentation_dependency->master: Added dependency to the Docs) #534 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/534 | 14:05 |
jjardon | Hi, can anyone else review https://gitlab.com/BuildStream/buildstream/merge_requests/535 so we can merge it? | 14:08 |
jonathanmaw | ssssam[m]: in the flatpak_image plugin, why do you stage all the files hardlink stage them to somewhere else? | 14:09 |
jonathanmaw | or should I ask valentind? ssssam[m] is the committer, but the source file has valentind as the author. | 14:09 |
jmac | jjardon: Yes, I can, hang on | 14:09 |
gitlab-br-bot | buildstream: merge request (willsalmon/documentation_dependency->master: Added dependency to the Docs) #534 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/534 | 14:09 |
jjardon | also https://gitlab.com/BuildStream/buildstream/merge_requests/542 ? | 14:10 |
juergbi | tlater: do you need deletion/pruning API from CAS beyond what remote expiry uses? | 14:11 |
jjardon | and https://gitlab.com/BuildStream/buildstream/merge_requests/534 (they are little doc fixes) | 14:12 |
juergbi | ideally, things should pretty much be in place already | 14:12 |
valentind | jonathanmaw, would you prefer the directory to be called "usr"? | 14:12 |
tlater | juergbi: I need to calculate the size of the cache and a bit more control over when an artifact is considered "used" | 14:12 |
tlater | Locally we don't want LRM, but LRU | 14:12 |
juergbi | tlater: LRU is actually in place for expiry already on the server side with CAS | 14:13 |
jonathanmaw | valentind: I'd like to know why it's needed | 14:13 |
jonathanmaw | in case I accidentally break it while changing things | 14:13 |
juergbi | tlater: but we might be missing proper API for it, so could need a bit of refactoring | 14:13 |
juergbi | or maybe you need it in a different way | 14:14 |
tlater | juergbi: Hm, iirc the artifact cache currently extracts the item and keeps a cached extracted artifact around, right? | 14:14 |
juergbi | yes, that hasn't been changed so far in OSTree->CAS | 14:14 |
tlater | Ah, nevermind, that shouldn't be a problem for list_artifacts | 14:14 |
tlater | juergbi: So, when I call cascache.extract, am I guaranteed to touch the mtime of the artifact I'm accessing? | 14:15 |
valentind | jonathanmaw, if you want you could make the top directory to be "usr" when it is a runtime. But not when it is an application. I would not mind fixing things in freedesktop SDK. If you have something else in mind, you have to explain it. | 14:15 |
juergbi | tlater: no, but maybe we should do that | 14:15 |
tlater | We definitely should ;) | 14:15 |
juergbi | pass update_mtime=True to resolve_ref() in extract() | 14:15 |
persia | Can it not be done with atime directly? | 14:16 |
*** leopi has joined #buildstream | 14:17 | |
tlater | persia: Playing with mtime is more stable for our purposes, I think | 14:17 |
juergbi | we most likely can't rely on atime being implicitly updated on the underlying filesystem as that's often disabled | 14:17 |
tlater | If we use atime we rely on CAS never touching artifacts, even when it works on related files. | 14:17 |
tlater | And that noatime on SSDs of course :) | 14:18 |
* persia rather wishes the response to solid state storage hadn't been "let's disable atime tracking" | 14:18 | |
valentind | jonathanmaw, flatpak build-export expects "usr" in case of runtime and "files" in case of application. But for us we always pass --files=files to flatpak build-export. | 14:18 |
juergbi | eh, that's not just a SSD thing | 14:18 |
juergbi | atime is also a bad idea on spinning rust | 14:18 |
juergbi | (when used for all files) | 14:18 |
gitlab-br-bot | buildstream: merge request (ps-install-linux->master: Fix 'main install' to be explicit that it is for Linux distros only) #535 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/535 | 14:19 |
jmac | jjardon: 535 is fine, can you rebase it so we can merge? | 14:19 |
persia | juergbi: I suppose. My memory of the defaults for most installs changing from tracking atime to ignoring atime was time-matched to people using SSDs (and +noatime was a common recommendation on early SSD usage documentation). I agree that for most files, atime isn't important enough to be worth the power/storage aging/etc. | 14:19 |
tlater | juergbi: If I want to *just* update the mtime and do nothing else, I suppose that resolve_ref method still works? | 14:19 |
gitlab-br-bot | buildstream: merge request (ps-install-linux->master: Fix 'main install' to be explicit that it is for Linux distros only) #535 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/535 | 14:20 |
tlater | Hm, I should probably have a look at documentation for this | 14:20 |
tlater | Is there something like that? | 14:20 |
jjardon | thanks jmac | 14:20 |
juergbi | tlater: it has no other side effects, if that's what you're asking | 14:20 |
tlater | I mostly wonder if it's the fastest way to do this. | 14:21 |
tlater | Though technically I don't need anything too fast here... | 14:21 |
juergbi | resolve_ref always reads the ref file, of course | 14:21 |
juergbi | which is pretty fast but not completely free | 14:21 |
jmac | jjardon: You may have to get someone else to do the actual merge - I've got the permissions, but I don't know how the merge process works on this project | 14:21 |
gitlab-br-bot | buildstream: merge request (tiagogomes/tarball_checkout->master: WIP Add support for dumping a tarball stream) #546 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/546 | 14:23 |
tlater | juergbi: Hmm, I guess keeping the number of helper methods low is worth that slight overhead | 14:25 |
jonathanmaw | valentind: ah, so what flatpak_image currently does is moves files from the artifact it build-depends on into /buildstream/install/files and creates a metadata file in /buildstream/install, and the significance of "files" is that's the directory passed to flatpak build-export | 14:26 |
juergbi | if it's not in the critical path, possibly | 14:26 |
gitlab-br-bot | buildstream: merge request (phil/203-BuildStream-crashes-when-dependency-tree-too-deep->master: Phil/203 build stream crashes when dependency tree too deep) #512 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/512 | 14:26 |
valentind | jonathanmaw, Yes. It is not documented. I did not expect it would end up in bst-external. | 14:27 |
valentind | jonathanmaw, The next step would be to create multiple subdirectories for every extension. And also run integration commands (for now we use a compose in front of it for that reason). | 14:28 |
jonathanmaw | valentind: yep, I've had an issue come up asking for integration commands to be run, so I'm looking into that now | 14:28 |
tlater | juergbi: Just so I don't keep distracting you every 5 minutes, any clues on where I should start to calculate the cache size? | 14:30 |
valentind | jonathanmaw, running integration commands correctly is difficult. So I think we should take the code from compose and make it available in the API for plugins. | 14:30 |
tlater | That's something the remote cache doesn't do iirc | 14:31 |
tlater | I could go with the naive ol' `du`, but surely CAS is smarter, right? | 14:32 |
juergbi | tlater: it isn't right now. contrary to popular belief, CAS doesn't solve world hunger ;) | 14:33 |
tlater | Aww | 14:33 |
juergbi | CAS is like OSTree in many ways for now | 14:33 |
* tlater will go back to his old implementation then | 14:33 | |
juergbi | but we could imprpove things easier in the future, of course | 14:33 |
juergbi | tlater: you could take a look at testutils/artifactshare.py _mock_statvfs() | 14:33 |
juergbi | but it's nothing smart | 14:33 |
tlater | Heh, yeah, not that much more useful than what I already have... | 14:34 |
tiagogomes | Where can I find any information about cas? | 14:34 |
tlater | Oh well, time to roll up my sleeves and try and merge ostree/cascache | 14:34 |
tlater | tiagogomes: finn just linked me this: https://docs.google.com/document/d/1AaGk7fOPByEvpAbqeXIyE8HX_A3_axxNnvroblTZ_6s/edit# | 14:34 |
tiagogomes | thanks | 14:35 |
tlater | Ah, link with the nicer reading mode: https://docs.google.com/document/d/1AaGk7fOPByEvpAbqeXIyE8HX_A3_axxNnvroblTZ_6s/view# | 14:35 |
finn | indeed | 14:35 |
finn | you can change your default settings to be view iirc | 14:36 |
finn | persia may have mentioned this | 14:36 |
gitlab-br-bot | buildstream: merge request (willsalmon/documentation_link->master: Adding a helpful link to the example) #533 changed state ("merged"): https://gitlab.com/BuildStream/buildstream/merge_requests/533 | 14:36 |
finn | though if it works on a link which directs you to edit, I'm not sure | 14:36 |
jonathanmaw | valentind: given earlier discussions with ssssam[m], I don't think the commands currently run in flatpak-integration are analogous to bst integration-commands, since buildstream's integration-commands are run as part of assembling a complete system, while flatpaks are individual packages that get installed, iiuc | 14:38 |
gitlab-br-bot | buildstream: merge request (ps-install-linux->master: Fix 'main install' to be explicit that it is for Linux distros only) #535 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/535 | 14:39 |
tiagogomes | Had people here wrote that document? | 14:42 |
finn | nope | 14:42 |
finn | as far as I'm aware | 14:42 |
valentind | jonathanmaw, We cannot run integration commands when flatpak launches. Also even if it was possible it would take forever. Flatpaks are not individual packages. They are a bundle coming with all dependencies. The main separation is /app and /usr. Of course there are extensions, but this is a detail. | 14:45 |
persia | finn: I used "suggest", rather than "view", but yes :) | 14:46 |
jonathanmaw | valentind: okay, so these integration commands need to be run during assembly, and a flatpak contains the app in /app and its runtime dependencies in /usr | 14:51 |
valentind | jonathanmaw, /usr is the "Platform" or "Sdk" used by the application. This is a separate image. All other dependencies of the applications are in /app. | 14:53 |
valentind | (you can switch from Platform to Sdk by using --devel in flatpak run) | 14:53 |
valentind | We make sure that integration commands do cross those boundaries (/app and /usr). And then everything is fine. | 14:54 |
valentind | So integration command in a dependency of an application that will go in /app should not write in /usr. | 14:54 |
*** cs_shadow has joined #buildstream | 14:55 | |
persia | valentind: Why? | 14:55 |
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 | 14:55 |
valentind | For extensions, we do not use integration commands. | 14:55 |
persia | Ah, local project choice. That makes sense. | 14:55 |
tiagogomes | Anyone fancy having a look at https://gitlab.com/BuildStream/buildstream/merge_requests/546 | 14:55 |
valentind | persia, because we will have two images. The runtime will not know about the application's integration commands. | 14:56 |
valentind | jonathanmaw, Of course, this is not the most beautiful thing to have. But this is the pragmatic approach when dealing with flatpak. Launching flatpak application should have little overhead. | 14:57 |
persia | valentind: Right. I confused "cannot" as applied to BuildStream generally with "cannot" as applied to a specific applicaiton of BuildStream for a specific system. My apologies for confusion. | 14:57 |
valentind | jonathanmaw, there are integration commands in flatpak, they are run the first time and cached. But they are static. For example ldconfig. | 14:58 |
valentind | Integration commands cannot be added. | 14:58 |
valentind | jonathanmaw, Must go now. Will answer any other questions later or tomorrow. | 15:00 |
jonathanmaw | ok, thanks valentind | 15:00 |
jonathanmaw | I'm not sure I fully understand flatpak, but I'll see what I can puzzle out | 15:00 |
juergbi | Nexus: please respect the 72 char line limit in commit messages | 15:04 |
juergbi | you also forgot to update the help text, I'll push a fix for that | 15:04 |
Nexus | eurk | 15:05 |
Nexus | i wonder why that didn't come up on my grep | 15:06 |
tiagogomes | Can I get CI on a branch without having to create a MR | 15:06 |
Nexus | CI/CD -> run pipeline -> create for (select branch) | 15:06 |
gitlab-br-bot | buildstream: merge request (ps-install-linux->master: Fix 'main install' to be explicit that it is for Linux distros only) #535 changed state ("merged"): https://gitlab.com/BuildStream/buildstream/merge_requests/535 | 15:07 |
Nexus | juergbi: i can do the fix if you want | 15:07 |
juergbi | already pushed | 15:08 |
jmac | I wasn't aware we were observing a 72 character limit, apologies if my commits have been longer than that | 15:08 |
juergbi | not sure whether we mention it in HACKING but it's a very common rule, see also https://chris.beams.io/posts/git-commit/ | 15:09 |
tlater | I also wasn't aware, just following best practives. Is this documented in our readme? | 15:09 |
tlater | Ah | 15:09 |
jmac | It's not in HACKING | 15:09 |
tlater | We should probably mention it in HACKING if we want to enforce it, juergbi | 15:10 |
juergbi | the reason is that it avoids line breaks in git log for 80 character terminals after the implicit tab (8 space) indentation | 15:10 |
juergbi | yes, makes sense | 15:10 |
*** noisecell has quit IRC | 15:10 | |
juergbi | we don't follow the 50 character rule on the subject line, that's just too constraining | 15:11 |
cs_shadow | tlater: hey! If you find time, please have a look at https://gitlab.com/BuildStream/buildstream/merge_requests/531 that'll help us move forward with the image deprecation | 15:11 |
tlater | cs_shadow: Scheduled for next time I run the test suite :) | 15:11 |
cs_shadow | thanks! | 15:12 |
tpollard | is there a public artifact server I can point at? wayland upstream seems to be down | 15:18 |
jjardon | juergbi: tristan can you please tag the latest commit just before introducing CAS and removing ostree cache server support? | 15:18 |
jjardon | Is it maybe 9067e269a9f2866e659ef33a69aad72b01cb6633 ? (It is difficult to know as this project doesn't use merge commits) | 15:19 |
juergbi | jjardon: 4c6512d6f6e4defbddebab56a19e5e7f9e50c20c is right before the merge | 15:20 |
jjardon | juergbi: thanks! | 15:21 |
* Nexus wasn't aware of the 72 chat limit | 15:21 | |
Nexus | also, who uses 80 char terminals? | 15:22 |
juergbi | Nexus: I suggest you to read https://chris.beams.io/posts/git-commit/ if you haven't yet | 15:22 |
Nexus | i have not | 15:22 |
jmac | There's lots of arguments for and against line length limits, here is probably not the place to discuss them | 15:23 |
juergbi | indeed, it's not much about actual 80 char terminals anymore | 15:23 |
juergbi | mails have more or less the same wrapping | 15:23 |
jjardon | juergbi: pip tried to install protobuf, grpcio using that commit; is that expected? | 15:23 |
tiagogomes | I skimmed through that google doc but I still don't know the internals of the cas | 15:23 |
Nexus | and i have issues because i disagree with his first point xD | 15:24 |
Nexus | i prefer the top example of the logs | 15:24 |
Nexus | if anything, some are a bit short | 15:24 |
jmac | Well, so do I, but it's no bother to limit to 72 | 15:24 |
juergbi | jjardon: no, it's not. the dependencies are introduced later | 15:24 |
Nexus | no it's not, i'll just multi line it | 15:24 |
gitlab-br-bot | buildstream: merge request (chandan/use-testsuite-fedora->master: .gitlab-ci.yml: Use testsuite images for running tests) #531 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/531 | 15:24 |
Nexus | unfortunately, the file path was already 30 something chars long | 15:25 |
finn | Can someone with experience of unittests check I'm on the right track here. I want to unittest this function: https://gitlab.com/BuildGrid/buildgrid/blob/finn/unittests/buildgrid/server/execution/execution_instance.py#L37 | 15:25 |
finn | This is my unittest: https://gitlab.com/BuildGrid/buildgrid/blob/finn/unittests/test/execution/execution_instance.py | 15:25 |
tlater | Nexus: I use 80 character terminals - in fact, my whole UI is designed around having space for multiple 80 character terminals next to each other ;) | 15:26 |
juergbi | Nexus: that's why we don't normally keep the buildstream/ prefix in the summary line as it's not useful and eats characters | 15:26 |
Nexus | juergbi: but "tests/frontend/buildcheckout.py" kindof needs the tests at the start | 15:26 |
Nexus | and thats the long one | 15:26 |
jjardon | juergbi: oops, just me being stupid, sorry for the noise | 15:27 |
juergbi | Nexus: yes, that does, but the main point of the summary is to quickly determine the topic/area. details can be in the body of the commit message | 15:27 |
juergbi | short summary lines are very useful when looking at tons of commits | 15:27 |
tlater | Nexus: You can use 'tests/.../buildcheckout.py' ;) | 15:27 |
Nexus | tlater: that doesn't help if you don't know where it is | 15:27 |
Nexus | if i saw that, i'd have no clue where to look | 15:27 |
tlater | It tells you enough to find it easily IMO | 15:27 |
Kinnison | It's pretty easy to find if it's in the commit anyway | 15:28 |
juergbi | I wouldn't use ... unless there is really no space for it | 15:28 |
juergbi | but we're getting into details / bikeshedding | 15:28 |
Kinnison | i.e I rarely include file paths in commit messages, unless they're unrelated to the files touched by the commit | 15:28 |
Nexus | Kinnison: i agree, but thats also an arguement for not putting hte file name before everything | 15:28 |
Nexus | i was told to | 15:28 |
Kinnison | I'd argue put topic area in the front, not filename, but I'm not yet in a position to critique the policy | 15:29 |
juergbi | the basic area is very useful in the summary line | 15:29 |
Kinnison | e.g. tests: validate checkouts properly | 15:29 |
* Nexus would prefer that | 15:29 | |
juergbi | for tests that would likely suffice | 15:30 |
Nexus | juergbi: s the character limit, is that for anything, or just the summary line? | 15:30 |
juergbi | the 72 limit is for the whole message | 15:30 |
Kinnison | Ideally for the whole thing | 15:30 |
Nexus | wut | 15:30 |
juergbi | like mails | 15:30 |
juergbi | (copy-pasted stack traces etc could be exempted) | 15:30 |
Nexus | that's way too short imo | 15:30 |
Kinnison | Summary line should be around 55 chars or less if poss. | 15:30 |
tlater | Nexus: You can wrap lines in the summary | 15:30 |
tlater | So the summary technically has no limit | 15:31 |
* Kinnison nods. word wrapping is a thing | 15:31 | |
juergbi | your editor probably can do it automatically... | 15:31 |
tlater | M-q in emacs :) | 15:31 |
juergbi | vim handles commit messages properly as well ;) | 15:31 |
juergbi | without M-q ;) | 15:32 |
Kinnison | juergbi: but with more ESC ESC OMG ARGH | 15:32 |
tlater | Heh, emacs does too, but in general M-q wraps ;p | 15:32 |
Nexus | ok, i was misunderstanding what was being said | 15:32 |
Kinnison | My favourite vim command is 'gqip' | 15:32 |
* Kinnison has no idea why it works, but it does | 15:32 | |
jmac | Yes, it read to me like the whole commit message should be 72 characters :) | 15:33 |
Nexus | ok thats fine, i'll just multi-line my commits in future | 15:33 |
Nexus | same jmac | 15:33 |
tlater | Nexus: Note that it's common practice to have a separating line between commit summary (72 chars) and the commit body. gitlab/github parse that correctly. | 15:34 |
Kinnison | OOI what's the s-o-b policy on buildstream? | 15:34 |
Nexus | tlater: i usually do that anyway | 15:34 |
Kinnison | tlater: I thought summary should be 55 or less, (maybe less than that anyway) | 15:35 |
tlater | Kinnison: *should* be, but we have a hard limit of 72 from what I gather. | 15:35 |
Kinnison | I see | 15:35 |
Phil | What's an s-o-b? | 15:35 |
Kinnison | Signed-off-by | 15:35 |
Kinnison | but lazier | 15:35 |
tlater | Phil: If tristan rejects your MR don't sob too loudly. | 15:35 |
* Phil can sob quietly | 15:36 | |
Nexus | i'll try do that in the future, i didn't realise, since i've seen other commits with a longer summary line be merged | 15:36 |
juergbi | Nexus: btw: just to be clear, you can't wrap the summary on multiple lines. the line wrapping for long lines is just for the commit body | 15:38 |
juergbi | (tools only recognize the first line as summary and the second line should really always be an empty line) | 15:39 |
juergbi | Kinnison: we don't currently use s-o-b | 15:39 |
Kinnison | juergbi: okay, thanks | 15:39 |
tlater | juergbi: I think I may have run into a bug with `cascache.prune` | 15:40 |
tlater | Let me grab a stacktrace... | 15:40 |
juergbi | Nexus: I just noticed that you have a very odd author email address in your latest commits. the commit email address is fine, though | 15:40 |
Nexus | huh, i wonder why | 15:41 |
Nexus | i changed nothing | 15:41 |
juergbi | for the list_dir_contents commit it was the other way round. author email fine, committer email bad | 15:41 |
juergbi | different laptops/homedirs, one without proper git email config? | 15:42 |
Nexus | same laptop | 15:42 |
Kinnison | chroots? | 15:42 |
Nexus | nope | 15:42 |
Kinnison | Oddness | 15:42 |
Nexus | indeed | 15:42 |
tlater | juergbi: Somehow one of the objects disappear in the middle of a prune operation: https://hastebin.com/adasagocoq | 15:42 |
tpollard | does http://bst-artifacts-gnome.codethink.co.uk/artifacts need a server-cert? | 15:43 |
tpollard | I'm trying to build gtk+-3.bst from gnome-build-meta, but it's failing at bringing in the wayland tarball directly from freedesktop | 15:44 |
Nexus | changed my email, not sure why it changed | 15:46 |
juergbi | tpollard: bst master just switched caching backend from OSTree to CAS. The server doesn't require server-cert but it's OSTree, so you can't use the very latest bst client if you want to use the cache | 15:47 |
tpollard | juergbi: is there a specific tag I can drop back to that would allow that then? | 15:49 |
juergbi | no tag at this point (except for the latest dev release 1.1.3), but you can use 4c6512d6f6 | 15:49 |
tlater | juergbi: |: I don't think this is caused by anything strange in my code - I assume the expected result in that case is not to register anything as reachable? | 15:51 |
tpollard | juergbi: cheers juergbi | 15:51 |
tlater | I.e., simply a pass if the file is missing? | 15:51 |
juergbi | tlater: is there concurrent pruning in the test? | 15:52 |
tlater | It should not be concurrent | 15:52 |
tlater | Hm, I'll make sure, but I really doubt it. | 15:53 |
juergbi | pretty odd, then | 15:53 |
juergbi | proper support for concurrent pruning etc. is anyway in the plans but not sure how this can happen without | 15:54 |
juergbi | we can simply skip it on FileNotFoundError but it would be nice to understand the issue | 15:55 |
tlater | Agreed... I'll try and dig a bit deeper | 15:56 |
finn | in case you missed this, it's a very short piece of code | 16:03 |
finn | <finn> Can someone with experience of unittests check I'm on the right track here. I want to unittest this function: https://gitlab.com/BuildGrid/buildgrid/blob/finn/unittests/buildgrid/server/execution/execution_instance.py#L37 | 16:03 |
finn | <finn> This is my unittest: https://gitlab.com/BuildGrid/buildgrid/blob/finn/unittests/test/execution/execution_instance.py | 16:03 |
tlater | I'm fairly sure there's something wrong with this remove implementation... It seems to remove more than just the element I give it. | 16:10 |
*** noisecell has joined #buildstream | 16:11 | |
*** noisecell has quit IRC | 16:11 | |
*** noisecell has joined #buildstream | 16:11 | |
tlater | That would explain why there's suddenly a missing object | 16:12 |
* tlater wonders if _reachable_refs_dir doesn't do a shallow copy the way the code expects it to | 16:16 | |
tiagogomes | whoever added --deps to bst checkout forgot to update the man page | 16:30 |
cs_shadow | I think man pages are updated periodically, usually just before a releases | 16:31 |
tiagogomes | So are they automatic generated? | 16:35 |
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 | 16:37 |
cs_shadow | tiagogomes: yes, click-man but there's a pesky issue, documented in https://gitlab.com/BuildStream/buildstream/issues/8, that won't allow us to do that as part of the build process | 16:40 |
cs_shadow | using click-man* | 16:40 |
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 | 16: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 | 16:53 |
tlater | Hm, so it looks like that the missing object is actually significant, and contains parts of the other refs | 16:54 |
tlater | So when I simply ignore that error, the entire | 16:54 |
tlater | cache is pruned | 16:54 |
tlater | But why is that object missing? |: | 16:54 |
tlater | Certainly no concurrency happening, it just goes missing after the first call to remove() | 16:54 |
tlater | Gah | 16:54 |
tlater | It's going to be a fun night | 16:55 |
tlater | Is it because directories are not marked as reachable? | 16:56 |
tlater | That would explain a lot... | 16:56 |
tlater | If I add directories as reachable suddenly we prune everything... | 16:57 |
tlater | Hmmm | 16:57 |
*** bethw has quit IRC | 17:00 | |
*** jonathanmaw has quit IRC | 17:02 | |
*** mablanch has quit IRC | 17:11 | |
*** jmac has quit IRC | 17:11 | |
*** benbrown has quit IRC | 17:11 | |
*** paulsherwood has quit IRC | 17:11 | |
*** Nexus has quit IRC | 17:11 | |
*** finn has quit IRC | 17:13 | |
*** benbrown has joined #buildstream | 17:14 | |
*** jmac has joined #buildstream | 17:15 | |
*** toscalix has joined #buildstream | 17:24 | |
juergbi | tlater: might there be concurrency between remove/prune and adding objects or is there no concurrency at all? | 17:24 |
tlater | The test itself can't even have any concurrency | 17:25 |
tlater | So, no, there's nothing suchlike going on | 17:25 |
tlater | I suspect that directories aren't considered "reachable" | 17:25 |
tlater | But I'm note 100% certain what "directories" are in this context | 17:26 |
tlater | Just from debugging it seems that they are being removed unconditionally during a prune, whereas they shouldn't be. | 17:26 |
juergbi | oh, right, that seems to be completely missing | 17:26 |
* juergbi wonders why the tests work | 17:26 | |
persia | While I'm uncertain of the specific implementation you are working against, my general experience with object stores is that "directories" don't actually exist: they are accidental side effects of naming objects with pathnames. | 17:27 |
*** tlater has left #buildstream | 17:27 | |
jmac | Directories definitely do exist in the CAS server | 17:28 |
*** tlater has joined #buildstream | 17:28 | |
juergbi | tlater has pruned himself? | 17:28 |
persia | (as opposed to many classic filesystems, wherein path information is actively stored in special files called "directories", such that reaching a path requires accessing several objects, each with indirect pointers) | 17:28 |
tlater | Heh | 17:28 |
juergbi | tlater: try this? https://pastebin.com/auQeYzRV | 17:28 |
tlater | Ah, that might work! | 17:28 |
juergbi | persia: jmac is correct, git/ostree/CAS all have directory/tree objects | 17:28 |
persia | In the case of git, while there is a "tree" object, one cannot represent a filesystem directory in a git store, except as a side effect. The internal implementation and the user interface differ in this regard. | 17:29 |
tlater | persia: Hence my confusion about the terminology here | 17:30 |
persia | That said, I have every confidence that jmac is indeed correct for CAS. | 17:30 |
juergbi | persia: the high level doesn't support it but the data model does | 17:30 |
tlater | It seems to actually be something very directory-like in CAS, though :) | 17:30 |
tlater | Yay! | 17:30 |
tlater | It works juergbi :) | 17:30 |
tlater | Now to remove a lot of debug statements... | 17:30 |
juergbi | great. that's an embarrassing bug | 17:30 |
* juergbi still wonders why the expiry tests work | 17:31 | |
tlater | Heh, it happens. | 17:31 |
tlater | The bug only triggers after the second call to prune() | 17:31 |
tlater | Because before then the directories still exist | 17:31 |
tlater | The expiry tests probably only ever prune once | 17:31 |
tlater | But my new ones are a bit more thorough :) | 17:32 |
*** Prince781 has joined #buildstream | 17:32 | |
juergbi | ok. i'll push the fix after CI passes but won't add new tests in anticipation of your branch | 17:33 |
gitlab-br-bot | buildstream: merge request (juerg/cas-prune->master: _artifactcache/cascache.py: Fix prune()) #547 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/547 | 17:36 |
*** Prince781 has quit IRC | 17:52 | |
gitlab-br-bot | buildstream: merge request (135-expire-artifacts-in-local-cache->master: WIP: Resolve "Expire artifacts in local cache") #347 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/347 | 18:01 |
tlater | Hm, the new CAS server needs more permissions to run in docker, I think | 18:02 |
tlater | It refuses to let me run the tests locally, at any rate | 18:02 |
* tlater tries gitlab CI | 18:02 | |
juergbi | hm, CAS server definitely works in gitlab CI | 18:05 |
tlater | bst-here/bst-test don't like it, but their settings certainly are different | 18:05 |
juergbi | it does require localhost TCP. for the old server I think we used pipes | 18:05 |
juergbi | but I wouldn't expect this to be an issue as it doesn't need privileged ports) | 18:05 |
tlater | I think it's a problem with docker network configuration | 18:06 |
tlater | It seems to run fine in CI though :) | 18:06 |
* tlater crosses his fingers | 18:06 | |
juergbi | have you already completed the rebasing/porting or is there still much to do? | 18:07 |
tlater | Iff the CI doesn't bring up anything I think I'm done | 18:07 |
juergbi | great | 18:08 |
tlater | I might have a look at messages later tonight, but rebasing looks to have gone smoothly :) | 18:08 |
tlater | Thanks for the help, juergbi | 18:08 |
juergbi | the least I could do after introducing the bug ;) | 18:09 |
tlater | dopefish | 18:12 |
tlater | Err | 18:12 |
tlater | Ignore me | 18:12 |
*** Prince781 has joined #buildstream | 18:19 | |
gitlab-br-bot | buildstream: merge request (135-expire-artifacts-in-local-cache->master: WIP: Resolve "Expire artifacts in local cache") #347 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/347 | 18:25 |
*** finn has joined #buildstream | 18:30 | |
gitlab-br-bot | buildstream: merge request (juerg/cas-prune->master: _artifactcache/cascache.py: Fix prune()) #547 changed state ("merged"): https://gitlab.com/BuildStream/buildstream/merge_requests/547 | 18:50 |
*** ernestask has quit IRC | 18:52 | |
*** Prince781 has quit IRC | 18:53 | |
*** leopi has quit IRC | 19:10 | |
toscalix | GUADEC call for feedback sent to the mailing list. | 19:45 |
toscalix | Wednesday BoF minutes too | 19:46 |
*** Prince781 has joined #buildstream | 19:52 | |
*** toscalix has quit IRC | 20:15 | |
*** toscalix has joined #buildstream | 20:20 | |
*** toscalix has quit IRC | 20:22 | |
*** tristan has quit IRC | 20:42 | |
*** jcampbell has quit IRC | 21:00 | |
*** gnurlu1 has quit IRC | 21:06 | |
gitlab-br-bot | buildstream: merge request (edbaunton/remote-source->master: Add remote source plugin) #541 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/541 | 21:48 |
*** finn has quit IRC | 22:09 | |
*** Prince781 has quit IRC | 22:20 | |
*** slaf has quit IRC | 22:35 | |
*** slaf has joined #buildstream | 22:35 | |
*** slaf has joined #buildstream | 22:35 | |
*** slaf has joined #buildstream | 22:36 | |
*** slaf has joined #buildstream | 22:36 | |
*** slaf has joined #buildstream | 22:36 | |
*** slaf has joined #buildstream | 22:36 | |
gitlab-br-bot | buildstream: merge request (135-expire-artifacts-in-local-cache->master: WIP: Resolve "Expire artifacts in local cache") #347 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/347 | 23:24 |
gitlab-br-bot | buildstream: merge request (135-expire-artifacts-in-local-cache->master: WIP: Resolve "Expire artifacts in local cache") #347 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/347 | 23:41 |
gitlab-br-bot | buildstream: merge request (135-expire-artifacts-in-local-cache->master: WIP: Resolve "Expire artifacts in local cache") #347 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/347 | 23:55 |
gitlab-br-bot | buildstream: merge request (135-expire-artifacts-in-local-cache->master: WIP: Resolve "Expire artifacts in local cache") #347 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/347 | 23:58 |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!