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

feat(core): use the native hasher by default #15071

Merged
merged 2 commits into from
Feb 17, 2023

Conversation

Cammisuli
Copy link
Member

@Cammisuli Cammisuli commented Feb 16, 2023

Current Behavior

We currently have 3 ways to hash a workspace:

  • node
  • git
  • rust

We first check if NX_NATIVE_HASHER is set in the env, then use the rust based hasher.

Otherwise we check to see if we're in a submodule, and if true, use the node based hasher.

Then all other cases use the Git hasher

Expected Behavior

The rust hasher is now used by default, unless NX_NON_NATIVE_HASHER is set to true. If it is, it uses the previous logic for the git and node hasher.

Related Issue(s)

Fixes #

@vercel
Copy link

vercel bot commented Feb 16, 2023

Deployment failed with the following error:

Creating the Deployment Timed Out.

Copy link
Collaborator

@FrozenPandaz FrozenPandaz left a comment

Choose a reason for hiding this comment

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

Slow down there. Let's make it default and allow people to opt out. Keep the other code.

@vercel
Copy link

vercel bot commented Feb 17, 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 17, 2023 at 0:12AM (UTC)

[fileName: string]: string;
};

export class TempFs {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we need this exactly?

Copy link
Member

Choose a reason for hiding this comment

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

My presumption is that memfs doesn't work with rust's fs access calls. I could be wrong though 🤷. Lets let @Cammisuli respond.

Copy link
Member Author

Choose a reason for hiding this comment

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

@AgentEnder is correct.

@FrozenPandaz FrozenPandaz merged commit 7d80f25 into nrwl:master Feb 17, 2023
@Cammisuli Cammisuli deleted the make_native_hasher_default branch February 21, 2023 13:49
FrozenPandaz added a commit to FrozenPandaz/nx that referenced this pull request Feb 24, 2023
@LongLiveCHIEF
Copy link
Contributor

@Cammisuli @FrozenPandaz I think this should be reverted, or logic should be inverted. Many development teams are cross-platform, and additionally, CI environments are on Linux. A common practice is to freeze dependencies for scanning and change control auditing purposes in release builds. This means teams will have failures in pipelines/builds that are in different environments than the one yarn/npm was initially run on.

@LongLiveCHIEF
Copy link
Contributor

Also, there was no documentation added for this as far as I can see.

@github-actions
Copy link

github-actions bot commented Mar 8, 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 8, 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.

None yet

4 participants