Skip to content

Build scripts should default to the Internal workspace using the internal SDK#26123

Merged
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
kmiller68:eng/Build-scripts-should-default-to-the-Internal-workspace-using-the-internal-SDK
Mar 19, 2024
Merged

Build scripts should default to the Internal workspace using the internal SDK#26123
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
kmiller68:eng/Build-scripts-should-default-to-the-Internal-workspace-using-the-internal-SDK

Conversation

@kmiller68
Copy link
Contributor

@kmiller68 kmiller68 commented Mar 19, 2024

29536a5

Build scripts should default to the Internal workspace using the internal SDK
https://bugs.webkit.org/show_bug.cgi?id=271245
rdar://125015308

Reviewed by Elliott Williams.

Right now in order to get WebKitAdditions when using these scripts you have to already have started a build
from Internal. This is annoying for clean builds or when WebKitBuild has a stale WebKitAdditions. This patch
changes the build scripts to default to the Internal workspace when building for a .internal SDK and the
regular workspace when building for a public SDK. If there's already a configured workspace then we just
use that.

* Tools/Scripts/webkitdirs.pm:
(determineConfiguredXcodeWorkspaceOrDefault):
(configuredXcodeWorkspace):
(XcodeOptions):
(determineConfiguredXcodeWorkspace): Deleted.
* Tools/Scripts/webkitperl/BuildSubproject.pm:
(buildUpToProject):

Canonical link: https://commits.webkit.org/276366@main

1c7399b

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-skia
🛠 🧪 jsc ✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
🛠 🧪 jsc-arm64 ✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 tv-sim ✅ 🧪 api-gtk
✅ 🛠 🧪 merge ✅ 🛠 watch ✅ 🛠 jsc-armv7
✅ 🛠 watch-sim ✅ 🧪 jsc-armv7-tests

@kmiller68 kmiller68 requested a review from JonWBedard as a code owner March 19, 2024 16:05
@kmiller68 kmiller68 self-assigned this Mar 19, 2024
@kmiller68 kmiller68 added the Tools / Tests Tools in the Tools directory, build issues, test infrastructure, and bugs in test cases label Mar 19, 2024
@kmiller68 kmiller68 force-pushed the eng/Build-scripts-should-default-to-the-Internal-workspace-using-the-internal-SDK branch from b846ac7 to 1c7399b Compare March 19, 2024 16:07
@kmiller68
Copy link
Contributor Author

I tested build-jsc/build-webgpu/build-webkit and they all seem to work for clean internal builds with this patch and they pick the existing workspace when set via set-webkit-configuration --workspace=X

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing to note is that this won't abort make unless we also change this logic to propagate the exit code from shell backticks:

WebKit/Makefile.shared

Lines 172 to 173 in 3d472e9

eval build_webkit_options=(`perl -I$(SCRIPTS_PATH) -Mwebkitdirs -e 'print XcodeOptionString()' -- $(BUILD_WEBKIT_BASE) $(BUILD_WEBKIT_OPTIONS)`); \
xcodebuild $1 $(OTHER_OPTIONS) $(XCODE_TARGET) "$${build_webkit_options[@]}" $(XCODE_OPTIONS) $2 | $(OUTPUT_FILTER) && exit $${PIPESTATUS[0]} \

I don't think you need to do that now—if webkitdirs does die here, the error will be printed up front and the Xcode build should fail immediately, too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, interesting that make doesn't propagate non-zero exit status from sub-commands. As you said it's probably not a big deal.

@kmiller68 kmiller68 added the safe-merge-queue Applied to automatically send a pull-request to merge-queue after passing EWS checks label Mar 19, 2024
@webkit-ews-buildbot webkit-ews-buildbot added merge-queue Applied to send a pull request to merge-queue and removed safe-merge-queue Applied to automatically send a pull-request to merge-queue after passing EWS checks labels Mar 19, 2024
@webkit-ews-buildbot
Copy link
Collaborator

Safe-Merge-Queue: Build #15423.

…rnal SDK

https://bugs.webkit.org/show_bug.cgi?id=271245
rdar://125015308

Reviewed by Elliott Williams.

Right now in order to get WebKitAdditions when using these scripts you have to already have started a build
from Internal. This is annoying for clean builds or when WebKitBuild has a stale WebKitAdditions. This patch
changes the build scripts to default to the Internal workspace when building for a .internal SDK and the
regular workspace when building for a public SDK. If there's already a configured workspace then we just
use that.

* Tools/Scripts/webkitdirs.pm:
(determineConfiguredXcodeWorkspaceOrDefault):
(configuredXcodeWorkspace):
(XcodeOptions):
(determineConfiguredXcodeWorkspace): Deleted.
* Tools/Scripts/webkitperl/BuildSubproject.pm:
(buildUpToProject):

Canonical link: https://commits.webkit.org/276366@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/Build-scripts-should-default-to-the-Internal-workspace-using-the-internal-SDK branch from 1c7399b to 29536a5 Compare March 19, 2024 21:06
@webkit-commit-queue
Copy link
Collaborator

Committed 276366@main (29536a5): https://commits.webkit.org/276366@main

Reviewed commits have been landed. Closing PR #26123 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit 29536a5 into WebKit:main Mar 19, 2024
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Mar 19, 2024
@kmiller68 kmiller68 deleted the eng/Build-scripts-should-default-to-the-Internal-workspace-using-the-internal-SDK branch October 17, 2024 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Tools / Tests Tools in the Tools directory, build issues, test infrastructure, and bugs in test cases

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants