This PR changes the tremendously spammy 'make depends' output to only print actual output if something breaks. As it stands, travis truncates the logs because it is too long, and I doubt anyone has any use for a million lines of build logs unless something actually doesn't compile.
Need to verify:
|| cat outputfile approach will signal the exit code to automake or if automake will continue puffing along.
You can use || ( cat outputfile && false ) to force stop make
@ken2812221 Nice!
Concept ACK, though we'll potentially run into timeouts when compiling qt.
172 | +$($(1)_fetched): | $($(1)_introduced) 173 | $(AT)mkdir -p $$(@D) $(SOURCES_PATH) 174 | $(AT)rm -f $$@ 175 | $(AT)touch $$@ 176 | - $(AT)cd $$(@D); $(call $(1)_fetch_cmds,$(1)) 177 | + $(AT)cd $$(@D); ( echo -n -e ""; $(call $(1)_fetch_cmds,$(1)) ) > .fetch_output 2>&1 || ( cat .fetch_output && false )
Instead of .fetch_input, let's use $$($(1)_download_dir)/.fetch_output
That way we're guaranteed to never have any weird races or overwrites.
It'll also take care of cleaning up the logs when successful.
181 | - $(AT)echo Extracting $(1)... 182 | +$($(1)_extracted): | $($(1)_introduced) $($(1)_fetched) 183 | + $(AT)echo `date $(shortdate)`: Extracting $(1)... 184 | $(AT)mkdir -p $$(@D) 185 | - $(AT)cd $$(@D); $(call $(1)_extract_cmds,$(1)) 186 | + $(AT)cd $$(@D); ( echo -n -e ""; $(call $(1)_extract_cmds,$(1)) ) > .extract_output 2>&1 || ( cat .extract_output && false )
Same as above, but $$($(1)_download_dir)/.fetch_output. Likewise for the others.
157 | @@ -157,48 +158,60 @@ $(1)_autoconf += LDFLAGS="$$($(1)_ldflags)" 158 | endif 159 | endef 160 | 161 | +shortdate="+%H:%M:%S" 162 | + 163 | define int_add_cmds 164 | -$($(1)_fetched): 165 | +$($(1)_introduced): 166 | + $(AT)echo "============================================================"
No need for the "$(AT)"s for the echos.
A line starting with $(AT) means that it will be echoed to the terminal if you use make V=1. For example, the mkdir/rm -f below aren't interesting, so we don't usually print them. But they can be shown for debugging with V=1.
Hm, got it. I'll look into undoing this if V=1.
Oddly, when I removed the $(AT)'s I got the opposite result:
$ make HOST=x86_64-apple-darwin11
echo "============================================================"
============================================================
echo " Package : native_biplist"
Package : native_biplist
echo " Date : `date`"
Date : Wed Jul 18 14:55:22 JST 2018
echo "============================================================"
============================================================
14:55:22: Extracting native_biplist...
14:55:22: Preprocessing native_biplist...
[...]
@kallewoof Sorry, I realize that was very misleading.
An "@" in front of a rule causes it to not be echoed.
$(AT) means "echo only if V=1" (otherwise it expands to "@").
Because we never want to echo the echo command itself, you can just use @echo foo rather than $(AT) echo foo.
177 | + $(AT)cd $$(@D); ( echo -n -e ""; $(call $(1)_fetch_cmds,$(1)) ) > .fetch_output 2>&1 || ( cat .fetch_output && false ) 178 | $(AT)cd $($(1)_source_dir); $(foreach source,$($(1)_all_sources),$(build_SHA256SUM) $(source) >> $$(@);) 179 | $(AT)touch $$@ 180 | -$($(1)_extracted): | $($(1)_fetched) 181 | - $(AT)echo Extracting $(1)... 182 | +$($(1)_extracted): | $($(1)_introduced) $($(1)_fetched)
I should think $($(1)_fetched) would be enough here, is it not for some reason?
The intro part would sometimes not show for packages with nothing to fetch, it appeared like. I can try again though.
205 | $(AT)touch $$@ 206 | $($(1)_built): | $($(1)_configured) 207 | - $(AT)echo Building $(1)... 208 | + $(AT)echo `date $(shortdate)`: Building $(1)... 209 | + $(AT)rm -f .built-$(1) 210 | + $(AT)( sleep 540; if [ ! -e ".built-$(1)" ]; then echo `date $(shortdate)`: ...; sleep 540; if [ ! -e ".built-$(1)" ]; then echo `date $(shortdate)`: ...; fi; fi ) &
I don't understand what this is supposed to be doing. Looks broken, though :)
It prints out "..." after 9 and 18 minutes, unless ".built-$(1)" has been deleted. It prevents travis from "nothing in 10 mins" killing at the qt build, which takes 17 mins.
Ah, I missed the "&" at the end.
I don't think we want to be concerned about that at this level. IMO we should just use travis_wait=20 or so for making depends.
I initially went that route, but travis_wait seems to block output completely.
You should definitely add a comment describing the interaction with travis, because this is pretty obscure...
See comments inline.
Additionally, we need to ensure that make V=1 still prints everything, so there's a way to force complete output.
210 | - $(AT)echo Building $(1)... 211 | - $(AT)mkdir -p $$(@D) 212 | - $(AT)+cd $$(@D); $($(1)_build_env) $(call $(1)_build_cmds, $(1)) 213 | + $(AT)echo `date $(shortdate)`: Building $(1)... 214 | + $(AT)rm -f .built-$(1) 215 | + $(AT)( sleep 540; if [ ! -e ".built-$(1)" ]; then echo `date $(shortdate)`: ...; sleep 540; if [ ! -e ".built-$(1)" ]; then echo `date $(shortdate)`: ...; fi; fi ) &
nit: What about
$(AT)( while true; do sleep 540; if [ ! -e ".built-$(1)" ]; then echo `date $(shortdate)`: ...; else break; fi; done ) &Looks cleaner, but I don't think we want to keep going forever, as the process will continue even if the user aborts (and spam them with date... unti lthey close the terminal).
~Tested ACK fc40bbd~ Please add depends/.built-* into .gitignore
<!--e57a25ab6845829454e8d69fc972939a-->No more conflicts as of last run.
ACK fba94bde752693258e319d595c624015cf52c333, tested locally and confirmed build works and new timestamped output shows up.
The sleep 540 workaround for travis is definitely hacky, but it seems fine if it is documented.
Probably just needs rebase to fix travis UnicodeEncodeError errors (https://github.com/bitcoin/bitcoin/pull/13859#issuecomment-411200990)
The more I think about it, the less I like accommodating Travis at this layer :(
I don't mind quieting the output and dumping the build log to files rather than stdout, that makes sense. But the printing to keep Travis happy really doesn't belong here.
IMO we should instead PR a feature to travis which creates an fd or character device for listening but not printing.
At the very least, it should be optional. I find it nice to watch the depends build output in gitian...
Travis should only print if one of the dependencies changes or the build environment, which happens rarely. And then requesting the raw log to take a look at it shouldn't be too much hassle to motivate for the changes here.
Only print actual output if something breaks, rather than printing all of it.
utACK 42fc414c832bb869557543d78fe8ec9938931d6c. No changes since my previous review, only rebase.
But maybe this issue should be marked work in progress, if you want to work on a different approach?
I'm giving up on this for now. I can't make travis happy for some reason.