| Message ID | 20241127091718.345541-1-bence@ferdinandy.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v1] fetch: add configuration for set_head behaviour | expand |
Bence Ferdinandy <bence@ferdinandy.com> writes: > Introduce a new setting, remote.$remote.followRemoteHEAD with four > options: > > - "never": do not ever do anything, not even create > - "create": the current behaviour, now the default behaviour > - "warn": print a message if remote and local HEAD is different > - "always": silently update HEAD on every change That seems to be plenty of choices to please many classes of users. Except for the one that I would want to use myself, which is "I understand their HEAD points at branch X right now; please warn when they flip their HEAD to a different branch, but until then please do nothing". That's somewhere between "never" and "warn". > @@ -1603,6 +1628,8 @@ static int set_head(const struct ref *remote_refs) > string_list_append(&heads, strip_refshead(ref->name)); > } > > + if (follow_remote_head < 0) > + goto cleanup; There is some "magical" value(s) that is/are negative; we will find out what they are later. > @@ -1614,6 +1641,7 @@ static int set_head(const struct ref *remote_refs) > if (!head_name) > goto cleanup; > is_bare = is_bare_repository(); > + create_only = follow_remote_head == 2 ? 0 : !is_bare; There is one more "magical" value that follow_remote_head can take. > @@ -1626,9 +1654,14 @@ static int set_head(const struct ref *remote_refs) > result = 1; > goto cleanup; > } > - if (refs_update_symref_extended(refs, b_head.buf, b_remote_head.buf, > - "fetch", NULL, !is_bare)) > + was_detached = refs_update_symref_extended(refs, b_head.buf, b_remote_head.buf, > + "fetch", &b_local_head, create_only); > + if (was_detached == -1) { > result = 1; > + goto cleanup; > + } > + if (follow_remote_head == 1 && verbosity >= 0) And there is one more. > diff --git a/remote.c b/remote.c > index 10104d11e3..5a768ddac2 100644 > --- a/remote.c > +++ b/remote.c > @@ -514,6 +514,15 @@ static int handle_config(const char *key, const char *value, > } else if (!strcmp(subkey, "serveroption")) { > return parse_transport_option(key, value, > &remote->server_options); > + } else if (!strcmp(subkey, "followremotehead")) { > + if (!strcmp(value, "never")) > + remote->follow_remote_head = -1; > + else if (!strcmp(value, "create")) > + remote->follow_remote_head = 0; > + else if (!strcmp(value, "warn")) > + remote->follow_remote_head = 1; > + else if (!strcmp(value, "always")) > + remote->follow_remote_head = 2; Use something like /* The setting for whether to update HEAD for the remote. */ enum follow_remote_head { FOLLOW_REMOTE_NEVER = -1, FOLLOW_REMOTE_CREATE = 0, FOLLOW_REMOTE_WARN = 1, FOLLOW_REMOTE_ALWAYS = 2, }; or something? I have no strong preference between "enum" and "#define" myself, but moderately strong preference for anything symbolic over magic numbers. > diff --git a/remote.h b/remote.h > index a7e5c4e07c..3ceadac820 100644 > --- a/remote.h > +++ b/remote.h > @@ -107,6 +107,15 @@ struct remote { > char *http_proxy_authmethod; > > struct string_list server_options; > + > + /* > + * The setting for whether to update HEAD for the remote. > + * -1 never update > + * 0 create only (default) > + * 1 warn on change > + * 2 always update > + */ > + int follow_remote_head; > }; Other than that, looking good from a cursory read. Thanks.
On Wed Nov 27, 2024 at 14:46, Junio C Hamano <gitster@pobox.com> wrote: > Bence Ferdinandy <bence@ferdinandy.com> writes: > >> Introduce a new setting, remote.$remote.followRemoteHEAD with four >> options: >> >> - "never": do not ever do anything, not even create >> - "create": the current behaviour, now the default behaviour >> - "warn": print a message if remote and local HEAD is different >> - "always": silently update HEAD on every change > > That seems to be plenty of choices to please many classes of users. > > Except for the one that I would want to use myself, which is "I > understand their HEAD points at branch X right now; please warn when > they flip their HEAD to a different branch, but until then please do > nothing". That's somewhere between "never" and "warn". Just to clarify, an example: the remote's HEAD is set to "master", and you have - git remote set-head origin next - git config set remote.origin.followRemoteHead "manual" - git config set remote.origin.followRemoteHeadManual "master" you manually set these explicitly. As long as the remote's HEAD is still "master" you do not get a warning when running fetch, but if it changes to something else (even "next"), you _do_ get a warning, that is not silenced until you set followRemoteHeadManual to "next". Would you expect `git remote set-head origin next` to set followRemoteHead to "manual" and to set the correct value for the followRemoteHeadManual to "master" if it actually changed the current refs/remote/origin/HEAD from master to next? Or is having this completely manual fine? I toyed with the thought of rolling the two settings together (an unrecognized string would mean the reference for which we must be silent), but then you couldn't have a remote/HEAD called "create" for example, so I think we need to store that separately. I'm also not quite happy with "followRemoteHeadManual" ... > >> @@ -1603,6 +1628,8 @@ static int set_head(const struct ref *remote_refs) >> string_list_append(&heads, strip_refshead(ref->name)); >> } >> >> + if (follow_remote_head < 0) >> + goto cleanup; > > There is some "magical" value(s) that is/are negative; we will find > out what they are later. > >> @@ -1614,6 +1641,7 @@ static int set_head(const struct ref *remote_refs) >> if (!head_name) >> goto cleanup; >> is_bare = is_bare_repository(); >> + create_only = follow_remote_head == 2 ? 0 : !is_bare; > > There is one more "magical" value that follow_remote_head can take. > >> @@ -1626,9 +1654,14 @@ static int set_head(const struct ref *remote_refs) >> result = 1; >> goto cleanup; >> } >> - if (refs_update_symref_extended(refs, b_head.buf, b_remote_head.buf, >> - "fetch", NULL, !is_bare)) >> + was_detached = refs_update_symref_extended(refs, b_head.buf, b_remote_head.buf, >> + "fetch", &b_local_head, create_only); >> + if (was_detached == -1) { >> result = 1; >> + goto cleanup; >> + } >> + if (follow_remote_head == 1 && verbosity >= 0) > > And there is one more. > >> diff --git a/remote.c b/remote.c >> index 10104d11e3..5a768ddac2 100644 >> --- a/remote.c >> +++ b/remote.c >> @@ -514,6 +514,15 @@ static int handle_config(const char *key, const char *value, >> } else if (!strcmp(subkey, "serveroption")) { >> return parse_transport_option(key, value, >> &remote->server_options); >> + } else if (!strcmp(subkey, "followremotehead")) { >> + if (!strcmp(value, "never")) >> + remote->follow_remote_head = -1; >> + else if (!strcmp(value, "create")) >> + remote->follow_remote_head = 0; >> + else if (!strcmp(value, "warn")) >> + remote->follow_remote_head = 1; >> + else if (!strcmp(value, "always")) >> + remote->follow_remote_head = 2; > > Use something like > > /* The setting for whether to update HEAD for the remote. */ > enum follow_remote_head { > FOLLOW_REMOTE_NEVER = -1, > FOLLOW_REMOTE_CREATE = 0, > FOLLOW_REMOTE_WARN = 1, > FOLLOW_REMOTE_ALWAYS = 2, > }; > > or something? I have no strong preference between "enum" and > "#define" myself, but moderately strong preference for anything > symbolic over magic numbers. Ah, my mistake sorry. Magic number bad. Best, Bence
"Bence Ferdinandy" <bence@ferdinandy.com> writes: > Just to clarify, an example: the remote's HEAD is set to "master", and you have > - git remote set-head origin next > - git config set remote.origin.followRemoteHead "manual" > - git config set remote.origin.followRemoteHeadManual "master" > > you manually set these explicitly. As long as the remote's HEAD is still > "master" you do not get a warning when running fetch, but if it changes to > something else (even "next"), you _do_ get a warning, that is not silenced > until you set followRemoteHeadManual to "next". Yup. > Would you expect `git remote set-head origin next` to set followRemoteHead to > "manual" and to set the correct value for the followRemoteHeadManual to > "master" if it actually changed the current refs/remote/origin/HEAD from master > to next? What I found missing is: "I know this is the value I expect them to have, because it was the value I last observed there. Please let me know when they changed their mind; I want to reconsider my position when it happens, and your warning me would help me to do so." My running "remote set-head" to manually change which of their branches I am interested in does not tell Git anything about what branch I expect them to be pointing at with HEAD. It may be the action after such "reconsideration" of my position, but there is 0 bit of information on what I expect their HEAD to be pointing at. > Or is having this completely manual fine? If it can be automated, that would be nicer, but I do not think a manual "remote set-head origin next" gives any information to help automating it. > I toyed with the thought of rolling the two settings together (an unrecognized > string would mean the reference for which we must be silent), but then you > couldn't have a remote/HEAD called "create" for example, so I think we need to > store that separately. I'm also not quite happy with "followRemoteHeadManual" > ... Yeah, I was thinking a value like "warn-if-not-pointing-at-$branch" where $branch part would be a token like 'bc/set-head-symref', 'master', etc. I do not think any of the above should block this topic. It can be added later without disrupting all other modes implemented there. Thanks.
On Thu Nov 28, 2024 at 01:12, Junio C Hamano <gitster@pobox.com> wrote: > "Bence Ferdinandy" <bence@ferdinandy.com> writes: > >> Just to clarify, an example: the remote's HEAD is set to "master", and you have >> - git remote set-head origin next >> - git config set remote.origin.followRemoteHead "manual" >> - git config set remote.origin.followRemoteHeadManual "master" >> >> you manually set these explicitly. As long as the remote's HEAD is still >> "master" you do not get a warning when running fetch, but if it changes to >> something else (even "next"), you _do_ get a warning, that is not silenced >> until you set followRemoteHeadManual to "next". > > Yup. > >> Would you expect `git remote set-head origin next` to set followRemoteHead to >> "manual" and to set the correct value for the followRemoteHeadManual to >> "master" if it actually changed the current refs/remote/origin/HEAD from master >> to next? > > What I found missing is: > > "I know this is the value I expect them to have, because it was > the value I last observed there. Please let me know when they > changed their mind; I want to reconsider my position when it > happens, and your warning me would help me to do so." > > My running "remote set-head" to manually change which of their > branches I am interested in does not tell Git anything about what > branch I expect them to be pointing at with HEAD. It may be the > action after such "reconsideration" of my position, but there is 0 > bit of information on what I expect their HEAD to be pointing at. Hmm. After a bit more thought: running `remote set-head` doesn't make much sense if you have "always" and makes a bit of sense if you have "warn". So maybe one thing set-head could do is _if_ you have always it could drop you to "warn" and "warn" could also include a line not just about following with "remote set-head" but something like "You can either follow with set-head, or you can turn this off but still get a warning if origin changes by setting ...". > >> Or is having this completely manual fine? > > If it can be automated, that would be nicer, but I do not think a > manual "remote set-head origin next" gives any information to help > automating it. > >> I toyed with the thought of rolling the two settings together (an unrecognized >> string would mean the reference for which we must be silent), but then you >> couldn't have a remote/HEAD called "create" for example, so I think we need to >> store that separately. I'm also not quite happy with "followRemoteHeadManual" >> ... > > Yeah, I was thinking a value like "warn-if-not-pointing-at-$branch" > where $branch part would be a token like 'bc/set-head-symref', > 'master', etc. Good idea. Probably shorter though like "warn-if-not-$branch". > > I do not think any of the above should block this topic. It can be > added later without disrupting all other modes implemented there. Ack, I'll send a respin with just enum fixed for now. Thanks, Bence
"Bence Ferdinandy" <bence@ferdinandy.com> writes: > Hmm. After a bit more thought: running `remote set-head` doesn't make much > sense if you have "always" and makes a bit of sense if you have "warn". Yeah, after "warn" tells the user what branch the remote points at, the user may decide to match. Or the user may decide to flip the remote-tracking HEAD to something unrelated. In the former case, the next and subsequent fetch will not warn, until the remote flips the HEAD again. In the latter case, the next and subsequent fetch will warn, until they happen to change their HEAD again and their choice happens to match ours. > So > maybe one thing set-head could do is _if_ you have always it could drop you to > "warn" and "warn" could also include a line not just about following with > "remote set-head" but something like "You can either follow with set-head, or > you can turn this off but still get a warning if origin changes by setting > ...". Hmph, if you've been happy with "always", if we notice that the remote flipped its HEAD, we switch to "warn" and the next and subsequent fetch will not warn, until they flip again, and then we will keep warning. If you do not want to follow (but still monitor origin), the above would not be sufficient, unless "by setting..." part gives a new choice via your .followRemoteHEADManual setting. >> Yeah, I was thinking a value like "warn-if-not-pointing-at-$branch" >> where $branch part would be a token like 'bc/set-head-symref', >> 'master', etc. > > Good idea. Probably shorter though like "warn-if-not-$branch". Sorry, I wasn't offering an alternative I thought was better. It was "we could do this, if we really want to avoid two variables". But thinking about it further, not having to worry about two variables may be a huge benefit. If you set .followRemoteHEAD to "manual" and .followRemoteHEADManual to some value, and then changed .followRemoteHEAD to something else, you may be very likely to forget unsetting .followRemoteHEADManual that is no longer in effect. >> I do not think any of the above should block this topic. It can be >> added later without disrupting all other modes implemented there. > > Ack, I'll send a respin with just enum fixed for now. Thanks.
diff --git a/Documentation/config/remote.txt b/Documentation/config/remote.txt index 6d8b7d6c63..024f92befc 100644 --- a/Documentation/config/remote.txt +++ b/Documentation/config/remote.txt @@ -101,6 +101,17 @@ remote.<name>.serverOption:: The default set of server options used when fetching from this remote. These server options can be overridden by the `--server-option=` command line arguments. + +remote.<name>.followRemoteHEAD:: + How linkgit:git-fetch[1] should handle updates to `remotes/<name>/HEAD`. + The default value is "create", which will create `remotes/<name>/HEAD` + if it exists on the remote, but not locally, but will not touch an + already existing local reference. Setting to "warn" will print + a message if the remote has a different value, than the local one and + in case there is no local reference, it behaves like "create". Setting + to "always" will silently update it to the value on the remote. + Finally, setting it to "never" will never change or create the local + reference. + This is a multi-valued variable, and an empty value can be used in a higher priority configuration file (e.g. `.git/config` in a repository) to clear diff --git a/builtin/fetch.c b/builtin/fetch.c index 2f416cf867..b619bddd7a 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -1579,10 +1579,35 @@ static const char *strip_refshead(const char *name){ return name; } -static int set_head(const struct ref *remote_refs) +static void report_set_head(const char *remote, const char *head_name, + struct strbuf *buf_prev, int updateres) { + struct strbuf buf_prefix = STRBUF_INIT; + const char *prev_head = NULL; + + strbuf_addf(&buf_prefix, "refs/remotes/%s/", remote); + skip_prefix(buf_prev->buf, buf_prefix.buf, &prev_head); + + if (prev_head && strcmp(prev_head, head_name)) { + printf("'HEAD' at '%s' is '%s', but we have '%s' locally.\n", + remote, head_name, prev_head); + printf("Run 'git remote set-head %s %s' to follow the change.\n", + remote, head_name); + } + else if (updateres && buf_prev->len) { + printf("'HEAD' at '%s' is '%s', " + "but we have a detached HEAD pointing to '%s' locally.\n", + remote, head_name, buf_prev->buf); + printf("Run 'git remote set-head %s %s' to follow the change.\n", + remote, head_name); + } + strbuf_release(&buf_prefix); +} + +static int set_head(const struct ref *remote_refs, int follow_remote_head) { - int result = 0, is_bare; - struct strbuf b_head = STRBUF_INIT, b_remote_head = STRBUF_INIT; + int result = 0, create_only, is_bare, was_detached; + struct strbuf b_head = STRBUF_INIT, b_remote_head = STRBUF_INIT, + b_local_head = STRBUF_INIT; const char *remote = gtransport->remote->name; char *head_name = NULL; struct ref *ref, *matches; @@ -1603,6 +1628,8 @@ static int set_head(const struct ref *remote_refs) string_list_append(&heads, strip_refshead(ref->name)); } + if (follow_remote_head < 0) + goto cleanup; if (!heads.nr) result = 1; @@ -1614,6 +1641,7 @@ static int set_head(const struct ref *remote_refs) if (!head_name) goto cleanup; is_bare = is_bare_repository(); + create_only = follow_remote_head == 2 ? 0 : !is_bare; if (is_bare) { strbuf_addstr(&b_head, "HEAD"); strbuf_addf(&b_remote_head, "refs/heads/%s", head_name); @@ -1626,9 +1654,14 @@ static int set_head(const struct ref *remote_refs) result = 1; goto cleanup; } - if (refs_update_symref_extended(refs, b_head.buf, b_remote_head.buf, - "fetch", NULL, !is_bare)) + was_detached = refs_update_symref_extended(refs, b_head.buf, b_remote_head.buf, + "fetch", &b_local_head, create_only); + if (was_detached == -1) { result = 1; + goto cleanup; + } + if (follow_remote_head == 1 && verbosity >= 0) + report_set_head(remote, head_name, &b_local_head, was_detached); cleanup: free(head_name); @@ -1636,6 +1669,7 @@ static int set_head(const struct ref *remote_refs) free_refs(matches); string_list_clear(&heads, 0); strbuf_release(&b_head); + strbuf_release(&b_local_head); strbuf_release(&b_remote_head); return result; } @@ -1855,7 +1889,7 @@ static int do_fetch(struct transport *transport, "you need to specify exactly one branch with the --set-upstream option")); } } - if (set_head(remote_refs)) + if (set_head(remote_refs, transport->remote->follow_remote_head)) ; /* * Way too many cases where this can go wrong diff --git a/remote.c b/remote.c index 10104d11e3..5a768ddac2 100644 --- a/remote.c +++ b/remote.c @@ -514,6 +514,15 @@ static int handle_config(const char *key, const char *value, } else if (!strcmp(subkey, "serveroption")) { return parse_transport_option(key, value, &remote->server_options); + } else if (!strcmp(subkey, "followremotehead")) { + if (!strcmp(value, "never")) + remote->follow_remote_head = -1; + else if (!strcmp(value, "create")) + remote->follow_remote_head = 0; + else if (!strcmp(value, "warn")) + remote->follow_remote_head = 1; + else if (!strcmp(value, "always")) + remote->follow_remote_head = 2; } return 0; } diff --git a/remote.h b/remote.h index a7e5c4e07c..3ceadac820 100644 --- a/remote.h +++ b/remote.h @@ -107,6 +107,15 @@ struct remote { char *http_proxy_authmethod; struct string_list server_options; + + /* + * The setting for whether to update HEAD for the remote. + * -1 never update + * 0 create only (default) + * 1 warn on change + * 2 always update + */ + int follow_remote_head; }; /** diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index 87698341f5..2467027d34 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -99,6 +99,108 @@ test_expect_success "fetch test remote HEAD change" ' branch=$(git rev-parse refs/remotes/origin/other) && test "z$head" = "z$branch"' +test_expect_success "fetch test followRemoteHEAD never" ' + test_when_finished "git config unset remote.origin.followRemoteHEAD" && + ( + cd "$D" && + cd two && + git update-ref --no-deref -d refs/remotes/origin/HEAD && + git config set remote.origin.followRemoteHEAD "never" && + git fetch && + test_must_fail git rev-parse --verify refs/remotes/origin/HEAD + ) +' + +test_expect_success "fetch test followRemoteHEAD warn no change" ' + test_when_finished "git config unset remote.origin.followRemoteHEAD" && + ( + cd "$D" && + cd two && + git rev-parse --verify refs/remotes/origin/other && + git remote set-head origin other && + git rev-parse --verify refs/remotes/origin/HEAD && + git rev-parse --verify refs/remotes/origin/main && + git config set remote.origin.followRemoteHEAD "warn" && + git fetch >output && + echo "${SQ}HEAD${SQ} at ${SQ}origin${SQ} is ${SQ}main${SQ}," \ + "but we have ${SQ}other${SQ} locally." >expect && + echo "Run ${SQ}git remote set-head origin main${SQ} to follow the change." >>expect && + test_cmp expect output && + head=$(git rev-parse refs/remotes/origin/HEAD) && + branch=$(git rev-parse refs/remotes/origin/other) && + test "z$head" = "z$branch" + ) +' + +test_expect_success "fetch test followRemoteHEAD warn create" ' + test_when_finished "git config unset remote.origin.followRemoteHEAD" && + ( + cd "$D" && + cd two && + git update-ref --no-deref -d refs/remotes/origin/HEAD && + git config set remote.origin.followRemoteHEAD "warn" && + git rev-parse --verify refs/remotes/origin/main && + output=$(git fetch) && + test "z" = "z$output" && + head=$(git rev-parse refs/remotes/origin/HEAD) && + branch=$(git rev-parse refs/remotes/origin/main) && + test "z$head" = "z$branch" + ) +' + +test_expect_success "fetch test followRemoteHEAD warn detached" ' + test_when_finished "git config unset remote.origin.followRemoteHEAD" && + ( + cd "$D" && + cd two && + git update-ref --no-deref -d refs/remotes/origin/HEAD && + git update-ref refs/remotes/origin/HEAD HEAD && + HEAD=$(git log --pretty="%H") && + git config set remote.origin.followRemoteHEAD "warn" && + git fetch >output && + echo "${SQ}HEAD${SQ} at ${SQ}origin${SQ} is ${SQ}main${SQ}," \ + "but we have a detached HEAD pointing to" \ + "${SQ}${HEAD}${SQ} locally." >expect && + echo "Run ${SQ}git remote set-head origin main${SQ} to follow the change." >>expect && + test_cmp expect output + ) +' + +test_expect_success "fetch test followRemoteHEAD warn quiet" ' + test_when_finished "git config unset remote.origin.followRemoteHEAD" && + ( + cd "$D" && + cd two && + git rev-parse --verify refs/remotes/origin/other && + git remote set-head origin other && + git rev-parse --verify refs/remotes/origin/HEAD && + git rev-parse --verify refs/remotes/origin/main && + git config set remote.origin.followRemoteHEAD "warn" && + output=$(git fetch --quiet) && + test "z" = "z$output" && + head=$(git rev-parse refs/remotes/origin/HEAD) && + branch=$(git rev-parse refs/remotes/origin/other) && + test "z$head" = "z$branch" + ) +' + +test_expect_success "fetch test followRemoteHEAD always" ' + test_when_finished "git config unset remote.origin.followRemoteHEAD" && + ( + cd "$D" && + cd two && + git rev-parse --verify refs/remotes/origin/other && + git remote set-head origin other && + git rev-parse --verify refs/remotes/origin/HEAD && + git rev-parse --verify refs/remotes/origin/main && + git config set remote.origin.followRemoteHEAD "always" && + git fetch && + head=$(git rev-parse refs/remotes/origin/HEAD) && + branch=$(git rev-parse refs/remotes/origin/main) && + test "z$head" = "z$branch" + ) +' + test_expect_success 'fetch --prune on its own works as expected' ' cd "$D" && git clone . prune &&
In the current implementatio, if refs/remotes/$remote/HEAD does not exist, running fetch will create it, but if it does exist it will not do anything, which is a somewhat safe and minimal approach. Unfortunately, for users who wish to NOT have refs/remotes/$remote/HEAD set for any reason (e.g. so that `git rev-parse origin` doesn't accidentally point them somewhere they do not want to), there is no way to remove this behaviour. On the other side of the spectrum, users may want fetch to automatically update HEAD or at least give them a warning if something changed on the remote. Introduce a new setting, remote.$remote.followRemoteHEAD with four options: - "never": do not ever do anything, not even create - "create": the current behaviour, now the default behaviour - "warn": print a message if remote and local HEAD is different - "always": silently update HEAD on every change Signed-off-by: Bence Ferdinandy <bence@ferdinandy.com> --- Notes: This patch is built off v15 of the set-head/fetch updates series: https://lore.kernel.org/git/D5WEJJBMSO1K.2TPXDI1K08SHT@ferdinandy.com/T/#m13a0c3e8919872188ef07fd9fd984c78e8aa35ba The patch is sent separately, because the rest of the series building up to here seems ready, while this patch will likely trigger some discussion. Documentation/config/remote.txt | 11 ++++ builtin/fetch.c | 46 ++++++++++++-- remote.c | 9 +++ remote.h | 9 +++ t/t5510-fetch.sh | 102 ++++++++++++++++++++++++++++++++ 5 files changed, 171 insertions(+), 6 deletions(-)