Clarify build.rs C compiler and linker configuration. #1697
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
briansmith
commented
Oct 5, 2023
| // Compile all the (dirty) source files into object files. | ||
| let objs = additional_srcs | ||
| .iter() | ||
| .chain(srcs.iter()) | ||
| .map(|f| compile(f, target, out_dir, out_dir)) | ||
| .map(|f| compile(&c, f, target, out_dir, out_dir)) |
There was a problem hiding this comment.
Now all the calls to compile() share a single build configuration that is constructed once per library, instead of once per source file.
| .collect::<Vec<_>>(); | ||
|
|
||
| // Rebuild the library if necessary. | ||
| let lib_path = PathBuf::from(out_dir).join(format!("lib{}.a", lib_name)); | ||
|
|
||
| let mut c = cc::Build::new(); | ||
|
|
||
| for f in LD_FLAGS { |
There was a problem hiding this comment.
LD_FLAGS was empty so this wasn't doing anything.
| match ext { | ||
| "c" | "S" => (), | ||
| e => panic!("Unsupported file extension: {:?}", e), | ||
| }; |
There was a problem hiding this comment.
Moved to the top of the new cc().
This was referenced Oct 5, 2023
Clarify which parts of the build script modify the compiler configuration (`configure_cc`) and which don't (`cc`). Ensure that the configuration is only done once per library, instead of once per source file, as each `cc` invocation can reuse the configuration work done by a single `configure_cc` call.
briansmith
force-pushed
the
b/simplify-build.rs-1
branch
from
ace2a13 to
43eb390
Compare
Codecov Report
@@ Coverage Diff @@
## main #1697 +/- ##
==========================================
+ Coverage 95.97% 95.98% +0.01%
==========================================
Files 134 134
Lines 19980 19980
Branches 199 199
==========================================
+ Hits 19175 19177 +2
Misses 767 767
+ Partials 38 36 -2 see 2 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
cc-rs's documentation says that it already handles this automatically, which is why we'd already removed it for other targets.
First, we were passing `-Wl,--gc-sections` to the compiler regardless of whether it is MSVC, which didn't make any sense on its own. But, even more generally, it doesn't make sense for us to try to tell the linker what to do when we aren't actually linking. (We're building static libraries of the C and assembly code.)
briansmith
force-pushed
the
b/simplify-build.rs-1
branch
2 times, most recently
from
ae3dcc2 to
34f6ec8
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See each individual commit message for details.