This is the mail archive of the 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]

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

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

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 ;)

Wapme Systems AG

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

phone: +49.211.74845.0
fax: +49.211.74845.299

Version: GnuPG v1.2.2 (Cygwin)


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