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.

Sign up for GitHub

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

Jump to bottom

Gazelle reports "could not merge expression" when using select statements in hdrs or srcs #1735

Closed
hofbi opened this issue Feb 15, 2024 · 7 comments · Fixed by #1853
Closed

Gazelle reports "could not merge expression" when using select statements in hdrs or srcs #1735

hofbi opened this issue Feb 15, 2024 · 7 comments · Fixed by #1853

Comments

@hofbi
Copy link
Contributor

hofbi commented Feb 15, 2024

What version of gazelle are you using?

bazel_dep(name = "gazelle", version = "0.35.0")

What version of rules_go are you using?

bazel_dep(name = "rules_go", version = "0.46.0")

What version of Bazel are you using?

7.0.2

Does this issue reproduce with the latest releases of all the above?

Yes, thats what we use

What operating system and processor architecture are you using?

Ubuntu 20.04 x86

What did you do?

  1. I use a select statement in hdrs or srcs with @platforms//os:XXX as key
  2. For some platforms such as @platforms//os:windows, @platforms//os:linux, @platforms//os:iosthe merge works fine, while for others such as @platforms//os:qnx or @platforms//os:osx this results in a could not merge expression error

What did you expect to see?

The same behavior as for the other platforms

What did you see instead?

could not merge expression error
@hofbi
Copy link
Contributor Author

hofbi commented Feb 15, 2024

Maybe https://github.com/bazelbuild/bazel-gazelle/blob/ec1591cb193591b9544b52b98b8ce52833b34c58/rule/platform.go#L51 is a good starting point, since it looks like the issue happens for all platforms not part of this list.

monogon-bot pushed a commit to monogon-dev/monogon that referenced this issue Jul 24, 2024
@fionera
treewide: fix "could not merge expression" from gazelle
93ba807 
This isn't any real bugfix, but more of a style thing. Gazelle can't work with select or glob statements (bazel-contrib/bazel-gazelle#1735), but will work fine with them and even ignore them if set to # keep. This adds that directive to make gazelle less verbose when running.

Change-Id: I7c638867d6b566e1d398eef37aff2316bf90a00f
Reviewed-on: https://review.monogon.dev/c/monogon/+/3241
Reviewed-by: Leopold Schabel <[email protected]>
Tested-by: Jenkins CI
@hofbi
Copy link
Contributor Author

hofbi commented Aug 1, 2024

I still see this issue for Bazel 7.2.1 and Gazelle 0.37.0. Do you think it would be enough adding the missing platforms to https://github.com/bazelbuild/bazel-gazelle/blob/ec1591cb193591b9544b52b98b8ce52833b34c58/rule/platform.go#L51? I can see a depratation message there so not sure if I should add something there?

@fmeum
Copy link
Member

fmeum commented Aug 1, 2024

It's deprecated only for use outside of Go. Feel free to submit a PR to add new platforms!

@hofbi hofbi mentioned this issue Aug 1, 2024
Merged
@hofbi
Copy link
Contributor Author

hofbi commented Aug 1, 2024

Created #1853, working on the CLA

fmeum pushed a commit that referenced this issue Aug 6, 2024
@hofbi
Add QNX and OSX to known platforms (#1853)
32de0bd 
<!-- Thanks for sending a PR! Before submitting:

1. If this is your first PR, please read CONTRIBUTING.md and sign the
CLA
   first. We cannot review code without a signed CLA.
2. Please file an issue *first*. All features and most bug fixes should
have
an associated issue with a design discussed and decided upon. Small bug
   fixes and documentation improvements don't need issues.
3. New features and bug fixes must have tests. Documentation may need to
be updated. If you're unsure what to update, send the PR, and we'll
discuss
   in review.
-->

**What type of PR is this?**

> Uncomment one line below and remove others.
>
Bug fix

**What package or component does this PR mostly affect?**

language/go
cmd/gazelle

**What does this PR do? Why is it needed?**

Adding more known platforms to avoid issue reported in
#1735.

**Which issues(s) does this PR fix?**

Fixes #1735

**Other notes for review**
@fmeum
Copy link
Member

fmeum commented Aug 13, 2024

@hofbi Could you also submit a commit to rules_go that adds these platforms there? It looks like they don't exist yet and thus result in error messages: #1862 (comment)

@fmeum fmeum reopened this Aug 13, 2024
@hofbi hofbi mentioned this issue Aug 13, 2024
Merged
@hofbi
Copy link
Contributor Author

hofbi commented Aug 13, 2024

@fmeum I came up with bazel-contrib/rules_go#4036

fmeum pushed a commit to bazel-contrib/rules_go that referenced this issue Aug 13, 2024
@hofbi
Add qnx and osx platforms (#4036)
9d1456e 
**What type of PR is this?**

Bug fix

**What does this PR do? Why is it needed?**

**Which issues(s) does this PR fix?**

**Other notes for review**

This change was requested in
bazel-contrib/bazel-gazelle#1735 (comment)
tyler-french pushed a commit to bazel-contrib/rules_go that referenced this issue Aug 30, 2024
@hofbi @tyler-french
Add qnx and osx platforms (#4036)
569acef 
**What type of PR is this?**

Bug fix

**What does this PR do? Why is it needed?**

**Which issues(s) does this PR fix?**

**Other notes for review**

This change was requested in
bazel-contrib/bazel-gazelle#1735 (comment)
tyler-french added a commit to bazel-contrib/rules_go that referenced this issue Aug 30, 2024
@tyler-french
Add qnx and osx platforms (#4036)
eb9a7af 
**What type of PR is this?**

Bug fix

**What does this PR do? Why is it needed?**

**Which issues(s) does this PR fix?**

**Other notes for review**

This change was requested in
bazel-contrib/bazel-gazelle#1735 (comment)
@hofbi
Copy link
Contributor Author

hofbi commented Oct 5, 2024

#1853 has fixed this

@hofbi hofbi closed this as completed Oct 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add QNX and OSX to known platforms hofbi/bazel-gazelle
2 participants
@fmeum @hofbi