A few --shallow-exclude fixes #1822
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
/submit |
upload-pack.c takes any --shallow-exclude argument(s) from
clone/fetch/etc. and passes them through expand_ref(). If it does not
get back exactly one ref from the call to expand_ref(), it will die with
the following error:
fatal: git upload-pack: ambiguous deepen-not: %s
Given that the documentation suggests to users that --shallow-exclude
accepts a revision rather than a ref (which will be corrected in a
subsequent commit), users may try to pass a revision. In such a case,
expand_ref() will return 0 matches, but the error message we print will
be misleading since "ambiguous" suggests there are multiple matches.
Provide a clearer error message for such a case.
Signed-off-by: Elijah Newren <[email protected]>
The documentation for the --shallow-exclude option to clone/fetch/etc. claims that the option takes a revision, but it does not. As per upload-pack.c's process_deepen_not(), it passes the option to expand_ref() and dies if it does not find exactly one ref matching the name passed. Further, this has always been the case ever since these options were introduced by the commits merged in a460ea4 (Merge branch 'nd/shallow-deepen', 2016-10-10). Fix the documentation to match the implementation. Signed-off-by: Elijah Newren <[email protected]>
newren
force-pushed
the
shallow-exclude
branch
from
9408dd9 to
4c7f2db
Compare
|
/submit |
|
Submitted as [email protected] To fetch this version into To fetch this version to local tag |
|
On the Git mailing list, Junio C Hamano wrote (reply to this): "Elijah Newren via GitGitGadget" <[email protected]> writes:
> Here's a couple patches to fix up an error message and the documentation
> around the --shallow-exclude option.
>
> Elijah Newren (2):
> upload-pack: fix ambiguous error message
> doc: correct misleading descriptions for --shallow-exclude
Both patches look sensible.
Will queue. Thanks. |
|
This patch series was integrated into seen via git@75d71b5. |
![gitgitgadget[bot]](https://avatars.githubusercontent.com/in/12836?s=60&v=4){kind=small}
| git -C shallow12 fetch --shallow-exclude one origin && | ||
| git -C shallow12 log --pretty=tformat:%s origin/main >actual && | ||
| test_write_lines three two >expected && | ||
| test_cmp expected actual |
There was a problem hiding this comment.
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
|
User |
| each remote branch history. | ||
|
|
||
| --shallow-since=<date>:: | ||
| Deepen or shorten the history of a shallow repository to |
There was a problem hiding this comment.
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.
PatrickThere was a problem hiding this comment.
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.
|
This patch series was integrated into seen via git@f5eeb8a. |
|
This patch series was integrated into seen via git@0d4ac18. |
|
This patch series was integrated into next via git@8c5d529. |
|
This patch series was integrated into seen via git@cabd763. |
|
This patch series was integrated into seen via git@dd36bcd. |
|
This patch series was integrated into seen via git@5920727. |
|
This patch series was integrated into seen via git@fdbd6bd. |
|
This patch series was integrated into seen via git@51ba601. |
|
This patch series was integrated into master via git@51ba601. |
|
This patch series was integrated into next via git@51ba601. |
|
Closed via 51ba601. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Here's a couple patches to fix up an error message and the documentation around the --shallow-exclude option.
cc: Patrick Steinhardt [email protected]