Skip to content

Makefile change to responsefiles: 1 entry per line#456

Closed
KageKirin wants to merge 3 commits intobkaradzic:masterfrom
KageKirin:makefile-attfile-perline
Closed

Makefile change to responsefiles: 1 entry per line#456
KageKirin wants to merge 3 commits intobkaradzic:masterfrom
KageKirin:makefile-attfile-perline

Conversation

@KageKirin
Copy link
Contributor

@KageKirin KageKirin commented Jul 3, 2019

Hi,
This is a small change to the gmake responsefile creation to have 1 entry per line, instead of 1 single line having space-separated entries.

Main reason: 1 of my projects has 2000+ source files, and exploded the command line limits when trying to echo all the object names into the responsefile (this happended on Mac).

Cheers.

CC: @rhoot for review.

@rhoot: Btw, you were right in the previous PR: there's no need for extra printing the object files/libs going into the responsefile since they'll be printed anyway in verbose mode, and the output is unwanted in silent mode.

@KageKirin KageKirin changed the title Makefile change to attfiles perline Makefile change to responsefiles: 1 entry per line Jul 3, 2019
@bkaradzic bkaradzic requested a review from rhoot July 3, 2019 14:40
@bkaradzic
Copy link
Owner

Terminate macro shell calls by semicolon

Why is this needed?

@KageKirin
Copy link
Contributor Author

KageKirin commented Jul 3, 2019

Terminate macro shell calls by semicolon

Why is this needed?

$(foreach) chaining of copy, mkdir etc. macros.
i.e. in the case of having a pre/postbuildcommand iterate over a number files to copy with $(foreach), the code will look like this:

without the added semicolon:

macro = $(SILENT) echo "$(1)"
$(foreach v,$(inputs),$(call macro,$(v));)

and now with the trailing semicolon in the macro definition:

macro = $(SILENT) echo "$(1)";
$(foreach v,$(inputs),$(call macro,$(v)))

IMHO, the 2nd one is easier to read and less confusing.
Also, I added the semicolon to have the regular macros be similar to the newly added FPRINT macro, i.e. with trailing semicolon.

I hope this answer is kind of satisfactory. The long story is that I was blocked by the lack of trailing semicolon for a while.

EDIT: I agree that 41ab1a1 does not really need to be part of this PR, and I can remove it if required.
My main reason for adding it was to have the same, semicolon-trailing form for all macros defined in the makefile, in order to avoid someone removing it from FPRINT by mistake b/c it looks different.

Copy link
Contributor

@rhoot rhoot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not work on Windows. The object response file for genie (after adding the flag) turns out to be this:

"" ;
"obj/Release/src/host/lua-5.3.0/src/lapi.o" ;  echo "obj/Release/src/host/lua-5.3.0/src/lauxlib.o" ;  echo "obj/Release/src/host/lua-5.3.0/src/lbaselib.o" ;  echo "obj/Release/src/host/lua-5.3.0/src/lbitlib.o" ;  echo "obj/Release/src/host/lua-5.3.0/src/lcode.o" ;  echo "obj/Release/src/host/lua-5.3.0/src/lcorolib.o" ;  echo "obj/Release/src/host/lua-5.3.0/src/lctype.o" ;  echo "obj/Release/src/host/lua-5.3.0/src/ldblib.o" ;  echo "obj/Release/src/host/lua-5.3.0/src/ldebug.o" ;  echo "obj/Release/src/host/lua-5.3.0/src/ldo.o" ;  echo "obj/Release/src/host/lua-5.3.0/src/ldump.o" ;  echo "obj/Release/src/host/lua-5.3.0/src/lfunc.o" ;  echo "obj/Release/src/host/lua-5.3.0/src/lgc.o" ;  echo "obj/Release/src/host/lua-5.3.0/src/linit.o" ;  echo "obj/Release/src/host/lua-5.3.0/src/liolib.o" ;  echo "obj/Release/src/host/lua-5.3.0/src/llex.o" ;  echo "obj/Release/src/host/lua-5.3.0/src/lmathlib.o" ;  echo "obj/Release/src/host/lua-5.3.0/src/lmem.o" ;  echo "obj/Release/src/host/lua-5.3.0/src/loadlib.o" ;  echo "obj/Release/src/host/lua-5.3.0/src/lobject.o" ;  echo "obj/Release/src/host/lua-5.3.0/src/lopcodes.o" ;  echo "obj/Release/src/host/lua-5.3.0/src/loslib.o" ;  echo "obj/Release/src/host/lua-5.3.0/src/lparser.o" ;  echo "obj/Release/src/host/lua-5.3.0/src/lstate.o" ;  echo "obj/Release/src/host/lua-5.3.0/src/lstring.o" ;  echo "obj/Release/src/host/lua-5.3.0/src/lstrlib.o" ;  echo "obj/Release/src/host/lua-5.3.0/src/ltable.o" ;  echo "obj/Release/src/host/lua-5.3.0/src/ltablib.o" ;  echo "obj/Release/src/host/lua-5.3.0/src/ltm.o" ;  echo "obj/Release/src/host/lua-5.3.0/src/lundump.o" ;  echo "obj/Release/src/host/lua-5.3.0/src/lutf8lib.o" ;  echo "obj/Release/src/host/lua-5.3.0/src/lvm.o" ;  echo "obj/Release/src/host/lua-5.3.0/src/lzio.o" ;  echo "obj/Release/src/host/os_chdir.o" ;  echo "obj/Release/src/host/os_copyfile.o" ;  echo "obj/Release/src/host/os_getcwd.o" ;  echo "obj/Release/src/host/os_is64bit.o" ;  echo "obj/Release/src/host/os_isdir.o" ;  echo "obj/Release/src/host/os_isfile.o" ;  echo "obj/Release/src/host/os_match.o" ;  echo "obj/Release/src/host/os_mkdir.o" ;  echo "obj/Release/src/host/os_pathsearch.o" ;  echo "obj/Release/src/host/os_rmdir.o" ;  echo "obj/Release/src/host/os_stat.o" ;  echo "obj/Release/src/host/os_ticks.o" ;  echo "obj/Release/src/host/os_uuid.o" ;  echo "obj/Release/src/host/path_getabsolute.o" ;  echo "obj/Release/src/host/path_getrelative.o" ;  echo "obj/Release/src/host/path_helpers.o" ;  echo "obj/Release/src/host/path_isabsolute.o"
;  echo "obj/Release/src/host/premake.o" ;  echo "obj/Release/src/host/premake_main.o" ;  echo "obj/Release/src/host/scripts.o" ;  echo "obj/Release/src/host/string_endswith.o" ;  echo "obj/Release/src/host/string_hash.o" ;

@KageKirin
Copy link
Contributor Author

Thanks for reviewing, and sorry wasting your time with a broken commit.
I just remembered that when I tested it on Windows, I ran make from a bash shell on Windows, not a Windows Command prompt... my bad for not being thourough.

Anyway, I have an idea for a fix that I will implement tomorrow, and then test on Windows (DOS shell and Powershell) as well.
I'll update this PR after testing.

KageKirin added 3 commits July 9, 2019 11:03
This fixes issues with many objects yielding a command line too long.
@KageKirin KageKirin force-pushed the makefile-attfile-perline branch from 41ab1a1 to b5a3826 Compare July 11, 2019 02:45
@KageKirin
Copy link
Contributor Author

@rhoot @bkaradzic
Ok, so I fixed the issue and completely changed the PR branch's history.

I also removed 41ab1a1 b/c it did not make any sense after my fixes.

The FPRINT macro (daaad72) is now a $(shell) call, with the net benefit of being executed once per $(foreach ) iteration.
And the Windows variant of FPRINT consists in echo.$(1), which with $(1) empty results in just an empty line.

Please not that I have a follow up commit already prepared here:
https://github.comKageKirin/GENie/tree/makefile-wholearchive-attfile
If you prefer, I can also close this PR and submit the for the branch above.

@KageKirin
Copy link
Contributor Author

Ok, closing this one in favor just merging both changes with #458 instead.

@KageKirin KageKirin closed this Jul 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants