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]

[Review - no go] boxes (Was Re: Pending Packages List, 2004-02-13)


On Mon, 16 Feb 2004, Jari Aalto+mail.linux wrote:

> * 2004-02-13 Daniel Reed <nmlorg <AT> cygwin.com> list.cygwin-apps
>
> | Package: joe 2.9.8-1  [2003-11-11]
> | Description: Fast and simple editor which emulates 5 other editors
> | [snip]
> |  Good to go: Gerrit P. Haase
> |      Status: Attained required 3 votes. Package available. Reviewed.
> |    HOLD-UPS:
>
> repackaged
>
>     mkdir boxes ; cd boxes
>     wget -q -O - http://tierra.dyndns.org:81/cygwin/boxes/get.sh | sh

BTW, should this be "s/boxes/joe/g"?

> | Package: boxes 2000.0401-1  [2004-01-29]
> | Description: Text filter which can draw any kind of ASCII art box around its input text.
> |    Proposer: Jari Aalto
> |    Proposal: http://cygwin.com/ml/cygwin-apps/2004-01/msg00246.html
> |              http://tierra.dyndns.org:81/cygwin/boxes/boxes-2000.0401-1.tar.bz2
> |              http://tierra.dyndns.org:81/cygwin/boxes/boxes-2000.0401-1-src.tar.bz2
> |              http://tierra.dyndns.org:81/cygwin/boxes/setup.hint
> |    Problems: 21943873id2594723 The README reports the package version as 1.0.1.
> |              21943873id2594728 Running the boxes.exe executable results in a "boxes: Can't find config file." message.  Looks like it expects the config file in "/usr/local/share/boxes".
> |              21943873id2594734 The man page has a --GLOBALCONF-- string for the system-wide config file name -- should that have been replaced by something like "/usr/share/boxes/boxes.cfg"?
> |              21943873id2594742 Source problems: See cygwin-apps-get.13000
> |      Status: Attained required 3 votes. Package available.
> |    HOLD-UPS: Unresolved problems. No "good to go" review.
>
> Problems fixed and repackaged.
>
>     mkdir boxes ; cd boxes
>     wget -q -O - http://tierra.dyndns.org:81/cygwin/boxes/get.sh | sh

Ok, here's a second review.  Problems (and nits) outlined below:

setup.hint:

1) ldesc has leftover hyphenation ("pro- gramming"), and is all on one
line.

Binary package:

2) The man page reports "This is boxes version --BVERSION--.", and still
has the --GLOBALCONF-- string.  Something wrong with macro substitution,
perhaps?

3) The Cygwin-specific readme still indicates 1.3.22 as the minimum Cygwin
version.

Otherwise the binary package looks good.
Now for the source package:

-- patch

4) The patch contains boxes.README and boxes.README.b (the latter being
the old version of the README, AFAICS).

5) The patch to the Makefile changes the value of GLOBALCONF, but there's
a build.sh script in the patch that explicitly passes GLOBALCONF to make.
That's overkill, IMO -- the build.sh script isn't necessary for this.

6) Is there a reason you remove boxes.1 (a generated file) via patch?
This is where build.sh can come in handy (and make the patch smaller ;-).

7) There's a typo in the patch for src/Makefile: "+exec_prefix = $(refix)".
Frankly, I don't see how this ever built correctly...

-- script

8) The script will put the build files in /usr/src/cygwin-packages.  I do
NOT want any script to muck with my /usr/src unless I run it from there.
The package build should be contained under the current directory.

9) The build produces some warnings (mostly "implicit declaration" --
missing headers?).

10) Looking at the output, the script excludes generated files from the
patch, but, apparently, doesn't pick up boxes.1 as a generated file...
This could explain 6) above.

11) The usr/share/man, usr/share/doc/boxes-20000401, and usr/share/boxes
directories (and their subdirectories) end up as non-executable in the
binary tarball created by the script (wrong flags to install?).

12) The build doesn't happen in boxes-20000401/.build, it happens directly
in boxes-20000401...

13) The script leaves the boxes-20000401 directory behind...  Don't know
if it should be expected to clean it up, though...

Some of the above are nits, but some (like the build script using
/usr/src) are serious.
	Igor
-- 
				http://cs.nyu.edu/~pechtcha/
      |\      _,,,---,,_		pechtcha@cs.nyu.edu
ZZZzz /,`.-'`'    -.  ;-;;,_		igor@watson.ibm.com
     |,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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]