* Commit cce8d6fdb introduces file t/t5100/nul, git tree is now incompatible with Cygwin (and probably Windows)
@ 2008-05-26 14:01 Mark Levedahl
2008-05-26 14:25 ` Johannes Schindelin
2008-05-28 6:12 ` Junio C Hamano
0 siblings, 2 replies; 42+ messages in thread
From: Mark Levedahl @ 2008-05-26 14:01 UTC (permalink / raw)
To: Git Mailing List, Johannes Schindelin, Junio C Hamano
Beginning with the referenced commit, the git project cannot be checked
out on Cygwin (and I assume cannot be checked out on Windows using
msysgit, though I have not verified this) as this commit introduces the
file "t/5100/nul." On Windows, the file name "nul" is reserved,
regardless of path, and cannot be created or deleted. It serves
essentially the same function as /dev/null.
As a for instance of the troubles:
git>git checkout -f origin/master
Previous HEAD position was a2f5be5... Merge branch
'jk/maint-send-email-compose' into maint
error: git-checkout-index: unable to create file t/t5100/nul (File exists)
As this commit is part of the published master branch, I am not sure the
correct resolution: leaving this commit in place means that any commit
between it and a commit fixing this will always cause an error on Cygwin
/ Windows. Of course, it *is* on the published master branch.
Mark
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: Commit cce8d6fdb introduces file t/t5100/nul, git tree is now incompatible with Cygwin (and probably Windows)
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
` (2 more replies)
2008-05-28 6:12 ` Junio C Hamano
1 sibling, 3 replies; 42+ messages in thread
From: Johannes Schindelin @ 2008-05-26 14:25 UTC (permalink / raw)
To: Mark Levedahl; +Cc: Git Mailing List, Junio C Hamano
Hi,
On Mon, 26 May 2008, Mark Levedahl wrote:
> Beginning with the referenced commit, the git project cannot be checked
> out on Cygwin (and I assume cannot be checked out on Windows using
> msysgit, though I have not verified this) as this commit introduces the
> file "t/5100/nul." On Windows, the file name "nul" is reserved,
> regardless of path, and cannot be created or deleted. It serves
> essentially the same function as /dev/null.
Even when referencing the full (or a relative) path? That's bad!
> As a for instance of the troubles:
>
> git>git checkout -f origin/master
> Previous HEAD position was a2f5be5... Merge branch
> 'jk/maint-send-email-compose' into maint
> error: git-checkout-index: unable to create file t/t5100/nul (File exists)
>
> As this commit is part of the published master branch, I am not sure the
> correct resolution: leaving this commit in place means that any commit
> between it and a commit fixing this will always cause an error on Cygwin
> / Windows. Of course, it *is* on the published master branch.
That's the case for all regressions: we do not rewrite history for them.
As for the resolution, could you quickly try the 'my-next' branch of
git://repo.or.cz/git/dscho.git?
If that works, I'll send a proper patch to Junio.
Ciao,
Dscho
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: Commit cce8d6fdb introduces file t/t5100/nul, git tree is now incompatible with Cygwin (and probably Windows)
2008-05-26 14:25 ` Johannes Schindelin
@ 2008-05-26 17:37 ` Mark Levedahl
2008-05-26 21:28 ` Johannes Schindelin
[not found] ` <483ADA17.3080401@viscovery.net>
2008-05-27 13:26 ` Commit cce8d6fdb introduces file t/t5100/nul, git tree is now incompatible with Cygwin (and probably Windows) Eric Blake
2 siblings, 1 reply; 42+ messages in thread
From: Mark Levedahl @ 2008-05-26 17:37 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Git Mailing List, Junio C Hamano
Johannes Schindelin wrote:
>> As a for instance of the troubles:
>>
>> git>git checkout -f origin/master
>> Previous HEAD position was a2f5be5... Merge branch
>> 'jk/maint-send-email-compose' into maint
>> error: git-checkout-index: unable to create file t/t5100/nul (File exists)
>>
>> As this commit is part of the published master branch, I am not sure the
>> correct resolution: leaving this commit in place means that any commit
>> between it and a commit fixing this will always cause an error on Cygwin
>> / Windows. Of course, it *is* on the published master branch.
>>
>
> That's the case for all regressions: we do not rewrite history for them.
>
>
I understand that, and the reasons: however, as this leads to a long
sequence of commits pointing to *illegal* trees, not just trees with bad
code, a different policy might be in order here. Or, it might not.
> As for the resolution, could you quickly try the 'my-next' branch of
> git://repo.or.cz/git/dscho.git?
>
>
I can check that branch out, but don't get very far in the tests:
*** t0004-unwritable.sh ***
* ok 1: setup
* FAIL 2: write-tree should notice unwritable repository
(
chmod a-w .git/objects
test_must_fail git write-tree
)
status=$?
chmod 775 .git/objects
(exit $status)
* FAIL 3: commit should notice unwritable repository
(
chmod a-w .git/objects
test_must_fail git commit -m second
)
status=$?
chmod 775 .git/objects
(exit $status)
* FAIL 4: update-index should notice unwritable repository
(
echo a >file &&
chmod a-w .git/objects
test_must_fail git update-index file
)
status=$?
chmod 775 .git/objects
(exit $status)
* FAIL 5: add should notice unwritable repository
(
echo b >file &&
chmod a-w .git/objects
test_must_fail git add file
)
status=$?
chmod 775 .git/objects
(exit $status)
* failed 4 among 5 test(s)
make[1]: *** [t0004-unwritable.sh] Error 1
make[1]: Leaving directory `/usr/src/dscho
make: *** [test] Error 2
I don't have access to a linux box today, so I can't manipulate master
to find if that branch with your patch would work right now.
Mark
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH] Makefile: wt-status.h is also a lib header
[not found] ` <483ADA17.3080401@viscovery.net>
@ 2008-05-26 21:21 ` Johannes Schindelin
2008-05-26 21:54 ` Junio C Hamano
0 siblings, 1 reply; 42+ messages in thread
From: Johannes Schindelin @ 2008-05-26 21:21 UTC (permalink / raw)
To: git, gitster
When a struct in wt-status.h changes, many files need to be rebuilt.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Hannes noticed that this patch was not yet accepted (or not
even submitted, I do not remember).
Makefile | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/Makefile b/Makefile
index dbfa2b6..aced19b 100644
--- a/Makefile
+++ b/Makefile
@@ -376,6 +376,7 @@ LIB_H += tree-walk.h
LIB_H += unpack-trees.h
LIB_H += utf8.h
LIB_H += levenshtein.h
+LIB_H += wt-status.h
LIB_OBJS += alias.o
LIB_OBJS += alloc.o
@@ -1129,7 +1130,6 @@ git-http-push$X: revision.o http.o http-push.o $(GITLIBS)
$(LIB_OBJS) $(BUILTIN_OBJS): $(LIB_H)
$(patsubst git-%$X,%.o,$(PROGRAMS)): $(LIB_H) $(wildcard */*.h)
-builtin-revert.o wt-status.o: wt-status.h
$(LIB_FILE): $(LIB_OBJS)
$(QUIET_AR)$(RM) $@ && $(AR) rcs $@ $(LIB_OBJS)
--
1.5.6.rc0.175.gdd78
^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: Commit cce8d6fdb introduces file t/t5100/nul, git tree is now incompatible with Cygwin (and probably Windows)
2008-05-26 17:37 ` Mark Levedahl
@ 2008-05-26 21:28 ` Johannes Schindelin
2008-05-26 22:49 ` Mark Levedahl
0 siblings, 1 reply; 42+ messages in thread
From: Johannes Schindelin @ 2008-05-26 21:28 UTC (permalink / raw)
To: Mark Levedahl; +Cc: Git Mailing List, Junio C Hamano
Hi,
On Mon, 26 May 2008, Mark Levedahl wrote:
> Johannes Schindelin wrote:
>
> > You wrote:
> >
> > > As this commit is part of the published master branch, I am not sure
> > > the correct resolution: leaving this commit in place means that any
> > > commit between it and a commit fixing this will always cause an
> > > error on Cygwin / Windows. Of course, it *is* on the published
> > > master branch.
> >
> > That's the case for all regressions: we do not rewrite history for
> > them.
>
> I understand that, and the reasons: however, as this leads to a long
> sequence of commits pointing to *illegal* trees, not just trees with bad
> code, a different policy might be in order here. Or, it might not.
I fail to see how Cygwin is so special as to merit a falsification of
history.
> > As for the resolution, could you quickly try the 'my-next' branch of
> > git://repo.or.cz/git/dscho.git?
>
> I can check that branch out, but don't get very far in the tests:
>
> *** t0004-unwritable.sh ***
> * ok 1: setup
> * FAIL 2: write-tree should notice unwritable repository
Sorry. Was worth a try.
> I don't have access to a linux box today, so I can't manipulate master
> to find if that branch with your patch would work right now.
Sure you can. You should be able to "git mv t/t5100/nul t/t5100/nul-file"
and then editing t/t5100-*.sh to refer to nul-file instead of nul.
Hth,
Dscho
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] Makefile: wt-status.h is also a lib header
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
0 siblings, 1 reply; 42+ messages in thread
From: Junio C Hamano @ 2008-05-26 21:54 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, gitster
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> When a struct in wt-status.h changes, many files need to be rebuilt.
What project, that has levenshtein.h, is this patch is for ;-)?
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: Commit cce8d6fdb introduces file t/t5100/nul, git tree is now incompatible with Cygwin (and probably Windows)
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
0 siblings, 2 replies; 42+ messages in thread
From: Mark Levedahl @ 2008-05-26 22:49 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Git Mailing List, Junio C Hamano
Johannes Schindelin wrote:
> Hi,
>
>
>
> I fail to see how Cygwin is so special as to merit a falsification of
> history.
>
>
>> I don't have access to a linux box today, so I can't manipulate master
>> to find if that branch with your patch would work right now.
>>
>
> Sure you can. You should be able to "git mv t/t5100/nul t/t5100/nul-file"
> and then editing t/t5100-*.sh to refer to nul-file instead of nul.
>
> Hth,
> Dscho
>
The above two points are related: the current master causes git to abort
before writing the index:
>git checkout -f origin/master
error: git-checkout-index: unable to create file t/t5100/nul (File exists)
>git mv t/t5100/nul t/t5100/nul-file
fatal: not under version control, source=t/t5100/nul,
destination=t/t5100/nul-file
So, there is now a range of git's history that is unusable (and
non-bisectable) on Windows, at least from the porcelain. And apparently,
somewhere in that unusable history, a change was introduced that causes
test failure on Cygwin. Great...
Mark
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] Makefile: wt-status.h is also a lib header
2008-05-26 21:54 ` Junio C Hamano
@ 2008-05-26 23:03 ` Johannes Schindelin
0 siblings, 0 replies; 42+ messages in thread
From: Johannes Schindelin @ 2008-05-26 23:03 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Hi,
On Mon, 26 May 2008, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > When a struct in wt-status.h changes, many files need to be rebuilt.
>
> What project, that has levenshtein.h, is this patch is for ;-)?
Heh. Missed that one. There was a discussion some time ago that the
rename detection should prefer names with a smaller Levenshtein distance,
so I implemented that (back when I still had some time).
This one is on top of the current 'next':
-- snipsnap --
[PATCH v2] Makefile: wt-status.h is also a lib header
When a struct in wt-status.h changes, many files need to be rebuilt.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Makefile | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/Makefile b/Makefile
index ef5ce5e..ab358b2 100644
--- a/Makefile
+++ b/Makefile
@@ -375,6 +375,7 @@ LIB_H += tree.h
LIB_H += tree-walk.h
LIB_H += unpack-trees.h
LIB_H += utf8.h
+LIB_H += wt-status.h
LIB_OBJS += alias.o
LIB_OBJS += alloc.o
@@ -1127,7 +1128,6 @@ git-http-push$X: revision.o http.o http-push.o $(GITLIBS)
$(LIB_OBJS) $(BUILTIN_OBJS): $(LIB_H)
$(patsubst git-%$X,%.o,$(PROGRAMS)): $(LIB_H) $(wildcard */*.h)
-builtin-revert.o wt-status.o: wt-status.h
$(LIB_FILE): $(LIB_OBJS)
$(QUIET_AR)$(RM) $@ && $(AR) rcs $@ $(LIB_OBJS)
--
1.5.6.rc0.175.gdd78
^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: Commit cce8d6fdb introduces file t/t5100/nul, git tree is now incompatible with Cygwin (and probably Windows)
2008-05-26 22:49 ` Mark Levedahl
@ 2008-05-26 23:10 ` Johannes Schindelin
2008-05-26 23:15 ` Johannes Schindelin
1 sibling, 0 replies; 42+ messages in thread
From: Johannes Schindelin @ 2008-05-26 23:10 UTC (permalink / raw)
To: Mark Levedahl; +Cc: Git Mailing List, Junio C Hamano
Hi,
On Mon, 26 May 2008, Mark Levedahl wrote:
> So, there is now a range of git's history that is unusable (and
> non-bisectable) on Windows, at least from the porcelain. And apparently,
> somewhere in that unusable history, a change was introduced that causes
> test failure on Cygwin. Great...
Oh, for the love of God! It is not like we will not fix this problem
eventually! Screwing a lot of users by rewriting history, just because of
Windows, which we need too many ugly work-arounds in Git's source code for
anyway, is _not_ an option.
Or would you suggest to scrap almost the complete history of Git just
because most of it does not compile on platform XYZ, while the initial
revision did? Exactly.
Ciao,
Dscho
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: Commit cce8d6fdb introduces file t/t5100/nul, git tree is now incompatible with Cygwin (and probably Windows)
2008-05-26 22:49 ` Mark Levedahl
2008-05-26 23:10 ` Johannes Schindelin
@ 2008-05-26 23:15 ` Johannes Schindelin
1 sibling, 0 replies; 42+ messages in thread
From: Johannes Schindelin @ 2008-05-26 23:15 UTC (permalink / raw)
To: Mark Levedahl; +Cc: Git Mailing List, Junio C Hamano
Hi,
On Mon, 26 May 2008, Mark Levedahl wrote:
> >git checkout -f origin/master
> error: git-checkout-index: unable to create file t/t5100/nul (File exists)
> >git mv t/t5100/nul t/t5100/nul-file
> fatal: not under version control, source=t/t5100/nul,
> destination=t/t5100/nul-file
I should not do this, because you cost me already too much time with your
outrageous proposal to rewrite history just for that stupid platform
called Windows, whose users seem to complain more than actually
contribute, but here it goes:
http://repo.or.cz/w/git/dscho.git?a=shortlog;h=refs/heads/nul-fix
The corresponding patch:
-- snipsnap --
[PATCH] Fix t5100 for Windows
On Windows, "nul" is not allowed as a file name. So bend over for that
platform even more than we do already.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
t/t5100-mailinfo.sh | 4 ++--
t/t5100/{nul => nul-plain} | Bin 91 -> 91 bytes
2 files changed, 2 insertions(+), 2 deletions(-)
rename t/t5100/{nul => nul-plain} (100%)
diff --git a/t/t5100-mailinfo.sh b/t/t5100-mailinfo.sh
index a8b78eb..577ecc2 100755
--- a/t/t5100-mailinfo.sh
+++ b/t/t5100-mailinfo.sh
@@ -27,8 +27,8 @@ done
test_expect_success 'respect NULs' '
- git mailsplit -d3 -o. ../t5100/nul &&
- cmp ../t5100/nul 001 &&
+ git mailsplit -d3 -o. ../t5100/nul-plain &&
+ cmp ../t5100/nul-plain 001 &&
(cat 001 | git mailinfo msg patch) &&
test 4 = $(wc -l < patch)
diff --git a/t/t5100/nul b/t/t5100/nul-plain
similarity index 100%
rename from t/t5100/nul
rename to t/t5100/nul-plain
--
1.5.6.rc0.175.gdd78
^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: Commit cce8d6fdb introduces file t/t5100/nul, git tree is now incompatible with Cygwin (and probably Windows)
2008-05-26 14:25 ` Johannes Schindelin
2008-05-26 17:37 ` Mark Levedahl
[not found] ` <483ADA17.3080401@viscovery.net>
@ 2008-05-27 13:26 ` Eric Blake
2 siblings, 0 replies; 42+ messages in thread
From: Eric Blake @ 2008-05-27 13:26 UTC (permalink / raw)
To: git
Johannes Schindelin <Johannes.Schindelin <at> gmx.de> writes:
> On Mon, 26 May 2008, Mark Levedahl wrote:
>
> > Beginning with the referenced commit, the git project cannot be checked
> > out on Cygwin (and I assume cannot be checked out on Windows using
> > msysgit, though I have not verified this) as this commit introduces the
> > file "t/5100/nul." On Windows, the file name "nul" is reserved,
> > regardless of path, and cannot be created or deleted. It serves
> > essentially the same function as /dev/null.
>
> Even when referencing the full (or a relative) path? That's bad!
Yes. And it is not limited to the case-insensitive name NUL; it also covers
names like "aux". See the very last paragraph in this section:
http://www.gnu.org/software/autoconf/manual/html_node/File-System-Conventions.html#File-System-Conventions
This will affect all versions of MSYS. However, with cygwin 1.5.x, you can use
the workaround of a managed mount which intentionally (and transparently) munges
such invalid file names so that you can appear to name a file "nul" in spite of
Windows (at the expense of making an already short PATH_MAX of 256 even
shorter). And if you are willing to experiment with the (still-in-development)
cygwin 1.7.0, this munging is done without even needing a managed mount and
without any penalty to the larger PATH_MAX of 4k.
At any rate, I agree with your patch to rename the file, as well as with your
aversion to rewriting history just so that a checkout on MSYS or a non-managed
mount on older cygwin can do a 'git bisect' that hits the small window of
commits with an invalid tree.
--
Eric Blake
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: Commit cce8d6fdb introduces file t/t5100/nul, git tree is now incompatible with Cygwin (and probably Windows)
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-28 6:12 ` Junio C Hamano
2008-05-28 9:46 ` Wincent Colaiuta
2008-05-28 16:33 ` Commit cce8d6fdb introduces file t/t5100/nul, git tree is now incompatible with Cygwin (and probably Windows) Avery Pennarun
1 sibling, 2 replies; 42+ messages in thread
From: Junio C Hamano @ 2008-05-28 6:12 UTC (permalink / raw)
To: Mark Levedahl; +Cc: Git Mailing List, Johannes Schindelin
Mark Levedahl <mlevedahl@gmail.com> writes:
> As this commit is part of the published master branch, I am not sure
> the correct resolution: leaving this commit in place means that any
> commit between it and a commit fixing this will always cause an error
> on Cygwin / Windows. Of course, it *is* on the published master branch.
Some broken filesystems may not be capable of checking out and using
project files. Too bad.
It's not a big deal. It is not limited to this project. We just fix them
or work them around and move on.
Perhaps we should remove the infamous gitweb/test/Märchen file while we
are at it? I do not think the file is ever used.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: Commit cce8d6fdb introduces file t/t5100/nul, git tree is now incompatible with Cygwin (and probably Windows)
2008-05-28 6:12 ` Junio C Hamano
@ 2008-05-28 9:46 ` Wincent Colaiuta
2008-05-28 15:53 ` Lea Wiemann
2008-05-29 13:22 ` Johannes Schindelin
2008-05-28 16:33 ` Commit cce8d6fdb introduces file t/t5100/nul, git tree is now incompatible with Cygwin (and probably Windows) Avery Pennarun
1 sibling, 2 replies; 42+ messages in thread
From: Wincent Colaiuta @ 2008-05-28 9:46 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Mark Levedahl, Git Mailing List, Johannes Schindelin
El 28/5/2008, a las 8:12, Junio C Hamano escribió:
> Mark Levedahl <mlevedahl@gmail.com> writes:
>
>> As this commit is part of the published master branch, I am not sure
>> the correct resolution: leaving this commit in place means that any
>> commit between it and a commit fixing this will always cause an error
>> on Cygwin / Windows. Of course, it *is* on the published master
>> branch.
>
> Some broken filesystems may not be capable of checking out and using
> project files. Too bad.
>
> It's not a big deal. It is not limited to this project. We just
> fix them
> or work them around and move on.
>
> Perhaps we should remove the infamous gitweb/test/Märchen file while
> we
> are at it? I do not think the file is ever used.
I for one would love to see it go, seeing as I live in the ghetto that
is HFS+ and am constantly annoyed by it cluttering up my status output
with spurious content.
I understand that the reason it lives in the tree is precisely to
discover problems with such filesystems, but the problem is well and
truly discovered by now and I'd much rather see this kind of thing
tested from within the test suite rather than every time I do "git
status" or "git checkout".
Cheers,
Wincent
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: Commit cce8d6fdb introduces file t/t5100/nul, git tree is now incompatible with Cygwin (and probably Windows)
2008-05-28 9:46 ` Wincent Colaiuta
@ 2008-05-28 15:53 ` Lea Wiemann
2008-05-28 15:58 ` Wincent Colaiuta
2008-05-29 13:22 ` Johannes Schindelin
1 sibling, 1 reply; 42+ messages in thread
From: Lea Wiemann @ 2008-05-28 15:53 UTC (permalink / raw)
To: Wincent Colaiuta
Cc: Junio C Hamano, Mark Levedahl, Git Mailing List,
Johannes Schindelin
Wincent Colaiuta wrote:
> El 28/5/2008, a las 8:12, Junio C Hamano escribió:
>> Perhaps we should remove the infamous gitweb/test/Märchen file
>
> [...] I'd much rather see this kind of thing
> tested from within the test suite rather than every time I do "git
> status" or "git checkout".
I don't believe the Märchen file is actually used in any test code, so
removing it should be fine. If/when we actually write test code for
gitweb, it seems to me that we might as well generate such test files on
the fly from within the test suite, rather than having them in the file
system permanently.
Best,
Lea
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: Commit cce8d6fdb introduces file t/t5100/nul, git tree is now incompatible with Cygwin (and probably Windows)
2008-05-28 15:53 ` Lea Wiemann
@ 2008-05-28 15:58 ` Wincent Colaiuta
2008-05-28 21:39 ` Jakub Narebski
0 siblings, 1 reply; 42+ messages in thread
From: Wincent Colaiuta @ 2008-05-28 15:58 UTC (permalink / raw)
To: Lea Wiemann
Cc: Junio C Hamano, Mark Levedahl, Git Mailing List,
Johannes Schindelin
El 28/5/2008, a las 17:53, Lea Wiemann escribió:
> Wincent Colaiuta wrote:
>> El 28/5/2008, a las 8:12, Junio C Hamano escribió:
>>> Perhaps we should remove the infamous gitweb/test/Märchen file
>> [...] I'd much rather see this kind of thing tested from within
>> the test suite rather than every time I do "git status" or "git
>> checkout".
>
> I don't believe the Märchen file is actually used in any test code,
> so removing it should be fine. If/when we actually write test code
> for gitweb, it seems to me that we might as well generate such test
> files on the fly from within the test suite, rather than having them
> in the file system permanently.
Yes, that's exactly what I intended my comment to imply. Test at test
time, not every time I do "git status" and "git checkout" etc.
Cheers,
Wincent
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: Commit cce8d6fdb introduces file t/t5100/nul, git tree is now incompatible with Cygwin (and probably Windows)
2008-05-28 6:12 ` Junio C Hamano
2008-05-28 9:46 ` Wincent Colaiuta
@ 2008-05-28 16:33 ` Avery Pennarun
2008-05-28 17:24 ` Junio C Hamano
1 sibling, 1 reply; 42+ messages in thread
From: Avery Pennarun @ 2008-05-28 16:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Mark Levedahl, Git Mailing List, Johannes Schindelin
On 5/28/08, Junio C Hamano <gitster@pobox.com> wrote:
> Mark Levedahl <mlevedahl@gmail.com> writes:
>
> > As this commit is part of the published master branch, I am not sure
> > the correct resolution: leaving this commit in place means that any
> > commit between it and a commit fixing this will always cause an error
> > on Cygwin / Windows. Of course, it *is* on the published master branch.
>
> Some broken filesystems may not be capable of checking out and using
> project files. Too bad.
>
> It's not a big deal. It is not limited to this project. We just fix them
> or work them around and move on.
>
> Perhaps we should remove the infamous gitweb/test/Märchen file while we
> are at it? I do not think the file is ever used.
As an unhappy Windows user myself (sometimes), I think it might be
better to simply fix git to *survive* failing to create files like
'nul' on Win32, rather than trying to *fix* such files in the repo.
It sounds (from the original post) like git has a *fatal* error
("cannot be checked out on cygwin") when trying to create 'nul', which
might be overkill.
The argument about rewriting the git history for this one file is
mostly predicated on the fact that you can't git-bisect if this file
exists. Rather than making the file not exist, it might be better to
make git work when it does.
Have fun,
Avery
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: Commit cce8d6fdb introduces file t/t5100/nul, git tree is now incompatible with Cygwin (and probably Windows)
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
` (2 more replies)
0 siblings, 3 replies; 42+ messages in thread
From: Junio C Hamano @ 2008-05-28 17:24 UTC (permalink / raw)
To: Avery Pennarun
Cc: Junio C Hamano, Mark Levedahl, Git Mailing List,
Johannes Schindelin
"Avery Pennarun" <apenwarr@gmail.com> writes:
> As an unhappy Windows user myself (sometimes), I think it might be
> better to simply fix git to *survive* failing to create files like
> 'nul' on Win32, rather than trying to *fix* such files in the repo.
> It sounds (from the original post) like git has a *fatal* error
> ("cannot be checked out on cygwin") when trying to create 'nul'.
Please learn to think before typing, let alone sending such a message to
waste other people's time.
We give the user an error message, and signal error by exiting with
non-zero. You cannot have that path on the system, and we are being
honest about it. It is not like we are suddenly painting the screen in
blue and refusing to get any more user input when you try to check out
such a tree. Which part of that is _not_ surviving?
The system with a *fatal* error is not git but the one that does not want
an not-so-unreasonable name "NUL" on it. Git survives on such a system
and tells you what happened --- you cannot do certain things, such as
checking out such a tree. You live with it, or get a better system ;-)
What alternatives do you want to implement? Certainly not silently
creating "nul-garbage" file instead and pretend that nothing bad happened,
as that would lead to madness.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: Commit cce8d6fdb introduces file t/t5100/nul, git tree is now incompatible with Cygwin (and probably Windows)
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:19 ` Daniel Barkalow
2 siblings, 0 replies; 42+ messages in thread
From: Sverre Rabbelier @ 2008-05-28 17:46 UTC (permalink / raw)
To: Junio C Hamano
Cc: Avery Pennarun, Mark Levedahl, Git Mailing List,
Johannes Schindelin
On Wed, May 28, 2008 at 7:24 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Please learn to think before typing, let alone sending such a message to
> waste other people's time.
Why so harsh?
> What alternatives do you want to implement? Certainly not silently
> creating "nul-garbage" file instead and pretend that nothing bad happened,
> as that would lead to madness.
Or instead we could have a '-f' switch or such with checkout that
allows you to checkout a revision that contains a bad file, but with
that file missing. Of course this needn't happen silently, a big
warning saying "could not checkout file %s because of %s" may still be
issued.
--
Cheers,
Sverre Rabbelier
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: Commit cce8d6fdb introduces file t/t5100/nul, git tree is now incompatible with Cygwin (and probably Windows)
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
2 siblings, 1 reply; 42+ messages in thread
From: Avery Pennarun @ 2008-05-28 17:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Mark Levedahl, Git Mailing List, Johannes Schindelin
On 5/28/08, Junio C Hamano <gitster@pobox.com> wrote:
> Please learn to think before typing, let alone sending such a message to
> waste other people's time.
Phew, just mentioning Windows on this list seems to get people flamed.
Reading over my message, I'm not sure what I would have written to
have it interpreted as being negative. I was simply suggesting that
git could fail more gracefully here.
> We give the user an error message, and signal error by exiting with
> non-zero. You cannot have that path on the system, and we are being
> honest about it. It is not like we are suddenly painting the screen in
> blue and refusing to get any more user input when you try to check out
> such a tree. Which part of that is _not_ surviving?
The part that's not surviving is git-bisect, which is a valid problem
that resulted in someone asking to rewrite the history. Clearly
rewriting the history is not a good solution, thus no good solution
has yet been proposed, which is why I wrote my message.
> The system with a *fatal* error is not git but the one that does not want
> an not-so-unreasonable name "NUL" on it.
That is clearly true. But knowing that doesn't seem to be making this
user's problem go away.
> What alternatives do you want to implement? Certainly not silently
> creating "nul-garbage" file instead and pretend that nothing bad happened,
> as that would lead to madness.
If the file failed to be created (with a warning), but we treated it
as having been deleted in 'git status' instead of throwing an error,
it would work much like the case sensitivity problem: the index is
annoyingly dirty, but you can still get work done. I think that
(perhaps with a little patch) would allow git-bisect to work.
Have fun,
Avery
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: Commit cce8d6fdb introduces file t/t5100/nul, git tree is now incompatible with Cygwin (and probably Windows)
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:19 ` Daniel Barkalow
2008-05-28 18:37 ` Junio C Hamano
2 siblings, 1 reply; 42+ messages in thread
From: Daniel Barkalow @ 2008-05-28 18:19 UTC (permalink / raw)
To: Junio C Hamano
Cc: Avery Pennarun, Mark Levedahl, Git Mailing List,
Johannes Schindelin
On Wed, 28 May 2008, Junio C Hamano wrote:
> "Avery Pennarun" <apenwarr@gmail.com> writes:
>
> > As an unhappy Windows user myself (sometimes), I think it might be
> > better to simply fix git to *survive* failing to create files like
> > 'nul' on Win32, rather than trying to *fix* such files in the repo.
> > It sounds (from the original post) like git has a *fatal* error
> > ("cannot be checked out on cygwin") when trying to create 'nul'.
>
> Please learn to think before typing, let alone sending such a message to
> waste other people's time.
>
> We give the user an error message, and signal error by exiting with
> non-zero. You cannot have that path on the system, and we are being
> honest about it. It is not like we are suddenly painting the screen in
> blue and refusing to get any more user input when you try to check out
> such a tree. Which part of that is _not_ surviving?
>
> The system with a *fatal* error is not git but the one that does not want
> an not-so-unreasonable name "NUL" on it. Git survives on such a system
> and tells you what happened --- you cannot do certain things, such as
> checking out such a tree. You live with it, or get a better system ;-)
>
> What alternatives do you want to implement? Certainly not silently
> creating "nul-garbage" file instead and pretend that nothing bad happened,
> as that would lead to madness.
Report a non-fatal error, mark in the index that that entry is not
reflected in the working directory, and allow the user to manipulate it
with commands that don't really need the working directory content.
$ git checkout origin/master
Warning: couldn't create 't/t5100/nul' in your working directory; ignoring
working directory for this filename.
$ git mv t/t5100/nul t/t5100/nul-plain
$ ls t/t5100/nul-plain
t/t5100/nul-plain
The working directory doesn't really have to be absolutely vital to git's
functioning (of course, the project you've checked out is going to have
problems unless you fix things). In particular, it should be possible, on
a machine with a broken filesystem, to modify a project that triggers the
filesystem breakage to not trigger it, rather than having to rely on
either read-tree/update-index --cachinfo/write-tree or a helpful user of a
non-broken system to get things working again.
Git should, I think, even be able to figure out by itself when to not
trust the filesystem; if open gives EEXIST, but readdir doesn't list it,
it's a filesystem problem and we should work around it.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: Commit cce8d6fdb introduces file t/t5100/nul, git tree is now incompatible with Cygwin (and probably Windows)
2008-05-28 17:52 ` Avery Pennarun
@ 2008-05-28 18:27 ` Junio C Hamano
0 siblings, 0 replies; 42+ messages in thread
From: Junio C Hamano @ 2008-05-28 18:27 UTC (permalink / raw)
To: Avery Pennarun; +Cc: Mark Levedahl, Git Mailing List, Johannes Schindelin
"Avery Pennarun" <apenwarr@gmail.com> writes:
> On 5/28/08, Junio C Hamano <gitster@pobox.com> wrote:
>> Please learn to think before typing, let alone sending such a message to
>> waste other people's time.
>
> Phew, just mentioning Windows on this list seems to get people flamed.
I did not flame Windows.
> Reading over my message, I'm not sure what I would have written to
> have it interpreted as being negative....
> The part that's not surviving is git-bisect, which is a valid problem
> that resulted in someone asking to rewrite the history. Clearly
> rewriting the history is not a good solution, thus no good solution
> has yet been proposed, which is why I wrote my message.
Here is what you wrote:
As an unhappy Windows user myself (sometimes), I think it might be
better to simply fix git to *survive* failing to create files like
'nul' on Win32, rather than trying to *fix* such files in the repo.
It sounds (from the original post) like git has a *fatal* error
("cannot be checked out on cygwin") when trying to create 'nul', which
might be overkill.
"Survive failing to create files"? "Fatal error when trying to create
'nul', which might be overkill"?
If you cannot faithfully recreate the work tree state, you cannot test
that revision. You cannot bisect and test that revision on that system.
Too bad, but you still have "git reset --hard". You reset to some other
revision that you _can_ checkout and do sensible tests and move on.
How would you know to reset to recover from your system's deficiency
unless git gives "fatal error"?
That's why I asked you to think before type, which again you did not do,
and that is what got you get "flamed", even though I do not think it was
particularly harsh.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: Commit cce8d6fdb introduces file t/t5100/nul, git tree is now incompatible with Cygwin (and probably Windows)
2008-05-28 18:19 ` Daniel Barkalow
@ 2008-05-28 18:37 ` Junio C Hamano
2008-05-28 20:06 ` Daniel Barkalow
0 siblings, 1 reply; 42+ messages in thread
From: Junio C Hamano @ 2008-05-28 18:37 UTC (permalink / raw)
To: Daniel Barkalow
Cc: Avery Pennarun, Mark Levedahl, Git Mailing List,
Johannes Schindelin
Daniel Barkalow <barkalow@iabervon.org> writes:
> Report a non-fatal error, mark in the index that that entry is not
> reflected in the working directory, and allow the user to manipulate it
> with commands that don't really need the working directory content.
>
> $ git checkout origin/master
> Warning: couldn't create 't/t5100/nul' in your working directory; ignoring
> working directory for this filename.
> $ git mv t/t5100/nul t/t5100/nul-plain
> $ ls t/t5100/nul-plain
> t/t5100/nul-plain
>
> The working directory doesn't really have to be absolutely vital to git's
> functioning (of course, the project you've checked out is going to have
> problems unless you fix things). In particular, it should be possible, on
> a machine with a broken filesystem, to modify a project that triggers the
> filesystem breakage to not trigger it,...
Now that is somebody who thinks before types.
Marking that the filesystem does not match what's in index is already
done, so you could argue that an alternative would be not to stop in the
middle of checkout_entry() loop and instead check out as much as we could,
write out the index perhaps, and signal error, _AFTER_ updating everything
else, including the HEAD. We try to be atomic when able (e.g. on a broken
patch, "apply" does not apply early half the patch and fail but rejects
the whole thing), but checkout_entry() loop is not something you can
sanely make atomic (it needs to first remove existing files and even
directories before writing new files), so that alternative approach might
be easier to work with.
Care to follow it up with a patch?
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: Commit cce8d6fdb introduces file t/t5100/nul, git tree is now incompatible with Cygwin (and probably Windows)
2008-05-28 18:37 ` Junio C Hamano
@ 2008-05-28 20:06 ` Daniel Barkalow
2008-05-28 20:43 ` Junio C Hamano
0 siblings, 1 reply; 42+ messages in thread
From: Daniel Barkalow @ 2008-05-28 20:06 UTC (permalink / raw)
To: Junio C Hamano
Cc: Avery Pennarun, Mark Levedahl, Git Mailing List,
Johannes Schindelin
On Wed, 28 May 2008, Junio C Hamano wrote:
> Daniel Barkalow <barkalow@iabervon.org> writes:
>
> > Report a non-fatal error, mark in the index that that entry is not
> > reflected in the working directory, and allow the user to manipulate it
> > with commands that don't really need the working directory content.
> >
> > $ git checkout origin/master
> > Warning: couldn't create 't/t5100/nul' in your working directory; ignoring
> > working directory for this filename.
> > $ git mv t/t5100/nul t/t5100/nul-plain
> > $ ls t/t5100/nul-plain
> > t/t5100/nul-plain
> >
> > The working directory doesn't really have to be absolutely vital to git's
> > functioning (of course, the project you've checked out is going to have
> > problems unless you fix things). In particular, it should be possible, on
> > a machine with a broken filesystem, to modify a project that triggers the
> > filesystem breakage to not trigger it,...
>
> Now that is somebody who thinks before types.
Well, and I've got the background to know what's possible and how git can
keep things straight.
> Marking that the filesystem does not match what's in index is already
> done, so you could argue that an alternative would be not to stop in the
> middle of checkout_entry() loop and instead check out as much as we could,
> write out the index perhaps, and signal error, _AFTER_ updating everything
> else, including the HEAD. We try to be atomic when able (e.g. on a broken
> patch, "apply" does not apply early half the patch and fail but rejects
> the whole thing), but checkout_entry() loop is not something you can
> sanely make atomic (it needs to first remove existing files and even
> directories before writing new files), so that alternative approach might
> be easier to work with.
Ah, yes, CE_VALID. But it doesn't quite work as well as I'd like, because
it doesn't ignore fstat/readdir not finding anything on the filesystem, so
it comes out looking deleted, at least if you're actually on Linux (with a
hack in create_file to refuse to create certain filenames for testing).
Also "git mv CE_VALID-source dest" doesn't ignore the filesystem like it
should (for this use, anyway). Perhaps we need an additional flag for "the
filesystem is irrelevant for this entry".
> Care to follow it up with a patch?
Unfortunately, I don't think I'll have a chance for a while to actually
work on git code. But if someone else (Avery?) wants to try it, I think
giving a big warning, setting CE_VALID, and returning 0 in entry.c before
the "unable to create file" message is the right thing to start with. And
someone with Windows access should figure out what happens next.
I think the right test for this is if create_file() returns EEXIST, but
readdir doesn't show anything. For that matter, it might be useful to have
logic 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.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: Commit cce8d6fdb introduces file t/t5100/nul, git tree is now incompatible with Cygwin (and probably Windows)
2008-05-28 20:06 ` Daniel Barkalow
@ 2008-05-28 20:43 ` Junio C Hamano
2008-05-28 21:19 ` [PATCH] "git checkout -- paths..." should signal error Junio C Hamano
2008-05-28 21:41 ` Commit cce8d6fdb introduces file t/t5100/nul, git tree is now incompatible with Cygwin (and probably Windows) Daniel Barkalow
0 siblings, 2 replies; 42+ messages in thread
From: Junio C Hamano @ 2008-05-28 20:43 UTC (permalink / raw)
To: Daniel Barkalow
Cc: Avery Pennarun, Mark Levedahl, Git Mailing List,
Johannes Schindelin
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.
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH] "git checkout -- paths..." should signal error
2008-05-28 20:43 ` Junio C Hamano
@ 2008-05-28 21:19 ` 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
1 sibling, 2 replies; 42+ messages in thread
From: Junio C Hamano @ 2008-05-28 21:19 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: Git Mailing List
When "git checkout -- paths..." cannot update work tree for whatever
reason, checkout_entry() correctly issued an error message for the path to
the end user, but the command ignored the error, causing the entire
command to succeed. This fixes it.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* Now that we detect this failure, post-checkout-hook _might_ want to
know that what we have is an incomplete checkout. Not calling the hook
in such a case may be another option, but we always called the hook and
that would be a change in behaviour.
builtin-checkout.c | 7 +++++--
1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/builtin-checkout.c b/builtin-checkout.c
index 1ea017f..00dc8ca 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -84,6 +84,7 @@ static int checkout_paths(struct tree *source_tree, const char **pathspec)
unsigned char rev[20];
int flag;
struct commit *head;
+ int errs = 0;
int newfd;
struct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));
@@ -106,13 +107,14 @@ static int checkout_paths(struct tree *source_tree, const char **pathspec)
if (report_path_error(ps_matched, pathspec, 0))
return 1;
+ /* Now we are committed to check them out */
memset(&state, 0, sizeof(state));
state.force = 1;
state.refresh_cache = 1;
for (pos = 0; pos < active_nr; pos++) {
struct cache_entry *ce = active_cache[pos];
if (pathspec_match(pathspec, NULL, ce->name, 0)) {
- checkout_entry(ce, &state, NULL);
+ errs |= checkout_entry(ce, &state, NULL);
}
}
@@ -123,7 +125,8 @@ static int checkout_paths(struct tree *source_tree, const char **pathspec)
resolve_ref("HEAD", rev, 0, &flag);
head = lookup_commit_reference_gently(rev, 1);
- return post_checkout_hook(head, head, 0);
+ errs |= post_checkout_hook(head, head, 0);
+ return errs;
}
static void show_local_changes(struct object *head)
^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: Commit cce8d6fdb introduces file t/t5100/nul, git tree is now incompatible with Cygwin (and probably Windows)
2008-05-28 15:58 ` Wincent Colaiuta
@ 2008-05-28 21:39 ` Jakub Narebski
0 siblings, 0 replies; 42+ messages in thread
From: Jakub Narebski @ 2008-05-28 21:39 UTC (permalink / raw)
To: git
Wincent Colaiuta wrote:
> El 28/5/2008, a las 17:53, Lea Wiemann escribió:
>> Wincent Colaiuta wrote:
>>> El 28/5/2008, a las 8:12, Junio C Hamano escribió:
>>>>
>>>> Perhaps we should remove the infamous gitweb/test/Märchen file
>>>
>>> [...] I'd much rather see this kind of thing tested from within
>>> the test suite rather than every time I do "git status" or "git
>>> checkout".
>>
>> I don't believe the Märchen file is actually used in any test code,
>> so removing it should be fine. If/when we actually write test code
>> for gitweb, it seems to me that we might as well generate such test
>> files on the fly from within the test suite, rather than having them
>> in the file system permanently.
>
> Yes, that's exactly what I intended my comment to imply. Test at test
> time, not every time I do "git status" and "git checkout" etc.
I think it is remainder (the whole gitweb/test/ directory) from the
times when gitweb was separate project, and not part of git.
--
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: Commit cce8d6fdb introduces file t/t5100/nul, git tree is now incompatible with Cygwin (and probably Windows)
2008-05-28 20:43 ` Junio C Hamano
2008-05-28 21:19 ` [PATCH] "git checkout -- paths..." should signal error Junio C Hamano
@ 2008-05-28 21:41 ` Daniel Barkalow
1 sibling, 0 replies; 42+ messages in thread
From: Daniel Barkalow @ 2008-05-28 21:41 UTC (permalink / raw)
To: Junio C Hamano
Cc: Avery Pennarun, Mark Levedahl, Git Mailing List,
Johannes Schindelin
On Wed, 28 May 2008, Junio C Hamano wrote:
> 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 there are two separate issues here: (1) you've gotten partway
through a checkout, and something fails, and we want to leave things in as
good shape as possible; (2) you're trying to check out a tree that can't
be accurately represented on your filesystem. In case (2), there's a good
chance that you want to leave the unrepresentable stuff alone or modify it
in the index without going through the filesystem.
Case (2) is very much like working on a filesystem without +x bits, where
you'd really like to ignore what the filesystem reports and only diverge
from what was read into the index from a tree object when the user makes
explicit modifications to the index, rather than updating the index from
the working directory state.
> > 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.
It's not just EEXIST; it's EEXIST for a filename with no struct dirent.
> 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.
I think the leading directory thing is qualitatively different; you're
probably not going to want to create a patch to rename all of the files in
that directory to be in a directory that's not owned by a different user
on your machine, while you are likely to try to get a project to use less
odd names if there's a filename with a : in it or something. I'm not sure
what error open() will give you for that.
The other interesting case is when the filesystem is case insensative and
the project contains files that differ only in case; again, the filesystem
will report EEXIST on open for a path that readdir doesn't list.
> 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".
For case (1), things are in an inconsistant state; for case (2), the index
and HEAD agree, and there are known gaps in the work tree.
> 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.
People seem to use Windows and OS X despite the filesystems being broken.
There seems to be a space of filesystems which aren't corrupted, but do
unexpected things with respect to filenames. If we can find appropriate
filenames, the content is stored reliably (or the open O_EXCL is refused).
We should be able to identify that the user isn't trying to make a change
by way of the working directory when the difference we see is something
that isn't clearly representable by the filesystem.
> But I wouldn't keep people from experimenting. Perhaps the end result
> could be even readable and mergeable, although I am quite pessimistic.
I think, in any case, that it should be pretty clean to have a "filesystem
is inadaquate" CE flag, which mean that we just ignore the filesystem.
Finding things that are reported with the wrong name is probably harder.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] "git checkout -- paths..." should signal error
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
1 sibling, 0 replies; 42+ messages in thread
From: Marius Storm-Olsen @ 2008-05-29 6:28 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Daniel Barkalow, Git Mailing List, msysGit
[-- Attachment #1: Type: text/plain, Size: 2043 bytes --]
Junio C Hamano said the following on 28.05.2008 23:19:
> When "git checkout -- paths..." cannot update work tree for whatever
> reason, checkout_entry() correctly issued an error message for the path to
> the end user, but the command ignored the error, causing the entire
> command to succeed. This fixes it.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>
> * Now that we detect this failure, post-checkout-hook _might_ want to
> know that what we have is an incomplete checkout. Not calling the hook
> in such a case may be another option, but we always called the hook and
> that would be a change in behaviour.
>
> builtin-checkout.c | 7 +++++--
> 1 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/builtin-checkout.c b/builtin-checkout.c
I'd like to just chip in that you actually _can_ create those files
(nul, con, prn, all which are "special" filenames on Windows), but
need a slight different construct to create them. That, and most tools
will break on _using_ them, since they's also have to refer to them in
a certain way.
So, certainly msysgit could write a workaround to handle such cases.
(Git would also know how to remove these files on Windows)
Though, I still think that your patch is a Good Thing(tm).
For example, if you:
D:\some\path> echo foo > nul # Ok, an no output, nor file created
D:\some\path> echo foo > .\nul # Same thing
D:\some\path> echo foo > \\.\d:\some\path\nul # Creates the nul file
Directory of D:\some\path
29.05.2008 08:23 <DIR> .
29.05.2008 08:23 <DIR> ..
29.05.2008 08:23 6 con
29.05.2008 08:22 6 nul
29.05.2008 08:23 6 prn
3 File(s) 18 bytes
D:\some\path> del .\nul
The filename, directory name, or volume label syntax is incorrect.
D:\some\path> del \\.\d:\some\path\nul # Success
--
.marius [@trolltech.com]
'if you know what you're doing, it's not research'
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 187 bytes --]
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] "git checkout -- paths..." should signal error
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
1 sibling, 0 replies; 42+ messages in thread
From: Daniel Barkalow @ 2008-05-29 13:05 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
On Wed, 28 May 2008, Junio C Hamano wrote:
> When "git checkout -- paths..." cannot update work tree for whatever
> reason, checkout_entry() correctly issued an error message for the path to
> the end user, but the command ignored the error, causing the entire
> command to succeed. This fixes it.
Acked-by: Daniel Barkalow <barkalow@iabervon.org>
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: Commit cce8d6fdb introduces file t/t5100/nul, git tree is now incompatible with Cygwin (and probably Windows)
2008-05-28 9:46 ` Wincent Colaiuta
2008-05-28 15:53 ` Lea Wiemann
@ 2008-05-29 13:22 ` Johannes Schindelin
2008-05-29 14:58 ` Wincent Colaiuta
1 sibling, 1 reply; 42+ messages in thread
From: Johannes Schindelin @ 2008-05-29 13:22 UTC (permalink / raw)
To: Wincent Colaiuta; +Cc: Junio C Hamano, Mark Levedahl, Git Mailing List
[-- Attachment #1: Type: TEXT/PLAIN, Size: 796 bytes --]
Hi,
On Wed, 28 May 2008, Wincent Colaiuta wrote:
> El 28/5/2008, a las 8:12, Junio C Hamano escribió:
>
> >Perhaps we should remove the infamous gitweb/test/Märchen file while we
> >are at it? I do not think the file is ever used.
>
> I for one would love to see it go, seeing as I live in the ghetto that
> is HFS+ and am constantly annoyed by it cluttering up my status output
> with spurious content.
>
> I understand that the reason it lives in the tree is precisely to
> discover problems with such filesystems, but the problem is well and
> truly discovered by now and I'd much rather see this kind of thing
> tested from within the test suite rather than every time I do "git
> status" or "git checkout".
Probably you are not enoyed enough to just go and fix it.
Ciao,
Dscho
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: Commit cce8d6fdb introduces file t/t5100/nul, git tree is now incompatible with Cygwin (and probably Windows)
2008-05-29 13:22 ` Johannes Schindelin
@ 2008-05-29 14:58 ` Wincent Colaiuta
2008-05-29 16:05 ` Johannes Schindelin
2008-05-31 17:37 ` Steffen Prohaska
0 siblings, 2 replies; 42+ messages in thread
From: Wincent Colaiuta @ 2008-05-29 14:58 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, Mark Levedahl, Git Mailing List
El 29/5/2008, a las 15:22, Johannes Schindelin escribió:
> Hi,
>
> On Wed, 28 May 2008, Wincent Colaiuta wrote:
>
>> El 28/5/2008, a las 8:12, Junio C Hamano escribió:
>>
>>> Perhaps we should remove the infamous gitweb/test/Märchen file
>>> while we
>>> are at it? I do not think the file is ever used.
>>
>> I for one would love to see it go, seeing as I live in the ghetto
>> that
>> is HFS+ and am constantly annoyed by it cluttering up my status
>> output
>> with spurious content.
>>
>> I understand that the reason it lives in the tree is precisely to
>> discover problems with such filesystems, but the problem is well and
>> truly discovered by now and I'd much rather see this kind of thing
>> tested from within the test suite rather than every time I do "git
>> status" or "git checkout".
>
> Probably you are not enoyed enough to just go and fix it.
No, that's not actually the case. In reality I was pleasantly
surprised when Junio commented that "perhaps we should remove" that
file; I had always gotten the impression from this list that such a
change would be unwelcome because it's easier to just blame the users
of bad filesystems for choosing those filesystems. I also remember a
comment from Linus to the effect that that file was kept in the tree
precisely _because_ it helped us discover such file systems.
Unfortunately I can't find that message right now but I think it was
about 6 months ago.
If the powers that be will accept a change that removes Märchen I'll
be more than happy to whip up a patch.
Wincent
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: Commit cce8d6fdb introduces file t/t5100/nul, git tree is now incompatible with Cygwin (and probably Windows)
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
1 sibling, 1 reply; 42+ messages in thread
From: Johannes Schindelin @ 2008-05-29 16:05 UTC (permalink / raw)
To: Wincent Colaiuta; +Cc: Junio C Hamano, Mark Levedahl, Git Mailing List
[-- Attachment #1: Type: TEXT/PLAIN, Size: 586 bytes --]
Hi,
On Thu, 29 May 2008, Wincent Colaiuta wrote:
> El 29/5/2008, a las 15:22, Johannes Schindelin escribió:
>
> >Probably you are not enoyed enough to just go and fix it.
>
> No, that's not actually the case. In reality I was pleasantly surprised
> when Junio commented that "perhaps we should remove" that file;
You misunderstood me. I was not talking about "fixing" it by removing the
file, and papering over the UTF-8 issue on HFS+.
I was talking about fixing it by handling UTF-8 in a way that is
compatible with (maybe stupid, but that cannot be helped) HFS+.
Ciao,
Dscho
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: Commit cce8d6fdb introduces file t/t5100/nul, git tree is now incompatible with Cygwin (and probably Windows)
2008-05-29 16:05 ` Johannes Schindelin
@ 2008-05-29 16:15 ` Wincent Colaiuta
0 siblings, 0 replies; 42+ messages in thread
From: Wincent Colaiuta @ 2008-05-29 16:15 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, Mark Levedahl, Git Mailing List
El 29/5/2008, a las 18:05, Johannes Schindelin escribió:
> Hi,
>
> On Thu, 29 May 2008, Wincent Colaiuta wrote:
>
>> El 29/5/2008, a las 15:22, Johannes Schindelin escribió:
>>
>>> Probably you are not enoyed enough to just go and fix it.
>>
>> No, that's not actually the case. In reality I was pleasantly
>> surprised
>> when Junio commented that "perhaps we should remove" that file;
>
> You misunderstood me. I was not talking about "fixing" it by
> removing the
> file, and papering over the UTF-8 issue on HFS+.
>
> I was talking about fixing it by handling UTF-8 in a way that is
> compatible with (maybe stupid, but that cannot be helped) HFS+.
Ah, ok. That's way beyond my skill set and a completely different
subject. I was responding to Junio's comment about _removing_ the file.
Wincent
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: Commit cce8d6fdb introduces file t/t5100/nul, git tree is now incompatible with Cygwin (and probably Windows)
2008-05-29 14:58 ` Wincent Colaiuta
2008-05-29 16:05 ` Johannes Schindelin
@ 2008-05-31 17:37 ` Steffen Prohaska
2008-05-31 18:28 ` [PATCH] gitweb: Remove gitweb/test/ directory Jakub Narebski
1 sibling, 1 reply; 42+ messages in thread
From: Steffen Prohaska @ 2008-05-31 17:37 UTC (permalink / raw)
To: Junio C Hamano, Wincent Colaiuta
Cc: Johannes Schindelin, Mark Levedahl, Git Mailing List
On May 29, 2008, at 4:58 PM, Wincent Colaiuta wrote:
> El 29/5/2008, a las 15:22, Johannes Schindelin escribió:
>>
>>
>> On Wed, 28 May 2008, Wincent Colaiuta wrote:
>>
>>> El 28/5/2008, a las 8:12, Junio C Hamano escribió:
>>>
>>>> Perhaps we should remove the infamous gitweb/test/Märchen file
>>>> while we
>>>> are at it? I do not think the file is ever used.
>>>
>>> I for one would love to see it go, seeing as I live in the ghetto
>>> that
>>> is HFS+ and am constantly annoyed by it cluttering up my status
>>> output
>>> with spurious content.
>>>
>>> I understand that the reason it lives in the tree is precisely to
>>> discover problems with such filesystems, but the problem is well and
>>> truly discovered by now and I'd much rather see this kind of thing
>>> tested from within the test suite rather than every time I do "git
>>> status" or "git checkout".
>>
>> Probably you are not enoyed enough to just go and fix it.
>
[...]
>
> If the powers that be will accept a change that removes Märchen I'll
> be more than happy to whip up a patch.
Unicode normalization is tested in t/t0050-filesystem.sh, which
reports on HFS+:
* still broken 8: rename (silent unicode normalization)
* still broken 9: merge (silent unicode normalization)
I believe there is no value in keeping gitweb/test/Märchen for the
reason of testing HFS+, so I vote for removing it, unless there
is another good reason for keeping it.
Steffen
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH] gitweb: Remove gitweb/test/ directory
2008-05-31 17:37 ` Steffen Prohaska
@ 2008-05-31 18:28 ` Jakub Narebski
2008-05-31 18:49 ` Wincent Colaiuta
2008-06-01 1:06 ` Junio C Hamano
0 siblings, 2 replies; 42+ messages in thread
From: Jakub Narebski @ 2008-05-31 18:28 UTC (permalink / raw)
To: git
Cc: Jakub Narebski, Steffen Prohaska, Junio C Hamano,
Wincent Colaiuta, Johannes Schindelin, Mark Levedahl,
Avery Pennarun, Daniel Barkalow
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=utf-8, Size: 2126 bytes --]
Testing if gitweb handles filenames with spaces, filenames with plus
sign ('+') which encodes spaces in CGI parameters (in URLs), and
filenames with Unicode characters should be handled by gitweb tests.
Those files are remainder of the time when gitweb was project on its
own, not a part of git (with its testsuite).
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
On Sat, 31 May 2008 19:37:48 +0200, Steffen Prohaska wrote:
> On May 29, 2008, at 4:58 PM, Wincent Colaiuta wrote:
>
[...]
>> If the powers that be will accept a change that removes Märchen I'll
>> be more than happy to whip up a patch.
>
> Unicode normalization is tested in t/t0050-filesystem.sh, which
> reports on HFS+:
>
> * still broken 8: rename (silent unicode normalization)
> * still broken 9: merge (silent unicode normalization)
>
> I believe there is no value in keeping gitweb/test/Märchen for the
> reason of testing HFS+, so I vote for removing it, unless there
> is another good reason for keeping it.
So here it is, the patch to remove offending file; well: the whole
gitweb/test/ directory.
"gitweb/test/M\303\244rchen" | 2 --
gitweb/test/file with spaces | 4 ----
gitweb/test/file+plus+sign | 6 ------
3 files changed, 0 insertions(+), 12 deletions(-)
delete mode 100644 gitweb/test/Märchen
delete mode 100644 gitweb/test/file with spaces
delete mode 100644 gitweb/test/file+plus+sign
diff --git "a/gitweb/test/M\303\244rchen" "b/gitweb/test/M\303\244rchen"
deleted file mode 100644
index 8f7a1d3..0000000
--- "a/gitweb/test/M\303\244rchen"
+++ /dev/null
@@ -1,2 +0,0 @@
-Märchen
-Märchen
diff --git a/gitweb/test/file with spaces b/gitweb/test/file with spaces
deleted file mode 100644
index f108543..0000000
--- a/gitweb/test/file with spaces
+++ /dev/null
@@ -1,4 +0,0 @@
-This
-filename
-contains
-spaces.
diff --git a/gitweb/test/file+plus+sign b/gitweb/test/file+plus+sign
deleted file mode 100644
index fd05278..0000000
--- a/gitweb/test/file+plus+sign
+++ /dev/null
@@ -1,6 +0,0 @@
-This
-filename
-contains
-+
-plus
-chars.
^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [PATCH] gitweb: Remove gitweb/test/ directory
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 1:06 ` Junio C Hamano
1 sibling, 1 reply; 42+ messages in thread
From: Wincent Colaiuta @ 2008-05-31 18:49 UTC (permalink / raw)
To: Jakub Narebski
Cc: git, Steffen Prohaska, Junio C Hamano, Johannes Schindelin,
Mark Levedahl, Avery Pennarun, Daniel Barkalow
El 31/5/2008, a las 20:28, Jakub Narebski escribió:
> Testing if gitweb handles filenames with spaces, filenames with plus
> sign ('+') which encodes spaces in CGI parameters (in URLs), and
> filenames with Unicode characters should be handled by gitweb tests.
>
> Those files are remainder of the time when gitweb was project on its
> own, not a part of git (with its testsuite).
>
> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
Ack.
Wincent
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] gitweb: Remove gitweb/test/ directory
2008-05-31 18:49 ` Wincent Colaiuta
@ 2008-05-31 23:19 ` Johannes Schindelin
2008-06-01 0:19 ` Jakub Narebski
2008-06-01 19:07 ` Wincent Colaiuta
0 siblings, 2 replies; 42+ messages in thread
From: Johannes Schindelin @ 2008-05-31 23:19 UTC (permalink / raw)
To: Wincent Colaiuta
Cc: Jakub Narebski, git, Steffen Prohaska, Junio C Hamano,
Mark Levedahl, Avery Pennarun, Daniel Barkalow
[-- Attachment #1: Type: TEXT/PLAIN, Size: 644 bytes --]
Hi,
On Sat, 31 May 2008, Wincent Colaiuta wrote:
> El 31/5/2008, a las 20:28, Jakub Narebski escribió:
>
> > Testing if gitweb handles filenames with spaces, filenames with plus
> > sign ('+') which encodes spaces in CGI parameters (in URLs), and
> > filenames with Unicode characters should be handled by gitweb tests.
> >
> > Those files are remainder of the time when gitweb was project on its
> > own, not a part of git (with its testsuite).
> >
> > Signed-off-by: Jakub Narebski <jnareb@gmail.com>
>
> Ack.
And I thought "Ack" was reserved for the people who are considered the
primary authors of the patched code...
Ciao,
Dscho
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] gitweb: Remove gitweb/test/ directory
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
1 sibling, 1 reply; 42+ messages in thread
From: Jakub Narebski @ 2008-06-01 0:19 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Wincent Colaiuta, git, Steffen Prohaska, Junio C Hamano,
Mark Levedahl, Avery Pennarun, Daniel Barkalow, Kay Sievers
On Sun, 1 Jan 2008, Johannes Schindelin wrote:
> On Sat, 31 May 2008, Wincent Colaiuta wrote:
>> El 31/5/2008, a las 20:28, Jakub Narebski escribió:
>>
>>> Testing if gitweb handles filenames with spaces, filenames with plus
>>> sign ('+') which encodes spaces in CGI parameters (in URLs), and
>>> filenames with Unicode characters should be handled by gitweb tests.
>>>
>>> Those files are remainder of the time when gitweb was project on its
>>> own, not a part of git (with its testsuite).
>>>
>>> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
>>
>> Ack.
>
> And I thought "Ack" was reserved for the people who are considered the
> primary authors of the patched code...
Unfortunately, as far as I know, primary and only author of those
lines of code, maintainer of gitweb when it was separate project,
Kay Sievers, is no longer active in git development.
--
Jakub Narebski
Poland
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] gitweb: Remove gitweb/test/ directory
2008-05-31 18:28 ` [PATCH] gitweb: Remove gitweb/test/ directory Jakub Narebski
2008-05-31 18:49 ` Wincent Colaiuta
@ 2008-06-01 1:06 ` Junio C Hamano
2008-06-01 1:59 ` Jakub Narebski
1 sibling, 1 reply; 42+ messages in thread
From: Junio C Hamano @ 2008-06-01 1:06 UTC (permalink / raw)
To: Jakub Narebski, Kay Sievers
Cc: git, Steffen Prohaska, Wincent Colaiuta, Johannes Schindelin,
Mark Levedahl, Avery Pennarun, Daniel Barkalow
Jakub Narebski <jnareb@gmail.com> writes:
> Testing if gitweb handles filenames with spaces, filenames with plus
> sign ('+') which encodes spaces in CGI parameters (in URLs), and
> filenames with Unicode characters should be handled by gitweb tests.
>
> Those files are remainder of the time when gitweb was project on its
> own, not a part of git (with its testsuite).
>
> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
> ---
> ...
> "gitweb/test/M\303\244rchen" | 2 --
> gitweb/test/file with spaces | 4 ----
> gitweb/test/file+plus+sign | 6 ------
> 3 files changed, 0 insertions(+), 12 deletions(-)
> delete mode 100644 gitweb/test/Märchen
> delete mode 100644 gitweb/test/file with spaces
> delete mode 100644 gitweb/test/file+plus+sign
I do not think Kay minds, but it would not hurt to give him a courtesy
copy of this before finally applying such a patch.
-jc
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] gitweb: Remove gitweb/test/ directory
2008-06-01 1:06 ` Junio C Hamano
@ 2008-06-01 1:59 ` Jakub Narebski
0 siblings, 0 replies; 42+ messages in thread
From: Jakub Narebski @ 2008-06-01 1:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Kay Sievers, git
On Sat, 1 Jun 2008, Junio C Hamano wrote:
> I do not think Kay minds, but it would not hurt to give him a courtesy
> copy of this before finally applying such a patch.
Done.
--
Jakub Narebski
Poland
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] gitweb: Remove gitweb/test/ directory
2008-06-01 0:19 ` Jakub Narebski
@ 2008-06-01 9:42 ` Kay Sievers
0 siblings, 0 replies; 42+ messages in thread
From: Kay Sievers @ 2008-06-01 9:42 UTC (permalink / raw)
To: Jakub Narebski
Cc: Johannes Schindelin, Wincent Colaiuta, git, Steffen Prohaska,
Junio C Hamano, Mark Levedahl, Avery Pennarun, Daniel Barkalow
On Sun, 2008-06-01 at 02:19 +0200, Jakub Narebski wrote:
> On Sun, 1 Jan 2008, Johannes Schindelin wrote:
> > On Sat, 31 May 2008, Wincent Colaiuta wrote:
> >> El 31/5/2008, a las 20:28, Jakub Narebski escribió:
> >>
> >>> Testing if gitweb handles filenames with spaces, filenames with plus
> >>> sign ('+') which encodes spaces in CGI parameters (in URLs), and
> >>> filenames with Unicode characters should be handled by gitweb tests.
> >>>
> >>> Those files are remainder of the time when gitweb was project on its
> >>> own, not a part of git (with its testsuite).
> >>>
> >>> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
> >>
> >> Ack.
> >
> > And I thought "Ack" was reserved for the people who are considered the
> > primary authors of the patched code...
>
> Unfortunately, as far as I know, primary and only author of those
> lines of code, maintainer of gitweb when it was separate project,
> Kay Sievers, is no longer active in git development.
Sure, feel free to do whatever makes sense, there is no reason to get my
ACK, as I'm not actively working on it anymore. I'm glad, you guys take
care of gitweb these days, and it has improved a lot since then.
Thanks,
Kay
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] gitweb: Remove gitweb/test/ directory
2008-05-31 23:19 ` Johannes Schindelin
2008-06-01 0:19 ` Jakub Narebski
@ 2008-06-01 19:07 ` Wincent Colaiuta
1 sibling, 0 replies; 42+ messages in thread
From: Wincent Colaiuta @ 2008-06-01 19:07 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Jakub Narebski, git, Steffen Prohaska, Junio C Hamano,
Mark Levedahl, Avery Pennarun, Daniel Barkalow
El 1/6/2008, a las 1:19, Johannes Schindelin escribió:
> Hi,
>
> On Sat, 31 May 2008, Wincent Colaiuta wrote:
>
>> El 31/5/2008, a las 20:28, Jakub Narebski escribió:
>>
>>> Testing if gitweb handles filenames with spaces, filenames with plus
>>> sign ('+') which encodes spaces in CGI parameters (in URLs), and
>>> filenames with Unicode characters should be handled by gitweb tests.
>>>
>>> Those files are remainder of the time when gitweb was project on its
>>> own, not a part of git (with its testsuite).
>>>
>>> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
>>
>> Ack.
>
> And I thought "Ack" was reserved for the people who are considered the
> primary authors of the patched code...
I had no idea. Thanks for letting me know.
Wincent
^ permalink raw reply [flat|nested] 42+ messages in thread
end of thread, other threads:[~2008-06-01 19:08 UTC | newest]
Thread overview: 42+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
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
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).