Hi Junio, On Fri, 29 Mar 2024, Junio C Hamano wrote: > Jeff King <peff@xxxxxxxx> writes: > > > On Thu, Mar 28, 2024 at 07:18:52PM -0700, Junio C Hamano wrote: > > > >> Junio C Hamano <gitster@xxxxxxxxx> writes: > >> > >> > +test_expect_success 'parsing a patch with no-contents and a funny pathname' ' > >> > git reset --hard && > >> > + empty_blob=$(test_oid empty_blob) && > >> > + echo "$empty_blob" >expect && > >> > > >> > + git update-index --add --cacheinfo "100644,$empty_blob,funny /empty" && > >> > >> It seems that on Windows, this step fails with "funny /empty" as > >> "invalid path". > >> > >> https://github.com/git/git/actions/runs/8475098601/job/23222724707#step:6:244 > > > > Ah, sorry, I didn't actually try my suggestion on Windows. I guess we > > are falling afoul of verify_path(), which calls is_valid_path(). That is > > a noop on most platforms, but is_valid_win32_path() has: > > > > switch (c) { > > case '\0': > > case '/': case '\\': > > /* cannot end in ` ` or `.`, except for `.` and `..` */ > > if (preceding_space_or_period && > > (i != periods || periods > 2)) > > return 0; > > Yes, and no need to say sorry. I was also surprised, as I thought > that the non working tree operations ought to be platform > independenty, with this. > > > It's interesting that there is no way to override this check via > > update-index, etc (like we have "--literally" for hash-object when you > > want to do something stupid). I think it would be sufficient to make > > things work everywhere for this test case. On the other hand, if you > > have to resort to "please add this index entry which is broken on my > > filesystem" to run the test, maybe that is a good sign it should just be > > skipped on that platform. ;) > > This is a far-away tangent but we may want to think about "the core > of Git made into a library that works only with the objects in the > object-store and does not deal with working trees". To work with > the objects, we would probably need something like the index that is > used in the original sense of the word (a database you consult with > a pathname as a key and obtain the object name with mode bits and a > stage number), etc. Elijah's merge-tree may fit well within the > scheme. > > There is no place like the above code in such a world. The > restriction must exist somewhere to protect the users that use on a > limited system, but should come in a layer far above that "core > library". Indeed, it should have been at another layer, but alas, I could not find a _better_ layer back when. BTW it _is_ possible to override this check. This invocation works: $ git -c core.protectNTFS=false update-index --add --cacheinfo "100644,$empty_blob,funny /empty" It has been on my radar for a long time that in particular with sparse checkouts, this check is overzealous. I would have loved to work on it, and once I find a position where I am funded to work meaningfully on Git for Windows again, I will. > Anyway, I think you convinced me in the other response that we > should just use an existing prerequisite, perhaps FUNNYNAMES. The > idea is to exclude platforms that are known to break with the test > without any hope of fix. Because they are incapable of taking their > users into the problematic state being tested in the first place, > this is not making things any worse. That was indeed the correct thing to do, as far as I am concerned. Thank you for fixing this, and sorry that I was not able to contribute meaningfully to the fix. Ciao, Johannes
