Re: [PATCH v2] configure.ac: move the private git m4 macros to a dedicated directory

2013-09-11 Thread Stefano Lattarini

Hi Elia.  Sorry, but I have to give my NAK to this patch.

On 09/11/2013 04:46 PM, Elia Pinto wrote:

Git use, as many project that use autoconf, private m4 macros.

When not using automake, and just relying on autoconf, the macro
files are not picked up by default.

A possibility, as git do today, is to put the private m4 macro
in the configure.ac file, so they will copied over the final configure
when calling autoreconf(that call also autoconf).
But this makes configure.ac difficult to read and maintain,
especially if you want to introduce new macros later. By separating
the definitions of the macros from configure.ac file the build system
would be more modular.


In which sense are we being more modular exactly?  After all:

  - the configure.ac of Git is the only user of these macros,

  - using m4_include doesn't offer any performance improvement, and

  - m4 doesn't offer any namespace granularity anyway.

So it seems to me that this patch only adds extra indirections without
adding any real benefit.


Starting from version 2.58, autoconf provide the macro AC_CONFIG_MACRO_DIR
to declare where additional macro files are to be put and found by aclocal.
The argument passed to this macro is commonly m4. Despite the documentation,
autoconf do nothing with it, only aclocal can use directly if invoked by
-I m4 or indirectly using automake. But autoreconf don't invoke aclocal
in this way. So in summary you can not use this macro in a useful
way if you only use autoconf, as git does.

Another historical possibility is to list all your macros in acinclude.m4.
This file will be included in aclocal.m4 when you run aclocal, and its macro(s)
will henceforth be visible to autoconf. However if it contains numerous macros,
it will rapidly become difficult to maintain, and for git this don't provide
any benefits or very little.

The actual autotool documentation recommend to write each
macro in its own file and gather all these files in a separate directory.


Where exactly id you find that recommendation?  If the autotools docs tell
to do so *unconditionally*, they are wrong and should be fixed.  In fact,
even the configure.ac from Automake itself keeps definition of private
macros in configure.ac...


Given the limitations i mentioned earlier, the only possibility is to use the 
m4_include
for including every macro file. The m4_include directive works quite like the
#include directive of the C programming language, and simply copies over the 
content
of the file(s).

Signed-off-by: Elia Pinto gitter.spi...@gmail.com
---
This is a second version of this patch 
http://article.gmane.org/gmane.comp.version-control.git/231984.
The first was plain wrong, my bad. I am sorry for the long delay.
Sure it is something low-hanging fruit


  configure.ac  |  148 +++--
  m4/git_arg_set_path.m4|   14 
  m4/git_check_func.m4  |   13 
  m4/git_conf_append_path.m4|   30 
  m4/git_conf_subst.m4  |   10 +++
  m4/git_conf_subst_init.m4 |   15 
  m4/git_parse_with.m4  |   22 ++
  m4/git_parse_with_set_make_var.m4 |   20 +
  m4/git_stash_flags.m4 |   15 
  m4/git_unstash_flags.m4   |   13 
  10 files changed, 162 insertions(+), 138 deletions(-)
  create mode 100644 m4/git_arg_set_path.m4
  create mode 100644 m4/git_check_func.m4
  create mode 100644 m4/git_conf_append_path.m4
  create mode 100644 m4/git_conf_subst.m4
  create mode 100644 m4/git_conf_subst_init.m4
  create mode 100644 m4/git_parse_with.m4
  create mode 100644 m4/git_parse_with_set_make_var.m4
  create mode 100644 m4/git_stash_flags.m4
  create mode 100644 m4/git_unstash_flags.m4


 [SNIP]


Regards,
  Stefano
--
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] Allow git-filter-branch to process large repositories with lots of branches.

2013-09-07 Thread Stefano Lattarini

On 07/09/13 22:03, Lee Carver wrote:

As noted in several forums, a recommended way to move trees between
repositories
is to use git-filter-branch to revise the history for a single tree:

http://gbayer.com/development/moving-files-from-one-git-repository-to-anoth
er-preserving-history/
http://stackoverflow.com/questions/1365541/how-to-move-files-from-one-git-r
epo-to-another-not-a-clone-preserving-history

However, this can lead to argument list too long errors when the original
repository has many retained branches (6k)

/usr/local/git/libexec/git-core/git-filter-branch: line 270:
/usr/local/git/libexec/git-core/git: Argument list too long
Could not get the commits

Piping the saved output from git rev-parse into git rev-list avoids this
problem, since the rev-parse output is not processed as a command line
argument.
---
  git-filter-branch.sh | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index ac2a005..60d239b 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -255,7 +255,7 @@ else
remap_to_ancestor=t
  fi

-rev_args=$(git rev-parse --revs-only $@)
+git rev-parse --revs-only $@  ../parse

  case $filter_subdir in
  )
@@ -267,8 +267,9 @@ case $filter_subdir in
;;
  esac

+cat ../parse | \
  git rev-list --reverse --topo-order --default HEAD \
-   --parents --simplify-merges $rev_args $@  ../revs ||
+   --parents --simplify-merges --stdin $@  ../revs ||


Useless use of cat IMO.  I'd suggest using a redirection instead:

  git rev-list --reverse --topo-order --default HEAD \
-   --parents --simplify-merges $rev_args $@  ../revs ||
+   --parents --simplify-merges --stdin $@  ../revs  ../parse ||


die Could not get the commits
  commits=$(wc -l ../revs | tr -d  )




Regards,
  Stefano
--
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] tag: Use OPT_BOOL instead of OPT_BOOLEAN to allow one action multiple times

2013-08-17 Thread Stefano Lattarini

(Going through old mail today, sorry for the late reply)

On 08/01/2013 12:10 AM, Junio C Hamano wrote: Stefan Beller 
stefanbel...@googlemail.com writes:


 On 07/31/13 00:28, Junio C Hamano wrote:

 we could just do

 #define OPT_CMDMODE(s, l, v, h) \
  { OPTION_CMDMODE, (s), (l), (v), NULL, \
(h), PARSE_OPT_NOARG|PARSE_OPT_NONEG, NULL, (s) }


 I agree that's a better proposal than mine.

 By the way, I haven't convinced myself that it is a good idea in
 general to encourage more use of command mode options, so I am a bit
 reluctant to add this before knowing which direction in the longer
 term we are going.

   - Some large-ish Git subcommands, like git submodule, use the
 mode word (e.g. git submodule status) to specify the operation
 mode (youe could consider status a subsubcommand that
 submodule subcommand takes).  These commands typically began
 their life from day one with the mode words.

   - On the other hand, many Git subcommands, like git tag, have
 the primary operation mode (e.g. create a new one is the
 primary operation mode for git tag), and use command mode
 options to specify other operation modes (e.g. --delete).
 These commands started as single purpose commands (i.e. to
 perform their primary operation) but have organically grown
 over time and acquired command mode options to invoke their
 secondary operations.

 As an end user, you need to learn which style each command takes,
 which is an unnecessary burden at the UI level.  In the longer term,
 we may want to consider picking a single style, and migrating
 everybody to it.  If I have to vote today, I would say we should
 teach git submodule to also take command mode options (e.g. git
 submodule --status will be understood the same way as git
 submodule status), make them issue warnings when mode words are
 used and encourage users to use command mode options instead, and
 optionally remove the support of mode words at a large version bump
 like 3.0.

 One clear advantage mode words have over command mode options is
 that there is no room for end user confusion.  The first word after
 git subcmd is the mode word, and you will not even dream of asking
 what would 'git submodule add del foo' do? as it is nonsensical.
 The command mode options, on the other hand, gives too much useless
 flexibility to ask for nonsense, e.g. git tag --delete --verify,
 git tag --no-delete --delete, etc., and extra code needs to detect
 and reject combinations.  But commands that took mode options cannot
 be easily migrated to take mode words without hurting existing users
 and scripts (e.g. git tag delete master can never be a request to
 delete the tag 'master', as it is a request to create a tag whose
 name is 'delete' that points at the same object as 'master' points
 at).

Why not encourage the use of a standardized '--action' option instead?
This can work with lesser compatibility headaches for both the commands
taking mode options and the commands taking mode words:

  git submodule init   becomes  git submodule --action=init
  git tag --delete TAG becomes  git tag --action=delete TAGNAME

Commands that have a primary operation mode can keep the use
of --action optional, while commands that have no such sensible
primary mode can make it mandatory (again, this gives more compatibility 
with the existing syntax).  And the old syntax

can be supported in parallel to the new one for a potentially
indefinite time; albeit, if I were to propose a timetable, I'd
got for:

  - Git 2.0: the new syntax is introduced
  - Git 2.x: any use the old syntax starts to trigger warnings
(which can be silenced with a config option)
  - Git 3.0: any use the old syntax starts to trigger fatal
errors (which can be turned into mandatory warnings with
a config option)
  - Git 4.0: any use the old syntax starts to trigger
mandatory fatal errors.
  - Git 4.x: Remove any handling of the old syntax.

Just my 2 cents,
  Stefano

--
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: New special handing of '@' character broke my use case

2013-08-15 Thread Stefano Lattarini

On 08/14/2013 09:57 PM, Junio C Hamano wrote:
 Johannes Sixt j...@kdbg.org writes:

 [SNIP]

 Stefano's use-case, where @/foo is turned into HEAD/foo,
 indicates a bug.

 In my opinion, the topic, which touches a central part of ref
 handling, was a bit hurried (and this report is a symptom of it), and
 I wouldn't mind seeing it reverted.

 Thanks; you said it much better than I did.  I think the short-hand
 is not a bad idea by itself, but the execution may need to be redone
 a bit more carefully, and it is prudent to revert it from the
 upcoming release.

Thanks both for taking care of this issue with the usual speed and
clarity.

Best regards,
  Stefano
--
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


New special handing of '@' character broke my use case

2013-08-14 Thread Stefano Lattarini

Hello list.

In the last year or so, I developed a personal idiom of using a
naming scheme of @/BRANCH-NAME/NUMto save the older versions
of branches I'm going to rebase (interactively or not).  Here is an
idealized example of my use case:

  $ git co --help
  `git co' is aliased to `checkout'
  $ git co -b topic
  $ ... hack hack hack ...
  $ git co master  git pull # Let's say this brings in 100 commits
  $ git co topic # I want to rebase this to avoid merge conflicts later
  $ git branch @/topic/1 # So I save original version of the branch
  $ git rebase master # Do the rebase, solve conflicts etc.
  $ ... hack hack hack (20 commits) ...
  # Here I notice botched commits messages and badly-ordered
  # commits in the last 10 commits or so.  I want to fix that.
  $ git branch @/topic/2 # Save latest version of the branch
  $ git rebase -i HEAD~12 # Fix issues

My problems is that some new automagical interpretation of the bare '@' 
character (introduced after 1.8.3) has destroyed my use case:


  $ /usr/bin/git --version
  git version 1.8.3
  $ git --version
  git version 1.8.4.rc2
  $ git co master
  $ /usr/bin/git branch @/foo# Old git
  $ git branch
@/foo   # -- good
  * master
  $ git branch   # New git.
@/foo# -- good
HEAD/bar # -- BAD!
  * master

I don't want to ask you to revert this new behaviour, but I'd like to
at least have an option to disable it.  Even better if such an option is 
already present -- in which case, could you just point me at it?


Failing that, I can re-train myself to use another character (like '='
or '+') to take the same role '@' has hold for me until today, no big
deal.  But in that case, I'd like some assurance that such a character 
is not going to be turned into a magical character some time in the 
future ;-)


Thanks,
  Stefano

--
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: New special handing of '@' character broke my use case

2013-08-14 Thread Stefano Lattarini

[re-sending to the list, sorry Junio for the duplicate mail]

On 08/14/2013 07:05 PM, Junio C Hamano wrote:
 Stefano Lattarini stefano.lattar...@gmail.com writes:

 My problems is that some new automagical interpretation of the bare
 @' character (introduced after 1.8.3) has destroyed my use case:
 ...
 I don't want to ask you to revert this new behaviour, but I'd like to
 at least have an option to disable it.

 I do not think it is simply not worth the complexity to selectively
 disable it.  If it is a regression, it is much better to simply
 revert, if we can (it appears that cdfd9483 (Add new @ shortcut for
 HEAD, 2013-05-07) can be reverted without any textual context, but
 there may already be new stuff that depends on the @).

I'm not sure I want to force a revert simply to cater to a personal 
idiom of mine (and not that important).  I'd first like to hear what the 
community thinks about the issue (with silence meaning don't revert).


 For the upcoming release, I am very much tempted to revert it and
 let the topic retried, by people who really want the let's save
 four keystrokes and replace it with @ aka Shift-something,
 without hurting your use case (and others), after the upcoming
 release.

 What do others think?

I second this question :-)

Thanks,
  Stefano

--
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] configure: fix help screen

2013-06-28 Thread Stefano Lattarini
The configure option to disable threading is '--disable-pthreads',
not '--without-pthreads'.

Signed-off-by: Stefano Lattarini stefano.lattar...@gmail.com
---
 configure.ac | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index f3462d9..2f43393 100644
--- a/configure.ac
+++ b/configure.ac
@@ -193,7 +193,7 @@ AC_ARG_ENABLE([pthreads],
   [FLAGS is the value to pass to the compiler to enable POSIX Threads.]
   [The default if FLAGS is not specified is to try first -pthread]
   [and then -lpthread.]
-  [--without-pthreads will disable threading.])],
+  [--disable-pthreads will disable threading.])],
 [
 if test x$enableval = xyes; then
AC_MSG_NOTICE([Will try -pthread then -lpthread to enable POSIX Threads])
-- 
1.8.3.1.605.g85318f5

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


Bad attitudes and problems in the Git community (was: Re: [PATCH 2/2] Move sequencer to builtin)

2013-06-10 Thread Stefano Lattarini
On 06/10/2013 07:15 AM, Felipe Contreras wrote:
 On Sun, Jun 9, 2013 at 6:40 PM, Stefano Lattarini
 stefano.lattar...@gmail.com wrote:
 
   I do accuse Felipe's *attitude* to bring on and nourish such
   unpleasantness toxicity.  His technical merits and the possible
   qualities of his patches do *nothing* to remove or quell such
   issues.
 
 How convenient to accuse me

I accuse your *attitude*, which IMHO is now even damaging your
code (that is, the acceptance of your code on the part of the
community).

 and not the others who have as much fault if not more.

Sorry, this is just your opinion.  Mine, by observing the proceeding
from the outside, is different: my opinion is that your attitude is
the problem.

 You need two sides to have an argument.

I disagree.  Unless you mean than, whenever a part behaves in a
hostile and aggressive way, the other part should just silently
knuckle under.

 The difference is; I did actually send code. Code that is good, code
 that works, and code that users need.
 
As I said, as a *user* (since I'm definitely not a Git developer
in any sense of the word), I also need a calm and constructive
environment in the community.  Or are you interested only in
users that can benefit from things you are good at?

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


Re: [PATCH 2/2] Move sequencer to builtin

2013-06-09 Thread Stefano Lattarini
[Sorry for the full quote, but sometimes, repetita iuvant]

On 06/09/2013 11:42 PM, Michael Haggerty wrote:
 On 06/09/2013 09:11 PM, Johan Herland wrote:
 [...]
 FWIW, I'd like to express my support for the opinions expressed by
 Jonathan, Jeff and Thomas. They accurately describe my impression of
 these discussion threads.
 
 I also agree.  In my opinion, Felipe, your abrasiveness, your disregard
 of project standards, and your eternal argumentativeness outweigh the
 benefit of your contributions, large though they may be.
 
 Writing code is only a small part of keeping the Git project going.
 
 * Reviewing code is an essential, more thankless, and therefore more
 precious, contribution.  Therefore the Git project has standards to make
 code review less unpleasant and more effective; for example: (1) patches
 shouldn't cause regressions; (2) commit messages have to be written to
 very high standards; (3) reviewers' comments should be accepted
 gratefully and taken very seriously.  Almost everybody in the Git
 community accepts these standards.  Felipe, you do not seem to.  The
 result is that reviewers' time and goodwill are wasted, and they
 justifiably feel unvalued.  We can't afford to misuse reviewers; they
 are the bedrock (and the bottleneck) of the project.
 
 * Gaining and keeping contributors is important to maintaining the
 success of the project.  The mailing list is the main forum for the
 development community; therefore, it is important that the mailing list
 be a place where people display a high degree of technical excellence,
 but also respect for one another, friendliness (or at least a lack of
 hostility), and discussions that do turn into flame wars.  It is
 possible to have a profound technical disagreement without losing
 respect for the other side; contrariwise it is NOT acceptable to twist a
 technical disagreement into a personal attack, even by the slightest
 insinuation.  Felipe, in my opinion your participation in the mailing
 list lowers the tone dramatically, and will result in loss of other
 contributors and the failure to attract new contributors.
 
 Felipe, I wish that you would devote a small fraction of your prodigious
 energy to the very difficult challenge of feeling empathy,
 understanding, and respect for the other members of the community.  But
 if things continue the way they have, I personally would, with sadness
 in my heart, prefer to forgo your patches in exchange for the more
 important benefit of a more collegial (and therefore overall more
 productive and sustainable) community.
 
 Michael
 
FWIW, from the meager but I hope not utterly irrelevant point
of view of a non-contrib-but-not-clueless user as I am:

  *a complete and hear-felt +1 on what Michael said here*

Until a couple of months ago, skimming this list was mostly a real
pleasure, and would often give me some valuable insight on the
upcoming features/incompatibilities of Git, help me organize my own
workflow as a Git user, and also steadily improve my understanding
and command of netiquette in both generic mailing lists and Open
Source and/or Free Software communities.

Now, when I open my mail and get to the git folder, I more and
more end up asking myself:

  1. What kind of flame am I going to have to see today?; and

  2. How much chaff will I have to navigate through to finally
  to get to interesting stuff (if any is actually left)?

*To reiterate:*

Sadly, the environment of the Git mailing list has been steadily
and slowly *sinking* -- sinking from being pleasant and useful
and even educational, into being annoying and frustrating and
often somewhat toxic.  I usually jeer and despise he who makes
public accusations by simply adding his voice to the disapproval
of the community, but this time, I feel compelled to do exactly
that:

  I do accuse Felipe's *attitude* to bring on and nourish such
  unpleasantness toxicity.  His technical merits and the possible
  qualities of his patches do *nothing* to remove or quell such
  issues.

Sorry for the extra potential controversy, but sometimes one has
to speak up,

  Stefano
--
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: *** glibc detected *** git: double free or corruption (fasttop): 0x0000000001fab820 ***

2013-06-04 Thread Stefano Lattarini
On 06/04/2013 05:46 PM, Jason Gross wrote:
 I get *** glibc detected *** git: double free or corruption
 (fasttop): 0x01fab820 *** reliably on the following set of
 commands.  I'm working on a remote machine where I don't have
 administrative privileges, so I can't update from git 1.7.2.5 to a
 newer version.

Sure you can; install from a release tarball.  The page
http://git-scm.com/downloads links the latest version
(1.8.3) as well as older ones.

As for the issue you reported, I'll leave that to the real git
developers (I'm only a lurker here).

Regards,
  Stefano
--
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 2/3] unpack-trees: plug a memory leak

2013-05-30 Thread Stefano Lattarini
On 05/30/2013 03:34 PM, Felipe Contreras wrote:
 Before overwriting the destination index, first let's discard it's

s/it's/its/

 contents.

 [SNIP]

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


Re: [RFC/PATCH v2 4/8] rebase: cherry-pick: fix abort of cherry mode

2013-05-29 Thread Stefano Lattarini
On 05/29/2013 06:16 AM, Felipe Contreras wrote:
 Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
 ---
  git-rebase.sh | 1 +
  1 file changed, 1 insertion(+)
 
 diff --git a/git-rebase.sh b/git-rebase.sh
 index 76900a0..9b5d78b 100755
 --- a/git-rebase.sh
 +++ b/git-rebase.sh
 @@ -335,6 +335,7 @@ skip)
   run_specific_rebase
   ;;
  abort)
 + test $type == cherrypick  git cherry-pick --abort

Bashism; please use =, not ==.

   git rerere clear
   read_basic_state
   case $head_name in

Regards,
  Stefano
--
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] fast-import: Remove redundant assignment of 'oe' to itself.

2013-05-26 Thread Stefano Lattarini
On 05/26/2013 10:05 PM, Stefan Beller wrote:
 Reported by cppcheck.
 
 Signed-off-by: Stefan Beller stefanbel...@googlemail.com
 ---
  fast-import.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/fast-import.c b/fast-import.c
 index 5f539d7..0142e3a 100644
 --- a/fast-import.c
 +++ b/fast-import.c
 @@ -2914,7 +2914,7 @@ static void cat_blob(struct object_entry *oe, unsigned 
 char sha1[20])
  static void parse_cat_blob(void)
  {
   const char *p;
 - struct object_entry *oe = oe;

This was done on purpose, to avoid spurious warnings with (at least)
some versions of GCC.

 + struct object_entry *oe;
   unsigned char sha1[20];
  
   /* cat-blob SP object LF */

Regards,
  Stefano
--
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] make color.ui default to 'auto'

2013-05-15 Thread Stefano Lattarini
On 05/15/2013 02:09 PM, Matthieu Moy wrote:
 Most users seem to like having colors enabled, and colors can help
 beginners to understand the output of some commands (e.g. notice
 immediately the boundary between commits in the output of git log).
 
 Many tutorials tell the users to set color.ui=auto as a very first step.
 These tutorials would benefit from skiping

s/skiping/skipping/

 this step and starting the
 real Git manipualtions earlier.

s/manipualtions/manipulations/

 Other beginners do not know about
 color.ui=auto, and may not discover it by themselves, hence live with
 blackwhite outputs while they may have prefered colors.

s/prefered/preferred/

 A few people (e.g. color-blind) prefer having no colors, but they can
 easily set color.ui=never for this (and googling disable colors in git
 already tells them how to do so).
 
 A transition period with Git emitting a warning when color.ui is unset
 would be possible, but the discomfort of having the warning seems
 superior to the benefit: users may be surprised by the change, but not
 harmed by it.
 
 The default value is changed, and the documentation is reworded to
 mention color.ui=false first, since the primary use of color.ui after
 this change is to disable colors, not to enable it.
 
 Signed-off-by: Matthieu Moy matthieu@imag.fr
 ---
 I'd love to see this by default, yes. Maybe a 2.0 change?

 If people agree that this is a good change, would we need a transition
 plan? I'd say no, as there is no real backward incompatibility involved.
 People who dislike colors can already set color.ui=false, and seeing
 colors can hardly harm them, just temporarily reduce the comfort for
 them.

 I vote for this. It's the first thing I do in any setup, even the ones
 that are note mine. I've also seen it in basically all the tutorials,
 even before setting user.name/email.

 I also don't see the point of a transition plan.
 
 OK, then let's try turning the discussion into code.
 
 I'm starting to wonder why we didn't do this earlier ;-).
 
  Documentation/config.txt | 11 ++-
  color.c  |  2 +-
  2 files changed, 7 insertions(+), 6 deletions(-)
 
 diff --git a/Documentation/config.txt b/Documentation/config.txt
 index 1009bfc..97550be 100644
 --- a/Documentation/config.txt
 +++ b/Documentation/config.txt
 @@ -913,11 +913,12 @@ color.ui::
   as `color.diff` and `color.grep` that control the use of color
   per command family. Its scope will expand as more commands learn
   configuration to set a default for the `--color` option.  Set it
 - to `always` if you want all output not intended for machine
 - consumption to use color, to `true` or `auto` if you want such
 - output to use color when written to the terminal, or to `false` or
 - `never` if you prefer Git commands not to use color unless enabled
 - explicitly with some other configuration or the `--color` option.
 + to `false` or `never` if you prefer Git commands not to use
 + color unless enabled explicitly with some other configuration
 + or the `--color` option. Set it to `always` if you want all
 + output not intended for machine consumption to use color, to
 + `true` or `auto` (this is the default since Git 2.0) if you
 + want such output to use color when written to the terminal.
  
  column.ui::
   Specify whether supported commands should output in columns.
 diff --git a/color.c b/color.c
 index e8e2681..f672885 100644
 --- a/color.c
 +++ b/color.c
 @@ -1,7 +1,7 @@
  #include cache.h
  #include color.h
  
 -static int git_use_color_default = 0;
 +static int git_use_color_default = GIT_COLOR_AUTO;
  int color_stdout_is_tty = -1;
  
  /*

With the typos above fixed:

  Reviewed and supported-by: Stefano Lattarini stefano.lattar...@gmail.com

Thanks,
  Stefano
--
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 2/2] remote-bzr: avoid bad refs

2013-05-04 Thread Stefano Lattarini
On 05/04/2013 02:31 AM, Felipe Contreras wrote:
 Turns out fast-export throws bad 'reset' commands because of a behavior
 in transport-helper that is not even needed.
 
 We should ignore them, otherwise we will threat

s/threat/treat/

 them as branches and fail.
 
 This was fixed in v1.8.2, but some people use this script in older
 versions of git.
 
 Also, check if the ref was a tag, and skip it for now.

Regards,
  Stefano
--
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/9] remote-bzr: trivial cleanups

2013-04-25 Thread Stefano Lattarini
On 04/25/2013 08:19 PM, Ramkumar Ramachandra wrote:
 Felipe Contreras wrote:
 diff --git a/contrib/remote-helpers/git-remote-bzr 
 b/contrib/remote-helpers/git-remote-bzr
 index aa7bc97..82bf7c7 100755
 --- a/contrib/remote-helpers/git-remote-bzr
 +++ b/contrib/remote-helpers/git-remote-bzr
 @@ -94,7 +94,7 @@ class Marks:
  return self.last_mark

  def is_marked(self, rev):
 -return self.marks.has_key(rev)
 +return rev in self.marks
 
 Why?  Is the new form faster than the older one?

I think the has_key() method is deprecated in modern python,
and the 'key in dict' usage is more idiomatic.

 @@ -224,7 +224,7 @@ def export_files(tree, files):
  else:
  mode = '100644'

 -# is the blog already exported?
 +# is the blob already exported?
 
 What is this?  Whitespace?

Typofix: s/blog/blob/

 @@ -521,7 +521,7 @@ def c_style_unescape(string):
  return string

  def parse_commit(parser):
 -global marks, blob_marks, bmarks, parsed_refs
 +global marks, blob_marks, parsed_refs
 
 How is this trivial?  You just removed one argument.

Maybe bmarks was no longer used there as a global variable
(left-over from previous patches?), so there is no longer any
need to declare it global.

 @@ -555,7 +555,7 @@ def parse_commit(parser):
  mark = int(mark_ref[1:])
  f = { 'mode' : m, 'data' : blob_marks[mark] }
  elif parser.check('D'):
 -t, path = line.split(' ')
 +t, path = line.split(' ', 1)
 
 How on earth is this trivial?  It changes the entire meaning!

Indeed, that should best go in a separate path with a proper
explanation in the commit message.

 @@ -643,6 +643,7 @@ def do_export(parser):
  wt = repo.bzrdir.open_workingtree()
  wt.update()
  print ok %s % ref
 +
 
 Whitespace?

Isn't that obvious?

 I'm outraged by this.  What kind of changes are you pushing to
 remote-hg?  A trivial cleanups bundling miscellaneous changes, with
 no commit message?  Why don't you just squash everything into one
 miscellaneous changes patch?

I have no opinion on this, so I won't comment.

Regard,
  Stefano
--
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] zlib: fix compilation failures with Sun C Compilaer

2013-04-22 Thread Stefano Lattarini
Do this by removing a couple of useless return statements.  Without this
change, compilation with Sun C Compiler 5.9 (SunOS_i386 Patch 124868-15
2010/08/11) fails with the following message:

  zlib.c, line 192: void function cannot return value
  zlib.c, line 201: void function cannot return value
  cc: acomp failed for zlib.c

Signed-off-by: Stefano Lattarini stefano.lattar...@gmail.com
---
 zlib.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/zlib.c b/zlib.c
index bbaa081..61e6df0 100644
--- a/zlib.c
+++ b/zlib.c
@@ -189,7 +189,7 @@ void git_deflate_init_gzip(git_zstream *strm, int level)
 * Use default 15 bits, +16 is to generate gzip header/trailer
 * instead of the zlib wrapper.
 */
-   return do_git_deflate_init(strm, level, 15 + 16);
+   do_git_deflate_init(strm, level, 15 + 16);
 }
 
 void git_deflate_init_raw(git_zstream *strm, int level)
@@ -198,7 +198,7 @@ void git_deflate_init_raw(git_zstream *strm, int level)
 * Use default 15 bits, negate the value to get raw compressed
 * data without zlib header and trailer.
 */
-   return do_git_deflate_init(strm, level, -15);
+   do_git_deflate_init(strm, level, -15);
 }
 
 int git_deflate_abort(git_zstream *strm)
-- 
1.8.1.rc3.897.gb3600c3

--
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] zlib: fix compilation failures with Sun C Compilaer

2013-04-22 Thread Stefano Lattarini
On 04/22/2013 06:48 PM, Junio C Hamano wrote:
 Stefano Lattarini stefano.lattar...@gmail.com writes:
 
 Do this by removing a couple of useless return statements.  Without this
 change, compilation with Sun C Compiler 5.9 (SunOS_i386 Patch 124868-15
 2010/08/11) fails with the following message:

   zlib.c, line 192: void function cannot return value
   zlib.c, line 201: void function cannot return value
   cc: acomp failed for zlib.c
 
 Thanks for catching a recent regression in the mainline before any
 tagged release is made out of it.  Very much appreciated.

Actually, I tried to build the bleeding-edge git on Solaris to use it
myself, rather than to test it ;-)  So, thanks to you and all the git
contributors for continuously improving the package, thus making it
worth to try to build and use the bleeding-edge version.

Best regards,
  Stefano
--
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] zlib: fix compilation failures with Sun C Compilaer

2013-04-22 Thread Stefano Lattarini
On 04/22/2013 11:41 PM, Eric Sunshine wrote:
 On Mon, Apr 22, 2013 at 12:18 PM, Stefano Lattarini
 stefano.lattar...@gmail.com wrote:
 zlib: fix compilation failures with Sun C Compilaer
 
 s/Compilaer/compiler/
 
Oops, well spotted.  Junio, can you fix this locally?

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


Re: [PATCH 3/6] transport-helper: clarify pushing without refspecs

2013-04-18 Thread Stefano Lattarini
On 04/18/2013 02:05 AM, Felipe Contreras wrote:
 This has never worked, since it's inception the code simply skips all

s/it's/its/ (sorry for nitpicking)

 the refs, essentially telling fast-export to do nothing.
 
 Let's at least tell the user what's going on.
 
 [SNIP]


Regards,
  Stefano
--
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] Fix various typos and grammaros

2013-04-12 Thread Stefano Lattarini
Hi Junio.

On 04/12/2013 02:45 AM, Junio C Hamano wrote:
 Stefano Lattarini stefano.lattar...@gmail.com writes:
 
 How much of this stuff have interact with real changes that are in
 flight, with various doneness cooking in different integration
 branches?
 
I don't know, since I only follow the master branch of Git.  And
honestly, I don't have time right now to go checking for possible
conflicts, or to resubmit ...   But I see Jonathan has taken up
the ball on this (thanks Jonathan!), so I'll leave the matter to
him.

Next time I'll try to prepare a patch broken up in more digestible
pieces, so that it will be easier for you to deal with conflicts,
and/or to selectively decide which fixes are worth applying.

Thanks, and sorry for the confusion,
  Stefano
--
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] RelNotes: few typofixes in notes for recent releases

2013-04-11 Thread Stefano Lattarini
Signed-off-by: Stefano Lattarini stefano.lattar...@gmail.com
---
 Documentation/RelNotes/1.8.2.1.txt | 2 +-
 Documentation/RelNotes/1.8.3.txt   | 8 
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/Documentation/RelNotes/1.8.2.1.txt 
b/Documentation/RelNotes/1.8.2.1.txt
index 1354ad0..769a6fc 100644
--- a/Documentation/RelNotes/1.8.2.1.txt
+++ b/Documentation/RelNotes/1.8.2.1.txt
@@ -49,7 +49,7 @@ Fixes since v1.8.2
common prefix and suffix between the two filenames overlapped.
 
  * git submodule update, when recursed into sub-submodules, did not
-   acccumulate the prefix paths.
+   accumulate the prefix paths.
 
  * git am $maildir/ applied messages in an unexpected order; sort
filenames read from the maildir/ in a way that is more likely to
diff --git a/Documentation/RelNotes/1.8.3.txt b/Documentation/RelNotes/1.8.3.txt
index ca76a2b..537fc7a 100644
--- a/Documentation/RelNotes/1.8.3.txt
+++ b/Documentation/RelNotes/1.8.3.txt
@@ -41,11 +41,11 @@ UI, Workflows  Features
revert session, just like it does for a cherry-pick and a bisect
session.
 
- * The handing by git branch --set-upstream-to against various forms
-   of errorneous inputs was suboptimal and has been improved.
+ * The handling by git branch --set-upstream-to against various forms
+   of erroneous inputs was suboptimal and has been improved.
 
  * When the interactive access to git-shell is not enabled, it issues
-   a message meant to help the system admininstrator to enable it.
+   a message meant to help the system administrator to enable it.
An explicit way to help the end users who connect to the service by
issuing custom messages to refuse such an access has been added.
 
@@ -294,4 +294,4 @@ details).
alphabetical order.
 
  * git submodule update, when recursed into sub-submodules, did not
-   acccumulate the prefix paths.
+   accumulate the prefix paths.
-- 
1.8.2.373.g961c512

--
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] Various typofixes

2013-04-11 Thread Stefano Lattarini
Mostly suggested by codespell https://github.com/lucasdemarchi/codespell

Signed-off-by: Stefano Lattarini stefano.lattar...@gmail.com
---
 Documentation/git-credential.txt   |  2 +-
 Documentation/git-remote-ext.txt   |  2 +-
 Documentation/git-svn.txt  |  4 ++--
 Documentation/git-tools.txt|  2 +-
 Documentation/revisions.txt|  2 +-
 Documentation/technical/api-argv-array.txt |  2 +-
 Documentation/technical/api-credentials.txt|  2 +-
 Documentation/technical/api-ref-iteration.txt  |  2 +-
 builtin/apply.c|  6 +++---
 commit.c   |  2 +-
 commit.h   |  2 +-
 compat/nedmalloc/Readme.txt|  2 +-
 compat/nedmalloc/malloc.c.h|  6 +++---
 compat/obstack.h   |  2 +-
 compat/precompose_utf8.c   |  2 +-
 compat/regex/regcomp.c |  4 ++--
 compat/regex/regex.c   |  2 +-
 compat/regex/regex_internal.c  |  6 +++---
 contrib/mw-to-git/git-remote-mediawiki.perl|  6 +++---
 contrib/mw-to-git/t/README |  6 +++---
 contrib/mw-to-git/t/install-wiki/LocalSettings.php |  2 +-
 contrib/mw-to-git/t/t9360-mw-to-git-clone.sh   |  2 +-
 contrib/mw-to-git/t/t9362-mw-to-git-utf8.sh| 10 +-
 contrib/subtree/t/t7900-subtree.sh |  2 +-
 diff.c |  2 +-
 git-add--interactive.perl  |  2 +-
 git-cvsserver.perl |  4 ++--
 git-gui/lib/blame.tcl  |  2 +-
 git-gui/lib/index.tcl  |  2 +-
 git-gui/lib/spellcheck.tcl |  4 ++--
 git-quiltimport.sh |  2 +-
 gitweb/INSTALL |  2 +-
 gitweb/gitweb.perl |  6 +++---
 kwset.c|  4 ++--
 perl/Git.pm|  2 +-
 perl/Git/I18N.pm   |  2 +-
 perl/private-Error.pm  |  2 +-
 po/README  |  6 +++---
 sequencer.c|  2 +-
 t/t1006-cat-file.sh|  2 +-
 t/t3511-cherry-pick-x.sh   |  4 ++--
 t/t3701-add-interactive.sh |  2 +-
 t/t4014-format-patch.sh|  6 +++---
 t/t4124-apply-ws-rule.sh   |  2 +-
 t/t6030-bisect-porcelain.sh|  2 +-
 t/t7601-merge-pull-config.sh   |  2 +-
 t/t7610-mergetool.sh   |  2 +-
 t/t9001-send-email.sh  |  4 ++--
 transport-helper.c |  2 +-
 transport.h|  2 +-
 xdiff/xdiffi.c |  2 +-
 xdiff/xhistogram.c |  2 +-
 52 files changed, 79 insertions(+), 79 deletions(-)

diff --git a/Documentation/git-credential.txt b/Documentation/git-credential.txt
index 472f00f..7da0f13 100644
--- a/Documentation/git-credential.txt
+++ b/Documentation/git-credential.txt
@@ -56,7 +56,7 @@ For example, if we want a password for
 `https://example.com/foo.git`, we might generate the following
 credential description (don't forget the blank line at the end; it
 tells `git credential` that the application finished feeding all the
-infomation it has):
+information it has):
 
 protocol=https
 host=example.com
diff --git a/Documentation/git-remote-ext.txt b/Documentation/git-remote-ext.txt
index 58b7fac..8cfc748 100644
--- a/Documentation/git-remote-ext.txt
+++ b/Documentation/git-remote-ext.txt
@@ -86,7 +86,7 @@ begins with `ext::`.  Examples:
edit .ssh/config.
 
 ext::socat -t3600 - ABSTRACT-CONNECT:/git-server %G/somerepo::
-   Represents repository with path /somerepo accessable over
+   Represents repository with path /somerepo accessible over
git protocol at abstract namespace address /git-server.
 
 ext::git-server-alias foo %G/repo::
diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index 1b8b649..7706d41 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -245,7 +245,7 @@ first have already been pushed into SVN.
patch), all (accept all patches), or quit.
+
'git svn dcommit' returns immediately if answer if no or quit, 
without
-   commiting anything to SVN.
+   committing anything to SVN.
 
 'branch'::
Create a branch in the SVN repository.
@@ -856,7 +856,7

[PATCH v2] Fix various typos and grammaros

2013-04-11 Thread Stefano Lattarini
Most typos suggested by codespell:
https://github.com/lucasdemarchi/codespell

Grammaros pointed out by Eric Sunshine.

