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

Make CMake sanitizers optional, and opt-in #795

Closed
probonopd opened this issue May 31, 2018 · 8 comments
Closed

Make CMake sanitizers optional, and opt-in #795

probonopd opened this issue May 31, 2018 · 8 comments
Assignees

Comments

@probonopd
Copy link
Member

probonopd commented May 31, 2018

I am trying to build libappimage using as many components from the distro as possible (since libappimage might eventually become distro-packaged one day).

Following https://github.com/AppImage/AppImageUpdate/blob/rewrite/BUILDING.md, I get when building AppImageKit:

CMake Warning at src/CMakeLists.txt:1 (find_package):
  By not providing "FindSanitizers.cmake" in CMAKE_MODULE_PATH this project
  has asked CMake to find a package configuration file provided by
  "Sanitizers", but CMake did not find one.

  Could not find a package configuration file provided by "Sanitizers" with
  any of the following names:

    SanitizersConfig.cmake
    sanitizers-config.cmake

  Add the installation prefix of "Sanitizers" to CMAKE_PREFIX_PATH or set
  "Sanitizers_DIR" to a directory containing one of the above files.  If
  "Sanitizers" provides a separate development package or SDK, be sure it has
  been installed.


-- Installing auxiliary files in path: lib/appimagekit
CMake Error at src/CMakeLists.txt:176 (add_sanitizers):
  Unknown CMake command "add_sanitizers".


-- Configuring incomplete, errors occurred!
@probonopd
Copy link
Member Author

https://github.com/AppImage/AppImageKit/blob/appimagetool/master/build.sh does not have this issue but since it does not even contain the word "sanitizers" I am lost...

@TheAssassin
Copy link
Member

git submodule update --init --recursive

Next time git clone --recursive <...>.

@probonopd
Copy link
Member Author

I am trying to build libappimage using as many components from the distro as possible

This is why I don't want to have to download all of that stuff... is there a way to do without --recursive and just get that one repo? Or use the sanitizers (whatever that is) from the system?

@probonopd probonopd reopened this May 31, 2018
probonopd added a commit to AppImageCommunity/AppImageUpdate that referenced this issue May 31, 2018
Would be better if we found a way without needing --recursive because it downloads a lot of stuff
@TheAssassin
Copy link
Member

Your bug report was titled Unknown CMake command "add_sanitizers"., so one has to assume that's what you want to fix.

The sanitizers are used to debug AppImageKit. This submodule provides some easy-to-use CMake modules to set everything up. If they're not explicitly activated, they're not used. You can for now just fetch them and continue. We should consider making the dependency optional then sometime in the future.

@probonopd probonopd changed the title Unknown CMake command "add_sanitizers". Make CMake sanitizers optional, and opt-in May 31, 2018
@TheAssassin
Copy link
Member

Upstream solution work in progress, will adjust our scripts accordingly once the PR has been merged.

@alehaa
Copy link

alehaa commented Jul 10, 2018

I don't think the variable would solve the problem ... find_package() prints a warning, whenever a specific module isn't found, unless you call it with the QUIET arg. Indeed, we'll need to add some kind of variable, so you can simply put a conditional around add_sanitizers() - we're discussing this in arsenm/sanitizers-cmake#16.

@probonopd The scripts of this submodule are not part of any distro afaik. These are just a bunch of simple scripts to detect some compiler features for development and will not be installed.

@probonopd
Copy link
Member Author

Are the sanitizers opt-in now?

@TheAssassin
Copy link
Member

They have always been opt-in, but now they're also optional. Please see #816 (comment) for details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants