IRC logs for #buildstream for Wednesday, 2018-09-19

*** alatiera_ has quit IRC00:25
*** xjuan has quit IRC00:58
*** mohan43u has quit IRC06:39
*** bochecha has joined #buildstream06:51
*** mohan43u has joined #buildstream06:54
*** tristan has joined #buildstream06:58
*** ChanServ sets mode: +o tristan07:01
*** finn has joined #buildstream07:38
*** toscalix has joined #buildstream07:49
*** tristan has quit IRC07:52
*** tiagogomes has joined #buildstream07:52
*** iker has joined #buildstream08:04
gitlab-br-botbuildstream: merge request (willsalmon/outOfSourecBuild->master: WIP: out of source builds) #776 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/77608:09
*** rdale has joined #buildstream08:30
tiagogomestoscalix where should information about the BuildStream infrastructure should be placed? This can be public, as it will not contain access details08:33
jjardontiagogomes: https://wiki.gnome.org/Projects/BuildStream/Infrastructure maybe?08:36
gitlab-br-botbuildstream: 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/79908:36
tiagogomesIs the Wiki still relevant? I know some things migrated to the website08:39
toscalixhi tiagogomes08:42
toscalixwas in a meeting08:42
toscalixI assume that we will have scripts and other config stuff that08:42
gitlab-br-botbuildstream: merge request (willsalmon/outOfSourecBuild->master: WIP: out of source builds) #776 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/77608:43
tiagogomesNot really. Perhaps just one configuration file08:44
toscalixwill need to be in a repo08:45
toscalixso I would create a new project and add the info as .md file08:45
toscalixI would add it to nosoftware subgroup08:46
toscalixso we do not increase the problem we have with so many repos on the root tree08:47
toscalixwhat do you think?08:47
toscalixtiagogomes: ?08:48
gitlab-br-botbuildstream: issue #642 ("Incorrect error when malformed project.conf is found") changed state ("closed") https://gitlab.com/BuildStream/buildstream/issues/64208:48
gitlab-br-botbuildstream: 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/79208:48
gitlab-br-botbuildstream: merge request (chandan/fix-source-bundle->master: Fix source-bundle command) #807 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/80708:51
tiagogomestoscalix I think it could be overkill. This will only be a small page stating which machines are available, and their purpose08:52
*** jonathanmaw has joined #buildstream08:54
gitlab-br-botbuildstream: merge request (willsalmon/outOfSourecBuild->master: WIP: out of source builds) #776 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/77608:56
skullmanjuergbi: are you able to and should https://gitlab.com/richardmaw-codethink/buildstream-integration be moved under the buildstream/ project group?08:57
toscalixtiagogomes: then I would go for the jjardon suggestion until is stop being overkill08:57
juergbiskullman: 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 group08:59
juergbiI can create an empty buildstream-integration repo if that helps08:59
skullmanjuergbi: there is a feature to move things, but it's no great loss to just push it09:00
juergbithere is export and import support. I don't see anything about moving09:02
juergbihowever, I don't have sufficient privileges for the existing repo to export09:02
skullmanadded you as a maintainer09:03
juergbiskullman: export was successful but import was not: no useful error message: "Error importing repository  into BuildStream/buildstream-integration - "09:07
juergbimaybe simply push with git09:07
skullmanyeah, it's just 1 MR that'll be lost09:07
* skullman doesn't have permisison to create it09:08
*** lachlan has joined #buildstream09:12
gitlab-br-botbuildstream: 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/72609:16
*** tristan has joined #buildstream09:19
gitlab-br-botbuildstream: 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/79909:21
gitlab-br-botbuildstream: 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/79909:22
gitlab-br-botbuildstream: merge request (willsalmon/outOfSourecBuild->master: Out of source builds) #776 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/77609:34
gitlab-br-botbuildstream: merge request (willsalmon/outOfSourecBuild->master: Out of source builds) #776 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/77609:34
gitlab-br-botbuildstream: merge request (Qinusty/skipped-rework->master: Add SkipJob for indicating a skipped activity) #765 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/76509:37
gitlab-br-botbuildstream: 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/79909:44
gitlab-br-botbuildstream: 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/80809:55
gitlab-br-botbuildstream: 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/80909:57
*** ChanServ sets mode: +o tristan10:00
tristantiagogomes, Care to give a lookover on https://gitlab.com/BuildStream/buildstream/merge_requests/808 ?10:00
tristanqinusty, I see you are updating the branches for SkipJob... is it ready for another review ?10:01
qinustyIt is :) Thanks for the review10:01
gitlab-br-botbuildstream: 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/72610:02
* tristan takes a look10:02
qinustyI agree with the early return points, I hadn't considered it since I was removing the returns all together.10:02
qinustyIt does help avoid unnecessary indents though10:02
tristanqinusty, That is mostly a matter of preserving valuable history10:02
tristanHowever, I agree, and I recall originally making early returns because of obnoxious indentation10:03
tristanqinusty, To be honest though, I consider that, in a purist sense... early returns are generally bad, excepting for argument checking and the like10:04
tristanCode is generally easier to debug and trace when a function has only one return10:05
qinustyHmmm, I'm torn on this. I previously worked with a coding standard which banned early returns in all cases. You need a balance really imo10:05
tristanqinusty, I agree and disagree about balance :)10:05
tristanHeh10:05
*** lachlan has quit IRC10:06
tristanqinusty, Problem is that when you have any ambiguity, everyone has a different perception of what to choose10:06
qinustyEarly returns within the first few lines is acceptable since it's mostly just performing early checks.10:06
tristanI rather agree with that, indeed (and consider it falls within the category of "argument checking and the like")10:07
qinustyThat is where I'd draw the line with balance :D10:07
qinustyReturning in the middle of a function isn't great. (but I've seen it in buildstream)10:07
tristanWell... ya know...10:08
tristanqinusty, You're more than welcome to untangle https://gitlab.com/BuildStream/buildstream/blob/master/buildstream/element.py#L105410:08
* tristan snickers10:08
* qinusty sticks his head in the sand10:08
tristanThat 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 juergbi10:09
tristanI am never confident about any change I make to that function10:09
qinustyI 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 elif10:10
tristanIf I had to fix breakage there, I would step back, use paper and pen, and take a week to completely rewrite10:10
tristanqinusty, That appears to be the place where we discover the strong cache key from a pulled artifact, which was pulled by it's weak key10:11
jmacqinusty: I think the comment is still valuable, and the interpreter should skip it10:12
tristanIn 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 happen10:12
tristanMaybe we're talking about the elif *above* the pass10:13
tristanqinusty, I believe that juergbi put a couple of these in that function to maintain some level of clarity10:13
tristanqinusty, At least all the cases are specced out, such that we don't forget them, even if we don't *do* anything for every case10:13
* juergbi would like to remove one complicating aspect by changing the way we handle workspace cache keys10:14
juergbiand then maybe more things can be simplified10:15
qinustytristan, on the topic of early returns10:15
qinustystatus.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 IRC10:21
tristanjuergbi, I'm not sure what you would propose in how we handle workspaces; the way things are implemented are there because we reduce builds10:22
juergbiI would like to keep the build output completely separate from the workspace source directory10:23
tristanjuergbi, How ?10:24
tristanqinusty, technically I could use for / break / else there (which is a cool little python construct), would make the code a bit more wordy10:25
juergbitristan: with FUSE it should definitely be possible. not sure whether it's possible without as unprivileged user10:26
tristanjuergbi, 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 tree10:26
*** mohan43u has quit IRC10:27
tristanMaybe with the fuse thing indeed, sounds very complicated10:27
juergbioverlayfs would easily solve it but I don't think Linux mainline allows this for unprivileged users, so probably would have to be FUSE10:27
tristanjuergbi, I think that would simplify things, but I also think it is orthogonal to the code construct of _update_state() though10:27
gitlab-br-botbuildstream: merge request (willsalmon/outOfSourecBuild->master: Out of source builds) #776 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/77610:28
juergbito some extent. there are two big reasons why that thing is complicated, one is non-strict builds and the other is workspaces10:28
juergbiwhile we could likely also find a better structure with the current workspace approach, it's increasing complexity a lot10:29
qinustyIf making it more wordy doesn't affect readability, I'd argue this is worthwhile perhaps?10:29
gitlab-br-botbuildstream: merge request (willsalmon/outOfSourecBuild->master: Out of source builds) #776 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/77610:29
tristanjuergbi, 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
tristanjuergbi, I know it's not a 1:1 comparison, we treat things differently in cases, but those can be classed out into separate functions10:30
*** mohan43u has joined #buildstream10:30
juergbithe support for optionally downloading strict artifacts for already available weak ones + workspaces make cache key handling not completely linear10:31
juergbibut explicitly named states could indeed be useful, if we can come up with understandable names10:32
juergbioriginally the function was much smaller and thus easier to understand10:32
tristanYes I know, I'm not trying to lay out blame man :)10:33
tristanAt the same time, I dont want to under emphasize how much of a concern that function is :)10:34
tristanwe should worry about that structure10:34
juergbisure and I agree10:34
juergbiI 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 structure10:36
*** lachlan has joined #buildstream10:36
juergbias that structure will be much simpler without having to worry about workspaces10:36
tiagogomestristan looking10:37
tristanjuergbi, 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
tristanjuergbi, 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
juergbiit might not be that difficult to do with the buildbox backend10:40
*** alatiera_ has joined #buildstream10:41
tristanqinusty, actually for / break / else doesn't work that way: https://docs.python.org/3/reference/compound_stmts.html#for10:43
tristanqinusty, I was wrong, I actually think it's a bit weird10:43
qinustyin for break else, else is only called when break isn't?10:44
qinustyThat's how I know it10:44
tristanYeah10:44
tristanAnd appears to be executed regardlessly even if break was never called10:44
qinustyElse is called when break isn't.10:45
qinustyWhen break is called, else is skipped.10:45
tristanI mean, that's like for (....) { I did something } else { I did something }10:45
tristanyeah, that doesnt really fit into this loop10:45
qinustyfair enough10:45
tristanI feel like break is when I want to abort... and else is if I aborted, but they disagree... strange10:46
qinustyI guess it's primarily in the case that you're searching for something in a for loop.10:47
tristanI guess it's conducive of... yeah I was thinking that10:47
qinustyYou're searching for x in y: ... else do this10:47
tristanfor (items) { if_I_found_it() { do something with it; break } else { i_didnt_find_it() }10:48
tristanor rather10:48
tristanfor (items) { if_I_found_it() { do something with it; break } } else { i_didnt_find_it()10:48
tristanbah10:48
tristananyway, we see eye to eye :)10:48
toscalixtiagogomes: 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/1110:49
gitlab-br-botbuildstream: 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/72610:49
gitlab-br-botbuildstream: 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/80810:51
tiagogomestiagogomes your MR looks fine, thought I am unsuited to do a proper review as I am not familiar with the curses library :)10:52
tiagogomeserm, s/tiagogomes/tristan10:53
tristantiagogomes, Yeah; and unfortunately it's not CI-able10:53
tristanPart of that interactive stuff that isn't covered10:53
tristanBut, it works :)10:53
qinustyCan I bisect while in a workspace or will I see issues tristan?10:54
tristantiagogomes, 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 purpose10:54
tristantiagogomes, 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 there10: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
tristanqinusty, You should be fine, every time you checkout a different version of something in a workspace, it will update timestamps10:56
*** slaf has joined #buildstream10:56
tristanqinusty, it will however do incremental builds, that might be an issue10:56
tristanqinusty, workspace cache keys are timestamp based, btw10:57
tristanqinusty, So going *back* to an already tried version, will rebuild regardless of whether there is a cached result or not10:57
tristanqinusty, I am very happy with it, I am ready to see it in action !10:58
tristanqinusty, your SkipJob branch, that is :D10:58
qinustyWoop woop10:58
tristanqinusty, 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 lunch10:59
tristanI will release tomorrow or friday10:59
gitlab-br-botbuildstream: merge request (Qinusty/skipped-rework->master: Add SkipJob for indicating a skipped activity) #765 changed state ("merged"): https://gitlab.com/BuildStream/buildstream/merge_requests/76510:59
tristanqinusty, automated bisect ? that I cannot tell you11:00
qinustyYou cna tell bisect to run a script11:00
qinustyand check for exit code11:00
tristanqinusty, I have no idea what you would do if you wanted to automate your bisection in BuildStream11:00
tiagogomesjjardon how do I edit the buildstream wiki on GNOME?11:00
qinustyIt would bisect until it failed my test11:00
qinusty:D11:00
tristanqinusty, So then you added your script/test case to the workspace, and you instruct your element.bst to do this somehow ?11:01
qinustySorry no, bisecting buildstream11:01
tristanAhhhh11:01
qinustyFor a workspace functionality11:01
tristanI see11:01
qinustyIt is trying to save_config from the child process11:01
qinustyand assert is saying nooop11:01
tristantiagogomes, You need to be added to a group of users, GNOME wiki is no longer free for all unfortunately11:02
tiagogomesScrolling back to the top, I tend to avoid "if $condition return", unless the code starts to have too much indentation levels11:03
*** lachlan has quit IRC11:03
jjardonHi! 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 long11:04
jjardontiagogomes: do you have an account in the GNOME wiki?11:04
jjardonyou should see an edit buttin there11:04
tiagogomesShamefully I don't have one11:05
tristanThat 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 it11:06
jjardontiagogomes: ping me back when you have done the account11:06
tristanjjardon, That is pretty much top priority - but won't be fixed in 1.2.1 unfortunately :-(11:07
tristanjjardon, tlater[m] is looking at it, but I think we need someone full time looking into it11:07
jjardontristan: ok, no worries, thanks for the update11:07
tristanThis came up in the meeting yesterday too11:07
gitlab-br-botbuildstream: 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/80811:08
gitlab-br-botbuildstream: 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/80911:10
tristanbochecha, blessings removed ! will be in 1.2.1 this week \o/11:10
bochechatristan: great :)11:11
tristanCode Quality report https://gitlab.com/BuildStream/buildstream/merge_requests/809 is totally bogus, and reports a huge amount of things unrelated to the patch11:11
tristanI continue to believe something is screwed up inside that black box11:11
tiagogomes"Code quality improved on 35 points and degraded on 279 points"11:12
flatmushIs there a way to add to the CFLAGS variable in "environment:" rather than fully replacing it?11:12
* tiagogomes gets the calculator11:12
tiagogomesflatmush "CFLAGS="$CFLAGS --even-more-cflags" ?11:13
tristantiagogomes, Expand the list and it shows a huge amount of lines of code that are not modified by the patch11:13
tristanflatmush, sec...11:13
flatmushtiagogomes: Tried that obviously, it's just directly putting "$CFLAGS" in the string though11:14
tiagogomesSo bst is disabling variable expansion I guess11:14
tristanAh that11:15
WSalmondepending on context but %{CFLAGS} if you want to refure to bulidstream variable, never hested before tho flatmush11:15
WSalmon*neasted11:15
tristanflatmush, No there is nothing for that unfortunately11:15
flatmushWSalmon: Just get unresolved variabe, but thanks11:16
tristanflatmush, You would need to decompose what you want to be used to construct CFLAGS in the related project11:16
flatmushtristan: Ok, I'll make a feture request11:17
flatmushI just need to add "-std=c90"11:17
tristanflatmush, only tangentially related but, observe how we have been handling configure flags for example: http://buildstream.gitlab.io/buildstream/elements/autotools.html11:18
tristanflatmush, 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 options11:19
tristanThose are applied at the end of the `conf-args` variable11:19
tristanflatmush, 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
tristanThat way, you define any project wide flags in project.conf, through the %{cflags-global} var, and elements can set %{cflags-local}11:21
tristanin this case, you might not need -global / -local... just define in project.conf CFLAGS: "--global --c-flags %{cflags}"11:22
paulsher1oodtoscalix: 'cassock'?11:25
gitlab-br-botbuildstream: 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/80811:27
jjardonflatmush: 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/43811:29
*** lachlan has joined #buildstream11:29
tristanjjardon, Assign it on command line ? It's an approach, I like mine a bit better :)11:30
gitlab-br-botbuildstream: 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/80011:31
tristanjjardon, Curiously, where is CFLAGS originally coming from ?11:31
tristanor LDFLAGS ?11:31
jjardontristan: I think I like your better too :)11:31
jjardonfrom project.conf11:31
jjardonhttps://gitlab.com/freedesktop-sdk/freedesktop-sdk/blob/18.08/project.conf#L9711:31
tristanjjardon, https://gitlab.com/freedesktop-sdk/freedesktop-sdk/blob/master/project.conf#L8411:32
tristanyep same link :)11:32
tristanor similar one11:32
tristanjjardon, 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
tristanjjardon, 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} variable11:34
tristanjjardon, I missed the line where you liked my approach better, and continued typing :D11:36
tristanHave to convince !11:36
tristanhehe11:36
jjardonalso per arch some flags can not work, so it's better to have a flexible and "blessed" way to do this11:37
tristanRight, 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 files11:38
tristanSo if you decompose into variables first, you can avoid many similar declarations11:39
* tpollard is amused with conclave & cassock11:39
tristanCFLAGS: "%{base-cflags} %{arch-specific-cflags} %{extra-cflags}"11:39
tristanjjardon, ^^ like that ?11:40
gitlab-br-botbuildstream: 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/80011:40
jjardontristan: looks good. bochecha what do you think ? ^11:40
mablanchjuergbi: Do you know what is the minimum bubblewrap version required for BuildBox? Is it written somewhere?11:44
toscalixpaulsher1ood: cassock... in case people take the word conclave too seriously11:44
toscalixisn't a cassock what the bishop wears ?11:45
juergbimablanch: I think it's 0.1.8 but could probably be lowered by making --die-with-parent optional, same as BuildStream11:46
juergbishould indeed document this11:46
* paulsher1ood missed the joke, but fine11:47
mablanchjuergbi: ok, cheers!11:48
tristanmablanch, 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
tristanAnd I think that issue had some fair research already done as to which options were supported in which version11:50
mablanchtristan: Thanks, --die-with-parent has indeed been introduced in 0.1.8 according to bubblewrap release note.11:54
tristanOh11:56
tristanmablanch, juergbi; FWIW, the *base* version we require is 0.1.2 in BuildStream11:56
tristanaccording to setup.py11:56
tristanqinusty, I'll be heading out soon... and I plan to release tomorrow; Can I expect a backport by then of SkipJob ?11:58
mablanchtristan, 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 buildbox12:04
mablanchjuergbi: 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
mablanchAs, most probably, one would want to use --die-with-parent if available.12:08
juergbiyes, the latter12:08
juergbisame as buildstream12:08
* paulsher1ood wonders if he's alone in finding that option name to be quite unsettling12:09
gitlab-br-botbuildstream: merge request (tpollard/494->master: WIP: Don't pull artifact buildtrees by default) #786 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/78612:09
tlater[m]paulsher1ood: BuildStream is a bit more ethical - we only kill zombies12:10
paulsher1ood:)12:10
juergbiwithout context it's indeed an odd name. bubblewrap developers chose that12:11
tiagogomestlater[m] Ive reasons to believe some zombies are escaping though, at least when running on the context of the tests12: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
qinustyYes tristan, sorry I was out for lunch. I'll backport today12:19
tristanqinusty, Great :)12:21
tristanInterestingly, these are parents chasing zombies, who formerly were their children12:23
*** tristan has quit IRC12:29
gitlab-br-botbuildstream: 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/81012:30
qinustyNice work on the CONTRIBUTING.rst adds68 https://imgur.com/8iWzgA612:33
adds68qinusty, oh nice! Thanks haha12:34
bochechajjardon: I had originally implemented the %{cppflags-extra} thing locally12:34
bochechajjardon: I stopped because it caused lots of churn for just a few rare elements needing it12:34
qinustyWeird how gitlab doesn't recognise HACKING(.rst|.md) as the same thing12:35
bochechaas 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
bochechajjardon: it just felt like too much work for very few gain12:36
bochechajjardon: 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 ratio12:36
bochechait's less elegant, yes12:36
*** lantw44 has quit IRC12:36
jjardonbochecha: yeah, the idea is that if this is supported by bst directly, maybe less churn is needed12:36
bochechajjardon: oh yes, absolutely12:36
jjardonand taking in account you and tristan have the same idea, probably we should go for it :)12:37
bochechawell, 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
qinustyCan I get a review on https://gitlab.com/BuildStream/buildstream/merge_requests/799?12:38
*** mohan43u has quit IRC12:42
*** mohan43u has joined #buildstream12:43
gitlab-br-botbuildstream: merge request (adamjones/contributing-links->master: Fix rst link formatting for guideline links) #811 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/81112:44
* adds68 noticed the rst formatting was wrong on the links12:45
adds68can someone merge the fix please :)12:45
tpollardgitlab dead?12:46
cs-shadowlooks that way12:47
jmacI'm seeing a few network problems12:47
Kinnisonmm 50312:47
gitlab-br-botbuildstream: merge request (adamjones/contributing-links->master: Fix rst link formatting for guideline links) #811 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/81112:50
gitlab-br-botbuildstream: 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/72612:56
finnFor some reason, searching the docs for me is taking a really long time12:57
finnGets hung at "Searching..."12:58
tlater[m]finn: Did anyone actually implement searching?12:59
* tlater[m] thinks that doesn't work by default12:59
finnI'm sure that's worked in the past12:59
finnhttps://buildstream.gitlab.io/buildstream/13:00
finnthe search docs box in the top left13:00
gitlab-br-botbuildstream: merge request (adamjones/contributing-links->master: Fix rst link formatting for guideline links) #811 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/81113:01
finnso 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 #buildstream13: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
adds68i've used it in the past for search CAS related docs13:08
finnI swear I've used it before13:08
finnI've used it to search for elements13:08
tlater[m]adds68: Do you mean the CAS in the *buildstream* docs?13:08
adds68yea13:09
phildawsonI've definitely used the search at https://buildstream.gitlab.io/buildstream/13:09
tlater[m]Well I stand corrected then13:09
finnIt's down on BuildGrid too tbf13:09
finnwonder if this is a gitlab issue13: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
finnI have13:10
*** lantw44 has quit IRC13:12
*** lachlan has quit IRC13:14
*** lachlan has joined #buildstream13:21
adds68jjardon, can you re approve the merge please: https://gitlab.com/BuildStream/buildstream/merge_requests/811 ?13:25
gitlab-br-botbuildstream: merge request (adamjones/contributing-links->master: Fix rst link formatting for guideline links) #811 changed state ("merged"): https://gitlab.com/BuildStream/buildstream/merge_requests/81113:25
tlater[m]o\13:35
* tlater[m] didn't notice tiagogomes shiny new gitlab issues link thing13:35
tiagogomesOn the Website? That would be Valentin13: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 IRC13:37
*** lantw44 has joined #buildstream13:49
*** lachlan has joined #buildstream13:50
gitlab-br-botbuildstream: merge request (willsalmon/outOfSourecBuild->master: Out of source builds) #776 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/77613:57
flatmushAnyone know why baserock used SYSLINUX rather than grub for the minimal system?14:00
gitlab-br-botbuildstream: issue #657 ("Setup our own x86_64 runners") changed state ("closed") https://gitlab.com/BuildStream/buildstream/issues/65714:01
qinustyIs it intentional that I am able have config: configure-commands set for `import` kind despite it doing nothing?14:06
paulsher1oodskullman: do you recall anything about why baserock used syslinux rather than grub for minimal?14:08
paulsher1oodor Kinnison ?14:08
*** lachlan has quit IRC14:09
KinnisonEasier to get running iirc14:09
skullmanpaulsher1ood: smaller and simpler14:09
tlater[m]qinusty: I don't think so. That should at least warn users. File an issue?14:09
paulsher1oodflatmush: ^^14:09
paulsher1oodthanks chaps14: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 #buildstream14:10
*** lachlan has joined #buildstream14:11
*** lachlan has quit IRC14:14
*** lachlan has joined #buildstream14:14
*** lachlan has quit IRC14:16
*** lachlan has joined #buildstream14:16
*** lachlan has quit IRC14:18
*** lachlan has joined #buildstream14:18
gitlab-br-botbuildstream: issue #662 ("No warning produced for import element with configure-commands set") changed state ("opened") https://gitlab.com/BuildStream/buildstream/issues/66214:21
gitlab-br-botbuildstream: 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/78714:22
* qinusty hates bisecting through the "PRE_CAS_MERGE_JULY_2018" problems14:23
*** lachlan has quit IRC14:27
gitlab-br-botbuildstream: 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/80014:27
jjardonpaulsher1ood:  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
jjardontiagogomes: what is you gnome wiki account?14:31
tiagogomesI PMed it to you14:32
flatmushjjardon: Yeah, ctgriffiths mentioned that to me earlier, so we might try that14:34
*** lachlan has joined #buildstream14:36
*** finn has quit IRC14:40
tlater[m]juergbi: Does this description of the CAS change seem ok? https://gitlab.com/BuildStream/website/merge_requests/89/diffs14:40
gitlab-br-botbuildstream: 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/78714:40
*** lachlan has quit IRC14:41
*** rdale has quit IRC14:43
tiagogomestlater[m] left a comment14:43
tiagogomesAn artifact created for a stack element, does it contains the files of the stacked artifacts or is just purely metadata?14:44
juergbitlater[m]: lgtm. we could also mention that it also replaces the tar cache on non-Linux, although there were probably not too many users14:44
* paulsher1ood hopes it's just the metadata14:44
* tiagogomes points that if its not, the CAS deduplication feature kicks in14:45
tiagogomesThere's still some overhead of course14:45
tlater[m]tiagogomes: It only contains metadata :)14:45
juergbitlater[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 servers14:46
persiaIn this case overhead == metadata, I think14:46
*** finn has joined #buildstream14:46
tlater[m]juergbi: Yes, I think that's a better comment14:46
* tlater[m] amends it14:46
juergbita14:46
*** abderrahim has quit IRC14:46
juergbitlater[m]: btw: I'd mention TLS, not SSL, to be precise14:47
*** abderrahim has joined #buildstream14: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/8914:49
tlater[m]Alternatively I think qinusty can approve?14:49
* qinusty is here14:49
qinustyWill take a quick look14:50
laurencetpollard, just re-reading your email to the list summarising the workspace DX design changes14:51
laurencegood email, btw :)14:51
laurenceI 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
laurenceI think I'd prefer that, for neatness. Interested in opinions of others14:51
tiagogomestlater[m], did OSTree handle file deduplication across artifacts built from different elements?14:52
juergbiyes14:52
tlater[m]tiagogomes: Yep14:52
tpollardlaurence: I think that's probably the right thing to do for existing tickets which are covered by it14:53
tiagogomesneat14: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
laurencetpollard, alright, let's get that done and maybe a ping to the list as an update14:54
tpollardlaurence: 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
laurencetpollard, my view is that you can close it. if the original person disagrees then they can always re-open14:59
*** lachlan has joined #buildstream14:59
laurencewith a note explaining the reasons then I think it's pretty clear cut, really14:59
tpollardYeh fair enough15:00
tlater[m]qinusty: I added that link - forgot to fill the placeholder15:03
qinusty:D15:03
qinustyAlso, follow the theme toscalix set with links at the bottom etc15: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 style15:06
qinustyWho knows, something to do with journalists being able to extract links from the page easily? Basically, you `[link text]` inline15:07
qinustyand `[link text]: url` at the bottom15:07
qinustyIt's used within that file you were editing15:07
tlater[m]Oh, I completely missed that, whoops15:09
tlater[m]qinusty: It should now have a link to Bazel CAS there15:10
qinustyYou missed one ;)15:11
qinustyInline link is unnecessary15:12
*** lachlan has quit IRC15:12
*** lachlan has joined #buildstream15:13
tlater[m]...15:13
* tlater[m] needs to read the markdown specification for this wiki15: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
qinustyNo worries, [TOC] is a plugin but the links are actually valid markdown.15:16
qinustyWe could do links in lots of different ways, this way was settled on at one point15:17
* qinusty personally disagrees with it. But we're sticking with it now :D15:18
tlater[m]And here I thought I knew the markdown spec well15:18
qinustyYeah there are 2-3 ways to link in markdown. It's weird15:18
tlater[m]Documentation for links is here for anyone interested: https://daringfireball.net/projects/markdown/syntax#link15:19
qinustymy 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 markdown15:19
qinustyE.g. `[link text][ref]` and `[ref]: url`15:19
qinustyThat's my preferred academic report format15:19
qinustyWhere the ref is shared rather than the text15:19
* qinusty plans to write his dissertation in markdown and have it build in gitlab CI15: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_markdown15:21
qinustynot tpollard15:21
tpollardha!15:22
qinustyI 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! ;p15:24
qinusty:O15:24
qinustyI was going to do latex, but for readability. Markdown wins every time15:24
qinustyAnd with the preprocessor, you can get the desired features15: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
qinustyYou /could/ dockerize15:25
qinustyAn image which can build your papers easy15:26
toscalixqinusty: tlater[m] the number one reason for the current link formatting is the content review15:26
tlater[m]Then I need to maintain a docker image though.15:26
toscalixthe reason related with copy-paste contents was an additional one, not the main one. The main one was provided by tiagogomes and I agreed15:26
tlater[m]toscalix: Content review?15:26
toscalixthen a little modification was proposed by tristan and accepted15:27
toscalixcode review <=> content review15:27
*** lachlan has quit IRC15: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
toscalixit is easier to read the text you are reviewing.15:28
toscalixwhen you do not have the links inserted in the text itself15:29
tlater[m]Ah, I understand. That's fair enough, yes, the raw text looks a lot nicer.15:30
*** lachlan has joined #buildstream15:34
*** lachlan has quit IRC15:43
*** lachlan has joined #buildstream15:45
abderrahimHi. Any idea how I can debug this error?15:55
abderrahimhttps://gitlab.com/freedesktop-sdk/freedesktop-sdk/-/jobs/9916935315:55
abderrahimI've tried opening a shell at the error prompt, but running sh or echo works fine15:56
qinustyAnyone looking to bisect in future, I made https://gitlab.com/BuildStream/buildstream/snippets/1755329.16:02
qinustyAutomated bisecting can allow you to run some beefy tests through a bisect without having to sit and wait16:03
*** lachlan has quit IRC16:03
qinustyNot too sure abderrahim, perhaps adds68 could be of use here.16:06
*** lachlan has joined #buildstream16:07
*** toscalix has quit IRC16:14
*** lachlan has quit IRC16:18
gitlab-br-botbuildstream: merge request (tpollard/494->master: WIP: Don't pull artifact buildtrees by default) #786 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/78616: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
qinustyGood question... It'll need a rebase obviously. Context manager can go or perhaps rethink how we can standardise the BUG thing.16:32
qinustyjob.py duplicates to make things easier to work with imo.16:32
qinustyI'd say that's a valid duplication, they need extra kwargs16:32
qinustyAnd if person x comes along, they might not be that aware of job.py to add the kwarg16:33
skullmanfascinating… it looks like the reason I was unable to nest bubblewrap in bubblewrap while mounting --proc was because I was running it as root16:33
skullmanI'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 locked16: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
qinustyMy point precisely, if you can think of a neat way to deal with it. Go ahead :D16:35
qinustyI just didn't see a neat alternative16: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 it16:36
tlater[m]qinusty: Why did you add that contextmanager in the first place?16:36
qinustyThe goal was to create a singular place in which BUG messages were created.16:37
qinustyCurrently they're all over the place and not consistent at all16:37
qinustyVaried messages are produced throughout master currently16:37
tlater[m]Hmm, I think that's simply a side effect of what BUG is used for16:38
* tlater[m] takes a look at the exact messages16:39
tlater[m]Ah, I see - in some cases people use the traceback as detail, in others something arbitrary16: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
qinustysure16:48
qinustygo ahead tlater[m], I won't really have the time tomorrow16:48
*** iker has quit IRC16:54
gitlab-br-botbuildstream: 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/81216:55
gitlab-br-botbuildstream: 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/81216:55
qinustyWSalmon, 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 IRC17:03
*** xjuan has joined #buildstream17:07
*** finn_ has joined #buildstream17:22
*** finn has quit IRC17:23
*** finn_ has quit IRC17:26
*** tiagogomes has quit IRC17:45
*** finn has joined #buildstream17:49
*** finn has quit IRC18:04
valentindjjardon, 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
valentindSome 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 debuild18:19
*** lachlan has joined #buildstream18:29
*** lachlan_ has joined #buildstream19:27
persiavalentind: 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 IRC19:37
*** lachlan has quit IRC19:37
valentindpersia, I find that most of packages actually do not provide get-orig-source. But maybe things have changed.19:42
valentindBut 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
valentindIt is just that testing the watch file is nice.19:43
*** finn has joined #buildstream20:07
*** tristan has joined #buildstream20:13
*** finn has quit IRC20:36
persiavalentind: 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
persiaOn 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 IRC21:53
*** WSalmon has quit IRC22:04
*** WSalmon has joined #buildstream22:04
*** alatiera_ has quit IRC22:12
*** bochecha has quit IRC23:26

Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!