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

Parallel build failure due to automakes BUILT_SOURCES and subdir-objects #156

Closed
ncopa opened this issue Jul 31, 2014 · 6 comments
Closed

Comments

@ncopa
Copy link
Contributor

ncopa commented Jul 31, 2014

I get build failure on my 12 core (with HT so 24 "cpus") build server with -j24:

...
  GEN      t/test-full.pb.cc
  GEN      t/test-full.pb.h
  CXX      protoc-c/protoc_c_protoc_c-c_bytes_field.o
  CXX      protoc-c/protoc_c_protoc_c-c_enum.o
  CXX      protoc-c/protoc_c_protoc_c-c_enum_field.o
  CXX      protoc-c/protoc_c_protoc_c-c_extension.o
  CXX      protoc-c/protoc_c_protoc_c-c_field.o
  CXX      protoc-c/protoc_c_protoc_c-c_file.o
  CXX      protoc-c/protoc_c_protoc_c-c_generator.o
  CXX      protoc-c/protoc_c_protoc_c-c_helpers.o
  CXX      protoc-c/protoc_c_protoc_c-c_message.o
  CXX      protoc-c/protoc_c_protoc_c-c_message_field.o
  CXX      protoc-c/protoc_c_protoc_c-c_primitive_field.o
  CXX      protoc-c/protoc_c_protoc_c-c_service.o
  CXX      protoc-c/protoc_c_protoc_c-c_string_field.o
  CXX      protoc-c/protoc_c_protoc_c-main.o
  CXX      t/generated-code2/t_generated_code2_cxx_generate_packed_data-cxx-generate-packed-data.o
[libprotobuf ERROR google/protobuf/descriptor.cc:4153] "foo.VALUE_B" uses the same enum value as "foo.VALUE_A". If this is intended, set 'option allow_alias = true;' to the enum definition.
[libprotobuf ERROR google/protobuf/descriptor.cc:4153] "foo.VALUE_C" uses the same enum value as "foo.VALUE_A". If this is intended, set 'option allow_alias = true;' to the enum definition.
[libprotobuf ERROR google/protobuf/descriptor.cc:4153] "foo.VALUE_E" uses the same enum value as "foo.VALUE_D". If this is intended, set 'option allow_alias = true;' to the enum definition.
[libprotobuf ERROR google/protobuf/descriptor.cc:4153] "foo.VALUE_AA" uses the same enum value as "foo.VALUE_F". If this is intended, set 'option allow_alias = true;' to the enum definition.
[libprotobuf ERROR google/protobuf/descriptor.cc:4153] "foo.VALUE_B" uses the same enum value as "foo.VALUE_A". If this is intended, set 'option allow_alias = true;' to the enum definition.
[libprotobuf ERROR google/protobuf/descriptor.cc:4153] "foo.VALUE_C" uses the same enum value as "foo.VALUE_A". If this is intended, set 'option allow_alias = true;' to the enum definition.
[libprotobuf ERROR google/protobuf/descriptor.cc:4153] "foo.VALUE_E" uses the same enum value as "foo.VALUE_D". If this is intended, set 'option allow_alias = true;' to the enum definition.
[libprotobuf ERROR google/protobuf/descriptor.cc:4153] "foo.VALUE_AA" uses the same enum value as "foo.VALUE_F". If this is intended, set 'option allow_alias = true;' to the enum definition.
t/generated-code2/cxx-generate-packed-data.cc:5:28: fatal error: t/test-full.pb.h: No such file or directory
 #include "t/test-full.pb.h"
                            ^
compilation terminated.
  CXX      t/t_generated_code2_cxx_generate_packed_data-test-full.pb.o
Preprocessed source stored into /tmp/ccpjigCB.out file, please attach this to your bugreport.
Makefile:1351: recipe for target 't/generated-code2/t_generated_code2_cxx_generate_packed_data-cxx-generate-packed-data.o' failed
make: *** [t/generated-code2/t_generated_code2_cxx_generate_packed_data-cxx-generate-packed-data.o] Error 1
make: *** Waiting for unfinished jobs....

I dont know if this is an automake bug or "feature". It seems like this row in Makefile.am is supposed to make sure that the generated t/test-full.pb.h is built first:

t/generated-code2/cxx-generate-packed-data.$(OBJEXT): t/test-full.pb.h

but it seems like automake adds some extra prefix due to building things in subdirs (and enabling 'subdir-objects' in with AM_INIT_AUTOMAKE) so there should probably be another line with:

