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

Static linking is too restrictive #102

Open
yshui opened this issue May 3, 2020 · 13 comments
Open

Static linking is too restrictive #102

yshui opened this issue May 3, 2020 · 13 comments

Comments

@yshui
Copy link

yshui commented May 3, 2020

pkg-config-rs/src/lib.rs

Lines 570 to 581 in ef356f3

fn is_static_available(name: &str, dirs: &[PathBuf]) -> bool {
let libname = format!("lib{}.a", name);
let system_roots = if cfg!(target_os = "macos") {
vec![Path::new("/Library"), Path::new("/System")]
} else {
vec![Path::new("/usr")]
};
dirs.iter().any(|dir| {
!system_roots.iter().any(|sys| dir.starts_with(sys)) && dir.join(&libname).exists()
})
}

Basically any library which falls into /usr/lib/ won't be statically linked (which is basically all libraries found by pkg-config). Why is this logic there?

@sdroege
Copy link
Collaborator

sdroege commented May 4, 2020

That seems to come from 9a57960 by @alexcrichton . I don't understand the rationale given in the commit message though. Maybe he can explain :)

@alexcrichton
Copy link
Member

Hm given how that was around 7 years ago, I don't really remember much about the context... 99% of the time commits like that were "this fixes a bug somewhere" and I didn't do a great job of documenting what bug was fixed where. I don't currently remember what bug was fixed either, so sorry but I don't think I can be of much help :(

@yshui
Copy link
Author

yshui commented May 4, 2020

Maybe we can remove it now?

@sdroege
Copy link
Collaborator

sdroege commented May 5, 2020

Possibly, but most likely it will break someone's setup :)

I'd like to do a pkg-config 0.5 release in the future that drops all these weird hacks that come without explanation and instead rely solely on the default pkg-config behavior (and if needed people can override that with the pkg-config environment variables, we don't have to set them ourselves, or with a wrapper script). Currently this crate is rather opinionated about the build environment, which causes a couple of problems.

@cjordan
Copy link

cjordan commented Oct 29, 2020

I've just come up against this issue. Is there something I could do to help accelerate the next big release of pkg-config?

cjordan added a commit to MWATelescope/mwalib that referenced this issue Nov 1, 2020
This allows callers to use whatever version of fitsio and fitsio-sys
that is used by mwalib, in turn ensuring that other dependent libraries
aren't using different versions of these crates. And, along with the
other change to this crate, means that statically-linking cfitsio from
other crates is simpler.

The `infer_static` function introduced to build.rs is a workaround for
pkg-config-rs being too restrictive when static
linking (rust-lang/pkg-config-rs#102).
Basically, if we decide to statically link, we emit a message to cargo,
and it'll work. Hopefully this hack can be removed in the future when
pkg-config-rs is a little more liberal.
cjordan added a commit to MWATelescope/mwalib that referenced this issue Nov 1, 2020
This allows callers to use whatever version of fitsio and fitsio-sys
that is used by mwalib, in turn ensuring that other dependent libraries
aren't using different versions of these crates. And, along with the
other change to this crate, means that statically-linking cfitsio from
other crates is simpler.

The `infer_static` function introduced to build.rs is a workaround for
pkg-config-rs being too restrictive when static
linking (rust-lang/pkg-config-rs#102).
Basically, if we decide to statically link, we emit a message to cargo,
and it'll work. Hopefully this hack can be removed in the future when
pkg-config-rs is a little more liberal.
cjordan added a commit to MWATelescope/mwa_hyperdrive that referenced this issue Nov 1, 2020
This commit breaks two crates out of hyperdrive: mwa_hyperdrive_core and
mwa_hyperdrive_srclist. Doing this means that core and srclist code can
be used elsewhere without all the dependencies of
hyperdrive (particularly CUDA). A tool "srclist" is also provided by the
srclist crate to handle source list verification and conversion.

Stop using nom to parse source lists. nom is nice, but complicated, and
hard to make good error messages around. The new approach is more error
prone, but is hopefully working well with a large number of tests.

Add support for Gaussian and Shapelet sources. Note that the existing
simulate-vis code won't use these sources correctly, but no one should
be using that code (it will be removed or updated at a later stage).

Add power law and curved power law source component types.

Add and/or update README files.

Provide GitHub workflows to run tests and provide a statically-compiled
srclist binary.

Attempt to make the module-level documentation style consistent across
all files.

With the new core crate, I've moved some code out of mongoose, so
mongoose can depend on core.

Static linking. I've discovered that the current version of
pkg-config-rs is too restrictive when attempting to statically
link (rust-lang/pkg-config-rs#102). Statically
linking elements of hyperdrive should now be more predictable. I've also
made mwalib export its usage of fitsio and fitsio-sys, so that these
don't need to be explicit dependencies of mwalib users, and statically
linking cfitsio is also more predictable.
@toxeus
Copy link

toxeus commented Jul 8, 2021

@sdroege wrote:

Possibly, but most likely it will break someone's setup :)

Maybe we can introduce another env var to disable the current behavior that refuses to statically link system libs? A quick and quite dirty workaround already exists by setting, e.g. SYSROOT=/asdf but it would be nicer to have something explicit rather than relying on such side-effects.

@otavio
Copy link

otavio commented Mar 20, 2022

@sdroege, any news in providing a way to get it linking to static libraries more easily?

@jirutka
Copy link

jirutka commented Apr 25, 2022

@alexcrichton, can you please remove this “feature” or at least allow us to disable it via an environment variable? It is clearly wrong and based on some weird assumption that doesn’t hold universally. The workaround with SYSROOT=/dummy cannot be used when cross-compiling.

@toxeus
Copy link

toxeus commented Apr 25, 2022

@sdroege established a plan on how to move forward but nothing happened since 2 years.

@teythoon
Copy link

This affects our Nettle bindings as well: https://gitlab.com/sequoia-pgp/nettle-sys/-/issues/16

@Wenzel
Copy link

Wenzel commented Feb 6, 2024

Any update on ticket ?
It's been almost 4 years now, the issue has been idenfified and the maintainer agreed for the removal of this chunk of code.

Anything blocking you @sdroege ?
I saw you made 2 releases recently, any chance you can remove is_static_available() and bump the version ?
It would unblock many people I think.

@sdroege
Copy link
Collaborator

sdroege commented Feb 6, 2024

It requires doing a breaking release, and as part of that it would be a good idea to go through everything in more detail and check if it makes sense to change other things. I didn't have time for that so far.

@teythoon
Copy link

teythoon commented Feb 6, 2024

Well, perfect is the enemy of good. The crate hasn't even reached version 1 yet, so maybe it is okay to cut a new minor release even if you don't fix other breaking changes with it.

dansgithubuser added a commit to dansgithubuser/rust-portaudio that referenced this issue Apr 25, 2024
can't use a modern static portaudio via pkg_config because of rust-lang/pkg-config-rs#102
wmedrano pushed a commit to RustAudio/rust-portaudio that referenced this issue May 5, 2024
* update portaudio

can't use a modern static portaudio via pkg_config because of rust-lang/pkg-config-rs#102

* allow passing extra args to portaudio configure

---------

Co-authored-by: Dan <daniel@foresightanalytics.ca>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants