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

Added htmlparser Luarock for newsdownloader #1110

Merged
merged 8 commits into from
Jun 4, 2020
Merged

Conversation

hngt
Copy link
Contributor

@hngt hngt commented Jun 4, 2020

My setup does not allow me to check how does it integrate with the complete build (Raspberry Pi). So, would be thankful if someone could verify if it works and then is used by the patched newsdownloader.


This change is Reviewable

Copy link
Member

@Frenzie Frenzie left a comment

Choose a reason for hiding this comment

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

You're missing the definitions in Makefile.defs so right now this PR it's just some dead lines. ;-)

LUA_HTMLPARSER_ROCK isn't set to anything, so you're saying in order to do nothing at all, execute these commands. But nothing at all doesn't need doing anything.

Besides the more obvious (search for LUA_SPORE_ROCK in the source to see where it occurs), also do:

diff --git a/Makefile.defs b/Makefile.defs
index ba4ff31..383e831 100644
--- a/Makefile.defs
+++ b/Makefile.defs
@@ -911,7 +911,7 @@ LPEG_DYNLIB=$(OUTPUT_DIR)/rocks/lib/lua/5.1/lpeg.so
 
 
 CMAKE_THIRDPARTY_LIBS := kpvcrlib,nanosvg
-CMAKE_THIRDPARTY_LIBS := $(CMAKE_THIRDPARTY_LIBS),evernote-sdk-lua,luajit,lpeg,turbo
+CMAKE_THIRDPARTY_LIBS := $(CMAKE_THIRDPARTY_LIBS),evernote-sdk-lua,lua-htmlparser,luajit,lpeg,turbo
 CMAKE_THIRDPARTY_LIBS := $(CMAKE_THIRDPARTY_LIBS),zyre,czmq,filemq,libzmq
 CMAKE_THIRDPARTY_LIBS := $(CMAKE_THIRDPARTY_LIBS),libk2pdfopt,tesseract,leptonica
 CMAKE_THIRDPARTY_LIBS := $(CMAKE_THIRDPARTY_LIBS),lj-wpaclient

@hngt
Copy link
Contributor Author

hngt commented Jun 4, 2020

I have added the fixes, still, unable to check whether it works.

Copy link
Member

@Frenzie Frenzie left a comment

Choose a reason for hiding this comment

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

It downloads it, so I assume it should also work. I'll give it a try shortly.

Makefile.defs Outdated Show resolved Hide resolved
Makefile Show resolved Hide resolved
Makefile.defs Outdated Show resolved Hide resolved
Copy link
Member

@Frenzie Frenzie left a comment

Choose a reason for hiding this comment

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

lgtm, any comments before merging @NiLuJe ?

thirdparty/lua-htmlparser/CMakeLists.txt Outdated Show resolved Hide resolved
@hngt
Copy link
Contributor Author

hngt commented Jun 4, 2020

Any other changes needed?

Co-authored-by: Frans de Jonge <fransdejonge@gmail.com>
@Frenzie
Copy link
Member

Frenzie commented Jun 4, 2020

Nothing for the moment, but I'll have to test again if you still can't and I'll also wait for a second opinion just in case a second pair of eyes spots something I missed.

@NiLuJe
Copy link
Member

NiLuJe commented Jun 4, 2020

LGTM, at a glance ;).

EDIT: Or not? c.f., gcc debug bot.

thirdparty/lua-htmlparser/CMakeLists.txt Outdated Show resolved Hide resolved
Makefile.defs Show resolved Hide resolved
@hngt
Copy link
Contributor Author

hngt commented Jun 4, 2020 via email

Makefile.defs Outdated Show resolved Hide resolved
Copy link
Member

@NiLuJe NiLuJe left a comment

Choose a reason for hiding this comment

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

Thanks!

LUA_SPORE_ROCK=$(LUAROCKS_DIR)/lua-spore/$(SPORE_VER)/lua-spore-$(LUA_SPORE_VER).rockspec
SPORE_VER=0.3.1-1
LUA_SPORE_VER=$(SPORE_VER)
LUA_SPORE_ROCK=$(LUAROCKS_DIR)/lua-spore/$(SPORE_VER)/lua-spore-$(SPORE_VER).rockspec
Copy link
Member

Choose a reason for hiding this comment

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

Oh, I see I'd missed the intermediate folder there, nice ;).

thirdparty/lua-htmlparser/CMakeLists.txt Outdated Show resolved Hide resolved
@Frenzie Frenzie merged commit 9f3f009 into koreader:master Jun 4, 2020
Frenzie added a commit to Frenzie/koreader that referenced this pull request Jun 4, 2020
Cf. <koreader#6228>

Includes:
* Update to OpenSSH 8.3p1 koreader/koreader-base#1107
* Update zsync2 koreader/koreader-base#1108
* thirdparty/harfbuzz 2.6.7 koreader/koreader-base#1109
* Add thirdparty/lua-htmlparser for newsdownloader koreader/koreader-base#1110
Frenzie added a commit to koreader/koreader that referenced this pull request Jun 4, 2020
Cf. <#6228>

Includes:
* Update to OpenSSH 8.3p1 koreader/koreader-base#1107
* Update zsync2 koreader/koreader-base#1108
* thirdparty/harfbuzz 2.6.7 koreader/koreader-base#1109
* Add thirdparty/lua-htmlparser for newsdownloader koreader/koreader-base#1110
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants