threads / patch / 10481

patchAdd NEED_WORK_TREE for more commands

Subject: [PATCH] Add NEED_WORK_TREE for more commands

## tl;dr

5 messages between Oct 27, 2007 and Oct 27, 2007. Diffs are folded; open one to read it.

replies: 4people: 3as markdown or json

Nguyễn Thái Ngọc Duy· Oct 27, 2007, 08:19 UTC · lore

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 git.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

Show changes to git.c +6 −6

diff --git a/git.c b/git.c
index 23a430c..9db40b3 100644
--- a/git.c
+++ b/git.c
@@ -307,10 +307,10 @@ static void handle_internal_command(int argc, const char **argv)
 	const char *cmd = argv[0];
 	static struct cmd_struct commands[] = {
 		{ "add", cmd_add, RUN_SETUP | NEED_WORK_TREE },
-		{ "annotate", cmd_annotate, RUN_SETUP },
+		{ "annotate", cmd_annotate, RUN_SETUP | NEED_WORK_TREE },
 		{ "apply", cmd_apply },
 		{ "archive", cmd_archive },
-		{ "blame", cmd_blame, RUN_SETUP },
+		{ "blame", cmd_blame, RUN_SETUP | NEED_WORK_TREE },
 		{ "branch", cmd_branch, RUN_SETUP },
 		{ "bundle", cmd_bundle },
 		{ "cat-file", cmd_cat_file, RUN_SETUP },
@@ -333,7 +333,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "fetch--tool", cmd_fetch__tool, RUN_SETUP },
 		{ "fmt-merge-msg", cmd_fmt_merge_msg, RUN_SETUP },
 		{ "for-each-ref", cmd_for_each_ref, RUN_SETUP },
-		{ "format-patch", cmd_format_patch, RUN_SETUP },
+		{ "format-patch", cmd_format_patch, RUN_SETUP | NEED_WORK_TREE },
 		{ "fsck", cmd_fsck, RUN_SETUP },
 		{ "fsck-objects", cmd_fsck, RUN_SETUP },
 		{ "gc", cmd_gc, RUN_SETUP },
@@ -346,7 +346,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "init", cmd_init_db },
 		{ "init-db", cmd_init_db },
 		{ "log", cmd_log, RUN_SETUP | USE_PAGER },
-		{ "ls-files", cmd_ls_files, RUN_SETUP },
+		{ "ls-files", cmd_ls_files, RUN_SETUP | NEED_WORK_TREE },
 		{ "ls-tree", cmd_ls_tree, RUN_SETUP },
 		{ "mailinfo", cmd_mailinfo },
 		{ "mailsplit", cmd_mailsplit },
@@ -363,7 +363,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "reflog", cmd_reflog, RUN_SETUP },
 		{ "repo-config", cmd_config },
 		{ "rerere", cmd_rerere, RUN_SETUP },
-		{ "reset", cmd_reset, RUN_SETUP },
+		{ "reset", cmd_reset, RUN_SETUP | NEED_WORK_TREE },
 		{ "rev-list", cmd_rev_list, RUN_SETUP },
 		{ "rev-parse", cmd_rev_parse, RUN_SETUP },
 		{ "revert", cmd_revert, RUN_SETUP | NEED_WORK_TREE },
@@ -377,7 +377,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "tag", cmd_tag, RUN_SETUP },
 		{ "tar-tree", cmd_tar_tree },
 		{ "unpack-objects", cmd_unpack_objects, RUN_SETUP },
-		{ "update-index", cmd_update_index, RUN_SETUP },
+		{ "update-index", cmd_update_index, RUN_SETUP | NEED_WORK_TREE },
 		{ "update-ref", cmd_update_ref, RUN_SETUP },
 		{ "upload-archive", cmd_upload_archive },
 		{ "verify-tag", cmd_verify_tag, RUN_SETUP },

-- 
1.5.3.rc4.3.gab089

Mike Hommey· Oct 27, 2007, 09:08 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH] Add NEED_WORK_TREE for more commands

On Sat, Oct 27, 2007 at 03:19:10PM +0700, Nguyễn Thái Ngọc Duy wrote:

Show 12 quoted lines

> 
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  git.c |   12 ++++++------
>  1 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/git.c b/git.c
> index 23a430c..9db40b3 100644
> --- a/git.c
> +++ b/git.c
> +		{ "ls-files", cmd_ls_files, RUN_SETUP | NEED_WORK_TREE },
> +		{ "update-index", cmd_update_index, RUN_SETUP | NEED_WORK_TREE },

