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

Allow embedding txiki in other applications #517

Merged
merged 1 commit into from
May 15, 2024

Conversation

jspngh
Copy link
Contributor

@jspngh jspngh commented May 14, 2024

Hi,

First of all, thanks for all the work on txiki.
Currently only a command-line interface is available, but I would like to embed this javascript runtime in my own application.

Is this change something that you would be open to?
It requires only a small adaptation of the CMakeLists.txt and Makefile.

Let me know what you think.

@jspngh jspngh force-pushed the feature/embeddable branch 2 times, most recently from a867314 to 46793d7 Compare May 14, 2024 09:21
Copy link
Owner

@saghul saghul left a comment

Choose a reason for hiding this comment

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

Alright, let's do this! Left some comments.

CMakeLists.txt Outdated
target_link_libraries(tjs qjs uv_a m3 sqlite3 m pthread ${CURL_LIBRARIES})

if (BUILD_WITH_MIMALLOC)
target_compile_definitions(tjs PRIVATE TJS__HAS_MIMALLOC)
target_link_libraries(tjs mimalloc-static)
endif()

add_executable(tjs-cli EXCLUDE_FROM_ALL
Copy link
Owner

Choose a reason for hiding this comment

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

Don't exclude it, it needs to always be built.

Copy link
Owner

Choose a reason for hiding this comment

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

If you'd like to only build the library, then add some BUILD_ONLY_LIBRARY option please.

Makefile Outdated
@@ -22,7 +22,7 @@ $(BUILD_DIR):
cmake -B $(BUILD_DIR) -DCMAKE_BUILD_TYPE=$(BUILDTYPE)

$(TJS): $(BUILD_DIR)
cmake --build $(BUILD_DIR) -j $(JOBS)
cmake --build $(BUILD_DIR) --target tjs-cli -j $(JOBS)
Copy link
Owner

Choose a reason for hiding this comment

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

You don't need this if you don't exclude it.

CMakeLists.txt Show resolved Hide resolved
@jspngh
Copy link
Contributor Author

jspngh commented May 15, 2024

Alright, let's do this! Left some comments.

Should be fixed now, let me know if there's anything else.

@saghul
Copy link
Owner

saghul commented May 15, 2024

Changes LGTM! Let's see if the CI is happy.

@saghul saghul merged commit c5f3469 into saghul:master May 15, 2024
14 checks passed
@jspngh jspngh deleted the feature/embeddable branch May 15, 2024 11:44
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

2 participants