#903 closed defect (fixed)
[PATCH] Premake4 makefile dependency tracking broken for PCH
Reported by: | Philip Taylor | Owned by: | Yves |
---|---|---|---|
Priority: | Release Blocker | Milestone: | Alpha 7 |
Component: | Core engine | Keywords: | |
Cc: | Patch: |
Description
- Run
make scriptinterface
- Edit a file like
lib/status.h
and add#error 1
- Run
make scriptinterface
- It doesn't try rebuilding anything at all
It ought to rebuild everything and hit the error. Looks like dependencies of the .gch file are being ignored entirely and not triggering rebuilds.
Attachments (3)
Change History (18)
comment:1 by , 13 years ago
Priority: | Must Have → Release Blocker |
---|
comment:2 by , 13 years ago
Owner: | set to |
---|
by , 13 years ago
Attachment: | pch_cxxtest_cleanworkspaces_v4.patch added |
---|
This is the same patch I also posted in the thread.
comment:3 by , 13 years ago
Keywords: | review added |
---|
comment:4 by , 13 years ago
Summary: | Premake4 makefile dependency tracking broken for PCH → [PATCH] Premake4 makefile dependency tracking broken for PCH |
---|
comment:5 by , 13 years ago
The patch fixes more problems than just the one with dependency tracking of PCH. The details are described in the thread, but they are scattered a bit so here's a summary:
- Add a line to clean-workspaces.sh to also run make clean on premake4 and not only premake3
- Fix relative path issue with test-generation on XCode and Visual Studio.
- Replace absolute paths to cxxtestgen.pl in the makefiles with a relative one
- Fix dependency tracking of PCH
- Remove MF and MT flags that aren't needed according to their description in the manpage
by , 13 years ago
Attachment: | pch_cxxtest_cleanworkspaces_buserror_v5.patch added |
---|
A workaround for the bus-error is included in v5 of the patch, otherwise it's the same as v4.
comment:6 by , 13 years ago
I've attached an updated patch that also includes a workaround for the bus error discussed here. It's not directly related to this ticket, but I've decided to put all patches together.
comment:7 by , 13 years ago
The patch removes the
-MF $(OBJDIR)/%s.d -MT "$@"
which results in the .d
files ending up in gcc/
instead of gcc/obj/whatever_Release/
etc, which seems to make the dependency checking fail for every source file other than precompiled.h
.
comment:8 by , 13 years ago
I believe the gcc segmentation fault is because precompiled.h
is built with -include obj/lowlevel_Release/precompiled.h
which means it tries to use the .gch
file as input, at the same time as being the output of that command, so it corrupts the file while it's reading it. (It's not GCC's fault that it dies in that situation.)
The rm
in the latest patch avoids that problem by deleting the .gch
file so GCC detects it's missing and doesn't try reading it, before it starts to write it. That seems safe enough but a bit of a hack. It'd probably be nicer if we could remove that -include
when building precompiled.h
(or, rather, only include it when building the other source files), if that's not too hard.
comment:9 by , 13 years ago
Thank you for reviewing the patch!
Are you sure about the first one? I just tested it again and the *.d files are still placed in gcc/obj/*_Release.
You explanation about the segmentation fault makes perfect sense. I'm going to check if I can remove that include for of precompiled.h there.
comment:10 by , 13 years ago
Oh, hmm, looks like the .d files are only wrong when I'm compiling with ccache (which acts as a wrapper around gcc) - if I run gcc directly then they go into the right place. Also, it's only failing with ccache 2.4; it seems to work okay with ccache 3.1.5.
(I think it's probably still worth including the -MF
etc as a workaround for what seems to be a ccache 2.4 bug, in case other people hit the same problem.)
comment:11 by , 13 years ago
Yes, I can add it again, no problem. There's no real benefit in removing it, I just thought it's not needed.
comment:12 by , 13 years ago
Any update on this? Can a new patch be provided ASAP? Ideally we'd release Alpha 7 with only the Premake4 build scripts.
by , 13 years ago
Attachment: | pch_cxxtest_cleanworkspaces_buserror_v6.patch added |
---|
comment:13 by , 13 years ago
I'm sorry for the dalay, I didn't have time last week. A new patch is attached with the discussed changes.
comment:14 by , 13 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
(In [10154]) Applies Yves' latest patch for premake4. Fixes test generation path in XCode and MSVC. Fixes PCH dependency tracking in GCC. Fixes #903. Fixes PCH being included in its own input (caused GCC bus error). Ensures premake4 gets rebuilt on *nix after running clean-workspaces.sh. Updates Windows premake4 binary.
comment:15 by , 8 years ago
Keywords: | review removed |
---|
A patch is ready for review in this thread.