Bug 224519 – [results.webkit.org] Fully report git commits

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

[Jonathan Bedard]

Git commits in a local checkout are fully defined, we shouldn't rely on the results database defining partial commits for us.

[Radar WebKit Bug Importer]

<rdar://problem/76619007>

[Jonathan Bedard]

Created attachment 425935 [details]
Patch

[Aakash Jain]

Comment on attachment 425935 [details]
Patch

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

> Tools/Scripts/webkitpy/port/base.py:1530
> +            # Don't rely on the results database to define git commits

what does this mean?

> Tools/Scripts/webkitpy/port/base.py:-1532
> -            # Special case for WebKit since we have multiple representations at the moment

Are we removing this special case?

[Jonathan Bedard]

Comment on attachment 425935 [details]
Patch

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

>> Tools/Scripts/webkitpy/port/base.py:1530
>> +            # Don't rely on the results database to define git commits
> 
> what does this mean?

if you send the results database a partially-defined commit (ie, only the commit hash and repository_id), the results database will talk to whatever backend it has defined for the specific repository (GitHub + svn.webkit.org, for example) to determine the author/timestamp/identifier/etc. However, if we have a Git checkout, all of this information is accessible locally, so we might as well just report fully defined commits (ie, commits with populated author/timestamp/identifier) in the first place. The reason we didn't do this originally is because getting timestamp and commit message in Subversion is a network call anyways.

>> Tools/Scripts/webkitpy/port/base.py:-1532
>> -            # Special case for WebKit since we have multiple representations at the moment
> 
> Are we removing this special case?

Because the representation of WebKit on the backend now handles this. Previously, our representation of WebKit was purely Subversion, and had no knowledge of what a git commit was, and so didn't know that "master" and "trunk" were the same branch. However, since the new WebKit repository representation is aware of Git, it will correctly map trunk, main and master as default branches.

[Aakash Jain]

(In reply to Jonathan Bedard from comment #4)
> >> Tools/Scripts/webkitpy/port/base.py:1530
> >> +            # Don't rely on the results database to define git commits
> > 
> > what does this mean?
> 
> if you send the results database a partially-defined commit (ie, only the
> commit hash and repository_id), the results database will talk to whatever
> backend it has defined for the specific repository (GitHub + svn.webkit.org,
> for example) to determine the author/timestamp/identifier/etc. However, if
> we have a Git checkout, all of this information is accessible locally, so we
> might as well just report fully defined commits (ie, commits with populated
> author/timestamp/identifier) in the first place. The reason we didn't do
> this originally is because getting timestamp and commit message in
> Subversion is a network call anyways.

ok, the comments needs to be updated to be more clear.


> Because the representation of WebKit on the backend now handles this.
ok

[Jonathan Bedard]

Created attachment 426119 [details]
Patch for landing

[EWS]

Committed r276038 (236582@main): <https://commits.webkit.org/236582@main>

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