lsl_archived icon indicating copy to clipboard operation
lsl_archived copied to clipboard

proposing changes to the CMake environment (discussion)

Open dmedine opened this issue 7 years ago • 8 comments

So I've finally started playing around with this and I really like it. So far, I have only used cmake to generate projects for liblsl and it is working very well for me. However, there are a couple of things I would like to change.

First of all and most importantly, I don't want to disturb the current structure of liblsl. I would prefer it if the generated projects target liblsl<(-debug)>. (e.g. liblsl32.dll or liblsl64-debug.dylib) and that these files get 'installed' into the liblsl\bin folder. It is important to me that the name of the libraries doesn't change.

In general, I think it is wise not to have it so that things can live in more than one place (with one notable exception that I will address below) or have more than one name. Also, the actual name of the library (liblsl) is associated with its 'brand', so to speak, which is reason enough not to change it (in my opinion). Also, suffixing the library name with the build tool is superfluous to me.

Also, I would like the liblsl/CMakelists.txt file to create projects that can build for either 32 or 64 bit. Maybe it is already doing this for other platforms, but not on Windows. If not, I'd be happy to add this feature.

Now about the apps. It is important that users be able to download individual zip files that contain the App exes and run them out of the box---without any system configuration or compiliation (this is the exception to the 'live in more than one place' rule). I haven't tried the cmake method on the apps yet, but I can't see that it violates this requirement. However, I think it is wise to avoid the word 'install' in the documentation. The apps aren't actually installed (in the sense that they don't go into /usr/bin or whatever) and I think it is important that users are aware that these are stand alone applications that they need to keep track of. Maybe I'm wasting words on a small point, but I find this a little confusing when I read the documentation.

dmedine avatar Oct 02 '17 14:10 dmedine

Hi David,

I think @tstenner 'installed' (I'll come back to that) the libs where he did so as to not stomp on the current structure. If we're fine with replacing the old way of doing things with the cmake way of doing things then we can certainly 'install' the libs to the old location.

Suffixing the library name with the compiler is actually relevant in Windows for some libraries, because a program being built in MSVC cannot work when linked to some libraries built in MINGW and vice versa, but this might not be true (?) for LSL. We should test.

Also, I would like the liblsl/CMakelists.txt file to create projects that can build for either 32 or 64 bit.

As far as I know, this can't be done. This is the one tradeoff of cmake that I dislike, but I understand because of how it uses knowledge of the build target architecture to choose which libraries should be linked. The best we can do is write a script that will create two different build folders and call cmake twice.

The apps are built to be standalone. The installLSLApp function copies all the dependencies into the correct location so the app folder (or .app bundle in mac) can be packaged and distributed as-is.

As for the 'install' terminology, that's something entrenched in build systems. i.e. make install will simply do the steps necessary for the 'install' target, including copying binaries into whatever the install directory is. The 'install' concept is used throughout cmake and IDEs regardless of whether or not we like it. For example, in order to do all the library copying and rpath fixups, we have to select the INSTALL target and build it. Just because we're not putting the libraries and exes into the system's default install directory doesn't mean they aren't being installed... but I agree this subtlety will probably be lost on many users who expect that 'install' means that the binaries and required libraries will be somewhere on the path. However, we can't not use the word 'install' for the relevant cmake and IDE steps because it's what they use. And we can't put everything on the path because we also want to ship stand-alone apps with their own libraries (which would create confusion with on-path libraries). So maybe we just need to touchup the docs a bit to make it clear that the install dir is where the libraries and apps will be copied to after building is complete, and to be clear about how to change the install dir if they choose.

cboulay avatar Oct 02 '17 16:10 cboulay

The problem I have with changing the names is that it requires changing project settings (I know, CMake fixes everything, (except when it doesn't)). Also, I've seen people mangle their build environments and systems with unknown versions of libraries floating around enough to be wary of that. Also, for some purely emotional, irrational reason it just feels wrong to me. 'There can be only one!' All of which is to say, I'm happy to have the CMake files direct the library builds to the liblsl/bin folder.

As far as the names go, I'm of the opinion that anyone fooling around with MINGW should have the wherewithal to know what she's doing with a linker. I'm pretty sure you can just right click the thing and find out what's inside (although I'm at home on Linux right now, so that is merely speculation at this moment).

It is unfortunate, frustrating, and lame that CMake can't generate a project file that supports multiple architectures. Why not? It's just some more xml in the end. I'll do some research and see what can be done about this.

Thanks for your thoughts, cboulay on the install controversy that I seem to have raised. To be continued.

On 10/02/2017 06:25 PM, Chadwick Boulay wrote:

Hi David,

I think @tstenner https://github.com/tstenner 'installed' (I'll come back to that) the libs where he did so as to not stomp on the current structure. If we're fine with replacing the old way of doing things with the cmake way of doing things then we can certainly 'install' the libs to the old location.

Suffixing the library name with the compiler is actually relevant in Windows for some libraries, because a program being built in MSVC cannot work when linked to some libraries built in MINGW and vice versa, but this might not be true (?) for LSL. We should test.

Also, I would like the liblsl/CMakelists.txt file to create
projects that can build for either 32 or 64 bit.
As far as I know, this can't be done. This is the one tradeoff of
cmake that I dislike, but I understand because of how it uses
knowledge of the build target architecture to choose which
libraries should be linked. The best we can do is write a script
that will call create two different build folders and call cmake
twice.

The apps are built to be standalone. The installLSLApp function https://github.com/sccn/labstreaminglayer/blob/master/cmake/LSLAppBoilerplate.cmake#L73-L172 copies all the dependencies into the correct location so the app folder (or .app bundle in mac) can be packaged and distributed as-is.

As for the 'install' terminology, that's something entrenched in build systems. i.e. |make install| will simply do the steps necessary for the 'install' target, including copying binaries into whatever the install directory is. The 'install' concept is used throughout cmake and IDEs regardless of whether or not we like it. For example, in order to do all the library copying and rpath fixups, we have to select the INSTALL target and build it. Just because we're not putting the libraries and exes into the system's default install directory doesn't mean they aren't being installed... but I agree this subtlety will probably be lost on many users who expect that 'install' means that the binaries and required libraries will be somewhere on the path. However, we can't not use the word 'install' for the relevant cmake and IDE steps because it's what they use. And we can't put everything on the path because we also want to ship stand-alone apps with their own libraries (which would create confusion with on-path libraries). So maybe we just need to touchup the docs a bit to make it clear that the install dir is where the libraries and apps will be copied to after building is complete, and to be clear about how to change the install dir if they choose.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/sccn/labstreaminglayer/issues/229#issuecomment-333587184, or mute the thread https://github.com/notifications/unsubscribe-auth/ADch7vszIIN2U7iWIb4ItXB-cCPlTnAUks5soQ6NgaJpZM4PqsLv.

dmedine avatar Oct 03 '17 09:10 dmedine

I agree that the "install" name is a bit unfortunate, but we can't change it. This should be noted in the documentation, but end users who just download some packages won't see it. As for the "install" location, we can still change this, but a separate build dir has some advantages:

  • liblsl64-debug.dll already tells which build this is, but the executable names don't. For me, two build trees (lsl64-debug/AppXY/AppXY.exe and lsl64-release/AppXY/AppXY.exe) allow me to have a link to the release tree on a network share and be sure that I won't accidentally put debug binaries in there (which would need the debug VC runtime)
  • the source tree stays clean, removing a certain build is a matter of removing the build directory and it's nearly impossible to add build files to the git repository
  • installLSLAuxFiles takes care of where to put the additional files (e.g. for Mac OS X)

The 32/64 split is an unfortunate side effect of CMake treating the 32 and 64bit compilers as separate build systems which I haven't found a way around yet.

The library file names should be changed back, but in addition to the MSVC/MinGW problems @cboulay mentioned it's also problematic to use binaries produced with different MSVC versions together ("How bad is it to mix and match Visual C++ runtime DLL files in one process?" - "very bad"). Since we haven't released any binaries yet we should declare one VC version as the officially supported version (I found it quite easy to install a minimal VC2017 version). That way, we could also hardcode the generator names on windows and have a script generate the solution files for that exact version.

As for the apps, the build currently generates one portable folder for each app, but creates one archive with all folders. This archive isn't for endusers, but to minimize that archive size, because n copies of each library will only be stored once instead of n times.

tstenner avatar Oct 03 '17 09:10 tstenner

I think the way around the MSVC compatibility issue is to keep it old. This is the reason for using vc90 in the first place. However, for those that want it, newer editions are now available. I think there is probably a more elegant solution to this naming issue. Boost does this, for example, but I'm not quite sure how it works internally on f'n Windows where everything like this is always a million times harder than it is on Unix. I will check.

On 10/03/2017 11:53 AM, Tristan Stenner wrote:

I agree that the "install" name is a bit unfortunate, but we can't change it. This should be noted in the documentation, but end users who just download some packages won't see it. As for the "install" location, we can still change this, but a separate build dir has some advantages:

  • liblsl64-debug.dll already tells which build this is, but the executable names don't. For me, two build trees (|lsl64-debug/AppXY/AppXY.exe| and |lsl64-release/AppXY/AppXY.exe|) allow me to have a link to the release tree on a network share and be sure that I won't accidentally put debug binaries in there (which would need the debug VC runtime)
  • the source tree stays clean, removing a certain build is a matter of removing the build directory and it's nearly impossible to add build files to the git repository
  • |installLSLAuxFiles| takes care of where to put the additional files (e.g. for Mac OS X)

The 32/64 split is an unfortunate side effect of CMake treating the 32 and 64bit compilers as separate build systems which I haven't found a way around yet.

The library file names should be changed back, but in addition to the MSVC/MinGW problems @cboulay https://github.com/cboulay mentioned it's also problematic to use binaries produced with different MSVC versions together (see e.g. here http://siomsystems.com/mixing-visual-studio-versions/ or "How bad is it to mix and match Visual C++ runtime DLL files in one process?" - "very bad" https://stackoverflow.com/questions/19944589/h). Since we haven't released any binaries yet we should declare one VC version as the officially supported version (I found it quite easy to install a minimal VC2017 version). That way, we could also hardcode the generator names on windows and have a script generate the solution files for that exact version.

As for the apps, the build currently generates one portable folder for each app, but creates one archive with all folders. This archive isn't for endusers, but to minimize that archive size, because n copies of each library will only be stored once instead of n times.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/sccn/labstreaminglayer/issues/229#issuecomment-333794647, or mute the thread https://github.com/notifications/unsubscribe-auth/ADch7nwtUrB-nyf_knLrb8ZKfh_8p4KHks5sogQSgaJpZM4PqsLv.

dmedine avatar Oct 03 '17 10:10 dmedine

@tstenner , I agree that we don't want to build in source, and we should still be able to build in labstreaminglayer/build and install lsl libraries into labstreaminglayer/LSL/liblsl . Is it trivial to then reset the install prefix (build/Apps?) for the apps?

There are a bunch of arguments for not locking down to an official MSVC version, but the most important is that we can't decide what version of MSVC the manufacturer-supplied SDK will work with.

cboulay avatar Oct 05 '17 16:10 cboulay

Installing the libraries to the corresponding source locations should be doable, at least the .gitignore only needs to contain the build folder and the usual binary extensions (and exceptions for lslboost, as I've recently discovered). Mixing LSL and SDKs compiled with different versions might lead to problems anyway, but I don't see the problem with one suggested version (with official binaries) and compatibility requirements (VS 2008?) for LSL in case some SDK requires it. Unless most SDKs require VS2008, of course.

tstenner avatar Oct 05 '17 17:10 tstenner

Most SDKs (as far as I can tell) are built on more modern compilers. It's been my experience that generally with MSVC that nearly everything is forward compatible and some things are back compatible. I've never actually had any problems mixing newer SDKs and older LSLs.

On 10/5/2017 7:31 PM, Tristan Stenner wrote:

Installing the libraries to the corresponding source locations should be doable, at least the .gitignore only needs to contain the build folder and the usual binary extensions (and exceptions for lslboost, as I've recently discovered). Mixing LSL and SDKs compiled with different versions might lead to problems anyway, but I don't see the problem with one suggested version (with official binaries) and compatibility requirements (VS 2008?) for LSL in case some SDK requires it. Unless most SDKs require VS2008, of course.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/sccn/labstreaminglayer/issues/229#issuecomment-334536234, or mute the thread https://github.com/notifications/unsubscribe-auth/ADch7udPxIPf4euc-imBD6eAc7imdihBks5spRJwgaJpZM4PqsLv.

dmedine avatar Oct 06 '17 06:10 dmedine

In-source builds (i.e. configuring the project via cmake . from the root directory) should put the binaries in the source directory (tested on linux). All the byproducts (object files etc.) are still put in separate (CMakeFiles) directories. If this works for you, I could leave the install part as it is.

tstenner avatar Oct 08 '17 09:10 tstenner