Git development
 help / color / mirror / Atom feed
* Re: Question about commit message wrapping
From: Sidney San Martín @ 2011-12-09 14:10 UTC (permalink / raw)
  To: Frans Klaver, git
In-Reply-To: <op.v57na7120aolir@keputer>

On Dec 9, 2011, at 2:05 AM, Frans Klaver wrote:

> On Fri, 09 Dec 2011 02:59:06 +0100, Sidney San Martín <s@sidneysm.com> wrote:
> 
>> Hey, I want to ask about the practice of wrapping commit messages to 70-something charaters.
>> 
>> The webpage most cited about it, which I otherwise really like, is
>> 
>> 	http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html
>> 
>> *Nothing else* in my everyday life works this way anymore. Line wrapping gets done on the display end in my email client, my web browser, my ebook reader entirely automatically, and it adapts to the size of the window.
> 
> Actually, opera-mail autowraps at 72 characters but sets the text format to flowed. It also wraps the quoted text when you reply. But there's a reasonable chance that you don't use opera in your daily life. On the other hand I would not be surprised if most decent e-mail clients worked that way.
> 

Interesting… either way, the end result is that the receiving mail client can wrap the lines to whatever length it (or you, as its operator) desires, which I think we can agree is a good thing, right?

> Nobody's forcing you to use the same practice in your own projects anyway.
> 
> 
>> 
>> That article gives two reasons why commits should be wrapped to 72 columns. First:
>> 
>>> git log doesn’t do any special special wrapping of the commit messages. With the default pager of less -S, this means your paragraphs flow far off the edge of the screen, making them difficult to read. On an 80 column terminal, if we subtract 4 columns for the indent on the left and 4 more for symmetry on the right, we’re left with 72 columns.
>> 
>> Here, I put a patch at the bottom of this email that wraps commit messages to, right now, 80 columns when they're displayed. (It’s a quick one, probably needs configurability. Also, beware, I don’t program in C too much.)
> 
> Hm. Saying "that's how the tool works" is not a good reason in my opinion. There might be tons of other reasons for wrapping at 80 characters. Readability is one that comes to mind for me.
> 

That's my basic point. I hope it didn't seem like I was arguing against reading commit messages wrapped to 80 columns, by default. I only wanted to discuss whether it makes more sense to handle it on the display end instead of asking committers to do it in advance.

- My phone shows text most comfortably at about 40 characters per line. I do look at terminals at 80 columns most of the time, but not always, and I sometimes browse projects in GUI tools that use a proportional font in a window may be narrower or wider than that.

- Right now, when I *am* in an 80-col terminal I have to trust everyone else to wrap their commit messages. Not everyone does. I feel like it would be more effective to give git the ability to wrap them automatically when I read them.

>> 
>> Second:
>> 
>>> git format-patch --stdout converts a series of commits to a series of emails, using the messages for the message body. Good email netiquette dictates we wrap our plain text emails such that there’s room for a few levels of nested reply indicators without overflow in an 80 column terminal. (The current rails.git workflow doesn’t include email, but who knows what the future will bring.)
>> 
>> There's been a standard for flowed plain text emails (which don't have to wrap at 80 columns) for well over ten years, RFC-2646 and is widely supported. Besides, code in diffs is often longer than 7x characters, and wrapping, like `git log`, could be done inside git. FWIW, there are a bunch of merge commits with lines longer than 80 characters in the git repo itself.
> 
> Yes, that standard allows e-mail clients to display the text more fluidly, even if the source text is word-wrapped. While git uses e-mail format, it isn't an e-mail client. I always interpreted this whole thing as git basically creating plain-text e-mails. You're actually writing the source of the e-mail in your commit message. If you care about actual use in e-mail (like we do here on the list) you might want to add the relevant header to the mails. That said, Apple Mail (the client you used to send your mail) doesn't even use the RFC you quote in the sent message. That mail is going to be a pain in the butt to read in mutt from work ;).
> 

Sorry, I'm not sure what you mean by, “If you care about actual use in e-mail (like we do here on the list) you might want to add the relevant header to the mails”.

Interesting, I didn't realize that Mail didn't use it. It does, however, use quoted-printable which, as far as I can tell, has a similar effect on line wrapping. What happens when you view this email in mutt?

> 
>> 
>> Finally, people read commits these days in many other places than `git log` (and make commits in many other places than a text editor configured to wrap). Most every GUI and already word wraps commit messages just fine. As a result, there are commits in popular repos much longer than the 72-column standard and no one notices. Instead, properly-formatted commit messages end up looking cramped when you see them in anywhere wider than 80 columns.
> 
> Cramped? I think it's compact and actually I prefer it over long lines.
> 
>> Am I crazy?
> 
> Probably not. Don't take my word for it. I'm not a psychiatrist.
> 
> 
>> If this makes sense to anyone else, I'd be happy to help massage this into something git-worthy, with some help (never worked on Git before).
>> 
>> - - -
>> 
>> From a93b390d1506652d4ad41d1cbd987ba98a8deca0 Mon Sep 17 00:00:00 2001
>> From: =?UTF-8?q?Sidney=20San=20Marti=CC=81n?= <s@sidneysm.com>
>> Date: Thu, 8 Dec 2011 20:26:23 -0500
>> Subject: [PATCH] Wrap commit messages on display
>> 
>> - Wrap to 80 characters minus the indent
>> - Use a hanging indent for lines which begin with "- "
>> - Do not wrap lines which begin with whitespace
>> ---
>> pretty.c |   10 ++++++++--
>> 1 files changed, 8 insertions(+), 2 deletions(-)
>> 
>> diff --git a/pretty.c b/pretty.c
>> index 230fe1c..15804ce 100644
>> --- a/pretty.c
>> +++ b/pretty.c
>> @@ -1243,8 +1243,14 @@ void pp_remainder(const struct pretty_print_context *pp,
>> 			memset(sb->buf + sb->len, ' ', indent);
>> 			strbuf_setlen(sb, sb->len + indent);
>> 		}
>> -		strbuf_add(sb, line, linelen);
>> -		strbuf_addch(sb, '\n');
>> +		if (line[0] == ' ' || line[0] == '\t') {
>> +			strbuf_add(sb, line, linelen);
>> +		} else {
>> +			struct strbuf wrapped = STRBUF_INIT;
>> +			strbuf_add(&wrapped, line, linelen);
>> +			strbuf_add_wrapped_text(sb, wrapped.buf, 0, indent + (line[0] == '-' && line[1] == ' ' ? 2 : 0), 80 - indent);
> 
> While on the subject, In my mail view, the new line started with the [1] from line[1], in the quote the line looks entirely different. Now this is code we're talking about, so it makes slightly more sense to have a proper wrapping hard-coded. Compare the above with the following:
> 
> +			int hanging_indent = ((line[0] == '-' && line[1] == ' ') ? 2 : 0);
> [...]
> +			strbuf_add_wrapped_text(sb, wrapped.buf, 0,
> +									indent + hanging_indent,
> +									80 - indent);
> 
> Much clearer, no? I personally usually have two or three terminals tucked next to each other, so I can look at two or three things at the same time. 80 characters limit is a nice feature then.

Good point, that makes it clearer either way. I put an updated patch at the bottom of this email (also fixed forgetting the newline after lines with leading whitespace). I hope it's OK to include patches this way, I understand that they're supposed to represent whole emails but want to include them with this discussion.

> 
> 
>> +			strbuf_addch(sb, '\n');
>> +		}
>> 	}
>> }
>> 
> 
> Cheers,
> Frans


From 53fd7deedaf5ac522c9d752e79cf71561cc57f07 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sidney=20San=20Marti=CC=81n?= <s@sidneysm.com>
Date: Thu, 8 Dec 2011 20:26:23 -0500
Subject: [PATCH] Wrap commit messages on display

- Wrap to 80 characters, minus the indent
- Use a hanging indent for lines which begin with "- "
- Do not wrap lines which begin with whitespace
---
 pretty.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/pretty.c b/pretty.c
index 230fe1c..841ccd1 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1243,7 +1243,14 @@ void pp_remainder(const struct pretty_print_context *pp,
 			memset(sb->buf + sb->len, ' ', indent);
 			strbuf_setlen(sb, sb->len + indent);
 		}
-		strbuf_add(sb, line, linelen);
+		if (line[0] == ' ' || line[0] == '\t') {
+			strbuf_add(sb, line, linelen);
+		} else {
+			struct strbuf wrapped = STRBUF_INIT;
+			strbuf_add(&wrapped, line, linelen);
+			int hanging_indent = ((line[0] == '-' && line[1] == ' ') ? 2 : 0);
+			strbuf_add_wrapped_text(sb, wrapped.buf, 0, indent + hanging_indent, 80 - indent);
+		}
 		strbuf_addch(sb, '\n');
 	}
 }
-- 
1.7.8

^ permalink raw reply related

* Re: Auto update submodules after merge and reset
From: Andreas T.Auer @ 2011-12-10  1:41 UTC (permalink / raw)
  To: Phil Hord; +Cc: Jens Lehmann, git
In-Reply-To: <CABURp0rcT2FR3uOmhyPUV5W3pu7WuJzjXktmUq0eb4nOiUwDKA@mail.gmail.com>



On 10.12.2011 00:57 Phil Hord wrote:
> On Thu, Dec 8, 2011 at 4:13 AM,<andreas.t.auer_gtml_37453@ursus.ath.cx>  wrote:
>    
>> On 07.12.2011 23:23 Jens Lehmann wrote:
>>      
>>>> If you have tracking branches, the supermodule can just update the
>>>> corresponding branch. If this branch is currently checkedout and
>>>> the work area is clean, then the work area is updated, too. If
>>>> there is currently a local branch or a diffent super-branch
>>>> checked out then the working area should be considered "detached"
>>>> from the superproject and not updated.
>>>>          
>>>   This sounds a lot like the "follow branch tip" model we discussed
>>>   recently (which could be configured via .gitmodules), but I'm not
>>>   sure you really are in the same boat here.
>>>        
>> When I understood that correctly it was just a configuration to what branch
>> should be automatically checked out in the submodule. This seems to be too
>> complicated IMO, because when you have different branches in the
>> superproject then you may want to have different branches in the submodules,
>> too, but you would need to configure that submodule branch in .gitmodules
>> for each branch separately. I.e. in the master branch the .gitmodule may
>> contain "master", in the maint branch the .gitmodules may have "maint" as
>> the branch to follow.
>>      
> Yes, but maybe you can update this information in the .gitmodules file
> easily with a command.  Maybe it could be something simpler than "git
> sync-gitmodules-branches", but that is essentially what it would do:
> it would save the current branch in each submodule as the "tracked"
> branch in the .gitmodules file.
>
> The advantages to this, I think, are that
>
> 1. Your "submodule A follows branch X" information is explicit in the
> .gitmodules file and so it is not hidden when I examine your patch.
> (It sounds to me like the refs/super/* branches would necessarily be
> hard to find since the refs/ hierarchy is usually meta data about
> local and remote branches.  Maybe I should think about tags and notes
> more, though.)
>    
Branches can be seen as "dynamic data" that can easily be updated, 
renamed or even deleted, if a branch is merged into another.
On the other hand .gitmodules can be seen as "static data" because it is 
committed to the object database, so if you checkout an old revision, 
you could get a version of the .gitmodules that refers to a branch, 
which existed at that time, but was deleted meanwhile.

> 2. When you change to "submodule A now follows branch Y", this
> information can be unambiguously saved in the commit where it occurred
> rather than tucked away, again, in refs/super/*.
>    
If you place a reference in refs/super/ it will be displayed by gitk 
currently, so it is not really hidden.
> The disadvantage, maybe, are that you must now use 'git submodule
> sync' or something like that to put any .gitmodules changes into
> effect.
> Or maybe that is an advantage.  How often will this branch tracking change?
>    
It depends on your use case. In mine it will change quite often.
> For example, I have some repos like this:
>
> super
>     +--subA
>     +--subB
>
> I wish I could do this:
>     cd super&&  checkout master
>
> to get this:
>     super   (master)
>        +--subA  (master)
>        +--subB  (master)
>
> Or, if I have SubB on super/'master' tracking 'foo', I could get this:
>     super   (master)
>        +--subA  (master)
>        +--subB  (foo)
>    
No, the branch super/master always follows the master of the 
superproject. That's why it is called super/, because it contains the 
branchnames from the supermodule's namespace. The normal "local" 
submodule branches are in refs/heads/*. The references in refs/super can 
easily be created "on the fly" by the superproject, so they are not 
really properties of the submodules. It is a little bit like a cookie 
jar ;-).
>    
>> I do want to follow the tip of the branch, if the superproject has that
>> currently checked out. If the superproject checks out a tagged version for a
>> rebuild, then the submodule should not follow the tip, but should get a
>> detached HEAD of the corresponding commit, just as the superproject. When
>> the superproject goes back to the branch, the submodule should go back to
>> its tracking branch.
>>      
> Now this makes sense.  I want the same thing.  I want to preserve
> history on "old" commits, but I want to "advance to tip" on "new"
> commits.
> The trouble, I think, is in telling the difference between "old" and
> "new".
My approach says: Just like the superproject. If it checks out an old 
commit, do that, if it checks out the branch, follow.

> So maybe I need a new command that does this:
>      git checkout master&&  git submodule foreach 'git checkout master'
>
> Maybe it's called "git checkout master --recurse-submodules".  But I
> seem to recall this is already a non-starter for some reason, and
> anyway it doesn't solve the "variant branches in some submodules"
> problem.
>    
I don't know that problem, but maybe it is because the master branch of 
the submodule is not corresponding to the master branch of the 
superproject, which is a common use case, when external modules are used 
with different release cycles.
For that reason I chose to use a different namespace in 
refs/super/master instead of that maybe existing refs/heads/master of 
the submodule.
>
> You can commit to a detached HEAD.  I do it all the time.  The trick
> is in not switching away from a detached HEAD with your local commits
> still on it.  :-)
>    
Yes. And you can't push it, it can't be fetched, etc. So it really 
shouldn't be used that way, but you can do a lot of things you shouldn't 
do in git.
>> The first answer to my question was "yes, you need to transfer the refs or
>> you get unreferenced objects" and "no, you can't transfer the refs, because
>> they are owned by the superproject, not the submodule."
>> But binding a submodule to a superproject makes perfect sense if it is _one_
>> project that is split into submodules. In that case you only have one
>> superproject for a submodule and for that purpose it would be good workflow.
>>      
> This is not useful to me, though.  Sorry.
>
>    
It is useful in huge projects.
>> It is even nice to see which commits in the submodule belong to what
>> branches in the superproject or to what release version (so tracking
>> superproject tags would make sense, too). If you have a submodule that has
>> more than one superproject but these are well-defined, it could be solved
>> using refspecs (e.g. refs/super/foo/* for one and refs/super/bar/* for the
>> other superproject), but currently I can't think of a context where this
>> makes sense.
>>      
> I can, but this does put the cart before the horse.  The submodule is
> subservient to the super project in all my setups.  The submodule is
> not aware who owns him.  He is a bit like the DAG in reverse.  He
> knows one direction only (children), not the other (parents).
>
>    
In the setup I have in mind, the submodules are not subservient to the 
superproject, but a part of the whole project.

^ permalink raw reply

* Re: What's cooking in git.git (Dec 2011, #03; Fri, 9)
From: Jakub Narebski @ 2011-12-10  2:16 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vk465b834.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> --------------------------------------------------
> [Cooking]

> * jn/gitweb-side-by-side-diff (2011-10-31) 8 commits
>  - gitweb: Add navigation to select side-by-side diff
>  - gitweb: Use href(-replay=>1,...) for formats links in "commitdiff"
>  - t9500: Add basic sanity tests for side-by-side diff in gitweb
>  - t9500: Add test for handling incomplete lines in diff by gitweb
>  - gitweb: Give side-by-side diff extra CSS styling
>  - gitweb: Add a feature to show side-by-side diff
>  - gitweb: Extract formatting of diff chunk header
>  - gitweb: Refactor diff body line classification
> 
> Replaces a series from Kato Kazuyoshi on the same topic.
> Is this ready for 'next'?

I think it is.

-- 
Jakub Narębski

^ permalink raw reply

* git for change control of software deployment updates
From: Neal Kreitzinger @ 2011-12-10  2:37 UTC (permalink / raw)
  To: git

I am considering using git with submodules to deploy most of our updates to 
our customer linux servers (not including third party rpm updates already 
tracked by linux distro rpm repository).  Has anyone else done this? 
Comments?  (Sanity check.)  (I am new to submodules.)

Here are some rationale and concerns I submit for review:

Reasons:
- (Main Premise)  I maintain dozens of divergent custom versions 
concurrently with each running on its own server (we do merge these versions 
eventually and then immediately start diverging again).  (This is the main 
reason we chose git for scm.)  Hundreds of servers run the 'main generic' 
version.  Git will allow these branches to deploy to their respective 
customers.
- (Main Supporting Premise)  others on the git-newsgroup talk about using 
git submodules to maintain web sites.  Not sure how robust they are.  In 
theory, a website seems no different than any other application server.
- git on the customer server would allow better tracking of what the 
customer is really running (ie, dirty tree).
- Submodules will track everything outside of linux distro rpms that we 
update outside of linux rpms (third party binaries, html, etc.).
- Submodules containing source will be separate and not deployed.
- I already use git to deploy test versions of the core software from 
dev-server to qa-servers (using git-pull --ff-only, git-clone, etc. on the 
qa-server end).
- I can do most of the work using git features/git-scripts with a minimum of 
homegrown bash scripts/tars.
- git with ssh will make it secure (most customer servers are in vpn 
anyway).
- using separate worktree will separate git repos from deployed binaries.
- git will figure out the 'patches' to transfer in order to effect a version 
change.
- versioning of config files (apache, etc.) to aid in troubleshooting.
- trying to do this with rpm's makes no sense when I can do it with git.
- trying to do this with homemade scripts/tars makes no sense when I can do 
this with git.
- gitweb will make semi-technical support personnel use possible on the 
customer server in the absence of gitk (gui portion of linux).
- I can have a development superproject pointing to all submodules, and a 
deployment superproject pointing to same submodules but not including source 
code submodules.
- if fetch/pull are better for this than push I can setup a deployment 
server that customers have access to.  It seems fetch/pull are more 
conducive to monitoring success/failure, but push is more conducive to 
central control.
- having some history of previous versions seems valuable for rapid 
remediation of regressions (git checkout <prev-version> -- mybinary).
- remote tracking of customer server configurations.
- git could track pre/post-conversion/initialization data file states.
- track customer server configuration file customizations and merge them 
with subsequent changes to base version.
- such a git deployment setup would make emulating a prod server on a test 
server a matter of course.

Concerns:
- is there something about git that would make this unsecure during the 
"patch" transfer (push, fetch, etc)?
- can I limit disk use (repo history) using shallow-clone or some disk 
saving tricks?  IOW, repo on customer server only needs sufficient common 
ancestor to accept updates and not the whole history before the common 
ancestor.  (The majority of deployed content will be binaries.)
- customer servers will not have gui portion of linux loaded (minimal VM 
images on an array in most cases).
- automation of the execution of data format conversion/initialization 
programs (receive hook that checks for and executes and waits for completion 
of such programs) as such commits apply on the repo on the customer server 
or as an aggregate after all commits apply.  (I think of the new execute 
option in rebase, but that is different from receiving during a push.)
- upgrading git versions (breaking deployment system).
- size limits in git to versioning large data files (works for most 
customers, but not the huge ones).
- gitweb is getting smarter and is no longer 'view only' like the old days. 
Can it be made 'view only'?
- github (paid version) is too dumbed down to play well with this usage. 
(I've only read about github but may be asked to use it to some extent.)
- one little git command could wreak havoc on a live customer server.  Then 
again, so can one little linux command.  Is there a way to 'sudo' the git 
commands?
- I rely on porcelain and do not recreate my own porcelain from plumbing. 
(I am a porcelain level user.)
- detection of failures allowing for reset to previous version of software 
and data.
- limits to mass deployment (max approx 500 servers in one night). 
Concurrent pushes?  Concurrent fetches from same remote?
- is there something else about git that might make this untenable over 
time?

Thanks in advance.

v/r,
neal

^ permalink raw reply

* Re: [PATCH] Copy resolve_ref() return value for longer use
From: Tony Wang @ 2011-12-10  3:43 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nguyen Thai Ngoc Duy, git
In-Reply-To: <7vobwgo3l5.fsf@alter.siamese.dyndns.org>

Hi, 

I don't know about the procedure, but wonder is any one following this? 

-- 
BR,
Tony Wang


On Sunday, November 13, 2011 at 15:59, Junio C Hamano wrote:

> Nguyen Thai Ngoc Duy <pclouds@gmail.com (mailto:pclouds@gmail.com)> writes:
> 
> > I went through all of them. Most of them only checks if return value
> > is NULL, or does one-time string comparison. Others do xstrdup() to
> > save the return value. Will update the patch message to reflect this.
> 
> 
> 
> Thanks. Given your analysis, I think the potential change I alluded to to
> return allocated memory from the function is probably overkill in the
> current state of the code.
> 
> But as the codepaths around the existing callsites evolve, some of these
> call sites that are not problematic in today's code can become problematic
> if we are not careful. I was primarily wondering if an updated API could
> force us to be careful when making future changes.
> 
> And a change along the lines of your suggestion
> 
> > ... Though if you don't mind a bigger patch, we could turn
> > 
> > const char *resolve_ref(const char *path, const char *sha1, int
> > reading, int *flag);
> > 
> > to
> > 
> > int resolve_ref(const char *path, const char *sha1, int reading, int
> > *flag, char **ref);
> > 
> > where *ref is the current return value and is only allocated by
> > resolve_ref() if ref != NULL.
> 
> 
> 
> might be one such updated API that makes mistakes harder to make. I dunno.
> 
> But if we were to go that route, as the first step, it might make sense to
> rewrite "only checks if it returns NULL and uses sha1" callers to call
> either read_ref() or ref_exists(), so that we do not have to worry about
> leaking at such callers when we later decide to return allocated memory
> from resolve_ref().
> 
> 
> builtin/branch.c | 3 +--
> builtin/merge.c | 4 ++--
> 2 files changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/builtin/branch.c b/builtin/branch.c
> index 51ca6a0..94948a4 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -565,7 +565,6 @@ static int print_ref_list(int kinds, int detached, int verbose, int abbrev, stru
> static void rename_branch(const char *oldname, const char *newname, int force)
> {
> struct strbuf oldref = STRBUF_INIT, newref = STRBUF_INIT, logmsg = STRBUF_INIT;
> - unsigned char sha1[20];
> struct strbuf oldsection = STRBUF_INIT, newsection = STRBUF_INIT;
> int recovery = 0;
> 
> @@ -577,7 +576,7 @@ static void rename_branch(const char *oldname, const char *newname, int force)
> * Bad name --- this could be an attempt to rename a
> * ref that we used to allow to be created by accident.
> */
> - if (resolve_ref(oldref.buf, sha1, 1, NULL))
> + if (ref_exists(oldref.buf))
> recovery = 1;
> else
> die(_("Invalid branch name: '%s'"), oldname);
> diff --git a/builtin/merge.c b/builtin/merge.c
> index dffd5ec..42b4f9e 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -420,7 +420,7 @@ static struct object *want_commit(const char *name)
> static void merge_name(const char *remote, struct strbuf *msg)
> {
> struct object *remote_head;
> - unsigned char branch_head[20], buf_sha[20];
> + unsigned char branch_head[20];
> struct strbuf buf = STRBUF_INIT;
> struct strbuf bname = STRBUF_INIT;
> const char *ptr;
> @@ -479,7 +479,7 @@ static void merge_name(const char *remote, struct strbuf *msg)
> strbuf_addstr(&truname, "refs/heads/");
> strbuf_addstr(&truname, remote);
> strbuf_setlen(&truname, truname.len - len);
> - if (resolve_ref(truname.buf, buf_sha, 1, NULL)) {
> + if (ref_exists(truname.buf)) {
> strbuf_addf(msg,
> "%s\t\tbranch '%s'%s of .\n",
> sha1_to_hex(remote_head->sha1),

^ permalink raw reply

* Re: [PATCH] Copy resolve_ref() return value for longer use
From: Nguyen Thai Ngoc Duy @ 2011-12-10  4:48 UTC (permalink / raw)
  To: Tony Wang; +Cc: Junio C Hamano, git
In-Reply-To: <626C086D699644D181B9FA573EDFFCA6@gmail.com>

On Sat, Dec 10, 2011 at 10:43 AM, Tony Wang <wwwjfy@gmail.com> wrote:
> Hi,
>
> I don't know about the procedure, but wonder is any one following this?

This series has been merged in master. I'll resend patches to rename
resolve_ref() to resolve_ref_unsafe() soon.
-- 
Duy

^ permalink raw reply

* Re: [RFC/PATCH] add update to branch support for "floating submodules"
From: Leif Gruenwoldt @ 2011-12-10  5:50 UTC (permalink / raw)
  To: git
In-Reply-To: <20111129220854.GB2812@sandbox-rc.fritz.box>

Heiko Voigt <hvoigt <at> hvoigt.net> writes:

> 
> Hi,
> 
> On Wed, Nov 09, 2011 at 10:01:33AM -0800, Junio C Hamano wrote:
> > Heiko Voigt <hvoigt <at> hvoigt.net> writes:
> > 
> > > This is almost ready but I would like to know what users of the
> > > "floating submodule" think about this.
> > 
> > Thanks for working on this.
> > 
> > I do like to hear from potential users as well, because the general
> > impression we got was that floating submodules is not a real need of
> > anybody, but it is merely an inertia of people who (perhaps mistakenly)
> > thought svn externals that are not anchored to a particular revision is a
> > feature when it is just a limitation in reality. During the GitTogether'11
> > we learned that Android that uses floating model does not really have to.
> 
> Since we did not get any reply from potential floating submodule users I
> do not mind to drop this patch for now. It is archived in the mailing list
> and it should be easy to revive once there is real world need for it.
> 
> Once we have the "exact" model support for checkout and friends this
> might be a handy tool to update submodules before releases and such. But
> currently I would like to focus on the "exact" front first.
> 
> Cheers Heiko
> 

If I understand the description of "floating submodules", it's something I have 
been wanting for a while now! The lack of it is currently a deal breaker for 
using submodules within my organisation.

Our use case is as follows. We have several repositories for our common code 
(commonA.git, commonB.git, etc) and a few different products that leverage these 
common repos (productA.git, productB.git, etc). When one of the products is in 
heavy development we often need to do a lot of work in the common repos. Having 
to increment the sha1 of the submodules to track the latest tip would be overly 
arduous. (Obviously when development of the product stabilizes we would want to 
change to anchoring to a specific sha1 in the common repos).

^ permalink raw reply

* Re: What's cooking in git.git (Dec 2011, #03; Fri, 9)
From: Junio C Hamano @ 2011-12-10  6:13 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <m3vcppgojy.fsf@localhost.localdomain>

Jakub Narebski <jnareb@gmail.com> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> --------------------------------------------------
>> [Cooking]
>
>> * jn/gitweb-side-by-side-diff (2011-10-31) 8 commits
>>  ...
>> 
>> Replaces a series from Kato Kazuyoshi on the same topic.
>> Is this ready for 'next'?
>
> I think it is.

Thanks.

^ permalink raw reply

* Re: [RFC/PATCH] add update to branch support for "floating submodules"
From: Jonathan Nieder @ 2011-12-10  6:19 UTC (permalink / raw)
  To: Leif Gruenwoldt; +Cc: git, Heiko Voigt, Junio C Hamano
In-Reply-To: <loom.20111210T062013-538@post.gmane.org>

(restoring cc list)
Hi Leif,

Leif Gruenwoldt wrote:

> If I understand the description of "floating submodules", it's something I have 
> been wanting for a while now! The lack of it is currently a deal breaker for 
> using submodules within my organisation.
>
> Our use case is as follows.
[...]
>                                                 When one of the products is in 
> heavy development we often need to do a lot of work in the common repos. Having 
> to increment the sha1 of the submodules to track the latest tip would be overly 
> arduous.

What happens when a bug was introduced in this period of heavy development
and someone wants to look back in the development history and build each
version to find which introduced the bug?

If I were part of such a project, I would be tempted to follow one of two
rules.  Either

 A. Each commit of productA strives to work with the latest version of
    the common code possible.  Which version of the common code that was
    tested against gets recorded (perhaps by some record-submodule-versions-
    and-commit script, or even a pre-commit hook) so others can
    reproduce the results.

or

 B. Occasionally (e.g., daily or weekly) the "baseline" version of the
    common code that can be relied on gets bumped, and each commit of
    productA should work with that version and all later versions for a
    while.  Everyday development might typically happen with the tip
    version of the common code which may be faster, have more
    bugfixes, and otherwise be more pleasant to work with, but commits
    should work against the baseline version as well.  When it is time
    to bump the baseline, that fact gets recorded (in a separate
    commit).

    For this, the '[submodule "<name>"] ignore' setting described in
    gitmodules(5) might be helpful.

Though of course other variations are possible.

Would you be able to try out using Heiko's patch for a while, adapt it
to your needs as necessary, and let us know how it goes?

Thanks very much, and good luck,
Jonathan

^ permalink raw reply

* Re: [RFC/PATCH] add update to branch support for "floating submodules"
From: Junio C Hamano @ 2011-12-10  6:30 UTC (permalink / raw)
  To: Leif Gruenwoldt; +Cc: git
In-Reply-To: <loom.20111210T062013-538@post.gmane.org>

Leif Gruenwoldt <leifer@gmail.com> writes:

> Our use case is as follows. We have several repositories for our common code 
> (commonA.git, commonB.git, etc) and a few different products that leverage these 
> common repos (productA.git, productB.git, etc). When one of the products is in 
> heavy development we often need to do a lot of work in the common repos. Having 
> to increment the sha1 of the submodules to track the latest tip would be overly 
> arduous. (Obviously when development of the product stabilizes we would want to 
> change to anchoring to a specific sha1 in the common repos).

Nobody forces you to update the commit in the submodule bound to the
superproject tree every time you update areas that are unrelated to or
independent from that frequently updated submodule.

During the period the submodule is so often updated that you feel "having
to increment ... would be overly arduous", it does not matter which exact
commit in that submodule is used in the tree for your other modules and
the superproject. Otherwise you _would_ want to say something like "for
this entire tree state from the top-level superproject to correctly work,
we absolutely need to have this commit, not any commit that is older and
is known to be broken, from this submodule", and cannot afford to use
floating.

Which means by definition anybody who wants floating can instead let such
an often updated submodule stay somewhat stale by not running "submodule
update" for it unnecessarily.  In a well-modularized set of projects, the
interface to the busy submodule may be stable and I can imagine that kind
of arrangement would well be not just possible but practical, and probably
yours may be such a project.

So that use case does not sound like a good rationale to require addition
of floating submodules.

^ permalink raw reply

* [RFC/PATCH] show tracking branches with their associated branch names
From: Santhosh Kumar Mani @ 2011-12-10  7:40 UTC (permalink / raw)
  To: git

The "git branch" command, by default, displays the local branches. There
is no visual distinction made between the tracking branches and normal
local branches. This patch enables the "git branch" to display
tracking info for tracking branches:

Before this patch:
  $ git branch
  * master
    local

After this patch:
  $ git branch
  * master [origin/master]
    local

Signed-off-by: Santhosh Kumar Mani <santhoshmani@gmail.com>
---
 builtin/branch.c |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index e1e486e..4841416 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -373,6 +373,19 @@ static int ref_cmp(const void *r1, const void *r2)
 	return strcmp(c1->name, c2->name);
 }
 
+static int get_tracking_branch_name(struct strbuf *name, const char
*branch_name)
+{
+	struct branch *branch = branch_get(branch_name);
+
+	if (branch && branch->merge && branch->merge[0]->dst) {
+		strbuf_addf(name, " [%s]",
+				shorten_unambiguous_ref(branch->merge[0]->dst, 0));
+		return 1;
+	}
+
+	return 0;
+}
+
 static void fill_tracking_info(struct strbuf *stat, const char
*branch_name,
 		int show_upstream_ref)
 {
@@ -475,6 +488,9 @@ static void print_ref_item(struct ref_item *item,
int maxwidth, int verbose,
 	else if (verbose)
 		/* " f7c0c00 [ahead 58, behind 197] vcs-svn: drop obj_pool.h" */
 		add_verbose_info(&out, item, verbose, abbrev);
+	else if (get_tracking_branch_name(&out, item->name))
+		;
+
 	printf("%s\n", out.buf);
 	strbuf_release(&name);
 	strbuf_release(&out);
-- 
1.7.0.4

^ permalink raw reply related

* Re: [PATCH 1/2] t3401: modernize style
From: Martin von Zweigbergk @ 2011-12-10  8:14 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Ramkumar Ramachandra, git, Junio C Hamano
In-Reply-To: <20111209200703.GA21280@elie.hsd1.il.comcast.net>

On Fri, Dec 9, 2011 at 12:07 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Ramkumar Ramachandra wrote:
>
>> The motivation is unclear: lazy afternoon? :P
>
> Perhaps he was reading the list and after noticing a few patches in
> the same vein, realized that this test script could be made easier to
> read, too.

Sort of. These patches have been sitting in my repo since late Sept
and the patches you mention made me decide to send them out now. The
reason I did this back then was while trying to fix rebase to pick the
right patches when used with --onto. See this old discussion:
http://thread.gmane.org/gmane.comp.version-control.git/161917/focus=162041.
Also in the same series are patches teach rebase to only feed the
commit names to git-am (wrapped in a silly "From $sha1 Mon
Sep 17 00:00:00 2001" to please git-mailsplit). These patches have
been taking way too long, which is why I'm sending these little
cleanups separately.

>> Martin von Zweigbergk wrote:
>
>>> +       echo First > A &&
>>> +       git update-index --add A &&
>>> +       git commit -m "Add A." &&
>>
>> Style nit: >[^ ] is prevalent FWIW.
>
> Finally I caught on that you mean that redirection operators tend to
> be flush against the filename they are redirecting to.

So did I. I think I'll leave the code unchanged, though, because the
end result, after patch 2/2 is unaffected anyways (it removes
redirections).

>> While at it, why not change this "test ! -d" to
>> "test_path_is_missing"?

Will bake it into patch 2/2 if you don't mind. Unless there are other
comments, that would mean this patch can be applies as is, Junio.

> The patch looks good to me, too.  Thanks, both.

Thanks to you both, too.

^ permalink raw reply

* Access to git repository through squid proxy: The remote end hung up unexpectedly
From: Mathieu Peltier @ 2011-12-10  8:56 UTC (permalink / raw)
  To: git

Hi,
I am trying to access a git repository (git:// URL) through a squid proxy.

squid allows CONNECT for port 9418:
$ more /etc/squid/squid.conf
...
acl password proxy_auth REQUIRED
acl git_port port 9418
...
http_access allow CONNECT git_port password
http_access deny CONNECT !SSL_ports

The proxy server can connect to git server :
$ telnet git.server.org 9418
Trying w.x.y.z...
Connected to git.server.org.
Escape character is '^]'.

Here is the error I get on the client side:
$ git config --list
core.gitproxy=gitproxy.sh

$ more ~/bin/gitproxy.sh
#!/bin/sh
PROXY=x.domain.org
PROXYPORT=8080
PROXYAUTH=user:pass
DEBUG="-d -d -d -d"
exec socat $DEBUG - PROXY:$PROXY:$1:$2,proxyport=$PROXYPORT,proxyauth=$PROXYAUTH

$ git clone git://git.server.org/gitroot/repo/repo
Initialized empty Git repository in /tmp/GIT/repo/.git/
...
2011/12/09 12:22:44 socat[21428] D socat version 1.7.1.3 on Aug 23 2010 23:26:54
2011/12/09 12:22:44 socat[21428] D setenv("SOCAT_VERSION", "1.7.1.3", 1)
...
2011/12/09 12:22:44 socat[21428] D running on Linux version #1 SMP Tue
Mar 23 09:47:08 UTC 2010, release x.y
2011/12/09 12:22:44 socat[21428] D argv[0]: "socat"
2011/12/09 12:22:44 socat[21428] D argv[1]: "-d"
2011/12/09 12:22:44 socat[21428] D argv[2]: "-d"
2011/12/09 12:22:44 socat[21428] D argv[3]: "-d"
2011/12/09 12:22:44 socat[21428] D argv[4]: "-d"
2011/12/09 12:22:44 socat[21428] D argv[5]: "-"
2011/12/09 12:22:44 socat[21428] D argv[6]:
"PROXY:x.domain.org:git.server.org:9418,proxyport=8080,proxyauth=user:pass"
...
2011/12/09 12:22:44 socat[21428] I setting option "proxyport" to "8080"
2011/12/09 12:22:44 socat[21428] I setting option
"proxy-authorization" to "user:pass"
...
2011/12/09 12:22:44 socat[21428] I sending "CONNECT
git.server.org:9418 HTTP/1.0\r\n"
...
2011/12/09 12:22:44 socat[21428] I proxy_connect: received answer
"HTTP/1.1 403 OK\r\n"
2011/12/09 12:22:44 socat[21428] E CONNECT git.server.org:9418: OK
2011/12/09 12:22:44 socat[21428] N exit(1)
2011/12/09 12:22:44 socat[21428] I shutdown(3, 2)
2011/12/09 12:22:44 socat[21428] D shutdown()  -> 0
fatal: The remote end hung up unexpectedly

I tried to use also nc but I get the same error.
Any advice?
Thanks in advance,
Mathieu

^ permalink raw reply

* Re: Question about commit message wrapping
From: Andreas Schwab @ 2011-12-10  9:10 UTC (permalink / raw)
  To: Sidney San Martín; +Cc: Frans Klaver, git
In-Reply-To: <06819C5A-C6D3-4A14-9930-73F66707CE3E@sidneysm.com>

Sidney San Martín <s@sidneysm.com> writes:

> Interesting… either way, the end result is that the receiving mail client can wrap the lines to whatever length it (or you, as its operator) desires, which I think we can agree is a good thing, right?

Only if format=flowed.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply

* Re: [RFC/PATCH] show tracking branches with their associated branch names
From: Vincent van Ravesteijn @ 2011-12-10  9:53 UTC (permalink / raw)
  To: Santhosh Kumar Mani; +Cc: git
In-Reply-To: <1323502829.1698.6.camel@sdesktop>

Op 10-12-2011 8:40, Santhosh Kumar Mani schreef:
> The "git branch" command, by default, displays the local branches. There
> is no visual distinction made between the tracking branches and normal
> local branches. This patch enables the "git branch" to display
> tracking info for tracking branches:
>
> Before this patch:
>    $ git branch
>    * master
>      local
>
> After this patch:
>    $ git branch
>    * master [origin/master]
>      local
>
>
Did you try "git branch -vv" ?

Vincent

^ permalink raw reply

* [PATCHv3 0/13] credential helpers
From: Jeff King @ 2011-12-10 10:28 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Here's the latest re-roll of the credential helpers series. I think this
one is probably ready to go to 'next'.

It's rebased on the latest tip of 'master' (applying it to an older
commit will get you a minor textual conflict in strbuf.c). It
incorporates the erase-safety we discussed, fixes a few commit message
typos, and tweaks the test scripts to make testing the external OS X
helper a little easier.

  [01/13]: test-lib: add test_config_global variant
  [02/13]: t5550: fix typo
  [03/13]: introduce credentials API
  [04/13]: credential: add function for parsing url components
  [05/13]: http: use credential API to get passwords
  [06/13]: credential: apply helper config
  [07/13]: credential: add credential.*.username
  [08/13]: credential: make relevance of http path configurable
  [09/13]: docs: end-user documentation for the credential subsystem
  [10/13]: credentials: add "cache" helper
  [11/13]: strbuf: add strbuf_add*_urlencode
  [12/13]: credentials: add "store" helper
  [13/13]: t: add test harness for external credential helpers

-Peff

^ permalink raw reply

* [PATCHv3 01/13] test-lib: add test_config_global variant
From: Jeff King @ 2011-12-10 10:30 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <20111210102827.GA16460@sigill.intra.peff.net>

The point of test_config is to simultaneously set a config
variable and register its cleanup handler, like:

  test_config core.foo bar

However, it stupidly assumes that $1 contained the name of
the variable, which means it won't work for:

  test_config --global core.foo bar

We could try to parse the command-line ourselves and figure
out which parts need to be fed to test_unconfig. But since
this is likely the most common variant, it's much simpler
and less error-prone to simply add a new function.

Signed-off-by: Jeff King <peff@peff.net>
---
 t/test-lib.sh |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index bdd9513..160479b 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -379,6 +379,11 @@ test_config () {
 	git config "$@"
 }
 
+test_config_global () {
+	test_when_finished "test_unconfig --global '$1'" &&
+	git config --global "$@"
+}
+
 # Use test_set_prereq to tell that a particular prerequisite is available.
 # The prerequisite can later be checked for in two ways:
 #
-- 
1.7.8.rc2.40.gaf387

^ permalink raw reply related

* [PATCHv3 02/13] t5550: fix typo
From: Jeff King @ 2011-12-10 10:30 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <20111210102827.GA16460@sigill.intra.peff.net>

This didn't have an impact, because it was just setting up
an "expect" file that happened to be identical to the one in
the test before it.

Signed-off-by: Jeff King <peff@peff.net>
---
 t/t5550-http-fetch.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/t/t5550-http-fetch.sh b/t/t5550-http-fetch.sh
index 311a33c..3d6e871 100755
--- a/t/t5550-http-fetch.sh
+++ b/t/t5550-http-fetch.sh
@@ -66,7 +66,7 @@ test_expect_success 'cloning password-protected repository can fail' '
 
 test_expect_success 'http auth can use user/pass in URL' '
 	>askpass-query &&
-	echo wrong >askpass-reponse &&
+	echo wrong >askpass-response &&
 	git clone "$HTTPD_URL_USER_PASS/auth/repo.git" clone-auth-none &&
 	test_cmp askpass-expect-none askpass-query
 '
-- 
1.7.8.rc2.40.gaf387

^ permalink raw reply related

* [PATCHv3 03/13] introduce credentials API
From: Jeff King @ 2011-12-10 10:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <20111210102827.GA16460@sigill.intra.peff.net>

There are a few places in git that need to get a username
and password credential from the user; the most notable one
is HTTP authentication for smart-http pushing.

Right now the only choices for providing credentials are to
put them plaintext into your ~/.netrc, or to have git prompt
you (either on the terminal or via an askpass program). The
former is not very secure, and the latter is not very
convenient.

Unfortunately, there is no "always best" solution for
password management. The details will depend on the tradeoff
you want between security and convenience, as well as how
git can integrate with other security systems (e.g., many
operating systems provide a keychain or password wallet for
single sign-on).

This patch provides an abstract notion of credentials as a
data item, and provides three basic operations:

  - fill (i.e., acquire from external storage or from the
    user)

  - approve (mark a credential as "working" for further
    storage)

  - reject (mark a credential as "not working", so it can
    be removed from storage)

These operations can be backed by external helper processes
that interact with system- or user-specific secure storage.

Signed-off-by: Jeff King <peff@peff.net>
---
 .gitignore                                  |    1 +
 Documentation/technical/api-credentials.txt |  241 +++++++++++++++++++++++++++
 Makefile                                    |    3 +
 credential.c                                |  234 ++++++++++++++++++++++++++
 credential.h                                |   28 +++
 t/lib-credential.sh                         |   33 ++++
 t/t0300-credentials.sh                      |  195 ++++++++++++++++++++++
 test-credential.c                           |   38 +++++
 8 files changed, 773 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/technical/api-credentials.txt
 create mode 100644 credential.c
 create mode 100644 credential.h
 create mode 100755 t/lib-credential.sh
 create mode 100755 t/t0300-credentials.sh
 create mode 100644 test-credential.c

diff --git a/.gitignore b/.gitignore
index 8572c8c..7d2fefc 100644
--- a/.gitignore
+++ b/.gitignore
@@ -167,6 +167,7 @@
 /gitweb/static/gitweb.js
 /gitweb/static/gitweb.min.*
 /test-chmtime
+/test-credential
 /test-ctype
 /test-date
 /test-delta
diff --git a/Documentation/technical/api-credentials.txt b/Documentation/technical/api-credentials.txt
new file mode 100644
index 0000000..f624aef
--- /dev/null
+++ b/Documentation/technical/api-credentials.txt
@@ -0,0 +1,241 @@
+credentials API
+===============
+
+The credentials API provides an abstracted way of gathering username and
+password credentials from the user (even though credentials in the wider
+world can take many forms, in this document the word "credential" always
+refers to a username and password pair).
+
+Data Structures
+---------------
+
+`struct credential`::
+
+	This struct represents a single username/password combination
+	along with any associated context. All string fields should be
+	heap-allocated (or NULL if they are not known or not applicable).
+	The meaning of the individual context fields is the same as
+	their counterparts in the helper protocol; see the section below
+	for a description of each field.
++
+The `helpers` member of the struct is a `string_list` of helpers.  Each
+string specifies an external helper which will be run, in order, to
+either acquire or store credentials. See the section on credential
+helpers below.
++
+This struct should always be initialized with `CREDENTIAL_INIT` or
+`credential_init`.
+
+
+Functions
+---------
+
+`credential_init`::
+
+	Initialize a credential structure, setting all fields to empty.
+
+`credential_clear`::
+
+	Free any resources associated with the credential structure,
+	returning it to a pristine initialized state.
+
+`credential_fill`::
+
+	Instruct the credential subsystem to fill the username and
+	password fields of the passed credential struct by first
+	consulting helpers, then asking the user. After this function
+	returns, the username and password fields of the credential are
+	guaranteed to be non-NULL. If an error occurs, the function will
+	die().
+
+`credential_reject`::
+
+	Inform the credential subsystem that the provided credentials
+	have been rejected. This will cause the credential subsystem to
+	notify any helpers of the rejection (which allows them, for
+	example, to purge the invalid credentials from storage).  It
+	will also free() the username and password fields of the
+	credential and set them to NULL (readying the credential for
+	another call to `credential_fill`). Any errors from helpers are
+	ignored.
+
+`credential_approve`::
+
+	Inform the credential subsystem that the provided credentials
+	were successfully used for authentication.  This will cause the
+	credential subsystem to notify any helpers of the approval, so
+	that they may store the result to be used again.  Any errors
+	from helpers are ignored.
+
+Example
+-------
+
+The example below shows how the functions of the credential API could be
+used to login to a fictitious "foo" service on a remote host:
+
+-----------------------------------------------------------------------
+int foo_login(struct foo_connection *f)
+{
+	int status;
+	/*
+	 * Create a credential with some context; we don't yet know the
+	 * username or password.
+	 */
+
+	struct credential c = CREDENTIAL_INIT;
+	c.protocol = xstrdup("foo");
+	c.host = xstrdup(f->hostname);
+
+	/*
+	 * Fill in the username and password fields by contacting
+	 * helpers and/or asking the user. The function will die if it
+	 * fails.
+	 */
+	credential_fill(&c);
+
+	/*
+	 * Otherwise, we have a username and password. Try to use it.
+	 */
+	status = send_foo_login(f, c.username, c.password);
+	switch (status) {
+	case FOO_OK:
+		/* It worked. Store the credential for later use. */
+		credential_accept(&c);
+		break;
+	case FOO_BAD_LOGIN:
+		/* Erase the credential from storage so we don't try it
+		 * again. */
+		credential_reject(&c);
+		break;
+	default:
+		/*
+		 * Some other error occured. We don't know if the
+		 * credential is good or bad, so report nothing to the
+		 * credential subsystem.
+		 */
+	}
+
+	/* Free any associated resources. */
+	credential_clear(&c);
+
+	return status;
+}
+-----------------------------------------------------------------------
+
+
+Credential Helpers
+------------------
+
+Credential helpers are programs executed by git to fetch or save
+credentials from and to long-term storage (where "long-term" is simply
+longer than a single git process; e.g., credentials may be stored
+in-memory for a few minutes, or indefinitely on disk).
+
+Each helper is specified by a single string. The string is transformed
+by git into a command to be executed using these rules:
+
+  1. If the helper string begins with "!", it is considered a shell
+     snippet, and everything after the "!" becomes the command.
+
+  2. Otherwise, if the helper string begins with an absolute path, the
+     verbatim helper string becomes the command.
+
+  3. Otherwise, the string "git credential-" is prepended to the helper
+     string, and the result becomes the command.
+
+The resulting command then has an "operation" argument appended to it
+(see below for details), and the result is executed by the shell.
+
+Here are some example specifications:
+
+----------------------------------------------------
+# run "git credential-foo"
+foo
+
+# same as above, but pass an argument to the helper
+foo --bar=baz
+
+# the arguments are parsed by the shell, so use shell
+# quoting if necessary
+foo --bar="whitespace arg"
+
+# you can also use an absolute path, which will not use the git wrapper
+/path/to/my/helper --with-arguments
+
+# or you can specify your own shell snippet
+!f() { echo "password=`cat $HOME/.secret`"; }; f
+----------------------------------------------------
+
+Generally speaking, rule (3) above is the simplest for users to specify.
+Authors of credential helpers should make an effort to assist their
+users by naming their program "git-credential-$NAME", and putting it in
+the $PATH or $GIT_EXEC_PATH during installation, which will allow a user
+to enable it with `git config credential.helper $NAME`.
+
+When a helper is executed, it will have one "operation" argument
+appended to its command line, which is one of:
+
+`get`::
+
+	Return a matching credential, if any exists.
+
+`store`::
+
+	Store the credential, if applicable to the helper.
+
+`erase`::
+
+	Remove a matching credential, if any, from the helper's storage.
+
+The details of the credential will be provided on the helper's stdin
+stream. The credential is split into a set of named attributes.
+Attributes are provided to the helper, one per line. Each attribute is
+specified by a key-value pair, separated by an `=` (equals) sign,
+followed by a newline. The key may contain any bytes except `=`,
+newline, or NUL. The value may contain any bytes except newline or NUL.
+In both cases, all bytes are treated as-is (i.e., there is no quoting,
+and one cannot transmit a value with newline or NUL in it). The list of
+attributes is terminated by a blank line or end-of-file.
+
+Git will send the following attributes (but may not send all of
+them for a given credential; for example, a `host` attribute makes no
+sense when dealing with a non-network protocol):
+
+`protocol`::
+
+	The protocol over which the credential will be used (e.g.,
+	`https`).
+
+`host`::
+
+	The remote hostname for a network credential.
+
+`path`::
+
+	The path with which the credential will be used. E.g., for
+	accessing a remote https repository, this will be the
+	repository's path on the server.
+
+`username`::
+
+	The credential's username, if we already have one (e.g., from a
+	URL, from the user, or from a previously run helper).
+
+`password`::
+
+	The credential's password, if we are asking it to be stored.
+
+For a `get` operation, the helper should produce a list of attributes
+on stdout in the same format. A helper is free to produce a subset, or
+even no values at all if it has nothing useful to provide. Any provided
+attributes will overwrite those already known about by git.
+
+For a `store` or `erase` operation, the helper's output is ignored.
+If it fails to perform the requested operation, it may complain to
+stderr to inform the user. If it does not support the requested
+operation (e.g., a read-only store), it should silently ignore the
+request.
+
+If a helper receives any other operation, it should silently ignore the
+request. This leaves room for future operations to be added (older
+helpers will just ignore the new requests).
diff --git a/Makefile b/Makefile
index ed82320..d69f1dd 100644
--- a/Makefile
+++ b/Makefile
@@ -431,6 +431,7 @@ PROGRAM_OBJS += sh-i18n--envsubst.o
 PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS))
 
 TEST_PROGRAMS_NEED_X += test-chmtime
+TEST_PROGRAMS_NEED_X += test-credential
 TEST_PROGRAMS_NEED_X += test-ctype
 TEST_PROGRAMS_NEED_X += test-date
 TEST_PROGRAMS_NEED_X += test-delta
@@ -525,6 +526,7 @@ LIB_H += compat/win32/poll.h
 LIB_H += compat/win32/dirent.h
 LIB_H += connected.h
 LIB_H += convert.h
+LIB_H += credential.h
 LIB_H += csum-file.h
 LIB_H += decorate.h
 LIB_H += delta.h
@@ -611,6 +613,7 @@ LIB_OBJS += connect.o
 LIB_OBJS += connected.o
 LIB_OBJS += convert.o
 LIB_OBJS += copy.o
+LIB_OBJS += credential.o
 LIB_OBJS += csum-file.o
 LIB_OBJS += ctype.o
 LIB_OBJS += date.o
diff --git a/credential.c b/credential.c
new file mode 100644
index 0000000..86397f3
--- /dev/null
+++ b/credential.c
@@ -0,0 +1,234 @@
+#include "cache.h"
+#include "credential.h"
+#include "string-list.h"
+#include "run-command.h"
+
+void credential_init(struct credential *c)
+{
+	memset(c, 0, sizeof(*c));
+	c->helpers.strdup_strings = 1;
+}
+
+void credential_clear(struct credential *c)
+{
+	free(c->protocol);
+	free(c->host);
+	free(c->path);
+	free(c->username);
+	free(c->password);
+	string_list_clear(&c->helpers, 0);
+
+	credential_init(c);
+}
+
+static void credential_describe(struct credential *c, struct strbuf *out)
+{
+	if (!c->protocol)
+		return;
+	strbuf_addf(out, "%s://", c->protocol);
+	if (c->username && *c->username)
+		strbuf_addf(out, "%s@", c->username);
+	if (c->host)
+		strbuf_addstr(out, c->host);
+	if (c->path)
+		strbuf_addf(out, "/%s", c->path);
+}
+
+static char *credential_ask_one(const char *what, struct credential *c)
+{
+	struct strbuf desc = STRBUF_INIT;
+	struct strbuf prompt = STRBUF_INIT;
+	char *r;
+
+	credential_describe(c, &desc);
+	if (desc.len)
+		strbuf_addf(&prompt, "%s for '%s': ", what, desc.buf);
+	else
+		strbuf_addf(&prompt, "%s: ", what);
+
+	/* FIXME: for usernames, we should do something less magical that
+	 * actually echoes the characters. However, we need to read from
+	 * /dev/tty and not stdio, which is not portable (but getpass will do
+	 * it for us). http.c uses the same workaround. */
+	r = git_getpass(prompt.buf);
+
+	strbuf_release(&desc);
+	strbuf_release(&prompt);
+	return xstrdup(r);
+}
+
+static void credential_getpass(struct credential *c)
+{
+	if (!c->username)
+		c->username = credential_ask_one("Username", c);
+	if (!c->password)
+		c->password = credential_ask_one("Password", c);
+}
+
+int credential_read(struct credential *c, FILE *fp)
+{
+	struct strbuf line = STRBUF_INIT;
+
+	while (strbuf_getline(&line, fp, '\n') != EOF) {
+		char *key = line.buf;
+		char *value = strchr(key, '=');
+
+		if (!line.len)
+			break;
+
+		if (!value) {
+			warning("invalid credential line: %s", key);
+			strbuf_release(&line);
+			return -1;
+		}
+		*value++ = '\0';
+
+		if (!strcmp(key, "username")) {
+			free(c->username);
+			c->username = xstrdup(value);
+		} else if (!strcmp(key, "password")) {
+			free(c->password);
+			c->password = xstrdup(value);
+		} else if (!strcmp(key, "protocol")) {
+			free(c->protocol);
+			c->protocol = xstrdup(value);
+		} else if (!strcmp(key, "host")) {
+			free(c->host);
+			c->host = xstrdup(value);
+		} else if (!strcmp(key, "path")) {
+			free(c->path);
+			c->path = xstrdup(value);
+		}
+		/*
+		 * Ignore other lines; we don't know what they mean, but
+		 * this future-proofs us when later versions of git do
+		 * learn new lines, and the helpers are updated to match.
+		 */
+	}
+
+	strbuf_release(&line);
+	return 0;
+}
+
+static void credential_write_item(FILE *fp, const char *key, const char *value)
+{
+	if (!value)
+		return;
+	fprintf(fp, "%s=%s\n", key, value);
+}
+
+static void credential_write(const struct credential *c, FILE *fp)
+{
+	credential_write_item(fp, "protocol", c->protocol);
+	credential_write_item(fp, "host", c->host);
+	credential_write_item(fp, "path", c->path);
+	credential_write_item(fp, "username", c->username);
+	credential_write_item(fp, "password", c->password);
+}
+
+static int run_credential_helper(struct credential *c,
+				 const char *cmd,
+				 int want_output)
+{
+	struct child_process helper;
+	const char *argv[] = { NULL, NULL };
+	FILE *fp;
+
+	memset(&helper, 0, sizeof(helper));
+	argv[0] = cmd;
+	helper.argv = argv;
+	helper.use_shell = 1;
+	helper.in = -1;
+	if (want_output)
+		helper.out = -1;
+	else
+		helper.no_stdout = 1;
+
+	if (start_command(&helper) < 0)
+		return -1;
+
+	fp = xfdopen(helper.in, "w");
+	credential_write(c, fp);
+	fclose(fp);
+
+	if (want_output) {
+		int r;
+		fp = xfdopen(helper.out, "r");
+		r = credential_read(c, fp);
+		fclose(fp);
+		if (r < 0) {
+			finish_command(&helper);
+			return -1;
+		}
+	}
+
+	if (finish_command(&helper))
+		return -1;
+	return 0;
+}
+
+static int credential_do(struct credential *c, const char *helper,
+			 const char *operation)
+{
+	struct strbuf cmd = STRBUF_INIT;
+	int r;
+
+	if (helper[0] == '!')
+		strbuf_addstr(&cmd, helper + 1);
+	else if (is_absolute_path(helper))
+		strbuf_addstr(&cmd, helper);
+	else
+		strbuf_addf(&cmd, "git credential-%s", helper);
+
+	strbuf_addf(&cmd, " %s", operation);
+	r = run_credential_helper(c, cmd.buf, !strcmp(operation, "get"));
+
+	strbuf_release(&cmd);
+	return r;
+}
+
+void credential_fill(struct credential *c)
+{
+	int i;
+
+	if (c->username && c->password)
+		return;
+
+	for (i = 0; i < c->helpers.nr; i++) {
+		credential_do(c, c->helpers.items[i].string, "get");
+		if (c->username && c->password)
+			return;
+	}
+
+	credential_getpass(c);
+	if (!c->username && !c->password)
+		die("unable to get password from user");
+}
+
+void credential_approve(struct credential *c)
+{
+	int i;
+
+	if (c->approved)
+		return;
+	if (!c->username || !c->password)
+		return;
+
+	for (i = 0; i < c->helpers.nr; i++)
+		credential_do(c, c->helpers.items[i].string, "store");
+	c->approved = 1;
+}
+
+void credential_reject(struct credential *c)
+{
+	int i;
+
+	for (i = 0; i < c->helpers.nr; i++)
+		credential_do(c, c->helpers.items[i].string, "erase");
+
+	free(c->username);
+	c->username = NULL;
+	free(c->password);
+	c->password = NULL;
+	c->approved = 0;
+}
diff --git a/credential.h b/credential.h
new file mode 100644
index 0000000..2ea7d49
--- /dev/null
+++ b/credential.h
@@ -0,0 +1,28 @@
+#ifndef CREDENTIAL_H
+#define CREDENTIAL_H
+
+#include "string-list.h"
+
+struct credential {
+	struct string_list helpers;
+	unsigned approved:1;
+
+	char *username;
+	char *password;
+	char *protocol;
+	char *host;
+	char *path;
+};
+
+#define CREDENTIAL_INIT { STRING_LIST_INIT_DUP }
+
+void credential_init(struct credential *);
+void credential_clear(struct credential *);
+
+void credential_fill(struct credential *);
+void credential_approve(struct credential *);
+void credential_reject(struct credential *);
+
+int credential_read(struct credential *, FILE *);
+
+#endif /* CREDENTIAL_H */
diff --git a/t/lib-credential.sh b/t/lib-credential.sh
new file mode 100755
index 0000000..54ae1f4
--- /dev/null
+++ b/t/lib-credential.sh
@@ -0,0 +1,33 @@
+#!/bin/sh
+
+# Try a set of credential helpers; the expected stdin,
+# stdout and stderr should be provided on stdin,
+# separated by "--".
+check() {
+	read_chunk >stdin &&
+	read_chunk >expect-stdout &&
+	read_chunk >expect-stderr &&
+	test-credential "$@" <stdin >stdout 2>stderr &&
+	test_cmp expect-stdout stdout &&
+	test_cmp expect-stderr stderr
+}
+
+read_chunk() {
+	while read line; do
+		case "$line" in
+		--) break ;;
+		*) echo "$line" ;;
+		esac
+	done
+}
+
+
+cat >askpass <<\EOF
+#!/bin/sh
+echo >&2 askpass: $*
+what=`echo $1 | cut -d" " -f1 | tr A-Z a-z | tr -cd a-z`
+echo "askpass-$what"
+EOF
+chmod +x askpass
+GIT_ASKPASS="$PWD/askpass"
+export GIT_ASKPASS
diff --git a/t/t0300-credentials.sh b/t/t0300-credentials.sh
new file mode 100755
index 0000000..81a455f
--- /dev/null
+++ b/t/t0300-credentials.sh
@@ -0,0 +1,195 @@
+#!/bin/sh
+
+test_description='basic credential helper tests'
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-credential.sh
+
+test_expect_success 'setup helper scripts' '
+	cat >dump <<-\EOF &&
+	whoami=`echo $0 | sed s/.*git-credential-//`
+	echo >&2 "$whoami: $*"
+	while IFS== read key value; do
+		echo >&2 "$whoami: $key=$value"
+		eval "$key=$value"
+	done
+	EOF
+
+	cat >git-credential-useless <<-\EOF &&
+	#!/bin/sh
+	. ./dump
+	exit 0
+	EOF
+	chmod +x git-credential-useless &&
+
+	cat >git-credential-verbatim <<-\EOF &&
+	#!/bin/sh
+	user=$1; shift
+	pass=$1; shift
+	. ./dump
+	test -z "$user" || echo username=$user
+	test -z "$pass" || echo password=$pass
+	EOF
+	chmod +x git-credential-verbatim &&
+
+	PATH="$PWD:$PATH"
+'
+
+test_expect_success 'credential_fill invokes helper' '
+	check fill "verbatim foo bar" <<-\EOF
+	--
+	username=foo
+	password=bar
+	--
+	verbatim: get
+	EOF
+'
+
+test_expect_success 'credential_fill invokes multiple helpers' '
+	check fill useless "verbatim foo bar" <<-\EOF
+	--
+	username=foo
+	password=bar
+	--
+	useless: get
+	verbatim: get
+	EOF
+'
+
+test_expect_success 'credential_fill stops when we get a full response' '
+	check fill "verbatim one two" "verbatim three four" <<-\EOF
+	--
+	username=one
+	password=two
+	--
+	verbatim: get
+	EOF
+'
+
+test_expect_success 'credential_fill continues through partial response' '
+	check fill "verbatim one \"\"" "verbatim two three" <<-\EOF
+	--
+	username=two
+	password=three
+	--
+	verbatim: get
+	verbatim: get
+	verbatim: username=one
+	EOF
+'
+
+test_expect_success 'credential_fill passes along metadata' '
+	check fill "verbatim one two" <<-\EOF
+	protocol=ftp
+	host=example.com
+	path=foo.git
+	--
+	username=one
+	password=two
+	--
+	verbatim: get
+	verbatim: protocol=ftp
+	verbatim: host=example.com
+	verbatim: path=foo.git
+	EOF
+'
+
+test_expect_success 'credential_approve calls all helpers' '
+	check approve useless "verbatim one two" <<-\EOF
+	username=foo
+	password=bar
+	--
+	--
+	useless: store
+	useless: username=foo
+	useless: password=bar
+	verbatim: store
+	verbatim: username=foo
+	verbatim: password=bar
+	EOF
+'
+
+test_expect_success 'do not bother storing password-less credential' '
+	check approve useless <<-\EOF
+	username=foo
+	--
+	--
+	EOF
+'
+
+
+test_expect_success 'credential_reject calls all helpers' '
+	check reject useless "verbatim one two" <<-\EOF
+	username=foo
+	password=bar
+	--
+	--
+	useless: erase
+	useless: username=foo
+	useless: password=bar
+	verbatim: erase
+	verbatim: username=foo
+	verbatim: password=bar
+	EOF
+'
+
+test_expect_success 'usernames can be preserved' '
+	check fill "verbatim \"\" three" <<-\EOF
+	username=one
+	--
+	username=one
+	password=three
+	--
+	verbatim: get
+	verbatim: username=one
+	EOF
+'
+
+test_expect_success 'usernames can be overridden' '
+	check fill "verbatim two three" <<-\EOF
+	username=one
+	--
+	username=two
+	password=three
+	--
+	verbatim: get
+	verbatim: username=one
+	EOF
+'
+
+test_expect_success 'do not bother completing already-full credential' '
+	check fill "verbatim three four" <<-\EOF
+	username=one
+	password=two
+	--
+	username=one
+	password=two
+	--
+	EOF
+'
+
+# We can't test the basic terminal password prompt here because
+# getpass() tries too hard to find the real terminal. But if our
+# askpass helper is run, we know the internal getpass is working.
+test_expect_success 'empty helper list falls back to internal getpass' '
+	check fill <<-\EOF
+	--
+	username=askpass-username
+	password=askpass-password
+	--
+	askpass: Username:
+	askpass: Password:
+	EOF
+'
+
+test_expect_success 'internal getpass does not ask for known username' '
+	check fill <<-\EOF
+	username=foo
+	--
+	username=foo
+	password=askpass-password
+	--
+	askpass: Password:
+	EOF
+'
+
+test_done
diff --git a/test-credential.c b/test-credential.c
new file mode 100644
index 0000000..dee200e
--- /dev/null
+++ b/test-credential.c
@@ -0,0 +1,38 @@
+#include "cache.h"
+#include "credential.h"
+#include "string-list.h"
+
+static const char usage_msg[] =
+"test-credential <fill|approve|reject> [helper...]";
+
+int main(int argc, const char **argv)
+{
+	const char *op;
+	struct credential c = CREDENTIAL_INIT;
+	int i;
+
+	op = argv[1];
+	if (!op)
+		usage(usage_msg);
+	for (i = 2; i < argc; i++)
+		string_list_append(&c.helpers, argv[i]);
+
+	if (credential_read(&c, stdin) < 0)
+		die("unable to read credential from stdin");
+
+	if (!strcmp(op, "fill")) {
+		credential_fill(&c);
+		if (c.username)
+			printf("username=%s\n", c.username);
+		if (c.password)
+			printf("password=%s\n", c.password);
+	}
+	else if (!strcmp(op, "approve"))
+		credential_approve(&c);
+	else if (!strcmp(op, "reject"))
+		credential_reject(&c);
+	else
+		usage(usage_msg);
+
+	return 0;
+}
-- 
1.7.8.rc2.40.gaf387

^ permalink raw reply related

* [PATCHv3 04/13] credential: add function for parsing url components
From: Jeff King @ 2011-12-10 10:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <20111210102827.GA16460@sigill.intra.peff.net>

All of the components of a credential struct can be found in
a URL.  For example, the URL:

  http://foo:bar@example.com/repo.git

contains:

  protocol=http
  host=example.com
  path=repo.git
  username=foo
  password=bar

We want to be able to turn URLs into broken-down credential
structs so that we know two things:

  1. Which parts of the username/password we still need

  2. What the context of the request is (for prompting or
     as a key for storing credentials).

This code is based on http_auth_init in http.c, but needed a
few modifications in order to get all of the components that
the credential object is interested in.

Once the http code is switched over to the credential API,
then http_auth_init can just go away.

Signed-off-by: Jeff King <peff@peff.net>
---
 Documentation/technical/api-credentials.txt |    4 ++
 credential.c                                |   52 +++++++++++++++++++++++++++
 credential.h                                |    1 +
 3 files changed, 57 insertions(+), 0 deletions(-)

diff --git a/Documentation/technical/api-credentials.txt b/Documentation/technical/api-credentials.txt
index f624aef..21ca6a2 100644
--- a/Documentation/technical/api-credentials.txt
+++ b/Documentation/technical/api-credentials.txt
@@ -67,6 +67,10 @@ Functions
 	that they may store the result to be used again.  Any errors
 	from helpers are ignored.
 
+`credential_from_url`::
+
+	Parse a URL into broken-down credential fields.
+
 Example
 -------
 
diff --git a/credential.c b/credential.c
index 86397f3..c349b9a 100644
--- a/credential.c
+++ b/credential.c
@@ -2,6 +2,7 @@
 #include "credential.h"
 #include "string-list.h"
 #include "run-command.h"
+#include "url.h"
 
 void credential_init(struct credential *c)
 {
@@ -232,3 +233,54 @@ void credential_reject(struct credential *c)
 	c->password = NULL;
 	c->approved = 0;
 }
+
+void credential_from_url(struct credential *c, const char *url)
+{
+	const char *at, *colon, *cp, *slash, *host, *proto_end;
+
+	credential_clear(c);
+
+	/*
+	 * Match one of:
+	 *   (1) proto://<host>/...
+	 *   (2) proto://<user>@<host>/...
+	 *   (3) proto://<user>:<pass>@<host>/...
+	 */
+	proto_end = strstr(url, "://");
+	if (!proto_end)
+		return;
+	cp = proto_end + 3;
+	at = strchr(cp, '@');
+	colon = strchr(cp, ':');
+	slash = strchrnul(cp, '/');
+
+	if (!at || slash <= at) {
+		/* Case (1) */
+		host = cp;
+	}
+	else if (!colon || at <= colon) {
+		/* Case (2) */
+		c->username = url_decode_mem(cp, at - cp);
+		host = at + 1;
+	} else {
+		/* Case (3) */
+		c->username = url_decode_mem(cp, colon - cp);
+		c->password = url_decode_mem(colon + 1, at - (colon + 1));
+		host = at + 1;
+	}
+
+	if (proto_end - url > 0)
+		c->protocol = xmemdupz(url, proto_end - url);
+	if (slash - host > 0)
+		c->host = url_decode_mem(host, slash - host);
+	/* Trim leading and trailing slashes from path */
+	while (*slash == '/')
+		slash++;
+	if (*slash) {
+		char *p;
+		c->path = url_decode(slash);
+		p = c->path + strlen(c->path) - 1;
+		while (p > c->path && *p == '/')
+			*p-- = '\0';
+	}
+}
diff --git a/credential.h b/credential.h
index 2ea7d49..8a6d162 100644
--- a/credential.h
+++ b/credential.h
@@ -24,5 +24,6 @@ struct credential {
 void credential_reject(struct credential *);
 
 int credential_read(struct credential *, FILE *);
+void credential_from_url(struct credential *, const char *url);
 
 #endif /* CREDENTIAL_H */
-- 
1.7.8.rc2.40.gaf387

^ permalink raw reply related

* [PATCHv3 06/13] credential: apply helper config
From: Jeff King @ 2011-12-10 10:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <20111210102827.GA16460@sigill.intra.peff.net>

The functionality for credential storage helpers is already
there; we just need to give the users a way to turn it on.
This patch provides a "credential.helper" configuration
variable which allows the user to provide one or more helper
strings.

Rather than simply matching credential.helper, we will also
compare URLs in subsection headings to the current context.
This means you can apply configuration to a subset of
credentials. For example:

  [credential "https://example.com"]
	helper = foo

would match a request for "https://example.com/foo.git", but
not one for "https://kernel.org/foo.git".

This is overkill for the "helper" variable, since users are
unlikely to want different helpers for different sites (and
since helpers run arbitrary code, they could do the matching
themselves anyway).

However, future patches will add new config variables where
this extra feature will be more useful.

Signed-off-by: Jeff King <peff@peff.net>
---
 credential.c           |   61 ++++++++++++++++++++++++++++++++++++++++++++++++
 credential.h           |    5 +++-
 t/t0300-credentials.sh |   42 +++++++++++++++++++++++++++++++++
 t/t5550-http-fetch.sh  |   12 +++++++++
 4 files changed, 119 insertions(+), 1 deletions(-)

diff --git a/credential.c b/credential.c
index c349b9a..96be1c2 100644
--- a/credential.c
+++ b/credential.c
@@ -22,6 +22,61 @@ void credential_clear(struct credential *c)
 	credential_init(c);
 }
 
+int credential_match(const struct credential *want,
+		     const struct credential *have)
+{
+#define CHECK(x) (!want->x || (have->x && !strcmp(want->x, have->x)))
+	return CHECK(protocol) &&
+	       CHECK(host) &&
+	       CHECK(path) &&
+	       CHECK(username);
+#undef CHECK
+}
+
+static int credential_config_callback(const char *var, const char *value,
+				      void *data)
+{
+	struct credential *c = data;
+	const char *key, *dot;
+
+	key = skip_prefix(var, "credential.");
+	if (!key)
+		return 0;
+
+	if (!value)
+		return config_error_nonbool(var);
+
+	dot = strrchr(key, '.');
+	if (dot) {
+		struct credential want = CREDENTIAL_INIT;
+		char *url = xmemdupz(key, dot - key);
+		int matched;
+
+		credential_from_url(&want, url);
+		matched = credential_match(&want, c);
+
+		credential_clear(&want);
+		free(url);
+
+		if (!matched)
+			return 0;
+		key = dot + 1;
+	}
+
+	if (!strcmp(key, "helper"))
+		string_list_append(&c->helpers, value);
+
+	return 0;
+}
+
+static void credential_apply_config(struct credential *c)
+{
+	if (c->configured)
+		return;
+	git_config(credential_config_callback, c);
+	c->configured = 1;
+}
+
 static void credential_describe(struct credential *c, struct strbuf *out)
 {
 	if (!c->protocol)
@@ -195,6 +250,8 @@ void credential_fill(struct credential *c)
 	if (c->username && c->password)
 		return;
 
+	credential_apply_config(c);
+
 	for (i = 0; i < c->helpers.nr; i++) {
 		credential_do(c, c->helpers.items[i].string, "get");
 		if (c->username && c->password)
@@ -215,6 +272,8 @@ void credential_approve(struct credential *c)
 	if (!c->username || !c->password)
 		return;
 
+	credential_apply_config(c);
+
 	for (i = 0; i < c->helpers.nr; i++)
 		credential_do(c, c->helpers.items[i].string, "store");
 	c->approved = 1;
@@ -224,6 +283,8 @@ void credential_reject(struct credential *c)
 {
 	int i;
 
+	credential_apply_config(c);
+
 	for (i = 0; i < c->helpers.nr; i++)
 		credential_do(c, c->helpers.items[i].string, "erase");
 
diff --git a/credential.h b/credential.h
index 8a6d162..e504272 100644
--- a/credential.h
+++ b/credential.h
@@ -5,7 +5,8 @@
 
 struct credential {
 	struct string_list helpers;
-	unsigned approved:1;
+	unsigned approved:1,
+		 configured:1;
 
 	char *username;
 	char *password;
@@ -25,5 +26,7 @@ struct credential {
 
 int credential_read(struct credential *, FILE *);
 void credential_from_url(struct credential *, const char *url);
+int credential_match(const struct credential *have,
+		     const struct credential *want);
 
 #endif /* CREDENTIAL_H */
diff --git a/t/t0300-credentials.sh b/t/t0300-credentials.sh
index 81a455f..42d0f5b 100755
--- a/t/t0300-credentials.sh
+++ b/t/t0300-credentials.sh
@@ -192,4 +192,46 @@ test_expect_success 'internal getpass does not ask for known username' '
 	EOF
 '
 
+HELPER="!f() {
+		cat >/dev/null
+		echo username=foo
+		echo password=bar
+	}; f"
+test_expect_success 'respect configured credentials' '
+	test_config credential.helper "$HELPER" &&
+	check fill <<-\EOF
+	--
+	username=foo
+	password=bar
+	--
+	EOF
+'
+
+test_expect_success 'match configured credential' '
+	test_config credential.https://example.com.helper "$HELPER" &&
+	check fill <<-\EOF
+	protocol=https
+	host=example.com
+	path=repo.git
+	--
+	username=foo
+	password=bar
+	--
+	EOF
+'
+
+test_expect_success 'do not match configured credential' '
+	test_config credential.https://foo.helper "$HELPER" &&
+	check fill <<-\EOF
+	protocol=https
+	host=bar
+	--
+	username=askpass-username
+	password=askpass-password
+	--
+	askpass: Username for '\''https://bar'\'':
+	askpass: Password for '\''https://askpass-username@bar'\'':
+	EOF
+'
+
 test_done
diff --git a/t/t5550-http-fetch.sh b/t/t5550-http-fetch.sh
index 398a2d2..c59908f 100755
--- a/t/t5550-http-fetch.sh
+++ b/t/t5550-http-fetch.sh
@@ -101,6 +101,18 @@ test_expect_success 'http auth can request both user and pass' '
 	expect_askpass both user@host
 '
 
+test_expect_success 'http auth respects credential helper config' '
+	test_config_global credential.helper "!f() {
+		cat >/dev/null
+		echo username=user@host
+		echo password=user@host
+	}; f" &&
+	>askpass-query &&
+	echo wrong >askpass-response &&
+	git clone "$HTTPD_URL/auth/repo.git" clone-auth-helper &&
+	expect_askpass none
+'
+
 test_expect_success 'fetch changes via http' '
 	echo content >>file &&
 	git commit -a -m two &&
-- 
1.7.8.rc2.40.gaf387

^ permalink raw reply related

* [PATCHv3 05/13] http: use credential API to get passwords
From: Jeff King @ 2011-12-10 10:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <20111210102827.GA16460@sigill.intra.peff.net>

This patch converts the http code to use the new credential
API, both for http authentication as well as for getting
certificate passwords.

Most of the code change is simply variable naming (the
passwords are now contained inside the credential struct)
or deletion of obsolete code (the credential code handles
URL parsing and prompting for us).

The behavior should be the same, with one exception: the
credential code will prompt with a description based on the
credential components. Therefore, the old prompt of:

  Username for 'example.com':
  Password for 'example.com':

now looks like:

  Username for 'https://example.com/repo.git':
  Password for 'https://user@example.com/repo.git':

Note that we include more information in each line,
specifically:

  1. We now include the protocol. While more noisy, this is
     an important part of knowing what you are accessing
     (especially if you care about http vs https).

  2. We include the username in the password prompt. This is
     not a big deal when you have just been prompted for it,
     but the username may also come from the remote's URL
     (and after future patches, from configuration or
     credential helpers).  In that case, it's a nice
     reminder of the user for which you're giving the
     password.

  3. We include the path component of the URL. In many
     cases, the user won't care about this and it's simply
     noise (i.e., they'll use the same credential for a
     whole site). However, that is part of a larger
     question, which is whether path components should be
     part of credential context, both for prompting and for
     lookup by storage helpers. That issue will be addressed
     as a whole in a future patch.

Similarly, for unlocking certificates, we used to say:

  Certificate Password for 'example.com':

and we now say:

  Password for 'cert:///path/to/certificate':

Showing the path to the client certificate makes more sense,
as that is what you are unlocking, not "example.com".

Signed-off-by: Jeff King <peff@peff.net>
---
 http.c                |  113 +++++++++++--------------------------------------
 t/t5550-http-fetch.sh |   38 ++++++++++++-----
 2 files changed, 52 insertions(+), 99 deletions(-)

diff --git a/http.c b/http.c
index 44fcc4d..8e72664 100644
--- a/http.c
+++ b/http.c
@@ -3,6 +3,7 @@
 #include "sideband.h"
 #include "run-command.h"
 #include "url.h"
+#include "credential.h"
 
 int active_requests;
 int http_is_verbose;
@@ -41,7 +42,7 @@
 static int curl_ftp_no_epsv;
 static const char *curl_http_proxy;
 static const char *curl_cookie_file;
-static char *user_name, *user_pass, *description;
+static struct credential http_auth = CREDENTIAL_INIT;
 static const char *user_agent;
 
 #if LIBCURL_VERSION_NUM >= 0x071700
@@ -52,7 +53,7 @@
 #define CURLOPT_KEYPASSWD CURLOPT_SSLCERTPASSWD
 #endif
 
-static char *ssl_cert_password;
+static struct credential cert_auth = CREDENTIAL_INIT;
 static int ssl_cert_password_required;
 
 static struct curl_slist *pragma_header;
@@ -136,27 +137,6 @@ static void process_curl_messages(void)
 }
 #endif
 
-static char *git_getpass_with_description(const char *what, const char *desc)
-{
-	struct strbuf prompt = STRBUF_INIT;
-	char *r;
-
-	if (desc)
-		strbuf_addf(&prompt, "%s for '%s': ", what, desc);
-	else
-		strbuf_addf(&prompt, "%s: ", what);
-	/*
-	 * NEEDSWORK: for usernames, we should do something less magical that
-	 * actually echoes the characters. However, we need to read from
-	 * /dev/tty and not stdio, which is not portable (but getpass will do
-	 * it for us). http.c uses the same workaround.
-	 */
-	r = git_getpass(prompt.buf);
-
-	strbuf_release(&prompt);
-	return xstrdup(r);
-}
-
 static int http_options(const char *var, const char *value, void *cb)
 {
 	if (!strcmp("http.sslverify", var)) {
@@ -229,11 +209,11 @@ static int http_options(const char *var, const char *value, void *cb)
 
 static void init_curl_http_auth(CURL *result)
 {
-	if (user_name) {
+	if (http_auth.username) {
 		struct strbuf up = STRBUF_INIT;
-		if (!user_pass)
-			user_pass = xstrdup(git_getpass_with_description("Password", description));
-		strbuf_addf(&up, "%s:%s", user_name, user_pass);
+		credential_fill(&http_auth);
+		strbuf_addf(&up, "%s:%s",
+			    http_auth.username, http_auth.password);
 		curl_easy_setopt(result, CURLOPT_USERPWD,
 				 strbuf_detach(&up, NULL));
 	}
@@ -241,18 +221,14 @@ static void init_curl_http_auth(CURL *result)
 
 static int has_cert_password(void)
 {
-	if (ssl_cert_password != NULL)
-		return 1;
 	if (ssl_cert == NULL || ssl_cert_password_required != 1)
 		return 0;
-	/* Only prompt the user once. */
-	ssl_cert_password_required = -1;
-	ssl_cert_password = git_getpass_with_description("Certificate Password", description);
-	if (ssl_cert_password != NULL) {
-		ssl_cert_password = xstrdup(ssl_cert_password);
-		return 1;
-	} else
-		return 0;
+	if (!cert_auth.password) {
+		cert_auth.protocol = xstrdup("cert");
+		cert_auth.path = xstrdup(ssl_cert);
+		credential_fill(&cert_auth);
+	}
+	return 1;
 }
 
 static CURL *get_curl_handle(void)
@@ -279,7 +255,7 @@ static int has_cert_password(void)
 	if (ssl_cert != NULL)
 		curl_easy_setopt(result, CURLOPT_SSLCERT, ssl_cert);
 	if (has_cert_password())
-		curl_easy_setopt(result, CURLOPT_KEYPASSWD, ssl_cert_password);
+		curl_easy_setopt(result, CURLOPT_KEYPASSWD, cert_auth.password);
 #if LIBCURL_VERSION_NUM >= 0x070903
 	if (ssl_key != NULL)
 		curl_easy_setopt(result, CURLOPT_SSLKEY, ssl_key);
@@ -321,42 +297,6 @@ static int has_cert_password(void)
 	return result;
 }
 
-static void http_auth_init(const char *url)
-{
-	const char *at, *colon, *cp, *slash, *host;
-
-	cp = strstr(url, "://");
-	if (!cp)
-		return;
-
-	/*
-	 * Ok, the URL looks like "proto://something".  Which one?
-	 * "proto://<user>:<pass>@<host>/...",
-	 * "proto://<user>@<host>/...", or just
-	 * "proto://<host>/..."?
-	 */
-	cp += 3;
-	at = strchr(cp, '@');
-	colon = strchr(cp, ':');
-	slash = strchrnul(cp, '/');
-	if (!at || slash <= at) {
-		/* No credentials, but we may have to ask for some later */
-		host = cp;
-	}
-	else if (!colon || at <= colon) {
-		/* Only username */
-		user_name = url_decode_mem(cp, at - cp);
-		user_pass = NULL;
-		host = at + 1;
-	} else {
-		user_name = url_decode_mem(cp, colon - cp);
-		user_pass = url_decode_mem(colon + 1, at - (colon + 1));
-		host = at + 1;
-	}
-
-	description = url_decode_mem(host, slash - host);
-}
-
 static void set_from_env(const char **var, const char *envname)
 {
 	const char *val = getenv(envname);
@@ -429,7 +369,7 @@ void http_init(struct remote *remote, const char *url)
 		curl_ftp_no_epsv = 1;
 
 	if (url) {
-		http_auth_init(url);
+		credential_from_url(&http_auth, url);
 		if (!ssl_cert_password_required &&
 		    getenv("GIT_SSL_CERT_PASSWORD_PROTECTED") &&
 		    !prefixcmp(url, "https://"))
@@ -478,10 +418,10 @@ void http_cleanup(void)
 		curl_http_proxy = NULL;
 	}
 
-	if (ssl_cert_password != NULL) {
-		memset(ssl_cert_password, 0, strlen(ssl_cert_password));
-		free(ssl_cert_password);
-		ssl_cert_password = NULL;
+	if (cert_auth.password != NULL) {
+		memset(cert_auth.password, 0, strlen(cert_auth.password));
+		free(cert_auth.password);
+		cert_auth.password = NULL;
 	}
 	ssl_cert_password_required = 0;
 }
@@ -837,17 +777,11 @@ static int http_request(const char *url, void *result, int target, int options)
 		else if (missing_target(&results))
 			ret = HTTP_MISSING_TARGET;
 		else if (results.http_code == 401) {
-			if (user_name && user_pass) {
+			if (http_auth.username && http_auth.password) {
+				credential_reject(&http_auth);
 				ret = HTTP_NOAUTH;
 			} else {
-				/*
-				 * git_getpass is needed here because its very likely stdin/stdout are
-				 * pipes to our parent process.  So we instead need to use /dev/tty,
-				 * but that is non-portable.  Using git_getpass() can at least be stubbed
-				 * on other platforms with a different implementation if/when necessary.
-				 */
-				if (!user_name)
-					user_name = xstrdup(git_getpass_with_description("Username", description));
+				credential_fill(&http_auth);
 				init_curl_http_auth(slot->curl);
 				ret = HTTP_REAUTH;
 			}
@@ -866,6 +800,9 @@ static int http_request(const char *url, void *result, int target, int options)
 	curl_slist_free_all(headers);
 	strbuf_release(&buf);
 
+	if (ret == HTTP_OK)
+		credential_approve(&http_auth);
+
 	return ret;
 }
 
diff --git a/t/t5550-http-fetch.sh b/t/t5550-http-fetch.sh
index 3d6e871..398a2d2 100755
--- a/t/t5550-http-fetch.sh
+++ b/t/t5550-http-fetch.sh
@@ -49,40 +49,56 @@ test_expect_success 'setup askpass helpers' '
 	EOF
 	chmod +x askpass &&
 	GIT_ASKPASS="$PWD/askpass" &&
-	export GIT_ASKPASS &&
-	>askpass-expect-none &&
-	echo "askpass: Password for '\''$HTTPD_DEST'\'': " >askpass-expect-pass &&
-	{ echo "askpass: Username for '\''$HTTPD_DEST'\'': " &&
-	  cat askpass-expect-pass
-	} >askpass-expect-both
-'
+	export GIT_ASKPASS
+'
+
+expect_askpass() {
+	dest=$HTTPD_DEST/auth/repo.git
+	{
+		case "$1" in
+		none)
+			;;
+		pass)
+			echo "askpass: Password for 'http://$2@$dest': "
+			;;
+		both)
+			echo "askpass: Username for 'http://$dest': "
+			echo "askpass: Password for 'http://$2@$dest': "
+			;;
+		*)
+			false
+			;;
+		esac
+	} >askpass-expect &&
+	test_cmp askpass-expect askpass-query
+}
 
 test_expect_success 'cloning password-protected repository can fail' '
 	>askpass-query &&
 	echo wrong >askpass-response &&
 	test_must_fail git clone "$HTTPD_URL/auth/repo.git" clone-auth-fail &&
-	test_cmp askpass-expect-both askpass-query
+	expect_askpass both wrong
 '
 
 test_expect_success 'http auth can use user/pass in URL' '
 	>askpass-query &&
 	echo wrong >askpass-response &&
 	git clone "$HTTPD_URL_USER_PASS/auth/repo.git" clone-auth-none &&
-	test_cmp askpass-expect-none askpass-query
+	expect_askpass none
 '
 
 test_expect_success 'http auth can use just user in URL' '
 	>askpass-query &&
 	echo user@host >askpass-response &&
 	git clone "$HTTPD_URL_USER/auth/repo.git" clone-auth-pass &&
-	test_cmp askpass-expect-pass askpass-query
+	expect_askpass pass user@host
 '
 
 test_expect_success 'http auth can request both user and pass' '
 	>askpass-query &&
 	echo user@host >askpass-response &&
 	git clone "$HTTPD_URL/auth/repo.git" clone-auth-both &&
-	test_cmp askpass-expect-both askpass-query
+	expect_askpass both user@host
 '
 
 test_expect_success 'fetch changes via http' '
-- 
1.7.8.rc2.40.gaf387

^ permalink raw reply related

* [PATCHv3 07/13] credential: add credential.*.username
From: Jeff King @ 2011-12-10 10:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <20111210102827.GA16460@sigill.intra.peff.net>

Credential helpers can help users avoid having to type their
username and password over and over. However, some users may
not want a helper for their password, or they may be running
a helper which caches for a short time. In this case, it is
convenient to provide the non-secret username portion of
their credential via config.

Signed-off-by: Jeff King <peff@peff.net>
---
 credential.c           |    4 ++++
 t/t0300-credentials.sh |   13 +++++++++++++
 t/t5550-http-fetch.sh  |   16 ++++++++++++++++
 3 files changed, 33 insertions(+), 0 deletions(-)

diff --git a/credential.c b/credential.c
index 96be1c2..3c17ea1 100644
--- a/credential.c
+++ b/credential.c
@@ -65,6 +65,10 @@ static int credential_config_callback(const char *var, const char *value,
 
 	if (!strcmp(key, "helper"))
 		string_list_append(&c->helpers, value);
+	else if (!strcmp(key, "username")) {
+		if (!c->username)
+			c->username = xstrdup(value);
+	}
 
 	return 0;
 }
diff --git a/t/t0300-credentials.sh b/t/t0300-credentials.sh
index 42d0f5b..53e94bc 100755
--- a/t/t0300-credentials.sh
+++ b/t/t0300-credentials.sh
@@ -234,4 +234,17 @@ test_expect_success 'do not match configured credential' '
 	EOF
 '
 
+test_expect_success 'pull username from config' '
+	test_config credential.https://example.com.username foo &&
+	check fill <<-\EOF
+	protocol=https
+	host=example.com
+	--
+	username=foo
+	password=askpass-password
+	--
+	askpass: Password for '\''https://foo@example.com'\'':
+	EOF
+'
+
 test_done
diff --git a/t/t5550-http-fetch.sh b/t/t5550-http-fetch.sh
index c59908f..3262f90 100755
--- a/t/t5550-http-fetch.sh
+++ b/t/t5550-http-fetch.sh
@@ -113,6 +113,22 @@ test_expect_success 'http auth respects credential helper config' '
 	expect_askpass none
 '
 
+test_expect_success 'http auth can get username from config' '
+	test_config_global "credential.$HTTPD_URL.username" user@host &&
+	>askpass-query &&
+	echo user@host >askpass-response &&
+	git clone "$HTTPD_URL/auth/repo.git" clone-auth-user &&
+	expect_askpass pass user@host
+'
+
+test_expect_success 'configured username does not override URL' '
+	test_config_global "credential.$HTTPD_URL.username" wrong &&
+	>askpass-query &&
+	echo user@host >askpass-response &&
+	git clone "$HTTPD_URL_USER/auth/repo.git" clone-auth-user2 &&
+	expect_askpass pass user@host
+'
+
 test_expect_success 'fetch changes via http' '
 	echo content >>file &&
 	git commit -a -m two &&
-- 
1.7.8.rc2.40.gaf387

^ permalink raw reply related

* [PATCHv3 08/13] credential: make relevance of http path configurable
From: Jeff King @ 2011-12-10 10:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <20111210102827.GA16460@sigill.intra.peff.net>

When parsing a URL into a credential struct, we carefully
record each part of the URL, including the path on the
remote host, and use the result as part of the credential
context.

This had two practical implications:

  1. Credential helpers which store a credential for later
     access are likely to use the "path" portion as part of
     the storage key. That means that a request to

       https://example.com/foo.git

     would not use the same credential that was stored in an
     earlier request for:

       https://example.com/bar.git

  2. The prompt shown to the user includes all relevant
     context, including the path.

In most cases, however, users will have a single password
per host. The behavior in (1) will be inconvenient, and the
prompt in (2) will be overly long.

This patch introduces a config option to toggle the
relevance of http paths. When turned on, we use the path as
before. When turned off, we drop the path component from the
context: helpers don't see it, and it does not appear in the
prompt.

This is nothing you couldn't do with a clever credential
helper at the start of your stack, like:

  [credential "http://"]
	helper = "!f() { grep -v ^path= ; }; f"
	helper = your_real_helper

But doing this:

  [credential]
	useHttpPath = false

is way easier and more readable. Furthermore, since most
users will want the "off" behavior, that is the new default.
Users who want it "on" can set the variable (either for all
credentials, or just for a subset using
credential.*.useHttpPath).

Signed-off-by: Jeff King <peff@peff.net>
---
 credential.c           |   14 ++++++++++++++
 credential.h           |    3 ++-
 t/t0300-credentials.sh |   29 +++++++++++++++++++++++++++++
 t/t5550-http-fetch.sh  |    2 +-
 4 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/credential.c b/credential.c
index 3c17ea1..a17eafe 100644
--- a/credential.c
+++ b/credential.c
@@ -69,16 +69,30 @@ static int credential_config_callback(const char *var, const char *value,
 		if (!c->username)
 			c->username = xstrdup(value);
 	}
+	else if (!strcmp(key, "usehttppath"))
+		c->use_http_path = git_config_bool(var, value);
 
 	return 0;
 }
 
+static int proto_is_http(const char *s)
+{
+	if (!s)
+		return 0;
+	return !strcmp(s, "https") || !strcmp(s, "http");
+}
+
 static void credential_apply_config(struct credential *c)
 {
 	if (c->configured)
 		return;
 	git_config(credential_config_callback, c);
 	c->configured = 1;
+
+	if (!c->use_http_path && proto_is_http(c->protocol)) {
+		free(c->path);
+		c->path = NULL;
+	}
 }
 
 static void credential_describe(struct credential *c, struct strbuf *out)
diff --git a/credential.h b/credential.h
index e504272..96ea41b 100644
--- a/credential.h
+++ b/credential.h
@@ -6,7 +6,8 @@
 struct credential {
 	struct string_list helpers;
 	unsigned approved:1,
-		 configured:1;
+		 configured:1,
+		 use_http_path:1;
 
 	char *username;
 	char *password;
diff --git a/t/t0300-credentials.sh b/t/t0300-credentials.sh
index 53e94bc..885af8f 100755
--- a/t/t0300-credentials.sh
+++ b/t/t0300-credentials.sh
@@ -247,4 +247,33 @@ test_expect_success 'pull username from config' '
 	EOF
 '
 
+test_expect_success 'http paths can be part of context' '
+	check fill "verbatim foo bar" <<-\EOF &&
+	protocol=https
+	host=example.com
+	path=foo.git
+	--
+	username=foo
+	password=bar
+	--
+	verbatim: get
+	verbatim: protocol=https
+	verbatim: host=example.com
+	EOF
+	test_config credential.https://example.com.useHttpPath true &&
+	check fill "verbatim foo bar" <<-\EOF
+	protocol=https
+	host=example.com
+	path=foo.git
+	--
+	username=foo
+	password=bar
+	--
+	verbatim: get
+	verbatim: protocol=https
+	verbatim: host=example.com
+	verbatim: path=foo.git
+	EOF
+'
+
 test_done
diff --git a/t/t5550-http-fetch.sh b/t/t5550-http-fetch.sh
index 3262f90..95a133d 100755
--- a/t/t5550-http-fetch.sh
+++ b/t/t5550-http-fetch.sh
@@ -53,7 +53,7 @@ test_expect_success 'setup askpass helpers' '
 '
 
 expect_askpass() {
-	dest=$HTTPD_DEST/auth/repo.git
+	dest=$HTTPD_DEST
 	{
 		case "$1" in
 		none)
-- 
1.7.8.rc2.40.gaf387

^ permalink raw reply related

* [PATCHv3 09/13] docs: end-user documentation for the credential subsystem
From: Jeff King @ 2011-12-10 10:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <20111210102827.GA16460@sigill.intra.peff.net>

The credential API and helper format is already defined in
technical/api-credentials.txt.  This presents the end-user
view.

Signed-off-by: Jeff King <peff@peff.net>
---
 Documentation/Makefile           |    1 +
 Documentation/config.txt         |   23 +++++
 Documentation/gitcredentials.txt |  171 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 195 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/gitcredentials.txt

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 304b31e..116f175 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -7,6 +7,7 @@ MAN5_TXT=gitattributes.txt gitignore.txt gitmodules.txt githooks.txt \
 MAN7_TXT=gitcli.txt gittutorial.txt gittutorial-2.txt \
 	gitcvs-migration.txt gitcore-tutorial.txt gitglossary.txt \
 	gitdiffcore.txt gitnamespaces.txt gitrevisions.txt gitworkflows.txt
+MAN7_TXT += gitcredentials.txt
 
 MAN_TXT = $(MAN1_TXT) $(MAN5_TXT) $(MAN7_TXT)
 MAN_XML=$(patsubst %.txt,%.xml,$(MAN_TXT))
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 8a7d2d4..c6630c7 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -834,6 +834,29 @@ commit.template::
 	"{tilde}/" is expanded to the value of `$HOME` and "{tilde}user/" to the
 	specified user's home directory.
 
+credential.helper::
+	Specify an external helper to be called when a username or
+	password credential is needed; the helper may consult external
+	storage to avoid prompting the user for the credentials. See
+	linkgit:gitcredentials[7] for details.
+
+credential.useHttpPath::
+	When acquiring credentials, consider the "path" component of an http
+	or https URL to be important. Defaults to false. See
+	linkgit:gitcredentials[7] for more information.
+
+credential.username::
+	If no username is set for a network authentication, use this username
+	by default. See credential.<context>.* below, and
+	linkgit:gitcredentials[7].
+
+credential.<url>.*::
+	Any of the credential.* options above can be applied selectively to
+	some credentials. For example "credential.https://example.com.username"
+	would set the default username only for https connections to
+	example.com. See linkgit:gitcredentials[7] for details on how URLs are
+	matched.
+
 include::diff-config.txt[]
 
 difftool.<tool>.path::
diff --git a/Documentation/gitcredentials.txt b/Documentation/gitcredentials.txt
new file mode 100644
index 0000000..07f6596
--- /dev/null
+++ b/Documentation/gitcredentials.txt
@@ -0,0 +1,171 @@
+gitcredentials(7)
+=================
+
+NAME
+----
+gitcredentials - providing usernames and passwords to git
+
+SYNOPSIS
+--------
+------------------
+git config credential.https://example.com.username myusername
+git config credential.helper "$helper $options"
+------------------
+
+DESCRIPTION
+-----------
+
+Git will sometimes need credentials from the user in order to perform
+operations; for example, it may need to ask for a username and password
+in order to access a remote repository over HTTP. This manual describes
+the mechanisms git uses to request these credentials, as well as some
+features to avoid inputting these credentials repeatedly.
+
+REQUESTING CREDENTIALS
+----------------------
+
+Without any credential helpers defined, git will try the following
+strategies to ask the user for usernames and passwords:
+
+1. If the `GIT_ASKPASS` environment variable is set, the program
+   specified by the variable is invoked. A suitable prompt is provided
+   to the program on the command line, and the user's input is read
+   from its standard output.
+
+2. Otherwise, if the `core.askpass` configuration variable is set, its
+   value is used as above.
+
+3. Otherwise, if the `SSH_ASKPASS` environment variable is set, its
+   value is used as above.
+
+4. Otherwise, the user is prompted on the terminal.
+
+AVOIDING REPETITION
+-------------------
+
+It can be cumbersome to input the same credentials over and over.  Git
+provides two methods to reduce this annoyance:
+
+1. Static configuration of usernames for a given authentication context.
+
+2. Credential helpers to cache or store passwords, or to interact with
+   a system password wallet or keychain.
+
+The first is simple and appropriate if you do not have secure storage available
+for a password. It is generally configured by adding this to your config:
+
+---------------------------------------
+[credential "https://example.com"]
+	username = me
+---------------------------------------
+
+Credential helpers, on the other hand, are external programs from which git can
+request both usernames and passwords; they typically interface with secure
+storage provided by the OS or other programs.
+
+To use a helper, you must first select one to use.  Git does not yet
+include any credential helpers, but you may have third-party helpers
+installed; search for `credential-*` in the output of `git help -a`, and
+consult the documentation of individual helpers.  Once you have selected
+a helper, you can tell git to use it by putting its name into the
+credential.helper variable.
+
+1. Find a helper.
++
+-------------------------------------------
+$ git help -a | grep credential-
+credential-foo
+-------------------------------------------
+
+2. Read its description.
++
+-------------------------------------------
+$ git help credential-foo
+-------------------------------------------
+
+3. Tell git to use it.
++
+-------------------------------------------
+$ git config --global credential.helper foo
+-------------------------------------------
+
+If there are multiple instances of the `credential.helper` configuration
+variable, each helper will be tried in turn, and may provide a username,
+password, or nothing. Once git has acquired both a username and a
+password, no more helpers will be tried.
+
+
+CREDENTIAL CONTEXTS
+-------------------
+
+Git considers each credential to have a context defined by a URL. This context
+is used to look up context-specific configuration, and is passed to any
+helpers, which may use it as an index into secure storage.
+
+For instance, imagine we are accessing `https://example.com/foo.git`. When git
+looks into a config file to see if a section matches this context, it will
+consider the two a match if the context is a more-specific subset of the
+pattern in the config file. For example, if you have this in your config file:
+
+--------------------------------------
+[credential "https://example.com"]
+	username = foo
+--------------------------------------
+
+then we will match: both protocols are the same, both hosts are the same, and
+the "pattern" URL does not care about the path component at all. However, this
+context would not match:
+
+--------------------------------------
+[credential "https://kernel.org"]
+	username = foo
+--------------------------------------
+
+because the hostnames differ. Nor would it match `foo.example.com`; git
+compares hostnames exactly, without considering whether two hosts are part of
+the same domain. Likewise, a config entry for `http://example.com` would not
+match: git compares the protocols exactly.
+
+
+CONFIGURATION OPTIONS
+---------------------
+
+Options for a credential context can be configured either in
+`credential.\*` (which applies to all credentials), or
+`credential.<url>.\*`, where <url> matches the context as described
+above.
+
+The following options are available in either location:
+
+helper::
+
+	The name of an external credential helper, and any associated options.
+	If the helper name is not an absolute path, then the string `git
+	credential-` is prepended. The resulting string is executed by the
+	shell (so, for example, setting this to `foo --option=bar` will execute
+	`git credential-foo --option=bar` via the shell. See the manual of
+	specific helpers for examples of their use.
+
+username::
+
+	A default username, if one is not provided in the URL.
+
+useHttpPath::
+
+	By default, git does not consider the "path" component of an http URL
+	to be worth matching via external helpers. This means that a credential
+	stored for `https://example.com/foo.git` will also be used for
+	`https://example.com/bar.git`. If you do want to distinguish these
+	cases, set this option to `true`.
+
+
+CUSTOM HELPERS
+--------------
+
+You can write your own custom helpers to interface with any system in
+which you keep credentials. See the documentation for git's
+link:technical/api-credentials.html[credentials API] for details.
+
+GIT
+---
+Part of the linkgit:git[1] suite
-- 
1.7.8.rc2.40.gaf387

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox