Closed spiderkeys closed 3 months ago
spiderkeys
commented
3 months ago By manually downloading and specifiying the runtime-fuse3-x86_64 file under the old release on the type2-runtime repo, I am able to successfully create and run the appimage:
/opt/mr/linuxdeploy/appimagetool-x86_64.AppImage --runtime-file ./runtime-fuse3-x86_64 ./squashfs-root/
# ...
Embedding ELF...
Marking the AppImage as executable...
Embedding MD5 digest
Success
stsp
commented
3 months ago Same problem here. Failure to d/l the runtime results in "success" with corrupted output.
stsp
commented
3 months ago By manually downloading and specifiying the runtime-fuse3-x86_64 file under the old release on the type2-runtime repo, I am able to successfully create and run the appimage:
I can additionally confirm that manually downloading the latest runtime, still gives the non-working result. Only literally following @spiderkeys suggestion (using old runtime) I was able to create a working appimage.
probonopd
commented
3 months ago @TheAssassin I think we need to add explicit HTTP status code handling; would this be the correct way?
I must say that having to create classes for curl ourselves is a big burden and errors and unhandled cases can slip in easily.
class GetRequest {
private:
// ... (rest of the class remains the same)
long _httpStatus;
public:
// ... (rest of the class remains the same)
long httpStatus() const {
return _httpStatus;
}
CurlResponse perform() {
auto result = curl_easy_perform(this->_handle);
_httpStatus = getOption<long>(CURLINFO_RESPONSE_CODE);
return {result == CURLE_OK, getOption<curl_off_t>(CURLINFO_CONTENT_LENGTH_DOWNLOAD_T), _buffer};
}
};
bool fetch_runtime(char *arch, size_t *size, char **buffer, bool verbose) {
// ... (rest of the function remains the same)
try {
GetRequest request(url, verbose);
auto response = request.perform();
if (response.statusCode() != 200) {
std::cerr << "Error: Received HTTP status code " << response.statusCode() << std::endl;
return false;
}
std::cerr << "Downloaded runtime binary of size " << response.contentLength() << std::endl;
// ... (rest of the function remains the same)
} catch (const CurlException& e) {
std::cerr << "libcurl error: " << e.what() << std::endl;
return false;
}
}
TheAssassin
commented
3 months ago I can't review this easily without seeing a diff. Adding a status code check is a good idea nevertheless.
I must say that having to create classes for curl ourselves is a big burden and errors and unhandled cases can slip in easily.
Using libcurl directly is extremely hard to get right. The classes help accomplishing that goal by introducing a memory safe layer in between that also greatly improves the API. Unfortunately, there are no good HTTP libraries for C++ and no proper C++ wrappers for libcurl other than cpr which you can't just install from Ubuntu or alike as easily. I think the amount of code is maintainable. Plus, it's idiomatic C++, so contributors could easily provide fixes if they understand C++.
probonopd
commented
3 months ago Thanks for implementing a proper solution to check the status code @TheAssassin
TheAssassin
commented
3 months ago It's pretty much what you suggested. By coincidence, actually. It turned out almost identical even though I didn't look at the code you provided. There were just a few things missing.
Using release:
I run into a situation where the download of the type2-runtime results in an 404, but AppImageTool doesn't seem to mind. What ends up happening is that instead of the runtime getting inserted at the beginning of the AppImage, the text "Not Found" is inserted (and thus the appimage isnt a valid ELF file:
This is the verbose output of the download step: