Re: [PATCH v3] send-email: Improve format of smtp initialization error message

2014-12-30 Thread Alex Kuleshov
What's about output like this:

Unable to initialize SMTP properly. Check config and use --smtp-debug.

VALUES:
server=smtp.gmail.com
encryption=
hello=localhost.localdomain
port=587

Junio C Hamano  @ 2014-12-30 00:50 QYZT:

> Alexander Kuleshov  writes:
>
>> Signed-off-by: Alexander Kuleshov 
>> ---
>>  git-send-email.perl | 8 
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/git-send-email.perl b/git-send-email.perl
>> index 82c6fea..60dcd8d 100755
>> --- a/git-send-email.perl
>> +++ b/git-send-email.perl
>> @@ -1275,10 +1275,10 @@ X-Mailer: git-send-email $gitversion
>>
>>  if (!$smtp) {
>>  die "Unable to initialize SMTP properly. Check config 
>> and use --smtp-debug. ",
>> -"VALUES: server=$smtp_server ",
>> -"encryption=$smtp_encryption ",
>> -"hello=$smtp_domain",
>> -defined $smtp_server_port ? " 
>> port=$smtp_server_port" : "";
>> +"\nVALUES: \n\tserver=$smtp_server ",
>> +"\n\tencryption=$smtp_encryption ",
>> +"\n\thello=$smtp_domain",
>> +defined $smtp_server_port ? " 
>> \n\tport=$smtp_server_port" : "";
>
> It may be a good convention to have LF at the beginning of a new
> string (i.e. we terminate the old line only when we have something
> more to say), but that is true only when we want to end the sentence
> without the final newline.  I wonder if that is true in this case;
> do we want perl to say "at line # in file X" at the end?
>
> In any case, you have two output lines that ends with a trailing SP
> just before LF, which is probably not what you wanted.
>
> If we want to see all lines end with LF, it may be far easier to
> read this way:
>
>   die "msg\n",
> "\tvar1=val1\n",
> "\tvar2=val2\n",
> defined $var3 ? "\tvar3=val3\n" : "";
>
> I dunno.

--
Best regards.
0xAX
--
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: Re*: [PATCH] change contract between system_path and it's callers

2014-11-24 Thread Alex Kuleshov

Junio C Hamano  @ 2014-11-25 01:33 ALMT:

>> -static const char *git_etc_gitattributes(void)
>> +static char *git_etc_gitattributes(void)
>
> Hmph, I think this should keep returning "const char *", as the
> caller is not expected to free the pointer or write into the memory
> held by the returned string.  The same applies to the change of the
> type in "struct git_config_source".
>
> The change in the semantics of system_path() made the "get once and
> keep returning it" this function does safer and correct, but this
> function itself did not change any behaviour from its caller's point
> of view.

Understand, will fix it.

>
>>  {
>> -static const char *system_wide;
>> +static char *system_wide;
>>  if (!system_wide)
>>  system_wide = system_path(ETC_GITATTRIBUTES);
>>  return system_wide;
>
>
>> @@ -489,7 +489,9 @@ static void bootstrap_attr_stack(void)
>>  attr_stack = elem;
>>
>>  if (git_attr_system()) {
>> -elem = read_attr_from_file(git_etc_gitattributes(), 1);
>> +char *etc_attributes = git_etc_gitattributes();
>> +elem = read_attr_from_file(etc_attributes, 1);
>> +free(etc_attributes);
>
> And freeing here is actively wrong, I think.  You are freeing the
> piece of memory still pointed by "static char *system_wide" in the
> function git_etc_gitattributes(); when it is called again, the
> caller will get a pointer into the memory you have freed here.

Why? If i understand correctly we don't use etc_attributes anymore in
this function and if we'll call this function again
git_etc_gitattributes will create new pointer and system_path alloc
memory for it or i'm wrong with it?

>
>> diff --git a/builtin/config.c b/builtin/config.c
>> index 15a7bea..266d42b 100644
>> --- a/builtin/config.c
>> +++ b/builtin/config.c
>> @@ -575,8 +575,14 @@ int cmd_config(int argc, const char **argv, const char 
>> *prefix)
>>  if (given_config_source.blob)
>>  die("editing blobs is not supported");
>>  git_config(git_default_config, NULL);
>> -config_file = xstrdup(given_config_source.file ?
>> -  given_config_source.file : 
>> git_path("config"));
>> +
>> +if (use_system_config)
>> +config_file = given_config_source.file ?
>> +
>> given_config_source.file : git_path("config");
>> +else
>> +config_file = xstrdup(given_config_source.file ?
>> +  
>> given_config_source.file : git_path("config"));
>> +
>
> Sorry, but I do not understand this change.
>
> Why do you need "if use-system-config, do not allocate" here and
> then later everywhere "if use-system-config, free it"?  I would
> understand if the change were "earlier we had mixture of borrowed
> and owned strings, but we make sure we always own the string by
> running xstrdup() in the caller when we used to let this function
> borrow, so that we can always free() before returning from here",
> or something.
>
> For example, in the original code, use_local_config codepath uses
> git_pathdup(), which is an allocated piece of memory, to initialize
> given_config_source.file.  Doesn't it need to be freed?
>
> Even if it is not in the use_system_config mode, if
> given_config_source.file is not an absolute path, we store a copy of
> prefix-filename result to given_config_source.file.  Doesn't it need
> to be freed?
>
> I think the implementation strategy you took makes the result
> unnecessarily messy and error prone.  Let's step back a bit.
>
> When you have code that sometimes borrows memory and sometimes owns
> memory, writing the clean-up part like this is error prone:
>
>   char *var;
>
>   if (A)
>   var = borrowed memory;
>   else if (B)
>   var = borrowed memory;
>   else if (C)
>   var = xstrdup(borrowed memory);
>   else
>   var = borrowed memory;
>
>   use var; /* a loong piece of code in reality */
>
>   if (C)
>   free(var);
>
> because the set-up part can and will change over time as the code
> evolves.  If written this way:
>
>   char *var, *to_free = NULL;
>
>   if (A)
>   var = borrowed memory;
>   else if (B)
>   var = borrowed memory;
>   else if (C)
>   to_free = var = xstrdup(borrowed memory);
>   else
>   var = borrowed memory;
>
>   use var; /* a loong piece of code in reality */
>
>   free(to_free);
>
> the clean-up part would not have to worry how the set-up part
> decided when to borrow memory and when to own memory.  Another way
> to do so would obviously be:
>
>   char *var;
>
>   if (A)
>   var = xstrdup(borrowed memory);
>   else if (B)
>   var = xstrdup(borrowed memory);
>   else if (C)
>   

Re: [PATCH] exec_cmd: system_path memory leak fix

2014-11-24 Thread Alex Kuleshov

Now system_path returns path which is allocated string to callers;
It prevents memory leaks in some places. All callers of system_path
are owners of path string and they must release it.

Added new parameter to wrapper.c/int access_or_die - etc_config, because
only etc_config in this case use system_path and we don't know do we
need to free path or not.

Signed-off-by: Alexander Kuleshov 
---
 attr.c|  8 +++---
 builtin/config.c  | 73 +--
 builtin/help.c| 30 ---
 builtin/init-db.c | 12 +++--
 cache.h   |  4 +--
 config.c  | 19 ---
 exec_cmd.c| 22 -
 exec_cmd.h|  4 +--
 git-compat-util.h |  2 +-
 git.c | 16 +---
 wrapper.c |  7 --
 11 files changed, 134 insertions(+), 63 deletions(-)

diff --git a/attr.c b/attr.c
index cd54697..f96ddb4 100644
--- a/attr.c
+++ b/attr.c
@@ -462,9 +462,9 @@ static void drop_attr_stack(void)
}
 }

-static const char *git_etc_gitattributes(void)
+static char *git_etc_gitattributes(void)
 {
-   static const char *system_wide;
+   static char *system_wide;
if (!system_wide)
system_wide = system_path(ETC_GITATTRIBUTES);
return system_wide;
@@ -489,7 +489,9 @@ static void bootstrap_attr_stack(void)
attr_stack = elem;

if (git_attr_system()) {
-   elem = read_attr_from_file(git_etc_gitattributes(), 1);
+   char *etc_attributes = git_etc_gitattributes();
+   elem = read_attr_from_file(etc_attributes, 1);
+   free(etc_attributes);
if (elem) {
elem->origin = NULL;
elem->prev = attr_stack;
diff --git a/builtin/config.c b/builtin/config.c
index 15a7bea..266d42b 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -575,8 +575,14 @@ int cmd_config(int argc, const char **argv, const char 
*prefix)
if (given_config_source.blob)
die("editing blobs is not supported");
git_config(git_default_config, NULL);
-   config_file = xstrdup(given_config_source.file ?
- given_config_source.file : 
git_path("config"));
+
+   if (use_system_config)
+   config_file = given_config_source.file ?
+   
given_config_source.file : git_path("config");
+   else
+   config_file = xstrdup(given_config_source.file ?
+ 
given_config_source.file : git_path("config"));
+
if (use_global_config) {
int fd = open(config_file, O_CREAT | O_EXCL | O_WRONLY, 
0666);
if (fd) {
@@ -600,29 +606,43 @@ int cmd_config(int argc, const char **argv, const char 
*prefix)
if (ret == CONFIG_NOTHING_SET)
error("cannot overwrite multiple values with a single 
value\n"
"   Use a regexp, --add or --replace-all to change 
%s.", argv[0]);
+
+   if (use_system_config)
+   free(given_config_source.file);
return ret;
}
else if (actions == ACTION_SET_ALL) {
+   int ret;
check_write();
check_argc(argc, 2, 3);
value = normalize_value(argv[0], argv[1]);
-   return git_config_set_multivar_in_file(given_config_source.file,
-  argv[0], value, argv[2], 
0);
+   ret = git_config_set_multivar_in_file(given_config_source.file,
+   
  argv[0], value, argv[2], 0);
+   if (use_system_config)
+   free(given_config_source.file);
+   return ret;
}
else if (actions == ACTION_ADD) {
+   int ret;
check_write();
check_argc(argc, 2, 2);
value = normalize_value(argv[0], argv[1]);
-   return git_config_set_multivar_in_file(given_config_source.file,
-  argv[0], value,
-  CONFIG_REGEX_NONE, 0);
+   ret = git_config_set_multivar_in_file(given_config_source.file, 
argv[0],
+   
  value, CONFIG_REGEX_NONE, 0);
+   if (use_system_config)
+   free(given_config_source.file);
+   return ret;
}
else if (actions == ACTION_REPLACE_ALL) {
+   int ret;
check_write();
check_argc(argc, 2, 3);
value = no

Re: [PATCH] exec_cmd: system_path memory leak fix

2014-11-24 Thread Alex Kuleshov

Junio C Hamano  @ 2014-11-24 13:37 ALMT:

> [jc: added those who were mentioned but were missing back to Cc]
>
> On Sun, Nov 23, 2014 at 11:02 PM, Alex Kuleshov  
> wrote:
>>
>> Junio C Hamano:
>>
>>>Fixing these callers are done as separate patches, that can be
>>>applied either before or after this patch.
>>
>> How to do it better? Update this patch, fix all callers which broken and
>> concat this patches to one or make separate patches?
>
> As I said, I do not think the approach your v2 takes is better than the 
> original
> approach to pass the ownership of the returned value to the caller. I'd say 
> that
> a cleaned up v1 that makes sure it adds a necessary strdup() in the codepath
> where it returns an absolute pathname given as-is, with necessary changes to
> callers that do not currently free the received result to free it when they 
> are
> done, and to callers that currently do strdup() of the received result not to 
> do
> strdup(), in a single patch, would be the right thing to do.
>
> I think I already wrote the bulk of proposed commit message for you for such
> a change earlier ;-) The one that talks about changing the contract between 
> the
> system_path() and its callers.
>
> Thanks.

OK, but i'm little confused now, long thread :)

So what's our next strategy?

I'll make system_path will return volatile value, so callers must free
it by itself, but we have two types of callers:

Callers which doesn't need to store system_path return value (like git
--man-path and etc.. in git.c), and which need to store it (as in attr.c).

We must to free system_path return value in first case, and does not
need to create copy of returned string with xstrdup.

Am i correct or i'm wrong with this?

Thank you.

--
Best regards.
0xAX
--
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] exec_cmd: system_path memory leak fix

2014-11-23 Thread Alex Kuleshov

Jeff King:


>If I am reading this right, calls to system_path() will always reuse the
>same buffer, even if they are called with another "path" argument. So
>all callers must make sure to make a copy if they are going to hold on
>to it for a long time. Grepping for callers shows us saving the result
>to a static variable in at least git_etc_gitattributes, copy_templates,
>and get_html_page_path. Don't these all need to learn to xstrdup the
>return value?

Hello Jeff, yes as i wrote in previous message i saw that there some
places uses system_path function and I just wanted to find correct way
to solve problem with system_path, in near time i'll check and adapt if
need all of this places for updated system_path.

Eric Sunshine:

>Curious. Did the unit tests pass with this change?

Yes i launched unit tests and there are the same result as in origin/pu.

>Not sure what this change is about. The last couple lines of this function are:
>
>setenv("PATH", new_path.buf, 1);
>strbuf_release(&new_path);
>
>which means that the buffer held by the strbuf is being released
>anyhow, whether static or not.

Ah, yes, just a type, will update it.

Junio C Hamano:

>Fixing these callers are done as separate patches, that can be
>applied either before or after this patch.

How to do it better? Update this patch, fix all callers which broken and
concat this patches to one or make separate patches?

Thanks all for feedback.

--
Best regards.
0xAX
--
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] exec_cmd: system_path memory leak fix

2014-11-23 Thread Alex Kuleshov

Signed-off-by: Alex Kuleshov 
---
 exec_cmd.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/exec_cmd.c b/exec_cmd.c
index 698e752..7ed9bcc 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -13,7 +13,7 @@ const char *system_path(const char *path)
 #else
static const char *prefix = PREFIX;
 #endif
-   struct strbuf d = STRBUF_INIT;
+   static struct strbuf d = STRBUF_INIT;

if (is_absolute_path(path))
return path;
@@ -34,8 +34,7 @@ const char *system_path(const char *path)
 #endif

strbuf_addf(&d, "%s/%s", prefix, path);
-   path = strbuf_detach(&d, NULL);
-   return path;
+   return d.buf;
 }

 const char *git_extract_argv0_path(const char *argv0)
@@ -94,7 +93,7 @@ static void add_path(struct strbuf *out, const char *path)
 void setup_path(void)
 {
const char *old_path = getenv("PATH");
-   struct strbuf new_path = STRBUF_INIT;
+   static struct strbuf new_path = STRBUF_INIT;

add_path(&new_path, git_exec_path());
add_path(&new_path, argv0_path);
--
2.2.0.rc3.190.g952f0aa.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] exec_cmd: system_path memory leak fix

2014-11-23 Thread Alex Kuleshov

Junio C Hamano  @ 2014-11-24 00:51 ALMT:

> 0xAX  writes:
>
>> Signed-off-by: 0xAX 
>> ---
>
> The comment on names I've already mentioned elsewhere.

Yes, i understand about names.

>
> You need a better explanation than a "no log message", as you are
> not doing "system-path memory leak fix".
>
> You are doing a lot more.  Perhaps the story would start like this:
>
> system_path(): make the callers own the returned string

Did it.

>
> The function sometimes returns a newly allocated string and
> sometimes returns a borrowed string, the latter of which the
> callers must not free().
>
> The existing callers all assume that the return value belongs to
> the callee and most of them copy it with strdup() when they want
> to keep it around.  They end up leaking the returned copy when
> the callee returned a new string.
>
> Change the contract between the callers and system_path() to
> make the returned string owned by the callers; they are
> responsible for freeing it when done, but they do not have to
> make their own copy to store it away.

Yes you're right, i just started to read git source code some days ago,
and it's hard to understand in some places for the start. Now i see it,
thanks for explanation.

>
> This accidentally fixes some unsafe callers as well.  For
> example, ...
>
>
>>  exec_cmd.c | 25 -
>>  exec_cmd.h |  4 ++--
>>  git.c  | 12 +---
>
> Even though I said that this changes the contract between the caller
> and the callee and make things wasteful for some, I personally think
> it is going in the right direction.
>
> The change accidentally fixes some unsafe callers.  For example, the
> first hit from "git grep system_path" is this:
>
> attr.c- static const char *system_wide;
> attr.c- if (!system_wide)
> attr.c: system_wide = system_path(ETC_GITATTRIBUTES);
> attr.c- return system_wide;
>
> This is obviously unsafe for a volatile return value from the callee
> and needs to have strdup() on it, but with the patch there no longer
> is need for such a caller-side strdup().
>
> But this change also introduces new bugs, I think.  For example, the
> second hit from "git grep system_path" is this:
>
>   builtin/help.c: strbuf_addstr(&new_path, system_path(GIT_MAN_PATH));
>
> Now the caller owns and is responsible for freeing the returned
> value, but without opening the file in question in an editor or a
> pager we can tell immediately that there is no way this line is not
> adding a new memory leak.
>
>> index 698e752..08f8f80 100644
>> --- a/exec_cmd.c
>> +++ b/exec_cmd.c
>> @@ -6,7 +6,7 @@
>>  static const char *argv_exec_path;
>>  static const char *argv0_path;
>>
>> -const char *system_path(const char *path)
>> +char *system_path(const char *path)
>>  {
>>  #ifdef RUNTIME_PREFIX
>>  static const char *prefix;
>> @@ -14,9 +14,10 @@ const char *system_path(const char *path)
>>  static const char *prefix = PREFIX;
>>  #endif
>>  struct strbuf d = STRBUF_INIT;
>> +char *new_path = NULL;
>>
>>  if (is_absolute_path(path))
>> -return path;
>> +return strdup(path);
>>
>>  #ifdef RUNTIME_PREFIX
>>  assert(argv0_path);
>> @@ -32,10 +33,13 @@ const char *system_path(const char *path)
>>  "Using static fallback '%s'.\n", prefix);
>>  }
>>  #endif
>> -
>>  strbuf_addf(&d, "%s/%s", prefix, path);
>> -path = strbuf_detach(&d, NULL);
>> -return path;
>> +new_path = malloc((strlen(prefix) + strlen(path)) + 2);
>> +sprintf(new_path, "%s/%s", prefix, path);
>> +
>> +strbuf_release(&d);
>> +
>> +return new_path;
>
> Are you duplicating what strbuf_addf() is doing on the strbuf d,
> manually creating the same in new_path, while discarding what the
> existing code you did not remove with this patch already computed?
>
> Isn't it sufficient to add strdup(path) for the absolute case and do
> nothing else to this function?  I have no idea what you are doing
> here.

I have added changes from your previous feedback, how can I attach
second (changed) patch to this mail thread with git send-email?

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


Re: [PATCH 1/1] gitk: po/ru.po russian translation typo fixed

2014-11-17 Thread Alex Kuleshov

Hello Max and Paul,

thank you for your feedback, so what's must be my next workflow? Resend
patch with "Reviewed-By:..." or somethine else?

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


Re: [PATCH 1/1] git-config: git-config --list fixed when GIT_CONFIG value starts with ~/

2014-11-14 Thread Alex Kuleshov

Hello Eric and Jeff,

>Eric Sunshine
>A few issues:
>
>(1) Style: s/char* /char */
>
>(2) Avoid declaration (of 'newpath') after statement.
>
>(3) You can drop 'newpath' altogether and just assign the result of
>expand_user_path() directly to given_config_source.file.
>
>This code is potentially leaking the old value of
>given_config_source.file, and (later) the new value, however, as
>given_config_source.file is already being leaked elsewhere, it
>probably does not make the situation much worse.

It is my first patch to git's code base, so many thanks for your
feedback, i'll fix these issues if there will be need in this patch.

>Jeff King 
>
> Yeah, I'd agree it is a little unexpected to expand here. The "~" is
> mostly a shell thing, and doing:
>
>   GIT_CONFIG=~/.gitconfig git config --list
>
> from the shell generally works, because the shell will expand the "~"
> before it even hits git. If you're not using a shell to set the
> variable, you probably should be pre-expanding it yourself.

Yes, you're right here, but i put GIT_CONFIG=~/.gitconfig to my .bashrc
and it doesn't work so.

>
> Note that this code path affects "git config --file=~/.gitconfig", too.
> At least there it would be a little bit useful because the shell will
> not expand for you, but it still feels a bit unconventional to me.
>
>> >  builtin/config.c | 2 ++
>> >  1 file changed, 2 insertions(+)
>> >
>> > diff --git a/builtin/config.c b/builtin/config.c
>> > index 7bba516..df1bee0 100644
>> > --- a/builtin/config.c
>> > +++ b/builtin/config.c
>> > @@ -540,6 +540,8 @@ int cmd_config(int argc, const char **argv, const char 
>> > *prefix)
>> >
>> > if (actions == ACTION_LIST) {
>> > check_argc(argc, 0, 0);
>> > +   const char* newpath = 
>> > expand_user_path(given_config_source.file);
>> > +   given_config_source.file = newpath;
>
> If we _were_ going to do such an expansion, this is absolutely the wrong
> place for it. It works only for the "--list" action; if we are going to
> expand it, we would want to do so everywhere. And we do not even know if
> given_config_source.file is non-NULL here (we could be reading from
> stdin, or a blob). Fortunately expand_user_path will pass through a NULL
> without segfaulting.
>
> Probably the right place would be the if/else chain around
> builtin/config.c:514, where we convert a relative path into an absolute
> one. But I'm not convinced it's a good thing to be doing in the first
> place.
>
> -Peff

What if we'll put path expanding right after getting value of file path,
after given_config_source.file = getenv(CONFIG_ENVIRONMENT); at 451?

Thank you.

--
Best regards.
0xAX
--
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: t9902-completion.sh failed

2014-11-13 Thread Alex Kuleshov

ah, i catched the problem, I launched make test with sudo and now all
tests passed successfully.

Jeff King  @ 2014-11-13 17:24 ALMT:

> On Thu, Nov 13, 2014 at 04:59:12PM +0600, Alex Kuleshov wrote:
>
>> i just got git from master (f6f61cbbad0611e03b712cc354f1665b5d7b087e),
>> built and installed it successfully, now i'm running make test and got
>> following error:
>>
>> *** t9902-completion.sh ***
>> t9902-completion.sh: 118:
>> /home/shk/dev/git/t/../contrib/completion/git-completion.bash: Syntax
>> error: "(" unexpected (expecting "fi")
>> FATAL: Unexpected exit with code 2
>> make[2]: *** [t9902-completion.sh] Error 1
>> make[2]: Leaving directory `/home/shk/dev/git/t'
>> make[1]: *** [test] Error 2
>> make[1]: Leaving directory `/home/shk/dev/git/t'
>> make: *** [test] Error 2
>>
>> $ bash --version
>> 4.3.11(1)-release (x86_64-pc-linux-gnu)
>
> Weird. I can't reproduce here, using the version of bash from Debian
> unstable (4.3.30(1)), nor compiling 4.3.11 from sources. What platform
> are you on (i.e., might it be bash + some other patches installed by the
> distro)?
>
> -Peff

--
Best regards.
0xAX
--
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: t9902-completion.sh failed

2014-11-13 Thread Alex Kuleshov

Hello Jeff,

I'm using ubuntu 14.04 x86_64 and bash GNU bash, version
4.3.11(1)-release (x86_64-pc-linux-gnu).

I didn't applied any patches to bash for all time since i installed
system. so it reall weird


Jeff King  @ 2014-11-13 17:24 ALMT:

> On Thu, Nov 13, 2014 at 04:59:12PM +0600, Alex Kuleshov wrote:
>
>> i just got git from master (f6f61cbbad0611e03b712cc354f1665b5d7b087e),
>> built and installed it successfully, now i'm running make test and got
>> following error:
>>
>> *** t9902-completion.sh ***
>> t9902-completion.sh: 118:
>> /home/shk/dev/git/t/../contrib/completion/git-completion.bash: Syntax
>> error: "(" unexpected (expecting "fi")
>> FATAL: Unexpected exit with code 2
>> make[2]: *** [t9902-completion.sh] Error 1
>> make[2]: Leaving directory `/home/shk/dev/git/t'
>> make[1]: *** [test] Error 2
>> make[1]: Leaving directory `/home/shk/dev/git/t'
>> make: *** [test] Error 2
>>
>> $ bash --version
>> 4.3.11(1)-release (x86_64-pc-linux-gnu)
>
> Weird. I can't reproduce here, using the version of bash from Debian
> unstable (4.3.30(1)), nor compiling 4.3.11 from sources. What platform
> are you on (i.e., might it be bash + some other patches installed by the
> distro)?
>
> -Peff

--
Best regards.
0xAX
--
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


t9902-completion.sh failed

2014-11-13 Thread Alex Kuleshov

Hello all,

i just got git from master (f6f61cbbad0611e03b712cc354f1665b5d7b087e),
built and installed it successfully, now i'm running make test and got
following error:

*** t9902-completion.sh ***
t9902-completion.sh: 118:
/home/shk/dev/git/t/../contrib/completion/git-completion.bash: Syntax
error: "(" unexpected (expecting "fi")
FATAL: Unexpected exit with code 2
make[2]: *** [t9902-completion.sh] Error 1
make[2]: Leaving directory `/home/shk/dev/git/t'
make[1]: *** [test] Error 2
make[1]: Leaving directory `/home/shk/dev/git/t'
make: *** [test] Error 2

$ bash --version
4.3.11(1)-release (x86_64-pc-linux-gnu)

--
Best regards.
0xAX
--
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