Skip to content

Commit 86f26ed

Browse files
robambalungoldbaumtimkpaine
authored
Clang build support (#132)
* fixes #83 - code updates to support clang compilation / cleaned up some clang warnings Signed-off-by: Ambalu, Robert <robert.ambalu@point72.com> * fixes #132 - final touches, all tests pass. Note this had to comment out the --arch options in CMakeLists.txt, not sure what the implications are Signed-off-by: Rob Ambalu <robert.ambalu@point72.com> * fixes #132 - attempt to kick off macos build Signed-off-by: Rob Ambalu <robert.ambalu@point72.com> * unset CC in mac cibuildwheel setup Signed-off-by: Nathan Goldbaum <nathan.goldbaum@gmail.com> * Add support for mac conda builds Signed-off-by: Nathan Goldbaum <nathan.goldbaum@gmail.com> * Update docs for mac builds Signed-off-by: Nathan Goldbaum <nathan.goldbaum@gmail.com> * attempt to fix macos-12 conda build Signed-off-by: Nathan Goldbaum <nathan.goldbaum@gmail.com> * one more try for intel mac support Signed-off-by: Nathan Goldbaum <nathan.goldbaum@gmail.com> * reduce length of timeout for test_threaded_run Signed-off-by: Nathan Goldbaum <nathan.goldbaum@gmail.com> * make this test more meaningful now that it has a shorter timeout Signed-off-by: Nathan Goldbaum <nathan.goldbaum@gmail.com> * CMakeLists.txt - attempt tp align RPATH on linux, drop ../lib Signed-off-by: Rob Ambalu <robert.ambalu@point72.com> * #132 temporarily add some debug logging to threaded engine to try to figure out why it fails on GH Signed-off-by: Rob Ambalu <robert.ambalu@point72.com> * #132 fix lint for test logging Signed-off-by: Rob Ambalu <robert.ambalu@point72.com> * #132 enable stdout logging for tests, revert test_engine back to old timing Signed-off-by: Rob Ambalu <robert.ambalu@point72.com> * #132 - next atempt Signed-off-by: Rob Ambalu <robert.ambalu@point72.com> * #132 - next attempt Signed-off-by: Rob Ambalu <robert.ambalu@point72.com> * #132 - next attempt Signed-off-by: Rob Ambalu <robert.ambalu@point72.com> * fixes #132 - Fix race condition exposed in macos build where push events can be deferred indefinitely if timer executes before event ( wait will keep waiting until max time, even if an event is pending ) Simplify QueueWaiter ( remove it entirely ) and move condvar/mutex/blocking logic directly into SRMWLockFreeQueue so that we can check on m_head directly. Revert all intermediate changes that were committed for testing / tracking down this issue Signed-off-by: Rob Ambalu <robert.ambalu@point72.com> * fixes #132 - undo intrusive LFQueue changes ( for now ), simplify the fix and attack the queue issue properly another day. fix cpp time test Signed-off-by: Rob Ambalu <robert.ambalu@point72.com> * fixes #132 - fix comment typo Signed-off-by: Rob Ambalu <robert.ambalu@point72.com> * clean up ci/cd code and ensure mac tests run Signed-off-by: Tim Paine <timothy.paine@cubistsystematic.com> --------- Signed-off-by: Ambalu, Robert <robert.ambalu@point72.com> Signed-off-by: Rob Ambalu <robert.ambalu@RA7293-M1.saccap.int> Signed-off-by: Rob Ambalu <robert.ambalu@point72.com> Signed-off-by: Nathan Goldbaum <nathan.goldbaum@gmail.com> Signed-off-by: Tim Paine <timothy.paine@cubistsystematic.com> Co-authored-by: Nathan Goldbaum <nathan.goldbaum@gmail.com> Co-authored-by: Tim Paine <timothy.paine@cubistsystematic.com> Co-authored-by: Tim Paine <3105306+timkpaine@users.noreply.github.com>
1 parent 6716c19 commit 86f26ed

35 files changed

Lines changed: 109 additions & 131 deletions

‎.github/workflows/build.yml‎

Lines changed: 33 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -233,12 +233,15 @@ jobs:
233233
# avoid unnecessary use of mac resources
234234
- is-full-run: false
235235
os: macos-12
236+
236237
- is-full-run: false
237238
os: macos-14
238239
python-version: "3.8"
240+
239241
- is-full-run: false
240242
os: macos-14
241243
python-version: "3.9"
244+
242245
- is-full-run: false
243246
os: macos-14
244247
python-version: "3.10"
@@ -284,7 +287,7 @@ jobs:
284287
run: make dist-py-cibw
285288
env:
286289
CIBW_BUILD: "${{ matrix.cibuildwheel }}-macos*"
287-
CIBW_ENVIRONMENT_MACOS: CC="/usr/local/bin/gcc-13" CXX="/usr/local/bin/g++-13" CCACHE_DIR="/Users/runner/work/csp/csp/.ccache" VCPKG_DEFAULT_BINARY_CACHE="${{ env.VCPKG_DEFAULT_BINARY_CACHE }}" VCPKG_DOWNLOADS="${{ env.VCPKG_DOWNLOADS }}"
290+
CIBW_ENVIRONMENT_MACOS: CCACHE_DIR="/Users/runner/work/csp/csp/.ccache" VCPKG_DEFAULT_BINARY_CACHE="${{ env.VCPKG_DEFAULT_BINARY_CACHE }}" VCPKG_DOWNLOADS="${{ env.VCPKG_DOWNLOADS }}"
288291
CIBW_ARCHS_MACOS: x86_64
289292
CIBW_BUILD_VERBOSITY: 3
290293
if: ${{ matrix.os == 'macos-12' }}
@@ -293,7 +296,7 @@ jobs:
293296
run: make dist-py-cibw
294297
env:
295298
CIBW_BUILD: "${{ matrix.cibuildwheel }}-macos*"
296-
CIBW_ENVIRONMENT_MACOS: PATH="/opt/homebrew/opt/bison/bin/:$PATH" CC="/opt/homebrew/bin/gcc-13" CXX="/opt/homebrew/bin/g++-13" LDFLAGS="-Wl,-ld_classic" CCACHE_DIR="/Users/runner/work/csp/csp/.ccache" VCPKG_DEFAULT_BINARY_CACHE="${{ env.VCPKG_DEFAULT_BINARY_CACHE }}" VCPKG_DOWNLOADS="${{ env.VCPKG_DOWNLOADS }}"
299+
CIBW_ENVIRONMENT_MACOS: CCACHE_DIR="/Users/runner/work/csp/csp/.ccache" VCPKG_DEFAULT_BINARY_CACHE="${{ env.VCPKG_DEFAULT_BINARY_CACHE }}" VCPKG_DOWNLOADS="${{ env.VCPKG_DOWNLOADS }}"
297300
CIBW_ARCHS_MACOS: arm64
298301
CIBW_BUILD_VERBOSITY: 3
299302
if: ${{ matrix.os == 'macos-14' }}
@@ -308,11 +311,11 @@ jobs:
308311
CIBW_ENVIRONMENT_WINDOWS: TODO="todo"
309312
if: ${{ matrix.os == 'windows-2022' }}
310313

311-
# Check dist files
314+
##########
315+
# Common
312316
- name: Check Wheels
313317
run: make dist-check
314318

315-
# Upload artifacts
316319
- name: Upload Wheel
317320
uses: actions/upload-artifact@v4
318321
with:
@@ -358,15 +361,12 @@ jobs:
358361
- name: Install python dependencies
359362
run: make requirements
360363

361-
# Build SDist
362364
- name: Python SDist Steps
363365
run: make dist-py-sdist
364366

365-
# Check dist files
366367
- name: Check sdist
367368
run: make dist-check
368369

369-
# Upload artifacts
370370
- name: Upload SDist
371371
uses: actions/upload-artifact@v4
372372
with:
@@ -414,28 +414,26 @@ jobs:
414414
- os: macos-14
415415
python-version: "3.9"
416416

417-
# Exclude windows builds
417+
# windows is slow, so dont build unless its a full run
418418
# - is-full-run: false
419419
# os: windows-2022
420420

421-
# Exclude macOS builds for now
422-
- os: macos-12
423-
# is-full-run: false # FIXME https://github.com/Point72/csp/issues/33
424-
425-
- os: macos-14
426-
# is-full-run: false # FIXME https://github.com/Point72/csp/issues/33
421+
# avoid unnecessary use of mac resources
422+
- is-full-run: false
423+
os: macos-12
427424

428-
# Exclude Python 3.8, 3.10, 3.11 builds
429425
- is-full-run: false
430-
python-version: 3.8
426+
os: macos-14
427+
python-version: "3.8"
431428

432429
- is-full-run: false
433-
python-version: 3.9
430+
os: macos-14
431+
python-version: "3.9"
434432

435433
- is-full-run: false
434+
os: macos-14
436435
python-version: "3.10"
437436

438-
439437
runs-on: ${{ matrix.os }}
440438

441439
steps:
@@ -452,26 +450,29 @@ jobs:
452450
- name: Install python dependencies
453451
run: make requirements
454452

453+
- name: Download wheel
454+
uses: actions/download-artifact@v4
455+
with:
456+
name: csp-dist-${{ runner.os }}-${{ runner.arch }}-${{ matrix.python-version }}
457+
458+
########
459+
# Linux
455460
- name: Install test dependencies (Linux)
456461
shell: bash
457462
run: sudo apt-get install graphviz
458463
if: ${{ runner.os == 'Linux' }}
459464

465+
- name: Install wheel (Linux)
466+
run: python -m pip install -U *manylinux2014*.whl --target .
467+
if: ${{ runner.os == 'Linux' }}
468+
469+
########
470+
# Macos
460471
- name: Install test dependencies (Mac)
461472
shell: bash
462473
run: brew install graphviz
463474
if: ${{ runner.os == 'macOS' }}
464475

465-
# Download artifact
466-
- name: Download wheel
467-
uses: actions/download-artifact@v4
468-
with:
469-
name: csp-dist-${{ runner.os }}-${{ runner.arch }}-${{ matrix.python-version }}
470-
471-
- name: Install wheel (Linux)
472-
run: python -m pip install -U *manylinux2014*.whl --target .
473-
if: ${{ runner.os == 'Linux' }}
474-
475476
- name: Install wheel (OSX x86)
476477
run: python -m pip install -U *x86*.whl --target .
477478
if: ${{ runner.os == 'macOS' && runner.arch == 'X64' }}
@@ -480,11 +481,14 @@ jobs:
480481
run: python -m pip install -U *arm64*.whl --target .
481482
if: ${{ runner.os == 'macOS' && runner.arch == 'ARM64' }}
482483

484+
########
485+
# Windows
483486
- name: Install wheel (windows)
484487
run: python -m pip install -U (Get-ChildItem .\*.whl | Select-Object -Expand FullName) --target .
485488
if: ${{ runner.os == 'Windows' }}
486489

487-
# Run tests
490+
##########
491+
# Common
488492
- name: Python Test Steps
489493
run: make test
490494

@@ -532,25 +536,21 @@ jobs:
532536
with:
533537
cibuildwheel: 'cp39'
534538

535-
# Python
536539
- name: Install python dependencies
537540
run: make requirements
538541

539-
# Download sdist
540542
- uses: actions/download-artifact@v4
541543
with:
542544
name: csp-sdist
543545
path: dist/
544546

545-
# Install sdist
546547
- name: Install sdist
547548
run: python -m pip install -U -vvv dist/csp*.tar.gz --target .
548549
env:
549550
CCACHE_DIR: /home/runner/work/csp/csp/.ccache
550551
VCPKG_DEFAULT_BINARY_CACHE: /home/runner/vcpkg_cache
551552
VCPKG_DOWNLOADS: /home/runner/vcpkg_download_cache
552553

553-
# Test sdist
554554
- name: Run tests against from-scratch sdist build
555555
run: make test
556556
env:
@@ -606,7 +606,6 @@ jobs:
606606
shell: bash
607607
run: sudo apt-get install graphviz
608608

609-
# Download artifact
610609
- name: Download wheel
611610
uses: actions/download-artifact@v4
612611
with:
@@ -618,7 +617,6 @@ jobs:
618617
- name: Install package - ${{ matrix.package }}
619618
run: python -m pip install -U "${{ matrix.package }}"
620619

621-
# Run tests
622620
- name: Python Test Steps
623621
run: make test TEST_ARGS="-k TestDBReader"
624622
if: ${{ contains( 'sqlalchemy', matrix.package )}}

‎.github/workflows/conda.yml‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,15 +29,21 @@ permissions:
2929

3030
jobs:
3131
build:
32-
runs-on: ubuntu-22.04
32+
strategy:
33+
matrix:
34+
os:
35+
- ubuntu-22.04
36+
- macos-14
37+
- macos-12
38+
runs-on: ${{ matrix.os }}
3339
steps:
3440
- name: Checkout
3541
uses: actions/checkout@v4
3642

3743
- uses: mamba-org/setup-micromamba@v1
3844
with:
3945
micromamba-version: '1.5.7-0'
40-
environment-file: conda/dev-environment-linux.yml
46+
environment-file: conda/dev-environment-unix.yml
4147
init-shell: >-
4248
bash
4349
cache-environment: true
@@ -48,14 +54,16 @@ jobs:
4854
with:
4955
cibuildwheel: 'cp311'
5056

51-
- name: Python Lint Steps (Linux)
57+
- name: Python Lint Steps
5258
run: make lint
5359
shell: micromamba-shell {0}
5460

55-
- name: Python Build Steps (Linux)
61+
- name: Python Build Steps
5662
run: make build-conda
5763
shell: micromamba-shell {0}
5864

59-
- name: Python Test Steps (Linux)
65+
- name: Python Test Steps
6066
run: make test
6167
shell: micromamba-shell {0}
68+
69+

‎CMakeLists.txt‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -144,19 +144,16 @@ endif()
144144
# RPath #
145145
#########
146146
if(MACOS)
147-
set(CMAKE_INSTALL_RPATH "@loader_path/:@loader_path/../lib")
147+
set(CMAKE_INSTALL_RPATH "@loader_path/")
148148
elseif(LINUX)
149-
set(CMAKE_INSTALL_RPATH "\$ORIGIN:\$ORIGIN/../lib")
149+
set(CMAKE_INSTALL_RPATH "\$ORIGIN")
150150
endif()
151151

152152
###################################################################################################################################################
153153
# Flags #
154154
#########
155155
# Compiler version flags
156-
if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
157-
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++17")
158-
endif()
159-
156+
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++17")
160157

161158
# Optimization Flags
162159
if(WIN32)
@@ -204,9 +201,13 @@ else()
204201
-Wall \
205202
-Wno-deprecated-declarations \
206203
-Wno-deprecated \
207-
-Wno-maybe-uninitialized \
208204
")
209205
add_definitions(-DNDEBUG)
206+
if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
207+
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} \
208+
-Wno-maybe-uninitialized \
209+
")
210+
endif()
210211
endif()
211212
endif()
212213

‎Makefile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ clean: ## clean the repository
182182
.PHONY: dependencies-mac dependencies-debian dependencies-fedora dependencies-vcpkg dependencies-win
183183

184184
dependencies-mac: ## install dependencies for mac
185-
HOMEBREW_NO_AUTO_UPDATE=1 brew install bison cmake flex make ninja # gcc@13
185+
HOMEBREW_NO_AUTO_UPDATE=1 brew install bison cmake flex make ninja
186186
brew unlink bison flex && brew link --force bison flex
187187

188188
dependencies-debian: ## install dependencies for linux

conda/dev-environment-linux.yml renamed to conda/dev-environment-unix.yml

File renamed without changes.

‎cpp/csp/adapters/parquet/ArrowSingleColumnArrayBuilder.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,7 @@ class PrimitiveTypedArrayBuilder : public BaseTypedArrayBuilder<ValueType, Arrow
304304
protected:
305305
void pushValueToArray()
306306
{
307-
this -> m_builderPtr -> Append( *this -> m_value );
307+
[[maybe_unused]] auto status = this -> m_builderPtr -> Append( *this -> m_value );
308308
}
309309
};
310310

‎cpp/csp/adapters/parquet/ParquetOutputAdapter.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ class ParquetOutputHandler
3131
{
3232
}
3333

