-
Notifications
You must be signed in to change notification settings - Fork 612
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
getgit: flag for installation without modifying the environment #548
base: main
Are you sure you want to change the base?
Conversation
passing -e has the following effects: 1. `/etc/profile.d` is not processed. 2. The context menu scripts `ctxmenu.bat` and `rm-ctxmenu.bat` are not created. 3. `/cmd` is not added to the path, programs in `/cmd` are installed to `[/usr]/bin`. Signed-off-by: Luna Saphie Mittelbach <[email protected]>
20e1727
to
6cff4a3
Compare
@@ -274,4 +284,4 @@ fi | |||
echo "Git is now available! If you want to add \"Git GUI Here\" and \"Git Bash" |
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.
These messsges should probably be modified if the context menu scripts aren't created.
@@ -13,6 +13,7 @@ function usage(){ | |||
echo " -k keep all downloaded, extracted files in \"/tmp\"" | |||
echo " -s skip downloading" | |||
echo " -f forcibly update no matter what version it is" | |||
echo " -e don't modify the environment (no /cmd in path, /etc/profile.d)" |
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.
"no /cmd in path" sounds like that part is only about not modifying $PATH
, whereas the code doesn't create /cmd
at all.
process "/cmd" | ||
process -f "/etc/profile.d" | ||
else | ||
cp -f $gitupk/cmd/* $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.
Does it make sense to copy these files to /usr/bin
or /bin
? Wouldn't they break MSYS2/Cygwin git
? And wouldn't they be broken in /usr/bin
anyways, because they expect ../mingw64/bin/git.exe
etc to exist?
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.
As far as I understand the various tools only care about finding each other in $PATH, so it works in /cmd, /usr/bin, etc.
I think it already breaks msys2/cygwin git due to copying files into /lib, but I'm not sure.
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.
Most files in /cmd
are compiled from git-wrapper.c
and can't rely on PATH
to define PATH
.
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 think it already breaks msys2/cygwin git due to copying files into /lib, but I'm not sure.
process
(without -f
) should backup and restore those files, so it's probably the corresponding Git for Windows parts that get broken by getgit
.
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.
Ah. Would the PR be acceptable if /cmd is left as is (and not copied to somewhere else) i.e. only skips /etc/profile.d, the registry PATH setting, and the context menu scripts? I can add a message informing the user they need to add /cmd to path themselves.
passing
-e
has the following effects:/etc/profile.d
is not processed.ctxmenu.bat
andrm-ctxmenu.bat
are not created./cmd
is not added to the path, programs in/cmd
are installed to[/usr]/bin
instead