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

Add root CMakeLists.txt #415

Merged
merged 24 commits into from
Nov 3, 2023
Merged

Conversation

mmuetzel
Copy link
Contributor

@mmuetzel mmuetzel commented Sep 18, 2023

This is a rudimentary attempt to add a CMakeLists.txt to the root directory.

It uses the CMake flag SUITESPARSE_ENABLE_PROJECTS to be able to select which projects should be build. The default value "all" means that (almost) all projects in SuiteSparse are built.
That excludes CSparse which is never installed. Users would need to build CSparse as a standalone project. Users would need to specify that library explicitly in SUITESPARSE_ENABLE_PROJECTS.
Additionally, the CUDA projects (SuiteSparse_GPURuntime and GPUQREngine) are only built automatically if a working CUDA environment can be detected by CMake (or if the user specifies that they should be built anyway).

When selecting a project, all its dependencies are also added to the list of enabled projects.

A possible workflow could look like this:

# inside the source tree
mkdir -p build && cd build
cmake -DCMAKE_INSTALL_PREFIX=.. ..
cmake --build .

Or if, e.g., only CHOLMOD (and its dependencies) should be built:

mkdir -p build && cd build
cmake -DCMAKE_INSTALL_PREFIX=.. -DSUITESPARSE_ENABLE_PROJECTS="cholmod" ..
cmake --build .

That list can also contain multiple projects. E.g.:

mkdir -p build && cd build
cmake -DCMAKE_INSTALL_PREFIX=.. -DSUITESPARSE_ENABLE_PROJECTS="cholmod;cxsparse" ..
cmake --build .

I tried to keep the option to build each project stand-alone. That is needed to stay compatible with the root Makefile.

This fixes #175.

@DrTimothyAldenDavis
Copy link
Owner

Thanks. It might be a while before I can take a look (surgery coming up Wednesday).

Also, can you rebase it to the dev2 branch?

@mmuetzel mmuetzel changed the base branch from dev to dev2 September 18, 2023 16:29
@mmuetzel
Copy link
Contributor Author

Thanks. It might be a while before I can take a look (surgery coming up Wednesday).

This probably needs a bit more testing before it can be merged. It'll also probably conflict with some other open PRs (which are arguably still higher priority - e.g., #404, #411, #412, #413).
I'm happy to rebase once those are merged.

All in all no hurry with this PR.

Also, can you rebase it to the dev2 branch?

Sorry. Forgot to change it from the default setting...
Should be targeting the correct branch now.

@mmuetzel
Copy link
Contributor Author

The last couple of changes add support for building CSparse with the root CMakeLists.txt file. It isn't built by default though.
If users would like to build it, they'd need to configure, e.g., with -DSUITESPARSE_ENABLE_PROJECTS="csparse;all".

@mmuetzel
Copy link
Contributor Author

I'm not sure what's going on with the CI. For some reason it's building on my fork:
https://github.com/mmuetzel/SuiteSparse/actions/runs/6226675524/job/16899667636
But it's failing here with errors like the following:

  CMake Error at Mongoose/CMakeLists.txt:291 (target_link_libraries):
    Target "demo_exe" links to:
  
      SuiteSparse::SuiteSparseConfig
  
    but the target was not found.  Possible reasons include:
  
      * There is a typo in the target name.
      * A find_package call is missing for an IMPORTED target.
      * An ALIAS target is missing.

That line doesn't even contain a target_link_libraries command. Where does it get that file from?

I'm giving up for today. Maybe more luck tomorrow...

@mmuetzel
Copy link
Contributor Author

Ah. I wasn't aware the the CI runs on a merge commit. So, the ominous build errors disappeared after a rebase.

The remaining build error will likely disappear once #409 is fixed.

@mmuetzel
Copy link
Contributor Author

mmuetzel commented Nov 3, 2023

There might still be room for improvement. But this is probably good enough for a start.

Marking as ready for test.

@mmuetzel mmuetzel marked this pull request as ready for review November 3, 2023 17:18
@DrTimothyAldenDavis
Copy link
Owner

OK! I'll merge it in and give it a try.

@DrTimothyAldenDavis DrTimothyAldenDavis merged commit 68297c3 into DrTimothyAldenDavis:dev2 Nov 3, 2023
10 checks passed
@DrTimothyAldenDavis
Copy link
Owner

Works great for me -- thanks! One minor change: I revised SuiteSparse_config/Config/README.md.in, updating it with the latest top-level README.md (see the latest PR).

A few minor questions I'm sure you're aware of:

LAGraph and Mongoose both have ctests, and all of the packages have their demos. How do those get run from the top-level cmake? If the simple answer is "they don't", that's fine. They can be run by building the individual packages as before.

LAGraph places its compiled libraries in SuiteSparse/build, but for all other packages they go in SuiteSparse/PACKAGE/build. Is that because LAGraph needs to remain a stand-alone package in its own repo, at
https://github.com/GraphBLAS/LAGraph?

@mmuetzel
Copy link
Contributor Author

mmuetzel commented Nov 4, 2023

LAGraph and Mongoose both have ctests

The Mongoose ctests currently don't run correctly cross-platform. See #497 for a potential fix.
After that is fixed, we could add the testing facility (for these two libraries) to the root CMakeLists.txt file.

all of the packages have their demos. How do those get run from the top-level cmake?

It might make sense to convert (some of) the demos to ctests. That way they would be run the same way from the root CMakeLists.txt file (once the previous part is also done).
Running the demos alone does probably not add the most value if it is difficult to tell whether they ran correctly. They only tell if something is completely wrong and they don't build or don't run at all (or get stuck indefinitely). (ctests on the other hand have a simple to understand, binary result: pass or fail.)

LAGraph places its compiled libraries in SuiteSparse/build, but for all other packages they go in SuiteSparse/PACKAGE/build.

Oops. That was an oversight. There is no actual reason to treat it differently. See #496 for a change that removes that difference.

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.

Suggestion for root CMakeLists.txt
2 participants