* Re: Force Confirmation for Dropping Changed Lines
From: Junio C Hamano @ 2017-01-25 23:46 UTC (permalink / raw)
To: Jacob Keller; +Cc: Hilco Wijbenga, Git Users
In-Reply-To: <CA+P7+xo9WBwHjAXeUTn4bh=F6hvw1gA-79h-GmwQoeRpeLj2jQ@mail.gmail.com>
Jacob Keller <jacob.keller@gmail.com> writes:
>> Mmm, that sounds complex. The "my-code.x" is made up so I could keep
>> my example as simple as possible. In reality, it's Maven's POM files
>> (pom.xml).
>>
>> So there is no setting for any of this? There is no way to switch off
>> auto merging for certain files?
>
> Not really sure, but a quick google search revealed
> https://github.com/ralfth/pom-merge-driver
>
> Maybe that will help you?
Its readme seems to indicate that it is either solving a different
problem, or solving the same problem with the opposite goal in mind,
in that its goal seems to be to forcibly resolve what textually does
not resolve cleanly by choosing sides with an arbitrary policy, so
that humans do not have to get involved when they ordinarily would
have to.
Hilco's goal sounded to me the opposite---to force conflict even
when the two histories did what textually does resolve cleanly and
require humans to get involved even when they ordinarily wouldn't
have to.
^ permalink raw reply
* Re: [PATCHv2] submodule update: run custom update script for initial populating as well
From: Brandon Williams @ 2017-01-25 23:41 UTC (permalink / raw)
To: Stefan Beller; +Cc: gitster, git, judge.packham, olsonse
In-Reply-To: <20170125233742.30370-1-sbeller@google.com>
On 01/25, Stefan Beller wrote:
> In 1b4735d9f3 (submodule: no [--merge|--rebase] when newly cloned,
> 2011-02-17), all actions were defaulted to checkout for populating
> a submodule initially, because merging or rebasing makes no sense
> in that situation.
>
> Other commands however do make sense, such as the custom command
> that was added later (6cb5728c43, submodule update: allow custom
> command to update submodule working tree, 2013-07-03).
>
> I am unsure about the "none" command, as I can see an initial
> checkout there as a useful thing. On the other hand going strictly
> by our own documentation, we should do nothing in case of "none"
> as well, because the user asked for it.
>
> Reported-by: Han-Wen Nienhuys <hanwen@google.com>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> git-submodule.sh | 5 ++++-
> t/t7406-submodule-update.sh | 15 +++++++++++++++
> 2 files changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 9788175979..63fc4fe9bc 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -607,7 +607,10 @@ cmd_update()
> if test $just_cloned -eq 1
> then
> subsha1=
> - update_module=checkout
> + case "$update_module" in
> + merge | rebase | none)
> + update_module=checkout ;;
> + esac
> else
> subsha1=$(sanitize_submodule_env; cd "$sm_path" &&
> git rev-parse --verify HEAD) ||
> diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
> index 725bbed1f8..2f83243c7d 100755
> --- a/t/t7406-submodule-update.sh
> +++ b/t/t7406-submodule-update.sh
> @@ -441,6 +441,19 @@ test_expect_success 'submodule update - command in .git/config catches failure -
> test_i18ncmp actual expect
> '
>
> +test_expect_success 'submodule update - command run for initial population of submodule' '
> + cat <<-\ EOF >expect
> + Execution of '\''false $submodulesha1'\'' failed in submodule path '\''submodule'\''
> + EOF
> + (
> + cd super &&
> + rm -rf submodule
> + test_must_fail git submodule update >../actual
> + )
> + test_cmp expect actual
> + git -C super submodule update --checkout
> +'
You can probably get away without the subshell:
rm -rf super/submodule
test_must_fail git -C super submodule upsate >actual
> +
> cat << EOF >expect
> Execution of 'false $submodulesha1' failed in submodule path '../super/submodule'
> Failed to recurse into submodule path '../super'
> @@ -493,6 +506,7 @@ test_expect_success 'submodule init picks up merge' '
> '
>
> test_expect_success 'submodule update --merge - ignores --merge for new submodules' '
> + test_config -C super submodule.submodule.update checkout &&
> (cd super &&
> rm -rf submodule &&
> git submodule update submodule &&
> @@ -505,6 +519,7 @@ test_expect_success 'submodule update --merge - ignores --merge for new submod
> '
>
> test_expect_success 'submodule update --rebase - ignores --rebase for new submodules' '
> + test_config -C super submodule.submodule.update checkout &&
> (cd super &&
> rm -rf submodule &&
> git submodule update submodule &&
> --
> 2.11.0.495.g04f60290a0.dirty
>
--
Brandon Williams
^ permalink raw reply
* Re: [PATCH v2 0/7] Macros for Asciidoctor support
From: brian m. carlson @ 2017-01-25 23:41 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Schindelin, git
In-Reply-To: <20170125232959.zdbf3n3ey7qtnv7j@sigill.intra.peff.net>
[-- Attachment #1: Type: text/plain, Size: 1684 bytes --]
On Wed, Jan 25, 2017 at 06:30:00PM -0500, Jeff King wrote:
> On Wed, Jan 25, 2017 at 11:19:26PM +0000, brian m. carlson wrote:
>
> > On Wed, Jan 25, 2017 at 04:35:44PM -0500, Jeff King wrote:
> > > On Wed, Jan 25, 2017 at 02:28:55PM +0100, Johannes Schindelin wrote:
> > >
> > > > > The need for the extensions could be replaced with a small amount of
> > > > > Ruby code, if that's considered desirable. Previous opinions on doing
> > > > > so were negative, however.
> > > >
> > > > Quite frankly, it is annoying to be forced to install the extensions. I
> > > > would much rather have the small amount of Ruby code in Git's repository.
> > >
> > > Me too. Dependencies can be a big annoyance. I'd reserve judgement until
> > > I saw the actual Ruby code, though. :)
> >
> > I've sent the patch before, but I can send it again. It's relatively
> > small and self-contained. I'm also happy to be responsible for
> > maintaining it.
>
> Ah, it's:
>
> http://public-inbox.org/git/1413070656-241955-5-git-send-email-sandals@crustytoothpaste.net/
>
> (and note there is some surrounding discussion there).
>
> The code is not _too_ bad. The main thing is that it would have to be
> kept up to date with changes to asciidoc.conf's version of the linkgit
> macro. But it's not like that changes frequently.
Yes. I think I can actually simplify it some more, since we always seem
to use the argument to linkgit, so I'll send out a simplified patch in a
few minutes.
--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]
^ permalink raw reply
* Re: [PATCH] tag: add tag.createReflog option
From: Cornelius Weig @ 2017-01-25 23:40 UTC (permalink / raw)
To: Junio C Hamano, Jeff King; +Cc: git, novalis, pclouds
In-Reply-To: <xmqqy3xy7nq0.fsf@gitster.mtv.corp.google.com>
>
> It may have been obvious, but to be explicit for somebody new,
> !prefixcmp() corresponds to starts_with(). IOW, we changed the
> meaning of the return value when moving from cmp-lookalike (where 0
> means "equal") to "does it start with this string?" bool (where 1
> means "yes").
>
I see. It reads much better that way!
I re-did all the changes from Jeff's patch, but some tests are breaking
now. I will have to mend that tomorrow, because it's already too late in
my timezone.
Thanks a lot for your support m(_ _)m
^ permalink raw reply
* Re: [PATCHv3 3/3] submodule absorbing: fix worktree/gitdir pointers recursively for non-moves
From: Brandon Williams @ 2017-01-25 23:39 UTC (permalink / raw)
To: Stefan Beller; +Cc: gitster, git, peff
In-Reply-To: <20170125230450.4393-1-sbeller@google.com>
On 01/25, Stefan Beller wrote:
> Consider having a submodule 'sub' and a nested submodule at 'sub/nested'.
> When nested is already absorbed into sub, but sub is not absorbed into
> its superproject, then we need to fixup the gitfile and core.worktree
> setting for 'nested' when absorbing 'sub', but we do not need to move
> its git dir around.
>
> Previously 'nested's gitfile contained "gitdir: ../.git/modules/nested";
> it has to be corrected to "gitdir: ../../.git/modules/sub1/modules/nested".
>
> An alternative I considered to do this work lazily, i.e. when resolving
> "../.git/modules/nested", we would notice the ".git" being a gitfile
> linking to another path. That seemed to be robuster by design, but harder
> to get the implementation right. Maybe we have to do that anyway once we
> try to have submodules and worktrees working nicely together, but for now
> just produce 'correct' (i.e. direct) pointers.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
Looks good to me!
--
Brandon Williams
^ permalink raw reply
* [PATCHv2] submodule update: run custom update script for initial populating as well
From: Stefan Beller @ 2017-01-25 23:37 UTC (permalink / raw)
To: gitster; +Cc: git, bmwill, judge.packham, olsonse, Stefan Beller
In-Reply-To: <CAGZ79kbi-cweTe-ydcRYFFnGT4EmN+O1TZDiGYFyg8-Ex8abUw@mail.gmail.com>
In 1b4735d9f3 (submodule: no [--merge|--rebase] when newly cloned,
2011-02-17), all actions were defaulted to checkout for populating
a submodule initially, because merging or rebasing makes no sense
in that situation.
Other commands however do make sense, such as the custom command
that was added later (6cb5728c43, submodule update: allow custom
command to update submodule working tree, 2013-07-03).
I am unsure about the "none" command, as I can see an initial
checkout there as a useful thing. On the other hand going strictly
by our own documentation, we should do nothing in case of "none"
as well, because the user asked for it.
Reported-by: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
git-submodule.sh | 5 ++++-
t/t7406-submodule-update.sh | 15 +++++++++++++++
2 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index 9788175979..63fc4fe9bc 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -607,7 +607,10 @@ cmd_update()
if test $just_cloned -eq 1
then
subsha1=
- update_module=checkout
+ case "$update_module" in
+ merge | rebase | none)
+ update_module=checkout ;;
+ esac
else
subsha1=$(sanitize_submodule_env; cd "$sm_path" &&
git rev-parse --verify HEAD) ||
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index 725bbed1f8..2f83243c7d 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -441,6 +441,19 @@ test_expect_success 'submodule update - command in .git/config catches failure -
test_i18ncmp actual expect
'
+test_expect_success 'submodule update - command run for initial population of submodule' '
+ cat <<-\ EOF >expect
+ Execution of '\''false $submodulesha1'\'' failed in submodule path '\''submodule'\''
+ EOF
+ (
+ cd super &&
+ rm -rf submodule
+ test_must_fail git submodule update >../actual
+ )
+ test_cmp expect actual
+ git -C super submodule update --checkout
+'
+
cat << EOF >expect
Execution of 'false $submodulesha1' failed in submodule path '../super/submodule'
Failed to recurse into submodule path '../super'
@@ -493,6 +506,7 @@ test_expect_success 'submodule init picks up merge' '
'
test_expect_success 'submodule update --merge - ignores --merge for new submodules' '
+ test_config -C super submodule.submodule.update checkout &&
(cd super &&
rm -rf submodule &&
git submodule update submodule &&
@@ -505,6 +519,7 @@ test_expect_success 'submodule update --merge - ignores --merge for new submod
'
test_expect_success 'submodule update --rebase - ignores --rebase for new submodules' '
+ test_config -C super submodule.submodule.update checkout &&
(cd super &&
rm -rf submodule &&
git submodule update submodule &&
--
2.11.0.495.g04f60290a0.dirty
^ permalink raw reply related
* Re: [PATCH v2 0/7] Macros for Asciidoctor support
From: Jeff King @ 2017-01-25 23:30 UTC (permalink / raw)
To: brian m. carlson, Johannes Schindelin, git
In-Reply-To: <20170125231926.usufhlugjotjw5zw@genre.crustytoothpaste.net>
On Wed, Jan 25, 2017 at 11:19:26PM +0000, brian m. carlson wrote:
> On Wed, Jan 25, 2017 at 04:35:44PM -0500, Jeff King wrote:
> > On Wed, Jan 25, 2017 at 02:28:55PM +0100, Johannes Schindelin wrote:
> >
> > > > The need for the extensions could be replaced with a small amount of
> > > > Ruby code, if that's considered desirable. Previous opinions on doing
> > > > so were negative, however.
> > >
> > > Quite frankly, it is annoying to be forced to install the extensions. I
> > > would much rather have the small amount of Ruby code in Git's repository.
> >
> > Me too. Dependencies can be a big annoyance. I'd reserve judgement until
> > I saw the actual Ruby code, though. :)
>
> I've sent the patch before, but I can send it again. It's relatively
> small and self-contained. I'm also happy to be responsible for
> maintaining it.
Ah, it's:
http://public-inbox.org/git/1413070656-241955-5-git-send-email-sandals@crustytoothpaste.net/
(and note there is some surrounding discussion there).
The code is not _too_ bad. The main thing is that it would have to be
kept up to date with changes to asciidoc.conf's version of the linkgit
macro. But it's not like that changes frequently.
-Peff
^ permalink raw reply
* Re: [PATCH 4/5] revision.c: refactor ref selection handler after --exclude
From: Junio C Hamano @ 2017-01-25 23:25 UTC (permalink / raw)
To: Jacob Keller
Cc: Jeff King, Nguyễn Thái Ngọc Duy,
Git mailing list
In-Reply-To: <CA+P7+xovOx9ebo6MU0e4v==+76jtoMXz45+LnBPFifHbjqFU4w@mail.gmail.com>
Jacob Keller <jacob.keller@gmail.com> writes:
> On Wed, Jan 25, 2017 at 1:27 PM, Jeff King <peff@peff.net> wrote:
>> On Wed, Jan 25, 2017 at 03:57:18PM -0500, Jeff King wrote:
>>
>>> IOW, the ref-selector options build up until a group option is given,
>>> which acts on the built-up options (over that group) and then resets the
>>> built-up options. Doing "--unrelated" as above is orthogonal (though I
>>> think in practice nobody would do that, because it's hard to read).
>>
>> So here's what I would have expected your series to look more like (with
>> probably one patch adding clear_ref_selection_options, and the other
>> adding the decorate stuff):
>>
>
> I agree that this is how I would have expected it to work as well.
That makes three of us ;-)
^ permalink raw reply
* Re: [PATCH] connect: handle putty/plink also in GIT_SSH_COMMAND
From: Junio C Hamano @ 2017-01-25 23:25 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Schindelin, git, Segev Finer
In-Reply-To: <20170125224043.jxjc4avuy4ztnkm4@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Wed, Jan 25, 2017 at 02:35:36PM -0800, Junio C Hamano wrote:
>
>> -- >8 --
>> Subject: [PATCH] connect: core.sshvariant to correct misidentification
>
> I have been watching this discussion from the sidelines, and I agree
> that this direction is a big improvement.
> ...
> IIRC, the "const" in git_config_get_string_const is only about avoiding
> an annoying cast. The result is still allocated and needs freed. Since
> you are not keeping the value after the function returns, I think you
> could just use git_config_get_value().
It is too late for today's pushout (I have this topic near the tip
of 'pu', so it is possible to tweak and redo only 'pu' branch, but I
generally hate tweaking things after 15:00 my time), but I'll fix
that before the topic hits 'jch' (which is a bit more than 'next'
and that is what I use for everyday work).
Thanks.
^ permalink raw reply
* Re: [PATCH] Retire the `relink` command
From: Eric Wong @ 2017-01-25 23:20 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Junio C Hamano
In-Reply-To: <10319c47ff3f7222c3a601827ebd9398861d509d.1485363528.git.johannes.schindelin@gmx.de>
Johannes Schindelin <johannes.schindelin@gmx.de> wrote:
> Back in the olden days, when all objects were loose and rubber boots were
> made out of wood, it made sense to try to share (immutable) objects
> between repositories.
>
> Ever since the arrival of pack files, it is but an anachronism.
>
> Let's move the script to the contrib/examples/ directory and no longer
> offer it.
On the other hand, we have no idea if there are still people
using it for whatever reason...
I suggest we have a deprecation period where:
1) there is warning message when it's run
2) a note in the manpage indicating it's to-be-removed
3) ...then wait a few distro LTS cycles to remove it entirely
^ permalink raw reply
* Re: diff --color-words breaks spacing
From: Jeff King @ 2017-01-25 23:20 UTC (permalink / raw)
To: Phil Hord; +Cc: Git
In-Reply-To: <CABURp0ryBtLALEnzRdoeYeUUdrzBxiT3+yTvW6B=vpqMG3p1Rw@mail.gmail.com>
On Tue, Jan 24, 2017 at 09:39:31AM -0800, Phil Hord wrote:
> I noticed some weird spacing when comparing files with git diff
> --color-words. The space before a colored word disappears sometimes.
I _think_ this is working as designed, though it is a bit tricky (and it
may be possible to make it better).
> echo "FOO foo; foo = bar" > a
> echo "FOO foo = baz" > b
> git diff --color-words --no-index a b
> FOOfoo; foo = barbaz
It might be easier to see with --word-diff, which uses the same code but
marks it with punctuation:
$ git diff --word-diff
FOO[-foo;-] foo = [-bar-]{+baz+}
The key thing is that what you are seeing is the post-image, plus
word-based annotations. And the post-image does not have that space in
its context, so we would not expect to see:
FOO [-foo;-] foo = [-bar-]{+baz+}
(you would if we showed the pre-image plus annotations; but then I think
you'd probably end up with the opposite problem when text was added).
However, we also don't see the space in the removed part. I.e., I think:
FOO[- foo;-] foo = [-bar-]{+baz+}
would be correct. But I think because of the way word-diff is
implemented, a non-word character will never be part of the annotation.
The way it works is basically to break the string down on word
boundaries, so we have:
pre: FOO, foo;, foo, =, bar
post: FOO, foo, =, baz
as a set of tokens. We stick each of those on their own line and feed
them back to xdiff, so we get a diff like:
@@ -1,5 +1,4 @@
FOO
-foo;
foo
=
-bar
+baz
and then walk through the post-image buffer, either inserting chunks of
context from the original buffer, or the changed lines. But the changed
lines themselves do not include the non-word characters, so we have no
idea that a space went away.
-Peff
^ permalink raw reply
* Re: [PATCH v2 25/27] attr: store attribute stack in attr_check structure
From: Brandon Williams @ 2017-01-25 23:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, sbeller, pclouds
In-Reply-To: <20170125215426.GB83343@google.com>
On 01/25, Brandon Williams wrote:
> On 01/25, Junio C Hamano wrote:
> > Brandon Williams <bmwill@google.com> writes:
> >
> > >> In my mind, the value of having a constant check_attr is primarily
> > >> that it gives us a stable pointer to serve as a hashmap key,
> > >> i.e. the identifier for each call site, in a later iteration.
> > >
> > > We didn't really discuss this notion of having the pointer be a key into
> > > a hashmap, what sort of information are you envisioning being stored in
> > > this sort of hashmap?
> >
> > The "entries relevant to this attr_check() call, that is specific to
> > the <check_attr instance, the thread> tuple" (aka "what used to be
> > called the global attr_stack") we discussed would be the primary
> > example. A thread is likely be looping in a caller that has many
> > paths inside a directory, calling a function that has a call to
> > attr_check() for each path. Having something that can use to
> > identify the check_attr instance in a stable way, even when the
> > inner function is called and returns many times, would allow us to
> > populate the "attr stack" just once for the thread when it enters a
> > directory for the first time (remember, another thread may be
> > executing the same codepath, checking for paths in a different
> > directory) and keep using it. There may be other mechanisms you can
> > come up with, so I wouldn't say it is the only valid way, but it is
> > a way. That is why I said:
> >
> > >> But all of the above comes from my intuition, and I'll very much
> > >> welcome to be proven wrong with an alternative design, or better
> > >> yet, a working code based on an alternative design ;-).
> >
> > near the end of my message.
> >
> > > One issue I can see with this is that the
> > > functions which have a static attr_check struct would still not be thread
> > > safe if the initialization of the structure isn't surrounded by a mutex
> > > itself. ie
> >
> > Yes, that goes without saying. That is why I suggested Stefan to do
> > not this:
> >
> > > static struct attr_check *check;
> > >
> > > if (!check)
> > > init(check);
> > >
> > > would need to be:
> > >
> > > lock()
> > > if (!check)
> > > init(check);
> > > unlock();
> >
> > but this:
> >
> > static struct attr_check *check;
> > init(&check);
> >
> > and hide the lock/unlock gymnastics inside the API. I thought that
> > already was in what you inherited from him and started your work
> > on top of?
>
> I essentially built off of the series you had while using Stefan's
> patches as inspiration, but I don't believe the kind of mechanism you
> are describing existed in Stefan's series. His series had a single lock
> for the entire system, only allowing a single caller to be in it at any
> given time. This definitely isn't ideal, hence why I picked it up.
>
> Implementation aside I want to try and nail down what the purpose of
> this refactor is. There are roughly two notions of being "thread-safe".
>
> 1. The first is that the subsystem itself is thread safe, that is
> multiple threads can be executing inside the subsystem without stepping
> on each others work.
>
> 2. The second is that the object itself is thread safe or that multiple
> threads can use the same object.
>
> I thought that the main purpose of this was to achieve (1) since
> currently that is not the case.
Ok, so I discovered a very good reason why we should do as Stefan
originally did and split the question and answer (beyond the reasoning
for using the reference as a hashkey).
One motivation behind making this API thread-safe is that we can use it
in pathspec code to match against attributes. This means that a
pathspec structure will contain an attr_check member describing the
attributes that a pathspec item is interested in. Then the pathspec
structure is passed to match_pathspec() as a const pointer. To me, when
passing something as 'const' I expect none of the members should change
at all. The struct should remain exactly in the same form as before I
invoked the function.
Requiring the attr_check structure to be modified in the process of a
check_attrs() call would violate this "contract" when calling
match_pathspec() as the attr_check structure would have modified state.
The compiler wouldn't catch this as the "const" modifier isn't passed on
to struct members.
--
Brandon Williams
^ permalink raw reply
* Re: [PATCH v2 0/7] Macros for Asciidoctor support
From: brian m. carlson @ 2017-01-25 23:19 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Schindelin, git
In-Reply-To: <20170125213544.eelk4pjhrhshi6zh@sigill.intra.peff.net>
[-- Attachment #1: Type: text/plain, Size: 938 bytes --]
On Wed, Jan 25, 2017 at 04:35:44PM -0500, Jeff King wrote:
> On Wed, Jan 25, 2017 at 02:28:55PM +0100, Johannes Schindelin wrote:
>
> > > The need for the extensions could be replaced with a small amount of
> > > Ruby code, if that's considered desirable. Previous opinions on doing
> > > so were negative, however.
> >
> > Quite frankly, it is annoying to be forced to install the extensions. I
> > would much rather have the small amount of Ruby code in Git's repository.
>
> Me too. Dependencies can be a big annoyance. I'd reserve judgement until
> I saw the actual Ruby code, though. :)
I've sent the patch before, but I can send it again. It's relatively
small and self-contained. I'm also happy to be responsible for
maintaining it.
--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]
^ permalink raw reply
* [PATCHv3 3/3] submodule absorbing: fix worktree/gitdir pointers recursively for non-moves
From: Stefan Beller @ 2017-01-25 23:04 UTC (permalink / raw)
To: gitster; +Cc: git, bmwill, peff, Stefan Beller
In-Reply-To: <xmqq37g692da.fsf@gitster.mtv.corp.google.com>
Consider having a submodule 'sub' and a nested submodule at 'sub/nested'.
When nested is already absorbed into sub, but sub is not absorbed into
its superproject, then we need to fixup the gitfile and core.worktree
setting for 'nested' when absorbing 'sub', but we do not need to move
its git dir around.
Previously 'nested's gitfile contained "gitdir: ../.git/modules/nested";
it has to be corrected to "gitdir: ../../.git/modules/sub1/modules/nested".
An alternative I considered to do this work lazily, i.e. when resolving
"../.git/modules/nested", we would notice the ".git" being a gitfile
linking to another path. That seemed to be robuster by design, but harder
to get the implementation right. Maybe we have to do that anyway once we
try to have submodules and worktrees working nicely together, but for now
just produce 'correct' (i.e. direct) pointers.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
This replaces the last patch of the series, containing Brandons SQUASH proposal
as well as the removal of the goto.
Thanks,
Stefan
submodule.c | 62 ++++++++++++++++++++++++++++----------
t/t7412-submodule-absorbgitdirs.sh | 27 +++++++++++++++++
2 files changed, 73 insertions(+), 16 deletions(-)
diff --git a/submodule.c b/submodule.c
index 4c4f033e8a..3b98766a6b 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1437,22 +1437,57 @@ void absorb_git_dir_into_superproject(const char *prefix,
const char *path,
unsigned flags)
{
- const char *sub_git_dir, *v;
- char *real_sub_git_dir = NULL, *real_common_git_dir = NULL;
+ int err_code;
+ const char *sub_git_dir;
struct strbuf gitdir = STRBUF_INIT;
-
strbuf_addf(&gitdir, "%s/.git", path);
- sub_git_dir = resolve_gitdir(gitdir.buf);
+ sub_git_dir = resolve_gitdir_gently(gitdir.buf, &err_code);
/* Not populated? */
- if (!sub_git_dir)
- goto out;
+ if (!sub_git_dir) {
+ char *real_new_git_dir;
+ const char *new_git_dir;
+ const struct submodule *sub;
+
+ if (err_code == READ_GITFILE_ERR_STAT_FAILED) {
+ /* unpopulated as expected */
+ strbuf_release(&gitdir);
+ return;
+ }
+
+ if (err_code != READ_GITFILE_ERR_NOT_A_REPO)
+ /* We don't know what broke here. */
+ read_gitfile_error_die(err_code, path, NULL);
+
+ /*
+ * Maybe populated, but no git directory was found?
+ * This can happen if the superproject is a submodule
+ * itself and was just absorbed. The absorption of the
+ * superproject did not rewrite the git file links yet,
+ * fix it now.
+ */
+ sub = submodule_from_path(null_sha1, path);
+ if (!sub)
+ die(_("could not lookup name for submodule '%s'"), path);
+ new_git_dir = git_path("modules/%s", sub->name);
+ if (safe_create_leading_directories_const(new_git_dir) < 0)
+ die(_("could not create directory '%s'"), new_git_dir);
+ real_new_git_dir = real_pathdup(new_git_dir);
+ connect_work_tree_and_git_dir(path, real_new_git_dir);
+
+ free(real_new_git_dir);
+ } else {
+ /* Is it already absorbed into the superprojects git dir? */
+ char *real_sub_git_dir = real_pathdup(sub_git_dir);
+ char *real_common_git_dir = real_pathdup(get_git_common_dir());
- /* Is it already absorbed into the superprojects git dir? */
- real_sub_git_dir = real_pathdup(sub_git_dir);
- real_common_git_dir = real_pathdup(get_git_common_dir());
- if (!skip_prefix(real_sub_git_dir, real_common_git_dir, &v))
- relocate_single_git_dir_into_superproject(prefix, path);
+ if (!starts_with(real_sub_git_dir, real_common_git_dir))
+ relocate_single_git_dir_into_superproject(prefix, path);
+
+ free(real_sub_git_dir);
+ free(real_common_git_dir);
+ }
+ strbuf_release(&gitdir);
if (flags & ABSORB_GITDIR_RECURSE_SUBMODULES) {
struct child_process cp = CHILD_PROCESS_INIT;
@@ -1478,9 +1513,4 @@ void absorb_git_dir_into_superproject(const char *prefix,
strbuf_release(&sb);
}
-
-out:
- strbuf_release(&gitdir);
- free(real_sub_git_dir);
- free(real_common_git_dir);
}
diff --git a/t/t7412-submodule-absorbgitdirs.sh b/t/t7412-submodule-absorbgitdirs.sh
index 1c47780e2b..e2bbb449b6 100755
--- a/t/t7412-submodule-absorbgitdirs.sh
+++ b/t/t7412-submodule-absorbgitdirs.sh
@@ -64,6 +64,33 @@ test_expect_success 'absorb the git dir in a nested submodule' '
test_cmp expect.2 actual.2
'
+test_expect_success 're-setup nested submodule' '
+ # un-absorb the direct submodule, to test if the nested submodule
+ # is still correct (needs a rewrite of the gitfile only)
+ rm -rf sub1/.git &&
+ mv .git/modules/sub1 sub1/.git &&
+ GIT_WORK_TREE=. git -C sub1 config --unset core.worktree &&
+ # fixup the nested submodule
+ echo "gitdir: ../.git/modules/nested" >sub1/nested/.git &&
+ GIT_WORK_TREE=../../../nested git -C sub1/.git/modules/nested config \
+ core.worktree "../../../nested" &&
+ # make sure this re-setup is correct
+ git status --ignore-submodules=none
+'
+
+test_expect_success 'absorb the git dir in a nested submodule' '
+ git status >expect.1 &&
+ git -C sub1/nested rev-parse HEAD >expect.2 &&
+ git submodule absorbgitdirs &&
+ test -f sub1/.git &&
+ test -f sub1/nested/.git &&
+ test -d .git/modules/sub1/modules/nested &&
+ git status >actual.1 &&
+ git -C sub1/nested rev-parse HEAD >actual.2 &&
+ test_cmp expect.1 actual.1 &&
+ test_cmp expect.2 actual.2
+'
+
test_expect_success 'setup a gitlink with missing .gitmodules entry' '
git init sub2 &&
test_commit -C sub2 first &&
--
2.11.0.495.g04f60290a0.dirty
^ permalink raw reply related
* Re: [PATCH] gpg-interface: Add some output from gpg when it errors out.
From: Junio C Hamano @ 2017-01-25 23:04 UTC (permalink / raw)
To: Mike Hommey; +Cc: git
In-Reply-To: <20170125030434.26448-1-mh@glandium.org>
Mike Hommey <mh@glandium.org> writes:
> For instance, after changing my laptop for a new one, I copied my
> configs, but had some environment differences that broke gpg.
> With this change applied, the output becomes, on this new machine:
> gpg: keyblock resource '/usr/share/keyrings/debian-keyring.gpg': No
> such file or directory
> error: gpg failed to sign the data
> error: unable to sign the tag
>
> which makes it clearer what's wrong.
Overall I think this is a good thing to do. Instead of eating the
status output, showing what we got, especially when we know the
command failed, would make the bug-hunting of user's environment
easier, like you showed above.
The only thing in the design that makes me wonder is the filtering
out based on "[GNUPG:]" prefix. Why do we need to filter them out?
Implementation-wise, I'd be happier if we do not add any new
callsites of strbuf_split(), which is a horrible interface. But
that is a minor detail.
Thanks.
^ permalink raw reply
* Re: [PATCHv2 3/3] submodule absorbing: fix worktree/gitdir pointers recursively for non-moves
From: Stefan Beller @ 2017-01-25 23:04 UTC (permalink / raw)
To: Brandon Williams; +Cc: Junio C Hamano, git@vger.kernel.org, Jeff King
In-Reply-To: <20170125004610.GC58021@google.com>
On Tue, Jan 24, 2017 at 4:46 PM, Brandon Williams <bmwill@google.com> wrote:
> On 01/24, Stefan Beller wrote:
>> + connect_work_tree_and_git_dir(path, real_new_git_dir);
>
> Memory leak with 'real_new_git_dir'
yup. :(
> The scope of 'real_sub_git_dir' and 'real_common_git_dir' variable can
> be narrowed. Move their declaration here and free it prior to exiting
> the else block.
ok.
> 'v' isn't ever used, just use starts_with() and get rid of 'v'
makes sense.
>
>> + relocate_single_git_dir_into_superproject(prefix, path);
>> + }
>>
>
> There's a label 'out' at the end of the function which can be removed as
> there is no 'goto' statement using it.
We do use it in
if (err_code == READ_GITFILE_ERR_STAT_FAILED)
goto out; /* unpopulated as expected */
I took your proposed SQUASH and removed the goto, see the followup email.
^ permalink raw reply
* Re: [PATCH] tag: add tag.createReflog option
From: Junio C Hamano @ 2017-01-25 22:56 UTC (permalink / raw)
To: Jeff King; +Cc: Cornelius Weig, git, novalis, pclouds
In-Reply-To: <xmqqpoja95o5.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> Jeff King <peff@peff.net> writes:
>
>> +enum log_refs_config {
>> + LOG_REFS_UNSET = -1,
>> + LOG_REFS_NONE = 0,
>> + LOG_REFS_NORMAL, /* see should_create_reflog for rules */
>> + LOG_REFS_ALWAYS
>> +};
>> +extern enum log_refs_config log_all_ref_updates;
>> +...
>> +int should_create_reflog(const char *refname)
>> +{
>> + switch (log_all_ref_updates) {
>> + case LOG_REFS_ALWAYS:
>> + return 1;
>> + case LOG_REFS_NORMAL:
>> + return !prefixcmp(refname, "refs/heads/") ||
>> + !prefixcmp(refname, "refs/remotes/") ||
>> + !prefixcmp(refname, "refs/notes/") ||
>> + !strcmp(refname, "HEAD");
>> + default:
>> + return 0;
>> + }
>> +}
>
> Yup, this is how I expected for the feature to be done.
>
> Just a hint for Cornelius; prefixcmp() is an old name for what is
> called starts_with() these days.
It may have been obvious, but to be explicit for somebody new,
!prefixcmp() corresponds to starts_with(). IOW, we changed the
meaning of the return value when moving from cmp-lookalike (where 0
means "equal") to "does it start with this string?" bool (where 1
means "yes").
^ permalink raw reply
* Re: [PATCHv2 3/3] submodule absorbing: fix worktree/gitdir pointers recursively for non-moves
From: Junio C Hamano @ 2017-01-25 22:54 UTC (permalink / raw)
To: Stefan Beller; +Cc: git, bmwill, peff
In-Reply-To: <20170124235651.18749-4-sbeller@google.com>
Stefan Beller <sbeller@google.com> writes:
> Consider having a submodule 'sub' and a nested submodule at 'sub/nested'.
> When nested is already absorbed into sub, but sub is not absorbed into
> its superproject, then we need to fixup the gitfile and core.worktree
> setting for 'nested' when absorbing 'sub', but we do not need to move
> its git dir around.
>
> Previously 'nested's gitfile contained "gitdir: ../.git/modules/nested";
> it has to be corrected to "gitdir: ../../.git/modules/sub1/modules/nested".
That sounds like a sensible way to make things consistent.
^ permalink raw reply
* Re: Force Confirmation for Dropping Changed Lines
From: Jacob Keller @ 2017-01-25 22:54 UTC (permalink / raw)
To: Hilco Wijbenga; +Cc: Git Users
In-Reply-To: <CAE1pOi3eh7ao3NocV=PRFDby8y5ttjFR=-_VB0FoJv4MpjExaA@mail.gmail.com>
On Wed, Jan 25, 2017 at 2:51 PM, Hilco Wijbenga
<hilco.wijbenga@gmail.com> wrote:
> On 25 January 2017 at 14:24, Jacob Keller <jacob.keller@gmail.com> wrote:
>> On Wed, Jan 25, 2017 at 2:16 PM, Hilco Wijbenga
>> <hilco.wijbenga@gmail.com> wrote:
>>> How can I force Git to not assume my change to the first line is "redundant"?
>>>
>>
>> My guess is that you probably want a custom merge driver for your file
>> types. That's where I would look initially.
>
> Mmm, that sounds complex. The "my-code.x" is made up so I could keep
> my example as simple as possible. In reality, it's Maven's POM files
> (pom.xml).
>
> So there is no setting for any of this? There is no way to switch off
> auto merging for certain files?
Not really sure, but a quick google search revealed
https://github.com/ralfth/pom-merge-driver
Maybe that will help you?
Thanks,
Jake
^ permalink raw reply
* Re: Force Confirmation for Dropping Changed Lines
From: Hilco Wijbenga @ 2017-01-25 22:51 UTC (permalink / raw)
To: Jacob Keller; +Cc: Git Users
In-Reply-To: <CA+P7+xrupLuYAj7tn_1EaUiN6eaCmtgX-_d4mnByDq95cuqiWQ@mail.gmail.com>
On 25 January 2017 at 14:24, Jacob Keller <jacob.keller@gmail.com> wrote:
> On Wed, Jan 25, 2017 at 2:16 PM, Hilco Wijbenga
> <hilco.wijbenga@gmail.com> wrote:
>> How can I force Git to not assume my change to the first line is "redundant"?
>>
>
> My guess is that you probably want a custom merge driver for your file
> types. That's where I would look initially.
Mmm, that sounds complex. The "my-code.x" is made up so I could keep
my example as simple as possible. In reality, it's Maven's POM files
(pom.xml).
So there is no setting for any of this? There is no way to switch off
auto merging for certain files?
^ permalink raw reply
* Re: What's cooking in git.git (Jan 2017, #04; Mon, 23)
From: Junio C Hamano @ 2017-01-25 22:51 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Schindelin, Lars Schneider, git
In-Reply-To: <20170125183924.6yclcjl4ggcu42yp@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I guess the way to dig would be to add a test that looks at the output
> of "type mv" or something, push it to a Travis-hooked branch, and then
> wait for the output
Sounds tempting ;-)
^ permalink raw reply
* Re: [PATCH] connect: handle putty/plink also in GIT_SSH_COMMAND
From: Jeff King @ 2017-01-25 22:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, git, Segev Finer
In-Reply-To: <xmqqinp2939j.fsf@gitster.mtv.corp.google.com>
On Wed, Jan 25, 2017 at 02:35:36PM -0800, Junio C Hamano wrote:
> -- >8 --
> Subject: [PATCH] connect: core.sshvariant to correct misidentification
I have been watching this discussion from the sidelines, and I agree
that this direction is a big improvement.
> +static void override_ssh_variant(int *port_option, int *needs_batch)
> +{
> + const char *variant;
> +
> + if (git_config_get_string_const("core.sshvariant", &variant))
> + return;
> + if (!strcmp(variant, "tortoiseplink")) {
> + *port_option = 'P';
> + *needs_batch = 1;
> + } else if (!strcmp(variant, "putty")) {
> + *port_option = 'P';
> + *needs_batch = 0;
> + } else {
> + /* default */
> + if (strcmp(variant, "ssh")) {
> + warning(_("core.sshvariant: unknown value '%s'"), variant);
> + warning(_("using OpenSSH compatible behaviour"));
> + }
> + *port_option = 'p';
> + *needs_batch = 0;
> + }
> +}
IIRC, the "const" in git_config_get_string_const is only about avoiding
an annoying cast. The result is still allocated and needs freed. Since
you are not keeping the value after the function returns, I think you
could just use git_config_get_value().
(Grepping around, I see a few other places that seem to make the same
mistake. I think this is a confusing interface that should probably be
fixed).
-Peff
^ permalink raw reply
* Re: SubmittingPatches: drop temporal reference for PGP signing
From: Stefan Beller @ 2017-01-25 22:38 UTC (permalink / raw)
To: Philip Oakley
Cc: Cornelius Weig, Johannes Sixt, bitte.keine.werbung.einwerfen,
git@vger.kernel.org, Junio C Hamano, thomas.braun, John Keeping
In-Reply-To: <33E354BCDB9A4192B69B9B399381659E@PhilipOakley>
On Tue, Jan 24, 2017 at 10:54 PM, Philip Oakley <philipoakley@iee.org> wrote:
>> -Do not PGP sign your patch, at least for now. Most likely, your
>> -maintainer or other people on the list would not have your PGP
>> -key and would not bother obtaining it anyway. Your patch is not
>> -judged by who you are; a good patch from an unknown origin has a
>> -far better chance of being accepted than a patch from a known,
>> -respected origin that is done poorly or does incorrect things.
>> +Do not PGP sign your patch. Most likely, your maintainer or other
>> +people on the list would not have your PGP key and would not bother
>> +obtaining it anyway. Your patch is not judged by who you are; a good
>> +patch from an unknown origin has a far better chance of being accepted
>> +than a patch from a known, respected origin that is done poorly or
>> +does incorrect things.
>
>
> Wouldn't this also benefit from a forward reference to the section 5 on the
> DOC signining? This would avoid Cornelius's case where he felt that section
> 5 no longer applied.
Yeah I agree. My patch was not the best shot by far.
^ permalink raw reply
* Re: [PATCH] connect: handle putty/plink also in GIT_SSH_COMMAND
From: Junio C Hamano @ 2017-01-25 22:37 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Segev Finer
In-Reply-To: <xmqqinp2939j.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> Yes. Here is what comes on an obvious clean-up patch (which will be
> sent as a follow-up to this message).
... and this is the "obvious clean-up patch" to apply directly on
top of Segev's GIT_SSH_COMMAND support, which comes before the
previous one.
-- >8 --
Subject: [PATCH] connect: rename tortoiseplink and putty variables
One of these two may have originally been named after "what exact
SSH implementation do we have" so that we can tweak the command line
options, but these days "putty=1" no longer means "We are using the
plink SSH implementation that comes with PuTTY". It is set when we
guess that either PuTTY plink or Tortoiseplink is in use.
Rename them after what effect is desired. The current "putty"
option is about using "-P <port>" when OpenSSH would use "-p <port>",
so rename it to port_option whose value is either 'p' or 'P". The
other one is about passing an extra command line option "-batch",
so rename it needs_batch.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
connect.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/connect.c b/connect.c
index c81f77001b..aa51b33bfc 100644
--- a/connect.c
+++ b/connect.c
@@ -769,7 +769,8 @@ struct child_process *git_connect(int fd[2], const char *url,
conn->in = conn->out = -1;
if (protocol == PROTO_SSH) {
const char *ssh;
- int putty = 0, tortoiseplink = 0;
+ int needs_batch = 0;
+ int port_option = 'p';
char *ssh_host = hostandport;
const char *port = NULL;
char *ssh_argv0 = NULL;
@@ -819,12 +820,14 @@ struct child_process *git_connect(int fd[2], const char *url,
if (ssh_argv0) {
const char *base = basename(ssh_argv0);
- tortoiseplink = !strcasecmp(base, "tortoiseplink") ||
- !strcasecmp(base, "tortoiseplink.exe");
- putty = tortoiseplink ||
- !strcasecmp(base, "plink") ||
- !strcasecmp(base, "plink.exe");
-
+ if (!strcasecmp(base, "tortoiseplink") ||
+ !strcasecmp(base, "tortoiseplink.exe")) {
+ port_option = 'P';
+ needs_batch = 1;
+ } else if (!strcasecmp(base, "plink") ||
+ !strcasecmp(base, "plink.exe")) {
+ port_option = 'P';
+ }
free(ssh_argv0);
}
@@ -833,11 +836,10 @@ struct child_process *git_connect(int fd[2], const char *url,
argv_array_push(&conn->args, "-4");
else if (flags & CONNECT_IPV6)
argv_array_push(&conn->args, "-6");
- if (tortoiseplink)
+ if (needs_batch)
argv_array_push(&conn->args, "-batch");
if (port) {
- /* P is for PuTTY, p is for OpenSSH */
- argv_array_push(&conn->args, putty ? "-P" : "-p");
+ argv_array_pushf(&conn->args, "-%c", port_option);
argv_array_push(&conn->args, port);
}
argv_array_push(&conn->args, ssh_host);
--
2.11.0-699-ga1f1475476
^ permalink raw reply related
* Re: [PATCH] connect: handle putty/plink also in GIT_SSH_COMMAND
From: Junio C Hamano @ 2017-01-25 22:35 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Segev Finer
In-Reply-To: <alpine.DEB.2.20.1701251327090.3469@virtualbox>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> Now, with the patch in question (without the follow-up, which I would like
> to ask you to ignore, just like you did so far), Git would not figure out
> that your script calls PuTTY eventually. The work-around? Easy:
>
> DUMMY=/plink.exe /path/to/junio-is-a-superstar.sh
Think about how you would explain that to an end-user in our
document? You'll need to explain how exactly the auto-detection
works, so that the user can "exploit" the loophole to do that. And
what maintenance burden does it add when auto-detection is updated?
I think I know you well enough that you know well enough that it is
too ugly to live, and I suspect that the above is a tongue-in-cheek
"arguing for the sake of argument" and would not need a serious
response, but just in case...
> ...
> Of course, given our discussion I think all of this should be documented
> in the commit message as well as in the man page.
Yes. Here is what comes on an obvious clean-up patch (which will be
sent as a follow-up to this message).
-- >8 --
Subject: [PATCH] connect: core.sshvariant to correct misidentification
While the logic we have been using to guess which variant of SSH
implementation is in use by looking at the name of the program has
been robust enough for GIT_SSH (which does not go through the shell,
hence it can only spell the name the SSH program and nothing else),
extending it to GIT_SSH_COMMAND and core.sshcommand opens it for
larger chance of misidentification, because these can specify
arbitrary shell command, and a simple "the first word on the line
must be the command name" heuristic may not even look at the command
name at all.
As any effort to improve the heuristic will give us only diminishing
returns, and especially given that most of the users are expected to
have a command line for which the heuristic would work correctly,
give an explicit escape hatch to override a misidentification, just
in case one happens.
It is arguably bad to add this new variable to the core.* set, in
the sense that you'll never see this variable if you do not interact
with other repositories over ssh, but core.sshcommand is already
there for some reason, so let's match it.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/config.txt | 13 +++++++++++++
connect.c | 26 ++++++++++++++++++++++++++
2 files changed, 39 insertions(+)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index af2ae4cc02..8ea1db49c6 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -455,6 +455,19 @@ core.sshCommand::
the `GIT_SSH_COMMAND` environment variable and is overridden
when the environment variable is set.
+core.sshVariant::
+ SSH implementations used by Git when running `git fetch`,
+ `git clone`, and `git push` use slightly different command
+ line options (e.g. PuTTY and TortoisePlink use `-P <port>`
+ while OpenSSH uses `-p <port>` to specify the port number.
+ TortoisePlink in addition requires `-batch` option to be
+ passed). Git guesses which variant is in use correctly from
+ the name of the ssh command used (see `core.sshCommand`
+ configuration variable and also `GIT_SSH_COMMAND`
+ environment variable) most of the time. You can set this
+ variable to 'putty', 'tortoiseplink', or 'ssh' to correct it
+ when Git makes an incorrect guess.
+
core.ignoreStat::
If true, Git will avoid using lstat() calls to detect if files have
changed by setting the "assume-unchanged" bit for those tracked files
diff --git a/connect.c b/connect.c
index aa51b33bfc..358e9c06f0 100644
--- a/connect.c
+++ b/connect.c
@@ -691,6 +691,29 @@ static const char *get_ssh_command(void)
return NULL;
}
+static void override_ssh_variant(int *port_option, int *needs_batch)
+{
+ const char *variant;
+
+ if (git_config_get_string_const("core.sshvariant", &variant))
+ return;
+ if (!strcmp(variant, "tortoiseplink")) {
+ *port_option = 'P';
+ *needs_batch = 1;
+ } else if (!strcmp(variant, "putty")) {
+ *port_option = 'P';
+ *needs_batch = 0;
+ } else {
+ /* default */
+ if (strcmp(variant, "ssh")) {
+ warning(_("core.sshvariant: unknown value '%s'"), variant);
+ warning(_("using OpenSSH compatible behaviour"));
+ }
+ *port_option = 'p';
+ *needs_batch = 0;
+ }
+}
+
/*
* This returns a dummy child_process if the transport protocol does not
* need fork(2), or a struct child_process object if it does. Once done,
@@ -836,6 +859,9 @@ struct child_process *git_connect(int fd[2], const char *url,
argv_array_push(&conn->args, "-4");
else if (flags & CONNECT_IPV6)
argv_array_push(&conn->args, "-6");
+
+ override_ssh_variant(&port_option, &needs_batch);
+
if (needs_batch)
argv_array_push(&conn->args, "-batch");
if (port) {
--
2.11.0-699-ga1f1475476
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox