| 20 Jan 2025 |
 emily | (I could be wrong about the timing there, but there's lots of examples like this) | 18:33:15 |
| emily | if we want to keep having "an X toolchain" as a monolithic concept then we shouldn't add additional variables that can't vary freely because we assume all the things are grouped together | 18:33:33 |
| emily | if we want to split it up (which I think is a good idea and we should do it), then we shouldn't leave a vestigial notion around that will break actually trying to benefit from the split-up variables | 18:34:00 |
 Tristan Ross | That's kinda why I added a way to check if things are the default values. Then it's easier to see if the toolchain is the standard setup. | 18:34:26 |
| emily | either we keep "toolchain is GNU", "toolchain is LLVM", etc. (which already breaks down for things like Darwin), or we need to actually say what we mean i.e. "C compiler is Clang", "C++ library is libc++", "C++ ABI library is libcxxrt", "unwinder is libunwind", and continuing to have checks for "toolchain is LLVM" is just going to defeat the point | 18:34:43 |
| emily | because trying to actually deviate from the monolithic form will break as checks assume it still means something | 18:35:06 |
| emily | the backwards compatibility issue also needs addressing | 18:35:09 |
| Tristan Ross | What about the backwards compatibility? We already have asserts to make sure people move over to the new form. | 18:35:51 |
| Tristan Ross | I could add a set of attributes which exports as the using form and is based on the defaults. | 18:36:28 |
| Tristan Ross | So if the defaults for the CC, cxxlib, bintools, etc and the toolchain is LLVM then useLLVM could equate to true. | 18:37:11 |
| emily | that's not really acceptable for something this core, people need to be able to write code that works with both versions. you need to warn after the last version that doesn't support the new form is EOL and then convert it to an error a release later. see uses of lib.oldestSupportedReleaseIsAtLeast in the tree | 18:37:25 |
| Tristan Ross | Ok, that's doable. | 18:39:00 |
| Tristan Ross | @emily what about this for a compromise for the toolchain attribute? We stage the attribute set. So the first form is the defaults, 2nd is with user input applied, final is the adjusted form based on the inputs. Then we can handle if the toolchain deviated from the defaults, we can return null. | 18:42:28 |
| Tristan Ross | I don't see any other alternative option until we have the fine grain control and test every case (example is something like doCheck in pkgs/by-name/li/libseccomp/package.nix). | 18:43:35 |
| Tristan Ross | The benefit of staging the platform stuff is then we can get the fine grain control, handle non default cases, and still have things work as they do now. | 18:44:44 |
| emily | that means that there'll be conditionals for toolchain == "llvm" or toolchain == "gnu" that just fail if you try to vary any of the variables that they're shorthand for, because it'll become null despite actually wanting to check e.g. unwinder library. having a staging system that separates inputs and outputs so that toolchain = "llvm"; could be used as a shorthand but is never present in the final structure would be okay, but seems like a more major rework and probably not worth it just to save ~5 lines when you define a new platform | 18:45:29 |
| emily | we just have to investigate the existing ambiguous uses and figure out what they actually care about, whether that's through pinging maintainers, experimenting, or diving into the relevant source code. it's not necessarily easy, but it's what's required to actually make it a cleanup and deconfusing rather than just adding more non-orthogonal variables that won't actually work properly, which will make a bigger mess | 18:46:06 |
| emily | e.g. do libseccomp tests run successfully with Clang + libstdc++? with Clang + libc++? is it the unwinder? what errors happen when we try? | 18:46:40 |
| emily | having difficult-to-figure-out conditionals like this where it's not clear what they're checking for is the result of having overly-broad flags like the toolchain your PR adds | 18:47:11 |
| emily | moving away from that is good – especially since Darwin already shows that useLLVM is kinda meaningless – but we have to actually move away from it, not just spread around the confusion a bit more | 18:47:32 |
| emily | looking at the log you linked in that libseccomp PR, some of those failures seem pretty scary, there's cases where things trying to ensure that seccomp denies an action incorrectly result in ALLOW results… | 18:49:29 |
| emily | so I don't think glossing over that is the right solution, those could be legitimate security vulnerabilities in pkgsLLVM | 18:50:05 |
| emily | coordinating with upstream seems like the best option there, since it seems like a pretty gnarly issue. I'm not sure we should have turned off tests in the first place at all without investigating the cause | 18:50:42 |
| emily | but even if we just want to keep disabling them and ignoring the potential issues, the nice thing about splitting up the variables is that it should make it easier to test various combinations of them to see which ones actually cause the issue | 18:51:13 |
| Tristan Ross | Ok, what about only accepting toolchain for user input and not relying upon it inside nixpkgs unless it's for a default value? | 18:51:45 |
| Tristan Ross | In reply to @emilazy:matrix.org
so I don't think glossing over that is the right solution, those could be legitimate security vulnerabilities in pkgsLLVM Yeah, I can understand that. Just don't have the bandwidth to investigate that. I believe there's an upstream issue already. I eventually want all tests running. | 18:52:35 |
| emily | I'm not sure what you mean by default value, but I think that allowing it for input only would be good in principle but that we have no way to ensure people don't actually condition on it without automated linting. if it's that important to avoid a handful of lines of boilerplate for defining platform toolchains (I think that given that we already have shorthand for pkgsLLVM it's not a big deal and it's good to be explicit), then we should ensure it doesn't appear in the elaborated structure, but I'm not sure what that would break since as sterni said on the issue the idea of elaboration just adding more stuff is pretty baked in | 18:54:33 |
| Tristan Ross | Now that I think about it, not using toolchain would mean we'd need a different attribute to rely upon checking whether to use the LLVM bootstrapping or GCC bootstrapping. | 18:58:18 |
| emily | it should hopefully be possible to bootstrap any combination of the variables that actually works? | 18:59:43 |
| emily | or if you mean from a choice of predefined bootstrap tools to start the chain with, then I guess that would have to be a separate variable, yeah (it could just point directly to an attribute of the right format for flexibility?) | 18:59:52 |
