Re: [PATCH 3/4] fast-import: let importers retrieve blobs
From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:50:10
Jonathan Nieder wrote:
The value for cat-blob-fd cannot be specified in the stream because that would be a layering violation: the decision of where to direct a stream has to be made when fast-import is started anyway, so we might as well make the stream format is independent of that detail.
Ungrammatical. I think I meant: There is no POSIX facility to open a file descriptor from outside after a process has already started; therefore, the frontend has to prepare a file descriptor for writing blobs before executing git fast-import. The --cat-blob-fd command line option indicates which file descriptor that is, defaulting to 1. It does not make sense to wait until the stream starts to specify which fd so it is not allowed, avoiding a potential layering violation. Other fast-import backends might provide other ways to specify where the blob stream should be written.
quoted hunk ↗ jump to hunk
+++ b/fast-import.c@@ -2824,6 +2910,8 @@ static int parse_one_feature(const char *feature, int from_stream) option_import_marks(feature + 13, from_stream); } else if (!prefixcmp(feature, "export-marks=")) { option_export_marks(feature + 13); + } else if (!strcmp(feature, "cat-blob")) { + ; /* Don't die - this feature is supported */
Implies support for a "--cat-blob" command line option that checks for cat-blob support. Is this wanted? (If so, it should be documented. If not, the condition should be "from_stream && !strcmp(...)".)
quoted hunk ↗ jump to hunk
@@ -2918,6 +3006,11 @@ static void parse_argv(void) if (parse_one_feature(a + 2, 0)) continue; + if (!prefixcmp(a + 2, "cat-blob-fd=")) { + option_cat_blob_fd(a + 2 + strlen("cat-blob-fd=")); + continue; + } +
Would be simpler and more explicit to put in parse_one_feature:
} else if (!from_stream && !prefixcmp(feature, "cat-blob-fd=")) {
Sorry this is taking so long to get right. :-/
Jonathan