| From f6d36ce67e58fad3a35a19c3b3d74914be7fa1c3 Mon Sep 17 00:00:00 2001 |
| From: Jeff King <peff@peff.net> |
| Date: Thu, 29 Aug 2019 13:33:48 -0400 |
| Subject: fast-import: delay creating leading directories for export-marks |
| |
| commit 019683025f1b14d7cb671312ab01f7330e9b33e7 upstream. |
| |
| 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> |
| Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> |
| --- |
| fast-import.c | 7 ++++++- |
| t/t9300-fast-import.sh | 13 +++++++++++-- |
| 2 files changed, 17 insertions(+), 3 deletions(-) |
| |
| diff --git a/fast-import.c b/fast-import.c |
| index b9144e90bd..c841953cdc 100644 |
| --- a/fast-import.c |
| +++ b/fast-import.c |
| @@ -1860,6 +1860,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); |
| @@ -3259,7 +3265,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 bf5b937a4c..f6fba4ae16 100755 |
| --- 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' ' |
| -- |
| 2.24.0.393.g34dc348eaf |
| |
