* [RFC/PATCH] Use mailmap by default in log, show and whatchanged
From: CB Bailey @ 2018-12-13 12:09 UTC (permalink / raw)
To: git
From: CB Bailey <cbailey32@bloomberg.net>
People who have changed their name or email address will usually know
that they need to set 'log.mailmap' in order to have their new details
reflected for old commits with 'git log', but others who interact with
them may not know or care enough to enable this option.
Change the default for 'git log' and friends to always use mailmap so
that everyone gets to see canonical names and email addresses.
Signed-off-by: CB Bailey <cbailey32@bloomberg.net>
---
Related to my patch to make shortlog pass the mailmap into the revision
walker which may end up being configuratble behavior, I thought I'd
propose this for discussion.
For people who change their names during their involvement in a project,
it can be important that the people with whom they work only see their
correct name, even when browsing old history.
I had a dig around in the mailing list archives and couldn't find any
specific reason not to use a mailmap in log where one is in use. I did
find this message which suggests that it makes sense to make it the
default for human-consumed formats. This patch doesn't affect
"--pretty=raw" formatting.
Documentation/config/log.txt | 4 ++--
Documentation/git-log.txt | 12 +++++++++---
builtin/log.c | 2 +-
t/t4203-mailmap.sh | 18 ++++++++++++++++++
4 files changed, 30 insertions(+), 6 deletions(-)
diff --git a/Documentation/config/log.txt b/Documentation/config/log.txt
index 78d9e4453a..8a01eed46b 100644
--- a/Documentation/config/log.txt
+++ b/Documentation/config/log.txt
@@ -39,5 +39,5 @@ log.showSignature::
linkgit:git-whatchanged[1] assume `--show-signature`.
log.mailmap::
- If true, makes linkgit:git-log[1], linkgit:git-show[1], and
- linkgit:git-whatchanged[1] assume `--use-mailmap`.
+ If false, makes linkgit:git-log[1], linkgit:git-show[1], and
+ linkgit:git-whatchanged[1] assume `--no-use-mailmap`.
diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 90761f1694..7815c9a6cb 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -50,9 +50,11 @@ OPTIONS
commit was reached.
--use-mailmap::
- Use mailmap file to map author and committer names and email
- addresses to canonical real names and email addresses. See
- linkgit:git-shortlog[1].
+--no-use-mailmap::
+ Use (or don't use) mailmap file to map author and committer names and
+ email addresses to canonical real names and email addresses. The default
+ is to use the mailmap file, but this can be overriden with the
+ `log.mailmap` configuration option. See linkgit:git-shortlog[1].
--full-diff::
Without this flag, `git log -p <path>...` shows commits that
@@ -205,6 +207,10 @@ log.showRoot::
`git log -p` output would be shown without a diff attached.
The default is `true`.
+log.mailmap::
+ If `false`, makes `git log` and related commands assume
+ `--no-use-mailmap`.
+
log.showSignature::
If `true`, `git log` and related commands will act as if the
`--show-signature` option was passed to them.
diff --git a/builtin/log.c b/builtin/log.c
index e8e51068bd..41a5eefb20 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -46,7 +46,7 @@ static int default_follow;
static int default_show_signature;
static int decoration_style;
static int decoration_given;
-static int use_mailmap_config;
+static int use_mailmap_config = 1;
static const char *fmt_patch_subject_prefix = "PATCH";
static const char *fmt_pretty;
diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh
index 43b1522ea2..efb145c4cd 100755
--- a/t/t4203-mailmap.sh
+++ b/t/t4203-mailmap.sh
@@ -424,6 +424,24 @@ EOF
test_expect_success 'Log output with --use-mailmap' '
git log --use-mailmap | grep Author >actual &&
+ test_cmp expect actual &&
+# --use-mailmap is the default
+ git log | grep Author >actual &&
+ test_cmp expect actual
+'
+
+cat >expect <<\EOF
+Author: CTO <cto@coompany.xx>
+Author: claus <me@company.xx>
+Author: santa <me@company.xx>
+Author: nick2 <nick2@company.xx>
+Author: nick2 <bugs@company.xx>
+Author: nick1 <bugs@company.xx>
+Author: A U Thor <author@example.com>
+EOF
+
+test_expect_success 'Log output with --no-use-mailmap' '
+ git log --no-use-mailmap | grep Author >actual &&
test_cmp expect actual
'
--
2.20.0
^ permalink raw reply related
* Re: [PATCH v2] run-command: report exec failure
From: Johannes Schindelin @ 2018-12-13 11:15 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King
In-Reply-To: <xmqqbm5qioca.fsf_-_@gitster-ct.c.googlers.com>
Hi Junio,
On Thu, 13 Dec 2018, Junio C Hamano wrote:
> I am taking that https://travis-ci.org/git/git/jobs/466908193
> that succeeded on Windows as a sign that this is now OK there.
I concur,
Dscho
^ permalink raw reply
* Re: [PATCH v2 1/3] http: add support for selecting SSL backends at runtime
From: Ævar Arnfjörð Bjarmason @ 2018-12-13 9:33 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget
Cc: git, Junio C Hamano, Johannes Schindelin
In-Reply-To: <85bd0fb27fcf7615b3f927344fd77ea49b9f5dcb.1540493630.git.gitgitgadget@gmail.com>
On Thu, Oct 25 2018, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> As of version 7.56.0, curl supports being compiled with multiple SSL
> backends.
>
> This patch adds the Git side of that feature: by setting http.sslBackend
> to "openssl" or "schannel", Git for Windows can now choose the SSL
> backend at runtime.
>
> This comes in handy on Windows because Secure Channel ("schannel") is
> the native solution, accessing the Windows Credential Store, thereby
> allowing for enterprise-wide management of certificates. For historical
> reasons, Git for Windows needs to support OpenSSL still, as it has
> previously been the only supported SSL backend in Git for Windows for
> almost a decade.
>
> The patch has been carried in Git for Windows for over a year, and is
> considered mature.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> Documentation/config.txt | 5 +++++
> http.c | 35 +++++++++++++++++++++++++++++++++++
> 2 files changed, 40 insertions(+)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 154683321..7d38f0bf1 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -1984,6 +1984,11 @@ http.sslCAPath::
> with when fetching or pushing over HTTPS. Can be overridden
> by the `GIT_SSL_CAPATH` environment variable.
>
> +http.sslBackend::
> + Name of the SSL backend to use (e.g. "openssl" or "schannel").
> + This option is ignored if cURL lacks support for choosing the SSL
> + backend at runtime.
> +
> http.pinnedpubkey::
> Public key of the https service. It may either be the filename of
> a PEM or DER encoded public key file or a string starting with
> diff --git a/http.c b/http.c
> index 98ff12258..7fb37a061 100644
> --- a/http.c
> +++ b/http.c
> @@ -155,6 +155,8 @@ static struct active_request_slot *active_queue_head;
>
> static char *cached_accept_language;
>
> +static char *http_ssl_backend;
> +
> size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_)
> {
> size_t size = eltsize * nmemb;
> @@ -302,6 +304,12 @@ static int http_options(const char *var, const char *value, void *cb)
> curl_ssl_try = git_config_bool(var, value);
> return 0;
> }
> + if (!strcmp("http.sslbackend", var)) {
> + free(http_ssl_backend);
> + http_ssl_backend = xstrdup_or_null(value);
> + return 0;
> + }
> +
> if (!strcmp("http.minsessions", var)) {
> min_curl_sessions = git_config_int(var, value);
> #ifndef USE_CURL_MULTI
> @@ -995,6 +1003,33 @@ void http_init(struct remote *remote, const char *url, int proactive_auth)
> git_config(urlmatch_config_entry, &config);
> free(normalized_url);
>
> +#if LIBCURL_VERSION_NUM >= 0x073800
> + if (http_ssl_backend) {
> + const curl_ssl_backend **backends;
> + struct strbuf buf = STRBUF_INIT;
> + int i;
> +
> + switch (curl_global_sslset(-1, http_ssl_backend, &backends)) {
> + case CURLSSLSET_UNKNOWN_BACKEND:
> + strbuf_addf(&buf, _("Unsupported SSL backend '%s'. "
> + "Supported SSL backends:"),
> + http_ssl_backend);
> + for (i = 0; backends[i]; i++)
> + strbuf_addf(&buf, "\n\t%s", backends[i]->name);
> + die("%s", buf.buf);
> + case CURLSSLSET_NO_BACKENDS:
> + die(_("Could not set SSL backend to '%s': "
> + "cURL was built without SSL backends"),
> + http_ssl_backend);
> + case CURLSSLSET_TOO_LATE:
> + die(_("Could not set SSL backend to '%s': already set"),
> + http_ssl_backend);
> + case CURLSSLSET_OK:
> + break; /* Okay! */
> + }
> + }
> +#endif
> +
> if (curl_global_init(CURL_GLOBAL_ALL) != CURLE_OK)
> die("curl_global_init failed");
Here's someone who upgraded to 2.20 on Arch linux & started getting
"Could not set..." errors because of this change:
https://www.reddit.com/r/git/comments/a5ne5v/git_fatal_could_not_set_ssl_backend_to_openssl/
I don't know the context well enough, but is there perhaps enough info
here so we could give a better error message, e.g. "don't set xyz twice
in your config", or just emit a warning?
^ permalink raw reply
* 2.20.0 - Undocumented change in submodule update wrt # parallel jobs
From: Sjon Hortensius @ 2018-12-13 9:15 UTC (permalink / raw)
To: git
When switching to 2.20 our `git submodule update' (which clones
through ssh) broke because our ssh-server rejected the ~20
simultaneous connections the git-client makes. This seems to be caused
by a (possibly unintended) change in
https://github.com/git/git/commit/90efe595c53f4bb1851371344c35eff71f604d2b
which removed the default of max_jobs=1
While this can easily be fixed by configuring submodule.fetchJobs I
think this change should be documented - or reverted back to it's
previous default of 1
--
Kind regards,
Sjon Hortensius
| Parse Software Development B.V.
| http://www.parse.nl
^ permalink raw reply
* Re: [PATCH v2] run-command: report exec failure
From: Junio C Hamano @ 2018-12-13 8:17 UTC (permalink / raw)
To: Jeff King; +Cc: git, Johannes Schindelin
In-Reply-To: <20181213080825.GB12132@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Thu, Dec 13, 2018 at 03:36:53AM +0900, Junio C Hamano wrote:
>
>> test_expect_success 'start_command reports ENOENT (slash)' '
>> - test-tool run-command start-command-ENOENT ./does-not-exist
>> + test-tool run-command start-command-ENOENT ./does-not-exist 2>err &&
>> + test_i18ngrep "\./does-not-exist" err
>> '
>
> I thought at first you could use "grep" here, since we know that the
> name of the file would appear untranslated. But I think the way
> GETTEXT_POISON works, it actually eats the whole string, including
> placeholders (which IMHO is a failing of GETTEXT_POISON, since no real
> translation would do that, but not worth caring too much about).
When Ævar's dynamic gettext poison topic was discussed, there was an
idea or two floated for a possible follow-up to introduce a true
"fake translation", replacing e with é, n with ñ, etc., while
keeping the printf formaters intact. When that comes, we should be
able to use grep and that would make the result more robust than the
current test_i18ngrep that always pretends to have seen a match, but
that hasn't happened yet.
^ permalink raw reply
* Re: [PATCH] run-command: report exec failure
From: Jeff King @ 2018-12-13 8:10 UTC (permalink / raw)
To: John Passaro; +Cc: Junio C Hamano, Johannes.Schindelin, git
In-Reply-To: <CAJdN7Khf+Y_jyyG2qoqiMHYPCHHSws15EDftOQni=gFJ7SoMXg@mail.gmail.com>
On Wed, Dec 12, 2018 at 10:27:40AM -0500, John Passaro wrote:
> Thank you for this incredibly quick fix.
>
> I see the fix made it to pu as 6b206be3e5 ("run-command: report exec
> failure" 2018-12-11). For what it's worth, it fixes the issue as far
> as I'm concerned and I'm very glad to see the behavior is covered by
> tests now.
>
> As a procedural question: I'd like to reference this patch in one of
> my own. Can I reference it as I typed it above? Or is there a chance
> of the SHA1 changing before it goes into some sort of a main history?
Commits in "pu" are still subject to change (and indeed, this one was
amended to e5a329a279 to fix the grep issue on Windows).
Once it hits "next" it is generally stable. That hasn't happened yet,
but I think what's there now is likely to get merged as-is (and will
retain that commit id).
-Peff
^ permalink raw reply
* Re: [PATCH v2] run-command: report exec failure
From: Jeff King @ 2018-12-13 8:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Johannes Schindelin
In-Reply-To: <xmqqbm5qioca.fsf_-_@gitster-ct.c.googlers.com>
On Thu, Dec 13, 2018 at 03:36:53AM +0900, Junio C Hamano wrote:
> In 321fd823 ("run-command: mark path lookup errors with ENOENT",
> 2018-10-24), we rewrote the logic to execute a command by looking
> in the directories on $PATH; as a side effect, a request to run a
> command that is not found on $PATH is noticed even before a child
> process is forked to execute it.
>
> We however stopped to report an exec failure in such a case by
> mistake. Add a logic to report the error unless silent-exec-failure
> is requested, to match the original code.
>
> Reported-by: John Passaro <john.a.passaro@gmail.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Thanks, this looks good to me.
> test_expect_success 'start_command reports ENOENT (slash)' '
> - test-tool run-command start-command-ENOENT ./does-not-exist
> + test-tool run-command start-command-ENOENT ./does-not-exist 2>err &&
> + test_i18ngrep "\./does-not-exist" err
> '
I thought at first you could use "grep" here, since we know that the
name of the file would appear untranslated. But I think the way
GETTEXT_POISON works, it actually eats the whole string, including
placeholders (which IMHO is a failing of GETTEXT_POISON, since no real
translation would do that, but not worth caring too much about).
-Peff
^ permalink raw reply
* Re: [PATCH v3 1/4] pack-protocol.txt: accept error packets in any context
From: Jeff King @ 2018-12-13 8:04 UTC (permalink / raw)
To: Masaya Suzuki; +Cc: Josh Steadmon, git, Junio C Hamano
In-Reply-To: <CAJB1erXRqQW0yQyZutJAJKC7WbdVhBAYUMWM+8ZutxA-W-7S8w@mail.gmail.com>
On Wed, Dec 12, 2018 at 05:17:01PM -0800, Masaya Suzuki wrote:
> > This is a change in the spec with an accompanying change in the code,
> > which raises the question: what do other implementations do with this
> > change (both older Git, and implementations like JGit, libgit2, etc)?
>
> JGit is similar to Git. It parses "ERR " in limited places. When it sees an ERR
> packet in an unexpected place, it'll fail somewhere in the parsing code.
>
> https://github.com/eclipse/jgit/blob/30c6c7542190c149e2aee792f992a312a5fc5793/org.eclipse.jgit/src/org/eclipse/jgit/transport/PacketLineIn.java#L145-L147
> https://github.com/eclipse/jgit/blob/f40b39345cd9b54473ee871bff401fe3d394ffe3/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackConnection.java#L208
>
> I'm not familiar with libgit2 code, but it seems it handles this at a
> lower level. An error type packet is parsed out at a low level, and
> the error handling is done by the callers of the packet parser.
>
> https://github.com/libgit2/libgit2/blob/bea65980c7a42e34edfafbdc40b199ba7b2a564e/src/transports/smart_pkt.c#L482-L483
>
> I cannot find an ERR packet handling in go-git. It seems to me that if
> an ERR packet appears it treats it as a parsing error.
>
> https://github.com/src-d/go-git/blob/master/plumbing/protocol/packp/common.go#L60-L62
Thanks for digging into these. It does make sense that other
implementations would give a parsing error. Hopefully they also produce
a sensible error message (ideally printing the bogus pktline), but even
if they don't we're probably no worse off than the status quo. With the
current scheme, the server can't give any message, and just has to hang
up anyway.
> > I think the answer for older Git is "hang up unceremoniously", which is
> > probably OK given the semantics of the change. And I'd suspect most
> > other implementations would do the same. I just wonder if anybody tested
> > it with other implementations.
>
> I'm thinking aloud here. There would be two aspects of the protocol
> compatibility: (1) new clients speak to old servers (2) old clients
> speak to a new server that speaks the updated protocol.
>
> For (1), I assume that in the Git pack protocol, a packet starting
> from "ERR " does not appear naturally except for a very special case
> that the server doesn't support sideband, but using the updated
> protocol. As you mentioned, at first it looks like this can mistakenly
> parse the pack file of git-receive-pack as an ERR packet, assuming
> that git-receive-pack's pack file is packetized. Actually
> git-receive-pack's pack file is not packetized in the Git pack
> protocol (https://github.com/git/git/blob/master/builtin/receive-pack.c#L1695).
> I recently wrote a Git protocol parser
> (https://github.com/google/gitprotocolio), and I confirmed that this
> is the case at least for the HTTP transport. git-upload-pack's pack
> file is indeed packetized, but packetized with sideband. Except for
> the case where sideband is not used, the packfiles wouldn't be
> considered as an ERR packet accidentally.
Right, that matches my understanding.
> For (2), if the old clients see an unexpected ERR packet, they cannot
> parse it. They would handle this unparsable data as if the server is
> not speaking Git protocol correctly. Even if the old clients just
> ignore the packet, due to the nature of the ERR packet, the server
> won't send further data. The client won't be able to proceed. Overall,
> the clients anyway face an error, and the only difference would be
> whether the clients can show an error nicely or not. The new clients
> will show the errors nicely to users. Old clients will not.
Yeah, this was the case I was more concerned about, and I think it is
probably OK (by this rationale, and what I wrote above).
> > So I think it's probably not a good idea to unconditionally have callers
> > of packet_read_with_status() handle this. We'd need a flag like
> > PACKET_READ_RESPECT_ERR, and to trigger it from the appropriate callers.
>
> This is outside of the Git pack protocol so having a separate parsing
> mode makes sense to me.
Yeah. Here's a sample script which works with current Git (the index
contains the uppercased content "ERR FOO"), but fails after this patch
(Git thinks the filter reported an error and dies; it's not great that
we die in the packet-reading code at all for this case, but your patch
is hardly the first call to die() in that function).
-- >8 --
git init -q repo
cd repo
echo '*.magic filter=magic' >.git/info/attributes
git config filter.magic.process $PWD/filter
# toy filter to uppercase content
cat >filter <<-\EOF
#!/usr/bin/perl
sub read_pkt {
my @r;
while (1) {
read(STDIN, my $len, 4);
last if $len eq "0000";
read(STDIN, my $buf, hex($len)-4);
push @r, $buf;
}
return @r;
}
sub write_pkt {
local $| = 1;
while (@_) {
my $buf = shift;
printf "%04x", length($buf) + 4;
print $buf;
}
print "0000";
}
read_pkt(); # handshake
write_pkt(qw(git-filter-server version=2));
read_pkt(); # capabilities
write_pkt(qw(capability=clean));
read_pkt(); # clean command
@content = read_pkt();
write_pkt(qw(status=success));
write_pkt(map { uc } @content);
write_pkt(); # final status
EOF
chmod +x filter
echo 'err foo' >foo.magic
git add foo.magic
git cat-file blob :foo.magic
^ permalink raw reply
* Re: [PATCH] cherry-pick: do not error on non-merge commits when '-m 1' is specified
From: Sergey Organov @ 2018-12-13 6:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqo99qf46q.fsf@gitster-ct.c.googlers.com>
Junio C Hamano <gitster@pobox.com> writes:
> Sergey Organov <sorganov@gmail.com> writes:
>
>> When cherry-picking multiple commits, it's impossible to have both
>> merge- and non-merge commits on the same command-line. Not specifying
>> '-m 1' results in cherry-pick refusing to handle merge commits, while
>> specifying '-m 1' fails on non-merge commits.
>>
>> This patch allows '-m 1' for non-merge commits. Besides, as mainline is
>> always the only parent for a non-merge commit, it made little sense to
>> disable it in the first place.
>
> In the original context to pick a single commit, it made perfect
> sense to avoid mistakes by blindly passing '-m 1' to non-merge
> commit.
>
> It may be fair to say that we failed to reconsider what to
> do with '-m 1' when we did 7e2bfd3f, but it is utterly an unfair
> history revisionism to say that it made little sense to disable it
> in the first place.
In fact I had zero intention on any revisionism. Now I see I should have
said "makes little sense" in present tense to avoid touching deep
historical issues, sorry!
Something like:
"This patch allows '-m 1' for non-merge commits. As mainline is always
the only parent for a non-merge commit, it makes little sense to disable
it."
sounds OK?
>
> The change to the code itself looks sane, but applying this patch
> alone will break existing tests whose expectations must be updated,
> and this new behaviour must be protected by a new test (or two) so
> that we won't accidentally stop accepting "-m 1" for a single-parent
> commit.
I fixed most of the tests, but
"t3510/4: cherry-pick persists opts correctly"
is an offender for me. It looks like it [ab]uses current "-m 1" behavior
just to stop in the middle of the sequence, and I'm not sure how to fix
it most suitably.
Thanks!
--
Sergey
^ permalink raw reply
* Preparing for 2.20.1 brown-paper-bag maintenance release
From: Junio C Hamano @ 2018-12-13 6:29 UTC (permalink / raw)
To: git
Cc: Johannes Schindelin, Derrick Stolee,
Nguyễn Thái Ngọc Duy
Here is an excerpt from a draft edition of "What's cooking" report
for topics that are about an immediate 2.20.1 maintenance release,
with five topics that I plan to merge to 'next' and then to 'maint'
soonish (they're all marked as "Will merge to 'next' and then to
'master'").
They'll be in 'pu' but not in 'next' in today's pushout, but unless
I hear breakage reports in time, I am hoping to merge them to 'next'
during tomorrow's integration cycle, so that we can start the new
week with 2.20.1.
Thanks.
--------------------------------------------------
* ds/hash-independent-tests-fix (2018-12-12) 1 commit
- .gitattributes: ensure t/oid-info/* has eol=lf
Test portability fix.
* jc/run-command-report-exec-failure-fix (2018-12-12) 1 commit
- run-command: report exec failure
A recent update accidentally squelched an error message when the
run_command API failed to run a missing command, which has been
corrected.
* nd/show-gitcomp-compilation-fix (2018-12-12) 1 commit
- parse-options: fix SunCC compiler warning
Portability fix for a recent update to parse-options API.
* js/help-commands-verbose-by-default-fix (2018-12-12) 2 commits
- help -a: handle aliases with long names gracefully
- help.h: fix coding style
"git help -a" did not work well when an overly long alias is
defined, which has been corrected.
* js/mailinfo-format-flowed-fix (2018-12-13) 1 commit
- t4256: mark support files as LF-only
Test portability fix.
^ permalink raw reply
* Re: [PATCH 2/2] mingw: allow absolute paths without drive prefix
From: Johannes Sixt @ 2018-12-13 6:25 UTC (permalink / raw)
To: Junio C Hamano
Cc: Johannes Schindelin, Johannes Schindelin via GitGitGadget, git
In-Reply-To: <xmqqmupagn0y.fsf@gitster-ct.c.googlers.com>
Am 13.12.18 um 03:48 schrieb Junio C Hamano:
> So,... what's the conclusion? The patch in the context of my tree
> would be a no-op, and we'd need a prerequisite change to the support
> function to accompany this patch to be effective?
Correct, that is my conclusion.
-- Hannes
^ permalink raw reply
* Re: [RFC] A global mailmap service
From: Lukas Fleischer @ 2018-12-13 6:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqq1s6mi30u.fsf@gitster-ct.c.googlers.com>
On Thu, 13 Dec 2018 at 03:17:21, Junio C Hamano wrote:
> Lukas Fleischer <lfleischer@lfos.de> writes:
>
> > The basic idea of the service I imagine is simple:
> >
> > 1. You register a primary email address and specify a password. You
> > receive a verification email to confirm that the address is yours.
>
> I would do so with my current, reachable address, I'd presume.
Correct.
>
> > 2. At any time, you can add additional email addresses and link them to
> > your primary email address, using your previously specified password.
> > You can also update your primary email address. Any new addresses
> > obtain verification emails such that you cannot steal somebody else's
> > identity.
>
> With this, I won't be able to add my ancient identities that appear
> in our history. I would imagine that one of the common reasons
> people use different identities in a project is that people changed
> e-mail providers or jobs.
>
Well, this is only a temporary issue. It holds for your current ancient
identities but won't hold for your ancient identities in the far future
because ancient times have always been present at some point in time. If
we agreed that most people register their current email addresses from
now on, we'd limit the issue to email addresses which were abandoned
before 2018-12-13. As projects grow, this will become a small fraction.
Projects started in the far future won't be affected at all.
As a short-term solution, we could keep the current mappings as
complementary mappings in the local .mailmap files.
That being said, I also had the idea of importing old mappings to the
service. There are two approaches that crossed my mind:
1. Link email addresses according to the .mailmap files of popular
trusted projects, such as Git or the Linux kernel. One of the issues
with this approach is that "topic email addresses", i.e. project
specific preferences, may not be mapped correctly. Maybe it doesn't
matter too much.
2. Add the option to register a "dead" email address to the service.
Instead of sending a verification email, the service sends a warning
stating that the email address will be linked if no action is taken.
It contains a link where the user can cancel the request and block
the email address from further requests. Another such warning is sent
after a couple of days. After a grace period with no action taken,
the address is linked. Not optimal but with additional measures such
as rate limits to prevent from abuse, this might be good enough.
Best regards,
Lukas
^ permalink raw reply
* Re: [PATCHSET] git-reverse-trailer-xrefs: Reverse map cherry-picks and other cross-references
From: Junio C Hamano @ 2018-12-13 5:47 UTC (permalink / raw)
To: Tejun Heo; +Cc: git, Jeff King, kernel-team, Stefan Xenos
In-Reply-To: <20181213034041.GR2509588@devbig004.ftw2.facebook.com>
Tejun Heo <tj@kernel.org> writes:
> Hmmm... I see. I still have a bit of trouble seeing why doing it that
> way is better tho. Wouldn't new-object-hook be simpler? They'll
> achieve about the same thing but one would need to keep the states in
> two places.
The new-object-hook thing will not have keep the states. Whenever
Git realizes that it created a new object, it must call that hook,
and at that point in time, without keeping any state, it knows which
objects are what it has just created. So "in two places" is not a
problem at all. There is only one place (i.e. the place the sweeper
would record what it just did to communicate with its future
invocation).
A new-object-hook that will always be called any time a new object
enters the picture would be a nightmare to maintain up-to-date. One
missed codepath and your coverage will have holes. Having a back-up
mechanism to sweep for new objects will give you a better chance of
staying correct as the system evolves, I'd think.
^ permalink raw reply
* Re: [PATCH 5/5] midx: implement midx_repack()
From: Junio C Hamano @ 2018-12-13 4:23 UTC (permalink / raw)
To: Derrick Stolee via GitGitGadget
Cc: git, sbeller, peff, jrnieder, avarab, Derrick Stolee
In-Reply-To: <xmqqbm5rjipo.fsf@gitster-ct.c.googlers.com>
Junio C Hamano <gitster@pobox.com> writes:
> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>> + SECOND_SMALLEST_SIZE=$(ls -l .git/objects/pack/*pack | awk "{print \$5;}" | sort -n | head -n 2 | tail -n 1) &&
>
> awk is capable of remembering $5 from each line of input, sorting
> them and picking the second smallest element from it, isn't it?
>
>> + BATCH_SIZE=$(($SECOND_SMALLEST_SIZE + 1)) &&
>
> ... or incrementing the number by one, before reporting, for that
> matter.
Oops, please disregard. My awk is rusty. Unless we are willing to
rely on gawk, which we are not, it is not practical to sort inside
awk.
^ permalink raw reply
* Re: [PATCH] cherry-pick: do not error on non-merge commits when '-m 1' is specified
From: Junio C Hamano @ 2018-12-13 4:20 UTC (permalink / raw)
To: Sergey Organov; +Cc: git
In-Reply-To: <871s6n5mtd.fsf@javad.com>
Sergey Organov <sorganov@gmail.com> writes:
> When cherry-picking multiple commits, it's impossible to have both
> merge- and non-merge commits on the same command-line. Not specifying
> '-m 1' results in cherry-pick refusing to handle merge commits, while
> specifying '-m 1' fails on non-merge commits.
>
> This patch allows '-m 1' for non-merge commits. Besides, as mainline is
> always the only parent for a non-merge commit, it made little sense to
> disable it in the first place.
The feature to give a range to cherry-pick came much much later in
7e2bfd3f ("revert: allow cherry-picking more than one commit",
2010-06-02) that first appeared in v1.7.2. The feature to allow
picking a merge commit came in 7791ecbc ("revert/cherry-pick: work
on merge commits as well", 2007-10-23), first appeared in v1.5.4.
In the original context to pick a single commit, it made perfect
sense to avoid mistakes by blindly passing '-m 1' to non-merge
commit. It may be fair to say that we failed to reconsider what to
do with '-m 1' when we did 7e2bfd3f, but it is utterly an unfair
history revisionism to say that it made little sense to disable it
in the first place.
The change to the code itself looks sane, but applying this patch
alone will break existing tests whose expectations must be updated,
and this new behaviour must be protected by a new test (or two) so
that we won't accidentally stop accepting "-m 1" for a single-parent
commit.
Thanks.
> Signed-off-by: Sergey Organov <sorganov@gmail.com>
> ---
> sequencer.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/sequencer.c b/sequencer.c
> index e1a4dd1..d0fd61b 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -1766,9 +1766,13 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
> return error(_("commit %s does not have parent %d"),
> oid_to_hex(&commit->object.oid), opts->mainline);
> parent = p->item;
> - } else if (0 < opts->mainline)
> - return error(_("mainline was specified but commit %s is not a merge."),
> - oid_to_hex(&commit->object.oid));
> + } else if (1 < opts->mainline)
> + /*
> + * Non-first parent explicitly specified as mainline for
> + * non-merge commit
> + */
> + return error(_("commit %s does not have parent %d"),
> + oid_to_hex(&commit->object.oid), opts->mainline);
> else
> parent = commit->parents->item;
^ permalink raw reply
* Re: [PATCH v2 1/3] git clone <url> C:\cygwin\home\USER\repo' is working (again)
From: Junio C Hamano @ 2018-12-13 3:52 UTC (permalink / raw)
To: Steven Penny; +Cc: j6t, git
In-Reply-To: <CAAXzdLU0Desw=kt2A3qHx8v=8hvKcN9OdV9fnXEcYiO=ht-t9A@mail.gmail.com>
Steven Penny <svnpenn@gmail.com> writes:
> On Wed, Dec 12, 2018 at 1:29 AM Johannes Sixt wrote:
>> As long as C:\Windows\System32 on my Windows computer contains only
>> 64-Bit binaries, I consider the characters "3" and "2" next to each
>> other in this order just noise and without any form of information. The
>> important part of the name is "win".
>
> sorry friend, but thats a logical fallacy :(
>
> http://yourlogicalfallacyis.com/no-true-scotsman
>
> just because the name "System32" is still in use (wrongly, I might add) doesnt
> mean that "Win32" should still be in use.
>
> Each name is a separate argument. The "Win32" name has been changed by Microsoft
> and shouldnt be used by Git if possible. Its an easy change and I could send
> a pull request myself. However just because Microsoft hasnt changed "Sytem32"
> doesnt mean that they wont or shouldnt - as you said its just as misleading. We
> should fix ambiguities where we can, not embrace them.
FWIW, in the context of the Git development ecosystem, whatever
Dscho wants to do in compat/ that is limited to Windows is
"officially endorsed by Microsoft" enough.
Also I do not think J6t felt 32 in System32 or in win32 was
misleading. At least I didn't read the above that way.
^ permalink raw reply
* Re: [PATCHSET] git-reverse-trailer-xrefs: Reverse map cherry-picks and other cross-references
From: Tejun Heo @ 2018-12-13 3:46 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King, kernel-team, Stefan Xenos
In-Reply-To: <xmqqa7lagm18.fsf@gitster-ct.c.googlers.com>
Hello, Junio, Stefan.
On Thu, Dec 13, 2018 at 12:09:39PM +0900, Junio C Hamano wrote:
> Please do not take the above as "don't do notes/xref-; instead read
> from the 'meta commits'". I do not have a preference between the
> two proposed implementations. The important thing is that we won't
> end up with having to maintain two separate mechanisms that want to
> keep track of essentially the same class of information. FWIW I'd
> be perfectly fine if the unification goes the other way, as long as
> goals of both parties are met, and for that, I'd like to see you two
> work together, or at least be aware of what each other is doing and
> see if cross-pollination would result in a mutually better solution.
So, from my POV, the only thing I want is being able to easily tell
which commit got cherry picked where. I really don't care how that's
achieved. Stefan, if there's anything I can do to push it forward,
let me know and if you see anything useful in my patchset, please feel
free to incorporate them in any way.
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCHSET] git-reverse-trailer-xrefs: Reverse map cherry-picks and other cross-references
From: Tejun Heo @ 2018-12-13 3:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King, kernel-team, Stefan Xenos
In-Reply-To: <xmqqefamgmey.fsf@gitster-ct.c.googlers.com>
Hello, Junio.
On Thu, Dec 13, 2018 at 12:01:25PM +0900, Junio C Hamano wrote:
> > Wouldn't it be more useful to have repo-updated-with-these-commits
> > hook instead rather than putting more logic on note handling?
> >
> >> and scan the commits, just like you scan what you fetched. And when
> >> you update the reverse mapping notes tree, the commit to record that
> >> notes update can record the tip of the above traversal.
>
> I do not consider what you do in notes/xref-* "more logic on note
> handling" in the sense that the logic is part of "notes" API.
>
> The moment you decided to reserve one hierarchy in refs/notes/ and
> designed what the mapping recorded there means, you designed a new
> trailer-xrefs API. It is a part of that API how blobs stored in
> your refs/notes/xref-cherry-picks are formatted and what they mean.
> It's the same thing---it is also part of your API how the log
> message for recording commits in that hierarchy is formatted and
> what it means.
Hmmm... I see. I still have a bit of trouble seeing why doing it that
way is better tho. Wouldn't new-object-hook be simpler? They'll
achieve about the same thing but one would need to keep the states in
two places.
> > As long as we can keep the reverse rference notes consistent, wouldn't
> > amend propagation just consume them?
>
> Yes. Would that mean you do not need the notes/xref-* series we are
> seeing here, and instead (re)use what Stefan's series, which already
> needs to have access to and record the information anyway, records?
Oh yeah, for sure. Didn't know Stefan was doing something similar.
Will continue on the other reply.
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH 0/4]
From: Junio C Hamano @ 2018-12-13 3:15 UTC (permalink / raw)
To: Stefan Beller; +Cc: git
In-Reply-To: <CAGZ79kbPQx4Z0FHioQWxUYoJOKU0TxZXgxEDPFE7XQCMxtRqaw@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
>> - now these three changes that were problematic in the past is
>> resent without any update (other than that it has one preparatory
>> patch to add tests).
>
> One of the three changes was problematic, (e98317508c02),
> the other two are good (in company of the third).
Ah, that is what I failed to read.
> This means I need to revamp the commit messages and
> cover letter altogether.
I guess that would help future readers. Thanks.
^ permalink raw reply
* Re: [PATCH 3/4] submodule--helper: fix BUG message in ensure_core_worktree
From: Junio C Hamano @ 2018-12-13 3:14 UTC (permalink / raw)
To: Stefan Beller; +Cc: git
In-Reply-To: <CAGZ79kb0Vqk8Gtao6OdKx7gJi6pCEpLzcqQsk=uqCLfePZrmVw@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
>> Unlike the step 2/4 I commented on, this does explain what this
>> wants to do and why, at least when looked from sideways. Is the
>> above saying the same as the following two-liner?
>>
>> An ealier mistake while rebasing to produce 74d4731da1
>> failed to update this BUG message. Fix this.
>
> I am not sure if it was rebasing, which was executed mistakenly.
> So maybe just saying "74d4731da1 contains a faulty BUG
> message. Fix it." would do.
>
> The intent of the longer message was to shed light in how I found
> the BUG (ie. I did not see the BUG message, which would ask me
> to actually fix a bug, but found it via code inspection), which I
> thought was valuable information, too.
I guess that it could be stated in a way to make it valuable, but in
the presented text, I somehow found it was making the more important
part of the description (i.e. "this patch fixes a mistake made by
74d4731da1") buried and harder to grok.
Thanks.
^ permalink raw reply
* Re: [PATCHSET] git-reverse-trailer-xrefs: Reverse map cherry-picks and other cross-references
From: Junio C Hamano @ 2018-12-13 3:09 UTC (permalink / raw)
To: Tejun Heo; +Cc: git, Jeff King, kernel-team, Stefan Xenos
In-Reply-To: <xmqqefamgmey.fsf@gitster-ct.c.googlers.com>
Junio C Hamano <gitster@pobox.com> writes:
>> As long as we can keep the reverse rference notes consistent, wouldn't
>> amend propagation just consume them?
>
> Yes. Would that mean you do not need the notes/xref-* series we are
> seeing here, and instead (re)use what Stefan's series, which already
> needs to have access to and record the information anyway, records?
Just to avoid confusion or unnecessary guessing.
Please do not take the above as "don't do notes/xref-; instead read
from the 'meta commits'". I do not have a preference between the
two proposed implementations. The important thing is that we won't
end up with having to maintain two separate mechanisms that want to
keep track of essentially the same class of information. FWIW I'd
be perfectly fine if the unification goes the other way, as long as
goals of both parties are met, and for that, I'd like to see you two
work together, or at least be aware of what each other is doing and
see if cross-pollination would result in a mutually better solution.
Thanks
^ permalink raw reply
* Re: [PATCHSET] git-reverse-trailer-xrefs: Reverse map cherry-picks and other cross-references
From: Junio C Hamano @ 2018-12-13 3:01 UTC (permalink / raw)
To: Tejun Heo; +Cc: git, Jeff King, kernel-team, Stefan Xenos
In-Reply-To: <20181212145456.GQ2509588@devbig004.ftw2.facebook.com>
Tejun Heo <tj@kernel.org> writes:
>> allowed to be a bit stale and be completed immediately before it
>> gets used? A totally different approach could be to record list of
>> commits, all commits behind which have been scanned for reverse
>> mapping, in the tip of the notes history, perhaps in the commit log
>> message (which is machine generated anyway). Then, before you need
>> the up-to-date-to-the-last-second reverse mapping, you could run
>>
>> git rev-list --all --not $these_tips_recorded
>
> Wouldn't it be more useful to have repo-updated-with-these-commits
> hook instead rather than putting more logic on note handling?
>
>> and scan the commits, just like you scan what you fetched. And when
>> you update the reverse mapping notes tree, the commit to record that
>> notes update can record the tip of the above traversal.
I do not consider what you do in notes/xref-* "more logic on note
handling" in the sense that the logic is part of "notes" API.
The moment you decided to reserve one hierarchy in refs/notes/ and
designed what the mapping recorded there means, you designed a new
trailer-xrefs API. It is a part of that API how blobs stored in
your refs/notes/xref-cherry-picks are formatted and what they mean.
It's the same thing---it is also part of your API how the log
message for recording commits in that hierarchy is formatted and
what it means.
> As long as we can keep the reverse rference notes consistent, wouldn't
> amend propagation just consume them?
Yes. Would that mean you do not need the notes/xref-* series we are
seeing here, and instead (re)use what Stefan's series, which already
needs to have access to and record the information anyway, records?
^ permalink raw reply
* Re: [PATCH 0/3] Some fixes and improvements
From: Junio C Hamano @ 2018-12-13 2:49 UTC (permalink / raw)
To: Jonathan Tan; +Cc: git, avarab, peff
In-Reply-To: <cover.1544573604.git.jonathantanmy@google.com>
Jonathan Tan <jonathantanmy@google.com> writes:
> Thanks, Aevar for looking into this. I haven't looked into detail, but:
> ...
> If you agree with the general direction I'm going in, when you send out
> another version, I would add patch 2 somewhere near the beginning of the
> set, and then squash both patch 1 and patch 3 in the G_T_P_V=2 patch.
Thanks, both. I'll let these sit in the list archive and expect an
updated series ready to be picked up.
^ permalink raw reply
* Re: [PATCH 2/2] mingw: allow absolute paths without drive prefix
From: Junio C Hamano @ 2018-12-13 2:48 UTC (permalink / raw)
To: Johannes Sixt
Cc: Johannes Schindelin, Johannes Schindelin via GitGitGadget, git
In-Reply-To: <8a484f86-1d43-fc0a-22b4-39c770cda6cb@kdbg.org>
Johannes Sixt <j6t@kdbg.org> writes:
>>>> + if (is_dir_sep(template[0]) && !is_dir_sep(template[1]) &&
>>>> + iswalpha(wtemplate[0]) && wtemplate[1] == L':') {
>>>> + /* We have an absolute path missing the drive prefix */
>>>
>>> This comment is true for the source part, template, but I can't find
>>> where the destination, wtemplate, suddenly gets the drive prefix. As far
>>> as I can see, xutftowcs_path() just does a plain textual conversion
>>> without any interpretation of the text as path. Can you explain it?
>>
>> It is legal on Windows for such a path to lack the drive prefix, also in
>> the wide-character version. So the explanation is: even `wtemplate` won't
>> get the drive prefix. It does not need to.
>
> I'm sorry, my question was extremely fuzzy. I actually wanted to know
> how the condition that you introduce in this patch can ever be true.
>
> And after looking at the Git for Windows code, I could answer it
> myself: it cannot. Not with this patch alone. In GfW, there is
> additional code in xutftowcs_path() that massages wtemplate to receive
> a drive prefix; but vanilla Git does not have that code, so that
> is_dir_sep(template[0]) and iswalpha(wtemplate[0]) can never be true
> at the same time at this point.
So,... what's the conclusion? The patch in the context of my tree
would be a no-op, and we'd need a prerequisite change to the support
function to accompany this patch to be effective?
^ permalink raw reply
* Re: [PATCH v2 2/2] t4256: mark support files as LF-only
From: Junio C Hamano @ 2018-12-13 2:40 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget; +Cc: git, Johannes Schindelin
In-Reply-To: <4275b8a5812b7108aecfc027fd6ace9b470a7c88.1544638490.git.gitgitgadget@gmail.com>
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> The test t4256-am-format-flowed.sh requires carefully applying a
> patch after ignoring padding whitespace. This breaks if the file
> is munged to include CRLF line endings instead of LF.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---
Thanks, will queue.
> t/.gitattributes | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/t/.gitattributes b/t/.gitattributes
> index e7acedabe1..df05434d32 100644
> --- a/t/.gitattributes
> +++ b/t/.gitattributes
> @@ -16,6 +16,7 @@ t[0-9][0-9][0-9][0-9]/* -whitespace
> /t4135/* eol=lf
> /t4211/* eol=lf
> /t4252/* eol=lf
> +/t4256/1/* eol=lf
> /t5100/* eol=lf
> /t5515/* eol=lf
> /t556x_common eol=lf
^ 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