Git development
 help / color / mirror / Atom feed
* Re: What's cooking in git.git (Sep 2016, #03; Fri, 9)
From: Jeff King @ 2016-09-12 21:46 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqlgyx2aui.fsf@gitster.mtv.corp.google.com>

On Mon, Sep 12, 2016 at 12:10:13PM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > I happened to notice today that this topic needs a minor tweak:
> >
> > -- >8 --
> > Subject: [PATCH] add_delta_base_cache: use list_for_each_safe
> >
> > We may remove elements from the list while we are iterating,
> > which requires using a second temporary pointer. Otherwise
> > stepping to the next element of the list might involve
> > looking at freed memory (which generally works in practice,
> > as we _just_ freed it, but of course is wrong to rely on;
> > valgrind notices it).
> 
> I failed to notice it, too.  Thanks.

After staring at this, I was wondering how the _original_ ever worked.
Because the problem is in the linked-list code, which I did not really
change (I switched it to LIST_HEAD(), but the code is equivalent).

The answer is that in the original, there was no free() in the original
code when we released an entry; it just went back to the (static) pool.

So the bug is in the conversion to hashmap, where we start allocating
(and freeing) the entries individually.

-Peff

^ permalink raw reply

* Re: [PATCH v2 17/25] sequencer: support amending commits
From: Junio C Hamano @ 2016-09-12 21:36 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Jakub Narębski, Johannes Sixt
In-Reply-To: <68f995eaaad2448f5793f0030f96aa501b985ed5.1473590966.git.johannes.schindelin@gmx.de>

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> This teaches the sequencer_commit() function to take an argument that
> will allow us to implement "todo" commands that need to amend the commit
> messages ("fixup", "squash" and "reword").
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  sequencer.c | 6 ++++--
>  sequencer.h | 2 +-
>  2 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/sequencer.c b/sequencer.c
> index 6e9732c..60b522e 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -485,7 +485,7 @@ static char **read_author_script(void)
>   * author metadata.
>   */
>  int sequencer_commit(const char *defmsg, struct replay_opts *opts,
> -			  int allow_empty, int edit)
> +			  int allow_empty, int edit, int amend)
>  {
>  	char **env = NULL;
>  	struct argv_array array;
> @@ -514,6 +514,8 @@ int sequencer_commit(const char *defmsg, struct replay_opts *opts,
>  	argv_array_push(&array, "commit");
>  	argv_array_push(&array, "-n");
>  
> +	if (amend)
> +		argv_array_push(&array, "--amend");
>  	if (opts->gpg_sign)
>  		argv_array_pushf(&array, "-S%s", opts->gpg_sign);
>  	if (opts->signoff)
> @@ -786,7 +788,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
>  	}
>  	if (!opts->no_commit)
>  		res = sequencer_commit(opts->edit ? NULL : git_path_merge_msg(),
> -			opts, allow, opts->edit);
> +			opts, allow, opts->edit, 0);
>  
>  leave:
>  	free_message(commit, &msg);

Hmm, this is more about a comment on 18/25, but I suspect that
"amend" or any opportunity given to the user to futz with the
contents in the editor invites a wish for the result to be treated
with stripspace.

No existing callers use "amend" to call this function, so there is
no change in behaviour, but at the same time, we do not have enough
information to see if 'amend' should by default toggle cleanup.


> diff --git a/sequencer.h b/sequencer.h
> index 7f5222f..c45f5c4 100644
> --- a/sequencer.h
> +++ b/sequencer.h
> @@ -54,7 +54,7 @@ int sequencer_rollback(struct replay_opts *opts);
>  int sequencer_remove_state(struct replay_opts *opts);
>  
>  int sequencer_commit(const char *defmsg, struct replay_opts *opts,
> -			  int allow_empty, int edit);
> +			  int allow_empty, int edit, int amend);
>  
>  extern const char sign_off_header[];

^ permalink raw reply

* Re: [PATCH v2 18/25] sequencer: support cleaning up commit messages
From: Junio C Hamano @ 2016-09-12 21:33 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Jakub Narębski, Johannes Sixt
In-Reply-To: <773ba280e64c2dfabff0d6e6a0a0808482461677.1473590966.git.johannes.schindelin@gmx.de>

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> @@ -788,7 +792,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
>  	}
>  	if (!opts->no_commit)
>  		res = sequencer_commit(opts->edit ? NULL : git_path_merge_msg(),
> -			opts, allow, opts->edit, 0);
> +			opts, allow, opts->edit, 0, 0);
>  
>  leave:
>  	free_message(commit, &msg);

So the existing caller did not ask to cleanup


>  int sequencer_commit(const char *defmsg, struct replay_opts *opts,
> -			  int allow_empty, int edit, int amend)
> +			  int allow_empty, int edit, int amend,
> +			  int cleanup_commit_message)
>  {
>  	char **env = NULL;
>  	struct argv_array array;
> @@ -522,9 +523,12 @@ int sequencer_commit(const char *defmsg, struct replay_opts *opts,
>  		argv_array_push(&array, "-s");
>  	if (defmsg)
>  		argv_array_pushl(&array, "-F", defmsg, NULL);
> +	if (cleanup_commit_message)
> +		argv_array_push(&array, "--cleanup=strip");
>  	if (edit)
>  		argv_array_push(&array, "-e");
> -	else if (!opts->signoff && !opts->record_origin &&
> +	else if (!cleanup_commit_message &&
> +		 !opts->signoff && !opts->record_origin &&
>  		 git_config_get_value("commit.cleanup", &value))
>  		argv_array_push(&array, "--cleanup=verbatim");

Which is consistent with this change.  This is a no-op enhancement
for existing callers and makes new callers to ask for cleaning up.

We will see if a hardcoded "--cleanup=strip" is sufficient when we
see new callers (we _know_ it would be sufficient for the first new
caller by definition ;-).

Makes sense.  Thanks.

^ permalink raw reply

* Re: [PATCH v2 19/25] sequencer: remember do_recursive_merge()'s return value
From: Junio C Hamano @ 2016-09-12 21:23 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Jakub Narębski, Johannes Sixt
In-Reply-To: <44c455710fd9c420a3f759d021c4864f3a83c97a.1473590966.git.johannes.schindelin@gmx.de>

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> The return value of do_recursive_merge() may be positive (indicating merge
> conflicts), so let's OR later error conditions so as not to overwrite them
> with 0.

Are the untold assumptions as follows?

 - The caller wants to act on positive (not quite an error), zero
   (success) or negative (error);

 - do_recursive_merge() can return positive (success with
   reservation), zero or negative, and the call to it would return
   immediately if it got negative;

 - all other functions that come later may return either zero or negative, and 
   never positive;

 - Hence the caller can be assured that "res" being positive can
   only mean do_recursive_merge() succeeded with reservation and
   everything else succeeded.

This can be extended if the only thing the caller cares about is
positive/zero/negative and it does not care what exact positive
value it gets--in such a case, we can loosen the condition on the
return values from other functions whose return values are OR'ed
together; they may also return positive to signal the same "not
quite an error", i.e. updating the latter two points to

 - all other functions that come later can return positive (success
   with reservation), zero or negative.

 - Hence the caller can be assured that "res" being positive can
   mean nobody failed with negative return, but it is not an
   unconditional success, which is signalled by value "res" being
   0.

I cannot quite tell which is the case, especially what is planned in
the future.  The proposed log message is a good place to explain the
future course this code will take.

> This is not yet a problem, but preparing for the patches to come: we will
> teach the sequencer to do rebase -i's job.

Thanks.

^ permalink raw reply

* Re: Git Miniconference at Plumbers
From: Jakub Narębski @ 2016-09-12 21:23 UTC (permalink / raw)
  To: Jon Loeliger, David Bainbridge; +Cc: Jeff King, git@vger.kernel.org
In-Reply-To: <E1bjVfp-0006sG-89@mylo.jdl.com>

W dniu 12.09.2016 o 20:09, Jon Loeliger napisał:
> So, like, David Bainbridge said:

>> Does anyone know whether the sessions will be recorded in any way?
> 
> I am uncertain about outright recording (digital video/audio),
> but there will be at least summarizing notes taken and posted.
> Anyone wishing to record the talks/discussions is likely welcome
> to do so.

I think LWN.net usually covers Linux Plumbers Conference, see e.g.
https://lwn.net/Archives/ConferenceByYear/#2015-Linux_Plumbers_Conference

Hopefully they would cover it this year too.

HTH
-- 
Jakub Narębski


^ permalink raw reply

* Re: git commit -p with file arguments
From: Jakub Narębski @ 2016-09-12 21:14 UTC (permalink / raw)
  To: Junio C Hamano, Jacob Keller; +Cc: Christian Neukirchen, Git mailing list
In-Reply-To: <xmqqa8fd6fs7.fsf@gitster.mtv.corp.google.com>

W dniu 12.09.2016 o 03:57, Junio C Hamano pisze:
> Jacob Keller <jacob.keller@gmail.com> writes:
> 
>> Yes, I'm actually confused by "git commit <files>" *not* usinng what's
>> in the index already, so I think that isn't intuitive as is.
> 
> You are excused ;-)
> 
> In ancient days, "git commit <pathspec>" was to add the contents
> from working tree files that match <pathspec> to what is already in
> the index and create a commit from that state.

That is, "git commit <pathspec>" meant --include, being equivalent to
"git commit --include <pathspec>".

>                                               This ran against the
> intuition of many users who knew older systems (e.g. cvs) and we had
> to migrate it to the current behaviour by breaking backward
> compatibility.

That is, "git commit <pathspec>" means --only, being equivalent to
"git commit --only <pathspec>".

But it was always about working tree version of <pathspec>.

And of course older version control systems didn't have the index...
-- 
Jakub Narębski


^ permalink raw reply

* [PATCH v3 4/4] add: modify already added files when --chmod is given
From: Thomas Gummerer @ 2016-09-12 21:08 UTC (permalink / raw)
  To: git
  Cc: Johannes Schindelin, Jeff King, Jan Keromnes, Ingo Brückl,
	Edward Thomson, Junio C Hamano, Thomas Gummerer
In-Reply-To: <20160912210818.26282-1-t.gummerer@gmail.com>

When the chmod option was added to git add, it was hooked up to the diff
machinery, meaning that it only works when the version in the index
differs from the version on disk.

As the option was supposed to mirror the chmod option in update-index,
which always changes the mode in the index, regardless of the status of
the file, make sure the option behaves the same way in git add.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 builtin/add.c      | 36 +++++++++++++++++++++++++-----------
 builtin/checkout.c |  2 +-
 builtin/commit.c   |  2 +-
 cache.h            | 10 +++++-----
 read-cache.c       | 14 ++++++--------
 t/t3700-add.sh     | 21 +++++++++++++++++++++
 6 files changed, 59 insertions(+), 26 deletions(-)

diff --git a/builtin/add.c b/builtin/add.c
index b1dddb4..892198a 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -26,10 +26,25 @@ static int patch_interactive, add_interactive, edit_interactive;
 static int take_worktree_changes;
 
 struct update_callback_data {
-	int flags, force_mode;
+	int flags;
 	int add_errors;
 };
 
+static void chmod_pathspec(struct pathspec *pathspec, int force_mode)
+{
+	int i;
+	
+	for (i = 0; i < active_nr; i++) {
+		struct cache_entry *ce = active_cache[i];
+
+		if (pathspec && !ce_path_match(ce, pathspec, NULL))
+			continue;
+
+		if (chmod_cache_entry(ce, force_mode) < 0)
+			fprintf(stderr, "cannot chmod '%s'", ce->name);
+	}
+}
+
 static int fix_unmerged_status(struct diff_filepair *p,
 			       struct update_callback_data *data)
 {
@@ -65,8 +80,7 @@ static void update_callback(struct diff_queue_struct *q,
 			die(_("unexpected diff status %c"), p->status);
 		case DIFF_STATUS_MODIFIED:
 		case DIFF_STATUS_TYPE_CHANGED:
-			if (add_file_to_index(&the_index, path,
-					data->flags, data->force_mode)) {
+			if (add_file_to_index(&the_index, path,	data->flags)) {
 				if (!(data->flags & ADD_CACHE_IGNORE_ERRORS))
 					die(_("updating files failed"));
 				data->add_errors++;
@@ -84,15 +98,14 @@ static void update_callback(struct diff_queue_struct *q,
 	}
 }
 
-int add_files_to_cache(const char *prefix, const struct pathspec *pathspec,
-	int flags, int force_mode)
+int add_files_to_cache(const char *prefix,
+		       const struct pathspec *pathspec, int flags)
 {
 	struct update_callback_data data;
 	struct rev_info rev;
 
 	memset(&data, 0, sizeof(data));
 	data.flags = flags;
-	data.force_mode = force_mode;
 
 	init_revisions(&rev, prefix);
 	setup_revisions(0, NULL, &rev, NULL);
@@ -281,7 +294,7 @@ static int add_config(const char *var, const char *value, void *cb)
 	return git_default_config(var, value, cb);
 }
 
-static int add_files(struct dir_struct *dir, int flags, int force_mode)
+static int add_files(struct dir_struct *dir, int flags)
 {
 	int i, exit_status = 0;
 
@@ -294,8 +307,7 @@ static int add_files(struct dir_struct *dir, int flags, int force_mode)
 	}
 
 	for (i = 0; i < dir->nr; i++)
-		if (add_file_to_index(&the_index, dir->entries[i]->name,
-				flags, force_mode)) {
+		if (add_file_to_index(&the_index, dir->entries[i]->name, flags)) {
 			if (!ignore_add_errors)
 				die(_("adding files failed"));
 			exit_status = 1;
@@ -441,11 +453,13 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 
 	plug_bulk_checkin();
 
-	exit_status |= add_files_to_cache(prefix, &pathspec, flags, force_mode);
+	exit_status |= add_files_to_cache(prefix, &pathspec, flags);
 
 	if (add_new_files)
-		exit_status |= add_files(&dir, flags, force_mode);
+		exit_status |= add_files(&dir, flags);
 
+	if (force_mode)
+		chmod_pathspec(&pathspec, force_mode);
 	unplug_bulk_checkin();
 
 finish:
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 8672d07..a83c78f 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -548,7 +548,7 @@ static int merge_working_tree(const struct checkout_opts *opts,
 			 * entries in the index.
 			 */
 
-			add_files_to_cache(NULL, NULL, 0, 0);
+			add_files_to_cache(NULL, NULL, 0);
 			/*
 			 * NEEDSWORK: carrying over local changes
 			 * when branches have different end-of-line
diff --git a/builtin/commit.c b/builtin/commit.c
index 77e3dc8..7a1ade0 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -387,7 +387,7 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
 	 */
 	if (all || (also && pathspec.nr)) {
 		hold_locked_index(&index_lock, 1);
-		add_files_to_cache(also ? prefix : NULL, &pathspec, 0, 0);
+		add_files_to_cache(also ? prefix : NULL, &pathspec, 0);
 		refresh_cache_or_die(refresh_flags);
 		update_main_cache_tree(WRITE_TREE_SILENT);
 		if (write_locked_index(&the_index, &index_lock, CLOSE_LOCK))
diff --git a/cache.h b/cache.h
index 44a4f76..eb6d6b8 100644
--- a/cache.h
+++ b/cache.h
@@ -367,8 +367,8 @@ extern void free_name_hash(struct index_state *istate);
 #define rename_cache_entry_at(pos, new_name) rename_index_entry_at(&the_index, (pos), (new_name))
 #define remove_cache_entry_at(pos) remove_index_entry_at(&the_index, (pos))
 #define remove_file_from_cache(path) remove_file_from_index(&the_index, (path))
-#define add_to_cache(path, st, flags) add_to_index(&the_index, (path), (st), (flags), 0)
-#define add_file_to_cache(path, flags) add_file_to_index(&the_index, (path), (flags), 0)
+#define add_to_cache(path, st, flags) add_to_index(&the_index, (path), (st), (flags))
+#define add_file_to_cache(path, flags) add_file_to_index(&the_index, (path), (flags))
 #define chmod_cache_entry(ce, force_mode) chmod_index_entry(&the_index, (ce), (force_mode))
 #define refresh_cache(flags) refresh_index(&the_index, (flags), NULL, NULL, NULL)
 #define ce_match_stat(ce, st, options) ie_match_stat(&the_index, (ce), (st), (options))
@@ -582,8 +582,8 @@ extern int remove_file_from_index(struct index_state *, const char *path);
 #define ADD_CACHE_IGNORE_ERRORS	4
 #define ADD_CACHE_IGNORE_REMOVAL 8
 #define ADD_CACHE_INTENT 16
-extern int add_to_index(struct index_state *, const char *path, struct stat *, int flags, int force_mode);
-extern int add_file_to_index(struct index_state *, const char *path, int flags, int force_mode);
+extern int add_to_index(struct index_state *, const char *path, struct stat *, int flags);
+extern int add_file_to_index(struct index_state *, const char *path, int flags);
 extern struct cache_entry *make_cache_entry(unsigned int mode, const unsigned char *sha1, const char *path, int stage, unsigned int refresh_options);
 extern int chmod_index_entry(struct index_state *, struct cache_entry *ce, int force_mode);
 extern int ce_same_name(const struct cache_entry *a, const struct cache_entry *b);
@@ -1820,7 +1820,7 @@ void packet_trace_identity(const char *prog);
  * return 0 if success, 1 - if addition of a file failed and
  * ADD_FILES_IGNORE_ERRORS was specified in flags
  */
-int add_files_to_cache(const char *prefix, const struct pathspec *pathspec, int flags, int force_mode);
+int add_files_to_cache(const char *prefix, const struct pathspec *pathspec, int flags);
 
 /* diff.c */
 extern int diff_auto_refresh_index;
diff --git a/read-cache.c b/read-cache.c
index 367be57..b92d72e 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -627,7 +627,7 @@ void set_object_name_for_intent_to_add_entry(struct cache_entry *ce)
 	hashcpy(ce->sha1, sha1);
 }
 
-int add_to_index(struct index_state *istate, const char *path, struct stat *st, int flags, int force_mode)
+int add_to_index(struct index_state *istate, const char *path, struct stat *st, int flags)
 {
 	int size, namelen, was_same;
 	mode_t st_mode = st->st_mode;
@@ -656,11 +656,10 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
 	else
 		ce->ce_flags |= CE_INTENT_TO_ADD;
 
-	if (S_ISREG(st_mode) && force_mode)
-		ce->ce_mode = create_ce_mode(force_mode);
-	else if (trust_executable_bit && has_symlinks)
+
+	if (trust_executable_bit && has_symlinks) {
 		ce->ce_mode = create_ce_mode(st_mode);
-	else {
+	} else {
 		/* If there is an existing entry, pick the mode bits and type
 		 * from it, otherwise assume unexecutable regular file.
 		 */
@@ -719,13 +718,12 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
 	return 0;
 }
 
-int add_file_to_index(struct index_state *istate, const char *path,
-	int flags, int force_mode)
+int add_file_to_index(struct index_state *istate, const char *path, int flags)
 {
 	struct stat st;
 	if (lstat(path, &st))
 		die_errno("unable to stat '%s'", path);
-	return add_to_index(istate, path, &st, flags, force_mode);
+	return add_to_index(istate, path, &st, flags);
 }
 
 struct cache_entry *make_cache_entry(unsigned int mode,
diff --git a/t/t3700-add.sh b/t/t3700-add.sh
index 2978cb9..62c7f71 100755
--- a/t/t3700-add.sh
+++ b/t/t3700-add.sh
@@ -349,4 +349,25 @@ test_expect_success POSIXPERM,SYMLINKS 'git add --chmod=+x with symlinks' '
 	test_mode_in_index 100755 foo2
 '
 
+test_expect_success 'git add --chmod=[+-]x changes index with already added file' '
+	echo foo >foo3 &&
+	git add foo3 &&
+	git add --chmod=+x foo3 &&
+	test_mode_in_index 100755 foo3 &&
+	echo foo >xfoo3 &&
+	chmod 755 xfoo3 &&
+	git add xfoo3 &&
+	git add --chmod=-x xfoo3 &&
+	test_mode_in_index 100644 xfoo3
+'
+
+test_expect_success 'file status is changed after git add --chmod=+x' '
+	echo "AM foo4" >expected &&
+	echo foo >foo4 &&
+	git add foo4 &&
+	git add --chmod=+x foo4 &&
+	git status -s foo4 >actual &&
+	test_cmp expected actual
+'
+
 test_done
-- 
2.10.0.304.gf2ff484


^ permalink raw reply related

* [PATCH v3 3/4] read-cache: introduce chmod_index_entry
From: Thomas Gummerer @ 2016-09-12 21:08 UTC (permalink / raw)
  To: git
  Cc: Johannes Schindelin, Jeff King, Jan Keromnes, Ingo Brückl,
	Edward Thomson, Junio C Hamano, Thomas Gummerer
In-Reply-To: <20160912210818.26282-1-t.gummerer@gmail.com>

As there are chmod options for both add and update-index, introduce a
new chmod_index_entry function to do the work.  Use it in update-index,
while it will be used in add in the next patch.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 builtin/update-index.c |  8 +-------
 cache.h                |  2 ++
 read-cache.c           | 19 +++++++++++++++++++
 3 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/builtin/update-index.c b/builtin/update-index.c
index 6d6cddd..809ce97 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -423,20 +423,14 @@ static void chmod_path(int force_mode, const char *path)
 {
 	int pos;
 	struct cache_entry *ce;
-	unsigned int mode;
 	char flip = force_mode == 0777 ? '+' : '-';
 
 	pos = cache_name_pos(path, strlen(path));
 	if (pos < 0)
 		goto fail;
 	ce = active_cache[pos];
-	mode = ce->ce_mode;
-	if (!S_ISREG(mode))
+	if (chmod_cache_entry(ce, force_mode) < 0)
 		goto fail;
-	ce->ce_mode = create_ce_mode(force_mode);
-	cache_tree_invalidate_path(&the_index, path);
-	ce->ce_flags |= CE_UPDATE_IN_BASE;
-	active_cache_changed |= CE_ENTRY_CHANGED;
 
 	report("chmod %cx '%s'", flip, path);
 	return;
diff --git a/cache.h b/cache.h
index b780a91..44a4f76 100644
--- a/cache.h
+++ b/cache.h
@@ -369,6 +369,7 @@ extern void free_name_hash(struct index_state *istate);
 #define remove_file_from_cache(path) remove_file_from_index(&the_index, (path))
 #define add_to_cache(path, st, flags) add_to_index(&the_index, (path), (st), (flags), 0)
 #define add_file_to_cache(path, flags) add_file_to_index(&the_index, (path), (flags), 0)
+#define chmod_cache_entry(ce, force_mode) chmod_index_entry(&the_index, (ce), (force_mode))
 #define refresh_cache(flags) refresh_index(&the_index, (flags), NULL, NULL, NULL)
 #define ce_match_stat(ce, st, options) ie_match_stat(&the_index, (ce), (st), (options))
 #define ce_modified(ce, st, options) ie_modified(&the_index, (ce), (st), (options))
@@ -584,6 +585,7 @@ extern int remove_file_from_index(struct index_state *, const char *path);
 extern int add_to_index(struct index_state *, const char *path, struct stat *, int flags, int force_mode);
 extern int add_file_to_index(struct index_state *, const char *path, int flags, int force_mode);
 extern struct cache_entry *make_cache_entry(unsigned int mode, const unsigned char *sha1, const char *path, int stage, unsigned int refresh_options);
+extern int chmod_index_entry(struct index_state *, struct cache_entry *ce, int force_mode);
 extern int ce_same_name(const struct cache_entry *a, const struct cache_entry *b);
 extern void set_object_name_for_intent_to_add_entry(struct cache_entry *ce);
 extern int index_name_is_other(const struct index_state *, const char *, int);
diff --git a/read-cache.c b/read-cache.c
index 491e52d..367be57 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -756,6 +756,25 @@ struct cache_entry *make_cache_entry(unsigned int mode,
 	return ret;
 }
 
+/*
+ * Change the mode of an index entry to force_mode, where force_mode can
+ * either be 0777 or 0666.
+ * Returns -1 if the chmod for the particular cache entry failed (if it's
+ * not a regular file), 0 otherwise.
+ */
+int chmod_index_entry(struct index_state *istate, struct cache_entry *ce,
+		       int force_mode)
+{
+	if (!S_ISREG(ce->ce_mode))
+		return -1;
+	ce->ce_mode = create_ce_mode(force_mode);
+	cache_tree_invalidate_path(istate, ce->name);
+	ce->ce_flags |= CE_UPDATE_IN_BASE;
+	istate->cache_changed |= CE_ENTRY_CHANGED;
+
+	return 0;
+}
+
 int ce_same_name(const struct cache_entry *a, const struct cache_entry *b)
 {
 	int len = ce_namelen(a);
-- 
2.10.0.304.gf2ff484


^ permalink raw reply related

* [PATCH v3 2/4] update-index: use the same structure for chmod as add
From: Thomas Gummerer @ 2016-09-12 21:08 UTC (permalink / raw)
  To: git
  Cc: Johannes Schindelin, Jeff King, Jan Keromnes, Ingo Brückl,
	Edward Thomson, Junio C Hamano, Thomas Gummerer
In-Reply-To: <20160912210818.26282-1-t.gummerer@gmail.com>

While the chmod options for update-index and the add have the same
functionality, they are using different ways to parse and handle the
option internally.  Unify these modes in order to make further
refactoring simpler.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 builtin/update-index.c        | 39 +++++++++++++++++++--------------------
 t/t2107-update-index-basic.sh | 13 +++++++++++++
 2 files changed, 32 insertions(+), 20 deletions(-)

diff --git a/builtin/update-index.c b/builtin/update-index.c
index ba04b19..6d6cddd 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -419,11 +419,12 @@ static int add_cacheinfo(unsigned int mode, const unsigned char *sha1,
 	return 0;
 }
 
-static void chmod_path(int flip, const char *path)
+static void chmod_path(int force_mode, const char *path)
 {
 	int pos;
 	struct cache_entry *ce;
 	unsigned int mode;
+	char flip = force_mode == 0777 ? '+' : '-';
 
 	pos = cache_name_pos(path, strlen(path));
 	if (pos < 0)
@@ -432,17 +433,11 @@ static void chmod_path(int flip, const char *path)
 	mode = ce->ce_mode;
 	if (!S_ISREG(mode))
 		goto fail;
-	switch (flip) {
-	case '+':
-		ce->ce_mode |= 0111; break;
-	case '-':
-		ce->ce_mode &= ~0111; break;
-	default:
-		goto fail;
-	}
+	ce->ce_mode = create_ce_mode(force_mode);
 	cache_tree_invalidate_path(&the_index, path);
 	ce->ce_flags |= CE_UPDATE_IN_BASE;
 	active_cache_changed |= CE_ENTRY_CHANGED;
+
 	report("chmod %cx '%s'", flip, path);
 	return;
  fail:
@@ -789,12 +784,16 @@ static int really_refresh_callback(const struct option *opt,
 }
 
 static int chmod_callback(const struct option *opt,
-				const char *arg, int unset)
+			  const char *arg, int unset)
 {
-	char *flip = opt->value;
-	if ((arg[0] != '-' && arg[0] != '+') || arg[1] != 'x' || arg[2])
-		return error("option 'chmod' expects \"+x\" or \"-x\"");
-	*flip = arg[0];
+	int *force_mode = opt->value;
+	if (!strcmp(arg, "-x"))
+		*force_mode = 0666;
+	else if (!strcmp(arg, "+x"))
+		*force_mode = 0777;
+	else
+		die(_("option 'chmod' expects \"+x\" or \"-x\""));
+
 	return 0;
 }
 
@@ -917,7 +916,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
 	int read_from_stdin = 0;
 	int prefix_length = prefix ? strlen(prefix) : 0;
 	int preferred_index_format = 0;
-	char set_executable_bit = 0;
+	int force_mode = 0;
 	struct refresh_params refresh_args = {0, &has_errors};
 	int lock_error = 0;
 	int split_index = -1;
@@ -955,7 +954,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
 			PARSE_OPT_NOARG | /* disallow --cacheinfo=<mode> form */
 			PARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP,
 			(parse_opt_cb *) cacheinfo_callback},
-		{OPTION_CALLBACK, 0, "chmod", &set_executable_bit, N_("(+/-)x"),
+		{OPTION_CALLBACK, 0, "chmod", &force_mode, N_("(+/-)x"),
 			N_("override the executable bit of the listed files"),
 			PARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP,
 			chmod_callback},
@@ -1055,8 +1054,8 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
 			setup_work_tree();
 			p = prefix_path(prefix, prefix_length, path);
 			update_one(p);
-			if (set_executable_bit)
-				chmod_path(set_executable_bit, p);
+			if (force_mode)
+				chmod_path(force_mode, p);
 			free(p);
 			ctx.argc--;
 			ctx.argv++;
@@ -1100,8 +1099,8 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
 			}
 			p = prefix_path(prefix, prefix_length, buf.buf);
 			update_one(p);
-			if (set_executable_bit)
-				chmod_path(set_executable_bit, p);
+			if (force_mode)
+				chmod_path(force_mode, p);
 			free(p);
 		}
 		strbuf_release(&unquoted);
diff --git a/t/t2107-update-index-basic.sh b/t/t2107-update-index-basic.sh
index dfe02f4..32ac6e0 100755
--- a/t/t2107-update-index-basic.sh
+++ b/t/t2107-update-index-basic.sh
@@ -80,4 +80,17 @@ test_expect_success '.lock files cleaned up' '
 	)
 '
 
+test_expect_success '--chmod=+x and chmod=-x in the same argument list' '
+	>A &&
+	>B &&
+	git add A B &&
+	git update-index --chmod=+x A --chmod=-x B &&
+	cat >expect <<-\EOF &&
+	100755 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0	A
+	100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0	B
+	EOF
+	git ls-files --stage A B >actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
2.10.0.304.gf2ff484


^ permalink raw reply related

* [PATCH v3 1/4] add: document the chmod option
From: Thomas Gummerer @ 2016-09-12 21:08 UTC (permalink / raw)
  To: git
  Cc: Johannes Schindelin, Jeff King, Jan Keromnes, Ingo Brückl,
	Edward Thomson, Junio C Hamano, Thomas Gummerer
In-Reply-To: <20160912210818.26282-1-t.gummerer@gmail.com>

The git add --chmod option was introduced in 4e55ed3 ("add: add
--chmod=+x / --chmod=-x options", 2016-05-31), but was never
documented.  Document the feature.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 Documentation/git-add.txt | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index 6a96a66..7ed63dc 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -11,7 +11,7 @@ SYNOPSIS
 'git add' [--verbose | -v] [--dry-run | -n] [--force | -f] [--interactive | -i] [--patch | -p]
 	  [--edit | -e] [--[no-]all | --[no-]ignore-removal | [--update | -u]]
 	  [--intent-to-add | -N] [--refresh] [--ignore-errors] [--ignore-missing]
-	  [--] [<pathspec>...]
+	  [--chmod=(+|-)x] [--] [<pathspec>...]
 
 DESCRIPTION
 -----------
@@ -165,6 +165,11 @@ for "git add --no-all <pathspec>...", i.e. ignored removed files.
 	be ignored, no matter if they are already present in the work
 	tree or not.
 
+--chmod=(+|-)x::
+	Override the executable bit of the added files.  The executable
+	bit is only changed in the index, the files on disk are left
+	unchanged.
+
 \--::
 	This option can be used to separate command-line options from
 	the list of files, (useful when filenames might be mistaken
-- 
2.10.0.304.gf2ff484


^ permalink raw reply related

* [PATCH v3 0/4] git add --chmod: always change the file
From: Thomas Gummerer @ 2016-09-12 21:08 UTC (permalink / raw)
  To: git
  Cc: Johannes Schindelin, Jeff King, Jan Keromnes, Ingo Brückl,
	Edward Thomson, Junio C Hamano, Thomas Gummerer
In-Reply-To: <20160911103028.5492-1-t.gummerer@gmail.com>

Thanks to Junio on setting me straight on the change of behaviour in
the previous round.

This round includes a similar change, which does however not change
the behaviour of update-index with repeated arguments.  I still think
the unification of the way git add and git update-index handle chmod
is useful, when we can keep the behaviour with multiple arguments in
update-index the same.

Thomas Gummerer (4):
  add: document the chmod option
  update-index: use the same structure for chmod as add
  read-cache: introduce chmod_index_entry
  add: modify already added files when --chmod is given

 Documentation/git-add.txt     |  7 ++++++-
 builtin/add.c                 | 36 +++++++++++++++++++++++-----------
 builtin/checkout.c            |  2 +-
 builtin/commit.c              |  2 +-
 builtin/update-index.c        | 45 ++++++++++++++++++-------------------------
 cache.h                       | 12 +++++++-----
 read-cache.c                  | 33 +++++++++++++++++++++++--------
 t/t2107-update-index-basic.sh | 13 +++++++++++++
 t/t3700-add.sh                | 21 ++++++++++++++++++++
 9 files changed, 118 insertions(+), 53 deletions(-)

-- 
2.10.0.304.gf2ff484


^ permalink raw reply

* Re: [PATCH v2 12/14] i18n: show-branch: mark error messages for translation
From: Junio C Hamano @ 2016-09-12 20:46 UTC (permalink / raw)
  To: Jean-Noël AVILA
  Cc: Vasco Almeida, git, Jiang Xin,
	Ævar Arnfjörð Bjarmason
In-Reply-To: <17140652.xHVhzLXte8@cayenne>

Jean-Noël AVILA <jn.avila@free.fr> writes:

> ..., and 
> plural forms can be quite different depending on its value.

AHHhhh, of course you are right.

^ permalink raw reply

* Re: [PATCH v3 0/2] patch-id for merges
From: Junio C Hamano @ 2016-09-12 20:44 UTC (permalink / raw)
  To: Jeff King
  Cc: git, Michael Haggerty, Kevin Willford, Xiaolong Ye,
	Johannes Schindelin, Josh Triplett
In-Reply-To: <20160912175641.nj4h6oupbwaedwbx@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Mon, Sep 12, 2016 at 10:18:33AM -0700, Junio C Hamano wrote:
>
>> > +static int patch_id_defined(struct commit *commit)
>> > +{
>> > +	/* must be 0 or 1 parents */
>> > +	return !commit->parents || !commit->parents->next;
>> > +}
>> 
>> If we make the first hunk begin like so:
>> 
>> > +	if (commit->parents) {
>> > +		if (!patch_id_defined(commit))
>> > +			return -1;
>> 
>> I wonder if the compiler gives us the same code.
>
> Good idea. I actually put the "patch_id_defined" check outside the "if"
> block you've quoted (otherwise we're making assumptions about the
> contents of patch_id_defined).

Facepalm.  I was mis-reading the condition in the helper function.
Of course, guarding up-front makes more sense.


^ permalink raw reply

* Re: [PATCH v2] checkout: eliminate unnecessary merge for trivial checkout
From: Junio C Hamano @ 2016-09-12 20:31 UTC (permalink / raw)
  To: Ben Peart; +Cc: git, pclouds, 'Ben Peart'
In-Reply-To: <13ef001d20d21$2d2ea840$878bf8c0$@gmail.com>

"Ben Peart" <peartben@gmail.com> writes:

> I completely agree that optimizing within merge_working_tree would provide 
> more opportunities for optimization.  I can certainly move the test into
> that function as a first step.

Note that "optimizing more" was not the primary point of my
response.

Quite honestly, I'd rather see us speed up _ONLY_ obviously correct
and commonly used cases, while leaving most cases that _MAY_ turn
out to be optimizable (if we did careful analysis) unoptimized, and
instead have them handled by generic but known to be correct
codepath, if it means we do NOT to have to spend mental bandwidth to
analyze not-common case--that is a much better tradeoff.

The suggestion to move the check one level down in the callchain was
primarily to avoid the proposed optimization from being overly eager
and ending up skipping necessary parts of what merge_working_tree()
does (e.g. like I suspected in the review that the proposed patch
skips the check for "you have unmerged entries" situation).

^ permalink raw reply

* Re: build issues on AIX - aka non-GNU environment, no gnu grep, no gcc
From: Junio C Hamano @ 2016-09-12 20:24 UTC (permalink / raw)
  To: Michael Felt; +Cc: git
In-Reply-To: <CANvxniXkbAKgjm+NZ0cyyCToEYp23Kd8s4yxSqUOsAUAHJSA7g@mail.gmail.com>

Michael Felt <aixtools@gmail.com> writes:

> I had a couple of issues when packaging git for AIX
> a) option -Wall by default - works fine with gcc I am sure, but not so
> well when gcc is not your compiler

That is expected.  As you said, it is merely "by default" and there
are mechanisms like config.mak provided for people to override them.

> b) needs a special (GNU) grep argument (-a from memory). This I
> resolved by downloading and packaging GNU grep. However, I hope this
> has not introduced a new dependency.

Read the Makefile and find SANE_TEXT_GREP, perhaps?

Thanks for helping to spread Git to minority platforms.

^ permalink raw reply

* Re: Git Miniconference at Plumbers
From: Junio C Hamano @ 2016-09-12 20:11 UTC (permalink / raw)
  To: Jon Loeliger; +Cc: David Bainbridge, Jeff King, git@vger.kernel.org
In-Reply-To: <E1bjVfp-0006sG-89@mylo.jdl.com>

Jon Loeliger <jdl@jdl.com> writes:

> So, like, David Bainbridge said:
>> Hi,
>> 
>> The subject matter of the conference looks really interesting but I am
>> unlikely to be able to attend, unfortunately.
>> 
>> The subjects being covered like the current State of Git and the
>> Future of Git, for example, deserve much wider exposure, and I would
>> certainly appreciate hearing the thoughts of Junio and others.
>
> Indeed.

You do not need to go to NM to _hear_ that.  Basically, I want us
not to have "big" plans that come from the top.

Now, you heard it ;-)

There are areas that we as Git community would want to address for
some audience that were discovered over the years, and that "some
audience" might even be a large population of Git users, but if that
does not have overlap with Kernel Plumbers, the Plumbers mini-conf
may not be a suitable venue for even mentioning them.  E.g. the
enhancement of the submodule subsystem to allow more end-user facing
commands to recurse into them; rearchitecting the index and redoing
the "sparse checkout" hack so that we can do narrow clones more
properly; supporting "huge objects" better in the object layer,
without having to resort to ugly hacks like GitLFS that will never
be part of the core Git.  These things may all be worth talking
about in wider Git setting, but some of them may be waste of time to
bring up in the Plumbers' venue.

The future of Git is shaped largely by end-user itches.  From my
point of view, Git people are going there primarily to find what
Kernel Plubmbers' itches are, and help assessing the workflow
improvements around Git the Plumbers are wishing for or designing
themselves by being there, because we are at the best position to
tell what kind of enhancement to Git is feasible and what is
unlikely to happen in the near term.

^ permalink raw reply

* git-am includes escape characters from 'From' field
From: Swift Geek @ 2016-09-12 20:10 UTC (permalink / raw)
  To: git

git-am seems to add backslash that escapes double quote character, example  
git format-patch

 From 63da989a5295214f9bd06cd7b409a86a65241eea Mon Sep 17 00:00:00 2001
From: "Sebastian \"Swift Geek\" Grzywna" <swiftgeek@gmail.com>
Date: Mon, 12 Sep 2016 21:27:32 +0200
Subject: [PATCH] Example showing git-am bug that includes escape characters

---
  testfile | 1 +
  1 file changed, 1 insertion(+)
  create mode 100644 testfile

diff --git a/testfile b/testfile
new file mode 100644
index 0000000..b48e119
--- /dev/null
+++ b/testfile
@@ -0,0 +1 @@
+test file contents
-- 
2.9.3

In .gitconfig I have
name = Sebastian \"Swift Geek\" Grzywna
which appears to work fine with regular git commit - backslashes are not  
included in 'From' field.

Regards,
Swift Geek

^ permalink raw reply related

* Re: [PATCH v2 04/25] sequencer: future-proof remove_sequencer_state()
From: Junio C Hamano @ 2016-09-12 19:53 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Jakub Narębski, Johannes Sixt
In-Reply-To: <d6b92cd7aa56ceb515c50467a0e6030401e2e1bc.1473590966.git.johannes.schindelin@gmx.de>

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> +static const char *get_dir(const struct replay_opts *opts)
> +{
> +	return git_path_seq_dir();
> +}

Presumably this is what "In a couple of commits" meant, i.e. opts
will be used soonish.

> -static void remove_sequencer_state(void)
> +static void remove_sequencer_state(const struct replay_opts *opts)
>  {
> -	struct strbuf seq_dir = STRBUF_INIT;
> +	struct strbuf dir = STRBUF_INIT;
>  
> -	strbuf_addstr(&seq_dir, git_path_seq_dir());
> -	remove_dir_recursively(&seq_dir, 0);
> -	strbuf_release(&seq_dir);
> +	strbuf_addf(&dir, "%s", get_dir(opts));
> +	remove_dir_recursively(&dir, 0);
> +	strbuf_release(&dir);
>  }

As long as "who called the sequencer" is the only thing that
determines where the state is kept, this should work fine, I would
think.  I wondered that it would introduce a chicken-and-egg problem
if we had to support "git sequencer --clear-state" command ;-) but
that is not the case.

Good.

^ permalink raw reply

* [ANNOUNCE] Git User's Survey 2016
From: Jakub Narębski @ 2016-09-12 19:51 UTC (permalink / raw)
  To: git
  Cc: Doug Rathbone, David Bainbridge, Stefan Beller, Andrew Ardill,
	Eric Wong

Hello all,

We would like to ask you a few questions about your use of the Git
version control system. This survey is mainly to understand who is
using Git, how and why.

The results will be published to the Git wiki on the GitSurvey2016
page (https://git.wiki.kernel.org/index.php/GitSurvey2016) and
discussed on the git mailing list.


The survey would be open from 12 September to 20 October 2016.


Please devote a few minutes of your time to fill this simple
questionnaire, it will help a lot the git community to understand your
needs, what you like of Git, and of course what you don't like.

The survey can be found here:
  https://tinyurl.com/GitSurvey2016
  https://survs.com/survey/lmo7ed3439

There is also alternate version which does not require cookies,
but it doesn't allow one to go back to response and edit it.
  https://tinyurl.com/GitSurvey2016-anon
  https://survs.com/survey/naeec8kwd8


P.S. At request I can open a separate channel in survey, with a separate
survey URL, so that responses from particular site or organization could
be separated out.

Please send me a email with name of channel, and I will return with
a separate survey URL to use.  Note that the name of the channel would
be visible to others.

P.P.S. Different announcements use different URLs (different channels)
to better track where one got information about this survey.

Thanks in advance for taking time to answer the survey,
-- 
Jakub Narębski
on behalf of
Git Development Community

^ permalink raw reply

* Re: [PATCH v2 03/25] sequencer: avoid unnecessary indirection
From: Junio C Hamano @ 2016-09-12 19:49 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Jakub Narębski, Johannes Sixt
In-Reply-To: <2587881a45d5a81cda4b2eb40e2d7d8d55b7931c.1473590966.git.johannes.schindelin@gmx.de>

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> -static int read_populate_opts(struct replay_opts **opts)
> +static int read_populate_opts(struct replay_opts *opts)
>  {
>  	if (!file_exists(git_path_opts_file()))
>  		return 0;
> @@ -823,7 +823,7 @@ static int read_populate_opts(struct replay_opts **opts)
>  	 * about this case, though, because we wrote that file ourselves, so we
>  	 * are pretty certain that it is syntactically correct.
>  	 */
> -	if (git_config_from_file(populate_opts_cb, git_path_opts_file(), *opts) < 0)
> +	if (git_config_from_file(populate_opts_cb, git_path_opts_file(), opts) < 0)
>  		return error(_("Malformed options sheet: %s"),
>  			git_path_opts_file());
>  	return 0;
> @@ -1054,7 +1054,7 @@ static int sequencer_continue(struct replay_opts *opts)
>  
>  	if (!file_exists(git_path_todo_file()))
>  		return continue_single_pick();
> -	if (read_populate_opts(&opts) ||
> +	if (read_populate_opts(opts) ||
>  			read_populate_todo(&todo_list, opts))
>  		return -1;

Good!

^ permalink raw reply

* Re: [PATCH v2 02/25] sequencer: use memoized sequencer directory path
From: Junio C Hamano @ 2016-09-12 19:48 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Jakub Narębski, Johannes Sixt
In-Reply-To: <5c52dcbff8b21b24a360d6f5bd2710fe1139eb4a.1473590966.git.johannes.schindelin@gmx.de>

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  builtin/commit.c |  2 +-
>  sequencer.c      | 11 ++++++-----
>  sequencer.h      |  5 +----
>  3 files changed, 8 insertions(+), 10 deletions(-)

OK.  It's nice to see code reduction.

> -#define SEQ_DIR		"sequencer"
> -#define SEQ_HEAD_FILE	"sequencer/head"
> -#define SEQ_TODO_FILE	"sequencer/todo"
> -#define SEQ_OPTS_FILE	"sequencer/opts"
> +const char *git_path_seq_dir(void);


^ permalink raw reply

* Re: [PATCH v2 01/25] sequencer: use static initializers for replay_opts
From: Junio C Hamano @ 2016-09-12 19:46 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Jakub Narębski, Johannes Sixt
In-Reply-To: <666493d3f166d839deb817042ed7d1636b11f19e.1473590966.git.johannes.schindelin@gmx.de>

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> This change is not completely faithful: instead of initializing all fields
> to 0, we choose to initialize command and subcommand to -1 (instead of
> defaulting to REPLAY_REVERT and REPLAY_NONE, respectively). Practically,
> it makes no difference at all, but future-proofs the code to require
> explicit assignments for both fields.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---

OK.

^ permalink raw reply

* Re: [PATCH v2 05/25] sequencer: allow the sequencer to take custody of malloc()ed data
From: Junio C Hamano @ 2016-09-12 19:46 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Jakub Narębski, Johannes Sixt
In-Reply-To: <942aa559a0af9b52e079c5c78fa313f49b87d50d.1473590966.git.johannes.schindelin@gmx.de>

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> The sequencer is our attempt to lib-ify cherry-pick. Yet it behaves
> like a one-shot command when it reads its configuration: memory is
> allocated and released only when the command exits.
>
> This is kind of okay for git-cherry-pick, which *is* a one-shot
> command. All the work to make the sequencer its work horse was
> done to allow using the functionality as a library function, though,
> including proper clean-up after use.
>
> This patch introduces an API to pass the responsibility of releasing
> certain memory to the sequencer. Example:
>
> 	const char *label =
> 		sequencer_entrust(opts, xstrfmt("From: %s", email));

I thought we (not just me) were already pretty clear during the last
round of review that we will not want this entrust() thing.




^ permalink raw reply

* Re: Gitattributes file is not respected when switching between branches
From: Torsten Bögershausen @ 2016-09-12 19:42 UTC (permalink / raw)
  To: Виталий Ищенко,
	git
In-Reply-To: <8df2883f-ec3c-3446-2e06-207c93452332@web.de>

On 12.09.16 21:35, Torsten Bögershausen wrote:
> On 12.09.16 14:55, Виталий Ищенко wrote:
>> Good day
>>
>> I faced following issue with gitattributes file (at least eol setting)
>> when was trying to force `lf` mode on windows.
>>
>> We have 2 branches: master & dev. With master set as HEAD in repository
>>
>> I've added `.gitattributes` with following content to `dev` branch
>>
>> ```
>> * text eol=lf
>> ```
>>
>> Now when you clone this repo on other machine and checkout dev branch,
>> eol setting is not respected.
>> As a workaround you can rm all files except .git folder and do hard reset.
>>
>> Issue is reproducible on windows & unix versions. Test repo can be
>> found on github
>> https://github.com/betalb/gitattributes-issue
>>
>> master branch - one file without gitattributes
>> feature-branch - .gitattributes added with eol=lf
>> unix-feature-branch - .gitattributes added with eol=crlf
>>
>> Thanks,
>> Vitalii
> Some more information may be needed, to help to debug.
>
> Which version of Git are you using ?
> What does
>
> git ls-files --eol
>
> say ?
Obs, All information was in the email.

tb@xxx:/tmp/gitattributes-issue> git ls-files --eol
i/lf    w/lf    attr/                   testfile-crlf.txt
tb@xxx:/tmp/gitattributes-issue> ls -al
total 8
drwxr-xr-x   4 tb    wheel  136 Sep 12 21:38 .
drwxrwxrwt  19 root  wheel  646 Sep 12 21:38 ..
drwxr-xr-x  13 tb    wheel  442 Sep 12 21:38 .git
-rw-r--r--   1 tb    wheel   60 Sep 12 21:38 testfile-crlf.txt
tb@xxx:/tmp/gitattributes-issue>

Could it be that you didn't commit the file ".gitattributes" ?
This could help:
git add .gitattributes && git commit -m "Add .gitattributes"








^ permalink raw reply

* Re: [PATCH v2 0/5] Pull out require_clean_work_tree() functionality from builtin/pull.c
From: Junio C Hamano @ 2016-09-12 19:36 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <cover.1473580914.git.johannes.schindelin@gmx.de>

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> Johannes Schindelin (5):
>   pull: drop confusing prefix parameter of die_on_unclean_work_tree()
>   pull: make code more similar to the shell script again
>   Make the require_clean_work_tree() function truly reusable
>   Export also the has_un{staged,committed}_changed() functions
>   wt-status: teach has_{unstaged,uncommitted}_changes() about submodules

Other than two minor things I've already mentioned, this round looks
ready to be queued.  Thanks.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox