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

Update install script #2265

Merged
merged 7 commits into from
Feb 21, 2025
Merged

Update install script #2265

merged 7 commits into from
Feb 21, 2025

Conversation

coolreader18
Copy link
Collaborator

@coolreader18 coolreader18 commented Feb 14, 2025

Description of Changes

Based on #2263, might make more sense to combine them as one PR.

This has a couple changes beyond just updating for the new spacetimedb-update functionality, mostly taken from looking at how rustup-init.sh does things:

  • Uses the get_architecture function from rustup-init.sh, since it's existing well-tested code that gives us the rust/llvm target tuple, and platform detection code is horrible to write.
  • checks for flags passed to the installer:
    • if -h or --help is passed, prints the help text of the installer without having to download and execute it.
  • If the download fails, check whether it was because of a 404 and give a good error message ("It seems like we may not have prebuilt binaries for your platform.")
  • check for the case chmod +x doesn't actually set the executable bit and give an error message (from rustup-init.sh)

Expected complexity level and risk

2: lotta shell code, but a lot was just taken from rustup's rustup-init.sh, which is already well-tested

Testing

  • Tested that spacetime-install.sh works in a fresh docker image
  • Need to test on windows.

@coolreader18 coolreader18 changed the title Noa/installscript Update install script Feb 14, 2025
@coolreader18 coolreader18 linked an issue Feb 19, 2025 that may be closed by this pull request
@coolreader18 coolreader18 force-pushed the noa/installscript branch 2 times, most recently from 132131d to ece7827 Compare February 19, 2025 18:31
@gefjon gefjon requested a review from bfops February 19, 2025 18:34
@bfops
Copy link
Collaborator

bfops commented Feb 19, 2025

I think I had imagined that the scripts would be basically exactly what we have right now, plus an extra unzip/untar step, and then running spacetimedb-update to finish the installation. (And then adding an extra flag for non-interactive mode, ideally in a separate PR)

The Linux script especially seems substantially changed (110 lines to 484 lines) - what's the reason behind that? I mean I guess if it works, it works, but I definitely don't feel equipped to review all of the new thing in detail.

Also, it looks like this hasn't been tested on Windows, is that right?

@bfops
Copy link
Collaborator

bfops commented Feb 19, 2025

Other testing scope:

  • MacOS (linux + homebrew)
  • Linux (non-docker)
  • Windows (Powershell)
  • Windows (Linux-like shell)

And in terms of features:

  • Running both interactively and non-interactively

It would also be helpful to have context on how this was tested (did you set SPACETIME_DOWNLOAD_ROOT to a local directory?)

@jdetter jdetter self-requested a review February 20, 2025 18:33
Copy link
Collaborator

@jdetter jdetter left a comment

Choose a reason for hiding this comment

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

I hosted this locally and tried the install script and it seems to somewhat work on Windows but there's a bug in the install script and also I had to manually add the binary directory to my PATH in order for SpacetimeDB to work. Can you bump this back to me when you fix it?

PS C:\Users\Boppy> iwr http://192.168.2.100/spacetime-install.ps1 -useb | iex
Downloading installer...
Our EULA for spacetimedb-cli can be found here: <https://spacetimedb.com/eula>
Agree to the EULA? yes
The SpacetimeDB command line tool will now be installed into C:\Users\Boppy\AppData\Local\SpacetimeDB
Would you like to continue? yes
Downloading latest version...
  Installing v1.0.0: done!
The `spacetime` command has been installed as C:\Users\Boppy\AppData\Local\SpacetimeDB\spacetime.exe
Note: we recommend making sure that this executable is in your PATH variable.

The install process is complete; check out our quickstart guide to get started!
        <https://spacetimedb.com/docs/quick-start>
iex : Method invocation failed because [System.Environment] does not contain a method named 'GetSpecialFolder'.
At line:1 char:56
+ iwr http://192.168.2.100/spacetime-install.ps1 -useb | iex
+                                                        ~~~
    + CategoryInfo          : InvalidOperation: (:) [Invoke-Expression], RuntimeException
    + FullyQualifiedErrorId : MethodNotFound,Microsoft.PowerShell.Commands.InvokeExpressionCommand

PS C:\Users\Boppy> spacetime --version
spacetime Path: C:\Users\Boppy\AppData\Local\SpacetimeDB\bin\current\spacetimedb-cli.exe
Commit: acf1715efceca1889396a4820e39ec11cad9bbe0
spacetimedb tool version 1.0.0; spacetimedb-lib version 1.0.0;

@coolreader18
Copy link
Collaborator Author

That's already fixed in the latest commit.

Copy link
Collaborator

@jdetter jdetter left a comment

Choose a reason for hiding this comment

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

I fixed a small debug issue - still testing

Copy link
Collaborator

@jdetter jdetter left a comment

Choose a reason for hiding this comment

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

(sorry ignore this, user error)

I am getting a new error unfortunately:

PS C:\Users\Boppy> iwr http://192.168.2.100/spacetime-install.ps1 -useb | iex
Downloading installer...
Invoke-WebRequest : The request was aborted: The connection was closed unexpectedly.
At line:24 char:5
+     Invoke-WebRequest $DownloadUrl -OutFile $Executable -UseBasicPars ...
+     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : InvalidOperation: (System.Net.HttpWebRequest:HttpWebRequest) [Invoke-WebRequest], WebExc
   eption
    + FullyQualifiedErrorId : WebCmdletWebResponseException,Microsoft.PowerShell.Commands.InvokeWebRequestCommand

Copy link
Collaborator

@jdetter jdetter left a comment

Choose a reason for hiding this comment

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

I have manually tested this on:

  • x86_64 Windows
  • x64_64 Linux (not in docker)

In both cases the scripts work just fine. Thanks @coolreader18!

@coolreader18 coolreader18 added this pull request to the merge queue Feb 21, 2025
Merged via the queue into master with commit 27a2afa Feb 21, 2025
13 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Install scripts should support non-interactive mode
3 participants