-
Notifications
You must be signed in to change notification settings - Fork 22
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
Provide build_info.json file for downstream builds #171
base: master
Are you sure you want to change the base?
Conversation
I also filed #172 as a follow-up issue to keep bazel and this new build_info.json in sync. Even without that, this change will make downstream, non-bazel usage much cleaner. If the file list is changed we just need to update it here and have downstream roll, this is much easier than having to sync changing the downstream's hard coded list with emboss's new actual list of files (or in case of Fuchsia having to coordinate Pigweed and Emboss rolls at the same time). |
build_info.json
Outdated
@@ -0,0 +1,63 @@ | |||
{ | |||
"comment": "Provides build info for use by downstream build rules.", | |||
"emboss_py_files": [ |
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.
Maybe name this something like emboss_compiler_files
? generated_code_templates
and prelude.emb
are not py files.
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.
Yes, better. I also removed the emboss_
prefix.
build_info.json
Outdated
"compiler/util/traverse_ir.py", | ||
"embossc" | ||
], | ||
"emboss_h_files": [ |
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.
Maybe emboss_cpp_runtime_header_files
? There may be other kinds of header files in the future, used at different stages or in different circumstances.
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.
Done. I also removed the emboss_
prefix.
Provide a JSON file that lists files used in the build. It will be used by downstream, non-bazel builds. For example, https://fxrev.dev/1100339 is one planned usage of this. Emboss Issue: google#152 Pigweed Bug: https://pwbug.dev/359386289 Change-Id: Id33d879cea95d86f6cc7a16cd395a8c1a4380cbf
d7bc5c1
to
da670b6
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.
Thanks Dave, lgtm as well. Small note on the format but I don't think it's a blocker to landing. Let me know if you want to land as-is and we can push it!
@@ -0,0 +1,63 @@ | |||
{ | |||
"comment": "Provides build info for use by downstream build rules.", |
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.
Maybe just make this json5 so we can have inline comments?
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.
The GN spec and CMake specs just say "JSON" so I hesitate to use JSON5 in case they or some other downstream build system doesn't support it.
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.
Ah yeah that's true. I imagined this being the input for an emboss build step that would generate GN, CMake, Bazel, Soong, etc integration files (as well as plain json), but I realize I didn't actually articulate that here.
Currently Emboss only provides build rules for bazel, but downstream projects use a variety of build systems (GN, CMake, Makefile, Soong, etc). So this change provides a JSON file that lists files used in the build for those downstream builds to use.
https://fxrev.dev/1100339 is one example planned usage of this.
Emboss Issue: #152
Pigweed Bug: https://pwbug.dev/359386289