[Home]

Summary:ASTERISK-04019: [patch] changes to build system to remove run-time use of hardcoded paths (and other improvements)
Reporter:Jeffrey C. Ollie (jcollie)Labels:
Date Opened:2005-04-29 12:15:47Date Closed:2008-01-15 15:37:24.000-0600
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) .cvsignore
( 1) asterisk-pathconf-3.patch.txt
( 2) make_defaults_h
( 3) make_version_h
Description:(Text updated to more clearly reflect intention of this patch).

This patch:

1) Removes any hardcoded paths in applications so that file locations can be overridden at run-time by asterisk.conf.  The exceptions are in asterisk.c where the path to asterisk.conf (by default /etc/asterisk/asterisk.conf) is hardcoded as a fallback from the '-C' command-line option and hardcoded defaults will be used if asterisk.conf doesn't exist.

Having the various Asterisk paths available as macros leads to modules being written that use the macros to hardcode paths. This is bad because you can't relocate the files that that module uses without recompiling Asterisk or creating a lot of symlinks.

2) Eliminates "-DAST*" flags from the general compiler command line (to make it harder for future modules to hardcode paths).

3) Merges astconf.h into asterisk.h (astconf.h will need to be manually deleted).  This is just a simplification.

4) Moves build.h to include/asterisk in case other modules need access to the BUILD_* macros.

5) Creates a new "version.h" in include/asterisk so that modules can access the ASTERISK_VERSION* macros.

6) Removes date from ASTERISK_VERSION when building CVS HEAD because it didn't really reflect either the date/time that the CVS checkout was done and it wasn't really a build date either.  There is a BUILD_DATE define in asterisk/build.h now.




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

Disclaimer on file.
Comments:By: Jeffrey C. Ollie (jcollie) 2005-04-29 12:19:07

The .cvsignore file is supposed to go into include/asterisk.  The make_version_h file goes into the top directory.

By: Kevin P. Fleming (kpfleming) 2005-05-01 15:15:08

Thoughts:

- the build date being embedded in the version string _is_ useful, as long as the builder is using 'make update' to make their builds, since it forcibly removes the old version.h file before doing the 'cvs update'. Granted, that's not a 100% certainty, but it has been useful in the past. I think it's an acceptable change to have it in BUILD_DATE and have 'show version' display that information, though.

- this isn't really part of your patch, but it's related: can we make some more intelligent make_(foo)_h scripts that build the new version in a temp file, compare to the old version and don't replace if there are no changes? That way, we don't force excessive rebuild/relink when the relevant data has not changed.

- it's not clear to me why Makefile has some hardcoded-dependencies on these built header files; it seems to make more sense to have the 'depend' target dependent on them, which will accomplish the same purpose of forcing rebuilds when they change, but will also get the relevant source files listed in the .depend file (which they are not now, i.e. cli.c does not appear in .depend because mkdep gets run before build.h has been made)

If you can address those issues, I'll run this one up the flagpole and see what we get... All in all, I think it's a very worthwhile change.

By: Kevin P. Fleming (kpfleming) 2005-05-01 15:15:34

Note for committer: make_version_h and make_default_h both go into the top directory, and must be marked executable.

By: Jeffrey C. Ollie (jcollie) 2005-05-02 00:06:14

- In the patches that I have posted, the build date is included in the information displayed by "show version".  Note that this will require that cli.o be recompiled and the asterisk binary be relinked every time "make" is called (since build.h will change every time).  I'm not married to to having a build date, so if the consensus is to leave it out I'm fine with that.

- I've modified the Makefile to create temporary .h files, compare them to the old versions and overwrite the old versions if any changes were detected.  That will mean fewer rebuilds.

- The autogenerated .h files do not exist at the time "make depend" runs and it does not appear to detect those dependencies.  If you comment out the dependencies the .h files will not be autogenerated and the compiles of cli.c and asterisk.c will error out because of the missing files.

By: Kevin P. Fleming (kpfleming) 2005-05-02 01:00:23

For the last issue, I believe if you make the 'depend' target in the top-level Makefile explicitly dependent on build.h/version.h/defaults.h then it will work as I wanted it to... 'make' will build them first, then run mkdep, then go on into the result of the build.

(Note that I haven't completely tested that theory, but I did modify the CVS HEAD Makefile to move the build.h dependency to the .depend target and it worked fine).

By: Jeffrey C. Ollie (jcollie) 2005-05-02 01:04:39

One interesting little tidbit... in the current makefile, the value of "ASTERISK_VERSION" actually changes depending on where in the build you are, until you get to a point where the .version file gets written out.  You can verify this yourself by running "make clean all 2>&1 | tee build.log" and looking for the string "CVS-HEAD".

By: Jeffrey C. Ollie (jcollie) 2005-05-02 09:40:50

Ok, here's a version of the patch where "make depend" depends on the autogenrated .h files.

By: Kevin P. Fleming (kpfleming) 2005-06-05 23:02:45

Committed to CVS HEAD (yes, it finally happened!). Thanks!

By: Digium Subversion (svnbot) 2008-01-15 15:37:21.000-0600

Repository: asterisk
Revision: 5855

_U  trunk/
U   trunk/.cleancount
U   trunk/.cvsignore
U   trunk/Makefile
U   trunk/app.c
U   trunk/apps/app_adsiprog.c
U   trunk/apps/app_chanspy.c
U   trunk/apps/app_directory.c
U   trunk/apps/app_hasnewvoicemail.c
U   trunk/apps/app_ices.c
U   trunk/apps/app_math.c
U   trunk/apps/app_meetme.c
U   trunk/apps/app_queue.c
U   trunk/apps/app_sms.c
U   trunk/apps/app_test.c
U   trunk/apps/app_voicemail.c
U   trunk/asterisk.c
U   trunk/cdr/cdr_csv.c
U   trunk/cdr/cdr_custom.c
U   trunk/cdr/cdr_manager.c
U   trunk/channels/chan_iax2.c
U   trunk/cli.c
U   trunk/config.c
U   trunk/db.c
U   trunk/file.c
U   trunk/image.c
D   trunk/include/astconf.h
U   trunk/include/asterisk.h
U   trunk/loader.c
U   trunk/logger.c
U   trunk/make_build_h
U   trunk/pbx/pbx_config.c
U   trunk/pbx/pbx_spool.c
U   trunk/res/res_agi.c
U   trunk/res/res_crypto.c
U   trunk/res/res_monitor.c
U   trunk/res/res_musiconhold.c

------------------------------------------------------------------------
r5855 | kpfleming | 2008-01-15 15:37:21 -0600 (Tue, 15 Jan 2008) | 2 lines

major Makefile and build process improvements, including removal of all hardcoded paths (modules must now use run-time paths as they should) (bug ASTERISK-4019)

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

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

By: Digium Subversion (svnbot) 2008-01-15 15:37:23.000-0600

Repository: asterisk
Revision: 5857

A   trunk/make_defaults_h
A   trunk/make_version_h

------------------------------------------------------------------------
r5857 | kpfleming | 2008-01-15 15:37:22 -0600 (Tue, 15 Jan 2008) | 2 lines

add missing files from bug ASTERISK-4019 commit

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

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

By: Digium Subversion (svnbot) 2008-01-15 15:37:24.000-0600

Repository: asterisk
Revision: 5858

U   trunk/apps/app_dictate.c

------------------------------------------------------------------------
r5858 | kpfleming | 2008-01-15 15:37:23 -0600 (Tue, 15 Jan 2008) | 2 lines

more breakage from bug ASTERISK-4019 commit

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

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