-
Notifications
You must be signed in to change notification settings - Fork 177
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
Update install script #2265
Conversation
132131d
to
ece7827
Compare
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 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? |
Other testing scope:
And in terms of features:
It would also be helpful to have context on how this was tested (did you set |
ece7827
to
09b8c81
Compare
There was a problem hiding this 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;
That's already fixed in the latest commit. |
There was a problem hiding this 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
There was a problem hiding this 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
There was a problem hiding this 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!
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:get_architecture
function fromrustup-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.-h
or--help
is passed, prints the help text of the installer without having to download and execute it.chmod +x
doesn't actually set the executable bit and give an error message (fromrustup-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-testedTesting