From 5083d7c732f374da2646edae6f02760522bd2118 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 6 Oct 2019 22:58:49 +0200 Subject: [PATCH 1/2] mingw: prepare for making `git config --system` do what users expect This moves the system config into a more logical location: the `mingw64` part of `C:\Program Files\Git\mingw64\etc\gitconfig` never made sense, as it is a mere implementation detail. Let's skip the `mingw64` part and move this to `C:\Program Files\Git\etc\gitconfig`. Side note: in the rare (and not recommended) case a user chooses to install 32-bit Git for Windows on a 64-bit system, the path will of course be `C:\Program Files (x86)\Git\etc\gitconfig`. The next step after this patch is to scrap support for `C:\ProgramData\Git\config`, which never really made sense to users, and which is inconsistent with non-Windows versions of Git, anyway. Background: During the Git for Windows v1.x days, the system config was located at `C:\Program Files (x86)\Git\etc\gitconfig`. With Git for Windows v2.x, it moved to `C:\Program Files\Git\mingw64\gitconfig` (or `C:\Program Files (x86)\Git\mingw32\gitconfig`). Obviously, this new location was not stable (because of the "mingw64" part) and this maintainer thought that it was a splendid time to introduce a "super-system" config, with a constant location, that would be shared by all Git implementations. Hence `C:\ProgramData\Git\config` was born. What we *should* have done instead is to move the location to the top-level `etc` directory instead of the `mingw64\etc` one (or `mingw32\etc` for 32-bit versions). Likewise, we should have treated the system `gitattributes` in a similar manner. This patch makes it so. Obviously, we are cautious to do this only for the known install locations `/mingw64` and `/mingw32`; If anybody wants to override that while building their version of Git (e.g. via `make prefix=$HOME`), we leave the default location of the system config and gitattributes alone. Signed-off-by: Johannes Schindelin --- config.mak.uname | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/config.mak.uname b/config.mak.uname index 5e4b8a19c4dbd9..b6bdf73e6dbc71 100644 --- a/config.mak.uname +++ b/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 From d8ab53dddb79b6aba12d4ea8934e0a55889e3fdf Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 16 Oct 2019 19:13:20 +0200 Subject: [PATCH 2/2] fixup! Windows: add support for a Windows-wide configuration In the previous commit, we moved the location of the system config to `C:\Program Files\Git\etc\gitconfig`, to give users a reliable location for the system config. Previously, we had introduced `C:\ProgramData\Git\config` as such a location. However, the purpose of `C:\ProgramData\Git\config` was always for the installed Git for Windows to share its configured options. Every upgrade will modify the settings in that file according to what the user specified when running Git for Windows' installer. So it is not really a file whose ownership is shared between all Git implementations, contrary to what the location suggests. Also, it was totally confusing to many users that there was a "system" config and then yet another "sort of system" config that did not even have a corresponding `git config` option such as `--system`. Further, Portable Git should probably never look at `C:\ProgramData\Git\config` to begin with: this makes it rather non-Portable ;-) Finally, support for `C:\ProgramData\Git\config` never really caught on: only libgit2 implemented it, but even JGit (which might be used by more users than libgit2-based applications by grace of backing Android Studio) did not. Therefore, let's finally admit that it was a mistake to introduce `C:\ProgramData\Git\config` and remove support for it. This reverts commit bcbbb4f89f57cdd6d2260d28b274e9e6334d7ed3. Signed-off-by: Johannes Schindelin --- Documentation/config.txt | 4 +- Documentation/git-config.txt | 8 -- Documentation/git.txt | 3 +- compat/mingw.c | 184 ----------------------------------- compat/mingw.h | 2 - config.c | 15 +-- git-compat-util.h | 4 - 7 files changed, 7 insertions(+), 213 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 7f14ed2e9fa620..e3f5bc3396d0c7 100644 --- a/Documentation/config.txt +++ b/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 diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt index db5d80c171f811..ff9310f9588a4a 100644 --- a/Documentation/git-config.txt +++ b/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 diff --git a/Documentation/git.txt b/Documentation/git.txt index e9fc12fd889661..6a034ef7fe61a7 100644 --- a/Documentation/git.txt +++ b/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 diff --git a/compat/mingw.c b/compat/mingw.c index 463dcc54856c7b..6aece30ac0a371 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -2,8 +2,6 @@ #include "win32.h" #include #include -#include -#include #include #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 * diff --git a/compat/mingw.h b/compat/mingw.h index 8d4431c261e9a8..ab1fb887756072 100644 --- a/compat/mingw.h +++ b/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" diff --git a/config.c b/config.c index 5b04a0cc350087..3900e4947be92b 100644 --- a/config.c +++ b/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)) diff --git a/git-compat-util.h b/git-compat-util.h index 19667cbaf8860b..66c1da79aa81c4 100644 --- a/git-compat-util.h +++ b/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