microsoft / git

A fork of Git containing Microsoft-specific patches.
http://git-scm.com/
Other
767 stars 92 forks source link

"Permission denied" while trying to install on Windows #449

Open dscho opened 3 years ago

dscho commented 3 years ago

Operating System: Windows

OS Version: Microsoft Windows [Version 10.0.19043.1288]

Description

Describe the bug: what happened? why would you consider it broken?

Ran the Windows git installer with git 2.32.0.vfs.0.2 already installed. During the install, hit [...]:

Unable to set system config "init.defaultBranch":="master": exit code 4
stderr:
error: could not write config file C:/Program Files/Git/etc/gitconfig: Permission denied

Unclear why there would be a permissions error here -- the config file does exist and is not read-only. Like the rest of the program files directory, only admins can write to it but the installer was elevated.

Originally posted by @marcelais in https://github.com/microsoft/git/discussions/441#discussioncomment-1479897

dscho commented 3 years ago

@marcelais could I bother you to clone https://github.com/git-for-windows/build-extra/ and then compile the cmd-test.iss I posted in https://github.com/git-for-windows/git/issues/2384#issuecomment-551555244 via ./installer/InnoSetup/ISCC.exe -o. cmd-test.iss, then run the generated installer? We'll probably need to modify that .iss file quite a bit in order to figure things out, but the first step is of course that you can replicate the issue...

marcelais commented 3 years ago

Sure... let me give that a try.

marcelais commented 3 years ago

First it prints a message box with a git command followed by "Could set the system config correctly."

dscho commented 2 years ago

Okay, next try. This cmd-test.iss is supposed to live in the installer/ subdirectory of a build-extra clone, as it uses some .iss files directly:

#define APP_NAME      'Git'

[Setup]
AppName=CmdTest
AppVersion=0.0
WizardStyle=modern
DefaultDirName=C:\My Program
PrivilegesRequired=none

#define MINGW_BITNESS 'mingw64'

[Files]
Source: "cmd-test.iss"; DestDir: "{app}"

[Code]
#include "helpers.inc.iss"
#include "modules.inc.iss"
#include "exec-with-capture.inc.iss"

var
    AppDir:String;

function ShellQuote(Value:String):String;
begin
    // Sadly, we cannot use the '\'' trick used throughout Git's
    // source code, as InnoSetup quotes those in a way that
    // git.exe does not understand them.
    //
    // So we try to imitate quote_arg_msvc() in Git's
    // compat/mingw.c instead: \ => \\, followed by " => \",
    // then surround with double quotes.
    StringChangeEx(Value,#92,#92+#92,True);
    StringChangeEx(Value,#34,#92+#34,True);
    Result:=#34+Value+#34;
end;

function GitSystemConfigSet(Key,Value:String):Boolean;
var
    ExitCode:DWORD;
    StdOut,StdErr:String;
begin
    if (Value=#0) then begin
        if ExecWithCapture('"'+AppDir+'\{#MINGW_BITNESS}\bin\git.exe" config --system --unset '+Key,StdOut,StdErr,ExitCode) And ((ExitCode=0) Or (ExitCode=5)) then
            // exit code 5 means it was already unset, so that's okay
            Result:=True
        else begin
            LogError('Unable to unset system config "'+Key+'": exit code '+IntToStr(ExitCode)+#13+#10+StdOut+#13+#10+'stderr:'+#13+#10+StdErr);
            Result:=False
        end
    end else if ExecWithCapture('"'+AppDir+'\{#MINGW_BITNESS}\bin\git.exe" config --system '+ShellQuote(Key)+' '+ShellQuote(Value),StdOut,StdErr,ExitCode) And (ExitCode=0) then
        Result:=True
    else begin
        LogError('Unable to set system config "'+Key+'":="'+Value+'": exit code '+IntToStr(ExitCode)+#13+#10+StdOut+#13+#10+'stderr:'+#13+#10+StdErr);
        Result:=False;
    end;
end;

function InitializeSetup(): Boolean;
begin
    AppDir:='C:\Program Files\Git';

    if GitSystemConfigSet('cmd.test','success') then
        MsgBox('Could set the system config correctly.',mbInformation,MB_OK)
    else
        MsgBox('Failed to set `cmdtest` in the system config.',mbError,MB_OK);
    Result:=False;
end;

I did manage to trigger an error message like you reported by misspelling the MINGW_BITNESS value (i.e. ensuring that the path to the git.exe file is invalid).

If this succeeds for you, then I have no idea what is going wrong, as that small "installer" should have replicated the issue.

If you can replicate the issue, it would be good to instrument the helper function(s), e.g. like this:

diff --git a/installer/exec-with-capture.inc.iss b/installer/exec-with-capture.inc.iss
index 2f38d6075..77057d970 100644
--- a/installer/exec-with-capture.inc.iss
+++ b/installer/exec-with-capture.inc.iss
@@ -160,7 +160,9 @@ begin
     StartupInfo.hStdOutput:=StdOutWriteHandle;
     StartupInfo.hStdError:=StdErrWriteHandle;

+LogError('launching "'+CommandLine+'"');
     if not CreateProcess(0,CommandLine,0,0,1,NORMAL_PRIORITY_CLASS,0,0,StartupInfo,ProcessInformation) then begin
+LogError('failed to create process '+IntToStr(GetLastError()));
         CloseHandle(StdOutReadHandle);
         CloseHandle(StdErrReadHandle);
         CloseHandle(StdOutWriteHandle);
@@ -173,6 +175,7 @@ begin
     CloseHandle(StdOutWriteHandle);
     CloseHandle(StdErrWriteHandle);

+LogError('reading stdout/stderr');
     BufferLength:=16384;
     Buffer:=StringOfChar('c',BufferLength);
     while (WaitForSingleObject(ProcessInformation.hProcess,50)=WAIT_TIMEOUT) do begin
marcelais commented 2 years ago

Sorry. I built that script and the generated installer also worked just fine. :-( I went to another dev machine that I have (which had git version 2.33.0.vfs.0.0 on it) and tried to repro with the installer I built above and by installing 2.33.0.vfs.0.2 and both succeeded without issue.

Sorry I'm not able to repro the original symptom for you again. If you can think of anything else for me to try, please let me know.

dscho commented 2 years ago

I guess this could have been due to some process (e.g. Defender) accessing the Git config at the exact wrong time.