Opened 13 years ago

Closed 13 years ago

Last modified 8 years ago

#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)

pch_cxxtest_cleanworkspaces_v4.patch (38.8 KB ) - added by Yves 13 years ago.
This is the same patch I also posted in the thread.
pch_cxxtest_cleanworkspaces_buserror_v5.patch (39.2 KB ) - added by Yves 13 years ago.
A workaround for the bus-error is included in v5 of the patch, otherwise it's the same as v4.
pch_cxxtest_cleanworkspaces_buserror_v6.patch (39.0 KB ) - added by Yves 13 years ago.

Download all attachments as: .zip

Change History (18)

comment:1 by Kieran P, 13 years ago

Priority: Must HaveRelease Blocker

comment:2 by Yves, 13 years ago

Owner: set to Yves

A patch is ready for review in this thread.

by Yves, 13 years ago

This is the same patch I also posted in the thread.

comment:3 by Yves, 13 years ago

Keywords: review added

comment:4 by Kieran P, 13 years ago

Summary: Premake4 makefile dependency tracking broken for PCH[PATCH] Premake4 makefile dependency tracking broken for PCH

comment:5 by Yves, 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 Yves, 13 years ago

A workaround for the bus-error is included in v5 of the patch, otherwise it's the same as v4.

comment:6 by Yves, 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 Philip Taylor, 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 Philip Taylor, 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 Yves, 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 Philip Taylor, 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 Yves, 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 Kieran P, 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.

comment:13 by Yves, 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 ben, 13 years ago

Resolution: fixed
Status: newclosed

(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 sanderd17, 8 years ago

Keywords: review removed
Note: See TracTickets for help on using tickets.