* Re: [PATCH 5/6] Documentation/git-merge.txt: improve short description in DESCRIPTION
From: Sergey Organov @ 2016-10-06 12:30 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqa8eio4n9.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> Sergey Organov <sorganov@gmail.com> writes:
>
>> OK, I see. So, what is the best way to handle this? Immediately follow
>> content change patch with another patch that only re-flows?
>
> Or no reflowing at all.
>
>>> the parents". I do not know if the updated phrasing is better. The
>>> "name" in the original was meant to be a short-hand for "object name",
>>> and I would support a change to spell it out to clarify; "reference"
>>> can be a vague word that can mean different things in Git, and when
>>> the word is given without context, most Git people would think that
>>> the word refers to "refs", but that is definitely not what the new
>>> commit records, so...
>>
>> I won't insist on the change, but "name" sounded wrong to me, and
>> "reference" was most general term I was able to come up with in this
>> context.
>> ...
>> Last, if "reference" is not good enough and we get to internals anyway,
>> why not say SHA1 then?
>
> Because that is still colloquial? I think s/name/object name/ is a
> sensible change, but not s/name/reference/.
No, "reference" is more sensible here than any of "name", "object name",
or "SHA-1", the same way as here:
$ git help glossary
[...]
chain
A list of objects, where each object in the list contains a
reference to its successor (for example, the successor of a
commit could be one of its parents).
[...]
$
The resulting merge commit is an origin for 2 chains, so it stores 2
references to its successors. No need to be aware of any [object] names
to understand all this.
-- Sergey
^ permalink raw reply
* Re: [PATCH 6/6] Documentation/git-merge.txt: get rid of irrelevant references to git-pull
From: Sergey Organov @ 2016-10-06 12:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqq60p6o4js.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> Sergey Organov <sorganov@gmail.com> writes:
>
>> So I believe this change is inline with the rest of the patch. The
>> reference to git-pull (if it remains) should be a side-note, not part of
>> explanation of operation.
>
> Not really. The thing is, "This is the most common" needs to be
> close to "Often...". "git merge" directly invoked by the end user
> is much less likely to encounter a fast forward situation; getting
> invoked indirectly by "git pull" makes it common.
Ah, I now see. I tried to keep the text intact as much as possible, and
only split it into description and a note. Well, how about this then:
diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index b758d55..479400f 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -135,15 +135,17 @@ will exit early with the message "Already up-to-date."
FAST-FORWARD MERGE
------------------
-Often the current branch head is an ancestor of the named commit.
-This is the most common case especially when invoked from 'git
-pull': you are tracking an upstream repository, you have committed
-no local changes, and now you want to update to a newer upstream
-revision. In this case, a new commit is not needed to store the
+When current branch head is an ancestor of the named commit,
+a new commit is not needed to store the
combined history; instead, the `HEAD` (along with the index) is
updated to point at the named commit, without creating an extra
merge commit.
+This is very common case when 'git merge' is invoked from 'git
+pull': you are tracking an upstream repository, you have committed
+no local changes, and now you want to update to a newer upstream
+revision.
+
This behavior can be suppressed with the `--no-ff` option.
TRUE MERGE
-- Sergey
^ permalink raw reply related
* Re: [PATCH v2 20/25] sequencer: left-trim lines read from the script
From: Johannes Schindelin @ 2016-10-06 13:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jakub Narębski, Johannes Sixt
In-Reply-To: <xmqqr38p3z1i.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Mon, 12 Sep 2016, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> >> I do not offhand see why we want to be lenient here,
> >> especially only to the left.
> >
> > Postel's Law.
>
> How would that compare/relate to yagni, though?
I did need it, though. Blame it on being overworked or simply tired: I
ended up inserting a spurious space before a "fixup" command. This command
was handled as intended by the scripted rebase -i, and with the patch in
question the rebase--helper agreed, too.
Note: we have no problem to the right because we do handle variable-length
whitespace between command and SHA-1, and we do not care one iota about
the exact form of the oneline (hence right-stripping is not necessary).
Ciao,
Dscho
^ permalink raw reply
* [PATCH 2/2] Feature Request: user defined suffix for temp files created by git-mergetool
From: Josef Ridky @ 2016-10-06 13:09 UTC (permalink / raw)
To: git
In-Reply-To: <1911899288.2172724.1475757782111.JavaMail.zimbra@redhat.com>
This is the second of two variant for request to add option to change
suffix of name of temporary files generated by git mergetool. This
change is requested for cases, when is git mergetool used for local
comparison between two version of same package during package rebase.
Signed-off-by: Josef Ridky <jridky@redhat.com>
---
Documentation/git-mergetool.txt | 26 ++++++++++++++++++++++++-
git-mergetool.sh | 43 ++++++++++++++++++++++++++++++++++++-----
2 files changed, 63 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
index e846c2e..6cf5935 100644
--- a/Documentation/git-mergetool.txt
+++ b/Documentation/git-mergetool.txt
@@ -8,7 +8,7 @@ git-mergetool - Run merge conflict resolution tools to resolve merge conflicts
SYNOPSIS
--------
[verse]
-'git mergetool' [--tool=<tool>] [-y | --[no-]prompt] [<file>...]
+'git mergetool' [--tool=<tool>] [-y | --[no-]prompt] [--local=<name>] [--remote=<name>] [--backup=<name>] [--base=<name>] [<file>...]
DESCRIPTION
-----------
@@ -79,6 +79,30 @@ success of the resolution after the custom tool has exited.
Prompt before each invocation of the merge resolution program
to give the user a chance to skip the path.
+--local=<name>::
+ Use string from <name> as part of suffix of name of temporary
+ file (local) for merging. If not used or is equal with any
+ other (remote|backup|base), default value is used.
+ Default suffix is LOCAL.
+
+--remote=<name>::
+ Use string from <name> as part of suffix of name of temporary
+ file (remote) for merging. If not used or is equal with any
+ other (local|backup|base), default value is used.
+ Default suffix is REMOTE.
+
+--backup=<name>::
+ Use string from <name> as part of suffix of name of temporary
+ file (backup) for merging. If not used or is equal with any
+ other (local|remote|base), default value is used.
+ Default suffix is BACKUP.
+
+--base=<name>::
+ Use string from <name> as part of suffix of name of temporary
+ file (base) for merging. If not used or is equal with any
+ other (local|remote|backup), default value is used.
+ Default suffix is BASE.
+
TEMPORARY FILES
---------------
`git mergetool` creates `*.orig` backup files while resolving merges.
diff --git a/git-mergetool.sh b/git-mergetool.sh
index bf86270..096ee5e 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -8,7 +8,7 @@
# at the discretion of Junio C Hamano.
#
-USAGE='[--tool=tool] [--tool-help] [-y|--no-prompt|--prompt] [file to merge] ...'
+USAGE='[--tool=tool] [--tool-help] [-y|--no-prompt|--prompt] [--local=name] [--remote=name] [--backup=name] [--base=name] [file to merge] ...'
SUBDIRECTORY_OK=Yes
NONGIT_OK=Yes
OPTIONS_SPEC=
@@ -16,6 +16,12 @@ TOOL_MODE=merge
. git-sh-setup
. git-mergetool--lib
+# Can be changed by user
+LOCAL_NAME='LOCAL'
+BASE_NAME='BASE'
+BACKUP_NAME='BACKUP'
+REMOTE_NAME='REMOTE'
+
# Returns true if the mode reflects a symlink
is_symlink () {
test "$1" = 120000
@@ -271,10 +277,10 @@ merge_file () {
BASE=${BASE##*/}
fi
- BACKUP="$MERGETOOL_TMPDIR/${BASE}_BACKUP_$$$ext"
- LOCAL="$MERGETOOL_TMPDIR/${BASE}_LOCAL_$$$ext"
- REMOTE="$MERGETOOL_TMPDIR/${BASE}_REMOTE_$$$ext"
- BASE="$MERGETOOL_TMPDIR/${BASE}_BASE_$$$ext"
+ BACKUP="$MERGETOOL_TMPDIR/${BASE}_${BACKUP_NAME}_$$$ext"
+ LOCAL="$MERGETOOL_TMPDIR/${BASE}_${LOCAL_NAME}_$$$ext"
+ REMOTE="$MERGETOOL_TMPDIR/${BASE}_${REMOTE_NAME}_$$$ext"
+ BASE="$MERGETOOL_TMPDIR/${BASE}_${BASE_NAME}_$$$ext"
base_mode=$(git ls-files -u -- "$MERGED" | awk '{if ($3==1) print $1;}')
local_mode=$(git ls-files -u -- "$MERGED" | awk '{if ($3==2) print $1;}')
@@ -396,6 +402,33 @@ do
--prompt)
prompt=true
;;
+ --local=*)
+ temp_name=${1#--local=}
+ if [ "$temp_name" != "" ] && [ "$temp_name" != "$REMOTE_NAME" ] && [ "$temp_name" != "$BASE_NAME" ] && [ "$temp_name" != "$BACKUP_NAME" ]
+ then
+ LOCAL_NAME=$temp_name
+ fi
+ ;;
+ --remote=*)
+ temp_name=${1#--remote=}
+ if [ "$temp_name" != "" ] && [ "$temp_name" != "$LOCAL_NAME" ] && [ "$temp_name" != "$BASE_NAME" ] && [ "$temp_name" != "$BACKUP_NAME" ]
+ then
+ REMOTE_NAME=$temp_name
+ fi
+ ;;
+ --base=*)
+ temp_name=${1#--base=}
+ if [ "$temp_name" != "" ] && [ "$temp_name" != "$LOCAL_NAME" ] && [ "$temp_name" != "$REMOTE_NAME" ] && [ "$temp_name" != "$BACKUP_NAME" ]
+ then
+ BASE_NAME=$temp_name
+ fi
+ ;;
+ --backup=*)
+ temp_name=${1#--backup=}
+ if [ "$temp_name" != "" ] && [ "$temp_name" != "$LOCAL_NAME" ] && [ "$temp_name" != "$REMOTE_NAME" ] && [ "$temp_name" != "$BASE_NAME" ]
+ BACKUP_NAME=$temp_name
+ fi
+ ;;
--)
shift
break
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v8 00/11] Git filter protocol
From: Lars Schneider @ 2016-10-06 13:13 UTC (permalink / raw)
To: Jakub Narębski
Cc: Junio C Hamano, Torsten Bögershausen, git, Jeff King,
Stefan Beller, Martin-Louis Bright, Ramsay Jones
In-Reply-To: <f7f9ca4c-229c-390a-beb0-a58e0d3d66b3@gmail.com>
> On 04 Oct 2016, at 21:04, Jakub Narębski <jnareb@gmail.com> wrote:
>
> W dniu 03.10.2016 o 19:13, Lars Schneider pisze:
>>> On 01 Oct 2016, at 22:48, Jakub Narębski <jnareb@gmail.com> wrote:
>>> W dniu 01.10.2016 o 20:59, Lars Schneider pisze:
>>>> On 29 Sep 2016, at 23:27, Junio C Hamano <gitster@pobox.com> wrote:
>>>>> Lars Schneider <larsxschneider@gmail.com> writes:
>>>>>
>>>>> If the filter process refuses to die forever when Git told it to
>>>>> shutdown (by closing the pipe to it, for example), that filter
>>>>> process is simply buggy. I think we want users to become aware of
>>>>> that, instead of Git leaving it behind, which essentially is to
>>>>> sweep the problem under the rug.
>>>
>>> Well, it would be good to tell users _why_ Git is hanging, see below.
>>
>> Agreed. Do you think it is OK to write the message to stderr?
>
> On the other hand, this is why GIT_TRACE (and GIT_TRACE_PERFORMANCE)
> was invented for. We do not signal troubles with single-shot filters,
> so I guess doing it for multi-file filters is not needed.
I am on the fence with this one.
@Junio/Peff:
Where would you prefer to see a "Waiting for filter 'XYZ'... " message?
On stderr or via GIT_TRACE?
>
>>>>> I agree with what Peff said elsewhere in the thread; if a filter
>>>>> process wants to take time to clean things up while letting Git
>>>>> proceed, it can do its own process management, but I think it is
>>>>> sensible for Git to wait the filter process it directly spawned.
>>>>
>>>> To realize the approach above I prototyped the run-command patch below:
>>>>
>>>> I added an "exit_timeout" variable to the "child_process" struct.
>>>> On exit, Git will close the pipe to the process and wait "exit_timeout"
>>>> seconds until it kills the child process. If "exit_timeout" is negative
>>>> then Git will wait until the process is done.
>>>
>>> That might be good approach. Probably the default would be to wait.
>>
>> I think I would prefer a 2sec timeout or something as default. This way
>> we can ensure Git would not wait indefinitely for a buggy filter by default.
>
> Actually this waiting for multi-file filter is only about waiting for
> the shutdown process of the filter. The filter could still hang during
> processing a file, and git would hang too, if I understand it correctly.
Correct.
>> [...] this function is also used with the async struct...
>
> Hmmm... now I wonder if it is a good idea (similar treatment for
> single-file async-invoked filter, and multi-file pkt-line filters).
>
> For single-file one-shot filter (correct me if I am wrong):
>
> - git sends contents to filter, signals end with EOF
> (after process is started)
> - in an async process:
> - process is started
> - git reads contents from filter, until EOF
> - if process did not end, it is killed
>
>
> For multi-process pkt-line based filter (simplified):
>
> - process is started
> - handshake
> - for each file
> - file is send to filter process over pkt-line,
> end signalled with flush packet
> - git reads from filter from pkt-line, until flush
> - ...
>
>
> See how single-shot filter is sent EOF, though in different part
> of code. We need to signal multi-file filter that no more files
> will be coming. Simplest solution is to send EOF (we could send
> "command=shutdown" for example...) to filter, and wait for EOF
> from filter (or for "status=finished" and EOF).
That's what we do. EOF does signal the multi-filter to shutdown.
> For full patch, you would need also to add to Documentation/config.txt
Why config.txt?
Thanks,
Lars
^ permalink raw reply
* Re: [PATCH v8 11/11] convert: add filter.<driver>.process option
From: Lars Schneider @ 2016-10-06 13:16 UTC (permalink / raw)
To: Jakub Narębski
Cc: git, Jeff King, Junio C Hamano, Stefan Beller,
Martin-Louis Bright, Torsten Bögershausen, Ramsay Jones
In-Reply-To: <2c53b837-4690-dae8-60ff-e7907a9b13ef@gmail.com>
> On 04 Oct 2016, at 22:50, Jakub Narębski <jnareb@gmail.com> wrote:
>
> [Some of answers may get invalidated by v9]
>
> W dniu 30.09.2016 o 20:56, Lars Schneider pisze:
>>> On 27 Sep 2016, at 00:41, Jakub Narębski <jnareb@gmail.com> wrote:
>>>
>>>> +
>>>> +After the filter has processed a blob it is expected to wait for
>>>> +the next "key=value" list containing a command. Git will close
>>>> +the command pipe on exit. The filter is expected to detect EOF
>>>> +and exit gracefully on its own.
>
> Is this still true?
Yes
>>>
>>> Good to have it documented.
>>>
>>> Anyway, as it is Git command that spawns the filter driver process,
>>> assuming that the filter process doesn't daemonize itself, wouldn't
>>> the operating system reap it after its parent process, that is the
>>> git command it invoked, dies? So detecting EOF is good, but not
>>> strictly necessary for simple filter that do not need to free
>>> its resources, or can leave freeing resources to the operating
>>> system? But I may be wrong here.
>>
>> The filter process runs independent of Git.
>
> Ah. So without some way to tell long-lived filter process that
> it can shut down, because no further data will be incoming, or
> killing it by Git, it would hang indefinitely?
Yes
- Lars
^ permalink raw reply
* Re: [PATCH 5/6] Documentation/git-merge.txt: improve short description in DESCRIPTION
From: Sergey Organov @ 2016-10-06 13:21 UTC (permalink / raw)
To: Jakub Narębski; +Cc: git, Junio C Hamano
In-Reply-To: <e03a84ad-9aa6-8ada-5828-7b28f76baaaf@gmail.com>
Jakub Narębski <jnareb@gmail.com> writes:
> W dniu 05.10.2016 o 16:46, sorganov@gmail.com pisze:
>> From: Sergey Organov <sorganov@gmail.com>
>>
>> Old description had a few problems:
>>
>> - sounded as if commits have changes
>>
>> - stated that changes are taken since some "divergence point"
>> that was not defined.
>>
>> New description rather uses "common ancestor" and "merge base",
>> definitions of which are easily discoverable in the rest of GIT
>> documentation.
>
> This is a step in a good direction, but it has a few issues.
>
>>
>> Signed-off-by: Sergey Organov <sorganov@gmail.com>
>> ---
>> Documentation/git-merge.txt | 25 +++++++++++++++----------
>> 1 file changed, 15 insertions(+), 10 deletions(-)
>>
>> diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
>> index cc0329d..351b8fc 100644
>> --- a/Documentation/git-merge.txt
>> +++ b/Documentation/git-merge.txt
>> @@ -16,11 +16,16 @@ SYNOPSIS
>>
>> DESCRIPTION
>> -----------
>> -Incorporates changes from the named commits (since the time their
>> -histories diverged from the current branch) into the current
>> -branch. This command is used by 'git pull' to incorporate changes
>> -from another repository and can be used by hand to merge changes
>> -from one branch into another.
>> +
>> +Incorporates changes that lead to the named commits into the current
>> +branch, and joins corresponding histories. The best common ancestor of
>> +named commits and the current branch, called "merge base", is
>> +calculated, and then net changes taken from the merge base to
>> +the named commits are applied.
>
> The first sentence is all right; it reads better than the original
> without the introduced part in parentheses. The only minor issue
> is with "joins corresponding histories" - it is a good description,
> but may imply that the branch we are merging vanishes: it doesn't.
> But all in all, it is a good change.
I've got "joins corresponding histories" from original NAME section, so
it's likely to be good enough.
>
> Second sentence has some problems. First, while it is a good idea
> to use well defined term "merge base", I think writing "since the
> time their histories diverged" or "(which is the point where histories
> diverged)" would be a good plain language description; it was removed
> entirely in the proposal.
I was not sure about it myself, but it sounded as if it could be the
case that I might need to resolve the same conflicts again and again, as
changes are taken from some "divergence point" that is apparently fixed.
OTOH, "merge base" not only is well-defined term, but it also doesn't
sound as some fixed point in history.
>
> Second, while "common ancestor" and "least common ancestor" are well
> defined in mathematics of graphs, "best common ancestor" isn't...
> but this is what git-merge-base(1) documentation uses.
That's were I took it from indeed, git-merge-base manual page. I wanted
things we mention to be discoverable.
> Also, the "best common ancestor" doesn't need to be only one. There
> might be many such ancestors... though Git would generate then a
> virtual best common ancestor thanks to recursive merge strategy.
> And usually there is only one "best common ancestor", that is a single
> merge base. So this may need clarification, but it is not much of
> a problem.
>
> Third, and most important, is that "net changes taken from the merge
> base to the named commits are applied" is simply not true. The
> `git merge` command does not reapply changes - that is what rebase
> and cherry-pick do.
"are applied" meant to be generic enough in this context to cover, say,
"-s ours" as well. Perhaps "are integrated" you suggest below, or "are
incorporated" that is already used in the beginning of DESCRIPTION, is
better indeed.
As for merge vs rebase (or cherry-pick), the only essential difference
for me is that merge applies net (cumulative, integrated) changes, while
rebase applies each change sequentially, one commit to another. Please
correct me if I'm wrong.
BTW, one can imagine "rebase" merge strategy: make a new chain of
commits on top of destination, then squash all that to get a new single
merge commit.
> The merge operation uses 3-way merge strategy (diff3) between
> merge-base, current branch, and merged commit. That is, it finds
> differences between differences, and "applies" that.
The problem is that at this point we need to cover other merge
strategies as well, so we can't writ the description as if 3-way merge
is the only strategy.
> See
> "A Formal Investigation of Diff3" paper by Sanjeev Khanna, Keshav
> Kunal, and Benjamin C. Pierce:
> http://www.cis.upenn.edu/~bcpierce/papers/diff3-short.pdf
>
> I'm not sure how to explain it succintly. Perhaps
>
> net changes between merge base to the current (merged into)
> branch and named commits are integrated
>
> There is description of trivial 3-way merge somewhere in Git docs,
> though in very unobvious place; we can link it.
As it's "TRUE MERGE" section that mentions 3-way merge for the first
time, it'd be the natural place for the link, I think.
>> +
>> +This command is used by 'git pull' to incorporate changes from another
>> +repository, and can be used by hand to merge changes from one branch
>> +into another.
>
> Rather "can be used by 'git pull'", or "is used by 'git pull' (unless
> configured otherwise)"...
>
> Separating this information makes a very good sense. Thanks.
Yeah, but I've rather deleted it entirely in 6/6.
-- Sergey.
^ permalink raw reply
* [PATCH 1/2] Feature Request: user defined suffix for temp files created by git-mergetool
From: Josef Ridky @ 2016-10-06 12:43 UTC (permalink / raw)
To: git
In-Reply-To: <1214659824.1976049.1475738509473.JavaMail.zimbra@redhat.com>
This is the first of two variant for request to add option to change
suffix of name of temporary files generated by git mergetool. This
change is requested for cases, when is git mergetool used for local
comparision between two version of same package during package rebase.
Signed-off-by: Josef Ridky <jridky@redhat.com>
---
Documentation/git-mergetool.txt | 7 ++++++-
git-mergetool.sh | 23 ++++++++++++++++++-----
2 files changed, 24 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
index e846c2e..a212cef 100644
--- a/Documentation/git-mergetool.txt
+++ b/Documentation/git-mergetool.txt
@@ -8,7 +8,7 @@ git-mergetool - Run merge conflict resolution tools to resolve merge conflicts
SYNOPSIS
--------
[verse]
-'git mergetool' [--tool=<tool>] [-y | --[no-]prompt] [<file>...]
+'git mergetool' [--tool=<tool>] [-y | --[no-]prompt] [-l | --local] [<file>...]
DESCRIPTION
-----------
@@ -79,6 +79,11 @@ success of the resolution after the custom tool has exited.
Prompt before each invocation of the merge resolution program
to give the user a chance to skip the path.
+-l::
+--local::
+ Change suffix of name of temporary files generated by git
+ mergetool from `_REMOTE_` and `_LOCAL_` to `_OLD_` and `_NEW_`.
+
TEMPORARY FILES
---------------
`git mergetool` creates `*.orig` backup files while resolving merges.
diff --git a/git-mergetool.sh b/git-mergetool.sh
index bf86270..1e04368 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -8,10 +8,11 @@
# at the discretion of Junio C Hamano.
#
-USAGE='[--tool=tool] [--tool-help] [-y|--no-prompt|--prompt] [file to merge] ...'
+USAGE='[--tool=tool] [--tool-help] [-y|--no-prompt|--prompt] [-l|--local] [file to merge] ...'
SUBDIRECTORY_OK=Yes
NONGIT_OK=Yes
OPTIONS_SPEC=
+LOCAL_MODE=false
TOOL_MODE=merge
. git-sh-setup
. git-mergetool--lib
@@ -271,10 +272,19 @@ merge_file () {
BASE=${BASE##*/}
fi
- BACKUP="$MERGETOOL_TMPDIR/${BASE}_BACKUP_$$$ext"
- LOCAL="$MERGETOOL_TMPDIR/${BASE}_LOCAL_$$$ext"
- REMOTE="$MERGETOOL_TMPDIR/${BASE}_REMOTE_$$$ext"
- BASE="$MERGETOOL_TMPDIR/${BASE}_BASE_$$$ext"
+ if test "$LOCAL_MODE"
+ then
+
+ BACKUP="$MERGETOOL_TMPDIR/${BASE}_BACKUP_$$$ext"
+ LOCAL="$MERGETOOL_TMPDIR/${BASE}_NEW_$$$ext"
+ REMOTE="$MERGETOOL_TMPDIR/${BASE}_OLD_$$$ext"
+ BASE="$MERGETOOL_TMPDIR/${BASE}_BASE_$$$ext"
+ else
+ BACKUP="$MERGETOOL_TMPDIR/${BASE}_BACKUP_$$$ext"
+ LOCAL="$MERGETOOL_TMPDIR/${BASE}_LOCAL_$$$ext"
+ REMOTE="$MERGETOOL_TMPDIR/${BASE}_REMOTE_$$$ext"
+ BASE="$MERGETOOL_TMPDIR/${BASE}_BASE_$$$ext"
+ fi
base_mode=$(git ls-files -u -- "$MERGED" | awk '{if ($3==1) print $1;}')
local_mode=$(git ls-files -u -- "$MERGED" | awk '{if ($3==2) print $1;}')
@@ -396,6 +406,9 @@ do
--prompt)
prompt=true
;;
+ -l|--local)
+ LOCAL_MODE=true
+ ;;
--)
shift
break
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v2 24/25] sequencer: quote filenames in error messages
From: Johannes Schindelin @ 2016-10-06 13:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jakub Narębski, Johannes Sixt
In-Reply-To: <xmqq60q26mfw.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Sun, 11 Sep 2016, Junio C Hamano wrote:
> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>
> > This makes the code consistent by fixing quite a couple of error messages.
>
> Looks OK. While at it, we may want another one to downcase the
> first word, perhaps?
>
> These may not be messages added by your series and can be left
> outside this series, but I have to point out that
>
> if (commit_lock_file(&msg_file) < 0)
> return error(_("Error wrapping up '%s'."), filename);
>
> results in "error: Error wrapping up", which sounds quite funny.
>
> "failed to finalize" or something would flow a bit better, I'd say.
Fair enough. I added a patch to make it so.
Thanks,
Dscho
^ permalink raw reply
* Fork Errors
From: Vacha, Brian [USA] @ 2016-10-06 14:02 UTC (permalink / raw)
To: git@vger.kernel.org
When starting Git Bash, I receive the following errors:
0 [main] bash 18088 fork: child 14072 - died waiting for dll loading, errno 11
bash: fork: retry: No child processes
1190419 [main] bash 18088 fork: child 8744 - died waiting for dll loading, errno 11
bash: fork: retry: No child processes
3343518 [main] bash 18088 fork: child 12324 - died waiting for dll loading, errno 11
bash: fork: retry: No child processes
7480858 [main] bash 18088 fork: child 17008 - died waiting for dll loading, errno 11
bash: fork: retry: No child processes
15635036 [main] bash 18088 fork: child 8108 - died waiting for dll loading, errno 11
bash: fork: Resource temporarily unavailable
bash-4.3$
My connection is great at 72 Mbps download and 93 Mbps upload. I don't receive other errors so it appears to be a Git Bash issue to me.
Thanks,
Brian
^ permalink raw reply
* Re: [PATCH v2 25/25] sequencer: remove bogus hint for translators
From: Johannes Schindelin @ 2016-10-06 14:18 UTC (permalink / raw)
To: Junio C Hamano
Cc: Ævar Arnfjörð Bjarmason, git, Jakub Narębski,
Johannes Sixt
In-Reply-To: <xmqqa8fe6mln.fsf@gitster.mtv.corp.google.com>
[-- Attachment #1: Type: text/plain, Size: 1534 bytes --]
Hi Junio,
On Sun, 11 Sep 2016, Junio C Hamano wrote:
> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>
> > When translating error messages, we need to be careful *not* to translate
> > the todo commands such as "pick", "reword", etc because they are commands,
> > and Git would not understand translated versions of those commands.
> >
> > Therefore, translating the commands in the error messages would simply be
> > misleading.
>
> This comes from b9c993e0 ("i18n: git-revert literal "me" messages",
> 2011-02-22) where it said
>
> Author: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>
> i18n: git-revert literal "me" messages
>
> Translate messages that use the `me' variable. These are all error
> messages referencing the command name, so the name shouldn't be
> translated.
>
> Reported-by: Jonathan Nieder <jrnieder@gmail.com>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
>
> This looks like a positive attempt to remind translators that their
> translation must flow with these literal command names that will not
> get translated in place, not a bogus hint at all, at least to me.
Alright, I misread it to mean: "You need to translate these terms".
I fixed it by skipping this patch, and amending the "revamp the todo
parsing" patch: the first %s now actually refers to the much more helpful
todo command rather than the overall Git command.
Thanks,
Dscho
^ permalink raw reply
* Re: Fork Errors
From: Konstantin Khomoutov @ 2016-10-06 14:40 UTC (permalink / raw)
To: Vacha, Brian [USA]; +Cc: git@vger.kernel.org
In-Reply-To: <2E2ADD8A4AA3FA44A8D8FB00E2B24154B5E9E1C6@ASHBDAG1M3.resource.ds.bah.com>
On Thu, 6 Oct 2016 14:02:09 +0000
"Vacha, Brian [USA]" <vacha_brian@bah.com> wrote:
> When starting Git Bash, I receive the following errors:
> 0 [main] bash 18088 fork: child 14072 - died waiting for dll loading,
> errno 11 bash: fork: retry: No child processes
> 1190419 [main] bash 18088 fork: child 8744 - died waiting for dll
> loading, errno 11 bash: fork: retry: No child processes
> 3343518 [main] bash 18088 fork: child 12324 - died waiting for dll
> loading, errno 11 bash: fork: retry: No child processes
> 7480858 [main] bash 18088 fork: child 17008 - died waiting for dll
> loading, errno 11 bash: fork: retry: No child processes
> 15635036 [main] bash 18088 fork: child 8108 - died waiting for dll
> loading, errno 11 bash: fork: Resource temporarily unavailable
> bash-4.3$
>
> My connection is great at 72 Mbps download and 93 Mbps upload. I
> don't receive other errors so it appears to be a Git Bash issue to me.
Have you tried searching through Git for Windows bugtracker [1] for
your problem. I'm pretty sure it was recently discussed there.
The issue #776 [2] looks like the one you're experiencing.
1. https://github.com/git-for-windows/git/issues
2. https://github.com/git-for-windows/git/issues/776
^ permalink raw reply
* Re: [PATCH v9 04/14] run-command: add wait_on_exit
From: Junio C Hamano @ 2016-10-06 15:53 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Lars Schneider, git, ramsay, jnareb, j6t, tboegi, peff, mlbright
In-Reply-To: <alpine.DEB.2.20.1610061129430.35196@virtualbox>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> Hi Junio & Lars,
>
> On Wed, 5 Oct 2016, Junio C Hamano wrote:
>
>> Lars Schneider <larsxschneider@gmail.com> writes:
>>
>> > OK. Something like the patch below would work nicely.
>>
>> Yeah, something along that line; it would eliminate the need to
>> worry about a field named "stdin" ;-)
>
> Not only a need to worry....
Thanks, but I (and more importantly Lars, too) knew that stdin is
problematic when I sent the message you are responding to, as raised
by Ramsay in a separate thread:
<f9b6abf6-8305-4a32-29af-765f08fee94b@ramsayjones.plus.com>
^ permalink raw reply
* Re: Feature Request: user defined suffix for temp files created by git-mergetool
From: Junio C Hamano @ 2016-10-06 15:54 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Josef Ridky, git
In-Reply-To: <e3306f5a-1fb3-bd66-48ac-72b75fc7681c@kdbg.org>
Johannes Sixt <j6t@kdbg.org> writes:
> Therefore, I think that your patch as written does not help to reduce
> the confusion. It may be a building block for further improvement, but
> if you stop here, it is pointless.
Yup, you're right.
^ permalink raw reply
* Re: [PATCH v8 00/11] Git filter protocol
From: Jeff King @ 2016-10-06 16:01 UTC (permalink / raw)
To: Lars Schneider
Cc: Jakub Narębski, Junio C Hamano, Torsten Bögershausen,
git, Stefan Beller, Martin-Louis Bright, Ramsay Jones
In-Reply-To: <5C30800A-C821-4B39-BEB7-7D55D86BD7EA@gmail.com>
On Thu, Oct 06, 2016 at 03:13:19PM +0200, Lars Schneider wrote:
> >>> Well, it would be good to tell users _why_ Git is hanging, see below.
> >>
> >> Agreed. Do you think it is OK to write the message to stderr?
> >
> > On the other hand, this is why GIT_TRACE (and GIT_TRACE_PERFORMANCE)
> > was invented for. We do not signal troubles with single-shot filters,
> > so I guess doing it for multi-file filters is not needed.
>
> I am on the fence with this one.
>
> @Junio/Peff:
> Where would you prefer to see a "Waiting for filter 'XYZ'... " message?
> On stderr or via GIT_TRACE?
I am not sure if I have followed all of this discussion, but I am of the
opinion that Git should not be doing any timeouts at all. There are
simply too many places where the filter (or any other process that git
is depending on) could inexplicably hang, and I
do not want to pepper random timeouts for all parts of the procedure
where we say "woah, this is taking longer than expected" (nor do I want
to have a timeout for _one_ spot, and ignore all the others).
If this is debugging output of the form "now I am calling wait() on all
of the filters", without respect to any timeout, that sounds reasonable.
Though I would argue that run-command should simply trace_printf()
any processes it is waiting for, which covers _any_ process, not just
the filters. That seems like a good match for the rest of the GIT_TRACE
output, which describes how and when we spawn the sub-processes.
Something like:
diff --git a/run-command.c b/run-command.c
index 5a4dbb6..b884605 100644
--- a/run-command.c
+++ b/run-command.c
@@ -226,6 +226,9 @@ static int wait_or_whine(pid_t pid, const char *argv0, int in_signal)
pid_t waiting;
int failed_errno = 0;
+ if (!in_signal)
+ trace_printf("waiting for pid %d", (int)pid);
+
while ((waiting = waitpid(pid, &status, 0)) < 0 && errno == EINTR)
; /* nothing */
if (in_signal)
but it does not have to be part of this series, I think.
-Peff
^ permalink raw reply related
* Re: Bug Report: "git submodule deinit" fails right after a clone
From: Thomas Bétous @ 2016-10-06 16:10 UTC (permalink / raw)
To: Heiko Voigt; +Cc: git, msysGit Mailinglist
In-Reply-To: <20161006092040.GA38550@book.hvoigt.net>
Hello,
Sorry again for the mailing list...
On Thu, Oct 6, 2016 at 11:20 AM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
> So I guess the same applies to 'git status'?
No, it is the strange thing.
As told in my very first message here what happens after git diff and
git status:
$ git clone https://github.com/githubtraining/example-dependency.git
Cloning into 'example-dependency'...
remote: Counting objects: 35, done.
remote: Total 35 (delta 0), reused 0 (delta 0), pack-reused 35
Unpacking objects: 100% (35/35), done.
Checking connectivity... done.
$ cd example-dependency
$ git submodule deinit js
fatal: Please stage your changes to .gitmodules or stash them to proceed
Submodule work tree 'js' contains local modifications; use '-f' to discard them
$ git diff
[no output]
$ git submodule deinit js
fatal: Please stage your changes to .gitmodules or stash them to proceed
Submodule work tree 'js' contains local modifications; use '-f' to discard them
$ git status
On branch master
Your branch is up-to-date with 'origin/master'.
nothing to commit, working directory clean
$ git submodule deinit js
Cleared directory 'js'
So as you can see, the 'git status' makes the problem magically disappear.
Thomas
^ permalink raw reply
* Re: [PATCH] push: Re-include "push.default=tracking" in the documentation
From: Ævar Arnfjörð Bjarmason @ 2016-10-06 16:18 UTC (permalink / raw)
To: Matthieu Moy; +Cc: Git, Junio C Hamano, Johan Herland, Ramkumar Ramachandra
In-Reply-To: <vpq1sztsmji.fsf@anie.imag.fr>
On Thu, Oct 6, 2016 at 2:13 PM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
> Ęvar Arnfjörš Bjarmason <avarab@gmail.com> writes:
>
>> That's bad, either we shouldn't support it at all, or we should
>> document what it does. This patch does the latter.
>
> I vaguely remember a similar discussion and probably even a patch in the
> past (maybe by you actually). I think the proposal was to add a mention
> of tracking but avoid promoting it at the same level as the others.
>
> I have a slight preference for a patch adding stg like "`tracking` is a
> deprecated alias supported only for backward compatibility" to the item
> of `upstream`, but I'm OK with the current patch too.
You're right! I wasn't trying to be sneaky here, but apparently I just
keep running into the same things. We talked about this back in 2012,
kicked off by you proposing a patch to remove it completely from
config.txt and me submitting this:
http://www.spinics.net/lists/git/msg198264.html
After some back & forth Junio ended up applying fa23348 to address
that, but that was wiped away just a few months later in 87a70e4.
I'd just like it mentioned in the docs in some way, because I *did*
run into exactly the situation I described in my E-Mail back in 2012,
i.e. found a repo with push.default=tracking and couldn't find any
mention of what it did in the docs.
Junio, looks like from the 2013 discussion that you preferred just
having that mention in parenthesis instead of its own item, how about
just re-applying your fa23348 (with conflicts resolved)?
>> --- a/Documentation/config.txt
>> +++ b/Documentation/config.txt
>> @@ -2344,6 +2344,10 @@ push.default::
>> pushing to the same repository you would normally pull from
>> (i.e. central workflow).
>>
>> +* `tracking` - Deprecated synonym for `upstream`, which we still
>> + support for backwards compatibility with existing configuration
>> + files.
>
> Nit: I think the doc normally doesn't use "we" this way (we = the Git
> developers or the Git tool). Hence my s/which we still support/still
> supported/ above.
>
> --
> Matthieu Moy
> http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [PATCH v2 20/25] sequencer: left-trim lines read from the script
From: Johannes Sixt @ 2016-10-06 16:23 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, git, Jakub Narębski
In-Reply-To: <alpine.DEB.2.20.1610061459390.35196@virtualbox>
Am 06.10.2016 um 15:08 schrieb Johannes Schindelin:
> Hi Junio,
>
> On Mon, 12 Sep 2016, Junio C Hamano wrote:
>
>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>>
>>>> I do not offhand see why we want to be lenient here,
>>>> especially only to the left.
>>>
>>> Postel's Law.
>>
>> How would that compare/relate to yagni, though?
>
> I did need it, though. Blame it on being overworked or simply tired: I
> ended up inserting a spurious space before a "fixup" command. This command
> was handled as intended by the scripted rebase -i, and with the patch in
> question the rebase--helper agreed, too.
>
> Note: we have no problem to the right because we do handle variable-length
> whitespace between command and SHA-1, and we do not care one iota about
> the exact form of the oneline (hence right-stripping is not necessary).
The last sentence is not entirely correct. See the patch below. You
may pick up the idea in one form or another, or just queue the patch
near the end of your series.
Let me take the opportunity to say
Thank You Very Much!
for the new implementation of rebase -i. I'm using your branch
rebase-i-extra, and it is such a pleasure to work with on Windows
compared to the old fully scripted version.
---- 8< ----
[PATCH] sequencer: strip CR from the end of exec insns
It is not unheard of that editors on Windows write CRLF even if the file
originally had only LF. This is particularly awkward for exec lines of a
rebase -i todo sheet. Take for example the insn "exec echo": The shell
script parser (either of the sequencer or of the shell that is invoked,
I do not know) splits at the LF and leaves the CR attached to "echo",
which leads to the unknown command "echo\r".
Work it around by stripping CR before the command specified with exec is
passed to the shell.
This happens to fix t9903.14 and .15 in my environment: the insn sheet
constructed here contains CRLF thanks to MSYS1's bash cleverness.
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
sequencer.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/sequencer.c b/sequencer.c
index daf8f13..6961820 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2005,8 +2005,11 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
}
else if (item->command == TODO_EXEC) {
char *end_of_arg = (char *)(item->arg + item->arg_len);
- int saved = *end_of_arg;
+ int saved;
+ while (end_of_arg != item->arg && end_of_arg[-1] == '\r')
+ end_of_arg--;
+ saved = *end_of_arg;
*end_of_arg = '\0';
res = do_exec(item->arg);
*end_of_arg = saved;
--
2.10.0.343.g37bc62b
^ permalink raw reply related
* Re: [PATCH] push: Re-include "push.default=tracking" in the documentation
From: Matthieu Moy @ 2016-10-06 16:27 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: Git, Junio C Hamano, Johan Herland, Ramkumar Ramachandra
In-Reply-To: <CACBZZX5ycZbFrrVgr4PVy5pKQDbnvmvDxuephx7-uHzzcm3u=g@mail.gmail.com>
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> Junio, looks like from the 2013 discussion that you preferred just
> having that mention in parenthesis instead of its own item, how about
> just re-applying your fa23348 (with conflicts resolved)?
(fa23348 did this:
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1749,7 +1749,8 @@ push.default::
+
This is currently the default, but Git 2.0 will change the default
to `simple`.
-* `upstream` - push the current branch to its upstream branch.
+* `upstream` - push the current branch to its upstream branch
+ (`tracking` is a deprecated synonym for this).
)
I agree that doing the same thing is the best option.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* [PATCH 0/2] fix infinite loop in ref resolution
From: Jeff King @ 2016-10-06 16:47 UTC (permalink / raw)
To: git; +Cc: Michael Haggerty
This fixes an infinite loop bug dating back to the v1.8.x era.
Triggering it requires creating a broken symbolic link in the .git
directory, so I don't think it's security-interesting. It should apply
cleanly on "maint".
[1/2]: files_read_raw_ref: avoid infinite loop on broken symlinks
[2/2]: files_read_raw_ref: prevent infinite retry loops in general
refs/files-backend.c | 14 +++++++++++++-
t/t1503-rev-parse-verify.sh | 5 +++++
2 files changed, 18 insertions(+), 1 deletion(-)
^ permalink raw reply
* [PATCH 2/2] files_read_raw_ref: prevent infinite retry loops in general
From: Jeff King @ 2016-10-06 16:48 UTC (permalink / raw)
To: git; +Cc: Michael Haggerty
In-Reply-To: <20161006164723.ocg2nbgtulpjcksp@sigill.intra.peff.net>
Limit the number of retries to 3. That should be adequate to
prevent any races, while preventing the possibility of
infinite loops if the logic fails to handle any other
possible error modes correctly.
After the fix in the previous commit, there's no known way
to trigger an infinite loop, but I did manually verify that
this fixes the test in that commit even when the code change
is not applied.
Signed-off-by: Jeff King <peff@peff.net>
---
refs/files-backend.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index d826557..76e5902 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1353,6 +1353,7 @@ static int files_read_raw_ref(struct ref_store *ref_store,
int fd;
int ret = -1;
int save_errno;
+ int remaining_retries = 3;
*type = 0;
strbuf_reset(&sb_path);
@@ -1373,8 +1374,14 @@ static int files_read_raw_ref(struct ref_store *ref_store,
* <-> symlink) between the lstat() and reading, then
* we don't want to report that as an error but rather
* try again starting with the lstat().
+ *
+ * We'll keep a count of the retries, though, just to avoid
+ * any confusing situation sending us into an infinite loop.
*/
+ if (remaining_retries-- <= 0)
+ goto out;
+
if (lstat(path, &st) < 0) {
if (errno != ENOENT)
goto out;
--
2.10.1.506.g904834d
^ permalink raw reply related
* [PATCH 1/2] files_read_raw_ref: avoid infinite loop on broken symlinks
From: Jeff King @ 2016-10-06 16:48 UTC (permalink / raw)
To: git; +Cc: Michael Haggerty
In-Reply-To: <20161006164723.ocg2nbgtulpjcksp@sigill.intra.peff.net>
Our ref resolution first runs lstat() on any path we try to
look up, because we want to treat symlinks specially (by
resolving them manually and considering them symrefs). But
if the results of `readlink` do _not_ look like a ref, we
fall through to treating it like a normal file, and just
read the contents of the linked path.
Since fcb7c76 (resolve_ref_unsafe(): close race condition
reading loose refs, 2013-06-19), that "normal file" code
path will stat() the file and if we see ENOENT, will jump
back to the lstat(), thinking we've seen inconsistent
results between the two calls. But for a symbolic ref, this
isn't a race: the lstat() found the symlink, and the stat()
is looking at the path it points to. We end up in an
infinite loop calling lstat() and stat().
We can fix this by avoiding the retry-on-inconsistent jump
when we know that we found a symlink. While we're at it,
let's add a comment explaining why the symlink case gets to
this code in the first place; without that, it is not
obvious that the correct solution isn't to avoid the stat()
code path entirely.
Signed-off-by: Jeff King <peff@peff.net>
---
refs/files-backend.c | 7 ++++++-
t/t1503-rev-parse-verify.sh | 5 +++++
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 0709f60..d826557 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1403,6 +1403,11 @@ static int files_read_raw_ref(struct ref_store *ref_store,
ret = 0;
goto out;
}
+ /*
+ * It doesn't look like a refname; fall through to just
+ * treating it like a non-symlink, and reading whatever it
+ * points to.
+ */
}
/* Is it a directory? */
@@ -1426,7 +1431,7 @@ static int files_read_raw_ref(struct ref_store *ref_store,
*/
fd = open(path, O_RDONLY);
if (fd < 0) {
- if (errno == ENOENT)
+ if (errno == ENOENT && !S_ISLNK(st.st_mode))
/* inconsistent with lstat; retry */
goto stat_ref;
else
diff --git a/t/t1503-rev-parse-verify.sh b/t/t1503-rev-parse-verify.sh
index ab27d0d..69d5135 100755
--- a/t/t1503-rev-parse-verify.sh
+++ b/t/t1503-rev-parse-verify.sh
@@ -139,4 +139,9 @@ test_expect_success 'master@{n} for various n' '
test_must_fail git rev-parse --verify master@{$Np1}
'
+test_expect_success SYMLINKS 'ref resolution not confused by broken symlinks' '
+ ln -s does-not-exist .git/broken &&
+ test_must_fail git rev-parse --verify broken
+'
+
test_done
--
2.10.1.506.g904834d
^ permalink raw reply related
* Re: Feature Request: user defined suffix for temp files created by git-mergetool
From: Junio C Hamano @ 2016-10-06 16:58 UTC (permalink / raw)
To: Josef Ridky; +Cc: Johannes Sixt, git
In-Reply-To: <1214659824.1976049.1475738509473.JavaMail.zimbra@redhat.com>
Josef Ridky <jridky@redhat.com> writes:
> I agree, that this patch is written as general as possible and can
> possibly bring more confusion than benefits.
I am not sure about that. Other people would have similar but
different workflow needs where they compare local new one with local
old one that would be helped by renaming local to old and remote to
new (i.e. the other way around from your need). If you just add a
toggle between local-remote vs new-old, that would be just an
additional code baggage that does not help people other than you.
I think J6t's "EDIT THIS" hits the center of the issue. If users
are trained to know LOCAL is the one to be edited, would the current
UI work well enough for them thru your custom workflow tools? If we
rename LOCAL to "EDIT THIS" and do nothing else, would such UI work
well for even untrained users thru your custom workflow tools?
^ permalink raw reply
* Re: [PATCH 2/2] Feature Request: user defined suffix for temp files created by git-mergetool
From: Junio C Hamano @ 2016-10-06 17:06 UTC (permalink / raw)
To: Josef Ridky; +Cc: git
In-Reply-To: <255814448.2197583.1475759366093.JavaMail.zimbra@redhat.com>
Josef Ridky <jridky@redhat.com> writes:
> + --local=*)
> + temp_name=${1#--local=}
> + if [ "$temp_name" != "" ] && [ "$temp_name" != "$REMOTE_NAME" ] && [ "$temp_name" != "$BASE_NAME" ] && [ "$temp_name" != "$BACKUP_NAME" ]
> + then
> + LOCAL_NAME=$temp_name
> + fi
It is not a good idea to ignore an explicit user request without
giving any indication and without giving any explanation.
You may have noticed that we do not say "[ cond ]" in this shell
script (we say "test" instead; see Documentation/CodingGuidelines).
Instead of having such a test all over the place, I'd suggest doing
it outside the loop:
while test $# != 0
do
case "$1" in
...
--local=*)
LOCAL_NAME=${1#--local=} ;;
--remote=*)
REMOTE_NAME=${1#--local=} ;;
...
esac
done
# sanity check _after_ parsing the command line
case ",$LOCAL_NAME,$REMOTE_NAME,$BASE_NAME,$BACKUP_NAME," in
*,,*) die "You cannot set --local/remote/... to empty" ;;
esac
... similarly, duplicate check comes here ...
^ permalink raw reply
* Re: [PATCH v8 00/11] Git filter protocol
From: Junio C Hamano @ 2016-10-06 17:17 UTC (permalink / raw)
To: Jeff King
Cc: Lars Schneider, Jakub Narębski, Torsten Bögershausen,
git, Stefan Beller, Martin-Louis Bright, Ramsay Jones
In-Reply-To: <20161006160104.nhpxhfhketr4rb4h@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I am not sure if I have followed all of this discussion, but I am of the
> opinion that Git should not be doing any timeouts at all.
> ...
> If this is debugging output of the form "now I am calling wait() on all
> of the filters", without respect to any timeout, that sounds reasonable.
> Though I would argue that run-command should simply trace_printf()
> any processes it is waiting for, which covers _any_ process, not just
> the filters. That seems like a good match for the rest of the GIT_TRACE
> output, which describes how and when we spawn the sub-processes.
Yup, I agree that having trace_printf() report the wait for any
process is the cleanest way to go. As you guessed the reason why
Lars is bringing up "now we are waiting for this filter" is because
I suggested it as a way to encourage users to file bugs when they
see a hung Git. Originally Lars wanted to have a timeout on wait
and after the timeout wanted to kill the process, and because I
really did not want such a random "you are too slow to die, so I'll
send a signal to you and exit myself without making sure you died"
there, I suggested that if we were to have a timeout, that would be
to timeout the wait only to have a chance to tell the user "we are
stuck waiting on this thing" (and then go back to wait), as it would
either be a buggy filter (i.e. the users need to debug their own
filter code) or a buggy use of wait on Git side (i.e. we would want
to hear about such bugs from them).
Without such a "wait with timeout so that we can tell the user", we
can still respond to "my 'git checkout' hangs forever" with "try
running with GIT_TRACE" as you outlined above, so I do not think we
need the timeout.
Thanks for straightening us out.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox