All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Gummerer <t.gummerer@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Joel Teichroeb" <joel@teichroeb.net>,
	"Johannes Schindelin" <Johannes.Schindelin@gmx.de>,
	"Jeff King" <peff@peff.net>,
	"Martin Ågren" <martin.agren@gmail.com>,
	"Thomas Gummerer" <t.gummerer@gmail.com>
Subject: [PATCH v3 0/3] make sure stash refreshes the index properly
Date: Tue,  3 Sep 2019 20:10:38 +0100	[thread overview]
Message-ID: <20190903191041.10470-1-t.gummerer@gmail.com> (raw)
In-Reply-To: <20190829182748.43802-1-t.gummerer@gmail.com>

Thanks Martin and Junio for the comments on the previous round.

Changes compared to the previous round:
- Document that when failing to refresh the index, the result won't be
  written to disk.
- Rollback the lock file if refreshing the index fails, so we don't
  end up with a lock file that can't be rolled back or committed after
  the function returns
- Some small tweaks in the commit message and documentation of the
  function.

Range-diff below:

1:  1f25fe227c ! 1:  7cc9f5fff4 factor out refresh_and_write_cache function
    @@ Commit message
         factor out refresh_and_write_cache function
     
         Getting the lock for the index, refreshing it and then writing it is a
    -    pattern that happens more than once throughout the codebase.  Factor
    -    out the refresh_and_write_cache function from builtin/am.c to
    -    read-cache.c, so it can be re-used in other places in a subsequent
    -    commit.
    +    pattern that happens more than once throughout the codebase, and isn't
    +    trivial to get right.  Factor out the refresh_and_write_cache function
    +    from builtin/am.c to read-cache.c, so it can be re-used in other
    +    places in a subsequent commit.
     
         Note that we return different error codes for failing to refresh the
         cache, and failing to write the index.  The current caller only cares
    @@ cache.h: void fill_stat_cache_info(struct index_state *istate, struct cache_entr
     + * 'COMMIT_LOCK | write_flags' is passed to 'write_locked_index()', so
     + * the lockfile is always either committed or rolled back.
     + *
    -+ * Return 1 if refreshing the cache failed, -1 if writing the cache to
    -+ * disk failed, 0 on success.
    ++ * Return 1 if refreshing the index returns an error, -1 if writing
    ++ * the index to disk fails, 0 on success.
    ++ *
    ++ * Note that if refreshing the index returns an error, we don't write
    ++ * the result to disk.
     + */
     +int repo_refresh_and_write_index(struct repository*, unsigned int refresh_flags, unsigned int write_flags, const struct pathspec *, char *seen, const char *header_msg);
     +
    @@ read-cache.c: static void show_file(const char * fmt, const char * name, int in_
     +	struct lock_file lock_file = LOCK_INIT;
     +
     +	repo_hold_locked_index(repo, &lock_file, LOCK_DIE_ON_ERROR);
    -+	if (refresh_index(repo->index, refresh_flags, pathspec, seen, header_msg))
    ++	if (refresh_index(repo->index, refresh_flags, pathspec, seen, header_msg)) {
    ++		rollback_lock_file(&lock_file);
     +		return 1;
    ++	}
     +	if (write_locked_index(repo->index, &lock_file, COMMIT_LOCK | write_flags))
     +		return -1;
     +	return 0;
2:  148a65d649 = 2:  0367d938b1 merge: use refresh_and_write_cache
3:  e0f6815192 = 3:  8ed3df9fec stash: make sure to write refreshed cache

Thomas Gummerer (3):
  factor out refresh_and_write_cache function
  merge: use refresh_and_write_cache
  stash: make sure to write refreshed cache

 builtin/am.c     | 16 ++--------------
 builtin/merge.c  | 13 +++----------
 builtin/stash.c  | 11 +++++++----
 cache.h          | 16 ++++++++++++++++
 read-cache.c     | 19 +++++++++++++++++++
 t/t3903-stash.sh | 16 ++++++++++++++++
 6 files changed, 63 insertions(+), 28 deletions(-)

-- 
2.23.0.rc2.194.ge5444969c9

  parent reply	other threads:[~2019-09-03 19:10 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-27 10:14 [PATCH 0/3] make sure stash refreshes the index properly Thomas Gummerer
2019-08-27 10:14 ` [PATCH 1/3] factor out refresh_and_write_cache function Thomas Gummerer
2019-08-28 15:49   ` Martin Ågren
2019-08-29 17:59     ` Thomas Gummerer
2019-08-27 10:14 ` [PATCH 2/3] merge: use refresh_and_write_cache Thomas Gummerer
2019-08-28 15:52   ` Martin Ågren
2019-08-29 18:00     ` Thomas Gummerer
2019-08-27 10:14 ` [PATCH 3/3] stash: make sure to write refreshed cache Thomas Gummerer
2019-08-29 18:27 ` [PATCH v2 0/3] make sure stash refreshes the index properly Thomas Gummerer
2019-08-29 18:27   ` [PATCH v2 1/3] factor out refresh_and_write_cache function Thomas Gummerer
2019-08-30 15:07     ` Martin Ågren
2019-08-30 17:06       ` Junio C Hamano
2019-09-02 17:15         ` Thomas Gummerer
2019-09-03 17:43           ` Junio C Hamano
2019-08-29 18:27   ` [PATCH v2 2/3] merge: use refresh_and_write_cache Thomas Gummerer
2019-08-29 18:27   ` [PATCH v2 3/3] stash: make sure to write refreshed cache Thomas Gummerer
2019-09-03 19:10   ` Thomas Gummerer [this message]
2019-09-03 19:10     ` [PATCH v3 1/3] factor out refresh_and_write_cache function Thomas Gummerer
2019-09-05 22:00       ` Junio C Hamano
2019-09-06 14:18         ` Thomas Gummerer
2019-09-11 10:57           ` Johannes Schindelin
2019-09-11 17:52             ` Thomas Gummerer
2019-09-12 16:46               ` Junio C Hamano
2019-09-03 19:10     ` [PATCH v3 2/3] merge: use refresh_and_write_cache Thomas Gummerer
2019-09-03 19:10     ` [PATCH v3 3/3] stash: make sure to write refreshed cache Thomas Gummerer
2019-09-11 18:20     ` [PATCH v4 0/3] make sure stash refreshes the index properly Thomas Gummerer
2019-09-11 18:20       ` [PATCH v4 1/3] factor out refresh_and_write_cache function Thomas Gummerer
2019-09-11 18:20       ` [PATCH v4 2/3] merge: use refresh_and_write_cache Thomas Gummerer
2019-09-11 18:20       ` [PATCH v4 3/3] stash: make sure to write refreshed cache Thomas Gummerer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190903191041.10470-1-t.gummerer@gmail.com \
    --to=t.gummerer@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=joel@teichroeb.net \
    --cc=martin.agren@gmail.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.