Re: [PATCH v2 2/3] Allow for MERGE_MODE to specify more then one mode

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

 



Kacper Kornet <draenog@xxxxxxxxxxxxx> writes:

> Presently only one merge mode exists: non-fast-forward. But in future
> the second one (transpose-parents) will be added, so the need to read
> all lines of MERGE_MODE.
>
> Signed-off-by: Kacper Kornet <draenog@xxxxxxxxxxxxx>
> ---
>  builtin/commit.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 273332f..ee0e884 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -1427,7 +1427,6 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
>  	unsigned char sha1[20];
>  	struct ref_lock *ref_lock;
>  	struct commit_list *parents = NULL, **pptr = &parents;
> -	struct stat statbuf;
>  	int allow_fast_forward = 1;
>  	struct commit *current_head = NULL;
>  	struct commit_extra_header *extra = NULL;
> @@ -1481,11 +1480,12 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
>  
>  		if (!reflog_msg)
>  			reflog_msg = "commit (merge)";
> -		if (!stat(git_path("MERGE_MODE"), &statbuf)) {
> -			if (strbuf_read_file(&sb, git_path("MERGE_MODE"), 0) < 0)
> -				die_errno(_("could not read MERGE_MODE"));
> -			if (!strcmp(sb.buf, "no-ff"))
> -				allow_fast_forward = 0;
> +		if((fp = fopen(git_path("MERGE_MODE"), "r"))) {

Style: s/if((fp/if ((fp/;

> +			while (strbuf_getline(&m, fp, '\n') != EOF) {
> +				if (!strcmp(m.buf, "no-ff"))
> +					allow_fast_forward = 0;
> +			}
> +			fclose(fp);

This needs a bit more careful planning for interacting with other
people's programs, I suspect.

Your updated builtin/merge.c may write an extra LF after no-ff to
make this parser to grok it, but it is entirely plausible that
people have their own Porcelain that writes "no-ff" without LF
(because that is what we read from this file, and I suspect the
current code would ignore "no-ff\n").

At least strbuf_getline() would give us "no-ff" when either "no-ff"
or "no-ff\n" terminates the file, so updated code would be able to
grok what other people would write, but if other people want to read
MERGE_MODE we write, at least we shouldn't break them when we only
write no-ff in it (once you start writing "reverse-parents" in the
file, they will be broken anyway, as they do not currently expect
such token in this file).

I am starting to wonder if this is worth it, though...
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[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]