* Re: [PATCH] remote-hg: store converted URL
From: Junio C Hamano @ 2013-01-15 16:51 UTC (permalink / raw)
To: Max Horn; +Cc: git, Felipe Contreras
In-Reply-To: <7vzk0a4ekj.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Max Horn <max@quendi.de> writes:
> ...
>> See also the discussion (yeah, this time a real one ;-) leading to this:
>> https://github.com/felipec/git/issues/2
>> ...
If I understand correctly, the $backend::$opaqueToken is a contract
between the remote-helper and the remote-$backend that says "When
user wants to interact with the same (foreign) repository, we agreed
to let her use 'origin' nickname. The remote-helper looks up this
opaque token that corresponds to 'origin' and gives it to the
remote-$backend, and whatever is in the opaque token should be
sufficient for the remote-$backend to figure out how to proceed from
there".
But in this hg::../over/there case, it seems that string is not
sufficient for remote-hg to do so and the contract is broken.
When "git clone $backend::$opaqueToken repo" is run in /dir/ecto/ry,
and then subsequent "git fetch origin" will be run in (a
subdirectory of) /dir/ecto/ry/repo, but anything relative to
/dir/ecto/ry will not work once you go inside /dir/ecto/ry/repo.
The "create a new repository here" argument could even be an
absolute path to a totally different place, so if the
remote-$backend wants to use $opaqueToken as anything relative to
the $(cwd) when "git clone" was invoked, that original location
needs to be available somehow.
Would a new helper protocol message be necessary, so that the
backend can rewrite the $opaqueToken at "clone" time and tell the
helper what to store as URL instead of the original? I do not think
that is much different from remote-$backend updating the value of the
remote.origin.URL using "git config".
An alternative approach may be for somebody (either the "git clone"
or the remote-$backend) to store a "base directory" when "git clone"
was invoked in remote.origin.dirAtCloneTime variable, so that the
next time remote-$backend runs, it can read that directory and
interpret the $opaqueToken as a relative path to that directory if
it wants to. That way, nobody needs to rewrite $opaqueToken.
How do other remote helpers solve this, I have to wonder, though.
By not allowing relative paths to a directory?
^ permalink raw reply
* Re: [PATCH] am: invoke perl's strftime in C locale
From: Jeff King @ 2013-01-15 16:50 UTC (permalink / raw)
To: Antoine Pelisse; +Cc: Dmitry V. Levin, Junio C Hamano, git
In-Reply-To: <CALWbr2w+q5=Z8__g+J_s2NtTMgziHrntFqsi8vCJyvfO2qi81A@mail.gmail.com>
On Tue, Jan 15, 2013 at 05:42:12PM +0100, Antoine Pelisse wrote:
> > This puts all of perl into the C locale, which would mean error messages
> > from perl would be in English rather than the user's language. It
> > probably isn't a big deal, because that snippet of perl is short and not
> > likely to produce problems, but I wonder how hard it would be to set the
> > locale just for the strftime call.
>
> Maybe just setting LC_TIME to C would do ...
Yeah, that is a nice simple solution. Dmitry, does just setting LC_TIME
fix the problem for you?
-Peff
^ permalink raw reply
* Re: [PATCH] am: invoke perl's strftime in C locale
From: Antoine Pelisse @ 2013-01-15 16:42 UTC (permalink / raw)
To: Jeff King; +Cc: Dmitry V. Levin, Junio C Hamano, git
In-Reply-To: <20130115155953.GB21815@sigill.intra.peff.net>
> This puts all of perl into the C locale, which would mean error messages
> from perl would be in English rather than the user's language. It
> probably isn't a big deal, because that snippet of perl is short and not
> likely to produce problems, but I wonder how hard it would be to set the
> locale just for the strftime call.
Maybe just setting LC_TIME to C would do ...
>From locale(7) man page:
LC_TIME
changes the behavior of the strftime(3) function to
display the current time in a locally acceptable form; for
example, most of Europe uses a 24-hour clock versus the
12-hour clock used in the United States.
^ permalink raw reply
* Re: [PATCH] attr: fix off-by-one directory component length calculation
From: Junio C Hamano @ 2013-01-15 16:17 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy
Cc: git, Ross Lagerwall, Jean-Noël AVILA
In-Reply-To: <1358256924-31578-1-git-send-email-pclouds@gmail.com>
Good spotting and a nicely explained patch. Thanks.
^ permalink raw reply
* Re: [PATCH] remote-hg: store converted URL
From: Junio C Hamano @ 2013-01-15 16:05 UTC (permalink / raw)
To: Max Horn; +Cc: git, Felipe Contreras
In-Reply-To: <64C81CD0-960A-47F2-89FC-8D3126B1F4D5@quendi.de>
Max Horn <max@quendi.de> writes:
> On 14.01.2013, at 19:14, Junio C Hamano wrote:
>
>> What is lacking from this description is why it even needs to work
>> from a different working directory....
In your rewrite below, this is still lacking, I think.
> Fully agreed. How about this commit message:
>
> -- >8 --
> remote-hg: store converted URL of hg repo in git config
>
> When remote-hg is invoked, read the remote repository URL from the git config,
> give Mercurial a chance to expand it, and if changed, store it back into
> the git config.
>
> This fixes the following problem: Suppose you clone a local hg repository
> using a relative path, e.g.
> git clone hg::hgrepo gitrepo
> This stores "hg::hgrepo" in gitrepo/.git/config. However, no information
> about the PWD is stored, making it impossible to correctly interpret the
> relative path later on.
Here, you say "correctly interpret relative path later on", but it
is not made clear why it is even necessary. And that makes ...
> Thus when latter attempting to, say, "git pull"
> from inside gitrepo, remote-hg cannot resolve the relative path correctly,
> and the user sees an unexpected error.
... "cannot resolve the relative path correctly" above sound like a
bug in remote-hg. Something like:
Cloning a local hg repository using a relative path, e.g.
git clone hg::hgrepo gitrepo
stores "hg::hgrepo" in gitrepo/.git/config as its URL. When
remote-hg is invoked by "git fetch", it chdirs to X (which is
different from the "gitrepo" directory) and uses the URL (which
is not correct, as it is a relative path but the cwd is
different when it is used) to interact with the original
"hgrepo", which will fail.
is needed, but you didn't explain what that X is. Perhaps it is a
temporary directory. Perhaps it is a hidden Hg repository somewhere
in gitrepo/.git directory. Or something else.
With that explained ...
> With this commit, the URL "hg::hgrepo" gets expanded (during cloning,
> but also during any other remote operation) and the resulting absolute
> URL (e.g. "hg::/abspath/hgrepo") is stored in gitrepo/.git/config.
> Thus the git clone of hgrepo becomes usable.
... the description of the fix start making sense, but not without.
>> Was this work done outside the list? I just want to make sure this
>> patch is not something Felipe did not want to sign off for whatever
>> reason but you are passing it to the list as a patch signed off by
>> him.
>
> The work was done by Felipe's and published in his github repository:
> https://github.com/felipec/git/commit/605bad5b52d2fcf3d8f5fd782a87d7c97d1b040a
> See also the discussion (yeah, this time a real one ;-) leading to this:
> https://github.com/felipec/git/issues/2
>
> I took his sign-off from there and interpreted it as saying that
> Felipe was OK with this being pushed to git.git. But perhaps this
> is not what I should have done?
You did nothing wrong, other than not having given the necessary
context to understand how the change flowed here and it is kosher.
Which you now have, so it is OK.
Thanks.
^ permalink raw reply
* Re: [PATCH 1/6] config: add helper function for parsing key names
From: Jeff King @ 2013-01-15 16:04 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Joachim Schmitz, René Scharfe, git
In-Reply-To: <7v8v7veixc.fsf@alter.siamese.dyndns.org>
On Mon, Jan 14, 2013 at 10:08:47AM -0800, Junio C Hamano wrote:
> > +extern int match_config_key(const char *var,
> > + const char *section,
> > + const char **subsection, int *subsection_len,
> > + const char **key);
> > +
>
> I agree with Jonathan about the naming s/match/parse/.
I see this is marked for re-roll in WC. I'm happy to re-roll it with the
suggestions from Jonathan, but before I do, did you have any comment on
the "struct config_key" alternative I sent as a follow-up?
You said:
> After looking at the callers in your later patches, I think the
> counted interface to subsection is probably fine. The caller can
> check !subsection to see if it is a two- or three- level name, and
>
> if (parse_config_key(var, "submodule", &name, &namelen, &key) < 0 ||
> !name)
> return 0;
>
> is very easy to follow (that is the result of your 5th step).
but I wasn't sure if that was "it is not worth the trouble of the other
one" or "I did not yet read the other one".
-Peff
^ permalink raw reply
* Re: [PATCH] am: invoke perl's strftime in C locale
From: Jeff King @ 2013-01-15 15:59 UTC (permalink / raw)
To: Dmitry V. Levin; +Cc: Junio C Hamano, git
In-Reply-To: <20130114205933.GA25947@altlinux.org>
On Tue, Jan 15, 2013 at 12:59:33AM +0400, Dmitry V. Levin wrote:
> diff --git a/git-am.sh b/git-am.sh
> index c682d34..64b88e4 100755
> --- a/git-am.sh
> +++ b/git-am.sh
> @@ -334,7 +334,7 @@ split_patches () {
> # Since we cannot guarantee that the commit message is in
> # git-friendly format, we put no Subject: line and just consume
> # all of the message as the body
> - perl -M'POSIX qw(strftime)' -ne 'BEGIN { $subject = 0 }
> + LC_ALL=C perl -M'POSIX qw(strftime)' -ne 'BEGIN { $subject = 0 }
> if ($subject) { print ; }
> elsif (/^\# User /) { s/\# User/From:/ ; print ; }
> elsif (/^\# Date /) {
This puts all of perl into the C locale, which would mean error messages
from perl would be in English rather than the user's language. It
probably isn't a big deal, because that snippet of perl is short and not
likely to produce problems, but I wonder how hard it would be to set the
locale just for the strftime call.
-Peff
^ permalink raw reply
* Re: [RFC/PATCH] ignore memcmp() overreading in bsearch() callback
From: René Scharfe @ 2013-01-15 15:55 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Carlos Martín Nieto, Johannes Schindelin
In-Reply-To: <7v38y38hhm.fsf@alter.siamese.dyndns.org>
Am 15.01.2013 00:36, schrieb Junio C Hamano:
> It appears that memcmp() uses the usual "one word at a time"
> comparison and triggers valgrind in a callback of bsearch() used in
> the refname search. I can easily trigger problems in any script
> with test_commit (e.g. "sh t0101-at-syntax.sh --valgrind -i -v")
> without this suppression.
I can't reproduce it on Debian, but can we perhaps do without the
suppression with a patch like this instead? I would expect it to
be slightly faster because we lose the strlen() call, but didn't
check. It's also simpler, perhaps with the exception of the last
line. Does it help in your case?
René
---
refs.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/refs.c b/refs.c
index 541fec2..1a0e049 100644
--- a/refs.c
+++ b/refs.c
@@ -335,12 +335,10 @@ static int ref_entry_cmp_sslice(const void *key_, const void *ent_)
{
struct string_slice *key = (struct string_slice *)key_;
struct ref_entry *ent = *(struct ref_entry **)ent_;
- int entlen = strlen(ent->name);
- int cmplen = key->len < entlen ? key->len : entlen;
- int cmp = memcmp(key->str, ent->name, cmplen);
+ int cmp = strncmp(key->str, ent->name, key->len);
if (cmp)
return cmp;
- return key->len - entlen;
+ return '\0' - ent->name[key->len];
}
/*
^ permalink raw reply related
* Re: [BUG] Possible bug in `remote set-url --add --push`
From: Junio C Hamano @ 2013-01-15 15:53 UTC (permalink / raw)
To: Michael J Gruber; +Cc: Jardel Weyrich, Sascha Cunz, git@vger.kernel.org
In-Reply-To: <50F524F8.5090803@drmicha.warpmail.net>
Michael J Gruber <git@drmicha.warpmail.net> writes:
> Also there is a conceptual confusion: pushurl is meant to push to the
> same repo using a different url, e.g. something authenticated
> (https/ssh) for push and something faster/easier for fetch.
That is not necessarily true, depending on the definition of your
"same". Having multiple URLs/PushURLs that refer to physically
different locations, as long as "git push there" immediately
followed by "git fetch here" should work with the repositories that
are conceptually equivalent, is a supported mode of operation. In
fact, they being physically different _was_ the original motivation
of the feature. See 755225d (git builtin "push", 2006-04-29).
The definition of the "immediate" above also depends on your use; it
could be tens of minutes (you may be fetching from git.k.org that
can be reached from the general public, which may be a cname for
multiple machines mirroring a single master.k.org that k.org account
holders push to, and there may be propagation delays). In such a
scenario, your URL may point at the public git.k.org, pushURL may
point at master.k.org, and you may have other pushURLs that point at
other places you use as back-up locations (e.g. git.or.cz or
github.com).
As long as you _mean_ to maintain their contents the same, you can
call them conceptually "the same repo" and your statement becomes
true.
> It never was meant to push to several repos.
This is false. It _was_ designed to be used that way from day one.
(I am not saying using it in other ways is an abuse---I am merely
saying that pushing to multiple physically different repositories is
within its scope).
> That being said, I don't mind changing the behaviour of set-url.
I do not think we want to change the behaviour of set-url. What
needs to be fixed is the output from "remote -v". It should:
* When there is no pushURL but there is a URL, then show it as
(fetch/push), and you are done;
* When there is one or more pushURLs and a URL, then show the URL
as (fetch), and show pushURLs as (push), and you are done;
* When there are more than one URLs, and there is no pushURL, then
show the first URL as (fetch/push), and the remainder in a
notation that says it is used only for push, but it shouldn't be
the same "(push)"; the user has to be able to distinguish it from
the pushURLs in a repository that also has URLs.
* When there are more than one URLs, and there are one or more
pushURLs, then show the first URL as (fetch), the other URLs
as (unused), and the pushURLs as (push).
Strictly speaking, the last one could be a misconfiguration. If you
have:
[remote "origin"]
url = one
url = two
pushurl = three
pushurl = four
then your "git fetch" will go to one, and "git push" will go to
three and four, and two is never used.
It should also be stressed that the third one a supported
configuration. With
[remote "origin"]
url = one
url = two
your "git fetch" goes to one, and your "git push" will go to one and
two. This is the originally intended use case of 755225d. It is to
push to and fetch from master.k.org (think of "one" above) and in
addition to push to backup.github.com ("two").
^ permalink raw reply
* Re: [RFC/PATCH] ignore memcmp() overreading in bsearch() callback
From: Jeff King @ 2013-01-15 15:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Carlos Martín Nieto, Johannes Schindelin
In-Reply-To: <7v38y38hhm.fsf@alter.siamese.dyndns.org>
On Mon, Jan 14, 2013 at 03:36:21PM -0800, Junio C Hamano wrote:
> It appears that memcmp() uses the usual "one word at a time"
> comparison and triggers valgrind in a callback of bsearch() used in
> the refname search. I can easily trigger problems in any script
> with test_commit (e.g. "sh t0101-at-syntax.sh --valgrind -i -v")
> without this suppression.
Out of curiosity, what platform do you see this on? I can't reproduce on
glibc.
> diff --git a/t/valgrind/default.supp b/t/valgrind/default.supp
> index 0a6724f..032332f 100644
> --- a/t/valgrind/default.supp
> +++ b/t/valgrind/default.supp
> @@ -49,3 +49,11 @@
> Memcheck:Addr4
> fun:copy_ref
> }
> +
> +{
> + ignore-memcmp-reading-too-much-in-bsearch-callback
> + Memcheck:Addr4
> + fun:ref_entry_cmp_sslice
> + fun:bsearch
> + fun:search_ref_dir
> +}
Given that it is valgrind-clean on my platform, and reading the code I
don't see any problems, I think it probably is a false positive, and
this suppression makes sense.
-Peff
^ permalink raw reply
* Re: [PATCH v2 00/14] Remove unused code from imap-send.c
From: Jeff King @ 2013-01-15 14:42 UTC (permalink / raw)
To: Michael Haggerty; +Cc: Junio C Hamano, git
In-Reply-To: <1358237193-8887-1-git-send-email-mhagger@alum.mit.edu>
On Tue, Jan 15, 2013 at 09:06:18AM +0100, Michael Haggerty wrote:
> This is a re-roll, incorporating the feedback of Jonathan Nieder
> (thanks!).
Thanks, I don't see anything wrong with this from a cursory reading.
> * Added some comments to lf_to_crlf(), simplified the code a bit
> further, and expanded the commit message.
I found this version pretty easy to read (the comments helped a lot).
-Peff
^ permalink raw reply
* [PATCH v2] test-lib.sh: unfilter GIT_PERF_*
From: Nguyễn Thái Ngọc Duy @ 2013-01-15 13:50 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Thomas Rast,
Nguyễn Thái Ngọc Duy
In-Reply-To: <1358254409-15187-1-git-send-email-pclouds@gmail.com>
These variables are user parameters to control how to run the perf
tests. Allow users to do so.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
t/test-lib.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index f50f834..e1c8c85 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -85,7 +85,7 @@ unset VISUAL EMAIL LANGUAGE COLUMNS $("$PERL_PATH" -e '
.*_TEST
PROVE
VALGRIND
- PERF_AGGREGATING_LATER
+ PERF
));
my @vars = grep(/^GIT_/ && !/^GIT_($ok)/o, @env);
print join("\n", @vars);
--
1.8.0.rc2.23.g1fb49df
^ permalink raw reply related
* Re: [PATCH] test-lib.sh: unfilter GIT_PERF_*
From: Duy Nguyen @ 2013-01-15 13:47 UTC (permalink / raw)
To: Thomas Rast; +Cc: git, Junio C Hamano
In-Reply-To: <87ehhmr28g.fsf@pctrast.inf.ethz.ch>
On Tue, Jan 15, 2013 at 8:43 PM, Thomas Rast <trast@student.ethz.ch> wrote:
> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
>
>> These variables are user parameters to control how to run the perf
>> tests. Allow users to do so.
> [...]
>> @@ -86,6 +86,9 @@ unset VISUAL EMAIL LANGUAGE COLUMNS $("$PERL_PATH" -e '
>> PROVE
>> VALGRIND
>> PERF_AGGREGATING_LATER
>> + PERF_LARGE_REPO
>> + PERF_REPEAT_COUNT
>> + PERF_REPO
>> ));
>
> Wouldn't it be more futureproof to put simply PERF as an entry, and rely
> on the leading-match logic
>
>> my @vars = grep(/^GIT_/ && !/^GIT_($ok)/o, @env);
>
> to allow all GIT_PERF variables?
Yeah.
> Other than that, Ack. I never noticed because I set mine through
> config.mak, which goes to GIT-BUILD-OPTIONS. Those options are not
> exported, which means perl does not pick them up. (That just took me
> far too long to realize.)
By the way is there an option to skip the first few runs (too lazy to
check out the source code, apparently)? I tried linux-2.6 as the large
repo and I think the first (cold cache) run ruins the numbers.
--
Duy
^ permalink raw reply
* Re: [PATCH] test-lib.sh: unfilter GIT_PERF_*
From: Thomas Rast @ 2013-01-15 13:43 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, Junio C Hamano, Thomas Rast
In-Reply-To: <1358254409-15187-1-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> These variables are user parameters to control how to run the perf
> tests. Allow users to do so.
[...]
> @@ -86,6 +86,9 @@ unset VISUAL EMAIL LANGUAGE COLUMNS $("$PERL_PATH" -e '
> PROVE
> VALGRIND
> PERF_AGGREGATING_LATER
> + PERF_LARGE_REPO
> + PERF_REPEAT_COUNT
> + PERF_REPO
> ));
Wouldn't it be more futureproof to put simply PERF as an entry, and rely
on the leading-match logic
> my @vars = grep(/^GIT_/ && !/^GIT_($ok)/o, @env);
to allow all GIT_PERF variables?
Other than that, Ack. I never noticed because I set mine through
config.mak, which goes to GIT-BUILD-OPTIONS. Those options are not
exported, which means perl does not pick them up. (That just took me
far too long to realize.)
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* [PATCH] attr: fix off-by-one directory component length calculation
From: Nguyễn Thái Ngọc Duy @ 2013-01-15 13:35 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Ross Lagerwall, Jean-Noël AVILA,
Nguyễn Thái Ngọc Duy
94bc671 (Add directory pattern matching to attributes - 2012-12-08)
uses find_basename() to calculate the length of directory part in
prepare_attr_stack. This function expects the directory without the
trailing slash (as "origin" field in match_attr struct is without the
trailing slash). find_basename() includes the trailing slash and
confuses push/pop algorithm.
Consider path = "abc/def" and the push down code:
while (1) {
len = strlen(attr_stack->origin);
if (dirlen <= len)
break;
cp = memchr(path + len + 1, '/', dirlen - len - 1);
if (!cp)
cp = path + dirlen;
dirlen is 4, not 3, without this patch. So when attr_stack->origin is
"abc", it'll miss the exit condition because 4 <= 3 is wrong. It'll
then try to push "abc/" down the attr stack (because "cp" would be
NULL). So we have both "abc" and "abc/" in the stack.
Next time when "abc/ghi" is checked, "abc/" is popped out because of
the off-by-one dirlen, only to be pushed back in again by the above
code. This repeats for all files in the same directory. Which means
at least one failed open syscall per file, or more if .gitattributes
exists.
This is the perf result with 10 runs on git.git:
Test 94bc671^ 94bc671 HEAD
----------------------------------------------------------------------------------------------------------
7810.1: grep worktree, cheap regex 0.02(0.01+0.04) 0.05(0.03+0.05) +150.0% 0.02(0.01+0.04) +0.0%
7810.2: grep worktree, expensive regex 0.25(0.94+0.01) 0.26(0.94+0.02) +4.0% 0.25(0.93+0.02) +0.0%
7810.3: grep --cached, cheap regex 0.11(0.10+0.00) 0.12(0.10+0.02) +9.1% 0.10(0.10+0.00) -9.1%
7810.4: grep --cached, expensive regex 0.61(0.60+0.01) 0.62(0.61+0.01) +1.6% 0.61(0.60+0.00) +0.0%
Reported-by: Ross Lagerwall <rosslagerwall@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
This may be an indication that our perf framework is never actively used :-(
attr.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/attr.c b/attr.c
index 466c93f..bb9a470 100644
--- a/attr.c
+++ b/attr.c
@@ -584,6 +584,13 @@ static void prepare_attr_stack(const char *path)
dirlen = find_basename(path) - path;
/*
+ * find_basename() includes the trailing slash, but we do
+ * _not_ want it.
+ */
+ if (dirlen)
+ dirlen--;
+
+ /*
* At the bottom of the attribute stack is the built-in
* set of attribute definitions, followed by the contents
* of $(prefix)/etc/gitattributes and a file specified by
--
1.8.0.rc2.23.g1fb49df
^ permalink raw reply related
* Re: [PATCH] remote-hg: fix handling of file perms when pushing
From: Max Horn @ 2013-01-15 13:06 UTC (permalink / raw)
To: Max Horn; +Cc: git, Felipe Contreras
In-Reply-To: <1358254959-50435-1-git-send-email-max@quendi.de>
On 15.01.2013, at 14:02, Max Horn wrote:
> Previously, when changing and committing an executable file, the file
> would loose its executable bit on the hg side. Likewise, symlinks ended
> up as "normal" files". This was not immediately apparent on the git side
> unless one did a fresh clone.
Sorry, forgot to sign off, please add:
Signed-off-by: Max Horn <max@quendi.de>
Max
^ permalink raw reply
* [PATCH] remote-hg: fix handling of file perms when pushing
From: Max Horn @ 2013-01-15 13:02 UTC (permalink / raw)
To: git; +Cc: Felipe Contreras, Max Horn
Previously, when changing and committing an executable file, the file
would loose its executable bit on the hg side. Likewise, symlinks ended
up as "normal" files". This was not immediately apparent on the git side
unless one did a fresh clone.
---
contrib/remote-helpers/git-remote-hg | 2 +-
contrib/remote-helpers/test-hg-hg-git.sh | 68 ++++++++++++++++++++++++++++++++
2 files changed, 69 insertions(+), 1 deletion(-)
diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg
index 7c74d8b..328c2dc 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -53,7 +53,7 @@ def gittz(tz):
return '%+03d%02d' % (-tz / 3600, -tz % 3600 / 60)
def hgmode(mode):
- m = { '0100755': 'x', '0120000': 'l' }
+ m = { '100755': 'x', '120000': 'l' }
return m.get(mode, '')
def get_config(config):
diff --git a/contrib/remote-helpers/test-hg-hg-git.sh b/contrib/remote-helpers/test-hg-hg-git.sh
index 3e76d9f..7e3967f 100755
--- a/contrib/remote-helpers/test-hg-hg-git.sh
+++ b/contrib/remote-helpers/test-hg-hg-git.sh
@@ -109,6 +109,74 @@ setup () {
setup
+test_expect_success 'executable bit' '
+ mkdir -p tmp && cd tmp &&
+ test_when_finished "cd .. && rm -rf tmp" &&
+
+ (
+ git init -q gitrepo &&
+ cd gitrepo &&
+ echo alpha > alpha &&
+ chmod 0644 alpha &&
+ git add alpha &&
+ git commit -m "add alpha" &&
+ chmod 0755 alpha &&
+ git add alpha &&
+ git commit -m "set executable bit" &&
+ chmod 0644 alpha &&
+ git add alpha &&
+ git commit -m "clear executable bit"
+ ) &&
+
+ for x in hg git; do
+ (
+ hg_clone_$x gitrepo hgrepo-$x &&
+ cd hgrepo-$x &&
+ hg_log . &&
+ hg manifest -r 1 -v &&
+ hg manifest -v
+ ) > output-$x &&
+
+ git_clone_$x hgrepo-$x gitrepo2-$x &&
+ git_log gitrepo2-$x > log-$x
+ done &&
+ cp -r log-* output-* /tmp/foo/ &&
+
+ test_cmp output-hg output-git &&
+ test_cmp log-hg log-git
+'
+
+test_expect_success 'symlink' '
+ mkdir -p tmp && cd tmp &&
+ test_when_finished "cd .. && rm -rf tmp" &&
+
+ (
+ git init -q gitrepo &&
+ cd gitrepo &&
+ echo alpha > alpha &&
+ git add alpha &&
+ git commit -m "add alpha" &&
+ ln -s alpha beta &&
+ git add beta &&
+ git commit -m "add beta"
+ ) &&
+
+ for x in hg git; do
+ (
+ hg_clone_$x gitrepo hgrepo-$x &&
+ cd hgrepo-$x &&
+ hg_log . &&
+ hg manifest -v
+ ) > output-$x &&
+
+ git_clone_$x hgrepo-$x gitrepo2-$x &&
+ git_log gitrepo2-$x > log-$x
+ done &&
+
+ test_cmp output-hg output-git &&
+ test_cmp log-hg log-git
+'
+
test_expect_success 'merge conflict 1' '
mkdir -p tmp && cd tmp &&
test_when_finished "cd .. && rm -rf tmp" &&
--
1.8.1.448.g79c577a.dirty
^ permalink raw reply related
* [PATCH] test-lib.sh: unfilter GIT_PERF_*
From: Nguyễn Thái Ngọc Duy @ 2013-01-15 12:53 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Thomas Rast,
Nguyễn Thái Ngọc Duy
These variables are user parameters to control how to run the perf
tests. Allow users to do so.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
t/test-lib.sh | 3 +++
1 file changed, 3 insertions(+)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index f50f834..b8d35d1 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -86,6 +86,9 @@ unset VISUAL EMAIL LANGUAGE COLUMNS $("$PERL_PATH" -e '
PROVE
VALGRIND
PERF_AGGREGATING_LATER
+ PERF_LARGE_REPO
+ PERF_REPEAT_COUNT
+ PERF_REPO
));
my @vars = grep(/^GIT_/ && !/^GIT_($ok)/o, @env);
print join("\n", @vars);
--
1.8.0.rc2.23.g1fb49df
^ permalink raw reply related
* Re: [PATCH] remote-hg: store converted URL
From: Max Horn @ 2013-01-15 11:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Felipe Contreras
In-Reply-To: <7vmwwbd43o.fsf@alter.siamese.dyndns.org>
On 14.01.2013, at 19:14, Junio C Hamano wrote:
> Max Horn <max@quendi.de> writes:
>
>> From: Felipe Contreras <felipe.contreras@gmail.com>
>>
>> Mercurial might convert the URL to something more appropriate, like an
>> absolute path.
>
> "What it is converted *TO*" is fairly clear with ", like an ...",
> but from the first reading it was unclear to me "what it is
> converted *FROM*" and "*WHEN* the conversion happens". Do you mean
> that the user gives "git clone" an URL "../hg-repo" via the command
> line (e.g. the argument to "git clone" is spelled something like
> "hg::../hg-repo"), and that "../hg-repo" is rewritten to something
> else (an absolute path, e.g. "/srv/project/hg-repo")?
Yes, that was meant.
>
>> Lets store that instead of the original URL, which won't
>> work from a different working directory if it's relative.
>
> What is lacking from this description is why it even needs to work
> from a different working directory. I am guessing that remote-hg
> later creates a hidden Hg repository or something in a different
> place and still tries to use the URL to interact with the upstream,
> and that is what breaks, but with only the above description without
> looking at your original report, people who will read the "git log"
> output and find this change will not be able to tell why this was
> needed, I am afraid.
>
> Of course, the above guess of mine may even be wrong, but then that
> is yet another reason that the log needs to explain the change
> better.
Fully agreed. How about this commit message:
-- >8 --
remote-hg: store converted URL of hg repo in git config
When remote-hg is invoked, read the remote repository URL from the git config,
give Mercurial a chance to expand it, and if changed, store it back into
the git config.
This fixes the following problem: Suppose you clone a local hg repository
using a relative path, e.g.
git clone hg::hgrepo gitrepo
This stores "hg::hgrepo" in gitrepo/.git/config. However, no information
about the PWD is stored, making it impossible to correctly interpret the
relative path later on. Thus when latter attempting to, say, "git pull"
from inside gitrepo, remote-hg cannot resolve the relative path correctly,
and the user sees an unexpected error.
With this commit, the URL "hg::hgrepo" gets expanded (during cloning,
but also during any other remote operation) and the resulting absolute
URL (e.g. "hg::/abspath/hgrepo") is stored in gitrepo/.git/config.
Thus the git clone of hgrepo becomes usable.
-- >8 --
>
>> Suggested-by: Max Horn <max@quendi.de>
>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>> Signed-off-by: Max Horn <max@quendi.de>
>> ---
>> For a discussion of the problem, see also
>> http://article.gmane.org/gmane.comp.version-control.git/210250
>
> I do not see any discussion; only your problem report.
Aha, an english language issue on my side I guess: For me, a single person can "discuss" a problem (often, a research paper is said to be "discussing a problem"). Sorry for that. Anyway, the reason I gave that link was because it attempts explains the problem and one solution (which this patch ended up implementing), but also express that I feel a bit uncomfortable with this. Which I still do. Relying on the remote helper to invoke "git config" feels like a hack and I was wondering whether this is deemed an acceptable solution -- or whether one should instead extend the remote-helper protocol, allowing the remote helper to signal a rewritten remote URL (perhaps only directly after a clone?). As it is, the remote helper seems (?) to have no way to distinguish whether it is being called duri
ng a clone or a pull; hence it has to "expand" and rewrite the URL every time it is called, just in case.
Anyway, as long as this particular command works somehow, I am fine:
git clone hg::../relative/path/to/hg-repo git-repo
> Was this work done outside the list? I just want to make sure this
> patch is not something Felipe did not want to sign off for whatever
> reason but you are passing it to the list as a patch signed off by
> him.
The work was done by Felipe's and published in his github repository:
https://github.com/felipec/git/commit/605bad5b52d2fcf3d8f5fd782a87d7c97d1b040a
See also the discussion (yeah, this time a real one ;-) leading to this:
https://github.com/felipec/git/issues/2
I took his sign-off from there and interpreted it as saying that Felipe was OK with this being pushed to git.git. But perhaps this is not what I should have done? In that case I am very sorry :-(. It's just that I feel this patch is quite useful and important for daily use (which is why I suggested it in the first place ;-), so I was/am quite eager to see it in.
Cheers,
Max
PS: recently, yet another tool has (re)emerged for using hg repos from inside git:
https://github.com/buchuki/gitifyhg
This is partially based on Felipe's work, but has several bug fixes atop that. It is also seems to be a priority for its author, so it os more actively developed... anyway, that's now, what, "solution" #5 or #6? I really hope the dust on this will settle soon and we'll have just one (or maybe two) tools doing a decent job, instead of attention splitting over so many different ones...
^ permalink raw reply
* Re: [PATCH v2 3/3] diff: Introduce --diff-algorithm command line option
From: Michal Privoznik @ 2013-01-15 10:07 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, peff, trast
In-Reply-To: <7vvcaz8of4.fsf@alter.siamese.dyndns.org>
On 14.01.2013 22:06, Junio C Hamano wrote:
> Michal Privoznik <mprivozn@redhat.com> writes:
>
>> +--diff-algorithm={patience|minimal|histogram|myers}::
>> + Choose a diff algorithm. The variants are as follows:
>> ++
>> +--
>> +`myers`;;
>> + The basic greedy diff algorithm.
>> +`minimal`;;
>> + Spend extra time to make sure the smallest possible diff is
>> + produced.
>> +`patience`;;
>> + Use "patience diff" algorithm when generating patches.
>> +`histogram`;;
>> + This algorithm extends the patience algorithm to "support
>> + low-occurrence common elements".
>> +--
>> ++
>> +For instance, if you configured diff.algorithm variable to a
>> +non-default value and want to use the default one, then you
>> +have to use `--diff-algorithm=myers` option.
>> +
>> +You should prefer this option over the `--minimal`, `--patience` and
>> +`--histogram` which are kept just for backwards compatibility.
>
> Much better; I'd drop the last paragraph, though.
>
> I also think we really should consider "default" synonym for
> whichever happens to be the built-in default (currently myers).
>
>> diff --git a/diff.c b/diff.c
>> index e9a7e4d..3e021d5 100644
>> --- a/diff.c
>> +++ b/diff.c
>> @@ -144,7 +144,7 @@ static int git_config_rename(const char *var, const char *value)
>> return git_config_bool(var,value) ? DIFF_DETECT_RENAME : 0;
>> }
>>
>> -static long parse_algorithm_value(const char *value)
>> +long parse_algorithm_value(const char *value)
>> {
>> if (!value || !strcasecmp(value, "myers"))
>> return 0;
>> @@ -3633,6 +3633,16 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
>> options->xdl_opts = DIFF_WITH_ALG(options, PATIENCE_DIFF);
>> else if (!strcmp(arg, "--histogram"))
>> options->xdl_opts = DIFF_WITH_ALG(options, HISTOGRAM_DIFF);
>> + else if (!prefixcmp(arg, "--diff-algorithm=")) {
>> + long value = parse_algorithm_value(arg+17);
>> + if (value < 0)
>> + return error("option diff-algorithm accepts \"myers\", "
>> + "\"minimal\", \"patience\" and \"histogram\"");
>> + /* clear out previous settings */
>> + DIFF_XDL_CLR(options, NEED_MINIMAL);
>> + options->xdl_opts &= ~XDF_DIFF_ALGORITHM_MASK;
>> + options->xdl_opts |= value;
>
> This makes me wonder if other places that use DIFF_WITH_ALG() also
> need to worry about clearing NEED_MINIMAL?
>
Not really. In my approach, --minimal looks at yet another algorithm.
However, current code sees it as orthogonal to the myers algorithm. The
flag doesn't have any effect on --patience or --histogram at all.
So I think we need to clear the flag only when using --diff-algorithm.
Michal
^ permalink raw reply
* Re: [BUG] Possible bug in `remote set-url --add --push`
From: Michael J Gruber @ 2013-01-15 9:44 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jardel Weyrich, Sascha Cunz, git@vger.kernel.org
In-Reply-To: <7vip6z54rh.fsf@alter.siamese.dyndns.org>
Junio C Hamano venit, vidit, dixit 15.01.2013 07:39:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Jardel Weyrich <jweyrich@gmail.com> writes:
>>
>>> If you allow me, I'd like you to forget about the concepts for a minute, and focus on the user experience.
>>> Imagine a simple hypothetical scenario in which the user wants to push to 2 distinct repositories. He already has cloned the repo from the 1st repository, thus (theoretically) all he needs to do, is to add a new repository for push. He then uses `remote set-url --add --push <2nd-repo>` (which I personally thought would suffice). However, if he tries to push a new commit to this remote, it would be pushed _only_ to the 2nd-repo.
>>
>> The primary reason behind push-url was that
>>
>> (1) usually you push to and fetch from the same, so no pushUrl is
>> ever needed, just a single Url will do (this is often true for
>> cvs/svn style shared repository workflow); and
>>
>> (2) sometimes you want to fetch from one place and push to another
>> (this is often true for "fetch from upstream, push to my own
>> and ask upstream to pull from it" workflow), and in that case
>> you want pushUrl in addition to Url. Most importantly, in this
>> case, you do *NOT* want to push to Url. You only push to
>> pushUrl.
>>
>> Setting *one* pushURL is a way to say "That URL I fetch from is
>> *not* the place I want to push (I may not even be able to push
>> there); when I say 'push', push there instead". Your proposed
>> semantics will make it impossible to arrange such an asymmetric
>> setting.
>
> Now I think I finally see where that misunderstanding comes from.
> It is "remote -v" that is misdesigned.
>
> $ git clone ../there here
> $ cd here
> $ git remote -v
> origin /var/tmp/there (fetch)
> origin /var/tmp/there (push)
>
> This is totally bogus. It should report something like this:
>
> $ git remote -v
> origin /var/tmp/there (fetch/push)
>
> Then after running "git remote set-url --push origin ../another" we
> should see
>
> $ git remote -v
> origin /var/tmp/there (fetch)
> origin /var/tmp/another (push)
>
> which would make it clear that the original fetch/push came from the
> (1) usuall you push and fetch from the same place so there is only
> one setting, and the two lines came from the (2) sometimes you need
> a separate places to fetch from and push to.
Yes, that is one big source of misunderstanding. Cleaning up remote -v
would help, along with the man page.
Also there is a conceptual confusion: pushurl is meant to push to the
same repo using a different url, e.g. something authenticated
(https/ssh) for push and something faster/easier for fetch.
It never was meant to push to several repos. That is what "remotes" are
for, and it would help if we could push to a remote group (which is
difficult because of refspecs etc.) easily.
That being said, I don't mind changing the behaviour of set-url.
> At this point, if you say "set-url --push origin ../third", then
> "another" will disappear and gets replaced by "third"; if you
> instead say "set-url --add --push origin ../third", then we will see
> two (push) lines, in addition to one (fetch), making it clear that
> you are still in (2) above, fetching from and pushing to different
> places, and having two places to push to.
>
> I misread your response
>
> From: Jardel Weyrich <jweyrich@gmail.com>
> Subject: Re: [BUG] Possible bug in `remote set-url --add --push`
> Date: Sat, 12 Jan 2013 06:09:35 -0200
> Message-ID: <CAN8TAOvP_HX6BEK86aYoX-kVqWDmsbyptxTT2nk+fx+Ut1Tojg@mail.gmail.com>
>
> where you showed that there was only remote.origin.url (and no
> pushurl) in the first step, and somehow thought you had a
> remote.origin.pushurl in the first place.
>
^ permalink raw reply
* [PATCH v2 14/14] imap-send.c: simplify logic in lf_to_crlf()
From: Michael Haggerty @ 2013-01-15 8:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Michael Haggerty
In-Reply-To: <1358237193-8887-1-git-send-email-mhagger@alum.mit.edu>
* The first character in the string used to be special-cased to get
around the fact that msg->buf[i - 1] is not defined for i == 0.
Instead, keep track of the previous character in a separate
variable, "lastc", initialized in such a way to let the loop handle
i == 0 correctly.
* Make the two loops over the string look as similar as possible to
make it more obvious that the count computed in the first pass
agrees with the true length of the new string written in the second
pass. As a side effect, this makes it possible to use the "j"
counter in place of lfnum and new_len.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
imap-send.c | 52 +++++++++++++++++++++++-----------------------------
1 file changed, 23 insertions(+), 29 deletions(-)
diff --git a/imap-send.c b/imap-send.c
index f2933e9..1d40207 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -1081,42 +1081,36 @@ bail:
return NULL;
}
+/*
+ * Insert CR characters as necessary in *msg to ensure that every LF
+ * character in *msg is preceded by a CR.
+ */
static void lf_to_crlf(struct strbuf *msg)
{
- size_t new_len;
char *new;
- int i, j, lfnum = 0;
-
- if (msg->buf[0] == '\n')
- lfnum++;
- for (i = 1; i < msg->len; i++) {
- if (msg->buf[i - 1] != '\r' && msg->buf[i] == '\n')
- lfnum++;
+ size_t i, j;
+ char lastc;
+
+ /* First pass: tally, in j, the size of the new string: */
+ for (i = j = 0, lastc = '\0'; i < msg->len; i++) {
+ if (msg->buf[i] == '\n' && lastc != '\r')
+ j++; /* a CR will need to be added here */
+ lastc = msg->buf[i];
+ j++;
}
- new_len = msg->len + lfnum;
- new = xmalloc(new_len + 1);
- if (msg->buf[0] == '\n') {
- new[0] = '\r';
- new[1] = '\n';
- i = 1;
- j = 2;
- } else {
- new[0] = msg->buf[0];
- i = 1;
- j = 1;
- }
- for ( ; i < msg->len; i++) {
- if (msg->buf[i] != '\n') {
- new[j++] = msg->buf[i];
- continue;
- }
- if (msg->buf[i - 1] != '\r')
+ new = xmalloc(j + 1);
+
+ /*
+ * Second pass: write the new string. Note that this loop is
+ * otherwise identical to the first pass.
+ */
+ for (i = j = 0, lastc = '\0'; i < msg->len; i++) {
+ if (msg->buf[i] == '\n' && lastc != '\r')
new[j++] = '\r';
- /* otherwise it already had CR before */
- new[j++] = '\n';
+ lastc = new[j++] = msg->buf[i];
}
- strbuf_attach(msg, new, new_len, new_len + 1);
+ strbuf_attach(msg, new, j, j + 1);
}
/*
--
1.8.0.3
^ permalink raw reply related
* [PATCH v2 11/14] imap-send.c: use struct imap_store instead of struct store
From: Michael Haggerty @ 2013-01-15 8:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Michael Haggerty
In-Reply-To: <1358237193-8887-1-git-send-email-mhagger@alum.mit.edu>
In fact, all struct store instances are upcasts of struct imap_store
anyway, so stop making the distinction.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
imap-send.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/imap-send.c b/imap-send.c
index 909e4db..48c646c 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -782,9 +782,9 @@ static void imap_close_server(struct imap_store *ictx)
free(imap);
}
-static void imap_close_store(struct store *ctx)
+static void imap_close_store(struct imap_store *ctx)
{
- imap_close_server((struct imap_store *)ctx);
+ imap_close_server(ctx);
free(ctx);
}
@@ -869,7 +869,7 @@ static int auth_cram_md5(struct imap_store *ctx, struct imap_cmd *cmd, const cha
return 0;
}
-static struct store *imap_open_store(struct imap_server_conf *srvc)
+static struct imap_store *imap_open_store(struct imap_server_conf *srvc)
{
struct imap_store *ctx;
struct imap *imap;
@@ -1079,10 +1079,10 @@ static struct store *imap_open_store(struct imap_server_conf *srvc)
} /* !preauth */
ctx->prefix = "";
- return (struct store *)ctx;
+ return ctx;
bail:
- imap_close_store(&ctx->gen);
+ imap_close_store(ctx);
return NULL;
}
@@ -1128,9 +1128,8 @@ static void lf_to_crlf(struct strbuf *msg)
* Store msg to IMAP. Also detach and free the data from msg->data,
* leaving msg->data empty.
*/
-static int imap_store_msg(struct store *gctx, struct strbuf *msg)
+static int imap_store_msg(struct imap_store *ctx, struct strbuf *msg)
{
- struct imap_store *ctx = (struct imap_store *)gctx;
struct imap *imap = ctx->imap;
struct imap_cmd_cb cb;
const char *prefix, *box;
@@ -1142,7 +1141,7 @@ static int imap_store_msg(struct store *gctx, struct strbuf *msg)
cb.dlen = msg->len;
cb.data = strbuf_detach(msg, NULL);
- box = gctx->name;
+ box = ctx->gen.name;
prefix = !strcmp(box, "INBOX") ? "" : ctx->prefix;
cb.create = 0;
ret = imap_exec_m(ctx, &cb, "APPEND \"%s%s\" ", prefix, box);
@@ -1298,7 +1297,7 @@ int main(int argc, char **argv)
{
struct strbuf all_msgs = STRBUF_INIT;
struct strbuf msg = STRBUF_INIT;
- struct store *ctx = NULL;
+ struct imap_store *ctx = NULL;
int ofs = 0;
int r;
int total, n = 0;
@@ -1354,7 +1353,7 @@ int main(int argc, char **argv)
}
fprintf(stderr, "sending %d message%s\n", total, (total != 1) ? "s" : "");
- ctx->name = imap_folder;
+ ctx->gen.name = imap_folder;
while (1) {
unsigned percent = n * 100 / total;
--
1.8.0.3
^ permalink raw reply related
* [PATCH v2 02/14] imap-send.c: remove struct msg_data
From: Michael Haggerty @ 2013-01-15 8:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Michael Haggerty
In-Reply-To: <1358237193-8887-1-git-send-email-mhagger@alum.mit.edu>
Now that its flags member has been deleted, all that is left is a
strbuf. So use a strbuf directly.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
imap-send.c | 18 +++++++-----------
1 file changed, 7 insertions(+), 11 deletions(-)
diff --git a/imap-send.c b/imap-send.c
index f1c8f5a..29c10a4 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -68,10 +68,6 @@ struct store {
int recent; /* # of recent messages - don't trust this beyond the initial read */
};
-struct msg_data {
- struct strbuf data;
-};
-
static const char imap_send_usage[] = "git imap-send < <mbox>";
#undef DRV_OK
@@ -1279,7 +1275,7 @@ static void lf_to_crlf(struct strbuf *msg)
* Store msg to IMAP. Also detach and free the data from msg->data,
* leaving msg->data empty.
*/
-static int imap_store_msg(struct store *gctx, struct msg_data *msg)
+static int imap_store_msg(struct store *gctx, struct strbuf *msg)
{
struct imap_store *ctx = (struct imap_store *)gctx;
struct imap *imap = ctx->imap;
@@ -1287,11 +1283,11 @@ static int imap_store_msg(struct store *gctx, struct msg_data *msg)
const char *prefix, *box;
int ret;
- lf_to_crlf(&msg->data);
+ lf_to_crlf(msg);
memset(&cb, 0, sizeof(cb));
- cb.dlen = msg->data.len;
- cb.data = strbuf_detach(&msg->data, NULL);
+ cb.dlen = msg->len;
+ cb.data = strbuf_detach(msg, NULL);
box = gctx->name;
prefix = !strcmp(box, "INBOX") ? "" : ctx->prefix;
@@ -1449,7 +1445,7 @@ static int git_imap_config(const char *key, const char *val, void *cb)
int main(int argc, char **argv)
{
struct strbuf all_msgs = STRBUF_INIT;
- struct msg_data msg = {STRBUF_INIT};
+ struct strbuf msg = STRBUF_INIT;
struct store *ctx = NULL;
int ofs = 0;
int r;
@@ -1511,10 +1507,10 @@ int main(int argc, char **argv)
unsigned percent = n * 100 / total;
fprintf(stderr, "%4u%% (%d/%d) done\r", percent, n, total);
- if (!split_msg(&all_msgs, &msg.data, &ofs))
+ if (!split_msg(&all_msgs, &msg, &ofs))
break;
if (server.use_html)
- wrap_in_html(&msg.data);
+ wrap_in_html(&msg);
r = imap_store_msg(ctx, &msg);
if (r != DRV_OK)
break;
--
1.8.0.3
^ permalink raw reply related
* [PATCH v2 07/14] imap-send.c: inline imap_parse_list() in imap_list()
From: Michael Haggerty @ 2013-01-15 8:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Michael Haggerty
In-Reply-To: <1358237193-8887-1-git-send-email-mhagger@alum.mit.edu>
The function is only called from here.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
imap-send.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/imap-send.c b/imap-send.c
index f193211..cbbf845 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -669,21 +669,16 @@ bail:
return -1;
}
-static struct imap_list *parse_imap_list(struct imap *imap, char **sp)
+static struct imap_list *parse_list(char **sp)
{
struct imap_list *head;
- if (!parse_imap_list_l(imap, sp, &head, 0))
+ if (!parse_imap_list_l(NULL, sp, &head, 0))
return head;
free_list(head);
return NULL;
}
-static struct imap_list *parse_list(char **sp)
-{
- return parse_imap_list(NULL, sp);
-}
-
static void parse_capability(struct imap *imap, char *cmd)
{
char *arg;
--
1.8.0.3
^ permalink raw reply related
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