Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 177584
Summary: | Review Request: zaptel | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Jeffrey C. Ollie <jeff> |
Component: | Package Review | Assignee: | David Woodhouse <dwmw2> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | andy, cweyl, erik.labianca, kevin, matthias, shiva, somlo, steve, tjb, will |
Target Milestone: | --- | Flags: | kevin:
fedora-cvs+
|
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2006-10-16 11:55:08 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: | |||
Bug Depends On: | |||
Bug Blocks: | 163779, 177603, 178922 |
Description
Jeffrey C. Ollie
2006-01-11 22:08:40 UTC
This spec is totally missing the kernel modules and also does not split the devel I would recommend you bases it on http://dag.wieers.com/packages/zaptel/zaptel.spec but make it compliant with any fedora conventions. Also as you will see from DAG's packaging, that the Makefile will work if you supply the correct parameters to it bug #177583 has the kernel modules 1) As Tim Jackson says, the kernel modules are in bug # 177583. See http://www.fedoraproject.org/wiki/Extras/KernelModuleProposal and the fedora-extras list archives for a discussion of the new kernel module packaging proposal. 2) Yes, my spec does split off a -devel package. 3) The problem with just specifying CFLAGS before running make is that the Makefile will append additional optimization parameters that I don't want. 4) The problem with using "make install" is that it would force a build of the device drivers (which are built in a separate package), as well as running a large number of other commands that are inappropriate to be run as part of the packaging process. I've used Dag's and other people's .spec files as inspiration - I've been packaging up Zaptel for a while now... Now that Fedora Extras has a process for packaging kernel modules it was time for me to share my packages with the rest of the world. (In reply to comment #3) > 4) The problem with using "make install" is that it would force a build of the > device drivers (which are built in a separate package), as well as running a > large number of other commands that are inappropriate to be run as part of the > packaging process. You really should use "make install" and remove the part of the makefile that builds the module; use a patch during %prep Updated Spec/SRPM: Spec Name or Url: http://www.ocjtech.us/zaptel-1.2.1-2.spec SRPM Name or Url: http://www.ocjtech.us/zaptel-1.2.1-2.src.rpm The devel subpackage needs to require the main package. (In reply to comment #4) > (In reply to comment #3) > > 4) The problem with using "make install" is that it would force a build of the > > device drivers (which are built in a separate package), as well as running a > > large number of other commands that are inappropriate to be run as part of the > > packaging process. > > You really should use "make install" and remove the part of the makefile that > builds the module; use a patch during %prep Hmm... the install section of the Makefile would require *EXTREME* surgery. Unless it's going to block approval of the package I'd rather install the files that I need manually in the spec until the upstream package gets fixed. (In reply to comment #6) > Hmm... the install section of the Makefile would require *EXTREME* surgery. Really? I took a quick look. From a first glance I would presume that removing everything between - if [ -f zaptel.ko ]; then \ and - fi should be enough. > Unless it's going to block approval of the package I'm not the reviewer. But if was I would consider it a blocker. > I'd rather install the files > that I need manually in the spec until the upstream package gets fixed. I did something like that in the past -- you have to recheck after each upstream update that you still install everything exactly as the makefile would do it. And that is a lot of work and often is forgotten (and that's the reason why I think it's a blocker). asterisk, zaptel and libpri 1.2.2 has been released :) Update Spec/SRPM: Spec Name or Url: http://www.ocjtech.us/zaptel-1.2.1-1.spec SRPM Name or Url: http://www.ocjtech.us/zaptel-1.2.1-1.src.rpm * Wed Jan 18 2006 Jeffrey C. Ollie <jeff> - 1.2.2-2 - Bump release number. * Wed Jan 18 2006 Jeffrey C. Ollie <jeff> - 1.2.2-1 - Update to 1.2.2. Updated Spec/SRPM: Spec Name or Url: http://www.ocjtech.us/zaptel-1.2.2-3.spec SRPM Name or Url: http://www.ocjtech.us/zaptel-1.2.2-3.src.rpm * Mon Jan 23 2006 Jeffrey C. Ollie <jeff> - 1.2.2-3 - provide zaptel-kmod-common Updated Spec/SRPM: Spec Name or Url: http://www.ocjtech.us/zaptel-1.2.3-1.spec SRPM Name or Url: http://www.ocjtech.us/zaptel-1.2.3-1.src.rpm %changelog * Tue Jan 31 2006 Jeffrey C. Ollie <jeff> - 1.2.3-1 - Preserve timestamps when we install. - Use custom init.d file that does all the fancy RH stuff. * Mon Jan 30 2006 Jeffrey C. Ollie <jeff> - 1.2.3-1 - Update to 1.2.3. Updated Spec/SRPM: Spec Name or Url: http://www.ocjtech.us/zaptel-1.2.4-1.spec SRPM Name or Url: http://www.ocjtech.us/zaptel-1.2.4-1.src.rpm %changelog * Wed Feb 15 2006 Jeffrey C. Ollie <jeff> - 1.2.4-1 - Update to 1.2.4 I would have to agree with Thorsten here, as it would be much cleaner, and have less chances to break in a future update, if "make install" was used, with the kernel modules build and install parts ripped out. Updated Spec/SRPM: Spec Name or Url: http://www.ocjtech.us/zaptel-1.2.5-1.spec SRPM Name or Url: http://www.ocjtech.us/zaptel-1.2.5-1.src.rpm %changelog * Mon Mar 27 2006 Jeffrey C. Ollie <jeff> - 1.2.5-1 - Update to 1.2.5 There is an issue with the zaptel.rules file for udev. The KERNEL= on each line has an additional = sign which causes udev not to create the /dev/zap entries. On a related note, the udev rule calls for the owner to be "asterisk", but neither this package or the asterisk package seems to be creating the user. In practice however, udev just creates the dev node (after the above is fixed) as root. Updated Spec/SRPM: Spec Name or Url: http://www.ocjtech.us/zaptel-1.2.5-2.spec SRPM Name or Url: http://www.ocjtech.us/zaptel-1.2.5-2.src.rpm %changelog * Thu Apr 27 2006 Jeffrey C. Ollie <jeff> - 1.2.5-2 - Changed ownership of device nodes to "root" in udev rules file. - Don't build sethdlc. The issue with the udev file is still there - --- zaptel.rules 2006-05-18 11:28:15.000000000 +0100 +++ zaptel.rules.clean 2006-05-18 11:28:42.000000000 +0100 @@ -1,5 +1,5 @@ -KERNEL=="zapctl", NAME="zap/ctl", OWNER="root", GROUP="root", MODE="0660" -KERNEL=="zaptimer", NAME="zap/timer", OWNER="root", GROUP="root", MODE="0660" -KERNEL=="zapchannel", NAME="zap/channel", OWNER="root", GROUP="root", MODE="0660" -KERNEL=="zappseudo", NAME="zap/pseudo", OWNER="root", GROUP="root", MODE="0660" -KERNEL=="zap[0-9]*", NAME="zap/%n", OWNER="root", GROUP="root", MODE="0660" +KERNEL="zapctl", NAME="zap/ctl", OWNER="root", GROUP="root", MODE="0660" +KERNEL="zaptimer", NAME="zap/timer", OWNER="root", GROUP="root", MODE="0660" +KERNEL="zapchannel", NAME="zap/channel", OWNER="root", GROUP="root", MODE="0660" +KERNEL="zappseudo", NAME="zap/pseudo", OWNER="root", GROUP="root", MODE="0660" +KERNEL="zap[0-9]*", NAME="zap/%n", OWNER="root", GROUP="root", MODE="0660" (In reply to comment #17) > The issue with the udev file is still there - The "extra" equals sign is a udev-version specific thing. I believe it was in udev version 054 where the change was made. The next version of zaptel (due RSN) will conditially generate a udev rules file appropriate for the installed version of udev. As far as the ownership of the device files goes, it makes a lot of sense to run asterisk as a non-root user. How best to accomplish that in RPM packages is something I'll have to look into. If you generate different udev files dynamically based on the udev version, then you should probably also include a versioned Requires on udev. (In reply to comment #19) > If you generate different udev files dynamically based on the udev version, then > you should probably also include a versioned Requires on udev. Yes, I will. I should also be calling udevstart in %post/%postun too. Hey Jeff. I am looking at this next since I started reviewing zaptel-kmod... Do you have an updated srpm/spec to review? Any chance of a patch to allow you to use 'make install'? Any udev changes to do diffrent things vs diffrent udev versions? Hi, What is the status of this package? Thanks, Gavin. In response to comment #22: I think it's waiting to hear the fate of the zaptel-kmod submission. Once thats decided we can close or move this one forward. Since its looking like zaptel will get included in the kernel RPM before the kmod gets approved I may try and convert this to a userland/library-only package. Yes, that's a good idea. Spec: http://repo.ocjtech.us/asterisk-1.4/fedora/development/SRPMS/zaptel-1.4.0-0.fc6.beta1.spec SRPM: http://repo.ocjtech.us/asterisk-1.4/fedora/development/SRPMS/zaptel-1.4.0-0.fc6.beta1.src.rpm OK, here's a SRPM updated to the latest 1.4 beta, without any dependencies on a kmod package. Dropping the dependency on bug #177583 since it's likely that the Zaptel kernel modules will someday become a part of the kernel package. Probably wants to drop the Provides: zaptel-kmod-common Probably also wants to drop the loading of modules in the initscript, since udev should handle that kind of stuff by the time we've finished. Why are we building with -fsigned-char on PPC? That's scary and doesn't match the rest of the system. It's usually a sign of buggy code -- where do we assume that 'char' is signed, and why? Let's just fix it instead. Utils should probably be going into %{_sbindir} instead of /sbin. What is the licence on the OCT6114-128D.ima firmware file? ifup-hdlc attempts to use 'sethdlc', which isn't present or in Requires. Consider being more biarch-friendly by putting libraries into their own package (which can then be install for both 32-bit and 64-bit simultaneously), while the other bits like configuration and initscript (if you still have one) move into the zaptel-utils package. udev rules give ownership of all zap devices to asterisk, but that user doesn't exist. Perhaps we should have a 'zaptel' group, and add both Asterisk and OpenPBX to that group? You'll need to create the zaptel group for yourself in the zaptel package, and the asterisk-zaptel and openpbx-zaptel packages would each need to add their PBX user to the zaptel group. Or can someone think of a better solution? Spec: http://repo.ocjtech.us/asterisk-1.4/fedora/development/SRPMS/zaptel-1.4.0-1.fc6.beta1.spec SRPM: http://repo.ocjtech.us/asterisk-1.4/fedora/development/SRPMS/zaptel-1.4.0-1.fc6.beta1.src.rpm * Fri Oct 13 2006 Jeffrey C. Ollie <jeff> - 1.4.0-1.beta1 - Remove "Provides: zaptel-kmod-common" - Don't load modules in initscript (except for possibly ztdummy) - leave that up to udev/hotplug. - Drop ifup-hdlc - it requires sethdlc which isn't available. - Move utilities to %%{_sbindir} rather than /sbin. - Split libtonezone off into a separate lib package. - Use 'zaptel' user/group to own device files. - Add patch to fix minor makefile bug. OK, looks good. Only outstanding question is the licence of the OCT6114-128D.ima file. Is that GPL'd? If so, where's the source. If not, it needs to go. I believe it's part of the GPL'd Oactasic API but I'm not sure... I say get rid of it until we can find out for sure the license on it. Spec: http://repo.ocjtech.us/asterisk-1.4/fedora/development/SRPMS/zaptel-1.4.0-1.fc6.beta1.spec SRPM: http://repo.ocjtech.us/asterisk-1.4/fedora/development/SRPMS/zaptel-1.4.0-1.fc6.beta1.src.rpm %changelog * Sun Oct 15 2006 Jeffrey C. Ollie <jeff> - 1.4.0-2.beta1 - Don't package firmware until license can be figured out. Looks good. Go ahead. Imported and built for FE-devel. Branch for FE-5 requested. zaptel user added to http://fedoraproject.org/wiki/PackageUserRegistry The package release is wrong (zaptel's too). It should follow the package naming guidelines and be something like "0.1.beta1.fc6" : http://fedoraproject.org/wiki/Packaging/NamingGuidelines#head-d97a3f40b6dd9d2288206ac9bd8f1bf9b791b22a And why this update to a beta version? If it had been only in FC6/devel, why not, but the plan seems to be to also release for FC5?? Package Change Request ====================== Package Name: zaptel New Branches: EL-5 Updated EPEL Owners: jcollie Needed to build Asterisk package on EL-5. Oops, forgot to set the CVS flag. cvs done. |