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

Add support for Bazel #2505

Merged
merged 1 commit into from
Sep 24, 2021
Merged

Add support for Bazel #2505

merged 1 commit into from
Sep 24, 2021

Conversation

Vertexwahn
Copy link
Contributor

@Vertexwahn Vertexwahn commented Sep 17, 2021

Goal of this PR

This PR helps users of the Bazel build system to integrate fmt into their builds.

Supporting two build systems (CMake and Bazel) is of course more effort to maintain but helps to spread fmt.
In the worst case the Bazel build does simply not work - in the best case, someone who uses Bazel can benefit from it.

Also, other projects support CMake and Bazel side by side (e.g. Catch2, GoogleTest, OpenEXR, oneTBB etc.)

Demo

A Demo of how to use this can be found here.

Everything a user has to do is:

Create a WORKSPACE.bazel file with the following content:

load("@bazel_tools//tools/build_defs/repo:git.bzl", "git_repository")

# Fetch fmt
git_repository(
    name = "fmt",
    branch = "add-bazel-support",
    remote = "https://github.com/Vertexwahn/fmt",
)

Create a BUILD.bazel file and add a dependency to fmt:

cc_binary( # Build a binary
    name = "Demo", # Name of the Binary
    srcs = ["main.cpp"], # List of files - we only have main.cpp
    deps = ["@fmt//:fmt"], # Depend on fmt
)

Make use of fmt in main.cpp:

#include "fmt/core.h"

#include <iostream>

int main() {
    std::string message = fmt::format("The answer is {}.", 42);

    std::cout << message << std::endl;
    
    return 0;
}

Tested with: GCC 9.3.0, Clang12, MSVC 2019

Support

I am willing to support the Bazel build support of fmt for a half year - after this period I can reconsider doing it for another half year

@vitaut
Copy link
Contributor

vitaut commented Sep 18, 2021

Thanks for the PR. I don't think we should put these build files in the top-level directory but if they can be moved to the support directory where we already have gradle files, I'll be happy to merge.

@Vertexwahn
Copy link
Contributor Author

Vertexwahn commented Sep 18, 2021

Bazel requires that the WORKSPACE.bazel file is placed in the root folder. Moving it e.g. tosupport/bazel/WORKSPACE.bazel does not work, since Bazel can only access files in subfolders of the WORKSPACE file.

The BUILD file can also access only the subdirectories. Therefore, I see currently now way to move it away from the root folder.

The .bazelrc is kind of optional. It could simply be deleted. In its current form, it just avoids that some temporary Bazel-related folders are created.

The .bazelversion file can also be deleted. It is just a file for Bazelisk and a nice way to document which Bazel version was aimed for/tested with (I am pretty sure that it works with almost every Bazel version).

If you want to keep the number of files in the root folder minimal I can easily delete .bazelversion and .bazelrc. Nevertheless, WORKSPACE.bazel and BUILD.bazel need to stay.

A different approach would be to move the WORKSPACE.bazel and BUILD.bazel files to some folder (e.g. support/bazel/) and add a README.md to explain that those files have to be copied over to the root folder to get the whole thing working with Bazel.

Having the WORSPACE and BUILD files in the root folder is really the game-changer. This way the above-shown example works out of the box.

I see three options:

  1. Keep it as it is (best option in my opinion)
  2. Keep WORSPACE and BUILD files in the root folder - get rid of .bazelversion and .bazelrc (a good alternative in my opinion to the first approach)
  3. Move Bazel related files to support/bazel (better than nothing, but not very practical and does not allow easy integration as shown in the example)

What option do you prefer? 1, 2, 3?

@vitaut
Copy link
Contributor

vitaut commented Sep 19, 2021

I prefer going with option 3. One extra step to copy build files is not critical and keeping the project directory clean is important.

@Vertexwahn
Copy link
Contributor Author

Ok, I changed everything to option 3. I also found a way to use the current state out of the box with Bazel. Details about this can be found in the provided README.md file under section Alternative approach.

support/bazel/README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@alexezeder alexezeder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few IMHOs, feel free to ignore 😉

support/bazel/BUILD.bazel Show resolved Hide resolved
support/bazel/README.md Outdated Show resolved Hide resolved
support/bazel/README.md Outdated Show resolved Hide resolved
support/bazel/README.md Outdated Show resolved Hide resolved
support/bazel/README.md Outdated Show resolved Hide resolved
support/bazel/README.md Outdated Show resolved Hide resolved
support/bazel/README.md Outdated Show resolved Hide resolved
support/bazel/WORKSPACE.bazel Show resolved Hide resolved
@Vertexwahn
Copy link
Contributor Author

Vertexwahn commented Sep 22, 2021

Added changes as requested. Header-only version is currently not part of this. Can be one of the next steps.

support/bazel/BUILD.bazel Show resolved Hide resolved
support/bazel/README.md Outdated Show resolved Hide resolved
@vitaut vitaut merged commit d9a731d into fmtlib:master Sep 24, 2021
@vitaut
Copy link
Contributor

vitaut commented Sep 24, 2021

Thank you

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

Successfully merging this pull request may close these issues.

3 participants