git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Daniel Barkalow <barkalow@iabervon.org>
Cc: Avery Pennarun <apenwarr@gmail.com>,
	Mark Levedahl <mlevedahl@gmail.com>,
	Git Mailing List <git@vger.kernel.org>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>
Subject: Re: Commit cce8d6fdb introduces file t/t5100/nul, git tree is now incompatible with Cygwin (and probably Windows)
Date: Wed, 28 May 2008 13:43:06 -0700	[thread overview]
Message-ID: <7v7idetb1h.fsf@gitster.siamese.dyndns.org> (raw)
In-Reply-To: <alpine.LNX.1.00.0805281455100.19665@iabervon.org> (Daniel Barkalow's message of "Wed, 28 May 2008 16:06:40 -0400 (EDT)")

Daniel Barkalow <barkalow@iabervon.org> writes:

> Ah, yes, CE_VALID. But it doesn't quite work as well as I'd like, because 

No, I do not think we should involve CE_VALID here.  It means something
completely different.  What I meant was that through "git status" the user
can tell there is an unexpected breakage in the work tree, _if_ we make
checkout to finish with "best effort" (and still report an error).

> I think the right test for this is if create_file() returns EEXIST, but 
> readdir doesn't show anything.

I think relying on EEXIST is too specific for this particular breakage,
even though such a test may catch it.  A checkout may fail in the middle
if a filesystem refuses to create a pathname that has certain characters
in it (e.g. dosen't NTFS refuse a path with :|<"?*> in it, or is it just
the Explorer UI layer rejecting them?), or perhaps one leading directory
may be unwritable.  We would want to catch and cope with such a brokage
the same way.

The checkout "unpack-trees" codepath does:

 - Make sure things can be checked out safely with the internal data
   before doing anything to the filesystem, i.e. no lost local changes, no
   lost untracked files, etc.

 - For each path:

   - make room for it, removing directory at the place as necessary where
     a blob must sit and removing an existing blob as needed;

   - create a new file or symlink;

And currently I think we stop on any failure.  The thing is, stopping on a
failure during the internal checking is fine --- no external damage has
been made yet.  But once we started updating the work tree, we _are_
committed and not aborting in the middle for a single failure would be the
saner thing to do.  In addition, even after such a failure after we are
committed, we probably should update the HEAD and the index.

"status" would then show the difference between what should have been
checked out and what is.  It might be enough to improve the issue of "git
bisect hitting a checkout failure --- the work tree is half checked-out
state, and the index, the HEAD, and the work tree are in a very
inconsistent state".

We would probably signal such an error from git-checkout differently from
an early refusal that does not do anything, to tell the callers, such as
"git-bisect", that the checkout _has been_ already done, even though there
may be breakages in the work tree.

> ... that notes the situation where you seem to have file A instead of 
> file B, but fstat("B") returns A's inode, and marks the index to say that 
> entry B is listed in the filesystem as A instead.

I personally do not think such auto-substution is a way to go --- what
makes you trust inode information from such an untrustworthy filesystem
that does not do what it was told to do?  I suspect that stopping at the
error site and not automatically making the damage yet larger by doing
such magic would keep the recovery procedure simpler.

But I wouldn't keep people from experimenting.  Perhaps the end result
could be even readable and mergeable, although I am quite pessimistic.

  reply	other threads:[~2008-05-28 20:44 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-05-26 14:01 Commit cce8d6fdb introduces file t/t5100/nul, git tree is now incompatible with Cygwin (and probably Windows) Mark Levedahl
2008-05-26 14:25 ` Johannes Schindelin
2008-05-26 17:37   ` Mark Levedahl
2008-05-26 21:28     ` Johannes Schindelin
2008-05-26 22:49       ` Mark Levedahl
2008-05-26 23:10         ` Johannes Schindelin
2008-05-26 23:15         ` Johannes Schindelin
     [not found]   ` <483ADA17.3080401@viscovery.net>
2008-05-26 21:21     ` [PATCH] Makefile: wt-status.h is also a lib header Johannes Schindelin
2008-05-26 21:54       ` Junio C Hamano
2008-05-26 23:03         ` Johannes Schindelin
2008-05-27 13:26   ` Commit cce8d6fdb introduces file t/t5100/nul, git tree is now incompatible with Cygwin (and probably Windows) Eric Blake
2008-05-28  6:12 ` Junio C Hamano
2008-05-28  9:46   ` Wincent Colaiuta
2008-05-28 15:53     ` Lea Wiemann
2008-05-28 15:58       ` Wincent Colaiuta
2008-05-28 21:39         ` Jakub Narebski
2008-05-29 13:22     ` Johannes Schindelin
2008-05-29 14:58       ` Wincent Colaiuta
2008-05-29 16:05         ` Johannes Schindelin
2008-05-29 16:15           ` Wincent Colaiuta
2008-05-31 17:37         ` Steffen Prohaska
2008-05-31 18:28           ` [PATCH] gitweb: Remove gitweb/test/ directory Jakub Narebski
2008-05-31 18:49             ` Wincent Colaiuta
2008-05-31 23:19               ` Johannes Schindelin
2008-06-01  0:19                 ` Jakub Narebski
2008-06-01  9:42                   ` Kay Sievers
2008-06-01 19:07                 ` Wincent Colaiuta
2008-06-01  1:06             ` Junio C Hamano
2008-06-01  1:59               ` Jakub Narebski
2008-05-28 16:33   ` Commit cce8d6fdb introduces file t/t5100/nul, git tree is now incompatible with Cygwin (and probably Windows) Avery Pennarun
2008-05-28 17:24     ` Junio C Hamano
2008-05-28 17:46       ` Sverre Rabbelier
2008-05-28 17:52       ` Avery Pennarun
2008-05-28 18:27         ` Junio C Hamano
2008-05-28 18:19       ` Daniel Barkalow
2008-05-28 18:37         ` Junio C Hamano
2008-05-28 20:06           ` Daniel Barkalow
2008-05-28 20:43             ` Junio C Hamano [this message]
2008-05-28 21:19               ` [PATCH] "git checkout -- paths..." should signal error Junio C Hamano
2008-05-29  6:28                 ` Marius Storm-Olsen
2008-05-29 13:05                 ` Daniel Barkalow
2008-05-28 21:41               ` Commit cce8d6fdb introduces file t/t5100/nul, git tree is now incompatible with Cygwin (and probably Windows) Daniel Barkalow

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=7v7idetb1h.fsf@gitster.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=apenwarr@gmail.com \
    --cc=barkalow@iabervon.org \
    --cc=git@vger.kernel.org \
    --cc=mlevedahl@gmail.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.
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).