Bug 228027 - [git-webkit] Add identifiers to 'log' and 'blame'
Summary: [git-webkit] Add identifiers to 'log' and 'blame'
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jonathan Bedard
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-07-16 10:19 PDT by Jonathan Bedard
Modified: 2021-07-30 14:59 PDT (History)
6 users (show)

See Also:


Attachments
Patch (19.44 KB, patch)
2021-07-16 14:01 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch (28.72 KB, patch)
2021-07-27 14:18 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch (27.69 KB, patch)
2021-07-29 08:38 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch (27.81 KB, patch)
2021-07-29 11:01 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Bedard 2021-07-16 10:19:42 PDT
We should have a `git-webkit log` and `git-webkit blame` command that use identifiers in those commands and their output.
Comment 1 Radar WebKit Bug Importer 2021-07-16 10:20:32 PDT
<rdar://problem/80691164>
Comment 2 Jonathan Bedard 2021-07-16 14:01:14 PDT
Created attachment 433700 [details]
Patch
Comment 3 Jonathan Bedard 2021-07-27 14:18:42 PDT
Created attachment 434313 [details]
Patch
Comment 4 dewei_zhu 2021-07-28 18:07:31 PDT
Comment on attachment 434313 [details]
Patch

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

> Tools/Scripts/libraries/webkitscmpy/webkitscmpy/program/log.py:70
> +    def main(cls, args, repository, **kwargs):
> +        if not repository.root_path:
> +            sys.stderr.write("Cannot run 'log' on remote repository\n")
> +            return 1
> +
> +        # If we're a terminal, rely on 'more' to display output
> +        if sys.stdin.isatty() and not isinstance(args, list):
> +            environ = os.environ
> +            environ['PYTHONPATH'] = ':'.join(sys.path)
> +
> +            child = subprocess.Popen(
> +                [sys.executable, __file__, repository.root_path, args.representation] + args.args,
> +                env=environ,
> +                stdout=subprocess.PIPE,
> +                stderr=subprocess.PIPE,
> +            )
> +            more = subprocess.Popen([which('more'), '-F'], stdin=child.stdout)
> +
> +            try:
> +                while more.poll() is None and not child.poll():
> +                    time.sleep(0.25)
> +            finally:
> +                child.kill()
> +                more.kill()
> +                child_error = child.stderr.read()
> +                if child_error:
> +                    sys.stderr.buffer.write(b'\n' + child_error)
> +
> +            return child.returncode
> +
> +        return FilteredCommand.main(args, repository, command='log', **kwargs)

This is almost identical to Blame.
The only differences are line 42 and Line 70, which can be changed to
```
sys.stderr.write("Cannot run '{}' on remote repository\n".format(cls.name))

return FilteredCommand.main(args, repository, command=cls.name, **kwargs)
```
Comment 5 Jonathan Bedard 2021-07-29 08:38:31 PDT
Created attachment 434522 [details]
Patch
Comment 6 dewei_zhu 2021-07-29 10:14:57 PDT
Comment on attachment 434522 [details]
Patch

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

r=me

> Tools/Scripts/libraries/webkitscmpy/webkitscmpy/program/blame.py:37
> +        return Blame.pager(args, repository, file=__file__, **kwargs)

Is It intentional to use 'Blame.pager' rather than 'cls.pager' ?

> Tools/Scripts/libraries/webkitscmpy/webkitscmpy/program/log.py:37
> +        return Log.pager(args, repository, file=__file__, **kwargs)

Ditto.
Comment 7 Jonathan Bedard 2021-07-29 11:01:51 PDT
Created attachment 434536 [details]
Patch
Comment 8 EWS 2021-07-29 11:32:31 PDT
Committed r280436 (240074@main): <https://commits.webkit.org/240074@main>

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