Skip to content

Conversation

@flakey5
Copy link
Member

@flakey5 flakey5 commented Feb 10, 2025

Old Node versions have file symlinks pointing to them, but the worker wasn't handling this (i.e. for https://nodejs.org/dist/v0.1.99/ there should be a node-v0.1.99.tar.gz file but there's not).

This:

  • Makes a new constants file fileSymlinks.json that holds whatever files need symlinks. The key is the symlink location and the value is the actual location of the file
  • Makes the build-r2-symlinks.js script read fileSymlinks.json and include them in the cached directory listings
  • Makes the R2 provider handle these symlinks

@flakey5 flakey5 requested a review from a team as a code owner February 10, 2025 20:40
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're still well under the 10mb limit for a worker but since this file is only going to grow, I think we should continue looking at something like #159

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed :)

// Let's add these to our cached directories.
const fileSymlinks = JSON.parse(await readFile(FILE_SYMLINKS, 'utf8'));

for (const file of Object.keys(fileSymlinks)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be cleaned up a bit? Maybe instead if else, we can have different loops just to separate concerns. Performance here should not be a driving factor.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not really sure separating it into two different loops would look much better,

for (const file of Object.keys(fileSymlinks)) {
  const directory = `${dirname(file)}/`

  if (!(directory in cachedDirectories)) {
    continue;
  }

  const actualFile = await headFile(client, fileSymlinks[file]);

  cachedDirectories[directory].files.push({
    ...actualFile,
    name: basename(file)
  })
}

for (const file of Object.keys(fileSymlinks)) {
  const directory = `${dirname(file)}/`

  if (!(directory in cachedDirectories)) {
    continue;
  }

  const actualFile = await headFile(client, fileSymlinks[file]);

  const contents = await listDirectory(client, directory);
  contents.files.push({
    ...actualFile,
    name: basename(file),
  })

  cachedDirectories[directory] = contents;
}

@flakey5 flakey5 force-pushed the flakey5/20250128/symlinks branch from 6b08c2e to 71edd3a Compare February 21, 2025 02:01
@flakey5 flakey5 merged commit 114c261 into main Feb 21, 2025
5 checks passed
@flakey5 flakey5 deleted the flakey5/20250128/symlinks branch February 21, 2025 02:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants