[PATCH] repack: call prune_packed_objects() and update_server_info() directly

[René Scharfe]

Call the functions behind git prune-packed and git update-server-info
directly instead of using run_command().  This is shorter, easier and
quicker.

Signed-off-by: Rene Scharfe l@web.de
---
 builtin/repack.c | 23 ++-
 1 file changed, 6 insertions(+), 17 deletions(-)

diff --git a/builtin/repack.c b/builtin/repack.c
index fc088db..2aae05d 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -377,6 +377,7 @@ int cmd_repack(int argc, const char **argv, const char 
*prefix)
/* End of pack replacement. */
 
if (delete_redundant) {
+   int opts = 0;
sort_string_list(names);
for_each_string_list_item(item, existing_packs) {
char *sha1;
@@ -387,25 +388,13 @@ int cmd_repack(int argc, const char **argv, const char 
*prefix)
if (!string_list_has_string(names, sha1))
remove_redundant_pack(packdir, item-string);
}
-   argv_array_push(cmd_args, prune-packed);
-   if (quiet)
-   argv_array_push(cmd_args, --quiet);
-
-   memset(cmd, 0, sizeof(cmd));
-   cmd.argv = cmd_args.argv;
-   cmd.git_cmd = 1;
-   run_command(cmd);
-   argv_array_clear(cmd_args);
+   if (!quiet  isatty(2))
+   opts |= PRUNE_PACKED_VERBOSE;
+   prune_packed_objects(opts);
}
 
-   if (!no_update_server_info) {
-   argv_array_push(cmd_args, update-server-info);
-   memset(cmd, 0, sizeof(cmd));
-   cmd.argv = cmd_args.argv;
-   cmd.git_cmd = 1;
-   run_command(cmd);
-   argv_array_clear(cmd_args);
-   }
+   if (!no_update_server_info)
+   update_server_info(0);
remove_temporary_files();
string_list_clear(names, 0);
string_list_clear(rollback, 0);
-- 
2.1.0

--
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 v4 11/32] delete_ref_loose(): don't muck around in the lock_file's filename

[Johannes Sixt]

Am 06.09.2014 um 09:50 schrieb Michael Haggerty:
 It's bad manners.  Especially since, if unlink_or_warn() failed, the
 memory wasn't restored to its original contents.

I do not see how the old code did not restore the file name. Except for
this nit, the patch looks good.

 
 So make our own copy to work with.
 
 Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
 ---
  refs.c | 15 +--
  1 file changed, 9 insertions(+), 6 deletions(-)
 
 diff --git a/refs.c b/refs.c
 index 828522d..8a63073 100644
 --- a/refs.c
 +++ b/refs.c
 @@ -2545,12 +2545,15 @@ static int repack_without_ref(const char *refname)
  static int delete_ref_loose(struct ref_lock *lock, int flag)
  {
   if (!(flag  REF_ISPACKED) || flag  REF_ISSYMREF) {
 - /* loose */
 - int err, i = strlen(lock-lk-filename) - LOCK_SUFFIX_LEN;
 -
 - lock-lk-filename[i] = 0;
 - err = unlink_or_warn(lock-lk-filename);
 - lock-lk-filename[i] = LOCK_SUFFIX[0];
 + /*
 +  * loose.  The loose file name is the same as the
 +  * lockfile name, minus .lock:
 +  */
 + char *loose_filename = xmemdupz(
 + lock-lk-filename,
 + strlen(lock-lk-filename) - LOCK_SUFFIX_LEN);
 + int err = unlink_or_warn(loose_filename);
 + free(loose_filename);
   if (err  errno != ENOENT)
   return 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: [RFC] allowing hooks to ignore input?

[Johannes Sixt]

Am 13.09.2014 um 00:48 schrieb Junio C Hamano:
 The pre-receive and post-receive hooks were designed to be an
 improvement over old style update and post-update hooks that used to
 take the update information on the command line and were limited by
 the command line length limit.  They take the same information from
 their standard input.  It has been mandatory for these new style
 hooks to consume the update information fully from the standard
 input stream.  Otherwise, they would risk killing the receive-pack
 process via SIGPIPE.
 
 This is easy, and it has already been done by existing hooks that
 are written correctly, to work around, if a hook does not want to
 look at all the information, by sending its standard input to
 /dev/null (perhaps a niche use of hook might need to know only the
 fact that a push was made, without having to know what objects have
 been pushed to update which refs).
 
 However, because there is no good way to consistently fail hooks
 that do not consume the input fully, it can lead to a hard to
 diagnose once in a blue moon phantom failure.
 
 I wonder if this you must consume the input fully, which is a
 mandate that is not enforced strictly, is not helping us to catch
 mistakes in hooks more than it is hurting us.  Perhaps we can do
 something like the attached patch to make it optional for them to
 read the input we feed?
 
 I dunno.
 
  builtin/receive-pack.c | 6 ++
  1 file changed, 6 insertions(+)
 
 diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
 index 30560a7..9d9d16d 100644
 --- a/builtin/receive-pack.c
 +++ b/builtin/receive-pack.c
 @@ -17,6 +17,7 @@
  #include version.h
  #include tag.h
  #include gpg-interface.h
 +#include sigchain.h
  
  static const char receive_pack_usage[] = git receive-pack git-dir;
  
 @@ -500,6 +501,8 @@ static int run_and_feed_hook(const char *hook_name, 
 feed_fn feed, void *feed_sta
   return code;
   }
  
 + sigchain_push(SIGPIPE, SIG_IGN);
 +
   while (1) {
   const char *buf;
   size_t n;
 @@ -511,6 +514,9 @@ static int run_and_feed_hook(const char *hook_name, 
 feed_fn feed, void *feed_sta
   close(proc.in);
   if (use_sideband)
   finish_async(muxer);
 +
 + sigchain_pop(SIGPIPE);
 +
   return finish_command(proc);
  }

I think this is a good move. Hooks are written by users, who sometimes
are not clueful enough.

But what do our writers do when they fail with EPIPE? Die? If so, this
patch alone is not sufficient.

-- Hannes

--
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: git-diff-tree --root

[Roman Neuhauser]

# gits...@pobox.com / 2014-09-12 10:31:30 -0700:
 Roman Neuhauser neuhau...@sigpipe.cz writes:
  git-diff-tree without --root is absolutely silent for the root commit,
  and i see no bad effects of --root on non-root commits.  are there any
  hidden gotchas?  IOW, why is the --root behavior not the default?
 
 Because tools that was written before you proposed that change
 expect to see nothing for the root commit, and then you are suddenly
 breaking their expectations?

i'm not proposing anything, i'm just curious why it is this way.
my line of thinking: there must be (or have been) a grave reason to
break the simple consistency, or the current behavior must be very
useful for something and i'm just missing what it is.  the reasons
for the behavior may have been invalidated by further developments,
or it may have been a wrong decision we're stuck with for BC; i'm
just curious about history.

motivation for my question is that i'm scripting git-diff-tree
and i need it to produce the diff for root commits as well.  i like
my scripts as simple as possible, so i'd like to use --root *always*.
is it safe?

-- 
roman
--
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] repack: call prune_packed_objects() and update_server_info() directly

[Stefan Beller]

On 13.09.2014 09:28, René Scharfe wrote:
 Call the functions behind git prune-packed and git update-server-info
 directly instead of using run_command().  This is shorter, easier and
 quicker.
 
 Signed-off-by: Rene Scharfe l@web.de

Thanks for cleaning up the literal rewrite of the shell script
and making it look more like a C program.

Reviewed-by: Stefan Beller stefanbel...@gmail.com

 ---
  builtin/repack.c | 23 ++-
  1 file changed, 6 insertions(+), 17 deletions(-)
 
 diff --git a/builtin/repack.c b/builtin/repack.c
 index fc088db..2aae05d 100644
 --- a/builtin/repack.c
 +++ b/builtin/repack.c
 @@ -377,6 +377,7 @@ int cmd_repack(int argc, const char **argv, const char 
 *prefix)
   /* End of pack replacement. */
  
   if (delete_redundant) {
 + int opts = 0;
   sort_string_list(names);
   for_each_string_list_item(item, existing_packs) {
   char *sha1;
 @@ -387,25 +388,13 @@ int cmd_repack(int argc, const char **argv, const char 
 *prefix)
   if (!string_list_has_string(names, sha1))
   remove_redundant_pack(packdir, item-string);
   }
 - argv_array_push(cmd_args, prune-packed);
 - if (quiet)
 - argv_array_push(cmd_args, --quiet);
 -
 - memset(cmd, 0, sizeof(cmd));
 - cmd.argv = cmd_args.argv;
 - cmd.git_cmd = 1;
 - run_command(cmd);
 - argv_array_clear(cmd_args);
 + if (!quiet  isatty(2))
 + opts |= PRUNE_PACKED_VERBOSE;
 + prune_packed_objects(opts);
   }
  
 - if (!no_update_server_info) {
 - argv_array_push(cmd_args, update-server-info);
 - memset(cmd, 0, sizeof(cmd));
 - cmd.argv = cmd_args.argv;
 - cmd.git_cmd = 1;
 - run_command(cmd);
 - argv_array_clear(cmd_args);
 - }
 + if (!no_update_server_info)
 + update_server_info(0);
   remove_temporary_files();
   string_list_clear(names, 0);
   string_list_clear(rollback, 0);
 

--
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 v4 0/6] Improve tag checking in fsck and with transfer.fsckobjects

[Johannes Schindelin]

Hi Junio,

On Fri, 12 Sep 2014, Junio C Hamano wrote:

 Thanks. I think this is ready for 'next' and then down to 'master'
 for the next release.

Thank you!
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: [PATCH 4/8] staging: et131x: Remove ununsed statistics

[Dan Carpenter]

On Thu, Sep 11, 2014 at 10:59:42PM +0100, Mark Einon wrote:
 From struct ce_stats; unicast_pkts_rcvd, unicast_pkts_xmtd,
 multicast_pkts_xmtd, broadcast_pkts_rcvd and broadcast_pkts_xmtd

For some reason something adds a '' to the start of lines which start
with 'From'.  I don't know what it is...

When I apply this patch with 'git am' then it just removes the From
line.

I have seen these 'From' lines before but I haven't seen anyone discuss
this problem.

regards,
dan carpenter

--
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: git archive and glob pathspecs

[Peter Wu]

On Wednesday 03 September 2014 13:21:06 Duy Nguyen wrote:
 On Wed, Sep 3, 2014 at 5:17 AM, Peter Wu pe...@lekensteyn.nl wrote:
  Hi,
 
  The `git archive` seems to accept a pathspec judging from the error message 
  (git
  version 2.1.0):
 
  git archive HEAD -- :x
  fatal: pathspec 'x' did not match any files
 
  When I try to use deeper glob specs however, it throws an error (this also
  happens if I use `:(glob)**/Makefile`, tested in the git source tree):
 
  $ git archive HEAD -- ':(glob)*/Makefile'
  fatal: pathspec '*/Makefile' did not match any files
 
  Strange enough, command `git log -- ':(glob)*/Makefile'` works. Any idea 
  what is
  wrong?
 
 There may be something wrong. This patch seems to make it work for me,
 but it includes lots of empty directories. I'll have a closer look
 later (btw it's surprising that negative pathspec works too..)

I can confirm that this patch shows Makefile's, but also includes a lot of empty
directories.

As for why this happens, my guess is that write_archive_entries() recurses the
full tree and adds every encountered directory (via read_tree_1, via
write_archive_entry()).

To fix this, write_archive (write_tar_archive, etc.) should be taught to handle
glob patterns, or parse_pathspec should expand globs (and then
parse_pathspec_arg might have to validate the remaining patterns).

Kind regards,
Peter

 diff --git a/archive.c b/archive.c
 index 3fc0fb2..a5be58d 100644
 --- a/archive.c
 +++ b/archive.c
 @@ -221,6 +221,7 @@ static int path_exists(struct tree *tree, const char 
 *path)
   int ret;
 
   parse_pathspec(pathspec, 0, 0, , paths);
 + pathspec.recursive = 1;
   ret = read_tree_recursive(tree, , 0, 0, pathspec, reject_entry, NULL);
   free_pathspec(pathspec);
   return ret != 0;
 @@ -237,6 +238,7 @@ static void parse_pathspec_arg(const char **pathspec,
   parse_pathspec(ar_args-pathspec, 0,
 PATHSPEC_PREFER_FULL,
 , pathspec);
 + ar_args-pathspec.recursive = 1;
   if (pathspec) {
   while (*pathspec) {
   if (**pathspec  !path_exists(ar_args-tree, *pathspec))

--
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] archive: support filtering paths with glob

[Peter Wu]

On Thursday 04 September 2014 20:37:52 Nguyễn Thái Ngọc Duy wrote:
 This patch fixes two problems with using :(glob) (or even *.c
 without :(glob)).
 
 The first one is we forgot to turn on the 'recursive' flag in struct
 pathspec. Without that, tree_entry_interesting() will not mark
 potential directories interesting so that it can confirm whether
 those directories have anything matching the pathspec.
 
 The marking directories interesting has a side effect that we need to
 walk inside a directory to realize that there's nothing interested in
 there. By that time, 'archive' code has already written the (empty)
 directory down. That means lots of empty directories in the result
 archive.
 
 This problem is fixed by lazily writing directories down when we know
 they are actually needed. There is a theoretical bug in this
 implementation: we can't write empty trees/directories that match that
 pathspec.
 
 Noticed-by: Peter Wu pe...@lekensteyn.nl
 Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com

Ah, ignore my last mail, I just noticed this one. This patch works fine. By the
way, the glob pattern is treated as a 'nullglob'. If you specify a glob pattern
that matches nothing, an empty archive is created. Perhaps an error message
should be raised for that as it is unlikely that a user wants that?

Tested-by: Peter Wu pe...@lekensteyn.nl

 ---
  Similar approach could probably be used for teaching ls-tree about pathspec..
 
  archive.c   | 82 
 -
  t/t5000-tar-tree.sh | 10 +++
  2 files changed, 91 insertions(+), 1 deletion(-)
 
 diff --git a/archive.c b/archive.c
 index 3fc0fb2..9e62bf4 100644
 --- a/archive.c
 +++ b/archive.c
 @@ -98,9 +98,19 @@ static void setup_archive_check(struct git_attr_check 
 *check)
   check[1].attr = attr_export_subst;
  }
  
 +struct directory {
 + struct directory *up;
 + unsigned char sha1[20];
 + int baselen, len;
 + unsigned mode;
 + int stage;
 + char path[FLEX_ARRAY];
 +};
 +
  struct archiver_context {
   struct archiver_args *args;
   write_archive_entry_fn_t write_entry;
 + struct directory *bottom;
  };
  
  static int write_archive_entry(const unsigned char *sha1, const char *base,
 @@ -146,6 +156,65 @@ static int write_archive_entry(const unsigned char 
 *sha1, const char *base,
   return write_entry(args, sha1, path.buf, path.len, mode);
  }
  
 +static void queue_directory(const unsigned char *sha1,
 + const char *base, int baselen, const char *filename,
 + unsigned mode, int stage, struct archiver_context *c)
 +{
 + struct directory *d;
 + d = xmallocz(sizeof(*d) + baselen + 1 + strlen(filename));
 + d-up  = c-bottom;
 + d-baselen = baselen;
 + d-mode= mode;
 + d-stage   = stage;
 + c-bottom  = d;
 + d-len = sprintf(d-path, %.*s%s/, baselen, base, filename);
 + hashcpy(d-sha1, sha1);
 +}
 +
 +static int write_directory(struct archiver_context *c)
 +{
 + struct directory *d = c-bottom;
 + int ret;
 +
 + if (!d)
 + return 0;
 + c-bottom = d-up;
 + d-path[d-len - 1] = '\0'; /* no trailing slash */
 + ret =
 + write_directory(c) ||
 + write_archive_entry(d-sha1, d-path, d-baselen,
 + d-path + d-baselen, d-mode,
 + d-stage, c) != READ_TREE_RECURSIVE;
 + free(d);
 + return ret ? -1 : 0;
 +}
 +
 +static int queue_or_write_archive_entry(const unsigned char *sha1,
 + const char *base, int baselen, const char *filename,
 + unsigned mode, int stage, void *context)
 +{
 + struct archiver_context *c = context;
 +
 + while (c-bottom 
 +!(baselen = c-bottom-len 
 +  !strncmp(base, c-bottom-path, c-bottom-len))) {
 + struct directory *next = c-bottom-up;
 + free(c-bottom);
 + c-bottom = next;
 + }
 +
 + if (S_ISDIR(mode)) {
 + queue_directory(sha1, base, baselen, filename,
 + mode, stage, c);
 + return READ_TREE_RECURSIVE;
 + }
 +
 + if (write_directory(c))
 + return -1;
 + return write_archive_entry(sha1, base, baselen, filename, mode,
 +stage, context);
 +}
 +
  int write_archive_entries(struct archiver_args *args,
   write_archive_entry_fn_t write_entry)
  {
 @@ -167,6 +236,7 @@ int write_archive_entries(struct archiver_args *args,
   return err;
   }
  
 + memset(context, 0, sizeof(context));
   context.args = args;
   context.write_entry = write_entry;
  
 @@ -187,9 +257,17 @@ int write_archive_entries(struct archiver_args *args,
   }
  
   err = read_tree_recursive(args-tree, , 0, 0, args-pathspec,
 -   write_archive_entry, context);
 +   

Re: Diffs for submodule conflicts during rebase usually empty

[Jens Lehmann]

Am 12.09.2014 um 15:03 schrieb Edward Z. Yang:

Hello Jens,

Excerpts from Jens Lehmann's message of 2014-09-11 15:29:28 -0400:

Git does know what's going on, just fails to display it properly
in the diff, as the output of ls-files shows:

 $git ls-files -u
 16 6a6e215138b7f343fba67ba1b6ffc152019c6085 1b
 16 fc12d3455b120916ec508c3ccd04f23957c08ea5 2b
 16 33d9fa9f9e25de2a85f84993d8f6c752f84c769a 3b


Right. But I'd also add that even though Git knows what's going
on, even if we reported /that/ it wouldn't be user friendly:
namely, because submodules are not updated automatically so the
first line would always be what the submodule was pointed to
before we started rebasing.  That's not so useful either...


I agree that this needs to be improved, but am currently lacking
the time to do it myself. But I believe this will get important
rather soonish when we recursively update submodules too ...


As I've said, I'm happy to contribute a patch, if we can agree
what the right resolution is...


Me thinks the next step would be that git diff --submodule
should learn to not only show 2-way diffs but also 3-way diffs.
Then we'll be able to display submodule merge results in a human
readable way. After that we would have to find a way to display
submodule merge conflicts in a human readable way, similar to
what we do with conflict markers for regular files.
--
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/RFC] submodule: add ability to shallowly clone any branch in a submodule

[Jens Lehmann]

Am 12.09.2014 um 07:21 schrieb Fredrik Gustafsson:

On Thu, Sep 11, 2014 at 10:33:51PM +0200, Cole wrote:

Also if there is anything else you are currently looking at regarding
submodules or thinking about, I would be glad to hear about it or to try
look at it while I am working on these changes. Or if there is anything
you can think of for me to check with regards to these changes that
would also be appreciated.


When implementing the --depth argument for submodules, I would have
prefered that the depth was set from the commit of the submodules
refered from the superprojekt and not it's branch.


That would help recursive fetch too.


However this can't be done, since you can only clone from refs and not
from a commit. However there's nothing that stops us from allowing to
clone from a commit (of course we need to make sure that that commit is
in a tree with a ref as leaf).

I see this as a natural next step for the --depth function and something
needed for it to be really useful. I'm actually suprised that people
successfully uses the --depth function already since you always need to
know how deep down the commit is.


I suspect this only works because users set the depth high enough or the
submodule is updated frequently to the tip of a 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

Re: [PATCH/RFC] submodule: add ability to shallowly clone any branch in a submodule

[Jens Lehmann]

Am 11.09.2014 um 22:33 schrieb Cole:

Thanks for the feedback, really appreciate it, and will try to reformat
the patches as you have asked.


Thanks!


When you say you would like the patches split, do you mean into two
separate threads, or just different patches part of the same thread?


I think you are solving a single problem here (submodule update does
not work with branch and depth), so I'd propose a single thread for
that topic.


As for --no-single-branch on 'git submodule update', I didn't want to
break existing functionality, but if you would prefer that to be the
default I can make it so.


We should discuss if --no-single-branch should be implied when used
with a branch. I believe that if one option needs another one to work,
we should think about implying the latter. But might be wrong on that
with regards to --no-single-branch because I missed something obvious
... ;-)


Also if there is anything else you are currently looking at regarding
submodules or thinking about, I would be glad to hear about it or to try
look at it while I am working on these changes. Or if there is anything
you can think of for me to check with regards to these changes that
would also be appreciated.


Sure, I keep a Todo list on the Wiki of my GitHub-repo:

  
https://github.com/jlehmann/git-submod-enhancements/wiki#issues-still-to-be-tackled

If you want to work on some of those - or other - topic(s), I'll be
happy to help.


I am still quite new to some of the git terms and functionality,
so please excuse me if I do get anything wrong or do not fully understand.


No worries, we're all still learning ;-)
--
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: git-new-workdir submodules interact poorly with core.worktree

[Jens Lehmann]

Am 12.09.2014 um 15:58 schrieb Edward Z. Yang:

tl;dr You can't git-new-workdir checkouts which use core.worktree.  This
is unfortunate because 'git submodule init' uses core.worktree by
default, which means you can't recursively git-new-workdir without a
hack.

In the beginning, the Developer created the remote Git repository and
the submodule.

 mkdir -p remote/sub
 (cd remote/sub  git init  touch a  git add a  git commit -m sub 
init)
 mkdir remote/top
 cd remote/top
 git init
 git submodule add ../sub
 git commit -m top init
 cd ../..

And the Developer said, Let there be a local clone and submodule, and
lo, there was a local clone and submodule:

 git clone remote/top top
 (cd top  git submodule init  git submodule update)

the Developer blessed the working copy, and said Be fruitful and
increase in number with git-new-workdir:

 git-new-workdir top worktop

Unfortunately, this workdir didn't have the submodules initialized.

 $ ls worktop/sub/
 $

Now, the Developer could have run:

 $ (cd worktop  git submodule init  git submodule update)

but the resulting submodule would not have been shared with the original
submodule, in the same way that git-new-workdir shared the Git metadata.

The Developer sought to create the submodule in its own likeness, but it
did not work:

 $ rmdir worktop/sub  git-new-workdir top/sub worktop/sub
 fatal: Could not chdir to '../../../sub': No such file or directory

What was the Developer's fall from grace?  A glance at the config of
the original and new submodule shed light on the matter:

 $ cat top/sub/.git
 gitdir: ../.git/modules/sub
 $ cat top/.git/modules/sub/config
 [core]
 repositoryformatversion = 0
 filemode = true
 bare = false
 logallrefupdates = true
 worktree = ../../../sub
 $ cat worktop/sub/.git/config
 [core]
 repositoryformatversion = 0
 filemode = true
 bare = false
 logallrefupdates = true
 worktree = ../../../sub

git-new-workdir sought to reuse the config of top/sub/.git, but this
configuration had core.worktree set.  For the original checkout,
this worked fine, since its location was .git/modules/sub; but for the
new workdir, this relative path was nonsense.

I do not think there is really a way to make this work with
core.worktree.  Our saving grace, however, is there is a hack that can
make this work: we just need to use the
pre-501770e1bb5d132ae4f79aa96715f07f6b84e1f6 style of cloning
submodules:

 git clone remote/top oktop
 git clone remote/sub oktop/sub
 (cd oktop  git submodule init  git submodule update)

Now recursive git-new-workdir will work.


Thanks for the report and a nice summary.


What's the upshot?  I propose two new features:

1. A flag for git submodule update which reverts to the old behavior
of making a seperate .git directory rather than collecting them together
in the top-level .git/modules


That would play bad with the upcoming recursive submodule update
(which needs .git/modules to safely remove a submodule work tree),
so I wouldn't want to do that step backwards.


2. Teach git-new-workdir to complain if core.worktree is set in the
source config, and how to recursively copy submodules.


I'd prefer pursuing this approach.
--
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] Makefile: fix some typos in the preamble

[Ian Liu Rodrigues]

Signed-off-by: Ian Liu Rodrigues ian.li...@gmail.com
---
 Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 9f984a9..496af55 100644
--- a/Makefile
+++ b/Makefile
@@ -14,11 +14,11 @@ all::
 # Define INLINE to a suitable substitute (such as '__inline' or '') if git
 # fails to compile with errors about undefined inline functions or similar.
 #
-# Define SNPRINTF_RETURNS_BOGUS if your are on a system which snprintf()
+# Define SNPRINTF_RETURNS_BOGUS if you are on a system which snprintf()
 # or vsnprintf() return -1 instead of number of characters which would
 # have been written to the final string if enough space had been available.
 #
-# Define FREAD_READS_DIRECTORIES if your are on a system which succeeds
+# Define FREAD_READS_DIRECTORIES if you are on a system which succeeds
 # when attempting to read from an fopen'ed directory.
 #
 # Define NO_OPENSSL environment variable if you do not have OpenSSL.
-- 
2.1.0
--
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: [boinc_dev] (local ?) BOINC repo broken again -or- how to act on the CR/LF changes made upstream

[Toralf Förster]

On 09/12/2014 09:10 PM, Rom Walton wrote:
 I found this:
 http://stackoverflow.com/questions/17223527/how-do-i-force-git-to-checkout-the-master-branch-and-remove-carriage-returns-aft
 
 That might help in the future.

This helped :

git reset --hard 9e860d0451
git pull
git clean -f
git gc
git prune


-- 
Toralf
pgp key: 0076 E94E

--
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 4/8] staging: et131x: Remove ununsed statistics

[Greg KH]

On Sat, Sep 13, 2014 at 12:37:46PM +0300, Dan Carpenter wrote:
 On Thu, Sep 11, 2014 at 10:59:42PM +0100, Mark Einon wrote:
  From struct ce_stats; unicast_pkts_rcvd, unicast_pkts_xmtd,
  multicast_pkts_xmtd, broadcast_pkts_rcvd and broadcast_pkts_xmtd
 
 For some reason something adds a '' to the start of lines which start
 with 'From'.  I don't know what it is...

It's an email protocol requirement, some RFC dictates it as From at
the start of the line is an email start flag.

 When I apply this patch with 'git am' then it just removes the From
 line.

As it should :)

