git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Orgad Shaneh <orgads@gmail.com>
To: Edward Thomson <ethomson@edwardthomson.com>
Cc: 八神和麻 <drizzd@gmx.net>, git <git@vger.kernel.org>,
	"Nikolai Kosjar" <nikolai.kosjar@qt.io>,
	ivan.donchevskii@qt.io
Subject: Re: [PATCH] checkout files in-place
Date: Wed, 13 Jun 2018 10:39:17 +0300	[thread overview]
Message-ID: <CAGHpTBJ9WiWdJw=SgxJpWqP9CucANatafx6iwCRCRY15wTBsVg@mail.gmail.com> (raw)
In-Reply-To: <20180612085119.GA5@aaaa10152750>

On Tue, Jun 12, 2018 at 11:51 AM Edward Thomson
<ethomson@edwardthomson.com> wrote:
>
> On Tue, Jun 12, 2018 at 09:13:54AM +0300, Orgad Shaneh wrote:
> > Some of my colleagues use an ancient version of Source Insight, which also
> > locks files for write.
>
> If that application is locking files for writing (that is to say, it did
> not specify the `FILE_SHARE_WRITE` bit in the sharing modes during
> `CreateFile`) then this patch would not help.
>
> Applications, generally speaking, should be locking files for write.
> It's the default in Win32 and .NET's file open APIs because few
> applications are prepared to detect and support a file changing out from
> underneath them in the middle of a read.

I agree.

> > It's less important than it was before those fixes, but it is still needed
> > for users of Qt Creator 4.6 (previous versions just avoided mmap, 4.7 uses
> > mmap only for system headers). Other tools on Windows might as well
> > misbehave.
>
> I don't understand what mmap'ing via `CreateFileMapping` has to do with
> this.  It takes an existing `HANDLE` that was opened with `CreateFile`,
> which is where the sharing mode was supplied.

I'm not completely sure. The file is opened using CreateFile[1] with
FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE.
Then this handle is passed to CreateFileMapping[2]. For a reason I don't
understand, when mapping is used, the handle is never released (until
the file is closed), but when it is not used, the file is being read, then the
handle is released.

Maybe Ivan or Nikolai can shed some light on this process.

Anyway, with Qt Creator 4.7 this should be a non-issue, so I'm reluctant about
this change here.

> I would be surprised if there are other tools on Windows that have
> specified `FILE_SHARE_WRITE` but not `FILE_SHARE_DELETE`.  Generally
> speaking, if you don't care about another process changing a file
> underneath you then you should specify both.  If you do then you should
> specify neither.

The problem is that even if you specify FILE_SHARE_WRITE and FILE_SHARE_DELETE,
the file can be unlinked, but it cannot be created with the same name
until its handle
is closed, unless you rename it *before* unlinking.

- Orgad

[1] https://github.com/llvm-mirror/llvm/blob/371257e/lib/Support/Windows/Path.inc#L1045
[2] https://github.com/llvm-mirror/llvm/blob/371257e/lib/Support/Windows/Path.inc#L836

  reply	other threads:[~2018-06-13  7:39 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-10 19:44 [PATCH] checkout files in-place Clemens Buchacher
2018-06-11  2:04 ` brian m. carlson
2018-06-11 17:48   ` Clemens Buchacher
2018-06-11 18:02   ` Junio C Hamano
2018-06-11 20:22     ` Clemens Buchacher
2018-06-11 17:59 ` Ævar Arnfjörð Bjarmason
2018-06-11 20:35 ` Edward Thomson
2018-06-11 20:57   ` Clemens Buchacher
     [not found]     ` <CAGHpTBJFwToEwnk4P17AJ+z-55Nzc04OBbTvsbFRrkXJpfXAkQ@mail.gmail.com>
2018-06-12  8:51       ` Edward Thomson
2018-06-13  7:39         ` Orgad Shaneh [this message]
2018-06-11 20:39 ` [PATCH v2] " Clemens Buchacher
2018-06-11 21:38   ` Ævar Arnfjörð Bjarmason
2018-06-11 22:46     ` Junio C Hamano

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to='CAGHpTBJ9WiWdJw=SgxJpWqP9CucANatafx6iwCRCRY15wTBsVg@mail.gmail.com' \
    --to=orgads@gmail.com \
    --cc=drizzd@gmx.net \
    --cc=ethomson@edwardthomson.com \
    --cc=git@vger.kernel.org \
    --cc=ivan.donchevskii@qt.io \
    --cc=nikolai.kosjar@qt.io \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).