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

Concatenation of dict select expressions #12457

Closed
alandonovan opened this issue Nov 11, 2020 · 5 comments
Closed

Concatenation of dict select expressions #12457

alandonovan opened this issue Nov 11, 2020 · 5 comments
Labels
P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) team-Loading-API BUILD file and macro processing: labels, package(), visibility, glob type: feature request

Comments

@alandonovan
Copy link
Contributor

alandonovan commented Nov 11, 2020

(Re-reporting on behalf of @sitaktif; see bazelbuild/starlark#114.)

Starlark allows the concatenation of select statements that return lists, such as:

my_binary(
  name = "greatbin",
  deps = ["some", "deps"]
    + select({"//:special_cpu": ["//some:target_for_special_cpu"], "//conditions:default": []})
    + select({"//:special_os": ["//some:target_for_special_os"], "//conditions:default": []})
)

This is very handy when generating code, which commonly happens when writing bazel rules.

Unfortunately, one cannot concatenate selects that return dicts:

my_rule(
  name = "greatbin",
  extra_env = {"FROB_PATH": "somepath"}
    + select({"//:special_cpu": {"SPECIAL_CPUFLAGS": "myspecialcpu"}, "//conditions:default": {}})
    + select({"//:special_os": {"SPECIAL_OSFLAGS": "myspecialos"}, "//conditions:default": {}})
    + select({"//:debug_mode": {"DEBUG": "true"}, "//conditions:default": {}})
    + ...
)

as this results in the following error: type 'dict(label, string)' doesn't support select concatenation.

This error is hit both when concatenating a dict and a select(), and when concatenating two select()s.

Related to see bazelbuild/starlark#14.

@stepancheg
Copy link
Contributor

There's inconsistency here:

{1: 2} + {3: 4}

doesn't work, but these three work:

{1: 2} + select({"//conditions:default": {3: 4})
select({"//conditions:default": {1: 2}) + {3: 4}
select({"//conditions:default": {1: 2}) + select({"//conditions:default": {3: 4})

while intuitively they look equivalent.

@sitaktif
Copy link
Contributor

but these three work

{1: 2} + select({"//conditions:default": {3: 4})
select({"//conditions:default": {1: 2}) + {3: 4}
select({"//conditions:default": {1: 2}) + select({"//conditions:default": {3: 4})

@stepancheg if you look at #114, you will notice that this is incorrect: unless it changed very recently, the + operator is only supported for selects that return lists, not for ones that return dicts.

@brandjon brandjon added team-Build-Language P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) and removed team-Starlark untriaged labels Feb 19, 2021
bazel-io pushed a commit that referenced this issue Mar 1, 2022
Implements the Starlark spec addition of bazelbuild/starlark#215.

Work towards #12457

Closes #14540.

PiperOrigin-RevId: 431737269
apattidb pushed a commit to databricks/bazel that referenced this issue Mar 17, 2022
Implements the Starlark spec addition of bazelbuild/starlark#215.

Work towards bazelbuild#12457

Closes bazelbuild#14540.

PiperOrigin-RevId: 431737269
apattidb pushed a commit to databricks/bazel that referenced this issue Aug 12, 2022
Implements the Starlark spec addition of bazelbuild/starlark#215.

Work towards bazelbuild#12457

Closes bazelbuild#14540.

PiperOrigin-RevId: 431737269
copybara-service bot pushed a commit that referenced this issue Aug 24, 2022
Starlark has recentely added support for union operations over dictionaries (bazelbuild/starlark#215). The syntax is already supported in bazel as of #14540, but the same operation with selects of dictionaries is still usupported.

Related issue: #12457

Closes #15075.

PiperOrigin-RevId: 469827107
Change-Id: If82cfaf577db41efc2e9b55af47b0b1710badc10
apattidb pushed a commit to databricks/bazel that referenced this issue Aug 25, 2022
Implements the Starlark spec addition of bazelbuild/starlark#215.

Work towards bazelbuild#12457

Closes bazelbuild#14540.

PiperOrigin-RevId: 431737269
apattidb pushed a commit to databricks/bazel that referenced this issue Aug 25, 2022
Starlark has recentely added support for union operations over dictionaries (bazelbuild/starlark#215). The syntax is already supported in bazel as of bazelbuild#14540, but the same operation with selects of dictionaries is still usupported.

Related issue: bazelbuild#12457

Closes bazelbuild#15075.

PiperOrigin-RevId: 469827107
Change-Id: If82cfaf577db41efc2e9b55af47b0b1710badc10
apattidb pushed a commit to databricks/bazel that referenced this issue Sep 1, 2022
Implements the Starlark spec addition of bazelbuild/starlark#215.

Work towards bazelbuild#12457

Closes bazelbuild#14540.

PiperOrigin-RevId: 431737269
aiuto pushed a commit to aiuto/bazel that referenced this issue Oct 12, 2022
Starlark has recentely added support for union operations over dictionaries (bazelbuild/starlark#215). The syntax is already supported in bazel as of bazelbuild#14540, but the same operation with selects of dictionaries is still usupported.

Related issue: bazelbuild#12457

Closes bazelbuild#15075.

PiperOrigin-RevId: 469827107
Change-Id: If82cfaf577db41efc2e9b55af47b0b1710badc10
@brandjon brandjon added team-Loading-API BUILD file and macro processing: labels, package(), visibility, glob and removed team-Build-Language labels Nov 4, 2022
ankit-agarwal-ai pushed a commit to AppliedIntuition/bazel that referenced this issue Dec 1, 2022
Starlark has recentely added support for union operations over dictionaries (bazelbuild/starlark#215). The syntax is already supported in bazel as of bazelbuild#14540, but the same operation with selects of dictionaries is still usupported.

Related issue: bazelbuild#12457

Closes bazelbuild#15075.

PiperOrigin-RevId: 469827107
Change-Id: If82cfaf577db41efc2e9b55af47b0b1710badc10
@fffonion
Copy link

With starlark's support of | operator of dicts (bazelbuild/starlark#14), it would be
awesome to add union operator support to select | dict and select | select.
Currently it's recognized as binary operator:

		env = nfpm_envs | {
Error: unsupported binary operation: select | dict

@fmeum
Copy link
Collaborator

fmeum commented Jan 27, 2023

This is available in Bazel 6: ebae486

@sgowroji Can be closed.

@sgowroji
Copy link
Member

We are closing this FR with respect to the above commit. Please feel free to reach us for any further information. Thanks !

apattidb pushed a commit to databricks/bazel that referenced this issue Apr 24, 2023
Implements the Starlark spec addition of bazelbuild/starlark#215.

Work towards bazelbuild#12457

Closes bazelbuild#14540.

PiperOrigin-RevId: 431737269
apattidb pushed a commit to databricks/bazel that referenced this issue Apr 24, 2023
Starlark has recentely added support for union operations over dictionaries (bazelbuild/starlark#215). The syntax is already supported in bazel as of bazelbuild#14540, but the same operation with selects of dictionaries is still usupported.

Related issue: bazelbuild#12457

Closes bazelbuild#15075.

PiperOrigin-RevId: 469827107
Change-Id: If82cfaf577db41efc2e9b55af47b0b1710badc10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) team-Loading-API BUILD file and macro processing: labels, package(), visibility, glob type: feature request
Projects
None yet
Development

No branches or pull requests

8 participants