{"thread":{"id":"52385","subject":"[PATCH 1/1] git-p4: use lfs.storage instead of local .git/lfs Use lfs.storage if it defined in git.config. If lfs.storage not define - used local .git/lfs. Original code uses local .git/lfs in sync/clone operations, but if you have external lfs storage better to use it.","startedAt":"2019-12-04T13:30:14Z","lastAt":"2019-12-12T19:47:50Z","messageCount":10,"participants":["panzercheg via GitGitGadget","r.burenkov via GitGitGadget","Junio C Hamano","Eric Sunshine","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"387488","messageId":"73d0dfc9dd7e6e800297b5084de84b727e28b900.1575466209.git.gitgitgadget@gmail.com","threadId":"52385","inReplyTo":"pull.483.git.1575466209.gitgitgadget@gmail.com","subject":"[PATCH 1/1] git-p4: use lfs.storage instead of local .git/lfs Use lfs.storage if it defined in git.config. If lfs.storage not define - used local .git/lfs. Original code uses local .git/lfs in sync/clone operations, but if you have external lfs storage better to use it.","fromName":"panzercheg via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-04T13:30:09Z","receivedAt":"2019-12-04T13:30:14Z","isPatch":true,"sender":{"key":"name:panzercheg","avatar":null},"body":"From: panzercheg <panzercheg@gmail.com>\n\nSigned-off-by: r.burenkov <panzercheg@gmail.com>\n---\n git-p4.py | 8 ++++++--\n 1 file changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 60c73b6a37..2bd0497c31 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -1257,9 +1257,13 @@ def generatePointer(self, contentFile):\n             pointerFile = re.sub(r'Git LFS pointer for.*\\n\\n', '', pointerFile)\n \n         oid = re.search(r'^oid \\w+:(\\w+)', pointerFile, re.MULTILINE).group(1)\n+        # if someone use external lfs.storage ( not in local repo git )\n+        lfs_path = os.path.join(os.getcwd(), '.git', 'lfs')\n+        if gitConfig('lfs.storage'):\n+            lfs_path = gitConfig('lfs.storage')\n         localLargeFile = os.path.join(\n-            os.getcwd(),\n-            '.git', 'lfs', 'objects', oid[:2], oid[2:4],\n+            lfs_path,\n+            'objects', oid[:2], oid[2:4],\n             oid,\n         )\n         # LFS Spec states that pointer files should not have the executable bit set.\n-- \ngitgitgadget\n"},{"id":"387489","messageId":"pull.483.git.1575466209.gitgitgadget@gmail.com","threadId":"52385","inReplyTo":null,"subject":"[PATCH 0/1] git-p4: use lfs.storage instead of local .git/lfs","fromName":"r.burenkov via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-04T13:30:08Z","receivedAt":"2019-12-04T13:30:15Z","isPatch":true,"sender":{"key":"name:r.burenkov","avatar":null},"body":"Use lfs.storage if it defined in git.config. If lfs.storage not define -\nused local .git/lfs.\n\nOriginal code uses local .git/lfs in sync/clone operations, but if you have\nexternal lfs storage better to use it.\n\npanzercheg (1):\n  git-p4: use lfs.storage instead of local .git/lfs Use lfs.storage if\n    it defined in git.config. If lfs.storage not define - used local\n    .git/lfs. Original code uses local .git/lfs in sync/clone\n    operations, but if you have external lfs storage better to use it.\n\n git-p4.py | 8 ++++++--\n 1 file changed, 6 insertions(+), 2 deletions(-)\n\n\nbase-commit: d9f6f3b6195a0ca35642561e530798ad1469bd41\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-483%2Fpanzercheg%2Fgit-p4-improve-lfs-logic-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-483/panzercheg/git-p4-improve-lfs-logic-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/483\n-- \ngitgitgadget\n"},{"id":"387493","messageId":"xmqqh82g5910.fsf@gitster-ct.c.googlers.com","threadId":"52385","inReplyTo":"73d0dfc9dd7e6e800297b5084de84b727e28b900.1575466209.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/1] git-p4: use lfs.storage instead of local .git/lfs Use lfs.storage if it defined in git.config. If lfs.storage not define - used local .git/lfs. Original code uses local .git/lfs in sync/clone operations, but if you have external lfs storage better to use it.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-12-04T16:51:23Z","receivedAt":"2019-12-04T16:51:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"panzercheg via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> From: panzercheg <panzercheg@gmail.com>\n\n> Subject: Re: [PATCH 1/1] git-p4: use lfs.storage instead of local .git/lfs Use lfs.storage if it defined in git.config. If lfs.storage not define - used local .git/lfs. Original code uses local .git/lfs in sync/clone operations, but if you have external lfs storage better to use it.\n\nThe reason why you ended up with this unreadably long subject line\nis because your commit did not follow the recommended\n\n    <area>: summary of the change in 50 or so characters\n\n    description of the issue being tackled,\n    where it came from,\n    why the current behaviour is considered not ideal, etc.\n\n    what the proposed change does, and why it is the good idea.\n\nformat, each clearly separated into their own paragraphs.\n\nEspecially the title MUST be in its own, single-line paragraph.\n\nLet's dissect what we see on the single long subject line.\n\n\n    git-p4: use lfs.storage instead of local .git/lfs\n\n    Use lfs.storage if it defined in git.config.\n\nWhat is \"git.config\"?  Is it our own usual configuration file(s), or\nis it the Git-LFS customized .lfsconfig file at the root of the\nrepository, or something else?\n\nGrammo: \"Use lfs.storage if it is defined ...\"\n\nBut do not start your proposed log message with the conclusion.\nBefore \"Use X\", we want to see readers convinced why using X is a\ngood idea; in other words, you need to tell the reason why the\ncurrent system that does not use X is not ideal, and explain that\nthe change makes the world a better place by teaching Git to use X.\n\n    If lfs.storage not define - used local .git/lfs.\n    Original code uses local .git/lfs in sync/clone operations,\n\nThese are saying pretty much the same thing, that can easily\ninferred from the \"Use lfs.storage if it is defined.\", iow, without\nhaving much original information content.\n\n    but if you have external lfs storage better to use it.\n\nThis is probably the sole line that attempts to justify the change.\nIt should be made stronger, I would think.\n\nPossible rewrite (don't use it literally---I am not an GitLFS user so\nI may be writing nonsense here):\n\n    git-p4: prefer lfs.storage over local .git/lfs directory\n\n    \"git lfs\" allows users to specify the custom storage location by\n    configuration variable lfs.storage, but when \"git p4\" interacts\n    with GitLFS pointers, it always used the hardcoded default that\n    is the .git/lfs/ directory, without paying attention to the\n    configuration.\n\n    Use the value configured in lfs.storage, if exists, as all the\n    \"git\" operations do, for consistency.\n\n    Signed-off-by: ...\n\n\n>\n> Signed-off-by: r.burenkov <panzercheg@gmail.com>\n> ---\n>  git-p4.py | 8 ++++++--\n>  1 file changed, 6 insertions(+), 2 deletions(-)\n>\n> diff --git a/git-p4.py b/git-p4.py\n> index 60c73b6a37..2bd0497c31 100755\n> --- a/git-p4.py\n> +++ b/git-p4.py\n> @@ -1257,9 +1257,13 @@ def generatePointer(self, contentFile):\n>              pointerFile = re.sub(r'Git LFS pointer for.*\\n\\n', '', pointerFile)\n>  \n>          oid = re.search(r'^oid \\w+:(\\w+)', pointerFile, re.MULTILINE).group(1)\n> +        # if someone use external lfs.storage ( not in local repo git )\n\ns/use/uses/; also lose SP after '(' and before ')'.\n\n> +        lfs_path = os.path.join(os.getcwd(), '.git', 'lfs')\n> +        if gitConfig('lfs.storage'):\n> +            lfs_path = gitConfig('lfs.storage')\n\nShouldn't the above three lines be more like this?\n\n\tlfs_path = gitConfig('lfs.storage');\n\tif not lfs_path:\n\t\tlfs_path = 'lfs'\n\tif not os.path.isabs(lfs_path):\n\t\tlfs_path = os.path.join(os.getcwd(), '.git', lfs_path)\n\nThe reason why I suspect so is because [*1*] says that the value of\nthe lfs.storage that is not an absolute path is relative to \".git\".\n\nThanks.\n\n\n\n[Reference]\n\n*1* https://github.com/git-lfs/git-lfs/blob/master/docs/man/git-lfs-config.5.ronn\n\n\n\n>          localLargeFile = os.path.join(\n> -            os.getcwd(),\n> -            '.git', 'lfs', 'objects', oid[:2], oid[2:4],\n> +            lfs_path,\n> +            'objects', oid[:2], oid[2:4],\n>              oid,\n>          )\n>          # LFS Spec states that pointer files should not have the executable bit set.\n"},{"id":"387776","messageId":"pull.483.v2.git.1575901722.gitgitgadget@gmail.com","threadId":"52385","inReplyTo":"pull.483.git.1575466209.gitgitgadget@gmail.com","subject":"[PATCH v2 0/1] git-p4: use lfs.storage instead of local .git/lfs","fromName":"r.burenkov via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-09T14:28:41Z","receivedAt":"2019-12-09T14:28:47Z","isPatch":true,"sender":{"key":"name:r.burenkov","avatar":null},"body":"Use lfs.storage if it defined in git.config. If lfs.storage not define -\nused local .git/lfs.\n\nOriginal code uses local .git/lfs in sync/clone operations, but if you have\nexternal lfs storage better to use it.\n\npanzercheg (1):\n  \"git lfs\" allows users to specify the custom storage location by\n    configuration variable lfs.storage, but when \"git p4\" interacts with\n    GitLFS pointers, it always used the hardcoded default that is the\n    .git/lfs/ directory, without paying attention to the configuration.\n\n git-p4.py | 10 ++++++++--\n 1 file changed, 8 insertions(+), 2 deletions(-)\n\n\nbase-commit: d9f6f3b6195a0ca35642561e530798ad1469bd41\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-483%2Fpanzercheg%2Fgit-p4-improve-lfs-logic-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-483/panzercheg/git-p4-improve-lfs-logic-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/483\n\nRange-diff vs v1:\n\n 1:  73d0dfc9dd ! 1:  e65375c528 git-p4: use lfs.storage instead of local .git/lfs Use lfs.storage if it defined in git.config. If lfs.storage not define - used local .git/lfs. Original code uses local .git/lfs in sync/clone operations, but if you have external lfs storage better to use it.\n     @@ -1,10 +1,13 @@\n      Author: panzercheg <panzercheg@gmail.com>\n      \n     -    git-p4: use lfs.storage instead of local .git/lfs\n     -    Use lfs.storage if it defined in git.config.\n     -    If lfs.storage not define - used local .git/lfs.\n     -    Original code uses local .git/lfs in sync/clone operations,\n     -    but if you have external lfs storage better to use it.\n     +    \"git lfs\" allows users to specify the custom storage location by\n     +    configuration variable lfs.storage, but when \"git p4\" interacts\n     +    with GitLFS pointers, it always used the hardcoded default that\n     +    is the .git/lfs/ directory, without paying attention to the\n     +    configuration.\n     +\n     +    Use the value configured in lfs.storage, if exists, as all the\n     +    \"git\" operations do, for consistency.\n      \n          Signed-off-by: r.burenkov <panzercheg@gmail.com>\n      \n     @@ -16,9 +19,11 @@\n       \n               oid = re.search(r'^oid \\w+:(\\w+)', pointerFile, re.MULTILINE).group(1)\n      +        # if someone use external lfs.storage ( not in local repo git )\n     -+        lfs_path = os.path.join(os.getcwd(), '.git', 'lfs')\n     -+        if gitConfig('lfs.storage'):\n     -+            lfs_path = gitConfig('lfs.storage')\n     ++        lfs_path = gitConfig('lfs.storage')\n     ++        if not lfs_path:\n     ++            lfs_path = 'lfs'\n     ++        if not os.path.isabs(lfs_path):\n     ++            lfs_path = os.path.join(os.getcwd(), '.git', lfs_path)\n               localLargeFile = os.path.join(\n      -            os.getcwd(),\n      -            '.git', 'lfs', 'objects', oid[:2], oid[2:4],\n\n-- \ngitgitgadget\n"},{"id":"387777","messageId":"e65375c528bdcdcc928dce8159edfccddcf77070.1575901722.git.gitgitgadget@gmail.com","threadId":"52385","inReplyTo":"pull.483.v2.git.1575901722.gitgitgadget@gmail.com","subject":"[PATCH v2 1/1] \"git lfs\" allows users to specify the custom storage location by configuration variable lfs.storage, but when \"git p4\" interacts with GitLFS pointers, it always used the hardcoded default that is the .git/lfs/ directory, without paying attention to the configuration.","fromName":"panzercheg via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-09T14:28:42Z","receivedAt":"2019-12-09T14:28:48Z","isPatch":true,"sender":{"key":"name:panzercheg","avatar":null},"body":"From: panzercheg <panzercheg@gmail.com>\n\nUse the value configured in lfs.storage, if exists, as all the\n\"git\" operations do, for consistency.\n\nSigned-off-by: r.burenkov <panzercheg@gmail.com>\n---\n git-p4.py | 10 ++++++++--\n 1 file changed, 8 insertions(+), 2 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 60c73b6a37..0b3a07cb31 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -1257,9 +1257,15 @@ def generatePointer(self, contentFile):\n             pointerFile = re.sub(r'Git LFS pointer for.*\\n\\n', '', pointerFile)\n \n         oid = re.search(r'^oid \\w+:(\\w+)', pointerFile, re.MULTILINE).group(1)\n+        # if someone use external lfs.storage ( not in local repo git )\n+        lfs_path = gitConfig('lfs.storage')\n+        if not lfs_path:\n+            lfs_path = 'lfs'\n+        if not os.path.isabs(lfs_path):\n+            lfs_path = os.path.join(os.getcwd(), '.git', lfs_path)\n         localLargeFile = os.path.join(\n-            os.getcwd(),\n-            '.git', 'lfs', 'objects', oid[:2], oid[2:4],\n+            lfs_path,\n+            'objects', oid[:2], oid[2:4],\n             oid,\n         )\n         # LFS Spec states that pointer files should not have the executable bit set.\n-- \ngitgitgadget\n"},{"id":"387856","messageId":"xmqq1rtdt9rc.fsf@gitster-ct.c.googlers.com","threadId":"52385","inReplyTo":"e65375c528bdcdcc928dce8159edfccddcf77070.1575901722.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 1/1] \"git lfs\" allows users to specify the custom storage location by configuration variable lfs.storage, but when \"git p4\" interacts with GitLFS pointers, it always used the hardcoded default that is the .git/lfs/ directory, without paying attention to the configuration.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-12-09T22:27:35Z","receivedAt":"2019-12-09T22:27:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"panzercheg via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n>Subject: Re: [PATCH v2 1/1] \"git lfs\" allows users to specify the custom storage location by configuration variable lfs.storage, but when \"git p4\" interacts with GitLFS pointers, it always used the hardcoded default that is the .git/lfs/ directory, without paying attention to the configuration.\n\nOops, what happened here?\n\nI wonder/I wish if GGG can be a bit more helpful when seeing a\ncommit that looks \"strange\".\n\n> From: panzercheg <panzercheg@gmail.com>\n>\n> Use the value configured in lfs.storage, if exists, as all the\n> \"git\" operations do, for consistency.\n>\n> Signed-off-by: r.burenkov <panzercheg@gmail.com>\n\nPlease make sure that the name/email as the author matches whom you\nsign-off the patch as.\n\n\n\n> ---\n>  git-p4.py | 10 ++++++++--\n>  1 file changed, 8 insertions(+), 2 deletions(-)\n>\n> diff --git a/git-p4.py b/git-p4.py\n> index 60c73b6a37..0b3a07cb31 100755\n> --- a/git-p4.py\n> +++ b/git-p4.py\n> @@ -1257,9 +1257,15 @@ def generatePointer(self, contentFile):\n>              pointerFile = re.sub(r'Git LFS pointer for.*\\n\\n', '', pointerFile)\n>  \n>          oid = re.search(r'^oid \\w+:(\\w+)', pointerFile, re.MULTILINE).group(1)\n> +        # if someone use external lfs.storage ( not in local repo git )\n> +        lfs_path = gitConfig('lfs.storage')\n> +        if not lfs_path:\n> +            lfs_path = 'lfs'\n> +        if not os.path.isabs(lfs_path):\n> +            lfs_path = os.path.join(os.getcwd(), '.git', lfs_path)\n>          localLargeFile = os.path.join(\n> -            os.getcwd(),\n> -            '.git', 'lfs', 'objects', oid[:2], oid[2:4],\n> +            lfs_path,\n> +            'objects', oid[:2], oid[2:4],\n>              oid,\n>          )\n>          # LFS Spec states that pointer files should not have the executable bit set.\n"},{"id":"387857","messageId":"CAPig+cR2=LZtsMtAR-0b5mvL6B161eGyM5th6CM3TQxLYmifxg@mail.gmail.com","threadId":"52385","inReplyTo":"xmqq1rtdt9rc.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 1/1] \"git lfs\" allows users to specify the custom storage location by configuration variable lfs.storage, but when \"git p4\" interacts with GitLFS pointers, it always used the hardcoded default that is the .git/lfs/ directory, without paying attention to the configuration.","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2019-12-09T22:50:17Z","receivedAt":"2019-12-09T22:50:34Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Dec 9, 2019 at 5:27 PM Junio C Hamano <gitster@pobox.com> wrote:\n> >Subject: Re: [PATCH v2 1/1] \"git lfs\" allows users to specify the custom storage location by configuration variable lfs.storage, but when \"git p4\" interacts with GitLFS pointers, it always used the hardcoded default that is the .git/lfs/ directory, without paying attention to the configuration.\n>\n> Oops, what happened here?\n>\n> I wonder/I wish if GGG can be a bit more helpful when seeing a\n> commit that looks \"strange\".\n\nThere is an open issue[1] regarding that.\n\n[1]: https://github.com/gitgitgadget/gitgitgadget/issues/120\n"},{"id":"387884","messageId":"nycvar.QRO.7.76.6.1912101317340.31080@tvgsbejvaqbjf.bet","threadId":"52385","inReplyTo":"xmqq1rtdt9rc.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 1/1] \"git lfs\" allows users to specify the custom storage location by configuration variable lfs.storage, but when \"git p4\" interacts with GitLFS pointers, it always used the hardcoded default that is the .git/lfs/ directory, without paying attention to the configuration.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-12-10T12:19:13Z","receivedAt":"2019-12-10T12:19:40Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Mon, 9 Dec 2019, Junio C Hamano wrote:\n\n> \"panzercheg via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n>\n> >Subject: Re: [PATCH v2 1/1] \"git lfs\" allows users to specify the custom storage location by configuration variable lfs.storage, but when \"git p4\" interacts with GitLFS pointers, it always used the hardcoded default that is the .git/lfs/ directory, without paying attention to the configuration.\n>\n> Oops, what happened here?\n>\n> I wonder/I wish if GGG can be a bit more helpful when seeing a\n> commit that looks \"strange\".\n\nThere is already a ticket about that:\nhttps://github.com/gitgitgadget/gitgitgadget/issues/120\n\nAll it requires is a contributor with a little time :-)\n\n> > From: panzercheg <panzercheg@gmail.com>\n> >\n> > Use the value configured in lfs.storage, if exists, as all the\n> > \"git\" operations do, for consistency.\n> >\n> > Signed-off-by: r.burenkov <panzercheg@gmail.com>\n>\n> Please make sure that the name/email as the author matches whom you\n> sign-off the patch as.\n\nThis, too, should be addressed as part of above-mentioned ticket.\n\nCiao,\nDscho\n"},{"id":"387957","messageId":"xmqq8snipxec.fsf@gitster-ct.c.googlers.com","threadId":"52385","inReplyTo":"nycvar.QRO.7.76.6.1912101317340.31080@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v2 1/1] \"git lfs\" allows users to specify the custom storage location by configuration variable lfs.storage, but when \"git p4\" interacts with GitLFS pointers, it always used the hardcoded default that is the .git/lfs/ directory, without paying attention to the configuration.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-12-11T17:47:23Z","receivedAt":"2019-12-11T17:47:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> On Mon, 9 Dec 2019, Junio C Hamano wrote:\n>\n>> \"panzercheg via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n>>\n>> >Subject: Re: [PATCH v2 1/1] \"git lfs\" allows users to specify the custom storage location by configuration variable lfs.storage, but when \"git p4\" interacts with GitLFS pointers, it always used the hardcoded default that is the .git/lfs/ directory, without paying attention to the configuration.\n>>\n>> Oops, what happened here?\n>>\n>> I wonder/I wish if GGG can be a bit more helpful when seeing a\n>> commit that looks \"strange\".\n>\n> There is already a ticket about that:\n> https://github.com/gitgitgadget/gitgitgadget/issues/120\n>\n> All it requires is a contributor with a little time :-)\n>\n>> > From: panzercheg <panzercheg@gmail.com>\n>> >\n>> > Use the value configured in lfs.storage, if exists, as all the\n>> > \"git\" operations do, for consistency.\n>> >\n>> > Signed-off-by: r.burenkov <panzercheg@gmail.com>\n>>\n>> Please make sure that the name/email as the author matches whom you\n>> sign-off the patch as.\n>\n> This, too, should be addressed as part of above-mentioned ticket.\n\nTooling improvement is fine, but let's not sink too much time on\ntangents and steal time from *this* patch.\n\nWould the following version (which I munged by hand) be close enough\nto what the author would have sent out in the ideal world?  If so,\nlet's queue it.\n\n-- >8 --\nSubject: git-p4: honor lfs.storage configuration variable\nFrom: r.burenkov <panzercheg@gmail.com>\n\n\"git lfs\" allows users to specify the custom storage location by the\nconfiguration variable `lfs.storage`, but when \"git p4\" interacts with\nGitLFS pointers, it always uses the hardcoded default that is the\n`.git/lfs/` directory, without paying attention to the configuration.\n\nUse the value configured in `lfs.storage`, if exists, as all the\n\"git\" operations do, for consistency.\n\nSigned-off-by: r.burenkov <panzercheg@gmail.com>\n---\n git-p4.py | 10 ++++++++--\n 1 file changed, 8 insertions(+), 2 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 60c73b6a37..0b3a07cb31 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -1257,9 +1257,15 @@ def generatePointer(self, contentFile):\n             pointerFile = re.sub(r'Git LFS pointer for.*\\n\\n', '', pointerFile)\n \n         oid = re.search(r'^oid \\w+:(\\w+)', pointerFile, re.MULTILINE).group(1)\n+        # if someone use external lfs.storage ( not in local repo git )\n+        lfs_path = gitConfig('lfs.storage')\n+        if not lfs_path:\n+            lfs_path = 'lfs'\n+        if not os.path.isabs(lfs_path):\n+            lfs_path = os.path.join(os.getcwd(), '.git', lfs_path)\n         localLargeFile = os.path.join(\n-            os.getcwd(),\n-            '.git', 'lfs', 'objects', oid[:2], oid[2:4],\n+            lfs_path,\n+            'objects', oid[:2], oid[2:4],\n             oid,\n         )\n         # LFS Spec states that pointer files should not have the executable bit set.\n-- \ngitgitgadget\n"},{"id":"388044","messageId":"nycvar.QRO.7.76.6.1912122046400.46@tvgsbejvaqbjf.bet","threadId":"52385","inReplyTo":"xmqq8snipxec.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 1/1] \"git lfs\" allows users to specify the custom storage location by configuration variable lfs.storage, but when \"git p4\" interacts with GitLFS pointers, it always used the hardcoded default that is the .git/lfs/ directory, without paying attention to the configuration.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-12-12T19:47:28Z","receivedAt":"2019-12-12T19:47:50Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Wed, 11 Dec 2019, Junio C Hamano wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n>\n> > On Mon, 9 Dec 2019, Junio C Hamano wrote:\n> >\n> >> \"panzercheg via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n> >>\n> >> >Subject: Re: [PATCH v2 1/1] \"git lfs\" allows users to specify the custom storage location by configuration variable lfs.storage, but when \"git p4\" interacts with GitLFS pointers, it always used the hardcoded default that is the .git/lfs/ directory, without paying attention to the configuration.\n> >>\n> >> Oops, what happened here?\n> >>\n> >> I wonder/I wish if GGG can be a bit more helpful when seeing a\n> >> commit that looks \"strange\".\n> >\n> > There is already a ticket about that:\n> > https://github.com/gitgitgadget/gitgitgadget/issues/120\n> >\n> > All it requires is a contributor with a little time :-)\n> >\n> >> > From: panzercheg <panzercheg@gmail.com>\n> >> >\n> >> > Use the value configured in lfs.storage, if exists, as all the\n> >> > \"git\" operations do, for consistency.\n> >> >\n> >> > Signed-off-by: r.burenkov <panzercheg@gmail.com>\n> >>\n> >> Please make sure that the name/email as the author matches whom you\n> >> sign-off the patch as.\n> >\n> > This, too, should be addressed as part of above-mentioned ticket.\n>\n> Tooling improvement is fine, but let's not sink too much time on\n> tangents and steal time from *this* patch.\n\nI fully agree.\n\n> Would the following version (which I munged by hand) be close enough\n> to what the author would have sent out in the ideal world?  If so,\n> let's queue it.\n\nI obviously cannot speak for r.burenkov, but from my point of view, the\nbelow patch is very much the intended outcome.\n\nThanks,\nDscho\n\n>\n> -- >8 --\n> Subject: git-p4: honor lfs.storage configuration variable\n> From: r.burenkov <panzercheg@gmail.com>\n>\n> \"git lfs\" allows users to specify the custom storage location by the\n> configuration variable `lfs.storage`, but when \"git p4\" interacts with\n> GitLFS pointers, it always uses the hardcoded default that is the\n> `.git/lfs/` directory, without paying attention to the configuration.\n>\n> Use the value configured in `lfs.storage`, if exists, as all the\n> \"git\" operations do, for consistency.\n>\n> Signed-off-by: r.burenkov <panzercheg@gmail.com>\n> ---\n>  git-p4.py | 10 ++++++++--\n>  1 file changed, 8 insertions(+), 2 deletions(-)\n>\n> diff --git a/git-p4.py b/git-p4.py\n> index 60c73b6a37..0b3a07cb31 100755\n> --- a/git-p4.py\n> +++ b/git-p4.py\n> @@ -1257,9 +1257,15 @@ def generatePointer(self, contentFile):\n>              pointerFile = re.sub(r'Git LFS pointer for.*\\n\\n', '', pointerFile)\n>\n>          oid = re.search(r'^oid \\w+:(\\w+)', pointerFile, re.MULTILINE).group(1)\n> +        # if someone use external lfs.storage ( not in local repo git )\n> +        lfs_path = gitConfig('lfs.storage')\n> +        if not lfs_path:\n> +            lfs_path = 'lfs'\n> +        if not os.path.isabs(lfs_path):\n> +            lfs_path = os.path.join(os.getcwd(), '.git', lfs_path)\n>          localLargeFile = os.path.join(\n> -            os.getcwd(),\n> -            '.git', 'lfs', 'objects', oid[:2], oid[2:4],\n> +            lfs_path,\n> +            'objects', oid[:2], oid[2:4],\n>              oid,\n>          )\n>          # LFS Spec states that pointer files should not have the executable bit set.\n> --\n> gitgitgadget\n>\n"}]}