* Re: git 2.10.1 test regression in t3700-add.sh
From: Johannes Sixt @ 2016-10-10 19:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeremy Huddleston Sequoia, t.gummerer, git
In-Reply-To: <xmqq8ttwgkyo.fsf@gitster.mtv.corp.google.com>
Am 10.10.2016 um 19:41 schrieb Junio C Hamano:
> I also notice that the problematic test uses "chmod 755"; don't we
> need POSIXPERM prerequisite on this test, too, I wonder?
Good point. Without POSIXPERM the test demonstrate that, since chmod 755
is basically a noop, the following add --chmod=-x does not leave an x
bit in the index when there was none there in the first place. I think
it does not hurt to keep the test even though it does not quite test the
same thing as on POSIXPERM enabled systems.
-- Hannes
^ permalink raw reply
* Re: [PATCH] clean up confusing suggestion for commit references
From: Junio C Hamano @ 2016-10-10 19:14 UTC (permalink / raw)
To: Jeff King; +Cc: Heiko Voigt, git
In-Reply-To: <20161010182623.hsczlsfc6oaa2byj@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Mon, Oct 10, 2016 at 11:24:01AM -0700, Junio C Hamano wrote:
>
>> I no longer have preference either way myself, even though I was in
>> favor of no-quotes simply because I had an alias to produce that
>> format and was used to it.
>
> I'll admit that I don't care _that_ much and am happy to leave it up to
> individual authors, as long as nobody quotes SubmittingPatches at me as
> some kind of gospel when I use the no-quotes form.
;-).
I just do not want to hear "gitk (or was it git-gui) produces quoted
form, why are you recommending no-quoted form in SubmittingPatches?"
I'd say "use common sense; sometimes it is less confusing to read
without quotes and it is perfectly OK to do so if that is the case".
^ permalink raw reply
* Re: [PATCH] documentation: clarify submodule.<name>.[url, path] variables
From: Junio C Hamano @ 2016-10-10 19:10 UTC (permalink / raw)
To: Stefan Beller; +Cc: git@vger.kernel.org, Jens Lehmann, Heiko Voigt
In-Reply-To: <CAGZ79kZiKwOTiJiw4X3uLit2LrBRe_Y1oVn0-HJT-ey15D49Qw@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
>> Does it mean "never appears in .git/config, and when it appears it
>> will not be used at all"? If so we shouldn't even list it here.
>
> I meant:
> Git wont put it into .git/config on its own. If you really wanted to have
> it there, you need to do it yourself.
>
> I assumed you can overwrite the path via the config. It turns out,
> the value from the config is ignored, so it doesn't even make sense
> to put it into the config. Only the value from the .gitmodules file counts.
>
> So with that knowledge, I'll just remove it here, but I'll cross check if it
> is referenced else where enough.
Good. I do recall we designed this part of the system in such a way
that .path is conceptually uncostimizable (it _is_ part of what the
containing tree already records), and was wondering why it is there
as if it can be overridden.
^ permalink raw reply
* Re: How to watch a mailing list & repo for patches which affect a certain area of code?
From: Stefan Beller @ 2016-10-10 19:08 UTC (permalink / raw)
To: Stefan Beller, git@vger.kernel.org, Ian Kelling, Xiaolong Ye
In-Reply-To: <AB0A757A7BE241B39C8193A633C61FED@black7>
On Mon, Oct 10, 2016 at 11:56 AM, Jason Pyeron <jpyeron@pdinc.us> wrote:
>> -----Original Message-----
>> From: Stefan Beller
>> Sent: Monday, October 10, 2016 14:43
>>
>> +cc Xiaolong Ye <xiaolong.ye@intel.com>
>>
>> On Sun, Oct 9, 2016 at 2:26 PM, Jason Pyeron <jpyeron@pdinc.us> wrote:
>> >> -----Original Message-----
>> >> From: Ian Kelling
>> >> Sent: Sunday, October 09, 2016 15:03
>> >>
>> >> I've got patches in various projects, and I don't have
>> time to keep up
>> >> with the mailing list, but I'd like to help out with
>> >> maintenance of that
>> >> code, or the functions/files it touches. People don't cc me.
>> >> I figure I
>> >> could filter the list, test patches submitted, commits made,
>> >> mentions of
>> >> files/functions, build filters based on the code I have in
>> >> the repo even
>> >> if it's been moved or changed subsequently. I'm wondering
>> what other
>> >> people have implemented already for automation around
>> this, or general
>> >> thoughts. Web search is not showing me much.
>> >>
>> >
>> > One thought would be to apply every patch automatically (to
>> the branches of interest?). Then trigger on the [successful] changed
>> > code. This would simplify the logic to working on the
>> source only and not parsing the emails.
>> >
>> > -Jason
>> >
>>
>> I think this is currently attempted by some kernel people.
>> However it is very hard to tell where to apply a patch, as it
>
> This is one of the reasons why I use bundles instead of format patch.
Oh! That sounds interesting for solving the problem where to apply
a change, but the big disadvantage of bundles to patches is the inability
to just comment on it with an inline response. So I assume you follow
a different workflow than git or the kernel do. Which project do you use it
for and do you have some documentation/blog that explains that workflow?
>
>> is not formalized.
>> See the series that was merged at 72ce3ff7b51c
>> ('xy/format-patch-base'),
>> which adds a footer to the patch, that tells you where
>> exactly a patch ought
>> to be applied.
>
> Cant wait for that.
Well it is found in 2.9 and later. Currently the base footer is
opt-in, e.g. you'd
need to convince people to run `git config format.useAutoBase true` or to
manually add the base to the patch via `format-patch --base=<commit>`.
>
>>
>> The intention behind that series was to have some CI system hooked up
>> and report failures to the mailing list as well IIUC. Maybe
>> that helps with
>> your use case, too?
>
> I envisioned that it would try for each head he was interested in.
>
Well the test system can be smart enough to differentiate between:
* the patch you sent did not even compile on your base, so why
are you sending bogus patches?
* the patch you sent was fine as you sent it, but in the mean time
the target head progressed, and it doesn't compile/test any more.
collaboration is hard.
* or an extension to the prior point: this patch is fine but is broken
by the series xyz that is also in flight, please coordinate with
name@email.
^ permalink raw reply
* Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd
From: Junio C Hamano @ 2016-10-10 19:07 UTC (permalink / raw)
To: Jeff King
Cc: Duy Nguyen, Johannes Schindelin, Johannes Sixt, Git Mailing List
In-Reply-To: <20161010182129.fta7j7m3lxcgsn2d@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> ... My main motive in switching to the
> "alias.$cmd.key" syntax is that it fixes the ancient mistake of putting
> arbitrary content into the key (just like pager.*, as we've discussed
> elsewhere).
Yup, we are on the same page. It's not too grave a mistake (we said
"these are command names and do not have to be able to contain
random letters" and deliberately did without the usual three-level
hierarchy; I think the reasoning is even more valid for pager.*),
but it does imply case insensitivity not to use three-level names,
and I think it is a good move.
^ permalink raw reply
* Re: [PATCH] pretty: respect color settings for %C placeholders
From: Jeff King @ 2016-10-10 18:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Duy Nguyen, René Scharfe, Tom Hale, git
In-Reply-To: <xmqq1szof34x.fsf@gitster.mtv.corp.google.com>
On Mon, Oct 10, 2016 at 11:52:14AM -0700, Junio C Hamano wrote:
> > + By default, colors are shown only when enabled for log output (by
> > + `color.diff`, `color.ui`, or `--color`, and respecting the `auto`
> > + settings of the former if we are going to a terminal). `%C(auto,...)`
> > + is accepted as a historical synonym for the default. Specifying
> > + `%C(always,...) will show the colors always, even when colors are not
> > + otherwise enabled (to enable this behavior for the whole format, use
> > + `--color=always`).
>
> It is not just "for the whole format", but also affects other parts
> of the output, no? I am thinking about "git log -p --format=...".
True. I consider that a feature and more likely to be pointing the user
in the right direction, but it should probably be s/format/output/ in
the last sentence.
> > diff --git a/pretty.c b/pretty.c
> > index 25efbca..73e58b5 100644
> > --- a/pretty.c
> > +++ b/pretty.c
> > @@ -965,22 +965,31 @@ static size_t parse_color(struct strbuf *sb, /* in UTF-8 */
> >
> > if (!end)
> > return 0;
> > - if (skip_prefix(begin, "auto,", &begin)) {
> > +
> > + if (!skip_prefix(begin, "always,", &begin)) {
> > if (!want_color(c->pretty_ctx->color))
> > return end - placeholder + 1;
> > }
>
> As a way to say "when color is not enabled, ignore everything unless
> it begins with 'always,'", this was a bit hard to read. Perhaps an
> in-code comment is in order?
I'll see what I can do. The most readable one is probably:
if (skip_prefix(begin, "auto,", &begin)) {
if (!want_color(c->pretty_ctx->color))
return end - placeholder + 1;
} else if (skip_prefix(begin, "always,", &begin)) {
/* nothing to do; we do not respect want_color at all */
} else {
/* the default is now the same as "auto" */
if (!want_color(c->pretty_ctx->color))
return end - placeholder + 1;
}
I avoided that because of the repetition, but it probably is not too
bad.
> > - if (skip_prefix(placeholder + 1, "red", &rest))
> > + if (skip_prefix(placeholder + 1, "red", &rest) &&
> > + want_color(c->pretty_ctx->color))
> > strbuf_addstr(sb, GIT_COLOR_RED);
>
> Hmm. If we are in "no I do not want color" mode and "always,red"
> was given, we countermanded !want_color() check up above and come
> here. Then we check want_color() again and refuse to paint it red?
>
> I must be reading the patch incorrectly, but I cannot quite tell
> where I want astray...
No, we don't come here from %C() at all. This is for bare "%Cred", which
cannot have "always" (or "auto"), as there is no syntactic spot for it.
It is mostly historical (it _only_ supports red, green, and blue). We
could actually leave this as-is to show the colors unconditionally. I
changed it to keep the new behavior consistent, but I doubt anybody
cares much either way.
-Peff
^ permalink raw reply
* RE: How to watch a mailing list & repo for patches which affect a certain area of code?
From: Jason Pyeron @ 2016-10-10 18:56 UTC (permalink / raw)
To: git; +Cc: 'Ian Kelling', 'Stefan Beller',
'Xiaolong Ye'
In-Reply-To: <CAGZ79kaHOBaVCsVPen-K_5LyitcDzVbjL_kAqMkYKk_fQxH4PQ@mail.gmail.com>
> -----Original Message-----
> From: Stefan Beller
> Sent: Monday, October 10, 2016 14:43
>
> +cc Xiaolong Ye <xiaolong.ye@intel.com>
>
> On Sun, Oct 9, 2016 at 2:26 PM, Jason Pyeron <jpyeron@pdinc.us> wrote:
> >> -----Original Message-----
> >> From: Ian Kelling
> >> Sent: Sunday, October 09, 2016 15:03
> >>
> >> I've got patches in various projects, and I don't have
> time to keep up
> >> with the mailing list, but I'd like to help out with
> >> maintenance of that
> >> code, or the functions/files it touches. People don't cc me.
> >> I figure I
> >> could filter the list, test patches submitted, commits made,
> >> mentions of
> >> files/functions, build filters based on the code I have in
> >> the repo even
> >> if it's been moved or changed subsequently. I'm wondering
> what other
> >> people have implemented already for automation around
> this, or general
> >> thoughts. Web search is not showing me much.
> >>
> >
> > One thought would be to apply every patch automatically (to
> the branches of interest?). Then trigger on the [successful] changed
> > code. This would simplify the logic to working on the
> source only and not parsing the emails.
> >
> > -Jason
> >
>
> I think this is currently attempted by some kernel people.
> However it is very hard to tell where to apply a patch, as it
This is one of the reasons why I use bundles instead of format patch.
> is not formalized.
> See the series that was merged at 72ce3ff7b51c
> ('xy/format-patch-base'),
> which adds a footer to the patch, that tells you where
> exactly a patch ought
> to be applied.
Cant wait for that.
>
> The intention behind that series was to have some CI system hooked up
> and report failures to the mailing list as well IIUC. Maybe
> that helps with
> your use case, too?
I envisioned that it would try for each head he was interested in.
^ permalink raw reply
* Re: [PATCH] pretty: respect color settings for %C placeholders
From: Junio C Hamano @ 2016-10-10 18:52 UTC (permalink / raw)
To: Jeff King; +Cc: Duy Nguyen, René Scharfe, Tom Hale, git
In-Reply-To: <20161010151517.6wszhuyp57yfncaj@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> So here's what a patch to do that would look like. I admit that "I can't
> think of a good use" does not mean there _isn't_ one, but perhaps by
> posting this, it might provoke other people to think on it, too. And if
> nobody can come up with, maybe it's a good idea.
I do not have a fundamental opposition to this approach myself,
modulo a minor nit.
> + By default, colors are shown only when enabled for log output (by
> + `color.diff`, `color.ui`, or `--color`, and respecting the `auto`
> + settings of the former if we are going to a terminal). `%C(auto,...)`
> + is accepted as a historical synonym for the default. Specifying
> + `%C(always,...) will show the colors always, even when colors are not
> + otherwise enabled (to enable this behavior for the whole format, use
> + `--color=always`).
It is not just "for the whole format", but also affects other parts
of the output, no? I am thinking about "git log -p --format=...".
> diff --git a/pretty.c b/pretty.c
> index 25efbca..73e58b5 100644
> --- a/pretty.c
> +++ b/pretty.c
> @@ -965,22 +965,31 @@ static size_t parse_color(struct strbuf *sb, /* in UTF-8 */
>
> if (!end)
> return 0;
> - if (skip_prefix(begin, "auto,", &begin)) {
> +
> + if (!skip_prefix(begin, "always,", &begin)) {
> if (!want_color(c->pretty_ctx->color))
> return end - placeholder + 1;
> }
As a way to say "when color is not enabled, ignore everything unless
it begins with 'always,'", this was a bit hard to read. Perhaps an
in-code comment is in order?
> +
> + /* this is a historical noop */
> + skip_prefix(begin, "auto,", &begin);
> +
> if (color_parse_mem(begin, end - begin, color) < 0)
> die(_("unable to parse --pretty format"));
> strbuf_addstr(sb, color);
> return end - placeholder + 1;
> }
> - if (skip_prefix(placeholder + 1, "red", &rest))
> + if (skip_prefix(placeholder + 1, "red", &rest) &&
> + want_color(c->pretty_ctx->color))
> strbuf_addstr(sb, GIT_COLOR_RED);
Hmm. If we are in "no I do not want color" mode and "always,red"
was given, we countermanded !want_color() check up above and come
here. Then we check want_color() again and refuse to paint it red?
I must be reading the patch incorrectly, but I cannot quite tell
where I want astray...
^ permalink raw reply
* Re: How to watch a mailing list & repo for patches which affect a certain area of code?
From: Stefan Beller @ 2016-10-10 18:42 UTC (permalink / raw)
To: git@vger.kernel.org, Ian Kelling, Xiaolong Ye
In-Reply-To: <4B3747D8D2724E98B6AC000FE4072A09@black7>
+cc Xiaolong Ye <xiaolong.ye@intel.com>
On Sun, Oct 9, 2016 at 2:26 PM, Jason Pyeron <jpyeron@pdinc.us> wrote:
>> -----Original Message-----
>> From: Ian Kelling
>> Sent: Sunday, October 09, 2016 15:03
>>
>> I've got patches in various projects, and I don't have time to keep up
>> with the mailing list, but I'd like to help out with
>> maintenance of that
>> code, or the functions/files it touches. People don't cc me.
>> I figure I
>> could filter the list, test patches submitted, commits made,
>> mentions of
>> files/functions, build filters based on the code I have in
>> the repo even
>> if it's been moved or changed subsequently. I'm wondering what other
>> people have implemented already for automation around this, or general
>> thoughts. Web search is not showing me much.
>>
>
> One thought would be to apply every patch automatically (to the branches of interest?). Then trigger on the [successful] changed
> code. This would simplify the logic to working on the source only and not parsing the emails.
>
> -Jason
>
I think this is currently attempted by some kernel people.
However it is very hard to tell where to apply a patch, as it is not formalized.
See the series that was merged at 72ce3ff7b51c ('xy/format-patch-base'),
which adds a footer to the patch, that tells you where exactly a patch ought
to be applied.
The intention behind that series was to have some CI system hooked up
and report failures to the mailing list as well IIUC. Maybe that helps with
your use case, too?
^ permalink raw reply
* Re: Feature request: use relative path in worktree config files
From: Junio C Hamano @ 2016-10-10 18:36 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Stéphane Klein, Git Mailing List
In-Reply-To: <CACsJy8DGvyWz2_VBsEfmDWOPGUGAUhKYhCa1qLEMcOdn83ocSQ@mail.gmail.com>
Duy Nguyen <pclouds@gmail.com> writes:
>> I think there are some pros and some cons for relative path and absolute path.
>> Maybe append a "--relative" option with `git worktree add` ?
>>
>> I've converted all path to relative and all work with success.
>>
>> What do you think to append this --relative option.
>
> Patches are welcome.
Hmm, are they really welcome?
Is an invocation of "git worktree add" really the right point in the
workflow to decide if these references to other repositories should
be relative or absolute? When you are moving referrer, it is more
convenient if the reference uses absolute path to name the
referrent. When you are moving both referrer and referrent, it is
more convenient to use relative.
I somehow doubt that users know which future move they would be
making when doing "git worktree add".
To me, this almost looks like a need for a new subcommand to "git
worktree" that lets you move existing worktree to elsewhere, or turn
absolute reference to relative and vice versa.
^ permalink raw reply
* Re: [PATCH] documentation: clarify submodule.<name>.[url, path] variables
From: Stefan Beller @ 2016-10-10 18:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git@vger.kernel.org, Jens Lehmann, Heiko Voigt
In-Reply-To: <xmqqpon8f54i.fsf@gitster.mtv.corp.google.com>
On Mon, Oct 10, 2016 at 11:09 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>> submodule.<name>.path::
>> + The path within this project for a submodule. This variable is
>> + kept in the .gitmodules file. See linkgit:git-submodule[1] and
>> + linkgit:gitmodules[5] for details.
>
> What does it exactly mean to be "kept"?
>
> Does it mean "never appears in .git/config, and when it appears it
> will not be used at all"? If so we shouldn't even list it here.
I meant:
Git wont put it into .git/config on its own. If you really wanted to have
it there, you need to do it yourself.
I assumed you can overwrite the path via the config. It turns out,
the value from the config is ignored, so it doesn't even make sense
to put it into the config. Only the value from the .gitmodules file counts.
So with that knowledge, I'll just remove it here, but I'll cross check if it
is referenced else where enough.
>
> I doubt there is any reason for .path to exist in .git/config; where
> each submodule appears in the working tree is what is recorded in
> the tree object, and the "identity" (i.e. that which links a
> submodule in a tree to one of the repositories kept in
> .git/modules/*) by reverse look-up of submodule.<name>.path from
> in-tree .gitmodules, not from configuration, and it is not something
> a per-repository configuration should be able to change at the
> conceptual level.
As said above, currently we'd only rely on the .gitmodules file.
However I think this may conceptually the wrong thing to do.
The name is set in stone and ought to never be changed, unlike
the path. But as we only obtain this information from within the
repository at all times and not make a copy of it, upstream may
go crazy and change the names within the .gitmodules file.
Then the downstream user is left with a mess and they have no
way of fixing it except by modifying the data in the repository. So it
may be a sensible thing to allow the user to create their own
path -> name mapping in the config file.
We as the Git developers may even think about whether we'd
want to have the path -> name lookup all the time from upstream,
or rather on initialization want to make a copy of the path->name
mapping. Then we can never change the name locally, but we'd
need to track upstream for rename detection of submodule.
>
>> submodule.<name>.url::
>> - The path within this project and URL for a submodule. These
>> - variables are initially populated by 'git submodule init'. See
>> - linkgit:git-submodule[1] and linkgit:gitmodules[5] for
>> - details.
>> + The URL for a submodule. This variable is copied from the .gitmodules
>> + file to the git config via 'git submodule init'. The user can change
>> + the configured URL before obtaining the submodule via 'git submodule
>> + update'. After obtaining the submodule, this variable is kept in the
>> + config as a boolean flag to indicate whether the submodule is of
>> + interest to git commands. See linkgit:git-submodule[1] and
>> + linkgit:gitmodules[5] for details.
>
> I think it is great that you are describing that this serves two
> purposes, but "as a boolean flag" is very misleading. It sounds as
> if at some point "git submodule $something" command stores
> true/false there.
>
> - It overrides the URL suggested by the project in .gitmodules and
> replace it with another URL viewed from the local environment
> (e.g. the project may suggest you to use http://github.com/ while
> you may have a local mirror elsewhere).
>
> - Its presence is also used as a sign that the user is interested in
> the submodule.
I see, that makes it clearer, I'll reword this.
^ permalink raw reply
* Re: [PATCH] contrib: add credential helper for libsecret
From: Jeff King @ 2016-10-10 18:34 UTC (permalink / raw)
To: Mantas Mikulėnas; +Cc: git
In-Reply-To: <20161009123417.147239-1-grawity@gmail.com>
On Sun, Oct 09, 2016 at 03:34:17PM +0300, Mantas Mikulėnas wrote:
> This is based on the existing gnome-keyring helper, but instead of
> libgnome-keyring (which was specific to GNOME and is deprecated), it
> uses libsecret which can support other implementations of XDG Secret
> Service API.
Thanks for working on this; I'm happy to see credential helpers for as
many storage APIs as possible.
> Passes t0303-credential-external.sh.
Thank you for running that, as my first question would have been about
its results. :)
I don't know much about the Secret Service API, nor do I run a desktop
environment which provides the server side of it. But the code looks
straightforward and reasonable, it passes t0303, and presumably it is
working for you. So this seems worth merging to me.
-Peff
^ permalink raw reply
* Re: [PATCH v4 4/4] mergetool: honor -O<orderfile>
From: Junio C Hamano @ 2016-10-10 18:28 UTC (permalink / raw)
To: David Aguilar; +Cc: Git Mailing List, Johannes Sixt, Luis Gutierrez
In-Reply-To: <20161008000130.22858-1-davvid@gmail.com>
David Aguilar <davvid@gmail.com> writes:
> Teach mergetool to pass "-O<orderfile>" down to `git diff` when
> specified on the command-line.
>
> Helped-by: Johannes Sixt <j6t@kdbg.org>
> Signed-off-by: David Aguilar <davvid@gmail.com>
> ---
> Since v3:
>
> I missed one last piped invocation of "git mergetool" in the tests,
> which has been fixed.
I only see 4/4 in v4; am I correct to assume that 1-3/4 of v4 are
the same as their counterparts in v3?
^ permalink raw reply
* Re: [PATCH] clean up confusing suggestion for commit references
From: Jeff King @ 2016-10-10 18:26 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Heiko Voigt, git
In-Reply-To: <xmqqh98kf4fy.fsf@gitster.mtv.corp.google.com>
On Mon, Oct 10, 2016 at 11:24:01AM -0700, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > On Fri, Oct 07, 2016 at 11:56:38AM +0200, Heiko Voigt wrote:
> >
> >> The description for referencing commits looks as if it is contradicting
> >> the example, since it is itself enclosed in double quotes. Lets use
> >> single quotes around the description and include the double quotes in
> >> the description so it matches the example.
> >> ---
> >> Sorry for opening this up again but I just looked up the format and was
> >> like: "Umm, which one is now the correct one..."
> >>
> >> For this makes more sense. What do others think?
> >
> > Looking over the threads, I wasn't sure there was consensus[1,2]. So it would
> > be equally correct to drop the quotes from the example.
> >
> > I dunno. I am in favor of no-quotes, myself, so maybe I am just
> > manufacturing dissent in my mind. :)
>
> I no longer have preference either way myself, even though I was in
> favor of no-quotes simply because I had an alias to produce that
> format and was used to it.
I'll admit that I don't care _that_ much and am happy to leave it up to
individual authors, as long as nobody quotes SubmittingPatches at me as
some kind of gospel when I use the no-quotes form.
-Peff
^ permalink raw reply
* Re: [PATCH] clean up confusing suggestion for commit references
From: Junio C Hamano @ 2016-10-10 18:24 UTC (permalink / raw)
To: Jeff King; +Cc: Heiko Voigt, git
In-Reply-To: <20161007143200.qw77pdsymbdmjhbw@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Fri, Oct 07, 2016 at 11:56:38AM +0200, Heiko Voigt wrote:
>
>> The description for referencing commits looks as if it is contradicting
>> the example, since it is itself enclosed in double quotes. Lets use
>> single quotes around the description and include the double quotes in
>> the description so it matches the example.
>> ---
>> Sorry for opening this up again but I just looked up the format and was
>> like: "Umm, which one is now the correct one..."
>>
>> For this makes more sense. What do others think?
>
> Looking over the threads, I wasn't sure there was consensus[1,2]. So it would
> be equally correct to drop the quotes from the example.
>
> I dunno. I am in favor of no-quotes, myself, so maybe I am just
> manufacturing dissent in my mind. :)
I no longer have preference either way myself, even though I was in
favor of no-quotes simply because I had an alias to produce that
format and was used to it.
^ permalink raw reply
* Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd
From: Jeff King @ 2016-10-10 18:21 UTC (permalink / raw)
To: Junio C Hamano
Cc: Duy Nguyen, Johannes Schindelin, Johannes Sixt, Git Mailing List
In-Reply-To: <xmqqzimcf5wl.fsf@gitster.mtv.corp.google.com>
On Mon, Oct 10, 2016 at 10:52:26AM -0700, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > Having separate exec/shell boolean options just punts the overlap from
> > the command key to those keys. If you have two mutually exclusive
> > options, I think the best thing is a single option, like:
> >
> > type = <shell | exec | whatever>
> >
> > and then it is obvious that a second appearance of "type" overrides an
> > earlier one, by our usual "last one wins" convention. As opposed to:
> >
> > shell = true
> > exec = true
> >
> > where you have to understand the meaning of each option to know that
> > "exec" overrides "shell".
>
> Good.
>
> Duy's "do we want to chdir or stay?" would be an orthogonal axis to
> "what does the command line look like?" and "how is the command line
> run?" so it adds one member to the "alias.<string>.*" family of
> variables, I guess.
Yeah, exactly. There may be other orthogonal axes to add, too, but I do
not have any in mind myself. My main motive in switching to the
"alias.$cmd.key" syntax is that it fixes the ancient mistake of putting
arbitrary content into the key (just like pager.*, as we've discussed
elsewhere).
-Peff
^ permalink raw reply
* Re: git filter-branch --subdirectory-filter not working as expected, history of other folders is preserved
From: Jeff King @ 2016-10-10 18:19 UTC (permalink / raw)
To: Seaders Oloinsigh; +Cc: git
In-Reply-To: <CAN40BKoWroM4ZJHHdMvO8YvvCho0FnWzV9BG9E5xnCZbYOO2pg@mail.gmail.com>
On Mon, Oct 10, 2016 at 05:12:25PM +0100, Seaders Oloinsigh wrote:
> Due to the structure of this repo, it looks like there are some
> branches that never had anything to do with the android/ subdirectory,
> so they're not getting wiped out. My branch is in a better state to
> how I want it, but still, if I run your suggestion,
> [...]
Hmm. Yeah, I think this is an artifact of the way that filter-branch
works with pathspec limiting. It keeps a mapping of commits that it has
rewritten (including ones that were rewritten only because their
ancestors were), and realizes that a branch ref needs updated when the
commit it points to was rewritten.
But if we don't touch _any_ commits in the history reachable from a
branch (because they didn't even show up in our pathspec-limited
rev-list), then it doesn't realize we touched the branch's history at
all.
I agree that the right outcome is for it to delete those branches
entirely. I suspect the fix would be pretty tricky, though.
In the meantime, I think you can work around it by either:
1. Make a pass beforehand for refs that do not touch your desired
paths at all, like:
path=android ;# or whatever
git for-each-ref --format='%(refname)' |
while read ref; do
if test "$(git rev-list --count "$ref" -- "$path")" = 0; then
echo "delete $ref"
fi
done |
git update-ref --stdin
and then filter what's left:
git filter-branch --subdirectory-filter $path -- --all
or
2. Do the filter-branch, and because you know you specified --all and
that your filters would touch all histories, any ref which _wasn't_
touched can be deleted. That list is anything which didn't get a
backup entry in refs/original. So something like:
git for-each-ref --format='%(refname)' |
perl -lne 'print $1 if m{^refs/original/(.*)}' >backups
git for-each-ref --format='%(refname)' |
grep -v ^refs/original >refs
comm -23 refs backups |
sed "s/^/delete /" |
git update-ref --stdin
-Peff
^ permalink raw reply
* Re: [PATCH] clean up confusing suggestion for commit references
From: Junio C Hamano @ 2016-10-10 18:15 UTC (permalink / raw)
To: Heiko Voigt; +Cc: git
In-Reply-To: <20161007095638.GA55445@book.hvoigt.net>
Heiko Voigt <hvoigt@hvoigt.net> writes:
> If you want to reference a previous commit in the history of a stable
> -branch, use the format "abbreviated sha1 (subject, date)",
> +branch, use the format 'abbreviated sha1 ("subject", date)',
> with the subject enclosed in a pair of double-quotes, like this:
>
> Commit f86a374 ("pack-bitmap.c: fix a memleak", 2015-03-30)
As long as the "like this" example is there close to the sentence
"use the format ...", I do not think it matters either way in
practice, but I think this change is adding unnecessary confusion.
Both the 'subject' and 'date' on that line are meant to be
placeholders, so where you see subject, you replace it "with the
subject enclosed in a pair of double-quotes" as the next line says.
But then you would end up with:
Commit f86a374 (""pack-bitmap.c: fix a memleak"", 2015-03-30)
which is not what we want to see.
^ permalink raw reply
* Re: How to watch a mailing list & repo for patches which affect a certain area of code?
From: Jakub Narębski @ 2016-10-10 18:10 UTC (permalink / raw)
To: Ian Kelling, git
In-Reply-To: <1476039798.3060702.750483225.1DE6C48B@webmail.messagingengine.com>
W dniu 09.10.2016 o 21:03, Ian Kelling pisze:
> I've got patches in various projects, and I don't have time to keep up
> with the mailing list, but I'd like to help out with maintenance of that
> code, or the functions/files it touches. People don't cc me. I figure I
> could filter the list, test patches submitted, commits made, mentions of
> files/functions, build filters based on the code I have in the repo even
> if it's been moved or changed subsequently. I'm wondering what other
> people have implemented already for automation around this, or general
> thoughts. Web search is not showing me much.
First, the practice on this mailing list is to Cc all, and from what
I have seen people tend to do that (well, at least the regular participants).
Second, you can read this mailing list (and send emails / respond) with
a news reader via the NNTP interface (gmane or public-inbox). News readers
usually have good search capability.
Hope that helps
--
Jakub Narębski
^ permalink raw reply
* Re: [PATCH] documentation: clarify submodule.<name>.[url, path] variables
From: Junio C Hamano @ 2016-10-10 18:09 UTC (permalink / raw)
To: Stefan Beller; +Cc: git, Jens.Lehmann, hvoigt
In-Reply-To: <20161006235149.10232-1-sbeller@google.com>
Stefan Beller <sbeller@google.com> writes:
> submodule.<name>.path::
> + The path within this project for a submodule. This variable is
> + kept in the .gitmodules file. See linkgit:git-submodule[1] and
> + linkgit:gitmodules[5] for details.
What does it exactly mean to be "kept"?
Does it mean "never appears in .git/config, and when it appears it
will not be used at all"? If so we shouldn't even list it here.
I doubt there is any reason for .path to exist in .git/config; where
each submodule appears in the working tree is what is recorded in
the tree object, and the "identity" (i.e. that which links a
submodule in a tree to one of the repositories kept in
.git/modules/*) by reverse look-up of submodule.<name>.path from
in-tree .gitmodules, not from configuration, and it is not something
a per-repository configuration should be able to change at the
conceptual level.
> submodule.<name>.url::
> - The path within this project and URL for a submodule. These
> - variables are initially populated by 'git submodule init'. See
> - linkgit:git-submodule[1] and linkgit:gitmodules[5] for
> - details.
> + The URL for a submodule. This variable is copied from the .gitmodules
> + file to the git config via 'git submodule init'. The user can change
> + the configured URL before obtaining the submodule via 'git submodule
> + update'. After obtaining the submodule, this variable is kept in the
> + config as a boolean flag to indicate whether the submodule is of
> + interest to git commands. See linkgit:git-submodule[1] and
> + linkgit:gitmodules[5] for details.
I think it is great that you are describing that this serves two
purposes, but "as a boolean flag" is very misleading. It sounds as
if at some point "git submodule $something" command stores
true/false there.
- It overrides the URL suggested by the project in .gitmodules and
replace it with another URL viewed from the local environment
(e.g. the project may suggest you to use http://github.com/ while
you may have a local mirror elsewhere).
- Its presence is also used as a sign that the user is interested in
the submodule.
^ permalink raw reply
* Re: [PATCH v2] gpg-interface: use more status letters
From: Junio C Hamano @ 2016-10-10 17:58 UTC (permalink / raw)
To: Michael J Gruber; +Cc: Ramsay Jones, git, Alex
In-Reply-To: <8e496061-ce6c-450f-7c9e-785572d50b79@drmicha.warpmail.net>
Michael J Gruber <git@drmicha.warpmail.net> writes:
> Sorry, this got "lost in vacation". Before that, I was looking for an
> easy way to test expired signatures, but gpg1 and gpg2 behave somewhat
> differently in that respect (2 does not allow to create already expired
> signatures).
>
> Is there anything I should or could do now?
I dunno. It's your itch.
You can say "I'll need more time to figure out the way to test what
I am not testing here, so do not merge it to 'next' yet".
You can also say "This is good enough for now, so go ahead and merge
it to 'next'; more detailed tests can be done as follow-up patches
if needed".
You can also say "Thinking about it again, there is no strong reason
why we need to differentiate EXPSIG and EXPKEYSIG, so don't do this
SQUASH and use my original one as-is".
I'd be happy with any of the above and there may be other ones I'd
be happy with that I haven't thought of ;-)
^ permalink raw reply
* [PATCH 1/2] submodule: ignore trailing slash on superproject URL
From: Stefan Beller @ 2016-10-10 17:56 UTC (permalink / raw)
To: gitster; +Cc: git, venv21, dennis, Stefan Beller
Before 63e95beb0 (2016-04-15, submodule: port resolve_relative_url from
shell to C), it did not matter if the superprojects URL had a trailing
slash or not. It was just chopped off as one of the first steps
(The "remoteurl=${remoteurl%/}" near the beginning of
resolve_relative_url(), which was removed in said commit).
When porting this to the C version, an off-by-one error was introduced
and we did not check the actual last character to be a slash, but the
NULL delimiter.
Reintroduce the behavior from before 63e95beb0, to ignore the trailing
slash.
Reported-by: <venv21@gmail.com>
Helped-by: Dennis Kaarsemaker <dennis@kaarsemaker.net>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
builtin/submodule--helper.c | 6 ++++--
t/t0060-path-utils.sh | 1 +
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 444ec06..a7841a5 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -95,6 +95,8 @@ static int chop_last_dir(char **remoteurl, int is_relative)
* NEEDSWORK: This works incorrectly on the domain and protocol part.
* remote_url url outcome expectation
* http://a.com/b ../c http://a.com/c as is
+ * http://a.com/b/ ../c http://a.com/c same as previous line, but
+ * ignore trailing slash in url
* http://a.com/b ../../c http://c error out
* http://a.com/b ../../../c http:/c error out
* http://a.com/b ../../../../c http:c error out
@@ -113,8 +115,8 @@ static char *relative_url(const char *remote_url,
struct strbuf sb = STRBUF_INIT;
size_t len = strlen(remoteurl);
- if (is_dir_sep(remoteurl[len]))
- remoteurl[len] = '\0';
+ if (is_dir_sep(remoteurl[len-1]))
+ remoteurl[len-1] = '\0';
if (!url_is_local_not_ssh(remoteurl) || is_absolute_path(remoteurl))
is_relative = 0;
diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
index bf2deee..82b98f8 100755
--- a/t/t0060-path-utils.sh
+++ b/t/t0060-path-utils.sh
@@ -319,6 +319,7 @@ test_submodule_relative_url "../" "foo/bar" "../submodule" "../foo/submodule"
test_submodule_relative_url "../" "foo" "../submodule" "../submodule"
test_submodule_relative_url "(null)" "../foo/bar" "../sub/a/b/c" "../foo/sub/a/b/c"
+test_submodule_relative_url "(null)" "../foo/bar/" "../sub/a/b/c" "../foo/sub/a/b/c"
test_submodule_relative_url "(null)" "../foo/bar" "../submodule" "../foo/submodule"
test_submodule_relative_url "(null)" "../foo/submodule" "../submodule" "../foo/submodule"
test_submodule_relative_url "(null)" "../foo" "../submodule" "../submodule"
--
2.10.1.382.ga23ca1b.dirty
^ permalink raw reply related
* [PATCH 2/2] submodule: ignore trailing slash in relative url
From: Stefan Beller @ 2016-10-10 17:56 UTC (permalink / raw)
To: gitster; +Cc: git, venv21, dennis, Stefan Beller
In-Reply-To: <20161010175611.1058-1-sbeller@google.com>
This is similar to the previous patch, though no user reported a bug and
I could not find a regressive behavior.
However it is a good thing to be strict on the output and for that we
always omit a trailing slash.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
builtin/submodule--helper.c | 2 ++
t/t0060-path-utils.sh | 1 +
2 files changed, 3 insertions(+)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index a7841a5..260f46f 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -149,6 +149,8 @@ static char *relative_url(const char *remote_url,
}
strbuf_reset(&sb);
strbuf_addf(&sb, "%s%s%s", remoteurl, colonsep ? ":" : "/", url);
+ if (ends_with(url, "/"))
+ strbuf_setlen(&sb, sb.len - 1);
free(remoteurl);
if (starts_with_dot_slash(sb.buf))
diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
index 82b98f8..25b48e5 100755
--- a/t/t0060-path-utils.sh
+++ b/t/t0060-path-utils.sh
@@ -319,6 +319,7 @@ test_submodule_relative_url "../" "foo/bar" "../submodule" "../foo/submodule"
test_submodule_relative_url "../" "foo" "../submodule" "../submodule"
test_submodule_relative_url "(null)" "../foo/bar" "../sub/a/b/c" "../foo/sub/a/b/c"
+test_submodule_relative_url "(null)" "../foo/bar" "../sub/a/b/c/" "../foo/sub/a/b/c"
test_submodule_relative_url "(null)" "../foo/bar/" "../sub/a/b/c" "../foo/sub/a/b/c"
test_submodule_relative_url "(null)" "../foo/bar" "../submodule" "../foo/submodule"
test_submodule_relative_url "(null)" "../foo/submodule" "../submodule" "../foo/submodule"
--
2.10.1.382.ga23ca1b.dirty
^ permalink raw reply related
* Re: git merge deletes my changes
From: Paul Smith @ 2016-10-10 17:52 UTC (permalink / raw)
To: Eduard Egorov, 'git@vger.kernel.org'
In-Reply-To: <AM4PR03MB1636EA0DEB4C8095F04AB92ADBDB0@AM4PR03MB1636.eurprd03.prod.outlook.com>
On Mon, 2016-10-10 at 10:19 +0000, Eduard Egorov wrote:
> # ~/gitbuild/git-2.10.1/git merge -s subtree --squash ceph_ansible
>
> Can somebody confirm this please? Doesn't "merge -s subtree" really
> merges branches?
I think possibly you're not fully understanding what the --squash flag
does... that's what's causing your issue here, not the "-s" option.
A squash merge takes the commits that would be merged from the origin
branch and squashes them into a single patch and applies them to the
current branch as a new commit... but this new commit is not a merge
commit (that is, when you look at it with "git show" etc. the commit
will have only one parent, not two--or more--parents like a normal merge
commit).
Basically, it's syntactic sugar for a diff plus patch operation plus
some Git goodness wrapped around it to make it easier to use.
But ultimately once you're done, Git has no idea that this new commit
has any relationship whatsoever to the origin branch. So the next time
you merge, Git doesn't know that there was a previous merge and it will
try to merge everything from scratch rather than starting at the
previous common merge point.
So either you'll have to use a normal, non-squash merge, or else you'll
have to tell Git by hand what the previous common merge point was (as
Jeff King's excellent email suggests). Or else, you'll have to live
with this behavior.
^ permalink raw reply
* Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd
From: Junio C Hamano @ 2016-10-10 17:52 UTC (permalink / raw)
To: Jeff King
Cc: Duy Nguyen, Johannes Schindelin, Johannes Sixt, Git Mailing List
In-Reply-To: <20161009205854.byq2wqgemtmwudfb@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Having separate exec/shell boolean options just punts the overlap from
> the command key to those keys. If you have two mutually exclusive
> options, I think the best thing is a single option, like:
>
> type = <shell | exec | whatever>
>
> and then it is obvious that a second appearance of "type" overrides an
> earlier one, by our usual "last one wins" convention. As opposed to:
>
> shell = true
> exec = true
>
> where you have to understand the meaning of each option to know that
> "exec" overrides "shell".
Good.
Duy's "do we want to chdir or stay?" would be an orthogonal axis to
"what does the command line look like?" and "how is the command line
run?" so it adds one member to the "alias.<string>.*" family of
variables, I guess.
^ 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