| *** Prince781 has quit IRC | 00:15 | |
| *** Prince781 has joined #buildstream | 03:10 | |
| *** Prince781 has quit IRC | 03:51 | |
| *** noisecell has joined #buildstream | 05:24 | |
| *** noisecell has quit IRC | 05:53 | |
| *** tristan has joined #buildstream | 06:19 | |
| *** noisecell has joined #buildstream | 07:11 | |
| *** noisecell has quit IRC | 07:18 | |
| *** toscalix has joined #buildstream | 07:44 | |
| *** jonathanmaw has joined #buildstream | 07:58 | |
| gitlab-br-bot | buildstream: merge request (tristan/except-hook->master: Handle exceptions in the main process) #427 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/427 | 08:02 |
|---|---|---|
| *** jennis has joined #buildstream | 08:06 | |
| *** bethw has joined #buildstream | 08:11 | |
| laurence | tristan, toscalix, have just seen email re GUADEC CfP and noticed we have one week to submit. | 08:26 |
| laurence | Deadline is Sunday 29 April. | 08:26 |
| tristan | Yes | 08:28 |
| laurence | Let's get that ball rolling :) | 08:29 |
| *** mohan43u has joined #buildstream | 08:46 | |
| *** aday has joined #buildstream | 08:53 | |
| gitlab-br-bot | buildstream: merge request (tristan/except-hook-no-test->master: Overwrite sys.excepthook to handle exceptions in the main application.) #429 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/429 | 08:56 |
| gitlab-br-bot | buildstream: merge request (tristan/except-hook->master: Handle exceptions in the main process) #427 changed state ("closed"): https://gitlab.com/BuildStream/buildstream/merge_requests/427 | 08:58 |
| gitlab-br-bot | buildstream: merge request (tristan/except-hook->master: Handle exceptions in the main process) #427 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/427 | 08:58 |
| gitlab-br-bot | buildstream: merge request (tristan/except-hook->master: Handle exceptions in the main process) #427 changed state ("closed"): https://gitlab.com/BuildStream/buildstream/merge_requests/427 | 08:58 |
| tristan | Gah, annoying | 08:59 |
| tristan | "Comment & close merge request" *really* doesnt work | 08:59 |
| gitlab-br-bot | buildstream: merge request (tristan/except-hook-no-test->master: Overwrite sys.excepthook to handle exceptions in the main application.) #429 changed state ("merged"): https://gitlab.com/BuildStream/buildstream/merge_requests/429 | 09:00 |
| *** tiago has joined #buildstream | 09:02 | |
| *** dominic has joined #buildstream | 09:20 | |
| tristan | jmac, So I've merged the original meat of it in !429 above, without the test; I tried a bit to make the test work but, it's not easily possible really | 09:42 |
| tristan | do you have other concerns than that ? | 09:42 |
| tristan | I think we've handled all the exceptions; note that while the scheduler is running, they have to be handled in the asyncio mainloop callbacks | 09:43 |
| tristan | (they wont reach the main execution context anyway) | 09:43 |
| tristan | so this really only handles unhandled (non BstError) exceptions at load time pre-scheduler, or whatever happens post-scheduler; this notably can include bugs which occur with workspace operations outside of scheduler invocations | 09:44 |
| jmac | No, not really - I'd like a test, but yes, it's going to be very difficult | 09:46 |
| jmac | It's difficult enough getting the problem to occur manually | 09:46 |
| jmac | If I get a chance I'll try and come up with a test for it, I think otherwise we leave it as is | 09:48 |
| jmac | Thanks for doing the research/refactor and merging | 09:48 |
| tristan | Running the `bst` command in that directory with the exception throwing `preflight()` implementation caused the error, but yeah it's not possible afaics without writing a custom plugin which explicitly breaks | 09:57 |
| tristan | The exception could just as well also occur in `Plugin.configure()` | 09:57 |
| tristan | (at load time | 09:57 |
| tristan | ) | 09:57 |
| *** toscalix has quit IRC | 09:59 | |
| *** toscalix has joined #buildstream | 09:59 | |
| jmac | tristan: Where? Running `bst show error.bst` in tests/pipeline/preflight-error ? | 10:00 |
| tristan | jmac, yep | 10:00 |
| tristan | umm no | 10:00 |
| tristan | jmac, in the 'unhandled' test which I did not apply to master | 10:01 |
| tristan | the preflight-error raises a valid preflight error, which is a BstError and explicitly handled | 10:01 |
| tristan | but the unhandled-error test with plugin which your branch was adding, properly caused the excepthook to catch it | 10:02 |
| *** cs_shadow has joined #buildstream | 10:02 | |
| tristan | I tested it by reverting the excepthook patch and trying `bst show error.bst` in tests/pipeline/unhandled-error, and got the default, also the original code was doing "\n".join() which I fixed to be "".join() (the formatted traceback retains newlines) | 10:03 |
| tristan | So, it was very clear that that fix effected how `bst show` behaves with an unhandled error :) | 10:03 |
| jmac | tristan: So before !429 went in, unhandled_error does dump the stack trace outside the normal mechanism - but it's not a demonstration of it being overwritten by the status bar, which is what I really want | 10:03 |
| tristan | Ah, yes | 10:04 |
| tristan | jmac, that is very hard to handle, and we discovered the reason for that after you had done that patch | 10:04 |
| tristan | jmac, and it's extremely hard to reproduce; I think you need to somehow get asyncio to have a bug for that to happen | 10:04 |
| jmac | Yes, it will be | 10:04 |
| tristan | the regular unhandled exceptions which trigger, happen in asyncio loop callbacks, and we fixed it there anyway | 10:05 |
| jmac | For the moment though I'm satisfied that this will solve #197 unless we show otherwise, so we can close that if you're happy with doing so | 10:06 |
| tristan | (i.e. now we catch them when running any Queue() implementation post-task processing, and fire a MessageType.BUG message) | 10:06 |
| tristan | Right, will do :) | 10:06 |
| gitlab-br-bot | buildstream: issue #197 ("Status bar obscures text not printed through the logging mechanism (e.g. stack traces)") changed state ("closed") https://gitlab.com/BuildStream/buildstream/issues/197 | 10:11 |
| jmac | Cool | 10:12 |
| tristan | tlater, I am suspecting that https://gitlab.com/BuildStream/buildstream/issues/374 may be fallout from incremental workspace builds, I am trying to reproduce it | 10:15 |
| tlater | Oof, that looks like a horrible one | 10:17 |
| tristan | it'll take a while to reproduce | 10:18 |
| tristan | and figure out what's up | 10:18 |
| tristan | It might be a result of not re-running configure, not sure... | 10:19 |
| * tristan wants to reach a stable-enough point for a 1.1.3 release | 10:19 | |
| tlater | I wonder what we'd have to do if it *is*, but I suppose there's no telling without debugging first | 10:19 |
| tristan | exactly | 10:20 |
| tristan | have to figure out why | 10:20 |
| tristan | seems like maybe broken symlinks | 10:20 |
| tristan | with ../../ | 10:20 |
| tristan | tlater, it could be that mcatanzaro ran `cmake` in such a way that places his builddir outside of the workspace dir | 10:21 |
| tristan | tlater, and then we stopped correcting it by rerunning `cmake` | 10:21 |
| tristan | in which case... it would perhaps be good enough to have a good enough error message | 10:21 |
| tristan | and clarify that "Hey ! You only build BuildStream workspaces using BuildStream" | 10:22 |
| tlater | Should we enforce that? Perhaps we could find a way to re-run configure if we detect someone mucking about with our build dir | 10:23 |
| tristan | Then again, it might be also worthwhile to think about possibly retrying a workspace build with a `prepare()` when it fails without `prepare()` | 10:23 |
| tristan | right, there is that | 10:24 |
| tristan | But, that's quite not possible to know | 10:24 |
| tristan | one might know if say... commands were running and tried to access outside of the build directory using `../../` | 10:24 |
| tristan | and with a fuse layer, we might catch that nastiness | 10:24 |
| tristan | We might provide a `--force-retry` or something more fantastically named to `bst build` | 10:28 |
| tristan | I might be called `--force-reconfigure` for the purpose of workspace incremental, but it should also apply to later incremental builds, and also the ability to force try to build a build which failed remotely (i.e. we only downloaded the failed build artifact) | 10:29 |
| tristan | wouldn't be nice to provide separate CLI options for all these cases | 10:29 |
| gitlab-br-bot | buildstream: merge request (214-filter-workspacing->master: Make workspace commands on a filter element transparently pass through to their build-dependency) #317 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/317 | 10:29 |
| tlater | It would be nice if buildstream could just do the right thing | 10:30 |
| tlater | But I suppose there's really no way of telling what the right thing is... | 10:30 |
| tristan | Well, clearly when a failed build artifact is downloaded, you want the chance of retrying it | 10:30 |
| tristan | a build can fail because say, you gave it too many CPUs | 10:31 |
| tlater | Perhaps some sort of API for plugins to tell us when the configure commands may need to be rerun? | 10:31 |
| tristan | and as such, it tried to link 10 huge things at once | 10:31 |
| tristan | and died from OOM kill | 10:31 |
| tristan | Well, maybe there is a way to have plugins hint about something which may have went wrong workspace-wise | 10:32 |
| tristan | like Element.tell_me_if_workspace_is_configured_to_build_outside_of_workspace_dir_please() | 10:32 |
| tristan | and have cmake/autotools/etc implement that separately, that is a possibility indeed | 10:33 |
| tristan | tlater, I dont think it's clear cut that it would "solve every case" | 10:33 |
| tristan | but it might be cool to have | 10:33 |
| tristan | rather, even if the build passes, we'd still want to know "If the workspace was reconfigured by the user" | 10:36 |
| tristan | Because using the configure results which the user did outside of BuildStream, will most certainly cause a bug | 10:37 |
| tristan | i.e. any configure option might be wrong, and the software you test against that workspace build will have an incorrect configuration | 10:37 |
| tristan | hmmm | 10:37 |
| *** tiago has quit IRC | 10:39 | |
| *** tiago has joined #buildstream | 10:43 | |
| gitlab-br-bot | buildstream: merge request (jmac/artifact-receive-profiling->master: WIP: Add artifact cache receive profiling domain and instructions.) #383 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/383 | 10:44 |
| Nexus | tristan: https://gitlab.com/BuildStream/buildstream/issues/311#note_69267797 | 10:44 |
| Nexus | https://gitlab.com/BuildStream/buildstream/issues/311#note_69267797 | 10:44 |
| gitlab-br-bot | buildstream: merge request (jmac/artifact-receive-profiling->master: Add artifact cache receive profiling domain and instructions.) #383 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/383 | 10:44 |
| tristan | cs_shadow, I've commented on the incremental workspace builds again https://gitlab.com/BuildStream/buildstream/issues/209, it's currently closed; related to the above discussion, you may be interested in that... | 10:44 |
| cs_shadow | tristan: thanks! will have a look in a bit | 10:45 |
| gitlab-br-bot | buildstream: merge request (jmac/artifact-receive-profiling->master: Add artifact cache receive profiling domain and instructions.) #383 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/383 | 10:45 |
| Nexus | tristan: Please can you clarify your second bullet point in https://gitlab.com/BuildStream/buildstream/issues/311#note_69267797 as it contradicts the point of the functionality i added. Are you now saying that you do not want the ability to open a workspace using the cached contents of a build tree? | 10:45 |
| Nexus | ignore my previous two, i was trying to get pasting to work, it was acting oddly | 10:46 |
| tristan | Nexus, I don't think it contradicts; do you see any circumstances where A.) You are opening a new workspace... B.) The cached build tree for the element you are opening a *new* workspace on, is a result of a workspace build | 10:48 |
| tristan | I.e. if I understand, you are talking about this bullet: "This also means that we should probably not, under any circumstance, ever be caching a workspace build directory" | 10:48 |
| Nexus | that's the one, yes | 10:48 |
| tristan | So, the utility of opening a workspace using a cached build artifact... would have to revolve around the cache key of the *unworkspaced* element anyway | 10:49 |
| tristan | we would use that build tree to open a new workspace, but we would not *feed* the cache with caching of build trees of already workspaced elements | 10:50 |
| tristan | (already as it stands, without cleaning things up, even without caching workspace builds we are talking about creating WebKit artifacts that are > 6GB, so that part also needs to be cleaned up before anything along these lines can land) | 10:51 |
| Nexus | tristan: other than changing the directory that buildstream looks at when doing so, is there a difference in functionality between building a project, and building a workspaced project? | 10:51 |
| tristan | What is a "project" ? you mean an "element" ? | 10:51 |
| Nexus | yes, sorry | 10:51 |
| tristan | we can have a "workspaced project" too, when we open a workspace on a junction | 10:51 |
| tristan | Right... so there is the changes we've been discussing above | 10:52 |
| tristan | I.e. we skip running configure-commands except for the first time | 10:52 |
| tristan | which leads to the issues described here: https://gitlab.com/BuildStream/buildstream/issues/209#note_69814163 | 10:52 |
| tristan | (which I just pointed cs_shadow to ^^^^^^^^^^) | 10:53 |
| tristan | Nexus, asides from that no there is not *currently*, except that if we're going to start omitting VCS directories from the staging area, that must not happen for workspaces) | 10:53 |
| tristan | i.e. a workspace must have the VCS related data, while a build dir... only might or might not | 10:54 |
| tristan | juergbi, or rather... we *might* reduce friction here by having the caching of build directories delegated somehow... such that a Source implementation might *Trim* the VCS data from an already staged directory | 10:54 |
| tristan | juergbi, do you think that maybe Source->trim_vcs() might be a nicer approach ? | 10:55 |
| tristan | heh | 10:55 |
| Nexus | tristan: to confirm a point you just made. you want to *ommit* VCS directories from the staging area (when building) but have them *included* on a workspace? | 10:55 |
| tristan | didnt think of that way before, that would allow the build to happen with VCS data in place (holding the hands of a few modules which love to have their .git/ directories in place) | 10:55 |
| tristan | Nexus, the VCS directories *MUST* be omitted from the build dirs cached inside the artifacts | 10:56 |
| juergbi | tristan: this wouldn't help remote execution, though | 10:56 |
| tristan | juergbi, but if things are stage on demand... it shouldnt make things significantly slower right ? | 10:56 |
| juergbi | we would have to put the whole .git directory into CAS, which we'd like to avoid | 10:57 |
| juergbi | remote execution itself in the worker should not be affected with on-demand staging | 10:57 |
| tristan | Or we'd only put it in case if the remote worker tried to access something in .git/ ? | 10:57 |
| juergbi | but preparation | 10:57 |
| tristan | hehe | 10:57 |
| * tristan pushes the limits | 10:57 | |
| juergbi | that's not really possible | 10:57 |
| juergbi | Merkle trees and such | 10:58 |
| juergbi | (without special provisions, that is) | 10:58 |
| tristan | you can't have CAS ask the "Data provider" to "give me the data for this merkle branch/sha" ? | 10:58 |
| tristan | Right, that would fall under "special provisions", i.e.: Modifying the spec to some degree | 10:58 |
| juergbi | we don't even want to calculate the SHA for the .git directory | 10:59 |
| juergbi | and without that SHA, the .git directory is not in the overall Merkle tree | 10:59 |
| tristan | Ah.. right, the remote worker would have to ask the filesystem for ".git/foo", then that would have to ask CAS for ".git/foo", which would have to say "Hey do we have a sha for it ?"... etc | 10:59 |
| tristan | juergbi, so in other words, you are absolutely convinced that we need to not have the VCS data at stage/build time ? | 11:00 |
| tristan | juergbi, or are you tempted to rework the spec into something more ideal ? (or is it already ideal ?) | 11:00 |
| tristan | juergbi, we get one shot at this, so let's be bastards and change the spec if it's worthwhile | 11:01 |
| * tristan has a feeling it's sufficiently designed as is and that we rather not change it because the change would be so drastic, it would cause everyone at google to rewrite all of their build systems | 11:02 | |
| juergbi | I don't have an idea yet how to improve the spec in a clean way to support this | 11:02 |
| tristan | i.e. it would change completely that "You know the sha of everything you will need before building" | 11:03 |
| tristan | and cause everything to happen on demand | 11:03 |
| juergbi | I would like to look into the possibility of getting the right git describe output even without a big .git directory | 11:03 |
| juergbi | If 'git describe' is the only real reason for the whole thing | 11:03 |
| tristan | that's an assumption not worthwhile making I think | 11:04 |
| tristan | i.e. either we support everything, or tell people that "You can't have .git/ in your build directory" | 11:04 |
| juergbi | Well, if there are other use cases, the special casing might not make sense either, so we'd need to know about them | 11:05 |
| tristan | it's not *that* horrible of a restriction | 11:05 |
| juergbi | The issue is that there are a significant number of projects that need it. And requiring upstream projects to be patched up to work with BuildStream is really bad | 11:06 |
| * tristan doesnt like making any assumption of why they would want ".git/", if they start depending on having ".git/", we should give them that | 11:06 | |
| gitlab-br-bot | buildstream: issue #375 ("Provide a way to run configure-commands again for workspaced elements") changed state ("opened") https://gitlab.com/BuildStream/buildstream/issues/375 | 11:07 |
| juergbi | If they depend on the whole history, the whole history would have to be in the source unique key | 11:07 |
| juergbi | Currently the git source unique key does not cover that | 11:07 |
| juergbi | And it would have to be in the cached build directory | 11:08 |
| juergbi | etc. | 11:08 |
| juergbi | We couldn't just fake it | 11:08 |
| juergbi | I don't think it's necessary, but going all the way to considering .git/ part of the sources has so many issues | 11:08 |
| tristan | juergbi, do you have any ideas of how to workaround this for the `git describe` only case ? | 11:10 |
| tristan | I have a feeling there are a number of ways to do that, too, people may be using `git tag` with `git show --some options` and `git branch` and parsing the output | 11:11 |
| tristan | we can't really give them a custom git tool, which actually works in their target runtime | 11:12 |
| tristan | and spoof the result | 11:12 |
| juergbi | tristan: one option could be to retrieving the list of commits since the last tag and creating a .git directory that only contains these objects + the tag | 11:12 |
| juergbi | not completely trivial but would allow use of regular git tooling | 11:13 |
| tristan | How many things actually want to have git describe at build time ? | 11:13 |
| juergbi | I don't have a number | 11:14 |
| juergbi | But I've seen this in various projects | 11:14 |
| tristan | "there are a significant number of projects that need it" | 11:14 |
| tristan | yeah, used perhaps | 11:14 |
| tristan | but lets assume that 90% of them have sane developers and don't *require* it | 11:15 |
| tristan | i.e. their build scripts will conditionally do some fancy if there is a .git directory | 11:15 |
| juergbi | I don't remember a case where it's optional | 11:15 |
| juergbi | It's not optional in BuildStream is it? | 11:15 |
| tristan | I am *completely* happy to admit that it is a complete trainwreck of a screwup if it is non-optional in buildstream | 11:16 |
| tristan | that is not the point | 11:16 |
| juergbi | well, it may be optional in that you'll get version UNKNOWN or something | 11:16 |
| juergbi | the project might still build | 11:16 |
| juergbi | but not what the user wants | 11:16 |
| gitlab-br-bot | buildstream: merge request (jjardon/bubblewrap->master: docs: Some small corrections about install requirements) #430 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/430 | 11:17 |
| tristan | So you think that `git describe` *replaces* all versioning information in the majority of user projects, instead of *augmenting* the hard coded major/minor/micro numbers ? | 11:18 |
| tristan | That is unfortunately a sane guess to make | 11:18 |
| juergbi | Yes, that's my guess | 11:18 |
| juergbi | As it's especially convenient in that case. You no longer have to bump versions everywhere for a release | 11:18 |
| gitlab-br-bot | buildstream: merge request (jjardon/install_deps->master: WIP: Update install instructions) #406 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/406 | 11:18 |
| juergbi | Linux kernel uses it for optional augmentation, and there are probably others where it's really optional | 11:19 |
| juergbi | in git itself it's also optional | 11:19 |
| juergbi | in spice and vala it generates version UNKNOWN if there is no .git (and no tarball) | 11:20 |
| juergbi | although convenient, it might indeed be considered bad practice | 11:21 |
| tristan | Ok ok so... maybe it is still reasonable to expect that... "If it costs me 6GB of diskspace to have webkit for build purposes, I totally expect to populate CAS with 6GB of webkit repo" | 11:21 |
| tristan | and to remove VCS data from the post-built built-tree for build purposes ? | 11:21 |
| tristan | Nah | 11:21 |
| juergbi | especially with git pulls later this doesn't make sense, imo | 11:22 |
| tristan | There is no circumstance where it makes sense to cache any build tree that is not exactly the build tree which was staged. | 11:22 |
| tristan | I.e. if we need .git/ when building... we need it in the cached build tree | 11:22 |
| tristan | Unless we expect to reconstruct it with Sources | 11:23 |
| juergbi | yes, I'd like to avoid this discrepancy as well | 11:23 |
| tristan | juergbi, for the case of git, would a shallow clone be enough ? | 11:26 |
| tristan | for the build dir ? | 11:26 |
| juergbi | no, git describe doesn't work properly on shallow clone, afaik, as the earlier commit objects (and possibly tags) are not included (depending on the depth setting) | 11:27 |
| tristan | grrrr | 11:27 |
| tristan | juergbi, I'm hearing that for git describe to work, you only need A.) The latest tagged object, B.) All the commits *since* the latest tag | 11:35 |
| tristan | which seems like a very small amount of history | 11:35 |
| juergbi | that's how I understand it, although I haven't tried it with just that | 11:35 |
| juergbi | hence my suggestion with creating a .git directory with just those objects | 11:36 |
| tristan | The git folks however have not been able to give me a command line which does "git checkout --all-commits-since-latest-tag-including-tag" | 11:36 |
| tristan | I guess we could use a loop or smth, hard to tell for sure | 11:37 |
| juergbi | combination of git describe and git clone --depth could work | 11:37 |
| juergbi | would still include way more objects than describe needs, though | 11:37 |
| cs_shadow | tlater: hey! Quick pm? | 11:38 |
| tlater | Sure | 11:38 |
| gitlab-br-bot | buildstream: merge request (214-filter-workspacing->master: Make workspace commands on a filter element transparently pass through to their build-dependency) #317 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/317 | 11:42 |
| juergbi | tristan: With some more code we could likely create a minimal .git directory. It shouldn't be too hard. Shall we give that a try? | 11:43 |
| tristan | Right was out for a sec | 11:45 |
| tristan | juergbi, first... lets pause for a moment, I think it would be enough, but what would we do with it... | 11:46 |
| tristan | juergbi, A.) Make all Source plugins "stage the minimum" by default I guess ? | 11:46 |
| tristan | B.) We'd deprecate / throw away the existing init_workspace() | 11:47 |
| tristan | OR, alternatively we would use that codepath for the workspace checkouts (in the absence of a build dir) | 11:48 |
| gitlab-br-bot | buildstream: merge request (jjardon/doc_sandbox->master: WIP: docs: Fix build with latest version of sphinx) #414 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/414 | 11:48 |
| tristan | And we'd "fill in the gaps" by doing init_workspace() into a tempdir, and copy-if-not-exist from the tempdir into the workspace target | 11:48 |
| tristan | juergbi, I'm *pretty* sure that would cover all the cases yes ? | 11:48 |
| juergbi | not the other way round? extract the cached build dir on top of the init_workspace() dir? | 11:49 |
| tristan | Or that, indeed | 11:49 |
| juergbi | one remaining question would be whether/how we'd cache the build dir of workspace builds | 11:49 |
| tristan | juergbi, still, you're going to have to figure out something for building workspaced elements over CAS | 11:49 |
| tristan | I *think* the answer to that is *never cache build trees of workspaced builds* | 11:50 |
| tristan | but, I'm not entirely sure | 11:50 |
| tristan | We already never share artifacts from workspaced builds | 11:50 |
| juergbi | I would also tend towards that | 11:51 |
| tristan | but, it's nice to have the exact build tree state of a failed build | 11:51 |
| tristan | right now we copy it into the build tree, but those failed build trees should turn into failed build artifacts | 11:52 |
| juergbi | yes, we'll have to think about this some more, but with this plan we're not actually changing API, so we can implement it and figure out the rest later | 11:53 |
| juergbi | without risking forth and back on the API side | 11:53 |
| tristan | we are changing API, subtly though | 11:54 |
| tristan | i.e. we're saying that Source.stage() does something different | 11:54 |
| tristan | and that Source.init_workspace() doesnt do that thing | 11:54 |
| tristan | it's not a big change and doesnt effect any known Source implementations in the wild, though | 11:55 |
| gitlab-br-bot | buildstream: merge request (jjardon/doc_sandbox->master: WIP: docs: Fix build with latest version of sphinx) #414 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/414 | 11:55 |
| gitlab-br-bot | buildstream: merge request (jjardon/doc_sandbox->master: docs: Fix build with latest version of sphinx) #414 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/414 | 11:55 |
| tristan | juergbi, ok well... lets try this then | 11:55 |
| juergbi | In a way it's just an optimization. That's assuming there aren't any projects that rely on even more from the .git directory | 11:55 |
| tristan | well, we'll have to clearly state that with the git plugin I suppose | 11:56 |
| juergbi | Yes, should definitely document it | 11:56 |
| gitlab-br-bot | buildstream: merge request (jjardon/doc_sandbox->master: WIP: docs: Fix build with latest version of sphinx) #414 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/414 | 11:56 |
| tristan | I think this might also effect .bzr though, right ? | 11:57 |
| juergbi | well possible, yes | 11:58 |
| juergbi | I know very little about bzr, though, e.g., whether the equivalent of git describe is used at all | 11:58 |
| tristan | both on the plugin author facing Source API and the user facing git/bzr plugins | 11:58 |
| tristan | true | 11:58 |
| gitlab-br-bot | buildstream: merge request (jjardon/doc_sandbox->master: WIP: docs: Fix build with latest version of sphinx) #414 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/414 | 12:01 |
| gitlab-br-bot | buildstream: merge request (214-filter-workspacing->master: Make workspace commands on a filter element transparently pass through to their build-dependency) #317 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/317 | 12:06 |
| gitlab-br-bot | buildstream: issue #376 ("Sources to only stage what is required for building, omitting large parts of VCS history") changed state ("opened") https://gitlab.com/BuildStream/buildstream/issues/376 | 12:06 |
| tristan | juergbi, Nexus: see https://gitlab.com/BuildStream/buildstream/issues/376 | 12:07 |
| juergbi | for bzr we might want to support a similar approach as git in the future, if necessary | 12:13 |
| tristan | agree | 12:13 |
| juergbi | but it's much easier to go from no .bzr to minimal .bzr | 12:13 |
| tristan | I think we can just cross our fingers | 12:13 |
| *** toscalix_ has joined #buildstream | 12:16 | |
| gitlab-br-bot | buildstream: merge request (jjardon/doc_sandbox->master: WIP: docs: Fix build with latest version of sphinx) #414 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/414 | 12:17 |
| gitlab-br-bot | buildstream: merge request (jjardon/doc_sandbox->master: WIP: docs: Fix build with latest version of sphinx) #414 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/414 | 12:21 |
| gitlab-br-bot | buildstream: merge request (jjardon/doc_sandbox->master: docs: Fix build with latest version of sphinx) #414 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/414 | 12:21 |
| jonathanmaw | juergbi: can you review https://gitlab.com/BuildStream/buildstream/merge_requests/317 when you have time, please? | 12:22 |
| juergbi | will do | 12:24 |
| tristan | juergbi, fwiw, I'm trying to take most of the MR work off your hands for the short term (because CAS etc), but I've been leaving the involved patch jonathanmaw mentioned clearly in your court :) | 12:24 |
| tristan | as you have already traveled some distance on that | 12:25 |
| jonathanmaw | tvm juergbi | 12:25 |
| juergbi | yes, makes sense | 12:26 |
| gitlab-br-bot | buildstream: merge request (jjardon/doc_sandbox->master: docs: Fix build with latest version of sphinx) #414 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/414 | 12:36 |
| *** noisecell has joined #buildstream | 12:40 | |
| tristan | jjardon, ugh.... :'( | 12:40 |
| tristan | jjardon, I just built jjardon/doc_sandbox | 12:41 |
| tristan | it takes away all the nice formatting we had, and uses this hectic random toc using the module index generated by sphinx-api-doc :'( | 12:41 |
| tristan | and puts sandbox there redundantly, which is reachable by buildstream.sandbox subpackage | 12:42 |
| jjardon | yeah, I though it was not needed with deph 5, as it is actually there, but is still needed to add it explicity | 12:44 |
| * tristan comments on issue instead | 12:44 | |
| tristan | jjardon, We had a nice flat list there specifying only the interesting entry points, which is all inter-linkable; we would rather not have the "module index" thing there *at all* | 12:45 |
| tristan | it would be nice to use explicit ToC references instead of links, for the :mod:`Bla <link>` entries we had before, that way we cooperate with the toc | 12:46 |
| tristan | but the moduleindex thing which sphinx-api-doc creates is something that we should hide completely | 12:46 |
| jjardon | tristan: ok, but that would mean we have tomaintain the index manually, instead it being autogenerated | 12:47 |
| tristan | jjardon, *definitely* | 12:47 |
| tristan | yes, it's a very short list | 12:47 |
| tristan | I do the same for plugin additions | 12:47 |
| tristan | if we can fix the plugins list to use ToC entries too, that would be stellar | 12:47 |
| *** toscalix_ has quit IRC | 12:53 | |
| gitlab-br-bot | buildstream: merge request (jjardon/bubblewrap->master: docs: Some small corrections about install requirements) #430 changed state ("merged"): https://gitlab.com/BuildStream/buildstream/merge_requests/430 | 13:00 |
| gitlab-br-bot | buildstream: merge request (jmac/artifact-receive-profiling->master: Add artifact cache receive profiling domain and instructions.) #383 changed state ("closed"): https://gitlab.com/BuildStream/buildstream/merge_requests/383 | 13:06 |
| gitlab-br-bot | buildstream: merge request (bst-here-update->master: bst-here: Add '-u' flag to upgrade buildstream (Issue #291)) #311 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/311 | 13:10 |
| *** Prince781 has joined #buildstream | 13:12 | |
| gitlab-br-bot | buildstream: merge request (jjardon/install_deps->master: WIP: Update install instructions) #406 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/406 | 13:16 |
| gitlab-br-bot | buildstream: merge request (sam/compose-symlinks-issue->master: Avoid compose element dropping files that are staged inside directory symlinks) #295 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/295 | 13:16 |
| gitlab-br-bot | buildstream: merge request (jjardon/install_deps->master: WIP: Update install instructions) #406 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/406 | 13:18 |
| gitlab-br-bot | buildstream: merge request (jjardon/install_deps->master: Update install instructions) #406 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/406 | 13:19 |
| gitlab-br-bot | buildstream: merge request (jjardon/install_deps->master: Update install instructions) #406 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/406 | 13:19 |
| tristan | cs_shadow, tlater; lets wait for mcatanzaro's reply but... I was also wondering maybe we should make `bst workspace open` have an `--incremental` switch | 13:21 |
| tristan | Not because I think optionality is important here | 13:21 |
| tristan | but rather only because I can't think of a way to make the user really understand that restriction other than to make the user ask for the functionality explicitly | 13:22 |
| tristan | or, that it would be a good way for the user to have a sane safe default, while understanding the consequences of asking explicitly for --incremental | 13:22 |
| tristan | *maybe* a message after opening the workspace would be good | 13:23 |
| tristan | but, the point is, we shouldnt be letting it escalate to the point where users are confused as to why things are acting strangely, and coming to ask us here or on the mailing list | 13:23 |
| tlater | tristan: That would work quite nicely with the build tree cache workspaces as well | 13:25 |
| gitlab-br-bot | buildstream: merge request (bst-here-update->master: bst-here: Add '-u' flag to upgrade buildstream (Issue #291)) #311 changed state ("merged"): https://gitlab.com/BuildStream/buildstream/merge_requests/311 | 13:25 |
| tlater | I did expect users to perhaps be slightly confused where these build trees in their workspaces are coming from - probably not a bad change overall | 13:26 |
| cs_shadow | I agree that being explicit about incremental workspace builds might provide a more intuitive UX | 13:26 |
| *** tristan has quit IRC | 13:27 | |
| cs_shadow | We should also say something about configure-commands as skipping it on subsequent runs may not be very intuitive for the users | 13:27 |
| tlater | Agreed, but it's hard to do without littering the buildstream output with warnings | 13:28 |
| tlater | Perhaps just leave it in the CLI documentation for incremental workspaces? | 13:28 |
| cs_shadow | CLI docs should work IMHO | 13:29 |
| *** Prince781 has quit IRC | 13:32 | |
| *** Prince781 has joined #buildstream | 13:33 | |
| *** tristan has joined #buildstream | 13:34 | |
| * tristan was swapping locations | 13:36 | |
| tristan | but got the backlog | 13:36 |
| gitlab-br-bot | buildstream: merge request (sam/compose-symlinks-issue->master: Avoid compose element dropping files that are staged inside directory symlinks) #295 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/295 | 13:37 |
| tristan | For now I'm leaving an extra comment on https://gitlab.com/BuildStream/buildstream/issues/374, lets see what Michael says | 13:42 |
| tristan | hmmm, lets take the plunge and merge https://gitlab.com/BuildStream/buildstream/merge_requests/295 | 13:52 |
| tristan | the code seems sound to me, and a lot of people looked at this | 13:52 |
| tristan | juergbi, jmac ^^^ any worries ? | 13:52 |
| * tristan has been wanting to merge that but was hoping someone might do a bit more research in the linked issue | 13:53 | |
| tristan | but, lets trust our regression tests, this is an improvement | 13:53 |
| juergbi | I don't have any concerns but I don't think I've looked into this in detail | 13:58 |
| * tristan hits merge | 13:58 | |
| tristan | symlinks need very close attention | 13:58 |
| gitlab-br-bot | buildstream: issue #270 ("Compose element throws away files inside directory symlinks at integration time") changed state ("closed") https://gitlab.com/BuildStream/buildstream/issues/270 | 13:58 |
| gitlab-br-bot | buildstream: merge request (sam/compose-symlinks-issue->master: Avoid compose element dropping files that are staged inside directory symlinks) #295 changed state ("merged"): https://gitlab.com/BuildStream/buildstream/merge_requests/295 | 13:58 |
| tristan | Yay, and we've snuck ssssam[m] into the contributor list for 1.1.3 \o/ | 13:59 |
| tlater | ssssam[m] should change a comment every buildstream release | 14:00 |
| *** xjuan has joined #buildstream | 14:06 | |
| tristan | tlater, https://gitlab.com/BuildStream/buildstream/issues/291 <-- think we can close this ? | 14:08 |
| tristan | We've now added the `-p` and when you run `bst-here --help` you should see an option about pulling the latest buildstream docker image | 14:09 |
| tristan | I think that makes it "easy to upgrade" | 14:09 |
| tlater | tristan: Yeah, just checked that, tyvm | 14:09 |
| tlater | I think that can be closed :) | 14:09 |
| gitlab-br-bot | buildstream: issue #291 ("Updating buildstream is non-trivial with bst-here") changed state ("closed") https://gitlab.com/BuildStream/buildstream/issues/291 | 14:10 |
| tristan | juergbi, any thoughts on https://gitlab.com/BuildStream/buildstream/issues/62#note_69867332 ? | 14:15 |
| tristan | I am really thinking of backing out etag and doing it differently | 14:15 |
| tristan | well, not "backing it out" | 14:15 |
| tristan | but, its really not right that this is saved as project data | 14:16 |
| tristan | it should be local cache state data | 14:16 |
| gitlab-br-bot | buildstream: issue #377 ("Change how `etag` is stored for file download source plugins") changed state ("opened") https://gitlab.com/BuildStream/buildstream/issues/377 | 14:22 |
| * tristan files #377 for the etag issue | 14:23 | |
| juergbi | tristan: Good point. Yes, I think this would make more sense | 14:27 |
| *** xjuan_ has joined #buildstream | 14:32 | |
| *** xjuan has quit IRC | 14:34 | |
| tlater | Could someone who knows about multiprocessing help me understand this: https://gitlab.com/BuildStream/buildstream/blob/master/buildstream/_scheduler/job.py#L287 ? | 14:47 |
| tlater | I figured at first that nothing between the parent and child was shared | 14:47 |
| tlater | But it turns out that stuff like self.element just copies over | 14:47 |
| tlater | I suspect that the child process inherits any existing data | 14:48 |
| tlater | But now I don't understand why we need to pass the queue | 14:48 |
| tlater | This is causing issues, python complains to me that apparently the queue is supposed to only be shared through inheritance... | 14:48 |
| tlater | I.e. `RuntimeError: Queue objects should only be shared between processes through inheritance` | 14:48 |
| tlater | Though that happens without a stacktrace and only after heavy modifications, so may not be related | 14:49 |
| juergbi | tlater: it's fork-based. the child inherits everything but it's copy-on-write, i.e., the parent doesn't see any changes that the child makes. and the child doesn't see any changes in the parent after the child has been created | 14:54 |
| dominic | tlater, this might be completely unrelated (not looked into the issue) but the man page for 2 fork was helpful for what is/isn't shared | 14:54 |
| tlater | Hm, ta gius | 14:56 |
| tlater | *guys | 14:56 |
| tlater | juergbi: Do you happen to know if multiprocessing.Queue works differently here? | 14:56 |
| juergbi | in case file descriptors are used internally, they are duplicated as is, there is no copy on write for that | 14:56 |
| tristan | tlater, I believe it does (i.e. the python highlevel stuff) | 14:57 |
| juergbi | I don't remember the Queue details | 14:57 |
| tristan | tlater, we have to poke into the internals of multiprocessing.Queue so we can get it's file descriptor | 14:57 |
| tlater | Alright, I'll dive into the doc for that I suppose... I really wonder what's failing here, was *almost* done getting rid of `element` in Jobs. | 14:57 |
| tristan | https://gitlab.com/BuildStream/buildstream/blob/master/buildstream/_scheduler/job.py#L580 | 14:58 |
| tristan | tlater, I suspect the machinery is really working fine | 14:59 |
| tristan | tlater, maybe sprinkle some debug statements around and discover what's going on... beware of going too far down the python implementation detail rabbit hole | 15:00 |
| tlater | Hm, tried quite a lot of debugging statements, it's a bit hard to pinpoint where it's breaking without stacktraces though | 15:01 |
| tristan | heh | 15:04 |
| tristan | tlater, you might be getting fooled by my recent landing of the override of sys.excepthook, which should technically not be inherited by the child task, but is (it doesnt really do harm as long as job.py is still handling them properly) | 15:05 |
| tristan | tlater, then you could try just piping through cat, disabling the queueing of error messaging for testing purposes, and seeing the messages in stderr | 15:06 |
| tristan | (the stack traces, if there indeed are any) | 15:06 |
| tlater | Ah, that's probably worth looking at, yeah ;p | 15:06 |
| tristan | tlater, fwiw, stdout/stderr is inherited | 15:06 |
| tristan | we dont do anything to prevent that, we just carefully collect stderr/stdout from child commands (like git, etc) | 15:07 |
| tristan | and we call anything that says `print("foo")` a bug | 15:07 |
| tristan | in fact, I use sys.stderr.write("...") at times directly just to debug stuff from a child task | 15:07 |
| tristan | and pipe bst | cat to ensure the status area doesnt stomp on anything | 15:08 |
| tlater | Yup, I've been making use of that trick | 15:08 |
| tlater | Really annoying that we can't work around that, actually | 15:08 |
| tristan | well, at other times I use the plugin.{info,warn,etc} mechanism to print stuff | 15:09 |
| tristan | depends, but with the code you're playing with, looks like you want something more raw | 15:10 |
| tristan | eventually, I'd like domain specific debug flags; so we can have something like Context.debug(domain, message) | 15:10 |
| tristan | and enable specific domains, but I dont think it's extremely important right now | 15:11 |
| tristan | nice to have | 15:11 |
| tristan | (this is especially useful if we want more context from a user, who is able to reproduce an issue I am not able to, if we've already put a lot of debug statements which people can enable at runtime) | 15:11 |
| gitlab-br-bot | buildstream: issue #374 ("Bad file descriptor errors") changed state ("closed") https://gitlab.com/BuildStream/buildstream/issues/374 | 15:36 |
| *** Prince781 has quit IRC | 16:10 | |
| *** Prince781 has joined #buildstream | 16:35 | |
| *** Prince781 has quit IRC | 16:40 | |
| gitlab-br-bot | buildstream: merge request (jennis/136-clean-remote-cache->master: WIP: jennis/136 clean remote cache) #421 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/421 | 16:58 |
| *** dominic has quit IRC | 16:59 | |
| *** toscalix has quit IRC | 17:16 | |
| *** noisecell has quit IRC | 17:18 | |
| jmac | Can you tell gitlab to stop testing a given branch? I'm working on a development branch and it's going to contain all sorts of rubbish which won't pass tests | 17:19 |
| tristan | it's not horrible that branches with failing tests exist | 17:20 |
| tristan | although I'll point out that it's annoying that people create MRs waaay in advance of ever proposing an MR | 17:20 |
| jmac | It's just using up runners pointlessly | 17:20 |
| jmac | This isn't going to be an MR | 17:20 |
| tristan | that is indeed problematic | 17:21 |
| tristan | I would like to have better control on that too | 17:21 |
| * tristan doesnt know how off hand, maybe something in the gitlab settings lets us do that | 17:22 | |
| tristan | I'd like to only run them on MR branches, or when explicitly created from the pipelines UI (in case someone wants to try CI in advance of creating an MR) | 17:22 |
| *** Prince781 has joined #buildstream | 17:23 | |
| *** aday has quit IRC | 17:46 | |
| *** Prince781 has quit IRC | 17:47 | |
| *** aday has joined #buildstream | 17:47 | |
| *** zalupik has quit IRC | 18:59 | |
| *** zalupik has joined #buildstream | 18:59 | |
| jjardon | jmac change the .gitlab-ci.yml so not job are being executed? | 19:42 |
| jjardon | (Removing the file completely will work if you don't want to remove jobs individually) | 19:43 |
| *** jonathanmaw has quit IRC | 19:58 | |
| *** bethw has quit IRC | 20:49 | |
| *** Prince781 has joined #buildstream | 21:05 | |
| *** xjuan_ has quit IRC | 21:29 | |
| *** Prince781 has quit IRC | 21:47 | |
| connorshea[m] | tristan: jmac you can maybe use only/except to run jobs only on the upstream, unfortunately there doesn't seem to be a way to only run jobs when there's an attached MR https://docs.gitlab.com/ce/ci/yaml/#only-and-except-simplified | 21:53 |
| bochecha | connorshea: that's a requested feature | 21:54 |
| bochecha | connorshea: https://gitlab.com/gitlab-org/gitlab-ce/issues/23902 | 21:56 |
| connorshea[m] | oh hey I remember pushing for that internally since it was required for Danger to work, but it never really got anywhere | 21:56 |
| connorshea[m] | it was an architectural decision to make CI pipelines run based on commits rather than merge requests IIRC so it's not an easy change | 21:57 |
| bochecha | "internally"? you work at gitlab? :) | 21:57 |
| connorshea[m] | used to :) | 21:58 |
| connorshea[m] | long story :D | 21:59 |
| bochecha | oh well… I don't know what part you played in what Gitlab is today, but thanks anyway, it's pretty great overall :) | 21:59 |
| connorshea[m] | I did some frontend performance work I'm pretty proud of, and also made the current docs site (previously it was a single bash script) | 22:00 |
| connorshea[m] | other than that I can't be thanked for too much :P | 22:00 |
| bochecha | the docs are pretty good :) | 22:02 |
| *** tristan has quit IRC | 22:16 | |
| *** aday has quit IRC | 22:59 | |
| *** albfan[m] has joined #buildstream | 23:59 | |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!