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): building project graphs is broken for Windows #15257

Merged
merged 1 commit into from
Feb 26, 2023

Conversation

mayfieldiv
Copy link
Contributor

Current Behavior

The new native hasher introduced in #14476 doesn't properly handle back-slashes on Windows, resulting in it always returning an empty collection of files. This breaks things like nx graph and the affected commands.

This issue can be readily reproduced by following the angular tutorial through step 2 in a Windows environment with the latest pre-release of nx: https://nx.dev/angular-tutorial/2-project-graph

Following that tutorial, running nx graph or nx print-affected only displays the implicit dependency declared in core-e2e, since it never properly inspects any of the source files (*.ts or package.json).

Expected Behavior

The native hasher should behave as expected on all platforms.

Two problems were preventing any files from being returned from hash_files in the new native hasher on Windows:
1. The redefined `workspace_root` contained a mix of back and forward slashes,
    which didn't work with the String implementation of strip_prefix
2. After that was fixed, the returned paths would have backslashes in them,
    which didn't work downstream on the nodejs side
@vercel
Copy link

vercel bot commented Feb 24, 2023

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

Name Status Preview Comments Updated
nx-dev ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 24, 2023 at 11:12PM (UTC)

@Cammisuli
Copy link
Member

Thanks for the PR!

We discussed the slashes while this was in development and I made an extra effort to test this on windows. And it passed for me 😮

What shell/terminal were you using? Powershell, pwsh, git bash, or cmd?

@mayfieldiv
Copy link
Contributor Author

Oh interesting! I'm using pwsh (not on WSL). I'm on the latest stable versions (or close to latest) of PowerShell Core v7, nodejs v18, git, nx, Windows 10, etc.

@Cammisuli
Copy link
Member

Hmm the only big difference I see is that I'm on Windows 11.

I wonder if Microsoft finally fixed the paths with 11 😂

Anyway, I'll give this a thorough check on Monday.

Thanks again

Copy link
Member

@Cammisuli Cammisuli left a comment

Choose a reason for hiding this comment

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

Thanks again!

I tested the changes on Windows (11), macOS 13, and Ubuntu 20 and 22 and it's all good 🙂

@Cammisuli Cammisuli merged commit 0b30f1f into nrwl:master Feb 26, 2023
@mayfieldiv mayfieldiv deleted the windows-native-hasher branch February 26, 2023 03:20
@github-actions
Copy link

github-actions bot commented Mar 3, 2023

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 Mar 3, 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.

2 participants