On 09 Sep 2015, at 19:20, Junio C Hamano <gitster@xxxxxxxxx> wrote: > larsxschneider@xxxxxxxxx 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html