{"thread":{"id":"42497","subject":"[PATCH] add: add --chmod=+x / --chmod=-x options","startedAt":"2016-05-31T22:08:18Z","lastAt":"2016-06-08T11:46:14Z","messageCount":8,"participants":["Edward Thomson","Junio C Hamano","Johannes Schindelin","Duy Nguyen"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"287940","messageId":"20160531220818.GB46739@zoidberg","threadId":"42497","inReplyTo":null,"subject":"[PATCH] add: add --chmod=+x / --chmod=-x options","fromName":"Edward Thomson","fromEmail":"ethomson@edwardthomson.com","sentAt":"2016-05-31T22:08:18Z","receivedAt":"2016-05-31T22:08:18Z","isPatch":true,"sender":{"key":"ethomson@edwardthomson.com","avatar":"https://avatars.githubusercontent.com/u/1130014?v=4"},"body":"The executable bit will not be detected (and therefore will not be\nset) for paths in a repository with `core.filemode` set to false,\nthough the users may still wish to add files as executable for\ncompatibility with other users who _do_ have `core.filemode`\nfunctionality.  For example, Windows users adding shell scripts may\nwish to add them as executable for compatibility with users on\nnon-Windows.\n\nAlthough this can be done with a plumbing command\n(`git update-index --add --chmod=+x foo`), teaching the `git-add`\ncommand allows users to set a file executable with a command that\nthey're already familiar with.\n\nSigned-off-by: Edward Thomson <ethomson@edwardthomson.com>\n---\n builtin/add.c  | 12 +++++++++++-\n cache.h        |  2 ++\n read-cache.c   | 11 +++++++++--\n t/t3700-add.sh | 30 ++++++++++++++++++++++++++++++\n 4 files changed, 52 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex 145f06e..44b6c97 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -238,6 +238,8 @@ static int ignore_add_errors, intent_to_add, ignore_missing;\n static int addremove = ADDREMOVE_DEFAULT;\n static int addremove_explicit = -1; /* unspecified */\n \n+static char *chmod_arg = NULL;\n+\n static int ignore_removal_cb(const struct option *opt, const char *arg, int unset)\n {\n \t/* if we are told to ignore, we are not adding removals */\n@@ -263,6 +265,7 @@ static struct option builtin_add_options[] = {\n \tOPT_BOOL( 0 , \"refresh\", &refresh_only, N_(\"don't add, only refresh the index\")),\n \tOPT_BOOL( 0 , \"ignore-errors\", &ignore_add_errors, N_(\"just skip files which cannot be added because of errors\")),\n \tOPT_BOOL( 0 , \"ignore-missing\", &ignore_missing, N_(\"check if - even missing - files are ignored in dry run\")),\n+\tOPT_STRING( 0 , \"chmod\", &chmod_arg, N_(\"(+/-)x\"), N_(\"override the executable bit of the listed files\")),\n \tOPT_END(),\n };\n \n@@ -336,6 +339,11 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \tif (!show_only && ignore_missing)\n \t\tdie(_(\"Option --ignore-missing can only be used together with --dry-run\"));\n \n+\tif (chmod_arg) {\n+\t\tif (strcmp(chmod_arg, \"-x\") && strcmp(chmod_arg, \"+x\"))\n+\t\t\tdie(_(\"--chmod param must be either -x or +x\"));\n+\t}\n+\n \tadd_new_files = !take_worktree_changes && !refresh_only;\n \trequire_pathspec = !(take_worktree_changes || (0 < addremove_explicit));\n \n@@ -346,7 +354,9 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \t\t (intent_to_add ? ADD_CACHE_INTENT : 0) |\n \t\t (ignore_add_errors ? ADD_CACHE_IGNORE_ERRORS : 0) |\n \t\t (!(addremove || take_worktree_changes)\n-\t\t  ? ADD_CACHE_IGNORE_REMOVAL : 0));\n+\t\t  ? ADD_CACHE_IGNORE_REMOVAL : 0)) |\n+\t\t (chmod_arg && *chmod_arg == '+' ? ADD_CACHE_FORCE_EXECUTABLE : 0) |\n+\t\t (chmod_arg && *chmod_arg == '-' ? ADD_CACHE_FORCE_NOTEXECUTABLE : 0);\n \n \tif (require_pathspec && argc == 0) {\n \t\tfprintf(stderr, _(\"Nothing specified, nothing added.\\n\"));\ndiff --git a/cache.h b/cache.h\nindex 6049f86..da03cd9 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -581,6 +581,8 @@ extern int remove_file_from_index(struct index_state *, const char *path);\n #define ADD_CACHE_IGNORE_ERRORS\t4\n #define ADD_CACHE_IGNORE_REMOVAL 8\n #define ADD_CACHE_INTENT 16\n+#define ADD_CACHE_FORCE_EXECUTABLE 32\n+#define ADD_CACHE_FORCE_NOTEXECUTABLE 64\n extern int add_to_index(struct index_state *, const char *path, struct stat *, int flags);\n extern int add_file_to_index(struct index_state *, const char *path, int flags);\n extern struct cache_entry *make_cache_entry(unsigned int mode, const unsigned char *sha1, const char *path, int stage, unsigned int refresh_options);\ndiff --git a/read-cache.c b/read-cache.c\nindex d9fb78b..d12d143 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -641,6 +641,8 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,\n \tint intent_only = flags & ADD_CACHE_INTENT;\n \tint add_option = (ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE|\n \t\t\t  (intent_only ? ADD_CACHE_NEW_ONLY : 0));\n+\tint force_executable = flags & ADD_CACHE_FORCE_EXECUTABLE;\n+\tint force_notexecutable = flags & ADD_CACHE_FORCE_NOTEXECUTABLE;\n \n \tif (!S_ISREG(st_mode) && !S_ISLNK(st_mode) && !S_ISDIR(st_mode))\n \t\treturn error(\"%s: can only add regular files, symbolic links or git-directories\", path);\n@@ -659,9 +661,14 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,\n \telse\n \t\tce->ce_flags |= CE_INTENT_TO_ADD;\n \n-\tif (trust_executable_bit && has_symlinks)\n+\tif (S_ISREG(st_mode) && (force_executable || force_notexecutable)) {\n+\t\tif (force_executable)\n+\t\t\tce->ce_mode = create_ce_mode(0777);\n+\t\telse\n+\t\t\tce->ce_mode = create_ce_mode(0666);\n+\t} else if (trust_executable_bit && has_symlinks) {\n \t\tce->ce_mode = create_ce_mode(st_mode);\n-\telse {\n+\t} else {\n \t\t/* If there is an existing entry, pick the mode bits and type\n \t\t * from it, otherwise assume unexecutable regular file.\n \t\t */\ndiff --git a/t/t3700-add.sh b/t/t3700-add.sh\nindex f14a665..4865304 100755\n--- a/t/t3700-add.sh\n+++ b/t/t3700-add.sh\n@@ -332,4 +332,34 @@ test_expect_success 'git add --dry-run --ignore-missing of non-existing file out\n \ttest_i18ncmp expect.err actual.err\n '\n \n+test_expect_success 'git add --chmod=+x stages a non-executable file with +x' '\n+\techo foo >foo1 &&\n+\tgit add --chmod=+x foo1 &&\n+\tcase \"$(git ls-files --stage foo1)\" in\n+\t100755\" \"*foo1) echo pass;;\n+\t*) echo fail; git ls-files --stage foo1; (exit 1);;\n+\tesac\n+'\n+\n+test_expect_success 'git add --chmod=-x stages an executable file with -x' '\n+\techo foo >xfoo1 &&\n+\tchmod 755 xfoo1 &&\n+\tgit add --chmod=-x xfoo1 &&\n+\tcase \"$(git ls-files --stage xfoo1)\" in\n+\t100644\" \"*xfoo1) echo pass;;\n+\t*) echo fail; git ls-files --stage xfoo1; (exit 1);;\n+\tesac\n+'\n+\n+test_expect_success POSIXPERM,SYMLINKS 'git add --chmod=+x with symlinks' '\n+\tgit config core.filemode 1 &&\n+\tgit config core.symlinks 1 &&\n+\techo foo >foo2 &&\n+\tgit add --chmod=+x foo2 &&\n+\tcase \"$(git ls-files --stage foo2)\" in\n+\t100755\" \"*foo2) echo pass;;\n+\t*) echo fail; git ls-files --stage foo2; (exit 1);;\n+\tesac\n+'\n+\n test_done\n-- \n2.7.4 (Apple Git-66)\n"},{"id":"287946","messageId":"xmqqtwhd3lht.fsf@gitster.mtv.corp.google.com","threadId":"42497","inReplyTo":"20160531220818.GB46739@zoidberg","subject":"Re: [PATCH] add: add --chmod=+x / --chmod=-x options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-31T22:34:22Z","receivedAt":"2016-05-31T22:34:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Edward Thomson <ethomson@edwardthomson.com> writes:\n\n> +static char *chmod_arg = NULL;\n> +\n\nI'll drop \" = NULL\", as it is our convention to let BSS take care of\nthe zero initialization for global variables as much as possible.\n\nOther than that, I did not see anything objectionable in this round,\nbut I did notice that you kept the \"this takes two bits out of 32\"\nDscho mentioned--I do not have a strong preference either way, so\nI'll queue it (at least tentatively) on 'pu'.\n\n> @@ -346,7 +354,9 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n>  \t\t (intent_to_add ? ADD_CACHE_INTENT : 0) |\n>  \t\t (ignore_add_errors ? ADD_CACHE_IGNORE_ERRORS : 0) |\n>  \t\t (!(addremove || take_worktree_changes)\n> -\t\t  ? ADD_CACHE_IGNORE_REMOVAL : 0));\n> +\t\t  ? ADD_CACHE_IGNORE_REMOVAL : 0)) |\n> +\t\t (chmod_arg && *chmod_arg == '+' ? ADD_CACHE_FORCE_EXECUTABLE : 0) |\n> +\t\t (chmod_arg && *chmod_arg == '-' ? ADD_CACHE_FORCE_NOTEXECUTABLE : 0);\n>  \n>  \tif (require_pathspec && argc == 0) {\n>  \t\tfprintf(stderr, _(\"Nothing specified, nothing added.\\n\"));\n> diff --git a/cache.h b/cache.h\n> index 6049f86..da03cd9 100644\n> --- a/cache.h\n> +++ b/cache.h\n> @@ -581,6 +581,8 @@ extern int remove_file_from_index(struct index_state *, const char *path);\n>  #define ADD_CACHE_IGNORE_ERRORS\t4\n>  #define ADD_CACHE_IGNORE_REMOVAL 8\n>  #define ADD_CACHE_INTENT 16\n> +#define ADD_CACHE_FORCE_EXECUTABLE 32\n> +#define ADD_CACHE_FORCE_NOTEXECUTABLE 64\n"},{"id":"287982","messageId":"alpine.DEB.2.20.1606010919360.5761@virtualbox","threadId":"42497","inReplyTo":"xmqqtwhd3lht.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] add: add --chmod=+x / --chmod=-x options","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-06-01T07:23:21Z","receivedAt":"2016-06-01T07:23:21Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio & Ed,\n\nOn Tue, 31 May 2016, Junio C Hamano wrote:\n\n> Edward Thomson <ethomson@edwardthomson.com> writes:\n> \n> > +static char *chmod_arg = NULL;\n> > +\n> \n> I'll drop \" = NULL\", as it is our convention to let BSS take care of\n> the zero initialization for global variables as much as possible.\n> \n> Other than that, I did not see anything objectionable in this round,\n> but I did notice that you kept the \"this takes two bits out of 32\"\n> Dscho mentioned--I do not have a strong preference either way, so\n> I'll queue it (at least tentatively) on 'pu'.\n\nAnd here is an add-on patch (Ed, feel free to squash) that avoids those\ntwo bits, and even saves one line overall:\n\n-- snipsnap --\ndiff --git a/builtin/add.c b/builtin/add.c\nindex 44b6c97..b1dddb4 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -26,7 +26,7 @@ static int patch_interactive, add_interactive, edit_interactive;\n static int take_worktree_changes;\n \n struct update_callback_data {\n-\tint flags;\n+\tint flags, force_mode;\n \tint add_errors;\n };\n \n@@ -65,7 +65,8 @@ static void update_callback(struct diff_queue_struct *q,\n \t\t\tdie(_(\"unexpected diff status %c\"), p->status);\n \t\tcase DIFF_STATUS_MODIFIED:\n \t\tcase DIFF_STATUS_TYPE_CHANGED:\n-\t\t\tif (add_file_to_index(&the_index, path, data->flags)) {\n+\t\t\tif (add_file_to_index(&the_index, path,\n+\t\t\t\t\tdata->flags, data->force_mode)) {\n \t\t\t\tif (!(data->flags & ADD_CACHE_IGNORE_ERRORS))\n \t\t\t\t\tdie(_(\"updating files failed\"));\n \t\t\t\tdata->add_errors++;\n@@ -83,14 +84,15 @@ static void update_callback(struct diff_queue_struct *q,\n \t}\n }\n \n-int add_files_to_cache(const char *prefix,\n-\t\t       const struct pathspec *pathspec, int flags)\n+int add_files_to_cache(const char *prefix, const struct pathspec *pathspec,\n+\tint flags, int force_mode)\n {\n \tstruct update_callback_data data;\n \tstruct rev_info rev;\n \n \tmemset(&data, 0, sizeof(data));\n \tdata.flags = flags;\n+\tdata.force_mode = force_mode;\n \n \tinit_revisions(&rev, prefix);\n \tsetup_revisions(0, NULL, &rev, NULL);\n@@ -238,7 +240,7 @@ static int ignore_add_errors, intent_to_add, ignore_missing;\n static int addremove = ADDREMOVE_DEFAULT;\n static int addremove_explicit = -1; /* unspecified */\n \n-static char *chmod_arg = NULL;\n+static char *chmod_arg;\n \n static int ignore_removal_cb(const struct option *opt, const char *arg, int unset)\n {\n@@ -279,7 +281,7 @@ static int add_config(const char *var, const char *value, void *cb)\n \treturn git_default_config(var, value, cb);\n }\n \n-static int add_files(struct dir_struct *dir, int flags)\n+static int add_files(struct dir_struct *dir, int flags, int force_mode)\n {\n \tint i, exit_status = 0;\n \n@@ -292,7 +294,8 @@ static int add_files(struct dir_struct *dir, int flags)\n \t}\n \n \tfor (i = 0; i < dir->nr; i++)\n-\t\tif (add_file_to_cache(dir->entries[i]->name, flags)) {\n+\t\tif (add_file_to_index(&the_index, dir->entries[i]->name,\n+\t\t\t\tflags, force_mode)) {\n \t\t\tif (!ignore_add_errors)\n \t\t\t\tdie(_(\"adding files failed\"));\n \t\t\texit_status = 1;\n@@ -305,7 +308,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \tint exit_status = 0;\n \tstruct pathspec pathspec;\n \tstruct dir_struct dir;\n-\tint flags;\n+\tint flags, force_mode;\n \tint add_new_files;\n \tint require_pathspec;\n \tchar *seen = NULL;\n@@ -339,10 +342,14 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \tif (!show_only && ignore_missing)\n \t\tdie(_(\"Option --ignore-missing can only be used together with --dry-run\"));\n \n-\tif (chmod_arg) {\n-\t\tif (strcmp(chmod_arg, \"-x\") && strcmp(chmod_arg, \"+x\"))\n-\t\t\tdie(_(\"--chmod param must be either -x or +x\"));\n-\t}\n+\tif (!chmod_arg)\n+\t\tforce_mode = 0;\n+\telse if (!strcmp(chmod_arg, \"-x\"))\n+\t\tforce_mode = 0666;\n+\telse if (!strcmp(chmod_arg, \"+x\"))\n+\t\tforce_mode = 0777;\n+\telse\n+\t\tdie(_(\"--chmod param '%s' must be either -x or +x\"), chmod_arg);\n \n \tadd_new_files = !take_worktree_changes && !refresh_only;\n \trequire_pathspec = !(take_worktree_changes || (0 < addremove_explicit));\n@@ -354,9 +361,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \t\t (intent_to_add ? ADD_CACHE_INTENT : 0) |\n \t\t (ignore_add_errors ? ADD_CACHE_IGNORE_ERRORS : 0) |\n \t\t (!(addremove || take_worktree_changes)\n-\t\t  ? ADD_CACHE_IGNORE_REMOVAL : 0)) |\n-\t\t (chmod_arg && *chmod_arg == '+' ? ADD_CACHE_FORCE_EXECUTABLE : 0) |\n-\t\t (chmod_arg && *chmod_arg == '-' ? ADD_CACHE_FORCE_NOTEXECUTABLE : 0);\n+\t\t  ? ADD_CACHE_IGNORE_REMOVAL : 0));\n \n \tif (require_pathspec && argc == 0) {\n \t\tfprintf(stderr, _(\"Nothing specified, nothing added.\\n\"));\n@@ -436,10 +441,10 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \n \tplug_bulk_checkin();\n \n-\texit_status |= add_files_to_cache(prefix, &pathspec, flags);\n+\texit_status |= add_files_to_cache(prefix, &pathspec, flags, force_mode);\n \n \tif (add_new_files)\n-\t\texit_status |= add_files(&dir, flags);\n+\t\texit_status |= add_files(&dir, flags, force_mode);\n \n \tunplug_bulk_checkin();\n \ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 3398c61..c3486bd 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -548,7 +548,7 @@ static int merge_working_tree(const struct checkout_opts *opts,\n \t\t\t * entries in the index.\n \t\t\t */\n \n-\t\t\tadd_files_to_cache(NULL, NULL, 0);\n+\t\t\tadd_files_to_cache(NULL, NULL, 0, 0);\n \t\t\t/*\n \t\t\t * NEEDSWORK: carrying over local changes\n \t\t\t * when branches have different end-of-line\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 443ff91..163dbca 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -386,7 +386,7 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix\n \t */\n \tif (all || (also && pathspec.nr)) {\n \t\thold_locked_index(&index_lock, 1);\n-\t\tadd_files_to_cache(also ? prefix : NULL, &pathspec, 0);\n+\t\tadd_files_to_cache(also ? prefix : NULL, &pathspec, 0, 0);\n \t\trefresh_cache_or_die(refresh_flags);\n \t\tupdate_main_cache_tree(WRITE_TREE_SILENT);\n \t\tif (write_locked_index(&the_index, &index_lock, CLOSE_LOCK))\ndiff --git a/cache.h b/cache.h\nindex da03cd9..c73becb 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -367,8 +367,8 @@ extern void free_name_hash(struct index_state *istate);\n #define rename_cache_entry_at(pos, new_name) rename_index_entry_at(&the_index, (pos), (new_name))\n #define remove_cache_entry_at(pos) remove_index_entry_at(&the_index, (pos))\n #define remove_file_from_cache(path) remove_file_from_index(&the_index, (path))\n-#define add_to_cache(path, st, flags) add_to_index(&the_index, (path), (st), (flags))\n-#define add_file_to_cache(path, flags) add_file_to_index(&the_index, (path), (flags))\n+#define add_to_cache(path, st, flags) add_to_index(&the_index, (path), (st), (flags), 0)\n+#define add_file_to_cache(path, flags) add_file_to_index(&the_index, (path), (flags), 0)\n #define refresh_cache(flags) refresh_index(&the_index, (flags), NULL, NULL, NULL)\n #define ce_match_stat(ce, st, options) ie_match_stat(&the_index, (ce), (st), (options))\n #define ce_modified(ce, st, options) ie_modified(&the_index, (ce), (st), (options))\n@@ -581,10 +581,8 @@ extern int remove_file_from_index(struct index_state *, const char *path);\n #define ADD_CACHE_IGNORE_ERRORS\t4\n #define ADD_CACHE_IGNORE_REMOVAL 8\n #define ADD_CACHE_INTENT 16\n-#define ADD_CACHE_FORCE_EXECUTABLE 32\n-#define ADD_CACHE_FORCE_NOTEXECUTABLE 64\n-extern int add_to_index(struct index_state *, const char *path, struct stat *, int flags);\n-extern int add_file_to_index(struct index_state *, const char *path, int flags);\n+extern int add_to_index(struct index_state *, const char *path, struct stat *, int flags, int force_mode);\n+extern int add_file_to_index(struct index_state *, const char *path, int flags, int force_mode);\n extern struct cache_entry *make_cache_entry(unsigned int mode, const unsigned char *sha1, const char *path, int stage, unsigned int refresh_options);\n extern int ce_same_name(const struct cache_entry *a, const struct cache_entry *b);\n extern void set_object_name_for_intent_to_add_entry(struct cache_entry *ce);\n@@ -1774,7 +1772,7 @@ void packet_trace_identity(const char *prog);\n  * return 0 if success, 1 - if addition of a file failed and\n  * ADD_FILES_IGNORE_ERRORS was specified in flags\n  */\n-int add_files_to_cache(const char *prefix, const struct pathspec *pathspec, int flags);\n+int add_files_to_cache(const char *prefix, const struct pathspec *pathspec, int flags, int force_mode);\n \n /* diff.c */\n extern int diff_auto_refresh_index;\ndiff --git a/read-cache.c b/read-cache.c\nindex d12d143..db27766 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -630,7 +630,7 @@ void set_object_name_for_intent_to_add_entry(struct cache_entry *ce)\n \thashcpy(ce->sha1, sha1);\n }\n \n-int add_to_index(struct index_state *istate, const char *path, struct stat *st, int flags)\n+int add_to_index(struct index_state *istate, const char *path, struct stat *st, int flags, int force_mode)\n {\n \tint size, namelen, was_same;\n \tmode_t st_mode = st->st_mode;\n@@ -641,8 +641,6 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,\n \tint intent_only = flags & ADD_CACHE_INTENT;\n \tint add_option = (ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE|\n \t\t\t  (intent_only ? ADD_CACHE_NEW_ONLY : 0));\n-\tint force_executable = flags & ADD_CACHE_FORCE_EXECUTABLE;\n-\tint force_notexecutable = flags & ADD_CACHE_FORCE_NOTEXECUTABLE;\n \n \tif (!S_ISREG(st_mode) && !S_ISLNK(st_mode) && !S_ISDIR(st_mode))\n \t\treturn error(\"%s: can only add regular files, symbolic links or git-directories\", path);\n@@ -661,14 +659,11 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,\n \telse\n \t\tce->ce_flags |= CE_INTENT_TO_ADD;\n \n-\tif (S_ISREG(st_mode) && (force_executable || force_notexecutable)) {\n-\t\tif (force_executable)\n-\t\t\tce->ce_mode = create_ce_mode(0777);\n-\t\telse\n-\t\t\tce->ce_mode = create_ce_mode(0666);\n-\t} else if (trust_executable_bit && has_symlinks) {\n+\tif (S_ISREG(st_mode) && force_mode)\n+\t\tce->ce_mode = create_ce_mode(force_mode);\n+\telse if (trust_executable_bit && has_symlinks)\n \t\tce->ce_mode = create_ce_mode(st_mode);\n-\t} else {\n+\telse {\n \t\t/* If there is an existing entry, pick the mode bits and type\n \t\t * from it, otherwise assume unexecutable regular file.\n \t\t */\n@@ -727,12 +722,13 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,\n \treturn 0;\n }\n \n-int add_file_to_index(struct index_state *istate, const char *path, int flags)\n+int add_file_to_index(struct index_state *istate, const char *path,\n+\tint flags, int force_mode)\n {\n \tstruct stat st;\n \tif (lstat(path, &st))\n \t\tdie_errno(\"unable to stat '%s'\", path);\n-\treturn add_to_index(istate, path, &st, flags);\n+\treturn add_to_index(istate, path, &st, flags, force_mode);\n }\n \n struct cache_entry *make_cache_entry(unsigned int mode,\n"},{"id":"287995","messageId":"alpine.DEB.2.20.1606011217230.5761@virtualbox","threadId":"42497","inReplyTo":"alpine.DEB.2.20.1606010919360.5761@virtualbox","subject":"Re: [PATCH] add: add --chmod=+x / --chmod=-x options","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-06-01T10:19:12Z","receivedAt":"2016-06-01T10:19:12Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"On Wed, 1 Jun 2016, Johannes Schindelin wrote:\n\n> And here is an add-on patch (Ed, feel free to squash) that avoids those\n> two bits, and even saves one line overall:\n> \n> [...]\n\nAnd here is a link for developers who prefer to work with Git directly\n(as opposed to working with Git through a mailbox):\n\n\thttps://github.com/git/git/compare/dscho:force-chmod\n"},{"id":"288013","messageId":"xmqqwpm8zyot.fsf@gitster.mtv.corp.google.com","threadId":"42497","inReplyTo":"alpine.DEB.2.20.1606010919360.5761@virtualbox","subject":"Re: [PATCH] add: add --chmod=+x / --chmod=-x options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-01T16:00:34Z","receivedAt":"2016-06-01T16:00:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> Hi Junio & Ed,\n>\n> On Tue, 31 May 2016, Junio C Hamano wrote:\n>\n>> Edward Thomson <ethomson@edwardthomson.com> writes:\n>> \n>> > +static char *chmod_arg = NULL;\n>> > +\n>> \n>> I'll drop \" = NULL\", as it is our convention to let BSS take care of\n>> the zero initialization for global variables as much as possible.\n>> \n>> Other than that, I did not see anything objectionable in this round,\n>> but I did notice that you kept the \"this takes two bits out of 32\"\n>> Dscho mentioned--I do not have a strong preference either way, so\n>> I'll queue it (at least tentatively) on 'pu'.\n>\n> And here is an add-on patch (Ed, feel free to squash) that avoids those\n> two bits, and even saves one line overall:\n\nUnlike the \"something like this\" we saw earlier, this draws the\nboundary of responsibility between the caller and the API at a much\nmore sensible place.\n\nThe difference between the versions with and without this update\nessentially is that the original had two bits in flags (among 32\ntotal, 5 bits are currently used and the patch uses 2 more bits),\nand this instead changes the function signature of functions that\ntook \"flags\" so that lal of them take one additional word.  So in\nthat sense, because we have enough bits to use, this is not a great\nimprovement.\n\nHowever.\n\nThe filemode that we force is not two independent bits, but a\ntristate: do not force, force executable and force non-executable.\nAnd from that point of view, the two seemingly-independent bits\nlooked a bit strange, and I prefer the separation this update gives\nus slightly more for that reason.  I didn't think carefully about\nthe change to add_to_index(), but doing it this way _may_ make it\neasier for us to later extend it to force \"this is file on\nfilesystem but record it as a symlink\", in which case, even if we do\nnot plan to implement such an enhancement in a near future, I think\nthis is the right direction to go in.\n\nThanks.\n\n>\n> -- snipsnap --\n> diff --git a/builtin/add.c b/builtin/add.c\n> index 44b6c97..b1dddb4 100644\n> --- a/builtin/add.c\n> +++ b/builtin/add.c\n> @@ -26,7 +26,7 @@ static int patch_interactive, add_interactive, edit_interactive;\n>  static int take_worktree_changes;\n>  \n>  struct update_callback_data {\n> -\tint flags;\n> +\tint flags, force_mode;\n>  \tint add_errors;\n>  };\n>  \n> @@ -65,7 +65,8 @@ static void update_callback(struct diff_queue_struct *q,\n>  \t\t\tdie(_(\"unexpected diff status %c\"), p->status);\n>  \t\tcase DIFF_STATUS_MODIFIED:\n>  \t\tcase DIFF_STATUS_TYPE_CHANGED:\n> -\t\t\tif (add_file_to_index(&the_index, path, data->flags)) {\n> +\t\t\tif (add_file_to_index(&the_index, path,\n> +\t\t\t\t\tdata->flags, data->force_mode)) {\n>  \t\t\t\tif (!(data->flags & ADD_CACHE_IGNORE_ERRORS))\n>  \t\t\t\t\tdie(_(\"updating files failed\"));\n>  \t\t\t\tdata->add_errors++;\n> @@ -83,14 +84,15 @@ static void update_callback(struct diff_queue_struct *q,\n>  \t}\n>  }\n>  \n> -int add_files_to_cache(const char *prefix,\n> -\t\t       const struct pathspec *pathspec, int flags)\n> +int add_files_to_cache(const char *prefix, const struct pathspec *pathspec,\n> +\tint flags, int force_mode)\n>  {\n>  \tstruct update_callback_data data;\n>  \tstruct rev_info rev;\n>  \n>  \tmemset(&data, 0, sizeof(data));\n>  \tdata.flags = flags;\n> +\tdata.force_mode = force_mode;\n>  \n>  \tinit_revisions(&rev, prefix);\n>  \tsetup_revisions(0, NULL, &rev, NULL);\n> @@ -238,7 +240,7 @@ static int ignore_add_errors, intent_to_add, ignore_missing;\n>  static int addremove = ADDREMOVE_DEFAULT;\n>  static int addremove_explicit = -1; /* unspecified */\n>  \n> -static char *chmod_arg = NULL;\n> +static char *chmod_arg;\n>  \n>  static int ignore_removal_cb(const struct option *opt, const char *arg, int unset)\n>  {\n> @@ -279,7 +281,7 @@ static int add_config(const char *var, const char *value, void *cb)\n>  \treturn git_default_config(var, value, cb);\n>  }\n>  \n> -static int add_files(struct dir_struct *dir, int flags)\n> +static int add_files(struct dir_struct *dir, int flags, int force_mode)\n>  {\n>  \tint i, exit_status = 0;\n>  \n> @@ -292,7 +294,8 @@ static int add_files(struct dir_struct *dir, int flags)\n>  \t}\n>  \n>  \tfor (i = 0; i < dir->nr; i++)\n> -\t\tif (add_file_to_cache(dir->entries[i]->name, flags)) {\n> +\t\tif (add_file_to_index(&the_index, dir->entries[i]->name,\n> +\t\t\t\tflags, force_mode)) {\n>  \t\t\tif (!ignore_add_errors)\n>  \t\t\t\tdie(_(\"adding files failed\"));\n>  \t\t\texit_status = 1;\n> @@ -305,7 +308,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n>  \tint exit_status = 0;\n>  \tstruct pathspec pathspec;\n>  \tstruct dir_struct dir;\n> -\tint flags;\n> +\tint flags, force_mode;\n>  \tint add_new_files;\n>  \tint require_pathspec;\n>  \tchar *seen = NULL;\n> @@ -339,10 +342,14 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n>  \tif (!show_only && ignore_missing)\n>  \t\tdie(_(\"Option --ignore-missing can only be used together with --dry-run\"));\n>  \n> -\tif (chmod_arg) {\n> -\t\tif (strcmp(chmod_arg, \"-x\") && strcmp(chmod_arg, \"+x\"))\n> -\t\t\tdie(_(\"--chmod param must be either -x or +x\"));\n> -\t}\n> +\tif (!chmod_arg)\n> +\t\tforce_mode = 0;\n> +\telse if (!strcmp(chmod_arg, \"-x\"))\n> +\t\tforce_mode = 0666;\n> +\telse if (!strcmp(chmod_arg, \"+x\"))\n> +\t\tforce_mode = 0777;\n> +\telse\n> +\t\tdie(_(\"--chmod param '%s' must be either -x or +x\"), chmod_arg);\n>  \n>  \tadd_new_files = !take_worktree_changes && !refresh_only;\n>  \trequire_pathspec = !(take_worktree_changes || (0 < addremove_explicit));\n> @@ -354,9 +361,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n>  \t\t (intent_to_add ? ADD_CACHE_INTENT : 0) |\n>  \t\t (ignore_add_errors ? ADD_CACHE_IGNORE_ERRORS : 0) |\n>  \t\t (!(addremove || take_worktree_changes)\n> -\t\t  ? ADD_CACHE_IGNORE_REMOVAL : 0)) |\n> -\t\t (chmod_arg && *chmod_arg == '+' ? ADD_CACHE_FORCE_EXECUTABLE : 0) |\n> -\t\t (chmod_arg && *chmod_arg == '-' ? ADD_CACHE_FORCE_NOTEXECUTABLE : 0);\n> +\t\t  ? ADD_CACHE_IGNORE_REMOVAL : 0));\n>  \n>  \tif (require_pathspec && argc == 0) {\n>  \t\tfprintf(stderr, _(\"Nothing specified, nothing added.\\n\"));\n> @@ -436,10 +441,10 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n>  \n>  \tplug_bulk_checkin();\n>  \n> -\texit_status |= add_files_to_cache(prefix, &pathspec, flags);\n> +\texit_status |= add_files_to_cache(prefix, &pathspec, flags, force_mode);\n>  \n>  \tif (add_new_files)\n> -\t\texit_status |= add_files(&dir, flags);\n> +\t\texit_status |= add_files(&dir, flags, force_mode);\n>  \n>  \tunplug_bulk_checkin();\n>  \n> diff --git a/builtin/checkout.c b/builtin/checkout.c\n> index 3398c61..c3486bd 100644\n> --- a/builtin/checkout.c\n> +++ b/builtin/checkout.c\n> @@ -548,7 +548,7 @@ static int merge_working_tree(const struct checkout_opts *opts,\n>  \t\t\t * entries in the index.\n>  \t\t\t */\n>  \n> -\t\t\tadd_files_to_cache(NULL, NULL, 0);\n> +\t\t\tadd_files_to_cache(NULL, NULL, 0, 0);\n>  \t\t\t/*\n>  \t\t\t * NEEDSWORK: carrying over local changes\n>  \t\t\t * when branches have different end-of-line\n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index 443ff91..163dbca 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -386,7 +386,7 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix\n>  \t */\n>  \tif (all || (also && pathspec.nr)) {\n>  \t\thold_locked_index(&index_lock, 1);\n> -\t\tadd_files_to_cache(also ? prefix : NULL, &pathspec, 0);\n> +\t\tadd_files_to_cache(also ? prefix : NULL, &pathspec, 0, 0);\n>  \t\trefresh_cache_or_die(refresh_flags);\n>  \t\tupdate_main_cache_tree(WRITE_TREE_SILENT);\n>  \t\tif (write_locked_index(&the_index, &index_lock, CLOSE_LOCK))\n> diff --git a/cache.h b/cache.h\n> index da03cd9..c73becb 100644\n> --- a/cache.h\n> +++ b/cache.h\n> @@ -367,8 +367,8 @@ extern void free_name_hash(struct index_state *istate);\n>  #define rename_cache_entry_at(pos, new_name) rename_index_entry_at(&the_index, (pos), (new_name))\n>  #define remove_cache_entry_at(pos) remove_index_entry_at(&the_index, (pos))\n>  #define remove_file_from_cache(path) remove_file_from_index(&the_index, (path))\n> -#define add_to_cache(path, st, flags) add_to_index(&the_index, (path), (st), (flags))\n> -#define add_file_to_cache(path, flags) add_file_to_index(&the_index, (path), (flags))\n> +#define add_to_cache(path, st, flags) add_to_index(&the_index, (path), (st), (flags), 0)\n> +#define add_file_to_cache(path, flags) add_file_to_index(&the_index, (path), (flags), 0)\n>  #define refresh_cache(flags) refresh_index(&the_index, (flags), NULL, NULL, NULL)\n>  #define ce_match_stat(ce, st, options) ie_match_stat(&the_index, (ce), (st), (options))\n>  #define ce_modified(ce, st, options) ie_modified(&the_index, (ce), (st), (options))\n> @@ -581,10 +581,8 @@ extern int remove_file_from_index(struct index_state *, const char *path);\n>  #define ADD_CACHE_IGNORE_ERRORS\t4\n>  #define ADD_CACHE_IGNORE_REMOVAL 8\n>  #define ADD_CACHE_INTENT 16\n> -#define ADD_CACHE_FORCE_EXECUTABLE 32\n> -#define ADD_CACHE_FORCE_NOTEXECUTABLE 64\n> -extern int add_to_index(struct index_state *, const char *path, struct stat *, int flags);\n> -extern int add_file_to_index(struct index_state *, const char *path, int flags);\n> +extern int add_to_index(struct index_state *, const char *path, struct stat *, int flags, int force_mode);\n> +extern int add_file_to_index(struct index_state *, const char *path, int flags, int force_mode);\n>  extern struct cache_entry *make_cache_entry(unsigned int mode, const unsigned char *sha1, const char *path, int stage, unsigned int refresh_options);\n>  extern int ce_same_name(const struct cache_entry *a, const struct cache_entry *b);\n>  extern void set_object_name_for_intent_to_add_entry(struct cache_entry *ce);\n> @@ -1774,7 +1772,7 @@ void packet_trace_identity(const char *prog);\n>   * return 0 if success, 1 - if addition of a file failed and\n>   * ADD_FILES_IGNORE_ERRORS was specified in flags\n>   */\n> -int add_files_to_cache(const char *prefix, const struct pathspec *pathspec, int flags);\n> +int add_files_to_cache(const char *prefix, const struct pathspec *pathspec, int flags, int force_mode);\n>  \n>  /* diff.c */\n>  extern int diff_auto_refresh_index;\n> diff --git a/read-cache.c b/read-cache.c\n> index d12d143..db27766 100644\n> --- a/read-cache.c\n> +++ b/read-cache.c\n> @@ -630,7 +630,7 @@ void set_object_name_for_intent_to_add_entry(struct cache_entry *ce)\n>  \thashcpy(ce->sha1, sha1);\n>  }\n>  \n> -int add_to_index(struct index_state *istate, const char *path, struct stat *st, int flags)\n> +int add_to_index(struct index_state *istate, const char *path, struct stat *st, int flags, int force_mode)\n>  {\n>  \tint size, namelen, was_same;\n>  \tmode_t st_mode = st->st_mode;\n> @@ -641,8 +641,6 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,\n>  \tint intent_only = flags & ADD_CACHE_INTENT;\n>  \tint add_option = (ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE|\n>  \t\t\t  (intent_only ? ADD_CACHE_NEW_ONLY : 0));\n> -\tint force_executable = flags & ADD_CACHE_FORCE_EXECUTABLE;\n> -\tint force_notexecutable = flags & ADD_CACHE_FORCE_NOTEXECUTABLE;\n>  \n>  \tif (!S_ISREG(st_mode) && !S_ISLNK(st_mode) && !S_ISDIR(st_mode))\n>  \t\treturn error(\"%s: can only add regular files, symbolic links or git-directories\", path);\n> @@ -661,14 +659,11 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,\n>  \telse\n>  \t\tce->ce_flags |= CE_INTENT_TO_ADD;\n>  \n> -\tif (S_ISREG(st_mode) && (force_executable || force_notexecutable)) {\n> -\t\tif (force_executable)\n> -\t\t\tce->ce_mode = create_ce_mode(0777);\n> -\t\telse\n> -\t\t\tce->ce_mode = create_ce_mode(0666);\n> -\t} else if (trust_executable_bit && has_symlinks) {\n> +\tif (S_ISREG(st_mode) && force_mode)\n> +\t\tce->ce_mode = create_ce_mode(force_mode);\n> +\telse if (trust_executable_bit && has_symlinks)\n>  \t\tce->ce_mode = create_ce_mode(st_mode);\n> -\t} else {\n> +\telse {\n>  \t\t/* If there is an existing entry, pick the mode bits and type\n>  \t\t * from it, otherwise assume unexecutable regular file.\n>  \t\t */\n> @@ -727,12 +722,13 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,\n>  \treturn 0;\n>  }\n>  \n> -int add_file_to_index(struct index_state *istate, const char *path, int flags)\n> +int add_file_to_index(struct index_state *istate, const char *path,\n> +\tint flags, int force_mode)\n>  {\n>  \tstruct stat st;\n>  \tif (lstat(path, &st))\n>  \t\tdie_errno(\"unable to stat '%s'\", path);\n> -\treturn add_to_index(istate, path, &st, flags);\n> +\treturn add_to_index(istate, path, &st, flags, force_mode);\n>  }\n>  \n>  struct cache_entry *make_cache_entry(unsigned int mode,\n"},{"id":"288669","messageId":"20160607225923.GA66447@zoidberg","threadId":"42497","inReplyTo":"xmqqwpm8zyot.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] add: add --chmod=+x / --chmod=-x options","fromName":"Edward Thomson","fromEmail":"ethomson@edwardthomson.com","sentAt":"2016-06-07T22:59:23Z","receivedAt":"2016-06-07T22:59:23Z","isPatch":true,"sender":{"key":"ethomson@edwardthomson.com","avatar":"https://avatars.githubusercontent.com/u/1130014?v=4"},"body":"On Wed, Jun 01, 2016 at 09:00:34AM -0700, Junio C Hamano wrote:\n> \n> Unlike the \"something like this\" we saw earlier, this draws the\n> boundary of responsibility between the caller and the API at a much\n> more sensible place.\n\nThis makes sense to me - Junio, are you taking (or have you already\ntaken) dscho's patch, or would you like me to squash it and resend?\n\nThanks-\n-ed\n"},{"id":"288673","messageId":"xmqq4m94o6nn.fsf@gitster.mtv.corp.google.com","threadId":"42497","inReplyTo":"20160607225923.GA66447@zoidberg","subject":"Re: [PATCH] add: add --chmod=+x / --chmod=-x options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-08T00:39:40Z","receivedAt":"2016-06-08T00:39:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Edward Thomson <ethomson@edwardthomson.com> writes:\n\n> On Wed, Jun 01, 2016 at 09:00:34AM -0700, Junio C Hamano wrote:\n>> \n>> Unlike the \"something like this\" we saw earlier, this draws the\n>> boundary of responsibility between the caller and the API at a much\n>> more sensible place.\n>\n> This makes sense to me - Junio, are you taking (or have you already\n> taken) dscho's patch, or would you like me to squash it and resend?\n\nI didn't plan to unilaterally squash them into one patch without\nhearing from you, so I haven't--Dscho's fix-up is queued directly\non top, ready to be squashed in.\n\nSo either is fine by me, either you send a final version to replace\nthe two patches on et/add-chmod-x topic, or you just tell me to go\nahead.\n\nWell, you practically said the latter already, so I'll do the\nsquashing.  Thanks.\n"},{"id":"288704","messageId":"CACsJy8DC_8LkZUCe=NY6wbg0+jTtwQTPE1zM1da9Vq8-PfyrsA@mail.gmail.com","threadId":"42497","inReplyTo":"20160531220818.GB46739@zoidberg","subject":"Re: [PATCH] add: add --chmod=+x / --chmod=-x options","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-06-08T11:46:14Z","receivedAt":"2016-06-08T11:46:14Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Jun 1, 2016 at 5:08 AM, Edward Thomson\n<ethomson@edwardthomson.com> wrote:\n> @@ -263,6 +265,7 @@ static struct option builtin_add_options[] = {\n>         OPT_BOOL( 0 , \"refresh\", &refresh_only, N_(\"don't add, only refresh the index\")),\n>         OPT_BOOL( 0 , \"ignore-errors\", &ignore_add_errors, N_(\"just skip files which cannot be added because of errors\")),\n>         OPT_BOOL( 0 , \"ignore-missing\", &ignore_missing, N_(\"check if - even missing - files are ignored in dry run\")),\n> +       OPT_STRING( 0 , \"chmod\", &chmod_arg, N_(\"(+/-)x\"), N_(\"override the executable bit of the listed files\")),\n\nIf this is only about +/-x, would --[no-]executable be a better option name?\n-- \nDuy\n"}]}