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

Add property function GetStableHash for String #4986

Closed
rainersigwald opened this issue Dec 13, 2019 · 3 comments · Fixed by #5723
Closed

Add property function GetStableHash for String #4986

rainersigwald opened this issue Dec 13, 2019 · 3 comments · Fixed by #5723
Assignees
Labels
Area: Language Issues impacting the MSBuild programming language. Feature Request Partner request triaged

Comments

@rainersigwald
Copy link
Member

Some projects wish to include hashes of strings (often paths) in output folder names, which must be defined at evaluation time. System.String.GetHashCode is available but unsuitable for many desirable purposes: it's documented to vary across 64- and 32-bit .NET implementations and can also vary between .NET Framework and .NET Core.

MSBuild has an internal method to get a stable hash that's currently used for IPC

https://github.com/microsoft/msbuild/blob/fa773bb8d44a358963481c02e772509dc408a6d9/src/Shared/CommunicationsUtilities.cs#L673-L707

We should expose it as $([MSBuild]::StableStringHash()).

Open questions:

  • Name
  • Should we override calls to String.GetHashCode()?
@rainersigwald rainersigwald added Feature Request Area: Language Issues impacting the MSBuild programming language. Partner request labels Dec 13, 2019
@Forgind Forgind self-assigned this Dec 16, 2019
@Forgind
Copy link
Member

Forgind commented Dec 16, 2019

image
This part looks like something that would cause me problems if I didn't know about it previously. I'd prefer to override System.String.GetHashCode.

@dasMulli
Copy link
Contributor

Would authors using this hash code be happy with String.GetHashCode() behaving differently between msbuild versions? This may add another unexpected (?) dimension to the differences matrix until every system is updated; all dev tools (vs/cli/vs-mac/rider) as well as CI systems.
From an API perspective it would make more sense to add a new API and offer build authors the possibility to require a minimum MSBuild version or condition on the current version instead of playing a "what tool is build me"-roulette.

@Forgind
Copy link
Member

Forgind commented Dec 26, 2019

That's true. I don't like how GetHashCode works now, but it's also good to avoid unexpected changes. If we were to add a System.String.HashCode that generates a stable hash in addition to System.String.GetHashCode, that would resolve the problem.

On the other hand, the warning I pasted previously explicitly says not to use them outside the application domain or persist them, so this might not be a problem. Also, if hashes are expected to "differ across .NET implementations," this just seems like more of the same.

Forgind added a commit to Forgind/msbuild that referenced this issue Sep 8, 2020
This method hashes a string without taking target framework or bitness into account. Fixes dotnet#4986
Forgind added a commit that referenced this issue Oct 2, 2020
Add StableStringHash method

This method hashes a string without taking target framework or bitness into account. Fixes #4986

Test StableStringHash
sujitnayak pushed a commit to NikolaMilosavljevic/msbuild that referenced this issue Oct 12, 2020
Add StableStringHash method

This method hashes a string without taking target framework or bitness into account. Fixes dotnet#4986

Test StableStringHash
@AR-May AR-May added the triaged label Feb 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Language Issues impacting the MSBuild programming language. Feature Request Partner request triaged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants