Bug 2148993

Summary:	Review request: caffe - A deep learning framework
Product:	[Fedora] Fedora	Reporter:	Tom Rix <trix>
Component:	Package Review	Assignee:	Neal Gompa <ngompa13>
Status:	ASSIGNED ---	QA Contact:	Fedora Extras Quality Assurance <extras-qa>
Severity:	unspecified	Docs Contact:
Priority:	unspecified
Version:	rawhide	CC:	davide, ngompa13, package-review, pbrobinson
Target Milestone:	---	Flags:	ngompa13: fedora-review?
Target Release:	---
Hardware:	Unspecified
OS:	Unspecified
Whiteboard:
Fixed In Version:		Doc Type:	If docs needed, set a value
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:		Type:	Bug
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:
Bug Depends On:
Bug Blocks:	1011110

Description Tom Rix 2022-11-28 12:50:40 UTC

Spec URL: https://trix.fedorapeople.org/caffe.spec
SRPM URL: https://trix.fedorapeople.org/caffe-1.0-1.20200212git9b89154.fc38.src.rpm
FAS: trix
Description:

Caffe is a deep learning framework made with expression, speed,                                                                                                     
and modularity in mind. It is developed by Berkeley AI Research                                                                                                     
(BAIR) and by community contributors. Yangqing Jia created the                                                                                                      
project during his PhD at UC Berkeley.

Comment 1 Neal Gompa 2023-05-19 21:52:59 UTC

Taking this for review.

Comment 2 Neal Gompa 2023-05-19 22:14:58 UTC

Initial spec review:

> License:        BSD

This is no longer valid. Please use SPDX-based identifiers that match the project's license.

Cf. https://docs.fedoraproject.org/en-US/legal/license-field/
Cf. https://docs.fedoraproject.org/en-US/legal/all-allowed/

> Version:        1.0
> Release:        1.%{?date0}git%{?shortcommit0}%{?dist}

Please move the snapshot information to the Version: field.

Cf. https://docs.fedoraproject.org/en-US/packaging-guidelines/Versioning/#_snapshots

> Group:          Development/Libraries

Not needed and can be dropped.

https://fedoraproject.org/wiki/RPMGroups

> ExclusiveArch:  x86_64

What about this restricts it to x86_64? I don't see anything in the codebase for it...

> Requires:  wget

What is this for?

> %cmake -DCMAKE_INSTALL_PREFIX=/usr \

The "CMAKE_INSTALL_PREFIX" option is already set by %cmake, so drop this.

> cd %{__cmake_builddir}
> %__make runtest

Would "%cmake_build --target runtest" work here?

Alternatively, "%{__make} -C %{__cmake_builddir} runtest" would work.

> %{_libdir}/lib%{name}proto.a

We don't typically package static link libraries...

Comment 3 Tom Rix 2023-05-21 13:42:01 UTC

Updates here

Spec URL: https://trix.fedorapeople.org/caffe.spec
SRPM URL: https://trix.fedorapeople.org/caffe-1.0.20200212git9b89154-2.fc39.src.rpm

All changes made.
For wget (and now gzip, tar) question, see
https://github.com/BVLC/caffe/blob/master/examples/cifar10/readme.md
These are used to fetch and unpack datasets used in the examples.

Comment 4 Davide Cavalca 2023-08-16 22:47:43 UTC

Some notes:
- in %install use cp -p to preserve timestamps
- instead of copying LICENSE and README in %{_datadir}/Caffe just flag them with %license / %doc
- use something like find %{buildroot} -name .gitignore -exec rm '{}' \; instead of the for loop to clean up gitignore
- any particular reason we're explicitly disabling python?

Comment 5 Neal Gompa 2023-08-16 22:55:54 UTC

> Version:        1.0.%{?date0}git%{?shortcommit0}

This format is not correct. I would suggest something like this for the Version: "1.0^git%{date0}.%{shortcommit0}"

Comment 6 Peter Robinson 2023-08-17 08:37:27 UTC

> - use something like find %{buildroot} -name .gitignore -exec rm '{}' \;

or "find %{buildroot} -name .gitignore -delete" for even more optimisation :)