| *** nimish has quit IRC | 00:34 | |
| *** alatiera has quit IRC | 01:03 | |
| *** xjuan has quit IRC | 02:36 | |
| *** tristan has joined #buildstream | 07:12 | |
| *** uglyfigurine has joined #buildstream | 07:25 | |
| *** kapil___ has quit IRC | 07:44 | |
| *** JimBuntu has joined #buildstream | 07:48 | |
| *** nimish has joined #buildstream | 07:53 | |
| *** monoidal has joined #buildstream | 07:54 | |
| *** nimish has quit IRC | 08:53 | |
| *** nimish has joined #buildstream | 08:53 | |
| *** Trevinho has quit IRC | 08:54 | |
| *** nimish has quit IRC | 09:03 | |
| *** nimish has joined #buildstream | 09:04 | |
| gitlab-br-bot | tristanvb opened MR !993 (tristan/misc-cleanup->master: Misc cleanups) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/993 | 09:06 |
|---|---|---|
| *** nimish has quit IRC | 09:09 | |
| *** nimish has joined #buildstream | 09:09 | |
| *** finn has joined #buildstream | 09:13 | |
| *** ChanServ sets mode: +o tristan | 09:20 | |
| tristan | juergbi, I am thinking of quickly new Source APIs `get_tracking_scheduled()` and `get_tracking_done()` | 09:20 |
| tristan | For the purpose of these two new warnings which can be fatal | 09:20 |
| juergbi | hm, public? | 09:20 |
| tristan | (A) git:unlisted-submodule, warns if a submodule exists, checking out of submodules is enabled, but it is not specified in the configuration | 09:21 |
| tristan | (B) git:invalid-submodule, warns if the configuration specifies a submodule which does not exist in the underlying repo | 09:21 |
| tristan | The thing is, when tracking happens, you are only really interested in these warnings *after* tracking happens, if it's going to happen | 09:22 |
| tristan | And the fact that they can be fatal, would mean a lot of headache configuring things when running `bst track` | 09:22 |
| tristan | Right now the core knows very well if a source will be tracked (so the plugin can avoid emitting the warning) and when it has been tracked (so it can start emitting the warning) | 09:23 |
| tristan | juergbi, any idea for a better approach ? | 09:23 |
| juergbi | the main issue for this is that track() runs in the subprocess? | 09:24 |
| tristan | I don't see how that is relevant no | 09:24 |
| tristan | The issue is that whether a submodule exists or not in the underlying repo, depends on the ref | 09:25 |
| tristan | which is discovered as a result of tracking | 09:25 |
| tristan | Or... maybe I don't need this and I'm jumping the gun | 09:25 |
| juergbi | and why can't this be done at the end of track()? | 09:25 |
| tristan | juergbi, Maybe a Source already knows this by virtue of being Source.INCONSISTENT ? | 09:26 |
| tristan | Or that stopped being true a long time ago | 09:26 |
| tristan | juergbi, The warning needs to happen in every other scenario | 09:26 |
| tristan | bst show, bst fetch, bst build... etc | 09:26 |
| tristan | Ah right... so a Source *is* forced inconsistent until such a time that it is tracked in this scenario | 09:27 |
| tristan | it is just that the ref is not cleared | 09:27 |
| tristan | Hrmm, except that we only cache the forced value hidden | 09:28 |
| juergbi | couldn't the plugin check this at the beginning of fetch() and build()? | 09:28 |
| juergbi | (ignoring bst show for now) | 09:28 |
| juergbi | eh, fetch() and stage() | 09:28 |
| tristan | That is suboptimal | 09:28 |
| juergbi | btw: also need to think about SourceCache | 09:29 |
| tristan | Ideally you want any possibly fatal warnings to happen at the earliest opportunity | 09:29 |
| tristan | Before you've actually been computing things | 09:29 |
| tristan | Only in the case of tracking, it needs to be postponed | 09:29 |
| juergbi | in a way we want something like a preflight() but for the post-tracking phase | 09:30 |
| tristan | Not really no | 09:30 |
| tristan | We need to also avoid triggering the warning with the knowledge that the current ref is not the one that will be used | 09:30 |
| tristan | We need to avoid triggering it in the case that it will be tracked | 09:31 |
| juergbi | that's why I said post-tracking | 09:31 |
| *** toscalix has joined #buildstream | 09:31 | |
| juergbi | preflight is probably the wrong word / confusing | 09:31 |
| tristan | juergbi, Yeah I know, except the existence of a method for that is not enough alone | 09:32 |
| tristan | Oh | 09:32 |
| *** kapil___ has joined #buildstream | 09:32 | |
| tristan | You mean a check "once we know the real ref" | 09:32 |
| juergbi | yes | 09:32 |
| tristan | Which would be run right away | 09:32 |
| tristan | It is also unfortunately not entirely suitable no | 09:32 |
| juergbi | right after get_consistency() returns consistent state | 09:32 |
| tristan | juergbi, When we run `bst show` before *fetching*, the plugin does not have the git repo on hand yet, so it cannot issue the warning | 09:33 |
| *** toscalix has quit IRC | 09:33 | |
| tristan | Only when we are fully consistent can we issue the warning | 09:33 |
| juergbi | so we could trigger it when get_consistency() returns cached? | 09:33 |
| tristan | So: first bst show = no warning... bst fetch = warning... consecutive bst show = warning | 09:34 |
| tristan | Right | 09:34 |
| juergbi | check_cache() | 09:34 |
| juergbi | or something like this | 09:34 |
| tristan | When a source becomes fully consistent would be the right time | 09:34 |
| *** nimish has quit IRC | 09:34 | |
| juergbi | _update_state() could trigger this easily, afaict | 09:34 |
| *** nimish has joined #buildstream | 09:35 | |
| tristan | I suppose, there is a bit of mish-mash around the initial load time | 09:35 |
| tristan | Because of (A) loading the elements... (B) scheduling tracking... care is needed between (A) and (B), after (B) we'd be all set | 09:35 |
| *** toscalix has joined #buildstream | 09:36 | |
| tristan | juergbi, I suppose a consistency_changed() thing could be given to the plugin | 09:36 |
| tristan | but that is a bit confusing since the plugin incorrectly believes that it is driving consistency by implementing get_consistency() | 09:36 |
| tristan | (i.e. it doesnt necessarily know that we artificially force it to be inconsistent in the case of tracking) | 09:37 |
| *** Trevinho has joined #buildstream | 09:37 | |
| juergbi | I'd prefer a more explicit check_cache() only for this state transition | 09:37 |
| gitlab-br-bot | tristanvb merged MR !993 (tristan/misc-cleanup->master: Misc cleanups) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/993 | 09:37 |
| juergbi | or is this not sufficient? | 09:38 |
| tristan | I think it is | 09:38 |
| tristan | Just thinking about it | 09:38 |
| tristan | juergbi, I think it is probably best yeah | 09:38 |
| tristan | juergbi, Currently my branch has redundant warnings being emitted and this would also fix that problem | 09:39 |
| tristan | Asides from this, I am getting very obnoxious breakage when building GNOME after the execution environment changes which landed yesterday | 09:40 |
| tristan | I wonder if we can improve that, the situation is doubly bad because of the junction (the default x86_64 derived from uname breaks the build, and is unfixable because of passing the parameters through junctions) | 09:41 |
| tristan | Oh yeah, and when you load freedesktop-sdk by manually specifying *two* arch options... you get pages of: | 09:43 |
| tristan | Exception ignored in: <function WeakValueDictionary.__init__.<locals>.remove at 0x7efc4a5640d0> | 09:43 |
| tristan | Traceback (most recent call last): | 09:43 |
| tristan | File "/usr/lib/python3.5/weakref.py", line 117, in remove | 09:43 |
| tristan | TypeError: 'NoneType' object is not callable | 09:43 |
| tristan | Also not fun :-S | 09:43 |
| *** nimish has quit IRC | 09:45 | |
| tristan | juergbi, Maybe we are missing the a part which conveniently translates host uname output to standardized arch names ? | 09:45 |
| *** nimish has joined #buildstream | 09:45 | |
| tristan | So only the projects need to be updated but things don't have to be manually specified ? | 09:45 |
| * tristan is not sure at this point what part of this is making it such a struggle | 09:46 | |
| tristan | haven't been focusing on that I admit, just been testing my warnings against working projects and frustrated with breakage :-S | 09:46 |
| juergbi | tristan: yes, I noticed, starting a fdo-sdk build myself | 09:46 |
| juergbi | I don't think anything is really missing but it needs changes in more places than I expected | 09:46 |
| tristan | juergbi, If we change the arch names used in both freedesktop-sdk and gnome-build-meta, then by default the user would not have to specify `--option` when building for host arch right ? | 09:47 |
| tristan | maybe that is not escapable | 09:47 |
| juergbi | yes, I already did this locally | 09:47 |
| tristan | I guess it would be good to have that somewhere for testing purposes, until a downstream transition which can require BuildStream 1.4 | 09:51 |
| *** tpollard has joined #buildstream | 09:54 | |
| juergbi | will push my WIP branch after the test run | 09:54 |
| tristan | juergbi, It appears Source._update_state() is an ideal place for a potential check_cache() delegate method indeed, and already provides the right guarantees | 09:55 |
| *** raoul has joined #buildstream | 09:56 | |
| *** dtf has joined #buildstream | 09:59 | |
| gitlab-br-bot | jjardon opened issue #806 (overnight tests are failing: "Error loading project: Invalid value for arch") on buildstream https://gitlab.com/BuildStream/buildstream/issues/806 | 10:00 |
| *** toscalix_ has joined #buildstream | 10:05 | |
| *** tristan has quit IRC | 10:06 | |
| *** toscalix has quit IRC | 10:07 | |
| *** toscalix_ has quit IRC | 10:09 | |
| *** toscalix has joined #buildstream | 10:09 | |
| *** tristan has joined #buildstream | 10:17 | |
| *** ChanServ sets mode: +o tristan | 10:17 | |
| valentind | juergbi, I have seen more connection issues to the cache server. This time on the GNOME server. | 10:24 |
| valentind | I propose this change: https://gitlab.com/BuildStream/buildstream/commit/8610a8a4de3480c4fb1fbb657b37b475da8174ec | 10:24 |
| valentind | API doc says that client can retry when getting UNAVAILABLE. | 10:25 |
| *** jonathanmaw has joined #buildstream | 10:26 | |
| valentind | Well, actually thinking about it, I would prefer to separate the initialization of the channel with the rest of the initialization. Because we should have exponential backoff automatically then. | 10:27 |
| juergbi | valentind: don't we need to set temporary=True in the error? | 10:32 |
| juergbi | maybe I'm mixing this up with something else | 10:32 |
| valentind | juergbi, I think RpcError sets it always to true. | 10:32 |
| valentind | But otherwise yes. | 10:32 |
| juergbi | you mean CASError? | 10:32 |
| valentind | Oh yes sorry. | 10:32 |
| juergbi | ah ok | 10:32 |
| valentind | But that might be a mistake. | 10:33 |
| valentind | But this is what I saw. | 10:33 |
| valentind | I think we should actually retry any request that got UNAVAILABLE. And fail temporary all with ABORTED. | 10:34 |
| valentind | From the API doc: | 10:34 |
| valentind | Use UNAVAILABLE if the client can retry just the failing call. | 10:35 |
| valentind | Use ABORTED if the client should retry at a higher-level | 10:35 |
| valentind | (e.g., restarting a read-modify-write sequence). | 10:35 |
| *** nimish has quit IRC | 10:35 | |
| *** nimish has joined #buildstream | 10:36 | |
| *** lachlan has joined #buildstream | 10:36 | |
| *** lachlan has joined #buildstream | 10:37 | |
| *** nimish has quit IRC | 10:45 | |
| *** nimish has joined #buildstream | 10:46 | |
| tpollard | ooh, corewarnings has moved out of plugins | 10:51 |
| valentind | Did we change x86_64 to x86-64? | 10:56 |
| juergbi | valentind: yes, see https://mail.gnome.org/archives/buildstream-list/2018-November/msg00003.html | 10:57 |
| juergbi | this requires more changes than in fdo-sdk than expected | 10:57 |
| juergbi | btw: I have a local branch already that at least mostly changes this | 10:58 |
| juergbi | but we may want to revisit compatibility concerns | 10:58 |
| tristan | tpollard, I did that yes, it's better to have the simple data types all in types.py as it makes it easier to avoid circular imports and circular module dependencies (that's what types.py is for) | 10:58 |
| tpollard | tristan: I think it's a good change :) | 10:58 |
| tristan | tpollard, Also it is fixed so that plugins can import it in the regular way (from buildstream import CoreWarnings) | 10:58 |
| valentind | OK so we need to change the parameters to the overnight builds. | 10:59 |
| tristan | juergbi, I wonder if we could be *permissive* in a sense to avoid this breakage | 10:59 |
| tristan | juergbi, I.e. could we in some way make the official thing "x86-64", but *allow* x86_64 to be an alias for it ? | 11:00 |
| juergbi | tristan: the issue is that there are lots of == checks | 11:01 |
| juergbi | we can't really do both | 11:02 |
| juergbi | unless we scan the list of accepted values and decide for one or the other based on that | 11:02 |
| valentind | It seems to me that there is a different CASRemote for every job. That means that we create many http2 connections. | 11:02 |
| juergbi | tbh, I'd prefer the format-version approach | 11:02 |
| tristan | juergbi, We could have a table of acceptable aliases ? | 11:02 |
| valentind | Should not we share the channel? | 11:02 |
| juergbi | valentind: yes, not sure whether we can properly fix this as long as we use fork() | 11:02 |
| valentind | I think this is the problem why clients fail often. | 11:03 |
| Nexus | juergbi: What are your thoughts on making the idea of "skipping elements with failing assert directives" work? | 11:03 |
| juergbi | tristan: maybe, yes. first get the new standardized name internally and then check aliases against the accepted list in the arch option | 11:04 |
| juergbi | valentind: gRPC has connection limits? | 11:04 |
| juergbi | valentind: long term I'd like to move away from non-execve forks but that'd be a huge undertaking | 11:05 |
| juergbi | if you have an idea how to handle it while keeping fork, I'm all ears | 11:05 |
| juergbi | tristan: we'd also have to do the alias handling / normalization in SandboxConfig, but that should be doable | 11:06 |
| juergbi | tristan: and we could even do some generic (non-table based) normalization such as accept _ for - | 11:07 |
| juergbi | Nexus: I'm not sure whether there are any technical hurdles. tristan raised concerns about this approach. I haven't thought about it in detail yet | 11:08 |
| tristan | juergbi, indeed, I think this seems like a good first step; and if we ever run into problems in the future we could have strict arch names as an optional thing | 11:08 |
| Nexus | juergbi: Can we work together to try and come up with a plan for how it could work? | 11:10 |
| tristan | juergbi, Nexus; My concerns is that results/behavior becomes unpredictable/undefined under that clause. If over the course of project development someone adds or removes an element, that can have side effects on the intents of what "build everything" should do. It is safer to either have a project which it works without a specified target element, and force people to be explicit otherwise, to remove any elements of surprise. | 11:11 |
| tristan | I think we had already a good plan for this the other day: Allow it to work if everything loads successfully, and only support an explicit default target otherwise. | 11:12 |
| juergbi | cs-shadow who raised this issue says that wouldn't solve his use case | 11:13 |
| juergbi | see ML post | 11:13 |
| juergbi | tristan: unpredictable/undefined is an exaggeration, imo, though | 11:14 |
| juergbi | it would still be fully defined and deterministic from the project author point of view | 11:14 |
| *** nimish has quit IRC | 11:16 | |
| tristan | juergbi, Reading his post I'm not convinced | 11:16 |
| *** nimish has joined #buildstream | 11:17 | |
| tristan | juergbi, cs-shadow says "If we are saying that that is only doable if we have a stack element that lists all the dependencies, then I don't think it solve the original use-case." | 11:17 |
| tristan | But I don't think that is what we are saying | 11:17 |
| tristan | juergbi, We are only saying it is only doable with a stack element *if* there are things which fail to load in the unspecified default target case, aren't we ? | 11:17 |
| tristan | Or did I misunderstand something | 11:18 |
| tristan | I.e. if you have organized your project such that everything is always compatible, then it is possible to "build everything" | 11:18 |
| juergbi | tristan: I understood it in such a way that he might use platform-specific elements himself in the future, and then he would have to resort to the stack element | 11:18 |
| tristan | Well | 11:19 |
| juergbi | right now the previously planned approach would work for him, afaict, as we would still default to all elements and he wrote that he doesn't have platform-specific elements yet | 11:19 |
| tristan | juergbi, we could instead explore more complicated territory where: | 11:19 |
| tristan | * BuildStream tries every possible combination of project options | 11:19 |
| tristan | * BuildStream summarizes clearly at the end which elements, under which option combinations were not compatible | 11:20 |
| *** tiagogomes has quit IRC | 11:20 | |
| tristan | juergbi, it seems that somewhere in between is not really satisfactory anyway | 11:20 |
| *** alatiera has joined #buildstream | 11:20 | |
| *** tiagogomes has joined #buildstream | 11:20 | |
| tristan | juergbi, Maybe at least maintaining a list of "valid option combinations" can help reduce how exponential that can become | 11:21 |
| juergbi | tristan: hm, why can't we just respect the specified project options (or defaults if not specified)? | 11:21 |
| juergbi | i.e., requiring a separate 'all' run per platform is reasonable, imo | 11:21 |
| tristan | Then the activity of "build everything" is not really automated anyway ? | 11:21 |
| juergbi | build everything under a particular project configuration | 11:22 |
| juergbi | that would still be useful, in my opinion | 11:22 |
| juergbi | but don't know whether cs-shadow agrees | 11:22 |
| tristan | juergbi, I give up, but propose that there be a way to have either/or - the angle I am seeing is that: "I have setup CI to build absolutely everything", and I do not want a change in project data to change the meaning of my CI to be "Just build everything you can" | 11:24 |
| tristan | When I say "everything", I would like it to continue to mean everything, regardless of what gets added/removed | 11:24 |
| juergbi | imo, a real CLI user (not CI), would expect a single platform build. similarly to a user typing ./configure && make all not expecting a build on all platforms | 11:26 |
| juergbi | CI can easily have a loop with the different option configurations | 11:27 |
| juergbi | and might even run them on different machines or whatever | 11:27 |
| juergbi | but maybe that's just me | 11:27 |
| tristan | juergbi, for some people, project options have to do with platforms | 11:27 |
| tristan | juergbi, for other people, project options might mean "Am I building an SDK or a system image, and have conditional elements and configure options for this" | 11:28 |
| tristan | And for others, it means a combination of both | 11:28 |
| juergbi | I still think 'building a single project configuration' is a reasonable goal | 11:29 |
| juergbi | no matter whether that's platform or something else | 11:29 |
| juergbi | and without self-junctions, bst build anyway only supports a single project configuration per session | 11:29 |
| tristan | I think that is orthogonal to what "everything" means | 11:29 |
| juergbi | so it shouldn't be a surprise to the user that it doesn't build other project option values | 11:29 |
| juergbi | we're talking about a default 'bst build' etc. here. going over all possible configurations would surprise me a lot, imo | 11:30 |
| juergbi | this is not bst build --very-special-try-to-build-everything | 11:30 |
| juergbi | (which could be interesting as well but `bst build` without arguments should not iterate over project configs) | 11:31 |
| tristan | It's try to build everything, I don't see any difference with that and "very special" | 11:31 |
| juergbi | the user should be explicit about something that broad | 11:31 |
| juergbi | i.e., if we were to add that multi-plaform build-all feature, I'd definitely want a special flag for it | 11:32 |
| tristan | juergbi, Regardless of whether it captures all project configurations or not... if I setup a CI which tries to build everything; it probably does build everything when I initially set it up | 11:32 |
| juergbi | while I think it's reasonable for a default 'bst build' to build everything it can for the default/configured project options | 11:32 |
| tristan | juergbi, I would not want it to start becoming "lenient" just because somebody added an incompatible element | 11:32 |
| tristan | I would rather be notified straight away that CI is broken, because *everything* doesnt build anymore | 11:32 |
| juergbi | if the element explicitly asserts platform == "aarch64" | 11:33 |
| juergbi | I don't think we should error out when trying to build all elements on x86-64 | 11:33 |
| juergbi | as that assert is a clear statement | 11:33 |
| juergbi | we should _not_ silently ignore non-assert errors | 11:33 |
| tristan | See, at that point I would probably say "My project is complex enough that now I have to maintain default stack elements and be very explicit about what my "bst build" does" | 11:34 |
| juergbi | there is definitely an argument for that | 11:34 |
| tristan | bottom line is that I want to fail when anything fails | 11:35 |
| tristan | juergbi, Circling back - I do give up in the sense that, I can see that people *want* something lenient and less defined | 11:35 |
| tristan | juergbi, but I would prefer that it be a choice, and the user is warned what they are getting into | 11:35 |
| tristan | This might just be a matter of adding `--allow-incompatible` or something | 11:35 |
| tristan | I don't know what is the best design | 11:36 |
| juergbi | ok, fair enough | 11:36 |
| juergbi | could also be in userconfig, i suppose? | 11:36 |
| juergbi | skip-incompatible | 11:36 |
| juergbi | but I'd only want this skipping for the default target. if you explicitly have a dependency on something incompatible, I don't think we should allow it in any case | 11:36 |
| tristan | juergbi, Maybe that would be part of a "default-build:" configuration block ? | 11:36 |
| juergbi | that sounds reasonable | 11:37 |
| tristan | Seems to be specific to the case, and might be the right place to add default targets (if we're not doing that at the same time) | 11:37 |
| juergbi | shall we propose that on the ML and see what others think and whether that would work for cs-shadow? | 11:37 |
| tristan | Sure, I've not been in the loop really on this | 11:38 |
| juergbi | Nexus: have you been following the discussion, can you write this up? | 11:38 |
| Nexus | been trying to, sure | 11:38 |
| juergbi | ta, let me know if something is unclear | 11:39 |
| juergbi | or if you see an issue | 11:39 |
| tristan | juergbi, Regarding Source.check_cache(), I've implemented that and it has one drawback - the warnings do not fire as a result of running `bst track`; because a git Source is not Consistency.CACHED until it actually downloads the submodules (which is correct) | 11:39 |
| tristan | However it *can* know that the configurations are wrong once tracking is complete | 11:40 |
| tristan | I wonder, perhaps I could just do the same check at the end of Source.track() anyway | 11:40 |
| tristan | even if it is redundant | 11:40 |
| tristan | Or, just leave the warning out until the next show/build/fetch invocation | 11:41 |
| juergbi | not sure. if the git plugin ever tracks without fetching, it would actually not know it yet at that point | 11:42 |
| tristan | juergbi, that is a good argument to just leave the error out until a Source is actually Consistency.CACHED | 11:43 |
| juergbi | yes | 11:43 |
| tristan | juergbi, I think your wording was confusing but I think I gleaned your meaning; i.e. derive the new ref from the remote without downloading any other data | 11:44 |
| tristan | which would be optimal indeed, I've been wanting that | 11:44 |
| tristan | Ok lets run with Source.check_cache() like this... Any thoughts on the name of the API ? | 11:44 |
| tristan | stay with "check_cache()" ? | 11:44 |
| juergbi | yes, that was what I meant :) | 11:44 |
| tristan | preflight_cache() I is more consistent with other names, but is not *really* a preflight, as it can happen in mid-flight | 11:45 |
| juergbi | can't think of another name right now, but you know, naming is hard | 11:45 |
| tristan | yeah | 11:45 |
| juergbi | tristan: another option would be validate_cache() | 11:46 |
| tristan | also check_cache() is a "do this" kind of name, rather than a source_became_cached() like name, which is a "hey by the way, your source became cached in case you wanted to know" | 11:46 |
| juergbi | you'd prefer the latter? | 11:47 |
| tristan | Not sure | 11:47 |
| tristan | just throwing it out there that there is a second avenue :) | 11:47 |
| tristan | validate sounds a bit better (more specific) to me | 11:47 |
| juergbi | yes, I think I also prefer it | 11:49 |
| juergbi | wondering whether validate_cache() or validate_cached() is better | 11:49 |
| tristan | At least with this, we wont have warnings/failures for submodules in advance of tracking, and also there is only one warning/error possible (no redundancy) | 11:49 |
| tristan | So my patch is much improved with this approach | 11:50 |
| tristan | juergbi, the former | 11:50 |
| juergbi | :) | 11:50 |
| tristan | juergbi, validate_cache() says to me "Please validate your cache", validate_cached() says to me "Please validate that your data is in fact, actually cached" | 11:50 |
| *** abderrahim has quit IRC | 11:50 | |
| juergbi | yes, you're probably right | 11:51 |
| juergbi | I meant it as 'validate now that the source is cached' | 11:51 |
| juergbi | but that's confusing | 11:51 |
| tristan | yeah I know :) | 11:51 |
| *** abderrahim has joined #buildstream | 11:51 | |
| juergbi | would probably have to be cached_validate(), but I prefer validate_cached() | 11:51 |
| juergbi | eh, validate_cache() | 11:51 |
| *** nimish has quit IRC | 11:52 | |
| *** nimish has joined #buildstream | 11:52 | |
| *** toscalix has quit IRC | 11:52 | |
| *** kapil___ has quit IRC | 12:01 | |
| *** nimish has quit IRC | 12:02 | |
| *** nimish has joined #buildstream | 12:02 | |
| gitlab-br-bot | valentindavid opened MR !995 (valentindavid/overnight_build_arch->master: .gitlab-ci.yml: Fix arch options for overnight freedesktop-sdk build) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/995 | 12:05 |
| *** nimish has quit IRC | 12:07 | |
| *** nimish has joined #buildstream | 12:08 | |
| *** kapil___ has joined #buildstream | 12:09 | |
| *** nimish has quit IRC | 12:18 | |
| *** nimish has joined #buildstream | 12:18 | |
| tristan | Looks like we can make the ref-not-in-track warning less convoluted by placing it in validate_cache() too | 12:20 |
| tristan | pretty strange the way it is implemented to only happen at stage() and init_workspace() time, with some weird conditionals and even a 'tracked' state variable | 12:21 |
| tristan | tpollard, looks like you wrote the ref-not-in-track patches... | 12:23 |
| tpollard | I did | 12:23 |
| tristan | tpollard, Do you see any issues with doing the check as soon as the plugin knows that the sources are locally cached, instead of postponing it ? | 12:23 |
| tristan | it will make the code more legible too, and remove this 'tracked' local state | 12:23 |
| *** nimish has quit IRC | 12:23 | |
| cs-shadow | juergbi: tristan: Don't want to stir up this thread further, but just wanted to mention that if we go with the option of building everything under default project config, assuming everything loads properly, it would work for me | 12:25 |
| cs-shadow | i.e. avoiding the need to maintain a stack element that lists all leaves | 12:25 |
| tristan | cs-shadow, I haven't been following, but I think that what was proposed was to *only require* a default element be specified if everything does not load properly | 12:26 |
| tristan | cs-shadow, i.e. "everything" would always mean "everything" unless a default element is specified, and if it fails to load in either case, it just fails | 12:27 |
| cs-shadow | tristan: that sounds good to me, will follow up on the list again as I believe I may have misunderstood this point | 12:27 |
| cs-shadow | thanks | 12:28 |
| tpollard | tristan: I don't have a strong option, from what I can gather with a quick skim that should work | 12:28 |
| tristan | cs-shadow, to be honest, distinguishing why elements fail and such is also more complex, and I think that if we one day want a "load as much as you can" semantic, it could be added with more work and a --allow-incompatible flag some day in the future | 12:29 |
| tristan | tpollard, thanks for the quick skim :) | 12:29 |
| tpollard | There should be a test for it, so if it breaks.... | 12:30 |
| tristan | tpollard, one more thing, why is the check done on the *checked out* repo ? | 12:30 |
| tristan | seems strange to me that assert_ref_in_track() needs a 'fullpath' parameter at all | 12:31 |
| tristan | I'll just go ahead and change it and see what happens | 12:32 |
| *** JeSuisPasLucas666 has joined #buildstream | 12:32 | |
| *** JeSuisPasLucas666 has quit IRC | 12:32 | |
| tpollard | yes it shouldn't need the git to be checked out | 12:32 |
| tpollard | I can't remember exactly why I implemented it at stage time | 12:35 |
| tristan | I wonder one more thing | 12:38 |
| *** raoul_ has joined #buildstream | 12:38 | |
| tpollard | https://gitlab.com/BuildStream/buildstream/merge_requests/564 has some of your original points | 12:38 |
| tristan | tpollard, It appears that assert_ref_in_track() does not support the case where track is set to a commit sha | 12:38 |
| tristan | Like, the warning will trigger regardless of whether the commit is in the history of that sha or not | 12:39 |
| *** raoul has quit IRC | 12:39 | |
| tpollard | you can set track as a sha? | 12:40 |
| tpollard | I thought it was tag or branch only | 12:41 |
| tristan | You can set it to a sha yeah, it is normally a branch or tag but it can be a sha | 12:43 |
| tpollard | then the docs need to be updated I think | 12:44 |
| tpollard | sorry that I missed that, but I wasn't aware | 12:45 |
| tristan | recently added mirgration notes here: https://docs.buildstream.build/sources/git.html even recommend it :-S | 12:45 |
| tristan | Well, that'll be a separate issue | 12:45 |
| tristan | I suppose it is quite possible to fix | 12:45 |
| *** tristan has quit IRC | 12:49 | |
| tpollard | those recommendations landed after assert_ref_in_track() | 12:49 |
| tpollard | Anyhow, we should fix it wrongly triggering in that usecase | 12:50 |
| *** tristan has joined #buildstream | 13:04 | |
| *** tristan has quit IRC | 13:10 | |
| *** tristan has joined #buildstream | 13:19 | |
| *** toscalix has joined #buildstream | 13:19 | |
| *** tristan has quit IRC | 13:22 | |
| *** nimish has joined #buildstream | 13:22 | |
| *** lachlan has quit IRC | 13:34 | |
| *** tristan has joined #buildstream | 13:36 | |
| *** ChanServ sets mode: +o tristan | 13:36 | |
| tristan | damn, network catastrophe, back online | 13:37 |
| * tristan asks again: Anyone know what is the reverse of `git submodule add` ? | 13:37 | |
| *** raoul_ has quit IRC | 13:40 | |
| *** raoul_ has joined #buildstream | 13:40 | |
| tpollard | some combo of deinit and rm I think | 13:41 |
| tristan | tpollard, Apparently `git rm` does it | 13:41 |
| tristan | now that I have google back :-S | 13:41 |
| *** nimish has quit IRC | 13:42 | |
| *** nimish has joined #buildstream | 13:43 | |
| Nexus | tristan: Having finally gotten my head around what was discussed earlier, just to comfirm. Are you suggesting that we do a "build all" which will fail/error on incompatible elements *unless* you add a "skip-incompatible" to the project configuration? | 13:47 |
| tristan | Nexus, I'm suggesting that the "build all" functionality fails if there are incompatibilities, in which case you can always set a default target to be more explicit about what you want | 13:50 |
| tristan | Nexus, which cs-shadow seems to be fine with now that he understands it | 13:50 |
| tristan | Nexus, I'm also suggesting that at some undetermined future point, we can add the "try to load as much as you possibly can" thing with an additional switch for it, and that this initial approach doesnt make that impossible | 13:51 |
| tristan | Nexus, this is after the conversation which happened with cs-shadow above in the recent backlog | 13:51 |
| Nexus | ah | 13:53 |
| * Nexus reads backlog | 13:53 | |
| *** raoul__ has joined #buildstream | 14:02 | |
| *** raoul_ has quit IRC | 14:03 | |
| *** nimish has quit IRC | 14:11 | |
| *** mtreinish has joined #buildstream | 14:14 | |
| *** lachlan has joined #buildstream | 14:17 | |
| Nexus | tristan: ok, that makes sense | 14:17 |
| Nexus | tristan: are we going for defaulting to `bst build` doing the build or a "--world" flag or equivalent | 14:17 |
| *** nimish has joined #buildstream | 14:17 | |
| *** nimish has quit IRC | 14:22 | |
| *** nimish has joined #buildstream | 14:23 | |
| gitlab-br-bot | tristanvb opened MR !996 (tristan/submodule-warnings->master: Implement submodule warnings) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/996 | 14:24 |
| tristan | Nexus, (cs-shadow, juergbi ^^^^); I believe consensus is mostly that we don't need any specific flag, and it would be the behavior when launching `bst build` without arguments from somewhere inside the project directory (as opposed to inside a workspace directory, which has different consequences) | 14:27 |
| juergbi | +1 | 14:27 |
| tristan | juergbi, Care to give a lookover of !996 ? | 14:27 |
| tristan | I think it's ready | 14:27 |
| juergbi | will do | 14:28 |
| *** nimish has quit IRC | 14:28 | |
| tristan | Thanks :) | 14:28 |
| cs-shadow | I agree, so long as we are consistent with this behavior, i.e. with commands other than build | 14:28 |
| *** nimish has joined #buildstream | 14:28 | |
| cs-shadow | Also see #789 that's related | 14:28 |
| gitlab-br-bot | Issue #789: Expected behavior for running commands with no elements specified? https://gitlab.com/BuildStream/buildstream/issues/789 | 14:28 |
| tristan | cs-shadow, That is a good point, we'd probably want fetch,track,etc to behave the same way right ? | 14:29 |
| tristan | I guess that is exactly what #789 says | 14:29 |
| cs-shadow | yeah, I was just not sure which way we wanted to go, hence the question mark at the end. But, if we are going down the route of building everything when no elements are specified, we should also do the same with all the other commands that accept a list of elements | 14:30 |
| cs-shadow | like fetch, track, show etc | 14:30 |
| tristan | juergbi, As a side note, I found that this validate_cache() can be useful in other places... for instance we can have a more early error if a `tar` source configurations says "I want to extract only the directory 'foo/'" but the tarball has no such directory matching | 14:31 |
| juergbi | tristan: ok, that particular case might not be that cheap, though, so not sure whether we should | 14:31 |
| tristan | juergbi, :) | 14:31 |
| tristan | yeah | 14:31 |
| tristan | I didn't go ahead and do it, but it does make logical sense | 14:32 |
| tristan | juergbi, maybe in a future with SourceCache it might make sense to shift some processing to different places and maybe in that world it will be cheap | 14:32 |
| tristan | cs-shadow, I'm a *little* bit concerned about 'show' in this case but I guess we can "just do it" - I.e. specifically I expect the order of elements listed by show to have very specific staging order meaning | 14:33 |
| *** nimish has quit IRC | 14:33 | |
| *** nimish has joined #buildstream | 14:34 | |
| jonathanmaw | hmm, bst source-checkout creates an empty directory if a workspace is open. | 14:48 |
| jonathanmaw | Is there ever a conceivable reason to use bst source-checkout while you have a workspace open? | 14:48 |
| tristan | I'm sure there is | 14:49 |
| tristan | It could be scripted for instance, part of something else | 14:49 |
| jonathanmaw | okay, I guess the normal behaviour would be to return exactly the same thing whether a workspace is open and has been fiddled with, or not. | 14:50 |
| * jonathanmaw goes code diving to find the root cause | 14:51 | |
| tristan | jonathanmaw, I guess I'm late answering things on the mailing list, but there was an idea to have a `source` subcommand to be more symmetrical with `artifact` subcommand no ? | 14:51 |
| tristan | I saw that suggestion fly by and thought it was nice | 14:51 |
| tristan | but then, it's a tiny detail, and can be changed before/after doing the actual implementation | 14:51 |
| juergbi | tristan: !996 looks good to me. I've mainly looked at the first two commits | 14:52 |
| gitlab-br-bot | MR !996: Implement submodule warnings https://gitlab.com/BuildStream/buildstream/merge_requests/996 | 14:52 |
| juergbi | tristan: yes, but it would be good to still change this all this year | 14:53 |
| *** nimish has quit IRC | 14:54 | |
| juergbi | (well, depending on when we'll actually release 1.4) | 14:54 |
| *** nimish has joined #buildstream | 14:54 | |
| tristan | juergbi, Thanks, the first two commits are the bulk of it, besides that there is the ref-not-in-track re-implementation (corresponding tests still work and are unchanged), and the added new tests | 14:54 |
| * jonathanmaw creates an issue for the source-checkout weirdness | 14:54 | |
| tristan | juergbi, I'll postpone it due to all the stuff we want to get in, and since we made the plan to make the release early mid-cycle I think it's fair... will send email tomorrow | 14:55 |
| tristan | juergbi, I'd *like* to also make a 1.3.1 dev release with that, but wonder if we can make the arches non-breaking before a snapshot goes out | 14:55 |
| juergbi | restoring arch compatibility would indeed make sense before release. maybe I can take a stab at this at some point over the weekend | 14:58 |
| juergbi | I think it's ok if 1.3.1 is released a bit later. before Christmas would definitely be good, though | 14:58 |
| tristan | Yeah, I was wanting it to be this friday but, it could be next week :D | 14:59 |
| tristan | Anyway from here on out, I will be giving it a lot of smoke testing | 14:59 |
| gitlab-br-bot | jonathanmaw opened issue #807 (bst source-checkout creates an empty checkout if a workspace is open) on buildstream https://gitlab.com/BuildStream/buildstream/issues/807 | 15:00 |
| juergbi | I'm also trying to fix up or at least file loose ends that I encounter | 15:01 |
| *** tristan has quit IRC | 15:03 | |
| *** nimish has quit IRC | 15:04 | |
| *** nimish has joined #buildstream | 15:04 | |
| Nexus | juergbi: Where can i add a "Default-Element:" field to the project.conf? | 15:10 |
| *** nimish has quit IRC | 15:10 | |
| juergbi | I don't see a suitable existing groups, so it should probably be a new group | 15:10 |
| juergbi | it should definitely be a new group, though, not just a key, as we may want to support the 'skip-incompatible' idea in the future | 15:11 |
| juergbi | group name default-target? not sure | 15:12 |
| Nexus | Where do i add that? | 15:13 |
| juergbi | projectconfig.yaml and _project.py | 15:14 |
| gitlab-br-bot | tristanvb merged MR !996 (tristan/submodule-warnings->master: Implement submodule warnings) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/996 | 15:14 |
| Nexus | juergbi: What do you mean by group | 15:16 |
| Nexus | ?* | 15:16 |
| juergbi | I mean top-level dict | 15:17 |
| juergbi | like `shell` | 15:18 |
| Nexus | i.e. alongside variables, environment, sand-box etc? | 15:18 |
| Nexus | sandbox* | 15:18 |
| juergbi | yes | 15:19 |
| Nexus | kk | 15:19 |
| juergbi | if there is an existing group that fits, that would be great as well, of course, but I don't see any | 15:20 |
| juergbi | would be good to think about possible other options that would fit in the same new group to come up with the best group name | 15:20 |
| juergbi | but that's difficult, of course | 15:20 |
| *** tristan has joined #buildstream | 15:34 | |
| jonathanmaw | \o/ the source checkout problem is simply a matter of setting the right option! | 15:38 |
| gitlab-br-bot | jmacarthur opened (was WIP) MR !991 (jmac/cache_artifacts_with_vdir->master: Cache artifacts with virtual directories instead of filesystem.) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/991 | 15:46 |
| gitlab-br-bot | jonathanmaw opened MR !997 (jonathan/source-checkout-workspace->master: Fix bst source-checkout not working with open workspaces) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/997 | 15:47 |
| jonathanmaw | tristan: do you have a minute to check over https://gitlab.com/BuildStream/buildstream/merge_requests/997 ? | 15:49 |
| *** nimish has joined #buildstream | 16:07 | |
| *** raoul__ has quit IRC | 16:27 | |
| jonathanmaw | WSalmon, juergbi: do you have time to look at https://gitlab.com/BuildStream/buildstream/merge_requests/924 ? | 16:31 |
| juergbi | jonathanmaw: will add it to my list | 16:32 |
| jonathanmaw | tvm juergbi | 16:32 |
| jjardon | Can I have reviews of https://gitlab.com/BuildStream/buildstream-docker-images/merge_requests/72 , please? | 16:36 |
| *** nimish has quit IRC | 16:37 | |
| *** nimish has joined #buildstream | 16:37 | |
| *** raoul__ has joined #buildstream | 16:39 | |
| *** raoul__ has quit IRC | 16:45 | |
| cs-shadow | jjardon: done | 16:46 |
| jjardon | tvm | 16:46 |
| *** raoul__ has joined #buildstream | 16:47 | |
| *** nimish has quit IRC | 16:47 | |
| *** nimish has joined #buildstream | 16:48 | |
| gitlab-br-bot | cs-shadow opened issue #809 (Allow cross-junction dependencies to be listed as strings) on buildstream https://gitlab.com/BuildStream/buildstream/issues/809 | 16:52 |
| *** nimish has quit IRC | 16:53 | |
| *** nimish has joined #buildstream | 16:53 | |
| *** nimish has joined #buildstream | 16:53 | |
| gitlab-br-bot | cs-shadow approved MR !997 (jonathan/source-checkout-workspace->master: Fix bst source-checkout not working with open workspaces) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/997 | 16:54 |
| phildawson | juergbi, I think I've resolved your latest comments on !959 | 16:55 |
| gitlab-br-bot | MR !959: Retire bst source bundle command https://gitlab.com/BuildStream/buildstream/merge_requests/959 | 16:55 |
| juergbi | ok, will review the update | 16:56 |
| phildawson | Thanks | 16:56 |
| gitlab-br-bot | cs-shadow opened MR !998 (chandan/junction-dependency-format->master: loader: Allow dependencies to use ":" to refer to junctioned elements) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/998 | 16:56 |
| *** alatiera has quit IRC | 17:12 | |
| *** alatiera has joined #buildstream | 17:12 | |
| *** alatiera has quit IRC | 17:16 | |
| *** finn has quit IRC | 17:37 | |
| *** lachlan has quit IRC | 17:43 | |
| *** alatiera has joined #buildstream | 17:44 | |
| gitlab-br-bot | jonathanmaw closed issue #807 (bst source-checkout creates an empty checkout if a workspace is open) on buildstream https://gitlab.com/BuildStream/buildstream/issues/807 | 17:46 |
| gitlab-br-bot | jonathanmaw merged MR !997 (jonathan/source-checkout-workspace->master: Fix bst source-checkout not working with open workspaces) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/997 | 17:46 |
| *** tpollard has quit IRC | 17:50 | |
| juergbi | skullman: replied to your ML post | 17:51 |
| skullman | interesting, I looked at allow_interspersed_args when I started, though can't remember why I concluded it wasn't sufficient | 17:56 |
| skullman | may just have been that I like being able to have interspersed args in general | 17:56 |
| *** lachlan has joined #buildstream | 17:56 | |
| juergbi | I think disabling it at least for the shell command is reasonable. option position anyway already matters (group vs. command options) | 17:59 |
| juergbi | whether we want to disable interspersed args for other commands that don't need command nesting, I don't know | 18:00 |
| juergbi | arbitrary mixing generally doesn't make sense, imo. however, putting options at the end can sometimes be convenient | 18:01 |
| skullman | aye, comes in handy when you just need to add one extra option to a command already in your shell history | 18:02 |
| *** nimish has quit IRC | 18:03 | |
| skullman | it's vexing that we've got to jump through these hurdles to cope with click.py getting in the way | 18:04 |
| *** nimish has joined #buildstream | 18:04 | |
| skullman | though for shell, aye, the common case is that we'd never expect anyone to expect that options after the arguments work | 18:04 |
| *** nimish has quit IRC | 18:09 | |
| *** nimish has joined #buildstream | 18:10 | |
| *** nimish has quit IRC | 18:14 | |
| *** nimish has joined #buildstream | 18:15 | |
| *** nimish has quit IRC | 18:25 | |
| *** nimish has joined #buildstream | 18:25 | |
| *** jonathanmaw has quit IRC | 18:26 | |
| *** raoul__ has quit IRC | 18:27 | |
| *** nimish has quit IRC | 18:30 | |
| *** nimish has joined #buildstream | 18:30 | |
| *** nimish has quit IRC | 18:35 | |
| *** nimish has joined #buildstream | 18:36 | |
| *** lachlan has quit IRC | 18:39 | |
| *** nimish has quit IRC | 18:42 | |
| *** xjuan has joined #buildstream | 18:51 | |
| *** raoul__ has joined #buildstream | 19:12 | |
| *** tristan has quit IRC | 21:33 | |
| *** finn has joined #buildstream | 21:38 | |
| *** finn has quit IRC | 21:41 | |
| *** eMBee has joined #buildstream | 21:56 | |
| *** xjuan has quit IRC | 22:07 | |
| *** xjuan has joined #buildstream | 22:26 | |
| gitlab-br-bot | valentindavid opened issue #810 (Client should open only one HTTP2 connection to CAS cache server) on buildstream https://gitlab.com/BuildStream/buildstream/issues/810 | 22:28 |
| *** toscalix has quit IRC | 23:17 | |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!