CGAL / cgal

The public CGAL repository, see the README below
https://github.com/CGAL/cgal#readme
Other
4.97k stars 1.38k forks source link

CGALConfig.cmake does not correctly handle /lib -> /usr/lib symlink #8521

Open scgtrp opened 3 weeks ago

scgtrp commented 3 weeks ago

Issue Details

I'm on Arch, which does not use a separate /usr partition and replaces /lib with a symlink to /usr/lib.

I'm trying to compile OpenSCAD. It finds CGAL's cmake configuration at /lib/cmake/CGAL/CGALConfig.cmake. Then:

My CGAL installation otherwise seems fine; bodging it with

get_filename_component(CGAL_CONFIG_DIR "${CMAKE_CURRENT_LIST_FILE}" PATH)
set(CGAL_CONFIG_DIR "/usr${CGAL_CONFIG_DIR}")

is enough to allow the build to succeed.

Environment

mglisse commented 3 weeks ago

Any idea why cmake looks for the file in /lib/cmake and not the equivalent and more canonical /usr/lib/cmake? On a debian system with the same /lib -> /usr/lib symlink, I do get a path starting with /usr/lib. Maybe something in your environment, or a strange cmake config in arch? I am unsure what the implication would be of adding REALPATH to one of the get_filename_component.

scgtrp commented 3 weeks ago

That's an excellent question. I don't know cmake well enough to answer it for sure.

OpenSCAD CMakeLists.txt does: find_package(CGAL REQUIRED COMPONENTS Core)

Going down the list of environment variables here (ctrl-F "The set of installation prefixes is constructed using the following steps.")

($PATH is my shell $PATH; all others checked by inserting message("var=${var}") right above the findpackage call in the openscad CMakeLists.txt. I have no `$CMAKEor$CGAL_` set in my environment.)

I did see the "Path entries ending in /bin or /sbin are automatically converted to their parent directories" bit in the cmake manual. I tried reverting my bodge and doing PATH=/usr/bin cmake -B build but this still failed complaining about the one in /lib.

Happy to dig into other things if you have ideas for where to look.

mglisse commented 3 weeks ago

When you tried again using a PATH that has /usr/bin before (or instead of) /bin, was it with a clean directory, or did it have remnants of previous tries? I can indeed reproduce the issue if I make my PATH start with /bin. This looks like an unintended consequence of the links to /usr, I think it would make sense for cmake itself to deal with it (either upstream or in distribution-specific patches) by replacing / with /usr as a prefix candidate when /lib is a symlink to /usr/lib.

scgtrp commented 3 weeks ago

When you tried again using a PATH that has /usr/bin before (or instead of) /bin, was it with a clean directory, or did it have remnants of previous tries?

I thought I was in a clean directory, but perhaps not. I just tried PATH=/usr/bin cmake -B build again after removing build for real this time, and it worked. So yeah, looks like it's picking it up from $PATH.

lrineau commented 2 weeks ago

So, eventually, @scgtrp, do you still think that is a bug from CGAL?

scgtrp commented 2 weeks ago

That's kind of a philosophical categorization problem, ne?

I can see an argument where it's CGAL's problem (should be doing realpath before doing path manipulations), or cmake's problem (shouldn't do sketchy manipulations on $PATH to generate cmake search paths, or should at least add a hack for the common merged / + /usr case as @mglisse suggested), or even the distros' problem (should set CMAKE_PREFIX_PATHS somewhere, or should have just deleted /usr and moved its contents into / instead of setting up a situation where most, but not all, system files are accessible at two different locations).

As a developer, I'd just hack it with realpath on the CGAL side and be done with it while grumbling about the other things. If I knew cmake upstream to be receptive to such changes (I've never dealt with them myself), I'd suggest it to them in parallel, but otherwise probably wouldn't bother.

As an end user, I don't care, I just want the thing to work without making me debug cmake scripts.

lrineau commented 2 weeks ago

That seems fine. I think I agree with you, but I am busy on other things.

@scgtrp @mglisse Does any of you has a patch we can commit? Maybe a pull-request?

lrineau commented 6 days ago

After two weeks, I am still recovering from the rush.

@scgtrp @mglisse Does any of you has a patch we can commit? Maybe a pull-request?

Can you re-summarize the issue and the proposed solution?