Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update SPM and CI setup to fetch the private WordPress-rs repo #23424

Merged
merged 12 commits into from
Jul 17, 2024
Merged

Update SPM and CI setup to fetch the private WordPress-rs repo #23424

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
bf9facd
Switch to SSH to fetch WordPress.rs
mokagio Jul 16, 2024
d6ae5e2
Set up private repo fetching in WP Prototype Build CI step
mokagio Jul 16, 2024
b76d75f
Extract steps to configure Git in CI and use in all builds
mokagio Jul 17, 2024
67585cd
Try to make `gym` use system SCM...
mokagio Jul 17, 2024
5271617
Try to bypass dependencies resolution in `gym` altogether
mokagio Jul 17, 2024
7867dfa
Source set up script because of the `export` call it makes
mokagio Jul 17, 2024
2e4d086
Ensure `set-up-git-for-private-repos.sh` is sourced, not run
mokagio Jul 17, 2024
2f9db71
Revert "Try to bypass dependencies resolution in `gym` altogether"
mokagio Jul 17, 2024
bc94d9f
Revert "Try to make `gym` use system SCM..."
mokagio Jul 17, 2024
52d7fa1
Remove trailing whitespaces from a couple of files (SwiftLint)
mokagio Jul 17, 2024
c370ed9
Update .buildkite/commands/set-up-git-for-private-repos.sh
jkmassel Jul 17, 2024
c895955
Fix comment character
jkmassel Jul 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .buildkite/commands/build-for-testing.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ if [[ "$APP" != "wordpress" && "$APP" != "jetpack" ]]; then
exit 1
fi

# shellcheck disable=SC1091
source "$(dirname "${BASH_SOURCE[0]}")/set-up-git-for-private-repos.sh"

echo "--- :rubygems: Setting up Gems"
install_gems

Expand Down
3 changes: 3 additions & 0 deletions .buildkite/commands/prototype-build-jetpack.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
#!/bin/bash -eu

# shellcheck disable=SC1091
source "$(dirname "${BASH_SOURCE[0]}")/set-up-git-for-private-repos.sh"

# Sentry CLI needs to be up-to-date
brew upgrade sentry-cli

Expand Down
3 changes: 3 additions & 0 deletions .buildkite/commands/prototype-build-wordpress.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
#!/bin/bash -eu

# shellcheck disable=SC1091
source "$(dirname "${BASH_SOURCE[0]}")/set-up-git-for-private-repos.sh"

# Sentry CLI needs to be up-to-date
brew upgrade sentry-cli

Expand Down
3 changes: 3 additions & 0 deletions .buildkite/commands/release-build-jetpack.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
#!/bin/bash -eu

# shellcheck disable=SC1091
source "$(dirname "${BASH_SOURCE[0]}")/set-up-git-for-private-repos.sh"

brew install imagemagick
brew install ghostscript
# Sentry CLI needs to be up-to-date
Expand Down
3 changes: 3 additions & 0 deletions .buildkite/commands/release-build-wordpress.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
#!/bin/bash -eu

# shellcheck disable=SC1091
source "$(dirname "${BASH_SOURCE[0]}")/set-up-git-for-private-repos.sh"

echo "--- :arrow_down: Installing Release Dependencies"
brew install imagemagick
brew install ghostscript
Expand Down
18 changes: 18 additions & 0 deletions .buildkite/commands/set-up-git-for-private-repos.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
#!/bin/bash -eu

if [[ "${BASH_SOURCE[0]}" == "${0}" ]]; then
echo "This script must be sourced, not executed, because it exports GIT_SSH_COMMAND."
exit 1
fi

echo "--- :git: Change Git SSH key to fetch private dependencies"
Copy link
Contributor

Choose a reason for hiding this comment

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

If I'm not mistaken, technically speaking this doesn't change the SSH key (as in: replacing it) but adds it to the agent? (Meaning SSH will still try other keys in the agent if the first one(s) it tries fails?)

Copy link
Contributor

Choose a reason for hiding this comment

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

It changes GIT_SSH_COMMAND so I think it's an accurate comment

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, right 👍

I think I conflated the use of -i alone (which would add the identity but, if used alone, still let ssh try other identities registered in the agent or the config file) with when -i is used in association with -o IdentitiesOnly=yes (like here) to ask the ssh-agent not to offer any keys, and only use the keys provided by -i on the command line, or by the config files.

To be even stricter we should also add -F /dev/null to also tell ssh not to use any keys configured in the config file either, to be sure that the only key it will proposed is the one provided via -i and not try any other key boforehand; but in the case of CI and how Buildkite works wrt to exposing keys to SSH, I don't think any Identity would be configured in the ~/.ssh/config of our agents and risk being used and tried before the bot's private key provided here… so ultimately -F /dev/null would probably unnecessary in this context.


PRIVATE_REPO_FETCH_KEY_NAME="private_repos_key"
# $PRIVATE_REPOS_BOT_KEY is declared in the `env` file of this pipeline in `mobile-secrets`
add_ssh_key_to_agent "$PRIVATE_REPOS_BOT_KEY" "$PRIVATE_REPO_FETCH_KEY_NAME"
jkmassel marked this conversation as resolved.
Show resolved Hide resolved
PRIVATE_REPO_FETCH_KEY="$HOME/.ssh/$PRIVATE_REPO_FETCH_KEY_NAME"

add_host_to_ssh_known_hosts 'github.com'

export GIT_SSH_COMMAND="ssh -i $PRIVATE_REPO_FETCH_KEY -o IdentitiesOnly=yes"
echo "Git SSH command is now $GIT_SSH_COMMAND"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is tidied up version of what Pocket Casts iOS does, too.

See Automattic/a8c-ci-toolkit-buildkite-plugin#102 to track the possibility of DRYing at the CI plugin level.

4 changes: 3 additions & 1 deletion Modules/Package.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,9 @@ let package = Package(
.package(url: "https://github.com/wordpress-mobile/WordPressUI-iOS", branch: "kean-patch-1"),
.package(url: "https://github.com/wordpress-mobile/wpxmlrpc", from: "0.10.0"),
.package(url: "https://github.com/zendesk/support_sdk_ios", from: "8.0.3"),
.package(url: "https://github.com/Automattic/wordpress-rs", from: "0.1.0"),
// This is currently a private repo.
// Fetching it via SSH to avoid HTTPS auth prompts in CI.
.package(url: "[email protected]:Automattic/wordpress-rs", from: "0.1.0"),
],
targets: XcodeSupport.targets + [
.target(name: "JetpackStatsWidgetsCore"),
Expand Down
2 changes: 1 addition & 1 deletion WordPress.xcworkspace/xcshareddata/swiftpm/Package.resolved
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -435,7 +435,7 @@
},
{
"package": "WordPress",
"repositoryURL": "https://github.com/Automattic/wordpress-rs",
"repositoryURL": "git@github.com:Automattic/wordpress-rs",
"state": {
"branch": null,
"revision": "6394b30b34ed6bf204be8de5558ba2c73d2a10e3",
Expand Down
2 changes: 1 addition & 1 deletion WordPress/Classes/Networking/LoginClient.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import WordPressAPI
import AutomatticTracks

actor LoginClient {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

SwiftLint picked this and the ones below in CI, so I addressed them because I like the look of a green build 🤓

struct ApiDetails {
let rootUrl: ParsedUrl
let loginUrl: String?
Expand Down
4 changes: 2 additions & 2 deletions WordPress/Classes/Utility/Analytics/WPAnalyticsEvent.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -682,15 +682,15 @@ import Foundation
case .gutenbergEditorHelpShown: "editor_help_shown"
case .gutenbergEditorBlockInserted: "editor_block_inserted"
case .gutenbergEditorBlockMoved: "editor_block_moved"

// Notifications permissions
case .pushNotificationsPrimerSeen: "notifications_primer_seen"
case .pushNotificationsPrimerAllowTapped: "notifications_primer_allow_tapped"
case .pushNotificationsPrimerNoTapped: "notifications_primer_no_tapped"
case .secondNotificationsAlertSeen: "notifications_second_alert_seen"
case .secondNotificationsAlertAllowTapped: "notifications_second_alert_allow_tapped"
case .secondNotificationsAlertNoTapped: "notifications_second_alert_no_tapped"

// Reader
case .selectInterestsShown: "select_interests_shown"
case .selectInterestsPicked: "select_interests_picked"
Expand Down