Re: [PATCH v3 01/13] refs: convert some internal functions to use object_id

[brian m. carlson]

On Tue, Oct 13, 2015 at 01:43:30PM +0200, Michael Haggerty wrote:
> On 10/09/2015 03:43 AM, brian m. carlson wrote:
> > Convert several internal functions in refs.c to use struct object_id,
> > and use the GIT_SHA1_HEXSZ constants in parse_ref_line.
> > 
> > Signed-off-by: brian m. carlson 
> > [...]
> 
> I looked over this patch at the diff level and didn't find any problems.
> 
> This patch conflicts heavily with [1]. But I noticed that it is
> independent of the rest of your series. I don't know when either patch
> series will be ready, but see [2] for my take on the other one.
> 
> Assuming neither series is rejected, I think it would be much easier to
> redo this patch on top of the first part of [1] than vice versa, so that
> would be my suggestion. If it comes down to that, I am willing to help
> redo this patch if you like.

I'll take a look at it over the next couple of days.  I'll probably drop
it from this series and either see if it can be put on top of the other
one, or defer it to a future series.  That way we can minimize
conflicts.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: PGP signature

Why does "git reset --hard" fail on file/folder conflicts?

[Christian Halstrick]

Hi,

git doesn't want to do a git reset hard when for a certain path the index
contains a tree, worktree contains a file and the commit to reset to
contains nothing. Is it a bug or is it intended? I would expect git to
simply delete that path from index and worktree.

> git init
Initialized empty Git repository in /tmp/y/.git/
> touch a
> git add a
> git commit -m addA
[master (root-commit) fa08136] addA
 1 file changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 a
> mkdir d
> touch d/f
> git add d/f
> rm -fr d
> touch d
> git reset --hard HEAD
warning: unable to unlink d/f: Not a directory
HEAD is now at fa08136 addA

Ciao
  Chris
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 02/44] refs: make repack_without_refs and is_branch public

[Michael Haggerty]

On 10/13/2015 04:39 AM, Michael Haggerty wrote:
> On 10/12/2015 11:51 PM, David Turner wrote:
>> is_branch was already non-static, but this patch declares it in the
>> header.
>>
>> Signed-off-by: Ronnie Sahlberg 
>> Signed-off-by: David Turner 
>> ---
>> [...]
> 
> It seems odd that repack_without_refs() should be made public (and
> ultimately end up in refs.h) given that it intrinsically only has to do
> with file-based references. But I will read on...

I think the reason you needed to do this was because you wanted to move
delete_refs() to the common code. It is true that delete_ref() can be
moved to the common code. And most of the code in delete_refs() is just
a loop calling delete_ref(). But delete_refs() also does the very
files-specific optimization of calling repack_without_refs() before the
loop. *That* call shouldn't be in the common code.

So my suggestion is that you write a common_delete_refs() function that
only includes the loop over delete_ref(), and a files_delete_refs()
function that is basically

{
result = repack_without_refs(refnames, );
if (result) {
...report error...
return result;
}
return common_delete_refs(...);
}

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Solaris: Fail to build git with CFLAGS=-m64

[evgeny litvinenko]

Hi.

Tried to build git-2.6.1 on Solaris (Oracle Solaris 11.2 and
OpenIndiana 151.1.9)
with CFLAGS="-m64" and got an error during make step:

GIT_VERSION = 2.6.1
* new build flags
gcc -o credential-store.o -c -MF ./.depend/credential-store.o.d -MQ
credential-store.o -MMD -MP  --save-temps -O2 -m64 -I.
-D__EXTENSIONS__ -D__sun__ -DHAVE_ALLOCA_H -DNO_D_TYPE_IN_DIRENT
-DNO_INET_NTOP -DNO_INET_PTON  -DHAVE_PATHS_H -DHAVE_STRINGS_H
-DHAVE_DEV_TTY -DHAVE_CLOCK_GETTIME -DHAVE_CLOCK_MONOTONIC
-DHAVE_GETDELIM -DSHA1_HEADER=''  -Icompat/regex
-DSHELL_PATH='"/bin/bash"'  credential-store.c
In file included from cache.h:4:0,
 from credential-store.c:1:
git-compat-util.h:689:13: error: conflicting types for inet_ntop
/usr/include/arpa/inet.h:43:20: note: previous declaration of inet_ntop was here
gmake: *** [credential-store.o] Error 1

Solaris has the following prototype in the file /usr/include/arpa/inet.h:

extern const char *inet_ntop(int, const void *_RESTRICT_KYWD, char
*_RESTRICT_KYWD, socklen_t);

Git's prototype for inet_ntop is in file git-compat-util.h:

#ifdef NO_INET_NTOP
const char *inet_ntop(int af, const void *src, char *dst, size_t size);
#endif

When build with -m64
typedefs for socklen_t

typedef unsigned int uint32_t;
typedef uint32_t socklen_t;

and typedefs for size_t

typedef unsigned long ulong_t;
typedef ulong_t size_t;

With -m32 both socklen_t and size_t are "unsigned int" and there is no
any errors.

Also Solaris has the functions inet_ntop and inet_pton in libnsl.so so
I did the following correction to configure.ac to build git with -m64:

diff --git a/configure.ac b/configure.ac
index 14012fa..4cf1929 100644
--- a/configure.ac
+++ b/configure.ac
@@ -637,6 +637,11 @@ AC_CHECK_FUNC([inet_ntop],
[NEEDS_RESOLV=YesPlease],
[NO_INET_NTOP=YesPlease])
 ])
+if test "x$ac_cv_func_inet_ntop" != xyes; then
+AC_CHECK_LIB([nsl], [inet_ntop],
+   [NEEDS_NSL=YesPlease; NO_INET_NTOP=],
+   [])
+fi
 GIT_CONF_SUBST([NO_INET_NTOP])
 #
 # Define NO_INET_PTON if linking with -lresolv is not enough.
@@ -648,6 +653,11 @@ AC_CHECK_FUNC([inet_pton],
[NEEDS_RESOLV=YesPlease],
[NO_INET_PTON=YesPlease])
 ])
+if test "x$ac_cv_func_inet_pton" != xyes; then
+AC_CHECK_LIB([nsl], [inet_pton],
+   [NEEDS_NSL=YesPlease; NO_INET_PTON=],
+   [])
+fi
 GIT_CONF_SUBST([NO_INET_PTON])
 #
 # Define NO_HSTRERROR if linking with -lresolv is not enough.

Is it possible to change prototype for inet_ntop in git-compat-util.h
or use Solaris's inet_ntop and inet_pton
?

Evgeny
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Solaris: Fail to build git with CFLAGS=-m64

[Junio C Hamano]

evgeny litvinenko  writes:

> Solaris has the following prototype in the file /usr/include/arpa/inet.h:
>
> extern const char *inet_ntop(int, const void *_RESTRICT_KYWD, char
> *_RESTRICT_KYWD, socklen_t);
>
> Git's prototype for inet_ntop is in file git-compat-util.h:
>
> #ifdef NO_INET_NTOP
> const char *inet_ntop(int af, const void *src, char *dst, size_t size);
> #endif

This tells me that it is designed to be used only when the platform
does not offer inet_ntop().  If your platform does have it, why is
your build define NO_INET_NTOP?

Perhaps configure generated by autoconf is faulty?

In this project, use of autoconf/configure is optional---in other
words, it is expected that you can build successfully by setting and
unsetting necessary Makefile macros in your own config.mak without
using autoconf/configure at all.  I'd try without configure and make
sure I do not define NO_INET_NTOP (and if you have inet_pton(), then
make sure you do not define NO_INET_PTON either) in config.mak if I
were you.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Add fetch.recurseSubmoduleParallelism config option

[Junio C Hamano]

Stefan Beller  writes:

>> The parallel_process API could learn a new "verbose" feature that it
>> by itself shows some messages like
>>
>> "processing the 'frotz' job with N tasks"
>> "M tasks finished (N still running)"
>
> I know what to fill in for M and N, 'frotz' is a bit unclear to me.

At least I don't know what M and N should be, and I'm curious how
you'll define them.  See below.

>> in the output stream from strategic places.  For example, the first
>> message will come at the end of pp_init(), and the second message
>> will be appended at the end of buffered output of a task that has
>> just been finished.  Once you have something like that, you could
>> check for them in a test in t/.
>>
>> Just a thought.
>
> I like that thought. :)


A few more random thoughts:

 * The only thing you could rely on if you were to use the above in
   your tests is the one from pp_init() that declares how many
   processes the machinery is going to use.  M/N will be unstable,
   depending on the scheduling order (e.g. the foreground process
   may take a lot of time to finish, while many other processes
   finish first).

 * Every time the foreground process (i.e. the one whose output is
   tee-ed to the overall output from the machinery) finishes, you
   can emit "M tasks finished (N still running)", but I am not sure
   what M should be.  It is debatable how to account for background
   processes that have already completed but whose output haven't
   been shown.

   One school of thought that is in line with the "pretend as if the
   background tasks are started immediately after the foreground
   task finishes, and they run at infinite speed and produce output
   in no time" idea, on which the "queue output from the background
   processes and emit at once in order to avoid intermixing" design
   was based on, would be not to include them in M (i.e. finished
   ones), because their output haven't been emitted and we are
   pretending that they haven't even been started.  If you take this
   approach, you however may have to include them in N (i.e. still
   running), but that would likely bump N beyond the maximum number
   of simultaneous processes.

   The other school of thought would of course tell the truth and
   include the number of finished background processes in M, as they
   have finished already in the reality.  This will not risk showing
   N that is beyond the maximum, but your first "progress" output
   might say "3 tasks finished", which will make it look odd in a
   different way.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 25/44] refs.h: document make refname_is_safe and add it to header

[Michael Haggerty]

On 10/12/2015 11:51 PM, David Turner wrote:
> This function might be used by other refs backends
> 
> Signed-off-by: David Turner 
> ---
>  refs.h | 11 +++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/refs.h b/refs.h
> index fc8a748..7a936e2 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -348,6 +348,17 @@ int verify_refname_available(const char *newname, struct 
> string_list *extra,
>struct string_list *skip, struct strbuf *err);
>  
>  /*
> + * Check if a refname is safe.
> + * For refs that start with "refs/" we consider it safe as long they do
> + * not try to resolve to outside of refs/.
> + *
> + * For all other refs we only consider them safe iff they only contain
> + * upper case characters and '_' (like "HEAD" AND "MERGE_HEAD", and not like
> + * "config").
> + */
> +int refname_is_safe(const char *refname);
> +
> +/*
>   * Flags controlling ref_transaction_update(), ref_transaction_create(), etc.
>   * REF_NODEREF: act on the ref directly, instead of dereferencing
>   *  symbolic references.
> 

The previous commit deleted this comment from where it previously
appeared in refs-be-files.c. It would make more sense to squash this
commit onto that one so it's clear that you are moving the comment
rather than creating a new comment out of thin air.

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 20/44] refs-be-files.c: add methods for misc ref operations

[Michael Haggerty]

On 10/12/2015 11:51 PM, David Turner wrote:
> From: Ronnie Sahlberg 
> 
> Add ref backend methods for:
> resolve_ref_unsafe, verify_refname_available, pack_refs, peel_ref,
> create_symref, resolve_gitlink_ref.
> 
> Signed-off-by: Ronnie Sahlberg 
> Signed-off-by: David Turner 
> ---
>  builtin/init-db.c |  1 +
>  cache.h   |  7 +++
>  refs-be-files.c   | 36 +++-
>  refs.c| 47 +++
>  refs.h| 18 ++
>  5 files changed, 100 insertions(+), 9 deletions(-)

This commit doesn't build:

refs-be-files.c:3636:2: error: initialization from incompatible pointer
type [-Werror]
  files_create_symref,
  ^
refs-be-files.c:3636:2: error: (near initialization for
‘refs_be_files.create_symref’) [-Werror]

Apparently one of the hunks from the subsequent commit is needed in this
commit.

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 19/44] refs-be-files.c: add a backend method structure with transaction functions

[Michael Haggerty]

On 10/12/2015 11:51 PM, David Turner wrote:
> From: Ronnie Sahlberg 
> 
> Add a ref structure for backend methods. Start by adding a method pointer
> for the transaction commit function.
> 
> Add a function set_refs_backend to switch between backends. The files
> based backend is the default.
> 
> Signed-off-by: Ronnie Sahlberg 
> Signed-off-by: David Turner 
> ---
>  refs-be-files.c | 10 --
>  refs.c  | 30 ++
>  refs.h  | 15 +++
>  3 files changed, 53 insertions(+), 2 deletions(-)
> 
> [...]
> diff --git a/refs.h b/refs.h
> index 4940ae9..419abf4 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -619,4 +619,19 @@ extern int reflog_expire(const char *refname, const 
> unsigned char *sha1,
>reflog_expiry_cleanup_fn cleanup_fn,
>void *policy_cb_data);
>  
> +/* refs backends */
> +typedef int ref_transaction_commit_fn(struct ref_transaction *transaction,
> +   struct strbuf *err);
> +typedef void ref_transaction_free_fn(struct ref_transaction *transaction);

The ref_transaction_free_fn typedef isn't used anywhere.

> +struct ref_be {
> + struct ref_be *next;
> + const char *name;
> + ref_transaction_commit_fn *transaction_commit;
> +};
> +
> +
> +extern struct ref_be refs_be_files;

I don't think that refs_be_files is needed in the public interface.

> +int set_refs_backend(const char *name);
> +
>  #endif /* REFS_H */

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: How to rebase when some commit hashes are in some commit messages

[Francois-Xavier Le Bail]

On 12/10/2015 22:21, Matthieu Moy wrote:
> Francois-Xavier Le Bail  writes:
> 
>> Hello,
>>
>> [I try some search engines without success, perhaps I have missed something].
>>
>> For example, if I rebase the following commits, I would want that if
>> the commit hash 222... become 777...,
>> the message
>> "Update test output for "
>> become
>> "Update test output for 777..."
>>
>> Is it possible currently? And if yes how?
> 
> AFAIK, it's not possible other than by editing the message by hand.

It seems to me useful to be able to do it. Can we hope a new option?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 37/44] refs: break out a ref conflict check

[Michael Haggerty]

On 10/12/2015 11:51 PM, David Turner wrote:
> Create new function verify_no_descendants, to hold one of the ref
> conflict checks used in verify_refname_available.  Multiple backends
> will need this function, so it goes in the common code.
> 
> Signed-off-by: David Turner 
> ---
>  refs-be-files.c | 33 -
>  refs.c  | 22 ++
>  refs.h  |  7 +++
>  3 files changed, 37 insertions(+), 25 deletions(-)
> 
> [...]
> diff --git a/refs.c b/refs.c
> index 5a3125d..3ae0274 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -1101,6 +1101,28 @@ enum peel_status peel_object(const unsigned char 
> *name, unsigned char *sha1)
>   return PEEL_PEELED;
>  }
>  
> +const char *find_descendant_ref(const char *refname,
> + const struct string_list *extras,
> + const struct string_list *skip)
> +{
> + int pos;
> + if (!extras)
> + return NULL;
> +
> + /* Look for the place where "$refname/" would be inserted in extras. */

The comment above is misleading. See my note at the bottom of this email
for an explanation.

> + for (pos = string_list_find_insert_index(extras, refname, 0);
> +  pos < extras->nr; pos++) {
> + const char *extra_refname = extras->items[pos].string;
> +
> + if (!starts_with(extra_refname, refname))
> + break;
> +
> + if (!skip || !string_list_has_string(skip, extra_refname))
> + return extra_refname;
> + }
> + return NULL;
> +}
> +
>  /* backend functions */
>  int refs_initdb(struct strbuf *err, int shared)
>  {
> diff --git a/refs.h b/refs.h
> index 3aad3b8..f8becea 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -637,6 +637,13 @@ int files_log_ref_write(const char *refname, const 
> unsigned char *old_sha1,
>   const unsigned char *new_sha1, const char *msg,
>   int flags, struct strbuf *err);
>  
> +/*
> + * Check for entries in extras that start with "$refname/", ignoring
> + * those in skip. If there is such an entry, then we have a conflict.
> + */
> +const char *find_descendant_ref(const char *refname,
> + const struct string_list *extras,
> + const struct string_list *skip);

The documentation is is not correct. As written, the function needs not
the refname as argument but the refname followed by '/'. Without the
trailing slash, "refs/heads/foo" would be blocked by "refs/heads/foobar".

>From the point of view of simplicity, it would be nice if this function
could take a refname (without the trailing slash) as argument. But then
it would basically be forced to allocate a new string, copy refname to
it, append a '/', then free the string again when it's done.
Alternatively, it could accept its refname argument in a strbuf and
temporarily append the '/'.

But neither one of these alternatives is so elegant, so maybe it's OK if
this function is documented to take a "directory name, including the
trailing '/'" as argument and rename the argument (e.g., to "dirname").

Please also document that "skip" and "extras" can be NULL, but if not
NULL they need to be sorted lists.

> [...]

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 1/3] Add Travis CI support

[Jean-Noël Avila]

Le 11/10/2015 19:55, larsxschnei...@gmail.com a écrit :
> +
> +before_script: make
> +
> +script: make --quiet test

Travis can be used in container mode but that would need getting rid of
"sudo" command and only installing from white-listed sources
(https://github.com/travis-ci/apt-source-whitelist/blob/master/ubuntu.json)

Anyway, even within the present VM mode, 1.5 cores are available, so it
makes sense to add "-j2" to every make commands.

 

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 25/44] refs.h: document make refname_is_safe and add it to header

[Michael Haggerty]

On 10/13/2015 09:33 AM, Michael Haggerty wrote:
> On 10/12/2015 11:51 PM, David Turner wrote:
>> This function might be used by other refs backends
>>
>> Signed-off-by: David Turner 
>> ---
>>  refs.h | 11 +++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/refs.h b/refs.h
>> index fc8a748..7a936e2 100644
>> --- a/refs.h
>> +++ b/refs.h
>> @@ -348,6 +348,17 @@ int verify_refname_available(const char *newname, 
>> struct string_list *extra,
>>   struct string_list *skip, struct strbuf *err);
>>  
>>  /*
>> + * Check if a refname is safe.
>> + * For refs that start with "refs/" we consider it safe as long they do
>> + * not try to resolve to outside of refs/.
>> + *
>> + * For all other refs we only consider them safe iff they only contain
>> + * upper case characters and '_' (like "HEAD" AND "MERGE_HEAD", and not like
>> + * "config").
>> + */
>> +int refname_is_safe(const char *refname);
>> +
>> +/*
>>   * Flags controlling ref_transaction_update(), ref_transaction_create(), 
>> etc.
>>   * REF_NODEREF: act on the ref directly, instead of dereferencing
>>   *  symbolic references.
>>
> 
> The previous commit deleted this comment from where it previously
> appeared in refs-be-files.c. It would make more sense to squash this
> commit onto that one so it's clear that you are moving the comment
> rather than creating a new comment out of thin air.

Also, after this commit the prototype for this function appears twice in
refs.h.

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 27/44] refs.c: move peel_object to the common code

[Michael Haggerty]

On 10/12/2015 11:51 PM, David Turner wrote:
> This function does not contain any backend specific code so we
> move it to the common code.
> 
> Signed-off-by: David Turner 
> ---
>  refs-be-files.c | 53 -
>  refs.c  | 31 +++
>  refs.h  | 27 +++
>  3 files changed, 58 insertions(+), 53 deletions(-)
> 
> [...]
> diff --git a/refs.c b/refs.c
> index bd8c71b..99b31f6 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -4,6 +4,9 @@
>  #include "cache.h"
>  #include "refs.h"
>  #include "lockfile.h"
> +#include "object.h"
> +#include "tag.h"
> +
>  /*
>   * We always have a files backend and it is the default.
>   */
> @@ -1059,6 +1062,34 @@ int refname_is_safe(const char *refname)
>   return 1;
>  }
>  
> +/*
> + * Peel the named object; i.e., if the object is a tag, resolve the
> + * tag recursively until a non-tag is found.  If successful, store the
> + * result to sha1 and return PEEL_PEELED.  If the object is not a tag
> + * or is not valid, return PEEL_NON_TAG or PEEL_INVALID, respectively,
> + * and leave sha1 unchanged.
> + */

Please move the docstring to the header file.

> [...]
> diff --git a/refs.h b/refs.h
> index 3da5d09..636f959 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -76,6 +76,33 @@ extern int is_branch(const char *refname);
>   */
>  extern int peel_ref(const char *refname, unsigned char *sha1);
>  
> +enum peel_status {
> + /* object was peeled successfully: */
> + PEEL_PEELED = 0,
> +
> + /*
> +  * object cannot be peeled because the named object (or an
> +  * object referred to by a tag in the peel chain), does not
> +  * exist.
> +  */
> + PEEL_INVALID = -1,
> +
> + /* object cannot be peeled because it is not a tag: */
> + PEEL_NON_TAG = -2,
> +
> + /* ref_entry contains no peeled value because it is a symref: */
> + PEEL_IS_SYMREF = -3,
> +
> + /*
> +  * ref_entry cannot be peeled because it is broken (i.e., the
> +  * symbolic reference cannot even be resolved to an object
> +  * name):
> +  */
> + PEEL_BROKEN = -4
> +};
> +
> +enum peel_status peel_object(const unsigned char *name, unsigned char *sha1);
> +
>  /**
>   * Resolve refname in the nested "gitlink" repository that is located
>   * at path.  If the resolution is successful, return 0 and set sha1 to
> 

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 26/44] refs.c: move copy_msg to the common code

[Michael Haggerty]

On 10/12/2015 11:51 PM, David Turner wrote:
> Rename copy_msg to copy_reflog_msg and make it public.
> 
> Signed-off-by: David Turner 
> ---
>  refs-be-files.c | 28 +---
>  refs.c  | 26 ++
>  refs.h  |  2 ++
>  3 files changed, 29 insertions(+), 27 deletions(-)
> 
> [...]
> diff --git a/refs.c b/refs.c
> index 2515f6e..bd8c71b 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -886,6 +886,32 @@ int for_each_remote_ref_submodule(const char *submodule, 
> each_ref_fn fn, void *c
>   return for_each_ref_in_submodule(submodule, "refs/remotes/", fn, 
> cb_data);
>  }
>  
> +/*
> + * copy the reflog message msg to buf, which has been allocated sufficiently
> + * large, while cleaning up the whitespaces.  Especially, convert LF to 
> space,
> + * because reflog file is one line per entry.
> + */

Please put the docstring by the declaration in the header file.

> [...]
> diff --git a/refs.h b/refs.h
> index 7a936e2..3da5d09 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -597,6 +597,8 @@ enum ref_type {
>  
>  enum ref_type ref_type(const char *refname);
>  
> +int copy_reflog_msg(char *buf, const char *msg);
> +
>  enum expire_reflog_flags {
>   EXPIRE_REFLOGS_DRY_RUN = 1 << 0,
>   EXPIRE_REFLOGS_UPDATE_REF = 1 << 1,
> 

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: How to rebase when some commit hashes are in some commit messages

[Philip Oakley]

From: "Konstantin Khomoutov" 

On Tue, 13 Oct 2015 10:50:40 +0200
Francois-Xavier Le Bail  wrote:


>> For example, if I rebase the following commits, I would want that
>> if the commit hash 222... become 777...,
>> the message
>> "Update test output for "
>> become
>> "Update test output for 777..."
>>
>> Is it possible currently? And if yes how?
>
> AFAIK, it's not possible other than by editing the message by hand.

It seems to me useful to be able to do it. Can we hope a new option?


How do you think this could be practically implemented?

A couple of things which immediately spring to my mind:

To begin with, you are free to specify just a few first characters of
the commit name you're referring to.  So the alogrythm which finds the
relevant commits them has to be smart to somehow avoid misfires.  Or
have knobs to tune it (like -M of `git log`).

OK, suppose that this is solved through the usage of some agreed-upon
keywords in the commit message.  Say, you adopt a policy to put
something like

 X-Refers-To: 2dd8a9d9bb33ebffccb2ff516497adc8535bcab4

in your commit message to make the finder tool happy.

Now think how exactly it should work.  First, any commit at all might
mention the name of the target commit in its commit message.  Okay,
let's suppose there will be some way to somehow prune the possible DAG
down.  Then what happens if the commit to change is a part of the chain
of commits reachable from some branch other than that you're rebasing?
Automatically rebasing it would rewrite that commits and all commits
"after" it -- possibly resulting in what the "Recovering from upstream
rebase" part of the git-rebase(1) manual page deals with.

Having said that, the feature you're after appears to me to be a
sensible thing to have but the possibility of its generic implementation
appears to be moot.

Note that to deal with narrow simple cases (all possibly affected
commits leave on the same branch you're rebasing, and come later than
the rebase's anchor) you could write a script which uses `git log` to
find those commits which need special care.


My tuppence is that the only sha1's that could/would be rewritten would be 
those for the commits within the rebase. During rebasing it is expected that 
the user is re-adjusting things for later upstream consumption, with social 
controls and understandings with colleagues.


Thus the only sha1 numbers that could be used are those that are within the 
(possibly implied) instruction sheet (which will list the current sha1s that 
will be converted by rebase to new sha1's).


It should be clear that the sha1's are always backward references (because 
of the impossibility of including a forward reference to an as yet 
un-created future commit's sha1).


The key question (for me) is whether short sha1s are accepted, or if they 
must be full 40 char sha1's (perhaps an option). There are already options 
for making sure that short refs are not ambiguous.


It sound to me like a sensible small project for those that have such a 
workflow. I'm not sure if it should work with a patch based flow when 
submitting upstream - I'm a little fuzzy on how would the upstream 
maintainer know which sha1 referred to which patch.


Philip 


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Suggestion] add alt texts to the images of the git book

[Lucas Radaelli]

Hey there,

the book at https://git-scm.com/book/en/v2  (which is awesome btw, 
thanks for that!), contains a lot of images that are inaccessible to me 
because I am visually impaired. Not sure if they are really really 
important, but in some cases  it was a little bit complicated to follow 
some examples, more specifically, in the branching chapter.


Some outputs were pasted as plain text, which was much easier to follow, 
and I wonder if would be possible in the future to have all outputs 
pasted as plain text for easier reading?


Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 01/13] refs: convert some internal functions to use object_id

[Michael Haggerty]

On 10/09/2015 03:43 AM, brian m. carlson wrote:
> Convert several internal functions in refs.c to use struct object_id,
> and use the GIT_SHA1_HEXSZ constants in parse_ref_line.
> 
> Signed-off-by: brian m. carlson 
> [...]

I looked over this patch at the diff level and didn't find any problems.

This patch conflicts heavily with [1]. But I noticed that it is
independent of the rest of your series. I don't know when either patch
series will be ready, but see [2] for my take on the other one.

Assuming neither series is rejected, I think it would be much easier to
redo this patch on top of the first part of [1] than vice versa, so that
would be my suggestion. If it comes down to that, I am willing to help
redo this patch if you like.

Michael

[1] http://thread.gmane.org/gmane.comp.version-control.git/279423
[2] http://article.gmane.org/gmane.comp.version-control.git/279496

-- 
Michael Haggerty
mhag...@alum.mit.edu

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] Reinstate the helpful message when `git pull --rebase` fails

[Johannes Schindelin]

Hi Junio,

On 2015-10-12 22:28, Junio C Hamano wrote:
> Johannes Schindelin  writes:
> 
>>> I think the most sensible regression fix as the first step at this
>>> point is to call it as a separate process, just like the code calls
>>> "apply" as a separate process for each patch.  Optimization can come
>>> later when it is shown that it matters---we need to regain
>>> correctness first.
>>
>> I fear that you might underestimate the finality of this "first
>> step". If you reintroduce that separate process, not only is it a
>> performance regression, but could we really realistically expect any
>> further steps to happen after that? I do not think so.
>> ...
>> For the above reasons, I respectfully remain convinced that
>> reintroducing the separate process would be a mistake.
> 
> I am not saying we should forever do run_command() going forward.

Fine, I will stop arguing about this and go back grumble in my corner.

Ciao,
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: How to rebase when some commit hashes are in some commit messages

[Konstantin Khomoutov]

On Tue, 13 Oct 2015 10:50:40 +0200
Francois-Xavier Le Bail  wrote:

> >> For example, if I rebase the following commits, I would want that
> >> if the commit hash 222... become 777...,
> >> the message
> >> "Update test output for "
> >> become
> >> "Update test output for 777..."
> >>
> >> Is it possible currently? And if yes how?
> > 
> > AFAIK, it's not possible other than by editing the message by hand.
> 
> It seems to me useful to be able to do it. Can we hope a new option?

How do you think this could be practically implemented?

A couple of things which immediately spring to my mind:

To begin with, you are free to specify just a few first characters of
the commit name you're referring to.  So the alogrythm which finds the
relevant commits them has to be smart to somehow avoid misfires.  Or
have knobs to tune it (like -M of `git log`).

OK, suppose that this is solved through the usage of some agreed-upon
keywords in the commit message.  Say, you adopt a policy to put
something like

  X-Refers-To: 2dd8a9d9bb33ebffccb2ff516497adc8535bcab4

in your commit message to make the finder tool happy.

Now think how exactly it should work.  First, any commit at all might
mention the name of the target commit in its commit message.  Okay,
let's suppose there will be some way to somehow prune the possible DAG
down.  Then what happens if the commit to change is a part of the chain
of commits reachable from some branch other than that you're rebasing?
Automatically rebasing it would rewrite that commits and all commits
"after" it -- possibly resulting in what the "Recovering from upstream
rebase" part of the git-rebase(1) manual page deals with.

Having said that, the feature you're after appears to me to be a
sensible thing to have but the possibility of its generic implementation
appears to be moot.

Note that to deal with narrow simple cases (all possibly affected
commits leave on the same branch you're rebasing, and come later than
the rebase's anchor) you could write a script which uses `git log` to
find those commits which need special care.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 18/44] refs: move transaction functions into common code

[Michael Haggerty]

I reviewed the patches up to here pretty carefully, and aside from the
comments I already sent, they look good.

I like the new approach where the ref_transaction-building code is
shared across backends.

It seems to me that a good breaking point for the first batch of patches
would be here, just before you start creating the VTABLE in commit
[19/44]. The patches before this point all have to do with moving code
around and a little bit of light refactoring. They cause a lot of code
churn that will soon conflict with other people's work (e.g., [1]), but
I think they are pretty uncontroversial.

After this you start making a few important design decisions that
*could* be controversial. Therefore, by making a cut, you can maximize
the chance that the earlier patches can be merged to master relatively
quickly, after which the cross section for future conflicts will be much
smaller.

Ideally, you would include a few later patches in the "pre-VTABLE" patch
series. It looks like the following patches also mostly have to do with
moving code around, and would fit logically with the "pre-VTABLE" changes:

[24] refs.c: move refname_is_safe to the common code
[25] refs.h: document make refname_is_safe and add it to header
[26] refs.c: move copy_msg to the common code
[27] refs.c: move peel_object to the common code
[28] refs.c: move should_autocreate_reflog to common code
[32] initdb: move safe_create_dir into common code
[36] refs: make files_log_ref_write functions public
[37] refs: break out a ref conflict check

I tried rebasing those commits on top of your patch 18 and it wasn't too
bad. The result is on branch "refs-backend-pre-vtable" on my GitHub repo
[2], including my suggested changes to those eight patches (which
therefore became seven because I squashed the first two together).

Michael

[1] http://article.gmane.org/gmane.comp.version-control.git/279286
[2] https://github.com/mhagger/git

-- 
Michael Haggerty
mhag...@alum.mit.edu

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: How to rebase when some commit hashes are in some commit messages

[Jacob Keller]

On Tue, Oct 13, 2015 at 6:29 AM, Philip Oakley  wrote:
> My tuppence is that the only sha1's that could/would be rewritten would be
> those for the commits within the rebase. During rebasing it is expected that
> the user is re-adjusting things for later upstream consumption, with social
> controls and understandings with colleagues.
>

Agreed here. There would be no need to change any sha1s that didn't
change during the rebase. This limits the scope. Alright.

> Thus the only sha1 numbers that could be used are those that are within the
> (possibly implied) instruction sheet (which will list the current sha1s that
> will be converted by rebase to new sha1's).
>

Correct, you would be able to limit the number of sha1s to search for.

However, (see below), any reasonable reference to a sha1 should be
relatively stable.

> It should be clear that the sha1's are always backward references (because
> of the impossibility of including a forward reference to an as yet
> un-created future commit's sha1).
>
> The key question (for me) is whether short sha1s are accepted, or if they
> must be full 40 char sha1's (perhaps an option). There are already options
> for making sure that short refs are not ambiguous.
>
> It sound to me like a sensible small project for those that have such a
> workflow. I'm not sure if it should work with a patch based flow when
> submitting upstream - I'm a little fuzzy on how would the upstream
> maintainer know which sha1 referred to which patch.
>

My issue: the only sha1s in commit messages are *generally* things
which will NOT be changed in general. Supporting a work flow that
wants to change these is definitely crazy.

Essentially: I don't see a reason that you would be rebasing a commit
and needing to change any references in it. You can reference a commit
which isn't changing, but here's the possible situations I see:

a) you are rebasing a commit which references in the message a commit
that is not being changed (it is ancient)

In this case, nothing needs to be done.

b) you are rebasing a commit which references another commit in the same rebase

I see no valid reason to reference a sha1 in this case. If you're
referencing as a "fixes", then you are being silly since you can just
squash the fix into the original commit and thus prevent introduction
of bug at all.

What other reason? If you are referencing such as "thix extends
implementation from sha1" then your commit message is probably poorly
formatted. I don't see a reason to support this flow.

c) you are rebasing a commit which is referencing a commit that has
already been changed. (?)

I think (maybe) this is your interesting case, but here are some caveats.

Let's say you are fixing some old commit such as "Fixes: " or something.

If you do a "git pull --rebase", your commit might be updated to play
ontop of more new work, but the sha1 should still be valid, *unless*
the remote history did some rewind, at which point I don't think any
algorithm will work, see the issues above.

It may be something worth doing in git-filter-branch, but then you're
looking at losing the two assumptions above making it really hard to
get right.

Regards,
Jake
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 09/10] branch: use ref-filter printing APIs

[Karthik Nayak]

On Tue, Oct 13, 2015 at 10:01 PM, Junio C Hamano  wrote:
> Karthik Nayak  writes:
>
>> Port branch.c to use ref-filter APIs for printing. This clears out
>> most of the code used in branch.c for printing and replaces them with
>> calls made to the ref-filter library.
>>
>> Introduce get_format() which gets the format required for printing of
>> refs. Make amendments to print_ref_list() to reflect these changes.
>
> Is it get_format() still?

Will change that, thanks :)

-- 
Regards,
Karthik Nayak
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/1] send-email: allow to compose only prepared cover letter

[Andy Shevchenko]

Since @rev_list_opts contains everything that goes to the git format-patch
including an additional option like '--cover-letter' we might be interested to
compose it before send.

My often use case is to do:
% git format-patch --cover-letter --subject-prefix="PATCH vN" 
rev1^..revXYZ
% $GIT_EDITOR -*
% git send-email 00* # assumes series less than 100 patches
% rm -f 00*

Since git-send-email may send directly from repository it would be nice to
reduce above to just
% git send-email --compose --cover-letter --subject-prefix="PATCH vN" 
rev1^..revXYZ

P.S. Going further we can even introduce something like --valid-cmd to
send-email to run, for example, checkpatch.pl.

Signed-off-by: Andy Shevchenko 
---
 git-send-email.perl | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index e3ff44b..fc62d28 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -631,7 +631,10 @@ sub get_patch_subject {
die "No subject line in $fn ?";
 }
 
-if ($compose) {
+if ($compose and @rev_list_opts and grep { $_ eq '--cover-letter' } 
@rev_list_opts) {
+   # Cover letter always goes first
+   do_edit($files[0]);
+} elsif ($compose) {
# Note that this does not need to be secure, but we will make a small
# effort to have it be unique
$compose_filename = ($repo ?
-- 
2.5.3

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 09/10] branch: use ref-filter printing APIs

[Junio C Hamano]

Karthik Nayak  writes:

> Port branch.c to use ref-filter APIs for printing. This clears out
> most of the code used in branch.c for printing and replaces them with
> calls made to the ref-filter library.
>
> Introduce get_format() which gets the format required for printing of
> refs. Make amendments to print_ref_list() to reflect these changes.

Is it get_format() still?

>
> Change calc_maxwidth() to also account for the length of HEAD ref, by
> calling ref-filter:get_head_discription().
>
> Mentored-by: Christian Couder 
> Mentored-by: Matthieu Moy 
> Signed-off-by: Karthik Nayak 
> ---
>  builtin/branch.c | 261 
> ---
>  t/t6040-tracking-info.sh |   2 +-
>  2 files changed, 66 insertions(+), 197 deletions(-)
>
> diff --git a/builtin/branch.c b/builtin/branch.c
> index 67ef9f1..4d8b570 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -35,12 +35,12 @@ static unsigned char head_sha1[20];
>  
>  static int branch_use_color = -1;
>  static char branch_colors[][COLOR_MAXLEN] = {
> - GIT_COLOR_RESET,
> - GIT_COLOR_NORMAL,   /* PLAIN */
> - GIT_COLOR_RED,  /* REMOTE */
> - GIT_COLOR_NORMAL,   /* LOCAL */
> - GIT_COLOR_GREEN,/* CURRENT */
> - GIT_COLOR_BLUE, /* UPSTREAM */
> + "%(color:reset)",
> + "%(color:reset)",   /* PLAIN */
> + "%(color:red)", /* REMOTE */
> + "%(color:reset)",   /* LOCAL */
> + "%(color:green)",   /* CURRENT */
> + "%(color:blue)",/* UPSTREAM */
>  };
>  enum color_branch {
>   BRANCH_COLOR_RESET = 0,
> @@ -271,192 +271,6 @@ static int delete_branches(int argc, const char **argv, 
> int force, int kinds,
>   return(ret);
>  }
>  
> -static void fill_tracking_info(struct strbuf *stat, const char *branch_name,
> - int show_upstream_ref)
> -{
> - int ours, theirs;
> - char *ref = NULL;
> - struct branch *branch = branch_get(branch_name);
> - const char *upstream;
> - struct strbuf fancy = STRBUF_INIT;
> - int upstream_is_gone = 0;
> - int added_decoration = 1;
> -
> - if (stat_tracking_info(branch, , , ) < 0) {
> - if (!upstream)
> - return;
> - upstream_is_gone = 1;
> - }
> -
> - if (show_upstream_ref) {
> - ref = shorten_unambiguous_ref(upstream, 0);
> - if (want_color(branch_use_color))
> - strbuf_addf(, "%s%s%s",
> - branch_get_color(BRANCH_COLOR_UPSTREAM),
> - ref, 
> branch_get_color(BRANCH_COLOR_RESET));
> - else
> - strbuf_addstr(, ref);
> - }
> -
> - if (upstream_is_gone) {
> - if (show_upstream_ref)
> - strbuf_addf(stat, _("[%s: gone]"), fancy.buf);
> - else
> - added_decoration = 0;
> - } else if (!ours && !theirs) {
> - if (show_upstream_ref)
> - strbuf_addf(stat, _("[%s]"), fancy.buf);
> - else
> - added_decoration = 0;
> - } else if (!ours) {
> - if (show_upstream_ref)
> - strbuf_addf(stat, _("[%s: behind %d]"), fancy.buf, 
> theirs);
> - else
> - strbuf_addf(stat, _("[behind %d]"), theirs);
> -
> - } else if (!theirs) {
> - if (show_upstream_ref)
> - strbuf_addf(stat, _("[%s: ahead %d]"), fancy.buf, ours);
> - else
> - strbuf_addf(stat, _("[ahead %d]"), ours);
> - } else {
> - if (show_upstream_ref)
> - strbuf_addf(stat, _("[%s: ahead %d, behind %d]"),
> - fancy.buf, ours, theirs);
> - else
> - strbuf_addf(stat, _("[ahead %d, behind %d]"),
> - ours, theirs);
> - }
> - strbuf_release();
> - if (added_decoration)
> - strbuf_addch(stat, ' ');
> - free(ref);
> -}
> -
> -static void add_verbose_info(struct strbuf *out, struct ref_array_item *item,
> -  struct ref_filter *filter, const char *refname)
> -{
> - struct strbuf subject = STRBUF_INIT, stat = STRBUF_INIT;
> - const char *sub = _("  invalid ref ");
> - struct commit *commit = item->commit;
> -
> - if (!parse_commit(commit)) {
> - pp_commit_easy(CMIT_FMT_ONELINE, commit, );
> - sub = subject.buf;
> - }
> -
> - if (item->kind == FILTER_REFS_BRANCHES)
> - fill_tracking_info(, refname, filter->verbose > 1);
> -
> - strbuf_addf(out, " %s %s%s",
> - find_unique_abbrev(item->commit->object.sha1, filter->abbrev),
> - stat.buf, sub);
> - strbuf_release();
> -

Re: [PATCH] Add fetch.recurseSubmoduleParallelism config option

[Stefan Beller]

On Tue, Oct 13, 2015 at 12:32 AM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>>> The parallel_process API could learn a new "verbose" feature that it
>>> by itself shows some messages like
>>>
>>> "processing the 'frotz' job with N tasks"
>>> "M tasks finished (N still running)"
>>
>> I know what to fill in for M and N, 'frotz' is a bit unclear to me.
>
> At least I don't know what M and N should be, and I'm curious how
> you'll define them.  See below.

I presumed the second school of thought. Another alternative there would
be to have 3 numbers:

"M tasks finished (N still running, K pending output)"

>
>>> in the output stream from strategic places.  For example, the first
>>> message will come at the end of pp_init(), and the second message
>>> will be appended at the end of buffered output of a task that has
>>> just been finished.  Once you have something like that, you could
>>> check for them in a test in t/.
>>>
>>> Just a thought.
>>
>> I like that thought. :)
>
>
> A few more random thoughts:
>
>  * The only thing you could rely on if you were to use the above in
>your tests is the one from pp_init() that declares how many
>processes the machinery is going to use.  M/N will be unstable,
>depending on the scheduling order (e.g. the foreground process
>may take a lot of time to finish, while many other processes
>finish first).
>
>  * Every time the foreground process (i.e. the one whose output is
>tee-ed to the overall output from the machinery) finishes, you
>can emit "M tasks finished (N still running)", but I am not sure
>what M should be.  It is debatable how to account for background
>processes that have already completed but whose output haven't
>been shown.

Assuming we go with your second school of thought (N are the real
running processes, M including the finished but still pending output tasks),
we may have confusing output, as the output order may confuse the
reader:

N=8 M=13 (output from live task)
...
N=8 M=12 (output from buffered task)
...

Anyone who has no knowledge of the internals, wonders why
M goes *down* ?

>
>One school of thought that is in line with the "pretend as if the
>background tasks are started immediately after the foreground
>task finishes, and they run at infinite speed and produce output
>in no time" idea, on which the "queue output from the background
>processes and emit at once in order to avoid intermixing" design
>was based on, would be not to include them in M (i.e. finished
>ones), because their output haven't been emitted and we are
>pretending that they haven't even been started.  If you take this
>approach, you however may have to include them in N (i.e. still
>running), but that would likely bump N beyond the maximum number
>of simultaneous processes.
>
>The other school of thought would of course tell the truth and
>include the number of finished background processes in M, as they
>have finished already in the reality.  This will not risk showing
>N that is beyond the maximum, but your first "progress" output
>might say "3 tasks finished", which will make it look odd in a
>different way.
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Solaris: Fail to build git with CFLAGS=-m64

[evgeny litvinenko]

Hi Junio,

On 10/13/15, Junio C Hamano  wrote:
> evgeny litvinenko  writes:
>
>> Solaris has the following prototype in the file /usr/include/arpa/inet.h:
>>
>> extern const char *inet_ntop(int, const void *_RESTRICT_KYWD, char
>> *_RESTRICT_KYWD, socklen_t);
>>
>> Git's prototype for inet_ntop is in file git-compat-util.h:
>>
>> #ifdef NO_INET_NTOP
>> const char *inet_ntop(int af, const void *src, char *dst, size_t size);
>> #endif
>
> This tells me that it is designed to be used only when the platform
> does not offer inet_ntop().  If your platform does have it, why is
> your build define NO_INET_NTOP?

I think this is because configure.ac doesn't check for inet_ntop in
libnsl (in Solaris 11.2 inet_ntop and inet_pton are in libnsl)
configure.ac checks for these function in libresolv only.

> Perhaps configure generated by autoconf is faulty?
>
> In this project, use of autoconf/configure is optional---in other
> words, it is expected that you can build successfully by setting and
> unsetting necessary Makefile macros in your own config.mak without
> using autoconf/configure at all.  I'd try without configure and make
> sure I do not define NO_INET_NTOP (and if you have inet_pton(), then
> make sure you do not define NO_INET_PTON either) in config.mak if I
> were you.
>

Yes, if I do not define NO_INET_NTOP/NO_INET_PTON the build is successful
as libnsl is linked - in configure.ac there is 'NEEDS_NSL = YesPlease'
for SunOS'es.

evgeny.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: How to rebase when some commit hashes are in some commit messages

[Philip Oakley]

From: "Mike Rappazzo" 
On Tue, Oct 13, 2015 at 1:07 PM, Jacob Keller  
wrote:
On Tue, Oct 13, 2015 at 6:29 AM, Philip Oakley  
wrote:
My tuppence is that the only sha1's that could/would be rewritten would 
be
those for the commits within the rebase. During rebasing it is expected 
that
the user is re-adjusting things for later upstream consumption, with 
social

controls and understandings with colleagues.



Agreed here. There would be no need to change any sha1s that didn't
change during the rebase. This limits the scope. Alright.

Thus the only sha1 numbers that could be used are those that are within 
the
(possibly implied) instruction sheet (which will list the current sha1s 
that

will be converted by rebase to new sha1's).



Correct, you would be able to limit the number of sha1s to search for.

However, (see below), any reasonable reference to a sha1 should be
relatively stable.

It should be clear that the sha1's are always backward references 
(because

of the impossibility of including a forward reference to an as yet
un-created future commit's sha1).

The key question (for me) is whether short sha1s are accepted, or if 
they
must be full 40 char sha1's (perhaps an option). There are already 
options

for making sure that short refs are not ambiguous.

It sound to me like a sensible small project for those that have such a
workflow. I'm not sure if it should work with a patch based flow when
submitting upstream - I'm a little fuzzy on how would the upstream
maintainer know which sha1 referred to which patch.



My issue: the only sha1s in commit messages are *generally* things
which will NOT be changed in general. Supporting a work flow that
wants to change these is definitely crazy.

Essentially: I don't see a reason that you would be rebasing a commit
and needing to change any references in it. You can reference a commit
which isn't changing, but here's the possible situations I see:

a) you are rebasing a commit which references in the message a commit
that is not being changed (it is ancient)

In this case, nothing needs to be done.

b) you are rebasing a commit which references another commit in the same 
rebase


I see no valid reason to reference a sha1 in this case. If you're
referencing as a "fixes", then you are being silly since you can just
squash the fix into the original commit and thus prevent introduction
of bug at all.

What other reason? If you are referencing such as "thix extends
implementation from sha1" then your commit message is probably poorly
formatted. I don't see a reason to support this flow.

c) you are rebasing a commit which is referencing a commit that has
already been changed. (?)

I think (maybe) this is your interesting case, but here are some caveats.

Let's say you are fixing some old commit such as "Fixes: " or something.

If you do a "git pull --rebase", your commit might be updated to play
ontop of more new work, but the sha1 should still be valid, *unless*
the remote history did some rewind, at which point I don't think any
algorithm will work, see the issues above.

It may be something worth doing in git-filter-branch, but then you're
looking at losing the two assumptions above making it really hard to
get right.

Regards,
Jake


It seems reasonable that this could be added as a feature of
interactive rebase.  The todo list could be automatically adjusted to
"reword" for those commits which are referring to other commits within
the same rebase.  As each commit is re-written, a mapping could be
kept of old sha1 -> new sha1.  Then when one of the reworded commits
is being applied, the old sha1 -> new sha1 mapping could be used to
add a line to the $COMMIT_MSG.
--
The extra fun begins if the commit message is of a one-line pretty quoted 
style, where more of the quote needs changing...

e.g.
[alias]
quote = log -1 --pretty='tformat:%h (%s, %ad)' --date=short
log1 = log -1 --pretty=\"format:%ad %h (%an): %s\" --date=short

Jake was concerned about the 'crazy' workflow, however almost all workflows 
are crazy at a distance.
The rebase is required if the workflow's allowed base point moves forward 
faster than one can complete the (likely long) patch series, so the series 
is rebased and then an acceptable series can be merged without 
modifications.
Git has the former issue i.e. master and next can move forward faster than a 
long series takes to be reviewed, but does not have the latter because Junio 
adds his signature to each commit, and uses the patch submission flow.


IIUC (as an alternate example),  in G4W one can submit a (long) pull request 
with internal back references that would be merged directly, so the sha1's 
could be updated as Francois-Xavier originally asked. I have a series that's 
been bumping along for a long while that needs regular rebasing, though 
doesn't have sha1 back references, so I can see that the need does happen. I 
can see that others 

Re: [PATCH v2 08/10] ref-filter: add support for %(upstream:track,nobracket)

[Junio C Hamano]

Matthieu Moy  writes:

>> If you see here, we detect "track" first for
>> %(upstream:track,nobracket)
>
> Yes, but I still think that this was a bad idea. If you want
> nobracket to apply to "track", then the syntax should be
> %(upstream:track=nobracket). I think the "nobracket" should apply
> to "upstream" (i.e. be global to the atom), hence
> %(upstream:nobracket,track) and %(upstream:track,nobracket) should
> both be accepted.

That makes sense to me, as I think what is being discussed would be
%(upstream:track=yes,bracket=no) when it is fully spelled out.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] merge: fix cache_entry use-after-free

[David Turner]

On Mon, 2015-10-12 at 15:28 -0700, Junio C Hamano wrote:
> David Turner  writes:
> 
> > From: Keith McGuigan 
> >
> > During merges, we would previously free entries that we no longer need
> > in the destination index.  But those entries might also be stored in
> > the dir_entry cache, and when a later call to add_to_index found them,
> > they would be used after being freed.
> >
> > To prevent this, add a ref count for struct cache_entry.  Whenever
> > a cache entry is added to a data structure, the ref count is incremented;
> > when it is removed from the data structure, it is decremented.  When
> > it hits zero, the cache_entry is freed.
> >
> > Signed-off-by: Keith McGuigan 
> > ---
> 
> I'll forge your "messenger's sign-off" here ;-)

Thanks.

> > diff --git a/unpack-trees.c b/unpack-trees.c
> > index f932e80..1a0a637 100644
> > --- a/unpack-trees.c
> > +++ b/unpack-trees.c
> > @@ -606,8 +606,10 @@ static int unpack_nondirectories(int n, unsigned long 
> > mask,
> > o);
> > for (i = 0; i < n; i++) {
> > struct cache_entry *ce = src[i + o->merge];
> > -   if (ce != o->df_conflict_entry)
> > -   free(ce);
> > +   if (ce != o->df_conflict_entry) {
> > +   drop_ce_ref(ce);
> > +   src[i + o->merge] = NULL;
> > +   }
> 
> This one smelled iffy.  I think it is safe because the caller does
> not look at src[] other than src[0] after this function returns, and
> this setting to NULL happens only when o->merge is set to 1, so I do
> not think this is buggy, but at the same time I do not think setting
> to NULL is necessary.
> 
> Other than that, looks nice.  Thanks.

I like to set a pointer to NULL after I free the thing pointed to by it,
because it helps make use-after-free bugs easier to detect.  

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: How to rebase when some commit hashes are in some commit messages

[Mike Rappazzo]

On Tue, Oct 13, 2015 at 1:07 PM, Jacob Keller  wrote:
> On Tue, Oct 13, 2015 at 6:29 AM, Philip Oakley  wrote:
>> My tuppence is that the only sha1's that could/would be rewritten would be
>> those for the commits within the rebase. During rebasing it is expected that
>> the user is re-adjusting things for later upstream consumption, with social
>> controls and understandings with colleagues.
>>
>
> Agreed here. There would be no need to change any sha1s that didn't
> change during the rebase. This limits the scope. Alright.
>
>> Thus the only sha1 numbers that could be used are those that are within the
>> (possibly implied) instruction sheet (which will list the current sha1s that
>> will be converted by rebase to new sha1's).
>>
>
> Correct, you would be able to limit the number of sha1s to search for.
>
> However, (see below), any reasonable reference to a sha1 should be
> relatively stable.
>
>> It should be clear that the sha1's are always backward references (because
>> of the impossibility of including a forward reference to an as yet
>> un-created future commit's sha1).
>>
>> The key question (for me) is whether short sha1s are accepted, or if they
>> must be full 40 char sha1's (perhaps an option). There are already options
>> for making sure that short refs are not ambiguous.
>>
>> It sound to me like a sensible small project for those that have such a
>> workflow. I'm not sure if it should work with a patch based flow when
>> submitting upstream - I'm a little fuzzy on how would the upstream
>> maintainer know which sha1 referred to which patch.
>>
>
> My issue: the only sha1s in commit messages are *generally* things
> which will NOT be changed in general. Supporting a work flow that
> wants to change these is definitely crazy.
>
> Essentially: I don't see a reason that you would be rebasing a commit
> and needing to change any references in it. You can reference a commit
> which isn't changing, but here's the possible situations I see:
>
> a) you are rebasing a commit which references in the message a commit
> that is not being changed (it is ancient)
>
> In this case, nothing needs to be done.
>
> b) you are rebasing a commit which references another commit in the same 
> rebase
>
> I see no valid reason to reference a sha1 in this case. If you're
> referencing as a "fixes", then you are being silly since you can just
> squash the fix into the original commit and thus prevent introduction
> of bug at all.
>
> What other reason? If you are referencing such as "thix extends
> implementation from sha1" then your commit message is probably poorly
> formatted. I don't see a reason to support this flow.
>
> c) you are rebasing a commit which is referencing a commit that has
> already been changed. (?)
>
> I think (maybe) this is your interesting case, but here are some caveats.
>
> Let's say you are fixing some old commit such as "Fixes:  summary, date>" or something.
>
> If you do a "git pull --rebase", your commit might be updated to play
> ontop of more new work, but the sha1 should still be valid, *unless*
> the remote history did some rewind, at which point I don't think any
> algorithm will work, see the issues above.
>
> It may be something worth doing in git-filter-branch, but then you're
> looking at losing the two assumptions above making it really hard to
> get right.
>
> Regards,
> Jake

It seems reasonable that this could be added as a feature of
interactive rebase.  The todo list could be automatically adjusted to
"reword" for those commits which are referring to other commits within
the same rebase.  As each commit is re-written, a mapping could be
kept of old sha1 -> new sha1.  Then when one of the reworded commits
is being applied, the old sha1 -> new sha1 mapping could be used to
add a line to the $COMMIT_MSG.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 19/44] refs-be-files.c: add a backend method structure with transaction functions

[David Turner]

On Tue, 2015-10-13 at 09:56 +0200, Michael Haggerty wrote:
> On 10/12/2015 11:51 PM, David Turner wrote:
> > From: Ronnie Sahlberg 
> > 
> > Add a ref structure for backend methods. Start by adding a method pointer
> > for the transaction commit function.
> > 
> > Add a function set_refs_backend to switch between backends. The files
> > based backend is the default.
> > 
> > Signed-off-by: Ronnie Sahlberg 
> > Signed-off-by: David Turner 
> > ---
> >  refs-be-files.c | 10 --
> >  refs.c  | 30 ++
> >  refs.h  | 15 +++
> >  3 files changed, 53 insertions(+), 2 deletions(-)
> > 
> > [...]
> > diff --git a/refs.h b/refs.h
> > index 4940ae9..419abf4 100644
> > --- a/refs.h
> > +++ b/refs.h
> > @@ -619,4 +619,19 @@ extern int reflog_expire(const char *refname, const 
> > unsigned char *sha1,
> >  reflog_expiry_cleanup_fn cleanup_fn,
> >  void *policy_cb_data);
> >  
> > +/* refs backends */
> > +typedef int ref_transaction_commit_fn(struct ref_transaction *transaction,
> > + struct strbuf *err);
> > +typedef void ref_transaction_free_fn(struct ref_transaction *transaction);
> 
> The ref_transaction_free_fn typedef isn't used anywhere.

Will fix, thanks.

> > +struct ref_be {
> > +   struct ref_be *next;
> > +   const char *name;
> > +   ref_transaction_commit_fn *transaction_commit;
> > +};
> > +
> > +
> > +extern struct ref_be refs_be_files;
> 
> I don't think that refs_be_files is needed in the public interface.

We use refs_be_lmdb in a few other places to set up the lmdb backend, so
I thought I would put refs_be_files in refs.h too.  But I can remove
refs_be_files and just stick it in the places it's needed.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 37/44] refs: break out a ref conflict check

[David Turner]

On Tue, 2015-10-13 at 12:25 +0200, Michael Haggerty wrote:
> On 10/12/2015 11:51 PM, David Turner wrote:
> > Create new function verify_no_descendants, to hold one of the ref
> > conflict checks used in verify_refname_available.  Multiple backends
> > will need this function, so it goes in the common code.
> > 
> > Signed-off-by: David Turner 
> > ---
> >  refs-be-files.c | 33 -
> >  refs.c  | 22 ++
> >  refs.h  |  7 +++
> >  3 files changed, 37 insertions(+), 25 deletions(-)
> > 
> > [...]
> > diff --git a/refs.c b/refs.c
> > index 5a3125d..3ae0274 100644
> > --- a/refs.c
> > +++ b/refs.c
> > @@ -1101,6 +1101,28 @@ enum peel_status peel_object(const unsigned char 
> > *name, unsigned char *sha1)
> > return PEEL_PEELED;
> >  }
> >  
> > +const char *find_descendant_ref(const char *refname,
> > +   const struct string_list *extras,
> > +   const struct string_list *skip)
> > +{
> > +   int pos;
> > +   if (!extras)
> > +   return NULL;
> > +
> > +   /* Look for the place where "$refname/" would be inserted in extras. */
> 
> The comment above is misleading. See my note at the bottom of this email
> for an explanation.
> 
> > +   for (pos = string_list_find_insert_index(extras, refname, 0);
> > +pos < extras->nr; pos++) {
> > +   const char *extra_refname = extras->items[pos].string;
> > +
> > +   if (!starts_with(extra_refname, refname))
> > +   break;
> > +
> > +   if (!skip || !string_list_has_string(skip, extra_refname))
> > +   return extra_refname;
> > +   }
> > +   return NULL;
> > +}
> > +
> >  /* backend functions */
> >  int refs_initdb(struct strbuf *err, int shared)
> >  {
> > diff --git a/refs.h b/refs.h
> > index 3aad3b8..f8becea 100644
> > --- a/refs.h
> > +++ b/refs.h
> > @@ -637,6 +637,13 @@ int files_log_ref_write(const char *refname, const 
> > unsigned char *old_sha1,
> > const unsigned char *new_sha1, const char *msg,
> > int flags, struct strbuf *err);
> >  
> > +/*
> > + * Check for entries in extras that start with "$refname/", ignoring
> > + * those in skip. If there is such an entry, then we have a conflict.
> > + */
> > +const char *find_descendant_ref(const char *refname,
> > +   const struct string_list *extras,
> > +   const struct string_list *skip);
> 
> The documentation is is not correct. As written, the function needs not
> the refname as argument but the refname followed by '/'. Without the
> trailing slash, "refs/heads/foo" would be blocked by "refs/heads/foobar".
> 
> From the point of view of simplicity, it would be nice if this function
> could take a refname (without the trailing slash) as argument. But then
> it would basically be forced to allocate a new string, copy refname to
> it, append a '/', then free the string again when it's done.
> Alternatively, it could accept its refname argument in a strbuf and
> temporarily append the '/'.
> 
> But neither one of these alternatives is so elegant, so maybe it's OK if
> this function is documented to take a "directory name, including the
> trailing '/'" as argument and rename the argument (e.g., to "dirname").
> 
> Please also document that "skip" and "extras" can be NULL, but if not
> NULL they need to be sorted lists.

I think `extras` may not be NULL.  But I've otherwise fixed this.
Thanks.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 08/10] ref-filter: add support for %(upstream:track,nobracket)

[Karthik Nayak]

