Skip to content
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

Consider reducing the number of attributes in or sharding sh_posix_toolchain #13

Closed
michajlo opened this issue Mar 12, 2020 · 2 comments · Fixed by #14
Closed

Consider reducing the number of attributes in or sharding sh_posix_toolchain #13

michajlo opened this issue Mar 12, 2020 · 2 comments · Fixed by #14
Assignees

Comments

@michajlo
Copy link

See also bazelbuild/bazel#10953

sh_posix_toolchain has 180 attributes, which wound up breaking Bazel's attempt to cap the number of attributes per rule-class at (what we thought was a generous) 150: bazelbuild/bazel@b839a51. It was easy enough to raise the limit to accommodate sh_posix_toolchain in this case, but it won't always be that way due to misc implementation details.

Please consider if this rule really needs so many attributes. Capping growth will ensure this rule's continued viability. Reducing attributes will help reduce outliers, which helps us better optimize for the common case.

@aherrmann
Copy link
Member

sh_posix_toolchain has one attribute per available command

rules_sh/sh/posix.bzl

Lines 13 to 14 in 0c274ad

# List of Unix commands as specified by IEEE Std 1003.1-2008.
# Extracted from https://en.wikipedia.org/wiki/List_of_Unix_commands.

It should be possible to change this to a single string_dict attribute instead.

@smelc
Copy link
Contributor

smelc commented Mar 25, 2020

rules_nixpkgs's nixpkgs.bzl must be updated accordingly when this change is performed, as it depends on sh_posix_toolchain prototype: https://github.com/tweag/rules_nixpkgs/blob/451876c1521612409d83865166cfdcb5dc7fcb0c/nixpkgs/nixpkgs.bzl#L507-L511

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants