* [PATCH] git-p4: fix problem when p4 login is not necessary
From: Peter Osterlund @ 2019-01-07 20:51 UTC (permalink / raw)
To: Git Users; +Cc: gitster, Luke Diamand
In a perforce setup where login is not required, communication fails
because p4_check_access does not understand the response from the p4
client. Fixed by detecting and ignoring the "info" response.
Signed-off-by: Peter Osterlund <peterosterlund2@gmail.com>
---
git-p4.py | 2 ++
1 file changed, 2 insertions(+)
diff --git a/git-p4.py b/git-p4.py
index 1998c3e141..3e12774f96 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -332,6 +332,8 @@ def p4_check_access(min_expiration=1):
die_bad_access("p4 error: {0}".format(data))
else:
die_bad_access("unknown error")
+ elif code == "info":
+ return
else:
die_bad_access("unknown error code {0}".format(code))
--
Peter Osterlund - peterosterlund2@gmail.com
http://hem.bredband.net/petero2b
^ permalink raw reply related
* Translation of git manpages
From: Jean-Noël AVILA @ 2019-01-07 20:45 UTC (permalink / raw)
To: Alexander Shopov, Jordi Mas, Ralf Thielow, Christopher Díaz,
Jean-Noël Avila, Marco Paolone, Gwan-gyeong Mun,
Vasco Almeida, Dimitriy Ryazantcev, Peter Krefting,
Trần Ngọc Quân, Alessandro Menti, Jiang Xin
Cc: git
Dear fellow translators,
I'm trying to put up a longstanding project of providing translated manual
pages for Git. After several experiments, the best choice seemed to be the use
of po4a[1] to convert the asciidoc[2] sources of git manpages into po files
that could be processed the same way we are already doing for the translation
of Git itself.
The text is segmented into paragraphs and structural units (titles, list
items...) and when translated, they are reinjected into the original text
structure. Only inline asciidoc formatting marks are passed in segments.
The translation takes place in a dedicated repository[3] . It simpler to not
meddle in git main workflow while adjusting the translation workflow. If
everybody is satisfied with it, we can maybe migrate the repo under git
organization. Now, this repo is standalone with respect to translation content
source, but a patch has been submitted so that the translated manpages can be
generated and installed from the main git project[4]. Symmetrically, there's a
script in the project to pull the manpages source files from the main git repo.
The repository is connected to Weblate[5] if you have collaborators who don't
know how to process po files and prefer translating in the browser.
The repository is also open to pull-request on the tooling. Let me know if you
have issues. In any case, the translation work can be reused for any other
arrangements.
There is already a kernel of translation in French, from my experiments and a
previous effort of German translation[6] was gettextized. If you have such
archives for other languages, I'd be happy to integrate them, even if they are
not up to date.
--
Jean-Noël
[1]: https://po4a.org/
[2]: http://asciidoc.org/
[3]: https://github.com/jnavila/git-manpages-l10n
[4]: https://public-inbox.org/git/20190104165406.22358-1-jn.avila@free.fr/
[5]: https://hosted.weblate.org/projects/git-manpages/translations/
[6]: https://repo.or.cz/w/gitman-de.git
^ permalink raw reply
* Re: [PATCH v4 0/8] Reimplement rebase --merge via interactive machinery
From: Elijah Newren @ 2019-01-07 20:39 UTC (permalink / raw)
To: Junio C Hamano
Cc: Git Mailing List, Johannes Schindelin, Pratik Karki, Phillip Wood
In-Reply-To: <xmqqzhsc6xdk.fsf@gitster-ct.c.googlers.com>
Hi,
On Mon, Jan 7, 2019 at 12:11 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Junio C Hamano <gitster@pobox.com> writes:
>
> > Elijah Newren <newren@gmail.com> writes:
> >
> >> On Tue, Dec 11, 2018 at 8:11 AM Elijah Newren <newren@gmail.com> wrote:
> >>>
> >>> This series continues the work of making rebase more self-consistent
> >>> by removing inconsistencies between different backends. In
> >>> particular, this series focuses on making the merge machinery behave
> >>> like the interactive machinery (though a few differences between the am
> >>> and interactive backends are also fixed along the way), and ultimately
> >>> removes the merge backend in favor of reimplementing the relevant
> >>> options on top of the interactive machinery.
> >>
> >> Friendly ping...let me know if you want me to simply resend v4.
> >>
> >
> > If you have anything newer than 90673135 ("rebase: Implement --merge
> > via the interactive machinery", 2018-12-11), then yeah, I haven't
> > seen it.
> >
> > Thanks.
> >
> > P.S. even if that one is latest, I would need to downcase Implement
> > before it hits 'next' ;-)
>
> Ah, one thing I forgot to mention. Some of the tests updated in
> this series are unhappy with Dscho's "drive 'am' directly from the
> built-in code, bypassing git-rebase--am.sh scriptlet" topic.
2018-12-11 is the newest (and is almost the same as the version from
mid November); it's just been waiting for review. I'll fix up the
casing of 'Implement' along with any other feedback, if any...maybe
including rebasing on Dscho's series depending on how he wants to take
it.
Dscho: Looks like our series conflicts slightly. Would you like me to
rebase mine on top of yours and squash the following change into
commit c91c944a068e ("rebase: define linearization ordering and
enforce it", 2018-12-11), or do you want to rebase your series on mine
and either make a new commit out of this change or squash it in
somewhere?
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 0317280f83..54023547ff 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -578,7 +578,8 @@ static int run_am(struct rebase_options *opts)
argv_array_pushl(&format_patch.args, "format-patch", "-k", "--stdout",
"--full-index", "--cherry-pick", "--right-only",
"--src-prefix=a/", "--dst-prefix=b/", "--no-renames",
- "--no-cover-letter", "--pretty=mboxrd", NULL);
+ "--no-cover-letter", "--pretty=mboxrd",
+ "--topo-order", NULL);
if (opts->git_format_patch_opt.len)
argv_array_split(&format_patch.args,
opts->git_format_patch_opt.buf);
Elijah
^ permalink raw reply related
* Re: [PATCH v4 0/8] Reimplement rebase --merge via interactive machinery
From: Junio C Hamano @ 2019-01-07 20:11 UTC (permalink / raw)
To: Elijah Newren
Cc: Git Mailing List, Johannes Schindelin, Pratik Karki, Phillip Wood
In-Reply-To: <xmqq4lak8d4g.fsf@gitster-ct.c.googlers.com>
Junio C Hamano <gitster@pobox.com> writes:
> Elijah Newren <newren@gmail.com> writes:
>
>> On Tue, Dec 11, 2018 at 8:11 AM Elijah Newren <newren@gmail.com> wrote:
>>>
>>> This series continues the work of making rebase more self-consistent
>>> by removing inconsistencies between different backends. In
>>> particular, this series focuses on making the merge machinery behave
>>> like the interactive machinery (though a few differences between the am
>>> and interactive backends are also fixed along the way), and ultimately
>>> removes the merge backend in favor of reimplementing the relevant
>>> options on top of the interactive machinery.
>>
>> Friendly ping...let me know if you want me to simply resend v4.
>>
>
> If you have anything newer than 90673135 ("rebase: Implement --merge
> via the interactive machinery", 2018-12-11), then yeah, I haven't
> seen it.
>
> Thanks.
>
> P.S. even if that one is latest, I would need to downcase Implement
> before it hits 'next' ;-)
Ah, one thing I forgot to mention. Some of the tests updated in
this series are unhappy with Dscho's "drive 'am' directly from the
built-in code, bypassing git-rebase--am.sh scriptlet" topic.
^ permalink raw reply
* Re: [PATCH v4 0/8] Reimplement rebase --merge via interactive machinery
From: Junio C Hamano @ 2019-01-07 19:46 UTC (permalink / raw)
To: Elijah Newren
Cc: Git Mailing List, Johannes Schindelin, Pratik Karki, Phillip Wood
In-Reply-To: <CABPp-BE83Oe15U4yrkcV_-qzWocMS4UcVeG1VEoac-jXgw9Peg@mail.gmail.com>
Elijah Newren <newren@gmail.com> writes:
> On Tue, Dec 11, 2018 at 8:11 AM Elijah Newren <newren@gmail.com> wrote:
>>
>> This series continues the work of making rebase more self-consistent
>> by removing inconsistencies between different backends. In
>> particular, this series focuses on making the merge machinery behave
>> like the interactive machinery (though a few differences between the am
>> and interactive backends are also fixed along the way), and ultimately
>> removes the merge backend in favor of reimplementing the relevant
>> options on top of the interactive machinery.
>
> Friendly ping...let me know if you want me to simply resend v4.
>
If you have anything newer than 90673135 ("rebase: Implement --merge
via the interactive machinery", 2018-12-11), then yeah, I haven't
seen it.
Thanks.
P.S. even if that one is latest, I would need to downcase Implement
before it hits 'next' ;-)
>> Differences since v3 (full range-diff below):
>> - Fixed the redundant "fatal: error:" error message prefixes, as pointed
>> out by Duy
>> - Rebased on 2.20.0
>>
>> Elijah Newren (8):
>> rebase: make builtin and legacy script error messages the same
>> rebase: fix incompatible options error message
>> t5407: add a test demonstrating how interactive handles --skip
>> differently
>> am, rebase--merge: do not overlook --skip'ed commits with post-rewrite
>> git-rebase, sequencer: extend --quiet option for the interactive
>> machinery
>> git-legacy-rebase: simplify unnecessary triply-nested if
>> rebase: define linearization ordering and enforce it
>> rebase: Implement --merge via the interactive machinery
>>
> ...
^ permalink raw reply
* Re: Recovering from a "detached from" HEAD
From: Junio C Hamano @ 2019-01-07 19:43 UTC (permalink / raw)
To: Alyssa Ross; +Cc: git
In-Reply-To: <20190107161748.pyhgpewymdgjmgoh@x220>
Alyssa Ross <hi@alyssa.is> writes:
> If I detach my head, then use `git subtree add` to generate a commit,
> the output from `git status` changes from "detached at SHA" to
> "detached from SHA". The sha doesn't change, but HEAD has updated.
This is expected, and there is nothing to "recover from". Use of
"git subtree add" should be irrelevant (iow, you should get the same
behaviour no matter _how_ you build new commits on the unnamed
branch).
$ git checkout --detach
$ git status -uno
HEAD detached at 9745ede235
$ git commit --allow-empty -m empty
[detached HEAD bc9a31f2df] empty
$ git status -uno
HEAD detached from 9745ede235
The commit the message shows is meant to indicate where your unnamed
branch diverged at named branches. Immediately after moving to the
unnamed branch by detaching the HEAD, the message says "at"; the
HEAD is pointing directly at the tip of the then-current branch and
that is where the tip of the unnamed branch is. You can tell from
that message that you will not lose any commit if you were to check
out a named branch from that state.
After you make a commit on the unnamed branch, you have something to
lose if you were to check out a named branch from that state, as the
detached HEAD is the _only_ thing these new commits you built on top
of the fork point. Upon seeing "HEAD detached from 9745ede235", you
could do "git log 9745ede235.." and see what you would end up losing
if you were to switch to another branch without saving them first to
a named branch.
> So my question is, what's going on here? Is this intentional behaviour,
> or a bug? How should I get my working tree back to a normal state?
There is nothing abnormal in this state while you are working on an
unnamed branch.
^ permalink raw reply
* Re: [PATCH v2 2/2] pack-protocol.txt: accept error packets in any context
From: Jonathan Tan @ 2019-01-07 19:41 UTC (permalink / raw)
To: masayasuzuki; +Cc: git, peff, steadmon, Jonathan Tan
In-Reply-To: <20181229211915.161686-3-masayasuzuki@google.com>
First of all, there was some discussion [1] about its relation with
js/smart-http-detect-remote-error. I noticed that
js/smart-http-detect-remote-error has all tests passing even if I remove
Masaya's patch, so I'm reviewing these patches independently, directly on
master.
[1] https://public-inbox.org/git/CAJB1erU0utjKGtv3LBFT6SEEKCfFRuxGvDtpkKeS3GSC1S89JA@mail.gmail.com/
> In the Git pack protocol definition, an error packet may appear only in
> a certain context. However, servers can face a runtime error (e.g. I/O
> error) at an arbitrary timing. This patch changes the protocol to allow
> an error packet to be sent instead of any packet.
>
> Without this protocol spec change, when a server cannot process a
> request, there's no way to tell that to a client. Since the server
> cannot produce a valid response, it would be forced to cut a connection
> without telling why. With this protocol spec change, the server can be
> more gentle in this situation. An old client may see these error packets
> as an unexpected packet, but this is not worse than having an unexpected
> EOF.
The other thing that happens is that servers send "ERR" anyway, even
though it is not allowed by the protocol.
This overall looks like a good direction - this makes explicit something
that is already being done.
My remaining concern is if "ERR " could be a non-error packet mistaken
for an error one. I glanced through pack-protocol.txt and as far as I
can tell, I don't see anything non-error sent by the server that could
be prefixed with "ERR". (There are push-option and gpg-signature-lines,
but those are sent by the client.) Packfiles can contain anything, of
course, but as far as I can tell, they are either sent un-PKT-ed or
preceded by a sideband (\1), so they are fine.
> diff --git a/serve.c b/serve.c
> index bda085f09..317256c1a 100644
> --- a/serve.c
> +++ b/serve.c
> @@ -167,7 +167,8 @@ static int process_request(void)
>
> packet_reader_init(&reader, 0, NULL, 0,
> PACKET_READ_CHOMP_NEWLINE |
> - PACKET_READ_GENTLE_ON_EOF);
> + PACKET_READ_GENTLE_ON_EOF |
> + PACKET_READ_DIE_ON_ERR_PACKET);
>
> /*
> * Check to see if the client closed their end before sending another
> @@ -175,7 +176,7 @@ static int process_request(void)
> */
> if (packet_reader_peek(&reader) == PACKET_READ_EOF)
> return 1;
> - reader.options = PACKET_READ_CHOMP_NEWLINE;
> + reader.options &= ~PACKET_READ_GENTLE_ON_EOF;
Here, the old line is meant to remove PACKET_READ_GENTLE_ON_EOF - the
new line is both necessary and clearer.
> diff --git a/t/t5703-upload-pack-ref-in-want.sh b/t/t5703-upload-pack-ref-in-want.sh
> index 3f58f05cb..d2a9d0c12 100755
> --- a/t/t5703-upload-pack-ref-in-want.sh
> +++ b/t/t5703-upload-pack-ref-in-want.sh
> @@ -208,7 +208,7 @@ test_expect_success 'server is initially ahead - no ref in want' '
> cp -r "$LOCAL_PRISTINE" local &&
> inconsistency master 1234567890123456789012345678901234567890 &&
> test_must_fail git -C local fetch 2>err &&
> - grep "ERR upload-pack: not our ref" err
> + grep "fatal: remote error: upload-pack: not our ref" err
> '
>
> test_expect_success 'server is initially ahead - ref in want' '
> @@ -254,7 +254,7 @@ test_expect_success 'server loses a ref - ref in want' '
> echo "s/master/raster/" >"$HTTPD_ROOT_PATH/one-time-sed" &&
> test_must_fail git -C local fetch 2>err &&
>
> - grep "ERR unknown ref refs/heads/raster" err
> + grep "fatal: remote error: unknown ref refs/heads/raster" err
> '
And this shows that we have tests that exercise the new code.
The rest of the diff is just the addition of the new
PACKET_READ_DIE_ON_ERR_PACKET, and looks correct.
^ permalink raw reply
* Re: [PATCH 4/3] object-store: retire odb_load_loose_cache()
From: Junio C Hamano @ 2019-01-07 19:32 UTC (permalink / raw)
To: René Scharfe; +Cc: git, Jeff King, Ævar Arnfjörð Bjarmason
In-Reply-To: <a836870b-c803-5ff4-0019-58012773efb7@web.de>
René Scharfe <l.s.r@web.de> writes:
> Inline odb_load_loose_cache() into its only remaining caller,
> odb_loose_cache(). The latter offers a nicer interface for loading the
> cache, as it doesn't require callers to deal with fanout directory
> numbers directly.
>
> Signed-off-by: Rene Scharfe <l.s.r@web.de>
> ---
OK, that's much better ;-) Thanks.
> object-store.h | 7 -------
> sha1-file.c | 9 ++-------
> 2 files changed, 2 insertions(+), 14 deletions(-)
>
> diff --git a/object-store.h b/object-store.h
> index 2fb6c0e4db..e16aa38cae 100644
> --- a/object-store.h
> +++ b/object-store.h
> @@ -47,13 +47,6 @@ void add_to_alternates_file(const char *dir);
> */
> void add_to_alternates_memory(const char *dir);
>
> -/*
> - * Populate an odb's loose object cache for one particular subdirectory (i.e.,
> - * the one that corresponds to the first byte of objects you're interested in,
> - * from 0 to 255 inclusive).
> - */
> -void odb_load_loose_cache(struct object_directory *odb, int subdir_nr);
> -
> /*
> * Populate and return the loose object cache array corresponding to the
> * given object ID.
> diff --git a/sha1-file.c b/sha1-file.c
> index c3c6e50704..efcb2cbe74 100644
> --- a/sha1-file.c
> +++ b/sha1-file.c
> @@ -2154,12 +2154,6 @@ struct oid_array *odb_loose_cache(struct object_directory *odb,
> const struct object_id *oid)
> {
> int subdir_nr = oid->hash[0];
> - odb_load_loose_cache(odb, subdir_nr);
> - return &odb->loose_objects_cache[subdir_nr];
> -}
> -
> -void odb_load_loose_cache(struct object_directory *odb, int subdir_nr)
> -{
> struct strbuf buf = STRBUF_INIT;
>
> if (subdir_nr < 0 ||
> @@ -2167,7 +2161,7 @@ void odb_load_loose_cache(struct object_directory *odb, int subdir_nr)
> BUG("subdir_nr out of range");
>
> if (odb->loose_objects_subdir_seen[subdir_nr])
> - return;
> + return &odb->loose_objects_cache[subdir_nr];
>
> strbuf_addstr(&buf, odb->path);
> for_each_file_in_obj_subdir(subdir_nr, &buf,
> @@ -2176,6 +2170,7 @@ void odb_load_loose_cache(struct object_directory *odb, int subdir_nr)
> &odb->loose_objects_cache[subdir_nr]);
> odb->loose_objects_subdir_seen[subdir_nr] = 1;
> strbuf_release(&buf);
> + return &odb->loose_objects_cache[subdir_nr];
> }
>
> void odb_clear_loose_cache(struct object_directory *odb)
^ permalink raw reply
* Re: [PATCH 1/1] Add optional targets for documentation l10n
From: Junio C Hamano @ 2019-01-07 19:29 UTC (permalink / raw)
To: Jean-Noël AVILA; +Cc: git
In-Reply-To: <220955359.FqXrlbFPp5@cayenne>
Jean-Noël AVILA <jn.avila@free.fr> writes:
>> The idea is to use $(filter PATTERN..., TEXT) that removes words in
>> TEXT that do not match any of the words in PATTERN, and for normal
>> build, MAN_FILTER is set identical to TMP_MAN_TXT (which is the
>> original MAN_TXT), so there is no filtering happen, but in a build
>> that does tweak MAN_FILTER, MAN_TXT can become a subset of the
>> original MAN_TXT.
>>
>> Am I on the right track?
>>
>
> Yes that's exactly the purpose of this trick. In fact, $(filter) in this
> configuration is equivalent to an intersection of lists, so the order does not
> change the end result.
That is only true if MAN_FILTER is literally a list of "I want
exactly these things", without any pattern. Once a future caller
wants to say "We now have translations for pages from [a-m]*", it
becomes apparent again that the order is wrong.
And if the caller is supposed to have a literal list of pages, not a
pattern, then it may be sufficient to update our Makefile so that
the caller can override the literal list of pages we (incrementally)
compute with its own list without any filtering.
> Ah, I see. The filter from MAN{1,5,7}_TXT would ripple the same way as MAN_TXT,
> just one level upstream. The filtering at this level would no longer be
> needed.
Yup. I see you sent v2; let me read it.
Thanks.
^ permalink raw reply
* Re: jk/loose-object-cache
From: Junio C Hamano @ 2019-01-07 19:29 UTC (permalink / raw)
To: René Scharfe; +Cc: git, Jeff King, Ævar Arnfjörð Bjarmason
In-Reply-To: <3512c798-aa42-6fba-ee82-d33a8985be91@web.de>
René Scharfe <l.s.r@web.de> writes:
> Am 28.12.2018 um 19:04 schrieb Junio C Hamano:
>> * jk/loose-object-cache (2018-11-24) 10 commits
> ...
> So this has hit master in the meantime. We discussed a sort performance
> fix in [1]; I'll reply with a short series containing a cleaned-up and
> rebased version as a follow-up.
Thanks.
-- >8 --
Subject: [PATCH 4/3] sha1-file.c: make odb_load_loose_cache() static
Now there is only a single internal caller to this helper function
that knows the implementation detail of the cache too much, hide it
from outside world by making it static.
Suggested-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
object-store.h | 7 -------
sha1-file.c | 24 +++++++++++++++---------
2 files changed, 15 insertions(+), 16 deletions(-)
diff --git a/object-store.h b/object-store.h
index 2fb6c0e4db..e16aa38cae 100644
--- a/object-store.h
+++ b/object-store.h
@@ -47,13 +47,6 @@ void add_to_alternates_file(const char *dir);
*/
void add_to_alternates_memory(const char *dir);
-/*
- * Populate an odb's loose object cache for one particular subdirectory (i.e.,
- * the one that corresponds to the first byte of objects you're interested in,
- * from 0 to 255 inclusive).
- */
-void odb_load_loose_cache(struct object_directory *odb, int subdir_nr);
-
/*
* Populate and return the loose object cache array corresponding to the
* given object ID.
diff --git a/sha1-file.c b/sha1-file.c
index 2adc56cde4..936d216ad4 100644
--- a/sha1-file.c
+++ b/sha1-file.c
@@ -2150,15 +2150,12 @@ static int append_loose_object(const struct object_id *oid, const char *path,
return 0;
}
-struct oid_array *odb_loose_cache(struct object_directory *odb,
- const struct object_id *oid)
-{
- int subdir_nr = oid->hash[0];
- odb_load_loose_cache(odb, subdir_nr);
- return &odb->loose_objects_cache[subdir_nr];
-}
-
-void odb_load_loose_cache(struct object_directory *odb, int subdir_nr)
+/*
+ * Populate an odb's loose object cache for one particular subdirectory (i.e.,
+ * the one that corresponds to the first byte of objects you're interested in,
+ * from 0 to 255 inclusive).
+ */
+static void odb_load_loose_cache(struct object_directory *odb, int subdir_nr)
{
struct strbuf buf = STRBUF_INIT;
@@ -2178,6 +2175,15 @@ void odb_load_loose_cache(struct object_directory *odb, int subdir_nr)
strbuf_release(&buf);
}
+struct oid_array *odb_loose_cache(struct object_directory *odb,
+ const struct object_id *oid)
+{
+ int subdir_nr = oid->hash[0];
+ odb_load_loose_cache(odb, subdir_nr);
+ return &odb->loose_objects_cache[subdir_nr];
+}
+
+
void odb_clear_loose_cache(struct object_directory *odb)
{
int i;
^ permalink raw reply related
* [PATCH] git-multimail: update to release 1.5.0
From: Matthieu Moy @ 2019-01-07 17:48 UTC (permalink / raw)
To: gitster
Cc: git, Matthieu Moy, William Stewart, Ville Skyttä, Dirk Olmes,
Björn Kautler, Konstantin Ryabitsev, Gareth Pye, David Lazar
Changes are described in CHANGES.
Contributions-by: Matthieu Moy <git@matthieu-moy.fr>
Contributions-by: William Stewart <william.stewart@booking.com>
Contributions-by: Ville Skyttä <ville.skytta@iki.fi>
Contributions-by: Dirk Olmes <dirk.olmes@codedo.de>
Contributions-by: Björn Kautler <Bjoern@Kautler.net>
Contributions-by: Konstantin Ryabitsev <konstantin@linuxfoundation.org>
Contributions-by: Gareth Pye <garethp@gpsatsys.com.au>
Contributions-by: David Lazar <lazard@csail.mit.edu>
Signed-off-by: Matthieu Moy <git@matthieu-moy.fr>
---
A very long time since I did a git-multimail release, sorry for the
lack of activity on this project. OTOH, there have not been many
pull-requests nor feature requests, which may be a sign that the
project reached some kind of maturity.
Help is welcome if some people would like to see git-multimail evolve
faster.
contrib/hooks/multimail/CHANGES | 56 ++++++
contrib/hooks/multimail/CONTRIBUTING.rst | 28 ++-
contrib/hooks/multimail/README.Git | 4 +-
.../hooks/multimail/{README => README.rst} | 38 +++-
contrib/hooks/multimail/doc/gitolite.rst | 9 +
contrib/hooks/multimail/git_multimail.py | 188 ++++++++++++++----
.../hooks/multimail/migrate-mailhook-config | 13 +-
contrib/hooks/multimail/post-receive.example | 2 +-
8 files changed, 281 insertions(+), 57 deletions(-)
rename contrib/hooks/multimail/{README => README.rst} (95%)
diff --git a/contrib/hooks/multimail/CHANGES b/contrib/hooks/multimail/CHANGES
index 2076cf972b..35791fd02c 100644
--- a/contrib/hooks/multimail/CHANGES
+++ b/contrib/hooks/multimail/CHANGES
@@ -1,3 +1,59 @@
+Release 1.5.0
+=============
+
+Backward-incompatible change
+----------------------------
+
+The name of classes for environment was misnamed as `*Environement`.
+It is now `*Environment`.
+
+New features
+------------
+
+* A Thread-Index header is now added to each email sent (except for
+ combined emails where it would not make sense), so that MS Outlook
+ properly groups messages by threads even though they have a
+ different subject line. Unfortunately, even adding this header the
+ threading still seems to be unreliable, but it is unclear whether
+ this is an issue on our side or on MS Outlook's side (see discussion
+ here: https://github.com/git-multimail/git-multimail/pull/194).
+
+* A new variable multimailhook.ExcludeMergeRevisions was added to send
+ notification emails only for non-merge commits.
+
+* For gitolite environment, it is now possible to specify the mail map
+ in a separate file in addition to gitolite.conf, using the variable
+ multimailhook.MailaddressMap.
+
+Internal changes
+----------------
+
+* The testsuite now uses GIT_PRINT_SHA1_ELLIPSIS where needed for
+ compatibility with recent Git versions. Only tests are affected.
+
+* We don't try to install pyflakes in the continuous integration job
+ for old Python versions where it's no longer available.
+
+* Stop using the deprecated cgi.escape in Python 3.
+
+* New flake8 warnings have been fixed.
+
+* Python 3.6 is now tested against on Travis-CI.
+
+* A bunch of lgtm.com warnings have been fixed.
+
+Bug fixes
+---------
+
+* SMTPMailer logs in only once now. It used to re-login for each email
+ sent which triggered errors for some SMTP servers.
+
+* migrate-mailhook-config was broken by internal refactoring, it
+ should now work again.
+
+This version was tested with Python 2.6 to 3.7. It was tested with Git
+1.7.10.406.gdc801, 2.15.1 and 2.20.1.98.gecbdaf0.
+
Release 1.4.0
=============
diff --git a/contrib/hooks/multimail/CONTRIBUTING.rst b/contrib/hooks/multimail/CONTRIBUTING.rst
index da65570e9b..de20a54287 100644
--- a/contrib/hooks/multimail/CONTRIBUTING.rst
+++ b/contrib/hooks/multimail/CONTRIBUTING.rst
@@ -4,9 +4,8 @@ Contributing
git-multimail is an open-source project, built by volunteers. We would
welcome your help!
-The current maintainers are Matthieu Moy
-<matthieu.moy@grenoble-inp.fr> and Michael Haggerty
-<mhagger@alum.mit.edu>.
+The current maintainers are `Matthieu Moy <http://matthieu-moy.fr>`__ and
+`Michael Haggerty <https://github.com/mhagger>`__.
Please note that although a copy of git-multimail is distributed in
the "contrib" section of the main Git project, development takes place
@@ -33,6 +32,29 @@ mailing list`_.
Please CC emails regarding git-multimail to the maintainers so that we
don't overlook them.
+Help needed: testers/maintainer for specific environments/OS
+------------------------------------------------------------
+
+The current maintainer uses and tests git-multimail on Linux with the
+Generic environment. More testers, or better contributors are needed
+to test git-multimail on other real-life setups:
+
+* Mac OS X, Windows: git-multimail is currently not supported on these
+ platforms. But since we have no external dependencies and try to
+ write code as portable as possible, it is possible that
+ git-multimail already runs there and if not, it is likely that it
+ could be ported easily.
+
+ Patches to improve support for Windows and OS X are welcome.
+ Ideally, there would be a sub-maintainer for each OS who would test
+ at least once before each release (around twice a year).
+
+* Gerrit, Stash, Gitolite environments: although the testsuite
+ contains tests for these environments, a tester/maintainer for each
+ environment would be welcome to test and report failure (or success)
+ on real-life environments periodically (here also, feedback before
+ each release would be highly appreciated).
+
.. _`git-multimail repository on GitHub`: https://github.com/git-multimail/git-multimail
.. _`Git mailing list`: git@vger.kernel.org
diff --git a/contrib/hooks/multimail/README.Git b/contrib/hooks/multimail/README.Git
index 161b0230a0..044444245d 100644
--- a/contrib/hooks/multimail/README.Git
+++ b/contrib/hooks/multimail/README.Git
@@ -6,10 +6,10 @@ website:
https://github.com/git-multimail/git-multimail
The version in this directory was obtained from the upstream project
-on August 17 2016 and consists of the "git-multimail" subdirectory from
+on January 07 2019 and consists of the "git-multimail" subdirectory from
revision
- 07b1cb6bfd7be156c62e1afa17cae13b850a869f refs/tags/1.4.0
+ 04e80e6c40be465cc62b6c246f0fcb8fd2cfd454 refs/tags/1.5.0
Please see the README file in this directory for information about how
to report bugs or contribute to git-multimail.
diff --git a/contrib/hooks/multimail/README b/contrib/hooks/multimail/README.rst
similarity index 95%
rename from contrib/hooks/multimail/README
rename to contrib/hooks/multimail/README.rst
index 5105373aea..7c0fc4a6ef 100644
--- a/contrib/hooks/multimail/README
+++ b/contrib/hooks/multimail/README.rst
@@ -1,4 +1,4 @@
-git-multimail version 1.4.0
+git-multimail version 1.5.0
===========================
.. image:: https://travis-ci.org/git-multimail/git-multimail.svg?branch=master
@@ -20,8 +20,8 @@ GPLv2 (see the COPYING file for details).
Please note: although, as a convenience, git-multimail may be
distributed along with the main Git project, development of
-git-multimail takes place in its own, separate project. See section
-"Getting involved" below for more information.
+git-multimail takes place in its own, separate project. Please, read
+`<CONTRIBUTING.rst>`__ for more information.
By default, for each push received by the repository, git-multimail:
@@ -89,6 +89,10 @@ Requirements
the multimailhook.mailer configuration variable below for how to
configure git-multimail to send emails via an SMTP server.
+* git-multimail is currently tested only on Linux. It may or may not
+ work on other platforms such as Windows and Mac OS. See
+ `<CONTRIBUTING.rst>`__ to improve the situation.
+
Invocation
----------
@@ -369,7 +373,7 @@ multimailhook.mailer
unset, then the value of multimailhook.from is used.
multimailhook.smtpServerTimeout
- Timeout in seconds.
+ Timeout in seconds. Default is 10.
multimailhook.smtpEncryption
Set the security type. Allowed values: ``none``, ``ssl``, ``tls`` (starttls).
@@ -419,8 +423,20 @@ multimailhook.from, multimailhook.fromCommit, multimailhook.fromRefchange
If config values are unset, the value of the From: header is
determined as follows:
- 1. (gitolite environment only) Parse gitolite.conf, looking for a
- block of comments that looks like this::
+ 1. (gitolite environment only)
+ 1.a) If ``multimailhook.MailaddressMap`` is set, and is a path
+ to an existing file (if relative, it is considered relative to
+ the place where ``gitolite.conf`` is located), then this file
+ should contain lines like::
+
+ username Firstname Lastname <email@example.com>
+
+ git-multimail will then look for a line where ``$GL_USER``
+ matches the ``username`` part, and use the rest of the line for
+ the ``From:`` header.
+
+ 1.b) Parse gitolite.conf, looking for a block of comments that
+ looks like this::
# BEGIN USER EMAILS
# username Firstname Lastname <email@example.com>
@@ -436,6 +452,11 @@ multimailhook.from, multimailhook.fromCommit, multimailhook.fromRefchange
3. Use the value of multimailhook.envelopeSender.
+multimailhook.MailaddressMap
+ (gitolite environment only)
+ File to look for a ``From:`` address based on the user doing the
+ push. Defaults to unset. See ``multimailhook.from`` for details.
+
multimailhook.administrator
The name and/or email address of the administrator of the Git
repository; used in FOOTER_TEMPLATE. Default is
@@ -484,6 +505,11 @@ multimailhook.maxCommitEmails
mailbombing, for example on an initial push. To disable commit
emails limit, set this option to 0. The default is 500.
+multimailhook.excludeMergeRevisions
+ When sending out revision emails, do not consider merge commits (the
+ functional equivalent of `rev-list --no-merges`).
+ The default is `false` (send merge commit emails).
+
multimailhook.emailStrictUTF8
If this boolean option is set to `true`, then the main part of the
email body is forced to be valid UTF-8. Any characters that are
diff --git a/contrib/hooks/multimail/doc/gitolite.rst b/contrib/hooks/multimail/doc/gitolite.rst
index 00aedd9c57..5054833105 100644
--- a/contrib/hooks/multimail/doc/gitolite.rst
+++ b/contrib/hooks/multimail/doc/gitolite.rst
@@ -46,6 +46,15 @@ and add::
config multimailhook.mailingList = # Where emails should be sent
config multimailhook.from = # From address to use
+Note that by default, gitolite forbids ``<`` and ``>`` in variable
+values (for security/paranoia reasons, see
+`compensating for UNSAFE_PATT
+<http://gitolite.com/gitolite/git-config/index.html#compensating-for-unsafe95patt>`__
+in gitolite's documentation for explanations and a way to disable
+this). As a consequence, you will not be able to use ``First Last
+<First.Last@example.com>`` as recipient email, but specifying
+``First.Last@example.com`` alone works.
+
Obviously, you can customize all parameters on a per-repository basis by
adding these ``config multimailhook.*`` lines in the section
corresponding to a repository or set of repositories.
diff --git a/contrib/hooks/multimail/git_multimail.py b/contrib/hooks/multimail/git_multimail.py
index 73fdda6b14..8823399e75 100755
--- a/contrib/hooks/multimail/git_multimail.py
+++ b/contrib/hooks/multimail/git_multimail.py
@@ -1,6 +1,6 @@
#! /usr/bin/env python
-__version__ = '1.4.0'
+__version__ = '1.5.0'
# Copyright (c) 2015-2016 Matthieu Moy and others
# Copyright (c) 2012-2014 Michael Haggerty and others
@@ -64,7 +64,9 @@
# Python < 2.6 do not have ssl, but that's OK if we don't use it.
pass
import time
-import cgi
+
+import uuid
+import base64
PYTHON3 = sys.version_info >= (3, 0)
@@ -73,7 +75,7 @@ def all(iterable):
for element in iterable:
if not element:
return False
- return True
+ return True
def is_ascii(s):
@@ -108,6 +110,12 @@ def read_line(f):
return out.decode(sys.getdefaultencoding())
except UnicodeEncodeError:
return out.decode(ENCODING)
+
+ import html
+
+ def html_escape(s):
+ return html.escape(s)
+
else:
def is_string(s):
try:
@@ -130,6 +138,10 @@ def read_line(f):
def next(it):
return it.next()
+ import cgi
+
+ def html_escape(s):
+ return cgi.escape(s, True)
try:
from email.charset import Charset
@@ -190,6 +202,7 @@ def next(it):
Message-ID: %(msgid)s
From: %(fromaddr)s
Reply-To: %(reply_to)s
+Thread-Index: %(thread_index)s
X-Git-Host: %(fqdn)s
X-Git-Repo: %(repo_shortname)s
X-Git-Refname: %(refname)s
@@ -322,6 +335,7 @@ def next(it):
Reply-To: %(reply_to)s
In-Reply-To: %(reply_to_msgid)s
References: %(reply_to_msgid)s
+Thread-Index: %(thread_index)s
X-Git-Host: %(fqdn)s
X-Git-Repo: %(repo_shortname)s
X-Git-Refname: %(refname)s
@@ -763,6 +777,9 @@ def get_summary(self):
def __eq__(self, other):
return isinstance(other, GitObject) and self.sha1 == other.sha1
+ def __ne__(self, other):
+ return not self == other
+
def __hash__(self):
return hash(self.sha1)
@@ -852,7 +869,7 @@ def expand_lines(self, template, html_escape_val=False, **extra_values):
if html_escape_val:
for k in values:
if is_string(values[k]):
- values[k] = cgi.escape(values[k], True)
+ values[k] = html_escape(values[k])
for line in template.splitlines(True):
yield line % values
@@ -909,7 +926,7 @@ def generate_email_intro(self, html_escape_val=False):
raise NotImplementedError()
- def generate_email_body(self):
+ def generate_email_body(self, push):
"""Generate the main part of the email body, a line at a time.
The text in the body might be truncated after a specified
@@ -936,7 +953,7 @@ def _wrap_for_html(self, lines):
yield "<pre style='margin:0'>\n"
for line in lines:
- yield cgi.escape(line)
+ yield html_escape(line)
yield '</pre>\n'
else:
@@ -1011,7 +1028,7 @@ def generate_email(self, push, body_filter=None, extra_header_values={}):
fgcolor = '404040'
# Chop the trailing LF, we don't want it inside <pre>.
- line = cgi.escape(line[:-1])
+ line = html_escape(line[:-1])
if bgcolor or fgcolor:
style = 'display:block; white-space:pre;'
@@ -1060,6 +1077,10 @@ def __init__(self, reference_change, rev, num, tot):
self.author = read_git_output(['log', '--no-walk', '--format=%aN <%aE>', self.rev.sha1])
self.recipients = self.environment.get_revision_recipients(self)
+ # -s is short for --no-patch, but -s works on older git's (e.g. 1.7)
+ self.parents = read_git_lines(['show', '-s', '--format=%P',
+ self.rev.sha1])[0].split()
+
self.cc_recipients = ''
if self.environment.get_scancommitforcc():
self.cc_recipients = ', '.join(to.strip() for to in self._cc_recipients())
@@ -1090,6 +1111,7 @@ def _compute_values(self):
oneline = oneline[:max_subject_length - 6] + ' [...]'
values['rev'] = self.rev.sha1
+ values['parents'] = ' '.join(self.parents)
values['rev_short'] = self.rev.short
values['change_type'] = self.change_type
values['refname'] = self.refname
@@ -1097,6 +1119,7 @@ def _compute_values(self):
values['short_refname'] = self.reference_change.short_refname
values['refname_type'] = self.reference_change.refname_type
values['reply_to_msgid'] = self.reference_change.msgid
+ values['thread_index'] = self.reference_change.thread_index
values['num'] = self.num
values['tot'] = self.tot
values['recipients'] = self.recipients
@@ -1244,6 +1267,23 @@ def create(environment, oldrev, newrev, refname):
old=old, new=new, rev=rev,
)
+ @staticmethod
+ def make_thread_index():
+ """Return a string appropriate for the Thread-Index header,
+ needed by MS Outlook to get threading right.
+
+ The format is (base64-encoded):
+ - 1 byte must be 1
+ - 5 bytes encode a date (hardcoded here)
+ - 16 bytes for a globally unique identifier
+
+ FIXME: Unfortunately, even with the Thread-Index field, MS
+ Outlook doesn't seem to do the threading reliably (see
+ https://github.com/git-multimail/git-multimail/pull/194).
+ """
+ thread_index = b'\x01\x00\x00\x12\x34\x56' + uuid.uuid4().bytes
+ return base64.standard_b64encode(thread_index).decode('ascii')
+
def __init__(self, environment, refname, short_refname, old, new, rev):
Change.__init__(self, environment)
self.change_type = {
@@ -1257,6 +1297,7 @@ def __init__(self, environment, refname, short_refname, old, new, rev):
self.new = new
self.rev = rev
self.msgid = make_msgid()
+ self.thread_index = self.make_thread_index()
self.diffopts = environment.diffopts
self.graphopts = environment.graphopts
self.logopts = environment.logopts
@@ -1276,6 +1317,7 @@ def _compute_values(self):
values['refname'] = self.refname
values['short_refname'] = self.short_refname
values['msgid'] = self.msgid
+ values['thread_index'] = self.thread_index
values['recipients'] = self.recipients
values['oldrev'] = str(self.old)
values['oldrev_short'] = self.old.short
@@ -1941,6 +1983,9 @@ class Mailer(object):
def __init__(self, environment):
self.environment = environment
+ def close(self):
+ pass
+
def send(self, lines, to_addrs):
"""Send an email consisting of lines.
@@ -2054,6 +2099,7 @@ def __init__(self, environment,
self.username = smtpuser
self.password = smtppass
self.smtpcacerts = smtpcacerts
+ self.loggedin = False
try:
def call(klass, server, timeout):
try:
@@ -2130,20 +2176,30 @@ def call(klass, server, timeout):
% (self.smtpserver, sys.exc_info()[1]))
sys.exit(1)
- def __del__(self):
+ def close(self):
if hasattr(self, 'smtp'):
self.smtp.quit()
del self.smtp
+ def __del__(self):
+ self.close()
+
def send(self, lines, to_addrs):
try:
if self.username or self.password:
- self.smtp.login(self.username, self.password)
+ if not self.loggedin:
+ self.smtp.login(self.username, self.password)
+ self.loggedin = True
msg = ''.join(lines)
# turn comma-separated list into Python list if needed.
if is_string(to_addrs):
to_addrs = [email for (name, email) in getaddresses([to_addrs])]
self.smtp.sendmail(self.envelopesender, to_addrs, msg)
+ except socket.timeout:
+ self.environment.get_logger().error(
+ '*** Error sending email ***\n'
+ '*** SMTP server timed out (timeout is %s)\n'
+ % self.smtpservertimeout)
except smtplib.SMTPResponseException:
err = sys.exc_info()[1]
self.environment.get_logger().error(
@@ -2171,7 +2227,8 @@ class OutputMailer(Mailer):
SEPARATOR = '=' * 75 + '\n'
- def __init__(self, f):
+ def __init__(self, f, environment=None):
+ super(OutputMailer, self).__init__(environment=environment)
self.f = f
def send(self, lines, to_addrs):
@@ -2382,6 +2439,7 @@ def __init__(self, osenv=None):
self.html_in_footer = False
self.commitBrowseURL = None
self.maxcommitemails = 500
+ self.excludemergerevisions = False
self.diffopts = ['--stat', '--summary', '--find-copies-harder']
self.graphopts = ['--oneline', '--decorate']
self.logopts = []
@@ -2621,6 +2679,8 @@ def __init__(self, config, **kw):
self.commitBrowseURL = config.get('commitBrowseURL')
+ self.excludemergerevisions = config.get('excludeMergeRevisions')
+
maxcommitemails = config.get('maxcommitemails')
if maxcommitemails is not None:
try:
@@ -3152,7 +3212,10 @@ def get_pusher(self):
return self.osenv.get('GL_USER', 'unknown user')
-class GitoliteEnvironmentLowPrecMixin(Environment):
+class GitoliteEnvironmentLowPrecMixin(
+ ConfigEnvironmentMixin,
+ Environment):
+
def get_repo_shortname(self):
# The gitolite environment variable $GL_REPO is a pretty good
# repo_shortname (though it's probably not as good as a value
@@ -3162,6 +3225,16 @@ def get_repo_shortname(self):
super(GitoliteEnvironmentLowPrecMixin, self).get_repo_shortname()
)
+ @staticmethod
+ def _compile_regex(re_template):
+ return (
+ re.compile(re_template % x)
+ for x in (
+ r'BEGIN\s+USER\s+EMAILS',
+ r'([^\s]+)\s+(.*)',
+ r'END\s+USER\s+EMAILS',
+ ))
+
def get_fromaddr(self, change=None):
GL_USER = self.osenv.get('GL_USER')
if GL_USER is not None:
@@ -3174,18 +3247,42 @@ def get_fromaddr(self, change=None):
GL_CONF = self.osenv.get(
'GL_CONF',
os.path.join(GL_ADMINDIR, 'conf', 'gitolite.conf'))
+
+ mailaddress_map = self.config.get('MailaddressMap')
+ # If relative, consider relative to GL_CONF:
+ if mailaddress_map:
+ mailaddress_map = os.path.join(os.path.dirname(GL_CONF),
+ mailaddress_map)
+ if os.path.isfile(mailaddress_map):
+ f = open(mailaddress_map, 'rU')
+ try:
+ # Leading '#' is optional
+ re_begin, re_user, re_end = self._compile_regex(
+ r'^(?:\s*#)?\s*%s\s*$')
+ for l in f:
+ l = l.rstrip('\n')
+ if re_begin.match(l) or re_end.match(l):
+ continue # Ignore these lines
+ m = re_user.match(l)
+ if m:
+ if m.group(1) == GL_USER:
+ return m.group(2)
+ else:
+ continue # Not this user, but not an error
+ raise ConfigurationException(
+ "Syntax error in mail address map.\n"
+ "Check file {}.\n"
+ "Line: {}".format(mailaddress_map, l))
+
+ finally:
+ f.close()
+
if os.path.isfile(GL_CONF):
f = open(GL_CONF, 'rU')
try:
in_user_emails_section = False
- re_template = r'^\s*#\s*%s\s*$'
- re_begin, re_user, re_end = (
- re.compile(re_template % x)
- for x in (
- r'BEGIN\s+USER\s+EMAILS',
- re.escape(GL_USER) + r'\s+(.*)',
- r'END\s+USER\s+EMAILS',
- ))
+ re_begin, re_user, re_end = self._compile_regex(
+ r'^\s*#\s*%s\s*$')
for l in f:
l = l.rstrip('\n')
if not in_user_emails_section:
@@ -3195,8 +3292,8 @@ def get_fromaddr(self, change=None):
if re_end.match(l):
break
m = re_user.match(l)
- if m:
- return m.group(1)
+ if m and m.group(1) == GL_USER:
+ return m.group(2)
finally:
f.close()
return super(GitoliteEnvironmentLowPrecMixin, self).get_fromaddr(change)
@@ -3228,7 +3325,7 @@ def __init__(self, user=None, repo=None, **kw):
self.__repo = repo
def get_pusher(self):
- return re.match('(.*?)\s*<', self.__user).group(1)
+ return re.match(r'(.*?)\s*<', self.__user).group(1)
def get_pusher_email(self):
return self.__user
@@ -3262,7 +3359,7 @@ def get_pusher(self):
if self.__submitter.find('<') != -1:
# Submitter has a configured email, we transformed
# __submitter into an RFC 2822 string already.
- return re.match('(.*?)\s*<', self.__submitter).group(1)
+ return re.match(r'(.*?)\s*<', self.__submitter).group(1)
else:
# Submitter has no configured email, it's just his name.
return self.__submitter
@@ -3615,6 +3712,9 @@ def send_emails(self, mailer, body_filter=None):
for (num, sha1) in enumerate(sha1s):
rev = Revision(change, GitObject(sha1), num=num + 1, tot=len(sha1s))
+ if len(rev.parents) > 1 and change.environment.excludemergerevisions:
+ # skipping a merge commit
+ continue
if not rev.recipients and rev.cc_recipients:
change.environment.log_msg('*** Replacing Cc: with To:')
rev.recipients = rev.cc_recipients
@@ -3664,11 +3764,14 @@ def run_as_post_receive_hook(environment, mailer):
changes.append(
ReferenceChange.create(environment, oldrev, newrev, refname)
)
- if changes:
- push = Push(environment, changes)
+ if not changes:
+ mailer.close()
+ return
+ push = Push(environment, changes)
+ try:
push.send_emails(mailer, body_filter=environment.filter_body)
- if hasattr(mailer, '__del__'):
- mailer.__del__()
+ finally:
+ mailer.close()
def run_as_update_hook(environment, mailer, refname, oldrev, newrev, force_send=False):
@@ -3687,10 +3790,14 @@ def run_as_update_hook(environment, mailer, refname, oldrev, newrev, force_send=
refname,
),
]
+ if not changes:
+ mailer.close()
+ return
push = Push(environment, changes, force_send)
- push.send_emails(mailer, body_filter=environment.filter_body)
- if hasattr(mailer, '__del__'):
- mailer.__del__()
+ try:
+ push.send_emails(mailer, body_filter=environment.filter_body)
+ finally:
+ mailer.close()
def check_ref_filter(environment):
@@ -3860,7 +3967,7 @@ def build_environment_klass(env_name):
low_prec_mixin = known_env['lowprec']
environment_mixins.append(low_prec_mixin)
environment_mixins.append(Environment)
- klass_name = env_name.capitalize() + 'Environement'
+ klass_name = env_name.capitalize() + 'Environment'
environment_klass = type(
klass_name,
tuple(environment_mixins),
@@ -4057,21 +4164,21 @@ def flush(self):
environment, 'git_multimail.error', environment.error_log_file, logging.ERROR)
self.loggers.append(error_log_file)
- def info(self, msg):
+ def info(self, msg, *args, **kwargs):
for l in self.loggers:
- l.info(msg)
+ l.info(msg, *args, **kwargs)
- def debug(self, msg):
+ def debug(self, msg, *args, **kwargs):
for l in self.loggers:
- l.debug(msg)
+ l.debug(msg, *args, **kwargs)
- def warning(self, msg):
+ def warning(self, msg, *args, **kwargs):
for l in self.loggers:
- l.warning(msg)
+ l.warning(msg, *args, **kwargs)
- def error(self, msg):
+ def error(self, msg, *args, **kwargs):
for l in self.loggers:
- l.error(msg)
+ l.error(msg, *args, **kwargs)
def main(args):
@@ -4189,7 +4296,7 @@ def main(args):
show_env(environment, sys.stderr)
if options.stdout or environment.stdout:
- mailer = OutputMailer(sys.stdout)
+ mailer = OutputMailer(sys.stdout, environment)
else:
mailer = choose_mailer(config, environment)
@@ -4234,5 +4341,6 @@ def main(args):
sys.stderr.write(msg)
sys.exit(1)
+
if __name__ == '__main__':
main(sys.argv[1:])
diff --git a/contrib/hooks/multimail/migrate-mailhook-config b/contrib/hooks/multimail/migrate-mailhook-config
index 992657bbdc..241ba22fa3 100755
--- a/contrib/hooks/multimail/migrate-mailhook-config
+++ b/contrib/hooks/multimail/migrate-mailhook-config
@@ -110,11 +110,12 @@ def is_section_empty(section, local):
try:
read_output(
- ['git', 'config']
- + local_option
- + ['--get-regexp', '^%s\.' % (section,)]
+ ['git', 'config'] +
+ local_option +
+ ['--get-regexp', '^%s\.' % (section,)]
)
- except CommandError, e:
+ except CommandError:
+ t, e, traceback = sys.exc_info()
if e.retcode == 1:
# This means that no settings were found.
return True
@@ -188,7 +189,9 @@ def migrate_config(strict=False, retain=False, overwrite=False):
sys.stderr.write(
'...copying "%s.%s" to "%s.%s"\n' % (old.section, name, new.section, name)
)
- new.set_recipients(name, old.get_recipients(name))
+ old_recipients = old.get_all(name, default=None)
+ old_recipients = ', '.join(o.strip() for o in old_recipients)
+ new.set_recipients(name, old_recipients)
if strict:
sys.stderr.write(
diff --git a/contrib/hooks/multimail/post-receive.example b/contrib/hooks/multimail/post-receive.example
index 1ea113d274..b9bb11834e 100755
--- a/contrib/hooks/multimail/post-receive.example
+++ b/contrib/hooks/multimail/post-receive.example
@@ -30,7 +30,6 @@ script's behavior could be changed or customized.
"""
import sys
-import os
# If necessary, add the path to the directory containing
# git_multimail.py to the Python path as follows. (This is not
@@ -86,6 +85,7 @@ mailer = git_multimail.choose_mailer(config, environment)
# Use Python's smtplib to send emails. Both arguments are required.
#mailer = git_multimail.SMTPMailer(
+# environment=environment,
# envelopesender='git-repo@example.com',
# # The smtpserver argument can also include a port number; e.g.,
# # smtpserver='mail.example.com:25'
--
2.20.1.98.gecbdaf0
^ permalink raw reply related
* [PATCH v3] Add optional targets for documentation l10n
From: Jean-Noël Avila @ 2019-01-07 19:21 UTC (permalink / raw)
To: git; +Cc: Jean-Noel Avila
In-Reply-To: <20190104165406.22358-1-jn.avila@free.fr>
From: Jean-Noel Avila <jn.avila@free.fr>
The standard doc lists can be filtered to allow using the compilation
rules with translated manpages where all the pages of the original
version may not be present.
The install variable are reused in the secondary repo so that the
configured paths can be used for translated manpages too.
Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
---
Documentation/Makefile | 22 +++++++++++++++++-----
1 file changed, 17 insertions(+), 5 deletions(-)
diff --git a/Documentation/Makefile b/Documentation/Makefile
index b5be2e2d3f..2bd2fb11f4 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -35,13 +35,18 @@ MAN7_TXT += gittutorial-2.txt
MAN7_TXT += gittutorial.txt
MAN7_TXT += gitworkflows.txt
+ifdef MAN_FILTER
+MAN_TXT = $(filter $(MAN_FILTER),$(MAN1_TXT) $(MAN5_TXT) $(MAN7_TXT))
+else
MAN_TXT = $(MAN1_TXT) $(MAN5_TXT) $(MAN7_TXT)
+MAN_FILTER = $(MAN_TXT)
+endif
+
MAN_XML = $(patsubst %.txt,%.xml,$(MAN_TXT))
MAN_HTML = $(patsubst %.txt,%.html,$(MAN_TXT))
OBSOLETE_HTML += everyday.html
OBSOLETE_HTML += git-remote-helpers.html
-DOC_HTML = $(MAN_HTML) $(OBSOLETE_HTML)
ARTICLES += howto-index
ARTICLES += git-tools
@@ -81,11 +86,13 @@ TECH_DOCS += technical/trivial-merge
SP_ARTICLES += $(TECH_DOCS)
SP_ARTICLES += technical/api-index
-DOC_HTML += $(patsubst %,%.html,$(ARTICLES) $(SP_ARTICLES))
+ARTICLES_HTML += $(patsubst %,%.html,$(ARTICLES) $(SP_ARTICLES))
+HTML_FILTER ?= $(ARTICLES_HTML) $(OBSOLETE_HTML)
+DOC_HTML = $(MAN_HTML) $(filter $(HTML_FILTER),$(ARTICLES_HTML) $(OBSOLETE_HTML))
-DOC_MAN1 = $(patsubst %.txt,%.1,$(MAN1_TXT))
-DOC_MAN5 = $(patsubst %.txt,%.5,$(MAN5_TXT))
-DOC_MAN7 = $(patsubst %.txt,%.7,$(MAN7_TXT))
+DOC_MAN1 = $(patsubst %.txt,%.1,$(filter $(MAN_FILTER),$(MAN1_TXT)))
+DOC_MAN5 = $(patsubst %.txt,%.5,$(filter $(MAN_FILTER),$(MAN5_TXT)))
+DOC_MAN7 = $(patsubst %.txt,%.7,$(filter $(MAN_FILTER),$(MAN7_TXT)))
prefix ?= $(HOME)
bindir ?= $(prefix)/bin
@@ -444,4 +451,9 @@ print-man1:
lint-docs::
$(QUIET_LINT)$(PERL_PATH) lint-gitlink.perl
+ifeq ($(wildcard po/Makefile),po/Makefile)
+doc-l10n install-l10n::
+ $(MAKE) -C po $@
+endif
+
.PHONY: FORCE
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v5 3/3] branch: Add an extra verbose output displaying worktree path for refs checked out in a linked worktree
From: Junio C Hamano @ 2019-01-07 19:09 UTC (permalink / raw)
To: nbelakovski; +Cc: git, peff, rafa.almas, avarab
In-Reply-To: <20190106002619.54741-4-nbelakovski@gmail.com>
nbelakovski@gmail.com writes:
> From: Nickolai Belakovski <nbelakovski@gmail.com>
>
> ---
All three patches lack sign off.
I am fairly negative on 2/3, but I think this one makes sense
without introducing a new verbosity level. We do not promise
stability of Porcelain command output and update the UI if we
have useful information to give. Just making
git branch --list -v -v
show additional information should be sufficient.
> diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
> index b3eca6ffdc..6d1fc59e32 100644
> --- a/Documentation/git-branch.txt
> +++ b/Documentation/git-branch.txt
> @@ -163,12 +163,15 @@ This option is only applicable in non-verbose mode.
>
> -v::
> -vv::
> +-vvv::
> --verbose::
> When in list mode,
> show sha1 and commit subject line for each head, along with
> relationship to upstream branch (if any). If given twice, print
> the name of the upstream branch, as well (see also `git remote
> - show <remote>`).
> + show <remote>`). If given 3 times, print the path of the linked
> + worktree, if applicable (not applicable for main worktree since
> + its path will be a subset of $PWD)
>
> -q::
> --quiet::
> diff --git a/builtin/branch.c b/builtin/branch.c
> index 2a24153b78..56589a3684 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -366,6 +366,10 @@ static char *build_format(struct ref_filter *filter, int maxwidth, const char *r
> strbuf_addstr(&local, branch_get_color(BRANCH_COLOR_RESET));
> strbuf_addf(&local, " %s ", obname.buf);
>
> + if (filter->verbose > 2)
> + strbuf_addf(&local, "%s%%(if:notequals=*)%%(HEAD)%%(then)%%(if)%%(worktreepath)%%(then)%%(worktreepath) %%(end)%%(end)%s",
> + branch_get_color(BRANCH_COLOR_WORKTREE), branch_get_color(BRANCH_COLOR_RESET));
> +
> if (filter->verbose > 1)
> strbuf_addf(&local, "%%(if)%%(upstream)%%(then)[%s%%(upstream:short)%s%%(if)%%(upstream:track)"
> "%%(then): %%(upstream:track,nobracket)%%(end)] %%(end)%%(contents:subject)",
^ permalink raw reply
* Re: [PATCH v5 2/3] branch: Mark and color a branch differently if it is checked out in a linked worktree
From: Junio C Hamano @ 2019-01-07 19:04 UTC (permalink / raw)
To: nbelakovski; +Cc: git, peff, rafa.almas, avarab
In-Reply-To: <20190106002619.54741-3-nbelakovski@gmail.com>
nbelakovski@gmail.com writes:
> From: Nickolai Belakovski <nbelakovski@gmail.com>
>
> In order to more clearly display which branches are active, the output
> of git branch is modified to mark branches checkout out in a linked
> worktree with a "+" and color them in cyan (in contrast to the current
> branch, which will still be denoted with a "*" and colored in green)
>
> This is meant to simplify workflows related to worktree, particularly
> due to the limitations of not being able to check out the same branch in
> two worktrees and the inability to delete a branch checked out in a
> worktree. When performing branch operations like checkout and delete, it
> would be useful to know more readily if the branches in which the user
> is interested are already checked out in a worktree.
I do not think it is warranted to paint the safety features as
"limitations".
A branch that is checked out in another worktree cannot be checked
out to be worked on, as that will make the checkout of the other
worktree out of sync. If you want to work on that branch, you can
either (1) go to that worktree that has a checkout of that branch
and work there, or (2) go to that worktree that has a checkout of
that branch, check out a different branch there, come back to the
worktree you want to work in and check out that branch. Knowing
where that other worktree is is the first step in either case.
And a branch that is checked out in a worktree cannot be removed, as
it is a sign that it is still being worked on for a branch to have
been checked out somewhere. If you do want to remove that branch,
you need to go to that worktree that has a checkout of that branch,
check out a different branch there, and then remove it. Again,
knowing where that other worktree is is the fist thing you need to
know.
But then I am not sure if the feature being added by these patches
is a good match for that justification.
For one thing, it would be more direct and helpful way for
git checkout one-branch
git branch -d one-branch
to say "The branch `one-branch` is checked out in a worktree at
$DIRECTORY" when they refused to go ahead. And that would eliminate
the need for this new feature to help these two use cases.
In fact, these two command already behave that way, so the paragraph
I just commented on is not a good justification for this new feature
at all.
Besides, showing "That branch is checked out somewhere" would not
help user to decide "ah, if I want to work on that branch, I need to
chdir to that directory" with the patch in question, as it only
shows "It is checked out _somewhere_" without saying where.
> The git worktree list command contains the relevant information, however
> this is a much less frquently used command than git branch.
It is not a good justification. If the "relevant information" given
by the command is necessary one, the user can run that command. If
the situation where that "relevant information" becomes necessary is
rare, the command is run much less frequently is not a problem---it
is expected. And overloading a more frequently used command with
information that is less frequently wanted is actually not a great
design.
A more relevant justification may be that even though the
information can already be found in "worktree list" output, it would
give us flexibility in presentation to allow the custom format in
for-each-ref to show it.
So, I am between moderately Meh to fairly negative on this step; Meh
in the sense that "thanks to the previous step, we _could_ do this,
it does not give incorrect information, and it makes the output more
cheerful, but it does not add that much useful and actionable piece
of information".
^ permalink raw reply
* Re: [PATCH v2 1/2] Use packet_reader instead of packet_read_line
From: Jonathan Tan @ 2019-01-07 19:01 UTC (permalink / raw)
To: masayasuzuki; +Cc: git, peff, steadmon, Jonathan Tan
In-Reply-To: <20181229211915.161686-2-masayasuzuki@google.com>
> By using and sharing a packet_reader while handling a Git pack protocol
> request, the same reader option is used throughout the code. This makes
> it easy to set a reader option to the request parsing code.
I see that packet_read() and packet_read_line_buf() invocations are also
removed, so it might be better to use "Use packet_reader instead of
packet_read.*" as the commit title.
The code looks correct to me - most of the changes are removals of
packet_read_line(), replaced with a packet_reader that has
PACKET_READ_CHOMP_NEWLINE. One instance is packet_read(), for which the
corresponding reader does not have PACKET_READ_CHOMP_NEWLINE (noted
below); and another instance is packet_read_line_buf(), for which the
corresponding reader is instantiated accordingly with the buffer (also
noted below).
> - if (!strcmp(line, "push-cert")) {
> + if (!strcmp(reader->line, "push-cert")) {
> int true_flush = 0;
> - char certbuf[1024];
> + int saved_options = reader->options;
> + reader->options &= ~PACKET_READ_CHOMP_NEWLINE;
>
> for (;;) {
> - len = packet_read(0, NULL, NULL,
> - certbuf, sizeof(certbuf), 0);
> - if (!len) {
> + packet_reader_read(reader);
> + if (reader->status == PACKET_READ_FLUSH) {
> true_flush = 1;
> break;
> }
> - if (!strcmp(certbuf, "push-cert-end\n"))
> + if (reader->status != PACKET_READ_NORMAL) {
> + die("protocol error: got an unexpected packet");
> + }
> + if (!strcmp(reader->line, "push-cert-end\n"))
> break; /* end of cert */
> - strbuf_addstr(&push_cert, certbuf);
> + strbuf_addstr(&push_cert, reader->line);
> }
> + reader->options = saved_options;
Here, packet_read() is used, so we shouldn't chomp the newline, so this
is correct.
> - char *line;
> + struct packet_reader reader;
> + packet_reader_init(&reader, -1, last->buf, last->len,
> + PACKET_READ_CHOMP_NEWLINE);
>
> /*
> * smart HTTP response; validate that the service
> * pkt-line matches our request.
> */
> - line = packet_read_line_buf(&last->buf, &last->len, NULL);
> - if (!line)
> + if (packet_reader_read(&reader) != PACKET_READ_NORMAL)
> die("invalid server response; expected service, got flush packet");
>
> strbuf_reset(&exp);
> strbuf_addf(&exp, "# service=%s", service);
> - if (strcmp(line, exp.buf))
> - die("invalid server response; got '%s'", line);
> + if (strcmp(reader.line, exp.buf))
> + die("invalid server response; got '%s'", reader.line);
> strbuf_release(&exp);
>
> /* The header can include additional metadata lines, up
> * until a packet flush marker. Ignore these now, but
> * in the future we might start to scan them.
> */
> - while (packet_read_line_buf(&last->buf, &last->len, NULL))
> - ;
> + for (;;) {
> + packet_reader_read(&reader);
> + if (reader.pktlen <= 0) {
> + break;
> + }
> + }
> +
> + last->buf = reader.src_buffer;
> + last->len = reader.src_len;
And here, packet_reader_init() correctly initializes the packet_reader
with the buffer, and we need to know where in the buffer to continue
after parsing the additional metadata lines and the packet flush, so we
assign the state of the reader to last->buf and last->len.
(Incidentally, with this change, there is no longer any usage of
packet_read_line_buf(), but we can remove that in a subsequent patch.)
In summary, this looks like a good change. Configuration of reader
metadata (file descriptors, input buffers, and flags) is now more
centralized, and there are fewer file descriptor constants and variables
(which all look like ints) strewn around.
^ permalink raw reply
* Re: [PATCH v5 1/3] ref-filter: add worktreepath atom
From: Junio C Hamano @ 2019-01-07 18:20 UTC (permalink / raw)
To: nbelakovski; +Cc: git, peff, rafa.almas, avarab
In-Reply-To: <20190106002619.54741-2-nbelakovski@gmail.com>
nbelakovski@gmail.com writes:
> +static struct hashmap ref_to_worktree_map;
> +static struct worktree **worktrees = NULL;
> +
> /*
> * An atom is a valid field atom listed below, possibly prefixed with
> * a "*" to denote deref_tag().
> @@ -420,6 +438,34 @@ static int head_atom_parser(const struct ref_format *format, struct used_atom *a
> return 0;
> }
>
> +static int worktree_atom_parser(const struct ref_format *format,
> + struct used_atom *atom,
> + const char *arg,
> + struct strbuf *unused_err)
> +{
> + int i;
> +
> + if (worktrees)
> + return 0;
OK, so verify_ref_format() etc. will trigger this to be called via
valid_atom[].parser when "%(worktreepath)" is seen in the user
format, and then this grabs all the worktrees and record their paths
in the hashmap. This if() statement makes sure that it happens only
once.
> + worktrees = get_worktrees(0);
> +
> + hashmap_init(&ref_to_worktree_map, ref_to_worktree_map_cmpfnc, NULL, 0);
> +
> + for (i = 0; worktrees[i]; i++) {
> + if (worktrees[i]->head_ref) {
> + struct ref_to_worktree_entry *entry;
> + entry = xmalloc(sizeof(*entry));
> + entry->wt = worktrees[i];
> + hashmap_entry_init(entry, strhash(worktrees[i]->head_ref));
> +
> + hashmap_add(&ref_to_worktree_map, entry);
> + }
> + }
> +
> + return 0;
> +}
> +
> static struct {
> const char *name;
> info_source source;
> @@ -461,6 +507,7 @@ static struct {
> { "flag", SOURCE_NONE },
> { "HEAD", SOURCE_NONE, FIELD_STR, head_atom_parser },
> { "color", SOURCE_NONE, FIELD_STR, color_atom_parser },
> + { "worktreepath", SOURCE_NONE, FIELD_STR, worktree_atom_parser },
> { "align", SOURCE_NONE, FIELD_STR, align_atom_parser },
> { "end", SOURCE_NONE },
> { "if", SOURCE_NONE, FIELD_STR, if_atom_parser },
> @@ -1500,6 +1547,21 @@ static int get_object(struct ref_array_item *ref, int deref, struct object **obj
> return 0;
> }
>
> +static char *get_worktree_path(const struct used_atom *atom, const struct ref_array_item *ref)
> +{
> + struct strbuf val = STRBUF_INIT;
> + struct hashmap_entry entry;
> + struct ref_to_worktree_entry *lookup_result;
And then this will be called from populate_value() for each of the
ref. It looks up the worktree that has checked out this branch, if
any, from the hashmap, and yields the path to it.
When seeing a tag or note ref, by definition that's not something we
can have checked out in any worktree. I wonder if it is worth to
optimize further by omitting this lookup when ref is not a local
branch?
IOW, with a typical number of worktrees and refs, how costly would ...
> + hashmap_entry_init(&entry, strhash(ref->refname));
> + lookup_result = hashmap_get(&ref_to_worktree_map, &entry, ref->refname);
... this sequence of calls be.
> +
> + if (lookup_result)
> + strbuf_addstr(&val, lookup_result->wt->path);
> +
> + return strbuf_detach(&val, NULL);
> +}
> +
> /*
> * Parse the object referred by ref, and grab needed value.
> */
> @@ -1537,6 +1599,10 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)
>
> if (starts_with(name, "refname"))
> refname = get_refname(atom, ref);
> + else if (starts_with(name, "worktreepath")) {
> + v->s = get_worktree_path(atom, ref);
> + continue;
> + }
> else if (starts_with(name, "symref"))
> refname = get_symref(atom, ref);
> else if (starts_with(name, "upstream")) {
> @@ -2020,6 +2086,11 @@ void ref_array_clear(struct ref_array *array)
> free_array_item(array->items[i]);
> FREE_AND_NULL(array->items);
> array->nr = array->alloc = 0;
> + if (worktrees)
> + {
Have this opening brace at the end of the previous line, i.e.
if (worktrees) {
> + hashmap_free(&ref_to_worktree_map, 1);
> + free_worktrees(worktrees);
> + }
> }
What's the point of ref_array_clear()? What does the caller of this
function really want? Is it merely to release the resources
consumed? If so, then this is good enough, but then the existing
calls to FREE_AND_NULL() for releasing resources in the function is
overkill.
Or is it envisioned that we are preparing a clean slate so that
another call, possibly after the external environment changed, can
be made into this machinery (i.e. imagine we lift ref-filter.c code
and link it to a long running service process; after serving one
for-each-ref request, a new worktree or a new branch may get
created, and then we may get another for-each-ref request)? If that
is the case, then the added code breaks that hope, as it leaves a
dangling pointer in the worktrees variable.
> static void do_merge_filter(struct ref_filter_cbdata *ref_cbdata)
> diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh
> index fc067ed672..87e0222ea1 100755
> --- a/t/t6302-for-each-ref-filter.sh
> +++ b/t/t6302-for-each-ref-filter.sh
> @@ -441,4 +441,19 @@ test_expect_success '--merged is incompatible with --no-merged' '
> test_must_fail git for-each-ref --merged HEAD --no-merged HEAD
> '
>
> +test_expect_success '"add" a worktree' '
> + mkdir worktree_dir &&
> + git worktree add -b master_worktree worktree_dir master
> +'
> +
> +test_expect_success 'validate worktree atom' '
> + cat >expect <<-EOF &&
> + master: $(pwd)
> + master_worktree: $(pwd)/worktree_dir
> + side: not checked out
> + EOF
> + git for-each-ref --format="%(refname:short): %(if)%(worktreepath)%(then)%(worktreepath)%(else)not checked out%(end)" refs/heads/ >actual &&
> + test_cmp expect actual
> +'
> +
> test_done
^ permalink raw reply
* Re: gitk shows local uncommit changes after touch file + reload
From: Philip Oakley @ 2019-01-07 17:41 UTC (permalink / raw)
To: Jacob Kroon, git
In-Reply-To: <CAPbeDCm5hjq06fbs=SUPR1rm3bD3GJvifZovP1d-Xd=01JfpYQ@mail.gmail.com>
On 06/01/2019 22:51, Jacob Kroon wrote:
> Hi,
>
> Not sure if this has already been reported, but I observe this odd
> behaviour in gitk from master:
>
> git status
> gitk # everything looks good
> touch <file-under-version-control>
> gitk # gitk shows "local uncomitted changes" on the file I touched
> git status
> gitk # gitk is back to normal again, showing no local uncommitted changes
>
> The issue has been discussed on stackoverflow here:
> https://stackoverflow.com/questions/49990403/after-tar-untar-of-git-repo-gitk-shows-local-uncommitted-changes-not-checked
>
> Any chance gitk could be changed to so that it doesn't display the
> "local uncommitted changes" blob in this case ?
>
> Regards Jacob
I believe this is doing the right thing (TM) at the level of
investigation that gitk uses to determine the status of the files. In
particular, Git uses the modified time stamp as a surrogate indication
for detecting that the user has probably edited the file (it's been
modified at time hhmmss, right?).
Now as I understand it, the full (without limiting options) git status
command does go and check the content of anything that's potentially
changed (but it can be costly), and at that point the status command
simply updates its 'Index' record with the new mtime after noticing that
nothing had really changed. Meanwhile, gitk, being a continuously
running GUI avoids the overhead of the git status (though you can force
it) and does report the mtime change as being a potential file modification.
There is a separate discussion on the git users forum regarding the
compatibility with other tools that has a similar root cause in the use
and abuse of mtime as a canary for modification, given that the Git repo
storage does not record any file times, so will get a (moderately)
arbitrary mtime & ctime when checked out.
--
Philip
^ permalink raw reply
* Re: [PATCH 1/3] object-store: factor out odb_loose_cache()
From: René Scharfe @ 2019-01-07 17:29 UTC (permalink / raw)
To: Jeff King; +Cc: git, Junio C Hamano, Ævar Arnfjörð Bjarmason
In-Reply-To: <4b9198d2-4994-6d26-74ee-e737ecc34af3@web.de>
Am 07.01.2019 um 14:26 schrieb René Scharfe:
> Yes, the functions are arranged so that odb_load_loose_cache() can be
> inlined easily. I meant to include a patch for that but then quibbled
> about keeping the BUG check (which is probably optimized out) or not,
> and dropped it for now to get the performance fix in more quickly..
... which is probably just my laziness talking. Sent a minimal patch
for that now.
René
^ permalink raw reply
* [PATCH 4/3] object-store: retire odb_load_loose_cache()
From: René Scharfe @ 2019-01-07 17:29 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason
In-Reply-To: <3512c798-aa42-6fba-ee82-d33a8985be91@web.de>
Inline odb_load_loose_cache() into its only remaining caller,
odb_loose_cache(). The latter offers a nicer interface for loading the
cache, as it doesn't require callers to deal with fanout directory
numbers directly.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
object-store.h | 7 -------
sha1-file.c | 9 ++-------
2 files changed, 2 insertions(+), 14 deletions(-)
diff --git a/object-store.h b/object-store.h
index 2fb6c0e4db..e16aa38cae 100644
--- a/object-store.h
+++ b/object-store.h
@@ -47,13 +47,6 @@ void add_to_alternates_file(const char *dir);
*/
void add_to_alternates_memory(const char *dir);
-/*
- * Populate an odb's loose object cache for one particular subdirectory (i.e.,
- * the one that corresponds to the first byte of objects you're interested in,
- * from 0 to 255 inclusive).
- */
-void odb_load_loose_cache(struct object_directory *odb, int subdir_nr);
-
/*
* Populate and return the loose object cache array corresponding to the
* given object ID.
diff --git a/sha1-file.c b/sha1-file.c
index c3c6e50704..efcb2cbe74 100644
--- a/sha1-file.c
+++ b/sha1-file.c
@@ -2154,12 +2154,6 @@ struct oid_array *odb_loose_cache(struct object_directory *odb,
const struct object_id *oid)
{
int subdir_nr = oid->hash[0];
- odb_load_loose_cache(odb, subdir_nr);
- return &odb->loose_objects_cache[subdir_nr];
-}
-
-void odb_load_loose_cache(struct object_directory *odb, int subdir_nr)
-{
struct strbuf buf = STRBUF_INIT;
if (subdir_nr < 0 ||
@@ -2167,7 +2161,7 @@ void odb_load_loose_cache(struct object_directory *odb, int subdir_nr)
BUG("subdir_nr out of range");
if (odb->loose_objects_subdir_seen[subdir_nr])
- return;
+ return &odb->loose_objects_cache[subdir_nr];
strbuf_addstr(&buf, odb->path);
for_each_file_in_obj_subdir(subdir_nr, &buf,
@@ -2176,6 +2170,7 @@ void odb_load_loose_cache(struct object_directory *odb, int subdir_nr)
&odb->loose_objects_cache[subdir_nr]);
odb->loose_objects_subdir_seen[subdir_nr] = 1;
strbuf_release(&buf);
+ return &odb->loose_objects_cache[subdir_nr];
}
void odb_clear_loose_cache(struct object_directory *odb)
--
2.20.1
^ permalink raw reply related
* Re: [PATCH 4/4] built-in rebase: call `git am` directly
From: Junio C Hamano @ 2019-01-07 17:19 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget; +Cc: git, Johannes Schindelin
In-Reply-To: <xmqqbm4wau51.fsf@gitster-ct.c.googlers.com>
Junio C Hamano <gitster@pobox.com> writes:
> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
>> While the scripted `git rebase` still has to rely on the
>> `git-rebase--am.sh` script to implement the glue between the `rebase`
>> and the `am` commands, we can go a more direct route in the built-in
>> rebase and avoid using a shell script altogether.
>
> ...
>
>> builtin/rebase.c | 183 +++++++++++++++++++++++++++++++++++++++++++++++
>
> Now at some point as a follow-up change, we'd need to remove the
> git-rebase--am.sh that is no longer used, together with the
> reference to it in the Makefile, no?
Answering to myself. That needs to wait until the legacy-rebase is
retired, so it's not like "immediately after this series is done"
kind of thing.
Anyway, thanks for working on this.
^ permalink raw reply
* Re: [PATCH v2] Add optional targets for documentation l10n
From: Junio C Hamano @ 2019-01-07 17:17 UTC (permalink / raw)
To: Jean-Noël Avila; +Cc: git
In-Reply-To: <20190105134438.11271-1-jn.avila@free.fr>
Jean-Noël Avila <jn.avila@free.fr> writes:
> +ifdef MAN_FILTER
> +MAN_TXT = $(filter $(MAN_FILTER),$(MAN1_TXT) $(MAN5_TXT) $(MAN7_TXT))
> +else
> MAN_TXT = $(MAN1_TXT) $(MAN5_TXT) $(MAN7_TXT)
> +MAN_FILTER = $(MAN_TXT)
> +endif
OK.
> OBSOLETE_HTML += everyday.html
> OBSOLETE_HTML += git-remote-helpers.html
> -DOC_HTML = $(MAN_HTML) $(OBSOLETE_HTML)
>
> ARTICLES += howto-index
> ARTICLES += git-tools
> @@ -81,11 +86,13 @@ TECH_DOCS += technical/trivial-merge
> SP_ARTICLES += $(TECH_DOCS)
> SP_ARTICLES += technical/api-index
>
> -DOC_HTML += $(patsubst %,%.html,$(ARTICLES) $(SP_ARTICLES))
> +SP_ARTICLES_HTML += $(patsubst %,%.html,$(ARTICLES) $(SP_ARTICLES))
I'd call that $(ARTICLES_HTML); SP_ARTICLES are those pages that
want to become regular articles but singled out because they need
special handling to format.
> +HTML_FILTER ?= $(SP_ARTICLES_HTML) $(OBSOLETE_HTML)
> +DOC_HTML = $(MAN_HTML) $(filter $(HTML_FILTER),$(SP_ARTICLES_HTML) $(OBSOLETE_HTML))
> -DOC_MAN1 = $(patsubst %.txt,%.1,$(MAN1_TXT))
> -DOC_MAN5 = $(patsubst %.txt,%.5,$(MAN5_TXT))
> -DOC_MAN7 = $(patsubst %.txt,%.7,$(MAN7_TXT))
> +DOC_MAN1 = $(patsubst %.txt,%.1,$(filter $(MAN_FILTER), $(MAN1_TXT)))
> +DOC_MAN5 = $(patsubst %.txt,%.5,$(filter $(MAN_FILTER), $(MAN5_TXT)))
> +DOC_MAN7 = $(patsubst %.txt,%.7,$(filter $(MAN_FILTER), $(MAN7_TXT)))
Makes sense; s/, /,/, though.
^ permalink raw reply
* Re: [PATCH v4 0/8] Reimplement rebase --merge via interactive machinery
From: Elijah Newren @ 2019-01-07 17:15 UTC (permalink / raw)
To: Git Mailing List
Cc: Junio C Hamano, Johannes Schindelin, Pratik Karki, Phillip Wood
In-Reply-To: <20181211161139.31686-1-newren@gmail.com>
On Tue, Dec 11, 2018 at 8:11 AM Elijah Newren <newren@gmail.com> wrote:
>
> This series continues the work of making rebase more self-consistent
> by removing inconsistencies between different backends. In
> particular, this series focuses on making the merge machinery behave
> like the interactive machinery (though a few differences between the am
> and interactive backends are also fixed along the way), and ultimately
> removes the merge backend in favor of reimplementing the relevant
> options on top of the interactive machinery.
Friendly ping...let me know if you want me to simply resend v4.
> Differences since v3 (full range-diff below):
> - Fixed the redundant "fatal: error:" error message prefixes, as pointed
> out by Duy
> - Rebased on 2.20.0
>
> Elijah Newren (8):
> rebase: make builtin and legacy script error messages the same
> rebase: fix incompatible options error message
> t5407: add a test demonstrating how interactive handles --skip
> differently
> am, rebase--merge: do not overlook --skip'ed commits with post-rewrite
> git-rebase, sequencer: extend --quiet option for the interactive
> machinery
> git-legacy-rebase: simplify unnecessary triply-nested if
> rebase: define linearization ordering and enforce it
> rebase: Implement --merge via the interactive machinery
>
...
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2018, #02; Fri, 28)
From: Elijah Newren @ 2019-01-07 17:13 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <nycvar.QRO.7.76.6.1901031424340.45@tvgsbejvaqbjf.bet>
Hi Dscho,
On Thu, Jan 3, 2019 at 5:27 AM Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
>
> Hi Elijah,
>
...
> > I was going to re-ping in early January. Anyway, it may be worth at
> > least updating your note to "reroll exists".
>
> It is early January! ;-)
Indeed...ping incoming. :-)
>
> Ciao,
> Dscho
^ permalink raw reply
* Re: [PATCH] config.mak.dev: add -Wformat
From: Junio C Hamano @ 2019-01-07 17:04 UTC (permalink / raw)
To: Thomas Gummerer
Cc: Jonathan Nieder, Jeff King, git, Josh Steadmon, Masaya Suzuki
In-Reply-To: <20190106181758.GF25639@hank.intra.tgummerer.com>
Thomas Gummerer <t.gummerer@gmail.com> writes:
> I agree with the choice of adding -Wall to the CFLAGS here, so even if
> it is not added to the CFLAGS generated by autoconf (or in mnually set
> up CFLAGS such as in my original case), we still get a complete set of
> warnings when DEVELOPER=YesPlease is set.
>
>> -- >8 --
>> From: Thomas Gummerer <t.gummerer@gmail.com>
>> Date: Fri, 12 Oct 2018 19:40:37 +0100
>> Subject: [PATCH] config.mak.dev: add -Wformat
Thanks. I noticed, before merging the topic to 'next', that I
needed to retitle this further. I'd use something like this.
Subject: config.mak.dev: add -Wall to help autoconf users
^ permalink raw reply
* Re: [PATCH v2 8/8] checkout: introduce checkout.overlayMode config
From: Junio C Hamano @ 2019-01-07 17:00 UTC (permalink / raw)
To: Thomas Gummerer; +Cc: git, Nguyễn Thái Ngọc Duy, Elijah Newren
In-Reply-To: <20190106183225.GH25639@hank.intra.tgummerer.com>
Thomas Gummerer <t.gummerer@gmail.com> writes:
> Maybe it would be enough to say "... `git checkout` never removes
> files, that are not in the tree being checked out, from the index or
> the working tree"? It is more technically correct, but dunno making
> the sentence harder to read is worth it.
Yeah, I share the same feeling. Let's say the text in the posted
patch is good enough and move on.
Thanks.
^ 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