This is the mail archive of the cygwin-apps@cygwin.com mailing list for the Cygwin project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
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.miles@ieee.org> * 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 arguments and these commands would complain about missing arguments. The fix is to add the -r parameter to xargs, which prevents xargs from executing its parameters if there is no input. * templates/generic-build-script : Altered references in install() from: "${instdir}${prefix}/share/doc/Cygwin/${BASEPKG}.README" to: "${PkgReadMe}" thus allowing only one definition of the README - "readmelist()" uses "${PkgReadMe}" -----Original Message----- From: cygwin-apps-owner@cygwin.com [mailto:cygwin-apps-owner@cygwin.com]On Behalf Of Alan Miles Sent: February 10, 2004 23:25 To: cygwin-apps@cygwin.com Subject: RE: Pending patches for generic build script Igor, Small attempt at humour here ... I guess I was just trying to provide a means to get the "listing" inside the readme, together was some variables like VER reducing the maintainers job. However, I will review your comments and suggestions etc. [snip] >> Umm, there are two statements above. The second is actually a question >> about ChangeLogs. It refers to the following two sentences from earlier >> in the message: >> > I'll use this message as a sort of a ChangeLog -- please let me know if >> > you'd rather construct your own ChangeLog entries. >> and later >> > Should I use the accompanying message for the ChangeLog? >> > >> > [3] http://cygwin.com/ml/cygwin-apps/2004-01/msg00042.html >> You haven't answered this (frankly, it's kind of hard to reconstruct a >> ChangeLog entry from your message anyway). It would be great if you >> recreated the patch against the latest CVS, taking my comments below into >> account, and resubmitted it with a ChangeLog (see packaging/ChangeLog in >> CVS for examples). I'll need to review the ChangeLog as I am not familiar with this piece (as suggested I need to "see packaging/ChangeLog in CVS for examples") - also back to the cygwin site to review submissions as well just to ensure I do this correctly ... >> Ok, now for the patch comments: >> 1) I'm a bit wary of modifying "list()" to update the README. I'd much >> rather have a separate function that does this. I guess I missed the point of what list() was to do - however, yes I agree it should be a different name >> 2) If you're replacing the file list, why not do the whole thing? Also, >> you could try sorting them in the order of the README (by playing with >> the order of paths supplied to 'find'). Alternatively, you'll need to >> filter out the files that *are* present as templates in the README from >> %THEFILES%. I wasn't sure what %THEFILES% entries a submitter needed to be compliant with, so I left a few items in. It is late, and my brain isn't functioning well to ensure I understand your comment correctly, so I will review 2) when my brain is clearer - I think I now what you want, but I cannot reason it out correctly right now. >> 3) What you're doing with %NEW_VER% is completely wrong. Technically, >> %NEW_VER% should always be equal to %VER%-%REL% (since it is the latest >> version that you're packaging). Also, you should leave the change >> history ("version %VER%" in your patch) well enough alone -- it should >> be static. I suggest taking it out of your patch for now and tackling >> it later (see <5>). I'll need to review this and make the corrections as needed ... >> 4) [Optional] I just realized that most people put the same text as >> project description as that in the "ldesc:" field of setup.hint (which >> should be in CYGWIN-PATCHES). It would be great if it could be >> extracted and placed into the README. The setup.hint itself could also >> be verified using the "depend()" function in Yaakov's patch. That same >> function could also be used to generate run-time dependences in the >> README. Sounds interesting and do-able - I am not familiar with Yaakov's patch [yet]... >> 5) [Optional] One of the things that a package maintainer may forget is to >> update the README with the porting notes for the latest version. If >> there was a mechanism for checking that the latest (%VER%-%REL%) >> version's porting notes are present in the section, and that version >> numbers in that section monotonically decrease, that would be helpful. >> The logic for comparing version numbers can be borrowed from the >> "upset" script. Also, if the porting notes for %VER%-%REL% *aren't* >> present, the script could insert a placeholder and then pop up the >> README in an editor (even with the cursor at the correct line). Sounds interesting and do-able - since I have a copy of the upset script (which I use to build a setup.ini file for my custom installation to get installed through setup) I could look at this too. Obviously, I will need to ensure my upset script is the latest from CVS. Again brain challenged, so will review your comments when my brain is more functional... >>6) Some *minor* nits about the patch: >> - "ThePackageReadmeFile", while a long and descriptive variable name, >> doesn't follow the variable name conventions in the surrounding code >> (which calls for something like "readmefile"); >> - Similarly, the rest of the script uses a "'then' on the same line as >> 'if'" convention. It would be good if it were followed. >> - I know the script is not reentrant, and that it's unlikely that >> someone will have a %PKG%-%VER%.README in their /tmp, but still, for >> peace of mind, could you please use "mktemp"? Or, if you don't want >> to add another dependence, at least use "$$" in the filename? >> - Some of your sed expressions suffer from LTS ("Leaning Toothpick >> Syndrome"), which is easily fixable by using an alternate separator >> character (e.g., "#"). You do use it in some places, but not >> everywhere it's needed. >> - Please don't indent the line continuation backslash all the way to >> the end -- one space is quite enough. Ditto for '&&'. Redirection, >> on the other hand, can be outdented to be a couple of spaces past the >> sed command, with the '>' on the same line as the output file. >> - One sed expression for everything is neat, but rather hard to read. >> It would help if you used comments. Also, a consistent separator >> character for the 's' command would be great. Some of this stylistic, but understood. I need to review mktemp (man mktemp) or the $$ items. Agreed I need additional comments, and the fix for LTS might help in readability as well. >> Well, hopefully this doesn't scare you off -- this seems like useful >> functionality. Oh, and you might want to wait a bit before working on the >> patch -- I'm planning to make some changes to the generic-build-script in >> the next few days that will make it easier to implement similar >> functionality. Not scared off - if it does not yet pass muster then it needs fixing. I will wait a few days to ensure I get the latest CVS files. With this weekend coming up, I don't know how much time I will get but the delay will buy me some time. Now to bed ... have to go to work tomorrow ... Alan
Attachment:
ChangeLog.txt
Description: Text document
Attachment:
packaging_templates.diff
Description: Binary data
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |