Bug 2394440

Copr build:
https://copr.fedorainfracloud.org/coprs/build/9544084
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2394440-ifcopenshell/fedora-rawhide-x86_64/09544084-IfcOpenShell/fedora-review/review.txt

Found issues:

- Unversioned so-files directly in %_libdir.
  Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_devel_packages
- License file LICENSE is not marked as %license
  Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuidelines/#_license_text

Please know that there can be false-positives.

---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

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.