Skip to content
This repository has been archived by the owner on Jan 6, 2021. It is now read-only.

remove duplicate of PATH environment variable on Windows #218

Closed

Conversation

aberkovsky
Copy link

What:
Issue #216

Why:
On Windows operating systems, environment variables are case-insensitive.
However, if when you spawn a new process and pass the env with two variables (PATH and Path), in this new process process.env will contains both variables, and this breaks the subsequent logic.

Npm has logic to set both Path and PATH (if exist) variables on Windows, and by default used Path on Windows.
But if I use cgwin terminal on Windows for example, process.env contains PATH.

How:
I add patchPathEnv.js

function patchPathEnv(env) {
  if (isWindows() && env.Path && env.PATH) {
    delete env.Path
  }
}

remove duplicate variable if both exists.

Checklist:

  • [N/A] Documentation
  • [ x] Tests
  • [ x] Ready to be merged

@codecov
Copy link

codecov bot commented Oct 17, 2019

Codecov Report

Merging #218 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #218   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           4      5    +1     
  Lines          78     84    +6     
  Branches       18     19    +1     
=====================================
+ Hits           78     84    +6
Impacted Files Coverage Δ
src/patch-path.js 100% <100%> (ø)
src/index.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 67f21c3...b212e54. Read the comment docs.

Copy link
Owner

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But if I use cgwin terminal on Windows for example, process.env contains PATH.

Does this break for people who aren't using cgwin?


function patchPathEnv(env) {
if (isWindows() && env.Path && env.PATH) {
delete env.Path
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add some comments here to explain why we're doing this because looking at this code as it is is unclear.

@kentcdodds
Copy link
Owner

#257

@kentcdodds kentcdodds closed this Dec 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants