Skip to content

WIP: A horrible hack to deal with packages with too many srcs #3390

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

toastwaffle
Copy link
Contributor

@toastwaffle toastwaffle commented Jul 18, 2025

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__"
Copy link
Contributor

Choose a reason for hiding this comment

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

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 to sysconf(_SC_ARG_MAX), set the value of SRCS environment variable (i.e., the current logic)
  • If len(strings.Join(sources, " ")) exceeds sysconf(_SC_ARG_MAX), write the strings in sources to a file in the working directory and set the value of SRCSFILE to 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).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

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.

2 participants