Skip to content
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

fix(core): do not use joinPathFragments for generating files #18753

Merged

Conversation

Cammisuli
Copy link
Member

@Cammisuli Cammisuli commented Aug 21, 2023

Current Behavior

When running npx create-nx-workspace on a drive that is not C:, the process fails because we strip out the drive letter before trying to gather file information. This failure happens during create-nx-workspace and ensurePackage because those two functions will create a temporary directory (usually in %LOCALAPPDATA%, ie - C:\Users\user\AppData\Local), and run nx generators from there. If we try to use readdirSync without the drive letter, it will default to looking at the current working directory. So if the working directory is D:\dev, and a generator file is in C:\Users\user\AppData\Local\etc, Nx will "normalize" the windows path and remove the drive letter, and node will try to look for the file in D:\Users\user\AppData\Local\etc.

Expected Behavior

Generating files do not remove the drive letter, and we just use a regular join

Related Issue(s)

Fixes #18677
fixes #18727

@Cammisuli Cammisuli requested review from a team as code owners August 21, 2023 21:30
@vercel
Copy link

vercel bot commented Aug 21, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
nx-dev ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 22, 2023 3:51pm

@Cammisuli Cammisuli force-pushed the fix_windows_drive_letters_for_cnw branch from 7549766 to 62d529a Compare August 22, 2023 00:35
@Cammisuli Cammisuli changed the title fix(core): do not remove drive letter if running from a temp directory on windows fix(core): do not use joinPathFragments for generating files Aug 22, 2023
@Cammisuli Cammisuli force-pushed the fix_windows_drive_letters_for_cnw branch from 62d529a to 44de0ff Compare August 22, 2023 14:01
@Cammisuli Cammisuli requested review from a team as code owners August 22, 2023 14:01
@Cammisuli Cammisuli force-pushed the fix_windows_drive_letters_for_cnw branch from 44de0ff to b4c121b Compare August 22, 2023 14:09
@Cammisuli Cammisuli force-pushed the fix_windows_drive_letters_for_cnw branch from b4c121b to 41f003e Compare August 22, 2023 15:45
@@ -5,14 +5,16 @@ function removeWindowsDriveLetter(osSpecificPath: string): string {
}

/**
* Coverts an os specific path to a unix style path
* Coverts an os specific path to a unix style path. Use this when writing paths to config files.
* This should not be used to read files on disk because of the removal of Windows drive letters.
Copy link
Member

Choose a reason for hiding this comment

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

This messaging will show up on nx.dev as well as intellisense for devkit users, I think its probably fine as-is, but maybe it'd be worth phrasing it more in line with it being ok for relative paths but not absolute? I can't think of a time that it wouldn't work when given relative path args.

@FrozenPandaz FrozenPandaz merged commit bbae14b into nrwl:master Aug 23, 2023
3 checks passed
@Cammisuli Cammisuli deleted the fix_windows_drive_letters_for_cnw branch August 23, 2023 18:15
@github-actions
Copy link

This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 29, 2023
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.

create-nx-workspace breaking in windows. again. create-nx-workspace not compatible with windows filesystem
6 participants