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

Introduce [MSBuild]::StableStringHash overload(s) with alternative hashing #9519

Closed
JanKrivanek opened this issue Dec 12, 2023 · 5 comments · Fixed by #9721
Closed

Introduce [MSBuild]::StableStringHash overload(s) with alternative hashing #9519

JanKrivanek opened this issue Dec 12, 2023 · 5 comments · Fixed by #9721

Comments

@JanKrivanek
Copy link
Member

JanKrivanek commented Dec 12, 2023

Blocked By

#9572
https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1937630

Context

#9387 introduced improved hashing for [MSBuild]::StableStringHash, that however broke internal functionality relying on the hash to be stable between versions (despite documented otherwise).

Proposal

The easiest way around it is to keep the current property function untouched, but introduce a new overload taking an enum argument:

<!-- Works unchanged -->
[MSBuild]::StableStringHash($x)

<!-- 
  $hashAlgo will currently allow:
    'GetHashCode' - the legacy behavior (mimicking string.GetHashCode)
    'FNV1a_32' - Fawler-Noll-Vo 1a 32bit
-->
[MSBuild]::StableStringHash($x, $hashAlgo)

This way other hashes (like xxHash) can be introduced in the future

Related

@KalleOlaviNiemitalo
Copy link

KalleOlaviNiemitalo commented Dec 13, 2023

This should not have been closed yet; 5af9301 mentioned this issue but did not implement the feature request.

@JanKrivanek JanKrivanek reopened this Dec 13, 2023
@danmoseley
Copy link
Member

Why not just change the existing method rather than add an overload? Keeping it simple. Would it just cause one unnecessary builds on upgrade?

@rainersigwald
Copy link
Member

hashtag we tried, @danmoseley.

The change caused a category of problem that's new now that we have caching plugins. The cache used today for some internal Microsoft repos (like the VS repo for example) is populated on official builds from CloudBuild, which run on a recent stable MSBuild, so the caches have the old hash behavior burned in if relevant. When a new hash behavior is deployed to the cache clients (local dev machines), the cache stops functioning correctly.

@JanKrivanek
Copy link
Member Author

Let's not move the hashing utils to StringTools until the #9572 is resolved - as we'd broke the DevKit!

@JanKrivanek
Copy link
Member Author

Blocked by AB#1937630

@JanKrivanek JanKrivanek changed the title Intorduce [MSBuild]::StableStringHash overload(s) with alternative hashing Introduce [MSBuild]::StableStringHash overload(s) with alternative hashing Jan 22, 2024
@AR-May AR-May added the triaged label Feb 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants