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

stack: update to 2.13.1, enable arm64 #20722

Merged
merged 2 commits into from Nov 15, 2023
Merged

stack: update to 2.13.1, enable arm64 #20722

merged 2 commits into from Nov 15, 2023

Conversation

i0ntempest
Copy link
Member

Description

Type(s)
  • bugfix
  • enhancement
  • security fix
Tested on

macOS x.y
Xcode x.y / Command Line Tools x.y.z

Verification

Have you

  • followed our Commit Message Guidelines?
  • squashed and minimized your commits?
  • checked that there aren't other open pull requests for the same change?
  • referenced existing tickets on Trac with full URL?
  • checked your Portfile with port lint --nitpick?
  • tried existing tests with sudo port test?
  • tried a full install with sudo port -vst install?
  • tested basic functionality of all binary files?
  • checked that the Portfile's most important variants haven't been broken?

@macportsbot
Copy link

Notifying maintainers:
@essandess for port stack.

@macportsbot macportsbot added type: enhancement type: update maintainer: open Affects an openmaintainer port by: member Created by a member with commit rights labels Oct 5, 2023
@i0ntempest
Copy link
Member Author

Seems the libiconv problem still persists...

@pmetzger
Copy link
Member

pmetzger commented Nov 1, 2023

@i0ntempest Any way to make progress on this?

@i0ntempest
Copy link
Member Author

See #19317 and the upstream ticket in it. Until this is resolved I can’t move forward. I could conflicts_build libiconv but I don’t think that’s a good idea as so many ports depend on it.

@pmetzger
Copy link
Member

pmetzger commented Nov 1, 2023

Did you try @kencu's suggestion of overriding the PKGCONFIG search dirs?

@i0ntempest
Copy link
Member Author

I believe he's talking about overriding the path for the one component of the temporary compiler, not the whole port. I can't figure out how to do that unfortunately.

@kencu
Copy link
Contributor

kencu commented Nov 2, 2023

libiconv is a big pain in the keester, being loose there in ${prefix} and therefore being opportunistically found by every single build in macports.

I found a way to force /usr/lib/libiconv.dylib to be loaded ahead of the one in ${prefix} -- nothing to do with pkgconfig -- by using a link line sent to the compiler. I'd have to find the PR where I sorted this out, a ghc PR I think, last year or the year before, but it went something like this:

-Wl,/usr/lib/libiconv.dylib

but that was not exactly it I think.

@kencu
Copy link
Contributor

kencu commented Nov 2, 2023

oh, it was like this, for stack:

build.args-append --ghc-options -optl=-Wl,/usr/lib/libiconv.dylib

the thing about pkgconfig was trying to see what was bringing in the -L${prefix}/lib in the first place, and I though it was zlib through it's pkgconfig file:

% cat /opt/local/lib/pkgconfig/zlib.pc
prefix=/opt/local
exec_prefix=${prefix}
libdir=${exec_prefix}/lib
sharedlibdir=${libdir}
includedir=${prefix}/include

Name: zlib
Description: zlib compression library
Version: 1.3

Requires:
Libs: -L${libdir} -L${sharedlibdir} -lz
Cflags: -I${includedir}

@kencu
Copy link
Contributor

kencu commented Nov 2, 2023

well, that is not presently working:

ld: file cannot be open()ed, errno=2 path=/usr/lib/libiconv.dylib in '/usr/lib/libiconv.dylib'

I think I previously found a way beyond this before, but I can't find it just now...

@kencu
Copy link
Contributor

kencu commented Nov 2, 2023

This works to force the system one to be found ahead of the one in ${prefix}/lib:

% clang -L/usr/lib -L/opt/local/lib -liconv hello.c
% otool -L a.out
a.out:
	/usr/lib/libiconv.2.dylib (compatibility version 7.0.0, current version 7.0.0)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1336.0.0)

@kencu
Copy link
Contributor

kencu commented Nov 2, 2023

Ok, well here is a workaround to get it installed:

 % port -v installed stack
The following ports are currently installed:
  stack @2.13.1_0 (active) requested_variants='' platform='darwin 23' archs='arm64' date='2023-11-01T19:16:28-0700'
% otool -L /opt/local/bin/stack
/opt/local/bin/stack:
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1336.0.0)
	/usr/lib/libz.1.dylib (compatibility version 1.0.0, current version 1.2.12)
	/usr/lib/libiconv.2.dylib (compatibility version 7.0.0, current version 7.0.0)
	/usr/lib/libffi.dylib (compatibility version 1.0.0, current version 32.0.0)
	/System/Library/Frameworks/Cocoa.framework/Versions/A/Cocoa (compatibility version 1.0.0, current version 24.0.0)
	/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation (compatibility version 150.0.0, current version 2201.0.0)
	/System/Library/Frameworks/CoreServices.framework/Versions/A/CoreServices (compatibility version 1.0.0, current version 1226.0.0)
	/usr/lib/libcharset.1.dylib (compatibility version 1.0.0, current version 1.0.0)

I'm not going to brag about it, but it works around macports' libiconv issues.

  1. I added this to the Portfile to sterilize the environment, just in case:
# sterilize MacPorts build environment; we want nothing picked up from MP prefix
compiler.cpath
compiler.library_path
  1. then I edited the pkgconfig file for zlib to make it return /usr instead of /opt/local
% cat /opt/local/lib/pkgconfig/zlib.pc
prefix=/usr
exec_prefix=${prefix}
libdir=${exec_prefix}/lib
sharedlibdir=${libdir}
includedir=${prefix}/include

Name: zlib
Description: zlib compression library
Version: 1.3

Requires:
Libs: -L${libdir} -L${sharedlibdir} -lz
Cflags: -I${includedir}

