On 09 Sep 2015, at 19:20, Junio C Hamano <[email protected]> wrote:
> [email protected] 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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
