Bug 216404

Summary: [webkitscmpy] Generate Commit object from local repository
Product: WebKit Reporter: Jonathan Bedard <jbedard>
Component: Tools / TestsAssignee: Jonathan Bedard <jbedard>
Status: RESOLVED FIXED    
Severity: Normal CC: dewei_zhu, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=215862
https://bugs.webkit.org/show_bug.cgi?id=216403
https://bugs.webkit.org/show_bug.cgi?id=216402
https://bugs.webkit.org/show_bug.cgi?id=217409
https://bugs.webkit.org/show_bug.cgi?id=218079
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Description Jonathan Bedard 2020-09-11 04:36:00 PDT
Add code to generate a commit object from a local Svn or Git repository.

Note that this is the first introduction of a functional commit identifier, which is defined as a per-branch monotonically increasing integer. This identifier is distinct from Svn's revision, since identifiers only increase when a change is made to the specific branch an identifier is tracking, rather than to any branch in the repository.
Comment 1 Radar WebKit Bug Importer 2020-09-11 04:36:56 PDT
<rdar://problem/68702897>
Comment 2 Jonathan Bedard 2020-09-30 15:56:30 PDT
Created attachment 410161 [details]
Patch
Comment 3 Jonathan Bedard 2020-10-01 17:33:49 PDT
Created attachment 410289 [details]
Patch
Comment 4 Jonathan Bedard 2020-10-01 17:38:29 PDT
Comment on attachment 410289 [details]
Patch

This change works, but it doesn't have testing yet. Wanted to put it up for now so it could be discussed.
Comment 5 Jonathan Bedard 2020-10-02 09:44:24 PDT
This change implements the scheme outlined in https://trac.webkit.org/wiki/commit-identifiers.
Comment 6 dewei_zhu 2020-10-05 16:14:41 PDT
Comment on attachment 410289 [details]
Patch

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

> Tools/Scripts/libraries/webkitcorepy/webkitcorepy/decorators.py:47
> +            keyargs = args + tuple(sorted([(key, value) for key, value in kwargs.items()]))
> +            last_called = self._last_called[function].get(keyargs, 0)
> +            is_cached = keyargs in self._cache[function]

This might work if kwargs has list as value.
For example:
kwargs = {'foo': []}
Comment 7 Jonathan Bedard 2020-10-05 16:18:11 PDT
Created attachment 410579 [details]
Patch
Comment 8 Jonathan Bedard 2020-10-05 16:28:37 PDT
Created attachment 410584 [details]
Patch
Comment 9 Jonathan Bedard 2020-10-06 10:40:15 PDT
Created attachment 410657 [details]
Patch
Comment 10 Jonathan Bedard 2020-10-06 11:42:58 PDT
(In reply to dewei_zhu from comment #6)
> Comment on attachment 410289 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=410289&action=review
> 
> > Tools/Scripts/libraries/webkitcorepy/webkitcorepy/decorators.py:47
> > +            keyargs = args + tuple(sorted([(key, value) for key, value in kwargs.items()]))
> > +            last_called = self._last_called[function].get(keyargs, 0)
> > +            is_cached = keyargs in self._cache[function]
> 
> This might work if kwargs has list as value.
> For example:
> kwargs = {'foo': []}

The list case does break things, but I think that's fine because the tuple case works. fine. On a more theoretical level, lists shouldn't really work as arguments to memoized functions anyways because they are mutable, we would basically be relying on Python primitives to enforce that rule.
Comment 11 dewei_zhu 2020-10-06 14:51:59 PDT
r=me. For the issue that arguments pass to Memoize can not be a list, there will be a separate change to address that.
Comment 12 Jonathan Bedard 2020-10-06 15:28:36 PDT
(In reply to dewei_zhu from comment #11)
> r=me. For the issue that arguments pass to Memoize can not be a list, there
> will be a separate change to address that.

https://bugs.webkit.org/show_bug.cgi?id=216404
Comment 13 EWS 2020-10-06 15:47:25 PDT
Committed r268080: <https://trac.webkit.org/changeset/268080>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 410657 [details].