Patchwork [RFC] Replicate src dir in build dir

login
register
mail settings
Submitter Yao Qi
Date Oct. 11, 2017, 1:25 a.m.
Message ID <CAH=s-PMsNcpmY=wygCSicyrQnQ49aDY-acR2owSSyi+-oLrJTQ@mail.gmail.com>
Download mbox | patch
Permalink /patch/23469/
State New
Headers show

Comments

Yao Qi - Oct. 11, 2017, 1:25 a.m.
[I happen to have some time today, so look into this
problem a little bit]

On Sun, Oct 8, 2017 at 4:24 AM, Tom Tromey <tom@tromey.com> wrote:
> But, there are a couple of bugs; one serious, one probably not.
>
> Yao>  override COMPILE.post = -c -o $@ -MT $@ -MMD -MP \
> Yao> -  -MF $(DEPDIR)/$(basename $(@F)).Tpo
> Yao> -override POSTCOMPILE = @mv $(DEPDIR)/$(basename $(@F)).Tpo \
> Yao> -  $(DEPDIR)/$(basename $(@F)).Po
> Yao> +  -MF $(@D)/$(DEPDIR)/$(@F).Tpo
> Yao> +override POSTCOMPILE = @mv $(@D)/$(DEPDIR)/$(@F).Tpo \
> Yao> +  $(@D)/$(DEPDIR)/$(@F).Po
> Yao>  else
> Yao>  override COMPILE.pre = source='$<' object='$@' libtool=no \
> Yao>    DEPDIR=$(DEPDIR) $(DEPMODE) $(depcomp) $(CC)
>
> This hunk removed the $(basename ...) wrapper.  This renames the
> dependency files from "basename.Tpo" to "basename.o.Tpo".
>
> However, it didn't change the dependencies include near the end of the
> Makefile:
>
>     -include $(patsubst %.o, $(DEPDIR)/%.Po, $(all_object_files))
>
> So, because the names differ, automatic dependency tracking no longer
> works.  I was really puzzled today when my rebuilds were so fast! :-)
>
> I think the correct solution is to add the $(basename ...) back.  I

Right, I should add $(basename ) back, like the patch
below.

> thought at first that the "-include" line could be fixed, but this won't
> work in case where "depcomp" is used -- because the name choice also has
> to coincide with what depcomp does.
>
> Finally, I think the arch/ change might not work properly with depcomp.
> The COMPILE.pre override (quoted in the context of the patch hunk above)
> probably needs a tweak to DEPDIR, I would guess DEPDIR=$(@D)/$(DEPDIR).
> However, I didn't try this case.  (I think when I wrote this I tested
> the depcomp case by hacking the generated Makefile to ensure that the
> depcomp code was used; otherwise you need a non-capable compiler.)

If I understand you correctly, depcomp has already
taken care of it.  See my reply to Simon's review.
https://sourceware.org/ml/gdb-patches/2017-09/msg00489.html
Tom Tromey - Oct. 11, 2017, 3:26 a.m.
>>>>> "Yao" == Yao Qi <qiyaoltc@gmail.com> writes:

Yao> Right, I should add $(basename ) back, like the patch
Yao> below.

Yeah.  Actually I wanted this to work so I sent a patch recently, which
is in now.

Yao> If I understand you correctly, depcomp has already
Yao> taken care of it.  See my reply to Simon's review.
Yao> https://sourceware.org/ml/gdb-patches/2017-09/msg00489.html

Yep, that's right; though it turned out that this code didn't work for
other reasons (nothing to do with your change); I fixed that as well.

Tom

Patch

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index da4095d..90ddc02 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -2952,8 +2952,8 @@  ifeq ($(DEPMODE),depmode=gcc3)
 # into place if the compile succeeds.  We need this because gcc does
 # not atomically write the dependency output file.
 override COMPILE.post = -c -o $@ -MT $@ -MMD -MP \
- -MF $(@D)/$(DEPDIR)/$(@F).Tpo
-override POSTCOMPILE = @mv $(@D)/$(DEPDIR)/$(@F).Tpo \
+ -MF $(@D)/$(DEPDIR)/$(basename $(@F)).Tpo
+override POSTCOMPILE = @mv $(@D)/$(DEPDIR)/$(basename $(@F)).Tpo \
  $(@D)/$(DEPDIR)/$(@F).Po
 else
 override COMPILE.pre = source='$<' object='$@' libtool=no \