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

Use hashing instead of other algorithms to guarantee scala classes re… #308

Closed
wants to merge 1 commit into from
Closed

Use hashing instead of other algorithms to guarantee scala classes re… #308

wants to merge 1 commit into from

Conversation

fido-node
Copy link

…build.
Fix for #307

@thesamet
Copy link
Owner

thesamet commented May 3, 2022

Thanks for sending the PR. Since getting the hash is more expensive, I am wondering why the "last modified" approach fails here? Do the files in the two branches have the exact same last modified time?

@fido-node
Copy link
Author

Hi. It is a little bit hard for me to debug why this happens.
I've attached reproduction case and repo with the smallest possible example to this issue:
#307

@fido-node
Copy link
Author

fido-node commented May 8, 2022

This behavior related to this enhancement in sbt. Since our build pipelines do not really need this sbt feature I can close this PR for now. I'll also write a comment in the issue to describe proper workaround.

@fido-node fido-node closed this May 8, 2022
@fido-node fido-node deleted the use-file-hashes branch May 8, 2022 15:23
@thesamet
Copy link
Owner

thesamet commented May 8, 2022

Thanks for reporting, investigating and documenting the workaround!

@bjaglin
Copy link
Contributor

bjaglin commented May 8, 2022

I don't have time to test nor to write a scripted test, but I believe this should address the issue with no overhead (actually it would save some FS calls).

val set = IO.unzip(dep, extractTarget, "*.proto")

-val set = IO.unzip(dep, extractTarget, "*.proto")
+val set = IO.unzip(dep, extractTarget, "*.proto", preserveLastModified = false)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants