{"thread":{"id":"30752","subject":"[PATCH_v1] add 'git credential' plumbing command","startedAt":"2012-06-09T18:45:02Z","lastAt":"2012-06-11T18:18:40Z","messageCount":15,"participants":["javier.roucher-iglesias@ensimag.imag.fr","konglu@minatec.inpg.fr","Junio C Hamano","Jeff King","Matthieu Moy","roucherj","Jonathan Nieder"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"193210","messageId":"1339267502-13803-1-git-send-email-Javier.Roucher-Iglesias@ensimag.imag.fr","threadId":"30752","inReplyTo":null,"subject":"[PATCH_v1] add 'git credential' plumbing command","fromName":"","fromEmail":"javier.roucher-iglesias@ensimag.imag.fr","sentAt":"2012-06-09T18:45:02Z","receivedAt":"2012-06-09T18:45:02Z","isPatch":false,"sender":{"key":"javier.roucher-iglesias@ensimag.imag.fr","avatar":null},"body":"From: Javier Roucher \n\n\nThe credential API is in C, and not available to scripting languages.\nExpose the functionalities of the API by wrapping them into a new\nplumbing command \"git credentials\".\n\nSigned-off-by: Pavel Volek \nSigned-off-by: NGUYEN Kim Thuat \nSigned-off-by: ROUCHER IGLESIAS Javier \nSigned-off-by: Matthieu Moy \n\n---\n .gitignore | 1 +\n Documentation/git-credential.txt | 74 ++++++++++++++++++++++++++++++++++++++++\n Makefile | 1 +\n builtin.h | 1 +\n builtin/credential.c | 40 ++++++++++++++++++++++\n git.c | 1 +\n 6 files changed, 118 insertions(+)\n create mode 100644 Documentation/git-credential.txt\n create mode 100644 builtin/credential.c\n\ndiff --git a/.gitignore b/.gitignore\nindex bf66648..7d1d86e 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -31,6 +31,7 @@\n /git-commit-tree\n /git-config\n /git-count-objects\n+/git-credential\n /git-credential-cache\n /git-credential-cache--daemon\n /git-credential-store\ndiff --git a/Documentation/git-credential.txt b/Documentation/git-credential.txt\nnew file mode 100644\nindex 0000000..ead23b5\n--- /dev/null\n+++ b/Documentation/git-credential.txt\n@@ -0,0 +1,74 @@\n+git-credential(7)\n+=================\n+\n+NAME\n+----\n+git-credential - Providing and strore user credentials to git\n+\n+SYNOPSIS\n+--------\n+------------------\n+git credential \n+\n+------------------\n+\n+DESCRIPTION\n+-----------\n+\n+Git-credential permits to the user of the script to save:\n+username, password, host, path and protocol. When the user of script\n+invoke git-credential, the script can ask for a password, using the command\n+'git credential fill'.\n+Taking data from the standard input, the program treats each line as a\n+separate data item, and the end of series of data item is signalled by a \n+blank line.\n+\n+\t\tusername=admin\\n \n+\t\tprotocol=[http|https]\\n\n+\t\thost=localhost\\n\n+\t\tpath=/dir\\n\\n\n+\n+-If git-credential system have the password already stored\n+git-credential will answer with by STDOUT:\n+\t\n+\t\tusername=admin\n+\t\tpassword=*****\n+\n+-If it is not stored, the user will be prompt for a password:\n+\t\t\n+\t\t> Password for '[http|https]admin@localhost':\n+\n+\n+Then if the password is correct, (note: is not git credential\n+how decides if password is correct or not. Is the external system\n+that have to authenticate the user) it can be stored using command \n+'git crendential approve' by providing the structure, by STDIN.\n+\n+\t\tusername=admin\n+\t\tpassword=*****\n+\t\tprotocol=[http|https]\n+\t\thost=localhost\n+\t\tpath=/dir\n+\n+If the password is refused, it can be deleted using command\n+'git credential reject' by providing the same structure.\n+\n+\n+REQUESTING CREDENTIALS\n+----------------------\n+\n+1. The 'git credential fill' makes the structure,\n+with this structure it will be able to save your\n+credentials, and if the credential is allready stored,\n+it will fill the password.\n+\n+\t\tusername=foo\n+\t\tpassword=****\n+\t\tprotocol=[http|https]\n+\t\tlocalhost=url\n+\t\tpath=/direction\n+\n+2. Then 'git credential approve' to store them.\n+\n+3. Otherwise, if the credential is not correct you can do\n+ 'git credential reject' to delete the credential.\ndiff --git a/Makefile b/Makefile\nindex 4592f1f..3f53da8 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -827,6 +827,7 @@ BUILTIN_OBJS += builtin/commit-tree.o\n BUILTIN_OBJS += builtin/commit.o\n BUILTIN_OBJS += builtin/config.o\n BUILTIN_OBJS += builtin/count-objects.o\n+BUILTIN_OBJS += builtin/credential.o\n BUILTIN_OBJS += builtin/describe.o\n BUILTIN_OBJS += builtin/diff-files.o\n BUILTIN_OBJS += builtin/diff-index.o\ndiff --git a/builtin.h b/builtin.h\nindex 338f540..48feddc 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -66,6 +66,7 @@ extern int cmd_commit(int argc, const char **argv, const char *prefix);\n extern int cmd_commit_tree(int argc, const char **argv, const char *prefix);\n extern int cmd_config(int argc, const char **argv, const char *prefix);\n extern int cmd_count_objects(int argc, const char **argv, const char *prefix);\n+extern int cmd_credential(int argc, const char **argv, const char *prefix);\n extern int cmd_describe(int argc, const char **argv, const char *prefix);\n extern int cmd_diff_files(int argc, const char **argv, const char *prefix);\n extern int cmd_diff_index(int argc, const char **argv, const char *prefix);\ndiff --git a/builtin/credential.c b/builtin/credential.c\nnew file mode 100644\nindex 0000000..9f00885\n--- /dev/null\n+++ b/builtin/credential.c\n@@ -0,0 +1,40 @@\n+#include \n+#include \"cache.h\"\n+#include \"credential.h\"\n+#include \"string-list.h\"\n+\n+static const char usage_msg[] =\n+\"credential \";\n+\n+void cmd_credential (int argc, char **argv, const char *prefix){\n+\tconst char *op;\n+\tstruct credential c = CREDENTIAL_INIT;\n+\tint i;\n+\n+\top = argv[1];\n+\tif (!op)\n+\t\tusage(usage_msg);\n+\n+\tfor (i = 2; i < argc; i++)\n+\t\tstring_list_append(&c.helpers, argv[i]);\n+\n+\tif (credential_read(&c, stdin) < 0)\n+\t\tdie(\"unable to read credential from stdin\");\n+\n+\tif (!strcmp(op, \"fill\")) {\n+\t\tcredential_fill(&c);\n+\t\tif (c.username)\n+\t\t\tprintf(\"username=%s\\n\", c.username);\n+\t\tif (c.password)\n+\t\t\tprintf(\"password=%s\\n\", c.password);\n+\t}\n+\telse if (!strcmp(op, \"approve\")) {\n+\t\tcredential_approve(&c);\n+\t}\n+\telse if (!strcmp(op, \"reject\")) {\n+\t\tcredential_reject(&c);\n+\t}\n+\telse\n+\t\tusage(usage_msg);\n+}\n+\ndiff --git a/git.c b/git.c\nindex d232de9..7cbd7d8 100644\n--- a/git.c\n+++ b/git.c\n@@ -353,6 +353,7 @@ static void handle_internal_command(int argc, const char **argv)\n \t\t{ \"commit-tree\", cmd_commit_tree, RUN_SETUP },\n \t\t{ \"config\", cmd_config, RUN_SETUP_GENTLY },\n \t\t{ \"count-objects\", cmd_count_objects, RUN_SETUP },\n+\t\t{ \"credential\", cmd_count_objects, RUN_SETUP },\n \t\t{ \"describe\", cmd_describe, RUN_SETUP },\n \t\t{ \"diff\", cmd_diff },\n \t\t{ \"diff-files\", cmd_diff_files, RUN_SETUP | NEED_WORK_TREE },\n-- \n1.7.11.rc2.9.ge2c5c96.dirty\n"},{"id":"193213","messageId":"20120609215236.Horde.J-h4cnwdC4BP06mEUeqxRlA@webmail.minatec.grenoble-inp.fr","threadId":"30752","inReplyTo":"1339267502-13803-1-git-send-email-Javier.Roucher-Iglesias@ensimag.imag.fr","subject":"Re: [PATCH_v1] add 'git credential' plumbing command","fromName":"","fromEmail":"konglu@minatec.inpg.fr","sentAt":"2012-06-09T19:52:36Z","receivedAt":"2012-06-09T19:52:36Z","isPatch":false,"sender":{"key":"konglu@minatec.inpg.fr","avatar":null},"body":"\nJavier.Roucher-Iglesias@ensimag.imag.fr a écrit :\n\n> +git-credential - Providing and strore user credentials to git\n\ns/Providing/Provides/ & s/strore/store\n\n> +-If git-credential system have the password already stored\n> +git-credential will answer with by STDOUT:\n\ns/have/has/\n\n> +Then if the password is correct, (note: is not git credential\n> +how decides if password is correct or not. Is the external system\n> +that have to authenticate the user) it can be stored using command\n> +'git crendential approve' by providing the structure, by STDIN.\n\nWouldn't the note be \"it's not git credential that decides if the password is\ncorrect or not. That part is done by the external system\" ?\n\n> +1. The 'git credential fill' makes the structure,\n> +with this structure it will be able to save your\n> +credentials, and if the credential is allready stored,\n> +it will fill the password.\n\ns/allready/already/\n\n> +void cmd_credential (int argc, char **argv, const char *prefix){\n> +\tconst char *op;\n> +\tstruct credential c = CREDENTIAL_INIT;\n> +\tint i;\n> +\n> +\top = argv[1];\n> +\tif (!op)\n> +\t\tusage(usage_msg);\n> +\n> +\tfor (i = 2; i < argc; i++)\n> +\t\tstring_list_append(&c.helpers, argv[i]);\n> +\n> +\tif (credential_read(&c, stdin) < 0)\n> +\t\tdie(\"unable to read credential from stdin\");\n> +\n> +\tif (!strcmp(op, \"fill\")) {\n> +\t\tcredential_fill(&c);\n> +\t\tif (c.username)\n> +\t\t\tprintf(\"username=%s\\n\", c.username);\n> +\t\tif (c.password)\n> +\t\t\tprintf(\"password=%s\\n\", c.password);\n> +\t}\n> +\telse if (!strcmp(op, \"approve\")) {\n> +\t\tcredential_approve(&c);\n> +\t}\n> +\telse if (!strcmp(op, \"reject\")) {\n> +\t\tcredential_reject(&c);\n> +\t}\n> +\telse\n> +\t\tusage(usage_msg);\n\nBraces for the last \"else\" part. In general, the structure should be\n\n if (...) {\n /*code*/\n } else if (...) {\n /*code*/\n } else {\n /*code*/\n }\n\nIf juste one block needs brances, all the other \"else if\"/\"else\" part\nneed it too.\n\nBTW, please be aware of the white spaces (here mostly in the doc) :).\n\nLucien Kong.\n"},{"id":"193227","messageId":"7vzk8baca0.fsf@alter.siamese.dyndns.org","threadId":"30752","inReplyTo":"1339267502-13803-1-git-send-email-Javier.Roucher-Iglesias@ensimag.imag.fr","subject":"Re: [PATCH_v1] add 'git credential' plumbing command","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-10T06:53:11Z","receivedAt":"2012-06-10T06:53:11Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Javier.Roucher-Iglesias@ensimag.imag.fr writes:\n\n> +Git-credential permits to the user of the script to save:\n> +username, password, host, path and protocol.\n\nThe above sounds like saying \"A filesystem allows you to save\npathname and contents\". While it may not be _wrong_ per-se, usually\nyou would think of a filesystem as something you store contents in;\npathname is primarily used as a key to find the contents (i.e. you\ndo not store it in the filesystem).\n\nIsn't the credential mechanism for storing password for tuple (i.e. the four-tuple is used as a\nlook-up key)?\n\n> diff --git a/builtin/credential.c b/builtin/credential.c\n> new file mode 100644\n> index 0000000..9f00885\n> --- /dev/null\n> +++ b/builtin/credential.c\n> @@ -0,0 +1,40 @@\n> +#include \n> +#include \"cache.h\"\n> +#include \"credential.h\"\n> +#include \"string-list.h\"\n> +\n> +static const char usage_msg[] =\n> +\"credential \";\n> +\n> +void cmd_credential (int argc, char **argv, const char *prefix){\n\nStyle:\n\n void cmd_credential(int argc, char **argv, const char *prefix)\n {\n\n> diff --git a/git.c b/git.c\n> index d232de9..7cbd7d8 100644\n> --- a/git.c\n> +++ b/git.c\n> @@ -353,6 +353,7 @@ static void handle_internal_command(int argc, const char **argv)\n> \t\t{ \"commit-tree\", cmd_commit_tree, RUN_SETUP },\n> \t\t{ \"config\", cmd_config, RUN_SETUP_GENTLY },\n> \t\t{ \"count-objects\", cmd_count_objects, RUN_SETUP },\n> +\t\t{ \"credential\", cmd_count_objects, RUN_SETUP },\n\nDoes \"git credential\" need to have a git repository (i.e. run in a\ngit repository or in a working tree that is controlled by one)? A\nscripted Porcelain you would write using \"git credential\" may want\nto implement something like \"git clone\" or \"git ls-remote\" where you\ndo not have to be in an existing repository.\n"},{"id":"193249","messageId":"20120610115619.GA6453@sigill.intra.peff.net","threadId":"30752","inReplyTo":"1339267502-13803-1-git-send-email-Javier.Roucher-Iglesias@ensimag.imag.fr","subject":"Re: [PATCH_v1] add 'git credential' plumbing command","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-10T11:56:19Z","receivedAt":"2012-06-10T11:56:19Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jun 09, 2012 at 08:45:02PM +0200, Javier.Roucher-Iglesias@ensimag.imag.fr wrote:\n\n> +DESCRIPTION\n> +-----------\n> +\n> +Git-credential permits to the user of the script to save:\n> +username, password, host, path and protocol. When the user of script\n> +invoke git-credential, the script can ask for a password, using the command\n> +'git credential fill'.\n\nI wonder if this description starts off a bit too literal, and should\nstart with the big picture. Tell people what it is generally, how it\nfits into git, and in what circumstances they would want to run it.\n\nLike:\n\n Git has an internal interface for storing and retrieving credentials\n from system-specific helpers, as well as prompting the user for\n usernames and passwords. The git-credential command exposes this\n interface to scripts which may want to retrieve, store, or prompt for\n credentials in the same manner as git. The design of this scriptable\n interface models the internal C API; see\n link:technical/api-credentials.txt[the git credential API] for more\n background on the concepts.\n\n> +Taking data from the standard input, the program treats each line as a\n> +separate data item, and the end of series of data item is signalled by a \n> +blank line.\n> +\n> +\t\tusername=admin\\n \n> +\t\tprotocol=[http|https]\\n\n> +\t\thost=localhost\\n\n> +\t\tpath=/dir\\n\\n\n\nIt's nice to have an example like this, but there's much detail missing\nin how the format is specified. However, this format is already\ndocumented in the \"helpers\" section of api-credentials.txt, so it\nprobably makes sense to refer to that document.\n\n> +-If git-credential system have the password already stored\n> +git-credential will answer with by STDOUT:\n> +\t\n> +\t\tusername=admin\n> +\t\tpassword=*****\n> +\n> +-If it is not stored, the user will be prompt for a password:\n> +\t\t\n> +\t\t> Password for '[http|https]admin@localhost':\n\nI'd rather see this broken up into a reference specification and\nseparate examples. Then the reference part can be very specific about\nwhat happens and the behavior in each case. For example:\n\n git-credential takes an \"action\" option on the command-line (one of\n \"fill\", \"approve\", or \"reject\") and reads a credential description\n on stdin. The format of the description is the same as that given to\n credential helpers; see link:technical/api-credential.txt[the git\n credential API] for a specification.\n\n If the action is \"fill\", git-credential will attempt to add \"username\"\n and \"password\" fields to the description by reading config files, by\n contacting any configured credential helpers, or by prompting the\n user. The username and password fields of the credential description\n are then printed to stdout.\n\n If the action is \"approve\", git-credential will send the description\n to any configured credential helpers, which may store the credential\n for later use. \n\n If the action is \"reject\", git-credential will send the description to\n any configured credential helpers, which may erase any stored\n credential matching the description.\n\nAnd now that we've introduced the actions and said what they've done, we\ncan go on to describe the expected workflow, which is a good place to\nput examples:\n\n An application using git-credential will typically follow this\n workflow:\n\n 1. Generate a credential description based on the context.\n\n For example, if we want a password for\n `https://example.com/foo.git`, we might generate the following\n credential description:\n\n protocol=https\n host=example.com\n path=foo.git\n\n 2. Ask git-credential to give us a username and password for this\n description. This is done by running `git credential fill`,\n feeding the description from step (1) to its stdin. The username\n and password will be produced on stdout, like:\n\n username=bob\n password=secr3t\n\n 3. Try to use the credential (e.g., by accessing the URL with the\n username and password from step (2)).\n\n 4. Report on the success or failure of the password. If the\n credential allowed the operation to complete successfully, then\n it can be marked with an \"approve\" action. If the credential was\n rejected during the operation, use the \"reject\" action. In either\n case, `git credential` should be fed with the credential\n description from step (1) concatenated with the attempted\n credential (i.e., the output you got in step (2)).\n\nNote in the above proposed documentation I was trying to describe the\nbehavior of the command in your patch. It does not reflect changes which\nI will propose to the command's behavior below. :)\n\n> diff --git a/builtin/credential.c b/builtin/credential.c\n> new file mode 100644\n> index 0000000..9f00885\n> --- /dev/null\n> +++ b/builtin/credential.c\n> @@ -0,0 +1,40 @@\n> +#include \n> +#include \"cache.h\"\n> +#include \"credential.h\"\n> +#include \"string-list.h\"\n> +\n> +static const char usage_msg[] =\n> +\"credential \";\n> +\n> +void cmd_credential (int argc, char **argv, const char *prefix){\n> +\tconst char *op;\n> +\tstruct credential c = CREDENTIAL_INIT;\n> +\tint i;\n> +\n> +\top = argv[1];\n> +\tif (!op)\n> +\t\tusage(usage_msg);\n> +\n> +\tfor (i = 2; i < argc; i++)\n> +\t\tstring_list_append(&c.helpers, argv[i]);\n\nSo arguments past the first one become helpers that we try, regardless\nof the credential.helper config settings. Is there a reason for this to\nbe in the user-facing command?\n\nI assume this got copied by looking at test-credential. I'd be OK with\nincluding this feature in git-credential (and converting our test\nscripts to use it, so we can drop test-credential entirely). But\nprobably it should not soak up all of the command-line arguments, and\ninstead should be a hidden option like:\n\n git credential --helper=cache fill\n\nThat will give us more flexibility later down the road.\n\n> +\tif (credential_read(&c, stdin) < 0)\n> +\t\tdie(\"unable to read credential from stdin\");\n\nI think we want to provide a straight credential-reader like this,\nbecause it is the most flexible form of describing a credential. But I\nsuspect many callers will have a URL, and we are creating extra work for\nthem to break it down into components themselves.\n\nPerhaps it would be simpler to accept a URL on the command line, and\nalso provide a --stdin option for callers that want to feed it directly.\nSo:\n\n git credential fill https://example.com/foo.git\n\nwould be identical to:\n\n git credential --stdin fill <<\\EOF\n protocol=https\n host=example.com\n path=foo.git\n EOF\n\n> +\tif (!strcmp(op, \"fill\")) {\n> +\t\tcredential_fill(&c);\n> +\t\tif (c.username)\n> +\t\t\tprintf(\"username=%s\\n\", c.username);\n> +\t\tif (c.password)\n> +\t\t\tprintf(\"password=%s\\n\", c.password);\n> +\t}\n\nI am tempted to suggest that this actually output the _whole_\ncredential, not just the username and password. Coupled with the above\nbehavior, you would get:\n\n $ git credential fill https://example.com/foo.git\n protocol=https\n host=example.com\n path=foo.git\n username=bob\n password=secr3t\n\nwhich happens to be exactly what you want to feed back to the \"approve\"\nand \"reject\" actions (and it is not really any harder to parse).\n\nWe _could_ get by with allowing:\n\n git credential --stdin approve https://example.com/foo.git <<\\EOF\n username=bob\n password=secr3t\n EOF\n\nand having it combine the URL on the command-line with the entries on\nstdin (and indeed, I think that is the only sane thing to do when\n--stdin and a URL are both given).\n\nBut that implies that there will never be any extra attributes that are\nrelevant to the approve/reject that are not in the URL (e.g., elsewhere\nthere has been talk about helpers being able to add arbitrary fields to\nthe descriptions in order to communicate with each other). So it may be\nthat we want to encourage callers to save the result of \"fill\" and feed\nit back to \"approve\" or \"reject\" verbatim.\n\n-Peff\n"},{"id":"193258","messageId":"vpq1ulnnw81.fsf@bauges.imag.fr","threadId":"30752","inReplyTo":"7vzk8baca0.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH_v1] add 'git credential' plumbing command","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2012-06-10T13:16:14Z","receivedAt":"2012-06-10T13:16:14Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano writes:\n\n>> diff --git a/git.c b/git.c\n>> index d232de9..7cbd7d8 100644\n>> --- a/git.c\n>> +++ b/git.c\n>> @@ -353,6 +353,7 @@ static void handle_internal_command(int argc, const char **argv)\n>> \t\t{ \"commit-tree\", cmd_commit_tree, RUN_SETUP },\n>> \t\t{ \"config\", cmd_config, RUN_SETUP_GENTLY },\n>> \t\t{ \"count-objects\", cmd_count_objects, RUN_SETUP },\n>> +\t\t{ \"credential\", cmd_count_objects, RUN_SETUP },\n>\n> Does \"git credential\" need to have a git repository (i.e. run in a\n> git repository or in a working tree that is controlled by one)?\n\nIt shouldn't (hence, should use RUN_SETUP_GENTLY).\n\n> A scripted Porcelain you would write using \"git credential\" may want\n> to implement something like \"git clone\" or \"git ls-remote\" where you\n> do not have to be in an existing repository.\n\n(\"git clone\" might not be the best example, as the authentication is\nusually done in the \"git fetch\" part of \"git clone\", but never mind)\n\nActually, \"git credential\" has very little to do with Git, and could\neven be used in a git-unrelated script.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"193259","messageId":"vpqvcizmhj5.fsf@bauges.imag.fr","threadId":"30752","inReplyTo":"1339267502-13803-1-git-send-email-Javier.Roucher-Iglesias@ensimag.imag.fr","subject":"Re: [PATCH_v1] add 'git credential' plumbing command","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2012-06-10T13:18:54Z","receivedAt":"2012-06-10T13:18:54Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Javier.Roucher-Iglesias@ensimag.imag.fr writes:\n\n> +static const char usage_msg[] =\n> +\"credential \";\n[...]\n> +\tfor (i = 2; i < argc; i++)\n> +\t\tstring_list_append(&c.helpers, argv[i]);\n\nThis helpers argument is still there, and is now totally undocumented. I\nshouldn't have to repeat that so many times.\n\n> --- a/git.c\n> +++ b/git.c\n> @@ -353,6 +353,7 @@ static void handle_internal_command(int argc, const char **argv)\n> \t\t{ \"commit-tree\", cmd_commit_tree, RUN_SETUP },\n> \t\t{ \"config\", cmd_config, RUN_SETUP_GENTLY },\n> \t\t{ \"count-objects\", cmd_count_objects, RUN_SETUP },\n> +\t\t{ \"credential\", cmd_count_objects, RUN_SETUP },\n\nYour \"git credential\" runs cmd_count_objects. A little bit of testing\nshould have found that (my remarks about lack of testing still apply).\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"193271","messageId":"38899eac92a1ea5c17b98f5e7bd5d948@telesun.imag.fr","threadId":"30752","inReplyTo":"20120609215236.Horde.J-h4cnwdC4BP06mEUeqxRlA@webmail.minatec.grenoble-inp.fr","subject":"Re: [PATCH_v1] add 'git credential' plumbing command","fromName":"roucherj","fromEmail":"roucherj@telesun.imag.fr","sentAt":"2012-06-10T17:41:17Z","receivedAt":"2012-06-10T17:41:17Z","isPatch":false,"sender":{"key":"roucherj@telesun.imag.fr","avatar":null},"body":"On Sat, 09 Jun 2012 21:52:36 +0200, konglu@minatec.inpg.fr wrote:\n> Javier.Roucher-Iglesias@ensimag.imag.fr a écrit :\n>\n>> +git-credential - Providing and strore user credentials to git\n>\n> s/Providing/Provides/ & s/strore/store\n>\n>> +-If git-credential system have the password already stored\n>> +git-credential will answer with by STDOUT:\n>\n> s/have/has/\n>\n>> +Then if the password is correct, (note: is not git credential\n>> +how decides if password is correct or not. Is the external system\n>> +that have to authenticate the user) it can be stored using command\n>> +'git crendential approve' by providing the structure, by STDIN.\n>\n> Wouldn't the note be \"it's not git credential that decides if the \n> password is\n> correct or not. That part is done by the external system\" ?\n>\n>> +1. The 'git credential fill' makes the structure,\n>> +with this structure it will be able to save your\n>> +credentials, and if the credential is allready stored,\n>> +it will fill the password.\n>\n> s/allready/already/\n>\n\nThank's for the english writting corrections\nit will be change it for the next patch\n\n>> +void cmd_credential (int argc, char **argv, const char *prefix){\n>> +\tconst char *op;\n>> +\tstruct credential c = CREDENTIAL_INIT;\n>> +\tint i;\n>> +\n>> +\top = argv[1];\n>> +\tif (!op)\n>> +\t\tusage(usage_msg);\n>> +\n>> +\tfor (i = 2; i < argc; i++)\n>> +\t\tstring_list_append(&c.helpers, argv[i]);\n>> +\n>> +\tif (credential_read(&c, stdin) < 0)\n>> +\t\tdie(\"unable to read credential from stdin\");\n>> +\n>> +\tif (!strcmp(op, \"fill\")) {\n>> +\t\tcredential_fill(&c);\n>> +\t\tif (c.username)\n>> +\t\t\tprintf(\"username=%s\\n\", c.username);\n>> +\t\tif (c.password)\n>> +\t\t\tprintf(\"password=%s\\n\", c.password);\n>> +\t}\n>> +\telse if (!strcmp(op, \"approve\")) {\n>> +\t\tcredential_approve(&c);\n>> +\t}\n>> +\telse if (!strcmp(op, \"reject\")) {\n>> +\t\tcredential_reject(&c);\n>> +\t}\n>> +\telse\n>> +\t\tusage(usage_msg);\n>\n> Braces for the last \"else\" part. In general, the structure should be\n>\n> if (...) {\n> /*code*/\n> } else if (...) {\n> /*code*/\n> } else {\n> /*code*/\n> }\n>\n> If juste one block needs brances, all the other \"else if\"/\"else\" part\n> need it too.\n>\n> BTW, please be aware of the white spaces (here mostly in the doc) :).\n>\n> Lucien Kong.\n\nI will remove brances.\n"},{"id":"193272","messageId":"vpq8vfvghoi.fsf@bauges.imag.fr","threadId":"30752","inReplyTo":"38899eac92a1ea5c17b98f5e7bd5d948@telesun.imag.fr","subject":"Re: [PATCH_v1] add 'git credential' plumbing command","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2012-06-10T18:12:13Z","receivedAt":"2012-06-10T18:12:13Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"roucherj writes:\n\n> On Sat, 09 Jun 2012 21:52:36 +0200, konglu@minatec.inpg.fr wrote:\n>>> +void cmd_credential (int argc, char **argv, const char *prefix){\n>>> +\tconst char *op;\n>>> +\tstruct credential c = CREDENTIAL_INIT;\n>>> +\tint i;\n>>> +\n>>> +\top = argv[1];\n>>> +\tif (!op)\n>>> +\t\tusage(usage_msg);\n>>> +\n>>> +\tfor (i = 2; i < argc; i++)\n>>> +\t\tstring_list_append(&c.helpers, argv[i]);\n>>> +\n>>> +\tif (credential_read(&c, stdin) < 0)\n>>> +\t\tdie(\"unable to read credential from stdin\");\n>>> +\n>>> +\tif (!strcmp(op, \"fill\")) {\n>>> +\t\tcredential_fill(&c);\n>>> +\t\tif (c.username)\n>>> +\t\t\tprintf(\"username=%s\\n\", c.username);\n>>> +\t\tif (c.password)\n>>> +\t\t\tprintf(\"password=%s\\n\", c.password);\n>>> +\t}\n>>> +\telse if (!strcmp(op, \"approve\")) {\n>>> +\t\tcredential_approve(&c);\n>>> +\t}\n>>> +\telse if (!strcmp(op, \"reject\")) {\n>>> +\t\tcredential_reject(&c);\n>>> +\t}\n>>> +\telse\n>>> +\t\tusage(usage_msg);\n>>\n>> Braces for the last \"else\" part. In general, the structure should be\n>>\n>> if (...) {\n>> /*code*/\n>> } else if (...) {\n>> /*code*/\n>> } else {\n>> /*code*/\n>> }\n>>\n>> If juste one block needs brances, all the other \"else if\"/\"else\" part\n>> need it too.\n>>\n>> BTW, please be aware of the white spaces (here mostly in the doc) :).\n>>\n>> Lucien Kong.\n>\n> I will remove brances.\n\nThe remark was about adding them, not removing them. There's one branch\nof the if/else if/ with several instructions, so we usually put braces\neverywhere.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"193282","messageId":"20120610203259.GA2380@burratino","threadId":"30752","inReplyTo":"vpq1ulnnw81.fsf@bauges.imag.fr","subject":"Re: [PATCH_v1] add 'git credential' plumbing command","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-06-10T20:32:59Z","receivedAt":"2012-06-10T20:32:59Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Matthieu Moy wrote:\n> Junio C Hamano writes:\n\n>> Does \"git credential\" need to have a git repository (i.e. run in a\n>> git repository or in a working tree that is controlled by one)?\n>\n> It shouldn't (hence, should use RUN_SETUP_GENTLY).\n\nRather, that means it should use 0:\n\n\t\t\t{ \"credential\", cmd_credential },\n\n[...]\n> Actually, \"git credential\" has very little to do with Git, and could\n> even be used in a git-unrelated script.\n\nI suspect it would be simplest to make it a non-builtin. There are\nsome examples to take inspiration from in the PROGRAM_OBJS variable of\nthe Makefile.\n\nHope that helps,\nJonathan\n"},{"id":"193284","messageId":"20120610210224.GA3112@burratino","threadId":"30752","inReplyTo":"20120610203259.GA2380@burratino","subject":"Re: [PATCH_v1] add 'git credential' plumbing command","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-06-10T21:02:24Z","receivedAt":"2012-06-10T21:02:24Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jonathan Nieder wrote:\n> Matthieu Moy wrote:\n>> Junio C Hamano writes:\n\n>>> Does \"git credential\" need to have a git repository (i.e. run in a\n>>> git repository or in a working tree that is controlled by one)?\n>>\n>> It shouldn't (hence, should use RUN_SETUP_GENTLY).\n>\n> Rather, that means it should use 0:\n>\n> \t\t\t{ \"credential\", cmd_credential },\n\n... and it turns out I'm talking nonsense. RUN_SETUP_GENTLY would\nbe a sensible choice indeed, to allow the command to discover the\ncurrent repository and read .git/config from there indeed.\n\nSorry for the confusion,\nJonathan\n"},{"id":"193314","messageId":"7vlijt980w.fsf@alter.siamese.dyndns.org","threadId":"30752","inReplyTo":"20120610115619.GA6453@sigill.intra.peff.net","subject":"Re: [PATCH_v1] add 'git credential' plumbing command","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-11T15:34:55Z","receivedAt":"2012-06-11T15:34:55Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Perhaps it would be simpler to accept a URL on the command line, and\n> also provide a --stdin option for callers that want to feed it directly.\n> So:\n>\n> git credential fill https://example.com/foo.git\n>\n> would be identical to:\n>\n> git credential --stdin fill <<\\EOF\n> protocol=https\n> host=example.com\n> path=foo.git\n> EOF\n\nand \"git credential fill https://peff@example.com/foo.git\" would be\nidentical to the latter one with user=peff already filled in?\n\n> I am tempted to suggest that this actually output the _whole_\n> credential, not just the username and password. Coupled with the above\n> behavior, you would get:\n>\n> $ git credential fill https://example.com/foo.git\n> protocol=https\n> host=example.com\n> path=foo.git\n> username=bob\n> password=secr3t\n>\n> which happens to be exactly what you want to feed back to the \"approve\"\n> and \"reject\" actions (and it is not really any harder to parse).\n>\n> We _could_ get by with allowing:\n>\n> git credential --stdin approve https://example.com/foo.git <<\\EOF\n> username=bob\n> password=secr3t\n> EOF\n>\n> and having it combine the URL on the command-line with the entries on\n> stdin (and indeed, I think that is the only sane thing to do when\n> --stdin and a URL are both given).\n\nAll good suggestions ;-).\n"},{"id":"193315","messageId":"7vhauh97zn.fsf@alter.siamese.dyndns.org","threadId":"30752","inReplyTo":"20120610210224.GA3112@burratino","subject":"Re: [PATCH_v1] add 'git credential' plumbing command","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-11T15:35:40Z","receivedAt":"2012-06-11T15:35:40Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> Jonathan Nieder wrote:\n>> Matthieu Moy wrote:\n>>> Junio C Hamano writes:\n>\n>>>> Does \"git credential\" need to have a git repository (i.e. run in a\n>>>> git repository or in a working tree that is controlled by one)?\n>>>\n>>> It shouldn't (hence, should use RUN_SETUP_GENTLY).\n>>\n>> Rather, that means it should use 0:\n>>\n>> \t\t\t{ \"credential\", cmd_credential },\n>\n> ... and it turns out I'm talking nonsense. RUN_SETUP_GENTLY would\n> be a sensible choice indeed, to allow the command to discover the\n> current repository and read .git/config from there indeed.\n\nYeah, I think \"it is OK to be called outside a repository, but if\ninside we read and honor the configuration\" is a sensible behaviour\nfor this command.\n"},{"id":"193317","messageId":"20120611154518.GA12773@sigill.intra.peff.net","threadId":"30752","inReplyTo":"7vlijt980w.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH_v1] add 'git credential' plumbing command","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-11T15:45:18Z","receivedAt":"2012-06-11T15:45:18Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 11, 2012 at 08:34:55AM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > Perhaps it would be simpler to accept a URL on the command line, and\n> > also provide a --stdin option for callers that want to feed it directly.\n> > So:\n> >\n> > git credential fill https://example.com/foo.git\n> >\n> > would be identical to:\n> >\n> > git credential --stdin fill <<\\EOF\n> > protocol=https\n> > host=example.com\n> > path=foo.git\n> > EOF\n> \n> and \"git credential fill https://peff@example.com/foo.git\" would be\n> identical to the latter one with user=peff already filled in?\n\nExactly. Though see my other response to Matthieu, which notes that:\n\n git credential fill https://peff:supersecret@example.com/foo.git\n\nis problematic. :(\n\nProbably it should be spelled:\n\n git credential fill <<\\EOF\n url=https://peff:supersecret@example.com/foo.git\n EOF\n\nand then:\n\n git credential fill <<\\EOF\n url=https://peff:supersecret@example.com/foo.git\n username=junio\n password=othersecret\n\nwould do what you expect (break the URL out into its components, and\nthen override particular fields).\n\n-Peff\n"},{"id":"193338","messageId":"vpqr4tl4ti9.fsf@bauges.imag.fr","threadId":"30752","inReplyTo":"20120610115619.GA6453@sigill.intra.peff.net","subject":"Re: [PATCH_v1] add 'git credential' plumbing command","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2012-06-11T18:02:06Z","receivedAt":"2012-06-11T18:02:06Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Jeff King writes:\n\n> It's nice to have an example like this, but there's much detail missing\n> in how the format is specified. However, this format is already\n> documented in the \"helpers\" section of api-credentials.txt, so it\n> probably makes sense to refer to that document.\n\nI'd do it the other way around. api-credentials.txt is in technical/,\nwhile the document we're writing will end-up in a man page, which cannot\nlink to technical/.\n\nSo, it makes more sense to move the format specification to\ngit-credential.txt, and link to it from api-credentials.txt (now that we\nhave a nice way to link to manpages from technical/ ;-) ).\n\n> I assume this got copied by looking at test-credential. I'd be OK with\n> including this feature in git-credential (and converting our test\n> scripts to use it, so we can drop test-credential entirely). But\n> probably it should not soak up all of the command-line arguments, and\n> instead should be a hidden option like:\n>\n> git credential --helper=cache fill\n>\n> That will give us more flexibility later down the road.\n\nActually, this should already be possible with\n\n git -c credential.helper=cache credential fill\n\nI suspect that this feature will never be used outside tests, and if so,\nI don't think it deserves a command-line option.\n\n> I am tempted to suggest that this actually output the _whole_\n> credential, not just the username and password. Coupled with the above\n> behavior, you would get:\n>\n> $ git credential fill https://example.com/foo.git\n> protocol=https\n> host=example.com\n> path=foo.git\n> username=bob\n> password=secr3t\n>\n> which happens to be exactly what you want to feed back to the \"approve\"\n> and \"reject\" actions (and it is not really any harder to parse).\n\nI like that, yes.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"193340","messageId":"20120611181840.GC20134@sigill.intra.peff.net","threadId":"30752","inReplyTo":"vpqr4tl4ti9.fsf@bauges.imag.fr","subject":"Re: [PATCH_v1] add 'git credential' plumbing command","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-11T18:18:40Z","receivedAt":"2012-06-11T18:18:40Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 11, 2012 at 08:02:06PM +0200, Matthieu Moy wrote:\n\n> Jeff King writes:\n> \n> > It's nice to have an example like this, but there's much detail missing\n> > in how the format is specified. However, this format is already\n> > documented in the \"helpers\" section of api-credentials.txt, so it\n> > probably makes sense to refer to that document.\n> \n> I'd do it the other way around. api-credentials.txt is in technical/,\n> while the document we're writing will end-up in a man page, which cannot\n> link to technical/.\n\nWe do so already in a few places:\n\n $ cd Documentation && git grep 'link:technical/'\n git.txt:link:technical/api-index.html[GIT API documentation].\n gitcredentials.txt:link:technical/api-credentials.html[credentials API] for details.\n user-manual.txt:found in link:technical/pack-format.txt[technical/pack-format.txt].\n\nI think the rationale is that you would have the HTML documentation\ninstalled into /usr/share/doc/git-doc or similar, and asciidoc does\ncorrectly generate footnote references from those links. However, I\nwould be fine with putting the meat of it into git-credential, and\nhaving api-credentials refer back to it. It's easier on the user that\nway.\n\n> > git credential --helper=cache fill\n> >\n> > That will give us more flexibility later down the road.\n> \n> Actually, this should already be possible with\n> \n> git -c credential.helper=cache credential fill\n> \n> I suspect that this feature will never be used outside tests, and if so,\n> I don't think it deserves a command-line option.\n\nYeah, that is even better. The tests could also use test_config. I\nwould pick whichever of the two is more convenient for a particular\ntest.\n\n-Peff\n"}]}