Hi Paul, Thanks for the feedback. Please see the notes below.
Paul Cunningham ??: > Liu Xin, > > See below for my comments, note I'm a bit confused at why you are doing > somethings as you are ... > > Paul > > Liu Xin wrote: > >> Please help review the fix of CR6689922 CR6739050 and CR6739053. Any >> comments are welcome. Thanks. >> >> Webrev is at: >> http://cr.opensolaris.org/~liuxin/ocaml_lablgtk_unison/ >> <http://cr.opensolaris.org/%7Eliuxin/ocaml_lablgtk_unison/> >> >> CR synopsis: >> >> 6689922 <http://monaco.sfbay.sun.com/detail.jsp?cr=6689922> unison 2.27.57 >> to be included into SFW consolidation >> 6739050 <http://monaco.sfbay.sun.com/detail.jsp?cr=6739050> ocaml-3.10.2 to >> be included into SFW consolidation >> 6739053 <http://monaco.sfbay.sun.com/detail.jsp?cr=6739053> lablgtk-2.10.1 >> to be included into SFW consolidation >> >> PSARC case: >> http://opensolaris.org/os/community/arc/caselog/2008/212/ >> http://opensolaris.org/os/community/arc/caselog/2008/406/ >> > > === Start of Comments ==== > > 1. usr/src/cmd/Makefile > Shouldn't unison have a build dependency on > the ocaml compiler build? > > For lablgtk see comment (5) > The unison will use the ocaml compliler that installed in the temporary dir by lablgtk. We can not use the one that was built by ocaml/Makefile.sfw, please see comment (3) > 2. usr/src/cmd/unison/Makefile.sfw > Why are you moving stuff about that belongs to > ocaml and lablgtk in this Makefile (lines 48-52), > shouldn't that have been done in the builds of > those packages at the appropriate time? > please see comment (3) first There are two ocaml now.One was built by ocaml/Makefile.sfw to be installed into the proto area, the other installed in the temporary dir by lablgtk/Makefile.sfw for compling. The unison will use the ocaml compliler that installed in the temporary dir by lablgtk/Makefile.sfw.(temporary dir=/proto/root_sparc/usr/share/ocaml/) But the lablgtk will be put under the ocaml dir that was built by ocaml/Makefile.sfw in order to be installed into the right place.(the ocaml dir=/proto/root_sparc/usr) The unison also use the lablgtk ,so copy the lablgtk files to the temporary dir for compling. > 3. usr/src/cmd/ocaml/Makefile.sfw > Why are you 'configure'ing this twice? My guess > is that you somehow want to be able to use the > ocaml you put into the proto area to build > unison - but the proto area should have the real > SUNWocaml files in it. > > Use the default 'configure --prefix=..' value defined > in Makefile.master (and any other defaults). You could > also use the CONFIGURE_OPTIONS method, eg. > './configure $(CONFIGURE_OPTIONS)' see other recent > pkg integrations. > The ocaml built by ocaml/Makefile.sfw will generate several binary files to install. such as ocamlc ocamlcp ocamlopt ocamlrun ...... Most of them need "ocamlrun" to run.They will be put in the proto area as the the real SUNWocaml files the 'configure --prefix=..' dose not only define the PATH where to install them,but also defines where is the "ocamlrun" to run them. For example if use the 'configure --prefix=/usr',then build the source to generate ocamlc ... ======================== #strings ocamlc | more #!/usr/bin/ocamlrun caml_alloc_dummy ...... ======================== if use the 'configure --prefix=/$(ROOT)/usr' ,then build the source to generate ocamlc ... ======================== #echo $ROOT /builds/.../proto/root_sparc # #strings ocamlc | more #!/builds/.../proto/root_sparc/usr/bin/ocamlrun caml_alloc_dummy ...... ======================== So I use the first 'configure --prefix=/usr' to defines where is the "ocamlrun" in the binary files (#!/usr/bin/ocamlrun) and build the source to generate them, then use the second 'configure --prefix=$(ROOT)/usr' to define the PATH in the unison-2.27.57/makefile to install them to the proto area. Since the ocamlc and other binary files with '#!/usr/bin/ocamlrun' in them can not run when they are in the proto area ($ROOT is not "/"), we have to install them again when labgtk and unison need. There are two ocaml now. One was built by ocaml/Makefile.sfw to be installed into the proto area, the other installed in the temporary dir by lablgtk/Makefile.sfw for compling. Unison will use the one built by lablgtk/Makefile.sfw. > 4. usr/src/cmd/ocaml/install-sfw > Add 'line space' after line 45 ('make install' line). > Is this 'make install' actually installing it into the > proto area (eg. no DESTDIR= or equivalent)? > > Yes, the install path will be defined by the "configue" in the Makefile.sfw . 'make install' will use the makefile that is generated by the "configue". > Do all these files need the write permission bit? > Ok , I will check and remove the write permission bit that do not need. > 5. usr/src/cmd/lablgtk/Makefile.sfw > Why are you building ocaml again in this lablgtk > Makefile ? Shouldn't you be using the one that was built > by ocaml/Makefile.sfw ? > > please see comment (3) > 6. all install-* files > Look back in sfwnv-discuss at 'Roland Mainz' comments > on other peoples install-* files and see if these are > appropriate to yours. > > I subscribe the sfwnv-discuss mail list for only a few days. I could not see the 'Roland Mainz' comments. Will you detail it ?or forward the mails to me,thanks. > 7. usr/src/cmd/lablgtk/install-sfw > Is the 'make install' actually installing it into the > proto area (eg. no DESTDIR= or equivalent)? > Yes, the install path will be defined by the "configue" in the Makefile.sfw . 'make install' will use the makefile that is generated by the "configue". > Do all these files need the write permission bit? > > Ok , I will check and remove the write permission bit that do not need. > 8. usr/src/cmd/lablgtk/ocaml-3.10.2.tar.gz > & usr/src/cmd/ocaml/ocaml-3.10.2.tar.gz > Why put this source tarball twice in gate? > > please see comment (3) > 9. usr/src/pkgdefs/SUNWunison/pkginfo.tmpl > Expand the DESC= so that its more descriptive > I agree > 10. usr/src/pkgdefs/SUNWocaml/copyright > Are the authors listed in here really the same as > in SUNWunison/copyright ? > And in SUNWlablgtk/copyright > > It will be update soon > 11. usr/src/pkgdefs/SUNWocaml/pkginfo.tmpl > Expand the DESC= so that its more descriptive > > I agree > 12. usr/src/pkgdefs/SUNWlablgtk/depend > Does it really depend on SUNWocaml, the ocaml compiler? > > yes, there are three binary files which will use ocamlrun to run. > 13. usr/src/pkgdefs/SUNWlablgtk/pkginfo.tmpl > Expand the DESC= so that its more descriptive > I agree > 14. usr/src/pkgdefs/SUNWlablgtk/prototype_com > Should you be delivering the static *.a libraries? > There are also some in SUNWocaml/prototype_com. > Well, I just install the files based on its original makefile.Maybe the Author thought it's necessary. > === End of Comments ====== > > -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://mail.opensolaris.org/pipermail/sfwnv-discuss/attachments/20080908/12126b88/attachment.html>
