Re: [PATCH v2] gitk: Fix missing commits when using -S or -G

[Stefan Dotterweich]

Nice catch; however, we should only update numcommits if the commits
are for the current view, i.e. if $v == $curview.

Do you want to update the patch?  If you prefer, I can update the
patch and put a note in the commit message about the issue.


Sure, feel free to update the patch as you see 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: t6044 broken on pu

[Torsten Bögershausen]

On 08.05.16 20:20, Junio C Hamano wrote:
> Torsten Bögershausen  writes:
>
>> May a  simple
>>  printf "1\n2\n3\n4\n5\n6\n7\n8\n9\n10\n"
>>
>> be an option ?
> If you were to do that, at least have the decency to make it more
> readable by doing something like:
>
>   printf "%s\n" 1 2 3 4 5 6 7 8 9 10
>
> ;-)
>
> But as I said, as a response to "t6044 broken on pu" bug report,
> s/seq/test_seq/ is the only sensible change.
>
> Improving "test_seq, the alternative to seq" is a separate topic.
>
> If you have aversion to $PERL, perhaps do them without using
> anything what is not expected to be built-in in modern shells,
> perhaps like this?
Please don't get me wrong -
I wasn't really clear why:
I think that the invocation of an external program
to produce a 10 line test program is a waste of CPU cycles  - in this very use 
case.

We can try to use the ideal tool to do the job, in this case
the fork() that needs to be done to invoke perl seems rather expensive in 
relation
to what we get in terms of functionality - a file with 10 lines of content.

I recently read a message why "grep | sed" is not ideal, when sed can do 
everything
that grep needs to do, and only 1 external program needs to be invoked - not 2.

I try to apply the same pattern to this TC: Stay in the shell, as long as 
possible.
But if you really need perl for e.g. regexp, then use it.

--
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] gitk: Fix missing commits when using -S or -G

[Paul Mackerras]

On Fri, May 06, 2016 at 02:16:54PM +0200, Stefan Dotterweich wrote:
> When -S or -G is used as a filter option, the resulting commit list
> rarely contains all matching commits. Only a certain number of commits
> are displayed and the rest are missing.
> 
> "git log --boundary -S" does not return as many boundary commits as you
> might expect. gitk makes up for this in closevargs() by adding missing
> parent (boundary) commits. However, it does not change $numcommits,
> which limits how many commits are shown. In the end, some commits at the
> end of the commit list are simply not shown.
> 
> Change $numcommits whenever a missing parent is added.

Nice catch; however, we should only update numcommits if the commits
are for the current view, i.e. if $v == $curview.

Do you want to update the patch?  If you prefer, I can update the
patch and put a note in the commit message about the issue.

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

Re: Re: [RFD/PATCH] submodule doc: describe where we can configure them

[Heiko Voigt]

Hi,

On Wed, May 04, 2016 at 02:13:47PM -0700, Junio C Hamano wrote:
> Jonathan Nieder  writes:
> 
> > This design is somewhat problematic for a few reasons:
> >
> > - When I want to stop paying attention to a particular submodule and
> >   start paying attention to it again later, all my local settings are
> >   gone.
> 
> True; "[submodule "foo"] enabled = no" may also be a way to fix it
> without throwing the whole with bathwater, though.

IMO we already have this. With

git config submodule..update none
rm -rf 
mkdir 

We remove a submodule from the working copy and disable any updates with
'git submodule update ...'. Maybe we should add this 'light' operation
as an option to 'git submodule deinit' in the long run?

> > - When upstream adds a new submodule, I have to do the same manual
> >   work to change the options for that new submodule.
> 
> Because a new module is not automatically "init"ed by default?
> 
> Isn't "config only" vs "config with gitmodules fallback" orthogonal
> to that issue?

What do you mean with "orthogonal to that issue"? AFAICS a gitmodule
fallback does not have that issue. Actually I would see it more like:
.gitmodule is the default and .git/config a possibility to override.
When viewing it like this and using .gitmodule directly is the default
a user does not have any issues when upstream changes submodule
configurations.

Or are we talking about subsequent forks from upstreams? Like C forked
from B and B from A... Then forget what I said.

> > - When upstream changes submodule options (perhaps to fix a URL
> >   typo), I don't get those updates.
> 
> True.

I would say it depends on what is your default view. See above.

> > A fix is to use settings from .git/config when present and fall back
> > to .gitmodules when not.  
> 
> How would that fix the "upstream updated"?

When .gitmodules is the default source "upstream updated" is
automatically read.

> I think an alternative suggested in an ancient time had a more
> elaborate scheme:
> 
>  * Use .git/config as the authoritative source, but record
>sufficient information to detect the case and cope with it when
>entry in .gitmodules changes (details below).
> 
>  * When seeing a new .gitmodules entry, either by "git pull" or even
>"git checkout other-branch", copy that to .git/config (just like
>what "git submodule init" does).  It would be a policy decision
>to automatically enabling it or not.  If the policy is "no
>autoinit", then "module..inited = no" may also have to be
>added to .git/config at this point.
> 
>Record contents of the entry in .gitmodules to the corresponding
>.git/config entry as "seen".
> 
>  * When the entry in .gitmodules for a submodule known to
>.git/config is different from what has been "seen", offer the
>user a chance to update corresponding .git/config entry, and
>append to the "seen" set of variants in .gitmodules so that the
>user will not be bugged with "we see .gitmodules entry for module
> is different from anything you have ever seen; do you want
>to make corresponding changes to the module entry in your
>.git/config" again.
> 
> which would handle all of the above, and without using anything from
> .gitmodules before the user has a chance to vet it.

I can see that for some users it might be important not to pull every
submodule that upstream decides they should have. On the other hand: Is
it really a decision a user can/should make during a pull or a checkout.
I would be annoyed by it, since it interrupts me from the thing I really
want to do and would mostly just choose some default (like always yes or
always no) depending on what is important to me (e.g. faster checkout or
complete repository). So IMO it is more sensible if we just give the
user some default to configure and then use that instead of asking
questions in a situation where the user is not ready to answer them.

And when the user has his defaults we can actually try to deduct such
decisions directly from .gitmodules and do not need to store anything in
.git/config as long as the user goes with the defaults.

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

Re: Re: [RFD/PATCH] submodule doc: describe where we can configure them

[Heiko Voigt]

Hi,

On Wed, May 04, 2016 at 01:50:24PM -0700, Stefan Beller wrote:
> On Wed, May 4, 2016 at 8:01 AM, Heiko Voigt  wrote:
> > On Tue, May 03, 2016 at 05:59:58PM -0700, Stefan Beller wrote:
> >> On Tue, May 3, 2016 at 4:56 PM, Jonathan Nieder  wrote:
> >> > Stefan Beller wrote:
> >> >
> >> >> This is similar to the gitignore document, but doesn't mirror
> >> >> the current situation. It is rather meant to start a discussion for
> >> >> the right approach for mirroring repositories with submodules.
> >> >
> >> > Ooh.
> >>
> >> Thanks for writing such a detailed answer. :)
> >
> > BTW, here is a pointer to the discussion (and what I wrote down) about
> > this from back in 2014:
> >
> > https://github.com/jlehmann/git-submod-enhancements/wiki/Ideas#special-ref-overriding-gitmodules-values
> 
> Thanks for pointing at the prior discussion!
> Although not much happened since then (code wise)?

Yes, IIRC nothing happened code wise. It went so far that a rough
consensus was made but nobody actually stepped in to scratch that itch.

> > Jonathan you suggested to copy the content from a remote to
> > .git/info//gitmodules locally. How would one get it to the
> > remote side? It seems to me as if we would need to implement additional
> > infrastructure to do this. Would it not be simpler if we just kept it on
> > a ref on the local side as well? We already have the infrastructure to
> > read those values from a ref. We only would need to add something to
> > write them. Then a simple push, which could be aliased in form of a
> > git-submodule subcommand, suffices to get the values to the remote.
> 
> That is good idea!

Thanks.

> > That also solves issues when people clone from their working copy.
> >
> > I would like to think a little bit further about the conflict situation
> > when two remotes are providing values. Configuring this looks to me like
> > a nightmare for users. Maybe there is some sort of elegant solution?
> > E.g. like we use the values from remote A during a fetch from A, the
> > ones from B during a fetch from B and no values from a special ref in
> > case there is no remote operation involved. Since the main goal is to
> > support forking of submodules isn't there always a remote operation
> > involved?
> 
> Here is what I imagine
> When B mirrors from A, B sets up this special ref for its repository,
> e.g. refs/meta/submodule-B and have a symbolic ref pointing at that.
> (e.g. SUBMODULE_CONFIG pointing at refs/meta/submodule-B,
> which has a worktree which contains a .gitmodules files which
> sets up
>   "submodule.baz.url = http://B/baz;
>   "submodule.relativeBase = http://A;
> 
> That way anyone cloning from B would get
> the superproject and the submodule baz from B while the
> rest of the submodules are found at A.

This sounds sensible. But my imagination of a conflict was in a
different way. E.g. project A has a submodule B. And now A has a remote
1 where you publish and maybe another remote 2 where someone else (a
colleague?) publishes. Which configuration do you use? Here the two
remotes are independent instead of subsequent forks. In this case my
solution would be to use the configuration branch from 1 for B when
interacting with 1. I do not have completely checked whether we always
have a remote at hand for such a resolution.

> When C mirrors from A, they add another branch  refs/meta/submodule-C,
> which can either be a fork of refs/meta/submodule-B with some changes on
> top of it or it can add a reference to refs/meta/submodule-B, i.e. the
> configuration
> would be:
> 
>   "submodule.baseConfig = refs/meta/submodule-B"
>   "submodule.foo.url = ssh://C/foo"
> 
> and SUBMODULE_CONFIG would point at refs/meta/submodule-C.
> 
> When cloning from C, the user would get
> 
>  * the superproject from C
>  * submodule foo from C
>  * submodule baz from B
>  * all other submodules from A
> 
> By the inheriting property of the branch of B there are no conflicting values.
> C could just overwrite submodule.baseConfig for example.

So that means in the default case we create a chain of all previous
forks embedded in repository database. I am not saying that this is
necessarily a bad thing but I feel that it is a new property which we
should think about. It helps because users will get updated values from
sources that are in the chain. On the other hand it adds a lot of
dependencies which are point of failures in case a remote disappears. I
am undecided on this. I would prefer if we could let people play with it
a little (maybe on pu?) and then decide if there are practical pitfalls
with this.

> > My suggested scheme above does not solve the currently quite typical use
> > case where you might 'git fetch' without submodules first and then do
> > the submodule fetches during a 'git submodule update'. On the other hand
> > in the 'ideal future world' where submodules behave like "normal files" the
> > fetch will be done during the superproject fetch so 

Re: [PATCH 78/83] Move libified code from builtin/apply.c to apply.{c,h}

[Christian Couder]

On Fri, May 6, 2016 at 11:07 PM, Junio C Hamano  wrote:
> Christian Couder  writes:
>
>> By the way does someone know how such a patch can be applied?
>
> I think in mid Feb 2015 I sent out a message that says "'diff -B -M'
> gives a broken result, do not use it".
> I do not have time to dig the mail archive right now.
>
> Perhaps this one?
>
> http://thread.gmane.org/gmane.linux.kernel/1879635

I get something strange with only -B:

$ git reset --keep 20f1d27~
$ git format-patch -B -1  -o ../../patches/test-libify-apply-use-in-am/ 20f1d27
../../patches/test-libify-apply-use-in-am/0001-Move-libified-code-from-builtin-apply.c-to-apply.-c-.patch
$ wc 
../../patches/test-libify-apply-use-in-am/0001-Move-libified-code-from-builtin-apply.c-to-apply.-c-.patch
  5264  23426 147127
../../patches/test-libify-apply-use-in-am/0001-Move-libified-code-from-builtin-apply.c-to-apply.-c-.patch

I get a conflict with `git am`:

$ git am --3way
../../patches/test-libify-apply-use-in-am/0001-Move-libified-code-from-builtin-apply.c-to-apply.-c-.patch
Applying: Move libified code from builtin/apply.c to apply.{c,h}
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging apply.c
CONFLICT (add/add): Merge conflict in apply.c
error: Failed to merge in the changes.
Patch failed at 0001 Move libified code from builtin/apply.c to apply.{c,h}
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

But it works using `git apply --3way`:

$ git reset --hard HEAD
HEAD is now at 557f659 apply: rename and move opt constants to apply.h
$ git apply --3way
../../patches/test-libify-apply-use-in-am/0001-Move-libified-code-from-builtin-apply.c-to-apply.-c-.patch
Falling back to three-way merge...
Applied patch to 'apply.c' cleanly.
$ git am --continue
Applying: Move libified code from builtin/apply.c to apply.{c,h}
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] i18n: remote: add comment for translators

[Vasco Almeida]

Add comment drawing translator attention in order to align "Push
URL:" and "Fetch URL:" fields translation of git remote show output.

Aligning both fields makes the output more appealing and easier to
grasp.

Signed-off-by: Vasco Almeida 
---

Translators, you can check if your translation does align these
fields by issuing, for instance

$ git remote show -n origin
* remote origin
  Fetch URL: https://github.com/git/git.git
  Push  URL: https://github.com/git/git.git


I know Portuguese translation align these fields. I know that French
and Deutsch translations don't, but don't know about others.

Obviously, this detail is not important, but now you know about it.

 builtin/remote.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/builtin/remote.c b/builtin/remote.c
index fda5c2e..d33766b 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -1154,6 +1154,8 @@ static int show(int argc, const char **argv)
url_nr = states.remote->url_nr;
}
for (i = 0; i < url_nr; i++)
+   /* TRANSLATORS: the colon ':' should align with
+  the one in "  Fetch URL: %s" translation */
printf_ln(_("  Push  URL: %s"), url[i]);
if (!i)
printf_ln(_("  Push  URL: %s"), "(no URL)");
-- 
2.7.3

--
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 v16 0/7] config commit verbose

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  writes:

> I.e. I really expect --verbose to be a more verbose version of the
> primary thing a command is doing, which in the case of "commit
> --amend" is giving me info I need to modify the commit.

That summarises what I wanted to say very well.  Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 1/2] rev-parse tests: add tests executed from a subdirectory

[Junio C Hamano]

Junio C Hamano  writes:

> What I had in mind was more a long the lines of this change. I only
> did the first several just for illustration (I added an 'exit' to
> mark where I stopped), but I think you get the idea.  The point is
> that by doing things in subprocess inside test_expect_success you
> can avoid disrupting the main test process even when a test fails as
> much as possible, and this illustrates how you would deal with the
> "cd" and "export".  What I didn't handle was the updates to
> .git/config whose effects by earlier tests are relied on later tests
> in this illustration.
>
> As to "table driven" vs "explicitly spelling out scripts, accepting
> some code repetition", I tend to favor the latter slightly, unless
> the table driven approach is done in such a way that each of its
> tests are truly independent from one another, i.e. if a row of the
> table represents one test, the tests would not be disrupted by
> reordering the rows, inserting a new row in the middle, or removing
> an existing row.  From my experience, "table-driven" sets of tests
> whose rows have inter-dependency has been much harder to debug when
> I had to hunt down a bug that manifests itself only in one test in
> the middle.
>
> Hope this helps clarify what I meant.


