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 dist configuration for generating air binaries #67

Merged
merged 5 commits into from
Nov 27, 2024

Conversation

DavisVaughan
Copy link
Collaborator

@DavisVaughan DavisVaughan commented Nov 26, 2024

This PR adds cli binary generation through cargo dist. It looks like the artifacts successfully built and uploaded in the test run in the PR. We are going to have to merge and do a "real" tag to be able to test out the installer script, because it always points at a github release https://github.com/posit-dev/air/actions/runs/12039244616/artifacts/2241394657

Run this to install dist

curl --proto '=https' --tlsv1.2 -LsSf https://github.com/axodotdev/cargo-dist/releases/latest/download/cargo-dist-installer.sh | sh

Then you should be able to run dist build, which will build a tar.gz of the air cli for your machine (ARM mac) along with a shell installer script. It puts it in target/distrib. You won't be able to run the installer script, it points to a github release. It's just a way to look at it.

The number 1 thing to remember is that if you change something in dist-workspace.toml, you MUST rerun dist generate to regenerate the updated release.yml.

I'll leave more notes inline


Here's the general gameplan:

  • On each release, we are supposed to create a tag (we can probably make a github workflow for bumping this too)

    git tag v0.1.0
    git push --tags
    
  • That triggers release.yml (ive manually triggered it with pr-run-mode = "upload" to test here)

  • The release.yml builds for all targets, creates a GitHub Release for you with that tag, and uploads all the results and shell / powershell installer scripts

  • We advertise that you install with something like (for unix)

    curl --proto '=https' --tlsv1.2 -LsSf https://github.com/posit-dev/air/releases/latest/download/air-installer.sh | sh

    Note this pulls the latest release, but you can also point to a specific version if you want. ruff has their own mirror of their installer scripts at a shorter url, we may want to do the same at some point, it's kinda nice.

  • You should be able to run air at the command line


For Positron itself, if we bundle an air binary then the process will be similar to ark. We look for the github release with the air binary aligning with the OS and air version number we specify in a positron package.json and download it.

Comment on lines 41 to 45
on:
pull_request:
push:
tags:
- '**[0-9]+.[0-9]+.[0-9]+*'
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

pull_request: is added by "upload", with "skip" it just uses tags

Comment on lines +250 to +271
# Create a GitHub Release while uploading all files to it
- name: "Download GitHub Artifacts"
uses: actions/download-artifact@v4
with:
pattern: artifacts-*
path: artifacts
merge-multiple: true
- name: Cleanup
run: |
# Remove the granular manifests
rm -f artifacts/*-dist-manifest.json
- name: Create GitHub Release
env:
PRERELEASE_FLAG: "${{ fromJson(steps.host.outputs.manifest).announcement_is_prerelease && '--prerelease' || '' }}"
ANNOUNCEMENT_TITLE: "${{ fromJson(steps.host.outputs.manifest).announcement_title }}"
ANNOUNCEMENT_BODY: "${{ fromJson(steps.host.outputs.manifest).announcement_github_body }}"
RELEASE_COMMIT: "${{ github.sha }}"
run: |
# Write and read notes from a file to avoid quoting breaking things
echo "$ANNOUNCEMENT_BODY" > $RUNNER_TEMP/notes.txt

gh release create "${{ needs.plan.outputs.tag }}" --target "$RELEASE_COMMIT" $PRERELEASE_FLAG --title "$ANNOUNCEMENT_TITLE" --notes-file "$RUNNER_TEMP/notes.txt" artifacts/*
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Note how they create a gh release for us using the tag name

Comment on lines +12 to +13
# The installers to generate for each app
installers = ["shell", "powershell"]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So we get .sh and .ps installer scripts

Comment on lines +18 to +19
# Target platforms to build apps for (Rust target-triple syntax)
targets = ["aarch64-apple-darwin", "x86_64-apple-darwin", "x86_64-unknown-linux-gnu", "x86_64-pc-windows-msvc"]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Comment on lines +24 to +25
# Path that installers should place binaries in
install-path = ["$XDG_BIN_HOME/", "$XDG_DATA_HOME/../bin", "~/.local/bin"]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is us mimicking ruff astral-sh/ruff#14457 (this is also a new thing for them, they are trying this out)

Oddly the default install path (used by the shell and powershell installers) is CARGO_HOME! But this isn't a cargo app, it's an lsp/formatter cli. It doesn't belong there.
https://opensource.axo.dev/cargo-dist/book/reference/config.html#install-path

Copy link
Collaborator

Choose a reason for hiding this comment

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

$CARGO_HOME/bin is not necessarily for cargo apps, it's for rust projects that you've installed with cargo install.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It looks like on maOS this will use ~/.local/bin. I don't have this folder and it isn't on my PATH. Where did ruff take this? Is it a convention?

On macOS I would expect it to go in /usr/local/bin.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We did a little research and $HOME/.local/bin is part of a freedesktop standard, along with the $XDG bits
https://specifications.freedesktop.org/basedir-spec/latest/


On Window you'll get a message telling you to "restart your system"

PS D:\Users\davis-vaughan> powershell -c "irm https://astral.sh/ruff/install.ps1 | iex"                                 
Downloading ruff 0.8.0 (x86_64-pc-windows-msvc)                                                                         
Installing to D:\Users\davis-vaughan\.local\bin
  ruff.exe
everything's installed!

To add D:\Users\davis-vaughan\.local\bin to your PATH, either restart your system or run:

    set Path=D:\Users\davis-vaughan\.local\bin;%Path%   (cmd)
    $env:Path = "D:\Users\davis-vaughan\.local\bin;$env:Path"   (powershell)

You really do need to restart your computer (or the explorer.exe process) for the changes to the Path to come into play. explorer.exe is the parent of all cmd.exe processes, so until that is restarted, no registry Environment modifications can come into affect.

The install script tweaks the Path field of the HKCU:\Environment part of the registry (which seems right, its for the current user). You'll see right away with Get-Item -Path "HKCU:\Environment" in powershell that \.local\bin is in there now, but the restart is what propagates it to new shells.

Note that updating the Path here is nice, it means that both cmd.exe and powershells will have air on the Path.

This is also just a 1 time restart that is required. Next time the user updates (or if theyve installed ruff before air), the Path will already be up to date, so it should just work without a restart.


On macOS, it's basically the same as above but you only need to restart your shell, not your whole system. The message on mac says either restart your _shell_ or run, which is nice.

@DavisVaughan DavisVaughan changed the title Add dist configuration Add dist configuration for generating air binaries Nov 26, 2024
@DavisVaughan DavisVaughan mentioned this pull request Nov 25, 2024
10 tasks
Comment on lines +24 to +25
# Path that installers should place binaries in
install-path = ["$XDG_BIN_HOME/", "$XDG_DATA_HOME/../bin", "~/.local/bin"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

$CARGO_HOME/bin is not necessarily for cargo apps, it's for rust projects that you've installed with cargo install.

Comment on lines +24 to +25
# Path that installers should place binaries in
install-path = ["$XDG_BIN_HOME/", "$XDG_DATA_HOME/../bin", "~/.local/bin"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

It looks like on maOS this will use ~/.local/bin. I don't have this folder and it isn't on my PATH. Where did ruff take this? Is it a convention?

On macOS I would expect it to go in /usr/local/bin.

@DavisVaughan DavisVaughan merged commit 47fc94c into main Nov 27, 2024
@DavisVaughan DavisVaughan deleted the feature/cargo-dist branch November 27, 2024 14:24
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.

2 participants