Merge pull request #2358 from dscho/reconcile-system-config-and-programdata-config

Make `git config --system` work like you think it should on Windows

Author: dscho

Documentation/config.txt

@@ -7,9 +7,7 @@ the Git commands' behavior. The files `.git/config` and optionally
 repository are used to store the configuration for that repository, and
 `$HOME/.gitconfig` is used to store a per-user configuration as
 fallback values for the `.git/config` file. The file `/etc/gitconfig`
-can be used to store a system-wide default configuration. On Windows,
-configuration can also be stored in `C:\ProgramData\Git\config`; This
-file will be used also by libgit2-based software.
+can be used to store a system-wide default configuration.
 
 The configuration variables are used by both the Git plumbing
 and the porcelains. The variables are divided into sections, wherein

Documentation/git-config.txt

@@ -272,16 +272,8 @@ FILES
 If not set explicitly with `--file`, there are four files where
 'git config' will search for configuration options:
 
-$PROGRAMDATA/Git/config::
-	(Windows-only) System-wide configuration file shared with other Git
-	implementations. Typically `$PROGRAMDATA` points to `C:\ProgramData`.
-
 $(prefix)/etc/gitconfig::
 	System-wide configuration file.
-	(Windows-only) This file contains only the settings which are
-	specific for this installation of Git for Windows and which should
-	not be shared with other Git implementations like JGit, libgit2.
-	`--system` will select this file.
 
 $XDG_CONFIG_HOME/git/config::
 	Second user-specific configuration file. If $XDG_CONFIG_HOME is not set

Documentation/git.txt

@@ -596,8 +596,7 @@ for further details.
 
 `GIT_CONFIG_NOSYSTEM`::
 	Whether to skip reading settings from the system-wide
-	`$(prefix)/etc/gitconfig` file (and on Windows, also from the
-	`%PROGRAMDATA%\Git\config` file).  This environment variable can
+	`$(prefix)/etc/gitconfig` file.  This environment variable can
 	be used along with `$HOME` and `$XDG_CONFIG_HOME` to create a
 	predictable environment for a picky script, or you can set it
 	temporarily to avoid using a buggy `/etc/gitconfig` file while

compat/mingw.c

@@ -2,8 +2,6 @@
 #include "win32.h"
 #include <conio.h>
 #include <wchar.h>
-#include <aclapi.h>
-#include <sddl.h>
 #include <winioctl.h>
 #include "../strbuf.h"
 #include "../run-command.h"
@@ -3299,188 +3297,6 @@ int uname(struct utsname *buf)
 	return 0;
 }
 
-/*
- * Determines whether the SID refers to an administrator or the current user.
- *
- * For convenience, the `info` parameter allows avoiding multiple calls to
- * `OpenProcessToken()` if this function is called more than once. The initial
- * value of `*info` is expected to be `NULL`, and it needs to be released via
- * `free()` after the last call to this function.
- *
- * Returns 0 if the SID indicates a dubious owner of system files, otherwise 1.
- */
-static int is_valid_system_file_owner(PSID sid, TOKEN_USER **info)
-{
-	HANDLE token;
-	DWORD len;
-	char builtin_administrators_sid[SECURITY_MAX_SID_SIZE];
-
-	if (IsWellKnownSid(sid, WinBuiltinAdministratorsSid) ||
-	    IsWellKnownSid(sid, WinLocalSystemSid))
-		return 1;
-
-	/* Obtain current user's SID */
-	if (!*info &&
-	    OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &token)) {
-		if (!GetTokenInformation(token, TokenUser, NULL, 0, &len)) {
-			*info = xmalloc((size_t)len);
-			if (!GetTokenInformation(token, TokenUser, *info, len,
-						 &len))
-				FREE_AND_NULL(*info);
-		}
-		CloseHandle(token);
-	}
-
-	if (*info && EqualSid(sid, (*info)->User.Sid))
-		return 1;
-
-	/* Is the owner at least a member of BUILTIN\Administrators? */
-	len = ARRAY_SIZE(builtin_administrators_sid);
-	if (CreateWellKnownSid(WinBuiltinAdministratorsSid, NULL,
-			       builtin_administrators_sid, &len)) {
-		wchar_t name[256], domain[256];
-		DWORD name_size = ARRAY_SIZE(name);
-		DWORD domain_size = ARRAY_SIZE(domain);
-		SID_NAME_USE type;
-		PSID *members;
-		DWORD dummy, i;
-		/*
-		 * We avoid including the `lm.h` header and linking to
-		 * `netapi32.dll` directly, in favor of lazy-loading that DLL
-		 * when, and _only_ when, needed.
-		 */
-		DECLARE_PROC_ADDR(netapi32.dll, DWORD,
-				  NetLocalGroupGetMembers, LPCWSTR,
-				  LPCWSTR, DWORD, LPVOID, DWORD,
-				  LPDWORD, LPDWORD, PDWORD_PTR);
-		DECLARE_PROC_ADDR(netapi32.dll, DWORD,
-				  NetApiBufferFree, LPVOID);
-
-		if (LookupAccountSidW(NULL, builtin_administrators_sid,
-				      name, &name_size, domain, &domain_size,
-				      &type) &&
-		    INIT_PROC_ADDR(NetLocalGroupGetMembers) &&
-		    /*
-		     * Technically, `NetLocalGroupGetMembers()` wants to assign
-		     * an array of type `LOCALGROUP_MEMBERS_INFO_0`, which
-		     * however contains only one field of type `PSID`,
-		     * therefore we can pretend that it is an array over the
-		     * type `PSID`.
-		     *
-		     * Also, we simply ignore the condition where
-		     * `ERROR_MORE_DATA` is returned; This should not happen
-		     * anyway, as we are passing `-1` as `prefmaxlen`
-		     * parameter, which is equivalent to the constant
-		     * `MAX_PREFERRED_LENGTH`.
-		     */
-		    !NetLocalGroupGetMembers(NULL, name, 0, &members, -1,
-					     &len, &dummy, NULL)) {
-			for (i = 0; i < len; i++)
-				if (EqualSid(sid, members[i]))
-					break;
-
-			if (INIT_PROC_ADDR(NetApiBufferFree))
-				NetApiBufferFree(members);
-
-			/* Did we find the `sid` in the members? */
-			return i < len;
-		}
-	}
-
-	return 0;
-}
-
-/*
- * Verify that the file in question is owned by an administrator or system
- * account, or at least by the current user.
- *
- * This function returns 1 if successful, 0 if the file is not owned by any of
- * these, or -1 on error.
- */
-static int validate_system_file_ownership(const char *path)
-{
-	WCHAR wpath[MAX_LONG_PATH];
-	PSID owner_sid = NULL, problem_sid = NULL;
-	PSECURITY_DESCRIPTOR descriptor = NULL;
-	TOKEN_USER* info = NULL;
-	DWORD err;
-	int ret;
-
-	if (xutftowcs_long_path(wpath, path) < 0)
-		return -1;
-
-	err = GetNamedSecurityInfoW(wpath, SE_FILE_OBJECT,
-				    OWNER_SECURITY_INFORMATION |
-				    DACL_SECURITY_INFORMATION,
-				    &owner_sid, NULL, NULL, NULL, &descriptor);
-
-	/* if the file does not exist, it does not have a valid owner */
-	if (err == ERROR_FILE_NOT_FOUND || err == ERROR_PATH_NOT_FOUND)
-		ret = 0;
-	else if (err != ERROR_SUCCESS)
-		ret = error(_("failed to validate '%s' (%ld)"), path, err);
-	else if (!IsValidSid(owner_sid))
-		ret = error(_("invalid owner: '%s'"), path);
-	else if (is_valid_system_file_owner(owner_sid, &info))
-		ret = 1;
-	else {
-		ret = 0;
-		problem_sid = owner_sid;
-	}
-
-	if (!ret && problem_sid) {
-#define MAX_NAME_OR_DOMAIN 256
-		wchar_t name[MAX_NAME_OR_DOMAIN];
-		wchar_t domain[MAX_NAME_OR_DOMAIN];
-		wchar_t *p = NULL;
-		DWORD size = MAX_NAME_OR_DOMAIN;
-		SID_NAME_USE type;
-		char utf[3 * MAX_NAME_OR_DOMAIN + 1];
-
-		if (!LookupAccountSidW(NULL, problem_sid, name, &size,
-				       domain, &size, &type) ||
-		    xwcstoutf(utf, name, ARRAY_SIZE(utf)) < 0) {
-			if (!ConvertSidToStringSidW(problem_sid, &p))
-				strlcpy(utf, "(unknown)", ARRAY_SIZE(utf));
-			else {
-				if (xwcstoutf(utf, p, ARRAY_SIZE(utf)) < 0)
-					strlcpy(utf, "(some user)",
-						ARRAY_SIZE(utf));
-				LocalFree(p);
-			}
-		}
-
-		warning(_("'%s' has a dubious owner: '%s'.\n"
-			  "For security reasons, it is therefore ignored.\n"
-			  "To fix this, please transfer ownership to an "
-			  "admininstrator."),
-			path, utf);
-	}
-
-	if (descriptor)
-		LocalFree(descriptor);
-	free(info);
-
-	return ret;
-}
-
-const char *program_data_config(void)
-{
-	static struct strbuf path = STRBUF_INIT;
-	static unsigned initialized;
-
-	if (!initialized) {
-		const char *env = mingw_getenv("PROGRAMDATA");
-		if (env) {
-			strbuf_addf(&path, "%s/Git/config", env);
-			if (validate_system_file_ownership(path.buf) != 1)
-				strbuf_setlen(&path, 0);
-		}
-		initialized = 1;
-	}
-	return *path.buf ? path.buf : NULL;
-}
-
 /*
  * Based on https://stackoverflow.com/questions/43002803
  *

compat/mingw.h

@@ -471,8 +471,6 @@ extern int (*win32_is_mount_point)(struct strbuf *path);
 #define PATH_SEP ';'
 extern char *mingw_query_user_email(void);
 #define query_user_email mingw_query_user_email
-extern const char *program_data_config(void);
-#define git_program_data_config program_data_config
 #if !defined(__MINGW64_VERSION_MAJOR) && (!defined(_MSC_VER) || _MSC_VER < 1800)
 #define PRIuMAX "I64u"
 #define PRId64 "I64d"

config.c

@@ -1711,16 +1711,11 @@ static int do_git_config_sequence(const struct config_options *opts,
 		repo_config = NULL;
 
 	current_parsing_scope = CONFIG_SCOPE_SYSTEM;
-	if (git_config_system()) {
-		int flags = opts->system_gently ? ACCESS_EACCES_OK : 0;
-		const char *program_data = git_program_data_config();
-		const char *etc = git_etc_gitconfig();
-
-		if (program_data && !access_or_die(program_data, R_OK, flags))
-			ret += git_config_from_file(fn, program_data, data);
-		if (!access_or_die(etc, R_OK, flags))
-			ret += git_config_from_file(fn, etc, data);
-	}
+	if (git_config_system() && !access_or_die(git_etc_gitconfig(), R_OK,
+						  opts->system_gently ?
+						  ACCESS_EACCES_OK : 0))
+		ret += git_config_from_file(fn, git_etc_gitconfig(),
+					    data);
 
 	current_parsing_scope = CONFIG_SCOPE_GLOBAL;
 	if (xdg_config && !access_or_die(xdg_config, R_OK, ACCESS_EACCES_OK))

config.mak.uname

@@ -424,6 +424,11 @@ ifeq ($(uname_S),Windows)
 	NO_POSIX_GOODIES = UnfortunatelyYes
 	NATIVE_CRLF = YesPlease
 	DEFAULT_HELP_FORMAT = html
+ifeq (/mingw64,$(subst 32,64,$(prefix)))
+	# Move system config into top-level /etc/
+	ETC_GITCONFIG = ../etc/gitconfig
+	ETC_GITATTRIBUTES = ../etc/gitattributes
+endif
 
 	CC = compat/vcbuild/scripts/clink.pl
 	AR = compat/vcbuild/scripts/lib.pl
@@ -670,6 +675,11 @@ else
 		NO_CURL =
 		USE_NED_ALLOCATOR = YesPlease
 		NO_PYTHON =
+		ifeq (/mingw64,$(subst 32,64,$(prefix)))
+			# Move system config into top-level /etc/
+			ETC_GITCONFIG = ../etc/gitconfig
+			ETC_GITATTRIBUTES = ../etc/gitattributes
+		endif
 	else
 		COMPAT_CFLAGS += -D__USE_MINGW_ANSI_STDIO
 		NO_CURL = YesPlease

git-compat-util.h

@@ -436,10 +436,6 @@ static inline int git_create_symlink(struct index_state *index, const char *targ
 #endif
 #endif
 
-#ifndef git_program_data_config
-#define git_program_data_config() NULL
-#endif
-
 #if defined(__HP_cc) && (__HP_cc >= 61000)
 #define NORETURN __attribute__((noreturn))
 #define NORETURN_PTR