{"thread":{"id":"11027","subject":"[PATCH] Make Git accept absolute path names for files within the work tree","startedAt":"2007-11-26T23:18:29Z","lastAt":"2007-11-29T02:05:10Z","messageCount":10,"participants":["Robin Rosenberg","Junio C Hamano","Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"61006","messageId":"1196119109-27483-1-git-send-email-robin.rosenberg@dewire.com","threadId":"11027","inReplyTo":null,"subject":"[PATCH] Make Git accept absolute path names for files within the work tree","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg@dewire.com","sentAt":"2007-11-26T23:18:29Z","receivedAt":"2007-11-26T23:18:29Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"This patch makes it possible to drag files and directories from\na graphical browser and drop them onto a shell and feed them\nto common git operations without editing away the path to the\nroot of the work tree.\n\nSigned-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>\n---\n setup.c               |   16 ++++++++++++++\n t/t3904-abspatharg.sh |   53 +++++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 69 insertions(+), 0 deletions(-)\n create mode 100755 t/t3904-abspatharg.sh\n\nWas it this simple?\n\ndiff --git a/setup.c b/setup.c\nindex 43cd3f9..9b3a9ff 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -6,6 +6,22 @@ static int inside_work_tree = -1;\n \n const char *prefix_path(const char *prefix, int len, const char *path)\n {\n+\tif (is_absolute_path(path)) {\n+\t\tconst char *work_tree = get_git_work_tree();\n+\t\tint n = strlen(work_tree);\n+\t\tif (!strncmp(path, work_tree, n) && (path[n] == '/' || !path[n])) {\n+\t\t\tif (path[n])\n+\t\t\t\tpath += 1;\n+\t\t\tpath += n;\n+\t\t\tif (prefix && !strncmp(path, prefix, len - 1)) {\n+\t\t\t    if (path[len - 1] == '/')\n+\t\t\t\t    path += len;\n+\t\t\t    else\n+\t\t\t\t    if (!path[len - 1])\n+\t\t\t\t\t    path += len - 1;\n+\t\t\t}\n+\t\t}\n+\t}\n \tconst char *orig = path;\n \tfor (;;) {\n \t\tchar c;\ndiff --git a/t/t3904-abspatharg.sh b/t/t3904-abspatharg.sh\nnew file mode 100755\nindex 0000000..aa47602\n--- /dev/null\n+++ b/t/t3904-abspatharg.sh\n@@ -0,0 +1,53 @@\n+#!/bin/sh\n+#\n+# Copyright (C) 2007 Robin Rosenberg\n+#\n+\n+test_description='Test absolute filename arguments to various git\n+commands.  Absolute arguments pointing to a location within the git\n+work tree should behave the same as relative arguments.  '\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'add files using absolute path names' '\n+echo a >afile &&\n+echo b >bfile &&\n+git-add afile &&\n+git-add $(pwd)/bfile &&\n+test \"afile bfile\" = \"$(echo $(git ls-files))\"\n+mkdir x &&\n+cd x &&\n+echo c >cfile &&\n+echo d >dfile &&\n+git-add cfile &&\n+git-add $(pwd) &&\n+cd .. &&\n+test \"afile bfile x/cfile x/dfile\" = \"$(echo $(git ls-files))\" &&\n+test \"$(echo $(git ls-files x))\" = \"$(echo $(git ls-files $(pwd)/x))\"\n+'\n+\n+test_expect_success 'commit using absolute path names' '\n+git commit -m \"foo\" &&\n+echo aa >>bfile &&\n+git commit -m \"bb\" $(pwd)/bfile\n+'\n+\n+test_expect_success 'log using absolute path names' '\n+git log afile >f1.txt &&\n+git log $(pwd)/afile >f2.txt &&\n+diff f1.txt f2.txt\n+'\n+\n+test_expect_success 'blame using absolute path names' '\n+git blame afile >f1.txt &&\n+git blame $(pwd)/afile >f2.txt &&\n+diff f1.txt f2.txt\n+'\n+\n+test_expect_success 'diff using absolute path names' '\n+git diff HEAD^ -- $(pwd)/afile >f1.txt &&\n+git diff HEAD^ -- afile >f2.txt &&\n+diff f1.txt f2.txt\n+'\n+\n+test_done\n-- \n1.5.3.5.1.gb2df9\n"},{"id":"61010","messageId":"7vmyt0edso.fsf@gitster.siamese.dyndns.org","threadId":"11027","inReplyTo":"1196119109-27483-1-git-send-email-robin.rosenberg@dewire.com","subject":"Re: [PATCH] Make Git accept absolute path names for files within the work tree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-27T00:24:07Z","receivedAt":"2007-11-27T00:24:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Robin Rosenberg <robin.rosenberg@dewire.com> writes:\n\n> Was it this simple?\n>\n> diff --git a/setup.c b/setup.c\n> index 43cd3f9..9b3a9ff 100644\n> --- a/setup.c\n> +++ b/setup.c\n> @@ -6,6 +6,22 @@ static int inside_work_tree = -1;\n>  \n>  const char *prefix_path(const char *prefix, int len, const char *path)\n>  {\n> +\tif (is_absolute_path(path)) {\n> +\t\tconst char *work_tree = get_git_work_tree();\n> +\t\tint n = strlen(work_tree);\n> +\t\tif (!strncmp(path, work_tree, n) && (path[n] == '/' || !path[n])) {\n> +\t\t\tif (path[n])\n> +\t\t\t\tpath += 1;\n> +\t\t\tpath += n;\n> +\t\t\tif (prefix && !strncmp(path, prefix, len - 1)) {\n> +\t\t\t    if (path[len - 1] == '/')\n> +\t\t\t\t    path += len;\n> +\t\t\t    else\n> +\t\t\t\t    if (!path[len - 1])\n> +\t\t\t\t\t    path += len - 1;\n> +\t\t\t}\n> +\t\t}\n> +\t}\n>  \tconst char *orig = path;\n\nDecl after statement.\n\nI do not think there is fundamental reason to object to this change, as\nlong as the prefixing is done to the path that is trying to name a path\nin the working tree.\n\nAlso some codepath that does not require any work tree may want to call\nprefix_path().  I do not know what would happen in such a case.\n\nAlthough I didn't look at all the callers, I think the caller from\nconfig.c is not talking about a path in the work tree, and not all users\nof config.c need to have work-tree.\n"},{"id":"61034","messageId":"474BD90C.5020102@viscovery.net","threadId":"11027","inReplyTo":"1196119109-27483-1-git-send-email-robin.rosenberg@dewire.com","subject":"Re: [PATCH] Make Git accept absolute path names for files within the work tree","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2007-11-27T08:45:00Z","receivedAt":"2007-11-27T08:45:00Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Robin Rosenberg schrieb:\n> +test_expect_success 'add files using absolute path names' '\n> +echo a >afile &&\n> +echo b >bfile &&\n> +git-add afile &&\n> +git-add $(pwd)/bfile &&\n\nPlease use \"$(pwd)/bfile\" (d-quotes) throughout. Otherwise the test would \nfail if conducted in a directory with spaces.\n\n-- Hannes\n"},{"id":"61131","messageId":"200711280014.55114.robin.rosenberg@dewire.com","threadId":"11027","inReplyTo":"474BD90C.5020102@viscovery.net","subject":"Re: [PATCH] Make Git accept absolute path names for files within the work tree","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg@dewire.com","sentAt":"2007-11-27T23:14:53Z","receivedAt":"2007-11-27T23:14:53Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"tisdag 27 november 2007 skrev Johannes Sixt:\n> Robin Rosenberg schrieb:\n> > +test_expect_success 'add files using absolute path names' '\n> > +echo a >afile &&\n> > +echo b >bfile &&\n> > +git-add afile &&\n> > +git-add $(pwd)/bfile &&\n> \n> Please use \"$(pwd)/bfile\" (d-quotes) throughout. Otherwise the test would \n> fail if conducted in a directory with spaces.\n\nAFAIK, None of the tests works in directories with spaces. But I'll fix it, just in case, so \nnoone can blame /me/ for the tests breaking in /my home/. I cannot verify it because\nthe testing infrastructure breaks before the actual tests start executing.\n\n-- robin\n"},{"id":"61133","messageId":"200711280020.57042.robin.rosenberg@dewire.com","threadId":"11027","inReplyTo":"7vmyt0edso.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Make Git accept absolute path names for files within the work tree","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg@dewire.com","sentAt":"2007-11-27T23:20:56Z","receivedAt":"2007-11-27T23:20:56Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"tisdag 27 november 2007 skrev Junio C Hamano:\n> Robin Rosenberg <robin.rosenberg@dewire.com> writes:\n[...]\n> >  \tconst char *orig = path;\n> \n> Decl after statement.\nStupid C..\n> \n> I do not think there is fundamental reason to object to this change, as\n> long as the prefixing is done to the path that is trying to name a path\n> in the working tree.\n> \n> Also some codepath that does not require any work tree may want to call\n> prefix_path().  I do not know what would happen in such a case.\nok, I'll add a test for whether we have a work tree.\n\n> Although I didn't look at all the callers, I think the caller from\n> config.c is not talking about a path in the work tree, and not all users\n> of config.c need to have work-tree.\n\nOh dear. That wasn't on master when I looked. That one seems to be the only \"other\"\nuse and it is wrapped inside an if (!absolute_path()) tests so my magic does not conflict\nthere.\n\n-- robin\n"},{"id":"61134","messageId":"1196205847-22968-1-git-send-email-robin.rosenberg@dewire.com","threadId":"11027","inReplyTo":"7vmyt0edso.fsf@gitster.siamese.dyndns.org","subject":"[PATCH] Make Git accept absolute path names for files within the work tree","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg@dewire.com","sentAt":"2007-11-27T23:24:07Z","receivedAt":"2007-11-27T23:24:07Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"This patch makes it possible to drag files and directories from\na graphical browser and drop them onto a shell and feed them\nto common git operations without editing away the path to the\nroot of the work tree.\n\nSigned-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>\n---\n setup.c               |   19 +++++++++++++++++\n t/t3904-abspatharg.sh |   55 +++++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 74 insertions(+), 0 deletions(-)\n create mode 100755 t/t3904-abspatharg.sh\n\ndiff --git a/setup.c b/setup.c\nindex f512ea0..ffc30bf 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -7,6 +7,25 @@ static int inside_work_tree = -1;\n const char *prefix_path(const char *prefix, int len, const char *path)\n {\n \tconst char *orig = path;\n+\tconst char *work_tree = get_git_work_tree();\n+\tif (is_absolute_path(path) && work_tree) {\n+\t\tint n = strlen(work_tree);\n+\t\tif (!strncmp(path, work_tree, n) && (path[n] == '/' || !path[n])) {\n+\t\t\tif (path[n])\n+\t\t\t\tpath += n + 1;\n+\t\t\telse\n+\t\t\t\tpath += n;\n+\n+\t\t\tif (prefix && !strncmp(path, prefix, len - 1)) {\n+\t\t\t\tif (path[len - 1] == '/')\n+\t\t\t\t\tpath += len;\n+\t\t\t\telse\n+\t\t\t\t\tif (!path[len - 1])\n+\t\t\t\t\t\tpath += len - 1;\n+\t\t\t}\n+\t\t}\n+\t}\n+\n \tfor (;;) {\n \t\tchar c;\n \t\tif (*path != '.')\ndiff --git a/t/t3904-abspatharg.sh b/t/t3904-abspatharg.sh\nnew file mode 100755\nindex 0000000..cd4a52e\n--- /dev/null\n+++ b/t/t3904-abspatharg.sh\n@@ -0,0 +1,55 @@\n+#!/bin/sh\n+#\n+# Copyright (C) 2007 Robin Rosenberg\n+#\n+\n+test_description='Test absolute filename arguments to various git\n+commands.  Absolute arguments pointing to a location within the git\n+work tree should behave the same as relative arguments.  '\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'add files using absolute path names' '\n+echo a >afile &&\n+echo b >bfile &&\n+git-add afile &&\n+git-add \"$(pwd)/bfile\" &&\n+test \"afile bfile\" = \"$(echo $(git ls-files))\"\n+mkdir x &&\n+cd x &&\n+echo c >cfile &&\n+echo d >dfile &&\n+git-add cfile &&\n+git-add \"$(pwd)\" &&\n+cd .. &&\n+test \"afile bfile x/cfile x/dfile\" = \"$(echo $(git ls-files))\" &&\n+git ls-files x >f1 &&\n+git ls-files \"$(pwd)/x\" >f2 &&\n+diff f1 f2\n+'\n+\n+test_expect_success 'commit using absolute path names' '\n+git commit -m \"foo\" &&\n+echo aa >>bfile &&\n+git commit -m \"bb\" \"$(pwd)/bfile\"\n+'\n+\n+test_expect_success 'log using absolute path names' '\n+git log afile >f1.txt &&\n+git log \"$(pwd)/afile\" >f2.txt &&\n+diff f1.txt f2.txt\n+'\n+\n+test_expect_success 'blame using absolute path names' '\n+git blame afile >f1.txt &&\n+git blame \"$(pwd)/afile\" >f2.txt &&\n+diff f1.txt f2.txt\n+'\n+\n+test_expect_success 'diff using absolute path names' '\n+git diff HEAD^ -- \"$(pwd)/afile\" >f1.txt &&\n+git diff HEAD^ -- afile >f2.txt &&\n+diff f1.txt f2.txt\n+'\n+\n+test_done\n-- \n1.5.3.5.1.gb2df9\n"},{"id":"61193","messageId":"7vmysy7oav.fsf@gitster.siamese.dyndns.org","threadId":"11027","inReplyTo":"1196205847-22968-1-git-send-email-robin.rosenberg@dewire.com","subject":"Re: [PATCH] Make Git accept absolute path names for files within the work tree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-28T08:43:36Z","receivedAt":"2007-11-28T08:43:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Robin Rosenberg <robin.rosenberg@dewire.com> writes:\n\n> This patch makes it possible to drag files and directories from\n> a graphical browser and drop them onto a shell and feed them\n> to common git operations without editing away the path to the\n> root of the work tree.\n>\n> Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>\n> ---\n>  setup.c               |   19 +++++++++++++++++\n>  t/t3904-abspatharg.sh |   55 +++++++++++++++++++++++++++++++++++++++++++++++++\n>  2 files changed, 74 insertions(+), 0 deletions(-)\n>  create mode 100755 t/t3904-abspatharg.sh\n>\n> diff --git a/setup.c b/setup.c\n> index f512ea0..ffc30bf 100644\n> --- a/setup.c\n> +++ b/setup.c\n> @@ -7,6 +7,25 @@ static int inside_work_tree = -1;\n>  const char *prefix_path(const char *prefix, int len, const char *path)\n>  {\n>  \tconst char *orig = path;\n> +\tconst char *work_tree = get_git_work_tree();\n> +\tif (is_absolute_path(path) && work_tree) {\n> +\t\tint n = strlen(work_tree);\n> +\t\tif (!strncmp(path, work_tree, n) && (path[n] == '/' || !path[n])) {\n> +\t\t\tif (path[n])\n> +\t\t\t\tpath += n + 1;\n> +\t\t\telse\n> +\t\t\t\tpath += n;\n> +\n> +\t\t\tif (prefix && !strncmp(path, prefix, len - 1)) {\n> +\t\t\t\tif (path[len - 1] == '/')\n> +\t\t\t\t\tpath += len;\n> +\t\t\t\telse\n> +\t\t\t\t\tif (!path[len - 1])\n> +\t\t\t\t\t\tpath += len - 1;\n> +\t\t\t}\n> +\t\t}\n> +\t}\n> +\n\nThis looks somewhat tighter than the previous one, but still made me\nworried if the caller of prefix_path() has run the setup sequence enough\nso that calling get_git_work_tree() is safe, so I ended up auditing the\ncallpath.  At least, I do not want to see that unconditional call to\nget_git_work_tree() when we do not need to do this \"ah prefix got an\nunusual absolute path\" stuff.\n\n * builtin-init-db.c uses prefix_path() to find where the template is\n   (this is mingw fallout change); in general, I do not think we would\n   want to trigger repository nor worktree discovery inside init-db,\n   although I suspect this particular callpath could be made Ok (because\n   it is taken only when template_dir is not absolute) if you do not\n   unconditionally call get_git_work_tree() in prefix_path().\n\n * config.c uses prefix_path() to find the ETC_GITCONFIG that is not\n   absolute (again, mingw fallout).  When git_config() is called, we\n   already should have discovered repository but worktree may not have\n   been found yet (config.worktree can be used to specify where it is,\n   so you have a chicken and egg problem).  Again, this particular\n   callpath happens to be Ok because this is used only for non-absolute\n   path, but that is a bit subtle.\n\n * get_pathspec() uses prefix_path() for obvious reasons, and the prefix\n   it gets must have been discovered by finding out where the worktree\n   is, so by definition that one is safe.\n\nEverybody else you would get from \"git grep prefix_path\" are after the\nproper setup, so they should all be safe. \n\n\n> diff --git a/t/t3904-abspatharg.sh b/t/t3904-abspatharg.sh\n> new file mode 100755\n> index 0000000..cd4a52e\n> --- /dev/null\n> +++ b/t/t3904-abspatharg.sh\n> @@ -0,0 +1,55 @@\n> +#!/bin/sh\n> +#\n> +# Copyright (C) 2007 Robin Rosenberg\n> +#\n> +\n> +test_description='Test absolute filename arguments to various git\n> +commands.  Absolute arguments pointing to a location within the git\n> +work tree should behave the same as relative arguments.  '\n> +\n> +. ./test-lib.sh\n> +\n> +test_expect_success 'add files using absolute path names' '\n> +echo a >afile &&\n> +echo b >bfile &&\n> +git-add afile &&\n> +git-add \"$(pwd)/bfile\" &&\n\nThis looks quite dense.  With indent like other existing tests this\nwould become a bit easier to read.\n\n> +test \"afile bfile\" = \"$(echo $(git ls-files))\"\n\nHmmm.  Looks a bit ugly but Ok.\n\n> +mkdir x &&\n> +cd x &&\n> +echo c >cfile &&\n> +echo d >dfile &&\n> +git-add cfile &&\n> +git-add \"$(pwd)\" &&\n> +cd .. &&\n\nIf this sequence fails in the middle, the next test will execute in a\nwrong directory.  Instead of \"cd there && ... && cd .. &&\", it is safer\nto do \"( cd there && ... ) &&\".\n\n> +test \"afile bfile x/cfile x/dfile\" = \"$(echo $(git ls-files))\" &&\n> +git ls-files x >f1 &&\n> +git ls-files \"$(pwd)/x\" >f2 &&\n> +diff f1 f2\n\n\"diff -u\" is much easier for people who ends up reading the output when\nsomething goes wrong.\n\nCases that are still not covered:\n\n\ttry to add a file in the parent directory from a subdirectory,\n\tusing absolute path;\n\n\ttry to add a file in a directory that is too high (i.e. outside\n\tthe work tree), using absolute path;\n\nThe latter I think should fail.  People tend to forget writing negative\ntests when they are too excited with their shiny new features.\n\nPerhaps adding something like:\n\n\ttry to add a sub-subdirectory using an absolute path, and make\n\tsure paths in subdirectory are not added;\n\nwould also be prudent to catch future possible breakages (off by one\ncutting the common directory prefix one level too deep or something),\nbut that is probably just me who tends to be paranoid.\n\nExactly the same comments apply to tests for other commands.  Also I\nthink mv, rm, reset and checkout take pathnames.\n"},{"id":"61341","messageId":"7vwss1zy26.fsf@gitster.siamese.dyndns.org","threadId":"11027","inReplyTo":"1196205847-22968-1-git-send-email-robin.rosenberg@dewire.com","subject":"Re: [PATCH] Make Git accept absolute path names for files within the work tree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-29T00:37:37Z","receivedAt":"2007-11-29T00:37:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Robin Rosenberg <robin.rosenberg@dewire.com> writes:\n\n> diff --git a/setup.c b/setup.c\n> index f512ea0..ffc30bf 100644\n> --- a/setup.c\n> +++ b/setup.c\n> @@ -7,6 +7,25 @@ static int inside_work_tree = -1;\n>  const char *prefix_path(const char *prefix, int len, const char *path)\n>  {\n>  \tconst char *orig = path;\n> +\tconst char *work_tree = get_git_work_tree();\n> +\tif (is_absolute_path(path) && work_tree) {\n\nCould you structure this part to read like this, into a separate\nfunction:\n\nstatic const char *strip_work_tree_path(const char *prefix, int len, const char *path)\n{\n\t...\n}\n\nconst char *prefix_path(const char *prefix, int len, const char *path)\n{\n\tconst char *orig = path;\n\n\tif (is_absolute_path(path))\n\t\tpath = strip_work_tree_path(prefix, len, path);\n\t...\n\n\nAbout the part that would be moved out of line with such a\nrestructuring,\n\n> +\t\tint n = strlen(work_tree);\n> +\t\tif (!strncmp(path, work_tree, n) && (path[n] == '/' || !path[n])) {\n> +\t\t\tif (path[n])\n> +\t\t\t\tpath += n + 1;\n> +\t\t\telse\n> +\t\t\t\tpath += n;\n> +\n> +\t\t\tif (prefix && !strncmp(path, prefix, len - 1)) {\n> +\t\t\t\tif (path[len - 1] == '/')\n> +\t\t\t\t\tpath += len;\n> +\t\t\t\telse\n> +\t\t\t\t\tif (!path[len - 1])\n> +\t\t\t\t\t\tpath += len - 1;\n> +\t\t\t}\n\nThis makes me wonder what happens if after stripping the worktree path\npath does not match the prefix.\n"},{"id":"61350","messageId":"200711290215.34237.robin.rosenberg@dewire.com","threadId":"11027","inReplyTo":"7vmysy7oav.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Make Git accept absolute path names for files within the work tree","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg@dewire.com","sentAt":"2007-11-29T01:15:33Z","receivedAt":"2007-11-29T01:15:33Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"onsdag 28 november 2007 skrev Junio C Hamano:\n> This looks somewhat tighter than the previous one, but still made me\n> worried if the caller of prefix_path() has run the setup sequence enough\n> so that calling get_git_work_tree() is safe, so I ended up auditing the\n> callpath.  At least, I do not want to see that unconditional call to\n> get_git_work_tree() when we do not need to do this \"ah prefix got an\n> unusual absolute path\" stuff.\n> \n>  * builtin-init-db.c uses prefix_path() to find where the template is\n>    (this is mingw fallout change); in general, I do not think we would\n>    want to trigger repository nor worktree discovery inside init-db,\n>    although I suspect this particular callpath could be made Ok (because\n>    it is taken only when template_dir is not absolute) if you do not\n>    unconditionally call get_git_work_tree() in prefix_path().\n> \n>  * config.c uses prefix_path() to find the ETC_GITCONFIG that is not\n>    absolute (again, mingw fallout).  When git_config() is called, we\n>    already should have discovered repository but worktree may not have\n>    been found yet (config.worktree can be used to specify where it is,\n>    so you have a chicken and egg problem).  Again, this particular\n>    callpath happens to be Ok because this is used only for non-absolute\n>    path, but that is a bit subtle.\n\nI wonder if this usage in config and initdb is what prefix_path() was intended for.  The\ninterface is declared in cache.h and there are error conditions like '%s is outside repository'.\n\nMaybe we should have a boolean indicating whether the arguments refer to filespecs\nor not to make this clear or rewite the mingw fallouts in some other way.\n\n>  * get_pathspec() uses prefix_path() for obvious reasons, and the prefix\n>    it gets must have been discovered by finding out where the worktree\n>    is, so by definition that one is safe.\n> \n> Everybody else you would get from \"git grep prefix_path\" are after the\n> proper setup, so they should all be safe. \n\nThanks for looking at the usages. I'll come up with some more tests too, though writing\nnegative tests sometimes is a challenge. Tests all to easily fail for the wrong reason which\nis bad when we expect them to fail for the right reason.\n\n-- robin\n"},{"id":"61354","messageId":"7v3aupzu09.fsf@gitster.siamese.dyndns.org","threadId":"11027","inReplyTo":"200711290215.34237.robin.rosenberg@dewire.com","subject":"Re: [PATCH] Make Git accept absolute path names for files within the work tree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-29T02:05:10Z","receivedAt":"2007-11-29T02:05:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Robin Rosenberg <robin.rosenberg@dewire.com> writes:\n\n> Maybe we should have a boolean indicating whether the arguments refer\n> to filespecs or not to make this clear or rewite the mingw fallouts in\n> some other way.\n\nI wondered the same, but my vote would be to make them separate\nfunctions.  You would need a function to abstract out pasting of paths\nto ease MinGW people even without this \"ah absolute -- let's make it\nrelative to work tree first\" hack, anyway.\n"}]}