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

More fixes for Wasm builds, and move configure one layer up #106

Merged
merged 1 commit into from
Nov 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 23 additions & 10 deletions makefiles/duckdb_extension_c_api.Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
# LINUX_CI_IN_DOCKER : indicates that the build is being run in/out of Docker in the linux CI
# SKIP_TESTS : makes the test targets turn into NOPs

.PHONY: platform extension_version test_extension_release test_extension_debug test_extension_release_internal test_extension_debug_internal tests_skipped clean_build clean_configure nop set_duckdb_tag set_duckdb_version output_distribution_matrix venv configure_ci check_configure
.PHONY: platform extension_version test_extension_release test_extension_debug test_extension_release_internal test_extension_debug_internal tests_skipped clean_build clean_configure nop set_duckdb_tag set_duckdb_version output_distribution_matrix venv configure_ci check_configure move_wasm_extension

#############################################
### Platform dependent config
Expand Down Expand Up @@ -171,10 +171,10 @@ output_distribution_matrix:
ifneq ($(DUCKDB_WASM_PLATFORM),)

link_wasm_debug:
emcc build/debug/$(EXTENSION_LIB_FILENAME) -o build/debug/$(EXTENSION_FILENAME_NO_METADATA) -O3 -g -sSIDE_MODULE=2 -sEXPORTED_FUNCTIONS="_$(EXTENSION_NAME)_init_c_api"
emcc build/$(DUCKDB_WASM_PLATFORM)/debug/$(EXTENSION_LIB_FILENAME) -o build/$(DUCKDB_WASM_PLATFORM)/debug/$(EXTENSION_FILENAME_NO_METADATA) -O3 -g -sSIDE_MODULE=2 -sEXPORTED_FUNCTIONS="_$(EXTENSION_NAME)_init_c_api"

link_wasm_release:
emcc build/release/$(EXTENSION_LIB_FILENAME) -o build/release/$(EXTENSION_FILENAME_NO_METADATA) -O3 -sSIDE_MODULE=2 -sEXPORTED_FUNCTIONS="_$(EXTENSION_NAME)_init_c_api"
emcc build/$(DUCKDB_WASM_PLATFORM)/release/$(EXTENSION_LIB_FILENAME) -o build/$(DUCKDB_WASM_PLATFORM)/release/$(EXTENSION_FILENAME_NO_METADATA) -O3 -sSIDE_MODULE=2 -sEXPORTED_FUNCTIONS="_$(EXTENSION_NAME)_init_c_api"

else
link_wasm_debug:
Expand All @@ -187,23 +187,23 @@ endif
#############################################
build_extension_with_metadata_debug: check_configure link_wasm_debug
$(PYTHON_VENV_BIN) extension-ci-tools/scripts/append_extension_metadata.py \
-l build/debug/$(EXTENSION_FILENAME_NO_METADATA) \
-o build/debug/$(EXTENSION_FILENAME) \
-l build/$(DUCKDB_WASM_PLATFORM)/debug/$(EXTENSION_FILENAME_NO_METADATA) \
-o build/$(DUCKDB_WASM_PLATFORM)/debug/$(EXTENSION_FILENAME) \
-n $(EXTENSION_NAME) \
-dv $(MINIMUM_DUCKDB_VERSION) \
-evf configure/extension_version.txt \
-pf configure/platform.txt
$(PYTHON_VENV_BIN) -c "import shutil;shutil.copyfile('build/debug/$(EXTENSION_FILENAME)', 'build/debug/extension/$(EXTENSION_NAME)/$(EXTENSION_FILENAME)')"
$(PYTHON_VENV_BIN) -c "import shutil;shutil.copyfile('build/$(DUCKDB_WASM_PLATFORM)/debug/$(EXTENSION_FILENAME)', 'build/$(DUCKDB_WASM_PLATFORM)/debug/extension/$(EXTENSION_NAME)/$(EXTENSION_FILENAME)')"

build_extension_with_metadata_release: check_configure link_wasm_release
$(PYTHON_VENV_BIN) extension-ci-tools/scripts/append_extension_metadata.py \
-l build/release/$(EXTENSION_FILENAME_NO_METADATA) \
-o build/release/$(EXTENSION_FILENAME) \
-l build/$(DUCKDB_WASM_PLATFORM)/release/$(EXTENSION_FILENAME_NO_METADATA) \
-o build/$(DUCKDB_WASM_PLATFORM)/release/$(EXTENSION_FILENAME) \
-n $(EXTENSION_NAME) \
-dv $(MINIMUM_DUCKDB_VERSION) \
-evf configure/extension_version.txt \
-pf configure/platform.txt
$(PYTHON_VENV_BIN) -c "import shutil;shutil.copyfile('build/release/$(EXTENSION_FILENAME)', 'build/release/extension/$(EXTENSION_NAME)/$(EXTENSION_FILENAME)')"
$(PYTHON_VENV_BIN) -c "import shutil;shutil.copyfile('build/$(DUCKDB_WASM_PLATFORM)/release/$(EXTENSION_FILENAME)', 'build/$(DUCKDB_WASM_PLATFORM)/release/extension/$(EXTENSION_NAME)/$(EXTENSION_FILENAME)')"