thanks,

greg k-h
--
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 v21 0/19] rs/ref-transaction (Re: Transaction patch series overview)

[Junio C Hamano]

Junio C Hamano gits...@pobox.com writes:

 I noticed that with this series merged to the version I use daily,
 detaching HEAD (i.e. git checkout HEAD^0) breaks my HEAD reflog,
 which made me redo the integration ejecting the series out of 'pu'.

 Not happy, as my workflow relies fairly heavily on detached HEAD
 X-.

Just FYI.

Bisecting the series using the attached as a test script points
branch -d: avoid repeated symref resolution as a possible culprit.
Perhaps these tests may want to be added to t3200 which is touched
by the said commit (or add them earlier in the series)?

-- 8 --

#!/bin/sh

test_description='reflog not nuked with co HEAD^0'
. ./test-lib.sh

check_reflog () {
while read name
do
git rev-parse --verify $name
done expect 
if test -f $2
then
while read object rest
do
git rev-parse --verify $object
done expect $2
fi 
while read object rest
do
git rev-parse --verify $object
done actual $1 
test_cmp expect actual
}

test_expect_success setup '
test_tick 
git commit --allow-empty -m initial 
git branch side 
test_tick 
git commit --allow-empty -m second 
git log -g --oneline baseline 
check_reflog baseline -\EOF
master
master^
EOF
'

test_expect_success 'switch to branch' '
git checkout side 
git log -g --oneline switched 
check_reflog switched baseline -\EOF
side
EOF
'

test_expect_success 'detach to other' '
git checkout master^0 
git log -g --oneline detach-1 
check_reflog detach-1 switched -\EOF
master
EOF
'

test_expect_success 'attach to self' '
git checkout master 
git log -g --oneline detach-2 
check_reflog detach-2 detach-1 -\EOF
master
EOF
'

test_expect_success 'detach to self' '
git checkout master^0 
git log -g --oneline detach-3 
check_reflog detach-3 detach-2 -\EOF
master
EOF
'

test_expect_success 'attach to other' '
git checkout HEAD^0 
git checkout side 
git log -g --oneline detach-4 
check_reflog detach-4 detach-3 -\EOF
side
EOF
'

test_done
--
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 v4 00/32] Lockfile correctness and refactoring

[Jeff King]

On Fri, Sep 12, 2014 at 04:21:09PM +0200, Michael Haggerty wrote:

  But I still wonder how hard it would be to just remove lock_file structs
  from the global list when they are committed or rolled back.
 [...]

 To make that change, we would have to remove entries from the list of
 lock_file objects in a way that the code can be interrupted at any time
 by a signal while leaving it in a state that is traversable by the
 signal handler.
 
 I think that can be done pretty easily with a singly-linked list. But
 with a singly-linked list, we would have to iterate through the list to
 find the node that needs to be removed. This could get expensive if
 there are a lot of nodes in the list (see below).

Yes, I considered that, but noticed that if we actually cleaned up
closed files, the list would not grow to more than a handful of entries.
But...

 The ref-transaction code is, I think, moving in the direction of
 updating all references in a single transaction. This means that we
 would need to hold locks for all of the references at once anyway. So it
 might be all for naught.

That nullifies the whole discussion. Besides the list-traversal thing
above, it would mean that we literally _do_ have all of the lockfiles
open at once. So cleaning up after ourselves would be nice, but it would
not impact the peak memory usage, which would necessarily have one
allocated struct per ref.

The use of a strbuf is probably a big enough change to save us there.
This case was pathological for a few reasons:

  1. A ridiculous number of refs in the repository.

  2. Touching a large number of them in sequence (via pack-refs).

  3. Allocating a 4K buffer per object.

For (3), if the average allocation is dropped even to 400 bytes (which
would accommodate quite a long pathname), that would reduce the memory
usage to ~700MB. Not amazing, but enough not to tip over most modern
machines.

-Peff
--
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 4/8] staging: et131x: Remove ununsed statistics

[Dan Carpenter]

On Sat, Sep 13, 2014 at 08:45:56AM -0700, Greg KH wrote:
 On Sat, Sep 13, 2014 at 12:37:46PM +0300, Dan Carpenter wrote:
  On Thu, Sep 11, 2014 at 10:59:42PM +0100, Mark Einon wrote:
   From struct ce_stats; unicast_pkts_rcvd, unicast_pkts_xmtd,
   multicast_pkts_xmtd, broadcast_pkts_rcvd and broadcast_pkts_xmtd
  
  For some reason something adds a '' to the start of lines which start
  with 'From'.  I don't know what it is...
 
 It's an email protocol requirement, some RFC dictates it as From at
 the start of the line is an email start flag.
 
  When I apply this patch with 'git am' then it just removes the From
  line.
 
 As it should :)
 

But now the changelog is corrupt.  I have tested with the git version
2.1.0.238.gce1d3a9.  The first line of the changelog gets chopped off
because of the From.

It's a little annoying.  Do we just tell Mark to resend with a different
changelog or is there a way to fix the tools?

regards,
dan carpenter

--
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] repack: call prune_packed_objects() and update_server_info() directly

[Jeff King]

On Sat, Sep 13, 2014 at 09:28:01AM +0200, René Scharfe wrote:

 Call the functions behind git prune-packed and git update-server-info
 directly instead of using run_command().  This is shorter, easier and
 quicker.

It can also introduce bugs, since a lot of git code assumes it is
running in a single process and can die() or mark up global variables at
will. :)

I gave a quick read-through of the code and I think these calls are OK.
The two things I noticed were:

  1. We might die on a malloc failure that would otherwise go unnoticed
 in a sub-process. That's probably OK.

  2. The info/packs file is generated from our internal packed_git list.
 This list can get crufty if you have a long-running process that
 accesses objects and other processes are repacking. I think that's
 OK here; the parent repack process is not very long-lived.

I did, however, notice that the code we are calling has some problems of
its own. :) Here are some fixes:

  [1/3]: prune-packed: fix minor memory leak
  [2/3]: make update-server-info more robust
  [3/3]: server-info: clean up after writing info/packs

-Peff
--
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/3] prune-packed: fix minor memory leak

[Jeff King]

We form all of our directories in a strbuf, but never release it.

Signed-off-by: Jeff King p...@peff.net
---
 builtin/prune-packed.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/builtin/prune-packed.c b/builtin/prune-packed.c
index 6879468..d430731 100644
--- a/builtin/prune-packed.c
+++ b/builtin/prune-packed.c
@@ -68,6 +68,7 @@ void prune_packed_objects(int opts)
rmdir(pathname.buf);
}
stop_progress(progress);
+   strbuf_release(pathname);
 }
 
 int cmd_prune_packed(int argc, const char **argv, const char *prefix)
-- 
2.1.0.373.g91ca799

--
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 2/3] make update-server-info more robust

[Jeff King]

Since git update-server-info may be called automatically
as part of a push or a gc --auto, we should be robust
against two processes trying to update it simultaneously.
However, we currently use a fixed tempfile, which means that
two simultaneous writers may step on each other's toes and
end up renaming junk into place.

Let's instead switch to using a unique tempfile via mkstemp.
We do not want to use a lockfile here, because it's OK for
two writers to simultaneously update (one will win the
rename race, but that's OK; they should be writing the same
information).

While we're there, let's clean up a few other things:

  1. Detect write errors. Report them and abort the update
 if any are found.

  2. Free path memory rather than leaking it (and clean up
 the tempfile when necessary).

  3. Use the pathdup functions consistently rather than
 static buffers or manually calculated lengths.

This last one fixes a potential overflow of infofile in
update_info_packs (e.g., by putting large junk into
$GIT_OBJECT_DIRECTORY). However, this overflow was probably
not an interesting attack vector for two reasons:

  a. The attacker would need to control the environment to
 do this, in which case it was already game-over.

  b. During its setup phase, git checks that the directory
 actually exists, which means it is probably shorter
 than PATH_MAX anyway.

Because both update_info_refs and update_info_packs share
these same failings (and largely duplicate each other), this
patch factors out the improved error-checking version into a
helper function.

Signed-off-by: Jeff King p...@peff.net
---
I guess point (b) may not apply on systems that have a really small
PATH_MAX that does not reflect what you can actually create in the
filesystem (Windows?). But I think point (a) still applies, so this is
not really a big deal security-wise (though it is certainly a bugfix for
such systems).

 server-info.c | 116 +++---
 1 file changed, 71 insertions(+), 45 deletions(-)

diff --git a/server-info.c b/server-info.c
index 9ec744e..d54a3d6 100644
--- a/server-info.c
+++ b/server-info.c
@@ -4,45 +4,80 @@
 #include commit.h
 #include tag.h
 
-/* refs */
-static FILE *info_ref_fp;
+/*
+ * Create the file path by writing to a temporary file and renaming
+ * it into place. The contents of the file come from generate, which
+ * should return non-zero if it encounters an error.
+ */
+static int update_info_file(char *path, int (*generate)(FILE *))
+{
+   char *tmp = mkpathdup(%s_XX, path);
+   int ret = -1;
+   int fd = -1;
+   FILE *fp = NULL;
+
+   safe_create_leading_directories(path);
+   fd = mkstemp(tmp);
+   if (fd  0)
+   goto out;
+   fp = fdopen(fd, w);
+   if (!fp)
+   goto out;
+   ret = generate(fp);
+   if (ret)
+   goto out;
+   if (fclose(fp))
+   goto out;
+   if (adjust_shared_perm(tmp)  0)
+   goto out;
+   if (rename(tmp, path)  0)
+   goto out;
+   ret = 0;
+
+out:
+   if (ret) {
+   error(unable to update %s: %s, path, strerror(errno));
+   if (fp)
+   fclose(fp);
+   else if (fd = 0)
+   close(fd);
+   unlink(tmp);
+   }
+   free(tmp);
+   return ret;
+}
 
 static int add_info_ref(const char *path, const unsigned char *sha1, int flag, 
void *cb_data)
 {
+   FILE *fp = cb_data;
struct object *o = parse_object(sha1);
if (!o)
return -1;
 
-   fprintf(info_ref_fp, %s%s\n, sha1_to_hex(sha1), path);
+   if (fprintf(fp, %s %s\n, sha1_to_hex(sha1), path)  0)
+   return -1;
+
if (o-type == OBJ_TAG) {
o = deref_tag(o, path, 0);
if (o)
-   fprintf(info_ref_fp, %s%s^{}\n,
-   sha1_to_hex(o-sha1), path);
+   if (fprintf(fp, %s %s^{}\n,
+   sha1_to_hex(o-sha1), path)  0)
+   return -1;
}
return 0;
 }
 
+static int generate_info_refs(FILE *fp)
+{
+   return for_each_ref(add_info_ref, fp);
+}
+
 static int update_info_refs(int force)
 {
-   char *path0 = git_pathdup(info/refs);
-   int len = strlen(path0);
-   char *path1 = xmalloc(len + 2);
-
-   strcpy(path1, path0);
-   strcpy(path1 + len, +);
-
-   safe_create_leading_directories(path0);
-   info_ref_fp = fopen(path1, w);
-   if (!info_ref_fp)
-   return error(unable to update %s, path1);
-   for_each_ref(add_info_ref, NULL);
-   fclose(info_ref_fp);
-   adjust_shared_perm(path1);
-   rename(path1, path0);
-   free(path0);
-   free(path1);
-   return 0;
+   char *path = git_pathdup(info/refs);
+   int ret = update_info_file(path, 

[PATCH 3/3] server-info: clean up after writing info/packs

[Jeff King]

We allocate pack information in a static global list but
never clean it up. This leaks memory, and means that calling
update_server_info twice will generate a buggy file (it will
have duplicate entries).

Signed-off-by: Jeff King p...@peff.net
---
 server-info.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/server-info.c b/server-info.c
index d54a3d6..31f4a74 100644
--- a/server-info.c
+++ b/server-info.c
@@ -233,6 +233,14 @@ static void init_pack_info(const char *infofile, int force)
info[i]-new_num = i;
 }
 
+static void free_pack_info(void)
+{
+   int i;
+   for (i = 0; i  num_pack; i++)
+   free(info[i]);
+   free(info);
+}
+
 static int write_pack_info_file(FILE *fp)
 {
int i;
@@ -252,6 +260,7 @@ static int update_info_packs(int force)
 
init_pack_info(infofile, force);
ret = update_info_file(infofile, write_pack_info_file);
+   free_pack_info();
free(infofile);
return ret;
 }
-- 
2.1.0.373.g91ca799
--
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 4/8] staging: et131x: Remove ununsed statistics

[Jeff King]

