[PATCH v2 2/2] [GSoC] diff:use is_dot_or_dotdot() in code

[Brian Bourn]

From: Brian Bourn 

Subject: replace manual "."/".." check with is_dot_or_dotdot()

Signed-off-by: Brian Bourn 
---
Part 2 of my GSoC submission where the actual change is made
 diff-no-index.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/diff-no-index.c b/diff-no-index.c
index ec51106..c554691 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -15,6 +15,7 @@
 #include "log-tree.h"
 #include "builtin.h"
 #include "string-list.h"
+#include "dir.h"
 
 static int read_directory_contents(const char *path, struct string_list *list)
 {
@@ -25,7 +26,7 @@ static int read_directory_contents(const char *path, struct 
string_list *list)
return error("Could not open directory %s", path);
 
while ((e = readdir(dir)))
-   if (strcmp(".", e->d_name) && strcmp("..", e->d_name))
+   if (!is_dot_or_dotdot(e->d_name))
string_list_insert(list, e->d_name);
 
closedir(dir);
-- 
1.9.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 v2 1/2] [GSoC] diff: rename read_directory()

[Brian Bourn]

From: Brian Bourn 

It is desirable to replace manual checking of "." or ".."
in diff-no-index.c with is_dot_or_dotdot(), which is defined
in dir.h. However, dir.h declares a read_directory which conflicts
with a (different) static read_directory() defined in
in diff-no-index.c. As a preparatory step, rename the local
read_directory() to avoid the collision

Signed-off-by: Brian Bourn 
---
Part 1 of my GSoC submission
 diff-no-index.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/diff-no-index.c b/diff-no-index.c
index 8e10bff..ec51106 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -16,7 +16,7 @@
 #include "builtin.h"
 #include "string-list.h"
 
-static int read_directory(const char *path, struct string_list *list)
+static int read_directory_contents(const char *path, struct string_list *list)
 {
DIR *dir;
struct dirent *e;
@@ -107,9 +107,9 @@ static int queue_diff(struct diff_options *o,
int i1, i2, ret = 0;
size_t len1 = 0, len2 = 0;
 
-   if (name1 && read_directory(name1, &p1))
+   if (name1 && read_directory_contents(name1, &p1))
return -1;
-   if (name2 && read_directory(name2, &p2)) {
+   if (name2 && read_directory_contents(name2, &p2)) {
string_list_clear(&p1, 0);
return -1;
}
-- 
1.9.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

Motichek Loans @ 2%

[Motichek Loans]

Hi,
In need of a loan for any purpose? Look no further, Motichek Loans Agency can 
help you with that loan you so seek and @ a 2% interest rate, you have access 
to funds between $10,000.00 to $20,000,000.00 If Interested, send your details 
via our email below for fast processing of that loan.
Email: motic...@foxmail.com

---
This email is free from viruses and malware because avast! Antivirus protection 
is active.
http://www.avast.com

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

Re: [PATCH v5] use starts_with() instead of !memcmp()

[Eric Sunshine]

On Tue, Mar 18, 2014 at 9:18 PM, Quint Guvernator
 wrote:
> Another version, this time very in line with the review and commentary of
> Junio, Eric, and Michael.  This version boasts a revamped commit message and
> fewer but surer hunks changed.

Explaining what changed in this version is indeed a courtesy to
reviewers. Thanks.

For bonus points, provide a link to the previous attempt, like this [1].

[1]: http://thread.gmane.org/gmane.comp.version-control.git/244292

> Thanks again for the guidance.
>
> Quint Guvernator (1):
>   use starts_with() instead of !memcmp()
>
>  builtin/apply.c|  4 ++--
>  builtin/for-each-ref.c |  2 +-
>  builtin/mktag.c|  2 +-
>  builtin/patch-id.c | 10 +-
>  connect.c  |  4 ++--
>  imap-send.c|  6 +++---
>  remote.c   |  2 +-
>  7 files changed, 15 insertions(+), 15 deletions(-)
>
> --
> 1.9.0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2][GSoC 2014] diff: used is_dot_or_dotdot() in code

[Eric Sunshine]

On Tue, Mar 18, 2014 at 11:58 PM, Brian Bourn  wrote:
> On Tue, Mar 18, 2014 at 11:45 PM, Eric Sunshine 
> wrote:
>> On Tue, Mar 18, 2014 at 9:30 PM, babourn  wrote:
>> > Subject: diff: used is_dot_or_dotdot() in code
>> > Signed-off-by: Brian Bourn 
>> > ---
>> >  diff-no-index.c | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/diff-no-index.c b/diff-no-index.c
>> > index ba915af..44cce25 100644
>> > --- a/diff-no-index.c
>> > +++ b/diff-no-index.c
>> > @@ -26,7 +26,7 @@ static int read_directory_contents(const char *path,
>> > struct string_list *list)
>> >   return error("Could not open directory %s", path);
>> >
>> >   while ((e = readdir(dir)))
>> > - if (strcmp(".", e->d_name) && strcmp("..", e->d_name))
>> > + if (!is_dot_or_dotdot(e->d_name))
>>
>> The patch is severely whitespace-damaged. (Did you post it through
>> Nabble?)
>
>I did post through Nabble, My email with the patch didn't seem to be
> going through.
>should I keep trying to resend it through email to undo the whitspace
> damage?

It's probably not necessary to try resending this version of the patch
since you'll (hopefully) be sending a newer version which takes
reviewer comments into consideration.

What method are you using to send the patches? git send-email?
Something other? This particular mailing list rejects HTML-formatted
messages, so that could be the culprit if you pasted the patch into
your email client. It's a good idea to try sending patches to yourself
via "git send-email". If you can get that to work successfully, then
they should be accepted by the mailing list when sent via the same
mechanism.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] tests: use "env" to run commands with temporary env-var settings

[Eric Sunshine]

On Tue, Mar 18, 2014 at 2:54 PM, David Tran  wrote:
> Originally, we would use "VAR=VAL command" to execute a test command with
> environment variable(s) only for that command. This does not work for commands
> that are shell functions (most notably test functions like "test_must_fail");
> the result of the assignment is retained and affects later commands.
>
> To avoid this, we assigned and exported the environment variables and run
> the test(s) in a subshell like this,
>
> (
> VAR=VAL &&
> export VAR

Append && to this line.

> test_must_fail git command to be tested
> )
>
> Using the "env" utility, we should be able to say
>
> test_must_fail git command to be tested
>
> which is much short and easier to read.

s/short/shorter/

> Signed-off-by: David Tran 
>
> ---
>
> Hopefully this should be all of it.

Much better. I didn't spot any errors in the patch this time around.

One final note for future submissions: As a courtesy to reviewers,
explain (below the "---" line) what changed in the current version,
and provide a reference to the previous attempt, like this [1].

[1]: http://thread.gmane.org/gmane.comp.version-control.git/244379

> Signed-off-by: David Tran 
> ---
>  t/t1300-repo-config.sh|   17 ++
>  t/t1510-repo-setup.sh |4 +--
>  t/t3200-branch.sh |   12 +--
>  t/t3301-notes.sh  |   22 +++-
>  t/t3404-rebase-interactive.sh |   69 
> -
>  t/t3413-rebase-hook.sh|6 +---
>  t/t4014-format-patch.sh   |   14 ++--
>  t/t5305-include-tag.sh|4 +--
>  t/t5602-clone-remote-exec.sh  |   13 ++--
>  t/t5801-remote-helpers.sh |6 +--
>  t/t6006-rev-list-format.sh|9 ++---
>  t/t7006-pager.sh  |   18 ++-
>  12 files changed, 42 insertions(+), 152 deletions(-)
--
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][GSoC 2014] diff: used is_dot_or_dotdot() in code

[Eric Sunshine]

On Tue, Mar 18, 2014 at 9:30 PM, babourn  wrote:
> Subject: diff: used is_dot_or_dotdot() in code

Use imperative voice: "use" rather than "used"

> in accordance with the GSoC Microproject implemented

This commentary will not have much meaning to someone reading the
commit log months or years from now. Place it below the "---" line
following your sign-off.

> the call is_dot_or_dotdot() in the code in order to
> further universalize the call to the function and
> increase code continuity.

It should be sufficient to explain the patch by just saying:

Subject: replace manual "."/".." check with is_dot_or_dotdot()

The rest of the explanatory text can be dropped since it doesn't add
anything (meaningful) beyond what the subject says.

> Signed-off-by: Brian Bourn 
> ---
>  diff-no-index.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/diff-no-index.c b/diff-no-index.c
> index ba915af..44cce25 100644
> --- a/diff-no-index.c
> +++ b/diff-no-index.c
> @@ -26,7 +26,7 @@ static int read_directory_contents(const char *path,
> struct string_list *list)
>   return error("Could not open directory %s", path);
>
>   while ((e = readdir(dir)))
> - if (strcmp(".", e->d_name) && strcmp("..", e->d_name))
> + if (!is_dot_or_dotdot(e->d_name))

The patch is severely whitespace-damaged. (Did you post it through Nabble?)

>   string_list_insert(list, e->d_name);
>
>   closedir(dir);
> --
> 1.9.0
>
>
>
> --
> View this message in context: 
> http://git.661346.n2.nabble.com/PATCH-GSoC-2014-diff-Imported-dir-h-and-renamed-read-directory-tp7605950p7605956.html
> Sent from the git mailing list archive at Nabble.com.
> --
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3] tests: use "env" to run commands with temporary env-var settings

[David Tran]

Originally, we would use "VAR=VAL command" to execute a test command with
environment variable(s) only for that command. This does not work for commands
that are shell functions (most notably test functions like "test_must_fail");
the result of the assignment is retained and affects later commands.

To avoid this, we assigned and exported the environment variables and run
the test(s) in a subshell like this,

(
VAR=VAL &&
export VAR
test_must_fail git command to be tested
)

Using the "env" utility, we should be able to say

test_must_fail git command to be tested

which is much short and easier to read.

Signed-off-by: David Tran 

---

>>Let's see if I replied correctly with send-email. Retrying this again.
>>How do I 'reply' to a thread using send-email?
>Look for --in-reply-to option in "man git-send-email".
Git prompts me for this but I don't know what to type. I tried typing just
the id, like 244379 and that didn't work. I guess I'll try
244...@gmane.comp.version-control.git? Google didn't bring many results on
how to use it but many results of what it is and its goal.

