Skip to content

Conversation

@dscho
Copy link
Member

@dscho dscho commented Nov 29, 2025

Cc: Toon Claes toon@iotcl.com
cc: Derrick Stolee stolee@gmail.com

In a sparse checkout, a user might want to run `last-modified` on a
directory outside the worktree.

And even in non-sparse checkouts, a user might need to run that command
on a directory that does not exist in the worktree.

These use cases should be supported via the `--` separator between
revision and file arguments, which is even advertised in the
documentation. This patch fixes a tiny bug that prevents that from
working.

This fixes git-for-windows#5978

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho dscho force-pushed the last-modified-vs-sparse-checkouts branch from a430cf4 to 7040ae9 Compare November 29, 2025 10:00
@dscho
Copy link
Member Author

dscho commented Nov 29, 2025

/submit

@gitgitgadget
Copy link

gitgitgadget bot commented Nov 29, 2025

Submitted as pull.2013.git.1764423826908.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-2013/dscho/last-modified-vs-sparse-checkouts-v1

To fetch this version to local tag pr-2013/dscho/last-modified-vs-sparse-checkouts-v1:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-2013/dscho/last-modified-vs-sparse-checkouts-v1

@gitgitgadget
Copy link

gitgitgadget bot commented Nov 30, 2025

On the Git mailing list, Derrick Stolee wrote (reply to this):

On 11/29/2025 8:43 AM, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> 
> In a sparse checkout, a user might want to run `last-modified` on a
> directory outside the worktree.
> 
> And even in non-sparse checkouts, a user might need to run that command
> on a directory that does not exist in the worktree.
> 
> These use cases should be supported via the `--` separator between
> revision and file arguments, which is even advertised in the
> documentation. This patch fixes a tiny bug that prevents that from
> working.

>  	argc = parse_options(argc, argv, prefix, last_modified_options,
>  			     last_modified_usage,
> -			     PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN_OPT);
> +			     PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN_OPT |
> +			     PARSE_OPT_KEEP_DASHDASH);

I'm intrigued that this is the only fix that was required.

> +test_expect_success 'last-modified in sparse checkout' '
> +	test_when_finished "git sparse-checkout disable" &&
> +	git sparse-checkout set b &&
> +	check_last_modified -- a <<-\EOF

Would we expect this to work without the '--'? Should it
fail for a directory that exists at HEAD but is outside of
the sparse-checkout?

Thanks,
-Stolee

@gitgitgadget
Copy link

gitgitgadget bot commented Nov 30, 2025

User Derrick Stolee <stolee@gmail.com> has been added to the cc: list.

@gitgitgadget
Copy link

gitgitgadget bot commented Dec 3, 2025

On the Git mailing list, Toon Claes wrote (reply to this):

Derrick Stolee <stolee@gmail.com> writes:

> On 11/29/2025 8:43 AM, Johannes Schindelin via GitGitGadget wrote:
>> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>> 
>> In a sparse checkout, a user might want to run `last-modified` on a
>> directory outside the worktree.
>> 
>> And even in non-sparse checkouts, a user might need to run that command
>> on a directory that does not exist in the worktree.
>> 
>> These use cases should be supported via the `--` separator between
>> revision and file arguments, which is even advertised in the
>> documentation. This patch fixes a tiny bug that prevents that from
>> working.
>
>>  	argc = parse_options(argc, argv, prefix, last_modified_options,
>>  			     last_modified_usage,
>> -			     PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN_OPT);
>> +			     PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN_OPT |
>> +			     PARSE_OPT_KEEP_DASHDASH);
>
> I'm intrigued that this is the only fix that was required.

I like it!

>> +test_expect_success 'last-modified in sparse checkout' '
>> +	test_when_finished "git sparse-checkout disable" &&
>> +	git sparse-checkout set b &&
>> +	check_last_modified -- a <<-\EOF
>
> Would we expect this to work without the '--'? Should it
> fail for a directory that exists at HEAD but is outside of
> the sparse-checkout?

I don't think we need to complicate things that much. Arguments after
the '--' are handled as pathspecs. If no paths match that pathspec, the
output is simply nothing.

Just to demonstrate:

On 'master':

    $ git last-modified a
    fatal: ambiguous argument 'a': unknown revision or path not in the working tree.
    Use '--' to separate paths from revisions, like this:
    'git <command> [<revision>...] -- [<file>...]'

    $ git last-modified -- a
    fatal: ambiguous argument 'a': unknown revision or path not in the working tree.
    Use '--' to separate paths from revisions, like this:
    'git <command> [<revision>...] -- [<file>...]'

With this patch applied:

    $ git last-modified a
    fatal: ambiguous argument 'a': unknown revision or path not in the working tree.
    Use '--' to separate paths from revisions, like this:
    'git <command> [<revision>...] -- [<file>...]'

    $ git last-modified -- a

    $ echo $?
    0

I agree this behavior is better. Thanks for this fix. I approve.

-- 
Cheers,
Toon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant