platform arch refactor

[dpastoor]

In working on #237 getting arm logic added in will take some interface redesign:

The existing Dependency interface is as such:

dependencies.ts

export interface Dependency {
  name: string;
  version: string;
  "darwin"?: PlatformDependency;
  "linux"?: PlatformDependency;
  "windows": PlatformDependency;
}

which is used by say the esBuild (and dart/pandoc) resolvers:

return {
    name: "esbuild javscript bundler",
    version,
    "windows": esBuildRelease("windows-64"),
    "linux": esBuildRelease("linux-64"),
    "darwin": esBuildRelease("darwin-64"),
  };

these then are used in configure.ts as kDependencies

// Download dependencies
  info("Downloading dependencies");
  for (const dependency of kDependencies) {
    info(`Preparing ${dependency.name}`);
    const platformDep = dependency[Deno.build.os];
    if (platformDep) {
      info(`Downloading ${dependency.name}`);
      const targetFile = await downloadBinaryDependency(platformDep, config);

      info(`Configuring ${dependency.name}`);
      await platformDep.configure(targetFile);

      info(`Cleaning up`);
      Deno.removeSync(targetFile);
    }
    info(`${dependency.name} complete.\n`);
  }

APIs to consider:

nested archs - Deno.build.arch can be "x86_64" | "aarch64" making it easy to use as a key

export interface Dependency {
  name: string;
  version: string;
  "aarch64": {
	  "darwin"?: PlatformDependency;
	  "linux"?: PlatformDependency;
	  "windows"?: PlatformDependency;
  };
  "x86_64": {
	  "darwin"?: PlatformDependency;
	  "linux"?: PlatformDependency;
	  "windows"?: PlatformDependency;
  };
}

which would essentially result in dl resolution code changing to:

-const platformDep = dependency[Deno.build.os];
+const platformDep = dependency[Deno.build.arch][Deno.build.os];

or some flatter designs:

• x86 default:
  • darwin + darwin-aarch64
• no default (my preference given a flat structure):
  • darwin-x86_64 + darwin-x86_64

and do something like

-const platformDep = dependency[Deno.build.os];
+const platformDep = dependency[`${Deno.build.os}-${Deno.build.arch}`];

Secondarily, the other implication to consider in the surrounding code is that arm builds are still generally not as first-class. While it seems everything has x86 builds in place, even for misc dev versions, arm versions are more hit or miss. As such, the general codebase could potentially use a little love to make more informative where its hitting problems and potentially more aggressively halt. The existing code just tries to dl if it exists and regardless will print info(${dependency.name} complete.\n);

@dragonstyle any particular preferences or other interface designs come to mind?

[dragonstyle]

This is a great analysis- I personally prefer your initial suggestion over the flatter designs as it seems to me to allow better use of types to validate (e.g. valid architectures) and would make it more straightforward to provide helpful errors for cases like a missing architecture for a particular operating system (since the two are easily separable).

One other thought that occurs to me would be to extend the current PlatformDependency interface:

export interface PlatformDependency {
  filename: string;
  url: string;
  configure(path: string): Promise<void>;
}

to include support for an architecture (using the Deno.build.arch as a key per your suggestion):

export interface PlatformDependency {
  architecture: "x86_64" | "aarch64",
  filename: string;
  url: string;
  configure(path: string): Promise<void>;
}

which I suppose is really just an inverted version of your first suggestion - the dl resolution code would just be:

-const platformDep = dependency[Deno.build.os];
+const platformDep = dependency[Deno.build.os][Deno.build.arch];

I'm pretty happy with either of these - you have thoughts about this approach vs your first suggestion?

Definitely agree we should be doing a better job of failing + in the case that a dependency is missing the request architecture, failing with a helpful / clear message (and definitely not silently ignoring the case where a platform dependency is missing 👍 ).

[dpastoor]

Responding now so I don't leave things hanging. I'm traveling some this week so kinda picking away at stuff between things but its been a blast digging into deno/quarto.

I did a quick assessment of docker, go, and rust around how they manage such builds. It seems to me that the target, at minimum, will be "all platforms rstudio would run on" as first class, and others that we can shoehorn in would be great as well. The tl;dr (longer write up incoming tonight when I'm back at the computer/notes) is that each language/area seemed to have similar but not-consistent patterns for naming or managing.

In addition, this is further conflated by some of the additional deps that will need to bundle/link in some way. esbuild is in really good shape to basically cover across the board without too many quirks, but pandoc is going to cause some grief. In particular, at least the homebrew arm builds have osx12 and osx11.6 as separate targets. I could not find any evidence as to why but its there.

One thing that came to mind was is there a way to leverage (whatever) rstudio is already doing to build/bundle pandoc for rstudio. I'm not sure if there is some s3 endpoint that has a bunch of binary assets that could be leveraged already (or adjusted/extended for quarto-dev to lean on). If not, could potentially mirror the homebrew builds for pandoc instead.

With that said, thinking back to the platform dep interface, using pandoc as an example, we'd likely need to extend PlatformDependency even further to optionally have like os version. Similarly on Linux might need different deb/rhel/Ubuntu/etc). As such, I've strongly warmed to PlatformDependency as the place to extend.

Then perhaps the Dependency interface could be some array(s) of PlatformDependencies. Given it will be so small storing in an object vs an array that is appended-to/filtered on wouldn't have any performance implication.

Let me collect more specific examples from my notes tonight in a follow up comment and hopefully that will make it easier to stress test the potential interface/code adjustments.

As a final thought/question, as I've started to familiarize with the code base a bit more, I was trying to establish where the boundaries for data fetching/design for a comprehensive "get everything needed for all platforms to be able to build all the bundles" vs "I'm on a specific platform and need the associated deps to get going". Do you have any brief summary of any thoughts on if there were any conscious decisions around how to approach that, or whether the codebase just evolved naturally by specific need over time? This could also help with the Dependency/PlatformDependency boundary considerations.

Thanks!

[dragonstyle]

As a final thought/question, as I've started to familiarize with the code base a bit more, I was trying to establish where the boundaries for data fetching/design for a comprehensive "get everything needed for all platforms to be able to build all the bundles" vs "I'm on a specific platform and need the associated deps to get going". Do you have any brief summary of any thoughts on if there were any conscious decisions around how to approach that, or whether the codebase just evolved naturally by specific need over time? This could also help with the Dependency/PlatformDependency boundary considerations.

I'm doing a little work in this area right now for an unrelated item, so I may make a few changes around PlatformDependency in anticipation of this... But as some general background as you're thinking about this, we currently break down the process of building an installer into three basic steps:

1. Configure the machine
   This configures the machine building the installer just as a developer will, primarily moving binary dependencies into place in dist/bin. This is entirely based upon the environment in which the command is being run (e.g. the OS / architecture of the machine being configured).

2. Prepare distribution (run the package/src/quarto-bld prepare-dist command)
   This does other work in the dist directory (creating share, version files, and so), basically putting everything in place that we'd like to distribute. This is largely platform independent as it it move resources around for the most part.

3. Build the installer based upon the contents of the list directory. This is platform dependent b/c of tooling required to produce the installers.

None of this really contemplated different architectures, so I'm not sure the best way to layer it through the process. Today, we're definitely assuming that the machine building the installer is the of the OS and architecture of the installer being produced (mostly because of that initial configure step and the fact that we subsequently use the locally configured deno environment to run steps 2 and 3). We'll have a little untangling to do I think (particularly of that configure step), but it shouldn't be so bad IMO.

I'll post any updates that occur as I work in that area here!

[dragonstyle]

Related to some other work around download archiving, I made some changes to fail more quickly when DLs don't succeed as a part of configure (there is still more work to do in this area, but it is forward progress). See f61b7c3 .

I also took a stab at improving the dependency architecture awareness per the above discussion. Basically ended up very close to original proposal:

// Defines a binary dependency for Quarto
export interface Dependency {
  name: string;
  bucket: string;
  version: string;
  architectureDependencies: Record<
    string,
    ArchitectureDependency
  >;
}

// Defines the specific Platform dependencies for
// a given architecture
export interface ArchitectureDependency {
  "darwin": PlatformDependency;
  "linux": PlatformDependency;
  "windows": PlatformDependency;
}

// Defines an individual binary dependency, specific
// to a Platform (and architecture)
export interface PlatformDependency {
  filename: string;
  url: string;
  configure(path: string): Promise<void>;
}

This was nice because it mean that I could make the platform dependencies non optional, so now to provide the dependency for a new architecture, you must be able to full all the platform dependencies.

Ultimately would be nice to do

  architectureDependencies: Record<
    "x86_64" | "aarch64",
    ArchitectureDependency
  >;

But this creates the expansion joint at the minimum, hopefully.

[dpastoor]

cool deal - I just pulled those changes into the PR #242 as well.

Do you by chance have a list of binary deps already present/built? In sluething around some more it seems like at least for arm macs homebrew has the most complete builds that we could pull and archive. Similarly there are some other repos that are building binaries - for example - https://github.com/LukeChannings/deno-arm64

[dpastoor]

ok install notes:

Running off most recent changes in #237 focused on esbuild, that also pulls in your mentioned changes, unsurprisingly it does blow up, but is clearly looking for the correct slug once the values get populated:

******************************************
Configuring local machine for development:
 - OS  : darwin
 - Arch: aarch64
 - Cwd : /Users/devin/repos/dpastoor/quarto-cli/package/src

******************************************

Preparing esbuild javscript bundler
Downloading esbuild javscript bundler
Downloading https://s3.amazonaws.com/rstudio-buildtools/quarto/esbuild/0.12.10/esbuild-darwin-arm64
to /Users/devin/repos/dpastoor/quarto-cli/package/dist/bin/esbuild-darwin-arm64
ERROR: Error: Failed to Download esbuild javscript bundler
Are you sure that version 0.12.10 of esbuild has been archived using './quarto-bld archive-bin-deps'?
Forbidden
    at configure (file:///Users/devin/repos/dpastoor/quarto-cli/package/src/common/configure.ts:56:15)
    at async Command.fn (file:///Users/devin/repos/dpastoor/quarto-cli/package/src/cmd/pkg-cmd.ts:37:5)
    at async Command.execute (https://deno.land/x/cliffy@v0.20.1/command/command.ts:1014:7)
    at async Command.parse (https://deno.land/x/cliffy@v0.20.1/command/command.ts:928:16)
    at async quartoBld (file:///Users/devin/repos/dpastoor/quarto-cli/package/src/bld.ts:53:3)
    at async file:///Users/devin/repos/dpastoor/quarto-cli/package/src/bld.ts:61:5

[dragonstyle]

OK this is actually a good thing (it shows all appears to be working from previous PR, though I think outputting the architecture in the error would be helpful too). We used to pull these dependencies directly from the source URLs, but we added an intermediary step of archiving them to internal S3 buckets (using a separate ./quarto-bld archive-bin-deps command implemented in archive-binary-dependencies.ts which we have to run internally on a machine with proper authorization to write to the archive buckets). Since you're introducing a dependency that has never been archived in that way, configure is complaining when we attempt to download the dependency.

I'm away from office, but tomorrow I can try archiving from this branch (which should test the the archiving of differencing architectures and make the binary available for that PR).

[dragonstyle]

Do you by chance have a list of binary deps already present/built?

Sorry, I don't (outside of the existing architecture)...