Skip to content
Open
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
11 changes: 9 additions & 2 deletions dltpy/native/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,22 @@ target_include_directories(dltreader_native
${PYTHON_INCLUDE_DIR}
)
target_link_libraries(dltreader_native
${PYTHON_LIBRARY}
${PYTHON_LIBRARIES}
fmt::fmt
pybind11::pybind11
)

if(WIN32)
Copy link
Owner

Choose a reason for hiding this comment

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

Have you tried using the pybind11_add_module macro from pybind11? https://pybind11.readthedocs.io/en/master/compiling.html#pybind11-add-module

It could be that it will solve the problem without adding platform-specific code here (I'd try it myself, but can't test it for windows at all...).

set(suffix ".pyd")
else()
set(suffix ".so")
endif()

set_target_properties(
dltreader_native
PROPERTIES
PREFIX ""
SUFFIX ".so"
SUFFIX ${suffix}
OUTPUT_NAME dltreader_native
LINKER_LANGUAGE C
)
12 changes: 5 additions & 7 deletions dltpy/native/dltreader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@
#include "bitmasks.h"
#include "parsers.h"
#include "headers.h"
#include <fcntl.h>
#include <unistd.h>
#include <cassert>
#include <vector>
#include "dltreader.h"
Expand Down Expand Up @@ -80,15 +78,15 @@ DltReader::DltReader(bool expectStorage)

std::string DltReader::str(){
return fmt::format("DltReader(buf={:p}, msg={:p}, data end={:p})",
iBuffer.begin(),
Copy link
Owner

Choose a reason for hiding this comment

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

Oh man, indeed, the iterators are not guaranteed to be pointers. I wonder if some linter could've found this (it's not even a warning on linux&osx).

iBuffer.data(),
iMessageBegin,
iDataEnd
);
}

void DltReader::selfcheck(){
assert(iMessageBegin >= iBuffer.begin() && iMessageBegin <= iDataEnd);
assert(iDataEnd >= iBuffer.begin() && iDataEnd <= iBuffer.end());
assert(iMessageBegin >= iBuffer.data() && iMessageBegin <= iDataEnd);
assert(iDataEnd >= iBuffer.data() && iDataEnd <= &(*iBuffer.end()));
}

