-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(pkgs/dmd): Version 2.092.1 supported #16
Conversation
7733c3d
to
b90d46d
Compare
d884879
to
d5da1a5
Compare
Needs to be moved to |
19ac974
to
94561da
Compare
@PetarKirov updated! |
We can add a function which given a compiler version returns the bootstrap version that should be used to build it. I assume that we will need to do this at some point (even if the reason is not supporting old versions) as newer compiler versions may need to be bootstrapped with new versions themselves. |
Thanks @dukc, but can you rebase this PR and resolve the conflicts, since I merged the 2.096.1 one? Additionally, can you see if you can use (and extend if necessary) the |
94561da
to
ea8a16d
Compare
@PetarKirov Yes, done and thanks.
I don't have a Darwin machine (perhaps I should...) so can't test what I add. I settled on copying 2.096.1 darwin config for 2.092.1.
I checked and noticed we're using build-status to skip |
Stupid me, I can check the Github CI results for that. |
cc7aa39
to
c01e713
Compare
I'm working on moving all version-conditional logical about skipping tests to a |
c9fcb1f
to
d2d518c
Compare
I agree in half. IMO, the question is do we want to make the test in question to work in the future? If we do, |
Was changing the hash format myself, got merge conflicts and noticed you have already fixed it yourself, and on top of that done a boatload of other changes to |
I have several goals (that are at odds with each other):
Given these two goals alone, I expect the list of comparability
It may be too ambitious, but yes. If at the time of a given pull request we can't make a given test work we will leave it disabled, but if possible, it would be nice if we could make it work at a later time. After all, all DMD releases have their test suites passing at 100%. Surely, with the power of Nix reproducibility we should be able to achieve the same. Well at least in theory. In practice, for the following examples I'd say:
|
Btw, I'm not satisfied with the
|
Yes, sorry for not being clear about my intentions. I did the first batch of changes before going to bed, but I really should've said that I've already done the changes that I requested. In my view, you did a good enough job so far with this PR, so I didn't want to ask for more with what I had planned about |
59a9320
to
644b8f1
Compare
Also fixed some build errors for 2.088.1, but it doesn't yet build with these since we're using a newer bootstrap compiler than OldDDerivations did.
And move all skipped tests from dmd's derivation to to pkgs/dmd/build-status.nix.
d14624c
to
ab36881
Compare
ab36881
to
50062d0
Compare
Thanks for your Pull Request! Below you will find a summary of the cachix status of each package, for each supported platform.
|
@dukc The pipeline is now green after my I'll now go ahead and merge this PR (as I have taken long enough to do it already!), but I'd appreciate if you could review these two commits, if you have the time. |
The Ddoc year test was in fact precisely what I was thinking as an example of what we don't want to clutter the derivation for, more than it takes to disable it at or before the last time it was hardcoded. But for anything else, if in doubt let's put it to |
I think the name is fine. If you want an alternative, maybe And as I wrote above, I think most of the version conditions that disable tests (as opposed to setting compiler flags of patching the source) actually do belong there. For the remaining patches, splitting the patches to another file, say |
Reviewed. All good. Not only that, they are in line of what I advocated for (test patches in |
Forgot to ping @PetarKirov . |
Thanks!
👍
Exactly!
To be honest, I'd prefer to keep all of the disabled tests out of the derivation. If you prefer, we can have something like
Yeah, I was thinking the same as well.
I don't have strong leanings in either direction, so let's leave it as it is. Of course, we can revisit this in the future. |
@dukc Btw, I finally managed to do the Nixpkgs upgrade (2023-11-04 -> 2024-02-16) in #49. The patch I added in this PR (9a750cd) certainly helped, but unfortunately, I had to disable additional C++ ABI tests in 75a2de5. The main change to darwin stdenv in Nixpkgs during this period was the clang 11 -> clang 16 bump (NixOS/nixpkgs#234710). Unfortunately, the issues with clang 16 are not surprising, as upstream DMD tests only clang versions v8 - v13. |
Also fixed some build errors for 2.088.1, but it doesn't yet build with these since we're using a newer bootstrap compiler than OldDDerivations did.
Note: I'm targeting #9 as opposed to main, so that we don't have to rebase one branch on top of the other later.