* [PATCH iproute2] testsuite: don't clobber /tmp @ 2019-06-11 18:03 Matteo Croce 2019-06-12 15:53 ` Stephen Hemminger 0 siblings, 1 reply; 8+ messages in thread From: Matteo Croce @ 2019-06-11 18:03 UTC (permalink / raw) To: netdev; +Cc: Stephen Hemminger, David Ahern 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 <mcroce@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) -- 2.21.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH iproute2] testsuite: don't clobber /tmp 2019-06-11 18:03 [PATCH iproute2] testsuite: don't clobber /tmp Matteo Croce @ 2019-06-12 15:53 ` Stephen Hemminger 2019-06-12 16:04 ` Matteo Croce 0 siblings, 1 reply; 8+ messages in thread From: Stephen Hemminger @ 2019-06-12 15:53 UTC (permalink / raw) To: Matteo Croce; +Cc: netdev, David Ahern On Tue, 11 Jun 2019 20:03:26 +0200 Matteo Croce <mcroce@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 <mcroce@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? ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH iproute2] testsuite: don't clobber /tmp 2019-06-12 15:53 ` Stephen Hemminger @ 2019-06-12 16:04 ` Matteo Croce 2019-06-12 17:32 ` Matteo Croce 0 siblings, 1 reply; 8+ messages in thread From: Matteo Croce @ 2019-06-12 16:04 UTC (permalink / raw) To: Stephen Hemminger; +Cc: netdev, David Ahern On Wed, Jun 12, 2019 at 5:55 PM Stephen Hemminger <stephen@networkplumber.org> wrote: > > On Tue, 11 Jun 2019 20:03:26 +0200 > Matteo Croce <mcroce@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 <mcroce@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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH iproute2] testsuite: don't clobber /tmp 2019-06-12 16:04 ` Matteo Croce @ 2019-06-12 17:32 ` Matteo Croce 2019-06-12 18:19 ` Stephen Hemminger 0 siblings, 1 reply; 8+ messages in thread From: Matteo Croce @ 2019-06-12 17:32 UTC (permalink / raw) To: Stephen Hemminger; +Cc: netdev, David Ahern On Wed, Jun 12, 2019 at 6:04 PM Matteo Croce <mcroce@redhat.com> wrote: > > On Wed, Jun 12, 2019 at 5:55 PM Stephen Hemminger > <stephen@networkplumber.org> wrote: > > > > On Tue, 11 Jun 2019 20:03:26 +0200 > > Matteo Croce <mcroce@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 <mcroce@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? -- Matteo Croce per aspera ad upstream ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH iproute2] testsuite: don't clobber /tmp 2019-06-12 17:32 ` Matteo Croce @ 2019-06-12 18:19 ` Stephen Hemminger 2019-06-13 17:15 ` Matteo Croce 0 siblings, 1 reply; 8+ messages in thread From: Stephen Hemminger @ 2019-06-12 18:19 UTC (permalink / raw) To: Matteo Croce; +Cc: netdev, David Ahern On Wed, 12 Jun 2019 19:32:29 +0200 Matteo Croce <mcroce@redhat.com> wrote: > On Wed, Jun 12, 2019 at 6:04 PM Matteo Croce <mcroce@redhat.com> wrote: > > > > On Wed, Jun 12, 2019 at 5:55 PM Stephen Hemminger > > <stephen@networkplumber.org> wrote: > > > > > > On Tue, 11 Jun 2019 20:03:26 +0200 > > > Matteo Croce <mcroce@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 <mcroce@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? ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH iproute2] testsuite: don't clobber /tmp 2019-06-12 18:19 ` Stephen Hemminger @ 2019-06-13 17:15 ` Matteo Croce 2019-06-25 14:39 ` Matteo Croce 0 siblings, 1 reply; 8+ messages in thread From: Matteo Croce @ 2019-06-13 17:15 UTC (permalink / raw) To: Stephen Hemminger; +Cc: netdev, David Ahern On Wed, Jun 12, 2019 at 8:20 PM Stephen Hemminger <stephen@networkplumber.org> wrote: > > On Wed, 12 Jun 2019 19:32:29 +0200 > Matteo Croce <mcroce@redhat.com> wrote: > > > On Wed, Jun 12, 2019 at 6:04 PM Matteo Croce <mcroce@redhat.com> wrote: > > > > > > On Wed, Jun 12, 2019 at 5:55 PM Stephen Hemminger > > > <stephen@networkplumber.org> wrote: > > > > > > > > On Tue, 11 Jun 2019 20:03:26 +0200 > > > > Matteo Croce <mcroce@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 <mcroce@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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH iproute2] testsuite: don't clobber /tmp 2019-06-13 17:15 ` Matteo Croce @ 2019-06-25 14:39 ` Matteo Croce 2019-10-12 13:32 ` Matteo Croce 0 siblings, 1 reply; 8+ messages in thread From: Matteo Croce @ 2019-06-25 14:39 UTC (permalink / raw) To: Stephen Hemminger; +Cc: netdev, David Ahern On Thu, Jun 13, 2019 at 7:15 PM Matteo Croce <mcroce@redhat.com> wrote: > > On Wed, Jun 12, 2019 at 8:20 PM Stephen Hemminger > <stephen@networkplumber.org> wrote: > > > > On Wed, 12 Jun 2019 19:32:29 +0200 > > Matteo Croce <mcroce@redhat.com> wrote: > > > > > On Wed, Jun 12, 2019 at 6:04 PM Matteo Croce <mcroce@redhat.com> wrote: > > > > > > > > On Wed, Jun 12, 2019 at 5:55 PM Stephen Hemminger > > > > <stephen@networkplumber.org> wrote: > > > > > > > > > > On Tue, 11 Jun 2019 20:03:26 +0200 > > > > > Matteo Croce <mcroce@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 <mcroce@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 Hi, any more thoughts about this patch? -- Matteo Croce per aspera ad upstream ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH iproute2] testsuite: don't clobber /tmp 2019-06-25 14:39 ` Matteo Croce @ 2019-10-12 13:32 ` Matteo Croce 0 siblings, 0 replies; 8+ messages in thread From: Matteo Croce @ 2019-10-12 13:32 UTC (permalink / raw) To: Stephen Hemminger; +Cc: netdev, David Ahern On Tue, Jun 25, 2019 at 4:39 PM Matteo Croce <mcroce@redhat.com> wrote: > > On Thu, Jun 13, 2019 at 7:15 PM Matteo Croce <mcroce@redhat.com> wrote: > > > > On Wed, Jun 12, 2019 at 8:20 PM Stephen Hemminger > > <stephen@networkplumber.org> wrote: > > > > 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 > > Hi, > > any more thoughts about this patch? > > -- > Matteo Croce > per aspera ad upstream Hi, almost forgot about this one. Should I resend it, or it was nacked? Regards, -- Matteo Croce per aspera ad upstream ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-10-12 13:33 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-06-11 18:03 [PATCH iproute2] testsuite: don't clobber /tmp Matteo Croce 2019-06-12 15:53 ` Stephen Hemminger 2019-06-12 16:04 ` Matteo Croce 2019-06-12 17:32 ` Matteo Croce 2019-06-12 18:19 ` Stephen Hemminger 2019-06-13 17:15 ` Matteo Croce 2019-06-25 14:39 ` Matteo Croce 2019-10-12 13:32 ` Matteo Croce
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).