{"thread":{"id":"48342","subject":"[PATCH 1/2] http-fetch: make `-a` standard behaviour","startedAt":"2018-04-22T18:13:07Z","lastAt":"2018-04-22T18:13:13Z","messageCount":2,"participants":["Martin Ågren"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"345405","messageId":"367243f0c8215f702fd38152952f47121ab337b1.1524420451.git.martin.agren@gmail.com","threadId":"48342","inReplyTo":null,"subject":"[PATCH 1/2] http-fetch: make `-a` standard behaviour","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2018-04-22T18:12:49Z","receivedAt":"2018-04-22T18:13:07Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"This is a follow-up to a6c786fce8 (Mark http-fetch without -a as\ndeprecated, 2011-08-23). For more than six years, we have been warning\nwhen `-a` is not provided, and the documentation has been saying that\n`-a` will become the default.\n\nIt is a bit unclear what \"default\" means here. There is no such thing as\n`http-fetch --no-a`. But according to my searches, no-one has been\nasking on the mailing list how they should silence the warning and\nprepare for overriding the flipped default. So let's assume that\neverybody is happy with `-a`. They should be, since not using it may\nbreak the repo in such a way that Git itself is unable to fix it.\n\nAlways behave as if `-a` was given. Since `-a` implies `-c` (get commit\nobjects) and `-t` (get trees), all three options are now unnecessary.\nDocument all of these as historical artefacts that have no effect.\n\nLeave no-op code for handling these options in http-fetch.c. The\noptions-handling is currently rather loose. If someone tightens it, we\nwill not want these ignored options to accidentally turn into hard\nerrors.\n\nSince `-a` was the only safe and sane usage and we have been pushing\npeople towards it for a long time, refrain from warning when it is used\n\"unnecessarily\" now. Similarly, do not add anything scary-looking to the\nman-page about how it will be removed in the future. We can always do so\nlater. (It is not like we are in desperate need of freeing up\none-letter arguments.)\n\nSigned-off-by: Martin Ågren <martin.agren@gmail.com>\n---\n Documentation/git-http-fetch.txt | 13 +++++--------\n t/t5550-http-fetch-dumb.sh       | 11 +++++++++++\n http-fetch.c                     | 18 +++---------------\n 3 files changed, 19 insertions(+), 23 deletions(-)\n\ndiff --git a/Documentation/git-http-fetch.txt b/Documentation/git-http-fetch.txt\nindex 21a33d2c41..666b042679 100644\n--- a/Documentation/git-http-fetch.txt\n+++ b/Documentation/git-http-fetch.txt\n@@ -15,8 +15,9 @@ DESCRIPTION\n -----------\n Downloads a remote Git repository via HTTP.\n \n-*NOTE*: use of this command without -a is deprecated.  The -a\n-behaviour will become the default in a future release.\n+This command always gets all objects. Historically, there were three options\n+`-a`, `-c` and `-t` for choosing which objects to download. They are now\n+silently ignored.\n \n OPTIONS\n -------\n@@ -24,12 +25,8 @@ commit-id::\n         Either the hash or the filename under [URL]/refs/ to\n         pull.\n \n--c::\n-\tGet the commit objects.\n--t::\n-\tGet trees associated with the commit objects.\n--a::\n-\tGet all the objects.\n+-a, -c, -t::\n+\tThese options are ignored for historical reasons.\n -v::\n \tReport what is downloaded.\n \ndiff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh\nindex 8552184e74..6d7d88ccc9 100755\n--- a/t/t5550-http-fetch-dumb.sh\n+++ b/t/t5550-http-fetch-dumb.sh\n@@ -169,6 +169,17 @@ test_expect_success 'fetch changes via manual http-fetch' '\n \ttest_cmp file clone2/file\n '\n \n+test_expect_success 'manual http-fetch without -a works just as well' '\n+\tcp -R clone-tmpl clone3 &&\n+\n+\tHEAD=$(git rev-parse --verify HEAD) &&\n+\t(cd clone3 &&\n+\t git http-fetch -w heads/master-new $HEAD $(git config remote.origin.url) &&\n+\t git checkout master-new &&\n+\t test $HEAD = $(git rev-parse --verify HEAD)) &&\n+\ttest_cmp file clone3/file\n+'\n+\n test_expect_success 'http remote detects correct HEAD' '\n \tgit push public master:other &&\n \t(cd clone &&\ndiff --git a/http-fetch.c b/http-fetch.c\nindex 8af380050c..a1564f5a41 100644\n--- a/http-fetch.c\n+++ b/http-fetch.c\n@@ -17,21 +17,13 @@ int cmd_main(int argc, const char **argv)\n \tchar *url = NULL;\n \tint arg = 1;\n \tint rc = 0;\n-\tint get_tree = 0;\n-\tint get_history = 0;\n-\tint get_all = 0;\n \tint get_verbosely = 0;\n \tint get_recover = 0;\n \n \twhile (arg < argc && argv[arg][0] == '-') {\n \t\tif (argv[arg][1] == 't') {\n-\t\t\tget_tree = 1;\n \t\t} else if (argv[arg][1] == 'c') {\n-\t\t\tget_history = 1;\n \t\t} else if (argv[arg][1] == 'a') {\n-\t\t\tget_all = 1;\n-\t\t\tget_tree = 1;\n-\t\t\tget_history = 1;\n \t\t} else if (argv[arg][1] == 'v') {\n \t\t\tget_verbosely = 1;\n \t\t} else if (argv[arg][1] == 'w') {\n@@ -55,10 +47,6 @@ int cmd_main(int argc, const char **argv)\n \t\tcommits = 1;\n \t}\n \n-\tif (get_all == 0)\n-\t\twarning(\"http-fetch: use without -a is deprecated.\\n\"\n-\t\t\t\"In a future release, -a will become the default.\");\n-\n \tif (argv[arg])\n \t\tstr_end_url_with_slash(argv[arg], &url);\n \n@@ -68,9 +56,9 @@ int cmd_main(int argc, const char **argv)\n \n \thttp_init(NULL, url, 0);\n \twalker = get_http_walker(url);\n-\twalker->get_tree = get_tree;\n-\twalker->get_history = get_history;\n-\twalker->get_all = get_all;\n+\twalker->get_tree = 1;\n+\twalker->get_history = 1;\n+\twalker->get_all = 1;\n \twalker->get_verbosely = get_verbosely;\n \twalker->get_recover = get_recover;\n \n-- \n2.17.0\n\n"},{"id":"345406","messageId":"44e0ee4a6617244e5d45806e100565f8f46a86c5.1524420451.git.martin.agren@gmail.com","threadId":"48342","inReplyTo":"367243f0c8215f702fd38152952f47121ab337b1.1524420451.git.martin.agren@gmail.com","subject":"[PATCH 2/2] walker: drop fields of `struct walker` which are always 1","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2018-04-22T18:12:50Z","receivedAt":"2018-04-22T18:13:13Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"After the previous commit, both users of `struct walker` set `get_tree`,\n`get_history` and `get_all` to 1. Drop those fields and simplify the\nwalker implementation accordingly.\n\nLet's hope that any out-of-tree users will not mind this change. They\nshould notice that the compilation fails as they try to set these\nfields. (If they do not set them, note that `get_http_walker()` leaves\nthem undefined, so the behavior will have been undefined all the time.)\n\nSigned-off-by: Martin Ågren <martin.agren@gmail.com>\n---\n walker.h      |  3 ---\n http-fetch.c  |  3 ---\n remote-curl.c |  3 ---\n walker.c      | 19 ++++++++-----------\n 4 files changed, 8 insertions(+), 20 deletions(-)\n\ndiff --git a/walker.h b/walker.h\nindex a869013e85..6d8ae00e5b 100644\n--- a/walker.h\n+++ b/walker.h\n@@ -9,9 +9,6 @@ struct walker {\n \tvoid (*prefetch)(struct walker *, unsigned char *sha1);\n \tint (*fetch)(struct walker *, unsigned char *sha1);\n \tvoid (*cleanup)(struct walker *);\n-\tint get_tree;\n-\tint get_history;\n-\tint get_all;\n \tint get_verbosely;\n \tint get_recover;\n \ndiff --git a/http-fetch.c b/http-fetch.c\nindex a1564f5a41..7b855d3349 100644\n--- a/http-fetch.c\n+++ b/http-fetch.c\n@@ -56,9 +56,6 @@ int cmd_main(int argc, const char **argv)\n \n \thttp_init(NULL, url, 0);\n \twalker = get_http_walker(url);\n-\twalker->get_tree = 1;\n-\twalker->get_history = 1;\n-\twalker->get_all = 1;\n \twalker->get_verbosely = get_verbosely;\n \twalker->get_recover = get_recover;\n \ndiff --git a/remote-curl.c b/remote-curl.c\nindex a7c4c9b5ff..dd86a6c4fa 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -797,9 +797,6 @@ static int fetch_dumb(int nr_heads, struct ref **to_fetch)\n \t\ttargets[i] = xstrdup(oid_to_hex(&to_fetch[i]->old_oid));\n \n \twalker = get_http_walker(url.buf);\n-\twalker->get_all = 1;\n-\twalker->get_tree = 1;\n-\twalker->get_history = 1;\n \twalker->get_verbosely = options.verbosity >= 3;\n \twalker->get_recover = 0;\n \tret = walker_fetch(walker, nr_heads, targets, NULL, NULL);\ndiff --git a/walker.c b/walker.c\nindex dffb9c8e37..8eb7db7a52 100644\n--- a/walker.c\n+++ b/walker.c\n@@ -72,6 +72,8 @@ static struct commit_list *complete = NULL;\n \n static int process_commit(struct walker *walker, struct commit *commit)\n {\n+\tstruct commit_list *parents;\n+\n \tif (parse_commit(commit))\n \t\treturn -1;\n \n@@ -86,19 +88,14 @@ static int process_commit(struct walker *walker, struct commit *commit)\n \n \twalker_say(walker, \"walk %s\\n\", oid_to_hex(&commit->object.oid));\n \n-\tif (walker->get_tree) {\n-\t\tif (process(walker, &commit->tree->object))\n+\tif (process(walker, &commit->tree->object))\n+\t\treturn -1;\n+\n+\tfor (parents = commit->parents; parents; parents = parents->next) {\n+\t\tif (process(walker, &parents->item->object))\n \t\t\treturn -1;\n-\t\tif (!walker->get_all)\n-\t\t\twalker->get_tree = 0;\n-\t}\n-\tif (walker->get_history) {\n-\t\tstruct commit_list *parents = commit->parents;\n-\t\tfor (; parents; parents = parents->next) {\n-\t\t\tif (process(walker, &parents->item->object))\n-\t\t\t\treturn -1;\n-\t\t}\n \t}\n+\n \treturn 0;\n }\n \n-- \n2.17.0\n\n"}]}