emacs-bazel-mode icon indicating copy to clipboard operation
emacs-bazel-mode copied to clipboard

Bazel mode doesn't automatically highlight all .bazel files.

Open yesudeep opened this issue 3 years ago • 6 comments

Some build files that are written for third party libraries don't get bazel mode enabled. I needed to add:

(add-to-list 'auto-mode-alist
             '("\\.bazel" . bazel-build-mode))

to my configuration to enable bazel mode for those files.

Upon browsing the code, I've noticed the following works as well.

(add-to-list 'auto-mode-alist
             ;; https://docs.bazel.build/versions/3.0.0/build-ref.html#packages
             (cons (rx (or "/BUILD" ".bazel") eos) #'bazel-build-mode))

Therefore I've attached a PR that addresses this issue.

Would it be possible to add this directly to the mode?

yesudeep avatar Oct 05 '21 04:10 yesudeep

Is this actually correct @laurentlb ? AFAIK we'd want WORKSPACE.bazel to be visited in workspace mode, not BUILD file mode. What other files ending in .bazel are there except BUILD.bazel and WORKSPACE.bazel?

phst avatar Oct 06 '21 08:10 phst

Heya @phst, I've seen libraries use both .bazel and .BUILD for build files that are added for third party repositories that have not incorporated bazel. Here are a couple examples:

  1. rules_rust generated https://github.com/mathetake/bazel-wasmtime-c-api/blob/03dea33441a2d88fd494556da0f2e690b7cd5f14/bazel/cargo/crates.bzl

  2. Envoy https://github.com/envoyproxy/envoy/blob/d60f86c6b54e4ba376f67474d4e5a11f2c8d4dfb/bazel/repositories.bzl#L2822.

The only reason I haven't added the .BUILD file extension is because other build systems also use it (e.g. meson). The .bazel extension makes it unambiguous.

yesudeep avatar Oct 06 '21 15:10 yesudeep

Hi @phst I've updated my PR to use predicates instead of regular expressions to determine whether a visited buffer file is a WORKSPACE or a BUILD file. Using that with magic-mode-alist instead of auto-mode-alist appears to use bazel-workspace-mode for workspace files and bazel-build-mode for build files.

Could you please take a look at the PR? https://github.com/bazelbuild/emacs-bazel-mode/pull/310

yesudeep avatar Oct 07 '21 02:10 yesudeep

Thanks for the pointer to https://github.com/mathetake/bazel-wasmtime-c-api/blob/03dea33441a2d88fd494556da0f2e690b7cd5f14/bazel/cargo/crates.bzl. From what I can see, all the files in https://github.com/mathetake/bazel-wasmtime-c-api/tree/03dea33441a2d88fd494556da0f2e690b7cd5f14/bazel/cargo/remote are of the form BUILD.*.bazel, so wouldn't it be enough to add a pattern that matches that form? I'm trying to find a balance here between auto-detecting files as correctly as possible and avoiding too much complexity or "magic." Are there many examples of files that can't be matched by a set of specific regexp pattern? I unfortunately don't have a good overview what typical conventions exist.

phst avatar Oct 09 '21 17:10 phst

tl;dr: I'd say it's far easier to strike a balance by having Bazel (and tools around it) formally use .bazel and .bzl file extensions. It would avoid a lot of ambiguity, guess work and I/O. It would also help users have a more featureful experience.

emacs-bazel-mode

I'd suggest the simplest way to tackle this for emacs-bazel-mode would be to make these associations:

  1. BUILD, *.bazel (excluding WORKSPACE.bazel) => bazel-build-mode (alternative *.BUILD.bazel ?)
  2. WORKSPACE, WORKSPACE.bazel => bazel-workspace-mode

Conventions in current use

Re BUILD.*.bazel: I'm not sure that would work, because one can find examples of people using other filename formats as well:

  1. foo.BUILD
  2. foo.bazel
  3. BUILD.foo.bazel
  4. foo.BUILD.bazel
  5. BUILD.bazel
  6. BUILD

Here's an example: https://github.com/ttiurani/bazel-prost-build-bug/blob/fe4acaeccfe584b1f9f18ce35b9fe689198ab844/bazel/third_party/cargo/crates.bzl#L38

User experience and integration with other tools, build rules and third party libraries:

  1. As a user it would be much better to not have to guess at file formats. The extensions .bazel and .bzl immediately tell me that these are Bazel build files without having to open them up or setting editor mode comments (e.g. # -*- mode: bazel; -*- within them to disambiguate.
  2. Not having these associations disables useful features like syntax highlighting + autoformatting and therefore makes editing build files less fun and more error-prone.
  3. Using these extensions also simplifies tools one may use at the command line (e.g. "find all Bazel build files in X directory" or "grep for pattern within bazel files"). Not having to do file/network I/O to determine whether a file is a Bazel build file can avoid overhead (say one is editing files remotely over SSH).
  4. Vim already treats .bazel files as Bazel build files.
  5. rules_foreign_cc provides rules to integrate with different build systems (cmake, autotools, etc.) and if tomorrow one decides to integrate something like meson, one will have to deal with the ambiguity between meson's .build file and bazel's .BUILD file, especially on case insensitive file systems (e.g. Mac OS, Windows). Therefore, I've refrained from using .build or .BUILD in my PR even though one can find examples of its usage in projects.
  6. I also considered using third_party/library/BUILD.bazel for remote workspaces that have build files defined in the current workspace but I don't think one would want Bazel to treat these files as part of the current workspace as they can exist side-by-side with actual current workspace build rules. Therefore, these have to be named something other than BUILD.bazel even though they are still Bazel build files. Here's an example tree:
❯ tree 
WORKSPACE.bazel                              # Workspace file.
com_google_fruit/
├── BUILD.bazel                              # Current workspace build rules.
├── config.patch                             # Patch for third party library.
├── example.cc               
└── com_google_fruit.bazel                   # Build file to be used for third party library.

Summary

In summary, I'd say the following should disambiguate, reduce extension fragmentation, and avoid quite a bit of potential for naming conflicts, guess work, and I/O.

  1. Build files (current workspace): BUILD (can cause conflicts on case-insensitive file systems with out-of-tree build directories named build/ e.g. see CMake instructions), BUILD.bazel (recommended)
  2. Build files for remote workspaces defined within the current workspace: *.bazel (e.g. com_google_fruit.bazel) while excluding BUILD.bazel and WORKSPACE.bazel because those would imply the current workspace. It's also possible to consider *.BUILD.bazel, but I'm not sure whether that would be redundant (are there any other types of .bazel file?).
  3. Workspace files: WORKSPACE, WORKSPACE.bazel (recommended)
  4. Build definitions and actions: *.bzl

What do you think?

yesudeep avatar Oct 09 '21 18:10 yesudeep

I'd say it's far easier to strike a balance by having Bazel (and tools around it) formally use .bazel and .bzl file extensions.

I don't disagree, but I guess it would be better to discuss that in https://github.com/bazelbuild/bazel-website and have bazel.el follow conventions established elsewhere.

Re BUILD.*.bazel: I'm not sure that would work, because one can find examples of people using other filename formats as well:

Yeah, and we should definitely add patterns to match these conventions, e.g. something like foo.BUILD, BUILD.foo, each with an optional .bazel suffix, and similar for workspaces.

What do you think?

As said before, I think the conventions and recommendations should be documented in the main Bazel documentation. Some additions look unambiguous enough, e.g. foo.BUILD.bazel or BUILD.foo.bazel, so feel free to add them right away.

Build files for remote workspaces defined within the current workspace: *.bazel (e.g. com_google_fruit.bazel) while excluding BUILD.bazel and WORKSPACE.bazel because those would imply the current workspace. It's also possible to consider *.BUILD.bazel, but I'm not sure whether that would be redundant (are there any other types of .bazel file?).

I'd exclude *.bazel filenames that don't contain BUILD anywhere, they might also contain remote workspace definitions. Or is there some widely-used convention that these files unambiguously refer to BUILD files? At least the documented examples I found (https://docs.bazel.build/versions/main/repo/http.html#http_archive, https://docs.bazel.build/versions/main/external.html#non-bazel-projects, https://docs.bazel.build/versions/4.2.1/be/workspace.html#new_local_repository) all used either foo.BUILD or BUILD.foo, but not foo.bazel.

To summarize, my suggestion would be to detect BUILD, foo.BUILD, BUILD.foo (each with an optional .bazel extension) as BUILD files, and the same with WORKSPACE instead of BUILD.

The conflict with Meson and other build systems is somewhat annoying, unfortunately, but I think it's unavoidable since the "filename pattern space" is too limited.

phst avatar Nov 01 '21 11:11 phst