On 09 Sep 2015, at 19:20, Junio C Hamano <gits...@pobox.com> wrote:

> larsxschnei...@gmail.com writes:
> 
>> @@ -2226,17 +2355,16 @@ class P4Sync(Command, P4UserMap):
>>             text = regexp.sub(r'$\1$', text)
>>             contents = [ text ]
>> 
>> -        self.gitStream.write("M %s inline %s\n" % (git_mode, relPath))
>> +        if relPath == '.gitattributes':
>> +            die('.gitattributes already exists in P4.')
> 
> This looks like an unfortunate limitation to me.
> 
> Is it really necessary that you need to reject unrelated attributes
> the user has (presumably for a good reason)?  It seems to me that
> you only need to _add_ entries to make file extension based decision
> to send paths selectively to LFS?
No, it is not necessary. I will remove this limitation.

> 
> Also the exact format of these attributes entries looks like very
> closely tied to GitHub LFS and not generic (for example, there is no
> reason to expect that any large-file support would always use the
> "filter" mechanism or the gitattributes mechanism for that
> matter), …
Agreed. Instead of just the filter name I will replace everything after the 
pathname with a single git-p4 config value:

['*.' + f.replace(' ', '[:space:]') + ' %s\n' % largeFileSystem().attributes()
    for f in sorted(gitConfigList("git-p4.largeFileExtensions"))
] +
['/' + f.replace(' ', '[:space:]') + ' %s\n' % largeFileSystem().attributes()
    for f in sorted(self.largeFiles) if not self.hasLargeFileExtension(f)
]

This, of course, would only work for gitattributes based solutions. 

> 
> +    def writeGitAttributesToStream(self):
> +        self.writeToGitStream(
> +            '100644',
> +            '.gitattributes',
> +            [
> +                '\n',
> +                '#\n',
> +                '# %s\n' % largeFileSystem().attributeDescription(),
> +                '#\n',
> +            ] +
> +            ['*.' + f.replace(' ', '[:space:]') + ' filter=%s -text\n'
> +                % largeFileSystem().attributeFilter()
> +                for f in sorted(gitConfigList("git-p4.largeFileExtensions"))
> +            ] +
> +            ['/' + f.replace(' ', '[:space:]') + ' filter=%s -text\n'
> +                % largeFileSystem().attributeFilter()
> +                for f in sorted(self.largeFiles) if not 
> self.hasLargeFileExtension(f)
> +            ]
> +        )
> +
> 
> ... so while I can see the code like the above needs to exist
> somewhere in "git p4" to support GitHub LFS, I am not sure if it
> belongs to the generic part of the code.  For the same reason, I do
> not know if these replacements with largeFileSystem().getters() are
> really adding much value.
I have the impression you would prefer to move all the attributes code from the 
generic code to the GitLFS code? I will explore that solution and see if I can 
come up with a nice generic interface.
 
> 
> How is collaboration between those who talk to the same p4 depot
> backed by GitHub LFS expected to work?  You use config to set size
> limits and list of file extensions in your repository, and grab new
> changes from p4 and turn them into Git commits (with pointers to LFS
> and the .gitattributes file that records your choice of the config).
> I as a new member to the same project come, clone the resulting Git
> repository from you and then what do I do before talking to the same
> p4 depot to further update the Git history?  Are the values recorded
> in .gitattributes (which essentially were derived from your
> configuration) somehow reflected back automatically to my config so
> that our world view would become consistent?  Perhaps you added
> 'iso' to largeFileExtensions before I cloned from you, and that
> would be present in the copy of .gitattributes I obtained from you.
> I may be trying to add a new ".iso" file, and I presume that an
> existing .gitattributes entry you created based on the file
> extension automatically covers it from the LFS side, but does my
> "git p4 submit" also know what to do, or would it be broken until I
> add a set of configrations that matches yours?
Well, you have a very good point here. From my point of view you can use git-p4 
in two ways:

_Way 1_: Your project is stored in Perforce and it will stay in Perforce. You 
use git-p4 on a regular basis to interact with the Perforce repository.
_Way 2_: Your project is stored in Perforce and you want to migrate it to Git. 
You use git-p4 once to perform the migration. Afterwards you never touch git-p4 
or Perforce again.

My large file system feature is intended to be used only for _Way 2_ for 
exactly the reasons you mentioned. Would it still be OK to add this feature to 
git-p4? Maybe with a appropriate warning in the docs?

Thanks,
Lars

 --
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

Reply via email to