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

merger: attributes with multiple dictionaries are not kept #55

Closed
jayconrod opened this issue Dec 27, 2017 · 0 comments
Closed

merger: attributes with multiple dictionaries are not kept #55

jayconrod opened this issue Dec 27, 2017 · 0 comments
Labels

Comments

@jayconrod
Copy link
Contributor

mergeExpr breaks an expression into a list and a dictionary (either of which may be empty) with exprListAndDict. This worked well before full multi-platform support was added, but now, generated expressions may have multiple select expressions, so this no longer works.

exprListAndDict will return an error when it encounters an expression it does not understand. When mergeRule gets this error, it will return the generated expression and discard the old expression, regardless of # keep comments.

Two things should be fixed:

  • exprListAndDict should be updated to handle expressions with generic, OS-specific, arch-specific, and platform-specific strings (it will need to inspect keys in select statements to do this). Each collection should be merged independently.
  • When exprListAndDict returns an error, mergeExpr should return the old expression if there are any # keep comments. Otherwise, it should return the generated expression. It should probably not return an error in any case.
@jayconrod jayconrod added the bug label Dec 27, 2017
jayconrod added a commit to jayconrod/bazel-gazelle that referenced this issue Dec 29, 2017
Gazelle can now merge expressions that contain multiple select
sub-expressions. Selects are identified as os-specific, arch-specific,
or platform-specific, based on the labels used in the select
dict. The dicts are merged independently.

Fixes bazelbuild#55
jayconrod added a commit that referenced this issue Jan 2, 2018
Gazelle can now merge expressions that contain multiple select
sub-expressions. Selects are identified as os-specific, arch-specific,
or platform-specific, based on the labels used in the select
dict. The dicts are merged independently.

Fixes #55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant