[git-webkit] Improve setup usability by JonWBedard · Pull Request #68 · WebKit/WebKit

JonWBedard · 2022-01-11T00:30:44Z

`d240b4f`

[git-webkit] Improve setup usability
https://bugs.webkit.org/show_bug.cgi?id=235051
<rdar://problem/87367948 >

Reviewed by Aakash Jain.

* Tools/Scripts/libraries/webkitbugspy/setup.py: Bump version.
* Tools/Scripts/libraries/webkitbugspy/webkitbugspy/__init__.py: Ditto.
* Tools/Scripts/libraries/webkitbugspy/webkitbugspy/github.py:
(Tracker.credentials): Retry credential prompt if the user provides an email.
* Tools/Scripts/libraries/webkitcorepy/setup.py: Bump version.
* Tools/Scripts/libraries/webkitcorepy/webkitcorepy/__init__.py: Ditto.
* Tools/Scripts/libraries/webkitcorepy/webkitcorepy/credentials.py:
(credentials): Use Terminal's choosing function.
(delete_credentials): Delete credentials for a service.
* Tools/Scripts/libraries/webkitcorepy/webkitcorepy/editor.py:
(Editor.vi): Add.
(Editor.programs): Add vi.
* Tools/Scripts/libraries/webkitscmpy/setup.py: Bump version.
* Tools/Scripts/libraries/webkitscmpy/webkitscmpy/__init__.py: Ditto.
* Scripts/libraries/webkitscmpy/webkitscmpy/program/setup.py:
(Setup.git): SSH replacement applies to http and https remotes.
(Setup.main): Include explicit conclusion messages.
* Tools/Scripts/libraries/webkitscmpy/webkitscmpy/remote/git_hub.py:
(GitHub.credentials): Retry credential prompt if the user provides an email.
* Tools/Scripts/libraries/webkitscmpy/webkitscmpy/test/setup_unittest.py:


Canonical link: https://commits.webkit.org/246186@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@288227 268f45cc-cd09-0410-ab3c-d52691b4dbfc

aj062

I am just testing github.

aj062 · 2022-01-18T14:14:24Z

how does self.url look like? Might be a good idea to store it in a variable for better readability.

aj062 · 2022-01-18T14:16:50Z

Nit: can reword something like:
Provided username contains an '@' symbol. Please make sure to enter your GitHub username, not the email.

aj062 · 2022-01-18T14:20:37Z

is this new import needed?

This is more accurately an "export." This allows us to do from webkitcorepy import delete_credentials instead of from webkitcorepy.credentials import delete_credentials

aj062 · 2022-01-18T14:26:02Z

Nit: can reword "for your password" = >"for your password every time"

It would be nice to clearly recommend ssh here as some (beginner) users might not know the difference or which is recommended approach.

Maybe:
convert to a ssh remote? => "convert to a ssh remote? (recommended)"

or maybe something like:
"http(s) based remotes will prompt for your password every time when pushing,\nIt is recommended to convert to a ssh remote, would you like to convert to a ssh remote?

aj062 · 2022-01-18T14:38:53Z

Nit: this is bailing out, right? If so, can print a message asking user to re-run this command again later-on.

This probably results in us bailing out, but we don't actually know the command being run. Ideally, it's git-webkit setup, but git-webkit pr also uses this code-path, as does git-webkit find if the repository in question declared that credentials are required (as it might if, for example, the repository was not a public one). It's probably better to have the caller handle this error than to print something out here because the caller will know if the command is safe to re-run

ok, sounds good

aj062 · 2022-01-18T14:42:39Z

same feedback here.
Also, why is this code duplicated? (I do see a FIXME, so that's fine to fix in later PR, just trying to understand the reason if we need it at both places currently)

At the moment, webkitscmpy (library that handles source code management) does not depend on webkitbugspy (library that handles interaction with bug trackers), although it will, and webkitbugspy will never depend on webkitscmpy. Basically, webkitbugspy is a super new library (as in it was committed for the first time in the last week) and before webkitscmpy actually uses it, I would webkitbugspy to have the things that webkitscmpy needs.

aj062 · 2022-01-18T14:44:21Z

is it storing username or password?

This function is all deleting, and this first part is explicitly username.

Some background on this: for bugzilla and SVN, the user's email address was the username. Which made finding credentials really easy because we could query the global git settings to find the username. This doesn't work so well for GitHub because emails and usernames don't necessarily have a relationship. The solution I came up with for this was to store the username for a given account in the keychain keyed by username along with password keyed by the actual username

aj062 · 2022-01-18T14:47:12Z

Looks good on high-level, left few comments though.

aj062 · 2022-01-19T15:47:49Z

ok, sounds good

aj062 · 2022-01-19T15:52:42Z

aj062 · 2022-01-19T15:54:24Z

Instead of deleting from keychain after we detect '@' in username, can we simply skip adding to keychain?

No, because then webkitcorepy would have to know that @ was not allowed in usernames, and the trouble is that's not generally true. Notably, it's not true for bugzilla.

aj062 · 2022-01-19T15:56:43Z

Also, please look at style failures in https://ews-build.webkit-uat.org/#/builders/9/builds/2082

JonWBedard · 2022-01-19T19:53:36Z

Also, please look at style failures in https://ews-build.webkit-uat.org/#/builders/9/builds/2082

I did, and I probably need to make some style-checker modifications here, because it's complaint about import location isn't correct for pip packages, and the trailing whitespace is for a multi-line string.

JonWBedard · 2022-01-19T20:14:16Z

Landed https://commits.webkit.org/246186@main (d240b4f)!

https://bugs.webkit.org/show_bug.cgi?id=258794 Reviewed by Youenn Fablet. The current WebRTC HEVC is using generic packetization instead of RFC 7789 Packetization. So this is about implementing the RFC 7798 Packetization. Fix HEVC depacketizer issues. (WebKit#185) Enalbing low latency mode for RTC (WebKit#169) Enable HEVC support. (WebKit#165) Fix out-of-bounds write in H265VpsSpsPpsTracker (WebKit#163) Apply fix bitstream logic to RtpVideoStreamReceiver2 (WebKit#142) Add missing CODEC_H265 switch case (WebKit#136) Add HEVC support for iOS/Android (WebKit#68) H265 packetization_mode setting fix (WebKit#53) Add H.265 QP parsing logic (WebKit#47) This patch is extracted from following Open WebRTC Toolkit code changes: <open-webrtc-toolkit/owt-deps-webrtc#185> <open-webrtc-toolkit/owt-deps-webrtc#169> <open-webrtc-toolkit/owt-deps-webrtc#165> <open-webrtc-toolkit/owt-deps-webrtc#163> <open-webrtc-toolkit/owt-deps-webrtc#142> <open-webrtc-toolkit/owt-deps-webrtc#136> <open-webrtc-toolkit/owt-deps-webrtc#68> <open-webrtc-toolkit/owt-deps-webrtc#53> <open-webrtc-toolkit/owt-deps-webrtc#47> co-authoured by: taste1981 <[email protected]> jianjunz <[email protected]> Cyril Lashkevich <[email protected]> Piasy <[email protected]> ShiJinCheng <[email protected]> Andreas Unterhuber <[email protected]> dong-heun <[email protected]> * Source/ThirdParty/libwebrtc/Configurations/libwebrtc.xcconfig: * Source/ThirdParty/libwebrtc/Source/webrtc/api/video/video_codec_type.h: * Source/ThirdParty/libwebrtc/Source/webrtc/api/video_codecs/video_codec.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/api/video_codecs/video_codec.h: (webrtc::VideoCodecH265::operator!= const): * Source/ThirdParty/libwebrtc/Source/webrtc/api/video_codecs/video_decoder_software_fallback_wrapper.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/api/video_codecs/video_encoder.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/api/video_codecs/video_encoder.h: * Source/ThirdParty/libwebrtc/Source/webrtc/call/rtp_payload_params.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/common_video/h265/h265_common.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/common_video/h265/h265_common.h: * Source/ThirdParty/libwebrtc/Source/webrtc/common_video/h265/h265_pps_parser.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/common_video/h265/h265_pps_parser.h: * Source/ThirdParty/libwebrtc/Source/webrtc/common_video/h265/h265_sps_parser.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/common_video/h265/h265_sps_parser.h: * Source/ThirdParty/libwebrtc/Source/webrtc/common_video/h265/h265_vps_parser.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/common_video/h265/h265_vps_parser.h: * Source/ThirdParty/libwebrtc/Source/webrtc/logging/rtc_event_log/rtc_event_log_parser.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/media/base/media_constants.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/media/base/media_constants.h: * Source/ThirdParty/libwebrtc/Source/webrtc/media/engine/internal_decoder_factory.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/modules/rtp_rtcp/source/create_video_rtp_depacketizer.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/modules/rtp_rtcp/source/rtp_format.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/modules/rtp_rtcp/source/rtp_format_h265.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/modules/rtp_rtcp/source/rtp_format_h265.h: (webrtc::RtpPacketizerH265::PacketUnit::PacketUnit): (webrtc::VideoRtpDepacketizerH265::~VideoRtpDepacketizerH265): Deleted. * Source/ThirdParty/libwebrtc/Source/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/modules/rtp_rtcp/source/rtp_video_header.h: * Source/ThirdParty/libwebrtc/Source/webrtc/modules/video_coding/codecs/h265/include/h265_globals.h: * Source/ThirdParty/libwebrtc/Source/webrtc/modules/video_coding/encoded_frame.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/modules/video_coding/h265_vps_sps_pps_tracker.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/modules/video_coding/h265_vps_sps_pps_tracker.h: * Source/ThirdParty/libwebrtc/Source/webrtc/modules/video_coding/include/video_codec_interface.h: * Source/ThirdParty/libwebrtc/Source/webrtc/modules/video_coding/packet_buffer.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/rtc_base/experiments/min_video_bitrate_experiment.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/components/video_codec/nalu_rewriter.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/components/video_codec/nalu_rewriter.h: * Source/ThirdParty/libwebrtc/Source/webrtc/test/scenario/video_stream.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/test/video_codec_settings.h: (webrtc::test::CodecSettings): * Source/ThirdParty/libwebrtc/Source/webrtc/video/config/video_encoder_config.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/video/config/video_encoder_config.h: * Source/ThirdParty/libwebrtc/Source/webrtc/video/encoder_overshoot_detector.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/video/rtp_video_stream_receiver2.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/video/rtp_video_stream_receiver2.h: * Source/ThirdParty/libwebrtc/Source/webrtc/video/send_statistics_proxy.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/video/video_stream_encoder.cc: * Source/ThirdParty/libwebrtc/libwebrtc.xcodeproj/project.pbxproj: Canonical link: https://commits.webkit.org/267677@main

@rpath

…de the WebKit bundles (#68) Move the private C++ runtime + CG polyfill from /System/Library/WebKitPrivateRuntime into JavaScriptCore.framework and WebCore.framework (Versions/A/Frameworks), referenced by absolute in-bundle path (not @rpath, to avoid shadowing the system libc++). Deploy after install_framework + the 32-bit graft; remove the legacy runtime dir. i386 slices are stock 10.9 and unaffected. Verified: browse, 32-bit WebView app, QuickLook. Co-Authored-By: Claude Opus 4.8 <[email protected]>

JonWBedard self-assigned this Jan 11, 2022

JonWBedard requested a review from aj062 January 11, 2022 00:31

JonWBedard force-pushed the eng/aakash-setup-feedback branch 2 times, most recently from e4d492f to 6ea0ceb Compare January 11, 2022 00:47

aj062 requested changes Jan 13, 2022

View reviewed changes

JonWBedard force-pushed the eng/aakash-setup-feedback branch from 6ea0ceb to ee7ffa7 Compare January 13, 2022 19:46

JonWBedard requested a review from aj062 January 13, 2022 19:52

aj062 reviewed Jan 18, 2022

View reviewed changes

JonWBedard force-pushed the eng/aakash-setup-feedback branch from ee7ffa7 to 01a4f45 Compare January 18, 2022 17:13

JonWBedard requested a review from aj062 January 18, 2022 17:13

aj062 approved these changes Jan 19, 2022

View reviewed changes

JonWBedard force-pushed the eng/aakash-setup-feedback branch from 01a4f45 to 453d602 Compare January 19, 2022 20:02

JonWBedard merged commit d240b4f into WebKit:main Jan 19, 2022

JonWBedard force-pushed the eng/aakash-setup-feedback branch from 453d602 to d240b4f Compare January 19, 2022 20:14

JonWBedard deleted the eng/aakash-setup-feedback branch January 19, 2022 20:15

basuke mentioned this pull request Dec 4, 2025

Re-entrant navigation commits wrong DocumentLoader #53490

Open

Uh oh!

Conversation

JonWBedard commented Jan 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aj062 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

aj062 commented Jan 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aj062 commented Jan 19, 2022

Uh oh!

JonWBedard commented Jan 19, 2022

Uh oh!

JonWBedard commented Jan 19, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

JonWBedard commented Jan 11, 2022 •

edited

Loading

aj062 commented Jan 18, 2022 •

edited

Loading