Refactor code and implement more version formats for dmd and ldc and support gdc

[the-horo]

A pretty big change to the code base with some fixes + enhancements.

Fixes:

• dmd-2.098.0-beta.2 doesn't work (wrong URL) #50
• Extend regex to be more user friendly when matching compilers #40

Also unreported but dub: latest is failing due to a separate reason than #64. It has been fixed as well.

CI has been modified to test all of the new ways a compiler can be specified and macos-latest has been downgraded to macos-13 due to aarch64 incompatibility of dmd and dub.

The ways a compiler can be specified are described in the code but, as a summary:

• dmd|dmd-beta|dmd-latest|ldc|ldc-beta|ldc-latest
• dmd-2.108.0|dmd-2.108.0-beta.1
• ldc-1.35.0|ldc-1.36.0-beta1
• dmd-2.108|ldc-1.34
• dmd^3|ldc^3
• dmd-master|ldc-master

The point of these changes is to make extending the action simpler and I have planned to implement basic support for gdc (only on linux and using apt, like #35 (comment) asked) but I want to put this out here to get it reviewed in the meantime.

Note that some of the macos jobs are failing due to https://issues.dlang.org/show_bug.cgi?id=24137. It has been fixed but it requires >=dmd-2.107.1

[the-horo]

I've implemented the support for gdc and gdmd.

The commits are currently kinda squished with a lot of fixes + enhancements intertwined, perhaps I should try to space them out more.

A summary of changes in the second commit:

• support gdc(-version)? and gdmd(-version)? for compiler. Only on linux, fixes Any idea how to integrate GDC? #35
• go back to dub latest in CI
• and another step in the dub testing job that uses that dub instead of the one that came with the compiler to build and run the hello.d file.
• the action takes in another optional argument, gdmd_sha. It is meant to be used to replace the gdmd version in the ubunty repos with something more up-to-date taken straight from https://github.com/D-Programming-GDC/gdmd. I wanted to implement this because there has been a recent commit that fixes -vcolumns which is quite relevant because dub adds that flag by default. I think that having every compiler be usable with dub is quite important and I didn't find another solution outside of letting users specify their needed version.
  You will notice that dub is still not happy with gdmd even though the version used corresponds to master. This is because of another bug related to the processing of @file arguments to read command line options from a file on disk.

[WebFreak001]

awesome, thanks for taking this on and giving us a much needed refactoring! As for the failing CI tests: it kinda looks like gdmd is missing the application arguments when it's run by dub. Maybe try running with dub -v to see what it's calling and try to manually call the command to see how it behaves and if it's flags that are breaking it or if it's missing the arguments altogether.

Do you need anything else?

[WebFreak001]

I'd also suggest keeping osx on latest, and adding dmd + osx to the exclude section in the actions matrix. With include you can add the earlier versions in as needed

[the-horo]

As for the failing CI tests: it kinda looks like gdmd is missing the application arguments when it's run by dub

Yes, I know what's wrong. gdmd only picks the first line of the @file, that's why arguments start going missing. I can make a PR that fixes it sometime today.

I'd also suggest keeping osx on latest, and adding dmd + osx to the exclude section in the actions matrix. With include you can add the earlier versions in as needed

I've went ahead and added different jobs for each compiler in order not to have to add some many -include and -exclude to the matrix. Check if all the platforms you wanted are tested.

Do you need anything else?

There are 2 unexpectedly failing jobs. I think I broke something so I will have to fix it. Other than this I only want to update the README and I think that should be all.

[WebFreak001]

if you specify only few things on the exclude/include matrix stuff, it should auto-generate the rest of the unspecified variables iirc

[the-horo]

That's right, I didn't think of that. Yet I still don't think it's applicable. To use exclude it would be needed to say «exclude the configurations that have the os field set to macos-latest and the dc field match a pattern» which I don't see how one would do.

If I'm missing something obvious feel free to point it out explicitly but I don't see how, in the original code, one would be able to generate a matrix that runs on macos-13 all the dmd configurations and on macos-latest and macos-13 all the ldc configurations, without specifying, one by one, all the ldc configurations to be included on macos-latest or all the dmd configurations to be excluded from macos-latest.

[the-horo]

I've made all the changes that I wanted and, hopefully, I didn't miss any mistakes/typoes/bad rebases.

[WebFreak001]

did you make the gdmd fix PR yet? we can use it to avoid the CI failures since I saw you added an option to specify which gdmd commit to pick

[the-horo]

did you make the gdmd fix PR yet? we can use it to avoid the CI failures since I saw you added an option to specify which gdmd commit to pick

Yes, I should have linked it here: D-Programming-GDC/gdmd#16. I don't think we'll be able to use it until it's merged. I've implemented being able to pick a commit from the gdmd repo but I don't think PRs qualify, i.e. the commit with the fix belongs to my fork, not the upstream repo.

[the-horo]

I've had to fix an issue in which I was using "" instead of `` in a string but CI is all good now

[kinke]

Has this changed the DC environment variable to an absolute path, whereas it used to be a filename only previously? This appears to have broken dub CI.

[kinke]

This contradicts line 88:

The action will automatically add the D binaries to the PATH environment variable and set the DC environment variable to the selected compiler executable name.

[the-horo]

I didn't read it carefully enough, sorry. Do you want to have the old behavior kept or is it acceptable to keep it absolute so long as the change is only available in v2?

[WebFreak001]

the new behavior has some advantages too, e.g. it can always be called no problem in a shell/cmd call. Since the names were previously just what was basically put in as input value and we now have much more dynamic things, I think it's fine to keep it a path.

However we should document this!

[kinke]

I don't have a too strong opinion on this. Some tools like reggae do use DC, but they are probably fine with an absolute path.

For dub CI, the problem was that it e.g. uses dub --compiler=$DC as a bash command on Windows, in the CI script. The backslashes in there [it wasn't really using the env var from bash, but using GHA's ${{ env.DC }} syntax, which definitely won't convert paths to bash-style paths] were swallowed and would probably need to be escaped. And it would need quoting too now, to allow for paths with spaces on Windows.

So I guess it's mainly these scripting usages which might need to be adapted for absolute paths in v2.

[WebFreak001]

I know DUB is fine with specifying paths as --compiler

I think the issue there sounds like escaping isn't being done properly in the action / caller

[kinke]

FWIW, DMD CI itself broke too, on Windows: https://github.com/dlang/dmd/actions/runs/9311624936/job/25631109320 (HOST_DMD is set to $DC, an absolute path)

[the-horo]

I've been looking at the action code to see if I can spot any more differences that people may hit. Let me see what broke here.

[the-horo]

Let me see. HOST_DMD is C:\hostedtoolcache\windows\dmd\2.108.1\x64\dmd2\windows\bin\dmd which https://github.com/dlang/dmd/blob/b0098317d74cd9379dae8acb61c0e0f45ac14393/compiler/src/build.d#L1741-L1742 leave unchanged and the check fails on https://github.com/dlang/dmd/blob/b0098317d74cd9379dae8acb61c0e0f45ac14393/compiler/src/build.d#L1311 because the actual file is ...\dmd.exe instead of ...\dmd.

[kinke]

Yeah no worries, just saying that this change is a pretty big deal and needs to be emphasized somewhere.

[the-horo]

Indeed. I think it's best we leave v2 without a release for now and we try to port current CI used by projects to it to see if anything breaks and then make a proper release. But, I don't have experience writing stuff that other people depend on and I've already managed to break it so I'm content just fixing things for a while and seeing what others want.

[WebFreak001]

as long as it's not updated to v2 this should not auto deploy

[WebFreak001]

I think the branch names might have been conflicting with the version resolution, I renamed the dev branch to main now and renamed the old branch to old

[kinke]

Neither does ldc support the dmd CLI options - that's what the bundled ldmd2 tool does.

In other places, we have DC set to the actual D compiler (dmd/ldc2/gdc), and DMD to the DMD-compatible driver (dmd/ldmd2/gdmd).

[the-horo]

The distinction is because you need to fetch different things for gdc vs gdmd, whereas with ldc2 ldmd2 comes wit it.

[the-horo]

I want to explain what my reasons were when I made this decision since they aren't properly described in the README.

Firstly, while ldc2 doesn't support all the options of dmd, some common ones are the same, like -wi, -de, -main, -run. and -of. gdc, however, only seems to agree on the -c, -I and -J flags. I know that some projects like DCD:common, D-Scanner, and dfmt do use -run in their build process.

It may be the case that I overestimate how much those packages are used so maybe very few people will ever need -run supported but I do want to encourage projects to start supporting gdc and I try to do this by providing the easy and accessible option of specifying gdmd to the action and not need to worry further. This is also why the gdmd_sha option exists, so that you can use a patched gdmd that supports dub (see D-Programming-GDC/gdmd#16) to minimize friction when beginning to use it because I expect most projects to be using dub.

Secondly, if you're using ldc2 but want your build to use ldmd2 you can just do DC=ldmd2 and it will work. If you do this with gdmd you may get unexpected results as the actual script you're supposed to call depends on the version you specified to the action. i.e. if you specify gdmd-12 but then, you run gdmd you will use gdc instead of gdc-12 which, currently, is gdc-11. The frontend version of gdc-11 is 2.076 and of gdc-12 it is 2.100. This is way too big a difference and having users accidentally specify the wrong version is something I want to prevent, because, again, I'm trying to make gdc as easy to use/hard to break as I can in the current circumstances. If you or somebody else wants to also have this option for ldc2 I have nothing against, it shouldn't be that hard to implement.

Lastly, like I said above, gdmd doesn't come by default when you apt install gdc. And, when you install gdc-12, if you want to install gdmd you will also need to install gdc so gdc-11 which will double the number of packages. Maybe I'm too greedy with runners' resources but I consider this not to be worth doing by default. This is also kinda fixed in ubuntu-24.04 since gdc there points to gdc-13.

[kinke]

If one is using dub or reggae for building, then the driver doesn't really matter, and using the native executable is probaby the better idea. So this distinction is IMO mostly relevant when building manually, where e.g. DMD's -version=Foo is -d-version=Foo for ldc2.

Wrt. gdmd, AFAIK all that's needed is that little script and putting it in the same directory as the gdc-<N> executable, matching the major version in the gdmd-<N> file name. So I'd expect this action to do just that, when specifying a gdc compiler. Then it's up to the user to select gdc or gdmd for whatever they need, or let dub/reggae find a default compiler.

[kinke]

Fetching the gdmd script is also what the dlang install.sh script does: https://github.com/dlang/installer/blob/e1cae8fdede66a6aab9bdbe9410751b589e308a6/script/install.sh#L875-L878

I think mimicking install.sh is the main goal of this action, extending it to Windows too.

[the-horo]

If one is using dub or reggae for building, then the driver doesn't really matter, and using the native executable is probaby the better idea. So this distinction is IMO mostly relevant when building manually, where e.g. DMD's -version=Foo is -d-version=Foo for ldc2.

I agree that it is preferred for build programs to use the compiler directly and not any DMD-compatible wrapper. This is what meson does, in fact it won't even allow you to use gdmd or ldmd2.

Wrt. gdmd, AFAIK all that's needed is that little script and putting it in the same directory as the gdc-<N> executable, matching the major version in the gdmd-<N> file name. So I'd expect this action to do just that, when specifying a gdc compiler. Then it's up to the user to select gdc or gdmd for whatever they need, or let dub/reggae find a default compiler.

gdmd is just a script, but since it can be installed from the ubuntu repos, like gdc is, it leads to this issue with the gdc package which is a fixed version instead of a virtual that can be satisfied by any gdc package.

Fetching the gdmd script is also what the dlang install.sh script does: https://github.com/dlang/installer/blob/e1cae8fdede66a6aab9bdbe9410751b589e308a6/script/install.sh#L875-L878

The action could resort to only downloading the script from the repo and default to a random commit. It's probably fine

I think mimicking install.sh is the main goal of this action, extending it to Windows too.

Sure, we can install the script on Windows, but without gdc it's useless.

Then it's up to the user to select gdc or gdmd for whatever they need, or let dub/reggae find a default compiler.

Let's consider that we go with always installing the gdmd script, whatever way it may be. We could set the environment like this:

DC=<path_to_compiler_binary>
DMD/DMDC/some_other_name=<path_to_dmd_driver>

Then tools like dub will pick the right compiler and users have an easy way to switch to the dmd wrapper if they need to. Does this sound good? This has been how it's structured on Gentoo and it has been working fine so far but the packages in Gentoo are not representative of the entire Dlang ecosystem.

[kinke]

The action could resort to only downloading the script from the repo and default to a random commit. It's probably fine

Yeah that's what I meant, just downloading latest master or so.

I think mimicking install.sh is the main goal of this action, extending it to Windows too.

Sure, we can install the script on Windows, but without gdc it's useless.

I didn't mean using that script at all, it's just the 'usual' way of installing an official compiler package on Posix, not a distro package. What I actually meant to say is that

1. the compiler selection seems to align between that script and this action here, and
2. the environment variables setup seems to match too (PATH, LD_LIBRARY_PATH and DC (just the basename!)). [The install.sh script generates little activate scripts, so that source ~/dlang/dmd-2.108.1/activate sets up the environment.]

We could set the environment like this: […]

Yeah that's what I meant earlier. In the places I was involved in, we used a DMD variable for the DMD-CLI-compatible driver. Edit: Oh, the activate script does set a DMD env var too.

[WebFreak001]

I think the tasks resulting from this:

• include gdmd inside the gdc installation
• add DMD variable that points to the dmd/ldmd/gdmd wrappers

[the-horo]

Seems reasonable to me. I can't promise how much if at all I'll be able to work on this the next week though.

• include gdmd inside the gdc installation

Do note that this would imply removing the gdmd-<version> format for dc from the action.

Yeah that's what I meant, just downloading latest master or so.

I don't know how sold I am on master by default. Maybe since it's just a script that barely receives any updates it's ok.

I think mimicking install.sh is the main goal of this action, extending it to Windows too.

Sure, we can install the script on Windows, but without gdc it's useless.

I didn't mean using that script at all, it's just the 'usual' way of installing an official compiler package on Posix, not a distro package. What I actually meant to say is that

1. the compiler selection seems to align between that script and this action here, and
2. the environment variables setup seems to match too (PATH, LD_LIBRARY_PATH and DC (just the basename!)). [The install.sh script generates little activate scripts, so that source ~/dlang/dmd-2.108.1/activate sets up the environment.]

It can go back to DC being just the basename. the activate scripts sound nice when working with multiple versions but I don't think they're that useful in the context of the action. Also, do we set LD_LIBRARY_PATH for every compiler like the install script or do we do it only for dmd, since it's the only one that seems to need it?

[kinke]

I've always seen this action here as a little shortcut for GHA, doing the same stuff as a local install.sh + activate would, but all as a single little convenient step, and additionally working on Windows too. [Edit: And persisting the env vars for the whole job, whereas activating the compiler would need to be done for every step using it.] The compatibility is nice for all local and non-GHA-CI use cases, allowing to reuse scripts etc.

Wrt. DC as absolute path or just the basename, I don't have a strong opinion as said earlier. As long as it's clearly documented (and it now is, thx!) and only in v2, I think it's fine, and CI scripts should ideally allow for both variants, coping with absolute paths too, just like the tools using DC do.

Also, do we set LD_LIBRARY_PATH for every compiler like the install script or do we do it only for dmd, since it's the only one that seems to need it?

I forgot that this was introduced at one point, wasn't always the case. Certainly years ago, but the action here probably still supports older versions which didn't bake an rpath into binaries. So probably safer to keep setting it.

[WebFreak001]

@kinke can you create a test case for what broke on DUB and what you need fixed? Otherwise it might regress easily again

[kinke]

Well I have much better stuff to do. I'm just stumbling from one problem to the next. ;)

Have you removed the v1 ref now altogether?!

[WebFreak001]

v1 was renamed to old (and I fixed the branch to be on the v1.4.0 commit again)

[kinke]

Well then you've just broken all @v1 usages, as we do in dub and certainly many other places: https://github.com/dlang/dub/actions/runs/9320838532/job/25658507321?pr=2910

[WebFreak001]

bleh that's annoying, then I'll just rename it back

[kinke]

That v1 is broken though, due to the DC var being absolute now. That's a big breaking change and must be reverted.

[WebFreak001]

v1 exists again and I reverted it to be 1.4.0 instead of having this PR merged. This PR is now only available on v2

[kinke]

Ah alright alright, that sounds much better, thanks!

[the-horo]

I've tried to write a proper changelog in #82