SPEC Url: https://download.copr.fedorainfracloud.org/results/aekoroglu/fedora/fedora-rawhide-aarch64/06698631-openvino/openvino.spec SRPM Url: https://download.copr.fedorainfracloud.org/results/aekoroglu/fedora/fedora-rawhide-aarch64/06698631-openvino/openvino-2023.2.0-1.fc40.src.rpm Description: OpenVINO is an open-source toolkit for optimizing and deploying AI inference. It can be used to develop applications and solutions based on deep learning tasks, such as: emulation of human vision, automatic speech recognition, natural language processing, recommendation systems, etc. Reproducible: Always
Copr build: https://copr.fedorainfracloud.org/coprs/build/6700410 (failed) Build log: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2251785-openvino/srpm-builds/06700410/builder-live.log.gz Please make sure the package builds successfully at least for Fedora Rawhide. - If the build failed for unrelated reasons (e.g. temporary network unavailability), please ignore it. - If the build failed because of missing BuildRequires, please make sure they are listed in the "Depends On" field --- 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.
On first look, the big problem is the third_party/ dir in the tarball. These should should be their own packages. If they are not needed, then the third_party dir should be rm-ed in %prep
What I mean is these third party packages need to be their own packages. Please see https://bugzilla.redhat.com/show_bug.cgi?id=2254599 this removes the openvino/thirdparty/json dependency. And if you have the time, please review.
Hello Tom, I disabled them in the patch file, but do you want me to delete them instead? --- a/CMakeLists.txt 2023-11-22 08:22:33.730665512 +0200 +++ b/CMakeLists.txt 2023-11-24 00:34:16.675363131 +0200 @@ -38,7 +38,7 @@ include(cmake/features.cmake) # These options are shared with 3rdparty plugins by means of developer package -include(cmake/dependencies.cmake) +# include(cmake/dependencies.cmake) if(ENABLE_COVERAGE) include(cmake/coverage.cmake) @@ -133,7 +133,7 @@ include(cmake/test_model_zoo.cmake) endif() -include(thirdparty/dependencies.cmake) +# include(thirdparty/dependencies.cmake) add_subdirectory(src) if(ENABLE_SAMPLES OR ENABLE_TESTS) @@ -146,10 +146,10 @@ endif() include(cmake/extra_modules.cmake) -add_subdirectory(docs) -add_subdirectory(tools) -add_subdirectory(scripts) -add_subdirectory(licensing) +# add_subdirectory(docs) +# add_subdirectory(tools) +# add_subdirectory(scripts) +# add_subdirectory(licensing) if(ENABLE_TESTS) # layers and other more high-level / e2e tests --- a/src/CMakeLists.txt 2023-11-22 08:23:48.722438766 +0200 +++ b/src/CMakeLists.txt 2023-11-22 08:25:27.304120882 +0200 @@ -8,7 +8,9 @@ ov_add_compiler_flags(-Wmissing-declarations) endif() -include(cmake/install_tbb.cmake) +# dont install_tbb +# include(cmake/install_tbb.cmake) +include(cmake/ov_parallel.cmake) # CC library should be registered before other cc targets add_subdirectory(common) --- a/src/plugins/intel_cpu/thirdparty/CMakeLists.txt 2023-11-22 08:35:23.572344257 +0200 +++ b/src/plugins/intel_cpu/thirdparty/CMakeLists.txt 2023-11-22 08:35:40.634293194 +0200 @@ -115,7 +115,7 @@ list(APPEND CMAKE_MODULE_PATH "${intel_cpu_thirdparty_SOURCE_DIR}") endif() - add_subdirectory(onednn EXCLUDE_FROM_ALL) + # add_subdirectory(onednn EXCLUDE_FROM_ALL) # install static libraries ov_install_static_lib(dnnl ${OV_CPACK_COMP_CORE})
Unwinding third_party can be complicated. To prove you do not needed it, remove it in prep Like https://src.fedoraproject.org/rpms/python-torch/blob/rawhide/f/python-torch.spec#_178 I was really only looking at the toplevel thirdparty/ , but it looks like from above that they are also in the plugins.
Hello Tom, thank you for the comments. SPEC Url: https://download.copr.fedorainfracloud.org/results/aekoroglu/fedora/fedora-rawhide-x86_64/06766692-openvino/openvino.spec SRPM Url: https://download.copr.fedorainfracloud.org/results/aekoroglu/fedora/fedora-rawhide-x86_64/06766692-openvino/openvino-2023.2.0-1.fc40.src.rpm
Copr build: https://copr.fedorainfracloud.org/coprs/build/6766923 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2251785-openvino/fedora-rawhide-x86_64/06766923-openvino/fedora-review/review.txt Please take a look if any issues were found. --- 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.
At least the licensecheck seems to be missing from the above auto review. Spec looks generally fine. This line should not be needed. -DCMAKE_CXX_FLAGS="%{optflags} -Wformat -Wformat-security" \ There is also at least third party for src/bindings/python/thirdparty src/plugins/intel_gpu/thirdparty That needs to be removed. I think the two small plugin subpackages should be rolled into the the main package. There should be a way, even a manual way to test. The samples/ would be good to include into the devel package. Can python be enabled ? python is widely used in AI so this would be good.
Hello Tom, it's been a while sorry about it. Python API, PyTorch + IR frontend supports are enabled and tested SPEC Url: https://download.copr.fedorainfracloud.org/results/aekoroglu/fedora/fedora-rawhide-x86_64/07399413-openvino/openvino.spec SRPM Url: https://download.copr.fedorainfracloud.org/results/aekoroglu/fedora/fedora-rawhide-x86_64/07399413-openvino/openvino-2024.0.0-1.fc41.src.rpm Known issues - onednn (opevino forked version) and intel-mlas bundled to compile intel_cpu_plugin - onnx disabled (requested version of Google Protobuf is 3.20.3) - aarch64 disabled (https://github.com/ARM-software/ComputeLibrary not packed yet) - 2024.1.0 version has some more thirdparty deps. PS: I added CXX_FLAGS="%{optflags} -Wformat -Wformat-security" to fix src/bindings/c/src/ov_dimension.cpp compilation error cc1plus: error: ‘-Wformat-security’ ignored without ‘-Wformat’ [-Werror=format-security] Manual test results: [aekoroglu@test samples]$ python sync_benchmark.py /home/aekoroglu/models/public/googlenet-v1/FP32/googlenet-v1.xml [ INFO ] OpenVINO: [ INFO ] Build ................................. 2024.0.0-000-- [ INFO ] Count: 1856 iterations [ INFO ] Duration: 10000.79 ms [ INFO ] Latency: [ INFO ] Median: 4.91 ms [ INFO ] Average: 5.39 ms [ INFO ] Min: 4.46 ms [ INFO ] Max: 15.30 ms [ INFO ] Throughput: 185.59 FPS [aekoroglu@test samples]$ python throughput_benchmark.py /home/aekoroglu/models/public/googlenet-v1/FP32/googlenet-v1.xml [ INFO ] OpenVINO: [ INFO ] Build ................................. 2024.0.0-000-- [ INFO ] Count: 2893 iterations [ INFO ] Duration: 10017.42 ms [ INFO ] Latency: [ INFO ] Median: 13.50 ms [ INFO ] Average: 13.71 ms [ INFO ] Min: 11.57 ms [ INFO ] Max: 28.85 ms [ INFO ] Throughput: 288.80 FPS https://github.com/openvinotoolkit/openvino/tree/2024.0.0/samples/python/benchmark/sync_benchmark https://github.com/openvinotoolkit/openvino/tree/2024.0.0/samples/python/benchmark/throughput_benchmark
Created attachment 2031038 [details] The .spec file difference from Copr build 6766923 to 7400792
Copr build: https://copr.fedorainfracloud.org/coprs/build/7400792 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2251785-openvino/fedora-rawhide-x86_64/07400792-openvino/fedora-review/review.txt Please take a look if any issues were found. --- 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.
License: Apache-2.0 [ ]: License field in the package spec file matches the actual license. Note: Checking patched sources after %prep for licenses. Licenses found: "Unknown or generated", "Apache License 2.0", "*No copyright* Apache License 2.0", "BSD 3-Clause License", "MIT License", "Apache License 2.0 and/or Intel Open Source License", "MIT License and/or zlib License", "Apache License", "*No copyright* MIT License", "*No copyright* Apache License 2.0 and/or MIT License", "GNU General Public License, Version 2", "BSD 3-Clause License and/or GNU General Public License, Version 2", "Apache License 2.0 and/or MIT License", "Apache License 2.0 and/or Boost Software License 1.0", "Apache License 2.0 and/or BSD 3-Clause License", "Apache License 2.0 [generated file]", "Apache License 2.0 and/or Historical Permission Notice and Disclaimer". 5539 files have unknown license. Detailed output of licensecheck in /home/trix/reviews/review-openvino/licensecheck.txt Your license: file is not complete. Since you have bundled components, go into detail which of these licenses are for the main package and which are for the others. [ ]: Package requires other packages for directories it uses. Note: No known owner of /usr/lib64/python3.12/site-packages, /usr/lib64/python3.12, /usr/lib64/pkgconfig, /usr/lib64/openvino-2024.0.0, /usr/lib64/cmake Need to add %dir's here ex/ %dir %_libdir/openvino-2024.0.0 ExclusiveArch: x86_64 %ifarch x86_64 BuildRequires: xbyak-devel %endif This ifarch check is not needed. Provides: bundled(onednn) should have a bundled(onednn) == version similar for mlas Requires: lib%{name}-ir-frontend = %{version} Requires: lib%{name}-pytorch-frontend = %{version} Recommends: %{name}-auto-plugin = %{version} Recommends: %{name}-auto-batch-plugin = %{version} Recommends: %{name}-hetero-plugin = %{version} Recommends: %{name}-intel-cpu-plugin = %{version} This is an overly complicated use of subpackages, most (all?) have a single *.so Reduce the subpackages to just the main, python and devel or make a strong case for each. -DCMAKE_CXX_FLAGS="%{optflags} -Wformat -Wformat-security" \ Should be redundant, why is this needed ? -python package seems to missing its distinfo/egg dir. The should be a %check or some option -test subpackage.
@trix : I fixed the licensing issues, converted the sub-plugin packages into one, resolved the Python packaging issue, and added tests. Ps: CXX_FLAGS="%{optflags} -Wformat -Wformat-security" added to fix src/bindings/c/src/ov_dimension.cpp compilation error SPEC Url: https://download.copr.fedorainfracloud.org/results/aekoroglu/fedora/fedora-rawhide-x86_64/07719158-openvino/openvino.spec SRPM Url: https://download.copr.fedorainfracloud.org/results/aekoroglu/fedora/fedora-rawhide-x86_64/07719158-openvino/openvino-2024.2.0-1.fc41.src.rpm
Need to add %dir's here ex/ %dir %_libdir/openvino-2024.0.0 Sorry forget this fix SPEC Url: https://download.copr.fedorainfracloud.org/results/aekoroglu/fedora/fedora-rawhide-x86_64/07719764-openvino/openvino.spec SRPM Url: https://download.copr.fedorainfracloud.org/results/aekoroglu/fedora/fedora-rawhide-x86_64/07719764-openvino/openvino-2024.2.0-1.fc41.src.rpm
Created attachment 2039354 [details] The .spec file difference from Copr build 7400792 to 7720382
Copr build: https://copr.fedorainfracloud.org/coprs/build/7720382 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2251785-openvino/fedora-rawhide-x86_64/07720382-openvino/fedora-review/review.txt Please take a look if any issues were found. --- 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.
Why is onednn bundled here? We have a onednn package in Fedora already, does it need to be updated to a newer version to make openvino happy?
This is an automatic action taken by review-stats script. The ticket reviewer failed to clear the NEEDINFO flag in a month. As per https://fedoraproject.org/wiki/Policy_for_stalled_package_reviews we reset the status and the assignee of this ticket.
The onednn bundling is fine here for now. It would be nice to see a way to try to build this with Fedora's onednn-devel instead, even if it doesn't work to do so yet. I hadn't caught the ExclusiveArch on the first pass. We should be able to build _some_ CPU support on non-x86, right?
We'll need to patch several components to demonstrate that Fedora's onednn-devel package will fail during the build. Are you sure you want to proceed with that? Yes, we should be able to build for arm64. But since the https://github.com/ARM-software/ComputeLibrary package is not available in our repository, I have added ExclusiveArch for x86_64.
I also submitted a package review request for the ARM Compute Library (https://bugzilla.redhat.com/show_bug.cgi?id=2316806) but I don't think that's a blocker at the moment.
Updated to 2024.5.0 SPEC Url: https://download.copr.fedorainfracloud.org/results/aekoroglu/fedora/fedora-rawhide-x86_64/08293571-openvino/openvino.spec SRPM Url: https://download.copr.fedorainfracloud.org/results/aekoroglu/fedora/fedora-rawhide-x86_64/08293571-openvino/openvino-2024.5.0-1.fc42.src.rpm
Created attachment 2059127 [details] The .spec file difference from Copr build 7720382 to 8295451
Copr build: https://copr.fedorainfracloud.org/coprs/build/8295451 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2251785-openvino/fedora-rawhide-x86_64/08295451-openvino/fedora-review/review.txt Please take a look if any issues were found. --- 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.
This doesn't build on F40 for missing dependencies, I'm okay with this for F41+.
The Pagure repository was created at https://src.fedoraproject.org/rpms/openvino