Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH] Revert "scripts/setlocalversion: git: Make -dirty check
 more robust"
To:     Christian Kujau <lists@nerdbynature.de>,
        Brian Norris <briannorris@chromium.org>
Cc:     Genki Sky <sky@genki.is>,
        Masahiro Yamada <yamada.masahiro@socionext.com>,
        linux-kernel@vger.kernel.org
References: <1541527838-4585-1-git-send-email-linux@roeck-us.net>
 <20181106.192305.406697677@genki.is> <20181107022156.GA254567@google.com>
 <alpine.DEB.2.21.999.1811061845161.5308@trent.utfs.org>
From:   Guenter Roeck <linux@roeck-us.net>
Message-ID: <43a04c4e-fa13-c162-5970-fe1aaa1ac557@roeck-us.net>
Date:   Tue, 6 Nov 2018 19:10:19 -0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.2.1
MIME-Version: 1.0
In-Reply-To: <alpine.DEB.2.21.999.1811061845161.5308@trent.utfs.org>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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