[Review - not yet] Re: [ITP] tree

Stipe Tolj tolj@wapme-systems.de
Thu Dec 18 17:41:00 GMT 2003


Igor Pechtchanski wrote:
> 
> Sounds interesting.  I'll vote for this.

I'll update this after we resolve this issues here.

> Now for the review:
> 
> 1) The documentation is still in /usr/doc and /usr/man...  Any plans on
>    moving it to /usr/share/{doc,man}?

ok, changed.

> 2) This uses method 1 packaging.  AFAIU, the patch in CYGWIN-PATCHES
>    should still be a forward patch.  Yours is reverse.

I did reverse patching for thre previous packages apache, php, etc
too. Non was objected until know. AFAIK, either you have the orginal
source tree and your patch provides the cygwin specific changes or
oposite. Actually this may you apply the patch to produce the orginal
source tree.

> 3) Any particular reason you have the make and install logs in
>    CYGWIN-PATCHES?  Also, the make log contains a warning that looks
>    suspicious, and the install log shows that "make install" will install
>    files directly into /usr/bin, rather than to a subdirectory.  Also,
>    "make install" will not put the Cygwin-specific README in the right
>    directory.

historical. AFAIK some people have been doing this for "documentation
purpose" while building packages. The log files may be helpful in
identifing possible problems that someone may observe on his local
installation.

Is it really required to patch Makefiles in order to "install" the
cygwin specific README to the ..doc/Cygwin place?! I consider this an
unneading hacking in sources. 

IMO, sources should be changed as minimally as required.

> 4) The file list in the Cygwin-specific README mentions /usr/bin/tree, and
>    not /usr/bin/tree.exe, which, IMO, is misleading.

ok, changed.

> 5) There are no port notes in the Cygwin-specific README, even though
>    there are some user-visible changes in the patch, such as changing the
>    prefix to /usr and removing the "-s" linker flag (any particular reason
>    why you did that?)

-s linker flag put in place again (was a simple typo). So we get
stripping again. I don't see any needs for port notes to be honest.

> 6) I don't see a "strip" step anywhere in the installation process
>    (although the executable in the binary tarball is stripped).  That's
>    actually what the -s linker flag does...

(see above)

> 7) There is a bug in the printing of inode numbers, since ino_t is 64-bit
>    in Cygwin now, and it's printed using "%ld" (tree.c:515).

yep, patched.

> 8) There is a bug that the file size is declared as u_long, and will be
>    truncated for large files.  Why not simply change it to off_t?

any scenario you can give that reproduces this?!

> I think this is it.  A lot of the above is very easy to fix, so we'll wait
> for a new installment soon, eh? ;-)

let's get this going.

BTW, @Volker: you should have voted to the -apps list too ;)

Stipe

mailto:tolj@wapme-systems.de
-------------------------------------------------------------------
Wapme Systems AG

Münsterstr. 248
40470 Düsseldorf, NRW, Germany

phone: +49.211.74845.0
fax: +49.211.74845.299

mailto:info@wapme-systems.de
http://www.wapme-systems.de/
-------------------------------------------------------------------

-----BEGIN PGP PUBLIC KEY BLOCK-----
Version: GnuPG v1.2.2 (Cygwin)

mIsEP6mcYwEEAMDnUiUwrbb+xwTFWN6TxF2+XZu7/alwJMeCwMBRvXtPZqfjpPhS
OkBpU0F4TrVuugz1HINTSaJTYq10AzDQXp5NkyWgckqW79nPAWuOX0dicbJk+cN2
nM2TI4KaxUDe6u8hghNEnH/i2lXsUu9apnP/iixzV81VC2je3uc9hZpnAAYptEVT
dGlwZSBUb2xqIChUZWNobm9sb2d5IENlbnRlciAmIFJlc2VhcmNoIExhYikgPHRv
bGpAd2FwbWUtc3lzdGVtcy5kZT6ItAQTAQIAHgUCP6mcYwIbAwYLCQgHAwIDFQID
AxYCAQIeAQIXgAAKCRABV0w1BqPYRuSqA/wPzsQxao2YePENCtgRTrO86U6zg3sl
OcS6CJFI4FZP5h/xD3GRsNH1+MPSvZlomDdpFnr547DGz/Kq9MXuQwVvlVig5yWZ
K5dtKp1r5YLhxJQBhfirZbRFFnYmf19f18J8OoS28tuFVftDl1AIwJS3HLyBTv6H
g2HyLAEKQIp30Q==
=aYCI
-----END PGP PUBLIC KEY BLOCK-----



More information about the Cygwin-apps mailing list