Bug 216404 – [webkitscmpy] Generate Commit object from local repository

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Jonathan Bedard]

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.

[Radar WebKit Bug Importer]

<rdar://problem/68702897>

[Jonathan Bedard]

Created attachment 410161 [details]
Patch

[Jonathan Bedard]

Created attachment 410289 [details]
Patch

[Jonathan Bedard]

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.

[Jonathan Bedard]

This change implements the scheme outlined in https://trac.webkit.org/wiki/commit-identifiers.

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': []}

[Jonathan Bedard]

Created attachment 410579 [details]
Patch

[Jonathan Bedard]

Created attachment 410584 [details]
Patch

[Jonathan Bedard]

Created attachment 410657 [details]
Patch

[Jonathan Bedard]

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

r=me. For the issue that arguments pass to Memoize can not be a list, there will be a separate change to address that.

[Jonathan Bedard]

(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

[EWS]

Committed r268080: <https://trac.webkit.org/changeset/268080>

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