Skip to content

Commit

Permalink
chore: Move bundling from CMake to Python (#508)
Browse files Browse the repository at this point in the history
Some of the work with the Meson build system has highlighted that the
bundling is currently very inflexible because it's implemented in CMake.
This was initially done because it made it easy to use
`configure_file()` in CMake to keep the version information consistent,
and because it allowed a straightforward path to testing the bundled
files in the same way as the normal build.

This had the unfortunate side effect of requiring CMake to "build" the R
or Python packages as is generally confusing, and used the CMake
"install" in a confusing way (it installed sources). Instead, we can do
this with a Python script (and call that from CMake, R, and Python). The
testing of the namespaced builds is still done in CMake, which is
essential to ensure that both build types are tested in the same way.

This also closes #485 by prefixing all nanoarrow includes with
"nanoarrow/" in the source files. Many of them had to not use this
prefix because when the files were bundled they weren't always bundled
into a "nanoarrow/" directory. This feature has been moved to the
bundling script which can now namespace or un-namespace the nanoarrow
includes.
  • Loading branch information
paleolimbot authored Jun 18, 2024
1 parent 0ae9fda commit 2b935cf
Show file tree
Hide file tree
Showing 34 changed files with 748 additions and 554 deletions.
29 changes: 8 additions & 21 deletions .github/workflows/bundle.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -35,28 +35,15 @@ jobs:

- name: Bundle nanoarrow
run: |
mkdir build && cd build
cmake .. -DNANOARROW_BUNDLE=ON
cmake --build .
cmake --install . --prefix=../nanoarrow-latest
cd ..
cp LICENSE.txt nanoarrow-latest/LICENSE.txt
- name: Bundle nanoarrow_ipc
run: |
rm -rf build
mkdir build && cd build
cmake .. -DNANOARROW_BUNDLE=ON -DNANOARROW_IPC=ON
cmake --build .
cmake --install . --prefix=../nanoarrow-latest
python3 ci/scripts/bundle.py \
--source-output-dir=nanoarrow-latest \
--include-output-dir=nanoarrow-latest \
--header-namespace= \
--with-device \
--with-ipc \
--with-flatcc
- name: Bundle nanoarrow_device
run: |
rm -rf build
mkdir build && cd build
cmake .. -DNANOARROW_BUNDLE=ON -DNANOARROW_DEVICE=ON
cmake --build .
cmake --install . --prefix=../nanoarrow-latest
cp LICENSE.txt nanoarrow-latest/LICENSE.txt
- name: Compress bundle
run: |
Expand Down
29 changes: 11 additions & 18 deletions .github/workflows/examples.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,9 @@ jobs:
- name: Minimal Vendored Example
run: |
cd examples/vendored-minimal
mkdir build && cd build
cmake ../../.. -DNANOARROW_BUNDLE=ON -DNANOARROW_NAMESPACE=ExampleVendored
cmake --build .
cmake --install . --prefix=../src
cd ../src
python3 ../../ci/scripts/bundle.py --source-output-dir=src --include-output-dir=src
cd src
gcc -c library.c nanoarrow.c
ar rcs libexample_vendored_minimal_library.a library.o nanoarrow.o
gcc -o example_vendored_minimal_app app.c libexample_vendored_minimal_library.a
Expand All @@ -85,18 +81,15 @@ jobs:
- name: Ipc Vendored Example
run: |
cd examples/vendored-ipc
mkdir build && cd build
cmake ../../.. -DNANOARROW_BUNDLE=ON -DNANOARROW_NAMESPACE=ExampleVendored
cmake --build .
cmake --install . --prefix=../src/nanoarrow
mkdir ../build_ipc && cd ../build_ipc
cmake ../../.. -DNANOARROW_BUNDLE=ON -DNANOARROW_IPC=ON
cmake --build .
cmake --install . --prefix=../src/nanoarrow
cd ../src
gcc -c library.c nanoarrow/nanoarrow.c nanoarrow/flatcc.c nanoarrow/nanoarrow_ipc.c -I./nanoarrow -I./nanoarrow/flatcc
python3 ../../ci/scripts/bundle.py \
--source-output-dir=src \
--include-output-dir=src \
--symbol-namespace=MyProject \
--with-ipc \
--with-flatcc
cd src
gcc -c library.c nanoarrow.c flatcc.c nanoarrow_ipc.c -I.
ar rcs libexample_vendored_ipc_library.a library.o nanoarrow.o nanoarrow_ipc.o flatcc.o
gcc -o example_vendored_ipc_app app.c libexample_vendored_ipc_library.a
Expand Down
Loading

0 comments on commit 2b935cf

Please sign in to comment.