[2/3] merge: use refresh_and_write_cache
diff mbox series

Message ID 20190827101408.76757-3-t.gummerer@gmail.com
State New
Headers show
Series
  • make sure stash refreshes the index properly
Related show

Commit Message

Thomas Gummerer Aug. 27, 2019, 10:14 a.m. UTC
Use the 'refresh_and_write_cache()' convenience function introduced in
the last commit, instead of refreshing and writing the index manually
in merge.c

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 builtin/merge.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

Comments

Martin Ågren Aug. 28, 2019, 3:52 p.m. UTC | #1
On Tue, 27 Aug 2019 at 12:15, Thomas Gummerer <t.gummerer@gmail.com> wrote:

>         struct lock_file lock = LOCK_INIT;
>         const char *head_arg = "HEAD";
>
> -       hold_locked_index(&lock, LOCK_DIE_ON_ERROR);
> -       refresh_cache(REFRESH_QUIET);
> -       if (write_locked_index(&the_index, &lock,
> -                              COMMIT_LOCK | SKIP_IF_UNCHANGED))
> -               return error(_("Unable to write index."));
> +       if (refresh_and_write_cache(REFRESH_QUIET, COMMIT_LOCK | SKIP_IF_UNCHANGED) < 0)
> +               return -1;

I wondered why you didn't drop the `struct lock_file`, but it turns out
we still need it further down.

>         if (!strcmp(strategy, "recursive") || !strcmp(strategy, "subtree")) {
>                 int clean, x;

What you could do, I guess, is to move its declaration to around here.
Probably not worth a re-roll.

> @@ -860,13 +857,9 @@ static int merge_trivial(struct commit *head, struct commit_list *remoteheads)
>  {
>         struct object_id result_tree, result_commit;
>         struct commit_list *parents, **pptr = &parents;
> -       struct lock_file lock = LOCK_INIT;
>
> -       hold_locked_index(&lock, LOCK_DIE_ON_ERROR);
> -       refresh_cache(REFRESH_QUIET);
> -       if (write_locked_index(&the_index, &lock,
> -                              COMMIT_LOCK | SKIP_IF_UNCHANGED))
> -               return error(_("Unable to write index."));
> +       if (refresh_and_write_cache(REFRESH_QUIET, COMMIT_LOCK | SKIP_IF_UNCHANGED) < 0)
> +               return -1;

Here you do drop the `struct lock_file` entirely, ok.



Martin
Thomas Gummerer Aug. 29, 2019, 6 p.m. UTC | #2
On 08/28, Martin Ågren wrote:
> On Tue, 27 Aug 2019 at 12:15, Thomas Gummerer <t.gummerer@gmail.com> wrote:
> 
> >         struct lock_file lock = LOCK_INIT;
> >         const char *head_arg = "HEAD";
> >
> > -       hold_locked_index(&lock, LOCK_DIE_ON_ERROR);
> > -       refresh_cache(REFRESH_QUIET);
> > -       if (write_locked_index(&the_index, &lock,
> > -                              COMMIT_LOCK | SKIP_IF_UNCHANGED))
> > -               return error(_("Unable to write index."));
> > +       if (refresh_and_write_cache(REFRESH_QUIET, COMMIT_LOCK | SKIP_IF_UNCHANGED) < 0)
> > +               return -1;
> 
> I wondered why you didn't drop the `struct lock_file`, but it turns out
> we still need it further down.
> 
> >         if (!strcmp(strategy, "recursive") || !strcmp(strategy, "subtree")) {
> >                 int clean, x;
> 
> What you could do, I guess, is to move its declaration to around here.
> Probably not worth a re-roll.

I'll re-roll anyway for the things you spotted in the first patch, so
I'll drop it down here while I'm at it, thanks!

> > @@ -860,13 +857,9 @@ static int merge_trivial(struct commit *head, struct commit_list *remoteheads)
> >  {
> >         struct object_id result_tree, result_commit;
> >         struct commit_list *parents, **pptr = &parents;
> > -       struct lock_file lock = LOCK_INIT;
> >
> > -       hold_locked_index(&lock, LOCK_DIE_ON_ERROR);
> > -       refresh_cache(REFRESH_QUIET);
> > -       if (write_locked_index(&the_index, &lock,
> > -                              COMMIT_LOCK | SKIP_IF_UNCHANGED))
> > -               return error(_("Unable to write index."));
> > +       if (refresh_and_write_cache(REFRESH_QUIET, COMMIT_LOCK | SKIP_IF_UNCHANGED) < 0)
> > +               return -1;
> 
> Here you do drop the `struct lock_file` entirely, ok.
> 
> 
> 
> Martin

Patch
diff mbox series

diff --git a/builtin/merge.c b/builtin/merge.c
index e2ccbc44e2..b5e31ce283 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -691,11 +691,8 @@  static int try_merge_strategy(const char *strategy, struct commit_list *common,
 	struct lock_file lock = LOCK_INIT;
 	const char *head_arg = "HEAD";
 
-	hold_locked_index(&lock, LOCK_DIE_ON_ERROR);
-	refresh_cache(REFRESH_QUIET);
-	if (write_locked_index(&the_index, &lock,
-			       COMMIT_LOCK | SKIP_IF_UNCHANGED))
-		return error(_("Unable to write index."));
+	if (refresh_and_write_cache(REFRESH_QUIET, COMMIT_LOCK | SKIP_IF_UNCHANGED) < 0)
+		return -1;
 
 	if (!strcmp(strategy, "recursive") || !strcmp(strategy, "subtree")) {
 		int clean, x;
@@ -860,13 +857,9 @@  static int merge_trivial(struct commit *head, struct commit_list *remoteheads)
 {
 	struct object_id result_tree, result_commit;
 	struct commit_list *parents, **pptr = &parents;
-	struct lock_file lock = LOCK_INIT;
 
-	hold_locked_index(&lock, LOCK_DIE_ON_ERROR);
-	refresh_cache(REFRESH_QUIET);
-	if (write_locked_index(&the_index, &lock,
-			       COMMIT_LOCK | SKIP_IF_UNCHANGED))
-		return error(_("Unable to write index."));
+	if (refresh_and_write_cache(REFRESH_QUIET, COMMIT_LOCK | SKIP_IF_UNCHANGED) < 0)
+		return -1;
 
 	write_tree_trivial(&result_tree);
 	printf(_("Wonderful.\n"));