* Re: [PATCH v3 14/21] read-cache: touch shared index files when used
From: Junio C Hamano @ 2017-01-31 21:09 UTC (permalink / raw)
To: Christian Couder
Cc: Duy Nguyen, git, Ævar Arnfjörð Bjarmason,
Christian Couder
In-Reply-To: <xmqqy3xrqbkr.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> Yes, but you need to realize that "it is better not to bother users
> with a report of failure to touch in read-only repository" and "we
> ignore all failures".
Sorry about an unfinished sentence here. "need to realize that
... and ... are different things."
> ... It is very similar
> to a situation where you ... run "status".
> The command first runs the equivalent of "update-index --refresh"
> only in-core, and it attempts to write the updated index because
> (1) it paid the cost of doing the refreshing already, and (2) if it
> can write into the index, it will help future operations in the
> repository. But it does not report a failure to write the index
> exactly because it is merely doing an opportunistic write.
>
> And in the "we read from the split index, and we attempt to touch
> the underlying shared one to update its timestamp" case, it is OK
> not to report if we failed to touch.
> ...
> ... On the
> other hand, if you added new information, i.e. wrote the split index
> based on it, it is a good indication that the <split,shared> index
> pair has information that is more valuable. We must warn in that
> case.
This reminds us of a third case. What should happen if we are doing
the "opportunistic writing of the index" in "git status", managed to
write the split index, but failed to touch the shared one?
In the ideal world, I think we should do the following sequence:
- "status" tries to write cache to the file.
- we try to write and on any error, we return error to the caller,
who is already prepared to ignore it and stay silent.
- as the first step of writing the index, we first try to touch
the shared one. If it fails, we return an error here without
writing the split one out.
- then we try to write the split one out. If this fails, we
also return an error.
- otherwise, both touching of the shared one and writing of the
split one are successful.
- "status" finishes the opportunistic refreshing of the index, by
either ignoring an error silently (if either touching of shared
one or writing of split one fails) or writing the refreshed index
out successfully.
It is OK to swap the order of touching the shared one and writing of
the split one in the above sequence, as long as an error in either
step signals a failure to the opportunistic caller.
I do not offhand know if the split-index code is structured in such
a way to allow the above sequence easily, or it needs refactoring.
If such a restructuring is required, it might not be within the
scope of the series and I am OK if you just left the NEEDSWORK
comment that describes the above (i.e. what we should be doing) as
an in-code comment so that we can pick it up later. The whole point
of the step 14/21 on the other hand is to make sure that a shared
index that is still in active use will not go stale, and from that
point of view, such a "punting" may not be a good idea---it
deliberately finishes the series knowing that it does not adequately
do what it promises to do.
So, ... I dunno.
^ permalink raw reply
* Re: [PATCH v3 2/3] refs: add option core.logAllRefUpdates = always
From: Junio C Hamano @ 2017-01-31 22:02 UTC (permalink / raw)
To: Cornelius Weig; +Cc: Jeff King, git
In-Reply-To: <ce8f90a6-d719-63c7-95d0-b2538270e263@tngtech.com>
Cornelius Weig <cornelius.weig@tngtech.com> writes:
> On 01/31/2017 06:08 PM, Junio C Hamano wrote:
>> I think it is probably a good idea to document the behaviour
>> (i.e. "--no-create" single-shot from the command line is ignored).
>> I am not sure we should error out, though, in order to "disallow"
>> it---a documented silent no-op may be sufficient.
>
> Yes, maybe abort on seeing "--no-create-reflog" is a too drastic
> measure. I presume that the best place to have the documentation would
> be to print a warning when seeing the ignored argument?
>
> Or did you just have man pages and code comment in mind?
I meant only in the documentation, but "you gave me a no-op option"
warning would not hurt. I do not care too deeply either way.
^ permalink raw reply
* Re: git push failing when push.recurseSubmodules on-demand and git commit --amend was used in submodule.
From: Stefan Beller @ 2017-01-31 22:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Carlo Wood, Heiko Voigt, git@vger.kernel.org
In-Reply-To: <xmqqvasxwee1.fsf@gitster.mtv.corp.google.com>
On Sun, Jan 29, 2017 at 5:00 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Carlo Wood <carlo@alinoe.com> writes:
>
>> there seems to be a problem with using 'git commit --amend' in
>> git submodules when using 'git push --recurse-submodules=on-demand'
>> in the parent.
>>
>> The latter fails, saying "The following submodule paths contain changes
>> that can not be found on any remote:" for such submodule, even though
>> the submodule is clean, pushed and reports 'Everything up-to-date'
>> when trying to push it.
>>
>> I believe that the reason has to be that the parent repository thinks
>> that the comment that was amended, but not pushed, must be on the remote
>> too, while the whole point of amend is that this commit is not pushed.
>
> I am not super familiar with the actualy implementation of the
> codepaths involved in this, so CC'ed the folks who can help you
> better.
>
> I suspect the submodule folks would say it is working as intended,
> if \
>
> - you made a commit in the submodule;
> - recorded the resulting commit in the superproject;
> - you amended the commit in the submodule; and then
> - you did "push, while pushing out in the submodule as needed" from
> the superproject.
Yes, for the current state of affairs, this is it.
>
> There are two commits in the submodule that are involved in the
> above scenario, and the first one before amending is needed by the
> other participants of the project in order for them to check out
> what you are trying to push in the superproject, because that is
> what the superproject's tree records. You somehow need to make that
> commit available to them, but after you amended, the original commit
> may no longer be reachable from any branch in your submodule, so
> even if you (or the "on-demand" mechanism) pushed any and all
> branches out, that would not make the needed commit available to
> others. If you push your top-level superproject out in such a
> situation, you would break others.
In the long term future, we may want to reject non-fastforward
submodule updates. (Not sure if that is feasible)
>
> I think you have two options.
>
> 1. If the amend was done to improve things in submodule but is not
> quite ready, then get rid of that amended commit and restore the
> branch in the submodule to the state before you amended, i.e.
> the tip of the branch will become the same commit as the one
> that is recorded in the superproject. Then push the submodule
> and the superproject out. After that, move the submodule branch
> to point at the amended commit (or record the amended commit as
> a child of the commit you pushed out).
>
> 2. If the amend is good and ready to go, "git add" to update the
> superproject to make that amended result the one that is needed
> in the submodule.
yup.
^ permalink raw reply
* Re: gitconfig get out of sync with submodule entries on branch switch
From: Stefan Beller @ 2017-01-31 22:04 UTC (permalink / raw)
To: Benjamin Schindler; +Cc: Brandon Williams, git@vger.kernel.org
In-Reply-To: <b614a44a-fbc6-b5fe-ae40-ccf43dd9fcfb@gmail.com>
On Mon, Jan 30, 2017 at 11:46 PM, Benjamin Schindler
<beschindler@gmail.com> wrote:
> Hi Brandon
>
> I did try your suggestion, so basically:
>
> git checkout branch
> git submodule init
> git submodule update
Eventually this becomes
git submudule update --init
git checkout --recurse-submodules $branch
>
> Unfortunately, I still have two entries in my git config this way. It seems
> that git submodule update only considers submodules listed in .gitmodules.
Did you rename the name in the gitmodules file or rename the path on the FS?
>
> The background of my question is this - we have a jenkins farm which needs
> to switch branches continuously in an automated fashion and this needs to
> work even in when submodules are around. I did however, just now, find a
> reliable way to switch a branch, keeping the gitconfig in sync:
> The basic workflow for switching a branch is:
> git submodule deinit .
This will become 'git submodule deinit --all' eventually
> git checkout branch
> git submodule init
> git submodule update
This ought to update all the submodules, sounds fine to me.
>
> Because the .git folder of the submodules are not within the submodule
> directories, this is, while still quite heavy-handed, reasonably fast and
> robust. At least it is better than deleting the entire repository every time
> a branch switch is issued.
In the next version there will be 'git submodule absorbgitdirs'
which moves the git dirs of submodules inside the superproject; would
a reversion of this also be useful?
>
> Regards
>
> Benjamin Schindler
>
>
> On 30.01.2017 18:51, Brandon Williams wrote:
>>
>> On 01/30, Benjamin Schindler wrote:
>>>
>>> Hi
>>>
>>> Consider the following usecase: I have the master branch where I
>>> have a submodule A. I create a branch where I rename the submodule
>>> to be in the directory B. After doing all of this, everything looks
>>> good.
>>> Now, I switch back to master. The first oddity is, that it fails to
>>> remove the folder B because there are still files in there:
>>>
>>> bschindler@metis ~/Projects/submodule_test (testbranch) $ git
>>> checkout master
>>> warning: unable to rmdir other_submodule: Directory not empty
>>> Switched to branch 'master'
>>>
>>> Git submodule deinit on B fails because the submodule is not known
>>> to git anymore (after all, the folder B exists only in the other
>>> branch). I can easily just remove the folder B from disk and
>>> initialize the submodule A again, so all seems good.
>>>
>>> However, what is not good is that the submodule b is still known in
>>> .git/config. This is in particular a problem for us, because I know
>>> a number of tools which use git config to retrieve the submodule
>>> list. Is it therefore a bug that upon branch switch, the submodule
>>> gets deregistered, but its entry in .git/config remains?
>>>
>>> thanks a lot
>>> Benjamin Schindler
>>>
>>> P.s. I did not subscribe to the mailing list, please add me at least
>>> do CC. Thanks
>>
>> submodules and checkout don't really play nicely with each other at the
>> moment. Stefan (cc'd) is currently working on a patch series to improve
>> the behavior of checkout with submodules. Currently, if you want to
>> ensure you have a good working state after a checkout you should run
>> `git submodule update` to update all of the submoules. As far as your
>> submodule still being listed in the config, that should be expected
>> given the scenario you described.
>>
>> If I'm understanding you correctly, A and B are both the same submodule
>> just renamed on a different branch. The moment you add a submoule to a
>> repository it is given a name which is fixed. Typically this is the
>> path from the root of the repository. The thing is, since you are able
>> to freely move a submodule, its path can change. To account for this
>> there is the .gitmodules file which allows you to do a lookup from
>> submodule name to the path at which it exists (or vice versa). The
>> submodules that are stored in .git/config are those which are
>> 'initialize' or rather the submodules in which you are interested in and
>> will be updated by `git submodule update`. So given your scenario you
>> should only have a single submodule in .git/config and the .gitmodules
>> file should have a single entry with a differing path for each branch.
>>
>> Hopefully this gives you a bit more information to work with. Since
>> Stefan has been working with this more recently than me he may have some
>> more input.
>>
>
^ permalink raw reply
* Re: [PATCH 4/4] git-prompt.sh: add tests for submodule indicator
From: Stefan Beller @ 2017-01-31 22:12 UTC (permalink / raw)
To: Junio C Hamano
Cc: SZEDER Gábor, Benjamin Fuchs, git@vger.kernel.org,
brian m. carlson, ville.skytta
In-Reply-To: <xmqq37fyriji.fsf@gitster.mtv.corp.google.com>
On Tue, Jan 31, 2017 at 2:06 PM, Junio C Hamano <gitster@pobox.com> wrote:
> SZEDER Gábor <szeder.dev@gmail.com> writes:
>
>> On Mon, Jan 30, 2017 at 9:44 PM, Benjamin Fuchs <email@benjaminfuchs.de> wrote:
>>> Signed-off-by: Benjamin Fuchs <email@benjaminfuchs.de>
>>> ---
>>> t/t9903-bash-prompt.sh | 43 +++++++++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 43 insertions(+)
>>>
>>> diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
>>> index 97c9b32..4dce366 100755
>>> --- a/t/t9903-bash-prompt.sh
>>> +++ b/t/t9903-bash-prompt.sh
>>> @@ -37,6 +37,11 @@ test_expect_success 'setup for prompt tests' '
>>> git commit -m "yet another b2" file &&
>>> mkdir ignored_dir &&
>>> echo "ignored_dir/" >>.gitignore &&
>>> + git checkout -b submodule &&
>>> + git submodule add ./. sub &&
>>
>> ./. ?
>
> Good eyes. This is a pattern we are trying to wean ourselves off
> of. E.g. cf.
>
> https://public-inbox.org/git/20170105192904.1107-2-sbeller@google.com/#t
>
> Hopefully this reminds us to resurrect and finish the test fixes in
> that thread?
I plan to eventually, yes. but that is a refactoring, that has lower prio
than getting checkout working recursing into submodules.
^ permalink raw reply
* Re: [PATCH 4/4] git-prompt.sh: add tests for submodule indicator
From: Junio C Hamano @ 2017-01-31 22:06 UTC (permalink / raw)
To: SZEDER Gábor
Cc: Benjamin Fuchs, git, Stefan Beller, brian m. carlson,
ville.skytta
In-Reply-To: <CAM0VKj=j8Fy8AQvYbbvwPf5kkV1GYYONADNsQO5RDNTUzdYt8w@mail.gmail.com>
SZEDER Gábor <szeder.dev@gmail.com> writes:
> On Mon, Jan 30, 2017 at 9:44 PM, Benjamin Fuchs <email@benjaminfuchs.de> wrote:
>> Signed-off-by: Benjamin Fuchs <email@benjaminfuchs.de>
>> ---
>> t/t9903-bash-prompt.sh | 43 +++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 43 insertions(+)
>>
>> diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
>> index 97c9b32..4dce366 100755
>> --- a/t/t9903-bash-prompt.sh
>> +++ b/t/t9903-bash-prompt.sh
>> @@ -37,6 +37,11 @@ test_expect_success 'setup for prompt tests' '
>> git commit -m "yet another b2" file &&
>> mkdir ignored_dir &&
>> echo "ignored_dir/" >>.gitignore &&
>> + git checkout -b submodule &&
>> + git submodule add ./. sub &&
>
> ./. ?
Good eyes. This is a pattern we are trying to wean ourselves off
of. E.g. cf.
https://public-inbox.org/git/20170105192904.1107-2-sbeller@google.com/#t
Hopefully this reminds us to resurrect and finish the test fixes in
that thread?
^ permalink raw reply
* Re: [PATCH v2 7/7] completion: recognize more long-options
From: SZEDER Gábor @ 2017-01-31 22:17 UTC (permalink / raw)
To: Cornelius Weig; +Cc: j6t, spearce, git
In-Reply-To: <20170127211703.24910-2-cornelius.weig@tngtech.com>
On Fri, Jan 27, 2017 at 10:17 PM, <cornelius.weig@tngtech.com> wrote:
> From: Cornelius Weig <cornelius.weig@tngtech.com>
>
> Recognize several new long-options for bash completion in the following
> commands:
Adding more long options that git commands learn along the way is
always an improvement. However, seeing "_several_ new long options"
(or "some long options" in one of the other patches in the series)
makes the reader wonder: are these the only new long options missing
or are there more? If there are more, why only these are added? If
there aren't any more missing long options left, then please say so,
e.g. "Add all missing long options, except the potentially
desctructive ones, for the following commands: ...."
> - apply: --recount --directory=
> - archive: --output
> - branch: --column --no-column --sort= --points-at
> - clone: --no-single-branch --shallow-submodules
> - commit: --patch --short --date --allow-empty
> - describe: --first-parent
> - fetch, pull: --unshallow --update-shallow
> - fsck: --name-objects
> - grep: --break --heading --show-function --function-context
> --untracked --no-index
> - mergetool: --prompt --no-prompt
> - reset: --keep
> - revert: --strategy= --strategy-option=
> - rm: --force
'--force' is a potentially destructive option, too.
> - shortlog: --email
> - tag: --merged --no-merged --create-reflog
>
> Signed-off-by: Cornelius Weig <cornelius.weig@tngtech.com>
> Helped-by: Johannes Sixt <j6t@kdbg.org>
> ---
> contrib/completion/git-completion.bash | 31 +++++++++++++++++++++----------
> 1 file changed, 21 insertions(+), 10 deletions(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 0e09519..933bb6e 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -936,6 +936,7 @@ _git_apply ()
> --apply --no-add --exclude=
> --ignore-whitespace --ignore-space-change
> --whitespace= --inaccurate-eof --verbose
> + --recount --directory=
> "
> return
> esac
> @@ -974,7 +975,7 @@ _git_archive ()
> --*)
> __gitcomp "
> --format= --list --verbose
> - --prefix= --remote= --exec=
> + --prefix= --remote= --exec= --output
> "
> return
> ;;
> @@ -1029,6 +1030,7 @@ _git_branch ()
> --track --no-track --contains --merged --no-merged
> --set-upstream-to= --edit-description --list
> --unset-upstream --delete --move --remotes
> + --column --no-column --sort= --points-at
> "
> ;;
> *)
> @@ -1142,6 +1144,8 @@ _git_clone ()
> --single-branch
> --branch
> --recurse-submodules
> + --no-single-branch
> + --shallow-submodules
> "
> return
> ;;
> @@ -1183,6 +1187,7 @@ _git_commit ()
> --reset-author --file= --message= --template=
> --cleanup= --untracked-files --untracked-files=
> --verbose --quiet --fixup= --squash=
> + --patch --short --date --allow-empty
> "
> return
> esac
> @@ -1201,7 +1206,7 @@ _git_describe ()
> --*)
> __gitcomp "
> --all --tags --contains --abbrev= --candidates=
> - --exact-match --debug --long --match --always
> + --exact-match --debug --long --match --always --first-parent
> "
> return
> esac
> @@ -1284,6 +1289,7 @@ __git_fetch_recurse_submodules="yes on-demand no"
> __git_fetch_options="
> --quiet --verbose --append --upload-pack --force --keep --depth=
> --tags --no-tags --all --prune --dry-run --recurse-submodules=
> + --unshallow --update-shallow
> "
>
> _git_fetch ()
> @@ -1333,7 +1339,7 @@ _git_fsck ()
> --*)
> __gitcomp "
> --tags --root --unreachable --cache --no-reflogs --full
> - --strict --verbose --lost-found
> + --strict --verbose --lost-found --name-objects
> "
> return
> ;;
> @@ -1377,6 +1383,8 @@ _git_grep ()
> --max-depth
> --count
> --and --or --not --all-match
> + --break --heading --show-function --function-context
> + --untracked --no-index
> "
> return
> ;;
> @@ -1576,7 +1584,7 @@ _git_mergetool ()
> return
> ;;
> --*)
> - __gitcomp "--tool="
> + __gitcomp "--tool= --prompt --no-prompt"
> return
> ;;
> esac
> @@ -2456,7 +2464,7 @@ _git_reset ()
>
> case "$cur" in
> --*)
> - __gitcomp "--merge --mixed --hard --soft --patch"
> + __gitcomp "--merge --mixed --hard --soft --patch --keep"
> return
> ;;
> esac
> @@ -2472,7 +2480,10 @@ _git_revert ()
> fi
> case "$cur" in
> --*)
> - __gitcomp "--edit --mainline --no-edit --no-commit --signoff"
> + __gitcomp "
> + --edit --mainline --no-edit --no-commit --signoff
> + --strategy= --strategy-option=
> + "
> return
> ;;
> esac
> @@ -2483,7 +2494,7 @@ _git_rm ()
> {
> case "$cur" in
> --*)
> - __gitcomp "--cached --dry-run --ignore-unmatch --quiet"
> + __gitcomp "--cached --dry-run --ignore-unmatch --quiet --force"
> return
> ;;
> esac
> @@ -2500,7 +2511,7 @@ _git_shortlog ()
> __gitcomp "
> $__git_log_common_options
> $__git_log_shortlog_options
> - --numbered --summary
> + --numbered --summary --email
> "
> return
> ;;
> @@ -2778,8 +2789,8 @@ _git_tag ()
> --*)
> __gitcomp "
> --list --delete --verify --annotate --message --file
> - --sign --cleanup --local-user --force --column --sort
> - --contains --points-at
> + --sign --cleanup --local-user --force --column --sort=
> + --contains --points-at --merged --no-merged --create-reflog
> "
> ;;
> esac
> --
> 2.10.2
>
^ permalink raw reply
* Re: [PATCH 1/5] add SWAP macro
From: Junio C Hamano @ 2017-01-31 22:29 UTC (permalink / raw)
To: Jeff King
Cc: René Scharfe, Brandon Williams, Johannes Schindelin,
Johannes Sixt, Git List
In-Reply-To: <20170131213507.uiwmkkcg7umvd3f4@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> ... I wonder if it would be more natural for it to take
> pointers-to-objects, making it look more like a real function (i.e.,
> SWAP(&a, &b) instead of SWAP(a, b)". And then these funny corner cases
> become quite obvious in the caller, because the caller is the one who
> has to type "&".
Hmmmm.
While this looks very attractive in theory by forcing 'a' and 'b' to
be lvalues, it probably invites mistakes go unnoticed during the
review when the code wants to swap two pointer variables.
For example,
apply.c: SWAP(p->new_name, p->old_name);
is now a bug and will swap only the first byte of these names, and
the correct way to spell it would become:
apply.c: SWAP(&p->new_name, &p->old_name);
The latter clearly looks like swapping the new and old names, which
is good, but I do not have any confidence that I will immediately
spot a bug when presented the former under the new world order.
^ permalink raw reply
* Re: [PATCH 1/5] add SWAP macro
From: Jeff King @ 2017-01-31 22:36 UTC (permalink / raw)
To: Junio C Hamano
Cc: René Scharfe, Brandon Williams, Johannes Schindelin,
Johannes Sixt, Git List
In-Reply-To: <xmqqy3xqq2w0.fsf@gitster.mtv.corp.google.com>
On Tue, Jan 31, 2017 at 02:29:51PM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > ... I wonder if it would be more natural for it to take
> > pointers-to-objects, making it look more like a real function (i.e.,
> > SWAP(&a, &b) instead of SWAP(a, b)". And then these funny corner cases
> > become quite obvious in the caller, because the caller is the one who
> > has to type "&".
>
> Hmmmm.
>
> While this looks very attractive in theory by forcing 'a' and 'b' to
> be lvalues, it probably invites mistakes go unnoticed during the
> review when the code wants to swap two pointer variables.
>
> For example,
>
> apply.c: SWAP(p->new_name, p->old_name);
>
> is now a bug and will swap only the first byte of these names, and
> the correct way to spell it would become:
>
> apply.c: SWAP(&p->new_name, &p->old_name);
>
> The latter clearly looks like swapping the new and old names, which
> is good, but I do not have any confidence that I will immediately
> spot a bug when presented the former under the new world order.
Yes, it's a problem with any function (or function-like interface) that
takes an untyped pointer. You don't know if the caller meant the pointer
or the pointer-to-pointer. In that sense it's no different than:
memcpy(p->new_name, p->old_name, len);
Is that right, or did we mean to copy the pointers themselves?
It does also help the reverse case:
int a, b;
SWAP(a, b);
which would give an error (you forgot the "&"). I don't know which is
more likely to be productive. But at least requiring pointer values
makes it consistent with other functions like memcpy, etc.
-Peff
^ permalink raw reply
* Re: [PATCH 3/7] completion: improve bash completion for git-add
From: SZEDER Gábor @ 2017-01-31 22:42 UTC (permalink / raw)
To: Cornelius Weig
Cc: SZEDER Gábor, bitte.keine.werbung.einwerfen, thomas.braun,
john, git
In-Reply-To: <20170122225724.19360-1-cornelius.weig@tngtech.com>
> Add some long-options for git-add and improve path completion when the
> --update flag is given.
Please tell us in the commit message _how_ this improves path
completion.
> ---
> contrib/completion/git-completion.bash | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 8329f09..652c7e2 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -947,13 +947,17 @@ _git_add ()
> --*)
> __gitcomp "
> --interactive --refresh --patch --update --dry-run
> - --ignore-errors --intent-to-add
> + --ignore-errors --intent-to-add --force --edit --chmod=
I almost started complaining that '--force' should be used with care,
but then realized that for 'git add' it only means adding ignored
files. So in this particular case '--force' is not destructive and we
can offer it among other long options. Good.
> "
> return
> esac
>
> - # XXX should we check for --update and --all options ?
> - __git_complete_index_file "--others --modified --directory --no-empty-directory"
> + local complete_opt="--others --modified --directory --no-empty-directory"
> + if test -n "$(__git_find_on_cmdline "-u --update")"
> + then
> + complete_opt="--modified"
> + fi
> + __git_complete_index_file "$complete_opt"
> }
>
> _git_archive ()
> --
> 2.10.2
^ permalink raw reply
* What's cooking in git.git (Jan 2017, #06; Tue, 31)
From: Junio C Hamano @ 2017-01-31 22:45 UTC (permalink / raw)
To: git
What's cooking in git.git (Jan 2017, #06; Tue, 31)
--------------------------------------------------
Here are the topics that have been cooking. Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'. The ones marked with '.' do not appear in any of
the integration branches, but I am still holding onto them.
Two biggies from Dscho are now in 'master'. Another thanks goes to
him for pointing out that I had a wrong version of isatty() fix
earlier in 'maint' and 'master', which is now fixed by merging the
updated one in both of these branches. Please give the tip of
'maint' a quick look, as I want to tag and push out 2.11.1 later
this week, probably at the same time as 2.12-rc0 goes out.
You can find the changes described here in the integration branches
of the repositories listed at
http://git-blame.blogspot.com/p/git-public-repositories.html
--------------------------------------------------
[Graduated to "master"]
* bw/push-submodule-only (2016-12-20) 3 commits
(merged to 'next' on 2017-01-23 at d6cd1c60ae)
+ push: add option to push only submodules
+ submodules: add RECURSE_SUBMODULES_ONLY value
+ transport: reformat flag #defines to be more readable
"git submodule push" learned "--recurse-submodules=only option to
push submodules out without pushing the top-level superproject.
* jk/clear-delta-base-cache-fix (2017-01-19) 1 commit
(merged to 'next' on 2017-01-23 at 5f4af2b0a5)
+ clear_delta_base_cache(): don't modify hashmap while iterating
A crashing bug introduced in v2.11 timeframe has been found (it is
triggerable only in fast-import) and fixed.
* jk/coding-guidelines-update (2017-01-17) 1 commit
(merged to 'next' on 2017-01-23 at 8c57afa288)
+ CodingGuidelines: clarify multi-line brace style
Developer doc update.
* jk/fsck-connectivity-check-fix (2017-01-26) 9 commits
(merged to 'next' on 2017-01-26 at dd03f7a17f)
+ fsck: lazily load types under --connectivity-only
+ fsck: move typename() printing to its own function
(merged to 'next' on 2017-01-25 at f3d7d93785)
+ t1450: use "mv -f" within loose object directory
(merged to 'next' on 2017-01-23 at e8e9b76b84)
+ fsck: check HAS_OBJ more consistently
+ fsck: do not fallback "git fsck <bogus>" to "git fsck"
+ fsck: tighten error-checks of "git fsck <head>"
+ fsck: prepare dummy objects for --connectivity-check
+ fsck: report trees as dangling
+ t1450: clean up sub-objects in duplicate-entry test
"git fsck --connectivity-check" was not working at all.
* jk/loose-object-fsck (2017-01-15) 6 commits
(merged to 'next' on 2017-01-23 at 4302ad090d)
+ fsck: detect trailing garbage in all object types
+ fsck: parse loose object paths directly
+ sha1_file: add read_loose_object() function
+ t1450: test fsck of packed objects
+ sha1_file: fix error message for alternate objects
+ t1450: refactor loose-object removal
"git fsck" inspects loose objects more carefully now.
* jk/vreport-sanitize (2017-01-11) 2 commits
(merged to 'next' on 2017-01-18 at 4bbf370981)
+ vreport: sanitize ASCII control chars
+ Revert "vreportf: avoid intermediate buffer"
An error message with an ASCII control character like '\r' in it
can alter the message to hide its early part, which is problematic
when a remote side gives such an error message that the local side
will relay with a "remote: " prefix.
* js/difftool-builtin (2017-01-25) 4 commits
(merged to 'next' on 2017-01-25 at 87d2a0976a)
+ difftool: hack around -Wzero-length-format warning
(merged to 'next' on 2017-01-23 at 6f4810dbd9)
+ difftool: retire the scripted version
+ difftool: implement the functionality in the builtin
+ difftool: add a skeleton for the upcoming builtin
Rewrite a scripted porcelain "git difftool" in C.
* js/exec-path-coverity-workaround (2017-01-09) 2 commits
(merged to 'next' on 2017-01-23 at bf5dfbf860)
+ git_exec_path: do not return the result of getenv()
+ git_exec_path: avoid Coverity warning about unfree()d result
Code cleanup.
Split out of another topic.
* js/mingw-isatty (2017-01-18) 1 commit
(merged to 'next' on 2017-01-23 at ae0f80e058)
+ mingw: follow-up to "replace isatty() hack"
An update to a topic that is already in 'master'.
* js/remote-rename-with-half-configured-remote (2017-01-19) 2 commits
(merged to 'next' on 2017-01-23 at a1b655dbac)
+ remote rename: more carefully determine whether a remote is configured
+ remote rename: demonstrate a bogus "remote exists" bug
With anticipatory tweaking for remotes defined in ~/.gitconfig
(e.g. "remote.origin.prune" set to true, even though there may or
may not actually be "origin" remote defined in a particular Git
repository), "git remote rename" and other commands misinterpreted
and behaved as if such a non-existing remote actually existed.
* js/sequencer-i-countdown-3 (2017-01-17) 38 commits
(merged to 'next' on 2017-01-23 at 251dd15139)
+ sequencer (rebase -i): write out the final message
+ sequencer (rebase -i): write the progress into files
+ sequencer (rebase -i): show the progress
+ sequencer (rebase -i): suggest --edit-todo upon unknown command
+ sequencer (rebase -i): show only failed cherry-picks' output
+ sequencer (rebase -i): show only failed `git commit`'s output
+ sequencer: use run_command() directly
+ sequencer: update reading author-script
+ sequencer (rebase -i): differentiate between comments and 'noop'
+ sequencer (rebase -i): implement the 'drop' command
+ sequencer (rebase -i): allow rescheduling commands
+ sequencer (rebase -i): respect strategy/strategy_opts settings
+ sequencer (rebase -i): respect the rebase.autostash setting
+ sequencer (rebase -i): run the post-rewrite hook, if needed
+ sequencer (rebase -i): record interrupted commits in rewritten, too
+ sequencer (rebase -i): copy commit notes at end
+ sequencer (rebase -i): set the reflog message consistently
+ sequencer (rebase -i): refactor setting the reflog message
+ sequencer (rebase -i): allow fast-forwarding for edit/reword
+ sequencer (rebase -i): implement the 'reword' command
+ sequencer (rebase -i): leave a patch upon error
+ sequencer (rebase -i): update refs after a successful rebase
+ sequencer (rebase -i): the todo can be empty when continuing
+ sequencer (rebase -i): skip some revert/cherry-pick specific code path
+ sequencer (rebase -i): remove CHERRY_PICK_HEAD when no longer needed
+ sequencer (rebase -i): allow continuing with staged changes
+ sequencer (rebase -i): write an author-script file
+ sequencer (rebase -i): implement the short commands
+ sequencer (rebase -i): add support for the 'fixup' and 'squash' commands
+ sequencer (rebase -i): write the 'done' file
+ sequencer (rebase -i): learn about the 'verbose' mode
+ sequencer (rebase -i): implement the 'exec' command
+ sequencer (rebase -i): implement the 'edit' command
+ sequencer (rebase -i): implement the 'noop' command
+ sequencer: support a new action: 'interactive rebase'
+ sequencer: use a helper to find the commit message
+ sequencer: move "else" keyword onto the same line as preceding brace
+ sequencer: avoid unnecessary curly braces
The sequencer machinery has been further enhanced so that a later
set of patches can start using it to reimplement "rebase -i".
I think I've said everything that needs to be said on this topic.
* ls/travis-p4-on-macos (2017-01-23) 1 commit
(merged to 'next' on 2017-01-23 at 2d51987faa)
+ travis-ci: fix Perforce install on macOS
Update the definition of the MacOSX test environment used by
TravisCI.
* rs/qsort-s (2017-01-23) 5 commits
(merged to 'next' on 2017-01-23 at 7e2813848b)
+ ref-filter: use QSORT_S in ref_array_sort()
+ string-list: use QSORT_S in string_list_sort()
+ perf: add basic sort performance test
+ add QSORT_S
+ compat: add qsort_s()
A few codepaths had to rely on a global variable when sorting
elements of an array because sort(3) API does not allow extra data
to be passed to the comparison function. Use qsort_s() when
natively available, and a fallback implementation of it when not,
to eliminate the need, which is a prerequisite for making the
codepath reentrant.
* sb/in-core-index-doc (2017-01-19) 4 commits
(merged to 'next' on 2017-01-23 at 30224463e8)
+ documentation: retire unfinished documentation
+ cache.h: document add_[file_]to_index
+ cache.h: document remove_index_entry_at
+ cache.h: document index_name_pos
Documentation and in-code comments updates.
* sb/retire-convert-objects-from-contrib (2017-01-19) 1 commit
(merged to 'next' on 2017-01-23 at decc1e237d)
+ contrib: remove git-convert-objects
Remove an ancient tool left in contrib/.
* st/verify-tag (2017-01-18) 6 commits
(merged to 'next' on 2017-01-23 at 2810959427)
+ t/t7004-tag: Add --format specifier tests
+ t/t7030-verify-tag: Add --format specifier tests
+ builtin/tag: add --format argument for tag -v
+ builtin/verify-tag: add --format to verify-tag
+ ref-filter: add function to print single ref_array_item
+ gpg-interface, tag: add GPG_VERIFY_OMIT_STATUS flag
"git tag" and "git verify-tag" learned to put GPG verification
status in their "--format=<placeholders>" output format.
* vp/show-ref-verify-head (2017-01-23) 6 commits
(merged to 'next' on 2017-01-23 at af6dd9d239)
+ show-ref: remove a stale comment
+ show-ref: remove dead `if (verify)' check
+ show-ref: detect dangling refs under --verify as well
+ show-ref: move --quiet handling into show_one()
+ show-ref: allow -d to work with --verify
+ show-ref: accept HEAD with --verify
"git show-ref HEAD" used with "--verify" because the user is not
interested in seeing refs/remotes/origin/HEAD, and used with
"--head" because the user does not want HEAD to be filtered out,
i.e. "git show-ref --head --verify HEAD", did not work as expected.
--------------------------------------------------
[New Topics]
* cw/doc-sign-off (2017-01-27) 1 commit
- doc: clarify distinction between sign-off and pgp-signing
Doc update.
Will merge to 'next'.
* jk/delta-chain-limit (2017-01-27) 2 commits
- pack-objects: convert recursion to iteration in break_delta_chain()
- pack-objects: enforce --depth limit in reused deltas
"git repack --depth=<n>" for a long time busted the specified depth
when reusing delta from existing packs. This has been corrected.
Will merge to 'next'.
* js/re-running-failed-tests (2017-01-27) 1 commit
- t/Makefile: add a rule to re-run previously-failed tests
"make -C t failed" will now run only the tests that failed in the
previous run. This is usable only when prove is not use, and gives
a useless error message when run after "make clean".
Will merge to 'next'.
* js/unzip-in-usr-bin-workaround (2017-01-27) 1 commit
- test-lib: on FreeBSD, look for unzip(1) in /usr/local/bin/
Test tweak for FreeBSD where /usr/bin/unzip is unsuitable to run
our tests but /usr/local/bin/unzip is usable.
Will merge to 'next'.
* gv/mingw-p4-mapuser (2017-01-30) 1 commit
- git-p4: fix git-p4.mapUser on Windows
"git p4" did not work well with multiple git-p4.mapUser entries on
Windows.
Will merge to 'next'.
* hv/mingw-help-is-executable (2017-01-30) 1 commit
- help: improve is_executable() on Windows
"git help" enumerates executable files in $PATH; the implementation
of "is this file executable?" on Windows has been optimized.
Will merge to 'next'.
* cw/log-updates-for-all-refs-really (2017-01-31) 3 commits
- update-ref: add test cases for bare repository
- refs: add option core.logAllRefUpdates = always
- config: add markup to core.logAllRefUpdates doc
The "core.logAllRefUpdates" that used to be boolean has been
enhanced to take 'always' as well, to record ref updates to refs
other than the ones that are expected to be updated (i.e. branches,
remote-tracking branches and notes).
Will merge to 'next'.
* mm/merge-rename-delete-message (2017-01-30) 1 commit
- merge-recursive: make "CONFLICT (rename/delete)" message show both paths
* mm/reset-facl-before-umask-test (2017-01-30) 1 commit
- t0001: don't let a default ACL interfere with the umask test
Test tweaks for those who have default ACL in their git source tree
that interfere with the umask test.
Will merge to 'next'.
* rs/object-id (2017-01-30) 3 commits
- checkout: convert post_checkout_hook() to struct object_id
- use oidcpy() for copying hashes between instances of struct object_id
- use oid_to_hex_r() for converting struct object_id hashes to hex strings
"uchar [40]" to "struct object_id" conversion continues.
Will merge to 'next'.
* rs/swap (2017-01-30) 5 commits
- graph: use SWAP macro
- diff: use SWAP macro
- use SWAP macro
- apply: use SWAP macro
- add SWAP macro
Code clean-up.
Will merge to 'next'.
* pl/complete-diff-submodule-diff (2017-01-30) 1 commit
- Completion: Add support for --submodule=diff
The command line completion (in contrib/) learned that
"git diff --submodule=" can take "diff" as a recently added option.
Will merge to 'next'.
* rs/receive-pack-cleanup (2017-01-30) 1 commit
- receive-pack: call string_list_clear() unconditionally
Code clean-up.
Will merge to 'next'.
* sb/submodule-add-force (2016-11-29) 1 commit
+ submodule add: extend force flag to add existing repos
(this branch is used by sb/push-make-submodule-check-the-default.)
"git submodule add" used to be confused and refused to add a
locally created repository; users can now use "--force" option
to add them.
Will merge to 'next'.
--------------------------------------------------
[Stalled]
* ls/p4-path-encoding (2016-12-18) 1 commit
- git-p4: fix git-p4.pathEncoding for removed files
When "git p4" imports changelist that removes paths, it failed to
convert pathnames when the p4 used encoding different from the one
used on the Git side. This has been corrected.
Will be rerolled.
cf. <7E1C7387-4F37-423F-803D-3B5690B49D40@gmail.com>
* sh/grep-tree-obj-tweak-output (2017-01-20) 2 commits
- grep: use '/' delimiter for paths
- grep: only add delimiter if there isn't one already
"git grep", when fed a tree-ish as an input, shows each hit
prefixed with "<tree-ish>:<path>:<lineno>:". As <tree-ish> is
almost always either a commit or a tag that points at a commit, the
early part of the output "<tree-ish>:<path>" can be used as the
name of the blob and given to "git show". When <tree-ish> is a
tree given in the extended SHA-1 syntax (e.g. "<commit>:", or
"<commit>:<dir>"), however, this results in a string that does not
name a blob (e.g. "<commit>::<path>" or "<commit>:<dir>:<path>").
"git grep" has been taught to be a bit more intelligent about these
cases and omit a colon (in the former case) or use slash (in the
latter case) to produce "<commit>:<path>" and
"<commit>:<dir>/<path>" that can be used as the name of a blob.
Waiting for the review discussion to settle, followed by a reroll.
* mh/ref-remove-empty-directory (2017-01-07) 23 commits
- files_transaction_commit(): clean up empty directories
- try_remove_empty_parents(): teach to remove parents of reflogs, too
- try_remove_empty_parents(): don't trash argument contents
- try_remove_empty_parents(): rename parameter "name" -> "refname"
- delete_ref_loose(): inline function
- delete_ref_loose(): derive loose reference path from lock
- log_ref_write_1(): inline function
- log_ref_setup(): manage the name of the reflog file internally
- log_ref_write_1(): don't depend on logfile argument
- log_ref_setup(): pass the open file descriptor back to the caller
- log_ref_setup(): improve robustness against races
- log_ref_setup(): separate code for create vs non-create
- log_ref_write(): inline function
- rename_tmp_log(): improve error reporting
- rename_tmp_log(): use raceproof_create_file()
- lock_ref_sha1_basic(): use raceproof_create_file()
- lock_ref_sha1_basic(): inline constant
- raceproof_create_file(): new function
- safe_create_leading_directories(): set errno on SCLD_EXISTS
- safe_create_leading_directories_const(): preserve errno
- t5505: use "for-each-ref" to test for the non-existence of references
- refname_is_safe(): correct docstring
- files_rename_ref(): tidy up whitespace
Deletion of a branch "foo/bar" could remove .git/refs/heads/foo
once there no longer is any other branch whose name begins with
"foo/", but we didn't do so so far. Now we do.
Expecting a reroll.
cf. <5051c78e-51f9-becd-e1a6-9c0b781d6912@alum.mit.edu>
* jc/diff-b-m (2015-02-23) 5 commits
. WIPWIP
. WIP: diff-b-m
- diffcore-rename: allow easier debugging
- diffcore-rename.c: add locate_rename_src()
- diffcore-break: allow debugging
"git diff -B -M" produced incorrect patch when the postimage of a
completely rewritten file is similar to the preimage of a removed
file; such a resulting file must not be expressed as a rename from
other place.
The fix in this patch is broken, unfortunately.
Will discard.
--------------------------------------------------
[Cooking]
* js/mingw-hooks-with-exe-suffix (2017-01-30) 1 commit
- mingw: allow hooks to be .exe files
Names of the various hook scripts must be spelled exactly, but on
Windows, an .exe binary must be named with .exe suffix; notice
$GIT_DIR/hooks/<hookname>.exe as a valid <hookname> hook.
Will merge to 'next'.
* js/retire-relink (2017-01-25) 2 commits
- relink: really remove the command
- relink: retire the command
Cruft removal.
Will merge to 'next'.
* js/status-pre-rebase-i (2017-01-26) 1 commit
(merged to 'next' on 2017-01-31 at 09e51b2e39)
+ status: be prepared for not-yet-started interactive rebase
After starting "git rebase -i", which first opens the user's editor
to edit the series of patches to apply, but before saving the
contents of that file, "git status" failed to show the current
state (i.e. you are in an interactive rebase session, but you have
applied no steps yet) correctly.
Will merge to 'master'.
* ps/urlmatch-wildcard (2017-01-31) 5 commits
. urlmatch: allow globbing for the URL host part
. urlmatch: include host in urlmatch ranking
. urlmatch: split host and port fields in `struct url_info`
. urlmatch: enable normalization of URLs with globs
. mailmap: add Patrick Steinhardt's work address
The <url> part in "http.<url>.<variable>" configuration variable
can now be spelled with '*' that serves as wildcard.
E.g. "http.https://*.example.com.proxy" can be used to specify the
proxy used for https://a.example.com, https://b.example.com, etc.,
i.e. any host in the example.com domain.
With the update it still seems to fail the same t5551#31
cf. <cover.1485853153.git.ps@pks.im>
* rs/absolute-pathdup (2017-01-27) 2 commits
(merged to 'next' on 2017-01-31 at f751f64876)
+ use absolute_pathdup()
+ abspath: add absolute_pathdup()
Code cleanup.
Will merge to 'master'.
* sb/submodule-recursive-absorb (2017-01-26) 3 commits
(merged to 'next' on 2017-01-31 at 0a24cfd06b)
+ submodule absorbing: fix worktree/gitdir pointers recursively for non-moves
+ cache.h: expose the dying procedure for reading gitlinks
+ setup: add gentle version of resolve_git_dir
When a submodule "sub", which has another submodule "module" nested
within it, is "absorbed" into the top-level superproject, the inner
submodule "module" is left in a strange state.
Will merge to 'master'.
* sb/submodule-update-initial-runs-custom-script (2017-01-26) 1 commit
(merged to 'next' on 2017-01-31 at d794f894c6)
+ submodule update: run custom update script for initial populating as well
The user can specify a custom update method that is run when
"submodule update" updates an already checked out submodule. This
was ignored when checking the submodule out for the first time and
we instead always just checked out the commit that is bound to the
path in the superproject's index.
Will merge to 'master'.
* sf/putty-w-args (2017-01-26) 3 commits
- connect: support GIT_SSH_VARIANT and ssh.variant
- connect: rename tortoiseplink and putty variables
- connect: handle putty/plink also in GIT_SSH_COMMAND
The command line options for ssh invocation needs to be tweaked for
some implementations of SSH (e.g. PuTTY plink wants "-P <port>"
while OpenSSH wants "-p <port>" to specify port to connect to), and
the variant was guessed when GIT_SSH environment variable is used
to specify it. Extend the guess to the command specified by the
newer GIT_SSH_COMMAND and also core.sshcommand configuration
variable, and give an escape hatch for users to deal with
misdetected cases.
Expecting a reroll of the last step to plug new memory leak.
cf. <xmqqpoj8z7su.fsf@gitster.mtv.corp.google.com>
* bc/use-asciidoctor-opt (2017-01-31) 8 commits
- Documentation: implement linkgit macro for Asciidoctor
- Makefile: add a knob to enable the use of Asciidoctor
- Documentation: move dblatex arguments into variable
- Documentation: add XSLT to fix DocBook for Texinfo
- Documentation: sort sources for gitman.texi
- Documentation: remove unneeded argument in cat-texi.perl
- Documentation: modernize cat-texi.perl
- Documentation: fix warning in cat-texi.perl
Asciidoctor, an alternative reimplementation of AsciiDoc, still
needs some changes to work with documents meant to be formatted
with AsciiDoc. "make USE_ASCIIDOCTOR=YesPlease" to use it out of
the box to document our pages is getting closer to reality.
Will merge to 'next'.
* jk/describe-omit-some-refs (2017-01-23) 5 commits
(merged to 'next' on 2017-01-23 at f8a14b4996)
+ describe: teach describe negative pattern matches
+ describe: teach --match to accept multiple patterns
+ name-rev: add support to exclude refs by pattern match
+ name-rev: extend --refs to accept multiple patterns
+ doc: add documentation for OPT_STRING_LIST
"git describe" and "git name-rev" have been taught to take more
than one refname patterns to restrict the set of refs to base their
naming output on, and also learned to take negative patterns to
name refs not to be used for naming via their "--exclude" option.
Will cook in 'next'.
* ep/commit-static-buf-cleanup (2017-01-31) 2 commits
(merged to 'next' on 2017-01-31 at 02d3c25219)
+ builtin/commit.c: switch to strbuf, instead of snprintf()
+ builtin/commit.c: remove the PATH_MAX limitation via dynamic allocation
Code clean-up.
Will merge to 'master'.
* sb/unpack-trees-super-prefix (2017-01-25) 4 commits
(merged to 'next' on 2017-01-31 at dabe6ca2b1)
+ unpack-trees: support super-prefix option
+ t1001: modernize style
+ t1000: modernize style
+ read-tree: use OPT_BOOL instead of OPT_SET_INT
"git read-tree" and its underlying unpack_trees() machinery learned
to report problematic paths prefixed with the --super-prefix option.
Will merge to 'master'.
* sb/submodule-doc (2017-01-12) 3 commits
- submodules: add a background story
- submodule update documentation: don't repeat ourselves
- submodule documentation: add options to the subcommand
Needs review.
* bw/attr (2017-01-23) 27 commits
- attr: reformat git_attr_set_direction() function
- attr: push the bare repo check into read_attr()
- attr: store attribute stack in attr_check structure
- attr: tighten const correctness with git_attr and match_attr
- attr: remove maybe-real, maybe-macro from git_attr
- attr: eliminate global check_all_attr array
- attr: use hashmap for attribute dictionary
- attr: change validity check for attribute names to use positive logic
- attr: pass struct attr_check to collect_some_attrs
- attr: retire git_check_attrs() API
- attr: convert git_check_attrs() callers to use the new API
- attr: convert git_all_attrs() to use "struct attr_check"
- attr: (re)introduce git_check_attr() and struct attr_check
- attr: rename function and struct related to checking attributes
- attr.c: outline the future plans by heavily commenting
- Documentation/gitattributes.txt: fix a typo
- attr.c: add push_stack() helper
- attr: support quoting pathname patterns in C style
- attr.c: plug small leak in parse_attr_line()
- attr.c: tighten constness around "git_attr" structure
- attr.c: simplify macroexpand_one()
- attr.c: mark where #if DEBUG ends more clearly
- attr.c: complete a sentence in a comment
- attr.c: explain the lack of attr-name syntax check in parse_attr()
- attr.c: update a stale comment on "struct match_attr"
- attr.c: use strchrnul() to scan for one line
- commit.c: use strchrnul() to scan for one line
The gitattributes machinery is being taught to work better in a
multi-threaded environment.
Expecting a reroll.
* vn/xdiff-func-context (2017-01-15) 1 commit
- xdiff -W: relax end-of-file function detection
"git diff -W" has been taught to handle the case where a new
function is added at the end of the file better.
Will hold.
An follow-up change to go back from the line that matches the
funcline to show comments before the function definition is still
being discussed.
* ls/filter-process-delayed (2017-01-08) 1 commit
. convert: add "status=delayed" to filter process protocol
Ejected, as does not build when merged to 'pu'.
* nd/worktree-move (2017-01-27) 7 commits
- fixup! worktree move: new command
- worktree remove: new command
- worktree move: refuse to move worktrees with submodules
- worktree move: accept destination as directory
- worktree move: new command
- worktree.c: add update_worktree_location()
- worktree.c: add validate_worktree()
"git worktree" learned move and remove subcommands.
Needs review.
* nd/log-graph-configurable-colors (2017-01-23) 3 commits
(merged to 'next' on 2017-01-23 at c369982ad8)
+ log --graph: customize the graph lines with config log.graphColors
+ color.c: trim leading spaces in color_parse_mem()
+ color.c: fix color_parse_mem() with value_len == 0
Some people feel the default set of colors used by "git log --graph"
rather limiting. A mechanism to customize the set of colors has
been introduced.
Reported to break "add -p".
cf. <20170128040709.tqx4u45ktgpkbfm4@sigill.intra.peff.net>
* cc/split-index-config (2016-12-26) 21 commits
- Documentation/git-update-index: explain splitIndex.*
- Documentation/config: add splitIndex.sharedIndexExpire
- read-cache: use freshen_shared_index() in read_index_from()
- read-cache: refactor read_index_from()
- t1700: test shared index file expiration
- read-cache: unlink old sharedindex files
- config: add git_config_get_expiry() from gc.c
- read-cache: touch shared index files when used
- sha1_file: make check_and_freshen_file() non static
- Documentation/config: add splitIndex.maxPercentChange
- t1700: add tests for splitIndex.maxPercentChange
- read-cache: regenerate shared index if necessary
- config: add git_config_get_max_percent_split_change()
- Documentation/git-update-index: talk about core.splitIndex config var
- Documentation/config: add information for core.splitIndex
- t1700: add tests for core.splitIndex
- update-index: warn in case of split-index incoherency
- read-cache: add and then use tweak_split_index()
- split-index: add {add,remove}_split_index() functions
- config: add git_config_get_split_index()
- config: mark an error message up for translation
The experimental "split index" feature has gained a few
configuration variables to make it easier to use.
Waiting for review comments to be addressed.
cf. <20161226102222.17150-1-chriscool@tuxfamily.org>
cf. <a1a44640-ff6c-2294-72ac-46322eff8505@ramsayjones.plus.com>
* sb/push-make-submodule-check-the-default (2017-01-26) 2 commits
(merged to 'next' on 2017-01-26 at 5f4715cea6)
+ Revert "push: change submodule default to check when submodules exist"
(merged to 'next' on 2016-12-12 at 1863e05af5)
+ push: change submodule default to check when submodules exist
(this branch uses sb/submodule-add-force.)
Turn the default of "push.recurseSubmodules" to "check" when
submodules seem to be in use.
Retracted.
* kn/ref-filter-branch-list (2017-01-31) 20 commits
- branch: implement '--format' option
- branch: use ref-filter printing APIs
- branch, tag: use porcelain output
- ref-filter: allow porcelain to translate messages in the output
- ref-filter: add an 'rstrip=<N>' option to atoms which deal with refnames
- ref-filter: modify the 'lstrip=<N>' option to work with negative '<N>'
- ref-filter: Do not abruptly die when using the 'lstrip=<N>' option
- ref-filter: rename the 'strip' option to 'lstrip'
- ref-filter: make remote_ref_atom_parser() use refname_atom_parser_internal()
- ref-filter: introduce refname_atom_parser()
- ref-filter: introduce refname_atom_parser_internal()
- ref-filter: make "%(symref)" atom work with the ':short' modifier
- ref-filter: add support for %(upstream:track,nobracket)
- ref-filter: make %(upstream:track) prints "[gone]" for invalid upstreams
- ref-filter: introduce format_ref_array_item()
- ref-filter: move get_head_description() from branch.c
- ref-filter: modify "%(objectname:short)" to take length
- ref-filter: implement %(if:equals=<string>) and %(if:notequals=<string>)
- ref-filter: include reference to 'used_atom' within 'atom_value'
- ref-filter: implement %(if), %(then), and %(else) atoms
The code to list branches in "git branch" has been consolidated
with the more generic ref-filter API.
Will merge to 'next'.
* jk/no-looking-at-dotgit-outside-repo-final (2016-10-26) 1 commit
(merged to 'next' on 2016-12-05 at 0c77e39cd5)
+ setup_git_env: avoid blind fall-back to ".git"
Originally merged to 'next' on 2016-10-26
This is the endgame of the topic to avoid blindly falling back to
".git" when the setup sequence said we are _not_ in Git repository.
A corner case that happens to work right now may be broken by a
call to die("BUG").
Will cook in 'next'.
* pb/bisect (2016-10-18) 27 commits
- bisect--helper: remove the dequote in bisect_start()
- bisect--helper: retire `--bisect-auto-next` subcommand
- bisect--helper: retire `--bisect-autostart` subcommand
- bisect--helper: retire `--bisect-write` subcommand
- bisect--helper: `bisect_replay` shell function in C
- bisect--helper: `bisect_log` shell function in C
- bisect--helper: retire `--write-terms` subcommand
- bisect--helper: retire `--check-expected-revs` subcommand
- bisect--helper: `bisect_state` & `bisect_head` shell function in C
- bisect--helper: `bisect_autostart` shell function in C
- bisect--helper: retire `--next-all` subcommand
- bisect--helper: retire `--bisect-clean-state` subcommand
- bisect--helper: `bisect_next` and `bisect_auto_next` shell function in C
- t6030: no cleanup with bad merge base
- bisect--helper: `bisect_start` shell function partially in C
- bisect--helper: `get_terms` & `bisect_terms` shell function in C
- bisect--helper: `bisect_next_check` & bisect_voc shell function in C
- bisect--helper: `check_and_set_terms` shell function in C
- bisect--helper: `bisect_write` shell function in C
- bisect--helper: `is_expected_rev` & `check_expected_revs` shell function in C
- bisect--helper: `bisect_reset` shell function in C
- wrapper: move is_empty_file() and rename it as is_empty_or_missing_file()
- t6030: explicitly test for bisection cleanup
- bisect--helper: `bisect_clean_state` shell function in C
- bisect--helper: `write_terms` shell function in C
- bisect: rewrite `check_term_format` shell function in C
- bisect--helper: use OPT_CMDMODE instead of OPT_BOOL
Move more parts of "git bisect" to C.
Expecting a reroll.
cf. <CAFZEwPPXPPHi8KiEGS9ggzNHDCGhuqMgH9Z8-Pf9GLshg8+LPA@mail.gmail.com>
cf. <CAFZEwPM9RSTGN54dzaw9gO9iZmsYjJ_d1SjUD4EzSDDbmh-XuA@mail.gmail.com>
* jc/merge-drop-old-syntax (2015-04-29) 1 commit
(merged to 'next' on 2016-12-05 at 041946dae0)
+ merge: drop 'git merge <message> HEAD <commit>' syntax
Originally merged to 'next' on 2016-10-11
Stop supporting "git merge <message> HEAD <commit>" syntax that has
been deprecated since October 2007, and issues a deprecation
warning message since v2.5.0.
Will cook in 'next'.
--------------------------------------------------
[Discarded]
* jc/bundle (2016-03-03) 6 commits
. index-pack: --clone-bundle option
. Merge branch 'jc/index-pack' into jc/bundle
. bundle v3: the beginning
. bundle: keep a copy of bundle file name in the in-core bundle header
. bundle: plug resource leak
. bundle doc: 'verify' is not about verifying the bundle
The beginning of "split bundle", which could be one of the
ingredients to allow "git clone" traffic off of the core server
network to CDN.
While I think it would make it easier for people to experiment and
build on if the topic is merged to 'next', I am at the same time a
bit reluctant to merge an unproven new topic that introduces a new
file format, which we may end up having to support til the end of
time. It is likely that to support a "prime clone from CDN", it
would need a lot more than just "these are the heads and the pack
data is over there", so this may not be sufficient.
* jk/nofollow-attr-ignore (2016-11-02) 5 commits
. exclude: do not respect symlinks for in-tree .gitignore
. attr: do not respect symlinks for in-tree .gitattributes
. exclude: convert "check_index" into a flags field
. attr: convert "macro_ok" into a flags field
. add open_nofollow() helper
As we do not follow symbolic links when reading control files like
.gitignore and .gitattributes from the index, match the behaviour
and not follow symbolic links when reading them from the working
tree. This also tightens security a bit by not leaking contents of
an unrelated file in the error messages when it is pointed at by
one of these files that is a symbolic link.
Perhaps we want to cover .gitmodules too with the same mechanism?
^ permalink raw reply
* [PATCH] color_parse_mem: allow empty color spec
From: Jeff King @ 2017-02-01 0:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nguyễn Thái Ngọc Duy, git
In-Reply-To: <xmqqpoj2q25n.fsf@gitster.mtv.corp.google.com>
On Tue, Jan 31, 2017 at 02:45:40PM -0800, Junio C Hamano wrote:
> * nd/log-graph-configurable-colors (2017-01-23) 3 commits
> (merged to 'next' on 2017-01-23 at c369982ad8)
> + log --graph: customize the graph lines with config log.graphColors
> + color.c: trim leading spaces in color_parse_mem()
> + color.c: fix color_parse_mem() with value_len == 0
>
> Some people feel the default set of colors used by "git log --graph"
> rather limiting. A mechanism to customize the set of colors has
> been introduced.
>
> Reported to break "add -p".
> cf. <20170128040709.tqx4u45ktgpkbfm4@sigill.intra.peff.net>
I hadn't heard anything back, so I wrapped up my fix with a commit
message and tests (it needs to go on top anyway, since the breakage is
in 'next').
-- >8 --
Subject: [PATCH] color_parse_mem: allow empty color spec
Prior to c2f41bf52 (color.c: fix color_parse_mem() with
value_len == 0, 2017-01-19), the empty string was
interpreted as a color "reset". This was an accidental
outcome, and that commit turned it into an error.
However, scripts may pass the empty string as a default
value to "git config --get-color" to disable color when the
value is not defined. The git-add--interactive script does
this. As a result, the script is unusable since c2f41bf52
unless you have color.diff.plain defined (if it is defined,
then we don't parse the empty default at all).
Our test scripts didn't notice the recent breakage because
they run without a terminal, and thus without color. They
never hit this code path at all. And nobody noticed the
original buggy "reset" behavior, because it was effectively
a noop.
Let's fix the code to have an empty color name produce an
empty sequence of color codes. The tests need a few fixups:
- we'll add a new test in t4026 to cover this case. But
note that we need to tweak the color() helper. While
we're there, let's factor out the literal ANSI ESC
character. Otherwise it makes the diff quite hard to
read.
- we'll add a basic sanity-check in t4026 that "git add
-p" works at all when color is enabled. That would have
caught this bug, as well as any others that are specific
to the color code paths.
- 73c727d69 (log --graph: customize the graph lines with
config log.graphColors, 2017-01-19) added a test to
t4202 that checks some "invalid" graph color config.
Since ",, blue" before yielded only "blue" as valid, and
now yields "empty, empty, blue", we don't match the
expected output.
One way to fix this would be to change the expectation
to the empty color strings. But that makes the test much
less interesting, since we show only two graph lines,
both of which would be colorless.
Since the empty-string case is now covered by t4026,
let's remove them entirely here. They're just in the way
of the primary thing the test is supposed to be
checking.
Signed-off-by: Jeff King <peff@peff.net>
---
color.c | 6 ++++--
t/t3701-add-interactive.sh | 14 ++++++++++++++
t/t4026-color.sh | 7 ++++++-
t/t4202-log.sh | 2 +-
4 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/color.c b/color.c
index 7bb4a96f8..2925a819b 100644
--- a/color.c
+++ b/color.c
@@ -212,8 +212,10 @@ int color_parse_mem(const char *value, int value_len, char *dst)
len--;
}
- if (!len)
- return -1;
+ if (!len) {
+ dst[0] = '\0';
+ return 0;
+ }
if (!strncasecmp(ptr, "reset", len)) {
xsnprintf(dst, end - dst, GIT_COLOR_RESET);
diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index deae948c7..5ffe78e92 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -380,4 +380,18 @@ test_expect_success 'patch mode ignores unmerged entries' '
test_cmp expected diff
'
+test_expect_success 'diffs can be colorized' '
+ git reset --hard &&
+
+ # force color even though the test script has no terminal
+ test_config color.ui always &&
+
+ echo content >test &&
+ printf y | git add -p >output 2>&1 &&
+
+ # We do not want to depend on the exact coloring scheme
+ # git uses for diffs, so just check that we saw some kind of color.
+ grep "$(printf "\\033")" output
+'
+
test_done
diff --git a/t/t4026-color.sh b/t/t4026-color.sh
index ec78c5e3a..671e951ee 100755
--- a/t/t4026-color.sh
+++ b/t/t4026-color.sh
@@ -6,10 +6,11 @@
test_description='Test diff/status color escape codes'
. ./test-lib.sh
+ESC=$(printf '\033')
color()
{
actual=$(git config --get-color no.such.slot "$1") &&
- test "$actual" = "^[$2"
+ test "$actual" = "${2:+$ESC}$2"
}
invalid_color()
@@ -21,6 +22,10 @@ test_expect_success 'reset' '
color "reset" "[m"
'
+test_expect_success 'empty color is empty' '
+ color "" ""
+'
+
test_expect_success 'attribute before color name' '
color "bold red" "[1;31m"
'
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 0aeabed96..1edbb1e7f 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -329,7 +329,7 @@ cat > expect.colors <<\EOF
EOF
test_expect_success 'log --graph with merge with log.graphColors' '
- test_config log.graphColors ",, blue,invalid-color, cyan, red , " &&
+ test_config log.graphColors " blue,invalid-color, cyan, red , " &&
git log --color=always --graph --date-order --pretty=tformat:%s |
test_decode_color | sed "s/ *\$//" >actual &&
test_cmp expect.colors actual
--
2.11.0.948.gca97da533
^ permalink raw reply related
* Re: [PATCH 1/5] add SWAP macro
From: Ramsay Jones @ 2017-02-01 0:44 UTC (permalink / raw)
To: René Scharfe; +Cc: Git List
In-Reply-To: <676ed19c-0c4e-341e-ba30-1f4a23440088@web.de>
On 31/01/17 21:02, René Scharfe wrote:
[snip]
>> It would be trivially "optimized" out of the box, even when compiling with
>> Tiny C or in debug mode.
>
> Such a compiler is already slowed down by memset(3) calls for initializing objects and lack of other optimizations. I doubt a few more memcpy(3) calls would make that much of a difference.
>
> NB: git as compiled with TCC fails several tests, alas. Builds wickedly fast, though.
Hmm, a couple of years ago, I used tcc to build git and it worked
just fine (and it passed all tests). Prompted by this note, I just
built tcc from the current mob branch (@5420bb8) and built git OK,
but, yes, lots of tests now fail. :(
ATB,
Ramsay Jones
^ permalink raw reply
* Re: [PATCH] color_parse_mem: allow empty color spec
From: Jacob Keller @ 2017-02-01 1:19 UTC (permalink / raw)
To: Jeff King
Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy,
Git mailing list
In-Reply-To: <20170201002129.xb62hmxwrzwgp6vg@sigill.intra.peff.net>
On Tue, Jan 31, 2017 at 4:21 PM, Jeff King <peff@peff.net> wrote:
> On Tue, Jan 31, 2017 at 02:45:40PM -0800, Junio C Hamano wrote:
>
>> * nd/log-graph-configurable-colors (2017-01-23) 3 commits
>> (merged to 'next' on 2017-01-23 at c369982ad8)
>> + log --graph: customize the graph lines with config log.graphColors
>> + color.c: trim leading spaces in color_parse_mem()
>> + color.c: fix color_parse_mem() with value_len == 0
>>
>> Some people feel the default set of colors used by "git log --graph"
>> rather limiting. A mechanism to customize the set of colors has
>> been introduced.
>>
>> Reported to break "add -p".
>> cf. <20170128040709.tqx4u45ktgpkbfm4@sigill.intra.peff.net>
>
> I hadn't heard anything back, so I wrapped up my fix with a commit
> message and tests (it needs to go on top anyway, since the breakage is
> in 'next').
>
I was just about to report this breakage... It definitely breaks usage
of git-add-interactive..
> -- >8 --
> Subject: [PATCH] color_parse_mem: allow empty color spec
>
> Prior to c2f41bf52 (color.c: fix color_parse_mem() with
> value_len == 0, 2017-01-19), the empty string was
> interpreted as a color "reset". This was an accidental
> outcome, and that commit turned it into an error.
>
> However, scripts may pass the empty string as a default
> value to "git config --get-color" to disable color when the
> value is not defined. The git-add--interactive script does
> this. As a result, the script is unusable since c2f41bf52
> unless you have color.diff.plain defined (if it is defined,
> then we don't parse the empty default at all).
>
> Our test scripts didn't notice the recent breakage because
> they run without a terminal, and thus without color. They
> never hit this code path at all. And nobody noticed the
> original buggy "reset" behavior, because it was effectively
> a noop.
>
> Let's fix the code to have an empty color name produce an
> empty sequence of color codes. The tests need a few fixups:
>
> - we'll add a new test in t4026 to cover this case. But
> note that we need to tweak the color() helper. While
> we're there, let's factor out the literal ANSI ESC
> character. Otherwise it makes the diff quite hard to
> read.
>
> - we'll add a basic sanity-check in t4026 that "git add
> -p" works at all when color is enabled. That would have
> caught this bug, as well as any others that are specific
> to the color code paths.
>
> - 73c727d69 (log --graph: customize the graph lines with
> config log.graphColors, 2017-01-19) added a test to
> t4202 that checks some "invalid" graph color config.
> Since ",, blue" before yielded only "blue" as valid, and
> now yields "empty, empty, blue", we don't match the
> expected output.
>
> One way to fix this would be to change the expectation
> to the empty color strings. But that makes the test much
> less interesting, since we show only two graph lines,
> both of which would be colorless.
>
> Since the empty-string case is now covered by t4026,
> let's remove them entirely here. They're just in the way
> of the primary thing the test is supposed to be
> checking.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
The patch looks good to me. Very nice to have a detailed explanation
of why we didn't catch it before, and how to ensure we don't have this
happen again.
Thanks,
Jake
> color.c | 6 ++++--
> t/t3701-add-interactive.sh | 14 ++++++++++++++
> t/t4026-color.sh | 7 ++++++-
> t/t4202-log.sh | 2 +-
> 4 files changed, 25 insertions(+), 4 deletions(-)
>
> diff --git a/color.c b/color.c
> index 7bb4a96f8..2925a819b 100644
> --- a/color.c
> +++ b/color.c
> @@ -212,8 +212,10 @@ int color_parse_mem(const char *value, int value_len, char *dst)
> len--;
> }
>
> - if (!len)
> - return -1;
> + if (!len) {
> + dst[0] = '\0';
> + return 0;
> + }
>
> if (!strncasecmp(ptr, "reset", len)) {
> xsnprintf(dst, end - dst, GIT_COLOR_RESET);
> diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
> index deae948c7..5ffe78e92 100755
> --- a/t/t3701-add-interactive.sh
> +++ b/t/t3701-add-interactive.sh
> @@ -380,4 +380,18 @@ test_expect_success 'patch mode ignores unmerged entries' '
> test_cmp expected diff
> '
>
> +test_expect_success 'diffs can be colorized' '
> + git reset --hard &&
> +
> + # force color even though the test script has no terminal
> + test_config color.ui always &&
> +
> + echo content >test &&
> + printf y | git add -p >output 2>&1 &&
> +
> + # We do not want to depend on the exact coloring scheme
> + # git uses for diffs, so just check that we saw some kind of color.
> + grep "$(printf "\\033")" output
> +'
> +
> test_done
> diff --git a/t/t4026-color.sh b/t/t4026-color.sh
> index ec78c5e3a..671e951ee 100755
> --- a/t/t4026-color.sh
> +++ b/t/t4026-color.sh
> @@ -6,10 +6,11 @@
> test_description='Test diff/status color escape codes'
> . ./test-lib.sh
>
> +ESC=$(printf '\033')
> color()
> {
> actual=$(git config --get-color no.such.slot "$1") &&
> - test "$actual" = " $2"
> + test "$actual" = "${2:+$ESC}$2"
> }
>
> invalid_color()
> @@ -21,6 +22,10 @@ test_expect_success 'reset' '
> color "reset" "[m"
> '
>
> +test_expect_success 'empty color is empty' '
> + color "" ""
> +'
> +
> test_expect_success 'attribute before color name' '
> color "bold red" "[1;31m"
> '
> diff --git a/t/t4202-log.sh b/t/t4202-log.sh
> index 0aeabed96..1edbb1e7f 100755
> --- a/t/t4202-log.sh
> +++ b/t/t4202-log.sh
> @@ -329,7 +329,7 @@ cat > expect.colors <<\EOF
> EOF
>
> test_expect_success 'log --graph with merge with log.graphColors' '
> - test_config log.graphColors ",, blue,invalid-color, cyan, red , " &&
> + test_config log.graphColors " blue,invalid-color, cyan, red , " &&
> git log --color=always --graph --date-order --pretty=tformat:%s |
> test_decode_color | sed "s/ *\$//" >actual &&
> test_cmp expect.colors actual
> --
> 2.11.0.948.gca97da533
>
^ permalink raw reply
* Re: git push failing when push.recurseSubmodules on-demand and git commit --amend was used in submodule.
From: Carlo Wood @ 2017-02-01 1:10 UTC (permalink / raw)
To: Stefan Beller; +Cc: Junio C Hamano, Heiko Voigt, git@vger.kernel.org
In-Reply-To: <CAGZ79kbCfKVDq+9Pr5LmOtT=+uB+u+EMQg1=FUNS2umCvtvHhg@mail.gmail.com>
On Tue, 31 Jan 2017 14:08:41 -0800
Stefan Beller <sbeller@google.com> wrote:
> On Sun, Jan 29, 2017 at 5:00 PM, Junio C Hamano <gitster@pobox.com>
> wrote:
> > 2. If the amend is good and ready to go, "git add" to update the
> > superproject to make that amended result the one that is needed
> > in the submodule.
>
> yup.
But that is what I am doing. The amended commit IS already
added to the superproject (and pushed to the remote).
Please have a look at my script, this happens here:
# Correct that in the parent too:
pushd parent
git add subm
git commit -m 'Updated subm.'
popd
The commit from before the amend was added to the super
project (but never pushed) but has now been completely
replaced. I still think this is a flaw in git. It shouldn't
not complain and simply push.
--
Carlo Wood <carlo@alinoe.com>
^ permalink raw reply
* Re: git push failing when push.recurseSubmodules on-demand and git commit --amend was used in submodule.
From: Carlo Wood @ 2017-02-01 1:22 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Heiko Voigt, Stefan Beller, git
In-Reply-To: <xmqqvasxwee1.fsf@gitster.mtv.corp.google.com>
On Sun, 29 Jan 2017 17:00:22 -0800
Junio C Hamano <gitster@pobox.com> wrote:
> I suspect the submodule folks would say it is working as intended,
> if \
>
> - you made a commit in the submodule;
> - recorded the resulting commit in the superproject;
> - you amended the commit in the submodule; and then
> - you did "push, while pushing out in the submodule as needed" from
> the superproject.
This is not what I'm doing.
This is what I'm doing (see my script):
- you made a commit in the submodule;
- recorded the resulting commit in the superproject;
- you amended the commit in the submodule;
- you record the amended commit in the superproject; <=== !
- you push the submodule out (or not, the on-demand does that
anyway)
- you try to push the superproject, but that fails,
as long as you use recurseSubmodules=on-demand.
>
> There are two commits in the submodule that are involved in the
> above scenario, and the first one before amending is needed by the
> other participants of the project in order for them to check out
> what you are trying to push in the superproject, because that is
> what the superproject's tree records.
I never pushed anything of that, so the other participants don't
know, nor have, the pre-amended commit.
It is true that the superproject THINKS that the pre-amended commit
is a normal commit though: the last recorded (amended) commit is
internally listed as being on top of the amended commit (which is
incorrect). This is why the superproject assumes that the current
add commit of the submodule needs the pre-amended commit to be
available too. This is not correct however, it is not needed to
be available to others and does not need to be pushed to a remote.
> I think you have two options.
>
> 1. If the amend was done to improve things in submodule but is not
> quite ready, then get rid of that amended commit and restore the
> branch in the submodule to the state before you amended, i.e.
> the tip of the branch will become the same commit as the one
> that is recorded in the superproject. Then push the submodule
> and the superproject out. After that, move the submodule branch
> to point at the amended commit (or record the amended commit as
> a child of the commit you pushed out).
That would work, but would be a horrible workaround for an existing
bug :p
> 2. If the amend is good and ready to go, "git add" to update the
> superproject to make that amended result the one that is needed
> in the submodule.
This was already done, also in the script that I provided.
Yet, the push in the superproject is still rejected.
--
Carlo Wood <carlo@alinoe.com>
^ permalink raw reply
* Re: [git-users] More on that "merge branch checkout" problem -- cannot abort/go back?
From: Michael @ 2017-02-01 6:36 UTC (permalink / raw)
To: git; +Cc: git-users, Konstantin Khomoutov
In-Reply-To: <B621FC58-AA73-4D88-B5E5-575BC1A6F0EE@gmail.com>
This is going to the main git list. I had an issue with a git gui checkin, where I did a checkin of selected lines (no problem), then tried to switch branches and check in the rest.
Things broke.
I attempted to get help on the git users list, but was unable to.
To recap what happened:
1. I started on develop.
2. I made a feature branch.
3. I did some stuff. About half would be checked in on the feature branch, and about half on a "code cleanup" branch.
4. One hunk in the diff was a close comment (for the code cleanup) and then a bunch of new stuff (For the feature).
5. I used git gui to select what portions I wanted to check in.
6. In the past, using git gui to check in some lines of a hunk in one commit, and the other lines in a succeeding commit _on the same branch_ did not give me trouble. But this time, I was switching branches in between.
7. Git checkout complained about my changes being overwritten. git checkout -m did "work", but complained about a merge conflict.
8. There was no way to switch back -- there was no merge to abort, and no way to recover the previous state.
What I don't understand is:
1. Why is there no way to abort a merge checkout, and
2. Why was it that I could check in a portion of a hunk in git gui, but then not switch without a conflict? I'm not asking why there was a conflict as reported by diff -- I checked in the bottom half of a hunk, and kept "not checked in" the top half, so the context lines would not match. I'm asking more like, why wasn't git able to tell that the other part of the hunk was already checked in, and what I wanted to keep here was the stuff that was not checked in -- the "live git diff" which only showed the close comment in that part of the diff.
To clarify #2: The three versions in the conflict file were "nothing" (the old develop that had none of these changes), "what was checked in" (the feature branch), and "everything"; what I wanted was "everything - what was checked in" (which is what git diff reported before I switched branches).
On 2017-01-30, at 5:45 PM, Michael <keybounce@gmail.com> wrote:
>
> On 2017-01-29, at 11:32 PM, Konstantin Khomoutov <flatworm@users.sourceforge.net> wrote:
>
>> On Sun, 29 Jan 2017 11:07:34 -0800
>> Michael <keybounce@gmail.com> wrote:
>>
>>> So since my attempt to switch branches with the "merge" flag (-m)
>>> gave me an error, I thought I'd try to go back.
>>>
>>> keybounceMBP:Finite-Fluids michael$ git merge --abort
>>> fatal: There is no merge to abort (MERGE_HEAD missing).
>>> keybounceMBP:Finite-Fluids michael$ git status
>>> On branch cleanup
>>> Unmerged paths:
>>> (use "git reset HEAD <file>..." to unstage)
>>> (use "git add <file>..." to mark resolution)
>>>
>>> both modified:
>>> src/main/java/com/mcfht/realisticfluids/fluids/BlockFiniteFluid.java
>>>
>>> Changes not staged for commit:
>>> (use "git add <file>..." to update what will be committed)
>>> (use "git checkout -- <file>..." to discard changes in working
>>> directory)
>>>
>>> modified:
>>> src/main/java/com/mcfht/realisticfluids/FluidData.java modified:
>>> src/main/java/com/mcfht/realisticfluids/commands/CommandEnableFlow.java
>>> modified:
>>> src/main/java/com/mcfht/realisticfluids/fluids/BlockFiniteLava.java
>>>
>>> no changes added to commit (use "git add" and/or "git commit -a")
>>>
>>> I'm not saying this is necessarily wrong (the other branch does have
>>> everything, and I'm on a branch where I could just commit all these
>>> changes now, and worry about fixing it later; no data would be lost),
>>> but I do think that it breaks the user's reasonable beliefs about
>>> what the system does -- including the ability of a version control
>>> system to roll back to a prior state (which in this case it cannot --
>>> or if it can, I don't know how.)
>>
>> To cite the documentation:
>>
>> | -m, --merge
>> | When switching branches, if you have local modifications to
>> | one or more files that are different between the current branch and
>> | the branch to which you are switching, the command refuses to switch
>> | branches in order to preserve your modifications in context. However,
>> | with this option, a three-way merge between the current branch, your
>> | working tree contents, and the new branch is done, and you will be on
>> | the new branch.
>> |
>> | When a merge conflict happens, the index entries for conflicting
>> | paths are left unmerged, and you need to resolve the conflicts and
>> | mark the resolved paths with git add (or git rm if the merge should
>> | result in deletion of the path).
>> |
>> | When checking out paths from the index, this option lets you recreate
>> | the conflicted merge in the specified paths.
>>
>> So I'd say your best bet is to run
>>
>> git checkout --ours path/in/conflict
>>
>> on conflicting paths.
>>
>> As to your idea, I find it to be sensible but in fact you did not
>> attempt true merge, and hence asking for aborting it is dubious.
>>
>> Maybe `git checkout --abort` would have more sense? But then the
>> question is: what state should it roll your back to? Should it revert
>> switching on a new branch as well?
>>
>> Please consider bringing this question on the main Git list to solicit
>> insight of those with mad Git skillz. ;-)
>
> I still don't understand "--ours" or "--theirs" on checkout.
>
> I also am not sure that any of them are what I want. The three "versions" at the merge hunk were:
> 1. Nothing (this was the previous 'develop' branch tip)
> 2. The stuff that was added in the checkin on the other branch (this was the previous 'feature' branch tip)
> 3. Everything, the close comment AND the stuff added (this was the whole conflicted hunk)
>
> What I needed was basically "whole hunk - stuff checked in on feature". And since the file had other things that merged in just fine (an open comment earlier up), neither version (ours or theirs) could be the right version, even if I could restrict it to just the conflicted hunk.
>
> Where should it roll back to? Where I was before. There's a ref (I think it's MERGE_HEAD) created to allow you to abort a merge back to where you were, so why not a CHECKOUT_HEAD (or, if it's doing a 3-way merge, why not record the full merge information?)
---
Entertaining minecraft videos
http://YouTube.com/keybounce
^ permalink raw reply
* Re: [ANNOUNCE] Git Merge Contributor Summit topic planning
From: Erik van Zijst @ 2017-02-01 9:32 UTC (permalink / raw)
To: peff; +Cc: git, ssaasen, mheemskerk
In-Reply-To: <20170131004804.p5sule4rh2xrgtwe@sigill.intra.peff.net>
On Tue, Jan 31, 2017 at 01:48:05AM +0100, Jeff King wrote:
> The list of topics is totally open. If you're coming and have something
> you'd like to present or discuss, then propose it here. If you're _not_
> coming, you may still chime in with input on topics, but please don't
> suggest a topic unless somebody who is there will agree to lead the
> discussion.
I would like to talk about the possibility of CDN-aided cloning
operations as mentioned on this list earlier this week:
http://public-inbox.org/git/CADoxLGPFgF7W4XJzt0X+xFJDoN6RmfFGx_96MO9GPSSOjDK0EQ@mail.gmail.com/
At Bitbucket we have recently rolled out so-called clonebundle support
for Mercurial repositories.
Full clone operations are rather expensive on the server and are
responsible for a substantial part of our CPU and IO load. CDN-based
clonebundles have allowed us to eliminate most of this load for
Mercurial repos and we've since built a clonebundle spike for Git.
Clients performing a full clone get redirected to a CDN where they seed
their new local repo from a pre-built bundle file, and then pull/fetch
any remaining changes. Mercurial has had native, built-in support for
this for a while now.
I imagine other large code hosts could benefit from this as well and
I'd love to gauge the group's interest for this. Could this make sense
for Git? Would it have a chance of landing?
Our spike implements it as an optional capability during ref
advertisement. What are your thoughts on this?
Cheers,
Erik
^ permalink raw reply
* Re: What's cooking in git.git (Jan 2017, #06; Tue, 31)
From: Patrick Steinhardt @ 2017-02-01 11:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, psteinhardt
In-Reply-To: <xmqqpoj2q25n.fsf@gitster.mtv.corp.google.com>
[-- Attachment #1: Type: text/plain, Size: 2396 bytes --]
On Tue, Jan 31, 2017 at 02:45:40PM -0800, Junio C Hamano wrote:
> * ps/urlmatch-wildcard (2017-01-31) 5 commits
> . urlmatch: allow globbing for the URL host part
> . urlmatch: include host in urlmatch ranking
> . urlmatch: split host and port fields in `struct url_info`
> . urlmatch: enable normalization of URLs with globs
> . mailmap: add Patrick Steinhardt's work address
>
> The <url> part in "http.<url>.<variable>" configuration variable
> can now be spelled with '*' that serves as wildcard.
> E.g. "http.https://*.example.com.proxy" can be used to specify the
> proxy used for https://a.example.com, https://b.example.com, etc.,
> i.e. any host in the example.com domain.
>
> With the update it still seems to fail the same t5551#31
> cf. <cover.1485853153.git.ps@pks.im>
Yeah, you're right. Sorry about this one.
I finally fixed the problem to get t5551 working again, see the
attached patch below. The problem was that I did not honor the
case where multiple configuration entries exist with the same
key. In some cases, this has to lead to the value being
accumulated multiple times, as for example with http.extraheaders
used in t5551. So the fixup below fixes this.
I just tried to write additional tests to exercise this in our
config-tests by using `git config --get-urlmatch` with multiple
`http.extraheader`s set. I've been stumped that it still didn't
work correctly with my patch, only the last value was actually
ever returned. But in fact, current Git (tested with v2.11.0)
also fails to do so correctly and only lists the last value.
As such, I'd argue this is a separate issue. If you're not
opposed, I'd like to fix this in a separate patch series later on
(probably after Git Merge).
--- >8 ---
Subject: [PATCH] fixup! urlmatch: include host in urlmatch ranking
---
urlmatch.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/urlmatch.c b/urlmatch.c
index 6c12f1a48..739a1a558 100644
--- a/urlmatch.c
+++ b/urlmatch.c
@@ -587,7 +587,7 @@ int urlmatch_config_entry(const char *var, const char *value, void *cb)
if (!item->util) {
item->util = xcalloc(1, sizeof(matched));
} else {
- if (cmp_matches(&matched, item->util) <= 0)
+ if (cmp_matches(&matched, item->util) < 0)
/*
* Our match is worse than the old one,
* we cannot use it.
--
2.11.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply related
* Re: [PATCH 1/5] add SWAP macro
From: Johannes Schindelin @ 2017-02-01 11:28 UTC (permalink / raw)
To: Jeff King
Cc: René Scharfe, Brandon Williams, Johannes Sixt, Git List,
Junio C Hamano
In-Reply-To: <20170131213507.uiwmkkcg7umvd3f4@sigill.intra.peff.net>
[-- Attachment #1: Type: text/plain, Size: 1390 bytes --]
Hi Peff,
On Tue, 31 Jan 2017, Jeff King wrote:
> On Tue, Jan 31, 2017 at 10:03:01PM +0100, René Scharfe wrote:
>
> > > Perhaps we could disallow a side-effect operator in the macro. By
> > > disallow I mean place a comment at the definition to the macro and
> > > hopefully catch something like that in code-review. We have the
> > > same issue with the `ALLOC_GROW()` macro.
> >
> > SWAP(a++, ...) is caught by the compiler, SWAP(*a++, ...) works fine.
> > Technically that should be enough. :) A comment wouldn't hurt, of
> > course.
>
> One funny thing is that your macro takes address-of itself, behind the
> scenes. I wonder if it would be more natural for it to take
> pointers-to-objects, making it look more like a real function (i.e.,
> SWAP(&a, &b) instead of SWAP(a, b)". And then these funny corner cases
> become quite obvious in the caller, because the caller is the one who
> has to type "&".
But forcing SWAP(&a, &b) would make it even more cumbersome to use, and it
would also make it harder to optimize, say, by using registers instead of
addressable memory (think swapping two 32-bit integers: there is *no* need
to write them into memory just to swap them).
And I think I should repeat my point that this discussion veers towards
making simple swaps *more* complicated, rather than less complicated. Bad
direction.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 1/5] add SWAP macro
From: Johannes Schindelin @ 2017-02-01 11:39 UTC (permalink / raw)
To: René Scharfe; +Cc: Git List, Junio C Hamano
In-Reply-To: <676ed19c-0c4e-341e-ba30-1f4a23440088@web.de>
[-- Attachment #1: Type: text/plain, Size: 2247 bytes --]
Hi René,
On Tue, 31 Jan 2017, René Scharfe wrote:
> Am 31.01.2017 um 13:13 schrieb Johannes Schindelin:
>
> > #define SIMPLE_SWAP(T, a, b) do { T tmp_ = a; a = b; b = tmp_; } while (0)
> > ...
> > uint32_t large;
> > char nybble;
> >
> > ...
> >
> > if (!(large & ~0xf)) {
> > SIMPLE_SWAP(char, nybble, large);
> > ...
> > }
> >
> > i.e. mixing types, when possible.
> >
> > And while I do not necessarily expect that we need anything like this
> > anytime soon, merely the fact that it allows for this flexibility, while
> > being very readable at the same time, would make it a pretty good design
> > in my book.
>
> Such a skinny macro which only hides repetition is kind of attractive due to
> its simplicity; I can't say the same about the mixed type example above,
> though.
>
> The fat version isn't that bad either even without inlining, includes a few
> safety checks and doesn't require us to tell the compiler something it
> already knows very well. I'd rather let the machine do the work.
I am a big fan of letting the machine do the work. But I am not a big fan
of *creating* work for the machine.
So if you asked me what I would think of a script that, given a patch "in
mbox format", automatically fixes all formatting issues that typically
take up a sizable chunk of review time, I would say: yeah, let's get this
done! It would probably take away some fun because then reviewers could
bike-shed less, but I'd think that is a good thing.
If you asked me what my opinion is about a program you wrote that gathers
all the threads and sub threads of code^Wpatch reviews on the Git mailing
list, and cross-references them with public Git branches, and with Junio's
What's Cooking mail, and with the SHA-1s in `pu`: Great! That would
relieve me of a ton of really boring and grueling work, if the machine can
do it, all the better.
And if you ask me about adding a complex macro that adds a bunch of work
for the C compiler just to produce the same assembler code as a one-liner
macro would have produced much easier, I will reply that we could maybe
instead spend that time on letting the machine perform tasks that already
need to be done... :-)
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 1/5] add SWAP macro
From: Jeff King @ 2017-02-01 11:47 UTC (permalink / raw)
To: Johannes Schindelin
Cc: René Scharfe, Brandon Williams, Johannes Sixt, Git List,
Junio C Hamano
In-Reply-To: <alpine.DEB.2.20.1702011225250.3469@virtualbox>
On Wed, Feb 01, 2017 at 12:28:13PM +0100, Johannes Schindelin wrote:
> > One funny thing is that your macro takes address-of itself, behind the
> > scenes. I wonder if it would be more natural for it to take
> > pointers-to-objects, making it look more like a real function (i.e.,
> > SWAP(&a, &b) instead of SWAP(a, b)". And then these funny corner cases
> > become quite obvious in the caller, because the caller is the one who
> > has to type "&".
>
> But forcing SWAP(&a, &b) would make it even more cumbersome to use, and it
> would also make it harder to optimize, say, by using registers instead of
> addressable memory (think swapping two 32-bit integers: there is *no* need
> to write them into memory just to swap them).
I don't find the register thing compelling at all. I'd expect modern
compilers to optimize *(&a) into just "a" and keep using a register. I'm
sure there are compilers that don't, but I'm also not sure how much we
_care_. If your compiler doesn't do basic micro-optimizations, then you
live with it or get a better compiler.
I'm much more interested in what's readable and maintainable, versus
trying to micro-optimize something that hasn't even been shown to be
measurably interesting.
That said...
> And I think I should repeat my point that this discussion veers towards
> making simple swaps *more* complicated, rather than less complicated. Bad
> direction.
I'm not altogether convinced that SWAP() is an improvement in
readability. I really like that it's shorter than the code it replaces,
but there is a danger with introducing opaque constructs. It's one more
thing for somebody familiar with C but new to the project to learn or
get wrong.
IMHO the main maintenance gain from René's patch is that you don't have
to specify the type, which means you can never have a memory-overflow
bug due to incorrect types. If we lose that, then I don't see all that
much value in the whole thing.
-Peff
^ permalink raw reply
* [PATCH v3 0/4] Handle PuTTY (plink/tortoiseplink) even in GIT_SSH_COMMAND
From: Johannes Schindelin @ 2017-02-01 11:57 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King
In-Reply-To: <cover.1485442231.git.johannes.schindelin@gmx.de>
We already handle PuTTY's plink and TortoiseGit's tortoiseplink in
GIT_SSH by automatically using the -P option to specify ports, and in
tortoiseplink's case by passing the --batch option.
For users who need to pass additional command-line options to plink,
this poses a problem: the only way to do that is to use GIT_SSH_COMMAND,
but Git does not handle that specifically, so those users have to
manually parse the command-line options passed via GIT_SSH_COMMAND and
replace -p (if present) by -P, and add --batch in the case of
tortoiseplink.
This is error-prone and a bad user experience.
To fix this, the changes proposed in this patch series introduce
handling this by splitting the GIT_SSH_COMMAND value and treating the
first parameter with the same grace as GIT_SSH. To counter any possible
misdetection, the user can also specify explicitly via GIT_SSH_VARIANT
or ssh.variant which SSH variant they are using.
Changes relative to v2:
- touched up the documentation for ssh.variant to make it even easier to
understand
- free()d the config variable
- completely refactored the code to fulfil Junio's burning desire to
avoid split_cmdline() when unnecessary
It is quite preposterous to call this an "iteration" of the patch
series, because the code is so different now. I say this because I want
to caution that this code has not been tested as thoroughly, by far, as
the first iteration.
The primary purpose of code review is correctness, everything else is
either a consequence of it, or a means to make reviewing easier.
That means that I highly encourage those who pushed for these extensive
changes that make the patch series a lot less robust to balance things
out by at least *rudimentary* testing.
Johannes Schindelin (1):
git_connect(): factor out SSH variant handling
Junio C Hamano (1):
connect: rename tortoiseplink and putty variables
Segev Finer (2):
connect: handle putty/plink also in GIT_SSH_COMMAND
connect: Add the envvar GIT_SSH_VARIANT and ssh.variant config
Documentation/config.txt | 11 +++++++
Documentation/git.txt | 6 ++++
connect.c | 75 ++++++++++++++++++++++++++++++++++++------------
t/t5601-clone.sh | 41 ++++++++++++++++++++++++++
4 files changed, 114 insertions(+), 19 deletions(-)
base-commit: 8f60064c1f538f06e1c579cbd9840b86b10bcd3d
Published-As: https://github.com/dscho/git/releases/tag/putty-w-args-v3
Fetch-It-Via: git fetch https://github.com/dscho/git putty-w-args-v3
Interdiff vs v2:
diff --git a/Documentation/config.txt b/Documentation/config.txt
index f2c210f0a0..b88df57ab6 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1950,11 +1950,15 @@ matched against are those given directly to Git commands. This means any URLs
visited as a result of a redirection do not participate in matching.
ssh.variant::
- Override the autodetection of plink/tortoiseplink in the SSH
- command that 'git fetch' and 'git push' use. It can be set to
- either `ssh`, `plink`, `putty` or `tortoiseplink`. Any other
- value will be treated as normal ssh. This is useful in case
- that Git gets this wrong.
+ Depending on the value of the environment variables `GIT_SSH` or
+ `GIT_SSH_COMMAND`, or the config setting `core.sshCommand`, Git
+ auto-detects whether to adjust its command-line parameters for use
+ with plink or tortoiseplink, as opposed to the default (OpenSSH).
++
+The config variable `ssh.variant` can be set to override this auto-detection;
+valid values are `ssh`, `plink`, `putty` or `tortoiseplink`. Any other value
+will be treated as normal ssh. This setting can be overridden via the
+environment variable `GIT_SSH_VARIANT`.
i18n.commitEncoding::
Character encoding the commit messages are stored in; Git itself
diff --git a/Documentation/git.txt b/Documentation/git.txt
index c322558aa7..a0c6728d1a 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -1021,11 +1021,10 @@ personal `.ssh/config` file. Please consult your ssh documentation
for further details.
`GIT_SSH_VARIANT`::
- If this environment variable is set, it overrides the autodetection
- of plink/tortoiseplink in the SSH command that 'git fetch' and 'git
- push' use. It can be set to either `ssh`, `plink`, `putty` or
- `tortoiseplink`. Any other value will be treated as normal ssh. This
- is useful in case that Git gets this wrong.
+ If this environment variable is set, it overrides Git's autodetection
+ whether `GIT_SSH`/`GIT_SSH_COMMAND`/`core.sshCommand` refer to OpenSSH,
+ plink or tortoiseplink. This variable overrides the config setting
+ `ssh.variant` that serves the same purpose.
`GIT_ASKPASS`::
If this environment variable is set, then Git commands which need to
diff --git a/connect.c b/connect.c
index 7b4437578b..7f1f802396 100644
--- a/connect.c
+++ b/connect.c
@@ -691,20 +691,45 @@ static const char *get_ssh_command(void)
return NULL;
}
-static int handle_ssh_variant(int *port_option, int *needs_batch)
+static int handle_ssh_variant(const char *ssh_command, int is_cmdline,
+ int *port_option, int *needs_batch)
{
- const char *variant;
+ const char *variant = getenv("GIT_SSH_VARIANT");
+ char *p = NULL;
+
+ if (variant)
+ ; /* okay, fall through */
+ else if (!git_config_get_string("ssh.variant", &p))
+ variant = p;
+ else if (!is_cmdline) {
+ p = xstrdup(ssh_command);
+ variant = basename(p);
+ } else {
+ const char **ssh_argv;
- if (!(variant = getenv("GIT_SSH_VARIANT")) &&
- git_config_get_string_const("ssh.variant", &variant))
- return 0;
+ p = xstrdup(ssh_command);
+ if (split_cmdline(p, &ssh_argv)) {
+ variant = basename((char *)ssh_argv[0]);
+ /*
+ * At this point, variant points into the buffer
+ * referenced by p, hence we do not need ssh_argv
+ * any longer.
+ */
+ free(ssh_argv);
+ } else
+ return 0;
+ }
- if (!strcmp(variant, "plink") || !strcmp(variant, "putty"))
+ if (!strcasecmp(variant, "plink") ||
+ !strcasecmp(variant, "plink.exe") ||
+ !strcasecmp(variant, "putty"))
*port_option = 'P';
- else if (!strcmp(variant, "tortoiseplink")) {
+ else if (!strcasecmp(variant, "tortoiseplink") ||
+ !strcasecmp(variant, "tortoiseplink.exe")) {
*port_option = 'P';
*needs_batch = 1;
}
+ free(p);
return 1;
}
@@ -791,7 +816,6 @@ struct child_process *git_connect(int fd[2], const char *url,
int port_option = 'p';
char *ssh_host = hostandport;
const char *port = NULL;
- char *ssh_argv0 = NULL;
transport_check_allowed("ssh");
get_host_and_port(&ssh_host, &port);
@@ -812,15 +836,10 @@ struct child_process *git_connect(int fd[2], const char *url,
}
ssh = get_ssh_command();
- if (ssh) {
- char *split_ssh = xstrdup(ssh);
- const char **ssh_argv;
-
- if (split_cmdline(split_ssh, &ssh_argv))
- ssh_argv0 = xstrdup(ssh_argv[0]);
- free(split_ssh);
- free((void *)ssh_argv);
- } else {
+ if (ssh)
+ handle_ssh_variant(ssh, 1, &port_option,
+ &needs_batch);
+ else {
/*
* GIT_SSH is the no-shell version of
* GIT_SSH_COMMAND (and must remain so for
@@ -831,26 +850,12 @@ struct child_process *git_connect(int fd[2], const char *url,
ssh = getenv("GIT_SSH");
if (!ssh)
ssh = "ssh";
-
- ssh_argv0 = xstrdup(ssh);
+ else
+ handle_ssh_variant(ssh, 0,
+ &port_option,
+ &needs_batch);
}
- if (!handle_ssh_variant(&port_option, &needs_batch) &&
- ssh_argv0) {
- const char *base = basename(ssh_argv0);
-
- 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);
-
argv_array_push(&conn->args, ssh);
if (flags & CONNECT_IPV4)
argv_array_push(&conn->args, "-4");
--
2.11.1.windows.prerelease.2.9.g3014b57
^ permalink raw reply
* [PATCH v3 1/4] connect: handle putty/plink also in GIT_SSH_COMMAND
From: Johannes Schindelin @ 2017-02-01 12:01 UTC (permalink / raw)
To: git; +Cc: Segev Finer, Junio C Hamano, Jeff King
In-Reply-To: <cover.1485950225.git.johannes.schindelin@gmx.de>
From: Segev Finer <segev208@gmail.com>
Git for Windows has special support for the popular SSH client PuTTY:
when using PuTTY's non-interactive version ("plink.exe"), we use the -P
option to specify the port rather than OpenSSH's -p option. TortoiseGit
ships with its own, forked version of plink.exe, that adds support for
the -batch option, and for good measure we special-case that, too.
However, this special-casing of PuTTY only covers the case where the
user overrides the SSH command via the environment variable GIT_SSH
(which allows specifying the name of the executable), not
GIT_SSH_COMMAND (which allows specifying a full command, including
additional command-line options).
When users want to pass any additional arguments to (Tortoise-)Plink,
such as setting a private key, they are required to either use a shell
script named plink or tortoiseplink or duplicate the logic that is
already in Git for passing the correct style of command line arguments,
which can be difficult, error prone and annoying to get right.
This patch simply reuses the existing logic and expands it to cover
GIT_SSH_COMMAND, too.
Note: it may look a little heavy-handed to duplicate the entire
command-line and then split it, only to extract the name of the
executable. However, this is not a performance-critical code path, and
the code is much more readable this way.
Signed-off-by: Segev Finer <segev208@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
connect.c | 23 ++++++++++++++++-------
t/t5601-clone.sh | 15 +++++++++++++++
2 files changed, 31 insertions(+), 7 deletions(-)
diff --git a/connect.c b/connect.c
index 8cb93b0720..c81f77001b 100644
--- a/connect.c
+++ b/connect.c
@@ -772,6 +772,7 @@ struct child_process *git_connect(int fd[2], const char *url,
int putty = 0, tortoiseplink = 0;
char *ssh_host = hostandport;
const char *port = NULL;
+ char *ssh_argv0 = NULL;
transport_check_allowed("ssh");
get_host_and_port(&ssh_host, &port);
@@ -792,10 +793,15 @@ struct child_process *git_connect(int fd[2], const char *url,
}
ssh = get_ssh_command();
- if (!ssh) {
- const char *base;
- char *ssh_dup;
-
+ if (ssh) {
+ char *split_ssh = xstrdup(ssh);
+ const char **ssh_argv;
+
+ if (split_cmdline(split_ssh, &ssh_argv))
+ ssh_argv0 = xstrdup(ssh_argv[0]);
+ free(split_ssh);
+ free((void *)ssh_argv);
+ } else {
/*
* GIT_SSH is the no-shell version of
* GIT_SSH_COMMAND (and must remain so for
@@ -807,8 +813,11 @@ struct child_process *git_connect(int fd[2], const char *url,
if (!ssh)
ssh = "ssh";
- ssh_dup = xstrdup(ssh);
- base = basename(ssh_dup);
+ ssh_argv0 = xstrdup(ssh);
+ }
+
+ if (ssh_argv0) {
+ const char *base = basename(ssh_argv0);
tortoiseplink = !strcasecmp(base, "tortoiseplink") ||
!strcasecmp(base, "tortoiseplink.exe");
@@ -816,7 +825,7 @@ struct child_process *git_connect(int fd[2], const char *url,
!strcasecmp(base, "plink") ||
!strcasecmp(base, "plink.exe");
- free(ssh_dup);
+ free(ssh_argv0);
}
argv_array_push(&conn->args, ssh);
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 4241ea5b32..9335e10c2a 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -386,6 +386,21 @@ test_expect_success 'tortoiseplink is like putty, with extra arguments' '
expect_ssh "-batch -P 123" myhost src
'
+test_expect_success 'double quoted plink.exe in GIT_SSH_COMMAND' '
+ copy_ssh_wrapper_as "$TRASH_DIRECTORY/plink.exe" &&
+ GIT_SSH_COMMAND="\"$TRASH_DIRECTORY/plink.exe\" -v" \
+ git clone "[myhost:123]:src" ssh-bracket-clone-plink-3 &&
+ expect_ssh "-v -P 123" myhost src
+'
+
+SQ="'"
+test_expect_success 'single quoted plink.exe in GIT_SSH_COMMAND' '
+ copy_ssh_wrapper_as "$TRASH_DIRECTORY/plink.exe" &&
+ GIT_SSH_COMMAND="$SQ$TRASH_DIRECTORY/plink.exe$SQ -v" \
+ git clone "[myhost:123]:src" ssh-bracket-clone-plink-4 &&
+ expect_ssh "-v -P 123" myhost src
+'
+
# Reset the GIT_SSH environment variable for clone tests.
setup_ssh_wrapper
--
2.11.1.windows.prerelease.2.9.g3014b57
^ permalink raw reply related
* [PATCH v3 2/4] connect: rename tortoiseplink and putty variables
From: Johannes Schindelin @ 2017-02-01 12:01 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Junio C Hamano, Jeff King
In-Reply-To: <cover.1485950225.git.johannes.schindelin@gmx.de>
From: Junio C Hamano <gitster@pobox.com>
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.
[jes: wrapped overly-long line]
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
connect.c | 23 +++++++++++++----------
1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/connect.c b/connect.c
index c81f77001b..9f750eacb6 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,11 @@ 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.1.windows.prerelease.2.9.g3014b57
^ 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