minlib tech with optimized rebuild #477
Conversation
| src/composer/target/debug/compose $script $name | ||
| local dir="system_binaries/cos_build-${name}" | ||
| else | ||
| echo "[cos executing] src/composer/target/debug/compose $script $name $minlib" |
There was a problem hiding this comment.
I don't think you need to replicate the echo here. If you simply print it out before the conditional, if $minlib is nothing, it won't print out anything. Which should be identical in behavior to this code, without the replication. I might be wrong.
| $(info ***********************************************) | ||
| $(info *********[ Building Implementations ]**********) | ||
| $(info ***********************************************) | ||
| $(info $(SUBDIRS)) |
There was a problem hiding this comment.
This is likely just for debugging, I'm assuming. If so, it should not appear in a PR.
| COMP_EXPIF_OBJS=$(foreach I,$(COMP_INTERFACES_CLEAN),$(INTERDIR)/$(I)/cosrt_s_stub.o) | ||
| COMP_DEP_OBJS=$(foreach D,$(COMP_IFDEPS_CLEAN),$(INTERDIR)/$(D)/cosrt_c_stub.o) | ||
|
|
||
| DEP_DIRS := $(foreach obj,$(COMP_DEP_OBJS),$(dir $(obj))) |
There was a problem hiding this comment.
Maybe a comment block above these to say what they are trying to do?
| $(info | Exported interfaces: $(COMP_INTERFACES_CLEAN)) | ||
| $(info | Interface dependencies: $(COMP_IFDEPS_CLEAN)) | ||
| $(info | Libraries: $(DEPENDENCY_LIBS) $(DEPENDENCY_LIBOBJS)) | ||
| $(info | Libraries:$(DEPENDENCY_LIBS) $(DEPENDENCY_LIBOBJS)) |
There was a problem hiding this comment.
If this was an accident, undo it.
|
|
||
| /* start the text/read-only sections */ | ||
| .text : ALIGN(4096) { *(.text*) } :text | ||
| .text : ALIGN(4096) { KEEP(*(.text.__cosrt_c_cosrtdefault)) *(.text*) } :text |
There was a problem hiding this comment.
I don't understand the spacing on this. Feel free to use multiple lines if that works with the syntax here.
.text : ALIGN(4096) {
KEEP(*(.text.__cosrt_c_cosrtdefault))
*(.text*)
} :text
There was a problem hiding this comment.
My concern with this is this: if this is necessary to keep symbols around, don't we need an automated way to construct all of the symbols, not just the default entry point?
| let mut cflag_minlib = "-ffunction-sections -fdata-sections"; | ||
|
|
||
| //rebuild the files under the related sub directories | ||
| let output = Command::new("make") |
There was a problem hiding this comment.
Should be able to use exec_pipeline here instead of pulling in the dependency on Command, right? I think you explained why you couldn't, but I can't figure out now.
There was a problem hiding this comment.
Coming back to this now. I'm guessing you need the actual output, and exec_pipeline doesn't provide that. I think that the correct abstraction level is to provide an exec_pipeline variant that returns (stdout, stderr) output strings.
|
|
||
| if output.status.success() { | ||
| let stdout = String::from_utf8_lossy(&output.stdout); | ||
| println!("Output: {}", stdout); |
There was a problem hiding this comment.
These seem like they are focused on debugging output? not sure if we really want this output normally.
| )); | ||
| } | ||
|
|
||
| let b_minlib= arg3.as_ref().map(|s| s == "minlib").unwrap_or(false); |
There was a problem hiding this comment.
Not sure what the b_ stands for here. Maybe use_minlib?
| let output = progs.iter() | ||
| .fold(None, | ||
| |upstream: Option<Pipe>, cmd| match upstream { | ||
| |upstream: Option<Pipe>, cmd| { println!("Executing command: {}", cmd); // This line added to print the command |
There was a problem hiding this comment.
Please remove debugging print statements, and format.
|
|
||
| // FIXME: progs should be a more general iteration type | ||
| // return a tuple of stdout/stderr | ||
| pub fn exec_pipeline(progs: Vec<String>) -> (String, String) { |
There was a problem hiding this comment.
OK, so this already does return the outputs. So it might work instead of using Command directly?
Summary of this Pull Request (PR)
Add description here.
Intent for your PR
Choose one (Mandatory):
Reviewers (Mandatory):
(Specify @<github.com username(s)> of the reviewers. Ex: @user1, @user2)
Code Quality
As part of this pull request, I've considered the following:
Style:
Code Craftsmanship:
Testing
I've tested the code using the following test programs (provide list here):