-
Notifications
You must be signed in to change notification settings - Fork 31
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
Provide standard build system/script to/and facilitate standard packaging #27
Comments
I, on the contrary, like conda a lot and it saved me months of work by now. My advice is to use pure local install and do not do any magic As stated in the README, if you want to build locally you'll need to understand what is in the python -m bindgen -n $n_cores parse ocp.toml out.pkl
python -m bindgen -n $n_cores transform ocp.toml out.pkl out_f.pkl
python -m bindgen -n $n_cores generate ocp.toml out_f.pkl
cmake -B build -S $OCP_src -G Ninja
cmake --build build -j $n_cores -- -k 0 Development in this repo boils down to modifying |
Hello, I'm running without conda. Not sure i will get something out one day....
Anyway, it looks like some OCCT headers are not 'compiled' well and it's disturbing. You flagged them Warnings so I think it should produce something in the end.
|
@grawp , ok, .OCP full of new c++ and the CMakelists.txt Next step: get something out of Cmake/make |
@Franck78 Yes, I've started working on that although just very recently so don't expect results tomorrow :D
|
good. for now I run basic, run.sh is from pywrap, notice I force 3 time 'python3':
with
CMakeLists.j2: force python3. Why is my opensuse still installing python2 and making it 'default'......
FindOpenCascade.cmake: headers are in /usr/local/include/opencascade after a local build of OCCT 7.4
as you saw in previous report, some warnings during the * python3 -m bindgen -n 7 -v transform* (I think) |
probably consequence of warnings in phase 2 (bindgen transform)
|
@grawp improvements are welcome. If you want to contribute I'd have some suggestions:
|
@Franck78 you can compare the results with the azure CI run (there are warnings being generated there as well). Intermediate results are available as pipeline artifacts. |
"with the azure CI run" sometime I understand, sometimes it is pure chineese. ;) Anyway, final result of " cmake --build build -j1 -- -k 0" is clear
If you have that(OCP) produced under the conda env, then, some kind of include file is bad between gcc clang python3 on openSuse. |
I meant, here you have the intermediate results: https://dev.azure.com/cadquery/OCP/_build/results?buildId=820&view=artifacts&type=publishedArtifacts |
python -m bindgen -n 7 -v generate ocp.toml out_f.pkl ok, i have ~ the same errors/warnings at the beginning. That doesn't bother you ? Outputs are not the same between my thing and azure. There is no 'bindgen' module in the OCT git repository |
Feel free to improve things here.
Pywrap is a submodule of this repo, not sure what you mean here. |
Oh, I'm sure you tried before me to fix them. Not a fan a c c++ and the includes headers and the syntax . It make no sense to me. Why would the thing process hundred of headers and suddenly decree "I know nothing about
grep (on the OCCT headers) returns tons of use of this name. Why one error. |
I hate it more! ;-) Too bad it seems we can't convince @adam-urbanczyk to hate it with us! Feel free to use my PKGBUILD as a reference for your noble unconda packaging efforts! |
@greyltc I've already used your PKGBUILD as a reference! It helped me understand how the OCP should be build. Btw. @adam-urbanczyk Are you aware that |
Great. I'm glad you're aware of it. The fewer people reinventing wheels, the better! |
@greyltc @grawp I'm very much against using language like "hate","garbage","perverted" with respect to other people’s hard work. If you don't like conda, don't use it. If there are ways to improve OCP or pywrap please open a PR (see my comment above for details). @grawp if the the cmake file is wrong, then don't use it. It does produce a working result for me. I'd rather expose the paths as an command line parameter to pywrap than make it call cmake. |
@adam-urbanczyk Probably a ton of stuff can be copied/learned from the pythonocc-core build system. As I understand it, from a 50,000ft view, that project does the exact same thing this project does: it makes python bindings for OCCT. I know there are some major differences going on under the hood with pybind and maybe it's naive to think that this project could have a similar build interface.
cmake there does magic to figure everything out. For a properly set up system, that works fine. However, locations for some important hard to find things can be overridden if needed by feeding parameters to cmake like this: I can imagine that there is a bunch of duplicated effort between here and pythonocc-core. I think historically they've been generating bindings to their intermediate fork of OCCT, so they've been really slow on keeping up with releases/features since they had so many moving parts, but it seems that they've now cut that all out and are binding to the same code base you're binding to here so they might now be a bit more agile than they were in the past. I wonder if it's worth revisiting a collaboration there. Of course I understand that each binding is meant to be in support of the owners' own projects, so maybe this sort of collaboration is not workable. |
Everything what @greyltc said.
Maybe I've misunderstood so please forgive me for possibly unnecessarily stating that:
Azure pipelines would be of course able to call pywrap directly without the cmake as well as call the cmake with parameters specified using |
@greyltc just to be clear, I don't think conda is to be blamed for anything. I'd never be able to set up all this without it (and conda-forge) given the time I have.
I'm very well aware of the existence of PythonOCC. The problem is not inspiration or knowledge (i.e. how), it's rather the resources (i.e. when). If you want to contribute said universal CMakeLists.txt for this project I'll merge it, but don't expect me to work on it anytime soon. @grawp we are on the same page. I'll try to get rid of the hardcoded paths in pywrap via a command line option. |
You might be suprised by how much improvement the CI tools on GitHub and GitLab have seen recently! They're pretty powerful and easy to use these days.
Knowing you'd be willing to merge something like that is great. |
You can now specify libclang location and headers as command line parameters. Additionally pywrap is pip-installable and provides a
|
Nice! I'll try this out on my build shortly |
FYI: I found some issues with the code, but all should be fixed in #37 . |
Thanks @adam-urbanczyk, is there a CI build script or something you can point me to that uses the new system that I can use as a reference for updating my packaging? |
This should do on the latest master (of pywrap):
Of course depending on the env you might need to use the additional |
I've given this a shot on the latest master branch here (using the pywrap submodule as configured). I think I'm pretty close to getting the build to work, but I don't quite have it working. Part of the problem I'm having is that I'm not 100% confident that I'm choosing the correct headers and ocp.toml to use. Today I seem to have the following choices for headers: (1) /usr/include/opencascade Those in (1) are the official headers on my system as shipped with the occt 7.5.0 release tarball. None of these three choices for headers agree with ether of the two others. Furthermore, I have the ocp.toml provided in the pywrap repo and that provided in this repo to choose between. Please correct me if I'm wrong: If I do nothing, it seems the headers that the various build steps use get chosen for me (I can't say I've really looked into exactly how they're chosen). I can override the headers chosen in the pywrap step using I get differeing build errors based on what choices I make for the inputs to the various build steps. @adam-urbanczyk If you can advise me on which headers and ocp.toml to use, then I'll post a more meaningful build error that I hope you can help me overcome! |
Please go for (3). For the generate step headers are specified in the ocp.toml file. For the rest you are spot on. |
Ok, here's what I get when I do it that way (abbreviated output): $ git clone https://github.com/CadQuery/OCP.git
$ cd OCP
$ git submodule update --init --recursive
$ export CONDA_PREFIX=/usr
$ export PYTHONPATH=pywrap
$ python -m bindgen --clean --libclang /usr/lib/libclang.so --include opencascade all ocp.toml
# ...
# [W 210111 20:37:08 translation_unit:47] ./opencascade/AppDef_BSpParFunctionOfMyBSplGradientOfBSplineCompute.hxx
# [W 210111 20:37:08 translation_unit:48] /../lib64/gcc/x86_64-pc-linux-gnu/10.2.0/../../../../include/c++/10.2.0/cstddef:50:10: fatal error: 'stddef.h' file not found
# ...success (? after hundreds/thousands of "warnings" that say "fatal error" as above) and an OCP folder is created
$ cmake -D OPENCASCADE_INCLUDE_DIR=opencascade -D CMAKE_BUILD_TYPE=Release -B build_dir -G Ninja -S OCP
-- The CXX compiler identification is GNU 10.2.0
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Found PythonInterp: /usr/bin/python (found version "3.9.1")
-- Found PythonLibs: /usr/lib/libpython3.9.so
-- Performing Test HAS_FLTO
-- Performing Test HAS_FLTO - Success
-- Found pybind11: /usr/include (found version "2.6.1" )
CMake Warning (dev) at /usr/share/cmake-3.19/Modules/FindPackageHandleStandardArgs.cmake:426 (message):
The package name passed to `find_package_handle_standard_args`
(OPENCASCADE) does not match the name of the calling package (OpenCascade).
This can lead to problems in calling code that expects `find_package`
result variables (e.g., `_FOUND`) to follow a certain pattern.
Call Stack (most recent call first):
FindOpenCascade.cmake:78 (find_package_handle_standard_args)
CMakeLists.txt:19 (find_package)
This warning is for project developers. Use -Wno-dev to suppress it.
-- Found OPENCASCADE: /usr/lib/libTKMath.so;/usr/lib/libTKernel.so;/usr/lib/libTKG2d.so;/usr/lib/libTKG3d.so;/usr/lib/libTKGeomBase.so;/usr/lib/libTKBRep.so;/usr/lib/libTKGeomAlgo.so;/usr/lib/libTKTopAlgo.so;/usr/lib/libTKPrim.so;/usr/lib/libTKShHealing.so;/usr/lib/libTKHLR.so;/usr/lib/libTKBO.so;/usr/lib/libTKBool.so;/usr/lib/libTKFeat.so;/usr/lib/libTKOffset.so;/usr/lib/libTKFillet.so;/usr/lib/libTKMesh.so;/usr/lib/libTKXMesh.so;/usr/lib/libTKXSBase.so;/usr/lib/libTKService.so;/usr/lib/libTKV3d.so;/usr/lib/libTKOpenGl.so;/usr/lib/libTKMeshVS.so;/usr/lib/libTKBin.so;/usr/lib/libTKBinL.so;/usr/lib/libTKBinTObj.so;/usr/lib/libTKCAF.so;/usr/lib/libTKCDF.so;/usr/lib/libTKLCAF.so;/usr/lib/libTKStd.so;/usr/lib/libTKStdL.so;/usr/lib/libTKTObj.so;/usr/lib/libTKVCAF.so;/usr/lib/libTKXml.so;/usr/lib/libTKXmlL.so;/usr/lib/libTKXmlTObj.so;/usr/lib/libTKIGES.so;/usr/lib/libTKSTEP.so;/usr/lib/libTKSTEP209.so;/usr/lib/libTKSTEPAttr.so;/usr/lib/libTKSTEPBase.so;/usr/lib/libTKSTL.so;/usr/lib/libTKXDESTEP.so;/usr/lib/libTKXCAF.so;/usr/lib/libTKBinXCAF.so;/usr/lib/libTKXmlXCAF.so;/usr/lib/libTKVRML.so;/usr/lib/libTKRWMesh.so
-- Configuring done
-- Generating done
-- Build files have been written to: OCP/build_dir
$ cmake --build build_dir -- -j1 > build.log.txt 2>&1 build.log.txt attached. |
Looks like you need to specify the correct include location for clang, something like (NB: made up path):
If you use one BTW: @greyltc here #33 there is more discussion on building on BSD. |
Thanks! That works! Here's what I have for the meat of my build commands now:
Should hopefully be pretty portable. |
I think you can even strip few more things now:
|
Ah, neat! I didn't realize I could get rid of CONDA_PREFIX. Thanks |
So is the current solution standard enough, i.e. is the issue resolved? BTW: I'm still thinking about adding the possibility to override the CMakeLists template |
I think the new cmake stuff in this repo is great and this issue can be closed here in my opinion. Thank you! It would be a good improvement if Do you think pywrap deserves the same or similar cmake treatment? I discovered CadQuery/pywrap#31 in my recent builds and thought that I noticed a few hardcoded paths and library and platform discovery logic in pywrap/bindgen/utils.py that might be better done by a tool like cmake. What do you think? Would it be appropriate for the pywrap call to be rolled into OCP's cmake invocation? Should pywrap have its own? |
It could be done using https://cmake.org/cmake/help/latest/command/execute_process.html but for now I don't want to spend time on writing something that likely will not be generic. I was wrong regarding the custom CMakeLists template - it is already supported. Regarding CadQuery/pywrap#31 I think you are wrong - as long as you provide libcalng path explicitly via |
I'm seeing the following on 6b105a3 (which is configured to pull in CadQuery/pywrap@367eb8d as a submodule)
v.s.
I assume you can recreate the issue by calling |
Thanks, you are right! I solved it in CadQuery/pywrap@420c92f , will pull this into OCP later. |
Solved |
I absolutely hate conda package manager. Not only it does not play well with existing OS packages whether binary or python, it also by default pollutes or even break one's environment in a very perverted way which is exactly what at least one other user stated in CadQuery/cadquery#153 .
Having said that I decided to try to improve the experience for myself and perhaps for other people as well by creating ebuilds for Gentoo and probably also rpms for Fedora etc..
But after examining this repository I'm confused at how to even build it. If it was a standard conda package as far as I understand there would be a
build.sh
with build steps or calls to another build system.But all commands which seem to be doing something useful are in
azure-pipelines.yml
andbuild-bindings-job.yml
? How does this work? In order to build it you send it to some Microsoft machine?! How do you even develop in such repo then?Would it be possible to use some normal build system like Make, Cmake or some other so one can then easily integrate it into OS specific packaging solutions?
The text was updated successfully, but these errors were encountered: