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

Unbreak all the things ;o) #10687

Merged
merged 1 commit into from
Jul 12, 2023
Merged

Unbreak all the things ;o) #10687

merged 1 commit into from
Jul 12, 2023

Conversation

NiLuJe
Copy link
Member

@NiLuJe NiLuJe commented Jul 12, 2023

koreader/koreader-base#1634


This change is Reviewable

@Frenzie Frenzie added this to the 2023.07 milestone Jul 12, 2023
@Frenzie Frenzie added the bug label Jul 12, 2023
@Frenzie
Copy link
Member

Frenzie commented Jul 12, 2023

I'm surprised the CI here didn't balk at the lack of that library. We don't explicitly test WebP but we do test MuPDF. And it's not like we use WebP at all… do we?

@NiLuJe
Copy link
Member Author

NiLuJe commented Jul 12, 2023

Nope, we don't, not actively.

It failed dependency resolving when loading CRe because it has a DT_NEEDED on the webp stuff; not quite sure why that doesn't fail on CIs.

(Probably the same reason it didn't fail on the emu: it looks up the system's instead?).

@Frenzie
Copy link
Member

Frenzie commented Jul 12, 2023

It failed dependency resolving when loading CRe because it has a DT_NEEDED on the webp stuff; not quite sure why that doesn't fail on CIs.

Hm, that's still surprising. I start on history, but perhaps some cover wasn't generated yet. In any case crengine is tested too, same difference.

(Probably the same reason it didn't fail on the emu: it looks up the system's instead?).

On my system, sure, whatever, but not on the Docker image.

@Frenzie
Copy link
Member

Frenzie commented Jul 12, 2023

On my system, sure, whatever, but not on the Docker image.

I take that back actually. My system has 0.6.1 and 1.2.4. Not 1.3.x.

/usr/lib/x86_64-linux-gnu/libwebp.so.7
/usr/lib/x86_64-linux-gnu/libwebp.so.7.1.5

@Frenzie
Copy link
Member

Frenzie commented Jul 12, 2023

D'oh, wrong system. I have three I regularly use. Ignore the above. ;-)

@Frenzie Frenzie merged commit 4223526 into koreader:master Jul 12, 2023
3 checks passed
@Frenzie
Copy link
Member

Frenzie commented Jul 12, 2023

Hm, no problem on this system either though. If it's getting libwebp itself from the system that might be slightly bothersome.

@NiLuJe
Copy link
Member Author

NiLuJe commented Jul 12, 2023

I don't recall how/if we set an LD_LIBRARY_PATH for the CI runs (and/or how the DT_RPATH are set in the binaries), but, yeah, that might be a possibility.

@Frenzie
Copy link
Member

Frenzie commented Jul 12, 2023

But I repeat, the CI doesn't have a libwebp other than the one it just built.

@NiLuJe
Copy link
Member Author

NiLuJe commented Jul 12, 2023

Oh.

New idea, then: Shitty libtool build-time rpaths making it to the final binaries, meaning they'll go looking in the build directory for the deps (and, well, find it since we don't clean, I think)?

@Frenzie
Copy link
Member

Frenzie commented Jul 12, 2023

That sounds more plausible, still weird & unexpected though.

@yparitcher
Copy link
Member

You might need to update the mac ci script. See f588edd

@NiLuJe
Copy link
Member Author

NiLuJe commented Jul 13, 2023

That sounds more plausible, still weird & unexpected though.

Yup!

  75   │ Dynamic section at offset 0x34ef8 contains 29 entries:
  76   │   Tag        Type                         Name/Value
  77   │  0x00000001 (NEEDED)                     Shared library: [libsharpyuv.so.0]
  78   │  0x00000001 (NEEDED)                     Shared library: [libm.so.6]
  79   │  0x00000001 (NEEDED)                     Shared library: [libpthread.so.0]
  80   │  0x00000001 (NEEDED)                     Shared library: [libc.so.6]
  81   │  0x0000000e (SONAME)                     Library soname: [libwebp.so.7]
  82   │  0x0000000f (RPATH)                      Library rpath: [/builds/koreader/nightly-builds/koreader/base/thirdparty/libwebp/build/arm-kobo-linux-gnueabihf/libwebp-prefix/src/libwebp-build/lib]

