*** alatiera_ has quit IRC | 00:25 | |
*** xjuan has quit IRC | 00:58 | |
*** mohan43u has quit IRC | 06:39 | |
*** bochecha has joined #buildstream | 06:51 | |
*** mohan43u has joined #buildstream | 06:54 | |
*** tristan has joined #buildstream | 06:58 | |
*** ChanServ sets mode: +o tristan | 07:01 | |
*** finn has joined #buildstream | 07:38 | |
*** toscalix has joined #buildstream | 07:49 | |
*** tristan has quit IRC | 07:52 | |
*** tiagogomes has joined #buildstream | 07:52 | |
*** iker has joined #buildstream | 08:04 | |
gitlab-br-bot | buildstream: merge request (willsalmon/outOfSourecBuild->master: WIP: out of source builds) #776 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/776 | 08:09 |
---|---|---|
*** rdale has joined #buildstream | 08:30 | |
tiagogomes | toscalix where should information about the BuildStream infrastructure should be placed? This can be public, as it will not contain access details | 08:33 |
jjardon | tiagogomes: https://wiki.gnome.org/Projects/BuildStream/Infrastructure maybe? | 08:36 |
gitlab-br-bot | buildstream: merge request (Qinusty/unit-test-utils->master: Add unit tests for some utils.py functions) #799 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/799 | 08:36 |
tiagogomes | Is the Wiki still relevant? I know some things migrated to the website | 08:39 |
toscalix | hi tiagogomes | 08:42 |
toscalix | was in a meeting | 08:42 |
toscalix | I assume that we will have scripts and other config stuff that | 08:42 |
gitlab-br-bot | buildstream: merge request (willsalmon/outOfSourecBuild->master: WIP: out of source builds) #776 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/776 | 08:43 |
tiagogomes | Not really. Perhaps just one configuration file | 08:44 |
toscalix | will need to be in a repo | 08:45 |
toscalix | so I would create a new project and add the info as .md file | 08:45 |
toscalix | I would add it to nosoftware subgroup | 08:46 |
toscalix | so we do not increase the problem we have with so many repos on the root tree | 08:47 |
toscalix | what do you think? | 08:47 |
toscalix | tiagogomes: ? | 08:48 |
gitlab-br-bot | buildstream: issue #642 ("Incorrect error when malformed project.conf is found") changed state ("closed") https://gitlab.com/BuildStream/buildstream/issues/642 | 08:48 |
gitlab-br-bot | buildstream: merge request (issue-642-Invalid_project.conf_seen_as_missing->master: Incorrect error when malformed project.conf) #792 changed state ("merged"): https://gitlab.com/BuildStream/buildstream/merge_requests/792 | 08:48 |
gitlab-br-bot | buildstream: merge request (chandan/fix-source-bundle->master: Fix source-bundle command) #807 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/807 | 08:51 |
tiagogomes | toscalix I think it could be overkill. This will only be a small page stating which machines are available, and their purpose | 08:52 |
*** jonathanmaw has joined #buildstream | 08:54 | |
gitlab-br-bot | buildstream: merge request (willsalmon/outOfSourecBuild->master: WIP: out of source builds) #776 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/776 | 08:56 |
skullman | juergbi: are you able to and should https://gitlab.com/richardmaw-codethink/buildstream-integration be moved under the buildstream/ project group? | 08:57 |
toscalix | tiagogomes: then I would go for the jjardon suggestion until is stop being overkill | 08:57 |
juergbi | skullman: I don't know whether it's possible to move it, but it should be possible to push it from the client to a new repository in the right group | 08:59 |
juergbi | I can create an empty buildstream-integration repo if that helps | 08:59 |
skullman | juergbi: there is a feature to move things, but it's no great loss to just push it | 09:00 |
juergbi | there is export and import support. I don't see anything about moving | 09:02 |
juergbi | however, I don't have sufficient privileges for the existing repo to export | 09:02 |
skullman | added you as a maintainer | 09:03 |
juergbi | skullman: export was successful but import was not: no useful error message: "Error importing repository into BuildStream/buildstream-integration - " | 09:07 |
juergbi | maybe simply push with git | 09:07 |
skullman | yeah, it's just 1 MR that'll be lost | 09:07 |
* skullman doesn't have permisison to create it | 09:08 | |
*** lachlan has joined #buildstream | 09:12 | |
gitlab-br-bot | buildstream: merge request (mac_fixes->master: WIP: Implement compatibility fixes for MacOSX and WSL Blocks #411 and #412") #726 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/726 | 09:16 |
*** tristan has joined #buildstream | 09:19 | |
gitlab-br-bot | buildstream: merge request (Qinusty/unit-test-utils->master: Fix issue with _pretty_size with large numbers of bytes) #799 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/799 | 09:21 |
gitlab-br-bot | buildstream: merge request (Qinusty/unit-test-utils->master: Fix issue with _pretty_size with large numbers of bytes) #799 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/799 | 09:22 |
gitlab-br-bot | buildstream: merge request (willsalmon/outOfSourecBuild->master: Out of source builds) #776 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/776 | 09:34 |
gitlab-br-bot | buildstream: merge request (willsalmon/outOfSourecBuild->master: Out of source builds) #776 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/776 | 09:34 |
gitlab-br-bot | buildstream: merge request (Qinusty/skipped-rework->master: Add SkipJob for indicating a skipped activity) #765 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/765 | 09:37 |
gitlab-br-bot | buildstream: merge request (Qinusty/unit-test-utils->master: Fix issue with _pretty_size with large numbers of bytes) #799 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/799 | 09:44 |
gitlab-br-bot | buildstream: merge request (tristan/remove-blessings->master: _frontend/status.py: Completely remove the blessings dependency from BuildStream) #808 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/808 | 09:55 |
gitlab-br-bot | buildstream: merge request (tristan/remove-blessings-1.2->bst-1.2: _frontend/status.py: Completely remove the blessings dependency from BuildStream) #809 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/809 | 09:57 |
*** ChanServ sets mode: +o tristan | 10:00 | |
tristan | tiagogomes, Care to give a lookover on https://gitlab.com/BuildStream/buildstream/merge_requests/808 ? | 10:00 |
tristan | qinusty, I see you are updating the branches for SkipJob... is it ready for another review ? | 10:01 |
qinusty | It is :) Thanks for the review | 10:01 |
gitlab-br-bot | buildstream: merge request (mac_fixes->master: WIP: Implement compatibility fixes for MacOSX and WSL Blocks #411 and #412") #726 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/726 | 10:02 |
* tristan takes a look | 10:02 | |
qinusty | I agree with the early return points, I hadn't considered it since I was removing the returns all together. | 10:02 |
qinusty | It does help avoid unnecessary indents though | 10:02 |
tristan | qinusty, That is mostly a matter of preserving valuable history | 10:02 |
tristan | However, I agree, and I recall originally making early returns because of obnoxious indentation | 10:03 |
tristan | qinusty, To be honest though, I consider that, in a purist sense... early returns are generally bad, excepting for argument checking and the like | 10:04 |
tristan | Code is generally easier to debug and trace when a function has only one return | 10:05 |
qinusty | Hmmm, I'm torn on this. I previously worked with a coding standard which banned early returns in all cases. You need a balance really imo | 10:05 |
tristan | qinusty, I agree and disagree about balance :) | 10:05 |
tristan | Heh | 10:05 |
*** lachlan has quit IRC | 10:06 | |
tristan | qinusty, Problem is that when you have any ambiguity, everyone has a different perception of what to choose | 10:06 |
qinusty | Early returns within the first few lines is acceptable since it's mostly just performing early checks. | 10:06 |
tristan | I rather agree with that, indeed (and consider it falls within the category of "argument checking and the like") | 10:07 |
qinusty | That is where I'd draw the line with balance :D | 10:07 |
qinusty | Returning in the middle of a function isn't great. (but I've seen it in buildstream) | 10:07 |
tristan | Well... ya know... | 10:08 |
tristan | qinusty, You're more than welcome to untangle https://gitlab.com/BuildStream/buildstream/blob/master/buildstream/element.py#L1054 | 10:08 |
* tristan snickers | 10:08 | |
* qinusty sticks his head in the sand | 10:08 | |
tristan | That is probably highest priority in the refactoring in BuildStream, is a very hard problem to solve elegantly, and generally the function has a bus factor of juergbi | 10:09 |
tristan | I am never confident about any change I make to that function | 10:09 |
qinusty | I just spotted https://gitlab.com/BuildStream/buildstream/blob/master/buildstream/element.py#L1153 and wonder if I'm missing something because it look like an unnecessary elif | 10:10 |
tristan | If I had to fix breakage there, I would step back, use paper and pen, and take a week to completely rewrite | 10:10 |
tristan | qinusty, That appears to be the place where we discover the strong cache key from a pulled artifact, which was pulled by it's weak key | 10:11 |
jmac | qinusty: I think the comment is still valuable, and the interpreter should skip it | 10:12 |
tristan | In my mind, I think that code should generally have functions for every state which can get resolved, with assertions in each function; ensuring ordering of things which need to happen | 10:12 |
tristan | Maybe we're talking about the elif *above* the pass | 10:13 |
tristan | qinusty, I believe that juergbi put a couple of these in that function to maintain some level of clarity | 10:13 |
tristan | qinusty, At least all the cases are specced out, such that we don't forget them, even if we don't *do* anything for every case | 10:13 |
* juergbi would like to remove one complicating aspect by changing the way we handle workspace cache keys | 10:14 | |
juergbi | and then maybe more things can be simplified | 10:15 |
qinusty | tristan, on the topic of early returns | 10:15 |
qinusty | status.py in your latest MR has a few within _init_terminal. I guess it's only return None (so it's fairly clear) | 10:16 |
*** slaf has quit IRC | 10:21 | |
tristan | juergbi, I'm not sure what you would propose in how we handle workspaces; the way things are implemented are there because we reduce builds | 10:22 |
juergbi | I would like to keep the build output completely separate from the workspace source directory | 10:23 |
tristan | juergbi, How ? | 10:24 |
tristan | qinusty, technically I could use for / break / else there (which is a cool little python construct), would make the code a bit more wordy | 10:25 |
juergbi | tristan: with FUSE it should definitely be possible. not sure whether it's possible without as unprivileged user | 10:26 |
tristan | juergbi, I mean... considering that a build can technically modify it's own sources at least the first time around with configure stage, and considering that not all builds can support out of tree | 10:26 |
*** mohan43u has quit IRC | 10:27 | |
tristan | Maybe with the fuse thing indeed, sounds very complicated | 10:27 |
juergbi | overlayfs would easily solve it but I don't think Linux mainline allows this for unprivileged users, so probably would have to be FUSE | 10:27 |
tristan | juergbi, I think that would simplify things, but I also think it is orthogonal to the code construct of _update_state() though | 10:27 |
gitlab-br-bot | buildstream: merge request (willsalmon/outOfSourecBuild->master: Out of source builds) #776 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/776 | 10:28 |
juergbi | to some extent. there are two big reasons why that thing is complicated, one is non-strict builds and the other is workspaces | 10:28 |
juergbi | while we could likely also find a better structure with the current workspace approach, it's increasing complexity a lot | 10:29 |
qinusty | If making it more wordy doesn't affect readability, I'd argue this is worthwhile perhaps? | 10:29 |
gitlab-br-bot | buildstream: merge request (willsalmon/outOfSourecBuild->master: Out of source builds) #776 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/776 | 10:29 |
tristan | juergbi, that function is scary, I'm not sure how to sort it out though; however I imaging a sort of construct similar to gtk_widget_show() for instance (i.e. map requires realize, realize requires allocate, etc) | 10:29 |
tristan | juergbi, I know it's not a 1:1 comparison, we treat things differently in cases, but those can be classed out into separate functions | 10:30 |
*** mohan43u has joined #buildstream | 10:30 | |
juergbi | the support for optionally downloading strict artifacts for already available weak ones + workspaces make cache key handling not completely linear | 10:31 |
juergbi | but explicitly named states could indeed be useful, if we can come up with understandable names | 10:32 |
juergbi | originally the function was much smaller and thus easier to understand | 10:32 |
tristan | Yes I know, I'm not trying to lay out blame man :) | 10:33 |
tristan | At the same time, I dont want to under emphasize how much of a concern that function is :) | 10:34 |
tristan | we should worry about that structure | 10:34 |
juergbi | sure and I agree | 10:34 |
juergbi | I just prefer getting rid of the workspace state complexity (which also has other issues such as can't do a clean build) before trying to come up with a better structure | 10:36 |
*** lachlan has joined #buildstream | 10:36 | |
juergbi | as that structure will be much simpler without having to worry about workspaces | 10:36 |
tiagogomes | tristan looking | 10:37 |
tristan | juergbi, The workspace cleanup you suggest sounds pretty far fetched honestly. Your opinion will vary depending on how soon you think that could realistically happen. | 10:38 |
tristan | juergbi, I of course agree that the workspace approach with FUSE, assuming it performs well, will be great to have; just reluctant to think it will really happen that soon :) | 10:40 |
juergbi | it might not be that difficult to do with the buildbox backend | 10:40 |
*** alatiera_ has joined #buildstream | 10:41 | |
tristan | qinusty, actually for / break / else doesn't work that way: https://docs.python.org/3/reference/compound_stmts.html#for | 10:43 |
tristan | qinusty, I was wrong, I actually think it's a bit weird | 10:43 |
qinusty | in for break else, else is only called when break isn't? | 10:44 |
qinusty | That's how I know it | 10:44 |
tristan | Yeah | 10:44 |
tristan | And appears to be executed regardlessly even if break was never called | 10:44 |
qinusty | Else is called when break isn't. | 10:45 |
qinusty | When break is called, else is skipped. | 10:45 |
tristan | I mean, that's like for (....) { I did something } else { I did something } | 10:45 |
tristan | yeah, that doesnt really fit into this loop | 10:45 |
qinusty | fair enough | 10:45 |
tristan | I feel like break is when I want to abort... and else is if I aborted, but they disagree... strange | 10:46 |
qinusty | I guess it's primarily in the case that you're searching for something in a for loop. | 10:47 |
tristan | I guess it's conducive of... yeah I was thinking that | 10:47 |
qinusty | You're searching for x in y: ... else do this | 10:47 |
tristan | for (items) { if_I_found_it() { do something with it; break } else { i_didnt_find_it() } | 10:48 |
tristan | or rather | 10:48 |
tristan | for (items) { if_I_found_it() { do something with it; break } } else { i_didnt_find_it() | 10:48 |
tristan | bah | 10:48 |
tristan | anyway, we see eye to eye :) | 10:48 |
toscalix | tiagogomes: your proposal (jjardon) of keeping the infra data on the wiki requires a reference to that page in the community web page, as I just mentioned in the ticket description https://gitlab.com/BuildStream/website/issues/11 | 10:49 |
gitlab-br-bot | buildstream: merge request (mac_fixes->master: WIP: Implement compatibility fixes for MacOSX and WSL Blocks #411 and #412") #726 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/726 | 10:49 |
gitlab-br-bot | buildstream: merge request (tristan/remove-blessings->master: _frontend/status.py: Completely remove the blessings dependency from BuildStream) #808 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/808 | 10:51 |
tiagogomes | tiagogomes your MR looks fine, thought I am unsuited to do a proper review as I am not familiar with the curses library :) | 10:52 |
tiagogomes | erm, s/tiagogomes/tristan | 10:53 |
tristan | tiagogomes, Yeah; and unfortunately it's not CI-able | 10:53 |
tristan | Part of that interactive stuff that isn't covered | 10:53 |
tristan | But, it works :) | 10:53 |
qinusty | Can I bisect while in a workspace or will I see issues tristan? | 10:54 |
tristan | tiagogomes, I followed basically what blessings is doing... My thoughts are, the encode/decode to latin1 is ugly, but seeing samples on the internet shows me it has good purpose | 10:54 |
tristan | tiagogomes, And that this is better for our purpose than blessings, because we bail out instead of printing a "" in the case that not all terminal capabilities are there | 10:55 |
tristan | (blessings would just say... Yeah OK, Terminal().does_styling = True ! ... and then give you a "" if a given escape code is unsupported) | 10:55 |
tristan | qinusty, You should be fine, every time you checkout a different version of something in a workspace, it will update timestamps | 10:56 |
*** slaf has joined #buildstream | 10:56 | |
tristan | qinusty, it will however do incremental builds, that might be an issue | 10:56 |
tristan | qinusty, workspace cache keys are timestamp based, btw | 10:57 |
tristan | qinusty, So going *back* to an already tried version, will rebuild regardless of whether there is a cached result or not | 10:57 |
tristan | qinusty, I am very happy with it, I am ready to see it in action ! | 10:58 |
tristan | qinusty, your SkipJob branch, that is :D | 10:58 |
qinusty | Woop woop | 10:58 |
tristan | qinusty, Ok, so then, please go ahead and merge that; and backport it to 1.2 pronto ! | 10:59 |
* qinusty wants to see if this workspace issue is a regression via automated bisect testing over lunch | 10:59 | |
tristan | I will release tomorrow or friday | 10:59 |
gitlab-br-bot | buildstream: merge request (Qinusty/skipped-rework->master: Add SkipJob for indicating a skipped activity) #765 changed state ("merged"): https://gitlab.com/BuildStream/buildstream/merge_requests/765 | 10:59 |
tristan | qinusty, automated bisect ? that I cannot tell you | 11:00 |
qinusty | You cna tell bisect to run a script | 11:00 |
qinusty | and check for exit code | 11:00 |
tristan | qinusty, I have no idea what you would do if you wanted to automate your bisection in BuildStream | 11:00 |
tiagogomes | jjardon how do I edit the buildstream wiki on GNOME? | 11:00 |
qinusty | It would bisect until it failed my test | 11:00 |
qinusty | :D | 11:00 |
tristan | qinusty, So then you added your script/test case to the workspace, and you instruct your element.bst to do this somehow ? | 11:01 |
qinusty | Sorry no, bisecting buildstream | 11:01 |
tristan | Ahhhh | 11:01 |
qinusty | For a workspace functionality | 11:01 |
tristan | I see | 11:01 |
qinusty | It is trying to save_config from the child process | 11:01 |
qinusty | and assert is saying nooop | 11:01 |
tristan | tiagogomes, You need to be added to a group of users, GNOME wiki is no longer free for all unfortunately | 11:02 |
tiagogomes | Scrolling back to the top, I tend to avoid "if $condition return", unless the code starts to have too much indentation levels | 11:03 |
*** lachlan has quit IRC | 11:03 | |
jjardon | Hi! CAS not being cleaned is hitting us (GNOME) quite hard, see https://gitlab.gnome.org/GNOME/gnome-build-meta/issues/48 (manual cleanup every ~5 days); any idea if the problem is on track to be fixed? only asking as we might have to disable it if it's going to take too long | 11:04 |
jjardon | tiagogomes: do you have an account in the GNOME wiki? | 11:04 |
jjardon | you should see an edit buttin there | 11:04 |
tiagogomes | Shamefully I don't have one | 11:05 |
tristan | That is a start, I think once you have one you still need to ask for permission, I might be wrong but I had to ask a year ago after a few years of not using it | 11:06 |
jjardon | tiagogomes: ping me back when you have done the account | 11:06 |
tristan | jjardon, That is pretty much top priority - but won't be fixed in 1.2.1 unfortunately :-( | 11:07 |
tristan | jjardon, tlater[m] is looking at it, but I think we need someone full time looking into it | 11:07 |
jjardon | tristan: ok, no worries, thanks for the update | 11:07 |
tristan | This came up in the meeting yesterday too | 11:07 |
gitlab-br-bot | buildstream: merge request (tristan/remove-blessings->master: _frontend/status.py: Completely remove the blessings dependency from BuildStream) #808 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/808 | 11:08 |
gitlab-br-bot | buildstream: merge request (tristan/remove-blessings-1.2->bst-1.2: _frontend/status.py: Completely remove the blessings dependency from BuildStream) #809 changed state ("merged"): https://gitlab.com/BuildStream/buildstream/merge_requests/809 | 11:10 |
tristan | bochecha, blessings removed ! will be in 1.2.1 this week \o/ | 11:10 |
bochecha | tristan: great :) | 11:11 |
tristan | Code Quality report https://gitlab.com/BuildStream/buildstream/merge_requests/809 is totally bogus, and reports a huge amount of things unrelated to the patch | 11:11 |
tristan | I continue to believe something is screwed up inside that black box | 11:11 |
tiagogomes | "Code quality improved on 35 points and degraded on 279 points" | 11:12 |
flatmush | Is there a way to add to the CFLAGS variable in "environment:" rather than fully replacing it? | 11:12 |
* tiagogomes gets the calculator | 11:12 | |
tiagogomes | flatmush "CFLAGS="$CFLAGS --even-more-cflags" ? | 11:13 |
tristan | tiagogomes, Expand the list and it shows a huge amount of lines of code that are not modified by the patch | 11:13 |
tristan | flatmush, sec... | 11:13 |
flatmush | tiagogomes: Tried that obviously, it's just directly putting "$CFLAGS" in the string though | 11:14 |
tiagogomes | So bst is disabling variable expansion I guess | 11:14 |
tristan | Ah that | 11:15 |
WSalmon | depending on context but %{CFLAGS} if you want to refure to bulidstream variable, never hested before tho flatmush | 11:15 |
WSalmon | *neasted | 11:15 |
tristan | flatmush, No there is nothing for that unfortunately | 11:15 |
flatmush | WSalmon: Just get unresolved variabe, but thanks | 11:16 |
tristan | flatmush, You would need to decompose what you want to be used to construct CFLAGS in the related project | 11:16 |
flatmush | tristan: Ok, I'll make a feture request | 11:17 |
flatmush | I just need to add "-std=c90" | 11:17 |
tristan | flatmush, only tangentially related but, observe how we have been handling configure flags for example: http://buildstream.gitlab.io/buildstream/elements/autotools.html | 11:18 |
tristan | flatmush, Note we use a combination of `conf-local` and `conf-global` here, to allow (A) The project to set conf-global options for all autotools elements, (B) The element to add conf-local options | 11:19 |
tristan | Those are applied at the end of the `conf-args` variable | 11:19 |
tristan | flatmush, Similarly, one could have a project.conf define a 'cflags-global' variable, have elements define a 'cflags-local' variable, and in the project.conf, set CFLAGS: "%{cflags-global} %{cflags-local}" | 11:21 |
tristan | That way, you define any project wide flags in project.conf, through the %{cflags-global} var, and elements can set %{cflags-local} | 11:21 |
tristan | in this case, you might not need -global / -local... just define in project.conf CFLAGS: "--global --c-flags %{cflags}" | 11:22 |
paulsher1ood | toscalix: 'cassock'? | 11:25 |
gitlab-br-bot | buildstream: merge request (tristan/remove-blessings->master: _frontend/status.py: Completely remove the blessings dependency from BuildStream) #808 changed state ("merged"): https://gitlab.com/BuildStream/buildstream/merge_requests/808 | 11:27 |
jjardon | flatmush: see how we have fixed it in freedesktop-sdk: https://gitlab.com/freedesktop-sdk/freedesktop-sdk/issues/312 -> https://gitlab.com/freedesktop-sdk/freedesktop-sdk/merge_requests/438 | 11:29 |
*** lachlan has joined #buildstream | 11:29 | |
tristan | jjardon, Assign it on command line ? It's an approach, I like mine a bit better :) | 11:30 |
gitlab-br-bot | buildstream: merge request (jmac/remote_exec_checkout_fix->master: Remote exec: Remove early warning and check directory is not None) #800 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/800 | 11:31 |
tristan | jjardon, Curiously, where is CFLAGS originally coming from ? | 11:31 |
tristan | or LDFLAGS ? | 11:31 |
jjardon | tristan: I think I like your better too :) | 11:31 |
jjardon | from project.conf | 11:31 |
jjardon | https://gitlab.com/freedesktop-sdk/freedesktop-sdk/blob/18.08/project.conf#L97 | 11:31 |
tristan | jjardon, https://gitlab.com/freedesktop-sdk/freedesktop-sdk/blob/master/project.conf#L84 | 11:32 |
tristan | yep same link :) | 11:32 |
tristan | or similar one | 11:32 |
tristan | jjardon, So taking CPPFLAGS for example, instead of "-O2 -D_FORTIFY_SOURCE=2", you could just do "-O2 -D_FORTIFY_SOURCE=2 %{cppflags-extra}" | 11:33 |
tristan | jjardon, Then any element has the choice, (A) Overwrite CPPFLAGS entirely (cause the base ones maybe dont work here) ? Or (B) Set additional flags in %{cppflags-extra} variable | 11:34 |
tristan | jjardon, I missed the line where you liked my approach better, and continued typing :D | 11:36 |
tristan | Have to convince ! | 11:36 |
tristan | hehe | 11:36 |
jjardon | also per arch some flags can not work, so it's better to have a flexible and "blessed" way to do this | 11:37 |
tristan | Right, so in the same vein, you can have arch conditionals on either the environment or variables in project.conf, as well as in the element.bst files | 11:38 |
tristan | So if you decompose into variables first, you can avoid many similar declarations | 11:39 |
* tpollard is amused with conclave & cassock | 11:39 | |
tristan | CFLAGS: "%{base-cflags} %{arch-specific-cflags} %{extra-cflags}" | 11:39 |
tristan | jjardon, ^^ like that ? | 11:40 |
gitlab-br-bot | buildstream: merge request (jmac/remote_exec_checkout_fix->master: Remote exec: Remove early warning and check directory is not None) #800 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/800 | 11:40 |
jjardon | tristan: looks good. bochecha what do you think ? ^ | 11:40 |
mablanch | juergbi: Do you know what is the minimum bubblewrap version required for BuildBox? Is it written somewhere? | 11:44 |
toscalix | paulsher1ood: cassock... in case people take the word conclave too seriously | 11:44 |
toscalix | isn't a cassock what the bishop wears ? | 11:45 |
juergbi | mablanch: I think it's 0.1.8 but could probably be lowered by making --die-with-parent optional, same as BuildStream | 11:46 |
juergbi | should indeed document this | 11:46 |
* paulsher1ood missed the joke, but fine | 11:47 | |
mablanch | juergbi: ok, cheers! | 11:48 |
tristan | mablanch, juergbi; I believe if you git blame the line of code in _platform/linux.py which asks about 0.1.8, you may find a commit pointing back to an issue... | 11:49 |
tristan | And I think that issue had some fair research already done as to which options were supported in which version | 11:50 |
mablanch | tristan: Thanks, --die-with-parent has indeed been introduced in 0.1.8 according to bubblewrap release note. | 11:54 |
tristan | Oh | 11:56 |
tristan | mablanch, juergbi; FWIW, the *base* version we require is 0.1.2 in BuildStream | 11:56 |
tristan | according to setup.py | 11:56 |
tristan | qinusty, I'll be heading out soon... and I plan to release tomorrow; Can I expect a backport by then of SkipJob ? | 11:58 |
mablanch | tristan, juergbi: what do you prefer here: bumping required version to 0.1.8 or get ride of the --die-with-parent in BuildBox and keep the 0.1.2 requirement? (0.1.8 was released 28 Mar 2017, debian stable has 0.1.7 though...) | 12:02 |
juergbi | --die-with-parent should definitely not be just removed. we either make it optional or require bwrap 0.1.8 for buildbox | 12:04 |
mablanch | juergbi: Sure, I was thinking of either a CLI option or having BuildBox systematically using it if bubblewrap supports it (would require version detection, maybe build time version check would be enough). | 12:07 |
mablanch | As, most probably, one would want to use --die-with-parent if available. | 12:08 |
juergbi | yes, the latter | 12:08 |
juergbi | same as buildstream | 12:08 |
* paulsher1ood wonders if he's alone in finding that option name to be quite unsettling | 12:09 | |
gitlab-br-bot | buildstream: merge request (tpollard/494->master: WIP: Don't pull artifact buildtrees by default) #786 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/786 | 12:09 |
tlater[m] | paulsher1ood: BuildStream is a bit more ethical - we only kill zombies | 12:10 |
paulsher1ood | :) | 12:10 |
juergbi | without context it's indeed an odd name. bubblewrap developers chose that | 12:11 |
tiagogomes | tlater[m] Ive reasons to believe some zombies are escaping though, at least when running on the context of the tests | 12:18 |
tlater[m] | tiagogomes: I'd think you're right. I've noticed issues with that too, just not had time to look into it in detail yet. | 12:19 |
qinusty | Yes tristan, sorry I was out for lunch. I'll backport today | 12:19 |
tristan | qinusty, Great :) | 12:21 |
tristan | Interestingly, these are parents chasing zombies, who formerly were their children | 12:23 |
*** tristan has quit IRC | 12:29 | |
gitlab-br-bot | buildstream: merge request (Qinusty/skipped-rework-backport-1.2->bst-1.2: Backport skipped reword (!765)) #810 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/810 | 12:30 |
qinusty | Nice work on the CONTRIBUTING.rst adds68 https://imgur.com/8iWzgA6 | 12:33 |
adds68 | qinusty, oh nice! Thanks haha | 12:34 |
bochecha | jjardon: I had originally implemented the %{cppflags-extra} thing locally | 12:34 |
bochecha | jjardon: I stopped because it caused lots of churn for just a few rare elements needing it | 12:34 |
qinusty | Weird how gitlab doesn't recognise HACKING(.rst|.md) as the same thing | 12:35 |
bochecha | as in, you have to define those variables first in project.conf (and bootstrap/project.conf) because Buildstream requires variables to be defined (even if empty), and then elements to change, … | 12:35 |
bochecha | jjardon: it just felt like too much work for very few gain | 12:36 |
bochecha | jjardon: I found some of our elements were passing the extra flags to the command line and found that it had a much better work:benefit ratio | 12:36 |
bochecha | it's less elegant, yes | 12:36 |
*** lantw44 has quit IRC | 12:36 | |
jjardon | bochecha: yeah, the idea is that if this is supported by bst directly, maybe less churn is needed | 12:36 |
bochecha | jjardon: oh yes, absolutely | 12:36 |
jjardon | and taking in account you and tristan have the same idea, probably we should go for it :) | 12:37 |
bochecha | well, now that I think about it, it's not really the same idea at all… | 12:38 |
bochecha | … I had called the variable cppflags-local /o\ | 12:38 |
bochecha | 😜 | 12:38 |
qinusty | Can I get a review on https://gitlab.com/BuildStream/buildstream/merge_requests/799? | 12:38 |
*** mohan43u has quit IRC | 12:42 | |
*** mohan43u has joined #buildstream | 12:43 | |
gitlab-br-bot | buildstream: merge request (adamjones/contributing-links->master: Fix rst link formatting for guideline links) #811 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/811 | 12:44 |
* adds68 noticed the rst formatting was wrong on the links | 12:45 | |
adds68 | can someone merge the fix please :) | 12:45 |
tpollard | gitlab dead? | 12:46 |
cs-shadow | looks that way | 12:47 |
jmac | I'm seeing a few network problems | 12:47 |
Kinnison | mm 503 | 12:47 |
gitlab-br-bot | buildstream: merge request (adamjones/contributing-links->master: Fix rst link formatting for guideline links) #811 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/811 | 12:50 |
gitlab-br-bot | buildstream: merge request (mac_fixes->master: WIP: Implement compatibility fixes for MacOSX and WSL Blocks #411 and #412") #726 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/726 | 12:56 |
finn | For some reason, searching the docs for me is taking a really long time | 12:57 |
finn | Gets hung at "Searching..." | 12:58 |
tlater[m] | finn: Did anyone actually implement searching? | 12:59 |
* tlater[m] thinks that doesn't work by default | 12:59 | |
finn | I'm sure that's worked in the past | 12:59 |
finn | https://buildstream.gitlab.io/buildstream/ | 13:00 |
finn | the search docs box in the top left | 13:00 |
gitlab-br-bot | buildstream: merge request (adamjones/contributing-links->master: Fix rst link formatting for guideline links) #811 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/811 | 13:01 |
finn | so tlater[m], you're saying this is no longer implemented? | 13:06 |
tlater[m] | finn: I recall jjardon mentioning it didn't work in the past :) | 13:06 |
tlater[m] | iirc, for this to work the server actually needs to do some work in the backend (because search indexing isn't usually done in the frontend) | 13:07 |
*** lantw44 has joined #buildstream | 13:07 | |
tlater[m] | And since we host it statically it shouldn't work. | 13:07 |
tlater[m] | But I'm happy to be corrected. | 13:07 |
adds68 | i've used it in the past for search CAS related docs | 13:08 |
finn | I swear I've used it before | 13:08 |
finn | I've used it to search for elements | 13:08 |
tlater[m] | adds68: Do you mean the CAS in the *buildstream* docs? | 13:08 |
adds68 | yea | 13:09 |
phildawson | I've definitely used the search at https://buildstream.gitlab.io/buildstream/ | 13:09 |
tlater[m] | Well I stand corrected then | 13:09 |
finn | It's down on BuildGrid too tbf | 13:09 |
finn | wonder if this is a gitlab issue | 13:09 |
tlater[m] | finn: Have you tried clearing your browser cache? | 13:09 |
tlater[m] | finn: It don't think it can be if the page is displayed at all. | 13:10 |
finn | I have | 13:10 |
*** lantw44 has quit IRC | 13:12 | |
*** lachlan has quit IRC | 13:14 | |
*** lachlan has joined #buildstream | 13:21 | |
adds68 | jjardon, can you re approve the merge please: https://gitlab.com/BuildStream/buildstream/merge_requests/811 ? | 13:25 |
gitlab-br-bot | buildstream: merge request (adamjones/contributing-links->master: Fix rst link formatting for guideline links) #811 changed state ("merged"): https://gitlab.com/BuildStream/buildstream/merge_requests/811 | 13:25 |
tlater[m] | o\ | 13:35 |
* tlater[m] didn't notice tiagogomes shiny new gitlab issues link thing | 13:35 | |
tiagogomes | On the Website? That would be Valentin | 13:36 |
tlater[m] | Well, in any case I suppose I'll stick to reviewing that one instead of pushing up mine. | 13:36 |
*** lachlan has quit IRC | 13:37 | |
*** lantw44 has joined #buildstream | 13:49 | |
*** lachlan has joined #buildstream | 13:50 | |
gitlab-br-bot | buildstream: merge request (willsalmon/outOfSourecBuild->master: Out of source builds) #776 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/776 | 13:57 |
flatmush | Anyone know why baserock used SYSLINUX rather than grub for the minimal system? | 14:00 |
gitlab-br-bot | buildstream: issue #657 ("Setup our own x86_64 runners") changed state ("closed") https://gitlab.com/BuildStream/buildstream/issues/657 | 14:01 |
qinusty | Is it intentional that I am able have config: configure-commands set for `import` kind despite it doing nothing? | 14:06 |
paulsher1ood | skullman: do you recall anything about why baserock used syslinux rather than grub for minimal? | 14:08 |
paulsher1ood | or Kinnison ? | 14:08 |
*** lachlan has quit IRC | 14:09 | |
Kinnison | Easier to get running iirc | 14:09 |
skullman | paulsher1ood: smaller and simpler | 14:09 |
tlater[m] | qinusty: I don't think so. That should at least warn users. File an issue? | 14:09 |
paulsher1ood | flatmush: ^^ | 14:09 |
paulsher1ood | thanks chaps | 14:09 |
lchlan | - tristan - I have been looking at the comments that you raised regarding the yaml cache testing and was wondering if you are thinking of this as a broader re-working of yaml.py tests? | 14:10 |
*** lachlan has joined #buildstream | 14:10 | |
*** lachlan has joined #buildstream | 14:11 | |
*** lachlan has quit IRC | 14:14 | |
*** lachlan has joined #buildstream | 14:14 | |
*** lachlan has quit IRC | 14:16 | |
*** lachlan has joined #buildstream | 14:16 | |
*** lachlan has quit IRC | 14:18 | |
*** lachlan has joined #buildstream | 14:18 | |
gitlab-br-bot | buildstream: issue #662 ("No warning produced for import element with configure-commands set") changed state ("opened") https://gitlab.com/BuildStream/buildstream/issues/662 | 14:21 |
gitlab-br-bot | buildstream: merge request (jonathan/pickle-yaml->master: WIP: Add a cache of parsed and provenanced yaml) #787 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/787 | 14:22 |
* qinusty hates bisecting through the "PRE_CAS_MERGE_JULY_2018" problems | 14:23 | |
*** lachlan has quit IRC | 14:27 | |
gitlab-br-bot | buildstream: merge request (jmac/remote_exec_checkout_fix->master: Remote exec: Remove early warning and check directory is not None) #800 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/800 | 14:27 |
jjardon | paulsher1ood: BTW, the new cool thing is systemd-boot (much more simpler and smaller than grub, with a lot less features as well, of course) | 14:28 |
jjardon | tiagogomes: what is you gnome wiki account? | 14:31 |
tiagogomes | I PMed it to you | 14:32 |
flatmush | jjardon: Yeah, ctgriffiths mentioned that to me earlier, so we might try that | 14:34 |
*** lachlan has joined #buildstream | 14:36 | |
*** finn has quit IRC | 14:40 | |
tlater[m] | juergbi: Does this description of the CAS change seem ok? https://gitlab.com/BuildStream/website/merge_requests/89/diffs | 14:40 |
gitlab-br-bot | buildstream: merge request (jonathan/pickle-yaml->master: WIP: Add a cache of parsed and provenanced yaml) #787 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/787 | 14:40 |
*** lachlan has quit IRC | 14:41 | |
*** rdale has quit IRC | 14:43 | |
tiagogomes | tlater[m] left a comment | 14:43 |
tiagogomes | An artifact created for a stack element, does it contains the files of the stacked artifacts or is just purely metadata? | 14:44 |
juergbi | tlater[m]: lgtm. we could also mention that it also replaces the tar cache on non-Linux, although there were probably not too many users | 14:44 |
* paulsher1ood hopes it's just the metadata | 14:44 | |
* tiagogomes points that if its not, the CAS deduplication feature kicks in | 14:45 | |
tiagogomes | There's still some overhead of course | 14:45 |
tlater[m] | tiagogomes: It only contains metadata :) | 14:45 |
juergbi | tlater[m]: i.e., maybe the last point should be tweaked. BuildStream core worked before as well but it was slower / no deduplication (tar cache) and didn't support remote artifact servers | 14:46 |
persia | In this case overhead == metadata, I think | 14:46 |
*** finn has joined #buildstream | 14:46 | |
tlater[m] | juergbi: Yes, I think that's a better comment | 14:46 |
* tlater[m] amends it | 14:46 | |
juergbi | ta | 14:46 |
*** abderrahim has quit IRC | 14:46 | |
juergbi | tlater[m]: btw: I'd mention TLS, not SSL, to be precise | 14:47 |
*** abderrahim has joined #buildstream | 14:47 | |
tlater[m] | Also a good point! x) | 14:47 |
tlater[m] | toscalix: The CAS paragraph is ready if you would like to approve it: https://gitlab.com/BuildStream/website/merge_requests/89 | 14:49 |
tlater[m] | Alternatively I think qinusty can approve? | 14:49 |
* qinusty is here | 14:49 | |
qinusty | Will take a quick look | 14:50 |
laurence | tpollard, just re-reading your email to the list summarising the workspace DX design changes | 14:51 |
laurence | good email, btw :) | 14:51 |
laurence | I am wondering if we want to find all of the old gitlab issues, close them down, raise new ones which also summarise, and point to them? | 14:51 |
laurence | I think I'd prefer that, for neatness. Interested in opinions of others | 14:51 |
tiagogomes | tlater[m], did OSTree handle file deduplication across artifacts built from different elements? | 14:52 |
juergbi | yes | 14:52 |
tlater[m] | tiagogomes: Yep | 14:52 |
tpollard | laurence: I think that's probably the right thing to do for existing tickets which are covered by it | 14:53 |
tiagogomes | neat | 14:53 |
tlater[m] | OSTree was pretty good, it was just a bit awkward to use. | 14:53 |
tlater[m] | (The awkwardness stemming completely from it not being designed for the purpose we used it for) | 14:53 |
laurence | tpollard, alright, let's get that done and maybe a ping to the list as an update | 14:54 |
tpollard | laurence: one we've commented in the respective tickets with what is happening should it be down to the original reporter to close it? | 14:58 |
laurence | tpollard, my view is that you can close it. if the original person disagrees then they can always re-open | 14:59 |
*** lachlan has joined #buildstream | 14:59 | |
laurence | with a note explaining the reasons then I think it's pretty clear cut, really | 14:59 |
tpollard | Yeh fair enough | 15:00 |
tlater[m] | qinusty: I added that link - forgot to fill the placeholder | 15:03 |
qinusty | :D | 15:03 |
qinusty | Also, follow the theme toscalix set with links at the bottom etc | 15:03 |
tlater[m] | qinusty: I'm not aware of this theme, is there a file that exemplifies it well? I don't see any other pages with an explicit links section or anything. | 15:06 |
* tlater[m] assumes it's some sort of academic-looking references style | 15:06 | |
qinusty | Who knows, something to do with journalists being able to extract links from the page easily? Basically, you `[link text]` inline | 15:07 |
qinusty | and `[link text]: url` at the bottom | 15:07 |
qinusty | It's used within that file you were editing | 15:07 |
tlater[m] | Oh, I completely missed that, whoops | 15:09 |
tlater[m] | qinusty: It should now have a link to Bazel CAS there | 15:10 |
qinusty | You missed one ;) | 15:11 |
qinusty | Inline link is unnecessary | 15:12 |
*** lachlan has quit IRC | 15:12 | |
*** lachlan has joined #buildstream | 15:13 | |
tlater[m] | ... | 15:13 |
* tlater[m] needs to read the markdown specification for this wiki | 15:14 | |
tlater[m] | qinusty: I think that should do it - sorry, I'm really not used to pelican's markdown format. | 15:16 |
tlater[m] | I take it that's what that [TOC] thing at the top of some files is, too? | 15:16 |
qinusty | No worries, [TOC] is a plugin but the links are actually valid markdown. | 15:16 |
qinusty | We could do links in lots of different ways, this way was settled on at one point | 15:17 |
* qinusty personally disagrees with it. But we're sticking with it now :D | 15:18 | |
tlater[m] | And here I thought I knew the markdown spec well | 15:18 |
qinusty | Yeah there are 2-3 ways to link in markdown. It's weird | 15:18 |
tlater[m] | Documentation for links is here for anyone interested: https://daringfireball.net/projects/markdown/syntax#link | 15:19 |
qinusty | my favourite is the reference style. where you put variables inline. | 15:19 |
tlater[m] | I think it's actually kinda neat, as someone who's tried writing academic reports in markdown | 15:19 |
qinusty | E.g. `[link text][ref]` and `[ref]: url` | 15:19 |
qinusty | That's my preferred academic report format | 15:19 |
qinusty | Where the ref is shared rather than the text | 15:19 |
* qinusty plans to write his dissertation in markdown and have it build in gitlab CI | 15:20 | |
tlater[m] | qinusty: When I did so I insisted on using a latex pre-processor and bibtex to support references :) | 15:20 |
* qinusty Notes that he found this https://github.com/tompollard/phd_thesis_markdown | 15:21 | |
qinusty | not tpollard | 15:21 |
tpollard | ha! | 15:22 |
qinusty | I tried using the pandoc docker container with his make files though and ran into strange behaviour where `docker ... run make ...` would fail and `docker ... run /bin/bash` and running the script inside would work :( | 15:23 |
* tpollard sadly wrote his dissertation on MS Word | 15:23 | |
tlater[m] | Shame! ;p | 15:24 |
qinusty | :O | 15:24 |
qinusty | I was going to do latex, but for readability. Markdown wins every time | 15:24 |
qinusty | And with the preprocessor, you can get the desired features | 15:25 |
tlater[m] | qinusty: The main reason I went back to latex is that those setups are a bit unstable and not very reproducible. | 15:25 |
tlater[m] | I suppose I could build my report using buildstream though... | 15:25 |
qinusty | You /could/ dockerize | 15:25 |
qinusty | An image which can build your papers easy | 15:26 |
toscalix | qinusty: tlater[m] the number one reason for the current link formatting is the content review | 15:26 |
tlater[m] | Then I need to maintain a docker image though. | 15:26 |
toscalix | the reason related with copy-paste contents was an additional one, not the main one. The main one was provided by tiagogomes and I agreed | 15:26 |
tlater[m] | toscalix: Content review? | 15:26 |
toscalix | then a little modification was proposed by tristan and accepted | 15:27 |
toscalix | code review <=> content review | 15:27 |
*** lachlan has quit IRC | 15:27 | |
tlater[m] | Is it because it's easier to check if all the links work by going down a list at the bottom? | 15:28 |
toscalix | it is easier to read the text you are reviewing. | 15:28 |
toscalix | when you do not have the links inserted in the text itself | 15:29 |
tlater[m] | Ah, I understand. That's fair enough, yes, the raw text looks a lot nicer. | 15:30 |
*** lachlan has joined #buildstream | 15:34 | |
*** lachlan has quit IRC | 15:43 | |
*** lachlan has joined #buildstream | 15:45 | |
abderrahim | Hi. Any idea how I can debug this error? | 15:55 |
abderrahim | https://gitlab.com/freedesktop-sdk/freedesktop-sdk/-/jobs/99169353 | 15:55 |
abderrahim | I've tried opening a shell at the error prompt, but running sh or echo works fine | 15:56 |
qinusty | Anyone looking to bisect in future, I made https://gitlab.com/BuildStream/buildstream/snippets/1755329. | 16:02 |
qinusty | Automated bisecting can allow you to run some beefy tests through a bisect without having to sit and wait | 16:03 |
*** lachlan has quit IRC | 16:03 | |
qinusty | Not too sure abderrahim, perhaps adds68 could be of use here. | 16:06 |
*** lachlan has joined #buildstream | 16:07 | |
*** toscalix has quit IRC | 16:14 | |
*** lachlan has quit IRC | 16:18 | |
gitlab-br-bot | buildstream: merge request (tpollard/494->master: WIP: Don't pull artifact buildtrees by default) #786 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/786 | 16:23 |
tlater[m] | qinusty: What do you think is still missing from the messaging api branch? | 16:29 |
tlater[m] | Obviously the weird context manager will have to go, but it seems pretty done otherwise. | 16:30 |
tlater[m] | Hmm... I guess `job.py` is still duplicating API... | 16:32 |
qinusty | Good question... It'll need a rebase obviously. Context manager can go or perhaps rethink how we can standardise the BUG thing. | 16:32 |
qinusty | job.py duplicates to make things easier to work with imo. | 16:32 |
qinusty | I'd say that's a valid duplication, they need extra kwargs | 16:32 |
qinusty | And if person x comes along, they might not be that aware of job.py to add the kwarg | 16:33 |
skullman | fascinating… it looks like the reason I was unable to nest bubblewrap in bubblewrap while mounting --proc was because I was running it as root | 16:33 |
skullman | I'm not sure what the chain of logic that causes it to end up refusing to mount proc in the inner bubblewrap instance is… possibly the outer bubblewrap's proc isn't flagged as locked | 16:34 |
tlater[m] | qinusty: As I said in my original commit, I would like to see a world in which `job.py` isn't special, though admittedly it's very difficult to get that done without needing to add that kwarg for every call... | 16:34 |
qinusty | My point precisely, if you can think of a neat way to deal with it. Go ahead :D | 16:35 |
qinusty | I just didn't see a neat alternative | 16:35 |
tlater[m] | Haha, that was my main issue before you took it on :) | 16:35 |
tlater[m] | Yep, looks like rebasing and removing that weird contextmanager thing will do it | 16:36 |
tlater[m] | qinusty: Why did you add that contextmanager in the first place? | 16:36 |
qinusty | The goal was to create a singular place in which BUG messages were created. | 16:37 |
qinusty | Currently they're all over the place and not consistent at all | 16:37 |
qinusty | Varied messages are produced throughout master currently | 16:37 |
tlater[m] | Hmm, I think that's simply a side effect of what BUG is used for | 16:38 |
* tlater[m] takes a look at the exact messages | 16:39 | |
tlater[m] | Ah, I see - in some cases people use the traceback as detail, in others something arbitrary | 16:40 |
tlater[m] | It makes some sense to point out to people that the traceback is eaten, but I think that burden should be on maintainers, not the messaging API - if it makes sense to have something arbitrary in some contexts, then that should be possible. | 16:41 |
tlater[m] | qinusty: Mind if I try and rebase this branch for you? I'm waiting on some builds to finish, and this is the next priority on my list. | 16:42 |
qinusty | sure | 16:48 |
qinusty | go ahead tlater[m], I won't really have the time tomorrow | 16:48 |
*** iker has quit IRC | 16:54 | |
gitlab-br-bot | buildstream: merge request (Qinusty/634-workspace-failed-builds->master: WIP: Do not save workspace on failed build) #812 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/812 | 16:55 |
gitlab-br-bot | buildstream: merge request (Qinusty/634-workspace-failed-builds->master: WIP: Do not save workspace on failed build) #812 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/812 | 16:55 |
qinusty | WSalmon, the MR above contains the fix. I still need to clean up and add my tests to the branch. It looks okay for the issue I saw. Can you try it on your issue at some point? | 16:56 |
*** jonathanmaw has quit IRC | 17:03 | |
*** xjuan has joined #buildstream | 17:07 | |
*** finn_ has joined #buildstream | 17:22 | |
*** finn has quit IRC | 17:23 | |
*** finn_ has quit IRC | 17:26 | |
*** tiagogomes has quit IRC | 17:45 | |
*** finn has joined #buildstream | 17:49 | |
*** finn has quit IRC | 18:04 | |
valentind | jjardon, I think if we wanted to make debian packages using buildstream out of debian source packages then we need to write new plugins. | 18:18 |
valentind | Some source plugin for downloading orig source with the watch file. Some source plugin to download packages through Build-Depends of the control. And then an element plugin calling debuild | 18:19 |
*** lachlan has joined #buildstream | 18:29 | |
*** lachlan_ has joined #buildstream | 19:27 | |
persia | valentind: On watch files: better to use the get-orig-source: target in debian/rules than rely on the watch file providing enough information. uscan(1) is often used in that rule, but not always (especially for content that needs to be scrubbed to be dfsg-free) | 19:34 |
*** lachlan_ has quit IRC | 19:37 | |
*** lachlan has quit IRC | 19:37 | |
valentind | persia, I find that most of packages actually do not provide get-orig-source. But maybe things have changed. | 19:42 |
valentind | But for our case I think we can just limit ourselves. Not a problem. You can even use other source plugins to download the orig tarball. | 19:43 |
valentind | It is just that testing the watch file is nice. | 19:43 |
*** finn has joined #buildstream | 20:07 | |
*** tristan has joined #buildstream | 20:13 | |
*** finn has quit IRC | 20:36 | |
persia | valentind: Sadly, you might be correct. For BuildStream purposes, I think it makes sense to try a number of common mechanisms in sequence, selecting the next only if the first fails/is impossible. So, get-orig-source, uscan, pull from upstream VCS, etc. | 20:48 |
persia | On the other hamd, the more I think about it, the more I think it isn't acutally particularly useful. If one is prepared to track upstream, one probably shouldn't import a debian-format source package. If one wants to import from a debian-style distro, it's *much* better not to advance things past the maintained versions with the same packaging: too mamy of the "changr this when" rules for maintainers aren't easliy machine-transcribable. | 20:51 |
*** tristan has quit IRC | 21:53 | |
*** WSalmon has quit IRC | 22:04 | |
*** WSalmon has joined #buildstream | 22:04 | |
*** alatiera_ has quit IRC | 22:12 | |
*** bochecha has quit IRC | 23:26 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!