Bug 473046 (miniupnpc)
Summary: | Review Request: miniupnpc - command line tool to control NAT in UPnP-enabled routers as Linksys, D-Link etc | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Avi Alkalay <avibrazil> |
Component: | Package Review | Assignee: | Nobody's working on this, feel free to take it <nobody> |
Status: | CLOSED NOTABUG | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | cweyl, fedora-package-review, herrold, ismael, misc, notting |
Target Milestone: | --- | Keywords: | Reopened |
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2012-04-19 15:53:21 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
Avi Alkalay
2008-11-26 09:33:47 UTC
Obviously you do not have a sponsor. Thus your Bug should block FE-NEEDSPONSOR. See http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored for more information. Both links: http://fedoraproject.org/wiki/PackageMaintainers/Join http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored Are not specific on how to get a sponsor. The "Join" one says: "When the package is APPROVED by the reviewer, you must separately obtain member sponsorship in order to check in and build your package. Sponsorship is not automatic and may require that you further participate in other ways in order to demonstrate your understanding of the packaging guidelines. Key to becoming sponsored is to convince an existing sponsor-level member that you understand and follow the project's guidelines and processes. See PackageMaintainers/HowToGetSponsored for more information on the process of becoming sponsored." And the HowToGetSponsored doc is purely philosophical and has no practical information that I could find. Need guidance on getting sponsorship. Thanks in advance Okay so one crucial step is to enter either 177841 or FE-NEEDSPONSOR into the "Blocks" field at the top of the bugreport and commit it. This will signalize that the contributer who filed this bug needs a sponsor. That makes it easier for sponsors to find your review. 'Source0: %{name}-%{version}.tar.gz' should be changed to 'Source0: http://miniupnp.tuxfamily.org/files/%{name}-%{version}.tar.gz' https://fedoraproject.org/wiki/Packaging/SourceURL OK, I'll change that. But just be aware that I copied this Source0 line format from the official Fedora rdesktop spec file. Before I send a new version, I need some guidance with the group. The spec file I used as a base had "User Interface/Desktops" but this is not the case for this package. Where I can find a list of valid groups ? BTW, this FE-NEEDSPONSOR tag is documented somewhere? I couldn't find a single line in the documentation about it. I would like to update the documentation to include that, after this package is accepted. (In reply to comment #5) > OK, I'll change that. But just be aware that I copied this Source0 line format > from the official Fedora rdesktop spec file. The rdesktop spec file is not reviewed so far. The 'Merge Review' is pending. Your Source0 is acceptable if there is no source code available from a online/upstream location or only out of a VCS. But in this case it has to be documented in the spec file. Basically all source must come from upstream. > Before I send a new version, I need some guidance with the group. The spec file > I used as a base had "User Interface/Desktops" but this is not the case for > this package. > Where I can find a list of valid groups ? less /usr/share/doc/rpm-*/GROUPS > BTW, this FE-NEEDSPONSOR tag is documented somewhere? I couldn't find a single > line in the documentation about it. I would like to update the documentation to > include that, after this package is accepted. See https://fedoraproject.org/wiki/PackageMaintainers/Join#Create_Your_Review_Request Suggestions incorporated into spec. Please recheck http://avi.alkalay.net/software/miniupnpc/ You bumped the release of the spec file but there is no entry in the %changelog section. https://fedoraproject.org/wiki/Packaging/Guidelines#Changelogs rpmlint is complaining about several things. rpmlint output [fab@laptop024 SRPMS]$ rpmlint miniupnpc* miniupnpc.src:59: E: files-attr-not-set miniupnpc.src:60: E: files-attr-not-set miniupnpc.src:61: E: files-attr-not-set miniupnpc.src: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 6) miniupnpc.src: E: summary-too-long Library and command line tool to control NAT in UPnP-enabled routers as Linksys, D-Link etc 1 packages and 0 specfiles checked; 4 errors, 1 warnings. [fab@laptop024 i386]$ rpmlint mini* miniupnpc.i386: W: devel-file-in-non-devel-package /usr/lib/libminiupnpc.so miniupnpc.i386: E: summary-too-long Library and command line tool to control NAT in UPnP-enabled routers as Linksys, D-Link etc miniupnpc.i386: W: incoherent-version-in-changelog 1.2 ['1.2-2.fc9', '1.2-2'] miniupnpc.i386: W: unstripped-binary-or-object /usr/lib/libminiupnpc.so.3 3 packages and 0 specfiles checked; 1 errors, 3 warnings. https://fedoraproject.org/wiki/PackageMaintainers/Common_Rpmlint_Issues Suggestions incorporated into spec. Please recheck http://avi.alkalay.net/software/miniupnpc/ What is the purpose of the line mv LICENCE LICENSE from the %build section ? Please do not compress the manpages yourself. rpmbuild will do this for you (and might also accomodate other forms of compression than gzip) Please use macros instead of explicit directories. This also goes for %{_mandir} which should be used instead of /usr/share/man in %install Also, please do not manually strip the binaries. rpmbuild will use these pieces of info to build a separate package with debugging information (and strip them from the final binaries afterwards Please make sure that Fedora's mandatory compilation flags are used (see https://fedoraproject.org/wiki/Packaging/Guidelines#Compiler_flags ). Your spec uses "-fPIC -O -Wall -DNDEBUG" instead. Mock build fails on x86_64: RPM build errors: File not found by glob: /builddir/build/BUILDROOT/miniupnpc-1.2-3.fc11.x86_64/usr/lib64/lib*.so.* I have not examined the Makefile closely, but I suspect that the culprit is the INSTALLDIRLIB line, which makes the generated libs to always be installed in /usr/lib instead of using the correct path depending on architecture. And last but not least, do not be shy and ADD changelog entries each time you modify the spec, leaving the older entries in place. For instance now, by looking at version "3" of the spec one could believe that this is the first attempt (because of the "initial build" line). But I know this is not true, because there exists a revision labeled "2". OK, I'll incorporate all suggestions. I am also preparing an initscript and /etc/sysconfig configuration file to setup NATs on boot. Just give me a few days. Suggestions incorporated. Please recheck http://avi.alkalay.net/software/miniupnpc/ Some notes: - The LICENCE file is still being renamed due to wrong spelling. - Package now includes an initscript and a sysconfig conf file. - To avoid a patch and to make it install correctly on x86_64, I included the following on %install: %ifarch x86_64 mv $RPM_BUILD_ROOT/usr/lib $RPM_BUILD_ROOT/%{_libdir} %endif Can't test this. I don't have a x86_64 build system. rpmlint still outputs the following which I need some guidance on how to solve: miniupnpc.i386: W: unstripped-binary-or-object /usr/lib/libminiupnpc.so.3 miniupnpc.i386: E: shlib-with-non-pic-code /usr/lib/libminiupnpc.so.3 miniupnpc.i386: W: conffile-without-noreplace-flag /etc/sysconfig/upnpnats miniupnpc.i386: W: service-default-enabled /etc/rc.d/init.d/upnpnats miniupnpc.i386: E: incoherent-subsys /etc/rc.d/init.d/upnpnats upnpcnats miniupnpc.i386: W: service-default-enabled /etc/rc.d/init.d/upnpnats miniupnpc.i386: W: incoherent-init-script-name upnpnats UNless you want to be sure the Americans are very very happy, you can remove the mv. "licence" is valid English (British English).See http://www.thefreedictionary.com/LICENCE Your pseudo fix for the %{_libdir} problem does not work and you still need to address the problem of Fedora's mandatory compilation flags. Quoting from the end of the build log: testigddescparse.c:51: warning: ignoring return value of 'fread', declared with attribute warn_unused_result gcc minixmlvalid.o minixml.o -o minixmlvalid gcc testminixml.o minixml.o igd_desc_parse.o -o testminixml gcc testupnpreplyparse.o minixml.o upnpreplyparse.o -o testupnpreplyparse gcc testigddescparse.o igd_desc_parse.o minixml.o -o testigddescparse minixml validation test ./minixmlvalid 14 events touch validateminixml ar crs libminiupnpc.a miniwget.o minixml.o igd_desc_parse.o minisoap.o miniupnpc.o upnpreplyparse.o upnpcommands.o minissdpc.o upnperrors.o gcc -shared -Wl,-soname,libminiupnpc.so.3 -o libminiupnpc.so miniwget.o minixml.o igd_desc_parse.o minisoap.o miniupnpc.o upnpreplyparse.o upnpcommands.o minissdpc.o upnperrors.o gcc -o upnpc-static upnpc.o libminiupnpc.a /usr/bin/ld: miniwget.o: relocation R_X86_64_32 against `a local symbol' can not be used when making a shared object; recompile with -fPIC miniwget.o: could not read symbols: Bad value collect2: ld returned 1 exit status make: *** [libminiupnpc.so] Error 1 make: *** Waiting for unfinished jobs.... error: Bad exit status from /var/tmp/rpm-tmp.jzb5MU (%build) From the above you can see that gcc is called without any flags and that linking fails. I'll look at the other problems after you solve these two. I was passing the compilation flags on %build like this: make %{?_smp_mflags} CFLAGS="%{optflags} -DNDEBUG" But in some gcc calls it was not working. Apparently miniupnpc Makefile is not very robust. So I did this: make %{?_smp_mflags} CFLAGS="%{optflags} -DNDEBUG" CC="gcc %{optflags} -DNDEBUG" About the x86_64 issues, I can't really help because I don't access to this platform to test it. Hope you won't ask me to build it on an s390x too :-) Seriously, how I can get access to a x86_64 machine to fix it? Does Fedora Project provides one for build purposes ? One can use koji scratch builds. As a sidenote, I fail to understand why do you keep trying to workaround instead of simply fixing the Makefile. And.. what's with that "DNDEBUG" flag ? What's wrong with Fedora's defaults ? Workaround is less intrusive, simpler, more readable, less patches, and provides exactly the same results. DNDEBUG is needed by the source. It fails to compile if removed (a needed function declaration is enclosed inside an #ifdef). I tried without it at the first time but had to put it back. Who can use koji? Is it mandatory for the acceptance of this package to have it built on ia32 AND x86_64? Which other platforms are mandatory ? I think a x86_64 user can also use the ia32 binary package. Please provide advice on what else can I do to have this simple package accepted ? So what is missing to get this done ? What else can I do ? Why the process stopped ? Access to do koji scratch builds is open to anyone with a FAS account, if I am not mistaken. As of what is missing: you miss a proper fix of the package. Despite having being told several times to patch the makefile, you keep coming with incorrect workarounds. gcc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic -DNDEBUG -o upnpc-sta tic upnpc.o libminiupnpc.a /usr/bin/ld: miniwget.o: relocation R_X86_64_32 against `a local symbol' can not be used when making a shared object; recompile with -fPIC miniwget.o: could not read symbols: Bad value collect2: ld returned 1 exit status make: *** [libminiupnpc.so] Error 1 make: *** Waiting for unfinished jobs.... error: Bad exit status from /var/tmp/rpm-tmp.lhMd1F (%build) RPM build errors: Bad exit status from /var/tmp/rpm-tmp.lhMd1F (%build) EXCEPTION: Command failed. See logs for output. # bash -l -c 'rpmbuild -bb --target x86_64 --nodeps //builddir/build/SPECS/miniupnpc.spec' Traceback (most recent call last): File "/usr/lib/python2.5/site-packages/mock/trace_decorator.py", line 70, in trace result = func(*args, **kw) File "/usr/lib/python2.5/site-packages/mock/util.py", line 286, in do raise mock.exception.Error, ("Command failed. See logs for output.\n # %s" % (command,), ret) Error: Command failed. See logs for output. # bash -l -c 'rpmbuild -bb --target x86_64 --nodeps //builddir/build/SPECS/miniupnpc.spec' LEAVE do --> EXCEPTION RAISED I can look for it, but do you have any fast link to how to use Koji ? Thanks I would like to suggest to remove 'Linksys' from the summary since this is probably a trademark. The same for the %description. Just my thought on the legal stuff... More details: https://fedoraproject.org/wiki/Packaging:Guidelines#Trademarks_in_Summary_or_Description Avi, any progress on this? ping again? Again ping? I will close this bug as NOTABUG if no response is received from the reporter within ONE WEEK. Closing. If someone wants to import this package into Fedora, please file a new review request and mark this one as a duplicate of the new one. Thank you! I have made updates to this package based on comments here and now a working x86_64 version is available. Please check here: http://avi.alkalay.net/software/miniupnpc/ http://avi.alkalay.net/software/miniupnpc/miniupnpc-1.4-5.fc13.src.rpm http://avi.alkalay.net/software/miniupnpc/miniupnpc-1.4-5.fc13.x86_64.rpm http://avi.alkalay.net/software/miniupnpc/miniupnpc-devel-1.4-5.fc13.x86_64.rpm Not sure if I'll get flamed for nitpicking again, but here are some comments: You should at least look at the rpmlint output and try to address what you can: miniupnpc.src: W: strange-permission upnpnats 0777L The file in the srpm is mode 777; not really a good idea. miniupnpc.src:54: E: hardcoded-library-path in $RPM_BUILD_ROOT/usr/lib I guess I understand that you're trying to avoid patching the makefile, but I'm curious why just doing make PREFIX=$RPM_BUILD_ROOT INSTALLDIRLIB=$RPM_BUILD_ROOT/%{_libdir} install doesn't work for you. miniupnpc.src:9: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 9) No reason not to clean that up. miniupnpc.x86_64: W: incoherent-version-in-changelog 1.2-5.fc9 ['1.4-5.fc15', '1.4-5'] Don't include the dist tag in changelog entries. miniupnpc.x86_64: W: unstripped-binary-or-object /usr/lib64/libminiupnpc.so.4 This is odd; you'll need to investigate why this isn't being stripped properly. It looks like the compiler flags are correct (although most of them seem to be listed twice, so maybe there's some sort issue relating to that). Or maybe it has somethig to do with the static stuff. miniupnpc.x86_64: W: conffile-without-noreplace-flag /etc/sysconfig/upnpnats Needs to be %config(noreplace), not just %config, else it will be overwritten when the package is updated. miniupnpc.x86_64: W: no-manual-page-for-binary upnpc It is nice to have manual pages but it's not essential. miniupnpc.x86_64: W: service-default-enabled /etc/rc.d/init.d/upnpnats miniupnpc.x86_64: W: service-default-enabled /etc/rc.d/init.d/upnpnats The services should definitely not be enabled by default. miniupnpc.x86_64: E: incoherent-subsys /etc/rc.d/init.d/upnpnats upnpcnats miniupnpc.x86_64: W: incoherent-init-script-name upnpnats ('miniupnpc', 'miniupnpcd') These are OK; rpmlint is confused by the service name differing from the package name. This package includes a static library. If you really wish to include a static lib, you must follow http://fedoraproject.org/wiki/Packaging:Guidelines#Packaging_Static_Libraries You are missing the the ldconfig dependencies for the scriptlets. There are some bits of the spec you can remove if you're only planning to target Fedora and RHEL6 or newer: BuildRoot, the first line of %install, and the entire %clean section. @Avi I can't sponsor you but I can review this package if you are still interested. I can't handle this anymore. I think this may be a useful package for many people but this process kills me. I appreciate if somebody can adopt and continue the work on this package. The sources, SPECs SRPMs etc can be found on links above. I addopt it. Ismael, can you correct the error that Jason speak of in comment 29, and post a updated url ? It's been another month since the last comment and a full three months since this ticket was supposedly adopted. I'm going to go ahead and close this. If someone really wants to adopt this package, please submit your own review so the original submitter of this isn't subjected to mail on a ticket in which he is no longer interested. |