If you want to go forward in this direction, three are a few
additional things to note.

> + test_expect_success "$name: is-bare-repository" "
> + (
> + $prep &&
> + test '$1' = \"\$(git rev-parse --is-bare-repository)\"
> + )"

This is not a new problem, but quoting the test body with dq instead
of sq pair make it ugly to read.  We might want to do something like

expect=$1
test_expect_success "$name: is-bare-repository" '
(
$prep &&
test "$outcome" = "$(git rev-parse --is-bare-repository)"
)'

>   shift
>   [ $# -eq 0 ] && return

Also, let's avoid []; i.e.

test $# = 0 && return

> -# label is-bare is-inside-git is-inside-work prefix git-dir
> +# label prep is-bare is-inside-git is-inside-work prefix git-dir
>  
>  ROOT=$(pwd)
> +mkdir -p sub/dir work

Remember this place in the script, I may refer to it later.

> -git config core.bare true
> -test_rev_parse 'core.bare = true' true false false
> +test_rev_parse subdirectory 'cd sub/dir' \
> + false false true sub/dir/ "$ROOT/.git"
>  
> -git config --unset core.bare
> -test_rev_parse 'core.bare undefined' false false true
> +test_rev_parse 'core.bare = true' 'git config core.bare true' \
> + true false false

I didn't use "test_config" and "test_unconfig" here, because you
cannot use them in a subprocess.  If we truly want to make these
tests independent from each other, we'd need to come up with a way
for the tests to revert the configuration state back to where it was
before they started.
>  
> -mkdir work || exit 1
> -cd work || exit 1
> -GIT_DIR=../.git
> -GIT_CONFIG="$(pwd)"/../.git/config
> -export GIT_DIR GIT_CONFIG
>  
> -git config core.bare false
> -test_rev_parse 'GIT_DIR=../.git, core.bare = false' false false true ''
> +test_rev_parse 'GIT_DIR=../.git, core.bare = false' '
> + cd work && 
> + GIT_DIR=../.git &&
> + GIT_CONFIG="$(pwd)"/../.git/config &&
> + export GIT_DIR GIT_CONFIG &&
> + git config core.bare false' \
> + false false true ''

The "prep" step for this test is long and I'd assume that the later
tests would need to do something similar themselves.  To avoid
repetition, create a helper shell function to reduce boilerplate
before the series of these tests start (i.e. the place above) and
call it from the prep parts with different arguments, e.g. making
the above test into

test_rev_parse 'GIT_DIR=../.git, core.bare = false' '
cd work && 
set_common_env ../.git false' \
false false true ''

by having something like

set_dir_and_core_bare () {
GIT_DIR=$1 &&
GIT_CONFIG="$(pwd)/$GIT_DIR/config" &&
export GIT_DIR GIT_CONFIG &&
git config core.bare "$2"
}

> +exit
>  
>  git config core.bare true
>  test_rev_parse 'GIT_DIR=../.git, core.bare = true' true false false ''
--
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: Config for git-blame flags -w, -M, and -C?

[Joe Lencioni]

> git config alias.bl "blame -w -M -C"

Indeed, that would do the trick when interacting with git directly.
However, I am interested in something that would provide a consistent
git blame across all tools that integrate with git, such as GUI tools
and editor integrations like Fugitive.
--
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: Force-with-lease and new branches

[Junio C Hamano]

John Keeping  writes:

> It looks like this has been the case since the first version of what
> would become --force-with-lease [1] and I can't find any discussion
> around this particular behaviour in the three versions of that patch set
> I found on Gmane [2], [3], [4].

I never considered the "creation of a new ref, ensuring that it must
not exist yet" use case when designing it.  I do not think anybody
in the discussion did so, either.

> So my questions are: what will break if we decide to treat "no remote
> tracking branch" as "new branch" and is that a reasonable thing to do?

If you are only following a subset of branches they have, you may
never get copies of them in your refs/remotes/$there/ hierarchy, so
your addition would make 'force-with-lease to create' mistakenly
think that a branch does not exist over there, when there is already
one, and it will attempt to push your version through.  As long as
that is caught and fail on the receiving end of the request, it is
OK, I would think.  I didn't think it through nor checked the code
to make sure the remote end behaves sensibly--please do so yourself
if you want to pursue this route ;-).

Thanks.

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

Re: Config for git-blame flags -w, -M, and -C?

[Junio C Hamano]

Joe Lencioni  writes:

> Thoughts?

For end-user facing commands that are expected to be used by human
typing on the keyboard, I'd recommend

git config alias.bl "blame -w -M -C"

instead.
--
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: t6044 broken on pu

[Junio C Hamano]

Torsten Bögershausen  writes:

> May a  simple
>  printf "1\n2\n3\n4\n5\n6\n7\n8\n9\n10\n"
>
> be an option ?

If you were to do that, at least have the decency to make it more
readable by doing something like:

printf "%s\n" 1 2 3 4 5 6 7 8 9 10

;-)

But as I said, as a response to "t6044 broken on pu" bug report,
s/seq/test_seq/ is the only sensible change.

Improving "test_seq, the alternative to seq" is a separate topic.

If you have aversion to $PERL, perhaps do them without using
anything what is not expected to be built-in in modern shells,
perhaps like this?

 t/test-lib-functions.sh | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 8d99eb3..4edddac 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -739,7 +739,12 @@ test_seq () {
2)  ;;
*)  error "bug in the test script: not 1 or 2 parameters to 
test_seq" ;;
esac
-   perl -le 'print for $ARGV[0]..$ARGV[1]' -- "$@"
+   test_seq_counter__=$1
+   while test "$test_seq_counter__" -le $2
+   do
+   echo "$test_seq_counter__"
+   test_seq_counter__=$((test_seq_counter__ + 1))
+   done
 }
 
 # This function can be used to schedule some commands to be run

--
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 1/2] rev-parse tests: add tests executed from a subdirectory

[Junio C Hamano]

Mike Rappazzo  writes:

>> Instead of cleaning things up like this, could you please please
>> please fix these existing tests that chdir around without being in a
>> subshell?  If the "previous tests" failed before going down as this
>> step expects, the "cd .. && rm -r" can make things worse.

> I originally copied the pattern from above this code:
> ...
> but Gábor had an objection to it [2].  So I went with this simple cleanup 
> test.

The "|| exit 1" you see everywhere is a sure sign that these tests
are not designed correctly.  These existing tests that do too many
things outside test_expect_success block needs to be redone, so that
each of them can do what it wants to test even when previous ones
failed.

What I had in mind was more a long the lines of this change. I only
did the first several just for illustration (I added an 'exit' to
mark where I stopped), but I think you get the idea.  The point is
that by doing things in subprocess inside test_expect_success you
can avoid disrupting the main test process even when a test fails as
much as possible, and this illustrates how you would deal with the
"cd" and "export".  What I didn't handle was the updates to
.git/config whose effects by earlier tests are relied on later tests
in this illustration.

As to "table driven" vs "explicitly spelling out scripts, accepting
some code repetition", I tend to favor the latter slightly, unless
the table driven approach is done in such a way that each of its
tests are truly independent from one another, i.e. if a row of the
table represents one test, the tests would not be disrupted by
reordering the rows, inserting a new row in the middle, or removing
an existing row.  From my experience, "table-driven" sets of tests
whose rows have inter-dependency has been much harder to debug when
I had to hunt down a bug that manifests itself only in one test in
the middle.

Hope this helps clarify what I meant.

 t/t1500-rev-parse.sh | 82 ++--
 1 file changed, 48 insertions(+), 34 deletions(-)

diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh
index 48ee077..22b52c0 100755
--- a/t/t1500-rev-parse.sh
+++ b/t/t1500-rev-parse.sh
@@ -5,65 +5,79 @@ test_description='test git rev-parse'
 
 test_rev_parse() {
name=$1
-   shift
-
-   test_expect_success "$name: is-bare-repository" \
-   "test '$1' = \"\$(git rev-parse --is-bare-repository)\""
+   prep=$2
+   shift 2
+
+   test_expect_success "$name: is-bare-repository" "
+   (
+   $prep &&
+   test '$1' = \"\$(git rev-parse --is-bare-repository)\"
+   )"
shift
[ $# -eq 0 ] && return
 
-   test_expect_success "$name: is-inside-git-dir" \
-   "test '$1' = \"\$(git rev-parse --is-inside-git-dir)\""
+   test_expect_success "$name: is-inside-git-dir" "
+   (
+   $prep &&
+   test '$1' = \"\$(git rev-parse --is-inside-git-dir)\"
+   )"
shift
[ $# -eq 0 ] && return
 
-   test_expect_success "$name: is-inside-work-tree" \
-   "test '$1' = \"\$(git rev-parse --is-inside-work-tree)\""
+   test_expect_success "$name: is-inside-work-tree" "
+   (
+   $prep &&
+   test '$1' = \"\$(git rev-parse --is-inside-work-tree)\"
+   )"
shift
[ $# -eq 0 ] && return
 
-   test_expect_success "$name: prefix" \
-   "test '$1' = \"\$(git rev-parse --show-prefix)\""
+   test_expect_success "$name: prefix" "
+   (
+   $prep &&
+   test '$1' = \"\$(git rev-parse --show-prefix)\"
+   )"
shift
[ $# -eq 0 ] && return
 
-   test_expect_success "$name: git-dir" \
-   "test '$1' = \"\$(git rev-parse --git-dir)\""
+   test_expect_success "$name: git-dir" "
+   (
+   $prep &&
+   test '$1' = \"\$(git rev-parse --git-dir)\"
+   )"
shift
[ $# -eq 0 ] && return
 }
 
-# label is-bare is-inside-git is-inside-work prefix git-dir
+# label prep is-bare is-inside-git is-inside-work prefix git-dir
 
 ROOT=$(pwd)
+mkdir -p sub/dir work
 
-test_rev_parse toplevel false false true '' .git
+test_rev_parse toplevel : false false true '' .git
 
-cd .git || exit 1
-test_rev_parse .git/ false true false '' .
-cd objects || exit 1
-test_rev_parse .git/objects/ false true false '' "$ROOT/.git"
-cd ../.. || exit 1
+test_rev_parse .git 'cd .git' false true false '' .
 
-mkdir -p sub/dir || exit 1
-cd sub/dir || exit 1
-test_rev_parse subdirectory false false true sub/dir/ "$ROOT/.git"
-cd ../.. || exit 1
+test_rev_parse .git/objects/ 'cd .git/objects' false true false '' "$ROOT/.git"
 
-git config core.bare true
-test_rev_parse 'core.bare = true' true false false
+test_rev_parse subdirectory 'cd sub/dir' \
+   false false true sub/dir/ "$ROOT/.git"
 
-git config --unset core.bare
-test_rev_parse 'core.bare undefined' false false true

Re: [PATCH v5 1/2] bisect--helper: use OPT_CMDMODE instead of OPT_BOOL

[Pranit Bauva]

On Sun, May 8, 2016 at 9:00 PM, Christian Couder
 wrote:
> On Sun, May 8, 2016 at 9:17 AM, Pranit Bauva  wrote:
>> On Sun, May 8, 2016 at 12:34 PM, Johannes Schindelin
>>  wrote:
>>> Hi Pranit,
>>>
>>> On Fri, 6 May 2016, Pranit Bauva wrote:
>>>
 diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
 index 3324229..d8de651 100644
 --- a/builtin/bisect--helper.c
 +++ b/builtin/bisect--helper.c
 @@ -8,13 +8,17 @@ static const char * const git_bisect_helper_usage[] = {
   NULL
  };

 +enum subcommand {
 + NEXT_ALL = 1
 +};
>>>
>>> I still do not think that this enum needs to have file scope. Function
>>> scope is enough.
>>
>> In the very initial patch I made it in function scope. To which you
>> pointed out[1] that in all other examples but for one have file scope
>> so then I thought maybe that exception was a wrong example and I
>> should stick to the convention of putting it in file scope.
>
> In the message Dscho wrote:
>
> "Interesting. I did not think that Git's source code declares enums inside
> functions, but builtin/remote.c's config_read_branches() does, so this
> code is fine." So you didn't need to put it back in file scope.
>
> Please don't change things when you are told they are fine unless
> there is a good reason like a bug and in this case explain the reason.

I will keep this in mind.

>> But now I
>> also realize that builtin/replace.c uses "cmdmode" instead of
>> "subcommand" so I am still wondering what would be the most
>> appropriate?
>>
  int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
  {
 - int next_all = 0;
 + int subcommand = 0;
>>>
>>> Since subcommand is not simply an integer, but wants to take on the values
>>> defined in the enum above, the type should be changed accordingly. You
>>> could do it this way (short and sweet, with the appropriate scope):
>>>
>>> enum { NEXT_ALL = 1 } subcommand = 0;
>>>
>>> See https://github.com/git/git/blob/v2.8.2/builtin/replace.c#L423-L430 for
>>> an example (which uses "cmdmode" instead of "subcommand", too).
>
> Yeah, please use "cmdmode" as Junio already suggested and do it like
> in the example that Dscho points to.

Yes sure!

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

Re: [PATCH v5 1/2] bisect--helper: use OPT_CMDMODE instead of OPT_BOOL

[Christian Couder]

On Sun, May 8, 2016 at 9:17 AM, Pranit Bauva  wrote:
> On Sun, May 8, 2016 at 12:34 PM, Johannes Schindelin
>  wrote:
>> Hi Pranit,
>>
>> On Fri, 6 May 2016, Pranit Bauva wrote:
>>
>>> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
>>> index 3324229..d8de651 100644
>>> --- a/builtin/bisect--helper.c
>>> +++ b/builtin/bisect--helper.c
>>> @@ -8,13 +8,17 @@ static const char * const git_bisect_helper_usage[] = {
>>>   NULL
>>>  };
>>>
>>> +enum subcommand {
>>> + NEXT_ALL = 1
>>> +};
>>
>> I still do not think that this enum needs to have file scope. Function
>> scope is enough.
>
> In the very initial patch I made it in function scope. To which you
> pointed out[1] that in all other examples but for one have file scope
> so then I thought maybe that exception was a wrong example and I
> should stick to the convention of putting it in file scope.

In the message Dscho wrote:

"Interesting. I did not think that Git's source code declares enums inside
functions, but builtin/remote.c's config_read_branches() does, so this
code is fine." So you didn't need to put it back in file scope.

Please don't change things when you are told they are fine unless
there is a good reason like a bug and in this case explain the reason.

> But now I
> also realize that builtin/replace.c uses "cmdmode" instead of
> "subcommand" so I am still wondering what would be the most
> appropriate?
>
>>>  int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
>>>  {
>>> - int next_all = 0;
>>> + int subcommand = 0;
>>
>> Since subcommand is not simply an integer, but wants to take on the values
>> defined in the enum above, the type should be changed accordingly. You
>> could do it this way (short and sweet, with the appropriate scope):
>>
>> enum { NEXT_ALL = 1 } subcommand = 0;
>>
>> See https://github.com/git/git/blob/v2.8.2/builtin/replace.c#L423-L430 for
>> an example (which uses "cmdmode" instead of "subcommand", too).

Yeah, please use "cmdmode" as Junio already suggested and do it like
in the example that Dscho points to.

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

Re: [PATCH v5 0/2] bisect--helper: rewrite of check-term-format()

[Pranit Bauva]

On Sun, May 8, 2016 at 11:53 AM, Pranit Bauva  wrote:
> On Sun, May 8, 2016 at 7:55 AM, Junio C Hamano  wrote:
>> Pranit Bauva  writes:
>>
>>> I completely missed your point and you want me to go the Eric Sunshine's 
>>> way?
>>
>> I am neutral.
>>
>> When I read your response to Eric's "top down" suggestion, I didn't
>> quite get much more than "I started going this way, and I do not
>> want to change to the other direction.", which was what I had the
>> most trouble with.  If your justification for the approach to start
>> from building a tiny bottom layer that will need to be dismantled
>> soon and repeat that (which sounds somewhat wasteful) were more
>> convincing, I may have felt differently.
>
> Sorry if it seemed that "I have done quite some work and I don't want
> to scrape it off and redo everything". This isn't a case for me. I
> think of this as just a small part in the process of learning and my
> efforts would be completely wasted as I can still reuse the methods I

efforts would **not** be completely wasted

> wrote. This is still open for a "philosophical" discussion. I am
> assuming 1e1ea69fa4e is how Eric is suggesting.
>
> Why I think its better to go the subcommand way:
>  * I can test **C implementation** of the function to verify whether
> it was done in a proper way. By using a "top down" approach, I can
> make the test suite passing by running the code from git-bisect.sh not
> the re-written C code.
>  * I have made a very few minor mistakes in conversion of
> check_term_format() which just messed up my head (I actually spent 3
> days before Christian pointed out that '!' was missing). Imagine
> debugging the complete git-bisect.sh to find a very small error after
> a complete implementation.
>  * Using subcommands, I can easily verify whether I have done it the
> right way or not.
>  * It makes the review process very simple. The reviewers/testers can
> verify that my method is working correctly and well computers can
> detect errors better than humans.
>  * I can convert small functions which can be reviewed easily rather
> than dumping a big series.
>
> What I think is that the bottom up approach will make life easier for
> the me and for reviewer. Yes, it does make the code "unclean" for the
> time being and if it is between releases then all the more painful.
> Well, the latter part can be avoided by keeping it in next.
>
> Please point out if I am mistaken about anything.

There is also a third way in which I can go which is kind of merge of
both the ways:

 * Make two branches, one will contain the top down approach and
another will contain the subcommand approach.

 * I will write convert a function as a subcommand and test it locally
as well as on Travis-Cl by a PR on my git.git repo. The
reviewers/testers can consult the Travis-CI for output of the tests.
This will also reduce the need of locally compiling/testing.

 * I will then just write the function as it is in my "top down"
branch and thus send the patch to the mailing list. These can be kept
in next until it finishes.

 * Then will remove linking of git-bisect.sh and thus finalize.

This will be a bit more intensive not in a coding way but in
maintaining the repo. So I will have more experience of maintaining a
git repo. :)

Regards,
Pranit Bauva
--
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

Draft of Git Rev News edition 15

[Christian Couder]

Hi,

A draft of a new Git Rev News edition is available here:

  https://github.com/git/git.github.io/blob/master/rev_news/drafts/edition-15.md

Everyone is welcome to contribute in any section either by editing the
above page on GitHub and sending a pull request, or by commenting on
this GitHub issue:

  https://github.com/git/git.github.io/issues/152

You can also reply to this email.

I tried to cc everyone who appears in this edition but maybe I missed
some people, sorry about that.

Thomas, Nicola and myself plan to publish this edition on Wednesday
May 11.

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

[GSOC update] Week 1

[Pranit Bauva]

My public git.git is available here[1]. I regularly keep pushing my work so
anyone interested can track me there. Feel free to participate in the
discussions going on PRs with my mentors. Your comments are valuable.

The things I was able to do:

 * Finish off with my micro project[2] about adding a config variable for
   git-commit. There is still a little discussion going on about whether
   verbose should be retired and instead "commit.showDiff" should be used.
   Junio also made a little change to squash it with my patch to make the
   patch look more clearer to digest. Its difficult that this will be included
   in the release of 2.8.3

 * I converted the function check_term_format()[3] from shell to C as a
   subcommand. Faced some difficulty but was greatly helped by Christian
   Couder.

 * I converted the function bisect_log()[4] in a branch from the previous
   check-term-format branch as a subcommand. I was waiting for the first patch
   to get queued on maintainer's git.git repo so as to rebase it there and
   then send the patch.

 * I converted the function write_terms()[5] in a branch from the previous
   bisect-log branch as a subcommand and I removed check-term-format subcommand.
   I was waiting for the bisect-log patch to get queued on maintainer's git.git
   repo so as to rebase it there and send send the patch

 * I am currently introducing subcommands and calling them from git-bisect.sh .
   Eric Sunshine had proposed another top bottom approach wherein I first
   implement a skeleton and redirect it to git-bisect.sh so that the test
   suite is still passing. This method was used by Paul Tan in his last year
   GSoC project. Commit ids for reference, e1ea69fa4e and 73c2779f4 .

 * I ignored it and sent another revision of the series to which Junio wasn't
   quite enthusiastic over my reasoning for using subcommand approach. So
   I wrote an email[6] explaining how using subcommand way is beneficial to
   me, reviewers/testers, and my mentors.

 * I am currently waiting for that discussion to come to a conclusion. Till
   then I have paused on a bit.

 * I am now reading on the API part and planning how will I go about with the
   future functions. The functions I have converted till now were quite simple
   ones and the main difficulty lies ahead!

 * Anywhich ways I think this week was quite productive and if I follow this
   speed then the project is in a good shape to be completed well in time.

 * On a lighter note: I will be on a short vacation from 16th May to 19th May.
   So I will be unavailable till that time.

[1]: https://github.com/pranitbauva1997/git
[2]: http://thread.gmane.org/gmane.comp.version-control.git/293635
[3]: https://github.com/pranitbauva1997/git/pull/1
[4]: https://github.com/pranitbauva1997/git/pull/2
[5]: https://github.com/pranitbauva1997/git/pull/3
[6]: http://article.gmane.org/gmane.comp.version-control.git/293909
--
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/5] git-gui i18n: mark strings for translation

[Vasco Almeida]

Mark strings for translation in lib/index.tcl that were seemingly
left behind by 700e560 ("git-gui: Mark forgotten strings for
translation.", 2008-09-04) which marks string in do_revert_selection
procedure.
These strings are passed to unstage_help and add_helper procedures.

Signed-off-by: Vasco Almeida 
---
 lib/index.tcl | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/index.tcl b/lib/index.tcl
index 74a81a7..3a3e534 100644
--- a/lib/index.tcl
+++ b/lib/index.tcl
@@ -291,7 +291,7 @@ proc do_unstage_selection {} {
 
if {[array size selected_paths] > 0} {
unstage_helper \
-   {Unstaging selected files from commit} \
+   [mc "Unstaging selected files from commit"] \
[array names selected_paths]
} elseif {$current_diff_path ne {}} {
unstage_helper \
@@ -343,7 +343,7 @@ proc do_add_selection {} {
 
if {[array size selected_paths] > 0} {
add_helper \
-   {Adding selected files} \
+   [mc "Adding selected files"] \
[array names selected_paths]
} elseif {$current_diff_path ne {}} {
add_helper \
@@ -385,7 +385,7 @@ proc do_add_all {} {
set paths [concat $paths $untracked_paths]
}
}
-   add_helper {Adding all changed files} $paths
+   add_helper [mc "Adding all changed files"] $paths
 }
 
 proc revert_helper {txt paths} {
-- 
2.7.3

--
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/5] git-gui: fix incorrect use of Tcl append command

[Vasco Almeida]

Fix wrong use of append command in strings marked for translation.
According to Tcl/Tk Documentation [1],
append varName ?value value value ...?
appends all value arguments to the current value of variable varName.
This means that
append "[appname] ([reponame]): " [mc "File Viewer"]
is setting a variable named "[appname] ([reponame]): " to the output of
[mc "File Viewer"], rather than returning the concatenation of both
expressions as one might expect.

The format for some strings enables, for instance, a French translator
to translate like "%s (%s) : Create Branch" (space before colon).
Conversely, strings already translated will be marked as fuzzy and the
translator must update them herself.

For some cases, use alternative way for concatenation instead of using
strcat procedure defined in git-gui.sh.

Reference: 31bb1d1 ("git-gui: Paper bag fix missing translated strings",
2007-09-14) fixes the same issue slightly differently.

[1] http://www.tcl.tk/man/tcl/TclCmd/append.htm

Signed-off-by: Vasco Almeida 
---
 lib/blame.tcl|  2 +-
 lib/branch_checkout.tcl  |  2 +-
 lib/branch_create.tcl|  2 +-
 lib/branch_delete.tcl|  2 +-
 lib/branch_rename.tcl|  2 +-
 lib/browser.tcl  |  4 ++--
 lib/database.tcl |  2 +-
 lib/diff.tcl | 11 +--
 lib/error.tcl|  4 ++--
 lib/merge.tcl|  2 +-
 lib/remote_add.tcl   |  2 +-
 lib/remote_branch_delete.tcl |  2 +-
 lib/shortcut.tcl |  6 +++---
 lib/tools_dlg.tcl|  6 +++---
 lib/transport.tcl|  2 +-
 15 files changed, 25 insertions(+), 26 deletions(-)

diff --git a/lib/blame.tcl b/lib/blame.tcl
index b1d15f4..a1aeb8b 100644
--- a/lib/blame.tcl
+++ b/lib/blame.tcl
@@ -70,7 +70,7 @@ constructor new {i_commit i_path i_jump} {
set path   $i_path
 
make_toplevel top w
-   wm title $top [append "[appname] ([reponame]): " [mc "File Viewer"]]
+   wm title $top [mc "%s (%s): File Viewer" [appname] [reponame]]
 
set font_w [font measure font_diff "0"]
 
diff --git a/lib/branch_checkout.tcl b/lib/branch_checkout.tcl
index 2e459a8..d06037d 100644
--- a/lib/branch_checkout.tcl
+++ b/lib/branch_checkout.tcl
@@ -13,7 +13,7 @@ constructor dialog {} {
global use_ttk NS
make_dialog top w
wm withdraw $w
-   wm title $top [append "[appname] ([reponame]): " [mc "Checkout Branch"]]
+   wm title $top [mc "%s (%s): Checkout Branch" [appname] [reponame]]
if {$top ne {.}} {
wm geometry $top "+[winfo rootx .]+[winfo rooty .]"
}
diff --git a/lib/branch_create.tcl b/lib/branch_create.tcl
index 4bb9077..ba367d5 100644
--- a/lib/branch_create.tcl
+++ b/lib/branch_create.tcl
@@ -20,7 +20,7 @@ constructor dialog {} {
 
make_dialog top w
wm withdraw $w
-   wm title $top [append "[appname] ([reponame]): " [mc "Create Branch"]]
+   wm title $top [mc "%s (%s): Create Branch" [appname] [reponame]]
if {$top ne {.}} {
wm geometry $top "+[winfo rootx .]+[winfo rooty .]"
}
diff --git a/lib/branch_delete.tcl b/lib/branch_delete.tcl
index 9aef0c9..a505163 100644
--- a/lib/branch_delete.tcl
+++ b/lib/branch_delete.tcl
@@ -13,7 +13,7 @@ constructor dialog {} {
 
make_dialog top w
wm withdraw $w
-   wm title $top [append "[appname] ([reponame]): " [mc "Delete Branch"]]
+   wm title $top [mc "%s (%s): Delete Branch" [appname] [reponame]]
if {$top ne {.}} {
wm geometry $top "+[winfo rootx .]+[winfo rooty .]"
}
diff --git a/lib/branch_rename.tcl b/lib/branch_rename.tcl
index 6e510ec..3a2d79a 100644
--- a/lib/branch_rename.tcl
+++ b/lib/branch_rename.tcl
@@ -12,7 +12,7 @@ constructor dialog {} {
 
make_dialog top w
wm withdraw $w
-   wm title $top [append "[appname] ([reponame]): " [mc "Rename Branch"]]
+   wm title $top [mc "%s (%s): Rename Branch" [appname] [reponame]]
if {$top ne {.}} {
wm geometry $top "+[winfo rootx .]+[winfo rooty .]"
}
diff --git a/lib/browser.tcl b/lib/browser.tcl
index 0328338..1580493 100644
--- a/lib/browser.tcl
+++ b/lib/browser.tcl
@@ -24,7 +24,7 @@ constructor new {commit {path {}}} {
global cursor_ptr M1B use_ttk NS
make_dialog top w
wm withdraw $top
-   wm title $top [append "[appname] ([reponame]): " [mc "File Browser"]]
+   wm title $top [mc "%s (%s): File Browser" [appname] [reponame]]
 
if {$path ne {}} {
if {[string index $path end] ne {/}} {
@@ -272,7 +272,7 @@ constructor dialog {} {
global use_ttk NS
make_dialog top w
wm withdraw $top
-   wm title $top [append "[appname] ([reponame]): " [mc "Browse Branch 
Files"]]
+   wm title $top [mc "%s (%s): Browse Branch Files" [appname] [reponame]]
if {$top ne {.}} {
wm 

[PATCH 3/5] git-gui i18n: mark "usage:" strings for translation

[Vasco Almeida]

Mark command-line "usage:" string for translation in git-gui.sh.

Signed-off-by: Vasco Almeida 
---
 git-gui.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/git-gui.sh b/git-gui.sh
index 11048c7..4ae344f 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -3007,7 +3007,7 @@ bind all <$M1B-Key-W> {destroy [winfo toplevel %W]}
 
 set subcommand_args {}
 proc usage {} {
-   set s "usage: $::argv0 $::subcommand $::subcommand_args"
+   set s "[mc usage:] $::argv0 $::subcommand $::subcommand_args"
if {[tk windowingsystem] eq "win32"} {
wm withdraw .
tk_messageBox -icon info -message $s \
@@ -3139,7 +3139,7 @@ gui {
# fall through to setup UI for commits
 }
 default {
-   set err "usage: $argv0 \[{blame|browser|citool}\]"
+   set err "[mc usage:] $argv0 \[{blame|browser|citool}\]"
if {[tk windowingsystem] eq "win32"} {
wm withdraw .
tk_messageBox -icon error -message $err \
-- 
2.7.3

--
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/5] git-gui i18n: mark string in lib/error.tcl for translation

[Vasco Almeida]

Mark string "$hook hook failed:" in lib/error.tcl for translation.

Signed-off-by: Vasco Almeida 
---
 lib/error.tcl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/error.tcl b/lib/error.tcl
index 71dc860..8968a57 100644
--- a/lib/error.tcl
+++ b/lib/error.tcl
@@ -77,7 +77,7 @@ proc hook_failed_popup {hook msg {is_fatal 1}} {
wm withdraw $w
 
${NS}::frame $w.m
-   ${NS}::label $w.m.l1 -text "$hook hook failed:" \
+   ${NS}::label $w.m.l1 -text [mc "%s hook failed:" $hook] \
-anchor w \
-justify left \
-font font_uibold
-- 
2.7.3

--
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/5] git-gui i18n: internationalize use of colon punctuation

[Vasco Almeida]

Internationalize use of colon punctuation ':' in options window, windows
titles, database statistics window. Some languages might use a different
style, for instance French uses "User Name :" (space before colon).

Signed-off-by: Vasco Almeida 
---
 lib/branch_delete.tcl | 2 +-
 lib/database.tcl  | 2 +-
 lib/error.tcl | 2 +-
 lib/option.tcl| 8 
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/lib/branch_delete.tcl b/lib/branch_delete.tcl
index 867938e..9aef0c9 100644
--- a/lib/branch_delete.tcl
+++ b/lib/branch_delete.tcl
@@ -128,7 +128,7 @@ method _delete {} {
set b [lindex $i 0]
set o [lindex $i 1]
if {[catch {git branch -D $b} err]} {
-   append failed " - $b: $err\n"
+   append failed [mc " - %s:" $b] " $err\n"
}
}
 
diff --git a/lib/database.tcl b/lib/database.tcl
index 1f187ed..8bd4b8e 100644
--- a/lib/database.tcl
+++ b/lib/database.tcl
@@ -54,7 +54,7 @@ proc do_stats {} {
set value "$value[lindex $s 2]"
}
 
-   ${NS}::label $w.stat.l_$name -text "$label:" -anchor w
+   ${NS}::label $w.stat.l_$name -text [mc "%s:" $label] -anchor w
${NS}::label $w.stat.v_$name -text $value -anchor w
grid $w.stat.l_$name $w.stat.v_$name -sticky we -padx {0 5}
}
diff --git a/lib/error.tcl b/lib/error.tcl
index c0fa69a..9b7d229 100644
--- a/lib/error.tcl
+++ b/lib/error.tcl
@@ -113,7 +113,7 @@ proc hook_failed_popup {hook msg {is_fatal 1}} {
 
bind $w  "grab $w; focus $w"
bind $w  "destroy $w"
-   wm title $w [strcat "[appname] ([reponame]): " [mc "error"]]
+   wm title $w [mc "%s (%s): error" [appname] [reponame]]
wm deiconify $w
tkwait window $w
 }
diff --git a/lib/option.tcl b/lib/option.tcl
index b5b6b2f..e43971b 100644
--- a/lib/option.tcl
+++ b/lib/option.tcl
@@ -179,7 +179,7 @@ proc do_options {} {
i-* {
regexp -- {-(\d+)\.\.(\d+)$} $type _junk min max
${NS}::frame $w.$f.$optid
-   ${NS}::label $w.$f.$optid.l -text "$text:"
+   ${NS}::label $w.$f.$optid.l -text [mc "%s:" 
$text]
pack $w.$f.$optid.l -side left -anchor w -fill x
tspinbox $w.$f.$optid.v \
-textvariable ${f}_config_new($name) \
@@ -194,7 +194,7 @@ proc do_options {} {
c -
t {
${NS}::frame $w.$f.$optid
-   ${NS}::label $w.$f.$optid.l -text "$text:"
+   ${NS}::label $w.$f.$optid.l -text [mc "%s:" 
$text]
${NS}::entry $w.$f.$optid.v \
-width 20 \
-textvariable ${f}_config_new($name)
@@ -217,7 +217,7 @@ proc do_options {} {
s {
set opts [eval [lindex $option 3]]
${NS}::frame $w.$f.$optid
-   ${NS}::label $w.$f.$optid.l -text "$text:"
+   ${NS}::label $w.$f.$optid.l -text [mc "%s:" 
$text]
if {$use_ttk} {
ttk::combobox $w.$f.$optid.v \
-textvariable 
${f}_config_new($name) \
@@ -279,7 +279,7 @@ proc do_options {} {
[font configure $font -size]
 
${NS}::frame $w.global.$name
-   ${NS}::label $w.global.$name.l -text "$text:"
+   ${NS}::label $w.global.$name.l -text [mc "%s:" $text]
${NS}::button $w.global.$name.b \
-text [mc "Change Font"] \
-command [list \
-- 
2.7.3

--
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 80/83] run-command: make dup_devnull() non static

[Duy Nguyen]

On Sun, May 8, 2016 at 1:33 PM, Johannes Schindelin
 wrote:
> The claim is that this libifies the procedure. But it makes the code
> really nasty for use as a library: if this is run in a thread (and you
> know that we are going to have to do this in the near future, for
> performance reasons), it will completely mess up all the other threads
> because it messes with the global file descriptors.

I vote one step at a time, leave multi-thread support for future.
There's a lot more shared state than file descriptors anyway, at least
there are object db and index access and probably a couple of hidden
static variables somewhere. And I'm not sure if multi-thread really
helps here. Are we really CPU-bound? If object inflation causes that
(wild guess), can we just inflate ahead in some separate process and
pass the result back?
-- 
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 v8 18/19] Add tracing to measure where most of the time is spent

[Duy Nguyen]

On Fri, May 6, 2016 at 5:48 AM, Junio C Hamano  wrote:
> David Turner  writes:
>
>> From: Nguyễn Thái Ngọc Duy 
>> Subject: Re: [PATCH v8 18/19] Add tracing to measure where most of the time 
>> is spent
>
> trace: measure where the time is spent in the index-heavy operations


Right. All these involve index somewhat, except read_packed_refs().
-- 
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

[PATCH v3 39/41] upload-pack.c: use error_errno()

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 upload-pack.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/upload-pack.c b/upload-pack.c
index dc802a0..f19444d 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -174,8 +174,7 @@ static void create_pack_file(void)
 
if (ret < 0) {
if (errno != EINTR) {
-   error("poll failed, resuming: %s",
- strerror(errno));
+   error_errno("poll failed, resuming");
sleep(1);
}
continue;
-- 
2.8.0.rc0.210.gd302cd2

--
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 35/41] server-info.c: use error_errno()

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 server-info.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/server-info.c b/server-info.c
index 5a86e29..75dd677 100644
--- a/server-info.c
+++ b/server-info.c
@@ -36,7 +36,7 @@ static int update_info_file(char *path, int (*generate)(FILE 
*))
 
 out:
if (ret) {
-   error("unable to update %s: %s", path, strerror(errno));
+   error_errno("unable to update %s", path);
if (fp)
fclose(fp);
else if (fd >= 0)
-- 
2.8.0.rc0.210.gd302cd2

--
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] Move test-* to t/helper/ subdirectory

[Duy Nguyen]

So among the options we have so far, which way should we go, or leave it as is?

On Tue, May 3, 2016 at 7:15 AM, Duy Nguyen  wrote:
> On Tue, May 3, 2016 at 12:34 AM, Junio C Hamano  wrote:
>> Duy Nguyen  writes:
>>
>>> I may have rushed to judgement. wrap-for-bin.sh has always been the
>>> dependency for bin-wrappers/*. If we force that file to change, then
>>> bin-wrappers/* will be recreated when switching branches. So how about
>>> this?
>>
>> I do not think you are "force updating wrap-for-bin" in any way in
>> the patch, though.  You are building it in such a way that it does
>> not have to get updated within the revision that contains e6e7530
>> (assuming that this will be queued directly on top it and merged to
>> everywhere e6e7530 is contained).
>
> Yep.
>
>> The new case/esac looks somewhat bad (its knowing that where test-*
>> lives, test-* is the only thing that is special, etc. troubles me at
>> the same time that case/esac is funnily formated).
>
> We could just make some random changes in this file. That would have
> the same effect.
>
>> Where does "@@PATH@@" come from and who rewrites it?  Is that a
>> misspelt "@@PROG@@"?
>
> Yep. Should have run make distclean before testing :(
> --
> Duy
-- 
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

[PATCH v3 38/41] unpack-trees.c: use error_errno()

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 unpack-trees.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/unpack-trees.c b/unpack-trees.c
index 9f55cc2..bb0d142 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1499,8 +1499,7 @@ static int verify_absent_1(const struct cache_entry *ce,
 
path = xmemdupz(ce->name, len);
if (lstat(path, ))
-   ret = error("cannot stat '%s': %s", path,
-   strerror(errno));
+   ret = error_errno("cannot stat '%s'", path);
else
ret = check_ok_to_remove(path, len, DT_UNKNOWN, NULL,
 , error_type, o);
@@ -1508,8 +1507,7 @@ static int verify_absent_1(const struct cache_entry *ce,
return ret;
} else if (lstat(ce->name, )) {
if (errno != ENOENT)
-   return error("cannot stat '%s': %s", ce->name,
-strerror(errno));
+   return error_errno("cannot stat '%s'", ce->name);
return 0;
} else {
return check_ok_to_remove(ce->name, ce_namelen(ce),
-- 
2.8.0.rc0.210.gd302cd2

--
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 37/41] transport-helper.c: use error_errno()

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 transport-helper.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/transport-helper.c b/transport-helper.c
index b934183..f09fadc 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -1166,7 +1166,7 @@ static int udt_do_read(struct unidirectional_transfer *t)
bytes = read(t->src, t->buf + t->bufuse, BUFFERSIZE - t->bufuse);
if (bytes < 0 && errno != EWOULDBLOCK && errno != EAGAIN &&
errno != EINTR) {
-   error("read(%s) failed: %s", t->src_name, strerror(errno));
+   error_errno("read(%s) failed", t->src_name);
return -1;
} else if (bytes == 0) {
transfer_debug("%s EOF (with %i bytes in buffer)",
@@ -1193,7 +1193,7 @@ static int udt_do_write(struct unidirectional_transfer *t)
transfer_debug("%s is writable", t->dest_name);
bytes = xwrite(t->dest, t->buf, t->bufuse);
if (bytes < 0 && errno != EWOULDBLOCK) {
-   error("write(%s) failed: %s", t->dest_name, strerror(errno));
+   error_errno("write(%s) failed", t->dest_name);
return -1;
} else if (bytes > 0) {
t->bufuse -= bytes;
@@ -1306,7 +1306,7 @@ static int tloop_join(pid_t pid, const char *name)
 {
int tret;
if (waitpid(pid, , 0) < 0) {
-   error("%s process failed to wait: %s", name, strerror(errno));
+   error_errno("%s process failed to wait", name);
return 1;
}
if (!WIFEXITED(tret) || WEXITSTATUS(tret)) {
-- 
2.8.0.rc0.210.gd302cd2

--
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 36/41] sha1_file.c: use {error,die,warning}_errno()

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 sha1_file.c | 32 +---
 1 file changed, 13 insertions(+), 19 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index d0f2aa0..a7f45b3 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1107,9 +1107,8 @@ unsigned char *use_pack(struct packed_git *p,
PROT_READ, MAP_PRIVATE,
p->pack_fd, win->offset);
if (win->base == MAP_FAILED)
-   die("packfile %s cannot be mapped: %s",
-   p->pack_name,
-   strerror(errno));
+   die_errno("packfile %s cannot be mapped",
+ p->pack_name);
if (!win->offset && win->len == p->pack_size
&& !p->do_not_close)
close_pack_fd(p);
@@ -1279,8 +1278,8 @@ static void prepare_packed_git_one(char *objdir, int 
local)
dir = opendir(path.buf);
if (!dir) {
if (errno != ENOENT)
-   error("unable to open object pack directory: %s: %s",
- path.buf, strerror(errno));
+   error_errno("unable to open object pack directory: %s",
+   path.buf);
strbuf_release();
return;
}
@@ -2984,7 +2983,7 @@ int finalize_object_file(const char *tmpfile, const char 
*filename)
unlink_or_warn(tmpfile);
if (ret) {
if (ret != EEXIST) {
-   return error("unable to write sha1 filename %s: %s", 
filename, strerror(ret));
+   return error_errno("unable to write sha1 filename %s", 
filename);
}
/* FIXME!!! Collision check here ? */
}
@@ -2998,7 +2997,7 @@ out:
 static int write_buffer(int fd, const void *buf, size_t len)
 {
if (write_in_full(fd, buf, len) < 0)
-   return error("file write error (%s)", strerror(errno));
+   return error_errno("file write error");
return 0;
 }
 
@@ -3081,7 +3080,7 @@ static int write_loose_object(const unsigned char *sha1, 
char *hdr, int hdrlen,
if (errno == EACCES)
return error("insufficient permission for adding an 
object to repository database %s", get_object_directory());
else
-   return error("unable to create temporary file: %s", 
strerror(errno));
+   return error_errno("unable to create temporary file");
}
 
/* Set it up */
@@ -3126,8 +3125,7 @@ static int write_loose_object(const unsigned char *sha1, 
char *hdr, int hdrlen,
utb.actime = mtime;
utb.modtime = mtime;
if (utime(tmp_file.buf, ) < 0)
-   warning("failed utime() on %s: %s",
-   tmp_file.buf, strerror(errno));
+   warning_errno("failed utime() on %s", tmp_file.buf);
}
 
return finalize_object_file(tmp_file.buf, filename);
@@ -3360,7 +3358,7 @@ static int index_core(unsigned char *sha1, int fd, size_t 
size,
if (size == read_in_full(fd, buf, size))
ret = index_mem(sha1, buf, size, type, path, flags);
else
-   ret = error("short read %s", strerror(errno));
+   ret = error_errno("short read");
free(buf);
} else {
void *buf = xmmap(NULL, size, PROT_READ, MAP_PRIVATE, fd, 0);
@@ -3425,18 +3423,14 @@ int index_path(unsigned char *sha1, const char *path, 
struct stat *st, unsigned
case S_IFREG:
fd = open(path, O_RDONLY);
if (fd < 0)
-   return error("open(\"%s\"): %s", path,
-strerror(errno));
+   return error_errno("open(\"%s\")", path);
if (index_fd(sha1, fd, st, OBJ_BLOB, path, flags) < 0)
return error("%s: failed to insert into database",
 path);
break;
case S_IFLNK:
-   if (strbuf_readlink(, path, st->st_size)) {
-   char *errstr = strerror(errno);
-   return error("readlink(\"%s\"): %s", path,
-errstr);
-   }
+   if (strbuf_readlink(, path, st->st_size))
+   return error_errno("readlink(\"%s\")", path);
if (!(flags & HASH_WRITE_OBJECT))
hash_sha1_file(sb.buf, sb.len, blob_type, sha1);
else if (write_sha1_file(sb.buf, sb.len, blob_type, sha1))
@@ -3492,7 +3486,7 @@ static int for_each_file_in_obj_subdir(int 

[PATCH v3 40/41] vcs-svn: use error_errno()

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 vcs-svn/line_buffer.c| 4 ++--
 vcs-svn/sliding_window.c | 2 +-
 vcs-svn/svndiff.c| 4 ++--
 vcs-svn/svndump.c| 4 ++--
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/vcs-svn/line_buffer.c b/vcs-svn/line_buffer.c
index 57cc1ce..e416caf 100644
--- a/vcs-svn/line_buffer.c
+++ b/vcs-svn/line_buffer.c
@@ -53,9 +53,9 @@ long buffer_tmpfile_prepare_to_read(struct line_buffer *buf)
 {
long pos = ftell(buf->infile);
if (pos < 0)
-   return error("ftell error: %s", strerror(errno));
+   return error_errno("ftell error");
if (fseek(buf->infile, 0, SEEK_SET))
-   return error("seek error: %s", strerror(errno));
+   return error_errno("seek error");
return pos;
 }
 
diff --git a/vcs-svn/sliding_window.c b/vcs-svn/sliding_window.c
index f11d490..06d273c 100644
--- a/vcs-svn/sliding_window.c
+++ b/vcs-svn/sliding_window.c
@@ -12,7 +12,7 @@ static int input_error(struct line_buffer *file)
 {
if (!buffer_ferror(file))
return error("delta preimage ends early");
-   return error("cannot read delta preimage: %s", strerror(errno));
+   return error_errno("cannot read delta preimage");
 }
 
 static int skip_or_whine(struct line_buffer *file, off_t gap)
diff --git a/vcs-svn/svndiff.c b/vcs-svn/svndiff.c
index 74c97c4..75c7531 100644
--- a/vcs-svn/svndiff.c
+++ b/vcs-svn/svndiff.c
@@ -64,13 +64,13 @@ static int write_strbuf(struct strbuf *sb, FILE *out)
 {
if (fwrite(sb->buf, 1, sb->len, out) == sb->len)/* Success. */
return 0;
-   return error("cannot write delta postimage: %s", strerror(errno));
+   return error_errno("cannot write delta postimage");
 }
 
 static int error_short_read(struct line_buffer *input)
 {
if (buffer_ferror(input))
-   return error("error reading delta: %s", strerror(errno));
+   return error_errno("error reading delta");
return error("invalid delta: unexpected end of file");
 }
 
diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c
index 31d1d83..e4b3959 100644
--- a/vcs-svn/svndump.c
+++ b/vcs-svn/svndump.c
@@ -501,7 +501,7 @@ static void init(int report_fd)
 int svndump_init(const char *filename)
 {
if (buffer_init(, filename))
-   return error("cannot open %s: %s", filename ? filename : 
"NULL", strerror(errno));
+   return error_errno("cannot open %s", filename ? filename : 
"NULL");
init(REPORT_FILENO);
return 0;
 }
@@ -509,7 +509,7 @@ int svndump_init(const char *filename)
 int svndump_init_fd(int in_fd, int back_fd)
 {
if(buffer_fdinit(, xdup(in_fd)))
-   return error("cannot open fd %d: %s", in_fd, strerror(errno));
+   return error_errno("cannot open fd %d", in_fd);
init(xdup(back_fd));
return 0;
 }
-- 
2.8.0.rc0.210.gd302cd2

--
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 41/41] wrapper.c: use warning_errno()

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 wrapper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/wrapper.c b/wrapper.c
index 9afc1a0..3df2fe0 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -572,7 +572,7 @@ static int warn_if_unremovable(const char *op, const char 
*file, int rc)
if (!rc || errno == ENOENT)
return 0;
err = errno;
-   warning("unable to %s %s: %s", op, file, strerror(errno));
+   warning_errno("unable to %s %s", op, file);
errno = err;
return rc;
 }
@@ -608,7 +608,7 @@ int remove_or_warn(unsigned int mode, const char *file)
 
 void warn_on_inaccessible(const char *path)
 {
-   warning(_("unable to access '%s': %s"), path, strerror(errno));
+   warning_errno(_("unable to access '%s'"), path);
 }
 
 static int access_error_is_ok(int err, unsigned flag)
-- 
2.8.0.rc0.210.gd302cd2

--
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 29/41] ident.c: use warning_errno()

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 ident.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/ident.c b/ident.c
index 4fd82d1..139c528 100644
--- a/ident.c
+++ b/ident.c
@@ -75,14 +75,12 @@ static int add_mailname_host(struct strbuf *buf)
mailname = fopen("/etc/mailname", "r");
if (!mailname) {
if (errno != ENOENT)
-   warning("cannot open /etc/mailname: %s",
-   strerror(errno));
+   warning_errno("cannot open /etc/mailname");
return -1;
}
if (strbuf_getline(, mailname) == EOF) {
if (ferror(mailname))
-   warning("cannot read /etc/mailname: %s",
-   strerror(errno));
+   warning_errno("cannot read /etc/mailname");
strbuf_release();
fclose(mailname);
return -1;
@@ -125,7 +123,7 @@ static void add_domainname(struct strbuf *out, int 
*is_bogus)
char buf[1024];
 
if (gethostname(buf, sizeof(buf))) {
-   warning("cannot get host name: %s", strerror(errno));
+   warning_errno("cannot get host name");
strbuf_addstr(out, "(none)");
*is_bogus = 1;
return;
-- 
2.8.0.rc0.210.gd302cd2

--
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 30/41] mailmap.c: use error_errno()

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 mailmap.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/mailmap.c b/mailmap.c
index 9726237..b5c521f 100644
--- a/mailmap.c
+++ b/mailmap.c
@@ -189,8 +189,7 @@ static int read_mailmap_file(struct string_list *map, const 
char *filename,
if (!f) {
if (errno == ENOENT)
return 0;
-   return error("unable to open mailmap at %s: %s",
-filename, strerror(errno));
+   return error_errno("unable to open mailmap at %s", filename);
}
 
while (fgets(buffer, sizeof(buffer), f) != NULL)
-- 
2.8.0.rc0.210.gd302cd2

--
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 25/41] fast-import.c: use error_errno()

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 fast-import.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/fast-import.c b/fast-import.c
index 9fc7093..21881d1 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -414,7 +414,7 @@ static void write_crash_report(const char *err)
struct recent_command *rc;
 
if (!rpt) {
-   error("can't write crash report %s: %s", loc, strerror(errno));
+   error_errno("can't write crash report %s", loc);
free(loc);
return;
}
@@ -1806,8 +1806,8 @@ static void dump_marks(void)
return;
 
if (hold_lock_file_for_update(_lock, export_marks_file, 0) < 0) {
-   failure |= error("Unable to write marks file %s: %s",
-   export_marks_file, strerror(errno));
+   failure |= error_errno("Unable to write marks file %s",
+  export_marks_file);
return;
}
 
@@ -1822,8 +1822,8 @@ static void dump_marks(void)
 
dump_marks_helper(f, 0, marks);
if (commit_lock_file(_lock)) {
-   failure |= error("Unable to write file %s: %s",
-   export_marks_file, strerror(errno));
+   failure |= error_errno("Unable to write file %s",
+  export_marks_file);
return;
}
 }
-- 
2.8.0.rc0.210.gd302cd2

--
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 34/41] sequencer.c: use error_errno()

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 sequencer.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index e66f2fe..4687ad4 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -875,8 +875,7 @@ static int sequencer_rollback(struct replay_opts *opts)
return rollback_single_pick();
}
if (!f)
-   return error(_("cannot open %s: %s"), git_path_head_file(),
-   strerror(errno));
+   return error_errno(_("cannot open %s"), git_path_head_file());
if (strbuf_getline_lf(, f)) {
error(_("cannot read %s: %s"), git_path_head_file(),
  ferror(f) ?  strerror(errno) : _("unexpected end of 
file"));
-- 
2.8.0.rc0.210.gd302cd2

--
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 31/41] reachable.c: use error_errno()

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 reachable.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/reachable.c b/reachable.c
index ed35201..d0199ca 100644
--- a/reachable.c
+++ b/reachable.c
@@ -119,8 +119,7 @@ static int add_recent_loose(const unsigned char *sha1,
 */
if (errno == ENOENT)
return 0;
-   return error("unable to stat %s: %s",
-sha1_to_hex(sha1), strerror(errno));
+   return error_errno("unable to stat %s", sha1_to_hex(sha1));
}
 
add_recent_object(sha1, st.st_mtime, data);
-- 
2.8.0.rc0.210.gd302cd2

--
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 26/41] gpg-interface.c: use error_errno()

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 gpg-interface.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/gpg-interface.c b/gpg-interface.c
index 2259938..c4b1e8c 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -219,11 +219,9 @@ int verify_signed_buffer(const char *payload, size_t 
payload_size,
args_gpg[0] = gpg_program;
fd = git_mkstemp(path, PATH_MAX, ".git_vtag_tmpXX");
if (fd < 0)
-   return error(_("could not create temporary file '%s': %s"),
-path, strerror(errno));
+   return error_errno(_("could not create temporary file '%s'"), 
path);
if (write_in_full(fd, signature, signature_size) < 0)
-   return error(_("failed writing detached signature to '%s': %s"),
-path, strerror(errno));
+   return error_errno(_("failed writing detached signature to 
'%s'"), path);
close(fd);
 
gpg.argv = args_gpg;
-- 
2.8.0.rc0.210.gd302cd2

--
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 27/41] grep.c: use error_errno()

[Nguyễn Thái Ngọc Duy]

While at there, improve the error message a bit (what operation failed?)

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 grep.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/grep.c b/grep.c
index 528b652..ec6f7ff 100644
--- a/grep.c
+++ b/grep.c
@@ -1732,7 +1732,7 @@ static int grep_source_load_file(struct grep_source *gs)
if (lstat(filename, ) < 0) {
err_ret:
if (errno != ENOENT)
-   error(_("'%s': %s"), filename, strerror(errno));
+   error_errno(_("failed to stat '%s'"), filename);
return -1;
}
if (!S_ISREG(st.st_mode))
@@ -1743,7 +1743,7 @@ static int grep_source_load_file(struct grep_source *gs)
goto err_ret;
data = xmallocz(size);
if (st.st_size != read_in_full(i, data, size)) {
-   error(_("'%s': short read %s"), filename, strerror(errno));
+   error_errno(_("'%s': short read"), filename);
close(i);
free(data);
return -1;
-- 
2.8.0.rc0.210.gd302cd2

--
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 24/41] entry.c: use error_errno()

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 entry.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/entry.c b/entry.c
index a410957..519e042 100644
--- a/entry.c
+++ b/entry.c
@@ -168,8 +168,8 @@ static int write_entry(struct cache_entry *ce,
ret = symlink(new, path);
free(new);
if (ret)
-   return error("unable to create symlink %s (%s)",
-path, strerror(errno));
+   return error_errno("unable to create symlink 
%s",
+  path);
break;
}
 
@@ -186,8 +186,7 @@ static int write_entry(struct cache_entry *ce,
fd = open_output_fd(path, ce, to_tempfile);
if (fd < 0) {
free(new);
-   return error("unable to create file %s (%s)",
-   path, strerror(errno));
+   return error_errno("unable to create file %s", path);
}
 
wrote = write_in_full(fd, new, size);
@@ -284,8 +283,7 @@ int checkout_entry(struct cache_entry *ce,
return error("%s is a directory", path.buf);
remove_subtree();
} else if (unlink(path.buf))
-   return error("unable to unlink old '%s' (%s)",
-path.buf, strerror(errno));
+   return error_errno("unable to unlink old '%s'", 
path.buf);
} else if (state->not_new)
return 0;
 
-- 
2.8.0.rc0.210.gd302cd2

--
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 32/41] rerere.c: use error_errno() and warning_errno()

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 rerere.c | 18 +++---
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/rerere.c b/rerere.c
index c8b9f40..1810c04 100644
--- a/rerere.c
+++ b/rerere.c
@@ -501,8 +501,7 @@ static int handle_file(const char *path, unsigned char 
*sha1, const char *output
error("There were errors while writing %s (%s)",
  path, strerror(io.io.wrerror));
if (io.io.output && fclose(io.io.output))
-   io.io.wrerror = error("Failed to flush %s: %s",
- path, strerror(errno));
+   io.io.wrerror = error_errno("Failed to flush %s", path);
 
if (hunk_no < 0) {
if (output)
@@ -684,20 +683,17 @@ static int merge(const struct rerere_id *id, const char 
*path)
 * Mark that "postimage" was used to help gc.
 */
if (utime(rerere_path(id, "postimage"), NULL) < 0)
-   warning("failed utime() on %s: %s",
-   rerere_path(id, "postimage"),
-   strerror(errno));
+   warning_errno("failed utime() on %s",
+ rerere_path(id, "postimage"));
 
/* Update "path" with the resolution */
f = fopen(path, "w");
if (!f)
-   return error("Could not open %s: %s", path,
-strerror(errno));
+   return error_errno("Could not open %s", path);
if (fwrite(result.ptr, result.size, 1, f) != 1)
-   error("Could not write %s: %s", path, strerror(errno));
+   error_errno("Could not write %s", path);
if (fclose(f))
-   return error("Writing %s failed: %s", path,
-strerror(errno));
+   return error_errno("Writing %s failed", path);
 
 out:
free(cur.ptr);
@@ -1071,7 +1067,7 @@ static int rerere_forget_one_path(const char *path, 
struct string_list *rr)
if (unlink(filename))
return (errno == ENOENT
? error("no remembered resolution for %s", path)
-   : error("cannot unlink %s: %s", filename, 
strerror(errno)));
+   : error_errno("cannot unlink %s", filename));
 
/*
 * Update the preimage so that the user can resolve the
-- 
2.8.0.rc0.210.gd302cd2

--
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 33/41] run-command.c: use error_errno()

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 run-command.c | 15 +++
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/run-command.c b/run-command.c
index e4593cd..842c8d1 100644
--- a/run-command.c
+++ b/run-command.c
@@ -233,7 +233,7 @@ static int wait_or_whine(pid_t pid, const char *argv0, int 
in_signal)
 
if (waiting < 0) {
failed_errno = errno;
-   error("waitpid for %s failed: %s", argv0, strerror(errno));
+   error_errno("waitpid for %s failed", argv0);
} else if (waiting != pid) {
error("waitpid is confused (%s)", argv0);
} else if (WIFSIGNALED(status)) {
@@ -420,8 +420,7 @@ fail_pipe:
}
}
if (cmd->pid < 0)
-   error("cannot fork() for %s: %s", cmd->argv[0],
-   strerror(errno));
+   error_errno("cannot fork() for %s", cmd->argv[0]);
else if (cmd->clean_on_exit)
mark_child_for_cleanup(cmd->pid);
 
@@ -482,7 +481,7 @@ fail_pipe:
cmd->dir, fhin, fhout, fherr);
failed_errno = errno;
if (cmd->pid < 0 && (!cmd->silent_exec_failure || errno != ENOENT))
-   error("cannot spawn %s: %s", cmd->argv[0], strerror(errno));
+   error_errno("cannot spawn %s", cmd->argv[0]);
if (cmd->clean_on_exit && cmd->pid >= 0)
mark_child_for_cleanup(cmd->pid);
 
@@ -703,7 +702,7 @@ int start_async(struct async *async)
if (pipe(fdin) < 0) {
if (async->out > 0)
close(async->out);
-   return error("cannot create pipe: %s", strerror(errno));
+   return error_errno("cannot create pipe");
}
async->in = fdin[1];
}
@@ -715,7 +714,7 @@ int start_async(struct async *async)
close_pair(fdin);
else if (async->in)
close(async->in);
-   return error("cannot create pipe: %s", strerror(errno));
+   return error_errno("cannot create pipe");
}
async->out = fdout[0];
}
@@ -740,7 +739,7 @@ int start_async(struct async *async)
 
async->pid = fork();
if (async->pid < 0) {
-   error("fork (async) failed: %s", strerror(errno));
+   error_errno("fork (async) failed");
goto error;
}
if (!async->pid) {
@@ -787,7 +786,7 @@ int start_async(struct async *async)
{
int err = pthread_create(>tid, NULL, run_thread, async);
if (err) {
-   error("cannot create thread: %s", strerror(err));
+   error_errno("cannot create thread");
goto error;
}
}
-- 
2.8.0.rc0.210.gd302cd2

--
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 28/41] http.c: use error_errno() and warning_errno()

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 http.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/http.c b/http.c
index 4304b80..7565c93 100644
--- a/http.c
+++ b/http.c
@@ -446,8 +446,7 @@ static int sockopt_callback(void *client, curl_socket_t fd, 
curlsocktype type)
 
rc = setsockopt(fd, SOL_SOCKET, SO_KEEPALIVE, (void *), len);
if (rc < 0)
-   warning("unable to set SO_KEEPALIVE on socket %s",
-   strerror(errno));
+   warning_errno("unable to set SO_KEEPALIVE on socket");
 
return 0; /* CURL_SOCKOPT_OK only exists since curl 7.21.5 */
 }
@@ -1894,8 +1893,7 @@ struct http_object_request *new_http_object_request(const 
char *base_url,
}
 
if (freq->localfile < 0) {
-   error("Couldn't create temporary file %s: %s",
- freq->tmpfile, strerror(errno));
+   error_errno("Couldn't create temporary file %s", freq->tmpfile);
goto abort;
}
 
@@ -1940,8 +1938,8 @@ struct http_object_request *new_http_object_request(const 
char *base_url,
prev_posn = 0;
lseek(freq->localfile, 0, SEEK_SET);
if (ftruncate(freq->localfile, 0) < 0) {
-   error("Couldn't truncate temporary file %s: %s",
- freq->tmpfile, strerror(errno));
+   error_errno("Couldn't truncate temporary file 
%s",
+   freq->tmpfile);
goto abort;
}
}
-- 
2.8.0.rc0.210.gd302cd2

--
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 23/41] editor.c: use error_errno()

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 editor.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/editor.c b/editor.c
index 01c644c..7519ede 100644
--- a/editor.c
+++ b/editor.c
@@ -63,7 +63,6 @@ int launch_editor(const char *path, struct strbuf *buffer, 
const char *const *en
if (!buffer)
return 0;
if (strbuf_read_file(buffer, path, 0) < 0)
-   return error("could not read file '%s': %s",
-   path, strerror(errno));
+   return error_errno("could not read file '%s'", path);
return 0;
 }
-- 
2.8.0.rc0.210.gd302cd2

--
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 18/41] config.c: use error_errno()

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 config.c | 22 +-
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/config.c b/config.c
index 10b5c95..80411e4 100644
--- a/config.c
+++ b/config.c
@@ -2012,7 +2012,7 @@ int git_config_set_multivar_in_file_gently(const char 
*config_filename,
lock = xcalloc(1, sizeof(struct lock_file));
fd = hold_lock_file_for_update(lock, config_filename, 0);
if (fd < 0) {
-   error("could not lock config file %s: %s", config_filename, 
strerror(errno));
+   error_errno("could not lock config file %s", config_filename);
free(store.key);
ret = CONFIG_NO_LOCK;
goto out_free;
@@ -2026,8 +2026,7 @@ int git_config_set_multivar_in_file_gently(const char 
*config_filename,
free(store.key);
 
if ( ENOENT != errno ) {
-   error("opening %s: %s", config_filename,
- strerror(errno));
+   error_errno("opening %s", config_filename);
ret = CONFIG_INVALID_FILE; /* same as "invalid config 
file" */
goto out_free;
}
@@ -2111,8 +2110,7 @@ int git_config_set_multivar_in_file_gently(const char 
*config_filename,
if (contents == MAP_FAILED) {
if (errno == ENODEV && S_ISDIR(st.st_mode))
errno = EISDIR;
-   error("unable to mmap '%s': %s",
- config_filename, strerror(errno));
+   error_errno("unable to mmap '%s'", config_filename);
ret = CONFIG_INVALID_FILE;
contents = NULL;
goto out_free;
@@ -2121,8 +2119,7 @@ int git_config_set_multivar_in_file_gently(const char 
*config_filename,
in_fd = -1;
 
if (chmod(get_lock_file_path(lock), st.st_mode & 0) < 0) {
-   error("chmod on %s failed: %s",
- get_lock_file_path(lock), strerror(errno));
+   error_errno("chmod on %s failed", 
get_lock_file_path(lock));
ret = CONFIG_NO_WRITE;
goto out_free;
}
@@ -2178,8 +2175,7 @@ int git_config_set_multivar_in_file_gently(const char 
*config_filename,
}
 
if (commit_lock_file(lock) < 0) {
-   error("could not write config file %s: %s", config_filename,
- strerror(errno));
+   error_errno("could not write config file %s", config_filename);
ret = CONFIG_NO_WRITE;
lock = NULL;
goto out_free;
@@ -2330,8 +2326,8 @@ int git_config_rename_section_in_file(const char 
*config_filename,
fstat(fileno(config_file), );
 
if (chmod(get_lock_file_path(lock), st.st_mode & 0) < 0) {
-   ret = error("chmod on %s failed: %s",
-   get_lock_file_path(lock), strerror(errno));
+   ret = error_errno("chmod on %s failed",
+ get_lock_file_path(lock));
goto out;
}
 
@@ -2385,8 +2381,8 @@ int git_config_rename_section_in_file(const char 
*config_filename,
fclose(config_file);
 unlock_and_out:
if (commit_lock_file(lock) < 0)
-   ret = error("could not write config file %s: %s",
-   config_filename, strerror(errno));
+   ret = error_errno("could not write config file %s",
+ config_filename);
 out:
free(filename_buf);
return ret;
-- 
2.8.0.rc0.210.gd302cd2

--
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 16/41] combine-diff.c: use error_errno()

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 combine-diff.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/combine-diff.c b/combine-diff.c
index 0e1d4b0..8f2313d 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -1005,8 +1005,7 @@ static void show_patch_diff(struct combine_diff_path 
*elem, int num_parent,
struct strbuf buf = STRBUF_INIT;
 
if (strbuf_readlink(, elem->path, st.st_size) < 0) {
-   error("readlink(%s): %s", elem->path,
- strerror(errno));
+   error_errno("readlink(%s)", elem->path);
return;
}
result_size = buf.len;
-- 
2.8.0.rc0.210.gd302cd2

--
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 22/41] diff-no-index.c: use error_errno()

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 diff-no-index.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/diff-no-index.c b/diff-no-index.c
index 03daadb..1f8999b 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -65,8 +65,7 @@ static int populate_from_stdin(struct diff_filespec *s)
size_t size = 0;
 
if (strbuf_read(, 0, 0) < 0)
-   return error("error while reading from stdin %s",
-strerror(errno));
+   return error_errno("error while reading from stdin");
 
s->should_munmap = 0;
s->data = strbuf_detach(, );
-- 
2.8.0.rc0.210.gd302cd2

--
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 17/41] compat/win32/syslog.c: use warning_errno()

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 compat/win32/syslog.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/compat/win32/syslog.c b/compat/win32/syslog.c
index b905aea..6c7c9b6 100644
--- a/compat/win32/syslog.c
+++ b/compat/win32/syslog.c
@@ -28,13 +28,13 @@ void syslog(int priority, const char *fmt, ...)
va_end(ap);
 
if (str_len < 0) {
-   warning("vsnprintf failed: '%s'", strerror(errno));
+   warning_errno("vsnprintf failed");
return;
}
 
str = malloc(st_add(str_len, 1));
if (!str) {
-   warning("malloc failed: '%s'", strerror(errno));
+   warning_errno("malloc failed");
return;
}
 
@@ -45,7 +45,7 @@ void syslog(int priority, const char *fmt, ...)
while ((pos = strstr(str, "%1")) != NULL) {
str = realloc(str, st_add(++str_len, 1));
if (!str) {
-   warning("realloc failed: '%s'", strerror(errno));
+   warning_errno("realloc failed");
return;
}
memmove(pos + 2, pos + 1, strlen(pos));
-- 
2.8.0.rc0.210.gd302cd2

--
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 21/41] credential-cache--daemon.c: use warning_errno()

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 credential-cache--daemon.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/credential-cache--daemon.c b/credential-cache--daemon.c
index 291c0fd..1f14d56 100644
--- a/credential-cache--daemon.c
+++ b/credential-cache--daemon.c
@@ -179,12 +179,12 @@ static int serve_cache_loop(int fd)
 
client = accept(fd, NULL, NULL);
if (client < 0) {
-   warning("accept failed: %s", strerror(errno));
+   warning_errno("accept failed");
return 1;
}
client2 = dup(client);
if (client2 < 0) {
-   warning("dup failed: %s", strerror(errno));
+   warning_errno("dup failed");
close(client);
return 1;
}
-- 
2.8.0.rc0.210.gd302cd2

--
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 19/41] connected.c: use error_errno()

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 connected.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/connected.c b/connected.c
index 299c560..bf1b12e 100644
--- a/connected.c
+++ b/connected.c
@@ -86,17 +86,14 @@ static int check_everything_connected_real(sha1_iterate_fn 
fn,
memcpy(commit, sha1_to_hex(sha1), 40);
if (write_in_full(rev_list.in, commit, 41) < 0) {
if (errno != EPIPE && errno != EINVAL)
-   error(_("failed write to rev-list: %s"),
- strerror(errno));
+   error_errno(_("failed write to rev-list"));
err = -1;
break;
}
} while (!fn(cb_data, sha1));
 
-   if (close(rev_list.in)) {
-   error(_("failed to close rev-list's stdin: %s"), 
strerror(errno));
-   err = -1;
-   }
+   if (close(rev_list.in))
+   err = error_errno(_("failed to close rev-list's stdin"));
 
sigchain_pop(SIGPIPE);
return finish_command(_list) || err;
-- 
2.8.0.rc0.210.gd302cd2

--
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 20/41] copy.c: use error_errno()

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 copy.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/copy.c b/copy.c
index 574fa1f..4de6a11 100644
--- a/copy.c
+++ b/copy.c
@@ -42,15 +42,15 @@ int copy_file(const char *dst, const char *src, int mode)
status = copy_fd(fdi, fdo);
switch (status) {
case COPY_READ_ERROR:
-   error("copy-fd: read returned %s", strerror(errno));
+   error_errno("copy-fd: read returned");
break;
case COPY_WRITE_ERROR:
-   error("copy-fd: write returned %s", strerror(errno));
+   error_errno("copy-fd: write returned");
break;
}
close(fdi);
if (close(fdo) != 0)
-   return error("%s: close error: %s", dst, strerror(errno));
+   return error_errno("%s: close error", dst);
 
if (!status && adjust_shared_perm(dst))
return -1;
-- 
2.8.0.rc0.210.gd302cd2

--
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 10/41] builtin/pack-objects.c: use die_errno() and warning_errno()

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 builtin/pack-objects.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index a27de5b..1145747 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -828,8 +828,7 @@ static void write_pack_file(void)
 * to preserve this property.
 */
if (stat(pack_tmp_name, ) < 0) {
-   warning("failed to stat %s: %s",
-   pack_tmp_name, strerror(errno));
+   warning_errno("failed to stat %s", 
pack_tmp_name);
} else if (!last_mtime) {
last_mtime = st.st_mtime;
} else {
@@ -837,8 +836,7 @@ static void write_pack_file(void)
utb.actime = st.st_atime;
utb.modtime = --last_mtime;
if (utime(pack_tmp_name, ) < 0)
-   warning("failed utime() on %s: %s",
-   pack_tmp_name, strerror(errno));
+   warning_errno("failed utime() on %s", 
pack_tmp_name);
}
 
strbuf_addf(, "%s-", base_name);
-- 
2.8.0.rc0.210.gd302cd2

--
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 15/41] check-racy.c: use error_errno()

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 check-racy.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/check-racy.c b/check-racy.c
index 00d92a1..24b6542 100644
--- a/check-racy.c
+++ b/check-racy.c
@@ -12,7 +12,7 @@ int main(int ac, char **av)
struct stat st;
 
if (lstat(ce->name, )) {
-   error("lstat(%s): %s", ce->name, strerror(errno));
+   error_errno("lstat(%s)", ce->name);
continue;
}
 
-- 
2.8.0.rc0.210.gd302cd2

--
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 09/41] builtin/merge-file.c: use error_errno()

[Nguyễn Thái Ngọc Duy]

All these error() calls do not print error message previously, but
because when they are called, errno should be set. Use error_errno()
instead to give more information.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 builtin/merge-file.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/builtin/merge-file.c b/builtin/merge-file.c
index 5544705..13e22a2 100644
--- a/builtin/merge-file.c
+++ b/builtin/merge-file.c
@@ -62,8 +62,7 @@ int cmd_merge_file(int argc, const char **argv, const char 
*prefix)
usage_with_options(merge_file_usage, options);
if (quiet) {
if (!freopen("/dev/null", "w", stderr))
-   return error("failed to redirect stderr to /dev/null: "
-"%s", strerror(errno));
+   return error_errno("failed to redirect stderr to 
/dev/null");
}
 
if (prefix)
@@ -95,12 +94,13 @@ int cmd_merge_file(int argc, const char **argv, const char 
*prefix)
FILE *f = to_stdout ? stdout : fopen(fpath, "wb");
 
if (!f)
-   ret = error("Could not open %s for writing", filename);
+   ret = error_errno("Could not open %s for writing",
+ filename);
else if (result.size &&
 fwrite(result.ptr, result.size, 1, f) != 1)
-   ret = error("Could not write to %s", filename);
+   ret = error_errno("Could not write to %s", filename);
else if (fclose(f))
-   ret = error("Could not close %s", filename);
+   ret = error_errno("Could not close %s", filename);
free(result.ptr);
}
 
-- 
2.8.0.rc0.210.gd302cd2

--
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 08/41] builtin/mailsplit.c: use error_errno()

[Nguyễn Thái Ngọc Duy]

There's one change, in split_mbox(), where an error() without strerror()
as argument is converted to error_errno(). This is correct because the
previous call is fopen (not shown in the context lines), which should
set errno if it returns NULL.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 builtin/mailsplit.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/builtin/mailsplit.c b/builtin/mailsplit.c
index 104277a..4859ede 100644
--- a/builtin/mailsplit.c
+++ b/builtin/mailsplit.c
@@ -109,7 +109,7 @@ static int populate_maildir_list(struct string_list *list, 
const char *path)
if ((dir = opendir(name)) == NULL) {
if (errno == ENOENT)
continue;
-   error("cannot opendir %s (%s)", name, strerror(errno));
+   error_errno("cannot opendir %s", name);
goto out;
}
 
@@ -174,12 +174,12 @@ static int split_maildir(const char *maildir, const char 
*dir,
 
f = fopen(file, "r");
if (!f) {
-   error("cannot open mail %s (%s)", file, 
strerror(errno));
+   error_errno("cannot open mail %s", file);
goto out;
}
 
if (strbuf_getwholeline(, f, '\n')) {
-   error("cannot read mail %s (%s)", file, 
strerror(errno));
+   error_errno("cannot read mail %s", file);
goto out;
}
 
@@ -210,7 +210,7 @@ static int split_mbox(const char *file, const char *dir, 
int allow_bare,
int file_done = 0;
 
if (!f) {
-   error("cannot open mbox %s", file);
+   error_errno("cannot open mbox %s", file);
goto out;
}
 
@@ -318,7 +318,7 @@ int cmd_mailsplit(int argc, const char **argv, const char 
*prefix)
}
 
if (stat(arg, ) == -1) {
-   error("cannot stat %s (%s)", arg, strerror(errno));
+   error_errno("cannot stat %s", arg);
return 1;
}
 
-- 
2.8.0.rc0.210.gd302cd2

--
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 14/41] builtin/worktree.c: use error_errno()

[Nguyễn Thái Ngọc Duy]

While at there, improve the error message to say _what_ failed to
remove.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 builtin/worktree.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index d8e3795..331ecf6 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -110,7 +110,7 @@ static void prune_worktrees(void)
if (ret < 0 && errno == ENOTDIR)
ret = unlink(path.buf);
if (ret)
-   error(_("failed to remove: %s"), strerror(errno));
+   error_errno(_("failed to remove '%s'"), path.buf);
}
closedir(dir);
if (!show_only)
-- 
2.8.0.rc0.210.gd302cd2

--
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 07/41] builtin/help.c: use warning_errno()

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 builtin/help.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/builtin/help.c b/builtin/help.c
index 3c55ce4..8848013 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -127,7 +127,7 @@ static void exec_woman_emacs(const char *path, const char 
*page)
path = "emacsclient";
strbuf_addf(_page, "(woman \"%s\")", page);
execlp(path, "emacsclient", "-e", man_page.buf, (char *)NULL);
-   warning(_("failed to exec '%s': %s"), path, strerror(errno));
+   warning_errno(_("failed to exec '%s'"), path);
}
 }
 
@@ -148,7 +148,7 @@ static void exec_man_konqueror(const char *path, const char 
*page)
path = "kfmclient";
strbuf_addf(_page, "man:%s(1)", page);
execlp(path, filename, "newTab", man_page.buf, (char *)NULL);
-   warning(_("failed to exec '%s': %s"), path, strerror(errno));
+   warning_errno(_("failed to exec '%s'"), path);
}
 }
 
@@ -157,7 +157,7 @@ static void exec_man_man(const char *path, const char *page)
if (!path)
path = "man";
execlp(path, "man", page, (char *)NULL);
-   warning(_("failed to exec '%s': %s"), path, strerror(errno));
+   warning_errno(_("failed to exec '%s'"), path);
 }
 
 static void exec_man_cmd(const char *cmd, const char *page)
@@ -165,7 +165,7 @@ static void exec_man_cmd(const char *cmd, const char *page)
struct strbuf shell_cmd = STRBUF_INIT;
strbuf_addf(_cmd, "%s %s", cmd, page);
execl(SHELL_PATH, SHELL_PATH, "-c", shell_cmd.buf, (char *)NULL);
-   warning(_("failed to exec '%s': %s"), cmd, strerror(errno));
+   warning(_("failed to exec '%s'"), cmd);
 }
 
 static void add_man_viewer(const char *name)
-- 
2.8.0.rc0.210.gd302cd2

--
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 12/41] builtin/update-index.c: prefer "err" to "errno" in process_lstat_error

[Nguyễn Thái Ngọc Duy]

"errno" is already passed in as "err". Here we should use err instead of
errno. errno is probably a copy/paste mistake in e011054 (Teach
git-update-index about gitlinks - 2007-04-12)

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 builtin/update-index.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/update-index.c b/builtin/update-index.c
index 1c94ca5..b8b8522 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -255,7 +255,7 @@ static int process_lstat_error(const char *path, int err)
 {
if (err == ENOENT || err == ENOTDIR)
return remove_one_path(path);
-   return error("lstat(\"%s\"): %s", path, strerror(errno));
+   return error("lstat(\"%s\"): %s", path, strerror(err));
 }
 
 static int add_one_path(const struct cache_entry *old, const char *path, int 
len, struct stat *st)
-- 
2.8.0.rc0.210.gd302cd2

--
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 13/41] builtin/upload-archive.c: use error_errno()

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 builtin/upload-archive.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/builtin/upload-archive.c b/builtin/upload-archive.c
index dbfe14f..2caedf1 100644
--- a/builtin/upload-archive.c
+++ b/builtin/upload-archive.c
@@ -104,8 +104,7 @@ int cmd_upload_archive(int argc, const char **argv, const 
char *prefix)
pfd[1].events = POLLIN;
if (poll(pfd, 2, -1) < 0) {
if (errno != EINTR) {
-   error("poll failed resuming: %s",
- strerror(errno));
+   error_errno("poll failed resuming");
sleep(1);
}
continue;
-- 
2.8.0.rc0.210.gd302cd2

--
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 11/41] builtin/rm.c: use warning_errno()

[Nguyễn Thái Ngọc Duy]

While at there, improve the message a bit (what operation failed?) and
mark it for translation since the format string is now a sentence.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 builtin/rm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/rm.c b/builtin/rm.c
index 8829b09..fd47d20 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -152,7 +152,7 @@ static int check_local_mod(unsigned char *head, int 
index_only)
 
if (lstat(ce->name, ) < 0) {
if (errno != ENOENT && errno != ENOTDIR)
-   warning("'%s': %s", ce->name, strerror(errno));
+   warning_errno(_("failed to stat '%s'"), 
ce->name);
/* It already vanished from the working tree */
continue;
}
-- 
2.8.0.rc0.210.gd302cd2

--
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 04/41] builtin/am.c: use error_errno()

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 builtin/am.c | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index d003939..3dfe70b 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -769,15 +769,15 @@ static int split_mail_conv(mail_conv_fn fn, struct 
am_state *state,
in = fopen(*paths, "r");
 
if (!in)
-   return error(_("could not open '%s' for reading: %s"),
-   *paths, strerror(errno));
+   return error_errno(_("could not open '%s' for reading"),
+  *paths);
 
mail = mkpath("%s/%0*d", state->dir, state->prec, i + 1);
 
out = fopen(mail, "w");
if (!out)
-   return error(_("could not open '%s' for writing: %s"),
-   mail, strerror(errno));
+   return error_errno(_("could not open '%s' for writing"),
+  mail);
 
ret = fn(out, in, keep_cr);
 
@@ -857,8 +857,7 @@ static int split_mail_stgit_series(struct am_state *state, 
const char **paths,
 
fp = fopen(*paths, "r");
if (!fp)
-   return error(_("could not open '%s' for reading: %s"), *paths,
-   strerror(errno));
+   return error_errno(_("could not open '%s' for reading"), 
*paths);
 
while (!strbuf_getline_lf(, fp)) {
if (*sb.buf == '#')
-- 
2.8.0.rc0.210.gd302cd2

--
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 05/41] builtin/branch.c: use error_errno()

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 builtin/branch.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 0adba62..6f1572d 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -593,8 +593,7 @@ static int edit_branch_description(const char *branch_name)
branch_name, comment_line_char);
if (write_file_gently(git_path(edit_description), "%s", buf.buf)) {
strbuf_release();
-   return error(_("could not write branch description template: 
%s"),
-strerror(errno));
+   return error_errno(_("could not write branch description 
template"));
}
strbuf_reset();
if (launch_editor(git_path(edit_description), , NULL)) {
-- 
2.8.0.rc0.210.gd302cd2

--
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 03/41] bisect.c: use die_errno() and warning_errno()

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 bisect.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/bisect.c b/bisect.c
index 7996c29..6d93edb 100644
--- a/bisect.c
+++ b/bisect.c
@@ -860,8 +860,8 @@ static void check_good_are_ancestors_of_bad(const char 
*prefix, int no_checkout)
/* Create file BISECT_ANCESTORS_OK. */
fd = open(filename, O_CREAT | O_TRUNC | O_WRONLY, 0600);
if (fd < 0)
-   warning("could not create file '%s': %s",
-   filename, strerror(errno));
+   warning_errno("could not create file '%s'",
+ filename);
else
close(fd);
  done:
@@ -910,8 +910,7 @@ void read_bisect_terms(const char **read_bad, const char 
**read_good)
*read_good = "good";
return;
} else {
-   die("could not read file '%s': %s", filename,
-   strerror(errno));
+   die_errno("could not read file '%s'", filename);
}
} else {
strbuf_getline_lf(, fp);
-- 
2.8.0.rc0.210.gd302cd2

--
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 00/41] Add and use error_errno() and warning_errno()

[Nguyễn Thái Ngọc Duy]

Code changes compared to v2 is below (one incorrect conversion
reverted, two error message improvements). Other changes are text in
these commits, not shown as diff in this mail:

  [06/41] builtin/fetch.c: use error_errno()
  [08/41] builtin/mailsplit.c: use error_errno()
  [09/41] builtin/merge-file.c: use error_errno()
  [11/41] builtin/rm.c: use warning_errno()
  [14/41] builtin/worktree.c: use error_errno()
  [27/41] grep.c: use error_errno()

I didn't mention string format changes (e.g. from "abc (error)" to
"abc: error") in the commit messages though.

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index e18e190..1145747 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2018,7 +2018,7 @@ static void ll_find_deltas(struct object_entry **list, 
unsigned list_size,
ret = pthread_create([i].thread, NULL,
 threaded_find_deltas, [i]);
if (ret)
-   die_errno("unable to create thread");
+   die("unable to create thread: %s", strerror(ret));
active_threads++;
}
 
diff --git a/builtin/rm.c b/builtin/rm.c
index 13b9639..fd47d20 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -152,7 +152,7 @@ static int check_local_mod(unsigned char *head, int 
index_only)
 
if (lstat(ce->name, ) < 0) {
if (errno != ENOENT && errno != ENOTDIR)
-   warning_errno("'%s'", ce->name);
+   warning_errno(_("failed to stat '%s'"), 
ce->name);
/* It already vanished from the working tree */
continue;
}
diff --git a/grep.c b/grep.c
index 4f3779a..ec6f7ff 100644
--- a/grep.c
+++ b/grep.c
@@ -1732,7 +1732,7 @@ static int grep_source_load_file(struct grep_source *gs)
if (lstat(filename, ) < 0) {
err_ret:
if (errno != ENOENT)
-   error_errno("'%s'", filename);
+   error_errno(_("failed to stat '%s'"), filename);
return -1;
}
if (!S_ISREG(st.st_mode))

--
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 06/41] builtin/fetch.c: use error_errno()

[Nguyễn Thái Ngọc Duy]

A couple of newlines are also removed, because both error() and
error_errno() automatically append a newline.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 builtin/fetch.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index f8455bd..1582ca7 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -607,7 +607,7 @@ static int store_updated_refs(const char *raw_url, const 
char *remote_name,
 
fp = fopen(filename, "a");
if (!fp)
-   return error(_("cannot open %s: %s\n"), filename, 
strerror(errno));
+   return error_errno(_("cannot open %s"), filename);
 
if (raw_url)
url = transport_anonymize_url(raw_url);
@@ -848,7 +848,7 @@ static int truncate_fetch_head(void)
FILE *fp = fopen_for_writing(filename);
 
if (!fp)
-   return error(_("cannot open %s: %s\n"), filename, 
strerror(errno));
+   return error_errno(_("cannot open %s"), filename);
fclose(fp);
return 0;
 }
-- 
2.8.0.rc0.210.gd302cd2

--
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 02/41] usage.c: add warning_errno() and error_errno()

[Nguyễn Thái Ngọc Duy]

Similar to die_errno(), these functions will append strerror()
automatically.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 git-compat-util.h |  2 ++
 usage.c   | 21 +
 2 files changed, 23 insertions(+)

diff --git a/git-compat-util.h b/git-compat-util.h
index 1f8b5f3..49d4029 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -409,7 +409,9 @@ extern NORETURN void usagef(const char *err, ...) 
__attribute__((format (printf,
 extern NORETURN void die(const char *err, ...) __attribute__((format (printf, 
1, 2)));
 extern NORETURN void die_errno(const char *err, ...) __attribute__((format 
(printf, 1, 2)));
 extern int error(const char *err, ...) __attribute__((format (printf, 1, 2)));
+extern int error_errno(const char *err, ...) __attribute__((format (printf, 1, 
2)));
 extern void warning(const char *err, ...) __attribute__((format (printf, 1, 
2)));
+extern void warning_errno(const char *err, ...) __attribute__((format (printf, 
1, 2)));
 
 #ifndef NO_OPENSSL
 #ifdef APPLE_COMMON_CRYPTO
diff --git a/usage.c b/usage.c
index 8675d72..1dad03f 100644
--- a/usage.c
+++ b/usage.c
@@ -148,6 +148,17 @@ void NORETURN die_errno(const char *fmt, ...)
va_end(params);
 }
 
+int error_errno(const char *fmt, ...)
+{
+   char buf[1024];
+   va_list params;
+
+   va_start(params, fmt);
+   error_routine(fmt_with_err(buf, sizeof(buf), fmt), params);
+   va_end(params);
+   return -1;
+}
+
 #undef error
 int error(const char *err, ...)
 {
@@ -159,6 +170,16 @@ int error(const char *err, ...)
return -1;
 }
 
+void warning_errno(const char *warn, ...)
+{
+   char buf[1024];
+   va_list params;
+
+   va_start(params, warn);
+   warn_routine(fmt_with_err(buf, sizeof(buf), warn), params);
+   va_end(params);
+}
+
 void warning(const char *warn, ...)
 {
va_list params;
-- 
2.8.0.rc0.210.gd302cd2

--
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 01/41] usage.c: move format processing out of die_errno()

[Nguyễn Thái Ngọc Duy]

fmt_with_err() will be shared with the coming error_errno() and
warning_errno().

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 usage.c | 27 ---
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/usage.c b/usage.c
index 82ff131..8675d72 100644
--- a/usage.c
+++ b/usage.c
@@ -109,19 +109,11 @@ void NORETURN die(const char *err, ...)
va_end(params);
 }
 
-void NORETURN die_errno(const char *fmt, ...)
+static const char *fmt_with_err(char *buf, int n, const char *fmt)
 {
-   va_list params;
-   char fmt_with_err[1024];
char str_error[256], *err;
int i, j;
 
-   if (die_is_recursing()) {
-   fputs("fatal: recursion detected in die_errno handler\n",
-   stderr);
-   exit(128);
-   }
-
err = strerror(errno);
for (i = j = 0; err[i] && j < sizeof(str_error) - 1; ) {
if ((str_error[j++] = err[i++]) != '%')
@@ -136,10 +128,23 @@ void NORETURN die_errno(const char *fmt, ...)
}
}
str_error[j] = 0;
-   snprintf(fmt_with_err, sizeof(fmt_with_err), "%s: %s", fmt, str_error);
+   snprintf(buf, n, "%s: %s", fmt, str_error);
+   return buf;
+}
+
+void NORETURN die_errno(const char *fmt, ...)
+{
+   char buf[1024];
+   va_list params;
+
+   if (die_is_recursing()) {
+   fputs("fatal: recursion detected in die_errno handler\n",
+   stderr);
+   exit(128);
+   }
 
va_start(params, fmt);
-   die_routine(fmt_with_err, params);
+   die_routine(fmt_with_err(buf, sizeof(buf), fmt), params);
va_end(params);
 }
 
-- 
2.8.0.rc0.210.gd302cd2

--
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 1/2] bisect--helper: use OPT_CMDMODE instead of OPT_BOOL

[Pranit Bauva]

On Sun, May 8, 2016 at 12:34 PM, Johannes Schindelin
 wrote:
> Hi Pranit,
>
> On Fri, 6 May 2016, Pranit Bauva wrote:
>
>> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
>> index 3324229..d8de651 100644
>> --- a/builtin/bisect--helper.c
>> +++ b/builtin/bisect--helper.c
>> @@ -8,13 +8,17 @@ static const char * const git_bisect_helper_usage[] = {
>>   NULL
>>  };
>>
>> +enum subcommand {
>> + NEXT_ALL = 1
>> +};
>
> I still do not think that this enum needs to have file scope. Function
> scope is enough.

In the very initial patch I made it in function scope. To which you
pointed out[1] that in all other examples but for one have file scope
so then I thought maybe that exception was a wrong example and I
should stick to the convention of putting it in file scope. But now I
also realize that builtin/replace.c uses "cmdmode" instead of
"subcommand" so I am still wondering what would be the most
appropriate?

>>  int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
>>  {
>> - int next_all = 0;
>> + int subcommand = 0;
>
> Since subcommand is not simply an integer, but wants to take on the values
> defined in the enum above, the type should be changed accordingly. You
> could do it this way (short and sweet, with the appropriate scope):
>
> enum { NEXT_ALL = 1 } subcommand = 0;
>
> See https://github.com/git/git/blob/v2.8.2/builtin/replace.c#L423-L430 for
> an example (which uses "cmdmode" instead of "subcommand", too).
>
> Ciao,
> Dscho
[1]: http://article.gmane.org/gmane.comp.version-control.git/289653

Regards,
Pranit Bauva
--
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 1/2] bisect--helper: use OPT_CMDMODE instead of OPT_BOOL

[Johannes Schindelin]

Hi Pranit,

On Fri, 6 May 2016, Pranit Bauva wrote:

> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
> index 3324229..d8de651 100644
> --- a/builtin/bisect--helper.c
> +++ b/builtin/bisect--helper.c
> @@ -8,13 +8,17 @@ static const char * const git_bisect_helper_usage[] = {
>   NULL
>  };
>  
> +enum subcommand {
> + NEXT_ALL = 1
> +};

I still do not think that this enum needs to have file scope. Function
scope is enough.

>  int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
>  {
> - int next_all = 0;
> + int subcommand = 0;

Since subcommand is not simply an integer, but wants to take on the values
defined in the enum above, the type should be changed accordingly. You
could do it this way (short and sweet, with the appropriate scope):

enum { NEXT_ALL = 1 } subcommand = 0;

See https://github.com/git/git/blob/v2.8.2/builtin/replace.c#L423-L430 for
an example (which uses "cmdmode" instead of "subcommand", too).

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

Re: t6044 broken on pu

[Torsten Bögershausen]

On 08.05.16 04:21, Junio C Hamano wrote:
> Torsten Bögershausen  writes:
>
>> That's true, but the test passes anyway.
> You can also remove the body of the test and replace it with "true"
> and say "the test passes anyway".  Changing the test to use a file
> with only one line is irresponsible, if you do not know the nature
> of expected future bug that requires 10 lines to be there to
> manifest that the test wants to try.
>
> test_seq was invented exactly for the purpose of accomodating
> platforms that lack seq, so using it would probably be the best
> first step.  Updating implementation of test_seq to avoid $PERL
> would be a separate step, if desired (I personally do not think
> that is worth it).
We don't need to invoke perl, when the shell can do that internally ?

May a  simple
 printf "1\n2\n3\n4\n5\n6\n7\n8\n9\n10\n"

be an option ?


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

Re: [PATCH 1/2] bisect--helper: use OPT_CMDMODE instead of OPT_BOOL

[Johannes Schindelin]

Hi Chris,

On Wed, 4 May 2016, Christian Couder wrote:

> On Wed, May 4, 2016 at 4:56 PM, Johannes Schindelin
>  wrote:
> >
> > On Wed, 4 May 2016, Christian Couder wrote:
> >
> >> My intent was to try to show that there is some important value to make
> >> the subject close to the "low level" thing the patch actually does.
> >
> > I disagree. The place to describe low-level details that are not
> > immediately obvious from the patch is the commit message. Way, way down on
> > the bottom.
> >
> > The commit message should start with a subject that gives me a good clue
> > why this is a good change. A low-level detail won't do that very often.
> 
> Let me give you another example.
> 
> You want to do X and you discover that you need to tweek knob foo to
> do X, and also that tweeking knob foo is a good thing to do in general
> (for example it could be a refactoring that save some lines of code).
> So you create a patch that tweeks knob foo and call it "prepare to do
> X", and then send it so that it can be merged. Then you want to do X
> but you are interupted by an event, for example you boss tells you to
> do Y instead of X right away because of an urgent business need. So
> you do Y instead of X. And then sometimes later (it could be weeks,
> months or years later), when you are finished with Y, you now want to
> go back to your previous work on X. At that time you discover that you
> need to tweek knob bar (it could be another refactoring of another
> part of the code), but you forgot about your previous patch that
> tweeked knob foo, so you call your new patch that tweeks knob bar
> "prepare to do X". As your previous patch that was also called
> "prepare to do X" has already been integrated, you now have too
> patches that do different things that are called the same, and you can
> easily mistake the first one for the second one.
> 
> If you had just called your first patch "tweek knob foo" and the
> second patch "tweek knob bar", it would be much clearer which patch
> does what.

And then, six months later, when *you* are stuck in a plane and bisect a
problem down to my "tweak knob foo", you curse at me, asking fellow
passengers "what was Dscho thinking? 'foo'? What the hell is 'foo'
supposed to mean? And what was the *intention* of this patch? There is a
bug! And now I cannot fix it properly because I do not understand what the
patch was intended to do!".

Do not believe even one second this scenario s foreign to *me*. Even with
some of git.git's commits whose messages were so in love with low-level
details, essentially repeating what the diff already told me, that they
forgot to include the context, the big picture, leaving me puzzled and
frustrated.

And my fellow passengers, too.

This puzzlement and frustration is unnecessary. I typically follow Peff's
examples. They typically do a good job of telling me right away what the
big picture is, even if they are in the middle of a patch series, not
worrying too much whether adjacent commits say related things, without
forgetting low-level details. The crucial point is that they start out by
assuming little familiarity of the reader with the context.

In short: you can have both. The commit message *can* be informative, and
*still* contain low-level details. You simply start with the former and
ease the reader into the latter, instead of skipping the first part, is
all.

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

Re: [PATCH 80/83] run-command: make dup_devnull() non static

[Johannes Schindelin]

Hi Chris,

On Sat, 7 May 2016, Christian Couder wrote:

> On Sat, May 7, 2016 at 2:13 PM, Johannes Schindelin
>  wrote:
> >
> > On Sat, 7 May 2016, Christian Couder wrote:
> >
> >> On Fri, May 6, 2016 at 5:34 PM, Johannes Schindelin
> >>  wrote:
> >> >
> >> > No, you should change the code that requires that ugly dup()ing so
> >> > that it can be configured to shut up.
> >>
> >> After taking a look, it looks like a routine that does nothing could
> >> be passed to set_error_routine() and that could do part of the trick.
> >>
> >> This part might not be too ugly, but it would anyway be more complex,
> >> less close to what the code is doing now and more error prone, as one
> >> also need to make sure that for example no warning() or
> >> fprintf(stderr, ...) are called and nothing is printed on stdout.
> >
> > I am afraid that you *have* to do that, though, if you truly want to
> > libify the code.
> >
> > Of course you can go with really ugly workarounds instead. Something
> > like a global flag that die() and error() and warning() respect. It
> > would incur some technical debt, but it would make your life easier in
> > the short run.
> >
> > Both the real solution and the workaround would be better than the
> > current version of the patches that dup() back and forth, just to
> > avoid addressing the real problem.
> 
> The code that is now in master in builtin/am.c does:
> 
> if (state->threeway && !index_file) {
> cp.no_stdout = 1;
> cp.no_stderr = 1;
> }
> 
> and in run-command.c there is already:
> 
> if (cmd->no_stdout)
> dup_devnull(1);
> [...]
> if (cmd->no_stderr)
> dup_devnull(2);

Of course it does that. Because there is no other way, that's why: you
cannot change the code that is spawned by start_command().

> for Linux and the following for Windows:
> 
> if (cmd->no_stderr)
> fherr = open("/dev/null", O_RDWR);
> [...]
> if (cmd->no_stdout)
> fhout = open("/dev/null", O_RDWR);

And it is very well contained on Windows. No other callers. The code
is limited to run_command.c.

> so the current code is already using dup_devnull() for Linux that you
> don't want me to use, and it looks like there is already a simple way
> to do that on Windows.

The difference between the code in master and what your patches try to do
is that in the latter case, you want to dup() just for a while, to shut up
a code path that is not only known very well, but our very own code that
is easily changed, only to dup() it *back* in the end.

The claim is that this libifies the procedure. But it makes the code
really nasty for use as a library: if this is run in a thread (and you
know that we are going to have to do this in the near future, for
performance reasons), it will completely mess up all the other threads
because it messes with the global file descriptors.

And that is why it is ugly: it incurs an enormous technical debt that will
make code changes substantially more complicated down the road.

In essence, you save yourself a little time by sloppily dup()ing back and
forth. At the expense of making the life much harder for the developer who
needs to use your code as a library function.

And actually, hiding even fatal errors might be an ugly side effect of the
current implementation, an unfortunate implementation detail, really, not
something we want to preserve when libifying the code. And actually,
dup()ing the *caller's* stdout is not exactly preserving the current
behavior that dup()s the *called process'* stdout.

So yes, I find the proposed patch inelegant.

If others are okay with this, I will shut up. But I have to point out that
it is ugly code, plain and simple, that silences an entire global file
descriptor, just temporarily, only to avoid a careful set of patches that
introduces a silent mode to the library functions that need to be called,
which might even facilitate other libifying efforts.

I hope you do not take this as a personal attack. It is not intended as
such. It is intended to help end up with the best possible code quality.

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

Re: [PATCH v5 0/2] bisect--helper: rewrite of check-term-format()

[Pranit Bauva]

On Sun, May 8, 2016 at 7:55 AM, Junio C Hamano  wrote:
> Pranit Bauva  writes:
>
>> I completely missed your point and you want me to go the Eric Sunshine's way?
>
> I am neutral.
>
> When I read your response to Eric's "top down" suggestion, I didn't
> quite get much more than "I started going this way, and I do not
> want to change to the other direction.", which was what I had the
> most trouble with.  If your justification for the approach to start
> from building a tiny bottom layer that will need to be dismantled
> soon and repeat that (which sounds somewhat wasteful) were more
> convincing, I may have felt differently.

Sorry if it seemed that "I have done quite some work and I don't want
to scrape it off and redo everything". This isn't a case for me. I
think of this as just a small part in the process of learning and my
efforts would be completely wasted as I can still reuse the methods I
wrote. This is still open for a "philosophical" discussion. I am
assuming 1e1ea69fa4e is how Eric is suggesting.

Why I think its better to go the subcommand way:
 * I can test **C implementation** of the function to verify whether
it was done in a proper way. By using a "top down" approach, I can
make the test suite passing by running the code from git-bisect.sh not
the re-written C code.
 * I have made a very few minor mistakes in conversion of
check_term_format() which just messed up my head (I actually spent 3
days before Christian pointed out that '!' was missing). Imagine
debugging the complete git-bisect.sh to find a very small error after
a complete implementation.
 * Using subcommands, I can easily verify whether I have done it the
right way or not.
 * It makes the review process very simple. The reviewers/testers can
verify that my method is working correctly and well computers can
detect errors better than humans.
 * I can convert small functions which can be reviewed easily rather
than dumping a big series.

What I think is that the bottom up approach will make life easier for
the me and for reviewer. Yes, it does make the code "unclean" for the
time being and if it is between releases then all the more painful.
Well, the latter part can be avoided by keeping it in next.

Please point out if I am mistaken about anything.

Regards,
Pranit Bauva
--
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