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

enable static linking #76

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

enable static linking #76

wants to merge 1 commit into from

Conversation

faldez
Copy link

@faldez faldez commented Nov 11, 2021

pkg-config can't statically link libraries inside /usr/lib which usually where libraries are installed as stated here rust-lang/pkg-config-rs#102

This PR enable static linking via feature flags. To test extra dependencies is needed

Linux

sudo apt install libarchive-dev \
    libicu-dev \
    nettle-dev \
    libacl1-dev \
    liblzma-dev \
    libzstd-dev \
    liblz4-dev \
    libbz2-dev \
    zlib1g-dev \
    libxml2-dev

macOS

brew install icu4c libarchive bzip2 lz4 zlib expat
EXPORT PKG_CONFIG_PATH="/usr/local/opt/icu4c/lib/pkgconfig:/usr/local/opt/libarchive/lib/pkgconfig:/usr/local/opt/zlib/lib/pkgconfig:/usr/local/opt/expat/lib/pkgconfig"

@otavio
Copy link
Member

otavio commented Nov 11, 2021

@faldez awesome. Could you also add a CI job to validate this?

@faldez
Copy link
Author

faldez commented Nov 12, 2021

Should I add separate job for static and dynamic? Or I just install necessary packages to existing jobs?

@otavio
Copy link
Member

otavio commented Nov 12, 2021

I'd add it to the matrix so we ensure it works for all platforms.

@otavio
Copy link
Member

otavio commented Mar 19, 2022

@faldez could you take a look at this? it'd be awesome to have it being tested on CI.

@faldez
Copy link
Author

faldez commented Mar 19, 2022

I've added the deps for linux and macos, but I think I need to update rust cross image for aarch64 and armv7 tests

@otavio
Copy link
Member

otavio commented Mar 20, 2022

I'm trying to get #81 fixed before we extend the CI; it is currently failing on Windows.

@otavio
Copy link
Member

otavio commented Mar 20, 2022

Please rebase the PR.

@faldez faldez force-pushed the master branch 5 times, most recently from a02e56a to 0baa1cb Compare March 21, 2022 01:24
otavio added a commit that referenced this pull request Mar 21, 2022
This applies part of commit from PR #76 which is unrelated.

Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
@otavio
Copy link
Member

otavio commented Mar 21, 2022

I have committed the unrelated workflow changes and merged the Docker images. Thanks for all that.

I was reviewing the new code, and it is relatively complicated. So did you try the workaround in rust-lang/pkg-config-rs#102 (comment)? It'd be easier to understand if a good comment is added explaining the issue.

@faldez
Copy link
Author

faldez commented Mar 22, 2022

actually I haven't tried that, I think I did this before noticing that workaround. All I did is linking every library necessary manually instead of relying on pkg-config crates. I'll add comment to explains the steps.

though, it still failing on CI for nightly, because RUSTFLAGS override all flags setup in build.rs

is the workaround includes moving/symlink necessary paths to another directory and declare them as SYSROOT ?

@otavio
Copy link
Member

otavio commented Mar 22, 2022

Maybe, it's worth asking in the original issue about the workaround so we could use it here instead of adding all the linking of libraries manually?

@faldez
Copy link
Author

faldez commented May 6, 2022

I tried to get back to this, but someone state that workaround in rust-lang/pkg-config-rs#102 (comment) cannot be used when cross compiling rust-lang/pkg-config-rs#102 (comment). I think manually link all the necessary libraries is the only option for now.

@otavio
Copy link
Member

otavio commented May 28, 2023

@faldez please fix clippy issues, so we can try to get this merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants