Bug 2248911 - Review Request: neon2sse - Open standard for machine learning interoperability
Summary: Review Request: neon2sse - Open standard for machine learning interoperability
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Simone Caronni
QA Contact: Fedora Extras Quality Assurance
URL: https://github.com/intel/ARM_NEON_2_x...
Whiteboard:
Depends On:
Blocks: ML-SIG
TreeView+ depends on / blocked
 
Reported: 2023-11-09 16:39 UTC by Ali Erdinc Koroglu
Modified: 2024-10-01 06:02 UTC (History)
3 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2024-09-17 11:25:12 UTC
Type: ---
Embargoed:
negativo17: fedora-review+


Attachments (Terms of Use)
The .spec file difference from Copr build 6616576 to 7428443 (2.97 KB, patch)
2024-05-09 07:53 UTC, Fedora Review Service
no flags Details | Diff

Description Ali Erdinc Koroglu 2023-11-09 16:39:50 UTC
SPEC Url: https://download.copr.fedorainfracloud.org/results/aekoroglu/fedora/fedora-rawhide-x86_64/06616531-neon2sse/neon2sse.spec
SRPM Url: https://download.copr.fedorainfracloud.org/results/aekoroglu/fedora/fedora-rawhide-x86_64/06616531-neon2sse/neon2sse-0.0%5E20230131git097a5eca-1.fc40.src.rpm

Description:
The platform independent header allowing to compile any C/C++ code containing
ARM NEON intrinsic functions for x86 target systems using SIMD up to SSE4
intrinsic functions

Reproducible: Always

Comment 1 Fedora Review Service 2023-11-09 16:46:23 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/6616576
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2248911-neon2sse/fedora-rawhide-x86_64/06616576-neon2sse/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.

Comment 2 Tom Rix 2023-12-17 15:04:39 UTC
> %global with_snapshot 1 

Clean this up, there should be only 1 way to get the source, this is a messy upstream project, make which git commit you fetch as easy as possible.

> Summary:        Open standard for machine learning interoperability 

This summary does not match the project.
Find a way to simplify

The platform independent header allowing to compile any C/C++ code containing ARM NEON intrinsic functions for x86 target systems using SIMD up to SSE4 intrinsic functions

> %global desc %{expand: \

I am not a fan of expanding and, the description should be close to the readme text

The NEON_2_SSE.h file is intended ...

Since it likely does not make sense to install this header on a non x86, likely you need some logic like

BuildArch: noarch
ExclusiveArch: x86_64

It would be good to have a %check
Can you do something like

https://src.fedoraproject.org/rpms/xbyak_aarch64/blob/rawhide/f/xbyak_aarch64.spec#_88

> Requires:       %{name}%{?_isa} = %{version}-%{release}

There is only the -devel package, without the normal package, this causes a failure to install. Remove this line.

Comment 4 Fedora Review Service 2024-05-09 07:53:02 UTC
Created attachment 2032272 [details]
The .spec file difference from Copr build 6616576 to 7428443

Comment 5 Fedora Review Service 2024-05-09 07:53:04 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/7428443
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2248911-neon2sse/fedora-rawhide-x86_64/07428443-neon2sse/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.

Comment 6 Simone Caronni 2024-09-12 08:24:46 UTC
Hi Ali,

there are no releases for the project, so the correct syntax for the snapshot should be as follows: https://docs.fedoraproject.org/en-US/packaging-guidelines/Versioning/#_snapshots

TL;DR: Remove the "with_snapshot" conditional and use this:

Version:	0.0^%{gitdate}git%{shortcommit}

These are up to you if you want:

Shorten the %shortcommit to 7 characters:
%global shortcommit %(c=%{commit}; echo ${c:0:7})

Move the test.c content into a single file that you check in with the SPEC file, I think it makes the SPEC file more readable.

Otherwise, with the changes you added based on Trix comments, the package is fine.

** Please change the version before committing **, otherwise approved!

Thanks.

Comment 7 Fedora Admin user for bugzilla script actions 2024-09-17 08:28:36 UTC
The Pagure repository was created at https://src.fedoraproject.org/rpms/neon2sse

Comment 8 Ali Erdinc Koroglu 2024-09-17 11:25:12 UTC
Thank you @negativo17


Note You need to log in before you can comment on or make changes to this bug.