247553 - Solaris patch-checker update needed for new C++ patch revision and Solaris 10

Status: RESOLVED FIXED

(SeaMonkey :: General, defect)

Description

[Iain MacDonnell]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7) Gecko/20040616 Firefox/0.9
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7) Gecko/20040616 Firefox/0.9


Roland,

Can you please update your patch checker to require the -11 rev of patches
111711 and 111712? These will be needed for builds done with Sun Studio 9...



Reproducible: Always
Steps to Reproduce:
1.
2.
3.

Comment 1

[Iain MacDonnell]

Also 108434-17 and 108435-17 for Solaris 8.

Comment 2

[Iain MacDonnell]

Attachment: Proposed patch

Snuck in a cosmetic fix for unrecognised SunOS releases too (otherwise it just
says "Patch checker not supported for SunOS".

Comment 3

[Roland Mainz]

*** Bug 247440 has been marked as a duplicate of this bug. ***

Comment 4

[Roland Mainz]

Comment on attachment 151146 [details] [diff] [review]
Proposed patch

The patch looks OK... could you please add a dummy check for Solaris 10 as
proposed in bug 247440, please ?

Comment 5

[Iain MacDonnell]

Attachment: New proposed fix, with Solaris 10 silencer

Comment 6

[Roland Mainz]

Comment on attachment 151152 [details] [diff] [review]
New proposed fix, with Solaris 10 silencer

r=[e-mail]

Requesting sr= + checkin= from leaf - and we have to checkin this into the 1.7
and aviary(sp?)-branches, too...

Comment 7

[Roland Mainz]

dvseven:
Greg Onufer suggested that the "not supported on"-warning should include the
output of "uname -p", too - his patch looked like this:
-- snip --
Index: moz_patch_checker.dtksh
===================================================================
RCS file: /cvsroot/mozilla/xpfe/bootstrap/init.d/moz_patch_checker.dtksh,v
retrieving revision 1.3
diff -u -3 -p -u -r1.3 moz_patch_checker.dtksh
--- moz_patch_checker.dtksh     24 Mar 2004 16:34:26 -0000      1.3
+++ moz_patch_checker.dtksh     18 Jun 2004 09:36:46 -0000
@@ -97,7 +97,7 @@ case "$(uname -p)" in
              num_required_patches=9
              ;;
          *)
-            echo "# WARNING: Patch checker not supported for $(uname -s)."
+            echo "# WARNING: Patch checker not supported for $(uname 
-sr) on $(uname -p)."
              exit 0
              ;;
        esac
@@ -149,7 +149,7 @@ case "$(uname -p)" in
              ;;

          *)
-            echo "# WARNING: Patch checker not supported for $(uname -s)."
+            echo "# WARNING: Patch checker not supported for $(uname 
-sr) on $(uname -p)."
              exit 0
              ;;
        esac
-- snip --

Can you make an updated patch for that, please ?

Comment 8

[Iain MacDonnell]

Attachment: Proposed patch #3

OK, I didn't see any point in distinguishing between unsupported platforms and
OS releases - they should all be warnings, so I consolidated them into a
function
(which reports both OS release and platform) and use that in all cases...

Comment 9

[Roland Mainz]

Comment on attachment 151167 [details] [diff] [review]
Proposed patch #3

The generic fallback returns |¤xit 1| by default:
-- snip --
   *)
-      echo "# ERROR: Patch checker not supported for $(uname -p)."
-      exit 1
+      unsupported_exit
       ;;
 esac
-- snip --
In theory this code should never be called - but when it is called for some
reason we should return |exit 1| here to make the condition FATAL since
something completely unexpected had happened which requires our attention.
The other changes and the new function are OK but please restore the |¤xit 1|
for the last case (unsupported OS version is OK and should return |exit 0|).

Comment 10

[Iain MacDonnell]

(In reply to comment #9)
> (From update of attachment 151167 [details] [diff] [review])
> The generic fallback returns |¤xit 1| by default:
> -- snip --
>    *)
> -      echo "# ERROR: Patch checker not supported for $(uname -p)."
> -      exit 1
> +      unsupported_exit
>        ;;
>  esac
> -- snip --
> In theory this code should never be called - but when it is called for some
> reason we should return |exit 1| here to make the condition FATAL since
> something completely unexpected had happened which requires our attention.
> The other changes and the new function are OK but please restore the |¤xit 1|
> for the last case (unsupported OS version is OK and should return |exit 0|).


I disagree... it could be just a platform that you haven't seen before
(perhaps "x86_64" or something, when Solaris 10 supports that). It get really
ticked off when wrappers like this error out just because it's something
they haven't seen before - warnings are good, but denying me the ability to
take the risk is not.

Comment 11

[Dan Price]

FWIW, the architect for the Opteron port tells me that 'uname -p' will be 'i386'
on Solaris Opteron when it ships.

Comment 12

[Roland Mainz]

Dan Price wrote:
> FWIW, the architect for the Opteron port tells me that 'uname -p' will be 
> 'i386' on Solaris Opteron when it ships.

Erm... does that mean that Solaris x86_64 will be a mixed 32bit/64bit
environment like Solaris>=2.7/SPARC ?

Comment 13

[Roland Mainz]

Comment on attachment 151167 [details] [diff] [review]
Proposed patch #3

dvseven wrote:
> It get really
> ticked off when wrappers like this error out just because it's something
> they haven't seen before - warnings are good, but denying me the ability to
> take the risk is not.

Yeah... but my fear is that the patch checker won't get updated then... most
people seem to totally ignore the console output... ;-(

... anyway... you convinced me... :)

r=[e-mail]

Comment 14

[Dan Price]

(In reply to comment #12)
> Dan Price wrote:
> > FWIW, the architect for the Opteron port tells me that 'uname -p' will be 
> > 'i386' on Solaris Opteron when it ships.
> 
> Erm... does that mean that Solaris x86_64 will be a mixed 32bit/64bit
> environment like Solaris>=2.7/SPARC ?

Maybe we can talk about it offline :)  I don't really know what we have and have
not announced.

Comment 15

[Roland Mainz]

timeless checked-in in the patch into "trunk"
(http://tinderbox.mozilla.org/bonsai/cvsquery.cgi?module=MozillaTinderboxAll&branch=HEAD&cvsroot=/cvsroot&date=explicit&mindate=1090126440&maxdate=1090127040&who=timeless%25mozdev.org)
...

... remaining work is to get it into the "aviary" and "1.7.2"-branches...

Comment 16

[Mike Kaply [:mkaply]]

Comment on attachment 151167 [details] [diff] [review]
Proposed patch #3

a=mkaply

Comment 17

[Leon Sha]

Why this patch is not in mozilla 1.7 branch.
Can I check it in since it has already got approval?
Also aviary need this patch too.

Comment 18

[Mick Kelleher]

The proposed patch has still not being checked into either of the two branches!.

We probably need it updated again to cover Solaris 11 (should also work for OpenSolaris), Sun are releasing test builds of Solaris based on the Solaris 11 codebase.

Comment 19

[Andrew Sutherland [:asuth] (he/him)]

moz_patch_checker.dtksh has been removed on the trunk by bug 380786.  Marking as FIXED since this was fixed on the trunk and the 1.7 branch is no longer supported.  Feel free to re-open this bug if you care very deeply about seeing the patch land on the 1.7 branch and are willing to see it through.