Hi, On Wed, 14 Nov 2007, Junio C Hamano wrote: > Johannes Sixt <johannes.sixt@xxxxxxxxxx> writes: > > > This is a series of smallish, unrelated changes that were necessary > > for the MinGW port. > > I was _VERY_ afraid of reviewing this series. Why? Because we get closer to MinGW integration into git.git for real? ;-) > > [PATCH 05/11] Use is_absolute_path() in sha1_file.c. > > [PATCH 06/11] Move #include <sys/select.h> and <sys/ioctl.h> to > > git-compat-util.h. > > > > These two are certainly undisputed. > > Except on esoteric/broken systems there might be some dependency > on the order the system include files are included, so 06/11 > needs some testing. But it is a change in the right direction. The safe thing, of course, would be to move them at the end of the include list in git-compat-util.h, since they are now included after cache.h, (which includes git-compat-util.h and only strbuf.h, the sha1 header and zlib.h). This way it should be really certainly undisputed. > > [PATCH 08/11] Close files opened by lock_file() before unlinking. > > > > This one was authored by Dscho. It is a definite MUST on Windows. > > This was something we've talked about doing a few times on the > list but did not. It is good that this saw some testing in the > field, as it is easy to get wrong while moving the call site of > close(2) around. Note that we are not strictly _moving_ it around. In fact, we are _adding_ more close() calls... And even ignoring the errors when close() was already called, so it feels a tad hacky. But it does the job. > > [PATCH 09/11] Allow a relative builtin template directory. > > [PATCH 10/11] Introduce git_etc_gitconfig() that encapsulates access > > of ETC_GITCONFIG. > > [PATCH 11/11] Allow ETC_GITCONFIG to be a relative path. > > > > These need probably some discussion. They avoid that $(prefix) is > > hardcoded and so allows that an arbitrary installation directory. > > I had to worry a bit about bootstrapping issues in 11/11. We > need to ensure that anybody who wants to read the configuration > data first does setup_path() because git_exec_path() reads from > argv_exec_path and setup_path() is what assigns to that > variable. Just to be safe in the future, we could check for that condition (by introducing a static variable setup_path_called) and die() should anybody introduce a code path where the order of calls is not maintained. > But other than that and 08/11, I found everything is trivially correct > and it was a pleasant read. Me, too. Ciao, Dscho - To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
