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

add support for debian packages #4434

Merged
merged 7 commits into from Jan 3, 2019

Conversation

Projects
None yet
3 participants
@pazos
Copy link
Contributor

pazos commented Dec 30, 2018

Fixes #3108 and maybe #4398

Requires koreader/koreader-base#785

@pazos pazos requested a review from Frenzie Dec 30, 2018

@pazos pazos force-pushed the pazos:debian branch 2 times, most recently from 84f5df1 to 2d0a4cd Dec 30, 2018

PACKAGE="KOReader"
AUTHOR="KOReader community"
DESC_1="An ebook reader application supporting PDF, DjVu, EPUB, FB2 and many more formats"
DESC_2="https://github.com/koreader/koreader"

This comment has been minimized.

@Frenzie

Frenzie Dec 31, 2018

Member

Probably better to use:

Suggested change Beta
DESC_2="https://github.com/koreader/koreader"
DESC_2="https://koreader.rocks"

This comment has been minimized.

@pazos

pazos Dec 31, 2018

Contributor

updated homepage 👍

} >"$CONTROL"
dpkg-deb -b "${PKG_PATH}" "${ROOTFS_BASE}/${FULL_NAME}"
else
echo "${COMMAND} not found, skipping target ${FULL_NAME}"

This comment has been minimized.

@Frenzie

Frenzie Dec 31, 2018

Member

Shouldn't you exit 1 here?

# we're always starting from our working directory
cd "${KOREADER_DIR}" || exit

# export load library path for some old firmware

This comment has been minimized.

@Frenzie

Frenzie Dec 31, 2018

Member

This is only for some old Pocketbooks really; I probably more or less accidentally added it to the AppImage since I partially based it on the Ubuntu Touch port (while I suppose you based this on the AppImage).

See 327624e

This comment has been minimized.

@pazos

pazos Dec 31, 2018

Contributor

ok, on my machine it works if I remove export LD_LIBRARY_PATH but I see that everywhere. https://github.com/koreader/koreader/search?q=export+LD_LIBRARY_PATH&unscoped_q=export+LD_LIBRARY_PATH

This comment has been minimized.

@Frenzie

Frenzie Dec 31, 2018

Member

Good point, it might otherwise fail to resolve the correct libraries (i.e., ours). Looks like it was added as a precaution in bb164c1

@pazos pazos force-pushed the pazos:debian branch from 2d0a4cd to 0257bcd Dec 31, 2018

@pazos

This comment has been minimized.

Copy link
Contributor

pazos commented Dec 31, 2018

updated a bunch of things:

  1. add common mimetypes on .desktop
  2. package as root and move datastorage to $HOME/.config/koreader
  3. beautifully do_debian_package.sh, now reports estimated size and the right homepage
  4. if arch is not specified asume native and try to get debian arch names from uname -m. Fallback is any for essoteric archs.
  5. add runtime depends on libsdl2

aa

@@ -41,7 +41,7 @@ if command_exists "$COMMAND"; then
echo "Depends: libsdl2-2.0-0"
echo "Architecture: ${ARCH}"
echo "Version: ${VERSION}"
echo "Installed-Size: $(du -ks ${INSTALL_DIR}/debian/usr/ | cut -f 1)"
echo "Installed-Size: $(du -ks "${INSTALL_DIR}/debian/usr/" | cut -f 1)"

This comment has been minimized.

@Frenzie

Frenzie Dec 31, 2018

Member

Not just shellcheck. Hail spaces! :-P

@pazos

This comment has been minimized.

Copy link
Contributor

pazos commented Jan 2, 2019

