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

TBB : Update to new build system & added CMake support patch #105

Closed
wants to merge 1 commit into from

Conversation

boberfly
Copy link
Contributor

@boberfly boberfly commented Sep 4, 2018

Tested on Linux.

@johnhaddon
Copy link
Member

Hey Alex,
I'd like to keep building things using their official build processes, rather than have to maintain ad-hoc CMake setups that may or may not match the official builds, and may or may not break when we update versions. Can you give me the rundown on why TBB can't be built on Windows using the official setup?
Cheers...
John

@boberfly
Copy link
Contributor Author

boberfly commented Sep 4, 2018

Hey John,

Yep I should sleep but nope. There is a bit of a discussion here about it:
uxlfoundation/oneTBB#6

The official way to build TBB tends to really "suck" in terms of needing to open up Visual Studio and convert their project to 2017 first and then build with the UI, it makes it problematic to do CI type builds.

I see that they have some python build script now that they didn't have before, but from reading that issue I'm not the most confident that it'll be as easy and straightforward than the CMake solution. Maybe we can keep the ./configure+make for Linux/macOS and apply this patch for Windows-only?

@johnhaddon
Copy link
Member

Is there any mileage in the suggestions from this comment on that thread?

Just a side comment on how easy it is to build TBB on Windows. Please take a look at how conda-forge builds TBB package: Windows
All you need is the right gmake package. E.g. you can take m2w64-make from conda-forge channel and execute mingw32-make in order to build TBB. conda-build tool provides desired level of environment isolation so that other mingw tools like sh are not on the way.

I see that they have some python build script now that they didn't have before, but from reading that issue I'm not the most confident that it'll be as easy and straightforward than the CMake solution. Maybe we can keep the ./configure+make for Linux/macOS and apply this patch for Windows-only?

I'd be happier if it was a Windows-only thing, for sure. Is the CMakeLists.txt in the patch a copy of Wenzel Jakob's CMake setup? If so, we would need to check the license and include it in the file.

But if there's an official way of building without having to maintain additional stuff, that would be preferable to my mind...

@boberfly
Copy link
Contributor Author

boberfly commented Sep 4, 2018

Just a side comment on how easy it is to build TBB on Windows. Please take a look at how conda-forge builds TBB package: Windows
All you need is the right gmake package. E.g. you can take m2w64-make from conda-forge channel and execute mingw32-make in order to build TBB. conda-build tool provides desired level of environment isolation so that other mingw tools like sh are not on the way.

One glaring problem is that it uses mingw32/mingw64 so the symbols probably won't link with VS2017. That and windows users would need to install msys/mingw.

I'd be happier if it was a Windows-only thing, for sure. Is the CMakeLists.txt in the patch a copy of Wenzel Jakob's CMake setup? If so, we would need to check the license and include it in the file.

He didn't make any license but in my patch file I do link his Github repo.

But if there's an official way of building without having to maintain additional stuff, that would be preferable to my mind...

Oh damn you're really not going to like it then when I try to introduce the third party build system for Python then! :) (Fortunately that one I think Eric found a URL package for it and it's not a big patch file).

@johnhaddon
Copy link
Member

He didn't make any license but in my patch file I do link his Github repo.

https://github.com/wjakob/tbb/blob/master/LICENSE

Oh damn you're really not going to like it then when I try to introduce the third party build system for Python then! :)

Probably not! It's really hard for me to comment with any authority because I don't even have a Windows box for testing, but I think there is a lot of value in minimising the amount of additional stuff that needs to be maintained. At the end of the day though, as long as the Linux/OSX stuff remains simple, and you assure me any additional Windows complexity is genuinely warranted, I won't have much choice...

@boberfly
Copy link
Contributor Author

boberfly commented Sep 4, 2018

https://github.com/wjakob/tbb/blob/master/LICENSE

I'm pretty sure that is just the regular TBB license though.

Probably not! It's really hard for me to comment with any authority because I don't even have a Windows box for testing, but I think there is a lot of value in minimising the amount of additional stuff that needs to be maintained. At the end of the day though, as long as the Linux/OSX stuff remains simple, and you assure me any additional Windows complexity is genuinely warranted, I won't have much choice...

That's fair enough, in this case I think the pros outweigh the cons (especially for CI and easily changing to debug builds as well), and it looks like Wenzel Jakob is reluctantly committed to maintaining his CMake scripts. I'll tweak this commit to revert to the old make scripts later tonight and we can do the Windows thing at a later date.

@johnhaddon
Copy link
Member

I'm pretty sure that is just the regular TBB license though.

Maybe so. In any case, it applies to what's in that repo, and it says this :

 4. Redistribution. You may reproduce and distribute copies of the
      Work or Derivative Works thereof in any medium, with or without
      modifications, and in Source or Object form, provided that You
      meet the following conditions:

      (a) You must give any other recipients of the Work or
Derivative Works a copy of this License

I'll tweak this commit to revert to the old make scripts later tonight and we can do the Windows thing at a later date.

Looks like you didn't get the chance? No worries, I'll close this and move it up myself, and we can worry about the CMake stuff later. Now I think about it, maybe we should just point the download URL to the releases from wjakob and then we don't need to worry about the license at all...and we'll have a tighter guarantee that his scripts work with the specific version we're building...

@johnhaddon johnhaddon closed this Sep 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants