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

Add support for bazel #56

Merged
merged 3 commits into from
Oct 8, 2020
Merged

Add support for bazel #56

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
817a6ad
Add support for bazel
FelixDuvalletKodiak Oct 2, 2020
d5115cb
Update BUILD file & specify http_archive in README
FelixDuvalletKodiak Oct 2, 2020
09248eb
Lint BUILD file
FelixDuvalletKodiak Oct 3, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,6 @@ install_manifest.txt
compile_commands.json
CTestTestfile.cmake
_deps

### Bazel build artifacts ###
bazel-*
Copy link
Contributor

Choose a reason for hiding this comment

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

[Nit] This will ignore bazel-* in all directories, but you actually only care about the symlinks created at the root of the repo.

Suggested change
bazel-*
/bazel-*

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, thanks.

56 changes: 56 additions & 0 deletions BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
licenses(["notice"])

exports_files(["LICENSE"])
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

AFAIK bazel only has plans to support license files as part of the build, but since the repo provides it I am following (what I believe to be) an accepted convention of exporting it.
GoogleTest does this, for instance.


package(default_visibility = ["//visibility:public"])

cc_library(
name = "magic_enum",
hdrs = ["include/magic_enum.hpp"],
strip_include_prefix = "include",
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you tested actually consuming this as an external dependency? I ask because IIRC strip_include_prefix doesn’t work in that case because there will be an extra external/ to contend with. I have not tested that your change doesn’t work, but the below modification definitely does.

Suggested change
strip_include_prefix = "include",
includes = ["include"],

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's much nicer. Thanks for the suggestion, done.

)

cc_binary(
name = "example",
srcs = ["example/example.cpp"],
deps = [":magic_enum"],
copts = ["-std=c++17"],
)

cc_binary(
name = "enum_flag_example",
srcs = ["example/enum_flag_example.cpp"],
deps = [":magic_enum"],
copts = ["-std=c++17"],
)

cc_binary(
name = "example_custom_name",
srcs = ["example/example_custom_name.cpp"],
deps = [":magic_enum"],
copts = ["-std=c++17"],
)

cc_library(
name = "catch",
srcs = [],
hdrs = ["test/3rdparty/Catch2/catch.hpp"],
strip_include_prefix = "test/3rdparty/Catch2",
)

cc_test(
name = "test",
srcs = [
"test/test.cpp",
],
deps = [":magic_enum", ":catch"],
copts = ["-std=c++17"],
)
cc_test(
name = "test_flags",
srcs = [
"test/test_flags.cpp",
],
deps = [":magic_enum", ":catch"],
copts = ["-std=c++17"],
)
11 changes: 11 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,17 @@ CPMAddPackage(
)
```

Bazel is also supported, simply add to your WORKSPACE file:
```
git_repository(
Copy link
Contributor

Choose a reason for hiding this comment

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

http_archive is preferred so I would use that as the example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion, done.

name = "magic_enum",
commit = <>,
remote = "https://github.com/Neargye/magic_enum",
)
```
To use bazel inside the repository it's possible to do:
`export CC=clang ; bazel build //... ; bazel test //... ; bazel run //:example`
Copy link
Contributor

Choose a reason for hiding this comment

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

Specifying clang is confusing, since right below it says that multiple compilers are supported.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated the comment, please let me know what you think.
I initially ran into an issue because the system-default compiler is not supported, but you're right that any supported can be used.


## Compiler compatibility

* Clang/LLVM >= 5
Expand Down
Empty file added WORKSPACE
View file
Open in desktop
Empty file.