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

Add github:artifact resource that unzips the doubly zipped files #1799

Draft
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

adamziel
Copy link
Collaborator

@adamziel adamziel commented Sep 24, 2024

Motivation for the change, related issues

GitHub artifacts are difficult to use as Blueprint resources. As doubly zipped archives, they need convoluted peeling. This PR introduces a new resource type called github:artifact that handles the peeling for the developer.

With this PR, a Blueprint may look like this:

{
    "steps": [
        {
            "step": "installPlugin",
            "pluginZipFile": {
                "resource": "github:artifact",
                "owner": "WordPress",
                "repo": "gutenberg",
                "workflow": "Build Gutenberg Plugin Zip",
                "artifact": "gutenberg-plugin",
                "pr": 65590
            }
        }
    ]
}

Instead of this:

{
	"steps": [
		{
			step: 'mkdir',
			path: '/wordpress/pr',
		},
		/*
		 * This is the most important step.
		 * It download the built plugin zip file exposed by GitHub CI.
		 *
		 * Because the zip file is not publicly accessible, we use the
		 * plugin-proxy API endpoint to download it. The source code of
		 * that endpoint is available at:
		 * https://github.com/WordPress/wordpress-playground/blob/trunk/packages/playground/website/public/plugin-proxy.php
		 */
		{
			step: 'writeFile',
			path: '/wordpress/pr/pr.zip',
			data: {
				resource: 'url',
				url: zipArtifactUrl,
				caption: `Downloading Gutenberg PR ${prNumber}`,
			},
			progress: {
				weight: 2,
				caption: `Applying Gutenberg PR ${prNumber}`,
			},
		},
		/**
		 * GitHub CI artifacts are doubly zipped:
		 *
		 * pr.zip
		 *    gutenberg.zip
		 *       gutenberg.php
		 *       ... other files ...
		 *
		 * This step extracts the inner zip file so that we get
		 * access directly to gutenberg.zip and can use it to
		 * install the plugin.
		 */
		{
			step: 'unzip',
			zipPath: '/wordpress/pr/pr.zip',
			extractToPath: '/wordpress/pr',
		},
		{
			step: 'installPlugin',
			pluginData: {
				resource: 'vfs',
				path: '/wordpress/pr/gutenberg.zip',
			},
		}
	]
}

Closes #1796

Remaining work

  • Fix this issue in Safari: TypeError: ReadableStreamBYOBReader needs a ReadableByteStreamController
  • Add unit tests and run them in Node.js v18, v20, and v22
  • Test in Firefox

Follow-up work

Add first-class Zip64 support to the stream compression package. Right now we're wiring it together manually in the Resource class.

GitHub artifacts are compressed as Zip64 and we cannot simply iterate through the files. Instead, we must first read the central directory index end, then the central directory index, and then use that information to find and unzip the right file entry. Unfortunately, the file headers list 0 as their compressed size.

Technically, this requires buffering the entire response stream, and repeatedly teeing it to seek to the central directory index end, then central directory index, and then to the right file.

Testing Instructions (or ideally a Blueprint)

  1. Go to http://localhost:5400/website-server/#{%20%22steps%22:%20[%20{%20%22step%22:%20%22installPlugin%22,%20%22pluginZipFile%22:%20{%20%22resource%22:%20%22github:artifact%22,%20%22owner%22:%20%22WordPress%22,%20%22repo%22:%20%22gutenberg%22,%20%22workflow%22:%20%22Build%20Gutenberg%20Plugin%20Zip%22,%20%22artifact%22:%20%22gutenberg-plugin%22,%20%22pr%22:%2065590%20}%20}%20]%20}
  2. Confirm the Gutenberg plugin was installed from a GitHub artifact

@adamziel
Copy link
Collaborator Author

Safari doesn't support BYOB streams to the extent needed to make this work. We'll need to either avoid using BYOB streams or use the web-streams-polyfill (but only in Safari). Here's an example of using the polyfill in another PR:

5973780/packages/php-wasm/stream-compression/src/polyfills.ts

@adamziel
Copy link
Collaborator Author

An easy solution: Ditch BYOB streams. We'd have to rearchitecting the stream-compression package to store the state (bytes read so far, buffer etc.) outside of the stream. That's what the PHP version of ZipStreamReader does. That will also make it easy to seek forward (via teeing the stream).

@adamziel
Copy link
Collaborator Author

adamziel commented Sep 25, 2024

A naive approach worked in this commit. I'm leaving this exploration open for now, I need an uninterrupted day or two to get to the bottom of this. Alternatively, if we can find a ~10KB JavaScript streaming library that can skip bytes, slice streams, fork them etc. without relying on BYOB streams while offering interop with DecompressionStream that would solve it, too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Blocked
Development

Successfully merging this pull request may close these issues.

[Blueprints] Add a github:artifact resource type
1 participant