Thanks for looking at this.

I'm attaching my complete current patch to make sure that we are
in sync.

On Tue, May 21, 2013 at 5:28 PM, Lars Ellenberg
<lars.ellenb...@linbit.com> wrote:
> Do you realize that set -e does NOT affect "lists",
> but only "simple commands"?
>
>>         for i in $* ; do
>>                 [ -f $i/a/conflict ] && false
>
> Do you realize that the exit code of the "list" above
> will never be 0?

No. Not from long experience with set -e, nor from reading the
docs. ;)

Here's the relevant output from running the tests:

# 203 main t/0003.hub-and-spoke.t
# =========== assert no conflict D1 = trace = {{{3
# -x : + assert_no_conflict /home/mikjo/d/csync2/tests/1
# -x : + set -xe
# -x : + for i in '$*'
# -x : + '[' -f /home/mikjo/d/csync2/tests/1/a/conflict ']'
# -x : + false
# =========== assert no conflict D1 = err = {{{3
# =========== assert no conflict D1 = out = {{{2
# =========== assert no conflict D1 = exit:1 = expected:0 }}}1

Here's a test shell script demonstrating that set -e does what
I expect:

$ cat /tmp/t
#!/bin/bash
(
set -xe
for i in a b c d ; do [ "$i" = c ] && false ; done
)
$ /tmp/t
+ for i in a b c d
+ '[' a = c ']'
+ for i in a b c d
+ '[' b = c ']'
+ for i in a b c d
+ '[' c = c ']'
+ false
$ echo $?
1

set -e does not apply to condition tests; you would not want set -e
to cause the shell to exit due to "if false; ..." It is explicitly
documented to apply to any element of a command list, unless
it's part of a conditional test (so it applies only to the final command
after the last &&/|| on the line).

> What you want is no set -e, it is misleading.

I disagree. I explicitly want the first failed command to bail out,
and I disagree that it is misleading. I think it's exactly correctly
leading.

> And probably no sub shell () either. What for?

Subshell to protect the outside from the set -xe.

I'd assume that you used subshell with set -xe for the same reason
when you used it in create_tree in 0002.long-and-strange-file-names.t
but I can't guess at your reasoning for sure...

I had it temporarily even in my single-shell-invocation functions because
I was adding and removing debugging statements. But I've removed it
from all single-shell-invocation functions in the attached patch.

> I'm not saying "notabug";
> there may well be a genuine csync2 bug in that area as well.
>
> I'm just pointing out that this implementation of the test is wrong.
>
>>         )
>> }
>> TEST    "record conflict file on d2"    csync2 -C n2 -N $N2 -cr $D2
>> TEST    "remove d2 conflict"    remove_conflict $D2
>> TEST    "assert no conflict D2" assert_no_conflict $D2
>> TEST_EXPECT_EXIT_CODE   1 "diff -rq D1D2 fail"  diff -rq $D1 $D2
>> TEST    "record conflict file deletion" csync2 -C n2 -N $N2 -cr $D2
>> TEST    "csync2 -uv inbound"    csync2_u ${N2}:n2 ${N1}:hub
>
> your modified "csync_u" function is using the part after the : as
> "config" (argument to the csync2 -C option)?

That's correct, as shown in the example patch that I posted Monday.

> If so, you did not tell the "n2" config about this file,
> or it's deletion.  csync2 -C n2 has not marked this file dirty,
> so csync2 -C n2 -u won't propagate anything.

OK, why does this
TEST    "record conflict file on d2"    csync2 -C n2 -N $N2 -cr $D2
(remove file)
TEST    "record conflict file deletion" csync2 -C n2 -N $N2 -cr $D2
not mark the file first as present and then as deleted in the n2
config database?

What command would I have to run to mark the file as deleted?

I find nothing at all in any of the documentation about what causes
a file deletion to be noticed. I could well believe that it's there and
that I've missed it. I'm rather sure that when I was doing my initial
csync2 tests with csync2 installed on two systems, that I saw a
file deletion propagate, so I wasn't expecting this problem when I
was writing these tests...

(I had actually fully synchronized the "conflict" file among all four
nodes in the previous set of tests; that previous set was making
sure that fully automated conflict resolution will do what I want. So
the first -cr invocation in this set of tests should be redundant but
was there to make it easier to think about the set of tests separately.)

> A "check" in one config will not set dirty flags,
> or anything, really, for other configs.

I reviewed my tests, and I'm completely consistent in using the n2
config for N2 and the hub config for N1. I understand the separate
database for separate configuration arrangement. In fact, you will
see that I had to modify the cleanup() method in my patch to make
it remove databases for alternative configurations. ☺

Anyway, thanks again for looking at this. I'm hoping that my
hub-and-spoke "story test" can turn into a worthwhile addition
to csync2.

Again, when this is ready to commit, I'll give you a proper pull
request. If you have no preference, I'll fork csync2 on github and
point you at a branch; if you prefer email, let me know.

Attachment: hub-and-spoke.patch
Description: Binary data

_______________________________________________
Csync2 mailing list
Csync2@lists.linbit.com
http://lists.linbit.com/mailman/listinfo/csync2

Reply via email to