Replacing PHP with equivalent Python CGI scripts
<rdar://problem/75183314>
Created attachment 422893 [details] Patch
Created attachment 422895 [details] Patch
Created attachment 422933 [details] Patch
Created attachment 422977 [details] Patch
Created attachment 422981 [details] Patch
Created attachment 423056 [details] Patch
Created attachment 423204 [details] Patch
Comment on attachment 423204 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=423204&action=review Biggest note is that I don't think our Windows bots have f strings available, although we can let EWS check that assumption > LayoutTests/http/tests/cache/post-redirect-get.php:-4 > -// This test loads an uncacheable main resource and a cacheable image subresource. We should keep this comment around > LayoutTests/http/tests/cache/post-redirect-get.py:12 > +sys.path.insert(0, http_root) No reason to do this if we aren't using any shared code > LayoutTests/http/tests/cache/post-redirect-get.py:27 > + f'Expires: {expires}\r\n' f strings are introduced in Python 3.6....I think our Windows bots are using 3.5 > LayoutTests/http/tests/cache/post-redirect-get.py:40 > + '<html>\r\n' \r\n isn't going to be harmful here, but is there a reason we're doing \r\n instead of just \n? > LayoutTests/http/tests/cache/post-with-cached-subresources.py:18 > +sys.path.insert(0, http_root) We don't need this since we aren't importing any shared code > LayoutTests/http/tests/cache/reload-main-resource.py:9 > +sys.path.insert(0, http_root) We don't need this since we aren't importing any shared code > LayoutTests/http/tests/cache/disk-cache/resources/redirect-chain.py:24 > + f'Location: redirect-chain.py?length={length}&uniqueId={unique_id}&random={randint(0, sys.maxsize)}\r\n\r\n' I think we need to avoid f strings because of Windows for the moment > LayoutTests/http/tests/cache/disk-cache/speculative-validation/resources/validation-request-frame.py:12 > + f'Set-Cookie: {cookie}\r\n\r\n' I think we should avoid f string for the sake of our Windows bots > LayoutTests/http/tests/cache/resources/cacheable-iframe.py:10 > + f'Cache-Control: public, max-age={max_age}\r\n' Ditto on the f strings > LayoutTests/http/tests/cache/resources/load-and-check-referer.py:7 > +def contentType(path): Existing function is fine as is, but might consider doing something like this: { 'html': 'text/html', 'manifest': 'text/cache-manifest', ... }.get(path.split('.')[-1], 'text/plain') > LayoutTests/http/tests/cache/resources/partitioned-cache-echo-state.py:9 > +sys.path.insert(0, http_root) We aren't using any shared code, so we don't need to edit the Python path > LayoutTests/http/tests/cache/resources/post-image-to-verify.py:1 > +#!/usr/bin/env python3 Where is the file we're replacing? > LayoutTests/http/tests/cache/resources/reload-main-resource-iframe.py:10 > +sys.path.insert(0, http_root) We aren't using any shared code, so we don't need to edit the Python path > LayoutTests/http/tests/cache/resources/stylesheet304.php:-21 > -?> Doesn't look live we've replaced this file. Is it no longer in use? > LayoutTests/http/tests/resources/touch-temp-file.py:1 > +#!/usr/bin/env python3 I don't see the file this is replacing. > LayoutTests/http/tests/resources/touch-temp-file.py:10 > +sys.path.insert(0, http_root) We aren't using any shared code, so we don't need to edit the Python path > LayoutTests/http/tests/resources/write-temp-file.py:1 > +#!/usr/bin/env python3 I don't see the file we're replacing. > LayoutTests/http/tests/resources/write-temp-file.py:10 > +sys.path.insert(0, http_root) We aren't using any shared code, so we don't need to edit the Python path
Created attachment 423370 [details] Patch
Comment on attachment 423370 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=423370&action=review > LayoutTests/http/tests/cache/post-redirect-get.py:46 > + '<html>\n' Fixed \r\n to \n > LayoutTests/http/tests/cache/resources/load-and-check-referer.py:7 > +def contentType(path): Converted func to your dictionary approach. Got errors when running test including images. Will look closer into changing to a dictionary approach the second pass with this directory with the more challenging conversions. > LayoutTests/http/tests/cache/resources/post-image-to-verify.py:1 > +#!/usr/bin/env python3 Removing PHP equivalent. > LayoutTests/http/tests/cache/resources/stylesheet304.php:-1 > -<?php No need to replace. No longer in use > LayoutTests/http/tests/resources/touch-temp-file.py:1 > +#!/usr/bin/env python3 Still used by multiple .php files. This directory/tests are reference throughout http/tests so this will be the last directory for full conversion. > LayoutTests/http/tests/resources/write-temp-file.py:1 > +#!/usr/bin/env python3 Still used by multiple .php files. This directory/tests are reference throughout http/tests so this will be the last directory for full conversion.
(In reply to Chris Gambrell from comment #11) > Comment on attachment 423370 [details] > Patch > Waiting to convert the f strings back to regular strings with the .format modifier since previous patch passed EWS. If the conversion isn't needed, will leave it with f strings. If conversion is needed, I will push another patch.
(In reply to Chris Gambrell from comment #12) > (In reply to Chris Gambrell from comment #11) > > Comment on attachment 423370 [details] > > Patch > > > > Waiting to convert the f strings back to regular strings with the .format > modifier since previous patch passed EWS. If the conversion isn't needed, > will leave it with f strings. If conversion is needed, I will push another > patch. Looks like I was wrong about the version of Python our Windows bots are running. The are running 3.6, we can use f-strings.
Committed r274574: <https://commits.webkit.org/r274574> All reviewed patches have been landed. Closing bug and clearing flags on attachment 423370 [details].
Reopening to complete conversion of directory
Created attachment 424137 [details] Patch
Created attachment 424139 [details] Patch
Created attachment 424151 [details] Patch
Comment on attachment 424151 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=424151&action=review > LayoutTests/http/tests/cache/resources/cache-control-redirect.py:10 > +code = query.get('code', [None])[-1] Should replace None with 302 and drop the if statement on line 15 > LayoutTests/http/tests/cache/resources/x-frame-options.py:1 > +#!/usr/bin/env python3 I don't see the file we are replacing
Created attachment 424856 [details] Patch
(In reply to Jonathan Bedard from comment #19) > Comment on attachment 424151 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=424151&action=review > > > LayoutTests/http/tests/cache/resources/cache-control-redirect.py:10 > > +code = query.get('code', [None])[-1] > > Should replace None with 302 and drop the if statement on line 15 > > > LayoutTests/http/tests/cache/resources/x-frame-options.py:1 > > +#!/usr/bin/env python3 > > I don't see the file we are replacing Fixed in Comment 20
Created attachment 424949 [details] Patch
Comment on attachment 424949 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=424949&action=review > LayoutTests/ChangeLog:13 > + * http/tests/cache/resources/x-frame-options.py: Added. PHP counterpart not removed because of an issue with it and apply-patch. When r+'d I will remove locally and land.
Comment on attachment 424949 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=424949&action=review > LayoutTests/http/tests/cache/resources/x-frame-options.py:15 > +one_year = 12 * 31 * 24 * 60 * 60 Super weird way to represent a year, we usually use 365 instead of 12 * 31
Committed r275399 (236063@main): <https://commits.webkit.org/236063@main>