Re: [PATCH 1/2] help: introduce option --command-only

[Philip Oakley]

From: "Ralf Thielow" 

Introduce option --command-only to the help command.  With this option
being passed, "git help" will open man pages only for commands.

Since we know it is a command, we can use function help_unknown_command
to give the user advice on typos.

Signed-off-by: Ralf Thielow 
---
I am not sure about the first test case, but I think it'd have
prevented me from making earlier mistakes of this change. That's
why I added it.
Just calling a git command that succeeds in a test isn't really
a check, so ... I dunno


Do the tests work on both *nix and Windows, given that Windows uses 
the --web option by default, so is likely to fire up a browser instead of 
the man pages? Otherwise it sounds to be a reasonable check.




Documentation/git-help.txt | 11 ---
builtin/help.c | 30 +++---
contrib/completion/git-completion.bash |  2 +-
t/t0012-help.sh| 21 +
4 files changed, 53 insertions(+), 11 deletions(-)
create mode 100755 t/t0012-help.sh

diff --git a/Documentation/git-help.txt b/Documentation/git-help.txt
index 40d328a..cf6a414 100644
--- a/Documentation/git-help.txt
+++ b/Documentation/git-help.txt
@@ -8,7 +8,7 @@ git-help - Display help information about Git
SYNOPSIS

[verse]
-'git help' [-a|--all] [-g|--guide]
+'git help' [-a|--all] [-c|--command-only] [-g|--guide]
[-i|--info|-m|--man|-w|--web] [COMMAND|GUIDE]

DESCRIPTION
@@ -29,8 +29,9 @@ guide is brought up. The 'man' program is used by 
default for this

purpose, but this can be overridden by other options or configuration
variables.

-Note that `git --help ...` is identical to `git help ...` because the
-former is internally converted into the latter.
+Note that `git --help ...` is almost identical to `git help ...` because
+the former is internally converted into the latter with 
option --command-only

+being added.

To display the linkgit:git[1] man page, use `git help git`.

@@ -43,6 +44,10 @@ OPTIONS
 Prints all the available commands on the standard output. This
 option overrides any given command or guide name.

+-c::
+--command-only::
+ Display help information only for commands.


s/commands/known commands/ ?


+
-g::
--guides::
 Prints a list of useful guides on the standard output. This
diff --git a/builtin/help.c b/builtin/help.c
index 8848013..2249a67 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -37,8 +37,10 @@ static int show_all = 0;
static int show_guides = 0;
static unsigned int colopts;
static enum help_format help_format = HELP_FORMAT_NONE;
+static int cmd_only;
static struct option builtin_help_options[] = {
 OPT_BOOL('a', "all", _all, N_("print all available commands")),
+ OPT_BOOL('c', "command-only", _only, N_("show help only for 
commands")),


s/commands/known commands/ ?


 OPT_BOOL('g', "guides", _guides, N_("print list of useful guides")),
 OPT_SET_INT('m', "man", _format, N_("show man page"), 
HELP_FORMAT_MAN),

 OPT_SET_INT('w', "web", _format, N_("show manual in web browser"),
@@ -433,10 +435,29 @@ static void list_common_guides_help(void)
 putchar('\n');
}

+static const char *check_git_cmd(const char* cmd)
+{
+ char *alias;
+
+ if (is_git_command(cmd))
+ return cmd;
+
+ alias = alias_lookup(cmd);
+ if (alias) {
+ printf_ln(_("`git %s' is aliased to `%s'"), cmd, alias);
+ free(alias);
+ exit(0);
+ }
+
+ if (cmd_only)
+ return help_unknown_cmd(cmd);
+
+ return cmd;
+}
+
int cmd_help(int argc, const char **argv, const char *prefix)
{
 int nongit;
- char *alias;
 enum help_format parsed_help_format;

 argc = parse_options(argc, argv, prefix, builtin_help_options,
@@ -476,12 +497,7 @@ int cmd_help(int argc, const char **argv, const char 
*prefix)

 if (help_format == HELP_FORMAT_NONE)
 help_format = parse_help_format(DEFAULT_HELP_FORMAT);

- alias = alias_lookup(argv[0]);
- if (alias && !is_git_command(argv[0])) {
- printf_ln(_("`git %s' is aliased to `%s'"), argv[0], alias);
- free(alias);
- return 0;
- }
+ argv[0] = check_git_cmd(argv[0]);

 switch (help_format) {
 case HELP_FORMAT_NONE:
diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash

index c1b2135..354afe5 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1393,7 +1393,7 @@ _git_help ()
{
 case "$cur" in
 --*)
- __gitcomp "--all --guides --info --man --web"
+ __gitcomp "--all --command-only --guides --info --man --web"
 return
 ;;
 esac
diff --git a/t/t0012-help.sh b/t/t0012-help.sh
new file mode 100755
index 000..e20f907
--- /dev/null
+++ b/t/t0012-help.sh
@@ -0,0 +1,21 @@
+#!/bin/sh
+
+test_description='help'
+
+. ./test-lib.sh
+
+test_expect_success "works for commands and guides by default" "
+ git help status &&
+ git help revisions
+"
+
+test_expect_success "--command-only does not work for guides" "
+ git help --command-only status &&
+ cat <<-EOF >expected &&
+ git: 

Re: [PATCH v7 5/7] submodule: correct output of submodule log format

[Stefan Beller]

On Wed, Aug 17, 2016 at 5:51 PM, Jacob Keller  wrote:
> From: Jacob Keller 
>
> The submodule log diff output incorrectly states that the submodule is
> "not checked out" in cases where it wants to say the submodule is "not
> initialized". Change the wording to reflect the actual check being
> performed.
>
> Signed-off-by: Jacob Keller 
> ---
>  submodule.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/submodule.c b/submodule.c
> index 1b5cdfb7e784..e1a51b7506ff 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -348,7 +348,7 @@ void show_submodule_summary(FILE *f, const char *path,
> if (is_null_sha1(two))
> message = "(submodule deleted)";
> else if (add_submodule_odb(path))
> -   message = "(not checked out)";
> +   message = "(not initialized)";

I think "not checked" out is actually correct here.

$ git clone https://gerrit.googlesource.com/gerrit
$ cd gerrit
$ git submodule update --init
  ...
$ git diff cc82b24..5222e66 plugins/
Submodule plugins/cookbook-plugin 2d40ee2..69b8f9f:
 > Organize imports
$ rm -rf plugins/cookbook-plugin/
$ git diff cc82b24..5222e66 plugins/
Submodule plugins/cookbook-plugin 2d40ee2...69b8f9f (not checked out)
$

Mind that by "rm -rf" of the working dir I create the "not checked out,
but initialized state and even cloned state".

So as a long term TODO:
I guess we could teach `add_submodule_odb` to operate
inside its git directory instead of its working directory, to have
it working whenever we have the object database (no matter if
checked out or not). Although this may collide with the plan of a
different refs backend? I dunno.

add_submodule_odb is used in a variety of places:
show_submodule_header
submodule_needs_pushing
push_submodule
is_submodule_commit_present
merge_submodule

And all of them seem to not require a checkout, but the presence of
objects is fine.
--
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 2/2] mingw: ensure temporary file handles are not inherited by child processes

[Johannes Schindelin]

Hi Eric,

On Wed, 17 Aug 2016, Eric Sunshine wrote:

> On Wed, Aug 17, 2016 at 8:41 AM, Johannes Schindelin
>  wrote:
> > When the index is locked and child processes inherit the handle to
> > said lock and the parent process wants to remove the lock before the
> > child process exits, on Windows there is a problem: it won't work
> > because files cannot be deleted if a process holds a handle on them.
> > The symptom:
> >
> > Rename from 'xxx/.git/index.lock' to 'xxx/.git/index' failed.
> > Should I try again? (y/n)
> >
> > Spawning child processes with bInheritHandles==FALSE would not work
> > because no file handles would be inherited, not even the hStdXxx
> > handles in STARTUPINFO (stdin/stdout/stderr).
> >
> > Opening every file with O_NOINHERIT does not work, either, as e.g.
> > git-upload-pack expects inherited file handles.
> >
> > This leaves us with the only way out: creating temp files with the
> > O_NOINHERIT flag. This flag is Windows-specific, however. For our
> > purposes, it is equivalent our purposes) to O_CLOEXEC (which does not
> 
> s/our purposes)//

Thanks!
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: Working with zip files

[David Lang]

On Thu, 18 Aug 2016, Jakub Narębski wrote:


On 18 August 2016 at 18:56, David Lang  wrote:

On Thu, 18 Aug 2016, Jakub Narębski wrote:


JN>> You can find rezip clean/smudge filter (originally intended for
JN>> OpenDocument Format (ODF), that is OpenOffice.org etc.) that stores
JN>> zip or zip-archive (like ODT, jar, etc.) uncompressed.  I think
JN>> you can find it on GitWiki, but I might be mistaken.

Using 'unzip -c' as separate / additional `textconv` filter for diff
generation allows to separate the problem of deltifiable storage format
from textual representation for diff-ing.

Though best results could be had with `diff` and `merge` drivers...



can you point at an example of how to do this? when I went looking about a
year ago to deal with single-line json data I wasn't able to find anything
good. I ended up using clean/smudge to pretty-print the json so it was
easier to handle.


Pro Git has a chapter "Customizing Git - Git Attributes" about gitattributes
https://git-scm.com/book/en/v2/Customizing-Git-Git-Attributes

The section "Diffing Binary Files" has two examples: docx2txt (with wrapper)
for DOCX (MS Word) files, and exiftool for images. For JSON you could use
some prettyprinter / formatter like pp-json.

"Performing text diffs of binary files" section of gitattributes(1) manpage
covers 'textconv' vs 'diff', and uses 'exif' tool as textconv example.


As I read that section, it only applies to the human readable output of git 
diff.


And the merge section only talks about the default of using patch vs accepting a 
specific version in a merge.


It seems to me that what I'm looking for would be something to tell git to use a 
different command instead of diff/patch internally when creating and using the 
bundles.


David Lang

Re: [PATCH v7 5/7] submodule: correct output of submodule log format

[Jacob Keller]

On Thu, Aug 18, 2016 at 11:25 AM, Stefan Beller  wrote:
> On Wed, Aug 17, 2016 at 5:51 PM, Jacob Keller  
> wrote:
>> From: Jacob Keller 
>>
>> The submodule log diff output incorrectly states that the submodule is
>> "not checked out" in cases where it wants to say the submodule is "not
>> initialized". Change the wording to reflect the actual check being
>> performed.
>>
>> Signed-off-by: Jacob Keller 
>> ---
>>  submodule.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/submodule.c b/submodule.c
>> index 1b5cdfb7e784..e1a51b7506ff 100644
>> --- a/submodule.c
>> +++ b/submodule.c
>> @@ -348,7 +348,7 @@ void show_submodule_summary(FILE *f, const char *path,
>> if (is_null_sha1(two))
>> message = "(submodule deleted)";
>> else if (add_submodule_odb(path))
>> -   message = "(not checked out)";
>> +   message = "(not initialized)";
>
> I think "not checked" out is actually correct here.
>

Hmmm.. It shouldn't say not checked out. I was running iinto problems
in some of my tests which indicated that the entire sub module didn't
exist. I think I had already assumed it wasn't failing when no
checkout. I think I have some fix for add_submodule_odb that can help
with this, making submodule dir behave correctly for this case.

I want add_submodule_odb to work in both of these cases:

a) we don't have a working checkout but we have the objects in the
usual location
b) we have a working directory with objects inside a .git folder but
it hasn't yet been moved into .git/modules/<>

I'll take a look and see what I can do.

> $ git clone https://gerrit.googlesource.com/gerrit
> $ cd gerrit
> $ git submodule update --init
>   ...
> $ git diff cc82b24..5222e66 plugins/
> Submodule plugins/cookbook-plugin 2d40ee2..69b8f9f:
>  > Organize imports
> $ rm -rf plugins/cookbook-plugin/
> $ git diff cc82b24..5222e66 plugins/
> Submodule plugins/cookbook-plugin 2d40ee2...69b8f9f (not checked out)
> $
>
> Mind that by "rm -rf" of the working dir I create the "not checked out,
> but initialized state and even cloned state".
>
> So as a long term TODO:
> I guess we could teach `add_submodule_odb` to operate
> inside its git directory instead of its working directory, to have
> it working whenever we have the object database (no matter if
> checked out or not). Although this may collide with the plan of a
> different refs backend? I dunno.
>
> add_submodule_odb is used in a variety of places:
> show_submodule_header
> submodule_needs_pushing
> push_submodule
> is_submodule_commit_present
> merge_submodule
>
> And all of them seem to not require a checkout, but the presence of
> objects is fine.

Yea, they really only need objects, not the working tree.

Thanks,
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: [PATCHv8 8/8] clone: recursive and reference option triggers submodule alternates

[Stefan Beller]

On Wed, Aug 17, 2016 at 5:17 PM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>>  and now with error handling of invalid options.
>
> Thanks.

Well this was not the end of the story. I sent that version out,
before doing a final test run, because changing a few lines is trivial right? :(

I'll resend with:

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index a1da3ea..a366757 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -562,7 +562,7 @@ static void prepare_possible_alternates(const char *sm_name,

if (!strcmp(sm_alternate, "superproject"))
foreach_alt_odb(add_possible_reference_from_superproject, );
-   else if (!strcmp(sm_alternate, "no")
+   else if (!strcmp(sm_alternate, "no"))
; /* do nothing */
else
die(_("Value '%s' for submodule.alternateLocation is
not recognized"), sm_alternate);
--
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 v7 6/7] submodule: refactor show_submodule_summary with helper function

[Jacob Keller]

On Thu, Aug 18, 2016 at 12:04 PM, Stefan Beller  wrote:
> On Wed, Aug 17, 2016 at 5:51 PM, Jacob Keller  
> wrote:
>> +   if (is_null_sha1(one))
>> +   message = "(new submodule)";
>> +   else if (is_null_sha1(two))
>> +   message = "(submodule deleted)";
>> +
>> +   if (add_submodule_odb(path)) {
>> +   if (!message)
>> +   message = "(submodule not initialized)";
>
> Before it was  "(not initialized)" for this case, I think we'd rather keep 
> that?
> Though this code path is only used by the porcelain commands, we'd rather not
> want to change this in a subtle way in this commit.
>
> If we were to change those, we could discuss if we want to go with
> full sentences
> all the time:
>
> submodule is new
> submodule is deleted
> submodule is not initialized
>
>

I agree, I'll make a new patch that does this as a cleanup prior to
this re-work.

Thanks,
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 2/2] mingw: ensure temporary file handles are not inherited by child processes

[Johannes Schindelin]

Hi Junio,

On Wed, 17 Aug 2016, Junio C Hamano wrote:

> But it deserves mention in the lockfile and the tempfile API docs in
>  and  that the file descriptor returned from
> these public entry points does not survive across fork(2).

I touched the comments up a little bit, and I also fixed the first commit
message as per your reply.

Will send out v2 in a moment.

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

[PATCH v2 0/2] Do not lock temporary files via child processes on Windows

[Johannes Schindelin]

This issue was originally reported and fixed in
https://github.com/git-for-windows/git/pull/755

The problem is that file handles to temporary files (such as
index.lock) were inherited by spawned processes. If those spawned
processes do not exit before the parent process wants to delete or
rename them, we are in big trouble.

The original use case triggering the bug is a merge driver that does
not quit, but listen to subsequent merge requests.

However, the same issue turned up in Lars Schneider's work on making
clean/smudge filters batchable (i.e. more efficient by avoiding
possibly thousands of child processes, one per file).

Changes since v1:

- the two commit messages have been corrected, as per Junio's and Eric's
  suggestion,
- lockfile.h and tempfile.h now sport explicit documentation that the
  current process needs to write to the files, no spawned processes.


Ben Wijen (2):
  t6026-merge-attr: child processes must not inherit index.lock handles
  mingw: ensure temporary file handles are not inherited by child
processes

 compat/mingw.h|  4 
 lockfile.h|  4 
 t/t6026-merge-attr.sh | 13 +
 tempfile.c|  2 +-
 tempfile.h|  4 
 5 files changed, 26 insertions(+), 1 deletion(-)

Published-As: https://github.com/dscho/git/releases/tag/mingw-index-lock-v2
Fetch-It-Via: git fetch https://github.com/dscho/git mingw-index-lock-v2

Interdiff vs v1:

 diff --git a/lockfile.h b/lockfile.h
 index 3d30193..d26ad27 100644
 --- a/lockfile.h
 +++ b/lockfile.h
 @@ -55,6 +55,10 @@
   *   * calling `fdopen_lock_file()` to get a `FILE` pointer for the
   * open file and writing to the file using stdio.
   *
 + *   Note that the file descriptor returned by hold_lock_file_for_update()
 + *   is marked O_CLOEXEC, so the new contents must be written by the
 + *   current process, not a spawned one.
 + *
   * When finished writing, the caller can:
   *
   * * Close the file descriptor and rename the lockfile to its final
 diff --git a/tempfile.h b/tempfile.h
 index 4219fe4..2f0038d 100644
 --- a/tempfile.h
 +++ b/tempfile.h
 @@ -33,6 +33,10 @@
   *   * calling `fdopen_tempfile()` to get a `FILE` pointer for the
   * open file and writing to the file using stdio.
   *
 + *   Note that the file descriptor returned by create_tempfile()
 + *   is marked O_CLOEXEC, so the new contents must be written by
 + *   the current process, not any spawned one.
 + *
   * When finished writing, the caller can:
   *
   * * Close the file descriptor and remove the temporary file by

-- 
2.9.2.691.g78954f3

base-commit: d63263a4dee8fc7da9b97bbdedf9c0d1f33024d4
--
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 v7 7/7] diff: teach diff to display submodule difference with an inline diff

[Jacob Keller]

On Thu, Aug 18, 2016 at 12:47 PM, Stefan Beller  wrote:
> On Wed, Aug 17, 2016 at 5:51 PM, Jacob Keller  
> wrote:
>
>
>
>> if (o->submodule_format == DIFF_SUBMODULE_LOG &&
>> (!one->mode || S_ISGITLINK(one->mode)) &&
>> (!two->mode || S_ISGITLINK(two->mode))) {
>> @@ -2311,6 +2322,17 @@ static void builtin_diff(const char *name_a,
>> two->dirty_submodule,
>> meta, del, add, reset);
>> return;
>> +   } else if (o->submodule_format == DIFF_SUBMODULE_INLINE_DIFF &&
>> +  (!one->mode || S_ISGITLINK(one->mode)) &&
>> +  (!two->mode || S_ISGITLINK(two->mode))) {
>
> The ! mode is for added and deleted submodules, I guess?
>

I think so? I don't really know, but it was there before for
show_submodule_summary so I left it this way.

>> diff --git a/diff.h b/diff.h
>> index ea5aba668eaa..192c0eedd0ff 100644
>> --- a/diff.h
>> +++ b/diff.h
>> @@ -112,6 +112,7 @@ enum diff_words_type {
>>  enum diff_submodule_format {
>> DIFF_SUBMODULE_SHORT = 0,
>> DIFF_SUBMODULE_LOG,
>> +   DIFF_SUBMODULE_INLINE_DIFF,
>>  };
>>
>>  struct diff_options {
>> diff --git a/submodule.c b/submodule.c
>> index e341ca7ffefd..e5f1138f4362 100644
>> --- a/submodule.c
>> +++ b/submodule.c
>> @@ -436,6 +436,67 @@ void show_submodule_summary(FILE *f, const char *path,
>> clear_commit_marks(right, ~0);
>>  }
>>
>> +void show_submodule_inline_diff(FILE *f, const char *path,
>> +   const char *line_prefix,
>> +   unsigned char one[20], unsigned char two[20],
>> +   unsigned dirty_submodule, const char *meta,
>> +   const char *del, const char *add, const char *reset,
>> +   const struct diff_options *o)
>> +{
>> +   const char *old = EMPTY_TREE_SHA1_BIN, *new = EMPTY_TREE_SHA1_BIN;
>
> submodule.c: In function ‘show_submodule_inline_diff’:
> cache.h:957:3: warning: pointer targets in initialization differ in
> signedness [-Wpointer-sign]
>((const unsigned char *) EMPTY_TREE_SHA1_BIN_LITERAL)
>
> submodule.c:446:20: note: in expansion of macro ‘EMPTY_TREE_SHA1_BIN’
>   const char *old = EMPTY_TREE_SHA1_BIN, *new = EMPTY_TREE_SHA1_BIN;
>

This should actually be "EMPTY_TREE_SHA1_BIN_LITERAL, and these should
be unsigned, you're right. This will be fixed differently by using
struct object_id * instead, though.

Thanks,
Jake

>
>
>> +   struct commit *left = NULL, *right = NULL;
>> +   struct strbuf submodule_dir = STRBUF_INIT;
>> +   struct child_process cp = CHILD_PROCESS_INIT;
>> +
>> +   show_submodule_header(f, path, line_prefix, one, two, 
>> dirty_submodule,
>> + meta, reset, , );
>> +
>> +   /* We need a valid left and right commit to display a difference */
>> +   if (!(left || is_null_sha1(one)) ||
>> +   !(right || is_null_sha1(two)))
>> +   goto done;
>> +
>> +   if (left)
>> +   old = one;
>
> submodule.c:460:7: warning: pointer targets in assignment differ in
> signedness [-Wpointer-sign]
>old = one;
>
>

This will also be fixed by switching to object_id.

>
>> +   if (right)
>> +   new = two;
>> +
>> +   fflush(f);
>> +   cp.git_cmd = 1;
>> +   cp.dir = path;
>> +   cp.out = dup(fileno(f));
>> +   cp.no_stdin = 1;
>> +
>> +   /* TODO: other options may need to be passed here. */
>> +   argv_array_pushl(, "diff");
>> +   argv_array_pushf(, "--line-prefix=%s", line_prefix);
>> +   if (DIFF_OPT_TST(o, REVERSE_DIFF)) {
>> +   argv_array_pushf(, "--src-prefix=%s%s/",
>> +o->b_prefix, path);
>> +   argv_array_pushf(, "--dst-prefix=%s%s/",
>> +o->a_prefix, path);
>> +   } else {
>> +   argv_array_pushf(, "--src-prefix=%s%s/",
>> +o->a_prefix, path);
>> +   argv_array_pushf(, "--dst-prefix=%s%s/",
>> +o->b_prefix, path);
>> +   }
>> +   argv_array_push(, sha1_to_hex(old));
>
> submodule.c:484:2: warning: pointer targets in passing argument 1 of
> ‘sha1_to_hex’ differ in signedness [-Wpointer-sign]
>   argv_array_push(, sha1_to_hex(old));
>
>
> /*
>  * nit: the following comment doesn't adhere to Gits way of doing comments:
>  */
>

Yea, sorry. I have a habbit of the other format because the Linux
kernel networking tree requires this style, despite the rest of the
Linux kernel requiring the style used by git. So, I have to break out
of that habbit but sometimes I miss them.

>> +   /* If the submodule has modified content, we will diff against the
>> +* work tree, under the assumption that the user has asked for the
>> +* diff format and wishes to actually see all differences even if 
>> they
>> +* 

Re: [PATCH v7 6/7] submodule: refactor show_submodule_summary with helper function

[Stefan Beller]

On Thu, Aug 18, 2016 at 1:24 PM, Jacob Keller  wrote:
> On Thu, Aug 18, 2016 at 12:04 PM, Stefan Beller  wrote:
>> On Wed, Aug 17, 2016 at 5:51 PM, Jacob Keller  
>> wrote:
>>> +   if (is_null_sha1(one))
>>> +   message = "(new submodule)";
>>> +   else if (is_null_sha1(two))
>>> +   message = "(submodule deleted)";
>>> +
>>> +   if (add_submodule_odb(path)) {
>>> +   if (!message)
>>> +   message = "(submodule not initialized)";
>>
>> Before it was  "(not initialized)" for this case, I think we'd rather keep 
>> that?
>> Though this code path is only used by the porcelain commands, we'd rather not
>> want to change this in a subtle way in this commit.
>>
>> If we were to change those, we could discuss if we want to go with
>> full sentences
>> all the time:
>>
>> submodule is new
>> submodule is deleted
>> submodule is not initialized
>>
>>
>
> I agree, I'll make a new patch that does this as a cleanup prior to
> this re-work.

I was actually not suggesting to change that. I rather wanted to point out
that if we'd want that we'd rather do it consistently and in a different patch.
Sorry for not being clear.

Thanks,
Stefan

>
> Thanks,
> 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 v7 6/7] submodule: refactor show_submodule_summary with helper function

[Stefan Beller]

On Wed, Aug 17, 2016 at 5:51 PM, Jacob Keller  wrote:
> From: Jacob Keller 
>
> A future patch is going to add a new submodule diff format which
> displays an inline diff of the submodule changes. To make this easier,
> and to ensure that both submodule diff formats use the same initial
> header, factor out show_submodule_header() function which will print the
> current submodule header line, and then leave the show_submodule_summary
> function to lookup and print the submodule log format.
>
> This does create one format change in that "(revision walker failed)"
> will now be displayed on its own line rather than as part of the message
> because we no longer perform this step directly in the header display
> flow. However, this is a rare case and shouldn't impact much. In
> addition, since we no longer need to run prepare_submodule_summary to
> get the fast_backward and fast_forward variables, these have been
> removed from that call, and the show_submodule_header() function does
> its own mergebase lookup.
>
> Signed-off-by: Jacob Keller 
> ---
>  submodule.c | 103 
> +++-
>  1 file changed, 74 insertions(+), 29 deletions(-)
>
> diff --git a/submodule.c b/submodule.c
> index e1a51b7506ff..e341ca7ffefd 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -277,8 +277,7 @@ void handle_ignore_submodules_arg(struct diff_options 
> *diffopt,
>  }
>
>  static int prepare_submodule_summary(struct rev_info *rev, const char *path,
> -   struct commit *left, struct commit *right,
> -   int *fast_forward, int *fast_backward)
> +   struct commit *left, struct commit *right)
>  {
> struct commit_list *merge_bases, *list;
>
> @@ -290,12 +289,6 @@ static int prepare_submodule_summary(struct rev_info 
> *rev, const char *path,
> add_pending_object(rev, >object, path);
> add_pending_object(rev, >object, path);
> merge_bases = get_merge_bases(left, right);
> -   if (merge_bases) {
> -   if (merge_bases->item == left)
> -   *fast_forward = 1;
> -   else if (merge_bases->item == right)
> -   *fast_backward = 1;
> -   }
> for (list = merge_bases; list; list = list->next) {
> list->item->object.flags |= UNINTERESTING;
> add_pending_object(rev, >item->object,
> @@ -333,31 +326,23 @@ static void print_submodule_summary(struct rev_info 
> *rev, FILE *f,
> strbuf_release();
>  }
>
> -void show_submodule_summary(FILE *f, const char *path,
> +/* Helper function to display the submodule header line prior to the full
> + * summary output. If it can locate the submodule objects directory it will
> + * attempt to lookup both the left and right commits and put them into the
> + * left and right pointers.
> + */
> +static void show_submodule_header(FILE *f, const char *path,
> const char *line_prefix,
> unsigned char one[20], unsigned char two[20],
> unsigned dirty_submodule, const char *meta,
> -   const char *del, const char *add, const char *reset)
> +   const char *reset,
> +   struct commit **left, struct commit **right)
>  {
> -   struct rev_info rev;
> -   struct commit *left = NULL, *right = NULL;
> +   struct commit_list *merge_bases;
> const char *message = NULL;
> struct strbuf sb = STRBUF_INIT;
> int fast_forward = 0, fast_backward = 0;
>
> -   if (is_null_sha1(two))
> -   message = "(submodule deleted)";
> -   else if (add_submodule_odb(path))
> -   message = "(not initialized)";
> -   else if (is_null_sha1(one))
> -   message = "(new submodule)";
> -   else if (!(left = lookup_commit_reference(one)) ||
> -!(right = lookup_commit_reference(two)))
> -   message = "(commits not present)";
> -   else if (prepare_submodule_summary(, path, left, right,
> -  _forward, _backward))
> -   message = "(revision walker failed)";
> -
> if (dirty_submodule & DIRTY_SUBMODULE_UNTRACKED)
> fprintf(f, "%sSubmodule %s contains untracked content\n",
> line_prefix, path);
> @@ -365,11 +350,46 @@ void show_submodule_summary(FILE *f, const char *path,
> fprintf(f, "%sSubmodule %s contains modified content\n",
> line_prefix, path);
>
> +   if (is_null_sha1(one))
> +   message = "(new submodule)";
> +   else if (is_null_sha1(two))
> +   message = "(submodule deleted)";
> +
> +   if (add_submodule_odb(path)) {
> +   if (!message)
> +   message = "(submodule not initialized)";

Before it was  "(not initialized)" for this case, I think 

Donation Funds

[Mrs Julie Leach]

You are a recipient to Mrs Julie leach Donation of 2M USD. Contact 
(julie_leac...@hotmail.com) for claims

--
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/4] cat-file: support --textconv/--filters in batch mode

[Torsten Bögershausen]

On Thu, Aug 18, 2016 at 02:46:28PM +0200, Johannes Schindelin wrote:
> With this patch, --batch can be combined with --textconv or --filters.
> For this to work, the input needs to have the form
> 
>   
> 
> so that the filters can be chosen appropriately.
> 
> Signed-off-by: Johannes Schindelin 
> ---
>  Documentation/git-cat-file.txt | 18 +++-
>  builtin/cat-file.c | 49 
> +-
>  t/t8010-cat-file-filters.sh| 10 +
>  3 files changed, 67 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
> index 59a3c37..1f4d954 100644
> --- a/Documentation/git-cat-file.txt
> +++ b/Documentation/git-cat-file.txt
> @@ -10,7 +10,7 @@ SYNOPSIS
>  
>  [verse]
>  'git cat-file' (-t [--allow-unknown-type]| -s [--allow-unknown-type]| -e | 
> -p |  | --textconv | --filters ) [--use-path=] 
> -'git cat-file' (--batch | --batch-check) [--follow-symlinks]
> +'git cat-file' (--batch | --batch-check) [ --textconv | --filters ] 
> [--follow-symlinks]
>  
>  DESCRIPTION
>  ---
> @@ -20,7 +20,11 @@ object type, or `-s` is used to find the object size, or 
> `--textconv` or
>  `--filters` is used (which imply type "blob").
>  
>  In the second form, a list of objects (separated by linefeeds) is provided on
> -stdin, and the SHA-1, type, and size of each object is printed on stdout.
> +stdin, and the SHA-1, type, and size of each object is printed on stdout. The
> +output format can be overridden using the optional `` argument. If
> +either `--textconv` or `--filters` was specified, the input is expected to
> +list the object names followed by the path name, separated by a single white
> +space, so that the appropriate drivers can be determined.
>  
>  OPTIONS
>  ---
> @@ -72,13 +76,17 @@ OPTIONS
>  --batch::
>  --batch=::
>   Print object information and contents for each object provided
> - on stdin.  May not be combined with any other options or arguments.
> - See the section `BATCH OUTPUT` below for details.
> + on stdin.  May not be combined with any other options or arguments
> + except `--textconv` or `--filters`, in which case the input lines
> + also need to specify the path, separated by white space.  See the
> + section `BATCH OUTPUT` below for details.
>  
>  --batch-check::
>  --batch-check=::
>   Print object information for each object provided on stdin.  May
> - not be combined with any other options or arguments.  See the
> + not be combined with any other options or arguments except
> + `--textconv` or `--filters`, in which case the input lines also
> + need to specify the path, separated by white space.  See the
>   section `BATCH OUTPUT` below for details.
>  
>  --batch-all-objects::
> diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> index 5ff58b3..5f91cf4 100644
> --- a/builtin/cat-file.c
> +++ b/builtin/cat-file.c
> @@ -17,6 +17,7 @@ struct batch_options {
>   int print_contents;
>   int buffer_output;
>   int all_objects;
> + int cmdmode; /* may be 'w' or 'c' for --filters or --textconv */
How do I read 'w' and 'c' ?
wilter and cextconv ? Does it make sense to use an enum here ?
Or a #define ?

--
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 v8 4/8] submodule: allow add_submodule_odb to work even if path is not checked out

[Jacob Keller]

From: Jacob Keller 

Currently, do_submodule_path will first try to locate the git directory
using read_gitfile on /.git. If this fails, it goes
ahead and assumes the path is actually the git directory. This is good
as it allows submodules which aren't stored in the superproject's .git
directory to function correctly. However, in some cases the submodule is
no longer locally checked out, but still has object data stored in the
parent project's .git/modules/.

To make this work, add code to check if we found a valid git directory.
If we haven't, then try the standard location of module data instead.
This has the advantage of allowing callers of do_submodule_path
(add_submodule_odb) to correctly function even if the submodule isn't
currently checked out, but was previously initialized.

Update the wording of the submodule log diff format to correctly
display that the submodule is "not initialized" instead of "not checked
out"

Add tests to ensure that even once we remove the submodule directory, it
works by checking in the .git directory.

Signed-off-by: Jacob Keller 
---
 path.c|   5 ++
 submodule.c   |   2 +-
 t/t4059-diff-submodule-not-initialized.sh | 105 ++
 3 files changed, 111 insertions(+), 1 deletion(-)
 create mode 100755 t/t4059-diff-submodule-not-initialized.sh

diff --git a/path.c b/path.c
index 17551c483476..0cb30123e988 100644
--- a/path.c
+++ b/path.c
@@ -482,6 +482,11 @@ static void do_submodule_path(struct strbuf *buf, const 
char *path,
strbuf_reset(buf);
strbuf_addstr(buf, git_dir);
}
+   if (!is_git_directory(buf->buf)) {
+   strbuf_reset(buf);
+   strbuf_git_path(buf, "%s/%s", "modules", path);
+   }
+
strbuf_addch(buf, '/');
strbuf_addbuf(_submodule_dir, buf);
 
diff --git a/submodule.c b/submodule.c
index 1b5cdfb7e784..e1a51b7506ff 100644
--- a/submodule.c
+++ b/submodule.c
@@ -348,7 +348,7 @@ void show_submodule_summary(FILE *f, const char *path,
if (is_null_sha1(two))
message = "(submodule deleted)";
else if (add_submodule_odb(path))
-   message = "(not checked out)";
+   message = "(not initialized)";
else if (is_null_sha1(one))
message = "(new submodule)";
else if (!(left = lookup_commit_reference(one)) ||
diff --git a/t/t4059-diff-submodule-not-initialized.sh 
b/t/t4059-diff-submodule-not-initialized.sh
new file mode 100755
index ..cc787454033a
--- /dev/null
+++ b/t/t4059-diff-submodule-not-initialized.sh
@@ -0,0 +1,105 @@
+#!/bin/sh
+#
+# Copyright (c) 2016 Jacob Keller, based on t4041 by Jens Lehmann
+#
+
+test_description='Test for submodule diff on non-checked out submodule
+
+This test tries to verify that add_submodule_odb works when the submodule was
+initialized previously but the checkout has since been removed.
+'
+
+TEST_NO_CREATE_REPO=1
+. ./test-lib.sh
+
+# Tested non-UTF-8 encoding
+test_encoding="ISO8859-1"
+
+# String "added" in German (translated with Google Translate), encoded in 
UTF-8,
+# used in sample commit log messages in add_file() function below.
+added=$(printf "hinzugef\303\274gt")
+add_file () {
+   (
+   cd "$1" &&
+   shift &&
+   for name
+   do
+   echo "$name" >"$name" &&
+   git add "$name" &&
+   test_tick &&
+   # "git commit -m" would break MinGW, as Windows refuse 
to pass
+   # $test_encoding encoded parameter to git.
+   echo "Add $name ($added $name)" | iconv -f utf-8 -t 
$test_encoding |
+   git -c "i18n.commitEncoding=$test_encoding" commit -F -
+   done >/dev/null &&
+   git rev-parse --short --verify HEAD
+   )
+}
+commit_file () {
+   test_tick &&
+   git commit "$@" -m "Commit $*" >/dev/null
+}
+
+test_create_repo sm1 &&
+test_create_repo sm2 &&
+add_file sm1 foo >/dev/null &&
+add_file sm2 foo1 foo2 >/dev/null &&
+smhead1=$(cd sm2; git rev-parse --short --verify HEAD) &&
+cd sm1
+
+test_expect_success 'setup - submodule directory' '
+   git submodule add ../sm2 sm2 &&
+   commit_file sm2 &&
+   git diff-tree -p --no-commit-id --submodule=log HEAD >actual &&
+   cat >expected <<-EOF &&
+   Submodule sm2 000...$smhead1 (new submodule)
+   EOF
+   test_cmp expected actual
+'
+
+test_expect_success 'submodule directory removed' '
+   rm -rf sm2 &&
+   git diff-tree -p --no-commit-id --submodule=log HEAD >actual &&
+   cat >expected <<-EOF &&
+   Submodule sm2 000...$smhead1 (new submodule)
+   EOF
+   test_cmp expected actual
+'
+
+test_expect_success 'setup - submodule multiple commits' '
+   git submodule update --checkout 

Re: [PATCH 4/4] cat-file: support --textconv/--filters in batch mode

[Jeff King]

On Thu, Aug 18, 2016 at 02:46:28PM +0200, Johannes Schindelin wrote:

> With this patch, --batch can be combined with --textconv or --filters.
> For this to work, the input needs to have the form
> 
>   
> 
> so that the filters can be chosen appropriately.

The object name can have spaces in it, too. E.g.:

  HEAD:path with spaces

or even:

  :/grep for this

(as was pointed out to me when I tried to turn on %(rest) handling by
default, long ago). How do those work with your patch?

It looks like the extra split isn't enabled unless one of those options
is selected. Since --filters is new, that's OK for backwards
compatibility. But --textconv isn't. So I think:

  echo "HEAD:path with spaces" |
  git cat-file --textconv --batch

is regressed by this patch.

I wonder if we need an option specifically to say "the object name can
be split". Right now it kicks in automatically if you use "%(rest)" in
your format, but you might not care about passing along that output
(e.g., a lot of times I am piping "rev-list" straight to cat-file, and I
have to use a separate "cut" to throw away the pathnames).

-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 v8 5/8] submodule: convert show_submodule_summary to use struct object_id *

[Jacob Keller]

From: Jacob Keller 

Since we're going to be changing this function in a future patch, lets
go ahead and convert this to use object_id now.

Signed-off-by: Jacob Keller 
---
 diff.c  |  2 +-
 submodule.c | 16 
 submodule.h |  2 +-
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/diff.c b/diff.c
index d6b321da3d1d..16253b191f53 100644
--- a/diff.c
+++ b/diff.c
@@ -2307,7 +2307,7 @@ static void builtin_diff(const char *name_a,
const char *add = diff_get_color_opt(o, DIFF_FILE_NEW);
show_submodule_summary(o->file, one->path ? one->path : 
two->path,
line_prefix,
-   one->oid.hash, two->oid.hash,
+   >oid, >oid,
two->dirty_submodule,
meta, del, add, reset);
return;
diff --git a/submodule.c b/submodule.c
index e1a51b7506ff..422353ccf6cc 100644
--- a/submodule.c
+++ b/submodule.c
@@ -335,7 +335,7 @@ static void print_submodule_summary(struct rev_info *rev, 
FILE *f,
 
 void show_submodule_summary(FILE *f, const char *path,
const char *line_prefix,
-   unsigned char one[20], unsigned char two[20],
+   struct object_id *one, struct object_id *two,
unsigned dirty_submodule, const char *meta,
const char *del, const char *add, const char *reset)
 {
@@ -345,14 +345,14 @@ void show_submodule_summary(FILE *f, const char *path,
struct strbuf sb = STRBUF_INIT;
int fast_forward = 0, fast_backward = 0;
 
-   if (is_null_sha1(two))
+   if (is_null_oid(two))
message = "(submodule deleted)";
else if (add_submodule_odb(path))
message = "(not initialized)";
-   else if (is_null_sha1(one))
+   else if (is_null_oid(one))
message = "(new submodule)";
-   else if (!(left = lookup_commit_reference(one)) ||
-!(right = lookup_commit_reference(two)))
+   else if (!(left = lookup_commit_reference(one->hash)) ||
+!(right = lookup_commit_reference(two->hash)))
message = "(commits not present)";
else if (prepare_submodule_summary(, path, left, right,
   _forward, _backward))
@@ -365,16 +365,16 @@ void show_submodule_summary(FILE *f, const char *path,
fprintf(f, "%sSubmodule %s contains modified content\n",
line_prefix, path);
 
-   if (!hashcmp(one, two)) {
+   if (!oidcmp(one, two)) {
strbuf_release();
return;
}
 
strbuf_addf(, "%s%sSubmodule %s %s..", line_prefix, meta, path,
-   find_unique_abbrev(one, DEFAULT_ABBREV));
+   find_unique_abbrev(one->hash, DEFAULT_ABBREV));
if (!fast_backward && !fast_forward)
strbuf_addch(, '.');
-   strbuf_addf(, "%s", find_unique_abbrev(two, DEFAULT_ABBREV));
+   strbuf_addf(, "%s", find_unique_abbrev(two->hash, DEFAULT_ABBREV));
if (message)
strbuf_addf(, " %s%s\n", message, reset);
else
diff --git a/submodule.h b/submodule.h
index 2af939099819..d83df57e24ff 100644
--- a/submodule.h
+++ b/submodule.h
@@ -43,7 +43,7 @@ const char *submodule_strategy_to_string(const struct 
submodule_update_strategy
 void handle_ignore_submodules_arg(struct diff_options *diffopt, const char *);
 void show_submodule_summary(FILE *f, const char *path,
const char *line_prefix,
-   unsigned char one[20], unsigned char two[20],
+   struct object_id *one, struct object_id *two,
unsigned dirty_submodule, const char *meta,
const char *del, const char *add, const char *reset);
 void set_config_fetch_recurse_submodules(int value);
-- 
2.10.0.rc0.217.g609f9e8.dirty

--
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 v8 7/8] cache: add empty_tree_oid object

[Jacob Keller]

From: Jacob Keller 

Add an empty_tree_oid object which can be used in place of
EMPTY_TREE_SHA1_BIN_LITERAL for code which is being converted to struct
object_id.

Signed-off-by: Jacob Keller 
---
 cache.h | 2 ++
 sha1_file.c | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/cache.h b/cache.h
index f30a4417efdf..da9f0be67d7b 100644
--- a/cache.h
+++ b/cache.h
@@ -964,6 +964,8 @@ static inline void oidclr(struct object_id *oid)
 #define EMPTY_BLOB_SHA1_BIN \
((const unsigned char *) EMPTY_BLOB_SHA1_BIN_LITERAL)
 
+extern const struct object_id empty_tree_oid;
+
 static inline int is_empty_blob_sha1(const unsigned char *sha1)
 {
return !hashcmp(sha1, EMPTY_BLOB_SHA1_BIN);
diff --git a/sha1_file.c b/sha1_file.c
index 1e23fc186a02..10883d56a600 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -38,6 +38,9 @@ static inline uintmax_t sz_fmt(size_t s) { return s; }
 
 const unsigned char null_sha1[20];
 const struct object_id null_oid;
+const struct object_id empty_tree_oid = {
+   .hash = EMPTY_TREE_SHA1_BIN_LITERAL
+};
 
 /*
  * This is meant to hold a *small* number of objects that you would
-- 
2.10.0.rc0.217.g609f9e8.dirty

--
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 v8 8/8] diff: teach diff to display submodule difference with an inline diff

[Jacob Keller]

From: Jacob Keller 

Teach git-diff and friends a new format for displaying the difference
of a submodule. The new format is an inline diff of the contents of the
submodule between the commit range of the update. This allows the user
to see the actual code change caused by a submodule update.

Add tests for the new format and option.

Signed-off-by: Jacob Keller 
---
 Documentation/diff-config.txt|   9 +-
 Documentation/diff-options.txt   |  17 +-
 diff.c   |  31 +-
 diff.h   |   1 +
 submodule.c  |  62 +++
 submodule.h  |   6 +
 t/t4060-diff-submodule-option-diff-format.sh | 746 +++
 7 files changed, 852 insertions(+), 20 deletions(-)
 create mode 100755 t/t4060-diff-submodule-option-diff-format.sh

diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt
index d5a5b17d5088..0eded24034b5 100644
--- a/Documentation/diff-config.txt
+++ b/Documentation/diff-config.txt
@@ -122,10 +122,11 @@ diff.suppressBlankEmpty::
 
 diff.submodule::
Specify the format in which differences in submodules are
-   shown.  The "log" format lists the commits in the range like
-   linkgit:git-submodule[1] `summary` does.  The "short" format
-   format just shows the names of the commits at the beginning
-   and end of the range.  Defaults to short.
+   shown.  The "short" format just shows the names of the commits
+   at the beginning and end of the range. The "log" format lists
+   the commits in the range like linkgit:git-submodule[1] `summary`
+   does. The "diff" format shows an inline diff of the changed
+   contents of the submodule. Defaults to "short".
 
 diff.wordRegex::
A POSIX Extended Regular Expression used to determine what is a "word"
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index cc4342e2034d..7805a0ccadf2 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -210,13 +210,16 @@ any of those replacements occurred.
of the `--diff-filter` option on what the status letters mean.
 
 --submodule[=]::
-   Specify how differences in submodules are shown.  When `--submodule`
-   or `--submodule=log` is given, the 'log' format is used.  This format 
lists
-   the commits in the range like linkgit:git-submodule[1] `summary` does.
-   Omitting the `--submodule` option or specifying `--submodule=short`,
-   uses the 'short' format. This format just shows the names of the commits
-   at the beginning and end of the range.  Can be tweaked via the
-   `diff.submodule` configuration variable.
+   Specify how differences in submodules are shown.  When specifying
+   `--submodule=short` the 'short' format is used.  This format just
+   shows the names of the commits at the beginning and end of the range.
+   When `--submodule` or `--submodule=log` is specified, the 'log'
+   format is used.  This format lists the commits in the range like
+   linkgit:git-submodule[1] `summary` does.  When `--submodule=diff`
+   is specified, the 'diff' format is used.  This format shows an
+   inline diff of the changes in the submodule contents between the
+   commit range.  Defaults to `diff.submodule` or the 'short' format
+   if the config option is unset.
 
 --color[=]::
Show colored diff.
diff --git a/diff.c b/diff.c
index 16253b191f53..b38d95eb249c 100644
--- a/diff.c
+++ b/diff.c
@@ -135,6 +135,8 @@ static int parse_submodule_params(struct diff_options 
*options, const char *valu
options->submodule_format = DIFF_SUBMODULE_LOG;
else if (!strcmp(value, "short"))
options->submodule_format = DIFF_SUBMODULE_SHORT;
+   else if (!strcmp(value, "diff"))
+   options->submodule_format = DIFF_SUBMODULE_INLINE_DIFF;
else
return -1;
return 0;
@@ -2300,6 +2302,15 @@ static void builtin_diff(const char *name_a,
struct strbuf header = STRBUF_INIT;
const char *line_prefix = diff_line_prefix(o);
 
+   diff_set_mnemonic_prefix(o, "a/", "b/");
+   if (DIFF_OPT_TST(o, REVERSE_DIFF)) {
+   a_prefix = o->b_prefix;
+   b_prefix = o->a_prefix;
+   } else {
+   a_prefix = o->a_prefix;
+   b_prefix = o->b_prefix;
+   }
+
if (o->submodule_format == DIFF_SUBMODULE_LOG &&
(!one->mode || S_ISGITLINK(one->mode)) &&
(!two->mode || S_ISGITLINK(two->mode))) {
@@ -2311,6 +2322,17 @@ static void builtin_diff(const char *name_a,
two->dirty_submodule,
meta, del, add, reset);
return;
+   } else if (o->submodule_format == DIFF_SUBMODULE_INLINE_DIFF &&
+   

[PATCH v8 2/8] graph: add support for --line-prefix on all graph-aware output

[Jacob Keller]

From: Jacob Keller 

Add an extension to git-diff and git-log (and any other graph-aware
displayable output) such that "--line-prefix=" will print the
additional line-prefix on every line of output.

To make this work, we have to fix a few bugs in the graph API that force
graph_show_commit_msg to be used only when you have a valid graph.
Additionally, we extend the default_diff_output_prefix handler to work
even when no graph is enabled.

This is somewhat of a hack on top of the graph API, but I think it
should be acceptable here.

This will be used by a future extension of submodule display which
displays the submodule diff as the actual diff between the pre and post
commit in the submodule project.

Add some tests for both git-log and git-diff to ensure that the prefix
is honored correctly.

Signed-off-by: Jacob Keller 
---
 Documentation/diff-options.txt |   3 +
 builtin/rev-list.c |  70 ++---
 diff.c |   7 +
 diff.h |   2 +
 graph.c|  98 ---
 graph.h|  22 +-
 log-tree.c |   5 +-
 t/t4013-diff-various.sh|   6 +
 ...diff.diff_--line-prefix=abc_master_master^_side |  29 ++
 t/t4013/diff.diff_--line-prefix_--cached_--_file0  |  15 +
 t/t4202-log.sh | 323 +
 11 files changed, 502 insertions(+), 78 deletions(-)
 create mode 100644 t/t4013/diff.diff_--line-prefix=abc_master_master^_side
 create mode 100644 t/t4013/diff.diff_--line-prefix_--cached_--_file0

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 705a87394200..cc4342e2034d 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -569,5 +569,8 @@ endif::git-format-patch[]
 --no-prefix::
Do not show any source or destination prefix.
 
+--line-prefix=::
+   Prepend an additional prefix to every line of output.
+
 For more detailed explanation on these common options, see also
 linkgit:gitdiffcore[7].
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 0ba82b1635b6..21cde8dd6b31 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -122,48 +122,40 @@ static void show_commit(struct commit *commit, void *data)
ctx.fmt = revs->commit_format;
ctx.output_encoding = get_log_output_encoding();
pretty_print_commit(, commit, );
-   if (revs->graph) {
-   if (buf.len) {
-   if (revs->commit_format != CMIT_FMT_ONELINE)
-   graph_show_oneline(revs->graph);
+   if (buf.len) {
+   if (revs->commit_format != CMIT_FMT_ONELINE)
+   graph_show_oneline(revs->graph);
 
-   graph_show_commit_msg(revs->graph, );
+   graph_show_commit_msg(revs->graph, stdout, );
 
-   /*
-* Add a newline after the commit message.
-*
-* Usually, this newline produces a blank
-* padding line between entries, in which case
-* we need to add graph padding on this line.
-*
-* However, the commit message may not end in a
-* newline.  In this case the newline simply
-* ends the last line of the commit message,
-* and we don't need any graph output.  (This
-* always happens with CMIT_FMT_ONELINE, and it
-* happens with CMIT_FMT_USERFORMAT when the
-* format doesn't explicitly end in a newline.)
-*/
-   if (buf.len && buf.buf[buf.len - 1] == '\n')
-   graph_show_padding(revs->graph);
-   putchar('\n');
-   } else {
-   /*
-* If the message buffer is empty, just show
-* the rest of the graph output for this
-* commit.
-*/
-   if (graph_show_remainder(revs->graph))
-   putchar('\n');
-   if (revs->commit_format == CMIT_FMT_ONELINE)
-   putchar('\n');
-   }
+   /*
+   

[PATCH v8 6/8] submodule: refactor show_submodule_summary with helper function

[Jacob Keller]

From: Jacob Keller 

A future patch is going to add a new submodule diff format which
displays an inline diff of the submodule changes. To make this easier,
and to ensure that both submodule diff formats use the same initial
header, factor out show_submodule_header() function which will print the
current submodule header line, and then leave the show_submodule_summary
function to lookup and print the submodule log format.

This does create one format change in that "(revision walker failed)"
will now be displayed on its own line rather than as part of the message
because we no longer perform this step directly in the header display
flow. However, this is a rare case as most causes of the failure will be
due to a missing commit which we already check for and avoid previously.
flow. However, this is a rare case and shouldn't impact much.

Signed-off-by: Jacob Keller 
---
 submodule.c | 108 ++--
 1 file changed, 76 insertions(+), 32 deletions(-)

diff --git a/submodule.c b/submodule.c
index 422353ccf6cc..7108b4786bc1 100644
--- a/submodule.c
+++ b/submodule.c
@@ -277,8 +277,7 @@ void handle_ignore_submodules_arg(struct diff_options 
*diffopt,
 }
 
 static int prepare_submodule_summary(struct rev_info *rev, const char *path,
-   struct commit *left, struct commit *right,
-   int *fast_forward, int *fast_backward)
+   struct commit *left, struct commit *right)
 {
struct commit_list *merge_bases, *list;
 
@@ -290,12 +289,6 @@ static int prepare_submodule_summary(struct rev_info *rev, 
const char *path,
add_pending_object(rev, >object, path);
add_pending_object(rev, >object, path);
merge_bases = get_merge_bases(left, right);
-   if (merge_bases) {
-   if (merge_bases->item == left)
-   *fast_forward = 1;
-   else if (merge_bases->item == right)
-   *fast_backward = 1;
-   }
for (list = merge_bases; list; list = list->next) {
list->item->object.flags |= UNINTERESTING;
add_pending_object(rev, >item->object,
@@ -333,31 +326,23 @@ static void print_submodule_summary(struct rev_info *rev, 
FILE *f,
strbuf_release();
 }
 
-void show_submodule_summary(FILE *f, const char *path,
+/* Helper function to display the submodule header line prior to the full
+ * summary output. If it can locate the submodule objects directory it will
+ * attempt to lookup both the left and right commits and put them into the
+ * left and right pointers.
+ */
+static void show_submodule_header(FILE *f, const char *path,
const char *line_prefix,
struct object_id *one, struct object_id *two,
unsigned dirty_submodule, const char *meta,
-   const char *del, const char *add, const char *reset)
+   const char *reset,
+   struct commit **left, struct commit **right)
 {
-   struct rev_info rev;
-   struct commit *left = NULL, *right = NULL;
+   struct commit_list *merge_bases;
const char *message = NULL;
struct strbuf sb = STRBUF_INIT;
int fast_forward = 0, fast_backward = 0;
 
-   if (is_null_oid(two))
-   message = "(submodule deleted)";
-   else if (add_submodule_odb(path))
-   message = "(not initialized)";
-   else if (is_null_oid(one))
-   message = "(new submodule)";
-   else if (!(left = lookup_commit_reference(one->hash)) ||
-!(right = lookup_commit_reference(two->hash)))
-   message = "(commits not present)";
-   else if (prepare_submodule_summary(, path, left, right,
-  _forward, _backward))
-   message = "(revision walker failed)";
-
if (dirty_submodule & DIRTY_SUBMODULE_UNTRACKED)
fprintf(f, "%sSubmodule %s contains untracked content\n",
line_prefix, path);
@@ -365,11 +350,46 @@ void show_submodule_summary(FILE *f, const char *path,
fprintf(f, "%sSubmodule %s contains modified content\n",
line_prefix, path);
 
+   if (is_null_oid(one))
+   message = "(new submodule)";
+   else if (is_null_oid(two))
+   message = "(submodule deleted)";
+
+   if (add_submodule_odb(path)) {
+   if (!message)
+   message = "(not initialized)";
+   goto output_header;
+   }
+
+   /*
+* Attempt to lookup the commit references, and determine if this is
+* a fast forward or fast backwards update.
+*/
+   *left = lookup_commit_reference(one->hash);
+   *right = lookup_commit_reference(two->hash);
+
+   /*
+* Warn about missing commits in the submodule project, but only if
+* they aren't null.
+

[PATCH v8 1/8] diff.c: remove output_prefix_length field

[Jacob Keller]

From: Junio C Hamano 

"diff/log --stat" has a logic that determines the display columns
available for the diffstat part of the output and apportions it for
pathnames and diffstat graph automatically.

5e71a84a (Add output_prefix_length to diff_options, 2012-04-16)
added the output_prefix_length field to diff_options structure to
allow this logic to subtract the display columns used for the
history graph part from the total "terminal width"; this matters
when the "git log --graph -p" option is in use.

The field must be set to the number of display columns needed to
show the output from the output_prefix() callback, which is error
prone.  As there is only one user of the field, and the user has the
actual value of the prefix string, let's get rid of the field and
have the user count the display width itself.

Signed-off-by: Junio C Hamano 
---
 diff.c  | 2 +-
 diff.h  | 1 -
 graph.c | 2 --
 3 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/diff.c b/diff.c
index 534c12e28ea8..50bef1f07658 100644
--- a/diff.c
+++ b/diff.c
@@ -1625,7 +1625,7 @@ static void show_stats(struct diffstat_t *data, struct 
diff_options *options)
 */
 
if (options->stat_width == -1)
-   width = term_columns() - options->output_prefix_length;
+   width = term_columns() - strlen(line_prefix);
else
width = options->stat_width ? options->stat_width : 80;
number_width = decimal_width(max_change) > number_width ?
diff --git a/diff.h b/diff.h
index 7883729edf10..747a204d75a4 100644
--- a/diff.h
+++ b/diff.h
@@ -174,7 +174,6 @@ struct diff_options {
diff_format_fn_t format_callback;
void *format_callback_data;
diff_prefix_fn_t output_prefix;
-   int output_prefix_length;
void *output_prefix_data;
 
int diff_path_counter;
diff --git a/graph.c b/graph.c
index dd1720148dc5..a46803840511 100644
--- a/graph.c
+++ b/graph.c
@@ -197,7 +197,6 @@ static struct strbuf *diff_output_prefix_callback(struct 
diff_options *opt, void
assert(opt);
assert(graph);
 
-   opt->output_prefix_length = graph->width;
strbuf_reset();
graph_padding_line(graph, );
return 
@@ -245,7 +244,6 @@ struct git_graph *graph_init(struct rev_info *opt)
 */
opt->diffopt.output_prefix = diff_output_prefix_callback;
opt->diffopt.output_prefix_data = graph;
-   opt->diffopt.output_prefix_length = 0;
 
return graph;
 }
-- 
2.10.0.rc0.217.g609f9e8.dirty

--
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 v8 3/8] diff: prepare for additional submodule formats

[Jacob Keller]

From: Jacob Keller 

A future patch will add a new format for displaying the difference of
a submodule. Make it easier by changing how we store the current
selected format. Replace the DIFF_OPT flag with an enumeration, as each
format will be mutually exclusive.

Signed-off-by: Jacob Keller 
---
 diff.c | 12 ++--
 diff.h |  7 ++-
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/diff.c b/diff.c
index e57cf39ad109..d6b321da3d1d 100644
--- a/diff.c
+++ b/diff.c
@@ -132,9 +132,9 @@ static int parse_dirstat_params(struct diff_options 
*options, const char *params
 static int parse_submodule_params(struct diff_options *options, const char 
*value)
 {
if (!strcmp(value, "log"))
-   DIFF_OPT_SET(options, SUBMODULE_LOG);
+   options->submodule_format = DIFF_SUBMODULE_LOG;
else if (!strcmp(value, "short"))
-   DIFF_OPT_CLR(options, SUBMODULE_LOG);
+   options->submodule_format = DIFF_SUBMODULE_SHORT;
else
return -1;
return 0;
@@ -2300,9 +2300,9 @@ static void builtin_diff(const char *name_a,
struct strbuf header = STRBUF_INIT;
const char *line_prefix = diff_line_prefix(o);
 
-   if (DIFF_OPT_TST(o, SUBMODULE_LOG) &&
-   (!one->mode || S_ISGITLINK(one->mode)) &&
-   (!two->mode || S_ISGITLINK(two->mode))) {
+   if (o->submodule_format == DIFF_SUBMODULE_LOG &&
+   (!one->mode || S_ISGITLINK(one->mode)) &&
+   (!two->mode || S_ISGITLINK(two->mode))) {
const char *del = diff_get_color_opt(o, DIFF_FILE_OLD);
const char *add = diff_get_color_opt(o, DIFF_FILE_NEW);
show_submodule_summary(o->file, one->path ? one->path : 
two->path,
@@ -3916,7 +3916,7 @@ int diff_opt_parse(struct diff_options *options,
DIFF_OPT_SET(options, OVERRIDE_SUBMODULE_CONFIG);
handle_ignore_submodules_arg(options, arg);
} else if (!strcmp(arg, "--submodule"))
-   DIFF_OPT_SET(options, SUBMODULE_LOG);
+   options->submodule_format = DIFF_SUBMODULE_LOG;
else if (skip_prefix(arg, "--submodule=", ))
return parse_submodule_opt(options, arg);
else if (skip_prefix(arg, "--ws-error-highlight=", ))
diff --git a/diff.h b/diff.h
index 1f57aad25c71..ea5aba668eaa 100644
--- a/diff.h
+++ b/diff.h
@@ -83,7 +83,6 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct 
diff_options *opt, void *data)
 #define DIFF_OPT_DIRSTAT_BY_FILE (1 << 20)
 #define DIFF_OPT_ALLOW_TEXTCONV  (1 << 21)
 #define DIFF_OPT_DIFF_FROM_CONTENTS  (1 << 22)
-#define DIFF_OPT_SUBMODULE_LOG   (1 << 23)
 #define DIFF_OPT_DIRTY_SUBMODULES(1 << 24)
 #define DIFF_OPT_IGNORE_UNTRACKED_IN_SUBMODULES (1 << 25)
 #define DIFF_OPT_IGNORE_DIRTY_SUBMODULES (1 << 26)
@@ -110,6 +109,11 @@ enum diff_words_type {
DIFF_WORDS_COLOR
 };
 
+enum diff_submodule_format {
+   DIFF_SUBMODULE_SHORT = 0,
+   DIFF_SUBMODULE_LOG,
+};
+
 struct diff_options {
const char *orderfile;
const char *pickaxe;
@@ -157,6 +161,7 @@ struct diff_options {
int stat_count;
const char *word_regex;
enum diff_words_type word_diff;
+   enum diff_submodule_format submodule_format;
 
/* this is set by diffcore for DIFF_FORMAT_PATCH */
int found_changes;
-- 
2.10.0.rc0.217.g609f9e8.dirty

--
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 2/4] cat-file: introduce the --filters option

[Torsten Bögershausen]

On Thu, Aug 18, 2016 at 02:46:17PM +0200, Johannes Schindelin wrote:
> As suggested by its name, the --filters option applies the filters

[]
> diff --git a/t/t8010-cat-file-filters.sh b/t/t8010-cat-file-filters.sh
Does it make sense to integrate tests into t1006-cat-file ?
> new file mode 100755
> index 000..e466634
> --- /dev/null
> +++ b/t/t8010-cat-file-filters.sh
> @@ -0,0 +1,34 @@
> +#!/bin/sh
> +
> +test_description='git cat-file filters support'
> +. ./test-lib.sh
> +
> +test_expect_success 'setup ' '
> + echo "*.txt eol=crlf diff=txt" >.gitattributes &&
> + echo "hello" | append_cr >world.txt &&
> + git add .gitattributes world.txt &&
> + test_tick &&
> + git commit -m "Initial commit"
> +'
> +
> +has_cr () {
> + tr '\015' Q <"$1" | grep Q >/dev/null
> +}
> +
> +test_expect_success 'no filters with `git show`' '
> + git show HEAD:world.txt >actual &&
I would prefer to have something using
  cat >expect <<-\EOF &&
  xxx
  test_cmp expect actual
to make it easier to debug in case of a failure ?
--
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 format-patch --break-rewrites broken in 2.9.3

[Olaf Hering]

On Thu, Aug 18, Jeff King wrote:

> Olaf, what version of patch are you using?

Mostly 2.7.x, but also add 2.5.x to the mix.
So far I did not try what the tools dealing with the resulting patch
file would actually do with such a stripped down variant.

Olaf


signature.asc
Description: PGP signature

[PATCH v8 0/8] submodule show inline diff

[Jacob Keller]

From: Jacob Keller 

v8 has changes suggested by Junio and Stefan Beller, as well as a few
modifications of my own, plus more tests.

Series interdiff of v7 and v8:
diff --git c/builtin/rev-list.c w/builtin/rev-list.c
index 1a75a83538f4..21cde8dd6b31 100644
--- c/builtin/rev-list.c
+++ w/builtin/rev-list.c
@@ -129,20 +129,20 @@ static void show_commit(struct commit *commit, void *data)
graph_show_commit_msg(revs->graph, stdout, );
 
/*
-   * Add a newline after the commit message.
-   *
-   * Usually, this newline produces a blank
-   * padding line between entries, in which case
-   * we need to add graph padding on this line.
-   *
-   * However, the commit message may not end in a
-   * newline.  In this case the newline simply
-   * ends the last line of the commit message,
-   * and we don't need any graph output.  (This
-   * always happens with CMIT_FMT_ONELINE, and it
-   * happens with CMIT_FMT_USERFORMAT when the
-   * format doesn't explicitly end in a newline.)
-   */
+   * Add a newline after the commit message.
+   *
+   * Usually, this newline produces a blank
+   * padding line between entries, in which case
+   * we need to add graph padding on this line.
+   *
+   * However, the commit message may not end in a
+   * newline.  In this case the newline simply
+   * ends the last line of the commit message,
+   * and we don't need any graph output.  (This
+   * always happens with CMIT_FMT_ONELINE, and it
+   * happens with CMIT_FMT_USERFORMAT when the
+   * format doesn't explicitly end in a newline.)
+   */
if (buf.len && buf.buf[buf.len - 1] == '\n')
graph_show_padding(revs->graph);
putchar('\n');
diff --git c/cache.h w/cache.h
index f30a4417efdf..da9f0be67d7b 100644
--- c/cache.h
+++ w/cache.h
@@ -964,6 +964,8 @@ static inline void oidclr(struct object_id *oid)
 #define EMPTY_BLOB_SHA1_BIN \
((const unsigned char *) EMPTY_BLOB_SHA1_BIN_LITERAL)
 
+extern const struct object_id empty_tree_oid;
+
 static inline int is_empty_blob_sha1(const unsigned char *sha1)
 {
return !hashcmp(sha1, EMPTY_BLOB_SHA1_BIN);
diff --git c/diff.c w/diff.c
index e119758aba82..b38d95eb249c 100644
--- c/diff.c
+++ w/diff.c
@@ -2318,7 +2318,7 @@ static void builtin_diff(const char *name_a,
const char *add = diff_get_color_opt(o, DIFF_FILE_NEW);
show_submodule_summary(o->file, one->path ? one->path : 
two->path,
line_prefix,
-   one->oid.hash, two->oid.hash,
+   >oid, >oid,
two->dirty_submodule,
meta, del, add, reset);
return;
@@ -2329,7 +2329,7 @@ static void builtin_diff(const char *name_a,
const char *add = diff_get_color_opt(o, DIFF_FILE_NEW);
show_submodule_inline_diff(o->file, one->path ? one->path : 
two->path,
line_prefix,
-   one->oid.hash, two->oid.hash,
+   >oid, >oid,
two->dirty_submodule,
meta, del, add, reset, o);
return;
diff --git c/graph.c w/graph.c
index b140c096b7f3..06f1139f2e20 100644
--- c/graph.c
+++ w/graph.c
@@ -1215,10 +1215,10 @@ void graph_show_commit(struct git_graph *graph)
struct strbuf msgbuf = STRBUF_INIT;
int shown_commit_line = 0;
 
-   if (!graph) {
-   graph_show_line_prefix(default_diffopt);
+   graph_show_line_prefix(default_diffopt);
+
+   if (!graph)
return;
-   }
 
/*
 * When showing a diff of a merge against each of its parents, we
@@ -1232,11 +1232,12 @@ void graph_show_commit(struct git_graph *graph)
 
while (!shown_commit_line && !graph_is_commit_finished(graph)) {
shown_commit_line = graph_next_line(graph, );
-   graph_show_line_prefix(>revs->diffopt);
fwrite(msgbuf.buf, sizeof(char), msgbuf.len,
graph->revs->diffopt.file);
-   if (!shown_commit_line)
+   if (!shown_commit_line) 

Re: git format-patch --break-rewrites broken in 2.9.3

[Matthieu Moy]

Olaf Hering  writes:

> On Thu, Aug 18, Jeff King wrote:
>
>> Olaf, what version of patch are you using?
>
> Mostly 2.7.x, but also add 2.5.x to the mix.
> So far I did not try what the tools dealing with the resulting patch
> file would actually do with such a stripped down variant.

I think the way to go is --no-renames until you stop using patch <2.7.
If you don't want to specify it each time, you can revert to the pre-2.9
behavior by setting

[diff]
renames = false

in ~/.gitconfig.

-- 
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 v7 4/7] submodule: allow do_submodule_path to work if given gitdir directly

[Jacob Keller]

On Thu, Aug 18, 2016 at 11:46 AM, Junio C Hamano  wrote:
> Jacob Keller  writes:
>
>> From: Jacob Keller 
>>
>> Currently, do_submodule_path relies on read_gitfile, which will die() if
>> it can't read from the specified gitfile. Unfortunately, this means that
>> do_submodule_path will not work when given the path to a submodule which
>> is checked out directly, such as a newly added submodule which you
>> cloned and then "git submodule add".
>
> Hmm, are you sure about that?
>
> A call to read_gitfile() turns into a call to read_gitfile_gently()
> with the return_error_code parameter set to NULL.  The function does
> a stat(2), and if the given path is not a file (e.g. we created the
> submodule working tree and repository in-place ourselves, instead of
> cloning an existing project from elsewhere, in which case we would
> see a directory there), it says READ_GIT_FILE_ERR_NOT_A_FILE and
> returns NULL, because that is not a fatal error condition.  The same
> thing happens if path does not yet exist.
>
> This caller is given , prepares "/.git" in buf, and
> calls read_gitfile().  If it returns a non-NULL, it replaces what is
> in buf and continues, but if it returns a NULL (i.e. the two cases I
> mentioned in the above paragraph), it continues with "/.git".
>
> While I do not think changing it to resolve_gitdir() is wrong per-se,
> I am not sure if it is necessary.
>
> I must be misreading something in the existing code.
>

No I think you're correct. I was under the assumption that it would
die here. I think it's probably better to stick with read_gitfile()
here, it should work. The main issue is what happens after this needs
to be fixed.

I'll investigate this more.

Thanks,
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 2/2] mingw: ensure temporary file handles are not inherited by child processes

[Eric Wong]

Junio C Hamano  wrote:
> Our codepaths themselves generally do not care about O_CLOEXEC, so
> giving a racy emulation of it is not worth the effort, making the
> later half of the above an overkill.

OK.

> Perhaps the three lines to
> define O_CLOEXEC to 0 on older UNIX might be sufficient.

I'd be more comfortable keeping the EINVAL check that got
snipped in your reply.  O_CLOEXEC can be defined to non-zero in
new userspace headers, but an older kernel chokes on it with
EINVAL.

I've seen this failure in the past with chroots.
--
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: Working from different machines.

[Jakub Narębski]

W dniu 18.08.2016 o 16:28, Tobiah pisze:

> ## [...]  Why does it still mention 'flipper' and 'tart' and all
> ## other branches I've played with.  When I delte them locally, I want
> ## them to be deleted everywhere.  They just keep accumulating.

In case of non-mirror setup. Git wouldn't delete remote-tracking branches
(e.g. origin/tart) if branch in remote repository is deleted (e.g. tart
in origin repository); you need to prune (or set up to auto prune), e.g.

 $ git remote origin prune

-- 
Jakub Narębski

--
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: Working with public-inbox.org [Was: [PATCH] rev-parse: respect core.hooksPath in --git-path]

[Eric Wong]

Johannes Schindelin  wrote:
> BTW I take this thread as yet another proof that people are unhappy with
> mail list-based review: if you have to build *that much* tooling around it
> (and Peff & Junio certainly have a megaton of advanced and sophisticated
> tooling around it, holy cow!) it is really incorrect to state that the
> mail list-driven approach works for you. It is much closer to the truth to
> say that the mail-list-plus-loads-of-custom-tools-driven approach works
> for you.
> 
> I am really not a fan of this.
> 
> The theory "it's inclusive because everyone has access to mail" falls on
> its face, badly, when even old timers have to build entire infrastructures
> around it just to begin to be able to use it efficiently.
> 
> It reminds me of an old software developer I met long ago, who claimed CVS
> works for him. He had written tens of thousands of lines of shell scripts,
> is what allowed "CVS" to work for him.
> 
> Same here. Old dogs claim the mail list-approach works for them. Nope.
> Doesn't. Else you would not have written all those custom scripts.

git and cogito started as a bunch of custom scripts, too.
IMHO, it's what makes Free Software (and *nix) great:
users have control to customize and improve things.
With scripts, they don't even need to learn a build process
to do so.

I see a choice of mail client as no different than a choice of
text editor.  Neither my mail client or text editor is heavily
customized.  The key feature I rely on from both tools is piping
data to external commands.


OTOH, today, I see people using git aliases all the time which
look more like ASM instructions than user commands.

Is the widespread use of these aliases a deficiency to git?
Maybe, I don't know.

Normally, I do not care about aliases: it's a private thing;
but it also makes it incredibly difficult for me to help
users when they're exposed in public.


Users ought to be able to pick, choose, and replace tools as
they wish as long as an interchange format remains stable
and widely-supported.

Fwiw, I still use patch(1) pretty often, even on patches
generated with git.  I see nothing wrong with that; patch is
lenient in ways git-apply was explicitly designed not to be.
And I don't always use git send-email or my normal MUA for
sending; or use git for generating diffs.  I do:

diff -u a b | mail -s WIP-blah-blah $SOMEONE


While you and I are long-time git hackers, I don't think it's
reasonable for everyone to use git or git-specific tools;
even to git.git for one-offs like portability or doc fixes.

Even today, at least one Linux kernel hacker still uses quilt to
generate patches: http://ozlabs.org/~akpm/mmotm/
--
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 2/2] mingw: ensure temporary file handles are not inherited by child processes

[Eric Wong]

Johannes Schindelin  wrote:
> +++ b/compat/mingw.h
> @@ -67,6 +67,10 @@ typedef int pid_t;
>  #define F_SETFD 2
>  #define FD_CLOEXEC 0x1
>  
> +#if !defined O_CLOEXEC && defined O_NOINHERIT
> +#define O_CLOEXECO_NOINHERIT
> +#endif


> +++ b/tempfile.c
> @@ -120,7 +120,7 @@ int create_tempfile(struct tempfile *tempfile, const char 
> *path)
>   prepare_tempfile_object(tempfile);
>  
>   strbuf_add_absolute_path(>filename, path);
> - tempfile->fd = open(tempfile->filename.buf, O_RDWR | O_CREAT | O_EXCL, 
> 0666);
> + tempfile->fd = open(tempfile->filename.buf, O_RDWR | O_CREAT | O_EXCL | 
> O_CLOEXEC, 0666);
>   if (tempfile->fd < 0) {
>   strbuf_reset(>filename);
>   return -1;

O_CLOEXEC only exists since Linux 2.6.23 and there are likely
still LTS (CentOS 5.x?) and non-Linux systems which do not have
it, as well as machines with could have it defined in userspace
headers but not have it in the kernel.

So I suggest something like the following: (untested)

#define GIT_O_TMP (O_RDWR | O_CREAT | O_EXCL)

#ifndef O_CLOEXEC
#  define O_CLOEXEC 0
#endif
/* state: -1=unknown; 0=broken; 1=working */
static int cloexec_state = O_CLOEXEC == 0 ? 0 : -1;
static int GIT_O_ETMP = (GIT_O_TMP | O_CLOEXEC)

int fd = open(filename, GIT_O_ETMP, 0666);

if (fd < 0 && errno == EINVAL && cloexec_state == -1 &&
GIT_O_ETMP != GIT_O_TMP) {
GIT_O_ETMP = GIT_O_TMP;

fd = open(filename, GIT_O_ETMP, 0666);
if (fd >= 0)
/* don't try O_CLOEXEC again */
cloexec_state = 0;
}

/*
 * This is racy in the presence of threads,
 * but the best we can do for old *nix:
 */
#if defined(F_GETFD) && defined(F_SETFD) && defined(FD_CLOEXEC)
if (fd >= 0 && cloexec_state != 1) {
int flags = fcntl(fd, F_GETFD);

if (flags == -1)
die_errno("F_GETFD failed");
if (flags & O_CLOEXEC)
cloexec_state = 1;
else {
flags = fcntl(fd, F_SETFD, flags | FD_CLOEXEC);
if (flags == -1)
die_errno("F_SETFD failed");
cloexec_state = 0;

}
}
#endif
...
--
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 v7 2/7] graph: add support for --line-prefix on all graph-aware output

[Stefan Beller]

On Wed, Aug 17, 2016 at 5:51 PM, Jacob Keller  wrote:
> From: Jacob Keller 
>
> Add an extension to git-diff and git-log (and any other graph-aware
> displayable output) such that "--line-prefix=" will print the
> additional line-prefix on every line of output.
>
> To make this work, we have to fix a few bugs in the graph API that force
> graph_show_commit_msg to be used only when you have a valid graph.
> Additionally, we extend the default_diff_output_prefix handler to work
> even when no graph is enabled.
>
> This is somewhat of a hack on top of the graph API, but I think it
> should be acceptable here.
>
> This will be used by a future extension of submodule display which
> displays the submodule diff as the actual diff between the pre and post
> commit in the submodule project.
>
> Add some tests for both git-log and git-diff to ensure that the prefix
> is honored correctly.
>
> Signed-off-by: Jacob Keller 
> ---
>  Documentation/diff-options.txt |   3 +
>  builtin/rev-list.c |  70 ++---
>  diff.c |   7 +
>  diff.h |   2 +
>  graph.c| 104 ---
>  graph.h|  22 +-
>  log-tree.c |   5 +-
>  t/t4013-diff-various.sh|   6 +
>  ...diff.diff_--line-prefix=abc_master_master^_side |  29 ++
>  t/t4013/diff.diff_--line-prefix_--cached_--_file0  |  15 +
>  t/t4202-log.sh | 323 
> +
>  11 files changed, 506 insertions(+), 80 deletions(-)
>  create mode 100644 t/t4013/diff.diff_--line-prefix=abc_master_master^_side
>  create mode 100644 t/t4013/diff.diff_--line-prefix_--cached_--_file0
>
> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
> index 705a87394200..cc4342e2034d 100644
> --- a/Documentation/diff-options.txt
> +++ b/Documentation/diff-options.txt
> @@ -569,5 +569,8 @@ endif::git-format-patch[]
>  --no-prefix::
> Do not show any source or destination prefix.
>
> +--line-prefix=::
> +   Prepend an additional prefix to every line of output.
> +
>  For more detailed explanation on these common options, see also
>  linkgit:gitdiffcore[7].
> diff --git a/builtin/rev-list.c b/builtin/rev-list.c
> index 0ba82b1635b6..1a75a83538f4 100644
> --- a/builtin/rev-list.c
> +++ b/builtin/rev-list.c
> @@ -122,48 +122,40 @@ static void show_commit(struct commit *commit, void 
> *data)
> ctx.fmt = revs->commit_format;
> ctx.output_encoding = get_log_output_encoding();
> pretty_print_commit(, commit, );
> -   if (revs->graph) {
> -   if (buf.len) {
> -   if (revs->commit_format != CMIT_FMT_ONELINE)
> -   graph_show_oneline(revs->graph);
> +   if (buf.len) {
> +   if (revs->commit_format != CMIT_FMT_ONELINE)
> +   graph_show_oneline(revs->graph);
>
> -   graph_show_commit_msg(revs->graph, );
> +   graph_show_commit_msg(revs->graph, stdout, );
>
> -   /*
> -* Add a newline after the commit message.
> -*
> -* Usually, this newline produces a blank
> -* padding line between entries, in which case
> -* we need to add graph padding on this line.
> -*
> -* However, the commit message may not end in 
> a
> -* newline.  In this case the newline simply
> -* ends the last line of the commit message,
> -* and we don't need any graph output.  (This
> -* always happens with CMIT_FMT_ONELINE, and 
> it
> -* happens with CMIT_FMT_USERFORMAT when the
> -* format doesn't explicitly end in a 
> newline.)
> -*/
> -   if (buf.len && buf.buf[buf.len - 1] == '\n')
> -   graph_show_padding(revs->graph);
> -   putchar('\n');
> -   } else {
> -   /*
> -* If the message buffer is empty, just show
> -* the rest of the graph output for this
> -* commit.
> -*/
> -   if 

[PATCH 0/2] help: make option --help open man pages only for Git commands

[Ralf Thielow]

In this version, one patch has been turned into two.  The first introduces the
option "command-only" to make 'help' only working for commands and additionally
give some nice help on typos.  The second makes option --help only work for 
actual
Git commands.

Ralf Thielow (2):
  help: introduce option --command-only
  help: make option --help open man pages only for Git commands

 Documentation/git-help.txt | 11 ---
 builtin/help.c | 30 +++---
 contrib/completion/git-completion.bash |  2 +-
 git.c  | 15 ++-
 t/t0012-help.sh| 29 +
 5 files changed, 75 insertions(+), 12 deletions(-)
 create mode 100755 t/t0012-help.sh

-- 
2.9.2.912.gd0c0e83

--
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 v7 2/7] graph: add support for --line-prefix on all graph-aware output

[Jacob Keller]

On Thu, Aug 18, 2016 at 10:56 AM, Stefan Beller  wrote:
> On Wed, Aug 17, 2016 at 5:51 PM, Jacob Keller  
> wrote:
>> - if (!graph)
>> + if (!graph) {
>> + graph_show_line_prefix(default_diffopt);
>>   return;
>> + }
>
>> -   if (graph_is_commit_finished(graph))
>> +   if (graph_is_commit_finished(graph)) {
>> +   graph_show_line_prefix(>revs->diffopt);
>> return 0;
>> +   }
>
> This seems to be a reoccurring pattern, i.e. we need to add a lot of
> one off instructions before a return. Is it possible to make the call
> earlier instead
> of just before the returns? (or later for that matter) ?
>

Not sure what you mean by this...? Just move the call up above this? I
think that will work, let me give it a try.

Thanks,
Jake

>
> Thanks,
> Stefan
--
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 v7 7/7] diff: teach diff to display submodule difference with an inline diff

[Stefan Beller]

On Wed, Aug 17, 2016 at 5:51 PM, Jacob Keller  wrote:



> if (o->submodule_format == DIFF_SUBMODULE_LOG &&
> (!one->mode || S_ISGITLINK(one->mode)) &&
> (!two->mode || S_ISGITLINK(two->mode))) {
> @@ -2311,6 +2322,17 @@ static void builtin_diff(const char *name_a,
> two->dirty_submodule,
> meta, del, add, reset);
> return;
> +   } else if (o->submodule_format == DIFF_SUBMODULE_INLINE_DIFF &&
> +  (!one->mode || S_ISGITLINK(one->mode)) &&
> +  (!two->mode || S_ISGITLINK(two->mode))) {

The ! mode is for added and deleted submodules, I guess?

> diff --git a/diff.h b/diff.h
> index ea5aba668eaa..192c0eedd0ff 100644
> --- a/diff.h
> +++ b/diff.h
> @@ -112,6 +112,7 @@ enum diff_words_type {
>  enum diff_submodule_format {
> DIFF_SUBMODULE_SHORT = 0,
> DIFF_SUBMODULE_LOG,
> +   DIFF_SUBMODULE_INLINE_DIFF,
>  };
>
>  struct diff_options {
> diff --git a/submodule.c b/submodule.c
> index e341ca7ffefd..e5f1138f4362 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -436,6 +436,67 @@ void show_submodule_summary(FILE *f, const char *path,
> clear_commit_marks(right, ~0);
>  }
>
> +void show_submodule_inline_diff(FILE *f, const char *path,
> +   const char *line_prefix,
> +   unsigned char one[20], unsigned char two[20],
> +   unsigned dirty_submodule, const char *meta,
> +   const char *del, const char *add, const char *reset,
> +   const struct diff_options *o)
> +{
> +   const char *old = EMPTY_TREE_SHA1_BIN, *new = EMPTY_TREE_SHA1_BIN;

submodule.c: In function ‘show_submodule_inline_diff’:
cache.h:957:3: warning: pointer targets in initialization differ in
signedness [-Wpointer-sign]
   ((const unsigned char *) EMPTY_TREE_SHA1_BIN_LITERAL)

submodule.c:446:20: note: in expansion of macro ‘EMPTY_TREE_SHA1_BIN’
  const char *old = EMPTY_TREE_SHA1_BIN, *new = EMPTY_TREE_SHA1_BIN;



> +   struct commit *left = NULL, *right = NULL;
> +   struct strbuf submodule_dir = STRBUF_INIT;
> +   struct child_process cp = CHILD_PROCESS_INIT;
> +
> +   show_submodule_header(f, path, line_prefix, one, two, dirty_submodule,
> + meta, reset, , );
> +
> +   /* We need a valid left and right commit to display a difference */
> +   if (!(left || is_null_sha1(one)) ||
> +   !(right || is_null_sha1(two)))
> +   goto done;
> +
> +   if (left)
> +   old = one;

submodule.c:460:7: warning: pointer targets in assignment differ in
signedness [-Wpointer-sign]
   old = one;



> +   if (right)
> +   new = two;
> +
> +   fflush(f);
> +   cp.git_cmd = 1;
> +   cp.dir = path;
> +   cp.out = dup(fileno(f));
> +   cp.no_stdin = 1;
> +
> +   /* TODO: other options may need to be passed here. */
> +   argv_array_pushl(, "diff");
> +   argv_array_pushf(, "--line-prefix=%s", line_prefix);
> +   if (DIFF_OPT_TST(o, REVERSE_DIFF)) {
> +   argv_array_pushf(, "--src-prefix=%s%s/",
> +o->b_prefix, path);
> +   argv_array_pushf(, "--dst-prefix=%s%s/",
> +o->a_prefix, path);
> +   } else {
> +   argv_array_pushf(, "--src-prefix=%s%s/",
> +o->a_prefix, path);
> +   argv_array_pushf(, "--dst-prefix=%s%s/",
> +o->b_prefix, path);
> +   }
> +   argv_array_push(, sha1_to_hex(old));

submodule.c:484:2: warning: pointer targets in passing argument 1 of
‘sha1_to_hex’ differ in signedness [-Wpointer-sign]
  argv_array_push(, sha1_to_hex(old));


/*
 * nit: the following comment doesn't adhere to Gits way of doing comments:
 */

> +   /* If the submodule has modified content, we will diff against the
> +* work tree, under the assumption that the user has asked for the
> +* diff format and wishes to actually see all differences even if they
> +* haven't yet been committed to the submodule yet.
> +*/

Makes sort of sense when new is HEAD.

However if I have given an explicit sha1 in the superproject, e.g.:

# (in the e.g. gerrit with a dirty submodule, I'd still expect to
# get the dirtyness ignored, but the diff of those two states?)
git diff --submodule=diff  cc82b24..5222e66 plugins/

> +   if (!(dirty_submodule & DIRTY_SUBMODULE_MODIFIED))
> +   argv_array_push(, sha1_to_hex(new));
> +


> +# Tested non-UTF-8 encoding
> +test_encoding="ISO8859-1"
...
> +# String "added" in German (translated with Google Translate), encoded in 
> UTF-8,
> +# used in sample commit log messages in add_file() function below.
> +added=$(printf "hinzugef\303\274gt")


> +add_file () {
> +   (
> +   cd "$1" &&
> + 

Re: [PATCH 4/4] cat-file: support --textconv/--filters in batch mode

[Junio C Hamano]

Johannes Schindelin  writes:

> With this patch, --batch can be combined with --textconv or --filters.
> For this to work, the input needs to have the form
>
>   
>
> so that the filters can be chosen appropriately.
>  --batch::
>  --batch=::
>   Print object information and contents for each object provided
> - on stdin.  May not be combined with any other options or arguments.
> - See the section `BATCH OUTPUT` below for details.
> + on stdin.  May not be combined with any other options or arguments
> + except `--textconv` or `--filters`, in which case the input lines
> + also need to specify the path, separated by white space.  See the
> + section `BATCH OUTPUT` below for details.

Makes sense.  This format still allows

HEAD: 

i.e. the object name is taken from path2 but we forget it and
pretend that the blob sits at path2, which is a good feature.

If I am not mistaken, your cover letter alluded that it might be
ideal to take "HEAD:" (nothing else) as input, grab ""
and feed that to the filtering machinery, but you decided to stop
short of doing that.  I actually think that it is the right thing to
do for this feature to ignore the trailing : in the object
name, iow, I agree with the result from the feature design POV.

The only thing that somewhat is worrysome is what would happen to
%(rest).  I guess [*1*] it is OK to declare that you cannot use %(rest) in
your output format when --filter/--textconv is in use, but if that
is the direction we are going, that limitation needs to be
documented.


[Footnote]

*1* This is just a "guess", because I do not know what people are
using %(rest) for.  It is plausible that their use case do not need
--filter/--textconv at all, and if that is the case, having this
limitation is perfectly fine.
--
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 v7 6/7] submodule: refactor show_submodule_summary with helper function

[Jacob Keller]

On Thu, Aug 18, 2016 at 1:49 PM, Junio C Hamano  wrote:
> Jacob Keller  writes:
>
> If we were to change those, we could discuss if we want to go with
> full sentences
> all the time:
>
> submodule is new
> submodule is deleted
> submodule is not initialized

 I agree, I'll make a new patch that does this as a cleanup prior to
 this re-work.
>>> ...
>> Sorry for being unclear myself, too. I'm keeping it as "not
>> initialized" and updating the description of the patch that changed it
>> from "not checked out" to "not initialized"
>
> Whether it is done inside or outside the scope of this series, the
> other two "is new"/"is deleted" updates look very sensible ones to
> make in the longer run.  I'd further suggest to unify "commits not
> present" and "revision walker failed" into one error class.  From
> the end user's point of view, they aren't very different.

That won't work exactly due to the new way we separate the header
format from the revision walk. I can change the error case though. The
new show_submodule_header will not actually attempt a revision walk at
all and thus won't know to display "revision walker failed" at the
same place as the header fails. However I think almost all of the time
it should succeed if commits are present.

Thanks,
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 1/4] cat-file: fix a grammo in the man page

[Junio C Hamano]

Johannes Schindelin  writes:

> "... has be ..." -> "... has to be ..."
>
> Signed-off-by: Johannes Schindelin 
> ---
>  Documentation/git-cat-file.txt | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
> index 18d03d8..071029b 100644
> --- a/Documentation/git-cat-file.txt
> +++ b/Documentation/git-cat-file.txt
> @@ -54,8 +54,9 @@ OPTIONS
>  
>  --textconv::
>   Show the content as transformed by a textconv filter. In this case,
> -  has be of the form :, or : in order
> - to apply the filter to the content recorded in the index at .
> +  has to be of the form :, or : in
> + order to apply the filter to the content recorded in the index at
> + .

Thanks.  That's an ancient typo dating back to 2010 ;-)


--
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 format-patch --break-rewrites broken in 2.9.3

[Jeff King]

On Thu, Aug 18, 2016 at 11:05:22AM -0400, Jeff King wrote:

> On Thu, Aug 18, 2016 at 04:44:21PM +0200, Olaf Hering wrote:
> 
> > This command used to create a diff which can be consumed by patch. But
> > at least with 2.9.3 it just gives a rename output:
> > 
> >  git format-patch \
> > --no-signature \
> > --stdout \
> > --break-rewrites \
> > --keep-subject \
> >  
> > 95fa0405c5991726e06c08ffcd8ff872f7fb4f2d^..95fa0405c5991726e06c08ffcd8ff872f7fb4f2d
> > 
> > 
> > What must be done now to get a usable patch?
> 
> Probably --no-renames.
> 
> Renames were enabled by default by 5404c11 (diff: activate diff.renames
> by default, 2016-02-25), which is in v2.9.0.
> 
> I wonder if we should consider undoing that for format-patch, whose
> output may be consumed by non-git endpoints.

By the way, this probably has nothing to do with --break-rewrites in
particular. It would come up for any case where git finds a rename. In
the absence of --break-rewrites, that requires a path being deleted and
one being added. But in this particular case, --break-rewrites turns a
large change into a delete/add pair, which lets git find the rename.

So it's a necessary option to show the problem in _this_ instance, but
there are other cases that would not need it.

-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: git format-patch --break-rewrites broken in 2.9.3

[Jeff King]

On Thu, Aug 18, 2016 at 05:16:55PM +0200, Matthieu Moy wrote:

> Jeff King  writes:
> 
> > On Thu, Aug 18, 2016 at 04:44:21PM +0200, Olaf Hering wrote:
> >
> >> This command used to create a diff which can be consumed by patch. But
> >> at least with 2.9.3 it just gives a rename output:
> >> 
> >>  git format-patch \
> >> --no-signature \
> >> --stdout \
> >> --break-rewrites \
> >> --keep-subject \
> >>  
> >> 95fa0405c5991726e06c08ffcd8ff872f7fb4f2d^..95fa0405c5991726e06c08ffcd8ff872f7fb4f2d
> >> 
> >> 
> >> What must be done now to get a usable patch?
> >
> > Probably --no-renames.
> >
> > Renames were enabled by default by 5404c11 (diff: activate diff.renames
> > by default, 2016-02-25), which is in v2.9.0.
> >
> > I wonder if we should consider undoing that for format-patch, whose
> > output may be consumed by non-git endpoints.
> 
> I would say no (or more precisely: we should consider, but we should
> reject the idea ;-) ), since patches with renames are useful and can be used
> even outside Git's scope. GNU patch, which is probably the most widely
> used implementation of patch supports git-style renames since 2.7,
> released in September 2012.

Ah, OK; I didn't realize GNU patch had picked up rename support. I agree
that makes it less-bad for format-patch to start using them by default.
Olaf, what version of patch are you using?

-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 2/2 v7] pack-objects: use reachability bitmap index when generating non-stdout pack

[Jeff King]

On Tue, Aug 09, 2016 at 10:32:17PM +0300, Kirill Smelkov wrote:

> Subject: Re: [PATCH 2/2 v7] pack-objects: use reachability bitmap index when
>generating non-stdout pack

This is v7, but as I understand your numbering, it goes with v5 of patch
1/2 that I just reviewed (usually we just increment the version number
on the whole series and treat it as a unit, even if some patches didn't
change from version to version).

> So we can teach pack-objects to use bitmap index for initial object
> counting phase when generating resultant pack file too:
> 
> - if we care it is not activated under git-repack:

Do you mean "if we take care that it is not..." here?

(I think you might just be getting tripped up in the English idioms;
"care" means that we have a preference; "to take care" means that we are
being careful).

> - if we know bitmap index generation is not enabled for resultant pack:
> 
>   Current code has singleton bitmap_git so cannot work simultaneously
>   with two bitmap indices.

Minor English fixes:

  The current code has a singleton bitmap_git, so it cannot work
  simultaneously with two bitmap indices.

> - if we keep pack reuse enabled still only for "send-to-stdout" case:
> 
>   Because on pack reuse raw entries are directly written out to destination
>   pack by write_reused_pack() bypassing needed for pack index generation
>   bookkeeping done by regular codepath in write_one() and friends.

Ditto on English:

  On pack reuse raw entries are directly written out to the destination
  pack by write_reused_pack(), bypassing the need for pack index
  generation bookkeeping done by the regular code path in write_one()
  and friends.

I think this is missing the implication. Why wouldn't we want to reuse
in this case? Certainly we don't when doing a "careful" on-disk repack.
I suspect the answer is that we cannot write a ".idx" off of the result
of write_reused_pack(), and write-to-disk always includes the .idx.

> More context:
> 
> http://marc.info/?t=14679210141=1=2

Can we turn this into a link to public-inbox? We have just been bit by
all of our old links to gmane dying, and they cannot easily be replaced
because they use a gmane-specific article number. public-inbox URLs use
message-ids, which should be usable for other archives if public-inbox
goes away.

> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> index b1007f2..c92d7fc 100644
> --- a/builtin/pack-objects.c
> +++ b/builtin/pack-objects.c

The code here looks fine.

> diff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh
> index a278d30..9602e9a 100755
> --- a/t/t5310-pack-bitmaps.sh
> +++ b/t/t5310-pack-bitmaps.sh
> @@ -196,6 +196,18 @@ test_expect_success 'pack-objects respects --local 
> (non-local bitmapped pack)' '
>   ! has_any packbitmap.objects 3b.objects
>  '
>  
> +test_expect_success 'pack-objects to file can use bitmap' '
> + # make sure we still have 1 bitmap index from previous tests
> + ls .git/objects/pack/ | grep bitmap >output &&
> + test_line_count = 1 output &&
> + # verify equivalent packs are generated with/without using bitmap index
> + packasha1=$(git pack-objects --no-use-bitmap-index --all packa 
>  + packbsha1=$(git pack-objects --use-bitmap-index --all packb  &&
> + list_packed_objects packa.objects &&
> + list_packed_objects packb.objects &&
> + test_cmp packa.objects packb.objects
> +'

Of course we can't know if bitmaps were actually used, or if they were
turned off under the hood. But at least this exercises the code a bit.

You could possibly add a perf test which shows off the improvement, but
I don't think it's strictly necessary.

-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: Working with zip files

[David Lang]

On Thu, 18 Aug 2016, Jakub Narębski wrote:


JN>> You can find rezip clean/smudge filter (originally intended for
JN>> OpenDocument Format (ODF), that is OpenOffice.org etc.) that stores
JN>> zip or zip-archive (like ODT, jar, etc.) uncompressed.  I think
JN>> you can find it on GitWiki, but I might be mistaken.

Using 'unzip -c' as separate / additional `textconv` filter for diff
generation allows to separate the problem of deltifiable storage format
from textual representation for diff-ing.

Though best results could be had with `diff` and `merge` drivers...


can you point at an example of how to do this? when I went looking about a year 
ago to deal with single-line json data I wasn't able to find anything good. I 
ended up using clean/smudge to pretty-print the json so it was easier to handle.


David Lang

Re: [PATCH 4/4] cat-file: support --textconv/--filters in batch mode

[Junio C Hamano]

Jeff King  writes:

> On Thu, Aug 18, 2016 at 02:46:28PM +0200, Johannes Schindelin wrote:
>
>> With this patch, --batch can be combined with --textconv or --filters.
>> For this to work, the input needs to have the form
>> 
>>  
>> 
>> so that the filters can be chosen appropriately.
>
> The object name can have spaces in it, too. E.g.:
>
>   HEAD:path with spaces
>
> or even:
>
>   :/grep for this

When I wrote my review, I didn't consider this use case.

There is no -z format in --batch, which is unfortunate.  If we had
one, it would trivially make it possible to do so, and we can even
have paths with LF in them ;-).  On the other hand, producing a NUL
separated input is a chore.

Perhaps a new and separate option that is similar to "--batch" but
lacks support for %(rest) and accepts _ONLY_ 40-hex as object name
is the best we can do, then?

> (as was pointed out to me when I tried to turn on %(rest) handling by
> default, long ago). How do those work with your patch?
>
> It looks like the extra split isn't enabled unless one of those options
> is selected. Since --filters is new, that's OK for backwards
> compatibility. But --textconv isn't. So I think:
>
>   echo "HEAD:path with spaces" |
>   git cat-file --textconv --batch
>
> is regressed by this patch.
>
> I wonder if we need an option specifically to say "the object name can
> be split". Right now it kicks in automatically if you use "%(rest)" in
> your format, but you might not care about passing along that output
> (e.g., a lot of times I am piping "rev-list" straight to cat-file, and I
> have to use a separate "cut" to throw away the pathnames).
>
> -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: git format-patch --break-rewrites broken in 2.9.3

[Jeff King]

On Thu, Aug 18, 2016 at 04:44:21PM +0200, Olaf Hering wrote:

> This command used to create a diff which can be consumed by patch. But
> at least with 2.9.3 it just gives a rename output:
> 
>  git format-patch \
> --no-signature \
> --stdout \
> --break-rewrites \
> --keep-subject \
>  
> 95fa0405c5991726e06c08ffcd8ff872f7fb4f2d^..95fa0405c5991726e06c08ffcd8ff872f7fb4f2d
> 
> 
> What must be done now to get a usable patch?

Probably --no-renames.

Renames were enabled by default by 5404c11 (diff: activate diff.renames
by default, 2016-02-25), which is in v2.9.0.

I wonder if we should consider undoing that for format-patch, whose
output may be consumed by non-git endpoints.

-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 v7 6/7] submodule: refactor show_submodule_summary with helper function

[Junio C Hamano]

Jacob Keller  writes:

 If we were to change those, we could discuss if we want to go with
 full sentences
 all the time:

 submodule is new
 submodule is deleted
 submodule is not initialized
>>>
>>> I agree, I'll make a new patch that does this as a cleanup prior to
>>> this re-work.
>> ...
> Sorry for being unclear myself, too. I'm keeping it as "not
> initialized" and updating the description of the patch that changed it
> from "not checked out" to "not initialized"

Whether it is done inside or outside the scope of this series, the
other two "is new"/"is deleted" updates look very sensible ones to
make in the longer run.  I'd further suggest to unify "commits not
present" and "revision walker failed" into one error class.  From
the end user's point of view, they aren't very different.
--
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 3/3] receive-pack: allow a maximum input size to be specified

[Jeff King]

On Thu, Aug 18, 2016 at 01:25:43PM -0700, Junio C Hamano wrote:

> > diff --git a/Documentation/config.txt b/Documentation/config.txt
> > index 0bcb679..f5b6061 100644
> > --- a/Documentation/config.txt
> > +++ b/Documentation/config.txt
> > @@ -2517,6 +2517,11 @@ receive.unpackLimit::
> > especially on slow filesystems.  If not set, the value of
> > `transfer.unpackLimit` is used instead.
> >  
> > +receive.maxsize::
> 
> Shouldn't this be maxInputSize, not just maxSize?  You are limiting
> the size of the input, not the size of the resulting pack, right?

Yeah, that is probably a better name. I don't recall why I picked
maxSize long ago.

> [...]

I agree, too, with all of the other comments you made.

-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 v7 4/7] submodule: allow do_submodule_path to work if given gitdir directly

[Junio C Hamano]

Jacob Keller  writes:

> From: Jacob Keller 
>
> Currently, do_submodule_path relies on read_gitfile, which will die() if
> it can't read from the specified gitfile. Unfortunately, this means that
> do_submodule_path will not work when given the path to a submodule which
> is checked out directly, such as a newly added submodule which you
> cloned and then "git submodule add". 

Hmm, are you sure about that?

A call to read_gitfile() turns into a call to read_gitfile_gently()
with the return_error_code parameter set to NULL.  The function does
a stat(2), and if the given path is not a file (e.g. we created the
submodule working tree and repository in-place ourselves, instead of
cloning an existing project from elsewhere, in which case we would
see a directory there), it says READ_GIT_FILE_ERR_NOT_A_FILE and
returns NULL, because that is not a fatal error condition.  The same
thing happens if path does not yet exist.

This caller is given , prepares "/.git" in buf, and
calls read_gitfile().  If it returns a non-NULL, it replaces what is
in buf and continues, but if it returns a NULL (i.e. the two cases I
mentioned in the above paragraph), it continues with "/.git".

While I do not think changing it to resolve_gitdir() is wrong per-se,
I am not sure if it is necessary.

I must be misreading something in the existing code.

> Instead, replace the call with
> resolve_gitdir. This first checks to see if we've been given a gitdir
> already.
>
> Because resolve_gitdir may return the same buffer it was passed, we have
> to check for this case as well, since strbuf_reset() will not work as
> expected here, and indeed is not necessary.
>
> Signed-off-by: Jacob Keller 
> ---
>  path.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/path.c b/path.c
> index 17551c483476..d1af029152a2 100644
> --- a/path.c
> +++ b/path.c
> @@ -477,8 +477,8 @@ static void do_submodule_path(struct strbuf *buf, const 
> char *path,
>   strbuf_complete(buf, '/');
>   strbuf_addstr(buf, ".git");
>  
> - git_dir = read_gitfile(buf->buf);
> - if (git_dir) {
> + git_dir = resolve_gitdir(buf->buf);
> + if (git_dir && git_dir != buf->buf) {
>   strbuf_reset(buf);
>   strbuf_addstr(buf, git_dir);
>   }
--
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 v7 6/7] submodule: refactor show_submodule_summary with helper function

[Jacob Keller]

On Thu, Aug 18, 2016 at 1:39 PM, Stefan Beller  wrote:
> On Thu, Aug 18, 2016 at 1:24 PM, Jacob Keller  wrote:
>> On Thu, Aug 18, 2016 at 12:04 PM, Stefan Beller  wrote:
>>> On Wed, Aug 17, 2016 at 5:51 PM, Jacob Keller  
>>> wrote:
 +   if (is_null_sha1(one))
 +   message = "(new submodule)";
 +   else if (is_null_sha1(two))
 +   message = "(submodule deleted)";
 +
 +   if (add_submodule_odb(path)) {
 +   if (!message)
 +   message = "(submodule not initialized)";
>>>
>>> Before it was  "(not initialized)" for this case, I think we'd rather keep 
>>> that?
>>> Though this code path is only used by the porcelain commands, we'd rather 
>>> not
>>> want to change this in a subtle way in this commit.
>>>
>>> If we were to change those, we could discuss if we want to go with
>>> full sentences
>>> all the time:
>>>
>>> submodule is new
>>> submodule is deleted
>>> submodule is not initialized
>>>
>>>
>>
>> I agree, I'll make a new patch that does this as a cleanup prior to
>> this re-work.
>
> I was actually not suggesting to change that. I rather wanted to point out
> that if we'd want that we'd rather do it consistently and in a different 
> patch.
> Sorry for not being clear.
>
> Thanks,
> Stefan
>

Sorry for being unclear myself, too. I'm keeping it as "not
initialized" and updating the description of the patch that changed it
from "not checked out" to "not initialized"

Thanks,
Jake

>>
>> Thanks,
>> 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 1/2 v5] pack-objects: respect --local/--honor-pack-keep/--incremental when bitmap is in use

[Jeff King]

On Tue, Aug 09, 2016 at 10:31:43PM +0300, Kirill Smelkov wrote:

> Since 6b8fda2d (pack-objects: use bitmaps when packing objects) there
> are two codepaths in pack-objects: with & without using bitmap
> reachability index.

Sorry, I got distracted from reviewing these patches. I'll give them a
detailed look now and hopefully we can finalize the topic.

> In want_object_in_pack() we care to start the checks from already found
> pack, if we have one, this way determining the answer right away
> in case neither --local nor --honour-pack-keep are active. In
> particular, as p5310-pack-bitmaps.sh shows, we do not do harm to
> served-with-bitmap clones performance-wise:
> 
> Test  56dfeb62  this tree
> -
> 5310.2: repack to disk9.63(8.67+0.33)   9.47(8.55+0.28) -1.7%
> 5310.3: simulated clone   2.07(2.17+0.12)   2.03(2.14+0.12) -1.9%
> 5310.4: simulated fetch   0.78(1.03+0.02)   0.76(1.00+0.03) -2.6%
> 5310.6: partial bitmap1.97(2.43+0.15)   1.92(2.36+0.14) -2.5%
> 
> with all differences strangely showing we are a bit faster now, but
> probably all being within noise.

Good to know there is no regression. It is curious that there is a
slight _improvement_ across the board. Do we have an explanation for
that? It seems odd that noise would be so consistent.

> And in the general case we care not to have duplicate
> find_pack_entry_one(*found_pack) calls. Worst what can happen is we can
> call want_found_object(*found_pack) -- newly introduced helper for
> checking whether we want object -- twice, but since want_found_object()
> is very lightweight it does not make any difference.

I had trouble parsing this. I think maybe:

  In the general case we do not want to call find_pack_entry_one() more
  than once, because it is expensive. This patch splits the loop in
  want_object_in_pack() into two parts: finding the object and seeing if
  it impacts our choice to include it in the pack. We may call the
  inexpensive want_found_object() twice, but we will never call
  find_pack_entry_one() if we do not need to.

> +static int want_found_object(int exclude, struct packed_git *p)
> +{
> + if (exclude)
> + return 1;
> + if (incremental)
> + return 0;
> +
> + /*
> +  * When asked to do --local (do not include an object that appears in a
> +  * pack we borrow from elsewhere) or --honor-pack-keep (do not include
> +  * an object that appears in a pack marked with .keep), finding a pack
> +  * that matches the criteria is sufficient for us to decide to omit it.
> +  * However, even if this pack does not satisfy the criteria, we need to
> +  * make sure no copy of this object appears in _any_ pack that makes us
> +  * to omit the object, so we need to check all the packs. Signal that by
> +  * returning -1 to the caller.
> +  */
> + if (!ignore_packed_keep &&
> + (!local || !have_non_local_packs))
> + return 1;

Hmm. The comment says "-1", but the return says "1". That is because the
comment is describing the return that happens at the end. :)

I wonder if the last sentence should be:

  We can check here whether these options can possibly matter; if not,
  we can return early from the function here. Otherwise, we signal "-1"
  at the end to tell the caller that we do not know either way, and it
  needs to check more packs.

> - *found_pack = NULL;
> - *found_offset = 0;
> + /*
> +  * If we already know the pack object lives in, start checks from that
> +  * pack - in the usual case when neither --local was given nor .keep 
> files
> +  * are present we will determine the answer right now.
> +  */
> + if (*found_pack) {
> + want = want_found_object(exclude, *found_pack);
> + if (want != -1)
> + return want;
> + }

Looks correct. Though it is not really "start checks from..." anymore,
but rather "do a quick check to see if we can quit early, and otherwise
start the loop". That might be nitpicking, though.

>   for (p = packed_git; p; p = p->next) {
> - off_t offset = find_pack_entry_one(sha1, p);
> + off_t offset;
> +
> + if (p == *found_pack)
> + offset = *found_offset;
> + else
> + offset = find_pack_entry_one(sha1, p);
> +

This hunk will conflict with the MRU optimizations in 'next', but I
think the resolution should be pretty trivial.

>  static int add_object_entry(const unsigned char *sha1, enum object_type type,
>   const char *name, int exclude)
>  {
> - struct packed_git *found_pack;
> - off_t found_offset;
> + struct packed_git *found_pack = NULL;
> + off_t found_offset = 0;

I think technically we don't need to initialize found_offset here (it is
considered only if *found_pack is not 

Re: git format-patch --break-rewrites broken in 2.9.3

[Junio C Hamano]

Junio C Hamano  writes:

> I think that is one half of the story.
>
> The other half is a long/well known bug that lets "diff -B -M" to
> produce incorrect/broken patch that cannot be applied.  It was
> documented in the thread that begins at:
>
> public-inbox.org/git/xmqqfvapuhkk@gitster.dls.corp.google.com
>
> but still hasn't been solved.

The problem report actually starts here:

public-inbox.org/git/xmqqegqaahnh@gitster.dls.corp.google.com/

--
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: Working with public-inbox.org [Was: [PATCH] rev-parse: respect core.hooksPath in --git-path]

[Junio C Hamano]

Eric Wong  writes:

> I see a choice of mail client as no different than a choice of
> text editor.  Neither my mail client or text editor is heavily
> customized.  The key feature I rely on from both tools is piping
> data to external commands.

FWIW, that applies to me exactly, too.

> unsubscribe: meta+unsubscr...@public-inbox.org

Did you mean this, really?

> archive: https://public-inbox.org/meta/

This I understand, though.

Ahh, that's coming from m...@public-inbox.org?
--
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 v7 2/7] graph: add support for --line-prefix on all graph-aware output

[Jacob Keller]

From: Jacob Keller 

On Thu, Aug 18, 2016 at 10:56 AM, Stefan Beller  wrote:
> On Wed, Aug 17, 2016 at 5:51 PM, Jacob Keller  
> wrote:
>> From: Jacob Keller 
>> -   /*
>> -* Add a newline after the commit message.
>> -*
>> -* Usually, this newline produces a blank
>> -* padding line between entries, in which 
>> case
>> -* we need to add graph padding on this line.
>> -*
>> -* However, the commit message may not end 
>> in a
>> -* newline.  In this case the newline simply
>> -* ends the last line of the commit message,
>> -* and we don't need any graph output.  (This
>> -* always happens with CMIT_FMT_ONELINE, and 
>> it
>> -* happens with CMIT_FMT_USERFORMAT when the
>> -* format doesn't explicitly end in a 
>> newline.)
>> -*/
>> -   if (buf.len && buf.buf[buf.len - 1] == '\n')
>> -   graph_show_padding(revs->graph);
>> -   putchar('\n');
>> -   } else {
>> -   /*
>> -* If the message buffer is empty, just show
>> -* the rest of the graph output for this
>> -* commit.
>> -*/
>> -   if (graph_show_remainder(revs->graph))
>> -   putchar('\n');
>> -   if (revs->commit_format == CMIT_FMT_ONELINE)
>> -   putchar('\n');
>> -   }
>> +   /*
>> +   * Add a newline after the commit message.
>
> I wondered if it is my webmailer displaying things wrongly here so I checked
> it at public inbox, and fetched the mails and applied them.
>
> It seems to me as if this long comment is broken in indentation
> (i.e. you removed a blank ' ' directly in front of the '*' instead of a '\t' 
> ?)
>

Yea it is indeed broken. Something like the following squash should fix it.
We could also re-flow the text if desired too, but I don't really see the
advantage.

8<-

>From 345bbaa47cc14aba7049738e99c3649e2c06748c Mon Sep 17 00:00:00 2001
From: Jacob Keller 
Date: Thu, 18 Aug 2016 11:13:04 -0700
Subject: [PATCH] SQUASH: fix indentation of comment in builtin/rev-list.c

Signed-off-by: Jacob Keller 
---
 builtin/rev-list.c | 28 ++--
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 1a75a83538f4..21cde8dd6b31 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -129,20 +129,20 @@ static void show_commit(struct commit *commit, void *data)
graph_show_commit_msg(revs->graph, stdout, );
 
/*
-   * Add a newline after the commit message.
-   *
-   * Usually, this newline produces a blank
-   * padding line between entries, in which case
-   * we need to add graph padding on this line.
-   *
-   * However, the commit message may not end in a
-   * newline.  In this case the newline simply
-   * ends the last line of the commit message,
-   * and we don't need any graph output.  (This
-   * always happens with CMIT_FMT_ONELINE, and it
-   * happens with CMIT_FMT_USERFORMAT when the
-   * format doesn't explicitly end in a newline.)
-   */
+   * Add a newline after the commit message.
+   *
+   * Usually, this newline produces a blank
+   * padding line between entries, in which case
+   * we need to add graph padding on this line.
+   *
+   * However, the commit message may not end in a
+   * newline.  In this case the newline simply
+   * ends the last line of the commit message,
+   * and we don't need any graph output.  (This
+   * 

Re: [PATCH 2/4] cat-file: introduce the --filters option

[Junio C Hamano]

Johannes Schindelin  writes:

> As suggested by its name, the --filters option applies the filters
> that are currently configured for the specified path.
>
> This feature comes in handy when a 3rd-party tool wants to work with
> the contents of files from past revisions as if they had been checked
> out, but without detouring via temporary files.
>
> Note that we ensure that symbolic links are unaffected (we know from
> looking at the mode whether it refers to a symbolic link or a regular
> file).

I think you can lose the last paragraph and enrich the first one
instead.  The text as-given does not even say anything about the new
option affecting only blobs.

The --filters option applies the convert_to_working_tree()
filter for the path when showing the contents of a regular
file blob object.

or something like that.

> +--filters::
> + Show the content as transformed by the filters configured in
> + the current working tree for the given  (i.e. smudge filters,
> + end-of-line conversion, etc). In this case,  has to be of
> + the form :, or :.

Makes sense.  We show the contents as if we are checking the blob
out to the given , in other words.

> diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> index 2dfe626..0b74afa 100644
> --- a/builtin/cat-file.c
> +++ b/builtin/cat-file.c
> @@ -20,6 +20,33 @@ struct batch_options {
>   const char *format;
>  };
>  
> +static int filter_object(const char *path, unsigned mode,
> +  const unsigned char *sha1,
> +  char **buf, unsigned long *size)
> +{
> + enum object_type type;
> +
> + *buf = read_sha1_file(sha1, , size);
> + if (!*buf)
> + return error(_("cannot read object %s '%s'"),
> + sha1_to_hex(sha1), path);
> + if (type != OBJ_BLOB) {
> + free(*buf);
> + return error(_("blob expected for %s '%s'"),
> + sha1_to_hex(sha1), path);
> + }
> + if (S_ISREG(mode)) {
> + struct strbuf strbuf = STRBUF_INIT;
> + if (convert_to_working_tree(path, *buf, *size, )) {
> + free(*buf);
> + *size = strbuf.len;
> + *buf = strbuf_detach(, NULL);
> + }
> + }

This is just a tangent for future clean-up of the coding convention,
but if we used size_t internally throughout the system, we would be
able to use strbuf_detach(, size) instead.  With the current
coding convention, all internal sizes are ulong, and this is the
best we could do.

> @@ -61,6 +88,16 @@ static int cat_one_file(int opt, const char *exp_type, 
> const char *obj_name,
>   case 'e':
>   return !has_sha1_file(sha1);
>  
> + case 'w':
> + if (!obj_context.path[0])
> + die("git cat-file --filters %s:  must be "
> + "", obj_name);
> +
> + if (filter_object(obj_context.path, obj_context.mode,
> +   sha1, , ))
> + return -1;
> + break;
> +

Makes sense.

> diff --git a/t/t8010-cat-file-filters.sh b/t/t8010-cat-file-filters.sh
> new file mode 100755
> index 000..e466634
> --- /dev/null
> +++ b/t/t8010-cat-file-filters.sh
> @@ -0,0 +1,34 @@
> +#!/bin/sh
> +
> +test_description='git cat-file filters support'
> +. ./test-lib.sh
> +
> +test_expect_success 'setup ' '
> + echo "*.txt eol=crlf diff=txt" >.gitattributes &&
> + echo "hello" | append_cr >world.txt &&
> + git add .gitattributes world.txt &&
> + test_tick &&
> + git commit -m "Initial commit"
> +'

OK, so we ask *.txt in the working tree to be CRLF (and presumably
that means the in-repository objects are LF).

> +has_cr () {
> + tr '\015' Q <"$1" | grep Q >/dev/null
> +}
> +
> +test_expect_success 'no filters with `git show`' '
> + git show HEAD:world.txt >actual &&
> + ! has_cr actual
> +
> +'
> +
> +test_expect_success 'no filters with cat-file' '
> + git cat-file blob HEAD:world.txt >actual &&
> + ! has_cr actual
> +'

OK.

> +test_expect_success 'cat-file --filters converts to worktree version' '
> + git cat-file --filters HEAD:world.txt >actual &&
> + has_cr actual
> +'

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

[PATCHv9 8/8] clone: recursive and reference option triggers submodule alternates

[Stefan Beller]

When `--recursive` and `--reference` is given, it is reasonable to
expect that the submodules are created with references to the submodules
of the given alternate for the superproject.

  An initial attempt to do this was presented to the mailing list, which
  used flags that are passed around ("--super-reference") that instructed
  the submodule clone to look for a reference in the submodules of the
  referenced superproject. This is not well thought out, as any further
  `submodule update` should also respect the initial setup.

  When a new  submodule is added to the superproject and the alternate
  of the superproject does not know about that submodule yet, we rather
  error out informing the user instead of being unclear if we did or did
  not use a submodules alternate.

To solve this problem introduce new options that store the configuration
for what the user wanted originally.

Signed-off-by: Stefan Beller 
---
 Documentation/config.txt   |  12 +
 builtin/clone.c|  19 
 builtin/submodule--helper.c| 102 +
 t/t7408-submodule-reference.sh |  43 +
 4 files changed, 176 insertions(+)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index bc1c433..e2571ea 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2837,6 +2837,18 @@ submodule.fetchJobs::
in parallel. A value of 0 will give some reasonable default.
If unset, it defaults to 1.
 
+submodule.alternateLocation::
+   Specifies how the submodules obtain alternates when submodules are
+   cloned. Possible values are `no`, `superproject`.
+   By default `no` is assumed, which doesn't add references. When the
+   value is set to `superproject` the submodule to be cloned computes
+   its alternates location relative to the superprojects alternate.
+
+submodule.alternateErrorStrategy
+   Specifies how to treat errors with the alternates for a submodule
+   as computed via `submodule.alternateLocation`. Possible values are
+   `ignore`, `info`, `die`. Default is `die`.
+
 tag.forceSignAnnotated::
A boolean to specify whether annotated tags created should be GPG 
signed.
If `--annotate` is specified on the command line, it takes
diff --git a/builtin/clone.c b/builtin/clone.c
index 0182665..404c5e8 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -947,6 +947,25 @@ int cmd_clone(int argc, const char **argv, const char 
*prefix)
else
fprintf(stderr, _("Cloning into '%s'...\n"), dir);
}
+
+   if (option_recursive) {
+   if (option_required_reference.nr &&
+   option_optional_reference.nr)
+   die(_("clone --recursive is not compatible with "
+ "both --reference and --reference-if-able"));
+   else if (option_required_reference.nr) {
+   string_list_append(_config,
+   "submodule.alternateLocation=superproject");
+   string_list_append(_config,
+   "submodule.alternateErrorStrategy=die");
+   } else if (option_optional_reference.nr) {
+   string_list_append(_config,
+   "submodule.alternateLocation=superproject");
+   string_list_append(_config,
+   "submodule.alternateErrorStrategy=info");
+   }
+   }
+
init_db(option_template, INIT_DB_QUIET);
write_config(_config);
 
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 7096848..a366757 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -472,6 +472,105 @@ static int clone_submodule(const char *path, const char 
*gitdir, const char *url
return run_command();
 }
 
+struct submodule_alternate_setup {
+   const char *submodule_name;
+   enum SUBMODULE_ALTERNATE_ERROR_MODE {
+   SUBMODULE_ALTERNATE_ERROR_DIE,
+   SUBMODULE_ALTERNATE_ERROR_INFO,
+   SUBMODULE_ALTERNATE_ERROR_IGNORE
+   } error_mode;
+   struct string_list *reference;
+};
+#define SUBMODULE_ALTERNATE_SETUP_INIT { NULL, \
+   SUBMODULE_ALTERNATE_ERROR_IGNORE, NULL }
+
+static int add_possible_reference_from_superproject(
+   struct alternate_object_database *alt, void *sas_cb)
+{
+   struct submodule_alternate_setup *sas = sas_cb;
+
+   /* directory name, minus trailing slash */
+   size_t namelen = alt->name - alt->base - 1;
+   struct strbuf name = STRBUF_INIT;
+   strbuf_add(, alt->base, namelen);
+
+   /*
+* If the alternate object store is another repository, try the
+* standard layout with .git/modules//objects
+*/
+   if (ends_with(name.buf, ".git/objects")) {
+   char 

Re: [PATCH 2/2] mingw: ensure temporary file handles are not inherited by child processes

[Junio C Hamano]

Eric Wong  writes:

> O_CLOEXEC only exists since Linux 2.6.23 and there are likely
> still LTS (CentOS 5.x?) and non-Linux systems which do not have
> it, as well as machines with could have it defined in userspace
> headers but not have it in the kernel.
>
> So I suggest something like the following: (untested)
>
> #define GIT_O_TMP (O_RDWR | O_CREAT | O_EXCL)
>
> #ifndef O_CLOEXEC
> #  define O_CLOEXEC 0
> #endif
>   /* state: -1=unknown; 0=broken; 1=working */
>   static int cloexec_state = O_CLOEXEC == 0 ? 0 : -1;
>   static int GIT_O_ETMP = (GIT_O_TMP | O_CLOEXEC)
>
>   int fd = open(filename, GIT_O_ETMP, 0666);
> ...
>   /*
>* This is racy in the presence of threads,
>* but the best we can do for old *nix:
>*/
> #if defined(F_GETFD) && defined(F_SETFD) && defined(FD_CLOEXEC)
>   if (fd >= 0 && cloexec_state != 1) {
>   int flags = fcntl(fd, F_GETFD);
>
>   if (flags == -1)
>   die_errno("F_GETFD failed");
>   if (flags & O_CLOEXEC)
>   cloexec_state = 1;
>   else {
>   flags = fcntl(fd, F_SETFD, flags | FD_CLOEXEC);
>   if (flags == -1)
>   die_errno("F_SETFD failed");
>   cloexec_state = 0;
>
>   }
>   }
> #endif
>   ...

Our codepaths themselves generally do not care about O_CLOEXEC, so
giving a racy emulation of it is not worth the effort, making the
later half of the above an overkill.  Perhaps the three lines to
define O_CLOEXEC to 0 on older UNIX might be sufficient.
--
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/2] help: introduce option --command-only

[Ralf Thielow]

Introduce option --command-only to the help command.  With this option
being passed, "git help" will open man pages only for commands.

Since we know it is a command, we can use function help_unknown_command
to give the user advice on typos.

Signed-off-by: Ralf Thielow 
---
I am not sure about the first test case, but I think it'd have
prevented me from making earlier mistakes of this change. That's
why I added it.
Just calling a git command that succeeds in a test isn't really
a check, so ... I dunno

 Documentation/git-help.txt | 11 ---
 builtin/help.c | 30 +++---
 contrib/completion/git-completion.bash |  2 +-
 t/t0012-help.sh| 21 +
 4 files changed, 53 insertions(+), 11 deletions(-)
 create mode 100755 t/t0012-help.sh

diff --git a/Documentation/git-help.txt b/Documentation/git-help.txt
index 40d328a..cf6a414 100644
--- a/Documentation/git-help.txt
+++ b/Documentation/git-help.txt
@@ -8,7 +8,7 @@ git-help - Display help information about Git
 SYNOPSIS
 
 [verse]
-'git help' [-a|--all] [-g|--guide]
+'git help' [-a|--all] [-c|--command-only] [-g|--guide]
   [-i|--info|-m|--man|-w|--web] [COMMAND|GUIDE]
 
 DESCRIPTION
@@ -29,8 +29,9 @@ guide is brought up. The 'man' program is used by default for 
this
 purpose, but this can be overridden by other options or configuration
 variables.
 
-Note that `git --help ...` is identical to `git help ...` because the
-former is internally converted into the latter.
+Note that `git --help ...` is almost identical to `git help ...` because
+the former is internally converted into the latter with option --command-only
+being added.
 
 To display the linkgit:git[1] man page, use `git help git`.
 
@@ -43,6 +44,10 @@ OPTIONS
Prints all the available commands on the standard output. This
option overrides any given command or guide name.
 
+-c::
+--command-only::
+   Display help information only for commands.
+
 -g::
 --guides::
Prints a list of useful guides on the standard output. This
diff --git a/builtin/help.c b/builtin/help.c
index 8848013..2249a67 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -37,8 +37,10 @@ static int show_all = 0;
 static int show_guides = 0;
 static unsigned int colopts;
 static enum help_format help_format = HELP_FORMAT_NONE;
+static int cmd_only;
 static struct option builtin_help_options[] = {
OPT_BOOL('a', "all", _all, N_("print all available commands")),
+   OPT_BOOL('c', "command-only", _only, N_("show help only for 
commands")),
OPT_BOOL('g', "guides", _guides, N_("print list of useful 
guides")),
OPT_SET_INT('m', "man", _format, N_("show man page"), 
HELP_FORMAT_MAN),
OPT_SET_INT('w', "web", _format, N_("show manual in web browser"),
@@ -433,10 +435,29 @@ static void list_common_guides_help(void)
putchar('\n');
 }
 
+static const char *check_git_cmd(const char* cmd)
+{
+   char *alias;
+
+   if (is_git_command(cmd))
+   return cmd;
+
+   alias = alias_lookup(cmd);
+   if (alias) {
+   printf_ln(_("`git %s' is aliased to `%s'"), cmd, alias);
+   free(alias);
+   exit(0);
+   }
+
+   if (cmd_only)
+   return help_unknown_cmd(cmd);
+
+   return cmd;
+}
+
 int cmd_help(int argc, const char **argv, const char *prefix)
 {
int nongit;
-   char *alias;
enum help_format parsed_help_format;
 
argc = parse_options(argc, argv, prefix, builtin_help_options,
@@ -476,12 +497,7 @@ int cmd_help(int argc, const char **argv, const char 
*prefix)
if (help_format == HELP_FORMAT_NONE)
help_format = parse_help_format(DEFAULT_HELP_FORMAT);
 
-   alias = alias_lookup(argv[0]);
-   if (alias && !is_git_command(argv[0])) {
-   printf_ln(_("`git %s' is aliased to `%s'"), argv[0], alias);
-   free(alias);
-   return 0;
-   }
+   argv[0] = check_git_cmd(argv[0]);
 
switch (help_format) {
case HELP_FORMAT_NONE:
diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index c1b2135..354afe5 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1393,7 +1393,7 @@ _git_help ()
 {
case "$cur" in
--*)
-   __gitcomp "--all --guides --info --man --web"
+   __gitcomp "--all --command-only --guides --info --man --web"
return
;;
esac
diff --git a/t/t0012-help.sh b/t/t0012-help.sh
new file mode 100755
index 000..e20f907
--- /dev/null
+++ b/t/t0012-help.sh
@@ -0,0 +1,21 @@
+#!/bin/sh
+
+test_description='help'
+
+. ./test-lib.sh
+
+test_expect_success "works for commands and guides by default" "
+   git help status &&
+   git help revisions
+"
+
+test_expect_success 

Re: [PATCH 2/2] help: make option --help open man pages only for Git commands

[Junio C Hamano]

Ralf Thielow  writes:

> If option --help is passed to a Git command, we try to open
> the man page of that command.  However, we do it even for commands
> we don't know.  Make sure it is a Git command.

What the patch does is correct, I think, but the explanation may
invite a false alarm.  If you added a custom command git-who in your
$PATH, with an appropriate documentation for git-who(1), we would
still show its documentation, no?

The same comment applies to 1/2, too, in that the word "command"
will be interpreted differently by different people.  For example,
"git co --help" and "git help co" would work, with or without 1/2 in
place when you have "[alias] co = checkout", so we are calling "Git
subcommands that we ship, custom commands 'git-$foo' the users have
in their $PATH, and aliases the users create" collectively "command".

As long as the reader understands that definition, both the log
messages of 1/2 and 2/2 _and_ the updated description for "git help"
we have in 1/2 are all very clear.  I do not care too much about the
commit log message, but we may want to think about the documentation
a bit more.

Here is what 1/2 adds to "git help" documentation:

+Note that `git --help ...` is almost identical to `git help ...` because
+the former is internally converted into the latter with option 
--command-only
+being added.

 To display the linkgit:git[1] man page, use `git help git`.

@@ -43,6 +44,10 @@ OPTIONS
Prints all the available commands on the standard output. This
option overrides any given command or guide name.

+-c::
+--command-only::
+   Display help information only for commands.
+

First, I do not think a short form is unnecessary; the users are not
expected to use that form, once they started typing "git help...".
If we flip the polarity and call it --exclude-guides or something,
would it make it less ambiguous?

> This breaks "git  --help" while "git help " still works.

I wouldn't call that a breakage; "git everyday --help" shouldn't
have worked in the first place.  It did something useful merely by
accident ;-).

> diff --git a/git.c b/git.c
> index 0f1937f..2cd2e06 100644
> --- a/git.c
> +++ b/git.c
> @@ -528,10 +528,23 @@ static void handle_builtin(int argc, const char **argv)
>   strip_extension(argv);
>   cmd = argv[0];
>  
> - /* Turn "git cmd --help" into "git help cmd" */
> + /* Turn "git cmd --help" into "git help --command-only cmd" */
>   if (argc > 1 && !strcmp(argv[1], "--help")) {
> + struct argv_array args;
> + int i;
> +
>   argv[1] = argv[0];
>   argv[0] = cmd = "help";
> +
> + argv_array_init();
> + for (i = 0; i < argc; i++) {
> + argv_array_push(, argv[i]);
> + if (!i)
> + argv_array_push(, "--command-only");
> + }
> +
> + argc++;
> + argv = argv_array_detach();
>   }
>  
>   builtin = get_builtin(cmd);

The code does this after it:

if (builtin)
exit(run_builtin(...));

and returns.  If we didn't get builtin, we risk leaking args.argv
here, but we assume argv[0] = cmd = "help" is always a builtin,
which I think is a safe assumption, so the code is OK.  Static
checkers that are only half intelligent may yell at you for not
releasing the resources, though.  I wonder if it is worth doing

static void handle_builtin(int argc, const char **argv)
{
struct argv_array args = ARGV_ARRAY_INIT;
...
if (argc > 1 && !strcmp(argv[1], "--help")) {
...
argv = args.argv;
}
builtin = get_builtin(cmd);
if (builtin)
exit(run_builtin(...));
argv_array_clear();
}   

to help unconfuse them.

By the way, I do not see these patches on gmane, public-inbox or
usual suspects.  Perhaps vger is having a bad day or something?

--
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 v7 4/7] submodule: allow do_submodule_path to work if given gitdir directly

[Stefan Beller]

On Wed, Aug 17, 2016 at 5:51 PM, Jacob Keller  wrote:
> From: Jacob Keller 
>
> Currently, do_submodule_path relies on read_gitfile, which will die() if
> it can't read from the specified gitfile. Unfortunately, this means that
> do_submodule_path will not work when given the path to a submodule which
> is checked out directly, such as a newly added submodule which you
> cloned and then "git submodule add". Instead, replace the call with
> resolve_gitdir. This first checks to see if we've been given a gitdir
> already.
>
> Because resolve_gitdir may return the same buffer it was passed, we have
> to check for this case as well, since strbuf_reset() will not work as
> expected here, and indeed is not necessary.
>
> Signed-off-by: Jacob Keller 
> ---
>  path.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Looks good,
Thanks,
Stefan
--
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 3/3] receive-pack: allow a maximum input size to be specified

[Junio C Hamano]

Christian Couder  writes:

> From: Jeff King 
>
> Receive-pack feeds its input to either index-pack or
> unpack-objects, which will happily accept as many bytes as
> a sender is willing to provide. Let's allow an arbitrary
> cutoff point where we will stop writing bytes to disk.
>
> Cleaning up what has already been written to disk is a
> related problem that is not addressed by this patch.
>
> Signed-off-by: Jeff King 
> Signed-off-by: Christian Couder 
> ---
>  Documentation/config.txt   |  5 +
>  Documentation/git-receive-pack.txt |  3 +++
>  builtin/receive-pack.c | 12 +++
>  t/t5546-push-limits.sh | 42 
> ++
>  4 files changed, 62 insertions(+)
>  create mode 100755 t/t5546-push-limits.sh
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 0bcb679..f5b6061 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -2517,6 +2517,11 @@ receive.unpackLimit::
>   especially on slow filesystems.  If not set, the value of
>   `transfer.unpackLimit` is used instead.
>  
> +receive.maxsize::

Shouldn't this be maxInputSize, not just maxSize?  You are limiting
the size of the input, not the size of the resulting pack, right?

> + If the size of a pack file is larger than this limit, then

So, s/a pack file/the incoming pack stream/ or something?

> diff --git a/t/t5546-push-limits.sh b/t/t5546-push-limits.sh
> new file mode 100755
> index 000..09e958f

Technically, that's receive-limits, no?  It is conceivable that the
client side can also grow a feature to limit the size of an outgoing
push, but that is not what this new script is about.

> --- /dev/null
> +++ b/t/t5546-push-limits.sh
> @@ -0,0 +1,42 @@
> +#!/bin/sh
> +
> +test_description='check input limits for pushing'
> +. ./test-lib.sh
> +
> +test_expect_success 'create remote repository' '
> + git init --bare dest
> +'
> +

> +# Let's run tests with different unpack limits: 1 and 10
> +# When the limit is 1, `git receive-pack` will call `git index-pack`.
> +# When the limit is 10, `git receive-pack` will call `git unpack-objects`.

I agree with Peff that these tests should push into an empty,
pristine destination.  Move the "create remote" step inside the
while loop and prefix its "git init" with "rm -rf dest", or
something like that.

It would make it crystal clear that the produced packstream for our
transfer won't ever be affected by what is leftover in the
destination repository.

Also, I think it would make it far more readable if you made the
body of the while loop into a helper function that runs tests,
keeping "1/10 corresponds to index/unpack" magic hidden inside the
helper, i.e. something like

test_pack_input_limit () {
size=$2
case "$1" in
unpack) unpack_limit=1 ;;
index) unpack_limit=1 ;;
esac

test_expect_success 'prepare destination repository' '
rm -fr dest &&
git --bare init dest
'

test_expect_success "set unpacklimit to $unpack_limit" '
git --git-dir=dest config receive.unpacklimit "$unpack_limit"
'

test_expect_success 'setting receive.maxsize to 512 rejects push' '
git --git-dir=dest config receive.maxsize 512 &&
test_must_fail git push dest HEAD
'

test_expect_success 'bumping limit to 4k allows push' '
git --git-dir=dest config receive.maxsize 4k &&
git push dest HEAD
'

test_expect_success 'prepare destination repository (again)' '
rm -fr dest &&
git --bare init dest
'

test_expect_success 'lifting the limit allows push' '
git --git-dir=dest config receive.maxsize 0 &&
git push dest HEAD
'
}

and have the body of the test more like this:

test_expect_success 'setup' '
test-genrandom foo 1024 >test-file &&
git add test-file &&
test_commit test-file
'

test_pack_input_limit unpack 1024 
test_pack_input_limit index 1024 

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

What's cooking in git.git (Aug 2016, #07; Thu, 18)

[Junio C Hamano]

Here are the topics that have been cooking.  Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.  The ones marked with '.' do not appear in any of
the integration branches, but I am still holding onto them.

The preview for the upcoming 2.10 release has been tagged as
v2.10.0-rc0.  The -rc1 is expected to happen very soon.

You can find the changes described here in the integration branches
of the repositories listed at

http://git-blame.blogspot.com/p/git-public-repositories.html

--
[Graduated to "master"]

* jk/tighten-alloc (2016-08-13) 2 commits
  (merged to 'next' on 2016-08-16 at 5399351)
 + receive-pack: use FLEX_ALLOC_MEM in queue_command()
 + correct FLEXPTR_* example in comment

 Small code and comment clean-up.


* js/test-lint-pathname (2016-08-16) 1 commit
  (merged to 'next' on 2016-08-16 at d154f90)
 + t/Makefile: ensure that paths are valid on platforms we care

 The "t/" hierarchy is prone to get an unusual pathname; "make test"
 has been taught to make sure they do not contain paths that cannot
 be checked out on Windows (and the mechanism can be reusable to
 catch pathnames that are not portable to other platforms as need
 arises).


* rs/mailinfo-lib (2016-08-13) 1 commit
  (merged to 'next' on 2016-08-16 at 14101e3)
 + mailinfo: recycle strbuf in check_header()

 Small code clean-up.


* sg/reflog-past-root (2016-08-15) 1 commit
  (merged to 'next' on 2016-08-16 at ee997a5)
 + t1410: remove superfluous 'git reflog' from the 'walk past root' test

 A small test clean-up for a topic introduced in v2.9.1 and later.


* va/i18n (2016-08-12) 3 commits
  (merged to 'next' on 2016-08-16 at 6a06cd3)
 + t7411: become resilient to GETTEXT_POISON
 + t5520: become resilient to GETTEXT_POISON
 + t3404: become resilient to GETTEXT_POISON

 A handful of tests that were broken under gettext-poison build have
 been fixed.

--
[New Topics]

* ab/hooks (2016-08-16) 1 commit
  (merged to 'next' on 2016-08-17 at b56e55d)
 + rev-parse: respect core.hooksPath in --git-path

 "git rev-parse --git-path hooks/" learned to take
 core.hooksPath configuration variable (introduced during 2.9 cycle)
 into account.

 Will merge to 'master'.


* sb/checkout-explit-detach-no-advice (2016-08-15) 1 commit
  (merged to 'next' on 2016-08-17 at fb64716)
 + checkout: do not mention detach advice for explicit --detach option

 "git checkout --detach " used to give the same advice
 message as that is issued when "git checkout " (or anything
 that is not a branch name) is given, but asking with "--detach" is
 an explicit enough sign that the user knows what is going on.  The
 advice message has been squelched in this case.

 Will merge to 'master'.


* lt/gpg-show-long-key-in-signature-verification (2016-08-16) 1 commit
  (merged to 'next' on 2016-08-17 at 1ee8a00)
 + Merge branch 'lt/gpg-show-long-key-in-signature-verification-maint' into 
lt/gpg-show-long-key-in-signature-verification
 (this branch uses lt/gpg-show-long-key-in-signature-verification-maint.)

 "git log --show-signature" and other commands that display the
 verification status of PGP signature now shows the longer key-id,
 as 32-bit key-id is so last century.

 Will merge to 'master'.


* lt/gpg-show-long-key-in-signature-verification-maint (2016-08-16) 1 commit
 + gpg-interface: prefer "long" key format output when verifying pgp signatures
 (this branch is used by lt/gpg-show-long-key-in-signature-verification.)

 "git log --show-signature" and other commands that display the
 verification status of PGP signature now shows the longer key-id,
 as 32-bit key-id is so last century.  This is based on older
 codebase, just in case somebody wants to have it.


* ak/curl-imap-send-explicit-scheme (2016-08-17) 1 commit
 - imap-send: Tell cURL to use imap:// or imaps://

 When we started cURL to talk to imap server when a new enough
 version of cURL library is available, we forgot to explicitly add
 imap(s):// before the destination.  To some folks, that didn't work
 and the library tried to make HTTP(s) requests instead.

 Needs review and testing.


* bw/mingw-avoid-inheriting-fd-to-lockfile (2016-08-18) 2 commits
 - mingw: ensure temporary file handles are not inherited by child processes
 - t6026-merge-attr: child processes must not inherit index.lock handles

 The tempfile (hence its user lockfile) API lets the caller to open
 a file descriptor to a temporary file, write into it and then
 finalize it by first closing the filehandle and then either
 removing or renaming the temporary file.  When the process spawns a
 subprocess after obtaining the file descriptor, and if the
 subprocess has not exited when the attempt to remove or rename is
 made, the last step fails on Windows, because the subprocess has
 the file descriptor still open.  Open tempfile with O_CLOEXEC flag
 to avoid this (on Windows, this is 

Re: git format-patch --break-rewrites broken in 2.9.3

[Junio C Hamano]

Johannes Schindelin  writes:

> Hi Olaf,
>
>> --break-rewrites \
>> --keep-subject \
>>  
>> 95fa0405c5991726e06c08ffcd8ff872f7fb4f2d^..95fa0405c5991726e06c08ffcd8ff872f7fb4f2d
>> 
>> What must be done now to get a usable patch?
>
> Maybe --no-renames? BTW this behavior was not introduced in 2.9.3, but in
> 2.9.0:
>
> https://github.com/git/git/blob/v2.9.0/Documentation/RelNotes/2.9.0.txt#L7-L9

I think that is one half of the story.

The other half is a long/well known bug that lets "diff -B -M" to
produce incorrect/broken patch that cannot be applied.  It was
documented in the thread that begins at:

public-inbox.org/git/xmqqfvapuhkk@gitster.dls.corp.google.com

but still hasn't been solved.
--
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 3/4] cat-file --textconv/--filters: allow specifying the path separately

[Junio C Hamano]

Johannes Schindelin  writes:

> There are circumstances when it is relatively easy to figure out the
> object name for a given path, but not the revision. For example, when

revision of the containing tree, I presume?

> Let's simplify this dramatically, because we do not really need that
> revision at all: all we care about is that we know the path. In the
> scenario described above, we do know the path, and we just want to
> specify it separately from the object name.

I wouldn't call it "simplifying dramatically".  It is just to
support two use cases that is different from the use case where you
want to use ":".

We already have a precedent in "hash-object --path=" for these
two different uses cases from the primary one.  That form can be
used when we know the contents but we do not know where the contents
came from.  It can also be used when we want to pretend that
contents came from a specific path, that may be different from the
file we are hashing.

Let's be consistent and (1) call it "--path", and (2) explain it as
a feature to allow you to tell the path separately or allow you to
pretend that the content is at a path different from reality.

After all, you would want to ignore  part in this construct:

git cat-file --filter --path= HEAD:

for the purpose of filtering, right?

> diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> index 0b74afa..5ff58b3 100644
> --- a/builtin/cat-file.c
> +++ b/builtin/cat-file.c
> @@ -20,6 +20,8 @@ struct batch_options {
>   const char *format;
>  };
>  
> +static const char *force_path;
> +
>  static int filter_object(const char *path, unsigned mode,
>const unsigned char *sha1,
>char **buf, unsigned long *size)
> @@ -89,21 +91,24 @@ static int cat_one_file(int opt, const char *exp_type, 
> const char *obj_name,
>   return !has_sha1_file(sha1);
>  
>   case 'w':
> - if (!obj_context.path[0])
> + if (!force_path && !obj_context.path[0])
>   die("git cat-file --filters %s:  must be "
>   "", obj_name);
>  
> - if (filter_object(obj_context.path, obj_context.mode,
> + if (filter_object(force_path ? force_path : obj_context.path,
> +   force_path ? 0100644 : obj_context.mode,
> sha1, , ))

The mode override is somewhat questionable.  Wouldn't you want to
reject

git cat-file --filter --path=Makefile HEAD:RelNotes

when HEAD:RelNotes blob is known to be a symbolic link?  After all,
you would reject this

git cat-file --filter --path=Makefile HEAD:t/

and it is quite similar, no?  I think this can be argued both ways,
but I have a feeling that obj_context.mode, if available, should be
honored with or without force_path.

> diff --git a/t/t8010-cat-file-filters.sh b/t/t8010-cat-file-filters.sh
> index e466634..fd17159 100755
> --- a/t/t8010-cat-file-filters.sh
> +++ b/t/t8010-cat-file-filters.sh
> @@ -31,4 +31,17 @@ test_expect_success 'cat-file --filters converts to 
> worktree version' '
>   has_cr actual
>  '
>  
> +test_expect_success 'cat-file --filters --use-path= works' '
> + sha1=$(git rev-parse -q --verify HEAD:world.txt) &&
> + git cat-file --filters --use-path=world.txt $sha1 >actual &&
> + has_cr actual
> +'
> +
> +test_expect_success 'cat-file --textconv --use-path= works' '
> + sha1=$(git rev-parse -q --verify HEAD:world.txt) &&
> + test_config diff.txt.textconv "tr A-Za-z N-ZA-Mn-za-m <" &&
> + git cat-file --textconv --use-path=hello.txt $sha1 >rot13 &&
> + test uryyb = "$(cat rot13 | remove_cr)"
> +'

I think I saw some code to ensure "when giving this option you need
that option in effect, too"; they should be tested here, too, no?
--
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 v7 3/7] diff: prepare for additional submodule formats

[Stefan Beller]

On Wed, Aug 17, 2016 at 5:51 PM, Jacob Keller  wrote:
> From: Jacob Keller 
>
> A future patch will add a new format for displaying the difference of
> a submodule. Make it easier by changing how we store the current
> selected format. Replace the DIFF_OPT flag with an enumeration, as each
> format will be mutually exclusive.
>
> Signed-off-by: Jacob Keller 
> ---

Looks good,
Thanks
Stefan
--
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 format-patch --break-rewrites broken in 2.9.3

[Johannes Schindelin]

Hi Olaf,

On Thu, 18 Aug 2016, Olaf Hering wrote:

> This command used to create a diff which can be consumed by patch. But
> at least with 2.9.3 it just gives a rename output:
> 
>  git format-patch \
> --no-signature \
> --stdout \
> --break-rewrites \
> --keep-subject \
>  
> 95fa0405c5991726e06c08ffcd8ff872f7fb4f2d^..95fa0405c5991726e06c08ffcd8ff872f7fb4f2d
> 
> 
> What must be done now to get a usable patch?

Maybe --no-renames? BTW this behavior was not introduced in 2.9.3, but in
2.9.0:

https://github.com/git/git/blob/v2.9.0/Documentation/RelNotes/2.9.0.txt#L7-L9

Ciao,
Johannes
--
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

git format-patch --break-rewrites broken in 2.9.3

[Olaf Hering]

This command used to create a diff which can be consumed by patch. But
at least with 2.9.3 it just gives a rename output:

 git format-patch \
--no-signature \
--stdout \
--break-rewrites \
--keep-subject \
 
95fa0405c5991726e06c08ffcd8ff872f7fb4f2d^..95fa0405c5991726e06c08ffcd8ff872f7fb4f2d


What must be done now to get a usable patch?

Olaf


signature.asc
Description: PGP signature

Re: Working from different machines.

[Tobiah]

On 08/17/2016 03:41 PM, Junio C Hamano wrote:

Tobiah  writes:


In other words, I want to work from different machines, and always
sit down to the environment exactly as I last left it.


You have several options, but it depends on untold expectations you
have beyond "exactly as I last left it."  So the following is only
to show directions and possibilities without going into details.

For example, do you save all changes you made in your editor buffers
before you leave the 'desktop'?  If the answer is "no", then that is
not a problem Git-the-tool is interested in solving, and the only
solution I could think of is to use mechanism to share the session
between 'desktop' and 'home', using things like remote-desktop or
screen session.

If the answer is "yes", the next question is if you commit all the
changes you made before you leave the 'desktop'.  If the answer is
"no", again, that is not a problem Git-the-tool is interested in
solving, and the only solution I could think of (in addition to the
"share the session" above) is to use networked filesystem shared
between 'desktop' and 'home'.

If the answer is "yes", then you are in the problem space that
Git-the-tool is interested in solving.  Assuming that you have
network connection into 'desktop' from 'home', the solution would
involve making it the first thing to do when get home to run "git
fetch" on 'home' to get the latest state from the 'desktop', and run
"git push" on 'home' to push out the latest state to the 'desktop'
before you leave 'home'.  If your 'server' is for your sole use, and
if 'home' has network connection into 'server', then you could
instead rendezvous at 'server' by running "git push server" on
'desktop' (or 'home') to 'server' as the last thing before you leave
'desktop' (or 'home'), and running "git fetch server" on 'home' (or
'desktop') as the first thing before you start working on 'home' (or
'desktop').



I've done a poor job of expressing my need.  I'm only
speaking about the state of each git project directory.
Shared sessions or filesystems are not what I'm loolking
for.

As far as the master branch goes, I have no problem syncing
everything to the server, then pulling to a different
machine.  When I said I want everything to be the same
between the home and work machines, I meant with regard to
all of the other branches.  I want to easily create an
entire movable git environment that is indistinguishable
from machine to machine.

It looks like --mirror is close to what I am looking for.

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: Working from different machines.

[Tobiah]

* First, `--mirror` might be what the relation between 'desktop'
  and 'home' repositories should be.




Here's what I'm trying, am I in the right ballpark?

desk> git branch
* master
desk> git checkout -b banana
Switched to a new branch 'banana'
desk> echo 'message' > oracle
desk> git add oracle
desk> git commit -a -moracle
[banana 66e7823] oracle
 1 file changed, 1 insertion(+)
 create mode 100644 oracle
desk> git push --mirror
Counting objects: 3, done.
Delta compression using up to 2 threads.
Compressing objects: 100% (2/2), done.
Writing objects: 100% (3/3), 260 bytes | 0 bytes/s, done.
Total 3 (delta 1), reused 0 (delta 0)
To t...@shells.rcsreg.com:/home/git/toby/exposmart
 * [new branch]  banana -> banana

# Then on another machine:

home> git branch
* master
## Forgot to 'pull' here, but it never shows new branches anyway
## I have to get them by name.  Can I automate that?

home> git checkout banana
Branch banana set up to track remote branch banana from origin.
Switched to a new branch 'banana'
home> cat oracle
message
home> echo 'other message' >> oracle
home> git commit -a -moracle2
[banana 78e6c45] oracle2
 1 file changed, 1 insertion(+)
home> git push --mirror
Counting objects: 36, done.
Delta compression using up to 2 threads.
Compressing objects: 100% (22/22), done.
Writing objects: 100% (36/36), 2.90 KiB | 0 bytes/s, done.
Total 36 (delta 13), reused 14 (delta 4)
To shells.rcsreg.com:/home/git/toby/exposmart
   66e7823..78e6c45  banana -> banana
   7e01bf8..c65ed75  origin/HEAD -> origin/HEAD
 * [new branch]  origin/auto -> origin/auto
 * [new branch]  origin/banana -> origin/banana
 * [new branch]  origin/develop -> origin/develop
 * [new branch]  origin/flipper -> origin/flipper
 * [new branch]  origin/tart -> origin/tart
home> vi
home> git push --mirror
Total 0 (delta 0), reused 0 (delta 0)
To shells.rcsreg.com:/home/git/toby/exposmart
   66e7823..78e6c45  origin/banana -> origin/banana
home> git push --mirror
Everything up-to-date
home>

## Why do I have to push three times to get the 'Everyting up-to-date'
## message?  Why does it still mention 'flipper' and 'tart' and all
## other branches I've played with.  When I delte them locally, I want
## them to be deleted everywhere.  They just keep accumulating.
##
## Now back a the other machine


desk> git pull
remote: Counting objects: 5, done.
remote: Compressing objects: 100% (2/2), done.
remote: Total 3 (delta 1), reused 0 (delta 0)
Unpacking objects: 100% (3/3), done.
From shells.rcsreg.com:/home/git/toby/exposmart
   66e7823..78e6c45  banana -> origin/banana
There is no tracking information for the current branch.
Please specify which branch you want to merge with.
See git-pull(1) for details.

git pull  

If you wish to set tracking information for this branch you can do so with:

git branch --set-upstream-to=origin/ banana

desk> git branch
* banana
  master
desk> git pull origin banana
From shells.rcsreg.com:/home/git/toby/exposmart
 * branchbanana -> FETCH_HEAD
Updating 66e7823..78e6c45
Fast-forward
 oracle | 1 +
 1 file changed, 1 insertion(+)
desk> cat oracle
message
other message


## So it seems to have worked, but the more I go back and forth
## The more I seem to have eventual problems getting branches
## and changes.  Am I doing it right?


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 v5 15/15] read-cache: make sure file handles are not inherited by child processes

[Johannes Schindelin]

Hi Lars,

On Wed, 10 Aug 2016, larsxschnei...@gmail.com wrote:

> From: Lars Schneider 
> 
> Consider the case of a file that requires filtering and is present in branch A
> but not in branch B. If A is the current HEAD and we checkout B then the
> following happens:
> 
> 1. ce_compare_data() opens the file
> 2.   index_fd() detects that the file requires to run a clean filter and
>  calls index_stream_convert_blob()
> 4. index_stream_convert_blob() calls convert_to_git_filter_fd()
> 5.   convert_to_git_filter_fd() calls apply_filter() which creates a new
>  long running filter process (in case it is the first file of this 
> kind
>  to be filtered)
> 6.   The new filter process inherits all file handles. This is the default
>  on Linux/OSX and is explicitly defined in the `CreateProcessW` call
>  in `mingw.c` on Windows.
> 7. ce_compare_data() closes the file
> 8. Git unlinks the file as it is not present in B
> 
> The unlink operation does not work on Windows because the filter process has
> still an open handle to the file. Apparently that is no problem on Linux/OSX.
> Probably because "[...] the two file descriptors share open file status flags"
> (see fork(2)).

We typically wrap the commit messages at 76 columns per row (personally,
I wrap already at 72, and it seems Junio wraps at 70).

> Fix this problem by opening files in read-cache with the `O_CLOEXEC` flag to
> ensure that the file descriptor does not remain open in a newly spawned 
> process.
> `O_CLOEXEX` is defined as `O_NOINHERIT` on Windows. A similar fix for 
> temporary

s/CLOEXEX/CLOEXEC/

> file handles was applied on Git for Windows already:
> https://github.com/git-for-windows/git/commit/667b8b51ec850c3e1c7d75dee69dc13c29d1f162

In response to your commit note on GitHub, I submitted this patch series
(slightly cleaned up) yesterday (and you already commented on it):

https://public-inbox.org/git/cover.1471437637.git.johannes.schinde...@gmx.de

The patch is obviously correct, and needs the patch series I submitted to
compile on Windows (this note is more for Junio's interest than a comment
on this patch).

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: git cherry-pick conflict error message is deceptive when cherry-picking multiple commits

[Johannes Schindelin]

Hi Stephen,

On Wed, 17 Aug 2016, Stephen Morton wrote:

> > multiple_commits = (todo_list->next) != NULL;
> > Why not "last_commit" instead of "multiple_commits"?
> 
> Because it *isn't*.

Personally, I do prefer to have the simpler instruction if the commit
happens to be the last one cherry-picked.

I do not follow your argument that it makes sense to decide based on the
total number of commits that were to be cherry-picked whether to use the
more cumbersome or the easier command.

Instead, I think the user should be given the simplest advice that gets
them out of the fix most quickly. And if `git commit --amend` is good
enough, we should not advise to use `git cherry-pick --continue`, and that
is that.

Ciao,
Johannes
--
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 3/3] receive-pack: allow a maximum input size to be specified

[Christian Couder]

On Tue, Aug 16, 2016 at 6:21 PM, Jeff King  wrote:
>> >
>> > Is there any reason to use different filenames and sizes for the two
>> > runs? They should behave identically, so it would make more sense to me
>> > to subject them to identical inputs.
>>
>> About the sizes, I thought that some people might want to try sizes
>> closer to the limit and also that it is good anyway in general to add
>> a bit of "randomness", or at least variability, in the tests.
>
> In general, I'd prefer a systematic approach to introducing variables
> into tests. If it's important that we test different sizes, then we
> should do so, and not only test some combinations (and if it is not
> important to test different sizes, then we should not waste CPU time and
> the mental energy of people reading the tests).
>
> IOW, when I see this, I wonder why the index-pack code path is not
> tested against a 2k file. But there really isn't a good reason. So
> either it does matter, and our tests do not have good coverage, or it
> does not, and it is just making me wonder if the tests are buggy.

I don't see things in the same way, but I will make the second file a
1k file too as I don't really care much about that.

> Worse, both files are created with the same seed via test-genrandom. So
> I suspect the 2k file is a superset of the 1k file, and we may send it
> is a thin delta (so it really is only testing a 1k input anyway!).

Yeah, I will use a different seed for the second file.

>> I thought that it would be a bit less wasteful to use the same "dest"
>> and also it would make the test more realistic as people often push in
>> non empty repos.
>
> I doubt it is expensive enough to matter in practice. Though note that
> if you push the same commit to two new repositories, then you can
> amortize the test-genrandom/add/commit step (i.e., push the exact same
> packfile in both cases).

Yeah, it probably doesn't matter anyway regarding efficiency, but I
still prefer to not create a new repo as I would find it more
confusing for the reader to use different repos.

Thanks,
Christian.
--
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 0/3] limit the size of the packs we receive

[Jeff King]

On Thu, Aug 18, 2016 at 03:15:50PM +0200, Christian Couder wrote:

> Diff with previous v1 version
> ~
> 
> diff --git a/t/t5546-push-limits.sh b/t/t5546-push-limits.sh
> index b38d508..09e958f 100755
> --- a/t/t5546-push-limits.sh
> +++ b/t/t5546-push-limits.sh
> @@ -11,11 +11,11 @@ test_expect_success 'create remote repository' '
>  # When the limit is 1, `git receive-pack` will call `git index-pack`.
>  # When the limit is 10, `git receive-pack` will call `git unpack-objects`.
>  
> -while read unpacklimit filesize filename
> +while read unpacklimit filesize filename seed
>  do
>  
>   test_expect_success "create known-size ($filesize bytes) commit 
> '$filename'" '
> - test-genrandom foo "$filesize" >"$filename" &&
> + test-genrandom "$seed" "$filesize" >"$filename" &&
>   git add "$filename" &&
>   test_commit "$filename"
>   '
> @@ -35,8 +35,8 @@ do
>   '
>  
>  done <<\EOF
> -1 1024 one-k-file
> -10 2048 two-k-file
> +1 1024 one-k-file foo
> +10 1024 other-one-k-file bar
>  EOF
>  
>  test_done

This is still not quite what I would have written, but I don't think
it's worth arguing over.

This version looks OK to me. Thanks.

-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 v2 1/3] index-pack: add --max-input-size= option

[Christian Couder]

From: Jeff King 

When receiving a pack-file, it can be useful to abort the
`git index-pack`, if the pack-file is too big.

Signed-off-by: Jeff King 
Signed-off-by: Christian Couder 
---
 Documentation/git-index-pack.txt | 2 ++
 builtin/index-pack.c | 5 +
 2 files changed, 7 insertions(+)

diff --git a/Documentation/git-index-pack.txt b/Documentation/git-index-pack.txt
index 7a4e055..1b4b65d 100644
--- a/Documentation/git-index-pack.txt
+++ b/Documentation/git-index-pack.txt
@@ -87,6 +87,8 @@ OPTIONS
Specifying 0 will cause Git to auto-detect the number of CPU's
and use maximum 3 threads.
 
+--max-input-size=::
+   Die, if the pack is larger than .
 
 Note
 
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 1d2ea58..4a8b4ae 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -87,6 +87,7 @@ static struct progress *progress;
 static unsigned char input_buffer[4096];
 static unsigned int input_offset, input_len;
 static off_t consumed_bytes;
+static off_t max_input_size;
 static unsigned deepest_delta;
 static git_SHA_CTX input_ctx;
 static uint32_t input_crc32;
@@ -297,6 +298,8 @@ static void use(int bytes)
if (signed_add_overflows(consumed_bytes, bytes))
die(_("pack too large for current definition of off_t"));
consumed_bytes += bytes;
+   if (max_input_size && consumed_bytes > max_input_size)
+   die(_("pack exceeds maximum allowed size"));
 }
 
 static const char *open_pack_file(const char *pack_name)
@@ -1714,6 +1717,8 @@ int cmd_index_pack(int argc, const char **argv, const 
char *prefix)
opts.off32_limit = strtoul(c+1, , 0);
if (*c || opts.off32_limit & 0x8000)
die(_("bad %s"), arg);
+   } else if (skip_prefix(arg, "--max-input-size=", )) 
{
+   max_input_size = strtoumax(arg, NULL, 10);
} else
usage(index_pack_usage);
continue;
-- 
2.10.0.rc0.3.geb1f4c9.dirty

--
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 v2 2/3] unpack-objects: add --max-input-size= option

[Christian Couder]

When receiving a pack-file, it can be useful to abort the
`git unpack-objects`, if the pack-file is too big.

Signed-off-by: Christian Couder 
---
 Documentation/git-unpack-objects.txt | 3 +++
 builtin/unpack-objects.c | 7 +++
 2 files changed, 10 insertions(+)

diff --git a/Documentation/git-unpack-objects.txt 
b/Documentation/git-unpack-objects.txt
index 3e887d1..b3de50d 100644
--- a/Documentation/git-unpack-objects.txt
+++ b/Documentation/git-unpack-objects.txt
@@ -44,6 +44,9 @@ OPTIONS
 --strict::
Don't write objects with broken content or links.
 
+--max-input-size=::
+   Die, if the pack is larger than .
+
 GIT
 ---
 Part of the linkgit:git[1] suite
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index 172470b..4532aa0 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -19,6 +19,7 @@ static const char unpack_usage[] = "git unpack-objects [-n] 
[-q] [-r] [--strict]
 static unsigned char buffer[4096];
 static unsigned int offset, len;
 static off_t consumed_bytes;
+static off_t max_input_size;
 static git_SHA_CTX ctx;
 static struct fsck_options fsck_options = FSCK_OPTIONS_STRICT;
 
@@ -87,6 +88,8 @@ static void use(int bytes)
if (signed_add_overflows(consumed_bytes, bytes))
die("pack too large for current definition of off_t");
consumed_bytes += bytes;
+   if (max_input_size && consumed_bytes > max_input_size)
+   die(_("pack exceeds maximum allowed size"));
 }
 
 static void *get_data(unsigned long size)
@@ -550,6 +553,10 @@ int cmd_unpack_objects(int argc, const char **argv, const 
char *prefix)
len = sizeof(*hdr);
continue;
}
+   if (skip_prefix(arg, "--max-input-size=", )) {
+   max_input_size = strtoumax(arg, NULL, 10);
+   continue;
+   }
usage(unpack_usage);
}
 
-- 
2.10.0.rc0.3.geb1f4c9.dirty

--
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 v2 3/3] receive-pack: allow a maximum input size to be specified

[Christian Couder]

From: Jeff King 

Receive-pack feeds its input to either index-pack or
unpack-objects, which will happily accept as many bytes as
a sender is willing to provide. Let's allow an arbitrary
cutoff point where we will stop writing bytes to disk.

Cleaning up what has already been written to disk is a
related problem that is not addressed by this patch.

Signed-off-by: Jeff King 
Signed-off-by: Christian Couder 
---
 Documentation/config.txt   |  5 +
 Documentation/git-receive-pack.txt |  3 +++
 builtin/receive-pack.c | 12 +++
 t/t5546-push-limits.sh | 42 ++
 4 files changed, 62 insertions(+)
 create mode 100755 t/t5546-push-limits.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 0bcb679..f5b6061 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2517,6 +2517,11 @@ receive.unpackLimit::
especially on slow filesystems.  If not set, the value of
`transfer.unpackLimit` is used instead.
 
+receive.maxsize::
+   If the size of a pack file is larger than this limit, then
+   git-receive-pack will error out, instead of accepting the pack
+   file. If not set or set to 0, then the size is unlimited.
+
 receive.denyDeletes::
If set to true, git-receive-pack will deny a ref update that deletes
the ref. Use this to prevent such a ref deletion via a push.
diff --git a/Documentation/git-receive-pack.txt 
b/Documentation/git-receive-pack.txt
index 000ee8d..0ccd5fb 100644
--- a/Documentation/git-receive-pack.txt
+++ b/Documentation/git-receive-pack.txt
@@ -33,6 +33,9 @@ post-update hooks found in the Documentation/howto directory.
 option, which tells it if updates to a ref should be denied if they
 are not fast-forwards.
 
+A number of other receive.* config options are available to tweak
+its behavior, see linkgit:git-config[1].
+
 OPTIONS
 ---
 ::
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 92e1213..8c2943d 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -46,6 +46,7 @@ static int transfer_unpack_limit = -1;
 static int advertise_atomic_push = 1;
 static int advertise_push_options;
 static int unpack_limit = 100;
+static off_t max_input_size;
 static int report_status;
 static int use_sideband;
 static int use_atomic;
@@ -212,6 +213,11 @@ static int receive_pack_config(const char *var, const char 
*value, void *cb)
return 0;
}
 
+   if (strcmp(var, "receive.maxsize") == 0) {
+   max_input_size = git_config_int64(var, value);
+   return 0;
+   }
+
return git_default_config(var, value, cb);
 }
 
@@ -1650,6 +1656,9 @@ static const char *unpack(int err_fd, struct shallow_info 
*si)
if (fsck_objects)
argv_array_pushf(, "--strict%s",
fsck_msg_types.buf);
+   if (max_input_size)
+   argv_array_pushf(, 
"--max-input-size=%"PRIuMAX,
+   (uintmax_t)max_input_size);
child.no_stdout = 1;
child.err = err_fd;
child.git_cmd = 1;
@@ -1678,6 +1687,9 @@ static const char *unpack(int err_fd, struct shallow_info 
*si)
fsck_msg_types.buf);
if (!reject_thin)
argv_array_push(, "--fix-thin");
+   if (max_input_size)
+   argv_array_pushf(, 
"--max-input-size=%"PRIuMAX,
+   (uintmax_t)max_input_size);
child.out = -1;
child.err = err_fd;
child.git_cmd = 1;
diff --git a/t/t5546-push-limits.sh b/t/t5546-push-limits.sh
new file mode 100755
index 000..09e958f
--- /dev/null
+++ b/t/t5546-push-limits.sh
@@ -0,0 +1,42 @@
+#!/bin/sh
+
+test_description='check input limits for pushing'
+. ./test-lib.sh
+
+test_expect_success 'create remote repository' '
+   git init --bare dest
+'
+
+# Let's run tests with different unpack limits: 1 and 10
+# When the limit is 1, `git receive-pack` will call `git index-pack`.
+# When the limit is 10, `git receive-pack` will call `git unpack-objects`.
+
+while read unpacklimit filesize filename seed
+do
+
+   test_expect_success "create known-size ($filesize bytes) commit 
'$filename'" '
+   test-genrandom "$seed" "$filesize" >"$filename" &&
+   git add "$filename" &&
+   test_commit "$filename"
+   '
+
+   test_expect_success "set unpacklimit to $unpacklimit" '
+   git --git-dir=dest config receive.unpacklimit "$unpacklimit"
+   '
+
+   test_expect_success 'setting receive.maxsize to 512 rejects push' '
+   git --git-dir=dest config receive.maxsize 512 &&
+   test_must_fail git push dest HEAD
+   '
+
+   test_expect_success 'bumping 

[PATCH v2 0/3] limit the size of the packs we receive

[Christian Couder]

Goal


In https://public-inbox.org/git/20150612182045.GA23698%40peff.net/,
Peff sent a patch that is used by GitHub to abort `git receive-pack`
when the size of the pack we receive is bigger than a configured
limit.

GitLab is interested in using the same approach and in standardizing
the error messages the user could get back.

Comments


I kept Peff as the author of the patches that are made mostly from his
patch, but I added my Signed-off-by to them.

Changes from previous v1 version


  - removed last sentences of the commit message in patch 3/3, as
suggested by Peff,

  - improved the tests in the last patch, as suggested by Peff

Links
~

This patch series is available here:

https://github.com/chriscool/git/commits/max-receive

The previous versions are here on GitHub:

RFC: https://github.com/chriscool/git/commits/max-receive2
v1: https://github.com/chriscool/git/commits/max-receive6

and here on the list:

RFC: 
https://public-inbox.org/git/20160815195729.16826-1-chrisc...@tuxfamily.org/
v1: https://public-inbox.org/git/20160816081701.29949-1-chrisc...@tuxfamily.org/

Peff's initial patch is:

https://public-inbox.org/git/20150612182045.GA23698%40peff.net/

Diff with previous v1 version
~

diff --git a/t/t5546-push-limits.sh b/t/t5546-push-limits.sh
index b38d508..09e958f 100755
--- a/t/t5546-push-limits.sh
+++ b/t/t5546-push-limits.sh
@@ -11,11 +11,11 @@ test_expect_success 'create remote repository' '
 # When the limit is 1, `git receive-pack` will call `git index-pack`.
 # When the limit is 10, `git receive-pack` will call `git unpack-objects`.
 
-while read unpacklimit filesize filename
+while read unpacklimit filesize filename seed
 do
 
test_expect_success "create known-size ($filesize bytes) commit 
'$filename'" '
-   test-genrandom foo "$filesize" >"$filename" &&
+   test-genrandom "$seed" "$filesize" >"$filename" &&
git add "$filename" &&
test_commit "$filename"
'
@@ -35,8 +35,8 @@ do
'
 
 done <<\EOF
-1 1024 one-k-file
-10 2048 two-k-file
+1 1024 one-k-file foo
+10 1024 other-one-k-file bar
 EOF
 
 test_done
---

Christian Couder (1):
  unpack-objects: add --max-input-size= option

Jeff King (2):
  index-pack: add --max-input-size= option
  receive-pack: allow a maximum input size to be specified

 Documentation/config.txt |  5 +
 Documentation/git-index-pack.txt |  2 ++
 Documentation/git-receive-pack.txt   |  3 +++
 Documentation/git-unpack-objects.txt |  3 +++
 builtin/index-pack.c |  5 +
 builtin/receive-pack.c   | 12 +++
 builtin/unpack-objects.c |  7 ++
 t/t5546-push-limits.sh   | 42 
 8 files changed, 79 insertions(+)
 create mode 100755 t/t5546-push-limits.sh

-- 
2.10.0.rc0.3.geb1f4c9.dirty

--
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 0/4] cat-file: optionally convert to worktree version

[Johannes Schindelin]

When third-party tools need to access to contents of blobs in the
database, they might be more interested in the worktree version than in
the "clean" version of said contents.

This branch introduces the --filters option to make that happen, the
--use-path option to provide the path separately if the blob name rather
than the tree name is availale, and offers batch support in which case
it expects the object names and the path on its input lines, separated
by white space.

The new --filters option is an obvious sibling of --textconv, and shares
the peculiar feature that the drivers (and end-of-line convention) are
determined from the current worktree, not from the attributes stored in
the revision that may have been part of the object name.

As --textconv is so similar to --filters, it was taught to understand
the --use-path option and it was made compatible with batch mode, too.

I briefly considered teaching the batch mode to extract the path from
object names if they are specified as :. The changes
would be quite intrusive, though, and uglify the code substanitially. So
I decided against that.


Johannes Schindelin (4):
  cat-file: fix a grammo in the man page
  cat-file: introduce the --filters option
  cat-file --textconv/--filters: allow specifying the path separately
  cat-file: support --textconv/--filters in batch mode

 Documentation/git-cat-file.txt |  40 
 builtin/cat-file.c | 106 +
 t/t8010-cat-file-filters.sh|  57 ++
 3 files changed, 185 insertions(+), 18 deletions(-)
 create mode 100755 t/t8010-cat-file-filters.sh

Published-As: https://github.com/dscho/git/releases/tag/cat-file-filters-v1
Fetch-It-Via: git fetch https://github.com/dscho/git cat-file-filters-v1

-- 
2.9.2.691.g78954f3

base-commit: d63263a4dee8fc7da9b97bbdedf9c0d1f33024d4
--
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/4] cat-file: fix a grammo in the man page

[Johannes Schindelin]

"... has be ..." -> "... has to be ..."

Signed-off-by: Johannes Schindelin 
---
 Documentation/git-cat-file.txt | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
index 18d03d8..071029b 100644
--- a/Documentation/git-cat-file.txt
+++ b/Documentation/git-cat-file.txt
@@ -54,8 +54,9 @@ OPTIONS
 
 --textconv::
Show the content as transformed by a textconv filter. In this case,
-has be of the form :, or : in order
-   to apply the filter to the content recorded in the index at .
+has to be of the form :, or : in
+   order to apply the filter to the content recorded in the index at
+   .
 
 --batch::
 --batch=::
-- 
2.9.2.691.g78954f3


--
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: Working from different machines.

[Jakub Narębski]

W dniu 18.08.2016 o 00:41, Junio C Hamano pisze:
> Tobiah  writes:

> [...] the next question is if you commit all the
> changes you made before you leave the 'desktop'. [...]
> 
> If the answer is "yes", then you are in the problem space that
> Git-the-tool is interested in solving.  Assuming that you have
> network connection into 'desktop' from 'home', the solution would
> involve making it the first thing to do when get home to run "git
> fetch" on 'home' to get the latest state from the 'desktop', and run
> "git push" on 'home' to push out the latest state to the 'desktop'
> before you leave 'home'.  If your 'server' is for your sole use, and
> if 'home' has network connection into 'server', then you could
> instead rendezvous at 'server' by running "git push server" on
> 'desktop' (or 'home') to 'server' as the last thing before you leave
> 'desktop' (or 'home'), and running "git fetch server" on 'home' (or
> 'desktop') as the first thing before you start working on 'home' (or
> 'desktop').

Two additional comments:

* First, `--mirror` might be what the relation between 'desktop'
  and 'home' repositories should be.

* Second, even if you can connect only from 'home' to 'desktop',
  but not from 'desktop' to 'home', the refspec mechanism of Git
  is flexible enough that you can emulate 'push' with 
  appropriately configured 'fetch', and vice versa.

-- 
Jakub Narębski

--
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 4/4] cat-file: support --textconv/--filters in batch mode

[Johannes Schindelin]

With this patch, --batch can be combined with --textconv or --filters.
For this to work, the input needs to have the form



so that the filters can be chosen appropriately.

Signed-off-by: Johannes Schindelin 
---
 Documentation/git-cat-file.txt | 18 +++-
 builtin/cat-file.c | 49 +-
 t/t8010-cat-file-filters.sh| 10 +
 3 files changed, 67 insertions(+), 10 deletions(-)

diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
index 59a3c37..1f4d954 100644
--- a/Documentation/git-cat-file.txt
+++ b/Documentation/git-cat-file.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 
 [verse]
 'git cat-file' (-t [--allow-unknown-type]| -s [--allow-unknown-type]| -e | -p 
|  | --textconv | --filters ) [--use-path=] 
-'git cat-file' (--batch | --batch-check) [--follow-symlinks]
+'git cat-file' (--batch | --batch-check) [ --textconv | --filters ] 
[--follow-symlinks]
 
 DESCRIPTION
 ---
@@ -20,7 +20,11 @@ object type, or `-s` is used to find the object size, or 
`--textconv` or
 `--filters` is used (which imply type "blob").
 
 In the second form, a list of objects (separated by linefeeds) is provided on
-stdin, and the SHA-1, type, and size of each object is printed on stdout.
+stdin, and the SHA-1, type, and size of each object is printed on stdout. The
+output format can be overridden using the optional `` argument. If
+either `--textconv` or `--filters` was specified, the input is expected to
+list the object names followed by the path name, separated by a single white
+space, so that the appropriate drivers can be determined.
 
 OPTIONS
 ---
@@ -72,13 +76,17 @@ OPTIONS
 --batch::
 --batch=::
Print object information and contents for each object provided
-   on stdin.  May not be combined with any other options or arguments.
-   See the section `BATCH OUTPUT` below for details.
+   on stdin.  May not be combined with any other options or arguments
+   except `--textconv` or `--filters`, in which case the input lines
+   also need to specify the path, separated by white space.  See the
+   section `BATCH OUTPUT` below for details.
 
 --batch-check::
 --batch-check=::
Print object information for each object provided on stdin.  May
-   not be combined with any other options or arguments.  See the
+   not be combined with any other options or arguments except
+   `--textconv` or `--filters`, in which case the input lines also
+   need to specify the path, separated by white space.  See the
section `BATCH OUTPUT` below for details.
 
 --batch-all-objects::
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 5ff58b3..5f91cf4 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -17,6 +17,7 @@ struct batch_options {
int print_contents;
int buffer_output;
int all_objects;
+   int cmdmode; /* may be 'w' or 'c' for --filters or --textconv */
const char *format;
 };
 
@@ -281,7 +282,32 @@ static void print_object_or_die(struct batch_options *opt, 
struct expand_data *d
if (data->type == OBJ_BLOB) {
if (opt->buffer_output)
fflush(stdout);
-   if (stream_blob_to_fd(1, sha1, NULL, 0) < 0)
+   if (opt->cmdmode) {
+   char *contents;
+   unsigned long size;
+
+   if (!data->rest)
+   die("missing path for '%s'", sha1_to_hex(sha1));
+
+   if (opt->cmdmode == 'w') {
+   if (filter_object(data->rest, 0100644, sha1,
+ , ))
+   die("could not convert '%s' %s",
+   sha1_to_hex(sha1), data->rest);
+   } else if (opt->cmdmode == 'c') {
+   enum object_type type;
+   if (!textconv_object(data->rest, 0100644, sha1,
+1, , ))
+   contents = read_sha1_file(sha1, ,
+ );
+   if (!contents)
+   die("could not convert '%s' %s",
+   sha1_to_hex(sha1), data->rest);
+   } else
+   die("BUG: invalid cmdmode: %c", opt->cmdmode);
+   batch_write(opt, contents, size);
+   free(contents);
+   } else if (stream_blob_to_fd(1, sha1, NULL, 0) < 0)
die("unable to stream %s to stdout", sha1_to_hex(sha1));
}
else {
@@ -418,6 +444,8 @@ static int batch_objects(struct batch_options *opt)
data.mark_query = 1;

[PATCH 2/4] cat-file: introduce the --filters option

[Johannes Schindelin]

As suggested by its name, the --filters option applies the filters
that are currently configured for the specified path.

This feature comes in handy when a 3rd-party tool wants to work with
the contents of files from past revisions as if they had been checked
out, but without detouring via temporary files.

Note that we ensure that symbolic links are unaffected (we know from
looking at the mode whether it refers to a symbolic link or a regular
file).

Signed-off-by: Johannes Schindelin 
---
 Documentation/git-cat-file.txt | 12 +---
 builtin/cat-file.c | 41 -
 t/t8010-cat-file-filters.sh| 34 ++
 3 files changed, 83 insertions(+), 4 deletions(-)
 create mode 100755 t/t8010-cat-file-filters.sh

diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
index 071029b..7d48735 100644
--- a/Documentation/git-cat-file.txt
+++ b/Documentation/git-cat-file.txt
@@ -9,15 +9,15 @@ git-cat-file - Provide content or type and size information 
for repository objec
 SYNOPSIS
 
 [verse]
-'git cat-file' (-t [--allow-unknown-type]| -s [--allow-unknown-type]| -e | -p 
|  | --textconv ) 
+'git cat-file' (-t [--allow-unknown-type]| -s [--allow-unknown-type]| -e | -p 
|  | --textconv | --filters ) 
 'git cat-file' (--batch | --batch-check) [--follow-symlinks]
 
 DESCRIPTION
 ---
 In its first form, the command provides the content or the type of an object in
 the repository. The type is required unless `-t` or `-p` is used to find the
-object type, or `-s` is used to find the object size, or `--textconv` is used
-(which implies type "blob").
+object type, or `-s` is used to find the object size, or `--textconv` or
+`--filters` is used (which imply type "blob").
 
 In the second form, a list of objects (separated by linefeeds) is provided on
 stdin, and the SHA-1, type, and size of each object is printed on stdout.
@@ -58,6 +58,12 @@ OPTIONS
order to apply the filter to the content recorded in the index at
.
 
+--filters::
+   Show the content as transformed by the filters configured in
+   the current working tree for the given  (i.e. smudge filters,
+   end-of-line conversion, etc). In this case,  has to be of
+   the form :, or :.
+
 --batch::
 --batch=::
Print object information and contents for each object provided
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 2dfe626..0b74afa 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -20,6 +20,33 @@ struct batch_options {
const char *format;
 };
 
+static int filter_object(const char *path, unsigned mode,
+const unsigned char *sha1,
+char **buf, unsigned long *size)
+{
+   enum object_type type;
+
+   *buf = read_sha1_file(sha1, , size);
+   if (!*buf)
+   return error(_("cannot read object %s '%s'"),
+   sha1_to_hex(sha1), path);
+   if (type != OBJ_BLOB) {
+   free(*buf);
+   return error(_("blob expected for %s '%s'"),
+   sha1_to_hex(sha1), path);
+   }
+   if (S_ISREG(mode)) {
+   struct strbuf strbuf = STRBUF_INIT;
+   if (convert_to_working_tree(path, *buf, *size, )) {
+   free(*buf);
+   *size = strbuf.len;
+   *buf = strbuf_detach(, NULL);
+   }
+   }
+
+   return 0;
+}
+
 static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
int unknown_type)
 {
@@ -61,6 +88,16 @@ static int cat_one_file(int opt, const char *exp_type, const 
char *obj_name,
case 'e':
return !has_sha1_file(sha1);
 
+   case 'w':
+   if (!obj_context.path[0])
+   die("git cat-file --filters %s:  must be "
+   "", obj_name);
+
+   if (filter_object(obj_context.path, obj_context.mode,
+ sha1, , ))
+   return -1;
+   break;
+
case 'c':
if (!obj_context.path[0])
die("git cat-file --textconv %s:  must be 
",
@@ -440,7 +477,7 @@ static int batch_objects(struct batch_options *opt)
 }
 
 static const char * const cat_file_usage[] = {
-   N_("git cat-file (-t [--allow-unknown-type]|-s 
[--allow-unknown-type]|-e|-p||--textconv) "),
+   N_("git cat-file (-t [--allow-unknown-type]|-s 
[--allow-unknown-type]|-e|-p||--textconv|--filters) "),
N_("git cat-file (--batch | --batch-check) [--follow-symlinks]"),
NULL
 };
@@ -486,6 +523,8 @@ int cmd_cat_file(int argc, const char **argv, const char 
*prefix)
OPT_CMDMODE('p', NULL, , N_("pretty-print object's 
content"), 'p'),
OPT_CMDMODE(0, "textconv", ,
N_("for blob 

[PATCH 3/4] cat-file --textconv/--filters: allow specifying the path separately

[Johannes Schindelin]

There are circumstances when it is relatively easy to figure out the
object name for a given path, but not the revision. For example, when
looking at a diff generated by Git, the object names are recorded, but
not the revision. As a matter of fact, the revisions from which the diff
was generated may not even exist locally.

In such a case, the user would have to generate a fake revision just to
be able to use --textconv or --filters.

Let's simplify this dramatically, because we do not really need that
revision at all: all we care about is that we know the path. In the
scenario described above, we do know the path, and we just want to
specify it separately from the object name.

Example usage:

git cat-file --textconv --use-path=main.c 0f1937fd

Signed-off-by: Johannes Schindelin 
---
 Documentation/git-cat-file.txt |  7 ++-
 builtin/cat-file.c | 22 +-
 t/t8010-cat-file-filters.sh| 13 +
 3 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
index 7d48735..59a3c37 100644
--- a/Documentation/git-cat-file.txt
+++ b/Documentation/git-cat-file.txt
@@ -9,7 +9,7 @@ git-cat-file - Provide content or type and size information for 
repository objec
 SYNOPSIS
 
 [verse]
-'git cat-file' (-t [--allow-unknown-type]| -s [--allow-unknown-type]| -e | -p 
|  | --textconv | --filters ) 
+'git cat-file' (-t [--allow-unknown-type]| -s [--allow-unknown-type]| -e | -p 
|  | --textconv | --filters ) [--use-path=] 
 'git cat-file' (--batch | --batch-check) [--follow-symlinks]
 
 DESCRIPTION
@@ -64,6 +64,11 @@ OPTIONS
end-of-line conversion, etc). In this case,  has to be of
the form :, or :.
 
+--use-path=::
+   For use with --textconv or --filters, to allow specifying an object
+   name and a path separately, e.g. when it is difficult to figure out
+   the revision from which the blob came.
+
 --batch::
 --batch=::
Print object information and contents for each object provided
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 0b74afa..5ff58b3 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -20,6 +20,8 @@ struct batch_options {
const char *format;
 };
 
+static const char *force_path;
+
 static int filter_object(const char *path, unsigned mode,
 const unsigned char *sha1,
 char **buf, unsigned long *size)
@@ -89,21 +91,24 @@ static int cat_one_file(int opt, const char *exp_type, 
const char *obj_name,
return !has_sha1_file(sha1);
 
case 'w':
-   if (!obj_context.path[0])
+   if (!force_path && !obj_context.path[0])
die("git cat-file --filters %s:  must be "
"", obj_name);
 
-   if (filter_object(obj_context.path, obj_context.mode,
+   if (filter_object(force_path ? force_path : obj_context.path,
+ force_path ? 0100644 : obj_context.mode,
  sha1, , ))
return -1;
break;
 
case 'c':
-   if (!obj_context.path[0])
+   if (!force_path && !obj_context.path[0])
die("git cat-file --textconv %s:  must be 
",
obj_name);
 
-   if (textconv_object(obj_context.path, obj_context.mode, sha1, 
1, , ))
+   if (textconv_object(force_path ? force_path : obj_context.path,
+   force_path ? 0100644 : obj_context.mode,
+   sha1, 1, , ))
break;
 
case 'p':
@@ -477,7 +482,7 @@ static int batch_objects(struct batch_options *opt)
 }
 
 static const char * const cat_file_usage[] = {
-   N_("git cat-file (-t [--allow-unknown-type]|-s 
[--allow-unknown-type]|-e|-p||--textconv|--filters) "),
+   N_("git cat-file (-t [--allow-unknown-type]|-s 
[--allow-unknown-type]|-e|-p||--textconv|--filters) [--use-path=] 
"),
N_("git cat-file (--batch | --batch-check) [--follow-symlinks]"),
NULL
 };
@@ -525,6 +530,8 @@ int cmd_cat_file(int argc, const char **argv, const char 
*prefix)
N_("for blob objects, run textconv on object's 
content"), 'c'),
OPT_CMDMODE(0, "filters", ,
N_("for blob objects, run filters on object's 
content"), 'w'),
+   OPT_STRING(0, "use-path", _path, N_("blob"),
+  N_("use a specific path for --textconv/--filters")),
OPT_BOOL(0, "allow-unknown-type", _type,
  N_("allow -s and -t to work with broken/corrupt 
objects")),
OPT_BOOL(0, "buffer", _output, N_("buffer --batch 
output")),
@@ -567,6 +574,11 @@ int cmd_cat_file(int argc, const char **argv, const char 

Re: Working with public-inbox.org [Was: [PATCH] rev-parse: respect core.hooksPath in --git-path]

[Johannes Schindelin]

Hi Stefan,

On Tue, 16 Aug 2016, Stefan Beller wrote:

> > BTW in light of the discussion we are having elsewhere I just need to
> > point out that it was *dramatically* faster for me to edit run-command.c,
> > find "hooks/" and adjust the code manually than it would have been to save
> > the diff and apply it.
> >
> > That's because I do not have advanced tooling on top of email (and I also
> > could not handle mutt, so I am stuck with a not-really-scriptable email
> > client).
> >
> > Just sayin'.
> 
> I ran into the same problem, just for a larger patch, so I figured I can
> download that from the public inbox and git-am it locally.
> So I maneuvered to the cover letter of the patch series I am interested in[1]
> and downloaded the series as a mbox.gz[2].

Maybe you can adapt the script I had written to perform that magic for
gmane?

https://github.com/git-for-windows/build-extra/blob/master/apply-from-gmane.sh

The relevant part is the one between the lines 48--72, where it detects
0/N mails and then looks for the first children containing k/N for k=1..N.

BTW I take this thread as yet another proof that people are unhappy with
mail list-based review: if you have to build *that much* tooling around it
(and Peff & Junio certainly have a megaton of advanced and sophisticated
tooling around it, holy cow!) it is really incorrect to state that the
mail list-driven approach works for you. It is much closer to the truth to
say that the mail-list-plus-loads-of-custom-tools-driven approach works
for you.

I am really not a fan of this.

The theory "it's inclusive because everyone has access to mail" falls on
its face, badly, when even old timers have to build entire infrastructures
around it just to begin to be able to use it efficiently.

It reminds me of an old software developer I met long ago, who claimed CVS
works for him. He had written tens of thousands of lines of shell scripts,
is what allowed "CVS" to work for him.

Same here. Old dogs claim the mail list-approach works for them. Nope.
Doesn't. Else you would not have written all those custom scripts.

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: Working with zip files

[Jakub Narębski]

W dniu 16.08.2016 o 22:19, Junio C Hamano pisze:
> Jakub Narębski  writes:
> 
>> There is also `textconv` filter that can be used instead; it might
>> be 'unzip -c' (extract files to stdout, with filenames), or 'unzip -p'
>> (same, without filenames).
> 
> That assumes that the in-repository data is zipped binary blob; the
> result won't delta well, will it?

Full solution would involve `clean` filter to rezip with no compression
(which should delta well) and optional `smudge` filter to recompress;
if round-trip bit-for-bit equality is needed, the original zip parameters
must be saved somewhere, e.g. as ZIP archive comments.  This was mentioned
in the earlier part of my email (which might have been not clear):

JN>> You can find rezip clean/smudge filter (originally intended for
JN>> OpenDocument Format (ODF), that is OpenOffice.org etc.) that stores
JN>> zip or zip-archive (like ODT, jar, etc.) uncompressed.  I think
JN>> you can find it on GitWiki, but I might be mistaken.
 
Using 'unzip -c' as separate / additional `textconv` filter for diff
generation allows to separate the problem of deltifiable storage format
from textual representation for diff-ing.

Though best results could be had with `diff` and `merge` drivers...

-- 
Jakub Narębski
--
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: Draft of Git Rev News edition 18

[Eric Wong]

Philip Oakley  wrote:
> Would including a 'help' link on the main pages be possible? Maybe to the
> www-design page or whatever, just to make the users aware of the wider
> resourses, and a few key ways of entering search queries (such as the
> "gmane:123456" which gets that msg, while "gmane/123456" will get all
> messages that refered to that message (pure text search)..

Finally added https://public-inbox.org/git/_/text/help/

I actually totally overdid some things and trimmed it down before
publishing because I felt like it was becoming it's own wiki :x

  https://public-inbox.org/meta/20160818095150.17118-...@80x24.org/T/

Anyways, I'm still thinking there might be too much or too little
detail.  I don't know how well-known Atom feeds are, even.
--
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

Oxfam Aid Donation..

[Oxfam International Aid]

Congratulations! You e-mail has just won you the sum of $1,000,000.00 
USD as a charity donations/aid from Oxfam International in conjunction 
with South African National Lotto. Contact us back as soon as possible 
for more details.
--
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: [ANNOUNCE] Git for Windows 2.9.3

[Johannes Schindelin]

Hi Junio,

On Wed, 17 Aug 2016, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> >> And then your "git cat-file" patch can be upstreamed with the option
> >> renamed to (or with an additional synonym) "--filters", which would make
> >> things consistent.
> >
> > Right. I would like to ask for a `--smudge` synonym nevertheless, just
> > because I already use this. On the other hand, it is early enough to tell
> > everybody who knows about this feature to change their invocation (anybody
> > who would know about `--smudge` would be in that 1% of users that have
> > read the release notes, so most likely would read the next release notes,
> > too).
> 
> It is OK if it were your private edition, but you end up hurting
> your users if you need to redo the feature differently.

Unfortunately, this is the situation of Git for Windows from its
beginning: there has not been a single time that Git for Windows could
live with unpatched upstream Git's source code.

Business as usual, though.

> That's the price of your using open source and taking a short-cut.
> Adding a "--smudge" synonym is spreading the same hurt to outside
> your fork.  Let's see if we can avoid doing that.  Perhaps mark that
> "--smudge" as experimental-and-subject-to-change and re-announce?

I do not think so.

I have plenty of experience to deal with the problem caused by Git for
Windows requiring plenty of patches on top of your Git versions. I can
easily deal with this here problem, too.

FYI there have been two very strong reasons why I did not go through the
review on the Git mailing list for the --smudge option: 1) I really needed
this quick, and last time I needed something quick, it did not exactly
work out, now, did it, and 2) as far as I am concerned, the most important
part of developing patches is the practical testing, and this belief was
reinforced by the core.hideDotFiles feature that was well-tested and
well-exercised through years, only to be broken by changes necessitated by
the review on the Git mailing list: despite the best efforts of both you
and me, we managed to worsimprove the patches to a point where they may
look more elegant than before, but unfortunately are also less correct at
the same time.

So I learned my lesson: I will try better to get patches robust and stable
by exercising them and developing them as needed (the --smudge feature,
for example, turned out to be only half of what I need, I developed more
patches on that front), and I will be careful to avoid major modifications
of my patches just to get things upstream. It is better to carry correct
patches in Git for Windows than to upstream incorrect revisions of them.

Ideally, all of the patches I carry in Git for Windows would make it into
git.git eventually, of course. I fully support that. But not at the price
of breakages.

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: [PATCH v7 6/7] submodule: refactor show_submodule_summary with helper function

[Jacob Keller]

On Thu, Aug 18, 2016 at 12:00 AM, David Aguilar  wrote:
>> +void show_submodule_summary(FILE *f, const char *path,
>> + const char *line_prefix,
>> + unsigned char one[20], unsigned char two[20],
>> + unsigned dirty_submodule, const char *meta,
>> + const char *del, const char *add, const char *reset)
>
>
> ... and here too.
>
> There's an ongoing effort to replace unsigned char sha1[20]
> with struct object_id.  It might make sense to pass "one" and
> "two" as "unsigned char*" instead of hard-coding the SHA1-length
> here.  Alternatively, we could pass in the struct object_id*
> instead.
>
> The [20] in the show_submodule_header() function above is
> pre-existing and not added by this patch, but that function's
> signature is touched by this patch so it might make sense to
> tidy that up while we're in here, possibly as a separate patch.
> --

I'll add a patch to the series that does this inbetween these commits.

Thanks,
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 v7 6/7] submodule: refactor show_submodule_summary with helper function

[David Aguilar]

On Wed, Aug 17, 2016 at 05:51:30PM -0700, Jacob Keller wrote:
> [snip]
> @@ -333,31 +326,23 @@ static void print_submodule_summary(struct rev_info 
> *rev, FILE *f,
>   strbuf_release();
>  }
>  
> -void show_submodule_summary(FILE *f, const char *path,
> +/* Helper function to display the submodule header line prior to the full
> + * summary output. If it can locate the submodule objects directory it will
> + * attempt to lookup both the left and right commits and put them into the
> + * left and right pointers.
> + */
> +static void show_submodule_header(FILE *f, const char *path,
>   const char *line_prefix,
>   unsigned char one[20], unsigned char two[20],
>   unsigned dirty_submodule, const char *meta,
> - const char *del, const char *add, const char *reset)
> + const char *reset,
> + struct commit **left, struct commit **right)


Note the pre-existing unsigned char[20]s in the signature above...


> [snip]
> @@ -381,14 +401,39 @@ void show_submodule_summary(FILE *f, const char *path,
>   strbuf_addf(, "%s:%s\n", fast_backward ? " (rewind)" : "", 
> reset);
>   fwrite(sb.buf, sb.len, 1, f);
>  
> - if (!message) /* only NULL if we succeeded in setting up the walk */
> - print_submodule_summary(, f, line_prefix, del, add, reset);
> + strbuf_release();
> +}
> +
> +void show_submodule_summary(FILE *f, const char *path,
> + const char *line_prefix,
> + unsigned char one[20], unsigned char two[20],
> + unsigned dirty_submodule, const char *meta,
> + const char *del, const char *add, const char *reset)


... and here too.

There's an ongoing effort to replace unsigned char sha1[20]
with struct object_id.  It might make sense to pass "one" and
"two" as "unsigned char*" instead of hard-coding the SHA1-length
here.  Alternatively, we could pass in the struct object_id*
instead.

The [20] in the show_submodule_header() function above is
pre-existing and not added by this patch, but that function's
signature is touched by this patch so it might make sense to
tidy that up while we're in here, possibly as a separate patch.
-- 
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

Re: Working from different machines.

[Jacob Keller]

On Wed, Aug 17, 2016 at 3:41 PM, Junio C Hamano  wrote:
> If the answer is "yes", then you are in the problem space that
> Git-the-tool is interested in solving.  Assuming that you have
> network connection into 'desktop' from 'home', the solution would
> involve making it the first thing to do when get home to run "git
> fetch" on 'home' to get the latest state from the 'desktop', and run
> "git push" on 'home' to push out the latest state to the 'desktop'
> before you leave 'home'.  If your 'server' is for your sole use, and
> if 'home' has network connection into 'server', then you could
> instead rendezvous at 'server' by running "git push server" on
> 'desktop' (or 'home') to 'server' as the last thing before you leave
> 'desktop' (or 'home'), and running "git fetch server" on 'home' (or
> 'desktop') as the first thing before you start working on 'home' (or
> 'desktop').
>
>

To add to this, you may need to modify the "git push" portion of the
phases and the "git fetch" potion to include all the branches you are
interested in.

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