-
Notifications
You must be signed in to change notification settings - Fork 296
Conversation
My initial plan was to try to get this working in order to debug failures in atom/atom#19189, but annoyingly specifying x86 as a platform still seems to use an x64 image...still nice to have Windows coverage though. |
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.
This looks reasonable to me, nice job as usual @50Wliu!
Are you concerned at all about introducing unexpected changes to apm on Windows? My understanding is that this pull request was opened just to run tests on AppVeyor, but we're changing production code as well. I am cool with it, but just thought I'd raise the question.
Either way, !
@50Wliu We have since moved from running tests on other platforms and we are fully embracing Github Actions. For that reason, I don't think this is relevant anymore. |
Functional changes: - Include the mingw directory in the PATH (this directory contains many of the actual git functions, such as `git-pull`, `git-push`, and `git-upload-pack`). - Search `env.ProgramW6432` for Git before `env.ProgramFiles`, as `ProgramFiles` is not guaranteed to always point to the x64 Program Files directory. Technically, we should get the mingw path from `git --exec-path` (and this is what the GitHub package does), but that would mean making `git.addGitToEnv(env)` async, which would have been a much larger PR. From atom/apm#839 Co-Authored-By: Winston Liu <2766036+50Wliu@users.noreply.github.com>
Functional changes: - Include the mingw directory in the PATH (this directory contains many of the actual git functions, such as `git-pull`, `git-push`, and `git-upload-pack`). - Search `env.ProgramW6432` for Git before `env.ProgramFiles`, as `ProgramFiles` is not guaranteed to always point to the x64 Program Files directory. Technically, we should get the mingw path from `git --exec-path` (and this is what the GitHub package does), but that would mean making `git.addGitToEnv(env)` async, which would have been a much larger PR. From atom/apm#839 Co-Authored-By: Winston Liu <2766036+50Wliu@users.noreply.github.com>
Functional changes: - Include the mingw directory in the PATH (this directory contains many of the actual git functions, such as `git-pull`, `git-push`, and `git-upload-pack`). - Search `env.ProgramW6432` for Git before `env.ProgramFiles`, as `ProgramFiles` is not guaranteed to always point to the x64 Program Files directory. Technically, we should get the mingw path from `git --exec-path` (and this is what the GitHub package does), but that would mean making `git.addGitToEnv(env)` async, which would have been a much larger PR. From atom/apm#839 Co-Authored-By: Winston Liu <2766036+50Wliu@users.noreply.github.com>
Let's get this working.
Functional changes:
git-pull
,git-push
, andgit-upload-pack
).env.ProgramW6432
for Git beforeenv.ProgramFiles
, asProgramFiles
is not guaranteed to always point to the x64 Program Files directory.Technically, we should get the mingw path from
git --exec-path
(and this is what the GitHub package does), but that would mean makinggit.addGitToEnv(env)
async, which would have been a much larger PR.