*** tristan has quit IRC | 05:04 | |
*** tristan has joined #buildstream | 05:27 | |
*** Prince781 has joined #buildstream | 05:30 | |
*** Prince781 has quit IRC | 06:30 | |
*** toscalix has joined #buildstream | 07:49 | |
*** toscalix has quit IRC | 07:50 | |
*** tristan has quit IRC | 07:56 | |
*** tristan has joined #buildstream | 08:28 | |
*** valentind has joined #buildstream | 09:08 | |
jennis | \o/ | 09:10 |
---|---|---|
*** jonathanmaw has joined #buildstream | 09:12 | |
*** dominic has joined #buildstream | 09:43 | |
tlater | Hm, I think I found a bug | 09:51 |
* tlater tests if he can build a workspaced element after it fails on master | 09:52 | |
*** tiago has joined #buildstream | 09:55 | |
tlater | Nevermind, looks to be my branch after all :\ | 10:00 |
*** toscalix has joined #buildstream | 10:01 | |
*** toscalix has quit IRC | 10:07 | |
tristan | juergbi, I'm in the middle of an aggressive refactor, which I hoped I would get done in a day, maybe I can, but shaking the tree, stuff is falling out | 10:13 |
tristan | juergbi, tlater, both of you actually; I wonder about how solid our logic is for setting up the build + track plan | 10:14 |
juergbi | do you have specific concerns? | 10:14 |
tristan | Before we added `--except`, Pipeline.build() with enabled tracking was quite straight forward | 10:14 |
tristan | One case I think is broken here, is if we track dependencies of cached build-only dependencies | 10:15 |
tristan | There will be cases where those intermediate elements wont get re-added to the build queue | 10:15 |
juergbi | tristan: hm, we don't drop elements when moving from queue to queue, do we? | 10:17 |
tristan | No, not that I know of | 10:18 |
juergbi | Then I don't know what you mean with re-adding to the build queue | 10:18 |
tristan | But lets say I decide to track a dependency of a build-only dependency, in a build session | 10:18 |
*** aday has joined #buildstream | 10:18 | |
tristan | First, we will calculate a build plan; which will exclude the build-only dependency | 10:18 |
tristan | Then, we will add in the build dependency of the build-only dependency to the track queue | 10:19 |
tristan | juergbi, finally, the build plan has never been instructed to build the intermediate element | 10:19 |
tlater | Oof, yeah, that sounds like it will get lost | 10:19 |
juergbi | ah, the intermediate element, right, missed that part | 10:20 |
tristan | This needs restructuring, or things are getting unlivable | 10:20 |
tristan | My quick refactor is trying to add PipelineSelection() object | 10:21 |
juergbi | as a quick fix we could skip the cached build-only dependency exclusion if --track FOO is used | 10:21 |
tristan | And not making some implicit assumption that the --except list has to do with a specific selection | 10:21 |
tristan | juergbi, indeed | 10:23 |
tristan | I had to read that 5 times to understand it though, and that is the real problem :) | 10:23 |
tlater | Yep, that code is definitely to complex atm | 10:24 |
tristan | juergbi, basically you mean - build + track *implies* `--all` argument | 10:24 |
juergbi | i didn't even know that we directly skip queues in some cases | 10:24 |
jmac | When running tests in unix mode I find it's often leaving mounts behind | 10:24 |
tristan | jmac, somebody filed a bug about that... like yesterday. | 10:24 |
tlater | jmac: That happens mostly when there's an exception during the build | 10:25 |
jmac | e.g. I can't umount /var/run/buildstream/tmpopw92h65/proc as it says it's always busy, although there aren't any bst procresses running | 10:25 |
jmac | Yeah, found it #298 | 10:25 |
jmac | But they can't be manually unmounted | 10:26 |
tlater | Yeah, I never figured out *why* in some cases - I suspect you need to recursively unmount | 10:26 |
tristan | jmac, fallback unix platform *implies* root | 10:26 |
tristan | Oh that | 10:26 |
tristan | Yes, I've encountered that | 10:26 |
skullman | `umount -l` will work | 10:26 |
tlater | Be careful with these, as it's your actual /proc mounted to that dir | 10:27 |
skullman | hopefully since it's a virtual filesystem it's not going to cause problems on shutdown, since the kernel will be able to kill whatever is holding it | 10:27 |
juergbi | tristan: implying --all should do the trick, yes. not always optimal but should work | 10:28 |
tristan | juergbi, yeah I have to prioritize, it's driving me mad hehe :) | 10:28 |
juergbi | of course | 10:28 |
tristan | juergbi, Ultimately, I think we need these selection objects, and ways to composite them together; splitting that code out entirely from the `build`, `fetch`, `track` toplevel entry points | 10:29 |
tristan | but I can't get there today, as a step I wanted to "just do" before landing project.refs :-( | 10:29 |
juergbi | yes, let's do it separately | 10:30 |
tristan | yes, the current state was so annoying that I wanted it done first; but that's proving to be too much | 10:30 |
dominic | I have https://pastebin.com/DsqECx4k and https://pastebin.com/2X21jkA5 as possible outputs, which one do people prefer? | 10:36 |
skullman | I'd prefer the second | 10:37 |
jennis | As do I | 10:37 |
dominic | OK, I do too. Will go with that one then | 10:40 |
jennis | However for the first one, I like the splitting with ,, on lines 7 and 14 | 10:40 |
jennis | Is it possible to incorporate something like that into the second? | 10:40 |
dominic | hmm, I'll try and see what happens. Should be able to though | 10:41 |
gitlab-br-bot | buildstream: merge request (jmac/build-uid-2->master: WIP: Sandbox configuration key to specify uid/gid for sandbox) #318 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/318 | 10:43 |
gitlab-br-bot | buildstream: merge request (215-workspace-builds-might-not-rebuild-correctly-when-dependecies-are-updated->master: WIP: Resolve "Workspace builds might not rebuild correctly when dependecies are updated") #315 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/315 | 10:52 |
gitlab-br-bot | buildstream: merge request (215-workspace-builds-might-not-rebuild-correctly-when-dependecies-are-updated->master: WIP: Resolve "Workspace builds might not rebuild correctly when dependecies are updated") #315 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/315 | 10:54 |
*** valentind has quit IRC | 10:57 | |
*** dominic has quit IRC | 11:16 | |
*** dominic has joined #buildstream | 11:17 | |
gitlab-br-bot | buildstream: merge request (jmac/build-uid-2->master: WIP: Sandbox configuration key to specify uid/gid for sandbox) #318 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/318 | 11:40 |
tristan | juergbi, ok so I'm aiming low and will want to expand on this after, along with some brutal refactoring... I have the error in place for cross-junction tracking without project.refs, it is a bit simple; it turns out very difficult to collect the list of elements I would want to report | 11:59 |
tristan | Unless I were to report *all* of them, which can be too big of a list | 11:59 |
gitlab-br-bot | buildstream: merge request (jmac/build-uid-2->master: WIP: Sandbox configuration key to specify uid/gid for sandbox) #318 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/318 | 12:00 |
tristan | juergbi, before I write up docs and nail in the final test cases; I wonder about .... wait for it... NAMES | 12:00 |
tristan | haha | 12:00 |
tristan | particularly, I've been working with a project.conf switch called 'separate-source-refs' | 12:00 |
tristan | I'm uncomfortable with that name... | 12:01 |
tristan | centralized-refs maybe... because source refs in a way actually become less "separate" than they were before, now they are all in one file *together* | 12:01 |
juergbi | hm | 12:02 |
juergbi | is it or will it be possible to specify a filename. or will this remain a boolean? | 12:02 |
tristan | Good point, it's currently not | 12:03 |
juergbi | something like refs-filename could be an option if it was a filename option | 12:03 |
tristan | It may be good to support that *first* for the sake of having one configuration for it | 12:03 |
tristan | given that it will be very easy to implement that | 12:03 |
juergbi | or refs-path | 12:03 |
tristan | What are we really going to accomplish by making that configurable, I wonder ? | 12:05 |
gitlab-br-bot | buildstream: merge request (215-workspace-builds-might-not-rebuild-correctly-when-dependecies-are-updated->master: WIP: Resolve "Workspace builds might not rebuild correctly when dependecies are updated") #315 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/315 | 12:05 |
tristan | You can maintain two differently versioned builds in the same branch | 12:05 |
juergbi | tristan: avoid git diff? | 12:05 |
gitlab-br-bot | buildstream: merge request (215-workspace-builds-might-not-rebuild-correctly-when-dependecies-are-updated->master: WIP: Resolve "Workspace builds might not rebuild correctly when dependecies are updated") #315 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/315 | 12:05 |
tristan | juergbi, I mean, allowing the user to decide on filename | 12:05 |
juergbi | actually, it'd have to be a CLI/user config, hm | 12:06 |
gitlab-br-bot | buildstream: merge request (215-workspace-builds-might-not-rebuild-correctly-when-dependecies-are-updated->master: WIP: Resolve "Workspace builds might not rebuild correctly when dependecies are updated") #315 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/315 | 12:06 |
tristan | There is something *nice* about forcing `project.refs` beside `project.conf`; seems easier knowledge to swallow | 12:06 |
gitlab-br-bot | buildstream: merge request (215-workspace-builds-might-not-rebuild-correctly-when-dependecies-are-updated->master: WIP: Resolve "Workspace builds might not rebuild correctly when dependecies are updated") #315 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/315 | 12:06 |
tristan | indeed, as a CLI switch it makes more sense to me | 12:06 |
juergbi | the only reason i suggested customizable filename was to avoid the issue where tracking makes the git tree dirty | 12:07 |
juergbi | but this doesn't really make sense in project.conf | 12:07 |
juergbi | although you could theoretically override it in the project-specific user config but it's a bit odd | 12:07 |
tristan | Yeah it's weird, and doesnt solve the naming issue of "Does this project use project.refs or not" | 12:08 |
tristan | It could be `use-project-refs`, but that... also sounds bad | 12:08 |
juergbi | ok, let's keep it a boolean | 12:08 |
juergbi | or invert it: embed-refs: False? | 12:10 |
gitlab-br-bot | buildstream: merge request (215-workspace-builds-might-not-rebuild-correctly-when-dependecies-are-updated->master: WIP: Resolve "Workspace builds might not rebuild correctly when dependecies are updated") #315 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/315 | 12:10 |
juergbi | or something like an enum: ref-storage: embedded/external | 12:10 |
tristan | I like 'inline' better than 'embedded' I think | 12:11 |
tristan | inline-refs: False... ref-storage: inline / centralized | 12:12 |
juergbi | inline is fine with me | 12:12 |
juergbi | instead of centralized we could also call that value 'project.refs' | 12:13 |
tristan | sounds good to me | 12:14 |
tristan | that will also imply default data/projectconfig.yaml will have a ref-storage setting in it (which I like for documentation purposes) | 12:15 |
tristan | I guess that would have been true for any default | 12:15 |
tlater | Do we want to disable incremental builds on the Unix platform to fix the updated dependencies bug? | 12:23 |
tlater | Or should we try to implement a terribly slow diff for tarfiles? | 12:23 |
juergbi | i would disable it | 12:27 |
juergbi | we might even decide to replace tarcache with the Google CAS based artifact cache | 12:27 |
juergbi | where we can do this efficiently | 12:27 |
tristan | juergbi, here is another place where; it may make sense to have the ostree/tar details really tied to push/pull ? | 12:27 |
tristan | and rely on the extract dir more extensively instead ? | 12:28 |
juergbi | extract dir would be much slower for diff | 12:28 |
juergbi | merkle tree diff is efficient | 12:28 |
tristan | true, the ostree cache will be much better at that | 12:28 |
juergbi | i.e., it's actually the other way round, a reason for relying on real artifact cache | 12:29 |
tristan | we may need merkle tree understanding as a first class citizen, though | 12:29 |
juergbi | yes, in case we decided to replace tarcache with Google CAS, all artifact caches would be merkle based | 12:30 |
juergbi | and we could add some internal API for this | 12:30 |
tristan | at some point; we call out to the build farm APIs with merkle trees | 12:30 |
juergbi | yes but build farm expects google CAS merkle trees, not arbitrary ones | 12:30 |
tristan | and having CAS as a possibility for an artifact cache, may be later than that point, if it ever comes | 12:30 |
juergbi | CAS as artifact cache is actually a step before remote execution | 12:31 |
juergbi | it's a prerequisite | 12:31 |
tristan | Hmmm :-/ | 12:31 |
juergbi | however, we don't have to switch to it for local execution, of course. that's to be decided | 12:32 |
juergbi | what's your concern here? | 12:32 |
tristan | One is of course, we *cannot* have remote execution without locally using a CAS ? | 12:34 |
tristan | Will we end up having many branchy ways of doing things, because we did not first make our own merkle tree object that we use extensively throughout the core ? | 12:35 |
tristan | I'm definitely foggy in this area of planning, though | 12:35 |
juergbi | tristan: for 'virtual staging' of dependency artifacts, we need at least some local CAS functionality | 12:36 |
juergbi | i've already implemented this in a branch | 12:36 |
juergbi | the current plan is to have this as a separate artifact cache implementation. only one of ostree or CAS artifact cache backend would be enabled in a single session | 12:37 |
juergbi | supporting both at the same time would be tricky. but one at a time should be straight forward, not increasing branching | 12:37 |
juergbi | (except for the place where we choose the artifact cache implementation, still not quite sure about that) | 12:38 |
tristan | So... the ArtifactCache abstraction gains a bit more responsibility, in terms of staging ? | 12:38 |
tristan | And then you have it done virtually in a farm with CAS, and locally with ostree ? | 12:38 |
juergbi | my current plan is to expose an API to work with a directory abstraction | 12:39 |
juergbi | which can be real local directories (regular staging) or merkle trees (virtual staging) | 12:39 |
juergbi | i.e., artifact cache itself wouldn't really know anything about staging per se | 12:39 |
tristan | Well... it's gonna be interesting :) | 12:40 |
tristan | We'd better clean house first | 12:40 |
juergbi | the current branch only deals with CAS support with local execution. so that's one of the next steps | 12:40 |
juergbi | also need to design API for the single script generation aspect. will probably be a proposal on the list | 12:41 |
juergbi | yes, there are quite a few pieces that have to be put together to get remote execution properly working | 12:42 |
gitlab-br-bot | buildstream: merge request (215-workspace-builds-might-not-rebuild-correctly-when-dependecies-are-updated->master: WIP: Resolve "Workspace builds might not rebuild correctly when dependecies are updated") #315 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/315 | 12:46 |
tristan | tlater, jennis; does the new linting check for circular imports ? | 13:15 |
tlater | tristan: Not currently, we have circular imports and refactoring those was too much work for the time being | 13:16 |
tlater | If you'd like to, you can enable it for a branch by modifying .pylintrc | 13:16 |
tristan | Hmm, that is a problem | 13:16 |
tristan | one which we would have caught had we fixed https://gitlab.com/BuildStream/buildstream/issues/146 by now | 13:17 |
tristan | This means that none of the current developers use python 3.4 anymore | 13:17 |
tristan | I know jonathanmaw was using it for a while | 13:18 |
gitlab-br-bot | buildstream: issue #301 ("Fix circular imports") changed state ("opened") https://gitlab.com/BuildStream/buildstream/issues/301 | 13:22 |
*** xjuan has joined #buildstream | 13:27 | |
*** mcatanzaro has joined #buildstream | 13:33 | |
gitlab-br-bot | buildstream: merge request (215-workspace-builds-might-not-rebuild-correctly-when-dependecies-are-updated->master: WIP: Resolve "Workspace builds might not rebuild correctly when dependecies are updated") #315 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/315 | 13:37 |
gitlab-br-bot | buildstream: merge request (215-workspace-builds-might-not-rebuild-correctly-when-dependecies-are-updated->master: WIP: Resolve "Workspace builds might not rebuild correctly when dependecies are updated") #315 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/315 | 13:39 |
dominic | didn't quite mean to make this a merge request with the main repo, but would someone mind taking a look anyway https://gitlab.com/BuildStream/benchmarks/merge_requests/3/commits | 13:44 |
tristan | dominic, I need somebody to be in charge of benchmarks | 13:46 |
tristan | dominic, my initial thought on seeing that headline is; why on earth would we *want* to convert to csv at all ? | 13:47 |
jmac | We agreed I'd do that yesterday but I'm about to be pushed to another project, so... | 13:47 |
gitlab-br-bot | buildstream: merge request (215-workspace-builds-might-not-rebuild-correctly-when-dependecies-are-updated->master: WIP: Resolve "Workspace builds might not rebuild correctly when dependecies are updated") #315 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/315 | 13:47 |
tlater | Hm, it's annoying that pylint doesn't run with ./setup.py | 13:48 |
tristan | What ?! | 13:48 |
tristan | tlater, it doesnt ?! | 13:48 |
tlater | tristan: You need to specify --addopts --pylint | 13:48 |
* tristan palmface | 13:48 | |
tlater | Yep, a bit of an overlook | 13:49 |
tristan | So... that is like a one liner change to fix | 13:49 |
dominic | tristan, we wanted something easily readable by a human and csv was the suggestion for that | 13:49 |
tristan | dominic, I very highly doubt that any format is going to be easily readable by a human; if it is, it means our benchmarks are severely weak and lack a lot of samples :-S | 13:50 |
tristan | Though, csv would seem the closest bet for that | 13:51 |
tristan | still, if I need to second-guess the graphs, or try to observe something that is not in presentable form, I'm going to be using search features into a really huge file, not sure the format makes much of a difference | 13:52 |
jmac | CSV is easier for someone non-technical to import into a spreadsheet | 13:52 |
tristan | There is that | 13:53 |
tristan | What will you do with it in a spreadsheet ? | 13:53 |
jmac | Plus, slightly easier to convert into a HTML table, although you might just output that directly | 13:53 |
tristan | Anyway, I have to leave the closing coffee shop :-/ | 13:53 |
jmac | A common use is to have baseline results compared against results from a new branch you're thinking of merging | 13:54 |
*** tristan has quit IRC | 13:57 | |
*** tristan has joined #buildstream | 14:25 | |
gitlab-br-bot | buildstream: merge request (tristan/project-refs->master: WIP: Optionally load and store source references in project.refs) #314 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/314 | 14:40 |
tristan | juergbi, ^^^ still needs docs and a few tests; I'll remove WIP status when that's done and ask you for a lookover | 14:41 |
juergbi | ok | 14:43 |
juergbi | tristan: you generally agree that intermediate commits in a branch should not break things, don't you? | 14:56 |
gitlab-br-bot | buildstream: merge request (test-run-doc->master: HACKING.rst: Add integration and pytest notes) #319 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/319 | 15:26 |
gitlab-br-bot | buildstream: merge request (215-workspace-builds-might-not-rebuild-correctly-when-dependecies-are-updated->master: WIP: Resolve "Workspace builds might not rebuild correctly when dependecies are updated") #315 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/315 | 15:30 |
gitlab-br-bot | buildstream: merge request (215-workspace-builds-might-not-rebuild-correctly-when-dependecies-are-updated->master: WIP: Resolve "Workspace builds might not rebuild correctly when dependecies are updated") #315 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/315 | 15:30 |
jennis | Hi guys, the docker source plugin makes use of the requests module: https://gitlab.com/BuildStream/bst-external/blob/master/bst_external/sources/docker.py#L50 | 15:55 |
jennis | So in order to include this in CI tests, we have "requests" within install=requires | 15:55 |
jennis | https://gitlab.com/BuildStream/bst-external/blob/master/setup.py#L38 | 15:56 |
jennis | However, when pushing my branch, I am getting this error: https://gitlab.com/BuildStream/bst-external/-/jobs/57640912 | 15:56 |
tlater | skullman: So you'd be happy with me trying to merge #277 if I added a few more tests? | 15:57 |
* tlater is tempted to do the workspace object refactor on that branch, but that's probably too much work to port over | 15:58 | |
skullman | to #277 or your other one. could be appropriate given the overlap | 15:58 |
gitlab-br-bot | buildstream: merge request (juerg/buildqueue->master: Avoid SIGCHLD errors) #320 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/320 | 15:59 |
tlater | Yeah, I could certainly swallow that branch - just 50 or so changes | 16:00 |
tlater | But I think there's been enough confusion about this already x) | 16:00 |
jennis | Just incase, ^^ no worries, I've sorted that problem | 16:12 |
jennis | (to what I said above) | 16:12 |
jennis | Ok so, if anyone has a chance to look at this I'd really appreciate it: https://gitlab.com/BuildStream/bst-external/-/jobs/57647277 | 16:13 |
jennis | Basically, in the sandbox, the permissions of the debian/rules file changes so essentially the build command for dpkg-build-test.bst fails. However, if I locally run `bst build dpkg-build/dpkg-build-test.bst` (which is in tests/project/elements directory of my branch) the build is successful | 16:16 |
tlater | jennis: Wait, that only fails on gitlab? | 16:16 |
jennis | yah | 16:16 |
jennis | well hang on | 16:16 |
tlater | It's possible git doesn't save the permissions you need | 16:16 |
jennis | the *test* fails locally, but a local build of the element does not | 16:17 |
tlater | Ah, nevermind me then :) | 16:17 |
jennis | i.e if I execute `./setup.py test` I'll get the same error | 16:17 |
jennis | But all that test is trying to do is this: https://gitlab.com/BuildStream/bst-external/blob/convert-to-new-style-integration-tests/tests/dpkg-build.py#L19 | 16:19 |
tristan | juergbi, generally speaking yes; I realize my branch isn't doing that though :) | 16:27 |
*** aday has quit IRC | 16:28 | |
juergbi | ok, i keep mentioning such issues then | 16:28 |
tristan | juergbi, I could squash some things and make it happen in this case, for larger changes (like junctions was, or introducing project conditionals was...) I would tend to not be too strict | 16:28 |
*** aday has joined #buildstream | 16:29 | |
juergbi | imo, it doesn't really make sense to prepare a clean branch of multiple commits if the individual commits aren't usable | 16:30 |
juergbi | it may sometimes be ok to break a minor feature but the basic functionality should remain functional | 16:30 |
juergbi | bad sentence | 16:30 |
juergbi | breaks bisect, individual revert etc | 16:31 |
*** Prince781 has joined #buildstream | 16:32 | |
Nexus | Seems like the reason my test is hanging is due to `fuse` not terminating | 16:32 |
*** aday has quit IRC | 16:33 | |
Nexus | juergbi: you seen anything like this before? ^ | 16:33 |
tristan | juergbi, good points, I guess at times I care about not having "too big commits", but perhaps it's pointless | 16:33 |
*** aday has joined #buildstream | 16:33 | |
skullman | from the strace log the fuse process hasn't been sent a kill signal but it is being waited for | 16:34 |
juergbi | Nexus: there have been fuse issues in the past. i've seen hangs for interrupted jobs. don't know whether related | 16:34 |
juergbi | Nexus: given that the MR changes paths, i would add debug prints for what paths are being unmounted. maybe there is some odd inconsistency between mount and unmount paths when build-root is overridden | 16:36 |
juergbi | tristan: i certainly care about keeping commits reasonably sized as well. i just still want them to be functional | 16:37 |
tristan | juergbi, oh... fwiw; dont bother trying to nitpick the commit changes on tristan/project-refs | 16:37 |
Nexus | juergbi: How do i do that? | 16:37 |
tristan | juergbi, saw your comment, I will take that into consideration and fix it before removing WIP status (dont want to waste your time) | 16:37 |
juergbi | tristan: i know it can sometimes be cumbersome but in most cases i'd like to follow that | 16:37 |
tristan | juergbi, agreed :) | 16:37 |
juergbi | Nexus: in the Mounter class, but looking at the code i don't see how it could unmount the wrong path, so probably doesn't make sense | 16:39 |
ltu | Nexus, re this saying quild and linking to an empty page - https://buildstream.gitlab.io/bst-external/ | 16:39 |
ltu | sis you say there was an MR to address that? I can't find one | 16:40 |
ltu | did* | 16:40 |
juergbi | Nexus: you only see this with custom build-root, correct? or is the issue still there when you use the (new) default build-root? | 16:40 |
gitlab-br-bot | buildstream: issue #299 ("Build sometimes hangs forever after "Unknown exception in SIGCHLD handler"") changed state ("closed") https://gitlab.com/BuildStream/buildstream/issues/299 | 16:41 |
gitlab-br-bot | buildstream: merge request (juerg/buildqueue->master: Avoid SIGCHLD errors) #320 changed state ("merged"): https://gitlab.com/BuildStream/buildstream/merge_requests/320 | 16:41 |
Nexus | juergbi: as far as i can tell, it has nothing to do with custom build-roots, it's the change working directory test. I'm not 100% about what it actually does | 16:42 |
ltu | jonathanmaw, thanks for the post to the list, good email - fyi there's also this issue that is relevant - https://gitlab.com/BuildStream/buildstream/issues/244 | 16:46 |
jonathanmaw | ah, ta ltu | 16:46 |
*** Prince781 has quit IRC | 16:54 | |
ltu | Nexus, have you seen my question? | 16:56 |
Nexus | oh, no | 16:56 |
Nexus | erm | 16:56 |
Nexus | ltu: that was fixed but needs to be merged iirc | 16:57 |
ltu | Nexus, still looking for the MR.. | 17:00 |
Nexus | https://gitlab.com/BuildStream/bst-external/merge_requests/21 | 17:02 |
ltu | as if by magic! tvm :) | 17:03 |
Nexus | i have to update it a bit now, it's a few commits beind | 17:04 |
Nexus | behind* | 17:04 |
juergbi | Nexus: trying it here i see the following https://pastebin.com/LaXXJPXK | 17:06 |
juergbi | have you seen this already? | 17:06 |
tlater | Ouch, that certainly looks like a Mounter issue | 17:07 |
tlater | And is consistent with the symptoms - usually these kinds of problems cause hangs | 17:07 |
juergbi | looking at the root directory, there is indeed no buildstream directory | 17:08 |
juergbi | and these might show two issues | 17:08 |
juergbi | 1) no buildstream directory for some reason | 17:08 |
juergbi | 2) cleanup handler in case of such an error is broken | 17:08 |
tlater | Nexus already has an issue for 2 open | 17:08 |
tlater | 1 is likely caused by his branch | 17:08 |
juergbi | ok | 17:08 |
juergbi | or possibly just triggered | 17:09 |
Nexus | juergbi: yes | 17:09 |
Nexus | thats some of what i've seen | 17:09 |
tlater | This might be a beautiful test case for the mount cleanup :) | 17:11 |
juergbi | so is the issue here that cwd is set to a directory that doesn't exist? | 17:13 |
tlater | juergbi: It does look like it | 17:14 |
juergbi | buildelement calls mark_directory for build-root and install-root | 17:14 |
juergbi | scriptelement doesn't | 17:14 |
tlater | Either cwd is set oddly or buildstream fails to create the sandbox properly | 17:14 |
juergbi | however, scriptelement should be creating directories that are specified as destination in the layout config | 17:15 |
juergbi | and /buildstream is in there | 17:15 |
juergbi | ah no, that test creates a new script element without layout | 17:16 |
tlater | Nexus: | 17:16 |
tlater | ^ | 17:16 |
* Nexus is reading but is a bit lost | 17:16 | |
juergbi | for one, i think we should call mark_directory at least for install-root also in scriptelement, same as we already do in buildelement | 17:18 |
juergbi | and also call mark_directory() for the cwd | 17:19 |
juergbi | or build-root. not quite sure. scriptelement doesn't really use build-root | 17:19 |
juergbi | i'm wondering why this works with bwrap | 17:20 |
tlater | juergbi: It's possible bwrap creates the directories slightly differently | 17:21 |
tlater | The chroot sandbox is built to mimic the process, but bwrap automatically sets up some sandboxing | 17:22 |
juergbi | ah, sandboxbwrap ensures cwd exists | 17:22 |
juergbi | we should mirror that in sandboxchroot | 17:22 |
juergbi | (or drop it from bwrap) | 17:22 |
juergbi | Nexus: will you handle the fix for this or shall i put this in a separate MR and you can then rebase? | 17:23 |
juergbi | it's a bug/inconsistency between sandbox implementations that is independent of your branch | 17:23 |
tlater | We should certainly fix 2 as well to make these kinds of issues debuggable | 17:25 |
tlater | juergbi: Do you happen to know if one needs to catch generic exceptions in context handlers to ensure cleanup? | 17:25 |
* tlater was under the impression that contextmanagers' __exit__ is always run, regardless of exceptions | 17:26 | |
tlater | But that almost certainly isn't the case, considering this | 17:26 |
Nexus | juergbi: i'd appreciate if you could do it, you have a far better grasp of what needs to be done than i do | 17:27 |
juergbi | ok, will do | 17:28 |
juergbi | tlater: yes, we should definitely fix (2) as well. don't know otoh | 17:28 |
* tlater tests his theory with Nexus' branch | 17:29 | |
* Nexus finds interesting bugs with his code | 17:30 | |
gitlab-br-bot | buildstream: merge request (juerg/sandbox-directories->master: Sandbox directory fixes) #321 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/321 | 17:46 |
juergbi | Nexus: should work with the above | 17:46 |
juergbi | this only fixes the missing directories, it does not fix the cleanup | 17:47 |
* tlater thinks he's found a fix for that, will create an MR in a bit | 17:47 | |
tlater | When I regain control of my shell, that is... | 17:47 |
juergbi | great | 17:47 |
juergbi | haha | 17:47 |
gitlab-br-bot | buildstream: merge request (modAndTest->master: Making changes to various documents:) #206 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/206 | 17:52 |
gitlab-br-bot | buildstream: merge request (issue-243_dpkg_import_source_plugin->master: WIP: Created DPKG Source plugin for Issue #10) #305 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/305 | 17:53 |
Nexus | ok, so what am i merging/rebasing with what? | 17:54 |
tlater | Nexus: If you rebase juergbi 's branch onto your branch you should not be running into the hanging test issue anymore | 17:55 |
tlater | Hm, need more testing... Maybe tomorrow | 17:59 |
*** dominic has quit IRC | 18:02 | |
*** jonathanmaw has quit IRC | 18:03 | |
gitlab-br-bot | buildstream: issue #302 ("File permissions changing inside the sandbox") changed state ("opened") https://gitlab.com/BuildStream/buildstream/issues/302 | 18:09 |
gitlab-br-bot | buildstream: merge request (juerg/sandbox-directories->master: Sandbox directory fixes) #321 changed state ("merged"): https://gitlab.com/BuildStream/buildstream/merge_requests/321 | 18:18 |
juergbi | Nexus: i've merged my fixes, so you can now simply rebase on top of master | 18:19 |
jjardon[m] | Hi, Is there any roadmap about planned buildstream features somewhere? Maybe milestones in gitlab or tags? | 18:40 |
juergbi | jjardon[m]: https://gitlab.com/BuildStream/buildstream/milestones/1 | 18:41 |
jjardon[m] | juergbi: tvm | 18:41 |
*** mcatanzaro has quit IRC | 19:04 | |
*** aday has quit IRC | 19:05 | |
*** tristan has quit IRC | 19:20 | |
*** valentind has joined #buildstream | 19:40 | |
*** toscalix has joined #buildstream | 20:49 | |
gitlab-br-bot | buildstream: merge request (jjardon/install_fixes->master: doc/source/install.rst: Divide in two sections) #322 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/322 | 21:15 |
gitlab-br-bot | buildstream: merge request (patch-1->master: install.rst: Update instructions for Arch) #264 changed state ("closed"): https://gitlab.com/BuildStream/buildstream/merge_requests/264 | 21:16 |
gitlab-br-bot | buildstream: merge request (jjardon/install_fixes->master: doc/source/install.rst: Divide in two sections) #322 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/322 | 21:25 |
*** toscalix has quit IRC | 21:27 | |
*** toscalix has joined #buildstream | 21:28 | |
*** xjuan has quit IRC | 21:32 | |
*** toscalix has quit IRC | 21:37 | |
gitlab-br-bot | buildstream: issue #293 ("Write "getting started" guide") changed state ("closed") https://gitlab.com/BuildStream/buildstream/issues/293 | 21:40 |
gitlab-br-bot | buildstream: merge request (jjardon/getting_started->master: Add getting started section) #323 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/323 | 22:14 |
gitlab-br-bot | buildstream: merge request (jjardon/getting_started->master: WIP: Add getting started section) #323 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/323 | 22:14 |
gitlab-br-bot | buildstream: merge request (jjardon/getting_started->master: WIP: Add getting started section) #323 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/323 | 22:14 |
gitlab-br-bot | buildstream: merge request (jjardon/getting_started->master: WIP: Add getting started section) #323 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/323 | 22:27 |
gitlab-br-bot | buildstream: merge request (jjardon/getting_started->master: WIP: Add getting started section) #323 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/323 | 22:38 |
gitlab-br-bot | buildstream: merge request (jjardon/getting_started->master: Add getting started section) #323 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/323 | 22:40 |
gitlab-br-bot | buildstream: merge request (jjardon/getting_started->master: Add getting started section) #323 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/323 | 22:42 |
gitlab-br-bot | buildstream: merge request (jjardon/getting_started->master: Add getting started section) #323 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/323 | 22:44 |
*** valentind has quit IRC | 22:46 | |
gitlab-br-bot | buildstream: merge request (jjardon/getting_started->master: Add getting started section) #323 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/323 | 22:46 |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!