Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

IDE is using wrong default checksum (Sha1) and compiler switched to Sha256 #43921

Open
KirillOsenkov opened this issue May 2, 2020 · 11 comments
Labels
Area-Compilers Concept-API This issue involves adding, removing, clarification, or modification of an API.

Comments

@KirillOsenkov
Copy link
Member

SourceText.From factory methods are still defaulting to checksum Sha1:
http://sourceroslyn.io/#Microsoft.CodeAnalysis/Text/SourceText.cs,d444ba9c80426d47

Meanwhile the compiler has switched to Sha256:
#24820

Also there's a disconnect that the compiler does not respect the user-specified checksum and hardcodes Sha256:
#24735

This can result in confusion as the checksum algorithm specified by the workspace host doesn't matter.

@tmat @gafter

@tmat
Copy link
Member

tmat commented May 3, 2020

We can't change defaults used by APIs. That would be a breaking change.

@KirillOsenkov
Copy link
Member Author

Curious about an actual sample scenario that would break.

@jinujoseph jinujoseph added Area-Compilers Concept-API This issue involves adding, removing, clarification, or modification of an API. and removed Area-IDE labels May 7, 2020
@Neme12
Copy link
Contributor

Neme12 commented Aug 6, 2023

We can't change defaults used by APIs. That would be a breaking change.

Wouldn't this an acceptable breaking change? I don't really expect people who don't explicitly specify a checksum algorithm to actually depend on it or use it.

This seems like a really small and low-risk breaking change that would be OK for a major release version. And it wouldn't be a binary breaking change - users would have to recompile with the new version to get the new checksum algorithm.

@Neme12
Copy link
Contributor

Neme12 commented Aug 6, 2023

Wasn't the compiler changing its default hash algorithm to sha256 technically a breaking change too? And that change was still taken.

@CyrusNajmabadi
Copy link
Member

I don't see any reason to make a breaking change. At best, no one is harmed. But at worse, it causes pain for some users.

I don't see what benefit we get from breaks here at all.

@Neme12
Copy link
Contributor

Neme12 commented Aug 6, 2023

Well, the reason would be that sha1 is officially deprecated. And the compiler already uses sha256 by default anyway.

@CyrusNajmabadi
Copy link
Member

Well, the reason would be that sha1 is officially deprecated.

I don't see what customer benefits from this. It how it benefits us. Breaking things needs avery strong justification as to the value it provides.

Right now, I'm not actually seeing any value. We also haven't gotten reports or upvotes from customers asking for this.

So I repeat that at best this is neutral, but at worse this hurts people.

And the compiler already uses sha256 by default anyway.

What the compiler does isn't relevant. They have different customers and different ship scenarios and rules they have to operate under. For example, they might have to operate under certain constraints that make things a requirement (fips, etc.).

Absent a requirement that we just remove sha1, or lots of customers indicating this default is a substantive problem, we should not arbitrarily break people :-)

@Neme12
Copy link
Contributor

Neme12 commented Aug 6, 2023

I just have a hard time seeing how this would break anyone.

@Neme12
Copy link
Contributor

Neme12 commented Aug 6, 2023

What the compiler does isn't relevant. They have different customers and different ship scenarios and rules they have to operate under.

Compared to who/what? We are talking about the compiler.

@CyrusNajmabadi
Copy link
Member

I just have a hard time seeing how this would break anyone.

Anyone using our apis and storing things based on hashes.

@CyrusNajmabadi
Copy link
Member

Compared to who/what? We are talking about the compiler.

Compared to consumers using the API who are not the compiler :-)

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Area-Compilers Concept-API This issue involves adding, removing, clarification, or modification of an API.
Projects
None yet
Development

No branches or pull requests

5 participants