Bug 50715

Summary: webkit-patch should warn users when they're using a 32-bit git on a 64-bit system
Product: WebKit Reporter: Eric Seidel (no email) <eric>
Component: New BugsAssignee: Eric Seidel (no email) <eric>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, commit-queue, eric, kling, mihaip, ojan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Attachments:
Description Flags
Patch
none
Patch
none
move _check_git_architecture to after SCM.__init__ none

Description Eric Seidel (no email) 2010-12-08 14:03:39 PST
webkit-patch should warn users when they're using a 32-bit git on a 64-bit system
Comment 1 Eric Seidel (no email) 2010-12-08 14:17:33 PST
Created attachment 75960 [details]
Patch
Comment 2 Eric Seidel (no email) 2010-12-08 14:22:52 PST
Comment on attachment 75960 [details]
Patch

Hmm... Nevermind, platform.architecture() is not working as expected.  Investigating.
Comment 3 Eric Seidel (no email) 2010-12-08 14:55:23 PST
Created attachment 75974 [details]
Patch
Comment 4 Eric Seidel (no email) 2010-12-08 15:13:06 PST
Created attachment 75977 [details]
move _check_git_architecture to after SCM.__init__
Comment 5 Mihai Parparita 2010-12-08 15:22:46 PST
Comment on attachment 75977 [details]
move _check_git_architecture to after SCM.__init__

View in context: https://bugs.webkit.org/attachment.cgi?id=75977&action=review

> WebKitTools/ChangeLog:19
> +        This message will print twice during 'webkit-patch upload',

You could keep track of whether or not you printed out the message in a static, so that it only shows up once.
Comment 6 Eric Seidel (no email) 2010-12-08 15:46:32 PST
(In reply to comment #5)
> (From update of attachment 75977 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=75977&action=review
> 
> > WebKitTools/ChangeLog:19
> > +        This message will print twice during 'webkit-patch upload',
> 
> You could keep track of whether or not you printed out the message in a static, so that it only shows up once.

webkit-patch calls out to check-webkit-style (a separate process).
Comment 7 Ojan Vafai 2010-12-09 09:53:06 PST
Comment on attachment 75977 [details]
move _check_git_architecture to after SCM.__init__

View in context: https://bugs.webkit.org/attachment.cgi?id=75977&action=review

> WebKitTools/ChangeLog:23
> +        the code is.  I considered writing a test for the log message, but it seemed not worth it.

I think testing _check_git_architecture is worthwhile just to ensure that the code actually runs. Don't even need to verify the output. It helps avoid simple programming errors (e.g. typos).
Comment 8 Eric Seidel (no email) 2010-12-09 10:49:46 PST
(In reply to comment #7)
> (From update of attachment 75977 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=75977&action=review
> 
> > WebKitTools/ChangeLog:23
> > +        the code is.  I considered writing a test for the log message, but it seemed not worth it.
> 
> I think testing _check_git_architecture is worthwhile just to ensure that the code actually runs. Don't even need to verify the output. It helps avoid simple programming errors (e.g. typos).

Possible, however having git installed is not required for WebKit.  So I could check that it throws an OSError or returns a value?
Comment 9 WebKit Commit Bot 2010-12-14 01:26:10 PST
The commit-queue encountered the following flaky tests while processing attachment 75977 [details]:

fast/dom/onerror-img.html bug 51019
fast/preloader/script.html bug 50879 (author: abarth@webkit.org)
The commit-queue is continuing to process your patch.
Comment 10 WebKit Commit Bot 2010-12-14 01:42:48 PST
The commit-queue encountered the following flaky tests while processing attachment 75977 [details]:

http/tests/xmlhttprequest/cross-site-denied-response-sync.html bug 51020 (author: ap@webkit.org)
The commit-queue is continuing to process your patch.
Comment 11 WebKit Commit Bot 2010-12-14 01:44:01 PST
Comment on attachment 75977 [details]
move _check_git_architecture to after SCM.__init__

Clearing flags on attachment: 75977

Committed r74008: <http://trac.webkit.org/changeset/74008>
Comment 12 WebKit Commit Bot 2010-12-14 01:44:08 PST
All reviewed patches have been landed.  Closing bug.