* Re: [PATCH] t0021, t5615: use $PWD instead of $(pwd) in PATH-like shell variables
From: Johannes Schindelin @ 2016-11-12 11:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, git, Jeff King
In-Reply-To: <xmqqa8d53ky9.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Fri, 11 Nov 2016, Junio C Hamano wrote:
> Johannes Sixt <j6t@kdbg.org> writes:
>
> > We have to use $PWD instead of $(pwd) because on Windows the latter
> > would add a C: style path to bash's Unix-style $PATH variable, which
> > becomes confused by the colon after the drive letter. ($PWD is a
> > Unix-style path.)
> >
> > In the case of GIT_ALTERNATE_OBJECT_DIRECTORIES, bash on Windows
> > assembles a Unix-style path list with the colon as separators. It
> > converts the value to a Windows-style path list with the semicolon as
> > path separator when it forwards the variable to git.exe. The same
> > confusion happens when bash's original value is contaminated with
> > Windows style paths.
> >
> > Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> > ---
> > Am 11.11.2016 um 18:11 schrieb Johannes Sixt:
> >> Am 11.11.2016 um 18:06 schrieb Junio C Hamano:
> >>> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> >>> ...
> >
> > When the MSYS program such as bash invokes a non-MSYS program, it
> > translates the Unix-style paths in arguments and environment variables
> > to Windows stlye. We only have to ensure that we inject only Unix-style
> > paths in these places so as not to confuse the conversion algorithm.
> > Most of the time, we do not have to worry.
> >
> > On the other hand, when we write a path to a file that git.exe consumes
> > or receive a path from git.exe, i.e., when the path travels through
> > stdout and stdin, no automatic translation happens (which is quite
> > understandable), and we have do the translation explicitly. An example
> > for such a case is when we write a .git/info/alternates file via the
> > shell.
> >
> >> A simpler fix is to use $PWD instead of $(pwd). I'll submit a patch in a
> >> moment.
> >
> > Here it is. I had proposed the t0021 part earlier, but it fell through
> > the cracks during the temporary maintainer change.
>
> Thanks. Dscho, does this fix both of these issues to you?
Apparently it does because the CI jobs for `master` and for `next` pass.
The one for `pu` still times out, of course.
Ciao,
Dscho
^ permalink raw reply
* Re: [RFC/PATCH 0/2] git diff <(command1) <(command2)
From: Johannes Schindelin @ 2016-11-12 10:08 UTC (permalink / raw)
To: Jacob Keller; +Cc: Junio C Hamano, Dennis Kaarsemaker, Git mailing list
In-Reply-To: <CA+P7+xrFYj7sepdz1xrEEaqmXQWfRkBz3CsWjYK6OZsZRMLbUA@mail.gmail.com>
Hi,
On Fri, 11 Nov 2016, Jacob Keller wrote:
> On Fri, Nov 11, 2016 at 1:27 PM, Junio C Hamano <gitster@pobox.com> wrote:
> > Dennis Kaarsemaker <dennis@kaarsemaker.net> writes:
> >
> >> No tests or documentation updates yet, and I'm not sure whether
> >> --follow-symlinks in other modes than --no-index should be supported, ignored
> >> (as it is now) or cause an error, but I'm leaning towards the third option.
> >
> > My knee-jerk reaction is:
> >
> > * The --no-index mode should default to your --follow-symlinks
> > behaviour, without any option to turn it on or off.
> >
>
> I agree. We shouldn't have to specify this for no-index.
Ummm. *My* idea of --no-index was for it to behave as similar to the
--index version as possible. For example when comparing directories
containing symlinks. You seem intent on breaking this scenario.
Ciao,
Johannes
^ permalink raw reply
* Re: [PATCH v7 04/17] ref-filter: modify "%(objectname:short)" to take length
From: Karthik Nayak @ 2016-11-12 9:56 UTC (permalink / raw)
To: Jacob Keller; +Cc: Git mailing list
In-Reply-To: <CA+P7+xqXY6XvUm8Kna_GwY4XOSSGPM=-ezZECgkGcbnX_Penmg@mail.gmail.com>
Hello,
On Fri, Nov 11, 2016 at 10:59 AM, Jacob Keller <jacob.keller@gmail.com> wrote:
> On Thu, Nov 10, 2016 at 9:36 AM, Karthik Nayak <karthik.188@gmail.com> wrote:
>> On Wed, Nov 9, 2016 at 4:57 AM, Jacob Keller <jacob.keller@gmail.com> wrote:
>>
>> That does make sense, It would also not error out when we use
>> %(objectname:short=) and
>> not specify the length. Idk, if that's desirable or not. But it does
>> make the code a little more
>> confusing to read at the same time.
>>
>
> I am not sure that would be the case. If you see "objectname:short"
> you trreat this as if they had passed "objectname:short=<default
> abbrev>" but if you see "objectname:short=" you die, no?
>
Sorry, my bad.
On Fri, Nov 11, 2016 at 5:02 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> else if (!strcmp(arg, "short"))
>> - atom->u.objectname = O_SHORT;
>> - else
>> + atom->u.objectname.option = O_SHORT;
>> + else if (skip_prefix(arg, "short=", &arg)) {
>> + atom->u.objectname.option = O_LENGTH;
>> + if (strtoul_ui(arg, 10, &atom->u.objectname.length) ||
>> + atom->u.objectname.length == 0)
>> + die(_("positive value expected objectname:short=%s"), arg);
>> + if (atom->u.objectname.length < MINIMUM_ABBREV)
>> + atom->u.objectname.length = MINIMUM_ABBREV;
>> + } else
>> die(_("unrecognized %%(objectname) argument: %s"), arg);
>> }
>
> Users who want to use the default-abbrev, i.e. the autoscaling one
> introduced recently, must use "short", not "short=-1", with this
> code (especially with the "must be at least MINIMUM_ABBREV" logic),
> but I do not think it is a problem, so I think this is good.
>
I think I'll leave this as it is. If that's okay
--
Regards,
Karthik Nayak
^ permalink raw reply
* Re: [PATCH v7 03/17] ref-filter: implement %(if:equals=<string>) and %(if:notequals=<string>)
From: Karthik Nayak @ 2016-11-12 9:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jacob Keller, Git mailing list
In-Reply-To: <xmqqoa1m5335.fsf@gitster.mtv.corp.google.com>
Hello,
On Fri, Nov 11, 2016 at 4:56 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Jacob Keller <jacob.keller@gmail.com> writes:
>
>>> @@ -49,6 +51,10 @@ static struct used_atom {
>>> enum { C_BARE, C_BODY, C_BODY_DEP, C_LINES, C_SIG, C_SUB } option;
>>> unsigned int nlines;
>>> } contents;
>>> + struct {
>>> + const char *if_equals,
>>> + *not_equals;
>>
>>
>> Same here, why do we need both strings here stored separately? Could
>> we instead store which state to check and store the string once? I'm
>> not sure that really buys us any storage.
>
> I am not sure if storage is an issue, but I tend to agree that it
> would be semantically cleaner if this was done as a pair of <what
> operation uses this string constant?, the string constant>, and the
> former would be enum { COMPARE_EQUAL, COMPARE_UNEQUAL}.
>
> You could later enhance the comparison operator more easily with
> such an arrangement (e.g. if-equals-case-insensitively).
That's a rather good point you make there, I think using an enum with a string
constant would be ideal for future extensibility too. Thanks for the
explanation,
I shall change it.
--
Regards,
Karthik Nayak
^ permalink raw reply
* [PATCH] git-worktree.txt: Fix typo "to"/"two", add comma
From: Ben North @ 2016-11-12 8:55 UTC (permalink / raw)
To: git; +Cc: pclouds
Signed-off-by: Ben North <ben@redfrontdoor.org>
---
Documentation/git-worktree.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 0aeb020..e257c19 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -133,7 +133,7 @@ OPTIONS
+
If the last path components in the working tree's path is unique among
working trees, it can be used to identify worktrees. For example if
-you only have to working trees at "/abc/def/ghi" and "/abc/def/ggg",
+you only have two working trees, at "/abc/def/ghi" and "/abc/def/ggg",
then "ghi" or "def/ghi" is enough to point to the former working tree.
DETAILS
--
2.9.3
^ permalink raw reply related
* Re: [RFC/PATCH 0/2] git diff <(command1) <(command2)
From: Jeff King @ 2016-11-12 7:06 UTC (permalink / raw)
To: Dennis Kaarsemaker; +Cc: Junio C Hamano, git
In-Reply-To: <1478931094.12153.3.camel@kaarsemaker.net>
On Sat, Nov 12, 2016 at 07:11:34AM +0100, Dennis Kaarsemaker wrote:
> > * If normal "diff" that follows symlinks by default has an option
> > to disable it, then it is OK to also add --no-follow-symlinks
> > that is only valid in the --no-index mode, so that we can mimick
> > it better (I do not think this is the case, though).
>
> It does not, so no new option.
There is "--no-dereference", but it is not that helpful:
$ echo one >1
$ echo two >2
$ ln -s 1 sl1
$ ln -s 2 sl2
$ diff --no-dereference sl1 sl2
Symbolic links sl1 and sl2 differ
$ diff --no-dereference sl1 2
File sl1 is a symbolic link while file 2 is a regular file
So it does do the thing we are talking about here, but diff's handling
of non-file content is not nearly as useful as git's.
That said, I have no problem with proceeding without it and waiting
until somebody actually shows up who really _wants_ --no-dereference. I
won't be surprised if that never happens.
-Peff
^ permalink raw reply
* Re: [RFC/PATCH 0/2] git diff <(command1) <(command2)
From: Dennis Kaarsemaker @ 2016-11-12 6:11 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqinrt1zcx.fsf@gitster.mtv.corp.google.com>
On Fri, 2016-11-11 at 13:27 -0800, Junio C Hamano wrote:
> Dennis Kaarsemaker <dennis@kaarsemaker.net> writes:
>
> > No tests or documentation updates yet, and I'm not sure whether
> > --follow-symlinks in other modes than --no-index should be supported, ignored
> > (as it is now) or cause an error, but I'm leaning towards the third option.
>
>
> My knee-jerk reaction is:
>
> * The --no-index mode should default to your --follow-symlinks
> behaviour, without any option to turn it on or off.
ok.
> * If normal "diff" that follows symlinks by default has an option
> to disable it, then it is OK to also add --no-follow-symlinks
> that is only valid in the --no-index mode, so that we can mimick
> it better (I do not think this is the case, though).
It does not, so no new option.
> * Other modes should not follow symbolic links ever, no need for a
> new option.
Makes sense.
> In any case, I'd advise you not to reroll this too quickly and
> frequently until the end of this cycle. During a feature freeze, I
> won't take new topics in 'pu' as that would add more things I need
> to worry about, and if you reroll in too quick succession, it will
> become harder to identify the latest set and queue after the
> release.
I'm in no hurry, so I'll sit on this until v2.11 is done.
D.
^ permalink raw reply
* [PATCH 11/11] worktree remove: new command
From: Nguyễn Thái Ngọc Duy @ 2016-11-12 2:23 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <20161112022337.13317-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 | 78 ++++++++++++++++++++++++++++++++++
contrib/completion/git-completion.bash | 5 ++-
t/t2028-worktree-move.sh | 26 ++++++++++++
4 files changed, 120 insertions(+), 10 deletions(-)
diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 81f4fee..df0d551 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 11be345..60a6199 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
};
@@ -621,6 +622,81 @@ 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();
+ 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]);
+ if ((reason = is_worktree_locked(wt))) {
+ 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[] = {
@@ -645,5 +721,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 613e03b..f6855af 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
* [PATCH 09/11] worktree move: accept destination as directory
From: Nguyễn Thái Ngọc Duy @ 2016-11-12 2:23 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <20161112022337.13317-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 c0d4a73..307019c 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -539,7 +539,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();
@@ -556,6 +562,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);
+ }
+
/*
* First try. Atomically move, and probably cheaper, if both
* source and target are on the same file system.
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* [PATCH 08/11] worktree move: new command
From: Nguyễn Thái Ngọc Duy @ 2016-11-12 2:23 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <20161112022337.13317-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 | 61 ++++++++++++++++++++++++++++++++++
contrib/completion/git-completion.bash | 2 +-
t/t2028-worktree-move.sh | 30 +++++++++++++++++
4 files changed, 98 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 0aeb020..81f4fee 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 5c4854d..c0d4a73 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
@@ -522,6 +523,64 @@ 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();
+ 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]);
+ if ((reason = is_worktree_locked(wt))) {
+ if (*reason)
+ die(_("already locked, reason: %s"), reason);
+ die(_("already locked, no reason"));
+ }
+ if (validate_worktree(wt, 0))
+ return -1;
+
+ /*
+ * First try. Atomically move, and probably cheaper, if both
+ * source and target are on the same file system.
+ */
+ if (rename(wt->path, dst.buf) == -1) {
+ if (errno != EXDEV)
+ die_errno(_("failed to move '%s' to '%s'"),
+ wt->path, dst.buf);
+
+ /* second try.. */
+ if (copy_dir_recursively(wt->path, dst.buf))
+ die(_("failed to copy '%s' to '%s'"),
+ wt->path, dst.buf);
+ else {
+ struct strbuf sb = STRBUF_INIT;
+
+ strbuf_addstr(&sb, wt->path);
+ (void)remove_dir_recursively(&sb, 0);
+ strbuf_release(&sb);
+ }
+ }
+
+ return update_worktree_location(wt, dst.buf);
+}
+
int cmd_worktree(int ac, const char **av, const char *prefix)
{
struct option options[] = {
@@ -544,5 +603,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 21016bf..613e03b 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 10/11] worktree move: refuse to move worktrees with submodules
From: Nguyễn Thái Ngọc Duy @ 2016-11-12 2:23 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <20161112022337.13317-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 307019c..11be345 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -523,6 +523,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[] = {
@@ -562,6 +583,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 04/11] copy.c: style fix
From: Nguyễn Thái Ngọc Duy @ 2016-11-12 2:23 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <20161112022337.13317-1-pclouds@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
copy.c | 50 +++++++++++++++++++++++++++++---------------------
1 file changed, 29 insertions(+), 21 deletions(-)
diff --git a/copy.c b/copy.c
index 074b609..60c7d8a 100644
--- a/copy.c
+++ b/copy.c
@@ -111,8 +111,10 @@ int FAST_FUNC copy_file(const char *source, const char *dest, int flags)
if (dest_exists) {
if (!S_ISDIR(dest_stat.st_mode))
return error(_("target '%s' is not a directory"), dest);
- /* race here: user can substitute a symlink between
- * this check and actual creation of files inside dest */
+ /*
+ * race here: user can substitute a symlink between
+ * this check and actual creation of files inside dest
+ */
} else {
/* Create DEST */
mode_t mode;
@@ -130,22 +132,24 @@ int FAST_FUNC copy_file(const char *source, const char *dest, int flags)
if (lstat(dest, &dest_stat) < 0)
return error_errno(_("can't stat '%s'"), dest);
}
- /* remember (dev,inode) of each created dir.
- * NULL: name is not remembered */
+ /*
+ * remember (dev,inode) of each created dir. name is
+ * not remembered
+ */
add_to_ino_dev_hashtable(&dest_stat, NULL);
/* Recursively copy files in SOURCE */
dp = opendir(source);
- if (dp == NULL) {
+ if (!dp) {
retval = -1;
goto preserve_mode_ugid_time;
}
- while ((d = readdir(dp)) != NULL) {
+ while ((d = readdir(dp))) {
char *new_source, *new_dest;
new_source = concat_subpath_file(source, d->d_name);
- if (new_source == NULL)
+ if (!new_source)
continue;
new_dest = concat_path_file(dest, d->d_name);
if (copy_file(new_source, new_dest, flags & ~FILEUTILS_DEREFERENCE_L0) < 0)
@@ -155,16 +159,15 @@ int FAST_FUNC copy_file(const char *source, const char *dest, int flags)
}
closedir(dp);
- if (!dest_exists
- && chmod(dest, source_stat.st_mode & ~saved_umask) < 0
- ) {
+ if (!dest_exists &&
+ chmod(dest, source_stat.st_mode & ~saved_umask) < 0) {
error_errno(_("can't preserve permissions of '%s'"), dest);
/* retval = -1; - WRONG! copy *WAS* made */
}
goto preserve_mode_ugid_time;
}
- if (S_ISREG(source_stat.st_mode) ) { /* "cp [-opts] regular_file thing2" */
+ if (S_ISREG(source_stat.st_mode)) { /* "cp [-opts] regular_file thing2" */
int src_fd;
int dst_fd;
mode_t new_mode;
@@ -199,7 +202,7 @@ int FAST_FUNC copy_file(const char *source, const char *dest, int flags)
if (!S_ISREG(source_stat.st_mode))
new_mode = 0666;
- // POSIX way is a security problem versus (sym)link attacks
+ /* POSIX way is a security problem versus (sym)link attacks */
if (!ENABLE_FEATURE_NON_POSIX_CP) {
dst_fd = open(dest, O_WRONLY|O_CREAT|O_TRUNC, new_mode);
} else { /* safe way: */
@@ -226,13 +229,15 @@ int FAST_FUNC copy_file(const char *source, const char *dest, int flags)
retval = error_errno(_("error writing to '%s'"), dest);
/* ...but read size is already checked by bb_copyfd_eof */
close(src_fd);
- /* "cp /dev/something new_file" should not
- * copy mode of /dev/something */
+ /*
+ * "cp /dev/something new_file" should not
+ * copy mode of /dev/something
+ */
if (!S_ISREG(source_stat.st_mode))
return retval;
goto preserve_mode_ugid_time;
}
- dont_cat:
+dont_cat:
/* Source is a symlink or a special file */
/* We are lazy here, a bit lax with races... */
@@ -252,20 +257,23 @@ int FAST_FUNC copy_file(const char *source, const char *dest, int flags)
if (lchown(dest, source_stat.st_uid, source_stat.st_gid) < 0)
error_errno(_("can't preserve %s of '%s'"), "ownership", dest);
}
- /* _Not_ jumping to preserve_mode_ugid_time:
- * symlinks don't have those */
+ /*
+ * _Not_ jumping to preserve_mode_ugid_time: symlinks
+ * don't have those
+ */
return 0;
}
- if (S_ISBLK(source_stat.st_mode) || S_ISCHR(source_stat.st_mode)
- || S_ISSOCK(source_stat.st_mode) || S_ISFIFO(source_stat.st_mode)
- ) {
+ if (S_ISBLK(source_stat.st_mode) ||
+ S_ISCHR(source_stat.st_mode) ||
+ S_ISSOCK(source_stat.st_mode) ||
+ S_ISFIFO(source_stat.st_mode)) {
if (mknod(dest, source_stat.st_mode, source_stat.st_rdev) < 0)
return error_errno(_("can't create '%s'"), dest);
} else
return error(_("unrecognized file '%s' with mode %x"),
source, source_stat.st_mode);
- preserve_mode_ugid_time:
+preserve_mode_ugid_time:
if (1 /*FILEUTILS_PRESERVE_STATUS*/) {
struct timeval times[2];
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* [PATCH 05/11] copy.c: convert copy_file() to copy_dir_recursively()
From: Nguyễn Thái Ngọc Duy @ 2016-11-12 2:23 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <20161112022337.13317-1-pclouds@gmail.com>
This finally enables busybox's copy_file() code under a new name
(because "copy_file" is already taken in Git code base). Because this
comes from busybox, POSIXy (or even Linuxy) behavior is expected. More
changes may be needed for Windows support.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
cache.h | 1 +
copy.c | 216 ++++++++++++++++++++++++++++++++++++++++++++++++++++------------
2 files changed, 179 insertions(+), 38 deletions(-)
diff --git a/cache.h b/cache.h
index a50a61a..a9a72f8 100644
--- a/cache.h
+++ b/cache.h
@@ -1857,6 +1857,7 @@ extern void fprintf_or_die(FILE *, const char *fmt, ...);
extern int copy_fd(int ifd, int ofd);
extern int copy_file(const char *dst, const char *src, int mode);
extern int copy_file_with_time(const char *dst, const char *src, int mode);
+extern int copy_dir_recursively(const char *source, const char *dest);
extern void write_or_die(int fd, const void *buf, size_t count);
extern void fsync_or_die(int fd, const char *);
diff --git a/copy.c b/copy.c
index 60c7d8a..8471f7f 100644
--- a/copy.c
+++ b/copy.c
@@ -1,4 +1,6 @@
#include "cache.h"
+#include "dir.h"
+#include "hashmap.h"
int copy_fd(int ifd, int ofd)
{
@@ -66,21 +68,126 @@ int copy_file_with_time(const char *dst, const char *src, int mode)
return status;
}
-#if 0
-/* Return:
- * -1 error, copy not made
- * 0 copy is made or user answered "no" in interactive mode
- * (failures to preserve mode/owner/times are not reported in exit code)
+struct inode_key {
+ struct hashmap_entry entry;
+ ino_t ino;
+ dev_t dev;
+ /*
+ * Reportedly, on cramfs a file and a dir can have same ino.
+ * Need to also remember "file/dir" bit:
+ */
+ char isdir; /* bool */
+};
+
+struct inode_value {
+ struct inode_key key;
+ char name[FLEX_ARRAY];
+};
+
+#define HASH_SIZE 311u /* Should be prime */
+static inline unsigned hash_inode(ino_t i)
+{
+ return i % HASH_SIZE;
+}
+
+static int inode_cmp(const void *entry, const void *entry_or_key,
+ const void *keydata)
+{
+ const struct inode_value *inode = entry;
+ const struct inode_key *key = entry_or_key;
+
+ return !(inode->key.ino == key->ino &&
+ inode->key.dev == key->dev &&
+ inode->key.isdir == key->isdir);
+}
+
+static const char *is_in_ino_dev_hashtable(const struct hashmap *map,
+ const struct stat *st)
+{
+ struct inode_key key;
+ struct inode_value *value;
+
+ key.entry.hash = hash_inode(st->st_ino);
+ key.ino = st->st_ino;
+ key.dev = st->st_dev;
+ key.isdir = !!S_ISDIR(st->st_mode);
+ value = hashmap_get(map, &key, NULL);
+ return value ? value->name : NULL;
+}
+
+static void add_to_ino_dev_hashtable(struct hashmap *map,
+ const struct stat *st,
+ const char *path)
+{
+ struct inode_value *v;
+ int len = strlen(path);
+
+ v = xmalloc(offsetof(struct inode_value, name) + len + 1);
+ v->key.entry.hash = hash_inode(st->st_ino);
+ v->key.ino = st->st_ino;
+ v->key.dev = st->st_dev;
+ v->key.isdir = !!S_ISDIR(st->st_mode);
+ memcpy(v->name, path, len + 1);
+ hashmap_add(map, v);
+}
+
+/*
+ * Find out if the last character of a string matches the one given.
+ * Don't underrun the buffer if the string length is 0.
*/
-int FAST_FUNC copy_file(const char *source, const char *dest, int flags)
+static inline char *last_char_is(const char *s, int c)
+{
+ if (s && *s) {
+ size_t sz = strlen(s) - 1;
+ s += sz;
+ if ( (unsigned char)*s == c)
+ return (char*)s;
+ }
+ return NULL;
+}
+
+static inline char *concat_path_file(const char *path, const char *filename)
+{
+ struct strbuf sb = STRBUF_INIT;
+ char *lc;
+
+ if (!path)
+ path = "";
+ lc = last_char_is(path, '/');
+ while (*filename == '/')
+ filename++;
+ strbuf_addf(&sb, "%s%s%s", path, (lc==NULL ? "/" : ""), filename);
+ return strbuf_detach(&sb, NULL);
+}
+
+static char *concat_subpath_file(const char *path, const char *f)
+{
+ if (f && is_dot_or_dotdot(f))
+ return NULL;
+ return concat_path_file(path, f);
+}
+
+static int do_unlink(const char *dest)
+{
+ int e = errno;
+
+ if (unlink(dest) < 0) {
+ errno = e; /* do not use errno from unlink */
+ return error_errno(_("can't create '%s'"), dest);
+ }
+ return 0;
+}
+
+static int copy_dir_1(struct hashmap *inode_map,
+ const char *source,
+ const char *dest)
{
/* This is a recursive function, try to minimize stack usage */
- /* NB: each struct stat is ~100 bytes */
struct stat source_stat;
struct stat dest_stat;
- smallint retval = 0;
- smallint dest_exists = 0;
- smallint ovr;
+ int retval = 0;
+ int dest_exists = 0;
+ int ovr;
if (lstat(source, &source_stat) < 0)
return error_errno(_("can't stat '%s'"), source);
@@ -102,7 +209,7 @@ int FAST_FUNC copy_file(const char *source, const char *dest, int flags)
mode_t saved_umask = 0;
/* Did we ever create source ourself before? */
- tp = is_in_ino_dev_hashtable(&source_stat);
+ tp = is_in_ino_dev_hashtable(inode_map, &source_stat);
if (tp)
/* We did! it's a recursion! man the lifeboats... */
return error(_("recursion detected, omitting directory '%s'"),
@@ -132,11 +239,12 @@ int FAST_FUNC copy_file(const char *source, const char *dest, int flags)
if (lstat(dest, &dest_stat) < 0)
return error_errno(_("can't stat '%s'"), dest);
}
+
/*
* remember (dev,inode) of each created dir. name is
* not remembered
*/
- add_to_ino_dev_hashtable(&dest_stat, NULL);
+ add_to_ino_dev_hashtable(inode_map, &dest_stat, "");
/* Recursively copy files in SOURCE */
dp = opendir(source);
@@ -152,7 +260,7 @@ int FAST_FUNC copy_file(const char *source, const char *dest, int flags)
if (!new_source)
continue;
new_dest = concat_path_file(dest, d->d_name);
- if (copy_file(new_source, new_dest, flags & ~FILEUTILS_DEREFERENCE_L0) < 0)
+ if (copy_dir_1(inode_map, new_source, new_dest) < 0)
retval = -1;
free(new_source);
free(new_dest);
@@ -177,53 +285,57 @@ int FAST_FUNC copy_file(const char *source, const char *dest, int flags)
goto dont_cat;
}
- if (ENABLE_FEATURE_PRESERVE_HARDLINKS) {
+ if (1 /*ENABLE_FEATURE_PRESERVE_HARDLINKS*/) {
const char *link_target;
- link_target = is_in_ino_dev_hashtable(&source_stat);
+ link_target = is_in_ino_dev_hashtable(inode_map, &source_stat);
if (link_target) {
if (link(link_target, dest) < 0) {
- ovr = ask_and_unlink(dest, flags);
- if (ovr <= 0)
+ ovr = do_unlink(dest);
+ if (ovr < 0)
return ovr;
if (link(link_target, dest) < 0)
return error_errno(_("can't create link '%s'"), dest);
}
return 0;
}
- add_to_ino_dev_hashtable(&source_stat, dest);
+ add_to_ino_dev_hashtable(inode_map, &source_stat, dest);
}
- src_fd = open_or_warn(source, O_RDONLY);
+ src_fd = open(source, O_RDONLY);
if (src_fd < 0)
- return -1;
+ return error_errno(_("can't open '%s'"), source);
/* Do not try to open with weird mode fields */
new_mode = source_stat.st_mode;
if (!S_ISREG(source_stat.st_mode))
new_mode = 0666;
- /* POSIX way is a security problem versus (sym)link attacks */
- if (!ENABLE_FEATURE_NON_POSIX_CP) {
- dst_fd = open(dest, O_WRONLY|O_CREAT|O_TRUNC, new_mode);
- } else { /* safe way: */
- dst_fd = open(dest, O_WRONLY|O_CREAT|O_EXCL, new_mode);
- }
+ dst_fd = open(dest, O_WRONLY|O_CREAT|O_EXCL, new_mode);
if (dst_fd == -1) {
- ovr = ask_and_unlink(dest, flags);
- if (ovr <= 0) {
+ ovr = do_unlink(dest);
+ if (ovr < 0) {
close(src_fd);
return ovr;
}
/* It shouldn't exist. If it exists, do not open (symlink attack?) */
- dst_fd = open3_or_warn(dest, O_WRONLY|O_CREAT|O_EXCL, new_mode);
+ dst_fd = open(dest, O_WRONLY|O_CREAT|O_EXCL, new_mode);
if (dst_fd < 0) {
close(src_fd);
- return -1;
+ return error_errno(_("can't open '%s'"), dest);
}
}
- if (bb_copyfd_eof(src_fd, dst_fd) == -1)
+ switch (copy_fd(src_fd, dst_fd)) {
+ case COPY_READ_ERROR:
+ error(_("copy-fd: read returned %s"), strerror(errno));
retval = -1;
+ break;
+ case COPY_WRITE_ERROR:
+ error(_("copy-fd: write returned %s"), strerror(errno));
+ retval = -1;
+ break;
+ }
+
/* Careful with writing... */
if (close(dst_fd) < 0)
retval = error_errno(_("error writing to '%s'"), dest);
@@ -243,19 +355,28 @@ int FAST_FUNC copy_file(const char *source, const char *dest, int flags)
/* We are lazy here, a bit lax with races... */
if (dest_exists) {
errno = EEXIST;
- ovr = ask_and_unlink(dest, flags);
- if (ovr <= 0)
+ ovr = do_unlink(dest);
+ if (ovr < 0)
return ovr;
}
if (S_ISLNK(source_stat.st_mode)) {
- char *lpath = xmalloc_readlink_or_warn(source);
- if (lpath) {
- int r = symlink(lpath, dest);
- free(lpath);
+ struct strbuf lpath = STRBUF_INIT;
+ if (!strbuf_readlink(&lpath, source, 0)) {
+ int r = symlink(lpath.buf, dest);
+ strbuf_release(&lpath);
if (r < 0)
return error_errno(_("can't create symlink '%s'"), dest);
if (lchown(dest, source_stat.st_uid, source_stat.st_gid) < 0)
error_errno(_("can't preserve %s of '%s'"), "ownership", dest);
+ } else {
+ /* EINVAL => "file: Invalid argument" => puzzled user */
+ const char *errmsg = _("not a symlink");
+ int err = errno;
+
+ if (err != EINVAL)
+ errmsg = strerror(err);
+ error(_("%s: cannot read link: %s"), source, errmsg);
+ strbuf_release(&lpath);
}
/*
* _Not_ jumping to preserve_mode_ugid_time: symlinks
@@ -293,4 +414,23 @@ int FAST_FUNC copy_file(const char *source, const char *dest, int flags)
return retval;
}
-#endif
+
+/*
+ * Return:
+ * -1 error, copy not made
+ * 0 copy is made
+ *
+ * Failures to preserve mode/owner/times are not reported in exit
+ * code. No support for preserving SELinux security context. Symlinks
+ * and hardlinks are preserved.
+ */
+int copy_dir_recursively(const char *source, const char *dest)
+{
+ int ret;
+ struct hashmap inode_map;
+
+ hashmap_init(&inode_map, inode_cmp, 1024);
+ ret = copy_dir_1(&inode_map, source, dest);
+ hashmap_free(&inode_map, 1);
+ return ret;
+}
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* [PATCH 01/11] copy.c: import copy_file() from busybox
From: Nguyễn Thái Ngọc Duy @ 2016-11-12 2:23 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <20161112022337.13317-1-pclouds@gmail.com>
This is busybox's unmodified copy_file() in libbb/copy_file.c from the
GPL2+ commit f2c043acfcf9dad9fd3d65821b81f89986bbe54e (busybox: fix
uninitialized memory when displaying IPv6 addresses -
2016-01-18). This is a no-op commit. More changes are needed before
this new code can compile.
This will be needed for implementing "git worktree move" where we have
to move a directory recursively. We can implement it from scratch, but
then we will have to deal with corner cases (failure to move, circular
symlinks...). And delegating the task to "/bin/mv" takes a way the
ability to clean things up properly when things fail and we may have
to deal with "mv" differences between platforms.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
copy.c | 331 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 331 insertions(+)
diff --git a/copy.c b/copy.c
index 4de6a11..79623ac 100644
--- a/copy.c
+++ b/copy.c
@@ -65,3 +65,334 @@ int copy_file_with_time(const char *dst, const char *src, int mode)
return copy_times(dst, src);
return status;
}
+
+#if 0
+/* Return:
+ * -1 error, copy not made
+ * 0 copy is made or user answered "no" in interactive mode
+ * (failures to preserve mode/owner/times are not reported in exit code)
+ */
+int FAST_FUNC copy_file(const char *source, const char *dest, int flags)
+{
+ /* This is a recursive function, try to minimize stack usage */
+ /* NB: each struct stat is ~100 bytes */
+ struct stat source_stat;
+ struct stat dest_stat;
+ smallint retval = 0;
+ smallint dest_exists = 0;
+ smallint ovr;
+
+/* Inverse of cp -d ("cp without -d") */
+#define FLAGS_DEREF (flags & (FILEUTILS_DEREFERENCE + FILEUTILS_DEREFERENCE_L0))
+
+ if ((FLAGS_DEREF ? stat : lstat)(source, &source_stat) < 0) {
+ /* This may be a dangling symlink.
+ * Making [sym]links to dangling symlinks works, so... */
+ if (flags & (FILEUTILS_MAKE_SOFTLINK|FILEUTILS_MAKE_HARDLINK))
+ goto make_links;
+ bb_perror_msg("can't stat '%s'", source);
+ return -1;
+ }
+
+ if (lstat(dest, &dest_stat) < 0) {
+ if (errno != ENOENT) {
+ bb_perror_msg("can't stat '%s'", dest);
+ return -1;
+ }
+ } else {
+ if (source_stat.st_dev == dest_stat.st_dev
+ && source_stat.st_ino == dest_stat.st_ino
+ ) {
+ bb_error_msg("'%s' and '%s' are the same file", source, dest);
+ return -1;
+ }
+ dest_exists = 1;
+ }
+
+#if ENABLE_SELINUX
+ if ((flags & FILEUTILS_PRESERVE_SECURITY_CONTEXT) && is_selinux_enabled() > 0) {
+ security_context_t con;
+ if (lgetfilecon(source, &con) >= 0) {
+ if (setfscreatecon(con) < 0) {
+ bb_perror_msg("can't set setfscreatecon %s", con);
+ freecon(con);
+ return -1;
+ }
+ } else if (errno == ENOTSUP || errno == ENODATA) {
+ setfscreatecon_or_die(NULL);
+ } else {
+ bb_perror_msg("can't lgetfilecon %s", source);
+ return -1;
+ }
+ }
+#endif
+
+ if (S_ISDIR(source_stat.st_mode)) {
+ DIR *dp;
+ const char *tp;
+ struct dirent *d;
+ mode_t saved_umask = 0;
+
+ if (!(flags & FILEUTILS_RECUR)) {
+ bb_error_msg("omitting directory '%s'", source);
+ return -1;
+ }
+
+ /* Did we ever create source ourself before? */
+ tp = is_in_ino_dev_hashtable(&source_stat);
+ if (tp) {
+ /* We did! it's a recursion! man the lifeboats... */
+ bb_error_msg("recursion detected, omitting directory '%s'",
+ source);
+ return -1;
+ }
+
+ if (dest_exists) {
+ if (!S_ISDIR(dest_stat.st_mode)) {
+ bb_error_msg("target '%s' is not a directory", dest);
+ return -1;
+ }
+ /* race here: user can substitute a symlink between
+ * this check and actual creation of files inside dest */
+ } else {
+ /* Create DEST */
+ mode_t mode;
+ saved_umask = umask(0);
+
+ mode = source_stat.st_mode;
+ if (!(flags & FILEUTILS_PRESERVE_STATUS))
+ mode = source_stat.st_mode & ~saved_umask;
+ /* Allow owner to access new dir (at least for now) */
+ mode |= S_IRWXU;
+ if (mkdir(dest, mode) < 0) {
+ umask(saved_umask);
+ bb_perror_msg("can't create directory '%s'", dest);
+ return -1;
+ }
+ umask(saved_umask);
+ /* need stat info for add_to_ino_dev_hashtable */
+ if (lstat(dest, &dest_stat) < 0) {
+ bb_perror_msg("can't stat '%s'", dest);
+ return -1;
+ }
+ }
+ /* remember (dev,inode) of each created dir.
+ * NULL: name is not remembered */
+ add_to_ino_dev_hashtable(&dest_stat, NULL);
+
+ /* Recursively copy files in SOURCE */
+ dp = opendir(source);
+ if (dp == NULL) {
+ retval = -1;
+ goto preserve_mode_ugid_time;
+ }
+
+ while ((d = readdir(dp)) != NULL) {
+ char *new_source, *new_dest;
+
+ new_source = concat_subpath_file(source, d->d_name);
+ if (new_source == NULL)
+ continue;
+ new_dest = concat_path_file(dest, d->d_name);
+ if (copy_file(new_source, new_dest, flags & ~FILEUTILS_DEREFERENCE_L0) < 0)
+ retval = -1;
+ free(new_source);
+ free(new_dest);
+ }
+ closedir(dp);
+
+ if (!dest_exists
+ && chmod(dest, source_stat.st_mode & ~saved_umask) < 0
+ ) {
+ bb_perror_msg("can't preserve %s of '%s'", "permissions", dest);
+ /* retval = -1; - WRONG! copy *WAS* made */
+ }
+ goto preserve_mode_ugid_time;
+ }
+
+ if (flags & (FILEUTILS_MAKE_SOFTLINK|FILEUTILS_MAKE_HARDLINK)) {
+ int (*lf)(const char *oldpath, const char *newpath);
+ make_links:
+ /* Hmm... maybe
+ * if (DEREF && MAKE_SOFTLINK) source = realpath(source) ?
+ * (but realpath returns NULL on dangling symlinks...) */
+ lf = (flags & FILEUTILS_MAKE_SOFTLINK) ? symlink : link;
+ if (lf(source, dest) < 0) {
+ ovr = ask_and_unlink(dest, flags);
+ if (ovr <= 0)
+ return ovr;
+ if (lf(source, dest) < 0) {
+ bb_perror_msg("can't create link '%s'", dest);
+ return -1;
+ }
+ }
+ /* _Not_ jumping to preserve_mode_ugid_time:
+ * (sym)links don't have those */
+ return 0;
+ }
+
+ if (/* "cp thing1 thing2" without -R: just open and read() from thing1 */
+ !(flags & FILEUTILS_RECUR)
+ /* "cp [-opts] regular_file thing2" */
+ || S_ISREG(source_stat.st_mode)
+ /* DEREF uses stat, which never returns S_ISLNK() == true.
+ * So the below is never true: */
+ /* || (FLAGS_DEREF && S_ISLNK(source_stat.st_mode)) */
+ ) {
+ int src_fd;
+ int dst_fd;
+ mode_t new_mode;
+
+ if (!FLAGS_DEREF && S_ISLNK(source_stat.st_mode)) {
+ /* "cp -d symlink dst": create a link */
+ goto dont_cat;
+ }
+
+ if (ENABLE_FEATURE_PRESERVE_HARDLINKS && !FLAGS_DEREF) {
+ const char *link_target;
+ link_target = is_in_ino_dev_hashtable(&source_stat);
+ if (link_target) {
+ if (link(link_target, dest) < 0) {
+ ovr = ask_and_unlink(dest, flags);
+ if (ovr <= 0)
+ return ovr;
+ if (link(link_target, dest) < 0) {
+ bb_perror_msg("can't create link '%s'", dest);
+ return -1;
+ }
+ }
+ return 0;
+ }
+ add_to_ino_dev_hashtable(&source_stat, dest);
+ }
+
+ src_fd = open_or_warn(source, O_RDONLY);
+ if (src_fd < 0)
+ return -1;
+
+ /* Do not try to open with weird mode fields */
+ new_mode = source_stat.st_mode;
+ if (!S_ISREG(source_stat.st_mode))
+ new_mode = 0666;
+
+ // POSIX way is a security problem versus (sym)link attacks
+ if (!ENABLE_FEATURE_NON_POSIX_CP) {
+ dst_fd = open(dest, O_WRONLY|O_CREAT|O_TRUNC, new_mode);
+ } else { /* safe way: */
+ dst_fd = open(dest, O_WRONLY|O_CREAT|O_EXCL, new_mode);
+ }
+ if (dst_fd == -1) {
+ ovr = ask_and_unlink(dest, flags);
+ if (ovr <= 0) {
+ close(src_fd);
+ return ovr;
+ }
+ /* It shouldn't exist. If it exists, do not open (symlink attack?) */
+ dst_fd = open3_or_warn(dest, O_WRONLY|O_CREAT|O_EXCL, new_mode);
+ if (dst_fd < 0) {
+ close(src_fd);
+ return -1;
+ }
+ }
+
+#if ENABLE_SELINUX
+ if ((flags & (FILEUTILS_PRESERVE_SECURITY_CONTEXT|FILEUTILS_SET_SECURITY_CONTEXT))
+ && is_selinux_enabled() > 0
+ ) {
+ security_context_t con;
+ if (getfscreatecon(&con) == -1) {
+ bb_perror_msg("getfscreatecon");
+ return -1;
+ }
+ if (con) {
+ if (setfilecon(dest, con) == -1) {
+ bb_perror_msg("setfilecon:%s,%s", dest, con);
+ freecon(con);
+ return -1;
+ }
+ freecon(con);
+ }
+ }
+#endif
+ if (bb_copyfd_eof(src_fd, dst_fd) == -1)
+ retval = -1;
+ /* Careful with writing... */
+ if (close(dst_fd) < 0) {
+ bb_perror_msg("error writing to '%s'", dest);
+ retval = -1;
+ }
+ /* ...but read size is already checked by bb_copyfd_eof */
+ close(src_fd);
+ /* "cp /dev/something new_file" should not
+ * copy mode of /dev/something */
+ if (!S_ISREG(source_stat.st_mode))
+ return retval;
+ goto preserve_mode_ugid_time;
+ }
+ dont_cat:
+
+ /* Source is a symlink or a special file */
+ /* We are lazy here, a bit lax with races... */
+ if (dest_exists) {
+ errno = EEXIST;
+ ovr = ask_and_unlink(dest, flags);
+ if (ovr <= 0)
+ return ovr;
+ }
+ if (S_ISLNK(source_stat.st_mode)) {
+ char *lpath = xmalloc_readlink_or_warn(source);
+ if (lpath) {
+ int r = symlink(lpath, dest);
+ free(lpath);
+ if (r < 0) {
+ bb_perror_msg("can't create symlink '%s'", dest);
+ return -1;
+ }
+ if (flags & FILEUTILS_PRESERVE_STATUS)
+ if (lchown(dest, source_stat.st_uid, source_stat.st_gid) < 0)
+ bb_perror_msg("can't preserve %s of '%s'", "ownership", dest);
+ }
+ /* _Not_ jumping to preserve_mode_ugid_time:
+ * symlinks don't have those */
+ return 0;
+ }
+ if (S_ISBLK(source_stat.st_mode) || S_ISCHR(source_stat.st_mode)
+ || S_ISSOCK(source_stat.st_mode) || S_ISFIFO(source_stat.st_mode)
+ ) {
+ if (mknod(dest, source_stat.st_mode, source_stat.st_rdev) < 0) {
+ bb_perror_msg("can't create '%s'", dest);
+ return -1;
+ }
+ } else {
+ bb_error_msg("unrecognized file '%s' with mode %x", source, source_stat.st_mode);
+ return -1;
+ }
+
+ preserve_mode_ugid_time:
+
+ if (flags & FILEUTILS_PRESERVE_STATUS
+ /* Cannot happen: */
+ /* && !(flags & (FILEUTILS_MAKE_SOFTLINK|FILEUTILS_MAKE_HARDLINK)) */
+ ) {
+ struct timeval times[2];
+
+ times[1].tv_sec = times[0].tv_sec = source_stat.st_mtime;
+ times[1].tv_usec = times[0].tv_usec = 0;
+ /* BTW, utimes sets usec-precision time - just FYI */
+ if (utimes(dest, times) < 0)
+ bb_perror_msg("can't preserve %s of '%s'", "times", dest);
+ if (chown(dest, source_stat.st_uid, source_stat.st_gid) < 0) {
+ source_stat.st_mode &= ~(S_ISUID | S_ISGID);
+ bb_perror_msg("can't preserve %s of '%s'", "ownership", dest);
+ }
+ if (chmod(dest, source_stat.st_mode) < 0)
+ bb_perror_msg("can't preserve %s of '%s'", "permissions", dest);
+ }
+
+ if (flags & FILEUTILS_VERBOSE) {
+ printf("'%s' -> '%s'\n", source, dest);
+ }
+
+ return retval;
+}
+#endif
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* [PATCH 07/11] worktree.c: add update_worktree_location()
From: Nguyễn Thái Ngọc Duy @ 2016-11-12 2:23 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <20161112022337.13317-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 7e15ec7..db63758 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 e782ae5..0477a3d 100644
--- a/worktree.h
+++ b/worktree.h
@@ -55,6 +55,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 03/11] copy.c: convert bb_(p)error_msg to error(_errno)
From: Nguyễn Thái Ngọc Duy @ 2016-11-12 2:23 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <20161112022337.13317-1-pclouds@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
copy.c | 85 ++++++++++++++++++++++++------------------------------------------
1 file changed, 31 insertions(+), 54 deletions(-)
diff --git a/copy.c b/copy.c
index b7a87f1..074b609 100644
--- a/copy.c
+++ b/copy.c
@@ -82,23 +82,16 @@ int FAST_FUNC copy_file(const char *source, const char *dest, int flags)
smallint dest_exists = 0;
smallint ovr;
- if (lstat(source, &source_stat) < 0) {
- bb_perror_msg("can't stat '%s'", source);
- return -1;
- }
+ if (lstat(source, &source_stat) < 0)
+ return error_errno(_("can't stat '%s'"), source);
if (lstat(dest, &dest_stat) < 0) {
- if (errno != ENOENT) {
- bb_perror_msg("can't stat '%s'", dest);
- return -1;
- }
+ if (errno != ENOENT)
+ return error_errno(_("can't stat '%s'"), dest);
} else {
- if (source_stat.st_dev == dest_stat.st_dev
- && source_stat.st_ino == dest_stat.st_ino
- ) {
- bb_error_msg("'%s' and '%s' are the same file", source, dest);
- return -1;
- }
+ if (source_stat.st_dev == dest_stat.st_dev &&
+ source_stat.st_ino == dest_stat.st_ino)
+ return error(_("'%s' and '%s' are the same file"), source, dest);
dest_exists = 1;
}
@@ -110,18 +103,14 @@ int FAST_FUNC copy_file(const char *source, const char *dest, int flags)
/* Did we ever create source ourself before? */
tp = is_in_ino_dev_hashtable(&source_stat);
- if (tp) {
+ if (tp)
/* We did! it's a recursion! man the lifeboats... */
- bb_error_msg("recursion detected, omitting directory '%s'",
- source);
- return -1;
- }
+ return error(_("recursion detected, omitting directory '%s'"),
+ source);
if (dest_exists) {
- if (!S_ISDIR(dest_stat.st_mode)) {
- bb_error_msg("target '%s' is not a directory", dest);
- return -1;
- }
+ if (!S_ISDIR(dest_stat.st_mode))
+ return error(_("target '%s' is not a directory"), dest);
/* race here: user can substitute a symlink between
* this check and actual creation of files inside dest */
} else {
@@ -134,15 +123,12 @@ int FAST_FUNC copy_file(const char *source, const char *dest, int flags)
mode |= S_IRWXU;
if (mkdir(dest, mode) < 0) {
umask(saved_umask);
- bb_perror_msg("can't create directory '%s'", dest);
- return -1;
+ return error_errno(_("can't create directory '%s'"), dest);
}
umask(saved_umask);
/* need stat info for add_to_ino_dev_hashtable */
- if (lstat(dest, &dest_stat) < 0) {
- bb_perror_msg("can't stat '%s'", dest);
- return -1;
- }
+ if (lstat(dest, &dest_stat) < 0)
+ return error_errno(_("can't stat '%s'"), dest);
}
/* remember (dev,inode) of each created dir.
* NULL: name is not remembered */
@@ -172,7 +158,7 @@ int FAST_FUNC copy_file(const char *source, const char *dest, int flags)
if (!dest_exists
&& chmod(dest, source_stat.st_mode & ~saved_umask) < 0
) {
- bb_perror_msg("can't preserve %s of '%s'", "permissions", dest);
+ error_errno(_("can't preserve permissions of '%s'"), dest);
/* retval = -1; - WRONG! copy *WAS* made */
}
goto preserve_mode_ugid_time;
@@ -196,10 +182,8 @@ int FAST_FUNC copy_file(const char *source, const char *dest, int flags)
ovr = ask_and_unlink(dest, flags);
if (ovr <= 0)
return ovr;
- if (link(link_target, dest) < 0) {
- bb_perror_msg("can't create link '%s'", dest);
- return -1;
- }
+ if (link(link_target, dest) < 0)
+ return error_errno(_("can't create link '%s'"), dest);
}
return 0;
}
@@ -238,10 +222,8 @@ int FAST_FUNC copy_file(const char *source, const char *dest, int flags)
if (bb_copyfd_eof(src_fd, dst_fd) == -1)
retval = -1;
/* Careful with writing... */
- if (close(dst_fd) < 0) {
- bb_perror_msg("error writing to '%s'", dest);
- retval = -1;
- }
+ if (close(dst_fd) < 0)
+ retval = error_errno(_("error writing to '%s'"), dest);
/* ...but read size is already checked by bb_copyfd_eof */
close(src_fd);
/* "cp /dev/something new_file" should not
@@ -265,12 +247,10 @@ int FAST_FUNC copy_file(const char *source, const char *dest, int flags)
if (lpath) {
int r = symlink(lpath, dest);
free(lpath);
- if (r < 0) {
- bb_perror_msg("can't create symlink '%s'", dest);
- return -1;
- }
+ if (r < 0)
+ return error_errno(_("can't create symlink '%s'"), dest);
if (lchown(dest, source_stat.st_uid, source_stat.st_gid) < 0)
- bb_perror_msg("can't preserve %s of '%s'", "ownership", dest);
+ error_errno(_("can't preserve %s of '%s'"), "ownership", dest);
}
/* _Not_ jumping to preserve_mode_ugid_time:
* symlinks don't have those */
@@ -279,14 +259,11 @@ int FAST_FUNC copy_file(const char *source, const char *dest, int flags)
if (S_ISBLK(source_stat.st_mode) || S_ISCHR(source_stat.st_mode)
|| S_ISSOCK(source_stat.st_mode) || S_ISFIFO(source_stat.st_mode)
) {
- if (mknod(dest, source_stat.st_mode, source_stat.st_rdev) < 0) {
- bb_perror_msg("can't create '%s'", dest);
- return -1;
- }
- } else {
- bb_error_msg("unrecognized file '%s' with mode %x", source, source_stat.st_mode);
- return -1;
- }
+ if (mknod(dest, source_stat.st_mode, source_stat.st_rdev) < 0)
+ return error_errno(_("can't create '%s'"), dest);
+ } else
+ return error(_("unrecognized file '%s' with mode %x"),
+ source, source_stat.st_mode);
preserve_mode_ugid_time:
@@ -297,13 +274,13 @@ int FAST_FUNC copy_file(const char *source, const char *dest, int flags)
times[1].tv_usec = times[0].tv_usec = 0;
/* BTW, utimes sets usec-precision time - just FYI */
if (utimes(dest, times) < 0)
- bb_perror_msg("can't preserve %s of '%s'", "times", dest);
+ error_errno(_("can't preserve %s of '%s'"), "times", dest);
if (chown(dest, source_stat.st_uid, source_stat.st_gid) < 0) {
source_stat.st_mode &= ~(S_ISUID | S_ISGID);
- bb_perror_msg("can't preserve %s of '%s'", "ownership", dest);
+ error_errno(_("can't preserve %s of '%s'"), "ownership", dest);
}
if (chmod(dest, source_stat.st_mode) < 0)
- bb_perror_msg("can't preserve %s of '%s'", "permissions", dest);
+ error_errno(_("can't preserve %s of '%s'"), "permissions", dest);
}
return retval;
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* [PATCH 02/11] copy.c: delete unused code in copy_file()
From: Nguyễn Thái Ngọc Duy @ 2016-11-12 2:23 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <20161112022337.13317-1-pclouds@gmail.com>
- selinux preservation code
- make-link code
- delete link dereference code
- non-recursive copy code
- stat no preservation code
- verbose printing code
Some of these are "cp" features that we don't need (for "git worktree
move"). Some do not make sense in source-control context (SELinux).
Some probably need a reimplementation to better fit in (verbose
printing code).
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
copy.c | 101 +++++------------------------------------------------------------
1 file changed, 7 insertions(+), 94 deletions(-)
diff --git a/copy.c b/copy.c
index 79623ac..b7a87f1 100644
--- a/copy.c
+++ b/copy.c
@@ -82,14 +82,7 @@ int FAST_FUNC copy_file(const char *source, const char *dest, int flags)
smallint dest_exists = 0;
smallint ovr;
-/* Inverse of cp -d ("cp without -d") */
-#define FLAGS_DEREF (flags & (FILEUTILS_DEREFERENCE + FILEUTILS_DEREFERENCE_L0))
-
- if ((FLAGS_DEREF ? stat : lstat)(source, &source_stat) < 0) {
- /* This may be a dangling symlink.
- * Making [sym]links to dangling symlinks works, so... */
- if (flags & (FILEUTILS_MAKE_SOFTLINK|FILEUTILS_MAKE_HARDLINK))
- goto make_links;
+ if (lstat(source, &source_stat) < 0) {
bb_perror_msg("can't stat '%s'", source);
return -1;
}
@@ -109,35 +102,12 @@ int FAST_FUNC copy_file(const char *source, const char *dest, int flags)
dest_exists = 1;
}
-#if ENABLE_SELINUX
- if ((flags & FILEUTILS_PRESERVE_SECURITY_CONTEXT) && is_selinux_enabled() > 0) {
- security_context_t con;
- if (lgetfilecon(source, &con) >= 0) {
- if (setfscreatecon(con) < 0) {
- bb_perror_msg("can't set setfscreatecon %s", con);
- freecon(con);
- return -1;
- }
- } else if (errno == ENOTSUP || errno == ENODATA) {
- setfscreatecon_or_die(NULL);
- } else {
- bb_perror_msg("can't lgetfilecon %s", source);
- return -1;
- }
- }
-#endif
-
if (S_ISDIR(source_stat.st_mode)) {
DIR *dp;
const char *tp;
struct dirent *d;
mode_t saved_umask = 0;
- if (!(flags & FILEUTILS_RECUR)) {
- bb_error_msg("omitting directory '%s'", source);
- return -1;
- }
-
/* Did we ever create source ourself before? */
tp = is_in_ino_dev_hashtable(&source_stat);
if (tp) {
@@ -160,8 +130,6 @@ int FAST_FUNC copy_file(const char *source, const char *dest, int flags)
saved_umask = umask(0);
mode = source_stat.st_mode;
- if (!(flags & FILEUTILS_PRESERVE_STATUS))
- mode = source_stat.st_mode & ~saved_umask;
/* Allow owner to access new dir (at least for now) */
mode |= S_IRWXU;
if (mkdir(dest, mode) < 0) {
@@ -210,45 +178,17 @@ int FAST_FUNC copy_file(const char *source, const char *dest, int flags)
goto preserve_mode_ugid_time;
}
- if (flags & (FILEUTILS_MAKE_SOFTLINK|FILEUTILS_MAKE_HARDLINK)) {
- int (*lf)(const char *oldpath, const char *newpath);
- make_links:
- /* Hmm... maybe
- * if (DEREF && MAKE_SOFTLINK) source = realpath(source) ?
- * (but realpath returns NULL on dangling symlinks...) */
- lf = (flags & FILEUTILS_MAKE_SOFTLINK) ? symlink : link;
- if (lf(source, dest) < 0) {
- ovr = ask_and_unlink(dest, flags);
- if (ovr <= 0)
- return ovr;
- if (lf(source, dest) < 0) {
- bb_perror_msg("can't create link '%s'", dest);
- return -1;
- }
- }
- /* _Not_ jumping to preserve_mode_ugid_time:
- * (sym)links don't have those */
- return 0;
- }
-
- if (/* "cp thing1 thing2" without -R: just open and read() from thing1 */
- !(flags & FILEUTILS_RECUR)
- /* "cp [-opts] regular_file thing2" */
- || S_ISREG(source_stat.st_mode)
- /* DEREF uses stat, which never returns S_ISLNK() == true.
- * So the below is never true: */
- /* || (FLAGS_DEREF && S_ISLNK(source_stat.st_mode)) */
- ) {
+ if (S_ISREG(source_stat.st_mode) ) { /* "cp [-opts] regular_file thing2" */
int src_fd;
int dst_fd;
mode_t new_mode;
- if (!FLAGS_DEREF && S_ISLNK(source_stat.st_mode)) {
+ if (S_ISLNK(source_stat.st_mode)) {
/* "cp -d symlink dst": create a link */
goto dont_cat;
}
- if (ENABLE_FEATURE_PRESERVE_HARDLINKS && !FLAGS_DEREF) {
+ if (ENABLE_FEATURE_PRESERVE_HARDLINKS) {
const char *link_target;
link_target = is_in_ino_dev_hashtable(&source_stat);
if (link_target) {
@@ -295,25 +235,6 @@ int FAST_FUNC copy_file(const char *source, const char *dest, int flags)
}
}
-#if ENABLE_SELINUX
- if ((flags & (FILEUTILS_PRESERVE_SECURITY_CONTEXT|FILEUTILS_SET_SECURITY_CONTEXT))
- && is_selinux_enabled() > 0
- ) {
- security_context_t con;
- if (getfscreatecon(&con) == -1) {
- bb_perror_msg("getfscreatecon");
- return -1;
- }
- if (con) {
- if (setfilecon(dest, con) == -1) {
- bb_perror_msg("setfilecon:%s,%s", dest, con);
- freecon(con);
- return -1;
- }
- freecon(con);
- }
- }
-#endif
if (bb_copyfd_eof(src_fd, dst_fd) == -1)
retval = -1;
/* Careful with writing... */
@@ -348,9 +269,8 @@ int FAST_FUNC copy_file(const char *source, const char *dest, int flags)
bb_perror_msg("can't create symlink '%s'", dest);
return -1;
}
- if (flags & FILEUTILS_PRESERVE_STATUS)
- if (lchown(dest, source_stat.st_uid, source_stat.st_gid) < 0)
- bb_perror_msg("can't preserve %s of '%s'", "ownership", dest);
+ if (lchown(dest, source_stat.st_uid, source_stat.st_gid) < 0)
+ bb_perror_msg("can't preserve %s of '%s'", "ownership", dest);
}
/* _Not_ jumping to preserve_mode_ugid_time:
* symlinks don't have those */
@@ -370,10 +290,7 @@ int FAST_FUNC copy_file(const char *source, const char *dest, int flags)
preserve_mode_ugid_time:
- if (flags & FILEUTILS_PRESERVE_STATUS
- /* Cannot happen: */
- /* && !(flags & (FILEUTILS_MAKE_SOFTLINK|FILEUTILS_MAKE_HARDLINK)) */
- ) {
+ if (1 /*FILEUTILS_PRESERVE_STATUS*/) {
struct timeval times[2];
times[1].tv_sec = times[0].tv_sec = source_stat.st_mtime;
@@ -389,10 +306,6 @@ int FAST_FUNC copy_file(const char *source, const char *dest, int flags)
bb_perror_msg("can't preserve %s of '%s'", "permissions", dest);
}
- if (flags & FILEUTILS_VERBOSE) {
- printf("'%s' -> '%s'\n", source, dest);
- }
-
return retval;
}
#endif
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* [PATCH 06/11] worktree.c: add validate_worktree()
From: Nguyễn Thái Ngọc Duy @ 2016-11-12 2:23 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <20161112022337.13317-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 f7869f8..7e15ec7 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 90e1311..e782ae5 100644
--- a/worktree.h
+++ b/worktree.h
@@ -50,6 +50,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 00/11] git worktree (re)move
From: Nguyễn Thái Ngọc Duy @ 2016-11-12 2:23 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
This is mostly a resend from last time [1]. The main difference is
patch 10/11 to prevent moving a worktree that contains submodules
because these .git files may need rewritten to point to the right
place. I'm leaving the rewriting .git files for future (preferably
done by "submodules guys").
[1] https://public-inbox.org/git/20160625075433.4608-1-pclouds@gmail.com/
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 | 179 ++++++++++++++++
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, 722 insertions(+), 11 deletions(-)
--
2.8.2.524.g6ff3d78
^ permalink raw reply
* [PATCH v3] rebase: add --forget to cleanup rebase, leave everything else untouched
From: Nguyễn Thái Ngọc Duy @ 2016-11-12 2:00 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <20161109091131.17933-1-pclouds@gmail.com>
There are occasions when you decide to abort an in-progress rebase and
move on to do something else but you forget to do "git rebase --abort"
first. Or the rebase has been in progress for so long you forgot about
it. By the time you realize that (e.g. by starting another rebase)
it's already too late to retrace your steps. The solution is normally
rm -r .git/<some rebase dir>
and continue with your life. But there could be two different
directories for <some rebase dir> (and it obviously requires some
knowledge of how rebase works), and the ".git" part could be much
longer if you are not at top-dir, or in a linked worktree. And
"rm -r" is very dangerous to do in .git, a mistake in there could
destroy object database or other important data.
Provide "git rebase --forget" for this exact use case.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
v3 rewrote the desscription of --forget in git-rebase.txt to be more
or less along the same line as --abort. What happens to the index and
worktree is also stated to avoid misunderstanding.
Documentation/git-rebase.txt | 7 ++++++-
contrib/completion/git-completion.bash | 4 ++--
git-rebase.sh | 6 +++++-
t/t3407-rebase-abort.sh | 24 ++++++++++++++++++++++++
4 files changed, 37 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index de222c8..1e16c70 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -12,7 +12,7 @@ SYNOPSIS
[<upstream> [<branch>]]
'git rebase' [-i | --interactive] [options] [--exec <cmd>] [--onto <newbase>]
--root [<branch>]
-'git rebase' --continue | --skip | --abort | --edit-todo
+'git rebase' --continue | --skip | --abort | --forget | --edit-todo
DESCRIPTION
-----------
@@ -252,6 +252,11 @@ leave out at most one of A and B, in which case it defaults to HEAD.
will be reset to where it was when the rebase operation was
started.
+--forget::
+ Abort the rebase operation but HEAD is not reset back to the
+ original branch. The index and working tree are also left
+ unchanged as a result.
+
--keep-empty::
Keep the commits that do not change anything from its
parents in the result.
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 21016bf..3143cb0 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1734,10 +1734,10 @@ _git_rebase ()
{
local dir="$(__gitdir)"
if [ -f "$dir"/rebase-merge/interactive ]; then
- __gitcomp "--continue --skip --abort --edit-todo"
+ __gitcomp "--continue --skip --abort --forget --edit-todo"
return
elif [ -d "$dir"/rebase-apply ] || [ -d "$dir"/rebase-merge ]; then
- __gitcomp "--continue --skip --abort"
+ __gitcomp "--continue --skip --abort --forget"
return
fi
__git_complete_strategy && return
diff --git a/git-rebase.sh b/git-rebase.sh
index 04f6e44..de712b7 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -43,6 +43,7 @@ continue! continue
abort! abort and check out the original branch
skip! skip current patch and continue
edit-todo! edit the todo list during an interactive rebase
+forget! abort but keep HEAD where it is
"
. git-sh-setup
set_reflog_action rebase
@@ -241,7 +242,7 @@ do
--verify)
ok_to_skip_pre_rebase=
;;
- --continue|--skip|--abort|--edit-todo)
+ --continue|--skip|--abort|--forget|--edit-todo)
test $total_argc -eq 2 || usage
action=${1##--}
;;
@@ -399,6 +400,9 @@ abort)
finish_rebase
exit
;;
+forget)
+ exec rm -rf "$state_dir"
+ ;;
edit-todo)
run_specific_rebase
;;
diff --git a/t/t3407-rebase-abort.sh b/t/t3407-rebase-abort.sh
index a6a6c40..6bc5e71 100755
--- a/t/t3407-rebase-abort.sh
+++ b/t/t3407-rebase-abort.sh
@@ -99,4 +99,28 @@ testrebase() {
testrebase "" .git/rebase-apply
testrebase " --merge" .git/rebase-merge
+test_expect_success 'rebase --forget' '
+ cd "$work_dir" &&
+ # Clean up the state from the previous one
+ git reset --hard pre-rebase &&
+ test_must_fail git rebase master &&
+ test_path_is_dir .git/rebase-apply &&
+ head_before=$(git rev-parse HEAD) &&
+ git rebase --forget &&
+ test $(git rev-parse HEAD) = $head_before &&
+ test ! -d .git/rebase-apply
+'
+
+test_expect_success 'rebase --merge --forget' '
+ cd "$work_dir" &&
+ # Clean up the state from the previous one
+ git reset --hard pre-rebase &&
+ test_must_fail git rebase --merge master &&
+ test_path_is_dir .git/rebase-merge &&
+ head_before=$(git rev-parse HEAD) &&
+ git rebase --forget &&
+ test $(git rev-parse HEAD) = $head_before &&
+ test ! -d .git/rebase-merge
+'
+
test_done
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* Re: [PATCH] doc: fix location of 'info/' with $GIT_COMMON_DIR
From: Duy Nguyen @ 2016-11-12 2:01 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: Git Mailing List
In-Reply-To: <20161111112332.27727-1-ps@pks.im>
On Fri, Nov 11, 2016 at 6:23 PM, Patrick Steinhardt <ps@pks.im> wrote:
> With the introduction of the $GIT_COMMON_DIR variable, the
> repository layout manual was changed to reflect the location for
> many files in case the variable is set. While adding the new
> locations, one typo snuck in regarding the location of the
> 'info/' folder, which is falsely claimed to reside at
> "$GIT_COMMON_DIR/index".
>
> Fix the typo to point to "$GIT_COMMON_DIR/info/" instead.
Oops. Ack!
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> Documentation/gitrepository-layout.txt | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/gitrepository-layout.txt b/Documentation/gitrepository-layout.txt
> index 577ee84..a5f99cb 100644
> --- a/Documentation/gitrepository-layout.txt
> +++ b/Documentation/gitrepository-layout.txt
> @@ -177,7 +177,7 @@ sharedindex.<SHA-1>::
> info::
> Additional information about the repository is recorded
> in this directory. This directory is ignored if $GIT_COMMON_DIR
> - is set and "$GIT_COMMON_DIR/index" will be used instead.
> + is set and "$GIT_COMMON_DIR/info" will be used instead.
>
> info/refs::
> This file helps dumb transports discover what refs are
> --
> 2.10.2
>
--
Duy
^ permalink raw reply
* Re: [PATCH v3 6/6] grep: search history of moved submodules
From: Stefan Beller @ 2016-11-12 0:30 UTC (permalink / raw)
To: Brandon Williams; +Cc: git@vger.kernel.org, Jonathan Tan, Junio C Hamano
In-Reply-To: <1478908273-190166-7-git-send-email-bmwill@google.com>
On Fri, Nov 11, 2016 at 3:51 PM, Brandon Williams <bmwill@google.com> wrote:
> +
> + rm -rf parent sub
This line sounds like a perfect candidate for "test_when_finished"
at the beginning of the test
^ permalink raw reply
* Re: [PATCH v3 2/6] submodules: load gitmodules file from commit sha1
From: Stefan Beller @ 2016-11-12 0:22 UTC (permalink / raw)
To: Brandon Williams; +Cc: git@vger.kernel.org, Jonathan Tan, Junio C Hamano
In-Reply-To: <1478908273-190166-3-git-send-email-bmwill@google.com>
On Fri, Nov 11, 2016 at 3:51 PM, Brandon Williams <bmwill@google.com> wrote:
> teach submodules to load a '.gitmodules' file from a commit sha1. This
> enables the population of the submodule_cache to be based on the state
> of the '.gitmodules' file from a particular commit.
This is the actual implementation that lead to
https://public-inbox.org/git/20161102231722.15787-4-sbeller@google.com/
(part of origin/sb/submodule-config-cleanup)
To produce cleaner history, we may want to pick that commit into this patch?
That would allow to extend the documentation or just this commit message
to talk about raciness in case we ever want to go multi-threaded with this,
as the current API is not ready for threading, AFAICT this will be used as:
gitmodules_config_sha1(&interested_sha1)
struct submodule *sub = submodule_by_path(path, null_sha1);
and the reason you need this API for now is because the
two lines of code happen to called at very different places, such that it is
more convenient to have this API instead of calling submodule_from_path with
the correct sha1 in the first place. This is because the sha1 is not
available at
the place where you want to call submodule_by_path.
>
> Signed-off-by: Brandon Williams <bmwill@google.com>
> ---
> cache.h | 2 ++
> config.c | 8 ++++----
> submodule-config.c | 6 +++---
> submodule-config.h | 3 +++
> submodule.c | 12 ++++++++++++
> submodule.h | 1 +
> 6 files changed, 25 insertions(+), 7 deletions(-)
>
> diff --git a/cache.h b/cache.h
> index 1be6526..559a461 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -1690,6 +1690,8 @@ extern int git_default_config(const char *, const char *, void *);
> extern int git_config_from_file(config_fn_t fn, const char *, void *);
> extern int git_config_from_mem(config_fn_t fn, const enum config_origin_type,
> const char *name, const char *buf, size_t len, void *data);
> +extern int git_config_from_blob_sha1(config_fn_t fn, const char *name,
> + const unsigned char *sha1, void *data);
> extern void git_config_push_parameter(const char *text);
> extern int git_config_from_parameters(config_fn_t fn, void *data);
> extern void git_config(config_fn_t fn, void *);
> diff --git a/config.c b/config.c
> index 83fdecb..4d78e72 100644
> --- a/config.c
> +++ b/config.c
> @@ -1214,10 +1214,10 @@ int git_config_from_mem(config_fn_t fn, const enum config_origin_type origin_typ
> return do_config_from(&top, fn, data);
> }
>
> -static int git_config_from_blob_sha1(config_fn_t fn,
> - const char *name,
> - const unsigned char *sha1,
> - void *data)
> +int git_config_from_blob_sha1(config_fn_t fn,
> + const char *name,
> + const unsigned char *sha1,
> + void *data)
> {
> enum object_type type;
> char *buf;
> diff --git a/submodule-config.c b/submodule-config.c
> index 098085b..8b9a2ef 100644
> --- a/submodule-config.c
> +++ b/submodule-config.c
> @@ -379,9 +379,9 @@ static int parse_config(const char *var, const char *value, void *data)
> return ret;
> }
>
> -static int gitmodule_sha1_from_commit(const unsigned char *commit_sha1,
> - unsigned char *gitmodules_sha1,
> - struct strbuf *rev)
> +int gitmodule_sha1_from_commit(const unsigned char *commit_sha1,
> + unsigned char *gitmodules_sha1,
> + struct strbuf *rev)
> {
> int ret = 0;
>
> diff --git a/submodule-config.h b/submodule-config.h
> index d05c542..78584ba 100644
> --- a/submodule-config.h
> +++ b/submodule-config.h
> @@ -29,6 +29,9 @@ const struct submodule *submodule_from_name(const unsigned char *commit_sha1,
> const char *name);
> const struct submodule *submodule_from_path(const unsigned char *commit_sha1,
> const char *path);
> +extern int gitmodule_sha1_from_commit(const unsigned char *commit_sha1,
> + unsigned char *gitmodules_sha1,
> + struct strbuf *rev);
> void submodule_free(void);
>
> #endif /* SUBMODULE_CONFIG_H */
> diff --git a/submodule.c b/submodule.c
> index f5107f0..062e58b 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -198,6 +198,18 @@ void gitmodules_config(void)
> }
> }
>
> +void gitmodules_config_sha1(const unsigned char *commit_sha1)
> +{
> + struct strbuf rev = STRBUF_INIT;
> + unsigned char sha1[20];
> +
> + if (gitmodule_sha1_from_commit(commit_sha1, sha1, &rev)) {
> + git_config_from_blob_sha1(submodule_config, rev.buf,
> + sha1, NULL);
> + }
> + strbuf_release(&rev);
> +}
> +
> /*
> * Determine if a submodule has been initialized at a given 'path'
> */
> diff --git a/submodule.h b/submodule.h
> index 6ec5f2f..9203d89 100644
> --- a/submodule.h
> +++ b/submodule.h
> @@ -37,6 +37,7 @@ void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,
> const char *path);
> int submodule_config(const char *var, const char *value, void *cb);
> void gitmodules_config(void);
> +extern void gitmodules_config_sha1(const unsigned char *commit_sha1);
> extern int is_submodule_initialized(const char *path);
> extern int is_submodule_populated(const char *path);
> int parse_submodule_update_strategy(const char *value,
> --
> 2.8.0.rc3.226.g39d4020
>
^ permalink raw reply
* [PATCH v3 3/6] grep: add submodules as a grep source type
From: Brandon Williams @ 2016-11-11 23:51 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, sbeller, jonathantanmy, gitster
In-Reply-To: <1478908273-190166-1-git-send-email-bmwill@google.com>
Add `GREP_SOURCE_SUBMODULE` as a grep_source type and cases for this new
type in the various switch statements in grep.c.
When initializing a grep_source with type `GREP_SOURCE_SUBMODULE` the
identifier can either be NULL (to indicate that the working tree will be
used) or a SHA1 (the REV of the submodule to be grep'd). If the
identifier is a SHA1 then we want to fall through to the
`GREP_SOURCE_SHA1` case to handle the copying of the SHA1.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
grep.c | 16 +++++++++++++++-
grep.h | 1 +
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/grep.c b/grep.c
index 1194d35..0dbdc1d 100644
--- a/grep.c
+++ b/grep.c
@@ -1735,12 +1735,23 @@ void grep_source_init(struct grep_source *gs, enum grep_source_type type,
case GREP_SOURCE_FILE:
gs->identifier = xstrdup(identifier);
break;
+ case GREP_SOURCE_SUBMODULE:
+ if (!identifier) {
+ gs->identifier = NULL;
+ break;
+ }
+ /*
+ * FALL THROUGH
+ * If the identifier is non-NULL (in the submodule case) it
+ * will be a SHA1 that needs to be copied.
+ */
case GREP_SOURCE_SHA1:
gs->identifier = xmalloc(20);
hashcpy(gs->identifier, identifier);
break;
case GREP_SOURCE_BUF:
gs->identifier = NULL;
+ break;
}
}
@@ -1760,6 +1771,7 @@ void grep_source_clear_data(struct grep_source *gs)
switch (gs->type) {
case GREP_SOURCE_FILE:
case GREP_SOURCE_SHA1:
+ case GREP_SOURCE_SUBMODULE:
free(gs->buf);
gs->buf = NULL;
gs->size = 0;
@@ -1831,8 +1843,10 @@ static int grep_source_load(struct grep_source *gs)
return grep_source_load_sha1(gs);
case GREP_SOURCE_BUF:
return gs->buf ? 0 : -1;
+ case GREP_SOURCE_SUBMODULE:
+ break;
}
- die("BUG: invalid grep_source type");
+ die("BUG: invalid grep_source type to load");
}
void grep_source_load_driver(struct grep_source *gs)
diff --git a/grep.h b/grep.h
index 5856a23..267534c 100644
--- a/grep.h
+++ b/grep.h
@@ -161,6 +161,7 @@ struct grep_source {
GREP_SOURCE_SHA1,
GREP_SOURCE_FILE,
GREP_SOURCE_BUF,
+ GREP_SOURCE_SUBMODULE,
} type;
void *identifier;
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH v3 4/6] grep: optionally recurse into submodules
From: Brandon Williams @ 2016-11-11 23:51 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, sbeller, jonathantanmy, gitster
In-Reply-To: <1478908273-190166-1-git-send-email-bmwill@google.com>
Allow grep to recognize submodules and recursively search for patterns in
each submodule. This is done by forking off a process to recursively
call grep on each submodule. The top level --super-prefix option is
used to pass a path to the submodule which can in turn be used to
prepend to output or in pathspec matching logic.
Recursion only occurs for submodules which have been initialized and
checked out by the parent project. If a submodule hasn't been
initialized and checked out it is simply skipped.
In order to support the existing multi-threading infrastructure in grep,
output from each child process is captured in a strbuf so that it can be
later printed to the console in an ordered fashion.
To limit the number of theads that are created, each child process has
half the number of threads as its parents (minimum of 1), otherwise we
potentailly have a fork-bomb.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
Documentation/git-grep.txt | 5 +
builtin/grep.c | 300 ++++++++++++++++++++++++++++++++++---
git.c | 2 +-
t/t7814-grep-recurse-submodules.sh | 99 ++++++++++++
4 files changed, 385 insertions(+), 21 deletions(-)
create mode 100755 t/t7814-grep-recurse-submodules.sh
diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index 0ecea6e..17aa1ba 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -26,6 +26,7 @@ SYNOPSIS
[--threads <num>]
[-f <file>] [-e] <pattern>
[--and|--or|--not|(|)|-e <pattern>...]
+ [--recurse-submodules]
[ [--[no-]exclude-standard] [--cached | --no-index | --untracked] | <tree>...]
[--] [<pathspec>...]
@@ -88,6 +89,10 @@ OPTIONS
mechanism. Only useful when searching files in the current directory
with `--no-index`.
+--recurse-submodules::
+ Recursively search in each submodule that has been initialized and
+ checked out in the repository.
+
-a::
--text::
Process binary files as if they were text.
diff --git a/builtin/grep.c b/builtin/grep.c
index 8887b6a..1fd292f 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -18,12 +18,20 @@
#include "quote.h"
#include "dir.h"
#include "pathspec.h"
+#include "submodule.h"
static char const * const grep_usage[] = {
N_("git grep [<options>] [-e] <pattern> [<rev>...] [[--] <path>...]"),
NULL
};
+static const char *super_prefix;
+static int recurse_submodules;
+static struct argv_array submodule_options = ARGV_ARRAY_INIT;
+
+static int grep_submodule_launch(struct grep_opt *opt,
+ const struct grep_source *gs);
+
#define GREP_NUM_THREADS_DEFAULT 8
static int num_threads;
@@ -174,7 +182,10 @@ static void *run(void *arg)
break;
opt->output_priv = w;
- hit |= grep_source(opt, &w->source);
+ if (w->source.type == GREP_SOURCE_SUBMODULE)
+ hit |= grep_submodule_launch(opt, &w->source);
+ else
+ hit |= grep_source(opt, &w->source);
grep_source_clear_data(&w->source);
work_done(w);
}
@@ -300,6 +311,10 @@ static int grep_sha1(struct grep_opt *opt, const unsigned char *sha1,
if (opt->relative && opt->prefix_length) {
quote_path_relative(filename + tree_name_len, opt->prefix, &pathbuf);
strbuf_insert(&pathbuf, 0, filename, tree_name_len);
+ } else if (super_prefix) {
+ strbuf_add(&pathbuf, filename, tree_name_len);
+ strbuf_addstr(&pathbuf, super_prefix);
+ strbuf_addstr(&pathbuf, filename + tree_name_len);
} else {
strbuf_addstr(&pathbuf, filename);
}
@@ -328,10 +343,13 @@ static int grep_file(struct grep_opt *opt, const char *filename)
{
struct strbuf buf = STRBUF_INIT;
- if (opt->relative && opt->prefix_length)
+ if (opt->relative && opt->prefix_length) {
quote_path_relative(filename, opt->prefix, &buf);
- else
+ } else {
+ if (super_prefix)
+ strbuf_addstr(&buf, super_prefix);
strbuf_addstr(&buf, filename);
+ }
#ifndef NO_PTHREADS
if (num_threads) {
@@ -378,31 +396,258 @@ static void run_pager(struct grep_opt *opt, const char *prefix)
exit(status);
}
-static int grep_cache(struct grep_opt *opt, const struct pathspec *pathspec, int cached)
+static void compile_submodule_options(const struct grep_opt *opt,
+ const struct pathspec *pathspec,
+ int cached, int untracked,
+ int opt_exclude, int use_index,
+ int pattern_type_arg)
+{
+ struct grep_pat *pattern;
+ int i;
+
+ if (recurse_submodules)
+ argv_array_push(&submodule_options, "--recurse-submodules");
+
+ if (cached)
+ argv_array_push(&submodule_options, "--cached");
+ if (!use_index)
+ argv_array_push(&submodule_options, "--no-index");
+ if (untracked)
+ argv_array_push(&submodule_options, "--untracked");
+ if (opt_exclude > 0)
+ argv_array_push(&submodule_options, "--exclude-standard");
+
+ if (opt->invert)
+ argv_array_push(&submodule_options, "-v");
+ if (opt->ignore_case)
+ argv_array_push(&submodule_options, "-i");
+ if (opt->word_regexp)
+ argv_array_push(&submodule_options, "-w");
+ switch (opt->binary) {
+ case GREP_BINARY_NOMATCH:
+ argv_array_push(&submodule_options, "-I");
+ break;
+ case GREP_BINARY_TEXT:
+ argv_array_push(&submodule_options, "-a");
+ break;
+ default:
+ break;
+ }
+ if (opt->allow_textconv)
+ argv_array_push(&submodule_options, "--textconv");
+ if (opt->max_depth != -1)
+ argv_array_pushf(&submodule_options, "--max-depth=%d",
+ opt->max_depth);
+ if (opt->linenum)
+ argv_array_push(&submodule_options, "-n");
+ if (!opt->pathname)
+ argv_array_push(&submodule_options, "-h");
+ if (!opt->relative)
+ argv_array_push(&submodule_options, "--full-name");
+ if (opt->name_only)
+ argv_array_push(&submodule_options, "-l");
+ if (opt->unmatch_name_only)
+ argv_array_push(&submodule_options, "-L");
+ if (opt->null_following_name)
+ argv_array_push(&submodule_options, "-z");
+ if (opt->count)
+ argv_array_push(&submodule_options, "-c");
+ if (opt->file_break)
+ argv_array_push(&submodule_options, "--break");
+ if (opt->heading)
+ argv_array_push(&submodule_options, "--heading");
+ if (opt->pre_context)
+ argv_array_pushf(&submodule_options, "--before-context=%d",
+ opt->pre_context);
+ if (opt->post_context)
+ argv_array_pushf(&submodule_options, "--after-context=%d",
+ opt->post_context);
+ if (opt->funcname)
+ argv_array_push(&submodule_options, "-p");
+ if (opt->funcbody)
+ argv_array_push(&submodule_options, "-W");
+ if (opt->all_match)
+ argv_array_push(&submodule_options, "--all-match");
+ if (opt->debug)
+ argv_array_push(&submodule_options, "--debug");
+ if (opt->status_only)
+ argv_array_push(&submodule_options, "-q");
+
+ switch (pattern_type_arg) {
+ case GREP_PATTERN_TYPE_BRE:
+ argv_array_push(&submodule_options, "-G");
+ break;
+ case GREP_PATTERN_TYPE_ERE:
+ argv_array_push(&submodule_options, "-E");
+ break;
+ case GREP_PATTERN_TYPE_FIXED:
+ argv_array_push(&submodule_options, "-F");
+ break;
+ case GREP_PATTERN_TYPE_PCRE:
+ argv_array_push(&submodule_options, "-P");
+ break;
+ case GREP_PATTERN_TYPE_UNSPECIFIED:
+ break;
+ }
+
+ for (pattern = opt->pattern_list; pattern != NULL;
+ pattern = pattern->next) {
+ switch (pattern->token) {
+ case GREP_PATTERN:
+ argv_array_pushf(&submodule_options, "-e%s",
+ pattern->pattern);
+ break;
+ case GREP_AND:
+ case GREP_OPEN_PAREN:
+ case GREP_CLOSE_PAREN:
+ case GREP_NOT:
+ case GREP_OR:
+ argv_array_push(&submodule_options, pattern->pattern);
+ break;
+ /* BODY and HEAD are not used by git-grep */
+ case GREP_PATTERN_BODY:
+ case GREP_PATTERN_HEAD:
+ break;
+ }
+ }
+
+ /*
+ * Limit number of threads for child process to use.
+ * This is to prevent potential fork-bomb behavior of git-grep as each
+ * submodule process has its own thread pool.
+ */
+ if (num_threads)
+ argv_array_pushf(&submodule_options, "--threads=%d",
+ (num_threads + 1) / 2);
+
+ /* Add Pathspecs */
+ argv_array_push(&submodule_options, "--");
+ for (i = 0; i < pathspec->nr; i++)
+ argv_array_push(&submodule_options,
+ pathspec->items[i].original);
+}
+
+/*
+ * Launch child process to grep contents of a submodule
+ */
+static int grep_submodule_launch(struct grep_opt *opt,
+ const struct grep_source *gs)
+{
+ struct child_process cp = CHILD_PROCESS_INIT;
+ int status, i;
+ struct work_item *w = opt->output_priv;
+
+ prepare_submodule_repo_env(&cp.env_array);
+
+ /* Add super prefix */
+ argv_array_pushf(&cp.args, "--super-prefix=%s%s/",
+ super_prefix ? super_prefix : "",
+ gs->name);
+ argv_array_push(&cp.args, "grep");
+
+ /* Add options */
+ for (i = 0; i < submodule_options.argc; i++)
+ argv_array_push(&cp.args, submodule_options.argv[i]);
+
+ cp.git_cmd = 1;
+ cp.dir = gs->path;
+
+ /*
+ * Capture output to output buffer and check the return code from the
+ * child process. A '0' indicates a hit, a '1' indicates no hit and
+ * anything else is an error.
+ */
+ status = capture_command(&cp, &w->out, 0);
+ if (status && (status != 1)) {
+ /* flush the buffer */
+ write_or_die(1, w->out.buf, w->out.len);
+ die("process for submodule '%s' failed with exit code: %d",
+ gs->name, status);
+ }
+
+ /* invert the return code to make a hit equal to 1 */
+ return !status;
+}
+
+/*
+ * Prep grep structures for a submodule grep
+ * sha1: the sha1 of the submodule or NULL if using the working tree
+ * filename: name of the submodule including tree name of parent
+ * path: location of the submodule
+ */
+static int grep_submodule(struct grep_opt *opt, const unsigned char *sha1,
+ const char *filename, const char *path)
+{
+ if (!(is_submodule_initialized(path) &&
+ is_submodule_populated(path)))
+ return 0;
+
+#ifndef NO_PTHREADS
+ if (num_threads) {
+ add_work(opt, GREP_SOURCE_SUBMODULE, filename, path, sha1);
+ return 0;
+ } else
+#endif
+ {
+ struct work_item w;
+ int hit;
+
+ grep_source_init(&w.source, GREP_SOURCE_SUBMODULE,
+ filename, path, sha1);
+ strbuf_init(&w.out, 0);
+ opt->output_priv = &w;
+ hit = grep_submodule_launch(opt, &w.source);
+
+ write_or_die(1, w.out.buf, w.out.len);
+
+ grep_source_clear(&w.source);
+ strbuf_release(&w.out);
+ return hit;
+ }
+}
+
+static int grep_cache(struct grep_opt *opt, const struct pathspec *pathspec,
+ int cached)
{
int hit = 0;
int nr;
+ struct strbuf name = STRBUF_INIT;
+ int name_base_len = 0;
+ if (super_prefix) {
+ name_base_len = strlen(super_prefix);
+ strbuf_addstr(&name, super_prefix);
+ }
+
read_cache();
for (nr = 0; nr < active_nr; nr++) {
const struct cache_entry *ce = active_cache[nr];
- if (!S_ISREG(ce->ce_mode))
- continue;
- if (!ce_path_match(ce, pathspec, NULL))
- continue;
- /*
- * If CE_VALID is on, we assume worktree file and its cache entry
- * are identical, even if worktree file has been modified, so use
- * cache version instead
- */
- if (cached || (ce->ce_flags & CE_VALID) || ce_skip_worktree(ce)) {
- if (ce_stage(ce) || ce_intent_to_add(ce))
- continue;
- hit |= grep_sha1(opt, ce->oid.hash, ce->name, 0,
- ce->name);
+ strbuf_setlen(&name, name_base_len);
+ strbuf_addstr(&name, ce->name);
+
+ if (S_ISREG(ce->ce_mode) &&
+ match_pathspec(pathspec, name.buf, name.len, 0, NULL,
+ S_ISDIR(ce->ce_mode) ||
+ S_ISGITLINK(ce->ce_mode))) {
+ /*
+ * If CE_VALID is on, we assume worktree file and its
+ * cache entry are identical, even if worktree file has
+ * been modified, so use cache version instead
+ */
+ if (cached || (ce->ce_flags & CE_VALID) ||
+ ce_skip_worktree(ce)) {
+ if (ce_stage(ce) || ce_intent_to_add(ce))
+ continue;
+ hit |= grep_sha1(opt, ce->oid.hash, ce->name,
+ 0, ce->name);
+ } else {
+ hit |= grep_file(opt, ce->name);
+ }
+ } else if (recurse_submodules && S_ISGITLINK(ce->ce_mode) &&
+ submodule_path_match(pathspec, name.buf, NULL)) {
+ hit |= grep_submodule(opt, NULL, ce->name, ce->name);
}
- else
- hit |= grep_file(opt, ce->name);
+
if (ce_stage(ce)) {
do {
nr++;
@@ -413,6 +658,8 @@ static int grep_cache(struct grep_opt *opt, const struct pathspec *pathspec, int
if (hit && opt->status_only)
break;
}
+
+ strbuf_release(&name);
return hit;
}
@@ -651,6 +898,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
N_("search in both tracked and untracked files")),
OPT_SET_INT(0, "exclude-standard", &opt_exclude,
N_("ignore files specified via '.gitignore'"), 1),
+ OPT_BOOL(0, "recurse-submodules", &recurse_submodules,
+ N_("recursivley search in each submodule")),
OPT_GROUP(""),
OPT_BOOL('v', "invert-match", &opt.invert,
N_("show non-matching lines")),
@@ -755,6 +1004,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
init_grep_defaults();
git_config(grep_cmd_config, NULL);
grep_init(&opt, prefix);
+ super_prefix = get_super_prefix();
/*
* If there is no -- then the paths must exist in the working
@@ -872,6 +1122,13 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
pathspec.max_depth = opt.max_depth;
pathspec.recursive = 1;
+ if (recurse_submodules) {
+ gitmodules_config();
+ compile_submodule_options(&opt, &pathspec, cached, untracked,
+ opt_exclude, use_index,
+ pattern_type_arg);
+ }
+
if (show_in_pager && (cached || list.nr))
die(_("--open-files-in-pager only works on the worktree"));
@@ -895,6 +1152,9 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
}
}
+ if (recurse_submodules && (!use_index || untracked || list.nr))
+ die(_("option not supported with --recurse-submodules."));
+
if (!show_in_pager && !opt.status_only)
setup_pager();
diff --git a/git.c b/git.c
index efa1059..a156efd 100644
--- a/git.c
+++ b/git.c
@@ -434,7 +434,7 @@ static struct cmd_struct commands[] = {
{ "fsck-objects", cmd_fsck, RUN_SETUP },
{ "gc", cmd_gc, RUN_SETUP },
{ "get-tar-commit-id", cmd_get_tar_commit_id },
- { "grep", cmd_grep, RUN_SETUP_GENTLY },
+ { "grep", cmd_grep, RUN_SETUP_GENTLY | SUPPORT_SUPER_PREFIX },
{ "hash-object", cmd_hash_object },
{ "help", cmd_help },
{ "index-pack", cmd_index_pack, RUN_SETUP_GENTLY },
diff --git a/t/t7814-grep-recurse-submodules.sh b/t/t7814-grep-recurse-submodules.sh
new file mode 100755
index 0000000..b670c70
--- /dev/null
+++ b/t/t7814-grep-recurse-submodules.sh
@@ -0,0 +1,99 @@
+#!/bin/sh
+
+test_description='Test grep recurse-submodules feature
+
+This test verifies the recurse-submodules feature correctly greps across
+submodules.
+'
+
+. ./test-lib.sh
+
+test_expect_success 'setup directory structure and submodule' '
+ echo "foobar" >a &&
+ mkdir b &&
+ echo "bar" >b/b &&
+ git add a b &&
+ git commit -m "add a and b" &&
+ git init submodule &&
+ echo "foobar" >submodule/a &&
+ git -C submodule add a &&
+ git -C submodule commit -m "add a" &&
+ git submodule add ./submodule &&
+ git commit -m "added submodule"
+'
+
+test_expect_success 'grep correctly finds patterns in a submodule' '
+ cat >expect <<-\EOF &&
+ a:foobar
+ b/b:bar
+ submodule/a:foobar
+ EOF
+
+ git grep -e "bar" --recurse-submodules >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'grep and basic pathspecs' '
+ cat >expect <<-\EOF &&
+ submodule/a:foobar
+ EOF
+
+ git grep -e. --recurse-submodules -- submodule >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'grep and nested submodules' '
+ git init submodule/sub &&
+ echo "foobar" >submodule/sub/a &&
+ git -C submodule/sub add a &&
+ git -C submodule/sub commit -m "add a" &&
+ git -C submodule submodule add ./sub &&
+ git -C submodule add sub &&
+ git -C submodule commit -m "added sub" &&
+ git add submodule &&
+ git commit -m "updated submodule" &&
+
+ cat >expect <<-\EOF &&
+ a:foobar
+ b/b:bar
+ submodule/a:foobar
+ submodule/sub/a:foobar
+ EOF
+
+ git grep -e "bar" --recurse-submodules > actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'grep and multiple patterns' '
+ cat >expect <<-\EOF &&
+ a:foobar
+ submodule/a:foobar
+ submodule/sub/a:foobar
+ EOF
+
+ git grep -e "bar" --and -e "foo" --recurse-submodules > actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'grep and multiple patterns' '
+ cat >expect <<-\EOF &&
+ b/b:bar
+ EOF
+
+ git grep -e "bar" --and --not -e "foo" --recurse-submodules > actual &&
+ test_cmp expect actual
+'
+
+test_incompatible_with_recurse_submodules ()
+{
+ test_expect_success "--recurse-submodules and $1 are incompatible" "
+ test_must_fail git grep -e. --recurse-submodules $1 2>actual &&
+ test_i18ngrep 'not supported with --recurse-submodules' actual
+ "
+}
+
+test_incompatible_with_recurse_submodules --untracked
+test_incompatible_with_recurse_submodules --no-index
+test_incompatible_with_recurse_submodules HEAD
+
+test_done
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox