Re: [PATCH v3 0/8] Hiding refs
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
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
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
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
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?
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
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
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?
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?
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
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 ?
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
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)
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
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?
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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?
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?
Æ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
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)
Æ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
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
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
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
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
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
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
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
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
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
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)
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
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
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
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
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)
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
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
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
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
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
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)
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
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
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
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
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
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
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
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
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)
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
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
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
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
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
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
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
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
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/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
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
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
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
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/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
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