Re: [PATCH] notmuch(1): clarify documentation about --option/value separators
On Thu, May 07 2020, Daniel Kahn Gillmor wrote: > +separator. Except for boolean options (wihch would be ambiguous), a Misspelling of "which". And while I'm here, strictly speaking Boolean is generally capitalized in English, (being one of those adjectives that is derived from a proper noun). Otherwise, this looks like a good improvement to me. Thanks! -Carl signature.asc Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 2/2] smime: tests of X.509 certificate validity are known-broken on GMime < 3.2.7
On Wed, May 06 2020, Daniel Kahn Gillmor wrote:
> When checking cryptographic signatures, Notmuch relies on GMime to
> tell it whether the certificate that signs a message has a valid User
> ID or not.
>
> If the User ID is not valid, then notmuch does not report the signer's
> User ID to the user. This means that the consumer of notmuch's
> cryptographic summary of a message (or of its protected headers) can
> be confident in relaying the reported identity to the user.
>
> However, some versions of GMime before 3.2.7 cannot report Certificate
> validity for X.509 certificates. This is resolved upstream in GMime
> at https://github.com/jstedfast/gmime/pull/90.
>
> We adapt to this by marking tests of reported User IDs for
> S/MIME-signed messages as known-broken if GMime is older than 3.2.7
> and has not been patched.
>
> If GMime >= 3.2.7 and certificate validity still doesn't work for
> X.509 certs, then there has likely been a regression in GMime and we
> should fail early, during ./configure.
>
> To break out these specific User ID checks from other checks, i had to
> split some tests into two parts, and reuse $output across the two
> subtests.
This works, on top of the previous series -- I skipped git am:ing 1/2,
so thos works without it -- it is good stuff just IMO requires some changes)
Tomi
>
> Signed-off-by: Daniel Kahn Gillmor
> ---
> configure | 79 ++
> test/T355-smime.sh | 19 +---
> test/T356-protected-headers.sh | 15 ++-
> 3 files changed, 104 insertions(+), 9 deletions(-)
>
> diff --git a/configure b/configure
> index 0cfdaa6f..92e5bd1b 100755
> --- a/configure
> +++ b/configure
> @@ -536,6 +536,82 @@ EOF
> if [ -n "$TEMP_GPG" -a -d "$TEMP_GPG" ]; then
> rm -rf "$TEMP_GPG"
> fi
> +
> +# see https://github.com/jstedfast/gmime/pull/90
> +# should be fixed in GMime in 3.2.7, but some distros might patch
> +printf "Checking for GMime X.509 certificate validity... "
> +
> +cat > _check_x509_validity.c < +#include
> +#include
> +
> +int main () {
> +GError *error = NULL;
> +GMimeParser *parser = NULL;
> +GMimeApplicationPkcs7Mime *body = NULL;
> +GMimeSignatureList *sig_list = NULL;
> +GMimeSignature *sig = NULL;
> +GMimeCertificate *cert = NULL;
> +GMimeObject *output = NULL;
> +GMimeValidity validity = GMIME_VALIDITY_UNKNOWN;
> +int len;
> +
> +g_mime_init ();
> +parser = g_mime_parser_new ();
> +g_mime_parser_init_with_stream (parser,
> g_mime_stream_file_open("test/corpora/pkcs7/smime-onepart-signed.eml", "r",
> &error));
> +if (error) return !! fprintf (stderr, "failed to instantiate parser with
> test/corpora/pkcs7/smime-onepart-signed.eml\n");
> +
> +body = GMIME_APPLICATION_PKCS7_MIME(g_mime_message_get_mime_part
> (g_mime_parser_construct_message (parser, NULL)));
> +if (body == NULL) return !! fprintf (stderr, "did not find a
> application/pkcs7 message\n");
> +
> +sig_list = g_mime_application_pkcs7_mime_verify (body,
> GMIME_VERIFY_NONE, &output, &error);
> +if (error || output == NULL) return !! fprintf (stderr, "verify
> failed\n");
> +
> +if (sig_list == NULL) return !! fprintf (stderr, "no GMimeSignatureList
> found\n");
> +len = g_mime_signature_list_length (sig_list);
> +if (len != 1) return !! fprintf (stderr, "expected 1 signature, got
> %d\n", len);
> +sig = g_mime_signature_list_get_signature (sig_list, 0);
> +if (sig == NULL) return !! fprintf (stderr, "no GMimeSignature found at
> position 0\n");
> +cert = g_mime_signature_get_certificate (sig);
> +if (cert == NULL) return !! fprintf (stderr, "no GMimeCertificate
> found\n");
> +validity = g_mime_certificate_get_id_validity (cert);
> +if (validity != GMIME_VALIDITY_FULL) return !! fprintf (stderr, "Got
> validity %d, expected %d\n", validity, GMIME_VALIDITY_FULL);
> +
> +return 0;
> +}
> +EOF
> +if ! TEMP_GPG=$(mktemp -d "${TMPDIR:-/tmp}/notmuch.XX"); then
> +printf 'No.\nCould not make tempdir for testing X.509 certificate
> validity support.\n'
> +errors=$((errors + 1))
> +elif ${CC} ${CFLAGS} ${gmime_cflags} _check_x509_validity.c
> ${gmime_ldflags} -o _check_x509_validity \
> +&& echo disable-crl-checks > "$TEMP_GPG/gpgsm.conf" \
> +&& echo
> "4D:E0:FF:63:C0:E9:EC:01:29:11:C8:7A:EE:DA:3A:9A:7F:6E:C1:0D S" >>
> "$TEMP_GPG/trustlist.txt" \
> +&& GNUPGHOME=${TEMP_GPG} gpgsm --batch --quiet --import <
> "$srcdir"/test/smime/ca.crt
> +then
> +if GNUPGHOME=${TEMP_GPG} ./_check_x509_validity; then
> +gmime_x509_cert_validity=1
> +printf "Yes.\n"
> +else
> +gmime_x509_cert_validity=0
> +printf "No.\n"
> +if pkg-config --exists "gmime-3.0 >= 3.2.7"; then
> +cat < +*** Error: GMime fails to calculate X.509 certificate validity,
[PATCH] notmuch(1): clarify documentation about --option/value separators
id:CA+Tk8fzRiqxWpd=r8=DRvEewNZXUZgD7MKyRLB1A=r-lxxg...@mail.gmail.com started a thread of discussion that showed that the cli's current idiosyncrasies around dealing with boolean options were not understandable. This attempts to improve the documentation at least (actual changes to the API might be better, but have not reached consensus). Note that no one in the discussion thread identified any other (non-boolean) command-line options that cannot use space as a separator. If such an option is identified (or introduced in the future), it should be added explicitly to this part of the manual. Signed-off-by: Daniel Kahn Gillmor --- doc/man1/notmuch.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/man1/notmuch.rst b/doc/man1/notmuch.rst index d2cd8da5..3dcc646c 100644 --- a/doc/man1/notmuch.rst +++ b/doc/man1/notmuch.rst @@ -128,9 +128,9 @@ OPTION SYNTAX - All options accepting an argument can be used with '=' or ':' as a -separator. For the cases where it's not ambiguous (in particular -excluding boolean options), a space can also be used. The following -are all equivalent: +separator. Except for boolean options (wihch would be ambiguous), a +space can also be used as a separator. The following are all +equivalent: :: -- 2.26.2 ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
status of the new python bindings
Hi, I've started tinkering with the "new" Python bindings (python-cffi / python-notmuch2) and have a couple questions/comments about them: 1) What is the logic behind choosing whether something is exported as a property or as a method? E.g. Database.needs_upgrade is a property, while Database.revision() is a method. In my own python code, I tend to use @property for things that are "cheap" - i.e. do not involve (significant) IO or heavy computation and methods for those that do. But both of the above attributes involve library calls, presumably(?) of similar complexity. Would be nice if this was consistent. 2) Atomic transactions are now exported as a context manager, which is nice and convenient for the usual use cases, but AFAIU does not have the same power. E.g. my tagging script does the tagging as a single atomic transaction and has a "dry-run" mode in which it omits the end_atomic() call, which is documented to throw away all the changes. This seems to not be possible with the new bindings. Would it be okay to add bindings for explicitly calling start/end_atomic()? Or is my approach considered invalid? 3) There seem to be no bindings for notmuch_database_set_config(). 4) The setup for building the documentation seems to be missing. Anything else of note that remains to be implemented? -- Anton Khirnov ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 1/2] test-lib: mark function variables as local
On Wed, May 06 2020, Daniel Kahn Gillmor wrote:
> Several functions in test/test-lib.sh used variable names that are
> also used outside of those functions (e.g. $output and $expected are
> used in many of the test scripts), but they are not expected to
> communicate via those variables.
>
> We mark those variables "local" within test-lib.sh so that they do not
> get clobbered when used outside test-lib.
Good stuff
robustness comment IMO:
There is slight difference when writing
local foo=`false`
and
local foo; foo=`false`
former does not "fail"; latter does,
Although there is (currently!) no difference in our test code
(we don't have `set -e` there, IMO the former serves as a bad
example for anyone looking the code.
(same applies to export foo=`bar`, readonly foo=`bar` and so
on, for anyone curious...)
IMO better declare all local variables in one line separately,
e.g.
local output expected
and then either
output=$1
expected=$2
or
output=$1 expected=$2
( FYI: exection latter in shell differs in a way one could do
output=$expected expected=$output ) (IIRC, did not test >;)
(add double quotes around $1 and $2 if you desire =D)
well, when doing change just add the `local` line, smaller diff :)
Tomi
>
> Signed-off-by: Daniel Kahn Gillmor
> ---
> test/test-lib.sh | 44 ++--
> 1 file changed, 22 insertions(+), 22 deletions(-)
>
> diff --git a/test/test-lib.sh b/test/test-lib.sh
> index 5c8eab7c..e8feab3b 100644
> --- a/test/test-lib.sh
> +++ b/test/test-lib.sh
> @@ -109,7 +109,6 @@ unset ALTERNATE_EDITOR
>
> add_gnupg_home ()
> {
> -local output
> [ -e "${GNUPGHOME}/gpg.conf" ] && return
> _gnupg_exit () { gpgconf --kill all 2>/dev/null || true; }
> at_exit_function _gnupg_exit
> @@ -427,7 +426,7 @@ emacs_fcc_message ()
> # number of messages.
> add_email_corpus ()
> {
> -corpus=${1:-default}
> +local corpus=${1:-default}
>
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch
