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.

Sign up for GitHub

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

Jump to bottom

add zstd compression #158

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

add zstd compression #158

wants to merge 3 commits into from

Conversation

hchunhui
Copy link

Fixes AppImage/AppImageKit#478

I have add zstd compression for AppImageKit. The final result is at https://github.com/hchunhui/AppImageKit/releases . Now we can run appimagetool with --comp zstd to use zstd compression.

This PR updates libappimage. If it can be merged, I will submit another PR to AppImageKit. It consists of 3 commits:

  • I'm not able to compile AppImageKit with the latest libappimage, so the first commit fixes the problem by restoring appimage_get_elf_size previously implemented in libappimage_shared.
  • The second commit compiles zstd and enables zstd support for squashfuse.
  • The third commit fixes a CI error when built on armhf and aarch64 ( "narrowing conversion inside { }", please see https://github.com/hchunhui/AppImageKit/runs/3927309532 ).

azubieta
azubieta suggested changes Oct 21, 2021
View reviewed changes
Copy link
Contributor

@azubieta azubieta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see major issues on merging this but you need to limit the changes in files to the PR intention.

include/appimage/appimage_legacy.h Show resolved Hide resolved
src/libappimage/libappimage_legacy.cpp
@@ -2,10 +2,6 @@
#include <appimage/appimage.h>

extern "C" {
ssize_t appimage_get_elf_size(const char* fname) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Limit the changes in files to the PR intention.

src/libappimage_shared/elf.c
@@ -117,6 +117,41 @@ static off_t read_elf64(FILE* fd)
return sht_end > last_section_end ? sht_end : last_section_end;
}

ssize_t appimage_get_elf_size(const char* fname) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Limit the changes in files to the PR intention.

@azubieta
Copy link
Contributor

I'm not able to compile AppImageKit with the latest libappimage, so the first commit fixes the problem by restoring appimage_get_elf_size previously implemented in libappimage_shared.

This needs to go in a separated PR.

@hchunhui
Copy link
Author

@azubieta Thanks! I have opened a separated PR #160 that just includes the first commit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@azubieta azubieta azubieta requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Use zstd compression
2 participants
@hchunhui @azubieta