Skip to content
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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jmdavis
Copy link
Member

@jmdavis jmdavis commented Apr 6, 2024

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.

@jmdavis jmdavis added the Phobos 3 The PR is for Phobos V3. label Apr 6, 2024
@dlang-bot
Copy link
Contributor

Thanks for your pull request, @jmdavis!

Bugzilla references

Your 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 locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + phobos#8972"

@jmdavis
Copy link
Member Author

jmdavis commented Apr 6, 2024

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

/usr/bin/link: extra operand '/OPT:ICF'
Try '/usr/bin/link --help' for more information.
Error: C:\Program Files\Git\usr\bin\link.exe failed with status: 1
Error C:\hostedtoolcache\windows\dc\ldc2-1.37.0\x64\ldc2-1.37.0-windows-multilib\bin\ldc2.exe failed with exit code 1.

And then buildkite is failing with

Error Package with target type "none" must have dependencies to build.

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.

@thewilsonator
Copy link
Contributor

#8992 should fix the lack of dub problems, the windows failure I have no idea.

@jmdavis jmdavis force-pushed the pv3_unittest_build branch 2 times, most recently from c82e664 to 67138f8 Compare April 29, 2024 04:49
@kinke
Copy link
Contributor

kinke commented May 3, 2024

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 link.exe in PATH is the MS one, not the GNU one. But when using DMD as compiler, not the LDC host compiler, that problem vanishes too.

@kinke
Copy link
Contributor

kinke commented May 3, 2024

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.

@jmdavis jmdavis force-pushed the pv3_unittest_build branch from 67138f8 to 4d31471 Compare October 26, 2024 07:26
@jmdavis
Copy link
Member Author

jmdavis commented Oct 26, 2024

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.

@jmdavis jmdavis force-pushed the pv3_unittest_build branch from 4d31471 to 84e8b13 Compare October 26, 2024 09:07
@jmdavis
Copy link
Member Author

jmdavis commented Oct 26, 2024

Okay, it now takes the DMD environment variable that the Makefile uses. So, hopefully, that will fix the CI.

@jmdavis jmdavis force-pushed the pv3_unittest_build branch from d3cd2cf to fe9a24c Compare October 26, 2024 09:43
@kinke
Copy link
Contributor

kinke commented Oct 29, 2024

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.
@jmdavis jmdavis force-pushed the pv3_unittest_build branch from fe9a24c to 56ef5fb Compare October 31, 2024 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Phobos 3 The PR is for Phobos V3.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants