Re: none

[Junio C Hamano]

Mihir Mehta  writes:

> Thanks, Junio. Instead of removing that part of the patch, I opted to
> expand it to make it a little clearer (in my opinion) than it was
> before. Let me know if this works.

I am mildly negative on that change.  "Omitting both would give an
empty diff" would be understandable to anybody who understands that
an omitted end of dot-dot is substituted with HEAD *and* thinks what
range HEAD..HEAD means, so it is just an additional noise to them,
and to those who do not want to waste time on thinking, it is a
statement that reads as if "it will be an error" without saying why
it is an error.  So overall, it seems, at least to me, that the
additional text adds negative value.

So, I dunno.

[PATCH v2 09/11] builtin rebase: start a new rebase only if none is in progress

[Pratik Karki via GitGitGadget]

From: Pratik Karki 

To run a new rebase, there needs to be a check to assure that no other
rebase is in progress. New rebase operation cannot start until an
ongoing rebase operation completes or is terminated.

Signed-off-by: Pratik Karki 
Signed-off-by: Johannes Schindelin 
---
 builtin/rebase.c | 48 +++-
 1 file changed, 47 insertions(+), 1 deletion(-)

diff --git a/builtin/rebase.c b/builtin/rebase.c
index 8a7bf3d468..d45f8f9008 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -87,6 +87,7 @@ struct rebase_options {
REBASE_VERBOSE = 1<<1,
REBASE_DIFFSTAT = 1<<2,
REBASE_FORCE = 1<<3,
+   REBASE_INTERACTIVE_EXPLICIT = 1<<4,
} flags;
struct strbuf git_am_opt;
 };
@@ -392,10 +393,11 @@ int cmd_rebase(int argc, const char **argv, const char 
*prefix)
.git_am_opt = STRBUF_INIT,
};
const char *branch_name;
-   int ret, flags;
+   int ret, flags, in_progress = 0;
int ok_to_skip_pre_rebase = 0;
struct strbuf msg = STRBUF_INIT;
struct strbuf revisions = STRBUF_INIT;
+   struct strbuf buf = STRBUF_INIT;
struct object_id merge_base;
struct option builtin_rebase_options[] = {
OPT_STRING(0, "onto", _name,
@@ -447,6 +449,30 @@ int cmd_rebase(int argc, const char **argv, const char 
*prefix)
 
git_config(rebase_config, );
 
+   if (is_directory(apply_dir())) {
+   options.type = REBASE_AM;
+   options.state_dir = apply_dir();
+   } else if (is_directory(merge_dir())) {
+   strbuf_reset();
+   strbuf_addf(, "%s/rewritten", merge_dir());
+   if (is_directory(buf.buf)) {
+   options.type = REBASE_PRESERVE_MERGES;
+   options.flags |= REBASE_INTERACTIVE_EXPLICIT;
+   } else {
+   strbuf_reset();
+   strbuf_addf(, "%s/interactive", merge_dir());
+   if(file_exists(buf.buf)) {
+   options.type = REBASE_INTERACTIVE;
+   options.flags |= REBASE_INTERACTIVE_EXPLICIT;
+   } else
+   options.type = REBASE_MERGE;
+   }
+   options.state_dir = merge_dir();
+   }
+
+   if (options.type != REBASE_UNSPECIFIED)
+   in_progress = 1;
+
argc = parse_options(argc, argv, prefix,
 builtin_rebase_options,
 builtin_rebase_usage, 0);
@@ -455,6 +481,26 @@ int cmd_rebase(int argc, const char **argv, const char 
*prefix)
usage_with_options(builtin_rebase_usage,
   builtin_rebase_options);
 
+   /* Make sure no rebase is in progress */
+   if (in_progress) {
+   const char *last_slash = strrchr(options.state_dir, '/');
+   const char *state_dir_base =
+   last_slash ? last_slash + 1 : options.state_dir;
+   const char *cmd_live_rebase =
+   "git rebase (--continue | --abort | --skip)";
+   strbuf_reset();
+   strbuf_addf(, "rm -fr \"%s\"", options.state_dir);
+   die(_("It seems that there is already a %s directory, and\n"
+ "I wonder if you are in the middle of another rebase.  "
+ "If that is the\n"
+ "case, please try\n\t%s\n"
+ "If that is not the case, please\n\t%s\n"
+ "and run me again.  I am stopping in case you still "
+ "have something\n"
+ "valuable there.\n"),
+   state_dir_base, cmd_live_rebase, buf.buf);
+   }
+
if (!(options.flags & REBASE_NO_QUIET))
strbuf_addstr(_am_opt, " -q");
 
-- 
gitgitgadget

Re: [PATCH 09/11] builtin rebase: start a new rebase only if none is in progress

[Johannes Schindelin]

Hi Stefan,

On Wed, 8 Aug 2018, Stefan Beller wrote:

> On Wed, Aug 8, 2018 at 6:51 AM Pratik Karki  wrote:
> >
> > diff --git a/builtin/rebase.c b/builtin/rebase.c
> > index 8a7bf3d468..a261f552f1 100644
> > --- a/builtin/rebase.c
> > +++ b/builtin/rebase.c
> > @@ -455,6 +481,26 @@ int cmd_rebase(int argc, const char **argv, const char 
> > *prefix)
> > usage_with_options(builtin_rebase_usage,
> >builtin_rebase_options);
> >
> > +   /* Make sure no rebase is in progress */
> 
> The faithful conversion doesn't even stop at the comments. ;-)

Yes, I insisted on it.

TBH it is a bit of a shame that we cannot fix all those error messages
going to stdout, but... you know... One step after the other.

> I shortly wondered if this is the best place for this comment,
> but let's just keep it here to have the 1:1 rewrite.

It should probably be inside the conditional block, but as you say: the
original had it in a funny spot, and so does the converted code.

> > +   if (in_progress) {
> [...]
> > +   state_dir_base, cmd_live_rebase,buf.buf);
> 
> In case a resend is needed, add a whitespace after the
> comma and buf.buf, please.

I will fix this before sending the next iteration.

Thanks for the review!
Dscho

Re: [PATCH 09/11] builtin rebase: start a new rebase only if none is in progress

[Stefan Beller]

On Wed, Aug 8, 2018 at 6:51 AM Pratik Karki  wrote:
>
> To run a new rebase, there needs to be a check to assure that no other
> rebase is in progress. New rebase operation cannot start until an
> ongoing rebase operation completes or is terminated.
>
> Signed-off-by: Pratik Karki 
> ---
>  builtin/rebase.c | 48 +++-
>  1 file changed, 47 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/rebase.c b/builtin/rebase.c
> index 8a7bf3d468..a261f552f1 100644
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@ -87,6 +87,7 @@ struct rebase_options {
> REBASE_VERBOSE = 1<<1,
> REBASE_DIFFSTAT = 1<<2,
> REBASE_FORCE = 1<<3,
> +   REBASE_INTERACTIVE_EXPLICIT = 1<<4,
> } flags;
> struct strbuf git_am_opt;
>  };
> @@ -392,10 +393,11 @@ int cmd_rebase(int argc, const char **argv, const char 
> *prefix)
> .git_am_opt = STRBUF_INIT,
> };
> const char *branch_name;
> -   int ret, flags;
> +   int ret, flags, in_progress = 0;
> int ok_to_skip_pre_rebase = 0;
> struct strbuf msg = STRBUF_INIT;
> struct strbuf revisions = STRBUF_INIT;
> +   struct strbuf buf = STRBUF_INIT;
> struct object_id merge_base;
> struct option builtin_rebase_options[] = {
> OPT_STRING(0, "onto", _name,
> @@ -447,6 +449,30 @@ int cmd_rebase(int argc, const char **argv, const char 
> *prefix)
>
> git_config(rebase_config, );
>
> +   if (is_directory(apply_dir())) {
> +   options.type = REBASE_AM;
> +   options.state_dir = apply_dir();
> +   } else if (is_directory(merge_dir())) {
> +   strbuf_reset();
> +   strbuf_addf(, "%s/rewritten", merge_dir());
> +   if (is_directory(buf.buf)) {
> +   options.type = REBASE_PRESERVE_MERGES;
> +   options.flags |= REBASE_INTERACTIVE_EXPLICIT;
> +   } else {
> +   strbuf_reset();
> +   strbuf_addf(, "%s/interactive", merge_dir());
> +   if(file_exists(buf.buf)) {
> +   options.type = REBASE_INTERACTIVE;
> +   options.flags |= REBASE_INTERACTIVE_EXPLICIT;
> +   } else
> +   options.type = REBASE_MERGE;
> +   }
> +   options.state_dir = merge_dir();
> +   }
> +
> +   if (options.type != REBASE_UNSPECIFIED)
> +   in_progress = 1;
> +
> argc = parse_options(argc, argv, prefix,
>  builtin_rebase_options,
>  builtin_rebase_usage, 0);
> @@ -455,6 +481,26 @@ int cmd_rebase(int argc, const char **argv, const char 
> *prefix)
> usage_with_options(builtin_rebase_usage,
>builtin_rebase_options);
>
> +   /* Make sure no rebase is in progress */

The faithful conversion doesn't even stop at the comments. ;-)
I shortly wondered if this is the best place for this comment,
but let's just keep it here to have the 1:1 rewrite.


> +   if (in_progress) {
[...]
> +   state_dir_base, cmd_live_rebase,buf.buf);

In case a resend is needed, add a whitespace after the
comma and buf.buf, please.

So far I have not seen anything major that would warrant a resend.

Thanks,
Stefan

[PATCH 09/11] builtin rebase: start a new rebase only if none is in progress

[Pratik Karki]

To run a new rebase, there needs to be a check to assure that no other
rebase is in progress. New rebase operation cannot start until an
ongoing rebase operation completes or is terminated.

Signed-off-by: Pratik Karki 
---
 builtin/rebase.c | 48 +++-
 1 file changed, 47 insertions(+), 1 deletion(-)

diff --git a/builtin/rebase.c b/builtin/rebase.c
index 8a7bf3d468..a261f552f1 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -87,6 +87,7 @@ struct rebase_options {
REBASE_VERBOSE = 1<<1,
REBASE_DIFFSTAT = 1<<2,
REBASE_FORCE = 1<<3,
+   REBASE_INTERACTIVE_EXPLICIT = 1<<4,
} flags;
struct strbuf git_am_opt;
 };
@@ -392,10 +393,11 @@ int cmd_rebase(int argc, const char **argv, const char 
*prefix)
.git_am_opt = STRBUF_INIT,
};
const char *branch_name;
-   int ret, flags;
+   int ret, flags, in_progress = 0;
int ok_to_skip_pre_rebase = 0;
struct strbuf msg = STRBUF_INIT;
struct strbuf revisions = STRBUF_INIT;
+   struct strbuf buf = STRBUF_INIT;
struct object_id merge_base;
struct option builtin_rebase_options[] = {
OPT_STRING(0, "onto", _name,
@@ -447,6 +449,30 @@ int cmd_rebase(int argc, const char **argv, const char 
*prefix)
 
git_config(rebase_config, );
 
+   if (is_directory(apply_dir())) {
+   options.type = REBASE_AM;
+   options.state_dir = apply_dir();
+   } else if (is_directory(merge_dir())) {
+   strbuf_reset();
+   strbuf_addf(, "%s/rewritten", merge_dir());
+   if (is_directory(buf.buf)) {
+   options.type = REBASE_PRESERVE_MERGES;
+   options.flags |= REBASE_INTERACTIVE_EXPLICIT;
+   } else {
+   strbuf_reset();
+   strbuf_addf(, "%s/interactive", merge_dir());
+   if(file_exists(buf.buf)) {
+   options.type = REBASE_INTERACTIVE;
+   options.flags |= REBASE_INTERACTIVE_EXPLICIT;
+   } else
+   options.type = REBASE_MERGE;
+   }
+   options.state_dir = merge_dir();
+   }
+
+   if (options.type != REBASE_UNSPECIFIED)
+   in_progress = 1;
+
argc = parse_options(argc, argv, prefix,
 builtin_rebase_options,
 builtin_rebase_usage, 0);
@@ -455,6 +481,26 @@ int cmd_rebase(int argc, const char **argv, const char 
*prefix)
usage_with_options(builtin_rebase_usage,
   builtin_rebase_options);
 
+   /* Make sure no rebase is in progress */
+   if (in_progress) {
+   const char *last_slash = strrchr(options.state_dir, '/');
+   const char *state_dir_base =
+   last_slash ? last_slash + 1 : options.state_dir;
+   const char *cmd_live_rebase =
+   "git rebase (--continue | --abort | --skip)";
+   strbuf_reset();
+   strbuf_addf(, "rm -fr \"%s\"", options.state_dir);
+   die(_("It seems that there is already a %s directory, and\n"
+ "I wonder if you are in the middle of another rebase.  "
+ "If that is the\n"
+ "case, please try\n\t%s\n"
+ "If that is not the case, please\n\t%s\n"
+ "and run me again.  I am stopping in case you still "
+ "have something\n"
+ "valuable there.\n"),
+   state_dir_base, cmd_live_rebase,buf.buf);
+   }
+
if (!(options.flags & REBASE_NO_QUIET))
strbuf_addstr(_am_opt, " -q");
 
-- 
2.18.0

Re: [PATCH 1/1] git-p4: unshelve: use action==add instead of rev==none

[Mazo, Andrey]

>> This was actually discussed in a separate thread [1] some time ago with 
>> patches proposed by Thandesha and me.
>> I haven't yet got time to cook a final patch, which addresses both 
>> Thandesha's and mine use-cases though,
>> so this wasn't submitted to Junio yet.
>> In the meantime, I guess, one of the patches [2] from that thread can be 
>> taken as is.
>>
>> [1] "[BUG] git p4 clone fails when p4 sizes does not return 'fileSize' key"
>>    
>>https://public-inbox.org/git/cajjpmi-plb4qcka5alkxa8b1vozfff+oaq0fguq9yviobrp...@mail.gmail.com/t/#mee2ec50a40242089741f808f06214a44278055b3
>> [2] "[PATCH 1/1] git-p4: fix `sync --verbose` traceback due to 'fileSize'"
>>    
>>https://public-inbox.org/git/2e2b2add4e4fffa4228b8ab9f6cd47fa9bf25207.1523981210.git.am...@checkvideo.com/
>
> Should I re-roll my patch without this change then?
If it's the question to me, then I'm fine either way -- I can rebase my changes 
easily.
However, re-rolling your patch would probably make the aforementioned fileSize 
patch apply cleanly to both maint and master branches.


Thank you,
Andrey Mazo

Re: [PATCH 1/1] git-p4: unshelve: use action==add instead of rev==none

[Luke Diamand]

On 23 May 2018 at 17:41, Mazo, Andrey  wrote:
>> The last one (i.e. "even if it is verbose, if fileSize is not
>> reported, do not write the verbose output") does not look like it is
>> limited to the unshelve feature, so it might, even though it is a
>> one-liner, deserve to be a separate preparatory patch if you want.
>> But I do not feel strongly about either way.
>
> This was actually discussed in a separate thread [1] some time ago with 
> patches proposed by Thandesha and me.
> I haven't yet got time to cook a final patch, which addresses both 
> Thandesha's and mine use-cases though,
> so this wasn't submitted to Junio yet.
> In the meantime, I guess, one of the patches [2] from that thread can be 
> taken as is.
>
> [1] "[BUG] git p4 clone fails when p4 sizes does not return 'fileSize' key"
>   
> https://public-inbox.org/git/cajjpmi-plb4qcka5alkxa8b1vozfff+oaq0fguq9yviobrp...@mail.gmail.com/t/#mee2ec50a40242089741f808f06214a44278055b3
> [2] "[PATCH 1/1] git-p4: fix `sync --verbose` traceback due to 'fileSize'"
>   
> https://public-inbox.org/git/2e2b2add4e4fffa4228b8ab9f6cd47fa9bf25207.1523981210.git.am...@checkvideo.com/

Should I re-roll my patch without this change then?

Luke

Re: [PATCH 1/1] git-p4: unshelve: use action==add instead of rev==none

[Mazo, Andrey]

> The last one (i.e. "even if it is verbose, if fileSize is not
> reported, do not write the verbose output") does not look like it is
> limited to the unshelve feature, so it might, even though it is a
> one-liner, deserve to be a separate preparatory patch if you want.
> But I do not feel strongly about either way.

This was actually discussed in a separate thread [1] some time ago with patches 
proposed by Thandesha and me.
I haven't yet got time to cook a final patch, which addresses both Thandesha's 
and mine use-cases though,
so this wasn't submitted to Junio yet.
In the meantime, I guess, one of the patches [2] from that thread can be taken 
as is.

[1] "[BUG] git p4 clone fails when p4 sizes does not return 'fileSize' key"
  
https://public-inbox.org/git/cajjpmi-plb4qcka5alkxa8b1vozfff+oaq0fguq9yviobrp...@mail.gmail.com/t/#mee2ec50a40242089741f808f06214a44278055b3
[2] "[PATCH 1/1] git-p4: fix `sync --verbose` traceback due to 'fileSize'"
  
https://public-inbox.org/git/2e2b2add4e4fffa4228b8ab9f6cd47fa9bf25207.1523981210.git.am...@checkvideo.com/

Thank you,
Andrey Mazo

Re: [PATCH 1/1] git-p4: unshelve: use action==add instead of rev==none

[Junio C Hamano]

Luke Diamand  writes:

>> However, instead of a separate patch, wouldn't it be better to squash
>> it into the previous one?  So 'make test' would succeed on every
>> commit even with a newer p4 version.
>
> Junio?
>
> I can squash together the original commit and the two fixes if that
> would be better?

Among the three hunks in this fix-up patch, the first two are
strictly fixing what you had in the previous patch, so it make sense
to fix them at the source by squashing.

The last one (i.e. "even if it is verbose, if fileSize is not
reported, do not write the verbose output") does not look like it is
limited to the unshelve feature, so it might, even though it is a
one-liner, deserve to be a separate preparatory patch if you want.
But I do not feel strongly about either way.

Thanks.

Re: [PATCH 1/1] git-p4: unshelve: use action==add instead of rev==none

[Luke Diamand]

On 22 May 2018 at 11:15, SZEDER Gábor <szeder@gmail.com> wrote:
> On Tue, May 22, 2018 at 10:41 AM, Luke Diamand <l...@diamand.org> wrote:
>> SZEDER Gábor found that the unshelve tests fail with newer
>> versions of Perforce (2016 vs 2015).
>>
>> The problem arises because when a file is added in a P4
>> shelved changelist, the depot revision is shown as "none"
>> if using the older p4d (which makes sense - the file doesn't
>> yet exist, so can't have a revision), but as "1" in the newer
>> versions of p4d.
>>
>> For example, adding a file called "new" with 2015.1 and then
>> shelving that change gives this from "p4 describe" :
>>
>> ... //depot/new#none add
>>
>> Using the 2018.1 server gives this:
>>
>> ... //depot/new#1 add
>>
>> We can detect that a file has been added simply by using the
>> file status ("add") instead, rather than the depot revision,
>> which is what this change does.
>>
>> This also fixes a few verbose prints used for debugging this
>> to be more friendly.
>>
>> Signed-off-by: Luke Diamand <l...@diamand.org>
>
> For what it's worth, I can confirm that 't9832-unshelve.sh' passes
> with these changes, here and in all Linux and OSX build jobs on Travis
> CI.

Thanks!

>
> However, instead of a separate patch, wouldn't it be better to squash
> it into the previous one?  So 'make test' would succeed on every
> commit even with a newer p4 version.

Junio?

I can squash together the original commit and the two fixes if that
would be better?

Thanks!
Luke

Re: [PATCH 1/1] git-p4: unshelve: use action==add instead of rev==none

[SZEDER Gábor]

On Tue, May 22, 2018 at 10:41 AM, Luke Diamand <l...@diamand.org> wrote:
> SZEDER Gábor found that the unshelve tests fail with newer
> versions of Perforce (2016 vs 2015).
>
> The problem arises because when a file is added in a P4
> shelved changelist, the depot revision is shown as "none"
> if using the older p4d (which makes sense - the file doesn't
> yet exist, so can't have a revision), but as "1" in the newer
> versions of p4d.
>
> For example, adding a file called "new" with 2015.1 and then
> shelving that change gives this from "p4 describe" :
>
> ... //depot/new#none add
>
> Using the 2018.1 server gives this:
>
> ... //depot/new#1 add
>
> We can detect that a file has been added simply by using the
> file status ("add") instead, rather than the depot revision,
> which is what this change does.
>
> This also fixes a few verbose prints used for debugging this
> to be more friendly.
>
> Signed-off-by: Luke Diamand <l...@diamand.org>

For what it's worth, I can confirm that 't9832-unshelve.sh' passes
with these changes, here and in all Linux and OSX build jobs on Travis
CI.

However, instead of a separate patch, wouldn't it be better to squash
it into the previous one?  So 'make test' would succeed on every
commit even with a newer p4 version.

[PATCH 1/1] git-p4: unshelve: use action==add instead of rev==none

[Luke Diamand]

SZEDER Gábor found that the unshelve tests fail with newer
versions of Perforce (2016 vs 2015).

The problem arises because when a file is added in a P4
shelved changelist, the depot revision is shown as "none"
if using the older p4d (which makes sense - the file doesn't
yet exist, so can't have a revision), but as "1" in the newer
versions of p4d.

For example, adding a file called "new" with 2015.1 and then
shelving that change gives this from "p4 describe" :

... //depot/new#none add

Using the 2018.1 server gives this:

... //depot/new#1 add

We can detect that a file has been added simply by using the
file status ("add") instead, rather than the depot revision,
which is what this change does.

This also fixes a few verbose prints used for debugging this
to be more friendly.

Signed-off-by: Luke Diamand <l...@diamand.org>
---
 git-p4.py | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index 364d86dbcc..c80d85af89 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2463,7 +2463,7 @@ class P4Sync(Command, P4UserMap):
 """
 ret = p4Cmd(["diff2", "{0}#{1}".format(path, filerev), 
"{0}@{1}".format(path, revision)])
 if verbose:
-print("p4 diff2 %s %s %s => %s" % (path, filerev, revision, ret))
+print("p4 diff2 path %s filerev %s revision %s => %s" % (path, 
filerev, revision, ret))
 return ret["status"] == "identical"
 
 def extractFilesFromCommit(self, commit, shelved=False, shelved_cl = 0, 
origin_revision = 0):
@@ -2492,7 +2492,12 @@ class P4Sync(Command, P4UserMap):
 if shelved:
 file["shelved_cl"] = int(shelved_cl)
 
-if file["rev"] != "none" and \
+# For shelved changelists, check that the revision of each 
file that the
+# shelve was based on matches the revision that we are using 
for the
+# starting point for git-fast-import (self.initialParent). 
Otherwise
+# the resulting diff will contain deltas from multiple commits.
+
+if file["action"] != "add" and \
 not self.cmp_shelved(path, file["rev"], origin_revision):
 sys.exit("change {0} not based on {1} for {2}, cannot 
unshelve".format(
 commit["change"], self.initialParent, path))
@@ -2610,7 +2615,7 @@ class P4Sync(Command, P4UserMap):
 def streamOneP4File(self, file, contents):
 relPath = self.stripRepoPath(file['depotFile'], self.branchPrefixes)
 relPath = self.encodeWithUTF8(relPath)
-if verbose:
+if verbose and 'fileSize' in self.stream_file:
 size = int(self.stream_file['fileSize'])
 sys.stdout.write('\r%s --> %s (%i MB)\n' % (file['depotFile'], 
relPath, size/1024/1024))
 sys.stdout.flush()
-- 
2.17.0.392.gdeb1a6e9b7

Re: [PATCH v4] daemon: add --log-destination=(stderr|syslog|none)

[Ævar Arnfjörð Bjarmason]

On Sun, Feb 04 2018, Lucas Werkmeister jotted:

> On 04.02.2018 19:55, Ævar Arnfjörð Bjarmason wrote:
>> 
>> On Sun, Feb 04 2018, Lucas Werkmeister jotted:
>> 
>>>  [--inetd |
>>>   [--listen=] [--port=]
>>>   [--user= [--group=]]]
>>> +[--log-destination=(stderr|syslog|none)]
>> 
>> I micronit, but maybe worthwhile to have a preceeding commit to fix up
>> that indentation of --listen and --user.
>
> I thought the indentation here is intentional, since we’re still inside
> the [] pair (either --inetd or --listen, --port, …).

Yes, sorry. Nevrmind.

>> 
>>> +--log-destination=::
>>> +   Send log messages to the specified destination.
>>> +   Note that this option does not imply --verbose,
>> 
>> Should `` quote --verbose, although I see similar to the WS change I
>> noted above there's plenty of existing stuff in that doc doing it wrong.
>
> I could send a follow-up to consistently ``-quote all options in
> git-daemon.txt… or would that be rejected as unnecessarily cluttering
> the history or the `git blame`?

I don't think anyone would mind. Tiny doc cleanups are neat-o.

>>> +   } else
>>> +   die("unknown log destination '%s'", v);
>> 
>> Should be die(_("unknown..., i.e. use the _() macro.
>> 
>> Anyway, this looks fine to be with our without these proposed
>> bikeshedding changes above. Thanks.
>>

Re: [PATCH v4] daemon: add --log-destination=(stderr|syslog|none)

[Junio C Hamano]

Eric Sunshine  writes:

> On Sun, Feb 4, 2018 at 1:30 PM, Lucas Werkmeister
>  wrote:
>> This new option can be used to override the implicit --syslog of
>> ...
> Thanks. With the 'log_destination' initialization bug fixed, this
> version looks good; I didn't find anything else worth commenting upon.
> Ævar's micronits[1] could be addressed by a follow-up patch (if
> desirable), but probably needn't hold up this patch.
>
> [1]: https://public-inbox.org/git/871si0mvo0@evledraar.gmail.com/

Nicely done.  Thanks, all.

Will queue.

Re: [PATCH v4] daemon: add --log-destination=(stderr|syslog|none)

[Eric Sunshine]

On Sun, Feb 4, 2018 at 1:55 PM, Ævar Arnfjörð Bjarmason
<ava...@gmail.com> wrote:
> On Sun, Feb 04 2018, Lucas Werkmeister jotted:
>>[--inetd |
>> [--listen=] [--port=]
>> [--user= [--group=]]]
>> +  [--log-destination=(stderr|syslog|none)]
>
> I micronit, but maybe worthwhile to have a preceeding commit to fix up
> that indentation of --listen and --user.

The '--listen' and '--user' lines are in the "alternate" ('|') branch
of '--inetd' so, as Lucas points out, this indentation appears
intentional, thus seems okay as-is.

>> +--log-destination=::
>> + Send log messages to the specified destination.
>> + Note that this option does not imply --verbose,
>
> Should `` quote --verbose, although I see similar to the WS change I
> noted above there's plenty of existing stuff in that doc doing it wrong.

As you mention, there are plenty of existing offenders already in this
file, so probably not worth a re-roll (perhaps Junio can fix this new
instance locally), but certainly good fodder for a follow-up patch.

>> + } else
>> + die("unknown log destination '%s'", v);
>
> Should be die(_("unknown..., i.e. use the _() macro.

No message in this source file use _(...) yet so probably not worth a
re-roll, but definitely something for a follow-up patch (by someone).

Re: [PATCH v4] daemon: add --log-destination=(stderr|syslog|none)

[Eric Sunshine]

On Sun, Feb 4, 2018 at 1:30 PM, Lucas Werkmeister
 wrote:
> This new option can be used to override the implicit --syslog of
> --inetd, or to disable all logging. (While --detach also implies
> --syslog, --log-destination=stderr with --detach is useless since
> --detach disassociates the process from the original stderr.) --syslog
> is retained as an alias for --log-destination=syslog.
> [...]
> Signed-off-by: Lucas Werkmeister 
> ---
> Notes:
> Fixes log_destination not being initialized correctly and some minor
> style issues.

Thanks. With the 'log_destination' initialization bug fixed, this
version looks good; I didn't find anything else worth commenting upon.
Ævar's micronits[1] could be addressed by a follow-up patch (if
desirable), but probably needn't hold up this patch.

[1]: https://public-inbox.org/git/871si0mvo0@evledraar.gmail.com/

Re: [PATCH v4] daemon: add --log-destination=(stderr|syslog|none)

[Lucas Werkmeister]

On 04.02.2018 19:55, Ævar Arnfjörð Bjarmason wrote:
> 
> On Sun, Feb 04 2018, Lucas Werkmeister jotted:
> 
>>   [--inetd |
>>[--listen=] [--port=]
>>[--user= [--group=]]]
>> + [--log-destination=(stderr|syslog|none)]
> 
> I micronit, but maybe worthwhile to have a preceeding commit to fix up
> that indentation of --listen and --user.

I thought the indentation here is intentional, since we’re still inside
the [] pair (either --inetd or --listen, --port, …).

> 
>> +--log-destination=::
>> +Send log messages to the specified destination.
>> +Note that this option does not imply --verbose,
> 
> Should `` quote --verbose, although I see similar to the WS change I
> noted above there's plenty of existing stuff in that doc doing it wrong.

I could send a follow-up to consistently ``-quote all options in
git-daemon.txt… or would that be rejected as unnecessarily cluttering
the history or the `git blame`?

>> +} else
>> +die("unknown log destination '%s'", v);
> 
> Should be die(_("unknown..., i.e. use the _() macro.
> 
> Anyway, this looks fine to be with our without these proposed
> bikeshedding changes above. Thanks.
>



smime.p7s
Description: S/MIME Cryptographic Signature

Re: [PATCH v4] daemon: add --log-destination=(stderr|syslog|none)

[Ævar Arnfjörð Bjarmason]

On Sun, Feb 04 2018, Lucas Werkmeister jotted:

>[--inetd |
> [--listen=] [--port=]
> [--user= [--group=]]]
> +  [--log-destination=(stderr|syslog|none)]

I micronit, but maybe worthwhile to have a preceeding commit to fix up
that indentation of --listen and --user.

> +--log-destination=::
> + Send log messages to the specified destination.
> + Note that this option does not imply --verbose,

Should `` quote --verbose, although I see similar to the WS change I
noted above there's plenty of existing stuff in that doc doing it wrong.
> + } else
> + die("unknown log destination '%s'", v);

Should be die(_("unknown..., i.e. use the _() macro.

Anyway, this looks fine to be with our without these proposed
bikeshedding changes above. Thanks.

[PATCH v4] daemon: add --log-destination=(stderr|syslog|none)

[Lucas Werkmeister]

This new option can be used to override the implicit --syslog of
--inetd, or to disable all logging. (While --detach also implies
--syslog, --log-destination=stderr with --detach is useless since
--detach disassociates the process from the original stderr.) --syslog
is retained as an alias for --log-destination=syslog.

--log-destination always overrides implicit --syslog regardless of
option order. This is different than the “last one wins” logic that
applies to some implicit options elsewhere in Git, but should hopefully
be less confusing. (I also don’t know if *all* implicit options in Git
follow “last one wins”.)

The combination of --inetd with --log-destination=stderr is useful, for
instance, when running `git daemon` as an instanced systemd service
(with associated socket unit). In this case, log messages sent via
syslog are received by the journal daemon, but run the risk of being
processed at a time when the `git daemon` process has already exited
(especially if the process was very short-lived, e.g. due to client
error), so that the journal daemon can no longer read its cgroup and
attach the message to the correct systemd unit (see systemd/systemd#2913
[1]). Logging to stderr instead can solve this problem, because systemd
can connect stderr directly to the journal daemon, which then already
knows which unit is associated with this stream.

[1]: https://github.com/systemd/systemd/issues/2913

Helped-by: Ævar Arnfjörð Bjarmason <ava...@gmail.com>
Helped-by: Junio C Hamano <gits...@pobox.com>
Helped-by: Eric Sunshine <sunsh...@sunshineco.com>
Signed-off-by: Lucas Werkmeister <m...@lucaswerkmeister.de>
---

Notes:
Fixes log_destination not being initialized correctly and some minor
style issues.

 Documentation/git-daemon.txt | 28 ---
 daemon.c | 46 +---
 2 files changed, 64 insertions(+), 10 deletions(-)

diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt
index 3c91db7be..56d54a489 100644
--- a/Documentation/git-daemon.txt
+++ b/Documentation/git-daemon.txt
@@ -20,6 +20,7 @@ SYNOPSIS
 [--inetd |
  [--listen=] [--port=]
  [--user= [--group=]]]
+[--log-destination=(stderr|syslog|none)]
 [...]
 
 DESCRIPTION
@@ -80,7 +81,8 @@ OPTIONS
do not have the 'git-daemon-export-ok' file.
 
 --inetd::
-   Have the server run as an inetd service. Implies --syslog.
+   Have the server run as an inetd service. Implies --syslog (may be
+   overridden with `--log-destination=`).
Incompatible with --detach, --port, --listen, --user and --group
options.
 
@@ -110,8 +112,28 @@ OPTIONS
zero for no limit.
 
 --syslog::
-   Log to syslog instead of stderr. Note that this option does not imply
-   --verbose, thus by default only error conditions will be logged.
+   Short for `--log-destination=syslog`.
+
+--log-destination=::
+   Send log messages to the specified destination.
+   Note that this option does not imply --verbose,
+   thus by default only error conditions will be logged.
+   The  must be one of:
++
+--
+stderr::
+   Write to standard error.
+   Note that if `--detach` is specified,
+   the process disconnects from the real standard error,
+   making this destination effectively equivalent to `none`.
+syslog::
+   Write to syslog, using the `git-daemon` identifier.
+none::
+   Disable all logging.
+--
++
+The default destination is `syslog` if `--inetd` or `--detach` is specified,
+otherwise `stderr`.
 
 --user-path::
 --user-path=::
diff --git a/daemon.c b/daemon.c
index e37e343d0..fb538e367 100644
--- a/daemon.c
+++ b/daemon.c
@@ -9,7 +9,12 @@
 #define initgroups(x, y) (0) /* nothing */
 #endif
 
-static int log_syslog;
+static enum log_destination {
+   LOG_DESTINATION_UNSET = -1,
+   LOG_DESTINATION_NONE = 0,
+   LOG_DESTINATION_STDERR = 1,
+   LOG_DESTINATION_SYSLOG = 2,
+} log_destination = LOG_DESTINATION_UNSET;
 static int verbose;
 static int reuseaddr;
 static int informative_errors;
@@ -25,6 +30,7 @@ static const char daemon_usage[] =
 "   [--access-hook=]\n"
 "   [--inetd | [--listen=] [--port=]\n"
 "  [--detach] [--user= [--group=]]\n"
+"   [--log-destination=(stderr|syslog|none)]\n"
 "   [...]";
 
 /* List of acceptable pathname prefixes */
@@ -74,11 +80,14 @@ static const char *get_ip_address(struct hostinfo *hi)
 
 static void logreport(int priority, const char *err, va_list params)
 {
-   if (log_syslog) {
+   switch (log_destination) {
+   case LOG_DESTINATION_SYSLOG: {
char buf[1024];
vsnprintf(buf, sizeof(buf), err, params);
syslog(priority, "%s", buf);
-   } else {
+   break;
+   }
+   ca

Re: [PATCH v3] daemon: add --log-destination=(stderr|syslog|none)

[Lucas Werkmeister]

On 04.02.2018 07:36, Eric Sunshine wrote:
> On Sat, Feb 3, 2018 at 6:08 PM, Lucas Werkmeister
> <m...@lucaswerkmeister.de> wrote:
>> This new option can be used to override the implicit --syslog of
>> --inetd, or to disable all logging. (While --detach also implies
>> --syslog, --log-destination=stderr with --detach is useless since
>> --detach disassociates the process from the original stderr.) --syslog
>> is retained as an alias for --log-destination=syslog.
>> [...]
>> Signed-off-by: Lucas Werkmeister <m...@lucaswerkmeister.de>
> 
> Thanks for the re-roll. There are a few comments below. Except for one
> apparent bug, I'm not sure the others are worth a re-roll...
> 
>> diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt
>> @@ -110,8 +112,26 @@ OPTIONS
>> +--log-destination=::
>> +   Send log messages to the specified destination.
>> +   Note that this option does not imply --verbose,
>> +   thus by default only error conditions will be logged.
>> +   The  defaults to `stderr`, and must be one of:
> 
> I wonder if this should say instead:
> 
> The default destination is `stderr` unless `syslog`
> is implied by `--inetd` or `--detach`, ...
> 
>> diff --git a/daemon.c b/daemon.c
>> @@ -9,7 +9,12 @@
>> -static int log_syslog;
>> +static enum log_destination {
>> +   LOG_DESTINATION_UNSET = -1,
>> +   LOG_DESTINATION_NONE = 0,
>> +   LOG_DESTINATION_STDERR = 1,
>> +   LOG_DESTINATION_SYSLOG = 2,
>> +} log_destination;
> 
> Doesn't log_destination need to be initialized to
> LOG_DESTINATION_UNSET (see [1])? As it stands, being static, it's
> initialized automatically to 0 (LOG_DESTINATION_NONE), which borks the
> logic below.

Thanks, I knew I’d forgotten something :)

> 
>> @@ -74,11 +80,14 @@ static const char *get_ip_address(struct hostinfo *hi)
>>  static void logreport(int priority, const char *err, va_list params)
>>  {
>> +   switch (log_destination) {
>> +   case LOG_DESTINATION_SYSLOG: {
>> char buf[1024];
>> vsnprintf(buf, sizeof(buf), err, params);
>> syslog(priority, "%s", buf);
>> +   break;
>> +   }
>> +   case LOG_DESTINATION_STDERR:
>> /*
>>  * Since stderr is set to buffered mode, the
>>  * logging of different processes will not overlap
>> @@ -88,6 +97,10 @@ static void logreport(int priority, const char *err, 
>> va_list params)
>> vfprintf(stderr, err, params);
>> fputc('\n', stderr);
>> fflush(stderr);
>> +   break;
>> +   case LOG_DESTINATION_NONE:
>> +   case LOG_DESTINATION_UNSET:
>> +   break;
> 
> Since LOG_DESTINATION_UNSET should never occur, perhaps this should be
> written as:
> 
> case LOG_DESTINATION_NONE:
> break;
> case LOG_DESTINATION_UNSET:
> BUG("impossible destination: %d", log_destination);

Good point, I didn’t know about the BUG() macro. But putting the
destination in the message seems unnecessary if it can only be a single
value – or would you make this a default: case?

> 
>> @@ -1297,9 +1309,22 @@ int cmd_main(int argc, const char **argv)
>> +   if (skip_prefix(arg, "--log-destination=", )) {
>> +   if (!strcmp(v, "syslog")) {
>> +   log_destination = LOG_DESTINATION_SYSLOG;
>> +   continue;
>> +   } else if (!strcmp(v, "stderr")) {
>> +   log_destination = LOG_DESTINATION_STDERR;
>> +   continue;
>> +   } else if (!strcmp(v, "none")) {
>> +   log_destination = LOG_DESTINATION_NONE;
>> +   continue;
>> +   } else
>> +   die("Unknown log destination %s", v);
> 
> Mentioned previously[1], this probably ought to start with lowercase.
> It also would be more readable to set off the unknown value with a
> colon or quotes:
> 
> die("unknown log destination '%s', v);
> 
>> @@ -1402,7 +1426,14 @@ int cmd_main(int argc, const char **argv)
>> -   if (log_syslog) {
>> +   if (log_destination == LOG_DESTINATION_UNSET) {
>> +   if (inetd_mode || detach)
>> +   log_destination = LOG_DESTINATION_SYSLOG;
>> +   else
>> +   log_destination = LOG_DESTINATION_STDERR;
>> +   }
>> +
>> +   if (log_destination == LOG_DESTINATION_SYSLOG) {
>> openlog("git-daemon", LOG_PID, LOG_DAEMON);
>> set_die_routine(daemon_die);
> 
> [1]: 
> https://public-inbox.org/git/capig+ctetjq9lsh68fe5otcj9twq9gsbgzdrjzhohtavfvr...@mail.gmail.com/
> 

I’ll send a new version shortly, also addressing your other comments
which I didn’t reply to here.

Cheers,
Lucas



smime.p7s
Description: S/MIME Cryptographic Signature

Re: [PATCH v3] daemon: add --log-destination=(stderr|syslog|none)

[Eric Sunshine]

On Sat, Feb 3, 2018 at 6:08 PM, Lucas Werkmeister
<m...@lucaswerkmeister.de> wrote:
> This new option can be used to override the implicit --syslog of
> --inetd, or to disable all logging. (While --detach also implies
> --syslog, --log-destination=stderr with --detach is useless since
> --detach disassociates the process from the original stderr.) --syslog
> is retained as an alias for --log-destination=syslog.
> [...]
> Signed-off-by: Lucas Werkmeister <m...@lucaswerkmeister.de>

Thanks for the re-roll. There are a few comments below. Except for one
apparent bug, I'm not sure the others are worth a re-roll...

> diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt
> @@ -110,8 +112,26 @@ OPTIONS
> +--log-destination=::
> +   Send log messages to the specified destination.
> +   Note that this option does not imply --verbose,
> +   thus by default only error conditions will be logged.
> +   The  defaults to `stderr`, and must be one of:

I wonder if this should say instead:

The default destination is `stderr` unless `syslog`
is implied by `--inetd` or `--detach`, ...

> diff --git a/daemon.c b/daemon.c
> @@ -9,7 +9,12 @@
> -static int log_syslog;
> +static enum log_destination {
> +   LOG_DESTINATION_UNSET = -1,
> +   LOG_DESTINATION_NONE = 0,
> +   LOG_DESTINATION_STDERR = 1,
> +   LOG_DESTINATION_SYSLOG = 2,
> +} log_destination;

Doesn't log_destination need to be initialized to
LOG_DESTINATION_UNSET (see [1])? As it stands, being static, it's
initialized automatically to 0 (LOG_DESTINATION_NONE), which borks the
logic below.

> @@ -74,11 +80,14 @@ static const char *get_ip_address(struct hostinfo *hi)
>  static void logreport(int priority, const char *err, va_list params)
>  {
> +   switch (log_destination) {
> +   case LOG_DESTINATION_SYSLOG: {
> char buf[1024];
> vsnprintf(buf, sizeof(buf), err, params);
> syslog(priority, "%s", buf);
> +   break;
> +   }
> +   case LOG_DESTINATION_STDERR:
> /*
>  * Since stderr is set to buffered mode, the
>  * logging of different processes will not overlap
> @@ -88,6 +97,10 @@ static void logreport(int priority, const char *err, 
> va_list params)
> vfprintf(stderr, err, params);
> fputc('\n', stderr);
> fflush(stderr);
> +   break;
> +   case LOG_DESTINATION_NONE:
> +   case LOG_DESTINATION_UNSET:
> +   break;

Since LOG_DESTINATION_UNSET should never occur, perhaps this should be
written as:

case LOG_DESTINATION_NONE:
break;
case LOG_DESTINATION_UNSET:
BUG("impossible destination: %d", log_destination);

> @@ -1297,9 +1309,22 @@ int cmd_main(int argc, const char **argv)
> +   if (skip_prefix(arg, "--log-destination=", )) {
> +   if (!strcmp(v, "syslog")) {
> +   log_destination = LOG_DESTINATION_SYSLOG;
> +   continue;
> +   } else if (!strcmp(v, "stderr")) {
> +   log_destination = LOG_DESTINATION_STDERR;
> +   continue;
> +   } else if (!strcmp(v, "none")) {
> +   log_destination = LOG_DESTINATION_NONE;
> +   continue;
> +   } else
> +   die("Unknown log destination %s", v);

Mentioned previously[1], this probably ought to start with lowercase.
It also would be more readable to set off the unknown value with a
colon or quotes:

die("unknown log destination '%s', v);

> @@ -1402,7 +1426,14 @@ int cmd_main(int argc, const char **argv)
> -   if (log_syslog) {
> +   if (log_destination == LOG_DESTINATION_UNSET) {
> +   if (inetd_mode || detach)
> +   log_destination = LOG_DESTINATION_SYSLOG;
> +   else
> +   log_destination = LOG_DESTINATION_STDERR;
> +   }
> +
> +   if (log_destination == LOG_DESTINATION_SYSLOG) {
> openlog("git-daemon", LOG_PID, LOG_DAEMON);
> set_die_routine(daemon_die);

[1]: 
https://public-inbox.org/git/capig+ctetjq9lsh68fe5otcj9twq9gsbgzdrjzhohtavfvr...@mail.gmail.com/

[PATCH v3] daemon: add --log-destination=(stderr|syslog|none)

[Lucas Werkmeister]

This new option can be used to override the implicit --syslog of
--inetd, or to disable all logging. (While --detach also implies
--syslog, --log-destination=stderr with --detach is useless since
--detach disassociates the process from the original stderr.) --syslog
is retained as an alias for --log-destination=syslog.

--log-destination always overrides implicit --syslog regardless of
option order. This is different than the “last one wins” logic that
applies to some implicit options elsewhere in Git, but should hopefully
be less confusing. (I also don’t know if *all* implicit options in Git
follow “last one wins”.)

The combination of --inetd with --log-destination=stderr is useful, for
instance, when running `git daemon` as an instanced systemd service
(with associated socket unit). In this case, log messages sent via
syslog are received by the journal daemon, but run the risk of being
processed at a time when the `git daemon` process has already exited
(especially if the process was very short-lived, e.g. due to client
error), so that the journal daemon can no longer read its cgroup and
attach the message to the correct systemd unit (see systemd/systemd#2913
[1]). Logging to stderr instead can solve this problem, because systemd
can connect stderr directly to the journal daemon, which then already
knows which unit is associated with this stream.

[1]: https://github.com/systemd/systemd/issues/2913

Helped-by: Ævar Arnfjörð Bjarmason <ava...@gmail.com>
Helped-by: Junio C Hamano <gits...@pobox.com>
Helped-by: Eric Sunshine <sunsh...@sunshineco.com>
Signed-off-by: Lucas Werkmeister <m...@lucaswerkmeister.de>
---

Notes:
Next version on my quest to make git-daemon logging more reliable :)
many thanks to Eric Sunshine for review. Main changes this version:

- Rename --send-log-to to --log-destination, following the
  postgresql option. A cursory search didn’t turn up any other
  daemons with a similar option – suggestions welcome!
- Make explicit --log-destination always override implicit --syslog,
  regardless of option order.

 Documentation/git-daemon.txt | 26 ++---
 daemon.c | 45 +---
 2 files changed, 61 insertions(+), 10 deletions(-)

diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt
index 3c91db7be..a0106e6aa 100644
--- a/Documentation/git-daemon.txt
+++ b/Documentation/git-daemon.txt
@@ -20,6 +20,7 @@ SYNOPSIS
 [--inetd |
  [--listen=] [--port=]
  [--user= [--group=]]]
+[--log-destination=(stderr|syslog|none)]
 [...]
 
 DESCRIPTION
@@ -80,7 +81,8 @@ OPTIONS
do not have the 'git-daemon-export-ok' file.
 
 --inetd::
-   Have the server run as an inetd service. Implies --syslog.
+   Have the server run as an inetd service. Implies --syslog (may be
+   overridden with `--log-destination=`).
Incompatible with --detach, --port, --listen, --user and --group
options.
 
@@ -110,8 +112,26 @@ OPTIONS
zero for no limit.
 
 --syslog::
-   Log to syslog instead of stderr. Note that this option does not imply
-   --verbose, thus by default only error conditions will be logged.
+   Short for `--log-destination=syslog`.
+
+--log-destination=::
+   Send log messages to the specified destination.
+   Note that this option does not imply --verbose,
+   thus by default only error conditions will be logged.
+   The  defaults to `stderr`, and must be one of:
++
+--
+stderr::
+   Write to standard error.
+   Note that if `--detach` is specified,
+   the process disconnects from the real standard error,
+   making this destination effectively equivalent to `none`.
+syslog::
+   Write to syslog, using the `git-daemon` identifier.
+none::
+   Disable all logging.
+--
++
 
 --user-path::
 --user-path=::
diff --git a/daemon.c b/daemon.c
index e37e343d0..f28400e3f 100644
--- a/daemon.c
+++ b/daemon.c
@@ -9,7 +9,12 @@
 #define initgroups(x, y) (0) /* nothing */
 #endif
 
-static int log_syslog;
+static enum log_destination {
+   LOG_DESTINATION_UNSET = -1,
+   LOG_DESTINATION_NONE = 0,
+   LOG_DESTINATION_STDERR = 1,
+   LOG_DESTINATION_SYSLOG = 2,
+} log_destination;
 static int verbose;
 static int reuseaddr;
 static int informative_errors;
@@ -25,6 +30,7 @@ static const char daemon_usage[] =
 "   [--access-hook=]\n"
 "   [--inetd | [--listen=] [--port=]\n"
 "  [--detach] [--user= [--group=]]\n"
+"   [--log-destination=(stderr|syslog|none)]\n"
 "   [...]";
 
 /* List of acceptable pathname prefixes */
@@ -74,11 +80,14 @@ static const char *get_ip_address(struct hostinfo *hi)
 
 static void logreport(int priority, const char *err, va_list params)
 {
-   if (log_syslog) {
+   switch (log_dest

Re: [PATCH v2] daemon: add --send-log-to=(stderr|syslog|none)

[Eric Sunshine]

On Sun, Jan 28, 2018 at 5:58 PM, Lucas Werkmeister
<m...@lucaswerkmeister.de> wrote:
> On 28.01.2018 07:40, Eric Sunshine wrote:
>> On Sat, Jan 27, 2018 at 1:31 PM, Lucas Werkmeister
>> <m...@lucaswerkmeister.de> wrote:
>>> This makes it possible to use --inetd while still logging to standard
>>> error. --syslog is retained as an alias for --send-log-to=syslog. A mode
>>> to disable logging explicitly is also provided.
>>>
>>> The combination of --inetd with --send-log-to=stderr is useful, for
>>> instance, when running `git daemon` as an instanced systemd service
>>> (with associated socket unit). In this case, log messages sent via
>>> syslog are received by the journal daemon, but run the risk of being
>>> processed at a time when the `git daemon` process has already exited
>>> (especially if the process was very short-lived, e.g. due to client
>>> error), so that the journal daemon can no longer read its cgroup and
>>> attach the message to the correct systemd unit (see systemd/systemd#2913
>>> [1]). Logging to stderr instead can solve this problem, because systemd
>>> can connect stderr directly to the journal daemon, which then already
>>> knows which unit is associated with this stream.
>>
>> The purpose of this patch would be easier to fathom if the problem was
>> presented first (systemd race condition), followed by the solution
>> (ability to log to stderr even when using --inetd), followed finally
>> by incidental notes ("--syslog is retained as an alias..." and ability
>> to disable logging).
>>
>> Not sure, though, if it's worth a re-roll.
>
> I didn’t want to sound like I was just scratching my own itch ;) I hope
> this option is useful for other use-cases as well?

If the reader does not know that --inetd implies --syslog, then

This makes it possible to use --inetd while still logging to
standard error.

leads to a "Huh?" moment since it is not self-contained. Had it said

Add new option --send-log-to=(stderr|syslog|none) which
allows the implied --syslog by --inetd to be overridden.

it would have provided enough information to understand the purpose of
the patch at a glance. Talking about the systemd race-condition first
would also have explained the patch's purpose, and since the proposed
solution is general (not specific to your use-case), scratching an
itch is not a point against it.

Anyhow, it's not that big of a deal, but it did give me a bit of a
pause when reading the first paragraph since it's customary on this
project to start by explaining the problem.

>> I understand that Junio suggested the name --send-log-to=, but I
>> wonder if the more concise --log= would be an possibility.
>
> --log sounds to me like it could also indicate *what* to log (e. g. “log
> verbosely” or “don’t log client IPs”). But perhaps --log-to= ?

Perhaps we can take into consideration precedent by other (non-Git)
daemon-like commands when naming this option. (None come to my mind
immediately, but I'm sure they're out there.)

>>> if (!strcmp(arg, "--inetd")) {
>>> inetd_mode = 1;
>>> -   log_syslog = 1;
>>> +   log_destination = LOG_TO_SYSLOG;
>>
>> Hmm, so an invocation "--inetd --send-log-to=stderr" works as
>> expected, but "--send-log-to=stderr --inetd" doesn't; output goes to
>> syslog despite the explicit request for stderr. Counterintuitive. This
>> should probably distinguish between 'log_destination' being unset and
>> set explicitly; if unset, then, and only then, have --inetd imply
>> syslog. Perhaps something like this:
>>
>> static enum log_destination {
>> LOG_TO_UNSET = -1
>> LOG_TO_NONE,
>> LOG_TO_STDERR,
>> LOG_TO_SYSLOG,
>> } log_destination = LOG_TO_UNSET;
>>
>> if (!strcmp(arg, "--inetd")) {
>> inetd_mode = 1;
>> if (log_destination == LOG_TO_UNSET)
>> log_destination = LOG_TO_SYSLOG;
>> ...
>> }
>> ...
>> if (log_destination == LOG_TO_UNSET)
>> log_destination = LOG_TO_STDERR
>>
>
> I’m not sure if that’s worth the extra complication… some existing
> options behave the same way already, e. g. in `git rebase --stat
> --quiet`, the `--stat` is ignored.

I took "last one wins" into consideration when writing the above but
was not convinced that it applies to this case since --inetd and
--send-log-to= have no obvious relation to one another (unlike, say,
--verbose and --quiet or other similar 

Re: [PATCH v2] daemon: add --send-log-to=(stderr|syslog|none)

[Lucas Werkmeister]

On 28.01.2018 07:40, Eric Sunshine wrote:
> On Sat, Jan 27, 2018 at 1:31 PM, Lucas Werkmeister
> <m...@lucaswerkmeister.de> wrote:
>> This makes it possible to use --inetd while still logging to standard
>> error. --syslog is retained as an alias for --send-log-to=syslog. A mode
>> to disable logging explicitly is also provided.
>>
>> The combination of --inetd with --send-log-to=stderr is useful, for
>> instance, when running `git daemon` as an instanced systemd service
>> (with associated socket unit). In this case, log messages sent via
>> syslog are received by the journal daemon, but run the risk of being
>> processed at a time when the `git daemon` process has already exited
>> (especially if the process was very short-lived, e.g. due to client
>> error), so that the journal daemon can no longer read its cgroup and
>> attach the message to the correct systemd unit (see systemd/systemd#2913
>> [1]). Logging to stderr instead can solve this problem, because systemd
>> can connect stderr directly to the journal daemon, which then already
>> knows which unit is associated with this stream.
> 
> The purpose of this patch would be easier to fathom if the problem was
> presented first (systemd race condition), followed by the solution
> (ability to log to stderr even when using --inetd), followed finally
> by incidental notes ("--syslog is retained as an alias..." and ability
> to disable logging).
> 
> Not sure, though, if it's worth a re-roll.

I didn’t want to sound like I was just scratching my own itch ;) I hope
this option is useful for other use-cases as well?

> 
>> Signed-off-by: Lucas Werkmeister <m...@lucaswerkmeister.de>
>> Helped-by: Ævar Arnfjörð Bjarmason <ava...@gmail.com>
>> Helped-by: Junio C Hamano <gits...@pobox.com>
>> ---
>>
>> Notes:
>> This was originally “daemon: add --no-syslog to undo implicit
>> --syslog”, but Junio pointed out that combining --no-syslog with
>> --detach isn’t especially useful and suggested --send-log-to=
>> instead. Is Helped-by: the right credit for this or should it be
>> something else?
> 
> Helped-by: is fine, though typically your Signed-off-by: would be last.
> 
> I understand that Junio suggested the name --send-log-to=, but I
> wonder if the more concise --log= would be an possibility.

--log sounds to me like it could also indicate *what* to log (e. g. “log
verbosely” or “don’t log client IPs”). But perhaps --log-to= ?

> 
> More below...
> 
>> diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt
>> @@ -110,8 +111,26 @@ OPTIONS
>> +--send-log-to=::
>> +   Send log messages to the specified destination.
>> +   Note that this option does not imply --verbose,
>> +   thus by default only error conditions will be logged.
>> +   The  defaults to `stderr`, and must be one of:
> 
> Perhaps also update the documentation of --inetd to mention that its
> implied --syslog can be overridden by --send-log-to=.

Very good idea!

> 
>> diff --git a/daemon.c b/daemon.c
>> @@ -74,11 +79,14 @@ static const char *get_ip_address(struct hostinfo *hi)
>>
>>  static void logreport(int priority, const char *err, va_list params)
>>  {
>> -   if (log_syslog) {
>> +   switch (log_destination) {
>> +   case LOG_TO_SYSLOG: {
>> char buf[1024];
>> vsnprintf(buf, sizeof(buf), err, params);
>> syslog(priority, "%s", buf);
>> -   } else {
>> +   break;
>> +   }
>> +   case LOG_TO_STDERR: {
> 
> There aren't many instances of:
> 
> case FOO: {
> 
> in the code-base, but those that exist don't use braces around cases
> which don't need it, so perhaps drop it from the STDERR and NONE
> cases. (Probably not worth a re-roll, though.)

Good point, forgot that part of the coding guidelines. Only the syslog
case needs it because the buf declaration can’t be labeled directly.

> 
>> /*
>>  * Since stderr is set to buffered mode, the
>>  * logging of different processes will not overlap
>> @@ -88,6 +96,11 @@ static void logreport(int priority, const char *err, 
>> va_list params)
>> vfprintf(stderr, err, params);
>> fputc('\n', stderr);
>> fflush(stderr);
>> +   break;
>> +   }
>> +   case LOG_TO_NONE: {
>> +   break;
>> +   }
>> }
> 
> Consecutive lines with braces at the same indentation level is rathe

Re: [PATCH v2] daemon: add --send-log-to=(stderr|syslog|none)

[Eric Sunshine]

On Sat, Jan 27, 2018 at 1:31 PM, Lucas Werkmeister
<m...@lucaswerkmeister.de> wrote:
> This makes it possible to use --inetd while still logging to standard
> error. --syslog is retained as an alias for --send-log-to=syslog. A mode
> to disable logging explicitly is also provided.
>
> The combination of --inetd with --send-log-to=stderr is useful, for
> instance, when running `git daemon` as an instanced systemd service
> (with associated socket unit). In this case, log messages sent via
> syslog are received by the journal daemon, but run the risk of being
> processed at a time when the `git daemon` process has already exited
> (especially if the process was very short-lived, e.g. due to client
> error), so that the journal daemon can no longer read its cgroup and
> attach the message to the correct systemd unit (see systemd/systemd#2913
> [1]). Logging to stderr instead can solve this problem, because systemd
> can connect stderr directly to the journal daemon, which then already
> knows which unit is associated with this stream.

The purpose of this patch would be easier to fathom if the problem was
presented first (systemd race condition), followed by the solution
(ability to log to stderr even when using --inetd), followed finally
by incidental notes ("--syslog is retained as an alias..." and ability
to disable logging).

Not sure, though, if it's worth a re-roll.

> Signed-off-by: Lucas Werkmeister <m...@lucaswerkmeister.de>
> Helped-by: Ævar Arnfjörð Bjarmason <ava...@gmail.com>
> Helped-by: Junio C Hamano <gits...@pobox.com>
> ---
>
> Notes:
> This was originally “daemon: add --no-syslog to undo implicit
> --syslog”, but Junio pointed out that combining --no-syslog with
> --detach isn’t especially useful and suggested --send-log-to=
> instead. Is Helped-by: the right credit for this or should it be
> something else?

Helped-by: is fine, though typically your Signed-off-by: would be last.

I understand that Junio suggested the name --send-log-to=, but I
wonder if the more concise --log= would be an possibility.

More below...

> diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt
> @@ -110,8 +111,26 @@ OPTIONS
> +--send-log-to=::
> +   Send log messages to the specified destination.
> +   Note that this option does not imply --verbose,
> +   thus by default only error conditions will be logged.
> +   The  defaults to `stderr`, and must be one of:

Perhaps also update the documentation of --inetd to mention that its
implied --syslog can be overridden by --send-log-to=.

> diff --git a/daemon.c b/daemon.c
> @@ -74,11 +79,14 @@ static const char *get_ip_address(struct hostinfo *hi)
>
>  static void logreport(int priority, const char *err, va_list params)
>  {
> -   if (log_syslog) {
> +   switch (log_destination) {
> +   case LOG_TO_SYSLOG: {
> char buf[1024];
> vsnprintf(buf, sizeof(buf), err, params);
> syslog(priority, "%s", buf);
> -   } else {
> +   break;
> +   }
> +   case LOG_TO_STDERR: {

There aren't many instances of:

case FOO: {

in the code-base, but those that exist don't use braces around cases
which don't need it, so perhaps drop it from the STDERR and NONE
cases. (Probably not worth a re-roll, though.)

> /*
>  * Since stderr is set to buffered mode, the
>  * logging of different processes will not overlap
> @@ -88,6 +96,11 @@ static void logreport(int priority, const char *err, 
> va_list params)
> vfprintf(stderr, err, params);
> fputc('\n', stderr);
> fflush(stderr);
> +   break;
> +   }
> +   case LOG_TO_NONE: {
> +   break;
> +   }
> }

Consecutive lines with braces at the same indentation level is rather
odd (but see previous comment).

>  }
>
> @@ -1289,7 +1302,7 @@ int cmd_main(int argc, const char **argv)
> }
> if (!strcmp(arg, "--inetd")) {
> inetd_mode = 1;
> -   log_syslog = 1;
> +   log_destination = LOG_TO_SYSLOG;

Hmm, so an invocation "--inetd --send-log-to=stderr" works as
expected, but "--send-log-to=stderr --inetd" doesn't; output goes to
syslog despite the explicit request for stderr. Counterintuitive. This
should probably distinguish between 'log_destination' being unset and
set explicitly; if unset, then, and only then, have --inetd imply
syslog. Perhaps something like this:

static enum log_destination {
LOG_TO_UNSET = -1
LOG_TO_NONE,
LOG_TO_STDERR,
LOG_TO_SY

[PATCH v2] daemon: add --send-log-to=(stderr|syslog|none)

[Lucas Werkmeister]

This makes it possible to use --inetd while still logging to standard
error. --syslog is retained as an alias for --send-log-to=syslog. A mode
to disable logging explicitly is also provided.

The combination of --inetd with --send-log-to=stderr is useful, for
instance, when running `git daemon` as an instanced systemd service
(with associated socket unit). In this case, log messages sent via
syslog are received by the journal daemon, but run the risk of being
processed at a time when the `git daemon` process has already exited
(especially if the process was very short-lived, e.g. due to client
error), so that the journal daemon can no longer read its cgroup and
attach the message to the correct systemd unit (see systemd/systemd#2913
[1]). Logging to stderr instead can solve this problem, because systemd
can connect stderr directly to the journal daemon, which then already
knows which unit is associated with this stream.

[1]: https://github.com/systemd/systemd/issues/2913

Signed-off-by: Lucas Werkmeister <m...@lucaswerkmeister.de>
Helped-by: Ævar Arnfjörð Bjarmason <ava...@gmail.com>
Helped-by: Junio C Hamano <gits...@pobox.com>
---

Notes:
This was originally “daemon: add --no-syslog to undo implicit
--syslog”, but Junio pointed out that combining --no-syslog with
--detach isn’t especially useful and suggested --send-log-to=
instead. Is Helped-by: the right credit for this or should it be
something else?

I’m also not quite sure if the systemd part of the commit message is
accurate – see my comment on the linked issue [2]. TL;DR: this might
no longer be necessary on systemd v235. (I’m experiencing the
problem on Debian Stretch, systemd v232.) As in the last patch, feel
free to remove that part of the commit message.

[2]: https://github.com/systemd/systemd/issues/2913#issuecomment-361002589

 Documentation/git-daemon.txt | 23 +--
 daemon.c | 43 ---
 2 files changed, 57 insertions(+), 9 deletions(-)

diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt
index 3c91db7be..e973f4390 100644
--- a/Documentation/git-daemon.txt
+++ b/Documentation/git-daemon.txt
@@ -20,6 +20,7 @@ SYNOPSIS
 [--inetd |
  [--listen=] [--port=]
  [--user= [--group=]]]
+[--send-log-to=(stderr|syslog|none)]
 [...]
 
 DESCRIPTION
@@ -110,8 +111,26 @@ OPTIONS
zero for no limit.
 
 --syslog::
-   Log to syslog instead of stderr. Note that this option does not imply
-   --verbose, thus by default only error conditions will be logged.
+   Short for `--send-log-to=syslog`.
+
+--send-log-to=::
+   Send log messages to the specified destination.
+   Note that this option does not imply --verbose,
+   thus by default only error conditions will be logged.
+   The  defaults to `stderr`, and must be one of:
++
+--
+stderr::
+   Write to standard error.
+   Note that if `--detach` is specified,
+   the process disconnects from the real standard error,
+   making this destination effectively equivalent to `none`.
+syslog::
+   Write to syslog, using the `git-daemon` identifier.
+none::
+   Disable all logging.
+--
++
 
 --user-path::
 --user-path=::
diff --git a/daemon.c b/daemon.c
index e37e343d0..3d8e16ede 100644
--- a/daemon.c
+++ b/daemon.c
@@ -9,7 +9,11 @@
 #define initgroups(x, y) (0) /* nothing */
 #endif
 
-static int log_syslog;
+static enum log_destination {
+   LOG_TO_NONE = -1,
+   LOG_TO_STDERR = 0,
+   LOG_TO_SYSLOG = 1,
+} log_destination;
 static int verbose;
 static int reuseaddr;
 static int informative_errors;
@@ -25,6 +29,7 @@ static const char daemon_usage[] =
 "   [--access-hook=]\n"
 "   [--inetd | [--listen=] [--port=]\n"
 "  [--detach] [--user= [--group=]]\n"
+"   [--send-log-to=(stderr|syslog|none)]\n"
 "   [...]";
 
 /* List of acceptable pathname prefixes */
@@ -74,11 +79,14 @@ static const char *get_ip_address(struct hostinfo *hi)
 
 static void logreport(int priority, const char *err, va_list params)
 {
-   if (log_syslog) {
+   switch (log_destination) {
+   case LOG_TO_SYSLOG: {
char buf[1024];
vsnprintf(buf, sizeof(buf), err, params);
syslog(priority, "%s", buf);
-   } else {
+   break;
+   }
+   case LOG_TO_STDERR: {
/*
 * Since stderr is set to buffered mode, the
 * logging of different processes will not overlap
@@ -88,6 +96,11 @@ static void logreport(int priority, const char *err, va_list 
params)
vfprintf(stderr, err, params);
fputc('\n', stderr);
fflush(stderr);
+   break;
+   }
+   case LOG_TO_NONE: {
+   

[PATCH 04/13] list-objects-filter-blobs-none: add filter to omit all blobs

[Jeff Hostetler]

From: Jeff Hostetler 

Create a simple filter for traverse_commit_list_worker() to omit
all blobs from the result.

This filter will be used in a future commit by rev-list and pack-objects
to create a "commits and trees" result.  This is intended for partial
clone and fetch support.

Signed-off-by: Jeff Hostetler 
---
 Makefile |  1 +
 list-objects-filter-blobs-none.c | 83 
 list-objects-filter-blobs-none.h | 18 +
 3 files changed, 102 insertions(+)
 create mode 100644 list-objects-filter-blobs-none.c
 create mode 100644 list-objects-filter-blobs-none.h

diff --git a/Makefile b/Makefile
index e59f12d..7e9d1f4 100644
--- a/Makefile
+++ b/Makefile
@@ -807,6 +807,7 @@ LIB_OBJS += levenshtein.o
 LIB_OBJS += line-log.o
 LIB_OBJS += line-range.o
 LIB_OBJS += list-objects.o
+LIB_OBJS += list-objects-filter-blobs-none.o
 LIB_OBJS += list-objects-filter-map.o
 LIB_OBJS += ll-merge.o
 LIB_OBJS += lockfile.o
diff --git a/list-objects-filter-blobs-none.c b/list-objects-filter-blobs-none.c
new file mode 100644
index 000..1b548b9
--- /dev/null
+++ b/list-objects-filter-blobs-none.c
@@ -0,0 +1,83 @@
+#include "cache.h"
+#include "dir.h"
+#include "tag.h"
+#include "commit.h"
+#include "tree.h"
+#include "blob.h"
+#include "diff.h"
+#include "tree-walk.h"
+#include "revision.h"
+#include "list-objects.h"
+#include "list-objects-filter-blobs-none.h"
+
+#define DEFAULT_MAP_SIZE (16*1024)
+
+/*
+ * A filter for list-objects to omit ALL blobs from the traversal.
+ * And to OPTIONALLY collect a list of the omitted OIDs.
+ */
+struct filter_blobs_none_data {
+   struct oidmap *omits;
+};
+
+static list_objects_filter_result filter_blobs_none(
+   list_objects_filter_type filter_type,
+   struct object *obj,
+   const char *pathname,
+   const char *filename,
+   void *filter_data_)
+{
+   struct filter_blobs_none_data *filter_data = filter_data_;
+
+   switch (filter_type) {
+   default:
+   die("unkown filter_type");
+   return LOFR_ZERO;
+
+   case LOFT_BEGIN_TREE:
+   assert(obj->type == OBJ_TREE);
+   /* always include all tree objects */
+   return LOFR_MARK_SEEN | LOFR_SHOW;
+
+   case LOFT_END_TREE:
+   assert(obj->type == OBJ_TREE);
+   return LOFR_ZERO;
+
+   case LOFT_BLOB:
+   assert(obj->type == OBJ_BLOB);
+   assert((obj->flags & SEEN) == 0);
+
+   if (filter_data->omits)
+   list_objects_filter_map_insert(
+   filter_data->omits, >oid, pathname,
+   obj->type);
+
+   return LOFR_MARK_SEEN; /* but not LOFR_SHOW (hard omit) */
+   }
+}
+
+void traverse_commit_list__blobs_none(
+   struct rev_info *revs,
+   show_commit_fn show_commit,
+   show_object_fn show_object,
+   list_objects_filter_map_foreach_cb print_omitted_object,
+   void *ctx_data)
+{
+   struct filter_blobs_none_data d;
+
+   memset(, 0, sizeof(d));
+   if (print_omitted_object) {
+   d.omits = xcalloc(1, sizeof(*d.omits));
+   oidmap_init(d.omits, DEFAULT_MAP_SIZE);
+   }
+
+   traverse_commit_list_worker(revs, show_commit, show_object, ctx_data,
+   filter_blobs_none, );
+
+   if (print_omitted_object) {
+   list_objects_filter_map_foreach(d.omits,
+   print_omitted_object,
+   ctx_data);
+   oidmap_free(d.omits, 1);
+   }
+}
diff --git a/list-objects-filter-blobs-none.h b/list-objects-filter-blobs-none.h
new file mode 100644
index 000..363c9de
--- /dev/null
+++ b/list-objects-filter-blobs-none.h
@@ -0,0 +1,18 @@
+#ifndef LIST_OBJECTS_FILTER_BLOBS_NONE_H
+#define LIST_OBJECTS_FILTER_BLOBS_NONE_H
+
+#include "list-objects-filter-map.h"
+
+/*
+ * A filter for list-objects to omit ALL blobs
+ * from the traversal.
+ */
+void traverse_commit_list__blobs_none(
+   struct rev_info *revs,
+   show_commit_fn show_commit,
+   show_object_fn show_object,
+   list_objects_filter_map_foreach_cb print_omitted_object,
+   void *ctx_data);
+
+#endif /* LIST_OBJECTS_FILTER_BLOBS_NONE_H */
+
-- 
2.9.3

Re: [PATCH v2] doc: add missing values "none" and "default" for diff.wsErrorHighlight

[Junio C Hamano]

Andreas Heiduk  writes:

> The values have eluded documentation so far. While at it streamline
> the wording by grouping relevant parts together.
>
> Signed-off-by: Andreas Heiduk 
> ---
>  Documentation/diff-config.txt  | 11 +++
>  Documentation/diff-options.txt | 17 -
>  2 files changed, 15 insertions(+), 13 deletions(-)

Looks sensible; thanks.  Will queue.

[PATCH v2] doc: add missing values "none" and "default" for diff.wsErrorHighlight

[Andreas Heiduk]

The values have eluded documentation so far. While at it streamline
the wording by grouping relevant parts together.

Signed-off-by: Andreas Heiduk <ashei...@gmail.com>
---
 Documentation/diff-config.txt  | 11 +++
 Documentation/diff-options.txt | 17 -
 2 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt
index cbce8ec63..5ca942ab5 100644
--- a/Documentation/diff-config.txt
+++ b/Documentation/diff-config.txt
@@ -200,7 +200,10 @@ diff.algorithm::
 +
 
 diff.wsErrorHighlight::
-   A comma separated list of `old`, `new`, `context`, that
-   specifies how whitespace errors on lines are highlighted
-   with `color.diff.whitespace`.  Can be overridden by the
-   command line option `--ws-error-highlight=`
+   Highlight whitespace errors in the `context`, `old` or `new`
+   lines of the diff.  Multiple values are separated by comma,
+   `none` resets previous values, `default` reset the list to
+   `new` and `all` is a shorthand for `old,new,context`.  The
+   whitespace errors are colored with `color.diff.whitespace`.
+   The command line option `--ws-error-highlight=`
+   overrides this setting.
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 89cc0f48d..d60f61ad4 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -300,15 +300,14 @@ ifndef::git-format-patch[]
with --exit-code.
 
 --ws-error-highlight=::
-   Highlight whitespace errors on lines specified by 
-   in the color specified by `color.diff.whitespace`.  
-   is a comma separated list of `old`, `new`, `context`.  When
-   this option is not given, only whitespace errors in `new`
-   lines are highlighted.  E.g. `--ws-error-highlight=new,old`
-   highlights whitespace errors on both deleted and added lines.
-   `all` can be used as a short-hand for `old,new,context`.
-   The `diff.wsErrorHighlight` configuration variable can be
-   used to specify the default behaviour.
+   Highlight whitespace errors in the `context`, `old` or `new`
+   lines of the diff.  Multiple values are separated by comma,
+   `none` resets previous values, `default` reset the list to
+   `new` and `all` is a shorthand for `old,new,context`.  When
+   this option is not given, and the configuration variable
+   `diff.wsErrorHighlight` is not set, only whitespace errors in
+   `new` lines are highlighted. The whitespace errors are colored
+   whith `color.diff.whitespace`.
 
 endif::git-format-patch[]
 
-- 
2.13.3

Re: [PATCH] doc: add missing "none" value for diff.wsErrorHighlight

[Junio C Hamano]

Andreas Heiduk <ashei...@gmail.com> writes:

> The value has not eluded documentation so far.

I am not sure what "has not eluded" means in this context (did you
mean "has eluded"?).  

The patch text itself is not wrong per-se, but if we are to add
documentation for 'none', diff-options.txt must also document that
it clears the default and previously given values, unlike new, old
and context that are cumulative.  For that matter, we do not list
'default' and 'all' (which also clears the previous ones before
setting their own) in that three-item list, either.

I think we need to either 

 - make it to a six-item list and then describe that 'none', 'all'
   and 'default' clear the slate before taking any effect, or 

 - keep it three-item list of cumulative things, and then in the
   sentence that talks about `all` in Documentation/diff-options.txt
   to also explain what 'default' and 'none' do.

Thanks.

> Signed-off-by: Andreas Heiduk <ashei...@gmail.com>
> ---
>  Documentation/diff-config.txt  | 2 +-
>  Documentation/diff-options.txt | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt
> index cbce8ec63..c84ced8f6 100644
> --- a/Documentation/diff-config.txt
> +++ b/Documentation/diff-config.txt
> @@ -200,7 +200,7 @@ diff.algorithm::
>  +
>  
>  diff.wsErrorHighlight::
> - A comma separated list of `old`, `new`, `context`, that
> + A comma separated list of `old`, `new`, `context` and `none`, that
>   specifies how whitespace errors on lines are highlighted
>   with `color.diff.whitespace`.  Can be overridden by the
>   command line option `--ws-error-highlight=`
> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
> index 89cc0f48d..903d68eb7 100644
> --- a/Documentation/diff-options.txt
> +++ b/Documentation/diff-options.txt
> @@ -302,7 +302,7 @@ ifndef::git-format-patch[]
>  --ws-error-highlight=::
>   Highlight whitespace errors on lines specified by 
>   in the color specified by `color.diff.whitespace`.  
> - is a comma separated list of `old`, `new`, `context`.  When
> + is a comma separated list of `old`, `new`, `context` and `none`.  When
>   this option is not given, only whitespace errors in `new`
>   lines are highlighted.  E.g. `--ws-error-highlight=new,old`
>   highlights whitespace errors on both deleted and added lines.

[PATCH] doc: add missing "none" value for diff.wsErrorHighlight

[Andreas Heiduk]

The value has not eluded documentation so far.

Signed-off-by: Andreas Heiduk <ashei...@gmail.com>
---
 Documentation/diff-config.txt  | 2 +-
 Documentation/diff-options.txt | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt
index cbce8ec63..c84ced8f6 100644
--- a/Documentation/diff-config.txt
+++ b/Documentation/diff-config.txt
@@ -200,7 +200,7 @@ diff.algorithm::
 +
 
 diff.wsErrorHighlight::
-   A comma separated list of `old`, `new`, `context`, that
+   A comma separated list of `old`, `new`, `context` and `none`, that
specifies how whitespace errors on lines are highlighted
with `color.diff.whitespace`.  Can be overridden by the
command line option `--ws-error-highlight=`
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 89cc0f48d..903d68eb7 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -302,7 +302,7 @@ ifndef::git-format-patch[]
 --ws-error-highlight=::
Highlight whitespace errors on lines specified by 
in the color specified by `color.diff.whitespace`.  
-   is a comma separated list of `old`, `new`, `context`.  When
+   is a comma separated list of `old`, `new`, `context` and `none`.  When
this option is not given, only whitespace errors in `new`
lines are highlighted.  E.g. `--ws-error-highlight=new,old`
highlights whitespace errors on both deleted and added lines.
-- 
2.13.3

Re: none

[Matthieu Moy]

miwilli...@google.com writes:

> From 7201fe08ede76e502211a781250c9a0b702a78b2 Mon Sep 17 00:00:00 2001
> From: Mike Williams 
> Date: Mon, 11 Apr 2016 14:18:39 -0400
> Subject: [PATCH 1/1] wt-status: Remove '!!' from
> wt_status_collect_changed_cb
>
> The wt_status_collect_changed_cb function uses an extraneous double
> negation (!!)
> when determining whether or not a submodule has new commits.

It's not just a double negation, it's a way to ensure that the value is
0 or 1 (it's a relatively common idiom in C at least in Git's codebase).

new_submodule_commits is a 1-bit bitfield, and you don't want to assign
anything other than 1 or 0 (or you'll get modulo 2^n semantics, with
n==1).

So the old code is correct and your patch would introduce a bug.

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

[PATCH 08/12] git submodule update: check for "none" in C

[Stefan Beller]

Signed-off-by: Stefan Beller <sbel...@google.com>
---
 builtin/submodule--helper.c | 38 --
 git-submodule.sh|  8 +---
 2 files changed, 37 insertions(+), 9 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index f81f37a..73954ac 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -255,9 +255,15 @@ static int module_clone(int argc, const char **argv, const 
char *prefix)
return 0;
 }
 
+static int git_submodule_config(const char *var, const char *value, void *cb)
+{
+   return parse_submodule_config_option(var, value);
+}
+
 static int module_list_or_clone(int argc, const char **argv, const char 
*prefix)
 {
int i;
+   char *update = NULL;
struct pathspec pathspec;
struct module_list list = MODULE_LIST_INIT;
 
@@ -265,6 +271,9 @@ static int module_list_or_clone(int argc, const char 
**argv, const char *prefix)
OPT_STRING(0, "prefix", ,
   N_("path"),
   N_("alternative anchor for relative paths")),
+   OPT_STRING(0, "update", ,
+  N_("string"),
+  N_("update command for submodules")),
OPT_END()
};
 
@@ -281,20 +290,45 @@ static int module_list_or_clone(int argc, const char 
**argv, const char *prefix)
return 1;
}
 
+   gitmodules_config();
+   /* Overlay the parsed .gitmodules file with .git/config */
+   git_config(git_submodule_config, NULL);
+
for (i = 0; i < list.nr; i++) {
+   const struct submodule *sub = NULL;
+   const char *displaypath = NULL;
const struct cache_entry *ce = list.entries[i];
+   struct strbuf sb = STRBUF_INIT;
+   const char *update_module = NULL;
 
char *env_prefix = getenv("prefix");
if (ce_stage(ce)) {
if (env_prefix)
-   fprintf(stderr, "Skipping unmerged submodule 
%s/%s",
+   fprintf(stderr, "Skipping unmerged submodule 
%s/%s\n",
env_prefix, ce->name);
else
-   fprintf(stderr, "Skipping unmerged submodule 
%s",
+   fprintf(stderr, "Skipping unmerged submodule 
%s\n",
ce->name);
continue;
}
 
+   sub = submodule_from_path(null_sha1, ce->name);
+   if (env_prefix)
+   displaypath = relative_path(env_prefix, ce->name, );
+   else
+   displaypath = ce->name;
+
+   if (update)
+   update_module = update;
+   if (!update_module)
+   update_module = sub->update;
+   if (!update_module)
+   update_module = "checkout";
+   if (!strcmp(update_module, "none")) {
+   fprintf(stderr, "Skipping submodule '%s'\n", 
displaypath);
+   continue;
+   }
+
printf("%06o %s %d\t", ce->ce_mode, sha1_to_hex(ce->sha1), 
ce_stage(ce));
utf8_fprintf(stdout, "%s\n", ce->name);
}
diff --git a/git-submodule.sh b/git-submodule.sh
index 0754ecd..227fed6 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -656,7 +656,7 @@ cmd_update()
fi
 
cloned_modules=
-   git submodule--helper list-or-clone --prefix "$wt_prefix" "$@" | {
+   git submodule--helper list-or-clone --prefix "$wt_prefix" 
${update:+--update "$update"} "$@" | {
err=
while read mode sha1 stage sm_path
do
@@ -677,12 +677,6 @@ cmd_update()
 
displaypath=$(relative_path "$prefix$sm_path")
 
-   if test "$update_module" = "none"
-   then
-   echo >&2 "Skipping submodule '$displaypath'"
-   continue
-   fi
-
if test -z "$url"
then
# Only mention uninitialized submodules when its
-- 
2.5.0.277.gfdc362b.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] gitk: Fix bad English grammar Matches none Commit Info

[Paul Mackerras]

On Thu, Apr 02, 2015 at 03:05:06PM -0600, Alex Henrie wrote:
 Signed-off-by: Alex Henrie alexhenri...@gmail.com
 ---
  gitk | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/gitk b/gitk
 index 9a2daf3..30fcd30 100755
 --- a/gitk
 +++ b/gitk
 @@ -4066,7 +4066,7 @@ set known_view_options {
  {committer t15  .  --committer=*  {mc Committer:}}
  {loginfo   t15  .. --grep=*   {mc Commit Message:}}
  {allmatch  b.. --all-match{mc Matches all Commit Info 
 criteria}}
 -{igrep b.. --invert-grep  {mc Matches none Commit Info 
 criteria}}
 +{igrep b.. --invert-grep  {mc Matches no Commit Info 
 criteria}}

Thanks, applied.

Paul.
--
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] gitk: Fix bad English grammar Matches none Commit Info

[Alex Henrie]

Signed-off-by: Alex Henrie alexhenri...@gmail.com
---
 gitk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gitk b/gitk
index 9a2daf3..30fcd30 100755
--- a/gitk
+++ b/gitk
@@ -4066,7 +4066,7 @@ set known_view_options {
 {committer t15  .  --committer=*  {mc Committer:}}
 {loginfo   t15  .. --grep=*   {mc Commit Message:}}
 {allmatch  b.. --all-match{mc Matches all Commit Info 
criteria}}
-{igrep b.. --invert-grep  {mc Matches none Commit Info 
criteria}}
+{igrep b.. --invert-grep  {mc Matches no Commit Info criteria}}
 {changes_l l+  {}   {mc Changes to Files:}}
 {pickaxe_s r0   .  {}   {mc Fixed String}}
 {pickaxe_t r1   .  --pickaxe-regex  {mc Regular Expression}}
-- 
2.3.5

--
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] git-log: added --none-match option

[Junio C Hamano]

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

 Christoph Junghans ott...@gentoo.org writes:

 The only useful thing I could image is using it in conjunction with
 --files-with-matches, but that is what --files-without-match is for.

 Yes, -l was exactly what I had in mind and I was hoping that git
 grep -l --no-match -e WIP -e TODO -e FIXME -e NEEDSWORK may be a
 way to find perfect files without needing any work.

 You can say git grep -L -e WIP -e TODO -e FIXME -e NEEDSWORK
 instead.  I missed that -L option.

Thanks for your patience.  I should have realized that this not only
can be log-only but _should_ be log-only.  As you pointed out,
when we already have -L, trying to extend it to grep does not
make much sense.

Continuing this line of thought, as we determined that it is
pointless to have this at grep level and it is needed only in the
log family of commands, I would very much prefer the approach
taken by your original log --invert-grep patch.  I would further
say that I prefer not to touch grep_opt at all.

The new global-invert bit is about we'd run the usual grep thing
on the log message, and instead of filtering to only show the
commits with matching message, we only show the ones with messages
that do not match.  That logically belongs to the revision struct
that is used to interpret what the underlying grep machinery figured
out, and should not have to affect the way how underlying grep
machinery works.

We would want a few test to make sure that we do not break the
feature in the future changes.  Here is an attempt.  The patch would
apply any commit your original --invert-grep option would have
applied.

Thanks again.

-- 8 --
Subject: log: teach --invert-grep option

git log --grep=string shows only commits with messages that
match the given string, but sometimes it is useful to be able to
show only commits that do *not* have certain messages (e.g. show me
ones that are not FIXUP commits).

Signed-off-by: Christoph Junghans ott...@gentoo.org
Signed-off-by: Junio C Hamano gits...@pobox.com
---

 jc: Christoph originally had the invert-grep flag in grep_opt, but
 because git grep --invert-grep does not make sense except in
 conjunction with --files-with-matches, which is already
 covered by --files-without-matches, I moved it to revisions
 structure.  I think it expresses what the feature is about
 better to have the flag there.

 When the newly inserted two tests run, the history would have
 commits with messages initial, second, third, fourth,
 fifth, sixth and Second, committed in this order.  The
 first commit that does not match either th or Sec is
 second, and initial is the one that does not match either
 th or Sec case insensitively.

 Documentation/rev-list-options.txt |  4 
 contrib/completion/git-completion.bash |  2 +-
 revision.c |  4 +++-
 revision.h |  2 ++
 t/t4202-log.sh | 12 
 5 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/Documentation/rev-list-options.txt 
b/Documentation/rev-list-options.txt
index deb8cca..05aa997 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -66,6 +66,10 @@ if it is part of the log message.
Limit the commits output to ones that match all given `--grep`,
instead of ones that match at least one.
 
+--invert-grep::
+   Limit the commits output to ones with log message that do not
+   match the pattern specified with `--grep=pattern`.
+
 -i::
 --regexp-ignore-case::
Match the regular expression limiting patterns without regard to letter
diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 06bf262..53857f0 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1428,7 +1428,7 @@ __git_log_gitk_options=
 # Options that go well for log and shortlog (not gitk)
 __git_log_shortlog_options=
--author= --committer= --grep=
-   --all-match
+   --all-match --invert-grep
 
 
 __git_log_pretty_formats=oneline short medium full fuller email raw format:
diff --git a/revision.c b/revision.c
index 615535c..84b33a3 100644
--- a/revision.c
+++ b/revision.c
@@ -1952,6 +1952,8 @@ static int handle_revision_opt(struct rev_info *revs, int 
argc, const char **arg
grep_set_pattern_type_option(GREP_PATTERN_TYPE_PCRE, 
revs-grep_filter);
} else if (!strcmp(arg, --all-match)) {
revs-grep_filter.all_match = 1;
+   } else if (!strcmp(arg, --invert-grep)) {
+   revs-invert_grep = 1;
} else if ((argcount = parse_long_opt(encoding, argv, optarg))) {
if (strcmp(optarg, none))
git_log_output_encoding = xstrdup(optarg);
@@ -2848,7 +2850,7 @@ static int commit_match(struct commit *commit, struct 
rev_info *opt)
 (char

Re: [PATCH] git-log: added --none-match option

[Christoph Junghans]

2015-01-06 16:02 GMT-07:00 Junio C Hamano gits...@pobox.com:
 Christoph Junghans ott...@gentoo.org writes:

 Implements a inverted match for git log, like in the case of
 git grep -v, which is useful from time to time to e.g. filter
 FIXUP message out of git log.

 Internally, a new bol 'none_match' has been introduces as
 revs-grep_filter.invert inverts the match line-wise, which cannot
 work as i.e. empty line always not match the pattern given.

 Signed-off-by: Christoph Junghans ott...@gentoo.org
 ---

 The patch itself looks like a good start, except that the above
 description no longer matches the implementation.

 I further suspect it would be better to rename all_match to
 all_or_none and then you can lose the these two are mutually
 incompatible check that is placed together with a wrong existing
 comment.  I also notice that you forgot to update the git grep
 where the original --all-match came from.
That was on purpose. I am not quite sure what would be the point of
showing only matches from files that match no patterns (option
description from your patch below).
If a file matches none of the patterns, what matches are there to show?

The only useful thing I could image is using it in conjunction with
--files-with-matches, but that is what --files-without-match is for.


 A partial fix-up may start like this on top of your version.  By
 renaming the variable used in the existing code, the compiler will
 remind you that there are a few more places that your patch did not
 touch that does something special for --all-match, which are a good
 candidates you need to think if doing something similarly special
 for the --none-match case is necessary.

 Thanks.

 diff --git a/builtin/grep.c b/builtin/grep.c
 index 4063882..9ba4254 100644
 --- a/builtin/grep.c
 +++ b/builtin/grep.c
 @@ -727,8 +727,12 @@ int cmd_grep(int argc, const char **argv, const char 
 *prefix)
   close_callback },
 OPT__QUIET(opt.status_only,
N_(indicate hit with exit status without 
 output)),
 -   OPT_BOOL(0, all-match, opt.all_match,
 -   N_(show only matches from files that match all 
 patterns)),
 +   OPT_SET_INT(0, all-match, opt.all_or_none,
 +   N_(show only matches from files that match all 
 patterns),
 +   GREP_ALL_MATCH),
 +   OPT_SET_INT(0, none-match, opt.all_or_none,
 +   N_(show only matches from files that match no 
 patterns),
 +   GREP_NONE_MATCH),
 { OPTION_SET_INT, 0, debug, opt.debug, NULL,
   N_(show parse tree for grep expression),
   PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, NULL, 1 },
 diff --git a/grep.c b/grep.c
 index f486ee5..1ff5dea 100644
 --- a/grep.c
 +++ b/grep.c
 @@ -1622,14 +1622,18 @@ static int chk_hit_marker(struct grep_expr *x)

  int grep_source(struct grep_opt *opt, struct grep_source *gs)
  {
 -   if(opt-none_match)
 -   return !grep_source_1(opt, gs, 0);
 /*
  * we do not have to do the two-pass grep when we do not check
 -* buffer-wide all-match.
 +* buffer-wide all-match or none-match.
  */
 -   if (!opt-all_match)
 +   switch (opt-all_or_none) {
 +   case GREP_ALL_MATCH:
 return grep_source_1(opt, gs, 0);
 +   case GREP_NONE_MATCH:
 +   return !grep_source_1(opt, gs, 0);
 +   default:
 +   break;
 +   }

 /* Otherwise the toplevel or terms hit a bit differently.
  * We first clear hit markers from them.
 diff --git a/grep.h b/grep.h
 index 8e50c95..2cdabf2 100644
 --- a/grep.h
 +++ b/grep.h
 @@ -101,8 +101,9 @@ struct grep_opt {
 int count;
 int word_regexp;
 int fixed;
 -   int all_match;
 -   int none_match;
 +#define GREP_ALL_MATCH 1
 +#define GREP_NONE_MATCH 2
 +   int all_or_none;
 int debug;
  #define GREP_BINARY_DEFAULT0
  #define GREP_BINARY_NOMATCH1
 diff --git a/revision.c b/revision.c
 index d43779e..b955848 100644
 --- a/revision.c
 +++ b/revision.c
 @@ -2010,9 +2010,9 @@ static int handle_revision_opt(struct rev_info *revs, 
 int argc, const char **arg
 } else if (!strcmp(arg, --perl-regexp)) {
 grep_set_pattern_type_option(GREP_PATTERN_TYPE_PCRE, 
 revs-grep_filter);
 } else if (!strcmp(arg, --all-match)) {
 -   revs-grep_filter.all_match = 1;
 +   revs-grep_filter.all_or_none = GREP_ALL_MATCH;
 } else if (!strcmp(arg, --none-match)) {
 -   revs-grep_filter.none_match = 1;
 +   revs-grep_filter.all_or_none = GREP_NONE_MATCH;
 } else if ((argcount = parse_long_opt(encoding, argv, optarg))) {
 if (strcmp(optarg, none))
 git_log_output_encoding = xstrdup(optarg);
 @@ -2335,8 +2335,6 @@ int setup_revisions

Re: [PATCH] git-log: added --none-match option

[Junio C Hamano]

Christoph Junghans ott...@gentoo.org writes:

 The only useful thing I could image is using it in conjunction with
 --files-with-matches, but that is what --files-without-match is for.

Yes, -l was exactly what I had in mind and I was hoping that git
grep -l --no-match -e WIP -e TODO -e FIXME -e NEEDSWORK may be a
way to find perfect files without needing any work.

You can say git grep -L -e WIP -e TODO -e FIXME -e NEEDSWORK
instead.  I missed that -L option.

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

[PATCH] git-log: added --none-match option

[Christoph Junghans]

Implements a inverted match for git log, like in the case of
git grep -v, which is useful from time to time to e.g. filter
FIXUP message out of git log.

Internally, a new bol 'none_match' has been introduces as
revs-grep_filter.invert inverts the match line-wise, which cannot
work as i.e. empty line always not match the pattern given.

Signed-off-by: Christoph Junghans ott...@gentoo.org
---
 Documentation/rev-list-options.txt | 4 
 contrib/completion/git-completion.bash | 2 +-
 grep.c | 2 ++
 grep.h | 1 +
 revision.c | 4 
 5 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/Documentation/rev-list-options.txt 
b/Documentation/rev-list-options.txt
index afccfdc..08e4ed8 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -66,6 +66,10 @@ if it is part of the log message.
Limit the commits output to ones that match all given `--grep`,
instead of ones that match at least one.
 
+--none-match::
+   Limit the commits output to ones that do not match any of the 
+   given `--grep`, instead of ones that match at least one.
+
 -i::
 --regexp-ignore-case::
Match the regular expression limiting patterns without regard to letter
diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 23988ec..b0720e9 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1425,7 +1425,7 @@ __git_log_gitk_options=
 # Options that go well for log and shortlog (not gitk)
 __git_log_shortlog_options=
--author= --committer= --grep=
-   --all-match
+   --all-match --none-match
 
 
 __git_log_pretty_formats=oneline short medium full fuller email raw format:
diff --git a/grep.c b/grep.c
index 6e085f8..eadf8d9 100644
--- a/grep.c
+++ b/grep.c
@@ -1622,6 +1622,8 @@ static int chk_hit_marker(struct grep_expr *x)
 
 int grep_source(struct grep_opt *opt, struct grep_source *gs)
 {
+   if(opt-none_match)
+   return !grep_source_1(opt, gs, 0);  
/*
 * we do not have to do the two-pass grep when we do not check
 * buffer-wide all-match.
diff --git a/grep.h b/grep.h
index 95f197a..8e50c95 100644
--- a/grep.h
+++ b/grep.h
@@ -102,6 +102,7 @@ struct grep_opt {
int word_regexp;
int fixed;
int all_match;
+   int none_match;
int debug;
 #define GREP_BINARY_DEFAULT0
 #define GREP_BINARY_NOMATCH1
diff --git a/revision.c b/revision.c
index 75dda92..d43779e 100644
--- a/revision.c
+++ b/revision.c
@@ -2011,6 +2011,8 @@ static int handle_revision_opt(struct rev_info *revs, int 
argc, const char **arg
grep_set_pattern_type_option(GREP_PATTERN_TYPE_PCRE, 
revs-grep_filter);
} else if (!strcmp(arg, --all-match)) {
revs-grep_filter.all_match = 1;
+   } else if (!strcmp(arg, --none-match)) {
+   revs-grep_filter.none_match = 1;
} else if ((argcount = parse_long_opt(encoding, argv, optarg))) {
if (strcmp(optarg, none))
git_log_output_encoding = xstrdup(optarg);
@@ -2333,6 +2335,8 @@ int setup_revisions(int argc, const char **argv, struct 
rev_info *revs, struct s
die(cannot combine --walk-reflogs with --graph);
if (!revs-reflog_info  revs-grep_filter.use_reflog_filter)
die(cannot use --grep-reflog without --walk-reflogs);
+   if (revs-grep_filter.all_match  revs-grep_filter.none_match)
+   die(cannot combine --all-match with --none-match);
 
return left;
 }
-- 
2.0.5

--
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_CONFIG should either apply to all commands, or none at all

[Jeff King]

On Wed, Oct 01, 2014 at 06:15:46PM -0700, Jonathan Nieder wrote:

  3) Warn when 'git config' is called with GIT_CONFIG set, explaining
 that support will eventually be removed and that callers should
 pass --file= instead.
 
  4) Once we're confident there are no scripts in the wild relying on
 that envvar, remove support for it.

I think you could do just these two without worrying about the
I_AM_PORCELAIN setting. It's completely redundant with `git config
--file` these days.

 Another possibility (B):
 
  1) Teach git's commands in C to respect the GIT_CONFIG environment
 variable.  Semantics: only configuration from that file would be
 respected and all other configuration will be ignored.  Advertise
 it in the git(1) manpage.

I think this is a bad idea. It originally _did_ impact each command, but
there were a lot of confusing corner cases to the semantics, and it led
to bugs and misbehavior. That's what led to dc87183. I wish we had just
dropped it for git-config then, too. We kept it for backwards
compatibility, but we probably should have deprecated it more clearly.

 Yet another possibility (C):
 
  1) Just skip to step (4) from plan (A).

I agree this is tempting. We have never deprecated it formally, but it
has been a little-used feature.

 C is kind of temping.  Do you know if there are scripts in the wild
 that rely on the GIT_CONFIG setting working?

Searching here:

  https://github.com/search?q=%22export+GIT_CONFIG%22type=Code

reveals that some people do set it, but from the handful I investigated,
it is probably not doing what they want. For example, in:

  
https://github.com/GNOME/sysadmin-bin/blob/8ef4165a4b38fd1488c194f0c562c7fe24545bca/git/gnome-post-receive

they are trying to use it as if it affects all git commands, but as we
just discussed, it does not. So their script is potentially buggy as-is.
Getting rid of GIT_CONFIG would make it _more_ buggy, so perhaps that is
not an excuse, but I think it points to actually doing something.

-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_CONFIG should either apply to all commands, or none at all

[Junio C Hamano]

Jonathan Nieder jrnie...@gmail.com writes:

 Yep.  One possibility would be to do something like the following (A):

  1) advertise in the git-config(1) manpage that the GIT_CONFIG
 environment variable only affects the behavior of the 'git config'
 command

  2) introduce an environment variable GIT_I_AM_PORCELAIN.  (If doing
 this, we could come up with a better name, but this is just an
 illustration.)  Set and export that envvar in git-sh-setup.sh.
 When that environment variable is set, make git-config stop paying
 attention to GIT_CONFIG.

 That way, git commands that happen to be scripts would not be
 affected by the GIT_CONFIG setting any more.

At the places you plan to update porcelains to set and export
GIT_I_AM_PORCELAIN, you could unset GIT_CONFIG if set.  Would that
achieve the same goal?

And you can stop there without doing 3 or 4, 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: $GIT_CONFIG should either apply to all commands, or none at all

[Jonathan Nieder]

Hi,

Frédéric Brière wrote[1]:

 This kind of stuff caused me a lot of hair-pulling:

   $ git config core.abbrev
   32
   git log --pretty=oneline --abbrev-commit
   89be foo

 Here's the source of the discrepancy:

   $ grep abbrev $GIT_CONFIG .git/config
   git.conf:   abbrev=32
   .git/config:abbrev=4

 Since dc87183, $GIT_CONFIG is ignored by any other Git command, but it
 *still* applies to git-config.  This basically means that values
 obtained via git-config are not necessarily those which are actually in
 effect.

 The really frustrating part (for me, at least) is that for any tool
 (gitweb in my case) which uses git-config, values from $GIT_CONFIG will
 take effect for that tool, but not for any subsequent Git command.

 git-config(1) doesn't make this clear either; it mentions $GIT_CONFIG as
 the configuration, without saying explicitly that this environment
 variable only applies to git-config.

Yep.  One possibility would be to do something like the following (A):

 1) advertise in the git-config(1) manpage that the GIT_CONFIG
environment variable only affects the behavior of the 'git config'
command

 2) introduce an environment variable GIT_I_AM_PORCELAIN.  (If doing
this, we could come up with a better name, but this is just an
illustration.)  Set and export that envvar in git-sh-setup.sh.
When that environment variable is set, make git-config stop paying
attention to GIT_CONFIG.

That way, git commands that happen to be scripts would not be
affected by the GIT_CONFIG setting any more.

 3) Warn when 'git config' is called with GIT_CONFIG set, explaining
that support will eventually be removed and that callers should
pass --file= instead.

 4) Once we're confident there are no scripts in the wild relying on
that envvar, remove support for it.

Another possibility (B):

 1) Teach git's commands in C to respect the GIT_CONFIG environment
variable.  Semantics: only configuration from that file would be
respected and all other configuration will be ignored.  Advertise
it in the git(1) manpage.

 2) Gnash teeth a little but continue to support it.

Yet another possibility (C):

 1) Just skip to step (4) from plan (A).

C is kind of temping.  Do you know if there are scripts in the wild
that rely on the GIT_CONFIG setting working?

Thanks for reporting,
Jonathan

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

Re: [PATCH/RFC] log doc: explain --encoding=none and default output encoding

[Junio C Hamano]

Jonathan Nieder jrnie...@gmail.com writes:

 Signed-off-by: Jonathan Nieder jrnie...@gmail.com
 ---
 I'm not thrilled with the wording.  This can probably be explained
 more simply.  Ideas?

Some random thoughts on text, both before and after this patch.

 - The first stentence in this paragraph up to the semicolon is
   irrelevant (it is an implementation detail that allows us to
   re-encode in the first place, and the user does not care).

 - The use of word default is fuzzy.  With this description, we
   want to tell the user to what encoding we reencode to by default,
   but it is easy to miss that the reencoding always happen for
   output with or without --encoding=anything option (which is not
   clear from the text, especially the original) and incorrectly
   assume that without --encoding=anything the recorded text is
   spit out without mangling.

So perhaps along this line?

--encoding=encoding::

The encoding used to output the log messages in commit
objects.  When this option is not given, non-plumbing
commands output messages in the commit log encoding
(`i18n.commitEncoding`, or UTF-8 if the configuration
variable is not set).  `--encoding=none` lets you view the
raw log message without any reencoding.


  Documentation/pretty-options.txt | 10 --
  1 file changed, 8 insertions(+), 2 deletions(-)

 diff --git a/Documentation/pretty-options.txt 
 b/Documentation/pretty-options.txt
 index 5e499421..e31fd494 100644
 --- a/Documentation/pretty-options.txt
 +++ b/Documentation/pretty-options.txt
 @@ -32,8 +32,14 @@ people using 80-column terminals.
   The commit objects record the encoding used for the log message
   in their encoding header; this option can be used to tell the
   command to re-code the commit log message in the encoding
 - preferred by the user.  For non plumbing commands this
 - defaults to UTF-8.
 + preferred by the user.  --encoding=none means to use the
 + raw log message without paying attention to its encoding header.
 ++
 +For non plumbing commands, the output encoding defaults to the commit
 +encoding (as set using the `i18n.commitEncoding` variable, or UTF-8
 +by default).  This default can be overridden using the
 +`i18n.logOutputEncoding` configuration item. See linkgit:git-config[1]
 +for details.
  
  --notes[=ref]::
   Show the notes (see linkgit:git-notes[1]) that annotate the
--
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/RFC] log doc: explain --encoding=none and default output encoding

[Jonathan Nieder]

Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
I'm not thrilled with the wording.  This can probably be explained
more simply.  Ideas?

 Documentation/pretty-options.txt | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt
index 5e499421..e31fd494 100644
--- a/Documentation/pretty-options.txt
+++ b/Documentation/pretty-options.txt
@@ -32,8 +32,14 @@ people using 80-column terminals.
The commit objects record the encoding used for the log message
in their encoding header; this option can be used to tell the
command to re-code the commit log message in the encoding
-   preferred by the user.  For non plumbing commands this
-   defaults to UTF-8.
+   preferred by the user.  --encoding=none means to use the
+   raw log message without paying attention to its encoding header.
++
+For non plumbing commands, the output encoding defaults to the commit
+encoding (as set using the `i18n.commitEncoding` variable, or UTF-8
+by default).  This default can be overridden using the
+`i18n.logOutputEncoding` configuration item. See linkgit:git-config[1]
+for details.
 
 --notes[=ref]::
Show the notes (see linkgit:git-notes[1]) that annotate the
-- 
1.8.4.rc1

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

Re: [PATCH v4 2/2] mergetools/p4merge: create a base if none available

[Junio C Hamano]

Kevin Bracey ke...@bracey.fi writes:

 Minor change from v3: that version moved initialisation of src1 higher up,
 detaching it from its associated comment. This move was only required by
 earlier versions, so v4 leaves src1 in its original position.

The funny filename comment was from b539c5e8fbd3 (git-merge-one:
new merge world order., 2005-12-07) where the removed code just
before that new comment ended with:

merge $4 $orig $src2

(yes, we used to use merge program from the RCS suite).  The
comment refers to one of the bad side effect the old code used to
have and warns against such a practice, i.e. it was talking about
the code that no longer existed.

I think the two-line comment should simply go.

Given that, I _think_ it is OK to move the initialization of src1
next to that of src2; that may make the result easier to read.

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

[PATCH v4 2/2] mergetools/p4merge: create a base if none available

[Kevin Bracey]

Originally, with no base, Git gave P4Merge $LOCAL as a dummy base:

   p4merge $LOCAL $LOCAL $REMOTE $MERGED

Commit 0a0ec7bd changed this to:

   p4merge empty file $LOCAL $REMOTE $MERGED

to avoid the problem of being unable to save in some circumstances with
similar inputs.

Unfortunately this approach produces much worse results on differing
inputs. P4Merge really regards the blank file as the base, and once you
have just a couple of differences between the two branches you end up
with one a massive full-file conflict. The 3-way diff is not readable,
and you have to invoke difftool MERGE_HEAD HEAD manually to get a
useful view.

The original approach appears to have invoked special 2-way merge
behaviour in P4Merge that occurs only if the base filename is  or
equal to the left input.  You get a good visual comparison, and it does
not auto-resolve differences. (Normally if one branch matched the base,
it would autoresolve to the other branch).

But there appears to be no way of getting this 2-way behaviour and being
able to reliably save. Having base==left appears to be triggering other
assumptions. There are tricks the user can use to force the save icon
on, but it's not intuitive.

So we now follow a suggestion given in the original patch's discussion:
generate a virtual base, consisting of the lines common to the two
branches. This is the same as the technique used in resolve and octopus
merges, so we relocate that code to a shared function.

Note that if there are no differences at the same location, this
technique can lead to automatic resolution without conflict, combining
everything from the 2 files.  As with the other merges using this
technique, we assume the user will inspect the result before saving.

Signed-off-by: Kevin Bracey ke...@bracey.fi
Reviewed-by: David Aguilar dav...@gmail.com
---
Minor change from v3: that version moved initialisation of src1 higher up,
detaching it from its associated comment. This move was only required by
earlier versions, so v4 leaves src1 in its original position.

Added Reviewed-by footer.

 Documentation/git-sh-setup.txt |  6 ++
 git-merge-one-file.sh  | 18 +-
 git-sh-setup.sh| 12 
 mergetools/p4merge |  6 +-
 4 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/Documentation/git-sh-setup.txt b/Documentation/git-sh-setup.txt
index 6a9f66d..5d709d0 100644
--- a/Documentation/git-sh-setup.txt
+++ b/Documentation/git-sh-setup.txt
@@ -82,6 +82,12 @@ get_author_ident_from_commit::
outputs code for use with eval to set the GIT_AUTHOR_NAME,
GIT_AUTHOR_EMAIL and GIT_AUTHOR_DATE variables for a given commit.
 
+create_virtual_base::
+   modifies the first file so only lines in common with the
+   second file remain. If there is insufficient common material,
+   then the first file is left empty. The result is suitable
+   as a virtual base input for a 3-way merge.
+
 GIT
 ---
 Part of the linkgit:git[1] suite
diff --git a/git-merge-one-file.sh b/git-merge-one-file.sh
index 3373c04..255c07a 100755
--- a/git-merge-one-file.sh
+++ b/git-merge-one-file.sh
@@ -104,30 +104,22 @@ case ${1:-.}${2:-.}${3:-.} in
;;
esac
 
-   src2=`git-unpack-file $3`
+   src2=$(git-unpack-file $3)
case $1 in
'')
echo Added $4 in both, but differently.
-   # This extracts OUR file in $orig, and uses git apply to
-   # remove lines that are unique to ours.
-   orig=`git-unpack-file $2`
-   sz0=`wc -c $orig`
-   @@DIFF@@ -u -La/$orig -Lb/$orig $orig $src2 | git apply --no-add
-   sz1=`wc -c $orig`
-
-   # If we do not have enough common material, it is not
-   # worth trying two-file merge using common subsections.
-   expr $sz0 \ $sz1 \* 2 /dev/null || : $orig
+   orig=$(git-unpack-file $2)
+   create_virtual_base $orig $src2
;;
*)
echo Auto-merging $4
-   orig=`git-unpack-file $1`
+   orig=$(git-unpack-file $1)
;;
esac
 
# Be careful for funny filename such as -L in $4, which
# would confuse merge greatly.
-   src1=`git-unpack-file $2`
+   src1=$(git-unpack-file $2)
git merge-file $src1 $orig $src2
ret=$?
msg=
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 9cfbe7f..2f78359 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -249,6 +249,18 @@ clear_local_git_env() {
unset $(git rev-parse --local-env-vars)
 }
 
+# Generate a virtual base file for a two-file merge. Uses git apply to
+# remove lines from $1 that are not in $2, leaving only common lines.
+create_virtual_base() {
+   sz0=$(wc -c $1)
+   @@DIFF@@ -u -La/$1 -Lb/$1 $1 $2 | git apply --no-add
+   sz1=$(wc -c $1)
+
+   # If we do not have enough common material, it 

[PATCH v3 2/3] mergetools/p4merge: create a base if none available

[Kevin Bracey]

Originally, with no base, Git gave P4Merge $LOCAL as a dummy base:

   p4merge $LOCAL $LOCAL $REMOTE $MERGED

Commit 0a0ec7bd changed this to:

   p4merge empty file $LOCAL $REMOTE $MERGED

to avoid the problem of being unable to save in some circumstances with
similar inputs.

Unfortunately this approach produces much worse results on differing
inputs. P4Merge really regards the blank file as the base, and once you
have just a couple of differences between the two branches you end up
with one a massive full-file conflict. The 3-way diff is not readable,
and you have to invoke difftool MERGE_HEAD HEAD manually to get a
useful view.

The original approach appears to have invoked special 2-way merge
behaviour in P4Merge that occurs only if the base filename is  or
equal to the left input.  You get a good visual comparison, and it does
not auto-resolve differences. (Normally if one branch matched the base,
it would autoresolve to the other branch).

But there appears to be no way of getting this 2-way behaviour and being
able to reliably save. Having base==left appears to be triggering other
assumptions. There are tricks the user can use to force the save icon
on, but it's not intuitive.

So we now follow a suggestion given in the original patch's discussion:
generate a virtual base, consisting of the lines common to the two
branches. This is the same as the technique used in resolve and octopus
merges, so we relocate that code to a shared function.

Note that if there are no differences at the same location, this
technique can lead to automatic resolution without conflict, combining
everything from the 2 files.  As with the other merges using this
technique, we assume the user will inspect the result before saving.

Signed-off-by: Kevin Bracey ke...@bracey.fi
---
 Documentation/git-sh-setup.txt |  6 ++
 git-merge-one-file.sh  | 18 +-
 git-sh-setup.sh| 12 
 mergetools/p4merge |  6 +-
 4 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/Documentation/git-sh-setup.txt b/Documentation/git-sh-setup.txt
index 6a9f66d..5d709d0 100644
--- a/Documentation/git-sh-setup.txt
+++ b/Documentation/git-sh-setup.txt
@@ -82,6 +82,12 @@ get_author_ident_from_commit::
outputs code for use with eval to set the GIT_AUTHOR_NAME,
GIT_AUTHOR_EMAIL and GIT_AUTHOR_DATE variables for a given commit.
 
+create_virtual_base::
+   modifies the first file so only lines in common with the
+   second file remain. If there is insufficient common material,
+   then the first file is left empty. The result is suitable
+   as a virtual base input for a 3-way merge.
+
 GIT
 ---
 Part of the linkgit:git[1] suite
diff --git a/git-merge-one-file.sh b/git-merge-one-file.sh
index f612cb8..0f164e5 100755
--- a/git-merge-one-file.sh
+++ b/git-merge-one-file.sh
@@ -104,30 +104,22 @@ case ${1:-.}${2:-.}${3:-.} in
;;
esac
 
-   src2=`git-unpack-file $3`
+   src1=$(git-unpack-file $2)
+   src2=$(git-unpack-file $3)
case $1 in
'')
echo Added $4 in both, but differently.
-   # This extracts OUR file in $orig, and uses git apply to
-   # remove lines that are unique to ours.
-   orig=`git-unpack-file $2`
-   sz0=`wc -c $orig`
-   @@DIFF@@ -u -La/$orig -Lb/$orig $orig $src2 | git apply --no-add
-   sz1=`wc -c $orig`
-
-   # If we do not have enough common material, it is not
-   # worth trying two-file merge using common subsections.
-   expr $sz0 \ $sz1 \* 2 /dev/null || : $orig
+   orig=$(git-unpack-file $2)
+   create_virtual_base $orig $src2
;;
*)
echo Auto-merging $4
-   orig=`git-unpack-file $1`
+   orig=$(git-unpack-file $1)
;;
esac
 
# Be careful for funny filename such as -L in $4, which
# would confuse merge greatly.
-   src1=`git-unpack-file $2`
git merge-file $src1 $orig $src2
ret=$?
msg=
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 795edd2..349a5d4 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -249,6 +249,18 @@ clear_local_git_env() {
unset $(git rev-parse --local-env-vars)
 }
 
+# Generate a virtual base file for a two-file merge. Uses git apply to
+# remove lines from $1 that are not in $2, leaving only common lines.
+create_virtual_base() {
+   sz0=$(wc -c $1)
+   @@DIFF@@ -u -La/$1 -Lb/$1 $1 $2 | git apply --no-add
+   sz1=$(wc -c $1)
+
+   # If we do not have enough common material, it is not
+   # worth trying two-file merge using common subsections.
+   expr $sz0 \ $sz1 \* 2 /dev/null || : $1
+}
+
 
 # Platform specific tweaks to work around some commands
 case $(uname -s) in
diff --git a/mergetools/p4merge b/mergetools/p4merge
index 46b3a5a..5a608ab 

[PATCH v2 2/3] mergetools/p4merge: create a base if none available

[Kevin Bracey]

Originally, with no base, Git gave P4Merge $LOCAL as a dummy base:

   p4merge $LOCAL $LOCAL $REMOTE $MERGED

Commit 0a0ec7bd changed this to:

   p4merge empty file $LOCAL $REMOTE $MERGED

to avoid the problem of being unable to save in some circumstances with
similar inputs.

Unfortunately this approach produces much worse results on differing
inputs. P4Merge really regards the blank file as the base, and once you
have just a couple of differences between the two branches you end up
with one a massive full-file conflict. The 3-way diff is not readable,
and you have to invoke difftool MERGE_HEAD HEAD manually to get a
useful view.

The original approach appears to have invoked special 2-way merge
behaviour in P4Merge that occurs only if the base filename is  or
equal to the left input.  You get a good visual comparison, and it does
not auto-resolve differences. (Normally if one branch matched the base,
it would autoresolve to the other branch).

But there appears to be no way of getting this 2-way behaviour and being
able to reliably save. Having base==left appears to be triggering other
assumptions. There are tricks the user can use to force the save icon
on, but it's not intuitive.

So we now follow a suggestion given in the original patch's discussion:
generate a virtual base, consisting of the lines common to the two
branches. This is the same as the technique used in resolve and octopus
merges, so we relocate that code to a shared function.

Note that if there are no differences at the same location, this
technique can lead to automatic resolution without conflict, combining
everything from the 2 files.  As with the other merges using this
technique, we assume the user will inspect the result before saving.

Signed-off-by: Kevin Bracey ke...@bracey.fi
---
 git-merge-one-file.sh | 18 +-
 git-sh-setup.sh   | 13 +
 mergetools/p4merge|  6 +-
 3 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/git-merge-one-file.sh b/git-merge-one-file.sh
index f612cb8..1236fbf 100755
--- a/git-merge-one-file.sh
+++ b/git-merge-one-file.sh
@@ -104,30 +104,22 @@ case ${1:-.}${2:-.}${3:-.} in
;;
esac
 
-   src2=`git-unpack-file $3`
+   src1=$(git-unpack-file $2)
+   src2=$(git-unpack-file $3)
case $1 in
'')
echo Added $4 in both, but differently.
-   # This extracts OUR file in $orig, and uses git apply to
-   # remove lines that are unique to ours.
-   orig=`git-unpack-file $2`
-   sz0=`wc -c $orig`
-   @@DIFF@@ -u -La/$orig -Lb/$orig $orig $src2 | git apply --no-add
-   sz1=`wc -c $orig`
-
-   # If we do not have enough common material, it is not
-   # worth trying two-file merge using common subsections.
-   expr $sz0 \ $sz1 \* 2 /dev/null || : $orig
+   orig=$(git-unpack-file $2)
+   create_virtual_base $orig $src1 $src2
;;
*)
echo Auto-merging $4
-   orig=`git-unpack-file $1`
+   orig=$(git-unpack-file $1)
;;
esac
 
# Be careful for funny filename such as -L in $4, which
# would confuse merge greatly.
-   src1=`git-unpack-file $2`
git merge-file $src1 $orig $src2
ret=$?
msg=
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 795edd2..aa9a732 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -249,6 +249,19 @@ clear_local_git_env() {
unset $(git rev-parse --local-env-vars)
 }
 
+# Generate a virtual base file for a two-file merge. On entry the
+# base file $1 should be a copy of $2. Uses git apply to remove
+# lines from $1 that are not in $3, leaving only common lines.
+create_virtual_base() {
+   sz0=$(wc -c $1)
+   @@DIFF@@ -u -La/$1 -Lb/$1 $2 $3 | git apply --no-add
+   sz1=$(wc -c $1)
+
+   # If we do not have enough common material, it is not
+   # worth trying two-file merge using common subsections.
+   expr $sz0 \ $sz1 \* 2 /dev/null || : $1
+}
+
 
 # Platform specific tweaks to work around some commands
 case $(uname -s) in
diff --git a/mergetools/p4merge b/mergetools/p4merge
index 46b3a5a..16ae0cc 100644
--- a/mergetools/p4merge
+++ b/mergetools/p4merge
@@ -21,7 +21,11 @@ diff_cmd () {
 
 merge_cmd () {
touch $BACKUP
-   $base_present || $BASE
+   if ! $base_present
+   then
+   cp -- $LOCAL $BASE
+   create_virtual_base $BASE $LOCAL $REMOTE
+   fi
$merge_tool_path $BASE $REMOTE $LOCAL $MERGED
check_unchanged
 }
-- 
1.8.2.rc3.7.g77aeedb

--
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] p4merge: create a virtual base if none available

[Kevin Bracey]

On 07/03/2013 04:23, David Aguilar wrote:

On Wed, Mar 6, 2013 at 12:32 PM, Kevin Bracey ke...@bracey.fi wrote:

+make_virtual_base() {
+   # Copied from git-merge-one-file.sh.

I think the reasoning behind these patches is good.

How do we feel about this duplication?

Bad.

Should we make a common function in the git-sh-setup.sh,
or is it okay to have a slightly modified version of this
function in two places?
I'd prefer to have a common function, I just didn't know if there was 
somewhere appropriate to place it, available from both files. And I'm 
going to have to learn a bit more sh to get it right.

Also, the @@DIFF@@ string may not work here.
This is a template string that is replaced by the Makefile.


It does work in git-mergetool--lib.sh, but not in mergetools/p4merge.


We prefer $(command) instead of `command`.
These should be adjusted.

Can the same thing be accomplished using git diff --no-index
so that we do not need a dependency on an external diff command here?
Do these comments still apply if it's a common function in 
git-sh-setup.sh that git-one-merge-file.sh will use? I'm wary of 
layering violations.


Kevin


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

[PATCH 2/2] p4merge: create a virtual base if none available

[Kevin Bracey]

Originally, with no base, Git gave P4Merge $LOCAL as a dummy base:

   p4merge $LOCAL $LOCAL $REMOTE $MERGED

Commit 0a0ec7bd changed this to:

   p4merge empty file $LOCAL $REMOTE $MERGED

to avoid the problem of being unable to save in some circumstances.

Unfortunately this approach does not produce good results at all on
differing inputs. P4Merge really regards the blank file as the base, and
once you have just a couple of differences between the two branches you
end up with one a massive full-file conflict. The diff is not readable,
and you have to invoke difftool MERGE_HEAD HEAD manually to see a
2-way diff.

The original form appears to have invoked special 2-way comparison
behaviour that occurs only if the base filename is  or equal to the
left input.  You get a good diff, and it does not auto-resolve in one
direction or the other. (Normally if one branch equals the base, it
would autoresolve to the other branch).

But there appears to be no way of getting this 2-way behaviour and being
able to reliably save. Having base=left appears to be triggering other
assumptions. There are tricks the user can use to force the save icon
on, but it's not intuitive.

So we now follow a suggestion given in the original patch's discussion:
generate a virtual base, consisting of the lines common to the two
branches. It produces a much nicer 3-way diff view than either of the
original forms, and than I suspect other mergetools are managing.

Signed-off-by: Kevin Bracey ke...@bracey.fi
---
 git-mergetool--lib.sh | 14 ++
 mergetools/p4merge|  2 +-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index e338be5..5b60cf5 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -108,6 +108,20 @@ check_unchanged () {
fi
 }
 
+make_virtual_base() {
+   # Copied from git-merge-one-file.sh.
+   # This starts with $LOCAL, and uses git apply to
+   # remove lines that are not in $REMOTE.
+   cp -- $LOCAL $BASE
+   sz0=`wc -c $BASE`
+   @@DIFF@@ -u -La/$BASE -Lb/$BASE $BASE $REMOTE | git 
apply --no-add
+   sz1=`wc -c $BASE`
+
+   # If we do not have enough common material, it is not
+   # worth trying two-file merge using common subsections.
+   expr $sz0 \ $sz1 \* 2 /dev/null || : $BASE
+}
+
 valid_tool () {
setup_tool $1  return 0
cmd=$(get_merge_tool_cmd $1)
diff --git a/mergetools/p4merge b/mergetools/p4merge
index 46b3a5a..f0a893b 100644
--- a/mergetools/p4merge
+++ b/mergetools/p4merge
@@ -21,7 +21,7 @@ diff_cmd () {
 
 merge_cmd () {
touch $BACKUP
-   $base_present || $BASE
+   $base_present || make_virtual_base
$merge_tool_path $BASE $REMOTE $LOCAL $MERGED
check_unchanged
 }
-- 
1.8.2.rc2.5.g1a80410.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/2] p4merge: create a virtual base if none available

[David Aguilar]

On Wed, Mar 6, 2013 at 12:32 PM, Kevin Bracey ke...@bracey.fi wrote:
 Originally, with no base, Git gave P4Merge $LOCAL as a dummy base:

p4merge $LOCAL $LOCAL $REMOTE $MERGED

 Commit 0a0ec7bd changed this to:

p4merge empty file $LOCAL $REMOTE $MERGED

 to avoid the problem of being unable to save in some circumstances.

 Unfortunately this approach does not produce good results at all on
 differing inputs. P4Merge really regards the blank file as the base, and
 once you have just a couple of differences between the two branches you
 end up with one a massive full-file conflict. The diff is not readable,
 and you have to invoke difftool MERGE_HEAD HEAD manually to see a
 2-way diff.

 The original form appears to have invoked special 2-way comparison
 behaviour that occurs only if the base filename is  or equal to the
 left input.  You get a good diff, and it does not auto-resolve in one
 direction or the other. (Normally if one branch equals the base, it
 would autoresolve to the other branch).

 But there appears to be no way of getting this 2-way behaviour and being
 able to reliably save. Having base=left appears to be triggering other
 assumptions. There are tricks the user can use to force the save icon
 on, but it's not intuitive.

 So we now follow a suggestion given in the original patch's discussion:
 generate a virtual base, consisting of the lines common to the two
 branches. It produces a much nicer 3-way diff view than either of the
 original forms, and than I suspect other mergetools are managing.

 Signed-off-by: Kevin Bracey ke...@bracey.fi
 ---
  git-mergetool--lib.sh | 14 ++
  mergetools/p4merge|  2 +-
  2 files changed, 15 insertions(+), 1 deletion(-)

 diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
 index e338be5..5b60cf5 100644
 --- a/git-mergetool--lib.sh
 +++ b/git-mergetool--lib.sh
 @@ -108,6 +108,20 @@ check_unchanged () {
 fi
  }

 +make_virtual_base() {
 +   # Copied from git-merge-one-file.sh.

I think the reasoning behind these patches is good.

How do we feel about this duplication?
Should we make a common function in the git-sh-setup.sh,
or is it okay to have a slightly modified version of this
function in two places?

 +   # This starts with $LOCAL, and uses git apply to
 +   # remove lines that are not in $REMOTE.
 +   cp -- $LOCAL $BASE
 +   sz0=`wc -c $BASE`
 +   @@DIFF@@ -u -La/$BASE -Lb/$BASE $BASE $REMOTE | git 
 apply --no-add
 +   sz1=`wc -c $BASE`
 +
 +   # If we do not have enough common material, it is not
 +   # worth trying two-file merge using common subsections.
 +   expr $sz0 \ $sz1 \* 2 /dev/null || : $BASE
 +}
 +
  valid_tool () {
 setup_tool $1  return 0
 cmd=$(get_merge_tool_cmd $1)
 diff --git a/mergetools/p4merge b/mergetools/p4merge
 index 46b3a5a..f0a893b 100644
 --- a/mergetools/p4merge
 +++ b/mergetools/p4merge
 @@ -21,7 +21,7 @@ diff_cmd () {

  merge_cmd () {
 touch $BACKUP
 -   $base_present || $BASE
 +   $base_present || make_virtual_base
 $merge_tool_path $BASE $REMOTE $LOCAL $MERGED
 check_unchanged
  }
 --
 1.8.2.rc2.5.g1a80410.dirty




-- 
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: [PATCH 2/2] p4merge: create a virtual base if none available

[David Aguilar]

On Wed, Mar 6, 2013 at 12:32 PM, Kevin Bracey ke...@bracey.fi wrote:
 +make_virtual_base() {
 +   # Copied from git-merge-one-file.sh.
 +   # This starts with $LOCAL, and uses git apply to
 +   # remove lines that are not in $REMOTE.
 +   cp -- $LOCAL $BASE
 +   sz0=`wc -c $BASE`
 +   @@DIFF@@ -u -La/$BASE -Lb/$BASE $BASE $REMOTE | git 
 apply --no-add
 +   sz1=`wc -c $BASE`
 +
 +   # If we do not have enough common material, it is not
 +   # worth trying two-file merge using common subsections.
 +   expr $sz0 \ $sz1 \* 2 /dev/null || : $BASE
 +}

This seems to be indented deeper then the other functions
(or gmail is whitespace damaging my view).
Please use one hard tab to indent here.

We prefer $(command) instead of `command`.
These should be adjusted.

Also, the @@DIFF@@ string may not work here.
This is a template string that is replaced by the Makefile.

I don't think the tools in the mergetools/ directory go through
cmd_munge_script so this is not going to work as-is.

Can the same thing be accomplished using git diff --no-index
so that we do not need a dependency on an external diff command here?


I am not a regular p4merge user myself so I'll defer to others
on the cc: list for their thoughts.  It does seem like a good idea, though.
-- 
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: [PATCH 2/2] p4merge: create a virtual base if none available

[Junio C Hamano]

David Aguilar dav...@gmail.com writes:

 How do we feel about this duplication?
 Should we make a common function in the git-sh-setup.sh,
 or is it okay to have a slightly modified version of this
 function in two places?

It probably is a good idea to have it in one place.  That would also
solve the @@DIFF@@ replacement issue you noticed at the same time.
--
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 v6 09/16] remote-hg: fake bookmark when there's none

[Felipe Contreras]

Or at least no current bookmark.

Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 contrib/remote-helpers/git-remote-hg | 19 ++-
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/contrib/remote-helpers/git-remote-hg 
b/contrib/remote-helpers/git-remote-hg
index 9db4b7e..dbe309a 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -26,7 +26,7 @@ import urllib
 # git:
 # Sensible defaults for git.
 # hg bookmarks are exported as git branches, hg branches are prefixed
-# with 'branches/'.
+# with 'branches/', HEAD is a special case.
 #
 # hg:
 # Emulate hg-git.
@@ -430,12 +430,21 @@ def get_branch_tip(repo, branch):
 return heads[0]
 
 def list_head(repo, cur):
-global g_head
+global g_head, bmarks
 
 head = bookmarks.readcurrent(repo)
-if not head:
-return
-node = repo[head]
+if head:
+node = repo[head]
+else:
+# fake bookmark from current branch
+head = cur
+node = repo['.']
+if not node:
+return
+if head == 'default':
+head = 'master'
+bmarks[head] = node
+
 print @refs/heads/%s HEAD % head
 g_head = (head, node)
 
-- 
1.8.0

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

[PATCH v5 10/14] remote-hg: fake bookmark when there's none

[Felipe Contreras]

Or at least no current bookmark.

Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 contrib/remote-hg/git-remote-hg | 16 
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/contrib/remote-hg/git-remote-hg b/contrib/remote-hg/git-remote-hg
index c2efadf..c41ec95 100755
--- a/contrib/remote-hg/git-remote-hg
+++ b/contrib/remote-hg/git-remote-hg
@@ -422,12 +422,20 @@ def list_branch_head(repo, cur):
 g_head = (head, 'branches', repo[tip])
 
 def list_bookmark_head(repo, cur):
-global g_head
+global g_head, bmarks
 
 head = bookmarks.readcurrent(repo)
-if not head:
-return
-node = repo[head]
+if head:
+node = repo[head]
+else:
+# fake bookmark from current branch
+head = cur
+tip = get_branch_tip(repo, head)
+if not tip:
+return
+node = repo[tip]
+bmarks[head] = node
+
 print @refs/heads/%s HEAD % head
 g_head = (head, 'bookmarks', node)
 
-- 
1.8.0

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

[PATCH v4 10/13] remote-hg: fake bookmark when there's none

[Felipe Contreras]

Or at least no current bookmark.

Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 contrib/remote-hg/git-remote-hg | 16 
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/contrib/remote-hg/git-remote-hg b/contrib/remote-hg/git-remote-hg
index 3bb3192..e8e3791 100755
--- a/contrib/remote-hg/git-remote-hg
+++ b/contrib/remote-hg/git-remote-hg
@@ -419,12 +419,20 @@ def list_branch_head(repo, cur):
 g_head = (head, 'branches', repo[tip])
 
 def list_bookmark_head(repo, cur):
-global g_head
+global g_head, bmarks
 
 head = bookmarks.readcurrent(repo)
-if not head:
-return
-node = repo[head]
+if head:
+node = repo[head]
+else:
+# fake bookmark from current branch
+head = cur
+tip = get_branch_tip(repo, head)
+if not tip:
+return
+node = repo[tip]
+bmarks[head] = node
+
 print @refs/heads/%s HEAD % head
 g_head = (head, 'bookmarks', node)
 
-- 
1.8.0

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

libata: don't normalize UNKNOWN to NONE after reset

[Linux Kernel Mailing List]

Gitweb: 
http://git.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=4ccd3329a2e51473a86547a55f9e5f98f8f65b33
Commit: 4ccd3329a2e51473a86547a55f9e5f98f8f65b33
Parent: 2695e36616c3ece5e8e30666868fc7c90dc3fc75
Author: Tejun Heo [EMAIL PROTECTED]
AuthorDate: Tue Jan 8 20:26:12 2008 +0900
Committer:  Jeff Garzik [EMAIL PROTECTED]
CommitDate: Thu Jan 10 16:53:22 2008 -0500

libata: don't normalize UNKNOWN to NONE after reset

After non-classifying reset, ehc-classes[] could contain
ATA_DEV_UNKNOWN which used to be normalized to ATA_DEV_NONE for
consistency.  However, this causes unfortunate side effect for drivers
which have non-classifying hardresets (e.g. sata_nv) by making
hardreset report ATA_DEV_NONE for non-classifying resets and thus
makes EH believe that the port is unoccupied and recovery can be
skipped.  The end result is that after a device is swapped with
another one, the new device isn't attached after the old one is
detached.

This patch makes ata_eh_reset() not normalize UNKNOWN to NONE after
non-classifying resets.  This fixes the above problem.  As UNKNOWN and
NONE are handled differently by only EH hotplug logic, this doesn't
cause other behavior changes.

Signed-off-by: Tejun Heo [EMAIL PROTECTED]
Cc: Robert Hancock [EMAIL PROTECTED]
Signed-off-by: Jeff Garzik [EMAIL PROTECTED]
---
 drivers/ata/libata-eh.c |4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 74269ed..21a81cd 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -2167,13 +2167,11 @@ int ata_eh_reset(struct ata_link *link, int classify,
if (ata_link_offline(link))
continue;
 
-   /* apply class override and convert UNKNOWN to NONE */
+   /* apply class override */
if (lflags  ATA_LFLAG_ASSUME_ATA)
classes[dev-devno] = ATA_DEV_ATA;
else if (lflags  ATA_LFLAG_ASSUME_SEMB)
classes[dev-devno] = ATA_DEV_SEMB_UNSUP; /* not yet */
-   else if (classes[dev-devno] == ATA_DEV_UNKNOWN)
-   classes[dev-devno] = ATA_DEV_NONE;
}
 
/* record current link speed */
-
To unsubscribe from this list: send the line unsubscribe git-commits-head in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[CIFS] Make sec=none force an anonymous mount

[Linux Kernel Mailing List]

Gitweb: 
http://git.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=8426c39c1289765a11fc9b9523212ed368ceebd8
Commit: 8426c39c1289765a11fc9b9523212ed368ceebd8
Parent: 796e5661f6b6be1600b3ab47c61ce61cf3e7a353
Author: Jeff Layton [EMAIL PROTECTED]
AuthorDate: Sat May 5 03:27:49 2007 +
Committer:  Steve French [EMAIL PROTECTED]
CommitDate: Sat May 5 03:27:49 2007 +

[CIFS] Make sec=none force an anonymous mount

We had a customer report that attempting to make CIFS mount with a null
username (i.e. doing an anonymous mount) doesn't work. Looking through the
code, it looks like CIFS expects a NULL username from userspace in order
to trigger an anonymous mount. The mount.cifs code doesn't seem to ever
pass a null username to the kernel, however.

It looks also like the kernel can take a sec=none option, but it only seems
to look at it if the username is already NULL. This seems redundant and
effectively makes sec=none useless.

The following patch makes sec=none force an anonymous mount.

Signed-off-by: Steve French [EMAIL PROTECTED]
---
 fs/cifs/CHANGES   |4 +++-
 fs/cifs/README|   20 +---
 fs/cifs/connect.c |8 
 3 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/fs/cifs/CHANGES b/fs/cifs/CHANGES
index 62dcf63..a9b6bc5 100644
--- a/fs/cifs/CHANGES
+++ b/fs/cifs/CHANGES
@@ -6,7 +6,9 @@ handle DNS host to ipv6 name translation).  Accept override of 
uid or gid
 on mount even when Unix Extensions are negotiated (it used to be ignored
 when Unix Extensions were ignored).  This allows users to override the
 default uid and gid for files when they are certain that the uids or
-gids on the server do not match those of the client.
+gids on the server do not match those of the client.  Make sec=none
+mount override username (so that null user connection is attempted)
+to match what documentation said.
 
 Version 1.48
 
diff --git a/fs/cifs/README b/fs/cifs/README
index 93fe359..80fcfb8 100644
--- a/fs/cifs/README
+++ b/fs/cifs/README
@@ -472,7 +472,7 @@ including:
-V  print mount.cifs version
-?  display simple usage information
 
-With recent 2.6 kernel versions of modutils, the version of the cifs kernel
+With most 2.6 kernel versions of modutils, the version of the cifs kernel
 module can be displayed via modinfo.
 
 Misc /proc/fs/cifs Flags and Debug Info
@@ -521,8 +521,22 @@ SecurityFlags  Flags which control security 
negotiation and
must use plaintext passwords0x20020
(reserved for future packet encryption) 0x00040
 
-cifsFYIIf set to one, additional debug information is
-   logged to the system error log. (default 0)
+cifsFYIIf set to non-zero value, additional debug 
information
+   will be logged to the system error log.  This field
+   contains three flags controlling different classes of
+   debugging entries.  The maximum value it can be set
+   to is 7 which enables all debugging points (default 0).
+   Some debugging statements are not compiled into the
+   cifs kernel unless CONFIG_CIFS_DEBUG2 is enabled in the
+   kernel configuration. cifsFYI may be set to one or
+   nore of the following flags (7 sets them all):
+
+   log cifs informational messages 0x01
+   log return codes from cifs entry points 0x02
+   log slow responses (ie which take longer than one 
second)
+ CONFIG_CIFS_STATS2 must be enabled in .config 0x03
+   
+   
 traceSMB   If set to one, debug information is logged to the
system error log with the start of smb requests
and responses (default 0)
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index cf40e24..216fb62 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -1721,12 +1721,12 @@ cifs_mount(struct super_block *sb, struct cifs_sb_info 
*cifs_sb,
return -EINVAL;
}
 
-   if (volume_info.username) {
+   if (volume_info.nullauth) {
+   cFYI(1,(null user));
+   volume_info.username = NULL;
+   } else if (volume_info.username) {
/* BB fixme parse for domain name here */
cFYI(1, (Username: %s , volume_info.username));
-
-   } else if (volume_info.nullauth) {
-   cFYI(1,(null user));
} else {
cifserror(No username specified);
 /* In userspace mount helper we can get user name from alternate
-
To unsubscribe from this list: send