On Sat, Sep 13, 2014 at 08:45:56AM -0700, Greg KH wrote:

 On Sat, Sep 13, 2014 at 12:37:46PM +0300, Dan Carpenter wrote:
  On Thu, Sep 11, 2014 at 10:59:42PM +0100, Mark Einon wrote:
   From struct ce_stats; unicast_pkts_rcvd, unicast_pkts_xmtd,
   multicast_pkts_xmtd, broadcast_pkts_rcvd and broadcast_pkts_xmtd
  
  For some reason something adds a '' to the start of lines which start
  with 'From'.  I don't know what it is...
 
 It's an email protocol requirement, some RFC dictates it as From at
 the start of the line is an email start flag.

It's not an RFC thing. It's a side effect of the mbox format, which
squashes together multiple messages with From  lines to mark their
starts. So many mbox implementations will quote them as From (others
introduce a Content-Length header, or are simply more careful about
making sure that the line looks like a real From  line, which should
contain a date).

If somebody's MUA is actually transmitting emails with the quoting,
that's wrong. It is a local storage problem, and they should not be
spreading the quoting disease to other systems.

  When I apply this patch with 'git am' then it just removes the From
  line.
 
 As it should :)

That seems wrong. We should either leave it as-is (i.e., assume the
writer used no quoting and really did mean From) or strip the  to
turn it into From (i.e., assume the writer did use quoting). In some
implementations, a literal From gets quoted to From and so on. So
we could even strip one level of quoting from such things (if we assume
the writer was such an implementation).

I don't think we can make this 100% foolproof without knowing which mbox
variant the writer used. But dropping the line is probably the worst
possible thing, as it does not match _any_ variants. :)

-Peff
--
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 4/8] staging: et131x: Remove ununsed statistics

[Dan Carpenter]

On Sat, Sep 13, 2014 at 09:47:45PM +0100, Mark Einon wrote:
 On Sat, Sep 13, 2014 at 04:36:45PM -0400, Jeff King wrote:
  I don't think we can make this 100% foolproof without knowing which mbox
  variant the writer used. But dropping the line is probably the worst
  possible thing, as it does not match _any_ variants. :)
 
 Hi,
 
 FYI it was 'git send-email' v2.1.0 that sent the mail, and I don't have
 the offending character in any versions of the mail I can see.
 

The mailing list version has it.

http://www.spinics.net/lists/linux-driver-devel/msg54372.html

regards,
dan carpenter

--
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 4/8] staging: et131x: Remove ununsed statistics

[Mark Einon]

On Sat, Sep 13, 2014 at 04:36:45PM -0400, Jeff King wrote:
 I don't think we can make this 100% foolproof without knowing which mbox
 variant the writer used. But dropping the line is probably the worst
 possible thing, as it does not match _any_ variants. :)

Hi,

FYI it was 'git send-email' v2.1.0 that sent the mail, and I don't have
the offending character in any versions of the mail I can see.

Cheers,

Mark
--
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 4/8] staging: et131x: Remove ununsed statistics

[Mark Einon]

On Sat, Sep 13, 2014 at 11:57:51PM +0300, Dan Carpenter wrote:
 On Sat, Sep 13, 2014 at 09:47:45PM +0100, Mark Einon wrote:
  On Sat, Sep 13, 2014 at 04:36:45PM -0400, Jeff King wrote:
   I don't think we can make this 100% foolproof without knowing which mbox
   variant the writer used. But dropping the line is probably the worst
   possible thing, as it does not match _any_ variants. :)
  
  Hi,
  
  FYI it was 'git send-email' v2.1.0 that sent the mail, and I don't have
  the offending character in any versions of the mail I can see.
  
 
 The mailing list version has it.
 
 http://www.spinics.net/lists/linux-driver-devel/msg54372.html

Fair enough. The marc.info version doesn't though, so it's proably not my MUA:

http://marc.info/?l=linux-driver-develm=141047281318963w=2

Cheers,

Mark
--
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 4/8] staging: et131x: Remove ununsed statistics

[Dan Carpenter]

On Sat, Sep 13, 2014 at 11:57:51PM +0300, Dan Carpenter wrote:
 On Sat, Sep 13, 2014 at 09:47:45PM +0100, Mark Einon wrote:
  On Sat, Sep 13, 2014 at 04:36:45PM -0400, Jeff King wrote:
   I don't think we can make this 100% foolproof without knowing which mbox
   variant the writer used. But dropping the line is probably the worst
   possible thing, as it does not match _any_ variants. :)
  
  Hi,
  
  FYI it was 'git send-email' v2.1.0 that sent the mail, and I don't have
  the offending character in any versions of the mail I can see.
  
 
 The mailing list version has it.
 
 http://www.spinics.net/lists/linux-driver-devel/msg54372.html

Or based on Peff's email it might be a bug in the spinics list software.
Here are some other examples:

Piper mail has 'From'.
http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2014-September/058299.html

but gmane gets it right.
http://comments.gmane.org/gmane.linux.drivers.driver-project.devel/57684

regards,
dan carpenter

--
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

[RFC/PATCH] mailinfo: do not treat From lines as in-body headers

[Jeff King]

[-cc driver-devel list, as this is getting into git patches]

On Sun, Sep 14, 2014 at 12:09:08AM +0300, Dan Carpenter wrote:

   FYI it was 'git send-email' v2.1.0 that sent the mail, and I don't have
   the offending character in any versions of the mail I can see.
 [...]
 Piper mail has 'From'.
 http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2014-September/058299.html
 
 but gmane gets it right.
 http://comments.gmane.org/gmane.linux.drivers.driver-project.devel/57684

Thanks both of you for following up. I did confirm that git-send-email
does not add such quoting. From your findings above, I'd agree that it's
the list-archive software munging it, and they are buggy IMHO (they
should de-quote on display).

Here's an RFC patch to help the git am side handle this case better.

-- 8 --
Subject: mailinfo: do not treat From lines as in-body headers

Since commit 81c5cf7 (mailinfo: skip bogus UNIX From line
inside body, 2006-05-21), we have treated lines like From
in the body as headers. This makes git am work for people
who erroneously paste the whole mbox entry:

  From 12345abcd...
  From: them
  Subject: [PATCH] whatever

into their email body. However, it also causes false
positives for people who really do have From in the first
paragraph of their commit messages. In this case, we'll drop
the line completely, breaking the commit message.

We could try to make our checking more robust by doing one
or both of:

  - making sure the line looks like a git From  line
(40-hex sha1, date, etc).

  - seeing if the following lines are actually rfc2822
headers

However, it's probably not worth the complexity. There are a
few reasons to think that this code is not actually
triggered very often. One, the patch was written in 2006,
when git was still relatively new, and people frequently
made mistakes in sending patches; these days we not see this
error much. And two, we check only the quoted From form,
and not the regular From. So whether it kicks in at all
depends entirely on how the mbox is saved by the user's MUA.
And in the intervening 8 years, nobody has complained about
the From case.

With this patch, we will simply treat such From lines as
normal body lines (and stop in-body header parsing). Note
that we do _not_ unquote them into From. Whether
from-quoting is in effect depends on the exact mbox format
being used, which depends on the MUA writing the file. We
cannot know for sure whether to unquote or not, so we leave
the line alone.

Signed-off-by: Jeff King p...@peff.net
---
I admit my arguments that it is not in use are a little flaky, and this
may just be me being lazy. Trying to match arbitrary From lines is
very hard and heuristic-filled, but if we are only trying to match
git-generated mbox lines, that's much easier. It would not hurt too much
to go that route.

I also tend to think we should unquote From into From. As discussed
above, we do know whether the author meant the literal former, or meant
the latter and it quoted into the former. But I'd guess that a literal
From is quite rare, so we'd probably serve more people by de-quoting.
That is really a separate issue for another patch, though.

 builtin/mailinfo.c |  4 
 t/t5100-mailinfo.sh|  9 +
 t/t5100/quoted-from.expect |  3 +++
 t/t5100/quoted-from.in | 10 ++
 4 files changed, 22 insertions(+), 4 deletions(-)
 create mode 100644 t/t5100/quoted-from.expect
 create mode 100644 t/t5100/quoted-from.in

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index cf11c8d..0a08e44 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -328,10 +328,6 @@ static int check_header(const struct strbuf *line,
}
 
/* for inbody stuff */
-   if (starts_with(line-buf, From)  isspace(line-buf[5])) {
-   ret = 1; /* Should this return 0? */
-   goto check_header_out;
-   }
if (starts_with(line-buf, [PATCH])  isspace(line-buf[7])) {
for (i = 0; header[i]; i++) {
if (!strcmp(Subject, header[i])) {
diff --git a/t/t5100-mailinfo.sh b/t/t5100-mailinfo.sh
index 3e64a7a..578ff16 100755
--- a/t/t5100-mailinfo.sh
+++ b/t/t5100-mailinfo.sh
@@ -89,4 +89,13 @@ test_expect_success 'mailinfo on from header without name 
works' '
 
 '
 
+test_expect_success 'mailinfo on message with quoted From' '
+   mkdir quoted-from 
+   git mailsplit -oquoted-from $TEST_DIRECTORY/t5100/quoted-from.in 
+   test_cmp $TEST_DIRECTORY/t5100/quoted-from.in quoted-from/0001 
+   git mailinfo quoted-from/msg quoted-from/patch \
+ quoted-from/0001 quoted-from/out 
+   test_cmp $TEST_DIRECTORY/t5100/quoted-from.expect quoted-from/msg
+'
+
 test_done
diff --git a/t/t5100/quoted-from.expect b/t/t5100/quoted-from.expect
new file mode 100644
index 000..8c9d48c
--- /dev/null
+++ b/t/t5100/quoted-from.expect
@@ -0,0 +1,3 @@
+From the depths of history, we are stuck with the
+flaky mbox format.
+
diff --git 

Re: [PATCH v2] stash: prefer --quiet over shell redirection

[David Aguilar]

On Fri, Sep 12, 2014 at 12:05:48PM -0700, Junio C Hamano wrote:
 David Aguilar dav...@gmail.com writes:
 
  Use `git rev-parse --verify --quiet` instead of redirecting
  stderr to /dev/null.
 
  Signed-off-by: David Aguilar dav...@gmail.com
  ---
 
 Has this patch ever been tested?  t3903 seems to break with this at
 least for me.

Oops, I thought I did but it's definitely failing in pu.

The root cause is that git rev-parse --verify --quiet is not
actually quiet.

I'll send a patch to fix this shortly.

It touches the ref machinery, which I know Ronnie has been improving,
so I hope this doesn't cause any conflicts with other in-flight topics.


   git-stash.sh | 13 +++--
   1 file changed, 7 insertions(+), 6 deletions(-)
 
  diff --git a/git-stash.sh b/git-stash.sh
  index bcc757b..2ff8b94 100755
  --- a/git-stash.sh
  +++ b/git-stash.sh
  @@ -50,7 +50,7 @@ clear_stash () {
  then
  die $(gettext git stash clear with parameters is 
  unimplemented)
  fi
  -   if current=$(git rev-parse --verify $ref_stash 2/dev/null)
  +   if current=$(git rev-parse --verify --quiet $ref_stash)
  then
  git update-ref -d $ref_stash $current
  fi
  @@ -292,7 +292,7 @@ save_stash () {
   }
   
   have_stash () {
  -   git rev-parse --verify $ref_stash /dev/null 21
  +   git rev-parse --verify --quiet $ref_stash /dev/null
   }
   
   list_stash () {
  @@ -392,12 +392,12 @@ parse_flags_and_rev()
  ;;
  esac
   
  -   REV=$(git rev-parse --quiet --symbolic --verify $1 2/dev/null) || {
  +   REV=$(git rev-parse --symbolic --verify --quiet $1) || {
  reference=$1
  die $(eval_gettext \$reference is not valid reference)
  }
   
  -   i_commit=$(git rev-parse --quiet --verify $REV^2 2/dev/null) 
  +   i_commit=$(git rev-parse --verify --quiet $REV^2) 
  set -- $(git rev-parse $REV $REV^1 $REV: $REV^1: $REV^2: 
  2/dev/null) 
  s=$1 
  w_commit=$1 
  @@ -409,7 +409,7 @@ parse_flags_and_rev()
  test $ref_stash = $(git rev-parse --symbolic-full-name ${REV%@*}) 
  
  IS_STASH_REF=t
   
  -   u_commit=$(git rev-parse --quiet --verify $REV^3 2/dev/null) 
  +   u_commit=$(git rev-parse --verify --quiet $REV^3) 
  u_tree=$(git rev-parse $REV^3: 2/dev/null)
   }
   
  @@ -531,7 +531,8 @@ drop_stash () {
  die $(eval_gettext \${REV}: Could not drop stash entry)
   
  # clear_stash if we just dropped the last stash entry
  -   git rev-parse --verify $ref_stash@{0} /dev/null 21 || clear_stash
  +   git rev-parse --verify --quiet $ref_stash@{0} /dev/null ||
  +   clear_stash
   }
   
   apply_to_branch () {

-- 
David
--
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] refs: make rev-parse --quiet actually quiet

[David Aguilar]

When a reflog is deleted, e.g. when git stash clears its stashes,
git rev-parse --verify --quiet dies:

fatal: Log for refs/stash is empty.

The reason is that the get_sha1() code path does not allow us
to suppress this message.

Pass the flags bitfield through the get_sha1_with_context()
so that read_ref_at() can suppress the message.

Use get_sha1_with_context1() instead of get_sha1() in rev-parse
so that the --quiet flag is honored.

Signed-off-by: David Aguilar dav...@gmail.com
---
This should be applied before stash: prefer --quiet over shell redirection
which is currently in pu.

This fixes t3903-stash.sh.

Ronnie, I see you've been making a lot of changes in this area so you
may want to take a look.

 builtin/rev-parse.c   |  5 -
 builtin/show-branch.c |  5 +++--
 refs.c| 10 +++---
 refs.h|  3 ++-
 sha1_name.c   |  7 ---
 5 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index d85e08c..8bc1374 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -508,7 +508,9 @@ int cmd_rev_parse(int argc, const char **argv, const char 
*prefix)
int has_dashdash = 0;
int output_prefix = 0;
unsigned char sha1[20];
+   unsigned int flags = 0;
const char *name = NULL;
+   struct object_context unused;
 
if (argc  1  !strcmp(--parseopt, argv[1]))
return cmd_parseopt(argc - 1, argv + 1, prefix);
@@ -596,6 +598,7 @@ int cmd_rev_parse(int argc, const char **argv, const char 
*prefix)
}
if (!strcmp(arg, --quiet) || !strcmp(arg, -q)) {
quiet = 1;
+   flags |= GET_SHA1_QUIETLY;
continue;
}
if (!strcmp(arg, --short) ||
@@ -818,7 +821,7 @@ int cmd_rev_parse(int argc, const char **argv, const char 
*prefix)
name++;
type = REVERSED;
}
-   if (!get_sha1(name, sha1)) {
+   if (!get_sha1_with_context(name, flags, sha1, unused)) {
if (verify)
revs_count++;
else
diff --git a/builtin/show-branch.c b/builtin/show-branch.c
index 298c95e..46498e1 100644
--- a/builtin/show-branch.c
+++ b/builtin/show-branch.c
@@ -723,6 +723,7 @@ int cmd_show_branch(int ac, const char **av, const char 
*prefix)
char nth_desc[256];
char *ref;
int base = 0;
+   unsigned int flags = 0;
 
if (ac == 0) {
static const char *fake_av[2];
@@ -749,7 +750,7 @@ int cmd_show_branch(int ac, const char **av, const char 
*prefix)
/* Ah, that is a date spec... */
unsigned long at;
at = approxidate(reflog_base);
-   read_ref_at(ref, at, -1, sha1, NULL,
+   read_ref_at(ref, flags, at, -1, sha1, NULL,
NULL, NULL, base);
}
}
@@ -760,7 +761,7 @@ int cmd_show_branch(int ac, const char **av, const char 
*prefix)
unsigned long timestamp;
int tz;
 
-   if (read_ref_at(ref, 0, base+i, sha1, logmsg,
+   if (read_ref_at(ref, flags, 0, base+i, sha1, logmsg,
timestamp, tz, NULL)) {
reflog = i;
break;
diff --git a/refs.c b/refs.c
index 27927f2..fff0513 100644
--- a/refs.c
+++ b/refs.c
@@ -3104,7 +3104,7 @@ static int read_ref_at_ent_oldest(unsigned char *osha1, 
unsigned char *nsha1,
return 1;
 }
 
-int read_ref_at(const char *refname, unsigned long at_time, int cnt,
+int read_ref_at(const char *refname, unsigned int flags, unsigned long 
at_time, int cnt,
unsigned char *sha1, char **msg,
unsigned long *cutoff_time, int *cutoff_tz, int *cutoff_cnt)
 {
@@ -3122,8 +3122,12 @@ int read_ref_at(const char *refname, unsigned long 
at_time, int cnt,
 
for_each_reflog_ent_reverse(refname, read_ref_at_ent, cb);
 
-   if (!cb.reccnt)
-   die(Log for %s is empty., refname);
+   if (!cb.reccnt) {
+   if (flags  GET_SHA1_QUIETLY)
+   exit(1);
+   else
+   die(Log for %s is empty., refname);
+   }
if (cb.found_it)
return 0;
 
diff --git a/refs.h b/refs.h
index ec46acd..fb4f2c5 100644
--- a/refs.h
+++ b/refs.h
@@ -171,7 +171,8 @@ extern int write_ref_sha1(struct ref_lock *lock, const 
unsigned char *sha1, cons
 int log_ref_setup(const char *refname, char *logfile, int bufsize);
 
 /** Reads 

Re: [RFC/PATCH] mailinfo: do not treat From lines as in-body headers

[brian m. carlson]

On Sat, Sep 13, 2014 at 05:25:05PM -0400, Jeff King wrote:
 Thanks both of you for following up. I did confirm that git-send-email
 does not add such quoting. From your findings above, I'd agree that it's
 the list-archive software munging it, and they are buggy IMHO (they
 should de-quote on display).

I wonder if git send-email should do what mutt does in this case, which
is use quoted-printable encoding and encode the first F as =46 (as well
as any equals signs as =3D).  It looks like mailinfo.c already is
capable of handling that, and that would avoid the entire issue.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature

Re: [RFC/PATCH] mailinfo: do not treat From lines as in-body headers

[Jeff King]

On Sat, Sep 13, 2014 at 10:57:14PM +, brian m. carlson wrote:

 On Sat, Sep 13, 2014 at 05:25:05PM -0400, Jeff King wrote:
  Thanks both of you for following up. I did confirm that git-send-email
  does not add such quoting. From your findings above, I'd agree that it's
  the list-archive software munging it, and they are buggy IMHO (they
  should de-quote on display).
 
 I wonder if git send-email should do what mutt does in this case, which
 is use quoted-printable encoding and encode the first F as =46 (as well
 as any equals signs as =3D).  It looks like mailinfo.c already is
 capable of handling that, and that would avoid the entire issue.

That's not an unreasonable tactic. However, I think we'd still want to
do something with mailinfo on the receiving end, similar to the patch I
sent. We don't know that the sending side is necessarily send-email.

-Peff
--
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: [RFC/PATCH] mailinfo: do not treat From lines as in-body headers

[Junio C Hamano]

On Sat, Sep 13, 2014 at 5:47 PM, Jeff King p...@peff.net wrote:

 On Sat, Sep 13, 2014 at 10:57:14PM +, brian m. carlson wrote:

  I wonder if git send-email should do what mutt does in this case, which
  is use quoted-printable encoding and encode the first F as =46 (as well
  as any equals signs as =3D).  It looks like mailinfo.c already is
  capable of handling that, and that would avoid the entire issue.

 That's not an unreasonable tactic. However, I think we'd still want to
 do something with mailinfo on the receiving end, similar to the patch I
 sent. We don't know that the sending side is necessarily send-email.

Hmm, isn't the  stuffing in front of a beginning-of-line From  purely
a local matter of MUA that stores messages in (old-style) mbox format
where a line that begins with From  is what defines the end of the
previous message? Why should send-email do anything when it sends
individual messages separately 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

[PATCH v4 2/2] headers: include dependent headers

[David Aguilar]

Add dependent headers so that including a header does not
require including additional headers.

Move the unicode interval types to unicode_width.h so that we
can include them from utf8.h.

This makes it so that gcc -c $header succeeds for each header.

Helped-by: René Scharfe l@web.de
Signed-off-by: David Aguilar dav...@gmail.com
---
Changes since last time:

This patch was redone to no longer add git-compat-util.h
additions, as it was run using the new check-headers.

The unicode interval types were moved into unicode_width.h
to minimize dependencies.

 archive.h |  1 +
 attr.h|  2 ++
 blob.h|  1 +
 branch.h  |  2 ++
 column.h  |  2 ++
 commit.h  |  1 +
 convert.h |  2 ++
 csum-file.h   |  2 ++
 diff.h|  2 +-
 diffcore.h|  2 ++
 dir.h |  2 ++
 ewah/ewok_rlw.h   |  2 ++
 fsck.h|  2 ++
 gpg-interface.h   |  2 ++
 graph.h   |  2 ++
 khash.h   |  2 ++
 line-log.h|  1 +
 list-objects.h|  2 ++
 ll-merge.h|  2 ++
 mailmap.h |  2 ++
 merge-recursive.h |  2 ++
 notes-merge.h |  4 
 notes-utils.h |  1 +
 notes.h   |  1 +
 object.h  |  2 ++
 pack-bitmap.h |  2 ++
 pack-objects.h|  2 ++
 patch-ids.h   |  3 +++
 reachable.h   |  2 ++
 reflog-walk.h |  1 +
 refs.h|  3 +++
 remote.h  |  1 +
 resolve-undo.h|  4 
 send-pack.h   |  3 +++
 sequencer.h   |  2 ++
 shortlog.h|  1 +
 submodule.h   |  5 +++--
 tree-walk.h   |  2 ++
 tree.h|  1 +
 unicode_width.h   | 12 
 unpack-trees.h|  2 ++
 url.h |  2 ++
 utf8.c|  5 -
 utf8.h|  3 ++-
 walker.h  |  1 +
 wt-status.h   |  1 +
 xdiff/xdiffi.h|  2 ++
 xdiff/xemit.h |  3 +++
 xdiff/xprepare.h  |  3 ++-
 xdiff/xutils.h|  3 ++-
 50 files changed, 104 insertions(+), 11 deletions(-)

diff --git a/archive.h b/archive.h
index 4a791e1..b2ca5bf 100644
--- a/archive.h
+++ b/archive.h
@@ -1,6 +1,7 @@
 #ifndef ARCHIVE_H
 #define ARCHIVE_H
 
+#include cache.h
 #include pathspec.h
 
 struct archiver_args {
diff --git a/attr.h b/attr.h
index 8b08d33..34e63f8 100644
--- a/attr.h
+++ b/attr.h
@@ -1,6 +1,8 @@
 #ifndef ATTR_H
 #define ATTR_H
 
+#include cache.h
+
 /* An attribute is a pointer to this opaque structure */
 struct git_attr;
 
diff --git a/blob.h b/blob.h
index 59b394e..25d8618 100644
--- a/blob.h
+++ b/blob.h
@@ -1,6 +1,7 @@
 #ifndef BLOB_H
 #define BLOB_H
 
+#include cache.h
 #include object.h
 
 extern const char *blob_type;
diff --git a/branch.h b/branch.h
index 64173ab..d5fdcc6 100644
--- a/branch.h
+++ b/branch.h
@@ -3,6 +3,8 @@
 
 /* Functions for acting on the information about branches. */
 
+#include cache.h
+
 /*
  * Creates a new branch, where head is the branch currently checked
  * out, name is the new branch name, start_name is the name of the
diff --git a/column.h b/column.h
index 0a61917..5d094b4 100644
--- a/column.h
+++ b/column.h
@@ -1,6 +1,8 @@
 #ifndef COLUMN_H
 #define COLUMN_H
 
+#include string-list.h
+
 #define COL_LAYOUT_MASK   0x000F
 #define COL_ENABLE_MASK   0x0030   /* always, never or auto */
 #define COL_PARSEOPT  0x0040   /* --column is given from cmdline */
diff --git a/commit.h b/commit.h
index a401ddf..a0f6d20 100644
--- a/commit.h
+++ b/commit.h
@@ -1,6 +1,7 @@
 #ifndef COMMIT_H
 #define COMMIT_H
 
+#include cache.h
 #include object.h
 #include tree.h
 #include strbuf.h
diff --git a/convert.h b/convert.h
index 0c2143c..82f81fa 100644
--- a/convert.h
+++ b/convert.h
@@ -4,6 +4,8 @@
 #ifndef CONVERT_H
 #define CONVERT_H
 
+#include strbuf.h
+
 enum safe_crlf {
SAFE_CRLF_FALSE = 0,
SAFE_CRLF_FAIL = 1,
diff --git a/csum-file.h b/csum-file.h
index bb543d5..9e29e35 100644
--- a/csum-file.h
+++ b/csum-file.h
@@ -1,6 +1,8 @@
 #ifndef CSUM_FILE_H
 #define CSUM_FILE_H
 
+#include cache.h
+
 struct progress;
 
 /* A SHA1-protected file */
diff --git a/diff.h b/diff.h
index b4a624d..27f7696 100644
--- a/diff.h
+++ b/diff.h
@@ -6,11 +6,11 @@
 
 #include tree-walk.h
 #include pathspec.h
+#include strbuf.h
 
 struct rev_info;
 struct diff_options;
 struct diff_queue_struct;
-struct strbuf;
 struct diff_filespec;
 struct userdiff_driver;
 struct sha1_array;
diff --git a/diffcore.h b/diffcore.h
index 33ea2de..7ae0293 100644
--- a/diffcore.h
+++ b/diffcore.h
@@ -9,6 +9,8 @@
  * in anything else.
  */
 
+#include diff.h
+
 /* We internally use unsigned short as the score value,
  * and rely on an int capable to hold 32-bits.  -B can take
  * -Bmerge_score/break_score format and the two scores are
diff --git a/dir.h b/dir.h
index 6c45e9d..3330771 100644
--- a/dir.h
+++ b/dir.h
@@ -3,7 +3,9 @@
 
 /* See Documentation/technical/api-directory-listing.txt */
 
+#include cache.h
 #include strbuf.h
+#include pathspec.h
 
 struct dir_entry {
unsigned int len;
diff --git a/ewah/ewok_rlw.h 

[PATCH v4 1/2] Makefile: add check-headers target

[David Aguilar]

This allows us to ensure that each header can be included
individually without needing to include other headers first.

Implicitly include git-compat-util.h during the check since
the implementation files will have already included it.

Helped-by: Jonathan Nieder jrnie...@gmail.com
Helped-by: Junio C Hamano gits...@pobox.com
Helped-by: Matthieu Moy matthieu@grenoble-inp.fr
Signed-off-by: David Aguilar dav...@gmail.com
---
Changes since last time:

We now automatically include git-compat-util.h during the check
per the CodingGuidelines rule to always include it in all .c files.

We now use a case statement to special-case common-cmds.h, which
allows us to replace usage of git ls-files and a pipe with a simpler
for header in ... loop.

 Makefile |  6 ++
 check-headers.sh | 34 ++
 2 files changed, 40 insertions(+)
 create mode 100755 check-headers.sh

diff --git a/Makefile b/Makefile
index e0f15a3..d7c9225 100644
--- a/Makefile
+++ b/Makefile
@@ -2408,6 +2408,12 @@ check-docs::
 check-builtins::
./check-builtins.sh
 
+### Make sure headers include their dependencies
+#
+check-headers::
+   ./check-headers.sh $(CC) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS)
+
+
 ### Test suite coverage testing
 #
 .PHONY: coverage coverage-clean coverage-compile coverage-test coverage-report
diff --git a/check-headers.sh b/check-headers.sh
new file mode 100755
index 000..ef06f56
--- /dev/null
+++ b/check-headers.sh
@@ -0,0 +1,34 @@
+#!/bin/sh
+
+exit_code=0
+
+maybe_exit () {
+   status=$1
+   if test $status != 0
+   then
+   exit_code=$status
+   if test -n $CHECK_HEADERS_STOP
+   then
+   exit $status
+   fi
+   fi
+}
+
+for header in *.h ewah/*.h vcs-svn/*.h xdiff/*.h
+do
+   case $header in
+   common-cmds.h)
+   # should only be included by help.c
+   ;;
+   *)
+   subdir=$(dirname $header) 
+   echo HEADER $header 
+   $@ -Wno-unused -I$subdir -include git-compat-util.h \
+   -c -o $header.check -x c - $header 
+   rm $header.check ||
+   maybe_exit $?
+   ;;
+   esac
+done
+
+exit $exit_code
-- 
2.0.4.1.g929bde9

--
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: [RFC/PATCH] mailinfo: do not treat From lines as in-body headers

[Jeff King]

On Sat, Sep 13, 2014 at 05:55:49PM -0700, Junio C Hamano wrote:

 On Sat, Sep 13, 2014 at 5:47 PM, Jeff King p...@peff.net wrote:
 
  On Sat, Sep 13, 2014 at 10:57:14PM +, brian m. carlson wrote:
 
   I wonder if git send-email should do what mutt does in this case, which
   is use quoted-printable encoding and encode the first F as =46 (as well
   as any equals signs as =3D).  It looks like mailinfo.c already is
   capable of handling that, and that would avoid the entire issue.
 
  That's not an unreasonable tactic. However, I think we'd still want to
  do something with mailinfo on the receiving end, similar to the patch I
  sent. We don't know that the sending side is necessarily send-email.
 
 Hmm, isn't the  stuffing in front of a beginning-of-line From  purely
 a local matter of MUA that stores messages in (old-style) mbox format
 where a line that begins with From  is what defines the end of the
 previous message?

Yes, it is[1].

 Why should send-email do anything when it sends individual messages
 separately out?

It does not need to, but the QP-transformation helps protect against
other, stupider software downstream.  And unlike From-quoting it is
actually well-specified and reversible.

-Peff

[1] We do use the mbox format in git, and AFAIK do not do any
From-quoting of this nature.  I haven't tested, but I suspect that
certain format-patch output would be corrupted when reading back via
git am, let alone other random mbox readers.  If we wanted to do
the QP magic brian suggests, it would probably make sense to do it
as part of format-patch.
--
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] help: ensure that common-cmds.h is only included by help.c

[David Aguilar]

Add a #ifndef guard to ensure that common-cmds.h can only
be included by help.c.

Suggested-by: Junio C Hamano gits...@pobox.com
Signed-off-by: David Aguilar dav...@gmail.com
---
 generate-cmdlist.sh | 4 
 help.c  | 3 ++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/generate-cmdlist.sh b/generate-cmdlist.sh
index 9a4c9b9..99cd140 100755
--- a/generate-cmdlist.sh
+++ b/generate-cmdlist.sh
@@ -1,6 +1,10 @@
 #!/bin/sh
 
 echo /* Automatically generated by $0 */
+#ifndef GIT_HELP_INTERNAL
+#error \common-cmds.h can only be included by help.c\
+#endif
+
 struct cmdname_help {
 char name[16];
 char help[80];
diff --git a/help.c b/help.c
index 7af65e2..abf1689 100644
--- a/help.c
+++ b/help.c
@@ -3,11 +3,12 @@
 #include exec_cmd.h
 #include levenshtein.h
 #include help.h
-#include common-cmds.h
 #include string-list.h
 #include column.h
 #include version.h
 #include refs.h
+#define GIT_HELP_INTERNAL
+#include common-cmds.h
 
 void add_cmdname(struct cmdnames *cmds, const char *name, int len)
 {
-- 
2.1.0.241.ga16d620

--
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: [RFC/PATCH] mailinfo: do not treat From lines as in-body headers

[Jeff King]

On Sat, Sep 13, 2014 at 09:01:20PM -0400, Jeff King wrote:

 [1] We do use the mbox format in git, and AFAIK do not do any
 From-quoting of this nature.  I haven't tested, but I suspect that
 certain format-patch output would be corrupted when reading back via
 git am, let alone other random mbox readers.  If we wanted to do
 the QP magic brian suggests, it would probably make sense to do it
 as part of format-patch.

It looks like we have a reasonably sane is_from_line() function. So at
least _we_ will not generally break on reading our own output, except in
some extreme circumstances (you'd have to come up with something
contrived like From me, at 10:30 30 minutes before 11!).

We can pretty easily reuse this to make the from-line check in mailinfo
more robust. Here's a replacement for the patch I sent earlier that
keeps the magically ignore extra From headers behavior but fixes the
case that started this discussion.

We could still do the QP thing to protect against other downstream tools
(or to cover the contrived cases as above), but I think with this patch
we at least cover the sane cases.

-- 8 --
Subject: mailinfo: make From in-body header check more robust

Since commit 81c5cf7 (mailinfo: skip bogus UNIX From line
inside body, 2006-05-21), we have treated lines like From
in the body as headers. This makes git am work for people
who erroneously paste the whole mbox entry:

  From 12345abcd...
  From: them
  Subject: [PATCH] whatever

into their email body (assuming that an mbox writer then
quotes From as From, as otherwise we would actually
mailsplit on the in-body line).

However, this has false positives if somebody actually has a
commit body that starts with From ; in this case we
erroneously remove the line entirely from the commit
message. We can make this check more robust by making sure
the line actually looks like a real mbox From line.

To do this we pull the is_from_line definition out of
mailsplit, and make it available for general use.

Signed-off-by: Jeff King p...@peff.net
---
 Makefile   |  1 +
 builtin/mailinfo.c |  2 +-
 builtin/mailsplit.c| 31 ---
 cache.h|  6 ++
 mbox.c | 32 
 t/t5100-mailinfo.sh| 18 ++
 t/t5100/embed-from.expect  |  5 +
 t/t5100/embed-from.in  | 13 +
 t/t5100/quoted-from.expect |  3 +++
 t/t5100/quoted-from.in | 10 ++
 10 files changed, 89 insertions(+), 32 deletions(-)
 create mode 100644 mbox.c
 create mode 100644 t/t5100/embed-from.expect
 create mode 100644 t/t5100/embed-from.in
 create mode 100644 t/t5100/quoted-from.expect
 create mode 100644 t/t5100/quoted-from.in

diff --git a/Makefile b/Makefile
index e0f15a3..e018450 100644
--- a/Makefile
+++ b/Makefile
@@ -704,6 +704,7 @@ LIB_OBJS += lockfile.o
 LIB_OBJS += log-tree.o
 LIB_OBJS += mailmap.o
 LIB_OBJS += match-trees.o
+LIB_OBJS += mbox.o
 LIB_OBJS += merge.o
 LIB_OBJS += merge-blobs.o
 LIB_OBJS += merge-recursive.o
diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index cf11c8d..a434d39 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -329,7 +329,7 @@ static int check_header(const struct strbuf *line,
 
/* for inbody stuff */
if (starts_with(line-buf, From)  isspace(line-buf[5])) {
-   ret = 1; /* Should this return 0? */
+   ret = is_from_line(line-buf + 1, line-len - 1);
goto check_header_out;
}
if (starts_with(line-buf, [PATCH])  isspace(line-buf[7])) {
diff --git a/builtin/mailsplit.c b/builtin/mailsplit.c
index 763cda0..11ba281 100644
--- a/builtin/mailsplit.c
+++ b/builtin/mailsplit.c
@@ -12,37 +12,6 @@
 static const char git_mailsplit_usage[] =
 git mailsplit [-dprec] [-fn] [-b] [--keep-cr] -odirectory 
[(mbox|Maildir)...];
 
-static int is_from_line(const char *line, int len)
-{
-   const char *colon;
-
-   if (len  20 || memcmp(From , line, 5))
-   return 0;
-
-   colon = line + len - 2;
-   line += 5;
-   for (;;) {
-   if (colon  line)
-   return 0;
-   if (*--colon == ':')
-   break;
-   }
-
-   if (!isdigit(colon[-4]) ||
-   !isdigit(colon[-2]) ||
-   !isdigit(colon[-1]) ||
-   !isdigit(colon[ 1]) ||
-   !isdigit(colon[ 2]))
-   return 0;
-
-   /* year */
-   if (strtol(colon+3, NULL, 10) = 90)
-   return 0;
-
-   /* Ok, close enough */
-   return 1;
-}
-
 static struct strbuf buf = STRBUF_INIT;
 static int keep_cr;
 
diff --git a/cache.h b/cache.h
index dfa1a56..9e71ca5 100644
--- a/cache.h
+++ b/cache.h
@@ -1568,4 +1568,10 @@ void stat_validity_update(struct stat_validity *sv, int 
fd);
 
 int versioncmp(const char *s1, const char *s2);
 
+/*
+ * Returns true if the line appears to be an mbox From line starting a new
+ * 

Re: [PATCH] help: ensure that common-cmds.h is only included by help.c

[Perry Hutchison]

David Aguilar dav...@gmail.com wrote:
 Add a #ifndef guard to ensure that common-cmds.h can only
 be included by help.c.

This strikes me as a very peculiar, and sub-optimal, way of
achieving the purpose.  If these definitions are intended to
be private to help.c, why not put them there and eliminate
common-cmds.h entirely?
--
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] help: ensure that common-cmds.h is only included by help.c

[Junio C Hamano]

On Sat, Sep 13, 2014 at 7:00 PM, Perry Hutchison per...@pluto.rain.com wrote:
 David Aguilar dav...@gmail.com wrote:
 Add a #ifndef guard to ensure that common-cmds.h can only
 be included by help.c.

 This strikes me as a very peculiar, and sub-optimal, way of
 achieving the purpose.  If these definitions are intended to
 be private to help.c, why not put them there and eliminate
 common-cmds.h entirely?

Have you studied where common-cmds.h comes from?
After you have done so, would you make the same suggestion?

Having said that, I also do not think this is such a good idea.
Wouldn't the new check script added in this series a better
place? For example, it may want to make sure that git-compat-util.h
(or a couple of its equivalents) is the first file included in any mainline
C source file, and such an inclusion is done unconditionally.

Which would mean that the checker would scan *.c files with grep
or a Perl script. It would be trivial to enforce nobody other than these
small selected C files is allowed to include common-cmds.h rule.

Regarding the other patch that butchers many *.h files, I am not
still very enthused. Including cache.h at the beginning of branch.h,
for example, would mean git-compat-util.h ends up included at the
beginning of branch.h.
--
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] compat-util: add _DEFAULT_SOURCE define

[Sergey Senozhatsky]

glibc has deprecated the use of _BSD_SOURCE define

  warning _BSD_SOURCE and _SVID_SOURCE are deprecated, use _DEFAULT_SOURCE

To make it easier to maintain a cross platform source code, that
warning can be suppressed by _DEFAULT_SOURCE.

Define both _BSD_SOURCE, _DEFAULT_SOURCE and cleanup the build.

Signed-off-by: Sergey Senozhatsky sergey.senozhat...@gmail.com
---
 git-compat-util.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/git-compat-util.h b/git-compat-util.h
index 4e7e3f8..08a9ee2 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -82,6 +82,7 @@
 #define _ALL_SOURCE 1
 #define _GNU_SOURCE 1
 #define _BSD_SOURCE 1
+#define _DEFAULT_SOURCE 1
 #define _NETBSD_SOURCE 1
 #define _SGI_SOURCE 1
 
-- 
2.1.0.350.g8b25fe0

--
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