-
Notifications
You must be signed in to change notification settings - Fork 8
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
Add an install target, and allow linking against an installed libfaust. #13
base: master
Are you sure you want to change the base?
Conversation
Can you describe what it does? Is it working on Windows and MacOS with XCode and Visual Studio? |
See https://cmake.org/cmake/help/latest/command/install.html. I can't tell you whether those install targets do anything useful with Xcode or Visual Studio. But they're certainly helpful from the command line, when running Right now But you can change the The lib/pd/extra/faustgen~ destination directory probably won't really be that useful on anything but Linux, but I can change that to just faustgen~ for macOS and Windows, so that you could then use something like Let me quickly augment the PR so that it does something more sensible on Mac and Windows. Thinking about it some more, that path should probably be a variable which you can change at configuration time. |
Ok, there you go. Better now? |
@pierreguillot You probably know cmake much better than I do, so feel free to improve this. I hate cmake with a passion -- die-hard autoconf and GNU make fan here, and proud of it. ;-) Thus I don't know cmake very well, but I can quickly hack something together. |
Also, please note that in my install target, I only install the *.lib files from faust/libraries and faust/libraries/old into the faustgen~/libs destination. This is in line with that the Faust compiler itself does (just that there the files get installed into /usr/share/faust, along with all the architecture files), and thus will match what we get when building the package against an installed Faust package (cf. #12). In that situation, we just want the *.lib files which are readily available in a Faust installation, whereas the other auxiliary stuff in faust/libraries is not available in a Faust installation. |
… INSTALL_DIR variable, and use a more sensible default for that directory on Mac and Windows.
…installed Faust when linking against an installed Faust version.
Ok, there we go. This also includes #14 now, so that we have everything in one place. |
Added another STATIC_FAUST option (enabled by default) which makes it possible to link against an installed libfaust statically. This currently only works with gcc (i.e., Linux, Un*x, Mac, and Windows with mingw, but not msvc), so this should cover all cases where you actually might want to do this. I have tested this in all possible combinations (INSTALLED_FAUST=OFF, INSTALLED_FAUST=ON with both STATIC_FAUST=ON and OFF) on Linux, also tested the possible error conditions, they all work fine for me. The cmake code I added is pretty generic, so there's a good chance that it will work on Mac and Windows, too, but I haven't tested this. If someone else can test on Mac and Windows, please do. @pierreguillot The latest commit also includes a work-around for the problem with LLVM 10 that you observed earlier, namely that llvm_map_components_to_libnames comes up with an empty set of libraries. In this case we also run Ok, I'm done with this, I hope that everyone is happy now. CI is all good, so this is ready to go in. @pierreguillot please merge while it's hot, so that I can update my Arch PKGBUILDs. :) |
Thank you! I just had a quick look and I have global remarks:
Let me know what you think of it. I will have a better look now and do some tests. |
I considered that, but decided against it, because there's simply no reliable way to detect an installed libfaust across all platforms. The code I used assumes that the user knows what she's doing and has everything set up so that the Faust headers and libraries will be found without any additional magic. If that doesn't work out, then she always has the option to use the included source, which is also the default. But feel free to change this if you want to.
I think I do, but I'm not sure what "line above" you're referring to. Please elaborate, so that I can double-check that I did everything right.
Nope, for UNIX not APPLE, the default value is "lib/pd/extra/faustgen~". You mean the Mac/Windows case where the default value is just "faustgen~", I suppose. In any case that variable is a directory which is taken relative to CMAKE_INSTALL_PREFIX, as is customary with install targets, so it won't just clobber the root directory, if that's what you mean. I chose the "faustgen~" destination for Mac/Windows so that you can run something like I must admit that all this assumes that a user proficient enough to use
Well, it will already give you a warning and tell you to set FAUSTLIB if the search fails. This is more forgiving, and at least lets you continue with the build and then later copy the missing lib files manually. I think that this is preferable, but feel free to change it if you disagree. |
Ok, I might as well give it a go. Stay tuned. |
Ok, that was easier than I expected. Tested on Linux, with the Faust 2.27.2 package from Arch, works fine there. I also added libfaust.dylib for the shared lib NOT MSVC case, so that the code will hopefully work on macOS, too. |
You can see the error here: https://travis-ci.org/github/CICM/pd-faustgen/builds/712971074
So I guess the default value of INSTALL_DIR on Windows should be "Pd/extra/faustgen~"? This way, it would give "c:/Program Files/Pd/extra" by default. And I have to check on macOS.
Yes, but in this case, it copies the libs from a folder specific to Linux... it seems that the MR is mainly done and tested on Linux and not the other OS, so perhaps the features should be enabled on Linux only. I'm afraid that we'll have to maintain features for Windows and macOS that never worked. And there are 3 different features in this MR, perhaps it's too much, this way, even if there is a problem with one of them we could merge the others. I guess what is important is to link against a pre-installed faust library on Linux, so let start with that. I'll try to do that before the end of August. |
Well spotted! This is no big deal, I just need to add a check
IMHO that's not a workable solution. On Windows, the exe installer from vanilla indeed installs into %PROGRAMFILES%/Pd by default, but the zip archives for both Windows and Mac contain a folder Pd-version[-i386] depending on the particular version and architecture, which might be installed just anywhere where the user finds it most convenient. Good luck finding that installation anywhere on the hard disk, not even considering that a user might well keep different versions around for some reason. That's exactly why I picked an approach that makes it easy to do a staged installation and then just copy the stuff from the staging area to anywhere you want it to go. And how many Mac and Windows users do you expect to go through all the hassle and compile faustgen~ themselves when they can readily download it here or even install it from Deken? Not many I'd say. And these few are power users like me who do want to build from source, even on Mac and Windows, and for them
I concede that this might have been a bad (and rather Linux-centric) idea, and that it's better to error out in that case. That's easy to fix, too.
Not so quick. I'm not blind to the needs of Mac and Windows users, I occasionally run these as well, and in any case most of my students do, so Mac and Windows compatibility is important for me as well. It's just that I haven't got around to testing there yet. Of course that needs to be done before releasing. But I'm confident that all this works on Mac and Windows, or can be made to with little effort. |
…library files fails, or the user specified the wrong library directory.
…, and search for the Faust include files.
… variable can now be used to point cmake to the right llvm-config executable.
This is a lot more robust now. As promised, I've added the On the other hand, I kept the default INSTALL_DIR on Mac and Windows as is, because I really think that this is the most reasonable and convenient approach on these systems for the reasons I explained. This is also what I'm using myself in my development branch now. I tested this on Linux, Mac (using LLVM10 from MacPorts) and Windows (also using LLVM10 in Msys2), it all appears to work fine as far as I can tell. (BTW, there are some snags with pd.build producing MSVC-centric compiler options even when used with MSYS2, it would be nice if you could look into these some time. But I was able to work around these, so that I could test the entire build process including There you go. Do with it as you please. :) Of course, I'm aware that this project has been laying dormant since 2018. So don't feel obliged to use any of this, I can understand perfectly well if you've moved on to other things. I would have done this work anyway, and being able to discuss these changes with you really helped a lot to improve these features and make them foolproof, thanks for that! Ok, I'll stop bugging you know, and move back to my development branch where I've already taken faustgen~ much further. I have almost full feature parity with pd-faust and Grame's faustgen now, and adding the missing OSC support will be easy. All this wouldn't have been possible without your work, so thanks again for that! I'm really looking forward to use this in my master courses in the winter semester. I've made some improvements which are not backward-compatible, though, so I'll eventually rename the external, to allow both externals to be installed and used at the same time. (It goes without saying that I'll keep your credits, so that it remains clear who wrote the original version.) |
This adds an install target, so that
make install
will do something sensible, at least on Linux and other Unix-like systems. This will also simplify packaging.Also added an option to link against an installed libfaust, which fixes #12 (see #14 for details).
So, to summarize, this adds:
an option (cached BOOL cmake variable) INSTALLED_FAUST which is OFF by default, but can be set to ON to link against an installed libfaust; in that case a second STATIC_FAUST option (ON by default) allows the installed libfaust to be linked statically (setting that option to OFF will link against a shared libfaust if possible)
the requisite install target to make
make install
andcmake --install
workThe install target also takes into account INSTALLED_FAUST, so that it copies the Faust library (.lib) files from the right location (in-tree source if INSTALLED_FAUST is OFF, installed .lib files if it is ON). In the latter case, the location is auto-detected using a search for [share/]faust/all.lib, but can be overridden with the FAUSTLIB variable. Moreover, the destination directory for the external can be set with the INSTALL_DIR variable, which is taken relative to CMAKE_INSTALL_PREFIX. The default for INSTALL_DIR is lib/pd/extra/faustgen~ on Linux and other generic Unix-like systems, and just faustgen~ on Mac and Windows.