Re: git format-patch can clobber existing patch

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Σταύρος Ντέντος  <stdedos@xxxxxxxxx> writes:

> During my work flow, I tend to use `git format-patch` to move patches
> around (nasty, I know, but bear with me)
>
> It has occured to me that, it is possible that `git format-patch` can
> clobber existing files, if they match in the name and in the "patch
> order".
> However, if the patch is one, then, it will normally start with `0001-x.patch`
>
> It was fine for me when I pushed updates of the same "patch series".
> Now that I wanted to diff a "previous patch file" with the patch
> upstream, however, it was almost a "disaster". :-)
>
> Would it make sense / be easy enough to have some clobbering check / flag?

Given that use of '-o' to redirect to a fresh/new directory would
reduce the risk of such clobbering, and use of '-v' to force
different filenames would reduce the risk of such clobbering,
it seems to me that aborting the operation when we fail to open
the output, without any option to override and allow clobbering,
would make sense.  If existing files record 4 patch series
0001-x.patch, 0002-y.patch, 0003-z.patch, and 0004-w.patch, and you
generate with "format-patch --allow-clobbering" a three-patch series,
it would overwrite 0001 thru 0003 but will not remove 0004, so the
end result will still be confusing.

This is not even compile-tested, but something along these lines may
work.  I am reasonably sure that existing tests won't let this patch
alone pass, as some may depend on being able to overwrite output
files left behind by previous tests.

 builtin/log.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/builtin/log.c b/builtin/log.c
index 3e145fe502..cb7a9eb7f9 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -864,6 +864,16 @@ static int git_format_config(const char *var, const char *value, void *cb)
 static const char *output_directory = NULL;
 static int outdir_offset;
 
+static FILE *fopen_excl(const char *filename)
+{
+	int fd = open(filename, O_CREAT | O_EXCL | O_WRONLY, 0666);
+	if (fd < 0) {
+		error_errno("%s", filename);
+		return NULL;
+	}
+	return fdopen(fd, "w");
+}
+
 static int open_next_file(struct commit *commit, const char *subject,
 			 struct rev_info *rev, int quiet)
 {
@@ -890,7 +900,7 @@ static int open_next_file(struct commit *commit, const char *subject,
 	if (!quiet)
 		printf("%s\n", filename.buf + outdir_offset);
 
-	if ((rev->diffopt.file = fopen(filename.buf, "w")) == NULL) {
+	if ((rev->diffopt.file = fopen_excl(filename.buf)) == NULL) {
 		error_errno(_("Cannot open patch file %s"), filename.buf);
 		strbuf_release(&filename);
 		return -1;



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux