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

Consider embedding libraries in source tree #735

Open
eteran opened this issue Dec 4, 2019 · 10 comments
Open

Consider embedding libraries in source tree #735

eteran opened this issue Dec 4, 2019 · 10 comments

Comments

@eteran
Copy link
Owner

eteran commented Dec 4, 2019

I was thinking, we basically have 3 uncommon dependencies.

  1. Capstone
  2. gdtoa-desktop
  3. libdouble-conversion

These are all relatively speaking, small. It may be reasonable to have a libs directory which contains copies of these projects which we would use if they are not installed system-wide, or perhaps always?

This lets us have a simpler dependency process, potentially lock in versions we are sensitive to, and make these dependencies unconditional, which is just simpler.

@10110111 Thoughts?


Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

@10110111
Copy link
Contributor

10110111 commented Dec 4, 2019

I wouldn't say libdouble-conversion is uncommon: it's used by Qt, at least in Debian, see libqt5core5a.
Capstone is also long packaged (and I don't actually install it from source on Ubuntu 18.04 — just libcapstone-dev).
The only package not common is gdtoa-desktop, since it's only recently appeared. But I guess once EDB has been released, and the distros start updating their packages, it will also get packaged.

So in summary, I don't think it's useful to bundle these libraries with EDB source, at least not all of them.

@eteran
Copy link
Owner Author

eteran commented Dec 4, 2019

Fair enough points. I agree about libdouble-conversion for certain; I had forgotten that Qt uses it.

Regarding Capstone, I'm kinda in the middle on it. Not because it isn't routinely packaged, but the version of it available from repos is a bit unreliable.

For example, I ran into errors doing the ARM32 build testing because one system had capstone 4 and another had capstone 3. If we had it baked in, we could just say "we use capstone 4 always".

But if I'm being honest, gdtoa-desktop really is the motivator for this idea. I've never had it installed, and I've never seen it available from any package managers. It's a little bit optimistic to say that edb will cause distros to start packaging it since it's a "soft" dependency. edb "works" just fine without it, they may not even notice it's an optional dep if they don't really read the CMake warnings.

So yea, I agree with your "not all of them" assessment.

@10110111
Copy link
Contributor

10110111 commented Dec 4, 2019

Well, gdtoa-desktop might indeed be a candidate for bundling. Especially if GCC finally implements std::to_chars for long double by the time EDB switches to C++17 or higher, in which case we could silently remove this dependency completely.

@eteran
Copy link
Owner Author

eteran commented Dec 4, 2019

Totally agree. So I may move forward with the gdtoa-desktop embedding sometime soon. I assume the copy in your repo is an appropriate source for it?

@10110111
Copy link
Contributor

10110111 commented Dec 4, 2019

Yeah, it's the original place to find it.

@eteran
Copy link
Owner Author

eteran commented Dec 15, 2020

@10110111 I am intending to revisit this ideas. Starting with gdtoa-desktop.

However, after more thinking, I am considering including things like capstone in the list of embedded libs too and possibly libdouble-conversion despite them being "widely available.

The main reason I am considering it, is that if we ever want to seriously target platforms like windows, it will make things simpler, significantly so for anyone who may want to do 3rd party contributions.

I have no idea if libdouble-conversion's headers let alone libraries are typically available on windows installs (I can say it's not in my C:\Qt\5.15.0\msvc2019_64\lib directory). And capstone certainly isn't typically installed.

If I did ever bundle these, I'd of course have it favor system available libs first if they are present.

Like I said, I'm just thinking about it, haven't made any decisions yet... but I do think it would improve the windows and macOS "build stories".

As always, I appreciate your imput.

@10110111
Copy link
Contributor

  1. We don't need libdouble-conversion on Windows: MSVC has implemented std::to_chars for floats and doubles, and by the time EDB is usable on Windows, we can rely on MSVC version being new enough to use these. We still need gdtoa-desktop because on MSVC long double has the same precision as double, which affects std::to_chars.
  2. That was for the case we actually want MSVC. If we use MinGW-W64 instead (with new enough GCC, which I suppose will be GCC 11, to be released), we'll get std::to_chars with long double support.

I have no idea if libdouble-conversion's headers let alone libraries are typically available on windows installs

Not sure of headers, but I can see it bundled with Qt5 sources. Twice, in fact:
qt5/qtbase/src/3rdparty/double-conversion and qt5/qtwebengine/src/3rdparty/chromium/third_party/icu/source/i18n/double-conversion-* :) Might be possible to somehow reach this, although I'm not really sure.

Conclusion: I don't think it's useful to bundle libdouble-conversion with EDB. Even if we still are on current GCC on Linux, the library is easily available. On Windows it's irrelevant always, and by the time macOS is supported by EDB, we'll have std::to_chars in GCC (and likely clang) too.

As for gdtoa-desktop and capstone, I'm OK with having them bundled.

@ped7g
Copy link

ped7g commented Dec 16, 2020

Usual pros:

  • versions are stable and you are not sensitive to platform package version
  • you are not sensitive to package being completely missing on platform, or having weird name

Usual cons:

  • generally frown-upon practice from distro maintainers, as any security fix to lib is not picked up by your app automatically

Considering the nature and functionality of edb-debugger, I'm not sure if the security of libs is of any meaningful practical concern. Any box which must run prod version of apps and remain as secure as possible probably should not have edb-dbg installed in the first place. And the risk of some developer running the debugger, being attacked from outside thanks to bug in the included library, while debugging some of their code, sounds very implausible.

But still you may make by this step the package look a bit more fishy to distro maintainers who dig deep into sources (and resulting binary package will be slightly larger, but with simpler dependencies ... does it even matter today, in the age of the snaps/flatpacks? It sometimes feels like I'm the last person on earth considering megabytes of data actually big chunk, and adding those libs is nowhere near "megabytes")

@10110111
Copy link
Contributor

Given this

If I did ever bundle these, I'd of course have it favor system available libs first if they are present.

the problem of larger binaries, as well as non-up-to-date dependencies is non-existent: the system libraries supposedly will be shared, so we'll get updated whenever corresponding package is updated.

@eteran
Copy link
Owner Author

eteran commented Dec 21, 2020

gdtoa-desktop is done! moving on to capstone, which unfortunately has an annoyingly archaic cmake script. I think I have a solution though.

Instead of importing it as a sub-module, we just literally add their sources at a given tag to the tree in a subdirectory of where we want it. For example:

lib/capstone/capstone-4.0.2/ this way, we can place our own CMakelists.txt in lib/capstone/. When we need to upgrade to a newer version of capstone, it'll be a slight annoyance, but I think it's likely manageable. Only really annoyance would be if they add/remove source files, which seems to be a rare occurance.

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

No branches or pull requests

3 participants