mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Douglas Anderson <dianders@chromium.org>
To: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: malat@debian.org, dave.hansen@intel.com, yang.s@alibaba-inc.com,
	linux@roeck-us.net, Douglas Anderson <dianders@chromium.org>,
	Matthias Kaehlcke <mka@chromium.org>,
	Cao jin <caoj.fnst@cn.fujitsu.com>, Arnd Bergmann <arnd@arndb.de>,
	Mark Charlebois <charlebm@gmail.com>,
	linux-kernel@vger.kernel.org, Ingo Molnar <mingo@kernel.org>,
	Nick Desaulniers <ndesaulniers@google.com>
Subject: [PATCH v2 2/2] kbuild: Don't mess with the .cache.mk when installing
Date: Fri, 22 Dec 2017 14:14:54 -0800	[thread overview]
Message-ID: <20171222221454.137767-3-dianders@chromium.org> (raw)
In-Reply-To: <20171222221454.137767-1-dianders@chromium.org>

As pointed out by Masahiro Yamada people often run "sudo make install"
or "sudo make modules_install".  In theory, that could cause a cache
file (or the directory containing it) to be created by root.  After
doing this then subsequent invocations to make would yell with a whole
bunch of warnings like:

  /bin/sh: ./.cache.mk: Permission denied

These yells would be mostly harmless (we'd just go on running without
the cache), but they're ugly.

The above situation would be fairly unlikely because it should only
happen if someone does "sudo make install" before doing a normal
"make", which is an invalid thing to do.  If you did a normal "make"
before the "sudo make install" then all the cache files and
directories should have already been created by a normal user and,
even if the superuser needed to add to the existing files, we wouldn't
end up with a permission problem.

The above situation would also not have been catastrophic because
presumably if the user was able to run "sudo make install" then they
should also be able to run "sudo make clean" to clean things up.

However, even though the problem described is relatively minor, it
probably makes sense to add a heuristic to avoid creating cache files
when one of the make goals looks like an install.  This heuristic
doesn't add a ton of overhead and it might save someone time tracking
down strange "Permission denied" messages.  We'll consider this
heuristic good enough because the problem really shouldn't be that
serious.

Fixes: 3298b690b21c ("kbuild: Add a cache for generated variables")
Suggested-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

Changes in v2: None

 scripts/Kbuild.include | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index f7efb59d85d1..314585b3efe4 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -124,6 +124,12 @@ __sanitize-opt = $(subst $$,_,$(subst $(right_paren),_,$(subst $(left_paren),_,$
 # previous invocation of make!) we'll return the value.  If not, we'll
 # compute it and store the result for future runs.
 #
+# NOTE: we won't ever add to the cache if one of the make goals looks like
+# it is installing.  Technically there's no reason we can't cache things
+# related to install but it's likely that 'make install' is called as root.
+# If we create the cache file as root we might have a hard time adding to it
+# (or deleting it in make clean).
+#
 # This is a bit of voodoo, but basic explanation is that if the variable
 # was undefined then we'll evaluate the shell command and store the result
 # into the variable.  We'll then store that value in the cache and finally
@@ -137,12 +143,14 @@ __sanitize-opt = $(subst $$,_,$(subst $(right_paren),_,$(subst $(left_paren),_,$
 define __run-and-store
 ifeq ($(origin $(1)),undefined)
   $$(eval $(1) := $$(shell $$(2)))
+ifeq ($(filter %install,$(MAKECMDGOALS)),)
 ifeq ($(create-cache-dir),1)
   $$(shell mkdir -p $(dir $(make-cache)))
   $$(eval create-cache-dir :=)
 endif
   $$(shell echo '$(1) := $$($(1))' >> $(make-cache))
 endif
+endif
 endef
 __shell-cached = $(eval $(call __run-and-store,$(1)))$($(1))
 shell-cached = $(call __shell-cached,__cached_$(call __sanitize-opt,$(1)),$(1))
-- 
2.15.1.620.gb9897f4670-goog

  parent reply	other threads:[~2017-12-22 22:15 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-22 22:14 [PATCH v2 0/2] kbuild: Fix corner caches with .cache.mk Douglas Anderson
2017-12-22 22:14 ` [PATCH v2 1/2] kbuild: Require a 'make clean' if we detect gcc changed underneath us Douglas Anderson
2017-12-31  7:46   ` Masahiro Yamada
2017-12-22 22:14 ` Douglas Anderson [this message]
2017-12-31  8:26   ` [PATCH v2 2/2] kbuild: Don't mess with the .cache.mk when installing Masahiro Yamada

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171222221454.137767-3-dianders@chromium.org \
    --to=dianders@chromium.org \
    --cc=arnd@arndb.de \
    --cc=caoj.fnst@cn.fujitsu.com \
    --cc=charlebm@gmail.com \
    --cc=dave.hansen@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=malat@debian.org \
    --cc=mingo@kernel.org \
    --cc=mka@chromium.org \
    --cc=ndesaulniers@google.com \
    --cc=yamada.masahiro@socionext.com \
    --cc=yang.s@alibaba-inc.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®