ssh: explicitly set ~ for home directory unless using cmd.exe #1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
deansheather
approved these changes
Mar 10, 2025
| @@ -4,6 +4,7 @@ import ( | |||
| "bytes" | |||
| "errors" | |||
| "fmt" | |||
| "github.com/mutagen-io/mutagen/pkg/prompting" | |||
Comment on lines
+171
to
+187
| pathSeparator := "/" | ||
| pathComponents := []string{filesystem.HomeDirectorySpecial} | ||
| if cmdExe { | ||
| pathSeparator = "\\" | ||
| pathComponents = nil | ||
| } | ||
| dataDirectoryName := filesystem.MutagenDataDirectoryName | ||
| if mutagen.DevelopmentModeEnabled { | ||
| dataDirectoryName = filesystem.MutagenDataDirectoryDevelopmentName | ||
| } | ||
| pathComponents = append(pathComponents, | ||
| dataDirectoryName, | ||
| filesystem.MutagenAgentsDirectoryName, | ||
| mutagen.Version, | ||
| BaseName, | ||
| ) | ||
| return strings.Join(pathComponents, pathSeparator) |
There was a problem hiding this comment.
Most of this could probably be moved into (another) new function like remoteHomePath(cmdExe bool, components ...string) so it could be used from both here and install.go
| // windowsPowershellCommandNotFoundFragment is a fragment of the error output | ||
| // returned on Windows systems running Powershell when a command cannot be | ||
| // found. | ||
| windowsPowershellCommandNotFoundFragment = "is not recognized as the name of a cmdlet, function, script file, or operable program." |
There was a problem hiding this comment.
This seems like it would break if you have a different OS language, but I guess mutagen was already doing similar error string checks previously and I can't really think of a better way to accomplish this. So it's probably fine for now
ethanndickson
approved these changes
Mar 11, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes coder/coder#16568
This PR uses an explicit
~in SSH paths to ensure that the mutagen agent is installed in the home directory, even when the SSH default directory is some other directory, as is the case for some Coder workspaces.Note that this breaks the Docker transport on Linux, since the Docker transport also sets the explicit home directory. If we ever wanted to upstream this, we would need a better fix, but for Coder's purposes we are only interested in SSH-based file sync.
The problem is that while mutagen defines a
Transportinterface, the abstraction is very leaky: in the case of Docker, mutagen actively probes the endpoint to figure out the home directory, and then adds it under the covers. In the case of SSH, mutagen assumes (incorrectly in Coder's case) that the default directory is the home directory. The implementations also differ in how they handle different operating systems, with Docker using probing, while SSH depends on the calling function to retry with different requests that might succeed on different operating systems.For now, I just don't think it's worth making a proper fix that refactors the use of the
Transportinterface, given that we don't care about Docker support. I raised an issue on the main mutagen repo asking if they would consider an PR and got no answer. While the repository is still getting some maintenance, it mainly consists of dependency updates. I don't think the team is able to devote time to considering a larger fix for us.