Bug 480373

Summary:	Review Request: cilk - Language for multithreaded parallel programming.
Product:	[Fedora] Fedora	Reporter:	Adam Miller <maxamillion>
Component:	Package Review	Assignee:	Ian Weller <ian>
Status:	CLOSED WONTFIX	QA Contact:	Fedora Extras Quality Assurance <extras-qa>
Severity:	medium	Docs Contact:
Priority:	medium
Version:	rawhide	CC:	fedora-package-review, ian, notting
Target Milestone:	---
Target Release:	---
Hardware:	All
OS:	Linux
Whiteboard:
Fixed In Version:		Doc Type:	Bug Fix
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2009-02-23 23:16:45 UTC	Type:	---
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:

Description Adam Miller 2009-01-16 18:31:17 UTC

Spec URL: http://maxamillion.fedorapeople.org/cilk.spec
SRPM URL: http://maxamillion.fedorapeople.org/cilk-5.4.6-1.src.rpm

Description:
Cilk (pronounced 'silk') is a language for multithreaded parallel 
programming based on ANSI C. Cilk is designed for general-purpose parallel 
programming, but it is especially effective for exploiting dynamic, highly 
asynchronous parallelism, which can be difficult to write in data-parallel 
or message-passing style.

Comment 1 Ian Weller 2009-01-17 03:11:23 UTC

I'll review this.

Comment 2 Ian Weller 2009-01-17 03:26:45 UTC

Things I see right now:

License field in spec matches: Fail
 -- The license is GPLv2+: license text in main.c and other files reads "or an
    other version"

Package compiles and builds on at least one arch: Fail
 -- See these Koji logs.
    http://koji.fedoraproject.org/koji/getfile?taskID=1060419&name=build.log
    "What is this? -pipe" -- near end of log.

Comment 3 Adam Miller 2009-01-20 05:21:56 UTC

Spec URL: http://maxamillion.fedorapeople.org/cilk.spec
SRPM URL: http://maxamillion.fedorapeople.org/cilk-5.4.6-2.src.rpm

Fixed the license field and prematurely filed the request without running in mock, the %optflags are slapped into %configure and the '-pipe' is what is causing the failure in the package build.

Comment 4 Ian Weller 2009-01-25 05:43:45 UTC

- %dir isn't necessary in %files. you can just list the directory and it will
  automatically add all the files under it too (so get rid of the two lines
  under the %dir lines while you're at it)

- rpmlint is very loud:
cilk.src:23: W: configure-without-libdir-spec
cilk.src:51: W: macro-in-%changelog optflags
cilk.src: W: summary-ended-with-dot Language for multithreaded parallel programming.
cilk.i386: W: devel-file-in-non-devel-package /usr/lib/libcilk.a
cilk.i386: W: devel-file-in-non-devel-package /usr/lib/libcilk.g.p.a
cilk.i386: W: devel-file-in-non-devel-package /usr/lib/cilk/libcilkrt0gp.a
cilk.i386: W: devel-file-in-non-devel-package /usr/lib/libcilk.p.so
cilk.i386: W: devel-file-in-non-devel-package /usr/include/cilk/gcc-builtin.h
cilk.i386: W: devel-file-in-non-devel-package /usr/lib/cilk/libcilkrt0g.a
cilk.i386: W: devel-file-in-non-devel-package /usr/lib/libcilk.g.a
cilk.i386: W: devel-file-in-non-devel-package /usr/lib/libcilk.so
cilk.i386: W: devel-file-in-non-devel-package /usr/include/cilk/cilk-sysdep.h
cilk.i386: W: devel-file-in-non-devel-package /usr/lib/libcilk.g.so
cilk.i386: W: devel-file-in-non-devel-package /usr/include/cilk/cilk-conf.h
cilk.i386: W: devel-file-in-non-devel-package /usr/include/cilk/cilk-cilk2c.h
cilk.i386: W: devel-file-in-non-devel-package /usr/include/cilk/cilk-lib.h
cilk.i386: W: devel-file-in-non-devel-package /usr/lib/cilk/libcilkrt0.a
cilk.i386: W: devel-file-in-non-devel-package /usr/lib/cilk/libcilkrt0p.a
cilk.i386: W: devel-file-in-non-devel-package /usr/lib/libcilk.g.p.so
cilk.i386: W: devel-file-in-non-devel-package /usr/include/cilk/cilk-cilk2c-pre.h
cilk.i386: W: devel-file-in-non-devel-package /usr/lib/libcilk.p.a
cilk.i386: W: devel-file-in-non-devel-package /usr/include/cilk/cilk.h
cilk.i386: W: summary-ended-with-dot Language for multithreaded parallel programming.
cilk.i386: W: shared-lib-calls-exit /usr/lib/libcilk.g.so.0.0.0 exit
cilk.i386: W: shared-lib-calls-exit /usr/lib/libcilk.p.so.0.0.0 exit
cilk.i386: W: shared-lib-calls-exit /usr/lib/libcilk.g.p.so.0.0.0 exit
cilk.i386: W: shared-lib-calls-exit /usr/lib/libcilk.so.0.0.0 exit
3 packages and 0 specfiles checked; 0 errors, 27 warnings.

- why did you replace %configure?

- you can use
    %configure CFLAGS="%(echo "%optflags" | sed -e 's/-pipe//' | sed -e 's/--param=ssp-buffer-size=4//')"
  instead of getting rid of the compiler flags altogether. This keeps cilkclocal
  happy and keeps most of the compiler flags.

Comment 5 Ian Weller 2009-01-25 05:44:56 UTC

- you need to add a %check section because "make check" is available

- please add "%{?_smp_mflags}" to the end of "make"
  https://fedoraproject.org/wiki/Packaging:Guidelines#Parallel_make

Comment 6 Adam Miller 2009-01-28 14:26:18 UTC

Spec URL: http://maxamillion.fedorapeople.org/cilk.spec
SRPM URL: http://maxamillion.fedorapeople.org/cilk-5.4.6-3.src.rpm

Added the %check, fixed the CFLAGS, also added the %{?_smp_mflags}

rpmlint still throws warning about devel-file-in-non-devel-package but i'm not entirely sure that's a valid warning as the libs are specific to the app.

Comment 7 Ian Weller 2009-01-28 14:52:29 UTC

rpmlint still throws warning about devel-file-in-non-devel-package because you need a -devel subpackage for those files. See the packaging guidelines.

Comment 8 Ian Weller 2009-02-15 19:56:21 UTC

Ping?

Comment 9 Adam Miller 2009-02-18 20:44:54 UTC

Very sorry, I have gotten a little bogged down with work, school, and trying to wrap up some other fedora related business. I will reference the guidelines and post back as soon as I am able.

Comment 10 Adam Miller 2009-02-23 23:16:45 UTC

The project I was packaging this up for has been shut down. Closing Review Request, thank you for your time during the review. Very sorry for consuming unnecessary amounts of time.