That reminds me that we probably need a fancy $ORIGIN rpath in there to fool-proof this...

NiLuJe added a commit to NiLuJe/koreader that referenced this pull request Jul 13, 2023
NiLuJe added a commit that referenced this pull request Jul 13, 2023
@Frenzie
Copy link
Member

Frenzie commented Jul 13, 2023

I'm not sure if I should do anything to any Docker images this month (or at least this week) so as not to interfere with the big Android upgrade, but could there also be a chance it's an issue fixed in newer libtool? I had intended to do some more work on 18.04 → 20.04 in July 2023 since 18.04 is officially EOL now.

@NiLuJe
Copy link
Member Author

NiLuJe commented Jul 13, 2023

Oh, no, it's been like this forever, libtool is the worst ;o).

One very quick way to confirm that is to use the amazing slibtool to build webp instead:

 130   │ Dynamic section at offset 0xb4d60 contains 28 entries:
 131   │   Tag        Type                         Name/Value
 132   │  0x0000000000000001 (NEEDED)             Shared library: [libm.so.6]
 133   │  0x0000000000000001 (NEEDED)             Shared library: [libc.so.6]
 134   │  0x000000000000000e (SONAME)             Library soname: [libwebp.so.7]
 135   │  0x000000000000000c (INIT)               0x3000

vs.

 130   │ Dynamic section at offset 0xaed40 contains 30 entries:
 131   │   Tag        Type                         Name/Value
 132   │  0x0000000000000001 (NEEDED)             Shared library: [libsharpyuv.so.0]
 133   │  0x0000000000000001 (NEEDED)             Shared library: [libm.so.6]
 134   │  0x0000000000000001 (NEEDED)             Shared library: [libc.so.6]
 135   │  0x000000000000000e (SONAME)             Library soname: [libwebp.so.7]
 136   │  0x000000000000001d (RUNPATH)            Library runpath: [/var/tmp/niluje/libwebp/sharpyuv/.libs]
 137   │  0x000000000000000c (INIT)               0x3000

I'm not terribly inclined to hack around this now, given that this will be a non-issue with meson ;).

@NiLuJe
Copy link
Member Author

NiLuJe commented Jul 13, 2023

(The missing DT_NEEDED on libsharpyuv is perhaps a prime example of the fact that slibtool isn't a magical cure-all, though ;p)

@Frenzie
Copy link
Member

Frenzie commented Jul 13, 2023

Oh, no, it's been like this forever, libtool is the worst ;o).

Did you mean "it's still like this"? That it's been that way forever doesn't really address what I said. ;-P

@NiLuJe
Copy link
Member Author

NiLuJe commented Jul 13, 2023

Supposedly, things have gotten better since 1.5.2 (c.f., https://wiki.debian.org/RpathIssue), but that's pretty damn old by now, and I've always seen libtool do this, except when drastic measures are taken (e.g., Portage patches libtool at runtime before each libtoolize run to murder that insanity, among other things).

So, no, I don't think you have anything to worry about it, libtool's just being libtool ;).

(FWIW, In this specific case, I suspect the buildsystem is explicitly asking for that, for some... mysterious reason).

@NiLuJe
Copy link
Member Author

NiLuJe commented Jul 14, 2023

(The missing DT_NEEDED on libsharpyuv is perhaps a prime example of the fact that slibtool isn't a magical cure-all, though ;p)

Correction: I must have been doing it wrong that time, I'm getting proper DT_NEEDED entries with slibtool now (with LIBTOOL=rlibtool at least, which isn't how I originally tested).

And its output is actually useful! All hail slibtool.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants