-
Notifications
You must be signed in to change notification settings - Fork 44
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
Write a new schema only if necessary #2877
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2877 +/- ##
==========================================
- Coverage 67.74% 67.73% -0.02%
==========================================
Files 328 328
Lines 42088 42116 +28
==========================================
+ Hits 28513 28526 +13
- Misses 11987 11999 +12
- Partials 1588 1591 +3 ☔ View full report in Codecov by Sentry. |
4c57e26
to
e18d7df
Compare
As I explain in my change: ``` For the Schema language in particular, we only write files if the write would cause a change. This allows build systems like Make to correctly include bridge-metadata.json and schema.json as build dependencies without always trying to rebuild. We don't do this for all systems because reading every file before writing it is expensive. ``` I've been playing around with using real file dependencies for Go and Make, and the way we always write bridge metadata (which is then a build input via go:embed) prevents Make from ever observing that a provider is already built.
e18d7df
to
7de12f4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Co-authored-by: VenelinMartinov <venelin.v.martinov@gmail.com>
@iwahbe I think this might have the unintened concequence of leaving the schema as always being out of date. If the timestamp on the emitted file is still before that of the source files, then the next time the build is run, it will still have to generate the schema, only to compare it to what's on disk again. In the make world, timestamps of completed file targets always need to be updated even if the file hasn't changed. I would suggest this might need to be reverted 🫤 |
This PR has been shipped in release v3.103.0. |
As I explain in my change:
I've been playing around with using real file dependencies for Go and Make, and the way we always write bridge metadata (which is then a build input via go:embed) prevents Make from ever observing that a provider is already built.