430166 - Treehydra: need proper makefile deps

Status: RESOLVED FIXED

(Developer Infrastructure :: Source Code Analysis, defect)

Description

[David Mandelin [:dmandelin]]

Rebuilding treehydra is unreliable without doing 'make clean' first, because the dependences on included .c files are not tracked in the makefile. This is annoying.

At this point, we may want to go to a system using 'gcc -M'.

Comment 1

[(dormant account)]

It's one of those top secret mental TODOs  I've had for a while

Comment 2

[David Mandelin [:dmandelin]]

Attachment: Temporary patch

For now, I "manually" figured out the deps and patched the makefile accordingly. I'm posting it here so anyone else that wants can also use it for now. 

One thing I noticed is that treehydra_generated.c is rebuilt any time dehydra.so changes, which is technically right, but usually not useful if you're really just hacking Treehydra. So you might want to remove that dep depending on what you're doing.

Comment 3

[(dormant account)]

(In reply to comment #2)
> Created an attachment (id=317610) [details]
> Temporary patch
> 
> For now, I "manually" figured out the deps and patched the makefile
> accordingly. I'm posting it here so anyone else that wants can also use it for
> now. 

I'm pretty sure you are going to do this next, but in case you aren't: this kind of verbose dependacy stuff belongs in a separate include makefile.

> 
> One thing I noticed is that treehydra_generated.c is rebuilt any time
> dehydra.so changes, which is technically right, but usually not useful if
> you're really just hacking Treehydra. So you might want to remove that dep
> depending on what you're doing.
> 
If you are hacking treehydra, dehydra.so shouldn't change :) I don't like overly configurable solutions cos they typically end up developing a distinct GNU stench.

If you are hacking dehydra, and this causes treehydra to build, it's a good thing. Generating treehydra is like a really complicated selftest for dehydra.

Also one can't complain about compilation speed of C projects after working on Elsa or any other application using g++ :)

Comment 4

[David Mandelin [:dmandelin]]

(In reply to comment #3)
> I'm pretty sure you are going to do this next, but in case you aren't: this
> kind of verbose dependacy stuff belongs in a separate include makefile.

Actually, I wasn't planning on doing anything next, at least not without further discussion. I gather that it's not too hard to regenerate the deps automatically, but we'd need to decide where to put the dep files. The 'make' manual gives an example where you get a .d file for every .o file in your build directory. Would you like that, or rather have a separate .deps directory or something?

> > One thing I noticed is that treehydra_generated.c is rebuilt any time
> > dehydra.so changes, which is technically right, but usually not useful if
> > you're really just hacking Treehydra. So you might want to remove that dep
> > depending on what you're doing.
> > 
> If you are hacking treehydra, dehydra.so shouldn't change :) I don't like
> overly configurable solutions cos they typically end up developing a distinct
> GNU stench.

Heh. I'm referring to the fact that in the current version, Treehydra includes some dehydra .c files. When hacking Treehydra, I often change those files in a way that doesn't "interestingly" affect the behavior of Dehydra. This won't apply if the two are separated again. 

> If you are hacking dehydra, and this causes treehydra to build, it's a good
> thing. Generating treehydra is like a really complicated selftest for dehydra.
> 
> Also one can't complain about compilation speed of C projects after working on
> Elsa or any other application using g++ :)

It's fairly quick, but all that extra output from convert-tree.js just fries my brain that extra 1% that I don't need. :-)

It is a good thing to test that changes to files used by dehydra don't break that part of the treehydra build, so on balance I would say we shouldn't remove that dep from the Makefile. At most I would say a super-secret user option to skip it would be welcome in some cases.

Comment 5

[(dormant account)]

> Actually, I wasn't planning on doing anything next, at least not without
> further discussion. I gather that it's not too hard to regenerate the deps
> automatically, but we'd need to decide where to put the dep files. The 'make'
> manual gives an example where you get a .d file for every .o file in your build
> directory. Would you like that, or rather have a separate .deps directory or
> something?

I don't know. I've never used that feature. I've used fastdep before and it put everything into 1 file.

> It's fairly quick, but all that extra output from convert-tree.js just fries my
> brain that extra 1% that I don't need. :-)
> 
> It is a good thing to test that changes to files used by dehydra don't break
> that part of the treehydra build, so on balance I would say we shouldn't remove
> that dep from the Makefile. At most I would say a super-secret user option to
> skip it would be welcome in some cases.
> 

*Shrink Voice*: Have you considered that your problem isn't dehydra being a dependency. Sounds like we should disable the convert_tree warnings since they aren't useful unless you are debugging generated code.
Will that make you happy?

Comment 6

[(dormant account)]

Attachment: gcc -M dep gen

I extrapolated this from http://make.paulandlesley.org/autodep.html

Comment 7

[David Mandelin [:dmandelin]]

Comment on attachment 318886 [details] [diff] [review]
gcc -M dep gen

Looks good given a few changes, see below. 

> %.o: %.c
>-	$(CC) -g3 $(CFLAGS) -c $<
>+	$(CC) -MD -g3 $(CFLAGS) -c $<

If you just download and run, none of the deps get made because DEPDIR doesn't exist. Adding this seems to do the trick. bsmedberg might have an idea of a better place to put this, but it seems OK to me.

 +      mkdir -p $(DEPDIR)

>+	@cp $*.d $(DEPDIR)/$*.P; \
>+	sed -e 's/#.*//' -e 's/^[^:]*: *//' -e 's/ *\\$$//' \
>+            -e '/^$$/ d' -e 's/$$/ :/' < $*.d >> $(DEPDIR)/$*.P; \
>+        rm -f $*.d

The following targets are no longer needed and should just be deleted.

>+dehydra_ast.o: dehydra_ast.c
>+treehydra.o: treehydra.c

Comment 8

[(dormant account)]

pushed