Skip to content
This repository was archived by the owner on Feb 8, 2024. It is now read-only.

Make bundled tsh available outside of Connect#1445

Merged
ravicious merged 17 commits into
masterfrom
ravicious/link-tsh
Jan 4, 2023
Merged

Make bundled tsh available outside of Connect#1445
ravicious merged 17 commits into
masterfrom
ravicious/link-tsh

Conversation

@ravicious
Copy link
Copy Markdown
Member

@ravicious ravicious commented Dec 20, 2022

This PR implements RFD 99 (gravitational/teleport#19284).

TODO:

I'm yet to add macOS support but I figured I'll submit the other changes for review already and then submit the macOS ones as another PR to this PR. macOS support will involve more changes in the application code.

So far there wasn't any substantial changes compared to the RFD but I'll add comments under some of the minor differences.

On Windows I ended up just adding a path to an env var, as I mentioned in the RFD. On Linux, I had to copy the install/uninstall scripts as there's no way to extend the default ones.

Comment on lines +39 to +42
# Is TSH_SYMLINK_TARGET a link that points at a file which doesn't exist?
if [ -L "$TSH_SYMLINK_TARGET" ] && [ ! -e "$TSH_SYMLINK_TARGET" ]; then
rm -f "$TSH_SYMLINK_TARGET"
fi
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The RFD says:

If Connect decides not to create or remove the symlink, it should echo a message explaining the reason.

Here I've decided to not echo a message if Connect doesn't remove the symlink. Idk, it doesn't seem like a necessary thing to me and we already emit a message regarding symlinks during installation.

I'm not even sure if I should emit a message during an upgrade. But I wanted to be able to tell what happened if someone sends logs from an upgrade. OTOH I don't think Linux packages echo stuff from within those scripts?

Comment on lines +54 to +57
OLD_SYMLINK=/usr/bin/teleport-connect
if [ -e "$OLD_SYMLINK" ]; then
rm $OLD_SYMLINK
fi
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I changed the location from /usr/bin to /usr/local/bin since this is where our teleport package places the binaries as well.

Comment on lines +24 to +25
# Create $BIN if it doesn't exist.
[ ! -d "$BIN" ] && mkdir -p "$BIN"
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Here I'm just following what tsh.pkg does already.

Comment thread packages/teleterm/build_resources/linux/after-install.tpl Outdated
@gzdunek
Copy link
Copy Markdown
Contributor

gzdunek commented Dec 21, 2022

I have no experience with scripts, but the code looks pro :)
Tomorrow I'd like to test the app on Linux/Windows.

@avatus
Copy link
Copy Markdown
Contributor

avatus commented Dec 21, 2022

This looks great! I'm excited for this.
Could you make a tag build so we can all individual test it on a bunch of different platforms? Thank you!

@ravicious
Copy link
Copy Markdown
Member Author

I just promoted 12.0.0-dev.ravicious.1.

@gzdunek
Copy link
Copy Markdown
Contributor

gzdunek commented Dec 22, 2022

Just to make sure: should I see the following message if I remove Connect on Fedora?

[parallels@fedora code]$ tsh
bash: /usr/local/bin/tsh: No such file or directory

I tested the app also on Ubuntu and Windows, everything works fine!

@ravicious
Copy link
Copy Markdown
Member Author

Just to make sure: should I see the following message if I remove Connect on Fedora?

I think that's correct because the shell within which you removed tsh still had tsh resolved to /usr/local/bin/tsh but now that file is gone. If you open a new shell and try to run tsh it should give you an error along the lines of "command not found".

@gzdunek
Copy link
Copy Markdown
Contributor

gzdunek commented Dec 22, 2022

Just to make sure: should I see the following message if I remove Connect on Fedora?

I think that's correct because the shell within which you removed tsh still had tsh resolved to /usr/local/bin/tsh but now that file is gone. If you open a new shell and try to run tsh it should give you an error along the lines of "command not found".

Ah, right

@avatus
Copy link
Copy Markdown
Contributor

avatus commented Dec 22, 2022

Working great on win 10/11

@ravicious ravicious enabled auto-merge (squash) January 4, 2023 16:31
@ravicious ravicious merged commit ca8b65c into master Jan 4, 2023
@ravicious ravicious deleted the ravicious/link-tsh branch January 5, 2023 10:44
ravicious added a commit that referenced this pull request Jan 5, 2023
* Rename assets to build_resources

* Add resources\bin to Path during installation on Windows

* Adjust docs related to USE_SYSTEM_FPM

It turns out you need that for deb packages too.

* Create symlink to bundled tsh on Linux targets

* after-install: Get rid of old symlink removal

* Expand story for QuickInput

* Make command suggestions stay in place

* Align suggestion icons to the top rather than center

This makes it easier to tell when one suggestion ends and another starts.

* Add install & uninstall cmds to command bar

* Exclude new commands from OSes other than macOS

* Implement commands for symlinking tsh
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants