Re: [PATCH v2] configure.ac: move the private git m4 macros to a dedicated directory
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.
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
(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
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
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
[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
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)
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
[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 ***
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
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
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.
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'
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
[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
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?
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?
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?
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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'
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
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
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
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
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
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
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
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
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 ?)
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