[PATCH] mingw: introduce the 'core.hideDotFiles' setting

classic Classic list List threaded Threaded
27 messages Options
12
Reply | Threaded
Open this post in threaded view
|

[PATCH] mingw: introduce the 'core.hideDotFiles' setting

Johannes Schindelin
From: Erik Faye-Lund <[hidden email]>

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 <[hidden email]>
Signed-off-by: Erik Faye-Lund <[hidden email]>
Signed-off-by: Johannes Schindelin <[hidden email]>
---

        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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] mingw: introduce the 'core.hideDotFiles' setting

Ramsay Jones-2


On 04/05/16 15:40, Johannes Schindelin wrote:

> From: Erik Faye-Lund <[hidden email]>
>
> 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 <[hidden email]>
> Signed-off-by: Erik Faye-Lund <[hidden email]>
> Signed-off-by: Johannes Schindelin <[hidden email]>
> ---
>
> 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


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] mingw: introduce the 'core.hideDotFiles' setting

Junio C Hamano
In reply to this post by Johannes Schindelin
Johannes Schindelin <[hidden email]> 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?

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] mingw: introduce the 'core.hideDotFiles' setting

Johannes Schindelin
In reply to this post by Ramsay Jones-2
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 [hidden email]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] mingw: introduce the 'core.hideDotFiles' setting

Johannes Schindelin
In reply to this post by Junio C Hamano
Hi Junio,

On Wed, 4 May 2016, Junio C Hamano wrote:

> Johannes Schindelin <[hidden email]> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] mingw: introduce the 'core.hideDotFiles' setting

Junio C Hamano
Johannes Schindelin <[hidden email]> 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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] mingw: introduce the 'core.hideDotFiles' setting

Junio C Hamano
Junio C Hamano <[hidden email]> 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".
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] mingw: introduce the 'core.hideDotFiles' setting

Johannes Schindelin
Hi Junio,

On Fri, 6 May 2016, Junio C Hamano wrote:

> Junio C Hamano <[hidden email]> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] mingw: introduce the 'core.hideDotFiles' setting

Johannes Schindelin
In reply to this post by Junio C Hamano
Hi Junio,

On Fri, 6 May 2016, Junio C Hamano wrote:

> Johannes Schindelin <[hidden email]> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 0/2] Support marking .git/ (or all files) as hidden on Windows

Johannes Schindelin
In reply to this post by Johannes Schindelin
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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 1/2] mingw: introduce the 'core.hideDotFiles' setting

Johannes Schindelin
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 <[hidden email]>
Initial-Test-By: Pat Thoyts <[hidden email]>
Signed-off-by: Johannes Schindelin <[hidden email]>
---
 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


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 2/2] mingw: remove unnecessary definition

Johannes Schindelin
In reply to this post by Johannes Schindelin
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 <[hidden email]>
---
 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 0/2] Support marking .git/ (or all files) as hidden on Windows

Junio C Hamano
In reply to this post by Johannes Schindelin
Johannes Schindelin <[hidden email]> 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?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 1/2] mingw: introduce the 'core.hideDotFiles' setting

Junio C Hamano
In reply to this post by Johannes Schindelin
Johannes Schindelin <[hidden email]> 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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 0/2] Support marking .git/ (or all files) as hidden on Windows

Johannes Schindelin
In reply to this post by Junio C Hamano
Hi Junio,

On Mon, 9 May 2016, Junio C Hamano wrote:

> Johannes Schindelin <[hidden email]> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 1/2] mingw: introduce the 'core.hideDotFiles' setting

Johannes Schindelin
In reply to this post by Junio C Hamano
Hi Junio,

On Mon, 9 May 2016, Junio C Hamano wrote:

> Johannes Schindelin <[hidden email]> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

[PATCH v3 0/2] Support marking .git/ (or all files) as hidden on Windows

Johannes Schindelin
In reply to this post by Johannes Schindelin
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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

[PATCH v3 1/2] mingw: introduce the 'core.hideDotFiles' setting

Johannes Schindelin
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 <[hidden email]>
Initial-Test-By: Pat Thoyts <[hidden email]>
Signed-off-by: Johannes Schindelin <[hidden email]>
---
 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


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

[PATCH v3 2/2] mingw: remove unnecessary definition

Johannes Schindelin
In reply to this post by Johannes Schindelin
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 <[hidden email]>
---
 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 1/2] mingw: introduce the 'core.hideDotFiles' setting

Junio C Hamano
In reply to this post by Johannes Schindelin
Johannes Schindelin <[hidden email]> writes:

> On Mon, 9 May 2016, Junio C Hamano wrote:
>
>> Johannes Schindelin <[hidden email]> 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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
12