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

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

 



Ramkumar Ramachandra <artagnon@xxxxxxxxx> writes:

> 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@xxxxxxxxx>
> Signed-off-by: Ramkumar Ramachandra <artagnon@xxxxxxxxx>

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.

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