build: update squirrel.mac to 8d808803, drop upstreamed patches#51584
Conversation
Bumps squirrel.mac from 0e5d146ba1 to 8d808803bc and removes 14 patches that have been upstreamed into Squirrel/Squirrel.Mac (mainly via Squirrel/Squirrel.Mac#312, plus Squirrel/Squirrel.Mac#298, Squirrel/Squirrel.Mac#302, Squirrel/Squirrel.Mac#308). Only build_add_gn_config.patch remains, slimmed down to GN-only changes since Squirrel/Squirrel.Mac#298 upstreamed the ReactiveCocoa -> ReactiveObjC import renames it was carrying.
VerteDinde
left a comment
There was a problem hiding this comment.
Thanks for doing this, Sam, glad we're just aligned with upstream and not carrying patches! 🙇♀️
|
Thanks for upstreaming all the patches, Sam! I'm a bit concerned that we don't have proper code review for changes to our updater now though. For example, this PR pulls in various upstream changes beyond the upstreamed patches. Those changes are probably useful, but they didn't go through code review. The updater is a pretty critical path in Electron. Obviously you could argue that this version bump PR was reviewed and approved, but I'm worried that "bump Squirrel.Mac to <commit hash>" PRs won't get the in-depth review that changes to our auto updater would have previously gotten. Would appreciate any thoughts on how to improve this. |
There was a problem hiding this comment.
LGTM. @MarshallOfSound would we want to backport Squirrel/Squirrel.Mac#314 to older Electron branches since we are not backporting this PR?
|
Release Notes Persisted
|
|
For posterity, I updated the PR description here as a bunch of the links in the last bullet point in "Notable upstream changes pulled in by the bump" were incorrectly autolinking to issue numbers on this repo, not |
@MarshallOfSound Thanks! In concrete terms, who will review the PRs? |
* fix: don't return a `nullptr` from `TargetForRect` Co-authored-by: Noah Gregory <nmggithub@electronjs.org> * fix: address concerns Co-authored-by: Noah Gregory <nmggithub@electronjs.org> * chore: use promise's context for memory dump callback. (#51570) Co-authored-by: BILL SHEN <15865969+cucbin@users.noreply.github.com> * build: only fallback to CHROMIUM_BUILDTOOLS_PATH if needed (#51558) * build: only fallback to CHROMIUM_BUILDTOOLS_PATH if needed * ci: fix lint workflow detection of src/buildtools * ci: bump build-tools SHA Co-authored-by: David Sanders <dsanders11@ucsbalum.com> * build: update squirrel.mac to 8d808803, drop upstreamed patches (#51584) Bumps squirrel.mac from 0e5d146ba1 to 8d808803bc and removes 14 patches that have been upstreamed into Squirrel/Squirrel.Mac (mainly via Squirrel/Squirrel.Mac#312, plus Squirrel/Squirrel.Mac#298, Squirrel/Squirrel.Mac#302, Squirrel/Squirrel.Mac#308). Only build_add_gn_config.patch remains, slimmed down to GN-only changes since Squirrel/Squirrel.Mac#298 upstreamed the ReactiveCocoa -> ReactiveObjC import renames it was carrying. Co-authored-by: Samuel Attard <sam@electronjs.org> * Revert "build: update squirrel.mac to 8d808803, drop upstreamed patches (#51584)" This reverts commit d4e9004. * Revert "build: only fallback to CHROMIUM_BUILDTOOLS_PATH if needed (#51558)" This reverts commit 090b568. * Revert "chore: use promise's context for memory dump callback. (#51570)" This reverts commit a74d8a1. --------- Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com> Co-authored-by: Noah Gregory <nmggithub@electronjs.org> Co-authored-by: BILL SHEN <15865969+cucbin@users.noreply.github.com> Co-authored-by: David Sanders <dsanders11@ucsbalum.com> Co-authored-by: Samuel Attard <sam@electronjs.org> Co-authored-by: Charles Kerr <charles@charleskerr.com>
Description
Bumps
squirrel.macfrom0e5d146ba1to8d808803bc(latestmain)and removes 14 of our 15 patches — all of which have been upstreamed into
Squirrel/Squirrel.Mac.
Patches removed
feat_add_new_squirrel_mac_bundle_installation_method_behind_flagfix_abort_installation_attempt_at_the_final_mile_if_the_app_isfix_resolve_target_bundle_path_once_at_start_of_installfeat_add_ability_to_prevent_version_downgradesrefactor_use_posix_spawn_instead_of_nstask_so_we_can_disclaim_thefix_trigger_shipit_mach_service_after_smjobsubmit_to_unblockchore_turn_off_launchapplicationaturl_deprecation_errors_in_squirrelrefactor_use_non-deprecated_nskeyedarchiver_apisfix_clean_up_orphaned_staged_updates_before_downloading_new_updatefix_add_explicit_json_property_mappings_for_shipit_request_modelfix_use_kseccschecknestedcode_kseccsstrictvalidate_in_the_secfix_crash_when_process_to_extract_zip_cannot_be_launchedfix_ensure_that_self_is_retained_until_the_racsignal_is_completeuse_uttype_class_instead_of_deprecated_uttypeconformstoThe
use_uttype_class_instead_of_deprecated_uttypeconformstopatch isdropped without an upstream equivalent. It was added in the Chromium 139
bump (#47348) when
mac_deployment_targetmoved to 12.0 andsquirrel_frameworkdid not yet carry-Wno-deprecated-declarations.That flag was added later in #38437, making the patch a no-op — verified
by building
squirrel_frameworkagainst the new pin (which still usesUTTypeConformsTo).Patch kept
build_add_gn_config.patch— slimmed down. Squirrel/Squirrel.Mac#298upstreamed the
ReactiveCocoa→ReactiveObjCimport renames the patchwas carrying, so it now only adds
BUILD.gn,filenames.gni,build/xcrun.gni,build/xcrun.py, and the.gitignoretweaks for theDEPS-managed
vendor/checkouts. It no longer touches any.m/.hsource files.
Notable upstream changes pulled in by the bump
isVersionStandardsupersetcheck so numeric versions pass (the
ElectronSquirrelPreventDowngradesformat gate was rejecting normal versions like
1.2.3)shipItLaunchererrorsacross update checks
requestForDownloadin-initWithUpdateRequest:requestForDownload:low-risk correctness fixes
is running again on GitHub Actions across the macOS / Xcode matrix
Checklist
script/lint.js --patchespassesgn gensucceedssquirrel_framework,squirrel_shipit,stripped_squirrel_frameworkbuild cleanly
Notes: Updated the Squirrel.Mac autoUpdater backend on macOS to pick up upstream fixes, including correct version-string validation when downgrade prevention is enabled, fixes for stale launcher errors persisting across update checks, and forwarding of custom download request headers.