[Home]

Summary:DAHLIN-00010: [patch] Provide make rpm functionality to Zaptel
Reporter:Daniel Hazelbaker (cabal95)Labels:
Date Opened:2007-10-11 13:05:56Date Closed:2008-09-08 15:06:31
Priority:MajorRegression?No
Status:Closed/CompleteComponents:NewFeature
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) make_rpm.diff
( 1) zaptel-1.4.5.1-rpm.patch
( 2) zaptel-svn-rpm-rev1.patch
( 3) zaptel-svn-rpm-rev2.patch
( 4) zaptel-svn-rpm-rev3.patch
Description:This patch provides the make rpm functionality to Zaptel, similiar to the Linux Kernel method of building rpms. The patch has been tested on FC6 but should probably receive more testing on various platforms before entering svn.

****** ADDITIONAL INFORMATION ******

Includes a patch that is in svn but not in a release version yet (bug id 10764).
Comments:By: Tzafrir Cohen (tzafrir) 2007-10-11 16:12:33

Thanks very much for your patch.

A few comments:

1. What should be the version of svn builds?

2. You should use KVERS rather than KVERSION to set the kernel source version string.
Providing a hook to set KSRC should also be useful, I guess.

3. I would avoid the "install-modconf" target: it's buggy. It was created to remove a buggy functionality from the default install target.

4. The doc includes README.fxsusb, but not README, nor xpp/REAME.Astribank .

5. The Makefile patch includes some changes that are already merged into the SVN version. Could you test your patch with the SVN version?

6. zaptel.spec: build_tools/mkspec FORCE
There must be a smarter way.

7. the rpm target runs a 'clean' (but not 'dist-clean'). This implies a very non-reproducable build. The way I see it: either package binary products (from 'make install DESTDIR=$PWD/target) or package a newely-opened tarball with a few explicit additions.

8. The scriptlet used for the spec file building in the rpm: target should be moved to a separate script, as it is complicate to debug as-is (you can't add comments. You can't re-out parts of it, errors refer to cryptic line numbers, etc).
By: Daniel Hazelbaker (cabal95) 2007-10-11 17:33:29

1) Right now (based upon the Makefile), if a .svn file is found then ZAPTELVERSION is set to "SVN-$(shell build_tools/make_svn_branch_name). Wether nor not this is appropraite I am not sure.  Guestimating I would say it is not as rpm will probably harf if the version number contains the letters "SVN".  Other projects I have seen use a revision # of 99999 instead of an ever incrementing one. This may be more appropriate, but I would still like to see a way to have an incrementing number. Maybe take the incrementing revision # and add 10000 to it.

2) The use of the custom KVERSION is to remove the "smp" if there is one, perhaps I should base KVERSION on KVERS rather than KERNEL.

3) Hrm. Is the install-modconf target buggy, or the script (genmodconf) buggy? It has worked for me so far.  I would hate to re-create the wheel on that one.  Perhaps I am missing something and we just don't need it at all?

4) Oops.  Yes, it should be including those.  I believe I need to put some conditionals in mkspec so that it only includes certain doc files if they are selected, as I did for the modules.

5) It fails miserably. :) Much of the Makefile in svn has been altered so I will have to build a different patch for svn. I did a svn checkout of zaptel and most of the changes that I thought were going to be put in were not. Specifically the CONFIG_FILE variable is incorrect. If DESTDIR is ever declared while building then zaptel will look for the config file in $(DESTDIR)/etc/zaptel.conf. This is related to bug ZAP-233 which was listed as fixed, but I am wondering if another commit later wiped it out.

6) There is... I had misread the make docs on .PHONY, I need to include zaptel.spec in the .PHONY target (I had tried the other way around and that was not helpful).

7) What do you specifically mean by "non-reproducable"? The idea behind a 'clean' instead of 'dist-clean' is I don't want to wipe out any configured options. Nor do I want to use a "fresh" tarball as that might preclude any custom patches the user loaded in. Even a 'dist-clean' does not return the directory to the state of a fresh tarball as it should, there are files left over (that is another patch I am working on though).

8) Yeah... Not one of my finer moments :) I just noticed that the linux kernel uses an external script to build a tarball so I will do something similiar.

I'll post a new patch once I get some of these issues resolved. Some of the things you noted need to be fixed in asterisk and addons too.  Although I notice that I can't seem to delete an attached file nor upload one with the same name.  Do I just add "-rev2" "-rev3" etc. to the filename or is there a more correct way?
By: Daniel Hazelbaker (cabal95) 2007-10-11 17:37:22

Hangon on number 5... What I checked out was the trunk which apparently is not the 1.4 branch (or at least not the branch it was fixed in).  How do I get the 1.4 branch when I do a checkout, and is that the one I should be working from when building this patch?


