MaterializeInc / rust-sasl

Cyrus SASL bindings for Rust
Apache License 2.0
4 stars 20 forks source link

Fix vendored sasl on lib64 systems #49

Closed JanZerebecki closed 4 months ago

JanZerebecki commented 1 year ago

as different systems use different names for these directories.

CLAassistant commented 1 year ago

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

benesch commented 1 year ago

Thanks for tracking this down! Do you know how OpenSUSE configures autotools to install into lib64 instead of lib? A config.site file, perhaps? I’d rather figure out how to disable that behavior than work around it.

JanZerebecki commented 1 year ago

Yes, removing the environment variable CONFIG_SITE might be a way. Shell profile scripts set the value of CONFIG_SITE to /usr/share/site/x86_64-pc-linux-gnu. This comes from the package site-config https://build.opensuse.org/package/show/openSUSE:Factory/site-config . The file contains:

#!/bin/sh
# Site script for configure. It is resourced via $CONFIG_SITE environment varaible.

# If user did not specify libdir, guess the correct target:
# Use lib64 for 64 bit bi-arch targets, keep the default for the rest.
if test "$libdir" = '${exec_prefix}/lib' ; then

        ac_config_site_64bit_host=NONE

        case "$host" in
        "" )
                # User did not specify host target.
                # The native platform x86_64 is a bi-arch platform.
                # Try to detect cross-compilation to inferior architecture.

                # We are trying to guess 32-bit target compilation. It's not as easy as
                # it sounds, as there is possible several intermediate combinations.
                ac_config_site_cross_to_32bit_host=NONE

                # User defined -m32 in CFLAGS or CXXFLAGS or CC or CXX:
                # (It's sufficient for 32-bit, but alone may cause mis-behavior of some checks.)
                case "$CFLAGS $CXXFLAGS $CC $CXX" in
                *-m32*)
                        ac_config_site_cross_to_32bit_host=YES
                        ;;
                esac

                # Running with linux32:
                # (Changes detected platform, but not the toolchain target.)
                case "`/bin/uname -i`" in
                x86_64 | ppc64 | s390x | aarch64 )
                        ;;
                * )
                        ac_config_site_cross_to_32bit_host=YES
                        ;;
                esac

                if test "x$ac_config_site_cross_to_32bit_host" = xNONE; then
                        ac_config_site_64bit_host=YES
                fi

                ;;
        *x86_64* | *ppc64* | *s390x* | *aarch64* )
                ac_config_site_64bit_host=YES
                ;;
        esac

        if test "x$ac_config_site_64bit_host" = xYES; then
                libdir='${exec_prefix}/lib64'
        fi
fi

# Continue with the standard behavior of configure defined in AC_SITE_LOAD:
if test "x$prefix" != xNONE; then
        ac_site_files="$prefix/share/config.site $prefix/etc/config.site"
else
        ac_site_files="$ac_default_prefix/share/config.site $ac_default_prefix/etc/config.site"
fi

for ac_site_file in $ac_site_files
do
        case $ac_site_file in #(
        */*) :
                 ;; #(
        *) :
                ac_site_file=./$ac_site_file ;;
esac
        if test -f "$ac_site_file" && test -r "$ac_site_file"; then
                { printf "%s\n" "/usr/share/site/x86_64-pc-linux-gnu:${as_lineno-$LINENO}: loading site script $ac_site_file" >&5
printf "%s\n" "/usr/share/site/x86_64-pc-linux-gnu: loading site script $ac_site_file" >&6;}
                sed 's/^/| /' "$ac_site_file" >&5
                . "$ac_site_file" \
                        || { { printf "%s\n" "/usr/share/site/x86_64-pc-linux-gnu:${as_lineno-$LINENO}: error: in \`$ac_pwd':" >&5
printf "%s\n" "/usr/share/site/x86_64-pc-linux-gnu: error: in \`$ac_pwd':" >&2;}
as_fn_error $? "failed to load site script $ac_site_file
See \`config.log' for more details" "$LINENO" 5; }
        fi
done
marioloko commented 1 year ago

It is possible to unset the CONFIG_SITE before calling cargo and it will work.

unset CONFIG_SITE
cargo build

The problem with this solution is that newer versions of linux distros contain this CONFIG_SITE, and if the developers decide to go with the vendored compilation, then, they will not be able to compile. In addition, they will not know how to solve the problem if they do not land in this issue, so I think that the solution should be implemented at crate level or at least give an error message.

Another option, to ensure that the configure still uses the right paths, is to remove the CONFIG_SITE environment when calling configure in build.rs:

    cmd(src_dir.join("configure"), &configure_args)
        .env_remove("CONFIG_SITE")
        .dir(&src_dir)
        .run()
        .expect("configure failed");

The problem with this solution is that it shall be added also to kbr5-src crate build.rs to compile.

        cmd(configure_path, &configure_args)
            .env_remove("CONFIG_SITE")
            .dir(&metadata.build_dir)
            .run()
            .expect("configure failed");
yuzhichang commented 1 year ago

With this PR change, the error becomes to:

error: could not find native static library `gssapi_krb5`, perhaps an -L flag is missing?

The following patch fix that new error:

diff --git a/sasl2-sys/build.rs b/sasl2-sys/build.rs
index 461b1c9..581a5e6 100644
--- a/sasl2-sys/build.rs
+++ b/sasl2-sys/build.rs
@@ -183,6 +183,12 @@ fn build_sasl(metadata: &Metadata) {
                 .join("lib")
                 .display(),
         );
+        println!(
+            "cargo:rustc-link-search=native={}",
+            PathBuf::from(env::var("DEP_KRB5_SRC_ROOT").unwrap())
+                .join("lib64")
+                .display(),
+        );
         println!("cargo:rustc-link-lib=static=gssapi_krb5");
         println!("cargo:rustc-link-lib=static=krb5");
         println!("cargo:rustc-link-lib=static=k5crypto");

My personal repo https://github.com/yuzhichang/rust-sasl, branch pr49 contains this PR change and my above patch. This fixes the vector build issue.

benesch commented 4 months ago

Another option, to ensure that the configure still uses the right paths, is to remove the CONFIG_SITE environment when calling configure in build.rs:

I like this solution.

The problem with this solution is that it shall be added also to kbr5-src crate build.rs to compile.

That seems okay. We can fix it there too.