* Re: [PATCH] git-send-email: add ~/.authinfo parsing
From: Matthieu Moy @ 2013-02-06 8:11 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jeff King, Michal Nazarewicz, git, Krzysztof Mazur,
Michal Nazarewicz
In-Reply-To: <7v6226pdb7.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> I see a lot of rerolls on the credential helper front, but is there
> anybody working on hooking send-email to the credential framework?
Not answering the question, but git-remote-mediawiki supports the
credential framework. It is written in perl, and the credential support
is rather cleanly written and doesn't have dependencies on the wiki
part, so the way to go for send-email is probably to libify the
credential support in git-remote-mediawiki, and to use it in send-email.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: importing two different trees into a fresh git repo
From: Jeff King @ 2013-02-06 9:07 UTC (permalink / raw)
To: Constantine A. Murenin; +Cc: git
In-Reply-To: <CAPKkNb6+ojb+uvBW+AkhGrhjR85LrJEbmR0KmvaKYb2Cj5Aa4g@mail.gmail.com>
On Tue, Feb 05, 2013 at 01:46:09PM -0800, Constantine A. Murenin wrote:
> I've encountered two problems so far:
>
> 0. After initialising the repository, I was unable to `git checkout
> --orphan Debian-6.0.4-nginx-1.0.12` -- presumably it doesn't work when
> the repo is empty? This sounds like a bug or an artefact of
> implementation. I presume this can be worked around by committing
> into master instead, and then doing `git checkout -b
> Debian-6.0.4-nginx-1.0.12`, and then force-fixing the master somehow
> later on.
What version of git are you using? Using both "-b" and "--orphan" from a
non-existing branch used to be broken, but was fixed by abe1998 (git
checkout -b: allow switching out of an unborn branch, 2012-01-30), which
first appeared in git v1.7.9.2.
> 1. After making a mistake on my first commit (my first commit into
> OpenBSD-5.2-nginx-1.2.2 orphan branch ended up including a directory
> from master by mistake), I am now unable to rebase and "fixup" the
> changes -- `git rebase --interactive HEAD~2` doesn't work, which, from
> one perspective, makes perfect sense (indeed there's no prior
> revision), but, from another, it's not immediately obvious how to
> quickly work around it.
You cannot ask to rebase onto HEAD~2 because it does not exist (I'm assuming
from your description that HEAD~1 is the root of your repository). But
you can use the "--root" flag to ask git to rebase all the way down to
the roots, like:
git rebase -i --root
However, note that older versions of git do not support using "--root"
with "-i". The first usable version is v1.7.12.
-Peff
^ permalink raw reply
* Re: git-svn problems with white-space in tag names
From: Erik Faye-Lund @ 2013-02-06 9:26 UTC (permalink / raw)
To: Hans-Juergen Euler; +Cc: git
In-Reply-To: <CAK3CF+46a2LsrZ0ef5Bc2XfwCVv1LVk6YbMasa6e5kx-Dp3JTg@mail.gmail.com>
On Sun, Jan 27, 2013 at 3:12 PM, Hans-Juergen Euler <waas.nett@gmail.com> wrote:
> This seems to be a problem of the windows version. At least with its
> complete severity. Installed git on Ubuntu in a virtual machine was
> able to clone the subversion repos past the tag with the white-space
> at the end. I am not sure but apparently this tag has not been
> converted.
>
> The git repos I could copy from Ubuntu to windows. So far no problems
> seen in this copy on windows.
>
The cause of the problem is that the tagname isn't a valid filename on
Windows, which git requires it to be.
^ permalink raw reply
* Re: [RFC] test-lib.sh: No POSIXPERM for cygwin
From: Erik Faye-Lund @ 2013-02-06 9:34 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: ramsay, git, j6t
In-Reply-To: <201301271557.08994.tboegi@web.de>
On Sun, Jan 27, 2013 at 3:57 PM, Torsten Bögershausen <tboegi@web.de> wrote:
> t0070 and t1301 fail when running the test suite under cygwin.
> Skip the failing tests by unsetting POSIXPERM.
>
But is this the real reason? I thought Cygwin implemented POSIX permissions...?
^ permalink raw reply
* Re: [PATCH v3 0/8] Hiding refs
From: Michael Haggerty @ 2013-02-06 10:17 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King, Shawn Pearce
In-Reply-To: <7v8v72u0vw.fsf@alter.siamese.dyndns.org>
On 02/05/2013 06:27 PM, Junio C Hamano wrote:
> Michael Haggerty <mhagger@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 take "will merge to next" pretty seriously, because I know how hard it
is to get *my* patch series to this state :-)
>> * 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).
There seem to be a few issues mixed up in this topic. It is hard to
reason about your patch series without understanding which scenarios and
problems it is meant to address. First the problems that we might like
to solve:
Clutter: The typical user is subjected to much unneeded clutter in the
form of references that he/she will likely never use.
Bandwidth: Interactions with the remote repo (clone, fetch, etc) are
slowed down by the large volume of unnecessary data.
Provenance: Users mistakenly think that content originates with the
repository owner whereas it in fact came from some other
(perhaps untrusted) source.
Now, what are some use-case scenarios in which these problems arise? As
I understand it, there are a few:
Scenario 1: Some providers junk up their users' repositories with
content that is not created by the repository's owner and that the owner
doesn't want to appear to vouch for (e.g., GitHub pull requests). These
references might sometimes be useful to fetch, singly or in bulk.
Scenario 2: Some systems junk up their users' repositories with
additional references that are not interesting to most pullers (e.g.,
Gerrit activity markers) though they don't add questionable content.
Scenario 3: Some repository owners might *themselves* want to push
references to their repository but hide them from most users (e.g.,
Junio's topic branches) or make them completely hidden from the rest of
the world (e.g., proprietary vs. open-source branches).
In most of these cases, it would be desirable for at least some users to
be able to fetch and/or push hidden content.
A first weakness of your proposal is that even though the hidden refs
are (optionally) fetchable, there is *no* way to discover them remotely
or to bulk-download them; they would have to be retrieved one by one
using out-of-band information. And if I understand correctly, there
would be no way to push hidden references remotely (whether manually or
from some automated process). Such processes would have to be local to
the machine holding the repository.
A second weakness of your proposal is that the repository owner would
*anyway* need local access to the repo server or the help of the
provider to implement reference hiding (since hidden references cannot
be configured remotely). Who will choose what references to hide? Most
likely each provider will pick a one-size-fits-all configuration and
apply it to all of the repos that they manage. All users would be at
the mercy of their provider to make wise choices and would not be able
to override the choice via their client.
A third weakness of your hidden references proposal is that it is
schizophrenic: some references are hidden and undiscoverable, but their
content can nevertheless be made fetchable if the user happens to know
the SHA1. This is more complicated to understand and reason about than
the rule "exactly the content that is referred to by published
references is fetchable".
What would be a better way? Providers could expose multiple views of
the same repository; for example, one view with just the uncluttered
content, and a second view that includes *all* fetchable references.
Accessing the repository via the first view would give all of the
benefits provided by your hidden reference proposal. Accessing it via
the second view would allow the hidden references to be fetched (even in
bulk) using purely git tools. The documentation for the second view
could explain that it contains un-vetted content.
But your proposal does not admit two-tiered access to a single
repository. You only support one hidden reference configuration that is
applied to all remote access [1]. See below for more ideas about
implementing multiple views.
>> * 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.
Yes, if the feature is turned off then it is not worse than the status
quo. But what if the feature is turned on?
Actually, I'm still not clear about how these hidden references are
supposed to be created. I know that you would forbid updating or
deleting hidden references via the remote protocol, but would you allow
them to be created? If so, then it seems that any pusher can create
dark content. Or can they only be created via a separate, local channel
to the repository? In this case, it seems rather limiting that any
process that wants to create hidden references has to be local.
>> * 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?
There are several possibilities:
1. Assuming the cooperation of the provider, the provider could offer
two separate URLs: one for the uncluttered view and one for the
cluttered view. The client would choose the view by choosing which URL
to clone from. On the provider side, both of these URLs could refer to
the same Git repository but, for example, set an environment variable
GIT_VIEW differently depending on which URL was used. This approach
would solve clutter, bandwidth and provenance but require cooperation
from the provider.
2a. Assuming no cooperation from the provider, the git client could have
options like "git fetch --view=uncluttered URL". This would receive all
references from the server but discard any that are not included in the
client's "uncluttered" view definition. This would solve clutter.
2b. Again assuming no cooperation from the provider, the user could
clone all references from the remote repo, but define a local
"uncluttered" view that hides the extra references on the local side.
The view could be selected by setting a local environment variable
GIT_VIEW or via configuration option "git config view.default
uncluttered". This would solve clutter in a more flexible way because
the clutter would still be available locally for those occasions when
the user wants to see it.
Please note that none of the above options require a new remote protocol.
If/when a new protocol is implemented, then the client could tell the
server what view it wants and the server would only advertise those refs
to the client:
3a. The client could tell the host what reference namespaces it wants to
fetch. Its choice would only be used for the single transaction and
would not be recorded on the server side.
3b. The client could pick a server-defined view by name. The server
would look up the name in its own configuration to translate it into a
subset of references. The views that a particular server supports would
be documented in the same place that the URL is documented and might
also be queryable by the client. There should probably be some standard
views like "default" and "full" that every server would be expected to
implement. Please note that this method can fall back to 2a when
communicating with a server that does not support the new protocol.
>> * 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".
Certainly they shouldn't be hidden from "gc", but it would be useful to
be able to hide references from user-facing commands like "log --all",
"log --decorate", "gitk", "grep --all" etc. For example, here are some
more scenarios where clutter is annoying:
Scenario 4: I occasionally share with colleague Foo, so I want to
configure his repo as a remote for mine and fetch his latest work:
git remote add foo $URL
git fetch foo
But now every time I do a "gitk --all" or "git log --decorate", the
output is cluttered with all of his references (most of which are just
old versions of references from the upstream repository that we both
use). I would like to be able to hide his references most of the time
but turn them back on when I need them.
Scenario 5: Our upstream repository has gazillions of release tags under
"refs/tags/releases/...", sometimes including customer-specific
releases. In my daily life these are just clutter. (This scenario is
made worse by the fact that AFAIK there is no way to tell Git to fetch
some tags but not others others.) But sometimes I need to track down a
bug in a particular release and need to access that release tag. So it
would be nice to be able to hide and unhide them locally.
> I appreciate the comments, but I do not think any point you raised
> in this message is very much relevant as objections.
Tl;dr summary:
* Hidden refs don't give a way to offer two-tiered remote access to a
repository (e.g., one uncluttered view and one full view), so
* local access to the repository would (apparently) be required to
put *anything* in the hidden namespaces.
* they don't help in any scenario where you *sometimes*
want to bulk fetch the hidden refs, and even make it awkward to
fetch single hidden refs.
* Hidden refs introduce a confusing schizophrenia between "advertised"
and "not advertised but nonetheless fetchable".
* Hidden refs require the cooperation of the provider to configure and
will therefore be unusable by many repository owners.
* Some small improvements (e.g. allowing *multiple* views to be
defined) would provide much more benefit for about the same effort,
and would be a better base for building other features in the future
(e.g., local views).
Thanks for listening.
Michael
[1] Theoretically one could support multiple views of a single
repository by using something like "GIT_CONFIG=view_1_config git
upload-pack ..." or "git -c transfer.hiderefs=... git upload-pack ...",
but this would be awkward.
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: [PATCH v3 0/8] Hiding refs
From: Duy Nguyen @ 2013-02-06 10:34 UTC (permalink / raw)
To: Michael Haggerty
Cc: Jonathan Nieder, Junio C Hamano, git, Jeff King, Shawn Pearce
In-Reply-To: <5110DF1D.8010505@alum.mit.edu>
On Tue, Feb 5, 2013 at 5:29 PM, Michael Haggerty <mhagger@alum.mit.edu> wrote:
> 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.
Or you can think hiderefs is the first step to addressing the initial
ref advertisment problem. The series says hidden refs are to be
fetched out of band, but that's not the only way. A new extension can
be added to the protocol later to let the client explore this dark
space. It's only truly dark for old clients. We could even shed some
light to old clients by sending a dummy ref with some loud name like
PLEASE_UPDATE_TO_LATEST_GIT_TO_FETCH_REMAINING_REFS (new clients
silently drop this ref)
--
Duy
^ permalink raw reply
* Re: [PATCH] Verify Content-Type from smart HTTP servers
From: Michael Schubert @ 2013-02-06 10:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Shawn Pearce, git, Jeff King
In-Reply-To: <7v38xhf1i3.fsf@alter.siamese.dyndns.org>
On 01/31/2013 11:09 PM, Junio C Hamano wrote:
>
> -static int http_request_reauth(const char *url, void *result, int target,
> +static int http_request_reauth(const char *url,
> + struct strbuf *type,
> + void *result, int target,
> int options)
> {
> - int ret = http_request(url, result, target, options);
> + int ret = http_request(url, type, result, target, options);
> if (ret != HTTP_REAUTH)
> return ret;
> - return http_request(url, result, target, options);
> + return http_request(url, type, result, target, options);
> }
This needs something like
diff --git a/http.c b/http.c
index d868d8b..da43be3 100644
--- a/http.c
+++ b/http.c
@@ -860,6 +860,8 @@ static int http_request_reauth(const char *url,
int ret = http_request(url, type, result, target, options);
if (ret != HTTP_REAUTH)
return ret;
+ if (type)
+ strbuf_reset(type);
return http_request(url, type, result, target, options);
}
on top. Otherwise we get
"text/plainapplication/x-git-receive-pack-advertisement"
when doing HTTP auth.
Thanks.
> -int http_get_strbuf(const char *url, struct strbuf *result, int options)
> +int http_get_strbuf(const char *url,
> + struct strbuf *type,
> + struct strbuf *result, int options)
> {
> - return http_request_reauth(url, result, HTTP_REQUEST_STRBUF, options);
> + return http_request_reauth(url, type, result,
> + HTTP_REQUEST_STRBUF, options);
> }
>
> /*
> @@ -878,7 +891,7 @@ static int http_get_file(const char *url, const char *filename, int options)
> goto cleanup;
> }
>
> - ret = http_request_reauth(url, result, HTTP_REQUEST_FILE, options);
> + ret = http_request_reauth(url, NULL, result, HTTP_REQUEST_FILE, options);
> fclose(result);
>
> if ((ret == HTTP_OK) && move_temp_to_file(tmpfile.buf, filename))
> @@ -904,7 +917,7 @@ int http_fetch_ref(const char *base, struct ref *ref)
> int ret = -1;
>
> url = quote_ref_url(base, ref->name);
> - if (http_get_strbuf(url, &buffer, HTTP_NO_CACHE) == HTTP_OK) {
> + if (http_get_strbuf(url, NULL, &buffer, HTTP_NO_CACHE) == HTTP_OK) {
> strbuf_rtrim(&buffer);
> if (buffer.len == 40)
> ret = get_sha1_hex(buffer.buf, ref->old_sha1);
> @@ -997,7 +1010,7 @@ int http_get_info_packs(const char *base_url, struct packed_git **packs_head)
> strbuf_addstr(&buf, "objects/info/packs");
> url = strbuf_detach(&buf, NULL);
>
> - ret = http_get_strbuf(url, &buf, HTTP_NO_CACHE);
> + ret = http_get_strbuf(url, NULL, &buf, HTTP_NO_CACHE);
> if (ret != HTTP_OK)
> goto cleanup;
>
> diff --git a/http.h b/http.h
> index 0a80d30..25d1931 100644
> --- a/http.h
> +++ b/http.h
> @@ -132,7 +132,7 @@ extern char *get_remote_object_url(const char *url, const char *hex,
> *
> * If the result pointer is NULL, a HTTP HEAD request is made instead of GET.
> */
> -int http_get_strbuf(const char *url, struct strbuf *result, int options);
> +int http_get_strbuf(const char *url, struct strbuf *content_type, struct strbuf *result, int options);
>
> /*
> * Prints an error message using error() containing url and curl_errorstr,
> diff --git a/remote-curl.c b/remote-curl.c
> index 9a8b123..e6f3b63 100644
> --- a/remote-curl.c
> +++ b/remote-curl.c
> @@ -92,6 +92,8 @@ static void free_discovery(struct discovery *d)
>
> static struct discovery* discover_refs(const char *service)
> {
> + struct strbuf exp = STRBUF_INIT;
> + struct strbuf type = STRBUF_INIT;
> struct strbuf buffer = STRBUF_INIT;
> struct discovery *last = last_discovery;
> char *refs_url;
> @@ -113,7 +115,7 @@ static struct discovery* discover_refs(const char *service)
> }
> refs_url = strbuf_detach(&buffer, NULL);
>
> - http_ret = http_get_strbuf(refs_url, &buffer, HTTP_NO_CACHE);
> + http_ret = http_get_strbuf(refs_url, &type, &buffer, HTTP_NO_CACHE);
> switch (http_ret) {
> case HTTP_OK:
> break;
> @@ -133,16 +135,19 @@ static struct discovery* discover_refs(const char *service)
> last->buf = last->buf_alloc;
>
> if (maybe_smart && 5 <= last->len && last->buf[4] == '#') {
> - /* smart HTTP response; validate that the service
> + /*
> + * smart HTTP response; validate that the service
> * pkt-line matches our request.
> */
> - struct strbuf exp = STRBUF_INIT;
> -
> + strbuf_addf(&exp, "application/x-%s-advertisement", service);
> + if (strbuf_cmp(&exp, &type))
> + die("invalid content-type %s", type.buf);
> if (packet_get_line(&buffer, &last->buf, &last->len) <= 0)
> die("%s has invalid packet header", refs_url);
> if (buffer.len && buffer.buf[buffer.len - 1] == '\n')
> strbuf_setlen(&buffer, buffer.len - 1);
>
> + strbuf_reset(&exp);
> strbuf_addf(&exp, "# service=%s", service);
> if (strbuf_cmp(&exp, &buffer))
> die("invalid server response; got '%s'", buffer.buf);
> @@ -160,6 +165,8 @@ static struct discovery* discover_refs(const char *service)
> }
>
> free(refs_url);
> + strbuf_release(&exp);
> + strbuf_release(&type);
> strbuf_release(&buffer);
> last_discovery = last;
> return last;
^ permalink raw reply related
* Re: [PATCH] Verify Content-Type from smart HTTP servers
From: Jeff King @ 2013-02-06 10:39 UTC (permalink / raw)
To: Michael Schubert; +Cc: Junio C Hamano, Shawn Pearce, git
In-Reply-To: <51122F69.9060704@elegosoft.com>
On Wed, Feb 06, 2013 at 11:24:41AM +0100, Michael Schubert wrote:
> On 01/31/2013 11:09 PM, Junio C Hamano wrote:
>
> >
> > -static int http_request_reauth(const char *url, void *result, int target,
> > +static int http_request_reauth(const char *url,
> > + struct strbuf *type,
> > + void *result, int target,
> > int options)
> > {
> > - int ret = http_request(url, result, target, options);
> > + int ret = http_request(url, type, result, target, options);
> > if (ret != HTTP_REAUTH)
> > return ret;
> > - return http_request(url, result, target, options);
> > + return http_request(url, type, result, target, options);
> > }
>
> This needs something like
>
> diff --git a/http.c b/http.c
> index d868d8b..da43be3 100644
> --- a/http.c
> +++ b/http.c
> @@ -860,6 +860,8 @@ static int http_request_reauth(const char *url,
> int ret = http_request(url, type, result, target, options);
> if (ret != HTTP_REAUTH)
> return ret;
> + if (type)
> + strbuf_reset(type);
> return http_request(url, type, result, target, options);
> }
>
> on top. Otherwise we get
>
> "text/plainapplication/x-git-receive-pack-advertisement"
>
> when doing HTTP auth.
Good catch. It probably makes sense to put it in http_request, so that
we also protect against any existing cruft from the callers of
http_get_*, like:
-- >8 --
Subject: [PATCH] http_request: reset "type" strbuf before adding
Callers may pass us a strbuf which we use to record the
content-type of the response. However, we simply appended to
it rather than overwriting its contents, meaning that cruft
in the strbuf gave us a bogus type. E.g., the multiple
requests triggered by http_request could yield a type like
"text/plainapplication/x-git-receive-pack-advertisement".
Reported-by: Michael Schubert <mschub@elegosoft.com>
Signed-off-by: Jeff King <peff@peff.net>
---
Is it worth having a strbuf_set* family of functions to match the
strbuf_add*? We seem to have these sorts of errors with strbuf from time
to time, and I wonder if that would make it easier (and more readable)
to do the right thing.
http.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/http.c b/http.c
index d868d8b..d9d1aad 100644
--- a/http.c
+++ b/http.c
@@ -841,6 +841,7 @@ static int http_request(const char *url, struct strbuf *type,
if (type) {
char *t;
+ strbuf_reset(type);
curl_easy_getinfo(slot->curl, CURLINFO_CONTENT_TYPE, &t);
if (t)
strbuf_addstr(type, t);
--
1.8.1.2.11.g1a2f572
^ permalink raw reply related
* Re: [PATCH v2 2/2] i18n: mark OPTION_NUMBER (-NUM) for translation
From: Duy Nguyen @ 2013-02-06 10:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jiang Xin, Git List
In-Reply-To: <7vip66njpj.fsf@alter.siamese.dyndns.org>
On Wed, Feb 6, 2013 at 11:35 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> 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;
>> }
>
> The above bugfix does not address my original concern about
> the name, though.
How about utf8_fwprintf? wprintf() deals with wide characters and
returns the number of wide characters, I think the name fits. And we
could just drop utf8_ and use the existing name, because we don't use
wchar_t* anyway.
--
Duy
^ permalink raw reply
* Re: Adding Missing Tags to a Repository
From: Michael J Gruber @ 2013-02-06 11:18 UTC (permalink / raw)
To: Neil; +Cc: git
In-Reply-To: <CANC5J9F5Pnp08KTem-fdcs_4DcmoN+OgqCHR0=r0y--U8=fdog@mail.gmail.com>
Neil venit, vidit, dixit 06.02.2013 05:45:
> Hi everyone,
>
> A while back I did a svn-to-git migration for my team. Our subversion
> repository had about 30K+ commits, 100+ branches, 2K+ tags, all made
> over a 20+ year period. I was doing the migration using git-svn, and
> my big problem was the tags. git-svn seemed to want to traverse the
> entire history of each tag, which was taking a long time. Because time
> and resources were limited, I ended up just migrating the branches and
> trunk, with the idea that I would handle the tags at a later date. My
> original plan to do that was to crawl the subversion log, find where
> the tags were made, and apply a git tag to the commit that was the
> source of the tag. This was a bad idea.
>
> I've found that over the years, people have made tags that are only
> subdirectories of the source tree, made tags off of other tags, and
> committed to tags. The latter is the biggest problem, since those
> commits don't seem to be stored in the git repository because they
> never appeared in the branches/trunk.
I think it's fair to say that svn encourages the abuse of tags ;)
I've cleaned up other people's mis-tags myself, and it's no fun. If the
tree layout is messed up then git-svn will often create a new root
commit, so that you have to stitch the history yourself.
In the case of "correct" tags, you're often better of converting the tag
creating commit into an annotated tag, provided that the tree is unchanged.
Tags with tree changes are really branches, usually maintenance branches
after a proper tag.
> So, I'm wondering what my options are to bring back this history. One
> idea is to somehow resume the git-svn download, but changing it to
> also scan tags (it sounds like it should be possible, but I haven't
> tried it yet). Or maybe there's some other tool that will more quickly
> clone the repository including tags, and then I can somehow splice the
> tags back into the repository we're already using?
>
> Any ideas or suggestions?
>
> Thanks!
>
If you add a git-svn refspec to your config and rerun git-svn fetch it
might rescan tags, although you may have to poke around to make it
rescan from revision 1.
Michael
^ permalink raw reply
* Re: [PATCH v3 3/8] upload/receive-pack: allow hiding ref hierarchies
From: Jeff King @ 2013-02-06 11:31 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Shawn Pearce
In-Reply-To: <7vwqumvk76.fsf@alter.siamese.dyndns.org>
On Tue, Feb 05, 2013 at 07:45:01AM -0800, Junio C Hamano wrote:
> > 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.
What I meant was that there should be transfer.hiderefs, and an
individual {receive,uploadpack}.hiderefs, similar to the way we have
transfer.unpacklimit. That makes the easy case (hiding the refs
completely) easy, but leaves the flexibility for more.
Like this:
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index a8248d9..131c163 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -59,7 +59,7 @@ static int receive_pack_config(const char *var, const char *value, void *cb)
static int receive_pack_config(const char *var, const char *value, void *cb)
{
- int status = parse_hide_refs_config(var, value, cb);
+ int status = parse_hide_refs_config(var, value, "receive");
if (status)
return status;
diff --git a/refs.c b/refs.c
index e3574ca..9bfea58 100644
--- a/refs.c
+++ b/refs.c
@@ -2560,9 +2560,13 @@ int parse_hide_refs_config(const char *var, const char *value, void *unused)
static struct string_list *hide_refs;
-int parse_hide_refs_config(const char *var, const char *value, void *unused)
+int parse_hide_refs_config(const char *var, const char *value, void *vsection)
{
- if (!strcmp("transfer.hiderefs", var)) {
+ const char *section = vsection;
+
+ if (!strcmp("transfer.hiderefs", var) ||
+ (!prefixcmp(var, section) &&
+ !strcmp(var + strlen(section), ".hiderefs"))) {
char *ref;
int len;
diff --git a/upload-pack.c b/upload-pack.c
index 37977e2..c0390af 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -794,7 +794,7 @@ static int upload_pack_config(const char *var, const char *value, void *unused)
{
if (!strcmp("uploadpack.allowtipsha1inwant", var))
allow_tip_sha1_in_want = git_config_bool(var, value);
- return parse_hide_refs_config(var, value, unused);
+ return parse_hide_refs_config(var, value, "uploadpack");
}
int main(int argc, char **argv)
As an aside, I wonder if there is any point to the void pointer
parameter of parse_hide_refs_config. It is not used as a git_config
callback anywhere.
> > 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.
Sure, I do not pretend that nobody cares. But it is certainly not a
pressing issue, or there would probably be more outcry. And we must also
weigh it against the silent majority that are perfectly happy with the
status quo, that lets them fetch refs/pull/* as any other ref.
In your case, I really think the problem is less that you have a problem
with PR refs in the repository, and more that you do not care about the
pull request feature at all. To you it is useless noise, both in the
repo and on the web. Your arguments about provenance could apply equally
well to PRs accessible via the web interface.
I think the refs/ clutter is only an issue if you want to do mirroring,
and then you have an obvious conflict: did the fetcher want to mirror
everything, including refs/pull, or do they consider that to be clutter?
Only the client knows, which is why I think refspecs are the right place
to deal with clutter (the fact that we cannot say "everything except
refs/pull/*" is a weakness in our refspecs).
> *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?
I don't think this is a problem in practice. DMCA notices do not go to
the repository owner; they go to GitHub. And as far as I know, our
support staff deals with them on a case by case basis (and knows what a
pull request is, and who is responsible for the content in question). It
is not like they see a report of something in refs/pull and lock down
the parent repository; they can see where the request came from and deal
with it appropriately.
But again, such a notice could just as easily come from the list of open
PRs against your repo in the web interface.
> 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),
We block updates to them explicitly in a hook; it looks like that went
in around mid-2011.
> 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).
Yes, point (2) is the main reason they are not simply attached to the
sender's repository.
-Peff
^ permalink raw reply related
* Why is ident_is_sufficient different on Windows?
From: Max Horn @ 2013-02-06 13:06 UTC (permalink / raw)
To: git
Hi there,
while trying to understand which parts of the author & committer identity are mandatory (name, email, or both), I ended up in ident.c, looking at ident_is_sufficient(), and to my surprise discovered that this seems to differ between Windows (were both are mandatory) and everyone else:
static int ident_is_sufficient(int user_ident_explicitly_given)
{
#ifndef WINDOWS
return (user_ident_explicitly_given & IDENT_MAIL_GIVEN);
#else
return (user_ident_explicitly_given == IDENT_ALL_GIVEN);
#endif
}
According to git blame, this was introduced here:
commit 5aeb3a3a838b2cb03d250f3376cf9c41f4d4608e
Author: Junio C Hamano <gitster@pobox.com>
Date: Sun Jan 17 13:54:28 2010 -0800
user_ident_sufficiently_given(): refactor the logic to be usable from elsewhere
The commit message sounds as if this was only a refactoring, but the patch to me look as if it changes behaviour, too. Of course this could very well be false, say due to code elsewhere that already caused Windows to behave differently; I wouldn't know.
Still, I wonder: Why does this difference exist?
Cheers,
Max
^ permalink raw reply
* Re: [PATCH] git-send-email: add ~/.authinfo parsing
From: Michal Nazarewicz @ 2013-02-06 13:26 UTC (permalink / raw)
To: Junio C Hamano, Jeff King; +Cc: git, Krzysztof Mazur
In-Reply-To: <7v6226pdb7.fsf@alter.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 874 bytes --]
On Wed, Feb 06 2013, Junio C Hamano <gitster@pobox.com> wrote:
> I see a lot of rerolls on the credential helper front, but is there
> anybody working on hooking send-email to the credential framework?
I assumed someone had, but if not I can take a stab at it. I'm not sure
however how should I map server, server-port, and user to credential
key-value pairs. I'm leaning towards
protocol=smtp
host=<smtp-server>:<smtp-port>
user=<user>
and than netrc/authinfo helper splitting host to host name and port
number, unless port is not in host in which case protocol is assumed as
port.
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michał “mina86” Nazarewicz (o o)
ooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--
[-- Attachment #2.1: Type: text/plain, Size: 0 bytes --]
[-- Attachment #2.2: Type: application/pgp-signature, Size: 835 bytes --]
^ permalink raw reply
* Re: [PATCH] git-send-email: add ~/.authinfo parsing
From: Ted Zlatanov @ 2013-02-06 14:47 UTC (permalink / raw)
To: Michal Nazarewicz; +Cc: Junio C Hamano, Jeff King, git, Krzysztof Mazur
In-Reply-To: <xa1tpq0dpo89.fsf@mina86.com>
On Wed, 06 Feb 2013 14:26:46 +0100 Michal Nazarewicz <mina86@mina86.com> wrote:
MN> On Wed, Feb 06 2013, Junio C Hamano <gitster@pobox.com> wrote:
>> I see a lot of rerolls on the credential helper front, but is there
>> anybody working on hooking send-email to the credential framework?
MN> I assumed someone had, but if not I can take a stab at it. I'm not sure
MN> however how should I map server, server-port, and user to credential
MN> key-value pairs. I'm leaning towards
MN> protocol=smtp
MN> host=<smtp-server>:<smtp-port>
MN> user=<user>
MN> and than netrc/authinfo helper splitting host to host name and port
MN> number, unless port is not in host in which case protocol is assumed as
MN> port.
That would work (with my PATCHv6 of the netrc credential helper) as
follows:
1) just host
host=H
maps to
machine H login Y password Z
2) host + protocol smtp
host=H
protocol=smtp
maps to any of:
machine H port smtp login Y password Z
machine H protocol smtp login Y password Z
3) host:port + protocol smtp
host=H:25
protocol=smtp
maps to any of:
machine H port 25 protocol smtp login Y password Z
machine H:25 port smtp login Y password Z
machine H:25 protocol smtp login Y password Z
That's my understanding of what we discussed with Peff and Junio about
token mapping. Note we don't split the input host, but instead say "if
token 'port' is numeric, append it to the host token" on the netrc side.
Does that sound reasonable? If yes, I can add it to the testing
Makefile for the netrc credential helper, to make sure it's clearly
stated and tested.
Ted
^ permalink raw reply
* Re: [PATCH] git-send-email: add ~/.authinfo parsing
From: Ted Zlatanov @ 2013-02-06 14:53 UTC (permalink / raw)
To: Matthieu Moy
Cc: Junio C Hamano, Jeff King, Michal Nazarewicz, git,
Krzysztof Mazur, Michal Nazarewicz
In-Reply-To: <vpqa9rhaml6.fsf@grenoble-inp.fr>
On Wed, 06 Feb 2013 09:11:17 +0100 Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote:
MM> Junio C Hamano <gitster@pobox.com> writes:
>> I see a lot of rerolls on the credential helper front, but is there
>> anybody working on hooking send-email to the credential framework?
MM> Not answering the question, but git-remote-mediawiki supports the
MM> credential framework. It is written in perl, and the credential support
MM> is rather cleanly written and doesn't have dependencies on the wiki
MM> part, so the way to go for send-email is probably to libify the
MM> credential support in git-remote-mediawiki, and to use it in send-email.
I looked and that's indeed very useful. If it's put in a library, I'd
use credential_read() and credential_write() in my netrc credential
helper. But I would formalize it a little more about the token names
and output, and I wouldn't necessarily die() on error. Maybe this can
be merged with the netrc credential helper's
read_credential_data_from_stdin() and print_credential_data()?
Let me know if you'd like me to libify this... I'm happy to leave it to
Matthieu or Michal, or anyone else interested.
Ted
^ permalink raw reply
* Re: Bug in "git log --graph -p -m" (version 1.7.7.6)
From: Dale R. Worley @ 2013-02-06 15:03 UTC (permalink / raw)
To: Matthieu Moy; +Cc: gitster, git
In-Reply-To: <vpqmwvia2n7.fsf@grenoble-inp.fr>
> From: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
>
> 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.Moy@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.Moy@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.
The second "commit" line should start with "| * ":
> | |
> | * commit a393ed598e9fb11436f85bd58f1a38c82f2cadb7 (from 33e70e70c0173d634826b998bdc304f93c0966b8)
> | | Merge: 2c1e6a3 33e70e7
> | | Author: Matthieu Moy <Matthieu.Moy@imag.fr>
> | | Date: Tue Feb 5 22:05:33 2013 +0100
Dale
^ permalink raw reply
* [RFC/PATCH 0/4] textconv for show and grep
From: Michael J Gruber @ 2013-02-06 15:08 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King
In-Reply-To: <20130205201106.GA29248@sigill.intra.peff.net>
This min series aims at completing the textconv support of user facing
commands. It is RFC for lack of documentation and tests, to check
whether we really want to go in that direction (I do).
1/4 covers the missing textconv support in the "blob case" of "git
show", which should be (and then is) analogous to "cat-file" and "diff".
2/4 remedies an oddity of "git cat-file --textconv", which errored out
when there is no textconv filter rather than giving an unfiltered blob
(like every other textconv aware command).
3/4 implements "--textconv" for "git grep" sans the blob case; the code
is all Jeff's.
4/4 adds blob support to 3/4 (the "rev:path" case).
3 and 4 can be squashed, of course.
Now I'm quite a happy differ/shower/grepper with my latin1 and OO files ;)
Jeff King (1):
grep: allow to use textconv filters
Michael J Gruber (3):
show: obey --textconv for blobs
cat-file: do not die on --textconv without textconv filters
grep: obey --textconv for the case rev:path
builtin/cat-file.c | 9 ++--
builtin/grep.c | 13 +++---
builtin/log.c | 24 +++++++++--
grep.c | 100 +++++++++++++++++++++++++++++++++++++------
grep.h | 1 +
object.c | 26 ++++++++---
object.h | 2 +
t/t8007-cat-file-textconv.sh | 20 +++------
8 files changed, 148 insertions(+), 47 deletions(-)
--
1.8.1.2.752.g32d147e
^ permalink raw reply
* [RFC/PATCH 1/4] show: obey --textconv for blobs
From: Michael J Gruber @ 2013-02-06 15:08 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King
In-Reply-To: <cover.1360162813.git.git@drmicha.warpmail.net>
Currently, "diff" and "cat-file" for blobs obey "--textconv" options
(with the former defaulting to "--textconv" and the latter to
"--no-textconv") whereas "show" does not obey this option, even though
it takes diff options.
Make "show" on blobs behave like "diff", i.e. obey "--textconv" by
default and "--no-textconv" when given.
Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
builtin/log.c | 24 +++++++++++++++++++++---
1 file changed, 21 insertions(+), 3 deletions(-)
diff --git a/builtin/log.c b/builtin/log.c
index 8f0b2e8..f83870d 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -402,10 +402,28 @@ static void show_tagger(char *buf, int len, struct rev_info *rev)
strbuf_release(&out);
}
-static int show_blob_object(const unsigned char *sha1, struct rev_info *rev)
+static int show_blob_object(const unsigned char *sha1, struct rev_info *rev, const char *obj_name)
{
+ unsigned char sha1c[20];
+ struct object_context obj_context;
+ char *buf;
+ unsigned long size;
+
fflush(stdout);
- return stream_blob_to_fd(1, sha1, NULL, 0);
+ if (!DIFF_OPT_TST(&rev->diffopt, ALLOW_TEXTCONV))
+ return stream_blob_to_fd(1, sha1, NULL, 0);
+
+ if (get_sha1_with_context(obj_name, 0, sha1c, &obj_context))
+ die("Not a valid object name %s", obj_name);
+ if (!obj_context.path[0] ||
+ !textconv_object(obj_context.path, obj_context.mode, sha1c, 1, &buf, &size))
+ return stream_blob_to_fd(1, sha1, NULL, 0);
+
+ if (!buf)
+ die("git show %s: bad file", obj_name);
+
+ write_or_die(1, buf, size);
+ return 0;
}
static int show_tag_object(const unsigned char *sha1, struct rev_info *rev)
@@ -491,7 +509,7 @@ int cmd_show(int argc, const char **argv, const char *prefix)
const char *name = objects[i].name;
switch (o->type) {
case OBJ_BLOB:
- ret = show_blob_object(o->sha1, NULL);
+ ret = show_blob_object(o->sha1, &rev, name);
break;
case OBJ_TAG: {
struct tag *t = (struct tag *)o;
--
1.8.1.2.752.g32d147e
^ permalink raw reply related
* [RFC/PATCH 2/4] cat-file: do not die on --textconv without textconv filters
From: Michael J Gruber @ 2013-02-06 15:08 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King
In-Reply-To: <cover.1360162813.git.git@drmicha.warpmail.net>
When a command is supposed to use textconv filters (by default or with
"--textconv") and none are configured then the blob is output without
conversion; the only exception to this rule is "cat-file --textconv".
Make it behave like the rest of textconv aware commands.
Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
builtin/cat-file.c | 9 +++++----
t/t8007-cat-file-textconv.sh | 20 +++++---------------
2 files changed, 10 insertions(+), 19 deletions(-)
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 00528dd..6912dc2 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -146,10 +146,11 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)
die("git cat-file --textconv %s: <object> must be <sha1:path>",
obj_name);
- if (!textconv_object(obj_context.path, obj_context.mode, sha1, 1, &buf, &size))
- die("git cat-file --textconv: unable to run textconv on %s",
- obj_name);
- break;
+ if (textconv_object(obj_context.path, obj_context.mode, sha1, 1, &buf, &size))
+ break;
+
+ /* otherwise expect a blob */
+ exp_type = "blob";
case 0:
if (type_from_string(exp_type) == OBJ_BLOB) {
diff --git a/t/t8007-cat-file-textconv.sh b/t/t8007-cat-file-textconv.sh
index 78a0085..83c6636 100755
--- a/t/t8007-cat-file-textconv.sh
+++ b/t/t8007-cat-file-textconv.sh
@@ -22,11 +22,11 @@ test_expect_success 'setup ' '
'
cat >expected <<EOF
-fatal: git cat-file --textconv: unable to run textconv on :one.bin
+bin: test version 2
EOF
test_expect_success 'no filter specified' '
- git cat-file --textconv :one.bin 2>result
+ git cat-file --textconv :one.bin >result &&
test_cmp expected result
'
@@ -36,10 +36,6 @@ test_expect_success 'setup textconv filters' '
git config diff.test.cachetextconv false
'
-cat >expected <<EOF
-bin: test version 2
-EOF
-
test_expect_success 'cat-file without --textconv' '
git cat-file blob :one.bin >result &&
test_cmp expected result
@@ -73,25 +69,19 @@ test_expect_success 'cat-file --textconv on previous commit' '
'
test_expect_success SYMLINKS 'cat-file without --textconv (symlink)' '
+ printf "%s" "one.bin" >expected &&
git cat-file blob :symlink.bin >result &&
- printf "%s" "one.bin" >expected
test_cmp expected result
'
test_expect_success SYMLINKS 'cat-file --textconv on index (symlink)' '
- ! git cat-file --textconv :symlink.bin 2>result &&
- cat >expected <<\EOF &&
-fatal: git cat-file --textconv: unable to run textconv on :symlink.bin
-EOF
+ git cat-file --textconv :symlink.bin >result &&
test_cmp expected result
'
test_expect_success SYMLINKS 'cat-file --textconv on HEAD (symlink)' '
- ! git cat-file --textconv HEAD:symlink.bin 2>result &&
- cat >expected <<EOF &&
-fatal: git cat-file --textconv: unable to run textconv on HEAD:symlink.bin
-EOF
+ git cat-file --textconv HEAD:symlink.bin >result &&
test_cmp expected result
'
--
1.8.1.2.752.g32d147e
^ permalink raw reply related
* [RFC/PATCH 3/4] grep: allow to use textconv filters
From: Michael J Gruber @ 2013-02-06 15:08 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King
In-Reply-To: <cover.1360162813.git.git@drmicha.warpmail.net>
From: Jeff King <peff@peff.net>
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 an option "--textconv" which makes git grep use any configured
textconv filters for grepping and output purposes. It is off by default.
Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
builtin/grep.c | 2 ++
grep.c | 100 +++++++++++++++++++++++++++++++++++++++++++++++++--------
grep.h | 1 +
3 files changed, 89 insertions(+), 14 deletions(-)
diff --git a/builtin/grep.c b/builtin/grep.c
index 8025964..915c8ef 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -659,6 +659,8 @@ 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_BOOL(0, "textconv", &opt.allow_textconv,
+ N_("process binary files with textconv filters")),
{ OPTION_INTEGER, 0, "max-depth", &opt.max_depth, N_("depth"),
N_("descend at most <depth> levels"), PARSE_OPT_NONEG,
NULL, 1 },
diff --git a/grep.c b/grep.c
index 4bd1b8b..3880d64 100644
--- a/grep.c
+++ b/grep.c
@@ -2,6 +2,8 @@
#include "grep.h"
#include "userdiff.h"
#include "xdiff-interface.h"
+#include "diff.h"
+#include "diffcore.h"
static int grep_source_load(struct grep_source *gs);
static int grep_source_is_binary(struct grep_source *gs);
@@ -1321,6 +1323,58 @@ static void std_output(struct grep_opt *opt, const void *buf, size_t size)
fwrite(buf, size, 1, stdout);
}
+static int fill_textconv_grep(struct userdiff_driver *driver,
+ struct grep_source *gs)
+{
+ struct diff_filespec *df;
+ char *buf;
+ size_t size;
+
+ if (!driver || !driver->textconv)
+ return grep_source_load(gs);
+
+ /*
+ * The textconv interface is intimately tied to diff_filespecs, so we
+ * have to pretend to be one. If we could unify the grep_source
+ * and diff_filespec structs, this mess could just go away.
+ */
+ df = alloc_filespec(gs->path);
+ switch (gs->type) {
+ case GREP_SOURCE_SHA1:
+ fill_filespec(df, gs->identifier, 1, 0100644);
+ break;
+ case GREP_SOURCE_FILE:
+ fill_filespec(df, null_sha1, 0, 0100644);
+ break;
+ default:
+ die("BUG: attempt to textconv something without a path?");
+ }
+
+ /*
+ * fill_textconv is not remotely thread-safe; it may load objects
+ * behind the scenes, and it modifies the global diff tempfile
+ * structure.
+ */
+ grep_read_lock();
+ size = fill_textconv(driver, df, &buf);
+ grep_read_unlock();
+ free_filespec(df);
+
+ /*
+ * The normal fill_textconv usage by the diff machinery would just keep
+ * the textconv'd buf separate from the diff_filespec. But much of the
+ * grep code passes around a grep_source and assumes that its "buf"
+ * pointer is the beginning of the thing we are searching. So let's
+ * install our textconv'd version into the grep_source, taking care not
+ * to leak any existing buffer.
+ */
+ grep_source_clear_data(gs);
+ gs->buf = buf;
+ gs->size = size;
+
+ return 0;
+}
+
static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int collect_hits)
{
char *bol;
@@ -1331,6 +1385,7 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle
unsigned count = 0;
int try_lookahead = 0;
int show_function = 0;
+ struct userdiff_driver *textconv = NULL;
enum grep_context ctx = GREP_CONTEXT_HEAD;
xdemitconf_t xecfg;
@@ -1352,19 +1407,36 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle
}
opt->last_shown = 0;
- switch (opt->binary) {
- case GREP_BINARY_DEFAULT:
- if (grep_source_is_binary(gs))
- binary_match_only = 1;
- break;
- case GREP_BINARY_NOMATCH:
- if (grep_source_is_binary(gs))
- return 0; /* Assume unmatch */
- break;
- case GREP_BINARY_TEXT:
- break;
- default:
- die("bug: unknown binary handling mode");
+ if (opt->allow_textconv) {
+ grep_source_load_driver(gs);
+ /*
+ * We might set up the shared textconv cache data here, which
+ * is not thread-safe.
+ */
+ grep_attr_lock();
+ textconv = userdiff_get_textconv(gs->driver);
+ grep_attr_unlock();
+ }
+
+ /*
+ * We know the result of a textconv is text, so we only have to care
+ * about binary handling if we are not using it.
+ */
+ if (!textconv) {
+ switch (opt->binary) {
+ case GREP_BINARY_DEFAULT:
+ if (grep_source_is_binary(gs))
+ binary_match_only = 1;
+ break;
+ case GREP_BINARY_NOMATCH:
+ if (grep_source_is_binary(gs))
+ return 0; /* Assume unmatch */
+ break;
+ case GREP_BINARY_TEXT:
+ break;
+ default:
+ die("bug: unknown binary handling mode");
+ }
}
memset(&xecfg, 0, sizeof(xecfg));
@@ -1372,7 +1444,7 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle
try_lookahead = should_lookahead(opt);
- if (grep_source_load(gs) < 0)
+ if (fill_textconv_grep(textconv, gs) < 0)
return 0;
bol = gs->buf;
diff --git a/grep.h b/grep.h
index 8fc854f..94a7ac2 100644
--- a/grep.h
+++ b/grep.h
@@ -106,6 +106,7 @@ struct grep_opt {
#define GREP_BINARY_NOMATCH 1
#define GREP_BINARY_TEXT 2
int binary;
+ int allow_textconv;
int extended;
int use_reflog_filter;
int pcre;
--
1.8.1.2.752.g32d147e
^ permalink raw reply related
* [RFC/PATCH 4/4] grep: obey --textconv for the case rev:path
From: Michael J Gruber @ 2013-02-06 15:08 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King
In-Reply-To: <cover.1360162813.git.git@drmicha.warpmail.net>
Make "grep" obey the "--textconv" option also for the object case, i.e.
when used with an argument "rev:path".
Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
builtin/grep.c | 11 ++++++-----
object.c | 26 ++++++++++++++++++++------
object.h | 2 ++
3 files changed, 28 insertions(+), 11 deletions(-)
diff --git a/builtin/grep.c b/builtin/grep.c
index 915c8ef..0f3c4db 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -458,10 +458,10 @@ static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec,
}
static int grep_object(struct grep_opt *opt, const struct pathspec *pathspec,
- struct object *obj, const char *name)
+ struct object *obj, const char *name, struct object_context *oc)
{
if (obj->type == OBJ_BLOB)
- return grep_sha1(opt, obj->sha1, name, 0, NULL);
+ return grep_sha1(opt, obj->sha1, name, 0, oc ? oc->path : NULL);
if (obj->type == OBJ_COMMIT || obj->type == OBJ_TREE) {
struct tree_desc tree;
void *data;
@@ -503,7 +503,7 @@ static int grep_objects(struct grep_opt *opt, const struct pathspec *pathspec,
for (i = 0; i < nr; i++) {
struct object *real_obj;
real_obj = deref_tag(list->objects[i].item, NULL, 0);
- if (grep_object(opt, pathspec, real_obj, list->objects[i].name)) {
+ if (grep_object(opt, pathspec, real_obj, list->objects[i].name, list->objects[i].context)) {
hit = 1;
if (opt->status_only)
break;
@@ -820,14 +820,15 @@ 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, 0, sha1, &oc)) {
struct object *object = parse_object(sha1);
if (!object)
die(_("bad object %s"), arg);
if (!seen_dashdash)
verify_non_filename(prefix, arg);
- add_object_array(object, arg, &list);
+ add_object_array_with_context(object, arg, &list, xmemdupz(&oc, sizeof(struct object_context)));
continue;
}
if (!strcmp(arg, "--")) {
diff --git a/object.c b/object.c
index 4af3451..1b796c7 100644
--- a/object.c
+++ b/object.c
@@ -245,12 +245,7 @@ int object_list_contains(struct object_list *list, struct object *obj)
return 0;
}
-void add_object_array(struct object *obj, const char *name, struct object_array *array)
-{
- add_object_array_with_mode(obj, name, array, S_IFINVALID);
-}
-
-void add_object_array_with_mode(struct object *obj, const char *name, struct object_array *array, unsigned mode)
+static void add_object_array_with_mode_context(struct object *obj, const char *name, struct object_array *array, unsigned mode, struct object_context *context)
{
unsigned nr = array->nr;
unsigned alloc = array->alloc;
@@ -265,9 +260,28 @@ void add_object_array_with_mode(struct object *obj, const char *name, struct obj
objects[nr].item = obj;
objects[nr].name = name;
objects[nr].mode = mode;
+ objects[nr].context = context;
array->nr = ++nr;
}
+void add_object_array(struct object *obj, const char *name, struct object_array *array)
+{
+ add_object_array_with_mode(obj, name, array, S_IFINVALID);
+}
+
+void add_object_array_with_mode(struct object *obj, const char *name, struct object_array *array, unsigned mode)
+{
+ add_object_array_with_mode_context(obj, name, array, mode, NULL);
+}
+
+void add_object_array_with_context(struct object *obj, const char *name, struct object_array *array, struct object_context *context)
+{
+ if (context)
+ add_object_array_with_mode_context(obj, name, array, context->mode, context);
+ else
+ add_object_array_with_mode_context(obj, name, array, S_IFINVALID, context);
+}
+
void object_array_remove_duplicates(struct object_array *array)
{
unsigned int ref, src, dst;
diff --git a/object.h b/object.h
index 6a97b6b..a11d719 100644
--- a/object.h
+++ b/object.h
@@ -13,6 +13,7 @@ struct object_array {
struct object *item;
const char *name;
unsigned mode;
+ struct object_context *context;
} *objects;
};
@@ -74,6 +75,7 @@ int object_list_contains(struct object_list *list, struct object *obj);
/* Object array handling .. */
void add_object_array(struct object *obj, const char *name, struct object_array *array);
void add_object_array_with_mode(struct object *obj, const char *name, struct object_array *array, unsigned mode);
+void add_object_array_with_context(struct object *obj, const char *name, struct object_array *array, struct object_context *context);
void object_array_remove_duplicates(struct object_array *);
void clear_object_flags(unsigned flags);
--
1.8.1.2.752.g32d147e
^ permalink raw reply related
* CodingGuidelines Perl amendment (was: [PATCH 1/3] Add contrib/credentials/netrc with GPG support)
From: Ted Zlatanov @ 2013-02-06 15:10 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King
In-Reply-To: <7v7gmn1xqi.fsf@alter.siamese.dyndns.org>
On Mon, 04 Feb 2013 15:10:45 -0800 Junio C Hamano <gitster@pobox.com> wrote:
JCH> Ted Zlatanov <tzz@lifelogs.com> writes:
JCH> I thought that we tend to avoid Emacs/Vim formatting cruft left in
JCH> the file. Do we have any in existing file outside contrib/?
>>
>> No, but it's a nice way to express the settings so no one is guessing
>> what the project prefers. At least for me it's not an issue anymore,
>> since I understand your criteria better now, so let me know if you want
>> me to express it in the CodingGuidelines, in a dir-locals.el file, or
>> somewhere else.
JCH> Historically we treated this from CodingGuidelines a sufficient
JCH> clue:
JCH> As for more concrete guidelines, just imitate the existing code
JCH> (this is a good guideline, no matter which project you are
JCH> contributing to). It is always preferable to match the _local_
JCH> convention. New code added to git suite is expected to match
JCH> the overall style of existing code. Modifications to existing
JCH> code is expected to match the style the surrounding code already
JCH> uses (even if it doesn't match the overall style of existing code).
JCH> but over time people wanted more specific guidelines and added
JCH> language specific style guides there. We have sections that cover
JCH> C, shell and Python, and I do not think adding Perl would not hurt.
The following is how I have interpreted the Perl guidelines. I hope
it's OK to include Emacs-specific settings; they make it much easier to
reindent code to be acceptable.
I will submit as a patch if you think this is reasonable at all.
The org-mode markers around the code are just a suggestion.
For Perl 5 programs:
- Most of the C guidelines above apply.
- We try to support Perl 5.8 and later ("use Perl 5.008").
- use strict and use warnings are strongly preferred.
- As in C (see above), we avoid using braces unnecessarily (but Perl
forces braces around if/unless/else/foreach blocks, so this is not
always possible).
- Don't abuse statement modifiers (unless $youmust).
- We try to avoid assignments inside if().
- Learn and use Git.pm if you need that functionality.
- For Emacs, it's useful to put the following in
GIT_CHECKOUT/.dir-locals.el, assuming you use cperl-mode:
#+begin_src lisp
((nil . ((indent-tabs-mode . t)
(tab-width . 8)
(fill-column . 80)))
(cperl-mode . ((cperl-indent-level . 8)
(cperl-extra-newline-before-brace . nil)
(cperl-merge-trailing-else . t))))
#+end_src
^ permalink raw reply
* Re: [PATCH] git-send-email: add ~/.authinfo parsing
From: Matthieu Moy @ 2013-02-06 15:10 UTC (permalink / raw)
To: Ted Zlatanov
Cc: Junio C Hamano, Jeff King, Michal Nazarewicz, git,
Krzysztof Mazur, Michal Nazarewicz
In-Reply-To: <876225o5mj.fsf@lifelogs.com>
Ted Zlatanov <tzz@lifelogs.com> writes:
> MM> [...] so the way to go for send-email is probably to libify the
> MM> credential support in git-remote-mediawiki, and to use it in send-email.
>
> I looked and that's indeed very useful. If it's put in a library, I'd
> use credential_read() and credential_write() in my netrc credential
> helper. But I would formalize it a little more about the token names
> and output,
Can you elaborate on this? The idea of the Perl code was to mimick a
call to the C API, keeping essentially the same names.
> and I wouldn't necessarily die() on error.
Sure, die()ing in a library is bad.
> Maybe this can be merged with the netrc credential helper's
> read_credential_data_from_stdin() and print_credential_data()?
I don't know about the netrc credential helper, but I guess that's
another layer. The git-remote-mediawiki code is the code to call the
credential C API, that in turn may (or may not) call a credential
helper.
> Let me know if you'd like me to libify this... I'm happy to leave it to
> Matthieu or Michal, or anyone else interested.
I'd happily let you do the job, but I can help if needed. One thing to
be careful about: git-remote-mediawiki is currently a standalone script,
so it can be installed with a plain "cp git-remote-mediawiki $somewhere/".
One consequence of libification is that it adds a dependency on the
library (e.g. Git.pm). We should be carefull to keep it easy for the
user to install it (e.g. some kind of "make install", or update the doc).
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [RFC/PATCH 3/4] grep: allow to use textconv filters
From: Matthieu Moy @ 2013-02-06 15:12 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git, Junio C Hamano, Jeff King
In-Reply-To: <da8c01b918c94c84ab61859b1b1453885bff5b06.1360162813.git.git@drmicha.warpmail.net>
Michael J Gruber <git@drmicha.warpmail.net> writes:
> Introduce an option "--textconv" which makes git grep use any configured
> textconv filters for grepping and output purposes. It is off by default.
>
> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
> ---
> builtin/grep.c | 2 ++
> grep.c | 100 +++++++++++++++++++++++++++++++++++++++++++++++++--------
> grep.h | 1 +
Don't forget to update Documentation/git-grep.txt too.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: Bug in "git log --graph -p -m" (version 1.7.7.6)
From: John Keeping @ 2013-02-06 15:14 UTC (permalink / raw)
To: Dale R. Worley; +Cc: Matthieu Moy, gitster, git
In-Reply-To: <201302061503.r16F30UA016375@freeze.ariadne.com>
On Wed, Feb 06, 2013 at 10:03:00AM -0500, Dale R. Worley wrote:
> > From: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
> >
> > 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.Moy@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.Moy@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.
>
> The second "commit" line should start with "| * ":
No. That would indicate a commit on the branch that is the second
parent of the first commit. But this is the same commit as the one
above, just with a diff against its second parent instead of its first
parent.
I would argue that the line should start with "| | ", since it really is
just a continuation of the same commit.
| |
| | commit a393ed598e9fb11436f85bd58f1a38c82f2cadb7 (from 33e70e70c0173d634826b998bdc304f93c0966b8)
| | Merge: 2c1e6a3 33e70e7
| | Author: Matthieu Moy <Matthieu.Moy@imag.fr>
| | Date: Tue Feb 5 22:05:33 2013 +0100
John
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox