| *** xjuan has joined #buildstream | 00:03 | |
| *** lsfranco has quit IRC | 01:03 | |
| *** lsfranco_ has joined #buildstream | 01:03 | |
| *** lsfranco_ is now known as lsfranco | 01:04 | |
| *** ajsalminen has joined #buildstream | 01:08 | |
| *** alatiera has quit IRC | 01:13 | |
| *** xjuan has quit IRC | 02:47 | |
| *** kapil___ has joined #buildstream | 02:52 | |
| *** flatmush_ has joined #buildstream | 03:07 | |
| *** flatmush has quit IRC | 03:08 | |
| *** nimish has quit IRC | 03:25 | |
| *** lsfranco has quit IRC | 04:08 | |
| *** kapil___ has quit IRC | 05:01 | |
| *** shinchiro has joined #buildstream | 05:10 | |
| *** tristan has joined #buildstream | 06:32 | |
| *** youtah has joined #buildstream | 06:47 | |
| *** tristan has quit IRC | 08:04 | |
| *** WSalmon_ has joined #buildstream | 08:37 | |
| *** WSalmon_ has quit IRC | 08:39 | |
| *** tristan has joined #buildstream | 08:42 | |
| *** WSalmon_ has joined #buildstream | 08:53 | |
| *** alatiera has joined #buildstream | 08:55 | |
| *** WSalmon_ has quit IRC | 08:58 | |
| *** ChanServ sets mode: +o tristan | 08:59 | |
| paulsher1ood | tristan: i'm worried that even after all this time we may have a different understanding of cache-key compatibility | 09:11 |
|---|---|---|
| paulsher1ood | my understanding is that *desired* behaviour is that cache-key-algorithm is versioned (and by that i mean whole number versions, not semver or anything fancy) | 09:13 |
| paulsher1ood | once we have a version of the algorithm, bst (or any other tool) should be able to calculate it | 09:14 |
| paulsher1ood | if we bump the version (because we realise we've missed an input/factor, say) we add support for the new version into bst (or any other tool) | 09:14 |
| tristan | paulsher1ood, I think that we have the same understanding, except for the "any other tool" part | 09:14 |
| *** phildawson has joined #buildstream | 09:15 | |
| paulsher1ood | i don't see why bumping the version means that a later version of bst can't support all previous versions of the algorithm | 09:15 |
| paulsher1ood | and hence i don't see why that was not done from day one on bst | 09:15 |
| paulsher1ood | any other tool could mean, for example that some other tool might want to figure out the cache-key for a given set of bst files using some particular version of the algorithm | 09:16 |
| tristan | paulsher1ood, That was not done from day one because of the efforts to support every artifact format version would have been high and caused a lot more friction during the feature development | 09:17 |
| tristan | paulsher1ood, Another script/tool should be able to use `bst show` with a specific artifact version set (when we support setting explicit artifact versions), but it could not parse a project and determine it by itself | 09:18 |
| *** toscalix has joined #buildstream | 09:18 | |
| paulsher1ood | tristan: i think think that was simply a mistake. the effort is not that high | 09:20 |
| paulsher1ood | we're just talking about some kind of hash of some inputs, after all. | 09:20 |
| tristan | paulsher1ood, Not really no | 09:23 |
| tristan | paulsher1ood, Not all plugin configurations contribute to a cache key, and plugins need to contribute what goes into the cache key | 09:23 |
| paulsher1ood | it's still just a hash of inputs | 09:24 |
| paulsher1ood | a plugin is either providing some inputs or not | 09:24 |
| tristan | And the configuration in the YAML is loaded by the plugin, the plugin is the only one who knows what that configuration means | 09:25 |
| paulsher1ood | well, 2 hashes, given the strong and weak approachs | 09:25 |
| tristan | And which part of that configuration needs to contribute to the cache key or not | 09:25 |
| gitlab-br-bot | tristanvb opened MR !1004 (tristan/stop-stripping-whitespace->master: Stop stripping whitespace) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1004 | 09:42 |
| *** jonathanmaw has joined #buildstream | 09:53 | |
| jmac | Does anyone know where we raise issues against the remote execution protocol these days? I'm guessing https://github.com/bazelbuild/remote-apis/issues | 09:56 |
| juergbi | jmac: yes, I think that's the right place. that said, I don't see an actual problem in the protocol from the ML thread | 10:00 |
| juergbi | (although it could probably use some clarification in the wording) | 10:01 |
| jmac | OK, let's say my server supports instance names, and I request this: /uploads/6c92172c-8064-4351-93a2-640d5e8761fe/blobs/187d384348c73a2c0246a42fd061167039e551c1fe8c24a51d9538f4536fa72c/1034/uploads/6c92172c-8064-4351-93a2-640d5e8761fe/blobs/187d384348c73a2c0246a42fd06116\ | 10:01 |
| jmac | 7039e551c1fe8c24a51d9538f4536fa72c/1034/uploads/6c92172c-8064-4351-93a2-640d5e8761fe/blobs/187d384348c73a2c0246a42fd061167039e551c1fe8c24a51d9538f4536fa72c/1034/foo.c | 10:01 |
| jmac | What's the instance name? | 10:01 |
| *** phildawson has quit IRC | 10:02 | |
| *** phildawson has joined #buildstream | 10:02 | |
| *** bilelmoussaoui has joined #buildstream | 10:04 | |
| *** tpollard has joined #buildstream | 10:05 | |
| *** bilelmoussaoui has left #buildstream | 10:05 | |
| valentind | tristan, would be nice to get !1000 merged and then backport to put it in the coming 1.2 release. | 10:12 |
| gitlab-br-bot | MR !1000: Force updating tags when fetching git repository https://gitlab.com/BuildStream/buildstream/merge_requests/1000 | 10:12 |
| juergbi | jmac: if I were to implement this on the server side, I would use component-level mapping. and would reject resource names that start with a / | 10:13 |
| jmac | I don't know what you mean by component-level mapping | 10:14 |
| juergbi | jmac: i.e., the instance name would depend on the server configuration. if the server is configured supporting with an instance with an empty name, your URL (if it didn't start with a /) I'd map to that instance with an empty name | 10:14 |
| juergbi | if there is no such instance configured, I'd check whether the server configuration has an instance whose first component is called 'uploads' | 10:15 |
| juergbi | looking at the buildfarm code, it does not appear to follow the protocol, though, it requires {size} to be the last URL component | 10:17 |
| juergbi | I have to admit that the whole thing is pretty odd and probably worth raising an issue | 10:17 |
| jmac | But you could legitimately have two instances, one called /uploads/6c92172c-8064-4351-93a2-640d5e8761fe/blobs/187d384348c73a2c0246a42fd061167039e551c1fe8c24a51d9538f4536fa72c/1034/ and another being that duplicated | 10:18 |
| juergbi | yes, you couldn't allow that in the server config | 10:18 |
| jmac | In which case, you may as well make a simpler rule and say slashes are not allowed in instance names for your particular server | 10:19 |
| juergbi | instance name followed by 'uploads' must not be a prefix of another instance name | 10:19 |
| jmac | But then you're applying a patch to a hole in the protocol, IMO | 10:19 |
| juergbi | you could argue that, I guess | 10:19 |
| juergbi | you could also argue it's disallowing impossible configs | 10:20 |
| juergbi | (besides 'uploads', also 'blobs' must be disallowed) | 10:20 |
| juergbi | and actions:execute and a few others | 10:20 |
| juergbi | in any case, I agree it's worth upstream discussion | 10:21 |
| juergbi | should at the very least be clarified | 10:21 |
| *** bilelmoussaoui_ has joined #buildstream | 10:26 | |
| *** bilelmoussaoui_ has left #buildstream | 10:26 | |
| *** lachlan has joined #buildstream | 10:38 | |
| *** nimish has joined #buildstream | 11:00 | |
| *** nimish has quit IRC | 11:15 | |
| *** nimish has joined #buildstream | 11:15 | |
| *** nimish has joined #buildstream | 11:16 | |
| tristan | valentind, !1000 looks fine to me | 11:19 |
| gitlab-br-bot | MR !1000: Force updating tags when fetching git repository https://gitlab.com/BuildStream/buildstream/merge_requests/1000 | 11:19 |
| tristan | valentind, I don't like how the summary talks about multiple remotes and mirroring, but I don't think the actual patch regresses or changes anything about that | 11:20 |
| tristan | valentind, I.e. if the behavior of fetch() or track() is impacted by having multiple remotes known for different mirrors, that would seem to be a bug, I think we are very explicit about fetching/tracking from one remote at a time | 11:21 |
| tristan | and if we are not, then the git plugin would appear to be breaking rules | 11:21 |
| valentind | tristan, Interestingly, on the clone, we use "origin". | 11:22 |
| valentind | Then on fetch, we create a new remote for the same url. | 11:22 |
| tristan | Seems like that is buggy indeed | 11:23 |
| valentind | When we use mirrors they might have different urls. | 11:23 |
| tristan | I guess the initial clone can only come from one of the mirrors | 11:23 |
| valentind | I think something also broke when we started to set "alias_override" even when we did not use a mirror. | 11:25 |
| valentind | I remember the code for mirroring in source.py slightly different. | 11:26 |
| valentind | So there are some more weird thing happening. However the test case I added legitimately requires a different remote. It is just that we seem to use extra remotes when we do not need to. | 11:30 |
| valentind | in _fetch, the "if alias_override:" is a wrong condition. Instead we should look if "origin" has the required url. | 11:31 |
| tristan | valentind, I think that is a separate issue from your patch, though | 11:43 |
| valentind | tristan, yes. | 11:44 |
| tristan | valentind, I.e. you patch fixes that the fetch should pull in refs, and by that virtue I think is still good, and doesnt make things worse | 11:44 |
| tristan | So lets go ahead with your patch + backport as you said | 11:44 |
| tristan | valentind, it would of course be good to raise that as a separate issue ;-) | 11:44 |
| valentind | OK, I just test changing last_commit, then I merge and backport. | 11:45 |
| *** alatiera has quit IRC | 11:47 | |
| *** alatiera has joined #buildstream | 11:52 | |
| *** abderrahim has quit IRC | 11:53 | |
| *** abderrahim has joined #buildstream | 11:54 | |
| tristan | valentind, Right that sounds sensible, I was asking the question there because I genuinely was unsure if the distinction is important | 11:59 |
| tristan | I think that changing the behavior of latest_commit() to use HEAD instead of master will be correct, though | 11:59 |
| valentind | tristan, yes, it worked. | 12:07 |
| valentind | tristan, I fixed that and simplified and clarified the commit message | 12:13 |
| *** nimish has quit IRC | 12:20 | |
| *** nimish has joined #buildstream | 12:21 | |
| tristan | valentind, I guess go ahead and merge and backport then :) | 12:24 |
| tristan | cs-shadow, I thought I'd go ahead and "just fix" that issue about stripping whitespace, since it was such a simple thing to do and has been open for months | 12:25 |
| tristan | cs-shadow, unfortunately I hit a wall: https://gitlab.com/BuildStream/buildstream/issues/403#note_124046476 | 12:25 |
| *** nimish has quit IRC | 12:26 | |
| *** nimish has joined #buildstream | 12:26 | |
| *** raoul has joined #buildstream | 12:33 | |
| *** alatiera has quit IRC | 12:42 | |
| gitlab-br-bot | valentindavid closed issue #812 (Not all git commits fetched.) on buildstream https://gitlab.com/BuildStream/buildstream/issues/812 | 12:44 |
| gitlab-br-bot | valentindavid merged MR !1000 (valentindavid/git_force_fetch_tags->master: Force updating tags when fetching git repository) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1000 | 12:44 |
| *** nimish has quit IRC | 12:51 | |
| *** nimish has joined #buildstream | 12:52 | |
| *** phildawson_ has joined #buildstream | 12:55 | |
| *** phildawson has quit IRC | 12:56 | |
| *** lachlan has quit IRC | 13:03 | |
| jonathanmaw | WSalmon: I've pushed some more changes to !924 | 13:08 |
| gitlab-br-bot | MR !924: Support invoking buildstream from a workspace outside a project https://gitlab.com/BuildStream/buildstream/merge_requests/924 | 13:08 |
| jonathanmaw | specifically: I've made it search for both the project.conf and the .bstproject.yml file; I've extended the tests to cover trying to run commands from subdirs of the workspace; and also the rest of the nitpicks. | 13:09 |
| cs-shadow | tristan: that's a shame, I thought it'd be a small one-liner patch, but apparently not | 13:09 |
| cs-shadow | tristan: FWIW I would mention that for our use-case, we don't really need this at the moment. And, it's not usually very bad when it happens, so it may not be very high priority item | 13:11 |
| *** tristan has quit IRC | 13:12 | |
| *** lachlan has joined #buildstream | 13:19 | |
| gitlab-br-bot | adds68 closed issue #816 (Unexpected caching behaviour with bst plugin) on buildstream https://gitlab.com/BuildStream/buildstream/issues/816 | 13:20 |
| *** nimish has quit IRC | 13:22 | |
| *** nimish has joined #buildstream | 13:23 | |
| Nexus | juergbi: could you have a look at https://gitlab.com/BuildStream/buildstream/merge_requests/925 and https://gitlab.com/BuildStream/buildstream/merge_requests/926 for me pls? :) | 13:33 |
| juergbi | adding them to my list | 13:33 |
| Nexus | thanks c: | 13:33 |
| juergbi | Nexus: do you think they are ready for review or do you have questions? | 13:34 |
| juergbi | they are marked WIP | 13:34 |
| Nexus | juergbi: i think they're ready for review, i haven't done the other commands, as i wanted to put that as a seperate MR | 13:35 |
| Nexus | but i wanted to make sure it matched what you expected | 13:35 |
| gitlab-br-bot | knownexus opened (was WIP) MR !925 (issue-638-validate-all-files->master: Added --all functionality to `bst show`) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/925 | 13:35 |
| gitlab-br-bot | knownexus opened (was WIP) MR !926 (issue_640-Build-All->master: Added build all functionality to `bst build`) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/926 | 13:35 |
| *** tristan has joined #buildstream | 13:36 | |
| *** nimish has quit IRC | 13:36 | |
| *** ChanServ sets mode: +o tristan | 13:37 | |
| tristan | cs-shadow, good to hear it's not that high priority | 13:37 |
| tristan | cs-shadow, maybe we can consider an only variables-get-stripped approach, but then it might have higher impact breakage on plugins | 13:38 |
| tristan | indeed the suspected behavior of auto stripping unless quoted would have been great | 13:38 |
| cs-shadow | yeah, if I find time for this, I'll look into the spec and ruamel to see if the library is doing the right thing | 13:40 |
| tristan | I did some quick googling but was unable to easily figure it out | 13:41 |
| tristan | there is a lot of material on "how to quote" for yaml | 13:41 |
| tristan | I didnt try parsing the actual spec, though | 13:42 |
| cs-shadow | :/ otherwise, stripping only variables may make sense but it may also make things more confusing | 13:42 |
| tristan | it has a higher chance of breaking plugins, though | 13:42 |
| tristan | as the default behavior would change to leading/trailing newlines for any string specified with the "|" thing | 13:43 |
| tristan | kind of drastic change, sort of feels like forcing ruamel.yaml to strip everything that is not quoted, even if defying the spec, would be better hehe | 13:43 |
| cs-shadow | to be honest, I don't think that a lot of plugins will be relying on whitestpace getting stripped but I may be wrong on that. But, I'll look more into it later in the day to see if we can get the desired behavior of only stripping non-quoted strings | 13:45 |
| tristan | cs-shadow, "track" parameters | 13:46 |
| tristan | we think that people specify them as "track: branch-name", but *if* they specify it as "track: |" and then specify the string below, suddenly the plugin gets newlines | 13:47 |
| tristan | that is weird | 13:47 |
| tristan | any string parameter really, if the user decides to express it that way, boom | 13:47 |
| cs-shadow | using "|" shouldn't have a leading newline as per the spec IIUC | 13:50 |
| cs-shadow | as per https://yaml.org/spec/1.2/spec.html#|%20literal%20style// | 13:51 |
| coldtom | tristan: would using ">" rather than "|" not strip the newlines? | 13:52 |
| tristan | cs-shadow, so the fact that we usually put an empty line between the | and the string for better readability, causes the leading newline perhaps, and the trailing newline is just not optional ? | 13:53 |
| cs-shadow | yeah the leading newline will be because of the explicit empty line and not if the text immediately follows "|" on the next line | 13:54 |
| tristan | coldtom, perhaps, I'm not familiar enough, but I've seen "|>" used and wondered what it means | 13:54 |
| tristan | cs-shadow, I would have expected that the string starts on the first line with indentation | 13:54 |
| cs-shadow | https://stackoverflow.com/a/21699210 is a good summary of all the nine (or 63, depending on how you count) different ways of having new lines in yaml | 13:55 |
| tristan | cs-shadow, i.e. in the test case I posted on the issue, the empty line is an empty line, not aligned with the beginning of the string | 13:55 |
| cs-shadow | tristan: yes, preserving that empty line is the expected behavior IMHO | 13:56 |
| tristan | cs-shadow, interesting, so we *could* say that you can use ">-" to explicitly strip | 13:57 |
| tristan | cs-shadow, not sure what you mean or if you agree or disagree... I am saying that that empty line should be ignored by YAML and really be meaningless, *because* it is different from a line with 2 leading spaces followed by a newline | 13:58 |
| tristan | if I wanted to express an empty leading line, I would put two leading spaces on that line | 13:58 |
| tristan | but this is just opinion at this point :) | 13:58 |
| cs-shadow | tristan: ah! I think I am saying that since the user put that newline explicitly after "|", I would expect it to be preserved. That is if we say that we are following the yaml spec | 13:59 |
| * cs-shadow needs to go to our standup now :) | 13:59 | |
| gitlab-br-bot | valentindavid opened MR !1005 (valentindavid/git_force_fetch_tags-1.2->bst-1.2: [backport 1.2] Force updating tags when fetching git repository) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1005 | 14:00 |
| gitlab-br-bot | knownexus closed MR !926 (issue_640-Build-All->master: Added build all functionality to `bst build`) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/926 | 14:14 |
| gitlab-br-bot | knownexus reopened MR !926 (issue_640-Build-All->master: Added build all functionality to `bst build`) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/926 | 14:15 |
| gitlab-br-bot | valentindavid merged MR !1005 (valentindavid/git_force_fetch_tags-1.2->bst-1.2: [backport 1.2] Force updating tags when fetching git repository) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1005 | 14:25 |
| WSalmon | my mr for chosing werethere or not to use a build tree for the bst shell --build https://gitlab.com/BuildStream/buildstream/merge_requests/986 is now ready for review :D | 14:33 |
| WSalmon | thanks Kinnison and tpollard for looking already | 14:33 |
| jmac | Nexus: I'm confused by !925 - where's the '--all' flag for bst show? | 14:35 |
| gitlab-br-bot | MR !925: Added --all functionality to `bst show` https://gitlab.com/BuildStream/buildstream/merge_requests/925 | 14:35 |
| Nexus | jmac: i've updated t, sorry my description was out of date | 14:38 |
| jmac | Aha | 14:38 |
| *** alatiera has joined #buildstream | 14:50 | |
| coldtom | does anyone know what the default value of %{libexecdir} is? | 14:58 |
| coldtom | ah, found it, sorry for the noise | 14:59 |
| *** toscalix_ has joined #buildstream | 15:12 | |
| Nexus | jmac: re: https://gitlab.com/BuildStream/buildstream/merge_requests/925#note_124101154 | 15:22 |
| Nexus | Won't it only remove strings that match the whole string provided? | 15:22 |
| Nexus | not the characters individually | 15:22 |
| Kinnison | lstrip() takes an iterable of characters | 15:25 |
| Kinnison | so I think jmac is right | 15:25 |
| jmac | Try it: "hello".lstrip("eh") | 15:26 |
| Kinnison | >>> element="elements/elementsong.bst" | 15:28 |
| Kinnison | >>> element.lstrip("elements/") | 15:28 |
| Kinnison | 'ong.bst' | 15:28 |
| Kinnison | >>> | 15:28 |
| *** toscalix_ has joined #buildstream | 15:28 | |
| tpollard | that's caught me a number of times | 15:30 |
| jmac | Esp. since "elements/elementsong.bst".lstrip("elements") appears to do what you want | 15:35 |
| skullman | blimey, I've gone years with that happening to work as I expected every time | 15:35 |
| Kinnison | Given Python 3 has some special stuff for managing paths, does it have a path-prefix stripper? | 15:38 |
| skullman | not that I can see, though there might be something weird in pathlib | 15:39 |
| jmac | os.path.relpath should do it, I think | 15:40 |
| *** kapil___ has joined #buildstream | 15:44 | |
| Nexus | kk, updated that | 15:45 |
| gitlab-br-bot | jonathanmaw merged MR !924 (jonathan/workspace-fragment-create->master: Support invoking buildstream from a workspace outside a project) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/924 | 16:01 |
| *** toscalix has quit IRC | 16:54 | |
| *** offus has joined #buildstream | 16:59 | |
| *** tiagogomes has quit IRC | 17:13 | |
| *** tpollard has quit IRC | 17:23 | |
| *** nimish has joined #buildstream | 17:29 | |
| *** jonathanmaw has quit IRC | 18:00 | |
| gitlab-br-bot | jjardon closed issue #609 (CAS server doesn't seem to automatically remove artifacts so it gets fill up) on buildstream https://gitlab.com/BuildStream/buildstream/issues/609 | 18:03 |
| jjardon | valentind: thanks a lot for all your work there ^ | 18:03 |
| *** tristan has quit IRC | 18:04 | |
| valentind | Thanks | 18:06 |
| *** raoul has quit IRC | 18:19 | |
| jjardon | valentind: are you ok if I add the "blocker" label to https://gitlab.com/BuildStream/buildstream/issues/806 ? | 18:23 |
| valentind | jjardon, Sure. | 18:25 |
| jennis | ahh, I just ran into this one | 18:26 |
| jennis | changing x86_64 -> x86-64 fixed everything for me... then checking out a commit before !969 breaks with the same error again | 18:28 |
| gitlab-br-bot | MR !969: Execution environment reqs https://gitlab.com/BuildStream/buildstream/merge_requests/969 | 18:28 |
| jennis | looks like enforcing a '- 'rather than a '_' is intentional | 18:30 |
| jennis | as there are a lot of explicit x86_64 -> x86-64 changes in the diff | 18:31 |
| *** kapil___ has quit IRC | 18:33 | |
| *** nimish has quit IRC | 18:39 | |
| *** nimish has joined #buildstream | 18:40 | |
| *** johnward has quit IRC | 18:44 | |
| *** jmac has quit IRC | 18:44 | |
| *** benbrown has quit IRC | 18:44 | |
| *** laurence has quit IRC | 18:44 | |
| *** jmac has joined #buildstream | 18:44 | |
| *** benbrown has joined #buildstream | 18:44 | |
| *** paulsher1ood has quit IRC | 18:44 | |
| *** ikerperez has quit IRC | 18:44 | |
| *** adds68 has quit IRC | 18:45 | |
| *** coldtom has quit IRC | 18:45 | |
| *** jennis has quit IRC | 18:45 | |
| *** paulsherwood has joined #buildstream | 18:45 | |
| *** coldtom has joined #buildstream | 18:47 | |
| *** johnward has joined #buildstream | 18:48 | |
| *** laurence has joined #buildstream | 18:48 | |
| *** nimish has quit IRC | 18:59 | |
| *** nimish has joined #buildstream | 19:00 | |
| *** nimish has quit IRC | 19:10 | |
| *** nimish has joined #buildstream | 19:10 | |
| *** lachlan has quit IRC | 19:12 | |
| *** adds68 has joined #buildstream | 19:12 | |
| *** nimish has quit IRC | 19:25 | |
| *** nimish has joined #buildstream | 19:26 | |
| *** nimish has quit IRC | 19:41 | |
| *** nimish has joined #buildstream | 19:41 | |
| cs-shadow | juergbi: Kinnison: hi, do either of you remember why the ".bst" extension check was added as a warning and not a LoadError? | 19:44 |
| Kinnison | cs-shadow: IIRC it happens outside of the loader too | 19:44 |
| Kinnison | cs-shadow: Erm | 19:44 |
| Kinnison | cs-shadow: Or perhaps it was simply that we were worried we'd break existing projets | 19:45 |
| juergbi | https://mail.gnome.org/archives/buildstream-list/2018-November/msg00067.html | 19:45 |
| juergbi | tristan suggested it in the above mail | 19:45 |
| juergbi | you can make the warning fatal | 19:45 |
| Kinnison | Either way, good luck :-) | 19:46 |
| * Kinnison heads off to the gym | 19:46 | |
| cs-shadow | I see, thanks! | 19:46 |
| cs-shadow | juergbi: I was asking this mostly because I was planning to implement a check for valid element names, and was wondering whether that should an error or a warning | 19:47 |
| cs-shadow | I was thinking that having invalid characters in the filename should probably be an error, but it seemed a bit odd to have the invalid suffix produce warnings, while the invalid chars check produces errors | 19:48 |
| cs-shadow | juergbi: do you have any preference on this? | 19:48 |
| juergbi | I'd keep the two consistent | 19:49 |
| cs-shadow | juergbi: I feel the same, but I don't think that having invalid chars in the name should probably be an error, as it hurts more than just discoverability. Perhaps this is a topic for the ML thread | 19:51 |
| cs-shadow | sorry, that should have been "I _think_ that it should be an error" | 19:52 |
| juergbi | well, except for very few exceptions (such as `:`), these names will typically still work | 19:53 |
| juergbi | on the same platform, that is | 19:53 |
| juergbi | we restrict the subset in order to reduce issues e.g. across platforms | 19:54 |
| juergbi | and with different unicode normalization and things like that | 19:54 |
| juergbi | so I don't think it's that bad if it's only a warning | 19:54 |
| cs-shadow | hmm, I was thinking mostly about `:` and that may produce unexpected behavior | 19:55 |
| juergbi | hm, actually, can we even detect invalid `:`? | 19:56 |
| juergbi | once we support junctions with `:`, we interpret this specially and won't warn/error about it in a dependency list | 19:56 |
| juergbi | (i.e., the error would then be that it would complain that the specified junction doesn't exist) | 19:57 |
| juergbi | so we could only warn about those unsupported characters that actually still mostly work | 19:58 |
| cs-shadow | fair point! I didn't think about how to catch this once we support the junctions syntax | 19:59 |
| cs-shadow | yeah, I guess a warning makes sense in that case | 20:00 |
| cs-shadow | it's a shame though we won't catch the only invalid character that may really cause issues, but oh well :) | 20:00 |
| *** phildawson_ has quit IRC | 20:15 | |
| *** phildawson has joined #buildstream | 20:22 | |
| *** xjuan has joined #buildstream | 20:25 | |
| *** nimish has quit IRC | 20:30 | |
| *** phildawson has quit IRC | 20:34 | |
| *** alatiera has quit IRC | 21:31 | |
| *** juergbi has quit IRC | 21:42 | |
| *** juergbi has joined #buildstream | 21:48 | |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!