off_t DltReader::dataLeft(){
Expand All @@ -107,8 +105,8 @@ void DltReader::consumeMessage(){

void DltReader::flush(){
selfcheck();
auto shift = iMessageBegin - iBuffer.begin();
iDataEnd = std::copy(iMessageBegin, iDataEnd, iBuffer.begin());
auto shift = iMessageBegin - iBuffer.data();
iDataEnd = std::copy(iMessageBegin, iDataEnd, iBuffer.data());
iCursor -= shift;
iMessageBegin -= shift;
selfcheck();
Expand Down
9 changes: 5 additions & 4 deletions dltpy/native/dltreader.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
// along with this program. If not, see <http://www.gnu.org/licenses/>.

#include "headers.h"
#include <stdexcept>
#include <vector>
#include <optional>
#include <tuple>
Expand Down Expand Up @@ -48,9 +49,9 @@ class DltReader{

std::array<char, 8196> iBuffer;

char* iDataEnd{iBuffer.begin()};
char* iCursor{iBuffer.begin()};
char* iMessageBegin{iBuffer.begin()};
char* iDataEnd{iBuffer.data()};
char* iCursor{iBuffer.data()};
char* iMessageBegin{iBuffer.data()};

off_t iBasicOffset{0};
off_t iPayloadOffset{0};
Expand All @@ -75,7 +76,7 @@ class DltReader{
void consumeMessage();
void flush();

std::tuple<char*, size_t> getBuffer(){flush(); return {iDataEnd, iBuffer.end() - iDataEnd};}
std::tuple<char*, size_t> getBuffer(){flush(); return {iDataEnd, &(*iBuffer.end()) - iDataEnd};}
void updateBuffer(size_t len){iDataEnd += len;}

const StorageHeader& getStorage() const{
Expand Down
2 changes: 1 addition & 1 deletion dltpy/native/dltreader_native.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ namespace py=pybind11;
template<size_t N>
auto pyArrayStringToBytes(const std::array<char, N>& arr){
off_t len = std::find(arr.begin(), arr.end(), '\0') - arr.begin();
return py::bytes(arr.begin(), len);
return py::bytes(arr.data(), len);
}

template<size_t N>
Expand Down
73 changes: 42 additions & 31 deletions setup.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
#! /usr/bin/env python3
from setuptools import setup, find_packages, Extension
from setuptools.command.build_ext import build_ext
from pathlib import Path
import os
import sys
import pathlib
Copy link
Owner

Choose a reason for hiding this comment

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

This pathlib. everywhere is nothing but redundant. In this context we don't have anything even coming close to have name Path ambiguous.

import shutil

class CMakeExtension(Extension):

Expand All @@ -17,49 +17,60 @@ class build_ext_cmake(build_ext):
def run(self):
for ext in self.extensions:
self.build_cmake(ext)
super().run()

def build_cmake(self, ext):
cwd = Path().absolute()
def build_cmake(self, extension):

# these dirs will be created in build_py, so if you don't have
# any python sources to bundle, the dirs will be missing
build_temp = Path(self.build_temp)
build_temp.mkdir(parents=True, exist_ok=True)
extdir = Path(self.get_ext_fullpath(ext.name))
extdir.mkdir(parents=True, exist_ok=True)
build_dir = pathlib.Path(self.build_temp)

extension_path = pathlib.Path(self.get_ext_fullpath(extension.name))

os.makedirs(build_dir, exist_ok=True)
os.makedirs(extension_path.parent.absolute(), exist_ok=True)

# Now that the necessary directories are created, build

self.announce("Configuring cmake project", level=3)
Copy link
Owner

Choose a reason for hiding this comment

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

Nice logs =)


# Change your cmake arguments below as necessary
# Below is just an example set of arguments for building Blender as a Python module

# example of cmake args
config = 'Debug' if self.debug else 'Release'
cmake_args = [
'-DCMAKE_LIBRARY_OUTPUT_DIRECTORY=' + str(extdir.parent.absolute()),
'-DCMAKE_BUILD_TYPE=' + config,
'-DPYBIND11_PYTHON_VERSION=%s.%s' % (sys.version_info.major, sys.version_info.minor),
]

# example of build args
build_args = [
'--config', config,
'--', '-j4'
]

os.chdir(str(build_temp))
self.spawn(['cmake', str(cwd)] + cmake_args)
if not self.dry_run:
self.spawn(['cmake', '--build', '.'] + build_args)
os.chdir(str(cwd))
self.spawn(['cmake', '-H'+str(pathlib.Path().absolute()), '-B'+self.build_temp,
Copy link
Owner

Choose a reason for hiding this comment

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

This .Path() looks a bit weird. Why is it needed?

'-DCMAKE_BUILD_TYPE=' + config,
'-DPYBIND11_PYTHON_VERSION=%s.%s' % (sys.version_info.major, sys.version_info.minor)])

self.announce("Building binaries", level=3)

self.spawn(['cmake', '--build', self.build_temp,
'--config', config])

# Build finished, now copy the files into the copy directory
# The copy directory is the parent directory of the extension (.pyd)

self.announce("Moving built python module", level=3)

bin_dir = os.path.join(build_dir, 'dltpy', 'native')
self.distribution.bin_dir = bin_dir

pyd_path = [os.path.join(bin_dir, _pyd) for _pyd in
Copy link
Owner

Choose a reason for hiding this comment

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

Maybe use pathlib? It should make this part much shorter

Copy link
Owner

Choose a reason for hiding this comment

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

Also, pyd is a pretty weird name for a variable containing a path to a library.

Copy link
Author

Choose a reason for hiding this comment

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

Its probably because the libraries have the ending .pyd on Windows.

os.listdir(bin_dir) if
os.path.isfile(os.path.join(bin_dir, _pyd)) and
os.path.splitext(_pyd)[0].startswith('dltreader_native') and
os.path.splitext(_pyd)[1] in [".pyd", ".so"]][0]

shutil.move(pyd_path, extension_path)

setup(
name='dltpy',
version='0.3.6.6',
description='DLT log reader',
long_description=Path('README.md').read_text(),
long_description=pathlib.Path('README.md').read_text(),
long_description_content_type="text/markdown",
author='Vladimir Shapranov',
author_email='equidamoid@gmail.com',
url='https://github.com/Equidamoid/dltpy',
packages=find_packages(),
ext_modules=[CMakeExtension('dltpy/native/native_dltfile')],
ext_modules=[CMakeExtension('dltpy/native/dltreader_native')],
cmdclass={'build_ext': build_ext_cmake,},
entry_points={
'console_scripts': [
Expand Down