Skip to content

Update suitesparse's find module scripts - #1

Merged
stellaraccident merged 1 commit into
stellaraccident:fix_cholmod_include_pathfrom
jmachado-amd:fix_cholmod_include_path
Feb 28, 2025
Merged

stellaraccident merged 1 commit into
stellaraccident:fix_cholmod_include_pathfrom
jmachado-amd:fix_cholmod_include_path

Conversation

@jmachado-amd

Copy link
Copy Markdown

hipSOLVER's find module scripts (for suitesparse) were using find_path on name suitesparse/cholmod.h instead of cholmod.h, which allowed hipSOLVER to #include <suitesparse/cholmod.h>.

This commit updates the aforementioned cmake scripts, to honour the way Suitesparse::CHOLMOD advertises its include directories.

hipSOLVER's find module scripts (for suitesparse) were using `find_path`
on name `suitesparse/cholmod.h` instead of `cholmod.h`, which allowed
hipSOLVER to `#include <suitesparse/cholmod.h>`.

This commit updates the aforementioned cmake scripts, to honour the way
`Suitesparse::CHOLMOD` advertises its include directories.
@jmachado-amd

Copy link
Copy Markdown
Author

@stellaraccident, this works in my own reproducer.

Would you mind merging those changes if you are happy with them?

@stellaraccident

Copy link
Copy Markdown
Owner

Just checking because the project settings seem to allow any merge option. We do squash and merge here?

@jmachado-amd

Copy link
Copy Markdown
Author

In hipsolver itself we squash and merge, and I personally also tend to squash and merge any time I have the option. But since this is a PR on your PR, I think that you can go any way you prefer. :)

@stellaraccident
stellaraccident merged commit c27e02c into stellaraccident:fix_cholmod_include_path Feb 28, 2025
@stellaraccident

Copy link
Copy Markdown
Owner

In hipsolver itself we squash and merge, and I personally also tend to squash and merge any time I have the option. But since this is a PR on your PR, I think that you can go any way you prefer. :)

Ok. Most projects I've seen have strong feelings on that and even have the settings set to exclude other options. Letting the general public decide is usually a recipe for getting your history cluttered with "Fix typo again" style commits

cgmb pushed a commit that referenced this pull request Mar 5, 2025
hipSOLVER's find module scripts (for suitesparse) were using `find_path`
on name `suitesparse/cholmod.h` instead of `cholmod.h`, which allowed
hipSOLVER to `#include <suitesparse/cholmod.h>`.

This commit updates the aforementioned cmake scripts, to honour the way
`Suitesparse::CHOLMOD` advertises its include directories.
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.

2 participants