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.

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

ci: Fix multiple issues: refs, scripts, variables, locations #38

Merged
merged 1 commit into from
Sep 29, 2023

Conversation

joeyparrish
Copy link
Member

@joeyparrish joeyparrish commented Sep 28, 2023

This fixes multiple CI issues:

  • Since moving to pull_request_target trigger (ci: Change test trigger #37), we were checking out the wrong ref in build.yaml.
  • Repository variables should be used instead of secrets. This new feature of GitHub was introduced in January 2023, and gives us an easy way to configure the repo without abusing the secrets system. https://github.blog/changelog/2023-01-10-github-actions-support-for-configuration-variables-in-workflows/
  • Move the api-client folder to the root where it is more discoverable.
  • Move the build matrix config file to the root where it is more discoverable.
  • Move package installation steps out of the workflow and into a script where they can be tested in a PR.

@joeyparrish joeyparrish marked this pull request as ready for review September 28, 2023 17:02
@joeyparrish
Copy link
Member Author

joeyparrish commented Sep 28, 2023

Unfortunately, this isn't triggering the test itself because the new trigger (pull_request_target) runs the workflow that exists on the target (shaka-project), not the fork (joeyparrish). So the effect of this isn't seen until after it is merged.

However, you can see the effect here on a dummy PR on my fork:

https://github.com/joeyparrish/static-ffmpeg-binaries/actions/runs/6341990006/job/17226831542?pr=5

Configure Build Matrix logs:

{
  enableDebug: true,
  enableSelfHosted: false,
  matrix: [
    {
      os: 'ubuntu-latest',
      os_name: 'linux',
      target_arch: 'x64',
      exe_ext: ''
    },
    {
      os: 'macos-latest',
      os_name: 'osx',
      target_arch: 'x64',
      exe_ext: ''
    },
    {
      os: 'windows-latest',
      os_name: 'win',
      target_arch: 'x64',
      exe_ext: '.exe'
    }
  ]
}

You can see the effect of vars.ENABLE_DEBUG.

After this is merged, we should see the effect of vars.ENABLE_SELF_HOSTED on the repo in shaka-project, after which I can do another manually-triggered build and verify that everything works on linux-arm64.

Above I mentioned that pull_request_target runs the workflow that exists on the target (shaka-project), not the fork (joeyparrish). So one good side-effect of splitting the build scripts out from the workflow into separate script files (#22) is that this enables pull_request_target to test changes to those scripts.

This fixes multiple CI issues:
 - Since moving to pull_request_target trigger, we were checking out
   the wrong ref in build.yaml.
 - Repository variables should be used instead of secrets.  This new
   feature of GitHub was introduced in January 2023, and gives us an
   easy way to configure the repo without abusing the secrets system.
   https://github.blog/changelog/2023-01-10-github-actions-support-for-configuration-variables-in-workflows/
 - Move the api-client folder to the root where it is more
   discoverable.
 - Move the build matrix config file to the root where it is more
   discoverable.
 - Move package installation steps out of the workflow and into a
   script where they can be tested in a PR.
@joeyparrish joeyparrish changed the title ci: Use repository variables instead of secrets ci: Fix multiple issues: refs, scripts, variables, locations Sep 28, 2023
@joeyparrish
Copy link
Member Author

I discovered some further issues that impact PR testing, so I've fixed those here and rewritten the PR description to match. PTAL!

@joeyparrish joeyparrish merged commit 8699131 into shaka-project:main Sep 29, 2023
1 check passed
@joeyparrish joeyparrish deleted the no-secrets branch September 29, 2023 15:13
@github-actions github-actions bot added the status: archived Archived and locked; will not be updated label Nov 28, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: archived Archived and locked; will not be updated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants