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

Conversation

@shana
Copy link
Member

@shana shana commented Aug 15, 2017

Fixes #192

On non-windows, git and other files might be symlinks, so when we're using them as a base for determining parent directories, we may need to resolve the symlink first. The rules for automatically
doing symlink resolution are tricky, so for now just do it manually.

Still not happy with how we're finding git on the system, but this should at least make the command line work across all systems.

An existing problem right now is that OSX overwrites values set to the PATH env variable when it loads /etc/profile in preparation to run bash, which is seriously screwed up on their part. That means that the opened command line won't have our path variable value (but it will have all the other env vars). I'm still investigating how to fix that... This is fixed now by creating a little shell script that in turn invokes bash with a proper environment.

FYI, we want the GitExecutablePath to reflect where the binary was found, and the GitInstallPath to reflect where it is installed. When the binary is a symlink (like /usr/local/bin/git -> ../Cellar/git/2.12.2/bin/git), this means GitExecutablePath will have /usr/local/bin/git and GitInstallPath will have /usr/local/Cellar/git/2.12.2/bin

StanleyGoldman and others added 5 commits August 14, 2017 18:42
Fixes #192

On non-windows, git and other files might be symlinks, so when
we're using them as a base for determining parent directories,
we may need to resolve the symlink first. The rules for automatically
doing symlink resolution are tricky, so for now just do it manually.
We want the GitExecutablePath to reflect where the binary was found,
and the GitInstallPath to reflect where it is installed. When the
binary is a symlink (like /usr/local/bin/git -> ../Cellar/git/2.12.2/bin/git),
this means GitExecutablePath will have /usr/local/bin/git and
GitInstallPath will have /usr/local/Cellar/git/2.12.2/bin

public bool DirectoryExists(NPath append)
{
if (append == null)
Copy link
Contributor

Choose a reason for hiding this comment

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

If this method was sent null before, what was the result?

Copy link
Member Author

Choose a reason for hiding this comment

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

Combine doesn't care if it's null so it wouldn't blow up, but it's a waste of resources creating new instances of the same thing over and over again, Combine always creates a new thing.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I follow now

if (path == null || DefaultEnvironment.OnWindows || path.IsRelative || !path.FileExists())
return path;

return new NPath(Mono.Unix.UnixPath.GetCompleteRealPath(path.ToString()));
Copy link
Contributor

Choose a reason for hiding this comment

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

GetCompleteRealPath is a method that will get the real link from a symlink?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, from Mono.Posix

// osx terminal app doesn't inherit the PATH env var and there's no way to pass it in
var envVarFile = environment.FileSystem.GetRandomFileName();
environment.FileSystem.WriteAllLines(envVarFile, new string[] { "cd $GHU_WORKINGDIR", "PATH=$GHU_FULLPATH:$PATH /bin/bash" });
Mono.Unix.Native.Syscall.chmod(envVarFile, (Mono.Unix.Native.FilePermissions)493); // -rwxr-xr-x mode (0755)
Copy link
Contributor

Choose a reason for hiding this comment

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

🎖

Copy link
Member Author

Choose a reason for hiding this comment

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

magic

StanleyGoldman
StanleyGoldman previously approved these changes Aug 15, 2017
@shana shana merged commit 4338edd into master Aug 15, 2017
@shana shana deleted the fixes/192-wrong-environment-mac branch August 15, 2017 17:52
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.

Push/pull/publish randomly failing on non-windows systems

3 participants