Improve packaging for lsif-clang by bundling dynamic libraries #83

varungandhi-src · 2022-07-06T09:51:04Z

Based off #78.

(cherry picked from commit 4d1b5d9)

Test plan

Add CI step to build and test this image

We can add a publish step in a follow-up PR if needed.

(cherry picked from commit 4d1b5d9)

ubuntu_1804.Dockerfile

varungandhi-src · 2022-07-06T10:15:11Z

clang-tools-extra/lsif-clang/package/copy_dependencies.sh

+for lib in $(ldd "$binary" | cut -d ">" -f 2 | awk '{print $1}' | grep -vE "(linux-vdso\.|ld-linux-x86-64\.|libc\.)"); do
+    cp --verbose "$lib" "$output/"
+done


We should double-check if the new libdwarf-dev and libelf-dev dependencies require updating this.

This by takes every linked shared object except for the linux kernel vdso, dynamic linker and libc. Those are best kept left with the system or not copyable. So as long as all dependencies are linked, and not loaded with dlopen they will be included here.

It would be a good idea to update the docs here to explain this.

(I would also rename this copy_dependencies.sh to copy_needed_libraries.sh)

Looks like this needed an update for librt and libpthread; copying them across OSes lead to errors, but removing them fixes the issue. See the commit message of be6a3a1 for details.

But it works fine otherwise.

ubuntu_1804.Dockerfile

varungandhi-src · 2022-07-06T12:44:41Z

When running the installation script

./clang-tools-extra/lsif-clang/package/build.sh

inside an Ubuntu 20.04 VM, I get an error with missing packages

WARNING: apt does not have a stable CLI interface. Use with caution in scripts.

Reading package lists...
Building dependency tree...
Reading state information...
E: Unable to locate package llvm-11
E: Unable to locate package clang-11
E: Unable to locate package libclang-11-dev
The command '/bin/sh -c apt install -y llvm-11 clang-11 libclang-11-dev cmake' returned a non-zero code: 100

This happens both with the original commit and after the refactoring a392f4d.

I wonder if this is aarch64 specific, where the older OSes are missing aarch64 packages. I can try reproducing on x86_64, but it seems like 22.04 avoids this.

Looks like this was missed during cherry-picking. Co-authored-by: Joseph Lisee <jlisee@aurora.tech>

varungandhi-src · 2022-07-06T13:58:50Z

Hmm, this doesn't seem to work for me across Ubuntu versions, because of a glibc version mismatch. Maybe because of the upgrade to Ubuntu 22.04 for the image?

$ /path/to/shim/lsif-clang compile_commands.json
<snipped>
lsif-clang: /lib/aarch64-linux-gnu/libc.so.6: version `GLIBC_2.32' not found (required by /home/parallels/Code/lsif-clang/build/bundled/lsif-clang-dde883593/libLLVM-11.so.1)
lsif-clang: /lib/aarch64-linux-gnu/libc.so.6: version `GLIBC_2.35' not found (required by /home/parallels/Code/lsif-clang/build/bundled/lsif-clang-dde883593/libgcc_s.so.1)
lsif-clang: /lib/aarch64-linux-gnu/libc.so.6: version `GLIBC_2.34' not found (required by /home/parallels/Code/lsif-clang/build/bundled/lsif-clang-dde883593/libgcc_s.so.1)
lsif-clang: /lib/aarch64-linux-gnu/libc.so.6: version `GLIBC_2.33' not found (required by /home/parallels/Code/lsif-clang/build/bundled/lsif-clang-dde883593/libedit.so.2)

jlisee

glibc has a high degree of backwards compatibility so the if we build on Ubuntu 18.04 and bring all the other libraries we should be able to run on a large number of linux systems of that age and newer. This should allow using lsif-clang in most environments that people are building C++.

To share code between Dockerfile's this is what I would do:

Create a clang-tools-extra/lsif-clang/install_dependencies.sh which uses lsb_release to be be portable and does the cmake, llvm and clang apt based installs
Create a clang-tools-extra/lsif-clang/configure.sh which does the CC=... CXX=... cmake steps

