New patch for generic build script (was RE: Pending patches for generic build script)
Alan Miles
miles0201@cox.net
Wed Feb 25 01:52:00 GMT 2004
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
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: ChangeLog.txt
URL: <http://cygwin.com/pipermail/cygwin-apps/attachments/20040225/d106914f/attachment.txt>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: packaging_templates.diff
Type: application/octet-stream
Size: 13442 bytes
Desc: not available
URL: <http://cygwin.com/pipermail/cygwin-apps/attachments/20040225/d106914f/attachment.obj>
More information about the Cygwin-apps
mailing list