ldc icon indicating copy to clipboard operation
ldc copied to clipboard

Support ldc2.conf as a directory

Open the-horo opened this issue 7 months ago • 15 comments

Short summary of changes:

  1. ldc2.conf can now be a directory
  2. cmake files have been changed to take advantage of this during the build => easier to configure independent sections of settings
  3. Support installing the config files as a directory
  4. Split the settings across compiler and runtime config files Compiler contains the -I includes, wasm flags and compiler-rt libdir Runtime contains phobos&druntime libdir and rpath => The runtime and the compiler can now be installed as separate projects without their config files colliding/being skipped
  5. Change the runtime config files for the native target to also include the triple like for multilib (e.g. x86_64-.*-linux-gnu instead of "default"). => ldc-build-runtime can now install cross-builds alongside their configuration files (it skipped the config previously to avoid overwriting the host one), removing the need for post-install interaction from the user.

the-horo avatar Jun 16 '25 10:06 the-horo

The current CI errors suggest an incomplete config for the built (non-installed) compiler. Let me know if you need help.

kinke avatar Jun 16 '25 16:06 kinke

The current CI errors suggest an incomplete config for the built (non-installed) compiler. Let me know if you need help.

Yup. I didn't think of testing that so I guess I did forget about something... It did work when installing so surely it can't be that broken.

the-horo avatar Jun 16 '25 16:06 the-horo

FAILED: etc/ldc2_install.conf D:/a/ldc/bootstrap-ldc/etc/ldc2_install.conf 
C:\Windows\system32\cmd.exe /C "cd /D D:\a\ldc\bootstrap-ldc\etc\ldc2_install.conf.d && sh -c "output=\"${1}\"; shift; cat \"${@}\" > \"${output}\"" script D:/a/ldc/bootstrap-ldc/etc/ldc2_install.conf 30-ldc-runtime-lib.conf 35-ldc-compiler.conf 40-ldc-wasm.conf"
cat: 30-ldc-runtime-lib.conf: No such file or directory

Are those cmake quotes interfering with my command's quotes or is there some other reason it's failing?

the-horo avatar Jun 16 '25 18:06 the-horo

Are those cmake quotes interfering with my command's quotes or is there some other reason it's failing?

Not sure, I'd need to check locally. In general, using the sh and cat tools isn't ideal on Windows, as those are usually NOT in PATH (but bundled with e.g. git).

Edit: I guess the error is earlier, when trying to invoke a .sh script to finalize the 30-ldc-runtime-lib.conf.in to .conf IIUC. AFAIK, that requires an explicit bash dir/file.sh … invocation on Windows, at least with a default cmd.exe shell.

kinke avatar Jun 16 '25 18:06 kinke

FYI: instead of many separate small test files, you can use split-file. See: https://llvm.org/docs/TestingGuide.html#extra-files That would make it much more clear what is being tested (no need to open 4 different files to see the full test case).

JohanEngelen avatar Jun 16 '25 18:06 JohanEngelen

FYI: instead of many separate small test files, you can use split-file. See: https://llvm.org/docs/TestingGuide.html#extra-files That would make it much more clear what is being tested (no need to open 4 different files to see the full test case).

Ohh, that looks useful

the-horo avatar Jun 16 '25 18:06 the-horo

Edit: I guess the error is earlier, when trying to invoke a .sh script to finalize the 30-ldc-runtime-lib.conf.in to .conf IIUC. AFAIK, that requires an explicit bash dir/file.sh … invocation on Windows, at least with a default cmd.exe shell.

You're right:

[148/212] Generating etc/ldc2_install.conf
 Volume in drive D is Temporary Storage
 Volume Serial Number is 0C26-1B92
 Directory of D:\a\ldc\bootstrap-ldc\etc\ldc2_install.conf.d
06/16/2025  06:47 PM    <DIR>          .
06/16/2025  06:47 PM    <DIR>          ..
06/16/2025  06:47 PM               279 30-ldc-runtime-lib.conf.in
06/16/2025  06:47 PM               180 35-ldc-compiler.conf
06/16/2025  06:47 PM               237 40-ldc-wasm.conf
               3 File(s)            696 bytes
               2 Dir(s)  153,611,042,816 bytes free

It seems that cmake is content to let the fill-multilib-section POST_BUILD command silently fail on windows

the-horo avatar Jun 16 '25 18:06 the-horo

Ok, one test failure for tomorrow me to go through. Other things that I thought of that could be useful:

  1. Add back the help text in the config files as now they're just settings

  2. Hook up the android cross-builds to use CONF_PREFER_DIR

  3. Not sure, I'd need to check locally. In general, using the sh and cat tools isn't ideal on Windows, as those are usually NOT in PATH (but bundled with e.g. git).

    I think I change the post build commands to depend only on cmake but we're still left with fill-multilib-section.sh which uses bash, grep and sed. I know that some of the druntime integration tests use the same commands so maybe it's fine as a dependency?

  4. Maybe it's worth adding a -dumpmachine switch like clang has that only prints the target triple to ease the processing that is carried out by the .sh script.

the-horo avatar Jun 16 '25 20:06 the-horo

  1. Add back the help text in the config files as now they're just settings

Ah yeah that'd be good.

I think I change the post build commands to depend only on cmake but we're still left with fill-multilib-section.sh which uses bash, grep and sed. I know that some of the druntime integration tests use the same commands so maybe it's fine as a dependency?

Hmm; I think the subset of people running tests is way smaller than those just building. And for those druntime integration tests, we set PATH using a hardcoded assumption: https://github.com/ldc-developers/ldc/blob/3acd7617fcfee90470adfeb99da8b9f06a67ca01/dmd/osmodel.mak#L60-L66

  1. Maybe it's worth adding a -dumpmachine switch like clang has that only prints the target triple to ease the processing that is carried out by the .sh script.

I still need to check your rationale for switching to explicit triples for the default target. The main problem I saw back then is that there's no clear 1:1 mapping to the normalized triple (the one with the vendor) - e.g. the vendor seems uninteresting for all but Apple targets (and can be anything from empty to none and unknown...), a trailing OS version suffix should be masked out, similarly a subarch (aarch64v9a), ... So converting the default normalized triple to a suitable pattern seems hard.

Edit: Another example: the (libc) environment. IIRC, one can use the prebuilt Alpine package (targeting x86_64-linux-musl by default and bundling those libs) with -mtriple=x86_64-linux-gnu to 'cross'-compile+link to glibc on a glibc host, linking static druntime and Phobos prebuilt for musl. At least for a simple test that I did. So IMO the least surprising behavior is to fall back to the default target libs for all triples unless the config has an according overriding section.

kinke avatar Jun 16 '25 20:06 kinke

Edit: Another example: the (libc) environment. IIRC, one can use the prebuilt Alpine package (targeting x86_64-linux-musl by default and bundling those libs) with -mtriple=x86_64-linux-gnu to 'cross'-compile+link to glibc on a glibc host, linking static druntime and Phobos prebuilt for musl. At least for a simple test that I did.

I don't agree with this point because:

  1. statically linking against musl does not solve every ABI compatibility your programs/libraries may have
  2. The stdlibs are not even linked statically, what you're describing is substituting the libc undefined symbols in the druntime&phobos of the alpine build with the hopefully existent functions from the glibc you're building against, and hoping that they have the same arguments (otherwise you get UB) and have the same semantics (otherwise you get logic errors).

I couldn't find an example of functions having different signatures but for different behavior just pick something from https://wiki.musl-libc.org/functional-differences-from-glibc.html. Here's an example with basename:

// b.c
#include <libgen.h>
#include <string.h>

int always_true(void) {
	char arr[] = "/lib/foo/";
	char *result = basename(arr);
	return strcmp(result, "foo") == 0;
}
// a.c
#include <unistd.h>
#include <stdio.h>

extern int always_true(void);

int main (int argc, char **argv) {
	printf("1 == %d\n", always_true());
}

If you compile both of those files on either musl or glibc you get the correct result:

musl /tmp # gcc -o a a.c b.c && ./a
1 == 1
glibc /tmp # gcc -o a a.c b.c && ./a
1 == 1

If you compile b.c on musl and try to link it on glibc you get the wrong results:

glibc /tmp # gcc -o a a.c b-musl.o && ./a
1 == 0

With the comment that this behavior in not specified so it can change at anytime (and even break).

For this reason, at least when it comes to the cenv part of the triple, it is not safe to normalize it away.

I have to think about the version components of the OS and ARCH fields and see if I can draw a conclusion, otherwise

So IMO the least surprising behavior is to fall back to the default target libs for all triples unless the config has an according overriding section.

I'll go with your suggestion here. It's not that important to encode the triple, it only makes the code a little shorter and marginally helps ldc-build-runtime as the cross-config file can be installed automatically. If the target triple were to stay default then ldc-build-runtime would just need to replace the string accordingly so no biggie.

