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

remove git diff feature from build.zig #7542

Merged
merged 2 commits into from Dec 26, 2020
Merged

Conversation

g-w1
Copy link
Contributor

@g-w1 g-w1 commented Dec 24, 2020

No description provided.

@FireFox317
Copy link
Contributor

Nice one, thanks! I have run into this as well, and it was really annoying. However I do think it is weird that it returns StreamTooLong, shouldn't it just work even if the git output is really long?

@g-w1
Copy link
Contributor Author

g-w1 commented Dec 25, 2020 via email

@andrewrk
Copy link
Member

Yes thanks for this. However I think it would be best to remove this code entirely from the build script. Compiler contributors will have to be aware that the cache compiler ID is not different based on working tree changes. In exchange for this we get a simpler build script, faster build, and ability to debug cache issues.

@g-w1 g-w1 changed the title Fix a case where if git diff HEAD ran from build script was too long, it would fail remove git diff feature from build.zig Dec 25, 2020
@LemonBoy
Copy link
Contributor

What about using something more compact like git diff raw format?

@andrewrk
Copy link
Member

andrewrk commented Dec 26, 2020

I think that would be a good bet if we wanted to keep this functionality. I say we move forward with removing it, try out development without it, and then we can see how it goes and reconsider adding it using the git diff raw format. Thanks for pointing that out; I was unaware of its existence.

@andrewrk andrewrk merged commit 864a544 into ziglang:master Dec 26, 2020
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.

None yet

4 participants