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]

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


On Thu, 18 Dec 2003, Stipe Tolj wrote:

> Igor Pechtchanski wrote:
> >
> > Sounds interesting.  I'll vote for this.
>
> I'll update this after we resolve this issues here.
>
> > Now for the review:
> > [snip]
> > 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.

Hmm, I guess it's not that important.  I was just going by what's on
<http://cygwin.com/setup.html>, which clearly states that applying the
patch *in reverse* should get you the original sources...

> > 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.

True.  It's usually a good idea, though, to be able to install somewhere
outside of the main tree (e.g., to recreate the package without polluting
your system)...  I'm not sure how important it is that a package is able
to do this.

> 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.

I agree, that's why method 2 became popular, since you don't have to
change the Makefiles for some of the Cygwin-specific stuff.  But if you
stick with method 1, the users should be able to fully install the package
using "make install"...

> [snip]
> > 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.

Well, I think changing the prefix from /usr/local to /usr is a pretty
serious change, in a sense that someone familiar with the package and
wanting to install it on their system will download it, run "make; make
install", and get the packages in /usr/bin instead of /usr/local/bin where
they would expect them.  IMO, it deserves a mention in the port notes, but
I'm not going to hold up the release of a package because of this.

> > 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?!

Well, I don't have any 4G+ files on my system, but it's easy to see that
anything with size over 4G will be displayed incorrectly.

In fact, most of the LINUX_BIGFILE code should be turned on, except that
the types are wrong in some places ("long long" instead of off_t), and
Cygwin has no such thing as "struct stat64" - its "struct stat" is 64-bit
already.

I'll point out that some of the types in "struct _info" are all wrong for
Cygwin (mode should be mode_t, uid should be uid_t, gid should be gid_t,
and size should be off_t).

> > 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.

Cool.  Let me know when I can download a new version.

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

-- 
				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]