search

ocaml / ocamlbuild

The legacy OCamlbuild build manager
Other
121 stars 81 forks source link

Add MSVC testing #347

Closed dra27 closed 2 months ago

dra27 commented 2 months ago

Some work needed on the testsuite, but starting to build on the truly astounding work already done on Windows support here!

Should fix #323 as we're now testing msvc

hhugo commented 2 months ago

I've pushed changes to the testsuite. CI is now happy

hhugo commented 2 months ago

One still need to properly fix conditional steps in the CI

gasche commented 2 months ago

The PR as-is looks fairly clean (unlike some intermediate iterations); congratulations are due to @dra27 and @hhugo. Isn't this ready for review?

hhugo commented 2 months ago

No, I'm unhappy about the conditional step change in the ci

hhugo commented 2 months ago

@gasche, this is now ready.

dra27 commented 2 months ago

Thanks, @hhugo - you got the fixes in for the testsuite before I'd had a chance to finish my version locally! Technically c664696 is not part of restoring MSVC - I don't know if you'd prefer that to be a in a separate PR?

hhugo commented 2 months ago

The mix of "foo" -.- bar and "foo." ^ bar in the testsuite is a bit jarring (it took me a few dozen seconds to convince myself than no, they were strictly equivalent), but maybe we can tolerate this if you find it cumbersome to cleanup, and generally the patch is nice to read. I am okay with having the commit that adds .exe to the install script as part of this PR.

fixed