Re: [PATCH v1 2/2] git-p4: fix Git LFS pointer parsing

[Sebastian Schuberth]

On Wed, Apr 20, 2016 at 5:30 PM, Junio C Hamano  wrote:

>> If clients rely on output targeted at human consumption it's not
>> surprising that these clients need to be adjusted from time to time.
>> What's troubling is not the change to git-lfs, but the very un-generic
>> way git-p4 is implemented.
>
> Sounds like the subcommand they are using is not meant for
> scripting?  What is the kosher way to get at the information they
> can use that is a supported interface for scripters?

The "pointer" subcommand indeed is listed under "Low level commands"
by "git lfs" (without any arguments), and as such it probably can be
considered for scripting use. However, before my fix in [1] the
subcommand was printing both output targeted at humans and output
targeted at scripts to stdout. After my fix, only output targeted at
script goes to stdout, and output targeted at humans goes to stderr.

[1] https://github.com/github/git-lfs/pull/1105

-- 
Sebastian Schuberth
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 2/2] git-p4: fix Git LFS pointer parsing

[Junio C Hamano]

Sebastian Schuberth  writes:

> If clients rely on output targeted at human consumption it's not
> surprising that these clients need to be adjusted from time to time.
> What's troubling is not the change to git-lfs, but the very un-generic
> way git-p4 is implemented.

Sounds like the subcommand they are using is not meant for
scripting?  What is the kosher way to get at the information they
can use that is a supported interface for scripters?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 2/2] git-p4: fix Git LFS pointer parsing

[Sebastian Schuberth]

On Tue, Apr 19, 2016 at 11:04 PM, Junio C Hamano  wrote:

>> I dropped the support for the older version to keep the code as
>> simple as possible (plus it would be cumbersome to test with an
>> outdated Git LFS version). Since it is probably a niche feature I
>> thought that might be acceptable.
>
> It is bad enough that clients need to be adjusted at all in the
> first place, but I would have found it very troubling if the
> problematic change to LFS thing were made in such a way that it
> makes backward compatible adjustment on the client code impossible.

If clients rely on output targeted at human consumption it's not
surprising that these clients need to be adjusted from time to time.
What's troubling is not the change to git-lfs, but the very un-generic
way git-p4 is implemented.

-- 
Sebastian Schuberth
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 2/2] git-p4: fix Git LFS pointer parsing

[Junio C Hamano]

Lars Schneider  writes:

>> On 19 Apr 2016, at 22:30, Junio C Hamano  wrote:
>> 
>> larsxschnei...@gmail.com writes:
>> 
>>> From: Lars Schneider 
>>> 
>>> Git LFS 1.2.0 removed a line from the output of the 'git lfs pointer'
>>> command [1] which broke the parsing of this output. Adjust the parser
>>> to the new output and add minimum Git LFS version to the docs.
>> 
>> Hmph, adjust to operate with both, or drop the support for the old
>> one?

> I dropped the support for the older version to keep the code as
> simple as possible (plus it would be cumbersome to test with an
> outdated Git LFS version). Since it is probably a niche feature I
> thought that might be acceptable.

It is bad enough that clients need to be adjusted at all in the
first place, but I would have found it very troubling if the
problematic change to LFS thing were made in such a way that it
makes backward compatible adjustment on the client code impossible.

But it seems that you could read their output and strip a line that
begins with a known substring to make it compatible with both?

"git P4" itself may be niche and using it with the LFS thing may
even be more so, but in Git land, traditionally we take the backward
compatibility seriously.  If it is not too much work, I'd prefer to
see this done the right way.

Thanks.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 2/2] git-p4: fix Git LFS pointer parsing

[Lars Schneider]

> On 19 Apr 2016, at 22:30, Junio C Hamano  wrote:
> 
> larsxschnei...@gmail.com writes:
> 
>> From: Lars Schneider 
>> 
>> Git LFS 1.2.0 removed a line from the output of the 'git lfs pointer'
>> command [1] which broke the parsing of this output. Adjust the parser
>> to the new output and add minimum Git LFS version to the docs.
> 
> Hmph, adjust to operate with both, or drop the support for the old
> one?
I dropped the support for the older version to keep the code as simple as 
possible (plus it would be cumbersome to test with an outdated Git LFS 
version). Since it is probably a niche feature I thought that might be 
acceptable.

Thanks,
Lars

>> 
>> [1] https://github.com/github/git-lfs/pull/1105
>> 
>> Signed-off-by: Lars Schneider 
>> ---
>> Documentation/git-p4.txt | 3 ++-
>> git-p4.py| 6 +++---
>> 2 files changed, 5 insertions(+), 4 deletions(-)
>> 
>> diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
>> index 88ba42b..b862cb9 100644
>> --- a/Documentation/git-p4.txt
>> +++ b/Documentation/git-p4.txt
>> @@ -522,7 +522,8 @@ git-p4.largeFileSystem::
>>that large file systems do not support the 'git p4 submit' command.
>>Only Git LFS is implemented right now (see https://git-lfs.github.com/
>>for more information). Download and install the Git LFS command line
>> -extension to use this option and configure it like this:
>> +extension (minimum version 1.2.0) to use this option and configure it
>> +like this:
>> +
>> -
>> git config   git-p4.largeFileSystem GitLFS
>> diff --git a/git-p4.py b/git-p4.py
>> index 527d44b..d2be574 100755
>> --- a/git-p4.py
>> +++ b/git-p4.py
>> @@ -1064,8 +1064,8 @@ class GitLFS(LargeFileSystem):
>> if pointerProcess.wait():
>> os.remove(contentFile)
>> die('git-lfs pointer command failed. Did you install the 
>> extension?')
>> -pointerContents = [i+'\n' for i in pointerFile.split('\n')[2:][:-1]]
>> -oid = pointerContents[1].split(' ')[1].split(':')[1][:-1]
>> +oidEntry = [i for i in pointerFile.split('\n') if 
>> i.startswith('oid')]
>> +oid = oidEntry[0].split(' ')[1].split(':')[1]
>> localLargeFile = os.path.join(
>> os.getcwd(),
>> '.git', 'lfs', 'objects', oid[:2], oid[2:4],
>> @@ -1073,7 +1073,7 @@ class GitLFS(LargeFileSystem):
>> )
>> # LFS Spec states that pointer files should not have the executable 
>> bit set.
>> gitMode = '100644'
>> -return (gitMode, pointerContents, localLargeFile)
>> +return (gitMode, pointerFile, localLargeFile)
>> 
>> def pushFile(self, localLargeFile):
>> uploadProcess = subprocess.Popen(
>> --
>> 2.5.1
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 2/2] git-p4: fix Git LFS pointer parsing

[Junio C Hamano]

larsxschnei...@gmail.com writes:

> From: Lars Schneider 
>
> Git LFS 1.2.0 removed a line from the output of the 'git lfs pointer'
> command [1] which broke the parsing of this output. Adjust the parser
> to the new output and add minimum Git LFS version to the docs.

Hmph, adjust to operate with both, or drop the support for the old
one?



>
> [1] https://github.com/github/git-lfs/pull/1105
>
> Signed-off-by: Lars Schneider 
> ---
>  Documentation/git-p4.txt | 3 ++-
>  git-p4.py| 6 +++---
>  2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
> index 88ba42b..b862cb9 100644
> --- a/Documentation/git-p4.txt
> +++ b/Documentation/git-p4.txt
> @@ -522,7 +522,8 @@ git-p4.largeFileSystem::
>   that large file systems do not support the 'git p4 submit' command.
>   Only Git LFS is implemented right now (see https://git-lfs.github.com/
>   for more information). Download and install the Git LFS command line
> - extension to use this option and configure it like this:
> + extension (minimum version 1.2.0) to use this option and configure it
> + like this:
>  +
>  -
>  git config   git-p4.largeFileSystem GitLFS
> diff --git a/git-p4.py b/git-p4.py
> index 527d44b..d2be574 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -1064,8 +1064,8 @@ class GitLFS(LargeFileSystem):
>  if pointerProcess.wait():
>  os.remove(contentFile)
>  die('git-lfs pointer command failed. Did you install the 
> extension?')
> -pointerContents = [i+'\n' for i in pointerFile.split('\n')[2:][:-1]]
> -oid = pointerContents[1].split(' ')[1].split(':')[1][:-1]
> +oidEntry = [i for i in pointerFile.split('\n') if 
> i.startswith('oid')]
> +oid = oidEntry[0].split(' ')[1].split(':')[1]
>  localLargeFile = os.path.join(
>  os.getcwd(),
>  '.git', 'lfs', 'objects', oid[:2], oid[2:4],
> @@ -1073,7 +1073,7 @@ class GitLFS(LargeFileSystem):
>  )
>  # LFS Spec states that pointer files should not have the executable 
> bit set.
>  gitMode = '100644'
> -return (gitMode, pointerContents, localLargeFile)
> +return (gitMode, pointerFile, localLargeFile)
>
>  def pushFile(self, localLargeFile):
>  uploadProcess = subprocess.Popen(
> --
> 2.5.1
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html