* [PATCH v2 00/11] git worktree (re)move
From: Nguyễn Thái Ngọc Duy @ 2016-11-28 9:43 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <20161112022337.13317-1-pclouds@gmail.com>
v2 contains some style fix and adapts to the new get_worktrees() api
from nd/worktree-list-fixup (which means it can't be built without
that series).
Nguyễn Thái Ngọc Duy (11):
copy.c: import copy_file() from busybox
copy.c: delete unused code in copy_file()
copy.c: convert bb_(p)error_msg to error(_errno)
copy.c: style fix
copy.c: convert copy_file() to copy_dir_recursively()
worktree.c: add validate_worktree()
worktree.c: add update_worktree_location()
worktree move: new command
worktree move: accept destination as directory
worktree move: refuse to move worktrees with submodules
worktree remove: new command
Documentation/git-worktree.txt | 28 ++-
builtin/worktree.c | 181 ++++++++++++++++
cache.h | 1 +
contrib/completion/git-completion.bash | 5 +-
copy.c | 369 +++++++++++++++++++++++++++++++++
t/t2028-worktree-move.sh | 56 +++++
worktree.c | 84 ++++++++
worktree.h | 11 +
8 files changed, 724 insertions(+), 11 deletions(-)
--
2.8.2.524.g6ff3d78
^ permalink raw reply
* [PATCH v2 4/5] worktree.c: get_worktrees() takes a new flag argument
From: Nguyễn Thái Ngọc Duy @ 2016-11-28 9:36 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, rappazzo, Nguyễn Thái Ngọc Duy
In-Reply-To: <20161128093656.15744-1-pclouds@gmail.com>
This is another no-op patch, in preparation for get_worktrees() to do
optional things, like sorting.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
branch.c | 2 +-
builtin/branch.c | 2 +-
builtin/worktree.c | 6 +++---
worktree.c | 4 ++--
worktree.h | 2 +-
5 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/branch.c b/branch.c
index 0d459b3..c431cbf 100644
--- a/branch.c
+++ b/branch.c
@@ -348,7 +348,7 @@ void die_if_checked_out(const char *branch, int ignore_current_worktree)
int replace_each_worktree_head_symref(const char *oldref, const char *newref)
{
int ret = 0;
- struct worktree **worktrees = get_worktrees();
+ struct worktree **worktrees = get_worktrees(0);
int i;
for (i = 0; worktrees[i]; i++) {
diff --git a/builtin/branch.c b/builtin/branch.c
index 60cc5c8..4757075 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -531,7 +531,7 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin
static void reject_rebase_or_bisect_branch(const char *target)
{
- struct worktree **worktrees = get_worktrees();
+ struct worktree **worktrees = get_worktrees(0);
int i;
for (i = 0; worktrees[i]; i++) {
diff --git a/builtin/worktree.c b/builtin/worktree.c
index b835b91..d7d195c 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -447,7 +447,7 @@ static int list(int ac, const char **av, const char *prefix)
if (ac)
usage_with_options(worktree_usage, options);
else {
- struct worktree **worktrees = get_worktrees();
+ struct worktree **worktrees = get_worktrees(0);
int path_maxlen = 0, abbrev = DEFAULT_ABBREV, i;
if (!porcelain)
@@ -478,7 +478,7 @@ static int lock_worktree(int ac, const char **av, const char *prefix)
if (ac != 1)
usage_with_options(worktree_usage, options);
- worktrees = get_worktrees();
+ worktrees = get_worktrees(0);
wt = find_worktree(worktrees, prefix, av[0]);
if (!wt)
die(_("'%s' is not a working tree"), av[0]);
@@ -511,7 +511,7 @@ static int unlock_worktree(int ac, const char **av, const char *prefix)
if (ac != 1)
usage_with_options(worktree_usage, options);
- worktrees = get_worktrees();
+ worktrees = get_worktrees(0);
wt = find_worktree(worktrees, prefix, av[0]);
if (!wt)
die(_("'%s' is not a working tree"), av[0]);
diff --git a/worktree.c b/worktree.c
index 3145522..ead088e 100644
--- a/worktree.c
+++ b/worktree.c
@@ -160,7 +160,7 @@ static void mark_current_worktree(struct worktree **worktrees)
free(git_dir);
}
-struct worktree **get_worktrees(void)
+struct worktree **get_worktrees(unsigned flags)
{
struct worktree **list = NULL;
struct strbuf path = STRBUF_INIT;
@@ -327,7 +327,7 @@ const struct worktree *find_shared_symref(const char *symref,
if (worktrees)
free_worktrees(worktrees);
- worktrees = get_worktrees();
+ worktrees = get_worktrees(0);
for (i = 0; worktrees[i]; i++) {
struct worktree *wt = worktrees[i];
diff --git a/worktree.h b/worktree.h
index 90e1311..2e68d4a 100644
--- a/worktree.h
+++ b/worktree.h
@@ -23,7 +23,7 @@ struct worktree {
* The caller is responsible for freeing the memory from the returned
* worktree(s).
*/
-extern struct worktree **get_worktrees(void);
+extern struct worktree **get_worktrees(unsigned flags);
/*
* Return git dir of the worktree. Note that the path may be relative.
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* [PATCH v2 3/5] get_worktrees() must return main worktree as first item even on error
From: Nguyễn Thái Ngọc Duy @ 2016-11-28 9:36 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, rappazzo, Nguyễn Thái Ngọc Duy
In-Reply-To: <20161128093656.15744-1-pclouds@gmail.com>
This is required by git-worktree.txt, stating that the main worktree is
the first line (especially in --porcelain mode when we can't just change
behavior at will).
There's only one case when get_worktrees() may skip main worktree, when
parse_ref() fails. Update the code so that we keep first item as main
worktree and return something sensible in this case:
- In user-friendly mode, since we're not constraint by anything,
returning "(error)" should do the job (we already show "(detached
HEAD)" which is not machine-friendly). Actually errors should be
printed on stderr by parse_ref() (*)
- In plumbing mode, we do not show neither 'bare', 'detached' or
'branch ...', which is possible by the format description if I read
it right.
Careful readers may realize that when the local variable "head_ref" in
get_main_worktree() is emptied, add_head_info() will do nothing to
wt->head_sha1. But that's ok because head_sha1 is zero-ized in the
previous patch.
(*) Well, it does not. But it's supposed to be a stop gap implementation
until we can reuse refs code to parse "ref: " stuff in HEAD, from
resolve_refs_unsafe(). Now may be the time since refs refactoring is
mostly done.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/worktree.c | 6 ++++--
t/t2027-worktree-list.sh | 21 +++++++++++++++++++++
worktree.c | 10 +++-------
3 files changed, 28 insertions(+), 9 deletions(-)
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 8a654e4..b835b91 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -388,7 +388,7 @@ static void show_worktree_porcelain(struct worktree *wt)
printf("HEAD %s\n", sha1_to_hex(wt->head_sha1));
if (wt->is_detached)
printf("detached\n");
- else
+ else if (wt->head_ref)
printf("branch %s\n", wt->head_ref);
}
printf("\n");
@@ -408,8 +408,10 @@ static void show_worktree(struct worktree *wt, int path_maxlen, int abbrev_len)
find_unique_abbrev(wt->head_sha1, DEFAULT_ABBREV));
if (wt->is_detached)
strbuf_addstr(&sb, "(detached HEAD)");
- else
+ else if (wt->head_ref)
strbuf_addf(&sb, "[%s]", shorten_unambiguous_ref(wt->head_ref, 0));
+ else
+ strbuf_addstr(&sb, "(error)");
}
printf("%s\n", sb.buf);
diff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh
index 1b1b65a..98b5f34 100755
--- a/t/t2027-worktree-list.sh
+++ b/t/t2027-worktree-list.sh
@@ -96,4 +96,25 @@ test_expect_success 'bare repo cleanup' '
rm -rf bare1
'
+test_expect_success 'broken main worktree still at the top' '
+ git init broken-main &&
+ (
+ cd broken-main &&
+ test_commit new &&
+ git worktree add linked &&
+ cat >expected <<-EOF &&
+ worktree $(pwd)
+ HEAD $_z40
+
+ EOF
+ cd linked &&
+ echo "worktree $(pwd)" >expected &&
+ echo "ref: .broken" >../.git/HEAD &&
+ git worktree list --porcelain | head -n 3 >actual &&
+ test_cmp ../expected actual &&
+ git worktree list | head -n 1 >actual.2 &&
+ grep -F "(error)" actual.2
+ )
+'
+
test_done
diff --git a/worktree.c b/worktree.c
index f7c1b5e..3145522 100644
--- a/worktree.c
+++ b/worktree.c
@@ -88,16 +88,13 @@ static struct worktree *get_main_worktree(void)
strbuf_addf(&path, "%s/HEAD", get_git_common_dir());
- if (parse_ref(path.buf, &head_ref, &is_detached) < 0)
- goto done;
-
worktree = xcalloc(1, sizeof(*worktree));
worktree->path = strbuf_detach(&worktree_path, NULL);
worktree->is_bare = is_bare;
worktree->is_detached = is_detached;
- add_head_info(&head_ref, worktree);
+ if (!parse_ref(path.buf, &head_ref, &is_detached))
+ add_head_info(&head_ref, worktree);
-done:
strbuf_release(&path);
strbuf_release(&worktree_path);
strbuf_release(&head_ref);
@@ -173,8 +170,7 @@ struct worktree **get_worktrees(void)
list = xmalloc(alloc * sizeof(struct worktree *));
- if ((list[counter] = get_main_worktree()))
- counter++;
+ list[counter++] = get_main_worktree();
strbuf_addf(&path, "%s/worktrees", get_git_common_dir());
dir = opendir(path.buf);
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* [PATCH v2 5/5] worktree list: keep the list sorted
From: Nguyễn Thái Ngọc Duy @ 2016-11-28 9:36 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, rappazzo, Nguyễn Thái Ngọc Duy
In-Reply-To: <20161128093656.15744-1-pclouds@gmail.com>
It makes it easier to write tests for. But it should also be good for
the user since locating a worktree by eye would be easier once they
notice this.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/worktree.c | 2 +-
t/t2027-worktree-list.sh | 19 +++++++++++++++++++
worktree.c | 14 ++++++++++++++
worktree.h | 2 ++
4 files changed, 36 insertions(+), 1 deletion(-)
diff --git a/builtin/worktree.c b/builtin/worktree.c
index d7d195c..9a97e37 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -447,7 +447,7 @@ static int list(int ac, const char **av, const char *prefix)
if (ac)
usage_with_options(worktree_usage, options);
else {
- struct worktree **worktrees = get_worktrees(0);
+ struct worktree **worktrees = get_worktrees(GWT_SORT_LINKED);
int path_maxlen = 0, abbrev = DEFAULT_ABBREV, i;
if (!porcelain)
diff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh
index 98b5f34..465eeea 100755
--- a/t/t2027-worktree-list.sh
+++ b/t/t2027-worktree-list.sh
@@ -117,4 +117,23 @@ test_expect_success 'broken main worktree still at the top' '
)
'
+test_expect_success 'linked worktrees are sorted' '
+ mkdir sorted &&
+ git init sorted/main &&
+ (
+ cd sorted/main &&
+ test_tick &&
+ test_commit new &&
+ git worktree add ../first &&
+ git worktree add ../second &&
+ git worktree list --porcelain | grep ^worktree >actual
+ ) &&
+ cat >expected <<-EOF &&
+ worktree $(pwd)/sorted/main
+ worktree $(pwd)/sorted/first
+ worktree $(pwd)/sorted/second
+ EOF
+ test_cmp expected sorted/main/actual
+'
+
test_done
diff --git a/worktree.c b/worktree.c
index ead088e..eb61212 100644
--- a/worktree.c
+++ b/worktree.c
@@ -160,6 +160,13 @@ static void mark_current_worktree(struct worktree **worktrees)
free(git_dir);
}
+static int compare_worktree(const void *a_, const void *b_)
+{
+ const struct worktree *const *a = a_;
+ const struct worktree *const *b = b_;
+ return fspathcmp((*a)->path, (*b)->path);
+}
+
struct worktree **get_worktrees(unsigned flags)
{
struct worktree **list = NULL;
@@ -191,6 +198,13 @@ struct worktree **get_worktrees(unsigned flags)
ALLOC_GROW(list, counter + 1, alloc);
list[counter] = NULL;
+ if (flags & GWT_SORT_LINKED)
+ /*
+ * don't sort the first item (main worktree), which will
+ * always be the first
+ */
+ QSORT(list + 1, counter - 1, compare_worktree);
+
mark_current_worktree(list);
return list;
}
diff --git a/worktree.h b/worktree.h
index 2e68d4a..d59ce1f 100644
--- a/worktree.h
+++ b/worktree.h
@@ -15,6 +15,8 @@ struct worktree {
/* Functions for acting on the information about worktrees. */
+#define GWT_SORT_LINKED (1 << 0) /* keeps linked worktrees sorted */
+
/*
* Get the worktrees. The primary worktree will always be the first returned,
* and linked worktrees will be pointed to by 'next' in each subsequent
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* [PATCH v2 2/5] worktree: reorder an if statement
From: Nguyễn Thái Ngọc Duy @ 2016-11-28 9:36 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, rappazzo, Nguyễn Thái Ngọc Duy
In-Reply-To: <20161128093656.15744-1-pclouds@gmail.com>
This is no-op. But it helps reduce diff noise in the next patch.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/worktree.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 5c4854d..8a654e4 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -406,10 +406,10 @@ static void show_worktree(struct worktree *wt, int path_maxlen, int abbrev_len)
else {
strbuf_addf(&sb, "%-*s ", abbrev_len,
find_unique_abbrev(wt->head_sha1, DEFAULT_ABBREV));
- if (!wt->is_detached)
- strbuf_addf(&sb, "[%s]", shorten_unambiguous_ref(wt->head_ref, 0));
- else
+ if (wt->is_detached)
strbuf_addstr(&sb, "(detached HEAD)");
+ else
+ strbuf_addf(&sb, "[%s]", shorten_unambiguous_ref(wt->head_ref, 0));
}
printf("%s\n", sb.buf);
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* [PATCH v2 1/5] worktree.c: zero new 'struct worktree' on allocation
From: Nguyễn Thái Ngọc Duy @ 2016-11-28 9:36 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, rappazzo, Nguyễn Thái Ngọc Duy
In-Reply-To: <20161128093656.15744-1-pclouds@gmail.com>
This keeps things a bit simpler when we add more fields, knowing that
default values are always zero.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
worktree.c | 14 ++------------
1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/worktree.c b/worktree.c
index f7869f8..f7c1b5e 100644
--- a/worktree.c
+++ b/worktree.c
@@ -91,16 +91,11 @@ static struct worktree *get_main_worktree(void)
if (parse_ref(path.buf, &head_ref, &is_detached) < 0)
goto done;
- worktree = xmalloc(sizeof(struct worktree));
+ worktree = xcalloc(1, sizeof(*worktree));
worktree->path = strbuf_detach(&worktree_path, NULL);
- worktree->id = NULL;
worktree->is_bare = is_bare;
- worktree->head_ref = NULL;
worktree->is_detached = is_detached;
- worktree->is_current = 0;
add_head_info(&head_ref, worktree);
- worktree->lock_reason = NULL;
- worktree->lock_reason_valid = 0;
done:
strbuf_release(&path);
@@ -138,16 +133,11 @@ static struct worktree *get_linked_worktree(const char *id)
if (parse_ref(path.buf, &head_ref, &is_detached) < 0)
goto done;
- worktree = xmalloc(sizeof(struct worktree));
+ worktree = xcalloc(1, sizeof(*worktree));
worktree->path = strbuf_detach(&worktree_path, NULL);
worktree->id = xstrdup(id);
- worktree->is_bare = 0;
- worktree->head_ref = NULL;
worktree->is_detached = is_detached;
- worktree->is_current = 0;
add_head_info(&head_ref, worktree);
- worktree->lock_reason = NULL;
- worktree->lock_reason_valid = 0;
done:
strbuf_release(&path);
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* [PATCH v2 0/5] nd/worktree-list-fixup
From: Nguyễn Thái Ngọc Duy @ 2016-11-28 9:36 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, rappazzo, Nguyễn Thái Ngọc Duy
In-Reply-To: <20161122100046.8341-1-pclouds@gmail.com>
This version
* changes get_worktrees() to take a flag, and adds one flag for
sorting.
* adds tests for both the 'main worktree always present' and the
sorting problems.
* reworks 3/5 a bit, keep changes closer, easier to see the cause and
consequence.
Nguyễn Thái Ngọc Duy (5):
worktree.c: zero new 'struct worktree' on allocation
worktree: reorder an if statement
get_worktrees() must return main worktree as first item even on error
worktree.c: get_worktrees() takes a new flag argument
worktree list: keep the list sorted
branch.c | 2 +-
builtin/branch.c | 2 +-
builtin/worktree.c | 14 ++++++++------
t/t2027-worktree-list.sh | 40 ++++++++++++++++++++++++++++++++++++++++
worktree.c | 42 +++++++++++++++++++++---------------------
worktree.h | 4 +++-
6 files changed, 74 insertions(+), 30 deletions(-)
--
2.8.2.524.g6ff3d78
^ permalink raw reply
* [PATCH] allow git-p4 to create shelved changelists
From: Vinicius Kursancew @ 2016-11-28 9:33 UTC (permalink / raw)
To: gitster, git; +Cc: Vinicius Kursancew
This patch adds a "--shelve" option to the submit subcommand, it will
save the changes to a perforce shelve instead of commiting them.
Vinicius Kursancew (1):
git-p4: allow submit to create shelved changelists.
Documentation/git-p4.txt | 5 +++++
git-p4.py | 36 ++++++++++++++++++++++--------------
t/t9807-git-p4-submit.sh | 31 +++++++++++++++++++++++++++++++
3 files changed, 58 insertions(+), 14 deletions(-)
--
2.6.0-rc1
^ permalink raw reply
* [PATCH] git-p4: allow submit to create shelved changelists.
From: Vinicius Kursancew @ 2016-11-28 9:33 UTC (permalink / raw)
To: gitster, git; +Cc: Vinicius Kursancew
In-Reply-To: <1480325598-12344-1-git-send-email-viniciusalexandre@gmail.com>
Add a --shelve command line argument which invokes p4 shelve instead
of submitting changes. After shelving the changes are reverted from the
p4 workspace.
Signed-off-by: Vinicius Kursancew <viniciusalexandre@gmail.com>
---
Documentation/git-p4.txt | 5 +++++
git-p4.py | 36 ++++++++++++++++++++++--------------
t/t9807-git-p4-submit.sh | 31 +++++++++++++++++++++++++++++++
3 files changed, 58 insertions(+), 14 deletions(-)
diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index c83aaf3..1bbf43d 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -303,6 +303,11 @@ These options can be used to modify 'git p4 submit' behavior.
submit manually or revert. This option always stops after the
first (oldest) commit. Git tags are not exported to p4.
+--shelve::
+ Instead of submitting create a series of shelved changelists.
+ After creating each shelve, the relevant files are reverted/deleted.
+ If you have multiple commits pending multiple shelves will be created.
+
--conflict=(ask|skip|quit)::
Conflicts can occur when applying a commit to p4. When this
happens, the default behavior ("ask") is to prompt whether to
diff --git a/git-p4.py b/git-p4.py
index fd5ca52..0c4f2af 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1289,6 +1289,9 @@ class P4Submit(Command, P4UserMap):
optparse.make_option("--conflict", dest="conflict_behavior",
choices=self.conflict_behavior_choices),
optparse.make_option("--branch", dest="branch"),
+ optparse.make_option("--shelve", dest="shelve", action="store_true",
+ help="Shelve instead of submit. Shelved files are reverted, "
+ "restoring the workspace to the state before the shelve"),
]
self.description = "Submit changes from git to the perforce depot."
self.usage += " [name of git branch to submit into perforce depot]"
@@ -1296,6 +1299,7 @@ class P4Submit(Command, P4UserMap):
self.detectRenames = False
self.preserveUser = gitConfigBool("git-p4.preserveUser")
self.dry_run = False
+ self.shelve = False
self.prepare_p4_only = False
self.conflict_behavior = None
self.isWindows = (platform.system() == "Windows")
@@ -1785,7 +1789,14 @@ class P4Submit(Command, P4UserMap):
if self.isWindows:
message = message.replace("\r\n", "\n")
submitTemplate = message[:message.index(separatorLine)]
- p4_write_pipe(['submit', '-i'], submitTemplate)
+ if self.shelve:
+ p4_write_pipe(['shelve', '-i'], submitTemplate)
+ else:
+ p4_write_pipe(['submit', '-i'], submitTemplate)
+ # The rename/copy happened by applying a patch that created a
+ # new file. This leaves it writable, which confuses p4.
+ for f in pureRenameCopy:
+ p4_sync(f, "-f")
if self.preserveUser:
if p4User:
@@ -1795,23 +1806,20 @@ class P4Submit(Command, P4UserMap):
changelist = self.lastP4Changelist()
self.modifyChangelistUser(changelist, p4User)
- # The rename/copy happened by applying a patch that created a
- # new file. This leaves it writable, which confuses p4.
- for f in pureRenameCopy:
- p4_sync(f, "-f")
submitted = True
finally:
# skip this patch
- if not submitted:
- print "Submission cancelled, undoing p4 changes."
- for f in editedFiles:
+ if not submitted or self.shelve:
+ if self.shelve:
+ print ("Reverting shelved files.")
+ else:
+ print ("Submission cancelled, undoing p4 changes.")
+ for f in editedFiles | filesToDelete:
p4_revert(f)
for f in filesToAdd:
p4_revert(f)
os.remove(f)
- for f in filesToDelete:
- p4_revert(f)
os.remove(fileName)
return submitted
@@ -2067,13 +2075,13 @@ class P4Submit(Command, P4UserMap):
break
chdir(self.oldWorkingDirectory)
-
+ shelved_applied = "shelved" if self.shelve else "applied"
if self.dry_run:
pass
elif self.prepare_p4_only:
pass
elif len(commits) == len(applied):
- print "All commits applied!"
+ print ("All commits {0}!".format(shelved_applied))
sync = P4Sync()
if self.branch:
@@ -2085,9 +2093,9 @@ class P4Submit(Command, P4UserMap):
else:
if len(applied) == 0:
- print "No commits applied."
+ print ("No commits {0}.".format(shelved_applied))
else:
- print "Applied only the commits marked with '*':"
+ print ("{0} only the commits marked with '*':".format(shelved_applied.capitalize()))
for c in commits:
if c in applied:
star = "*"
diff --git a/t/t9807-git-p4-submit.sh b/t/t9807-git-p4-submit.sh
index 5931528..42a5fad 100755
--- a/t/t9807-git-p4-submit.sh
+++ b/t/t9807-git-p4-submit.sh
@@ -413,6 +413,37 @@ test_expect_success 'submit --prepare-p4-only' '
)
'
+test_expect_success 'submit --shelve' '
+ test_when_finished cleanup_git &&
+ git p4 clone --dest="$git" //depot &&
+ (
+ cd "$cli" &&
+ p4 revert ... &&
+ cd "$git" &&
+ git config git-p4.skipSubmitEdit true &&
+ test_commit "shelveme1" &&
+ git p4 submit --origin=HEAD^ &&
+
+ echo 654321 >shelveme2.t &&
+ echo 123456 >>shelveme1.t &&
+ git add shelveme* &&
+ git commit -m"shelvetest" &&
+ git p4 submit --shelve --origin=HEAD^ &&
+
+ test_path_is_file shelveme1.t &&
+ test_path_is_file shelveme2.t
+ ) &&
+ (
+ cd "$cli" &&
+ change=$(p4 -G changes -s shelved -m 1 //depot/... | \
+ marshal_dump change) &&
+ p4 describe -S $change | grep shelveme2 &&
+ p4 describe -S $change | grep 123456 &&
+ test_path_is_file shelveme1.t &&
+ test_path_is_missing shelveme2.t
+ )
+'
+
test_expect_success 'kill p4d' '
kill_p4d
'
--
2.6.0-rc1
^ permalink raw reply related
* cherry-pick -Xrenormalize fails with formerly CRLF files
From: Eevee (Lexy Munroe) @ 2016-11-28 6:19 UTC (permalink / raw)
To: git
I'm working with a repo that used to be all CRLF. At some point it was
changed to all LF, with `text=auto` in .gitattributes for the sake of
Windows devs. I'm on Linux and have never touched any twiddles relating
to line endings. I'm trying to cherry-pick some commits from before the
switchover.
Straightforward cherry-picking causes entire files at a time to
conflict, which I've seen before when switching from tabs to spaces. So
I tried -Xrenormalize and got:
fatal: CRLF would be replaced by LF in [path]
The error comes from check_safe_crlf, which warns if checksafe is
CRLF_SAFE_WARN and dies if it's (presumably) CRLF_SAFE_FAIL. The funny
thing is that it's CRLF_SAFE_RENORMALIZE.
I don't know what the semantics of this value are, but the caller
(crlf_to_git) explicitly checks for CRLF_SAFE_RENORMALIZE and changes it
to CRLF_SAFE_FALSE instead. But that check only happens if crlf_action
is CRLF_AUTO*, and for me it's CRLF_TEXT_INPUT.
I moved the check to happen regardless of the value of crlf_action, and
at least in this case, git appears to happily do the right thing. So I
think this is a bug, but line endings are such a tangle that I'm really
not sure. :)
The repository in question is ZDoom: https://github.com/rheit/zdoom
I'm trying to cherry-pick commits from the 3dfloors3 branch (e.g.,
9fb2daf58e9d512170859302a1ac0ea9c2ec5993) onto a slightly outdated
master, 6384e81d0f135a2c292ac3e874f6fe26093f45b1.
^ permalink raw reply
* Re: trustExitCode doesn't apply to vimdiff mergetool
From: David Aguilar @ 2016-11-28 2:15 UTC (permalink / raw)
To: Jeff King; +Cc: Dun Peal, Git ML
In-Reply-To: <20161128014538.GA18691@gmail.com>
On Sun, Nov 27, 2016 at 05:45:38PM -0800, David Aguilar wrote:
> On Sun, Nov 27, 2016 at 11:55:59AM -0500, Jeff King wrote:
> > On Sun, Nov 27, 2016 at 08:46:40AM -0500, Dun Peal wrote:
> >
> > > Ignoring a non-zero exit code from the merge tool, and assuming a
> > > successful merge in that case, seems like the wrong default behavior
> > > to me.
> >
> > Yeah, I'm inclined to agree. But like I said, I'm not too familiar with
> > this area, so maybe there are subtle things I'm missing.
>
> I think this may have been an oversight in how the
> trust-exit-code feature is implemented across builtins.
>
> Right now, specific builtin tools could in theory opt-in to the
> feature, but I think it should be handled in a central place.
> For vimdiff, the exit code is not considered because the
> scriptlet calls check_unchanged(), which only cares about
> modifciation time.
>
> I have a patch that makes it so that none of the tools do the
> check_unchanged logic themselves and instead rely on the
> library code to handle it for them. This makes the
> implementation uniform across all tools, and allows tools to
> opt-in to trustExitCode=true.
>
> This means that all of the builtin tools will default to
> trustExitCode=false, and they can opt-in by setting the
> configuration variable.
>
> For tkdiff and kdiff3, this is a subtle change in behavior, but
> not one that should be problematic, and the upside is that we'll
> have consistency across all tools.
>
> In this scenario specifically, what happens is that the
> scriptlet is calling check_unchanged(), which checks the
> modification time of the file, and if the file is new then it
> assumes that the merge succeeded. check_unchanged() is clearing
> the exit code.
>
> Try the patch below. I tested it with vimdiff and it seems to
> provide the desired behavior:
> - the modificaiton time behavior is the default
> - setting mergetool.vimdiff.trustExitCode = true will make it
> honor vim's exit code via :cq
>
> One possible idea that could avoid the subtle tkdiff/kdiff3
> change in behavior would be to allow the scriptlets to advertise
> their preference for the default trustExitCode setting. These
> tools could say, "default to true", and the rest can assume
> false.
>
> If others feel that this is worth the extra machinery, and the
> mental burden of tools having different defaults, then that
> could be implemented as a follow-up patch. IMO I'd be okay with
> not needing it and only adding it if someone notices, but if
> others feel otherwise we can do it sooner rather than later.
>
> Thoughts?
For the curious, here is what that patch might look like.
This allows scriptlets to redefine trust_exit_code() so that
they can advertise that they prefer default=true.
The main benefit of this is that we're preserving the original
behavior before these patches. I'll let this sit out here for
comments for a few days to see what others think.
--- >8 ---
Date: Sun, 27 Nov 2016 18:08:08 -0800
Subject: [PATCH] mergetool: restore trustExitCode behavior for builtins tools
deltawalker, diffmerge, emerge, kdiff3, kompare, and tkdiff originally
provided behavior that matched trustExitCode=true.
The default for all tools is trustExitCode=false, which conflicts with
these tools' defaults. Allow tools to advertise their own default value
for trustExitCode so that users do not need to opt-in to the original
behavior.
While this makes the default inconsistent between tools, it can still be
overridden, and it makes it consistent with the current Git behavior.
Signed-off-by: David Aguilar <davvid@gmail.com>
---
git-mergetool--lib.sh | 15 +++++++++++++--
mergetools/deltawalker | 6 ++++++
mergetools/diffmerge | 6 ++++++
mergetools/emerge | 6 ++++++
mergetools/kdiff3 | 6 ++++++
mergetools/kompare | 6 ++++++
mergetools/tkdiff | 6 ++++++
7 files changed, 49 insertions(+), 2 deletions(-)
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 3d8a873ab..be079723a 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -120,6 +120,12 @@ setup_user_tool () {
merge_tool_cmd=$(get_merge_tool_cmd "$tool")
test -n "$merge_tool_cmd" || return 1
+ trust_exit_code () {
+ # user-defined tools default to trustExitCode = false
+ git config --bool "mergetool.$1.trustExitCode" ||
+ echo false
+ }
+
diff_cmd () {
( eval $merge_tool_cmd )
}
@@ -153,6 +159,12 @@ setup_tool () {
echo "$1"
}
+ trust_exit_code () {
+ # built-in tools default to trustExitCode = false
+ git config --bool "mergetool.$1.trustExitCode" ||
+ echo false
+ }
+
if ! test -f "$MERGE_TOOLS_DIR/$tool"
then
setup_user_tool
@@ -221,8 +233,7 @@ run_merge_cmd () {
merge_cmd "$1"
status=$?
- trust_exit_code=$(git config --bool \
- "mergetool.$1.trustExitCode" || echo false)
+ trust_exit_code=$(trust_exit_code "$1")
if test "$trust_exit_code" = "false"
then
check_unchanged
diff --git a/mergetools/deltawalker b/mergetools/deltawalker
index b3c71b623..ad978f83d 100644
--- a/mergetools/deltawalker
+++ b/mergetools/deltawalker
@@ -19,3 +19,9 @@ merge_cmd () {
translate_merge_tool_path() {
echo DeltaWalker
}
+
+trust_exit_code () {
+ # Default to trustExitCode = true
+ git config --bool "mergetool.$1.trustExitCode" ||
+ echo true
+}
diff --git a/mergetools/diffmerge b/mergetools/diffmerge
index f138cb4e7..437b34996 100644
--- a/mergetools/diffmerge
+++ b/mergetools/diffmerge
@@ -12,3 +12,9 @@ merge_cmd () {
--result="$MERGED" "$LOCAL" "$REMOTE"
fi
}
+
+trust_exit_code () {
+ # Default to trustExitCode = true
+ git config --bool "mergetool.$1.trustExitCode" ||
+ echo true
+}
diff --git a/mergetools/emerge b/mergetools/emerge
index 7b895fdb1..8c950d678 100644
--- a/mergetools/emerge
+++ b/mergetools/emerge
@@ -20,3 +20,9 @@ merge_cmd () {
translate_merge_tool_path() {
echo emacs
}
+
+trust_exit_code () {
+ # Default to trustExitCode = true
+ git config --bool "mergetool.$1.trustExitCode" ||
+ echo true
+}
diff --git a/mergetools/kdiff3 b/mergetools/kdiff3
index 793d1293b..9d94876b9 100644
--- a/mergetools/kdiff3
+++ b/mergetools/kdiff3
@@ -21,3 +21,9 @@ merge_cmd () {
>/dev/null 2>&1
fi
}
+
+trust_exit_code () {
+ # Default to trustExitCode = true
+ git config --bool "mergetool.$1.trustExitCode" ||
+ echo true
+}
diff --git a/mergetools/kompare b/mergetools/kompare
index 433686c12..0ae0bdc02 100644
--- a/mergetools/kompare
+++ b/mergetools/kompare
@@ -5,3 +5,9 @@ can_merge () {
diff_cmd () {
"$merge_tool_path" "$LOCAL" "$REMOTE"
}
+
+trust_exit_code () {
+ # Default to trustExitCode = true
+ git config --bool "mergetool.$1.trustExitCode" ||
+ echo true
+}
diff --git a/mergetools/tkdiff b/mergetools/tkdiff
index 618c438e8..d73792a21 100644
--- a/mergetools/tkdiff
+++ b/mergetools/tkdiff
@@ -10,3 +10,9 @@ merge_cmd () {
"$merge_tool_path" -o "$MERGED" "$LOCAL" "$REMOTE"
fi
}
+
+trust_exit_code () {
+ # Default to trustExitCode = true
+ git config --bool "mergetool.$1.trustExitCode" ||
+ echo true
+}
--
2.11.0.rc3.1.g2633b1d.dirty
^ permalink raw reply related
* Re: trustExitCode doesn't apply to vimdiff mergetool
From: Jeff King @ 2016-11-28 2:01 UTC (permalink / raw)
To: David Aguilar; +Cc: Dun Peal, Git ML
In-Reply-To: <20161128014538.GA18691@gmail.com>
On Sun, Nov 27, 2016 at 05:45:38PM -0800, David Aguilar wrote:
> I have a patch that makes it so that none of the tools do the
> check_unchanged logic themselves and instead rely on the
> library code to handle it for them. This makes the
> implementation uniform across all tools, and allows tools to
> opt-in to trustExitCode=true.
>
> This means that all of the builtin tools will default to
> trustExitCode=false, and they can opt-in by setting the
> configuration variable.
FWIW, that was the refactoring that came to mind when I looked at the
code yesterday. This is the first time I've looked at the mergetool
code, though, so you can take that with the appropriate grain of salt.
Your patch looks mostly good to me. One minor comment:
> merge_cmd () {
> - trust_exit_code=$(git config --bool \
> - "mergetool.$1.trustExitCode" || echo false)
> - if test "$trust_exit_code" = "false"
> - then
> - touch "$BACKUP"
> - ( eval $merge_tool_cmd )
> - check_unchanged
> - else
> - ( eval $merge_tool_cmd )
> - fi
> + ( eval $merge_tool_cmd )
> }
> }
>
> @@ -225,7 +216,20 @@ run_diff_cmd () {
>
> # Run a either a configured or built-in merge tool
> run_merge_cmd () {
> + touch "$BACKUP"
> +
> merge_cmd "$1"
> + status=$?
> +
> + trust_exit_code=$(git config --bool \
> + "mergetool.$1.trustExitCode" || echo false)
> + if test "$trust_exit_code" = "false"
> + then
> + check_unchanged
> + status=$?
> + fi
> +
In the original, we only touch $BACKUP if we care about timestamps. I
can't think of a reason it would matter to do the touch in the
trustExitCode=true case, but you could also write it as:
if test "$trust_exit_code" = "false"
then
touch "$BACKUP"
merge_cmd "$1"
check_unchanged
else
merge_cmd "$1"
fi
# now $? is from either merge_cmd or check_unchanged
Yours is arguably less subtle, though, which may be a good thing.
-Peff
^ permalink raw reply
* Re: trustExitCode doesn't apply to vimdiff mergetool
From: David Aguilar @ 2016-11-28 1:45 UTC (permalink / raw)
To: Jeff King; +Cc: Dun Peal, Git ML
In-Reply-To: <20161127165559.j5okxyztwescheug@sigill.intra.peff.net>
On Sun, Nov 27, 2016 at 11:55:59AM -0500, Jeff King wrote:
> On Sun, Nov 27, 2016 at 08:46:40AM -0500, Dun Peal wrote:
>
> > Ignoring a non-zero exit code from the merge tool, and assuming a
> > successful merge in that case, seems like the wrong default behavior
> > to me.
>
> Yeah, I'm inclined to agree. But like I said, I'm not too familiar with
> this area, so maybe there are subtle things I'm missing.
I think this may have been an oversight in how the
trust-exit-code feature is implemented across builtins.
Right now, specific builtin tools could in theory opt-in to the
feature, but I think it should be handled in a central place.
For vimdiff, the exit code is not considered because the
scriptlet calls check_unchanged(), which only cares about
modifciation time.
I have a patch that makes it so that none of the tools do the
check_unchanged logic themselves and instead rely on the
library code to handle it for them. This makes the
implementation uniform across all tools, and allows tools to
opt-in to trustExitCode=true.
This means that all of the builtin tools will default to
trustExitCode=false, and they can opt-in by setting the
configuration variable.
For tkdiff and kdiff3, this is a subtle change in behavior, but
not one that should be problematic, and the upside is that we'll
have consistency across all tools.
In this scenario specifically, what happens is that the
scriptlet is calling check_unchanged(), which checks the
modification time of the file, and if the file is new then it
assumes that the merge succeeded. check_unchanged() is clearing
the exit code.
Try the patch below. I tested it with vimdiff and it seems to
provide the desired behavior:
- the modificaiton time behavior is the default
- setting mergetool.vimdiff.trustExitCode = true will make it
honor vim's exit code via :cq
One possible idea that could avoid the subtle tkdiff/kdiff3
change in behavior would be to allow the scriptlets to advertise
their preference for the default trustExitCode setting. These
tools could say, "default to true", and the rest can assume
false.
If others feel that this is worth the extra machinery, and the
mental burden of tools having different defaults, then that
could be implemented as a follow-up patch. IMO I'd be okay with
not needing it and only adding it if someone notices, but if
others feel otherwise we can do it sooner rather than later.
Thoughts?
--- 8< ---
Date: Sun, 27 Nov 2016 17:26:55 -0800
Subject: [PATCH] mergetool: honor mergetool.<tool>.trustExitCode for all tools
The built-in mergetools originally required that each tool scriptlet
opt-in to the trustExitCode behavior, based on whether or not the tool
called check_unchanged() itself.
Refactor the functions so that run_merge_cmd() (rather than merge_cmd())
takes care of calling check_unchanged() so that all tools handle
the trustExitCode behavior uniformly.
Remove the check_unchanged() calls from the scriptlets.
A subtle benefit of this change is that the responsibility of
merge_cmd() has been narrowed to running the command only,
rather than also needing to deal with the backup file and
checking for changes.
Reported-by: Dun Peal <dunpealer@gmail.com>
Signed-off-by: David Aguilar <davvid@gmail.com>
---
git-mergetool--lib.sh | 24 ++++++++++++++----------
mergetools/araxis | 2 --
mergetools/bc | 2 --
mergetools/codecompare | 2 --
mergetools/diffuse | 2 --
mergetools/ecmerge | 2 --
mergetools/examdiff | 2 --
mergetools/meld | 3 +--
mergetools/opendiff | 2 --
mergetools/p4merge | 2 --
mergetools/tortoisemerge | 2 --
mergetools/vimdiff | 2 --
mergetools/winmerge | 2 --
mergetools/xxdiff | 2 --
14 files changed, 15 insertions(+), 36 deletions(-)
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 9abd00be2..3d8a873ab 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -125,16 +125,7 @@ setup_user_tool () {
}
merge_cmd () {
- trust_exit_code=$(git config --bool \
- "mergetool.$1.trustExitCode" || echo false)
- if test "$trust_exit_code" = "false"
- then
- touch "$BACKUP"
- ( eval $merge_tool_cmd )
- check_unchanged
- else
- ( eval $merge_tool_cmd )
- fi
+ ( eval $merge_tool_cmd )
}
}
@@ -225,7 +216,20 @@ run_diff_cmd () {
# Run a either a configured or built-in merge tool
run_merge_cmd () {
+ touch "$BACKUP"
+
merge_cmd "$1"
+ status=$?
+
+ trust_exit_code=$(git config --bool \
+ "mergetool.$1.trustExitCode" || echo false)
+ if test "$trust_exit_code" = "false"
+ then
+ check_unchanged
+ status=$?
+ fi
+
+ return $status
}
list_merge_tool_candidates () {
diff --git a/mergetools/araxis b/mergetools/araxis
index 64f97c5e9..e2407b65b 100644
--- a/mergetools/araxis
+++ b/mergetools/araxis
@@ -3,7 +3,6 @@ diff_cmd () {
}
merge_cmd () {
- touch "$BACKUP"
if $base_present
then
"$merge_tool_path" -wait -merge -3 -a1 \
@@ -12,7 +11,6 @@ merge_cmd () {
"$merge_tool_path" -wait -2 \
"$LOCAL" "$REMOTE" "$MERGED" >/dev/null 2>&1
fi
- check_unchanged
}
translate_merge_tool_path() {
diff --git a/mergetools/bc b/mergetools/bc
index b6319d206..3a69e60fa 100644
--- a/mergetools/bc
+++ b/mergetools/bc
@@ -3,7 +3,6 @@ diff_cmd () {
}
merge_cmd () {
- touch "$BACKUP"
if $base_present
then
"$merge_tool_path" "$LOCAL" "$REMOTE" "$BASE" \
@@ -12,7 +11,6 @@ merge_cmd () {
"$merge_tool_path" "$LOCAL" "$REMOTE" \
-mergeoutput="$MERGED"
fi
- check_unchanged
}
translate_merge_tool_path() {
diff --git a/mergetools/codecompare b/mergetools/codecompare
index 3f0486bc8..9f60e8da6 100644
--- a/mergetools/codecompare
+++ b/mergetools/codecompare
@@ -3,7 +3,6 @@ diff_cmd () {
}
merge_cmd () {
- touch "$BACKUP"
if $base_present
then
"$merge_tool_path" -MF="$LOCAL" -TF="$REMOTE" -BF="$BASE" \
@@ -12,7 +11,6 @@ merge_cmd () {
"$merge_tool_path" -MF="$LOCAL" -TF="$REMOTE" \
-RF="$MERGED"
fi
- check_unchanged
}
translate_merge_tool_path() {
diff --git a/mergetools/diffuse b/mergetools/diffuse
index 02e0843f4..5a3ae8b56 100644
--- a/mergetools/diffuse
+++ b/mergetools/diffuse
@@ -3,7 +3,6 @@ diff_cmd () {
}
merge_cmd () {
- touch "$BACKUP"
if $base_present
then
"$merge_tool_path" \
@@ -13,5 +12,4 @@ merge_cmd () {
"$merge_tool_path" \
"$LOCAL" "$MERGED" "$REMOTE" | cat
fi
- check_unchanged
}
diff --git a/mergetools/ecmerge b/mergetools/ecmerge
index 13c2e439d..6c5101c4f 100644
--- a/mergetools/ecmerge
+++ b/mergetools/ecmerge
@@ -3,7 +3,6 @@ diff_cmd () {
}
merge_cmd () {
- touch "$BACKUP"
if $base_present
then
"$merge_tool_path" "$BASE" "$LOCAL" "$REMOTE" \
@@ -12,5 +11,4 @@ merge_cmd () {
"$merge_tool_path" "$LOCAL" "$REMOTE" \
--default --mode=merge2 --to="$MERGED"
fi
- check_unchanged
}
diff --git a/mergetools/examdiff b/mergetools/examdiff
index 7b524d408..e72b06fc4 100644
--- a/mergetools/examdiff
+++ b/mergetools/examdiff
@@ -3,14 +3,12 @@ diff_cmd () {
}
merge_cmd () {
- touch "$BACKUP"
if $base_present
then
"$merge_tool_path" -merge "$LOCAL" "$BASE" "$REMOTE" -o:"$MERGED" -nh
else
"$merge_tool_path" -merge "$LOCAL" "$REMOTE" -o:"$MERGED" -nh
fi
- check_unchanged
}
translate_merge_tool_path() {
diff --git a/mergetools/meld b/mergetools/meld
index 83ebdfb4c..bc178e888 100644
--- a/mergetools/meld
+++ b/mergetools/meld
@@ -7,7 +7,7 @@ merge_cmd () {
then
check_meld_for_output_version
fi
- touch "$BACKUP"
+
if test "$meld_has_output_option" = true
then
"$merge_tool_path" --output "$MERGED" \
@@ -15,7 +15,6 @@ merge_cmd () {
else
"$merge_tool_path" "$LOCAL" "$MERGED" "$REMOTE"
fi
- check_unchanged
}
# Check whether we should use 'meld --output <file>'
diff --git a/mergetools/opendiff b/mergetools/opendiff
index 0942b2a73..b608dd6de 100644
--- a/mergetools/opendiff
+++ b/mergetools/opendiff
@@ -3,7 +3,6 @@ diff_cmd () {
}
merge_cmd () {
- touch "$BACKUP"
if $base_present
then
"$merge_tool_path" "$LOCAL" "$REMOTE" \
@@ -12,5 +11,4 @@ merge_cmd () {
"$merge_tool_path" "$LOCAL" "$REMOTE" \
-merge "$MERGED" | cat
fi
- check_unchanged
}
diff --git a/mergetools/p4merge b/mergetools/p4merge
index 5a608abf9..7a5b291dd 100644
--- a/mergetools/p4merge
+++ b/mergetools/p4merge
@@ -20,14 +20,12 @@ diff_cmd () {
}
merge_cmd () {
- touch "$BACKUP"
if ! $base_present
then
cp -- "$LOCAL" "$BASE"
create_virtual_base "$BASE" "$REMOTE"
fi
"$merge_tool_path" "$BASE" "$REMOTE" "$LOCAL" "$MERGED"
- check_unchanged
}
create_empty_file () {
diff --git a/mergetools/tortoisemerge b/mergetools/tortoisemerge
index 3b89f1c82..d7ab666a5 100644
--- a/mergetools/tortoisemerge
+++ b/mergetools/tortoisemerge
@@ -5,7 +5,6 @@ can_diff () {
merge_cmd () {
if $base_present
then
- touch "$BACKUP"
basename="$(basename "$merge_tool_path" .exe)"
if test "$basename" = "tortoisegitmerge"
then
@@ -17,7 +16,6 @@ merge_cmd () {
-base:"$BASE" -mine:"$LOCAL" \
-theirs:"$REMOTE" -merged:"$MERGED"
fi
- check_unchanged
else
echo "$merge_tool_path cannot be used without a base" 1>&2
return 1
diff --git a/mergetools/vimdiff b/mergetools/vimdiff
index 74ea6d547..a841ffdb4 100644
--- a/mergetools/vimdiff
+++ b/mergetools/vimdiff
@@ -4,7 +4,6 @@ diff_cmd () {
}
merge_cmd () {
- touch "$BACKUP"
case "$1" in
gvimdiff|vimdiff)
if $base_present
@@ -31,7 +30,6 @@ merge_cmd () {
fi
;;
esac
- check_unchanged
}
translate_merge_tool_path() {
diff --git a/mergetools/winmerge b/mergetools/winmerge
index f3819d316..74d03259f 100644
--- a/mergetools/winmerge
+++ b/mergetools/winmerge
@@ -6,10 +6,8 @@ diff_cmd () {
merge_cmd () {
# mergetool.winmerge.trustExitCode is implicitly false.
# touch $BACKUP so that we can check_unchanged.
- touch "$BACKUP"
"$merge_tool_path" -u -e -dl Local -dr Remote \
"$LOCAL" "$REMOTE" "$MERGED"
- check_unchanged
}
translate_merge_tool_path() {
diff --git a/mergetools/xxdiff b/mergetools/xxdiff
index 05b443394..e284811ff 100644
--- a/mergetools/xxdiff
+++ b/mergetools/xxdiff
@@ -6,7 +6,6 @@ diff_cmd () {
}
merge_cmd () {
- touch "$BACKUP"
if $base_present
then
"$merge_tool_path" -X --show-merged-pane \
@@ -21,5 +20,4 @@ merge_cmd () {
-R 'Accel.SearchForward: "Ctrl-G"' \
--merged-file "$MERGED" "$LOCAL" "$REMOTE"
fi
- check_unchanged
}
--
2.11.0.rc3.dirty
^ permalink raw reply related
* [PATCH] contrib/subtree: added --no-show-signature to git log invocation
From: Kieran Colford @ 2016-11-27 21:21 UTC (permalink / raw)
To: git; +Cc: Kieran Colford
When having log.showSignature enabled by default (as is good practice
with signed commits), it still shows the signature when git-subtree
passes custom format specifiers to git-log. This causes an error when
trying to push a subtree when signed commits are involved.
Adding this command line flag fixes the above bug. The command line
flag was added to all invocations of git-log so that it would behave
as expected by the original developers.
The flag could be more judiciously applied, but that requires a deeper
understanding of the code. It may be more desirable to disable
--show-signature when any custom format specifier is given, but that
change has far more wide reaching consequences, as well as a change to
the documentation.
Signed-off-by: Kieran Colford <kieran@kcolford.com>
---
contrib/subtree/git-subtree.sh | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index dec085a..d9e89d1 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -296,7 +296,7 @@ find_latest_squash () {
sq=
main=
sub=
- git log --grep="^git-subtree-dir: $dir/*\$" \
+ git log --no-show-signature --grep="^git-subtree-dir: $dir/*\$" \
--pretty=format:'START %H%n%s%n%n%b%nEND%n' HEAD |
while read a b junk
do
@@ -340,7 +340,7 @@ find_existing_splits () {
revs="$2"
main=
sub=
- git log --grep="^git-subtree-dir: $dir/*\$" \
+ git log --no-show-signature --grep="^git-subtree-dir: $dir/*\$" \
--pretty=format:'START %H%n%s%n%n%b%nEND%n' $revs |
while read a b junk
do
@@ -382,7 +382,7 @@ copy_commit () {
# We're going to set some environment vars here, so
# do it in a subshell to get rid of them safely later
debug copy_commit "{$1}" "{$2}" "{$3}"
- git log -1 --pretty=format:'%an%n%ae%n%aD%n%cn%n%ce%n%cD%n%B' "$1" |
+ git log --no-show-signature -1 --pretty=format:'%an%n%ae%n%aD%n%cn%n%ce%n%cD%n%B' "$1" |
(
read GIT_AUTHOR_NAME
read GIT_AUTHOR_EMAIL
@@ -462,8 +462,8 @@ squash_msg () {
oldsub_short=$(git rev-parse --short "$oldsub")
echo "Squashed '$dir/' changes from $oldsub_short..$newsub_short"
echo
- git log --pretty=tformat:'%h %s' "$oldsub..$newsub"
- git log --pretty=tformat:'REVERT: %h %s' "$newsub..$oldsub"
+ git log --no-show-signature --pretty=tformat:'%h %s' "$oldsub..$newsub"
+ git log --no-show-signature --pretty=tformat:'REVERT: %h %s' "$newsub..$oldsub"
else
echo "Squashed '$dir/' content from commit $newsub_short"
fi
@@ -475,7 +475,7 @@ squash_msg () {
toptree_for_commit () {
commit="$1"
- git log -1 --pretty=format:'%T' "$commit" -- || exit $?
+ git log --no-show-signature -1 --pretty=format:'%T' "$commit" -- || exit $?
}
subtree_for_commit () {
--
2.10.2
^ permalink raw reply related
* Re: trustExitCode doesn't apply to vimdiff mergetool
From: Jeff King @ 2016-11-27 16:55 UTC (permalink / raw)
To: Dun Peal; +Cc: Git ML
In-Reply-To: <CAD03jn7gU9g7NyDk_3wYTKsYQUtRGg6msvumZqUDs44hMOVX7w@mail.gmail.com>
On Sun, Nov 27, 2016 at 08:46:40AM -0500, Dun Peal wrote:
> Ignoring a non-zero exit code from the merge tool, and assuming a
> successful merge in that case, seems like the wrong default behavior
> to me.
Yeah, I'm inclined to agree. But like I said, I'm not too familiar with
this area, so maybe there are subtle things I'm missing.
> Finally, if you're not using mergetools, how do you resolve conflicts?
I just edit the conflicted sections in vim. I do use git-jump (see
contrib/git-jump), but that's just to get to them quickly.
-Peff
^ permalink raw reply
* Re: [PATCH v3 1/2] difftool: add a skeleton for the upcoming builtin
From: Jeff King @ 2016-11-27 16:50 UTC (permalink / raw)
To: Johannes Schindelin
Cc: git, Junio C Hamano, David Aguilar, Dennis Kaarsemaker
In-Reply-To: <alpine.DEB.2.20.1611261400300.117539@virtualbox>
On Sat, Nov 26, 2016 at 02:01:36PM +0100, Johannes Schindelin wrote:
> > If you want to control it from outside the test script, you'd need
> > something like:
> >
> > if test "$GIT_TEST_DIFFTOOL" = "builtin"
>
> That is a bit magic. I first used "GIT_USE_BUILTIN_DIFFTOOL" and it did
> not work. My name is arguably more correct (see also Jakub's note about
> "naming is hard"), but yours works because there is a "TEST" substring in
> it.
Yes. You are free to add an exception to the env list in test-lib.sh,
but we usually use GIT_TEST_* to avoid having to do so.
-Peff
^ permalink raw reply
* [PATCH/RFC v1 1/1] New way to normalize the line endings
From: tboegi @ 2016-11-27 16:22 UTC (permalink / raw)
To: git; +Cc: Torsten Bögershausen
In-Reply-To: <5502e894-bb22-e8b9-ab7a-49346d238283@web.de>
From: Torsten Bögershausen <tboegi@web.de>
Sincec commit 6523728499e7 'convert: unify the "auto" handling of CRLF'
the normalization instruction in Documentation/gitattributes.txt
doesn't work any more.
Update the documentation and add a test case.
Reported by Kristian Adrup
https://github.com/git-for-windows/git/issues/954
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
Documentation/gitattributes.txt | 7 +++----
t/t0025-crlf-auto.sh | 29 +++++++++++++++++++++++++++++
2 files changed, 32 insertions(+), 4 deletions(-)
diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 976243a..1f7529a 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -227,11 +227,10 @@ From a clean working directory:
-------------------------------------------------
$ echo "* text=auto" >.gitattributes
-$ rm .git/index # Remove the index to force Git to
-$ git reset # re-scan the working directory
+$ git ls-files --eol | egrep "i/(crlf|mixed)" # find not normalized files
+$ rm .git/index # Remove the index to re-scan the working directory
+$ git add .
$ git status # Show files that will be normalized
-$ git add -u
-$ git add .gitattributes
$ git commit -m "Introduce end-of-line normalization"
-------------------------------------------------
diff --git a/t/t0025-crlf-auto.sh b/t/t0025-crlf-auto.sh
index d0bee08..4ad4d02 100755
--- a/t/t0025-crlf-auto.sh
+++ b/t/t0025-crlf-auto.sh
@@ -152,4 +152,33 @@ test_expect_success 'eol=crlf _does_ normalize binary files' '
test -z "$LFwithNULdiff"
'
+test_expect_success 'prepare unnormalized' '
+
+ > .gitattributes &&
+ git config core.autocrlf false &&
+ printf "LINEONE\nLINETWO\r\n" >mixed &&
+ git add mixed .gitattributes &&
+ git commit -m "Add mixed" &&
+ git ls-files --eol | egrep "i/crlf" &&
+ git ls-files --eol | egrep "i/mixed"
+
+'
+
+test_expect_success 'normalize unnormalized' '
+ echo "* text=auto" >.gitattributes &&
+ rm .git/index &&
+ git add . &&
+ git commit -m "Introduce end-of-line normalization" &&
+ git ls-files --eol | tr "\\t" " " | sort >act &&
+cat >exp <<EOF &&
+i/-text w/-text attr/text=auto LFwithNUL
+i/lf w/crlf attr/text=auto CRLFonly
+i/lf w/crlf attr/text=auto LFonly
+i/lf w/lf attr/text=auto .gitattributes
+i/lf w/mixed attr/text=auto mixed
+EOF
+ test_cmp exp act
+
+'
+
test_done
--
2.10.0
^ permalink raw reply related
* Re: trustExitCode doesn't apply to vimdiff mergetool
From: Dun Peal @ 2016-11-27 13:46 UTC (permalink / raw)
To: Jeff King; +Cc: Git ML
In-Reply-To: <20161127050818.rmjpvha64y4wosq2@sigill.intra.peff.net>
Thanks, Jeff.
Ignoring a non-zero exit code from the merge tool, and assuming a
successful merge in that case, seems like the wrong default behavior
to me.
If your merge tool quit with an error, it is more sensible to assume
that the resolution you were working on has not been successfully
concluded.
In the rare case where one did successfully conclude the resolution,
you can always quickly mark the file resolved. I'm not even sure how
to do that short of `git checkout -m -- file`, which would lose any
work you've already done towards the merge.
Long story short, I hope the developers change this default, or at
least let us override it for the builtin invocations.
Finally, if you're not using mergetools, how do you resolve conflicts?
On Sun, Nov 27, 2016 at 12:08 AM, Jeff King <peff@peff.net> wrote:
> On Sat, Nov 26, 2016 at 09:44:36PM -0500, Dun Peal wrote:
>
>> I'm using vimdiff as my mergetool, and have the following lines in
>> ~/.gitconfig:
>>
>> [merge]
>> tool = vimdiff
>> [mergetool "vimdiff"]
>> trustExitCode = true
>>
>>
>> My understanding from the docs is that this sets
>> mergetool.vimdiff.trustExitCode to true, thereby concluding that a
>> merge hasn't been successful if vimdiff's exit code is non-zero.
>>
>> Unfortunately, when I exit Vim using `:cq` - which returns code 1 -
>> the merge is still presumed to have succeeded.
>>
>> Is there a way to accomplish the desired effect, such that exiting
>> vimdiff with a non-zero code would prevent git from resolving the
>> conflict in the merged file?
>
> I don't use mergetool myself, but peeking at the code, it looks like
> trustExitCode is used only for a "user" tool, not for the builtin tool
> profiles. That sounds kind of confusing to me, but I'll leave discussion
> of that to people more interested in mergetool.
>
> However, I think you can work around it by defining your own tool that
> runs vimdiff:
>
> git config merge.tool foo
> git config mergetool.foo.cmd 'vimdiff "$LOCAL" "$BASE" "$REMOTE" "$MERGED"'
> git config mergetool.foo.trustExitCode true
>
> Though note that the builtin vimdiff invocation is a little more
> complicated than that. You may want to adapt what is in git.git's
> mergetools/vimdiff to your liking.
>
> -Peff
^ permalink raw reply
* Re: [PATCH v3 2/2] difftool: implement the functionality in the builtin
From: Jakub Narębski @ 2016-11-27 11:20 UTC (permalink / raw)
To: Johannes Schindelin
Cc: git, Junio C Hamano, David Aguilar, Dennis Kaarsemaker
In-Reply-To: <alpine.DEB.2.20.1611261407520.117539@virtualbox>
Hello Johannes,
On 27 November 2016 at 12:10, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> On Fri, 25 Nov 2016, Jakub Narębski wrote:
> > W dniu 24.11.2016 o 21:55, Johannes Schindelin pisze:
[...]
> > > +static int difftool_config(const char *var, const char *value, void *cb)
> > > +{
> > > + if (!strcmp(var, "diff.guitool")) {
> >
> > Shouldn't you also read other configuration variables, like "diff.tool",
> > and it's mergetool fallbacks ("merge.guitool", "merge.tool")?
>
> No, as those configuration variables are not used by the builtin difftool
> directly but read by subsequently spawned commands separately. There would
> be no use reading them here, for now.
Ah, all right then.
Though NEEDSWORK comment would be nice to have here (for when
we don't spawn commands).
[...]
> I read the rest of your review, but it appears that it is more about
> style than about substance, while I am only willing to address the latter
> issues at the moment. You see, I want to focus on getting difftool correct
> first before attempting to make it pretty.
>
> Ciao,
> Dscho
Well, excet for the submodule-relates stuff, which I have skipped,
it looks good to me.
--
Jakub Narębski
^ permalink raw reply
* Re: [PATCH v3 2/2] difftool: implement the functionality in the builtin
From: Johannes Schindelin @ 2016-11-27 11:10 UTC (permalink / raw)
To: Jakub Narębski
Cc: git, Junio C Hamano, David Aguilar, Dennis Kaarsemaker
In-Reply-To: <2994b0d6-4b6c-84e7-d0d5-257bcae3be98@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3725 bytes --]
Hi Jakub,
On Fri, 25 Nov 2016, Jakub Narębski wrote:
> W dniu 24.11.2016 o 21:55, Johannes Schindelin pisze:
>
> > The current version of the builtin difftool does not, however, make
> > full use of the internals but instead chooses to spawn a couple of Git
> > processes, still, to make for an easier conversion. There remains a
> > lot of room for improvement, left for a later date.
> [...]
>
> > Sadly, the speedup is more noticable on Linux than on Windows: a quick
> > test shows that t7800-difftool.sh runs in (2.183s/0.052s/0.108s)
> > (real/user/sys) in a Linux VM, down from (6.529s/3.112s/0.644s),
> > while on Windows, it is (36.064s/2.730s/7.194s), down from
> > (47.637s/2.407s/6.863s). The culprit is most likely the overhead
> > incurred from *still* having to shell out to mergetool-lib.sh and
> > difftool--helper.sh.
>
> Does this mean that our shell-based testsuite is not well suited to be
> benchmark suite for comparing performance on MS Windows?
It is quite likely the case that shell-based testing will always be
inappropriate for performance testing. Even on Linux.
> Or does it mean that "builtin-difftool" spawning Git processes is the
> problem?
At the moment I would have to guess, and I'd rather not.
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> > builtin/difftool.c | 670 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 669 insertions(+), 1 deletion(-)
> >
> > diff --git a/builtin/difftool.c b/builtin/difftool.c
> > index 53870bb..3480920 100644
> > --- a/builtin/difftool.c
> > +++ b/builtin/difftool.c
> > @@ -11,9 +11,608 @@
> > *
> > * Copyright (C) 2016 Johannes Schindelin
> > */
> > +#include "cache.h"
> > #include "builtin.h"
> > #include "run-command.h"
> > #include "exec_cmd.h"
> > +#include "parse-options.h"
> > +#include "argv-array.h"
> > +#include "strbuf.h"
> > +#include "lockfile.h"
> > +#include "dir.h"
> > +
> > +static char *diff_gui_tool;
> > +static int trust_exit_code;
> > +
> > +static const char *const builtin_difftool_usage[] = {
> > + N_("git difftool [<options>] [<commit> [<commit>]] [--] [<path>...]"),
> > + NULL
> > +};
> > +
> > +static int difftool_config(const char *var, const char *value, void *cb)
> > +{
> > + if (!strcmp(var, "diff.guitool")) {
>
> Shouldn't you also read other configuration variables, like "diff.tool",
> and it's mergetool fallbacks ("merge.guitool", "merge.tool")?
No, as those configuration variables are not used by the builtin difftool
directly but read by subsequently spawned commands separately. There would
be no use reading them here, for now.
> > +static int print_tool_help(void)
> > +{
> > + const char *argv[] = { "mergetool", "--tool-help=diff", NULL };
> > + return run_command_v_opt(argv, RUN_GIT_CMD);
>
> This looks a bit strange to me, but I guess this is to avoid recursively
> invoking ourself, and { "legacy-difftool", "--tool-help", NULL }; isn't
> that much better.
This is obviously a straight translation of the Perl script (see
https://github.com/git/git/blob/v2.10.2/git-difftool.perl#L40-L46):
sub print_tool_help
{
# See the comment at the bottom of file_diff() for the reason
# behind
# using system() followed by exit() instead of exec().
my $rc = system(qw(git mergetool --tool-help=diff));
exit($rc | ($rc >> 8));
}
I read the rest of your review, but it appears that it is more about
style than about substance, while I am only willing to address the latter
issues at the moment. You see, I want to focus on getting difftool correct
first before attempting to make it pretty.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH v3 1/2] difftool: add a skeleton for the upcoming builtin
From: Johannes Schindelin @ 2016-11-26 13:01 UTC (permalink / raw)
To: Jeff King; +Cc: git, Junio C Hamano, David Aguilar, Dennis Kaarsemaker
In-Reply-To: <20161126161907.xol62zytn2jb45gh@sigill.intra.peff.net>
Hi Peff,
On Sat, 26 Nov 2016, Jeff King wrote:
> On Sat, Nov 26, 2016 at 01:22:28PM +0100, Johannes Schindelin wrote:
>
> > In other words, GIT_CONFIG_PARAMETERS is *explicitly scrubbed* from the
> > environment when we run our tests (by the code block between the "before"
> > and the "after" statements in the diff above).
>
> Sorry if I wasn't clear. I meant to modify t7800 to run the tests twice,
> once with the existing script and once with the builtin. I.e., to set
> the variable after test-lib.sh has done its scrubbing, and then use a
> loop or similar to go through the tests twice.
>
> If you want to control it from outside the test script, you'd need
> something like:
>
> if test "$GIT_TEST_DIFFTOOL" = "builtin"
That is a bit magic. I first used "GIT_USE_BUILTIN_DIFFTOOL" and it did
not work. My name is arguably more correct (see also Jakub's note about
"naming is hard"), but yours works because there is a "TEST" substring in
it.
Ciao,
Dscho
^ permalink raw reply
* [PATCH] t7610: clean up foo.* tmpdir
From: Jeff King @ 2016-11-27 6:34 UTC (permalink / raw)
To: git; +Cc: Armin Kunaschik
[resend; the original subject with foo.XXXXXX was bounced by vger for
being too sexy]
-- >8 --
Subject: [PATCH] t7610: clean up foo.XXXXXX tmpdir
The lazy prereq for MKTEMP uses "mktemp -t" to see if
mergetool's internal mktemp call will be able to run. But
unlike the call inside mergetool, we do not ever bother to
clean up the result, and the /tmp of git developers will
slowly fill up with "foo.XXXXXX" directories as they run the
test suite over and over. Let's clean up the directory
after we've verified its creation.
Note that we don't use test_when_finished here, and instead
just make rmdir part of the &&-chain. We should only remove
something that we're confident we just created. A failure in
the middle of the chain either means there's nothing to
clean up, or we are very confused and should err on the side
of caution.
Signed-off-by: Jeff King <peff@peff.net>
---
This has been happening since c578a09bd (t7610: test for mktemp before
test execution, 2016-07-02). I have noticed the foo.* directories
building in /tmp, but I never bothered to track it down before. I just
assumed from the name it was one of my personal hacky scripts. :)
It does make me wonder if test-lib.sh ought to just set $TMPDIR inside
the trash directory so that any cruft we fail to cleanup is contained.
t/t7610-mergetool.sh | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
index 6d9f21511..63d36fb28 100755
--- a/t/t7610-mergetool.sh
+++ b/t/t7610-mergetool.sh
@@ -591,7 +591,8 @@ test_expect_success 'filenames seen by tools start with ./' '
test_lazy_prereq MKTEMP '
tempdir=$(mktemp -d -t foo.XXXXXX) &&
- test -d "$tempdir"
+ test -d "$tempdir" &&
+ rmdir "$tempdir"
'
test_expect_success MKTEMP 'temporary filenames are used with mergetool.writeToTemp' '
--
2.11.0.rc3.315.gde8259a
^ permalink raw reply related
* Re: trustExitCode doesn't apply to vimdiff mergetool
From: Jeff King @ 2016-11-27 5:08 UTC (permalink / raw)
To: Dun Peal; +Cc: Git ML
In-Reply-To: <CAD03jn5PAZcFeesaq2osjo7cYd1frWZeN0odNqTh+AMxSEmLgQ@mail.gmail.com>
On Sat, Nov 26, 2016 at 09:44:36PM -0500, Dun Peal wrote:
> I'm using vimdiff as my mergetool, and have the following lines in
> ~/.gitconfig:
>
> [merge]
> tool = vimdiff
> [mergetool "vimdiff"]
> trustExitCode = true
>
>
> My understanding from the docs is that this sets
> mergetool.vimdiff.trustExitCode to true, thereby concluding that a
> merge hasn't been successful if vimdiff's exit code is non-zero.
>
> Unfortunately, when I exit Vim using `:cq` - which returns code 1 -
> the merge is still presumed to have succeeded.
>
> Is there a way to accomplish the desired effect, such that exiting
> vimdiff with a non-zero code would prevent git from resolving the
> conflict in the merged file?
I don't use mergetool myself, but peeking at the code, it looks like
trustExitCode is used only for a "user" tool, not for the builtin tool
profiles. That sounds kind of confusing to me, but I'll leave discussion
of that to people more interested in mergetool.
However, I think you can work around it by defining your own tool that
runs vimdiff:
git config merge.tool foo
git config mergetool.foo.cmd 'vimdiff "$LOCAL" "$BASE" "$REMOTE" "$MERGED"'
git config mergetool.foo.trustExitCode true
Though note that the builtin vimdiff invocation is a little more
complicated than that. You may want to adapt what is in git.git's
mergetools/vimdiff to your liking.
-Peff
^ permalink raw reply
* [PATCH] common-main: stop munging argv[0] path
From: Jeff King @ 2016-11-27 4:31 UTC (permalink / raw)
To: Mike Galbraith; +Cc: git
In-Reply-To: <1480182671.3830.38.camel@gmail.com>
On Sat, Nov 26, 2016 at 06:51:11PM +0100, Mike Galbraith wrote:
> > On the other hand, I am sympathetic that something used to work and now
> > doesn't. It probably wouldn't be that hard to work around it.
> >
> > The reason for the behavior change is that one of the cmd_main()
> > functions was relying on the basename side-effect of the
> > extract_argv0_path function, so 650c449250d7 just feeds the munged
> > argv[0] to all of the programs. The cleanest fix would probably be
> > something like:
>
> That did fix it up, thanks. I'll try twiddling the script instead.
Thanks for confirming. Here it is, with a commit message and a little
bit of polish.
-- >8 --
Subject: [PATCH] common-main: stop munging argv[0] path
Since 650c44925 (common-main: call git_extract_argv0_path(),
2016-07-01), the argv[0] that is seen in cmd_main() of
individual programs is always the basename of the
executable, as common-main strips off the full path. This
can produce confusing results for git-daemon, which wants to
re-exec itself.
For instance, if the program was originally run as
"/usr/lib/git/git-daemon", it will try just re-execing
"git-daemon", which will find the first instance in $PATH.
If git's exec-path has not been prepended to $PATH, we may
find the git-daemon from a different version (or no
git-daemon at all).
Normally this isn't a problem. Git commands are run as "git
daemon", the git wrapper puts the exec-path at the front of
$PATH, and argv[0] is already "daemon" anyway. But running
git-daemon via its full exec-path, while not really a
recommended method, did work prior to 650c44925. Let's make
it work again.
The real goal of 650c44925 was not to munge argv[0], but to
reliably set the argv0_path global. The only reason it
munges at all is that one caller, the git.c wrapper,
piggy-backed on that computation to find the command
basename. Instead, let's leave argv[0] untouched in
common-main, and have git.c do its own basename computation.
While we're at it, let's drop the return value from
git_extract_argv0_path(). It was only ever used in this one
callsite, and its dual purposes is what led to this
confusion in the first place.
Note that by changing the interface, the compiler can
confirm for us that there are no other callers storing the
return value. But the compiler can't tell us whether any of
the cmd_main() functions (besides git.c) were relying on the
basename munging. However, we can observe that prior to
650c44925, no other cmd_main() functions did that munging,
and no new cmd_main() functions have been introduced since
then. So we can't be regressing any of those cases.
Signed-off-by: Jeff King <peff@peff.net>
---
common-main.c | 2 +-
exec_cmd.c | 10 +++-------
exec_cmd.h | 2 +-
git.c | 5 +++++
4 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/common-main.c b/common-main.c
index 44a29e8b1..c654f9555 100644
--- a/common-main.c
+++ b/common-main.c
@@ -33,7 +33,7 @@ int main(int argc, const char **argv)
git_setup_gettext();
- argv[0] = git_extract_argv0_path(argv[0]);
+ git_extract_argv0_path(argv[0]);
restore_sigpipe_to_default();
diff --git a/exec_cmd.c b/exec_cmd.c
index 9d5703a15..19ac2146d 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -38,21 +38,17 @@ char *system_path(const char *path)
return strbuf_detach(&d, NULL);
}
-const char *git_extract_argv0_path(const char *argv0)
+void git_extract_argv0_path(const char *argv0)
{
const char *slash;
if (!argv0 || !*argv0)
- return NULL;
+ return;
slash = find_last_dir_sep(argv0);
- if (slash) {
+ if (slash)
argv0_path = xstrndup(argv0, slash - argv0);
- return slash + 1;
- }
-
- return argv0;
}
void git_set_argv_exec_path(const char *exec_path)
diff --git a/exec_cmd.h b/exec_cmd.h
index 1f6b43378..ff0b48048 100644
--- a/exec_cmd.h
+++ b/exec_cmd.h
@@ -4,7 +4,7 @@
struct argv_array;
extern void git_set_argv_exec_path(const char *exec_path);
-extern const char *git_extract_argv0_path(const char *path);
+extern void git_extract_argv0_path(const char *path);
extern const char *git_exec_path(void);
extern void setup_path(void);
extern const char **prepare_git_cmd(struct argv_array *out, const char **argv);
diff --git a/git.c b/git.c
index e8b2baf2d..dce529fcb 100644
--- a/git.c
+++ b/git.c
@@ -654,6 +654,11 @@ int cmd_main(int argc, const char **argv)
cmd = argv[0];
if (!cmd)
cmd = "git-help";
+ else {
+ const char *slash = find_last_dir_sep(cmd);
+ if (slash)
+ cmd = slash + 1;
+ }
trace_command_performance(argv);
--
2.11.0.rc3.313.g1055eca
^ permalink raw reply related
* trustExitCode doesn't apply to vimdiff mergetool
From: Dun Peal @ 2016-11-27 2:44 UTC (permalink / raw)
To: Git ML
I'm using vimdiff as my mergetool, and have the following lines in ~/.gitconfig:
[merge]
tool = vimdiff
[mergetool "vimdiff"]
trustExitCode = true
My understanding from the docs is that this sets
mergetool.vimdiff.trustExitCode to true, thereby concluding that a
merge hasn't been successful if vimdiff's exit code is non-zero.
Unfortunately, when I exit Vim using `:cq` - which returns code 1 -
the merge is still presumed to have succeeded.
Is there a way to accomplish the desired effect, such that exiting
vimdiff with a non-zero code would prevent git from resolving the
conflict in the merged file?
^ 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