[ccan] [PATCH 2/2] ccanlint: Move ccanlint test options from _info comments to code

Rusty Russell rusty at rustcorp.com.au
Tue Dec 27 14:35:07 AEDT 2016


David Gibson <david at gibson.dropbear.id.au> writes:
> Currently, _info files can specify options, or note expected failures, for
> ccanlint checks in the _info file with specially structured comments. That
> differs from most other things ccanlint gets from _info, where it instead
> executes the info file with certain parameters.
>
> This changes ccanlint and existing _info files to use the normal method for
> the ccanlint test options as well.  This also has the advantage that an
> info file can alter its test options based on things from config.h - in
> some cases whether a test can work or not might depend on various things.

Agreed, please apply!

Cheers,
Rusty.

> Signed-off-by: David Gibson <david at gibson.dropbear.id.au>
> ---
>  ccan/block_pool/_info              |  9 +++--
>  ccan/ccan_tokenizer/_info          | 14 ++++----
>  ccan/coroutine/_info               | 10 +++---
>  ccan/crcsync/_info                 |  8 +++--
>  ccan/deque/_info                   | 14 ++++----
>  ccan/generator/_info               | 14 ++++----
>  ccan/minmax/_info                  | 10 +++---
>  ccan/rszshm/_info                  | 12 ++++---
>  ccan/stringmap/_info               |  9 +++--
>  tools/ccanlint/ccanlint.c          | 71 +++++++++++++++++---------------------
>  tools/ccanlint/ccanlint.h          |  2 +-
>  tools/ccanlint/tests/info_exists.c |  2 +-
>  tools/depends.c                    | 18 ++++++++++
>  tools/tools.h                      |  3 ++
>  14 files changed, 115 insertions(+), 81 deletions(-)
>
> diff --git a/ccan/block_pool/_info b/ccan/block_pool/_info
> index a42cdba..e216976 100644
> --- a/ccan/block_pool/_info
> +++ b/ccan/block_pool/_info
> @@ -38,9 +38,6 @@
>   * Author: Joey Adams <joeyadams3.14159 at gmail.com>
>   * License: MIT
>   * Version: 0.1
> - * Ccanlint:
> - *	// We actually depend on the LGPL talloc
> - *	license_depends_compat FAIL
>   */
>  int main(int argc, char *argv[])
>  {
> @@ -53,5 +50,11 @@ int main(int argc, char *argv[])
>  		return 0;
>  	}
>  
> +	if (strcmp(argv[1], "ccanlint") == 0) {
> +		/* We actually depend on the LGPL talloc */
> +		printf("license_depends_compat FAIL\n");
> +		return 0;
> +	}
> +
>  	return 1;
>  }
> diff --git a/ccan/ccan_tokenizer/_info b/ccan/ccan_tokenizer/_info
> index ee1477a..f5116b4 100644
> --- a/ccan/ccan_tokenizer/_info
> +++ b/ccan/ccan_tokenizer/_info
> @@ -82,12 +82,6 @@
>   * }
>   *
>   * License: BSD (3 clause)
> - *
> - * Ccanlint:
> - *	// We actually depend on the LGPL dependencies
> - *	license_depends_compat FAIL
> - *	// We don't put the license line in all files.
> - *	license_comment FAIL
>   */
>  int main(int argc, char *argv[])
>  {
> @@ -101,5 +95,13 @@ int main(int argc, char *argv[])
>  		return 0;
>  	}
>  
> +	if (strcmp(argv[1], "ccanlint") == 0) {
> +		/* We actually depend on the LGPL dependencies */
> +		printf("license_depends_compat FAIL\n");
> +		/* We don't put the license line in all files. */
> +		printf("license_comment FAIL\n");
> +		return 0;
> +	}
> +
>  	return 1;
>  }
> diff --git a/ccan/coroutine/_info b/ccan/coroutine/_info
> index a99629c..07e4b96 100644
> --- a/ccan/coroutine/_info
> +++ b/ccan/coroutine/_info
> @@ -14,10 +14,6 @@
>   *
>   * Author: David Gibson <david at gibson.dropbear.id.au>
>   * License: LGPL (v2.1 or any later version)
> - *
> - * Ccanlint:
> - *	// Context switching really confuses valgrind
> - *	tests_pass_valgrind FAIL
>   */
>  int main(int argc, char *argv[])
>  {
> @@ -40,5 +36,11 @@ int main(int argc, char *argv[])
>  		return 0;
>  	}
>  
> +	if (strcmp(argv[1], "ccanlint") == 0) {
> +		/* Context switching really confuses valgrind */
> +		printf("tests_pass_valgrind FAIL\n");
> +		return 0;
> +	}
> +
>  	return 1;
>  }
> diff --git a/ccan/crcsync/_info b/ccan/crcsync/_info
> index 9ebb8bd..7516f44 100644
> --- a/ccan/crcsync/_info
> +++ b/ccan/crcsync/_info
> @@ -71,9 +71,6 @@
>   *
>   * License: LGPL (v2.1 or any later version)
>   * Author: Rusty Russell <rusty at rustcorp.com.au>
> - * Ccanlint:
> - *	// We actually depend on the GPL crc routines, so not really LGPL :(
> - *	license_depends_compat FAIL
>   */
>  int main(int argc, char *argv[])
>  {
> @@ -88,6 +85,11 @@ int main(int argc, char *argv[])
>  		printf("ccan/array_size\n");
>  		return 0;
>  	}
> +	if (strcmp(argv[1], "ccanlint") == 0) {
> +		/* We actually depend on the GPL crc routines, so not really LGPL :( */
> +		printf("license_depends_compat FAIL\n");
> +		return 0;
> +	}
>  
>  	return 1;
>  }
> diff --git a/ccan/deque/_info b/ccan/deque/_info
> index 63687b7..1c5460f 100644
> --- a/ccan/deque/_info
> +++ b/ccan/deque/_info
> @@ -122,12 +122,6 @@
>   *
>   * License: APACHE-2
>   * Author: Dan Good <dan at dancancode.com>
> - *
> - * Ccanlint:
> - *	// uses statement expressions
> - *	// supported by gcc, clang, icc, and some others, but not msvc
> - *	// (see https://gcc.gnu.org/onlinedocs/gcc/Statement-Exprs.html)
> - *	objects_build_without_features FAIL
>   */
>  int main(int argc, char *argv[])
>  {
> @@ -142,5 +136,13 @@ int main(int argc, char *argv[])
>  		return 0;
>  	}
>  
> +	if (strcmp(argv[1], "ccanlint") == 0) {
> +		/* uses statement expressions
> +		 * supported by gcc, clang, icc, and some others, but not msvc
> +		 * (see https://gcc.gnu.org/onlinedocs/gcc/Statement-Exprs.html) */
> +		printf("objects_build_without_features FAIL\n");
> +		return 0;
> +	}
> +
>  	return 1;
>  }
> diff --git a/ccan/generator/_info b/ccan/generator/_info
> index e40d7bb..aa6048e 100644
> --- a/ccan/generator/_info
> +++ b/ccan/generator/_info
> @@ -43,12 +43,6 @@
>   *
>   * Author: David Gibson <david at gibson.dropbear.id.au>
>   * License: LGPL (v2.1 or any later version)
> - *
> - * Ccanlint:
> - *      // We need several gcc extensions
> - *	objects_build_without_features FAIL
> - *      tests_compile_without_features FAIL
> - *	tests_helpers_compile_without_features FAIL
>   */
>  int main(int argc, char *argv[])
>  {
> @@ -78,5 +72,13 @@ int main(int argc, char *argv[])
>  		return 0;
>  	}
>  
> +	if (strcmp(argv[1], "ccanlint") == 0) {
> +		/* We need several gcc extensions */
> +		printf("objects_build_without_features FAIL\n");
> +		printf("tests_compile_without_features FAIL\n");
> +		printf("tests_helpers_compile_without_features FAIL\n");
> +		return 0;
> +	}
> +
>  	return 1;
>  }
> diff --git a/ccan/minmax/_info b/ccan/minmax/_info
> index 4d95336..2f6437e 100644
> --- a/ccan/minmax/_info
> +++ b/ccan/minmax/_info
> @@ -26,10 +26,6 @@
>   *
>   * Author: David Gibson <david at gibson.dropbear.id.au>
>   * License:  CC0 (Public domain)
> - *
> - * Ccanlint:
> - *      // We need several gcc extensions
> - *      tests_compile_without_features FAIL
>   */
>  int main(int argc, char *argv[])
>  {
> @@ -42,5 +38,11 @@ int main(int argc, char *argv[])
>  		return 0;
>  	}
>  
> +	if (strcmp(argv[1], "ccanlint") == 0) {
> +		/* We need several gcc extensions */
> +		printf("tests_compile_without_features FAIL\n");
> +		return 0;
> +	}
> +
>  	return 1;
>  }
> diff --git a/ccan/rszshm/_info b/ccan/rszshm/_info
> index cc95400..e9122e0 100644
> --- a/ccan/rszshm/_info
> +++ b/ccan/rszshm/_info
> @@ -76,13 +76,8 @@
>   * 	// $ tail -c +29 /dev/shm/rszshm_LAsEvt/0 | sed 's/./&\n/g' | sort | uniq -c | tr '\n' '\t'; echo
>   * 	//  515532 A   527251 B    512930 C    513062 D    544326 E    545876 F    512936 G    522363 H
>   *
> - * Ccanlint: tests_pass_valgrind FAIL
>   * License: APACHE-2
>   * Author: Dan Good <dan at dancancode.com>
> - *
> - * Ccanlint:
> - *	// tests use optional macros containing statement expressions
> - *	tests_compile_without_features FAIL
>   */
>  int main(int argc, char *argv[])
>  {
> @@ -93,5 +88,12 @@ int main(int argc, char *argv[])
>  	if (strcmp(argv[1], "depends") == 0)
>  		return 0;
>  
> +	if (strcmp(argv[1], "ccanlint") == 0) {
> +		printf("tests_pass_valgrind FAIL\n");
> +		/* tests use optional macros containing statement expressions */
> +		printf("tests_compile_without_features FAIL\n");
> +		return 0;
> +	}
> +
>  	return 1;
>  }
> diff --git a/ccan/stringmap/_info b/ccan/stringmap/_info
> index 1f6465b..c01c576 100644
> --- a/ccan/stringmap/_info
> +++ b/ccan/stringmap/_info
> @@ -52,9 +52,6 @@
>   * Authors: Joey Adams, Anders Magnusson
>   * License: BSD (3 clause)
>   * Version: 0.2
> - * Ccanlint:
> - *	// We actually depend (indirectly) on the LGPL talloc
> - *	license_depends_compat FAIL
>   */
>  int main(int argc, char *argv[])
>  {
> @@ -67,5 +64,11 @@ int main(int argc, char *argv[])
>  		return 0;
>  	}
>  
> +	if (strcmp(argv[1], "ccanlint") == 0) {
> +		/* We actually depend (indirectly) on the LGPL talloc */
> +		printf("license_depends_compat FAIL\n");
> +		return 0;
> +	}
> +
>  	return 1;
>  }
> diff --git a/tools/ccanlint/ccanlint.c b/tools/ccanlint/ccanlint.c
> index 0ed969c..f06f200 100644
> --- a/tools/ccanlint/ccanlint.c
> +++ b/tools/ccanlint/ccanlint.c
> @@ -456,50 +456,43 @@ static void add_options(struct ccanlint *test, char **options,
>  	memcpy(&test->options[num], options, (num_options + 1)*sizeof(char *));
>  }
>  
> -void add_info_options(struct ccan_file *info)
> +void add_info_options(struct manifest *m)
>  {
> -	struct doc_section *d;
>  	unsigned int i;
> -	struct ccanlint *test;
> +	char **info_options = get_ccanlint(m, m->dir, get_or_compile_info);
>  
> -	list_for_each(get_ccan_file_docs(info), d, list) {
> -		if (!streq(d->type, "ccanlint"))
> +	for (i = 0; info_options[i]; i++) {
> +		char **words = tal_strsplit(m, info_options[i], " \t",
> +					    STR_NO_EMPTY);
> +		struct ccanlint *test;
> +
> +		if (!words[0])
> +			continue;
> +
> +		test = find_test(words[0]);
> +		if (!test) {
> +			warnx("%s: unknown ccanlint test '%s'",
> +			      m->info_file->fullname, words[0]);
>  			continue;
> +		}
> +
> +		if (!words[1]) {
> +			warnx("%s: no argument to test '%s'",
> +			      m->info_file->fullname, words[0]);
> +			continue;
> +		}
>  
> -		for (i = 0; i < d->num_lines; i++) {
> -			char **words = tal_strsplit(d, d->lines[i], " \t",
> -						    STR_NO_EMPTY);
> -			if (!words[0])
> -				continue;
> -
> -			if (strncmp(words[0], "//", 2) == 0)
> -				continue;
> -
> -			test = find_test(words[0]);
> -			if (!test) {
> -				warnx("%s: unknown ccanlint test '%s'",
> -				      info->fullname, words[0]);
> -				continue;
> -			}
> -
> -			if (!words[1]) {
> -				warnx("%s: no argument to test '%s'",
> -				      info->fullname, words[0]);
> -				continue;
> -			}
> -
> -			/* Known failure? */
> -			if (strcasecmp(words[1], "FAIL") == 0) {
> -				if (!targeting)
> -					skip_test_and_deps(test,
> -							   "excluded in _info"
> -							   " file");
> -			} else {
> -				if (!test->takes_options)
> -					warnx("%s: %s doesn't take options",
> -					      info->fullname, words[0]);
> -				add_options(test, words+1, tal_count(words)-1);
> -			}
> +		/* Known failure? */
> +		if (strcasecmp(words[1], "FAIL") == 0) {
> +			if (!targeting)
> +				skip_test_and_deps(test,
> +						   "excluded in _info"
> +						   " file");
> +		} else {
> +			if (!test->takes_options)
> +				warnx("%s: %s doesn't take options",
> +				      m->info_file->fullname, words[0]);
> +			add_options(test, words+1, tal_count(words)-1);
>  		}
>  	}
>  }
> diff --git a/tools/ccanlint/ccanlint.h b/tools/ccanlint/ccanlint.h
> index 9cb858f..7df9510 100644
> --- a/tools/ccanlint/ccanlint.h
> +++ b/tools/ccanlint/ccanlint.h
> @@ -186,7 +186,7 @@ struct dependent {
>  bool is_excluded(const char *name);
>  
>  /* Called to add options from _info, once it's located. */
> -void add_info_options(struct ccan_file *info);
> +void add_info_options(struct manifest *m);
>  
>  /* Are we happy to compile stuff, or just non-intrusive tests? */
>  extern bool safe_mode;
> diff --git a/tools/ccanlint/tests/info_exists.c b/tools/ccanlint/tests/info_exists.c
> index 5e4459b..8f01946 100644
> --- a/tools/ccanlint/tests/info_exists.c
> +++ b/tools/ccanlint/tests/info_exists.c
> @@ -20,7 +20,7 @@ static void check_has_info(struct manifest *m,
>  	if (m->info_file) {
>  		score->pass = true;
>  		score->score = score->total;
> -		add_info_options(m->info_file);
> +		add_info_options(m);
>  	} else {
>  		score->error = tal_strdup(score,
>  	"You have no _info file.\n\n"
> diff --git a/tools/depends.c b/tools/depends.c
> index f1ff309..95bda1b 100644
> --- a/tools/depends.c
> +++ b/tools/depends.c
> @@ -253,6 +253,12 @@ static char **get_one_cflags(const void *ctx, const char *dir,
>  	return get_one_prop(ctx, dir, "cflags", get_info);
>  }
>  
> +static char **get_one_ccanlint(const void *ctx, const char *dir,
> +			       char *(*get_info)(const void *ctx, const char *dir))
> +{
> +	return get_one_prop(ctx, dir, "ccanlint", get_info);
> +}
> +
>  /* O(n^2) but n is small. */
>  static char **add_deps(char **deps1, char **deps2)
>  {
> @@ -282,6 +288,18 @@ char **get_cflags(const void *ctx, const char *dir,
>  	return flags;
>  }
>  
> +char **get_ccanlint(const void *ctx, const char *dir,
> +		    char *(*get_info)(const void *ctx, const char *dir))
> +{
> +	char **ccanlint;
> +	unsigned int len;
> +	ccanlint = get_one_ccanlint(ctx, dir, get_info);
> +	len = tal_count(ccanlint);
> +	tal_resize(&ccanlint, len + 1);
> +	ccanlint[len] = NULL;
> +	return ccanlint;
> +}
> +
>  static char *get_one_ported(const void *ctx, const char *dir,
>  			    char *(*get_info)(const void *ctx, const char *dir))
>  {
> diff --git a/tools/tools.h b/tools/tools.h
> index 668c91a..d29a25f 100644
> --- a/tools/tools.h
> +++ b/tools/tools.h
> @@ -48,6 +48,9 @@ char **get_libs(const void *ctx, const char *dir, const char *style,
>  char **get_cflags(const void *ctx, const char *dir,
>  		char *(*get_info)(const void *ctx, const char *dir));
>  
> +char **get_ccanlint(const void *ctx, const char *dir,
> +		    char *(*get_info)(const void *ctx, const char *dir));
> +
>  char *get_ported(const void *ctx, const char *dir, bool recurse,
>  		 char *(*get_info)(const void *ctx, const char *dir));
>  
> -- 
> 2.9.3


More information about the ccan mailing list