Bug 478640 (mingw32-dlfcn) - Review Request: mingw32-dlfcn - Implements a wrapper for dlfcn (dlopen dlclose dlsym dlerror)
Summary: Review Request: mingw32-dlfcn - Implements a wrapper for dlfcn (dlopen dlclos...
Keywords:
Status: CLOSED NEXTRELEASE
Alias: mingw32-dlfcn
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Adel Gadllah
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 454410
Blocks: mingw32-pixman mingw32-freetype mingw32-cairo mingw32-SDL
TreeView+ depends on / blocked
 
Reported: 2009-01-02 17:20 UTC by Itamar Reis Peixoto
Modified: 2009-01-21 21:28 UTC (History)
6 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2009-01-21 21:28:29 UTC
Type: ---
Embargoed:
adel.gadllah: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Itamar Reis Peixoto 2009-01-02 17:20:50 UTC
Spec URL: http://ispbrasil.com.br/dlfcn/mingw32-dlfcn.spec
SRPM URL: http://ispbrasil.com.br/dlfcn/mingw32-dlfcn-r11-1.fc10.src.rpm

Description:

This library implements a wrapper for dlfcn, as specified in POSIX and SUS,
around the dynamic link library functions found in the Windows API.

(dlopen dlclose dlsym dlerror)

Comment 1 Richard W.M. Jones 2009-01-03 10:49:55 UTC
Itamar, can you try making a Koji scratch-build of this please.
It will make it easier to review.  You will need to do something
like:

koji build --scratch dist-f11 mingw32-dlfcn-r11-1.fc10.src.rpm

(I'm not exactly sure if that is the correct command, and
upgraded my machine yesterday so Koji is now broken for
me, so please check it).

Comment 2 Richard W.M. Jones 2009-01-13 12:09:27 UTC
I imported this package into the Fedora MinGW temporary
repository because I want to have other packages depend
on it.

Here is an updated package:

Spec URL: http://hg.et.redhat.com/cgi-bin/hg-misc.cgi/fedora-mingw--devel/file/tip/dlfcn/mingw32-dlfcn.spec
SRPM URL: http://koji.fedoraproject.org/koji/getfile?taskID=1049268&name=mingw32-dlfcn-0.1-0.2.r11.fc11.src.rpm
* Tue Jan 13 2009 Richard W.M. Jones <rjones> - 0.1-0.2.r11
- Import into fedora-mingw temporary repository because there are packages
  which will depend on this.
- Fix the version/release according to packaging guidelines.
- Tidy up the spec file.
- Use dos2unix and keep the timestamps.

Here is a successful Koji scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1049267

rpmlint says:
mingw32-dlfcn.noarch: W: devel-file-in-non-devel-package /usr/i686-pc-mingw32/sys-root/mingw/lib/libdl.dll.a
mingw32-dlfcn.noarch: W: devel-file-in-non-devel-package /usr/i686-pc-mingw32/sys-root/mingw/include/dlfcn.h
mingw32-dlfcn.noarch: E: arch-independent-package-contains-binary-or-object /usr/i686-pc-mingw32/sys-root/mingw/lib/libdl.dll.a
mingw32-dlfcn.noarch: W: non-standard-dir-in-usr i686-pc-mingw32
2 packages and 0 specfiles checked; 1 errors, 3 warnings.

These are all errors which can be ignored for MinGW packages.

Comment 3 Richard W.M. Jones 2009-01-13 13:06:06 UTC
Doesn't need to be in NEEDINFO any more.

Comment 4 Adel Gadllah 2009-01-13 20:03:31 UTC
"Fix it with sed in the %prep section: %{__sed} -i 's/\r//' src/somefile -- DONT use dos2unix, that can cause build fail on FC3. "

Well you should stick to the guidelines even if FC3 is long dead.
Pass %{?_smp_mflags} to make unless you have a reason not to do so.

Will do a formal review later this week (limited time).

Comment 5 Richard W.M. Jones 2009-01-14 00:03:00 UTC
(In reply to comment #4)
> "Fix it with sed in the %prep section: %{__sed} -i 's/\r//' src/somefile --
> DONT use dos2unix, that can cause build fail on FC3. "
> 
> Well you should stick to the guidelines even if FC3 is long dead.

I'm missing the context here.  Was the above in reply to something?

> Pass %{?_smp_mflags} to make unless you have a reason not to do so.

I agree, this should be added.

> Will do a formal review later this week (limited time).

Thanks.

Comment 6 Adel Gadllah 2009-01-14 07:25:43 UTC
(In reply to comment #5)
> (In reply to comment #4)
> > "Fix it with sed in the %prep section: %{__sed} -i 's/\r//' src/somefile --
> > DONT use dos2unix, that can cause build fail on FC3. "
> > 
> > Well you should stick to the guidelines even if FC3 is long dead.
> 
> I'm missing the context here.  Was the above in reply to something?

Yeah forgot to quote the context:
"- Use dos2unix and keep the timestamps."
dos2unix shouldn't be used, use sed instead. (see above quote from the guidelines)

Comment 7 Richard W.M. Jones 2009-01-14 10:48:36 UTC
dos2unix issue raised with FPC:

https://www.redhat.com/archives/fedora-packaging/2009-January/msg00066.html

Here's an updated package which reverts the dos2unix change
and should fix everything else:

Spec URL: http://hg.et.redhat.com/cgi-bin/hg-misc.cgi/fedora-mingw--devel/file/tip/dlfcn/mingw32-dlfcn.spec
SRPM URL: http://www.annexia.org/tmp/mingw/fedora-10/src/SRPMS/mingw32-dlfcn-0-0.3.r11.fc10.src.rpm

* Wed Jan 14 2009 Richard W.M. Jones <rjones> - 0-0.3.r11
- Use Version 0
  (https://www.redhat.com/archives/fedora-packaging/2009-January/msg00064.html)
- Revert use of dos2unix for now
  (https://www.redhat.com/archives/fedora-packaging/2009-January/msg00066.html)
- Use _smp_mflags.

Comment 8 Adel Gadllah 2009-01-16 10:54:01 UTC
REVIEW:

[+] = OK
[-] = NOT OK
[1] = SEE COMMENTS
[?] = WTF?

===========================

[+]	source files match upstream:
		sha1: 0b691107a23554d927987d4ddfca6e84dfd85313
[+]	package meets naming and versioning guidelines.
[+]	specfile is properly named, is cleanly written and uses macros consistently.
[+]	dist tag is present.
[+]	build root is correct.
[+]	license field matches the actual license.
[+]	license is open source-compatible.
 		LGPLv2+
[+]	license text included in package.
[+]	latest version is being packaged.
[+]	BuildRequires are proper.
[+]	compiler flags are appropriate.
[+]	%clean is present.
[+]	package builds in koji.
		http://koji.fedoraproject.org/koji/taskinfo?taskID=1058151
[+]	package installs properly.
[1]	rpmlint is silent.
[+]	final provides and requires are sane
[+]	no shared libraries are added to the regular linker search paths.
[+]	doesn't own any directories it shouldn't.
[+]	no duplicates in %files.
[+]	file permissions are appropriate.
[+]	no scriptlets present.
[+]	code, not content.
[+]	documentation is small, so no -docs subpackage is necessary.
[+]	%docs are not necessary for the proper functioning of the package.
[2]	no headers.
[+]	no pkgconfig files.
[+]	no libtool .la droppings.

============================

COMMENTS

1: not silent.
mingw32-dlfcn.noarch: W: devel-file-in-non-devel-package /usr/i686-pc-mingw32/sys-root/mingw/lib/libdl.dll.a
mingw32-dlfcn.noarch: W: devel-file-in-non-devel-package /usr/i686-pc-mingw32/sys-root/mingw/include/dlfcn.h
mingw32-dlfcn.noarch: E: arch-independent-package-contains-binary-or-object /usr/i686-pc-mingw32/sys-root/mingw/lib/libdl.dll.a
mingw32-dlfcn.noarch: W: non-standard-dir-in-usr i686-pc-mingw32

Those are fine thought (package used for cross compiling)

2: header is present, but its fine (cross compile package)

============================

Package looks good => APPROVED

Comment 9 Richard W.M. Jones 2009-01-16 11:36:04 UTC
New Package CVS Request
=======================
Package Name: mingw32-dlfcn
Short Description: Implements a wrapper for dlfcn (dlopen dlclose dlsym dlerror)
Owners: rjones berrange lfarkas
Branches: EL-5 F-10
InitialCC:

Comment 10 Kevin Fenzi 2009-01-17 03:22:14 UTC
cvs done.

Comment 11 Fedora Update System 2009-01-17 11:54:01 UTC
mingw32-dlfcn-0-0.3.r11.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/mingw32-dlfcn-0-0.3.r11.fc10

Comment 12 Fedora Update System 2009-01-21 21:28:26 UTC
mingw32-dlfcn-0-0.3.r11.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.


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