https://bugzilla.redhat.com/show_bug.cgi?id=2394440 Bruno Postle <bruno@xxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |bruno@xxxxxxxxxx --- Comment #2 from Bruno Postle <bruno@xxxxxxxxxx> --- I'm not able to do a full review, but I'm very familiar with this software and maintain copr snapshots: https://copr.fedorainfracloud.org/coprs/bpostle/IfcOpenShell/ Some notes: 1. The git clone command is pulling a snapshot of the v0.8.0 branch HEAD, but this is just a branch name. There are tagged stable releases within this branch: bonsai-0.8.0, bonsai-0.8.1, bonsai-0.8.2, bonsai-0.8.3 (these tags include 'bonsai-' because bonsai and IfcOpenShell have synchronised releases, the current stable release of IfcOpenShell is 0.8.3). 2. The spec file works with a tar'd git clone, but there is a cmake package_source target that produces a tarball using the correct version number: cd IfcOpenShell && rm -rf build && mkdir build cd build && cmake ../cmake/ -DEXTRA_VERSION= \ -DADD_COMMIT_SHA=ON -DVERSION_OVERRIDE=ON \ -DEIGEN_DIR=/usr/include/eigen3 \ -DCPACK_SOURCE_IGNORE_FILES="/test/;/src/opencdeserver/;/src/bonsai/scripts/" \ -DCOLLADA_SUPPORT=OFF -DGLTF_SUPPORT=OFF -DCITYJSON_SUPPORT=OFF -DGMP_LIBRARY_DIR=/usr/lib64 -DMPFR_LIBRARY_DIR=/usr/lib64 make package_source 3. The build command has -DHDF5_LIBRARY_DIR=%{_libdir}, but there is no BuildRequires: hdf5-devel 4. The `make install` command installs the c++ and python bindings, but the python bindings don't install dist-info metadata (this is arguably a bug in the CMakeLists.txt). However the python module has a working pyproject.toml file, so you should be able to get a full installation of the python module using standard spec macros: %build %pyproject_wheel %install %pyproject_install %pyproject_save_files IfcOpenShell %files -n python3-IfcOpenShell -f %{pyproject_files} 5. Similarly there is a collection of python libraries for working with IFC data that can be installed using their pyproject.toml configuration, these should be installed too (they could be sub-packages but they are not very big so this is up to you): bcf bsdd ifc4d ifc5d ifcbimtester ifccityjson ifcclash ifccsv ifcdiff ifcfm ifcpatch ifctester 6. Looking at your patch file: a. You shouldn't have to mess with the xml2 path, I build successfully with -DLIBXML2_INCLUDE_DIR:FILEPATH=%{_includedir}/libxml2 b. Renaming the Serializers library to IfcSerializers seems likely to cause problems. Unless there is actually a conflict with an existing fedora package I wouldn't want this. c. Similarly svgfill is renamed IfcSvgFill, but this library is specific to IfcOpenShell, so isn't going to conflict with any existing libraries called svgfill. 7. I'm not sure the Qt viewer is supported, have you tested it? 8. As discussed previously, I agree that the Bonsai Blender add-on shouldn't be shipped. This is because it is undergoing rapid development and most users are better served by getting a nightly snapshot through the Blender Extensions menu (though note this doesn't work with the fedora blender because official Extensions are all python-3.11), or by using my copr repo (which does work with the fedora blender). That's it for now, any questions please ask. -- You are receiving this mail because: You are always notified about changes to this product and component You are on the CC list for the bug. https://bugzilla.redhat.com/show_bug.cgi?id=2394440 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202394440%23c2 -- _______________________________________________ package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue
