Re: [PATCH v3 0/8] Hiding refs

2013-02-05 Thread Michael Haggerty
On 01/30/2013 07:45 PM, Junio C Hamano wrote:
 The third round.
 
  - Multi-valued variable transfer.hiderefs lists prefixes of ref
hierarchies to be hidden from the requests coming over the
network.
 
  - A configuration optionally allows uploadpack to accept fetch
requests for an object at the tip of a hidden ref.
 
 Elsewhere, we discussed delaying ref advertisement (aka expand
 refs), but it is an orthogonal feature and this hiding refs
 completely from advertisement series does not attempt to address.
 
 Patch #2 (simplify request validation), #4 (clarify the codeflow),
 and #5 (use struct ref) are new.  The are all long overdue clean-ups
 for these codepaths.
 
 The last patch is an illustration why it wouldn't make sense to
 optionally allow pushing into hidden refs, and not meant to be part
 of the series proper.
 
 For those who missed it, earlier rounds are at:
 
 http://thread.gmane.org/gmane.comp.version-control.git/213951
 http://thread.gmane.org/gmane.comp.version-control.git/214888

I would again like to express my discomfort about this feature, which is
already listed as will merge to next.  Frankly, I have the feeling
that this feature is being steamrolled in before a community consensus
has been reached and indeed before many valid points raised by other
members of the community have even been addressed.  For example:

* I didn't see a response to Peff's convincing arguments that this
should be a client-side feature rather than a server-side feature [1].

* I didn't see an answer to Duy's question [2] about what is different
between the proposed feature and gitnamespaces.

* I didn't see a response to my worries that this feature could be
abused [3].

I also think that the feature is poorly designed.  For example:

* Why should a repository have exactly one setting for what refs should
be hidden?  Wouldn't it make more sense to allow multiple views to be
defined?:

[view official]
hiderefs = refs/pull
hiderefs = refs/heads/??/*
[view pu]
hiderefs = refs/pull
[view current]
hiderefs = refs/tags/releases

with the view perhaps selected via a server-side environment variable?
This would allow multiple views to be published via different URLs but
referring to the same git repository.

* Is it enough to support only reference exclusion (as opposed to
exclusion and inclusion rules)?  Is it enough to support only reference
selection by hierarchy (for example, how would you hide contributed
branches from your repo)?  Can your configuration scheme be expanded in
a backwards-compatible way if these or other extensions are added later?

* Why should this feature only be available remotely?  It would be handy
to clone everything but usually only see some subset of references in my
daily work: GIT_VIEW=official gitk --all .  Or to hide some remote
branches most of the time without having to remove them from my repo:

[view brief]
refs = refs
refs = !refs/remotes
refs = refs/remotes/origin
refs = refs/remotes/my-boss

I think there are still more questions than answers about this feature
and FWIW vote -1 on merging it to next at this time.

Michael

[1] http://article.gmane.org/gmane.comp.version-control.git/214168
[2] http://article.gmane.org/gmane.comp.version-control.git/214070
[3] http://article.gmane.org/gmane.comp.version-control.git/213957

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 0/8] Hiding refs

2013-02-05 Thread Jonathan Nieder
Hi Michael,

Michael Haggerty wrote:

 I would again like to express my discomfort about this feature, which is
 already listed as will merge to next.  Frankly, I have the feeling
 that this feature is being steamrolled in before a community consensus
 has been reached and indeed before many valid points raised by other
 members of the community have even been addressed.  For example:

In $dayjob I work with Gerrit, so I think I can start to answer some
of these questions.

 * I didn't see a response to Peff's convincing arguments that this
 should be a client-side feature rather than a server-side feature [1].

The client can't control the size of the ref advertisement.  That is
the main motivation if I understood correctly.

 * I didn't see an answer to Duy's question [2] about what is different
 between the proposed feature and gitnamespaces.

Namespaces are more complicated and don't sit well in existing setups
involving git repositories whose refs are not namespaced.

 * I didn't see a response to my worries that this feature could be
 abused [3].

Can you elaborate?  Do you mean that through social engineering an
attacker would convince the server admin to store secrets using a
hidden ref and enable the upload-archive service?

That does sound like a reasonable concern.  Perhaps the documentation
should be updated along these lines

transfer.hiderefs::
String(s) `upload-pack` and `receive-pack` use to decide
which refs to omit from their initial advertisement.  Use
more than one transfer.hiderefs configuration variables to
specify multiple prefix strings. A ref that are under the
hierarchies listed on the value of this variable is excluded,
and is hidden from `git ls-remote`, `git fetch`, `git push :`,
etc.  An attempt to update or delete a hidden ref by `git push`
is rejected, and an attempt to fetch a hidden ref by `git fetch`
will fail.
+
This setting does not currently affect the `upload-archive` service.

until someone interested implements the same for upload-archive.

 I also think that the feature is poorly designed.  For example:

That's another reasonable concern.  It's very hard to get a design
correct right away, which is presumably part of the motivation of
getting this into the hands of interested users who can give feedback
on it.  What would potentially be worth blocking even that is concerns
about the wire protocol, since it is hard to take back mistakes there.

 * Why should a repository have exactly one setting for what refs should
 be hidden?  Wouldn't it make more sense to allow multiple views to be
 defined?:

How do I request a different view of the repository at
/path/to/repo.git over the network?  How can we make the common case
of only one view easy to achieve?  Isn't the multiple-views case
exactly what gitnamespaces is for?

[...]
 * Is it enough to support only reference exclusion (as opposed to
 exclusion and inclusion rules)?

The motivating example is turning off advertisement of the
refs/changes hierarchy.  If and when more complicated cases come up,
that would presumably be the time to support more complicated
configuration.

[...]
 * Why should this feature only be available remotely?

It is about transport.  Ref namespaces have their own set of use cases
and are a distinct feature.

Hoping that clarifies,
Jonathan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [WIP/RFH/RFD/PATCH] grep: allow to use textconv filters

2013-02-05 Thread Michael J Gruber
Junio C Hamano venit, vidit, dixit 04.02.2013 18:12:
 Michael J Gruber g...@drmicha.warpmail.net writes:
 
 Recently and not so recently, we made sure that log/grep type operations
 use textconv filters when a userfacing diff would do the same:

 ef90ab6 (pickaxe: use textconv for -S counting, 2012-10-28)
 b1c2f57 (diff_grep: use textconv buffers for add/deleted files, 2012-10-28)
 0508fe5 (combine-diff: respect textconv attributes, 2011-05-23)

 git grep currently does not use textconv filters at all, that is
 neither for displaying the match and context nor for the actual grepping.

 Introduce a binary mode --textconv (in addition to --text and -I)
 which makes git grep use any configured textconv filters for grepping
 and output purposes.

 Signed-off-by: Michael J Gruber g...@drmicha.warpmail.net
 ---

 Notes:
 I'm somehow stuck in textconv/filespec/... hell, so I'm sending this out
 in request for help. I'm sure there are people for whom it's a breeze to
 get this right.
 
 Looks like the patch touches the right places in the codepaths that
 need to be updated from a quick read.

I should have added that this coredumps when used on blobs and that
textconv isn't invoked in any case (because it crashes on blobs, and I
haven't implemented tetxconv on worktree files yet).

I'm somehow struggling to use just the right bits from the more diff
centric helpers like fill_textconv or fill_one (which is static so far).

 The difficulty is in getting the different cases (blob/sha1 vs.
 worktree) right, and in making the changes minimally invasive.
 
 I think grep_source abstraction was intended to be a way for that,
 and if what it gives you is not sufficient for your needs, extending
 it should not be seen as invasive at all.

It seems to me that we textconvified the diff versions of these
abstractions, but not the grep_source abstractions. Doing it at the
source appears to be the right thing.

Michael
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 3/8] upload/receive-pack: allow hiding ref hierarchies

2013-02-05 Thread Jeff King
On Wed, Jan 30, 2013 at 10:45:37AM -0800, Junio C Hamano wrote:

 Teach upload-pack and receive-pack to omit some refs from their
 initial advertisements by paying attention to the transfer.hiderefs
 multi-valued configuration variable.  Any ref that is under the
 hierarchies listed on the value of this variable is excluded from
 responses to requests made by ls-remote, fetch, clone, push,
 etc.
 
 A typical use case may be
 
   [transfer]
   hiderefs = refs/pull
 
 to hide the refs that are internally used by the hosting site and
 should not be exposed over the network.

In the earlier review, I mentioned making this per-service, but I see
that is not the case here. Do you have an argument against doing so?

I'm specifically thinking of the way we do refs/pull at GitHub (which we
hide only from receive-pack).  I know that you think it would be cleaner
to hide those, and at some level I agree. But at the same time, the
current mechanism has been in place for some time; changing what we
present via upload-pack is likely to break people's workflows. And I
have not seen complaints about the current system. So unless there is a
compelling reason to do so, I'd rather let the fetcher make the
decision.

Gerrit's refs/changes may be a different story, if they have a large
enough number of them to make upload-pack's ref advertisement
overwhelming.

I'm happy to do the per-service patch on top, but I just expected it
here, so I'm wondering if you are against having the feature.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Feature request: Allow extracting revisions into directories

2013-02-05 Thread Sitaram Chamarty
On Mon, Feb 4, 2013 at 10:22 PM, Junio C Hamano gits...@pobox.com wrote:
 Tomas Carnecky tomas.carne...@gmail.com writes:

 That's what `git checkout` is for. And I would even argue that it's the 
 better
 choice in your situation because it would delete files from /var/www/foo 
 which
 you have deleted in your repo. git archive|tar wouldn't do that.

 The point about removal is an interesting one.  From that /var/www
 location I guess that you are discussing some webapp, but if you let
 it _write_ into it, you may also have to worry about how to handle
 the case where an update from the source end that comes from the
 checkout and an update by the webapp collide with each other.

 You also need to maintain the .git/index file that corresponds to
 what should be in /var/www/foo/ if you go that route.

 Just to be sure, I am not saying checkout is an inappropriate
 solution to whatever problem you are trying to solve. I am just
 pointing out things you need to be aware of if you take that
 approach.

http://sitaramc.github.com/the-list-and-irc/deploy.html is a fairly
popular URL on #git, where the bot responds to !deploy with some
text and this URL.

Just sayin'... :)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] Should log --cc imply log --cc -p?

2013-02-05 Thread Michael J Gruber
Junio C Hamano venit, vidit, dixit 04.02.2013 17:36:
 Michael J Gruber g...@drmicha.warpmail.net writes:
 
 But diffs are on here (-p), it's just that the default diff option for
 merges is to not display them. Well, I admit there's two different ways
 of thinking here:

 A) git log -p turns on diffs for all commits, and the default diff
 options is the (non-existing) --no-show diff-option for merges.

 B) git log -p applies -p to all commits except merge commits.

 I'm strongly in the A camp,...
 
 I can personally be trained to think either way, but I suspect that
 we already came halfway to
 
   C) -p asks for two-way patches, and -c/--cc ask for n-way
  patches (including but not limited to n==2); it is not that -p
  asks for patch generation and others modify the finer behaviour
  of patch generation.
 
 git log/diff-files -U8 do not need -p to enable textual patches,
 for example.  It is I already told you that I want 8-line context.

That's a good point that I wasn't aware of.

 For what else, other than showing textual diff, do you think I told
 you that for? and replacing 8-line context with various other
 options that affect patch generation will give us a variety of end
 user complaints that would tell us that C) is more intuitive to
 them.
 
 But I do not feel very strongly that I am *right* on this issue, so
 I won't do anything about it hastily right now.

I'm not sure how many of these we have already, but to me it indicates
that we should turn diffs on for log with any diff option that only
makes sense with a diff. That would make the ui more consistent without
taking anything away.

For scripting/aliasing purposes, we have -s in order to override any
implied -p (as I just learned).

Michael
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Assorted contrib/subtree Patches

2013-02-05 Thread Jakub Suder
Can I please unsubscribe from this thing?...

Thanks.

Jakub Suder

On 5 February 2013 05:06, David A. Greene gree...@obbligato.org wrote:
 All of the patches I have received from others as well as a few of my
 own follow.  Probably the most controversial is a patch to remove
 --annotate.  After some discussion on the list it became clear that we
 really want a more general commit rewrite feature.  Removing
 --annotate means we don't have to also support --unannotate and carry
 both forward as backward-compatibility baggage.

 Before --annotate was added, git-subtree would force an annotation of
 * on every split commit message.  It now does no such thing so
 there's no need to unannotate anything.

 Please review and integrate.  Thanks!

 -David

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 7/8] fetch: fetch objects by their exact SHA-1 object names

2013-02-05 Thread Jeff King
On Wed, Jan 30, 2013 at 10:45:41AM -0800, Junio C Hamano wrote:

 Teach git fetch to accept an exact SHA-1 object name the user may
 obtain out of band on the LHS of a pathspec, and send it on a want
 message when the server side advertises the allow-tip-sha1-in-want
 capability.

Hmm. The UI on this is a little less nice than I would have hoped. Right
now if you want a ref outside of refs/heads, it's up to you to configure
a refspec or do a one-off fetch of the ref:

  git config --add remote.origin.fetch '+refs/pull/*:refs/pull/*'
  git fetch
  git checkout refs/pull/123/head
  ... inspect the contents ...

Without advertisement, we have to learn that refs/pull/123/head exists
out of band. We can no longer fetch all of the refs/pull hierarchy
preemptively, but we can in theory grab at least that one ref like this:

  git fetch refs/pull/123/head
  git checkout FETCH_HEAD
  ... inspect the contents ...

But that does not work with your patch; instead you have to learn not
just the existence of the ref, but also its sha1. This may seem like a
little thing, since you are already learning of the ref out-of-band,
but:

  1. The full sha1 is more annoying to work with. You'd have to cut and
 paste or otherwise script getting it to fetch.  A human-readable
 ref, though, is much easier to remember. The refs/pull/N/head
 pattern is simple to learn and type.

  2. Related to (1) above, is that it may be easier to come up with a
 hidden ref name out of band than the full sha1. E.g., if I am
 looking at https://github.com/me/foo.git/pulls/123, I can easily
 construct the ref from that. Getting the sha1 will take extra
 steps.

  3. You have to do the out-of-band step, which may be inconvenient,
 every time the ref is updated. There is no way to say just give me
 what is at the tip of refs/pull/123/head.

I think you could solve it by teaching upload-pack to understand refs on
want lines and convert them into the pointed-to object.

But taking a step back, this really seems quite inferior to an extension
that would allow the client to share its refspecs with the server. That
would solve the bandwidth efficiency problem for normal fetchers who are
looking at refs/heads/*, while still giving people who are interested
in refs/pull/* (or even a specific refs/pull tip) the information they
need to fetch.

The obvious problem is that the server speaks first. But I recall
somebody suggested a combination of:

  1. For git-over-ssh and git-over-tcp, the server advertises
 tell-me-your-refspecs as it starts advertising.  Client interrupts
 advertisement with refspecs once it sees that it is OK to do so.

 We waste some bandwidth during the round-trip, but there will still
 be a benefit for repos with many refs (I wonder if we could even
 re-order the advertisement to show refs/heads/ first, as they are
 the most likely case to be requested). And as time goes on and the
 majority of clients support tell-me-your-refspecs, the server side
 can introduce a short delay after the first advertisement.

  2. For git-over-http, the client speaks first via the http protocol.
 We can stuff the refspecs into extra query parameters.

It's a little more complicated as a solution, but I feel like it gets
the efficiency without a loss of functionality. And it helps in more
situations than the hidden refs proposal (e.g., fetching refs/heads/foo
can avoid enumerating all of refs/heads/*).

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] Should log --cc imply log --cc -p?

2013-02-05 Thread Jeff King
On Mon, Feb 04, 2013 at 08:36:43AM -0800, Junio C Hamano wrote:

 git log/diff-files -U8 do not need -p to enable textual patches,
 for example.  It is I already told you that I want 8-line context.
 For what else, other than showing textual diff, do you think I told
 you that for? and replacing 8-line context with various other
 options that affect patch generation will give us a variety of end
 user complaints that would tell us that C) is more intuitive to
 them.

Yeah, I'd agree with this. My feeling is that when there are two
options, A and B, and A is a no-op if B is not also specified, that it
makes sense for A to imply B. We do it in several places already (and I
just added some for git branch --list recently).

Is --cc a no-op when -p is not specified? Certainly -c is not, but
I do not think you are proposing that. At first glance, --cc is
nonsensical without -p, but what about other xdiff callers? For
example, in:

  git log --cc --stat

the --cc is significant. So I don't think it is right for --cc to
always imply -p. But if the rule kicked in only when no other format
had been specified, that might make sense.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] Should log --cc imply log --cc -p?

2013-02-05 Thread Ævar Arnfjörð Bjarmason
On Mon, Feb 4, 2013 at 5:36 PM, Junio C Hamano gits...@pobox.com wrote:
 git log/diff-files -U8 do not need -p to enable textual patches,
 for example.  It is I already told you that I want 8-line context.
 For what else, other than showing textual diff, do you think I told
 you that for? and replacing 8-line context with various other
 options that affect patch generation will give us a variety of end
 user complaints that would tell us that C) is more intuitive to
 them.

On a related note I think --full-diff should imply -p too.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 0/8] Hiding refs

2013-02-05 Thread Michael Haggerty
On 02/05/2013 09:33 AM, Jonathan Nieder wrote:
 Michael Haggerty wrote:
 
 I would again like to express my discomfort about this feature, which is
 already listed as will merge to next.  Frankly, I have the feeling
 that this feature is being steamrolled in before a community consensus
 has been reached and indeed before many valid points raised by other
 members of the community have even been addressed.  For example:
 
 In $dayjob I work with Gerrit, so I think I can start to answer some
 of these questions.
 
 * I didn't see a response to Peff's convincing arguments that this
 should be a client-side feature rather than a server-side feature [1].
 
 The client can't control the size of the ref advertisement.  That is
 the main motivation if I understood correctly.

Not according to Junio [4]:

  Look at this as a mechanism for the repository owner to control the
  clutter in what is shown to the intended audience of what s/he
  publishes in the repository.  Network bandwidth reduction of
  advertisement is a side effect of clutter reduction, and not
  necessarily the primary goal.

 * I didn't see an answer to Duy's question [2] about what is different
 between the proposed feature and gitnamespaces.
 
 Namespaces are more complicated and don't sit well in existing setups
 involving git repositories whose refs are not namespaced.

Thanks.

 * I didn't see a response to my worries that this feature could be
 abused [3].
 
 Can you elaborate?  Do you mean that through social engineering an
 attacker would convince the server admin to store secrets using a
 hidden ref and enable the upload-archive service?

The upload-archive service is not needed; after patch v3 7/8 remote
clients are able to retrieve hidden content by SHA1.

Hiderefs creates a dark corner of a remote git repo that can hold
arbitrary content that is impossible for anybody to discover but
nevertheless possible for anybody to download (if they know the name of
a hidden reference).  In earlier versions of the patch series I believe
that it was possible to push to a hidden reference hierarchy, which made
it possible to upload dark content.  The new version appears (from the
code) to prohibit adding references in a hidden hierarchy, which would
close the main loophole that I was worried about.  But the documentation
and the unit tests only explicitly say that updates and deletes are
prohibited; nothing is said about adding references (unless update is
understood to include add).  I think the true behavior should be
clarified and tested.

I was worried that somehow this dark content could be used for
malicious purposes; for example, pushing compromised code then
convincing somebody to download it by SHA1 with the implicit argument
it's safe since it comes directly from the project's official
repository.  If it is indeed impossible to populate the dark namespace
remotely then I can't think of a way to exploit it.

 That does sound like a reasonable concern.  Perhaps the documentation
 should be updated along these lines
 
   transfer.hiderefs::
   String(s) `upload-pack` and `receive-pack` use to decide
   which refs to omit from their initial advertisement.  Use
   more than one transfer.hiderefs configuration variables to
   specify multiple prefix strings. A ref that are under the
   hierarchies listed on the value of this variable is excluded,
   and is hidden from `git ls-remote`, `git fetch`, `git push :`,
   etc.  An attempt to update or delete a hidden ref by `git push`
   is rejected, and an attempt to fetch a hidden ref by `git fetch`
   will fail.
   +
   This setting does not currently affect the `upload-archive` service.
 
 until someone interested implements the same for upload-archive.

Yes, this sounds reasonable.

 I also think that the feature is poorly designed.  For example:
 
 That's another reasonable concern.  It's very hard to get a design
 correct right away, which is presumably part of the motivation of
 getting this into the hands of interested users who can give feedback
 on it.  What would potentially be worth blocking even that is concerns
 about the wire protocol, since it is hard to take back mistakes there.
 
 * Why should a repository have exactly one setting for what refs should
 be hidden?  Wouldn't it make more sense to allow multiple views to be
 defined?:
 
 How do I request a different view of the repository at
 /path/to/repo.git over the network?  How can we make the common case
 of only one view easy to achieve?  Isn't the multiple-views case
 exactly what gitnamespaces is for?

Hidden references can only be configured by somebody with local access
to the repository being served.  Somebody with that access could also
configure views.  He could also probably organize a mapping from URL -
(GIT_PATH, GIT_VIEW) and offer several different views of the same
repository.  If the setting of 

will git provide `submodule remove` option ?‏

2013-02-05 Thread Lingcha X
As we all know, git provides `submodule add , init, update, sync, sumary, 
foreach, status`, but where is `submodule remove`?

will
 I not delete one of them sometime in the future? Although most people 
will not use submodule or one who uses it can remove submodule by hand, 
providing complete options may be a good idea.

I am not familiar with C, but i can help test it. will anyone finish this 
feature?


Thanks and Best Regards,
lingchax  

Re: [WIP/RFH/RFD/PATCH] grep: allow to use textconv filters

2013-02-05 Thread Jeff King
On Mon, Feb 04, 2013 at 04:27:31PM +0100, Michael J Gruber wrote:

 Recently and not so recently, we made sure that log/grep type operations
 use textconv filters when a userfacing diff would do the same:
 
 ef90ab6 (pickaxe: use textconv for -S counting, 2012-10-28)
 b1c2f57 (diff_grep: use textconv buffers for add/deleted files, 2012-10-28)
 0508fe5 (combine-diff: respect textconv attributes, 2011-05-23)
 
 git grep currently does not use textconv filters at all, that is
 neither for displaying the match and context nor for the actual grepping.
 
 Introduce a binary mode --textconv (in addition to --text and -I)
 which makes git grep use any configured textconv filters for grepping
 and output purposes.

Sounds like a reasonable goal.

 The difficulty is in getting the different cases (blob/sha1 vs.
 worktree) right, and in making the changes minimally invasive. It seems
 that some more refactoring could help: git show --textconv does not
 use textconv filters when used on blobs either. (It does for diffs, of
 course.) Most existing helper functions are tailored for diffs.

I think git show with blobs originally did not because we have no
filename with which to look up the attributes. IIRC, the patches to
support cat-file --textconv taught get_sha1_with_context to report the
path at which we found a blob. I suspect it is mostly a matter of
plumbing that information from the revision parser through to
show_blob_object.

 Nota bene: --textconv does not affect diff --stat either...

Yeah, though I wonder if it should be on by default. The diffstat for a
binary file, unlike the diff, is already useful. The diffstat of the
textconv'd data may _also_ be useful, of course.

 @@ -659,6 +659,9 @@ int cmd_grep(int argc, const char **argv, const char 
 *prefix)
   OPT_SET_INT('I', NULL, opt.binary,
   N_(don't match patterns in binary files),
   GREP_BINARY_NOMATCH),
 + OPT_SET_INT(0, textconv, opt.binary,
 + N_(process binary files with textconv filters),
 + GREP_BINARY_TEXTCONV),

Is this really a new form of GREP_BINARY_*? What happens when a file
does not have a textconv filter?

I would expect this to be more like diff's --textconv flag, which is
really allow textconv to be respected. Then you could do:

  git grep -I --textconv foo

to grep in the text version of files which support it, and ignore the
rest.

 -static int grep_source_load(struct grep_source *gs);
 -static int grep_source_is_binary(struct grep_source *gs);
 +static int grep_source_load(struct grep_source *gs, struct grep_opt *opt);
 +static int grep_source_is_binary(struct grep_source *gs, struct grep_opt 
 *opt);

Hmm. grep_source_load is more or less the analogue of
diff_populate_filespec, which does not know about textconv at all. So I
feel like this might be going in at the wrong layer...

 @@ -1354,14 +1356,15 @@ static int grep_source_1(struct grep_opt *opt, struct 
 grep_source *gs, int colle
  
   switch (opt-binary) {
   case GREP_BINARY_DEFAULT:
 - if (grep_source_is_binary(gs))
 + if (grep_source_is_binary(gs, opt))
   binary_match_only = 1;
   break;
   case GREP_BINARY_NOMATCH:
 - if (grep_source_is_binary(gs))
 + if (grep_source_is_binary(gs, opt))
   return 0; /* Assume unmatch */
   break;

