fast-import: delay creating leading directories for export-marks

When we parse the --export-marks option, we don't immediately open the
file, but we do create any leading directories. This can be especially
confusing when a command-line option overrides an in-stream one, in
which case we'd create the leading directory for the in-stream file,
even though we never actually write the file.

Let's instead create the directories just before opening the file, which
means we'll create only useful directories. Note that this could change
the handling of relative paths if we chdir() in between, but we don't
actually do so; the only permanent chdir is from setup_git_directory()
which runs before either code path (potentially we should take the
pre-setup dir into account to avoid surprising the user, but that's an
orthogonal change).

The test just adapts the existing "override" test to use paths with
leading directories. This checks both that the correct directory is
created (which worked before but was not tested), and that the
overridden one is not (our new fix here).

While we're here, let's also check the error result of
safe_create_leading_directories(). We'd presumably notice any failure
immediately after when we try to open the file itself, but we can give a
more specific error message in this case.

Signed-off-by: Jeff King <peff@peff.net>

diff --git a/fast-import.c b/fast-import.c
index a5b7685e845e7438a9c4d8c55ba09598b054e606..d64609b0838f19a5c158da074b6e957b8782db5f 100644 (file)
--- a/fast-import.c
+++ b/fast-import.c
@@ -1861,6 +1861,12 @@ static void dump_marks(void)
        if (!export_marks_file || (import_marks_file && !import_marks_file_done))
                return;
 
+       if (safe_create_leading_directories_const(export_marks_file)) {
+               failure |= error_errno("unable to create leading directories of %s",
+                                      export_marks_file);
+               return;
+       }
+
        if (hold_lock_file_for_update(&mark_lock, export_marks_file, 0) < 0) {
                failure |= error_errno("Unable to write marks file %s",
                                       export_marks_file);
@@ -3268,7 +3274,6 @@ static void option_active_branches(const char *branches)
 static void option_export_marks(const char *marks)
 {
        export_marks_file = make_fast_import_path(marks);
-       safe_create_leading_directories_const(export_marks_file);
 }
 
 static void option_cat_blob_fd(const char *fd)

diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index c0d04ec3ee19d059b7f2fb15f3e07eaa18ce3bec..1ba20c1f1a2577954bf033f2e334d5890be3399f 100755 (executable)
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -2132,8 +2132,17 @@ test_expect_success 'R: export-marks feature results in a marks file being creat
 '
 
 test_expect_success 'R: export-marks options can be overridden by commandline options' '
-       git fast-import --export-marks=other.marks <input &&
-       grep :1 other.marks
+       cat >input <<-\EOF &&
+       feature export-marks=feature-sub/git.marks
+       blob
+       mark :1
+       data 3
+       hi
+
+       EOF
+       git fast-import --export-marks=cmdline-sub/other.marks <input &&
+       grep :1 cmdline-sub/other.marks &&
+       test_path_is_missing feature-sub
 '
 
 test_expect_success 'R: catch typo in marks file name' '