{"thread":{"id":"2642","subject":"Git Future Proofing","startedAt":"2005-11-22T00:28:12Z","lastAt":"2005-11-23T00:57:13Z","messageCount":12,"participants":["Martin Atukunda","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"12494","messageId":"11326192921291-git-send-email-matlads@dsmagic.com","threadId":"2642","inReplyTo":null,"subject":"Git Future Proofing","fromName":"Martin Atukunda","fromEmail":"matlads@dsmagic.com","sentAt":"2005-11-22T00:28:12Z","receivedAt":"2005-11-22T00:28:12Z","isPatch":false,"sender":{"key":"matlads@dsmagic.com","avatar":null},"body":"This patch series adds git repository future proofing to git.\n\nIt adds checks for core.repositoryformatversion at various points in the git\narchitecture, and this is an overview of the patch series\n\nPatch 1 adds GIT_REPO_VERSION and repository_format_version\nPatch 2 fixes init-db's template copy so that it handles copying a config file.\nPatch 3 adds support for re-reading gits env variables in certain cases\nPatch 4 adds the repo format version check for various major operation\nPatch 5 adds support for explictly specifying which config file to use\nPatch 6 fixes up init-db config copying so as to never copy anything newer.\n\ncomments and suggestions welcome.\n\n- Martin -\n"},{"id":"12495","messageId":"11326192922669-git-send-email-matlads@dsmagic.com","threadId":"2642","inReplyTo":"11326192921291-git-send-email-matlads@dsmagic.com","subject":"[PATCH 2/6] Make init-db check repo format version if copying a config file.","fromName":"Martin Atukunda","fromEmail":"matlads@dsmagic.com","sentAt":"2005-11-22T00:28:12Z","receivedAt":"2005-11-22T00:28:12Z","isPatch":true,"sender":{"key":"matlads@dsmagic.com","avatar":null},"body":"This patch enables init-db to check the repo format version for a\nrepository being re-initialised. If the version of the repo is larger than\nGIT_VERSION_REPO, it simply dies with an appropriate message.\n\nSigned-Off-By: Martin Atukunda \n\n---\n\n init-db.c | 14 ++++++++++++++\n setup.c | 19 +++++++++++++++++++\n 2 files changed, 33 insertions(+), 0 deletions(-)\n\napplies-to: 3e86d91031df530695933e7d9d5d8d9c2f3a683d\n601dd50b595a9aa0a57972364262d98290a5d7b2\ndiff --git a/init-db.c b/init-db.c\nindex bd88291..90be428 100644\n--- a/init-db.c\n+++ b/init-db.c\n@@ -110,6 +110,19 @@ static void copy_templates_1(char *path,\n \t}\n }\n \n+static int init_db_config_check(const char *template_path)\n+{\n+\tDIR *dir;\n+\tstruct dirent *de;\n+\n+\tdir = opendir(template_path);\n+\twhile((de = readdir(dir)) != NULL) {\n+\t\tif ((strncmp(de->d_name, \"config\", 5) == 0))\n+\t\t\treturn check_repo_format();\n+\t}\n+\treturn 0;\n+}\n+\n static void copy_templates(const char *git_dir, int len, char *template_dir)\n {\n \tchar path[PATH_MAX];\n@@ -131,6 +144,7 @@ static void copy_templates(const char *g\n \t\t\ttemplate_dir);\n \t\treturn;\n \t}\n+\tinit_db_config_check(template_path);\n \n \tmemcpy(path, git_dir, len);\n \tpath[len] = 0;\ndiff --git a/setup.c b/setup.c\nindex c487d7e..8597424 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -127,3 +127,22 @@ const char *setup_git_directory(void)\n \tcwd[len] = 0;\n \treturn cwd + offset;\n }\n+\n+static int check_repo_config(const char *var, const char *value)\n+{\n+ if (strcmp(var, \"core.repositoryformatversion\") == 0) {\n+ repository_format_version = git_config_int(var, value);\n+ return 0;\n+ }\n+ return 1;\n+}\n+\n+int check_repo_format(void)\n+{\n+\tif (git_config(check_repo_config) == -1)\n+\t\treturn -1;\n+\tif (repository_format_version > GIT_REPO_VERSION)\n+\t\tdie (\"Expected git repo version <= %d, found %d\", GIT_REPO_VERSION,\n+\t\t\trepository_format_version);\n+\treturn 0;\n+}\n---\n0.99.9.GIT\n"},{"id":"12497","messageId":"11326192923683-git-send-email-matlads@dsmagic.com","threadId":"2642","inReplyTo":"11326192921291-git-send-email-matlads@dsmagic.com","subject":"[PATCH 3/6] Make get_git_dir take a flag that makes it re-read the env. variables","fromName":"Martin Atukunda","fromEmail":"matlads@dsmagic.com","sentAt":"2005-11-22T00:28:12Z","receivedAt":"2005-11-22T00:28:12Z","isPatch":true,"sender":{"key":"matlads@dsmagic.com","avatar":null},"body":"Signed-Off-By: Martin Atukunda \n\n---\n\n cache.h | 2 +-\n environment.c | 4 ++--\n path.c | 2 +-\n 3 files changed, 4 insertions(+), 4 deletions(-)\n\napplies-to: 0748fd804fbe155503235e2c36d812fc3ef641a0\ne3973791e841440ad92d91340fd954b5a58101c7\ndiff --git a/cache.h b/cache.h\nindex 54c283d..a455373 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -138,7 +138,7 @@ extern unsigned int active_nr, active_al\n #define INDEX_ENVIRONMENT \"GIT_INDEX_FILE\"\n #define GRAFT_ENVIRONMENT \"GIT_GRAFT_FILE\"\n \n-extern char *get_git_dir(void);\n+extern char *get_git_dir(int recheck_env);\n extern char *get_object_directory(void);\n extern char *get_refs_directory(void);\n extern char *get_index_file(void);\ndiff --git a/environment.c b/environment.c\nindex 3f19473..6a961ca 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -39,9 +39,9 @@ static void setup_git_env(void)\n \t\tgit_graft_file = strdup(git_path(\"info/grafts\"));\n }\n \n-char *get_git_dir(void)\n+char *get_git_dir(int recheck_env)\n {\n-\tif (!git_dir)\n+\tif (!git_dir || recheck_env)\n \t\tsetup_git_env();\n \treturn git_dir;\n }\ndiff --git a/path.c b/path.c\nindex 4d88947..e322dc0 100644\n--- a/path.c\n+++ b/path.c\n@@ -42,7 +42,7 @@ char *mkpath(const char *fmt, ...)\n \n char *git_path(const char *fmt, ...)\n {\n-\tconst char *git_dir = get_git_dir();\n+\tconst char *git_dir = get_git_dir(0);\n \tva_list args;\n \tunsigned len;\n \n---\n0.99.9.GIT\n"},{"id":"12498","messageId":"11326192923321-git-send-email-matlads@dsmagic.com","threadId":"2642","inReplyTo":"11326192921291-git-send-email-matlads@dsmagic.com","subject":"[PATCH 1/6] Add GIT_REPO_VERSION, and repository_format_version","fromName":"Martin Atukunda","fromEmail":"matlads@dsmagic.com","sentAt":"2005-11-22T00:28:12Z","receivedAt":"2005-11-22T00:28:12Z","isPatch":true,"sender":{"key":"matlads@dsmagic.com","avatar":null},"body":"This variable will enable git to track the repository version. It's\ncurrently set to 0. (in true C style :)\n\nSigned-Off-By: Martin Atukunda \n\n---\n\n cache.h | 4 ++++\n environment.c | 1 +\n 2 files changed, 5 insertions(+), 0 deletions(-)\n\napplies-to: 339319e60db7b3f96f8c711407b135a54da7aa2e\n976b8d57a80d79853df9c142ba30d39b414e1b8e\ndiff --git a/cache.h b/cache.h\nindex c7c6637..54c283d 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -182,6 +182,10 @@ extern int trust_executable_bit;\n extern int only_use_symrefs;\n extern int diff_rename_limit_default;\n \n+#define GIT_REPO_VERSION 0\n+extern int repository_format_version;\n+extern int check_repo_format(void);\n+\n #define MTIME_CHANGED\t0x0001\n #define CTIME_CHANGED\t0x0002\n #define OWNER_CHANGED\t0x0004\ndiff --git a/environment.c b/environment.c\nindex b5026f1..3f19473 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -13,6 +13,7 @@ char git_default_email[MAX_GITNAME];\n char git_default_name[MAX_GITNAME];\n int trust_executable_bit = 1;\n int only_use_symrefs = 0;\n+int repository_format_version = 0;\n \n static char *git_dir, *git_object_dir, *git_index_file, *git_refs_dir,\n \t*git_graft_file;\n---\n0.99.9.GIT\n"},{"id":"12496","messageId":"113261929333-git-send-email-matlads@dsmagic.com","threadId":"2642","inReplyTo":"11326192921291-git-send-email-matlads@dsmagic.com","subject":"[PATCH 4/6] Add check_repo_format check for all major operations.","fromName":"Martin Atukunda","fromEmail":"matlads@dsmagic.com","sentAt":"2005-11-22T00:28:13Z","receivedAt":"2005-11-22T00:28:13Z","isPatch":true,"sender":{"key":"matlads@dsmagic.com","avatar":null},"body":"The git-* command set uses 3 entry points in order to prepare\nto work with a git repo: enter_repo, get_git_dir, and obviously\nsetup_git_directory.\n\nThis patch adds a check for the repo format version to each of these\nentry points. This will automatically enable repo format version\nchecking for all the git-* programs.\n\nSigned-Off-By: Martin Atukunda \n\n---\n\n environment.c | 3 +++\n path.c | 1 +\n setup.c | 3 +++\n 3 files changed, 7 insertions(+), 0 deletions(-)\n\napplies-to: 8084b72bfb91efc08a9fa83e0893f21c5f60ad92\n5d902692ef7eef2763cadfa646eb9245422579af\ndiff --git a/environment.c b/environment.c\nindex 6a961ca..458eff8 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -37,6 +37,9 @@ static void setup_git_env(void)\n \tgit_graft_file = getenv(GRAFT_ENVIRONMENT);\n \tif (!git_graft_file)\n \t\tgit_graft_file = strdup(git_path(\"info/grafts\"));\n+\n+\t/* check the repo */\n+\tcheck_repo_format();\n }\n \n char *get_git_dir(int recheck_env)\ndiff --git a/path.c b/path.c\nindex e322dc0..84cb1c5 100644\n--- a/path.c\n+++ b/path.c\n@@ -199,6 +199,7 @@ char *enter_repo(char *path, int strict)\n \tif(access(\"objects\", X_OK) == 0 && access(\"refs\", X_OK) == 0 &&\n \t validate_symref(\"HEAD\") == 0) {\n \t\tputenv(\"GIT_DIR=.\");\n+\t\tget_git_dir(1); /* re-read the env variables */\n \t\treturn current_dir();\n \t}\n \ndiff --git a/setup.c b/setup.c\nindex 8597424..934f9a3 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -97,6 +97,9 @@ const char *setup_git_directory(void)\n \tstatic char cwd[PATH_MAX+1];\n \tint len, offset;\n \n+\tget_git_dir(1);\n+\tcheck_repo_format();\n+\n \t/*\n \t * If GIT_DIR is set explicitly, we're not going\n \t * to do any discovery\n---\n0.99.9.GIT\n"},{"id":"12499","messageId":"11326192933767-git-send-email-matlads@dsmagic.com","threadId":"2642","inReplyTo":"11326192921291-git-send-email-matlads@dsmagic.com","subject":"[PATCH 5/6] Allow Specification of the conf file to read for git_config operations","fromName":"Martin Atukunda","fromEmail":"matlads@dsmagic.com","sentAt":"2005-11-22T00:28:13Z","receivedAt":"2005-11-22T00:28:13Z","isPatch":true,"sender":{"key":"matlads@dsmagic.com","avatar":null},"body":"This patch adds a git_config_from_file which allows us to specify the\nconfig file to use for git_config operations.\n\nSigned-Off-By: Martin Atukunda \n\n---\n\n cache.h | 1 +\n config.c | 10 +++++++---\n 2 files changed, 8 insertions(+), 3 deletions(-)\n\napplies-to: d6c3faa0566795b1c74e693ecc004439c116e6c6\n39e4ab307abb445115f3c9ee0e09ed1812568247\ndiff --git a/cache.h b/cache.h\nindex a455373..48018ab 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -388,6 +388,7 @@ extern int gitfakemunmap(void *start, si\n \n typedef int (*config_fn_t)(const char *, const char *);\n extern int git_default_config(const char *, const char *);\n+extern int git_config_from_file(const char *, config_fn_t fn);\n extern int git_config(config_fn_t fn);\n extern int git_config_int(const char *, const char *);\n extern int git_config_bool(const char *, const char *);\ndiff --git a/config.c b/config.c\nindex 5d237c8..c5a5312 100644\n--- a/config.c\n+++ b/config.c\n@@ -245,11 +245,10 @@ int git_default_config(const char *var, \n \treturn 0;\n }\n \n-int git_config(config_fn_t fn)\n+int git_config_from_file(const char *confpath, config_fn_t fn)\n {\n \tint ret;\n-\tFILE *f = fopen(git_path(\"config\"), \"r\");\n-\n+\tFILE *f = fopen(confpath, \"r\");\n \tret = -1;\n \tif (f) {\n \t\tconfig_file = f;\n@@ -260,6 +259,11 @@ int git_config(config_fn_t fn)\n \treturn ret;\n }\n \n+int git_config(config_fn_t fn)\n+{\n+\treturn git_config_from_file(git_path(\"config\"), fn);\n+}\n+\n /*\n * Find all the stuff for git_config_set() below.\n */\n---\n0.99.9.GIT\n"},{"id":"12500","messageId":"11326192931463-git-send-email-matlads@dsmagic.com","threadId":"2642","inReplyTo":"11326192921291-git-send-email-matlads@dsmagic.com","subject":"[PATCH 6/6] Add check for downgrading of repo format version via init-db","fromName":"Martin Atukunda","fromEmail":"matlads@dsmagic.com","sentAt":"2005-11-22T00:28:13Z","receivedAt":"2005-11-22T00:28:13Z","isPatch":true,"sender":{"key":"matlads@dsmagic.com","avatar":null},"body":"This corrects an earlier assumption that init-db made. It assumed that the\nconfig file was specifying a correct repo format version.\n\nThis patch clarifies the assumption by checking the repo format version\nspecified in the config to be copied, and dies if the copy will result in\nan upgrade.\n\nIt however, warns if the copy will result in a downgrade of the repo format\nversion, as git tools are supposed (or will be able) to handle this case :)\n\nSigned-Off-By: Martin Atukunda \n\n---\n\n init-db.c | 41 +++++++++++++++++++++++++++++++++++++++--\n 1 files changed, 39 insertions(+), 2 deletions(-)\n\napplies-to: 0095aa60b05c91308a25a00dc939bcd95e63b03f\n7858de7a1a57e73d2585271b57acfcd044e27e68\ndiff --git a/init-db.c b/init-db.c\nindex 90be428..d1fc142 100644\n--- a/init-db.c\n+++ b/init-db.c\n@@ -110,6 +110,15 @@ static void copy_templates_1(char *path,\n \t}\n }\n \n+static int check_repo_config(const char *var, const char *value)\n+{\n+ if (strcmp(var, \"core.repositoryformatversion\") == 0) {\n+ repository_format_version = git_config_int(var, value);\n+ return 0;\n+ }\n+ return 1;\n+}\n+\n static int init_db_config_check(const char *template_path)\n {\n \tDIR *dir;\n@@ -117,8 +126,36 @@ static int init_db_config_check(const ch\n \n \tdir = opendir(template_path);\n \twhile((de = readdir(dir)) != NULL) {\n-\t\tif ((strncmp(de->d_name, \"config\", 5) == 0))\n-\t\t\treturn check_repo_format();\n+\t\tif ((strncmp(de->d_name, \"config\", 5) == 0)) {\n+\t\t\tint rfv1, rfv2;\n+\t\t\tchar cpath[PATH_MAX];\n+\t\t\tcheck_repo_format();\n+\n+\t\t\t/* is the file we are copying friendly? */\n+\t\t\trfv1 = repository_format_version;\n+\t\t\tsnprintf(cpath, sizeof(cpath), \"%s%s\", template_path,\n+\t\t\t\tde->d_name);\n+\t\t\tgit_config_from_file(cpath, check_repo_config);\n+\t\t\trfv2 = repository_format_version;\n+\t\t\tif (rfv1 == rfv2) {\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t\tif (rfv2 < rfv1) {\n+\t\t\t\t/* the repo format specified in the conf file\n+\t\t\t\t * we are copying is older than the repo we\n+\t\t\t\t * are re-initialising! Downgrading?\n+\t\t\t\t */\n+\t\t\t\tfprintf(stderr, \"Possibly downgrading repo\"\n+\t\t\t\t\t\" format version from %d to %d. Check\"\n+\t\t\t\t\t\" config template file!\\n\", rfv1, rfv2);\n+\t\t\t\tbreak;\n+\t\t\t} else \n+\t\t\t\t/* OK we die */\n+\t\t\t\tdie (\"Won't copy config file\"\n+\t\t\t\t\t\" for repo format version %d over\"\n+\t\t\t\t\t\" one for version %d\",\n+\t\t\t\t\trfv2, rfv1);\n+\t\t}\n \t}\n \treturn 0;\n }\n---\n0.99.9.GIT\n"},{"id":"12503","messageId":"7vmzjxjxi6.fsf@assigned-by-dhcp.cox.net","threadId":"2642","inReplyTo":"11326192921291-git-send-email-matlads@dsmagic.com","subject":"Re: Git Future Proofing","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-11-22T01:13:05Z","receivedAt":"2005-11-22T01:13:05Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin Atukunda writes:\n\n> Patch 2 fixes init-db's template copy so that it handles\n> copying a config file.\n\nThe readdir() loop in init_db_config_check() confuses me. Why\ncheck the prefix config (and \"config\" has 6 bytes not 5 ;-) not\njust open(\"$template_path/config\")???\n\n> Patch 6 fixes up init-db config copying so as to never copy anything newer.\n\n>> It however, warns if the copy will result in a downgrade of the repo format\n>> version, as git tools are supposed (or will be able) to handle this case :)\n\nI suspect that it is not enough to copy an older version of\nconfig file along with older version of templates.\n\nSuppose version 0 had .git/remotes/{origin,linus,...} and\nversion 1 moved that information to a flat file \".git/remotes\",\nthat has a bunch of sections like [remotes.origin] in the config\nfile format, because we have a mechanism in your patch 5 that\nlets us read from more than one configuration file.\n\nNow suppose you are running a version 1 repository, so all your\nremotes trees you subscribe to are described in .git/remotes\nfile. You somehow used git-init-db to reiniailize it, using\nversion 0 template, which has \"remotes/origin\" and\n\"remotes/linus\". What happens?\n\nTemplate-copying is designed not to overwrite what is in the\nrepository, so your .git/remotes file will hopefully be kept,\nand the configuration file now claims the repository is in\nversion 0 format. But is it really in version 0 format? You\ncannot create .git/remote/frotz file in such a repository.\n\nI think copying older one into a fresh repository might be safe,\nbut I'd feel safer if we do not play downgrade games like this.\n"},{"id":"12527","messageId":"7vlkzhf5li.fsf@assigned-by-dhcp.cox.net","threadId":"2642","inReplyTo":"113261929333-git-send-email-matlads@dsmagic.com","subject":"Re: [PATCH 4/6] Add check_repo_format check for all major operations.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-11-22T08:29:29Z","receivedAt":"2005-11-22T08:29:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin Atukunda writes:\n\n> The git-* command set uses 3 entry points in order to prepare\n> to work with a git repo: enter_repo, get_git_dir, and obviously\n> setup_git_directory.\n\nThanks, but I think this one is wrong.\n\n> diff --git a/environment.c b/environment.c\n> index 6a961ca..458eff8 100644\n> --- a/environment.c\n> +++ b/environment.c\n> @@ -37,6 +37,9 @@ static void setup_git_env(void)\n> \tgit_graft_file = getenv(GRAFT_ENVIRONMENT);\n> \tif (!git_graft_file)\n> \t\tgit_graft_file = strdup(git_path(\"info/grafts\"));\n> +\n> +\t/* check the repo */\n> +\tcheck_repo_format();\n> }\n\n> diff --git a/setup.c b/setup.c\n> index 8597424..934f9a3 100644\n> --- a/setup.c\n> +++ b/setup.c\n> @@ -97,6 +97,9 @@ const char *setup_git_directory(void)\n> \tstatic char cwd[PATH_MAX+1];\n> \tint len, offset;\n> \n> +\tget_git_dir(1);\n> +\tcheck_repo_format();\n> +\n> \t/*\n> \t * If GIT_DIR is set explicitly, we're not going\n> \t * to do any discovery\n\nIn setup_git_env() you have only read GIT_DIR environment but\nhave not done the toplevel discovery. Especially, this is\ncalled from get_git_dir(), and you call that as the first thing\nas setup_git_directory(). However, that function is supposed to\nbe callable by processes that are in a subdirectory, without\nGIT_DIR explicitly specified, and the place get_git_dir() is\ncalled in that function is way before the discovery of the\ntoplevel happens. Until then, you do not know where your .git/\ndirectory or .git/config file is. If you start in Documentation\nsubdirectory in git project, your setup_git_directory() would\nfirst call get_git_dir(), which says \"I assume the config file\nis at ./.git/config -- oh there is no such thing\". At that\npoint you are checking Documentation/.git/config. \n\nIt would happen to work because you intend to allow version 0\nrepository for any future version of tool, and even if this\ncodepath mistakenly thinks the repository is version 0, it does\nnot hurt, as long as your setup_git_directory() calls\ncheck_repo_format again after doing the toplevel discovery and\nchecks the true .git/config file, but I do not think you have\nthat call in the current series yet. Even if you had, this does\nnot feel quite right to me.\n"},{"id":"12535","messageId":"200511221555.24572.matlads@dsmagic.com","threadId":"2642","inReplyTo":"7vlkzhf5li.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 4/6] Add check_repo_format check for all major operations.","fromName":"Martin Atukunda","fromEmail":"matlads@dsmagic.com","sentAt":"2005-11-22T12:55:23Z","receivedAt":"2005-11-22T12:55:23Z","isPatch":true,"sender":{"key":"matlads@dsmagic.com","avatar":null},"body":"On Tuesday 22 November 2005 11:29, Junio C Hamano wrote:\n> Martin Atukunda writes:\n> > The git-* command set uses 3 entry points in order to prepare\n> > to work with a git repo: enter_repo, get_git_dir, and obviously\n> > setup_git_directory.\n>\n> Thanks, but I think this one is wrong.\n\n> In setup_git_env() you have only read GIT_DIR environment but\n> have not done the toplevel discovery. Especially, this is\n> called from get_git_dir(), and you call that as the first thing\n> as setup_git_directory(). However, that function is supposed to\n> be callable by processes that are in a subdirectory, without\n> GIT_DIR explicitly specified, and the place get_git_dir() is\n> called in that function is way before the discovery of the\n> toplevel happens. Until then, you do not know where your .git/\n> directory or .git/config file is. If you start in Documentation\n> subdirectory in git project, your setup_git_directory() would\n> first call get_git_dir(), which says \"I assume the config file\n> is at ./.git/config -- oh there is no such thing\". At that\n> point you are checking Documentation/.git/config.\n>\n> It would happen to work because you intend to allow version 0\n> repository for any future version of tool, and even if this\n> codepath mistakenly thinks the repository is version 0, it does\n> not hurt, as long as your setup_git_directory() calls\n> check_repo_format again after doing the toplevel discovery and\n> checks the true .git/config file, but I do not think you have\n> that call in the current series yet. Even if you had, this does\n> not feel quite right to me.\n\nwould something like the following apply in this case: (totally untested :)\n\n--\n\nAdd check_repo_format check for setup_git_directory(). This check needs\nto be done in 2 cases in this function. first if GIT_DIR is set, and also\nafter the top_level directory is found.\n\nSigned-Off-By: Martin Atukunda \n\ndiff --git a/setup.c b/setup.c\nindex 44b9866..45e716a 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -101,8 +101,11 @@ const char *setup_git_directory(void)\n \t * If GIT_DIR is set explicitly, we're not going\n \t * to do any discovery\n \t */\n-\tif (getenv(GIT_DIR_ENVIRONMENT))\n+\tif (getenv(GIT_DIR_ENVIRONMENT)) {\n+\t\tget_git_dir(1);\n+\t\tcheck_repo_format();\n \t\treturn NULL;\n+\t}\n \n \tif (!getcwd(cwd, sizeof(cwd)) || cwd[0] != '/')\n \t\tdie(\"Unable to read current working directory\");\n@@ -118,6 +121,8 @@ const char *setup_git_directory(void)\n \t\t} while (cwd[--offset] != '/');\n \t}\n \n+\tcheck_repo_format();\n+\n \tif (offset == len)\n \t\treturn NULL;\n \n"},{"id":"12550","messageId":"7vd5ksefth.fsf@assigned-by-dhcp.cox.net","threadId":"2642","inReplyTo":"200511221555.24572.matlads@dsmagic.com","subject":"Re: [PATCH 4/6] Add check_repo_format check for all major operations.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-11-22T17:46:18Z","receivedAt":"2005-11-22T17:46:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin Atukunda writes:\n\n>> that call in the current series yet. Even if you had, this does\n>> not feel quite right to me.\n>\n> would something like the following apply in this case: (totally untested :)\n\nYes, although that is exactly what I said \"this does not quite\nfeel right\" ;-). Is it hard to arrange things so that a process\ndoes exactly one check_repo_format() during its lifetime?\n\nTwo more issues I've been thinking about:\n\n1. The core.repositoryformatversion scheme assumes and relies on\n that at least .git/config would stay forward compatible. But\n if you cover unconditionally the three main entry points, I\n suspect \"git-var\" or \"git-config-set --get\" would stop\n working in a wrong repository. I think the scripts and\n Porcelains need a way to check if we are on a repository from\n the correct vintage before running other low-level commands,\n so one of these commands probably needs to be made to work\n without check_repo_format() dying. Or we could introduce a\n new command 'git-check-repo-format' for this specific\n purpose, and special case only that one.\n\n2. Some commands are read-only and are handy for problem\n diagnosis (e.g. cat-file), so it _might_ make sense to allow\n them to attempt running in a newer repository (which may well\n fail due to repository format difference). I am not sure\n about the merit of doing that outweighs the complexity,\n though. What you did covers _everything_ uniformly, and\n certainly is simpler, easier to explain, and nicer.\n\n\n\n \n"},{"id":"12582","messageId":"7v8xvg89li.fsf@assigned-by-dhcp.cox.net","threadId":"2642","inReplyTo":"7vd5ksefth.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 4/6] Add check_repo_format check for all major operations.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-11-23T00:57:13Z","receivedAt":"2005-11-23T00:57:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Yes, although that is exactly what I said \"this does not quite\n> feel right\" ;-). Is it hard to arrange things so that a process\n> does exactly one check_repo_format() during its lifetime?\n\nLet's back up a bit. I'll deal with only low-level commands\nhere.\n\nFirst, the easiest group. The following commands do not look at\nthe repository (.git directory) at all.\n\n check-ref-format get-tar-commit-id git index-pack mailinfo\n mailsplit patch-id shell show-index stripspace verify-pack\n\nWe do not need to do anything special about them.\n\nThe following three use enter_repo() to the given path (either\nfrom the end user at the command line or over the network):\n\n daemon receive-pack upload-pack\n\nThe repo format check should be done at the same place as we\nmake sure enter_repo() finds the given directory a satisfactory\npath. That is, just before putenv(GIT_DIR=.) in enter_repo().\n\nThe following use setup_git_directory(), which chdir()s to the\ntoplevel unless GIT_DIR is set.\n\n cat-file config-set diff-files diff-index diff-stages diff-tree\n ls-files name-rev rev-list rev-parse show-branch symbolic-ref\n update-index update-ref\n\nMaybe we can have a thin wrapper around setup_git_directory()\nand after it returns check \"${GIT_DIR-.git}\" for repository\nformat mismatch. What to do when GIT_DIR is set? Then we can\njust use it to read the config from \"$GIT_DIR/config\" and check\nthe version.\n\nThe following commands implicitly assume that they are either\nrun from the toplevel or GIT_DIR environment tells them where\nthe .git/ directory is:\n\n apply checkout-index clone-pack commit-tree convert-objects\n fetch-pack fsck-objects hash-object http-fetch http-push init-db\n local-fetch ls-tree merge-base merge-index mktag pack-objects\n pack-redundant peek-remote prune-packed read-tree send-pack\n ssh-fetch ssh-upload tar-tree unpack-file unpack-objects\n update-server-info var write-tree\n\nWith some exceptions, they are pretty much repository wide\ncommands, so I think it is OK for them to assume they start at\nthe toplevel (the Porcelain would chdir to the top for them\notherwise). The ones that take paths, namely, checkout-index,\nhash-object and ls-tree, may want to use setup_git_directory()\nand do the path prefixing.\n\nThat means we would need to have some way for the rest of the\ncommands to check if \"${GIT_DIR-.git}\" is of the right format\nversion, and call that *once* per process invocation,\nperferrably at the beginning of the main().\n\nWe need an access to .git/config file to do the repository\nformat check anyway, which means we need setup_git_directory()\nif we ever want to run them from subdirectories. And running\nsetup_git_directory() from the toplevel would not hurt, so\nperhaps if we add setup_git_directory() at the beginning of\nmain() for the \"implicity toplevel\" class, and rewrite\nsetup_git_directory() like this:\n\n static const char *setup_git_directory_main(void)\n {\n /* current setup_git_driectory() */\n }\n\n\tconst char *setup_git_directory(void)\n {\n \tconst char *retval = setup_git_directory_main();\n check_repository_format_version(); /* dies on mismatch */\n\t\treturn retval;\n\t}\n\nit _might_ be good enough.\n\nI said \"it _might_\" here, because we need to be careful. Right\nnow, if you run the \"implicitly toplevel\" commands from a\nsubdirectory, they fail. Some Porcelains and scripts may be\nrelying on that and there will be consequences if things\nsuddenly start not to fail but do something unexpected in higher\ndirectories.\n"}]}