Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Documentation/fetch-options.txt
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
Deepen or shorten the history of a shallow repository to
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Patrick Steinhardt wrote (reply to this):

On Mon, Nov 04, 2024 at 07:02:44PM +0000, Elijah Newren via GitGitGadget wrote:
> diff --git a/builtin/clone.c b/builtin/clone.c
> index 59fcb317a68..93fe6d69659 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -147,7 +147,7 @@ static struct option builtin_clone_options[] = {
>  		    N_("create a shallow clone of that depth")),
>  	OPT_STRING(0, "shallow-since", &option_since, N_("time"),
>  		    N_("create a shallow clone since a specific time")),
> -	OPT_STRING_LIST(0, "shallow-exclude", &option_not, N_("revision"),
> +	OPT_STRING_LIST(0, "shallow-exclude", &option_not, N_("ref"),
>  			N_("deepen history of shallow clone, excluding rev")),

We also need to replace "rev" with "ref" here. We already do that in the
other cases.

Patrick

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Junio C Hamano wrote (reply to this):

Patrick Steinhardt <[email protected]> writes:

> On Mon, Nov 04, 2024 at 07:02:44PM +0000, Elijah Newren via GitGitGadget wrote:
>> diff --git a/builtin/clone.c b/builtin/clone.c
>> index 59fcb317a68..93fe6d69659 100644
>> --- a/builtin/clone.c
>> +++ b/builtin/clone.c
>> @@ -147,7 +147,7 @@ static struct option builtin_clone_options[] = {
>>  		    N_("create a shallow clone of that depth")),
>>  	OPT_STRING(0, "shallow-since", &option_since, N_("time"),
>>  		    N_("create a shallow clone since a specific time")),
>> -	OPT_STRING_LIST(0, "shallow-exclude", &option_not, N_("revision"),
>> +	OPT_STRING_LIST(0, "shallow-exclude", &option_not, N_("ref"),
>>  			N_("deepen history of shallow clone, excluding rev")),
>
> We also need to replace "rev" with "ref" here. We already do that in the
> other cases.

Ah, I missed that one.  Let me locally amend, as there does not seem
to be any other issues.

include all reachable commits after <date>.

--shallow-exclude=<revision>::
--shallow-exclude=<ref>::
Deepen or shorten the history of a shallow repository to
exclude commits reachable from a specified remote branch or tag.
This option can be specified multiple times.
Expand Down
2 changes: 1 addition & 1 deletion Documentation/git-clone.txt
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ corresponding `--mirror` and `--no-tags` options instead.
`--shallow-since=<date>`::
Create a shallow clone with a history after the specified time.

`--shallow-exclude=<revision>`::
`--shallow-exclude=<ref>`::
Create a shallow clone with a history, excluding commits
reachable from a specified remote branch or tag. This option
can be specified multiple times.
Expand Down
2 changes: 1 addition & 1 deletion Documentation/git-fetch-pack.txt
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ be in a separate packet, and the list must end with a flush packet.
Deepen or shorten the history of a shallow repository to
include all reachable commits after <date>.

--shallow-exclude=<revision>::
--shallow-exclude=<ref>::
Deepen or shorten the history of a shallow repository to
exclude commits reachable from a specified remote branch or tag.
This option can be specified multiple times.
Expand Down
2 changes: 1 addition & 1 deletion builtin/clone.c
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ static struct option builtin_clone_options[] = {
N_("create a shallow clone of that depth")),
OPT_STRING(0, "shallow-since", &option_since, N_("time"),
N_("create a shallow clone since a specific time")),
OPT_STRING_LIST(0, "shallow-exclude", &option_not, N_("revision"),
OPT_STRING_LIST(0, "shallow-exclude", &option_not, N_("ref"),
N_("deepen history of shallow clone, excluding rev")),
OPT_BOOL(0, "single-branch", &option_single_branch,
N_("clone only one branch, HEAD or --branch")),
Expand Down
4 changes: 2 additions & 2 deletions builtin/fetch.c
Original file line number Diff line number Diff line change
Expand Up @@ -2216,8 +2216,8 @@ int cmd_fetch(int argc,
N_("deepen history of shallow clone")),
OPT_STRING(0, "shallow-since", &deepen_since, N_("time"),
N_("deepen history of shallow repository based on time")),
OPT_STRING_LIST(0, "shallow-exclude", &deepen_not, N_("revision"),
N_("deepen history of shallow clone, excluding rev")),
OPT_STRING_LIST(0, "shallow-exclude", &deepen_not, N_("ref"),
N_("deepen history of shallow clone, excluding ref")),
OPT_INTEGER(0, "deepen", &deepen_relative,
N_("deepen history of shallow clone")),
OPT_SET_INT_F(0, "unshallow", &unshallow,
Expand Down
4 changes: 2 additions & 2 deletions builtin/pull.c
Original file line number Diff line number Diff line change
Expand Up @@ -218,8 +218,8 @@ static struct option pull_options[] = {
OPT_PASSTHRU_ARGV(0, "shallow-since", &opt_fetch, N_("time"),
N_("deepen history of shallow repository based on time"),
0),
OPT_PASSTHRU_ARGV(0, "shallow-exclude", &opt_fetch, N_("revision"),
N_("deepen history of shallow clone, excluding rev"),
OPT_PASSTHRU_ARGV(0, "shallow-exclude", &opt_fetch, N_("ref"),
N_("deepen history of shallow clone, excluding ref"),
0),
OPT_PASSTHRU_ARGV(0, "deepen", &opt_fetch, N_("n"),
N_("deepen history of shallow clone"),
Expand Down
7 changes: 7 additions & 0 deletions t/t5500-fetch-pack.sh
Original file line number Diff line number Diff line change
Expand Up @@ -926,6 +926,13 @@ test_expect_success 'fetch exclude tag one' '
test_cmp expected actual
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Patrick Steinhardt wrote (reply to this):

On Mon, Nov 04, 2024 at 07:02:43PM +0000, Elijah Newren via GitGitGadget wrote:
> diff --git a/upload-pack.c b/upload-pack.c
> index 6d6e0f9f980..640d45295e1 100644
> --- a/upload-pack.c
> +++ b/upload-pack.c
> @@ -1025,10 +1025,14 @@ static int process_deepen_not(const char *line, struct oidset *deepen_not, int *
>  {
>  	const char *arg;
>  	if (skip_prefix(line, "deepen-not ", &arg)) {
> +		int cnt;
>  		char *ref = NULL;
>  		struct object_id oid;
> -		if (expand_ref(the_repository, arg, strlen(arg), &oid, &ref) != 1)
> +		cnt = expand_ref(the_repository, arg, strlen(arg), &oid, &ref);
> +		if (cnt > 1)
>  			die("git upload-pack: ambiguous deepen-not: %s", line);
> +		if (cnt < 1)
> +			die("git upload-pack: deepen-not is not a ref: %s", line);

I was wondering whether `expand_ref()` could ever return negative, but
after reading through its implementation that doesn't seem to be the
case. It's somewhat misleading that it returns `int`, as `size_t` would
convey the return value in a better spirit.

Anyway, that is not an issue of this patch series, and the change looks
good.

Patrick

'

test_expect_success 'fetch exclude tag one as revision' '
test_when_finished rm -f rev err &&
git -C shallow-exclude rev-parse one >rev &&
test_must_fail git -C shallow12 fetch --shallow-exclude $(cat rev) origin 2>err &&
grep "deepen-not is not a ref:" err
'

test_expect_success 'fetching deepen' '
test_create_repo shallow-deepen &&
(
Expand Down
6 changes: 5 additions & 1 deletion upload-pack.c
Original file line number Diff line number Diff line change
Expand Up @@ -1025,10 +1025,14 @@ static int process_deepen_not(const char *line, struct oidset *deepen_not, int *
{
const char *arg;
if (skip_prefix(line, "deepen-not ", &arg)) {
int cnt;
char *ref = NULL;
struct object_id oid;
if (expand_ref(the_repository, arg, strlen(arg), &oid, &ref) != 1)
cnt = expand_ref(the_repository, arg, strlen(arg), &oid, &ref);
if (cnt > 1)
die("git upload-pack: ambiguous deepen-not: %s", line);
if (cnt < 1)
die("git upload-pack: deepen-not is not a ref: %s", line);
oidset_insert(deepen_not, &oid);
free(ref);
*deepen_rev_list = 1;
Expand Down
Loading