-
Notifications
You must be signed in to change notification settings - Fork 14
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
Fix msvc compilation #132
Fix msvc compilation #132
Conversation
I still am not thrilled with this solution, as the build script is harder to read.
Co-authored-by: Mateusz Front <[email protected]>
they were only used in one place anyway
The last thing I'm working on which will get this into a place that seems reasonable (after review, of course) is just getting tests working. |
I'd like some feedback on 6482bb8 and f6e6740. As far as I understand, these are the possible values, which don't really line up with the typical cpu values. I'd also rather have Erlang return a proper triple - I opened an issue, but previous and current OTP builds still need this sort-of-hacky handling, I think. If there's any recommendation for how to handle this better, I'm all ears (or eyes, I guess). EDIT: I'm realizing that the architecture is used in the test suite for some precompiled ffmpeg binaries. I'm not sure if it's used specifically in any plugins that would need updating, but something to consider. |
I'd love to help, but I have no idea 😕
Yes, we ship precompiled binaries and they are used in various plugins, but, of course, not in Windows, so this shouldn't break anything, as behaviour on Unix is unchanged |
There's an ongoing PR related to Windows builds on arm64, which is apparently the main active work touching things like host triples from
Sorry, I wasn't very clear (or accurate). I was more thinking about if any plugins match on architecture. The values of I could canonicalize these to what's currently seen from Unix/Darwin values, but I'm not sure if it's worth it, especially since OTP might use different values in the future. So I think just using the output of |
Nope, they match on the OS too, I don't think we'll have anything specific to architecture but generic to OS ;) Please format the code and we should be good to merge 🚀 |
I think I got cnodes and ports working correctly, ish. Can't fully test yet because of other issues (test suite relies on pkg-config, trying to build mad mp3 plugin has issues from shmex, etc.), but so far it's looking promising. I think this is at the point of functionally being able to be merged. |
This is my (mostly) minimal set of changes to get Windows compilation working. There's still a couple things I want to add to this (cross-architecture compilation, maybe ability to pass different options to
vswhere
, etc.), but this is enough, I think, for one to compile a simple Bundlex app on Windows for Windows.One thing of note is the use of
vswhere
itself. As far as I understand, it's been the preferred way to find Visual Studio-related locations for various things since Visual Studio 2017, so I think it's within the realm of general availability. I could add back the option for an environment variable, but I believe the intent for Windows development is to modify the paths in a way wherevswhere
can still find them? I haven't done any Windows development since before VS 2017 came out, so I'm not sure. If there are any Windows experts that can chime in, that would help.