From 3612661714e86333ceacca7314959a5ed938dc6a Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 25 Sep 2009 17:04:04 +0930 Subject: [PATCH] ccanlint: only compile _info once; speeds tdb test from 18 to 16 seconds. Generally clean up temp file handling. --- tools/Makefile | 12 ++- tools/ccanlint/Makefile | 4 +- tools/ccanlint/ccanlint.h | 2 +- tools/ccanlint/tests/build_objs.c | 11 +-- tools/ccanlint/tests/check_build.c | 20 ++-- tools/ccanlint/tests/check_depends_exist.c | 6 +- tools/ccanlint/tests/check_includes_build.c | 15 +-- tools/ccanlint/tests/compile_test_helpers.c | 9 +- tools/ccanlint/tests/compile_tests.c | 25 +++-- tools/compile.c | 32 +++++++ tools/depends.c | 101 +++++++++++++------- tools/namespacize.c | 4 +- tools/tools.c | 39 ++++++++ tools/tools.h | 18 +++- 14 files changed, 194 insertions(+), 104 deletions(-) create mode 100644 tools/compile.c diff --git a/tools/Makefile b/tools/Makefile index f3575b3d..8301ba52 100644 --- a/tools/Makefile +++ b/tools/Makefile @@ -1,19 +1,21 @@ ALL_TOOLS = tools/ccan_depends tools/run_tests tools/doc_extract tools/namespacize tools/ccanlint/ccanlint +DEP_OBJS = tools/depends.o tools/compile.o tools/tools.o ccan/str_talloc/str_talloc.o ccan/grab_file/grab_file.o ccan/talloc/talloc.o ccan/noerr/noerr.o ccan/read_write_all/read_write_all.o + .PHONY: tools tools: $(ALL_TOOLS) -tools/ccan_depends: tools/ccan_depends.o tools/depends.o tools/tools.o ccan/str_talloc/str_talloc.o ccan/grab_file/grab_file.o ccan/talloc/talloc.o ccan/noerr/noerr.o +tools/ccan_depends: tools/ccan_depends.o $(DEP_OBJS) -tools/run_tests: tools/run_tests.o tools/depends.o ccan/str_talloc/str_talloc.o ccan/grab_file/grab_file.o ccan/tap/tap.o ccan/noerr/noerr.o ccan/talloc/talloc.o +tools/run_tests: tools/run_tests.o ccan/tap/tap.o $(DEP_OBJS) -tools/doc_extract: tools/doc_extract.o tools/doc_extract-core.o ccan/str_talloc/str_talloc.o ccan/grab_file/grab_file.o ccan/noerr/noerr.o ccan/talloc/talloc.o +tools/doc_extract: tools/doc_extract.o tools/doc_extract-core.o $(DEP_OBJS) -tools/namespacize: tools/namespacize.o tools/depends.o tools/tools.o ccan/str_talloc/str_talloc.o ccan/grab_file/grab_file.o ccan/noerr/noerr.o ccan/talloc/talloc.o +tools/namespacize: tools/namespacize.o $(DEP_OBJS) tools/run_tests.o tools/namespacize.o tools/depends.o: tools/tools.h tools-clean: ccanlint-clean - rm -f $(ALL_TOOLS) + $(RM) $(ALL_TOOLS) include tools/ccanlint/Makefile diff --git a/tools/ccanlint/Makefile b/tools/ccanlint/Makefile index dcbd6f9c..9efed3a5 100644 --- a/tools/ccanlint/Makefile +++ b/tools/ccanlint/Makefile @@ -7,8 +7,10 @@ CORE_OBJS := tools/ccanlint/ccanlint.o \ tools/doc_extract-core.o \ tools/depends.o \ tools/tools.o \ + tools/compile.o \ ccan/str_talloc/str_talloc.o ccan/grab_file/grab_file.o \ - ccan/talloc/talloc.o ccan/noerr/noerr.o + ccan/talloc/talloc.o ccan/noerr/noerr.o \ + ccan/read_write_all/read_write_all.o OBJS := $(CORE_OBJS) $(TEST_OBJS) diff --git a/tools/ccanlint/ccanlint.h b/tools/ccanlint/ccanlint.h index c4ae191f..e019c139 100644 --- a/tools/ccanlint/ccanlint.h +++ b/tools/ccanlint/ccanlint.h @@ -128,7 +128,7 @@ struct ccan_file { struct list_head *doc_sections; /* If this file gets compiled (eg. .C file to .o file), result here. */ - const char *compiled; + char *compiled; }; /* A new ccan_file, with the given name (talloc_steal onto returned value). */ diff --git a/tools/ccanlint/tests/build_objs.c b/tools/ccanlint/tests/build_objs.c index d932ec74..e0ea2223 100644 --- a/tools/ccanlint/tests/build_objs.c +++ b/tools/ccanlint/tests/build_objs.c @@ -23,15 +23,14 @@ static const char *can_build(struct manifest *m) static bool compile_obj(struct ccan_file *c_file, char *objfile, char **report) { - char *contents; + char *err; - contents = run_command(objfile, "cc " CFLAGS " -o %s -c %s", - objfile, c_file->name); - if (contents) { + err = compile_object(objfile, objfile, c_file->name); + if (err) { if (*report) - *report = talloc_append_string(*report, contents); + *report = talloc_append_string(*report, err); else - *report = contents; + *report = err; return false; } return true; diff --git a/tools/ccanlint/tests/check_build.c b/tools/ccanlint/tests/check_build.c index dcba0e49..eb968e11 100644 --- a/tools/ccanlint/tests/check_build.c +++ b/tools/ccanlint/tests/check_build.c @@ -21,12 +21,6 @@ static const char *can_build(struct manifest *m) return NULL; } -static int cleanup_testfile(char *testfile) -{ - unlink(testfile); - return 0; -} - static char *obj_list(const struct manifest *m) { char *list = talloc_strdup(m, ""); @@ -42,7 +36,7 @@ static char *obj_list(const struct manifest *m) static char *lib_list(const struct manifest *m) { unsigned int i, num; - char **libs = get_libs(m, ".", ".", &num); + char **libs = get_libs(m, ".", ".", &num, &m->info_file->compiled); char *ret = talloc_strdup(m, ""); for (i = 0; i < num; i++) @@ -53,13 +47,10 @@ static char *lib_list(const struct manifest *m) static void *check_use_build(struct manifest *m) { char *contents; - char *tmpfile, *outfile; + char *tmpfile, *err; int fd; - tmpfile = talloc_strdup(m, tempnam("/tmp", "ccanlint")); - talloc_set_destructor(tmpfile, cleanup_testfile); - outfile = talloc_strdup(m, tempnam("/tmp", "ccanlint")); - talloc_set_destructor(outfile, cleanup_testfile); + tmpfile = temp_file(m, ".c"); fd = open(tmpfile, O_WRONLY | O_CREAT | O_EXCL, 0600); if (fd < 0) @@ -79,8 +70,9 @@ static void *check_use_build(struct manifest *m) } close(fd); - return run_command(m, "cc " CFLAGS " -o %s -x c %s -x none %s %s", - outfile, tmpfile, obj_list(m), lib_list(m)); + if (!compile_and_link(m, tmpfile, obj_list(m), "", lib_list(m), &err)) + return err; + return NULL; } static const char *describe_use_build(struct manifest *m, void *check_result) diff --git a/tools/ccanlint/tests/check_depends_exist.c b/tools/ccanlint/tests/check_depends_exist.c index 0978340f..2ac60a2a 100644 --- a/tools/ccanlint/tests/check_depends_exist.c +++ b/tools/ccanlint/tests/check_depends_exist.c @@ -40,9 +40,11 @@ static void *check_depends_exist(struct manifest *m) char **deps; if (safe_mode) - deps = get_safe_ccan_deps(m, "..", m->basename, true); + deps = get_safe_ccan_deps(m, "..", m->basename, true, + &m->info_file->compiled); else - deps = get_deps(m, "..", m->basename, true); + deps = get_deps(m, "..", m->basename, true, + &m->info_file->compiled); for (i = 0; deps[i]; i++) { if (!strstarts(deps[i], "ccan/")) diff --git a/tools/ccanlint/tests/check_includes_build.c b/tools/ccanlint/tests/check_includes_build.c index 33668ed8..d9c6f495 100644 --- a/tools/ccanlint/tests/check_includes_build.c +++ b/tools/ccanlint/tests/check_includes_build.c @@ -22,22 +22,14 @@ static const char *can_build(struct manifest *m) return NULL; } -static int cleanup_testfile(char *testfile) -{ - unlink(testfile); - return 0; -} - static void *check_includes_build(struct manifest *m) { char *contents; char *tmpfile, *objfile; int fd; - tmpfile = talloc_strdup(m, tempnam("/tmp", "ccanlint")); - talloc_set_destructor(tmpfile, cleanup_testfile); - objfile = talloc_strdup(m, tempnam("/tmp", "ccanlint")); - talloc_set_destructor(objfile, cleanup_testfile); + tmpfile = temp_file(m, ".c"); + objfile = temp_file(m, ".o"); fd = open(tmpfile, O_WRONLY | O_CREAT | O_EXCL, 0600); if (fd < 0) @@ -52,8 +44,7 @@ static void *check_includes_build(struct manifest *m) } close(fd); - return run_command(m, "cc " CFLAGS " -o %s -c -x c %s", - objfile, tmpfile); + return compile_object(m, objfile, tmpfile); } static const char *describe_includes_build(struct manifest *m, diff --git a/tools/ccanlint/tests/compile_test_helpers.c b/tools/ccanlint/tests/compile_test_helpers.c index 017a5308..2aa8d863 100644 --- a/tools/ccanlint/tests/compile_test_helpers.c +++ b/tools/ccanlint/tests/compile_test_helpers.c @@ -21,12 +21,6 @@ static const char *can_build(struct manifest *m) return NULL; } -static int cleanup_testfile(char *testfile) -{ - unlink(testfile); - return 0; -} - static char *objname(const void *ctx, const char *cfile) { return talloc_asprintf(ctx, "%.*s.o ", strlen(cfile) - 2, cfile); @@ -37,8 +31,7 @@ static char *compile(struct manifest *m, const char *cfile) char *obj; obj = objname(m, cfile); - talloc_set_destructor(obj, cleanup_testfile); - return run_command(m, "cc " CFLAGS " -c -o %s %s", obj, cfile); + return compile_object(m, obj, cfile); } static void *do_compile_test_helpers(struct manifest *m) diff --git a/tools/ccanlint/tests/compile_tests.c b/tools/ccanlint/tests/compile_tests.c index 7f0338a4..1d24f655 100644 --- a/tools/ccanlint/tests/compile_tests.c +++ b/tools/ccanlint/tests/compile_tests.c @@ -40,7 +40,7 @@ static char *obj_list(const struct manifest *m, bool link_with_module) static char *lib_list(const struct manifest *m) { unsigned int i, num; - char **libs = get_libs(m, ".", ".", &num); + char **libs = get_libs(m, ".", ".", &num, &m->info_file->compiled); char *ret = talloc_strdup(m, ""); for (i = 0; i < num; i++) @@ -48,23 +48,20 @@ static char *lib_list(const struct manifest *m) return ret; } -static int cleanup_testfile(const char *testfile) -{ - unlink(testfile); - return 0; -} - static char *compile(const void *ctx, struct manifest *m, struct ccan_file *file, bool fail, bool link_with_module) { - file->compiled = talloc_strdup(ctx, tempnam("/tmp", "ccanlint")); - talloc_set_destructor(file->compiled, cleanup_testfile); - - return run_command(m, "cc " CFLAGS " %s -o %s %s %s %s", - fail ? "-DFAIL" : "", - file->compiled, file->name, - obj_list(m, link_with_module), lib_list(m)); + char *errmsg; + + file->compiled = compile_and_link(ctx, file->name, + obj_list(m, link_with_module), + fail ? "-DFAIL" : "", + lib_list(m), &errmsg); + if (!file->compiled) + return errmsg; + talloc_steal(ctx, file->compiled); + return NULL; } struct compile_tests_result { diff --git a/tools/compile.c b/tools/compile.c new file mode 100644 index 00000000..5ec41228 --- /dev/null +++ b/tools/compile.c @@ -0,0 +1,32 @@ +#include "tools.h" +#include +#include + +/* Compile multiple object files into a single. Returns errmsg if fails. */ +char *link_objects(const void *ctx, const char *outfile, const char *objs) +{ + return run_command(ctx, "cc " CFLAGS " -c -o %s %s", outfile, objs); +} + +/* Compile a single C file to an object file. Returns errmsg if fails. */ +char *compile_object(const void *ctx, const char *outfile, const char *cfile) +{ + return run_command(ctx, "cc " CFLAGS " -c -o %s %s", outfile, cfile); +} + +/* Compile and link single C file, with object files. + * Returns name of result, or NULL (and fills in errmsg). */ +char *compile_and_link(const void *ctx, const char *cfile, const char *objs, + const char *extra_cflags, const char *libs, + char **errmsg) +{ + char *file = temp_file(ctx, ""); + + *errmsg = run_command(ctx, "cc " CFLAGS " %s -o %s %s %s %s", + extra_cflags, file, cfile, objs, libs); + if (*errmsg) { + talloc_free(file); + return NULL; + } + return file; +} diff --git a/tools/depends.c b/tools/depends.c index 203788fe..6c61cca0 100644 --- a/tools/depends.c +++ b/tools/depends.c @@ -2,7 +2,11 @@ #include #include #include +#include #include "tools.h" +#include +#include +#include #include #include #include @@ -31,36 +35,46 @@ lines_from_cmd(const void *ctx, unsigned int *num, char *format, ...) return strsplit(ctx, buffer, "\n", num); } -static int unlink_info(char *infofile) -{ - unlink(infofile); - return 0; -} - -/* Be careful about trying to compile over running programs (parallel make) */ +/* Be careful about trying to compile over running programs (parallel make). + * temp_file helps here. */ static char *compile_info(const void *ctx, const char *dir, const char *name) { - char *infofile = talloc_asprintf(ctx, "%s/info.%u", dir, getpid()); - char *cmd = talloc_asprintf(ctx, "cc " CFLAGS - " -o %s -x c %s/%s/_info", - infofile, dir, name); - talloc_set_destructor(infofile, unlink_info); - if (system(cmd) != 0) + char *info_c_file, *info, *errmsg; + size_t len; + int fd; + + /* Copy it to a file with proper .c suffix. */ + info = grab_file(ctx, talloc_asprintf(ctx, "%s/%s/_info", dir, name), + &len); + if (!info) + return NULL; + + info_c_file = temp_file(ctx, ".c"); + fd = open(info_c_file, O_WRONLY|O_CREAT|O_EXCL, 0600); + if (fd < 0) + return NULL; + if (!write_all(fd, info, len)) + return NULL; + + if (close(fd) != 0) return NULL; - return infofile; + return compile_and_link(ctx, info_c_file, "", "", "", &errmsg); } static char **get_one_deps(const void *ctx, const char *dir, - const char *name, unsigned int *num) + const char *name, unsigned int *num, + char **infofile) { - char **deps, *cmd, *infofile; + char **deps, *cmd; - infofile = compile_info(ctx, dir, name); - if (!infofile) - errx(1, "Could not compile _info for '%s'", name); + if (!*infofile) { + *infofile = compile_info(ctx, dir, name); + if (!*infofile) + errx(1, "Could not compile _info for '%s'", name); + } - cmd = talloc_asprintf(ctx, "%s depends", infofile); + cmd = talloc_asprintf(ctx, "%s depends", *infofile); deps = lines_from_cmd(cmd, num, "%s", cmd); if (!deps) err(1, "Could not run '%s'", cmd); @@ -97,7 +111,8 @@ static char *replace(const void *ctx, const char *src, /* This is a terrible hack. We scan for ccan/ strings. */ static char **get_one_safe_deps(const void *ctx, const char *dir, const char *name, - unsigned int *num) + unsigned int *num, + char **infofile) { char **deps, **lines, *raw, *fname; unsigned int i, n = 0; @@ -155,13 +170,14 @@ static bool have_dep(char **deps, unsigned int num, const char *dep) /* Gets all the dependencies, recursively. */ static char ** get_all_deps(const void *ctx, const char *dir, const char *name, + char **infofile, char **(*get_one)(const void *, const char *, const char *, - unsigned int *)) + unsigned int *, char **)) { char **deps; unsigned int i, num; - deps = get_one(ctx, dir, name, &num); + deps = get_one(ctx, dir, name, &num, infofile); for (i = 0; i < num; i++) { char **newdeps; unsigned int j, newnum; @@ -170,7 +186,7 @@ get_all_deps(const void *ctx, const char *dir, const char *name, continue; newdeps = get_one(ctx, dir, deps[i] + strlen("ccan/"), - &newnum); + &newnum, infofile); /* Should be short, so brute-force out dups. */ for (j = 0; j < newnum; j++) { @@ -186,15 +202,18 @@ get_all_deps(const void *ctx, const char *dir, const char *name, } char **get_libs(const void *ctx, const char *dir, - const char *name, unsigned int *num) + const char *name, unsigned int *num, + char **infofile) { - char **libs, *cmd, *infofile; + char **libs, *cmd; - infofile = compile_info(ctx, dir, name); - if (!infofile) - errx(1, "Could not compile _info for '%s'", name); + if (!*infofile) { + *infofile = compile_info(ctx, dir, name); + if (!*infofile) + errx(1, "Could not compile _info for '%s'", name); + } - cmd = talloc_asprintf(ctx, "%s libs", infofile); + cmd = talloc_asprintf(ctx, "%s libs", *infofile); libs = lines_from_cmd(cmd, num, "%s", cmd); if (!libs) err(1, "Could not run '%s'", cmd); @@ -202,21 +221,31 @@ char **get_libs(const void *ctx, const char *dir, } char **get_deps(const void *ctx, const char *dir, const char *name, - bool recurse) + bool recurse, char **infofile) { + char *temp = NULL, **ret; + if (!infofile) + infofile = &temp; + if (!recurse) { unsigned int num; - return get_one_deps(ctx, dir, name, &num); + ret = get_one_deps(ctx, dir, name, &num, infofile); + } else + ret = get_all_deps(ctx, dir, name, infofile, get_one_deps); + + if (infofile == &temp && temp) { + unlink(temp); + talloc_free(temp); } - return get_all_deps(ctx, dir, name, get_one_deps); + return ret; } char **get_safe_ccan_deps(const void *ctx, const char *dir, - const char *name, bool recurse) + const char *name, bool recurse, char **infofile) { if (!recurse) { unsigned int num; - return get_one_safe_deps(ctx, dir, name, &num); + return get_one_safe_deps(ctx, dir, name, &num, infofile); } - return get_all_deps(ctx, dir, name, get_one_safe_deps); + return get_all_deps(ctx, dir, name, infofile, get_one_safe_deps); } diff --git a/tools/namespacize.c b/tools/namespacize.c index 2e6dcbfb..3a3fe47e 100644 --- a/tools/namespacize.c +++ b/tools/namespacize.c @@ -458,7 +458,7 @@ static void adjust_dir(const char *dir) verbose("Adjusting %s\n", dir); verbose_indent(); for (deps = get_deps(parent, parent, talloc_basename(parent, dir), - false); + false, NULL); *deps; deps++) { char *depdir; struct adjusted *adj = NULL; @@ -498,7 +498,7 @@ static void adjust_dependents(const char *dir) continue; for (deps = get_deps(*file, talloc_dirname(*file, *file), - talloc_basename(*file, *file), false); + talloc_basename(*file, *file), false, NULL); *deps; deps++) { if (!strstarts(*deps, "ccan/")) continue; diff --git a/tools/tools.c b/tools/tools.c index d50a67fd..2da37234 100644 --- a/tools/tools.c +++ b/tools/tools.c @@ -1,11 +1,17 @@ #include #include +#include +#include #include #include #include #include +#include #include "tools.h" +static char *tmpdir = NULL; +static unsigned int count; + char *talloc_basename(const void *ctx, const char *dir) { char *p = strrchr(dir, '/'); @@ -42,6 +48,7 @@ char *talloc_getcwd(const void *ctx) return cwd; } +/* Returns output if command fails. */ char *run_command(const void *ctx, const char *fmt, ...) { va_list ap; @@ -67,3 +74,35 @@ char *run_command(const void *ctx, const char *fmt, ...) talloc_free(cmd); return NULL; } + +static int unlink_all(char *dir) +{ + char cmd[strlen(dir) + sizeof("rm -rf ")]; + sprintf(cmd, "rm -rf %s", dir); +// system(cmd); + return 0; +} + +char *temp_file(const void *ctx, const char *extension) +{ + /* For first call, create dir. */ + while (!tmpdir) { + tmpdir = getenv("TMPDIR"); + if (!tmpdir) + tmpdir = "/tmp"; + tmpdir = talloc_asprintf(talloc_autofree_context(), + "%s/ccanlint-%u.%lu", + tmpdir, getpid(), random()); + if (mkdir(tmpdir, 0700) != 0) { + if (errno == EEXIST) { + talloc_free(tmpdir); + tmpdir = NULL; + continue; + } + err(1, "mkdir %s failed", tmpdir); + } + talloc_set_destructor(tmpdir, unlink_all); + } + + return talloc_asprintf(ctx, "%s/%u%s", tmpdir, count++, extension); +} diff --git a/tools/tools.h b/tools/tools.h index 440abc89..c311809e 100644 --- a/tools/tools.h +++ b/tools/tools.h @@ -13,19 +13,31 @@ /* This actually compiles and runs the info file to get dependencies. */ char **get_deps(const void *ctx, const char *dir, const char *name, - bool recurse); + bool recurse, char **infofile); /* This is safer: just looks for ccan/ strings in info */ char **get_safe_ccan_deps(const void *ctx, const char *dir, const char *name, - bool recurse); + bool recurse, char **infofile); /* This also needs to compile the info file. */ char **get_libs(const void *ctx, const char *dir, - const char *name, unsigned int *num); + const char *name, unsigned int *num, char **infofile); /* From tools.c */ char *talloc_basename(const void *ctx, const char *dir); char *talloc_dirname(const void *ctx, const char *dir); char *talloc_getcwd(const void *ctx); char *run_command(const void *ctx, const char *fmt, ...); +char *temp_file(const void *ctx, const char *extension); + +/* From compile.c. */ +/* Compile multiple object files into a single. Returns errmsg if fails. */ +char *link_objects(const void *ctx, const char *outfile, const char *objs); +/* Compile a single C file to an object file. Returns errmsg if fails. */ +char *compile_object(const void *ctx, const char *outfile, const char *cfile); +/* Compile and link single C file, with object files. + * Returns name of result, or NULL (and fills in errmsg). */ +char *compile_and_link(const void *ctx, const char *cfile, const char *objs, + const char *extra_cflags, const char *libs, + char **errmsg); #endif /* CCAN_TOOLS_H */ -- 2.39.2