The is_binary function learned about opt so that it could pass it to
grep_source_load, which might do the textconv for us. But we _know_ that
we will not here, because we see that we have other GREP_BINARY flags.

And when we do have _TEXTCONV:

   case GREP_BINARY_TEXT:
 + case GREP_BINARY_TEXTCONV:
   break;

We do not call is_binary. :)

 -static int grep_source_load_sha1(struct grep_source *gs)
 +static int grep_source_load_sha1(struct grep_source *gs, struct grep_opt 
 *opt)
  {
   enum object_type type;
 -
   grep_read_lock();
 - gs-buf = read_sha1_file(gs-identifier, type, gs-size);
 + if (opt-binary == GREP_BINARY_TEXTCONV) {
 + struct diff_filespec *df = alloc_filespec(gs-name);
 + gs-size = fill_textconv(gs-driver, df, gs-buf);
 + free_filespec(df);
 + } else {
 + gs-buf = read_sha1_file(gs-identifier, type, gs-size);
 + }

So here we do the textconv for the sha1 case. But what about file
sources?

This is why I think the layer is wrong; you want the fill_textconv
function to call your abstract _load function (in the diff_filespec
world, it is diff_filespec_populate, but it is the moral equivalent).
And you want it to hold off as long as possible in case we can pull the
value from cache, or feed the working tree version of a file straight to
the filter.

 -void grep_source_load_driver(struct grep_source *gs)
 +void grep_source_load_driver(struct grep_source *gs, struct grep_opt *opt)
  {
   

Re: Is anyone working on a next-gen Git protocol (Re: [PATCH v3 0/8] Hiding refs)

2013-02-05 Thread Ævar Arnfjörð Bjarmason
On Wed, Jan 30, 2013 at 7:45 PM, Junio C Hamano gits...@pobox.com wrote:
 The third round.

  - Multi-valued variable transfer.hiderefs lists prefixes of ref
hierarchies to be hidden from the requests coming over the
network.

  - A configuration optionally allows uploadpack to accept fetch
requests for an object at the tip of a hidden ref.

 Elsewhere, we discussed delaying ref advertisement (aka expand
 refs), but it is an orthogonal feature and this hiding refs
 completely from advertisement series does not attempt to address.

I'm a bit late to this so sorry if this has been covered before.

In the initial draft of this series the rationale for it was reducing
the network cost while talking with a repository with tons of
refs[1]. But later you seem to have changed your mind, and network
bandwidth reduction of advertisement is a side effect of clutter
reduction, and not necessarily the primary goal.

Do you have any plans for something that *does* have the reduction of
network bandwidth as a primary goal?

In October I asked if anyone was working on a next-gen Git protocol[3]
that would provide clients with the ability to specify what refs they
wanted. You replied to me off-list saying Yes.

Is this what you've been working on? Because if so I misunderstood you
thinking you were going to work on something that gave clients the
ability specify what they wanted before the initial ref advertisement.

I'm still very keen to have that ability, so if you're not working on
it I just might give it a go.

1. http://article.gmane.org/gmane.comp.version-control.git/213951
2. http://article.gmane.org/gmane.comp.version-control.git/213984
3. http://article.gmane.org/gmane.comp.version-control.git/214025
4. http://thread.gmane.org/gmane.comp.version-control.git/207190
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 7/8] fetch: fetch objects by their exact SHA-1 object names

2013-02-05 Thread Jeff King
On Tue, Feb 05, 2013 at 04:19:38AM -0500, Jeff King wrote:

 But taking a step back, this really seems quite inferior to an
 extension that would allow the client to share its refspecs with the
 server.  That would solve the bandwidth efficiency problem for normal
 fetchers who are

I should have read your cover letter more closely, as I see you make the
point there that this is no longer about the efficiency, but about
uncluttering.

I'm not sure I like it as an uncluttering tool, though; for the reasons
I stated in my previous mail, it makes it much more awkward for the
moments when you actually do want to fetch some of the clutter.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] Should log --cc imply log --cc -p?

2013-02-05 Thread Jeff King
On Tue, Feb 05, 2013 at 11:16:52AM +0100, Ævar Arnfjörð Bjarmason wrote:

 On Mon, Feb 4, 2013 at 5:36 PM, Junio C Hamano gits...@pobox.com wrote:
  git log/diff-files -U8 do not need -p to enable textual patches,
  for example.  It is I already told you that I want 8-line context.
  For what else, other than showing textual diff, do you think I told
  you that for? and replacing 8-line context with various other
  options that affect patch generation will give us a variety of end
  user complaints that would tell us that C) is more intuitive to
  them.
 
 On a related note I think --full-diff should imply -p too.

I don't think that is in the same class. --full-diff is quite useful for
many other diff formats. E.g.:

  git log --full-diff --raw Makefile

If you are proposing to default to -p when --full-diff is used but
no format is given, that is a slightly different thing. The --full-diff
in such a case is indeed useless, but I do not think it necessarily
follows that -p was what the user wanted.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Rebased git-subtree changes

2013-02-05 Thread David A. Greene
Here's a re-send of the git-subtree patches after rebasing onto
master.  Hopefully Junio will have better luck applying these.

-David

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 02/13] contrib/subtree: Use %B for Split Subject/Body

2013-02-05 Thread David A. Greene
From: Techlive Zheng techlivezh...@gmail.com

Use %B to format the commit message and body to avoid an extra newline
if a commit only has a subject line.

Signed-off-by: Techlive Zheng techlivezh...@gmail.com
Signed-off-by: David A. Greene gree...@obbligato.org
---
 contrib/subtree/git-subtree.sh |2 +-
 contrib/subtree/t/t7900-subtree.sh |   15 +++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 920c664..5598210 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -296,7 +296,7 @@ copy_commit()
# We're going to set some environment vars here, so
# do it in a subshell to get rid of them safely later
debug copy_commit {$1} {$2} {$3}
-   git log -1 --pretty=format:'%an%n%ae%n%ad%n%cn%n%ce%n%cd%n%s%n%n%b' 
$1 |
+   git log -1 --pretty=format:'%an%n%ae%n%ad%n%cn%n%ce%n%cd%n%B' $1 |
(
read GIT_AUTHOR_NAME
read GIT_AUTHOR_EMAIL
diff --git a/contrib/subtree/t/t7900-subtree.sh 
b/contrib/subtree/t/t7900-subtree.sh
index 6cf9fb9..3f17f55 100755
--- a/contrib/subtree/t/t7900-subtree.sh
+++ b/contrib/subtree/t/t7900-subtree.sh
@@ -74,6 +74,10 @@ test_expect_success 'add sub1' '
 git branch -m master subproj
 '
 
+# Save this hash for testing later.
+
+subdir_hash=`git rev-parse HEAD`
+
 test_expect_success 'add sub2' '
 create sub2 
 git commit -m sub2 
@@ -211,6 +215,17 @@ test_expect_success 'check split with --branch' '
 check_equal ''$(git rev-parse splitbr1)'' $spl1
 '
 
+test_expect_success 'check hash of split' '
+spl1=$(git subtree split --prefix subdir) 
+undo 
+git subtree split --prefix subdir --branch splitbr1test 
+check_equal ''$(git rev-parse splitbr1test)'' $spl1
+git checkout splitbr1test 
+new_hash=$(git rev-parse HEAD~2) 
+git checkout mainline 
+check_equal ''$new_hash'' $subdir_hash
+'
+
 test_expect_success 'check split with --branch for an existing branch' '
 spl1=''$(git subtree split --annotate=''*'' --prefix subdir --onto 
FETCH_HEAD --message Split  rejoin --rejoin)'' 
 undo 
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 03/13] contrib/subtree: Better Error Handling for add

2013-02-05 Thread David A. Greene
From: David A. Greene gree...@obbligato.org

Check refspecs for validity before passing them on to other commands.
This lets us generate more helpful error messages.

Signed-off-by: David A. Greene gree...@obbligato.org
---
 contrib/subtree/git-subtree.sh |   12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 5598210..771f39d 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -497,12 +497,18 @@ cmd_add()
ensure_clean

if [ $# -eq 1 ]; then
-   cmd_add_commit $@
+   git rev-parse -q --verify $1^{commit} /dev/null ||
+   die '$1' does not refer to a commit
+
+   cmd_add_commit $@
elif [ $# -eq 2 ]; then
-   cmd_add_repository $@
+   git rev-parse -q --verify $2^{commit} /dev/null ||
+   die '$2' does not refer to a commit
+
+   cmd_add_repository $@
else
say error: parameters were '$@'
-   die Provide either a refspec or a repository and refspec.
+   die Provide either a commit or a repository and commit.
fi
 }
 
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 04/13] contrib/subtree: Fix Synopsis

2013-02-05 Thread David A. Greene
From: David A. Greene gree...@obbligato.org

Fix the documentation of add to show that a repository can be
specified along with a commit.

Suggested by Yann Dirson dir...@bertin.fr.

Signed-off-by: David A. Greene gree...@obbligato.org
---
 contrib/subtree/git-subtree.sh  |6 ++
 contrib/subtree/git-subtree.txt |3 ++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 771f39d..8a23f58 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -9,6 +9,7 @@ if [ $# -eq 0 ]; then
 fi
 OPTS_SPEC=\
 git subtree add   --prefix=prefix commit
+git subtree add   --prefix=prefix repository commit
 git subtree merge --prefix=prefix commit
 git subtree pull  --prefix=prefix repository refspec...
 git subtree push  --prefix=prefix repository refspec...
@@ -502,6 +503,11 @@ cmd_add()
 
cmd_add_commit $@
elif [ $# -eq 2 ]; then
+   # Technically we could accept a refspec here but we're
+   # just going to turn around and add FETCH_HEAD under the
+   # specified directory.  Allowing a refspec might be
+   # misleading because we won't do anything with any other
+   # branches fetched via the refspec.
git rev-parse -q --verify $2^{commit} /dev/null ||
die '$2' does not refer to a commit
 
diff --git a/contrib/subtree/git-subtree.txt b/contrib/subtree/git-subtree.txt
index c5bce41..7ba853e 100644
--- a/contrib/subtree/git-subtree.txt
+++ b/contrib/subtree/git-subtree.txt
@@ -9,7 +9,8 @@ git-subtree - Merge subtrees together and split repository into 
subtrees
 SYNOPSIS
 
 [verse]
-'git subtree' add   -P prefix commit
+'git subtree' add   -P prefix refspec
+'git subtree' add   -P prefix repository refspec
 'git subtree' pull  -P prefix repository refspec...
 'git subtree' push  -P prefix repository refspec...
 'git subtree' merge -P prefix commit
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 06/13] contrib/subtree: Make the Manual Directory if Needed

2013-02-05 Thread David A. Greene
From: Jesper L. Nielsen lya...@gmail.com

Before install git-subtree documentation, make sure the manpage
directory exists.

Signed-off-by: Jesper L. Nielsen lya...@gmail.com
Signed-off-by: David A. Greene gree...@obbligato.org
---
 contrib/subtree/Makefile |1 +
 1 file changed, 1 insertion(+)

diff --git a/contrib/subtree/Makefile b/contrib/subtree/Makefile
index 36ae3e4..b507505 100644
--- a/contrib/subtree/Makefile
+++ b/contrib/subtree/Makefile
@@ -35,6 +35,7 @@ install: $(GIT_SUBTREE)
 install-doc: install-man
 
 install-man: $(GIT_SUBTREE_DOC)
+   $(INSTALL) -d -m 755 $(DESTDIR)$(man1dir)
$(INSTALL) -m 644 $^ $(DESTDIR)$(man1dir)
 
 $(GIT_SUBTREE_DOC): $(GIT_SUBTREE_XML)
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 09/13] contrib/subtree: Ignore testing directory

2013-02-05 Thread David A. Greene
From: Techlive Zheng techlivezh...@gmail.com

Signed-off-by: Techlive Zheng techlivezh...@gmail.com
Signed-off-by: David A. Greene gree...@obbligato.org
---
 contrib/subtree/.gitignore |5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/contrib/subtree/.gitignore b/contrib/subtree/.gitignore
index 91360a3..59aeeb4 100644
--- a/contrib/subtree/.gitignore
+++ b/contrib/subtree/.gitignore
@@ -1,6 +1,5 @@
 *~
 git-subtree
-git-subtree.xml
 git-subtree.1
-mainline
-subproj
+git-subtree.xml
+t/trash\ directory.*
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 08/13] contrib/subtree: Add vim modeline

2013-02-05 Thread David A. Greene
From: Techlive Zheng techlivezh...@gmail.com

Signed-off-by: Techlive Zheng techlivezh...@gmail.com
Signed-off-by: David A. Greene gree...@obbligato.org
---
 contrib/subtree/git-subtree.sh |2 ++
 contrib/subtree/t/t7900-subtree.sh |2 ++
 2 files changed, 4 insertions(+)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 6c3929b..c72af95 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -722,3 +722,5 @@ cmd_push()
 }
 
 cmd_$command $@
+
+# vim: set ts=4 sw=4 noet
diff --git a/contrib/subtree/t/t7900-subtree.sh 
b/contrib/subtree/t/t7900-subtree.sh
index e6a3702..e6bcd50 100755
--- a/contrib/subtree/t/t7900-subtree.sh
+++ b/contrib/subtree/t/t7900-subtree.sh
@@ -466,3 +466,5 @@ test_expect_success 'verify one file change per commit' '
 '
 
 test_done
+
+# vim: set ts=4 sw=4 noet
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 07/13] contrib/subtree: Fix whitespaces

2013-02-05 Thread David A. Greene
From: Techlive Zheng techlivezh...@gmail.com

Previous code does not fulfill Git's whitespace policy.

Signed-off-by: Techlive Zheng techlivezh...@gmail.com
Signed-off-by: David A. Greene gree...@obbligato.org
---
 contrib/subtree/git-subtree.sh |   68 
 contrib/subtree/git-subtree.txt|   42 ++---
 contrib/subtree/t/t7900-subtree.sh |  314 ++--
 3 files changed, 212 insertions(+), 212 deletions(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 8a23f58..6c3929b 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -5,7 +5,7 @@
 # Copyright (C) 2009 Avery Pennarun apenw...@gmail.com
 #
 if [ $# -eq 0 ]; then
-set -- -h
+   set -- -h
 fi
 OPTS_SPEC=\
 git subtree add   --prefix=prefix commit
@@ -111,9 +111,9 @@ if [ -z $prefix ]; then
 fi
 
 case $command in
-   add) [ -e $prefix ]  
+   add) [ -e $prefix ] 
die prefix '$prefix' already exists. ;;
-   *)   [ -e $prefix ] || 
+   *)   [ -e $prefix ] ||
die '$prefix' does not exist; use 'git subtree add' ;;
 esac
 
@@ -182,8 +182,8 @@ cache_set()
oldrev=$1
newrev=$2
if [ $oldrev != latest_old \
--a $oldrev != latest_new \
--a -e $cachedir/$oldrev ]; then
+   -a $oldrev != latest_new \
+   -a -e $cachedir/$oldrev ]; then
die cache for $oldrev already exists!
fi
echo $newrev $cachedir/$oldrev
@@ -305,7 +305,7 @@ copy_commit()
read GIT_COMMITTER_NAME
read GIT_COMMITTER_EMAIL
read GIT_COMMITTER_DATE
-   export  GIT_AUTHOR_NAME \
+   export GIT_AUTHOR_NAME \
GIT_AUTHOR_EMAIL \
GIT_AUTHOR_DATE \
GIT_COMMITTER_NAME \
@@ -328,7 +328,7 @@ add_msg()
fi
cat -EOF
$commit_message
-   
+
git-subtree-dir: $dir
git-subtree-mainline: $latest_old
git-subtree-split: $latest_new
@@ -356,7 +356,7 @@ rejoin_msg()
fi
cat -EOF
$commit_message
-   
+
git-subtree-dir: $dir
git-subtree-mainline: $latest_old
git-subtree-split: $latest_new
@@ -369,7 +369,7 @@ squash_msg()
oldsub=$2
newsub=$3
newsub_short=$(git rev-parse --short $newsub)
-   
+
if [ -n $oldsub ]; then
oldsub_short=$(git rev-parse --short $oldsub)
echo Squashed '$dir/' changes from 
$oldsub_short..$newsub_short
@@ -379,7 +379,7 @@ squash_msg()
else
echo Squashed '$dir/' content from commit $newsub_short
fi
-   
+
echo
echo git-subtree-dir: $dir
echo git-subtree-split: $newsub
@@ -428,7 +428,7 @@ new_squash_commit()
newsub=$3
tree=$(toptree_for_commit $newsub) || exit $?
if [ -n $old ]; then
-   squash_msg $dir $oldsub $newsub | 
+   squash_msg $dir $oldsub $newsub |
git commit-tree $tree -p $old || exit $?
else
squash_msg $dir  $newsub |
@@ -456,7 +456,7 @@ copy_or_skip()
else
nonidentical=$parent
fi
-   
+
# sometimes both old parents map to the same newparent;
# eliminate duplicates
is_new=1
@@ -471,7 +471,7 @@ copy_or_skip()
p=$p -p $parent
fi
done
-   
+
if [ -n $identical ]; then
echo $identical
else
@@ -496,7 +496,7 @@ cmd_add()
fi
 
