protobuf
protobuf copied to clipboard
protocolbuffers
Fail with Bazel future changes
According to Bazel CI: https://buildkite.com/bazel/bazel-at-release-plus-incompatible-flags/builds/95#09a501c4-e212-4885-908f-889ef0fd9524
Protobuf compilation when using Bazel flag --incompatible_merge_genfiles_directory on Windows.
This flag removes the distinction between bazel-bin and bazel-genfiles.
Some tests fail: https://storage.googleapis.com/bazel-untrusted-buildkite-artifacts/09a501c4-e212-4885-908f-889ef0fd9524/..%5C..%5Cprotobuf_test%5Cattempt_2.log
We plan to enable the flag in Bazel soon.
Since it fails on Windows only, do you know if your tests behave differently on Windows?
No, I don't know of any significant differences to how the tests are run on Windows. Looking through the failed tests it appears that we get errors any time we try to open a file. Is it possible that this flag changes the working directory of the test runs? That is one thing I could imagine would cause us to be unable to open files where we expect them to be.
Those are the files are originally in bazel-genfiles but moved to bazel-bin folder with this flag.
pcloudy@pcloudy0-w MSYS ~/workspace/protobuf
$ tree bazel-genfiles
bazel-genfiles
└── src
└── google
└── protobuf
├── any_test.pb.cc
├── any_test.pb.h
├── compiler
│ └── cpp
│ ├── cpp_test_bad_identifiers.pb.cc
│ ├── cpp_test_bad_identifiers.pb.h
│ ├── cpp_test_large_enum_value.pb.cc
│ └── cpp_test_large_enum_value.pb.h
├── map_lite_unittest.pb.cc
├── map_lite_unittest.pb.h
├── map_proto2_unittest.pb.cc
├── map_proto2_unittest.pb.h
├── map_unittest.pb.cc
├── map_unittest.pb.h
├── unittest.pb.cc
├── unittest.pb.h
├── unittest_arena.pb.cc
├── unittest_arena.pb.h
├── unittest_custom_options.pb.cc
├── unittest_custom_options.pb.h
├── unittest_drop_unknown_fields.pb.cc
├── unittest_drop_unknown_fields.pb.h
├── unittest_embed_optimize_for.pb.cc
├── unittest_embed_optimize_for.pb.h
├── unittest_empty.pb.cc
├── unittest_empty.pb.h
├── unittest_enormous_descriptor.pb.cc
├── unittest_enormous_descriptor.pb.h
├── unittest_import.pb.cc
├── unittest_import.pb.h
├── unittest_import_lite.pb.cc
├── unittest_import_lite.pb.h
├── unittest_import_public.pb.cc
├── unittest_import_public.pb.h
├── unittest_import_public_lite.pb.cc
├── unittest_import_public_lite.pb.h
├── unittest_lazy_dependencies.pb.cc
├── unittest_lazy_dependencies.pb.h
├── unittest_lazy_dependencies_custom_option.pb.cc
├── unittest_lazy_dependencies_custom_option.pb.h
├── unittest_lazy_dependencies_enum.pb.cc
├── unittest_lazy_dependencies_enum.pb.h
├── unittest_lite.pb.cc
├── unittest_lite.pb.h
├── unittest_lite_imports_nonlite.pb.cc
├── unittest_lite_imports_nonlite.pb.h
├── unittest_mset.pb.cc
├── unittest_mset.pb.h
├── unittest_mset_wire_format.pb.cc
├── unittest_mset_wire_format.pb.h
├── unittest_no_arena.pb.cc
├── unittest_no_arena.pb.h
├── unittest_no_arena_import.pb.cc
├── unittest_no_arena_import.pb.h
├── unittest_no_arena_lite.pb.cc
├── unittest_no_arena_lite.pb.h
├── unittest_no_field_presence.pb.cc
├── unittest_no_field_presence.pb.h
├── unittest_no_generic_services.pb.cc
├── unittest_no_generic_services.pb.h
├── unittest_optimize_for.pb.cc
├── unittest_optimize_for.pb.h
├── unittest_preserve_unknown_enum.pb.cc
├── unittest_preserve_unknown_enum.pb.h
├── unittest_preserve_unknown_enum2.pb.cc
├── unittest_preserve_unknown_enum2.pb.h
├── unittest_proto3.pb.cc
├── unittest_proto3.pb.h
├── unittest_proto3_arena.pb.cc
├── unittest_proto3_arena.pb.h
├── unittest_proto3_arena_lite.pb.cc
├── unittest_proto3_arena_lite.pb.h
├── unittest_proto3_lite.pb.cc
├── unittest_proto3_lite.pb.h
├── unittest_well_known_types.pb.cc
├── unittest_well_known_types.pb.h
└── util
├── internal
│ └── testdata
│ ├── anys.pb.cc
│ ├── anys.pb.h
│ ├── books.pb.cc
│ ├── books.pb.h
│ ├── default_value.pb.cc
│ ├── default_value.pb.h
│ ├── default_value_test.pb.cc
│ ├── default_value_test.pb.h
│ ├── field_mask.pb.cc
│ ├── field_mask.pb.h
│ ├── maps.pb.cc
│ ├── maps.pb.h
│ ├── oneofs.pb.cc
│ ├── oneofs.pb.h
│ ├── proto3.pb.cc
│ ├── proto3.pb.h
│ ├── struct.pb.cc
│ ├── struct.pb.h
│ ├── timestamp_duration.pb.cc
│ ├── timestamp_duration.pb.h
│ ├── wrappers.pb.cc
│ └── wrappers.pb.h
├── json_format.pb.cc
├── json_format.pb.h
├── json_format_proto3.pb.cc
├── json_format_proto3.pb.h
├── message_differencer_unittest.pb.cc
└── message_differencer_unittest.pb.h
Another thing I noticed, if you do:
bazel test //:protobuf_test --incompatible_merge_genfiles_directory
bazel test //:protobuf_test
The test fails in the second command as well, so I suspect the generated files under bazel-bin some how changed the order of header files or something.
The command line to compile a test source file with genfiles directory is:
cl.exe
/nologo
/DCOMPILER_MSVC
/DNOMINMAX
/D_WIN32_WINNT=0x0601
/D_CRT_SECURE_NO_DEPRECATE
/D_CRT_SECURE_NO_WARNINGS
/bigobj
/Zm500
/EHsc
/wd4351
/wd4291
/wd4250
/wd4996
/I.
/Ibazel-out/x64_windows-fastbuild/genfiles
/Ibazel-out/x64_windows-fastbuild/bin
/Iexternal/submodule_gmock
/Ibazel-out/x64_windows-fastbuild/genfiles/external/submodule_gmock
/Ibazel-out/x64_windows-fastbuild/bin/external/submodule_gmock
/Iexternal/bazel_tools
/Ibazel-out/x64_windows-fastbuild/genfiles/external/bazel_tools
/Ibazel-out/x64_windows-fastbuild/bin/external/bazel_tools
/Isrc
/Ibazel-out/x64_windows-fastbuild/genfiles/src
/Ibazel-out/x64_windows-fastbuild/bin/src
/Iexternal/submodule_gmock/googlemock
/Ibazel-out/x64_windows-fastbuild/genfiles/external/submodule_gmock/googlemock
/Ibazel-out/x64_windows-fastbuild/bin/external/submodule_gmock/googlemock
/Iexternal/submodule_gmock/googlemock/include
/Ibazel-out/x64_windows-fastbuild/genfiles/external/submodule_gmock/googlemock/include
/Ibazel-out/x64_windows-fastbuild/bin/external/submodule_gmock/googlemock/include
/Iexternal/submodule_gmock/googletest
/Ibazel-out/x64_windows-fastbuild/genfiles/external/submodule_gmock/googletest
/Ibazel-out/x64_windows-fastbuild/bin/external/submodule_gmock/googletest
/Iexternal/submodule_gmock/googletest/include
/Ibazel-out/x64_windows-fastbuild/genfiles/external/submodule_gmock/googletest/include
/Ibazel-out/x64_windows-fastbuild/bin/external/submodule_gmock/googletest/include
/showIncludes
/MD
/Od
/Z7
/wd4117
-D__DATE__="redacted"
-D__TIMESTAMP__="redacted"
-D__TIME__="redacted"
/DHAVE_PTHREAD
/wd4018
/wd4065
/wd4146
/wd4244
/wd4251
/wd4267
/wd4305
/wd4307
/wd4309
/wd4334
/wd4355
/wd4506
/wd4514
/wd4800
/wd4996
/Fobazel-out/x64_windows-fastbuild/bin/_objs/protobuf_test/cpp_bootstrap_unittest.obj
/c
src/google/protobuf/compiler/cpp/cpp_bootstrap_unittest.cc
Without genfiles directory, it is:
cl.exe
/nologo
/DCOMPILER_MSVC
/DNOMINMAX
/D_WIN32_WINNT=0x0601
/D_CRT_SECURE_NO_DEPRECATE
/D_CRT_SECURE_NO_WARNINGS
/bigobj
/Zm500
/EHsc
/wd4351
/wd4291
/wd4250
/wd4996
/I.
/Ibazel-out/x64_windows-fastbuild/bin
/Iexternal/submodule_gmock
/Ibazel-out/x64_windows-fastbuild/bin/external/submodule_gmock
/Iexternal/bazel_tools
/Ibazel-out/x64_windows-fastbuild/bin/external/bazel_tools
/Isrc
/Ibazel-out/x64_windows-fastbuild/bin/src
/Iexternal/submodule_gmock/googlemock
/Ibazel-out/x64_windows-fastbuild/bin/external/submodule_gmock/googlemock
/Iexternal/submodule_gmock/googlemock/include
/Ibazel-out/x64_windows-fastbuild/bin/external/submodule_gmock/googlemock/include
/Iexternal/submodule_gmock/googletest
/Ibazel-out/x64_windows-fastbuild/bin/external/submodule_gmock/googletest
/Iexternal/submodule_gmock/googletest/include
/Ibazel-out/x64_windows-fastbuild/bin/external/submodule_gmock/googletest/include
/showIncludes
/MD
/Od
/Z7
/wd4117
-D__DATE__="redacted"
-D__TIMESTAMP__="redacted"
-D__TIME__="redacted"
/DHAVE_PTHREAD
/wd4018
/wd4065
/wd4146
/wd4244
/wd4251
/wd4267
/wd4305
/wd4307
/wd4309
/wd4334
/wd4355
/wd4506
/wd4514
/wd4800
/wd4996
/Fobazel-out/x64_windows-fastbuild/bin/_objs/protobuf_test/cpp_bootstrap_unittest.obj
/c
src/google/protobuf/compiler/cpp/cpp_bootstrap_unittest.cc
I think I finally see what is going on. The logic for finding the top of the directory tree on Windows effectively just cd's upward until it finds src/google/protobuf, and with this change it will find the wrong directory because it will see src/google/protobuf inside bazel-bin. I haven't tested this out because I don't have easy access to a Windows build machine but this makes sense and lines up with @meteorcloudy's observation. I guess the question is, what's the proper way for us to figure out the path to the top of the project source tree? The tests just need to do this so that they can find the path to test data files in src/google/protobuf/testdata.
@acozzette Sorry for the late reply here, I think you should use the C++ runfiles library provided by Bazel to look up for test data files: See the usage here: https://github.com/bazelbuild/bazel/blob/master/tools/cpp/runfiles/runfiles_src.h#L15 @laszlocsomor can help runfiles library related question if you have any.
BTW, why didn't it see src/google/protobuf inside bazel-genfiles before?
@meteorcloudy My guess was that the working directory for test runs must have been inside bazel-bin/ and not bazel-genfiles/.
I just sent out #5927 to fix this. I don't have a good way to test whether the fix works but I think it should. I ended up not using the runfiles library because I realized it would be complicated to have to use a Bazel-specific library when building with Bazel but keep the other logic around for other build systems. Instead I tweaked the existing logic to look for a particular file (descriptor.cc) so that should prevent it from mistakenly using the bazel-bin/ directory.
@acozzette Thank you so much for the fix! I verified manually, it did fix this issue!
We triage inactive PRs and issues in order to make it easier to find active work. If this issue should remain active or becomes active again, please add a comment.
This issue is labeled inactive because the last activity was over 90 days ago.
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}