Re: [PATCH v2 2/9] setup_git_directory(): use is_dir_sep() helper
On Fri, Mar 03, 2017 at 03:04:07AM +0100, Johannes Schindelin wrote: > It is okay in practice to test for forward slashes in the output of > getcwd(), because we go out of our way to convert backslashes to forward > slashes in getcwd()'s output on Windows. > > Still, the correct way to test for a dir separator is by using the > helper function we introduced for that very purpose. It also serves as a > good documentation what the code tries to do (not "how"). Makes sense, but... > @@ -910,7 +910,8 @@ static const char *setup_git_directory_gently_1(int > *nongit_ok) > return setup_bare_git_dir(&cwd, offset, nongit_ok); > > offset_parent = offset; > - while (--offset_parent > ceil_offset && cwd.buf[offset_parent] > != '/'); > + while (--offset_parent > ceil_offset && > +!is_dir_sep(dir->buf[offset_parent])); What is "dir"? I'm guessing this patch got reordered and it should stay as cwd.buf? -Peff
Re: [PATCH v2 2/9] setup_git_directory(): use is_dir_sep() helper
Hi Peff, On Thu, 2 Mar 2017, Jeff King wrote: > On Fri, Mar 03, 2017 at 03:04:07AM +0100, Johannes Schindelin wrote: > > > It is okay in practice to test for forward slashes in the output of > > getcwd(), because we go out of our way to convert backslashes to > > forward slashes in getcwd()'s output on Windows. > > > > Still, the correct way to test for a dir separator is by using the > > helper function we introduced for that very purpose. It also serves as > > a good documentation what the code tries to do (not "how"). > > Makes sense, but... > > > @@ -910,7 +910,8 @@ static const char *setup_git_directory_gently_1(int > > *nongit_ok) > > return setup_bare_git_dir(&cwd, offset, nongit_ok); > > > > offset_parent = offset; > > - while (--offset_parent > ceil_offset && cwd.buf[offset_parent] > > != '/'); > > + while (--offset_parent > ceil_offset && > > + !is_dir_sep(dir->buf[offset_parent])); > > What is "dir"? I'm guessing this patch got reordered and it should stay > as cwd.buf? Oh drats. Usually I do a final `git rebase -x "make test" upstream/master` run before submitting, but I was really, really tired by the end of that stretch. Thanks for being thorough (and I fixed it, of course), Dscho
Re: [PATCH v2 2/9] setup_git_directory(): use is_dir_sep() helper
On Fri, Mar 03, 2017 at 12:16:31PM +0100, Johannes Schindelin wrote: > > What is "dir"? I'm guessing this patch got reordered and it should stay > > as cwd.buf? > > Oh drats. Usually I do a final `git rebase -x "make test" upstream/master` > run before submitting, but I was really, really tired by the end of that > stretch. I usually do the same, and have done the "too tired" thing, too, only to have it bite me. That's why I so readily recognized the problem. :) I've recently switched to using Michael's "git test" program[1], which caches the test results for each tree in a git-note. That makes the final "rebase -x" a lot less painful if you've left the early commits alone. The python dependency might be a blocker for you, but I suspect the caching parts would be easy to hack together with shell. -Peff [1] https://github.com/mhagger/git-test
Re: [PATCH v2 2/9] setup_git_directory(): use is_dir_sep() helper
Hi Peff, On Fri, 3 Mar 2017, Jeff King wrote: > On Fri, Mar 03, 2017 at 12:16:31PM +0100, Johannes Schindelin wrote: > > > > What is "dir"? I'm guessing this patch got reordered and it should > > > stay as cwd.buf? > > > > Oh drats. Usually I do a final `git rebase -x "make test" > > upstream/master` run before submitting, but I was really, really tired > > by the end of that stretch. > > I usually do the same, and have done the "too tired" thing, too, only to > have it bite me. That's why I so readily recognized the problem. :) :-) > I've recently switched to using Michael's "git test" program[1], which > caches the test results for each tree in a git-note. That makes the > final "rebase -x" a lot less painful if you've left the early commits > alone. Good point. I meant to have a look, but got really overwhelmed with other things. > The python dependency might be a blocker for you, but I suspect the > caching parts would be easy to hack together with shell. No, personally I have no problem with Python. If you asked me to include it in Git for Windows' installer, increasing the size noticably, that would be a totally different matter, of course. Ciao, Dscho
