Bug 183696 - Use --relative git parameter when applicable to generate WPT patch
Summary: Use --relative git parameter when applicable to generate WPT patch
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-03-16 07:25 PDT by Brendan McLoughlin
Modified: 2018-06-25 08:52 PDT (History)
5 users (show)

See Also:


Attachments
Patch (6.47 KB, patch)
2018-03-16 07:29 PDT, Brendan McLoughlin
no flags Details | Formatted Diff | Diff
Patch (7.30 KB, patch)
2018-03-19 06:07 PDT, Brendan McLoughlin
no flags Details | Formatted Diff | Diff
Patch (7.38 KB, patch)
2018-03-19 06:21 PDT, Brendan McLoughlin
rniwa: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brendan McLoughlin 2018-03-16 07:25:54 PDT
Use --relative git parameter when applicable to generate WPT patch
Comment 1 Brendan McLoughlin 2018-03-16 07:29:25 PDT
Created attachment 335934 [details]
Patch
Comment 2 youenn fablet 2018-03-16 12:30:28 PDT
webkitpy results are failing with the patch.
You can run them using Tools/Scripts/test-webkitpy.
Comment 3 youenn fablet 2018-03-16 12:36:24 PDT
Comment on attachment 335934 [details]
Patch

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

> Tools/Scripts/webkitpy/common/checkout/scm/scm_unittest.py:553
> +        test_file_path = test_dir + '/test_file'

I am not sure about slash, since we might at some point move away from Cygwin.
Given that this is done extensively in this file, this might be ok...

> Tools/Scripts/webkitpy/common/checkout/scm/svn.py:302
> +            patch = patch.replace(relative + '/', '')

In that general context, it is not sure that it will do exactly what we want.
This is fine in the context of the exporter.
I would add a comment here stating that the relative parameter should be chosen carefully or a better svn-specific approach be taken.

> Tools/Scripts/webkitpy/w3c/test_exporter.py:211
> +        patch_data = self._host.scm().create_patch(git_commit, [WEBKIT_WPT_DIR], relative=WEBKIT_WPT_DIR)

You might need to update Tools/Scripts/webkitpy/w3c/test_exporter_unittest.py as well
Comment 4 Brendan McLoughlin 2018-03-19 06:07:52 PDT
Created attachment 336040 [details]
Patch
Comment 5 Brendan McLoughlin 2018-03-19 06:21:39 PDT
Created attachment 336041 [details]
Patch
Comment 6 Ryosuke Niwa 2018-03-19 16:29:22 PDT
Comment on attachment 336041 [details]
Patch

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

r- because SVN implementation is inadequate.

> Tools/Scripts/webkitpy/common/checkout/scm/svn.py:303
> +            relative_path = relative if relative.endswith('/') else relative + '/'
> +            patch = patch.replace(relative_path, '')

This would replace any string which contains this string. That could be problematic inside Source.
Do a regex match against file names protruded in lines 392-395 in ./Tools/Scripts/svn-create-patch

But a better fix is to add an option to svn-create-patch to specify a relative path as you've done to git.