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.

Sign up for GitHub

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

Purge RPATH from libs installed with julia #5743

Closed
wants to merge 1 commit into from
Closed

Purge RPATH from libs installed with julia #5743

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 2 additions & 1 deletion B/Binutils/build_tarballs.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ else
--host=\${MACHTYPE} \\
--with-sysroot="\${prefix}/$(compiler_target)/sys-root" \\
--enable-multilib \\
--disable-werror
--disable-werror \\
--enable-new-dtags
Copy link
Member

Choose a reason for hiding this comment

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

this is not supported by this version of binutils


make -j\${nproc}
make install
Expand Down
4 changes: 4 additions & 0 deletions C/CompilerSupportLibraries/common.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,10 @@ if [[ ${target} == *apple* ]]; then
done
fi

for lib in libgfortran libasan libtsan libubsan liblsan; do
[ ! -f "${libdir}/${lib}.so" ] || patchelf --set-rpath "\$ORIGIN" "${libdir}/${lib}.so"
done

Comment on lines +116 to +119
Copy link
Member

Choose a reason for hiding this comment

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

See #3703 (comment). I believe this can be fixed in the audit pass, instead of having to manually fix it here.

Copy link
Member

Choose a reason for hiding this comment

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

that would be somewhat simpler, but @staticfloat had thought it might fail in the verifier, so he suggested we do this manually

Copy link
Member

Choose a reason for hiding this comment

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

I replied in #3703 (comment), I think the assessment was ill-informed

# Remove extraneous libraries
rm -f ${libdir}/{libiconv,libxml2,libz}*.${dlext}*

Expand Down
8 changes: 8 additions & 0 deletions L/LLVM/common.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,14 @@ ninja -j${nproc} -vv
# Install!
ninja install

shopt -s nullglob
for x in ${prefix}/lib/*.so; do
# All things that have RPATH or doesn't have $ORIGIN,
# but not those without $RUNPATH and $ORIGIN
[ -n "$(objdump -x "$x" | grep 'RPATH\|RUNPATH')" ] && \
patchelf --set-rpath "\$ORIGIN" "$x";
done
Comment on lines +276 to +282
Copy link
Member

@giordano giordano Oct 25, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Also here, the fact that we have $ORIGIN/../lib as RPATH makes me think this should have always been $ORIGIN (library is in ${prefix}/lib, so $ORIGIN/../lib is just $ORIGIN). Maybe when we normalise the rpaths we should flatten relative paths, so that these identities become clear and we can simplify the paths we put there. However experimenting with LLVM is way less fun than with CSL, since building LLVM in the first place takes forever (at least with cold ccache) and audit also takes long.

Copy link
Member

Choose a reason for hiding this comment

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

the library may later get moved to lib/julia, resulting in breakage for people linking against it

that was fixed in Julia v1.7 (JuliaLang/julia#42788), but strictly only on that release branch (@vchuravy)


# Life is harsh on Windows and dynamic libraries are
# expected to live alongside the binaries. So we have
# to copy the *.dll from bin/ to tools/ as well...
Expand Down