-
-
Notifications
You must be signed in to change notification settings - Fork 700
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
Add the Phobos v3 unit tests to the regular unittest build. #8972
base: master
Are you sure you want to change the base?
Conversation
Thanks for your pull request, @jmdavis! Bugzilla referencesYour PR doesn't reference any Bugzilla issue. If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog. Testing this PR locallyIf you don't have a local development environment setup, you can use Digger to test this PR: dub run digger -- build "master + phobos#8972" |
Hmmm. It looks like the Linux, Mac, and FreeBSD VMs are all missing dub, and for better or worse, the Phobos v3 build uses dub, so those VMs don't work work with this. And the Windows x64 one is failing with an error that I don't understand
And then buildkite is failing with
implying that there's probably something about the dub.sdl file that doesn't work with the version of dub that it's running, but I don't know. So, I'm not quite sure what to do here. Obviously, the test runners in general are not set up to use dub. |
#8992 should fix the lack of dub problems, the windows failure I have no idea. |
c82e664
to
67138f8
Compare
The host compiler needs to be activated first, see e.g. https://github.com/dlang/dmd/blob/e5d7779794a1df4aa607e1ca989b3adf1f14073c/ci/run.sh#L130-L158. Then when using dub naively like this, it will most likely use the host compiler to compile Phobos, not fresh DMD master/stable. The Windows issue is actually an LDC problem, which assumes |
Extra care might also be required to make sure that Phobos v2 isn't importable and linked-in by default, only depending on druntime alone. As DMD doesn't have separate druntime and Phobos libs, but merges both to a single lib, that might be more difficult than it sounds. |
67138f8
to
4d31471
Compare
Okay. I reworked this to use the new build script and reworked the new build script slightly so that it could build the unit tests in both debug and release modes to match Phobos v2. So, while I'm sure that we'll want to do more tweaking later, this should at least get Phobos v3 tested by the CI in a similar fashion to Phobos v2. |
4d31471
to
84e8b13
Compare
Okay, it now takes the DMD environment variable that the Makefile uses. So, hopefully, that will fix the CI. |
d3cd2cf
to
fe9a24c
Compare
A rebase should fix the CI errors. |
I'm sure that we'll need to rework the build stuff at some point here (both for Phobos v2 and v3), but this gets the v3 tests run by the v2 Makefile so that the CI will build and run them.
fe9a24c
to
56ef5fb
Compare
This probably isn't the best way to do this, and presumably, it'll need to be reworked at some point (just like the Phobos v3 build in general likely will need to be reworked), but this should at least make it so that the Phobos v3 tests get run as part of the CI.