34+
virtual ~ParquetOutputHandler() {}
35+
3436
uint32_t getChunkSize() const;
3537

3638
virtual uint32_t getNumColumns() = 0;

‎cpp/csp/adapters/parquet/ParquetReader.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -376,7 +376,7 @@ class SingleTableParquetReader : public ParquetReader
376376
const std::optional<utils::Symbol> &symbol, const DialectGenericListReaderInterface::Ptr &listReaderInterface ) override
377377
{
378378
ParquetReader::addListSubscriber( column, inputAdapter, symbol, listReaderInterface);
379-
m_columnSubscriptionContainer.m_listColumnSubscriptions[column].push_back(ListColumnSubscriberInfo{inputAdapter, symbol, listReaderInterface});
379+
m_columnSubscriptionContainer.m_listColumnSubscriptions[column].push_back(ListColumnSubscriberInfo{{inputAdapter, symbol}, listReaderInterface});
380380
}
381381

382382
protected:

‎cpp/csp/adapters/parquet/ParquetReaderColumnAdapter.h‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ class MissingColumnAdapter : public ParquetColumnAdapter
134134

135135
bool isMissingColumn() const override{ return true; }
136136

137-
virtual CspTypePtr getNativeCspType() const
137+
virtual CspTypePtr getNativeCspType() const override
138138
{
139139
CSP_THROW( csp::RuntimeException, "Trying to get native type of a missing column " << getColumnName() );
140140
}
@@ -190,8 +190,7 @@ template< typename ValueType, typename ArrowArrayType >
190190
class NativeTypeColumnAdapter : public BaseTypedColumnAdapter<ValueType, ArrowArrayType>
191191
{
192192
public:
193-
using BASE = BaseTypedColumnAdapter<ValueType, ArrowArrayType>;
194-
using BASE::BaseTypedColumnAdapter;
193+
using BaseTypedColumnAdapter<ValueType, ArrowArrayType>::BaseTypedColumnAdapter;
195194
virtual CspTypePtr getNativeCspType() const override {return CspType::fromCType<ValueType>::type();}
196195

197196
protected:

‎cpp/csp/adapters/utils/JSONMessageWriter.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ class JSONMessageWriter : public MessageWriter
6363
}
6464

6565
private:
66-
void processTickImpl( const OutputDataMapper & dataMapper, const TimeSeriesProvider * sourcets )
66+
void processTickImpl( const OutputDataMapper & dataMapper, const TimeSeriesProvider * sourcets ) override
6767
{
6868
dataMapper.apply( *this, sourcets );
6969
}

0 commit comments

Comments
 (0)