By: Tzafrir Cohen (tzafrir) 2007-10-11 18:01:49

2. KVERSION is meaningless in the zaptel Makefile, regardless of what %{kversion} means in the spec file.

3. Generally the modules file is not needed: with a decent package you have a proper init.d script and hence really no need for running ztcfg automatically at modprobe time (which is, anyway, the wrong time to run it).

4. If you exclude xpp you also exclude genzaptelconf and the zaptel-perl utilities (lszaptel, zaptel_hardware, zapconf).

5. Right. At the moment the zaptel "development HEAD" is branches/1.4 . Sorry for the confusion.

7. Non-reproducable: that tarball may contain quite some noise besides the source. For instance, packaging from svn, it will probably contain a host of .svn directories.

This makes me feel a bit uneasy about that script.


9. You move binaries from /sbin to /usr/sbin in the spec . Shouldn't that be done somehow in the Makefile? and anyway, is the init.d script updated accordingly (ztcfg, fxotune)?
By: Daniel Hazelbaker (cabal95) 2007-10-11 18:40:20

2. Ahh, my bad I was mis-understanding what you were saying. Yes the make commands in the spec file should be using KVERS= instead of KVERSION=.

3, 4. Check.

5. Okay, I'll report on the svn patching tomorrow when I post a new patch.

7. Sorry if I am not getting it. Since that tarball is temporary and deleted after the make rpm completes, does it matter if it has "noise" in it? The final RPM wouldn't have any of that noise, such as .svn files. Essentially we are making a snapshot of the "working" directory and then using that to build the binaries from, just as we would if we did a make install, and then we delete that tarball from the system.  I suppose if we wanted to clean up the 'dist-clean' system so that it worked completely (it leaves a lot of random junk around right now) we could have a 'rpm-clean' that does a full clean and the 'dist-clean' that does rpm-clean and rm the specfile.

9. Hrm.  Another blonde moment.  I can't think why I was moving those.  On my system they are in /usr/sbin, I wonder if I moved them by hand and then thought that is where they were supposed to be for some idiotic reason.  Next patch will leave them in /sbin as they should be.

I'll try to get a new patch up friday that fixes 1, 2, 3, 6, 8 and 9 and applies to 5 (svn patch).  7 shouldn't effect the build process for testing while we figure out exactly how to do it properly and 4 will take some time to completely sort out so it may or may not make it in by tomorrow.

Thanks for the feedback so far!
By: Daniel Hazelbaker (cabal95) 2007-10-12 13:34:47

1. Right now, it comes out as Version = SVNbranch1.4r3121M, Release = 10000 (base number, it increments every 'make rpm'). It seems to me that something slightly different might make more sense but I don't know enough about how asterisk/zaptel svn is layed out to make any educated guesses as to what to name it.

2. Actually, I took KVERS= out. Since it is exported from the Makefile initially, KVERS and KSRC are already set when the build is run from the spec file so I don't see a particular need for them.

3. Done

4. Done.

5. Patch is for SVN.

6. Done, now part of .PHONY

7. Just checked, zaptel has a working 'dist-clean', asterisk does not. I would like to come up with something that works across the board and maybe that means I need to fix asterisk's 'dist-clean' system as well. We can use a "dual clean" target system such as 'dist-clean' and 'rpm-clean' or does anybody have a better recommendation?  Also, I can include a --exclude .svn in the tar command, but are there any other files that should be excluded?

8. Done, build_tools/make_tarball

9. Done, back in /sbin
By: Daniel Hazelbaker (cabal95) 2007-10-18 13:08:04

Any further thoughts on number 7, which files should be excluded and/or having some sort of dist-clean and rpm-clean build targets?
By: Tzafrir Cohen (tzafrir) 2007-10-18 13:34:12

Regarding 7: well, what I mean is making it depend as least as possible on the build environment in an unpredictable way (e.g: on the phase of the moon).

Two runs of 'make rpm' may produce an rpm of the same name with different content and functionality from the same source directory.

But I guess that this is what you want here.
By: Daniel Hazelbaker (cabal95) 2007-10-18 14:40:30

To a degree, yes.  Obviously sitting there and running make rpm over and over should produce the same binaries, but version 1.4.10.1 might be different than 1.4.10.2 because I decided to add in some modules and re-'make rpm'.  Ideally the only thing it should depend on in terms of changing functionality is the results of your 'make menuselect' and possibly ./configure.

I would expect that if digium, fedora, yellowdog, etc. were to ever prepare "standard custom" rpms then they should also follow the linux kernel model.  I.e. my fedora core 6 kernels all have .fc6 somewhere in the version.  Perhaps we could do something like the following to differentiate between a "stock" build and a custom build:

