| 20 Jan 2025 |
 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 |