Helped-by: Eric Sunshine sunsh...@sunshineco.com
Signed-off-by: Stefano Lattarini stefano.lattar...@gmail.com
---
 Documentation/git-credential.txt   |  2 +-
 Documentation/git-remote-ext.txt   |  2 +-
 Documentation/git-svn.txt  |  4 ++--
 Documentation/git-tools.txt|  2 +-
 Documentation/revisions.txt|  2 +-
 Documentation/technical/api-argv-array.txt |  2 +-
 Documentation/technical/api-credentials.txt|  2 +-
 Documentation/technical/api-ref-iteration.txt  |  2 +-
 builtin/apply.c|  6 +++---
 commit.c   |  2 +-
 commit.h   |  2 +-
 compat/nedmalloc/Readme.txt|  2 +-
 compat/nedmalloc/malloc.c.h|  6 +++---
 compat/obstack.h   |  2 +-
 compat/precompose_utf8.c   |  2 +-
 compat/regex/regcomp.c |  4 ++--
 compat/regex/regex.c   |  2 +-
 compat/regex/regex_internal.c  |  6 +++---
 contrib/mw-to-git/git-remote-mediawiki.perl|  6 +++---
 contrib/mw-to-git/t/README |  6 +++---
 contrib/mw-to-git/t/install-wiki/LocalSettings.php |  2 +-
 contrib/mw-to-git/t/t9360-mw-to-git-clone.sh   |  2 +-
 contrib/mw-to-git/t/t9362-mw-to-git-utf8.sh| 14 +++---
 contrib/subtree/t/t7900-subtree.sh |  2 +-
 diff.c |  2 +-
 git-add--interactive.perl  |  2 +-
 git-cvsserver.perl |  4 ++--
 git-gui/lib/blame.tcl  |  2 +-
 git-gui/lib/index.tcl  |  2 +-
 git-gui/lib/spellcheck.tcl |  4 ++--
 git-quiltimport.sh |  2 +-
 gitweb/INSTALL |  2 +-
 gitweb/gitweb.perl |  6 +++---
 kwset.c|  4 ++--
 perl/Git.pm|  2 +-
 perl/Git/I18N.pm   |  2 +-
 perl/private-Error.pm  |  2 +-
 po/README  |  6 +++---
 sequencer.c|  2 +-
 t/t1006-cat-file.sh|  2 +-
 t/t3511-cherry-pick-x.sh   |  4 ++--
 t/t3701-add-interactive.sh |  2 +-
 t/t4014-format-patch.sh|  6 +++---
 t/t4124-apply-ws-rule.sh   |  2 +-
 t/t6030-bisect-porcelain.sh|  2 +-
 t/t7601-merge-pull-config.sh   |  2 +-
 t/t7610-mergetool.sh   |  2 +-
 t/t9001-send-email.sh  |  4 ++--
 transport-helper.c |  2 +-
 transport.h|  2 +-
 xdiff/xdiffi.c |  2 +-
 xdiff/xhistogram.c |  2 +-
 52 files changed, 81 insertions(+), 81 deletions(-)

diff --git a/Documentation/git-credential.txt b/Documentation/git-credential.txt
index 472f00f..7da0f13 100644
--- a/Documentation/git-credential.txt
+++ b/Documentation/git-credential.txt
@@ -56,7 +56,7 @@ For example, if we want a password for
 `https://example.com/foo.git`, we might generate the following
 credential description (don't forget the blank line at the end; it
 tells `git credential` that the application finished feeding all the
-infomation it has):
+information it has):
 
 protocol=https
 host=example.com
diff --git a/Documentation/git-remote-ext.txt b/Documentation/git-remote-ext.txt
index 58b7fac..8cfc748 100644
--- a/Documentation/git-remote-ext.txt
+++ b/Documentation/git-remote-ext.txt
@@ -86,7 +86,7 @@ begins with `ext::`.  Examples:
edit .ssh/config.
 
 ext::socat -t3600 - ABSTRACT-CONNECT:/git-server %G/somerepo::
-   Represents repository with path /somerepo accessable over
+   Represents repository with path /somerepo accessible over
git protocol at abstract namespace address /git-server.
 
 ext::git-server-alias foo %G/repo::
diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index 1b8b649..7706d41 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -245,7 +245,7 @@ first have already been pushed into SVN.
patch), all (accept all patches), or quit.
+
'git svn dcommit' returns immediately if answer if no or quit, 
without
-   commiting anything to SVN.
+   committing

Re: [PATCH] Bugfix: undefined htmldir in config.mak.autogen

2013-02-20 Thread Stefano Lattarini
On 02/20/2013 02:39 AM, Jiang Xin wrote:

 [SNIP]
 
 I am not familiar with autoconf. After clone autoconf and check,
 I cannot find a neat way to change htmldir default location to
 use ${datarootdir} (just like mandir).

This one-line change should be enough to do what you want:

  diff --git a/configure.ac b/configure.ac
  index 1991258..2bfbec9 100644
  --- a/configure.ac
  +++ b/configure.ac
  @@ -149,6 +149,8 @@ AC_CONFIG_SRCDIR([git.c])
   config_file=config.mak.autogen
   config_in=config.mak.in

  +AC_SUBST([htmldir], ['${datarootdir}'])
  +
   GIT_CONF_SUBST([AUTOCONFIGURED], [YesPlease])

   # Directories holding saner versions of common or POSIX binaries.

Not sure whether this a good idea though (I haven't really followed the
discussion); but it is easily doable.

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


[BUG] Infinite make recursion when configure.ac changes

2013-02-20 Thread Stefano Lattarini
From a pristine master checkout:

  $ make configure  ./configure make
  ... # All successfull
  $ touch configure.ac
  $ make
GEN config.status
  make[1]: Entering directory `/storage/home/stefano/git/src'
GEN config.status
  make[2]: Entering directory `/storage/home/stefano/git/src'
GEN config.status
  make[3]: Entering directory `/storage/home/stefano/git/src'
GEN config.status
  make[4]: Entering directory `/storage/home/stefano/git/src'
