Scheduled build improvements

[dnzxy]

This PR builds on the scheduled sync and build functionality introduced by @bjornoleh. It makes the feature truly opt-in rather than breaking existing browser build setups for Loop.

Proposed changes:

• Introduces two new variables, SCHEDULED_BUILD and SCHEDULED_SYNC and replaces SYNC_UPSTREAM.
• Based on these two variables there now are two types of automated/scheduled builds possible:
  (1) Scheduled build and (2) scheduled sync (with build).
• Utilizes GH workflow permissions and secrets.GITHUB_TOKEN for any repo-based tasks during keepalive and scheduled sync, thereby avoiding changes to GH_PAT access roles (adding workflow role no more necessary)

The automated build will now run on the following conditions:

• If no scheduling was enabled, Loop build only runs when manually triggered.
• If only scheduled building is enabled, it will run every night at 04:00am UTC to do a keepalive commit, that keeps the fork active. By default, a scheduled build will run every 1st of the month, also at 04:00am UTC.
• If scheduled sync is enabled, by default it will run every night at 04:00am UTC to do a keepalive commit and check for changes; if there are changes, it will then build. If not, it will only do the keepalive commit.

Required changes:

• Create alive branch based on LoopKit/LoopWorkspace:dev
• Create a repository variable SCHEDULED_BUILD set to true
• Create a repository variable SCHEDULED_SYNC set to true

Thanks to @bjornoleh for laying the groundwork and to @billybooth for being a great sparring partner

[dnzxy]

Updated the PR. After all, we still need to make use of GH_PAT token to facilitate changes to workflow files, e.g., updating Xcode or macOS versions in build_loop.yaml. I missed to test this; thanks to @bjornoleh for the nod in the right direction.

• Updated build_loop.yaml to use GH_PAT instead of GITHUB_TOKEN with limited scopes (just like before)
• Updated testflight.md with instructions how to add the workflow scope; moved this instruction down to the OPTIONAL section.

[dnzxy]

Updated PR once again. Fixed an issue where the build would be triggered during the "sync cron tab" (0 4 * * *) although there were no new commits. The culprit seems to have been the usage of github expressions ${{ }} within action step's if: checks. As per the docs:

When you use expressions in an if conditional, you may omit the ${{ }} expression syntax because GitHub Actions automatically evaluates the if conditional as an expression. Using the ${{ }} expression syntax turns the contents into a string, and strings are truthy. For example, if: true && ${{ false }} will evaluate to true. For more information, see "Expressions."

This was fixed.

Test runs see here:

• Action triggered by the "sync+build" cron with NO new commits -> log here -> skips build ✅
• Action triggered by the "sync+build cron" with NEW commits -> log here -> builds ✅
• Action triggered by the "build" cron (no new commits, but cron is checked as build condition) -> log here -> builds ✅
• Manually triggered action with scheduled build + sync enabled -> log here -> builds ✅ (action failed due to Testflight version numbering; I triggered the manual build too soon)
• Manually triggered action with no scheduling enabled (Loop main-style build) -> log here -> builds ✅

[dnzxy]

Reworked this PR after discussions on Zulip chat around opt-out vs. opt-in for automated builds.

The workflow now takes as much configuration as possible out of the Looper's hands and an added check for workflow permissions of GH_PAT is the `opt-in semaphore* to enabled automated builds.

• If GH_PAT holds workflowpermission, the automated keepalive, sync and build is enabled and works out of the box.
• If GH_PAT lacks these permissions, the manual build still works and nothing is broken upon release; no automation happens
• The two added variables SCHEDULED_BUILD and SCHEDULED_SYNC can still be used to granularly configure the automated build
• By default, automated keepalive + sync + build are enabled if automation is enabled.

I think there currently are two limitations:

• Users that have customized their build may run into failing builds, when changes are synced in that lead to merge conflicts with their customization. This can happen when the customizations aren't adjusted to new versions of Loop. I guess this will primarily be an issue for Loopers running dev.
• Fine-grained PATs currently do not hold scopes, as the x-oauth-scope header is empty. This is due to fine-grained tokens still being in beta status. We consider an empty response for the workflow check as, so ""as success; if a token only holds repo permissions, the check for workflow will fail and the repo permission is checked as part of the validation workflow.

Happy to receive feedback.

[ps2]

Could you avoid overwriting GITHUB_TOKEN? GH_PAT and GITHUB_TOKEN are two different things. It'd be nice to keep them separate in this file to avoid confusion about what's being used where.

Ideally we could even rename GH_PAT to be more explicit about how it is different from GITHUB_TOKEN. Something like MATCH_REPO_ACCESS_TOKEN, or something like that. But that's adding even more changes for the user to deal with.

[dnzxy]

I was under the impression, that gh CLI requires the environment variable to be named GITHUB_TOKEN but just double-checked and GH_PAT is fine and even what is instructed in the manual. Change commit coming in.

Edit: The token does much more now than just provide access to the Match-Secrets repo. It is used for checking workflow permissions, checking for alive branch existence and conditionally creating it, etc. It is, in a way, a real GH access token.

[dnzxy]

In the documentation, for the PAT creation this used to be named GH_PAT and has since been changed to Fastlane Access Token (the name of the token, not so much the menu item, since secret is still named that way). We can change GH_PAT to FASTLANE_ACCESS_TOKEN if you want, but I feel like that may cause issues when people can't differentiate what secrets are based on Apple Developer information / keys / IDs and what is GitHub-based.

[ps2]

I liked the change to use the built in GITHUB_TOKEN where possible. Can we allow it to have workflow permissions, do the sync and checking for alive branch? Just didn't want to overwrite it in the env with GH_PAT. Then GH_PAT has a smaller set of responsibilities.

[dnzxy]

We cannot use GITHUB_TOKEN to check the workflow permission, cause the cURL request we send to check for x-oauth-scopes must contain the token to-be-checked. We also cannot use it for any syncing, because whenever we sync, there are possible changes to workflow files, and GITHUB_TOKEN unfortunately cannot hold workflow permissions, so we need the Looper-created GH_PAT with repo and workflow scopes for that.

I misunderstood your initial comment, so I'll change that back. We can use the standardized GITHUB_TOKEN to check for alive branch and, if not existent, create it. I will rename the environment variable in the permissions check so that it becomes more readable what actually happens there 👍

[dnzxy]

Changed now. Hope this is okay now 😄

[marionbarker]

Comment regarding testflight.md modification:

The testflight.md is set up for an earlier version of the PR before the scope for GH_PAT was selected as a determining factor for actions if the two variables, SCHEDULED_BUILD and SCHEDULED_SYNC, are not configured.

In other words - it is consistent with information in this comment:

Required changes:
  Create alive branch based on LoopKit/LoopWorkspace:dev
  Create a repository variable SCHEDULED_BUILD set to true
  Create a repository variable SCHEDULED_SYNC set to true

Not this later comment:

• The workflow now takes as much configuration as possible out of the Looper's hands and an added check for workflow permissions of GH_PAT is the `opt-in semaphore* to enabled automated builds.

The optional variables SCHEDULED_BUILD and SCHEDULED_SYNC can be configured to modify the default behavior - (assuming GH_PAT has workflow scope enabled): if new commits are found, then auto-sync and auto-build.

[marionbarker]

Detailed testing (first of several comments):

Start with dev branch at commit 868fcd7.
apply PR 71 to dev.
Following files are modified:

• modified: .github/workflows/build_loop.yml
• modified: fastlane/testflight.md

Repository: https://github.com/marionbarker/LoopWorkspace
Default Branch: dev_plus_pr71
Configuration:

• All 6 secrets are valid
• Match-Secrets is brand new and private
• All distribution certs at Apple removed, so new ones can be generated
• Run Verify Secrets, Add Identifiers and Create Certificates successfully

Start by deleting my alive branch (as if coming from main).

First test: alive branch does not exist, run Build Loop manually

• Build Loop failure log
  • error message is "Check alive branch and permissions"
  • I thought this PR was supposed to create alive branch and it does not, but that can be fixed and tested later

Second test: create alive branch manually, run Build Loop manually

• Build Loop success log

The other tests need to run at specific times (cron job).
Modify build_loop.yml so that tests will run more frequently so I can test automation.
Report these in new comments.

[dnzxy]

The testflight.md is set up for an earlier version of the PR before the scope for GH_PAT was selected as a determining factor for actions if the two variables, SCHEDULED_BUILD and SCHEDULED_SYNC, are not configured.

That is correct. As I wrote in this comment on Zulip I wanted to wait for "final okay" regarding my modifications and the scope of this PR before I changed testflight.md again and again. I will change it to reflect the current feature scope if everyone, especially @ps2 , is happy with it (which I think is the case).

Will await further tests and then fix in one go. I'll start looking into the permission issues with alive auto-creation though – I tested this thoroughly and it always worked for me. Will look into it! Fixed the broken auto-create for alive branch.

[marionbarker]

Modified several items (one commit at a time):

• Set time_elapsed to 1 day (instead of 20) to trigger an automated keep-alive commit (more frequently)
• Set cron for check for updates to be earlier so it will run.

Leave the GH_PAT scope as workflow.

• Build Loop (scheduled) ran as expected: link
• The keep alive portion did run, alive showed up with the same age as dev

[marionbarker]

Modify GH_PAT scope to be repo (not workflow) << edited - forgot to save so the scope was still workflow.

• modify cron to run again soon: marionbarker@b577e84

Expect it to skip the check for commits portion because of scope change
While waiting, cherry-pick the alive fix commit from dnzxy.
So go ahead and delete the alive branch.

Expectation - nothing will happen because the scope is repo (not workflow).

• successful "run" link
• Expectation (almost) met
  • see warning below
  • alive branch was created (I guess this doesn't require workflow)

Reran this with alive branch deleted and GH_PAT set to repo (not workflow).

• alive branch is still created

[marionbarker]

Realized I had not modified GH_PAT before - so still at workflow scope; add the variable SCHEDULED_SYNC to repository and set value to false.

• modify cron to run soon
• expectation: build_loop.yml will deploy as scheduled task but nothing will happen
• successful run link
• not a real test because there are no updates - the keep alive worked (because of the workflow scope)

[marionbarker]

Configuration for the test:

• Modify cron for build to be soon link.
• Restore GH_PAT to workflow scope.
• Delete the alive branch

Expectation is buld_loop will:

• create alive branch - yes
• apply a commit to alive branch - to keep it alive - yes
• skip syncing the branch (if there had been commits) - "sync upstream changes" was not run
• build current branch - successful build

[marionbarker]

Configuration for the test:

• Modify cron for build to be soon link.
• leave SCHEDULED_SYNC false
• add SCHEDULED_BUILD and set to false

Expectation is buld_loop will - each expectation was met:

• skip commit to alive branch - already updated once today
• skip syncing the branch (if there had been commits) - "sync upstream changes" was not run
• skip building the branch
• successful build loop log

[marionbarker]

update SCHEDULED_SYNC and SCHEDULED_BUILD to true
delete alive branch
modify cron to run soon

Expectation is build_loop will:

• create alive branch
• add commit to alive branch
• run sync but won't do anything because there is nothing to sync
• build loop

What really happened:

• build loop launched twice
• the first one is still running while second one (2 minutes later) already completed
• link to first one (completed in 20 min)
  • success - did all the things listed in "Expectation" above
• link to second one (completed in 46 sec)
  • success that said all was fine and nothing needed to happen

Conclusion - I believe this line should be removed or changed into a comment:
https://github.com/dnzxy/LoopWorkspace/blob/8ee1b9c5ba0084f59d9bbf6c3ad31c17f0e2713b/.github/workflows/build_loop.yml#L11

I believe this line is what is important
https://github.com/dnzxy/LoopWorkspace/blob/8ee1b9c5ba0084f59d9bbf6c3ad31c17f0e2713b/.github/workflows/build_loop.yml#L166

I tried that idea (see next comment and it did not work)

[marionbarker]

Configuration:

• Modify build_loop.yml to have only one cron statement link to commit
• Delete the alive branch

This only did the creation of alive and adding the commit.
It did not build the app.
link to build_loop action

[bjornoleh]

Both cron statements are needed to allow the app to always be built on a monthly basis, see the if statement around line 166.

[marionbarker]

In my opinion, the default behavior of build_loop.yml for users of main branch (after next release) should be to automatically check for updates and build once a month.

• This will keep the repository actions working (actions are disabled after 60 days of inactivity) and include updates if there are any
• The capability for more frequent update checks is still available in build_loop.yml but the user would need to modify the file to enable that feature
• With this default behavior, every Build Loop scheduled action (that is successful) will be a build

See this modification (where the cron time/date change is just for testing)

• marionbarker@87dc3e0

Test Configuration (similar to someone coming from 3.2.2 with next release):

• alive branch does not exist
• no variables have been added
• GH_PAT has workflow scope
  • (yes I know some won't have that yet, but that part was already tested)

Test Expectation (and results matched expectations)

• create alive branch
• update alive branch if needed (not needed)
  • this update happens if more than 20 days - but in this case, it's only been 2 days
• build loop
• only one scheduled action happens (not two)
• success log

[bjornoleh]

With the current infrastructure, we can let updates happen automatically, within the next day after a release. Your suggestion is to only make this happen once a month? Are there compelling reasons to not automatically update when new versions are released? Sometimes there are critical issues that should be fixed sooner than this. But having automated updates can certainly be controversial for some. Auto updates once a month is in principle the same thing though, but can optionally be disabled.

I think we really should not propose changing the workflow file (or other parts of the code) unless absolutely necessary, as it might sometimes cause merge conflicts. Patching does alter the workflow file, but then there is specific support for this (and probably how to revert changes to fix merge issues).

[dnzxy]

@bjornoleh I think Marion is proposing just to have a monthly scheduled build and that build will
also check for changes in the upstream. But she wants to get rid of the automatically triggered build upon changes when that build would not happen on a different cadence. That’s because to check for updates we need to have frequent action runs and that clutters the log. The argument here is that changes aren’t as frequent for Loop main, so daily checks aren’t necessary.

I agree with you, we should have builds as soon as there are changes and an additional build once a month.

[billybooth]

@bjornoleh said

I think we really should not propose changing the workflow file (or other parts of the code) unless absolutely necessary, as it might sometimes cause merge conflicts. Patching does alter the workflow file, but then there is specific support for this (and probably how to revert changes to fix merge issues).

Agreed. Preferences about run frequency aside, no solution to this problem or disagreement should come with instructions to change the build_loop.yml in the fork to alter behavior. This PR, in large part, exists to avoid downstream users from modifying the workflow file and creating subsequent merge conflicts.

[bjornoleh]

@bjornoleh I think Marion is proposing just to have a monthly scheduled build and that build will

also check for changes in the upstream. But she wants to get rid of the automatically triggered build upon changes when that build would not happen on a different cadence. That’s because to check for updates we need to have frequent action runs and that clutters the log. The argument here is that changes aren’t as frequent for Loop main, so daily checks aren’t necessary.

I agree with you, we should have builds as soon as there are changes and an additional build once a month.

I am not aware of any method to pick up changes besides running a workflow. This can certainly happen on something like a weekly basis instead. It will just mean that updates will happen up to a week after release. But I am not sure if clutter in the workflow logs is of any concern, when things work properly, no one will look at those logs, and if it fails, the last one will probably be all you need to look at.

There is one benefit of running the workflow on a lower cadence though: Sometimes we randomly get the "Could not install WWDR certificate" error. It has always resolved by a second run of the workflow, but reducing the volume of such events would be beneficial for everyone. Not sure if this specific error still happens though, I haven't seen it in a while.

[marionbarker]

I agree completely with that position - editing build_loop.yml to change behavior is not desirable. I had not thought of that.

A compromise position would be to modify

    - cron: '0 4 * * *' # Checks for updates at 04:00 UTC every day

to (change to weekly)

    - cron: '0 5 * * 3' # Checks for updates at 05:00 UTC every Wed

I do think even if we leave it as daily, we should change time of the daily cron to separate the case where the update/alive check happens on the same day of the month as the scheduled build because you do get 2 jobs running at essentially the same time.

    - cron: '0 5 * * *' # Checks for updates at 05:00 UTC every day

Edited my suggestion to make the check for updates cron later than monthly build cron. This avoids two builds on one day for rare cases.

[bjornoleh]

I agree it's not ideal for the two builds to happen at the same time. But perhaps not too big an issue.

They could be moved a little further apart of course. And thinking about it, if the monthly one happens at an earlier time of day, the daily or weekly one will not produce a new build, but stop after checking for new commits, even if there happened to be a release this day. Creating two builds the same day might be confusing. Having two runs at the same day should mostly go unnoticed.

[marionbarker]

Agreed. Edited my earlier comment.

[marionbarker]

Two builds at one time will probably result in one failing. My experience is they both try to use the same TestFlight build number.

[bjornoleh]

Yes, I have also seen issues with build numbers. But if the monthly one runs an hour before the daily one, it will update the fork, and the daily run will then stop after the check for new commits, as there will be no new commits.

Regarding timing of these runs, I have tried to ask before if 04 UTC is a good time. It was chosen for testing in an European setting where it corresponds to 05 or 06 local time, ie a time where there is a very low risk of having a set of incoming code changes to the upstream repository. I guess it's the local time zone/typical working hours for Pete that should form the basis for this with Loop. I don't know what that might be.

[dnzxy]

Fully agree with where this conversation is headed.

Following suggestion:

• Keepalive / check upstream / sync workflow at every Wednesday 2am EST, once weekly
• Scheduled one time build every first Saturday of the month at 2am EST, once monthly
• In cron tabs, this means
  • Sync+Keepalive: 0 8 * * 3
  • Scheduled Build: 0 8 1 * 6

Why 2am EST? So that it is during the night for all Americans and also early morning for Europeans.
Why on different days? So that we don’t have 2 workflow runs and possible builds on one day if a release happened on the same day as the scheduled build.

What do you think? 🤓

[marionbarker]

I made an alternative version for testflight.md that can be reviewed here: https://github.com/marionbarker/LoopWorkspace/blob/dev_plus_pr71/fastlane/testflight.md

• I started with Deniz updated version and then did some reorganization and reformatting
• I added a truth table for the variables (if used)