jlisee · 2022-07-06T12:21:37Z

clang-tools-extra/lsif-clang/package/copy_dependencies.sh

+for lib in $(ldd "$binary" | cut -d ">" -f 2 | awk '{print $1}' | grep -vE "(linux-vdso\.|ld-linux-x86-64\.|libc\.)"); do
+    cp --verbose "$lib" "$output/"
+done


This by takes every linked shared object except for the linux kernel vdso, dynamic linker and libc. Those are best kept left with the system or not copyable. So as long as all dependencies are linked, and not loaded with dlopen they will be included here.

It would be a good idea to update the docs here to explain this.

(I would also rename this copy_dependencies.sh to copy_needed_libraries.sh)

jlisee · 2022-07-06T14:03:03Z

Bundled.Dockerfile

+#
+# See issue https://github.com/sourcegraph/lsif-clang/issues/72
+
+FROM ubuntu:22.04 as build


See my note, building on such a new version of Ubuntu greatly restricts the platforms that can run the resulting release.

Yep, that makes sense. I'm testing the PR with that reverted right now on an x86_64 machine.

jlisee · 2022-07-06T14:06:27Z

Hmm, this doesn't seem to work for me across Ubuntu versions, because of a glibc version mismatch. Maybe because of the upgrade to Ubuntu 22.04 for the image?

Yes sorry I didn't replay last night, but the entire point of using 18.04 is that you can still build new software on it but the resulting binaries are widely portable.

varungandhi-src · 2022-07-06T14:10:26Z

No worries about the response, I think you're in a US timezone, whereas I am in Taipei right now. I'd like to double-check the change manually once before merging. On aarch64, it doesn't work, so I'm going to test with x86_64.

As for debugging, I've been working on updating the Python driver script so that it can perform index merging after-the-fact, gracefully handling OOMs since it is multi-process instead of multi-threaded. I'm not 100% sure if it'll work (without blowing up the index size) but just giving you a heads up to avoid duplicate work.

This reverts commit dde8835.

They link in private symbols from Glibc, leading to linker errors when trying to use lsif-clang across Ubuntu versions. Error with libpthread: lsif-clang: symbol lookup error: /home/varun/lsif-clang/build/bundled/lsif-clang-34e509b99/libpthread.so.0: undefined symbol: __libc_vfork, version GLIBC_PRIVATE Error with librt: lsif-clang: symbol lookup error: /home/varun/lsif-clang/build/bundled/lsif-clang-34e509b99/librt.so.1: undefined symbol: __clock_nanosleep, version GLIBC_PRIVATE

It messes up the lookup for the compile_commands.json file.

Using fewer RUN steps should lead to smaller images. Also use fewer apt-get update calls.

varungandhi-src · 2022-07-06T15:14:32Z

I've reverted the bump to 22.04 for the Dockerfile. There were also some other bugs that I noticed:

The cd in the shim bash script seemed to lead to incorrect paths for lsif-clang, so it would look in the wrong place for the compile_commands.json and fail to find it (not 100% sure if my "refactoring" broke this or if the cd earlier was also incorrect). So I've removed that.
I noticed linker errors when copying librt and libpthread across Ubuntu versions (from 18.04 to 20.04), so I've removed those.

Thank you for submitting the original PR with this approach. I was investigating rewriting the CMake in the style of upstream Clang-based tools, which are often statically linked in a busybox-style binary. However, that would've likely been much more time-consuming to implement.

I hadn't considered this approach. Sure, it feels hackier, but if it gets the job done for you, I don't have complaints. :)

varungandhi-src · 2022-07-06T15:31:34Z

The COPY . . step is taking very long, likely because of the large number of files in the repo, most of which are probably useless for us. 😐 I'm going to leave it in for now, as well as the bit of duplication between the Dockerfiles (since it is not all that much + there is some divergence due to different OS versions).

Feel free to leave post-review comments and I'll address them tomorrow; I'm merging this for now.

