@matheusbsbs / matheus-bagliano / commits / 92481d1b26

merge-ort: return early when failing to write a blob

In the previous commit, we fixed a segmentation fault when a tree object could not be written. However, before the tree object is written, `merge-ort` wants to write out a blob object (except in cases where the merge results in a blob that already exists in the database). And this can fail, too, but we ignore that write failure so far. Let's pay close attention and error out early if the blob could not be written. This reduces the error output of t4301.25 ("merge-ort fails gracefully in a read-only repository") from: error: insufficient permission for adding an object to repository database ./objects error: error: Unable to add numbers to database error: insufficient permission for adding an object to repository database ./objects error: error: Unable to add greeting to database error: insufficient permission for adding an object to repository database ./objects fatal: failure to merge to: error: insufficient permission for adding an object to repository database ./objects error: error: Unable to add numbers to database fatal: failure to merge This is _not_ just a cosmetic change: Even though one might assume that the operation would have failed anyway at the point when the new tree object is written (and the corresponding tree object _will_ be new if it contains a blob that is new), but that is not so: As pointed out by Elijah Newren, when Git has previously been allowed to add loose objects via `sudo` calls, it is very possible that the blob object cannot be written (because the corresponding `.git/objects/??/` directory may be owned by `root`) but the tree object can be written (because the corresponding objects directory is owned by the current user). This would result in a corrupt repository because it is missing the blob object, and with this here patch we prevent that. Note: This patch adjusts two variable declarations from `unsigned` to `int` because their purpose is to hold the return value of `handle_content_merge()`, which is of type `int`. The existing users of those variables are only interested whether that variable is zero or non-zero, therefore this type change does not affect the existing code. Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Johannes Schindelin committed Sep 28, 2022 at 07:29 UTC 92481d1b26ab57525f5efe01d01c7a3d337b8df7

1 file changed +19 -9

merge-ort.c

+19 -9

@@ -2807,6 +2807,8 @@ static int process_renames(struct merge_options *opt,

2807 pathnames,

2808 1 + 2 * opt->priv->call_depth,

2809 &merged);

2810 + if (clean_merge < 0)

2811 + return -1;

2812 if (!clean_merge &&

2813 merged.mode == side1->stages[1].mode &&

2814 oideq(&merged.oid, &side1->stages[1].oid))

@@ -2916,7 +2918,7 @@ static int process_renames(struct merge_options *opt,

2918 struct version_info merged;

2919

2920 struct conflict_info *base, *side1, *side2;

2919 - unsigned clean;

2921 + int clean;

2922

2923 pathnames[0] = oldpath;

2924 pathnames[other_source_index] = oldpath;

@@ -2937,6 +2939,8 @@ static int process_renames(struct merge_options *opt,

2939 pathnames,

2940 1 + 2 * opt->priv->call_depth,

2941 &merged);

2942 + if (clean < 0)

2943 + return -1;

2944

2945 memcpy(&newinfo->stages[target_index], &merged,

2946 sizeof(merged));

@@ -3806,10 +3810,10 @@ static int write_completed_directory(struct merge_options *opt,

3810 }

3811

3812 /* Per entry merge function */

3809 -static void process_entry(struct merge_options *opt,

3810 - const char *path,

3811 - struct conflict_info *ci,

3812 - struct directory_versions *dir_metadata)

3813 +static int process_entry(struct merge_options *opt,

3814 + const char *path,

3815 + struct conflict_info *ci,

3816 + struct directory_versions *dir_metadata)

3817 {

3818 int df_file_index = 0;

3819

@@ -3823,7 +3827,7 @@ static void process_entry(struct merge_options *opt,

3827 record_entry_for_tree(dir_metadata, path, &ci->merged);

3828 if (ci->filemask == 0)

3829 /* nothing else to handle */

3826 - return;

3830 + return 0;

3831 assert(ci->df_conflict);

3832 }

3833

@@ -3870,7 +3874,7 @@ static void process_entry(struct merge_options *opt,

3874 */

3875 if (ci->filemask == 1) {

3876 ci->filemask = 0;

3873 - return;

3877 + return 0;

3878 }

3879

3880 /*

@@ -4065,7 +4069,7 @@ static void process_entry(struct merge_options *opt,

4069 } else if (ci->filemask >= 6) {

4070 /* Need a two-way or three-way content merge */

4071 struct version_info merged_file;

4068 - unsigned clean_merge;

4072 + int clean_merge;

4073 struct version_info *o = &ci->stages[0];

4074 struct version_info *a = &ci->stages[1];

4075 struct version_info *b = &ci->stages[2];

@@ -4074,6 +4078,8 @@ static void process_entry(struct merge_options *opt,

4078 ci->pathnames,

4079 opt->priv->call_depth * 2,

4080 &merged_file);

4081 + if (clean_merge < 0)

4082 + return -1;

4083 ci->merged.clean = clean_merge &&

4084 !ci->df_conflict && !ci->path_conflict;

4085 ci->merged.result.mode = merged_file.mode;

@@ -4169,6 +4175,7 @@ static void process_entry(struct merge_options *opt,

4175

4176 /* Record metadata for ci->merged in dir_metadata */

4177 record_entry_for_tree(dir_metadata, path, &ci->merged);

4178 + return 0;

4179 }

4180

4181 static void prefetch_for_content_merges(struct merge_options *opt,

@@ -4285,7 +4292,10 @@ static int process_entries(struct merge_options *opt,

4292 record_entry_for_tree(&dir_metadata, path, mi);

4293 else {

4294 struct conflict_info *ci = (struct conflict_info *)mi;

4288 - process_entry(opt, path, ci, &dir_metadata);

4295 + if (process_entry(opt, path, ci, &dir_metadata) < 0) {

4296 + ret = -1;

4297 + goto cleanup;

4298 + };

4299 }

4300 }

4301 trace2_region_leave("merge", "processing", opt->repo);