>Isn't GIT_CONFIG here another way of saying:
>
>   test_must_fail git config -f doesnotexist --list
>
>Perhaps that is shorter and more readable still (and there are a few
>similar cases in this patch.
I'll ignore this for now. If needed I can make another patch to resolve this.

Hopefully this should be all of it.

I am David Tran a graduating CS/Math senior from Sonoma State University,
United States. I would like to work with git for GSoC'14, specifically the line
options for git rebase --interactive [1][2]. I have used git for a few years and
know how destructive but important rebase is to git. I have created a few shell
scripts here and there to make life using bash/zsh easier. I would like to
apply these skills and work with the best.

Github: unsignedzero

[1]: http://thread.gmane.org/gmane.comp.version-control.git/243933/focus=243967
[2]: http://thread.gmane.org/gmane.comp.version-control.git/242701

Signed-off-by: David Tran 
---
 t/t1300-repo-config.sh|   17 ++
 t/t1510-repo-setup.sh |4 +--
 t/t3200-branch.sh |   12 +--
 t/t3301-notes.sh  |   22 +++-
 t/t3404-rebase-interactive.sh |   69 -
 t/t3413-rebase-hook.sh|6 +---
 t/t4014-format-patch.sh   |   14 ++--
 t/t5305-include-tag.sh|4 +--
 t/t5602-clone-remote-exec.sh  |   13 ++--
 t/t5801-remote-helpers.sh |6 +--
 t/t6006-rev-list-format.sh|9 ++---
 t/t7006-pager.sh  |   18 ++-
 12 files changed, 42 insertions(+), 152 deletions(-)

diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index c9c426c..3e3f77b 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -974,24 +974,15 @@ test_expect_success SYMLINKS 'symlinked configuration' '
 '

 test_expect_success 'nonexistent configuration' '
-   (
-   GIT_CONFIG=doesnotexist &&
-   export GIT_CONFIG &&
-   test_must_fail git config --list &&
-   test_must_fail git config test.xyzzy
-   )
+   test_must_fail env GIT_CONFIG=doesnotexist git config --list &&
+   test_must_fail env GIT_CONFIG=doesnotexist git config test.xyzzy
 '

 test_expect_success SYMLINKS 'symlink to nonexistent configuration' '
ln -s doesnotexist linktonada &&
ln -s linktonada linktolinktonada &&
-   (
-   GIT_CONFIG=linktonada &&
-   export GIT_CONFIG &&
-   test_must_fail git config --list &&
-   GIT_CONFIG=linktolinktonada &&
-   test_must_fail git config --list
-   )
+   test_must_fail env GIT_CONFIG=linktonada git config --list &&
+   test_must_fail env GIT_CONFIG=linktolinktonada git config --list
 '

 test_expect_success 'check split_cmdline return' "
diff --git a/t/t1510-repo-setup.sh b/t/t1510-repo-setup.sh
index cf2ee78..e1b2a99 100755
--- a/t/t1510-repo-setup.sh
+++ b/t/t1510-repo-setup.sh
@@ -777,9 +777,7 @@ test_expect_success '#30: core.worktree and core.bare 
conflict (gitfile version)
setup_repo 30 "$here/30" gitfile true &&
(
cd 30 &&
-   GIT_DIR=.git &&
-   export GIT_DIR &&
-   test_must_fail git symbolic-ref HEAD 2>result
+   test_must_fail env GIT_DIR=.git git symbolic-ref HEAD 2>result
) &&
grep "core.bare and core.worktree" 30/result
 '
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index fcdb867..d45e95c 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -849,11 +849,7 @@ test_expect_success 'detect typo in branch name when using 
--edit-description' '
write_script editor <<-\EOF &&
echo "New contents" >"$1"
EOF
-   (
-   EDITOR=./editor &&
-   export

[PATCH 2/2][GSoC 2014] diff: used is_dot_or_dotdot() in code

[babourn]

in accordance with the GSoC Microproject implemented
the call is_dot_or_dotdot() in the code in order to
further universalize the call to the function and
increase code continuity.

Signed-off-by: Brian Bourn 
---
 diff-no-index.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/diff-no-index.c b/diff-no-index.c
index ba915af..44cce25 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -26,7 +26,7 @@ static int read_directory_contents(const char *path,
struct string_list *list)
  return error("Could not open directory %s", path);

  while ((e = readdir(dir)))
- if (strcmp(".", e->d_name) && strcmp("..", e->d_name))
+ if (!is_dot_or_dotdot(e->d_name))
  string_list_insert(list, e->d_name);

  closedir(dir);
--
1.9.0



--
View this message in context: 
http://git.661346.n2.nabble.com/PATCH-GSoC-2014-diff-Imported-dir-h-and-renamed-read-directory-tp7605950p7605956.html
Sent from the git mailing list archive at Nabble.com.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] [GSoC 2014]diff: Imported dir.h and renamed read_directory()

[Eric Sunshine]

Thanks for the submission. Comments below to give you a taste of the
Git review process...

On Tue, Mar 18, 2014 at 8:35 PM, Brian Bourn  wrote:
> Subject: diff: Imported dir.h and renamed read_directory()

Use imperative voice: "import dir.h and rename read_directory()"

> this was done in order to implement the GSoC
> Micro project for diff-no-index.c

This commentary won't be relevant to anyone reading the commit message
months or years from now. Place it below the "---" line following your
sign-off.

> this is the
> first patch importing dir.h, for the use of
> is_dot_or_dotdot(), and renaming
> read_directory() to read_directory_contents()
> in order to deal with the conflicting function
> in dir.h

Since this preparatory change and the one which will follow are so
closely related, they should be submitted together as a two-patch
series.

The commit message itself it somewhat rambling. It would be clearer if
you could explain the problem precisely, then the solution. Perhaps
something like this:

It would be desirable to replace manual checking of "." or ".."
in diff-no-index.c with is_dot_or_dotdot(), which is defined in
dir.h, however, dir.h declares a read_directory() which conflicts
with a (different) static read_directory() defined in
diff-no-index.c. As a preparatory step, rename the local
read_directory() to avoid the collision.

> Signed-off-by: Brian Bourn 
> ---
>
> I plan on applying to GSoC 2014
>
>  diff-no-index.c | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/diff-no-index.c b/diff-no-index.c
> index 8e10bff..ba915af 100644
> --- a/diff-no-index.c
> +++ b/diff-no-index.c
> @@ -10,13 +10,14 @@
>  #include "blob.h"
>  #include "tag.h"
>  #include "diff.h"
> +#include "dir.h"
>  #include "diffcore.h"

dir.h is not needed for this patch, so including it here only confuses
matters. Include it instead when it is actually required by the
follow-up patch which uses is_dot_or_dotdot().

(Was it intentional to place the new #include between diff.h and
diffcore.h? Just being curious; it's not particularly important.)

Other than this, the patch looks sane.

>  #include "revision.h"
>  #include "log-tree.h"
>  #include "builtin.h"
>  #include "string-list.h"
>
> -static int read_directory(const char *path, struct string_list *list)
> +static int read_directory_contents(const char *path, struct string_list 
> *list)
>  {
>   DIR *dir;
>   struct dirent *e;
> @@ -107,9 +108,9 @@ static int queue_diff(struct diff_options *o,
>   int i1, i2, ret = 0;
>   size_t len1 = 0, len2 = 0;
>
> - if (name1 && read_directory(name1, &p1))
> + if (name1 && read_directory_contents(name1, &p1))
>   return -1;
> - if (name2 && read_directory(name2, &p2)) {
> + if (name2 && read_directory_contents(name2, &p2)) {
>   string_list_clear(&p1, 0);
>   return -1;
>   }
> --
> 1.9.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] use starts_with() instead of !memcmp()

[Quint Guvernator]

Another version, this time very in line with the review and commentary of
Junio, Eric, and Michael.  This version boasts a revamped commit message and
fewer but surer hunks changed.

Thanks again for the guidance.

Quint Guvernator (1):
  use starts_with() instead of !memcmp()

 builtin/apply.c|  4 ++--
 builtin/for-each-ref.c |  2 +-
 builtin/mktag.c|  2 +-
 builtin/patch-id.c | 10 +-
 connect.c  |  4 ++--
 imap-send.c|  6 +++---
 remote.c   |  2 +-
 7 files changed, 15 insertions(+), 15 deletions(-)

-- 
1.9.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] use starts_with() instead of !memcmp()

[Quint Guvernator]

When checking if a string begins with a constant string, using
starts_with() indicates the intention of the check more clearly and is
less error-prone than calling !memcmp() with an explicit byte count.

Signed-off-by: Quint Guvernator 
---
 builtin/apply.c|  4 ++--
 builtin/for-each-ref.c |  2 +-
 builtin/mktag.c|  2 +-
 builtin/patch-id.c | 10 +-
 connect.c  |  4 ++--
 imap-send.c|  6 +++---
 remote.c   |  2 +-
 7 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index 0189523..826b3e2 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -1631,7 +1631,7 @@ static int parse_fragment(const char *line, unsigned long 
size,
 * l10n of "\ No newline..." is at least that long.
 */
case '\\':
-   if (len < 12 || memcmp(line, "\\ ", 2))
+   if (len < 12 || !starts_with(line, "\\ "))
return -1;
break;
}
@@ -1646,7 +1646,7 @@ static int parse_fragment(const char *line, unsigned long 
size,
 * it in the above loop because we hit oldlines == newlines == 0
 * before seeing it.
 */
-   if (12 < size && !memcmp(line, "\\ ", 2))
+   if (12 < size && starts_with(line, "\\ "))
offset += linelen(line, size);
 
patch->lines_added += added;
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 3e1d5c3..4135980 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -193,7 +193,7 @@ static int verify_format(const char *format)
at = parse_atom(sp + 2, ep);
cp = ep + 1;
 
-   if (!memcmp(used_atom[at], "color:", 6))
+   if (starts_with(used_atom[at], "color:"))
need_color_reset_at_eol = !!strcmp(used_atom[at], 
color_reset);
}
return 0;
diff --git a/builtin/mktag.c b/builtin/mktag.c
index 640ab64..d2d9310 100644
--- a/builtin/mktag.c
+++ b/builtin/mktag.c
@@ -57,7 +57,7 @@ static int verify_tag(char *buffer, unsigned long size)
 
/* Verify type line */
type_line = object + 48;
-   if (memcmp(type_line - 1, "\ntype ", 6))
+   if (!starts_with(type_line - 1, "\ntype "))
return error("char%d: could not find \"\\ntype \"", 47);
 
/* Verify tag-line */
diff --git a/builtin/patch-id.c b/builtin/patch-id.c
index 3cfe02d..23ef424 100644
--- a/builtin/patch-id.c
+++ b/builtin/patch-id.c
@@ -81,14 +81,14 @@ static int get_one_patchid(unsigned char *next_sha1, 
git_SHA_CTX *ctx, struct st
}
 
/* Ignore commit comments */
-   if (!patchlen && memcmp(line, "diff ", 5))
+   if (!patchlen && !starts_with(line, "diff "))
continue;
 
/* Parsing diff header?  */
if (before == -1) {
-   if (!memcmp(line, "index ", 6))
+   if (starts_with(line, "index "))
continue;
-   else if (!memcmp(line, "--- ", 4))
+   else if (starts_with(line, "--- "))
before = after = 1;
else if (!isalpha(line[0]))
break;
@@ -96,14 +96,14 @@ static int get_one_patchid(unsigned char *next_sha1, 
git_SHA_CTX *ctx, struct st
 
/* Looking for a valid hunk header?  */
if (before == 0 && after == 0) {
-   if (!memcmp(line, "@@ -", 4)) {
+   if (starts_with(line, "@@ -")) {
/* Parse next hunk, but ignore line numbers.  */
scan_hunk_header(line, &before, &after);
continue;
}
 
/* Split at the end of the patch.  */
-   if (memcmp(line, "diff ", 5))
+   if (!starts_with(line, "diff "))
break;
 
/* Else we're parsing another header.  */
diff --git a/connect.c b/connect.c
index 4150412..5ae2aaa 100644
--- a/connect.c
+++ b/connect.c
@@ -30,11 +30,11 @@ static int check_ref(const char *name, int len, unsigned 
int flags)
return 0;
 
/* REF_HEADS means that we want regular branch heads */
-   if ((flags & REF_HEADS) && !memcmp(name, "heads/", 6))
+   if ((flags & REF_HEADS) && starts_with(name, "heads/"))
return 1;
 
/* REF_TAGS means that we want tags */
-   if ((flags & REF_TAGS) && !memcmp(name, "tags/", 5))
+   if ((flags & REF_TAGS) && starts_with(name, "tags/"))
return 1;
 
/* All type bits clear means that we are ok with anything */
diff --git a/imap-send.c b/imap-send.c
index 0bc6f7f..019de18 

[PATCH] Enable index-pack threading in msysgit.

[szager]

This adds a Windows implementation of pread.  Note that it is NOT
safe to intersperse calls to read() and pread() on a file
descriptor.  According to the ReadFile spec, using the 'overlapped'
argument should not affect the implicit position pointer of the
descriptor.  Experiments have shown that this is, in fact, a lie.

To accomodate that fact, this change also incorporates:

http://article.gmane.org/gmane.comp.version-control.git/196042

... which gives each index-pack thread its own file descriptor.
---
 builtin/index-pack.c | 21 -
 compat/mingw.c   | 31 ++-
 compat/mingw.h   |  3 +++
 config.mak.uname |  1 -
 4 files changed, 49 insertions(+), 7 deletions(-)

diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 2f37a38..c02dd4c 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -51,6 +51,7 @@ struct thread_local {
 #endif
struct base_data *base_cache;
size_t base_cache_used;
+   int pack_fd;
 };
 
 /*
@@ -91,7 +92,8 @@ static off_t consumed_bytes;
 static unsigned deepest_delta;
 static git_SHA_CTX input_ctx;
 static uint32_t input_crc32;
-static int input_fd, output_fd, pack_fd;
+static const char *curr_pack;
+static int input_fd, output_fd;
 
 #ifndef NO_PTHREADS
 
@@ -134,6 +136,7 @@ static inline void unlock_mutex(pthread_mutex_t *mutex)
  */
 static void init_thread(void)
 {
+   int i;
init_recursive_mutex(&read_mutex);
pthread_mutex_init(&counter_mutex, NULL);
pthread_mutex_init(&work_mutex, NULL);
@@ -141,11 +144,17 @@ static void init_thread(void)
pthread_mutex_init(&deepest_delta_mutex, NULL);
pthread_key_create(&key, NULL);
thread_data = xcalloc(nr_threads, sizeof(*thread_data));
+   for (i = 0; i < nr_threads; i++) {
+   thread_data[i].pack_fd = open(curr_pack, O_RDONLY);
+   if (thread_data[i].pack_fd == -1)
+   die_errno("unable to open %s", curr_pack);
+   }
threads_active = 1;
 }
 
 static void cleanup_thread(void)
 {
+   int i;
if (!threads_active)
return;
threads_active = 0;
@@ -155,6 +164,8 @@ static void cleanup_thread(void)
if (show_stat)
pthread_mutex_destroy(&deepest_delta_mutex);
pthread_key_delete(key);
+   for (i = 0; i < nr_threads; i++)
+   close(thread_data[i].pack_fd);
free(thread_data);
 }
 
@@ -288,13 +299,13 @@ static const char *open_pack_file(const char *pack_name)
output_fd = open(pack_name, O_CREAT|O_EXCL|O_RDWR, 
0600);
if (output_fd < 0)
die_errno(_("unable to create '%s'"), pack_name);
-   pack_fd = output_fd;
+   nothread_data.pack_fd = output_fd;
} else {
input_fd = open(pack_name, O_RDONLY);
if (input_fd < 0)
die_errno(_("cannot open packfile '%s'"), pack_name);
output_fd = -1;
-   pack_fd = input_fd;
+   nothread_data.pack_fd = input_fd;
}
git_SHA1_Init(&input_ctx);
return pack_name;
@@ -542,7 +553,7 @@ static void *unpack_data(struct object_entry *obj,
 
do {
ssize_t n = (len < 64*1024) ? len : 64*1024;
-   n = pread(pack_fd, inbuf, n, from);
+   n = pread(get_thread_data()->pack_fd, inbuf, n, from);
if (n < 0)
die_errno(_("cannot pread pack file"));
if (!n)
@@ -1490,7 +1501,7 @@ static void show_pack_info(int stat_only)
 int cmd_index_pack(int argc, const char **argv, const char *prefix)
 {
int i, fix_thin_pack = 0, verify = 0, stat_only = 0;
-   const char *curr_pack, *curr_index;
+   const char *curr_index;
const char *index_name = NULL, *pack_name = NULL;
const char *keep_name = NULL, *keep_msg = NULL;
char *index_name_buf = NULL, *keep_name_buf = NULL;
diff --git a/compat/mingw.c b/compat/mingw.c
index 383cafe..6cc85d6 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -329,7 +329,36 @@ int mingw_mkdir(const char *path, int mode)
return ret;
 }
 
-int mingw_open (const char *filename, int oflags, ...)
+
+ssize_t mingw_pread(int fd, void *buf, size_t count, off64_t offset)
+{
+   HANDLE hand = (HANDLE)_get_osfhandle(fd);
+   if (hand == INVALID_HANDLE_VALUE) {
+   errno = EBADF;
+   return -1;
+   }
+
+   LARGE_INTEGER offset_value;
+   offset_value.QuadPart = offset;
+
+   DWORD bytes_read = 0;
+   OVERLAPPED overlapped = {0};
+   overlapped.Offset = offset_value.LowPart;
+   overlapped.OffsetHigh = offset_value.HighPart;
+   BOOL result = ReadFile(hand, buf, count, &bytes_read, &overlapped);
+
+   ssize_t ret = bytes_read;
+
+   if (!result && GetLastError() != ERROR_HANDLE_EOF)
+   {
+   errno =

[no subject]

[szager]

Subject: [PATCH] Enable index-pack threading in msysgit.

This adds a Windows implementation of pread.  Note that it is NOT
safe to intersperse calls to read() and pread() on a file
descriptor.  According to the ReadFile spec, using the 'overlapped'
argument should not affect the implicit position pointer of the
descriptor.  Experiments have shown that this is, in fact, a lie.

To accomodate that fact, this change also incorporates:

http://article.gmane.org/gmane.comp.version-control.git/196042

... which gives each index-pack thread its own file descriptor.
---
 builtin/index-pack.c | 21 -
 compat/mingw.c   | 31 ++-
 compat/mingw.h   |  3 +++
 config.mak.uname |  1 -
 4 files changed, 49 insertions(+), 7 deletions(-)

diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 2f37a38..c02dd4c 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -51,6 +51,7 @@ struct thread_local {
 #endif
struct base_data *base_cache;
size_t base_cache_used;
+   int pack_fd;
 };
 
 /*
@@ -91,7 +92,8 @@ static off_t consumed_bytes;
 static unsigned deepest_delta;
 static git_SHA_CTX input_ctx;
 static uint32_t input_crc32;
-static int input_fd, output_fd, pack_fd;
+static const char *curr_pack;
+static int input_fd, output_fd;
 
 #ifndef NO_PTHREADS
 
@@ -134,6 +136,7 @@ static inline void unlock_mutex(pthread_mutex_t *mutex)
  */
 static void init_thread(void)
 {
+   int i;
init_recursive_mutex(&read_mutex);
pthread_mutex_init(&counter_mutex, NULL);
pthread_mutex_init(&work_mutex, NULL);
@@ -141,11 +144,17 @@ static void init_thread(void)
pthread_mutex_init(&deepest_delta_mutex, NULL);
pthread_key_create(&key, NULL);
thread_data = xcalloc(nr_threads, sizeof(*thread_data));
+   for (i = 0; i < nr_threads; i++) {
+   thread_data[i].pack_fd = open(curr_pack, O_RDONLY);
+   if (thread_data[i].pack_fd == -1)
+   die_errno("unable to open %s", curr_pack);
+   }
threads_active = 1;
 }
 
 static void cleanup_thread(void)
 {
+   int i;
if (!threads_active)
return;
threads_active = 0;
@@ -155,6 +164,8 @@ static void cleanup_thread(void)
if (show_stat)
pthread_mutex_destroy(&deepest_delta_mutex);
pthread_key_delete(key);
+   for (i = 0; i < nr_threads; i++)
+   close(thread_data[i].pack_fd);
free(thread_data);
 }
 
@@ -288,13 +299,13 @@ static const char *open_pack_file(const char *pack_name)
output_fd = open(pack_name, O_CREAT|O_EXCL|O_RDWR, 
0600);
if (output_fd < 0)
die_errno(_("unable to create '%s'"), pack_name);
-   pack_fd = output_fd;
+   nothread_data.pack_fd = output_fd;
} else {
input_fd = open(pack_name, O_RDONLY);
if (input_fd < 0)
die_errno(_("cannot open packfile '%s'"), pack_name);
output_fd = -1;
-   pack_fd = input_fd;
+   nothread_data.pack_fd = input_fd;
}
git_SHA1_Init(&input_ctx);
return pack_name;
@@ -542,7 +553,7 @@ static void *unpack_data(struct object_entry *obj,
 
do {
ssize_t n = (len < 64*1024) ? len : 64*1024;
-   n = pread(pack_fd, inbuf, n, from);
+   n = pread(get_thread_data()->pack_fd, inbuf, n, from);
if (n < 0)
die_errno(_("cannot pread pack file"));
if (!n)
@@ -1490,7 +1501,7 @@ static void show_pack_info(int stat_only)
 int cmd_index_pack(int argc, const char **argv, const char *prefix)
 {
int i, fix_thin_pack = 0, verify = 0, stat_only = 0;
-   const char *curr_pack, *curr_index;
+   const char *curr_index;
const char *index_name = NULL, *pack_name = NULL;
const char *keep_name = NULL, *keep_msg = NULL;
char *index_name_buf = NULL, *keep_name_buf = NULL;
diff --git a/compat/mingw.c b/compat/mingw.c
index 383cafe..6cc85d6 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -329,7 +329,36 @@ int mingw_mkdir(const char *path, int mode)
return ret;
 }
 
-int mingw_open (const char *filename, int oflags, ...)
+
+ssize_t mingw_pread(int fd, void *buf, size_t count, off64_t offset)
+{
+   HANDLE hand = (HANDLE)_get_osfhandle(fd);
+   if (hand == INVALID_HANDLE_VALUE) {
+   errno = EBADF;
+   return -1;
+   }
+
+   LARGE_INTEGER offset_value;
+   offset_value.QuadPart = offset;
+
+   DWORD bytes_read = 0;
+   OVERLAPPED overlapped = {0};
+   overlapped.Offset = offset_value.LowPart;
+   overlapped.OffsetHigh = offset_value.HighPart;
+   BOOL result = ReadFile(hand, buf, count, &bytes_read, &overlapped);
+
+   ssize_t ret = bytes_read;
+
+   if (!result && GetLastErro

[PATCH] [GSoC 2014]diff: Imported dir.h and renamed read_directory()

[Brian Bourn]

this was done in order to implement the GSoC
Micro project for diff-no-index.c this is the
first patch importing dir.h, for the use of
is_dot_or_dotdot(), and renaming
read_directory() to read_directory_contents()
in order to deal with the conflicting function
in dir.h

Signed-off-by: Brian Bourn 
---

I plan on applying to GSoC 2014

 diff-no-index.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/diff-no-index.c b/diff-no-index.c
index 8e10bff..ba915af 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -10,13 +10,14 @@
 #include "blob.h"
 #include "tag.h"
 #include "diff.h"
+#include "dir.h"
 #include "diffcore.h"
 #include "revision.h"
 #include "log-tree.h"
 #include "builtin.h"
 #include "string-list.h"

-static int read_directory(const char *path, struct string_list *list)
+static int read_directory_contents(const char *path, struct string_list *list)
 {
  DIR *dir;
  struct dirent *e;
@@ -107,9 +108,9 @@ static int queue_diff(struct diff_options *o,
  int i1, i2, ret = 0;
  size_t len1 = 0, len2 = 0;

- if (name1 && read_directory(name1, &p1))
+ if (name1 && read_directory_contents(name1, &p1))
  return -1;
- if (name2 && read_directory(name2, &p2)) {
+ if (name2 && read_directory_contents(name2, &p2)) {
  string_list_clear(&p1, 0);
  return -1;
  }
-- 
1.9.0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GSoC14][RFC] Is there any interest in adding a port of checkpatch.pl to contrib/?

[Jacopo Notarstefano]

On Tue, Mar 18, 2014 at 8:29 PM, Junio C Hamano  wrote:
>
> I've never said any such thing.
>
> I only said I am not enthused against a proposal to add a build
> target that runs checkpatch or equivalent over *all* existing code,
> which will invite needless churn (read again the part of the message
> you quoted before I say "I am not enthused").
>
> It is a totally separate issue for submitters to make sure they do
> not introduce *new* problems, and use of "checkpatch --no-tree"
> could be one way to do so.
>

Sorry for misrepresenting your opinion. My summarization algorithm
sometimes is _very_ lossy :)
--
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: [PATCHv2] branch.c: simplify chain of if statements

[Eric Sunshine]

On Tue, Mar 18, 2014 at 6:31 PM, Eric Sunshine  wrote:
> On Mon, Mar 17, 2014 at 11:51 AM, Dragos Foianu  
> wrote:
>> This patch uses a table to store the different messages that can
>> be emitted by the verbose install_branch_config function. It
>> computes an index based on the three flags and prints the message
>> located at the specific index in the table of messages. If the
>> index somehow is not within the table, we have a bug.
>>
>> Signed-off-by: Dragos Foianu 
>> ---
>>  branch.c | 44 +---
>>  1 file changed, 25 insertions(+), 19 deletions(-)
>>
>> diff --git a/branch.c b/branch.c
>> index 723a36b..95645d5 100644
>> --- a/branch.c
>> +++ b/branch.c
>> @@ -54,6 +54,18 @@ void install_branch_config(int flag, const char *local, 
>> const char *origin, cons
>> +   int index = 0;
>> +   if (origin)
>> +   index += 1;
>> +   if (remote_is_branch)
>> +   index += 2;
>> +   if (rebasing)
>> +   index += 4;
>> +
>> +   if (index < 0 || index > sizeof(messages) / 
>> sizeof(*messages))
>> die("BUG: impossible combination of %d and %p",
>> remote_is_branch, origin);

One other observation: You have a one-off error in your out-of-bounds
check. It should be 'index >= sizeof...'

> You can use ARRAY_SIZE() in place of sizeof(...)/sizeof(...).
>
> Since an out-of-bound index would be a programmer bug, it would
> probably be more appropriate to use an assert(), just after 'index' is
> computed, rather than if+die(). The original code used die() because
> it couldn't detect the error until the end of the if-chain.
>
>> }
>> --
>> 1.8.3.2
--
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] fsck.c:fsck_commit() use starts_with() and skip_prefix()

[Michael Haggerty]

On 03/19/2014 12:09 AM, Eric Sunshine wrote:
> Thanks for the submission. Comments below to give you a taste of the
> Git review process...
> 
> On Tue, Mar 18, 2014 at 6:41 PM, Othman Darraz  wrote:
>> use of starts_with() instead of memcmp()
>>
>> use of skip_prefix instead of memcmp() and strlen()
> 
> Write proper sentences to explain and justify the change; not sentence
> fragments.
> 
>> Signed-off-by: Othman Darraz 
>> ---
>>
>> I am planning to apply to GSOC 214
> 
> Your logic in these changes is rather severely flawed. Running the
> test suite (t1450, in particular), as the GSoC microproject page
> suggests, would have clued you in that something was amiss.
> 
>>  fsck.c | 8 
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/fsck.c b/fsck.c
>> index 64bf279..5eae856 100644
>> --- a/fsck.c
>> +++ b/fsck.c
>> @@ -290,7 +290,7 @@ static int fsck_commit(struct commit *commit, fsck_error 
>> error_func)
>> int parents = 0;
>> int err;
>>
>> -   if (memcmp(buffer, "tree ", 5))
>> +   if (starts_with(buffer, "tree "))
>> return error_func(&commit->object, FSCK_ERROR, "invalid 
>> format - expected 'tree' line");
>> if (get_sha1_hex(buffer+5, tree_sha1) || buffer[45] != '\n')
> 
> One of the reasons for using starts_with() rather than memcmp() is
> that it allows you to eliminate magic numbers, such as 5. However, if
> you look closely at this code fragment, you will see that the magic
> number is still present in the expression 'buffer+5'. starts_with(),
> might be a better fit.

Eric, I think you meant to say that *skip_prefix()* might be a better fit.

> [...]

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] fsck.c:fsck_commit() use starts_with() and skip_prefix()

[Eric Sunshine]

On Tue, Mar 18, 2014 at 7:09 PM, Eric Sunshine  wrote:
>> diff --git a/fsck.c b/fsck.c
>> index 64bf279..5eae856 100644
>> --- a/fsck.c
>> +++ b/fsck.c
>> @@ -290,7 +290,7 @@ static int fsck_commit(struct commit *commit, fsck_error 
>> error_func)
>> int parents = 0;
>> int err;
>>
>> -   if (memcmp(buffer, "tree ", 5))
>> +   if (starts_with(buffer, "tree "))
>> return error_func(&commit->object, FSCK_ERROR, "invalid 
>> format - expected 'tree' line");
>> if (get_sha1_hex(buffer+5, tree_sha1) || buffer[45] != '\n')
>
> One of the reasons for using starts_with() rather than memcmp() is
> that it allows you to eliminate magic numbers, such as 5. However, if
> you look closely at this code fragment, you will see that the magic
> number is still present in the expression 'buffer+5'. starts_with(),
> might be a better fit.

Of course, I meant "skip_prefix() might be a better fit".
--
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] fsck.c:fsck_commit() use starts_with() and skip_prefix()

[Eric Sunshine]

Thanks for the submission. Comments below to give you a taste of the
Git review process...

On Tue, Mar 18, 2014 at 6:41 PM, Othman Darraz  wrote:
> use of starts_with() instead of memcmp()
>
> use of skip_prefix instead of memcmp() and strlen()

Write proper sentences to explain and justify the change; not sentence
fragments.

> Signed-off-by: Othman Darraz 
> ---
>
> I am planning to apply to GSOC 214

Your logic in these changes is rather severely flawed. Running the
test suite (t1450, in particular), as the GSoC microproject page
suggests, would have clued you in that something was amiss.

>  fsck.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/fsck.c b/fsck.c
> index 64bf279..5eae856 100644
> --- a/fsck.c
> +++ b/fsck.c
> @@ -290,7 +290,7 @@ static int fsck_commit(struct commit *commit, fsck_error 
> error_func)
> int parents = 0;
> int err;
>
> -   if (memcmp(buffer, "tree ", 5))
> +   if (starts_with(buffer, "tree "))
> return error_func(&commit->object, FSCK_ERROR, "invalid 
> format - expected 'tree' line");
> if (get_sha1_hex(buffer+5, tree_sha1) || buffer[45] != '\n')

One of the reasons for using starts_with() rather than memcmp() is
that it allows you to eliminate magic numbers, such as 5. However, if
you look closely at this code fragment, you will see that the magic
number is still present in the expression 'buffer+5'. starts_with(),
might be a better fit.

> return error_func(&commit->object, FSCK_ERROR, "invalid 
> 'tree' line format - bad sha1");
> @@ -322,15 +322,15 @@ static int fsck_commit(struct commit *commit, 
> fsck_error error_func)
> if (p || parents)
> return error_func(&commit->object, FSCK_ERROR, 
> "parent objects missing");
> }
> -   if (memcmp(buffer, "author ", 7))
> +   if (starts_with(buffer, "author "))
> return error_func(&commit->object, FSCK_ERROR, "invalid 
> format - expected 'author' line");
> buffer += 7;

Same problem here with magic number 7.

> err = fsck_ident(&buffer, &commit->object, error_func);
> if (err)
> return err;
> -   if (memcmp(buffer, "committer ", strlen("committer ")))
> +   buffer = (char* )skip_prefix(buffer,"committer ");

Style: (char *)
Style: whitespace: skip_prefix(foo, "bar")

Regarding the (char *) cast: Is 'buffer' ever modified in this
function? If not, would it make sense to make it 'const' (and fix any
other small fallout from that change)?

> +   if (!buffer)
> return error_func(&commit->object, FSCK_ERROR, "invalid 
> format - expected 'committer' line");
> -   buffer += strlen("committer ");
> err = fsck_ident(&buffer, &commit->object, error_func);
> if (err)
> return err;
> --
> 1.9.0.258.g00eda23.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 v2] tests: set temp variables using 'env' in test function instead of subshell

[Jeff King]

On Tue, Mar 18, 2014 at 03:16:27PM -0700, Junio C Hamano wrote:

> > Isn't GIT_CONFIG here another way of saying:
> >
> >   test_must_fail git config -f doesnotexist --list
> >
> > Perhaps that is shorter and more readable still (and there are a few
> > similar cases in this patch.
> 
> Surely, but are we assuming that "git config" correctly honors the
> equivalence between GIT_CONFIG=file and -f file, or is that also
> something we are testing in these tests?

I think we can assume that they are equivalent, and it is not worth
testing (and they are equivalent in code since 270a344 (config: stop
using config_exclusive_filename, 2012-02-16).

My recollection is that GIT_CONFIG mostly exists as a historical
footnote. Recall that at one time it affected all commands, but that had
many problems and was done away with in dc87183 (Only use GIT_CONFIG in
"git config", not other programs, 2008-06-30). I think we left it in
place for git-config mostly for backward compatibility, but I didn't see
that point explicitly addressed in the list discussion (the main issue
was that setting it for things besides "git config" is a bad idea, as it
suppresses ~/.gitconfig).

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

[PATCH] fsck.c:fsck_commit() use starts_with() and skip_prefix()

[Othman Darraz]

use of starts_with() instead of memcmp()

use of skip_prefix instead of memcmp() and strlen()

Signed-off-by: Othman Darraz 
---

I am planning to apply to GSOC 214
 fsck.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fsck.c b/fsck.c
index 64bf279..5eae856 100644
--- a/fsck.c
+++ b/fsck.c
@@ -290,7 +290,7 @@ static int fsck_commit(struct commit *commit, fsck_error 
error_func)
int parents = 0;
int err;
 
-   if (memcmp(buffer, "tree ", 5))
+   if (starts_with(buffer, "tree "))
return error_func(&commit->object, FSCK_ERROR, "invalid format 
- expected 'tree' line");
if (get_sha1_hex(buffer+5, tree_sha1) || buffer[45] != '\n')
return error_func(&commit->object, FSCK_ERROR, "invalid 'tree' 
line format - bad sha1");
@@ -322,15 +322,15 @@ static int fsck_commit(struct commit *commit, fsck_error 
error_func)
if (p || parents)
return error_func(&commit->object, FSCK_ERROR, "parent 
objects missing");
}
-   if (memcmp(buffer, "author ", 7))
+   if (starts_with(buffer, "author "))
return error_func(&commit->object, FSCK_ERROR, "invalid format 
- expected 'author' line");
buffer += 7;
err = fsck_ident(&buffer, &commit->object, error_func);
if (err)
return err;
-   if (memcmp(buffer, "committer ", strlen("committer ")))
+   buffer = (char* )skip_prefix(buffer,"committer ");
+   if (!buffer)
return error_func(&commit->object, FSCK_ERROR, "invalid format 
- expected 'committer' line");
-   buffer += strlen("committer ");
err = fsck_ident(&buffer, &commit->object, error_func);
if (err)
return err;
-- 
1.9.0.258.g00eda23.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 v2] tests: set temp variables using 'env' in test function instead of subshell

[Eric Sunshine]

On Tue, Mar 18, 2014 at 5:45 PM, Jeff King  wrote:
> On Tue, Mar 18, 2014 at 01:37:39PM -0700, Junio C Hamano wrote:
>
>> > diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
>> > index c9c426c..3e3f77b 100755
>> > --- a/t/t1300-repo-config.sh
>> > +++ b/t/t1300-repo-config.sh
>> > @@ -974,24 +974,15 @@ test_expect_success SYMLINKS 'symlinked 
>> > configuration' '
>> >  '
>> >
>> >  test_expect_success 'nonexistent configuration' '
>> > -   (
>> > -   GIT_CONFIG=doesnotexist &&
>> > -   export GIT_CONFIG &&
>> > -   test_must_fail git config --list &&
>> > -   test_must_fail git config test.xyzzy
>> > -   )
>> > +   test_must_fail env GIT_CONFIG=doesnotexist git config --list &&
>> > +   test_must_fail env GIT_CONFIG=doesnotexist git config test.xyzzy
>> >  '
>
> Isn't GIT_CONFIG here another way of saying:
>
>   test_must_fail git config -f doesnotexist --list
>
> Perhaps that is shorter and more readable still (and there are a few
> similar cases in this patch.

Such a change could be the subject of a separate cleanup patch, but is
tangental to the GSoC microproject which begat this submission.
--
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] mv: prevent mismatched data when ignoring errors.

[Junio C Hamano]

Michael Haggerty  writes:

> I had recently been thinking along the same lines.  In many of the
> potential callers that I noticed, ALLOC_GROW() was used immediately
> before making space in the array for a new element.  So I suggest
> something more like
>
> +#define MOVE_DOWN(array, nr, at, count)  \
> + memmove((array) + (at) + (count),   \
> + (array) + (at), \
> + sizeof((array)[0]) * ((nr) - (at)))
> +#define ALLOC_INSERT_GAP(array, nr, at, count, alloc) \
> + do { \
> + ALLOC_GROW((array), (nr) + (count), (alloc));\
> + MOVE_DOWN((array), (nr), (at), (count)); \
> + } while (0)
>
> Also, count==1 is so frequent that this special case might deserve its
> own macro pair.

Yeah, probably.

> I'm not inspired by these macro names, though.

Me neither, about ups and downs.

Peff's suggestion to name these around the concept of "gap" sounded
sensible.
--
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: [PATCHv2] branch.c: simplify chain of if statements

[Eric Sunshine]

Thanks for the resubmission. Comments below...

On Mon, Mar 17, 2014 at 11:51 AM, Dragos Foianu  wrote:
> This patch uses a table to store the different messages that can
> be emitted by the verbose install_branch_config function. It
> computes an index based on the three flags and prints the message
> located at the specific index in the table of messages. If the
> index somehow is not within the table, we have a bug.

Most of this text can be dropped due to redundancy.

Saying "This patch..." is unnecessary.

The remaining text primarily says in prose what the patch itself
conveys more concisely and precisely. It's easier to read and
understand the actual code than it is to wade through a lengthy
description of the code change.

Speak in imperative voice: "Use a table to store..."

You might, for instance, say instead something like this:

install_branch_config() uses a long, somewhat complex if-chain to
select a message to display in verbose mode.  Simplify the logic
by moving the messages to a table from which they can be
easily retrieved without complex logic.

> Signed-off-by: Dragos Foianu 
> ---
>  branch.c | 44 +---
>  1 file changed, 25 insertions(+), 19 deletions(-)
>
> diff --git a/branch.c b/branch.c
> index 723a36b..95645d5 100644
> --- a/branch.c
> +++ b/branch.c
> @@ -54,6 +54,18 @@ void install_branch_config(int flag, const char *local, 
> const char *origin, cons
> struct strbuf key = STRBUF_INIT;
> int rebasing = should_setup_rebase(origin);
>
> +   const char *messages[] = {
> +   N_("Branch %s set up to track local ref %s."),
> +   N_("Branch %s set up to track remote ref %s."),
> +   N_("Branch %s set up to track local branch %s."),
> +   N_("Branch %s set up to track remote branch %s from %s."),
> +   N_("Branch %s set up to track local ref %s by rebasing."),
> +   N_("Branch %s set up to track remote ref %s by rebasing."),
> +   N_("Branch %s set up to track local branch %s by rebasing."),
> +   N_("Branch %s set up to track remote branch %s from %s by 
> rebasing.")
> +   };
> +   int index = 0;
> +
> if (remote_is_branch
> && !strcmp(local, shortname)
> && !origin) {
> @@ -76,28 +88,22 @@ void install_branch_config(int flag, const char *local, 
> const char *origin, cons
> }
> strbuf_release(&key);
>
> +   if (origin)
> +   index += 1;
> +   if (remote_is_branch)
> +   index += 2;
> +   if (rebasing)
> +   index += 4;

Other submissions have computed this value mathematically without need
for conditionals. For instance, we've seen:

index = (!!origin << 0) + (!!remote_is_branch << 1) + (!!rebasing << 2)

as, well as the equivalent:

index = !!origin + !!remote_is_branch * 2 + !!rebasing * 4

Although this works, it does place greater cognitive demands on the
reader by requiring more effort to figure out what is going on and how
it relates to table position. The original (ungainly) chain of 'if'
statements in the original code does not suffer this problem. It
likewise is harder to understand than merely indexing into a
multi-dimension table where each variable is a key.

> if (flag & BRANCH_CONFIG_VERBOSE) {
> if (remote_is_branch && origin)
> -   printf_ln(rebasing ?
> - _("Branch %s set up to track remote branch 
> %s from %s by rebasing.") :
> - _("Branch %s set up to track remote branch 
> %s from %s."),
> - local, shortname, origin);
> -   else if (remote_is_branch && !origin)
> -   printf_ln(rebasing ?
> - _("Branch %s set up to track local branch 
> %s by rebasing.") :
> - _("Branch %s set up to track local branch 
> %s."),
> - local, shortname);
> -   else if (!remote_is_branch && origin)
> -   printf_ln(rebasing ?
> - _("Branch %s set up to track remote ref %s 
> by rebasing.") :
> - _("Branch %s set up to track remote ref 
> %s."),
> - local, remote);
> -   else if (!remote_is_branch && !origin)
> -   printf_ln(rebasing ?
> - _("Branch %s set up to track local ref %s 
> by rebasing.") :
> - _("Branch %s set up to track local ref 
> %s."),
> - local, remote);
> +   printf_ln(_(messages[index]),
> +   local, shortname, origin);
> else
> +   printf_ln(_(messages[index]),
> +  

Re: [PATCH v2] tests: set temp variables using 'env' in test function instead of subshell

[Junio C Hamano]

Jeff King  writes:

> On Tue, Mar 18, 2014 at 01:37:39PM -0700, Junio C Hamano wrote:
>
>> > diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
>> > index c9c426c..3e3f77b 100755
>> > --- a/t/t1300-repo-config.sh
>> > +++ b/t/t1300-repo-config.sh
>> > @@ -974,24 +974,15 @@ test_expect_success SYMLINKS 'symlinked 
>> > configuration' '
>> >  '
>> >
>> >  test_expect_success 'nonexistent configuration' '
>> > -  (
>> > -  GIT_CONFIG=doesnotexist &&
>> > -  export GIT_CONFIG &&
>> > -  test_must_fail git config --list &&
>> > -  test_must_fail git config test.xyzzy
>> > -  )
>> > +  test_must_fail env GIT_CONFIG=doesnotexist git config --list &&
>> > +  test_must_fail env GIT_CONFIG=doesnotexist git config test.xyzzy
>> >  '
>
> Isn't GIT_CONFIG here another way of saying:
>
>   test_must_fail git config -f doesnotexist --list
>
> Perhaps that is shorter and more readable still (and there are a few
> similar cases in this patch.

Surely, but are we assuming that "git config" correctly honors the
equivalence between GIT_CONFIG=file and -f file, or is that also
something we are testing in these tests?

--
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] test-lib.sh: use printf instead of echo

[Jonathan Nieder]

Junio C Hamano wrote:
>> Uwe Storbeck wrote:

>>> +   printf '%s\n' "$@" | sed -e 's/^/#  /'
>
> This is wrong, isn't it?  Why do we want one line per item here?

Yes, Hannes caught the same, too.  Sorry for the sloppiness.

We currently use "echo" all over the place (e.g., 'echo "$path"' in
git-sh-setup), and every time we fix it there is a chance of making
mistakes.  I wonder if it would make sense to add a helper to make the
echo calls easier to replace:

-- >8 --
Subject: git-sh-setup: introduce sane_echo helper

Using 'echo' with arguments that might contain backslashes or "-e" or
"-n" can produce confusing results that vary from platform to platform
(e.g., dash always interprets \ escape sequences and echoes "-e"
verbatim, whereas bash does not interpret \ escapes unless "-e" is
passed as an argument to echo and suppresses the "-e" from its
output).

Instead, we should use printf, which is more predictable:

printf '%s\n' "$foo"; # Just prints $foo, on all platforms.

Blindly replacing echo with "printf '%s\n'" would not be good enough
because that printf prints each argument on its own line.  Provide a
sane_echo helper that prints its arguments, space-delimited, on a
single line, to make this easier to remember, and tweak 'say'
and 'die_with_status' to illustrate how it is used.

No functional change intended.

Signed-off-by: Jonathan Nieder 
---
 git-sh-setup.sh | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git i/git-sh-setup.sh w/git-sh-setup.sh
index 256c89a..f35b5b9 100644
--- i/git-sh-setup.sh
+++ w/git-sh-setup.sh
@@ -43,6 +43,10 @@ git_broken_path_fix () {
 
 # @@BROKEN_PATH_FIX@@
 
+sane_echo () {
+   printf '%s\n' "$*"
+}
+
 die () {
die_with_status 1 "$@"
 }
@@ -50,7 +54,7 @@ die () {
 die_with_status () {
status=$1
shift
-   printf >&2 '%s\n' "$*"
+   sane_echo >&2 "$*"
exit "$status"
 }
 
@@ -59,7 +63,7 @@ GIT_QUIET=
 say () {
if test -z "$GIT_QUIET"
then
-   printf '%s\n' "$*"
+   sane_echo "$*"
fi
 }
 
--
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

[ANNOUNCE] Git v1.9.1

[Junio C Hamano]

The latest maintenance release Git v1.9.1 is now available at
the usual places.

The release tarballs are found at:

https://www.kernel.org/pub/software/scm/git/

The following public repositories all have a copy of the v1.9.1
tag and the maint branch that the tag points at:

  url = https://kernel.googlesource.com/pub/scm/git/git
  url = git://repo.or.cz/alt-git.git
  url = https://code.google.com/p/git-core/
  url = git://git.sourceforge.jp/gitroot/git-core/git.git
  url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core
  url = https://github.com/gitster/git


Git v1.9.1 Release Notes


Fixes since v1.9.0
--

 * "git clean -d pathspec" did not use the given pathspec correctly
   and ended up cleaning too much.

 * "git difftool" misbehaved when the repository is bound to the
   working tree with the ".git file" mechanism, where a textual file
   ".git" tells us where it is.

 * "git push" did not pay attention to branch.*.pushremote if it is
   defined earlier than remote.pushdefault; the order of these two
   variables in the configuration file should not matter, but it did
   by mistake.

 * Codepaths that parse timestamps in commit objects have been
   tightened.

 * "git diff --external-diff" incorrectly fed the submodule directory
   in the working tree to the external diff driver when it knew it is
   the same as one of the versions being compared.

 * "git reset" needs to refresh the index when working in a working
   tree (it can also be used to match the index to the HEAD in an
   otherwise bare repository), but it failed to set up the working
   tree properly, causing GIT_WORK_TREE to be ignored.

 * "git check-attr" when working on a repository with a working tree
   did not work well when the working tree was specified via the
   --work-tree (and obviously with --git-dir) option.

 * "merge-recursive" was broken in 1.7.7 era and stopped working in
   an empty (temporary) working tree, when there are renames
   involved.  This has been corrected.

 * "git rev-parse" was loose in rejecting command line arguments
   that do not make sense, e.g. "--default" without the required
   value for that option.

 * include.path variable (or any variable that expects a path that
   can use ~username expansion) in the configuration file is not a
   boolean, but the code failed to check it.

 * "git diff --quiet -- pathspec1 pathspec2" sometimes did not return
   correct status value.

 * Attempting to deepen a shallow repository by fetching over smart
   HTTP transport failed in the protocol exchange, when no-done
   extension was used.  The fetching side waited for the list of
   shallow boundary commits after the sending end stopped talking to
   it.

 * Allow "git cmd path/", when the 'path' is where a submodule is
   bound to the top-level working tree, to match 'path', despite the
   extra and unnecessary trailing slash (such a slash is often
   given by command line completion).



Changes since v1.9.0 are as follows:

Brad King (4):
  t3030-merge-recursive: test known breakage with empty work tree
  read-cache.c: refactor --ignore-missing implementation
  read-cache.c: extend make_cache_entry refresh flag with options
  merge-recursive.c: tolerate missing files while refreshing index

David Aguilar (1):
  difftool: support repositories with .git-files

David Sharp (1):
  rev-parse: check i before using argv[i] against argc

Jeff King (12):
  expand_user_path: do not look at NULL path
  handle_path_include: don't look at NULL value
  tests: auto-set LIB_HTTPD_PORT from test name
  t4212: test bogus timestamps with git-log
  fsck: report integer overflow in author timestamps
  date: check date overflow against time_t
  log: handle integer overflow in timestamps
  log: do not segfault on gmtime errors
  remote: handle pushremote config in any order
  show_ident_date: fix tz range check
  clean: respect pathspecs with "-d"
  clean: simplify dir/not-dir logic

Junio C Hamano (4):
  t0003: do not chdir the whole test process
  check-attr: move to the top of working tree when in non-bare repository
  t7800: add a difftool test for .git-files
  Git 1.9.1

Nguyễn Thái Ngọc Duy (17):
  test: rename http fetch and push test files
  pack-protocol.txt: clarify 'obj-id' in the last ACK after 'done'
  protocol-capabilities.txt: refer multi_ack_detailed back to 
pack-protocol.txt
  protocol-capabilities.txt: document no-done
  fetch-pack: fix deepen shallow over smart http with no-done cap
  t5537: move http tests out to t5539
  reset: optionally setup worktree and refresh index on --mixed
  pathspec: convert some match_pathspec_depth() to ce_path_match()
  pathspec: convert some match_pathspec_depth() to dir_path_match()
  pathspec: rename match_pathspec_depth() to matc

Re: [PATCH v2] tests: set temp variables using 'env' in test function instead of subshell

[Jeff King]

On Tue, Mar 18, 2014 at 01:37:39PM -0700, Junio C Hamano wrote:

> > diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
> > index c9c426c..3e3f77b 100755
> > --- a/t/t1300-repo-config.sh
> > +++ b/t/t1300-repo-config.sh
> > @@ -974,24 +974,15 @@ test_expect_success SYMLINKS 'symlinked 
> > configuration' '
> >  '
> >
> >  test_expect_success 'nonexistent configuration' '
> > -   (
> > -   GIT_CONFIG=doesnotexist &&
> > -   export GIT_CONFIG &&
> > -   test_must_fail git config --list &&
> > -   test_must_fail git config test.xyzzy
> > -   )
> > +   test_must_fail env GIT_CONFIG=doesnotexist git config --list &&
> > +   test_must_fail env GIT_CONFIG=doesnotexist git config test.xyzzy
> >  '

Isn't GIT_CONFIG here another way of saying:

  test_must_fail git config -f doesnotexist --list

Perhaps that is shorter and more readable still (and there are a few
similar cases in this patch.

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

Re: [PATCH] branch.c: simplify chain of if statements

[Eric Sunshine]

On Mon, Mar 17, 2014 at 7:46 AM, Dragos Foianu  wrote:
> Eric Sunshine  sunshineco.com> writes:
>> Matthieu already mentioned [2] that this sort of "lego" string
>> construction is not internationalization-friendly. See section 4.3 [3]
>> of the gettext manual for details.
>
> I was hoping to get away with using less memory by having only four entries
> in the table. I suppose that is not possible. The rebasing check can still
> be moved outside of the four if statements and calculate the index
> correctly. The strings would then have to be arranged in such a way to make
> this work.
>
> Using a multiple-dimension array as suggested in other submissions for this
> particular microproject would probably be better, but it has already been 
> done.

If a multi-dimension table is indeed better than other alternatives,
then that's a good reason to choose it, even if others have already
used that approach in their submissions. It's more important that the
code is clean and easy to understand and maintain than to be clever.

If you're really interested in trying an approach not already
submitted by others, take a look at Jonathan's idea [1]. If you play
around with it and find that it actually does make the code clearer
and simpler, then perhaps it's worth submitting. If not, then not.

[1]: http://thread.gmane.org/gmane.comp.version-control.git/198882/focus=198902

>> These hard-coded indexing constants (0, 1, 2, 3) are fragile and
>> convey little meaning to the reader. Try to consider how to compute
>> the index into verbose_prints[] based upon the values of
>> 'remote_is_branch' and 'origin'. What are the different ways you could
>> do so?
>
> I was going to do something like this: if !remote_is_branch the index goes
> incremented by 2, because the first two entries are of no interest and if
> !origin, the index is incremented by 1. This would correctly compute the
> index. It should also work with the rebasing check if the four
> rebasing-specific messages are at the end of the table and when rebasing the
> index is set to start at those messages.
>
> The reason I did not go with this is because I would still need the four ifs
> in order to keep the bug check part of the code. I might be able to find a
> work-around for it on the second attempt.

Since the result is just a number, its possible to compute it directly
without conditionals, however, it does start resembling a magical
incantation. (I'll comment further in your v2 submission.)

> I have seen N_() used in other code but I wasn't sure what its purpose was.
>
> Thank you very much for the review.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] tests: set temp variables using 'env' in test function instead of subshell

[Eric Sunshine]

On Tue, Mar 18, 2014 at 8:08 AM, David Tran  wrote:
> Originally, the code used subshells instead of FOO=BAR command because
> the variable would otherwise leak into the surrounding context of the POSIX
> shell when 'command' is a shell function. The subshell was used to hold the
> context for the test. Using 'env' in the test function sets the temp variables
> without leaking, removing the need of a subshell.
>
> Signed-off-by: David Tran 
> ---
>> Oh, really ;-)?
> Missed that.
>
>> Thanks.  Getting closer, I think.
> Slowly but surely.

Getting better. See below.

> Signed-off-by: David Tran 
> ---
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index 50e22b1..4c7364a 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -104,9 +104,7 @@ test_expect_success 'rebase -i with the exec command 
> checks tree cleanness' '
> git checkout master &&
> (
> set_fake_editor &&
> -   FAKE_LINES="exec_echo_foo_>file1 1" &&
> -   export FAKE_LINES &&
> -   test_must_fail git rebase -i HEAD^
> +   test_must_fail env FAKE_LINES="exec_echo_foo_>file1 1" git rebase -i 
> HEAD^
> ) &&

In a previous review, I asked if this subshell could be dropped or if
it was required for set_fake_editor. I didn't quite understand your
response, so I tested it myself, and found that the subshell can be
eliminated safely without breaking this or later tests.

> test_cmp_rev master^ HEAD &&
> git reset --hard &&
> @@ -118,9 +116,8 @@ test_expect_success 'rebase -i with exec of inexistent 
> command' '
> test_when_finished "git rebase --abort" &&
> (
> set_fake_editor &&
> -   FAKE_LINES="exec_this-command-does-not-exist 1" &&
> -   export FAKE_LINES &&
> -   test_must_fail git rebase -i HEAD^ >actual 2>&1
> +   test_must_fail env FAKE_LINES="exec_this-command-does-not-exist 1" \
> +   git rebase -i HEAD^ >actual 2>&1
> ) &&

Ditto for this subshell.

> ! grep "Maybe git-rebase is broken" actual
>  '
> @@ -528,11 +509,7 @@ test_expect_success 'aborted --continue does not squash 
> commits after "edit"' '
> FAKE_LINES="edit 1" git rebase -i HEAD^ &&
> echo "edited again" > file7 &&
> git add file7 &&
> -   (
> -   FAKE_COMMIT_MESSAGE=" " &&
> -   export FAKE_COMMIT_MESSAGE &&
> -   test_must_fail git rebase --continue
> -   ) &&
> +   test_must_fail env FAKE_COMMIT_MESSAGE=" " git rebase --continue

Broken &&-chain.

> test $old = $(git rev-parse HEAD) &&
> git rebase --abort
>  '
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] tests: set temp variables using 'env' in test function instead of subshell

[Junio C Hamano]

David Tran  writes:

> Originally, the code used subshells instead of FOO=BAR command
> because the variable would otherwise leak into the surrounding
> context of the POSIX shell when 'command' is a shell function.
> The subshell was used to hold the context for the test. Using
> 'env' in the test function sets the temp variables without
> leaking, removing the need of a subshell.

These are not "temp variables" ;-).

You are improving the way how commands are run under a different
settings to environment variables.

Hmm, let's try to see if I can do better:

Subject: tests: use "env" to run commands with temporary env-var 
settings

Ordinarily, we would say "VAR=VAL command" to execute a
tested command with environment variable(s) set only for
that command.  This however does not work if 'command' is a
shell function (most notably 'test_must_fail'); the result
of the assignment is retained and affects later commands.

To avoid this, we used to assign and export environment
variables and run such a test in a subshell,

(
VAR=VAL && export VAR &&
test_must_fail git command to be tested
)

but with "env" utility, we should be able to say

test_must_fail env VAR=VAL git command to be tested

which is much shorter and easier to read.

> Let's see if I replied correctly with send-email. Retrying this again.
> How do I 'reply' to a thread using send-email?

Look for --in-reply-to option in "man git-send-email".

> Signed-off-by: David Tran 
> ---
>  t/t1300-repo-config.sh|   17 ++
>  t/t1510-repo-setup.sh |4 +--
>  t/t3200-branch.sh |   12 +--
>  t/t3301-notes.sh  |   22 --
>  t/t3404-rebase-interactive.sh |   65 
>  t/t3413-rebase-hook.sh|6 +---
>  t/t4014-format-patch.sh   |   14 ++---
>  t/t5305-include-tag.sh|4 +--
>  t/t5602-clone-remote-exec.sh  |   13 ++--
>  t/t5801-remote-helpers.sh |6 +--
>  t/t6006-rev-list-format.sh|9 ++
>  t/t7006-pager.sh  |   18 ++-
>  12 files changed, 42 insertions(+), 148 deletions(-)

Thanks.  The numbers look very good ;-)  We love code reduction.

> diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
> index c9c426c..3e3f77b 100755
> --- a/t/t1300-repo-config.sh
> +++ b/t/t1300-repo-config.sh
> @@ -974,24 +974,15 @@ test_expect_success SYMLINKS 'symlinked configuration' '
>  '
>
>  test_expect_success 'nonexistent configuration' '
> - (
> - GIT_CONFIG=doesnotexist &&
> - export GIT_CONFIG &&
> - test_must_fail git config --list &&
> - test_must_fail git config test.xyzzy
> - )
> + test_must_fail env GIT_CONFIG=doesnotexist git config --list &&
> + test_must_fail env GIT_CONFIG=doesnotexist git config test.xyzzy
>  '
>
>  test_expect_success SYMLINKS 'symlink to nonexistent configuration' '
>   ln -s doesnotexist linktonada &&
>   ln -s linktonada linktolinktonada &&
> - (
> - GIT_CONFIG=linktonada &&
> - export GIT_CONFIG &&
> - test_must_fail git config --list &&
> - GIT_CONFIG=linktolinktonada &&
> - test_must_fail git config --list
> - )
> + test_must_fail env GIT_CONFIG=linktonada git config --list &&
> + test_must_fail env GIT_CONFIG=linktolinktonada git config --list
>  '
>
>  test_expect_success 'check split_cmdline return' "
> diff --git a/t/t1510-repo-setup.sh b/t/t1510-repo-setup.sh
> index cf2ee78..e1b2a99 100755
> --- a/t/t1510-repo-setup.sh
> +++ b/t/t1510-repo-setup.sh
> @@ -777,9 +777,7 @@ test_expect_success '#30: core.worktree and core.bare 
> conflict (gitfile version)
>   setup_repo 30 "$here/30" gitfile true &&
>   (
>   cd 30 &&
> - GIT_DIR=.git &&
> - export GIT_DIR &&
> - test_must_fail git symbolic-ref HEAD 2>result
> + test_must_fail env GIT_DIR=.git git symbolic-ref HEAD 2>result
>   ) &&
>   grep "core.bare and core.worktree" 30/result
>  '
> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
> index fcdb867..d45e95c 100755
> --- a/t/t3200-branch.sh
> +++ b/t/t3200-branch.sh
> @@ -849,11 +849,7 @@ test_expect_success 'detect typo in branch name when 
> using --edit-description' '
>   write_script editor <<-\EOF &&
>   echo "New contents" >"$1"
>   EOF
> - (
> - EDITOR=./editor &&
> - export EDITOR &&
> - test_must_fail git branch --edit-description no-such-branch
> - )
> + test_must_fail env EDITOR=./editor git branch --edit-description 
> no-such-branch
>  '
>
>  test_expect_success 'refuse --edit-description on unborn branch for now' '
> @@ -861,11 +857,7 @@ test_expect_success 'refuse --edit-desc

Re: [PATCH] Add grep.fullName config variable

[Junio C Hamano]

Andreas Schwab  writes:

> Junio C Hamano  writes:
>
>> Don't we have the exact same issue for the editor, by the way?
>> Shouldn't we be running it in the original subdirectory as well?
>
> It's called with an absolute name, so it shouldn't care.

But we should not have to call with absolute paths when a short and
sweet pathname relative to the user's current directory. That is the
primary point of my comment.


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

Re: [PATCH v2] Documentation/gitk: Document new config file location

[Junio C Hamano]

Astril Hayato  writes:

> User config file location now complies with XDG base directory specification
>
> Signed-off-by: Astril Hayato 
> ---
>  Documentation/gitk.txt | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/gitk.txt b/Documentation/gitk.txt
> index 1e9e38a..c2aa514 100644
> --- a/Documentation/gitk.txt
> +++ b/Documentation/gitk.txt
> @@ -166,8 +166,13 @@ gitk --max-count=100 --all \-- Makefile::
>  
>  Files
>  -
> -Gitk creates the .gitk file in your $HOME directory to store preferences
> -such as display options, font, and colors.
> +User configuration and preferences are stored at (in order of priority):
> +
> +* '$XDG_CONFIG_HOME/git/gitk' if it exists and '$XDG_CONFIG_HOME' is set
> +* '$HOME/.config/git/gitk' if it exists
> +* '$HOME/.gitk' if it exists
> +
> +If none of the above exist then '$HOME/.config/git/gitk' is created and used 
> by default.

The last line is a bit of a lie, isn't it?

If XDG_CONFIG_HOME is set to an non-empty string, config_file is set
to $XDG_CONFIG_HOME/git/gitk.  Otherwise config_file is set to
$HOME/.config/git/gitk.

After that happens:

 - if that file exists, it is used;

 - otherwise:

   . if $HOME/.gitk exists, we use it (in other words, $HOME/.gitk
 is never used unless the user is an old timer who has one);

   . if $HOME/.gitk does not exist (in other words, if none of the
 above exists), then an empty $config_file is created and used.

We use either $HOME/.config/git/gitk or $XDG_CONFIG_HOME/git/gitk
and never $HOME/.gitk when none of the above exists, and the choice
between the two depends on XDG_CONFIG_HOME.

I'll queue this patch as-is, but we may want to further clarify with
a follow-up patch.

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 v2] tests: set temp variables using 'env' in test function instead of subshell

[David Tran]

Originally, the code used subshells instead of FOO=BAR command because
the variable would otherwise leak into the surrounding context of the POSIX
shell when 'command' is a shell function. The subshell was used to hold the
context for the test. Using 'env' in the test function sets the temp variables
without leaking, removing the need of a subshell.

Signed-off-by: David Tran 

---

Let's see if I replied correctly with send-email. Retrying this again.
How do I 'reply' to a thread using send-email?

I am David Tran a graduating CS/Math senior from Sonoma State University,
United States. I would like to work with git for GSoC'14, specifically the line
options for git rebase --interactive [1]. I have used git for a few years and
know how destructive but important rebase is to git. I have created a few shell
scripts here and there to make life using bash/zsh easier. I would like to
apply these skills and work with the best.

Github: unsignedzero

[1]: http://thread.gmane.org/gmane.comp.version-control.git/243933/focus=243967

> Oh, really ;-)?
Missed that.

> Thanks.  Getting closer, I think.
Slowly but surely.

Signed-off-by: David Tran 
---
 t/t1300-repo-config.sh|   17 ++
 t/t1510-repo-setup.sh |4 +--
 t/t3200-branch.sh |   12 +--
 t/t3301-notes.sh  |   22 --
 t/t3404-rebase-interactive.sh |   65 
 t/t3413-rebase-hook.sh|6 +---
 t/t4014-format-patch.sh   |   14 ++---
 t/t5305-include-tag.sh|4 +--
 t/t5602-clone-remote-exec.sh  |   13 ++--
 t/t5801-remote-helpers.sh |6 +--
 t/t6006-rev-list-format.sh|9 ++
 t/t7006-pager.sh  |   18 ++-
 12 files changed, 42 insertions(+), 148 deletions(-)

diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index c9c426c..3e3f77b 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -974,24 +974,15 @@ test_expect_success SYMLINKS 'symlinked configuration' '
 '

 test_expect_success 'nonexistent configuration' '
-   (
-   GIT_CONFIG=doesnotexist &&
-   export GIT_CONFIG &&
-   test_must_fail git config --list &&
-   test_must_fail git config test.xyzzy
-   )
+   test_must_fail env GIT_CONFIG=doesnotexist git config --list &&
+   test_must_fail env GIT_CONFIG=doesnotexist git config test.xyzzy
 '

 test_expect_success SYMLINKS 'symlink to nonexistent configuration' '
ln -s doesnotexist linktonada &&
ln -s linktonada linktolinktonada &&
-   (
-   GIT_CONFIG=linktonada &&
-   export GIT_CONFIG &&
-   test_must_fail git config --list &&
-   GIT_CONFIG=linktolinktonada &&
-   test_must_fail git config --list
-   )
+   test_must_fail env GIT_CONFIG=linktonada git config --list &&
+   test_must_fail env GIT_CONFIG=linktolinktonada git config --list
 '

 test_expect_success 'check split_cmdline return' "
diff --git a/t/t1510-repo-setup.sh b/t/t1510-repo-setup.sh
index cf2ee78..e1b2a99 100755
--- a/t/t1510-repo-setup.sh
+++ b/t/t1510-repo-setup.sh
@@ -777,9 +777,7 @@ test_expect_success '#30: core.worktree and core.bare 
conflict (gitfile version)
setup_repo 30 "$here/30" gitfile true &&
(
cd 30 &&
-   GIT_DIR=.git &&
-   export GIT_DIR &&
-   test_must_fail git symbolic-ref HEAD 2>result
+   test_must_fail env GIT_DIR=.git git symbolic-ref HEAD 2>result
) &&
grep "core.bare and core.worktree" 30/result
 '
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index fcdb867..d45e95c 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -849,11 +849,7 @@ test_expect_success 'detect typo in branch name when using 
--edit-description' '
write_script editor <<-\EOF &&
echo "New contents" >"$1"
EOF
-   (
-   EDITOR=./editor &&
-   export EDITOR &&
-   test_must_fail git branch --edit-description no-such-branch
-   )
+   test_must_fail env EDITOR=./editor git branch --edit-description 
no-such-branch
 '

 test_expect_success 'refuse --edit-description on unborn branch for now' '
@@ -861,11 +857,7 @@ test_expect_success 'refuse --edit-description on unborn 
branch for now' '
echo "New contents" >"$1"
EOF
git checkout --orphan unborn &&
-   (
-   EDITOR=./editor &&
-   export EDITOR &&
-   test_must_fail git branch --edit-description
-   )
+   test_must_fail env EDITOR=./editor git branch --edit-description
 '

 test_expect_success '--merged catches invalid object names' '
diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index 3bb79a4..cfd67ff 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -17,7 +17,7 @@ GIT_EDITOR=./fake_editor.sh
 export GIT_EDITOR

 test_expect_success 'can

Re: [PATCH] Add grep.fullName config variable

[Andreas Schwab]

Andreas Schwab  writes:

> Junio C Hamano  writes:
>
>> Andreas Schwab  writes:
>>
>>> This configuration variable sets the default for the --full-name option.
>>>
>>> Signed-off-by: Andreas Schwab 
>>> ---
>>
>> Would this change break Porcelains (e.g. Emacs modes) and force them
>> to be updated to explicitly pass --no-full-name to unbreak them?
>
> Yes, that would be required.  On the other hand, currently it is
> impossible to cut-n-paste a file name without --full-name, since the
> pager is always started in top-level.  Perhaps it is better to fix the
> latter?

On the third hand, git grep isn't plumbing, so variation of output is to
be expected?  We already have grep.lineNumber and grep.patternType /
grep.extendedRegexp (vc-git-grep uses -n itself, but does not protect
against grep.patternType).

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Git won Linux Magazine's Linux New Media Award in the category "Outstanding Contribution to Open Source/Linux/Free Software"

[Junio C Hamano]

Richard Hartmann  writes:

> Dear all,
>
> Git won an award in the main category of the English & German Linux
> Magazine at CeBIT, this year. Jens Lehmann, Heiko Voigt, and myself
> were present to accept the award on behalf of the Git community as a
> whole.
>
> You can find a short blurb on my blog[1], including a picture of the
> physical prize.
>
> It seems the video of the award ceremony is not up yet, but I have
> been told it will come soon(tm).
>
>
> Best regards,
> Richard
>
> [1] 
> http://richardhartmann.de/blog/posts/2014/03/14-Git_prize-Outstanding_Contribution_to_Open_Source__Linux__Free_Software/

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

Re: [PATCH] Add grep.fullName config variable

[Andreas Schwab]

Junio C Hamano  writes:

> Don't we have the exact same issue for the editor, by the way?
> Shouldn't we be running it in the original subdirectory as well?

It's called with an absolute name, so it shouldn't care.

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH][GSOC2014] add: Rewrite run_add_interactive to use struct argv_array

[Junio C Hamano]

Movchan Pavel  writes:

> Origin code are code with own realisation argv array editing.
> It was changed, and code modified for using unified argv-array
> realisation from argv-array.h.
> Commit for Google Summer of Code 2014
>
> Signed-off-by: Movchan Pavel 
> ---

Thanks.  "Commit for ..." is not something we would want to see in
"git log" output, though.

>  builtin/add.c |   21 ++---
>  1 file changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/builtin/add.c b/builtin/add.c
> index 4b045ba..258b491 100644
> --- a/builtin/add.c
> +++ b/builtin/add.c
> @@ -15,6 +15,7 @@
>  #include "diffcore.h"
>  #include "revision.h"
>  #include "bulk-checkin.h"
> +#include "argv-array.h"
>  
>  static const char * const builtin_add_usage[] = {
>   N_("git add [options] [--] ..."),
> @@ -141,23 +142,21 @@ static void refresh(int verbose, const struct pathspec 
> *pathspec)
>  int run_add_interactive(const char *revision, const char *patch_mode,
>   const struct pathspec *pathspec)
>  {
> - int status, ac, i;
> - const char **args;
> + int status, i;
> + struct argv_array *argv = ARGV_ARRAY_INIT;

Where does that pointer point at and who allocated the piece of
memory used by it?

See http://thread.gmane.org/gmane.comp.version-control.git/244151
for an example solution.

>  
> - args = xcalloc(sizeof(const char *), (pathspec->nr + 6));
> - ac = 0;
> - args[ac++] = "add--interactive";
> + argv_array_push(argv, "add--interactive");
>   if (patch_mode)
> - args[ac++] = patch_mode;
> + argv_array_push(argv, patch_mode);
>   if (revision)
> - args[ac++] = revision;
> - args[ac++] = "--";
> + argv_array_push(argv, revision);
> + argv_array_push(argv, "--");
>   for (i = 0; i < pathspec->nr; i++)
>   /* pass original pathspec, to be re-parsed */
> - args[ac++] = pathspec->items[i].original;
> + argv_array_push(argv, pathspec->items[i].original);
>  
> - status = run_command_v_opt(args, RUN_GIT_CMD);
> - free(args);
> + status = run_command_v_opt(argv->argv, RUN_GIT_CMD);
> + argv_array_clear(argv);
>   return status;
>  }
--
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-rebase: Teach rebase "-" shorthand.

[Junio C Hamano]

Brian Gesiak  writes:

> Teach rebase the same shorthand as checkout and merge; that is, that "-"
> means "the branch we were previously on".
>
> Reported-by: Tim Chase 
> Signed-off-by: Brian Gesiak 
> ---
>  git-rebase.sh | 4 
>  t/t3400-rebase.sh | 6 ++
>  2 files changed, 10 insertions(+)
>
> diff --git a/git-rebase.sh b/git-rebase.sh
> index 5f6732b..2c75e9f 100755
> --- a/git-rebase.sh
> +++ b/git-rebase.sh
> @@ -453,6 +453,10 @@ then
>   test "$fork_point" = auto && fork_point=t
>   ;;
>   *)  upstream_name="$1"
> + if test "$upstream_name" = "-"
> + then
> + upstream_name="@{-1}"
> + fi
>   shift
>   ;;
>   esac
> diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
> index 6d94b1f..00aba9f 100755
> --- a/t/t3400-rebase.sh
> +++ b/t/t3400-rebase.sh
> @@ -88,6 +88,12 @@ test_expect_success 'rebase from ambiguous branch name' '
>   git rebase master
>  '
>  
> +test_expect_success 'rebase using shorthand' '
> + git checkout master
> + git checkout -b shorthand HEAD^
> + GIT_TRACE=1 git rebase -

I'd rather not to see that TRACE there.  We would also want to make
sure the result is what we expect to see, not only the command does
not error out, no?

> +'
> +
>  test_expect_success 'rebase a single mode change' '
>   git checkout master &&
>   git branch -D topic &&
--
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: [GSoC14][RFC] Is there any interest in adding a port of checkpatch.pl to contrib/?

[Junio C Hamano]

Eric Sunshine  writes:

>> 1. Git style guidelines are somewhat different and less strict than
>> their Linux equivalents.
>
> Are checkpatch.pl's customization options, such as --ignore,
> insufficient to make it behave in the desired fashion for git?

If we are to officially encourage the use of checkpatch.pl, we
should stress that mechanically adhering to its check is *not* the
goal.  As long as people view it as a tool to _help_ them spot
obvious problems before sending their patches out, but the final
responsibility to produce readable code is upon themselves, not the
script, I am personally fine.  Oftentimes we find an occasional long
line that is slightly longer than 80-column limit a lot easier to
read than chopping it artificially in the middle, for example.

>> 2. Several patch threads bounce back and forth because of style fixes.
>> A checkpatch script added as a hook could help reduce these and use
>> more efficiently our time.
>> 3. As far as I can tell, checkpatch needs to be run from the root
>> folder of a linux repository clone. Cloning several hundred MBs for a
>> single perl script looks a little foolish to me.
>
> No need to clone the kernel. checkpatch.pl runs fine standalone with
> the --no-tree option.
>
>> So, is there any interest in adding a port of checkpatch.pl to
>> contrib/?

Not really.  Maintaining the forked version is an additional pain we
do not necessarily need.  Are we going to carry a port of gcc and
others?

I would rather prefer to add a paragraph in CodingGuidelines that
points at the canonical location to obtain the script, e.g.

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/scripts/checkpatch.pl

and a procedure to run the script over their patch.  I am too lazy
to check myself, but there must be somewhere we mention the use of
format-patch and send-email in the document, and use of the script
to check would fall naturally between these two steps, I would
think.
--
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: [GSoC14][RFC] Is there any interest in adding a port of checkpatch.pl to contrib/?

[Junio C Hamano]

Jacopo Notarstefano  writes:

> It seems to me that the topic of adding the checkpatch.pl script to
> Git's source tree has cropped up several times in the past, as
> recently as a couple of days ago: $gmane/243607.
>
> It should be noted that its usage for its sake has been discouraged by
> Junio Hamano in $gmane/205998.

I've never said any such thing.

I only said I am not enthused against a proposal to add a build
target that runs checkpatch or equivalent over *all* existing code,
which will invite needless churn (read again the part of the message
you quoted before I say "I am not enthused").

It is a totally separate issue for submitters to make sure they do
not introduce *new* problems, and use of "checkpatch --no-tree"
could be one way to do so.
--
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-rebase: Teach rebase "-" shorthand.

[Torsten Bögershausen]

On 03/18/2014 09:44 AM, Brian Gesiak wrote:

Teach rebase the same shorthand as checkout and merge; that is, that "-"
means "the branch we were previously on".

Reported-by: Tim Chase 
Signed-off-by: Brian Gesiak 
---
  git-rebase.sh | 4 
  t/t3400-rebase.sh | 6 ++
  2 files changed, 10 insertions(+)

diff --git a/git-rebase.sh b/git-rebase.sh
index 5f6732b..2c75e9f 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -453,6 +453,10 @@ then
test "$fork_point" = auto && fork_point=t
;;
*)  upstream_name="$1"
+   if test "$upstream_name" = "-"
+   then
+   upstream_name="@{-1}"
+   fi
shift
;;
esac
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index 6d94b1f..00aba9f 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -88,6 +88,12 @@ test_expect_success 'rebase from ambiguous branch name' '
git rebase master
  '

+test_expect_success 'rebase using shorthand' '
+   git checkout master

we schould have the "&&"   ^^

+   git checkout -b shorthand HEAD^

  we schould have the "&&"  ^^

+   GIT_TRACE=1 git rebase -

And why the GIT_TRACE ?

+'
+
  test_expect_success 'rebase a single mode change' '
git checkout master &&
git branch -D topic &&



--
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] general style: replaces memcmp() with starts_with()

[Junio C Hamano]

Eric Sunshine  writes:

>>> A patch of this nature doesn't require much more description than
>>> stating what it does ("replace memcmp() with starts_with()") and why
>>> ("improve code clarity"). The following rewrite might be sufficient:
>>>
>>> Subject: replace memcmp() with starts_with()
>>>
>>> starts_with() indicates the intention of the check more clearly
>>> than memcmp().
>>
>> This is more concise; thank you. I will adapt this as the message for
>> the next version of this patch. Would it be wise to mention magic
>> numbers, as the discussion surrounding the rationale of this patch,
>> especially with Junio and Michael, has centered around that?
>
> I was thinking of mentioning magic numbers in the example, but decided
> that their removal was a natural and obvious consequence of the
> improvement to code clarity, so it wasn't strictly necessary to talk
> about it. On the other hand, it is a good secondary justification,
> thus it should be perfectly acceptable to mention it. If I was writing
> the commit message, I might start by saying "As an additional
> benefit..." and then talk a bit about magic number retirement.

I think your subject is good (as you said, it makes it clear that it
is a project-wide clean-up by not mentioning any specific area), but
"indicates the intention of the check more clearly" would not tell
new readers who are unfamiliar with the helper API what "intention"
it is talking about very much, so perhaps

Subject: use starts_with() instead of !memcmp()

When checking if a string begins with a constant string,
using starts_with() is less error prone than calling
!memcmp() with an explicit byte count.

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

Re: [PATCH v2] log: add --nonlinear-barrier to help see non-linear history

[Junio C Hamano]

Duy Nguyen  writes:

>> I do not quite understand the "if we do not have previous parents"
>> bit, though.  Is it meant to trigger only at the very beginning?
>
> Only at the beginning.

Then I am not sure !revs->previous_parents match that condition.

What happens in a history with more than one root commits and the
first one the revision traversal finds is one of the roots?  When
showing the second commit, after processing the root and storing its
parent list, which is NULL, to revs->previous_parents, wouldn't it
mistakenly think that it is the first round and fail to show the
break line?
--
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: Using "-" for "previous branch" failing with rebase

[Junio C Hamano]

Tim Chase  writes:

> On 2014-03-16 23:37, Junio C Hamano wrote:
>> Tim Chase  writes:
>> 
>> > Is this just an interface inconsistency or is there a some
>> > technical reason this doesn't work (or, has it been
>> > addressed/fixed, and just not pulled into Debian Stable's
>> > 1.7.10.4 version of git)?
>> 
>> It is merely that nobody thought "rebase" would benefit from such a
>> short-hand, I think.
>> 
>> Teach more commands that operate on branch names about "-"
>> shorthand for "the branch we were previously on", like we did
>> for "git merge -" sometime after we introduced "git checkout -"
>> 
>> has been sitting in my "leftover bits" list at
>> 
>> http://git-blame.blogspot.com/p/leftover-bits.html
>> 
>> for quite some time.  Hint, hint...
>
> Not sure if the "Hint, hint" was intended for me,...

No.  It is primarily a search-engine bait ;-) and is secondarily a
hint to any aspiring Git hackers on the list.

In general, I am not very enthused to see the single letter "-" used
where you name any commit object (like "diff").  It is a short-hand
for the @{-1} notation that names the branch name, and it is OK in
general to add support for it to the places where we expect a branch
name and not just any commit object name (e.g. "checkout -"), but
maybe it is just me who was heavily involved in the original design.

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

Re: [PATCH v3] test-lib.sh: do not "echo" externally supplied strings

[Junio C Hamano]

Uwe Storbeck  writes:

> In some places we "echo" a string that is supplied by the calling
> test script and may contain backslash sequences. The echo command
> of some shells, most notably "dash", interprets these backslash
> sequences (POSIX.1 allows this) which may scramble the test
> output.
>
> Signed-off-by: Uwe Storbeck 
> ---
>
> Commit message rewritten to avoid title continuation in the body.
> Thanks Junio C Hamano for the hint.

Here is what I queued yesterday.  I was wrong to say "control
characters"; a backslash sequence is not necessarily a control
character (e.g. \c at the end that suppresses the final LF), so I'll
replace it with your version.

The test titles are not externally supplied but under our control,
so it is less problematic than the "rebase -i" case where we do get
bitten by user supplied commit title string.  Still it is a good
idea to apply this change to protect ourselves.

Thanks.  

commit 215cd588caebe86fe77115bdda97930b4659aecd
Author: Uwe Storbeck 
Date:   Sat Mar 15 00:57:36 2014 +0100

test-lib.sh: do not "echo" test titles

The test title could be a string with backslash in it, and can be
interpreted as control characters by the echo command of some shells
(e.g. dash).

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

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 1531c24..3c7cb1d 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -277,7 +277,7 @@ error "Test script did not set test_description."
 
 if test "$help" = "t"
 then
-   echo "$test_description"
+   printf '%s\n' "$test_description"
exit 0
 fi
 
@@ -328,7 +328,7 @@ test_failure_ () {
test_failure=$(($test_failure + 1))
say_color error "not ok $test_count - $1"
shift
-   echo "$@" | sed -e 's/^/#   /'
+   printf '%s\n' "$*" | sed -e 's/^/#  /'
test "$immediate" = "" || { GIT_EXIT_OK=t; exit 1; }
 }
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Add grep.fullName config variable

[Junio C Hamano]

Andreas Schwab  writes:

> Yes, that would be required.  On the other hand, currently it is
> impossible to cut-n-paste a file name without --full-name, since the
> pager is always started in top-level.  Perhaps it is better to fix the
> latter?

So far we never cared where the pager runs, but as a principle, I
think it would be nice if we run it in the original subdirectory,
not at the top of the working tree (unless we have to bend backwards
to make the codepath involved too ugly, that is).

Don't we have the exact same issue for the editor, by the way?
Shouldn't we be running it in the original subdirectory as well?

--
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/7] merge hook tests: fix missing '&&' in test

[Junio C Hamano]

Thanks; will replace what has been on 'pu' with the updated series.
--
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

Rebasing merge commits

[Robert Dailey]

What's the general recommendation on rebasing after creating a merge
commit on my branch? I realize rebase has the -p option but it seems
like it does a lot of complicated stuff, and it discourages
interactive rebase with the option.

My situation is simple: I have a topic1 branch that has a few commits
on it. During my work, I notice someone else implements a small bug
fix that I need. I do a `git merge --no-ff` to force that into my
branch as a separate merge commit. After that, I continue my work on
my local topic1 branch as usual. When I want latest from master, I do
`git rebase master`. However now that I have a merge commit on my
branch, things get complicated.

What's a good workflow for this, since it seems pretty common?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] Documentation/gitk: Document new config file location

[Astril Hayato]

User config file location now complies with XDG base directory specification

Signed-off-by: Astril Hayato 
---
 Documentation/gitk.txt | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/Documentation/gitk.txt b/Documentation/gitk.txt
index 1e9e38a..c2aa514 100644
--- a/Documentation/gitk.txt
+++ b/Documentation/gitk.txt
@@ -166,8 +166,13 @@ gitk --max-count=100 --all \-- Makefile::
 
 Files
 -
-Gitk creates the .gitk file in your $HOME directory to store preferences
-such as display options, font, and colors.
+User configuration and preferences are stored at (in order of priority):
+
+* '$XDG_CONFIG_HOME/git/gitk' if it exists and '$XDG_CONFIG_HOME' is set
+* '$HOME/.config/git/gitk' if it exists
+* '$HOME/.gitk' if it exists
+
+If none of the above exist then '$HOME/.config/git/gitk' is created and used 
by default.
 
 History
 ---
-- 
1.9.0

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

Re: Apply commits from one branch to another branch (tree structure is different)

[Brandon McCaig]

Jagan:

On Fri, Mar 14, 2014 at 12:39 PM, Jagan Teki  wrote:
> Hi,

Hello,

> I have two branch in one repo that I need to maintain for 2 different
> deliveries.
> Say branch1 and branch2 in test.git repo.
>
> test.git
> - branch1
>  foo_v1/text.txt
>  foo_v2/text.txt
> - branch2
>  foo/text.txt
>
> branch1 is developers branch all source looks version'ed manner and
> branch2 is superset for branch1, example foo_v1 and foo_v2 are the directories
> in branch1 where developer will update the latest one here foo_v2 and branch2
> foo is same as the latest one of branch1 for an instance.
>
> Suppose developer send 10 patches on branch1 where are changes in terms
> of _/ then I need to apply on my local repo branch1, till now
> is fine then I need to apply same 10 patches on to my branch2 where source
> tree  which is quite question here how can I do.
>
> Request for any help! let me know for any questions.

This just sounds like a painful workflow to me. I would suggest not
doing this at all, but rather using tags to mark specific releases,
and using individual branches for continued development (e.g., stable
or v1.x or whatever is most appropriate). You can have unstable or
master or dev or whatever for developers to work on freely without
breaking releases (albeit, there are many different workflows you can
use to manage the transition from unstable to stable code).

I would avoid using subtrees (subdirectories) within a Git repository
to represent different releases of the code. Git already tracks
versions. That is redundant and messy. It's really an outdated way of
thinking about version control.

/my 2 cents

Regards,


-- 
Brandon McCaig  
Castopulence Software 
Blog 
perl -E '$_=q{V zrna gur orfg jvgu jung V fnl. }.
q{Vg qbrfa'\''g nyjnlf fbhaq gung jnl.};
tr/A-Ma-mN-Zn-z/N-Zn-zA-Ma-m/;say'
--
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: Apply commits from one branch to another branch (tree structure is different)

[Brandon McCaig]

Jagan:

On Fri, Mar 14, 2014 at 1:11 PM, Jagan Teki  wrote:
> Don't know what happen, I'm unable to join #git channel
> [23:40]  hi
> [23:40] == Cannot send to channel: #git

I'm not sure if this is the problem that you were having, but #git on
freenode is NickServ protected. You need to register with NickServ (a
bot on the network) and identify yourself with it in order to be
allowed to speak in #git. This cuts down on spam. You can `/msg
NickServ help' to learn how to use it (and also Google will be your
friend).

Alternatively, check the channel topic for an alternative solution.

Regards,


-- 
Brandon McCaig  
Castopulence Software 
Blog 
perl -E '$_=q{V zrna gur orfg jvgu jung V fnl. }.
q{Vg qbrfa'\''g nyjnlf fbhaq gung jnl.};
tr/A-Ma-mN-Zn-z/N-Zn-zA-Ma-m/;say'
--
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][GSOC] Selection of the verbose message is replaced with generated message in install_branch_config()

[Aleksey Mokhovikov]

Matthieu Moy  grenoble-inp.fr> writes:

>
> Hi,
>
> Aleksey Mokhovikov  gmail.com> writes:
>
> Please, read the threads for other submissions for this microproject.
> Most remarks done there also apply for your case. See for example:
>
>   http://thread.gmane.org/gmane.comp.version-control.git/244210
>


Thank you for your reply.

I've read a bit more GNU gettext manual to get information
about translation with GNU gettext. Long story short, the idea to
generate message from parts will produce even more problems, despite
the message strings passed to the _() are equal before and after the patch.

So I decided to make an array with all messages and mark them for translation 
with "N_()" to keep them together. Because
we now have an array we can improve it to make a table with message format 
string and arguments. Now we need just to
find the proper message index to print the message.

Please look at another approach:
http://permalink.gmane.org/gmane.comp.version-control.git/244357

--
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][GSOC] Selection of the verbose message is replaced with generated message in install_branch_config()

[Aleksey Mokhovikov]

This patch replaces if chain with
2 dimensional array of format strings and arguments.


Signed-off-by: Aleksey Mokhovikov 
---
This patch is unrelated with previous one, but related to GSoC.
So I don't know if I should create new thread for this patch.

Compare with original construction
Pros:
1) Remove if chain.
2) Single table contains all messages with arguments in one construction.
3) Less code duplication.
Cons:
1) Harder to associate the case with message.
2) Harder for compiler to warn the programmer about not
   enough arguments for format string.
3) Harder to add non-string argument to messages.

If !!(x) is out of the coding guide, then message_id construction
can be rewritten in several other ways.
The guideline tells that !!(x) is not welcome and should not be
unless needed. But looks like this is normal place for it.

P.S.
Thanks to comments I got, so
I've read more GNU gettext manual to get info
about translation workflow. When I posted a first patch
I've passed the same message format strings to gettext /*_()*/
as in original, to save the context of the message. And they
will be translated if every possible string combination
will be marked separately for translation. Because of
xgettext can extract string from source automatically,
it ruins the idea to generate message from parts. Even if the
exaclty same message format string is passed to gettext.

 branch.c | 53 ++---
 1 file changed, 30 insertions(+), 23 deletions(-)

diff --git a/branch.c b/branch.c
index 723a36b..51a5faf 100644
--- a/branch.c
+++ b/branch.c
@@ -47,12 +47,32 @@ static int should_setup_rebase(const char *origin)
return 0;
 }

+   
 void install_branch_config(int flag, const char *local, const char *origin, 
const char *remote)
 {
const char *shortname = remote + 11;
int remote_is_branch = starts_with(remote, "refs/heads/");
struct strbuf key = STRBUF_INIT;
int rebasing = should_setup_rebase(origin);
+   int message_id = (!!remote_is_branch << 2) | (!!origin << 1) | 
(!!rebasing);
+   const char *message_table[][4] =
+   {{N_("Branch %s set up to track local ref %s."),
+ local, remote},
+{N_("Branch %s set up to track local ref %s by rebasing."),
+ local, remote},
+{N_("Branch %s set up to track remote ref %s."),
+ local, remote},
+{N_("Branch %s set up to track remote ref %s by rebasing."),
+ local, remote},
+{N_("Branch %s set up to track local branch %s."),
+ local, shortname},
+{N_("Branch %s set up to track local branch %s by rebasing."),
+ local, shortname},
+{N_("Branch %s set up to track remote branch %s from %s."),
+ local, shortname, origin},
+{N_("Branch %s set up to track remote branch %s from %s by 
rebasing."),
+ local, shortname, origin}};
+   const char **message = NULL;

if (remote_is_branch
&& !strcmp(local, shortname)
@@ -68,7 +88,7 @@ void install_branch_config(int flag, const char *local, const 
char *origin, cons
strbuf_reset(&key);
strbuf_addf(&key, "branch.%s.merge", local);
git_config_set(key.buf, remote);
-
+   
if (rebasing) {
strbuf_reset(&key);
strbuf_addf(&key, "branch.%s.rebase", local);
@@ -77,29 +97,16 @@ void install_branch_config(int flag, const char *local, 
const char *origin, cons
strbuf_release(&key);

if (flag & BRANCH_CONFIG_VERBOSE) {
-   if (remote_is_branch && origin)
-   printf_ln(rebasing ?
- _("Branch %s set up to track remote branch %s 
from %s by rebasing.") :
- _("Branch %s set up to track remote branch %s 
from %s."),
- local, shortname, origin);
-   else if (remote_is_branch && !origin)
-   printf_ln(rebasing ?
- _("Branch %s set up to track local branch %s 
by rebasing.") :
- _("Branch %s set up to track local branch 
%s."),
- local, shortname);
-   else if (!remote_is_branch && origin)
-   printf_ln(rebasing ?
- _("Branch %s set up to track remote ref %s by 
rebasing.") :
- _("Branch %s set up to track remote ref %s."),
- local, remote);
-   else if (!remote_is_branch && !origin)
-   printf_ln(rebasing ?
- _("Branch %s set up to track local ref %s by 
rebasing.") :
- _("Branch %s set up to track local ref %s."),
-

Re: [PATCH v3 0/8] Hiding refs

[Duy Nguyen]

On Tue, Mar 18, 2014 at 12:17:39AM -0400, Jeff King wrote:
> On Fri, Mar 14, 2014 at 05:09:45PM -0700, Shawn Pearce wrote:
> 
> > On Fri, Mar 14, 2014 at 4:30 PM, Duy Nguyen  wrote:
> > > On Fri, Mar 14, 2014 at 11:45 PM, Shawn Pearce  
> > > wrote:
> > >>
> > >> You missed the SSH case. It doesn't have this slot to hide the data into.
> > >
> > > Right now we run this for ssh case: "ssh  git-upload-pack
> > > ". New client can do this instead
> > >
> > > ssh  git-upload-pack  
> > 
> > Older servers will fail on this command, and the client must reconnect
> > over SSH, which may mean supplying their password/passphrase again.
> > But its remembered that the uploadPack2 didn't work so this can be
> > blacklisted and not retried for a while.
> 
> I wonder if we could use the environment for optional values. E.g., can
> we run:
> 
>   ssh host GIT_CAPABILITIES=... git-upload-pack 
> 
> That will not work everywhere, of course. Sites with git-shell will
> fail, as will sites with custom ssh handler (GitHub, for example, and I
> imagine Gerrit sites, if they support ssh). So we'd still need some
> fallback, but it would work out-of-the-box in a reasonable number of
> cases (and it is really not that different than the http case, which is
> just stuffing the values into $QUERY_STRING anyway :) ).

Aggressively gc'ing linux-2.6 takes forever (and it's being timed so I
can't really do any heavy lifting), so I outlined what the new
protocol would be instead.

Note that at least for upload-pack client capabilities can be
advertised twice: the first time at transport connection level, the
second time in the first "want", like in v1. I think this will keep
the code change down when we have to support both protocols. Moving
all capabilities to the first negotiation may touch many places, but
that's for now a baseless guess.

The new capability negotiation is also added for push. We didn't pay
much attention to it so far.

I thought about "GIT_CAPABILITIES= git-upload-pack ..." (and actually
added it in pack-protocol.txt then deleted). The thing is, if you want
to new upload-pack, you would need new git-upload-pack at the remote
end that must understand "git-upload-pack  "
already. Making it aware about GIT_CAPABILITIES is extra cost for
nothing. And we have to update git-shell to support it eventually.

Well, the "must understand" part is not entirely true. If you make
git-daemon pass the early capabilities via GIT_CAPABILITIES too,
upload-pack does not have to support " " syntax. The
upside is if old git-upload-pack ignores this GIT_CAPABILITIES, it'll
break the protocol (see below) and can print friendly error
messages. git-daemon has no way of printing friendly messages because
it can't negotiate side-band.

I'm still not sure. But we should support either way, not both. Anyway
the text for new protocols:

-- 8< --
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 79e5768..c329eb1 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2084,6 +2084,16 @@ remote.pushdefault::
`branch..remote` for all branches, and is overridden by
`branch..pushremote` for specific branches.
 
+remote.useUploadPack2::
+   Set to "always" to use only upload-pack version 2, "never" to
+   always use the original "upload-pack", "auto" to use the
+   original protocol, but if the remote claims it support version
+   2, then set "remote..useUploadPack2" to
+   "always". Default to "auto".
+
+remote..useUploadPack2::
+   Override remote.useUploadPack2 per remote.
+
 remote..url::
The URL of a remote repository.  See linkgit:git-fetch[1] or
linkgit:git-push[1].
diff --git a/Documentation/technical/pack-protocol.txt 
b/Documentation/technical/pack-protocol.txt
index 39c6410..3db4219 100644
--- a/Documentation/technical/pack-protocol.txt
+++ b/Documentation/technical/pack-protocol.txt
@@ -40,15 +40,22 @@ hostname parameter, terminated by a NUL byte.
 
0032git-upload-pack /project.git\0host=myserver.com\0
 
+Some service may accept an extra argument (e.g. upload-pack version
+2). The extra argument must follow "host".
+
+   0042git-upload-pack /project.git\0host=myserver.com\0flags=someflags\0
+
 --
-   git-proto-request = request-command SP pathname NUL [ host-parameter NUL ]
+   git-proto-request = request-command SP pathname NUL
+  [ host-parameter NUL [ flags-parameter NUL ] ]
request-command   = "git-upload-pack" / "git-receive-pack" /
   "git-upload-archive"   ; case sensitive
pathname  = *( %x01-ff ) ; exclude NUL
host-parameter= "host=" hostname [ ":" port ]
+   flags-parameter   = "flags=" *( %x01-ff ) ; exclude NULL
 --
 
-Only host-parameter is allowed in the git-proto-request. Clients
+No other parameters are allowed in the git-proto-request. Clients
 MUST NOT attempt to send additional parameters. It is used for the
 git-daemon name based virtual hosting.  See --interpola

Re: [PATCH v3 0/8] Hiding refs

[Duy Nguyen]

On Tue, Mar 18, 2014 at 9:27 PM, Duy Nguyen  wrote:
> I thought about "GIT_CAPABILITIES= git-upload-pack ..." (and actually
> added it in pack-protocol.txt then deleted). The thing is, if you want
> to new upload-pack, you would need new git-upload-pack at the remote
> end that must understand "git-upload-pack  "
> already. Making it aware about GIT_CAPABILITIES is extra cost for
> nothing. And we have to update git-shell to support it eventually.
>
> Well, the "must understand" part is not entirely true. If you make
> git-daemon pass the early capabilities via GIT_CAPABILITIES too,
> upload-pack does not have to support " " syntax. The
> upside is if old git-upload-pack ignores this GIT_CAPABILITIES, it'll
> break the protocol (see below) and can print friendly error
> messages. git-daemon has no way of printing friendly messages because
> it can't negotiate side-band.

I should have read my mail one more time before sending. The
"git-upload-pack ignores..." sentence is wrong. If it's old, its
behavior is fixed and it cannot not send or do anything new.

But on the other hand, this is good. The new protocol expects
upload-pack to send its caps in a new pkt-line. The old upload-pack
does not follow this, which should be the indicator for the client
that this server does not support v2, so it could fall back to v1
gracefully. git:// still fails hard because git-daemon is likely old
too and rejects it from the beginning. But ssh:// (without git-shell)
should work, http:// too. This is a very good point for
GIT_CAPABILITIES.
-- 
Duy
--
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][GSOC] Selection of the verbose message is replaced with generated message in install_branch_config()

[Aleksey Mokhovikov]

Eric Sunshine  sunshineco.com> writes:

> The subject should be concise. Try to keep it at 65-70 characters or
> less. More detailed information can be written following the subject
> (separated from the subject by a blank line).
>
> Write in imperative tone: say "replace X with Y" rather than "X is
> replaced with Y".
>
> Mention the module or function you're touching.
>
> You might say something like this:
>
> Subject: install_branch_config: replace if-chain with string composition
> Wrap lines to 65-70 characters.
>
> This prose is almost pure email commentary. It doesn't really convey
> useful information to a person reading the patch months or years from
> now. Place commentary below the "---" line under your sign-off.

Thanks a lot for you language and message formatting style advices.

I've make a new patch taking into account the GNU gettext requirements.
I don't know if I should create a new thread for another patch, but

I'd be glad if you will give me some information about new patch:
http://permalink.gmane.org/gmane.comp.version-control.git/244357


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

Git won Linux Magazine's Linux New Media Award in the category "Outstanding Contribution to Open Source/Linux/Free Software"

[Richard Hartmann]

Dear all,


Git won an award in the main category of the English & German Linux
Magazine at CeBIT, this year. Jens Lehmann, Heiko Voigt, and myself
were present to accept the award on behalf of the Git community as a
whole.


You can find a short blurb on my blog[1], including a picture of the
physical prize.

It seems the video of the award ceremony is not up yet, but I have
been told it will come soon(tm).


Best regards,
Richard

[1] 
http://richardhartmann.de/blog/posts/2014/03/14-Git_prize-Outstanding_Contribution_to_Open_Source__Linux__Free_Software/
--
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

I have a deal

[Leung W Lok]

Please is your email really active?
--
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][GSOC2014] add: Rewrite run_add_interactive to use struct argv_array

[Movchan Pavel]

Origin code are code with own realisation argv array editing.
It was changed, and code modified for using unified argv-array
realisation from argv-array.h.
Commit for Google Summer of Code 2014

Signed-off-by: Movchan Pavel 
---
 builtin/add.c |   21 ++---
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/builtin/add.c b/builtin/add.c
index 4b045ba..258b491 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -15,6 +15,7 @@
 #include "diffcore.h"
 #include "revision.h"
 #include "bulk-checkin.h"
+#include "argv-array.h"
 
 static const char * const builtin_add_usage[] = {
N_("git add [options] [--] ..."),
@@ -141,23 +142,21 @@ static void refresh(int verbose, const struct pathspec 
*pathspec)
 int run_add_interactive(const char *revision, const char *patch_mode,
const struct pathspec *pathspec)
 {
-   int status, ac, i;
-   const char **args;
+   int status, i;
+   struct argv_array *argv = ARGV_ARRAY_INIT;
 
-   args = xcalloc(sizeof(const char *), (pathspec->nr + 6));
-   ac = 0;
-   args[ac++] = "add--interactive";
+   argv_array_push(argv, "add--interactive");
if (patch_mode)
-   args[ac++] = patch_mode;
+   argv_array_push(argv, patch_mode);
if (revision)
-   args[ac++] = revision;
-   args[ac++] = "--";
+   argv_array_push(argv, revision);
+   argv_array_push(argv, "--");
for (i = 0; i < pathspec->nr; i++)
/* pass original pathspec, to be re-parsed */
-   args[ac++] = pathspec->items[i].original;
+   argv_array_push(argv, pathspec->items[i].original);
 
-   status = run_command_v_opt(args, RUN_GIT_CMD);
-   free(args);
+   status = run_command_v_opt(argv->argv, RUN_GIT_CMD);
+   argv_array_clear(argv);
return status;
 }
 
-- 
1.7.10.4

--
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][GSOC] Selection of the verbose message is replaced with generated message in install_branch_config()

[Aleksey Mokhovikov]

This patch replaces if chain that selects the message with
2 dimensional array of format strings and arguments.


Signed-off-by: Aleksey Mokhovikov 
---
This patch is unrelated with previous one, but related to GSoC.
So I don't know if I should create new thread for this patch.

Compare with original construction
Pros:
1) Remove if chain.
2) Single table contains all messages with arguments in one contruction.
3) Less code duplication.
Cons:
1) Harder to associate the case with message.
2) Harder for compiler to warn the programmer about not
   enough arguments for format string. 
3) Harder to add non-string argument to messages.

If !!(x) is out of the coding guide, then message_id construction
can be rewritten in several other ways.
The guideline tells that !!(x) is not welcome and should not be
unless needed. But looks like this is normal place for it.

P.S.
Thanks to comments I got, so
I've read more GNU gettext manual to get info
about translation workflow. When I posted a first patch
I've passed the same message format strings to gettext /*_()*/
as in original, to save the context of the message. And they
will be translated if every possible string combination
will be marked separately for translation. Because of
xgettext can extract string from source automatically,
it ruins the idea to generate message from parts. Even if the
exaclty same message format string is passed to gettext.

 branch.c | 53 ++---
 1 file changed, 30 insertions(+), 23 deletions(-)

diff --git a/branch.c b/branch.c
index 723a36b..51a5faf 100644
--- a/branch.c
+++ b/branch.c
@@ -47,12 +47,32 @@ static int should_setup_rebase(const char *origin)
return 0;
 }
 
+   
 void install_branch_config(int flag, const char *local, const char *origin, 
const char *remote)
 {
const char *shortname = remote + 11;
int remote_is_branch = starts_with(remote, "refs/heads/");
struct strbuf key = STRBUF_INIT;
int rebasing = should_setup_rebase(origin);
+   int message_id = (!!remote_is_branch << 2) | (!!origin << 1) | 
(!!rebasing);
+   const char *message_table[][4] =
+   {{N_("Branch %s set up to track local ref %s."),
+ local, remote},
+{N_("Branch %s set up to track local ref %s by rebasing."),
+ local, remote},
+{N_("Branch %s set up to track remote ref %s."),
+ local, remote},
+{N_("Branch %s set up to track remote ref %s by 
rebasing."),
+ local, remote},
+{N_("Branch %s set up to track local branch %s."),
+ local, shortname},
+{N_("Branch %s set up to track local branch %s by 
rebasing."),
+ local, shortname},
+{N_("Branch %s set up to track remote branch %s from %s."),
+ local, shortname, origin},
+{N_("Branch %s set up to track remote branch %s from %s by 
rebasing."),
+ local, shortname, origin}};
+   const char **message = NULL;
 
if (remote_is_branch
&& !strcmp(local, shortname)
@@ -68,7 +88,7 @@ void install_branch_config(int flag, const char *local, 
const char *origin, cons
strbuf_reset(&key);
strbuf_addf(&key, "branch.%s.merge", local);
git_config_set(key.buf, remote);
-
+   
if (rebasing) {
strbuf_reset(&key);
strbuf_addf(&key, "branch.%s.rebase", local);
@@ -77,29 +97,16 @@ void install_branch_config(int flag, const char *local, 
const char *origin, cons
strbuf_release(&key);
 
if (flag & BRANCH_CONFIG_VERBOSE) {
-   if (remote_is_branch && origin)
-   printf_ln(rebasing ?
- _("Branch %s set up to track remote branch 
%s from %s by rebasing.") :
- _("Branch %s set up to track remote branch 
%s from %s."),
- local, shortname, origin);
-   else if (remote_is_branch && !origin)
-   printf_ln(rebasing ?
- _("Branch %s set up to track local branch 
%s by rebasing.") :
- _("Branch %s set up to track local branch 
%s."),
- local, shortname);
-   else if (!remote_is_branch && origin)
-   printf_ln(rebasing ?
- _("Branch %s set up to track remote ref %s 
by rebasing.") :
- _("Branch %s set up to track remote ref 
%s."),
- local, remote);
-   else if (!remote_is_branch && !origin)
-   printf_ln(rebasing ?
- _("Branch %s set up to track local ref %s 
by rebasing.") :
- _("Branch %s set up to track local ref 
%s."),
-

Re: [PATCH v2] log: add --nonlinear-barrier to help see non-linear history

[Duy Nguyen]

On Tue, Mar 18, 2014 at 3:32 AM, Junio C Hamano  wrote:
> Nguyễn Thái Ngọc Duy   writes:
>
>> Signed-off-by: Nguyễn Thái Ngọc Duy 
>> ---
>>  v2 renames the option name to --nonlinear-barrier and fixes using it
>>  with --dense. Best used with --no-merges to see patch series.
>
> I think that the earlier name "show linear-break" is more easily
> understood than the new name, but maybe that is just me.  It's not
> like you are blocking something from going forward with a barrier,
> and internally it is called a "break-bar".

I'll change it back.

>>   opt->loginfo = NULL;
>>   maybe_flush_or_die(stdout, "stdout");
>>   return shown;
>
> Does this new feature interact with -z format output in any way?

Hmm.. never thought of it. Right now it's part of the previous commit.

> Should it, and if so how?

No idea.

>> +define_commit_slab(saved_linear, int);
>> +
>> +static void track_linear(struct rev_info *revs, struct commit *commit)
>> +{
>> + struct commit_list *p = revs->previous_parents;
>> +
>> + if (p) {
>> + int got_parent = 0;
>> + for (; p && !got_parent; p = p->next)
>> + got_parent = !hashcmp(p->item->object.sha1,
>> +   commit->object.sha1);
>> + revs->linear = got_parent;
>> + free_commit_list(revs->previous_parents);
>> + } else
>> + revs->linear = 1;
>> + if (revs->reverse) {
>> + if (!revs->saved_linear_slab) {
>> + revs->saved_linear_slab = xmalloc(sizeof(struct 
>> saved_linear));
>> + init_saved_linear(revs->saved_linear_slab);
>> + }
>> +
>> + *saved_linear_at(revs->saved_linear_slab, commit) = 
>> revs->linear;
>> + }
>> + revs->previous_parents = copy_commit_list(commit->parents);
>
> We are showing commit, and the parents (after history
> simplification) of the commit we showed before this commit is kept
> in previous-parents.  If we are one of them, we are showing
> linearly, which makes sense.  While we are accumulating the output
> in the forward direction in preparation for later emitting them in
> reverse, we need to save away the linear-ness bit somewhere, and a
> slab is a logical place to save that, which also makes sense.  But
> why do you need a full int?  Doesn't an unsigned char wide enough?

Yes it is. Will change.

> I also wondered if the saved-parents slab we already have can be
> easily reused for this, but it probably would not help.

That could end up a maintenance nightmare. revision.c is complex
enough as it is.

> I do not quite understand the "if we do not have previous parents"
> bit, though.  Is it meant to trigger only at the very beginning?

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

Re: [RFC/PATCH] diff: simplify cpp funcname regex

[René Scharfe]

Am 18.03.2014 09:02, schrieb Johannes Sixt:

Cc René; do you have any comments regarding grep --function-context?

Am 3/18/2014 6:24, schrieb Jeff King:

On Fri, Mar 14, 2014 at 07:56:46AM +0100, Johannes Sixt wrote:


Consider this code:

   void above()
   {}
   static int Y;
   static int A;
   int bar()
   {
 return X;
   }
   void below()
   {}


Thanks, this example is very helpful.


When you 'git grep --function-context X', then you get this output with
the current pattern, you proposal, and my proposal (file name etc omitted
for brevity):

   int bar()
   {
 return X;
   }


Right, that makes sense to me.


When you 'git grep --function-context Y', what do you want to see? With
the current pattern, and with your pattern that forbids semicolon we get:

   void above()
   {}
   static int Y;
   static int A;

and with my simple pattern, which allows semicolon, we get merely

   static int Y;

because the line itself is a hunk header (and we do not look back any
further) and the next line is as well. That is not exactly "function
context", and that is what I'm a bit worried about.


In global context there is no "function context", of course, so the 
latter makes sense.


"grep --function-context" is about useful context and its implementation 
piggy-backs on the hunk header definitions.  If those are useful then 
the grep output should be fine as well.  IAW: No worries, go ahead. :)


However, I only use the defaults heuristic (which shows just the Y-line 
as well) and don't know C++, so I my opinion on this matter isn't worth 
that much.


René

--
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: Bug report -- "Illegal instruction" on Mac 10.6.8 without XCode installed

[Konstantin Khomoutov]

On Tue, 18 Mar 2014 01:33:25 -0700
Ray Hengst  wrote:

> Hi,
> I am running a Mac 10.6.8 and tried to install git-1.9.0 off of the
> installer (git-1.9.0-intel-universal-snow-leopard.dmg). The
> installation worked fine and gave no error messages. But whenever I
> type in a git command (see below for some I tried), it gives me this
> error message:
> Illegal instruction
> 
> I am completely new to git and mostly new of Unix, but here are some
> commands I tried:
> git
> git help
> git config
> git init
> git clean
> git config --global user.name "John Doe"
> git status
> 
> However, typing "man git" displays typical man pages.

This has nothing to do with your problem: the `man` program is not part
of Git and presumably works; it just finds and reads the specified
manual page--which is just plain text--renders it and shows to you.
And your problem is with misbehaving Git binary.

> I do not have Xcode installed. (it's very hard to find a legacy copy);
> the "make" command also is not present, so I can't use any of the
> workarounds I saw listed.
> I uninstalled git-1.9.0 successfully using the provided script, then
> downloaded the same file again (and installed it) to make sure I
> didn't get a corrupt copy. I had the same problem, however.
> If I can provide any more information just let me know.
> Thanks for any help you can provide.

I once came across this thread [1] on SO which says this might be due
to the binaries compiled for a newer version of the OS than you have
installed.  In any case, [2] suggests the installer is prepared by the
guy behind [3], and that project's bug tracker has a bug for exactly
your problem already filed [4].  You might want to chime in there
with more details if you feel like it.

1. http://stackoverflow.com/q/14268887/720999
2. http://git-scm.com/download/mac
3. http://sourceforge.net/projects/git-osx-installer/
4. http://sourceforge.net/p/git-osx-installer/tickets/97/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Add grep.fullName config variable

[Andreas Schwab]

Junio C Hamano  writes:

> Andreas Schwab  writes:
>
>> This configuration variable sets the default for the --full-name option.
>>
>> Signed-off-by: Andreas Schwab 
>> ---
>
> Would this change break Porcelains (e.g. Emacs modes) and force them
> to be updated to explicitly pass --no-full-name to unbreak them?

Yes, that would be required.  On the other hand, currently it is
impossible to cut-n-paste a file name without --full-name, since the
pager is always started in top-level.  Perhaps it is better to fix the
latter?

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 6/7] merge hook tests: fix and update tests

[Benoit Pierre]

- update 'no editor' hook test and add 'editor' hook test
- make sure the tree is reset to a clean state after running a test
  (using test_when_finished) so later tests are not impacted

Signed-off-by: Benoit Pierre 
---
 t/t7505-prepare-commit-msg-hook.sh | 27 +--
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/t/t7505-prepare-commit-msg-hook.sh 
b/t/t7505-prepare-commit-msg-hook.sh
index 5531abb..03dce09 100755
--- a/t/t7505-prepare-commit-msg-hook.sh
+++ b/t/t7505-prepare-commit-msg-hook.sh
@@ -134,14 +134,26 @@ test_expect_success 'with hook (-c)' '
 
 test_expect_success 'with hook (merge)' '
 
-   head=`git rev-parse HEAD` &&
-   git checkout -b other HEAD@{1} &&
-   echo "more" >> file &&
+   test_when_finished "git checkout -f master" &&
+   git checkout -B other HEAD@{1} &&
+   echo "more" >>file &&
+   git add file &&
+   git commit -m other &&
+   git checkout - &&
+   git merge --no-ff other &&
+   test "`git log -1 --pretty=format:%s`" = "merge (no editor)"
+'
+
+test_expect_success 'with hook and editor (merge)' '
+
+   test_when_finished "git checkout -f master" &&
+   git checkout -B other HEAD@{1} &&
+   echo "more" >>file &&
git add file &&
git commit -m other &&
git checkout - &&
-   git merge other &&
-   test "`git log -1 --pretty=format:%s`" = merge
+   env GIT_EDITOR="\"\$FAKE_EDITOR\"" git merge --no-ff -e other &&
+   test "`git log -1 --pretty=format:%s`" = "merge"
 '
 
 cat > "$HOOK" <<'EOF'
@@ -151,6 +163,7 @@ EOF
 
 test_expect_success 'with failing hook' '
 
+   test_when_finished "git checkout -f master" &&
head=`git rev-parse HEAD` &&
echo "more" >> file &&
git add file &&
@@ -160,6 +173,7 @@ test_expect_success 'with failing hook' '
 
 test_expect_success 'with failing hook (--no-verify)' '
 
+   test_when_finished "git checkout -f master" &&
head=`git rev-parse HEAD` &&
echo "more" >> file &&
git add file &&
@@ -169,6 +183,7 @@ test_expect_success 'with failing hook (--no-verify)' '
 
 test_expect_success 'with failing hook (merge)' '
 
+   test_when_finished "git checkout -f master" &&
git checkout -B other HEAD@{1} &&
echo "more" >> file &&
git add file &&
@@ -178,7 +193,7 @@ test_expect_success 'with failing hook (merge)' '
exit 1
EOF
git checkout - &&
-   test_must_fail git merge other
+   test_must_fail git merge --no-ff other
 
 '
 
-- 
1.9.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 2/7] merge hook tests: use 'test_must_fail' instead of '!'

[Benoit Pierre]

Signed-off-by: Benoit Pierre 
---
 t/t7505-prepare-commit-msg-hook.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t7505-prepare-commit-msg-hook.sh 
b/t/t7505-prepare-commit-msg-hook.sh
index 1c95652..5531abb 100755
--- a/t/t7505-prepare-commit-msg-hook.sh
+++ b/t/t7505-prepare-commit-msg-hook.sh
@@ -154,7 +154,7 @@ test_expect_success 'with failing hook' '
head=`git rev-parse HEAD` &&
echo "more" >> file &&
git add file &&
-   ! GIT_EDITOR="\"\$FAKE_EDITOR\"" git commit -c $head
+   test_must_fail env GIT_EDITOR="\"\$FAKE_EDITOR\"" git commit -c $head
 
 '
 
@@ -163,7 +163,7 @@ test_expect_success 'with failing hook (--no-verify)' '
head=`git rev-parse HEAD` &&
echo "more" >> file &&
git add file &&
-   ! GIT_EDITOR="\"\$FAKE_EDITOR\"" git commit --no-verify -c $head
+   test_must_fail env GIT_EDITOR="\"\$FAKE_EDITOR\"" git commit 
--no-verify -c $head
 
 '
 
-- 
1.9.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 3/7] test patch hunk editing with "commit -p -m"

[Benoit Pierre]

Add (failing) tests: with commit changing the environment to let hooks
know that no editor will be used (by setting GIT_EDITOR to ":"), the
"edit hunk" functionality does not work (no editor is launched and the
whole hunk is committed).

Signed-off-by: Benoit Pierre 
---
 t/t7514-commit-patch.sh | 34 ++
 1 file changed, 34 insertions(+)
 create mode 100755 t/t7514-commit-patch.sh

diff --git a/t/t7514-commit-patch.sh b/t/t7514-commit-patch.sh
new file mode 100755
index 000..41dd37a
--- /dev/null
+++ b/t/t7514-commit-patch.sh
@@ -0,0 +1,34 @@
+#!/bin/sh
+
+test_description='hunk edit with "commit -p -m"'
+. ./test-lib.sh
+
+if ! test_have_prereq PERL
+then
+   skip_all="skipping '$test_description' tests, perl not available"
+   test_done
+fi
+
+test_expect_success 'setup (initial)' '
+   echo line1 >file &&
+   git add file &&
+   git commit -m commit1
+'
+
+test_expect_failure 'edit hunk "commit -p -m message"' '
+   test_when_finished "rm -f editor_was_started" &&
+   rm -f editor_was_started &&
+   echo more >>file &&
+   echo e | env GIT_EDITOR=": >editor_was_started" git commit -p -m 
commit2 file &&
+   test -r editor_was_started
+'
+
+test_expect_failure 'edit hunk "commit --dry-run -p -m message"' '
+   test_when_finished "rm -f editor_was_started" &&
+   rm -f editor_was_started &&
+   echo more >>file &&
+   echo e | env GIT_EDITOR=": >editor_was_started" git commit -p -m 
commit3 file &&
+   test -r editor_was_started
+'
+
+test_done
-- 
1.9.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 1/7] merge hook tests: fix missing '&&' in test

[Benoit Pierre]

Signed-off-by: Benoit Pierre 
---
 t/t7505-prepare-commit-msg-hook.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t7505-prepare-commit-msg-hook.sh 
b/t/t7505-prepare-commit-msg-hook.sh
index 3573751..1c95652 100755
--- a/t/t7505-prepare-commit-msg-hook.sh
+++ b/t/t7505-prepare-commit-msg-hook.sh
@@ -174,7 +174,7 @@ test_expect_success 'with failing hook (merge)' '
git add file &&
rm -f "$HOOK" &&
git commit -m other &&
-   write_script "$HOOK" <<-EOF
+   write_script "$HOOK" <<-EOF &&
exit 1
EOF
git checkout - &&
-- 
1.9.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 5/7] merge: fix GIT_EDITOR override for commit hook

[Benoit Pierre]

Don't set GIT_EDITOR to ":" when calling prepare-commit-msg hook if the
editor is going to be called (e.g. with "merge -e").

Signed-off-by: Benoit Pierre 
---
 builtin/merge.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index bdf6655..e15d0e1 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -824,7 +824,7 @@ static void prepare_to_commit(struct commit_list 
*remoteheads)
if (0 < option_edit)
strbuf_commented_addf(&msg, _(merge_editor_comment), 
comment_line_char);
write_merge_msg(&msg);
-   if (run_commit_hook(1, get_index_file(), "prepare-commit-msg",
+   if (run_commit_hook(0 < option_edit, get_index_file(), 
"prepare-commit-msg",
git_path("MERGE_MSG"), "merge", NULL))
abort_commit(remoteheads, NULL);
if (0 < option_edit) {
-- 
1.9.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 7/7] run-command: mark run_hook_with_custom_index as deprecated

[Benoit Pierre]

Signed-off-by: Benoit Pierre 
---
 run-command.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/run-command.h b/run-command.h
index 88460f9..3653bfa 100644
--- a/run-command.h
+++ b/run-command.h
@@ -51,6 +51,7 @@ extern int run_hook_le(const char *const *env, const char 
*name, ...);
 extern int run_hook_ve(const char *const *env, const char *name, va_list args);
 
 LAST_ARG_MUST_BE_NULL
+__attribute__((deprecated))
 extern int run_hook_with_custom_index(const char *index_file, const char 
*name, ...);
 
 #define RUN_COMMAND_NO_STDIN 1
-- 
1.9.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 4/7] commit: fix patch hunk editing with "commit -p -m"

[Benoit Pierre]

Don't change git environment: move the GIT_EDITOR=":" override to the
hook command subprocess, like it's already done for GIT_INDEX_FILE.

Signed-off-by: Benoit Pierre 
---
 builtin/checkout.c  |  8 
 builtin/clone.c |  4 ++--
 builtin/commit.c| 35 ---
 builtin/gc.c|  2 +-
 builtin/merge.c |  6 +++---
 commit.h|  3 +++
 run-command.c   | 44 
 run-command.h   |  6 +-
 t/t7514-commit-patch.sh |  4 ++--
 9 files changed, 80 insertions(+), 32 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index ada51fa..1b86d9c 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -53,10 +53,10 @@ struct checkout_opts {
 static int post_checkout_hook(struct commit *old, struct commit *new,
  int changed)
 {
-   return run_hook(NULL, "post-checkout",
-   sha1_to_hex(old ? old->object.sha1 : null_sha1),
-   sha1_to_hex(new ? new->object.sha1 : null_sha1),
-   changed ? "1" : "0", NULL);
+   return run_hook_le(NULL, "post-checkout",
+  sha1_to_hex(old ? old->object.sha1 : null_sha1),
+  sha1_to_hex(new ? new->object.sha1 : null_sha1),
+  changed ? "1" : "0", NULL);
/* "new" can be NULL when checking out from the index before
   a commit exists. */
 
diff --git a/builtin/clone.c b/builtin/clone.c
index 43e772c..9b3c04d 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -660,8 +660,8 @@ static int checkout(void)
commit_locked_index(lock_file))
die(_("unable to write new index file"));
 
-   err |= run_hook(NULL, "post-checkout", sha1_to_hex(null_sha1),
-   sha1_to_hex(sha1), "1", NULL);
+   err |= run_hook_le(NULL, "post-checkout", sha1_to_hex(null_sha1),
+  sha1_to_hex(sha1), "1", NULL);
 
if (!err && option_recursive)
err = run_command_v_opt(argv_submodule, RUN_GIT_CMD);
diff --git a/builtin/commit.c b/builtin/commit.c
index 3783bca..68a90b3 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -610,7 +610,7 @@ static int prepare_to_commit(const char *index_file, const 
char *prefix,
/* This checks and barfs if author is badly specified */
determine_author_info(author_ident);
 
-   if (!no_verify && run_hook(index_file, "pre-commit", NULL))
+   if (!no_verify && run_commit_hook(use_editor, index_file, "pre-commit", 
NULL))
return 0;
 
if (squash_message) {
@@ -867,8 +867,8 @@ static int prepare_to_commit(const char *index_file, const 
char *prefix,
return 0;
}
 
-   if (run_hook(index_file, "prepare-commit-msg",
-git_path(commit_editmsg), hook_arg1, hook_arg2, NULL))
+   if (run_commit_hook(use_editor, index_file, "prepare-commit-msg",
+   git_path(commit_editmsg), hook_arg1, hook_arg2, 
NULL))
return 0;
 
if (use_editor) {
@@ -884,7 +884,7 @@ static int prepare_to_commit(const char *index_file, const 
char *prefix,
}
 
if (!no_verify &&
-   run_hook(index_file, "commit-msg", git_path(commit_editmsg), NULL)) 
{
+   run_commit_hook(use_editor, index_file, "commit-msg", 
git_path(commit_editmsg), NULL)) {
return 0;
}
 
@@ -1068,8 +1068,6 @@ static int parse_and_validate_options(int argc, const 
char *argv[],
use_editor = 0;
if (0 <= edit_flag)
use_editor = edit_flag;
-   if (!use_editor)
-   setenv("GIT_EDITOR", ":", 1);
 
/* Sanity check options */
if (amend && !current_head)
@@ -1450,6 +1448,29 @@ static int run_rewrite_hook(const unsigned char *oldsha1,
return finish_command(&proc);
 }
 
+int run_commit_hook(int editor_is_used, const char *index_file, const char 
*name, ...)
+{
+   const char *hook_env[3] =  { NULL };
+   char index[PATH_MAX];
+   va_list args;
+   int ret;
+
+   snprintf(index, sizeof(index), "GIT_INDEX_FILE=%s", index_file);
+   hook_env[0] = index;
+
+   /*
+* Let the hook know that no editor will be launched.
+*/
+   if (!editor_is_used)
+   hook_env[1] = "GIT_EDITOR=:";
+
+   va_start(args, name);
+   ret = run_hook_ve(hook_env, name, args);
+   va_end(args);
+
+   return ret;
+}
+
 int cmd_commit(int argc, const char **argv, const char *prefix)
 {
static struct wt_status s;
@@ -1674,7 +1695,7 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
 "not exceeded, and then \"git reset HEAD\" to recover."));
 
rerere(0);
-   run_hook(get_index_file(), "post-commit", NULL);
+   run_commit_hook(use_editor, get_index_file(), "post-co

[PATCH] git-rebase: Teach rebase "-" shorthand.

[Brian Gesiak]

Teach rebase the same shorthand as checkout and merge; that is, that "-"
means "the branch we were previously on".

Reported-by: Tim Chase 
Signed-off-by: Brian Gesiak 
---
 git-rebase.sh | 4 
 t/t3400-rebase.sh | 6 ++
 2 files changed, 10 insertions(+)

diff --git a/git-rebase.sh b/git-rebase.sh
index 5f6732b..2c75e9f 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -453,6 +453,10 @@ then
test "$fork_point" = auto && fork_point=t
;;
*)  upstream_name="$1"
+   if test "$upstream_name" = "-"
+   then
+   upstream_name="@{-1}"
+   fi
shift
;;
esac
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index 6d94b1f..00aba9f 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -88,6 +88,12 @@ test_expect_success 'rebase from ambiguous branch name' '
git rebase master
 '
 
+test_expect_success 'rebase using shorthand' '
+   git checkout master
+   git checkout -b shorthand HEAD^
+   GIT_TRACE=1 git rebase -
+'
+
 test_expect_success 'rebase a single mode change' '
git checkout master &&
git branch -D topic &&
-- 
1.8.5.2 (Apple Git-48)

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

Bug report -- "Illegal instruction" on Mac 10.6.8 without XCode installed

[Ray Hengst]

Hi,
I am running a Mac 10.6.8 and tried to install git-1.9.0 off of the
installer (git-1.9.0-intel-universal-snow-leopard.dmg). The
installation worked fine and gave no error messages. But whenever I
type in a git command (see below for some I tried), it gives me this
error message:
Illegal instruction

I am completely new to git and mostly new of Unix, but here are some
commands I tried:
git
git help
git config
git init
git clean
git config --global user.name "John Doe"
git status

However, typing "man git" displays typical man pages.
I do not have Xcode installed. (it's very hard to find a legacy copy);
the "make" command also is not present, so I can't use any of the
workarounds I saw listed.
I uninstalled git-1.9.0 successfully using the provided script, then
downloaded the same file again (and installed it) to make sure I
didn't get a corrupt copy. I had the same problem, however.
If I can provide any more information just let me know.
Thanks for any help you can provide.
-Ray
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH] diff: simplify cpp funcname regex

[Johannes Sixt]

Cc René; do you have any comments regarding grep --function-context?

Am 3/18/2014 6:24, schrieb Jeff King:
> On Fri, Mar 14, 2014 at 07:56:46AM +0100, Johannes Sixt wrote:
> 
>> Consider this code:
>>
>>   void above()
>>   {}
>>   static int Y;
>>   static int A;
>>   int bar()
>>   {
>> return X;
>>   }
>>   void below()
>>   {}
> 
> Thanks, this example is very helpful.
> 
>> When you 'git grep --function-context X', then you get this output with
>> the current pattern, you proposal, and my proposal (file name etc omitted
>> for brevity):
>>
>>   int bar()
>>   {
>> return X;
>>   }
> 
> Right, that makes sense to me.
> 
>> When you 'git grep --function-context Y', what do you want to see? With
>> the current pattern, and with your pattern that forbids semicolon we get:
>>
>>   void above()
>>   {}
>>   static int Y;
>>   static int A;
>>
>> and with my simple pattern, which allows semicolon, we get merely
>>
>>   static int Y;
>>
>> because the line itself is a hunk header (and we do not look back any
>> further) and the next line is as well. That is not exactly "function
>> context", and that is what I'm a bit worried about.
> 
> Hmm. To be honest, I do not see yours as all that bad. Is "above()" or
> "A" actually interesting here? I'm not sure that they are. But then I do
> not use --function-context myself.
> 
> I guess it violates the "show things that are vaguely nearby, rather
> than a container" view of context that we discussed earlier. But somehow
> that seems less important to me with "--function-context".
> 
> So I dunno. I kind of like your version.

Then I'll polish my patch series (it also rewrites the test framework) and
post it.

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

Re: [PATCH 4/4] gc --aggressive: three phase repacking

[David Kastrup]

Duy Nguyen  writes:

> On Tue, Mar 18, 2014 at 12:16 PM, Duy Nguyen  wrote:
>> But I think it's orthogonal to gc --aggressive improvement.
>
> There's another reason that improving gc may be a good idea (or not).
> It depends on how other git implementations handle long delta chains.

"Other git implementations" including current installations that have a
half-life of at last a year.

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