automattic / a8c-ci-toolkit-buildkite-plugin Goto Github PK

A utility like validate_xcode_project_consistency or whatever could be useful to ensure that some common misconfigurations we encountered in the past wouldn't get us rejected during TestFlight upload.

Why?

In particular, tests around the following would be useful, because such situations won't prevent Xcode from building and won't appear as errors during Xcode compilation and in PRs, and will thus only be noticed at the time we try our first upload to TestFlight after such misconfiguration would be introduced.
And when those TestFlight validation errors happen it's sometimes tricky to debug what the root cause is (because the iTMSTransporter / altool errors are not always clear and sometimes even misleading), while those usually happen when we're in a rush to release a new version and thus need to be unblocked quickly; so better detect those ahead of time instead.

What?

• Ensure that all the .app and .appex targets all use the same CFBundleVersion and CFBundleShortVersionString values (see example TestFlight upload/validation rejection here)
• Ensure that none of the app icons (*.xcassets/*.appiconset/*.png) have an alpha channel (see this Tumblr iOS script for inspiration
• Ensure that the final generated .app bundle doesn't contain nested frameworks, to avoid the issues described in paNNhX-ee-p2. We could use some inspiration from this script, or simply just use find {path-to-final-app} -path '*/Frameworks/**/Frameworks/*' to detect those in the final .app, and print some guidance in the error message to suggest to use "Do Not Embed" setting on the dependent frameworks of the offending target(s).

Alternatives Considered

This sort of check could alternatively be better suited to be implemented by a fastlane action? (either in our release-toolkit, or even better, by contributing to fastlane's core repo to add such checks… either as a separate action, or as a final set of checks that would be run at the end of the gym/build_app action (opt-in via some ConfigItem, maybe?)

See https://github.com/Automattic/bash-cache-buildkite-plugin/pull/19/files#r838036148.

Ideally, we should extract the logic that decides which sha* command to call in its own script, so we can refer to it in the other files to avoid duplication.

This is a bit of a nitpick, if we're honest. But... I just reviewed wordpress-mobile/WordPressAuthenticator-iOS#839 and looked at the logs of the new SwiftLint CI step

From there, it's not clear what the step does other than downloading the image from Docker. This is because we are in the happy path and no warning or error was raised. Still, it would be good to see a log like

SwiftLint finished with exit status 0.

From: https://github.com/Automattic/studio/pull/166/files#r1613715724

The install_npm_packages command installs NPM packages. It was a bit limited for Studio, so Studio used a custom shell script, with the notable differences:

• use patches/ directory to build a cache key
• use npm ci vs npm install

Let's review if it makes sense to bring those improvements upstream and use the main command in Studio.

Done is:

• We review if it's with porting Studio NPM install script to upstream

Background

While working on adding a pipeline to rebuild the pods and Git cache on a schedule for Pocket Casts iOS I run into what looks like a Bundler related issue:

bundler: failed to load command: pod (/usr/local/var/buildkite-agent/builds/builder/wordpress-mobile/pocket-casts-ios/vendor/bundle/ruby/2.7.0/bin/pod)

Gem::LoadError: You have already activated bundler 2.2.28, but your Gemfile requires bundler 2.1.4. Since bundler is a default gem, you can either remove your dependency on it or try updating to a newer version of bundler that supports bundler as a default gem.

See:

• The first time I got the error
• The build that gets the error again after I reverted the fix as a test

The only way I managed to work around it was by calling gem install bundler:2.2.32 (the latest version at the time of writing) before calling install_gems. Green build here.

The issue, as far as I can place it, is that the machine is running on a 2.1.x version of Bundler.

Suggestion / Question

I wonder if it would be safe, or useful, to parse the Gemfile.lock for the BUNDLE WITH value and install that version of Bundler before running other commands in those actions that depend of bundle exec 🤔

Alternatively, if we can prove the issue is just a matter of 2.1.x vs 2.2.x quirks, we might want to ensure we run on the latest 2.2.x as part as our VMs boot process (which is of course out of scope for this repo).

Now that we're starting to have project which are building on Windows agents (esp. Desktop apps like Studio, etc), it would be useful if we can confirm that the generic helpers from a8c-ci-toolkit (e.g. save_cache, hash_file, …) are able to be used on Windows agents too, and if not, make them Windows-compatible.

We'll first need to check:

• If our Windows agent already have bash support built-in (maybe everything already works already? We just haven't tested it yet)
• Or if the calls to any a8c-ci-toolkit-buildkite-plugin/bin helper needs to be wrapped in a .ps1 PowerShell script that would run them in a bash container or something…
• Or if we'll need to translate each of our helpers in .ps1 (thus having two copies of each helper, one written in .ps1 and one in bash?)

I can't get add_ssh_key_to_agent to run, neither locally nor from CI. In both case, I get

Error loading key "/Users/gio/.ssh/<key name>": invalid format

To reproduce:

1. Fetch the latest mobile-secrets and this repo (which should always be in ~/.mobile-secrets)
2. Run sh ~/.mobile-secrets/CI/secrets/pocket-casts-ios/env to export the various env variables
3. From this repo, run bin/add_ssh_key_to_agent $READ_ONLY_DEPLOY_KEY deploy_key (deploy_key is the name the key wll have in ~/.ssh/). You should get Error loading key "/Users/gio/.ssh/deploy_key": invalid format
4. Run bin/add_ssh_key_to_agent $UTILS_FETCH_KEY utils_key. You should get the same kind of error

Notice that is possible the issue is with the keys in that folder, but that seems unlikely at least for READ_ONLY_DEPLOY_KEY because that's what the Buildkite agent uses, right?

It's also possible I'm not using add_ssh_key_to_agent as intended. I think my approach to use the env var with the key directly might be incorrect because it might adds garbage like newlines at the end? Not sure... I'd need to keep experimenting.

Revise the implementation of the comment_on_pr command:

• Do not install gh using brew
• Question: What is the use case for this command?

From: Automattic/studio#164 (comment)

We should fix that in https://github.com/Automattic/a8c-ci-toolkit-buildkite-plugin/blob/trunk/bin/hash_directory#L14 so that it calls our hash_file wrapper in place of calling sha256sum directly in that find … command.

Done is:

• hash_directory works on other systems than Mac

When adding the install_swiftpm_dependencies action, I added a tar option in the save_cache action that uses a relative path. See this comment thread for details: #43 (comment)

Existing tar usage: tar -czf "$CACHE_KEY" "$CACHE_FILE"
Relative path: tar -czf "$CACHE_KEY" -C "$CACHE_FILE" .

Let's research whether the relative path example could be used by all the instances of save_cache in the A8C code base.

While working on adding the scheduled cache rebuilding pipeline on Pocket Casts iOS, I noticed the latest version is tagged 1.7.0. This name, while correct SemVer-wise, breaks the pattern from the previous versions, which were tagged using the v prefix: v1.6.1, v1.6.0, etc.

@AliSoftware, you created that tag, so I'm wondering if this was intentional or muscle-memory from other repos 😄 cc @jkmassel

I did a quick search for "1.7.0" in Slack but it came out short.