On Wed, Jun 12, 2019 at 8:20 PM Stephen Hemminger <step...@networkplumber.org> wrote: > > On Wed, 12 Jun 2019 19:32:29 +0200 > Matteo Croce <mcr...@redhat.com> wrote: > > > On Wed, Jun 12, 2019 at 6:04 PM Matteo Croce <mcr...@redhat.com> wrote: > > > > > > On Wed, Jun 12, 2019 at 5:55 PM Stephen Hemminger > > > <step...@networkplumber.org> wrote: > > > > > > > > On Tue, 11 Jun 2019 20:03:26 +0200 > > > > Matteo Croce <mcr...@redhat.com> wrote: > > > > > > > > > Even if not running the testsuite, every build will leave > > > > > a stale tc_testkenv.* file in the system temp directory. > > > > > Conditionally create the temp file only if we're running the > > > > > testsuite. > > > > > > > > > > Signed-off-by: Matteo Croce <mcr...@redhat.com> > > > > > --- > > > > > testsuite/Makefile | 5 ++++- > > > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/testsuite/Makefile b/testsuite/Makefile > > > > > index 7f247bbc..5353244b 100644 > > > > > --- a/testsuite/Makefile > > > > > +++ b/testsuite/Makefile > > > > > @@ -14,7 +14,9 @@ TESTS_DIR := $(dir $(TESTS)) > > > > > > > > > > IPVERS := $(filter-out iproute2/Makefile,$(wildcard iproute2/*)) > > > > > > > > > > -KENVFN := $(shell mktemp /tmp/tc_testkenv.XXXXXX) > > > > > +ifeq ($(MAKECMDGOALS),alltests) > > > > > + KENVFN := $(shell mktemp /tmp/tc_testkenv.XXXXXX) > > > > > +endif > > > > > ifneq (,$(wildcard /proc/config.gz)) > > > > > KCPATH := /proc/config.gz > > > > > else > > > > > @@ -94,3 +96,4 @@ endif > > > > > rm "$$TMP_ERR" "$$TMP_OUT"; \ > > > > > sudo dmesg > $(RESULTS_DIR)/$@.$$o.dmesg; \ > > > > > done > > > > > + @$(RM) $(KENVFN) > > > > > > > > My concern is that there are several targets in this one Makefile. > > > > > > > > Why not use -u which gives name but does not create the file? > > > > > > As the manpage says, this is unsafe, as a file with the same name can > > > be created in the meantime. > > > Another option is to run the mktemp in the target shell, but this will > > > require to escape every single end of line to make it a single shell > > > command, e.g.: > > > > > > KENVFN=$$(mktemp /tmp/tc_testkenv.XXXXXX); \ > > > if [ "$(KCPATH)" = "/proc/config.gz" ]; then \ > > > gunzip -c $(KCPATH) >$$KENVFN; \ > > > ... > > > done ; \ > > > $(RM) $$KENVFN > > > > > > -- > > > Matteo Croce > > > per aspera ad upstream > > > > Anyway, looking for "tc" instead of "alltests" is probably better, as > > it only runs mktemp when at least the tc test is selected, both > > manually or via make check from topdir, eg.g > > > > ifeq ($(MAKECMDGOALS),tc) > > > > Do you agree? > > Why use /tmp at all for this config file?
To me any path could work, both /tmp or in the current dir, I have no preference. The important thing is to remove them wherever they are, as clobbering the build dir is bad as messing /tmp. Anyway, I double checked, and the only target which uses that temporary file is 'alltests' so, if the path is ok, I think that the condition "ifeq ($(MAKECMDGOALS),alltests)" is the only one which fixes the issue and keeps the behaviour unaltered. I did some quick tests and it works for me. Bye, -- Matteo Croce per aspera ad upstream
