Appveyor -- first stab#1399
Conversation
|
This looks like a mismatch between compiler flags, generally you must use the same /MT, /MTd, /MD or /MDd flag for compiling all libraries. The Blender libraries used are built with /MT, so to link with them you must also use /MT for OIIO instead of the default /MD. The Blender OIIO build script uses /MT: |
.appveyor.yml
Outdated
|
|
||
| install: | ||
| - cinstall: python | ||
| - cinst ninja |
There was a problem hiding this comment.
What's the distinction between cinstall and cinst here?
There was a problem hiding this comment.
The distinction is that I probably cut one line from one project's appveyor.yml, and the other line from a different project's appveyor.yml.
This is a truly Frankenstein's monster of build scripts. I literally don't know what 90% does.
I'm flying blind here, googling other projects' appveyor files and cutting and pasting the bits that look like they do things that my project needs. :-)
There was a problem hiding this comment.
Haha :-)
AFAICT from a search, cinst is a batch command alias of choco install, to install ninja via the chocolatey package manager.
When lines inside the install block are prefixed with <interpreter>: that's the script interpreter used to run them. Presumably cinstall is the appveyor alias of chocolatey install... not sure though.
|
@brechtvl : so it's just a matter of having an add_definitions("/MT") in my cmake? |
|
For the purposes of appveyor, should I prefer to do a static library build or a dynamic library build? |
|
Yes, it's a matter of doing it consistently, besides that it doesn't matter here. In CMakeLists.txt you could do something like this, but it would need to be an option since not everyone wants this. In the Blender OIIO build script we use -DCMAKE_CXX_FLAGS_RELEASE="..." and override the flags entirely instead of appending /MT. The Boost and zlib libraries would need to be built with the same flags as well, it's not clear to me where those Boost libraries in the script are from. Or alternatively you could build everything with /MD, but then you can't use the Blender libraries. |
Yeah, that's a good way to get everything consistent. The same ABI issues exist with gcc, but to a much lesser extent (-std=c++11 vs c++98 is the only one which I've had to worry about myself). |
|
OK, so I grabbed the Blender libraries just for convenience. Maybe I should jettison those in favor of building those dependencies myself, now that I have other examples of how to do so. So, in a world where the Blender deps were not an issue, would it be better to build everything with /MT or /MD? |
|
Personally I'm inclined to build third party libs by hand, especially if they have portable build systems of their own and aren't too huge. Large deps can lead to long build times, but the appveyor build cache could help you there if necessary. No idea about /MT vs /MD sorry! |
|
Got it! With some help from the openexr mail list, among others, and a little bit of messing with this in my spare time, I was able to get all the important dependencies to build, fix the handful of compiler errors in OIIO itself, and get to a clean build. It's a more extensive patch than I originally thought, but it works. At this point, it build the whole project cleanly, so therefore is already going to be a big help by preventing us from breaking the Windows build with new checkins. I don't have it running the testsuite as part of the appveyor process yet; I wanted to get the build checkpointed before moving on to running tests. Will do that after this is committed. |
Doesn't run tests yet, but it builds! This includes many minor fixes to make the Microsoft compiler happy, addressing a few compile errors and many warnings. Also includes steps to speed up OpenEXR build: supply our own b44ExpLogTable.h and dwaLookups.h. Also try not to build OpenEXR tests and binaries.
appveyor.yml
Outdated
| else { | ||
| $env:vcvar_arg = 'x86'; | ||
| } | ||
| - call "C:/Program Files (x86)/Microsoft Visual Studio 14.0/VC/vcvarsall.bat" |
There was a problem hiding this comment.
I'm not quite sure, but can you get rid of the ps and call commands above?
There was a problem hiding this comment.
I'm not quite sure either. A lot of this whole process is black magic.
I'll comment these lines out and try one more build, just to see if it works. But if that breaks things, I'm going to commit what I have for now rather than try to understand why it didn't work.
I need to make a second pass over this all (including making it run the testsuite in addition to performing the build), but I'm really itching to commit what I have so far. I've been working on this in tiny bits of free time here and there for weeks.
There was a problem hiding this comment.
Yes, turned out that eliminating those lines entirely had no apparent ill effect.
|
Sure, LGTM really. |
3 participants
First stab at using Appveyor (like TravisCI, but for Windows). It kinda works, in the sense that I git it far enough that it installs several dependencies, and compiles most of OIIO. But it has link errors, and I've about hit the limit of what I know about Windows development. Since this can't hurt anything, I think it's best to have it reviewed, merge it, and hope somebody with more Windows + Appveyor experience can suggest the changes (assumed to be minor) to let it build fully. At which point, it will be an important component of making sure we keep changes to the software from breaking the Windows build.
This PR includes a bunch of minor changes to fix warnings that showed up with the MS compiler.
You can see the results of the run here: (I think that link should be publicly readable)
https://ci.appveyor.com/project/lgritz/oiio/build/lg-appveyor-59/job/phg4w2jt2qamlnbi
I'm open to suggestions! I truly have no no idea how to fix it.