linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Guenter Roeck <linux@roeck-us.net>
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
Subject: Re: [PATCH] Revert "scripts/setlocalversion: git: Make -dirty check more robust"
Date: Tue, 6 Nov 2018 19:10:19 -0800	[thread overview]
Message-ID: <43a04c4e-fa13-c162-5970-fe1aaa1ac557@roeck-us.net> (raw)
In-Reply-To: <alpine.DEB.2.21.999.1811061845161.5308@trent.utfs.org>

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

  reply	other threads:[~2018-11-07  3:10 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-06 18:10 [PATCH] Revert "scripts/setlocalversion: git: Make -dirty check more robust" Guenter Roeck
2018-11-06 19:23 ` Genki Sky
2018-11-07  2:22   ` Brian Norris
2018-11-07  2:58     ` Christian Kujau
2018-11-07  3:10       ` Guenter Roeck [this message]
2018-11-07  4:00       ` Brian Norris
2018-11-07 18:44         ` Brian Norris
2018-11-07 20:43           ` Genki Sky
2018-11-07 20:55             ` Guenter Roeck
2018-11-07 21:07               ` Genki Sky
2018-11-07 21:18                 ` Doug Anderson
2018-11-07 21:26                   ` Genki Sky
2018-11-08  3:16                   ` Brian Norris
2018-11-09  2:55                     ` Masahiro Yamada
2018-11-09 18:34                       ` [PATCH] scripts/setlocalversion: Improve -dirty check with git-status --no-optional-locks Brian Norris
2018-11-09 20:43                         ` Guenter Roeck
2018-11-10  8:58                         ` Genki Sky
2018-11-10 10:42                           ` Andreas Schwab
2018-11-10 20:10                             ` Genki Sky
2018-11-11 14:48                               ` Alexander Kapshuk
2018-11-11 15:03                                 ` Alexander Kapshuk
2018-11-11 17:41                                 ` Genki Sky
2018-11-11 19:59                                   ` Alexander Kapshuk
2018-11-12  8:42                                     ` Alexander Kapshuk
2018-11-13  0:09                                       ` Brian Norris
2018-11-13  8:35                                         ` Alexander Kapshuk
2018-11-13 18:32                                           ` Brian Norris
2018-11-13 19:03                                             ` [PATCH v2] " Brian Norris
2018-11-13 19:47                                               ` Genki Sky
2018-11-13 22:03                                               ` Andreas Schwab
2018-11-15  2:06                                                 ` Brian Norris
2018-11-15  2:11                                                   ` [PATCH v3] " Brian Norris
2018-11-16 15:17                                                     ` Masahiro Yamada
2018-11-19 14:42                                                     ` Masahiro Yamada
2018-11-13 19:15                                             ` [PATCH] " Alexander Kapshuk
2018-11-09  2:55               ` [PATCH] Revert "scripts/setlocalversion: git: Make -dirty check more robust" Masahiro Yamada
2018-11-09 16:39       ` Masahiro Yamada
2018-11-09 16:39 ` 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=43a04c4e-fa13-c162-5970-fe1aaa1ac557@roeck-us.net \
    --to=linux@roeck-us.net \
    --cc=briannorris@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lists@nerdbynature.de \
    --cc=sky@genki.is \
    --cc=yamada.masahiro@socionext.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;
as well as URLs for NNTP newsgroup(s).