Normalize path used for git add in respect to OS/environment
#1134
Conversation
test/GitUtilities.js
Outdated
| }); | ||
| it("works with absolute paths for file and cwd", () => { | ||
| const cwd = path.resolve("test"); | ||
| const file = path.resolve("test", "foo"); |
There was a problem hiding this comment.
Shouldn't this be const file = path.resolve(cwd, "foo");? Otherwise I don't see how this test is any different than the previous one.
There was a problem hiding this comment.
In the previous test only cwd is absolute; the file path is still relative.
In this one both are absolute.
Your line of code is equivalent to mine, i.e. path.resolve(path.resolve("test"), "foo") (your code but with cwd inlined) returns the same value as path.resolve("test", "foo").
Should I change it?
src/GitUtilities.js
Outdated
| @@ -38,7 +38,8 @@ export default class GitUtilities { | |||
|
|
|||
| static addFile(file, opts) { | |||
| log.silly("addFile", file); | |||
| ChildProcessUtilities.execSync("git", ["add", file], opts); | |||
| const portablePath = path.relative(opts.cwd, path.resolve(opts.cwd, file)).replace(/\\/g, '/'); | |||
There was a problem hiding this comment.
It's a pity there's no built-in way to coerce paths to posix... (path.posix.* methods don't work with Windows paths, near as I can tell)
There was a problem hiding this comment.
Would it be worthwhile to pull in one of these dependencies?
https://www.npmjs.com/package/path-posix (edit: not what we're looking for)
https://www.npmjs.com/package/ensure-posix-path (no README but tests look good)
|
@evocateur Added some commits. I think my changes to the two new tests helps to clarify the difference (there's only 1 line different between them now, I pulled in Also added one more test to make sure Windows paths are always transformed to POSIX paths for |
|
This thread has been automatically locked because there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Description
Added code to transform file paths used in
git addto OS- (POSIX vs Windows) neutral form, for improved portability.Motivation and Context
See issue #1133
How Has This Been Tested?
Added two tests to the
GitUtilitiestest suite.Also
npm linked the package to my monorepo project to verify that my changes resolve the problem in #1133Types of changes
Checklist:
Actually some tests were failing on my Windows 7 Pro system after a fresh clone and install. Ignoring those, yes, all new and existing tests passed. I expect tests to pass on CI.