Re: [PATCH v4 4/5] git-p4: add support for large file systems

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 9, 2015, 17:20 UTC
Message-ID: <xmqq37yn7bni.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <1441799950-54633-5-git-send-email-larsxschneider@gmail.com>

larsxschneider@gmail.com writes:

Show 7 quoted lines

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

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), ...

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

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?