-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Support linux install path from src #1409
Conversation
Update custom install to define a default path in `Packaging.Linux.csproj`. Include a few spelling/grammar/refactoring tweaks as well.
install from source: custom install location edits
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.
Small issue with adding the correct directory to the path that needs correcting.
Also, we may wish to consider changing "install location" something like "prefix" or "install prefix", since this is a piece of common terminology in the realm of build/configure scripts etc.
This would also help clarify that actually we're going to be using the provided value as a prefix for multiple paths, including $PREFIX/share/gcm-core
and $PREFIX/bin
.
$sudo_cmd env "PATH=$PATH" $DOTNET_ROOT/dotnet build ./src/linux/Packaging.Linux/Packaging.Linux.csproj -c Release -p:InstallFromSource=true | ||
add_to_PATH "/usr/local/bin" | ||
$sudo_cmd env "PATH=$PATH" $DOTNET_ROOT/dotnet build ./src/linux/Packaging.Linux/Packaging.Linux.csproj -c Release -p:InstallFromSource=true -p:InstallLocation=$installLocation | ||
add_to_PATH $installLocation |
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.
$installLocation
will be something like /usr/local
but we want to add /usr/local/bin
to the $PATH
.. we should therefore be using something like this instead here:
add_to_PATH "${installLocation}/bin"
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.
Good catch! I also agree with the "prefix" name change.
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.
@m4ss1m0g - I opened https://github.com/m4ss1m0g/git-credential-manager/pull/2 to address these comments. Once merged, I think we will actually be good to go here 😀.
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.
Thanks
Refactor references to "install location" to instead reference "install prefix," since we use this value as a prefix for multiple paths (rather than a static location).
Ensure the correct path for GCM is added to `PATH` at the end of the `install-from-source` script.
install from source: refactor install location and fix path
This PR fix #1304.
I found that the add func add_to_PATH doesn't persist the path to the $PATH env variable.
To persist in the current shell is necessary to use a dot or
source
see docWhen a script is run using source it runs within the existing shell, any variables created or modified by the script will remain available after the script completes. In contrast if the script is run just as filename, then a separate subshell (with a completely separate set of variables) would be spawned to run the script.
To persist the PATH among the reboot/new shell, it is necessary to update the .profile with
echo 'PATH="$PATH:$installPath' >>~/.profile
.I don't know if this is the correct behavior, Prior to this commit, it was working because the
/usr/local/bin
is already in the PATH.