the-horo avatar Jun 17 '25 10:06 the-horo

Yeah I didn't mean to encourage anything like that, just saying that some guys might already use such stuff, regardless of whether by accident or knowing what they are doing. The env can be versioned too - androideabi29 (32-bit), android30 (64-bit)… it's all very fuzzy.

kinke avatar Jun 17 '25 12:06 kinke

Ok, I added back the documentation (though perhaps a little bit too much for a config file) and made the cat portable on windows. Still unsure what to do with the default triple situation, it sounds most reasonable to skip the last commit and keep "default".

the-horo avatar Jun 17 '25 16:06 the-horo

Changed the documentation file to only be installed with the compiler, not with every runtime build.

the-horo avatar Jun 18 '25 08:06 the-horo

Tests passed but I'm looking over the artifacts and checking that the config files aren't broken. The android one seems to be so I'll have to fix it

the-horo avatar Jun 18 '25 13:06 the-horo

The android config now looks fine and I've changed it to be a directory in the release artifacts. The test failure is a timeout

the-horo avatar Jun 18 '25 15:06 the-horo

Note that I'd be okay with generating the ldc2.conf directory exclusively now via CMake, so no more support for the single file (as part of the CMake build - but still supported by the compiler). There are breaking changes already, such as the now unused *.conf.in files and dropped ADDITIONAL_DEFAULT_LDC_SWITCHES, so I don't see why we couldn't switch to the directory too.

Not sure how much that would simplify. I can take care of the CI packages, would want to e.g. redo the macOS package configs anyway, now somewhat easier by adding separate files instead of appending multiple sections.

kinke avatar Jun 21 '25 10:06 kinke

Note that I'd be okay with generating the ldc2.conf directory exclusively now via CMake, so no more support for the single file (as part of the CMake build - but still supported by the compiler).

As in remove the CONF_PREFER_DIR=OFF code?

the-horo avatar Jun 21 '25 10:06 the-horo

As in remove the CONF_PREFER_DIR=OFF code?

Exactly. Again, not sure how much that would simplify the code; you already found a way for the portable concat of all files to a single one. But I suspect it'd be worth it to reduce CMake complexity. In case a package maintainer prefers the single file, he/she can presumably insert the cat one-liner somewhere in the build process.

kinke avatar Jun 21 '25 11:06 kinke

I can take care of the CI packages, would want to e.g. redo the macOS package configs anyway, now somewhat easier by adding separate files instead of appending multiple sections.

I'll prepare a PR where I'm switching to CONF_PREFER_DIR=ON by default (so incl. CI builds), and adapt the CI packaging accordingly. So that CI won't be a blocker to remove CONF_PREFER_DIR altogether. And we should e.g. immediately see simplifications for the existing configs-merging of the windows-multilib package.

kinke avatar Jun 21 '25 12:06 kinke

I'll prepare a PR where I'm switching to CONF_PREFER_DIR=ON by default (so incl. CI builds), and adapt the CI packaging accordingly. So that CI won't be a blocker to remove CONF_PREFER_DIR altogether. And we should e.g. immediately see simplifications for the existing configs-merging of the windows-multilib package.

I've tried to guess-adapt the windows and macos merge actions, let's see what fails

the-horo avatar Jun 21 '25 12:06 the-horo

If everything works, possible followups include:

  1. Creating different configs for wasn and naked-wasm
  2. Adding a --configTriple option to ldc-build-runtime (and either require the user to pass the triple or the d code can pick it up from ldc2 -v) in order to automate the config installation
  3. Removing MULTILIB and using ldc-build-runtime in its place
  4. Adding a -m (or a better named) flag to ldc-build-runtime that's an alias for --cflags=-m<> --ldflags=-m<> --installWithSuffix=<>
  5. Fix LDC_DYNAMIC_COMPILE being disabled on solely-runtime builds
  6. Fix the compiler testsuite depending on variables declared in the runtime CMakeLists.txt (I only got failures about two things)

the-horo avatar Jun 21 '25 14:06 the-horo

Okay, this was my final review pass now, just a few nits remaining. I wanna follow-up on https://github.com/ldc-developers/ldc/pull/4954#discussion_r2160065476 after this, shuffling around the order and filenames a bit.

kinke avatar Aug 25 '25 17:08 kinke

Alright, thx a lot for the hard work, cool stuff! :+1:

kinke avatar Aug 26 '25 19:08 kinke