jlisee

I was able to successfully to run this locally on my x64, Ubuntu 18.04 machine.

Bundled_Ubuntu1804.Dockerfile

Copy all dependencies into self contained section

19bf791

(cherry picked from commit 4d1b5d9)

varungandhi-src mentioned this pull request Jul 6, 2022

Add a self contained Ubuntu 18.04 based linux build #78

Closed

varungandhi-src added 2 commits July 6, 2022 17:55

docker: Remove mkdir/cd calls with cmake -B.

75801d8

build: Clean up build script a bit + use UPPER_CASE

a392f4d

varungandhi-src changed the title ~~Copy all dependencies into self contained section~~ Improve packaging for lsif-clang by bundling dynamic libraries Jul 6, 2022

varungandhi-src commented Jul 6, 2022

View reviewed changes

ubuntu_1804.Dockerfile Outdated Show resolved Hide resolved

varungandhi-src commented Jul 6, 2022

View reviewed changes

ubuntu_1804.Dockerfile Outdated Show resolved Hide resolved

varungandhi-src and others added 5 commits July 6, 2022 20:54

Add missing lsif_clang_shim.sh

89ab5e2

Looks like this was missed during cherry-picking. Co-authored-by: Joseph Lisee <jlisee@aurora.tech>

ci: Add check that new Dockerfile builds.

8a259bc

Fix missing executable bit on lsif_clang_shim.sh

04782f1

build: Put artifacts under build/ and optionally copy them out.

3e7c180

build: Simplify build with Ubuntu 22.04.

dde8835

jlisee reviewed Jul 6, 2022

View reviewed changes

varungandhi-src added 6 commits July 6, 2022 14:17

Revert "build: Simplify build with Ubuntu 22.04."

34e509b

This reverts commit dde8835.

packaging: Rename shell script.

b84064d

shim: Don't change working directory.

4788bad

It messes up the lookup for the compile_commands.json file.

docker: Document bundling image + clean-up.

d5dac42

Using fewer RUN steps should lead to smaller images. Also use fewer apt-get update calls.

Rename Dockerfile to reflect usage.

897aad4

packaging: Document library copying.

db1926e

varungandhi-src merged commit 8afb061 into main Jul 6, 2022

varungandhi-src deleted the vg/better-packaging branch July 6, 2022 15:32

varungandhi-src mentioned this pull request Jul 6, 2022

Provide pre-built binaries for Linux #72

Closed

jlisee approved these changes Jul 6, 2022

View reviewed changes

Bundled_Ubuntu1804.Dockerfile Show resolved Hide resolved

Improve packaging for lsif-clang by bundling dynamic libraries #83

Improve packaging for lsif-clang by bundling dynamic libraries #83

Uh oh!

Conversation

varungandhi-src commented Jul 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test plan

Uh oh!

Uh oh!

varungandhi-src Jul 6, 2022

Choose a reason for hiding this comment

Uh oh!

jlisee Jul 6, 2022

Choose a reason for hiding this comment

Uh oh!

varungandhi-src Jul 6, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

varungandhi-src commented Jul 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

varungandhi-src commented Jul 6, 2022

Uh oh!

jlisee left a comment

Choose a reason for hiding this comment

Uh oh!

jlisee Jul 6, 2022

Choose a reason for hiding this comment

Uh oh!

jlisee Jul 6, 2022

Choose a reason for hiding this comment

Uh oh!

varungandhi-src Jul 6, 2022

Choose a reason for hiding this comment

Uh oh!

jlisee commented Jul 6, 2022

Uh oh!

varungandhi-src commented Jul 6, 2022

Uh oh!

varungandhi-src commented Jul 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

varungandhi-src commented Jul 6, 2022

Uh oh!

jlisee left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

varungandhi-src commented Jul 6, 2022 •

edited

Loading

varungandhi-src commented Jul 6, 2022 •

edited

Loading

varungandhi-src commented Jul 6, 2022 •

edited

Loading