| Message ID | 20180313061109.72629-3-dianders@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
* Douglas Anderson <dianders@chromium.org> wrote: > +# Don't create Makefile caches if running as root since they can't be deleted > +# easily; in the real world we might be root when doing "sudo make install" > +ifeq ($(shell id -u),0) > +export KBUILD_NOCACHE := 1 > +endif Please don't do this - many prominent kernel developers build their kernels as root - this makes the build slower for them, and also bifurcates testing. Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Mon, Mar 12, 2018 at 11:16 PM, Ingo Molnar <mingo@kernel.org> wrote: > > * Douglas Anderson <dianders@chromium.org> wrote: > >> +# Don't create Makefile caches if running as root since they can't be deleted >> +# easily; in the real world we might be root when doing "sudo make install" >> +ifeq ($(shell id -u),0) >> +export KBUILD_NOCACHE := 1 >> +endif > > Please don't do this - many prominent kernel developers build their kernels as > root - this makes the build slower for them, and also bifurcates testing. Ah, interesting. I hadn't realized that! I'm OK with dropping this patch myself. It was mostly addressing a potential problem pointed out by Masahiro Yamada when we were talking about .cache.mk, but I don't think anyone has actually experienced the problems listed in the CL description. I suppose the other option would be to go back to keying off the "install" target (like v2 of this patch did), but it kinda feels like if someone did "sudo make install" and then followed up by a non-sudo "make" they probably would be able to figure out how to fix it without much trouble (just do a "sudo make clean"). In case it's not obvious, though, we should still try to land patch #1 of this series. That's a real problems that people reported when the .cache.mk stuff first landed and then folks tried to upgrade their gcc. -Doug -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Mar 12, 2018 at 11:23 PM Doug Anderson <dianders@chromium.org> wrote: > Hi, > On Mon, Mar 12, 2018 at 11:16 PM, Ingo Molnar <mingo@kernel.org> wrote: > > > > * Douglas Anderson <dianders@chromium.org> wrote: > > > >> +# Don't create Makefile caches if running as root since they can't be deleted > >> +# easily; in the real world we might be root when doing "sudo make install" > >> +ifeq ($(shell id -u),0) > >> +export KBUILD_NOCACHE := 1 > >> +endif > > > > Please don't do this - many prominent kernel developers build their kernels as > > root - this makes the build slower for them, and also bifurcates testing. > Ah, interesting. I hadn't realized that! > I'm OK with dropping this patch myself. It was mostly addressing a > potential problem pointed out by Masahiro Yamada when we were talking > about .cache.mk, but I don't think anyone has actually experienced the > problems listed in the CL description. > /bin/sh: ./.cache.mk: Permission denied I feel like I've definitely seen that permission error before. Is there any issue if I: $ make clean $ make $ sudo make install <hack around> $ make Or it's only a problem if: $ make clean $ sudo make install <hack around> $ make -- Thanks, ~Nick Desaulniers -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Tue, Mar 13, 2018 at 9:33 AM, Nick Desaulniers <ndesaulniers@google.com> wrote: > On Mon, Mar 12, 2018 at 11:23 PM Doug Anderson <dianders@chromium.org> > wrote: > >> Hi, > >> On Mon, Mar 12, 2018 at 11:16 PM, Ingo Molnar <mingo@kernel.org> wrote: >> > >> > * Douglas Anderson <dianders@chromium.org> wrote: >> > >> >> +# Don't create Makefile caches if running as root since they can't be > deleted >> >> +# easily; in the real world we might be root when doing "sudo make > install" >> >> +ifeq ($(shell id -u),0) >> >> +export KBUILD_NOCACHE := 1 >> >> +endif >> > >> > Please don't do this - many prominent kernel developers build their > kernels as >> > root - this makes the build slower for them, and also bifurcates > testing. > >> Ah, interesting. I hadn't realized that! > >> I'm OK with dropping this patch myself. It was mostly addressing a >> potential problem pointed out by Masahiro Yamada when we were talking >> about .cache.mk, but I don't think anyone has actually experienced the >> problems listed in the CL description. > >> /bin/sh: ./.cache.mk: Permission denied > > I feel like I've definitely seen that permission error before. Hopefully the error message was obvious enough that it didn't take you too long to think to type "sudo make clean"? I know it's better for people not to have to figure this out, but my hope is at least it's not too arcane of an errror message. > Is there any issue if I: > > $ make clean > $ make > $ sudo make install > <hack around> > $ make I don't personally know of any problems with the above flow. > Or it's only a problem if: > > $ make clean > $ sudo make install > <hack around> > $ make Yeah, just that one. NOTE: as Masahiro noted in response to the cover letter [1], he's come up with a better solution to the problem that was solved by the .cache.mk and he's planning to remove it shortly. [1] https://lkml.org/lkml/2018/3/13/100 -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Mar 13, 2018 at 9:44 AM, Doug Anderson <dianders@chromium.org> wrote: >> Is there any issue if I: >> >> $ make clean >> $ make >> $ sudo make install >> <hack around> >> $ make > > I don't personally know of any problems with the above flow. This is my workflow, and what I suggest people do - only do "make install" as root, everything else as a normal user. And in fact, it occasionally _has_ broken, when "make install" has done more than just install things, and created root-owned files and directories (particularly the generated headers). And then I complain to people, because then things like "make clean" as a normal user ends up breaking too when there's some root-owned file. So it can break, but it's pretty rare. Usually it's something like "people didn't use the proper sequence to update a file only if it changed, and just blindly over-wrote a new version. Generally, I prefer "make install" not even checking dependencies at all, and just blindly copy things. And I'd definitely be ok with an error rather than root generating files, although then I'd not special-case mkcache, but all the *other* random file changes we do. If Ingo wants to build as root, maybe we could even make him set some environment flag to avoid errors. (But I don't actually like the patch in question, because I really think KBUILD_NOCACHE is much too specific a special case, and it should be way more generic). Linus -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Tue, Mar 13, 2018 at 10:39 AM, Linus Torvalds <torvalds@linux-foundation.org> wrote: > On Tue, Mar 13, 2018 at 9:44 AM, Doug Anderson <dianders@chromium.org> wrote: >>> Is there any issue if I: >>> >>> $ make clean >>> $ make >>> $ sudo make install >>> <hack around> >>> $ make >> >> I don't personally know of any problems with the above flow. > > This is my workflow, and what I suggest people do - only do "make > install" as root, everything else as a normal user. > > And in fact, it occasionally _has_ broken, when "make install" has > done more than just install things, and created root-owned files and > directories (particularly the generated headers). > > And then I complain to people, because then things like "make clean" > as a normal user ends up breaking too when there's some root-owned > file. > > So it can break, but it's pretty rare. Usually it's something like > "people didn't use the proper sequence to update a file only if it > changed, and just blindly over-wrote a new version. > > Generally, I prefer "make install" not even checking dependencies at > all, and just blindly copy things. And I'd definitely be ok with an > error rather than root generating files, although then I'd not > special-case mkcache, but all the *other* random file changes we do. > > If Ingo wants to build as root, maybe we could even make him set some > environment flag to avoid errors. > > (But I don't actually like the patch in question, because I really > think KBUILD_NOCACHE is much too specific a special case, and it > should be way more generic). Please consider the patch abandoned. As per other threads pointed to in this conversation, Masahiro Yamada is planning to fully gut the ".cache.mk" from the build system anyway. :-) -Doug -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Linus Torvalds <torvalds@linux-foundation.org> wrote: > If Ingo wants to build as root, maybe we could even make him set some > environment flag to avoid errors. I only build as root infrequently, but I think PeterZ does it more frequently? Distro package builds are also often done as root. I don't mind warnings, etc. - I only objected to the workaround of silently turning off a build optimization when root. Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Mar 14, 2018 at 08:23:16AM +0100, Ingo Molnar wrote: > > * Linus Torvalds <torvalds@linux-foundation.org> wrote: > > > If Ingo wants to build as root, maybe we could even make him set some > > environment flag to avoid errors. > > I only build as root infrequently, but I think PeterZ does it more frequently? Yeah, a bunch of my testboxes don't even have a regular user account. -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Makefile b/Makefile index f1e61470640b..c4d2131831ba 100644 --- a/Makefile +++ b/Makefile @@ -268,6 +268,12 @@ __build_one_by_one: else +# Don't create Makefile caches if running as root since they can't be deleted +# easily; in the real world we might be root when doing "sudo make install" +ifeq ($(shell id -u),0) +export KBUILD_NOCACHE := 1 +endif + # We need some generic definitions (do not try to remake the file). scripts/Kbuild.include: ; include scripts/Kbuild.include diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include index 065324a8046f..581f93f20119 100644 --- a/scripts/Kbuild.include +++ b/scripts/Kbuild.include @@ -137,12 +137,14 @@ __sanitize-opt = $(subst $$,_,$(subst $(right_paren),_,$(subst $(left_paren),_,$ define __run-and-store ifeq ($(origin $(1)),undefined) $$(eval $(1) := $$(shell $$(2))) +ifneq ($(KBUILD_NOCACHE),1) 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))
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 we're running as root. 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 v3: - Use "uid 0" as the heuristic instead of install - Do the checking in the main Makefile instead of Kbuild.include Changes in v2: None Makefile | 6 ++++++ scripts/Kbuild.include | 2 ++ 2 files changed, 8 insertions(+)