IRC logs for #buildstream for Wednesday, 2017-11-29

*** valentind has quit IRC00:37
*** xjuan_ has joined #buildstream01:57
*** xjuan_ has quit IRC03:02
*** mcatanzaro has quit IRC05:10
*** bochecha has joined #buildstream08:41
*** jude has joined #buildstream09:02
*** tristan has quit IRC09:11
*** jude has quit IRC09:19
*** jude has joined #buildstream09:36
*** bethw has joined #buildstream09:39
*** tristan has joined #buildstream09:44
*** bethw has quit IRC09:50
*** jonathanmaw has joined #buildstream09:52
*** valentind has joined #buildstream09:56
* juergbi is wondering about a small junction syntax change. see mailing list09:58
*** ssam2 has joined #buildstream10:08
*** bethw has joined #buildstream10:15
gitlab-br-botbuildstream: merge request (tracking-changes->master: Tracking changes) #119 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/11910:17
gitlab-br-botbuildstream: merge request (tracking-changes->master: Tracking changes) #119 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/11910:18
tlatertristan: I think that MR should be ready now :)10:18
*** bethw has quit IRC10:20
* tlater ended up not changing how the planner works, because it's a massive headache to keep the sorting correct when things are done in two steps.10:20
*** bethw has joined #buildstream10:21
gitlab-br-botbuildstream: merge request (sam/multiple-caches->master: WIP: multiple remote cache support) #166 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/16610:22
*** ssam2 has quit IRC10:30
*** ssam2 has joined #buildstream10:31
gitlab-br-botbuildstream: merge request (132-loading-external-plugins-works-without-explicit-requirement-in-project-conf->master: Resolve "Loading external plugins works without explicit requirement in project.conf") #125 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/12510:36
gitlab-br-botbuildstream: merge request (bst-here-permissions->master: bst-here: Mitigate permission issues) #169 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/16910:44
jonathanmawtristan, can you review https://gitlab.com/BuildStream/buildstream/merge_requests/125 when you have time?10:47
gitlab-br-botbuildstream: merge request (bst-here-permissions->master: bst-here: Mitigate permission issues) #169 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/16910:52
*** valentind has quit IRC11:10
ssam2does anyone have a clue about why adding a new plugin to bst-external would fail in this way ?11:25
ssam2oh wait, it's a new failure now11:26
ssam2ignore me, my error was missing __init__.py11:26
tlaterssam2: Didn't that happen last week too?11:26
ssam2errors due to python's weird "package" concept ? probably11:27
*** tristan has quit IRC11:36
*** tristan has joined #buildstream11:38
*** tristan has quit IRC12:15
gitlab-br-botbuildstream: merge request (juerg/recursive-pipelines->master: WIP: Junctions and subprojects) #160 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/16012:26
gitlab-br-botbuildstream: merge request (114-give-better-warnings-on-overlaps->master: WIP: Resolve "Give better warnings on overlaps") #165 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/16512:58
gitlab-br-botbuildstream: merge request (114-give-better-warnings-on-overlaps->master: WIP: Resolve "Give better warnings on overlaps") #165 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/16513:02
gitlab-br-botbuildstream: merge request (bst-here-permissions->master: bst-here: Mitigate permission issues) #169 changed state ("merged"): https://gitlab.com/BuildStream/buildstream/merge_requests/16913:19
gitlab-br-botbuildstream: merge request (114-give-better-warnings-on-overlaps->master: WIP: Resolve "Give better warnings on overlaps") #165 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/16513:26
gitlab-br-botbuildstream: merge request (114-give-better-warnings-on-overlaps->master: WIP: Resolve "Give better warnings on overlaps") #165 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/16513:47
*** tristan has joined #buildstream14:11
gitlab-br-botbuildstream: merge request (114-give-better-warnings-on-overlaps->master: WIP: Resolve "Give better warnings on overlaps") #165 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/16514:14
tlaterjuergbi: https://irclogs.baserock.org/buildstream/%23buildstream.2017-11-21.log.html#t2017-11-21T11:02:1014:17
tlaterMy problem is that, in fact, _yaml.composite() does not receive a provenance *all* the time anymore.14:18
tlaterIt doesn't because in this case: https://gitlab.com/BuildStream/buildstream/blob/master/buildstream/_options/optionpool.py#L23014:19
tlaterWell, the provenance is handled outside of the node, because optionals aren't actual nodes14:20
tlaterSo creating a stack of provenances doesn't really solve the problem :|14:21
tlaterI do have a branch that solves that particular issue, but I think the overall idea was to avoid this kind of situation in the future, not just prop the bug.14:21
juergbihm, i'm not exactly a yaml provenance expert ;)14:23
tlaterHeh, that's alright, just a bit stuck for how to solve this in the future :)14:24
tristanoh that one, would be awesome to tolve that one14:24
tristantlater, _yaml.composite() *never* "receives" provenance14:25
tristantlater, _yaml.composite() however *always* receives a node14:26
tristanfor both the target and the source14:26
tlatertristan: Not for the source, due to the bit of code I linked earlier14:26
tristanand the provenance of the node, or particular members, can always be extracted14:26
tristanhmmm did I miss something earlier ?14:27
tristanI'm looking at https://gitlab.com/BuildStream/buildstream/blob/master/buildstream/_options/optionpool.py#L23014:27
tlater'values' there is not a node14:27
tlater'value', sorr14:28
tristannow is it not ?14:28
tristanHow14:28
tlaterI'm not sure why it isn't, it shows up as an array14:29
tlaterMaybe *that* is the issue, actually14:29
tristantlater, trying to think here, but I believe we only ever support conditionals in dictionaries14:29
tristanand that simplifies things14:29
tlaterAh, right14:29
tlaterIt's because technically the user can define it in an array.14:30
tristanso in a conditional statement... (?) is the special key for a list of conditionals14:30
juergbimaybe because of the list() conversion?14:30
tristaneach conditional itself is a dictionary14:30
juergbi(on line 222)?14:30
tristanand the key is the conditional statement, the value is the dictionary to be composited into the dictionary containing the special (?) key14:30
* tlater looks a bit further into why this is an array14:31
tristanthe list conversion there converts the generator into a list, I believe for the purpose of calling len()14:32
tristanthe generator yields a key/value tuple for each conditional14:33
tristanwhich is the expression (key) and value to apply, which... is it always a dict ?14:33
tristanI think it must always be a dict, since (?) is always a dict14:33
tlaterIt isn't, oddly enough14:33
tristanin what case ?14:33
* tlater finds the test case14:33
tlaterIt turns into an array containing this node: https://gitlab.com/BuildStream/buildstream/blob/8c4d8ca2b7c886690fb933428c6768c745667db7/tests/format/list-directive-type-error/element.bst#L614:34
tristanmaybe we need to assert that as an invariant14:34
tlater(Actually the line above that)14:34
tristanoh my14:34
tristanthat looks wrong14:34
tristanssam2, see that line above ^^^^14:35
tristanso... do we want to support that ?14:35
*** laurenceurhegyi has quit IRC14:36
tlatertristan: The context of that test case is that it used to throw a trace14:36
*** nexus has quit IRC14:36
*** paulsherwood has quit IRC14:36
*** paulsherwood has joined #buildstream14:36
*** nexus has joined #buildstream14:36
tlaterssam2 fixed it by checking for that explicitly14:36
tristansee, _yaml.composite() has an assertion immediately on the source, but not on the target14:37
tristanwhich states that it must be something that has `.get()` (i.e. a mapping of some sort)14:37
*** laurenceurhegyi has joined #buildstream14:37
tristanUmmm tlater I'm more inclined to make that case invalid14:38
tristanit's the easier thing at least14:38
tlatertristan: It is currently, the assertion makes it invalid :)14:38
tristan:-S14:38
tlaterOnly issue is that we can now occasionally get a node without a provenance in _yaml.composite()14:38
tristandefine currently ?14:38
tlatercurrently == current bst master14:39
tristantlater, if it's not a dictionary, it's not a node14:39
tlaterRight, so I think the issue is that we should probably check if it is a node before we give it to _yaml.composite(), and throw a LoadError before we ever enter that function?14:39
juergbiso have a special error path for that?14:40
tristanah that assertion, ok so... the value should be asserted to be a dictionary in process_one_node(), in optionpool.py14:40
tristanand the provenance used for that, I suppose would be _yaml.node_get_provenance(condition_node, condition_member)14:40
tristanor I suppose the provenance collected in that loop will do fine14:41
tristanjuergbi, exactly, special error path for that14:41
tristanbefore throwing invalid stuff at _yaml.composite()14:41
tlaterAnd then get rid of the assertion in _yaml.composite() - that would solve the issue, yeah :)14:41
jjardon[m]Hi, buildstream seems to be stuck here in the "checking" phase: "Checking:  132/092" doing a strace I keep getting https://paste.gnome.org/pov6ck588 ;  I can open that file and it seems ok; any idea what can go wrong?14:43
tristanI dont think there is a need to get rid of the other assertion in _yaml.composite(), is that what sam added ?14:43
tlatertristan: Yup, but it can occasionally print an error without a provenance14:43
tristantlater, is it what sam added in that previous patch ?14:44
tristanis that what the Yep means ?14:44
tlaterYes, it is14:44
tristanOk, then make it simply `assert`14:44
tristaninstead of removing completely14:44
tlaterRight, makes sense14:44
tristanthat way if it ever happens, we should get a BUG: message or stack trace14:45
juergbijjardon[m]: you could print the name of the element that is being checked, https://paste.gnome.org/pzwqxjgvr14:47
juergbi(as it's not sure that it's related to the patch file)14:47
* tlater wonders if that assertion should be on both source and target14:54
tlaterBut then we're at the slippery slope of asserting all argument types14:54
jjardon[m]juergbi: Checking:  132/092 graphite2.bst14:55
tristantlater, meh, can just remove it if that's the case14:55
tristantlater, seems to be a good assertion to keep in place, though14:55
jjardon[m]but Im pretty sure it will go to the next element after a while: strace is spitting the same error for the same 5 files all the time14:55
tlaterHm, I'll just keep it on only the source, in case we ever run into this again14:56
*** mcatanzaro has joined #buildstream14:58
jjardon[m]yup: "Checking:  132/095 libproxy.bst"15:03
juergbijjardon[m]: do you mean the ENOTTY error?15:12
jjardon[m]juergbi: yes; well I think is actually not an error but bst is open and closing the same files all the time15:14
jjardon[m]let me try to comment those patches from the .bst files15:14
gitlab-br-botbuildstream: merge request (142-potentially-printing-provenance-more-than-once-in-loaderrors->master: Issue #142: Ensure we don't append provenances twice) #170 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/17015:15
jjardon[m]ok, no more problems with the files anymore; now is spitting:15:15
jjardon[m]mmap(NULL, 262144, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f4fc0f1700015:15
jjardon[m]munmap(0x7f4fc0f17000, 262144)          = 015:15
ssam2the patch files will be being touched each time it calculates a cache key15:15
juergbithe TCGETS ioctl is probably just Python trying to figure it whether this is a special file15:15
ssam2because local/patch sources need to hash the files they import in order to calculate their cache key15:15
juergbithat's just memory allocation15:15
* tristan is building GNOME converted to tarballs on a fairly slow internet connection...15:16
juergbimaybe there is an infinite loop somewhere, or something very slow15:16
ssam2although, if it's calculating cache keys a lot, maybe that's the slow down15:16
ssam2does every component in your pipeline have a 'ref' ?15:16
tristanI've built 47 modules now, and still have a constant 18 tasks going on15:16
ssam2or are you running `build --track` ?15:16
tristan8 fetches, 8 pushes and 4 builds15:16
ssam2tristan: my laptop would hang for sure with that config !15:16
ssam24 fetches + 2 builds hung it this morning15:17
tristanssam2, you need to get a developer laptop man ;-P15:17
tristangrandma's browsing and email utility, just not enough15:17
ssam2this machine is battle hardened15:17
jjardon[m]it can very well that some of the .bst files is broken: let me check15:18
ssam2note that it's not /broken/ to not have a ref field, but I have noticed hangs involving `build --track` before15:18
gitlab-br-botbuildstream: merge request (juerg/recursive-pipelines->master: WIP: Junctions and subprojects) #160 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/16015:18
tristanjjardon[m], of course, broken bst files should not cause hangs, there is certainly a bug *somewhere* in here :)15:18
tristanssam2, fwiw, I noticed something today... if a low level component in a big pipeline has no ref... it can take a long time for the pipeline to report the error15:19
* jjardon[m] happy to find new bugs :)15:19
tristanthis seems again part of this regression of overcalculation of cache keys15:19
tristanwe should just "know" that reverse deps cannot get cache keys when their deps cant15:20
tristanall comes back to our messy all over the place handling of transient element state15:20
gitlab-br-botbuildstream: merge request (128-status-ticker-fails-to-update-periodically-on-some-builds->master: WIP: Resolve "Status ticker fails to update periodically on some builds") #123 changed state ("closed"): https://gitlab.com/BuildStream/buildstream/merge_requests/12315:20
tristanssam2, and I thought there was a bug with `bst track --deps all` "hanging" in a certain situation, which I worked around by not printing the toplevel target at the end15:21
tristanbecause the loop becomes super exponential, it hangs at 100% cpu for a long time, and eventually completes15:22
ssam2might have been thinking of that15:25
ssam2ah no, mine was https://gitlab.com/BuildStream/buildstream/issues/14915:25
gitlab-br-botbuildstream: merge request (tar-symlinks->master: Fix untar of symlinks. Only hardlinks are relative to top of archive and should be normalized.) #163 changed state ("closed"): https://gitlab.com/BuildStream/buildstream/merge_requests/16315:42
gitlab-br-botbuildstream: merge request (sam/plugin-log->master: plugin.py: Add log() method) #168 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/16815:46
gitlab-br-botbuildstream: merge request (invoking_page_changes->master: WIP: Separated :show-nested: into individual calls) #157 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/15715:49
jjardon[m]ssam2: success! that was the problem15:55
jjardon[m]some git sources where using track: but not ref:15:55
tristanjjardon[m], it is expected to maintain ref-less projects15:56
tristani.e. it is still a valid project15:56
jjardon[m]problem is that the track: pointed to a ref: instead to a branch15:57
tristanjust not a buildable one, it should report that it is not buildable quickly, though15:57
tristanAh, and doesnt that work ?15:57
tristanI thought that worked15:57
tristannot that it's really an intended use case15:57
tristanbut when you `bst track` it, I thought it did put the same sha in ref:15:57
jjardon[m]tristan: this is the patch that make it work: https://paste.gnome.org/p5rkcgcgy15:58
gitlab-br-botbuildstream: issue #164 ("Minimise overlaps by having overlaps raise exceptions unless configured not to") changed state ("opened") https://gitlab.com/BuildStream/buildstream/issues/16415:58
tristanstrange... ok so ssam2, jjardon[m]... can you work out what exactly the bug was and make sure it is filed, please ?15:59
* tristan is unsure at the moment15:59
jjardon[m]sure15:59
tristanbut it sounds like buildstream should have been telling you something quickly, and instead left you wondering what was going on15:59
gitlab-br-botbuildstream: merge request (sam/plugin-log->master: plugin.py: Add log() method) #168 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/16816:05
jjardon[m]tristan: yup16:07
*** tristan has quit IRC16:12
gitlab-br-botbuildstream: merge request (sam/plugin-log->master: plugin.py: Add log() method) #168 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/16816:28
*** tristan has joined #buildstream16:32
gitlab-br-botbuildstream: issue #165 ("bst gets confused and hangs forever if a git element have a `track:` parameter but not `ref:` (ie, you try to build before running bst track)") changed state ("opened") https://gitlab.com/BuildStream/buildstream/issues/16516:34
tristanjjardon[m], thanks for filing that... that is indeed strange :-/16:37
nexustristan: What project would you like me to use as an example piece?16:38
tristanhrm16:41
jjardon[m]tristan: np, thanks to you,  ssam2 and juergbi for your help!16:42
tristannexus, I guess you need one ? I mean... I dont like to tie the docs to a specific project16:43
tristanbut maybe for now we can use the GNOME modulesets as an example16:43
tristannexus, it is also planned to give a similar guide, make examples for creating projects and using various constructs provided by the existing core set of plugins16:43
tristannexus, so I'm thinking, we may want to later change our user facing examples, to use projects which we ship for example purposes16:44
tristannexus, so I guess the best is, for now; create the user facing workflow docs using GNOME as an example, but keep in mind that we will later want to change the specific examples used to ones that we provide in another project authoring section16:45
tristanso keeping that in mind mostly means, try to keep the actual text outside of the examples as project agnostic as possible ?16:45
juergbiusing sshd in tests doesn't exactly seem ideal. we have the issue that we have to find an unused TCP port, have to generate a key pair, and sshd does a permission check on every path component of the key, which may or may not be an issue in the test environment16:46
juergbii'm wondering whether we should rather exec pushreceive without ssh. we would still test the largest part that way16:46
ssam2would paramiko be easier ?16:46
juergbipossibly16:47
ssam2or executing pushreceive another way ... the main issue with that is we have to have some ugly custom protocol that we only use in tests, right ?16:47
juergbiyes16:48
juergbione other option could be to make local path use pushreceive16:48
juergbijust without ssh16:49
ssam2oh, that would make sense since we only use that for testing anyway16:52
juergbiyes, would actually reduce the number of code paths in some points16:53
tristannod, we discussed this the other day :)16:54
tristanor similar16:54
tristanfwiw, I dont object to paramoko really, it seems like it would be nice to have some drop in replacement for the ssh/sshd thing16:55
tristanthat would require the least test-conditional code16:55
tristanand best coverage16:55
juergbiwell, if we used paramiko for client as well, we'd already not test the 'ssh' invocation line16:56
juergbithe only additional missing coverage with local pushreceive execution would be ssh url parsing, afaict16:56
tristanso it's not a drop in replacement I guess :-/16:56
tristanbut still, whatever you find easy and good, will be better than the nothing we have :D16:57
juergbiparamiko server with openssh client could be a possibility. would probably be slightly less painful than openssh client+server and slightly better code coverage16:57
juergbinot sure whether it's worth it16:58
ssam2local file + pushreceive seems a simpler first step either way17:00
juergbii agree17:01
gitlab-br-botbuildstream: merge request (142-potentially-printing-provenance-more-than-once-in-loaderrors->master: Issue #142: Ensure we don't append provenances twice) #170 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/17017:05
tlaterThis is where I wish my event hooks branch was merged, I could set up a script to shut down my PC after this build finishes ;P17:27
gitlab-br-botbuildstream: merge request (sam/plugin-log->master: plugin.py: Add log() method) #168 changed state ("merged"): https://gitlab.com/BuildStream/buildstream/merge_requests/16817:29
*** jonathanmaw has quit IRC17:56
juergbiexecuting bst-artifact-receive more or less works17:57
juergbihowever, we don't get coverage for that part. that needs some more work, or we use fork without exec for it17:57
*** bethw has quit IRC18:01
*** ssam2 has quit IRC18:05
*** WSalmon has quit IRC18:05
*** adds68 has quit IRC18:05
*** jude has quit IRC18:06
*** tpollard has quit IRC18:06
*** ssam2 has joined #buildstream18:16
*** ssam2 has quit IRC18:19
*** jude has joined #buildstream18:53
*** WSalmon has joined #buildstream18:58
*** adds68 has joined #buildstream19:00
*** adds68_ has joined #buildstream19:14
*** adds68 has quit IRC19:15
*** mcatanzaro has quit IRC19:15
*** mcatanzaro has joined #buildstream19:16
gitlab-br-botbuildstream: merge request (sam/multiple-caches->master: WIP: multiple remote cache support) #166 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/16619:17
*** valentind has joined #buildstream19:29
*** tpollard has joined #buildstream20:52
*** valentind has quit IRC22:27
*** mcatanzaro has quit IRC23:29

Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!