GEN config.status
  make[5]: Entering directory `/storage/home/stefano/git/src'
GEN config.status
  ...

and I have to hit ^C to interrupt that recursion.

This seems due to the change in commit v1.7.12.4-1-g1226504: the
issue is still present there, but no longer is in the preceding
commit 7e201053 (a.k.a. v1.7.12.4).

I haven't investigated this any further for the moment.

Regards,
  Stefano
--
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] Bugfix: undefined htmldir in config.mak.autogen

2013-02-20 Thread Stefano Lattarini
On 02/20/2013 11:42 AM, Jiang Xin wrote:
  2013/2/20 Stefano Lattarini stefano.lattar...@gmail.com:
 On 02/20/2013 02:39 AM, Jiang Xin wrote:

 [SNIP]

 I am not familiar with autoconf. After clone autoconf and check,
 I cannot find a neat way to change htmldir default location to
 use ${datarootdir} (just like mandir).

 This one-line change should be enough to do what you want:

   diff --git a/configure.ac b/configure.ac
   index 1991258..2bfbec9 100644
   --- a/configure.ac
   +++ b/configure.ac
   @@ -149,6 +149,8 @@ AC_CONFIG_SRCDIR([git.c])
config_file=config.mak.autogen
config_in=config.mak.in

   +AC_SUBST([htmldir], ['${datarootdir}'])
   +
GIT_CONF_SUBST([AUTOCONFIGURED], [YesPlease])

 
 If changed like that, set:
 
  AC_SUBST([htmldir], ['${datarootdir}/doc/git-doc'])
 
 In the generated configure file, this instruction will be inserted
 after the option_parse block (not before), and will override what
 the user provided by running ./configure --htmldir=DOCDIR.

Yikes, you're right.  Scratch my suggestion then; the issue should
probably be brought up on the autoconf mailing list.  Albeit I think
it is by design that autoconf doesn't let a package to override the
defaults for installation directory: this way, the end users can
expect consistent, well-documented defaults for all autoconf-based
packages.


 BTW, add docdir = @docdir@ to config.mak.in, also let
 ./configure --docdir=DIR works properly.
 

Thanks, and sorry for the noise,
  Stefano
--
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] build: do not automatically reconfigure unless configure.ac changed

2013-01-02 Thread Stefano Lattarini
On 01/02/2013 09:48 AM, Jonathan Nieder wrote:
 Jeff King wrote:
 
 It seems I am late to the party. But FWIW, this looks the most sane to
 me of the patches posted in this thread.
 
 Thanks.  config.status runs ./configure itself, though, so the rule
 should actually be
 
   config.status: configure.ac
   $(QUIET_GEN)$(MAKE) configure  \
   if test -f config.status; then \
 ./config.status --recheck; \
   else \
 ./configure;
   fi
 
 Rather than screw it up yet again, I'm going to sleep. :)  If someone
 else corrects the patch before tomorrow, I won't mind.

FYI, this seems a sane approach to me.  At least until Autoconf is
improved to offer better (read: some :-) support to dynamic package
version numbers specified at configure runtime.  I hope that day
isn't too far, since the current Autoconf limitation has been causing
its share of annoyances small woes in Automake and Gnulib as well.

The only nit I have to offer is that I'd like to see more comments in
the git Makefile about why this semi-hack is needed.

Thanks,
  Stefano
--
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] build: do not automatically reconfigure unless configure.ac changed

2013-01-02 Thread Stefano Lattarini
On 01/02/2013 05:50 PM, Junio C Hamano wrote:
 Stefano Lattarini stefano.lattar...@gmail.com writes:
 
 On 01/02/2013 09:48 AM, Jonathan Nieder wrote:
 Jeff King wrote:

 It seems I am late to the party. But FWIW, this looks the most sane to
 me of the patches posted in this thread.
 ...
 FYI, this seems a sane approach to me
 The only nit I have to offer is that I'd like to see more comments in
 the git Makefile about why this semi-hack is needed.
 
 Thanks, everybody.
 
 Please eyeball the below for (hopefully) the last time, to be
 eventually merged to maint-1.7.12, maint-1.8.0 and maint (aka
 maint-1.8.1) branches.
 
 -- 8 --
 From: Jonathan Nieder jrnie...@gmail.com
 Date: Wed, 2 Jan 2013 00:25:44 -0800
 Subject: [PATCH] build: do not automatically reconfigure unless configure.ac 
 changed
 
 Starting with v1.7.12-rc0~4^2 (build: reconfigure automatically if
 configure.ac changes, 2012-07-19), config.status --recheck is
 automatically run every time the configure script changes.  In
 particular, that means the configuration procedure repeats whenever
 the version number changes (since the configure script changes to
 support ./configure --version and ./configure --help), making
 bisecting painfully slow.
 
 The intent was to make the reconfiguration process only trigger for
 changes to configure.ac's logic.  Tweak the Makefile rule to match
 that intent by depending on configure.ac instead of configure.
 
 Reported-by: Martin von Zweigbergk martinv...@gmail.com
 Signed-off-by: Jonathan Nieder jrnie...@gmail.com
 Reviewed-by: Jeff King p...@peff.net
 Reviewed-by: Stefano Lattarini stefano.lattar...@gmail.com
 ---
  Makefile | 10 --
  1 file changed, 8 insertions(+), 2 deletions(-)
 
 diff --git a/Makefile b/Makefile
 index 26b697d..2f5e2ab 100644
 --- a/Makefile
 +++ b/Makefile
 @@ -2167,8 +2167,14 @@ configure: configure.ac GIT-VERSION-FILE
   $(RM) $+
  
  ifdef AUTOCONFIGURED
 -config.status: configure
 - $(QUIET_GEN)if test -f config.status; then \
 +# We avoid depending on 'configure' here, because it gets rebuilt
 +# every time GIT-VERSION-FILE is modified, only to update the embedded
 +# version number string, which config.status does not care about.

Alas, config.status *do* care about it, in that the '@PACKAGE_VERSION@',
'@PACKAGE_STRING@' and '@DEFS@' substitutions are affected by what is
hard-coded in configure as the version number [1].  But if we do not
use those substitutions in any of our files (and I believe we don't),
then *we* can happily not care about the configure embedded version
number string, and thus avoid the extra configure runs.  Phew.

 [1] Yes, this is a mess.  We know.  Sorry!

 +# We
 +# do want to recheck when the platform/environment detection logic
 +# changes, hence this depends on configure.ac.
 +config.status: configure.ac
 + $(QUIET_GEN)$(MAKE) configure  \
 + if test -f config.status; then \
 ./config.status --recheck; \
   else \
 ./configure; \

HTH,
  Stefano
--
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] build: do not automatically reconfigure unless configure.ac changed

2013-01-02 Thread Stefano Lattarini
On 01/02/2013 09:25 PM, Junio C Hamano wrote:
 Stefano Lattarini stefano.lattar...@gmail.com writes:
 
  ifdef AUTOCONFIGURED
 -config.status: configure
 -   $(QUIET_GEN)if test -f config.status; then \
 +# We avoid depending on 'configure' here, because it gets rebuilt
 +# every time GIT-VERSION-FILE is modified, only to update the embedded
 +# version number string, which config.status does not care about.

 Alas, config.status *do* care about it, in that the '@PACKAGE_VERSION@',
 '@PACKAGE_STRING@' and '@DEFS@' substitutions are affected by what is
 hard-coded in configure as the version number [1].  But if we do not
 use those substitutions in any of our files (and I believe we don't),
 then *we* can happily not care about the configure embedded version
 number string, and thus avoid the extra configure runs.  Phew.

  [1] Yes, this is a mess.  We know.  Sorry!
 
 Heh.  Should we warn against the use of these symbols somewhere in
 configure.ac, perhaps, then?

Actually, they should be checked against in files processed by
'config.status', i.e., files listed in AC_CONFIG_FILES calls in
'configure.ac'.  But I honestly believe that would be overkill;
I say we simply adjust your comment to read something like:

  # We avoid depending on 'configure' here, because it gets rebuilt
  # every time GIT-VERSION-FILE is modified, only to update the
  # embedded version number string, which we however do not
  # substitute in any file processed by config.status.

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


Re: [PATCH v7 2/7] tests: paint known breakages in yellow

2012-12-21 Thread Stefano Lattarini
On 12/21/2012 04:12 AM, Junio C Hamano wrote:
 From: Adam Spiers g...@adamspiers.org
 
 Yellow seems a more appropriate color than bold green when
 considering the universal traffic lights coloring scheme, where
 green conveys the impression that everything's OK, and amber that
 something's not quite right.

Here are few more details about the behaviour of other testing
tools, in case you want to squash them in the commit message for
future references:

1. Automake (at least up to 1.13) and Autotest (at least up to the
   2.69 Autoconf release) use bold green for reporting expected
   failures.

2. On the other hand, the 'prove' utility (as of TAP::Harness v3.23
   and Perl v5.14.2) use yellow (not bold) for the same purpose.

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


Re: [PATCH v7 2/7] tests: paint known breakages in yellow

2012-12-21 Thread Stefano Lattarini
On 12/21/2012 04:46 PM, Junio C Hamano wrote:

 [SNIP]

 The only thing the additional knowledge adds seems to be to give
 rationale for the old choice of bold green---it was not chosen
 from thin-air but can be viewed as following the automake/autotest
 scheme, and other systems cannot agree on what color to pick for
 this purpose.
 
 I do not see a need to justify why we chose differently from
 automake/autotest; we could say something like:
 
 Yellow seems a more appropriate color than bold green when
 considering the universal traffic lights coloring scheme, where
 green conveys the impression that everything's OK, and amber that
 something's not quite right.  This is in line with what 'prove'
 uses, but different from 'automake/autotest' do.
 
 but we are not in the business of choosing which is more correct
 between prove and automake/autotest, and I do not see how it adds
 much value to tell readers that color choices are not universally
 agreed upon across various test software suites---that's kind of
 known, isn't it?
 
 So...

That is fine with me, I just pointed it out because I suspected not
everybody was aware of all these details.  If you decide they don't
matter, it's perfectly OK -- but at least now it's an informed
choice ;-)

Thanks,
  Stefano


--
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] README: Git is released under the GPLv2, not just the GPL

2012-12-15 Thread Stefano Lattarini
On 12/15/2012 07:35 PM, Junio C Hamano wrote:
 Junio C Hamano gits...@pobox.com writes:
 
 Stefano Lattarini stefano.lattar...@gmail.com writes:

 And this is clearly stressed by Linus in the COPYING file.  So make it
 clear in the README as well, to avoid possible misunderstandings.

 Signed-off-by: Stefano Lattarini stefano.lattar...@gmail.com
 ---

 I have nothing against this patch, but I am curious if you saw any
 misunderstandings in the real world, or if you are merely trying to
 avoid possible ones.

Only playing safe against possible misunderstandings, especially
now that the GPLv3 has taken root and has supplanted the v2 in several
projects (e.g., Perl, and obviously most GNU packages).

  README | 7 ---
  1 file changed, 4 insertions(+), 3 deletions(-)

 diff --git a/README b/README
 index d2690ec..c50e6f4 100644
 --- a/README
 +++ b/README
 @@ -19,9 +19,10 @@ Git is a fast, scalable, distributed revision control 
 system with an
  unusually rich command set that provides both high-level operations
  and full access to internals.
  
 -Git is an Open Source project covered by the GNU General Public License.
 -It was originally written by Linus Torvalds with help of a group of
 -hackers around the net. It is currently maintained by Junio C Hamano.
 +Git is an Open Source project covered by the GNU General Public License
 +(version 2).  It was originally written by Linus Torvalds with help
 +of a group of hackers around the net. It is currently maintained by
 +Junio C Hamano.
  
  Please read the file INSTALL for installation instructions.
 
 The project as a whole is GPLv2, and inclusion of pieces licensed
 under different but compatible terms does not change it, but we may
 want to do this instead.
 
 I am just one of the group of hackers around the net in the
 context of this overview, so I think it is OK to drop that
 currently maintained by bit. The audience of this document does
 not have to find out and interact with the maintainer.
 
 diff --git a/README b/README
 index d2690ec..c365e3c 100644
 --- a/README
 +++ b/README
 @@ -19,9 +19,10 @@ Git is a fast, scalable, distributed revision control 
 system with an
  unusually rich command set that provides both high-level operations
  and full access to internals.
  
 -Git is an Open Source project covered by the GNU General Public License.
 +Git is an Open Source project covered by the GNU General Public
 +License version 2 (some parts of it are under different licenses).

Maybe you could be even more explicit ans state some parts of it are
under different licenses, compatible with the GPLv2.  But maybe this
is just overkill?

  It was originally written by Linus Torvalds with help of a group of
 -hackers around the net. It is currently maintained by Junio C Hamano.
 +hackers around the net.
  
  Please read the file INSTALL for installation instructions.
  

Thanks,
  Stefano
--
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] README: Git is released under the GPLv2, not just the GPL

2012-12-15 Thread Stefano Lattarini
On 12/15/2012 08:39 PM, Junio C Hamano wrote:
 Stefano Lattarini stefano.lattar...@gmail.com writes:
 
 On 12/15/2012 07:35 PM, Junio C Hamano wrote:
 ...
 -Git is an Open Source project covered by the GNU General Public License.
 +Git is an Open Source project covered by the GNU General Public
 +License version 2 (some parts of it are under different licenses).

 Maybe you could be even more explicit ans state some parts of it are
 under different licenses, compatible with the GPLv2.  But maybe this
 is just overkill?
 
 I personally do not think it is an overkill; because this clarify
 that we are version 2, followed by not everything is, but as a
 whole it is still GPLv2 is in the end about covering our ass, it is
 better to be as clear as possible without making it into a novel.
 
Well, if you put it this way :-)  I agree that it's better be safe
than be sorry.

Thanks,
  Stefano
--
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] Makefile: whitespace style fixes in macro definitions

2012-12-09 Thread Stefano Lattarini
Consistently use a single space before and after the = (or :=, +=,
etc.) in assignments to make macros.  Granted, this was not a big deal,
but I did find the needless inconsistency quite distracting.

Signed-off-by: Stefano Lattarini stefano.lattar...@gmail.com
---
 Makefile  | 56 
 config.mak.in |  2 +-
 t/Makefile|  2 +-
 3 files changed, 30 insertions(+), 30 deletions(-)

diff --git a/Makefile b/Makefile
index 4ad6fbd..736ecd4 100644
--- a/Makefile
+++ b/Makefile
@@ -374,7 +374,7 @@ htmldir = share/doc/git-doc
 ETC_GITCONFIG = $(sysconfdir)/gitconfig
 ETC_GITATTRIBUTES = $(sysconfdir)/gitattributes
 lib = lib
-# DESTDIR=
+# DESTDIR =
 pathsep = :
 
 export prefix bindir sharedir sysconfdir gitwebdir localedir
@@ -575,9 +575,9 @@ endif
 export PERL_PATH
 export PYTHON_PATH
 
-LIB_FILE=libgit.a
-XDIFF_LIB=xdiff/lib.a
-VCSSVN_LIB=vcs-svn/lib.a
+LIB_FILE = libgit.a
+XDIFF_LIB = xdiff/lib.a
+VCSSVN_LIB = vcs-svn/lib.a
 
 LIB_H += xdiff/xinclude.h
 LIB_H += xdiff/xmacros.h
@@ -1139,7 +1139,7 @@ ifeq ($(uname_S),NetBSD)
 endif
 ifeq ($(uname_S),AIX)
DEFAULT_PAGER = more
-   NO_STRCASESTR=YesPlease
+   NO_STRCASESTR = YesPlease
NO_MEMMEM = YesPlease
NO_MKDTEMP = YesPlease
NO_MKSTEMPS = YesPlease
@@ -1147,7 +1147,7 @@ ifeq ($(uname_S),AIX)
NO_NSEC = YesPlease
FREAD_READS_DIRECTORIES = UnfortunatelyYes
INTERNAL_QSORT = UnfortunatelyYes
-   NEEDS_LIBICONV=YesPlease
+   NEEDS_LIBICONV = YesPlease
BASIC_CFLAGS += -D_LARGE_FILES
ifeq ($(shell expr $(uname_V) : '[1234]'),1)
NO_PTHREADS = YesPlease
@@ -1155,13 +1155,13 @@ ifeq ($(uname_S),AIX)
PTHREAD_LIBS = -lpthread
endif
ifeq ($(shell expr $(uname_V).$(uname_R) : '5\.1'),3)
-   INLINE=''
+   INLINE = ''
endif
GIT_TEST_CMP = cmp
 endif
 ifeq ($(uname_S),GNU)
# GNU/Hurd
-   NO_STRLCPY=YesPlease
+   NO_STRLCPY = YesPlease
NO_MKSTEMPS = YesPlease
HAVE_PATHS_H = YesPlease
LIBC_CONTAINS_LIBINTL = YesPlease
@@ -1187,9 +1187,9 @@ ifeq ($(uname_S),IRIX)
NEEDS_LIBGEN = YesPlease
 endif
 ifeq ($(uname_S),IRIX64)
-   NO_SETENV=YesPlease
+   NO_SETENV = YesPlease
NO_UNSETENV = YesPlease
-   NO_STRCASESTR=YesPlease
+   NO_STRCASESTR = YesPlease
NO_MEMMEM = YesPlease
NO_MKSTEMPS = YesPlease
NO_MKDTEMP = YesPlease
@@ -1203,14 +1203,14 @@ ifeq ($(uname_S),IRIX64)
NO_REGEX = YesPlease
NO_FNMATCH_CASEFOLD = YesPlease
SNPRINTF_RETURNS_BOGUS = YesPlease
-   SHELL_PATH=/usr/gnu/bin/bash
+   SHELL_PATH = /usr/gnu/bin/bash
NEEDS_LIBGEN = YesPlease
 endif
 ifeq ($(uname_S),HP-UX)
INLINE = __inline
-   NO_IPV6=YesPlease
-   NO_SETENV=YesPlease
-   NO_STRCASESTR=YesPlease
+   NO_IPV6 = YesPlease
+   NO_SETENV = YesPlease
+   NO_STRCASESTR = YesPlease
NO_MEMMEM = YesPlease
NO_MKSTEMPS = YesPlease
NO_STRLCPY = YesPlease
@@ -1386,10 +1386,10 @@ ifeq ($(uname_S),NONSTOP_KERNEL)
MKDIR_WO_TRAILING_SLASH = YesPlease
# RFE 10-120912-4693 submitted to HP NonStop development.
NO_SETITIMER = UnfortunatelyYes
-   SANE_TOOL_PATH=/usr/coreutils/bin:/usr/local/bin
-   SHELL_PATH=/usr/local/bin/bash
+   SANE_TOOL_PATH = /usr/coreutils/bin:/usr/local/bin
+   SHELL_PATH = /usr/local/bin/bash
# as of H06.25/J06.14, we might better use this
-   #SHELL_PATH=/usr/coreutils/bin/bash
+   #SHELL_PATH = /usr/coreutils/bin/bash
 endif
 ifneq (,$(findstring MINGW,$(uname_S)))
pathsep = ;
@@ -1437,7 +1437,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
X = .exe
SPARSE_FLAGS = -Wno-one-bit-signed-bitfield
 ifneq (,$(wildcard ../THIS_IS_MSYSGIT))
-   htmldir=doc/git/html/
+   htmldir = doc/git/html/
prefix =
INSTALL = /bin/install
EXTLIBS += /mingw/lib/libz.a
@@ -1559,7 +1559,7 @@ else
CURL_LIBCURL = -lcurl
endif
ifdef NEEDS_SSL_WITH_CURL
-   CURL_LIBCURL += -lssl
+   CURL_LIBCURL += -lssl
ifdef NEEDS_CRYPTO_WITH_SSL
CURL_LIBCURL += -lcrypto
endif
@@ -1768,7 +1768,7 @@ ifdef OBJECT_CREATION_USES_RENAMES
 endif
 ifdef NO_STRUCT_ITIMERVAL
COMPAT_CFLAGS += -DNO_STRUCT_ITIMERVAL
-   NO_SETITIMER=YesPlease
+   NO_SETITIMER = YesPlease
 endif
 ifdef NO_SETITIMER
COMPAT_CFLAGS += -DNO_SETITIMER
@@ -1920,15 +1920,15 @@ ifneq (,$(XDL_FAST_HASH))
 endif
 
 ifeq ($(TCLTK_PATH),)
-NO_TCLTK=NoThanks
+NO_TCLTK = NoThanks
 endif
 
 ifeq ($(PERL_PATH),)
-NO_PERL=NoThanks
+NO_PERL = NoThanks
 endif
 
 ifeq ($(PYTHON_PATH),)
-NO_PYTHON=NoThanks
+NO_PYTHON = NoThanks
 endif
 
 QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
@@ -1975,13 +1975,13 @@ PROFILE_DIR

Re: Weird problem with git-submodule.sh

2012-12-09 Thread Stefano Lattarini
Hi Junio, Marc.

On 12/07/2012 10:08 PM, Junio C Hamano wrote:
 Marc Branchaud marcn...@xiplink.com writes:
 
 It's FreeBSD 7.2, which I know is an obsolete version but I'm not able to
 upgrade the machine.  I believe FreeBSD's sh is, or is derived from, dash.
 
 Finally.  Yes, as you suspected, I am perfectly fine to explicitly
 set IFS to the default values.
 
 I wanted to have specific names to write in the commit log message,
 in-code comments and possibly release notes.  That way, people can
 decide if the issue affects them and they should upgrade once the
 fix is made.

The Autoconf manual suggests against unsetting IFS instead of resetting
it to the default sequence for yet another reason: if IFS is unset, code
that tries to save and restore its value will incorrectly reset it to an
empty value, thus disabling field splitting:

unset IFS
# default separators used for field splitting
# ...
saved_IFS=$IFS
IFS=:
# code using the new IFS
IFS=$saved_IFS
# no field splitting performed from now on!

Not sure how this is relevant for the Git codebase, but maybe it is
something worth reporting in the commit message of a proposed patch.

Regards,
  Stefano

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


Re: [RFC] Add basic syntax check on shell scripts

2012-12-02 Thread Stefano Lattarini
On 12/02/2012 02:17 PM, Torsten Bögershausen wrote:
 The test suite needs to be run on different platforms.
 As it may be difficult for contributors to catch syntax
 which work on GNU/linux, but is unportable, make a quick check
 for the most common problems.
 sed -i, echo -n or array in shell scripts
 This list is not complete, and may need to be extended
 
 Signed-off-by: Torsten Bögershausen tbo...@web.de
 ---
 We add 1 second test execution time
 Is this a useful idea at all?
 
FWIW, I think such an idea is useful (and also easy to implement,
so another +1 from me).

  t/t9-syntax-check.sh | 28 
  1 file changed, 28 insertions(+)
  create mode 100755 t/t9-syntax-check.sh
 
 diff --git a/t/t9-syntax-check.sh b/t/t9-syntax-check.sh
 new file mode 100755
 index 000..c4a9289
 --- /dev/null
 +++ b/t/t9-syntax-check.sh
 @@ -0,0 +1,28 @@
 +#!/bin/sh
 +
 +test_description='Basic check if shell syntax is portable'
 +
 +. ./test-lib.sh
 +
 +
 +test_expect_success 'No arrays in shell scripts' '
 + expected 
 + (grep -i -n ^[  ]*declare[  ][  ]* ../*.sh ../../git-* 
 actual 21 || : ) 

Here I'd simply use:

! grep -n ^declare[ ][  ]* ../*.sh ../../*.sh

And similarly for the tests below.

In addition, the globs above still miss some files ('perf/perf-lib.sh'
and 'valgrind/analyze.sh', for example); so we might want to improve
it, using, say, git ls-files (or find(1), in case the test is to be
run also from distribution tarballs).

HTH,
  Stefano
--
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: Python extension commands in git - request for policy change

2012-11-25 Thread Stefano Lattarini
Hi David.  One minor but important correction ...

On 11/25/2012 12:51 PM, David Lang wrote:

 You may think that C and Bash are poor choices, but that is what the
 community is familar with.

Actually, it is C and POSIX shell -- not merely bash.  Indeed, the shell
code in Git is expected to work with the Solaris Korn shell, the BSD
/bin/sh, the dash shell (which is now the default /bin/sh on Debian and
Ubuntu), etc.

(Oh, and on the python vs. C vs. shell diatribe I'm mostly neutral,
mostly because I'm no Git developer, and I have no cents to throw).

Regards,
  Stefano

--
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: Unable to compile Git on HP-UX B.11.31 U ia64

2012-11-14 Thread Stefano Lattarini
On 11/14/2012 12:18 PM, Quintin Ronan wrote:
 Hello,
 
 I’m trying to compile git 1.7 on a HPUX server using make.
 The ./configure worked well :

 [SNIP]
 
 But when i run make (with –d) it simply doesn’t work with a message
 which isn’t really helpfull :
 
   [SNIP]
   Make: line 313: syntax error.  Stop.
 
 Can you help me ?
 
The Git build system requires GNU make, but it seems to me you are using
your system native make instead.  That won't work.  It might be the case
GNU make is installed on your system, but is named something like 'gmake'
or 'gnumake' rather than just 'make'.  What happens if you run the
following?

   $ gmake --version
   $ gnumake --version

If GNU make is not installed on your system, you can download the latest
version from here:

http://ftp.gnu.org/gnu/make/make-3.82.tar.gz

For more information about GNU make:

http://www.gnu.org/software/make/

HTH,
  Stefano
--
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: t9350-fast-export.sh broken on peff/pu under Mac OS X

2012-11-11 Thread Stefano Lattarini
On 11/11/2012 01:58 AM, Felipe Contreras wrote:

 It doesn't seem like zsh listens to that variable in sh mode:
   $ zsh -c 'emulate sh; NULLCMD=foobar;  content'

Right; emulate sh by itself is probably enough today (autoconf, trying
to make its generated scripts extra-portable, tends to accumulate a lot
of cruft unfortunately).

Thanks,
  Stefano

--
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: t9350-fast-export.sh broken on peff/pu under Mac OS X

2012-11-10 Thread Stefano Lattarini
On 11/11/2012 12:11 AM, Felipe Contreras wrote:
 On Sat, Nov 10, 2012 at 11:39 PM, Junio C Hamano gits...@pobox.com wrote:
 Felipe Contreras felipe.contre...@gmail.com writes:

 On Sat, Nov 10, 2012 at 3:37 PM, Torsten Bögershausen tbo...@web.de wrote:
 The short version:
 echo -n doesn't seem to be portable.
 The following works for me:

 Right, I was supposed to change that to:

   true  marks-cur 

 Please make it like so:

 marks-cur 

 No command is necessary when creating an empty file or truncating an
 existing file to empty, and no SP between redirection and its target.
 
 That hangs on zsh (presumably waiting for stdin).

Unless you set:

NULLCMD=:

early in your test script.

Or, to be extra-safe, you could steal this initialization code from
autoconf:

# Be more Bourne compatible.
if test -n ${ZSH_VERSION+set}  (emulate sh) /dev/null 21; then
  emulate sh
  NULLCMD=:
  setopt NO_GLOB_SUBST
  # Pre-4.2 versions of Zsh do word splitting on ${1+$@}, which
  # is contrary to our usage.  Disable this feature.
  alias -g '${1+$@}'='$@'
else
  case `(set -o) 2/dev/null` in *posix*) set -o posix ;; esac
fi

All of this untested with the real Git testsuite, of course ;-)

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


Re: [PATCH v4 04/14] Add new simplified git-remote-testgit

2012-11-02 Thread Stefano Lattarini
On 11/02/2012 03:02 AM, Felipe Contreras wrote:
 It's way simpler. It exerceises the same features of remote helpers.
 It's easy to read and understand. It doesn't depend on python.
 
 It does _not_ exercise the python remote helper framework; there's
 another tool and another test for that.
 
 For now let's just copy the old remote-helpers test script, although
 some of those tests don't make sense for this testgit (they still pass).
 
 In addition, this script would be able to test other features not
 currently being tested.
 
 Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
 ---
  Documentation/git-remote-testgit.txt |   2 +-
  git-remote-testgit   |  62 
  t/t5801-remote-helpers.sh| 134 
 +++
  3 files changed, 197 insertions(+), 1 deletion(-)
  create mode 100755 git-remote-testgit
  create mode 100755 t/t5801-remote-helpers.sh
 
 diff --git a/git-remote-testgit b/git-remote-testgit
 new file mode 100755
 index 000..6650402
 --- /dev/null
 +++ b/git-remote-testgit
 @@ -0,0 +1,62 @@
 +#!/bin/bash

I think git can't assume the existence of bash unconditionally, neither
in its scripts, nor in its tests (the exception being the tests on
bash completion, of course).  This script probably need to be re-written
to be a valid POSIX shell script.

It almost is, anyway, apart from the nits below ...

 +# Copyright (c) 2012 Felipe Contreras
 +
 +alias=$1

Just FYI: the double quoting here (and in several variable assignments
below) is redundant.  You can portably write it as:

alias=$1

and still be safe in the face of spaces and metacharacters in $1.
I'm not sure whether the Git coding guidelines suggest the use of
quoting in this situation though; if this is the case, feel free
to disregard my observation.

 +url=$2
 +
 +# huh?
 +url=${url#file://}
 +
 +dir=$GIT_DIR/testgit/$alias
 +prefix=refs/testgit/$alias
 +refspec=refs/heads/*:${prefix}/heads/*
 +
 +gitmarks=$dir/git.marks
 +testgitmarks=$dir/testgit.marks
 +
 +export GIT_DIR=$url/.git
 +
I believe this should be rewritten as:

  GIT_DIR=$url/.git; export GIT_DIR

in order to be portable to all the POSIX shells targeted by Git.

 +mkdir -p $dir
 +
 +test -e $gitmarks || echo -n  $gitmarks
 +test -e $testgitmarks || echo -n  $testgitmarks
 +
The '-n' option to echo is not portable.  To create an empty
file, you can just use

   :  file

or

   true  file

 +while read line; do
 +case $line in

Useless double quoting (my previous observation about Git coding
guidelines applies here as well, of course).

 +capabilities)
 +echo 'import'
 +echo 'export'
 +echo refspec $refspec
 +echo *import-marks $gitmarks
 +echo *export-marks $gitmarks
 +echo
 +;;
 +list)
 +git for-each-ref --format='? %(refname)' 'refs/heads/'
 +head=$(git symbolic-ref HEAD)
 +echo @$head HEAD
 +echo
 +;;
 +import*)
 +# read all import lines
 +while true; do
 +ref=${line#* }
 +refs=$refs $ref
 +read line
 +test ${line%% *} != import  break
 +done
 +
 +echo feature import-marks=$gitmarks
 +echo feature export-marks=$gitmarks
 +git fast-export --use-done-feature 
 --{import,export}-marks=$testgitmarks $refs | \

Better avoid the tricky {foo,bar} bashism:

git fast-export --use-done-feature \
--import-marks=$testgitmarks \
--export-marks=$testgitmarks \
$refs | \

 +sed -e s#refs/heads/#${prefix}/heads/#g
 +;;
 +export)
 +git fast-import --{import,export}-marks=$testgitmarks --quiet

Here too.

 +echo
 +;;
 +'')
 +exit

I'd put an explicit exit status here, for clarity (this is a matter
of personal preference, so feel free to disregard this nit).

 +;;
 +esac
 +done

Regards,
  Stefano

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


Re: [PATCH v4 09/14] remote-testgit: report success after an import

2012-11-02 Thread Stefano Lattarini
On 11/02/2012 03:02 AM, Felipe Contreras wrote:
 Doesn't make a difference for the tests, but it does for the ones
 seeking reference.
 
 Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
 ---
  git-remote-testgit | 11 +++
  1 file changed, 11 insertions(+)
 
 diff --git a/git-remote-testgit b/git-remote-testgit
 index 6c348b0..4e8b356 100755
 --- a/git-remote-testgit
 +++ b/git-remote-testgit
 @@ -59,7 +59,18 @@ while read line; do
  sed -e s#refs/heads/#${prefix}/heads/#g
  ;;
  export)
 +declare -A before after
 +
If you convert this script to be a valid POSIX shell script (as I've
suggested in my reply to [PATCH v4 04/14]), you'll need to get rid of
this bashism (and those below) as well.

 +eval $(git for-each-ref --format='before[%(refname)]=%(objectname)')
 +
  git fast-import --{import,export}-marks=$testgitmarks --quiet
 +
 +eval $(git for-each-ref --format='after[%(refname)]=%(objectname)')
 +
 +for ref in ${!after[@]}; do
 +test ${before[$ref]} ==  ${after[$ref]}  continue
 +echo ok $ref
 +done
  echo
  ;;
  '')

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


Re: [PATCH v4 04/14] Add new simplified git-remote-testgit

2012-11-02 Thread Stefano Lattarini
On 11/02/2012 04:42 PM, Felipe Contreras wrote:
 On Fri, Nov 2, 2012 at 2:55 PM, Stefano Lattarini
 stefano.lattar...@gmail.com wrote:
 
 +#!/bin/bash

 I think git can't assume the existence of bash unconditionally, neither
 in its scripts, nor in its tests (the exception being the tests on
 bash completion, of course).  This script probably need to be re-written
 to be a valid POSIX shell script.
 
 Well, this is a _reference_ script, and that is used only for testing
 purposes. The test itself can be like the bash completion tests, and
 simply be skipped.
 
 The reason I chose bash is because associative arrays, which you see
 in a later patch.
 
 It almost is, anyway, apart from the nits below ...

 +# Copyright (c) 2012 Felipe Contreras
 +
 +alias=$1

 Just FYI: the double quoting here (and in several variable assignments
 below) is redundant.  You can portably write it as:

 alias=$1

 and still be safe in the face of spaces and metacharacters in $1.
 I'm not sure whether the Git coding guidelines suggest the use of
 quoting in this situation though; if this is the case, feel free
 to disregard my observation.
 
 What happens when you call this with:
 
  ./script alias with spaces

'$alias' will correctly expand to alias with spaces.  Try out:

  $ sh -c 'alias=$1; echo $alias' dummy '1   2*3'
  1   2*3

This works consistently with every known shell (even non-POSIX
relics like Solaris /bin/sh).

 +url=$2
 +
 +# huh?
 +url=${url#file://}
 +
 +dir=$GIT_DIR/testgit/$alias
 +prefix=refs/testgit/$alias
 +refspec=refs/heads/*:${prefix}/heads/*
 +
 +gitmarks=$dir/git.marks
 +testgitmarks=$dir/testgit.marks
 +
 +export GIT_DIR=$url/.git
 +
 I believe this should be rewritten as:

   GIT_DIR=$url/.git; export GIT_DIR

 in order to be portable to all the POSIX shells targeted by Git.
 
 _If_ we want this as POSIX, yeah.

Why don't we?  Why add an extra requirement for a test that

 1. can be easily written in POSIX shell, and
 2. tests a feature that doesn't require bash to work (unless
I'm sorely mistaken, that is)?

Honest question.  But of course, if the Git active contributors
deem the extra requirement (which is not an invasive one, given
how often bash is installed even on non-Linux systems) acceptable
in order to have the test case simpler and clearer, feel free to
disregard all my observations in this thread.

 +mkdir -p $dir
 +
 +test -e $gitmarks || echo -n  $gitmarks
 +test -e $testgitmarks || echo -n  $testgitmarks
 +
 The '-n' option to echo is not portable.  To create an empty
 file, you can just use

:  file

 or

true  file
 
 All right, thanks.
 
 +while read line; do
 +case $line in

 Useless double quoting (my previous observation about Git coding
 guidelines applies here as well, of course).
 
 What if line has multiple spaces?

Still no problem, as in the case of the 'alias=$1' assignment before:

  $ sh -c 'case $1 in *x  x*) echo ok;; *) exit 1;; esac' dummy 'x  x'
  ok

 To me it makes sense to quote it.

Surely it doesn't cause any problem to over-quote in this case;
it's better than risking to under-quote in other.  I just pointed
out that the quoting it's not really necessary, in case you weren't
aware of that.

 +echo feature import-marks=$gitmarks
 +echo feature export-marks=$gitmarks
 +git fast-export --use-done-feature 
 --{import,export}-marks=$testgitmarks $refs | \

 Better avoid the tricky {foo,bar} bashism:

 git fast-export --use-done-feature \
 --import-marks=$testgitmarks \
 --export-marks=$testgitmarks \
 $refs | \
 
 If that's what we want, yeah.

Honestly, I find my longer-and-more-explicit version clearer, even
if you can assume bash for your script.  But that's a matter of
personal preference (sorry for not stating that right away), so
feel free to ignore it if you decide to keep the bash requirement
in the end.

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


Re: [PATCH v4 09/14] remote-testgit: report success after an import

2012-11-02 Thread Stefano Lattarini
On 11/02/2012 04:46 PM, Felipe Contreras wrote:

 In the end I liked this approach much better.
 
 If you have a solution for this that works in POSIX shell, I'll be
 glad to consider it, but for the moment, I think a simple, easy to
 understand and maintain code is more important, and if it needs bash,
 so be it.

If this is a deliberate decision, it's ok with me.  I'm just a casual
reviewer here, not an active contributor, so I'll gladly accept
preferences and decisions of the active crew, once it's clear that
they are deliberate and not the result of mistakes or confusion.

In any case, I agree that having a clean, understandable code as a
starting point is better than having a more portable but trickier
one right away.  If it will need converting to POSIX, that can be
done as a follow up (and as we've both noticed, this would be the
only point where such a conversion might be problematic -- the other
changes would be trivial, almost automatic).

Regards,
  Stefano

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


Re: [PATCH v4 09/14] remote-testgit: report success after an import

2012-11-02 Thread Stefano Lattarini
On 11/02/2012 05:19 PM, Felipe Contreras wrote:

 [SNIP]

 As things are the options are:
 
 1) Remove this code and move to POSIX sh. People looking for reference
 might scratch their heads as to why 'git push' is not showing the
 update.
 2) Keep this code and remain in bash.
 
 Until we have a:
 
 3) Replace this code with a clean POSIX sh alternative
 
 I would rather vote for 2)
 
That's perfectly fine with me, now that you've explained the rationale
for requiring bash in the first place (maybe adding a comment to the
script or the commit messages that reports this rationale for choosing
to require bash might be worthwhile; but it's no big deal anyway).

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


Re: [PATCH] test-lib: avoid full path to store test results

2012-10-31 Thread Stefano Lattarini
On 10/30/2012 11:17 PM, Elia Pinto wrote:
 Thanks. I know that posix support these usages, but exists some
 traditional shell that not support it.

True, but those shells are not POSIX shells -- the major example that
comes to mind is the accursed Solaris /bin/sh.

Since Git assumes a POSIX shell in its scripts and testsuite, use of
any POSIX feature should be fine -- until someone can show a real-world
POSIX shell that (likely due to a bug) fails to grasp such feature, in
which case a pragmatic workaround is needed.

Oh, and BTW, there are talks (and mostly consensus) among the Autotools
developers to start requiring a POSIX shell in the configure scripts
and Makefile recipes in the near future:

  http://lists.gnu.org/archive/html/bug-autoconf/2012-06/msg9.html

And also, related:

  http://lists.gnu.org/archive/html/automake/2012-08/msg00046.html
  http://lists.gnu.org/archive/html/coreutils/2012-10/msg00127.html

These are described in the
 autoconf manual, last time i have checked. As the construct ; export
 var = x should be portable, but it is not.

I don't think POSIX requires that to be portable.

 If this is important these days i don't know.

I hope the above helps to clarify the matter a little.

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


Re: [RFC/PATCH 3/4v2] test-lib: provide lazy TIME_COMMAND prereq

2012-10-16 Thread Stefano Lattarini
Hi Andreas.  I hope you don't mind my nitpickiness, but ...

On 10/16/2012 06:28 PM, Andreas Schwab wrote:
 Michael J Gruber g...@drmicha.warpmail.net writes:
 
 Some test want to use the time command (not the shell builtin) and test
 for its availability at /usr/bin/time.
 
 An alternative way to suppress the builtin meaning is to quote it, like
 \time.

... to be 100% precise, this quoting trick works because 'time' is a
shell keyword, rather than a builtin:

$ type time
time is a shell keyword

Regards,
  Stefano
--
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] configure.ac: Add missing comma to CC_LD_DYNPATH

2012-10-09 Thread Stefano Lattarini
[Re-sending because I forgot to CC: the list, sorry]

On 10/09/2012 06:36 PM, Øyvind A. Holm wrote:
 From: Øyvind A. Holm su...@sunbase.org

 40bfbde (build: don't duplicate substitution of make variables,
 2012-09-11)

Oops, stupid copy and paste error on my part.  Sorry.

 breaks make by removing a necessary comma at the end of
 CC_LD_DYNPATH=-rpath in line 414 and 423.

Here, s/-rpath/-Wl,-rpath/, as you've noted yourself in a follow-up
message.  And the reference to line 423 should be removed.

Also, as a very minor nit, I'd write might break make rather then
breaks make, because the breakage depends on which code path is
taken at configure time (and that's why I hadn't noticed the error
until now -- I never ran configure with the '--with-zlib' option).

 When executing ./configure --with-zlib=PATH, this resulted in

   [...]
   CC xdiff/xhistogram.o
   AR xdiff/lib.a
   LINK git-credential-store
   /usr/bin/ld: bad -rpath option
   collect2: ld returned 1 exit status
   make: *** [git-credential-store] Error 1
   $

 during make.

Indeed, I can reproduce and confirm this error :-(

 Signed-off-by: Øyvind A. Holm su...@sunbase.org
 ---
  configure.ac | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

 diff --git a/configure.ac b/configure.ac
 index da1f41f..ea79ea2 100644
 --- a/configure.ac
 +++ b/configure.ac
 @@ -411,7 +411,7 @@ else
LDFLAGS=${SAVE_LDFLAGS}
 ])
 if test $git_cv_ld_wl_rpath = yes; then
 -  CC_LD_DYNPATH=-Wl,-rpath
 +  CC_LD_DYNPATH=-Wl,-rpath,
 else
AC_CACHE_CHECK([if linker supports -rpath], git_cv_ld_rpath, [
   SAVE_LDFLAGS=${LDFLAGS}
 @@ -420,7 +420,7 @@ else
   LDFLAGS=${SAVE_LDFLAGS}
])
if test $git_cv_ld_rpath = yes; then
 - CC_LD_DYNPATH=-rpath
 + CC_LD_DYNPATH=-rpath,

And as Junio noted, this second hunk is unneeded, and in fact wrong.
Just remove it please.

With that done,
Acked-by: Stefano Lattarini stefano.lattar...@gmail.com

else
   CC_LD_DYNPATH=
   AC_MSG_WARN([linker does not support runtime path to dynamic 
 libraries])
Thanks,
  Stefano

--
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: [ENHANCEMENT] Allow '**' pattern in .gitignore

2012-10-02 Thread Stefano Lattarini
On 10/02/2012 09:21 AM, Ramkumar Ramachandra wrote:
 Hi,
 
 I've often found the '**' (extended) shell glob useful for matching
 any string crossing directory boundaries: it's especially useful if
 you only have a toplevel .gitignore, as opposed to a per-directory
 .gitignore.  Unfortunately, .gitignore currently uses fnmatch(3), and
 doesn't recognize '**'.  Would extending the .gitignore format to
 accept this be a useful feature?  Would it involve re-implementing and
 extending fnmatch, or is there some other way?

I think there is a topic in flight about this:

http://thread.gmane.org/gmane.comp.version-control.git/206406

HTH,
  Stefano
--
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: How do I run tests under Valgrind?

2012-09-22 Thread Stefano Lattarini
On 09/21/2012 10:49 PM, Jeff King wrote:

 Oh. It sounds like setting $SHELL to zsh is really the problem, then. If
 it is not Bourne-compatible when called as zsh, then it really should
 be called in a way that turns on compatibility mode (bash will do this
 when called as sh, but you can also do it with bash --posix).

AFAIK, if Zsh is called as sh, it too will run in Bourne compatibility
mode; not sure how to force this compatibility from the command line though
(albeit I'd guess there is some way to do so).

As further reference, here is the trick autoconf uses to (try to) put Zsh
in Bourne compatibility mode after it has been invoked:

if test -n ${ZSH_VERSION+set}  (emulate sh) /dev/null 21; then
   emulate sh
   NULLCMD=:
   # Pre-4.2 versions of Zsh do word splitting on ${1+$@}, which
   # is contrary to our usage.  Disable this feature.
   alias -g '${1+$@}'='$@'
   setopt NO_GLOB_SUBST
fi

You might think to use something like that in 't/test-lib.sh' (and other shell
libraries of the testsuite), but sadly that would not be enough; this excerpt
from the Automake suite shows why:

   # If Zsh is not started directly in POSIX-compatibility mode, it has some
   # incompatibilities in the handling of $0 that conflict with our usage;
   # i.e., $0 inside a file sourced with the '.' builtin is temporarily set
   # to the name of the sourced file.  Work around that.  The apparently
   # useless 'eval' here is needed by at least dash 0.5.2, to prevent it
   # from bailing out with an error like Syntax error: Bad substitution.
   # Note that a bug in some versions of Zsh prevents us from resetting $0
   # in a sourced script, so the use of $argv0.  For more info see:
   #  http://www.zsh.org/mla/workers/2009/msg01140.html
   eval 'argv0=${functrace[-1]%:*}'  test -f $argv0 || {
 echo Cannot determine the path of running test script. 2
 echo Your Zsh (version $ZSH_VERSION) is probably too old. 2
 exit 99
   }

Since I see that '$0' is used in (at least) 't/perf/perf-lib.sh' and
't/test-lib.sh', you'd need to copy the snippet above (or write an
equivalent one) in those files, and change them to use '$argv0' instead
of '$0' in few (but not all) places.  Not sure whether it's worth it
though -- given that you seems to have solved the issue already with a
simpler change, I'd say it's not.

Regards,
  Stefano
--
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: How do I run tests under Valgrind?

2012-09-22 Thread Stefano Lattarini
On 09/22/2012 07:47 PM, Jeff King wrote:
 On Sat, Sep 22, 2012 at 03:03:58PM +0200, Stefano Lattarini wrote:
 
 On 09/21/2012 10:49 PM, Jeff King wrote:

 Oh. It sounds like setting $SHELL to zsh is really the problem, then. If
 it is not Bourne-compatible when called as zsh, then it really should
 be called in a way that turns on compatibility mode (bash will do this
 when called as sh, but you can also do it with bash --posix).

 AFAIK, if Zsh is called as sh, it too will run in Bourne compatibility
 mode; not sure how to force this compatibility from the command line though
 (albeit I'd guess there is some way to do so).
 [...]
 
 Thanks for digging. I think this case, though, is that we were simply
 using the wrong variable ($SHELL instead of $SHELL_PATH). Your
 workarounds would help if somebody put zsh into $SHELL_PATH, but
 fundamentally that is not a sane thing to be doing, so I think we can
 just consider doing so user error and not bother working around it.
 
FWIW, I agree.

Best regards,
  Stefano

--
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: How do I run tests under Valgrind?

2012-09-21 Thread Stefano Lattarini
On 09/21/2012 09:58 PM, Ramkumar Ramachandra wrote:
 Hi again,
 
 Ramkumar Ramachandra wrote:
 I was able to reproduce the problem on all my machines, and I consider
 this very disturbing.  However, I was successfully able to corner the
 issue. I have an overtly long $PATH that's not getting split properly
 by `IFS=:` in one corner case -- in other words, this shell script
 fails to execute properly when called with `--tee` (just set a really
 long $PATH and try):
 
 Oops.  Looks like it has nothing to do with an overtly long $PATH.  It
 has something to do with $SHELL being zsh though, because other shells
 work.  Looking deeper into this.
 
Zsh doesn't do word-splitting by default on variable expansions:

$ zsh -c 'v=1 2 3; for x in $v; do echo $x; done'
1 2 3

unless you set the SH_WORD_SPLIT option, or put Zsh in Bourne-compatibility
mode somehow:

$ zsh -o SH_WORD_SPLIT -c 'v=1 2 3; for x in $v; do echo $x; done'
1
2
3

$ zsh -c 'emulate sh; v=1 2 3; for x in $v; do echo $x; done'
1
2
3

More info at: http://zsh.sourceforge.net/FAQ/zshfaq02.html

HTH,
  Stefano
--
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 3/3] Color skipped tests bold blue

2012-09-20 Thread Stefano Lattarini
Hi Adam.

On 09/20/2012 11:08 AM, Adam Spiers wrote:
 Skipped tests indicate incomplete test coverage.  Whilst this is
 not a test failure or other error, it's still not complete
 success, so according to the universal traffic lights coloring
 scheme, yellow/brown seems more suitable than green.  However,
 it's more informational than cautionary, so instead we use blue
 which is a universal color for information signs.  Bold blue
 should work better on both black and white backgrounds than
 normal blue.

A very minor nit (feel free to ignore it): IMHO, it should be nice
to state explicitly in the commit message that blue is already used
by other testsuite-related software to highlight skipped tests; you
can report the examples of at least Automake, Autotest and prove --
and extra kudos if you find further examples ;-)

Thanks,
  Stefano

--
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] Make test output coloring more intuitive

2012-09-19 Thread Stefano Lattarini
On 09/17/2012 10:11 PM, Jeff King wrote:
 On Mon, Sep 17, 2012 at 12:50:37PM +0100, Adam Spiers wrote:
 
 The end result of these changes is that:

   - red is _only_ used for things which have gone unexpectedly wrong:
 test failures, unexpected test passes, and failures with the
 framework,

   - yellow is _only_ used for known breakages, and

   - green is _only_ used for things which have gone to plan and
 require no further work to be done.
 
 Sounds reasonable, and I think the new output looks nice. I notice that
 skipped tests are still in green. I wonder if they should be in yellow,
 too.

What about blue instead?   This would keep the colouring scheme more
consistent with the one used by prove:
  http://search.cpan.org/~ovid/Test-Harness/bin/prove
by autotest:
  http://www.gnu.org/software/autoconf/manual/autoconf.html#Using-Autotest
and by the Automake-generated test harness:
  
http://www.gnu.org/software/automake/manual/automake.html#Scripts_002dbased-Testsuites

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


[PATCH 1/2] build: improve GIT_CONF_SUBST signature

2012-09-11 Thread Stefano Lattarini
Now, in configure.ac, a call like:

GIT_CONF_SUBST([FOO])

will be considered equivalent to:

GIT_CONF_SUBST([FOO], [$FOO])

This is mostly a preparatory refactoring in view of future changes.
No semantic change to the generated configure or config.mak.auto is
intended.

Signed-off-by: Stefano Lattarini stefano.lattar...@gmail.com
---
 configure.ac | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/configure.ac b/configure.ac
index df7e376..450bbe7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -7,8 +7,9 @@
 # 
 # Cause the line VAR=VAL to be eventually appended to ${config_file}.
 AC_DEFUN([GIT_CONF_SUBST],
-   [AC_REQUIRE([GIT_CONF_SUBST_INIT])
-   config_appended_defs=$config_appended_defs${newline}$1=$2])
+[AC_REQUIRE([GIT_CONF_SUBST_INIT])
+config_appended_defs=$config_appended_defs${newline}dnl
+$1=m4_if([$#],[1],[${$1}],[$2])])
 
 # GIT_CONF_SUBST_INIT
 # ---
@@ -179,7 +180,7 @@ AC_ARG_WITH([lib],
   else
lib=$withval
AC_MSG_NOTICE([Setting lib to '$lib'])
-   GIT_CONF_SUBST([lib], [$withval])
+   GIT_CONF_SUBST([lib])
   fi])
 
 if test -z $lib; then
@@ -215,7 +216,7 @@ AC_ARG_ENABLE([jsmin],
 [
   JSMIN=$enableval;
   AC_MSG_NOTICE([Setting JSMIN to '$JSMIN' to enable JavaScript minifying])
-  GIT_CONF_SUBST([JSMIN], [$enableval])
+  GIT_CONF_SUBST([JSMIN])
 ])
 
 # Define option to enable CSS minification
@@ -225,7 +226,7 @@ AC_ARG_ENABLE([cssmin],
 [
   CSSMIN=$enableval;
   AC_MSG_NOTICE([Setting CSSMIN to '$CSSMIN' to enable CSS minifying])
-  GIT_CONF_SUBST([CSSMIN], [$enableval])
+  GIT_CONF_SUBST([CSSMIN])
 ])
 
 ## Site configuration (override autodetection)
@@ -265,8 +266,8 @@ AS_HELP_STRING([],   [ARG can be also prefix for 
libpcre library and hea
 else
USE_LIBPCRE=YesPlease
LIBPCREDIR=$withval
-   AC_MSG_NOTICE([Setting LIBPCREDIR to $withval])
-   GIT_CONF_SUBST([LIBPCREDIR], [$withval])
+   AC_MSG_NOTICE([Setting LIBPCREDIR to $LIBPCREDIR])
+   GIT_CONF_SUBST([LIBPCREDIR])
 fi)
 #
 # Define NO_CURL if you do not have curl installed.  git-http-pull and
-- 
1.7.12.317.g1c54b74

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


[PATCH 2/2] build: don't duplicate substitution of make variables

2012-09-11 Thread Stefano Lattarini
Thanks to our 'GIT_CONF_SUBST' layer in configure.ac, a make variable 'VAR'
can be defined to a value 'VAL' at ./configure runtime in our build system
simply by using GIT_CONF_SUBST([VAR], [VAL]) in configure.ac, rather than
having both to call AC_SUBST([VAR], [VAL]) in configure.ac and adding the
'VAR = @VAR@' definition in config.mak.in.  Less duplication, less margin
for error, less possibility of confusion.

While at it, fix some formatting issues in configure.ac that unnecessarily
obscured the code flow.

Signed-off-by: Stefano Lattarini stefano.lattar...@gmail.com
---
 config.mak.in |  49 
 configure.ac  | 144 +++---
 2 files changed, 76 insertions(+), 117 deletions(-)

diff --git a/config.mak.in b/config.mak.in
index 802d342..69d4838 100644
--- a/config.mak.in
+++ b/config.mak.in
@@ -5,12 +5,10 @@ CC = @CC@
 CFLAGS = @CFLAGS@
 CPPFLAGS = @CPPFLAGS@
 LDFLAGS = @LDFLAGS@
-CC_LD_DYNPATH = @CC_LD_DYNPATH@
 AR = @AR@
 TAR = @TAR@
 DIFF = @DIFF@
 #INSTALL = @INSTALL@   # needs install-sh or install.sh in sources
-TCLTK_PATH = @TCLTK_PATH@
 
 prefix = @prefix@
 exec_prefix = @exec_prefix@
@@ -27,50 +25,3 @@ VPATH = @srcdir@
 
 export exec_prefix mandir
 export srcdir VPATH
-
-NEEDS_SSL_WITH_CRYPTO=@NEEDS_SSL_WITH_CRYPTO@
-NO_OPENSSL=@NO_OPENSSL@
-NO_CURL=@NO_CURL@
-NO_EXPAT=@NO_EXPAT@
-NO_LIBGEN_H=@NO_LIBGEN_H@
-HAVE_PATHS_H=@HAVE_PATHS_H@
-HAVE_LIBCHARSET_H=@HAVE_LIBCHARSET_H@
-NO_GETTEXT=@NO_GETTEXT@
-LIBC_CONTAINS_LIBINTL=@LIBC_CONTAINS_LIBINTL@
-NEEDS_LIBICONV=@NEEDS_LIBICONV@
-NEEDS_SOCKET=@NEEDS_SOCKET@
-NEEDS_RESOLV=@NEEDS_RESOLV@
-NEEDS_LIBGEN=@NEEDS_LIBGEN@
-NO_SYS_SELECT_H=@NO_SYS_SELECT_H@
-NO_D_INO_IN_DIRENT=@NO_D_INO_IN_DIRENT@
-NO_D_TYPE_IN_DIRENT=@NO_D_TYPE_IN_DIRENT@
-NO_SOCKADDR_STORAGE=@NO_SOCKADDR_STORAGE@
-NO_IPV6=@NO_IPV6@
-NO_HSTRERROR=@NO_HSTRERROR@
-NO_STRCASESTR=@NO_STRCASESTR@
-NO_STRTOK_R=@NO_STRTOK_R@
-NO_FNMATCH=@NO_FNMATCH@
-NO_FNMATCH_CASEFOLD=@NO_FNMATCH_CASEFOLD@
-NO_MEMMEM=@NO_MEMMEM@
-NO_STRLCPY=@NO_STRLCPY@
-NO_UINTMAX_T=@NO_UINTMAX_T@
-NO_STRTOUMAX=@NO_STRTOUMAX@
-NO_SETENV=@NO_SETENV@
-NO_UNSETENV=@NO_UNSETENV@
-NO_MKDTEMP=@NO_MKDTEMP@
-NO_MKSTEMPS=@NO_MKSTEMPS@
-NO_INET_NTOP=@NO_INET_NTOP@
-NO_INET_PTON=@NO_INET_PTON@
-NO_ICONV=@NO_ICONV@
-OLD_ICONV=@OLD_ICONV@
-NO_REGEX=@NO_REGEX@
-USE_LIBPCRE=@USE_LIBPCRE@
-NO_DEFLATE_BOUND=@NO_DEFLATE_BOUND@
-INLINE=@INLINE@
-SOCKLEN_T=@SOCKLEN_T@
-FREAD_READS_DIRECTORIES=@FREAD_READS_DIRECTORIES@
-SNPRINTF_RETURNS_BOGUS=@SNPRINTF_RETURNS_BOGUS@
-NO_PTHREADS=@NO_PTHREADS@
-PTHREAD_CFLAGS=@PTHREAD_CFLAGS@
-PTHREAD_LIBS=@PTHREAD_LIBS@
-CHARSET_LIB=@CHARSET_LIB@
diff --git a/configure.ac b/configure.ac
index 450bbe7..da1f41f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -267,6 +267,8 @@ AS_HELP_STRING([],   [ARG can be also prefix for 
libpcre library and hea
USE_LIBPCRE=YesPlease
LIBPCREDIR=$withval
AC_MSG_NOTICE([Setting LIBPCREDIR to $LIBPCREDIR])
+dnl USE_LIBPCRE can still be modified below, so don't substitute
+dnl it yet.
GIT_CONF_SUBST([LIBPCREDIR])
 fi)
 #
@@ -387,9 +389,10 @@ AC_MSG_NOTICE([CHECKS for programs])
 AC_PROG_CC([cc gcc])
 AC_C_INLINE
 case $ac_cv_c_inline in
-  inline | yes | no)   ;;
-  *)   AC_SUBST([INLINE], [$ac_cv_c_inline]) ;;
+  inline | yes | no) INLINE='';;
+  *) INLINE=$ac_cv_c_inline ;;
 esac
+GIT_CONF_SUBST([INLINE])
 
 # which switch to pass runtime path to dynamic libraries to the linker
 AC_CACHE_CHECK([if linker supports -R], git_cv_ld_dashr, [
@@ -399,7 +402,7 @@ AC_CACHE_CHECK([if linker supports -R], git_cv_ld_dashr, [
LDFLAGS=${SAVE_LDFLAGS}
 ])
 if test $git_cv_ld_dashr = yes; then
-   AC_SUBST(CC_LD_DYNPATH, [-R])
+   CC_LD_DYNPATH=-R
 else
AC_CACHE_CHECK([if linker supports -Wl,-rpath,], git_cv_ld_wl_rpath, [
   SAVE_LDFLAGS=${LDFLAGS}
@@ -408,7 +411,7 @@ else
   LDFLAGS=${SAVE_LDFLAGS}
])
if test $git_cv_ld_wl_rpath = yes; then
-  AC_SUBST(CC_LD_DYNPATH, [-Wl,-rpath,])
+  CC_LD_DYNPATH=-Wl,-rpath
else
   AC_CACHE_CHECK([if linker supports -rpath], git_cv_ld_rpath, [
  SAVE_LDFLAGS=${LDFLAGS}
@@ -417,32 +420,35 @@ else
  LDFLAGS=${SAVE_LDFLAGS}
   ])
   if test $git_cv_ld_rpath = yes; then
- AC_SUBST(CC_LD_DYNPATH, [-rpath])
+ CC_LD_DYNPATH=-rpath
   else
+ CC_LD_DYNPATH=
  AC_MSG_WARN([linker does not support runtime path to dynamic 
libraries])
   fi
fi
 fi
+GIT_CONF_SUBST([CC_LD_DYNPATH])
 #AC_PROG_INSTALL   # needs install-sh or install.sh in sources
 AC_CHECK_TOOLS(AR, [gar ar], :)
 AC_CHECK_PROGS(TAR, [gtar tar])
 AC_CHECK_PROGS(DIFF, [gnudiff gdiff diff])
 # TCLTK_PATH will be set to some value if we want Tcl/Tk
 # or will be empty otherwise.
-if test -z $NO_TCLTK; then
+if test -n $NO_TCLTK; then
+  TCLTK_PATH=
+else
   if test $with_tcltk = ; then
   # No Tcl/Tk switches given. Do not check

Re: [PATCH 2/2] build: don't duplicate substitution of make variables

2012-09-11 Thread Stefano Lattarini
On 09/11/2012 07:27 PM, Junio C Hamano wrote:
 Stefano Lattarini stefano.lattar...@gmail.com writes:
 
 Thanks to our 'GIT_CONF_SUBST' layer in configure.ac, a make variable 'VAR'
 can be defined to a value 'VAL' at ./configure runtime in our build system
 simply by using GIT_CONF_SUBST([VAR], [VAL]) in configure.ac, rather than
 having both to call AC_SUBST([VAR], [VAL]) in configure.ac and adding the
 'VAR = @VAR@' definition in config.mak.in.  Less duplication, less margin
 for error, less possibility of confusion.

 While at it, fix some formatting issues in configure.ac that unnecessarily
 obscured the code flow.

 Signed-off-by: Stefano Lattarini stefano.lattar...@gmail.com
 ---
  config.mak.in |  49 
  configure.ac  | 144 
 +++---
  2 files changed, 76 insertions(+), 117 deletions(-)
 
 Whoa ;-).

Well, I could have converted one variable at the time, but that seemed
an overkill :-)

 diff --git a/configure.ac b/configure.ac
 index 450bbe7..da1f41f 100644
 --- a/configure.ac
 +++ b/configure.ac
 @@ -267,6 +267,8 @@ AS_HELP_STRING([],   [ARG can be also prefix for 
 libpcre library and hea
  USE_LIBPCRE=YesPlease
  LIBPCREDIR=$withval
  AC_MSG_NOTICE([Setting LIBPCREDIR to $LIBPCREDIR])
 +dnl USE_LIBPCRE can still be modified below, so don't substitute
 +dnl it yet.
  GIT_CONF_SUBST([LIBPCREDIR])
  fi)
  #
 ...
  AC_CHECK_FUNC([hstrerror],
 -[],
 +[],
 
 Is there some consistent policy regarding SP vs HT in the
 indentation you are using in this patch?

Basically I'm trying to follow the style of the surrounding code, while
keeping in mind that in the Git codebase tabs seem to be preferred to spaces.
In this case, the indentation of the following text (that was the meat of
the expression) seemed to favour 4 spaces for indentation, so I followed
suit.

 These two hunks suggest
 that you may be favoring spaces, but other places you seem to use
 tabs, so...

I can convert the new tabs to spaces if you prefer (that would have been
my preference too, but thought trying to follow the Git preferences
was more important).  No big deal either way for me.

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


Re: [PATCH 2/2] build: don't duplicate substitution of make variables

2012-09-11 Thread Stefano Lattarini
On 09/11/2012 09:52 PM, Junio C Hamano wrote:
 Stefano Lattarini stefano.lattar...@gmail.com writes:
 
 On 09/11/2012 07:27 PM, Junio C Hamano wrote:

 These two hunks suggest that you may be favoring spaces, but other
 places you seem to use tabs, so...

 I can convert the new tabs to spaces if you prefer (that would have been
 my preference too, but thought trying to follow the Git preferences
 was more important).  No big deal either way for me.
 
 If this were other parts of the system, my preference would be to
 use tabs, but because I do not help very much in the autoconf part
 myself, I do not have a particular preference.  If it is more common
 to indent the configure.ac script with spaces,

There is no general standard about it that I know of; it's just that
GNU projects tend to prefer space-based indentation over tab-based one,
and since I mostly touch configure.ac files from GNU projects, I've
picked up the habit.

 that would be more
 familiar to the folks who work on it, and I do not have much against
 choosing and sticking to space indented configure.ac file if that is
 the policy.
 
Then I might send a patch that normalize indentation in configure.ac
to spaces only, if that's OK with you.  But that's obviously for a
separated thread.

 But if this patch is not about cleaning up the style to make it
 conform to a policy (whichever it is), I would have preferred to see
 a clean-up patch as a separate step, not mixed together with this.

The reason those few clean-ups are mixed into this patch is that the
pre-existing strange indentation style was actually making it more
difficult for me to grasp the code flow; that is, I didn't see them
as a cosmetic change, but as a way to make it easier for me and the
reader to see that my changes were correct and sensible.

 That's all; either way, no big deal.
 
OK.  Just let me know if you'd still prefer to have the indentation
cleanups done by a preparatory patch, and I'll send a re-roll.

Thanks,
  Stefano
--
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: misleading diff-hunk header

2012-08-26 Thread Stefano Lattarini
On 08/25/2012 02:56 PM, Tim Chase wrote:
 On 08/24/12 23:29, Junio C Hamano wrote:
 Tim Chase g...@tim.thechases.com writes:
 If the documented purpose of diff -p (and by proxy
 diff.{type}.xfuncname) is to show the name of the *function*
 containing the changed lines,

 Yeah, the documentation is misleading, but I do not offhand think of
 a better phrasing. Perhaps you could send in a patch to improve it.

 How does GNU manual explain the option?
 
 Tersely. :-)
 
-p  --show-c-function
   Show which C function each change is in.

That's in the manpage, which is basically just a copy of the output from
diff --help.  In the texinfo manual (which is the real documentation),
there are additional explanations, saying, among other things:

To show in which functions differences occur for C and similar languages,
you can use the --show-c-function (-p) option. This option automatically
defaults to the context output format (see Context Format), with the
default number of lines of context. You can override that number with
-C lines elsewhere in the command line. You can override both the format
and the number with -U lines elsewhere in the command line.
The -p option is equivalent to -F '^[[:alpha:]$_]' if the unified format
is specified, otherwise -c -F '^[[:alpha:]$_]' (see Specified Headings).
GNU diff provides this option for the sake of convenience.
...
The --show-function-line (-F) option finds the nearest unchanged line
that precedes each hunk of differences and matches the given regular
expression.

You can find more information in the on-line documentation:

  http://www.gnu.org/software/diffutils/manual/diffutils.html

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


Re: [PATCH v4 5/7] i18n: am: mark more strings for translation

2012-07-25 Thread Stefano Lattarini
Sorry to be so nit-picky, but ...

On 07/25/2012 05:53 AM, Jiang Xin wrote:
 Mark strings in 'git-am.sh' for translation. In the last chunk, I
 changed '$1' to '-b/--binary' for this reason:
 
  * First, if there is a variable in the l10n message, we could not use
gettext. Because the variable will be expanded (evaluated) and will
never match the entry in the po file.
 
  * Second, if there is a positional parameter ($1, $2,...) in the
message, we could not use eval_gettext either. Because
eval_gettext may be a wapper for gettext, and the positional
parameter would loose it's original context.

... here you should s/it's/its/.

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


Re: [PATCH 3/3] difftool: Disable --symlinks on cygwin

2012-07-25 Thread Stefano Lattarini
On 07/25/2012 05:14 AM, David Aguilar wrote:
 Symlinks are not ubiquitous on Windows so make --no-symlinks the default.
 
 Signed-off-by: David Aguilar dav...@gmail.com
 ---
 I don't have cygwin so I can't verify this one myself.
 Is 'cygwin' really the value of $^O there?

Apparently yes:

  $ uname -rso
  CYGWIN_NT-5.1 1.5.25(0.156/4/2) Cygwin
  $ perl -e 'print $^O'
  cygwin

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


Re: [PATCH] test: some testcases failed if cwd is on a symlink

2012-07-24 Thread Stefano Lattarini
Some grammatical nits about the commit message.  I hope this doesn't
come across as too picky/annoying ...  And you might want to wait for
a native to confirm whether these nits are actually all warranted.

On 07/24/2012 10:00 AM, Jiang Xin wrote:
 Run

s/Run/Running/

 command 'git rev-parse --git-dir' under subdir

s/under subdir/under a subdir/.  Or even better IMHO,
s/under subdir/in a subdir/

 will return realpath

s/realpath/the realpath/

 of '.git' directory.

s/of/of the/

 Some test scripts compare this realpath against
 $TRASH_DIRECTORY, they are not equal

s/they are not/but they are not/

 if current working directory is on a symlink.

s/current/the current/

 In this fix, get realpath

s/realpath/the realpath/

 of $TRASH_DIRECTORY, store it in
 $TRASH_REALPATH variable, and use it when necessary.
 
 Signed-off-by: Jiang Xin worldhello@gmail.com
 ---
  t/t4035-diff-quiet.sh  |  8 +---
  t/t9903-bash-prompt.sh | 13 +++--
  2 个文件被修改,插入 12 行(+),删除 9 行(-)
 
 diff --git a/t/t4035-diff-quiet.sh b/t/t4035-diff-quiet.sh
 index 23141..5855 100755
 --- a/t/t4035-diff-quiet.sh
 +++ b/t/t4035-diff-quiet.sh
 @@ -4,6 +4,8 @@ test_description='Return value of diffs'
  
  . ./test-lib.sh
  
 +TRASH_REALPATH=$(cd $TRASH_DIRECTORY; pwd -P)
 +
BTW, the outer quotes are not needed; this is enough:

TRASH_REALPATH=$(cd $TRASH_DIRECTORY; pwd -P)

Regards,
  Stefano
--
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/7] i18n: New keywords for xgettext extraction from sh

2012-07-24 Thread Stefano Lattarini
On 07/24/2012 08:59 AM, Jiang Xin wrote:
 Since we have additional shell wrappers (gettextln and eval_gettextln)
 for gettext, we need to take into account these wrappers when run

s/when run/when running/ or s/when we run/.

Sorry for not spotting that in my first review!

 'make pot' to extract messages from shell scripts.
 

Regards,
  Stefano
--
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 2/7] i18n: rebase: mark strings for translation

2012-07-24 Thread Stefano Lattarini
On 07/24/2012 08:59 AM, Jiang Xin wrote:
 Mark strings in git-rebase.sh for translation.
 
 Some test scripts are affected by this update, and would fail if are

s/if are/if/

 tested with GETTEXT_POISON switch turned on. Use i18n-specific test

s/Use/Using/, or s/Use/Use of/

 functions, such as test_i18ngrep

Missing comma after 'test_i18ngrep' here.

 in the related test scripts will fix these issues.
 

Regards,
  Stefano
--
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 7/7] i18n: merge-recursive: mark strings for translation

2012-07-24 Thread Stefano Lattarini
On 07/24/2012 08:59 AM, Jiang Xin wrote:
 Mark strings in merge-recursive for translation.
 
 Some test scripts are affected by this update, and would fail if are
 tested with GETTEXT_POISON switch turned on. Use i18n-specific test
 functions, such as test_i18ngrep in the related test scripts will fix
 these issues.

The same comments I made for [PATCH 2/7] in this series apply here.

Regards,
  Stefano

--
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 7/7] build: reconfigure automatically if configure.ac changes

2012-07-23 Thread Stefano Lattarini
Hi Junio.  I've just noticed a minor typo in the commit message ...

On 07/19/2012 09:50 AM, Stefano Lattarini wrote:
 This provides a reduced but still useful sibling of the Automake's
 automatic Makefile rebuild feature.  It's important to note that
 we take care to enable the new rules only if the tree that has already
 be

... here, it should read been, not be.  Can you fix that locally
before merging to 'next', or should I send a re-roll?

 configured with './configure', so that users relying on manual
 configuration won't be negatively impacted.

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


Re: [PATCH 1/7] i18n: New keywords for xgettext extraction from sh

2012-07-21 Thread Stefano Lattarini
On 07/21/2012 05:50 PM, Jiang Xin wrote:
 Since we have additional shell wrappers (gettextln and eval_gettextln)
 for gettext, we need to take into account these wrapers

s/wrapers/wrappers/

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


Re: [PATCH 4/7] Remove obsolete LONG_USAGE which breaks xgettext

2012-07-21 Thread Stefano Lattarini
On 07/21/2012 05:50 PM, Jiang Xin wrote:
 The obsolete LONG_USAGE variable has the following message in it:
 
 A'\''--B'\''--C'\''
 
 And such complex LONG_USAGE message will breaks xgettext when extract

s/extract/extracting/ I think.

 l10n messages. But if remove single quotes from the message,

s/remove/we remove/; or, if the passive voice is not a problem, you might
reformulate the sentence as follows:

   But if single quotes are removed from the message, ...

 xgettext works fine on 'git-rebase.sh'.
 
 Since there is a mordern

s/mordern/modern/


 OPTIONS_SPEC variable in use in this script,
 it's safe to remove the obsolte

s/obsolte/obsolete/

 USAGE and LONG_USAGE variables.
 

Regards,
  Stefano
--
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 5/7] i18n: am: mark more strings for translation

2012-07-21 Thread Stefano Lattarini
On 07/21/2012 05:50 PM, Jiang Xin wrote:
 Signed-off-by: Jiang Xin worldhello@gmail.com
 ---
  git-am.sh | 11 +--
  1 file changed, 5 insertions(+), 6 deletions(-)
 
 diff --git a/git-am.sh b/git-am.sh
 index b6a53..20c1a 100755
 --- a/git-am.sh
 +++ b/git-am.sh
 @@ -92,7 +92,7 @@ safe_to_abort () {
   then
   return 0
   fi
 - gettextln You seem to have moved HEAD since the last 'am' 
 failure.
 + gettextln You seem to have moved HEAD since the last 'am' failure.

Spurious whitespace change?  It certainly is unrelated with what you say in
the commit message ...

Regards,
  Stefano
--
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 6/7] Remove unused and bad gettext block from git-am

2012-07-21 Thread Stefano Lattarini
On 07/21/2012 05:50 PM, Jiang Xin wrote:
 Gettext message

s/message/messages/ I think.

 should not start with '-' nor '--'. Since the '-d' and
 '--dotest' options are not exist

s/are not/do not/

 in OPTIONS_SPEC variable,

s/OPTIONS_SPEC/the OPTIONS_SPEC/

 so it's safe to remove the block.

This so is redundant, in light of the earlier Since.  I'd just remove it.

Regards,
  Stefano
--
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 6/7] build: make clean should not remove configure-generated files

2012-07-19 Thread Stefano Lattarini
On 07/19/2012 08:56 AM, Matthieu Moy wrote:
 Stefano Lattarini stefano.lattar...@gmail.com writes:
 
 for example, an autotools old-timer that has run:

 ./configure --prefix /opt/git

 in the past, without running make distclean afterwards, would
 expect a make install issued after a make clean to rebuild and
 install git in '/opt/git';
 
 I've been hit by that behavior once. Thanks for fixing it. The patch
 looks good.
 
Should I add Acked-by: Matthieu Moy then?  (Sorry if it's a dumb
question, but I'm not sure which the preferred policy is around here).

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


Re: [PATCH 2/7] autoconf: GIT_CONF_APPEND_LINE - GIT_CONF_SUBST

2012-07-19 Thread Stefano Lattarini
On 07/19/2012 02:13 AM, Junio C Hamano wrote:
 Stefano Lattarini stefano.lattar...@gmail.com writes:
 
 The new name fits better with the macro signature, and underlines the
 similarities with the autoconf-provided macro AC_SUBST (which will be
 made even more pronounced in planned future commits).

 Once again, no semantic change is intended, and indeed no change to the
 generated configure script is expected.

 Signed-off-by: Stefano Lattarini stefano.lattar...@gmail.com
 ---
  configure.ac | 24 
  1 file changed, 12 insertions(+), 12 deletions(-)

 diff --git a/configure.ac b/configure.ac
 index 14c7960..789926f 100644
 --- a/configure.ac
 +++ b/configure.ac
 @@ -3,10 +3,10 @@
  
  ## Definitions of private macros.
  
 -# GIT_CONF_APPEND_LINE(LINE)
 +# GIT_CONF_SUBST(LINE)
 
 I see that [PATCH 1/7] needs to be updated so that it describes the
 new two-argument form of GIT_CONF_APPEND_LINE(VAR, VAL), and this
 patch needs to be updated for GIT_CONF_SUBST() with the same.

Oops, you're right.  I will fix that in the re-roll.

  # --
  # Append LINE to file ${config_append}
 
 Also the description definitely wants to be updated; it is no longer
 LINEness that matters.
 
 Other than that, 1  2 looked very nice and sensible.

Thanks!

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


[PATCH v2 0/7] build system: support automatic reconfiguration for autotools user

2012-07-19 Thread Stefano Lattarini
 Except for miniscule nits in the the bottom two, I didn't find
 anything objectionable---nicely done.

Thanks.

Here is the re-roll, which should address all the reported nits.

Stefano Lattarini (7):
  autoconf: GIT_CONF_APPEND_LINE: change signature
  autoconf: GIT_CONF_APPEND_LINE - GIT_CONF_SUBST
  autoconf: remove some redundant shell indirections
  autoconf: remove few redundant semicolons
  autoconf: use AC_CONFIG_COMMANDS instead of ad-hoc 'config.mak.append'
  build: make clean should not remove configure-generated files
  build: reconfigure automatically if configure.ac changes

 Makefile | 17 +++--
 configure.ac | 56 +++-
 2 files changed, 46 insertions(+), 27 deletions(-)

-- 
1.7.10.2.1067.g553d16e

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


[PATCH v2 1/7] autoconf: GIT_CONF_APPEND_LINE: change signature

2012-07-19 Thread Stefano Lattarini
From:

   GIT_CONF_APPEND_LINE([VAR=VAL])

to:

   GIT_CONF_APPEND_LINE([VAR], [VAL])

This is only a preparatory change in view of future refactorings.
No semantic change is intended.  In fact, the generated configure
file doesn't change at all.

Signed-off-by: Stefano Lattarini stefano.lattar...@gmail.com
---
 configure.ac | 28 ++--
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/configure.ac b/configure.ac
index 4e9012f..5f63269 100644
--- a/configure.ac
+++ b/configure.ac
@@ -3,11 +3,11 @@
 
 ## Definitions of private macros.
 
-# GIT_CONF_APPEND_LINE(LINE)
-# --
-# Append LINE to file ${config_append}
+# GIT_CONF_APPEND_LINE(VAL, VAR)
+# --
+# Append the line VAR=VAL to file ${config_append}
 AC_DEFUN([GIT_CONF_APPEND_LINE],
- [echo $1  ${config_append}])
+ [echo $1=$2  ${config_append}])
 
 # GIT_ARG_SET_PATH(PROGRAM)
 # -
@@ -34,8 +34,8 @@ AC_DEFUN([GIT_CONF_APPEND_PATH],
if test -n $2; then
GIT_UC_PROGRAM[]_PATH=$withval
AC_MSG_NOTICE([Disabling use of ${PROGRAM}])
-   GIT_CONF_APPEND_LINE(NO_${PROGRAM}=YesPlease)
-   GIT_CONF_APPEND_LINE(${PROGRAM}_PATH=)
+   GIT_CONF_APPEND_LINE([NO_${PROGRAM}], [YesPlease])
+   GIT_CONF_APPEND_LINE([${PROGRAM}_PATH], [])
else
AC_MSG_ERROR([You cannot use git without $1])
fi
@@ -45,7 +45,7 @@ AC_DEFUN([GIT_CONF_APPEND_PATH],
else
GIT_UC_PROGRAM[]_PATH=$withval
AC_MSG_NOTICE([Setting GIT_UC_PROGRAM[]_PATH to $withval])
-   GIT_CONF_APPEND_LINE(${PROGRAM}_PATH=$withval)
+   GIT_CONF_APPEND_LINE([${PROGRAM}_PATH], [$withval])
fi
 fi
 m4_popdef([GIT_UC_PROGRAM])])
@@ -67,7 +67,7 @@ AC_DEFUN([GIT_PARSE_WITH],
NO_[]GIT_UC_PACKAGE=
GIT_UC_PACKAGE[]DIR=$withval
AC_MSG_NOTICE([Setting GIT_UC_PACKAGE[]DIR to $withval])
-   GIT_CONF_APPEND_LINE(${PACKAGE}DIR=$withval)
+   GIT_CONF_APPEND_LINE([${PACKAGE}DIR], [$withval])
 fi
 m4_popdef([GIT_UC_PACKAGE])])
 
@@ -87,7 +87,7 @@ AC_DEFUN([GIT_PARSE_WITH_SET_MAKE_VAR],
 [a value for $1 ($2).  Maybe you do...?])
   fi
   AC_MSG_NOTICE([Setting $2 to $withval])
-  GIT_CONF_APPEND_LINE($2=$withval)
+  GIT_CONF_APPEND_LINE([$2], [$withval])
  fi)])# GIT_PARSE_WITH_SET_MAKE_VAR
 
 #
@@ -150,7 +150,7 @@ AC_ARG_WITH([sane-tool-path],
   else
 AC_MSG_NOTICE([Setting SANE_TOOL_PATH to '$withval'])
   fi
-  GIT_CONF_APPEND_LINE([SANE_TOOL_PATH=$withval])],
+  GIT_CONF_APPEND_LINE([SANE_TOOL_PATH], [$withval])],
   [# If the --with-sane-tool-path option was not given, don't touch
# SANE_TOOL_PATH here, but let defaults in Makefile take care of it.
# This should minimize spurious differences in the behaviour of the
@@ -169,7 +169,7 @@ AC_ARG_WITH([lib],
   else
lib=$withval
AC_MSG_NOTICE([Setting lib to '$lib'])
-   GIT_CONF_APPEND_LINE(lib=$withval)
+   GIT_CONF_APPEND_LINE([lib], [$withval])
   fi])
 
 if test -z $lib; then
@@ -205,7 +205,7 @@ AC_ARG_ENABLE([jsmin],
 [
   JSMIN=$enableval;
   AC_MSG_NOTICE([Setting JSMIN to '$JSMIN' to enable JavaScript minifying])
-  GIT_CONF_APPEND_LINE(JSMIN=$enableval);
+  GIT_CONF_APPEND_LINE([JSMIN], [$enableval]);
 ])
 
 # Define option to enable CSS minification
@@ -215,7 +215,7 @@ AC_ARG_ENABLE([cssmin],
 [
   CSSMIN=$enableval;
   AC_MSG_NOTICE([Setting CSSMIN to '$CSSMIN' to enable CSS minifying])
-  GIT_CONF_APPEND_LINE(CSSMIN=$enableval);
+  GIT_CONF_APPEND_LINE([CSSMIN], [$enableval]);
 ])
 
 ## Site configuration (override autodetection)
@@ -256,7 +256,7 @@ AS_HELP_STRING([],   [ARG can be also prefix for 
libpcre library and hea
USE_LIBPCRE=YesPlease
LIBPCREDIR=$withval
AC_MSG_NOTICE([Setting LIBPCREDIR to $withval])
-   GIT_CONF_APPEND_LINE(LIBPCREDIR=$withval)
+   GIT_CONF_APPEND_LINE([LIBPCREDIR], [$withval])
 fi)
 #
 # Define NO_CURL if you do not have curl installed.  git-http-pull and
-- 
1.7.10.2.1067.g553d16e

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


[PATCH v2 2/7] autoconf: GIT_CONF_APPEND_LINE - GIT_CONF_SUBST

2012-07-19 Thread Stefano Lattarini
The new name fits better with the macro signature, and underlines the
similarities with the autoconf-provided macro AC_SUBST (which will be
made even more pronounced in planned future commits).

Once again, no semantic change is intended, and indeed no change to the
generated configure script is expected.

Signed-off-by: Stefano Lattarini stefano.lattar...@gmail.com
---
 configure.ac | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/configure.ac b/configure.ac
index 5f63269..02b9a49 100644
--- a/configure.ac
+++ b/configure.ac
@@ -3,8 +3,8 @@
 
 ## Definitions of private macros.
 
-# GIT_CONF_APPEND_LINE(VAL, VAR)
-# --
+# GIT_CONF_SUBST(VAL, VAR)
+# 
 # Append the line VAR=VAL to file ${config_append}
 AC_DEFUN([GIT_CONF_APPEND_LINE],
  [echo $1=$2  ${config_append}])
@@ -34,8 +34,8 @@ AC_DEFUN([GIT_CONF_APPEND_PATH],
if test -n $2; then
GIT_UC_PROGRAM[]_PATH=$withval
AC_MSG_NOTICE([Disabling use of ${PROGRAM}])
-   GIT_CONF_APPEND_LINE([NO_${PROGRAM}], [YesPlease])
-   GIT_CONF_APPEND_LINE([${PROGRAM}_PATH], [])
+   GIT_CONF_SUBST([NO_${PROGRAM}], [YesPlease])
+   GIT_CONF_SUBST([${PROGRAM}_PATH], [])
else
AC_MSG_ERROR([You cannot use git without $1])
fi
@@ -45,7 +45,7 @@ AC_DEFUN([GIT_CONF_APPEND_PATH],
else
GIT_UC_PROGRAM[]_PATH=$withval
AC_MSG_NOTICE([Setting GIT_UC_PROGRAM[]_PATH to $withval])
-   GIT_CONF_APPEND_LINE([${PROGRAM}_PATH], [$withval])
+   GIT_CONF_SUBST([${PROGRAM}_PATH], [$withval])
fi
 fi
 m4_popdef([GIT_UC_PROGRAM])])
@@ -67,7 +67,7 @@ AC_DEFUN([GIT_PARSE_WITH],
NO_[]GIT_UC_PACKAGE=
GIT_UC_PACKAGE[]DIR=$withval
AC_MSG_NOTICE([Setting GIT_UC_PACKAGE[]DIR to $withval])
-   GIT_CONF_APPEND_LINE([${PACKAGE}DIR], [$withval])
+   GIT_CONF_SUBST([${PACKAGE}DIR], [$withval])
 fi
 m4_popdef([GIT_UC_PACKAGE])])
 
@@ -87,7 +87,7 @@ AC_DEFUN([GIT_PARSE_WITH_SET_MAKE_VAR],
 [a value for $1 ($2).  Maybe you do...?])
   fi
   AC_MSG_NOTICE([Setting $2 to $withval])
-  GIT_CONF_APPEND_LINE([$2], [$withval])
+  GIT_CONF_SUBST([$2], [$withval])
  fi)])# GIT_PARSE_WITH_SET_MAKE_VAR
 
 #
@@ -150,7 +150,7 @@ AC_ARG_WITH([sane-tool-path],
   else
 AC_MSG_NOTICE([Setting SANE_TOOL_PATH to '$withval'])
   fi
-  GIT_CONF_APPEND_LINE([SANE_TOOL_PATH], [$withval])],
+  GIT_CONF_SUBST([SANE_TOOL_PATH], [$withval])],
   [# If the --with-sane-tool-path option was not given, don't touch
# SANE_TOOL_PATH here, but let defaults in Makefile take care of it.
# This should minimize spurious differences in the behaviour of the
@@ -169,7 +169,7 @@ AC_ARG_WITH([lib],
   else
lib=$withval
AC_MSG_NOTICE([Setting lib to '$lib'])
-   GIT_CONF_APPEND_LINE([lib], [$withval])
+   GIT_CONF_SUBST([lib], [$withval])
   fi])
 
 if test -z $lib; then
@@ -205,7 +205,7 @@ AC_ARG_ENABLE([jsmin],
 [
   JSMIN=$enableval;
   AC_MSG_NOTICE([Setting JSMIN to '$JSMIN' to enable JavaScript minifying])
-  GIT_CONF_APPEND_LINE([JSMIN], [$enableval]);
+  GIT_CONF_SUBST([JSMIN], [$enableval]);
 ])
 
 # Define option to enable CSS minification
@@ -215,7 +215,7 @@ AC_ARG_ENABLE([cssmin],
 [
   CSSMIN=$enableval;
   AC_MSG_NOTICE([Setting CSSMIN to '$CSSMIN' to enable CSS minifying])
-  GIT_CONF_APPEND_LINE([CSSMIN], [$enableval]);
+  GIT_CONF_SUBST([CSSMIN], [$enableval]);
 ])
 
 ## Site configuration (override autodetection)
@@ -256,7 +256,7 @@ AS_HELP_STRING([],   [ARG can be also prefix for 
libpcre library and hea
USE_LIBPCRE=YesPlease
LIBPCREDIR=$withval
AC_MSG_NOTICE([Setting LIBPCREDIR to $withval])
-   GIT_CONF_APPEND_LINE([LIBPCREDIR], [$withval])
+   GIT_CONF_SUBST([LIBPCREDIR], [$withval])
 fi)
 #
 # Define NO_CURL if you do not have curl installed.  git-http-pull and
-- 
1.7.10.2.1067.g553d16e

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


[PATCH v2 3/7] autoconf: remove some redundant shell indirections

2012-07-19 Thread Stefano Lattarini
They are merely useless now, but would get in the way of future changes.

No semantic change is intended.

Signed-off-by: Stefano Lattarini stefano.lattar...@gmail.com
---
 configure.ac | 12 +---
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/configure.ac b/configure.ac
index 02b9a49..200776f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -29,13 +29,12 @@ AC_DEFUN([GIT_ARG_SET_PATH],
 # --without-PROGRAM is used.
 AC_DEFUN([GIT_CONF_APPEND_PATH],
 [m4_pushdef([GIT_UC_PROGRAM], m4_toupper([$1]))dnl
-PROGRAM=GIT_UC_PROGRAM
 if test $withval = no; then
if test -n $2; then
GIT_UC_PROGRAM[]_PATH=$withval
-   AC_MSG_NOTICE([Disabling use of ${PROGRAM}])
-   GIT_CONF_SUBST([NO_${PROGRAM}], [YesPlease])
-   GIT_CONF_SUBST([${PROGRAM}_PATH], [])
+   AC_MSG_NOTICE([Disabling use of GIT_UC_PROGRAM])
+   GIT_CONF_SUBST([NO_]GIT_UC_PROGRAM, [YesPlease])
+   GIT_CONF_SUBST(GIT_UC_PROGRAM[]_PATH, [])
else
AC_MSG_ERROR([You cannot use git without $1])
fi
@@ -45,7 +44,7 @@ AC_DEFUN([GIT_CONF_APPEND_PATH],
else
GIT_UC_PROGRAM[]_PATH=$withval
AC_MSG_NOTICE([Setting GIT_UC_PROGRAM[]_PATH to $withval])
-   GIT_CONF_SUBST([${PROGRAM}_PATH], [$withval])
+   GIT_CONF_SUBST(GIT_UC_PROGRAM[]_PATH, [$withval])
fi
 fi
 m4_popdef([GIT_UC_PROGRAM])])
@@ -58,7 +57,6 @@ AC_DEFUN([GIT_CONF_APPEND_PATH],
 # * Unset NO_PACKAGE for --with-PACKAGE without ARG
 AC_DEFUN([GIT_PARSE_WITH],
 [m4_pushdef([GIT_UC_PACKAGE], m4_toupper([$1]))dnl
-PACKAGE=GIT_UC_PACKAGE
 if test $withval = no; then
NO_[]GIT_UC_PACKAGE=YesPlease
 elif test $withval = yes; then
@@ -67,7 +65,7 @@ AC_DEFUN([GIT_PARSE_WITH],
NO_[]GIT_UC_PACKAGE=
GIT_UC_PACKAGE[]DIR=$withval
AC_MSG_NOTICE([Setting GIT_UC_PACKAGE[]DIR to $withval])
-   GIT_CONF_SUBST([${PACKAGE}DIR], [$withval])
+   GIT_CONF_SUBST(GIT_UC_PACKAGE[DIR], [$withval])
 fi
 m4_popdef([GIT_UC_PACKAGE])])
 
-- 
1.7.10.2.1067.g553d16e

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


[PATCH v2 5/7] autoconf: use AC_CONFIG_COMMANDS instead of ad-hoc 'config.mak.append'

2012-07-19 Thread Stefano Lattarini
This will allow ./config.status --recheck; ./config.status to work
correctly as a mean of reconfiguring the tree with the same configure
argument used in the previous ./configure invocation.

Signed-off-by: Stefano Lattarini stefano.lattar...@gmail.com
---
 configure.ac | 28 +---
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/configure.ac b/configure.ac
index b453ba5..a63fe77 100644
--- a/configure.ac
+++ b/configure.ac
@@ -5,9 +5,22 @@
 
 # GIT_CONF_SUBST(VAL, VAR)
 # 
-# Append the line VAR=VAL to file ${config_append}
-AC_DEFUN([GIT_CONF_APPEND_LINE],
- [echo $1=$2  ${config_append}])
+# Cause the line VAR=VAL to be eventually appended to ${config_file}.
+AC_DEFUN([GIT_CONF_SUBST],
+   [AC_REQUIRE([GIT_CONF_SUBST_INIT])
+   config_appended_defs=$config_appended_defs${newline}$1=$2])
+
+# GIT_CONF_SUBST_INIT
+# ---
+# Prepare shell variables and autoconf machine required by later calls
+# to GIT_CONF_SUBST.
+AC_DEFUN([GIT_CONF_SUBST_INIT],
+[config_appended_defs=; newline='
+'
+AC_CONFIG_COMMANDS([$config_file],
+   [echo $config_appended_defs  $config_file],
+   [config_file=$config_file
+config_appended_defs=$config_appended_defs])])
 
 # GIT_ARG_SET_PATH(PROGRAM)
 # -
@@ -133,11 +146,8 @@ AC_INIT([git], [@@GIT_VERSION@@], [git@vger.kernel.org])
 AC_CONFIG_SRCDIR([git.c])
 
 config_file=config.mak.autogen
-config_append=config.mak.append
 config_in=config.mak.in
 
-echo # ${config_append}.  Generated by configure.  ${config_append}
-
 # Directories holding saner versions of common or POSIX binaries.
 AC_ARG_WITH([sane-tool-path],
   [AS_HELP_STRING(
@@ -1041,9 +1051,5 @@ AC_SUBST(PTHREAD_LIBS)
 AC_SUBST(NO_PTHREADS)
 
 ## Output files
-AC_CONFIG_FILES([${config_file}:${config_in}:${config_append}])
+AC_CONFIG_FILES([${config_file}:${config_in}])
 AC_OUTPUT
-
-
-## Cleanup
-rm -f ${config_append}
-- 
1.7.10.2.1067.g553d16e

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


[PATCH v2 7/7] build: reconfigure automatically if configure.ac changes

2012-07-19 Thread Stefano Lattarini
This provides a reduced but still useful sibling of the Automake's
automatic Makefile rebuild feature.  It's important to note that
we take care to enable the new rules only if the tree that has already
be configured with './configure', so that users relying on manual
configuration won't be negatively impacted.

Signed-off-by: Stefano Lattarini stefano.lattar...@gmail.com
---
 Makefile | 12 
 configure.ac |  2 ++
 2 files changed, 14 insertions(+)

diff --git a/Makefile b/Makefile
index 88a76a3..f4e8fac 100644
--- a/Makefile
+++ b/Makefile
@@ -2158,6 +2158,18 @@ configure: configure.ac GIT-VERSION-FILE
autoconf -o $@ $+  \
$(RM) $+
 
+ifdef AUTOCONFIGURED
+config.status: configure
+   $(QUIET_GEN)if test -f config.status; then \
+ ./config.status --recheck; \
+   else \
+ ./configure; \
+   fi
+reconfigure config.mak.autogen: config.status
+   $(QUIET_GEN)./config.status
+.PHONY: reconfigure # This is a convenience target.
+endif
+
 XDIFF_OBJS += xdiff/xdiffi.o
 XDIFF_OBJS += xdiff/xprepare.o
 XDIFF_OBJS += xdiff/xutils.o
diff --git a/configure.ac b/configure.ac
index a63fe77..df7e376 100644
--- a/configure.ac
+++ b/configure.ac
@@ -148,6 +148,8 @@ AC_CONFIG_SRCDIR([git.c])
 config_file=config.mak.autogen
 config_in=config.mak.in
 
+GIT_CONF_SUBST([AUTOCONFIGURED], [YesPlease])
+
 # Directories holding saner versions of common or POSIX binaries.
 AC_ARG_WITH([sane-tool-path],
   [AS_HELP_STRING(
-- 
1.7.10.2.1067.g553d16e

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


[PATCH v2 6/7] build: make clean should not remove configure-generated files

2012-07-19 Thread Stefano Lattarini
Those filed hold variables, settings and information set by the
configuration process run by './configure'; in Autotools-based
build system that kind of stuff should only be removed by
make distclean.  Having it removed by make clean is not only
inconsistent, but causes real confusion for that part of the Git
audience that is used to the Autotools semantics; for example,
an autotools old-timer that has run:

./configure --prefix /opt/git

in the past, without running make distclean afterwards, would
expect a make install issued after a make clean to rebuild and
install git in '/opt/git'; but with the current behaviour, the
make clean invocation removes (among the other things) the file
'config.mak.autogen', so that the make install falls back to the
default prefix of '$HOME', thus installing git in the user's home
directory -- definitely unexpected.

Signed-off-by: Stefano Lattarini stefano.lattar...@gmail.com
---
 Makefile | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 285c660..88a76a3 100644
--- a/Makefile
+++ b/Makefile
@@ -2742,6 +2742,9 @@ dist-doc:
 
 distclean: clean
$(RM) configure
+   $(RM) config.log config.status config.cache
+   $(RM) config.mak.autogen config.mak.append
+   $(RM) -r autom4te.cache
 
 profile-clean:
$(RM) $(addsuffix *.gcda,$(addprefix $(PROFILE_DIR)/, $(object_dirs)))
@@ -2756,8 +2759,6 @@ clean: profile-clean
$(RM) -r $(dep_dirs)
$(RM) -r po/build/
$(RM) *.spec *.pyc *.pyo */*.pyc */*.pyo common-cmds.h $(ETAGS_TARGET) 
tags cscope*
-   $(RM) -r autom4te.cache
-   $(RM) config.log config.mak.autogen config.mak.append config.status 
config.cache
$(RM) -r $(GIT_TARNAME) .doc-tmp-dir
$(RM) $(GIT_TARNAME).tar.gz git-core_$(GIT_VERSION)-*.tar.gz
$(RM) $(htmldocs).tar.gz $(manpages).tar.gz
-- 
1.7.10.2.1067.g553d16e

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


[PATCH v2 4/7] autoconf: remove few redundant semicolons

2012-07-19 Thread Stefano Lattarini
They are merely useless now, but would get in the way of future changes.

No semantic change is intended.

Signed-off-by: Stefano Lattarini stefano.lattar...@gmail.com
---
 configure.ac | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/configure.ac b/configure.ac
index 200776f..b453ba5 100644
--- a/configure.ac
+++ b/configure.ac
@@ -203,7 +203,7 @@ AC_ARG_ENABLE([jsmin],
 [
   JSMIN=$enableval;
   AC_MSG_NOTICE([Setting JSMIN to '$JSMIN' to enable JavaScript minifying])
-  GIT_CONF_SUBST([JSMIN], [$enableval]);
+  GIT_CONF_SUBST([JSMIN], [$enableval])
 ])
 
 # Define option to enable CSS minification
@@ -213,7 +213,7 @@ AC_ARG_ENABLE([cssmin],
 [
   CSSMIN=$enableval;
   AC_MSG_NOTICE([Setting CSSMIN to '$CSSMIN' to enable CSS minifying])
-  GIT_CONF_SUBST([CSSMIN], [$enableval]);
+  GIT_CONF_SUBST([CSSMIN], [$enableval])
 ])
 
 ## Site configuration (override autodetection)
-- 
1.7.10.2.1067.g553d16e

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


[PATCH 0/7] build system: support automatic reconfiguration for autotools user

2012-07-18 Thread Stefano Lattarini
This series aims at improving the user experience for those people who
(like me) use that Autotools-based interface to the build system of Git.

The two actual improvements (equipped with proper explanations and
rationales) are implemented in the last two patches.  The other five
patches are just preparatory changes.

The series as general or as clean as it could actually be, but it's
enough to scratch the itch that motivated me to write it.

Thanks,
  Stefano

Stefano Lattarini (7):
  autoconf: GIT_CONF_APPEND_LINE: change signature
  autoconf: GIT_CONF_APPEND_LINE - GIT_CONF_SUBST
  autoconf: remove some redundant shell indirections
  autoconf: remove few redundant semicolons
  autoconf: use AC_CONFIG_COMMANDS instead of ad-hoc 'config.mak.append'
  build: make clean should not remove configure-generated files
  build: reconfigure automatically if configure.ac changes

 Makefile | 17 +++--
 configure.ac | 53 ++---
 2 files changed, 45 insertions(+), 25 deletions(-)

-- 
1.7.10.2.1067.g553d16e

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


[PATCH 1/7] autoconf: GIT_CONF_APPEND_LINE: change signature

2012-07-18 Thread Stefano Lattarini
From:

   GIT_CONF_APPEND_LINE([VAR=VAL])

to:

   GIT_CONF_APPEND_LINE([VAR], [VAL])

This is only a preparatory change in view of future refactorings.
No semantic change is intended.  In fact, the generated configure
file doesn't change at all.

Signed-off-by: Stefano Lattarini stefano.lattar...@gmail.com
---
 configure.ac | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/configure.ac b/configure.ac
index 4e9012f..14c7960 100644
--- a/configure.ac
+++ b/configure.ac
@@ -7,7 +7,7 @@
 # --
 # Append LINE to file ${config_append}
 AC_DEFUN([GIT_CONF_APPEND_LINE],
- [echo $1  ${config_append}])
+ [echo $1=$2  ${config_append}])
 
 # GIT_ARG_SET_PATH(PROGRAM)
 # -
@@ -34,8 +34,8 @@ AC_DEFUN([GIT_CONF_APPEND_PATH],
if test -n $2; then
GIT_UC_PROGRAM[]_PATH=$withval
AC_MSG_NOTICE([Disabling use of ${PROGRAM}])
-   GIT_CONF_APPEND_LINE(NO_${PROGRAM}=YesPlease)
-   GIT_CONF_APPEND_LINE(${PROGRAM}_PATH=)
+   GIT_CONF_APPEND_LINE([NO_${PROGRAM}], [YesPlease])
+   GIT_CONF_APPEND_LINE([${PROGRAM}_PATH], [])
else
AC_MSG_ERROR([You cannot use git without $1])
fi
@@ -45,7 +45,7 @@ AC_DEFUN([GIT_CONF_APPEND_PATH],
else
GIT_UC_PROGRAM[]_PATH=$withval
AC_MSG_NOTICE([Setting GIT_UC_PROGRAM[]_PATH to $withval])
-   GIT_CONF_APPEND_LINE(${PROGRAM}_PATH=$withval)
+   GIT_CONF_APPEND_LINE([${PROGRAM}_PATH], [$withval])
fi
 fi
 m4_popdef([GIT_UC_PROGRAM])])
@@ -67,7 +67,7 @@ AC_DEFUN([GIT_PARSE_WITH],
NO_[]GIT_UC_PACKAGE=
GIT_UC_PACKAGE[]DIR=$withval
AC_MSG_NOTICE([Setting GIT_UC_PACKAGE[]DIR to $withval])
-   GIT_CONF_APPEND_LINE(${PACKAGE}DIR=$withval)
+   GIT_CONF_APPEND_LINE([${PACKAGE}DIR], [$withval])
 fi
 m4_popdef([GIT_UC_PACKAGE])])
 
@@ -87,7 +87,7 @@ AC_DEFUN([GIT_PARSE_WITH_SET_MAKE_VAR],
 [a value for $1 ($2).  Maybe you do...?])
   fi
   AC_MSG_NOTICE([Setting $2 to $withval])
-  GIT_CONF_APPEND_LINE($2=$withval)
+  GIT_CONF_APPEND_LINE([$2], [$withval])
  fi)])# GIT_PARSE_WITH_SET_MAKE_VAR
 
 #
@@ -150,7 +150,7 @@ AC_ARG_WITH([sane-tool-path],
   else
 AC_MSG_NOTICE([Setting SANE_TOOL_PATH to '$withval'])
   fi
-  GIT_CONF_APPEND_LINE([SANE_TOOL_PATH=$withval])],
+  GIT_CONF_APPEND_LINE([SANE_TOOL_PATH], [$withval])],
   [# If the --with-sane-tool-path option was not given, don't touch
# SANE_TOOL_PATH here, but let defaults in Makefile take care of it.
# This should minimize spurious differences in the behaviour of the
@@ -169,7 +169,7 @@ AC_ARG_WITH([lib],
   else
lib=$withval
AC_MSG_NOTICE([Setting lib to '$lib'])
-   GIT_CONF_APPEND_LINE(lib=$withval)
+   GIT_CONF_APPEND_LINE([lib], [$withval])
   fi])
 
 if test -z $lib; then
@@ -205,7 +205,7 @@ AC_ARG_ENABLE([jsmin],
 [
   JSMIN=$enableval;
   AC_MSG_NOTICE([Setting JSMIN to '$JSMIN' to enable JavaScript minifying])
-  GIT_CONF_APPEND_LINE(JSMIN=$enableval);
+  GIT_CONF_APPEND_LINE([JSMIN], [$enableval]);
 ])
 
 # Define option to enable CSS minification
@@ -215,7 +215,7 @@ AC_ARG_ENABLE([cssmin],
 [
   CSSMIN=$enableval;
   AC_MSG_NOTICE([Setting CSSMIN to '$CSSMIN' to enable CSS minifying])
-  GIT_CONF_APPEND_LINE(CSSMIN=$enableval);
+  GIT_CONF_APPEND_LINE([CSSMIN], [$enableval]);
 ])
 
 ## Site configuration (override autodetection)
@@ -256,7 +256,7 @@ AS_HELP_STRING([],   [ARG can be also prefix for 
libpcre library and hea
USE_LIBPCRE=YesPlease
LIBPCREDIR=$withval
AC_MSG_NOTICE([Setting LIBPCREDIR to $withval])
-   GIT_CONF_APPEND_LINE(LIBPCREDIR=$withval)
+   GIT_CONF_APPEND_LINE([LIBPCREDIR], [$withval])
 fi)
 #
 # Define NO_CURL if you do not have curl installed.  git-http-pull and
-- 
1.7.10.2.1067.g553d16e

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


[PATCH 4/7] autoconf: remove few redundant semicolons

2012-07-18 Thread Stefano Lattarini
They are merely useless now, but would get in the way of future changes.

No semantic change is intended.

Signed-off-by: Stefano Lattarini stefano.lattar...@gmail.com
---
 configure.ac | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/configure.ac b/configure.ac
index 9472f6b..5fb9734 100644
--- a/configure.ac
+++ b/configure.ac
@@ -203,7 +203,7 @@ AC_ARG_ENABLE([jsmin],
 [
   JSMIN=$enableval;
   AC_MSG_NOTICE([Setting JSMIN to '$JSMIN' to enable JavaScript minifying])
-  GIT_CONF_SUBST([JSMIN], [$enableval]);
+  GIT_CONF_SUBST([JSMIN], [$enableval])
 ])
 
 # Define option to enable CSS minification
@@ -213,7 +213,7 @@ AC_ARG_ENABLE([cssmin],
 [
   CSSMIN=$enableval;
   AC_MSG_NOTICE([Setting CSSMIN to '$CSSMIN' to enable CSS minifying])
-  GIT_CONF_SUBST([CSSMIN], [$enableval]);
+  GIT_CONF_SUBST([CSSMIN], [$enableval])
 ])
 
 ## Site configuration (override autodetection)
-- 
1.7.10.2.1067.g553d16e

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


[PATCH 7/7] build: reconfigure automatically if configure.ac changes

2012-07-18 Thread Stefano Lattarini
This provides a reduced but still useful sibling of the Automake's
automatic Makefile rebuild feature.  It's important to note that
we take care to enable the new rules only if the tree that has already
be configured with './configure', so that users relying on manual
configuration won't be negatively impacted.

Signed-off-by: Stefano Lattarini stefano.lattar...@gmail.com
---
 Makefile | 12 
 configure.ac |  2 ++
 2 files changed, 14 insertions(+)

diff --git a/Makefile b/Makefile
index 88a76a3..f4e8fac 100644
--- a/Makefile
+++ b/Makefile
@@ -2158,6 +2158,18 @@ configure: configure.ac GIT-VERSION-FILE
autoconf -o $@ $+  \
$(RM) $+
 
+ifdef AUTOCONFIGURED
+config.status: configure
+   $(QUIET_GEN)if test -f config.status; then \
+ ./config.status --recheck; \
+   else \
+ ./configure; \
+   fi
+reconfigure config.mak.autogen: config.status
+   $(QUIET_GEN)./config.status
+.PHONY: reconfigure # This is a convenience target.
+endif
+
 XDIFF_OBJS += xdiff/xdiffi.o
 XDIFF_OBJS += xdiff/xprepare.o
 XDIFF_OBJS += xdiff/xutils.o
diff --git a/configure.ac b/configure.ac
index 64eecbc..8949935 100644
--- a/configure.ac
+++ b/configure.ac
@@ -149,6 +149,8 @@ AC_CONFIG_SRCDIR([git.c])
 config_file=config.mak.autogen
 config_in=config.mak.in
 
+GIT_CONF_SUBST([AUTOCONFIGURED], [YesPlease])
+
 # Directories holding saner versions of common or POSIX binaries.
 AC_ARG_WITH([sane-tool-path],
   [AS_HELP_STRING(
-- 
1.7.10.2.1067.g553d16e

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


[PATCH 6/7] build: make clean should not remove configure-generated files

2012-07-18 Thread Stefano Lattarini
Those filed hold variables, settings and information set by the
configuration process run by './configure'; in Autotools-based
build system that kind of stuff should only be removed by
make distclean.  Having it removed by make clean is not only
inconsistent, but causes real confusion for that part of the Git
audience that is used to the Autotools semantics; for example,
an autotools old-timer that has run:

./configure --prefix /opt/git

in the past, without running make distclean afterwards, would
expect a make install issued after a make clean to rebuild and
install git in '/opt/git'; but with the current behaviour, the
make clean invocation removes (among the other things) the file
'config.mak.autogen', so that the make install falls back to the
default prefix of '$HOME', thus installing git in the user's home
directory -- definitely unexpected.

Signed-off-by: Stefano Lattarini stefano.lattar...@gmail.com
---
 Makefile | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 285c660..88a76a3 100644
--- a/Makefile
+++ b/Makefile
@@ -2742,6 +2742,9 @@ dist-doc:
 
 distclean: clean
$(RM) configure
+   $(RM) config.log config.status config.cache
+   $(RM) config.mak.autogen config.mak.append
+   $(RM) -r autom4te.cache
 
 profile-clean:
$(RM) $(addsuffix *.gcda,$(addprefix $(PROFILE_DIR)/, $(object_dirs)))
@@ -2756,8 +2759,6 @@ clean: profile-clean
$(RM) -r $(dep_dirs)
$(RM) -r po/build/
$(RM) *.spec *.pyc *.pyo */*.pyc */*.pyo common-cmds.h $(ETAGS_TARGET) 
tags cscope*
-   $(RM) -r autom4te.cache
-   $(RM) config.log config.mak.autogen config.mak.append config.status 
config.cache
$(RM) -r $(GIT_TARNAME) .doc-tmp-dir
$(RM) $(GIT_TARNAME).tar.gz git-core_$(GIT_VERSION)-*.tar.gz
$(RM) $(htmldocs).tar.gz $(manpages).tar.gz
-- 
1.7.10.2.1067.g553d16e

--
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: Mini bug report origin/pu: t1512 failed on Mac OS X (commit 957d74062c1f0e ?)

2012-07-13 Thread Stefano Lattarini
On 07/12/2012 01:30 AM, Junio C Hamano wrote:
 Junio C Hamano gits...@pobox.com writes:
 
 I think the other tests in t/ prefer to unquote it so that we would
 ignore spaces around wc -l output, i.e.

  test $(wc -l actual) = 16

 Thanks for a report.
 
 -- 8 --
 Subject: [PATCH] t1512: ignore whitespaces in wc -l output
 
 Some implementations of sed (e.g. MacOS X)

'sed'?  Shouldn't this read 'wc'?

 have whitespaces in the output of wc -l that reads from the standard
 input.

FYI, the extra space is present with Solaris wc as well:

$ wc -l /dev/null
   0

Regards,
  Stefano
--
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