WIP: A horrible hack to deal with packages with too many srcs#3390
WIP: A horrible hack to deal with packages with too many srcs#3390toastwaffle wants to merge 1 commit intothought-machine:masterfrom
Conversation
| env["SRCS"] = strings.Join(sources, " ") | ||
| } else { | ||
| // Set a value for the variable to make silent failure less likely | ||
| env["SRCS"] = "__BUILD_TARGET_HAS_TOO_MANY_SRCS__" |
There was a problem hiding this comment.
This behaviour kind of leaves build actions in the lurch - in the general case, there's no way they can recover from this state, so there's no opportunity for them to succeed, and debugging the root cause from the user's perspective is going to be difficult.
The problem here is that the length of the envp string exceeds ARG_MAX, so what if we instead use the following logic:
- If
len(strings.Join(sources, " "))is less than or equal tosysconf(_SC_ARG_MAX), set the value ofSRCSenvironment variable (i.e., the current logic) - If
len(strings.Join(sources, " "))exceedssysconf(_SC_ARG_MAX), write the strings insourcesto a file in the working directory and set the value ofSRCSFILEto be the absolute path to this file
The current value of _SC_ARG_MAX could be retrieved using https://github.com/tklauser/go-sysconf or a similar module. It would only need to be retrieved once per process so we could cache the result somewhere. (It does have to be done dynamically though, because _SC_ARG_MAX varies by OS/kernel and can be tuned.)
Build rules would then need to check whether SRCSFILE is defined, and fall back to SRCS if it isn't, although I suspect the number of cases in which ARG_MAX is being exceeded is small enough that the check would be redundant for most rules. To simplify things, we might even want to make the choice explicit - we could add a srcs_as_files parameter to build_rule to allow callers to indicate they want to receive the sources list via file rather than via the environment.
Similar logic could be followed for named sources (below).
There was a problem hiding this comment.
I think it would be better to make this wholly explicit - otherwise I'd be concerned about build targets potentially failing silently and producing invalid builds
please-build/go-rules#319