*** nimish2711 has joined #buildstream | 00:46 | |
*** rdale has quit IRC | 01:28 | |
*** nimish2711 has quit IRC | 03:32 | |
*** nimish2711 has joined #buildstream | 03:52 | |
*** tristan has quit IRC | 04:38 | |
*** tristan has joined #buildstream | 04:44 | |
*** dftxbs3e has quit IRC | 05:20 | |
*** ChanServ sets mode: +o tristan | 05:36 | |
tristan | abderrahim[m], Around ? | 05:36 |
---|---|---|
tristan | Are you sure you did not encounter tracking errors for the said 3 upstreams which have disappeared ? | 05:37 |
tristan | Your paste at https://gitlab.com/BuildStream/buildstream/issues/1014 shows no failed elements in the Track queue, is that accurate ? | 05:37 |
tristan | juergbi, The strict cache key is the one we use to download artifacts with correct ? | 06:06 |
tristan | The one which is only different from the strong one when in non-strict mode ? | 06:06 |
juergbi | tristan: first attempt is always with strict cache key, yes. if that doesn't exist, we try with the weak key (unless we already have that in the local cache) | 06:07 |
*** dftxbs3e has joined #buildstream | 06:07 | |
tristan | juergbi, in the course of debugging #1014, I stumbled upon a comment you added to Element._can_query_cache() in commit 33b14eb50, I think the comment does not really explain what is going on, or I'm not reading it right | 06:08 |
gitlab-br-bot | Issue #1014: bst build --track-all doesn't build anything https://gitlab.com/BuildStream/buildstream/issues/1014 | 06:08 |
tristan | I.e. it is saying that "If we're gonna build it, we can try to download it, even if we don't know what key we're gonna ask the server to download it by" | 06:08 |
juergbi | do you mean the first comment in _can_query_cache()? | 06:09 |
*** nimish2711 has quit IRC | 06:09 | |
juergbi | this is not about downloading, this is about allowing local cache query | 06:09 |
juergbi | and that uses the weak key in non-strict mode | 06:10 |
tristan | _can_query_cache() determines whether the *local* cache can be queried ? | 06:14 |
juergbi | yes | 06:15 |
juergbi | i.e., to be able to decide whether we need to build the element or not | 06:15 |
juergbi | fetch/pull queues use that to wait until it's clear whether we need to build | 06:16 |
tristan | Things are getting quite jenga here and incredibly hard to follow, I hope that jonathanmaw's branch fixes all of this :-/ | 06:16 |
tristan | Yes I am seeing that | 06:16 |
tristan | it appears that it might be the root cause of #1014 | 06:16 |
tristan | Which I did manage to reproduce just now, without failing track | 06:17 |
tristan | See: https://gitlab.com/BuildStream/buildstream/issues/1014#note_167765732 | 06:17 |
tristan | juergbi, I have some debug messages added in pull queue where it returns with QueueStatus.SKIP, and it is telling me basically that the target vm image is "Assemble scheduled: False, Strict cache key: None", after all tracking has successfully completed | 06:18 |
tristan | Oh sorry | 06:18 |
tristan | I mean where it returns with QueueStatus.WAIT due to a false element._can_query_cache() | 06:19 |
juergbi | hm, strict cache key none means that either a dependency also doesn't have a strict cache key or update_state wasn't called after all dependencies got their strict cache key | 06:20 |
juergbi | the only thing needed for the strict cache key for non-workspaced elements is the ref of the sources, though | 06:21 |
juergbi | assuming this isn't about workspaces, might track not trigger update_state or something like that? | 06:21 |
juergbi | or maybe only of the element itself instead of also triggering update_state of reverse dependencies? | 06:21 |
juergbi | (or an element wasn't marked required) | 06:22 |
tristan | Well as per the referred comment, it is indeed an interesting case because we expect the elements to not be required while tracking | 06:23 |
tristan | I.e. we expect to (A) Track all the elements... (B) As a result, determine the cache key of the very high level build-only dependency... (C) Try to download the artifact based on the newly discovered cache key... (D) Mark those elements as required as a result of not being able to download that artifact | 06:24 |
tristan | lemme check that we are actually recursively updating state based on successful track | 06:24 |
tristan | Yes we do call _update_state_recursively() there | 06:25 |
tristan | That only has a comment "Update the state of all reverse dependencies, recursively", it should be fresh in my memory but even after a month, but hard to say what this ready for runtime actually is :-/ | 06:28 |
tristan | The comment there says "Wether the element has all its dependencies ready and has a cache key" | 06:28 |
juergbi | i.e., whether the current element and all its (transitive) runtime dependencies have the final (strong) cache key | 06:32 |
juergbi | or in yet other words, whether that element can be used/staged as a build dependency | 06:33 |
tristan | Ah right | 06:37 |
tristan | Basically, whether not only it's build dependencies have their cache keys resolved (which can be determined by the cache key itself existing), but whether all of it's (transitive) runtime dependencies also have cache keys ? | 06:38 |
tristan | juergbi, correct ? | 06:38 |
juergbi | correct | 06:38 |
tristan | Ok, I've verified #1014 is equally triggering on master and bst-1.2 | 06:39 |
gitlab-br-bot | Issue #1014: bst build --track-all doesn't build anything https://gitlab.com/BuildStream/buildstream/issues/1014 | 06:39 |
juergbi | do you also already have an idea how to reproduce it in a smaller test case? | 06:40 |
juergbi | does the tristan/test-broken-track-all branch successfully track everything (i.e., no upstream issues) but then fail to schedule all required builds? | 06:40 |
tristan | juergbi, The test-broken-track-all branch simply removes dependencies on elements which depend on mythical source code that dont actually exist | 06:42 |
juergbi | ok, I see | 06:42 |
tristan | I.e. there is a rumor that espeak, liboauth, and libwacom actually exist, but no evidence of such | 06:43 |
juergbi | haha | 06:43 |
tristan | :) | 06:43 |
tristan | juergbi, fwiw, I just pushed tristan/debugging-build-track-all-1.2 to buildstream in case you want to look, in fact I was *not* able to reproduce it in a test case | 06:44 |
tristan | but that branch adds test_build_track_all() to tests/frontend/buildtrack.py | 06:44 |
tristan | I think I need to try again, but really it's a bit of guess work | 06:45 |
tristan | that branch also sprinkles some desperate debug tracing in the code (which pointed me at _can_query_cache() as the culprit essentially blocking elements from continuing on to later queues) | 06:46 |
tristan | It could also be that the main scheduler loop has a bug where it fails to observe that the highlevel dependencies have finally been set to be required, after the last successful tracking caused their cache keys to be resolved | 06:47 |
tristan | all theory at this point though | 06:47 |
juergbi | should probably try to figure out the lowest level element that doesn't get a strict cache key | 06:48 |
juergbi | and then debug why? e.g., whether its build dependencies don't trigger that update_state | 06:49 |
tristan | mhmmm | 06:55 |
tristan | juergbi, you pointed me in a direction I didnt consider | 06:55 |
tristan | lemme check something | 06:55 |
tristan | I think I got it | 07:00 |
tristan | Wow | 07:00 |
tristan | Yup | 07:01 |
tristan | juergbi, --no-strict is significant here | 07:01 |
tristan | Ok so, I'll get some food and come back to create a real test case, but here's my short assessment | 07:02 |
tristan | (A) You do not have freedesktop-sdk cross junction elements in your cache, or at least not artifacts where "strong key == strict key"... (B) Build a gnome-build-meta element which has only build-only dependencies on the cross junction elements... (C) --track-all causes all gnome-build-meta elements to have reset cache keys... (D) --no-strict causes the freedesktop-sdk elements to have unresolved cache keys *until they are pulled* | 07:05 |
tristan | juergbi, You see what is circular here ? | 07:05 |
tristan | We don't *require* the elements which we can only determine their cache keys by pulling them, and we can only require them if we have determined their cache keys | 07:06 |
tristan | We can only determine their cache keys by pulling them, but we don't pull them cause they are not yet required | 07:07 |
tristan | That was fun ! | 07:07 |
juergbi | oh | 07:09 |
juergbi | only in _schedule_assemble() we 'require' build dependencies | 07:10 |
juergbi | hm, but we only need the strict cache keys to determine whether to build or not | 07:11 |
tristan | So what I noticed is that when I run `bst --no-strict build --track-all`: (A) ALL elements (except for local source imports) appear with unresolved '????????' cache keys (B) Only the toplevel project will be tracked | 07:13 |
tristan | But without --no-strict, the junctioned project cache keys are resolved | 07:13 |
tristan | juergbi, I also have to wonder if we have an *other* parallel bug here | 07:14 |
juergbi | oh, that's interesting | 07:14 |
juergbi | do we invalidate cache keys for elements that are excluded from tracking? | 07:14 |
tristan | juergbi, i.e. if --no-strict and --track-all are enabled together, *regardless* of cross-junctioning... *why* is a cache key resolved as a result of tracking ? | 07:14 |
tristan | Tracking should only cause the strict key to get resolved, and the strong key should only be discovered later as a result of pulling | 07:15 |
tristan | Right ? | 07:15 |
juergbi | unless the artifact is already in the local cache with the strict key, yes, it should at least attempt pulling before determining the strong cache key | 07:16 |
tristan | juergbi, We invalidate cache keys for elements in strict mode, regardless of whether tracking is going to happen | 07:16 |
tristan | Right | 07:16 |
tristan | Affording that exception where local cache already has "strict == strong" | 07:16 |
juergbi | we don't seem to call _schedule_tracking() for elements we don't track, though, right? | 07:19 |
tristan | juergbi, So anyway I think that in conclusion (A) There are other conditions which require an element to be pulled rather than only if the element is "required" for the build plan... (B) We need to also reexamine if we are actually delaying strong key resolution of tracked elements until *after* they are pulled | 07:19 |
tristan | I don't believe we call _schedule_track() on any elements that we don't track, no | 07:20 |
juergbi | (A) I don't think so | 07:20 |
tristan | or _schedule_tracking() or whichever it's called | 07:20 |
juergbi | we only need the strict cache key for those elements and for that we need no pulling | 07:20 |
tristan | Ok, that is a fair point, indeed | 07:21 |
juergbi | the only thing needed for strict cache key is a consistent source and that we actually call _update_state() when appropriate | 07:21 |
juergbi | (incl. for build dependencies) | 07:21 |
juergbi | might this be a regression of the update_state() call reductions that got backported to 1.2? | 07:21 |
tristan | It might, I will check that after a breakfast which has been delayed until way passed lunch :) | 07:22 |
juergbi | haha, enjoy | 07:22 |
tristan | ok yeah | 07:22 |
* juergbi had breakfast before you despite the huge time zone difference | 07:22 | |
* tristan tears himself away from screen | 07:22 | |
*** tristan has quit IRC | 07:26 | |
Kinnison | jennis: Looking at the performance report mail -- shows seem slower but builds faster, both using less memory. Am I reading that right? | 07:27 |
jennis | Kinnison, that's just the "settle" from post-revert-gc-play | 07:30 |
Kinnison | jennis: okay cool. I still don't grok how it makes build faster, but hey :D | 07:31 |
*** bochecha has joined #buildstream | 07:40 | |
*** rdale has joined #buildstream | 08:00 | |
*** tristan has joined #buildstream | 08:42 | |
*** bochecha has quit IRC | 08:49 | |
*** raoul has joined #buildstream | 08:57 | |
*** phildawson_ has joined #buildstream | 09:03 | |
*** jonathanmaw has joined #buildstream | 09:09 | |
*** bochecha has joined #buildstream | 09:18 | |
*** lachlan has joined #buildstream | 09:44 | |
*** ChanServ sets mode: +o tristan | 09:49 | |
* tristan comes back to reread this... test case still not reproducing | 09:49 | |
tristan | The question then is... *what* causes an element to not have it's cache key resolved in non-strict mode ? | 09:50 |
tristan | As I recall, I don't *really* need a configured artifact server to reproduce | 09:51 |
* tristan goes back to the cache-server disabled state, to observe cache keys with the gnome-build-meta case | 09:51 | |
tristan | Hmmm, ok so, with artifact shares disabled, --no-strict combined with --track-all build *still* causes the cache keys to appear unresolved | 09:56 |
tristan | when building gnome-build-meta | 09:56 |
tristan | But... my test case does not | 09:56 |
tristan | juergbi, Do you have an idea what conditions cause the cache keys to be unresolved in non-strict mode ? | 09:57 |
tristan | What I have in my test case is 3 elements and a junction... the junction itself is wrapped into a git repo and tracked before anything else (expected to always pass)... the junction contains 1 import element that is a git repo, which depends on 1 import element that is a local source | 09:58 |
tristan | The junctioning project has one import element that is a git repo, which depends on the cross-junction git element | 09:59 |
tristan | Maybe I am missing a build-only dependency (compose element at the top ?) | 09:59 |
tristan | Because they are not yet "required", they didnt get resolved yet | 09:59 |
tristan | Hmmm | 09:59 |
juergbi | yes, build only dependencies in non-strict mode don't have strong cache keys until they are required | 10:00 |
juergbi | they should still have strict cache keys, though | 10:00 |
juergbi | (but the UI doesn't show strict cache keys) | 10:00 |
tristan | Right | 10:02 |
tristan | This makes sense | 10:02 |
* tristan is adjusting case | 10:02 | |
jennis | juergbi, phildawson_, I can't tell what you've agreed on here: https://gitlab.com/BuildStream/bst-plugins-experimental/merge_requests/1#note_166870376 do you mean to separate the _ostree.py module out further, or to bundle it all in with ostree.py? | 10:02 |
*** lachlan has quit IRC | 10:02 | |
juergbi | jennis: the latter | 10:03 |
juergbi | (post merge) | 10:03 |
jennis | Thanks | 10:03 |
tristan | Gotcha !!!!!!! | 10:03 |
jennis | Yeah just writing an issue for it so that it's tracked | 10:03 |
tristan | juergbi, That did it | 10:03 |
juergbi | great | 10:03 |
juergbi | jennis: ta | 10:03 |
tristan | I wonder, where should I put this case... it looks a bit involved for buildtrack.py | 10:03 |
tristan | but I suppose it's related enough to buildtrack.py | 10:04 |
tristan | I'll seal up this test case and push it in branches with WIP merge requests for both master and bst-1.2 I guess as a first step | 10:04 |
*** lachlan has joined #buildstream | 10:09 | |
*** bochecha has quit IRC | 10:10 | |
*** bochecha has joined #buildstream | 10:13 | |
jennis | Could someone with write access to bst-plugins-experimental perhaps review and merge this: https://gitlab.com/BuildStream/bst-plugins-experimental/merge_requests/8 ? | 10:18 |
jennis | Trivial change ^^ pinning sphinx and sphinx_rtd_theme | 10:18 |
tpollard | jennis: can we not apply this fix to remove the need to pin? https://gitlab.com/BuildStream/buildstream/merge_requests/1306 | 10:20 |
jennis | Potentially, let me test | 10:21 |
raoul | benschubert and cs-shadow !1330 should fix the tracking issue, probably worth you guys having a review given you encountered this problem to begin with | 10:24 |
gitlab-br-bot | MR !1330: Ensure previous sources refs are updated during track https://gitlab.com/BuildStream/buildstream/merge_requests/1330 | 10:24 |
jennis | Ahh cool, pushing a WIP: commit automatically changes the MR's title | 10:25 |
*** lachlan has quit IRC | 10:26 | |
benschubert | raoul: I've had a look at it, it seems to fix it indeed, thanks a lot! I've just added a nit on the PR | 10:28 |
tristan | juergbi, I came across something unexpected when porting the test case to master: https://bpaste.net/show/5dab422ad971 | 10:42 |
tristan | juergbi, It appears that in non-strict mode, the junction *itself* does not have it's cache key resolved after the loader explicitly calls Element._update_state() on it | 10:43 |
tristan | Maybe we need a separate issue/test case to fix this first in master (I think most of our test cases use local source junctions, maybe that is why this went unnoticed ?) | 10:44 |
juergbi | hm, might be a regression from the update_state() call reduction | 10:45 |
tristan | Maybe junction.py should just call _schedule_assemble() on itself, or _set_required() more like ? | 10:46 |
*** toscalix has joined #buildstream | 10:46 | |
jennis | tpollard, that didn't work :/ | 10:54 |
tristan | It appears I missed this when testing it against master locally because (A) I needed to use a *ported* project to test it... (B) In order to easily port to bst 2 for testing, I used a workspaced junction, side stepping the issue that the junction's cache key was not resolved | 10:54 |
tristan | Looking closely, it's not easy to reproduce in tests with generate_junction() asides from my case... my buildtrack.py case is probably already best for this | 10:55 |
tristan | Hmmm, no scratch that, I think I can reproduce that separately, I think it only requires non-strict and is unrelated to a target with only build dependencies | 10:56 |
tpollard | jennis: :/ | 10:56 |
gitlab-br-bot | tristanvb opened issue #1018 (BuildStream crashes on junctions in non-strict mode) on buildstream https://gitlab.com/BuildStream/buildstream/issues/1018 | 11:05 |
gitlab-br-bot | tristanvb opened MR !1333 (tristan/fix-no-strict-junctions->master: Fix no strict junctions) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1333 | 11:12 |
tristan | juergbi, jonathanmaw -- As a first step, could you please give your opinion on !1333 above ? | 11:13 |
gitlab-br-bot | cs-shadow closed issue #640 (RFE: Allow easy way of building all elements) on buildstream https://gitlab.com/BuildStream/buildstream/issues/640 | 11:19 |
tristan | jonathanmaw, thanks for the comment... I think it is a sensible change - i.e. it "makes sense that junctions are unconditionally required", although required might not be quite exactly the right thing (they don't need to be an artifact, although their cache key needs to be resolved) | 11:24 |
tristan | jonathanmaw, anyway it's rather important that you at least know about this, given the refactors you are working on | 11:25 |
tristan | You may be inclined to change this approach with those refactors | 11:25 |
jonathanmaw | nod | 11:25 |
tristan | Ok, back to the more serious issue that this unlocks, the infamous #1014 | 11:26 |
gitlab-br-bot | Issue #1014: bst build --track-all doesn't build anything https://gitlab.com/BuildStream/buildstream/issues/1014 | 11:26 |
jennis | How long have the docs been for bst-1.2 instead of master? | 11:29 |
tristan | jonathanmaw, Damn, it appears that it's not correct and breaks tracking of junctions themselves :-/ | 11:30 |
jennis | The bst-1.2 docs do not include the "advanced" junctions tutorial | 11:30 |
tristan | jennis, I am not aware of that | 11:30 |
jennis | Of what, the docs being for bst-1.2 or the docs not having junction tutorials? | 11:30 |
tristan | jennis, I believe the docs have been generated from master forever, and there is an open issue about how we should be generating both separately | 11:30 |
jennis | https://buildstream.build/ | 11:31 |
tristan | docs.buildstream.build | 11:31 |
jennis | I went there ^ then clicked documentation and it's for 1.2 | 11:31 |
tristan | How do you know this for sure ? | 11:31 |
tristan | jennis, https://gitlab.com/BuildStream/buildstream/blob/master/.gitlab-ci.yml#L366 | 11:32 |
tristan | We certainly only ever publish the docs built from master | 11:32 |
tristan | Now | 11:33 |
jennis | i) I greped for junction tutorials in master, they exist, and they do not exist in bst-1.2 ii) that link: docs.buildstream.build has a badge: 1.2.6+10.g879dca35 | 11:33 |
tristan | There is https://gitlab.com/BuildStream/docs-website/merge_requests/1 which I noticed in my inbox | 11:33 |
tristan | Frankly, I am not aware of any repository named docs-website | 11:33 |
cs-shadow | jennis: https://buildstream.gitlab.io/buildstream/ still has stuff from master | 11:33 |
tristan | I don't know why such a repository exists | 11:33 |
cs-shadow | it's just docs.buildstream.build that's now serving from 1.2 after https://gitlab.com/BuildStream/docs-website/merge_requests/1 | 11:34 |
tristan | coldtom, Any idea what docs-website is for ? | 11:34 |
jennis | Ok that is clearer | 11:34 |
tristan | cs-shadow, From what I recall... there is some kind of magic redirect which causes some dns-foo to make https://buildstream.gitlab.io/buildstream/ appear at docs.buildstream.build | 11:34 |
jennis | tristan, I'm very sure that https://docs.buildstream.build/ is for bst-1.2 | 11:34 |
tristan | But now there is this *other* repository ? That is frankly... weird | 11:35 |
tristan | jennis, Yes I do believe you | 11:35 |
jennis | heh ok | 11:35 |
tristan | jennis, I am a bit peeved as to why there is moving parts here | 11:35 |
tristan | Like, what is the point of this middle-man docs-website at all ? | 11:35 |
jennis | Me too, without any notification :p | 11:35 |
tristan | jennis, you have to watch the project ;-) | 11:36 |
jennis | I need more eyes | 11:36 |
cs-shadow | tristan: i don't think there's dns-foo happening, it simply seems to unpack the zip file from the CI run - https://gitlab.com/BuildStream/docs-website/blob/master/.gitlab-ci.yml#L11 | 11:36 |
tristan | Not "participate", "participate" is for fly-by patchers who dont want to be involved :) | 11:36 |
tristan | So... can we nuke docs-website off the face of the earth ? | 11:36 |
*** lachlan has joined #buildstream | 11:36 | |
* cs-shadow doesn't has any insights into why it was setup this way but remembers being confused by this yesterday | 11:37 | |
tristan | I was also, when seeing coldtom's MR for a mysterious repo - but I didn't have time to give it thought | 11:37 |
tpollard | I recall we now have an artifact cache for the docs tutorials | 11:38 |
*** lachlan has quit IRC | 11:41 | |
tristan | jonathanmaw, juergbi: I changed the approach for https://gitlab.com/BuildStream/buildstream/merge_requests/1333, instead of modifying junction.py, I've done 's/element._update_state()/element._set_required()/' in _loader/loader.py | 11:43 |
juergbi | tristan: works for me | 11:44 |
tristan | Alright | 11:44 |
*** raoul has quit IRC | 11:45 | |
tristan | juergbi, fwiw, this change does cause the test case on !1332 (master) to fail in the expected way (for #1014) | 11:46 |
gitlab-br-bot | MR !1332: WIP: Fix build track all no strict https://gitlab.com/BuildStream/buildstream/merge_requests/1332 | 11:46 |
gitlab-br-bot | Issue #1014: bst build --track-all doesn't build anything https://gitlab.com/BuildStream/buildstream/issues/1014 | 11:46 |
abderrahim[m] | tristan: Hi, seems you made a good progress on the issue :) | 11:59 |
abderrahim[m] | tristan: btw, I reported #993 some time ago (which you re-discovered today as #1018). Should I tag such issues critical so they won't go unnoticed? | 12:00 |
gitlab-br-bot | Issue #993: BUG when building a project with junctions in non-strict mode https://gitlab.com/BuildStream/buildstream/issues/993 | 12:00 |
gitlab-br-bot | Issue #1018: BuildStream crashes on junctions in non-strict mode https://gitlab.com/BuildStream/buildstream/issues/1018 | 12:00 |
tristan | abderrahim[m], I don't think you need to tag them for them to get noticed :) | 12:05 |
tristan | abderrahim[m], We are probably overloaded in general, or I am | 12:05 |
tristan | A better fix for the issue of people not noticing newly filed issues, *cough* would be for people to "watch" the project rather than just "participate" in it | 12:06 |
* abderrahim[m] is just participating :p | 12:07 | |
tristan | abderrahim[m], interesting, I didn't know how far back that issue went (in terms of git history) | 12:08 |
tristan | I will close it as a duplicate of the new one, since the merge request is already going to close the new one | 12:08 |
tristan | But I will carry your research over and mention the offending commit in #1018 for posterity | 12:08 |
abderrahim[m] | yeah, I was going to close it anyway | 12:08 |
gitlab-br-bot | tristanvb closed issue #993 (BUG when building a project with junctions in non-strict mode) on buildstream https://gitlab.com/BuildStream/buildstream/issues/993 | 12:09 |
tristan | abderrahim[m], the bisection is very appreciated though, so I copied that over so it is traceable (anyone git blaming that new line in loader.py can trace it to the merge request and have information on what caused that line to be important) | 12:12 |
tristan | I'm still a bit stumped about how to go about fixing the --track-all issue | 12:14 |
tristan | Probably leave the actual fix for tomorrow | 12:14 |
*** raoul has joined #buildstream | 12:14 | |
tristan | given what we've learned, I'm certainly hopeful that anyone else might have an opinion on how to fix it | 12:15 |
*** lachlan has joined #buildstream | 12:17 | |
*** lachlan has quit IRC | 12:21 | |
tpollard | juergbi: in terms of https://gitlab.com/BuildStream/buildstream/merge_requests/1292#note_167128387 would the logic/helper method for generating the fullname/key be best placed in utils? | 12:53 |
tpollard | so both element and the test share class can access it | 12:53 |
juergbi | tpollard: I think I'd rather place this inside element.py but other people might see this differently | 12:55 |
juergbi | to me utils is about generic utility functions | 12:55 |
tpollard | juergbi: hmm, would you be ok with that test class importing element then for that purpose? as it doesn't receive an element object to call on | 12:57 |
*** tristan has quit IRC | 12:58 | |
juergbi | I'd be ok with that | 12:59 |
tpollard | cool | 13:00 |
juergbi | tpollard: that said, it might make sense to have a method on the cli test class to get the ref for an element (instead of just getting the key) | 13:01 |
juergbi | and maybe we should actually have a CLI format for this | 13:01 |
juergbi | in which case we might not need such a static element.py method | 13:02 |
juergbi | however, that's probably out of scope of this MR | 13:02 |
juergbi | although the first point could be easy enough (if we don't have to construct the ref any other way in other test code) | 13:03 |
tpollard | yeh maybe have it in cli is better, and then passing the ref directly into the share class | 13:07 |
tpollard | I'll give it a go | 13:07 |
*** tristan has joined #buildstream | 13:25 | |
*** lachlan has joined #buildstream | 13:30 | |
gitlab-br-bot | shashwatdalal opened MR !1336 (pip-elem-install-from-pip-source->master: Pip elem install from pip source) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1336 | 13:34 |
*** lachlan has quit IRC | 13:36 | |
*** lachlan has joined #buildstream | 13:38 | |
*** lachlan has quit IRC | 13:42 | |
*** lachlan has joined #buildstream | 13:48 | |
*** lachlan has quit IRC | 13:52 | |
*** nimish2711 has joined #buildstream | 14:02 | |
tpollard | juergbi: before I go about changing all the tests to use it, what do you think about this implementation? https://paste.gnome.org/px951zanl | 14:11 |
tpollard | in place of the current commit | 14:14 |
juergbi | tpollard: I think this is definitely an improvement. the element method should be internal, though | 14:16 |
tpollard | I could add cache_key as a default parameter actually for cases where the test case has already retrieved it, removing the need for extra bst show calls | 14:16 |
juergbi | and there is one part that is still duplicated which is splitext / replace(os.sep, '-') | 14:16 |
tpollard | yep | 14:16 |
tpollard | it felt a bit heavy splitting 'self.normal_name = os.path.splitext(self.name.replace(os.sep, '-'))[0]' out into a method | 14:17 |
tpollard | but should I? | 14:17 |
*** lachlan has joined #buildstream | 14:18 | |
tpollard | and when you say internal for the element method, single underscored but still a non class method? | 14:18 |
gitlab-br-bot | cs-shadow approved MR !1336 (pip-elem-install-from-pip-source->master: Pip elem install from pip source) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1336 | 14:23 |
*** lachlan has quit IRC | 14:26 | |
*** phildawson_ is now known as phildawson | 14:41 | |
phildawson | Hi all, please could I have some review on https://gitlab.com/BuildStream/bst-plugins-experimental/merge_requests/9 | 14:42 |
*** nimish2711 has quit IRC | 14:42 | |
*** nimish2711 has joined #buildstream | 14:43 | |
*** lachlan has joined #buildstream | 14:59 | |
*** lachlan has quit IRC | 15:03 | |
*** lachlan has joined #buildstream | 15:05 | |
*** tristan has quit IRC | 15:07 | |
*** tristan has joined #buildstream | 15:10 | |
*** lachlan has quit IRC | 15:10 | |
*** lachlan has joined #buildstream | 15:11 | |
gitlab-br-bot | pointswaves opened issue #1019 (Buildstream fails to spot bad project.conf) on buildstream https://gitlab.com/BuildStream/buildstream/issues/1019 | 15:12 |
*** lachlan has quit IRC | 15:28 | |
*** lachlan has joined #buildstream | 15:29 | |
*** lachlan has quit IRC | 15:35 | |
*** lachlan has joined #buildstream | 15:39 | |
*** lachlan has quit IRC | 15:43 | |
gitlab-br-bot | aevri opened (was WIP) MR !1335 (aevri/retry_flag->master: jobs/job.py: refactor, rm redundant _retry_flag) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1335 | 15:47 |
*** rdale has quit IRC | 16:12 | |
*** lachlan has joined #buildstream | 16:13 | |
*** lachlan has quit IRC | 16:16 | |
*** lachlan has joined #buildstream | 16:21 | |
*** nimish2711 has quit IRC | 16:24 | |
*** lachlan has quit IRC | 16:27 | |
*** lachlan has joined #buildstream | 17:04 | |
*** phildawson_ has joined #buildstream | 17:08 | |
*** phildawson has quit IRC | 17:09 | |
*** raoul has quit IRC | 17:13 | |
*** lachlan has quit IRC | 17:21 | |
*** lachlan has joined #buildstream | 17:26 | |
*** lachlan has quit IRC | 17:30 | |
*** jonathanmaw has quit IRC | 17:31 | |
gitlab-br-bot | cs-shadow approved MR !1335 (aevri/retry_flag->master: jobs/job.py: refactor, rm redundant _retry_flag) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1335 | 17:51 |
*** lachlan has joined #buildstream | 18:07 | |
*** nimish2711 has joined #buildstream | 18:08 | |
*** lachlan has quit IRC | 18:10 | |
*** nimish2711 has quit IRC | 18:12 | |
*** phildawson_ has quit IRC | 18:31 | |
*** lachlan has joined #buildstream | 18:36 | |
*** lachlan has quit IRC | 18:53 | |
*** nimish2711 has joined #buildstream | 19:16 | |
*** lachlan has joined #buildstream | 19:29 | |
*** lachlan has quit IRC | 19:35 | |
*** lachlan has joined #buildstream | 19:40 | |
*** lachlan has quit IRC | 19:43 | |
*** slaf has quit IRC | 20:31 | |
*** toscalix has quit IRC | 20:47 | |
*** slaf has joined #buildstream | 20:58 | |
*** slaf has joined #buildstream | 20:58 | |
*** slaf has joined #buildstream | 20:58 | |
*** slaf has joined #buildstream | 20:59 | |
*** nimish2711 has quit IRC | 21:49 | |
*** nimish2711 has joined #buildstream | 22:00 | |
*** bochecha has quit IRC | 22:02 | |
*** nimish2711 has quit IRC | 22:12 |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!