From: Dmitry Kurochkin <dmitry.kurochkin@gmail.com>
To: Mike Kelly <pioto@pioto.org>
Subject: Re: FreeBSD Support Patches
In-Reply-To:
 <CAFb0NsJ7Bv-Lp03WHvZQh_yaimzaj34VmkVQCjr_u5MDrfqAKw@mail.gmail.com>
References: <1337953411-21000-1-git-send-email-pioto@pioto.org>
	<87ipfktkgy.fsf@gmail.com>
	<CAFb0NsJ7Bv-Lp03WHvZQh_yaimzaj34VmkVQCjr_u5MDrfqAKw@mail.gmail.com>
User-Agent: Notmuch/0.13+30~g6d44c5a (http://notmuchmail.org) Emacs/23.4.1
	(x86_64-pc-linux-gnu)
Date: Sat, 26 May 2012 05:07:36 +0400
Message-ID: <87aa0vu4vb.fsf@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Cc: notmuch@notmuchmail.org
Precedence: list

Mike Kelly <pioto@pioto.org> writes:

> On Fri, May 25, 2012 at 10:15 AM, Dmitry Kurochkin
> <dmitry.kurochkin@gmail.com> wrote:
>> Why do we need to explicitly declare Emacs dependency for tests? =C2=A0T=
here
>> should be no need for it. =C2=A0We have "implicit" dependencies which are
>> declared once (see test_declare_external_prereq calls at the end of
>> test-lib.sh) and are automatically handled when a test tries to use a
>> missing binary. =C2=A0Explicit dependencies are hard to maintain (e.g. y=
our
>> patch adds explicit emacs dependency for crypto test but misses gpg).
>> With rare exceptions we should not use explicit dependencies.
>
> Because not every test actually has those implicit dependencies. For
> example, some of the crypto tests depend upon emacs_deliver_message
> working correctly for subsequents tests. Those emacs_deliver_message
> tests are skipped, but not the ones after it that try to do something
> with that injected message.
>
> For the emacs-* test files, there are some tests that act the same
> way.

These subtests do not directly depend on Emacs.  They depend on other
subtests.  Currently, we do not support such dependencies.  But what you
propose is not the solution.  We have two options here: make all
subtests independent or introduce proper subtests dependencies.  The
former might require many changes to existing tests and may be hard to
enforce.  The latter is not trivial as well but is doable.  I planned to
implement subtests dependencies but never really got to it (and I do not
think I will anytime soon).

> However, it is also a minor speed improvement to say that,
> obviously, none of the emacs tests are going to work, so just don't
> bother.
>

I do not think maintaining an explicit list of dependencies worth a
minor speed improvement.

Given all above, I understand that your patches fix a common problem in
a simple way.  And it does not look like we would get proper solution
anytime soon.  So I am ok with these patches with two comments:

  * Provide a proper commit message to explain the issue in more detail.

  * Add an XXX comment for each explicit dependency, something like:

    // XXX: Workaround for subtests that depend on other subtests (and,
    // hence, indirectly depend on emacs).  Should be removed when we
    // have proper subtests dependencies.

Regards,
  Dmitry

> --=20
> Mike Kelly