* Re: [PATCH 0/3] fixing some random blame corner cases
From: Junio C Hamano @ 2017-01-08 3:39 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20170106041541.rjzzofal5hscv6yi@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> [1/3]: blame: fix alignment with --abbrev=40
> [2/3]: blame: handle --no-abbrev
> [3/3]: blame: output porcelain "previous" header for each file
Thanks. 1 & 2 obviously look correct. I'd need to look at 3 when I
am not exhausted, even though I expect it to be also fine from a
cursory read.
^ permalink raw reply
* Re: "git fsck" not detecting garbage at the end of blob object files...
From: Jeff King @ 2017-01-08 5:26 UTC (permalink / raw)
To: Dennis Kaarsemaker; +Cc: John Szakmeister, git
In-Reply-To: <1483825623.31837.9.camel@kaarsemaker.net>
On Sat, Jan 07, 2017 at 10:47:03PM +0100, Dennis Kaarsemaker wrote:
> On Sat, 2017-01-07 at 07:50 -0500, John Szakmeister wrote:
> > I was perusing StackOverflow this morning and ran across this
> > question: http://stackoverflow.com/questions/41521143/git-fsck-full-only-checking-directories/
> >
> > It was a simple question about why "checking objects" was not
> > appearing, but in it was another issue. The user purposefully
> > corrupted a blob object file to see if `git fsck` would catch it by
> > tacking extra data on at the end. `git fsck` happily said everything
> > was okay, but when I played with things locally I found out that `git
> > gc` does not like that extra garbage. I'm not sure what the trade-off
> > needs to be here, but my expectation is that if `git fsck` says
> > everything is okay, then all operations using that object (file)
> > should work too.
> >
> > Is that unreasonable? What would be the impact of fixing this issue?
>
> If you do this with a commit object or tree object, fsck does complain.
> I think it's sensible to do so for blob objects as well.
The existing extra-garbage check is in unpack_sha1_rest(), which is
called as part of read_sha1_file(). And that's what we hit for commits
and trees. However, we check the sha1 of blobs using the streaming
interface (in case they're large). I think you'd want to put a similar
check into read_istream_loose(). But note if you are grepping for it, it
is hidden behind a macro; look for read_method_decl(loose).
I'm actually not sure if this should be downgrade to a warning. It's
true that it's a form of corruption, but it doesn't actually prohibit us
from getting the data we need to complete the operation. Arguably fsck
should be more picky, but it is just relying on the same parse_object()
code path that the rest of git uses.
I doubt anybody cares too much either way, though. It's not like this is
a common thing.
I did notice another interesting case when looking at this. Fsck ends up
in fsck_loose(), which has the sha1 and path of the loose object. It
passes the sha1 to fsck_sha1(), and ignores the path entirely!
So if you have a duplicate copy of the object in a pack, we'd actually
find and check the duplicate. This can happen, e.g., if you had a loose
object and fetched a thin-pack which made a copy of the loose object to
complete the pack).
Probably fsck_loose() should be more picky about making sure we are
reading the data from the loose version we found.
-Peff
^ permalink raw reply
* [PATCH] Makefile: put LIBS after LDFLAGS for imap-send
From: Steven Penny @ 2017-01-08 6:12 UTC (permalink / raw)
To: git; +Cc: Steven Penny
This matches up with the targets git-%, git-http-fetch, git-http-push and
git-remote-testsvn. It must be done this way on Windows else lcrypto cannot find
lgdi32 and lws2_32
Signed-off-by: Steven Penny <svnpenn@gmail.com>
---
Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index d861bd9..f0f65ea 100644
--- a/Makefile
+++ b/Makefile
@@ -2046,7 +2046,7 @@ git-%$X: %.o GIT-LDFLAGS $(GITLIBS)
git-imap-send$X: imap-send.o $(IMAP_SEND_BUILDDEPS) GIT-LDFLAGS $(GITLIBS)
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
- $(LIBS) $(IMAP_SEND_LDFLAGS)
+ $(IMAP_SEND_LDFLAGS) $(LIBS)
git-http-fetch$X: http.o http-walker.o http-fetch.o GIT-LDFLAGS $(GITLIBS)
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
--
2.8.3
^ permalink raw reply related
* [PATCH 0/6] git worktree move/remove
From: Nguyễn Thái Ngọc Duy @ 2017-01-08 9:39 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
This version is the same as nd/worktree-move but with the recursive
directory copy code removed. We rely on rename() to move directories.
Nguyễn Thái Ngọc Duy (6):
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 | 162 +++++++++++++++++++++++++++++++++
contrib/completion/git-completion.bash | 5 +-
t/t2028-worktree-move.sh | 56 ++++++++++++
worktree.c | 84 +++++++++++++++++
worktree.h | 11 +++
6 files changed, 335 insertions(+), 11 deletions(-)
--
2.8.2.524.g6ff3d78
^ permalink raw reply
* [PATCH 1/6] worktree.c: add validate_worktree()
From: Nguyễn Thái Ngọc Duy @ 2017-01-08 9:39 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170108094003.637-1-pclouds@gmail.com>
This function is later used by "worktree move" and "worktree remove"
to ensure that we have a good connection between the repository and
the worktree. For example, if a worktree is moved manually, the
worktree location recorded in $GIT_DIR/worktrees/.../gitdir is
incorrect and we should not move that one.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
worktree.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
worktree.h | 5 +++++
2 files changed, 68 insertions(+)
diff --git a/worktree.c b/worktree.c
index eb61212..929072a 100644
--- a/worktree.c
+++ b/worktree.c
@@ -291,6 +291,69 @@ const char *is_worktree_locked(struct worktree *wt)
return wt->lock_reason;
}
+static int report(int quiet, const char *fmt, ...)
+{
+ va_list params;
+
+ if (quiet)
+ return -1;
+
+ va_start(params, fmt);
+ vfprintf(stderr, fmt, params);
+ fputc('\n', stderr);
+ va_end(params);
+ return -1;
+}
+
+int validate_worktree(const struct worktree *wt, int quiet)
+{
+ struct strbuf sb = STRBUF_INIT;
+ const char *path;
+ int err;
+
+ if (is_main_worktree(wt)) {
+ /*
+ * Main worktree using .git file to point to the
+ * repository would make it impossible to know where
+ * the actual worktree is if this function is executed
+ * from another worktree. No .git file support for now.
+ */
+ strbuf_addf(&sb, "%s/.git", wt->path);
+ if (!is_directory(sb.buf)) {
+ strbuf_release(&sb);
+ return report(quiet, _("'%s/.git' at main worktree is not the repository directory"),
+ wt->path);
+ }
+ return 0;
+ }
+
+ /*
+ * Make sure "gitdir" file points to a real .git file and that
+ * file points back here.
+ */
+ if (!is_absolute_path(wt->path))
+ return report(quiet, _("'%s' file does not contain absolute path to the worktree location"),
+ git_common_path("worktrees/%s/gitdir", wt->id));
+
+ strbuf_addf(&sb, "%s/.git", wt->path);
+ if (!file_exists(sb.buf)) {
+ strbuf_release(&sb);
+ return report(quiet, _("'%s/.git' does not exist"), wt->path);
+ }
+
+ path = read_gitfile_gently(sb.buf, &err);
+ strbuf_release(&sb);
+ if (!path)
+ return report(quiet, _("'%s/.git' is not a .git file, error code %d"),
+ wt->path, err);
+
+ if (fspathcmp(path, real_path(git_common_path("worktrees/%s", wt->id))))
+ return report(quiet, _("'%s' does not point back to"),
+ wt->path, git_common_path("worktrees/%s", wt->id));
+
+ return 0;
+}
+
int is_worktree_being_rebased(const struct worktree *wt,
const char *target)
{
diff --git a/worktree.h b/worktree.h
index d59ce1f..4433db2 100644
--- a/worktree.h
+++ b/worktree.h
@@ -52,6 +52,11 @@ extern int is_main_worktree(const struct worktree *wt);
*/
extern const char *is_worktree_locked(struct worktree *wt);
+/*
+ * Return zero if the worktree is in good condition.
+ */
+extern int validate_worktree(const struct worktree *wt, int quiet);
+
/*
* Free up the memory for worktree(s)
*/
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* [PATCH 2/6] worktree.c: add update_worktree_location()
From: Nguyễn Thái Ngọc Duy @ 2017-01-08 9:39 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170108094003.637-1-pclouds@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
worktree.c | 21 +++++++++++++++++++++
worktree.h | 6 ++++++
2 files changed, 27 insertions(+)
diff --git a/worktree.c b/worktree.c
index 929072a..7684951 100644
--- a/worktree.c
+++ b/worktree.c
@@ -354,6 +354,27 @@ int validate_worktree(const struct worktree *wt, int quiet)
return 0;
}
+int update_worktree_location(struct worktree *wt, const char *path_)
+{
+ struct strbuf path = STRBUF_INIT;
+ int ret = 0;
+
+ if (is_main_worktree(wt))
+ return 0;
+
+ strbuf_add_absolute_path(&path, path_);
+ if (fspathcmp(wt->path, path.buf)) {
+ write_file(git_common_path("worktrees/%s/gitdir",
+ wt->id),
+ "%s/.git", real_path(path.buf));
+ free(wt->path);
+ wt->path = strbuf_detach(&path, NULL);
+ ret = 0;
+ }
+ strbuf_release(&path);
+ return ret;
+}
+
int is_worktree_being_rebased(const struct worktree *wt,
const char *target)
{
diff --git a/worktree.h b/worktree.h
index 4433db2..1ee03f4 100644
--- a/worktree.h
+++ b/worktree.h
@@ -57,6 +57,12 @@ extern const char *is_worktree_locked(struct worktree *wt);
*/
extern int validate_worktree(const struct worktree *wt, int quiet);
+/*
+ * Update worktrees/xxx/gitdir with the new path.
+ */
+extern int update_worktree_location(struct worktree *wt,
+ const char *path_);
+
/*
* Free up the memory for worktree(s)
*/
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* [PATCH 3/6] worktree move: new command
From: Nguyễn Thái Ngọc Duy @ 2017-01-08 9:40 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170108094003.637-1-pclouds@gmail.com>
There are two options to move the main worktree, but both have
complications, so it's not implemented yet. Anyway the options are:
- convert the main worktree to a linked one and move it away, leave the
git repository where it is. The repo essentially becomes bare after
this move.
- move the repository with the main worktree. The tricky part is make
sure all file descriptors to the repository are closed, or it may
fail on Windows.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Documentation/git-worktree.txt | 7 +++++-
builtin/worktree.c | 43 ++++++++++++++++++++++++++++++++++
contrib/completion/git-completion.bash | 2 +-
t/t2028-worktree-move.sh | 30 ++++++++++++++++++++++++
4 files changed, 80 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index e257c19..1310513 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -12,6 +12,7 @@ SYNOPSIS
'git worktree add' [-f] [--detach] [--checkout] [-b <new-branch>] <path> [<branch>]
'git worktree list' [--porcelain]
'git worktree lock' [--reason <string>] <worktree>
+'git worktree move' <worktree> <new-path>
'git worktree prune' [-n] [-v] [--expire <expire>]
'git worktree unlock' <worktree>
@@ -71,6 +72,11 @@ files from being pruned automatically. This also prevents it from
being moved or deleted. Optionally, specify a reason for the lock
with `--reason`.
+move::
+
+Move a working tree to a new location. Note that the main working tree
+cannot be moved yet.
+
prune::
Prune working tree information in $GIT_DIR/worktrees.
@@ -252,7 +258,6 @@ performed manually, such as:
- `remove` to remove a linked working tree and its administrative files (and
warn if the working tree is dirty)
-- `mv` to move or rename a working tree and update its administrative files
GIT
---
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 9a97e37..0d8b57c 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -15,6 +15,7 @@ static const char * const worktree_usage[] = {
N_("git worktree add [<options>] <path> [<branch>]"),
N_("git worktree list [<options>]"),
N_("git worktree lock [<options>] <path>"),
+ N_("git worktree move <worktree> <new-path>"),
N_("git worktree prune [<options>]"),
N_("git worktree unlock <path>"),
NULL
@@ -524,6 +525,46 @@ static int unlock_worktree(int ac, const char **av, const char *prefix)
return ret;
}
+static int move_worktree(int ac, const char **av, const char *prefix)
+{
+ struct option options[] = {
+ OPT_END()
+ };
+ struct worktree **worktrees, *wt;
+ struct strbuf dst = STRBUF_INIT;
+ const char *reason;
+
+ ac = parse_options(ac, av, prefix, options, worktree_usage, 0);
+ if (ac != 2)
+ usage_with_options(worktree_usage, options);
+
+ strbuf_addstr(&dst, prefix_filename(prefix,
+ strlen(prefix),
+ av[1]));
+ if (file_exists(dst.buf))
+ die(_("target '%s' already exists"), av[1]);
+
+ worktrees = get_worktrees(0);
+ wt = find_worktree(worktrees, prefix, av[0]);
+ if (!wt)
+ die(_("'%s' is not a working directory"), av[0]);
+ if (is_main_worktree(wt))
+ die(_("'%s' is a main working directory"), av[0]);
+ reason = is_worktree_locked(wt);
+ if (reason) {
+ if (*reason)
+ die(_("already locked, reason: %s"), reason);
+ die(_("already locked, no reason"));
+ }
+ if (validate_worktree(wt, 0))
+ return -1;
+
+ if (rename(wt->path, dst.buf) == -1)
+ die_errno(_("failed to move '%s' to '%s'"), wt->path, dst.buf);
+
+ return update_worktree_location(wt, dst.buf);
+}
+
int cmd_worktree(int ac, const char **av, const char *prefix)
{
struct option options[] = {
@@ -546,5 +587,7 @@ int cmd_worktree(int ac, const char **av, const char *prefix)
return lock_worktree(ac - 1, av + 1, prefix);
if (!strcmp(av[1], "unlock"))
return unlock_worktree(ac - 1, av + 1, prefix);
+ if (!strcmp(av[1], "move"))
+ return move_worktree(ac - 1, av + 1, prefix);
usage_with_options(worktree_usage, options);
}
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 6721ff8..e80392b 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2715,7 +2715,7 @@ _git_whatchanged ()
_git_worktree ()
{
- local subcommands="add list lock prune unlock"
+ local subcommands="add list lock move prune unlock"
local subcommand="$(__git_find_on_cmdline "$subcommands")"
if [ -z "$subcommand" ]; then
__gitcomp "$subcommands"
diff --git a/t/t2028-worktree-move.sh b/t/t2028-worktree-move.sh
index 8298aaf..74070bd 100755
--- a/t/t2028-worktree-move.sh
+++ b/t/t2028-worktree-move.sh
@@ -59,4 +59,34 @@ test_expect_success 'unlock worktree twice' '
test_path_is_missing .git/worktrees/source/locked
'
+test_expect_success 'move non-worktree' '
+ mkdir abc &&
+ test_must_fail git worktree move abc def
+'
+
+test_expect_success 'move locked worktree' '
+ git worktree lock source &&
+ test_must_fail git worktree move source destination &&
+ git worktree unlock source
+'
+
+test_expect_success 'move worktree' '
+ git worktree move source destination &&
+ test_path_is_missing source &&
+ git worktree list --porcelain | grep "^worktree" >actual &&
+ cat <<-EOF >expected &&
+ worktree $TRASH_DIRECTORY
+ worktree $TRASH_DIRECTORY/destination
+ worktree $TRASH_DIRECTORY/elsewhere
+ EOF
+ test_cmp expected actual &&
+ git -C destination log --format=%s >actual2 &&
+ echo init >expected2 &&
+ test_cmp expected2 actual2
+'
+
+test_expect_success 'move main worktree' '
+ test_must_fail git worktree move . def
+'
+
test_done
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* [PATCH 4/6] worktree move: accept destination as directory
From: Nguyễn Thái Ngọc Duy @ 2017-01-08 9:40 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170108094003.637-1-pclouds@gmail.com>
Similar to "mv a b/", which is actually "mv a b/a", we extract basename
of source worktree and create a directory of the same name at
destination if dst path is a directory.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/worktree.c | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 0d8b57c..900b68b 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -541,7 +541,13 @@ static int move_worktree(int ac, const char **av, const char *prefix)
strbuf_addstr(&dst, prefix_filename(prefix,
strlen(prefix),
av[1]));
- if (file_exists(dst.buf))
+ if (is_directory(dst.buf))
+ /*
+ * keep going, dst will be appended after we get the
+ * source's absolute path
+ */
+ ;
+ else if (file_exists(dst.buf))
die(_("target '%s' already exists"), av[1]);
worktrees = get_worktrees(0);
@@ -559,6 +565,17 @@ static int move_worktree(int ac, const char **av, const char *prefix)
if (validate_worktree(wt, 0))
return -1;
+ if (is_directory(dst.buf)) {
+ const char *sep = find_last_dir_sep(wt->path);
+
+ if (!sep)
+ die(_("could not figure out destination name from '%s'"),
+ wt->path);
+ strbuf_addstr(&dst, sep);
+ if (file_exists(dst.buf))
+ die(_("target '%s' already exists"), dst.buf);
+ }
+
if (rename(wt->path, dst.buf) == -1)
die_errno(_("failed to move '%s' to '%s'"), wt->path, dst.buf);
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* [PATCH 5/6] worktree move: refuse to move worktrees with submodules
From: Nguyễn Thái Ngọc Duy @ 2017-01-08 9:40 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170108094003.637-1-pclouds@gmail.com>
Submodules contains .git files with relative paths. After a worktree
move, these files need to be updated or they may point to nowhere.
This is a bandage patch to make sure "worktree move" don't break
people's worktrees by accident. When .git file update code is in
place, this validate_no_submodules() could be removed.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/worktree.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 900b68b..64d0264 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -525,6 +525,27 @@ static int unlock_worktree(int ac, const char **av, const char *prefix)
return ret;
}
+static void validate_no_submodules(const struct worktree *wt)
+{
+ struct index_state istate = {0};
+ int i, found_submodules = 0;
+
+ if (read_index_from(&istate, worktree_git_path(wt, "index")) > 0) {
+ for (i = 0; i < istate.cache_nr; i++) {
+ struct cache_entry *ce = istate.cache[i];
+
+ if (S_ISGITLINK(ce->ce_mode)) {
+ found_submodules = 1;
+ break;
+ }
+ }
+ }
+ discard_index(&istate);
+
+ if (found_submodules)
+ die(_("This working tree contains submodules and cannot be moved yet"));
+}
+
static int move_worktree(int ac, const char **av, const char *prefix)
{
struct option options[] = {
@@ -565,6 +586,8 @@ static int move_worktree(int ac, const char **av, const char *prefix)
if (validate_worktree(wt, 0))
return -1;
+ validate_no_submodules(wt);
+
if (is_directory(dst.buf)) {
const char *sep = find_last_dir_sep(wt->path);
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* [PATCH 6/6] worktree remove: new command
From: Nguyễn Thái Ngọc Duy @ 2017-01-08 9:40 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170108094003.637-1-pclouds@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Documentation/git-worktree.txt | 21 +++++----
builtin/worktree.c | 79 ++++++++++++++++++++++++++++++++++
contrib/completion/git-completion.bash | 5 ++-
t/t2028-worktree-move.sh | 26 +++++++++++
4 files changed, 121 insertions(+), 10 deletions(-)
diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 1310513..bbde6b8 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -14,6 +14,7 @@ SYNOPSIS
'git worktree lock' [--reason <string>] <worktree>
'git worktree move' <worktree> <new-path>
'git worktree prune' [-n] [-v] [--expire <expire>]
+'git worktree remove' [--force] <worktree>
'git worktree unlock' <worktree>
DESCRIPTION
@@ -81,6 +82,13 @@ prune::
Prune working tree information in $GIT_DIR/worktrees.
+remove::
+
+Remove a working tree. Only clean working trees (no untracked files
+and no modification in tracked files) can be removed. Unclean working
+trees can be removed with `--force`. The main working tree cannot be
+removed.
+
unlock::
Unlock a working tree, allowing it to be pruned, moved or deleted.
@@ -90,9 +98,10 @@ OPTIONS
-f::
--force::
- By default, `add` refuses to create a new working tree when `<branch>`
- is already checked out by another working tree. This option overrides
- that safeguard.
+ By default, `add` refuses to create a new working tree when
+ `<branch>` is already checked out by another working tree and
+ `remove` refuses to remove an unclean working tree. This option
+ overrides that safeguard.
-b <new-branch>::
-B <new-branch>::
@@ -253,12 +262,6 @@ Multiple checkout in general is still experimental, and the support
for submodules is incomplete. It is NOT recommended to make multiple
checkouts of a superproject.
-git-worktree could provide more automation for tasks currently
-performed manually, such as:
-
-- `remove` to remove a linked working tree and its administrative files (and
- warn if the working tree is dirty)
-
GIT
---
Part of the linkgit:git[1] suite
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 64d0264..339c622e 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -17,6 +17,7 @@ static const char * const worktree_usage[] = {
N_("git worktree lock [<options>] <path>"),
N_("git worktree move <worktree> <new-path>"),
N_("git worktree prune [<options>]"),
+ N_("git worktree remove [<options>] <worktree>"),
N_("git worktree unlock <path>"),
NULL
};
@@ -605,6 +606,82 @@ static int move_worktree(int ac, const char **av, const char *prefix)
return update_worktree_location(wt, dst.buf);
}
+static int remove_worktree(int ac, const char **av, const char *prefix)
+{
+ int force = 0;
+ struct option options[] = {
+ OPT_BOOL(0, "force", &force,
+ N_("force removing even if the worktree is dirty")),
+ OPT_END()
+ };
+ struct worktree **worktrees, *wt;
+ struct strbuf sb = STRBUF_INIT;
+ const char *reason;
+ int ret = 0;
+
+ ac = parse_options(ac, av, prefix, options, worktree_usage, 0);
+ if (ac != 1)
+ usage_with_options(worktree_usage, options);
+
+ worktrees = get_worktrees(0);
+ wt = find_worktree(worktrees, prefix, av[0]);
+ if (!wt)
+ die(_("'%s' is not a working directory"), av[0]);
+ if (is_main_worktree(wt))
+ die(_("'%s' is a main working directory"), av[0]);
+ reason = is_worktree_locked(wt);
+ if (reason) {
+ if (*reason)
+ die(_("already locked, reason: %s"), reason);
+ die(_("already locked, no reason"));
+ }
+ if (validate_worktree(wt, 0))
+ return -1;
+
+ if (!force) {
+ struct argv_array child_env = ARGV_ARRAY_INIT;
+ struct child_process cp;
+ char buf[1];
+
+ argv_array_pushf(&child_env, "%s=%s/.git",
+ GIT_DIR_ENVIRONMENT, wt->path);
+ argv_array_pushf(&child_env, "%s=%s",
+ GIT_WORK_TREE_ENVIRONMENT, wt->path);
+ memset(&cp, 0, sizeof(cp));
+ argv_array_pushl(&cp.args, "status", "--porcelain", NULL);
+ cp.env = child_env.argv;
+ cp.git_cmd = 1;
+ cp.dir = wt->path;
+ cp.out = -1;
+ ret = start_command(&cp);
+ if (ret)
+ die_errno(_("failed to run git-status on '%s', code %d"),
+ av[0], ret);
+ ret = xread(cp.out, buf, sizeof(buf));
+ if (ret)
+ die(_("'%s' is dirty, use --force to delete it"), av[0]);
+ close(cp.out);
+ ret = finish_command(&cp);
+ if (ret)
+ die_errno(_("failed to run git-status on '%s', code %d"),
+ av[0], ret);
+ }
+ strbuf_addstr(&sb, wt->path);
+ if (remove_dir_recursively(&sb, 0)) {
+ error_errno(_("failed to delete '%s'"), sb.buf);
+ ret = -1;
+ }
+ strbuf_reset(&sb);
+ strbuf_addstr(&sb, git_common_path("worktrees/%s", wt->id));
+ if (remove_dir_recursively(&sb, 0)) {
+ error_errno(_("failed to delete '%s'"), sb.buf);
+ ret = -1;
+ }
+ strbuf_release(&sb);
+ free_worktrees(worktrees);
+ return ret;
+}
+
int cmd_worktree(int ac, const char **av, const char *prefix)
{
struct option options[] = {
@@ -629,5 +706,7 @@ int cmd_worktree(int ac, const char **av, const char *prefix)
return unlock_worktree(ac - 1, av + 1, prefix);
if (!strcmp(av[1], "move"))
return move_worktree(ac - 1, av + 1, prefix);
+ if (!strcmp(av[1], "remove"))
+ return remove_worktree(ac - 1, av + 1, prefix);
usage_with_options(worktree_usage, options);
}
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index e80392b..f413431 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2715,7 +2715,7 @@ _git_whatchanged ()
_git_worktree ()
{
- local subcommands="add list lock move prune unlock"
+ local subcommands="add list lock move prune remove unlock"
local subcommand="$(__git_find_on_cmdline "$subcommands")"
if [ -z "$subcommand" ]; then
__gitcomp "$subcommands"
@@ -2733,6 +2733,9 @@ _git_worktree ()
prune,--*)
__gitcomp "--dry-run --expire --verbose"
;;
+ remove,--*)
+ __gitcomp "--force"
+ ;;
*)
;;
esac
diff --git a/t/t2028-worktree-move.sh b/t/t2028-worktree-move.sh
index 74070bd..084acc6 100755
--- a/t/t2028-worktree-move.sh
+++ b/t/t2028-worktree-move.sh
@@ -89,4 +89,30 @@ test_expect_success 'move main worktree' '
test_must_fail git worktree move . def
'
+test_expect_success 'remove main worktree' '
+ test_must_fail git worktree remove .
+'
+
+test_expect_success 'remove locked worktree' '
+ git worktree lock destination &&
+ test_must_fail git worktree remove destination &&
+ git worktree unlock destination
+'
+
+test_expect_success 'remove worktree with dirty tracked file' '
+ echo dirty >>destination/init.t &&
+ test_must_fail git worktree remove destination
+'
+
+test_expect_success 'remove worktree with untracked file' '
+ git -C destination checkout init.t &&
+ : >destination/untracked &&
+ test_must_fail git worktree remove destination
+'
+
+test_expect_success 'force remove worktree with untracked file' '
+ git worktree remove --force destination &&
+ test_path_is_missing destination
+'
+
test_done
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* Re: [ANNOUNCE] git-test: run automated tests against a range of Git commits
From: Michael Haggerty @ 2017-01-08 9:52 UTC (permalink / raw)
To: Jeff King; +Cc: git discussion list
In-Reply-To: <20170107071832.2rucap3rskzmkgq4@sigill.intra.peff.net>
On 01/07/2017 08:18 AM, Jeff King wrote:
> On Fri, Jan 06, 2017 at 04:52:16PM +0100, Michael Haggerty wrote:
>
>> I just released `git test`, a script for running automated
>> tests across a range of Git commits and keeping track of the results in
>> git notes:
>>
>> https://github.com/mhagger/git-test
>>
>> This is a script that I've been using in one form or another for years
>> and I find it really handy [1].
>
> Neat. I usually "git rebase -x 'make -j8 test' @{u}" after finishing a
> topic to make sure the intermediate steps are good. But it would be neat
> to have this running continuously in the background to alert me to
> problems sooner (and the key thing there is that it remembers
> already-run tests, so it should be safe to basically for new commits
> every 10 seconds or so).
>
> I did hit a few interesting cases trying out "git test". So here's a
> narrative, and you can pick out where there may be room for improvement
> in the tool, and where I'm just being dumb. :)
>
> I tried it out first on a topic I finished earlier today, which has 3
> commits. So I did:
>
> $ git test add 'make -j8 test'
> $ git test range @{u}..HEAD
>
> It barfed on the first commit, because the script expects "git co" to
> work, but I don't have that alias. No big deal (and I already submitted
> a PR to fix it).
I make the same mistake in most of my scripts :-/ Thanks for the PR; I
merged it.
> So then I reinvoked it like:
>
> $ git test range @{u}..HEAD
>
> and it actually ran some tests. Yay.
>
> And then of course I wanted to prove to myself how cool the notes
> feature is, so I ran it again. It didn't run any tests this time. Yay
> again. But there were a few surprises:
>
> $ git test range @{u}..HEAD
> setup_test default
> Using test default; command: make -j8 test
> Old status: bad
> Tree 9fcdbd5c78^{tree} is already known to be bad!
> Old status: good
> Tree c22f4f6624^{tree} is already known to be good.
> Old status: good
> Tree 19e2e62e5e^{tree} is already known to be good.
> Already on 'jk/wait-for-child-cleanup'
> Your branch is ahead of 'origin/master' by 3 commits.
>
> ALL TESTS SUCCESSFUL
>
> My initial run with "git co" had left the first commit marked as "bad".
> That's not _too_ surprising, since it did indeed fail. I think it's
> probably a bug to record a failure note, though, if checking out fails.
> It's not necessarily an immutable property of the tree.
I definitely agree. This was an oversight which I just fixed.
> [...]
>
> The second thing that surprised me was "ALL TESTS SUCCESSFUL", when
> clearly one of them was known-bad. :)
Replayed results weren't being treated internally as failures. That's
fixed, too.
> So at this point I knew I needed to re-run the test. Looks like there's
> a "--force" option. Let's try it. There's no need to re-run the other
> two, so let's just give it one commit:
>
> $ git test range -f HEAD~2
> ...
> Object 95649d6cf9ec68f05d1dc57ec1b989b8d263a7ae^{tree} has no note
> Object e1970ce43abfbf625bce68516857e910748e5965^{tree} has no note
> Object 368f99d57e8ed17243f2e164431449d48bfca2fb^{tree} has no note
> Object ceede59ea90cebad52ba9c8263fef3fb6ef17593^{tree} has no note
> Object dfe070511c652f2b8e1bf6540f238c9ca9ba41d3^{tree} has no note
> Object 902d960b382a0cd424618ff4e1316da40e4be2f6^{tree} has no note
> ...
>
> This started spewing out many lines like the one above, until I hit ^C.
> Yikes!
>
> [...]
> I see the symmetry and simplicity in allowing the user to specify a full
> range. But it also seems like it's easy to make a mistake that's going
> to want to test a lot of commits. I wonder if it should complain when
> there's no lower bound to the commit range. Or alternatively, if there's
> a single positive reference, treat it as a lower bound, with HEAD as the
> upper bound (which is vaguely rebase-like).
I see how this might be unexpected, and it's definitely inconvenient at
some times (like when you want to test a single commit). I thought it
would be nice to allow arbitrary `rev-list` expressions (albeit
currently only a single word), but I think that you are right that other
semantics would be more convenient.
I'm thinking of maybe
* If an argument matches `*..*`, pass it to `rev-list` (like now).
* Otherwise, treat each argument as a single commit/tree (i.e., pass it
to `rev-parse`).
* If no argument is specified, test `@{u}..` (assuming that an
upstream is configured). Though actually, this won't be as
convenient as it sounds, because (a) `git test` is often run
in a separate worktree, and (2) on errors, it currently leaves the
repository with a detached `HEAD`.
* Support a `--stdin` option, to read a list of commits/trees to test
from standard input. By this mechanism, users could use arbitrary
`rev-list` commands to choose what to test.
> A few other observations about the note deletion:
>
> - The "has no note" message should perhaps be suppressed. We're just
> trying to overwrite the value if there is one (alternatively,
> instead of removing it, just overwrite it, so the old note stays
> until we get a result one way or the other).
Yes. That's a one-time problem that I haven't seen in a long time. The
script is overly chatty in general.
Aside: It would be nice if `git notes` had a subcommand to initialize a
note reference with an empty tree. (I know how to do it longhand, but
it's awkward and it should be possible to do it via the `notes` interface.)
I think ideally `git notes add` would look for pre-existing notes, and:
* If none are found, create an empty notes reference.
* If pre-existing notes are found and there was no existing test with
that name, probably just leave the old notes in place.
* If pre-existing notes are found and there was already a test with
that name but a different command, perhaps insist that the user
decide explicitly whether to forget the old results or continue using
them. This might help users avoid the mistake of re-using old results
even if they change the manner of testing.
> - It was sufficiently slow that it looks like we invoke "git notes
> remove" once per commit. It would be a lot more efficient to batch
> them (not just in terms of process startup, but because you're going
> to write a _ton_ of intermediate notes trees).
>
> Of course none of that matters if you don't do something stupid like
> trying to "git test" 45,000 commits. :)
Yeah, I've never experienced that problem myself :-P But I see that
notes supports `git notes remove --ignore-missing --stdin`, so that will
be easy to implement.
> [...]
> It would be even easier if I could just repeat my range and only re-test
> the "bad" commits. It was then that I decided to actually read the rest
> of "git test help range" and see that you already wrote such an option,
> cleverly hidden under the name "--retest".
I think you were being ironic, but if not, would this have been easier
to find under another name?
> And one final nit. I notice there is also a "--keep-going" option. Which
> made me surprised that we bothered to test HEAD~1 and HEAD, when we knew
> that HEAD~2 was bogus. I suspect this is related to the "ALL TESTS
> SUCCESSFUL" issue.
Yes, that's part of the same bug from above.
> So those were all little cosmetic things. The other big thing I wanted
> to see was what it's like to fix a bug deep in a topic. So I used "git
> rebase -i" to inset a compile error into the first commit of my 3-patch
> series. And then I tested it:
>
> $ git test add -t compile 'make -j8'
> $ git test range -t compile HEAD~3..
>
> As predicted, it stopped at the first commit and told me it was buggy.
> But I'm dumped onto a detached HEAD, and I'm on my own to actually get
> the working tree to a state where I can test and fix on my actual
> branch.
Yeah, this is awkward, not only because many people don't know what to
make of detached HEAD, but also because it makes it awkward in general
to use `git test` in your main working directory. I didn't model this
behavior on `git rebase --interactive`'s `edit` command, because I
rarely use that. But I can see how they would fit together pretty well
for people who like that workflow.
I've considered that rather than leave you in a detached HEAD state,
maybe `git test` should always restore your old branch. But it seems
like it would more often be useful to be in the directory with the
broken commit checked out and any test results, coredumps, etc intact.
I would definitely like to implement a `git test reset` command that
returns you to your initial branch (like `git bisect reset`).
I like your idea of a `git test fix` command:
> [...]
> I think it should be possible to script the next steps, though.
> Something like like "git test fix foo", which would:
>
> - expand the range of foo@{u}..foo to get the list of commits
>
> - see which ones were marked as broken
>
> - kick off an interactive rebase, but override GIT_EDITOR to mark any
> broken ones as "edit" instead of "pick"
>
> That lets you separate the act of testing from the act of fixing. You
> can let the tester run continuously in the background, and only stop to
> fix when you're at an appropriate point in your work.
I think you would usually only want to mark only the *first* broken
commit as "edit", because often errors cascade to descendant commits.
Thanks for all the great feedback!
Michael
^ permalink raw reply
* [PATCH v3] log --graph: customize the graph lines with config log.graphColors
From: Nguyễn Thái Ngọc Duy @ 2017-01-08 10:13 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <xmqqzijjd34j.fsf@gitster.mtv.corp.google.com>
If you have a 256 colors terminal (or one with true color support), then
the predefined 12 colors seem limited. On the other hand, you don't want
to draw graph lines with every single color in this mode because the two
colors could look extremely similar. This option allows you to hand pick
the colors you want.
Even with standard terminal, if your background color is neither black
or white, then the graph line may match your background and become
hidden. You can exclude your background color (or simply the colors you
hate) with this.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Compared to v2:
* set_column_colors_by_config() is renamed to set_column_colors()
* no memory leak if the function is called the second time
* proper space trimming since color_parse_mem expects so
* fixed the warning message, giving some context
* at least one test to exercise the code
* I'm not going with the cumulative behavior because I think that's
just harder to manage colors, and we would need a way to remove
colors from the config too.
Documentation/config.txt | 4 ++++
graph.c | 46 ++++++++++++++++++++++++++++++++++++++++++++--
t/t4202-log.sh | 22 ++++++++++++++++++++++
3 files changed, 70 insertions(+), 2 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 0bcb679..33a007b 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2003,6 +2003,10 @@ log.follow::
i.e. it cannot be used to follow multiple files and does not work well
on non-linear history.
+log.graphColors::
+ A list of colors, separated by commas, that can be used to draw
+ history lines in `git log --graph`.
+
log.showRoot::
If true, the initial commit will be shown as a big creation event.
This is equivalent to a diff against an empty tree.
diff --git a/graph.c b/graph.c
index dd17201..048f5cb 100644
--- a/graph.c
+++ b/graph.c
@@ -62,6 +62,49 @@ enum graph_state {
static const char **column_colors;
static unsigned short column_colors_max;
+static void set_column_colors(void)
+{
+ static char **colors;
+ static int colors_max, colors_alloc;
+ char *string = NULL;
+ const char *end, *start;
+ int i;
+
+ for (i = 0; i < colors_max; i++)
+ free(colors[i]);
+ if (colors)
+ free(colors[colors_max]);
+ colors_max = 0;
+
+ if (git_config_get_string("log.graphcolors", &string)) {
+ graph_set_column_colors(column_colors_ansi,
+ column_colors_ansi_max);
+ return;
+ }
+
+ start = string;
+ end = string + strlen(string);
+ while (start < end) {
+ const char *comma = strchrnul(start, ',');
+ char color[COLOR_MAXLEN];
+
+ while (start < comma && isspace(*start))
+ start++;
+
+ if (!color_parse_mem(start, comma - start, color)) {
+ ALLOC_GROW(colors, colors_max + 1, colors_alloc);
+ colors[colors_max++] = xstrdup(color);
+ } else
+ warning(_("ignore invalid color '%.*s' in log.graphColors"),
+ (int)(comma - start), start);
+ start = comma + 1;
+ }
+ free(string);
+ ALLOC_GROW(colors, colors_max + 1, colors_alloc);
+ colors[colors_max] = xstrdup(GIT_COLOR_RESET);
+ graph_set_column_colors((const char **)colors, colors_max);
+}
+
void graph_set_column_colors(const char **colors, unsigned short colors_max)
{
column_colors = colors;
@@ -208,8 +251,7 @@ struct git_graph *graph_init(struct rev_info *opt)
struct git_graph *graph = xmalloc(sizeof(struct git_graph));
if (!column_colors)
- graph_set_column_colors(column_colors_ansi,
- column_colors_ansi_max);
+ set_column_colors();
graph->commit = NULL;
graph->revs = opt;
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index e2db47c..afe0715 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -313,6 +313,28 @@ test_expect_success 'log --graph with merge' '
test_cmp expect actual
'
+cat > expect.colors <<\EOF
+* Merge branch 'side'
+<BLUE>|<RESET><CYAN>\<RESET>
+<BLUE>|<RESET> * side-2
+<BLUE>|<RESET> * side-1
+* <CYAN>|<RESET> Second
+* <CYAN>|<RESET> sixth
+* <CYAN>|<RESET> fifth
+* <CYAN>|<RESET> fourth
+<CYAN>|<RESET><CYAN>/<RESET>
+* third
+* second
+* initial
+EOF
+
+test_expect_success 'log --graph with merge with log.graphColors' '
+ test_config log.graphColors " blue , cyan , red " &&
+ git log --color=always --graph --date-order --pretty=tformat:%s |
+ test_decode_color | sed "s/ *\$//" >actual &&
+ test_cmp expect.colors actual
+'
+
test_expect_success 'log --raw --graph -m with merge' '
git log --raw --graph --oneline -m master | head -n 500 >actual &&
grep "initial" actual
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* Re: [PATCH] Makefile: put LIBS after LDFLAGS for imap-send
From: Johannes Schindelin @ 2017-01-08 11:54 UTC (permalink / raw)
To: Steven Penny; +Cc: git
In-Reply-To: <20170108061238.2604-1-svnpenn@gmail.com>
Hi Steven,
On Sun, 8 Jan 2017, Steven Penny wrote:
> This matches up with the targets git-%, git-http-fetch, git-http-push
> and git-remote-testsvn. It must be done this way on Windows else lcrypto
> cannot find lgdi32 and lws2_32
I am curious: how do you build Git? I ask because I build Git on Windows
many times a day, and I did not encounter any link problems. This hints at
a difference of build environment (I use the Git for Windows SDK) that
needs to be mentioned in the commit message.
Ciao,
Johannes
^ permalink raw reply
* Re: [PATCH] Makefile: put LIBS after LDFLAGS for imap-send
From: Steven Penny @ 2017-01-08 15:12 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.2.20.1701081250580.3469@virtualbox>
On Sun, Jan 8, 2017 at 5:54 AM, Johannes Schindelin wrote:
> I am curious: how do you build Git? I ask because I build Git on Windows
> many times a day, and I did not encounter any link problems.
My end goal is to build static native Windows Git via Cygwin and the
mingw64-x86_64-gcc-core package. This is certainly possible but definitely not
considered in the current Git codebase. I have a patch to config.mak.uname
coming as well.
^ permalink raw reply
* Re: [PATCH] don't use test_must_fail with grep
From: Pranit Bauva @ 2017-01-08 16:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, Luke Diamand, Git Users, Stefan Beller
In-Reply-To: <xmqqy3ym36y2.fsf@gitster.mtv.corp.google.com>
Hey Junio,
On Sun, Jan 8, 2017 at 2:48 AM, Junio C Hamano <gitster@pobox.com> wrote:
> I see v3 that has 2>&1 but according to Luke's comment ("the message
> comes from cat"), it shouldn't be there? I am behind clearing the
> backlog in my mailbox and I could tweak it out from v3 while
> queuing, or I may forget about it after looking at other topics ;-)
> in which case you may want to send v4 with the fix?
Yeah sure! No problem! :)
Regards,
Pranit Bauva
^ permalink raw reply
* [PATCH v4 1/2] don't use test_must_fail with grep
From: Pranit Bauva @ 2017-01-08 16:55 UTC (permalink / raw)
To: git
In-Reply-To: <20170103195708.15157-1-pranit.bauva@gmail.com>
test_must_fail should only be used for testing git commands. To test the
failure of other commands use `!`.
Reported-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
---
t/t3510-cherry-pick-sequence.sh | 6 +++---
t/t5504-fetch-receive-strict.sh | 2 +-
t/t5516-fetch-push.sh | 2 +-
t/t5601-clone.sh | 2 +-
t/t6030-bisect-porcelain.sh | 2 +-
t/t7610-mergetool.sh | 2 +-
t/t9001-send-email.sh | 2 +-
t/t9117-git-svn-init-clone.sh | 12 ++++++------
t/t9813-git-p4-preserve-users.sh | 8 ++++----
t/t9814-git-p4-rename.sh | 6 +++---
10 files changed, 22 insertions(+), 22 deletions(-)
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 372307c..0acf4b1 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -385,7 +385,7 @@ test_expect_success '--continue respects opts' '
git cat-file commit HEAD~1 >picked_msg &&
git cat-file commit HEAD~2 >unrelatedpick_msg &&
git cat-file commit HEAD~3 >initial_msg &&
- test_must_fail grep "cherry picked from" initial_msg &&
+ ! grep "cherry picked from" initial_msg &&
grep "cherry picked from" unrelatedpick_msg &&
grep "cherry picked from" picked_msg &&
grep "cherry picked from" anotherpick_msg
@@ -426,9 +426,9 @@ test_expect_failure '--signoff is automatically propagated to resolved conflict'
git cat-file commit HEAD~1 >picked_msg &&
git cat-file commit HEAD~2 >unrelatedpick_msg &&
git cat-file commit HEAD~3 >initial_msg &&
- test_must_fail grep "Signed-off-by:" initial_msg &&
+ ! grep "Signed-off-by:" initial_msg &&
grep "Signed-off-by:" unrelatedpick_msg &&
- test_must_fail grep "Signed-off-by:" picked_msg &&
+ ! grep "Signed-off-by:" picked_msg &&
grep "Signed-off-by:" anotherpick_msg
'
diff --git a/t/t5504-fetch-receive-strict.sh b/t/t5504-fetch-receive-strict.sh
index 9b19cff..49d3621 100755
--- a/t/t5504-fetch-receive-strict.sh
+++ b/t/t5504-fetch-receive-strict.sh
@@ -152,7 +152,7 @@ test_expect_success 'push with receive.fsck.missingEmail=warn' '
git --git-dir=dst/.git config --add \
receive.fsck.badDate warn &&
git push --porcelain dst bogus >act 2>&1 &&
- test_must_fail grep "missingEmail" act
+ ! grep "missingEmail" act
'
test_expect_success \
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 26b2caf..0fc5a7c 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -1004,7 +1004,7 @@ test_expect_success 'push --porcelain' '
test_expect_success 'push --porcelain bad url' '
mk_empty testrepo &&
test_must_fail git push >.git/bar --porcelain asdfasdfasd refs/heads/master:refs/remotes/origin/master &&
- test_must_fail grep -q Done .git/bar
+ ! grep -q Done .git/bar
'
test_expect_success 'push --porcelain rejected' '
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index a433394..4241ea5 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -151,7 +151,7 @@ test_expect_success 'clone --mirror does not repeat tags' '
git clone --mirror src mirror2 &&
(cd mirror2 &&
git show-ref 2> clone.err > clone.out) &&
- test_must_fail grep Duplicate mirror2/clone.err &&
+ ! grep Duplicate mirror2/clone.err &&
grep some-tag mirror2/clone.out
'
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 5e5370f..8c2c6ea 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -407,7 +407,7 @@ test_expect_success 'good merge base when good and bad are siblings' '
test_i18ngrep "merge base must be tested" my_bisect_log.txt &&
grep $HASH4 my_bisect_log.txt &&
git bisect good > my_bisect_log.txt &&
- test_must_fail grep "merge base must be tested" my_bisect_log.txt &&
+ ! grep "merge base must be tested" my_bisect_log.txt &&
grep $HASH6 my_bisect_log.txt &&
git bisect reset
'
diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
index 63d36fb..0fe7e58 100755
--- a/t/t7610-mergetool.sh
+++ b/t/t7610-mergetool.sh
@@ -602,7 +602,7 @@ test_expect_success MKTEMP 'temporary filenames are used with mergetool.writeToT
test_config mergetool.myecho.trustExitCode true &&
test_must_fail git merge master &&
git mergetool --no-prompt --tool myecho -- both >actual &&
- test_must_fail grep ^\./both_LOCAL_ actual >/dev/null &&
+ ! grep ^\./both_LOCAL_ actual >/dev/null &&
grep /both_LOCAL_ actual >/dev/null &&
git reset --hard master >/dev/null 2>&1
'
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 3dc4a34..0f398dd 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -50,7 +50,7 @@ test_no_confirm () {
--smtp-server="$(pwd)/fake.sendmail" \
$@ \
$patches >stdout &&
- test_must_fail grep "Send this email" stdout &&
+ ! grep "Send this email" stdout &&
>no_confirm_okay
}
diff --git a/t/t9117-git-svn-init-clone.sh b/t/t9117-git-svn-init-clone.sh
index 69a6750..044f65e 100755
--- a/t/t9117-git-svn-init-clone.sh
+++ b/t/t9117-git-svn-init-clone.sh
@@ -55,7 +55,7 @@ test_expect_success 'clone to target directory with --stdlayout' '
test_expect_success 'init without -s/-T/-b/-t does not warn' '
test ! -d trunk &&
git svn init "$svnrepo"/project/trunk trunk 2>warning &&
- test_must_fail grep -q prefix warning &&
+ ! grep -q prefix warning &&
rm -rf trunk &&
rm -f warning
'
@@ -63,7 +63,7 @@ test_expect_success 'init without -s/-T/-b/-t does not warn' '
test_expect_success 'clone without -s/-T/-b/-t does not warn' '
test ! -d trunk &&
git svn clone "$svnrepo"/project/trunk 2>warning &&
- test_must_fail grep -q prefix warning &&
+ ! grep -q prefix warning &&
rm -rf trunk &&
rm -f warning
'
@@ -86,7 +86,7 @@ EOF
test_expect_success 'init with -s/-T/-b/-t assumes --prefix=origin/' '
test ! -d project &&
git svn init -s "$svnrepo"/project project 2>warning &&
- test_must_fail grep -q prefix warning &&
+ ! grep -q prefix warning &&
test_svn_configured_prefix "origin/" &&
rm -rf project &&
rm -f warning
@@ -95,7 +95,7 @@ test_expect_success 'init with -s/-T/-b/-t assumes --prefix=origin/' '
test_expect_success 'clone with -s/-T/-b/-t assumes --prefix=origin/' '
test ! -d project &&
git svn clone -s "$svnrepo"/project 2>warning &&
- test_must_fail grep -q prefix warning &&
+ ! grep -q prefix warning &&
test_svn_configured_prefix "origin/" &&
rm -rf project &&
rm -f warning
@@ -104,7 +104,7 @@ test_expect_success 'clone with -s/-T/-b/-t assumes --prefix=origin/' '
test_expect_success 'init with -s/-T/-b/-t and --prefix "" still works' '
test ! -d project &&
git svn init -s "$svnrepo"/project project --prefix "" 2>warning &&
- test_must_fail grep -q prefix warning &&
+ ! grep -q prefix warning &&
test_svn_configured_prefix "" &&
rm -rf project &&
rm -f warning
@@ -113,7 +113,7 @@ test_expect_success 'init with -s/-T/-b/-t and --prefix "" still works' '
test_expect_success 'clone with -s/-T/-b/-t and --prefix "" still works' '
test ! -d project &&
git svn clone -s "$svnrepo"/project --prefix "" 2>warning &&
- test_must_fail grep -q prefix warning &&
+ ! grep -q prefix warning &&
test_svn_configured_prefix "" &&
rm -rf project &&
rm -f warning
diff --git a/t/t9813-git-p4-preserve-users.sh b/t/t9813-git-p4-preserve-users.sh
index 0fe2312..76004a5 100755
--- a/t/t9813-git-p4-preserve-users.sh
+++ b/t/t9813-git-p4-preserve-users.sh
@@ -126,13 +126,13 @@ test_expect_success 'not preserving user with mixed authorship' '
grep "git author charlie@example.com does not match" &&
make_change_by_user usernamefile3 alice alice@example.com &&
- git p4 commit |\
- test_must_fail grep "git author.*does not match" &&
+ git p4 commit >actual &&
+ ! grep "git author.*does not match" actual &&
git config git-p4.skipUserNameCheck true &&
make_change_by_user usernamefile3 Charlie charlie@example.com &&
- git p4 commit |\
- test_must_fail grep "git author.*does not match" &&
+ git p4 commit >actual &&
+ ! grep "git author.*does not match" actual &&
p4_check_commit_author usernamefile3 alice
)
diff --git a/t/t9814-git-p4-rename.sh b/t/t9814-git-p4-rename.sh
index c89992c..e7e0268 100755
--- a/t/t9814-git-p4-rename.sh
+++ b/t/t9814-git-p4-rename.sh
@@ -141,7 +141,7 @@ test_expect_success 'detect copies' '
git diff-tree -r -C HEAD &&
git p4 submit &&
p4 filelog //depot/file8 &&
- p4 filelog //depot/file8 | test_must_fail grep -q "branch from" &&
+ ! p4 filelog //depot/file8 | grep -q "branch from" &&
echo "file9" >>file2 &&
git commit -a -m "Differentiate file2" &&
@@ -154,7 +154,7 @@ test_expect_success 'detect copies' '
git config git-p4.detectCopies true &&
git p4 submit &&
p4 filelog //depot/file9 &&
- p4 filelog //depot/file9 | test_must_fail grep -q "branch from" &&
+ ! p4 filelog //depot/file9 | grep -q "branch from" &&
echo "file10" >>file2 &&
git commit -a -m "Differentiate file2" &&
@@ -202,7 +202,7 @@ test_expect_success 'detect copies' '
git config git-p4.detectCopies $(($level + 2)) &&
git p4 submit &&
p4 filelog //depot/file12 &&
- p4 filelog //depot/file12 | test_must_fail grep -q "branch from" &&
+ ! p4 filelog //depot/file12 | grep -q "branch from" &&
echo "file13" >>file2 &&
git commit -a -m "Differentiate file2" &&
--
https://github.com/git/git/pull/314
^ permalink raw reply related
* [PATCH v4 2/2] t9813: avoid using pipes
From: Pranit Bauva @ 2017-01-08 16:55 UTC (permalink / raw)
To: git
In-Reply-To: <010201597f017978-356bf9e9-ee78-498b-926b-5c00466b1d9e-000000@eu-west-1.amazonses.com>
The exit code of the upstream in a pipe is ignored thus we should avoid
using it. By writing out the output of the git command to a file, we can
test the exit codes of both the commands.
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
---
t/t9813-git-p4-preserve-users.sh | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/t/t9813-git-p4-preserve-users.sh b/t/t9813-git-p4-preserve-users.sh
index 76004a5..bda222a 100755
--- a/t/t9813-git-p4-preserve-users.sh
+++ b/t/t9813-git-p4-preserve-users.sh
@@ -118,12 +118,12 @@ test_expect_success 'not preserving user with mixed authorship' '
make_change_by_user usernamefile3 Derek derek@example.com &&
P4EDITOR=cat P4USER=alice P4PASSWD=secret &&
export P4EDITOR P4USER P4PASSWD &&
- git p4 commit |\
- grep "git author derek@example.com does not match" &&
+ git p4 commit >actual &&
+ grep "git author derek@example.com does not match" actual &&
make_change_by_user usernamefile3 Charlie charlie@example.com &&
- git p4 commit |\
- grep "git author charlie@example.com does not match" &&
+ git p4 commit >actual &&
+ grep "git author charlie@example.com does not match" actual &&
make_change_by_user usernamefile3 alice alice@example.com &&
git p4 commit >actual &&
--
https://github.com/git/git/pull/314
^ permalink raw reply related
* Re: [PATCH] Makefile: put LIBS after LDFLAGS for imap-send
From: Johannes Schindelin @ 2017-01-08 18:54 UTC (permalink / raw)
To: Steven Penny; +Cc: git
In-Reply-To: <CAAXzdLVXUdCAcJL6DratNwLFUSN4UAV+TmALSZe-zSSTAJcWWw@mail.gmail.com>
Hi Steven,
On Sun, 8 Jan 2017, Steven Penny wrote:
> On Sun, Jan 8, 2017 at 5:54 AM, Johannes Schindelin wrote:
> > I am curious: how do you build Git? I ask because I build Git on Windows
> > many times a day, and I did not encounter any link problems.
>
> My end goal is to build static native Windows Git via Cygwin and the
> mingw64-x86_64-gcc-core package.
That is certainly a worthy goal, and I would highly recommend to mention
that particular cross-compiling setup in the commit message. It's not like
this is the easiest way to build native Git on Windows...
Ciao,
Johannes
^ permalink raw reply
* [PATCH v1] convert: add "status=delayed" to filter process protocol
From: larsxschneider @ 2017-01-08 19:17 UTC (permalink / raw)
To: git; +Cc: gitster, e, jnareb, Lars Schneider
From: Lars Schneider <larsxschneider@gmail.com>
Some `clean` / `smudge` filters might require a significant amount of
time to process a single blob. During this process the Git checkout
operation is blocked and Git needs to wait until the filter is done to
continue with the checkout.
Teach the filter process protocol (introduced in edcc858) to accept the
status "delayed" as response to a filter request. Upon this response Git
continues with the checkout operation and asks the filter to process the
blob again after all other blobs have been processed.
Git has a multiple code paths that checkout a blob. Support delayed
checkouts only in `clone` (in unpack-trees.c) and `checkout` operations.
Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
---
You can find the RFC thread for this topic here:
http://public-inbox.org/git/D10F7C47-14E8-465B-8B7A-A09A1B28A39F@gmail.com/
Notes:
Base Commit: e05806da9e (master)
Diff on Web: https://github.com/larsxschneider/git/commit/ea25a1834b
Checkout: git fetch https://github.com/larsxschneider/git filter-process/delay-v1 && git checkout ea25a1834b
Interdiff (filter-process/delay-v0..filter-process/delay-v1):
Documentation/gitattributes.txt | 9 +++++++
apply.c | 2 +-
archive.c | 2 +-
builtin/cat-file.c | 2 +-
builtin/checkout.c | 1 +
cache.h | 1 +
convert.c | 33 ++++++++++++++-----------
convert.h | 2 +-
diff.c | 2 +-
entry.c | 40 +++++++++++++++++++++++++++----
merge-recursive.c | 2 +-
t/t0021-conversion.sh | 53 +++++++++++++++++++++++++++++++++++++++++
t/t0021/rot13-filter.pl | 19 +++++++++++++++
unpack-trees.c | 1 +
14 files changed, 145 insertions(+), 24 deletions(-)
diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index e0b66c1220..f6bad8db40 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -473,6 +473,15 @@ packet: git< 0000 # empty content!
packet: git< 0000 # empty list, keep "status=success" unchanged!
------------------------
+If the request cannot be fulfilled within a reasonable amount of time
+then the filter can respond with a "delayed" status and a flush packet.
+Git will perform the same request at a later point in time, again. The
+filter can delay a response multiple times for a single request.
+------------------------
+packet: git< status=delayed
+packet: git< 0000
+------------------------
+
In case the filter cannot or does not want to process the content,
it is expected to respond with an "error" status.
------------------------
diff --git a/apply.c b/apply.c
index 2ed808d429..aa7e6e4359 100644
--- a/apply.c
+++ b/apply.c
@@ -4328,7 +4328,7 @@ static int try_create_file(const char *path, unsigned int mode, const char *buf,
if (fd < 0)
return 1;
- if (convert_to_working_tree(path, buf, size, &nbuf)) {
+ if (convert_to_working_tree(path, buf, size, &nbuf, NULL)) {
size = nbuf.len;
buf = nbuf.buf;
}
diff --git a/archive.c b/archive.c
index 01751e574b..90d02463ef 100644
--- a/archive.c
+++ b/archive.c
@@ -77,7 +77,7 @@ void *sha1_file_to_archive(const struct archiver_args *args,
size_t size = 0;
strbuf_attach(&buf, buffer, *sizep, *sizep + 1);
- convert_to_working_tree(path, buf.buf, buf.len, &buf);
+ convert_to_working_tree(path, buf.buf, buf.len, &buf, NULL);
if (commit)
format_subst(commit, buf.buf, buf.len, &buf);
buffer = strbuf_detach(&buf, &size);
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 30383e9eb4..4e3e31a00b 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -35,7 +35,7 @@ static int filter_object(const char *path, unsigned mode,
oid_to_hex(oid), path);
if ((type == OBJ_BLOB) && S_ISREG(mode)) {
struct strbuf strbuf = STRBUF_INIT;
- if (convert_to_working_tree(path, *buf, *size, &strbuf)) {
+ if (convert_to_working_tree(path, *buf, *size, &strbuf, NULL)) {
free(*buf);
*size = strbuf.len;
*buf = strbuf_detach(&strbuf, NULL);
diff --git a/builtin/checkout.c b/builtin/checkout.c
index bfe685c198..42885cfe92 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -369,6 +369,7 @@ static int checkout_paths(const struct checkout_opts *opts,
pos = skip_same_name(ce, pos) - 1;
}
}
+ errs |= checkout_delayed_entries(&state);
if (write_locked_index(&the_index, lock_file, COMMIT_LOCK))
die(_("unable to write new index file"));
diff --git a/cache.h b/cache.h
index a50a61a197..5d119ffc6b 100644
--- a/cache.h
+++ b/cache.h
@@ -1375,6 +1375,7 @@ struct checkout {
#define TEMPORARY_FILENAME_LENGTH 25
extern int checkout_entry(struct cache_entry *ce, const struct checkout *state, char *topath);
+extern int checkout_delayed_entries(const struct checkout *state);
struct cache_def {
struct strbuf path;
diff --git a/convert.c b/convert.c
index 4e17e45ed2..4075a58e64 100644
--- a/convert.c
+++ b/convert.c
@@ -672,7 +672,7 @@ static struct cmd2process *start_multi_file_filter(struct hashmap *hashmap, cons
}
static int apply_multi_file_filter(const char *path, const char *src, size_t len,
- int fd, struct strbuf *dst, const char *cmd,
+ int fd, struct strbuf *dst, int *delayed, const char *cmd,
const unsigned int wanted_capability)
{
int err;
@@ -738,9 +738,14 @@ static int apply_multi_file_filter(const char *path, const char *src, size_t len
goto done;
read_multi_file_filter_status(process->out, &filter_status);
- err = strcmp(filter_status.buf, "success");
- if (err)
+ if (delayed && !strcmp(filter_status.buf, "delayed")) {
+ *delayed = 1;
goto done;
+ } else {
+ err = strcmp(filter_status.buf, "success");
+ if (err)
+ goto done;
+ }
err = read_packetized_to_strbuf(process->out, &nbuf) < 0;
if (err)
@@ -787,8 +792,8 @@ static struct convert_driver {
} *user_convert, **user_convert_tail;
static int apply_filter(const char *path, const char *src, size_t len,
- int fd, struct strbuf *dst, struct convert_driver *drv,
- const unsigned int wanted_capability)
+ int fd, struct strbuf *dst, int *delayed,
+ struct convert_driver *drv, const unsigned int wanted_capability)
{
const char *cmd = NULL;
@@ -806,7 +811,7 @@ static int apply_filter(const char *path, const char *src, size_t len,
if (cmd && *cmd)
return apply_single_file_filter(path, src, len, fd, dst, cmd);
else if (drv->process && *drv->process)
- return apply_multi_file_filter(path, src, len, fd, dst, drv->process, wanted_capability);
+ return apply_multi_file_filter(path, src, len, fd, dst, delayed, drv->process, wanted_capability);
return 0;
}
@@ -1152,7 +1157,7 @@ int would_convert_to_git_filter_fd(const char *path)
if (!ca.drv->required)
return 0;
- return apply_filter(path, NULL, 0, -1, NULL, ca.drv, CAP_CLEAN);
+ return apply_filter(path, NULL, 0, -1, NULL, NULL, ca.drv, CAP_CLEAN);
}
const char *get_convert_attr_ascii(const char *path)
@@ -1189,7 +1194,7 @@ int convert_to_git(const char *path, const char *src, size_t len,
convert_attrs(&ca, path);
- ret |= apply_filter(path, src, len, -1, dst, ca.drv, CAP_CLEAN);
+ ret |= apply_filter(path, src, len, -1, dst, NULL, ca.drv, CAP_CLEAN);
if (!ret && ca.drv && ca.drv->required)
die("%s: clean filter '%s' failed", path, ca.drv->name);
@@ -1214,7 +1219,7 @@ void convert_to_git_filter_fd(const char *path, int fd, struct strbuf *dst,
assert(ca.drv);
assert(ca.drv->clean || ca.drv->process);
- if (!apply_filter(path, NULL, 0, fd, dst, ca.drv, CAP_CLEAN))
+ if (!apply_filter(path, NULL, 0, fd, dst, NULL, ca.drv, CAP_CLEAN))
die("%s: clean filter '%s' failed", path, ca.drv->name);
crlf_to_git(path, dst->buf, dst->len, dst, ca.crlf_action, checksafe);
@@ -1222,7 +1227,7 @@ void convert_to_git_filter_fd(const char *path, int fd, struct strbuf *dst,
}
static int convert_to_working_tree_internal(const char *path, const char *src,
- size_t len, struct strbuf *dst,
+ size_t len, struct strbuf *dst, int *delayed,
int normalizing)
{
int ret = 0, ret_filter = 0;
@@ -1248,21 +1253,21 @@ static int convert_to_working_tree_internal(const char *path, const char *src,
}
}
- ret_filter = apply_filter(path, src, len, -1, dst, ca.drv, CAP_SMUDGE);
+ ret_filter = apply_filter(path, src, len, -1, dst, delayed, ca.drv, CAP_SMUDGE);
if (!ret_filter && ca.drv && ca.drv->required)
die("%s: smudge filter %s failed", path, ca.drv->name);
return ret | ret_filter;
}
-int convert_to_working_tree(const char *path, const char *src, size_t len, struct strbuf *dst)
+int convert_to_working_tree(const char *path, const char *src, size_t len, struct strbuf *dst, int *delayed)
{
- return convert_to_working_tree_internal(path, src, len, dst, 0);
+ return convert_to_working_tree_internal(path, src, len, dst, delayed, 0);
}
int renormalize_buffer(const char *path, const char *src, size_t len, struct strbuf *dst)
{
- int ret = convert_to_working_tree_internal(path, src, len, dst, 1);
+ int ret = convert_to_working_tree_internal(path, src, len, dst, NULL, 1);
if (ret) {
src = dst->buf;
len = dst->len;
diff --git a/convert.h b/convert.h
index 82871a11d5..72f24078de 100644
--- a/convert.h
+++ b/convert.h
@@ -41,7 +41,7 @@ extern const char *get_convert_attr_ascii(const char *path);
extern int convert_to_git(const char *path, const char *src, size_t len,
struct strbuf *dst, enum safe_crlf checksafe);
extern int convert_to_working_tree(const char *path, const char *src,
- size_t len, struct strbuf *dst);
+ size_t len, struct strbuf *dst, int *delayed);
extern int renormalize_buffer(const char *path, const char *src, size_t len,
struct strbuf *dst);
static inline int would_convert_to_git(const char *path)
diff --git a/diff.c b/diff.c
index 84dba60c40..243cd0ffdf 100644
--- a/diff.c
+++ b/diff.c
@@ -2960,7 +2960,7 @@ static void prep_temp_blob(const char *path, struct diff_tempfile *temp,
if (fd < 0)
die_errno("unable to create temp-file");
if (convert_to_working_tree(path,
- (const char *)blob, (size_t)size, &buf)) {
+ (const char *)blob, (size_t)size, &buf, NULL)) {
blob = buf.buf;
size = buf.len;
}
diff --git a/entry.c b/entry.c
index c6eea240b6..43f3ae7b69 100644
--- a/entry.c
+++ b/entry.c
@@ -2,6 +2,14 @@
#include "blob.h"
#include "dir.h"
#include "streaming.h"
+#include "list.h"
+
+static LIST_HEAD(delayed_cache_entry_queue_head);
+
+struct delayed_cache_entry {
+ struct cache_entry *ce;
+ struct list_head node;
+};
static void create_directories(const char *path, int path_len,
const struct checkout *state)
@@ -140,12 +148,13 @@ static int write_entry(struct cache_entry *ce,
char *path, const struct checkout *state, int to_tempfile)
{
unsigned int ce_mode_s_ifmt = ce->ce_mode & S_IFMT;
- int fd, ret, fstat_done = 0;
+ int fd, ret, fstat_done = 0, delayed = 0;
char *new;
struct strbuf buf = STRBUF_INIT;
unsigned long size;
size_t wrote, newsize = 0;
struct stat st;
+ struct delayed_cache_entry *delayed_ce;
if (ce_mode_s_ifmt == S_IFREG) {
struct stream_filter *filter = get_stream_filter(ce->name,
@@ -178,10 +187,17 @@ static int write_entry(struct cache_entry *ce,
* Convert from git internal format to working tree format
*/
if (ce_mode_s_ifmt == S_IFREG &&
- convert_to_working_tree(ce->name, new, size, &buf)) {
+ convert_to_working_tree(ce->name, new, size, &buf, &delayed)) {
free(new);
- new = strbuf_detach(&buf, &newsize);
- size = newsize;
+ if (delayed) {
+ delayed_ce = xmalloc(sizeof(*delayed_ce));
+ delayed_ce->ce = ce;
+ list_add_tail(&delayed_ce->node, &delayed_cache_entry_queue_head);
+ goto finish;
+ } else {
+ new = strbuf_detach(&buf, &newsize);
+ size = newsize;
+ }
}
fd = open_output_fd(path, ce, to_tempfile);
@@ -291,3 +307,19 @@ int checkout_entry(struct cache_entry *ce,
create_directories(path.buf, path.len, state);
return write_entry(ce, path.buf, state, 0);
}
+
+int checkout_delayed_entries(const struct checkout *state)
+{
+ struct delayed_cache_entry *head;
+ int errs = 0;
+
+ while (!list_empty(&delayed_cache_entry_queue_head)) {
+ head = list_first_entry(&delayed_cache_entry_queue_head,
+ struct delayed_cache_entry, node);
+ list_del(&head->node);
+ head->ce->ce_flags &= ~CE_UPDATE;
+ errs |= checkout_entry(head->ce, state, NULL);
+ }
+
+ return errs;
+}
diff --git a/merge-recursive.c b/merge-recursive.c
index d327209443..73dd33e61f 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -807,7 +807,7 @@ static int update_file_flags(struct merge_options *o,
}
if (S_ISREG(mode)) {
struct strbuf strbuf = STRBUF_INIT;
- if (convert_to_working_tree(path, buf, size, &strbuf)) {
+ if (convert_to_working_tree(path, buf, size, &strbuf, NULL)) {
free(buf);
size = strbuf.len;
buf = strbuf_detach(&strbuf, NULL);
diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index 161f560446..8ae5b1a521 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -701,4 +701,57 @@ test_expect_success PERL 'invalid process filter must fail (and not hang!)' '
)
'
+test_expect_success PERL 'delayed checkout in process filter' '
+ test_config_global filter.protocol.process "rot13-filter.pl clean smudge" &&
+ test_config_global filter.protocol.required true &&
+ rm -rf repo &&
+ mkdir repo &&
+ (
+ cd repo &&
+ git init &&
+ echo "*.r filter=protocol" >.gitattributes &&
+ cp "$TEST_ROOT/test.o" test.r &&
+ cp "$TEST_ROOT/test.o" test-delay1.r &&
+ cp "$TEST_ROOT/test.o" test-delay3.r &&
+ git add . &&
+ git commit -m "test commit 1"
+ ) &&
+
+ S=$(file_size repo/test.r) &&
+ rm -rf repo-cloned &&
+ filter_git clone repo repo-cloned &&
+ cat >expected.log <<-EOF &&
+ START
+ init handshake complete
+ IN: smudge test.r $S [OK] -- OUT: $S . [OK]
+ IN: smudge test-delay1.r $S [OK] -- OUT: $S [DELAYED]
+ IN: smudge test-delay1.r $S [OK] -- OUT: $S . [OK]
+ IN: smudge test-delay3.r $S [OK] -- OUT: $S [DELAYED]
+ IN: smudge test-delay3.r $S [OK] -- OUT: $S [DELAYED]
+ IN: smudge test-delay3.r $S [OK] -- OUT: $S [DELAYED]
+ IN: smudge test-delay3.r $S [OK] -- OUT: $S . [OK]
+ STOP
+ EOF
+ test_cmp_count expected.log repo-cloned/rot13-filter.log &&
+
+ (
+ cd repo-cloned &&
+ rm *.r rot13-filter.log &&
+ filter_git checkout . &&
+ cat >expected.log <<-EOF &&
+ START
+ init handshake complete
+ IN: smudge test.r $S [OK] -- OUT: $S . [OK]
+ IN: smudge test-delay1.r $S [OK] -- OUT: $S [DELAYED]
+ IN: smudge test-delay1.r $S [OK] -- OUT: $S . [OK]
+ IN: smudge test-delay3.r $S [OK] -- OUT: $S [DELAYED]
+ IN: smudge test-delay3.r $S [OK] -- OUT: $S [DELAYED]
+ IN: smudge test-delay3.r $S [OK] -- OUT: $S [DELAYED]
+ IN: smudge test-delay3.r $S [OK] -- OUT: $S . [OK]
+ STOP
+ EOF
+ test_cmp_count expected.log rot13-filter.log
+ )
+'
+
test_done
diff --git a/t/t0021/rot13-filter.pl b/t/t0021/rot13-filter.pl
index 617f581e56..b10be1e543 100644
--- a/t/t0021/rot13-filter.pl
+++ b/t/t0021/rot13-filter.pl
@@ -17,6 +17,10 @@
# operation then the filter signals that it cannot or does not want
# to process the file and any file after that is processed with the
# same command.
+# (5) If data with a pathname that is a key in the DELAY hash is
+# processed (e.g. 'test-delay1.r') then the filter signals n times
+# to Git that the processing is delayed (n being the value of the
+# DELAY hash key).
#
use strict;
@@ -25,6 +29,12 @@ use IO::File;
my $MAX_PACKET_CONTENT_SIZE = 65516;
my @capabilities = @ARGV;
+my $DELAY3 = 3;
+my $DELAY1 = 1;
+
+my %DELAY;
+$DELAY{'test-delay1.r'} = 1;
+$DELAY{'test-delay3.r'} = 3;
open my $debug, ">>", "rot13-filter.log" or die "cannot open log file: $!";
@@ -166,6 +176,15 @@ while (1) {
packet_txt_write("status=abort");
packet_flush();
}
+ elsif ( $command eq "smudge" and
+ exists $DELAY{$pathname} and
+ $DELAY{$pathname} gt 0 ) {
+ $DELAY{$pathname} = $DELAY{$pathname} - 1;
+ print $debug "[DELAYED]\n";
+ $debug->flush();
+ packet_txt_write("status=delayed");
+ packet_flush();
+ }
else {
packet_txt_write("status=success");
packet_flush();
diff --git a/unpack-trees.c b/unpack-trees.c
index 7a6df99d10..662b75f72a 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -268,6 +268,7 @@ static int check_updates(struct unpack_trees_options *o,
}
}
}
+ errs |= checkout_delayed_entries(state);
stop_progress(&progress);
if (o->update)
git_attr_set_direction(GIT_ATTR_CHECKIN, NULL);
--
2.11.0
^ permalink raw reply related
* Re: [PATCH v1] convert: add "status=delayed" to filter process protocol
From: Torsten Bögershausen @ 2017-01-08 20:14 UTC (permalink / raw)
To: larsxschneider; +Cc: git, gitster, e, jnareb
In-Reply-To: <20170108191736.47359-1-larsxschneider@gmail.com>
On Sun, Jan 08, 2017 at 08:17:36PM +0100, larsxschneider@gmail.com wrote:
> From: Lars Schneider <larsxschneider@gmail.com>
>
> Some `clean` / `smudge` filters might require a significant amount of
> time to process a single blob. During this process the Git checkout
> operation is blocked and Git needs to wait until the filter is done to
> continue with the checkout.
>
> Teach the filter process protocol (introduced in edcc858) to accept the
> status "delayed" as response to a filter request. Upon this response Git
> continues with the checkout operation and asks the filter to process the
> blob again after all other blobs have been processed.
>
> Git has a multiple code paths that checkout a blob. Support delayed
> checkouts only in `clone` (in unpack-trees.c) and `checkout` operations.
>
> Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
> ---
>
Some feeling tells me that it may be better to leave convert_to_working_tree() as it is.
And change convert_to_working_tree_internal as suggested:
int convert_to_working_tree(const char *path, const char *src, size_t len, struct strbuf *dst)
{
- return convert_to_working_tree_internal(path, src, len, dst, 0);
+ return convert_to_working_tree_internal(path, src, len, dst, NULL, 0);
}
^ permalink raw reply
* Re: [PATCH v1] convert: add "status=delayed" to filter process protocol
From: Eric Wong @ 2017-01-08 20:45 UTC (permalink / raw)
To: larsxschneider; +Cc: git, gitster, jnareb
In-Reply-To: <20170108191736.47359-1-larsxschneider@gmail.com>
larsxschneider@gmail.com wrote:
> +++ b/t/t0021/rot13-filter.pl
> +$DELAY{'test-delay1.r'} = 1;
> +$DELAY{'test-delay3.r'} = 3;
>
> open my $debug, ">>", "rot13-filter.log" or die "cannot open log file: $!";
>
> @@ -166,6 +176,15 @@ while (1) {
> packet_txt_write("status=abort");
> packet_flush();
> }
> + elsif ( $command eq "smudge" and
> + exists $DELAY{$pathname} and
> + $DELAY{$pathname} gt 0 ) {
Use '>' for numeric comparisons. 'gt' is for strings (man perlop)
Sidenote, staying <= 80 columns for the rest of the changes is
strongly preferred, some of us need giant fonts. I think what
Torsten said about introducing a new *_internal function can
also help with that.
Thanks.
^ permalink raw reply
* Re: [PATCH v1] convert: add "status=delayed" to filter process protocol
From: Junio C Hamano @ 2017-01-08 23:42 UTC (permalink / raw)
To: larsxschneider; +Cc: git, e, jnareb
In-Reply-To: <20170108191736.47359-1-larsxschneider@gmail.com>
larsxschneider@gmail.com writes:
> From: Lars Schneider <larsxschneider@gmail.com>
>
> Some `clean` / `smudge` filters might require a significant amount of
> time to process a single blob. During this process the Git checkout
> operation is blocked and Git needs to wait until the filter is done to
> continue with the checkout.
>
> Teach the filter process protocol (introduced in edcc858) to accept the
> status "delayed" as response to a filter request. Upon this response Git
> continues with the checkout operation and asks the filter to process the
> blob again after all other blobs have been processed.
Hmm, I would have expected that the basic flow would become
for each paths to be processed:
convert-to-worktree to buf
if not delayed:
do the caller's thing to use buf
else:
remember path
for each delayed paths:
ensure filter process finished processing for path
fetch the thing to buf from the process
do the caller's thing to use buf
and that would make quite a lot of sense. However, what is actually
implemented is a bit disappointing from that point of view. While
its first part is the same as above, the latter part instead does:
for each delayed paths:
checkout the path
Presumably, checkout_entry() does the "ensure that the process is
done converting" (otherwise the result is simply buggy), but what
disappoints me is that this does not allow callers that call
"convert-to-working-tree", whose interface is obtain the bytestream
in-core in the working tree representation, given an object in the
object-db representation in an in-core buffer, to _use_ the result
of the conversion. The caller does not have a chance to even see
the result as it is written straight to the filesystem, once it
calls checkout_delayed_entries().
^ permalink raw reply
* Re: [PATCH] connect: handle putty/plink also in GIT_SSH_COMMAND
From: Junio C Hamano @ 2017-01-09 1:08 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Segev Finer
In-Reply-To: <xmqqy3ym1dsc.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> I suspect that this will break when GIT_SSH_COMMAND, which is meant
> to be processed by the shell, hence the user can write anything,
> begins with a one-shot environment variable assignment, e.g.
>
> [core] sshcommand = VAR1=VAL1 VAR2=VAL2 //path/to/tortoiseplink
>
> One possible unintended side effect of this patch is when VAL1 ends
> with /plink (or /tortoiseplink) and the command is not either of
> these, in which case the "tortoiseplink" and "putty" variables will
> tweak the command line for an SSH implementation that does not want
> such a tweak to be made. There may be other unintended fallouts.
Thinking about this further, as the sshcommand (or GIT_SSH_COMMAND)
interface takes any shell-interpretable commands, the first "token"
you see is not limited to a one-shot environment assignment. It
could even be "cmd1 && cmd2 && ..." or even:
if test -f ${TPLINK:=//path/to/tortoiseplink}
then
exec "$TPLINK" "$@"
elif test -f ${PLINK:=//path/to/plink}
exec "$PLINK" "$@"
else
echo >&2 no usable ssh on this host
fi
Worse, the above may be in "myssh" script found on user's PATH and
then core.sshcommand may be set to
TPLINK=//my/path/to/tortoiseplink PLINK=//my/path/to/plink myssh
in the configuration the user uses on multiple hosts, some have
tortoiseplink installed and some do not. The idea the user who set
it up may be to use whatever available depending on the host.
The posted patch would be confused that we are using tortoiseplink
but the "myssh" script would correctly notice that on a particular
host it is unavailable and choose to use plink instead.
> Sorry, no concrete suggestion to get this to work comes to my mind
> offhand.
>
> Perhaps give an explicit way to force "tortoiseplink" and "putty"
> variables without looking at and guessing from the pathname, so that
> the solution does not have to split and peek the command line?
A user with such an elaborate set-up with multiple hosts with
different configurations would likely to want to say "Just give me a
regular SSH command line, and in my 'myssh' script I'll futz with
-p/-P differences and such, as I'm writing a small script to cope
with Tortoiseplink and Puttyplink anyway". With a separate,
explicit configuration variable to tell Git what variant of SSH you
have, even such a user can be served (she would just set ssh.variant
to "vanilla" or whatever that is not "tortoiseplink" or "plink").
.
^ permalink raw reply
* Re: [PATCH v4 1/4] Avoid Coverity warning about unfree()d git_exec_path()
From: Junio C Hamano @ 2017-01-09 1:25 UTC (permalink / raw)
To: Stefan Beller
Cc: Johannes Schindelin, git@vger.kernel.org, David Aguilar,
Dennis Kaarsemaker, Paul Sbarra
In-Reply-To: <CAGZ79kZ--jp08pK+xwn1N2VQQr8bA5+DveE2HsoY90R1gR6c_A@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
> On Mon, Jan 2, 2017 at 8:22 AM, Johannes Schindelin
> <johannes.schindelin@gmx.de> wrote:
>> Technically, it is correct that git_exec_path() returns a possibly
>> malloc()ed string. Practically, it is *sometimes* not malloc()ed. So
>> let's just use a static variable to make it a singleton. That'll shut
>> Coverity up, hopefully.
>
> I picked up this patch and applied it to the coverity branch
> that I maintain at github/stefanbeller/git.
>
> I'd love to see this patch upstream as it reduces my maintenance
> burden of the coverity branch by a patch.
So with the above, are you saying "Dscho said 'hopefully', and I
confirm that this change does squelch misdiagnosis by Coverity"?
> If this patch is only to appease coverity (as the commit message eludes
> to) I think this may be a bad idea for upstream. If this patch fixes an
> actual problem, then the commit message needs to spell that out.
That is true, and I see Peff pointed out another possible issue
around getenv(), but I think from the "one step at a time" point of
view, it is an improvement to call system_path() just once and cache
it in "static char *".
How about explaining it like this then?
(only the log message has been corrected; diff is from the original).
commit c9bb5d101ca657fa466afa8c4368c43ea7b7aca8
Author: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Mon Jan 2 17:22:33 2017 +0100
git_exec_path: avoid Coverity warning about unfree()d result
Technically, it is correct that git_exec_path() returns a possibly
malloc()ed string returned from system_path(), and it is sometimes
not allocated. Cache the result in a static variable and make sure
that we call system_path() only once, which plugs a potential leak.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
diff --git a/exec_cmd.c b/exec_cmd.c
index 9d5703a157..eae56fefba 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -69,6 +69,7 @@ void git_set_argv_exec_path(const char *exec_path)
const char *git_exec_path(void)
{
const char *env;
+ static char *system_exec_path;
if (argv_exec_path)
return argv_exec_path;
@@ -78,7 +79,9 @@ const char *git_exec_path(void)
return env;
}
- return system_path(GIT_EXEC_PATH);
+ if (!system_exec_path)
+ system_exec_path = system_path(GIT_EXEC_PATH);
+ return system_exec_path;
}
static void add_path(struct strbuf *out, const char *path)
^ permalink raw reply related
* Re: [PATCH v4 4/4] t7800: run both builtin and scripted difftool, for now
From: Junio C Hamano @ 2017-01-09 1:38 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, David Aguilar, Dennis Kaarsemaker, Paul Sbarra
In-Reply-To: <0ae4a950a4cd2c8c4f05a6b46c60f127611580cf.1483373635.git.johannes.schindelin@gmx.de>
Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> This is uglier than a simple
>
> touch "$GIT_EXEC_PATH/use-builtin-difftool"
>
> of course. But oh well.
That is totally irrelevant.
The more important point is that we do the same set of tests so
instead of running just one, we run BOTH. If we did a bit more
work, we could even say "even when there is no Perl installed, we
run tests with only the new built-in one". This does not go that
far yet, but that should not be too hard, I would think. See below.
> diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
> index e94910c563..273ab55723 100755
> --- a/t/t7800-difftool.sh
> +++ b/t/t7800-difftool.sh
> @@ -23,6 +23,20 @@ prompt_given ()
> test "$prompt" = "Launch 'test-tool' [Y/n]? branch"
> }
>
> +for use_builtin_difftool in false true
> +do
Instead of the above, I'd suggest to make the loop like so:
for use_builtin_difftool in $TESTED_VARIANTS
do
and before the loop begins, set TESTED_VARIANTS to either "false
true" or "true" by checking the PERL prerequisite.
> +test_expect_success 'verify we are running the correct difftool' '
> + if test true = '$use_builtin_difftool'
> + then
> + test_must_fail ok=129 git difftool -h >help &&
> + grep "g, --gui" help
> + else
> + git difftool -h >help &&
> + grep "g|--gui" help
> + fi
> +'
> +
> # NEEDSWORK: lose all the PERL prereqs once legacy-difftool is retired.
And then we can lose this comment. As all existing tests have PERL
prereq that can go away with the above suggested change, we'd need
to touch quite a many lines with "s/_success PERL /_success /".
It may be a good time to indent these existing tests to make it
clear that they are inside the new "for use_builtin_difftool" loop.
> +test true != $use_builtin_difftool || break
> +
> +test_expect_success 'tear down for re-run' '
> + rm -rf * .[a-z]* &&
> + git init
> +'
> +
> +# run as builtin difftool now
> +GIT_CONFIG_PARAMETERS="'difftool.usebuiltin=true'"
> +export GIT_CONFIG_PARAMETERS
... and indentation would extend to these newly added lines, of
course.
> +done
> +
> test_done
^ 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