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

Makefile: specify version.h as a dependency of all object files #590

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

trofi
Copy link

@trofi trofi commented Aug 20, 2023

Otherwise parallel builds fail occasionally as:

$ make --shuffle -j
...
g++ -c   -O3 -D__STDC_FORMAT_MACROS   argStruct.cpp
g++ -c   -O3 -D__STDC_FORMAT_MACROS   ancestral_likes.cpp
gcc -c   -O3 -D__STDC_FORMAT_MACROS   fet.c
g++ -c   -O3 -D__STDC_FORMAT_MACROS   abcGL.cpp
g++ -c   -O3 -D__STDC_FORMAT_MACROS   pop1_read.cpp
g++ -c   -O3 -D__STDC_FORMAT_MACROS   angsd.cpp
g++ -c   -O3 -D__STDC_FORMAT_MACROS   abcHWE_F.cpp
gcc -c   -O3 -D__STDC_FORMAT_MACROS   kprobaln.c
g++ -c   -O3 -D__STDC_FORMAT_MACROS   mpileup.cpp
argStruct.cpp:3:10: fatal error: version.h: No such file or directory
    3 | #include "version.h"
      |          ^~~~~~~~~~~
compilation terminated.
angsd.cpp:11:10: fatal error: version.h: No such file or directory
   11 | #include "version.h"
      |          ^~~~~~~~~~~

Before the change it was enough to try to build angsd with 1-2 make --shuffle runs to get the failure. After the change the build survives 20 rebuilds.

Otherwise parallel builds fail occasionally as:

    $ make --shuffle -j
    ...
    g++ -c   -O3 -D__STDC_FORMAT_MACROS   argStruct.cpp
    g++ -c   -O3 -D__STDC_FORMAT_MACROS   ancestral_likes.cpp
    gcc -c   -O3 -D__STDC_FORMAT_MACROS   fet.c
    g++ -c   -O3 -D__STDC_FORMAT_MACROS   abcGL.cpp
    g++ -c   -O3 -D__STDC_FORMAT_MACROS   pop1_read.cpp
    g++ -c   -O3 -D__STDC_FORMAT_MACROS   angsd.cpp
    g++ -c   -O3 -D__STDC_FORMAT_MACROS   abcHWE_F.cpp
    gcc -c   -O3 -D__STDC_FORMAT_MACROS   kprobaln.c
    g++ -c   -O3 -D__STDC_FORMAT_MACROS   mpileup.cpp
    argStruct.cpp:3:10: fatal error: version.h: No such file or directory
        3 | #include "version.h"
          |          ^~~~~~~~~~~
    compilation terminated.
    angsd.cpp:11:10: fatal error: version.h: No such file or directory
       11 | #include "version.h"
          |          ^~~~~~~~~~~

Before the change it was enough to try to build `angsd` with 1-2
`make --shuffle` runs to get the failure. After the change the build
survives 20 rebuilds.
ANGSD referenced this pull request Aug 20, 2023
…r testing for concurrency issues PR#590 found an issue. After looking into it there was indeed atleast two problems that this commit fixed.
@ANGSD
Copy link
Owner

ANGSD commented Aug 20, 2023

Hi @trofi, thanks for this input. I had not heard about the --shuffle. There were multiple problems in my Makefiles that the --shuffle parameter helped me find.

I made a different fix by using the. .WAIT in the makefile rather than including the version. h in all objects as proposed in this PR. I will let the PR be open for now in case my fix caused other problems.

ANGSD added a commit that referenced this pull request Aug 20, 2023
Had to remove .WAIT from the makefile. It looks like this option is not recognized by existing make. The PR #590 might still be relevant. I will discuss this with @isinaltinkaya
@trofi
Copy link
Author

trofi commented Aug 21, 2023

I think there are a few problems with .WAIT solution:

  1. (minor) it was added in recently released GNU make 4.4, thus might not be widely supported
  2. (major) it does not add direct dependency between version.h and object files derived from source files that use it. Those are currently: angsd.cpp. argStruct.cpp and multiReader.cpp.

[2.] means that more directed builds like $ make angsd.o will still fail to build even in presence of .WAIT. To fix it sometimes build systems manually or automatically generate dependencies like:

angsd.o: version.h
argStruct.o: version.h
multiReader.o: version.h

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.

2 participants