I had to move installation from /usr/local/ to /usr to make the desktop file able to open files using free-desktop standard. This has a few benefits (dpkg won't display warnings when updating/erasing the package, graphical installers will have the nice KOReader icon)

I also moved the subpath from share/koreader to lib/koreader, since it is the default for all-in-one applications (like firefox).

Last, but not least, I removed duplicated fonts (they're in fonts/ and in resources/fonts) to reduce the installation size to ~80MB and the package itself to ~27MB. I guess we should do the same on the appimage front.

1
2

@pazos pazos force-pushed the pazos:debian branch from 10b1744 to fcfcf33 Jan 3, 2019

@Frenzie

This comment has been minimized.

Copy link
Member

Frenzie commented Jan 3, 2019

Last, but not least, I removed duplicated fonts (they're in fonts/ and in resources/fonts) to reduce the installation size to ~80MB and the package itself to ~27MB. I guess we should do the same on the appimage front.

Good catch! :-)

@Frenzie

Frenzie approved these changes Jan 3, 2019

Copy link
Member

Frenzie left a comment

All looks good to me.

Frenzie added a commit to Frenzie/koreader that referenced this pull request Jan 3, 2019

Bump base
Includes:

*  [build] Add Debian native + cross targets koreader/koreader-base#785
  For koreader#4434
* [PocketBook] Rejig PB fb setup shenanigans koreader/koreader-base#783

@Frenzie Frenzie referenced this pull request Jan 3, 2019

Merged

Bump base #4442

Frenzie added a commit that referenced this pull request Jan 3, 2019

Bump base (#4442)
Includes:

*  [build] Add Debian native + cross targets koreader/koreader-base#785
  For #4434
* [PocketBook] Rejig PB fb setup shenanigans koreader/koreader-base#783

@Frenzie Frenzie merged commit 6de5927 into koreader:master Jan 3, 2019

1 check passed

ci/circleci Your tests passed on CircleCI!
Details
@Frenzie

This comment has been minimized.

Copy link
Member

Frenzie commented Jan 3, 2019

Btw, there are still some Lintian errors that can be easily fixed. But I figure it's more interesting to have it merged already right now.

Most or all of this for example, most notably for the short term the package name and copyright file:

E: KOReader: missing-dependency-on-libc needed by usr/lib/koreader/common/libluacompat52.so and 40 others
E: KOReader: debian-changelog-file-missing
E: KOReader: no-copyright-file
W: KOReader: description-synopsis-starts-with-article
W: KOReader: description-too-long
W: KOReader: extended-description-line-too-long
W: KOReader: extended-description-line-too-long
E: KOReader: bad-package-name
E: KOReader: package-not-lowercase
E: KOReader: maintainer-address-missing KOReader team
W: KOReader: duplicate-font-file usr/lib/koreader/fonts/freefont/FreeSans.ttf also in fonts-freefont-ttf
W: KOReader: duplicate-font-file usr/lib/koreader/fonts/freefont/FreeSerif.ttf also in fonts-freefont-ttf
W: KOReader: duplicate-font-file usr/lib/koreader/fonts/noto/NotoSans-Bold.ttf also in fonts-noto-hinted
W: KOReader: duplicate-font-file usr/lib/koreader/fonts/noto/NotoSans-BoldItalic.ttf also in fonts-noto-hinted
W: KOReader: duplicate-font-file usr/lib/koreader/fonts/noto/NotoSans-Italic.ttf also in fonts-noto-hinted
W: KOReader: duplicate-font-file usr/lib/koreader/fonts/noto/NotoSans-Regular.ttf also in fonts-noto-hinted
W: KOReader: duplicate-font-file usr/lib/koreader/fonts/noto/NotoSerif-Bold.ttf also in fonts-noto-hinted
W: KOReader: duplicate-font-file usr/lib/koreader/fonts/noto/NotoSerif-BoldItalic.ttf also in fonts-noto-hinted
W: KOReader: duplicate-font-file usr/lib/koreader/fonts/noto/NotoSerif-Italic.ttf also in fonts-noto-hinted
W: KOReader: duplicate-font-file usr/lib/koreader/fonts/noto/NotoSerif-Regular.ttf also in fonts-noto-hinted
W: KOReader: package-contains-vcs-control-file usr/lib/koreader/jit/.gitignore
W: KOReader: binary-without-manpage usr/bin/koreader

As an aside, I was slightly confused by these:

E: KOReader: unstripped-binary-or-object usr/lib/koreader/common/libluacompat52.so

The release version isn't supposed to be a debug build, so did Lintian effectively find a bug? It'll have to be investigated somewhat. :-)

Anyway, thanks!

@pazos

This comment has been minimized.

Copy link
Contributor

pazos commented Jan 3, 2019

@Frenzie: I didn't know lintian. Ooops..

E: KOReader: missing-dependency-on-libc needed ...

Yes, we probably need to specify that as a build-depends extracting libc6-dev version from the system where the package is built. If we want to release the package as nightlies then we want to build on the oldest debian-based system available.

W: KOReader: duplicate-font-file ...

Not sure how to fix that. Font path seems hardcoded.

E: KOReader: maintainer-address-missing KOReader team

Yep, maintainer field expects: Name of Maintainer email@domain.com

E: KOReader: debian-changelog-file-missing
E: KOReader: no-copyright-file
W: KOReader: description-synopsis-starts-with-article
W: KOReader: description-too-long
W: KOReader: extended-description-line-too-long
W: KOReader: extended-description-line-too-long
E: KOReader: bad-package-name
E: KOReader: package-not-lowercase

easy to fix 👍

Will check myself w/ lintian output

@Frenzie

This comment has been minimized.

Copy link
Member

Frenzie commented Jan 3, 2019

manpage is also easy to fix. We can basically just stick this in there:

koreader/reader.lua

Lines 45 to 59 in 6de5927

print("usage: ./reader.lua [OPTION] ... path")
print("Read all the books on your E-Ink reader")
print("")
print("-d start in debug mode")
print("-v debug in verbose mode")
print("-p enable Lua code profiling")
print("-h show this usage help")
print("")
print("If you give the name of a directory instead of a file path, a file")
print("chooser will show up and let you select a file")
print("")
print("If you don't pass any path, the last viewed document will be opened")
print("")
print("This software is licensed under the AGPLv3.")
print("See http://github.com/koreader/koreader for more info.")

Not sure how to fix that. Font path seems hardcoded.

You could symlink them in an install script or something, but those are harder and simultaneously less important issues imho.

If we want to release the package as nightlies then we want to build on the oldest debian-based system available.

I'd say the same Ubuntu 16.04 already used for Docker should do just fine. ;-)

@zaoqi-unsafe

This comment has been minimized.

Copy link

zaoqi-unsafe commented Jan 4, 2019

When can I download prebuilt *.deb?

@Frenzie

This comment has been minimized.

Copy link
Member

Frenzie commented Jan 4, 2019

See the comment in the issue #4398 (comment)

@pazos pazos deleted the pazos:debian branch Jan 6, 2019

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