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

[fix] Makefile.defs: add sqlite to CMAKE_THIRDPARTY_LIBS #865

Merged
merged 1 commit into from Mar 16, 2019

Conversation

Frenzie
Copy link
Member

@Frenzie Frenzie commented Mar 16, 2019

Otherwise it won't get cleaned by make clean or make dist-clean.

Also make it a little more ordered & legible.

Otherwise it won't get cleaned by `make clean` or `make dist-clean`.

Also make it a little more ordered & legible.
@Frenzie Frenzie added the bug label Mar 16, 2019
@Frenzie Frenzie requested a review from NiLuJe March 16, 2019 10:58
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.

LGTM!

@Frenzie Frenzie merged commit 1783337 into koreader:master Mar 16, 2019
@Frenzie Frenzie deleted the fix-sqlite-omission branch March 16, 2019 11:54
Frenzie added a commit to Frenzie/koreader-base that referenced this pull request Mar 17, 2019
Just a passing thought, putting it up in a PR on GH for public archival/inspiration purposes or possibly to extend upon. This is a proof of concept of building in thirdparty/build/$(MACHINE) instead of thirdparty/build/$DEPENDENCY/$(MACHINE).

That way instead of using the Makefile.third with a lot of complex variable passing to CMake etc. you'd keep the logic almost entirely in CMake (not implemented in this proof of concept), so you should be able to write things like `add_dependencies(mupdf zlib)`.

Also the dist-clean command could be simplified to removing `THIRDPARTY_BUILD_DIR`.

Or to put it another way, if you're using CMake you might as well actually use it, so that errors like koreader#865 can't occur.
Frenzie added a commit to Frenzie/koreader-base that referenced this pull request Mar 17, 2019
Just a passing thought, putting it up in a PR on GH for public archival/inspiration purposes or possibly to extend upon. This is a proof of concept of building in thirdparty/build/$(MACHINE) instead of thirdparty/build/$DEPENDENCY/$(MACHINE).

That way instead of using the Makefile.third with a lot of complex variable passing to CMake etc. you'd keep the logic almost entirely in CMake (not implemented in this proof of concept), so you should be able to write things like `add_dependencies(mupdf zlib)`.

Also the dist-clean command could be simplified to removing `THIRDPARTY_BUILD_DIR`.

Or to put it another way, if you're using CMake you might as well actually use it, so that errors like koreader#865 can't occur.
Frenzie added a commit to Frenzie/koreader-base that referenced this pull request Mar 17, 2019
Like koreader#865 but only occurs when cross-compiling for certain platforms.

Cf. koreader#866 for a possible solution where this could never be a problem.
Frenzie added a commit that referenced this pull request Mar 17, 2019
Like #865 but only occurs when cross-compiling for certain platforms.

Cf. #866 for a possible solution where this could never be a problem.
NiLuJe added a commit to NiLuJe/koreader that referenced this pull request Mar 18, 2019
NiLuJe added a commit to NiLuJe/koreader that referenced this pull request Mar 18, 2019
Frenzie pushed a commit to koreader/koreader that referenced this pull request Mar 18, 2019
* Properly account for MuPDF feeding us premultiplied alpha

* Bump base to pickup necessary backend changes

Also includes a bunch of CMake refactoring
(koreader/koreader-base#865
koreader/koreader-base#867
koreader/koreader-base#868)
mwoz123 pushed a commit to mwoz123/koreader that referenced this pull request Mar 29, 2020
…ader#4807)

* Properly account for MuPDF feeding us premultiplied alpha

* Bump base to pickup necessary backend changes

Also includes a bunch of CMake refactoring
(koreader/koreader-base#865
koreader/koreader-base#867
koreader/koreader-base#868)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants