* Re: [RFC/PATCH 2/3] remote: reorganize check_pattern_match()
From: Junio C Hamano @ 2012-02-17 22:34 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Jeff King
In-Reply-To: <1329505957-24595-3-git-send-email-felipe.contreras@gmail.com>
Felipe Contreras <felipe.contreras@gmail.com> writes:
> There's a lot of code that can be consolidated there, and will be useful
> for next patches.
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
> remote.c | 59 ++++++++++++++++++++++++++++++-----------------------------
> 1 files changed, 30 insertions(+), 29 deletions(-)
>
> diff --git a/remote.c b/remote.c
> index 55d68d1..019aafc 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -1110,10 +1110,11 @@ static int match_explicit_refs(struct ref *src, struct ref *dst,
> return errs;
> }
>
> -static const struct refspec *check_pattern_match(const struct refspec *rs,
> - int rs_nr,
> - const struct ref *src)
> +static char *check_pattern_match(const struct refspec *rs, int rs_nr, struct ref *ref,
> + int send_mirror, const struct refspec **ret_pat)
> {
For a change that not just adds parameters but removes an existing one,
this is way under-described with neither in-code comment nor log message.
So I'll have to think aloud in this review. Take it as a chance to learn
how the thought process of other people with lessor intelligence than you
have might work, and how to help those slower than you are ;-)
> + const struct refspec *pat;
> + char *name;
> int i;
> int matching_refs = -1;
> for (i = 0; i < rs_nr; i++) {
> @@ -1123,14 +1124,31 @@ static const struct refspec *check_pattern_match(const struct refspec *rs,
> continue;
> }
>
> - if (rs[i].pattern && match_name_with_pattern(rs[i].src, src->name,
> - NULL, NULL))
> - return rs + i;
> + if (rs[i].pattern) {
> + const char *dst_side = rs[i].dst ? rs[i].dst : rs[i].src;
> + if (match_name_with_pattern(rs[i].src, ref->name, dst_side, &name)) {
> + matching_refs = i;
> + break;
We used to discard what match_name_with_pattern() finds out by matching a
wildcard refspec against the ref by passing two NULLs. This updates the
code to capture what destination ref ref->name is mapped to, by using the
same logic as the original and only caller, i.e. 'foo' without destination
maps to the same 'foo' destination, 'foo:bar' maps to the named 'bar'.
This function is not used by fetching side of the codepath, so we do not
have to worry about its need to use different dst_side selection logic
(i.e. 'foo' without destination maps to "do not store anywhere other than
FETCH_HEAD"). Good.
> + }
> + }
> }
> -...
> + if (matching_refs == -1)
> return NULL;
> +
> + pat = rs + matching_refs;
> + if (pat->matching) {
> + /*
> + * "matching refs"; traditionally we pushed everything
> + * including refs outside refs/heads/ hierarchy, but
> + * that does not make much sense these days.
> + */
> + if (!send_mirror && prefixcmp(ref->name, "refs/heads/"))
> + return NULL;
> + name = xstrdup(ref->name);
> + }
So you are moving some code from what the sole caller of this function
does after calling us, and that is where the new parameters come from.
And by doing so, you do not have to run the same match_name_with_pattern()
again. OK.
> + if (ret_pat)
> + *ret_pat = pat;
> + return name;
> }
You did not initialize name to anything at the beginning, but if the
earlier match_name_with_pattern() didn't find anything, we could only come
here after matching_refs is set by the if (rs[i].matching) codepath; by
the time we come here, we would have xstrdup(ref->name) in name, so we
would never return a garbage pointer to the caller. OK.
> static struct ref **tail_ref(struct ref **head)
> @@ -1171,36 +1189,19 @@ int match_push_refs(struct ref *src, struct ref **dst,
> struct ref *dst_peer;
> const struct refspec *pat = NULL;
> char *dst_name;
> +
> if (ref->peer_ref)
> continue;
>
> - pat = check_pattern_match(rs, nr_refspec, ref);
> - if (!pat)
> + dst_name = check_pattern_match(rs, nr_refspec, ref, send_mirror, &pat);
> + if (!dst_name)
> continue;
>
> - if (pat->matching) {
> - /*
> - * "matching refs"; traditionally we pushed everything
> - * including refs outside refs/heads/ hierarchy, but
> - * that does not make much sense these days.
> - */
> - if (!send_mirror && prefixcmp(ref->name, "refs/heads/"))
> - continue;
> - dst_name = xstrdup(ref->name);
> -
> -
> - } else {
> - const char *dst_side = pat->dst ? pat->dst : pat->src;
> - if (!match_name_with_pattern(pat->src, ref->name,
> - dst_side, &dst_name))
> - die("Didn't think it matches any more");
> - }
> dst_peer = find_ref_by_name(*dst, dst_name);
> if (dst_peer) {
> if (dst_peer->peer_ref)
> /* We're already sending something to this ref. */
> goto free_name;
> -
> } else {
> if (pat->matching && !(send_all || send_mirror))
> /*
OK, it is easy to tell that the patch is trivially correct, once a reader
figures out that the patch is really about:
Move code to check_pattern_match() from its sole caller to make it
unnecessary to call match_name_with_pattern() twice.
Saying so in the log message would have prepared the reader, instead of
the "There's a lot of code that can be consolidated there." which does not
give hints on what to look for in the patch.
Also this changes the semantics (because it changed the meaning of its
return value) of check_pattern_match() so much that it would deserve a
rename, which I will address in my review of 3/3.
Otherwise this step looks good.
^ permalink raw reply
* Re: [RFC/PATCH 3/3] push: add 'prune' option
From: Junio C Hamano @ 2012-02-17 22:34 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Jeff King
In-Reply-To: <1329505957-24595-4-git-send-email-felipe.contreras@gmail.com>
Felipe Contreras <felipe.contreras@gmail.com> writes:
> This will allow us to remove refs from the remote that have been removed
> locally.
Can you enhance this a bit more to summarize the gist of what the semantic
of this new feature is, perhaps like this:
After pushing refs, "git push --prune" will remove refs from the
remote that existed before the push and would have been pushed
from us if we had some local refs that would have matched the
refspecs used. For example,
$ git push --prune remote refs/heads/*:refs/remotes/repo1/*
will push all local branches in our repository to refs with
corresponding names under refs/remotes/repo1/ at the remote, and
removes remote's refs in refs/remotes/repo1/ that no longer have
corresponding local branches in our repository. The refs at the
remote outside refs/remotes/repo1/ are not affected.
In order to alley the worries raised in the previous discussion, something
to the effect of the last sentence above is crucial to have, I would think.
> It's useful to conveniently synchronize all the local branches to
> certain remote.
When an update to this patch comes with a documentation update to
illustrate how to exercise this useful feature with an example, it will
start to make sense to write "this is useful" in the log message. I know
you haven't gotten around the documentation part while the patch is marked
as RFC, and that is OK.
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
> builtin/push.c | 2 ++
> remote.c | 29 ++++++++++++++++++++++++++---
> remote.h | 3 ++-
> transport.c | 2 ++
> transport.h | 1 +
> 5 files changed, 33 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/push.c b/builtin/push.c
> index 35cce53..46b99b1 100644
> --- a/builtin/push.c
> +++ b/builtin/push.c
> @@ -261,6 +261,8 @@ int cmd_push(int argc, const char **argv, const char *prefix)
> OPT_BIT('u', "set-upstream", &flags, "set upstream for git pull/status",
> TRANSPORT_PUSH_SET_UPSTREAM),
> OPT_BOOLEAN(0, "progress", &progress, "force progress reporting"),
> + OPT_BIT('p', "prune", &flags, "prune locally removed refs",
> + TRANSPORT_PUSH_PRUNE),
Please refrain from squatting on a short-and-sweet one letter option
before this new feature proves to be popular and useful in a few cycles,
especially when there already is a long option that begins with 'p'.
> OPT_END()
> };
>
> diff --git a/remote.c b/remote.c
> index 019aafc..0900bb5 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -1111,7 +1111,7 @@ static int match_explicit_refs(struct ref *src, struct ref *dst,
> }
>
> static char *check_pattern_match(const struct refspec *rs, int rs_nr, struct ref *ref,
> - int send_mirror, const struct refspec **ret_pat)
> + int send_mirror, int dir, const struct refspec **ret_pat)
Can we name this a bit better? I first thought "Huh? directory?", and had
to scratch my head, wondering if it is an offset into refs/heads/* string
or something....
> {
> const struct refspec *pat;
> char *name;
> @@ -1126,7 +1126,12 @@ static char *check_pattern_match(const struct refspec *rs, int rs_nr, struct ref
>
> if (rs[i].pattern) {
> const char *dst_side = rs[i].dst ? rs[i].dst : rs[i].src;
> - if (match_name_with_pattern(rs[i].src, ref->name, dst_side, &name)) {
> + int match;
> + if (dir == 0)
> + match = match_name_with_pattern(rs[i].src, ref->name, dst_side, &name);
> + else
> + match = match_name_with_pattern(dst_side, ref->name, rs[i].src, &name);
....until the code told us that it is some sort of direction of the
matching. A symbolic constant or two would be even better.
Originally this funcion was fed a list of refs in the source (i.e. on our
end, as this is only used in 'push') and matched them against the source
side of the refspec, rs[i].src, to see under what name the destination
side will store it (i.e. give dst_side as value to find out the result in
&name). This patch adds a new caller, who feeds a list of refs in the
destination (i.e. on the remote end) to find out how they map to the names
on our end (i.e. source). So "direction" is not necessarily incorrect; it
is the direction this function maps the names (either src-to-dst for the
original caller, or dst-to-src for the new caller).
Perhaps "enum map_direction { SRC_TO_DST, DST_TO_SRC }" or something?
> + if (match) {
> matching_refs = i;
> break;
> }
So what is the updated semantics of this function? Is it still
appropriate to name it "check_pattern_match()"?
It seems that by now this does a lot more than just "check if a pattern
matches". Since your patch 2/3, it is a function that finds out the
refname in the remote that the given one refspec would try to update, and
with this patch, it can also map in the reverse direction, given the list
of remote refs, finding out which local ref a refspec would use to update
them.
At the same time, to reduce risk of future breakage, we probably should
rename this function to make it clear that this function is to be only
used by the push side.
Perhaps rename this to "map_push_refs()" or something in the patch 2/3?
> @@ -1173,6 +1178,7 @@ int match_push_refs(struct ref *src, struct ref **dst,
> struct refspec *rs;
> int send_all = flags & MATCH_REFS_ALL;
> int send_mirror = flags & MATCH_REFS_MIRROR;
> + int send_prune = flags & MATCH_REFS_PRUNE;
> int errs;
> static const char *default_refspec[] = { ":", NULL };
> struct ref *ref, **dst_tail = tail_ref(dst);
> @@ -1193,7 +1199,7 @@ int match_push_refs(struct ref *src, struct ref **dst,
> if (ref->peer_ref)
> continue;
>
> - dst_name = check_pattern_match(rs, nr_refspec, ref, send_mirror, &pat);
> + dst_name = check_pattern_match(rs, nr_refspec, ref, send_mirror, 0, &pat);
> if (!dst_name)
> continue;
>
> @@ -1220,6 +1226,23 @@ int match_push_refs(struct ref *src, struct ref **dst,
> free_name:
> free(dst_name);
> }
> + if (send_prune) {
> + /* check for missing refs on the remote */
> + for (ref = *dst; ref; ref = ref->next) {
> + char *src_name;
> +
> + if (ref->peer_ref)
> + /* We're already sending something to this ref. */
> + continue;
> +
> + src_name = check_pattern_match(rs, nr_refspec, ref, send_mirror, 1, NULL);
> + if (src_name) {
> + if (!find_ref_by_name(src, src_name))
> + ref->peer_ref = try_explicit_object_name("");
Yuck. You do not want it to "try" as its name says. You just want to
trigger its "delete" codepath.
Please extract the body of "if (!*name) { ... }" block out of that
function into a separate helper function, i.e.
static struct ref *deleted_ref(void)
{
struct ref *ref = alloc_ref("(delete)");
hashclr(ref->new_sha1);
return ref;
}
then update try_explicit_...() to call it, and call the same helper here.
This is not for runtime efficiency; feeding a constant to a function that
says try_foo() or check_bar() that makes decision on the parameter only to
trigger a partial codepath hurts readability.
> + free(src_name);
> + }
> + }
> + }
> if (errs)
> return -1;
> return 0;
> diff --git a/remote.h b/remote.h
> index b395598..341142c 100644
> --- a/remote.h
> +++ b/remote.h
> @@ -145,7 +145,8 @@ int branch_merge_matches(struct branch *, int n, const char *);
> enum match_refs_flags {
> MATCH_REFS_NONE = 0,
> MATCH_REFS_ALL = (1 << 0),
> - MATCH_REFS_MIRROR = (1 << 1)
> + MATCH_REFS_MIRROR = (1 << 1),
> + MATCH_REFS_PRUNE = (1 << 2),
> };
>
> /* Reporting of tracking info */
> diff --git a/transport.c b/transport.c
> index cac0c06..c20267c 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -1028,6 +1028,8 @@ int transport_push(struct transport *transport,
> match_flags |= MATCH_REFS_ALL;
> if (flags & TRANSPORT_PUSH_MIRROR)
> match_flags |= MATCH_REFS_MIRROR;
> + if (flags & TRANSPORT_PUSH_PRUNE)
> + match_flags |= MATCH_REFS_PRUNE;
Does it make sense to specify --prune when --mirror is in effect? If so,
how would it behave differently from a vanilla --mirror? If not, should
it be detected as an error?
I couldn't infer from the context shown in the patch, but how in general
does this new feature interact with the codepath for --mirror?
> if (match_push_refs(local_refs, &remote_refs,
> refspec_nr, refspec, match_flags)) {
> diff --git a/transport.h b/transport.h
> index 059b330..5d30328 100644
> --- a/transport.h
> +++ b/transport.h
> @@ -101,6 +101,7 @@ struct transport {
> #define TRANSPORT_PUSH_MIRROR 8
> #define TRANSPORT_PUSH_PORCELAIN 16
> #define TRANSPORT_PUSH_SET_UPSTREAM 32
> +#define TRANSPORT_PUSH_PRUNE 64
> #define TRANSPORT_RECURSE_SUBMODULES_CHECK 64
Hrm...?
> #define TRANSPORT_SUMMARY_WIDTH (2 * DEFAULT_ABBREV + 3)
^ permalink raw reply
* Re: [PATCH v3 0/3]
From: Junio C Hamano @ 2012-02-17 23:28 UTC (permalink / raw)
To: Thomas Rast, Jehan Bing; +Cc: git
In-Reply-To: <7v62f5v1d1.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Thomas Rast <trast@student.ethz.ch> writes:
> ...
> I seem to be getting intermittent test failures, and every time the
> failing tests are different, when these three are queued to 'pu'. I didn't
> look for what goes wrong and how.
False alarm. I suspect that it is jb/required-filter topic that is causing
intermittent failures from convert.c depending on the timing of how fast
filter subprocess dies vs how fast we consume its result or something.
Repeatedly running t0021 like this:
$ cd t
$ while sh t0021-conversion.sh ; do :; done
under load seems to make it fail every once in a while.
test_must_fail: died by signal: git add test.fc
Are we dying on SIGPIPE or something?
^ permalink raw reply
* Re: [PATCH v2] Add a setting to require a filter to be successful
From: Junio C Hamano @ 2012-02-18 0:07 UTC (permalink / raw)
To: jehan; +Cc: git, Johannes Sixt
In-Reply-To: <4F3DFCD0.6070002@viscovery.net>
A few test in t0021 use 'false' as the filter, which can exit without
reading any byte from us, before we start writing and causes us to die
with SIGPIPE, leading to intermittent test failure. I think treating this
as a failure of running the filter (the end user's filter should read what
is fed in full, produce its output and write the result back to us) is the
right thing to do, and this patch needs more work to handle such a
situation better, probably by using sigchain_push(SIGPIPE) or something.
^ permalink raw reply
* git-svn won't remember pem password
From: Igor @ 2012-02-18 0:36 UTC (permalink / raw)
To: git; +Cc: Eric Wong
I'm running into an issue where I have to enter my pem certificate password every time I git-svn fetch or git-svn dcommit. Vanilla svn uses OS X KeyChain and remembers my password just fine. Is there a known solution for this? Other users have ran into same issue as described here: http://stackoverflow.com/questions/605519/does-git-svn-store-svn-passwords. However, that solution of removing .subversion folder did not work for me.
Thanks,
Igor
^ permalink raw reply
* Re: [PATCH v2] Add a setting to require a filter to be successful
From: Jehan Bing @ 2012-02-18 0:43 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Johannes Sixt
In-Reply-To: <7vd39dqa1i.fsf@alter.siamese.dyndns.org>
On 2012-02-17 16:07, Junio C Hamano wrote:
> A few test in t0021 use 'false' as the filter, which can exit without
> reading any byte from us, before we start writing and causes us to die
> with SIGPIPE, leading to intermittent test failure. I think treating this
> as a failure of running the filter (the end user's filter should read what
> is fed in full, produce its output and write the result back to us) is the
> right thing to do, and this patch needs more work to handle such a
> situation better, probably by using sigchain_push(SIGPIPE) or something.
If I understand what you're saying, current version of git already have
the problem: if a filter fails without reading anything, git will die
instead of using the unfiltered content. My patch has only made the
issue apparent by testing with a failing filter.
Am I understanding correctly?
^ permalink raw reply
* Re: [PATCH v3 0/3]
From: Jeff King @ 2012-02-18 0:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Thomas Rast, Jehan Bing, git
In-Reply-To: <7vk43lqbt8.fsf@alter.siamese.dyndns.org>
On Fri, Feb 17, 2012 at 03:28:51PM -0800, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
> > Thomas Rast <trast@student.ethz.ch> writes:
> > ...
> > I seem to be getting intermittent test failures, and every time the
> > failing tests are different, when these three are queued to 'pu'. I didn't
> > look for what goes wrong and how.
>
> False alarm. I suspect that it is jb/required-filter topic that is causing
> intermittent failures from convert.c depending on the timing of how fast
> filter subprocess dies vs how fast we consume its result or something.
>
> Repeatedly running t0021 like this:
>
> $ cd t
> $ while sh t0021-conversion.sh ; do :; done
>
> under load seems to make it fail every once in a while.
>
> test_must_fail: died by signal: git add test.fc
>
> Are we dying on SIGPIPE or something?
I would be unsurprised if that is the case. Joey Hess mentioned similar
issues with hooks a month or two ago. And I have been seeing
intermittent failures of t5541 under load that I traced back to SIGPIPE.
I've been meaning to dig further and come up with a good solution.
Here's some previous discussion:
http://article.gmane.org/gmane.comp.version-control.git/186291
I'd be happy if we just ignored SIGPIPE everywhere, but turned it on for
the log family.
-Peff
^ permalink raw reply
* Re: git-status does not propagate -uall to submodules
From: Jens Lehmann @ 2012-02-17 22:11 UTC (permalink / raw)
To: Thomas Rast; +Cc: git
In-Reply-To: <874nupq0la.fsf@thomas.inf.ethz.ch>
Am 17.02.2012 10:18, schrieb Thomas Rast:
> Hi,
>
> While helping with the submodule display on #git I noticed that if you
> have a submodule with status.showuntrackedfiles=no, and run 'git status
> -uall' from the superproject, then this does not propagate into the
> submodule's status. In code:
>
> $ (cd bar && git config status.showuntrackedfiles)
> no
> $ git ls-files -s
> 100644 926c01b7259c489a422442a8dc5cb5ea7c58f60c 0 .gitmodules
> 160000 eb5af46e1a938d064c9f7bae9561013654a43316 0 bar
> $ (cd bar && git status -s -unormal)
> ?? otheruntracked
> ?? untracked
> $ git status
> # On branch master
> nothing to commit (working directory clean)
>
> So far that's expected; after all the submodule is configured not to
> display untracked files. But with -uall:
>
> $ git status -uall
> # On branch master
> nothing to commit (working directory clean)
>
> Shouldn't the -uall propagate, since the user is explicitly asking for
> it? That is, the display should summarize what git-status *with the
> same arguments* would show inside the submodules?
Yes, that makes sense. In 3bfc45047 (git status: ignoring untracked
files must apply to submodules too) I added that using -uno will
propagate into submodules. But -uall (and I suspect -unormal too)
should also be passed to the status commands forked inside the
submodules (even though in both cases -unormal should suffice as only
the presence or absence of untracked files will be shown anyway).
But opposed to -uno, which overrides any diff.ignoreSubmodules or
submodule.<name>.ignore settings for the submodules, what should
-unormal and -uall do? These options are used to override the
status.showuntrackedfiles setting, so I suspect even when these
options are given submodules which are configured to ignore untracked
files via diff.ignoreSubmodules or submodule.<name>.ignore should
still be dropped, right?
^ permalink raw reply
* Does pack v4 do anything to commits?
From: Nguyen Thai Ngoc Duy @ 2012-02-18 4:44 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Git Mailing List, Shawn O. Pearce
Hi Nico,
I had an experiment on speeding up "rev-list --all". If I cache sha-1
of tree and parent, and committer date of single-parent commits, in
binary form, rev-list can be sped up significantly. On linux-2.6.git,
it goes from 14s to 4s (2s to 0.8 for git.git). Profiling shows that
commit parsing (get_sha1_hex, parse_commit_date) dominates rev-list
time.
>From what I remember, pack v4 is mainly about changing tree
representation so that we can traverse object DAG as fast as possible.
Do you do anything to commit representation too? Maybe it's worth
storing the above info along with the compressed commit objects in
pack to shave some more seconds.
By the way, is latest packv4 code available somewhere to fetch?
--
Duy
^ permalink raw reply
* Re: [PATCH] git-send-email: allow overriding smtp-encryption config to 'none'
From: Brian Norris @ 2012-02-18 5:27 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20120216004903.GA21170@sigill.intra.peff.net>
On Wed, Feb 15, 2012 at 4:49 PM, Jeff King <peff@peff.net> wrote:
> Ah, I see. I misunderstood the original problem you were trying to solve
> (I thought your example was "see? Encryption is off, so the server won't
> do AUTH, demonstrating that the patch works.").
Yeah, I got a little bit off track on what my actual goal was...
> Overriding the smtp user from the config is a separate issue, and I
> don't think that is currently possible. The usual way to spell an option
> like that in git is "--no-smtp-user", but it seems that we use perl's
> GetOptions, which does not understand that syntax. So you'd have to add
> a "--no-smtp-user" by hand.
I think the "--no-smtp-user" is what I really wanted. I've written a
different patch that actually targets the user name properly, but I've
also found a current solution that can work for scripting purposes:
just redirect the $GIT_CONFIG environment variable to /dev/null
temporarily. Perhaps I'll send my new patch sometime, but it's not
pressing and I'm not sure what kind of use it would actually get.
Thanks for the pointers.
Brian
^ permalink raw reply
* Re: [PATCH] git-send-email: allow overriding smtp-encryption config to 'none'
From: Jeff King @ 2012-02-18 6:24 UTC (permalink / raw)
To: Brian Norris; +Cc: git
In-Reply-To: <CAN8TOE-vek=ooq4DRcNF0iCg+rJMt6SUhMi4+_dOWaRJ44KLLA@mail.gmail.com>
On Fri, Feb 17, 2012 at 09:27:44PM -0800, Brian Norris wrote:
> > Overriding the smtp user from the config is a separate issue, and I
> > don't think that is currently possible. The usual way to spell an option
> > like that in git is "--no-smtp-user", but it seems that we use perl's
> > GetOptions, which does not understand that syntax. So you'd have to add
> > a "--no-smtp-user" by hand.
>
> I think the "--no-smtp-user" is what I really wanted. I've written a
> different patch that actually targets the user name properly, but I've
> also found a current solution that can work for scripting purposes:
> just redirect the $GIT_CONFIG environment variable to /dev/null
> temporarily.
Just FYI, the fact that doing so works is somewhat accidental. Long ago,
GIT_CONFIG was respected everywhere as an override to stop reading any
other config. Later, it was dropped, but retained its meaning only for
the git-config command, mostly for historical reasons (although these
days one would do better to use "git config -f $file" instead).
So the reason it works for git-send-email is that send-email in turn
calls git-config to actually look at config values, because send-email
is a perl script and not a C program. In other words, the fact that
GIT_CONFIG is respected is a coincidence of some implementation
decisions, not an intended behavior.
I don't think we have any plans for those implementation details to
change in the near future. So by all means, use it if you like for the
time being. But know that it's not a behavior which is guaranteed not to
change in future versions.
> Perhaps I'll send my new patch sometime, but it's not pressing and I'm
> not sure what kind of use it would actually get.
I think the ideal case would be a patch that teaches the send-email
option parsing code to understand a "--no-*" counterpart for every
option, without having to modify each option individually. I haven't
looked at how easy or hard that would be, though.
-Peff
^ permalink raw reply
* Re: [PATCH v2] Add a setting to require a filter to be successful
From: Junio C Hamano @ 2012-02-18 7:27 UTC (permalink / raw)
To: Jehan Bing; +Cc: git, Johannes Sixt
In-Reply-To: <4F3EF43D.2040102@orb.com>
Jehan Bing <jehan@orb.com> writes:
> If I understand what you're saying, current version of git already
> have the problem: if a filter fails without reading anything, git will
> die instead of using the unfiltered content. My patch has only made
> the issue apparent by testing with a failing filter.
> Am I understanding correctly?
Yes.
^ permalink raw reply
* Re: [PATCH v3 0/3]
From: Junio C Hamano @ 2012-02-18 7:27 UTC (permalink / raw)
To: Jeff King; +Cc: Thomas Rast, Jehan Bing, git
In-Reply-To: <20120218005148.GA1940@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I'd be happy if we just ignored SIGPIPE everywhere, but turned it on for
> the log family.
Hmmmm, now you confused me... What is special about the log family? Do
you mean "when we use pager"? But then we are writing into the pager,
which the user can make it exit, which in turn causes us to write into the
pipe, so I would expect that we would want to ignore SIGPIPE --- ah, then
we explicitly catch error in xwrite() and say die() which we do not want.
So you want to let SIGPIPE silently kill us when the pager is in use; is
that it?
^ permalink raw reply
* Re: [PATCH v3 0/3]
From: Jeff King @ 2012-02-18 8:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Thomas Rast, Jehan Bing, git
In-Reply-To: <7v8vk0r481.fsf@alter.siamese.dyndns.org>
On Fri, Feb 17, 2012 at 11:27:26PM -0800, Junio C Hamano wrote:
> > I'd be happy if we just ignored SIGPIPE everywhere, but turned it on for
> > the log family.
>
> Hmmmm, now you confused me... What is special about the log family? Do
> you mean "when we use pager"? But then we are writing into the pager,
> which the user can make it exit, which in turn causes us to write into the
> pipe, so I would expect that we would want to ignore SIGPIPE --- ah, then
> we explicitly catch error in xwrite() and say die() which we do not want.
Sort of. I mentioned the log family because those are the commands that
tend to generate large amounts of input, and which are likely to hit
SIGPIPE. But let me elaborate on my thinking a bit.
There are basically three types of writing callsites in git:
1. Careful calls to write() or fwrite().
These are the majority of calls. In these, we check the return
value of write() and die, return the error code, or whatever is
appropriate for the callsite. SIGPIPE kills the program before the
careful code gets the chance to make a decision about what to do.
At best, this is a nuisance; even if the program is going to die(),
it's likely that the custom code could produce a useful error
message. At worst it causes bugs. For example, we may write to a
subprocess that only needs part of our input to make a decision
(e.g., some hooks and credential helpers). With SIGPIPE, we end up
dying even though no error has occurred. Worse, these are often
annoying heisenbugs that depend on the timing of write() and
close() between the two processes.
2. Non-careful calls of small, fixed-size output.
Things like "git remote" will write some output to stdout via
printf and not bother checking the return code. As the main purpose
of the program is to create output, it's OK to be killed by
SIGPIPE if it happens due to a write to stdout. But respecting
SIGPIPE doesn't buy as all that much. It might save us from making
a few write() calls that will go to nowhere, but most of the work
has already been done by the time we are outputting.
And it has a cost to the other careful write calls in the same
program, because respecting SIGPIPE is a per-process thing. So for
something like "git remote show foo", while we would like SIGPIPE
to kick in for output to stdout, we would not want it for a pipe we
opened to talk to ls-remote.
3. Non-careful calls of large, streaming data.
Commands like "git log" will non-carefully output to stdout, as
well, but they will generate tons of data, consuming possibly
minutes of CPU time (e.g., "git log -p"). If whoever is reading the
output stops doing so, we really want to kill the program to avoid
wasting CPU time. In this instance, SIGPIPE is a big win.
It still has the downside that careful calls in the same program
will be subject to using SIGPIPE. For "log" and friends, this is
probably OK with the current code, as we don't make a lot of pipes.
But that is somewhat of an implementation detail. E.g., "git log
-p" with external diff or textconv writes to a tempfile, and then
runs a subprocess with the tempfile as input. But we could just as
easily have used pipes, and may choose to do so in the future. You
may even be able to trigger a convert_to_git filter in the current
code, which does use pipes.
So basically, I find SIGPIPE to be a simplistic too-heavy hammer that
ends up affecting all writes to pipes, when in reality we are only
interested in affecting ones to some "main" output (usually stdout).
That works OK for small Unix-y programs like "head", but is overly
simplistic for something as big as git.
In an ideal world, we could set a per-descriptor version of SIGPIPE, and
just turn it on for stdout (or somehow find out which descriptor caused
the SIGPIPE after the fact). But that's not possible.
So our next best thing would be to actually check the results of
these non-careful writes. Unfortunately, this means either:
a. wrapping every printf with a function that will appropriately die
on error. This makes the code more cumbersome.
or
b. occasionally checking ferror(stdout) while doing long streams
(e.g., checking it after each commit is written in git log, and
aborting if we saw a write error). This is less cumbersome, but it
does mean that errno may or may not still be accurate by the time
we notice the error. So it's hard to reliably differentiate EPIPE
from other errors. It would be nice, for example, to have git log
exit silently on EPIPE, but properly print an error for something
like ENOSPC. But perhaps that isn't a big deal, as I believe right
now that we would silently ignore something like ENOSPC.
Less robust than that is to just ignore SIGPIPE in most git programs
(which don't benefit from it, and where it is only a liability), but
then manually enable it for the few that care (the log family, and
perhaps diff. Maybe things like "git tag -l", though that output tends
to be pretty small). But I think it would work OK in practice, because
those commands don't tend to make pipes other than the "main" output.
And it's quite easy to implement.
> So you want to let SIGPIPE silently kill us when the pager is in use; is
> that it?
That's an OK heuristic, as the pager being in use is a good sign that we
will generate long, streaming output. It has two downsides, though. One
is that it suffers from the too-heavy hammer of the preceding paragraph.
The other is that it only catches when _we_ create the pipe. You would
also want to catch something like:
git rev-list | head
and stop the traversal. So I think it is less about whether a pager is
in use and more about whether we are creating long output that would
benefit from being cut off early. The pager is an indicator of that, but
it's not a perfect one; I think we'd do better to mark those spots
manually (i.e., by re-enabling SIGPIPE in commands that we deem
appropriate).
-Peff
^ permalink raw reply
* SIGPIPE handling (Re: [PATCH v3 0/3])
From: Jonathan Nieder @ 2012-02-18 10:06 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Thomas Rast, Jehan Bing, git
In-Reply-To: <20120218085221.GA13922@sigill.intra.peff.net>
Hi,
Jeff King wrote:
> Less robust than that is to just ignore SIGPIPE in most git programs
> (which don't benefit from it, and where it is only a liability), but
> then manually enable it for the few that care
This seems backwards. Aren't the only places where it is just a
liability places where git is writing to a pipe that git has created?
We could keep the benefits of SIGPIPE (including simpler error
handling and lack of distracting EPIPE message) in most code, and only
switch to SIGPIPE-ignored semantics where the signal has a chance to
cause harm. Maybe run_command should automatically ignore SIGPIPE
when creating a pipe for the launched command's standard input (with a
flag to ask not to), as a rough heuristic.
There's a subtlety I'm glossing over here, which is that for commands
that produce a lot of output (think: "git fetch --all"), output may
still not the primary goal. I think even they should not block
SIGPIPE, to follow the principle of least surprise in the following
interaction:
git fetch --all 2>&1 | less
... one page later, get bored ...
q (to quit)
Most Unix programs would be killed by SIGPIPE after such a sequence,
so I would expect git to be, too.
Just my two cents,
Jonathan
^ permalink raw reply
* Re: SIGPIPE handling (Re: [PATCH v3 0/3])
From: Jonathan Nieder @ 2012-02-18 10:10 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Thomas Rast, Jehan Bing, git
In-Reply-To: <20120218100517.GA8998@burratino>
Jonathan Nieder wrote:
> There's a subtlety I'm glossing over here, which is that for commands
> that produce a lot of output (think: "git fetch --all"), output may
> still not the primary goal.
Gah. The output that goes to the terminal may not be the primary
goal, I mean (missing "be").
Sorry for the noise.
Jonathan
^ permalink raw reply
* Re: SIGPIPE handling (Re: [PATCH v3 0/3])
From: Jeff King @ 2012-02-18 10:24 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Junio C Hamano, Thomas Rast, Jehan Bing, git
In-Reply-To: <20120218100517.GA8998@burratino>
On Sat, Feb 18, 2012 at 04:06:07AM -0600, Jonathan Nieder wrote:
> Jeff King wrote:
>
> > Less robust than that is to just ignore SIGPIPE in most git programs
> > (which don't benefit from it, and where it is only a liability), but
> > then manually enable it for the few that care
>
> This seems backwards. Aren't the only places where it is just a
> liability places where git is writing to a pipe that git has created?
Yes. But the problem is that those spots are buried deep within library
code, and are an implementation detail that the caller shouldn't need to
know about.
But more importantly, I see SIGPIPE as an optimization. The program on
the generating side of a pipe _could_ keep making output and dumping it
nowhere. So the optimization is about telling it early that it can stop
bothering. But that optimization is affecting correctness in some cases.
And in cases of correctness versus optimization, it's nice if we can be
correct by default and then optimize in places where it matters most and
where we've verified that correctness isn't harmed.
I also think it's simply easier to list the places that benefit from
SIGPIPE than those that are harmed by it. But if we wanted to go the
other way (allow by default and turn it off in a few programs), at the
very least all of the network programs (receive-pack. upload-pack, etc)
should start ignoring it completely.
> We could keep the benefits of SIGPIPE (including simpler error
> handling and lack of distracting EPIPE message) in most code, and only
> switch to SIGPIPE-ignored semantics where the signal has a chance to
> cause harm. Maybe run_command should automatically ignore SIGPIPE
> when creating a pipe for the launched command's standard input (with a
> flag to ask not to), as a rough heuristic.
That's a reasonable heuristic, but not foolproof. If I have a
long-running subprocess with a pipe, then SIGPIPE will be off for a long
time, meaning the code you thought was protected by it is not. In
practice, I think it would work OK with our current code-base, as we
tend to have short-lived subprocesses. You'd have to special case the
starting of the pager, of course, but that's not too hard.
> There's a subtlety I'm glossing over here, which is that for commands
> that produce a lot of output (think: "git fetch --all"), output may
> still not the primary goal. I think even they should not block
> SIGPIPE, to follow the principle of least surprise in the following
> interaction:
>
> git fetch --all 2>&1 | less
> ... one page later, get bored ...
> q (to quit)
>
> Most Unix programs would be killed by SIGPIPE after such a sequence,
> so I would expect git to be, too.
But it's quite non-deterministic whether or when git-fetch will be
killed. It may have written all data to the pipe. You may quit, but
fetch still runs for a while before producing output. If you want it
killed, you are much better off to do so by sending it SIGINT.
-Peff
^ permalink raw reply
* [PATCH] remote: align set-branches builtin usage and documentation
From: Philip Jägenstedt @ 2012-02-18 11:17 UTC (permalink / raw)
To: git; +Cc: Philip Jägenstedt
The order of [--add] <name> <branch>... was inconsistent
between builtin remote usage messages and git-remote.txt.
Align it closer to the order used in set-url.
Signed-off-by: Philip Jägenstedt <philip@foolip.org>
---
Documentation/git-remote.txt | 2 +-
builtin/remote.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt
index 5a8c506..d376d19 100644
--- a/Documentation/git-remote.txt
+++ b/Documentation/git-remote.txt
@@ -14,7 +14,7 @@ SYNOPSIS
'git remote rename' <old> <new>
'git remote rm' <name>
'git remote set-head' <name> (-a | -d | <branch>)
-'git remote set-branches' <name> [--add] <branch>...
+'git remote set-branches' [--add] <name> <branch>...
'git remote set-url' [--push] <name> <newurl> [<oldurl>]
'git remote set-url --add' [--push] <name> <newurl>
'git remote set-url --delete' [--push] <name> <url>
diff --git a/builtin/remote.c b/builtin/remote.c
index f54a89a..fec92bc 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -16,7 +16,7 @@ static const char * const builtin_remote_usage[] = {
"git remote [-v | --verbose] show [-n] <name>",
"git remote prune [-n | --dry-run] <name>",
"git remote [-v | --verbose] update [-p | --prune] [(<group> | <remote>)...]",
- "git remote set-branches <name> [--add] <branch>...",
+ "git remote set-branches [--add] <name> <branch>...",
"git remote set-url <name> <newurl> [<oldurl>]",
"git remote set-url --add <name> <newurl>",
"git remote set-url --delete <name> <url>",
--
1.7.9.1.246.g77c8c.dirty
^ permalink raw reply related
* Re: git-svn won't remember pem password
From: Jakub Narebski @ 2012-02-18 11:30 UTC (permalink / raw)
To: Igor; +Cc: git, Eric Wong
In-Reply-To: <E56535F6-2C9B-4D14-A88F-2471E34D2769@gmail.com>
Igor <mrigor83@gmail.com> writes:
> I'm running into an issue where I have to enter my pem certificate
> password every time I git-svn fetch or git-svn dcommit. Vanilla svn
> uses OS X KeyChain and remembers my password just fine. Is there a
> known solution for this? Other users have ran into same issue as
> described here:
>
> http://stackoverflow.com/questions/605519/does-git-svn-store-svn-passwords
> However, that solution of removing .subversion folder did not work
> for me.
I don't know if it is svn that has to remember password, or git that
has to remember password. Git 1.7.9 learned "credentials API" that
allows integration with platform native keychain mechanisms, and I
think OS X Keychain is one of examples / supported platforms (but it
might not made it into core git)... though I am not sure if it affects
git-svn, or only HTTP(S) transport.
--
Jakub Narebski
^ permalink raw reply
* Re: Gitk local language issue
From: Pat Thoyts @ 2012-02-18 13:09 UTC (permalink / raw)
To: shyamal; +Cc: git
In-Reply-To: <1329467459691-7293532.post@n2.nabble.com>
shyamal <shyamal2005@gmail.com> writes:
>Hi,
>
>I am working in Japan now on a windows environment.
>I installed GIT on my machine.When I run the application,The menus are in
>Japanese.To get the English menu I added
>@set LANG=en
>at the beginning of git.cmd file.This worked like a magic :-)
>But when I click the Visualize master' history from the repository menu of
>Git Gui, a new interface (Gitk:Websites) opens where all the menus are still
>in Japanese.Any idea how to change the menu to English in Gitk too?
>
>Thanks in advance
>
>Regards,
>Shyamal.
If I modify the git.cmd file here on my English system to include
@set LANG=fr just after the @set PLINK_PROTOCOL=ssh command, then git
gui runs with French menus and selecting the view history menu item
launches gitk with French menus.
This is because git-gui executes a new tcl interpreter subprocess passing
in the known gitk script location. So the gitk process inherits the
git-gui environment (including this LANG setting). It doesn't call the
gitk.cmd script on Windows.
Possibly your interpreter is picking up some other locale setting. The
msgcat script will use LC_ALL, LC_MESSAGES or LANG (in that order) and
only makes use of the first one it sees. So perhaps you have an LC_ALL
set someplace - however, I would assume that would force the git-gui
script to use that locale too.
One quick hack would be to modify bin/gitk and after the msgcat
initialization force the locale using:
msgcat::mclocale en
--
Pat Thoyts http://www.patthoyts.tk/
PGP fingerprint 2C 6E 98 07 2C 59 C8 97 10 CE 11 E6 04 E0 B9 DD
^ permalink raw reply
* Re: [PATCH] remote: align set-branches builtin usage and documentation
From: Philip Jägenstedt @ 2012-02-18 13:11 UTC (permalink / raw)
To: git
In-Reply-To: <1329563867-13283-1-git-send-email-philip@foolip.org>
On Sat, Feb 18, 2012 at 12:17, Philip Jägenstedt
<philip.jagenstedt@gmail.com> wrote:
> The order of [--add] <name> <branch>... was inconsistent
> between builtin remote usage messages and git-remote.txt.
To clarify, it's the order of "git remote set-branches" that is
inconsistent with the two instances the patch touches:
usage: git remote set-branches <name> <branch>...
or: git remote set-branches --add <name> <branch>...
--
Philip Jägenstedt
^ permalink raw reply
* [PATCH] completion: remote set-* <name> and <branch>
From: Philip Jägenstedt @ 2012-02-18 13:32 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, Philip Jägenstedt
Complete <name> only for set-url. For set-branches and
set-head, complete <name> and <branch> over the network,
like e.g. git pull already does.
Signed-off-by: Philip Jägenstedt <philip@foolip.org>
---
contrib/completion/git-completion.bash | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 1505cff..8e7abb6 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -738,6 +738,9 @@ __git_complete_remote_or_refspec ()
{
local cur_="$cur" cmd="${words[1]}"
local i c=2 remote="" pfx="" lhs=1 no_complete_refspec=0
+ if [ "$cmd" = "remote" ]; then
+ c=$((++c))
+ fi
while [ $c -lt $cword ]; do
i="${words[c]}"
case "$i" in
@@ -788,7 +791,7 @@ __git_complete_remote_or_refspec ()
__gitcomp_nl "$(__git_refs)" "$pfx" "$cur_"
fi
;;
- pull)
+ pull|remote)
if [ $lhs = 1 ]; then
__gitcomp_nl "$(__git_refs "$remote")" "$pfx" "$cur_"
else
@@ -2289,7 +2292,7 @@ _git_config ()
_git_remote ()
{
- local subcommands="add rename rm show prune update set-head"
+ local subcommands="add rename rm set-head set-branches set-url show prune update"
local subcommand="$(__git_find_on_cmdline "$subcommands")"
if [ -z "$subcommand" ]; then
__gitcomp "$subcommands"
@@ -2297,9 +2300,12 @@ _git_remote ()
fi
case "$subcommand" in
- rename|rm|show|prune)
+ rename|rm|set-url|show|prune)
__gitcomp_nl "$(__git_remotes)"
;;
+ set-head|set-branches)
+ __git_complete_remote_or_refspec
+ ;;
update)
local i c='' IFS=$'\n'
for i in $(git --git-dir="$(__gitdir)" config --get-regexp "remotes\..*" 2>/dev/null); do
--
1.7.9.1.245.g4cbe6
^ permalink raw reply related
* Re: Does pack v4 do anything to commits?
From: Nicolas Pitre @ 2012-02-18 15:34 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Git Mailing List, Shawn O. Pearce
In-Reply-To: <CACsJy8CZPG3b3LBF-EFO_kOv6i157jy5414+bHcqiwOKyC+8VA@mail.gmail.com>
On Sat, 18 Feb 2012, Nguyen Thai Ngoc Duy wrote:
> Hi Nico,
>
> I had an experiment on speeding up "rev-list --all". If I cache sha-1
> of tree and parent, and committer date of single-parent commits, in
> binary form, rev-list can be sped up significantly. On linux-2.6.git,
> it goes from 14s to 4s (2s to 0.8 for git.git). Profiling shows that
> commit parsing (get_sha1_hex, parse_commit_date) dominates rev-list
> time.
>
> >From what I remember, pack v4 is mainly about changing tree
> representation so that we can traverse object DAG as fast as possible.
> Do you do anything to commit representation too? Maybe it's worth
> storing the above info along with the compressed commit objects in
> pack to shave some more seconds.
Both the tree and commit object representations are completely changed
to evacuate SHA1 parsing and searching entirely. The SHA1 references
are uncompressed, and replaced by indices into the pack for direct
lookup without any binary search. And the commit dates are stored in
binary form. All path names as well as author/committer names are
factored out into a table as well. This should make history traversal
operations almost as fast as walking a linked list in memory, while
making the actual pack size smaller at the same time.
> By the way, is latest packv4 code available somewhere to fetch?
Well, not yet. Incidentally, I'm going in the Caribbeans for a week in
a week, with no kids and only my wife who is going to be busy with scuba
diving activities. Like I did last year, I'm going to take some time to
pursue my work on Pack v4 during that time. And I intend to publish it
when I come back, whatever state it is in, so someone else can complete
the work eventually (I have too much to do to spend significant time on
Git these days).
Nicolas
^ permalink raw reply
* [PATCH 1/1] Don't append -lintl when there is no gettext support
From: John Szakmeister @ 2012-02-18 19:38 UTC (permalink / raw)
To: git; +Cc: John Szakmeister
In-Reply-To: <1329593884-9999-1-git-send-email-john@szakmeister.net>
The check for libintl in a C library incorrectly assumes that if it's
not builtin then it must exist externally. Instead, let's check for
the existence of libintl.h first. If libintl.h exists, and libintl is
not in libc, then we append the library.
Signed-off-by: John Szakmeister <john@szakmeister.net>
---
configure.ac | 20 ++++++++++++--------
1 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/configure.ac b/configure.ac
index 630dbdd..8471f5c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -640,7 +640,18 @@ AC_CHECK_LIB([c], [gettext],
[LIBC_CONTAINS_LIBINTL=YesPlease],
[LIBC_CONTAINS_LIBINTL=])
AC_SUBST(LIBC_CONTAINS_LIBINTL)
-test -n "$LIBC_CONTAINS_LIBINTL" || LIBS="$LIBS -lintl"
+
+#
+# Define NO_GETTEXT if you don't want Git output to be translated.
+# A translated Git requires GNU libintl or another gettext implementation
+AC_CHECK_HEADER([libintl.h],
+[NO_GETTEXT=],
+[NO_GETTEXT=YesPlease])
+AC_SUBST(NO_GETTEXT)
+
+if test -z "$NO_GETTEXT"; then
+ test -n "$LIBC_CONTAINS_LIBINTL" || LIBS="$LIBS -lintl"
+fi
## Checks for header files.
AC_MSG_NOTICE([CHECKS for header files])
@@ -824,13 +835,6 @@ AC_CHECK_HEADER([paths.h],
[HAVE_PATHS_H=])
AC_SUBST(HAVE_PATHS_H)
#
-# Define NO_GETTEXT if you don't want Git output to be translated.
-# A translated Git requires GNU libintl or another gettext implementation
-AC_CHECK_HEADER([libintl.h],
-[NO_GETTEXT=],
-[NO_GETTEXT=YesPlease])
-AC_SUBST(NO_GETTEXT)
-#
# Define HAVE_LIBCHARSET_H if have libcharset.h
AC_CHECK_HEADER([libcharset.h],
[HAVE_LIBCHARSET_H=YesPlease],
--
1.7.9.1
^ permalink raw reply related
* [PATCH 0/1] Make libintl in libc detection more robust
From: John Szakmeister @ 2012-02-18 19:38 UTC (permalink / raw)
To: git; +Cc: John Szakmeister
When building the latest release, I noticed that pthreads support
was disabled. It turns out that the libintl in libc support is
adding "-lintl" to LIBS, even though I don't have that library on my
Mac. This patch fixes the issue by moving the check for libintl.h
closer to the checks for libintl in libc, and only adding "-lintl"
when NO_GETTEXT is empty.
This is my first time submitting a patch to git. I hope I've done
things correctly!
John Szakmeister (1):
Don't append -lintl when there is no gettext support
configure.ac | 20 ++++++++++++--------
1 files changed, 12 insertions(+), 8 deletions(-)
--
1.7.9.1
^ 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