ensure_clean
-   
+
if [ $# -eq 1 ]; then
git rev-parse -q --verify $1^{commit} /dev/null ||
die '$1' does not refer to a commit
@@ -513,8 +513,8 @@ cmd_add()
 
cmd_add_repository $@
else
-   say error: parameters were '$@'
-   die Provide either a commit or a repository and commit.
+   say error: parameters were '$@'
+   die Provide either a commit or a repository and commit.
fi
 }
 
@@ -534,19 +534,19 @@ cmd_add_commit()
revs=$(git rev-parse $default --revs-only $@) || exit $?
set -- $revs
rev=$1
-   
+
debug Adding $dir as '$rev'...
git read-tree --prefix=$dir $rev || exit $?
git checkout -- $dir || exit $?
tree=$(git write-tree) || exit $?
-   
+
headrev=$(git rev-parse HEAD) || exit $?
if [ -n $headrev -a $headrev != $rev ]; then
headp=-p $headrev
else
headp=
fi
-   
+
if [ -n $squash ]; then
rev=$(new_squash_commit   $rev) || exit $?
commit=$(add_squashed_msg $rev $dir |
@@ -556,7 +556,7 

[PATCH 05/13] contrib/subtree: Honor DESTDIR

2013-02-05 Thread David A. Greene
From: Adam Tkac at...@redhat.com

Teach git-subtree's Makefile to honor DESTDIR.

Signed-off-by: Adam Tkac at...@redhat.com
Signed-off-by: David A. Greene gree...@obbligato.org
---
 contrib/subtree/Makefile |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/contrib/subtree/Makefile b/contrib/subtree/Makefile
index 05cdd5c..36ae3e4 100644
--- a/contrib/subtree/Makefile
+++ b/contrib/subtree/Makefile
@@ -30,12 +30,12 @@ $(GIT_SUBTREE): $(GIT_SUBTREE_SH)
 doc: $(GIT_SUBTREE_DOC)
 
 install: $(GIT_SUBTREE)
-   $(INSTALL) -m 755 $(GIT_SUBTREE) $(libexecdir)
+   $(INSTALL) -m 755 $(GIT_SUBTREE) $(DESTDIR)$(libexecdir)
 
 install-doc: install-man
 
 install-man: $(GIT_SUBTREE_DOC)
-   $(INSTALL) -m 644 $^ $(man1dir)
+   $(INSTALL) -m 644 $^ $(DESTDIR)$(man1dir)
 
 $(GIT_SUBTREE_DOC): $(GIT_SUBTREE_XML)
xmlto -m $(MANPAGE_NORMAL_XSL)  man $^
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Rebased git-subtree changes

2013-02-05 Thread greened
David A. Greene gree...@obbligato.org writes:

 Here's a re-send of the git-subtree patches after rebasing onto
 master.  Hopefully Junio will have better luck applying these.

Damn.  Sorry, I sent this before getting all of the feedback.  Just
ignore this sequence and I'll send a fixed version in the future.

 -David
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 13/13] contrib/subtree: Remove --annotate

2013-02-05 Thread David A. Greene
From: David A. Greene gree...@obbligato.org

Remove --annotate.  This obviates the need for an --unannotate
command.  We really want a more generalized commit message rewrite
mechanism.

Signed-off-by: David A. Greene gree...@obbligato.org
---
 contrib/subtree/git-subtree.sh |6 +
 contrib/subtree/t/t7900-subtree.sh |   50 ++--
 2 files changed, 26 insertions(+), 30 deletions(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 0493e47..888a191 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -21,7 +21,6 @@ d show debug messages
 P,prefix= the name of the subdir to split out
 m,message=use the given message as the commit message for the merge commit
  options for 'split'
-annotate= add a prefix to commit message of new commits
 b,branch= create a new branch from the split subtree
 ignore-joins  ignore prior --rejoin commits
 onto= try connecting new tree to an existing one
@@ -43,7 +42,6 @@ command=
 onto=
 rejoin=
 ignore_joins=
-annotate=
 squash=
 message=
 
@@ -79,8 +77,6 @@ while [ $# -gt 0 ]; do
case $opt in
-q) quiet=1 ;;
-d) debug=1 ;;
-   --annotate) annotate=$1; shift ;;
-   --no-annotate) annotate= ;;
-b) branch=$1; shift ;;
-P) prefix=${1%/}; shift ;;
-m) message=$1; shift ;;
@@ -311,7 +307,7 @@ copy_commit()
GIT_COMMITTER_NAME \
GIT_COMMITTER_EMAIL \
GIT_COMMITTER_DATE
-   (echo -n $annotate; cat ) |
+   (echo -n ; cat ) |
git commit-tree $2 $3  # reads the rest of stdin
) || die Can't copy commit $1
 }
diff --git a/contrib/subtree/t/t7900-subtree.sh 
b/contrib/subtree/t/t7900-subtree.sh
index 1afd544..59889f1 100755
--- a/contrib/subtree/t/t7900-subtree.sh
+++ b/contrib/subtree/t/t7900-subtree.sh
@@ -318,8 +318,8 @@ test_expect_success 'split subdir/ with --rejoin' '
cd $test_count 
git fetch ./subproj master 
git subtree merge --prefix=subdir FETCH_HEAD 
-   split_hash=$(git subtree split --prefix=subdir --annotate=*) 

-   git subtree split --prefix=subdir --annotate=* --rejoin 
+   split_hash=$(git subtree split --prefix=subdir) 
+   git subtree split --prefix=subdir --rejoin 
test_equal $(last_commit_message) Split '\''subdir/'\'' into 
commit '\''$split_hash'\''
)
  '
@@ -342,7 +342,7 @@ test_expect_success 'split subdir/ with --rejoin and 
--message' '
cd $test_count 
git fetch ./subproj master 
git subtree merge --prefix=subdir FETCH_HEAD 
-   git subtree split --prefix=subdir --message=Split  rejoin 
--annotate=* --rejoin 
+   git subtree split --prefix=subdir --message=Split  rejoin 
--rejoin 
test_equal $(last_commit_message) Split  rejoin
)
 '
@@ -365,8 +365,8 @@ test_expect_success 'split subdir/ with --branch' '
cd $test_count 
git fetch ./subproj master 
git subtree merge --prefix=subdir FETCH_HEAD 
-   split_hash=$(git subtree split --prefix=subdir --annotate=*) 

-   git subtree split --prefix=subdir --annotate=* --branch 
subproj-br 
+   split_hash=$(git subtree split --prefix=subdir) 
+   git subtree split --prefix=subdir --branch subproj-br 
test_equal $(git rev-parse subproj-br) $split_hash
)
  '
@@ -390,8 +390,8 @@ test_expect_success 'split subdir/ with --branch for an 
existing branch' '
cd $test_count 
git fetch ./subproj master 
git subtree merge --prefix=subdir FETCH_HEAD 
-   split_hash=$(git subtree split --prefix=subdir --annotate=*) 

-   git subtree split --prefix=subdir --annotate=* --branch 
subproj-br 
+   split_hash=$(git subtree split --prefix=subdir) 
+   git subtree split --prefix=subdir --branch subproj-br 
test_equal $(git rev-parse subproj-br) $split_hash
)
 '
@@ -441,7 +441,7 @@ test_expect_success 'make sure exactly the right set of 
files ends up in the sub
cd $test_count 
git fetch ./subproj master 
git subtree merge --prefix=subdir FETCH_HEAD 
-   git subtree split --prefix=subdir --annotate=* --branch 
subproj-br --rejoin
+   git subtree split --prefix=subdir --branch subproj-br --rejoin
) 
test_create_commit $test_count/subproj sub3 
test_create_commit $test_count subdir/main-sub3 
@@ -452,12 +452,12 @@ test_expect_success 'make sure exactly the right set of 
files ends up in the sub
test_create_commit $test_count/subproj 

[PATCH 12/13] contrib/subtree: Handle '--prefix' argument with a slash appended

2013-02-05 Thread David A. Greene
From: Techlive Zheng techlivezh...@gmail.com

'git subtree merge' will fail if the argument of '--prefix' has a slash
appended.

Signed-off-by: Techlive Zheng techlivezh...@gmail.com
Signed-off-by: David A. Greene gree...@obbligato.org
---
 contrib/subtree/git-subtree.sh |2 +-
 contrib/subtree/t/t7900-subtree.sh |   19 +++
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index c72af95..0493e47 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -82,7 +82,7 @@ while [ $# -gt 0 ]; do
--annotate) annotate=$1; shift ;;
--no-annotate) annotate= ;;
-b) branch=$1; shift ;;
-   -P) prefix=$1; shift ;;
+   -P) prefix=${1%/}; shift ;;
-m) message=$1; shift ;;
--no-prefix) prefix= ;;
--onto) onto=$1; shift ;;
diff --git a/contrib/subtree/t/t7900-subtree.sh 
b/contrib/subtree/t/t7900-subtree.sh
index 769b116..1afd544 100755
--- a/contrib/subtree/t/t7900-subtree.sh
+++ b/contrib/subtree/t/t7900-subtree.sh
@@ -239,6 +239,25 @@ test_expect_success 'merge new subproj history into 
subdir/ with --squash and --
)
 '
 
+test_expect_success 'merge new subproj history into subdir/ with a slash 
appended to the argument of --prefix' '
+   test_create_repo $test_count 
+   test_create_repo $test_count/subproj 
+   test_create_commit $test_count main1 
+   test_create_commit $test_count/subproj sub1 
+   (
+   cd $test_count 
+   git fetch ./subproj master 
+   git subtree add --prefix=subdir/ FETCH_HEAD
+   ) 
+   test_create_commit $test_count/subproj sub2 
+   (
+   cd $test_count 
+   git fetch ./subproj master 
+   git subtree merge --prefix=subdir/ FETCH_HEAD 
+   test_equal $(last_commit_message) Merge commit '\''$(git 
rev-parse FETCH_HEAD)'\''
+   )
+'
+
 #
 # Tests for 'git subtree split'
 #
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 10/13] contrib/subtree: Code cleaning and refactoring

2013-02-05 Thread David A. Greene
From: Techlive Zheng techlivezh...@gmail.com

Mostly prepare for the later tests refactoring.

Signed-off-by: Techlive Zheng techlivezh...@gmail.com
Signed-off-by: David A. Greene gree...@obbligato.org
---
 contrib/subtree/t/t7900-subtree.sh |  270 ++--
 1 file changed, 136 insertions(+), 134 deletions(-)

diff --git a/contrib/subtree/t/t7900-subtree.sh 
b/contrib/subtree/t/t7900-subtree.sh
index e6bcd50..9cfaaf9 100755
--- a/contrib/subtree/t/t7900-subtree.sh
+++ b/contrib/subtree/t/t7900-subtree.sh
@@ -4,7 +4,7 @@
 #
 test_description='Basic porcelain support for subtrees
 
-This test verifies the basic operation of the merge, pull, add
+This test verifies the basic operation of the add, pull, merge
 and split subcommands of git subtree.
 '
 
@@ -18,19 +18,6 @@ create()
git add $1
 }
 
-
-check_equal()
-{
-   test_debug 'echo'
-   test_debug echo \check a:\ \{$1}\
-   test_debug echo \  b:\ \{$2}\
-   if [ $1 = $2 ]; then
-   return 0
-   else
-   return 1
-   fi
-}
-
 fixnl()
 {
t=
@@ -55,6 +42,42 @@ undo()
git reset --hard HEAD~
 }
 
+test_equal()
+{
+   test_debug 'echo'
+   test_debug echo \check a:\ \{$1}\
+   test_debug echo \  b:\ \{$2}\
+   if [ $1 = $2 ]; then
+   return 0
+   else
+   return 1
+   fi
+}
+
+# Make sure no patch changes more than one file.
+# The original set of commits changed only one file each.
+# A multi-file change would imply that we pruned commits
+# too aggressively.
+join_commits()
+{
+   commit=
+   all=
+   while read x y; do
+   if [ -z $x ]; then
+   continue
+   elif [ $x = commit: ]; then
+   if [ -n $commit ]; then
+   echo $commit $all
+   all=
+   fi
+   commit=$y
+   else
+   all=$all $y
+   fi
+   done
+   echo $commit $all
+}
+
 last_commit_message()
 {
git log --pretty=format:%s -1
@@ -97,7 +120,7 @@ test_expect_success 'add main4' '
create main4 
git commit -m main4 
git branch -m master mainline 
-   git branch subdir
+   git branch init
 '
 
 test_expect_success 'fetch subproj history' '
@@ -105,40 +128,43 @@ test_expect_success 'fetch subproj history' '
git branch sub1 FETCH_HEAD
 '
 
-test_expect_success 'no subtree exists in main tree' '
-   test_must_fail git subtree merge --prefix=subdir sub1
-'
-
 test_expect_success 'no pull from non-existant subtree' '
test_must_fail git subtree pull --prefix=subdir ./subproj sub1
 '
 
-test_expect_success 'check if --message works for add' '
-   git subtree add --prefix=subdir --message=Added subproject 
sub1 
-   check_equal ''$(last_commit_message)'' Added subproject 
+test_expect_success 'no merge from non-existant subtree' '
+   test_must_fail git subtree merge --prefix=subdir FETCH_HEAD
+'
+
+test_expect_success 'add subproj as subtree into subdir/ with --prefix' '
+   git subtree add --prefix=subdir FETCH_HEAD 
+   test_equal $(last_commit_message) Add '\''subdir/'\'' from 
commit '\''$(git rev-parse FETCH_HEAD)'\'' 
undo
 '
 
-test_expect_success 'check if --message works as -m and --prefix as -P' '
-   git subtree add -P subdir -m Added subproject using git 
subtree sub1 
-   check_equal ''$(last_commit_message)'' Added subproject 
using git subtree 
+test_expect_success 'add subproj as subtree into subdir/ with --prefix and 
--message' '
+   git subtree add --prefix=subdir --message=Added subproject 
FETCH_HEAD 
+   test_equal $(last_commit_message) Added subproject 
undo
 '
 
-test_expect_success 'check if --message works with squash too' '
-   git subtree add -P subdir -m Added subproject with squash 
--squash sub1 
-   check_equal ''$(last_commit_message)'' Added subproject with 
squash 
+test_expect_success 'add subproj as subtree into subdir/ with --prefix as -P 
and --message as -m' '
+   git subtree add -P subdir -m Added subproject FETCH_HEAD 
+   test_equal $(last_commit_message) Added subproject 
undo
 '
 
-test_expect_success 'add subproj to mainline' '
-   git subtree add --prefix=subdir/ FETCH_HEAD 
-   check_equal ''$(last_commit_message)'' Add ''subdir/'' 
from commit '$(git rev-parse sub1)'
+test_expect_success 'add subproj as subtree into subdir/ with --squash and 
--prefix and --message' '
+   git subtree add --prefix=subdir --message=Added subproject 
with squash --squash FETCH_HEAD 
+   test_equal 

[PATCH 11/13] contrib/subtree: Make each test self-contained

2013-02-05 Thread David A. Greene
From: Techlive Zheng techlivezh...@gmail.com

Signed-off-by: Techlive Zheng techlivezh...@gmail.com
Signed-off-by: David A. Greene gree...@obbligato.org
---
 contrib/subtree/t/t7900-subtree.sh |  871 +---
 1 file changed, 613 insertions(+), 258 deletions(-)

diff --git a/contrib/subtree/t/t7900-subtree.sh 
b/contrib/subtree/t/t7900-subtree.sh
index 9cfaaf9..769b116 100755
--- a/contrib/subtree/t/t7900-subtree.sh
+++ b/contrib/subtree/t/t7900-subtree.sh
@@ -12,12 +12,6 @@ export TEST_DIRECTORY=$(pwd)/../../../t
 
 . ../../../t/test-lib.sh
 
-create()
-{
-   echo $1 $1
-   git add $1
-}
-
 fixnl()
 {
t=
@@ -37,11 +31,6 @@ multiline()
done
 }
 
-undo()
-{
-   git reset --hard HEAD~
-}
-
 test_equal()
 {
test_debug 'echo'
@@ -78,381 +67,746 @@ join_commits()
echo $commit $all
 }
 
+test_create_commit() (
+   repo=$1
+   commit=$2
+   cd $repo
+   mkdir -p $(dirname $commit)
+   echo $commit  $commit
+   git add $commit
+   git commit -m $commit
+)
+
 last_commit_message()
 {
git log --pretty=format:%s -1
 }
 
-test_expect_success 'init subproj' '
-   test_create_repo subproj
-'
-
-# To the subproject!
-cd subproj
-
-test_expect_success 'add sub1' '
-   create sub1 
-   git commit -m sub1 
-   git branch sub1 
-   git branch -m master subproj
-'
-
-# Save this hash for testing later.
-
-subdir_hash=`git rev-parse HEAD`
-
-test_expect_success 'add sub2' '
-   create sub2 
-   git commit -m sub2 
-   git branch sub2
-'
-
-test_expect_success 'add sub3' '
-   create sub3 
-   git commit -m sub3 
-   git branch sub3
-'
-
-# Back to mainline
-cd ..
-
-test_expect_success 'add main4' '
-   create main4 
-   git commit -m main4 
-   git branch -m master mainline 
-   git branch init
-'
+#
+# Tests for 'git subtree add'
+#
 
-test_expect_success 'fetch subproj history' '
-   git fetch ./subproj sub1 
-   git branch sub1 FETCH_HEAD
-'
 
 test_expect_success 'no pull from non-existant subtree' '
-   test_must_fail git subtree pull --prefix=subdir ./subproj sub1
+   test_create_repo $test_count 
+   test_create_repo $test_count/subproj 
+   test_create_commit $test_count main1 
+   test_create_commit $test_count/subproj sub1 
+   (
+   cd $test_count 
+   git fetch ./subproj master 
+   test_must_fail git subtree pull --prefix=subdir ./subproj master
+   )
 '
 
 test_expect_success 'no merge from non-existant subtree' '
+   test_create_repo $test_count 
+   test_create_repo $test_count/subproj 
+   test_create_commit $test_count main1 
+   test_create_commit $test_count/subproj sub1 
+   (
+   cd $test_count 
+   git fetch ./subproj master 
test_must_fail git subtree merge --prefix=subdir FETCH_HEAD
+   )
 '
 
 test_expect_success 'add subproj as subtree into subdir/ with --prefix' '
+   test_create_repo $test_count 
+   test_create_repo $test_count/subproj 
+   test_create_commit $test_count main1 
+   test_create_commit $test_count/subproj sub1 
+   (
+   cd $test_count 
+   git fetch ./subproj master 
git subtree add --prefix=subdir FETCH_HEAD 
-   test_equal $(last_commit_message) Add '\''subdir/'\'' from 
commit '\''$(git rev-parse FETCH_HEAD)'\'' 
-   undo
+   test_equal $(last_commit_message) Add '\''subdir/'\'' from 
commit '\''$(git rev-parse FETCH_HEAD)'\''
+   )
 '
 
 test_expect_success 'add subproj as subtree into subdir/ with --prefix and 
--message' '
+   test_create_repo $test_count 
+   test_create_repo $test_count/subproj 
+   test_create_commit $test_count main1 
+   test_create_commit $test_count/subproj sub1 
+   (
+   cd $test_count 
+   git fetch ./subproj master 
git subtree add --prefix=subdir --message=Added subproject 
FETCH_HEAD 
-   test_equal $(last_commit_message) Added subproject 
-   undo
+   test_equal $(last_commit_message) Added subproject
+   )
 '
 
 test_expect_success 'add subproj as subtree into subdir/ with --prefix as -P 
and --message as -m' '
+   test_create_repo $test_count 
+   test_create_repo $test_count/subproj 
+   test_create_commit $test_count main1 
+   test_create_commit $test_count/subproj sub1 
+   (
+   cd $test_count 
+   git fetch ./subproj master 
git subtree add -P subdir -m Added subproject FETCH_HEAD 
-   test_equal $(last_commit_message) Added subproject 
-   undo
+   test_equal $(last_commit_message) Added subproject
+  

Re: [PATCH] Get correct column with for options in command usage

2013-02-05 Thread Duy Nguyen
On Tue, Feb 05, 2013 at 03:40:32PM +0800, Jiang Xin wrote:
 Command usage would not align well if command options are translated,
 especially to CJK. Call utf8_strwidth in function usage_argh, so that
 the caller will get correct column width.

Yeah, I just noticed a misalignment in Vietnamese translation (just
two spaces to the left, hard to notice unless paying attention).

  static int usage_argh(const struct option *opts, FILE *outfile)
  {
 - const char *s;
 + const char *s, *p;
   int literal = (opts-flags  PARSE_OPT_LITERAL_ARGHELP) || !opts-argh;
   if (opts-flags  PARSE_OPT_OPTARG)
   if (opts-long_name)
 @@ -482,7 +482,9 @@ static int usage_argh(const struct option *opts, FILE 
 *outfile)
   s = literal ? [%s] : [%s];
   else
   s = literal ?  %s :  %s;
 - return fprintf(outfile, s, opts-argh ? _(opts-argh) : _(...));
 + p = opts-argh ? _(opts-argh) : _(...);
 + fprintf(outfile, s, p);
 + return utf8_strwidth(p) + strlen(s) - 2;
  }

First of all, #include utf8.h is required for utf8_strwidth().

The strlen(s) - 2 is quite sensitive to how s is written. How
about this? A bit longer but clearer. Your version is OK too with a
comment explaining strlen(s) - 2.

-- 8 --
diff --git a/parse-options.c b/parse-options.c
index 67e98a6..0f803bd 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -474,6 +474,8 @@ int parse_options(int argc, const char **argv, const char 
*prefix,
 static int usage_argh(const struct option *opts, FILE *outfile)
 {
const char *s;
+   struct strbuf sb = STRBUF_INIT;
+   int columns;
int literal = (opts-flags  PARSE_OPT_LITERAL_ARGHELP) || !opts-argh;
if (opts-flags  PARSE_OPT_OPTARG)
if (opts-long_name)
@@ -482,7 +484,11 @@ static int usage_argh(const struct option *opts, FILE 
*outfile)
s = literal ? [%s] : [%s];
else
s = literal ?  %s :  %s;
-   return fprintf(outfile, s, opts-argh ? _(opts-argh) : _(...));
+   strbuf_addf(sb, s, opts-argh ? _(opts-argh) : _(...));
+   fprintf(outfile, sb.buf);
+   columns = utf8_strwidth(sb.buf);
+   strbuf_release(sb);
+   return columns;
 }
 
 #define USAGE_OPTS_WIDTH 24
-- 8 --

While looking at parse-options.c, I notice -NUM is not marked for
translation. I think you might want to mark it so that you can
translate NUM to a similar abbreviation in a local language. A
similar patch like this is required so we get columns for -NUM
translation instead of the number of bytes.
--
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3] status: show the branch name if possible in in-progress info

2013-02-05 Thread Duy Nguyen
On Tue, Feb 5, 2013 at 1:38 PM, Jonathan Nieder jrnie...@gmail.com wrote:
 Missing description.  Stealing from the link you sent:

 The typical use-case is starting a rebase, do something else, come 
 back
 the day after, run git status or make a new commit and wonder what
 in the world's going on. Which branch is being rebased is probably the
 most useful tidbit to help, but the target may help too.

 Ideally, I would have loved to see rebasing master on origin/master,
 but the target ref name is not stored during rebase, so this patch
 writes rebasing master on a78c8c98b as a half-measure to remind
 future users of that potential improvement.

 Signed-off-by: ...

Looking good. Junio, do you need a new patch mail or can you just
amend the commit message?
-- 
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 13/13] contrib/subtree: Remove --annotate

2013-02-05 Thread James Nylen
On Tue, Feb 5, 2013 at 6:44 AM,  gree...@obbligato.org wrote:
 Junio C Hamano gits...@pobox.com writes:

 Remove --annotate.  This obviates the need for an --unannotate
 command.  We really want a more generalized commit message rewrite
 mechanism.

 That may be a good goal as the end result, but wouldn't it be a bit
 unhelpful to remove these before adding such a more generalized
 mechanism to replace them?

 I did think about that.  I sent out an e-mail some time ago asking for
 opinions on this.  No one responded.  Since this is in contrib/ I feel
 comfortable getting rid of this option early so that people don't get
 too attached to it.  :)

I don't agree that removing `--annotate` obviates the need for `--unannotate`.

I responded on 1/17 with what I think is a typical and normal use case
for that option:

 - add fancylib as a subtree of myprog
 - commit to myprog repo: fancylib: don't crash as much
 - split these commits back out to fancylib main repo, and remove
the fancylib:  prefix

In my opinion this is a pretty normal workflow.  Commits to fancylib
in the myprog repo are prefixed with fancylib: , and that prefix
becomes redundant and should be removed if those commits are split
back out into the fancylib main repo.

I also tried to come up with another situation that would justify a
more general commit message rewriting facility, and I couldn't think
of any other good use cases that don't involve removing a prefix.  But
that doesn't mean there aren't any.

`--unannotate` is a clunky name, but I think this functionality is
worth taking another look at.  Maybe it could be called
`--remove-prefix` ?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 11/13] contrib/subtree: Make each test self-contained

2013-02-05 Thread Junio C Hamano
gree...@obbligato.org writes:

 Junio C Hamano gits...@pobox.com writes:

 David A. Greene gree...@obbligato.org writes:

 +test_create_commit() (
 +   repo=$1
 +   commit=$2
 +   cd $repo
 +   mkdir -p $(dirname $commit)
 +   echo $commit  $commit

 Style.

 I need a little more explanation.  :)  Is there a style guide somewhere?

Documentation/CodingGuidelines?


 +   git add $commit
 +   git commit -m $commit
 +)

 Very nice, but don't we want to check for possible errors in any of
 the above commands?

 I'll fix that.  :)
 ...
 Ok.  I'll rework this.

Thanks.

I also think it would be a good idea for you to learn to push back
to the original authors; fixing problems in patches by others, while
is a good way to learn how their thinking process went, is not
necessarily fun.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 3/8] upload/receive-pack: allow hiding ref hierarchies

2013-02-05 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 On Wed, Jan 30, 2013 at 10:45:37AM -0800, Junio C Hamano wrote:

 Teach upload-pack and receive-pack to omit some refs from their
 initial advertisements by paying attention to the transfer.hiderefs
 multi-valued configuration variable.  Any ref that is under the
 hierarchies listed on the value of this variable is excluded from
 responses to requests made by ls-remote, fetch, clone, push,
 etc.
 
 A typical use case may be
 
  [transfer]
  hiderefs = refs/pull
 
 to hide the refs that are internally used by the hosting site and
 should not be exposed over the network.

 In the earlier review, I mentioned making this per-service, but I see
 that is not the case here. Do you have an argument against doing so?

Perhaps then I misunderstood your intention.  By reminding me of the
receive-pack side, I thought you were hinting to unify these two
into one, which I did.  There is no argument against it.

 And I
 have not seen complaints about the current system.

Immediately after I added github to the set of places I push into,
which I think is long before you joined GitHub, I noticed that _my_
repository gets contaminated by second rate commits called pull
requests, and I may even have complained, but most likely I didn't,
as I could easily tell that, even though I know it is _not_ the only
way, nor even the best way [*1*], to implement the GitHub's pull
request workflow, I perfectly well understood that it would be the
most expedient way for GitHub folks to implement this feature.

I think you should take lack of complaints with a huge grain of
salt.  It does not suggest much.

 Gerrit's refs/changes may be a different story, if they have a large
 enough number of them to make upload-pack's ref advertisement
 overwhelming.

This is probably a stale count, but platform/frameworks/base part of
AOSP has 3200+ refs; the corresponding repository internal to Google
has 60k+ refs (this is because there are many in-between states
recorded in the internal repository, even though the end result
published to the open source repository may be the same) and results
in ~4MB advertisement.  Which is fairly significant when all you are
interested in doing is an Am I up to date? poll.


[Footnote]

*1* From the ownership point of view, objects that are only
reachable from these refs/pull/* refs do *not* belong to the
requestee, until the requestee chooses to accept the changes.

A malicious requestor can fork your repository, add an objectionable
blob to it, and throw a pull request at you.  GitHub shows that the
blob now belongs to your repository, so the requestor turns around
and file a DMCA takedown complaint against your repository.  A
clueful judge would then agree with the complaint after running a
clone --mirror and seeing the blob in your repository.  Oops?

A funny thing is that you cannot push :refs/pull/1/head to remove
it anymore (I think in the early days, I took them out by doing this
a few times, but I may be misremembering), so you cannot make
yourself into compliance, even though you are not the offending
party.  Your repository is held responsible for whatever the rogue
requestor added.  That is not very nice, is it?

In an ideal world, I would have chosen to create a dedicated fork
managed by the hosting company (i.e. GitHub) for your repository
whose only purpose is to house these refs/pull/ refs (the hosting
site is ultimately who has to respond to DMCA notices anyway, and an
arrangement like this makes it clear who is reponsible for what).

The e-mail sent to you to let you know about outstanding pull
requests and the web UI could just point at that forked repository,
not your own (you also could choose to leave the outging pull
requests in the requestor's repository, but that is only OK if you
do not worry about (1) a requestor sending a pull request, then
updating the branch the pull request talks about later, to trick you
with bait-and-switch, or (2) a requestor sending a pull request,
thinks he is done with the topic and removes the repository).
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] Should log --cc imply log --cc -p?

2013-02-05 Thread Junio C Hamano
Jeff King p...@peff.net writes:

   git log --cc --stat

 the --cc is significant. So I don't think it is right for --cc to
 always imply -p. But if the rule kicked in only when no other format
 had been specified, that might make sense.

Yes, it was my mistake that I left it unsaid.  Obviously the
description of the minor irritation must be qualified with unless
no other output options like --raw, --stat, etc. are given.

Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] Should log --cc imply log --cc -p?

2013-02-05 Thread Junio C Hamano
Ævar Arnfjörð Bjarmason ava...@gmail.com writes:

 On a related note then, it's a bit confusing that it's called
 --full-diff when it doesn't actually show a diff.

It is too late to change the name of the option, but we could add a
synonym if it makes easier to understand what the option is for.  It
is about saying my pathspec are only to limit the commits to be
shown and do not limit the diff output.  In that sense, the current
name that asks to give me the diff output fully is not that wrong,
but I wouldn't be surprised if other people can come up with better
names.



--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 7/8] fetch: fetch objects by their exact SHA-1 object names

2013-02-05 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 On Wed, Jan 30, 2013 at 10:45:41AM -0800, Junio C Hamano wrote:

 Teach git fetch to accept an exact SHA-1 object name the user may
 obtain out of band on the LHS of a pathspec, and send it on a want
 message when the server side advertises the allow-tip-sha1-in-want
 capability.

 Hmm. The UI on this is a little less nice than I would have hoped.

Naming with unadvertised *refname*, not object name, needs protocol
extension for the serving side to expand the name to object name;
otherwise the receiving end wouldn't know what tip what it asked
resulted in.

And that belongs to a separate expand refs extension (aka
delaying ref advertisement) that is outside the scope of this
series but can be built on top, as I said in the cover letter.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Is anyone working on a next-gen Git protocol (Re: [PATCH v3 0/8] Hiding refs)

2013-02-05 Thread Junio C Hamano
Ævar Arnfjörð Bjarmason ava...@gmail.com writes:

 Do you have any plans for something that *does* have the reduction of
 network bandwidth as a primary goal?

Uncluttering gives reduction of bandwidth anyway, so I do not see
much point in the distinction you seem to be making.

 Is this what you've been working on? Because if so I misunderstood you
 thinking you were going to work on something that gave clients the
 ability specify what they wanted before the initial ref advertisement.
 ...
 4. http://thread.gmane.org/gmane.comp.version-control.git/207190

Who speaks first mentioned in 4. above, was primarily about
delaying ref advertisement, which would be a larger protocol
change.  Nobody seems to have attacked it since it was discussed,
and I was tired of hearing nothing but complaints and whines.  This
hiding refs series was done as a cheaper way to solve a related
issue, without having to wait for the solution of delaying
advertisement, which is an orthogonal issue.



--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Add contrib/credentials/netrc with GPG support, try #2

2013-02-05 Thread Junio C Hamano
Ted Zlatanov t...@lifelogs.com writes:

 On Mon, 04 Feb 2013 16:15:47 -0800 Junio C Hamano gits...@pobox.com wrote: 

 JCH Ted Zlatanov t...@lifelogs.com writes:

 - do you want to support backslashed newlines?

 JCH What for?  netrc/authinfo is not a line oriented file format at all,
 JCH and

 JCH  machine k.org
 JCH  login me
 JCH password mysecret

 JCH is a single entry; you do not need backslash at the end of any line.

 Hmm. The parser I implemented only does single-line parsing, and I
 misunderstood the format to be single-line (partly because I have never
 seen anyone using the multi-line format you show).  Looking at
 Net::Netrc more carefully, it seems that the machine token is what
 defines an entry, so a new entry starts with a new line that contains a
 machine token.  Is that acceptable and does it match your
 understanding of the format?  It matches Net::Netrc, at least.

I thought I've given a more concrete outline than I'll read
Net::Netrc and do whatever I think it does in a separate message.

It would be better to read man netrc carefully at least once ;-)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 2/8] git_remote_helpers: fix input when running under Python 3

2013-02-05 Thread Erik Faye-Lund
On Fri, Jan 25, 2013 at 9:23 PM, Brandon Casey draf...@gmail.com wrote:
 On Wed, Jan 23, 2013 at 12:36 PM, Junio C Hamano gits...@pobox.com wrote:
 Sverre Rabbelier srabbel...@gmail.com writes:

 On Wed, Jan 23, 2013 at 11:47 AM, John Keeping j...@keeping.me.uk wrote:
 When did we last revisit what minimal python version we are ok with 
 requiring?

 I was wondering if people would weigh in discussing that in response to
 [1] but no one has commented on that part of it.  As another datapoint,
 Brandon Casey was suggesting patching git-p4.py to support Python 2.4
 [2].

 [1] http://article.gmane.org/gmane.comp.version-control.git/213920
 [2] http://article.gmane.org/gmane.comp.version-control.git/214048

 I for one would be happy to kill off support for anything older than
 2.6 (which had it's latest release on October 1st, 2008).

 Junio, how have we decided in the past which version of x to support?

 I do not think there was any conclusion.  $gmane/212215 claiming 2.4
 support matters for RHEL 5.x users was the last on the topic as far
 as I can tell, so it boils down to another question: do users on
 RHEL 5.x matter?

 I can read from $gmane/212215 that users of the said platform can
 safely keep using Python 2.4 under their vendor support contract
 until 2017.  But let's focus on what do these users expect of their
 system and software they run on it a bit.

 When they want to run a piece software that is not shipped with
 RHEL, either by writing their own or by importing from elsewhere,
 that needs 2.6 features, what are their options?

  (a) The platform vendor optionally supplies 2.6 with or without
  support;

  (b) The users can and do install 2.6 as /usr/local/bin/python2.6,
  which may even be community-supported, but the vendor does not
  support it; or

  (c) The vendor terminates the support contract for users who choose
  to go (b).

 I think we can safely discard (c); if that is the case, the users on
 the said platform will not choose to update Git either, so it does
 not matter where the future versions of Git sets the lower bound of
 Python version at.

 If we are not talking about the situation (c), then the users can
 choose to use 2.6, and more importantly, Python being a popular
 software, I would imagine that there are reputable sources of
 prepackaged RPMs for them to do so without going too much hassle of
 configuring, compiling and installing.

 Now how does the decision we make today for releases of Git that
 haven't yet happened will affect these users?  As these versions of
 newer Git were not shipped with RHEL 5.x, and also I am assuming
 that Git is a more niche product than Python is, I would imagine
 that it is very unlikely that the vendor gives it the users as an
 optional package.  The users will have to do the same thing to be
 able to use such versions of Git as whatever they do in order to use
 Python 2.6.

 Given that, what the vendor originally shipped and officially
 supports does not affect the choices we would make today for newer
 versions of Git.  The users in a shop where additional third-party
 software in /usr/local/bin is strictly forbidden, they are stuck
 with the version of Git that the vendor shipped anyway, because they
 won't be able to install an updated Git in /usr/local/bin, either.

 That is, unless installing 2.6 as /usr/local/bin/python2.6 (or if
 you are really paranoid, /usr/local/only-for-git/bin/python2.6 where
 nobody's $PATH points at) is impossible.

 So personally I do not think dropping 2.4 is a huge problem for
 future versions of Git, but I'd like to hear from those working in
 IT support for large and slow-moving organizations (aka RHEL 5
 customers).

 I'm not really in the demographic that you asked to hear from, but
 I'll give my 2 cents anyway. :)

 Firstly, I defer to those with more knowledge and experience with
 python to decide which version should be the minimum version
 supported.  Python 2.6 seems to be the consensus and that's fine with
 me.

 With respect to older platforms like RHEL 5.X that don't ship with
 Python 2.6 or later, I suspect most people who work in an organization
 with a dedicated IT staff can request that a more recent version of
 python be installed.  So, I don't think a python 2.6 requirement (if
 there was one) would be a blocker for them, and I don't think it would
 be a major pain for the sysadmin to install.


Just a datapoint: I'm working with customers on RHEL 5.X that
unfortunately has an extremely lengthy (3 months) process of
approving non-standard packages for install. Yeah, it's horrible, but
some times that's reality.

We are currently not using Git with that client, but we are in the
process of changing that. Said customer already have an exception for
all versions of Git.

I doubt this will end up being a problem in reality or not, but if it
will be, I'm sure it can be worked around out. I'm just pointing out
that the above suspicion might not be accurate.
--
To 

Re: [PATCH] Get correct column with for options in command usage

2013-02-05 Thread Junio C Hamano
Duy Nguyen pclo...@gmail.com writes:

 On Tue, Feb 5, 2013 at 7:15 PM, Duy Nguyen pclo...@gmail.com wrote:
 +   fprintf(outfile, sb.buf);

 Use fputs instead. I looked up fputs man page but somehow still left
 fprintf there.

Once the streams of oops that was wrong comments are done, I'd
appreciate if one of you guys send a finished patch for application,
instead of forcing me to follow all the messages in the discussion
and picking up pieces.

Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Add contrib/credentials/netrc with GPG support, try #2

2013-02-05 Thread Junio C Hamano
Ted Zlatanov t...@lifelogs.com writes:

 +# build reverse token map
 +my %rmap;
 +foreach my $k (keys %{$options{tmap}}) {
 + push @{$rmap{$options{tmap}-{$k}}}, $k;
 +}

Mental note: $rmap{foo} -eq 'bar' means that what Git calls 'bar'
is found as 'foo' in the netrc/authinfo file.  Keys in %rmap are
what we expect to read from the netrc/authinfo file.

 +# there are CPAN modules to do this better, but we want to avoid
 +# dependencies and generally, complex netrc-style files are rare
 +
 +if ($debug) {
 + printf STDERR searching for %s = %s\n, $_, $q{$_} || '(any value)'
 + foreach sort keys %q;
 +}
 +
 +LINE: foreach my $line (@data) {
 +
 + print STDERR line [$line]\n if $debug;
 + my @tok;
 + # gratefully stolen from Net::Netrc
 + while (length $line 
 +$line =~ s/^(((?:[^]+|\\.)*)|((?:[^\\\s]+|\\.)*))\s*//) {
 + (my $tok = $+) =~ s/\\(.)/$1/g;
 + push(@tok, $tok);
 + }
 +
 + # skip blank lines, comments, etc.
 + next LINE unless scalar @tok;
 +
 + my %tokens;
 + my $num_port;
 + while (@tok) {
 + my ($k, $v) = (shift @tok, shift @tok);
 + next unless defined $v;
 + next unless exists $options{tmap}-{$k};
 + $tokens{$options{tmap}-{$k}} = $v;
 + $num_port = $v if $k eq 'port'  $v =~ m/^\d+$/;
 + }

So you grabbed one line of input, split them into token pairs, and
built %tokens = ('key Git may want to see' = 'value read from file')
mapping.

 + # for host X port Y where Y is an integer (captured by
 + # $num_port above), set the host to X:Y
 + $tokens{host} = join(':', $tokens{host}, $num_port)
 + if defined $tokens{host}  defined $num_port;

What happens when 'host' does not exist?  netrc/authinfo should be a
stream of SP/HT/LF delimited tokens and 'machine' token (or
'default') begins a new entry, so it would mean the input file is
corrupt if we do not have $tokens{host} when we get here, I think.

Oh, another thing. 'default' is like 'machine' followed by any
machine name, so the above while loop that reads two tokens
pair-wise needs to be aware that 'default' is not followed by a
value.  I think the loop will fail to parse this:

default   login anonymouspassword me@home
machine k.org login me   password mysecret

 + foreach my $check (sort keys %q) {

Hmph, aren't you checking what you read a bit too early?  This is a
valid input:

default   
login anonymous
password me@home
machine k.org
login me
password mysecret

but does this loop gives mysecret back to me when asked for
host=k.org and user=me? 

 + if (exists $tokens{$check}  defined $q{$check}) {
 + print STDERR comparing [$tokens{$check}] to 
 [$q{$check}] in line [$line]\n if $debug;
 + next LINE unless $tokens{$check} eq $q{$check};
 + }
 + else {
 + print STDERR we could not find [$check] but it's OK\n 
 if $debug;
 + }
 + }

I would probably structure this part like this:

%pending = ();
split the whole input into tokens, regardless of lines;
iterate over the tokens {
peek the token
if (it is not default) {
take (token, value) pair;
} else {
take default as token; value does not matter.
}
if (token is default or machine) {
# finished reading one entry and we are
# at the beginning of the next entry.
# see if this entry matches
if (%pending is not empty 
%pending matches %q) {
found a match; use %pending;
}
# done with that entry. now start a new one.
%pending = ();
}
$pending{token} = value;
}

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Add contrib/credentials/netrc with GPG support, try #2

2013-02-05 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

 I thought I've given a more concrete outline than I'll read
 Net::Netrc and do whatever I think it does in a separate message.

And it turns out that the message was sitting in my outbox.  Sorry.

I just told the outbox to send it out.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 1/2] Get correct column with for options in command usage

2013-02-05 Thread Jiang Xin
Command usage would not align well if command options are translated,
especially to CJK. Call utf8_strwidth in function usage_argh, so that
the caller will get correct column width.

Signed-off-by: Jiang Xin worldhello@gmail.com
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 parse-options.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index 67e98..cd029f 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -3,6 +3,7 @@
 #include cache.h
 #include commit.h
 #include color.h
+#include utf8.h
 
 static int parse_options_usage(struct parse_opt_ctx_t *ctx,
   const char * const *usagestr,
@@ -473,7 +474,7 @@ int parse_options(int argc, const char **argv, const char 
*prefix,
 
 static int usage_argh(const struct option *opts, FILE *outfile)
 {
-   const char *s;
+   const char *s, *p;
int literal = (opts-flags  PARSE_OPT_LITERAL_ARGHELP) || !opts-argh;
if (opts-flags  PARSE_OPT_OPTARG)
if (opts-long_name)
@@ -482,7 +483,10 @@ static int usage_argh(const struct option *opts, FILE 
*outfile)
s = literal ? [%s] : [%s];
else
s = literal ?  %s :  %s;
-   return fprintf(outfile, s, opts-argh ? _(opts-argh) : _(...));
+   p = opts-argh ? _(opts-argh) : _(...);
+   fprintf(outfile, s, p);
+   /* Remove extra 2 chars (%s in s) to get column width of utf8 string 
*/
+   return utf8_strwidth(p) + strlen(s) - 2;
 }
 
 #define USAGE_OPTS_WIDTH 24
-- 
1.8.1.1.368.g6034fad.dirty

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 2/2] i18n: mark OPTION_NUMBER (-NUM) for translation

2013-02-05 Thread Jiang Xin
Signed-off-by: Jiang Xin worldhello@gmail.com
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 parse-options.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index cd029f..be916 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -497,6 +497,8 @@ static int usage_with_options_internal(struct 
parse_opt_ctx_t *ctx,
   const struct option *opts, int full, int 
err)
 {
FILE *outfile = err ? stderr : stdout;
+   const char *opt_num_buff = _(-NUM);
+   int opt_num_size = utf8_strwidth(opt_num_buff);
 
if (!usagestr)
return PARSE_OPT_HELP;
@@ -544,8 +546,10 @@ static int usage_with_options_internal(struct 
parse_opt_ctx_t *ctx,
pos += fprintf(outfile, , );
if (opts-long_name)
pos += fprintf(outfile, --%s, opts-long_name);
-   if (opts-type == OPTION_NUMBER)
-   pos += fprintf(outfile, -NUM);
+   if (opts-type == OPTION_NUMBER) {
+   fputs(opt_num_buff, outfile);
+   pos += opt_num_size;
+   }
 
if ((opts-flags  PARSE_OPT_LITERAL_ARGHELP) ||
!(opts-flags  PARSE_OPT_NOARG))
-- 
1.8.1.1.368.g6034fad.dirty

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [WIP/RFH/RFD/PATCH] grep: allow to use textconv filters

2013-02-05 Thread Michael J Gruber
Jeff King venit, vidit, dixit 05.02.2013 12:13:
 On Mon, Feb 04, 2013 at 04:27:31PM +0100, Michael J Gruber wrote:
 
 Recently and not so recently, we made sure that log/grep type operations
 use textconv filters when a userfacing diff would do the same:

 ef90ab6 (pickaxe: use textconv for -S counting, 2012-10-28)
 b1c2f57 (diff_grep: use textconv buffers for add/deleted files, 2012-10-28)
 0508fe5 (combine-diff: respect textconv attributes, 2011-05-23)

 git grep currently does not use textconv filters at all, that is
 neither for displaying the match and context nor for the actual grepping.

 Introduce a binary mode --textconv (in addition to --text and -I)
 which makes git grep use any configured textconv filters for grepping
 and output purposes.
 
 Sounds like a reasonable goal.
 
 The difficulty is in getting the different cases (blob/sha1 vs.
 worktree) right, and in making the changes minimally invasive. It seems
 that some more refactoring could help: git show --textconv does not
 use textconv filters when used on blobs either. (It does for diffs, of
 course.) Most existing helper functions are tailored for diffs.
 
 I think git show with blobs originally did not because we have no
 filename with which to look up the attributes. IIRC, the patches to
 support cat-file --textconv taught get_sha1_with_context to report the
 path at which we found a blob. I suspect it is mostly a matter of
 plumbing that information from the revision parser through to
 show_blob_object.
 
 Nota bene: --textconv does not affect diff --stat either...
 
 Yeah, though I wonder if it should be on by default. The diffstat for a
 binary file, unlike the diff, is already useful. The diffstat of the
 textconv'd data may _also_ be useful, of course.
 
 @@ -659,6 +659,9 @@ int cmd_grep(int argc, const char **argv, const char 
 *prefix)
  OPT_SET_INT('I', NULL, opt.binary,
  N_(don't match patterns in binary files),
  GREP_BINARY_NOMATCH),
 +OPT_SET_INT(0, textconv, opt.binary,
 +N_(process binary files with textconv filters),
 +GREP_BINARY_TEXTCONV),
 
 Is this really a new form of GREP_BINARY_*? What happens when a file
 does not have a textconv filter?
 
 I would expect this to be more like diff's --textconv flag, which is
 really allow textconv to be respected. Then you could do:
 
   git grep -I --textconv foo
 
 to grep in the text version of files which support it, and ignore the
 rest.
 
 -static int grep_source_load(struct grep_source *gs);
 -static int grep_source_is_binary(struct grep_source *gs);
 +static int grep_source_load(struct grep_source *gs, struct grep_opt *opt);
 +static int grep_source_is_binary(struct grep_source *gs, struct grep_opt 
 *opt);
 
 Hmm. grep_source_load is more or less the analogue of
 diff_populate_filespec, which does not know about textconv at all. So I
 feel like this might be going in at the wrong layer...
 
 @@ -1354,14 +1356,15 @@ static int grep_source_1(struct grep_opt *opt, 
 struct grep_source *gs, int colle
  
  switch (opt-binary) {
  case GREP_BINARY_DEFAULT:
 -if (grep_source_is_binary(gs))
 +if (grep_source_is_binary(gs, opt))
  binary_match_only = 1;
  break;
  case GREP_BINARY_NOMATCH:
 -if (grep_source_is_binary(gs))
 +if (grep_source_is_binary(gs, opt))
  return 0; /* Assume unmatch */
  break;
 
 The is_binary function learned about opt so that it could pass it to
 grep_source_load, which might do the textconv for us. But we _know_ that
 we will not here, because we see that we have other GREP_BINARY flags.
 
 And when we do have _TEXTCONV:
 
  case GREP_BINARY_TEXT:
 +case GREP_BINARY_TEXTCONV:
  break;
 
 We do not call is_binary. :)
 
 -static int grep_source_load_sha1(struct grep_source *gs)
 +static int grep_source_load_sha1(struct grep_source *gs, struct grep_opt 
 *opt)
  {
  enum object_type type;
 -
  grep_read_lock();
 -gs-buf = read_sha1_file(gs-identifier, type, gs-size);
 +if (opt-binary == GREP_BINARY_TEXTCONV) {
 +struct diff_filespec *df = alloc_filespec(gs-name);
 +gs-size = fill_textconv(gs-driver, df, gs-buf);
 +free_filespec(df);
 +} else {
 +gs-buf = read_sha1_file(gs-identifier, type, gs-size);
 +}
 
 So here we do the textconv for the sha1 case. But what about file
 sources?
 
 This is why I think the layer is wrong; you want the fill_textconv
 function to call your abstract _load function (in the diff_filespec
 world, it is diff_filespec_populate, but it is the moral equivalent).
 And you want it to hold off as long as possible in case we can pull the
 value from cache, or feed the working tree version of a file straight to
 the filter.
 
 -void grep_source_load_driver(struct grep_source *gs)
 +void 

Re: [PATCH v3] status: show the branch name if possible in in-progress info

2013-02-05 Thread Junio C Hamano
Duy Nguyen pclo...@gmail.com writes:

 On Tue, Feb 5, 2013 at 1:38 PM, Jonathan Nieder jrnie...@gmail.com wrote:
 Missing description.  Stealing from the link you sent:

 The typical use-case is starting a rebase, do something else, come 
 back
 the day after, run git status or make a new commit and wonder what
 in the world's going on. Which branch is being rebased is probably 
 the
 most useful tidbit to help, but the target may help too.

 Ideally, I would have loved to see rebasing master on 
 origin/master,
 but the target ref name is not stored during rebase, so this patch
 writes rebasing master on a78c8c98b as a half-measure to remind
 future users of that potential improvement.

 Signed-off-by: ...

 Looking good. Junio, do you need a new patch mail or can you just
 amend the commit message?

I'd like to see you either

 - send a reroll, making it clear it is a reroll, or
 - tell me to amend.

instead of asking which one I would prefer ;-)  One less message I
have to respond to that way.

commit --amend done.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Add contrib/credentials/netrc with GPG support, try #2

2013-02-05 Thread Ted Zlatanov
On Tue, 05 Feb 2013 08:15:48 -0800 Junio C Hamano gits...@pobox.com wrote: 

JCH Ted Zlatanov t...@lifelogs.com writes:
 +# build reverse token map
 +my %rmap;
 +foreach my $k (keys %{$options{tmap}}) {
 +push @{$rmap{$options{tmap}-{$k}}}, $k;
 +}

JCH Mental note: $rmap{foo} -eq 'bar' means that what Git calls 'bar'
JCH is found as 'foo' in the netrc/authinfo file.  Keys in %rmap are
JCH what we expect to read from the netrc/authinfo file.

I'll document that better in PATCHv4.

JCH So you grabbed one line of input, split them into token pairs, and
JCH built %tokens = ('key Git may want to see' = 'value read from file')
JCH mapping.

This will be fixed with PATCHv4 to do multiple lines (as defined by the
netrc manpage, etc.).

 +# for host X port Y where Y is an integer (captured by
 +# $num_port above), set the host to X:Y
 +$tokens{host} = join(':', $tokens{host}, $num_port)
 +if defined $tokens{host}  defined $num_port;

JCH What happens when 'host' does not exist?  netrc/authinfo should be a
JCH stream of SP/HT/LF delimited tokens and 'machine' token (or
JCH 'default') begins a new entry, so it would mean the input file is
JCH corrupt if we do not have $tokens{host} when we get here, I think.

Yes.  I'll make the host/machine token required, which will avoid this
issue.

JCH Oh, another thing. 'default' is like 'machine' followed by any
JCH machine name, so the above while loop that reads two tokens
JCH pair-wise needs to be aware that 'default' is not followed by a
JCH value.  I think the loop will fail to parse this:

JCH default   login anonymouspassword me@home
JCH machine k.org login me   password mysecret

I'd prefer to ignore default because it should not be used for the Git
credential helpers (its only use case is for anonymous services AFAIK).
So I'll add a case to ignore it in PATCHv4, if that's OK.

JCH Hmph, aren't you checking what you read a bit too early?  This is a
JCH valid input:

JCH default   
JCH login anonymous
JCH password me@home
JCH machine k.org
JCH login me
JCH password mysecret

JCH but does this loop gives mysecret back to me when asked for
JCH host=k.org and user=me? 

To be fixed in PATCHv4, which will require the host/machine, use it as
the primary key, and only examine entries with it.

JCH I would probably structure this part like this: [...]

I will do it like that, thank you for the suggestion.

I'll also add a simple testing Makefile for my own use, and you can
consider adding tests to the general framework later.

Ted
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Bug in git log --graph -p -m (version 1.7.7.6)

2013-02-05 Thread Dale R. Worley
I have found a situation where git log produces (apparently)
endless output.  Presumably this is a bug.  Following is a (Linux)
script that reliably reproduces the error for me (on Fedora 16):

--
set -ve

# Print the git version.
git --version

# Create respository.
rm -rf .git
git init

# Initial commit.
( echo 1 ; echo 2 ; echo 3 ) file
git add file
git commit -m 'Commit P'
git branch B HEAD

# Next commit on master adds line 1a.
( echo 1 ; echo 1a ; echo 2 ; echo 3 ) file
git add file
git commit -m 'Commit Q'

git checkout B

# Next commit on B adds line 2a.
( echo 1 ; echo 2 ; echo 2a ; echo 3 ) file
git add file
git commit -m 'Commit R'

# Merge the two commits, but add line 3a to the commit as well.
git checkout master
git merge --no-commit B
# Show what the merge produces.
cat file
# Add line 3a.
( echo 1 ; echo 1a ; echo 2 ; echo 2a ; echo 3 ; echo 3a ) file
git commit -m 'Commit S'

# These log commands work.
git log
git log --graph
git log --graph -p

# This log command produces infinite output.
git log --graph -p -m
--

Dale
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 2/2] i18n: mark OPTION_NUMBER (-NUM) for translation

2013-02-05 Thread Junio C Hamano
Jiang Xin worldhello@gmail.com writes:

 Signed-off-by: Jiang Xin worldhello@gmail.com
 Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
 ---
  parse-options.c | 8 ++--
  1 file changed, 6 insertions(+), 2 deletions(-)

 diff --git a/parse-options.c b/parse-options.c
 index cd029f..be916 100644
 --- a/parse-options.c
 +++ b/parse-options.c
 @@ -497,6 +497,8 @@ static int usage_with_options_internal(struct 
 parse_opt_ctx_t *ctx,
  const struct option *opts, int full, int 
 err)
  {
   FILE *outfile = err ? stderr : stdout;
 + const char *opt_num_buff = _(-NUM);
 + int opt_num_size = utf8_strwidth(opt_num_buff);
  
   if (!usagestr)
   return PARSE_OPT_HELP;
 @@ -544,8 +546,10 @@ static int usage_with_options_internal(struct 
 parse_opt_ctx_t *ctx,
   pos += fprintf(outfile, , );
   if (opts-long_name)
   pos += fprintf(outfile, --%s, opts-long_name);
 - if (opts-type == OPTION_NUMBER)
 - pos += fprintf(outfile, -NUM);
 + if (opts-type == OPTION_NUMBER) {
 + fputs(opt_num_buff, outfile);
 + pos += opt_num_size;
 + }

I somehow suspect that this is going in a direction that makes this
piece of code much less maintainable.

Look at the entire function and see how many places you do fprintf
on strings that are marked with _().  short_name and long_name are
not likely to be translated, but everything else is, especially
multiple places that show _(opts-help) neither of these patches
touch.

I wonder if it makes more sense to add a helper function that
returns the number of column positions (not bytes) with a signature
similar to fprintf() and use that throughout the function instead.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 0/8] Hiding refs

2013-02-05 Thread Junio C Hamano
Michael Haggerty mhag...@alum.mit.edu writes:

 I would again like to express my discomfort about this feature, which is
 already listed as will merge to next.

Do not take will merge to next too literally.  One major purpose
of marking a topic as such is exactly to solicit comments like this
;-)

 * I didn't see a response to Peff's convincing arguments that this
 should be a client-side feature rather than a server-side feature [1].

Uncluttering is not about a choice client should make.  delayed
advertisement is an orthogonal issue and requires a larger protocol
update (it needs to make git fetch speak first instead of the
current protocol in which upload-pack speaks first).

 * I didn't see an answer to Duy's question [2] about what is different
 between the proposed feature and gitnamespaces.

I think Jonathan addressed this already.

 * I didn't see a response to my worries that this feature could be
 abused [3].

You can choose not to advertise allow-tip-sha1-in-want capability; I
do not think it is making things worse than the status quo.

 * Why should a repository have exactly one setting for what refs should
 be hidden?  Wouldn't it make more sense to allow multiple views to be
 defined?:

You are welcome to extend to have different views, but how would
your clients express which view they would want?

Giving a single view that the serving end decides gives us an
immediate benefit of showing an uncluttered set of refs of server's
choice, without making the problem space larger than necessary.

 * Is it enough to support only reference exclusion (as opposed to
 exclusion and inclusion rules)?

Again, I do not think you cannot extend it to do positive and
negative filtering exclude these, but include those even though
they match the 'exclude these' patterns I gave you earlier.

 * Why should this feature only be available remotely?

The whole point is to give the server side a choice to show selected
refs, so that it can use hidden portion for its own use.  These refs
should not be hidden from local operations like gc.

I appreciate the comments, but I do not think any point you raised
in this message is very much relevant as objections.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 0/8] Hiding refs

2013-02-05 Thread Junio C Hamano
Jonathan Nieder jrnie...@gmail.com writes:

 * I didn't see a response to Peff's convincing arguments that this
 should be a client-side feature rather than a server-side feature [1].

 The client can't control the size of the ref advertisement.  That is
 the main motivation if I understood correctly.

The answer to this question is more nuanced.

With the current protocol, it is upload-pack who speaks first, so
there is no way for the requestor to say I am from an updated Git
suite and understand how to tell you to give me limited set of
refs, before upload-pack blasts 4MB of ref advertisement to it.

If the side that fetches is potentially interested in finding out
any and all refs, then an alternative solution would be to break
the current protocol, open a separate port and have upload-pack-2
listen to it, sit silently to let the requestor speak first when it
gets connection to that port.

But if the primary thing you are interested in is to hide the
references that:

 (1) the server side needs to keep track of for its own use; but
 (2) the requestors do not have to learn about from upload-pack,

we can do so without breaking older requestors.  That is what the
early part of this series is about.  We can view the last patch to
add the allow-tip-sha1-in-want as an icing on the cake.

It has the side effect of reducing the transfer overhead, because by
hiding the internal refs, the server side will stop blasting 4MB of
ref advertisements the requestors are not interested in, and that
would be the primary observable outcome from the end-user's point of
view (i.e. your git pull --ff-only will become a lot faster).



--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 0/8] Hiding refs

2013-02-05 Thread Junio C Hamano
Michael Haggerty mhag...@alum.mit.edu writes:

 On 02/05/2013 09:33 AM, Jonathan Nieder wrote:
 Michael Haggerty wrote:
 
 I would again like to express my discomfort about this feature, which is
 already listed as will merge to next.  Frankly, I have the feeling
 that this feature is being steamrolled in before a community consensus
 has been reached and indeed before many valid points raised by other
 members of the community have even been addressed.  For example:
 
 In $dayjob I work with Gerrit, so I think I can start to answer some
 of these questions.
 
 * I didn't see a response to Peff's convincing arguments that this
 should be a client-side feature rather than a server-side feature [1].
 
 The client can't control the size of the ref advertisement.  That is
 the main motivation if I understood correctly.

 Not according to Junio [4]:

   Look at this as a mechanism for the repository owner to control the
   clutter in what is shown to the intended audience of what s/he
   publishes in the repository.  Network bandwidth reduction of
   advertisement is a side effect of clutter reduction, and not
   necessarily the primary goal.

See my response to Jonathan.

 Hiderefs creates a dark corner of a remote git repo that can hold
 arbitrary content that is impossible for anybody to discover but
 nevertheless possible for anybody to download (if they know the name of
 a hidden reference).

That is why allow-tip-sha1-in-want is a separate opt-in feature only
the server side controls.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Bug in git log --graph -p -m (version 1.7.7.6)

2013-02-05 Thread Junio C Hamano
wor...@alum.mit.edu (Dale R. Worley) writes:

 I have found a situation where git log produces (apparently)
 endless output.  Presumably this is a bug.  Following is a (Linux)
 script that reliably reproduces the error for me (on Fedora 16):

Wasn't this fixed at v1.8.1.1~13 or is this a different issue?

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCHv4] Add contrib/credentials/netrc with GPG support

2013-02-05 Thread Ted Zlatanov
Changes since PATCHv3:

- simple tests in Makefile
- support multiple files, code refactored
- documentation and comments updated
- fix IO::File for GPG pipe
- exit peacefully in almost every situation, die on bad invocation or query
- use log_verbose() and -v for logging for the user
- use log_debug() and -d for logging for the developer
- use Net::Netrc parser and `man netrc' to improve parsing
- ignore 'default' and 'macdef' netrc entries
- require 'machine' token in netrc lines
- ignore netrc files with bad permissions or owner (from Net::Netrc)

Signed-off-by: Ted Zlatanov t...@lifelogs.com
---
 contrib/credential/netrc/Makefile |   10 +
 contrib/credential/netrc/git-credential-netrc |  423 +
 2 files changed, 433 insertions(+), 0 deletions(-)
 create mode 100644 contrib/credential/netrc/Makefile
 create mode 100755 contrib/credential/netrc/git-credential-netrc

diff --git a/contrib/credential/netrc/Makefile 
b/contrib/credential/netrc/Makefile
new file mode 100644
index 000..ee8c5f0
--- /dev/null
+++ b/contrib/credential/netrc/Makefile
@@ -0,0 +1,10 @@
+test_netrc:
+   @(echo bad data | ./git-credential-netrc -f A -d -v) || echo Bad 
invocation test, ignoring failure
+   @echo - Silent invocation... nothing should show up here with a 
missing file
+   @echo bad data | ./git-credential-netrc -f A get
+   @echo - Back to noisy: -v and -d used below, missing file
+   echo bad data | ./git-credential-netrc -f A -d -v get
+   @echo - Look for any entry in the default file set
+   echo  | ./git-credential-netrc -d -v get
+   @echo - Look for github.com in the default file set
+   echo host=google.com | ./git-credential-netrc -d -v get
diff --git a/contrib/credential/netrc/git-credential-netrc 
b/contrib/credential/netrc/git-credential-netrc
new file mode 100755
index 000..6946217
--- /dev/null
+++ b/contrib/credential/netrc/git-credential-netrc
@@ -0,0 +1,423 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings;
+
+use Getopt::Long;
+use File::Basename;
+
+my $VERSION = 0.1;
+
+my %options = (
+   help = 0,
+   debug = 0,
+   verbose = 0,
+  file = [],
+
+   # identical token maps, e.g. host - host, will be inserted 
later
+   tmap = {
+port = 'protocol',
+machine = 'host',
+path = 'path',
+login = 'username',
+user = 'username',
+password = 'password',
+   }
+  );
+
+# Map each credential protocol token to itself on the netrc side.
+foreach (values %{$options{tmap}}) {
+   $options{tmap}-{$_} = $_;
+}
+
+# Now, $options{tmap} has a mapping from the netrc format to the Git credential
+# helper protocol.
+
+# Next, we build the reverse token map.
+
+# When $rmap{foo} contains 'bar', that means that what the Git credential 
helper
+# protocol calls 'bar' is found as 'foo' in the netrc/authinfo file.  Keys in
+# %rmap are what we expect to read from the netrc/authinfo file.
+
+my %rmap;
+foreach my $k (keys %{$options{tmap}}) {
+   push @{$rmap{$options{tmap}-{$k}}}, $k;
+}
+
+Getopt::Long::Configure(bundling);
+
+# TODO: maybe allow the token map $options{tmap} to be configurable.
+GetOptions(\%options,
+   help|h,
+   debug|d,
+   verbose|v,
+   file|f=s@,
+  );
+
+if ($options{help}) {
+   my $shortname = basename($0);
+   $shortname =~ s/git-credential-//;
+
+   print EOHIPPUS;
+
+$0 [-f AUTHFILE1] [-f AUTHFILEN] [-d] [-v] get
+
+Version $VERSION by tzz\@lifelogs.com.  License: BSD.
+
+Options:
+
+  -f|--file AUTHFILE : specify netrc-style files.  Files with the .gpg 
extension
+   will be decrypted by GPG before parsing.  Multiple -f
+   arguments are OK, and the order is respected.
+
+  -d|--debug : turn on debugging (developer info)
+
+  -v|--verbose   : be more verbose (show files and information found)
+
+To enable this credential helper:
+
+  git config credential.helper '$shortname -f AUTHFILE1 -f AUTHFILE2'
+
+(Note that Git will prepend git-credential- to the helper name and look for 
it
+in the path.)
+
+...and if you want lots of debugging info:
+
+  git config credential.helper '$shortname -f AUTHFILE -d'
+
+...or to see the files opened and data found:
+
+  git config credential.helper '$shortname -f AUTHFILE -v'
+
+Only get mode is supported by this credential helper.  It opens every 
AUTHFILE
+and looks for the first entry that matches the requested search criteria:
+
+ 'port|protocol':
+   The protocol that will be used (e.g., https). (protocol=X)
+
+ 'machine|host':
+   The remote hostname for a network credential. (host=X)
+
+ 'path':
+   The path with which the credential will be used. (path=X)
+
+ 'login|user|username':
+   The credential’s username, if we 

Re* [PATCH 3/3] apply: diagnose incomplete submodule object name better

2013-02-05 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

 We could read from the payload part of the patch to learn the full
 object name of the commit, but the primary user git rebase has
 been fixed to give us a full object name, so this should suffice
 for now.

And the patch on top to do so looks like this.  With this patch in
place, we could drop 1/3 of this series, but there is no strong
reason to do so. After all, 1/3 is all about internal implementation
details.

-- 8 --
Subject: [PATCH 4/3] apply: verify submodule commit object name better

A textual patch also records the submodule commit object name in
full.  Make the parsing more robust by reading from there and
verifying the (possibly abbreviated) name on the index line matches.

Signed-off-by: Junio C Hamano gits...@pobox.com
---
 builtin/apply.c | 40 ++--
 1 file changed, 38 insertions(+), 2 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index 1f78e2c..e0f1474 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -3587,6 +3587,40 @@ static int get_current_sha1(const char *path, unsigned 
char *sha1)
return 0;
 }
 
+static int preimage_sha1_in_gitlink_patch(struct patch *p, unsigned char 
sha1[20])
+{
+   /*
+* A usable gitlink patch has only one fragment (hunk) that looks like:
+* @@ -1 +1 @@
+* -Subproject commit old sha1
+* +Subproject commit new sha1
+* or
+* @@ -1 +0,0 @@
+* -Subproject commit old sha1
+* for a removal patch.
+*/
+   struct fragment *hunk = p-fragments;
+   static const char heading[] = -Subproject commit ;
+   char *preimage;
+
+   if (/* does the patch have only one hunk? */
+   hunk  !hunk-next 
+   /* is its preimage one line? */
+   hunk-oldpos == 1  hunk-oldlines == 1 
+   /* does preimage begin with the heading? */
+   (preimage = memchr(hunk-patch, '\n', hunk-size)) != NULL 
+   !prefixcmp(++preimage, heading) 
+   /* does it record full SHA-1? */
+   !get_sha1_hex(preimage + sizeof(heading) - 1, sha1) 
+   preimage[sizeof(heading) + 40 - 1] == '\n' 
+   /* does the abbreviated name on the index line agree with it? */
+   !prefixcmp(preimage + sizeof(heading) - 1, p-old_sha1_prefix))
+   return 0; /* it all looks fine */
+
+   /* we may have full object name on the index line */
+   return get_sha1_hex(p-old_sha1_prefix, sha1);
+}
+
 /* Build an index that contains the just the files needed for a 3way merge */
 static void build_fake_ancestor(struct patch *list, const char *filename)
 {
@@ -3607,8 +3641,10 @@ static void build_fake_ancestor(struct patch *list, 
const char *filename)
continue;
 
if (S_ISGITLINK(patch-old_mode)) {
-   if (get_sha1_hex(patch-old_sha1_prefix, sha1))
-   die(submoule change for %s without full index 
name,
+   if (!preimage_sha1_in_gitlink_patch(patch, sha1))
+   ; /* ok, the textual part looks sane */
+   else
+   die(sha1 information is lacking or useless for 
submoule %s,
name);
} else if (!get_sha1_blob(patch-old_sha1_prefix, sha1)) {
; /* ok */
-- 
1.8.1.2.639.g9428735

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Add contrib/credentials/netrc with GPG support, try #2

2013-02-05 Thread Junio C Hamano
Ted Zlatanov t...@lifelogs.com writes:

 JCH Oh, another thing. 'default' is like 'machine' followed by any
 JCH machine name, so the above while loop that reads two tokens
 JCH pair-wise needs to be aware that 'default' is not followed by a
 JCH value.  I think the loop will fail to parse this:

 JCH default   login anonymouspassword me@home
 JCH machine k.org login me   password mysecret

 I'd prefer to ignore default because it should not be used for the Git
 credential helpers (its only use case is for anonymous services AFAIK).
 So I'll add a case to ignore it in PATCHv4, if that's OK.

You still need to parse a file that has a default entry correctly;
otherwise the users won't be able to share existing .netrc files
with other applications e.g. ftp, which is the whole point of this
series.  Not using values from the default entry is probably fine,
though.

Thanks.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCHv4] Add contrib/credentials/netrc with GPG support

2013-02-05 Thread Junio C Hamano
Ted Zlatanov t...@lifelogs.com writes:

 Changes since PATCHv3:

 - simple tests in Makefile
 - support multiple files, code refactored
 - documentation and comments updated
 - fix IO::File for GPG pipe
 - exit peacefully in almost every situation, die on bad invocation or query
 - use log_verbose() and -v for logging for the user
 - use log_debug() and -d for logging for the developer
 - use Net::Netrc parser and `man netrc' to improve parsing
 - ignore 'default' and 'macdef' netrc entries
 - require 'machine' token in netrc lines
 - ignore netrc files with bad permissions or owner (from Net::Netrc)

Please place the above _after_ the three-dashes.

The space here, above ---, is to justify why this change is a good
idea to people who see this patch for the first time who never saw
the earlier rounds of this patch, e.g. reading git log output 6
months down the road (see Documentation/SubmittingPatches (2)
Describe your changes well).


 Signed-off-by: Ted Zlatanov t...@lifelogs.com
 ---
  contrib/credential/netrc/Makefile |   10 +
  contrib/credential/netrc/git-credential-netrc |  423 
 +
  2 files changed, 433 insertions(+), 0 deletions(-)
  create mode 100644 contrib/credential/netrc/Makefile
  create mode 100755 contrib/credential/netrc/git-credential-netrc

 diff --git a/contrib/credential/netrc/Makefile 
 b/contrib/credential/netrc/Makefile
 new file mode 100644
 index 000..ee8c5f0
 --- /dev/null
 +++ b/contrib/credential/netrc/Makefile
 @@ -0,0 +1,10 @@
 +test_netrc:
 + @(echo bad data | ./git-credential-netrc -f A -d -v) || echo Bad 
 invocation test, ignoring failure
 + @echo - Silent invocation... nothing should show up here with a 
 missing file

Avoid starting an argument to echo with a dash; some
implementations choke with unknown option.

 + @echo bad data | ./git-credential-netrc -f A get
 + @echo - Back to noisy: -v and -d used below, missing file
 + echo bad data | ./git-credential-netrc -f A -d -v get
 + @echo - Look for any entry in the default file set
 + echo  | ./git-credential-netrc -d -v get
 + @echo - Look for github.com in the default file set
 + echo host=google.com | ./git-credential-netrc -d -v get
 diff --git a/contrib/credential/netrc/git-credential-netrc 
 b/contrib/credential/netrc/git-credential-netrc
 new file mode 100755
 index 000..6946217
 --- /dev/null
 +++ b/contrib/credential/netrc/git-credential-netrc
 @@ -0,0 +1,423 @@
 +#!/usr/bin/perl
 +
 +use strict;
 +use warnings;
 +
 +use Getopt::Long;
 +use File::Basename;
 +
 +my $VERSION = 0.1;
 +
 +my %options = (
 +   help = 0,
 +   debug = 0,
 +   verbose = 0,
 +file = [],

Looks like there is some funny indentation going on here.

 +
 +   # identical token maps, e.g. host - host, will be inserted 
 later
 +   tmap = {
 +port = 'protocol',
 +machine = 'host',
 +path = 'path',
 +login = 'username',
 +user = 'username',
 +password = 'password',
 +   }
 +  );
 +
 +# Map each credential protocol token to itself on the netrc side.
 +foreach (values %{$options{tmap}}) {
 + $options{tmap}-{$_} = $_;
 +}
 +
 +# Now, $options{tmap} has a mapping from the netrc format to the Git 
 credential
 +# helper protocol.
 +
 +# Next, we build the reverse token map.
 +
 +# When $rmap{foo} contains 'bar', that means that what the Git credential 
 helper
 +# protocol calls 'bar' is found as 'foo' in the netrc/authinfo file.  Keys in
 +# %rmap are what we expect to read from the netrc/authinfo file.
 +
 +my %rmap;
 +foreach my $k (keys %{$options{tmap}}) {
 + push @{$rmap{$options{tmap}-{$k}}}, $k;
 +}
 +
 +Getopt::Long::Configure(bundling);
 +
 +# TODO: maybe allow the token map $options{tmap} to be configurable.
 +GetOptions(\%options,
 +   help|h,
 +   debug|d,
 +   verbose|v,
 +   file|f=s@,
 +  );
 +
 +if ($options{help}) {
 + my $shortname = basename($0);
 + $shortname =~ s/git-credential-//;
 +
 + print EOHIPPUS;
 +
 +$0 [-f AUTHFILE1] [-f AUTHFILEN] [-d] [-v] get
 +
 +Version $VERSION by tzz\@lifelogs.com.  License: BSD.
 +
 +Options:
 +
 +  -f|--file AUTHFILE : specify netrc-style files.  Files with the .gpg 
 extension
 +   will be decrypted by GPG before parsing.  Multiple -f
 +   arguments are OK, and the order is respected.

Saying order is respected without mentioning the collision
resolution rules is not helpful to the users when deciding in what
order they should give these files.  First one wins, or last one
wins?  Later you say looks for the first entry, but it will be
much easier to read the above to mention it here as well.

 +  -d|--debug : turn on debugging (developer info)
 +
 +  

Re: [PATCH] Add contrib/credentials/netrc with GPG support, try #2

2013-02-05 Thread Ted Zlatanov
On Tue, 05 Feb 2013 11:47:56 -0800 Junio C Hamano gits...@pobox.com wrote: 

JCH Ted Zlatanov t...@lifelogs.com writes:
JCH Oh, another thing. 'default' is like 'machine' followed by any
JCH machine name, so the above while loop that reads two tokens
JCH pair-wise needs to be aware that 'default' is not followed by a
JCH value.  I think the loop will fail to parse this:
 
JCH default   login anonymouspassword me@home
JCH machine k.org login me   password mysecret
 
 I'd prefer to ignore default because it should not be used for the Git
 credential helpers (its only use case is for anonymous services AFAIK).
 So I'll add a case to ignore it in PATCHv4, if that's OK.

JCH You still need to parse a file that has a default entry correctly;
JCH otherwise the users won't be able to share existing .netrc files
JCH with other applications e.g. ftp, which is the whole point of this
JCH series.  Not using values from the default entry is probably fine,
JCH though.

OK; done in PATCHv4.

Ted
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Is anyone working on a next-gen Git protocol (Re: [PATCH v3 0/8] Hiding refs)

2013-02-05 Thread Ævar Arnfjörð Bjarmason
On Tue, Feb 5, 2013 at 5:03 PM, Junio C Hamano gits...@pobox.com wrote:
 Ævar Arnfjörð Bjarmason ava...@gmail.com writes:

 Do you have any plans for something that *does* have the reduction of
 network bandwidth as a primary goal?

 Uncluttering gives reduction of bandwidth anyway, so I do not see
 much point in the distinction you seem to be making.

Doing this work wouldn't only give us a way to specify which refs we
want, but if done correctly would future-proof the protocol in case we
want to add any other extensions down the line in a
backwards-compatible fashion without having the server first spew all
his refs at us.

Anyway, an implementation that allows a client to say I want X is
simpler than an implementation where a server has to anticipate in
advance which X the clients will ask for.

 Is this what you've been working on? Because if so I misunderstood you
 thinking you were going to work on something that gave clients the
 ability specify what they wanted before the initial ref advertisement.
 ...
 4. http://thread.gmane.org/gmane.comp.version-control.git/207190

 Who speaks first mentioned in 4. above, was primarily about
 delaying ref advertisement, which would be a larger protocol
 change.  Nobody seems to have attacked it since it was discussed,
 and I was tired of hearing nothing but complaints and whines.  This
 hiding refs series was done as a cheaper way to solve a related
 issue, without having to wait for the solution of delaying
 advertisement, which is an orthogonal issue.

Oh sure. I just wanted to know if you were working on delaying ref
advertisement to avoid duplicating efforts. I had the impression you
were given your earlier E-Mail, but obviously we had a
misunderstanding.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [WIP/RFH/RFD/PATCH] grep: allow to use textconv filters

2013-02-05 Thread Jeff King
On Tue, Feb 05, 2013 at 05:21:18PM +0100, Michael J Gruber wrote:

 Thanks Jeff, that helps a lot! It covers grep expr and grep expr rev
 -- path just fine. I'll look into grep expr rev:path which does not
 work yet because of an empty driver.
 
 I also have show --textconv covered and a suggestion for cat-file
 --textconv (to work without a textconv filter).
 
 Expect a mini-series soon :)

Cool, I'm glad it helped. It would be great if diff_filespec and
grep_source could grow together into a unified object. One of the gross
things about the patch I posted is that we will now sometimes read the
file/blob data via grep_source_load, and sometimes via
diff_populate_filespec. They _should_ be equivalent, but in an ideal
world, they would be the same code path.

That may be too much to tackle for your series, though (I wanted to do
it when I factored out grep_source, but backed off for the same reason).

The grep expr rev:path fix should look something like this:

diff --git a/builtin/grep.c b/builtin/grep.c
index 915c8ef..cdc7d32 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -820,13 +820,17 @@ int cmd_grep(int argc, const char **argv, const char 
*prefix)
for (i = 0; i  argc; i++) {
const char *arg = argv[i];
unsigned char sha1[20];
+   struct object_context oc;
/* Is it a rev? */
-   if (!get_sha1(arg, sha1)) {
+   if (!get_sha1_with_context(arg, sha1, oc)) {
struct object *object = parse_object(sha1);
if (!object)
die(_(bad object %s), arg);
if (!seen_dashdash)
verify_non_filename(prefix, arg);
+   /* oops, we need something that will remember oc.path
+* here, so that we can pass it along to
+* grep_source_init  */
add_object_array(object, arg, list);
continue;
}

But you'll have to replace the object_array with something more
featureful, I think.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Add contrib/credentials/netrc with GPG support, try #2

2013-02-05 Thread Junio C Hamano
Ted Zlatanov t...@lifelogs.com writes:

 JCH You still need to parse a file that has a default entry correctly;
 JCH otherwise the users won't be able to share existing .netrc files
 JCH with other applications e.g. ftp, which is the whole point of this
 JCH series.  Not using values from the default entry is probably fine,
 JCH though.

 OK; done in PATCHv4.

Hmph.

Didn't you remove that from your version of net_netrc_loader when
you borrowed the bulk of the code from Net::Netrc::_readrc?  I see
default token handled at the beginning of TOKEN: while (@tok)
loop in the original but not in your version I see in v4.


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] t4038: add tests for diff --cc --raw trees

2013-02-05 Thread John Keeping
Signed-off-by: John Keeping j...@keeping.me.uk
---
On Sun, Feb 03, 2013 at 04:24:52PM -0800, Junio C Hamano wrote:
 Ideally it should also have test cases
 to show git diff --cc --raw blob1 blob2...blob$n for n=4 and n=40
 (or any two values clearly below and above the old hardcoded limit)
 behave sensibly, exposing the old breakage, which I'll leave as a
 LHF (low-hanging-fruit).  Hint, hint...

Hint taken ;-)

git-diff uses a different code path for blobs, so I've had to use trees
to trigger this.  The last test fails without
jc/combine-diff-many-parents and passes with it.

 t/t4038-diff-combined.sh | 29 +
 1 file changed, 29 insertions(+)

diff --git a/t/t4038-diff-combined.sh b/t/t4038-diff-combined.sh
index 40277c7..a0701bc 100755
--- a/t/t4038-diff-combined.sh
+++ b/t/t4038-diff-combined.sh
@@ -89,4 +89,33 @@ test_expect_success 'diagnose truncated file' '
grep diff --cc file out
 '
 
+test_expect_success 'setup for --cc --raw' '
+   blob=$(echo file |git hash-object --stdin -w) 
+   base_tree=$(echo 100644 blob $blob file | git mktree) 
+   trees= 
+   for i in `test_seq 1 40`
+   do
+   blob=$(echo file$i |git hash-object --stdin -w) 
+   trees=$trees $(echo 100644 blob $blob file |git mktree)
+   done
+'
+
+test_expect_success 'check --cc --raw with four trees' '
+   four_trees=$(echo $trees |awk -e {
+   print \$1
+   print \$2
+   print \$3
+   print \$4
+   }) 
+   git diff --cc --raw $four_trees $base_tree out 
+   # Check for four leading colons in the output:
+   grep ^[^:] out
+'
+
+test_expect_success 'check --cc --raw with forty trees' '
+   git diff --cc --raw $trees $base_tree out 
+   # Check for forty leading colons in the output:
+   grep ^[^:] out
+'
+
 test_done
-- 
1.8.1.2

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] tests: turn on test-lint-shell-syntax by default

2013-02-05 Thread Torsten Bögershausen
On 27.01.13 18:34, Junio C Hamano wrote:
 Torsten Bögershausen tbo...@web.de writes:

 Back to the which:
 ...
 and running make test gives the following, at least in my system:
 ...
 I think everybody involved in this discussion already knows that;
 the point is that it can easily give false negative, without the
 scripts working very hard to do so.

 If we did not care about incurring runtime performance cost, we
 could arrange:

  - the test framework to define a variable $TEST_ABORT that has a
full path to a file that is in somewhere test authors cannot
touch unless they really try hard to (i.e. preferrably outside
$TRASH_DIRECTORY, as it is not uncommon for to tests to do rm *
there). This location should be per $(basename $0 .sh) to allow
running multiple tests in paralell;

  - the test framework to rm -f $TEST_ABORT at the beginning of
test_expect_success/failure;

  - test_expect_success/failure to check $TEST_ABORT and if it
exists, abort the execution, showing the contents of the file as
an error message.

 Then you can wrap commands whose use we want to limit, perhaps like
 this, in the test framework:

   which () {
   cat $TEST_ABORT -\EOF
   Do not use unportable 'which' in the test script.
 if type $cmd is a good way to see if $cmd exists.
   EOF
   }

   sed () {
   saw_wantarg= must_abort=
 for arg
 do
   if test -n $saw_wantarg
 then
   saw_wantarg=
 continue
   fi
   case $arg in
   --) break ;; # end of options
   -i) echo $TEST_ABORT Do not use 'sed -i'
   must_abort=yes
   break ;;
 -e)   saw_wantarg=yes ;; # skip next arg
   -*) continue ;; # options without arg
   *)  break ;; # filename
   esac
   done
   if test -z $must_abort
   sed $@
   fi
   }

 Then you can check that TEST_ABORT does not appear in test scripts
 (ensuring that they do not attempt to circumvent the mechanis) and
 catch use of unwanted commands or unwanted extended features of
 commands at runtime.

 But this will incur runtime performace hit, so I am not sure it
 would be worth it.
Thanks for the detailed suggestion.
Instead of using a file for putting out non portable syntax,
can we can use a similar logic as test_failure ?

I did some benchmarking, the test suite on a Laptop is 37..38 minutes,
including make clean  make both on next, pu, master or with the patch below.
I couldn't find a measurable impact on the execution time.
What do we think about a patch like this
(Not sure if this cut-and-paste data applies, it's for review)


[PATCH] test-lib: which, echo -n and sed -i are not portable

The posix version of sed command supports options -n -e -f
The gnu extension -i to edit a file in place is not
available on all systems.
To catch the usage of non-posix options with sed a
wrapper function is added in test-lib.sh.
The wrapper checks that only -n -e -f are used.
The short form -ne for -n -e is allowed as well.

echo -n is not portable and not available on all systems,
printf can be used instead.
Add a wrapper to catch echo -n

which is not portable, the output differs between different
implementations, and the return code may not be reliable.

Add a function test_bad_syntax_ in test-lib.sh, which increments
the variable test_bad_syntax and works similar to test_failure_

Signed-off-by: Torsten Bögershausen tbo...@web.de
---
 t/test-lib.sh | 46 ++
 1 file changed, 46 insertions(+)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 1a6c4ab..248ed34 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -266,6 +266,7 @@ else
 exec 4/dev/null 3/dev/null
 fi
 
+test_bad_syntax=0
 test_failure=0
 test_count=0
 test_fixed=0
@@ -300,6 +301,12 @@ test_ok_ () {
 say_color  ok $test_count - $@
 }
 
+test_bad_syntax_ () {
+test_bad_syntax=$(($test_bad_syntax + 1))
+say_color error $@
+test $immediate =  || { GIT_EXIT_OK=t; exit 1; }
+}
+
 test_failure_ () {
 test_failure=$(($test_failure + 1))
 say_color error not ok $test_count - $1
@@ -402,10 +409,15 @@ test_done () {
 fixed $test_fixed
 broken $test_broken
 failed $test_failure
+bad_syntax $test_bad_syntax
 
 EOF
 fi
 
+if test $test_bad_syntax != 0
+then
+say_color error # $test_bad_syntax non portable shell syntax
+fi
 if test $test_fixed != 0
 then
 say_color error # $test_fixed known breakage(s) vanished; please 
update test(s)
@@ -645,6 +657,40 @@ yes () {
 done
 }
 
+
+# which is 

Re: [PATCHv4] Add contrib/credentials/netrc with GPG support

2013-02-05 Thread Ted Zlatanov
On Tue, 05 Feb 2013 11:53:20 -0800 Junio C Hamano gits...@pobox.com wrote: 

JCH Ted Zlatanov t...@lifelogs.com writes:
 Changes since PATCHv3:
 
 - simple tests in Makefile
 - support multiple files, code refactored
 - documentation and comments updated
 - fix IO::File for GPG pipe
 - exit peacefully in almost every situation, die on bad invocation or query
 - use log_verbose() and -v for logging for the user
 - use log_debug() and -d for logging for the developer
 - use Net::Netrc parser and `man netrc' to improve parsing
 - ignore 'default' and 'macdef' netrc entries
 - require 'machine' token in netrc lines
 - ignore netrc files with bad permissions or owner (from Net::Netrc)

JCH Please place the above _after_ the three-dashes.

JCH The space here, above ---, is to justify why this change is a good
JCH idea to people who see this patch for the first time who never saw
JCH the earlier rounds of this patch, e.g. reading git log output 6
JCH months down the road (see Documentation/SubmittingPatches (2)
JCH Describe your changes well).

Will do in PATCHv5.

JCH Avoid starting an argument to echo with a dash; some
JCH implementations choke with unknown option.

Nice, thanks.  It's purely decorative so I use '=' now.

 +my %options = (
 +   help = 0,
 +   debug = 0,
 +   verbose = 0,
 +   file = [],

JCH Looks like there is some funny indentation going on here.

Fixed.

 +  -f|--file AUTHFILE : specify netrc-style files.  Files with the .gpg 
 extension
 +   will be decrypted by GPG before parsing.  Multiple -f
 +   arguments are OK, and the order is respected.

JCH Saying order is respected without mentioning the collision
JCH resolution rules is not helpful to the users when deciding in what
JCH order they should give these files.  First one wins, or last one
JCH wins?  Later you say looks for the first entry, but it will be
JCH much easier to read the above to mention it here as well.

Right.  Reworded.

 +Thus, when we get this query on STDIN:
 +
 +protocol=https
 +username=tzz
 +
 +this credential helper will look for the first entry in every AUTHFILE that
 +matches
 +
 +port https login tzz
 +
 +OR
 +
 +protocol https login tzz
 +
 +OR... etc. acceptable tokens as listed above.  Any unknown tokens are
 +simply ignored.
 +
 +Then, the helper will print out whatever tokens it got from the entry, 
 including
 +password tokens, mapping back to Git's helper protocol; e.g. port is 
 mapped
 +back to protocol.

JCH Isn't hostname typically what users expect to see?  It is somewhat
JCH unnerving to see an example that throws the same password back to
JCH any host you happen to have an accoutn tzz on, even though that is
JCH not technically an invalid way to use this helper.

Yeah, I changed it to show machine in the query (which would be more typical).

 +if ($stat[2]  077) {
 +log_verbose(Insecure $file (mode=%04o); 
 skipping it,
 +$stat[2]  0);

JCH Nice touch, although I am not sure rejecting world or group readable
JCH encrypted file is absolutely necessary.

Right.  Fixed.

 +if ($stat[4] != $) {
 +log_verbose(Not owner of $file; skipping it);
 +next FILE;

JCH OK.  A group of local users may share the same account at the
JCH remote, but that would be unusual.

I added --insecure/-k to override this check.

 +if ($mode  077)

JCH Again?  Didn't you just do this?

Damn, sorry.

JCH I think the prevalent style is to

JCHif (condition) {
JCHdo this;
JCH} elsif (another condition) {
JCHdo that
JCH} else {
JCHdo that other thing;
JCH}

JCH (this comment applies to all if/elsif/else cascades in this patch).

Yup.  I was working with Net::Netrc code and forgot to reformat it.  Sorry.

 +
 +next FILE;

JCH Isn't this outermost loop, by the way?  What the motivation to have
JCH an explicit label everywhere (not complaining---it could be your own
JCH discipline thing---just wondering).

I think it's more readable with large loops, and it actually makes sense
when you read the code.  Not a big deal to me either, I just felt for
this particular script it was OK.

 +if ($file =~ m/\.gpg$/) {
 +log_verbose(Using GPG to open $file);
 +# GPG doesn't work well with 2- or 3-argument open

JCH If that is the case, please quote $file properly against shell
JCH munging it.

Ahhh that gets ugly.  OK, quoted.

JCH The only thing you do on $io is to read from it via while ($io),
JCH so I would personally have written this part like this without
JCH having to use IO::File(), though:

JCH$io = open(-|, qw(gpg --decrypt), $file);

That doesn't work for me, unfortunately.  I'm trying to avoid the IPC::*
modules and such.  Please test it yourself with GPG.  I'm on Perl
5.14.2.

I think 

Re: [PATCH] t4038: add tests for diff --cc --raw trees

2013-02-05 Thread Junio C Hamano
John Keeping j...@keeping.me.uk writes:

 Signed-off-by: John Keeping j...@keeping.me.uk
 ---
 ...
 diff --git a/t/t4038-diff-combined.sh b/t/t4038-diff-combined.sh
 index 40277c7..a0701bc 100755
 --- a/t/t4038-diff-combined.sh
 +++ b/t/t4038-diff-combined.sh
 @@ -89,4 +89,33 @@ test_expect_success 'diagnose truncated file' '
   grep diff --cc file out
  '
  
 +test_expect_success 'setup for --cc --raw' '
 + blob=$(echo file |git hash-object --stdin -w) 
 + base_tree=$(echo 100644 blob $blob file | git mktree) 
 + trees= 
 + for i in `test_seq 1 40`
 + do
 + blob=$(echo file$i |git hash-object --stdin -w) 
 + trees=$trees $(echo 100644 blob $blob file |git mktree)

Please have a SP after each of these '|' pipes.

If you collect trees this way:

trees=$trees$(echo ... | git mktree)$LF

then ...

 + done
 +'
 +
 +test_expect_success 'check --cc --raw with four trees' '
 + four_trees=$(echo $trees |awk -e {
 + print \$1
 + print \$2
 + print \$3
 + print \$4
 + }) 

(What's awk -e?)

... you can do

echo $trees | sed -e 4q

which is less repetitive.

Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] tests: turn on test-lint-shell-syntax by default

2013-02-05 Thread Junio C Hamano
Torsten Bögershausen tbo...@web.de writes:

 Thanks for the detailed suggestion.
 Instead of using a file for putting out non portable syntax,
 can we can use a similar logic as test_failure ?

Your test_bad_syntax_ function can be called from a subshell, and
its exit 1 will not exit, no?

test_expect_success 'prepare a blob with incomplete line' '
(
echo first line
echo second line
echo -n final and incomplete line
) incomplete.txt
'

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCHv5] Add contrib/credentials/netrc with GPG support

2013-02-05 Thread Ted Zlatanov
Add Git credential helper that can parse netrc/authinfo files.

This credential helper support multiple files, returning the first one
that matches.  It checks file permissions and owner.  For *.gpg files,
it will run GPG to decrypt the file.

Signed-off-by: Ted Zlatanov t...@lifelogs.com
---
Changes since PATCHv4:

- indentation and brace fixes
- test makefile uses = as the decorative prefix
- documentation fixes about order and show query with hostname
- add --insecure to ignore owner and permission checks
- check permissions just once and only for unencrypted files
- change IO::File to simple open() and quote $file
- fixed macdef buglet from Net::Netrc
- ignore 'default' entries

 contrib/credential/netrc/Makefile |   12 +
 contrib/credential/netrc/git-credential-netrc |  424 +
 2 files changed, 436 insertions(+), 0 deletions(-)
 create mode 100644 contrib/credential/netrc/Makefile
 create mode 100755 contrib/credential/netrc/git-credential-netrc

diff --git a/contrib/credential/netrc/Makefile 
b/contrib/credential/netrc/Makefile
new file mode 100644
index 000..18a924f
--- /dev/null
+++ b/contrib/credential/netrc/Makefile
@@ -0,0 +1,12 @@
+test_netrc:
+   @(echo bad data | ./git-credential-netrc -f A -d -v) || echo Bad 
invocation test, ignoring failure
+   @echo = Silent invocation... nothing should show up here with a 
missing file
+   @echo bad data | ./git-credential-netrc -f A get
+   @echo = Back to noisy: -v and -d used below, missing file
+   echo bad data | ./git-credential-netrc -f A -d -v get
+   @echo = Look for any entry in the default file set
+   echo  | ./git-credential-netrc -d -v get
+   @echo = Look for github.com in the default file set
+   echo host=google.com | ./git-credential-netrc -d -v get
+   @echo = Look for a nonexistent machine in the default file set
+   echo host=korovamilkbar | ./git-credential-netrc -d -v get
diff --git a/contrib/credential/netrc/git-credential-netrc 
b/contrib/credential/netrc/git-credential-netrc
new file mode 100755
index 000..8298564
--- /dev/null
+++ b/contrib/credential/netrc/git-credential-netrc
@@ -0,0 +1,424 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings;
+
+use Getopt::Long;
+use File::Basename;
+
+my $VERSION = 0.1;
+
+my %options = (
+  help = 0,
+  debug = 0,
+  verbose = 0,
+  insecure = 0,
+  file = [],
+
+  # identical token maps, e.g. host - host, will be inserted later
+  tmap = {
+   port = 'protocol',
+   machine = 'host',
+   path = 'path',
+   login = 'username',
+   user = 'username',
+   password = 'password',
+  }
+ );
+
+# Map each credential protocol token to itself on the netrc side.
+foreach (values %{$options{tmap}}) {
+   $options{tmap}-{$_} = $_;
+}
+
+# Now, $options{tmap} has a mapping from the netrc format to the Git credential
+# helper protocol.
+
+# Next, we build the reverse token map.
+
+# When $rmap{foo} contains 'bar', that means that what the Git credential 
helper
+# protocol calls 'bar' is found as 'foo' in the netrc/authinfo file.  Keys in
+# %rmap are what we expect to read from the netrc/authinfo file.
+
+my %rmap;
+foreach my $k (keys %{$options{tmap}}) {
+   push @{$rmap{$options{tmap}-{$k}}}, $k;
+}
+
+Getopt::Long::Configure(bundling);
+
+# TODO: maybe allow the token map $options{tmap} to be configurable.
+GetOptions(\%options,
+   help|h,
+   debug|d,
+   insecure|k,
+   verbose|v,
+   file|f=s@,
+  );
+
+if ($options{help}) {
+   my $shortname = basename($0);
+   $shortname =~ s/git-credential-//;
+
+   print EOHIPPUS;
+
+$0 [-f AUTHFILE1] [-f AUTHFILEN] [-d] [-v] [-k] get
+
+Version $VERSION by tzz\@lifelogs.com.  License: BSD.
+
+Options:
+
+  -f|--file AUTHFILE : specify netrc-style files.  Files with the .gpg 
extension
+   will be decrypted by GPG before parsing.  Multiple -f
+   arguments are OK.  They are processed in order, and the
+   first matching entry found is returned via the 
credential
+   helper protocol (see below).
+
+  -k|--insecure  : ignore bad file ownership or permissions
+
+  -d|--debug : turn on debugging (developer info)
+
+  -v|--verbose   : be more verbose (show files and information found)
+
+To enable this credential helper:
+
+  git config credential.helper '$shortname -f AUTHFILE1 -f AUTHFILE2'
+
+(Note that Git will prepend git-credential- to the helper name and look for 
it
+in the path.)
+
+...and if you want lots of debugging info:
+
+  git config credential.helper '$shortname -f AUTHFILE -d'
+
+...or to see the files opened and data found:
+
+  git config credential.helper 

Re: Bug in git log --graph -p -m (version 1.7.7.6)

2013-02-05 Thread Matthieu Moy
Junio C Hamano gits...@pobox.com writes:

 wor...@alum.mit.edu (Dale R. Worley) writes:

 I have found a situation where git log produces (apparently)
 endless output.  Presumably this is a bug.  Following is a (Linux)
 script that reliably reproduces the error for me (on Fedora 16):

 Wasn't this fixed at v1.8.1.1~13 or is this a different issue?

In any case, I can't reproduce with 1.8.1.2.526.gf51a757: I don't get
undless output. On the other hand, I get a slightly misformatted output:

*   commit a393ed598e9fb11436f85bd58f1a38c82f2cadb7 (from 
2c1e6a36f4b712e914fac994463da7d0fdb2bc6d)
|\  Merge: 2c1e6a3 33e70e7
| | Author: Matthieu Moy matthieu@imag.fr
| | Date:   Tue Feb 5 22:05:33 2013 +0100
| | 
| | Commit S
| | 
| | diff --git a/file b/file
| | index 6bb4d3e..afd2e75 100644
| | --- a/file
| | +++ b/file
| | @@ -1,4 +1,5 @@
| |  1
| |  1a
| |  2
| | +2a
| |  3
| | 
commit a393ed598e9fb11436f85bd58f1a38c82f2cadb7 (from 
33e70e70c0173d634826b998bdc304f93c0966b8)
| | Merge: 2c1e6a3 33e70e7
| | Author: Matthieu Moy matthieu@imag.fr
| | Date:   Tue Feb 5 22:05:33 2013 +0100

The second commit line (diff with second parent) doesn't have the
| | prefix, I don't think this is intentional.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2] t4038: add tests for diff --cc --raw trees

2013-02-05 Thread John Keeping
Signed-off-by: John Keeping j...@keeping.me.uk

---
Changes since v1:

- more spaces around '|'
- create trees with line feeds and use 'sed -e 4q'
---
 t/t4038-diff-combined.sh | 24 
 1 file changed, 24 insertions(+)

diff --git a/t/t4038-diff-combined.sh b/t/t4038-diff-combined.sh
index 40277c7..614425a 100755
--- a/t/t4038-diff-combined.sh
+++ b/t/t4038-diff-combined.sh
@@ -89,4 +89,28 @@ test_expect_success 'diagnose truncated file' '
grep diff --cc file out
 '
 
+test_expect_success 'setup for --cc --raw' '
+   blob=$(echo file | git hash-object --stdin -w) 
+   base_tree=$(echo 100644 blob $blob file | git mktree) 
+   trees= 
+   for i in `test_seq 1 40`
+   do
+   blob=$(echo file$i | git hash-object --stdin -w) 
+   trees=$trees$(echo 100644 blob $blob  file | git mktree)$LF
+   done
+'
+
+test_expect_success 'check --cc --raw with four trees' '
+   four_trees=$(echo $trees | sed -e 4q) 
+   git diff --cc --raw $four_trees $base_tree out 
+   # Check for four leading colons in the output:
+   grep ^[^:] out
+'
+
+test_expect_success 'check --cc --raw with forty trees' '
+   git diff --cc --raw $trees $base_tree out 
+   # Check for forty leading colons in the output:
+   grep ^[^:] out
+'
+
 test_done
-- 
1.8.1.2.689.g36c777b

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] tests: turn on test-lint-shell-syntax by default

2013-02-05 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

 Torsten Bögershausen tbo...@web.de writes:

 Thanks for the detailed suggestion.
 Instead of using a file for putting out non portable syntax,
 can we can use a similar logic as test_failure ?

 Your test_bad_syntax_ function can be called from a subshell, and
 its exit 1 will not exit, no?

What is more important is that the increment to $test_bad_syntax
done in that function will not be propagated up to the main process
that runs the test framework.

Of course, that is why I mentioned communicating using the
filesystem.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCHv4] Add contrib/credentials/netrc with GPG support

2013-02-05 Thread Junio C Hamano
Ted Zlatanov t...@lifelogs.com writes:

 On Tue, 05 Feb 2013 11:53:20 -0800 Junio C Hamano gits...@pobox.com wrote: 

 I think it's more readable with large loops, and it actually makes sense
 when you read the code.  Not a big deal to me either, I just felt for
 this particular script it was OK.

 +   if ($file =~ m/\.gpg$/) {
 +   log_verbose(Using GPG to open $file);
 +   # GPG doesn't work well with 2- or 3-argument open

 JCH If that is the case, please quote $file properly against shell
 JCH munging it.

 Ahhh that gets ugly.  OK, quoted.

 JCH The only thing you do on $io is to read from it via while ($io),
 JCH so I would personally have written this part like this without
 JCH having to use IO::File(), though:

 JCH  $io = open(-|, qw(gpg --decrypt), $file);

 That doesn't work for me, unfortunately.  I'm trying to avoid the IPC::*
 modules and such.  Please test it yourself with GPG.  I'm on Perl
 5.14.2.

This works for me as expected (sorry for that open $io syntax
gotcha).

-- cut here -- 8 -- cut here --

#!/usr/bin/perl
my $io;
open $io, -|, qw(gpg --decrypt), $ARGV[0]
or die $!: gpg open;
while ($io) {
print;
}
close $io
or die $!: gpg close;

-- cut here -- 8 -- cut here --

$ perl --version
This is perl, v5.10.1 (*) built for x86_64-linux-gnu-thread-multi
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] t4038: add tests for diff --cc --raw trees

2013-02-05 Thread Junio C Hamano
Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: importing two different trees into a fresh git repo

2013-02-05 Thread Junio C Hamano
Constantine A. Murenin muren...@gmail.com writes:

 I have two distinct trees that were not managed by any RCS, and I'd
 like to import them into a single repository into two separate orphan
 branches, then make sense of what's in there, merge, and unify into
 'master'.

 (To give some context, it's /etc/nginx config files from nginx/1.0.12
 on Debian 6 and nginx/1.2.2 on OpenBSD 5.2.)

As these come from two totally separate sources, I'd find it more
natural to do two repositories, deb-nginx-conf and obsd-nginx-conf,
each with one commit and then pull one into the other (or pull both
to master-nginx-conf if you really wanted to), to me.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] git-send-email: add ~/.authinfo parsing

2013-02-05 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 On Tue, Jan 29, 2013 at 11:53:19AM -0800, Junio C Hamano wrote:

 Either way it still encourages a plaintext password to be on disk,
 which may not be what we want, even though it may be slight if not
 really much of an improvement.  Again the Help-for-users has this
 amusing bit:

 I do not mind a .netrc or .authinfo parser, because while those formats
 do have security problems, they are standard files that may already be
 in use. So as long as we are not encouraging their use, I do not see a
 problem in supporting them (and we already do the same with curl's netrc
 support).

 But it would probably make sense for send-email to support the existing
 git-credential subsystem, so that it can take advantage of secure
 system-specific storage. And that is where we should be pointing new
 users. I think contrib/mw-to-git even has credential support written in
 perl, so it would just need to be factored out to Git.pm.

I see a lot of rerolls on the credential helper front, but is there
anybody working on hooking send-email to the credential framework?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Rebased git-subtree changes

2013-02-05 Thread Junio C Hamano
This looks to be of mixed quality.  The earlier ones look fairly
finished, while the later ones not so much.

I am tempted to take up to 06/13 and advance them to 'next', without
the rest.


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCHv5] Add contrib/credentials/netrc with GPG support

2013-02-05 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

 Otherwise, looks almost ready to me.

For now, I've queued this as a minimum fix-up on top of your patch
and pushed the result out.  It is an equivalent of the previous
review comments in a patch form.  Please review and incorporate in
your reroll as appropriate.

I haven't looked at the part that interacts with the credential
subsystem itself, though.

 contrib/credential/netrc/git-credential-netrc | 35 ---
 1 file changed, 16 insertions(+), 19 deletions(-)

diff --git a/contrib/credential/netrc/git-credential-netrc 
b/contrib/credential/netrc/git-credential-netrc
index 8298564..30e05fb 100755
--- a/contrib/credential/netrc/git-credential-netrc
+++ b/contrib/credential/netrc/git-credential-netrc
@@ -74,6 +74,10 @@ Options:
first matching entry found is returned via the 
credential
helper protocol (see below).
 
+   When no -f option is given, .authinfo.gpg, .netrc.gpg,
+  .authinfo, and .netrc files in your home directory are 
used
+  in this order.
+
   -k|--insecure  : ignore bad file ownership or permissions
 
   -d|--debug : turn on debugging (developer info)
@@ -206,8 +210,7 @@ foreach my $file (@$files) {
unless (scalar @entries) {
if ($!) {
log_verbose(Unable to open $file: $!);
-   }
-   else {
+   } else {
log_verbose(No netrc entries found in $file);
}
 
@@ -230,15 +233,10 @@ sub load_netrc {
 
my $io;
if ($gpgmode) {
-   # typical shell character escapes from 
http://www.slac.stanford.edu/slac/www/resource/how-to-use/cgi-rexx/cgi-esc.html
-   my $f = $file;
-   $f =~ s/([;\*\|`\$!#\(\)\[\]\{\}:'])/\\$1/g;
-   # GPG doesn't work well with 2- or 3-argument open
-   my $cmd = gpg --decrypt $f;
-   log_verbose(Using GPG to open $file: [$cmd]);
-   open $io, $cmd|;
-   }
-   else {
+   my @cmd = (qw(gpg --decrypt), $file)
+   log_verbose(Using GPG to open $file: [@cmd]);
+   open $io, -|, @cmd;
+   } else {
log_verbose(Opening $file...);
open $io, '', $file;
}
@@ -257,6 +255,9 @@ sub load_netrc {
my %entry;
my $num_port;
 
+   if (!defined $nentry-{machine}) {
+   next;
+   }
if (defined $nentry-{port}  $nentry-{port} =~ m/^\d+$/) {
$num_port = $nentry-{port};
delete $nentry-{port};
@@ -302,8 +303,7 @@ sub net_netrc_loader {
while (@tok) {
if ($tok[0] eq default) {
shift(@tok);
-   undef $mach; # ignore 'default' lines
-
+   $mach = { machine = undef }
next TOKEN;
}
 
@@ -313,8 +313,7 @@ sub net_netrc_loader {
my $host = shift @tok;
$mach = { machine = $host };
push @entries, $mach;
-   }
-   elsif (exists $options{tmap}-{$tok}) {
+   } elsif (exists $options{tmap}-{$tok}) {
unless ($mach) {
log_debug(Skipping token $tok because 
no machine was given);
next TOKEN;
@@ -329,8 +328,7 @@ sub net_netrc_loader {
# Following line added by rmerrell to remove 
'/' escape char in .netrc
$value =~ s/\/\\/\\/g;
$mach-{$tok} = $value;
-   }
-   elsif ($tok eq macdef) { # we ignore macros
+   } elsif ($tok eq macdef) { # we ignore macros
next TOKEN unless $mach;
my $value = shift @tok;
$macdef = 1;
@@ -380,8 +378,7 @@ sub find_netrc_entry {
unless ($query-{$check} eq $entry-{$check}) {
next ENTRY;
}
-   }
-   else {
+   } else {
log_debug(OK: any value satisfies check 
$check);
}
}
-- 
1.8.1.2.641.g0b90ac4

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 2/2] i18n: mark OPTION_NUMBER (-NUM) for translation

2013-02-05 Thread Jiang Xin
2013/2/6 Junio C Hamano gits...@pobox.com:
 I somehow suspect that this is going in a direction that makes this
 piece of code much less maintainable.

 Look at the entire function and see how many places you do fprintf
 on strings that are marked with _().  short_name and long_name are
 not likely to be translated, but everything else is, especially
 multiple places that show _(opts-help) neither of these patches
 touch.

 I wonder if it makes more sense to add a helper function that
 returns the number of column positions (not bytes) with a signature
 similar to fprintf() and use that throughout the function instead.

I agree, a helper named 'utf8_fprintf' in utf8.c is better.
I will send a patch latter.


-- 
Jiang Xin
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: importing two different trees into a fresh git repo

2013-02-05 Thread Constantine A. Murenin
On 5 February 2013 14:29, Junio C Hamano gits...@pobox.com wrote:
 Constantine A. Murenin muren...@gmail.com writes:

 I have two distinct trees that were not managed by any RCS, and I'd
 like to import them into a single repository into two separate orphan
 branches, then make sense of what's in there, merge, and unify into
 'master'.

 (To give some context, it's /etc/nginx config files from nginx/1.0.12
 on Debian 6 and nginx/1.2.2 on OpenBSD 5.2.)

 As these come from two totally separate sources, I'd find it more
 natural to do two repositories, deb-nginx-conf and obsd-nginx-conf,
 each with one commit and then pull one into the other (or pull both
 to master-nginx-conf if you really wanted to), to me.

Yeah, I guess it might be more of a git-style to have two/three
separate repositories here.  (The sources are just a couple of files,
so I think my specific example still calls for merely two orphan
branches.)

Still, is it really expected that you can't create an orphan branch in
an empty repository?  On the outside, this sounds like a rather benign
bug.

C.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCHv6] Add contrib/credentials/netrc with GPG support

2013-02-05 Thread Ted Zlatanov
Add Git credential helper that can parse netrc/authinfo files.

This credential helper supports multiple files, returning the first one
that matches.  It checks file permissions and owner.  For *.gpg files,
it will run GPG to decrypt the file.

Signed-off-by: Ted Zlatanov t...@lifelogs.com
---
Changes since PATCHv5:

- reroll from tz/credential-authinfo:
 * brace fixes
 * list attempted default files in help doc
 * 3-arg open() for the GPG pipe
 * instead of skipping 'default' tokens, store them as machine = undef

 contrib/credential/netrc/Makefile |   12 +
 contrib/credential/netrc/git-credential-netrc |  421 +
 2 files changed, 433 insertions(+), 0 deletions(-)
 create mode 100644 contrib/credential/netrc/Makefile
 create mode 100755 contrib/credential/netrc/git-credential-netrc

diff --git a/contrib/credential/netrc/Makefile 
b/contrib/credential/netrc/Makefile
new file mode 100644
index 000..18a924f
--- /dev/null
+++ b/contrib/credential/netrc/Makefile
@@ -0,0 +1,12 @@
+test_netrc:
+   @(echo bad data | ./git-credential-netrc -f A -d -v) || echo Bad 
invocation test, ignoring failure
+   @echo = Silent invocation... nothing should show up here with a 
missing file
+   @echo bad data | ./git-credential-netrc -f A get
+   @echo = Back to noisy: -v and -d used below, missing file
+   echo bad data | ./git-credential-netrc -f A -d -v get
+   @echo = Look for any entry in the default file set
+   echo  | ./git-credential-netrc -d -v get
+   @echo = Look for github.com in the default file set
+   echo host=google.com | ./git-credential-netrc -d -v get
+   @echo = Look for a nonexistent machine in the default file set
+   echo host=korovamilkbar | ./git-credential-netrc -d -v get
diff --git a/contrib/credential/netrc/git-credential-netrc 
b/contrib/credential/netrc/git-credential-netrc
new file mode 100755
index 000..30e05fb
--- /dev/null
+++ b/contrib/credential/netrc/git-credential-netrc
@@ -0,0 +1,421 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings;
+
+use Getopt::Long;
+use File::Basename;
+
+my $VERSION = 0.1;
+
+my %options = (
+  help = 0,
+  debug = 0,
+  verbose = 0,
+  insecure = 0,
+  file = [],
+
+  # identical token maps, e.g. host - host, will be inserted later
+  tmap = {
+   port = 'protocol',
+   machine = 'host',
+   path = 'path',
+   login = 'username',
+   user = 'username',
+   password = 'password',
+  }
+ );
+
+# Map each credential protocol token to itself on the netrc side.
+foreach (values %{$options{tmap}}) {
+   $options{tmap}-{$_} = $_;
+}
+
+# Now, $options{tmap} has a mapping from the netrc format to the Git credential
+# helper protocol.
+
+# Next, we build the reverse token map.
+
+# When $rmap{foo} contains 'bar', that means that what the Git credential 
helper
+# protocol calls 'bar' is found as 'foo' in the netrc/authinfo file.  Keys in
+# %rmap are what we expect to read from the netrc/authinfo file.
+
+my %rmap;
+foreach my $k (keys %{$options{tmap}}) {
+   push @{$rmap{$options{tmap}-{$k}}}, $k;
+}
+
+Getopt::Long::Configure(bundling);
+
+# TODO: maybe allow the token map $options{tmap} to be configurable.
+GetOptions(\%options,
+   help|h,
+   debug|d,
+   insecure|k,
+   verbose|v,
+   file|f=s@,
+  );
+
+if ($options{help}) {
+   my $shortname = basename($0);
+   $shortname =~ s/git-credential-//;
+
+   print EOHIPPUS;
+
+$0 [-f AUTHFILE1] [-f AUTHFILEN] [-d] [-v] [-k] get
+
+Version $VERSION by tzz\@lifelogs.com.  License: BSD.
+
+Options:
+
+  -f|--file AUTHFILE : specify netrc-style files.  Files with the .gpg 
extension
+   will be decrypted by GPG before parsing.  Multiple -f
+   arguments are OK.  They are processed in order, and the
+   first matching entry found is returned via the 
credential
+   helper protocol (see below).
+
+   When no -f option is given, .authinfo.gpg, .netrc.gpg,
+  .authinfo, and .netrc files in your home directory are 
used
+  in this order.
+
+  -k|--insecure  : ignore bad file ownership or permissions
+
+  -d|--debug : turn on debugging (developer info)
+
+  -v|--verbose   : be more verbose (show files and information found)
+
+To enable this credential helper:
+
+  git config credential.helper '$shortname -f AUTHFILE1 -f AUTHFILE2'
+
+(Note that Git will prepend git-credential- to the helper name and look for 
it
+in the path.)
+
+...and if you want lots of debugging info:
+
+  git config credential.helper '$shortname -f AUTHFILE -d'
+
+...or to see the files opened and data found:
+
+  git 

[PATCH v3] Add utf8_fprintf helper which returns correct columns

2013-02-05 Thread Jiang Xin
Since command usages can be translated, they may not align well especially
when they are translated to CJK. A wrapper utf8_fprintf can help to return
the correct columns required.

Signed-off-by: Jiang Xin worldhello@gmail.com
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 parse-options.c |  5 +++--
 utf8.c  | 20 
 utf8.h  |  1 +
 3 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index 67e98..a6ce9e 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -3,6 +3,7 @@
 #include cache.h
 #include commit.h
 #include color.h
+#include utf8.h
 
 static int parse_options_usage(struct parse_opt_ctx_t *ctx,
   const char * const *usagestr,
@@ -482,7 +483,7 @@ static int usage_argh(const struct option *opts, FILE 
*outfile)
s = literal ? [%s] : [%s];
else
s = literal ?  %s :  %s;
-   return fprintf(outfile, s, opts-argh ? _(opts-argh) : _(...));
+   return utf8_fprintf(outfile, s, opts-argh ? _(opts-argh) : _(...));
 }
 
 #define USAGE_OPTS_WIDTH 24
@@ -541,7 +542,7 @@ static int usage_with_options_internal(struct 
parse_opt_ctx_t *ctx,
if (opts-long_name)
pos += fprintf(outfile, --%s, opts-long_name);
if (opts-type == OPTION_NUMBER)
-   pos += fprintf(outfile, -NUM);
+   pos += utf8_fprintf(outfile, _(-NUM));
 
if ((opts-flags  PARSE_OPT_LITERAL_ARGHELP) ||
!(opts-flags  PARSE_OPT_NOARG))
diff --git a/utf8.c b/utf8.c
index a4ee6..52dbd 100644
--- a/utf8.c
+++ b/utf8.c
@@ -430,6 +430,26 @@ int same_encoding(const char *src, const char *dst)
 }
 
 /*
+ * Wrapper for fprintf and returns the total number of columns required
+ * for the printed string, assuming that the string is utf8.
+ */
+int utf8_fprintf(FILE *stream, const char *format, ...)
+{
+   struct strbuf buf = STRBUF_INIT;
+   va_list arg;
+   int columns;
+
+   va_start (arg, format);
+   strbuf_vaddf(buf, format, arg);
+   va_end (arg);
+
+   fputs(buf.buf, stream);
+   columns = utf8_strwidth(buf.buf);
+   strbuf_release(buf);
+   return columns;
+}
+
+/*
  * Given a buffer and its encoding, return it re-encoded
  * with iconv.  If the conversion fails, returns NULL.
  */
diff --git a/utf8.h b/utf8.h
index a2142..501b2 100644
--- a/utf8.h
+++ b/utf8.h
@@ -8,6 +8,7 @@ int utf8_strwidth(const char *string);
 int is_utf8(const char *text);
 int is_encoding_utf8(const char *name);
 int same_encoding(const char *, const char *);
+int utf8_fprintf(FILE *, const char *, ...);
 
 void strbuf_add_wrapped_text(struct strbuf *buf,
const char *text, int indent, int indent2, int width);
-- 
1.8.1.1.370.g47b6ee8.dirty

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 2/2] i18n: mark OPTION_NUMBER (-NUM) for translation

2013-02-05 Thread Junio C Hamano
Jiang Xin worldhello@gmail.com writes:

 2013/2/6 Junio C Hamano gits...@pobox.com:
 I somehow suspect that this is going in a direction that makes this
 piece of code much less maintainable.

 Look at the entire function and see how many places you do fprintf
 on strings that are marked with _().  short_name and long_name are
 not likely to be translated, but everything else is, especially
 multiple places that show _(opts-help) neither of these patches
 touch.

 I wonder if it makes more sense to add a helper function that
 returns the number of column positions (not bytes) with a signature
 similar to fprintf() and use that throughout the function instead.

 I agree, a helper named 'utf8_fprintf' in utf8.c is better.
 I will send a patch latter.

Yeah, the idea of a helper function I agree with; I am not thrilled
with the name utf8_fprintf() though.  People use the return value of
fprintf() for error detection (negative return value means an error)
most of the time (even though non-negative value gives the number of
bytes shown), but the primary use of the return value from the
utf8_fprintf() function will be to get the display width, and the
name does not quite capture that.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 2/2] i18n: mark OPTION_NUMBER (-NUM) for translation

2013-02-05 Thread Jiang Xin
2013/2/6 Junio C Hamano gits...@pobox.com:
 Jiang Xin worldhello@gmail.com writes:
 I agree, a helper named 'utf8_fprintf' in utf8.c is better.
 I will send a patch latter.

 Yeah, the idea of a helper function I agree with; I am not thrilled
 with the name utf8_fprintf() though.  People use the return value of
 fprintf() for error detection (negative return value means an error)
 most of the time (even though non-negative value gives the number of
 bytes shown), but the primary use of the return value from the
 utf8_fprintf() function will be to get the display width, and the
 name does not quite capture that.


How about this since [PATCH v3]:

diff --git a/utf8.c b/utf8.c
index 52dbd..b893a 100644
--- a/utf8.c
+++ b/utf8.c
@@ -443,8 +443,11 @@ int utf8_fprintf(FILE *stream, const char *format, ...)
strbuf_vaddf(buf, format, arg);
va_end (arg);

-   fputs(buf.buf, stream);
-   columns = utf8_strwidth(buf.buf);
+   columns = fputs(buf.buf, stream);
+   /* If no error occurs, and really write something (columns  0),
+* calculate really columns width with utf8_strwidth. */
+   if (columns  0)
+   columns = utf8_strwidth(buf.buf);
strbuf_release(buf);
return columns;
 }


-- 
蒋鑫

北京群英汇信息技术有限公司
邮件: worldhello@gmail.com
网址: http://www.ossxp.com/
博客: http://www.worldhello.net/
微博: http://weibo.com/gotgit/
电话: 010-51262007, 18601196889
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 2/2] i18n: mark OPTION_NUMBER (-NUM) for translation

2013-02-05 Thread Junio C Hamano
Jiang Xin worldhello@gmail.com writes:

 2013/2/6 Junio C Hamano gits...@pobox.com:
 Jiang Xin worldhello@gmail.com writes:
 I agree, a helper named 'utf8_fprintf' in utf8.c is better.
 I will send a patch latter.

 Yeah, the idea of a helper function I agree with; I am not thrilled
 with the name utf8_fprintf() though.  People use the return value of
 fprintf() for error detection (negative return value means an error)
 most of the time (even though non-negative value gives the number of
 bytes shown), but the primary use of the return value from the
 utf8_fprintf() function will be to get the display width, and the
 name does not quite capture that.


 How about this since [PATCH v3]:

 diff --git a/utf8.c b/utf8.c
 index 52dbd..b893a 100644
 --- a/utf8.c
 +++ b/utf8.c
 @@ -443,8 +443,11 @@ int utf8_fprintf(FILE *stream, const char *format, ...)
 strbuf_vaddf(buf, format, arg);
 va_end (arg);

 -   fputs(buf.buf, stream);
 -   columns = utf8_strwidth(buf.buf);
 +   columns = fputs(buf.buf, stream);
 +   /* If no error occurs, and really write something (columns  0),
 +* calculate really columns width with utf8_strwidth. */
 +   if (columns  0)
 +   columns = utf8_strwidth(buf.buf);
 strbuf_release(buf);
 return columns;
  }

Yeah, the error checking is necessary; it would make your intention
more clear to say:

if (0 = columns)
columns = utf8_strwidth(buf.buf);

though, as buf.buf _may_ be an empty string, and with the if
statement you are saying we return the width only when output did
not result in an error.

The above bugfix does not address my original concern about
the name, though.



--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html