Hi Paul, Thanks for your review. I have made changes as below:
http://cr.opensolaris.org/~rszheng/jedit/ > > 1. usr/src/cmd/jedit/Makefile.sfw > > Line ... > 59 -rm -rf $(JUNIT) > is JUNIT correct ? > It should be JEDIT_SRC_DIR, "clean" section now are: 59 clean: 60 -rm -rf $(JEDIT_SRC_DIR); 61 -rm -rf $(JEDIT_NOT_USE); 62 -rm -f jedit JEDIT_NOT_USE=build-support is contained in source package but not used to build jedit. So it should be removed also when cleaning. >> I also added two new variables PROG_NAME and PROG_VER to hold jedit >> name and version, which will be used in install.sfw. > You might want to explicitly pass those into install.sfw > so that it is clearer that is what is happening. I modified it like: 56 install: all 57 PROG_NAME=$(PROG_NAME) PROG_VER=$(PROG_VER) JEDIT_SRC_DIR=$(JEDIT_SRC_DIR) $(SHELL) ./install-sfw > > 2. usr/src/cmd/jedit/install-sfw > Line ... > 69 cd ${TOP}/jEdit/build > you could use the following instead I think .. > cd ${TOP}/${JEDIT_SRC_DIR}/build Done > > Line .. > 92 _install N ../../jedit ${BINDIR}/jedit 555 > could this be a symbolic link instead to the file > installed in .. > 91 _install N ../../jedit ${JEDIT_ROOT}/jedit 555 > > Also, is jedit a binary or a script, if so you should > probably use either '_install E ' or '_install S ' Now them are: 89 #install binary 90 _install N jedit.jar ${JEDIT_ROOT}/jedit.jar 444 91 _install S ../../jedit ${JEDIT_ROOT}/jedit 555 92 _install L ${JEDIT_ROOT}/jedit ${BINDIR}/jedit 555 jedit is a executable script to be installed in /usr/share/jedit/4.3pre16/ and linked as /usr/bin/jedit > 3. usr/src/cmd/jedit/build.xml.patch > Observation: then file that this is patching has .. > debug="true" > switched on, do you really want that ? I have switch debug "off" > > 4. usr/src/cmd/jedit/jedit.patch > The 'read permissions' are no set correctly on this > file, I can not see it. Add "r" to its access mode. jedit.patch is additional script patch to run jedit.jar, referring to item 2. > > 5. usr/src/pkgdefs/Makefile > & usr/src/cmd/Makefile > & usr/src/Targetdirs > These files need resyncing with the gate so it does look > as though you are trying to change other stuff Done > > 6. dependencies > You install a jar file so doesn't it have a dependency on > the java runtime stuff ? >> Though the jre is needed to run jedit ( but for solaris/opensolaris, >> the jre is always installed with the fresh system) > but if i were to remove jre then jedit wouldn't work, so without > the dependency there is nothing to stop me removing jre without > a warning! > Add depend file and removed 30 DATAFILES= depend from usr/src/pkgdefs/SUNWjedit/Makefile
