crystal-lang / distribution-scripts

40 stars 24 forks source link

Remove compilation of libcrystal.a #87

Closed maxfierke closed 3 years ago

maxfierke commented 3 years ago

With https://github.com/crystal-lang/crystal/pull/10463 being merged, libcrystal.a no longer exists

straight-shoota commented 3 years ago

There is also a mention of libcrystal.a in docker/alpine.Dockerfile left.

straight-shoota commented 3 years ago

Running maintenance_release workflow in https://app.circleci.com/pipelines/github/crystal-lang/crystal/5591/workflows/33c792de-0047-4e84-aa30-5c816d174c6e

straight-shoota commented 3 years ago

The maintenance build failed (https://app.circleci.com/pipelines/github/crystal-lang/crystal/5591/workflows/33c792de-0047-4e84-aa30-5c816d174c6e/jobs/59375).

The failure is caused by [ "$(ldd .build/crystal 2&> i | wc -l)" -eq "1" ]. It breaks because ldd .build/crystal doesn't print a line to stdout anymore. It just prints to stderr.

Somehow the removal of libcrystal.a has changed the behaviour of Alpine's ldd.

$ ldd /usr/lib/crystal/bin/crystal
        /lib/ld-musl-x86_64.so.1 (0x7f41d908b000)
$ ldd .build/crystal
/lib/ld-musl-x86_64.so.1: .build/crystal: Not a valid dynamic program
$ file /usr/lib/crystal/bin/crystal
/usr/lib/crystal/bin/crystal: ELF 64-bit LSB shared object, x86-64, version 1 (SYSV), statically linked, with debug_info, not stripped
$ file .build/crystal
.build/crystal: ELF 64-bit LSB executable, x86-64, version 1 (SYSV), statically linked, with debug_info, not stripped

I don't know why this change appears, but it seems we'll have to change the static executable check. Changing the expected line count to 0 would probably be fine. Although that would probably also succeed with any other kind of error. So maybe checking file build/crystal | grep statically linked would be better? file is an additional dependency, though.

On a similar though, it might also be a good idea to test executing the produced binary. At least crystal help should serve as a smoke test that it is indeed runnable.

bcardiff commented 3 years ago

@straight-shoota The issue something with #79. That is way 1.0.0 is built with that PR excluded as stated in https://github.com/crystal-lang/distribution-scripts/pull/83#issue-589836036

straight-shoota commented 3 years ago

Okay, so it's unrelated to this.

The CI failure linked from #83 shows exactly the same behaviour. But Not a valid dynamic program seems very explicit that it does not link some shared libraries. I presume it's just that the linker changed the output type. But that doesn't really matter. The only difference between LSB shared object and LSB executable seems to be that the former could be linked against as a dynamic library (https://askubuntu.com/questions/690631/executables-vs-shared-objects). That's irrelevant for our use case. The binaries are statically linked in both cases.

So I think we can just fix the static linking check (or configure the linker to build a shared object) on top of #79/master, and then rebase this PR.

straight-shoota commented 3 years ago

The maintenance build after #88 was merged succeeds: https://app.circleci.com/pipelines/github/crystal-lang/crystal/5600/workflows/fcf1003b-9115-41bf-a546-b2c2643ab3d1