Re: [PATCH] fast-import: Create import-marks file when necessary

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 9, 2011, 22:48 UTC
Message-ID: <7vwrmd7kxe.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1294601637-18416-1-git-send-email-artagnon@gmail.com>

Ramkumar Ramachandra <artagnon@gmail.com> writes:

Show 7 quoted lines

> When both --import-marks and --export-marks are given, and specify the
> same file, that file should be created if it doesn't exist. Without
> this patch, the caller would have to check for the existence of a
> marks file and vary its fast-import invocation accordingly.
>
> Requested-by: Jonathan Nieder <jrnieder@gmail.com>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>

Do you mean that you are solving a problem for bootstrapping a script that says "read from this file if exists, do work, and then write to the same file for later invocation"?

Assuming that is what you are solving, why do you create the file in your patch in the import codepath? I would imagine a sane logic flow for "read if exists, do work and then write to the same" would be more like:

	if (import_marks) {
		FILE = *f = fopen(import_marks);
                if (f)
                	read_marks();
		else if (errno == ENOENT && !strcmp(import_marks, export_marks))
                	; /* Bootstrapping with "read if exists" */
		else
                	barf();
	}

no?

Also I find it a bad taste to use "The user gave the same file to import and export" as a hint for "read if exists".

The first two lines of the proposed commit message is way suboptimal, if the problem is "Not allowing read-if-exists-but-do-not-barf-if-missing makes it hard to bootstrap repeated invocations that use a single import/export marks file as the state-keeping mechanism".

It does not have to be created if it doesn't exist---the program only needs not to barf. The condition does not have to be "if they are the same file". The caller may want to script

	command --import-if-exists=state --export=new-state+ &&
        mv new-state+ state

to make sure that when the underlying command fails, the previous state is still kept intact without getting overwritten.