conda-forge / conda-forge/r-rjava-feedstock

Linking undetected/possible undeclared dependencies

Open
#41 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Shell
Stars
0
Forks
10
PR merge metrics
No merged PRs in 30d

Description

Reposting this as an issue. Don't have time to work this right now, but if anyone else is so motivated, here's my assessment:

> I think this recipe needs more scrutiny. The link analysis doesn't find that `pcre2` is actually linked to in the final library. Moreover, there are many more library flags than this recipe is declaring:
>
> ```bash
> ## [NB: line breaks added for readability]
> x86_64-apple-darwin13.4.0-clang -o libjri.jnilib Rengine.o jri.o \
> Rcallbacks.o Rinit.o globals.o rjava.o -Wl,-dead_strip_dylibs \
> -Wl,-pie -Wl,-headerpad_max_install_names -Wl,-dead_strip_dylibs \
> -Wl,-rpath,$PREFIX/lib -L$PREFIX/lib -Wl,-pie -Wl,-headerpad_max_install_names \
> -Wl,-dead_strip_dylibs -Wl,-rpath,$PREFIX/lib -L$PREFIX/lib -L$PREFIX/lib/R/lib \
> -dynamiclib -dynamiclib -Wl,-headerpad_max_install_names \
> -undefined dynamic_lookup -single_module -multiply_defined suppress \
> -L$PREFIX/lib/jvm/lib/server -ljvm -fopenmp -Wl,-dead_strip_dylibs -Wl,-pie \
> -Wl,-headerpad_max_install_names -Wl,-dead_strip_dylibs -Wl,-rpath,$PREFIX/lib \
> -L$PREFIX/lib -L$PREFIX/lib/R/lib -lR -L$PREFIX/lib -lpcre2-8 -llzma -lbz2 -lz -ldl \
> -lm -liconv -licuuc -licui18n
> ```
>
> I suspect either the linking analysis is not looking at `libjri.jnilib` or `pcre2` really isn't needed. In the former case, we probably should be declaring several more dependencies; in the latter, this migration is moot.
>
> Either way, I'd like to see this answered before merging.

_Originally posted by @mfansler in https://github.com/conda-forge/r-rjava-feedstock/pull/38#pullrequestreview-1738747654_

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.