setup-git-for-windows-sdk icon indicating copy to clipboard operation
setup-git-for-windows-sdk copied to clipboard

getViaGit: get minimal flavor from ci-artifacts

Open dennisameling opened this issue 1 year ago • 1 comments

As a result of recent changes, the Git SDK ci-artifacts are published as GitHub release assets now. This eliminates the need for us to clone and cache the artifacts in this GitHub action.

Let's simply download the latest .tar.gz from the ci-artifacts and extract it.

Ref: https://github.com/git-for-windows/git-sdk-64/commit/fdb0cea373893ce7d40bcfcfbeb7fd091a3c4020 Ref: https://github.com/git-for-windows/git-sdk-64/pull/87

dennisameling avatar Sep 22 '24 17:09 dennisameling

Some initial results:

  • Running the action now takes only 3s, down from 2m43s (uncached)!!!
  • Triggered a test build of git-for-windows/git here, ~awaiting results~ and it seems to be working as expected.

dennisameling avatar Sep 22 '24 17:09 dennisameling

Range-diff relative to pre-force-push
  • 1: 5242b29 ! 1: 6db6522 getViaGit: get minimal flavor from ci-artifacts

    @@ Metadata
     Author: Dennis Ameling <[email protected]>
    
      ## Commit message ##
    -    getViaGit: get minimal flavor from ci-artifacts
    +    minimal: get from ci-artifacts
    
    -    As a result of recent changes, the Git SDK `ci-artifacts` are published as GitHub release assets now. This eliminates the need for us to clone and cache the artifacts in this GitHub action.
    +    As a result of recent changes, the Git SDK `ci-artifacts` are published
    +    as GitHub release assets now, including several variants of the minimal
    +    flavor of Git for Windows' SDK. This eliminates the need for us to clone
    +    the minimal SDK in this GitHub Action.
    
    -    Let's simply download the latest `.tar.gz` from the `ci-artifacts` and extract it.
    +    Let's simply download the latest `.tar.gz` from the `ci-artifacts` and
    +    extract it.
    +
    +    Note: As per the analysis in
    +    https://github.com/git-for-windows/git-sdk-64/commit/fdb0cea37389, we
    +    use the native `tar.exe` to unpack the minimal SDK while it is
    +    downloaded, for maximal speed.
    +
    +    The analysis also suggests to use the `.zip` file, as this results in
    +    the fastest operation when download speeds are above 6MB/second (which
    +    we hope will be reliably the case in GitHub Actions). However, we want
    +    to pipe the archive to `tar -xf -` while fetching, and it seems that for
    +    some reason `C:\Windows\system32\tar.exe` misses `.sparse/` and `etc/`
    +    when extracting `.zip` files from `stdin`, but the same is not true with
    +    `.tar.gz` files. So let's use the latter.
    +
    +    See also: https://github.com/git-for-windows/git-sdk-64/pull/87.
    
    -    Ref: https://github.com/git-for-windows/git-sdk-64/commit/fdb0cea373893ce7d40bcfcfbeb7fd091a3c4020
    -    Ref: https://github.com/git-for-windows/git-sdk-64/pull/87
         Signed-off-by: Dennis Ameling <[email protected]>
    +    Signed-off-by: Johannes Schindelin <[email protected]>
    
      ## main.ts ##
    -@@ main.ts: async function run(): Promise<void> {
    -         useCache = true
    -         break
    -       case 'auto':
    --        useCache = flavor !== 'full'
    -+        useCache = !['full', 'minimal'].includes(flavor)
    -         break
    -       default:
    -         useCache = false
    -
    - ## src/git.ts ##
    -@@ src/git.ts: import {ChildProcess, spawn} from 'child_process'
    - import {Octokit} from '@octokit/rest'
    - import {delimiter} from 'path'
    +@@ main.ts: import {
    +   getViaGit,
    +   gitForWindowsUsrBinPath
    + } from './src/git'
    ++import {getViaCIArtifacts} from './src/ci_artifacts'
      import * as fs from 'fs'
    -+import os from 'os'
    -+import fetch, {RequestInit} from 'node-fetch'
    
    - // If present, do prefer the build agent's copy of Git
    - const externalsGitDir = `${process.env.AGENT_HOMEDIRECTORY}/externals/git`
    -@@ src/git.ts: async function updateHEAD(
    -   })
    - }
    + const flavor = core.getInput('flavor')
    +@@ main.ts: async function run(): Promise<void> {
    +     const verbose = core.getInput('verbose')
    +     const msysMode = core.getInput('msys') === 'true'
    
    --export async function getViaGit(
    --  flavor: string,
    --  architecture: string,
    --  githubToken?: string
    --): Promise<{
    -+type GetViaGitResult = {
    -   artifactName: string
    -   id: string
    -   download: (
    -     outputDirectory: string,
    -     verbose?: number | boolean
    -   ) => Promise<void>
    --}> {
    -+}
    +-    const {artifactName, download, id} = await getViaGit(
    +-      flavor,
    +-      architecture,
    +-      githubToken
    +-    )
    ++    const {artifactName, download, id} =
    ++      flavor === 'minimal'
    ++        ? await getViaCIArtifacts(architecture, githubToken)
    ++        : await getViaGit(flavor, architecture, githubToken)
    +     const outputDirectory =
    +       core.getInput('path') || `${getDriveLetterPrefix()}${artifactName}`
    +     core.setOutput('result', outputDirectory)
    +
    + ## src/ci_artifacts.ts (new) ##
    +@@
    ++import * as core from '@actions/core'
    ++import {Octokit} from '@octokit/rest'
    ++import {getArtifactMetadata} from './git'
    ++import {spawn} from 'child_process'
    ++import * as fs from 'fs'
     +
    -+export async function getViaGit(
    -+  flavor: string,
    ++export async function getViaCIArtifacts(
     +  architecture: string,
     +  githubToken?: string
    -+): Promise<GetViaGitResult> {
    -   const owner = 'git-for-windows'
    - 
    -   const {repo, artifactName} = getArtifactMetadata(flavor, architecture)
    - 
    -   const octokit = githubToken ? new Octokit({auth: githubToken}) : new Octokit()
    --  let head_sha: string
    ++): Promise<{
    ++  artifactName: string
    ++  id: string
    ++  download: (
    ++    outputDirectory: string,
    ++    verbose?: number | boolean
    ++  ) => Promise<void>
    ++}> {
    ++  const owner = 'git-for-windows'
     +
    -   if (flavor === 'minimal') {
    --    const info = await octokit.actions.listWorkflowRuns({
    --      owner,
    --      repo,
    --      workflow_id: 938271,
    --      status: 'success',
    --      branch: 'main',
    --      event: 'push',
    --      per_page: 1
    --    })
    --    head_sha = info.data.workflow_runs[0].head_sha
    --    /*
    --     * There was a GCC upgrade to v14.1 that broke the build with `DEVELOPER=1`,
    --     * and `ci-artifacts` was not updated to test-build with `DEVELOPER=1` (this
    --     * was fixed in https://github.com/git-for-windows/git-sdk-64/pull/83).
    --     *
    --     * Work around that by forcing the incorrectly-passing revision back to the
    --     * last one before that GCC upgrade.
    --     */
    --    if (head_sha === '5f6ba092f690c0bbf84c7201be97db59cdaeb891') {
    --      head_sha = 'e37e3f44c1934f0f263dabbf4ed50a3cfb6eaf71'
    --    }
    --  } else {
    --    const info = await octokit.repos.getBranch({
    --      owner,
    --      repo,
    --      branch: 'main'
    --    })
    --    head_sha = info.data.commit.sha
    -+    return getMinimalFlavor(owner, repo, artifactName, octokit, githubToken)
    -   }
    ++  const {repo, artifactName} = getArtifactMetadata('minimal', architecture)
     +
    -+  const info = await octokit.repos.getBranch({
    -+    owner,
    -+    repo,
    -+    branch: 'main'
    -+  })
    -+  const head_sha = info.data.commit.sha
    -   const id = `${artifactName}-${head_sha}${head_sha === 'e37e3f44c1934f0f263dabbf4ed50a3cfb6eaf71' ? '-2' : ''}`
    -   core.info(`Got commit ${head_sha} for ${repo}`)
    - 
    -@@ src/git.ts: export async function getViaGit(
    -     }
    -   }
    - }
    ++  const octokit = githubToken ? new Octokit({auth: githubToken}) : new Octokit()
     +
    -+async function getMinimalFlavor(
    -+  owner: string,
    -+  repo: string,
    -+  artifactName: string,
    -+  octokit: Octokit,
    -+  githubToken?: string
    -+): Promise<GetViaGitResult> {
     +  const ciArtifactsResponse = await octokit.repos.getReleaseByTag({
     +    owner,
     +    repo,
    @@ src/git.ts: export async function getViaGit(
     +    )
     +  }
     +
    ++  core.info(`Found ci-artifacts release: ${ciArtifactsResponse.data.html_url}`)
     +  const tarGzArtifact = ciArtifactsResponse.data.assets.find(asset =>
     +    asset.name.endsWith('.tar.gz')
     +  )
     +
     +  if (!tarGzArtifact) {
     +    throw new Error(
    -+      `Failed to find a tar.gz artifact in the ci-artifacts release of the ${owner}/${repo} repo`
    ++      `Failed to find a .tar.gz artifact in the ci-artifacts release of the ${owner}/${repo} repo`
     +    )
     +  }
     +
    ++  const url = tarGzArtifact.browser_download_url
    ++  core.info(`Found ${tarGzArtifact.name} at ${url}`)
    ++
     +  return {
     +    artifactName,
     +    id: `ci-artifacts-${tarGzArtifact.updated_at}`,
    @@ src/git.ts: export async function getViaGit(
     +      outputDirectory: string,
     +      verbose: number | boolean = false
     +    ): Promise<void> => {
    -+      const tmpFile = `${os.tmpdir()}/${tarGzArtifact.name}`
    -+      core.info(
    -+        `Downloading ${tarGzArtifact.browser_download_url} to ${tmpFile}...`
    -+      )
    -+      await downloadFile(
    -+        tarGzArtifact.browser_download_url,
    -+        {
    -+          headers: {
    -+            ...(githubToken ? {Authorization: `Bearer ${githubToken}`} : {}),
    -+            Accept: 'application/octet-stream'
    -+          }
    -+        },
    -+        tmpFile
    -+      )
    -+      core.info(`Extracting ${tmpFile} to ${outputDirectory}...`)
    -+      fs.mkdirSync(outputDirectory)
    -+      const child = spawn(
    -+        'C:\\Windows\\system32\\tar.exe',
    -+        [`-xz${verbose ? 'v' : ''}f`, tmpFile, '-C', outputDirectory],
    -+        {
    -+          stdio: [undefined, 'inherit', 'inherit']
    -+        }
    -+      )
     +      return new Promise<void>((resolve, reject) => {
    -+        child.on('close', code => {
    -+          if (code === 0) {
    -+            core.info('Finished extracting archive.')
    -+            fs.rm(tmpFile, () => resolve())
    -+          } else {
    -+            reject(new Error(`tar -xzf process exited with code ${code}`))
    ++        const curl = spawn(
    ++          `${process.env.SYSTEMROOT}/system32/curl.exe`,
    ++          [
    ++            ...(githubToken
    ++              ? ['-H', `Authorization: Bearer ${githubToken}`]
    ++              : []),
    ++            '-H',
    ++            'Accept: application/octet-stream',
    ++            `-${verbose === true ? '' : 's'}fL`,
    ++            url
    ++          ],
    ++          {
    ++            stdio: ['ignore', 'pipe', process.stderr]
     +          }
    ++        )
    ++        curl.on('error', error => reject(error))
    ++
    ++        fs.mkdirSync(outputDirectory, {recursive: true})
    ++
    ++        const tar = spawn(
    ++          `${process.env.SYSTEMROOT}/system32/tar.exe`,
    ++          ['-C', outputDirectory, `-x${verbose === true ? 'v' : ''}f`, '-'],
    ++          {stdio: ['pipe', process.stdout, process.stderr]}
    ++        )
    ++        tar.on('error', error => reject(error))
    ++        tar.on('close', code => {
    ++          if (code === 0) resolve()
    ++          else reject(new Error(`tar exited with code ${code}`))
     +        })
    ++
    ++        curl.stdout.pipe(tar.stdin)
     +      })
     +    }
     +  }
     +}
    -+
    -+async function downloadFile(
    -+  url: string,
    -+  options: RequestInit,
    -+  destination: string
    -+): Promise<void> {
    -+  const response = await fetch(url, options)
    -+
    -+  if (!response.ok) {
    -+    throw new Error(`Failed to fetch ${url}: ${response.statusText}`)
    -+  }
    -+
    -+  const fileStream = fs.createWriteStream(destination)
    -+  response.body.pipe(fileStream)
    -+
    -+  return new Promise((resolve, reject) => {
    -+    fileStream.on('finish', resolve)
    -+    fileStream.on('error', reject)
    -+  })
    -+}
    
  • -: ------- > 2: 3d4ea07 getViaCIArtifacts: add some resilience

  • -: ------- > 3: 64d3606 minimal: avoid caching it

  • 2: 5a3cf09 = 4: 8bfbac8 npm run build && npm run package

Not sure how useful this is ;-) I guess it shows that I almost rewrote the patch... 😊

dscho avatar Dec 17 '24 17:12 dscho