@matheusbsbs / matheus-bagliano / commits / 0c668f559c

blame: tighten command line parser

The command line parser of "git blame" is prepared to take an ancient odd argument order "blame <path> <rev>" in addition to the usual "blame [<rev>] <path>". It has at least two negative ramifications: - In order to tell these two apart, it checks if the last command line argument names a path in the working tree, using file_exists(). However, "blame <rev> <path>" is a request to explain each and every line in the contents of <path> stored in revision <rev> and does not need to have a working tree version of the file. A check with file_exists() is simply wrong. - To coerce that mistaken file_exists() check to work, the code calls setup_work_tree() before doing so, because the path it has is relative to the top-level of the project tree. However, "blame <rev> <path>" MUST be usable even in a bare repository, and there is no reason for letting setup_work_tree() complain and die with "This operation must be run in a work tree". To correct the former, switch to check if the last token is a revision (and if so, parse arguments using "blame <path> <rev>" rule). Correct the latter by getting rid of setup_work_tree() and file_exists() check--the only case the call to this function matters is when we are running "blame <path>" (i.e. no starting revision and asking to blame the working tree file at <path>, digging through the HEAD revision), but there is a call in setup_scoreboard() just before it calls fake_working_tree_commit(). Signed-off-by: Junio C Hamano <gitster@pobox.com>

Junio C Hamano committed Feb 5, 2018 at 15:12 UTC 0c668f559cc149720b999d7bd1c0060e9fd7caf3

1 file changed +14 -6

builtin/blame.c

+14 -6

@@ -649,6 +649,15 @@ static int blame_move_callback(const struct option *option, const char *arg, int

649 return 0;

650 }

651

652 +static int is_a_rev(const char *name)

653 +{

654 + struct object_id oid;

655 +

656 + if (get_oid(name, &oid))

657 + return 0;

658 + return OBJ_NONE < sha1_object_info(oid.hash, NULL);

659 +}

660 +

661 int cmd_blame(int argc, const char **argv, const char *prefix)

662 {

663 struct rev_info revs;

@@ -845,16 +854,15 @@ parse_done:

854 } else {

855 if (argc < 2)

856 usage_with_options(blame_opt_usage, options);

848 - path = add_prefix(prefix, argv[argc - 1]);

849 - if (argc == 3 && !file_exists(path)) { /* (2b) */

857 + if (argc == 3 && is_a_rev(argv[argc - 1])) { /* (2b) */

858 path = add_prefix(prefix, argv[1]);

859 argv[1] = argv[2];

860 + } else { /* (2a) */

861 + if (argc == 2 && is_a_rev(argv[1]) && !get_git_work_tree())

862 + die("missing <path> to blame");

863 + path = add_prefix(prefix, argv[argc - 1]);

864 }

865 argv[argc - 1] = "--";

854 -

855 - setup_work_tree();

856 - if (!file_exists(path))

857 - die_errno("cannot stat path '%s'", path);

866 }

867

868 revs.disable_stdin = 1;