Bug 50715 – webkit-patch should warn users when they're using a 32-bit git on a 64-bit system

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Eric Seidel (no email)]

webkit-patch should warn users when they're using a 32-bit git on a 64-bit system

[Eric Seidel (no email)]

Created attachment 75960 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 75960 [details]
Patch

Hmm... Nevermind, platform.architecture() is not working as expected.  Investigating.

[Eric Seidel (no email)]

Created attachment 75974 [details]
Patch

[Eric Seidel (no email)]

Created attachment 75977 [details]
move _check_git_architecture to after SCM.__init__

[Mihai Parparita]

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.

[Eric Seidel (no email)]

(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).

[Ojan Vafai]

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).

[Eric Seidel (no email)]

(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?

[WebKit Commit Bot]

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: [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

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: [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

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>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.