Hello, I have made the changes to the patch as suggested by Jan.
http://cr.opensolaris.org/~herch/rev02/webrev/ The patch is tested as follows, Test -1 1. apply the patch 2. rebuild the AI image 3. keep the AI manifest to default value - babel_install Result - No issues. Test-2 1. same as 1 and 2 above 3. Modify AI manifest so that only minimal required packages are included, remove babel_install Result - No issues. Please review the patch and let me know what could be the next step for me to get this thing in caiman's next release. Harshal On Fri, Sep 4, 2009 at 4:30 PM, Jan Damborsky <Jan.Damborsky at sun.com> wrote: > Hi Harshal, > > > Harshal wrote: > >> Hi, >> >> Please review the webrev for the patch which is suppose to fix bug 5558 ( >> http://defect.opensolaris.org/bz/show_bug.cgi?id=5558). The webrev >> location is, >> http://cr.opensolaris.org/~herch/rev01/webrev/<http://cr.opensolaris.org/%7Eherch/rev01/webrev/>< >> http://cr.opensolaris.org/%7Eherch/rev01/webrev/> >> > > The fix looks good, I have just couple of minor comments: > > * I think we should record the case when ICT is skipped > * I might recommend to slightly simplify the fix for > better readability > > the code might be changed in following way: > > 1447 browserInstallstatus = > os.path.exists('/usr/lib/firefox/browserconfig.properties') > 1448 if browserInstallstatus == True: > > [...] > > 1478 else: > 1479 return 0 > > -> > > if not os.path.exists('/usr/lib/firefox/browserconfig.properties'): > info_msg('Skipping fix_browser_home_page() ICT task as > /usr/lib/firefox/browserconfig.properties doesn't exist') > return 0 > > [...] > > Also please use bug number and synopsis as commit message - > e.g. in your case > > $ hg commit -m "5558 RFE: Install-finish ICT_FIX_BROWSER_HOME_PAGE_FAILED > must be soft" > > If the changes are already committed in your local workspace, you could > > $ hg recommit -m "5558 RFE: Install-finish ICT_FIX_BROWSER_HOME_PAGE_FAILED > must be soft" > > > >> >> We were in looking to install the just bare minimal installation of >> opensolaris so that we can decrease the provisioning time for new systems >> where don't really require firefox or other bells and whistles of a Desktop >> Environment. Caiman installer by default looks for the presence of the >> firefox before it completes the installation. Because of this we were unable >> to drop firefox from our AI manifest which also meant bringing in lot of GUI >> libs which we will never use. >> >> So in order to address the issue, I wrote a patch which will let the >> installation go ahead and succeed even if firefox is not present. However if >> the firefox is present it will do what it used to (set the homepage). >> >> To test this patch, I followed steps, >> >> Test -1 >> 1. apply the patch >> 2. rebuild the AI image >> 3. keep the AI manifest to default value - babel_install >> Result - No issues. >> >> Test-2 >> 1. same as 1 and 2 above >> 3. Modify AI manifest so that only minimal required packages are included, >> remove babel_install >> Result - No issues. >> >> >> Please let me know what could be the next step to get this thing into main >> caiman release. >> > > If other agrees, I might recommend following procedure (assuming that you > already > went through the required paper work - you filed Sun Contributor > Agreement). > > [1] Process all code review comments and do the suggested changes > > [2] Retest using the same test procedures > > [3] Resend the updated webrev for another round of code review - make sure > that 'hg pbchk' is fine with modifications. > > [4] As soon as people who review the changes are fine with the final state, > I could take the modifications from you and integrate > - you would send me file created by means of 'hg bundle' as Dave > suggested > - I would 'hg unbundle' and push the change into our gate > > [5] I would change state of bug 5558 to RESOLVED-FIXINSOURCE and update it > with > the information about changeset which delivered the fix. > > Jan > > -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://mail.opensolaris.org/pipermail/caiman-discuss/attachments/20090917/bfebbefc/attachment.html>
