ldc2.conf dir: Slightly shuffle order and file names #4978
Conversation
kinke
force-pushed
the
conf_shuffle
branch
2 times, most recently
from
2b836ed to
144e6b7
Compare
| @@ -304,7 +304,7 @@ if(MULTILIB AND NOT "${TARGET_SYSTEM}" MATCHES "APPLE") | |||
| set(install_rpath "${install_libdir}") | |||
| endif() | |||
|
|
|||
| set(name "31-ldc-runtime-lib${MULTILIB_SUFFIX}") | |||
| set(name "55-target-m${MULTILIB_SUFFIX}") | |||
There was a problem hiding this comment.
I think that keeping the config files (here and in CI) be predictably named would help in the future with ldc-build-runtime. I think that the current CI setup should be an approximation of what we would want ldc-build-runtime to be able to do. Since CI currently does ldc-build-runtime --installWithSuffix=-ios-arm64 I think we should name the config files 55-target-ios-arm64 as that would make a possible future transition smoother. Maybe it's worth using the target-lib form as a runtime built with LIB_SUFFIX="" would correspond to a config file named 55-target which looks weird.
Somewhat unrelated to this but we should also consider config switches that don't really have something to do with the runtime build, but are still necessary. Like the -Xcc=${sysroot} on macos and a supposed -gcc=aarch64-unknown-linux-gnu on linux. I think it would be best to put these settings in a separate file (maybe 55-target-ios-arm64-env) in order to allow ldc-build-runtime to freely install the D portion of the config file, which we should be able to fully generate.
The ideal setup I picture would then be:
- the official releases come with
55-target-ios-arm64and55-target-ios-arm64-env. The users can modify-ios-arm64-envhowever they wish, in case somebody has broken their setup or they are using an older compiler - during CI, we would call
ldc-build-runtime --installWithSuffix=-ios-arm64and that would build and install the libraries, as well as install55-target-ios-arm64. We would then manually write55-target-ios-arm64-envwith the same settings that we currently add now - if a user wanted to build another copy of the runtime, they would follow the same steps as CI, i.e.
ldc-build-runtime --installWithSuffix=<target>and, if they need to, write a55-target-<target>-envfile, preferably following documentation that we make available.
There was a problem hiding this comment.
I think that keeping the config files (here and in CI) be predictably named would help in the future with ldc-build-runtime
For this specific line, i.e., the multilib extra .conf, I went with 55-target-m32.conf (derived from -m32), as I guess we don't have enough info at CMake time, and I also didn't want the fill-multilib-triple.sh script to also come up with a suited logic to additionally rename the file based on the normalized triple.
Yeah wrt. ldc-build-runtime, I haven't thought that through yet. So far, one would have to rename the generated 50-target-default.conf to some 55-….conf, and potentially add the extra switches that were presumably already passed in --dflags (with the exception of -mtriple, we'd have to filter that one out). An optional CMake var (set externally) for the config base name would make things a bit easier, and that would be the installWithSuffix option of that tool then. Edit: Hmm or we just do the rename as part of ldc-build-runtime --installWithSuffix, leaving CMake alone.
There's one scenario currently where we don't have a solution: the 50 default target only comes with static or shared druntime/Phobos, and so appends a -link-default-shared[=false] switch. Now a 55 override target comes with both variants - but it cannot re-enable both sets without completely overriding all switches, as it's a boolean switch, no way to undefine it...
There was a problem hiding this comment.
and potentially add the extra switches that were presumably already passed in
--dflags
Okay no that definitely doesn't cut it, neither additionally using the --cflags, after a look at the CI usages for cross-compiling to iOS and Android - both are mainly configured via CMake, either by specifying CMAKE_OSX_SYSROOT etc. or even a CMAKE_TOOLCHAIN_FILE on Android. Plus we might have e.g. dflags that should only apply to the runtime builds, not all user builds later on too.
So yeah I guess adding the cross-switches remains a manual step. I don't like a pair of generated + manual .env files per such target though, and prefer in-place editing that file then, so that we'd normally have a single 55-target-… file per distinct cross-compilation target.
#4974 is already going to be a slight challenge - we'd have two iOS arm64 targets for the universal package, a native iOS one and the iOS-simulator one. And as it's apparently just a -simulator environment addition to the triple, we have a ordering problem which should otherwise not be the case in the 55 group - the simulator one needs to come after the normal iOS one, and it inherits the normal section. Hmm, maybe we should try to make the normal iOS one stricter, with something like arm64-apple-ios[^-]*$, so that it doesn't apply to the -simulator one...
There was a problem hiding this comment.
Oh and wrt. the generated 55 .conf file - one major problem is figuring out a matching triple regex, which will almost certainly have to come from the user.
There was a problem hiding this comment.
I went with 55-target-m32.conf (derived from -m32), as I guess we don't have enough info at CMake time
I don't like this. It is inconsistent with how the default built gets it's name (derived from LIB_SUFFIX), I think using MULTILIB_SUFFIX here would be more appropriate. In fact, I would rather have ldc-build-runtime be able to build and install the -m32 libraries and nuke all the multilib logic out of cmake.
and potentially add the extra switches that were presumably already passed in --dflags (with the exception of -mtriple, we'd have to filter that one out)
No, build flags should not end up in the installed image.
And as it's apparently just a -simulator environment addition to the triple, we have a ordering problem which should otherwise not be the case in the 55 group - the simulator one needs to come after the normal iOS one, and it inherits the normal section. Hmm, maybe we should try to make the normal iOS one stricter [...]
If we want to put everything in a 55 group then those triples should be disjoint. Yes, making one stricter is the only sane solution I can think of.
There's one scenario currently where we don't have a solution: the 50 default target only comes with static or shared druntime/Phobos, and so appends a -link-default-shared[=false] switch.
We can just add -link-defaultlib-shared=false in a BUILD_SHARED_LIBS=BOTH build. This is the current behavior and a user can always overwrite it on the command line with -link-defaultlib-shared.
Oh and wrt. the generated 55 .conf file - one major problem is figuring out a matching triple regex, which will almost certainly have to come from the user.
How about this: ldc-build-runtime -installWithSuffix=foo -configTriple=my-foo-target <build_flags> Would install a 55-target-foo config that contains:
my-foo-target:
{
lib-dirs = [ "/install/libfoo" ]
rpath = "/insta/foo"
}
This should work with any build system that properly configures a C compiler and sets CC to it, regardless of any switches that would be needed by the foo target. Internally, the cmake build would have a CONFIG_TRIPLE option and would change the filename for the configuration file to 55 instead of 50, and change the "default" triple to my-foo-target. ldc-build-runtime then just needs to ninja install and that would be all.
What would be missing is the out-of-the-box support for the target. Adding it would require to find the -Xcc switches for the target and embed them in the config file, which starts to sound like we're trying to implement a mini build system in the config files, one that just hardcodes what CC and args should be. I don't think this would be a bad idea straight out, so long as we keep the switches to a small list and the user can overwrite them on the command line, in case we miss a more exotic setup or in case some external entity decides that it would be a good idea to break previous setups.
What I want to highlight is that those two config files have entirely different jobs. There's the target-foo for the druntime & phobos side, with a suitable libdir and possible switches, that should basically always be correct, users shouldn't need changing those. But there's also the target-foo for the: you're trying to cross compile a program, outside of a build system which (should) have the appropriate settings for configuring a C compiler (and maybe even a D one), said C compiler being a dependency for linking your binary, so we will try to pick some defaults that should match what you probably want to happen.
I don't like a pair of generated + manual .env files per such target though, and prefer in-place editing that file then, so that we'd normally have a single 55-target-… file per distinct cross-compilation target.
For the reasons above, and the fact that implementing a diff analysis to figure out what can be changed in the config file (think of a build that didn't have any switches followed by a build for the same target that now contains switches) sounds way more complicated than splitting the parts that solely depend on how the runtime was built, that can be easily picked up in the cmake files, from the portions that have to do with how a user uses the built runtime; I think it would be better to have separately named files, though maybe not so close in name (maybe a 70-env-ios-simulator or something). I also want ldc-build-runtime to keep being a thin layout around cmake so I don't want it to be required to support manual installation or moving of files or having to edit their contents too much, I would prefer the cmake build to be able to do as much as it's needed for a minimal setup.
There was a problem hiding this comment.
What would be missing is the out-of-the-box support for the target. Adding it would require to find the -Xcc switches for the target and embed them in the config file, which starts to sound like we're trying to implement a mini build system in the config files, one that just hardcodes what CC and args should be. I don't think this would be a bad idea straight out, so long as we keep the switches to a small list and the user can overwrite them on the command line, in case we miss a more exotic setup or in case some external entity decides that it would be a good idea to break previous setups.
We're not just trying, this is the way the official packages have worked for years, and what people are accustomed to. No need for a build system to prepare anything.
For cross-linking to Windows, no extra switches are required at all, regardless of the host platform (thanks to -link-internally being the default on non-Windows hosts, and the bundled MinGW-based import libs, so that no official WinSDK + MSVCRT libs are required). Sure, importC isn't handled out of the box.
For cross-linking to other Apple targets on a Mac, we need to pass the according Xcode sysroot alone, that's 2 -Xcc switches. Probably works for importC too.
For cross-linking to non-Apple Posix targets (incl. Android), you usually need an according C cross-compiler, and so a simple -gcc switch in the LDC config. Should work for importC too.
There was a problem hiding this comment.
There's one scenario currently where we don't have a solution: the 50 default target only comes with static or shared druntime/Phobos, and so appends a -link-default-shared[=false] switch.
We can just add -link-defaultlib-shared=false in a BUILD_SHARED_LIBS=BOTH build. This is the current behavior and a user can always overwrite it on the command line with -link-defaultlib-shared.
Right, good point.
There was a problem hiding this comment.
I don't think this would be a bad idea straight out, so long as we keep the switches to a small list and the user can overwrite them on the command line
The very few expected switches should be overrideable (by the user or a build system); -Xcc switches depending on whether the C compiler allows them to be overridden. E.g., Apple clang's -isysroot seems overrideable just fine, that's currently used in #4974 (with 2 matching iOS sections for the simulator target, and so 2 -isysroot).
The only real problem OTOH would be a Edit: Nope - an empty -gcc in the config, to be overridden by a CC env var set up by the external build system (or user) - the build system would have to set -gcc explicitly to that CC (as the explicit cmdline option takes precedence over the env var), and in case it'd be more than just the path to that C compiler (so including space-separated flags that you've added support for), it'd have to pass those separately as -Xcc.-gcc= activates the CC env var, so this is also doable easily.
There was a problem hiding this comment.
Maybe take a 2nd look at the diff? You had 31-ldc-runtime-lib${MULTILIB_SUFFIX}, I changed it to 55-target-m${MULTILIB_SUFFIX}.
I meant this as in keeping the multilib name exactly like one that would be produced by a cross-build, so 55-target-32 rather than 55-target-m32. It's not that big of a deal to use different names though, I just think it would be nice to decide on how the config files would be named and keep the scheme, regardless of how the the runtime build and the config generation happens. I won't insist if you don't care about it.
We're not just trying, this is the way the official packages have worked for years, and what people are accustomed to. No need for a build system to prepare anything.
Sorry, I didn't mean my point there to be read as asking whether such support should be implemented because, like you said, the current setup has been working like this on macos and it cannot be broken. Note that on other systems we still have the freedom to choose whether to configure CC in the config file, I'm still trying to go through the arguments to see if one setup would be more appropriate...
There was a problem hiding this comment.
I meant this as in keeping the multilib name exactly like one that would be produced by a cross-build, so
55-target-32rather than55-target-m32.
Well, the scheme I'd suggest would be 55-target-linux-x86 (or 55-target-freebsd-x86 etc.). But yeah I don't really care about this multilib config name, since the -m32 support is IMO just legacy - x86 hardly needed nowadays on Posix, plus I think more recent archs don't support that switch anymore (e.g., no -m32 for AArch64 compilers - aarch64-unknown-linux-gnu-g++: error: unrecognized command-line option '-m32' on godbolt).
|
I'm just playing around here with the 2nd commit for now, the |
kinke
force-pushed
the
conf_shuffle
branch
2 times, most recently
from
4b096c2 to
040a99d
Compare
kinke
force-pushed
the
conf_shuffle
branch
4 times, most recently
from
28da18a to
e1e0319
Compare
Either the default `50-target-default.conf` (overwriting the existing one) if the suffix is empty, or a `55-target<suffix>.conf` one, which still needs the triple regex patched and potentially some extra switches for cross-compilation/linking.
|
Rough sketch for a future Tasks for a
Would need a mapping for each supported target, such as: windows-arm64:
artifact_suffix: "windows-multilib.7z"
artifact_lib_dir: "lib-windows-arm64" # would be "lib64" for the windows-x86_64 target
artifact_conf_file: "55-target-windows-arm64.conf" # would be "50-target-default.conf" for the windows-x86_64 target
conf_triple_regex: "(aarch|arm)64-.*-windows-msvc" # only needed for 50-target-default.conf's `default`, so trivial to patchThen all a user would additionally need is the according C cross-compiler for non-Apple Posix targets ( |
That would work with This is another point that I want to make. When cross-compiling, you're building things for another system. The issue here is that you can build for multiple other systems, and those systems may require different configs. Maybe on one system you need to use Another, albeit less Should this be something that
There's also an important precedent here. The config file can, technically, contain anything, but people will assume that only some flags may be present. If we make the decision to embed Now, I think I also understand your points. Those being that most of my above concerns are very niche edge cases, and, so long as it is technically possible to support them (like by passing However https://wiki.dlang.org/Cross-compiling_with_LDC does document some of the things I'm arguing against. Anyway, I haven't drawn a conclusion yet, I'm still trying to understand the consequences of each choice. |
|
I've asked around a meson dev what he think of the changes and the short summary is: it would be great if this sort of CC detection didn't overwrite the |
|
Yeah in essence I think the only controversial point here is the And my stance there wrt. So similarly, when invoking LDC package maintainers for distros such as you are free to do what they want, tweaking the config files to their system, and maybe even patching the The only problematic aspect of defaulting to a |
IIRC, Edit: But you could patch |
I wouldn't want that because, in my case, I mostly care about cross runtime builds in the sense that they would be installed on a cross-system. When they are initially built, the files do end up in a I do use this form of cross builds and I would like the cmake build to able to create an exact copy of what a native build on the target system would have produced, and whatever
I very much dislike the idea of making diff --git i/driver/tool.cpp w/driver/tool.cpp
index 6e25c4ce8c..929364a600 100644
--- i/driver/tool.cpp
+++ w/driver/tool.cpp
@@ -86,7 +86,12 @@ std::string getGcc(std::vector<std::string> &additional_args,
// Posix: in case $CC contains spaces split it into a command and arguments
std::string cc = env::get("CC");
if (cc.empty())
+ {
+ auto crossGcc = global.params.targetTriple->getTriple() + "-gcc";
+ if (llvm::sys::findProgramByName(crossGcc))
+ return getProgram(crossGcc.c_str(), &gcc);
return getProgram(fallback, &gcc);
+ }
// $CC is set so fallback doesn't matter anymore.
if (cc.find(' ') == cc.npos) |
Okay, making the compiler choose a somewhat sane default cross-CC when explicitly providing an |
Alright then, how do we want the list of lookups to be like?
clang here doesn't come with a The other OSes I don't know about. |
|
Thx for checking. - Oof, so Gentoo does include the nonsensical vendor. I'd go with NOT tampering with the vendor then, so that it works on Gentoo when using And wrt. clang vs. gcc, I'd only use clang where we know it's the only or default supported compiler. I.e., Android for recent NDKs, and maybe FreeBSD if it comes with these wrappers; no idea where else clang is the default by now (=> hope for PRs from users if they care about it). |
The proposal is to have:
00-docs.conf30-compiler.conf: 'compiler-specific defaults'-Ipost-switch(es) (=> min priority as post-switches) for the bundled druntime/Phobos imports.-func-specialization-size-threshold=1000000000for LLVM 16.-defaultlib=phobos2-ldc,druntime-ldc.lib-dirsand accordingrpath, as well as potential extra switches (for cross-compilation and/or to default to-link-defaultlib-shared[=false]if only one shared/static version of the libs is available). So by far the most interesting group.50-target-default.conf: default target settings, applying to all targets by default (as safe fallback for all the weird triples one might use for the actual default/native target)55-target-…: cross-compilation targets, all overriding thelib-dirs(andrpathif supported by the target) inherited from the default target, and potentially appending extra switches required for cross-compilation/linking (-Xcc,-gcc,-defaultlibetc.).70-compiler-rt.confto append alib-dir(in most cases to a directory provided by the system-default LLVM, which might support a few targets). Anylib-dirin the target-specific 5X group takes precedence.