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

Updates dockerlizard and cMake -- fixes #68 #66

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nimish
Copy link

@nimish nimish commented Feb 11, 2023

The existing docker build and cmake build were not really using cmake to its fullest. This creates and builds libsparselizard.so and has a Dockerfile that'll install it to a GNU-style directory tree in /usr/local

You can override where it finds libraries by changing the CMAKE_PREFIX_PATH (or CMAKE_INCLUDE_PATH + CMAKE_LIBRARY_PATH) to wherever your custom libs and includes are. LD_LIBRARY_PATH or RPATH would need to change for runtime loading.

In addition, this makes it possible to cleanly build using system libs on both macOS and linux, which will make it much easier for sparselizard to be packaged into distributions.

The docker container that is built is barebones, but it is still good enough to allow for VS Code "remote container" style development. It's best used as a starting point for customization.

Effectively supersedes #50

@nimish nimish changed the title Updates dockerlizard and cMake Updates dockerlizard and cMake -- fixes #68 Feb 13, 2023
@Drinausaur
Copy link

Looking at the diff, it looks like you copied .h and a lot of .cpp files in the include directory.
Therefore a lot of files are dupplicates.

for instance:
include/expression/expression.cpp and src/expression/expression.cpp

I might be wrong, but I think you don't want .cpp files in your include dir

@nimish
Copy link
Author

nimish commented Feb 15, 2023

Looking at the diff, it looks like you copied .h and a lot of .cpp files in the include directory. Therefore a lot of files are dupplicates.

for instance: include/expression/expression.cpp and src/expression/expression.cpp

I might be wrong, but I think you don't want .cpp files in your include dir

yep, deleted them -- thanks!

@nimish nimish force-pushed the updated_cmake_docker branch 2 times, most recently from 1de94c2 to f4c6abd Compare February 15, 2023 19:00
@nimish
Copy link
Author

nimish commented Feb 15, 2023

@halbux @Drinausaur I think I fixed the excessive files issue: you can ignore everything in include/ since that's just a move.

@Drinausaur
Copy link

@halbux @Drinausaur I think I fixed the excessive files issue: you can ignore everything in include/ since that's just a move.

I don't think it is fixed. I'm pretty sure you're still having some duplicates:
image

In doubt, look at the diff GitHub offers. There should not be that many files with a "+", if you only moved some files.
Because right now, your changes add up to 14000 lines, I don't think some cmake changes should make that many changes.

PS: I am not a maintainer or anything on that repo, I only stumbled upon your MR and looked at it because I was interested ^^

@nimish nimish force-pushed the updated_cmake_docker branch 2 times, most recently from 6c0986b to 2f9a120 Compare February 15, 2023 23:23
@nimish
Copy link
Author

nimish commented Feb 15, 2023

OK I think this got fixed -- annoying issues with rsync. Everything in include is just the headers w/ dir structure respected.

@nimish nimish force-pushed the updated_cmake_docker branch from 128dc38 to d477f5f Compare February 16, 2023 00:22
@nimish nimish force-pushed the updated_cmake_docker branch from d477f5f to 2b5518c Compare February 16, 2023 00:41
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.

3 participants