At least these two work very fine without a working tree (and I *do* need them to work without a working tree).

Mike

Nguyen Thai Ngoc Duy· Oct 27, 2007, 10:18 UTC · re: Mike Hommey · lore

Re: [PATCH] Add NEED_WORK_TREE for more commands

On Sat, Oct 27, 2007 at 11:08:22AM +0200, Mike Hommey wrote:

Show 16 quoted lines

> On Sat, Oct 27, 2007 at 03:19:10PM +0700, Nguy???n Th??i Ng???c Duy wrote:
> > 
> > Signed-off-by: Nguy???n Th??i Ng???c Duy <pclouds@gmail.com>
> > ---
> >  git.c |   12 ++++++------
> >  1 files changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/git.c b/git.c
> > index 23a430c..9db40b3 100644
> > --- a/git.c
> > +++ b/git.c
> > +		{ "ls-files", cmd_ls_files, RUN_SETUP | NEED_WORK_TREE },
> > +		{ "update-index", cmd_update_index, RUN_SETUP | NEED_WORK_TREE },
> 
> At least these two work very fine without a working tree (and I *do*
> need them to work without a working tree).

How can you do that while both need index and a working directory to operate?

> Mike

-- 
Duy

Mike Hommey· Oct 27, 2007, 10:42 UTC · re: Nguyen Thai Ngoc Duy · lore

Re: [PATCH] Add NEED_WORK_TREE for more commands

On Sat, Oct 27, 2007 at 05:18:39PM +0700, Nguyen Thai Ngoc Duy wrote:

Show 20 quoted lines

> On Sat, Oct 27, 2007 at 11:08:22AM +0200, Mike Hommey wrote:
> > On Sat, Oct 27, 2007 at 03:19:10PM +0700, Nguy???n Th??i Ng???c Duy wrote:
> > > 
> > > Signed-off-by: Nguy???n Th??i Ng???c Duy <pclouds@gmail.com>
> > > ---
> > >  git.c |   12 ++++++------
> > >  1 files changed, 6 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/git.c b/git.c
> > > index 23a430c..9db40b3 100644
> > > --- a/git.c
> > > +++ b/git.c
> > > +		{ "ls-files", cmd_ls_files, RUN_SETUP | NEED_WORK_TREE },
> > > +		{ "update-index", cmd_update_index, RUN_SETUP | NEED_WORK_TREE },
> > 
> > At least these two work very fine without a working tree (and I *do*
> > need them to work without a working tree).
> 
> How can you do that while both need index and a working directory to
> operate?

For example, ls-files -c and update-index --remove don't need more than the index, and you don't need a working tree to fiddle with the index with these commands.

By the way, git-rm has a --cached option that allows it to only work on the index, but it has a NEED_WORK_TREE which prevents its use without a working tree in such a case (though I don't know if the implementation itself relies on the working tree or not, but if it does, it shouldn't). It's sad to have to do git-ls-files -c directory | xargs -d "\n" git-update-index --remove instead of git-rm -r --cached directory when you want to remove a directory in a bare repository.

Mike

Johannes Schindelin· Oct 27, 2007, 12:42 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH] Add NEED_WORK_TREE for more commands

Hi,

On Sat, 27 Oct 2007, Nguyễn Thái Ngọc Duy wrote:

> +		{ "annotate", cmd_annotate, RUN_SETUP | NEED_WORK_TREE },
> +		{ "blame", cmd_blame, RUN_SETUP | NEED_WORK_TREE },

Both can be sensibly run without a work tree: "git blame <commit> <file>"

> +		{ "format-patch", cmd_format_patch, RUN_SETUP | NEED_WORK_TREE },

This can be run without work tree in any case, methinks. AFAICT format-patch only ever uses information from the object database.

> +		{ "ls-files", cmd_ls_files, RUN_SETUP | NEED_WORK_TREE },

That has already been commented on.

> +		{ "reset", cmd_reset, RUN_SETUP | NEED_WORK_TREE },

"git reset --soft HEAD <commit>" makes sense in a bare repository.

> +		{ "update-index", cmd_update_index, RUN_SETUP | NEED_WORK_TREE },

That has been coommented on, too.

I agree that there are modes for these commands (except format-patch), in which they want a work-tree, but they have other modes, too, so IMHO it should be solved a la 6d9ba67b(Commands requiring a work tree must not run in GIT_DIR). You'll see that ls-files already has some special handling here, and this could be refactorised to requires_work_tree(), which should make the code much more elegant.

Ciao, Dscho