Bazel support #83
Bazel support #83
Conversation
Codecov Report
@@ Coverage Diff @@
## master #83 +/- ##
=======================================
Coverage 94.63% 94.63%
=======================================
Files 54 54
Lines 12524 12524
=======================================
Hits 11852 11852
Misses 672 672
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
Thanks for the PR, and sorry for the delay -- github's notifications are really broken for me. I am not familiar with Bazel, but AFAICT the structure is OK, and I see that you took good care to deal with the build options. To merge this, adding a CI job would be needed, all the more because it hardcodes the lists of source files and tests, which would otherwise diverge from the main cmake project. Another thing that will be needed is adding notes to the main README. |
|
@zaucy do you intend to proceed with this PR? |
|
Yes I do! Thanks for the reminder |
zaucy
force-pushed
the
feat/bazel-support
branch
from
1071e1d
to
4f5b8a9
Compare
|
I added some bazel specific CI. The Test failure output |
|
The test failure is curious. It occurs on the last line of this test, so what's happening here is that But this works ok in every tested architecture/compiler in the unit tests (just look at the CI runs), including exotics as s390. I don't see how this could be related to this PR, and suspect instead that you caught something in your run setup. But I may be wrong. What is your cpu/platform/compiler? |
|
@zaucy thanks for all the hard work, your PR is very thorough. The remarks are all minor, and the main sticking point is to find out whether the problem with the failed test has anything to do with this changeset or with the bazel environment, which I suspect is not the case. |
|
I was mainly testing on Windows x86_64 using msvc cl. It also failed on my branches github actions for the I just ran the tests on a Linux machine and realized there are a few things that need to be configured for the tests still. I'll update my PR later today. |
|
ok, so given that all CI runs other than bazel are ok, this leads us to conclude that bazel, or the way it runs the tests, has something to do with the failure. Would you agree? If so, do you have an idea what may be happening here? |
|
It must be. I can't see why else. I'll investigate further later today. |
|
It also occurred to me that it may have something to do with how bazel compiles the code. The fact that it happens with |
I'm unaware of any differences in regards to that. I've extracted the compile/link commands that were generated by bazel. Does anything look like it would cause quiet NaN to differ? Compile commandsLink Commands |
|
I've looked through the compile flags and nothing there seems to warrant this behavior. One last idea/try on this front: in the failing test, add |
biojppm
force-pushed
the
feat/bazel-support
branch
from
1732c1b
to
94686b5
Compare
|
@zaucy I have rebased and pushed the quiet nan test to your branch. Let's see what the results are. |
biojppm
force-pushed
the
feat/bazel-support
branch
from
94686b5
to
e3d520d
Compare
|
I'm having trouble building the test using bazel: $ bazel build test/charconv
INFO: Analyzed target //test:charconv (27 packages loaded, 275 targets configured).
INFO: Found 1 target...
ERROR: /home/jpmag/proj/c4core/test/BUILD.bazel:76:8: Compiling test/test_charconv.cpp failed: (Exit 1): gcc failed: error executing command /usr/bin/gcc -U_FORTIFY_SOURCE -fstack-protector -Wall -Wunused-but-set-parameter -Wno-free-nonheap-object -fno-omit-frame-pointer '-std=c++0x' -MD -MF ... (remaining 33 arguments skipped)
Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
In file included from src/c4/std/vector.hpp:10,
from src/c4/std/std.hpp:6,
from test/test_charconv.cpp:4:
src/c4/substr.hpp: In instantiation of 'OStream& c4::operator<<(OStream&, basic_substring<C>) [with OStream = doctest::detail::MessageBuilder; C = char]':
test/test_charconv.cpp:2107:9: required from 'void c4::test_rtoa(substr, Real, int, const char*, const char*, const char*, const char*, const char*) [with Real = float; substr = basic_substring<char>]'
test/test_charconv.cpp:2143:14: required from here
src/c4/substr.hpp:1865:8: error: 'struct doctest::detail::MessageBuilder' has no member named 'write'
1865 | os.write(s.str, s.len);
| ~~~^~~~~
src/c4/substr.hpp: In instantiation of 'OStream& c4::operator<<(OStream&, basic_substring<C>) [with OStream = doctest::detail::MessageBuilder; C = const char]':
test/test_charconv.cpp:2454:5: required from 'void c4::to_chars_roundtrip(char (&)[N], csubstr) [with long unsigned int N = 128; csubstr = basic_substring<const char>]'
test/test_charconv.cpp:2527:23: required from here
src/c4/substr.hpp:1865:8: error: 'struct doctest::detail::MessageBuilder' has no member named 'write'
Target //test:charconv failed to build
Use --verbose_failures to see the command lines of failed build steps.
INFO: Elapsed time: 5.014s, Critical Path: 4.08s
INFO: 2 processes: 2 internal.
FAILED: Build did NOT complete successfullyI noticed the bazel workspace specifies doctest 2.4.8, while cmake will pick 2.4.6. However, then I tried building in cmake with doctest 2.4.8 and it builds without any problem. Do you know what may be going on in here? |
|
I saw that too. I haven't had a chance to investigate what the difference is. I'm not too familiar with doctest. It doesn't happen on Windows. |
biojppm
force-pushed
the
master
branch
2 times, most recently
from
88d2abd
to
51126b3
Compare
I'm using bazel to build my projects and I was hoping to use
c4coreandrapidyaml. I figured I'd share my bazel build files upstream. If this gets merged then including this repository in other bazel based projects becomes very trivial.If this gets approved I was hoping to also contribute the files necessary to add c4core and rapid yaml to the bazel central registry.
The test library
doctestalready had bazel support so I added the tests in as well! If you want I could also add some bazel-based CI jobs.If this PR gets a positive reception I'll make another one in the rapidyaml repository as well.
Let me know what you think!