On Thu, Oct 8, 2015 at 2:48 PM, Karthik Nayak  wrote:
> Add support for %(upstream:track,nobracket) which will print the
> tracking information without the brackets (i.e. "ahead N, behind M").
>
> Add test and documentation for the same.
> ---
>  Documentation/git-for-each-ref.txt |  6 --
>  ref-filter.c   | 28 +++-
>  t/t6300-for-each-ref.sh|  9 +
>  3 files changed, 36 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/git-for-each-ref.txt 
> b/Documentation/git-for-each-ref.txt
> index c713ec0..a38cbf6 100644
> --- a/Documentation/git-for-each-ref.txt
> +++ b/Documentation/git-for-each-ref.txt
> @@ -114,8 +114,10 @@ upstream::
> `refname` above.  Additionally respects `:track` to show
> "[ahead N, behind M]" and `:trackshort` to show the terse
> version: ">" (ahead), "<" (behind), "<>" (ahead and behind),
> -   or "=" (in sync).  Has no effect if the ref does not have
> -   tracking information associated with it.
> +   or "=" (in sync).  Append `:track,nobracket` to show tracking
> +   information without brackets (i.e "ahead N, behind M").  Has
> +   no effect if the ref does not have tracking information
> +   associated with it.
>
>  push::
> The name of a local ref which represents the `@{push}` location
> diff --git a/ref-filter.c b/ref-filter.c
> index 6a38089..6044eb0 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -1112,27 +1112,45 @@ static void populate_value(struct ref_array_item *ref)
> if (!strcmp(formatp, "short"))
> refname = shorten_unambiguous_ref(refname,
>   warn_ambiguous_refs);
> -   else if (!strcmp(formatp, "track") &&
> +   else if (skip_prefix(formatp, "track", ) &&
> +strcmp(formatp, "trackshort") &&
>  (starts_with(name, "upstream") ||
>   starts_with(name, "push"))) {

If you see here, we detect "track" first for
%(upstream:track,nobracket) so although
the idea was to use something similar to %(align:...) I don't see a
good way of going
about this. If we want %(upstream:nobracket,track) to be supported then, I think
we'll have to change this whole layout and have the detection done up where we
locat "upstream" / "push", what would be a nice way to go around this?

What I could think of:
1. Do the cleanup that Junio and Matthieu suggested, where we
basically parse the
atoms and store them into a used_atom struct. I could either work on
those patches
before this and then rebase this on top.
2. Let this be and come back on it when implementing the above series.
After reading Matthieu's and Junio's discussion, I lean towards the latter.

> char buf[40];
> +   unsigned int nobracket = 0;
> +
> +   if (!strcmp(valp, ",nobracket"))
> +   nobracket = 1;
>
> if (stat_tracking_info(branch, _ours,
>_theirs, NULL)) {
> -   v->s = "[gone]";
> +   if (nobracket)
> +   v->s = "gone";
> +   else
> +   v->s = "[gone]";
> continue;
> }
>
> if (!num_ours && !num_theirs)
> v->s = "";
> else if (!num_ours) {
> -   sprintf(buf, "[behind %d]", 
> num_theirs);
> +   if (nobracket)
> +   sprintf(buf, "behind %d", 
> num_theirs);
> +   else
> +   sprintf(buf, "[behind %d]", 
> num_theirs);
> v->s = xstrdup(buf);
> } else if (!num_theirs) {
> -   sprintf(buf, "[ahead %d]", num_ours);
> +   if (nobracket)
> +   sprintf(buf, "ahead %d", 
> num_ours);
> +   else
> +   sprintf(buf, "[ahead %d]", 
> num_ours);
> v->s = xstrdup(buf);
> } else {
> -   sprintf(buf, "[ahead %d, behind %d]",
> +   if (nobracket)
> +

Re: [PATCH v3 25/44] refs.h: document make refname_is_safe and add it to header

[David Turner]

On Tue, 2015-10-13 at 11:18 +0200, Michael Haggerty wrote:
> On 10/13/2015 09:33 AM, Michael Haggerty wrote:
> > On 10/12/2015 11:51 PM, David Turner wrote:
> >> This function might be used by other refs backends
> >>
> >> Signed-off-by: David Turner 
> >> ---
> >>  refs.h | 11 +++
> >>  1 file changed, 11 insertions(+)
> >>
> >> diff --git a/refs.h b/refs.h
> >> index fc8a748..7a936e2 100644
> >> --- a/refs.h
> >> +++ b/refs.h
> >> @@ -348,6 +348,17 @@ int verify_refname_available(const char *newname, 
> >> struct string_list *extra,
> >> struct string_list *skip, struct strbuf *err);
> >>  
> >>  /*
> >> + * Check if a refname is safe.
> >> + * For refs that start with "refs/" we consider it safe as long they do
> >> + * not try to resolve to outside of refs/.
> >> + *
> >> + * For all other refs we only consider them safe iff they only contain
> >> + * upper case characters and '_' (like "HEAD" AND "MERGE_HEAD", and not 
> >> like
> >> + * "config").
> >> + */
> >> +int refname_is_safe(const char *refname);
> >> +
> >> +/*
> >>   * Flags controlling ref_transaction_update(), ref_transaction_create(), 
> >> etc.
> >>   * REF_NODEREF: act on the ref directly, instead of dereferencing
> >>   *  symbolic references.
> >>
> > 
> > The previous commit deleted this comment from where it previously
> > appeared in refs-be-files.c. It would make more sense to squash this
> > commit onto that one so it's clear that you are moving the comment
> > rather than creating a new comment out of thin air.
> 
> Also, after this commit the prototype for this function appears twice in
> refs.h.

Will squash and fix.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 08/10] ref-filter: add support for %(upstream:track,nobracket)

[Matthieu Moy]

Karthik Nayak  writes:

> On Thu, Oct 8, 2015 at 2:48 PM, Karthik Nayak  wrote:
>> Add support for %(upstream:track,nobracket) which will print the
>> tracking information without the brackets (i.e. "ahead N, behind M").
>>
>> Add test and documentation for the same.
>> ---
>>  Documentation/git-for-each-ref.txt |  6 --
>>  ref-filter.c   | 28 +++-
>>  t/t6300-for-each-ref.sh|  9 +
>>  3 files changed, 36 insertions(+), 7 deletions(-)
>>
>> diff --git a/Documentation/git-for-each-ref.txt 
>> b/Documentation/git-for-each-ref.txt
>> index c713ec0..a38cbf6 100644
>> --- a/Documentation/git-for-each-ref.txt
>> +++ b/Documentation/git-for-each-ref.txt
>> @@ -114,8 +114,10 @@ upstream::
>> `refname` above.  Additionally respects `:track` to show
>> "[ahead N, behind M]" and `:trackshort` to show the terse
>> version: ">" (ahead), "<" (behind), "<>" (ahead and behind),
>> -   or "=" (in sync).  Has no effect if the ref does not have
>> -   tracking information associated with it.
>> +   or "=" (in sync).  Append `:track,nobracket` to show tracking
>> +   information without brackets (i.e "ahead N, behind M").  Has
>> +   no effect if the ref does not have tracking information
>> +   associated with it.
>>
>>  push::
>> The name of a local ref which represents the `@{push}` location
>> diff --git a/ref-filter.c b/ref-filter.c
>> index 6a38089..6044eb0 100644
>> --- a/ref-filter.c
>> +++ b/ref-filter.c
>> @@ -1112,27 +1112,45 @@ static void populate_value(struct ref_array_item 
>> *ref)
>> if (!strcmp(formatp, "short"))
>> refname = shorten_unambiguous_ref(refname,
>>   warn_ambiguous_refs);
>> -   else if (!strcmp(formatp, "track") &&
>> +   else if (skip_prefix(formatp, "track", ) &&
>> +strcmp(formatp, "trackshort") &&
>>  (starts_with(name, "upstream") ||
>>   starts_with(name, "push"))) {
>
> If you see here, we detect "track" first for
> %(upstream:track,nobracket)

Yes, but I still think that this was a bad idea. If you want nobracket
to apply to "track", then the syntax should be
%(upstream:track=nobracket). I think the "nobracket" should apply to
"upstream" (i.e. be global to the atom), hence
%(upstream:nobracket,track) and %(upstream:track,nobracket) should both
be accepted. Possibly %(upstream:,nobracket) could complain,
but just ignoring "nobracket" in this case would make sense to me.

Special-casing the implementation of "nobracket" also means you're
special-casing its user-visible behavior. And as a user, I hate it when
the behavior is subtely different for things that look like each other
(in this case, %(align:...) and %(upstream:...) ).

> %(upstream:nobracket,track) to be supported then, I think we'll have
> to change this whole layout and have the detection done up where we
> locat "upstream" / "push", what would be a nice way to go around this?

You mean, below

else if (starts_with(name, "upstream")) {

within populate_value()?

I think it would, yes.

> What I could think of:
> 1. Do the cleanup that Junio and Matthieu suggested, where we
> basically parse the
> atoms and store them into a used_atom struct. I could either work on
> those patches
> before this and then rebase this on top.
> 2. Let this be and come back on it when implementing the above series.
> After reading Matthieu's and Junio's discussion, I lean towards the latter.

Leaving it as-is does not fit in my arguments to do the refactoring
later. It's not introducing "another instance of an existing pattern",
but actually a new pattern.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 05/44] refs.c: move update_ref to refs.c

[David Turner]

On Tue, 2015-10-13 at 05:41 +0200, Michael Haggerty wrote:

> If its removal was intentional, it deserves a careful explanation (and
> should probably be done as a separate commit). If it was an accident,
> please consider how this accident arose and try to think about whether
> similar accidents might have happened elsewhere in this series.

This was an accident. I think it must have happened when I
forward-ported Ronnie's changes over my change that introduced that
check.  Usually, when there were conflicts during this process
(indicating that the moved code had changed in the meantime), I did the
move by copy-pasting the code (rather than by choosing the old version).
Apparently, I missed this one.

Will fix.

> > [...]
> 
> Michael
> 


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] send-email: allow to compose only prepared cover letter

[Junio C Hamano]

Andy Shevchenko  writes:

> My often use case is to do:
>   % git format-patch --cover-letter --subject-prefix="PATCH vN" 
> rev1^..revXYZ
>   % $GIT_EDITOR -*
>   % git send-email 00* # assumes series less than 100 patches
>   % rm -f 00*

I guess this patch would not hurt too much, but the above would
vastly be improved if you used "-vN" option, instead of the
hand-rolled subject prefix, and dropped the last "rm -f" (which in
turn would mean you would want to use -o option to specify where to
keep the older iterations of the topic).  Then you can easily refer
to cover letters and patches from previous rounds while preparing
the latest to send out.



--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 19/44] refs-be-files.c: add a backend method structure with transaction functions

[Michael Haggerty]

On 10/13/2015 08:28 PM, David Turner wrote:
> On Tue, 2015-10-13 at 09:56 +0200, Michael Haggerty wrote:
>> On 10/12/2015 11:51 PM, David Turner wrote:
>>> [...]
>>> +extern struct ref_be refs_be_files;
>>
>> I don't think that refs_be_files is needed in the public interface.
> 
> We use refs_be_lmdb in a few other places to set up the lmdb backend, so
> I thought I would put refs_be_files in refs.h too.  But I can remove
> refs_be_files and just stick it in the places it's needed.

It's OK then. It doesn't hurt to leave it for consistency's sake.

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] merge: fix cache_entry use-after-free

[Junio C Hamano]

David Turner  writes:

>> This one smelled iffy.  I think it is safe because the caller does
>> not look at src[] other than src[0] after this function returns, and
>> this setting to NULL happens only when o->merge is set to 1, so I do
>> not think this is buggy, but at the same time I do not think setting
>> to NULL is necessary.
>> 
>> Other than that, looks nice.  Thanks.
>
> I like to set a pointer to NULL after I free the thing pointed to by it,
> because it helps make use-after-free bugs easier to detect.  

Yes, but it also helps unintended bugs to creep in if done blindly,
and that is why I vetted what happens in the codepath of the caller
of this function after src[] entries are NULLed (the caller could be
expecting to do things only to NULLed fields, for example, in which
case clearing them like this patch would have changed the behaviour
of the caller after this function returns).

Thanks.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Add fetch.recurseSubmoduleParallelism config option

[Junio C Hamano]

Stefan Beller  writes:

> Assuming we go with your second school of thought (N are the real
> running processes, M including the finished but still pending output tasks),

That's neither of my two, I would think.

Anyway, I think it is now clear that it not very easy to say "I know
what to fill in M and N" there ;-).
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 02/44] refs: make repack_without_refs and is_branch public

[David Turner]

On Tue, 2015-10-13 at 09:23 +0200, Michael Haggerty wrote:
> On 10/13/2015 04:39 AM, Michael Haggerty wrote:
> > On 10/12/2015 11:51 PM, David Turner wrote:
> >> is_branch was already non-static, but this patch declares it in the
> >> header.
> >>
> >> Signed-off-by: Ronnie Sahlberg 
> >> Signed-off-by: David Turner 
> >> ---
> >> [...]
> > 
> > It seems odd that repack_without_refs() should be made public (and
> > ultimately end up in refs.h) given that it intrinsically only has to do
> > with file-based references. But I will read on...
> 
> I think the reason you needed to do this was because you wanted to move
> delete_refs() to the common code. It is true that delete_ref() can be
> moved to the common code. And most of the code in delete_refs() is just
> a loop calling delete_ref(). But delete_refs() also does the very
> files-specific optimization of calling repack_without_refs() before the
> loop. *That* call shouldn't be in the common code.
> 
> So my suggestion is that you write a common_delete_refs() function that
> only includes the loop over delete_ref(), and a files_delete_refs()
> function that is basically
> 
> {
> result = repack_without_refs(refnames, );
> if (result) {
> ...report error...
> return result;
> }
> return common_delete_refs(...);
> }

OK, I can do that.  That will have to be part of the backends work, so
I'll exclude it from the refs-backend-pre-vtable patch set.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: How to rebase when some commit hashes are in some commit messages

[Jacob Keller]

On Tue, Oct 13, 2015 at 12:24 PM, Philip Oakley  wrote:
> IIUC (as an alternate example),  in G4W one can submit a (long) pull request
> with internal back references that would be merged directly, so the sha1's
> could be updated as Francois-Xavier originally asked. I have a series that's
> been bumping along for a long while that needs regular rebasing, though
> doesn't have sha1 back references, so I can see that the need does happen. I
> can see that others may have a workflow that would work well with the sha1
> auto-update.
>
> --
> Philip
>

I still don't see how this is useful, because the part that *can* be
implemented is not valuable and the part that is valuable can't be
implemented.

So, what we can implement easily enough:

you rebase a series and any time the message contains sha1 of a commit
we're modifying in this rebase, we update the sha1 to match again.
This seems reasonable, but not useful. Why would you reference a
commit that is *ITSELF* being rebased. No one has explained a
reasonable use for this... I'm sure there exists one, but I would want
an explanation of one first.

The "useful" case is if you rebase "onto" a tree that has a previous
history that has been changed. In this case, how do you propose we
find it. Doing as suggested above, ie: only changing sha1s that we are
already rebasing works, but why are you backreferencing it if you are
re-writing the commit? That doesn't make sense to me at all. Yes, you
can do it, but I don't get why this is valuable. If you're backref is
"fixes xyz" why not just fix xyz instead of have two commits. If the
back ref has some other value... what is that value? I don't
understand it I guess.

It just seems pretty narrow focus. I mean if someone wants to
implement it, that is fine.

Regards,
Jake
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 18/44] refs: move transaction functions into common code

[David Turner]

On Tue, 2015-10-13 at 13:29 +0200, Michael Haggerty wrote:
> I reviewed the patches up to here pretty carefully, and aside from the
> comments I already sent, they look good.
> 
> I like the new approach where the ref_transaction-building code is
> shared across backends.
> 
> It seems to me that a good breaking point for the first batch of patches
> would be here, just before you start creating the VTABLE in commit
> [19/44]. The patches before this point all have to do with moving code
> around and a little bit of light refactoring. They cause a lot of code
> churn that will soon conflict with other people's work (e.g., [1]), but
> I think they are pretty uncontroversial.
> 
> After this you start making a few important design decisions that
> *could* be controversial. Therefore, by making a cut, you can maximize
> the chance that the earlier patches can be merged to master relatively
> quickly, after which the cross section for future conflicts will be much
> smaller.
> 
> Ideally, you would include a few later patches in the "pre-VTABLE" patch
> series. It looks like the following patches also mostly have to do with
> moving code around, and would fit logically with the "pre-VTABLE" changes:
> 
> [24] refs.c: move refname_is_safe to the common code
> [25] refs.h: document make refname_is_safe and add it to header
> [26] refs.c: move copy_msg to the common code
> [27] refs.c: move peel_object to the common code
> [28] refs.c: move should_autocreate_reflog to common code
> [32] initdb: move safe_create_dir into common code
> [36] refs: make files_log_ref_write functions public
> [37] refs: break out a ref conflict check
> 
> I tried rebasing those commits on top of your patch 18 and it wasn't too
> bad. The result is on branch "refs-backend-pre-vtable" on my GitHub repo
> [2], including my suggested changes to those eight patches (which
> therefore became seven because I squashed the first two together).

Thanks.  I started from that, and made the changes that you suggested 
in reviews that were not yet in there.

I also added Jeff's extension patch, since it seems uncontroversial to
me, and since we'll need it sooner or later anyway.

I put the result on my github at:
https://github.com/dturner-tw/git
on the refs-backend-pre-vtable branch 




--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 23/26] mailinfo: handle errors found in decode_header() better

[Junio C Hamano]

The decode_header() function tries to unwrap RFC2047-style quoted
strings but punts when it finds anything it cannot parse.  Allow the
function to report if it punted to the caller, and use the resulting
string in the caller only when the function says it parsed the input
successfully.

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 27 ---
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 4b9b1cc..6452a4c 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -293,7 +293,7 @@ static void cleanup_space(struct strbuf *sb)
}
 }
 
-static void decode_header(struct mailinfo *mi, struct strbuf *line);
+static int decode_header(struct mailinfo *mi, struct strbuf *line);
 static const char *header[MAX_HDR_PARSED] = {
"From","Subject","Date",
 };
@@ -336,8 +336,8 @@ static int check_header(struct mailinfo *mi,
 * normalize the meta information to utf8.
 */
strbuf_add(, line->buf + len + 2, line->len - len - 
2);
-   decode_header(mi, );
-   handle_header(_data[i], );
+   if (!decode_header(mi, ))
+   handle_header(_data[i], );
ret = 1;
goto check_header_out;
}
@@ -347,25 +347,26 @@ static int check_header(struct mailinfo *mi,
if (cmp_header(line, "Content-Type")) {
len = strlen("Content-Type: ");
strbuf_add(, line->buf + len, line->len - len);
-   decode_header(mi, );
-   strbuf_insert(, 0, "Content-Type: ", len);
-   handle_content_type(mi, );
+   if (!decode_header(mi, )) {
+   strbuf_insert(, 0, "Content-Type: ", len);
+   handle_content_type(mi, );
+   }
ret = 1;
goto check_header_out;
}
if (cmp_header(line, "Content-Transfer-Encoding")) {
len = strlen("Content-Transfer-Encoding: ");
strbuf_add(, line->buf + len, line->len - len);
-   decode_header(mi, );
-   handle_content_transfer_encoding(mi, );
+   if (!decode_header(mi, ))
+   handle_content_transfer_encoding(mi, );
ret = 1;
goto check_header_out;
}
if (cmp_header(line, "Message-Id")) {
len = strlen("Message-Id: ");
strbuf_add(, line->buf + len, line->len - len);
-   decode_header(mi, );
-   handle_message_id(mi, );
+   if (!decode_header(mi, ))
+   handle_message_id(mi, );
ret = 1;
goto check_header_out;
}
@@ -537,11 +538,12 @@ static void convert_to_utf8(struct mailinfo *mi,
strbuf_attach(line, out, strlen(out), strlen(out));
 }
 
-static void decode_header(struct mailinfo *mi, struct strbuf *it)
+static int decode_header(struct mailinfo *mi, struct strbuf *it)
 {
char *in, *ep, *cp;
struct strbuf outbuf = STRBUF_INIT, *dec;
struct strbuf charset_q = STRBUF_INIT, piecebuf = STRBUF_INIT;
+   int found_error = -1; /* pessimism */
 
in = it->buf;
while (in - it->buf <= it->len && (ep = strstr(in, "=?")) != NULL) {
@@ -610,10 +612,13 @@ static void decode_header(struct mailinfo *mi, struct 
strbuf *it)
strbuf_addstr(, in);
strbuf_reset(it);
strbuf_addbuf(it, );
+   found_error = 0;
 release_return:
strbuf_release();
strbuf_release(_q);
strbuf_release();
+
+   return found_error;
 }
 
 static void decode_transfer_encoding(struct mailinfo *mi, struct strbuf *line)
-- 
2.6.1-320-g86a1181

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 09/26] mailinfo: move keep_subject & keep_non_patch_bracket to struct mailinfo

[Junio C Hamano]

These two are the only easy ones that do not require passing the
structure around to deep corners of the callchain.

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 7b61187..d642b0d 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -9,13 +9,13 @@
 
 static FILE *cmitmsg, *patchfile, *fin, *fout;
 
-static int keep_subject;
-static int keep_non_patch_brackets_in_subject;
 static const char *metainfo_charset;
 
 struct mailinfo {
struct strbuf name;
struct strbuf email;
+   int keep_subject;
+   int keep_non_patch_brackets_in_subject;
 };
 static char *message_id;
 
@@ -224,7 +224,7 @@ static int is_multipart_boundary(const struct strbuf *line)
!memcmp(line->buf, (*content_top)->buf, (*content_top)->len));
 }
 
-static void cleanup_subject(struct strbuf *subject)
+static void cleanup_subject(struct mailinfo *mi, struct strbuf *subject)
 {
size_t at = 0;
 
@@ -252,7 +252,7 @@ static void cleanup_subject(struct strbuf *subject)
if (!pos)
break;
remove = pos - subject->buf + at + 1;
-   if (!keep_non_patch_brackets_in_subject ||
+   if (!mi->keep_non_patch_brackets_in_subject ||
(7 <= remove &&
 memmem(subject->buf + at, remove, "PATCH", 5)))
strbuf_remove(subject, at, remove);
@@ -976,8 +976,8 @@ static void handle_info(struct mailinfo *mi)
continue;
 
if (!strcmp(header[i], "Subject")) {
-   if (!keep_subject) {
-   cleanup_subject(hdr);
+   if (!mi->keep_subject) {
+   cleanup_subject(mi, hdr);
cleanup_space(hdr);
}
output_header_lines(fout, "Subject", hdr);
@@ -1071,9 +1071,9 @@ int cmd_mailinfo(int argc, const char **argv, const char 
*prefix)
 
while (1 < argc && argv[1][0] == '-') {
if (!strcmp(argv[1], "-k"))
-   keep_subject = 1;
+   mi.keep_subject = 1;
else if (!strcmp(argv[1], "-b"))
-   keep_non_patch_brackets_in_subject = 1;
+   mi.keep_non_patch_brackets_in_subject = 1;
else if (!strcmp(argv[1], "-m") || !strcmp(argv[1], 
"--message-id"))
add_message_id = 1;
else if (!strcmp(argv[1], "-u"))
-- 
2.6.1-320-g86a1181

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 04/26] mailinfo: move handle_boundary() lower

[Junio C Hamano]

This function wants to call find_boundary() and is called only from
one place without any recursing, so it becomes easier to read if it
appears after the called function.

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 114 ++---
 1 file changed, 56 insertions(+), 58 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 3b015a5..175c6ae 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -626,64 +626,6 @@ static void decode_transfer_encoding(struct strbuf *line)
free(ret);
 }
 
-static void handle_filter(struct strbuf *line);
-
-static int find_boundary(void)
-{
-   while (!strbuf_getline(, fin, '\n')) {
-   if (*content_top && is_multipart_boundary())
-   return 1;
-   }
-   return 0;
-}
-
-static int handle_boundary(void)
-{
-   struct strbuf newline = STRBUF_INIT;
-
-   strbuf_addch(, '\n');
-again:
-   if (line.len >= (*content_top)->len + 2 &&
-   !memcmp(line.buf + (*content_top)->len, "--", 2)) {
-   /* we hit an end boundary */
-   /* pop the current boundary off the stack */
-   strbuf_release(*content_top);
-   free(*content_top);
-   *content_top = NULL;
-
-   /* technically won't happen as is_multipart_boundary()
-  will fail first.  But just in case..
-*/
-   if (--content_top < content) {
-   fprintf(stderr, "Detected mismatched boundaries, "
-   "can't recover\n");
-   exit(1);
-   }
-   handle_filter();
-   strbuf_release();
-
-   /* skip to the next boundary */
-   if (!find_boundary())
-   return 0;
-   goto again;
-   }
-
-   /* set some defaults */
-   transfer_encoding = TE_DONTCARE;
-   strbuf_reset();
-
-   /* slurp in this section's info */
-   while (read_one_header_line(, fin))
-   check_header(, p_hdr_data, 0);
-
-   strbuf_release();
-   /* replenish line */
-   if (strbuf_getline(, fin, '\n'))
-   return 0;
-   strbuf_addch(, '\n');
-   return 1;
-}
-
 static inline int patchbreak(const struct strbuf *line)
 {
size_t i;
@@ -879,6 +821,62 @@ static void handle_filter(struct strbuf *line)
}
 }
 
+static int find_boundary(void)
+{
+   while (!strbuf_getline(, fin, '\n')) {
+   if (*content_top && is_multipart_boundary())
+   return 1;
+   }
+   return 0;
+}
+
+static int handle_boundary(void)
+{
+   struct strbuf newline = STRBUF_INIT;
+
+   strbuf_addch(, '\n');
+again:
+   if (line.len >= (*content_top)->len + 2 &&
+   !memcmp(line.buf + (*content_top)->len, "--", 2)) {
+   /* we hit an end boundary */
+   /* pop the current boundary off the stack */
+   strbuf_release(*content_top);
+   free(*content_top);
+   *content_top = NULL;
+
+   /* technically won't happen as is_multipart_boundary()
+  will fail first.  But just in case..
+*/
+   if (--content_top < content) {
+   fprintf(stderr, "Detected mismatched boundaries, "
+   "can't recover\n");
+   exit(1);
+   }
+   handle_filter();
+   strbuf_release();
+
+   /* skip to the next boundary */
+   if (!find_boundary())
+   return 0;
+   goto again;
+   }
+
+   /* set some defaults */
+   transfer_encoding = TE_DONTCARE;
+   strbuf_reset();
+
+   /* slurp in this section's info */
+   while (read_one_header_line(, fin))
+   check_header(, p_hdr_data, 0);
+
+   strbuf_release();
+   /* replenish line */
+   if (strbuf_getline(, fin, '\n'))
+   return 0;
+   strbuf_addch(, '\n');
+   return 1;
+}
+
 static void handle_body(void)
 {
struct strbuf prev = STRBUF_INIT;
-- 
2.6.1-320-g86a1181

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 26/26] mailinfo: libify the whole thing

[Junio C Hamano]

With this, the builtin "git am" can hopefully make an internal call
to the mailinfo() function without going through the run_command()
interface, whose overhead is heavyweight compared to what mailinfo()
itself does on some platforms.

Signed-off-by: Junio C Hamano 
---
 Makefile   |1 +
 builtin/mailinfo.c | 1091 +---
 mailinfo.c | 1058 ++
 mailinfo.h |   41 ++
 4 files changed, 1101 insertions(+), 1090 deletions(-)
 create mode 100644 mailinfo.c
 create mode 100644 mailinfo.h

diff --git a/Makefile b/Makefile
index 8d5df7e..7dd3bff 100644
--- a/Makefile
+++ b/Makefile
@@ -726,6 +726,7 @@ LIB_OBJS += list-objects.o
 LIB_OBJS += ll-merge.o
 LIB_OBJS += lockfile.o
 LIB_OBJS += log-tree.o
+LIB_OBJS += mailinfo.o
 LIB_OBJS += mailmap.o
 LIB_OBJS += match-trees.o
 LIB_OBJS += merge.o
diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index eb9ff96..f6df274 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -6,1096 +6,7 @@
 #include "builtin.h"
 #include "utf8.h"
 #include "strbuf.h"
-
-#define MAX_BOUNDARIES 5
-
-struct mailinfo {
-   FILE *input;
-   FILE *output;
-   FILE *patchfile;
-
-   struct strbuf name;
-   struct strbuf email;
-   int keep_subject;
-   int keep_non_patch_brackets_in_subject;
-   int add_message_id;
-   int use_scissors;
-   int use_inbody_headers; /* defaults to 1 */
-   const char *metainfo_charset;
-
-   struct strbuf *content[MAX_BOUNDARIES];
-   struct strbuf **content_top;
-   struct strbuf charset;
-   char *message_id;
-   enum  {
-   TE_DONTCARE, TE_QP, TE_BASE64
-   } transfer_encoding;
-   int patch_lines;
-   int filter_stage; /* still reading log or are we copying patch? */
-   int header_stage; /* still checking in-body headers? */
-   struct strbuf **p_hdr_data;
-   struct strbuf **s_hdr_data;
-
-   struct strbuf log_message;
-   int input_error;
-};
-
-#define MAX_HDR_PARSED 10
-
-static void cleanup_space(struct strbuf *sb);
-
-
-static void get_sane_name(struct strbuf *out, struct strbuf *name, struct 
strbuf *email)
-{
-   struct strbuf *src = name;
-   if (name->len < 3 || 60 < name->len || strchr(name->buf, '@') ||
-   strchr(name->buf, '<') || strchr(name->buf, '>'))
-   src = email;
-   else if (name == out)
-   return;
-   strbuf_reset(out);
-   strbuf_addbuf(out, src);
-}
-
-static void parse_bogus_from(struct mailinfo *mi, const struct strbuf *line)
-{
-   /* John Doe  */
-
-   char *bra, *ket;
-   /* This is fallback, so do not bother if we already have an
-* e-mail address.
-*/
-   if (mi->email.len)
-   return;
-
-   bra = strchr(line->buf, '<');
-   if (!bra)
-   return;
-   ket = strchr(bra, '>');
-   if (!ket)
-   return;
-
-   strbuf_reset(>email);
-   strbuf_add(>email, bra + 1, ket - bra - 1);
-
-   strbuf_reset(>name);
-   strbuf_add(>name, line->buf, bra - line->buf);
-   strbuf_trim(>name);
-   get_sane_name(>name, >name, >email);
-}
-
-static void handle_from(struct mailinfo *mi, const struct strbuf *from)
-{
-   char *at;
-   size_t el;
-   struct strbuf f;
-
-   strbuf_init(, from->len);
-   strbuf_addbuf(, from);
-
-   at = strchr(f.buf, '@');
-   if (!at) {
-   parse_bogus_from(mi, from);
-   return;
-   }
-
-   /*
-* If we already have one email, don't take any confusing lines
-*/
-   if (mi->email.len && strchr(at + 1, '@')) {
-   strbuf_release();
-   return;
-   }
-
-   /* Pick up the string around '@', possibly delimited with <>
-* pair; that is the email part.
-*/
-   while (at > f.buf) {
-   char c = at[-1];
-   if (isspace(c))
-   break;
-   if (c == '<') {
-   at[-1] = ' ';
-   break;
-   }
-   at--;
-   }
-   el = strcspn(at, " \n\t\r\v\f>");
-   strbuf_reset(>email);
-   strbuf_add(>email, at, el);
-   strbuf_remove(, at - f.buf, el + (at[el] ? 1 : 0));
-
-   /* The remainder is name.  It could be
-*
-* - "John Doe "   (a), or
-* - "john.doe@xz (John Doe)"   (b), or
-* - "John (zzz) Doe  (Comment)"   (c)
-*
-* but we have removed the email part, so
-*
-* - remove extra spaces which could stay after email (case 'c'), and
-* - trim from both ends, possibly removing the () pair at the end
-*   (cases 'a' and 'b').
-*/
-   cleanup_space();
-   strbuf_trim();
-   if (f.buf[0] == '(' && f.len && 

[PATCH 07/26] mailinfo: move global "line" into mailinfo() function

[Junio C Hamano]

The mailinfo() function is the only one that wants the "line_global"
to be directly touchable.  Note that handle_body() has to be passed
this strbuf so that it sees the "first line of the input" after the
loop in this function processes the headers.  It feels a bit dirty
that handle_body() then keeps reusing this strbuf to read more lines
and does its processing, but that is how the code is structured now.

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index c3c7d67..6c671fb 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -12,7 +12,6 @@ static FILE *cmitmsg, *patchfile, *fin, *fout;
 static int keep_subject;
 static int keep_non_patch_brackets_in_subject;
 static const char *metainfo_charset;
-static struct strbuf line_global = STRBUF_INIT;
 static struct strbuf name = STRBUF_INIT;
 static struct strbuf email = STRBUF_INIT;
 static char *message_id;
@@ -995,6 +994,8 @@ static void handle_info(void)
 static int mailinfo(FILE *in, FILE *out, const char *msg, const char *patch)
 {
int peek;
+   struct strbuf line = STRBUF_INIT;
+
fin = in;
fout = out;
 
@@ -1019,10 +1020,10 @@ static int mailinfo(FILE *in, FILE *out, const char 
*msg, const char *patch)
ungetc(peek, in);
 
/* process the email header */
-   while (read_one_header_line(_global, fin))
-   check_header(_global, p_hdr_data, 1);
+   while (read_one_header_line(, fin))
+   check_header(, p_hdr_data, 1);
 
-   handle_body(_global);
+   handle_body();
handle_info();
 
return 0;
-- 
2.6.1-320-g86a1181

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 08/26] mailinfo: introduce "struct mailinfo" to hold globals

[Junio C Hamano]

Initially have only 'email' and 'name' fields in there and remove
the corresponding globals.  In subsequent patches, more globals will
be moved to this and the structure will be passed around as a new
parameter to more functions.

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 61 +-
 1 file changed, 37 insertions(+), 24 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 6c671fb..7b61187 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -12,8 +12,11 @@ static FILE *cmitmsg, *patchfile, *fin, *fout;
 static int keep_subject;
 static int keep_non_patch_brackets_in_subject;
 static const char *metainfo_charset;
-static struct strbuf name = STRBUF_INIT;
-static struct strbuf email = STRBUF_INIT;
+
+struct mailinfo {
+   struct strbuf name;
+   struct strbuf email;
+};
 static char *message_id;
 
 static enum  {
@@ -45,7 +48,7 @@ static void get_sane_name(struct strbuf *out, struct strbuf 
*name, struct strbuf
strbuf_addbuf(out, src);
 }
 
-static void parse_bogus_from(const struct strbuf *line)
+static void parse_bogus_from(struct mailinfo *mi, const struct strbuf *line)
 {
/* John Doe  */
 
@@ -53,7 +56,7 @@ static void parse_bogus_from(const struct strbuf *line)
/* This is fallback, so do not bother if we already have an
 * e-mail address.
 */
-   if (email.len)
+   if (mi->email.len)
return;
 
bra = strchr(line->buf, '<');
@@ -63,16 +66,16 @@ static void parse_bogus_from(const struct strbuf *line)
if (!ket)
return;
 
-   strbuf_reset();
-   strbuf_add(, bra + 1, ket - bra - 1);
+   strbuf_reset(>email);
+   strbuf_add(>email, bra + 1, ket - bra - 1);
 
-   strbuf_reset();
-   strbuf_add(, line->buf, bra - line->buf);
-   strbuf_trim();
-   get_sane_name(, , );
+   strbuf_reset(>name);
+   strbuf_add(>name, line->buf, bra - line->buf);
+   strbuf_trim(>name);
+   get_sane_name(>name, >name, >email);
 }
 
-static void handle_from(const struct strbuf *from)
+static void handle_from(struct mailinfo *mi, const struct strbuf *from)
 {
char *at;
size_t el;
@@ -83,14 +86,14 @@ static void handle_from(const struct strbuf *from)
 
at = strchr(f.buf, '@');
if (!at) {
-   parse_bogus_from(from);
+   parse_bogus_from(mi, from);
return;
}
 
/*
 * If we already have one email, don't take any confusing lines
 */
-   if (email.len && strchr(at + 1, '@')) {
+   if (mi->email.len && strchr(at + 1, '@')) {
strbuf_release();
return;
}
@@ -109,8 +112,8 @@ static void handle_from(const struct strbuf *from)
at--;
}
el = strcspn(at, " \n\t\r\v\f>");
-   strbuf_reset();
-   strbuf_add(, at, el);
+   strbuf_reset(>email);
+   strbuf_add(>email, at, el);
strbuf_remove(, at - f.buf, el + (at[el] ? 1 : 0));
 
/* The remainder is name.  It could be
@@ -132,7 +135,7 @@ static void handle_from(const struct strbuf *from)
strbuf_setlen(, f.len - 1);
}
 
-   get_sane_name(, , );
+   get_sane_name(>name, , >email);
strbuf_release();
 }
 
@@ -958,7 +961,7 @@ static void output_header_lines(FILE *fout, const char 
*hdr, const struct strbuf
}
 }
 
-static void handle_info(void)
+static void handle_info(struct mailinfo *mi)
 {
struct strbuf *hdr;
int i;
@@ -980,9 +983,9 @@ static void handle_info(void)
output_header_lines(fout, "Subject", hdr);
} else if (!strcmp(header[i], "From")) {
cleanup_space(hdr);
-   handle_from(hdr);
-   fprintf(fout, "Author: %s\n", name.buf);
-   fprintf(fout, "Email: %s\n", email.buf);
+   handle_from(mi, hdr);
+   fprintf(fout, "Author: %s\n", mi->name.buf);
+   fprintf(fout, "Email: %s\n", mi->email.buf);
} else {
cleanup_space(hdr);
fprintf(fout, "%s: %s\n", header[i], hdr->buf);
@@ -991,7 +994,8 @@ static void handle_info(void)
fprintf(fout, "\n");
 }
 
-static int mailinfo(FILE *in, FILE *out, const char *msg, const char *patch)
+static int mailinfo(struct mailinfo *mi,
+   FILE *in, FILE *out, const char *msg, const char *patch)
 {
int peek;
struct strbuf line = STRBUF_INIT;
@@ -1024,7 +1028,7 @@ static int mailinfo(FILE *in, FILE *out, const char *msg, 
const char *patch)
check_header(, p_hdr_data, 1);
 
handle_body();
-   handle_info();
+   handle_info(mi);
 
return 0;
 }
@@ -1041,17 +1045,26 @@ static int git_mailinfo_config(const char *var, const 
char 

[PATCH 05/26] mailinfo: get rid of function-local static states

[Junio C Hamano]

Two helper functions use "static int" in their scope to keep track
of the state while repeatedly getting called once for each input
line.  Move these state variables their ultimate caller and pass
down pointers to them, as a small step in preparation for making
this entire callchain more reentrant.

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 48 +++-
 1 file changed, 23 insertions(+), 25 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 175c6ae..5a74811 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -713,21 +713,20 @@ static int is_scissors_line(const struct strbuf *line)
gap * 2 < perforation);
 }
 
-static int handle_commit_msg(struct strbuf *line)
+static int handle_commit_msg(struct strbuf *line, int *still_looking)
 {
/*
 * Are we still scanning and discarding in-body headers?
 * It is initially set to 1, set to 2 when we do see a
 * valid in-body header.
 */
-   static int still_looking = 1;
int is_empty_line;
 
if (!cmitmsg)
return 0;
 
is_empty_line = (!line->len || (line->len == 1 && line->buf[0] == 
'\n'));
-   if (still_looking == 1) {
+   if (*still_looking == 1) {
/*
 * Haven't seen a known in-body header; discard an empty line.
 */
@@ -735,33 +734,33 @@ static int handle_commit_msg(struct strbuf *line)
return 0;
}
 
-   if (use_inbody_headers && still_looking) {
+   if (use_inbody_headers && *still_looking) {
int is_known_header = check_header(line, s_hdr_data, 0);
 
-   if (still_looking == 2) {
+   if (*still_looking == 2) {
/*
 * an empty line after the in-body header block,
 * or a line obviously not an attempt to invent
 * an unsupported in-body header.
 */
if (is_empty_line || !is_rfc2822_header(line))
-   still_looking = 0;
+   *still_looking = 0;
if (is_empty_line)
return 0;
/* otherwise do not discard the line, but keep going */
} else if (is_known_header) {
-   still_looking = 2;
-   } else if (still_looking != 2) {
-   still_looking = 0;
+   *still_looking = 2;
+   } else if (*still_looking != 2) {
+   *still_looking = 0;
}
 
-   if (still_looking)
+   if (*still_looking)
return 0;
} else
/* Only trim the first (blank) line of the commit message
 * when ignoring in-body headers.
 */
-   still_looking = 0;
+   *still_looking = 0;
 
/* normalize the log message to UTF-8. */
if (metainfo_charset)
@@ -773,7 +772,7 @@ static int handle_commit_msg(struct strbuf *line)
die_errno("Could not rewind output message file");
if (ftruncate(fileno(cmitmsg), 0))
die_errno("Could not truncate output message file at 
scissors");
-   still_looking = 1;
+   *still_looking = 1;
 
/*
 * We may have already read "secondary headers"; purge
@@ -805,16 +804,13 @@ static void handle_patch(const struct strbuf *line)
patch_lines++;
 }
 
-static void handle_filter(struct strbuf *line)
+static void handle_filter(struct strbuf *line, int *filter_stage, int 
*header_stage)
 {
-   static int filter = 0;
-
-   /* filter tells us which part we left off on */
-   switch (filter) {
+   switch (*filter_stage) {
case 0:
-   if (!handle_commit_msg(line))
+   if (!handle_commit_msg(line, header_stage))
break;
-   filter++;
+   (*filter_stage)++;
case 1:
handle_patch(line);
break;
@@ -830,7 +826,7 @@ static int find_boundary(void)
return 0;
 }
 
-static int handle_boundary(void)
+static int handle_boundary(int *filter_stage, int *header_stage)
 {
struct strbuf newline = STRBUF_INIT;
 
@@ -852,7 +848,7 @@ again:
"can't recover\n");
exit(1);
}
-   handle_filter();
+   handle_filter(, filter_stage, header_stage);
strbuf_release();
 
/* skip to the next boundary */
@@ -880,6 +876,8 @@ again:
 static void handle_body(void)
 {
struct strbuf prev = STRBUF_INIT;
+   int filter_stage = 0;
+   int header_stage = 1;
 
  

[PATCH 20/26] mailinfo: move [ps]_hdr_data to struct mailinfo

[Junio C Hamano]

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 28 ++--
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index b591a2f..c9629c8 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -30,10 +30,10 @@ struct mailinfo {
int patch_lines;
int filter_stage; /* still reading log or are we copying patch? */
int header_stage; /* still checking in-body headers? */
+   struct strbuf **p_hdr_data;
+   struct strbuf **s_hdr_data;
 };
 
-static struct strbuf **p_hdr_data, **s_hdr_data;
-
 #define MAX_HDR_PARSED 10
 #define MAX_BOUNDARIES 5
 
@@ -743,7 +743,7 @@ static int handle_commit_msg(struct mailinfo *mi, struct 
strbuf *line)
}
 
if (mi->use_inbody_headers && mi->header_stage) {
-   int is_known_header = check_header(mi, line, s_hdr_data, 0);
+   int is_known_header = check_header(mi, line, mi->s_hdr_data, 0);
 
if (mi->header_stage == 2) {
/*
@@ -786,9 +786,9 @@ static int handle_commit_msg(struct mailinfo *mi, struct 
strbuf *line)
 * them to give ourselves a clean restart.
 */
for (i = 0; header[i]; i++) {
-   if (s_hdr_data[i])
-   strbuf_release(s_hdr_data[i]);
-   s_hdr_data[i] = NULL;
+   if (mi->s_hdr_data[i])
+   strbuf_release(mi->s_hdr_data[i]);
+   mi->s_hdr_data[i] = NULL;
}
return 0;
}
@@ -870,7 +870,7 @@ again:
 
/* slurp in this section's info */
while (read_one_header_line(line, mi->input))
-   check_header(mi, line, p_hdr_data, 0);
+   check_header(mi, line, mi->p_hdr_data, 0);
 
strbuf_release();
/* replenish line */
@@ -971,10 +971,10 @@ static void handle_info(struct mailinfo *mi)
 
for (i = 0; header[i]; i++) {
/* only print inbody headers if we output a patch file */
-   if (mi->patch_lines && s_hdr_data[i])
-   hdr = s_hdr_data[i];
-   else if (p_hdr_data[i])
-   hdr = p_hdr_data[i];
+   if (mi->patch_lines && mi->s_hdr_data[i])
+   hdr = mi->s_hdr_data[i];
+   else if (mi->p_hdr_data[i])
+   hdr = mi->p_hdr_data[i];
else
continue;
 
@@ -1014,8 +1014,8 @@ static int mailinfo(struct mailinfo *mi, const char *msg, 
const char *patch)
return -1;
}
 
-   p_hdr_data = xcalloc(MAX_HDR_PARSED, sizeof(*p_hdr_data));
-   s_hdr_data = xcalloc(MAX_HDR_PARSED, sizeof(*s_hdr_data));
+   mi->p_hdr_data = xcalloc(MAX_HDR_PARSED, sizeof(*(mi->p_hdr_data)));
+   mi->s_hdr_data = xcalloc(MAX_HDR_PARSED, sizeof(*(mi->s_hdr_data)));
 
do {
peek = fgetc(mi->input);
@@ -1024,7 +1024,7 @@ static int mailinfo(struct mailinfo *mi, const char *msg, 
const char *patch)
 
/* process the email header */
while (read_one_header_line(, mi->input))
-   check_header(mi, , p_hdr_data, 1);
+   check_header(mi, , mi->p_hdr_data, 1);
 
handle_body(mi, );
handle_info(mi);
-- 
2.6.1-320-g86a1181

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

bug report: `git stash save -u` deletes directory whose contents are .gitignored

[lennart spitzner]

hi,

- `git stash save -u` deletes a directory, even though the _contents_
of that directory are .gitignored (e.g. "foo/*" in .gitignore).
Detailed reproduction below.
- The behaviour is not present when instead .gitignoring the directory
itself (e.g. "foo"). This does imo not excuse the behaviour of the
described case.
- The behaviour is not present with just `git stash save`.
- The expected behaviour is: Only files listed in `git status` are
stashed away; other things are left untouched. Potential exception:
empty directories.
- I am aware that `git stash save -u` is supposed to have semantics
involving the equivalent of `reset --hard` and `clean -f`. Neither of
those expose the directory-deletion behaviour.

additional comments, not directly relevant to this bug:
- both `git stash save; git stash pop` and `git stash save -u; git
stash pop` should ideally behave as an "identity effect". The latter
might affect the index (i.e. have an effect equivalent to `git reset`
or something in that direction). Any other effect / special cases are
bad interface design imo. If a differing design was chosen on purpose,
i am thankful for pointers on the reasoning.
- no, i luckily did not truely lose files due to the current behaviour.

lennart



reproduction:

> git init
Initialized empty Git repository in $SOMEDIR/.git/
> mkdir ignore-contents
> echo "ignore-contents/*" > .gitignore
> git add .gitignore
> git commit -m "initial" > /dev/null
> touch ignore-contents/blah.txt
> touch abc.txt
> git status
On branch master
Untracked files:
  (use "git add ..." to include in what will be committed)

  abc.txt

nothing added to commit but untracked files present (use "git add" to
track)
> ls -1a
.
..
abc.txt
.git
.gitignore
ignore-contents
> git stash save -u
Saved working directory and index state WIP on master: adc468b initial
HEAD is now at adc468b initial
> ls -1a
.
..
.git
.gitignore
> git stash pop &> /dev/null
> ls -1a
.
..
abc.txt
.git
.gitignore
> git --version
git version 2.6.1
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 18/44] refs: move transaction functions into common code

[David Turner]

On Tue, 2015-10-13 at 18:21 -0400, David Turner wrote:
> On Tue, 2015-10-13 at 13:29 +0200, Michael Haggerty wrote:
> > I reviewed the patches up to here pretty carefully, and aside from the
> > comments I already sent, they look good.
> > 
> > I like the new approach where the ref_transaction-building code is
> > shared across backends.
> > 
> > It seems to me that a good breaking point for the first batch of patches
> > would be here, just before you start creating the VTABLE in commit
> > [19/44]. The patches before this point all have to do with moving code
> > around and a little bit of light refactoring. They cause a lot of code
> > churn that will soon conflict with other people's work (e.g., [1]), but
> > I think they are pretty uncontroversial.
> > 
> > After this you start making a few important design decisions that
> > *could* be controversial. Therefore, by making a cut, you can maximize
> > the chance that the earlier patches can be merged to master relatively
> > quickly, after which the cross section for future conflicts will be much
> > smaller.
> > 
> > Ideally, you would include a few later patches in the "pre-VTABLE" patch
> > series. It looks like the following patches also mostly have to do with
> > moving code around, and would fit logically with the "pre-VTABLE" changes:
> > 
> > [24] refs.c: move refname_is_safe to the common code
> > [25] refs.h: document make refname_is_safe and add it to header
> > [26] refs.c: move copy_msg to the common code
> > [27] refs.c: move peel_object to the common code
> > [28] refs.c: move should_autocreate_reflog to common code
> > [32] initdb: move safe_create_dir into common code
> > [36] refs: make files_log_ref_write functions public
> > [37] refs: break out a ref conflict check
> > 
> > I tried rebasing those commits on top of your patch 18 and it wasn't too
> > bad. The result is on branch "refs-backend-pre-vtable" on my GitHub repo
> > [2], including my suggested changes to those eight patches (which
> > therefore became seven because I squashed the first two together).
> 
> Thanks.  I started from that, and made the changes that you suggested 
> in reviews that were not yet in there.
> 
> I also added Jeff's extension patch, since it seems uncontroversial to
> me, and since we'll need it sooner or later anyway.
> 
> I put the result on my github at:
> https://github.com/dturner-tw/git
> on the refs-backend-pre-vtable branch 

While rebasing the rest of the series on this, I noticed an issue in one
of the patches, which I have now fixed.  I re-pushed.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 22/26] mailinfo: move content/content_top to struct mailinfo

[Junio C Hamano]

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 45 ++---
 1 file changed, 26 insertions(+), 19 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index d38d716..4b9b1cc 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -7,6 +7,8 @@
 #include "utf8.h"
 #include "strbuf.h"
 
+#define MAX_BOUNDARIES 5
+
 struct mailinfo {
FILE *input;
FILE *output;
@@ -21,6 +23,8 @@ struct mailinfo {
int use_inbody_headers; /* defaults to 1 */
const char *metainfo_charset;
 
+   struct strbuf *content[MAX_BOUNDARIES];
+   struct strbuf **content_top;
struct strbuf charset;
char *message_id;
enum  {
@@ -36,7 +40,6 @@ struct mailinfo {
 };
 
 #define MAX_HDR_PARSED 10
-#define MAX_BOUNDARIES 5
 
 static void cleanup_space(struct strbuf *sb);
 
@@ -181,10 +184,6 @@ static int slurp_attr(const char *line, const char *name, 
struct strbuf *attr)
return 1;
 }
 
-static struct strbuf *content[MAX_BOUNDARIES];
-
-static struct strbuf **content_top = content;
-
 static void handle_content_type(struct mailinfo *mi, struct strbuf *line)
 {
struct strbuf *boundary = xmalloc(sizeof(struct strbuf));
@@ -192,11 +191,11 @@ static void handle_content_type(struct mailinfo *mi, 
struct strbuf *line)
 
if (slurp_attr(line->buf, "boundary=", boundary)) {
strbuf_insert(boundary, 0, "--", 2);
-   if (++content_top >= [MAX_BOUNDARIES]) {
+   if (++mi->content_top >= >content[MAX_BOUNDARIES]) {
fprintf(stderr, "Too many boundaries to handle\n");
exit(1);
}
-   *content_top = boundary;
+   *(mi->content_top) = boundary;
boundary = NULL;
}
slurp_attr(line->buf, "charset=", >charset);
@@ -224,10 +223,12 @@ static void handle_content_transfer_encoding(struct 
mailinfo *mi,
mi->transfer_encoding = TE_DONTCARE;
 }
 
-static int is_multipart_boundary(const struct strbuf *line)
+static int is_multipart_boundary(struct mailinfo *mi, const struct strbuf 
*line)
 {
-   return (((*content_top)->len <= line->len) &&
-   !memcmp(line->buf, (*content_top)->buf, (*content_top)->len));
+   struct strbuf *content_top = *(mi->content_top);
+
+   return ((content_top->len <= line->len) &&
+   !memcmp(line->buf, content_top->buf, content_top->len));
 }
 
 static void cleanup_subject(struct mailinfo *mi, struct strbuf *subject)
@@ -825,7 +826,7 @@ static void handle_filter(struct mailinfo *mi, struct 
strbuf *line)
 static int find_boundary(struct mailinfo *mi, struct strbuf *line)
 {
while (!strbuf_getline(line, mi->input, '\n')) {
-   if (*content_top && is_multipart_boundary(line))
+   if (*(mi->content_top) && is_multipart_boundary(mi, line))
return 1;
}
return 0;
@@ -837,18 +838,18 @@ static int handle_boundary(struct mailinfo *mi, struct 
strbuf *line)
 
strbuf_addch(, '\n');
 again:
-   if (line->len >= (*content_top)->len + 2 &&
-   !memcmp(line->buf + (*content_top)->len, "--", 2)) {
+   if (line->len >= (*(mi->content_top))->len + 2 &&
+   !memcmp(line->buf + (*(mi->content_top))->len, "--", 2)) {
/* we hit an end boundary */
/* pop the current boundary off the stack */
-   strbuf_release(*content_top);
-   free(*content_top);
-   *content_top = NULL;
+   strbuf_release(*(mi->content_top));
+   free(*(mi->content_top));
+   *(mi->content_top) = NULL;
 
/* technically won't happen as is_multipart_boundary()
   will fail first.  But just in case..
 */
-   if (--content_top < content) {
+   if (--mi->content_top < mi->content) {
fprintf(stderr, "Detected mismatched boundaries, "
"can't recover\n");
exit(1);
@@ -883,14 +884,14 @@ static void handle_body(struct mailinfo *mi, struct 
strbuf *line)
struct strbuf prev = STRBUF_INIT;
 
/* Skip up to the first boundary */
-   if (*content_top) {
+   if (*(mi->content_top)) {
if (!find_boundary(mi, line))
goto handle_body_out;
}
 
do {
/* process any boundary lines */
-   if (*content_top && is_multipart_boundary(line)) {
+   if (*(mi->content_top) && is_multipart_boundary(mi, line)) {
/* flush any leftover */
if (prev.len) {
handle_filter(mi, );
@@ -1057,6 +1058,7 @@ static void setup_mailinfo(struct mailinfo *mi)
strbuf_init(>log_message, 0);
mi->header_stage = 

[PATCH 19/26] mailinfo: move cmitmsg and patchfile to struct mailinfo

[Junio C Hamano]

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 28 ++--
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 256d04a..b591a2f 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -7,11 +7,11 @@
 #include "utf8.h"
 #include "strbuf.h"
 
-static FILE *cmitmsg, *patchfile;
-
 struct mailinfo {
FILE *input;
FILE *output;
+   FILE *cmitmsg;
+   FILE *patchfile;
 
struct strbuf name;
struct strbuf email;
@@ -775,9 +775,9 @@ static int handle_commit_msg(struct mailinfo *mi, struct 
strbuf *line)
 
if (mi->use_scissors && is_scissors_line(line)) {
int i;
-   if (fseek(cmitmsg, 0L, SEEK_SET))
+   if (fseek(mi->cmitmsg, 0L, SEEK_SET))
die_errno("Could not rewind output message file");
-   if (ftruncate(fileno(cmitmsg), 0))
+   if (ftruncate(fileno(mi->cmitmsg), 0))
die_errno("Could not truncate output message file at 
scissors");
mi->header_stage = 1;
 
@@ -795,19 +795,19 @@ static int handle_commit_msg(struct mailinfo *mi, struct 
strbuf *line)
 
if (patchbreak(line)) {
if (mi->message_id)
-   fprintf(cmitmsg, "Message-Id: %s\n", mi->message_id);
-   fclose(cmitmsg);
-   cmitmsg = NULL;
+   fprintf(mi->cmitmsg, "Message-Id: %s\n", 
mi->message_id);
+   fclose(mi->cmitmsg);
+   mi->cmitmsg = NULL;
return 1;
}
 
-   fputs(line->buf, cmitmsg);
+   fputs(line->buf, mi->cmitmsg);
return 0;
 }
 
 static void handle_patch(struct mailinfo *mi, const struct strbuf *line)
 {
-   fwrite(line->buf, 1, line->len, patchfile);
+   fwrite(line->buf, 1, line->len, mi->patchfile);
mi->patch_lines++;
 }
 
@@ -1002,15 +1002,15 @@ static int mailinfo(struct mailinfo *mi, const char 
*msg, const char *patch)
int peek;
struct strbuf line = STRBUF_INIT;
 
-   cmitmsg = fopen(msg, "w");
-   if (!cmitmsg) {
+   mi->cmitmsg = fopen(msg, "w");
+   if (!mi->cmitmsg) {
perror(msg);
return -1;
}
-   patchfile = fopen(patch, "w");
-   if (!patchfile) {
+   mi->patchfile = fopen(patch, "w");
+   if (!mi->patchfile) {
perror(patch);
-   fclose(cmitmsg);
+   fclose(mi->cmitmsg);
return -1;
}
 
-- 
2.6.1-320-g86a1181

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: How to rebase when some commit hashes are in some commit messages

[Philip Oakley]

From: "Jacob Keller" 

On Tue, Oct 13, 2015 at 12:24 PM, Philip Oakley 
wrote:

IIUC (as an alternate example),  in G4W one can submit a (long) pull
request
with internal back references that would be merged directly, so the
sha1's
could be updated as Francois-Xavier originally asked. I have a series
that's
been bumping along for a long while that needs regular rebasing, though
doesn't have sha1 back references, so I can see that the need does
happen. I
can see that others may have a workflow that would work well with the
sha1
auto-update.

--
Philip



I still don't see how this is useful, because the part that *can* be
implemented is not valuable and the part that is valuable can't be
implemented.

So, what we can implement easily enough:

you rebase a series and any time the message contains sha1 of a commit
we're modifying in this rebase, we update the sha1 to match again.
This seems reasonable, but not useful. Why would you reference a
commit that is *ITSELF* being rebased. No one has explained a
reasonable use for this... I'm sure there exists one, but I would want
an explanation of one first.


This particular case is about self-references within a long series. At the
moment, on the Git list there is general comments about say [PATCH v3
18/44], whichs is great for for the list ($gmane) but not for a `git log`.
In flows where PRs are valid, one can have what was [34/44] refering to
prior patch [26/44] as `deadbeaf` or whatever. It won't be suitable for most
flows but will be useful for a proportion (as already evidenced by the
request).


The "useful" case is if you rebase "onto" a tree that has a previous
history that has been changed. In this case, how do you propose we
find it.


This use case (where upstream also rebases) hasn't been considered. It would
be a tricky one. As long as the possibility (of such an A depends on B
re-write) isn't closed off then the smaller requested case could still go
ahead.


Doing as suggested above, ie: only changing sha1s that we are
already rebasing works, but why are you backreferencing it if you are
re-writing the commit?


Essentially one wants to say `$CURR_COMMIT~nn` (i.e. "see nn commits
earlier in my series") and have that replaced with its cannonical sha1, and
updated when rebased.
It sort of begs the question whether there should be a ref shorthand for
"the (this) current commit" to allow THIS~ as an interpretable [valid?]
format.


That doesn't make sense to me at all. Yes, you
can do it, but I don't get why this is valuable.



If you're backref is
"fixes xyz" why not just fix xyz instead of have two commits. If the
back ref has some other value... what is that value? I don't
understand it I guess.

For the 'fixes' (of a bug report) case we are already talking about an
immutable so it would not be part of this.
Its use may be more of the type "Using helper function xyz introduced
earlier in patch abcde", which would change after each rebase.



It just seems pretty narrow focus. I mean if someone wants to
implement it, that is fine.


Agreed
--
Philip 


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 06/26] mailinfo: always pass "line" as an argument

[Junio C Hamano]

Some functions in this module accessed the global "struct strbuf
line" while many others used a strbuf passed as an argument.
Convert the former to ensure that nobody deeper in the callchains
relies on the global one.

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 48 
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 5a74811..c3c7d67 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -12,7 +12,7 @@ static FILE *cmitmsg, *patchfile, *fin, *fout;
 static int keep_subject;
 static int keep_non_patch_brackets_in_subject;
 static const char *metainfo_charset;
-static struct strbuf line = STRBUF_INIT;
+static struct strbuf line_global = STRBUF_INIT;
 static struct strbuf name = STRBUF_INIT;
 static struct strbuf email = STRBUF_INIT;
 static char *message_id;
@@ -817,23 +817,23 @@ static void handle_filter(struct strbuf *line, int 
*filter_stage, int *header_st
}
 }
 
-static int find_boundary(void)
+static int find_boundary(struct strbuf *line)
 {
-   while (!strbuf_getline(, fin, '\n')) {
-   if (*content_top && is_multipart_boundary())
+   while (!strbuf_getline(line, fin, '\n')) {
+   if (*content_top && is_multipart_boundary(line))
return 1;
}
return 0;
 }
 
-static int handle_boundary(int *filter_stage, int *header_stage)
+static int handle_boundary(struct strbuf *line, int *filter_stage, int 
*header_stage)
 {
struct strbuf newline = STRBUF_INIT;
 
strbuf_addch(, '\n');
 again:
-   if (line.len >= (*content_top)->len + 2 &&
-   !memcmp(line.buf + (*content_top)->len, "--", 2)) {
+   if (line->len >= (*content_top)->len + 2 &&
+   !memcmp(line->buf + (*content_top)->len, "--", 2)) {
/* we hit an end boundary */
/* pop the current boundary off the stack */
strbuf_release(*content_top);
@@ -852,7 +852,7 @@ again:
strbuf_release();
 
/* skip to the next boundary */
-   if (!find_boundary())
+   if (!find_boundary(line))
return 0;
goto again;
}
@@ -862,18 +862,18 @@ again:
strbuf_reset();
 
/* slurp in this section's info */
-   while (read_one_header_line(, fin))
-   check_header(, p_hdr_data, 0);
+   while (read_one_header_line(line, fin))
+   check_header(line, p_hdr_data, 0);
 
strbuf_release();
/* replenish line */
-   if (strbuf_getline(, fin, '\n'))
+   if (strbuf_getline(line, fin, '\n'))
return 0;
-   strbuf_addch(, '\n');
+   strbuf_addch(line, '\n');
return 1;
 }
 
-static void handle_body(void)
+static void handle_body(struct strbuf *line)
 {
struct strbuf prev = STRBUF_INIT;
int filter_stage = 0;
@@ -881,24 +881,24 @@ static void handle_body(void)
 
/* Skip up to the first boundary */
if (*content_top) {
-   if (!find_boundary())
+   if (!find_boundary(line))
goto handle_body_out;
}
 
do {
/* process any boundary lines */
-   if (*content_top && is_multipart_boundary()) {
+   if (*content_top && is_multipart_boundary(line)) {
/* flush any leftover */
if (prev.len) {
handle_filter(, _stage, 
_stage);
strbuf_reset();
}
-   if (!handle_boundary(_stage, _stage))
+   if (!handle_boundary(line, _stage, 
_stage))
goto handle_body_out;
}
 
/* Unwrap transfer encoding */
-   decode_transfer_encoding();
+   decode_transfer_encoding(line);
 
switch (transfer_encoding) {
case TE_BASE64:
@@ -907,7 +907,7 @@ static void handle_body(void)
struct strbuf **lines, **it, *sb;
 
/* Prepend any previous partial lines */
-   strbuf_insert(, 0, prev.buf, prev.len);
+   strbuf_insert(line, 0, prev.buf, prev.len);
strbuf_reset();
 
/*
@@ -915,7 +915,7 @@ static void handle_body(void)
 * multiple new lines.  Pass only one chunk
 * at a time to handle_filter()
 */
-   lines = strbuf_split(, '\n');
+   lines = strbuf_split(line, '\n');
for (it = lines; (sb = *it); it++) {
if (*(it + 1) == NULL) /* The last line */
if (sb->buf[sb->len - 1] != '\n') {

[PATCH 02/26] mailinfo: fix for off-by-one error in boundary stack

[Junio C Hamano]

We pre-increment the pointer that we will use to store something at,
so the pointer is already beyond the end of the array if it points
at content[MAX_BOUNDARIES].

Signed-off-by: Junio C Hamano 
---

 As always, I am very bad at checking and fixing off-by-one errors.
 A few extra sets of eyeballs are very much appreciated.
---
 builtin/mailinfo.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 330bef4..2742d0d 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -185,7 +185,7 @@ static void handle_content_type(struct strbuf *line)
 
if (slurp_attr(line->buf, "boundary=", boundary)) {
strbuf_insert(boundary, 0, "--", 2);
-   if (++content_top > [MAX_BOUNDARIES]) {
+   if (++content_top >= [MAX_BOUNDARIES]) {
fprintf(stderr, "Too many boundaries to handle\n");
exit(1);
}
-- 
2.6.1-320-g86a1181

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 14/26] mailinfo: move use_scissors and use_inbody_headers to struct mailinfo

[Junio C Hamano]

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 23 +--
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 35e1ab9..5302c03 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -20,6 +20,8 @@ struct mailinfo {
int keep_subject;
int keep_non_patch_brackets_in_subject;
int add_message_id;
+   int use_scissors;
+   int use_inbody_headers; /* defaults to 1 */
 
char *message_id;
int patch_lines;
@@ -34,8 +36,6 @@ static enum  {
 static struct strbuf charset = STRBUF_INIT;
 
 static struct strbuf **p_hdr_data, **s_hdr_data;
-static int use_scissors;
-static int use_inbody_headers = 1;
 
 #define MAX_HDR_PARSED 10
 #define MAX_BOUNDARIES 5
@@ -745,7 +745,7 @@ static int handle_commit_msg(struct mailinfo *mi, struct 
strbuf *line)
return 0;
}
 
-   if (use_inbody_headers && mi->header_stage) {
+   if (mi->use_inbody_headers && mi->header_stage) {
int is_known_header = check_header(mi, line, s_hdr_data, 0);
 
if (mi->header_stage == 2) {
@@ -777,7 +777,7 @@ static int handle_commit_msg(struct mailinfo *mi, struct 
strbuf *line)
if (metainfo_charset)
convert_to_utf8(line, charset.buf);
 
-   if (use_scissors && is_scissors_line(line)) {
+   if (mi->use_scissors && is_scissors_line(line)) {
int i;
if (fseek(cmitmsg, 0L, SEEK_SET))
die_errno("Could not rewind output message file");
@@ -1036,12 +1036,14 @@ static int mailinfo(struct mailinfo *mi, const char 
*msg, const char *patch)
return 0;
 }
 
-static int git_mailinfo_config(const char *var, const char *value, void 
*unused)
+static int git_mailinfo_config(const char *var, const char *value, void *mi_)
 {
+   struct mailinfo *mi = mi_;
+
if (!starts_with(var, "mailinfo."))
-   return git_default_config(var, value, unused);
+   return git_default_config(var, value, NULL);
if (!strcmp(var, "mailinfo.scissors")) {
-   use_scissors = git_config_bool(var, value);
+   mi->use_scissors = git_config_bool(var, value);
return 0;
}
/* perhaps others here */
@@ -1054,6 +1056,7 @@ static void setup_mailinfo(struct mailinfo *mi)
strbuf_init(>name, 0);
strbuf_init(>email, 0);
mi->header_stage = 1;
+   mi->use_inbody_headers = 1;
git_config(git_mailinfo_config, );
 }
 
@@ -1087,11 +1090,11 @@ int cmd_mailinfo(int argc, const char **argv, const 
char *prefix)
else if (starts_with(argv[1], "--encoding="))
metainfo_charset = argv[1] + 11;
else if (!strcmp(argv[1], "--scissors"))
-   use_scissors = 1;
+   mi.use_scissors = 1;
else if (!strcmp(argv[1], "--no-scissors"))
-   use_scissors = 0;
+   mi.use_scissors = 0;
else if (!strcmp(argv[1], "--no-inbody-headers"))
-   use_inbody_headers = 0;
+   mi.use_inbody_headers = 0;
else
usage(mailinfo_usage);
argc--; argv++;
-- 
2.6.1-320-g86a1181

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 13/26] mailinfo: move add_message_id and message_id to struct mailinfo

[Junio C Hamano]

This requires us to pass the structure into check_header() codepath.

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 30 --
 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 163032e..35e1ab9 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -19,12 +19,13 @@ struct mailinfo {
struct strbuf email;
int keep_subject;
int keep_non_patch_brackets_in_subject;
+   int add_message_id;
 
+   char *message_id;
int patch_lines;
int filter_stage; /* still reading log or are we copying patch? */
int header_stage; /* still checking in-body headers? */
 };
-static char *message_id;
 
 static enum  {
TE_DONTCARE, TE_QP, TE_BASE64
@@ -34,7 +35,6 @@ static struct strbuf charset = STRBUF_INIT;
 
 static struct strbuf **p_hdr_data, **s_hdr_data;
 static int use_scissors;
-static int add_message_id;
 static int use_inbody_headers = 1;
 
 #define MAX_HDR_PARSED 10
@@ -209,10 +209,10 @@ static void handle_content_type(struct strbuf *line)
}
 }
 
-static void handle_message_id(const struct strbuf *line)
+static void handle_message_id(struct mailinfo *mi, const struct strbuf *line)
 {
-   if (add_message_id)
-   message_id = strdup(line->buf);
+   if (mi->add_message_id)
+   mi->message_id = strdup(line->buf);
 }
 
 static void handle_content_transfer_encoding(const struct strbuf *line)
@@ -321,11 +321,13 @@ static int is_format_patch_separator(const char *line, 
int len)
return !memcmp(SAMPLE + (cp - line), cp, strlen(SAMPLE) - (cp - line));
 }
 
-static int check_header(const struct strbuf *line,
-   struct strbuf *hdr_data[], int overwrite)
+static int check_header(struct mailinfo *mi,
+   const struct strbuf *line,
+   struct strbuf *hdr_data[], int overwrite)
 {
int i, ret = 0, len;
struct strbuf sb = STRBUF_INIT;
+
/* search for the interesting parts */
for (i = 0; header[i]; i++) {
int len = strlen(header[i]);
@@ -363,7 +365,7 @@ static int check_header(const struct strbuf *line,
len = strlen("Message-Id: ");
strbuf_add(, line->buf + len, line->len - len);
decode_header();
-   handle_message_id();
+   handle_message_id(mi, );
ret = 1;
goto check_header_out;
}
@@ -744,7 +746,7 @@ static int handle_commit_msg(struct mailinfo *mi, struct 
strbuf *line)
}
 
if (use_inbody_headers && mi->header_stage) {
-   int is_known_header = check_header(line, s_hdr_data, 0);
+   int is_known_header = check_header(mi, line, s_hdr_data, 0);
 
if (mi->header_stage == 2) {
/*
@@ -796,8 +798,8 @@ static int handle_commit_msg(struct mailinfo *mi, struct 
strbuf *line)
}
 
if (patchbreak(line)) {
-   if (message_id)
-   fprintf(cmitmsg, "Message-Id: %s\n", message_id);
+   if (mi->message_id)
+   fprintf(cmitmsg, "Message-Id: %s\n", mi->message_id);
fclose(cmitmsg);
cmitmsg = NULL;
return 1;
@@ -872,7 +874,7 @@ again:
 
/* slurp in this section's info */
while (read_one_header_line(line, mi->input))
-   check_header(line, p_hdr_data, 0);
+   check_header(mi, line, p_hdr_data, 0);
 
strbuf_release();
/* replenish line */
@@ -1026,7 +1028,7 @@ static int mailinfo(struct mailinfo *mi, const char *msg, 
const char *patch)
 
/* process the email header */
while (read_one_header_line(, mi->input))
-   check_header(, p_hdr_data, 1);
+   check_header(mi, , p_hdr_data, 1);
 
handle_body(mi, );
handle_info(mi);
@@ -1077,7 +1079,7 @@ int cmd_mailinfo(int argc, const char **argv, const char 
*prefix)
else if (!strcmp(argv[1], "-b"))
mi.keep_non_patch_brackets_in_subject = 1;
else if (!strcmp(argv[1], "-m") || !strcmp(argv[1], 
"--message-id"))
-   add_message_id = 1;
+   mi.add_message_id = 1;
else if (!strcmp(argv[1], "-u"))
metainfo_charset = def_charset;
else if (!strcmp(argv[1], "-n"))
-- 
2.6.1-320-g86a1181

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 11/26] mailinfo: move filter/header stage to struct mailinfo

[Junio C Hamano]

Earlier we got rid of two function-scope static variables that kept
track of the states of helper functions by making them extra arguments
that are passed throughout the callchain.  Now we have a convenient
place to store and pass them around in the form of "struct mailinfo",
change them into two fields in the struct.

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 50 ++
 1 file changed, 26 insertions(+), 24 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index a9da338..7e39769 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -19,6 +19,9 @@ struct mailinfo {
struct strbuf email;
int keep_subject;
int keep_non_patch_brackets_in_subject;
+
+   int filter_stage; /* still reading log or are we copying patch? */
+   int header_stage; /* still checking in-body headers? */
 };
 static char *message_id;
 
@@ -28,6 +31,7 @@ static enum  {
 
 static struct strbuf charset = STRBUF_INIT;
 static int patch_lines;
+
 static struct strbuf **p_hdr_data, **s_hdr_data;
 static int use_scissors;
 static int add_message_id;
@@ -718,7 +722,7 @@ static int is_scissors_line(const struct strbuf *line)
gap * 2 < perforation);
 }
 
-static int handle_commit_msg(struct strbuf *line, int *still_looking)
+static int handle_commit_msg(struct mailinfo *mi, struct strbuf *line)
 {
/*
 * Are we still scanning and discarding in-body headers?
@@ -731,7 +735,7 @@ static int handle_commit_msg(struct strbuf *line, int 
*still_looking)
return 0;
 
is_empty_line = (!line->len || (line->len == 1 && line->buf[0] == 
'\n'));
-   if (*still_looking == 1) {
+   if (mi->header_stage == 1) {
/*
 * Haven't seen a known in-body header; discard an empty line.
 */
@@ -739,33 +743,33 @@ static int handle_commit_msg(struct strbuf *line, int 
*still_looking)
return 0;
}
 
-   if (use_inbody_headers && *still_looking) {
+   if (use_inbody_headers && mi->header_stage) {
int is_known_header = check_header(line, s_hdr_data, 0);
 
-   if (*still_looking == 2) {
+   if (mi->header_stage == 2) {
/*
 * an empty line after the in-body header block,
 * or a line obviously not an attempt to invent
 * an unsupported in-body header.
 */
if (is_empty_line || !is_rfc2822_header(line))
-   *still_looking = 0;
+   mi->header_stage = 0;
if (is_empty_line)
return 0;
/* otherwise do not discard the line, but keep going */
} else if (is_known_header) {
-   *still_looking = 2;
-   } else if (*still_looking != 2) {
-   *still_looking = 0;
+   mi->header_stage = 2;
+   } else if (mi->header_stage != 2) {
+   mi->header_stage = 0;
}
 
-   if (*still_looking)
+   if (mi->header_stage)
return 0;
} else
/* Only trim the first (blank) line of the commit message
 * when ignoring in-body headers.
 */
-   *still_looking = 0;
+   mi->header_stage = 0;
 
/* normalize the log message to UTF-8. */
if (metainfo_charset)
@@ -777,7 +781,7 @@ static int handle_commit_msg(struct strbuf *line, int 
*still_looking)
die_errno("Could not rewind output message file");
if (ftruncate(fileno(cmitmsg), 0))
die_errno("Could not truncate output message file at 
scissors");
-   *still_looking = 1;
+   mi->header_stage = 1;
 
/*
 * We may have already read "secondary headers"; purge
@@ -809,13 +813,13 @@ static void handle_patch(const struct strbuf *line)
patch_lines++;
 }
 
-static void handle_filter(struct strbuf *line, int *filter_stage, int 
*header_stage)
+static void handle_filter(struct mailinfo *mi, struct strbuf *line)
 {
-   switch (*filter_stage) {
+   switch (mi->filter_stage) {
case 0:
-   if (!handle_commit_msg(line, header_stage))
+   if (!handle_commit_msg(mi, line))
break;
-   (*filter_stage)++;
+   mi->filter_stage++;
case 1:
handle_patch(line);
break;
@@ -831,8 +835,7 @@ static int find_boundary(struct mailinfo *mi, struct strbuf 
*line)
return 0;
 }
 
-static int handle_boundary(struct mailinfo *mi, struct strbuf *line,
-  int 

[PATCH 24/26] mailinfo: handle charset conversion errors in the caller

[Junio C Hamano]

Instead of dying in convert_to_utf8(), just report an error and let
the callers handle it.  Between the two callers:

 - decode_header() knows how to handle malformed line by reporting
   the breakage to the caller, so let it follow the pattern it
   already knows about;

 - handle_commit_msg() doesn't cope with a malformed line well, so
   die there for now.  We'll lift this even higher in later changes
   in this series.

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 19 +++
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 6452a4c..f14c504 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -521,21 +521,22 @@ static struct strbuf *decode_b_segment(const struct 
strbuf *b_seg)
return out;
 }
 
-static void convert_to_utf8(struct mailinfo *mi,
-   struct strbuf *line, const char *charset)
+static int convert_to_utf8(struct mailinfo *mi,
+  struct strbuf *line, const char *charset)
 {
char *out;
 
if (!mi->metainfo_charset || !charset || !*charset)
-   return;
+   return 0;
 
if (same_encoding(mi->metainfo_charset, charset))
-   return;
+   return 0;
out = reencode_string(line->buf, mi->metainfo_charset, charset);
if (!out)
-   die("cannot convert from %s to %s",
-   charset, mi->metainfo_charset);
+   return error("cannot convert from %s to %s",
+charset, mi->metainfo_charset);
strbuf_attach(line, out, strlen(out), strlen(out));
+   return 0;
 }
 
 static int decode_header(struct mailinfo *mi, struct strbuf *it)
@@ -602,7 +603,8 @@ static int decode_header(struct mailinfo *mi, struct strbuf 
*it)
dec = decode_q_segment(, 1);
break;
}
-   convert_to_utf8(mi, dec, charset_q.buf);
+   if (convert_to_utf8(mi, dec, charset_q.buf))
+   goto release_return;
 
strbuf_addbuf(, dec);
strbuf_release(dec);
@@ -778,7 +780,8 @@ static int handle_commit_msg(struct mailinfo *mi, struct 
strbuf *line)
mi->header_stage = 0;
 
/* normalize the log message to UTF-8. */
-   convert_to_utf8(mi, line, mi->charset.buf);
+   if (convert_to_utf8(mi, line, mi->charset.buf))
+   exit(128);
 
if (mi->use_scissors && is_scissors_line(line)) {
int i;
-- 
2.6.1-320-g86a1181

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 12/26] mailinfo: move patch_lines to struct mailinfo

[Junio C Hamano]

This one is trivial thanks to previous steps that started passing
the structure throughout the input codepaths.

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 7e39769..163032e 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -20,6 +20,7 @@ struct mailinfo {
int keep_subject;
int keep_non_patch_brackets_in_subject;
 
+   int patch_lines;
int filter_stage; /* still reading log or are we copying patch? */
int header_stage; /* still checking in-body headers? */
 };
@@ -30,7 +31,6 @@ static enum  {
 } transfer_encoding;
 
 static struct strbuf charset = STRBUF_INIT;
-static int patch_lines;
 
 static struct strbuf **p_hdr_data, **s_hdr_data;
 static int use_scissors;
@@ -807,10 +807,10 @@ static int handle_commit_msg(struct mailinfo *mi, struct 
strbuf *line)
return 0;
 }
 
-static void handle_patch(const struct strbuf *line)
+static void handle_patch(struct mailinfo *mi, const struct strbuf *line)
 {
fwrite(line->buf, 1, line->len, patchfile);
-   patch_lines++;
+   mi->patch_lines++;
 }
 
 static void handle_filter(struct mailinfo *mi, struct strbuf *line)
@@ -821,7 +821,7 @@ static void handle_filter(struct mailinfo *mi, struct 
strbuf *line)
break;
mi->filter_stage++;
case 1:
-   handle_patch(line);
+   handle_patch(mi, line);
break;
}
 }
@@ -973,7 +973,7 @@ static void handle_info(struct mailinfo *mi)
 
for (i = 0; header[i]; i++) {
/* only print inbody headers if we output a patch file */
-   if (patch_lines && s_hdr_data[i])
+   if (mi->patch_lines && s_hdr_data[i])
hdr = s_hdr_data[i];
else if (p_hdr_data[i])
hdr = p_hdr_data[i];
-- 
2.6.1-320-g86a1181

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 15/26] mailinfo: move metainfo_charset to struct mailinfo

[Junio C Hamano]

This requires us to pass the struct down to decode_header() and
convert_to_utf8() callchain.

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 40 +++-
 1 file changed, 19 insertions(+), 21 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 5302c03..7e01efa 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -9,8 +9,6 @@
 
 static FILE *cmitmsg, *patchfile;
 
-static const char *metainfo_charset;
-
 struct mailinfo {
FILE *input;
FILE *output;
@@ -22,6 +20,7 @@ struct mailinfo {
int add_message_id;
int use_scissors;
int use_inbody_headers; /* defaults to 1 */
+   const char *metainfo_charset;
 
char *message_id;
int patch_lines;
@@ -293,7 +292,7 @@ static void cleanup_space(struct strbuf *sb)
}
 }
 
-static void decode_header(struct strbuf *line);
+static void decode_header(struct mailinfo *mi, struct strbuf *line);
 static const char *header[MAX_HDR_PARSED] = {
"From","Subject","Date",
 };
@@ -336,7 +335,7 @@ static int check_header(struct mailinfo *mi,
 * normalize the meta information to utf8.
 */
strbuf_add(, line->buf + len + 2, line->len - len - 
2);
-   decode_header();
+   decode_header(mi, );
handle_header(_data[i], );
ret = 1;
goto check_header_out;
@@ -347,7 +346,7 @@ static int check_header(struct mailinfo *mi,
if (cmp_header(line, "Content-Type")) {
len = strlen("Content-Type: ");
strbuf_add(, line->buf + len, line->len - len);
-   decode_header();
+   decode_header(mi, );
strbuf_insert(, 0, "Content-Type: ", len);
handle_content_type();
ret = 1;
@@ -356,7 +355,7 @@ static int check_header(struct mailinfo *mi,
if (cmp_header(line, "Content-Transfer-Encoding")) {
len = strlen("Content-Transfer-Encoding: ");
strbuf_add(, line->buf + len, line->len - len);
-   decode_header();
+   decode_header(mi, );
handle_content_transfer_encoding();
ret = 1;
goto check_header_out;
@@ -364,7 +363,7 @@ static int check_header(struct mailinfo *mi,
if (cmp_header(line, "Message-Id")) {
len = strlen("Message-Id: ");
strbuf_add(, line->buf + len, line->len - len);
-   decode_header();
+   decode_header(mi, );
handle_message_id(mi, );
ret = 1;
goto check_header_out;
@@ -520,23 +519,24 @@ static struct strbuf *decode_b_segment(const struct 
strbuf *b_seg)
return out;
 }
 
-static void convert_to_utf8(struct strbuf *line, const char *charset)
+static void convert_to_utf8(struct mailinfo *mi,
+   struct strbuf *line, const char *charset)
 {
char *out;
 
-   if (!charset || !*charset)
+   if (!mi->metainfo_charset || !charset || !*charset)
return;
 
-   if (same_encoding(metainfo_charset, charset))
+   if (same_encoding(mi->metainfo_charset, charset))
return;
-   out = reencode_string(line->buf, metainfo_charset, charset);
+   out = reencode_string(line->buf, mi->metainfo_charset, charset);
if (!out)
die("cannot convert from %s to %s",
-   charset, metainfo_charset);
+   charset, mi->metainfo_charset);
strbuf_attach(line, out, strlen(out), strlen(out));
 }
 
-static void decode_header(struct strbuf *it)
+static void decode_header(struct mailinfo *mi, struct strbuf *it)
 {
char *in, *ep, *cp;
struct strbuf outbuf = STRBUF_INIT, *dec;
@@ -599,8 +599,7 @@ static void decode_header(struct strbuf *it)
dec = decode_q_segment(, 1);
break;
}
-   if (metainfo_charset)
-   convert_to_utf8(dec, charset_q.buf);
+   convert_to_utf8(mi, dec, charset_q.buf);
 
strbuf_addbuf(, dec);
strbuf_release(dec);
@@ -774,8 +773,7 @@ static int handle_commit_msg(struct mailinfo *mi, struct 
strbuf *line)
mi->header_stage = 0;
 
/* normalize the log message to UTF-8. */
-   if (metainfo_charset)
-   convert_to_utf8(line, charset.buf);
+   convert_to_utf8(mi, line, charset.buf);
 
if (mi->use_scissors && is_scissors_line(line)) {
int i;
@@ -1074,7 +1072,7 @@ int cmd_mailinfo(int argc, const char **argv, const char 
*prefix)
setup_mailinfo();
 
def_charset = get_commit_output_encoding();
-   metainfo_charset = def_charset;
+   mi.metainfo_charset = def_charset;
 

[PATCH 25/26] mailinfo: remove calls to exit() and die() deep in the callchain

[Junio C Hamano]

The top-level mailinfo() would instead punt when the code in the
deeper part of the callchain detects an unrecoverable error in the
input.

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 36 +---
 1 file changed, 25 insertions(+), 11 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index f14c504..eb9ff96 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -37,6 +37,7 @@ struct mailinfo {
struct strbuf **s_hdr_data;
 
struct strbuf log_message;
+   int input_error;
 };
 
 #define MAX_HDR_PARSED 10
@@ -192,8 +193,10 @@ static void handle_content_type(struct mailinfo *mi, 
struct strbuf *line)
if (slurp_attr(line->buf, "boundary=", boundary)) {
strbuf_insert(boundary, 0, "--", 2);
if (++mi->content_top >= >content[MAX_BOUNDARIES]) {
-   fprintf(stderr, "Too many boundaries to handle\n");
-   exit(1);
+   error("Too many boundaries to handle");
+   mi->input_error = -1;
+   mi->content_top = mi->content + MAX_BOUNDARIES - 1;
+   return;
}
*(mi->content_top) = boundary;
boundary = NULL;
@@ -532,9 +535,11 @@ static int convert_to_utf8(struct mailinfo *mi,
if (same_encoding(mi->metainfo_charset, charset))
return 0;
out = reencode_string(line->buf, mi->metainfo_charset, charset);
-   if (!out)
+   if (!out) {
+   mi->input_error = -1;
return error("cannot convert from %s to %s",
 charset, mi->metainfo_charset);
+   }
strbuf_attach(line, out, strlen(out), strlen(out));
return 0;
 }
@@ -781,7 +786,7 @@ static int handle_commit_msg(struct mailinfo *mi, struct 
strbuf *line)
 
/* normalize the log message to UTF-8. */
if (convert_to_utf8(mi, line, mi->charset.buf))
-   exit(128);
+   return 0; /* the caller will reject this input */
 
if (mi->use_scissors && is_scissors_line(line)) {
int i;
@@ -820,6 +825,9 @@ static void handle_patch(struct mailinfo *mi, const struct 
strbuf *line)
 
 static void handle_filter(struct mailinfo *mi, struct strbuf *line)
 {
+   if (mi->input_error)
+   return;
+
switch (mi->filter_stage) {
case 0:
if (!handle_commit_msg(mi, line))
@@ -858,12 +866,15 @@ again:
   will fail first.  But just in case..
 */
if (--mi->content_top < mi->content) {
-   fprintf(stderr, "Detected mismatched boundaries, "
-   "can't recover\n");
-   exit(1);
+   error("Detected mismatched boundaries, can't recover");
+   mi->input_error = -1;
+   mi->content_top = mi->content;
+   return 0;
}
handle_filter(mi, );
strbuf_release();
+   if (mi->input_error)
+   return 0; /* punt to the end */
 
/* skip to the next boundary */
if (!find_boundary(mi, line))
@@ -948,6 +959,8 @@ static void handle_body(struct mailinfo *mi, struct strbuf 
*line)
handle_filter(mi, line);
}
 
+   if (mi->input_error)
+   break;
} while (!strbuf_getwholeline(line, mi->input, '\n'));
 
 handle_body_out:
@@ -1035,12 +1048,13 @@ static int mailinfo(struct mailinfo *mi, const char 
*msg, const char *patch)
check_header(mi, , mi->p_hdr_data, 1);
 
handle_body(mi, );
-   handle_info(mi);
-
-   fwrite(mi->log_message.buf, 1, mi->log_message.len, cmitmsg);
+   if (!mi->input_error) {
+   handle_info(mi);
+   fwrite(mi->log_message.buf, 1, mi->log_message.len, cmitmsg);
+   }
fclose(cmitmsg);
 
-   return 0;
+   return mi->input_error;
 }
 
 static int git_mailinfo_config(const char *var, const char *value, void *mi_)
-- 
2.6.1-320-g86a1181

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 18/26] mailinfo: handle_commit_msg() shouldn't be called after finding patchbreak

[Junio C Hamano]

There is a strange "if (!cmitmsg) return 0" at the very beginning of
handle_commit_msg(), but the condition should never trigger,
because:

 * The only place cmitmsg is set to NULL is after this function sees
   a patch break, closes the FILE * to write the commit log message
   and returns 1.  This function returns non-zero only from that
   codepath.

 * The caller of this function, upon seeing a non-zero return,
   increments filter_stage, starts treating the input as patch text
   and will never call handle_commit_msg() again.

Replace it with an assert(!mi->filter_stage).

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index a51b2c5..256d04a 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -731,8 +731,7 @@ static int handle_commit_msg(struct mailinfo *mi, struct 
strbuf *line)
 */
int is_empty_line;
 
-   if (!cmitmsg)
-   return 0;
+   assert(!mi->filter_stage);
 
is_empty_line = (!line->len || (line->len == 1 && line->buf[0] == 
'\n'));
if (mi->header_stage == 1) {
-- 
2.6.1-320-g86a1181

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 21/26] mailinfo: keep the parsed log message in a strbuf

[Junio C Hamano]

When mailinfo() is eventually libified, the calling "git am" still
will have to write out the log message in the "msg" file for hooks
and other users of the information, but at least it does not have to
reopen and reread what was written back if the function kept it in a
strbuf so that the caller can read it from there.

This also removes the need for seeking and truncating the output
file when we see a scissors mark in the input, which in turn reduces
two callsites of die_errno().

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 41 -
 1 file changed, 28 insertions(+), 13 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index c9629c8..d38d716 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -10,7 +10,6 @@
 struct mailinfo {
FILE *input;
FILE *output;
-   FILE *cmitmsg;
FILE *patchfile;
 
struct strbuf name;
@@ -32,6 +31,8 @@ struct mailinfo {
int header_stage; /* still checking in-body headers? */
struct strbuf **p_hdr_data;
struct strbuf **s_hdr_data;
+
+   struct strbuf log_message;
 };
 
 #define MAX_HDR_PARSED 10
@@ -775,10 +776,8 @@ static int handle_commit_msg(struct mailinfo *mi, struct 
strbuf *line)
 
if (mi->use_scissors && is_scissors_line(line)) {
int i;
-   if (fseek(mi->cmitmsg, 0L, SEEK_SET))
-   die_errno("Could not rewind output message file");
-   if (ftruncate(fileno(mi->cmitmsg), 0))
-   die_errno("Could not truncate output message file at 
scissors");
+
+   strbuf_setlen(>log_message, 0);
mi->header_stage = 1;
 
/*
@@ -795,13 +794,12 @@ static int handle_commit_msg(struct mailinfo *mi, struct 
strbuf *line)
 
if (patchbreak(line)) {
if (mi->message_id)
-   fprintf(mi->cmitmsg, "Message-Id: %s\n", 
mi->message_id);
-   fclose(mi->cmitmsg);
-   mi->cmitmsg = NULL;
+   strbuf_addf(>log_message,
+   "Message-Id: %s\n", mi->message_id);
return 1;
}
 
-   fputs(line->buf, mi->cmitmsg);
+   strbuf_addbuf(>log_message, line);
return 0;
 }
 
@@ -999,18 +997,19 @@ static void handle_info(struct mailinfo *mi)
 
 static int mailinfo(struct mailinfo *mi, const char *msg, const char *patch)
 {
+   FILE *cmitmsg;
int peek;
struct strbuf line = STRBUF_INIT;
 
-   mi->cmitmsg = fopen(msg, "w");
-   if (!mi->cmitmsg) {
+   cmitmsg = fopen(msg, "w");
+   if (!cmitmsg) {
perror(msg);
return -1;
}
mi->patchfile = fopen(patch, "w");
if (!mi->patchfile) {
perror(patch);
-   fclose(mi->cmitmsg);
+   fclose(cmitmsg);
return -1;
}
 
@@ -1029,6 +1028,9 @@ static int mailinfo(struct mailinfo *mi, const char *msg, 
const char *patch)
handle_body(mi, );
handle_info(mi);
 
+   fwrite(mi->log_message.buf, 1, mi->log_message.len, cmitmsg);
+   fclose(cmitmsg);
+
return 0;
 }
 
@@ -1052,11 +1054,20 @@ static void setup_mailinfo(struct mailinfo *mi)
strbuf_init(>name, 0);
strbuf_init(>email, 0);
strbuf_init(>charset, 0);
+   strbuf_init(>log_message, 0);
mi->header_stage = 1;
mi->use_inbody_headers = 1;
git_config(git_mailinfo_config, );
 }
 
+static void clear_mailinfo(struct mailinfo *mi)
+{
+   strbuf_release(>name);
+   strbuf_release(>email);
+   strbuf_release(>charset);
+   strbuf_release(>log_message);
+}
+
 static const char mailinfo_usage[] =
"git mailinfo [-k | -b] [-m | --message-id] [-u | --encoding= 
| -n] [--scissors | --no-scissors]   < mail >info";
 
@@ -1064,6 +1075,7 @@ int cmd_mailinfo(int argc, const char **argv, const char 
*prefix)
 {
const char *def_charset;
struct mailinfo mi;
+   int status;
 
/* NEEDSWORK: might want to do the optional .git/ directory
 * discovery
@@ -1102,5 +1114,8 @@ int cmd_mailinfo(int argc, const char **argv, const char 
*prefix)
 
mi.input = stdin;
mi.output = stdout;
-   return !!mailinfo(, argv[1], argv[2]);
+   status = !!mailinfo(, argv[1], argv[2]);
+   clear_mailinfo();
+
+   return status;
 }
-- 
2.6.1-320-g86a1181

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 03/26] mailinfo: fold decode_header_bq() into decode_header()

[Junio C Hamano]

In olden days we might have wanted to behave differently in
decode_header() if the header line was encoded with RFC2047, but we
apparently do not do so, hence this helper function can go, together
with its return value.

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 23 +++
 1 file changed, 7 insertions(+), 16 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 2742d0d..3b015a5 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -525,19 +525,17 @@ static void convert_to_utf8(struct strbuf *line, const 
char *charset)
strbuf_attach(line, out, strlen(out), strlen(out));
 }
 
-static int decode_header_bq(struct strbuf *it)
+static void decode_header(struct strbuf *it)
 {
char *in, *ep, *cp;
struct strbuf outbuf = STRBUF_INIT, *dec;
struct strbuf charset_q = STRBUF_INIT, piecebuf = STRBUF_INIT;
-   int rfc2047 = 0;
 
in = it->buf;
while (in - it->buf <= it->len && (ep = strstr(in, "=?")) != NULL) {
int encoding;
strbuf_reset(_q);
strbuf_reset();
-   rfc2047 = 1;
 
if (in != ep) {
/*
@@ -567,22 +565,22 @@ static int decode_header_bq(struct strbuf *it)
ep += 2;
 
if (ep - it->buf >= it->len || !(cp = strchr(ep, '?')))
-   goto decode_header_bq_out;
+   goto release_return;
 
if (cp + 3 - it->buf > it->len)
-   goto decode_header_bq_out;
+   goto release_return;
strbuf_add(_q, ep, cp - ep);
 
encoding = cp[1];
if (!encoding || cp[2] != '?')
-   goto decode_header_bq_out;
+   goto release_return;
ep = strstr(cp + 3, "?=");
if (!ep)
-   goto decode_header_bq_out;
+   goto release_return;
strbuf_add(, cp + 3, ep - cp - 3);
switch (tolower(encoding)) {
default:
-   goto decode_header_bq_out;
+   goto release_return;
case 'b':
dec = decode_b_segment();
break;
@@ -601,17 +599,10 @@ static int decode_header_bq(struct strbuf *it)
strbuf_addstr(, in);
strbuf_reset(it);
strbuf_addbuf(it, );
-decode_header_bq_out:
+release_return:
strbuf_release();
strbuf_release(_q);
strbuf_release();
-   return rfc2047;
-}
-
-static void decode_header(struct strbuf *it)
-{
-   if (decode_header_bq(it))
-   return;
 }
 
 static void decode_transfer_encoding(struct strbuf *line)
-- 
2.6.1-320-g86a1181

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 16/26] mailinfo: move transfer_encoding to struct mailinfo

[Junio C Hamano]

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 27 ++-
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 7e01efa..fbfa27e 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -23,14 +23,14 @@ struct mailinfo {
const char *metainfo_charset;
 
char *message_id;
+   enum  {
+   TE_DONTCARE, TE_QP, TE_BASE64
+   } transfer_encoding;
int patch_lines;
int filter_stage; /* still reading log or are we copying patch? */
int header_stage; /* still checking in-body headers? */
 };
 
-static enum  {
-   TE_DONTCARE, TE_QP, TE_BASE64
-} transfer_encoding;
 
 static struct strbuf charset = STRBUF_INIT;
 
@@ -214,14 +214,15 @@ static void handle_message_id(struct mailinfo *mi, const 
struct strbuf *line)
mi->message_id = strdup(line->buf);
 }
 
-static void handle_content_transfer_encoding(const struct strbuf *line)
+static void handle_content_transfer_encoding(struct mailinfo *mi,
+const struct strbuf *line)
 {
if (strcasestr(line->buf, "base64"))
-   transfer_encoding = TE_BASE64;
+   mi->transfer_encoding = TE_BASE64;
else if (strcasestr(line->buf, "quoted-printable"))
-   transfer_encoding = TE_QP;
+   mi->transfer_encoding = TE_QP;
else
-   transfer_encoding = TE_DONTCARE;
+   mi->transfer_encoding = TE_DONTCARE;
 }
 
 static int is_multipart_boundary(const struct strbuf *line)
@@ -356,7 +357,7 @@ static int check_header(struct mailinfo *mi,
len = strlen("Content-Transfer-Encoding: ");
strbuf_add(, line->buf + len, line->len - len);
decode_header(mi, );
-   handle_content_transfer_encoding();
+   handle_content_transfer_encoding(mi, );
ret = 1;
goto check_header_out;
}
@@ -615,11 +616,11 @@ release_return:
strbuf_release();
 }
 
-static void decode_transfer_encoding(struct strbuf *line)
+static void decode_transfer_encoding(struct mailinfo *mi, struct strbuf *line)
 {
struct strbuf *ret;
 
-   switch (transfer_encoding) {
+   switch (mi->transfer_encoding) {
case TE_QP:
ret = decode_q_segment(line, 0);
break;
@@ -867,7 +868,7 @@ again:
}
 
/* set some defaults */
-   transfer_encoding = TE_DONTCARE;
+   mi->transfer_encoding = TE_DONTCARE;
strbuf_reset();
 
/* slurp in this section's info */
@@ -905,9 +906,9 @@ static void handle_body(struct mailinfo *mi, struct strbuf 
*line)
}
 
/* Unwrap transfer encoding */
-   decode_transfer_encoding(line);
+   decode_transfer_encoding(mi, line);
 
-   switch (transfer_encoding) {
+   switch (mi->transfer_encoding) {
case TE_BASE64:
case TE_QP:
{
-- 
2.6.1-320-g86a1181

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 17/26] mailinfo: move charset to struct mailinfo

[Junio C Hamano]

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 15 +++
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index fbfa27e..a51b2c5 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -22,6 +22,7 @@ struct mailinfo {
int use_inbody_headers; /* defaults to 1 */
const char *metainfo_charset;
 
+   struct strbuf charset;
char *message_id;
enum  {
TE_DONTCARE, TE_QP, TE_BASE64
@@ -31,9 +32,6 @@ struct mailinfo {
int header_stage; /* still checking in-body headers? */
 };
 
-
-static struct strbuf charset = STRBUF_INIT;
-
 static struct strbuf **p_hdr_data, **s_hdr_data;
 
 #define MAX_HDR_PARSED 10
@@ -186,7 +184,7 @@ static struct strbuf *content[MAX_BOUNDARIES];
 
 static struct strbuf **content_top = content;
 
-static void handle_content_type(struct strbuf *line)
+static void handle_content_type(struct mailinfo *mi, struct strbuf *line)
 {
struct strbuf *boundary = xmalloc(sizeof(struct strbuf));
strbuf_init(boundary, line->len);
@@ -200,7 +198,7 @@ static void handle_content_type(struct strbuf *line)
*content_top = boundary;
boundary = NULL;
}
-   slurp_attr(line->buf, "charset=", );
+   slurp_attr(line->buf, "charset=", >charset);
 
if (boundary) {
strbuf_release(boundary);
@@ -349,7 +347,7 @@ static int check_header(struct mailinfo *mi,
strbuf_add(, line->buf + len, line->len - len);
decode_header(mi, );
strbuf_insert(, 0, "Content-Type: ", len);
-   handle_content_type();
+   handle_content_type(mi, );
ret = 1;
goto check_header_out;
}
@@ -774,7 +772,7 @@ static int handle_commit_msg(struct mailinfo *mi, struct 
strbuf *line)
mi->header_stage = 0;
 
/* normalize the log message to UTF-8. */
-   convert_to_utf8(mi, line, charset.buf);
+   convert_to_utf8(mi, line, mi->charset.buf);
 
if (mi->use_scissors && is_scissors_line(line)) {
int i;
@@ -869,7 +867,7 @@ again:
 
/* set some defaults */
mi->transfer_encoding = TE_DONTCARE;
-   strbuf_reset();
+   strbuf_reset(>charset);
 
/* slurp in this section's info */
while (read_one_header_line(line, mi->input))
@@ -1054,6 +1052,7 @@ static void setup_mailinfo(struct mailinfo *mi)
memset(mi, 0, sizeof(*mi));
strbuf_init(>name, 0);
strbuf_init(>email, 0);
+   strbuf_init(>charset, 0);
mi->header_stage = 1;
mi->use_inbody_headers = 1;
git_config(git_mailinfo_config, );
-- 
2.6.1-320-g86a1181

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 00/26] mailinfo libification

[Junio C Hamano]

So here is an attempt to libify "git mailinfo" so that the built-in
version of "git am" does not have to run it via run_command()
interface.  "git am", when fed an N-patch series, runs one
"mailsplit", N "mailinfo" and N "apply" all via run_command()
interface (plus 2 more "apply" and 1 "merge-recursive" per a patch
that does not apply cleanly, when run with the "-3" option), and
among the various programs spawned from "git am", "mailinfo" is the
most straight-forward, stupid and light-weight program, so it is a
no-brainer to pick it as the candidate for libification.

This goes on top of c5920b21 (mailinfo: ignore in-body header that
we do not care about, 2015-10-08) that was posted earlier as a
weatherbaloon patch.

Junio C Hamano (26):
  mailinfo: remove a no-op call convert_to_utf8(it, "")
  mailinfo: fix for off-by-one error in boundary stack
  mailinfo: fold decode_header_bq() into decode_header()
  mailinfo: move handle_boundary() lower
  mailinfo: get rid of function-local static states
  mailinfo: always pass "line" as an argument
  mailinfo: move global "line" into mailinfo() function
  mailinfo: introduce "struct mailinfo" to hold globals
  mailinfo: move keep_subject & keep_non_patch_bracket to struct mailinfo
  mailinfo: move global "FILE *fin, *fout" to struct mailinfo
  mailinfo: move filter/header stage to struct mailinfo
  mailinfo: move patch_lines to struct mailinfo
  mailinfo: move add_message_id and message_id to struct mailinfo
  mailinfo: move use_scissors and use_inbody_headers to struct mailinfo
  mailinfo: move metainfo_charset to struct mailinfo
  mailinfo: move transfer_encoding to struct mailinfo
  mailinfo: move charset to struct mailinfo
  mailinfo: handle_commit_msg() shouldn't be called after finding patchbreak
  mailinfo: move cmitmsg and patchfile to struct mailinfo
  mailinfo: move [ps]_hdr_data to struct mailinfo
  mailinfo: keep the parsed log message in a strbuf
  mailinfo: move content/content_top to struct mailinfo
  mailinfo: handle errors found in decode_header() better
  mailinfo: handle charset conversion errors in the caller
  mailinfo: remove calls to exit() and die() deep in the callchain
  mailinfo: libify the whole thing

 Makefile   |1 +
 builtin/mailinfo.c | 1083 +---
 mailinfo.c | 1058 ++
 mailinfo.h |   41 ++
 4 files changed, 1120 insertions(+), 1063 deletions(-)
 create mode 100644 mailinfo.c
 create mode 100644 mailinfo.h

-- 
2.6.1-320-g86a1181

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 10/26] mailinfo: move global "FILE *fin, *fout" to struct mailinfo

[Junio C Hamano]

This requires us to pass "struct mailinfo" to more functions
throughout the codepath that read input lines, which makes
later steps easier.

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 54 --
 1 file changed, 28 insertions(+), 26 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index d642b0d..a9da338 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -7,11 +7,14 @@
 #include "utf8.h"
 #include "strbuf.h"
 
-static FILE *cmitmsg, *patchfile, *fin, *fout;
+static FILE *cmitmsg, *patchfile;
 
 static const char *metainfo_charset;
 
 struct mailinfo {
+   FILE *input;
+   FILE *output;
+
struct strbuf name;
struct strbuf email;
int keep_subject;
@@ -819,16 +822,17 @@ static void handle_filter(struct strbuf *line, int 
*filter_stage, int *header_st
}
 }
 
-static int find_boundary(struct strbuf *line)
+static int find_boundary(struct mailinfo *mi, struct strbuf *line)
 {
-   while (!strbuf_getline(line, fin, '\n')) {
+   while (!strbuf_getline(line, mi->input, '\n')) {
if (*content_top && is_multipart_boundary(line))
return 1;
}
return 0;
 }
 
-static int handle_boundary(struct strbuf *line, int *filter_stage, int 
*header_stage)
+static int handle_boundary(struct mailinfo *mi, struct strbuf *line,
+  int *filter_stage, int *header_stage)
 {
struct strbuf newline = STRBUF_INIT;
 
@@ -854,7 +858,7 @@ again:
strbuf_release();
 
/* skip to the next boundary */
-   if (!find_boundary(line))
+   if (!find_boundary(mi, line))
return 0;
goto again;
}
@@ -864,18 +868,18 @@ again:
strbuf_reset();
 
/* slurp in this section's info */
-   while (read_one_header_line(line, fin))
+   while (read_one_header_line(line, mi->input))
check_header(line, p_hdr_data, 0);
 
strbuf_release();
/* replenish line */
-   if (strbuf_getline(line, fin, '\n'))
+   if (strbuf_getline(line, mi->input, '\n'))
return 0;
strbuf_addch(line, '\n');
return 1;
 }
 
-static void handle_body(struct strbuf *line)
+static void handle_body(struct mailinfo *mi, struct strbuf *line)
 {
struct strbuf prev = STRBUF_INIT;
int filter_stage = 0;
@@ -883,7 +887,7 @@ static void handle_body(struct strbuf *line)
 
/* Skip up to the first boundary */
if (*content_top) {
-   if (!find_boundary(line))
+   if (!find_boundary(mi, line))
goto handle_body_out;
}
 
@@ -895,7 +899,7 @@ static void handle_body(struct strbuf *line)
handle_filter(, _stage, 
_stage);
strbuf_reset();
}
-   if (!handle_boundary(line, _stage, 
_stage))
+   if (!handle_boundary(mi, line, _stage, 
_stage))
goto handle_body_out;
}
 
@@ -938,7 +942,7 @@ static void handle_body(struct strbuf *line)
handle_filter(line, _stage, _stage);
}
 
-   } while (!strbuf_getwholeline(line, fin, '\n'));
+   } while (!strbuf_getwholeline(line, mi->input, '\n'));
 
 handle_body_out:
strbuf_release();
@@ -980,29 +984,25 @@ static void handle_info(struct mailinfo *mi)
cleanup_subject(mi, hdr);
cleanup_space(hdr);
}
-   output_header_lines(fout, "Subject", hdr);
+   output_header_lines(mi->output, "Subject", hdr);
} else if (!strcmp(header[i], "From")) {
cleanup_space(hdr);
handle_from(mi, hdr);
-   fprintf(fout, "Author: %s\n", mi->name.buf);
-   fprintf(fout, "Email: %s\n", mi->email.buf);
+   fprintf(mi->output, "Author: %s\n", mi->name.buf);
+   fprintf(mi->output, "Email: %s\n", mi->email.buf);
} else {
cleanup_space(hdr);
-   fprintf(fout, "%s: %s\n", header[i], hdr->buf);
+   fprintf(mi->output, "%s: %s\n", header[i], hdr->buf);
}
}
-   fprintf(fout, "\n");
+   fprintf(mi->output, "\n");
 }
 
-static int mailinfo(struct mailinfo *mi,
-   FILE *in, FILE *out, const char *msg, const char *patch)
+static int mailinfo(struct mailinfo *mi, const char *msg, const char *patch)
 {
int peek;
struct strbuf line = STRBUF_INIT;
 
-   fin = in;
-   fout = out;
-
cmitmsg = fopen(msg, "w");
if (!cmitmsg) {
perror(msg);
@@ 

[PATCH 01/26] mailinfo: remove a no-op call convert_to_utf8(it, "")

[Junio C Hamano]

The called function checks if the second parameter is either a NULL
or an empty string at the very beginning and returns without doing
anything.  Remove the useless call.

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 169ee54..330bef4 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -612,11 +612,6 @@ static void decode_header(struct strbuf *it)
 {
if (decode_header_bq(it))
return;
-   /* otherwise "it" is a straight copy of the input.
-* This can be binary guck but there is no charset specified.
-*/
-   if (metainfo_charset)
-   convert_to_utf8(it, "");
 }
 
 static void decode_transfer_encoding(struct strbuf *line)
-- 
2.6.1-320-g86a1181

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[BUG] Broken links in Git Documentation/user-manual.txt

[Xue Fuqiao]

Hi list,

In https://git-scm.com/docs/user-manual.html , all links to the
glossary[1] are broken.

I'm aware of a recent bug report about broken links ($gmane/279048/),
but that one seems unrelated to what I'm reporting.

[1] For example: https://git-scm.com/docs/user-manual.html#def_head
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 05/44] refs.c: move update_ref to refs.c

[David Turner]

On Tue, 2015-10-13 at 05:41 +0200, Michael Haggerty wrote:

> The original read
> 
>   if (read_ref(pseudoref, actual_old_sha1))
>   die("could not read ref '%s'", pseudoref);
> 
> This seems like an important test. What happened to it?
> 
> If its removal was intentional, it deserves a careful explanation (and
> should probably be done as a separate commit). If it was an accident,
> please consider how this accident arose and try to think about whether
> similar accidents might have happened elsewhere in this series.

I went ahead and manually rechecked all of the patches to ensure that
nothing else was screwed up.  While doing so, I found two or three
spurious whitespace changes (moving newlines around), which I fixed (in
refs-backend-pre-vtable, which I'll send to the list tomorrow-ish).

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 27/26] mailinfo: close patchfile

[Junio C Hamano]

"git mailinfo" itself did not need this, as open file handles are
flushed and closed upon process exit, but the libified version needs
to clean up after itself when it is done.

Signed-off-by: Junio C Hamano 
---
 mailinfo.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mailinfo.c b/mailinfo.c
index ca40030..00e2ea4 100644
--- a/mailinfo.c
+++ b/mailinfo.c
@@ -1013,7 +1013,7 @@ int mailinfo(struct mailinfo *mi, const char *msg, const 
char *patch)
fwrite(mi->log_message.buf, 1, mi->log_message.len, cmitmsg);
}
fclose(cmitmsg);
-
+   fclose(mi->patchfile);
return mi->input_error;
 }
 
-- 
2.6.1-320-g86a1181

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 28/26] am: make direct call to mailinfo

[Junio C Hamano]

And finally the endgame.  Instead of spawning "git mailinfo" via the
run_command() API the same number of times as there are incoming
patches, make direct internal call to the libified mailinfo() from
"git am" to reduce the spawning overhead, which would matter on some
platforms.

Signed-off-by: Junio C Hamano 
---

 This is to be applied on top of a merge across

  - jc/am-3-fallback-regression-fix
  - jc/mailinfo-lib (i.e. the 27-patch series)

 As I do not have ready access to such a platform with slow
 run_command(), it would be nice to see some numbers produced on
 such platform by other people.

 Thanks.

 builtin/am.c | 38 +++---
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index c869796..0471355 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -27,6 +27,7 @@
 #include "notes-utils.h"
 #include "rerere.h"
 #include "prompt.h"
+#include "mailinfo.h"
 
 /**
  * Returns 1 if the file is empty or does not exist, 0 otherwise.
@@ -1258,58 +1259,57 @@ static void am_append_signoff(struct am_state *state)
 static int parse_mail(struct am_state *state, const char *mail)
 {
FILE *fp;
-   struct child_process cp = CHILD_PROCESS_INIT;
struct strbuf sb = STRBUF_INIT;
struct strbuf msg = STRBUF_INIT;
struct strbuf author_name = STRBUF_INIT;
struct strbuf author_date = STRBUF_INIT;
struct strbuf author_email = STRBUF_INIT;
int ret = 0;
+   struct mailinfo mi;
 
-   cp.git_cmd = 1;
-   cp.in = xopen(mail, O_RDONLY, 0);
-   cp.out = xopen(am_path(state, "info"), O_WRONLY | O_CREAT, 0777);
+   setup_mailinfo();
 
-   argv_array_push(, "mailinfo");
-   argv_array_push(, state->utf8 ? "-u" : "-n");
+   if (state->utf8)
+   mi.metainfo_charset = get_commit_output_encoding();
+   else
+   mi.metainfo_charset = NULL;
 
switch (state->keep) {
case KEEP_FALSE:
break;
case KEEP_TRUE:
-   argv_array_push(, "-k");
+   mi.keep_subject = 1;
break;
case KEEP_NON_PATCH:
-   argv_array_push(, "-b");
+   mi.keep_non_patch_brackets_in_subject = 1;
break;
default:
die("BUG: invalid value for state->keep");
}
-
if (state->message_id)
-   argv_array_push(, "-m");
+   mi.add_message_id = 1;
 
switch (state->scissors) {
case SCISSORS_UNSET:
break;
case SCISSORS_FALSE:
-   argv_array_push(, "--no-scissors");
+   mi.use_scissors = 0;
break;
case SCISSORS_TRUE:
-   argv_array_push(, "--scissors");
+   mi.use_scissors = 1;
break;
default:
die("BUG: invalid value for state->scissors");
}
 
-   argv_array_push(, am_path(state, "msg"));
-   argv_array_push(, am_path(state, "patch"));
+   mi.input = fopen(mail, "r");
+   mi.output = fopen(am_path(state, "info"), "w");
 
-   if (run_command() < 0)
+   if (mailinfo(, am_path(state, "msg"), am_path(state, "patch")))
die("could not parse patch");
 
-   close(cp.in);
-   close(cp.out);
+   fclose(mi.input);
+   fclose(mi.output);
 
/* Extract message and author information */
fp = xfopen(am_path(state, "info"), "r");
@@ -1341,8 +1341,7 @@ static int parse_mail(struct am_state *state, const char 
*mail)
}
 
strbuf_addstr(, "\n\n");
-   if (strbuf_read_file(, am_path(state, "msg"), 0) < 0)
-   die_errno(_("could not read '%s'"), am_path(state, "msg"));
+   strbuf_addbuf(, _message);
stripspace(, 0);
 
if (state->signoff)
@@ -1366,6 +1365,7 @@ finish:
strbuf_release(_email);
strbuf_release(_name);
strbuf_release();
+   clear_mailinfo();
return ret;
 }
 
-- 
2.6.1-320-g86a1181

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 18/44] refs: move transaction functions into common code

[Michael Haggerty]

On 10/12/2015 11:51 PM, David Turner wrote:
> The common ref code will build up a ref transaction.  Backends will
> then commit it.  So the transaction creation and update functions should
> be in the common code.  We also need to move the ref structs into
> the common code so that alternate backends can access them.
> 
> Later, we will modify struct ref_update to support alternate backends.

This patch leaks internal implementation details into the public refs
interface. I want to make sure that code elsewhere in Git treats these
structures as opaque. That is why I suggested creating an extra module
to hold "protected" code for the reference backend "class hierarchy" [1].

For a sketch of what my suggestion would look like, see the two commits
at the tip of branch "refs-be-common" on my GitHub repo [2].

Michael

[1] http://article.gmane.org/gmane.comp.version-control.git/279049
[2] https://github.com/mhagger/git

-- 
Michael Haggerty
mhag...@alum.mit.edu

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html