New patch for generic build script (was RE: Pending patches for generic build script)
All, I am attaching a new release of the this patch to mitigate below. Igor, please review and advise. Note: I built against 1.19 for the generic-build-script and 1.7 for the generic readme. Change log entry: 2004-02-24 Alan Miles [EMAIL PROTECTED] * templates/generic-readme : Changed the runtime requirements string from other requirements, e.g., libintl1 to: other Runtime requirements, e.g., libintl1 allowing better text replacement using the values generated by depend() function. The generic build script will replace this string in its function readmelist(). * templates/generic-readme : Changed the build requirements string from other requirements, e.g., gettext to: other Build requirements, e.g., gettext allowing better text replacement using values from the generic build script, which will replace this string in its function readmelist(). * templates/generic-readme : Replaced the listed files under the Files included section with the string THEFILES. The generic build script will replace the string THEFILES with a sorted list of files which will comprise the binary distribution. The generic build scripts readmelist() function does this. * templates/generic-readme : Changed the port notes string from Other information to: Other Port information allowing better text replacement using values from the generic build script, which will replace this string in its function readmelist(). * templates/generic-readme : Changed the older release string from PKG-older VER-1 to: PKG-older VER-older REL since the release number may not be 1. The generic build script will analyse the setup.hint file and set the values of older VER and older REL, substituting these values. If the script cannot find the setup.hint file, these values become values of VER and REL respectively.Again the generic build script's readmelist() and hintfiledata() functions do the substitutions and extract the data from the setup.hint file. * templates/generic-build-script : Added defaults and instructions to Package maintainers regarding some new constants: export SHORTDESC=short description, 2 or 3 lines export OTHERRUNTIME= other Runtime requirements, e.g., libintl1 export OTHERBUILD= other Build requirements, e.g., gettext export OTHERPORT=Other Port information export OLDERVER=${VER} # Set the default to the current package's version number export OLDERREL=${REL} # Set the default to the current package's release number export MAINTNAME=Your Name Here export MAINTEMAIL=your email here The readmelist() function substitutes these values into the binary package README. Note: Function hintfiledata() overrides the values of SHORTDESC, setting it to the setup.hint file's ldesc value, and obtains the OLDERVER, OLDERREL values from the setup.hint if it contains the prev: value * templates/generic-build-script : Added function hintfiledata() which parses and extracts the ldesc: value and prev: values, if the ${srcdir}/CYGWIN-PATCHES/setup.hint file exists, altering the SHORTDESC, OLDERVER, OLDERREL values respectively. * templates/generic-build-script : Added function readmelist() which examines all the files in ${instdir}, sorting them and then replacing the THEFILES string with the lines. This replacement exploits sed's e command, which allows one to pipe input from a shell command into pattern space. The command does the following replacements: (a) string PKG becomes the value of ${PKG} (b) string VER becomes the value of ${VER} (c) string REL becomes the value of ${REL} (d) string package name becomes the value of ${PKG} (e) string short description, 2 or 3 lines becomes the value of ${SHORTDESC} (f) string other Build requirements, e.g., gettext becomes the value of ${OTHERBUILD} (g) string Other Port information becomes the value of ${OTHERPORT} (h) string older VER becomes the value of ${OLDERVER} (i) string older REL becomes the value of ${OLDERREL} (j) string Your Name Here becomes the value of ${MAINTNAME} (k) string your email here becomes the value of ${MAINTMAIL} (l) string other Runtime requirements, e.g., libintl1 becomes the value of sorted depends() function. * templates/generic-build-script : Added the call to function readmelist() in the all list * templates/generic-build-script : Fixed a problem in the functions strip(), mkpatch(), and depend() when there are no dll's or exes (the find returns no files) - xargs would try to execute strip or cygcheck/cygpath with no
Re: New patch for generic build script
Alan, I was going to say that I'll review the patch later, and offer a few quick comments, but it looks like I did a full patch review, so here goes: (1) WOW, this is a HUGE ChangeLog. Wa-ay too detailed. Most of this stuff should go into comments in the script, if you feel like documenting it at all and if it isn't self-documenting. I also forgot to include a link to the Cygwin ChangeLog guidelines (http://cygwin.com/contrib.html, based on the GNU ChangeLog guidelines, to which there's a link on the above page), so please review that. Just to give you a feel for what would be the right level of detail, here's an attempt to shorten this: 2004-02-24 Alan Miles alan.milesatieee.org * templates/generic-readme: Fix phrasing for better pattern matching. Parameterize initial release version. * templates/generic-build-script: Add 'readmelist' argument that calls readmelist(). Add readmelist() to the 'all' sequence. Define constant strings for pattern matching. (readmelist): New function to update the README. (hintfiledata): New function to extract setup.hint data. (strip,mkpatch,depend): Add xargs option to prevent complaints when no executables are found. (install): Parameterize README filename. Even that might be too detailed, but at least it's manageable. :-) I may have missed something, too. (2) It's usually a good idea to submit independent changes as separate patches. For example, I'd do the xargs change separately. I'm not sure if the rest of the patch is acceptable as-is (pending further review), but I'd suggest singling this change out and sending a separate short patch. (3) Please, please set your linewrap at 80 columns (for both the patch and the ChangeLog). It might even be better for the ChangeLog to be wrapped at 76 columns or so... (4) I'd like to keep the order or the file entries (i.e., /etc/postinstall at the end). I also think it would be good to keep the (this file) after the README entry. (5) Frankly, I don't see the point in the first hunk of the readme patch. You can just as well match all of the lines between 'Runtime requirements' and 'other requirements.*' (or 'Build requirements'...). (6) I've been thinking that it might actually be better to have a computer-readable README.in, and a separate 'gen_readme' script that would produce a README from it. The reasoning here is that the package build script is invoked not only by the package maintainers, but also (hopefully) every time someone builds a package from sources. Do we really want to regenerate the README for these people? Besides, the dependences are all going to be different, at the very least. Alternatively, leave the functionality in the generic-build-script, but don't call it by default (like signing). (7) If the variable's value comes from the ldesc field, why not name it LDESC? You might want to give similar descriptive naming to other variables. (8) The OTHERPORT idea is wrong, wrong, wrong. I mentioned this earlier, but I guess I wasn't articulate enough. OTHERPORT is actually a whole change log for the previous ports. There should be no reason to regenerate it, and certainly no reason to stick the port notes information into the script. It might be ok to have the *latest* version's changes in the script (after all, that's the version that this script builds), but the others should be put into the readme only once. Perhaps we should table it and get the rest of the patch going. In fact, let's do this a couple of patterns at a time. Some things can be easily extracted from other places, such as setup.hint, so for those it makes sense to have auto-generation. If the maintainer has to modify a variable in the script to put some text into the README, why not put it directly into the README and avoid the hassle? If you think that having only one file to maintain is easier, I doubt that's true -- eventually the 'one-size-fits-all' generic-build-script will become a maintenance nightmare, especially with trying to pull in the latest changes from CVS. So, please split your patch into multiple contained patches, each of which has one independent part of this functionality. We'll look at one patch at a time, and decide what to do with them on a patch-by-patch basis, rather than trying to get approval for one monster patch. Ooh, boy, I just read all of the above... I'm getting really verbose as the night grows older. Good thing I'm not writing ChangeLogs. :-) In your words: now to bed... Igor P.S. I'm too tired to snip out the relevant parts from the message below. Apologies to all for a long quote. On Tue, 24 Feb 2004, Alan Miles wrote: All, I am attaching a new release of the this patch to mitigate below. Igor, please review and advise. Note: I built against 1.19 for the generic-build-script and 1.7 for the generic readme. Change log entry: 2004-02-24 Alan Miles alan.milesatieee.org
Re: New patch for generic-build-script
On Wed, 18 Feb 2004, Yaakov Selkowitz wrote: Igor, OK, here's the patch again, along with my ChangeLog. Feel free to edit as necessary. Yaakov 2004-02-17 Yaakov Selkowitz yselkowitz AT users DOT sourceforge DOT net * generic-build-script: Add 'configure', 'make', and 'test' arguments as aliases for 'conf', 'build', and 'check' respectively. Add 'first' argument that calls mkdir(), spkg(), and finish(). Add checksig() to beginning of 'all' sequence. (install_docs): Add ABOUT-NLS, FAQ, THANKS. (SIG): Define as 1 (on by default). (reconf): New function for clean reconfigure. (install): Clean ${instdir} before install. Automatically gzip info and man pages, if present. Code cleanup. (list): Simplify sed expression. (depend): New function for listing runtime dependencies of package executables and libraries. (strip,mkpatch): Remove redirection of output to /dev/null. (spkg): Improve readability. Change test of ${SIG} to equality. (checksig): Change order to ORIGINAL PACKAGE, SCRIPT, PATCH. Change test of ${SIG} to equality. Yaakov, This looks good. The only two comments I have are: 1) You replaced ''s by ';' after 'fi's in install() (where they were needed), and left the one after 'find' in mkpatch() (where it was not). This is minor, and I edited the patch accordingly[*] -- just letting you know to expect conflicts with your local copy after the patch is applied. 2) Did you really mean to turn package signing on by default? I, for example, don't use package signing for my packages, and don't even have GPG installed. I'd rather the default setting was to not sign the packages, and signing could be turned on on a package-by-package basis. If you don't mind, I'll change that '1' to a '0' for now (please speak up if you disagree). Alternatively, we could check for the existence of /usr/bin/gpg as one of the factors for turning on package signing, something like 'SIG=`[ ! -x /usr/bin/gpg ]; echo $?`', but that could come in a later patch. Once you respond to this, I'll apply the patch. Thanks for contributing. Igor [*] FYI, here are the diffs between the edited patch and the original: @@ -44,7 +44,7 @@ TODO \ export test_rule=test -+export SIG=1 #set to 0 to turn off signing by default ++export SIG=0 #set to 1 to turn on signing by default # helper function # unpacks the original package source archive into ./${BASEPKG}/ @@ -75,12 +75,12 @@ done \ + if [ -d ${instdir}${prefix}/share/info ] ; then \ +find ${instdir}${prefix}/share/info -name *.info | xargs gzip -q ; \ -+ fi ; \ ++ fi \ + if [ -d ${instdir}${prefix}/share/man ] ; then \ +find ${instdir}${prefix}/share/man -name *.1 -o -name *.3 -o \ + -name *.3x -o -name *.3pm -o -name *.5 -o -name *.6 -o \ + -name *.7 -o -name *.8 | xargs gzip -q ; \ -+ fi ; \ ++ fi \ templist= \ for f in ${install_docs} ; do \ if [ -f ${srcdir}/$f ] ; then \ @@ -126,7 +126,7 @@ mkpatch() { (cd ${srcdir} \ - find . -name autom4te.cache | xargs rm -rf /dev/null ; \ -+ find . -name autom4te.cache | xargs rm -rf \ ++ find . -name autom4te.cache | xargs rm -rf ; \ unpack ${src_orig_pkg} \ mv ${BASEPKG} ../${BASEPKG}-orig \ cd ${topdir} \ -- http://cs.nyu.edu/~pechtcha/ |\ _,,,---,,_ [EMAIL PROTECTED] ZZZzz /,`.-'`'-. ;-;;,_[EMAIL PROTECTED] |,4- ) )-,_. ,\ ( `'-'Igor Pechtchanski, Ph.D. '---''(_/--' `-'\_) fL a.k.a JaguaR-R-R-r-r-r-.-.-. Meow! I have since come to realize that being between your mentor and his route to the bathroom is a major career booster. -- Patrick Naughton
Re: New patch for generic-build-script
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Igor Pechtchanski wrote: | Yaakov, | | This looks good. The only two comments I have are: | | 1) You replaced ''s by ';' after 'fi's in install() (where they were | needed), and left the one after 'find' in mkpatch() (where it was not). | This is minor, and I edited the patch accordingly[*] -- just letting you | know to expect conflicts with your local copy after the patch is applied. Oh, THAT'S what you meant. I missed that one. Sorry. | 2) Did you really mean to turn package signing on by default? I, for | example, don't use package signing for my packages, and don't even have | GPG installed. I'd rather the default setting was to not sign the | packages, and signing could be turned on on a package-by-package basis. | If you don't mind, I'll change that '1' to a '0' for now (please speak up | if you disagree). Alternatively, we could check for the existence of | /usr/bin/gpg as one of the factors for turning on package signing, | something like 'SIG=`[ ! -x /usr/bin/gpg ]; echo $?`', but that could come | in a later patch. | | Once you respond to this, I'll apply the patch. Thanks for contributing. | Igor | [*] FYI, here are the diffs between the edited patch and the original: Alright, everything looks fine. Go ahead and check it in. When I see this clear on cygwin-cvs-apps, I'll start looking at my next round of patches. Thanks! Yaakov -BEGIN PGP SIGNATURE- Version: GnuPG v1.2.3 (MingW32) Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org iD8DBQFAM3t0piWmPGlmQSMRAgvpAKDq6OiH5mhJY8fg+rF0G7VB9v5WnQCgrwub 0vClXN56GT+7L+W7kDiUSuo= =5UFz -END PGP SIGNATURE-
Re: New patch for generic-build-script
On Wed, 18 Feb 2004, Yaakov Selkowitz wrote: Igor Pechtchanski wrote: | Yaakov, | | This looks good. The only two comments I have are: | | 1) You replaced ''s by ';' after 'fi's in install() (where they were | needed), and left the one after 'find' in mkpatch() (where it was not). | This is minor, and I edited the patch accordingly[*] -- just letting you | know to expect conflicts with your local copy after the patch is applied. Oh, THAT'S what you meant. I missed that one. Sorry. | 2) Did you really mean to turn package signing on by default? I, for | example, don't use package signing for my packages, and don't even have | GPG installed. I'd rather the default setting was to not sign the | packages, and signing could be turned on on a package-by-package basis. | If you don't mind, I'll change that '1' to a '0' for now (please speak up | if you disagree). Alternatively, we could check for the existence of | /usr/bin/gpg as one of the factors for turning on package signing, | something like 'SIG=`[ ! -x /usr/bin/gpg ]; echo $?`', but that could come | in a later patch. | | Once you respond to this, I'll apply the patch. Thanks for contributing. | Igor | [*] FYI, here are the diffs between the edited patch and the original: Alright, everything looks fine. Go ahead and check it in. When I see this clear on cygwin-cvs-apps, I'll start looking at my next round of patches. Thanks! Yaakov Applied, thanks. Igor -- http://cs.nyu.edu/~pechtcha/ |\ _,,,---,,_[EMAIL PROTECTED] ZZZzz /,`.-'`'-. ;-;;,_[EMAIL PROTECTED] |,4- ) )-,_. ,\ ( `'-' Igor Pechtchanski, Ph.D. '---''(_/--' `-'\_) fL a.k.a JaguaR-R-R-r-r-r-.-.-. Meow! I have since come to realize that being between your mentor and his route to the bathroom is a major career booster. -- Patrick Naughton
New patch for generic-build-script
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Igor, OK, here's the patch again, along with my ChangeLog. Feel free to edit as necessary. Yaakov 2004-02-17 Yaakov Selkowitz yselkowitz AT users DOT sourceforge DOT net * generic-build-script: Add 'configure', 'make', and 'test' arguments as aliases for 'conf', 'build', and 'check' respectively. Add 'first' argument that calls mkdir(), spkg(), and finish(). Add checksig() to beginning of 'all' sequence. (install_docs): Add ABOUT-NLS, FAQ, THANKS. (SIG): Define as 1 (on by default). (reconf): New function for clean reconfigure. (install): Clean ${instdir} before install. Automatically gzip info and man pages, if present. Code cleanup. (list): Simplify sed expression. (depend): New function for listing runtime dependencies of package executables and libraries. (strip,mkpatch): Remove redirection of output to /dev/null. (spkg): Improve readability. Change test of ${SIG} to equality. (checksig): Change order to ORIGINAL PACKAGE, SCRIPT, PATCH. Change test of ${SIG} to equality. -BEGIN PGP SIGNATURE- Version: GnuPG v1.2.3 (MingW32) Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org iD8DBQFAMvcHpiWmPGlmQSMRAnB1AJ94zOX6FJ7Y8LInjO5V091+e3YrMQCfXM6z nUQa5TGs7L7o6lPUbsMerYc= =otD5 -END PGP SIGNATURE- --- generic-build-script-orig 2004-02-17 19:23:24.62196 -0500 +++ generic-build-script2004-02-18 00:18:01.195696000 -0500 @@ -80,6 +80,7 @@ MY_LDFLAGS= export install_docs=\ + ABOUT-NLS \ ANNOUNCE \ AUTHORS \ BUG-REPORTS \ @@ -88,6 +89,7 @@ COPYING \ CREDITS \ ChangeLog \ + FAQ \ HOW-TO-CONTRIBUTE \ INSTALL \ KNOWNBUG \ @@ -97,9 +99,11 @@ NOTES \ PROGLIST \ README \ + THANKS \ TODO \ export test_rule=test +export SIG=1 #set to 0 to turn off signing by default # helper function # unpacks the original package source archive into ./${BASEPKG}/ @@ -134,6 +138,12 @@ --libexecdir='${sbindir}' --localstatedir=/var \ --datadir='${prefix}/share' ) } +reconf() { + (cd ${topdir} \ + rm -fr ${objdir} \ + mkdir -p ${objdir} \ + conf ) +} build() { (cd ${objdir} \ CFLAGS=${MY_CFLAGS} make ) @@ -148,6 +158,7 @@ } install() { (cd ${objdir} \ + rm -fr ${instdir}/* \ make install DESTDIR=${instdir} \ for f in ${prefix}/share/info/dir ${prefix}/info/dir ; do \ if [ -f ${instdir}${f} ] ; then \ @@ -159,6 +170,14 @@ mkdir -p ${instdir}${d} ;\ fi ;\ done \ + if [ -d ${instdir}${prefix}/share/info ] ; then \ +find ${instdir}${prefix}/share/info -name *.info | xargs gzip -q ; \ + fi ; \ + if [ -d ${instdir}${prefix}/share/man ] ; then \ +find ${instdir}${prefix}/share/man -name *.1 -o -name *.3 -o \ + -name *.3x -o -name *.3pm -o -name *.5 -o -name *.6 -o \ + -name *.7 -o -name *.8 | xargs gzip -q ; \ + fi ; \ templist= \ for f in ${install_docs} ; do \ if [ -f ${srcdir}/$f ] ; then \ @@ -172,11 +191,9 @@ if [ -f ${srcdir}/CYGWIN-PATCHES/${PKG}.README ]; then \ /usr/bin/install -m 644 ${srcdir}/CYGWIN-PATCHES/${PKG}.README \ ${instdir}${prefix}/share/doc/Cygwin/${BASEPKG}.README ; \ - else \ -if [ -f ${srcdir}/CYGWIN-PATCHES/README ]; then \ - /usr/bin/install -m 644 ${srcdir}/CYGWIN-PATCHES/README \ -${instdir}${prefix}/share/doc/Cygwin/${BASEPKG}.README ; \ -fi ;\ + elif [ -f ${srcdir}/CYGWIN-PATCHES/README ] ; then \ +/usr/bin/install -m 644 ${srcdir}/CYGWIN-PATCHES/README \ + ${instdir}${prefix}/share/doc/Cygwin/${BASEPKG}.README ; \ fi \ if [ -f ${srcdir}/CYGWIN-PATCHES/postinstall.sh ] ; then \ if [ ! -d ${instdir}${sysconfdir}/postinstall ]; then \ @@ -188,12 +205,19 @@ } strip() { (cd ${instdir} \ - find . -name *.dll -or -name *.exe | xargs strip /dev/null 21 ; \ + find . -name *.dll -or -name *.exe | xargs strip 21 ; \ true ) } list() { (cd ${instdir} \ - find . -name * ! -type d | sed 's/\.\/\(.*\)/\1/' ; \ + find . -name * ! -type d | sed 's%^\.% %' ; \ + true ) +} +depend() { + (cd ${instdir} \ + find ${instdir} -name *.exe -o -name *.dll | xargs cygcheck | \ + sed -e '/\.exe/d' -e 's,\\,/,g' | sort -bu | xargs -n1 cygpath -u \ + | xargs cygcheck -f | sed 's%^% %' ; \ true ) } pkg() { @@ -202,7 +226,7 @@ } mkpatch() { (cd ${srcdir} \ - find . -name autom4te.cache | xargs rm -rf /dev/null ; \ + find . -name autom4te.cache | xargs rm -rf \ unpack ${src_orig_pkg} \ mv ${BASEPKG} ../${BASEPKG}-orig \ cd ${topdir} \ @@ -213,12 +237,18 @@ } spkg() { (mkpatch \ - if [ ${SIG} ]; then name=${srcinstdir}/${src_patch_name} text=PATCH sigfile; fi \ + if [ ${SIG} -eq 1 ] ; then \ +name=${srcinstdir}/${src_patch_name} text=PATCH sigfile ; \ + fi \ cp