* [PATCH] mingw: introduce the 'core.hideDotFiles' setting @ 2016-05-04 14:40 Johannes Schindelin 2016-05-04 16:18 ` Ramsay Jones ` (2 more replies) 0 siblings, 3 replies; 27+ messages in thread From: Johannes Schindelin @ 2016-05-04 14:40 UTC (permalink / raw) To: Junio C Hamano; +Cc: Erik Faye-Lund, Pat Thoyts, git From: Erik Faye-Lund <kusmabite@googlemail.com> On Unix (and Linux) it is common that files and directories whose names start with a dot are not shown by default. This convention is used by Git: the .git/ directory should be left alone by regular users, and only accessed through Git itself. On Windows, no such convention exists. Instead, there is an explicit flag to mark files or directories as hidden. In the early days, Git for Windows did not mark the .git/ directory (or for that matter, any file or directory whose name starts with a dot) hidden. This lead to quite a bit of confusion, and even loss of data. Consequently, Git for Windows introduced the core.hideDotFiles setting, with three possible values: true, false, and dotGitOnly, defaulting to marking only the .git/ directory as hidden. The rationale: users do not need to access .git/ directly, and indeed (as was demonstrated) should not really see that directory, either. However, not all dot files should be hidden, as e.g. Eclipse does not show them (and the user would therefore be unable to add, say, a .gitattributes file). In over five years since the last attempt to bring this patch into core Git, this patch has served Git for Windows' users well: no single report indicated problems with the hidden .git/ directory, and the stream of problems caused by the previously non-hidden .git/ directory simply stopped. Initial-Test-By: Pat Thoyts <patthoyts@users.sourceforge.net> Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- Let's try this again (I will not point you to the previous submission, out of personal embarrassment). This patch has served us so well in the Git for Windows project that there is little sense in hiding it from core Git. Documentation/config.txt | 6 ++++++ builtin/init-db.c | 1 + cache.h | 7 +++++++ compat/mingw.c | 38 ++++++++++++++++++++++++++++++++++++++ config.c | 8 ++++++++ environment.c | 1 + git-compat-util.h | 4 ++++ t/t0001-init.sh | 30 ++++++++++++++++++++++++++++++ 8 files changed, 95 insertions(+) diff --git a/Documentation/config.txt b/Documentation/config.txt index 42d2b50..a9f599d 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -269,6 +269,12 @@ See linkgit:git-update-index[1]. + The default is true (when core.filemode is not specified in the config file). +core.hideDotFiles:: + (Windows-only) If true (which is the default), mark newly-created + directories and files whose name starts with a dot as hidden. + If 'dotGitOnly', only the .git/ directory is hidden, but no other + files starting with a dot. + core.ignoreCase:: If true, this option enables various workarounds to enable Git to work better on filesystems that are not case sensitive, diff --git a/builtin/init-db.c b/builtin/init-db.c index b2d8d40..c4269ac 100644 --- a/builtin/init-db.c +++ b/builtin/init-db.c @@ -370,6 +370,7 @@ int init_db(const char *template_dir, unsigned int flags) check_repository_format(); reinit = create_default_files(template_dir); + mark_as_git_dir(get_git_dir()); create_object_directory(); diff --git a/cache.h b/cache.h index fd728f0..a8e9a62 100644 --- a/cache.h +++ b/cache.h @@ -700,6 +700,13 @@ extern int ref_paranoia; extern char comment_line_char; extern int auto_comment_line_char; +enum hide_dotfiles_type { + HIDE_DOTFILES_FALSE = 0, + HIDE_DOTFILES_TRUE, + HIDE_DOTFILES_DOTGITONLY, +}; +extern enum hide_dotfiles_type hide_dotfiles; + enum branch_track { BRANCH_TRACK_UNSPECIFIED = -1, BRANCH_TRACK_NEVER = 0, diff --git a/compat/mingw.c b/compat/mingw.c index 0413d5c..8b8b01c 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -286,6 +286,33 @@ int mingw_rmdir(const char *pathname) return ret; } +static inline int needs_hiding(const char *path) +{ + return hide_dotfiles == HIDE_DOTFILES_TRUE && + starts_with(basename((char*)path), "."); +} + +static int make_hidden(const wchar_t *path) +{ + DWORD attribs = GetFileAttributesW(path); + if (SetFileAttributesW(path, FILE_ATTRIBUTE_HIDDEN | attribs)) + return 0; + errno = err_win_to_posix(GetLastError()); + return -1; +} + +void mingw_mark_as_git_dir(const char *dir) +{ + wchar_t wdir[MAX_PATH]; + if (hide_dotfiles != HIDE_DOTFILES_FALSE && !is_bare_repository()) + if (xutftowcs_path(wdir, dir) < 0 || make_hidden(wdir)) + warning("Failed to make '%s' hidden", dir); + git_config_set("core.hideDotFiles", + hide_dotfiles == HIDE_DOTFILES_FALSE ? "false" : + (hide_dotfiles == HIDE_DOTFILES_DOTGITONLY ? + "dotGitOnly" : "true")); +} + int mingw_mkdir(const char *path, int mode) { int ret; @@ -293,6 +320,8 @@ int mingw_mkdir(const char *path, int mode) if (xutftowcs_path(wpath, path) < 0) return -1; ret = _wmkdir(wpath); + if (!ret && needs_hiding(path)) + return make_hidden(wpath); return ret; } @@ -319,6 +348,9 @@ int mingw_open (const char *filename, int oflags, ...) if (attrs != INVALID_FILE_ATTRIBUTES && (attrs & FILE_ATTRIBUTE_DIRECTORY)) errno = EISDIR; } + if ((oflags & O_CREAT) && fd >= 0 && needs_hiding(filename) && + make_hidden(wfilename)) + warning("Could not mark '%s' as hidden.", filename); return fd; } @@ -350,6 +382,7 @@ int mingw_fgetc(FILE *stream) #undef fopen FILE *mingw_fopen (const char *filename, const char *otype) { + int hide = needs_hiding(filename) && access(filename, F_OK); FILE *file; wchar_t wfilename[MAX_PATH], wotype[4]; if (filename && !strcmp(filename, "/dev/null")) @@ -358,11 +391,14 @@ FILE *mingw_fopen (const char *filename, const char *otype) xutftowcs(wotype, otype, ARRAY_SIZE(wotype)) < 0) return NULL; file = _wfopen(wfilename, wotype); + if (file && hide && make_hidden(wfilename)) + warning("Could not mark '%s' as hidden.", filename); return file; } FILE *mingw_freopen (const char *filename, const char *otype, FILE *stream) { + int hide = needs_hiding(filename) && access(filename, F_OK); FILE *file; wchar_t wfilename[MAX_PATH], wotype[4]; if (filename && !strcmp(filename, "/dev/null")) @@ -371,6 +407,8 @@ FILE *mingw_freopen (const char *filename, const char *otype, FILE *stream) xutftowcs(wotype, otype, ARRAY_SIZE(wotype)) < 0) return NULL; file = _wfreopen(wfilename, wotype, stream); + if (file && hide && make_hidden(wfilename)) + warning("Could not mark '%s' as hidden.", filename); return file; } diff --git a/config.c b/config.c index 10b5c95..1b44d46 100644 --- a/config.c +++ b/config.c @@ -912,6 +912,14 @@ static int git_default_core_config(const char *var, const char *value) return 0; } + if (!strcmp(var, "core.hidedotfiles")) { + if (value && !strcasecmp(value, "dotgitonly")) + hide_dotfiles = HIDE_DOTFILES_DOTGITONLY; + else + hide_dotfiles = git_config_bool(var, value); + return 0; + } + /* Add other config variables here and to Documentation/config.txt. */ return 0; } diff --git a/environment.c b/environment.c index 57acb2f..96160a7 100644 --- a/environment.c +++ b/environment.c @@ -63,6 +63,7 @@ int core_apply_sparse_checkout; int merge_log_config = -1; int precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */ unsigned long pack_size_limit_cfg; +enum hide_dotfiles_type hide_dotfiles = HIDE_DOTFILES_DOTGITONLY; #ifndef PROTECT_HFS_DEFAULT #define PROTECT_HFS_DEFAULT 0 diff --git a/git-compat-util.h b/git-compat-util.h index 1f8b5f3..ea007e4 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -1042,4 +1042,8 @@ struct tm *git_gmtime_r(const time_t *, struct tm *); #define getc_unlocked(fh) getc(fh) #endif +#ifndef mark_as_git_dir +#define mark_as_git_dir(x) /* noop */ +#endif + #endif diff --git a/t/t0001-init.sh b/t/t0001-init.sh index a5b9e7a..a6fdd5e 100755 --- a/t/t0001-init.sh +++ b/t/t0001-init.sh @@ -354,4 +354,34 @@ test_expect_success SYMLINKS 're-init to move gitdir symlink' ' test_path_is_dir realgitdir/refs ' +# Tests for the hidden file attribute on windows +is_hidden () { + # Use the output of `attrib`, ignore the absolute path + case "$(attrib "$1")" in *H*?:*) return 0;; esac + return 1 +} + +test_expect_success MINGW '.git hidden' ' + rm -rf newdir && + ( + unset GIT_DIR GIT_WORK_TREE + mkdir newdir && + cd newdir && + git init && + is_hidden .git + ) && + check_config newdir/.git false unset +' + +test_expect_success MINGW 'bare git dir not hidden' ' + rm -rf newdir && + ( + unset GIT_DIR GIT_WORK_TREE GIT_CONFIG + mkdir newdir && + cd newdir && + git --bare init + ) && + ! is_hidden newdir +' + test_done -- 2.8.1.306.gff998f2 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH] mingw: introduce the 'core.hideDotFiles' setting 2016-05-04 14:40 [PATCH] mingw: introduce the 'core.hideDotFiles' setting Johannes Schindelin @ 2016-05-04 16:18 ` Ramsay Jones 2016-05-06 12:06 ` Johannes Schindelin 2016-05-04 19:06 ` Junio C Hamano 2016-05-07 6:44 ` [PATCH v2 0/2] Support marking .git/ (or all files) as hidden on Windows Johannes Schindelin 2 siblings, 1 reply; 27+ messages in thread From: Ramsay Jones @ 2016-05-04 16:18 UTC (permalink / raw) To: Johannes Schindelin, Junio C Hamano; +Cc: Erik Faye-Lund, Pat Thoyts, git On 04/05/16 15:40, Johannes Schindelin wrote: > From: Erik Faye-Lund <kusmabite@googlemail.com> > > On Unix (and Linux) it is common that files and directories whose names > start with a dot are not shown by default. This convention is used by Git: > the .git/ directory should be left alone by regular users, and only > accessed through Git itself. > > On Windows, no such convention exists. Instead, there is an explicit flag > to mark files or directories as hidden. > > In the early days, Git for Windows did not mark the .git/ directory (or > for that matter, any file or directory whose name starts with a dot) > hidden. This lead to quite a bit of confusion, and even loss of data. > > Consequently, Git for Windows introduced the core.hideDotFiles setting, > with three possible values: true, false, and dotGitOnly, defaulting to > marking only the .git/ directory as hidden. > > The rationale: users do not need to access .git/ directly, and indeed (as > was demonstrated) should not really see that directory, either. However, > not all dot files should be hidden, as e.g. Eclipse does not show them > (and the user would therefore be unable to add, say, a .gitattributes > file). > > In over five years since the last attempt to bring this patch into core > Git, this patch has served Git for Windows' users well: no single report > indicated problems with the hidden .git/ directory, and the stream of > problems caused by the previously non-hidden .git/ directory simply > stopped. > > Initial-Test-By: Pat Thoyts <patthoyts@users.sourceforge.net> > Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> > --- > > Let's try this again (I will not point you to the previous > submission, out of personal embarrassment). > > This patch has served us so well in the Git for Windows project > that there is little sense in hiding it from core Git. > > Documentation/config.txt | 6 ++++++ > builtin/init-db.c | 1 + > cache.h | 7 +++++++ > compat/mingw.c | 38 ++++++++++++++++++++++++++++++++++++++ > config.c | 8 ++++++++ > environment.c | 1 + > git-compat-util.h | 4 ++++ > t/t0001-init.sh | 30 ++++++++++++++++++++++++++++++ > 8 files changed, 95 insertions(+) > > diff --git a/Documentation/config.txt b/Documentation/config.txt > index 42d2b50..a9f599d 100644 > --- a/Documentation/config.txt > +++ b/Documentation/config.txt > @@ -269,6 +269,12 @@ See linkgit:git-update-index[1]. > + > The default is true (when core.filemode is not specified in the config file). > > +core.hideDotFiles:: > + (Windows-only) If true (which is the default), mark newly-created The patch (if I'm reading it correctly) and the commit message indicate that the default is 'dotGitOnly'. > + directories and files whose name starts with a dot as hidden. > + If 'dotGitOnly', only the .git/ directory is hidden, but no other > + files starting with a dot. > + ATB, Ramsay Jones ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] mingw: introduce the 'core.hideDotFiles' setting 2016-05-04 16:18 ` Ramsay Jones @ 2016-05-06 12:06 ` Johannes Schindelin 0 siblings, 0 replies; 27+ messages in thread From: Johannes Schindelin @ 2016-05-06 12:06 UTC (permalink / raw) To: Ramsay Jones; +Cc: Junio C Hamano, Erik Faye-Lund, Pat Thoyts, git Hi Ramsay, On Wed, 4 May 2016, Ramsay Jones wrote: > On 04/05/16 15:40, Johannes Schindelin wrote: > > > > diff --git a/Documentation/config.txt b/Documentation/config.txt > > index 42d2b50..a9f599d 100644 > > --- a/Documentation/config.txt > > +++ b/Documentation/config.txt > > @@ -269,6 +269,12 @@ See linkgit:git-update-index[1]. > > + > > The default is true (when core.filemode is not specified in the config file). > > > > +core.hideDotFiles:: > > + (Windows-only) If true (which is the default), mark newly-created > > The patch (if I'm reading it correctly) and the commit message indicate that > the default is 'dotGitOnly'. Good catch! The original version did default to "true", but we introduced the "dotGitOnly" option later because it was too cumbersome to access things like .mailmap and .gitmodules when they were hidden, and changed the default accordingly. Missing the documentation update. Fixed in the upcoming v2, Dscho > > > + directories and files whose name starts with a dot as hidden. > > + If 'dotGitOnly', only the .git/ directory is hidden, but no other > > + files starting with a dot. > > + > > ATB, > Ramsay Jones > > > -- > To unsubscribe from this list: send the line "unsubscribe git" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] mingw: introduce the 'core.hideDotFiles' setting 2016-05-04 14:40 [PATCH] mingw: introduce the 'core.hideDotFiles' setting Johannes Schindelin 2016-05-04 16:18 ` Ramsay Jones @ 2016-05-04 19:06 ` Junio C Hamano 2016-05-06 15:19 ` Johannes Schindelin 2016-05-07 6:44 ` [PATCH v2 0/2] Support marking .git/ (or all files) as hidden on Windows Johannes Schindelin 2 siblings, 1 reply; 27+ messages in thread From: Junio C Hamano @ 2016-05-04 19:06 UTC (permalink / raw) To: Johannes Schindelin; +Cc: Erik Faye-Lund, Pat Thoyts, git Johannes Schindelin <johannes.schindelin@gmx.de> writes: > diff --git a/builtin/init-db.c b/builtin/init-db.c > index b2d8d40..c4269ac 100644 > --- a/builtin/init-db.c > +++ b/builtin/init-db.c > @@ -370,6 +370,7 @@ int init_db(const char *template_dir, unsigned int flags) > check_repository_format(); > > reinit = create_default_files(template_dir); > + mark_as_git_dir(get_git_dir()); > > create_object_directory(); > I agree with the goal of the change, but I am having a hard time justifying this addition. Primarily because I do not understand the need for this. In order to be prepared to handle HIDE_DOTFILES_TRUE case, mingw_mkdir() needs to be taught about needs_hiding() and make_hidden() already. Why isn't it sufficient to teach needs_hiding() to check with !strcmp(basename(path, ".git")) under HIDE_DOTFILES_DOTGITONLY? I do not understand why core.hideDotFiles needs to be explicitly copied to the config file in the resulting repository, either. Once you created a repository, Git won't unhide the hidden .git of that reposiotry, so the idea must be to propagate the setting to a new repository that will further be created, but wouldn't that get the "please hide" preference from the same place as we have got the preference to hide .git when the current repository was created anyway? ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] mingw: introduce the 'core.hideDotFiles' setting 2016-05-04 19:06 ` Junio C Hamano @ 2016-05-06 15:19 ` Johannes Schindelin 2016-05-06 16:34 ` Junio C Hamano 0 siblings, 1 reply; 27+ messages in thread From: Johannes Schindelin @ 2016-05-06 15:19 UTC (permalink / raw) To: Junio C Hamano; +Cc: Erik Faye-Lund, Pat Thoyts, git Hi Junio, On Wed, 4 May 2016, Junio C Hamano wrote: > Johannes Schindelin <johannes.schindelin@gmx.de> writes: > > > diff --git a/builtin/init-db.c b/builtin/init-db.c > > index b2d8d40..c4269ac 100644 > > --- a/builtin/init-db.c > > +++ b/builtin/init-db.c > > @@ -370,6 +370,7 @@ int init_db(const char *template_dir, unsigned int flags) > > check_repository_format(); > > > > reinit = create_default_files(template_dir); > > + mark_as_git_dir(get_git_dir()); > > > > create_object_directory(); > > > > I agree with the goal of the change, but I am having a hard time > justifying this addition. Primarily because I do not understand the > need for this. > > In order to be prepared to handle HIDE_DOTFILES_TRUE case, > mingw_mkdir() needs to be taught about needs_hiding() and > make_hidden() already. Why isn't it sufficient to teach > needs_hiding() to check with !strcmp(basename(path, ".git")) > under HIDE_DOTFILES_DOTGITONLY? The reason was that I wanted to avoid to compare a name unnecessarily when I already had a code path that knew perfectly well that a given directory is the .git/ directory. I made the change. It was more painful than I expected, as two bugs were uncovered, both introduced after the original patch by Erik. First bug: basename() on Windows used to not remove the trailing slashes. Since we adjusted it, to conform with the POSIX specs, the implicit mkdir() in copy_templates() could replace the trailing slash by a NUL, breaking the template copying (it truncated pretty much all paths). I did not catch this earlier because basename() was only called with HIDE_DOTFILES_TRUE, and that case was never thoroughly tested. So I had to reroll a non-modifying basename(). It's not elegant to have this, but it is better than strdup()ing each and every path, just to determine whether the basename starts with a dot (or is equal to ".git") or not. Second bug: when we switched to override open()/fopen()/freopen() using Windows' UTF-16 functions, we lost the ability to open hidden files (internally, _wopen() uses CreateFile(), which allows the equivalent of O_CREAT only if the attributes match any existing files' attributes *exactly*, and there is no way to tell _wopen() that we want to create a hidden file). Again, this was only exercised with HIDE_DOTFILES_TRUE until the change proposed by you: needs_hiding() now also triggers for .git *files* in HIDE_DOTFILES_DOTGITONLY mode. It worries me slightly that the new code is so different from the code that was tried and tested through all those years (although admittedly it is unlikely anybody ever ran with core.hideDotFiles = true, given above findings). But I guess that cannot be helped. Not unless we reintroduce those two bugs. > I do not understand why core.hideDotFiles needs to be explicitly > copied to the config file in the resulting repository, either. > > Once you created a repository, Git won't unhide the hidden .git of > that reposiotry, so the idea must be to propagate the setting to a > new repository that will further be created, but wouldn't that get > the "please hide" preference from the same place as we have got the > preference to hide .git when the current repository was created > anyway? I had a look in the mail archives, and it looks as if I wanted to support `git clone -c core.hideDotFiles...`. I introduced a new regression test and verified that we no longer need to write that config setting explicitly. I will send out v2 as soon as the test suite passed (which is hopefully 30-45 minutes from now). Ciao, Dscho ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] mingw: introduce the 'core.hideDotFiles' setting 2016-05-06 15:19 ` Johannes Schindelin @ 2016-05-06 16:34 ` Junio C Hamano 2016-05-06 17:17 ` Junio C Hamano 2016-05-07 6:44 ` Johannes Schindelin 0 siblings, 2 replies; 27+ messages in thread From: Junio C Hamano @ 2016-05-06 16:34 UTC (permalink / raw) To: Johannes Schindelin; +Cc: Erik Faye-Lund, Pat Thoyts, git Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: >> I agree with the goal of the change, but I am having a hard time >> justifying this addition. Primarily because I do not understand the >> need for this. >> >> In order to be prepared to handle HIDE_DOTFILES_TRUE case, >> mingw_mkdir() needs to be taught about needs_hiding() and >> make_hidden() already. Why isn't it sufficient to teach >> needs_hiding() to check with !strcmp(basename(path, ".git")) >> under HIDE_DOTFILES_DOTGITONLY? > > The reason was that I wanted to avoid to compare a name unnecessarily when > I already had a code path that knew perfectly well that a given directory > is the .git/ directory. > > I made the change. It was more painful than I expected, as two bugs were > uncovered, both introduced after the original patch by Erik. > ... > It worries me slightly that the new code is so different from the code > that was tried and tested through all those years (although admittedly it > is unlikely anybody ever ran with core.hideDotFiles = true, given above > findings). But I guess that cannot be helped. Not unless we reintroduce > those two bugs. I have a huge preference for a code that has been production for years over a new code that would cook at most two weeks in 'next'. As I said, the part regarding the mark_as_git_dir() in the message you are responding to was me asking you to explain, not me objecting to the code. > I had a look in the mail archives, and it looks as if I wanted to support > `git clone -c core.hideDotFiles...`. I introduced a new regression test > and verified that we no longer need to write that config setting > explicitly. If you are sure we do not need that, that is one less reason we would be better off without mark_as_git_dir(). It was another way how a $GIT_DIR creation codepath behaved differently from other mingw_mkdir() codepath in the patch. Having said that, I actually was leaning towards an opinion that it might actually be better to have mark_as_git_dir() there. Platforms may need to do other things in their implementation of the function to tweak things inside $GIT_DIR, just like you had to have a place to add configuration variables and mark_as_git_dir() was the perfect place for it. But mark_as_git_dir() is not a generic enough name to express its purpose. It wasn't even when all it did was to see the HIDE_DOTFILES configuration and hide it (you are not marking it, in the sense that after you return, you cannot tell which directories are "marked" as "git_dir" by only looking at the resulting filesystem entities), and as an all-purpose place to hook platform specific tweaks, it is even less about "marking it as a $GIT_DIR". A name that hints the fact that it is a place to do a platform specific extra preparation of $GIT_DIR would be more appropriate. So given the knowledge that - I am not fundamentally opposed to having an extra call there; - in fact, I suspect it may even be a good thing to have one; - I am not entirely happy with the name mark_as_git_dir; and - the rewrite to lose that call was more painful than anticipated. would you still choose to lose the extra call and go with !stricmp(basename(path), ".git")? The best approach for v2 might be to - Keep the two bugfixes that was uncovered during this exercise; - keep the change to init_db() to add a call to mark_as_git_dir(); - optionally, come up with a better name for that function; and - drop the setting of configuration varaibles that was unnecessary. That is what I think, with the new knowledge I learned from your message. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] mingw: introduce the 'core.hideDotFiles' setting 2016-05-06 16:34 ` Junio C Hamano @ 2016-05-06 17:17 ` Junio C Hamano 2016-05-07 6:01 ` Johannes Schindelin 2016-05-07 6:44 ` Johannes Schindelin 1 sibling, 1 reply; 27+ messages in thread From: Junio C Hamano @ 2016-05-06 17:17 UTC (permalink / raw) To: Johannes Schindelin; +Cc: Erik Faye-Lund, Pat Thoyts, git Junio C Hamano <gitster@pobox.com> writes: > If you are sure we do not need that, that is one less reason we > would be better off without mark_as_git_dir(). Oops. To many rounds of rewriting followed by a rather careless final round of reading. I couldn't decide between "That is one less reason why we need it" and "That is one more reason why we are better off without it". ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] mingw: introduce the 'core.hideDotFiles' setting 2016-05-06 17:17 ` Junio C Hamano @ 2016-05-07 6:01 ` Johannes Schindelin 0 siblings, 0 replies; 27+ messages in thread From: Johannes Schindelin @ 2016-05-07 6:01 UTC (permalink / raw) To: Junio C Hamano; +Cc: Erik Faye-Lund, Pat Thoyts, git Hi Junio, On Fri, 6 May 2016, Junio C Hamano wrote: > Junio C Hamano <gitster@pobox.com> writes: > > > If you are sure we do not need that, that is one less reason we > > would be better off without mark_as_git_dir(). > > Oops. To many rounds of rewriting followed by a rather careless > final round of reading. I couldn't decide between "That is one less > reason why we need it" and "That is one more reason why we are > better off without it". Heh. For the record, I did understand what you meant the first time round. Ciao, Dscho ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] mingw: introduce the 'core.hideDotFiles' setting 2016-05-06 16:34 ` Junio C Hamano 2016-05-06 17:17 ` Junio C Hamano @ 2016-05-07 6:44 ` Johannes Schindelin 1 sibling, 0 replies; 27+ messages in thread From: Johannes Schindelin @ 2016-05-07 6:44 UTC (permalink / raw) To: Junio C Hamano; +Cc: Erik Faye-Lund, Pat Thoyts, git Hi Junio, On Fri, 6 May 2016, Junio C Hamano wrote: > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > > >> I agree with the goal of the change, but I am having a hard time > >> justifying this addition. Primarily because I do not understand the > >> need for this. > >> > >> In order to be prepared to handle HIDE_DOTFILES_TRUE case, > >> mingw_mkdir() needs to be taught about needs_hiding() and > >> make_hidden() already. Why isn't it sufficient to teach > >> needs_hiding() to check with !strcmp(basename(path, ".git")) under > >> HIDE_DOTFILES_DOTGITONLY? > > > > The reason was that I wanted to avoid to compare a name unnecessarily > > when I already had a code path that knew perfectly well that a given > > directory is the .git/ directory. > > > > I made the change. It was more painful than I expected, as two bugs > > were uncovered, both introduced after the original patch by Erik. > > ... > > It worries me slightly that the new code is so different from the code > > that was tried and tested through all those years (although admittedly > > it is unlikely anybody ever ran with core.hideDotFiles = true, given > > above findings). But I guess that cannot be helped. Not unless we > > reintroduce those two bugs. > > I have a huge preference for a code that has been production for > years over a new code that would cook at most two weeks in 'next'. I agree. However, it does not fill me with confidence that we did not catch those two bugs earlier. Even one round of reviews (including a partial rewrite) was better than all that time since the regressions were introduced. Besides, your innocuous remark that needs_hiding() could determine whether we are looking at ".git" revealed a problem with the original design: there can be .git *files*. Support for .git files was introduced in February 2008, so it is my fault that I did not catch this in January 2010, when I added the "dotGitOnly" option. I do not think that we can fix this design other than abandoning the mark_as_git_dir() function. > As I said, the part regarding the mark_as_git_dir() in the message > you are responding to was me asking you to explain, not me objecting > to the code. I understood. My initial reaction was: it makes a total lot of sense to simplify the patch by removing that global. It was more painful than anticipated only because I did not expect any bugs to be revealed in the process, certainly not hard-to-debug ones (I had to patch submodule--helper's code to allow attaching a debugger at a very specific point during t1013 so I could single-step through it, and it took quite a couple of iterations to pinpoint the problem). Even as it was painful, it was useful, too, though, as bugs were revealed and squashed. And an additional test was introduced and unnecessary code was dropped. In the same vein, was wondering whether we want to hide those Windows-only core config options behind a platform_core_config() which would then be #define'd to point to mingw_core_config()? > So given the knowledge that > > - I am not fundamentally opposed to having an extra call there; - in > fact, I suspect it may even be a good thing to have one; - I am not > entirely happy with the name mark_as_git_dir; and - the rewrite to lose > that call was more painful than anticipated. > > would you still choose to lose the extra call and go with > !stricmp(basename(path), ".git")? The best approach for v2 might be to > > - Keep the two bugfixes that was uncovered during this exercise; - keep > the change to init_db() to add a call to mark_as_git_dir(); - > optionally, come up with a better name for that function; and - drop > the setting of configuration varaibles that was unnecessary. Well, given that I learned that I cannot use basename() but have to partially copy its code, the simpler solution *is* to abandon the mark_as_git_dir() approach. See v2 (my apologies for sending it only today, I encountered a bug in my patch series sending script and was unable to fix it yesterday). Ciao, Dscho ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v2 0/2] Support marking .git/ (or all files) as hidden on Windows 2016-05-04 14:40 [PATCH] mingw: introduce the 'core.hideDotFiles' setting Johannes Schindelin 2016-05-04 16:18 ` Ramsay Jones 2016-05-04 19:06 ` Junio C Hamano @ 2016-05-07 6:44 ` Johannes Schindelin 2016-05-07 6:45 ` [PATCH v2 1/2] mingw: introduce the 'core.hideDotFiles' setting Johannes Schindelin ` (3 more replies) 2 siblings, 4 replies; 27+ messages in thread From: Johannes Schindelin @ 2016-05-07 6:44 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Ramsay Jones, Erik Faye-Lund, Pat Thoyts Windows does not share Unix' convention that files and directories whose names start with a dot are hidden. Hence `.git/`, for example, is in plain view, and caused quite a bit of trouble: some users wanted to peek inside and did not understand what it contains, others modified files. There was a stream of bug reports, until Git for Windows introduced the (opt-out) option to hide at least the .git/ directory by default. The option is configured via the config setting core.hideDotFiles, with the possible values false, true and dotGitOnly (the latter being the default). This is a heavily version of patches we carried in Git for Windows for way too long without submitting them upstream. In this iteration, I also claim authorship for the patch because by now Kusma's changes were so contorted and mutilated beyond recognition by me that I do not want anybody to blame him for my sins. Johannes Schindelin (2): mingw: introduce the 'core.hideDotFiles' setting mingw: remove unnecessary definition Documentation/config.txt | 6 ++++ cache.h | 7 +++++ compat/mingw.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++ compat/mingw.h | 3 -- config.c | 8 ++++++ environment.c | 1 + t/t0001-init.sh | 30 ++++++++++++++++++++ t/t5611-clone-config.sh | 20 +++++++++++++ 8 files changed, 146 insertions(+), 3 deletions(-) Published-As: https://github.com/dscho/git/releases/tag/hide-dotgit-v2 Interdiff vs v1: diff --git a/Documentation/config.txt b/Documentation/config.txt index 5d4e3b2..8747c2c 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -270,10 +270,10 @@ See linkgit:git-update-index[1]. The default is true (when core.filemode is not specified in the config file). core.hideDotFiles:: - (Windows-only) If true (which is the default), mark newly-created - directories and files whose name starts with a dot as hidden. - If 'dotGitOnly', only the .git/ directory is hidden, but no other - files starting with a dot. + (Windows-only) If true, mark newly-created directories and files whose + name starts with a dot as hidden. If 'dotGitOnly', only the `.git/` + directory is hidden, but no other files starting with a dot. The + default mode is to mark only the `.git/` directory as hidden. core.ignoreCase:: If true, this option enables various workarounds to enable diff --git a/builtin/init-db.c b/builtin/init-db.c index c4269ac..b2d8d40 100644 --- a/builtin/init-db.c +++ b/builtin/init-db.c @@ -370,7 +370,6 @@ int init_db(const char *template_dir, unsigned int flags) check_repository_format(); reinit = create_default_files(template_dir); - mark_as_git_dir(get_git_dir()); create_object_directory(); diff --git a/compat/mingw.c b/compat/mingw.c index 8b8b01c..3ecde84 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -288,31 +288,47 @@ int mingw_rmdir(const char *pathname) static inline int needs_hiding(const char *path) { - return hide_dotfiles == HIDE_DOTFILES_TRUE && - starts_with(basename((char*)path), "."); + const char *basename; + + if (hide_dotfiles == HIDE_DOTFILES_FALSE) + return 0; + + /* We cannot use basename(), as it would remove trailing slashes */ + mingw_skip_dos_drive_prefix((char **)&path); + if (!*path) + return 0; + + for (basename = path; *path; path++) + if (is_dir_sep(*path)) { + do { + path++; + } while (is_dir_sep(*path)); + /* ignore trailing slashes */ + if (*path) + basename = path; + } + + if (hide_dotfiles == HIDE_DOTFILES_TRUE) + return *basename == '.'; + + assert(hide_dotfiles == HIDE_DOTFILES_DOTGITONLY); + return !strncasecmp(".git", basename, 4) && + (!basename[4] || is_dir_sep(basename[4])); } -static int make_hidden(const wchar_t *path) +static int set_hidden_flag(const wchar_t *path, int set) { DWORD attribs = GetFileAttributesW(path); - if (SetFileAttributesW(path, FILE_ATTRIBUTE_HIDDEN | attribs)) + if (set) + attribs |= FILE_ATTRIBUTE_HIDDEN; + else + attribs &= ~FILE_ATTRIBUTE_HIDDEN; + if (SetFileAttributesW(path, attribs)) return 0; errno = err_win_to_posix(GetLastError()); return -1; } -void mingw_mark_as_git_dir(const char *dir) -{ - wchar_t wdir[MAX_PATH]; - if (hide_dotfiles != HIDE_DOTFILES_FALSE && !is_bare_repository()) - if (xutftowcs_path(wdir, dir) < 0 || make_hidden(wdir)) - warning("Failed to make '%s' hidden", dir); - git_config_set("core.hideDotFiles", - hide_dotfiles == HIDE_DOTFILES_FALSE ? "false" : - (hide_dotfiles == HIDE_DOTFILES_DOTGITONLY ? - "dotGitOnly" : "true")); -} - int mingw_mkdir(const char *path, int mode) { int ret; @@ -321,7 +337,7 @@ int mingw_mkdir(const char *path, int mode) return -1; ret = _wmkdir(wpath); if (!ret && needs_hiding(path)) - return make_hidden(wpath); + return set_hidden_flag(wpath, 1); return ret; } @@ -348,9 +364,21 @@ int mingw_open (const char *filename, int oflags, ...) if (attrs != INVALID_FILE_ATTRIBUTES && (attrs & FILE_ATTRIBUTE_DIRECTORY)) errno = EISDIR; } - if ((oflags & O_CREAT) && fd >= 0 && needs_hiding(filename) && - make_hidden(wfilename)) - warning("Could not mark '%s' as hidden.", filename); + if ((oflags & O_CREAT) && needs_hiding(filename)) { + /* + * Internally, _wopen() uses the CreateFile() API which errors + * out with an ERROR_ACCESS_DENIED if CREATE_ALWAYS was + * specified and an already existing file's attributes do not + * match *exactly*. As there is no mode or flag we can set that + * would correspond to FILE_ATTRIBUTE_HIDDEN, let's just try + * again *without* the O_CREAT flag (that corresponds to the + * CREATE_ALWAYS flag of CreateFile()). + */ + if (fd < 0 && errno == EACCES) + fd = _wopen(wfilename, oflags & ~O_CREAT, mode); + if (fd >= 0 && set_hidden_flag(wfilename, 1)) + warning("Could not mark '%s' as hidden.", filename); + } return fd; } @@ -382,7 +410,7 @@ int mingw_fgetc(FILE *stream) #undef fopen FILE *mingw_fopen (const char *filename, const char *otype) { - int hide = needs_hiding(filename) && access(filename, F_OK); + int hide = needs_hiding(filename); FILE *file; wchar_t wfilename[MAX_PATH], wotype[4]; if (filename && !strcmp(filename, "/dev/null")) @@ -390,15 +418,19 @@ FILE *mingw_fopen (const char *filename, const char *otype) if (xutftowcs_path(wfilename, filename) < 0 || xutftowcs(wotype, otype, ARRAY_SIZE(wotype)) < 0) return NULL; + if (hide && !access(filename, F_OK) && set_hidden_flag(wfilename, 0)) { + error("Could not unhide %s", filename); + return NULL; + } file = _wfopen(wfilename, wotype); - if (file && hide && make_hidden(wfilename)) + if (file && hide && set_hidden_flag(wfilename, 1)) warning("Could not mark '%s' as hidden.", filename); return file; } FILE *mingw_freopen (const char *filename, const char *otype, FILE *stream) { - int hide = needs_hiding(filename) && access(filename, F_OK); + int hide = needs_hiding(filename); FILE *file; wchar_t wfilename[MAX_PATH], wotype[4]; if (filename && !strcmp(filename, "/dev/null")) @@ -406,8 +438,12 @@ FILE *mingw_freopen (const char *filename, const char *otype, FILE *stream) if (xutftowcs_path(wfilename, filename) < 0 || xutftowcs(wotype, otype, ARRAY_SIZE(wotype)) < 0) return NULL; + if (hide && !access(filename, F_OK) && set_hidden_flag(wfilename, 0)) { + error("Could not unhide %s", filename); + return NULL; + } file = _wfreopen(wfilename, wotype, stream); - if (file && hide && make_hidden(wfilename)) + if (file && hide && set_hidden_flag(wfilename, 1)) warning("Could not mark '%s' as hidden.", filename); return file; } diff --git a/compat/mingw.h b/compat/mingw.h index 1de70ff..a1808b4 100644 --- a/compat/mingw.h +++ b/compat/mingw.h @@ -416,9 +416,6 @@ int mingw_offset_1st_component(const char *path); void mingw_open_html(const char *path); #define open_html mingw_open_html -void mingw_mark_as_git_dir(const char *dir); -#define mark_as_git_dir mingw_mark_as_git_dir - /** * Converts UTF-8 encoded string to UTF-16LE. * diff --git a/git-compat-util.h b/git-compat-util.h index ea007e4..1f8b5f3 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -1042,8 +1042,4 @@ struct tm *git_gmtime_r(const time_t *, struct tm *); #define getc_unlocked(fh) getc(fh) #endif -#ifndef mark_as_git_dir -#define mark_as_git_dir(x) /* noop */ -#endif - #endif diff --git a/t/t5611-clone-config.sh b/t/t5611-clone-config.sh index 27d730c..e4850b7 100755 --- a/t/t5611-clone-config.sh +++ b/t/t5611-clone-config.sh @@ -37,4 +37,24 @@ test_expect_success 'clone -c config is available during clone' ' test_cmp expect child/file ' +# Tests for the hidden file attribute on windows +is_hidden () { + # Use the output of `attrib`, ignore the absolute path + case "$(attrib "$1")" in *H*?:*) return 0;; esac + return 1 +} + +test_expect_success MINGW 'clone -c core.hideDotFiles' ' + test_commit attributes .gitattributes "" && + rm -rf child && + git clone -c core.hideDotFiles=false . child && + ! is_hidden child/.gitattributes && + rm -rf child && + git clone -c core.hideDotFiles=dotGitOnly . child && + ! is_hidden child/.gitattributes && + rm -rf child && + git clone -c core.hideDotFiles=true . child && + is_hidden child/.gitattributes +' + test_done -- 2.8.2.463.g99156ee ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v2 1/2] mingw: introduce the 'core.hideDotFiles' setting 2016-05-07 6:44 ` [PATCH v2 0/2] Support marking .git/ (or all files) as hidden on Windows Johannes Schindelin @ 2016-05-07 6:45 ` Johannes Schindelin 2016-05-09 17:23 ` Junio C Hamano 2016-05-07 6:45 ` [PATCH v2 2/2] mingw: remove unnecessary definition Johannes Schindelin ` (2 subsequent siblings) 3 siblings, 1 reply; 27+ messages in thread From: Johannes Schindelin @ 2016-05-07 6:45 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Ramsay Jones, Erik Faye-Lund, Pat Thoyts On Unix (and Linux) it is common that files and directories whose names start with a dot are not shown by default. This convention is used by Git: the .git/ directory should be left alone by regular users, and only accessed through Git itself. On Windows, no such convention exists. Instead, there is an explicit flag to mark files or directories as hidden. In the early days, Git for Windows did not mark the .git/ directory (or for that matter, any file or directory whose name starts with a dot) hidden. This lead to quite a bit of confusion, and even loss of data. Consequently, Git for Windows introduced the core.hideDotFiles setting, with three possible values: true, false, and dotGitOnly, defaulting to marking only the .git/ directory as hidden. The rationale: users do not need to access .git/ directly, and indeed (as was demonstrated) should not really see that directory, either. However, not all dot files should be hidden, as e.g. Eclipse does not show them (and the user would therefore be unable to add, say, a .gitattributes file). In over five years since the last attempt to bring this patch into core Git, this patch has served Git for Windows' users well: no single report indicated problems with the hidden .git/ directory, and the stream of problems caused by the previously non-hidden .git/ directory simply stopped. Original-patch-by: Erik Faye-Lund <kusmabite@gmail.com> Initial-Test-By: Pat Thoyts <patthoyts@users.sourceforge.net> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- Documentation/config.txt | 6 ++++ cache.h | 7 +++++ compat/mingw.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++ config.c | 8 ++++++ environment.c | 1 + t/t0001-init.sh | 30 ++++++++++++++++++++ t/t5611-clone-config.sh | 20 +++++++++++++ 7 files changed, 146 insertions(+) diff --git a/Documentation/config.txt b/Documentation/config.txt index c7bbe98..8747c2c 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -269,6 +269,12 @@ See linkgit:git-update-index[1]. + The default is true (when core.filemode is not specified in the config file). +core.hideDotFiles:: + (Windows-only) If true, mark newly-created directories and files whose + name starts with a dot as hidden. If 'dotGitOnly', only the `.git/` + directory is hidden, but no other files starting with a dot. The + default mode is to mark only the `.git/` directory as hidden. + core.ignoreCase:: If true, this option enables various workarounds to enable Git to work better on filesystems that are not case sensitive, diff --git a/cache.h b/cache.h index 160f8e3..1c488fd 100644 --- a/cache.h +++ b/cache.h @@ -700,6 +700,13 @@ extern int ref_paranoia; extern char comment_line_char; extern int auto_comment_line_char; +enum hide_dotfiles_type { + HIDE_DOTFILES_FALSE = 0, + HIDE_DOTFILES_TRUE, + HIDE_DOTFILES_DOTGITONLY, +}; +extern enum hide_dotfiles_type hide_dotfiles; + enum branch_track { BRANCH_TRACK_UNSPECIFIED = -1, BRANCH_TRACK_NEVER = 0, diff --git a/compat/mingw.c b/compat/mingw.c index 0413d5c..3ecde84 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -286,6 +286,49 @@ int mingw_rmdir(const char *pathname) return ret; } +static inline int needs_hiding(const char *path) +{ + const char *basename; + + if (hide_dotfiles == HIDE_DOTFILES_FALSE) + return 0; + + /* We cannot use basename(), as it would remove trailing slashes */ + mingw_skip_dos_drive_prefix((char **)&path); + if (!*path) + return 0; + + for (basename = path; *path; path++) + if (is_dir_sep(*path)) { + do { + path++; + } while (is_dir_sep(*path)); + /* ignore trailing slashes */ + if (*path) + basename = path; + } + + if (hide_dotfiles == HIDE_DOTFILES_TRUE) + return *basename == '.'; + + assert(hide_dotfiles == HIDE_DOTFILES_DOTGITONLY); + return !strncasecmp(".git", basename, 4) && + (!basename[4] || is_dir_sep(basename[4])); +} + +static int set_hidden_flag(const wchar_t *path, int set) +{ + DWORD attribs = GetFileAttributesW(path); + if (set) + attribs |= FILE_ATTRIBUTE_HIDDEN; + else + attribs &= ~FILE_ATTRIBUTE_HIDDEN; + if (SetFileAttributesW(path, attribs)) + return 0; + errno = err_win_to_posix(GetLastError()); + return -1; +} + int mingw_mkdir(const char *path, int mode) { int ret; @@ -293,6 +336,8 @@ int mingw_mkdir(const char *path, int mode) if (xutftowcs_path(wpath, path) < 0) return -1; ret = _wmkdir(wpath); + if (!ret && needs_hiding(path)) + return set_hidden_flag(wpath, 1); return ret; } @@ -319,6 +364,21 @@ int mingw_open (const char *filename, int oflags, ...) if (attrs != INVALID_FILE_ATTRIBUTES && (attrs & FILE_ATTRIBUTE_DIRECTORY)) errno = EISDIR; } + if ((oflags & O_CREAT) && needs_hiding(filename)) { + /* + * Internally, _wopen() uses the CreateFile() API which errors + * out with an ERROR_ACCESS_DENIED if CREATE_ALWAYS was + * specified and an already existing file's attributes do not + * match *exactly*. As there is no mode or flag we can set that + * would correspond to FILE_ATTRIBUTE_HIDDEN, let's just try + * again *without* the O_CREAT flag (that corresponds to the + * CREATE_ALWAYS flag of CreateFile()). + */ + if (fd < 0 && errno == EACCES) + fd = _wopen(wfilename, oflags & ~O_CREAT, mode); + if (fd >= 0 && set_hidden_flag(wfilename, 1)) + warning("Could not mark '%s' as hidden.", filename); + } return fd; } @@ -350,6 +410,7 @@ int mingw_fgetc(FILE *stream) #undef fopen FILE *mingw_fopen (const char *filename, const char *otype) { + int hide = needs_hiding(filename); FILE *file; wchar_t wfilename[MAX_PATH], wotype[4]; if (filename && !strcmp(filename, "/dev/null")) @@ -357,12 +418,19 @@ FILE *mingw_fopen (const char *filename, const char *otype) if (xutftowcs_path(wfilename, filename) < 0 || xutftowcs(wotype, otype, ARRAY_SIZE(wotype)) < 0) return NULL; + if (hide && !access(filename, F_OK) && set_hidden_flag(wfilename, 0)) { + error("Could not unhide %s", filename); + return NULL; + } file = _wfopen(wfilename, wotype); + if (file && hide && set_hidden_flag(wfilename, 1)) + warning("Could not mark '%s' as hidden.", filename); return file; } FILE *mingw_freopen (const char *filename, const char *otype, FILE *stream) { + int hide = needs_hiding(filename); FILE *file; wchar_t wfilename[MAX_PATH], wotype[4]; if (filename && !strcmp(filename, "/dev/null")) @@ -370,7 +438,13 @@ FILE *mingw_freopen (const char *filename, const char *otype, FILE *stream) if (xutftowcs_path(wfilename, filename) < 0 || xutftowcs(wotype, otype, ARRAY_SIZE(wotype)) < 0) return NULL; + if (hide && !access(filename, F_OK) && set_hidden_flag(wfilename, 0)) { + error("Could not unhide %s", filename); + return NULL; + } file = _wfreopen(wfilename, wotype, stream); + if (file && hide && set_hidden_flag(wfilename, 1)) + warning("Could not mark '%s' as hidden.", filename); return file; } diff --git a/config.c b/config.c index 10b5c95..1b44d46 100644 --- a/config.c +++ b/config.c @@ -912,6 +912,14 @@ static int git_default_core_config(const char *var, const char *value) return 0; } + if (!strcmp(var, "core.hidedotfiles")) { + if (value && !strcasecmp(value, "dotgitonly")) + hide_dotfiles = HIDE_DOTFILES_DOTGITONLY; + else + hide_dotfiles = git_config_bool(var, value); + return 0; + } + /* Add other config variables here and to Documentation/config.txt. */ return 0; } diff --git a/environment.c b/environment.c index 57acb2f..96160a7 100644 --- a/environment.c +++ b/environment.c @@ -63,6 +63,7 @@ int core_apply_sparse_checkout; int merge_log_config = -1; int precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */ unsigned long pack_size_limit_cfg; +enum hide_dotfiles_type hide_dotfiles = HIDE_DOTFILES_DOTGITONLY; #ifndef PROTECT_HFS_DEFAULT #define PROTECT_HFS_DEFAULT 0 diff --git a/t/t0001-init.sh b/t/t0001-init.sh index a5b9e7a..a6fdd5e 100755 --- a/t/t0001-init.sh +++ b/t/t0001-init.sh @@ -354,4 +354,34 @@ test_expect_success SYMLINKS 're-init to move gitdir symlink' ' test_path_is_dir realgitdir/refs ' +# Tests for the hidden file attribute on windows +is_hidden () { + # Use the output of `attrib`, ignore the absolute path + case "$(attrib "$1")" in *H*?:*) return 0;; esac + return 1 +} + +test_expect_success MINGW '.git hidden' ' + rm -rf newdir && + ( + unset GIT_DIR GIT_WORK_TREE + mkdir newdir && + cd newdir && + git init && + is_hidden .git + ) && + check_config newdir/.git false unset +' + +test_expect_success MINGW 'bare git dir not hidden' ' + rm -rf newdir && + ( + unset GIT_DIR GIT_WORK_TREE GIT_CONFIG + mkdir newdir && + cd newdir && + git --bare init + ) && + ! is_hidden newdir +' + test_done diff --git a/t/t5611-clone-config.sh b/t/t5611-clone-config.sh index 27d730c..e4850b7 100755 --- a/t/t5611-clone-config.sh +++ b/t/t5611-clone-config.sh @@ -37,4 +37,24 @@ test_expect_success 'clone -c config is available during clone' ' test_cmp expect child/file ' +# Tests for the hidden file attribute on windows +is_hidden () { + # Use the output of `attrib`, ignore the absolute path + case "$(attrib "$1")" in *H*?:*) return 0;; esac + return 1 +} + +test_expect_success MINGW 'clone -c core.hideDotFiles' ' + test_commit attributes .gitattributes "" && + rm -rf child && + git clone -c core.hideDotFiles=false . child && + ! is_hidden child/.gitattributes && + rm -rf child && + git clone -c core.hideDotFiles=dotGitOnly . child && + ! is_hidden child/.gitattributes && + rm -rf child && + git clone -c core.hideDotFiles=true . child && + is_hidden child/.gitattributes +' + test_done -- 2.8.2.463.g99156ee ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v2 1/2] mingw: introduce the 'core.hideDotFiles' setting 2016-05-07 6:45 ` [PATCH v2 1/2] mingw: introduce the 'core.hideDotFiles' setting Johannes Schindelin @ 2016-05-09 17:23 ` Junio C Hamano 2016-05-10 11:58 ` Johannes Schindelin 0 siblings, 1 reply; 27+ messages in thread From: Junio C Hamano @ 2016-05-09 17:23 UTC (permalink / raw) To: Johannes Schindelin; +Cc: git, Ramsay Jones, Erik Faye-Lund, Pat Thoyts Johannes Schindelin <johannes.schindelin@gmx.de> writes: > +core.hideDotFiles:: > + (Windows-only) If true, mark newly-created directories and files whose > + name starts with a dot as hidden. If 'dotGitOnly', only the `.git/` > + directory is hidden, but no other files starting with a dot. The > + default mode is to mark only the `.git/` directory as hidden. I think "The default mode is 'dotGitOnly'" is sufficient, given that it is described just one sentence before, which is still likely to be in readers' mind. But I'll let it pass without tweaking. > +enum hide_dotfiles_type { > + HIDE_DOTFILES_FALSE = 0, > + HIDE_DOTFILES_TRUE, > + HIDE_DOTFILES_DOTGITONLY, We allow ',' after the last array initializer, but not after the last enum definition. I'll tweak it out while queuing. > @@ -319,6 +364,21 @@ int mingw_open (const char *filename, int oflags, ...) > if (attrs != INVALID_FILE_ATTRIBUTES && (attrs & FILE_ATTRIBUTE_DIRECTORY)) > errno = EISDIR; > } > + if ((oflags & O_CREAT) && needs_hiding(filename)) { > + /* > + * Internally, _wopen() uses the CreateFile() API which errors > + * out with an ERROR_ACCESS_DENIED if CREATE_ALWAYS was > + * specified and an already existing file's attributes do not > + * match *exactly*. As there is no mode or flag we can set that > + * would correspond to FILE_ATTRIBUTE_HIDDEN, let's just try > + * again *without* the O_CREAT flag (that corresponds to the > + * CREATE_ALWAYS flag of CreateFile()). > + */ > + if (fd < 0 && errno == EACCES) > + fd = _wopen(wfilename, oflags & ~O_CREAT, mode); This "retry if we got EACCESS" felt strange to me in two ways. One is explained well in the comment and you know what you are doing, as opposed to me who is clueless with CreateFile() API. The other is why you do not have to retry creation in a similar way when !needs_hiding(filename). I didn't see anything in the function before reaching this point that does anything differently based on needs_hiding(). Can't the same 'ERROR_ACCESS_DENIED' error trigger if CREATE_ALWAYS was specified and file attributes of an existing file match, and if it can, don't you want to retry that too, even if you are not going to hide the filename? That is, I am wondering, without knowing much about Windows API, why the code is more like this: fd = _wopen(...); if (fd < 0 && ...) { if (attrs != INVALID_...) errno = ISDIR; } if ((oflags & O_CREAT) && fd < 0 && errno == EACCESS) /* That big comment here ... */ fd = _open(wfilename, oflags & ~O_CREAT, mode); if ((oflags & O_CREAT) && needs_hiding(filename)) { if (fd >= 0 && set_hidden_flag(wfilename, 1)) warning("could not mark '%s' as hidden"...); } Obviously, I will *not* do this tweak myself ;-) > + if (fd >= 0 && set_hidden_flag(wfilename, 1)) > + warning("Could not mark '%s' as hidden.", filename); > + } I'll tweak all new instances of "Could" with s/Could/could/ to save Micheal trouble (cf. b846ae21daf). Thanks. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2 1/2] mingw: introduce the 'core.hideDotFiles' setting 2016-05-09 17:23 ` Junio C Hamano @ 2016-05-10 11:58 ` Johannes Schindelin 2016-05-10 17:19 ` Junio C Hamano 0 siblings, 1 reply; 27+ messages in thread From: Johannes Schindelin @ 2016-05-10 11:58 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Ramsay Jones, Erik Faye-Lund, Pat Thoyts Hi Junio, On Mon, 9 May 2016, Junio C Hamano wrote: > Johannes Schindelin <johannes.schindelin@gmx.de> writes: > > > +core.hideDotFiles:: > > + (Windows-only) If true, mark newly-created directories and files whose > > + name starts with a dot as hidden. If 'dotGitOnly', only the `.git/` > > + directory is hidden, but no other files starting with a dot. The > > + default mode is to mark only the `.git/` directory as hidden. > > I think "The default mode is 'dotGitOnly'" is sufficient, given that > it is described just one sentence before, which is still likely to > be in readers' mind. But I'll let it pass without tweaking. Yeah, when rewriting this after Ramsay pointed out the erroneous documentation, I wanted to fail on the crystal-clear side. > > +enum hide_dotfiles_type { > > + HIDE_DOTFILES_FALSE = 0, > > + HIDE_DOTFILES_TRUE, > > + HIDE_DOTFILES_DOTGITONLY, > > We allow ',' after the last array initializer, but not after the > last enum definition. I'll tweak it out while queuing. Whoops. Sorry. I should have caught that five years ago. > > @@ -319,6 +364,21 @@ int mingw_open (const char *filename, int oflags, ...) > > if (attrs != INVALID_FILE_ATTRIBUTES && (attrs & FILE_ATTRIBUTE_DIRECTORY)) > > errno = EISDIR; > > } > > + if ((oflags & O_CREAT) && needs_hiding(filename)) { > > + /* > > + * Internally, _wopen() uses the CreateFile() API which errors > > + * out with an ERROR_ACCESS_DENIED if CREATE_ALWAYS was > > + * specified and an already existing file's attributes do not > > + * match *exactly*. As there is no mode or flag we can set that > > + * would correspond to FILE_ATTRIBUTE_HIDDEN, let's just try > > + * again *without* the O_CREAT flag (that corresponds to the > > + * CREATE_ALWAYS flag of CreateFile()). > > + */ > > + if (fd < 0 && errno == EACCES) > > + fd = _wopen(wfilename, oflags & ~O_CREAT, mode); > > This "retry if we got EACCESS" felt strange to me in two ways. One > is explained well in the comment and you know what you are doing, as > opposed to me who is clueless with CreateFile() API. > > The other is why you do not have to retry creation in a similar way > when !needs_hiding(filename). I didn't see anything in the function > before reaching this point that does anything differently based on > needs_hiding(). Can't the same 'ERROR_ACCESS_DENIED' error trigger > if CREATE_ALWAYS was specified and file attributes of an existing > file match, and if it can, don't you want to retry that too, even if > you are not going to hide the filename? The attributes that can trigger the ERROR_ACCESS_DENIED error are the hidden flag and the system flag. Since Git *never* marks any file as system file, we should be safe. Granted, it would be theoretically possible that some enterprisey person tracks system files and marks them via some hook or some such. I am not willing to introduce support for that until somebody *actually* needs it and puts in the work to verify that it will work as expected. There is another case where ERROR_ACCESS_DENIED can be returned that is worth mentioning: under certain circumstances, deleting a file does not delete the file right away, but waits until the last handle to said file was closed. In such a case, the user cannot create that file, either. It does not exactly hurt to try without O_CREAT again, but it does not make sense in this case, either. And finally, there are programs that lock files when they access them (MS Access, I am looking at you). I believe that we would end up with an EACCES in that case, too, but we would not really be able to do much about it. We *could* introduce the same "Try again?" handling as we already have for rename() and friends, but I'd rather get Karsten's clean-up patch https://github.com/git-for-windows/git/commit/86e8394c2 in before doing that. If we do it for fopen()/freopen()/open() at all. > > + if (fd >= 0 && set_hidden_flag(wfilename, 1)) > > + warning("Could not mark '%s' as hidden.", filename); > > + } > > I'll tweak all new instances of "Could" with s/Could/could/ to save > Micheal trouble (cf. b846ae21daf). You mean Michael? ;-) Anyway, the next iteration of this patch is on its way. Ciao, Dscho ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2 1/2] mingw: introduce the 'core.hideDotFiles' setting 2016-05-10 11:58 ` Johannes Schindelin @ 2016-05-10 17:19 ` Junio C Hamano 2016-05-11 8:40 ` Johannes Schindelin 0 siblings, 1 reply; 27+ messages in thread From: Junio C Hamano @ 2016-05-10 17:19 UTC (permalink / raw) To: Johannes Schindelin; +Cc: git, Ramsay Jones, Erik Faye-Lund, Pat Thoyts Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > On Mon, 9 May 2016, Junio C Hamano wrote: > >> Johannes Schindelin <johannes.schindelin@gmx.de> writes: >> >> > +core.hideDotFiles:: >> > + (Windows-only) If true, mark newly-created directories and files whose >> > + name starts with a dot as hidden. If 'dotGitOnly', only the `.git/` >> > + directory is hidden, but no other files starting with a dot. The >> > + default mode is to mark only the `.git/` directory as hidden. >> >> I think "The default mode is 'dotGitOnly'" is sufficient, given that >> it is described just one sentence before, which is still likely to >> be in readers' mind. But I'll let it pass without tweaking. > > Yeah, when rewriting this after Ramsay pointed out the erroneous > documentation, I wanted to fail on the crystal-clear side. Saying the same thing twice in rapid succession does not make it any clearer, though. If you are rerolling anyway, then I think it would be better to shorten it to clarify. >> The other is why you do not have to retry creation in a similar way >> when !needs_hiding(filename). I didn't see anything in the function >> before reaching this point that does anything differently based on >> needs_hiding(). Can't the same 'ERROR_ACCESS_DENIED' error trigger >> if CREATE_ALWAYS was specified and file attributes of an existing >> file match, and if it can, don't you want to retry that too, even if >> you are not going to hide the filename? > > The attributes that can trigger the ERROR_ACCESS_DENIED error are the > hidden flag and the system flag. Since Git *never* marks any file as > system file, we should be safe. I was just wondering Git's trying to open it via this codepath trigger the "if CREATE_ALWAYS was specified and an already existing file's attributes do not match *exactly*.", if the path is already hidden. But again, I am clueless on Windows API, expected use pattern of Git for Windows, and your future project plans (e.g. basing future work on Karsten's clean-up patch), so I defer to your judgment. > Anyway, the next iteration of this patch is on its way. OK. Thanks. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2 1/2] mingw: introduce the 'core.hideDotFiles' setting 2016-05-10 17:19 ` Junio C Hamano @ 2016-05-11 8:40 ` Johannes Schindelin 0 siblings, 0 replies; 27+ messages in thread From: Johannes Schindelin @ 2016-05-11 8:40 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Ramsay Jones, Erik Faye-Lund, Pat Thoyts Hi Junio, On Tue, 10 May 2016, Junio C Hamano wrote: > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > > > On Mon, 9 May 2016, Junio C Hamano wrote: > > > >> Johannes Schindelin <johannes.schindelin@gmx.de> writes: > >> > >> > +core.hideDotFiles:: > >> > + (Windows-only) If true, mark newly-created directories and > >> > files whose + name starts with a dot as hidden. If > >> > 'dotGitOnly', only the `.git/` + directory is hidden, but no other > >> > files starting with a dot. The + default mode is to mark > >> > only the `.git/` directory as hidden. > >> > >> I think "The default mode is 'dotGitOnly'" is sufficient, given that > >> it is described just one sentence before, which is still likely to > >> be in readers' mind. But I'll let it pass without tweaking. > > > > Yeah, when rewriting this after Ramsay pointed out the erroneous > > documentation, I wanted to fail on the crystal-clear side. > > Saying the same thing twice in rapid succession does not make it > any clearer, though. If you are rerolling anyway, then I think it > would be better to shorten it to clarify. Okay, will send out the next iteration in a moment (this is good exercise for my cross-validating interactive rebase, and it is really not all that much hassle for me to send out subsequent iterations). Ciao, Dscho ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v2 2/2] mingw: remove unnecessary definition 2016-05-07 6:44 ` [PATCH v2 0/2] Support marking .git/ (or all files) as hidden on Windows Johannes Schindelin 2016-05-07 6:45 ` [PATCH v2 1/2] mingw: introduce the 'core.hideDotFiles' setting Johannes Schindelin @ 2016-05-07 6:45 ` Johannes Schindelin 2016-05-09 17:01 ` [PATCH v2 0/2] Support marking .git/ (or all files) as hidden on Windows Junio C Hamano 2016-05-10 11:59 ` [PATCH v3 " Johannes Schindelin 3 siblings, 0 replies; 27+ messages in thread From: Johannes Schindelin @ 2016-05-07 6:45 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Ramsay Jones, Erik Faye-Lund, Pat Thoyts For some reason, the definition of the MINGW version of `mark_as_git_dir()` slipped into this developer's patch series to support building Git for Windows. As the `mark_as_git_dir()` function is not needed at all anymore (it was used originally to support the core.hideDotFiles = gitDirOnly setting, but we now use a different method to support that case), let's just remove it. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- compat/mingw.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/compat/mingw.h b/compat/mingw.h index 1de70ff..a1808b4 100644 --- a/compat/mingw.h +++ b/compat/mingw.h @@ -416,9 +416,6 @@ int mingw_offset_1st_component(const char *path); void mingw_open_html(const char *path); #define open_html mingw_open_html -void mingw_mark_as_git_dir(const char *dir); -#define mark_as_git_dir mingw_mark_as_git_dir - /** * Converts UTF-8 encoded string to UTF-16LE. * -- 2.8.2.463.g99156ee ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v2 0/2] Support marking .git/ (or all files) as hidden on Windows 2016-05-07 6:44 ` [PATCH v2 0/2] Support marking .git/ (or all files) as hidden on Windows Johannes Schindelin 2016-05-07 6:45 ` [PATCH v2 1/2] mingw: introduce the 'core.hideDotFiles' setting Johannes Schindelin 2016-05-07 6:45 ` [PATCH v2 2/2] mingw: remove unnecessary definition Johannes Schindelin @ 2016-05-09 17:01 ` Junio C Hamano 2016-05-10 8:41 ` Johannes Schindelin 2016-05-10 11:59 ` [PATCH v3 " Johannes Schindelin 3 siblings, 1 reply; 27+ messages in thread From: Junio C Hamano @ 2016-05-09 17:01 UTC (permalink / raw) To: Johannes Schindelin; +Cc: git, Ramsay Jones, Erik Faye-Lund, Pat Thoyts Johannes Schindelin <johannes.schindelin@gmx.de> writes: > This is a heavily version of patches we carried in Git for Windows for > way too long without submitting them upstream. > > In this iteration, I also claim authorship for the patch because by now > Kusma's changes were so contorted and mutilated beyond recognition by me > that I do not want anybody to blame him for my sins. OK, so what do you want me to do with this "heavily modified version"? Earlier you responded: > I have a huge preference for a code that has been production for > years over a new code that would cook at most two weeks in 'next'. I agree. However, it does not fill me with confidence that we did not catch those two bugs earlier. Even one round of reviews (including a partial rewrite) was better than all that time since the regressions were introduced. So do we want to follow the regular "a few days in 'pu' in case somebody finds 'oops this trivial change is needed', a week or two in 'next' for simmering as everybody else, and finally down to 'master'" schedule? ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2 0/2] Support marking .git/ (or all files) as hidden on Windows 2016-05-09 17:01 ` [PATCH v2 0/2] Support marking .git/ (or all files) as hidden on Windows Junio C Hamano @ 2016-05-10 8:41 ` Johannes Schindelin 2016-05-10 17:22 ` Junio C Hamano 0 siblings, 1 reply; 27+ messages in thread From: Johannes Schindelin @ 2016-05-10 8:41 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Ramsay Jones, Erik Faye-Lund, Pat Thoyts Hi Junio, On Mon, 9 May 2016, Junio C Hamano wrote: > Johannes Schindelin <johannes.schindelin@gmx.de> writes: > > > This is a heavily version of patches we carried in Git for Windows for s/patches/patched/ I wish I had a penny for each time I wrote this particular typo. > > way too long without submitting them upstream. > > > > In this iteration, I also claim authorship for the patch because by now > > Kusma's changes were so contorted and mutilated beyond recognition by me > > that I do not want anybody to blame him for my sins. > > OK, so what do you want me to do with this "heavily modified > version"? Earlier you responded: > > > I have a huge preference for a code that has been production for > > years over a new code that would cook at most two weeks in 'next'. > > I agree. However, it does not fill me with confidence that we did not > catch those two bugs earlier. Even one round of reviews (including a > partial rewrite) was better than all that time since the regressions > were introduced. > > So do we want to follow the regular "a few days in 'pu' in case > somebody finds 'oops this trivial change is needed', a week or two > in 'next' for simmering as everybody else, and finally down to > 'master'" schedule? Well, I plan to include this patch (replacing the original version) in whatever Git for Windows version I release next. I guess that we can go with the regular way in git.git. You could just as well merge it to master right away, it won't matter much as far as Git for Windows is concerned. Ciao, Dscho ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2 0/2] Support marking .git/ (or all files) as hidden on Windows 2016-05-10 8:41 ` Johannes Schindelin @ 2016-05-10 17:22 ` Junio C Hamano 2016-05-11 8:34 ` Johannes Schindelin 0 siblings, 1 reply; 27+ messages in thread From: Junio C Hamano @ 2016-05-10 17:22 UTC (permalink / raw) To: Johannes Schindelin; +Cc: git, Ramsay Jones, Erik Faye-Lund, Pat Thoyts Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > Hi Junio, > > On Mon, 9 May 2016, Junio C Hamano wrote: > >> Johannes Schindelin <johannes.schindelin@gmx.de> writes: >> >> > This is a heavily version of patches we carried in Git for Windows for > > s/patches/patched/ > > I wish I had a penny for each time I wrote this particular typo. This is a heavily version of patched we carried... does not sound all that grammatical. These are heavily modified version of patches... is a possibility, but perhaps you meant This is a heavily patched version of what we carried... >> OK, so what do you want me to do with this "heavily modified >> version"? Earlier you responded: >> >> > I have a huge preference for a code that has been production for >> > years over a new code that would cook at most two weeks in 'next'. >> >> I agree. However, it does not fill me with confidence that we did not >> catch those two bugs earlier. Even one round of reviews (including a >> partial rewrite) was better than all that time since the regressions >> were introduced. >> >> So do we want to follow the regular "a few days in 'pu' in case >> somebody finds 'oops this trivial change is needed', a week or two >> in 'next' for simmering as everybody else, and finally down to >> 'master'" schedule? > > Well, I plan to include this patch (replacing the original > version) in whatever Git for Windows version I release next. I > guess that we can go with the regular way in git.git. You could > just as well merge it to master right away, it won't matter much > as far as Git for Windows is concerned. OK. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2 0/2] Support marking .git/ (or all files) as hidden on Windows 2016-05-10 17:22 ` Junio C Hamano @ 2016-05-11 8:34 ` Johannes Schindelin 0 siblings, 0 replies; 27+ messages in thread From: Johannes Schindelin @ 2016-05-11 8:34 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Ramsay Jones, Erik Faye-Lund, Pat Thoyts Hi Junio, On Tue, 10 May 2016, Junio C Hamano wrote: > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > > > On Mon, 9 May 2016, Junio C Hamano wrote: > > > >> Johannes Schindelin <johannes.schindelin@gmx.de> writes: > >> > >> > This is a heavily version of patches we carried in Git for Windows for > > > > s/patches/patched/ > > > > I wish I had a penny for each time I wrote this particular typo. > > This is a heavily version of patched we carried... > > does not sound all that grammatical. Aargh. What I meant to say is: "This is a modified version of patches we carried in..." Sorry for the noise. Ciao, Dscho ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v3 0/2] Support marking .git/ (or all files) as hidden on Windows 2016-05-07 6:44 ` [PATCH v2 0/2] Support marking .git/ (or all files) as hidden on Windows Johannes Schindelin ` (2 preceding siblings ...) 2016-05-09 17:01 ` [PATCH v2 0/2] Support marking .git/ (or all files) as hidden on Windows Junio C Hamano @ 2016-05-10 11:59 ` Johannes Schindelin 2016-05-10 11:59 ` [PATCH v3 1/2] mingw: introduce the 'core.hideDotFiles' setting Johannes Schindelin ` (3 more replies) 3 siblings, 4 replies; 27+ messages in thread From: Johannes Schindelin @ 2016-05-10 11:59 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Ramsay Jones, Erik Faye-Lund, Pat Thoyts Windows does not share Unix' convention that files and directories whose names start with a dot are hidden. Hence `.git/`, for example, is in plain view, and caused quite a bit of trouble: some users wanted to peek inside and did not understand what it contains, others modified files. There was a stream of bug reports, until Git for Windows introduced the (opt-out) option to hide at least the .git/ directory by default. The option is configured via the config setting core.hideDotFiles, with the possible values false, true and dotGitOnly (the latter being the default). This is a heavily version of patches we carried in Git for Windows for way too long without submitting them upstream. This iteration addresses Junio's most recent round of concerns. Oh, and while at it, it also avoids setting attributes unnecessarily. Johannes Schindelin (2): mingw: introduce the 'core.hideDotFiles' setting mingw: remove unnecessary definition Documentation/config.txt | 6 ++++ cache.h | 7 +++++ compat/mingw.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++ compat/mingw.h | 3 -- config.c | 8 ++++++ environment.c | 1 + t/t0001-init.sh | 30 ++++++++++++++++++++ t/t5611-clone-config.sh | 20 +++++++++++++ 8 files changed, 146 insertions(+), 3 deletions(-) Published-As: https://github.com/dscho/git/releases/tag/hide-dotgit-v3 Interdiff vs v2: diff --git a/cache.h b/cache.h index 743b081..5f72f59 100644 --- a/cache.h +++ b/cache.h @@ -704,7 +704,7 @@ extern int auto_comment_line_char; enum hide_dotfiles_type { HIDE_DOTFILES_FALSE = 0, HIDE_DOTFILES_TRUE, - HIDE_DOTFILES_DOTGITONLY, + HIDE_DOTFILES_DOTGITONLY }; extern enum hide_dotfiles_type hide_dotfiles; diff --git a/compat/mingw.c b/compat/mingw.c index 3ecde84..a8218e6 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -318,12 +318,12 @@ static inline int needs_hiding(const char *path) static int set_hidden_flag(const wchar_t *path, int set) { - DWORD attribs = GetFileAttributesW(path); + DWORD original = GetFileAttributesW(path), modified; if (set) - attribs |= FILE_ATTRIBUTE_HIDDEN; + modified = original | FILE_ATTRIBUTE_HIDDEN; else - attribs &= ~FILE_ATTRIBUTE_HIDDEN; - if (SetFileAttributesW(path, attribs)) + modified = original & ~FILE_ATTRIBUTE_HIDDEN; + if (original == modified || SetFileAttributesW(path, modified)) return 0; errno = err_win_to_posix(GetLastError()); return -1; @@ -377,7 +377,7 @@ int mingw_open (const char *filename, int oflags, ...) if (fd < 0 && errno == EACCES) fd = _wopen(wfilename, oflags & ~O_CREAT, mode); if (fd >= 0 && set_hidden_flag(wfilename, 1)) - warning("Could not mark '%s' as hidden.", filename); + warning("could not mark '%s' as hidden.", filename); } return fd; } @@ -419,12 +419,12 @@ FILE *mingw_fopen (const char *filename, const char *otype) xutftowcs(wotype, otype, ARRAY_SIZE(wotype)) < 0) return NULL; if (hide && !access(filename, F_OK) && set_hidden_flag(wfilename, 0)) { - error("Could not unhide %s", filename); + error("could not unhide %s", filename); return NULL; } file = _wfopen(wfilename, wotype); if (file && hide && set_hidden_flag(wfilename, 1)) - warning("Could not mark '%s' as hidden.", filename); + warning("could not mark '%s' as hidden.", filename); return file; } @@ -439,12 +439,12 @@ FILE *mingw_freopen (const char *filename, const char *otype, FILE *stream) xutftowcs(wotype, otype, ARRAY_SIZE(wotype)) < 0) return NULL; if (hide && !access(filename, F_OK) && set_hidden_flag(wfilename, 0)) { - error("Could not unhide %s", filename); + error("could not unhide %s", filename); return NULL; } file = _wfreopen(wfilename, wotype, stream); if (file && hide && set_hidden_flag(wfilename, 1)) - warning("Could not mark '%s' as hidden.", filename); + warning("could not mark '%s' as hidden.", filename); return file; } -- 2.8.2.463.g99156ee ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v3 1/2] mingw: introduce the 'core.hideDotFiles' setting 2016-05-10 11:59 ` [PATCH v3 " Johannes Schindelin @ 2016-05-10 11:59 ` Johannes Schindelin 2016-05-10 11:59 ` [PATCH v3 2/2] mingw: remove unnecessary definition Johannes Schindelin ` (2 subsequent siblings) 3 siblings, 0 replies; 27+ messages in thread From: Johannes Schindelin @ 2016-05-10 11:59 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Ramsay Jones, Erik Faye-Lund, Pat Thoyts On Unix (and Linux), files and directories whose names start with a dot are usually not shown by default. This convention is used by Git: the .git/ directory should be left alone by regular users, and only accessed through Git itself. On Windows, no such convention exists. Instead, there is an explicit flag to mark files or directories as hidden. In the early days, Git for Windows did not mark the .git/ directory (or for that matter, any file or directory whose name starts with a dot) hidden. This lead to quite a bit of confusion, and even loss of data. Consequently, Git for Windows introduced the core.hideDotFiles setting, with three possible values: true, false, and dotGitOnly, defaulting to marking only the .git/ directory as hidden. The rationale: users do not need to access .git/ directly, and indeed (as was demonstrated) should not really see that directory, either. However, not all dot files should be hidden by default, as e.g. Eclipse does not show them (and the user would therefore be unable to see, say, a .gitattributes file). In over five years since the last attempt to bring this patch into core Git, a slightly buggy version of this patch has served Git for Windows' users well: no single report indicated problems with the hidden .git/ directory, and the stream of problems caused by the previously non-hidden .git/ directory simply stopped. The bugs have been fixed during the process of getting this patch upstream. Note that there is a funny quirk we have to pay attention to when creating hidden files: we use Win32's _wopen() function which transmogrifies its arguments and hands off to Win32's CreateFile() function. That latter function errors out with ERROR_ACCESS_DENIED (the equivalent of EACCES) when the equivalent of the O_CREAT flag was passed and the file attributes (including the hidden flag) do not match an existing file's. And _wopen() accepts no parameter that would be transmogrified into said hidden flag. Therefore, we simply try again without O_CREAT. A slightly different method is required for our fopen()/freopen() function as we cannot even *remove* the implicit O_CREAT flag. Therefore, we briefly mark existing files as unhidden when opening them via fopen()/freopen(). The ERROR_ACCESS_DENIED error can also be triggered by opening a file that is marked as a system file (which is unlikely to be tracked in Git), and by trying to create a file that has *just* been deleted and is awaiting the last open handles to be released (which would be handled better by the "Try again?" logic, a story for a different patch series, though). In both cases, it does not matter much if we try again without the O_CREAT flag, read: it does not hurt, either. For details how ERROR_ACCESS_DENIED can be triggered, see https://msdn.microsoft.com/en-us/library/windows/desktop/aa363858 Original-patch-by: Erik Faye-Lund <kusmabite@gmail.com> Initial-Test-By: Pat Thoyts <patthoyts@users.sourceforge.net> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- Documentation/config.txt | 6 ++++ cache.h | 7 +++++ compat/mingw.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++ config.c | 8 ++++++ environment.c | 1 + t/t0001-init.sh | 30 ++++++++++++++++++++ t/t5611-clone-config.sh | 20 +++++++++++++ 7 files changed, 146 insertions(+) diff --git a/Documentation/config.txt b/Documentation/config.txt index f6a5106..e2bf0f8 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -279,6 +279,12 @@ See linkgit:git-update-index[1]. + The default is true (when core.filemode is not specified in the config file). +core.hideDotFiles:: + (Windows-only) If true, mark newly-created directories and files whose + name starts with a dot as hidden. If 'dotGitOnly', only the `.git/` + directory is hidden, but no other files starting with a dot. The + default mode is to mark only the `.git/` directory as hidden. + core.ignoreCase:: If true, this option enables various workarounds to enable Git to work better on filesystems that are not case sensitive, diff --git a/cache.h b/cache.h index ca23a39..5f72f59 100644 --- a/cache.h +++ b/cache.h @@ -701,6 +701,13 @@ extern int ref_paranoia; extern char comment_line_char; extern int auto_comment_line_char; +enum hide_dotfiles_type { + HIDE_DOTFILES_FALSE = 0, + HIDE_DOTFILES_TRUE, + HIDE_DOTFILES_DOTGITONLY +}; +extern enum hide_dotfiles_type hide_dotfiles; + enum branch_track { BRANCH_TRACK_UNSPECIFIED = -1, BRANCH_TRACK_NEVER = 0, diff --git a/compat/mingw.c b/compat/mingw.c index 0413d5c..a8218e6 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -286,6 +286,49 @@ int mingw_rmdir(const char *pathname) return ret; } +static inline int needs_hiding(const char *path) +{ + const char *basename; + + if (hide_dotfiles == HIDE_DOTFILES_FALSE) + return 0; + + /* We cannot use basename(), as it would remove trailing slashes */ + mingw_skip_dos_drive_prefix((char **)&path); + if (!*path) + return 0; + + for (basename = path; *path; path++) + if (is_dir_sep(*path)) { + do { + path++; + } while (is_dir_sep(*path)); + /* ignore trailing slashes */ + if (*path) + basename = path; + } + + if (hide_dotfiles == HIDE_DOTFILES_TRUE) + return *basename == '.'; + + assert(hide_dotfiles == HIDE_DOTFILES_DOTGITONLY); + return !strncasecmp(".git", basename, 4) && + (!basename[4] || is_dir_sep(basename[4])); +} + +static int set_hidden_flag(const wchar_t *path, int set) +{ + DWORD original = GetFileAttributesW(path), modified; + if (set) + modified = original | FILE_ATTRIBUTE_HIDDEN; + else + modified = original & ~FILE_ATTRIBUTE_HIDDEN; + if (original == modified || SetFileAttributesW(path, modified)) + return 0; + errno = err_win_to_posix(GetLastError()); + return -1; +} + int mingw_mkdir(const char *path, int mode) { int ret; @@ -293,6 +336,8 @@ int mingw_mkdir(const char *path, int mode) if (xutftowcs_path(wpath, path) < 0) return -1; ret = _wmkdir(wpath); + if (!ret && needs_hiding(path)) + return set_hidden_flag(wpath, 1); return ret; } @@ -319,6 +364,21 @@ int mingw_open (const char *filename, int oflags, ...) if (attrs != INVALID_FILE_ATTRIBUTES && (attrs & FILE_ATTRIBUTE_DIRECTORY)) errno = EISDIR; } + if ((oflags & O_CREAT) && needs_hiding(filename)) { + /* + * Internally, _wopen() uses the CreateFile() API which errors + * out with an ERROR_ACCESS_DENIED if CREATE_ALWAYS was + * specified and an already existing file's attributes do not + * match *exactly*. As there is no mode or flag we can set that + * would correspond to FILE_ATTRIBUTE_HIDDEN, let's just try + * again *without* the O_CREAT flag (that corresponds to the + * CREATE_ALWAYS flag of CreateFile()). + */ + if (fd < 0 && errno == EACCES) + fd = _wopen(wfilename, oflags & ~O_CREAT, mode); + if (fd >= 0 && set_hidden_flag(wfilename, 1)) + warning("could not mark '%s' as hidden.", filename); + } return fd; } @@ -350,6 +410,7 @@ int mingw_fgetc(FILE *stream) #undef fopen FILE *mingw_fopen (const char *filename, const char *otype) { + int hide = needs_hiding(filename); FILE *file; wchar_t wfilename[MAX_PATH], wotype[4]; if (filename && !strcmp(filename, "/dev/null")) @@ -357,12 +418,19 @@ FILE *mingw_fopen (const char *filename, const char *otype) if (xutftowcs_path(wfilename, filename) < 0 || xutftowcs(wotype, otype, ARRAY_SIZE(wotype)) < 0) return NULL; + if (hide && !access(filename, F_OK) && set_hidden_flag(wfilename, 0)) { + error("could not unhide %s", filename); + return NULL; + } file = _wfopen(wfilename, wotype); + if (file && hide && set_hidden_flag(wfilename, 1)) + warning("could not mark '%s' as hidden.", filename); return file; } FILE *mingw_freopen (const char *filename, const char *otype, FILE *stream) { + int hide = needs_hiding(filename); FILE *file; wchar_t wfilename[MAX_PATH], wotype[4]; if (filename && !strcmp(filename, "/dev/null")) @@ -370,7 +438,13 @@ FILE *mingw_freopen (const char *filename, const char *otype, FILE *stream) if (xutftowcs_path(wfilename, filename) < 0 || xutftowcs(wotype, otype, ARRAY_SIZE(wotype)) < 0) return NULL; + if (hide && !access(filename, F_OK) && set_hidden_flag(wfilename, 0)) { + error("could not unhide %s", filename); + return NULL; + } file = _wfreopen(wfilename, wotype, stream); + if (file && hide && set_hidden_flag(wfilename, 1)) + warning("could not mark '%s' as hidden.", filename); return file; } diff --git a/config.c b/config.c index d2cfca1..bb83137 100644 --- a/config.c +++ b/config.c @@ -911,6 +911,14 @@ static int git_default_core_config(const char *var, const char *value) return 0; } + if (!strcmp(var, "core.hidedotfiles")) { + if (value && !strcasecmp(value, "dotgitonly")) + hide_dotfiles = HIDE_DOTFILES_DOTGITONLY; + else + hide_dotfiles = git_config_bool(var, value); + return 0; + } + /* Add other config variables here and to Documentation/config.txt. */ return 0; } diff --git a/environment.c b/environment.c index 2857e3f..ca72464 100644 --- a/environment.c +++ b/environment.c @@ -64,6 +64,7 @@ int core_apply_sparse_checkout; int merge_log_config = -1; int precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */ unsigned long pack_size_limit_cfg; +enum hide_dotfiles_type hide_dotfiles = HIDE_DOTFILES_DOTGITONLY; #ifndef PROTECT_HFS_DEFAULT #define PROTECT_HFS_DEFAULT 0 diff --git a/t/t0001-init.sh b/t/t0001-init.sh index a5b9e7a..a6fdd5e 100755 --- a/t/t0001-init.sh +++ b/t/t0001-init.sh @@ -354,4 +354,34 @@ test_expect_success SYMLINKS 're-init to move gitdir symlink' ' test_path_is_dir realgitdir/refs ' +# Tests for the hidden file attribute on windows +is_hidden () { + # Use the output of `attrib`, ignore the absolute path + case "$(attrib "$1")" in *H*?:*) return 0;; esac + return 1 +} + +test_expect_success MINGW '.git hidden' ' + rm -rf newdir && + ( + unset GIT_DIR GIT_WORK_TREE + mkdir newdir && + cd newdir && + git init && + is_hidden .git + ) && + check_config newdir/.git false unset +' + +test_expect_success MINGW 'bare git dir not hidden' ' + rm -rf newdir && + ( + unset GIT_DIR GIT_WORK_TREE GIT_CONFIG + mkdir newdir && + cd newdir && + git --bare init + ) && + ! is_hidden newdir +' + test_done diff --git a/t/t5611-clone-config.sh b/t/t5611-clone-config.sh index 27d730c..e4850b7 100755 --- a/t/t5611-clone-config.sh +++ b/t/t5611-clone-config.sh @@ -37,4 +37,24 @@ test_expect_success 'clone -c config is available during clone' ' test_cmp expect child/file ' +# Tests for the hidden file attribute on windows +is_hidden () { + # Use the output of `attrib`, ignore the absolute path + case "$(attrib "$1")" in *H*?:*) return 0;; esac + return 1 +} + +test_expect_success MINGW 'clone -c core.hideDotFiles' ' + test_commit attributes .gitattributes "" && + rm -rf child && + git clone -c core.hideDotFiles=false . child && + ! is_hidden child/.gitattributes && + rm -rf child && + git clone -c core.hideDotFiles=dotGitOnly . child && + ! is_hidden child/.gitattributes && + rm -rf child && + git clone -c core.hideDotFiles=true . child && + is_hidden child/.gitattributes +' + test_done -- 2.8.2.463.g99156ee ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v3 2/2] mingw: remove unnecessary definition 2016-05-10 11:59 ` [PATCH v3 " Johannes Schindelin 2016-05-10 11:59 ` [PATCH v3 1/2] mingw: introduce the 'core.hideDotFiles' setting Johannes Schindelin @ 2016-05-10 11:59 ` Johannes Schindelin 2016-05-11 8:40 ` [PATCH v4 0/2] Support marking .git/ (or all files) as hidden on Windows Johannes Schindelin 2016-05-11 8:43 ` Johannes Schindelin 3 siblings, 0 replies; 27+ messages in thread From: Johannes Schindelin @ 2016-05-10 11:59 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Ramsay Jones, Erik Faye-Lund, Pat Thoyts For some reason, the definition of the MINGW version of `mark_as_git_dir()` slipped into this developer's patch series to support building Git for Windows. As the `mark_as_git_dir()` function is not needed at all anymore (it was used originally to support the core.hideDotFiles = gitDirOnly setting, but we now use a different method to support that case), let's just remove it. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- compat/mingw.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/compat/mingw.h b/compat/mingw.h index edec9e0..69bb43d 100644 --- a/compat/mingw.h +++ b/compat/mingw.h @@ -417,9 +417,6 @@ int mingw_offset_1st_component(const char *path); void mingw_open_html(const char *path); #define open_html mingw_open_html -void mingw_mark_as_git_dir(const char *dir); -#define mark_as_git_dir mingw_mark_as_git_dir - /** * Converts UTF-8 encoded string to UTF-16LE. * -- 2.8.2.463.g99156ee ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v4 0/2] Support marking .git/ (or all files) as hidden on Windows 2016-05-10 11:59 ` [PATCH v3 " Johannes Schindelin 2016-05-10 11:59 ` [PATCH v3 1/2] mingw: introduce the 'core.hideDotFiles' setting Johannes Schindelin 2016-05-10 11:59 ` [PATCH v3 2/2] mingw: remove unnecessary definition Johannes Schindelin @ 2016-05-11 8:40 ` Johannes Schindelin 2016-05-11 8:43 ` Johannes Schindelin 3 siblings, 0 replies; 27+ messages in thread From: Johannes Schindelin @ 2016-05-11 8:40 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Ramsay Jones, Erik Faye-Lund, Pat Thoyts Windows does not share Unix' convention that files and directories whose names start with a dot are hidden. Hence `.git/`, for example, is in plain view, and caused quite a bit of trouble: some users wanted to peek inside and did not understand what it contains, others modified files. There was a stream of bug reports, until Git for Windows introduced the (opt-out) option to hide at least the .git/ directory by default. The option is configured via the config setting core.hideDotFiles, with the possible values false, true and dotGitOnly (the latter being the default). This is a heavily version of patches we carried in Git for Windows for way too long without submitting them upstream. This iteration addresses Junio's most recent round of concerns. Johannes Schindelin (2): mingw: introduce the 'core.hideDotFiles' setting mingw: remove unnecessary definition Documentation/config.txt | 6 ++++ cache.h | 7 +++++ compat/mingw.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++ compat/mingw.h | 3 -- config.c | 8 ++++++ environment.c | 1 + t/t0001-init.sh | 30 ++++++++++++++++++++ t/t5611-clone-config.sh | 20 +++++++++++++ 8 files changed, 146 insertions(+), 3 deletions(-) Published-As: https://github.com/dscho/git/releases/tag/hide-dotgit-v4 Interdiff vs v3: diff --git a/Documentation/config.txt b/Documentation/config.txt index 10c0088..acd0b56 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -273,7 +273,7 @@ core.hideDotFiles:: (Windows-only) If true, mark newly-created directories and files whose name starts with a dot as hidden. If 'dotGitOnly', only the `.git/` directory is hidden, but no other files starting with a dot. The - default mode is to mark only the `.git/` directory as hidden. + default mode is 'dotGitOnly'. core.ignoreCase:: If true, this option enables various workarounds to enable -- 2.8.2.465.gb077790 ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v4 0/2] Support marking .git/ (or all files) as hidden on Windows 2016-05-10 11:59 ` [PATCH v3 " Johannes Schindelin ` (2 preceding siblings ...) 2016-05-11 8:40 ` [PATCH v4 0/2] Support marking .git/ (or all files) as hidden on Windows Johannes Schindelin @ 2016-05-11 8:43 ` Johannes Schindelin 2016-05-11 8:43 ` [PATCH v4 1/2] mingw: introduce the 'core.hideDotFiles' setting Johannes Schindelin 2016-05-11 8:43 ` [PATCH v4 2/2] mingw: remove unnecessary definition Johannes Schindelin 3 siblings, 2 replies; 27+ messages in thread From: Johannes Schindelin @ 2016-05-11 8:43 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Ramsay Jones, Erik Faye-Lund, Pat Thoyts Windows does not share Unix' convention that files and directories whose names start with a dot are hidden. Hence `.git/`, for example, is in plain view, and caused quite a bit of trouble: some users wanted to peek inside and did not understand what it contains, others modified files. There was a stream of bug reports, until Git for Windows introduced the (opt-out) option to hide at least the .git/ directory by default. The option is configured via the config setting core.hideDotFiles, with the possible values false, true and dotGitOnly (the latter being the default). This is a heavily version of patches we carried in Git for Windows for way too long without submitting them upstream. This iteration addresses Junio's most recent round of concerns. Johannes Schindelin (2): mingw: introduce the 'core.hideDotFiles' setting mingw: remove unnecessary definition Documentation/config.txt | 6 ++++ cache.h | 7 +++++ compat/mingw.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++ compat/mingw.h | 3 -- config.c | 8 ++++++ environment.c | 1 + t/t0001-init.sh | 30 ++++++++++++++++++++ t/t5611-clone-config.sh | 20 +++++++++++++ 8 files changed, 146 insertions(+), 3 deletions(-) Published-As: https://github.com/dscho/git/releases/tag/hide-dotgit-v4 Interdiff vs v3: diff --git a/Documentation/config.txt b/Documentation/config.txt index 10c0088..acd0b56 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -273,7 +273,7 @@ core.hideDotFiles:: (Windows-only) If true, mark newly-created directories and files whose name starts with a dot as hidden. If 'dotGitOnly', only the `.git/` directory is hidden, but no other files starting with a dot. The - default mode is to mark only the `.git/` directory as hidden. + default mode is 'dotGitOnly'. core.ignoreCase:: If true, this option enables various workarounds to enable -- 2.8.2.465.gb077790 ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v4 1/2] mingw: introduce the 'core.hideDotFiles' setting 2016-05-11 8:43 ` Johannes Schindelin @ 2016-05-11 8:43 ` Johannes Schindelin 2016-05-11 8:43 ` [PATCH v4 2/2] mingw: remove unnecessary definition Johannes Schindelin 1 sibling, 0 replies; 27+ messages in thread From: Johannes Schindelin @ 2016-05-11 8:43 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Ramsay Jones, Erik Faye-Lund, Pat Thoyts On Unix (and Linux), files and directories whose names start with a dot are usually not shown by default. This convention is used by Git: the .git/ directory should be left alone by regular users, and only accessed through Git itself. On Windows, no such convention exists. Instead, there is an explicit flag to mark files or directories as hidden. In the early days, Git for Windows did not mark the .git/ directory (or for that matter, any file or directory whose name starts with a dot) hidden. This lead to quite a bit of confusion, and even loss of data. Consequently, Git for Windows introduced the core.hideDotFiles setting, with three possible values: true, false, and dotGitOnly, defaulting to marking only the .git/ directory as hidden. The rationale: users do not need to access .git/ directly, and indeed (as was demonstrated) should not really see that directory, either. However, not all dot files should be hidden by default, as e.g. Eclipse does not show them (and the user would therefore be unable to see, say, a .gitattributes file). In over five years since the last attempt to bring this patch into core Git, a slightly buggy version of this patch has served Git for Windows' users well: no single report indicated problems with the hidden .git/ directory, and the stream of problems caused by the previously non-hidden .git/ directory simply stopped. The bugs have been fixed during the process of getting this patch upstream. Note that there is a funny quirk we have to pay attention to when creating hidden files: we use Win32's _wopen() function which transmogrifies its arguments and hands off to Win32's CreateFile() function. That latter function errors out with ERROR_ACCESS_DENIED (the equivalent of EACCES) when the equivalent of the O_CREAT flag was passed and the file attributes (including the hidden flag) do not match an existing file's. And _wopen() accepts no parameter that would be transmogrified into said hidden flag. Therefore, we simply try again without O_CREAT. A slightly different method is required for our fopen()/freopen() function as we cannot even *remove* the implicit O_CREAT flag. Therefore, we briefly mark existing files as unhidden when opening them via fopen()/freopen(). The ERROR_ACCESS_DENIED error can also be triggered by opening a file that is marked as a system file (which is unlikely to be tracked in Git), and by trying to create a file that has *just* been deleted and is awaiting the last open handles to be released (which would be handled better by the "Try again?" logic, a story for a different patch series, though). In both cases, it does not matter much if we try again without the O_CREAT flag, read: it does not hurt, either. For details how ERROR_ACCESS_DENIED can be triggered, see https://msdn.microsoft.com/en-us/library/windows/desktop/aa363858 Original-patch-by: Erik Faye-Lund <kusmabite@gmail.com> Initial-Test-By: Pat Thoyts <patthoyts@users.sourceforge.net> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- Documentation/config.txt | 6 ++++ cache.h | 7 +++++ compat/mingw.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++ config.c | 8 ++++++ environment.c | 1 + t/t0001-init.sh | 30 ++++++++++++++++++++ t/t5611-clone-config.sh | 20 +++++++++++++ 7 files changed, 146 insertions(+) diff --git a/Documentation/config.txt b/Documentation/config.txt index ece0acd..acd0b56 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -269,6 +269,12 @@ See linkgit:git-update-index[1]. + The default is true (when core.filemode is not specified in the config file). +core.hideDotFiles:: + (Windows-only) If true, mark newly-created directories and files whose + name starts with a dot as hidden. If 'dotGitOnly', only the `.git/` + directory is hidden, but no other files starting with a dot. The + default mode is 'dotGitOnly'. + core.ignoreCase:: If true, this option enables various workarounds to enable Git to work better on filesystems that are not case sensitive, diff --git a/cache.h b/cache.h index 160f8e3..6d41b53 100644 --- a/cache.h +++ b/cache.h @@ -700,6 +700,13 @@ extern int ref_paranoia; extern char comment_line_char; extern int auto_comment_line_char; +enum hide_dotfiles_type { + HIDE_DOTFILES_FALSE = 0, + HIDE_DOTFILES_TRUE, + HIDE_DOTFILES_DOTGITONLY +}; +extern enum hide_dotfiles_type hide_dotfiles; + enum branch_track { BRANCH_TRACK_UNSPECIFIED = -1, BRANCH_TRACK_NEVER = 0, diff --git a/compat/mingw.c b/compat/mingw.c index 0413d5c..a8218e6 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -286,6 +286,49 @@ int mingw_rmdir(const char *pathname) return ret; } +static inline int needs_hiding(const char *path) +{ + const char *basename; + + if (hide_dotfiles == HIDE_DOTFILES_FALSE) + return 0; + + /* We cannot use basename(), as it would remove trailing slashes */ + mingw_skip_dos_drive_prefix((char **)&path); + if (!*path) + return 0; + + for (basename = path; *path; path++) + if (is_dir_sep(*path)) { + do { + path++; + } while (is_dir_sep(*path)); + /* ignore trailing slashes */ + if (*path) + basename = path; + } + + if (hide_dotfiles == HIDE_DOTFILES_TRUE) + return *basename == '.'; + + assert(hide_dotfiles == HIDE_DOTFILES_DOTGITONLY); + return !strncasecmp(".git", basename, 4) && + (!basename[4] || is_dir_sep(basename[4])); +} + +static int set_hidden_flag(const wchar_t *path, int set) +{ + DWORD original = GetFileAttributesW(path), modified; + if (set) + modified = original | FILE_ATTRIBUTE_HIDDEN; + else + modified = original & ~FILE_ATTRIBUTE_HIDDEN; + if (original == modified || SetFileAttributesW(path, modified)) + return 0; + errno = err_win_to_posix(GetLastError()); + return -1; +} + int mingw_mkdir(const char *path, int mode) { int ret; @@ -293,6 +336,8 @@ int mingw_mkdir(const char *path, int mode) if (xutftowcs_path(wpath, path) < 0) return -1; ret = _wmkdir(wpath); + if (!ret && needs_hiding(path)) + return set_hidden_flag(wpath, 1); return ret; } @@ -319,6 +364,21 @@ int mingw_open (const char *filename, int oflags, ...) if (attrs != INVALID_FILE_ATTRIBUTES && (attrs & FILE_ATTRIBUTE_DIRECTORY)) errno = EISDIR; } + if ((oflags & O_CREAT) && needs_hiding(filename)) { + /* + * Internally, _wopen() uses the CreateFile() API which errors + * out with an ERROR_ACCESS_DENIED if CREATE_ALWAYS was + * specified and an already existing file's attributes do not + * match *exactly*. As there is no mode or flag we can set that + * would correspond to FILE_ATTRIBUTE_HIDDEN, let's just try + * again *without* the O_CREAT flag (that corresponds to the + * CREATE_ALWAYS flag of CreateFile()). + */ + if (fd < 0 && errno == EACCES) + fd = _wopen(wfilename, oflags & ~O_CREAT, mode); + if (fd >= 0 && set_hidden_flag(wfilename, 1)) + warning("could not mark '%s' as hidden.", filename); + } return fd; } @@ -350,6 +410,7 @@ int mingw_fgetc(FILE *stream) #undef fopen FILE *mingw_fopen (const char *filename, const char *otype) { + int hide = needs_hiding(filename); FILE *file; wchar_t wfilename[MAX_PATH], wotype[4]; if (filename && !strcmp(filename, "/dev/null")) @@ -357,12 +418,19 @@ FILE *mingw_fopen (const char *filename, const char *otype) if (xutftowcs_path(wfilename, filename) < 0 || xutftowcs(wotype, otype, ARRAY_SIZE(wotype)) < 0) return NULL; + if (hide && !access(filename, F_OK) && set_hidden_flag(wfilename, 0)) { + error("could not unhide %s", filename); + return NULL; + } file = _wfopen(wfilename, wotype); + if (file && hide && set_hidden_flag(wfilename, 1)) + warning("could not mark '%s' as hidden.", filename); return file; } FILE *mingw_freopen (const char *filename, const char *otype, FILE *stream) { + int hide = needs_hiding(filename); FILE *file; wchar_t wfilename[MAX_PATH], wotype[4]; if (filename && !strcmp(filename, "/dev/null")) @@ -370,7 +438,13 @@ FILE *mingw_freopen (const char *filename, const char *otype, FILE *stream) if (xutftowcs_path(wfilename, filename) < 0 || xutftowcs(wotype, otype, ARRAY_SIZE(wotype)) < 0) return NULL; + if (hide && !access(filename, F_OK) && set_hidden_flag(wfilename, 0)) { + error("could not unhide %s", filename); + return NULL; + } file = _wfreopen(wfilename, wotype, stream); + if (file && hide && set_hidden_flag(wfilename, 1)) + warning("could not mark '%s' as hidden.", filename); return file; } diff --git a/config.c b/config.c index 262d8d7..127d9f0 100644 --- a/config.c +++ b/config.c @@ -912,6 +912,14 @@ static int git_default_core_config(const char *var, const char *value) return 0; } + if (!strcmp(var, "core.hidedotfiles")) { + if (value && !strcasecmp(value, "dotgitonly")) + hide_dotfiles = HIDE_DOTFILES_DOTGITONLY; + else + hide_dotfiles = git_config_bool(var, value); + return 0; + } + /* Add other config variables here and to Documentation/config.txt. */ return 0; } diff --git a/environment.c b/environment.c index 57acb2f..96160a7 100644 --- a/environment.c +++ b/environment.c @@ -63,6 +63,7 @@ int core_apply_sparse_checkout; int merge_log_config = -1; int precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */ unsigned long pack_size_limit_cfg; +enum hide_dotfiles_type hide_dotfiles = HIDE_DOTFILES_DOTGITONLY; #ifndef PROTECT_HFS_DEFAULT #define PROTECT_HFS_DEFAULT 0 diff --git a/t/t0001-init.sh b/t/t0001-init.sh index a5b9e7a..a6fdd5e 100755 --- a/t/t0001-init.sh +++ b/t/t0001-init.sh @@ -354,4 +354,34 @@ test_expect_success SYMLINKS 're-init to move gitdir symlink' ' test_path_is_dir realgitdir/refs ' +# Tests for the hidden file attribute on windows +is_hidden () { + # Use the output of `attrib`, ignore the absolute path + case "$(attrib "$1")" in *H*?:*) return 0;; esac + return 1 +} + +test_expect_success MINGW '.git hidden' ' + rm -rf newdir && + ( + unset GIT_DIR GIT_WORK_TREE + mkdir newdir && + cd newdir && + git init && + is_hidden .git + ) && + check_config newdir/.git false unset +' + +test_expect_success MINGW 'bare git dir not hidden' ' + rm -rf newdir && + ( + unset GIT_DIR GIT_WORK_TREE GIT_CONFIG + mkdir newdir && + cd newdir && + git --bare init + ) && + ! is_hidden newdir +' + test_done diff --git a/t/t5611-clone-config.sh b/t/t5611-clone-config.sh index 27d730c..e4850b7 100755 --- a/t/t5611-clone-config.sh +++ b/t/t5611-clone-config.sh @@ -37,4 +37,24 @@ test_expect_success 'clone -c config is available during clone' ' test_cmp expect child/file ' +# Tests for the hidden file attribute on windows +is_hidden () { + # Use the output of `attrib`, ignore the absolute path + case "$(attrib "$1")" in *H*?:*) return 0;; esac + return 1 +} + +test_expect_success MINGW 'clone -c core.hideDotFiles' ' + test_commit attributes .gitattributes "" && + rm -rf child && + git clone -c core.hideDotFiles=false . child && + ! is_hidden child/.gitattributes && + rm -rf child && + git clone -c core.hideDotFiles=dotGitOnly . child && + ! is_hidden child/.gitattributes && + rm -rf child && + git clone -c core.hideDotFiles=true . child && + is_hidden child/.gitattributes +' + test_done -- 2.8.2.465.gb077790 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v4 2/2] mingw: remove unnecessary definition 2016-05-11 8:43 ` Johannes Schindelin 2016-05-11 8:43 ` [PATCH v4 1/2] mingw: introduce the 'core.hideDotFiles' setting Johannes Schindelin @ 2016-05-11 8:43 ` Johannes Schindelin 1 sibling, 0 replies; 27+ messages in thread From: Johannes Schindelin @ 2016-05-11 8:43 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Ramsay Jones, Erik Faye-Lund, Pat Thoyts For some reason, the definition of the MINGW version of `mark_as_git_dir()` slipped into this developer's patch series to support building Git for Windows. As the `mark_as_git_dir()` function is not needed at all anymore (it was used originally to support the core.hideDotFiles = gitDirOnly setting, but we now use a different method to support that case), let's just remove it. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- compat/mingw.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/compat/mingw.h b/compat/mingw.h index 1de70ff..a1808b4 100644 --- a/compat/mingw.h +++ b/compat/mingw.h @@ -416,9 +416,6 @@ int mingw_offset_1st_component(const char *path); void mingw_open_html(const char *path); #define open_html mingw_open_html -void mingw_mark_as_git_dir(const char *dir); -#define mark_as_git_dir mingw_mark_as_git_dir - /** * Converts UTF-8 encoded string to UTF-16LE. * -- 2.8.2.465.gb077790 ^ permalink raw reply related [flat|nested] 27+ messages in thread
end of thread, other threads:[~2016-05-11 8:46 UTC | newest] Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-05-04 14:40 [PATCH] mingw: introduce the 'core.hideDotFiles' setting Johannes Schindelin 2016-05-04 16:18 ` Ramsay Jones 2016-05-06 12:06 ` Johannes Schindelin 2016-05-04 19:06 ` Junio C Hamano 2016-05-06 15:19 ` Johannes Schindelin 2016-05-06 16:34 ` Junio C Hamano 2016-05-06 17:17 ` Junio C Hamano 2016-05-07 6:01 ` Johannes Schindelin 2016-05-07 6:44 ` Johannes Schindelin 2016-05-07 6:44 ` [PATCH v2 0/2] Support marking .git/ (or all files) as hidden on Windows Johannes Schindelin 2016-05-07 6:45 ` [PATCH v2 1/2] mingw: introduce the 'core.hideDotFiles' setting Johannes Schindelin 2016-05-09 17:23 ` Junio C Hamano 2016-05-10 11:58 ` Johannes Schindelin 2016-05-10 17:19 ` Junio C Hamano 2016-05-11 8:40 ` Johannes Schindelin 2016-05-07 6:45 ` [PATCH v2 2/2] mingw: remove unnecessary definition Johannes Schindelin 2016-05-09 17:01 ` [PATCH v2 0/2] Support marking .git/ (or all files) as hidden on Windows Junio C Hamano 2016-05-10 8:41 ` Johannes Schindelin 2016-05-10 17:22 ` Junio C Hamano 2016-05-11 8:34 ` Johannes Schindelin 2016-05-10 11:59 ` [PATCH v3 " Johannes Schindelin 2016-05-10 11:59 ` [PATCH v3 1/2] mingw: introduce the 'core.hideDotFiles' setting Johannes Schindelin 2016-05-10 11:59 ` [PATCH v3 2/2] mingw: remove unnecessary definition Johannes Schindelin 2016-05-11 8:40 ` [PATCH v4 0/2] Support marking .git/ (or all files) as hidden on Windows Johannes Schindelin 2016-05-11 8:43 ` Johannes Schindelin 2016-05-11 8:43 ` [PATCH v4 1/2] mingw: introduce the 'core.hideDotFiles' setting Johannes Schindelin 2016-05-11 8:43 ` [PATCH v4 2/2] mingw: remove unnecessary definition Johannes Schindelin
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.