Dependency paths in dep file completely broken in some cases

[andrewchambers]

https://github.com/andrewchambers/redo-c-bug

$ cd redo-c-bug
$ cd mescc
$ redo-ifchange bin/mescc
+ exec
+ case "$1" in
+ redo-ifchange mes-sources.list nyacc-sources.list ../stage0/all.done
redo   mes-sources.list # ./default.do
+ exec
+ case "$1" in
+ date
redo   nyacc-sources.list # ./default.do
+ exec
+ case "$1" in
+ date
redo   ../stage0/all.done # ./default.do
+ exec
+ case "$1" in
+ redo-ifchange ./some-dir/some-file.txt
+ date
+ date
# Bogus paths in the depfile, .././default.do does not exist.
$ cat .dep.mes-sources.list
=18d60ae6787a1f39a5960226550793655233ae81df384a8a33150a178412960d 00000000627dcd35 .././default.do
=3de81c64ade6bde7632191d281f07652963c0e077fb8632c7d98d62dc41a3617 00000000627dd1d1 ../mes-sources.list
# Next we get infinite spurious builds because the deps don't exit.
$ redo-ifchange bin/mescc
redo   mes-sources.list # ./default.do
+ exec
+ case "$1" in
+ date
redo   nyacc-sources.list # ./default.do
+ exec
+ case "$1" in
+ date
redo   ../stage0/all.done # ./default.do
+ exec
+ case "$1" in
+ redo-ifchange ./some-dir/some-file.txt
+ date
+ date

[andrewchambers]

The symptom of this bug looks a lot like a circular dependency, which has probably confused a lot of users in the past (me included)

[AndreyDobrovolskyOdessa]

Hi Andrew, hi Leah,

It appeared, that I've already hit this bug in my "dev" branch, You can look at https://github.com/AndreyDobrovolskyOdessa/redo-c/commit/40b16244c04c86e8928a74404d00d43d823f20ba Seems that I was simply trying to avoid extra double-dots, but this commit seems very likely to fight the issue. As You can notice it costs additional global dnrel[] which in fact stores REDO_DIRPREFIX and then use it in write_dep(). Will it be appropriate to create the pull request?

Best regards!

[AndreyDobrovolskyOdessa]

The commit I've mentioned is from the stalled branch and of course needs correction. I want to note that the same filename conversion must be applied in redo_ifcreate(). As I can see in my dev branch I united later both into write_dep() with additional parameter.

[andrewchambers]

Thanks Andrey, it's an unfortunate bug indeed.

[lions-tech]

I've just discovered this issue. I also observed the described behaviour.

Another thing I observed is that paths like .././projdir/text.txt are written. The deeper the directory structure the longer and more confusing these paths will get.

I tried fixing these issues and stumbled upon a new problem: The last dependency written, is the target itself. The path passed to write_dep in this case is just a filname (i.e. text.txt). Just avoiding to prepend uprel to that, is enough to record a valid path. But if the dofile contains a call like redo-ifchange test.meta.xml.attrs the path passed to write_dep is also just a filename, with the difference that it is not located in the directory of our dependency file.

To summarize: If uprel is prepended to all paths which only consist of a filename, the path to our test.meta.xml.attrs would be correct, but the path to the target file would be wrong. However, if uprel is not prepended to such paths, the path to our target is correct and the path to test.meta.xml.attrs is wrong. Perhaps a solution might be to prepend the REDO_DIRPREFIX before passing the target to write_dep.

I have created a small dummy project to reproduce these bugs: https://github.com/lions-tech/redo-c-testproj.