Re: [PATCH] Revert "scripts/setlocalversion: git: Make -dirty check more robust"

From: Guenter Roeck
Date: Tue Nov 06 2018 - 22:10:48 EST


On 11/6/18 6:58 PM, Christian Kujau wrote:
On Tue, 6 Nov 2018, Brian Norris wrote:
Perhaps both scenarios could be satisfied by having
scripts/setlocalversion first check if .git has write permissions, and
acting accordingly. Looking into history, this actually used to be
done, but cdf2bc632ebc ("scripts/setlocalversion on write-protected
source tree", 2013-06-14) removed the updating of the index.

A "writeable" check (e.g., [ -w . ]) would be sufficient for our case.
But I'm not so sure about that older NFS report, and I'm also not sure
that we should be writing to the source tree at all in this case. Maybe
we can also check whether there's a build output directory specified?

FWIW, the issue I reported back in 2013[0] was not an ill-configured NFS
export, but a read-only NFS export (and then a read-write exported NFS
export, but the user compiling the kernel did not have write permission)
and so "test -w .git" did not help in determining if the source tree can
actually written to. And depending on the user's shell[1], this may or may
not still be the case.

So I'm all for the $(touch .git/some-file-here) test to decide if the
kernel has to be modified during build.

Christian.

[0] https://lkml.org/lkml/2013/6/14/574
[1] https://manpages.debian.org/unstable/dash/dash.1.en.html

However, I admit I don't understand the justification in that commit
from 2013. I'm no NFS expert, but perhaps the real problem there is an
incorrectly configured NFS setup (uid/gid mismatch between NFS
client/server, or permissions mismatch between mount options and NFS
server?). Christian Kujau: can you speak to that?

Well, we could also make our check $(touch .git/some-file-here
2>/dev/null && ...) instead of $(test -w .git) to handle misconfigured
NFS setups. But not sure if that has its own problems.

Trying to 'touch' the source tree will also break us. No matter whether
you redirect stderr, our sandbox will still notice the build is doing
something fishy and complain.

For whatever reason, I did not get any of the interim e-mails, only this one.
My apologies to not responding earlier. But then Brian has said it all:
Any write attempt onto the read-only file system will cause our build
system to bail out.

Also, again: I think it is reasonable to expect that the source tree
is not touched if an output directory is specified. Any write attempt
violates this assumption. This most definitely includes any "touch"
commands on the source tree.

Thanks,
Guenter