#############################################
### Python
Expand Down Expand Up @@ -234,4 +234,17 @@ configure_ci: $(CONFIGURE_CI_STEP)
# Because the configure_ci may differ from configure, we don't automatically run configure on make build, this makes the error a bit nicer
check_configure:
@$(PYTHON_BIN) -c "import os; assert os.path.exists('configure/platform.txt'), 'The configure step appears to not be run. Please try running make configure'"
@$(PYTHON_BIN) -c "import os; assert os.path.exists('configure/venv'), 'The configure step appears to not be run. Please try running make configure'"
@$(PYTHON_BIN) -c "import os; assert os.path.exists('configure/venv'), 'The configure step appears to not be run. Please try running make configure'"

move_wasm_extension:
$(PYTHON_VENV_BIN) -c "from pathlib import Path;Path('./build/$(DUCKDB_WASM_PLATFORM)/extension/$(EXTENSION_NAME)').mkdir(parents=True, exist_ok=True)"
$(PYTHON_VENV_BIN) -c "import shutil;shutil.copyfile('build/$(DUCKDB_WASM_PLATFORM)/release/extension/$(EXTENSION_NAME)/$(EXTENSION_FILENAME)', 'build/$(DUCKDB_WASM_PLATFORM)/extension/$(EXTENSION_NAME)/$(EXTENSION_FILENAME)')"

wasm_mvp:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps the wasm_mvp, wasm_eh wasm_threads targets can stay in the main makefile? I think that's a decently clean split? that way we do move the internals into extension-ci-tools but the extension repo can still control what is done for those targets.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just to be clear, you mean moving:

+wasm_mvp:
+       DUCKDB_PLATFORM=wasm_mvp make configure release move_wasm_extension
+
+wasm_eh:
+       DUCKDB_PLATFORM=wasm_eh make configure release move_wasm_extension
+
+wasm_threads:
+       DUCKDB_PLATFORM=wasm_coi make configure release move_wasm_extension

to the extension own makefile, and keeping the infra here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think we agreed: wasm_* targets stay in the inner makefiles, users are supposed to invoke it like:

DUCKDB_PLATFORM=wasm_eh make configure release

DUCKDB_PLATFORM=wasm_mvp make configure release move_wasm_extension

wasm_eh:
DUCKDB_PLATFORM=wasm_eh make configure release move_wasm_extension

wasm_threads:
DUCKDB_PLATFORM=wasm_coi make configure release move_wasm_extension
8 changes: 4 additions & 4 deletions makefiles/duckdb_extension_rs.Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,13 @@ endif

build_extension_library_debug: check_configure
DUCKDB_EXTENSION_NAME=$(EXTENSION_NAME) DUCKDB_EXTENSION_MIN_DUCKDB_VERSION=$(MINIMUM_DUCKDB_VERSION) cargo build $(CARGO_OVERRIDE_DUCKDB_RS_FLAG) $(TARGET_INFO)
$(PYTHON_VENV_BIN) -c "from pathlib import Path;Path('./build/debug/extension/$(EXTENSION_NAME)').mkdir(parents=True, exist_ok=True)"
$(PYTHON_VENV_BIN) -c "import shutil;shutil.copyfile('target/$(TARGET)/debug/$(IS_EXAMPLE)/$(EXTENSION_LIB_FILENAME)', 'build/debug/$(EXTENSION_LIB_FILENAME)')"
$(PYTHON_VENV_BIN) -c "from pathlib import Path;Path('./build/$(DUCKDB_WASM_PLATFORM)/debug/extension/$(EXTENSION_NAME)').mkdir(parents=True, exist_ok=True)"
$(PYTHON_VENV_BIN) -c "import shutil;shutil.copyfile('target/$(TARGET)/debug/$(IS_EXAMPLE)/$(EXTENSION_LIB_FILENAME)', 'build/$(DUCKDB_WASM_PLATFORM)/debug/$(EXTENSION_LIB_FILENAME)')"

build_extension_library_release: check_configure
DUCKDB_EXTENSION_NAME=$(EXTENSION_NAME) DUCKDB_EXTENSION_MIN_DUCKDB_VERSION=$(MINIMUM_DUCKDB_VERSION) cargo build $(CARGO_OVERRIDE_DUCKDB_RS_FLAG) --release $(TARGET_INFO)
$(PYTHON_VENV_BIN) -c "from pathlib import Path;Path('./build/release/extension/$(EXTENSION_NAME)').mkdir(parents=True, exist_ok=True)"
$(PYTHON_VENV_BIN) -c "import shutil;shutil.copyfile('target/$(TARGET)/release/$(IS_EXAMPLE)/$(EXTENSION_LIB_FILENAME)', 'build/release/$(EXTENSION_LIB_FILENAME)')"
$(PYTHON_VENV_BIN) -c "from pathlib import Path;Path('./build/$(DUCKDB_WASM_PLATFORM)/release/extension/$(EXTENSION_NAME)').mkdir(parents=True, exist_ok=True)"
$(PYTHON_VENV_BIN) -c "import shutil;shutil.copyfile('target/$(TARGET)/release/$(IS_EXAMPLE)/$(EXTENSION_LIB_FILENAME)', 'build/$(DUCKDB_WASM_PLATFORM)/release/$(EXTENSION_LIB_FILENAME)')"

#############################################
### Misc
Expand Down
Loading