Opened 4 years ago

Closed 4 years ago

#60951 closed defect (fixed)

cgal4 @4.14.3_0: build failure due to zlib and conflict

Reported by: dstrubbe (David Strubbe) Owned by: Veence (Vincent)
Priority: Normal Milestone:
Component: ports Version:
Keywords: haspatch Cc: michaelld (Michael Dickens)
Port: cgal4

Description

I got two build failures for cgal4, which were related to finding the /usr/lib version of zlib rather than the macports one, and due to a conflict with the cgal port about a CMake file. Both are fixed in the attached patch.

Attachments (1)

Portfile-cgal4.diff (1.1 KB) - added by dstrubbe (David Strubbe) 4 years ago.

Download all attachments as: .zip

Change History (16)

Changed 4 years ago by dstrubbe (David Strubbe)

Attachment: Portfile-cgal4.diff added

comment:1 Changed 4 years ago by michaelld (Michael Dickens)

You should be able to specify just -DZLIB_ROOT=${prefix} for the configure args (or without the -D for configure environment), and CMake should then find the MP version, without all of the specifics. See for example https://trac.macports.org/ticket/60896 and https://trac.macports.org/ticket/60938 ...

Last edited 4 years ago by michaelld (Michael Dickens) (previous) (diff)

comment:2 Changed 4 years ago by michaelld (Michael Dickens)

That said, I'll take a look at the base CMake to see what changed. 3.17 worked in this regard; 3.18 isn't. Should be simple to fix one the change is tracked down ...

comment:3 Changed 4 years ago by michaelld (Michael Dickens)

Cc: michaelld added

comment:4 Changed 4 years ago by kencu (Ken)

this same issue keeps coming up with zlib and libiconv so far at least. Something must have changed in the way that cmake searches for packages.

comment:6 Changed 4 years ago by michaelld (Michael Dickens)

Hmmm ... I don't see how that commit changed it to remove the MP PREFIX ... but it might have. I can certainly look into tweaking the CMAKE_SYSTEM_PREFIX_PATH so that MP PREFIX comes as first as it can be, not knowing if CMake might tweak it after we set it ... of course.

comment:7 Changed 4 years ago by michaelld (Michael Dickens)

How about in Darwin.cmake, I append the following:

list(INSERT CMAKE_SYSTEM_PREFIX_PATH 0
  __PREFIX__ # MacPorts
)

This would make the MP prefix first in the search paths for the SYSTEM prefix path. I'll try this & see if it works, by removing the ZLIB_ROOT additions from the other commits, and of those now work again with those changes, then I think it's worth more testing.

comment:8 Changed 4 years ago by michaelld (Michael Dickens)

Yup ... looks like that does the trick ... I'll do a PR for it shortly ...

comment:9 Changed 4 years ago by michaelld (Michael Dickens)

In bc5236adc362f024aebd41ca198130bc40db49d0/macports-ports (master):

cmake: tweak patch to more appropriately set CMAKE_SYSTEM_PREFIX_PATH

==> prepend MacPorts' prefix instead of appending it

Should fix issues such as #60951 , #60938 , #60896 ; without having to set ZLIB_ROOT and other variables

comment:10 Changed 4 years ago by michaelld (Michael Dickens)

I bypassed the PR stage ... this change just makes so much sense & is small enough that I just pushed it directly. In my testing, it removes the need for the changes for the other issues. Please update the test this port to verify.

comment:11 Changed 4 years ago by dstrubbe (David Strubbe)

Ok. I think marking the conflict with the old cgal port is valuable too.

comment:12 in reply to:  11 Changed 4 years ago by michaelld (Michael Dickens)

Replying to dstrubbe:

Ok. I think marking the conflict with the old cgal port is valuable too.

@dstrubbe one issue at a time ...

Can someone confirm that my CMake tweak fixes this issue? It does for me in my testing. Best to have others test as well.

comment:13 Changed 4 years ago by Veence (Vincent)

Am I supposed to do anything? Or is the fix going to be implemented at the CMAKE level?

comment:14 Changed 4 years ago by michaelld (Michael Dickens)

If the current cgal4 builds correctly, then this issue can be closed IMHO, since it was "fixed" elsewhere. IIRC, I tested and found that it builds for me with the noted changes I made to the cmake port.

comment:15 Changed 4 years ago by Veence (Vincent)

Resolution: fixed
Status: assignedclosed

OK. I’m closing this. Please re-open if needed.

Note: See TracTickets for help on using tickets.