You can't just remove pkgconfig or the pkgconfig zlib file as the build errors.

One way this could be done in the Portfile is to copy the zlib pkgconfig file somewhere, reinplace /opt/local with /usr and then direct macports' pkgconfig to look in that new directory first for pkgconfig files. It should find the altered one, and then the build would succeed.

Pretty? No it is not.

@kencu
Copy link
Contributor

kencu commented Nov 2, 2023

It is tidier to include the modified zlib.pc file in a folder inside the filesdir, and direct the build to use that.

This works (but might break some build with legacysupport, etc, on old systems):

% git diff head
diff --git a/lang/stack/Portfile b/lang/stack/Portfile
index f8abc3bf851..80d12404655 100644
--- a/lang/stack/Portfile
+++ b/lang/stack/Portfile
@@ -132,7 +132,16 @@ if { [variant_isset "prebuilt"] } {
                     ${workpath}/bin/${name}
     }
 
+    # sterilize MacPorts build environment; we want nothing picked up from MP prefix
+    compiler.cpath
+    compiler.library_path
+
     depends_build-append port:pkgconfig
 
+    # use a modified zlib.pc that forces the system paths to be used
+    # this prevents -L${prefix}/lib from being added to the library link path
+    # which causes libiconv linkage errors
+    build.env-append PKG_CONFIG_PATH=${filespath}/zlibsystempkgconfig
+
     set haskell_stack.bin ${workpath}/bin/stack
 }
diff --git a/lang/stack/files/zlibsystempkgconfig/zlib.pc b/lang/stack/files/zlibsystempkgconfig/zlib.pc
new file mode 100644
index 00000000000..49e07a49c78
--- /dev/null
+++ b/lang/stack/files/zlibsystempkgconfig/zlib.pc
@@ -0,0 +1,13 @@
+prefix=/usr
+exec_prefix=${prefix}
+libdir=${exec_prefix}/lib
+sharedlibdir=${libdir}
+includedir=${prefix}/include
+
+Name: zlib
+Description: zlib compression library
+Version: 1.3
+
+Requires:
+Libs: -L${libdir} -L${sharedlibdir} -lz
+Cflags: -I${includedir}

@pmetzger
Copy link
Member

pmetzger commented Nov 2, 2023

@kencu Thank you so much for helping with this! It's very much appreciated! What do you think the right next step should be?

@kencu
Copy link
Contributor

kencu commented Nov 2, 2023

Given the inelegance of this fix, nobody is going to be very happy about it...as far as I can see, solutions would appear to be:

  1. upstream might statically link libiconv.a into their ghc build on MacOS, and I believe that would prevent this issue in the first place.
  2. Macports could change the way they provide the libiconv port, so it is sequestered in a subdirectory and never opportunistically found
  3. Some genius could find a way to force /usr/lib/libiconv.dylib to be used even if ${prefix}/lib/libiconv.dylib is present and -L${prefix}/lib is first on the search path. I previously found a way to fudge this by linking both of the libiconv.dylibs into one executable, but that was very kludgy and Apple has disallowed that specifically with their new linker.
  4. the fix I found here, with the modified zlib.pc file

So whoever commits this zlib.pc fix, if it is committed, can expect a lot of questions about it, as it is very nonstandard.

@kencu
Copy link
Contributor

kencu commented Nov 2, 2023

Oh, and this zlib.pc sterilization fix might completely break the use of legacysupport... someone would need to test that.

@i0ntempest
Copy link
Member Author

From my understanding this is at its root a mismatch between the included iconv.h and the linked libiconv dylib. So apart from a bunch of ports depend on libiconv, is there anything preventing us from conflicts_build libiconv and build using headers from SDK and link system libiconv?

@kencu
Copy link
Contributor

kencu commented Nov 2, 2023

Many (nearly 1000) ports depend on libiconv. The CI fails here because at least one of them pulled in libiconv. So that might be hard in practice. pkgconfig, needed for the build, depends on libiconv, for example.

@kencu
Copy link
Contributor

kencu commented Nov 3, 2023

because macports' libiconv is free in the ${prefix}, it is
picked up opportunistically by the build of ghc.

macports version of libiconv is different than the system version
of libiconv. The bootstrap ghc compiler is built against the
system libiconv, and so requires linking against that version.

during the build of stack, pkg-config calls for the zlib.pc. In
macports, the zlib.pc file brings in flags that link against the
libraries in ${prefix}/lib, where libiconv is then found.

to get around this, bring in a modified zlib.pc that uses only
system paths. This forces the use of and link against the system
zlib, and prevents libiconv in the ${prefix} from being
opportunistically linked.
@kencu
Copy link
Contributor

kencu commented Nov 15, 2023

well, this builds for me, and currently seems to be our only path forward, so I'll push it to this branch and see if the CI system likes it as much as my system does.

@kencu
Copy link
Contributor

kencu commented Nov 15, 2023

worked on all three systems.... what do you think? Hopelessly broken otherwise, I believe.

@kencu kencu merged commit d0c0bd7 into macports:master Nov 15, 2023
3 checks passed
@kencu
Copy link
Contributor

kencu commented Nov 15, 2023

done. now I own the headache, it seems, at least fornow.

@i0ntempest
Copy link
Member Author

Thanks. Does seem to be the only way tho.

@i0ntempest i0ntempest deleted the stack branch November 22, 2023 03:03
@mouse07410
Copy link

IMHO, the fact that the decision to use a non-system libiconv was made 20 years ago - does not make that bad decision any better. It simply makes it an old bad decision.

Perhaps, after 20 years it's time for MacPorts to reconsider.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
by: member Created by a member with commit rights maintainer: open Affects an openmaintainer port type: enhancement type: update
5 participants