--- ./Makefile.am.orig
+++ ./Makefile.am
@@ -142,6 +142,7 @@
        t/generated-code2/cxx-generate-packed-data.cc \
        t/test-full.pb.cc
 t/generated-code2/cxx-generate-packed-data.$(OBJEXT): t/test-full.pb.h
+t/generated-code2/t_generated_code2_cxx_generate_packed_data-cxx-generate-packed-data.$(OBJEXT): t/test-full.pb.h
 t_generated_code2_cxx_generate_packed_data_CXXFLAGS = \
        $(AM_CXXFLAGS) \
        $(protobuf_CFLAGS)
@lipnitsk
Copy link
Member

@ncopa, can you submit a Pull Request so we can review and merge in your fix? Do you think it would be possible to test this issue in the Travis CI environment?

@edmonds
Copy link
Member

edmonds commented Sep 5, 2014

This is a bug that got overlooked in the conversion to subdir-objects. I'm guessing something like this is the right way to express this:

diff --git a/Makefile.am b/Makefile.am
index 3ec4b6e..c5dd657 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -141,7 +141,7 @@ noinst_PROGRAMS += \
 t_generated_code2_cxx_generate_packed_data_SOURCES = \
        t/generated-code2/cxx-generate-packed-data.cc \
        t/test-full.pb.cc
-t/generated-code2/cxx-generate-packed-data.$(OBJEXT): t/test-full.pb.h
+EXTRA_t_generated_code_test_generated_code_DEPENDENCIES = t/test-full.pb.h
 t_generated_code2_cxx_generate_packed_data_CXXFLAGS = \
        $(AM_CXXFLAGS) \
        $(protobuf_CFLAGS)

I've never run into the reporter's problem on my desktop system, but I only have four CPU cores, so...

I'm not sure we can test for this in travis-ci, though.

@edmonds
Copy link
Member

edmonds commented Sep 5, 2014

Actually, wait, listing t/test-full.pb.h in BUILT_SOURCES is supposed to already take care of this, but "BUILT_SOURCES is honored only by ‘make all’, ‘make check’ and ‘make install’" (per http://www.gnu.org/software/automake/manual/html_node/Sources.html).

@ncopa Can you confirm what make command you used to build the source? You trimmed this information from your initial report.

@ncopa
Copy link
Contributor Author

ncopa commented Sep 8, 2014

I doubt this will be reproduceable in travis. looks like they give you 1.5 virutal cpus...

the make command I used is something like export MAKEFLAGS=-j25; make

It also happens with make all. I think that is becuase, it does a pre-build to generate all the BUILT_SOURCES before it starts the 'real' built. But this prebuild is also run in parallel, so the deps of the prebuild needs be exact.

-t/generated-code2/cxx-generate-packed-data.$(OBJEXT): t/test-full.pb.h
+EXTRA_t_generated_code_test_generated_code_DEPENDENCIES = t/test-full.pb.h

This did not solve it. I think you maybe meant:

t_generated_code2_cxx_generate_packed_data_DEPENDENCIES = t/test-full.pb.h

But that is also wrong because it adds the needed header as a link time dependency of the final executable. What we need is adding t/test-full.pb.h as a dependency of the object generated from t/generated-code2/cxx-generate-packed-data.cc, not a dep fo the final generated executable.

I don't think automake gives any proper way to specify dependencies of the _SOURCES.

But what we could do is add this line:

$(t_generated_code2_cxx_generate_packed_data_OBJECTS): t/test-full.pb.h

That will the t/test-full.pb.h as a dependency for all the code2_cxx_generate_packed_data objects, including the cxx-generate-packed-data.$(OBJEXT) and the test-full.pb.h should always get generated first.

I tested it, it works - and I think that is better than my originally proposed patch since this is not as dependant on the automake internals.

ncopa added a commit to ncopa/protobuf-c that referenced this issue Sep 8, 2014
The object file name is not what expected when building non-recursive.
Rather that expect autoamek to generate a specific filename, we simply
make the generated header a dependency of all objects for
cxx-generate-packed-data.

This fixes issue protobuf-c#156.
edmonds pushed a commit that referenced this issue Sep 8, 2014
…ked-data

The object file name is not what is expected when building with
automake's "subdir-objects" option. Rather than expecting automake to
generate a specific filename, we simply make the generated header a
dependency of all objects for cxx-generate-packed-data.

(Issue #156, #169.)
@edmonds
Copy link
Member

edmonds commented Sep 8, 2014

Fix commited to next.

@edmonds edmonds closed this as completed Sep 8, 2014
@juhao5208
Copy link

juhao5208 commented Jan 11, 2023

hi,but it's not slove my problem,it is not work,i don't konw why.

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

No branches or pull requests

4 participants