'make dist-rpm' -> zaptel-1.4.5-1.rpm
make rpm -> zaptel-1.4.5-1.local.rpm
make rpm DISTRO='fc6' -> zaptel-1.4.5-1.fc6.rpm

If DISTRO is not defined then we would use "local" (maybe use `hostname -s` or something), otherwise use it as given as part of the release build number.  make dist-rpm could just map to 'make rpm DISTRO=""'

Your "stock" rpm would always have the same functionality for a given version number, and any custom built rpms would have a tag in the version/revision showing that they are not stock and "user beware" that they should check the README.functionality file (hmm, could that be generated on the fly from Makefile.menuselect?) that describes what functionality is included in the rpm.

I do agree with you that either 1) make rpm should always provide the same functionality or 2) (reading between the lines a bit) the version number should be modified to show that number 1 may not be true.
By: Tzafrir Cohen (tzafrir) 2008-03-04 11:36:34.000-0600

I have updated the patch apply and almost sort-of work with recent zaptel (svn branches/1.4). Setting status to "feedback", as the author needs to bring the patch to shape once again.

I have refreshed the patch to apply. The patch assumes .EXPORT_ALL_VARIABLES, which is not used anymore. Hence I have provided some variables explicitly.

* The generated version number from an SVN branch is: zaptel-SVNbranch1.4r3921M  . This is not a number. It may interact in strange ways with one installed from a tarball.
* hpec is not dealt with in the main makefile.
* I believe that the zaptel-perl stuff is useful even if you don't have an Astribank installed.
* /usr/share/zaptel is currently only used by xpp stuff. But this may change in the future.
* I removed installation of the hdlc ifup script. If your kernel actually supports it and you can make it work, try adding it. The code used in the script seem to have picked up the wrong location.
By: Tzafrir Cohen (tzafrir) 2008-06-17 02:35:54

I don't see a bugs repo yet for dahdi or its two sub projects. So I'll ask here:

How do we recommend to name its packages?
By: Daniel Hazelbaker (cabal95) 2008-07-07 13:14:04

Sorry, as I noted in one of the related issues, I was apparently not getting e-mails on this issue so didn't know you had ever updated this issue.  I was waiting for an official dahdi release, as I expected a number of changes in the naming conventions that I wanted to incorporate, to update the patch but since you have done some updates yourself I will get a patch up shortly that brings it up to date with my internal stuff.
By: Daniel Hazelbaker (cabal95) 2008-07-07 15:03:52

I have updated the patch to apply cleanly to the latest 1.4 svn (r4391). Fixed up various file locations and references so all files are packaged correctly (at least with my installation).

Version number: How should we tag it instead?  To my digging, in an svn checkout there is no .version file to say what version number it is based upon.  I am not sure we will even run into this that often, but I suppose we could try to modify the version "number" from "SVNbranch1.4r3921M" into something more like "1.4.0.r3921", though I am not sure that would be any better as it might technically be 1.4.11.  The revision number adds an automatic 10,000 if this is an SVN rpm, so that helps us there but we still need a sane version number.  Is there a way to pull the latest tagged version number from the revision number?

hpec: Not sure what you mean.  You are stating a fact is all I am understanding, you are correct. Makefile does not do anything with hpec directly.  I am assuming you meant more.

zaptel-perl: I am currently following the build sequence.  To my knowledge, the zaptel-perl stuff is only installed if xpp is selected, so I can only package it as same.

/usr/share/zaptel: fixed.

hdlc-ifup: Well, actually in the latest svn branch that I just checked out the hdlc-ifup installed (and it never had before for me) so I added it in to the packaged files list.

Package names: Was that to me in reference to naming the .rpm files or somebody/something else?
By: Daniel Hazelbaker (cabal95) 2008-07-15 17:04:42

Uploaded new patch. This patch completely replaces the mkspec file with one written from scratch. It should also be easier to read and understand than the previous version.
By: Digium Subversion (svnbot) 2008-09-08 15:06:28

Repository: asterisk
Revision: 141741

U   branches/1.4/Makefile
D   branches/1.4/redhat/

------------------------------------------------------------------------
r141741 | qwell | 2008-09-08 15:06:23 -0500 (Mon, 08 Sep 2008) | 8 lines

Remove RPM package targets from Makefile (and all associated parts).
This has never worked in 1.4, and we decided that it makes no sense to be done here.
There are many distros out there that already have "proper" spec files that can be (re)used.

Closes issue ASTERISK-12413
Closes issue DAHLIN-10
Closes issue ASTERISK-10503

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=141741