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

rubyPackages.iconv, v8: fix build with clang 16 #265307

Merged
merged 3 commits into from
Nov 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions pkgs/development/ruby-modules/gem-config/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,12 @@ in
};

iconv = attrs: {
dontBuild = false;
buildFlags = lib.optional stdenv.isDarwin "--with-iconv-dir=${libiconv}";
patches = [
# Fix incompatible function pointer conversion errors with clang 16
./iconv-fix-incompatible-function-pointer-conversions.patch
];
};

idn-ruby = attrs: {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
diff --git a/ext/iconv/iconv.c b/ext/iconv/iconv.c
index 2801049..77fae7e 100644
--- a/ext/iconv/iconv.c
+++ b/ext/iconv/iconv.c
@@ -188,7 +188,7 @@ static VALUE iconv_convert _((iconv_t cd, VALUE str, long start, long length, in
static VALUE iconv_s_allocate _((VALUE klass));
static VALUE iconv_initialize _((int argc, VALUE *argv, VALUE self));
static VALUE iconv_s_open _((int argc, VALUE *argv, VALUE self));
-static VALUE iconv_s_convert _((struct iconv_env_t* env));
+static VALUE iconv_s_convert _((VALUE env));
static VALUE iconv_s_iconv _((int argc, VALUE *argv, VALUE self));
static VALUE iconv_init_state _((VALUE cd));
static VALUE iconv_finish _((VALUE self));
@@ -204,7 +204,7 @@ static VALUE charset_map;
* Returns the map from canonical name to system dependent name.
*/
static VALUE
-charset_map_get(void)
+charset_map_get(VALUE klass)
{
return charset_map;
}
@@ -642,7 +642,7 @@ iconv_s_allocate(VALUE klass)
}

static VALUE
-get_iconv_opt_i(VALUE i, VALUE arg)
+get_iconv_opt_i(RB_BLOCK_CALL_FUNC_ARGLIST(i, arg))
{
VALUE name;
#if defined ICONV_SET_TRANSLITERATE || defined ICONV_SET_DISCARD_ILSEQ
@@ -784,8 +784,9 @@ iconv_s_open(int argc, VALUE *argv, VALUE self)
}

static VALUE
-iconv_s_convert(struct iconv_env_t* env)
+iconv_s_convert(VALUE env_value)
{
+ struct iconv_env_t* env = (struct iconv_env_t*)env_value;
VALUE last = 0;

for (; env->argc > 0; --env->argc, ++env->argv) {
@@ -906,7 +907,7 @@ list_iconv(unsigned int namescount, const char *const *names, void *data)

#if defined(HAVE_ICONVLIST) || defined(HAVE___ICONV_FREE_LIST)
static VALUE
-iconv_s_list(void)
+iconv_s_list(VALUE klass)
{
#ifdef HAVE_ICONVLIST
int state;
44 changes: 44 additions & 0 deletions pkgs/stdenv/adapters.nix
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,50 @@ rec {
stdenv.override (prev: { allowedRequisites = null; extraBuildInputs = (prev.extraBuildInputs or []) ++ pkgs; });


# Override the libc++ dynamic library used in the stdenv to use the one from the platform’s
# default stdenv. This allows building packages and linking dependencies with different
# compiler versions while still using the same libc++ implementation for compatibility.
#
# Note that this adapter still uses the headers from the new stdenv’s libc++. This is necessary
# because older compilers may not be able to parse the headers from the default stdenv’s libc++.
overrideLibcxx = stdenv:
assert stdenv.cc.libcxx != null;
let
llvmLibcxxVersion = lib.getVersion llvmLibcxx;
stdenvLibcxxVersion = lib.getVersion stdenvLibcxx;

stdenvLibcxx = pkgs.stdenv.cc.libcxx;
stdenvCxxabi = pkgs.stdenv.cc.libcxx.cxxabi;

llvmLibcxx = stdenv.cc.libcxx;
llvmCxxabi = stdenv.cc.libcxx.cxxabi;

libcxx = pkgs.runCommand "${stdenvLibcxx.name}-${llvmLibcxxVersion}" {
outputs = [ "out" "dev" ];
inherit cxxabi;
isLLVM = true;
} ''
mkdir -p "$dev/nix-support"
ln -s '${stdenvLibcxx}' "$out"
echo '${stdenvLibcxx}' > "$dev/nix-support/propagated-build-inputs"
ln -s '${lib.getDev llvmLibcxx}/include' "$dev/include"
'';

cxxabi = pkgs.runCommand "${stdenvCxxabi.name}-${llvmLibcxxVersion}" {
outputs = [ "out" "dev" ];
inherit (stdenvCxxabi) libName;
} ''
mkdir -p "$dev/nix-support"
ln -s '${stdenvCxxabi}' "$out"
echo '${stdenvCxxabi}' > "$dev/nix-support/propagated-build-inputs"
ln -s '${lib.getDev llvmCxxabi}/include' "$dev/include"
'';
in
overrideCC stdenv (stdenv.cc.override {
inherit libcxx;
extraPackages = [ cxxabi pkgs.pkgsTargetTarget."llvmPackages_${lib.versions.major llvmLibcxxVersion}".compiler-rt ];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does pkgs.pkgsTargetTarget work for you for any cross-cases? I tried pkgsLLVM.asciidoc-full and it fails the eval as:

nix build --no-link -f. pkgsLLVM.asciidoc-full
error:
       error: attribute 'llvmPackages_13' missing

       at pkgs/stdenv/adapters.nix:86:32:

           85|       inherit libcxx;
           86|       extraPackages = [ cxxabi pkgs.pkgsTargetTarget."llvmPackages_${lib.versions.major llvmLibcxxVersion}".compiler-rt ];
             |                                ^
           87|     });

It looks like pkgsTargetTarget is always empty for cross-compilers. On the contrast the following seems to work:

--- a/pkgs/stdenv/adapters.nix
+++ b/pkgs/stdenv/adapters.nix
@@ -83,7 +83,10 @@ rec {
     in
     overrideCC stdenv (stdenv.cc.override {
       inherit libcxx;
-      extraPackages = [ cxxabi pkgs.pkgsTargetTarget."llvmPackages_${lib.versions.major llvmLibcxxVersion}".compiler-rt ];
+      extraPackages = [
+        cxxabi
+        pkgs.buildPackages.targetPackages."llvmPackages_${lib.versions.major llvmLibcxxVersion}".compiler-rt
+      ];
     });

   # Override the setup script of stdenv.  Useful for testing new

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Proposed the change as #279463

});

# Override the setup script of stdenv. Useful for testing new
# versions of the setup script without causing a rebuild of
# everything.
Expand Down
11 changes: 10 additions & 1 deletion pkgs/top-level/all-packages.nix
Original file line number Diff line number Diff line change
Expand Up @@ -25465,7 +25465,16 @@ with pkgs;

ucommon = callPackage ../development/libraries/ucommon { };

v8 = darwin.apple_sdk_11_0.callPackage ../development/libraries/v8 { };
v8 = callPackage ../development/libraries/v8 (
let
stdenv' = if stdenv.cc.isClang && lib.versionAtLeast (lib.getVersion stdenv.cc.cc) "16"
then overrideLibcxx llvmPackages_15.stdenv
else stdenv;
in
{
stdenv = if stdenv'.isDarwin then overrideSDK stdenv' "11.0" else stdenv';
}
);

intel-vaapi-driver = callPackage ../development/libraries/intel-vaapi-driver { };

Expand Down