Feature request: function to replace ldd
Opened this issue · 13 comments
Please see probonopd/linuxdeployqt#24
Already have plans for it :)
referencing linuxdeploy/linuxdeploy#8
This is WIP in the background. @azubieta is writing an implementation right now, using https://github.com/lief-project/LIEF.
I would suggest to retain an ldd based method as well, though, in case our own version doesn't work on some platform (ldd is just way better tested, I'd say, and we know it works fine). Users of the library could simply choose which version to use then. We could make ElfFile an interface, and provide different implementations for it (maybe putting an abstract class in between, containing all the common functionality).
Suggested class hierarchy:
Wow. Feels like Software Engineering rather than cobbling something together now. 👍
The above representation of the ElfFile interface is responsible for:
- reading and writing data at elf files like run paths, architecture, "DT_NEEDED entries" and endianness
- building the whole dependency tree of a given elf file. which implies reading DT_NEEDED entries and translating them to file paths. This translation implies several rules described at the ldd manual page.
As those are two quite different responsibilities (read: "reason of change") therefore is a violation of the SRP. It's recommended to place it in a separated class, like ElfDependenciesResolver (tentative name).
There is no point on having an interface and an Abstract Base Class with almost the same public virtual methods.
Making the concrete implementations of the ElfFile class part of the public interface may not be a good idea as it will expand it and will force us to keep backward compatibility. As an alternative, we could use some kind of Object Factory.
There is no point on having an interface and an Abstract Base Class with almost the same public virtual methods.
In C++, there is nothing like interfaces anyway, this was just a UML-ish description of the idea (more like Java, I guess).
As those are two quite different responsibilities (read: "reason of change") therefore is a violation of the SRP. It's recommended to place it in a separated class, like ElfDependenciesResolver (tentative name).
So, your point is that a "dependency resolving" algorithm is a sort of "representation" which should not be generated by the class itself due to the SRP (as the underlying functionality might change over time)?
I guess I must've misunderstood you earlier. That might make sense. I'm not a fan of strictly following the SRP (it tends to generate lots of functors (a.k.a. objects with a single method only), and tend to put helper methods whose return value will always remain the same (here, a vector of paths) into the same class. Formally, you're right, the SRP might be violated.
The question is, does it matter to us? The output format won't ever change, I guess, it will always include all library dependencies (not excluding stuff from the excludelist etc.), and as long as we can guarantee that, we wouldn't run into issues.
If that isn't good enough for you, though, then let's add that ElfDependenciesResolver that decorates ElfFile.
Suggested class diagram:
Depending on e.g., user wishes, either implementation could be instantiated, and called through the abstract class's interface.
Clarification: in the above diagram, there is an extension relation between AbstractDependencyResolver and ElfFile. Is it intended to make AbstractDependencyResolver a kind of ElfFile?
Please re-read the last paragraph from the previous post, I missed a keyword on it, so I was meaningless. It's regarding if we should expose the concrete classes through the public API.
A decorator is one way of doing it (admittedly, the names are quite bad, should be e.g. AbstractDependencyResolvingElfFile etc.), providing a class whose methods and properties are 
ElfFile provides.
An alternative approach would be the strategy pattern, or the visitor pattern, the latter of which requires a method to be added to ElfFile accepting a visitor object that does all the work. The former allows for setting a property to the strategy that shall be used.
You implied to use a Servant, but they're generally discouraged, or maybe an Extension object, which is not as widely spread in programming, as it's not the cleanest approach.
Please check the new "interface draft" here. It's just a draft for discussion purposes, it doesn't even compile yet.
Two base classes are provided "elffile" and "elffiledependenciesresolver". To get instances of those classes the factory method pattern is used. This allow user to instantiated the "elffile" or "elffiledependenciesresolver" implementation they which without exposing the implementation details.
By the way we should use camel case or something else in the class names.
You seem to ignore that the class names are PascalCase (not camelCase), but the headers are all lowercase (sometimes with under_scores). That's a convention in the majority of C++ libraries. You always seem to do either option (both filename and class PascalCase, or all lowercase).
Please adhere to that convention developed in all other projects
If I don't provide you with a review until tomorrow, please ping me again.
PascalCase (not camelCase)
It seems we have read different books see CamelCase
That's a convention in the majority of C++ libraries
There is no such convention, I invite you to search a bit about it. Every project uses the naming style they like the most. I just found useless to have different file name and class name.
As each header file should contain only one class declaration. Using UpperCamelCase for C++ classes headers and snake_case for c headers helps do difference them at first glance.
- https://en.wikipedia.org/wiki/Camel_case
- https://docs.microsoft.com/en-us/previous-versions/dotnet/netframework-1.1/x2dbyw72(v=vs.71)
- http://wiki.c2.com/?CamelCase
- https://www.quora.com/What-is-the-difference-between-Pascal-Case-and-Camel-Case
One could say, PascalCase is a specialization of camelCase, requiring the first character to be upper case. camelCase is used mostly for lower-case-first-letter stuff.
Example:
class MyClass { // PascalCase
int my_var; // snake_case
void myFunction(); // camelCase
...There is no such convention
How would you judge that? Conventions aren't standards, if many projects use some pattern, you can call it a convention.
Examples using the convention involve boost, the C++ STL, etc., the most popular C++ libraries in the world. Also, AppImage software has always used lower case headers. No need to change that. And Qt uses lower-case for *.h headers as well, the #include <QSomething> stuff are aliases for the actual headers. You could as well say #include <qsomething.h>.
helps do difference them at first glance
As a user, I don't really care about the language, but the ABI. As all modern C headers can be used in C++ without having to wrap them with some ifdefs, they can be treated like C++ headers. I don't think that one should have to differentiate.