-
-
Notifications
You must be signed in to change notification settings - Fork 136
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
allows for proper building of LuaJit on Windows x64 #251
base: master
Are you sure you want to change the base?
Conversation
…s it allows for building LuaJit on windows.
The Windows build fails because we don't call |
This explains it: |
It seems that |
My apologies, I had been running from vcvars64 when I did my tests as that is usually something that one does when building cython projects on windows in my experience. I am not sure how one would find the location of vcvars64 or vcvarsall to build against however as it seems like it could be version specific. Additionally, I am unsure about the Visual Studio installation location on the CI. |
…the path that the vs2019 image uses.
I have added a check that searches for where I believe the file should be in the currently selected appveyor image. from what little I could find by looking through the install script. I found the following for reference: |
try: | ||
subprocess.run(["C:\\Program Files(x86)\\Microsoft Visual Studio\\2019\\Community\\VC\\Auxiliary\\Build\\vcvarsall.bat", "x86_x64",]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it helps to do this. IIUC, the batch file sets up environment variables, so it needs to run in the same command shell that then executes the build.
Also, I don't think it's a good idea to use a hard coded path. There must be a way to figure it out programmatically. It might also be that the batch file is simply on the search PATH already and can be started as is.
Note that appveyor isn't really used any more since we use GHA also for the Windows builds. I should disable it.
This pull request makes slight changes to the currently existing setup.py to bypass the check for the name of the platform to start with 'win'. This allows LuaJIT to be built successfully on my machines, two running Windows 10 and another running Ubuntu 20.04. I have no way to test this on a Mac, however I do not believe it should impact it in any way. The only potential issue I could see with this fix is one mentioned in #235 where the CI may fail due to a hardcoded Unix path. However, as I am currently not aware of how the directory structure may change under a windows build host, I am unable to make this alteration myself. If anyone knows if there is a change in the directory structure, I would be happy to implement that here as well as to not clutter the repository with merges.