[PATCH] Fix bugs in the simple branch

53 views
Skip to first unread message

Mildred Ki'Lya

unread,
Dec 28, 2012, 6:23:10 AM12/28/12
to redo...@googlegroups.com
Hi,

I created a pull request on github, but I thought I would repeat myself here:

Attached two commits for the simple branch.

The first one add a test for a problem I found when I had a target depending on two redo-stamp targets that were also marked with redo-always. Basically, the use case for this is:

cflags.do:
redo-always
echo $CFLAGS >$3
redo-stamp <$3

config.do:
redo-always
echo $CONFIG >$3
redo-stamp <$3

compile.do:
redo-ifchange cflags config
. ./cflags
. ./config
cc-$CONFIG $CFLAGS -o $3
...

This was a very useful technique I found to be able to depend on environment variables. The problem was that compile.do was always rebuilt even if neither cflags or config changed. I should have fixed that.

The other problem is that the test 130 failed because ls -l shown -rw-r--r--. instead of just -rw-r--r-- making the test fail. I don't know if the correction is ideal but it works on my computer. You might want to drop this find a workaround (such as tr -d .).

Is there a chance of getting these patches included. I noticed that redo was not updated regularly and also noticed there are many other pull requests not answered.

I am also thinking of improving redo and add to it the possibility of a top-down workflow. There are cases where this is useful (such as when you have no idea of the files that are going to be generated). I was thinking of a redo-did command to register files after they are created (solving part of the problem of multiple generated files), but I still need to think about it.



Mildred

0001-Fixed-problem-when-depending-on-two-targets-redo-alw.patch
0002-Fix-test-130-ls-l-output-is-not-always-as-expected.patch

Avery Pennarun

unread,
Dec 29, 2012, 1:35:58 AM12/29/12
to Mildred Ki'Lya, redo...@googlegroups.com
Hi,

Thanks for the contribution. As you can probably tell, I've been
massively drowning in other projects lately and haven't been giving
redo the attention it deserves. I appreciate that other people have
been improving stuff on github and someday I hope to have time to
collect all the patches on github and the mailing list. If someone
wants to pull the contributions together sooner than me, please feel
free to do so, upload it in your own github fork, and tell the list,
who I'm sure will appreciate it.

I haven't looked at the patches attached in this thread, but your
problems and suggested solutions seem like they make sense.

Have fun,

Avery

Bill Casarin

unread,
Feb 19, 2013, 9:34:06 AM2/19/13
to Avery Pennarun, Mildred Ki'Lya, redo...@googlegroups.com
I tried this patch and it doesn't seem to work for me, I believe I'm having the same issue. I have a target that depends on two rules that both use `redo-always` and `redo-stamp`. When I only use one of the rules it works, when I use both it always rebuilds.

I also tried your most recent HEADs Mildred, they all seem to break my build :(

Anyone get this working on `master`? I took a crack at it, but the debugging the recursive function in deps.py was a bit dizzying.

Cheers,
Bill

Mildred Ki'Lya

unread,
Feb 22, 2013, 11:19:05 AM2/22/13
to Bill Casarin, Avery Pennarun, redo...@googlegroups.com
On 2013-02-19 15:34, Bill Casarin wrote:
> I tried this patch and it doesn't seem to work for me, I believe I'm
> having the same issue. I have a target that depends on two rules that
> both use `redo-always` and `redo-stamp`. When I only use one of the
> rules it works, when I use both it always rebuilds.

This patch contains some errors.

The one that applies on master is simply broken (and I wouldn't dare
trying to get it working, the simple branch is much more simple)

The one that applies on simple is missing a few things I guess. You can
look at the history on my repository to get a commit that probably
contains the missing few things.
>
> I also tried your most recent HEADs Mildred, they all seem to break my
> build :(
>
Try --old-stdout

> Anyone get this working on `master`? I took a crack at it, but the
> debugging the recursive function in deps.py was a bit dizzying.

Especially in master, that's why I only bothered for the simple branch.

Mildred

Reply all
Reply to author
Forward
0 new messages