I figured that I might not get a chance to look at this later this
weekend and
decided to pass on a few hand of poker so I took a look at your webrev. I
didn't go through this in the detail I might have liked, but it's
getting late.
Here are some comments.
-Norm
usr/src/lib/ilmbase/Makefile.sfw
Line 34:
"all" target should be building the acutal bits and perhaps
split into
32/64 bit targets.
(cd $(VERS) ; $(CCSMAKE))
(cd $(VERS)-64 ; $(CCSMAKE))
Line 41:
If you plan on using the component's install target, you might use
INSTALL="$(INSTALL_PROTO)", which guarantee files install in the
proto area and will post process them as they are
installed. On the
otherhand, if you use an install-xxx script, it doesn't matter.
Line 42:
You should quote the values in setting the environment variables
CC="$(CC)" CXX="$(CCC)"
Lines 43:
You shouldn't need to specify these libraries, they should be in the
default set when using CC to link C++ programs and libraries
Line 44:
Presumably you will want to set --prefix=$(CFGREFIX) and
--mandir=$(CFGMAN)
Lines 45:
You don't need to "cd .." in your subshell, leaving the subshell
() puts
you right back where you started. Also, this doens't belong
here. It
belongs as part of the "all" target.
Line 46:
You should use $(SH), $(KSH93) or whatever shell this was
written for,
but not $(SHELL). Also this doesn't belong here. It belongs as
part
the "all" target
Line 48: see line 41
Line 49: see line 42, but use the 64-bit variants.
CC="$(CC64)" CXX="$(CCC64)"
Line 50: use the predefined macros from Makefile.master, ...
CXXFLAGS="$(CCFLAGS64)"
Lines 51-54: see lines 43-46
Lines 56-76:
You shouldn't need any of these patches if you define
CXXLINK="$(CCC) -norunpath"
in the calling environment to configure.
CXXLINK="$(CCC64) -norunpath"
for 64-bit
usr/src/lib/ilmbase/install-ilmbase
usr/src/lib/ilmbase/install-ilmbase-64
pulling bits out of .libs directories can be dangerous because
libtool has
a final relink that it does prior to installation. You might seriously
consider just using the (cd $(VER) ; make install) approach to
installation
instead of ($(SH) ./install-XXX).
For the 64 bit support you may have to do something like
(cd $(VER)-64 ; make DESTDIR=$(SRC)/.../$(VER)-64/proto install ; \
$(SH) ../install-XXX-64)
to get the bits linked properly and pick out what you want to put in the
proto area.
I notice that you are installing writable files in /usr, don't
There is no
reason for them to be 644 or 755 when 444 and 555 is sufficient.
If you "make install", you will want to use proto-fix to make sure
that the
permissions match the packaging in your makefile.sfw
$(SRC)/tools/protofix --pkg $$pkg --perm
usr/src/lib/ilmbase/ltmain.sh
Why the new ltmain?
usr/src/lib/ilmbase/patch/Makefile.in.Half.patch
usr/src/lib/ilmbase/patch/Makefile.in.Half64.patch
usr/src/lib/ilmbase/patch/Makefile.in.Iex.patch
usr/src/lib/ilmbase/patch/Makefile.in.Iex64.patch
usr/src/lib/ilmbase/patch/Makefile.in.IlmThread.patch
usr/src/lib/ilmbase/patch/Makefile.in.IlmThread64.patch
usr/src/lib/ilmbase/patch/Makefile.in.Imath.patch
usr/src/lib/ilmbase/patch/Makefile.in.Imath64.patch
You shouldn't need any of these patches.
usr/src/lib/openexr/Makefile.sfw
I have similar comments to the ilmbase Makefile.sfw and
at lines 51 and 57, --prefix=$(CFGPREFIX), not =$(ROOT)/usr
usr/src/lib/openexr/install-openexr
usr/src/lib/openexr/install-openexr-64
see corresponding ilmbase comment
usr/src/lib/openexr/ltmain.sh.new
see corresponting ilmbase comment
usr/src/lib/openexr/patch/Makefile.in.IlmImf.patch
usr/src/lib/openexr/patch/Makefile.in.IlmImf64.patch
usr/src/lib/openexr/patch/Makefile.in.exrenvmap.patch
usr/src/lib/openexr/patch/Makefile.in.exrheader.patch
usr/src/lib/openexr/patch/Makefile.in.exrmakepreview.patch
usr/src/lib/openexr/patch/Makefile.in.exrmaketiled.patch
usr/src/lib/openexr/patch/Makefile.in.exrstdattr.patch
You shouldn't need these patches.
usr/src/pkgdefs/SUNW*/prototype_*
Lots of stuff that doesn't need to be writable.
Norm Jacobs wrote:
> I haven't really had a chance to review this yet, but your comment below
> concerns me. You say that you are patching open source component
> makefiles in order to fix the RUNPATH. That *may* be necessary, but
> assuming that the RUNPATH issue is due to linking C++ with the Sun C++
> compiler and it placing it's own favorite directories into the RUNPATH,
> you can turn this off with "-norunpath" and not have to patch the
> makefiles. Just add "CCC += -norunpath" at the appropriate place after
> you include "Makefile.lib" in your Makefile.sfw. You should also quote
> your use of various variables being passed into the calling environment
> for things like configure, "make all", ... Ex:
> cd $(VER) ; env CXX="$(CCC)" CXXFLAGS="$(CCFLAGS)" ... ./configure
> or
> cd $(VER)-64 ; env CXX="$(CCC64)" CXXFLAGS="$(CCFLAGS64)" ...
> ./configure
>
> At any rate, I will try to take a closer look at your workspace over the
> weekend.
>
> -Norm
>
>
> Sheng-qi Jiang wrote:
>
>> Jim Walker wrote:
>>
>>
>>> Thanks Henry,
>>>
>>> I have no further comments.
>>>
>>>
>> Please help review the Makefile.sfw and patch directory.
>> ( these patch files was to remove the unused RPATH of delivery binaries.
>>
>> http://cr.opensolaris.org/~henryj/openexr/
>>
>>
>> Thanks!
>>
>>
>>
>
> _______________________________________________
> sfwnv-discuss mailing list
> sfwnv-discuss at opensolaris.org
> http://mail.opensolaris.org/mailman/listinfo/sfwnv-discuss
>
