* Re: [PATCH] gitk: Update and fix Makefile
From: Paul Mackerras @ 2008-01-09 3:43 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Christian Stimming, git
In-Reply-To: <7vk5mkq669.fsf@gitster.siamese.dyndns.org>
Junio C Hamano writes:
> I see somwhat funny spacing there. I'd suggest giving up
> aligning with spaces and consistently saying "var ?= val"
> instead.
I made those lines all have one space before and after the ?=, and
committed Christian's patches (plus one from Gerrit Pape), and pushed
it out. Please do a pull.
Paul.
^ permalink raw reply
* Re: [PATCH] Add [HOWTO] using merge subtree.
From: Junio C Hamano @ 2008-01-09 3:46 UTC (permalink / raw)
To: Miklos Vajna; +Cc: Sean, git
In-Reply-To: <7vwsqjpyqi.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Miklos Vajna <vmiklos@frugalware.org> writes:
>
> NAK. It may be well intentioned, but it lacks too much context
> to be usable as a generic teaching material.
>
>> +Abstract: In this article, Sean demonstrates how one can use the subtree merge
>> + strategy.
>> +Message-ID: <BAYC1-PASMTP12374B54BA370A1E1C6E78AE4E0@CEZ.ICE>
>> +
>> +How to use the subtree merge strategy
>> +=====================================
>
> Here, before diving to the actual command sequence, you need to
> tell the reader what the objective of the whole exercise is.
Just to avoid discouraging contributors too much, here are
pointers to some other materials in the mailing list archive
that may serve as the basis for the "discussion" section of this
HOWTO.
Side note. I think an ideal HOWTO should
- describe the objective briefly so that the reader can tell if
that is relevant to the task at hand;
- describe the procedure;
- mention other possible avenues and discuss pros and cons, so
that the reader can make an informed decision to employ or not
to employ the solution.
My complaints were that the original had only the second part.
And this message is to give pointers to possible material for
the third part.
* http://thread.gmane.org/gmane.comp.version-control.git/39443
This was the explanation on the way how the initial git-gui
subtree merge was created.
* http://thread.gmane.org/gmane.comp.version-control.git/39963
The introduction of subtree strategy. The original cover letter
and the following discussion is rich with discussions on merge
philosophy and pros and cons.
Back then "submodule" was not available, and we would still
avoid it today in git.git, because use of submodule in a
repository would make the repository non-interoperable with any
clients older than 1.5.2 series (that's one pros-and-cons item).
Somewhere in the thread it was also suggested "am -pN" may be
necessary.
* http://thread.gmane.org/gmane.comp.version-control.git/46569/focus=46609
This is a side note -- it turns out that "am -pN" used on the
context of subtree merge is often not necessary, as --3way is a
magic.
^ permalink raw reply
* [PATCH] Simplified the invocation of command action in submodule
From: imyousuf @ 2008-01-09 3:59 UTC (permalink / raw)
To: git; +Cc: gitster, Imran M Yousuf, Imran M Yousuf
From: Imran M Yousuf <imran@smartitengineering.com>
- Simplified the invocation of action.
- Changed switch case based action invoke rather more direct command
invocation. Previously first switch case was used to go through $@ and
determine the action, i.e. add, init, update etc, and second switch case
just to invoke the action. It is modified to determine the action name in
the first case structure instead and later just invoke it.
Signed-off-by: Imran M Yousuf <imyousuf@smartitengineering.com>
---
git-submodule.sh | 32 ++++++++++++--------------------
1 files changed, 12 insertions(+), 20 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index ad9fe62..8a29382 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -16,6 +16,7 @@ update=
status=
quiet=
cached=
+command=
#
# print stuff on stdout unless -q was specified
@@ -293,20 +294,23 @@ modules_list()
done
}
+# command specifies the whole function name since
+# one of theirs prefix is module not modules
while test $# != 0
do
case "$1" in
add)
add=1
+ command="module_$1"
;;
init)
- init=1
+ command="modules_$1"
;;
update)
- update=1
+ command="modules_$1"
;;
status)
- status=1
+ command="modules_list"
;;
-q|--quiet)
quiet=1
@@ -320,7 +324,7 @@ do
branch="$2"; shift
;;
--cached)
- cached=1
+ command="modules_list"
;;
--)
break
@@ -345,20 +349,8 @@ case "$add,$branch" in
;;
esac
-case "$add,$init,$update,$status,$cached" in
-1,,,,)
- module_add "$@"
- ;;
-,1,,,)
- modules_init "$@"
- ;;
-,,1,,)
- modules_update "$@"
- ;;
-,,,*,*)
- modules_list "$@"
- ;;
-*)
+if [ -z $command ]; then
usage
- ;;
-esac
+else
+ "$command" "$@"
+fi
--
1.5.3.7
^ permalink raw reply related
* Re: gmail smtp server and git-send-mail. Is this combination working?
From: Douglas Stockwell @ 2008-01-09 4:06 UTC (permalink / raw)
To: git
In-Reply-To: <4d8e3fd30801080858h5f109b47v87abc6b315fcfa08@mail.gmail.com>
Paolo Ciarrocchi wrote:
> " Mailing off a set of patches to a mailing list can be quite neatly
> done by git-send-email.
> One of the problems you may encounter there is figuring out which machine
> is going to send your mail.
> I tried smtp.gmail.com, but that one requires tls and a password,
> and git-send-email could not handle that "
>
> From http://git.or.cz/gitwiki/GitTips.
>
> Is this statemant still correct ?
> Is msmtp the only solution for using git-send-mail with gmail? (tls +
> autentication).
No, as of 34cc60ce2b48f6037997543ddbab1ed9903df4a8 you can use SSL and
SMTP-Auth.
[sendemail]
smtpserver = smtp.gmail.com
smtpuser = <user>@gmail.com
smtppass = <password>
smtpssl = true
Can you suggest changes to the documentation if these options are unclear?
Doug
^ permalink raw reply
* [PATCH] - Added recurse command to git submodule
From: imyousuf @ 2008-01-09 5:51 UTC (permalink / raw)
To: git; +Cc: gitster, Imran M Yousuf
From: Imran M Yousuf <imyousuf@smartitengineering.com>
- The purpose of the recurse command in the git submodule is to recurse
a command in its submodule at depths specified. For example if one wants
to do a diff on its project with submodules at once, one can simply do
git-submodule recurse diff HEAD and would see the diff for all the modules
it contains. If during recursion it is found that a module has not been
initialized or updated than the command also initializes and updates them.
It is to be noted that argument specification to the command intended (in
above example diff) to recurse will be same as its original command (i.e.
git diff). If the original command spec changes it will have no effect on
the recurse command. Depth of recursion can be specified simply by
mentioning the -d <recursion depth> argument to recurse command. If not
specified or specified to 0, it will be comprehended to recurse at all
depth; if a positive number is specified than maximum recursion depth will
never cross that depth. In order to see some information -v option may be
used
Signed-off-by: Imran M Yousuf <imyousuf@smartitengineering.com>
---
git-submodule.sh | 112 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 111 insertions(+), 1 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index 8a29382..5cb931e 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -4,7 +4,7 @@
#
# Copyright (c) 2007 Lars Hjemli
-USAGE='[--quiet] [--cached] [add <repo> [-b branch]|status|init|update] [--] [<path>...]'
+USAGE='[[--quiet] [--cached] [add <repo> [-b branch]|status|init|update] [--] [<path>...]|[recurse [-v] [-d <recursion depth>] <command> <arguments> ...]]'
OPTIONS_SPEC=
. git-sh-setup
require_work_tree
@@ -17,6 +17,9 @@ status=
quiet=
cached=
command=
+recurse_verbose=0
+recursion_depth=0
+current_recursion_depth=0
#
# print stuff on stdout unless -q was specified
@@ -294,6 +297,82 @@ modules_list()
done
}
+# Initializes the submodule if already not initialized
+initialize_sub_module() {
+ if [ ! -d "$1"/.git ]; then
+ if [ $recurse_verbose -eq 1 ]; then
+ echo Initializing and updating "$1"
+ fi
+ git-submodule init "$1"; git-submodule update "$1"
+ fi
+}
+
+# This actually traverses the module; checks
+# whether the module is initialized or not.
+# if not initialized, then done so and then the
+# intended command is evaluated. Then it
+# recursively goes into it modules.
+traverse_module() {
+ if [ $recurse_verbose -eq 1 ]; then
+ echo Current recursion depth: "$current_recursion_depth"
+ fi
+ initialize_sub_module "$1"
+ (
+ submod_path="$1"
+ shift
+ cd "$submod_path"
+ if [ $recurse_verbose -eq 1 ]; then
+ echo Working in mod "$submod_path" @ `pwd` with "$@" \("$#"\)
+ fi
+ git "$@"
+ # Check whether submodules exists only if recursion to that depth
+ # was requested by user
+ if test "$recursion_depth" -eq 0 || test "$current_recursion_depth" -lt "$recursion_depth"
+ then
+ if [ -f .gitmodules ]; then
+ for mod_path in `sed -n -e 's/path = //p' .gitmodules`; do
+ export current_recursion_depth=$(echo "$current_recursion_depth+1" | bc)
+ traverse_module "$mod_path" "$@"
+ export current_recursion_depth=$(echo "$current_recursion_depth-1" | bc)
+ done
+ fi
+ fi
+ )
+}
+
+# Propagates or recurses over all the submodules at any
+# depth with any git command, e.g. git-clone, git-status,
+# git-commit etc., with the arguments supplied exactly as
+# it would have been supplied to the command otherwise.
+# This actually starts the recursive propagation
+modules_recurse() {
+ project_home=`pwd`
+ echo Project Home: "$project_home"
+ if [ $recurse_verbose -eq 1 ]; then
+ if [ $recursion_depth = 0 ]; then
+ echo Recursion will go to all depths
+ else
+ echo Recursion depth specified to "$recursion_depth"
+ fi
+ fi
+ if [ -d "$project_home"/.git/ ]; then
+ if [ $recurse_verbose -eq 1 ]; then
+ echo Command to recurse: git "$@"
+ fi
+ git "$@"
+ if [ -f .gitmodules ]; then
+ for mod_path in `sed -n -e 's/path = //p' .gitmodules`; do
+ export current_recursion_depth=1
+ traverse_module $mod_path "$@"
+ done
+ fi
+ else
+ echo "$project_home" not a git repo thus exiting
+ exit
+ fi
+ unset current_recursion_depth
+}
+
# command specifies the whole function name since
# one of theirs prefix is module not modules
while test $# != 0
@@ -326,6 +405,37 @@ do
--cached)
command="modules_list"
;;
+ recurse)
+ command="modules_$1"
+ repeat=1
+ while test $repeat = 1
+ do
+ case "$2" in
+ -v)
+ recurse_verbose=1
+ shift
+ ;;
+ -d)
+ if [ -z $3 ]; then
+ echo No \<recursion depth\> specified
+ usage
+ elif [ `expr "$3" : '[1-9][0-9]*.*'` -eq `expr "$3" : '.*'` ]; then
+ recursion_depth="$3"
+ shift
+ shift
+ else
+ echo \<recursion depth\> not an integer
+ usage
+ fi
+ ;;
+ *)
+ repeat=0
+ ;;
+ esac
+ done
+ shift
+ break
+ ;;
--)
break
;;
--
1.5.3.7
^ permalink raw reply related
* Re: [PATCH] Added initialize and update support for submodule in git clone
From: Imran M Yousuf @ 2008-01-09 6:05 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vr6gsx2j5.fsf@gitster.siamese.dyndns.org>
Thanks again Junio.
I will resend the patch with the latest code base. After implementing
I also felt that it is not logical to go all way down, so I was
thinking of adding -l | --level with -w or use --depth already in use;
that will add some flexibility; what do you thing about this? I will
make the similar changes you mentioned in the earlier email in this as
well.
I was actually concerned about unsetting and re-setting the
environment variables. Is there a alternate to it?
Thank you,
Imran
On Jan 8, 2008 12:19 PM, Junio C Hamano <gitster@pobox.com> wrote:
> "Imran M Yousuf" <imyousuf@gmail.com> writes:
>
> > This patch adds support for initializing and updating submodules when
> > a repo is cloned. The advantage it adds is, the user actually does not
> > have to know whether it has a module or not and if it does to what
> > depth and path. For this I added a option -w or --with-submodule for
> > initializing and updating during clone stage.
>
> For everything else, I strongly agree [*1*] that the notion that
> all subprojects are populated is a bug. I am not convinced the
> all-or-nothing approach you implemented in "git clone" is useful
> outside small toy projects where all of your users are almost
> always interested in everything (which inevitably invites a very
> valid question: why use submodule at all then?), but in the very
> narrow special case of "clone", all-or-nothing is the best you
> can do without giving additional hints somewhere in-tree
> (perhaps enhanced .gitmodules entries), and it certainly is
> better than "you do not have any choice --- you only get the
> toplevel".
>
> > Following is the diff with git-clone 1.5.3.7; I also attached the diff
> > and modified file in the attachment.
>
> The same comment as diff plus attachment applies to this patch
> as the other message. Also please do not base new development
> on 4-digit maintenance releases, which are meant to contain only
> bugfixes and no new features. A patch like this, primarily for
> discussion and not for immediate inclusion, is Ok, but it is
> better to get into the habit of producing applicable patches
> earlier rather than later.
>
> I'll step aside and let others discuss code and design of the
> patch.
>
> [Reference]
>
> *1* http://thread.gmane.org/gmane.comp.version-control.git/44106/focus=44308
>
--
Imran M Yousuf
Entrepreneur & Software Engineer
Smart IT Engineering
Dhaka, Bangladesh
Email: imran@smartitengineering.com
Mobile: +880-1711402557
^ permalink raw reply
* Re: [PATCH] Add [HOWTO] using merge subtree.
From: Junio C Hamano @ 2008-01-09 6:28 UTC (permalink / raw)
To: Miklos Vajna; +Cc: Sean, git
In-Reply-To: <7vfxx7psoq.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> * http://thread.gmane.org/gmane.comp.version-control.git/39443
>
> This was the explanation on the way how the initial git-gui
> subtree merge was created.
For the record, the very first cross-project merge is:
http://thread.gmane.org/gmane.comp.version-control.git/5126
The procedure to maintain the resulting re-merge of two
histories do not involve subtree merge because "The coolest
merge ever" merges non-overlapping two trees at the same level,
but the principle used to bootstrap the process is the same.
You prepare a tree that is a overlay of the tips of your two
projects, and record that tree in a commit that is a merge
between two parents.
^ permalink raw reply
* [PATCH] Trim leading / off of paths in git-svn prop_walk
From: Kevin Ballard @ 2008-01-09 6:37 UTC (permalink / raw)
To: git; +Cc: gitster, Kevin Ballard
prop_walk adds a leading / to all subdirectory paths. Unfortunately
this causes a problem when the remote repo lives in a subdirectory itself,
as the leading / causes subsequent PROPFIND calls to be executed on
the wrong path. Trimming the / before calling the PROPFIND fixes this problem.
Signed-off-by: Kevin Ballard <kevin@sb.org>
---
All tests passed after this change, but since it seems to only apply
to WebDAV SVN repos I saw no way to add a new test.
git-svn.perl | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index 3308fe1..d5316eb 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -1858,6 +1858,7 @@ sub rel_path {
sub prop_walk {
my ($self, $path, $rev, $sub) = @_;
+ $path =~ s#^/##;
my ($dirent, undef, $props) = $self->ra->get_dir($path, $rev);
$path =~ s#^/*#/#g;
my $p = $path;
--
1.5.4.rc2.68.ge708a-dirty
^ permalink raw reply related
* Re: [PATCH 1/2] sideband.c: Use xmalloc() instead of variable-sized arrays.
From: Johannes Sixt @ 2008-01-09 7:26 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <7v4pdotdtl.fsf@gitster.siamese.dyndns.org>
Junio C Hamano schrieb:
> Johannes Sixt <j.sixt@viscovery.net> writes:
>> @@ -19,7 +19,10 @@ int recv_sideband(const char *me, int in_stream, int out, int err)
>> {
>> unsigned pf = strlen(PREFIX);
>> unsigned sf = strlen(SUFFIX);
>> - char buf[pf + LARGE_PACKET_MAX + sf + 1];
>> + char *buf, *save;
>> +
>> + save = xmalloc(sf);
>> + buf = xmalloc(pf + LARGE_PACKET_MAX + sf + 1);
>
> I have to wonder if the malloc() overhead is small enough
> compared to the network bandwidth to make a two malloc-free
> pairs per packet a non-issue...
recv_sideband() is called _once_per_connection_ and not for each packet.
Hence, these two mallocs should not concern us.
-- Hannes
^ permalink raw reply
* Re: [PATCH 1/2] sideband.c: Use xmalloc() instead of variable-sized arrays.
From: Johannes Sixt @ 2008-01-09 7:34 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <alpine.LFD.1.00.0801081154460.3054@xanadu.home>
Nicolas Pitre schrieb:
> On Tue, 8 Jan 2008, Johannes Sixt wrote:
>> How come we got along with this not very portable construct for so long?
>> Probably because the array sizes were computed from the results of
>> strlen() of string constants.
>
> Maybe because it isn't not so unportable anymore? I doubt that
> compilers that don't know about automatic arrays would be smart enough
> to notice the variable was actually a constant due to the strlen() of a
> constant string and just do like if there wasn't any variable for the
> array size.
I just tried it with Visual Age 6, and got this:
CC sideband.o
"sideband.c", line 22.18: 1506-195 (S) Integral constant expression with a
value greater than zero is required.
"sideband.c", line 62.51: 1506-195 (S) Integral constant expression with a
value greater than zero is required.
make: *** [sideband.o] Error 1
But before I got to this point I had to change all 'static inline' in
git-compat-util.h to plain 'static'. So this compiler is out of the game
anyway.
Having said that, I'd actually prefer to stay with variable-sized arrays
if they prove portable enough because we don't need the handful of free()s
on function exits. Junio, if you like I can resend patch 2/2 using
variable-sized arrays.
-- Hannes
^ permalink raw reply
* Re: [PATCH 1/2] sideband.c: Use xmalloc() instead of variable-sized arrays.
From: Junio C Hamano @ 2008-01-09 7:53 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Nicolas Pitre, Git Mailing List
In-Reply-To: <4784791F.6090904@viscovery.net>
Johannes Sixt <j.sixt@viscovery.net> writes:
> Having said that, I'd actually prefer to stay with variable-sized arrays
> if they prove portable enough because we don't need the handful of free()s
> on function exits. Junio, if you like I can resend patch 2/2 using
> variable-sized arrays.
As an old fashoned git myself, and given the fact that the
possible prefix and suffix are small number of short constant
strings, I actually prefer a simpler-and-more-stupid approach.
sideband.c | 18 +++++++++++++-----
1 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/sideband.c b/sideband.c
index 756bbc2..24f7f12 100644
--- a/sideband.c
+++ b/sideband.c
@@ -13,14 +13,22 @@
*/
#define PREFIX "remote:"
-#define SUFFIX "\033[K" /* change to " " if ANSI sequences don't work */
int recv_sideband(const char *me, int in_stream, int out, int err)
{
unsigned pf = strlen(PREFIX);
- unsigned sf = strlen(SUFFIX);
- char buf[pf + LARGE_PACKET_MAX + sf + 1];
+ unsigned sf;
+ char buf[LARGE_PACKET_MAX + 100]; /* for marker slop */
+ char *suffix, *term;
+
memcpy(buf, PREFIX, pf);
+ term = getenv("TERM");
+ if (term && strcmp(term, "dumb"))
+ suffix = "\033[K";
+ else
+ suffix = " ";
+ sf = strlen(suffix);
+
while (1) {
int band, len;
len = packet_read_line(in_stream, buf + pf, LARGE_PACKET_MAX);
@@ -59,10 +67,10 @@ int recv_sideband(const char *me, int in_stream, int out, int err)
* line data actually contains something.
*/
if (brk > pf+1 + 1) {
- char save[sf];
+ char save[100]; /* enough slop */
memcpy(save, buf + brk, sf);
buf[brk + sf - 1] = buf[brk - 1];
- memcpy(buf + brk - 1, SUFFIX, sf);
+ memcpy(buf + brk - 1, suffix, sf);
safe_write(err, buf, brk + sf);
memcpy(buf + brk, save, sf);
} else
^ permalink raw reply related
* Re: CRLF problems with Git on Win32
From: Junio C Hamano @ 2008-01-09 8:03 UTC (permalink / raw)
To: Linus Torvalds
Cc: J. Bruce Fields, Steffen Prohaska, Johannes Schindelin,
Robin Rosenberg, Jeff King, Peter Karlsson, Git Mailing List,
msysGit
In-Reply-To: <alpine.LFD.1.00.0801081232120.3148@woody.linux-foundation.org>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> On Tue, 8 Jan 2008, Junio C Hamano wrote:
>>
>> I think the project can mark text files as text with attributes
>> and if the port to the platform initialized core.autocrlf
>> appropriately for the platform everything should work as you
>> described.
>
> Yes, I think core.autocrlf should default to "true" on Windows, since
> that is what it's about. The alternative is to have "fail"/"warn", to just
> make sure that nobody can do the wrong thing by mistake.
>
> We could just do something like this, although that probably does mean
> that the whole test-suite needs to be double-checked (ie now we really do
> behave differently on windows outside of any config options!))
>
> People who really dislike it can always do the
>
> git config --global core.autocrlf false
>
> thing.
>
> (And no, I don't know if "#ifdef __WINDOWS__" is the right thing to do,
> it's almost certainly not. This is just a draft.)
Perhaps we can do something similar to core.filemode? Create a
file that we would need to create anyway in "text" mode, and
read it back in "binary" mode to see what stdio did?
^ permalink raw reply
* Re: [PATCH 1/2] sideband.c: Use xmalloc() instead of variable-sized arrays.
From: Johannes Sixt @ 2008-01-09 8:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nicolas Pitre, Git Mailing List
In-Reply-To: <7vk5mjmo4f.fsf@gitster.siamese.dyndns.org>
Junio C Hamano schrieb:
> Johannes Sixt <j.sixt@viscovery.net> writes:
>
>> Having said that, I'd actually prefer to stay with variable-sized arrays
>> if they prove portable enough because we don't need the handful of free()s
>> on function exits. Junio, if you like I can resend patch 2/2 using
>> variable-sized arrays.
>
> As an old fashoned git myself, and given the fact that the
> possible prefix and suffix are small number of short constant
> strings, I actually prefer a simpler-and-more-stupid approach.
>
> sideband.c | 18 +++++++++++++-----
> 1 files changed, 13 insertions(+), 5 deletions(-)
Thanks, your version works well here, too, as a replacement of my two patches.
-- Hannes
^ permalink raw reply
* Re: [PATCH] gitk: Update and fix Makefile
From: Junio C Hamano @ 2008-01-09 8:19 UTC (permalink / raw)
To: Paul Mackerras; +Cc: Christian Stimming, git
In-Reply-To: <18308.17099.334609.80415@cargo.ozlabs.ibm.com>
Paul Mackerras <paulus@samba.org> writes:
> Junio C Hamano writes:
>
>> I see somwhat funny spacing there. I'd suggest giving up
>> aligning with spaces and consistently saying "var ?= val"
>> instead.
>
> I made those lines all have one space before and after the ?=, and
> committed Christian's patches (plus one from Gerrit Pape), and pushed
> it out. Please do a pull.
There were a couple of things I noticed that made me somewhat curious:
* There are more spaces around ?= in Christian's patch to Makefile.
* You have two patches from Christian and one patch from
Gerrit; the author and commit timestamps of these commits are
the same and in your timezone.
but nothing to complain about. Pulled and pushed out.
^ permalink raw reply
* Re: [PATCH] Simplified the invocation of command action in submodule
From: Junio C Hamano @ 2008-01-09 8:19 UTC (permalink / raw)
To: imyousuf; +Cc: git, Imran M Yousuf
In-Reply-To: <1199851140-31853-1-git-send-email-imyousuf@gmail.com>
imyousuf@gmail.com writes:
> diff --git a/git-submodule.sh b/git-submodule.sh
> index ad9fe62..8a29382 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -16,6 +16,7 @@ update=
> status=
> quiet=
> cached=
> +command=
Doesn't the patch make some if not all of the above variables
unused?
> case "$1" in
> add)
> add=1
> + command="module_$1"
> ;;
> init)
> - init=1
> + command="modules_$1"
> ;;
Does the remaining code still use $add?
^ permalink raw reply
* Re: [PATCH] Simplified the invocation of command action in submodule
From: Imran M Yousuf @ 2008-01-09 8:23 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vsl17l8bi.fsf@gitster.siamese.dyndns.org>
Hi Junio,
Firstly, $add is still used later in the code.
Secondly, yes the variables should be deleted. Will make the change
and send the patch again; I forgot to clean the unused variables from
the declaration, sorry.
Best regards,
Imran
On Jan 9, 2008 2:19 PM, Junio C Hamano <gitster@pobox.com> wrote:
> imyousuf@gmail.com writes:
>
> > diff --git a/git-submodule.sh b/git-submodule.sh
> > index ad9fe62..8a29382 100755
> > --- a/git-submodule.sh
> > +++ b/git-submodule.sh
> > @@ -16,6 +16,7 @@ update=
> > status=
> > quiet=
> > cached=
> > +command=
>
> Doesn't the patch make some if not all of the above variables
> unused?
>
> > case "$1" in
> > add)
> > add=1
> > + command="module_$1"
> > ;;
> > init)
> > - init=1
> > + command="modules_$1"
> > ;;
>
> Does the remaining code still use $add?
>
--
Imran M Yousuf
^ permalink raw reply
* Re: [PATCH] - Added recurse command to git submodule
From: Junio C Hamano @ 2008-01-09 8:38 UTC (permalink / raw)
To: imyousuf; +Cc: git, Imran M Yousuf
In-Reply-To: <1199857906-26321-1-git-send-email-imyousuf@gmail.com>
imyousuf@gmail.com writes:
> From: Imran M Yousuf <imyousuf@smartitengineering.com>
>
>
> - The purpose of the recurse command in the git submodule is to recurse
> a command in its submodule at depths specified. For example if one wants
> to do a diff on its project with submodules at once, one can simply do
> git-submodule recurse diff HEAD and would see the diff for all the modules
> it contains. If during recursion it is found that a module has not been
> initialized or updated than the command also initializes and updates them.
> It is to be noted that argument specification to the command intended (in
> above example diff) to recurse will be same as its original command (i.e.
> git diff). If the original command spec changes it will have no effect on
> the recurse command. Depth of recursion can be specified simply by
> mentioning the -d <recursion depth> argument to recurse command. If not
> specified or specified to 0, it will be comprehended to recurse at all
> depth; if a positive number is specified than maximum recursion depth will
> never cross that depth. In order to see some information -v option may be
> used
The indentation style seems, eh, unique. An overlong single
paragraph with solid run of sentences is somewhat hard to read.
I am not still convinced that a subcommand other than init,
which is started recursively, should initialize and update
submodules that are uninitialized. Currently there is no way,
other than not having an initialized submodule repository, to
mark that the user is _not_ interested in a particular
submodule, and you will lose that if you make recursing into
uninitialized ones too easy and aggressive.
I suspect that it might be a saner approach to:
- allow "git submodule recurse init [-d depth]" (although I am
not sure if limit by depth is so useful in practice -- only
experience will tell us) to auto-initialize the uninitialized
submodules;
- allow any other "git submodule recurse $cmd" to run
recursively to _any_ depth, but never auto-initialize the
uninitialized submodules.
In other words, I think it is a very bad idea to rob the
existing mechanism from the user to mark uninteresting
submodules by not initializing them. An alternative would be to
give them other means to mark "I am not interested in these
submodules", if you insist on this auto-initialization, but I do
not offhand think of a mechanism that is easier to use than what
we already have (i.e. not checking them out).
> @@ -17,6 +17,9 @@ status=
> quiet=
> cached=
> command=
> +recurse_verbose=0
> +recursion_depth=0
> +current_recursion_depth=0
I wonder if we want this "verbose" option to be specific to the
recurse subcommand, or perhaps other subcommands may want to
support more verbose output mode, even if they currently don't.
Perhaps the variable should be just called $verbose?
> +# Initializes the submodule if already not initialized
> +initialize_sub_module() {
> + if [ ! -d "$1"/.git ]; then
> + if [ $recurse_verbose -eq 1 ]; then
> + echo Initializing and updating "$1"
That's a sensible message for the $verbose mode.
> + fi
> + git-submodule init "$1"; git-submodule update "$1"
Can init fail for repository "$1"? Do you want to proceed
updating it if it does?
> +# This actually traverses the module; checks
> +# whether the module is initialized or not.
> +# if not initialized, then done so and then the
> +# intended command is evaluated. Then it
> +# recursively goes into it modules.
> +traverse_module() {
> + if [ $recurse_verbose -eq 1 ]; then
> + echo Current recursion depth: "$current_recursion_depth"
> + fi
> + initialize_sub_module "$1"
> + (
> + submod_path="$1"
> + shift
> + cd "$submod_path"
> + if [ $recurse_verbose -eq 1 ]; then
> + echo Working in mod "$submod_path" @ `pwd` with "$@" \("$#"\)
Is this a sensible message, I have to wonder... Saying `pwd`
after already saying "$submod_path" feels more like a debugging
message than just being $verbose.
> + fi
> + git "$@"
Is the user interested in exit status from this command? Does
the user want to continue on to other submodules if this one
fails?
> + # Check whether submodules exists only if recursion to that depth
> + # was requested by user
> + if test "$recursion_depth" -eq 0 || test "$current_recursion_depth" -lt "$recursion_depth"
Overly long line. Perhaps...
if test "$depth" -eq 0 ||
test "$current_depth" -lt "$depth"
then
...
> + then
> + if [ -f .gitmodules ]; then
> + for mod_path in `sed -n -e 's/path = //p' .gitmodules`; do
> + export current_recursion_depth=$(echo "$current_recursion_depth+1" | bc)
(1) I do not think you need to export this variable;
(2) It was reported some shells that we intend to support
mishandle "export VAR=VAL" construct and we tend to write
this "VAR=VAL" followed by "export VAR" as two separate
commands on two separate lines;
(3) We do not use "bc" (and traditionally, shell scripts
outside git don't, either) in scripts.
So, I think:
current_recursion_depth=$(( $current_recursion_depth + 1 ))
is enough, although the variable name feels overly long.
Probably I would even do:
if test "$depth -ne 0 && test "$current_depth" -ge "$depth"
then
exit 0
fi
if test -f .gitmodules
then
current_recursion_depth=$(( $current_recursion_depth + 1 ))
for p in $(sed -n -e 's/path = ....)
do
traverse_module "$p" "$@"
done
fi
which would avoid one level of nesting (and indentation), and
removes unnecessary increment and decrement inside the loop.
You are in your own subprocess, so your increment will not
affect the process that called you, and after leaving the loop
the only thing you do is just to exit the subprocess.
> +# Propagates or recurses over all the submodules at any
> +# depth with any git command, e.g. git-clone, git-status,
> +# git-commit etc., with the arguments supplied exactly as
> +# it would have been supplied to the command otherwise.
> +# This actually starts the recursive propagation
> +modules_recurse() {
> + project_home=`pwd`
> + echo Project Home: "$project_home"
Doesn't this message belong to $verbose mode?
> + if [ $recurse_verbose -eq 1 ]; then
> + if [ $recursion_depth = 0 ]; then
> + echo Recursion will go to all depths
> + else
> + echo Recursion depth specified to "$recursion_depth"
> + fi
These sounds more like debugging message than $verbose.
> + fi
> + if [ -d "$project_home"/.git/ ]; then
> + if [ $recurse_verbose -eq 1 ]; then
> + echo Command to recurse: git "$@"
Likewise -- you will repeat that inside traverse_module anyway.
> + fi
> + git "$@"
> + if [ -f .gitmodules ]; then
> + for mod_path in `sed -n -e 's/path = //p' .gitmodules`; do
> + export current_recursion_depth=1
> + traverse_module $mod_path "$@"
> + done
> + fi
Do I see a code duplication here? Why isn't this done as the
first level recursion inside traverse_module? _Even_ _if_ you
insist auto-initializing submodules, by moving the
initialize_sub_module call in traverse_module a bit down
(namely, before it recursively invoke traverse_module itself and
make it auto initialize $submod_path, not "$1"), I think you can
remove this duplication.
> + else
> + echo "$project_home" not a git repo thus exiting
> + exit
And its exit code is 0 which tells the caller that there is no
error?
> + fi
> + unset current_recursion_depth
The reason for this unset is...?
> @@ -326,6 +405,37 @@ do
> --cached)
> command="modules_list"
> ;;
> + recurse)
> + command="modules_$1"
> + repeat=1
> + while test $repeat = 1
> + do
You do not need that $repeat thing. Just use "break", like this.
while :
do
case ... in
...
*)
break ;;
esac
done
> + -d)
> + if [ -z $3 ]; then
(minor style)
if test -z "$3"
then
...
> + echo No \<recursion depth\> specified
> + usage
> + elif [ `expr "$3" : '[1-9][0-9]*.*'` -eq `expr "$3" : '.*'` ]; then
> + recursion_depth="$3"
> + shift
> + shift
> + else
> + echo \<recursion depth\> not an integer
> + usage
> + fi
> + ;;
Instead of checking integerness by hand, it would probably be
much simpler if you did something like this:
depth="$3"
depth=$(( $depth + 0 ))
if test "$depth" != "$3"
then
die "what's that '$3' for?"
else
: happy
fi
For a rough guideline of shell constructs we use (and do not
use), please see Documentation/CodingGuidelines.
^ permalink raw reply
* Re: [STG PATCH] change usage string of refresh to not refer to removed options
From: Karl Hasselström @ 2008-01-09 7:20 UTC (permalink / raw)
To: Peter Oberndorfer; +Cc: Git Mailing List
In-Reply-To: <200801082143.53125.kumbayo84@arcor.de>
Thanks! But ...
On 2008-01-08 21:43:53 +0100, Peter Oberndorfer wrote:
> for changing the patch author, commiter and description
the first line in the commit message (which ends up being the Subject:
line in the mail) should be a short and self-contained summary. No
need to resend just because of that, though -- I can fix it up.
> I saw this while doing the refresh --index patch. There was a
> discussion about bringing back refresh -e If that also brings back
> this options, please do not apply the patch:-)
Resurrecting "refresh -e" is on my todo list. But until then, this is
a good fix.
--
Karl Hasselström, kha@treskal.com
www.treskal.com/kalle
^ permalink raw reply
* Re: [STG PATCH] add a --index option to refresh which takes the contents of the index as the new commit
From: Karl Hasselström @ 2008-01-09 7:23 UTC (permalink / raw)
To: Peter Oberndorfer; +Cc: Git Mailing List, Catalin Marinas
In-Reply-To: <200801082142.47060.kumbayo84@arcor.de>
On 2008-01-08 21:42:46 +0100, Peter Oberndorfer wrote:
> On Montag 07 Januar 2008, Karl Hasselström wrote:
>
> > So the use_index parameter to refresh_patch is actually not
> > necessary? In that case I'd rather you didn't add it, since the
> > functions in stgit/stack.py have quite enough parameters already.
>
> In the beginning i was afraid it would be to obscure to call it this
> way with all parameters set to some specific values. But having more
> parameters does not make it better :-) Done
Thanks.
> Patch now comes with a Signed-off-by and a log message that explains
> how this feature could be used. It was tested with the testcase,
> used during development of this patch and on another repo, but still
> take care when using it :-)
I may be promising too much now, but hopefully I'll get to this
tonight.
--
Karl Hasselström, kha@treskal.com
www.treskal.com/kalle
^ permalink raw reply
* Re: CRLF problems with Git on Win32
From: Abdelrazak Younes @ 2008-01-09 8:43 UTC (permalink / raw)
To: msysgit-/JYPxA39Uh5TLH3MbocFFw; +Cc: git-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <alpine.LFD.1.00.0801081325010.3148-5CScLwifNT1QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org>
Linus Torvalds wrote:
>
>
> On Tue, 8 Jan 2008, Dmitry Potapov wrote:
>> Perhaps, this option can be called core.autocrlf=safe
>
> We already do half of that:
>
> if (action == CRLF_GUESS) {
> /*
> * We're currently not going to even try to convert stuff
> * that has bare CR characters. Does anybody do that crazy
> * stuff?
> */
> if (stats.cr != stats.crlf)
> return 0;
>
> but we don't check that there are no "naked" LF characters.
>
> So the only thing you'd need to add is to add a
>
> /* No naked LF's! */
> if (safecrlf && stats.lf)
> return 0;
>
> to that sequence too, but the thing is, having mixed line-endings isn't
> actually all that unusual, so I think that kind of "autocrlf=safe" thing
> is actually almost useless - because when that thing triggers, you almost
> always *do* want to convert it to be just one way.
Sorry for the irruption in this discussion but as a potential git user
for cross-platform development I'd like to share my experience/opinion,
hope you don't mind.
I am investigating the use of git for our cross-platform project which
uses svn currently. In our project, we mark manually *all* source file
(*.h and *.cpp) with 'eol-style=native'. This way, if some editor on
Windows added some CRLF in such marked file, svn will refuse to commit
this file until you clean it up. This means that all C/C++/python files
uses LF eol exclusively on all platforms. I believe this is the only
sane way to do cross-platform development.
Now, marking any new file manually is cumbersome and some developers
often forget to do it. I would like to be able mark all files with a
given extension (.c, .cpp, .h) with "LF only". This way, Windows only
files (like visual studio projects) can stay with CRLF. It would be
fantastic if git could do that.
>
> I've seen it multiple times when people cooperate with windows files with
> unix tools, where unix editors often preserve old CRLF's, but write new
> lines with just LF.
Multiple versions of Visual studio do just this indeed.
Abdel.
^ permalink raw reply
* Re: [PATCH] - Added recurse command to git submodule
From: Imran M Yousuf @ 2008-01-09 8:55 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vmyrfjsw1.fsf@gitster.siamese.dyndns.org>
Hi Junio,
Thanks for the feedback and specially thank you for the coding
standard documentation, I was looking for it. I will make fixes to
both the patches and email them again. Will send this patch again,
probably tomorrow.
Thank you,
Imran
On Jan 9, 2008 2:38 PM, Junio C Hamano <gitster@pobox.com> wrote:
> imyousuf@gmail.com writes:
>
> > From: Imran M Yousuf <imyousuf@smartitengineering.com>
> >
> >
> > - The purpose of the recurse command in the git submodule is to recurse
> > a command in its submodule at depths specified. For example if one wants
> > to do a diff on its project with submodules at once, one can simply do
> > git-submodule recurse diff HEAD and would see the diff for all the modules
> > it contains. If during recursion it is found that a module has not been
> > initialized or updated than the command also initializes and updates them.
> > It is to be noted that argument specification to the command intended (in
> > above example diff) to recurse will be same as its original command (i.e.
> > git diff). If the original command spec changes it will have no effect on
> > the recurse command. Depth of recursion can be specified simply by
> > mentioning the -d <recursion depth> argument to recurse command. If not
> > specified or specified to 0, it will be comprehended to recurse at all
> > depth; if a positive number is specified than maximum recursion depth will
> > never cross that depth. In order to see some information -v option may be
> > used
>
> The indentation style seems, eh, unique. An overlong single
> paragraph with solid run of sentences is somewhat hard to read.
>
> I am not still convinced that a subcommand other than init,
> which is started recursively, should initialize and update
> submodules that are uninitialized. Currently there is no way,
> other than not having an initialized submodule repository, to
> mark that the user is _not_ interested in a particular
> submodule, and you will lose that if you make recursing into
> uninitialized ones too easy and aggressive.
>
> I suspect that it might be a saner approach to:
>
> - allow "git submodule recurse init [-d depth]" (although I am
> not sure if limit by depth is so useful in practice -- only
> experience will tell us) to auto-initialize the uninitialized
> submodules;
>
> - allow any other "git submodule recurse $cmd" to run
> recursively to _any_ depth, but never auto-initialize the
> uninitialized submodules.
>
> In other words, I think it is a very bad idea to rob the
> existing mechanism from the user to mark uninteresting
> submodules by not initializing them. An alternative would be to
> give them other means to mark "I am not interested in these
> submodules", if you insist on this auto-initialization, but I do
> not offhand think of a mechanism that is easier to use than what
> we already have (i.e. not checking them out).
>
> > @@ -17,6 +17,9 @@ status=
> > quiet=
> > cached=
> > command=
> > +recurse_verbose=0
> > +recursion_depth=0
> > +current_recursion_depth=0
>
> I wonder if we want this "verbose" option to be specific to the
> recurse subcommand, or perhaps other subcommands may want to
> support more verbose output mode, even if they currently don't.
> Perhaps the variable should be just called $verbose?
>
> > +# Initializes the submodule if already not initialized
> > +initialize_sub_module() {
> > + if [ ! -d "$1"/.git ]; then
> > + if [ $recurse_verbose -eq 1 ]; then
> > + echo Initializing and updating "$1"
>
> That's a sensible message for the $verbose mode.
>
> > + fi
> > + git-submodule init "$1"; git-submodule update "$1"
>
> Can init fail for repository "$1"? Do you want to proceed
> updating it if it does?
>
> > +# This actually traverses the module; checks
> > +# whether the module is initialized or not.
> > +# if not initialized, then done so and then the
> > +# intended command is evaluated. Then it
> > +# recursively goes into it modules.
> > +traverse_module() {
> > + if [ $recurse_verbose -eq 1 ]; then
> > + echo Current recursion depth: "$current_recursion_depth"
> > + fi
> > + initialize_sub_module "$1"
> > + (
> > + submod_path="$1"
> > + shift
> > + cd "$submod_path"
> > + if [ $recurse_verbose -eq 1 ]; then
> > + echo Working in mod "$submod_path" @ `pwd` with "$@" \("$#"\)
>
> Is this a sensible message, I have to wonder... Saying `pwd`
> after already saying "$submod_path" feels more like a debugging
> message than just being $verbose.
>
> > + fi
> > + git "$@"
>
> Is the user interested in exit status from this command? Does
> the user want to continue on to other submodules if this one
> fails?
>
> > + # Check whether submodules exists only if recursion to that depth
> > + # was requested by user
> > + if test "$recursion_depth" -eq 0 || test "$current_recursion_depth" -lt "$recursion_depth"
>
> Overly long line. Perhaps...
>
> if test "$depth" -eq 0 ||
> test "$current_depth" -lt "$depth"
> then
> ...
>
> > + then
> > + if [ -f .gitmodules ]; then
> > + for mod_path in `sed -n -e 's/path = //p' .gitmodules`; do
> > + export current_recursion_depth=$(echo "$current_recursion_depth+1" | bc)
>
> (1) I do not think you need to export this variable;
>
> (2) It was reported some shells that we intend to support
> mishandle "export VAR=VAL" construct and we tend to write
> this "VAR=VAL" followed by "export VAR" as two separate
> commands on two separate lines;
>
> (3) We do not use "bc" (and traditionally, shell scripts
> outside git don't, either) in scripts.
>
> So, I think:
>
> current_recursion_depth=$(( $current_recursion_depth + 1 ))
>
> is enough, although the variable name feels overly long.
>
> Probably I would even do:
>
> if test "$depth -ne 0 && test "$current_depth" -ge "$depth"
> then
> exit 0
> fi
> if test -f .gitmodules
> then
> current_recursion_depth=$(( $current_recursion_depth + 1 ))
> for p in $(sed -n -e 's/path = ....)
> do
> traverse_module "$p" "$@"
> done
> fi
>
> which would avoid one level of nesting (and indentation), and
> removes unnecessary increment and decrement inside the loop.
> You are in your own subprocess, so your increment will not
> affect the process that called you, and after leaving the loop
> the only thing you do is just to exit the subprocess.
>
> > +# Propagates or recurses over all the submodules at any
> > +# depth with any git command, e.g. git-clone, git-status,
> > +# git-commit etc., with the arguments supplied exactly as
> > +# it would have been supplied to the command otherwise.
> > +# This actually starts the recursive propagation
> > +modules_recurse() {
> > + project_home=`pwd`
> > + echo Project Home: "$project_home"
>
> Doesn't this message belong to $verbose mode?
>
> > + if [ $recurse_verbose -eq 1 ]; then
> > + if [ $recursion_depth = 0 ]; then
> > + echo Recursion will go to all depths
> > + else
> > + echo Recursion depth specified to "$recursion_depth"
> > + fi
>
> These sounds more like debugging message than $verbose.
>
> > + fi
> > + if [ -d "$project_home"/.git/ ]; then
> > + if [ $recurse_verbose -eq 1 ]; then
> > + echo Command to recurse: git "$@"
>
> Likewise -- you will repeat that inside traverse_module anyway.
>
> > + fi
> > + git "$@"
> > + if [ -f .gitmodules ]; then
> > + for mod_path in `sed -n -e 's/path = //p' .gitmodules`; do
> > + export current_recursion_depth=1
> > + traverse_module $mod_path "$@"
> > + done
> > + fi
>
> Do I see a code duplication here? Why isn't this done as the
> first level recursion inside traverse_module? _Even_ _if_ you
> insist auto-initializing submodules, by moving the
> initialize_sub_module call in traverse_module a bit down
> (namely, before it recursively invoke traverse_module itself and
> make it auto initialize $submod_path, not "$1"), I think you can
> remove this duplication.
>
> > + else
> > + echo "$project_home" not a git repo thus exiting
> > + exit
>
> And its exit code is 0 which tells the caller that there is no
> error?
>
> > + fi
> > + unset current_recursion_depth
>
> The reason for this unset is...?
>
> > @@ -326,6 +405,37 @@ do
> > --cached)
> > command="modules_list"
> > ;;
> > + recurse)
> > + command="modules_$1"
> > + repeat=1
> > + while test $repeat = 1
> > + do
>
> You do not need that $repeat thing. Just use "break", like this.
>
> while :
> do
> case ... in
> ...
> *)
> break ;;
> esac
> done
>
> > + -d)
> > + if [ -z $3 ]; then
>
> (minor style)
>
> if test -z "$3"
> then
> ...
>
> > + echo No \<recursion depth\> specified
> > + usage
> > + elif [ `expr "$3" : '[1-9][0-9]*.*'` -eq `expr "$3" : '.*'` ]; then
> > + recursion_depth="$3"
> > + shift
> > + shift
> > + else
> > + echo \<recursion depth\> not an integer
> > + usage
> > + fi
> > + ;;
>
> Instead of checking integerness by hand, it would probably be
> much simpler if you did something like this:
>
> depth="$3"
> depth=$(( $depth + 0 ))
> if test "$depth" != "$3"
> then
> die "what's that '$3' for?"
> else
> : happy
> fi
>
> For a rough guideline of shell constructs we use (and do not
> use), please see Documentation/CodingGuidelines.
>
--
Imran M Yousuf
Entrepreneur & Software Engineer
Smart IT Engineering
Dhaka, Bangladesh
Email: imran@smartitengineering.com
Mobile: +880-1711402557
^ permalink raw reply
* Re: [PATCH] Simplified the invocation of command action in submodule
From: Johannes Sixt @ 2008-01-09 8:59 UTC (permalink / raw)
To: imyousuf; +Cc: git, gitster, Imran M Yousuf
In-Reply-To: <1199851140-31853-1-git-send-email-imyousuf@gmail.com>
imyousuf@gmail.com schrieb:
> @@ -16,6 +16,7 @@ update=
> status=
> quiet=
> cached=
> +command=
>
> #
> # print stuff on stdout unless -q was specified
> @@ -293,20 +294,23 @@ modules_list()
> done
> }
>
> +# command specifies the whole function name since
> +# one of theirs prefix is module not modules
> while test $# != 0
> do
> case "$1" in
> add)
> add=1
> + command="module_$1"
> ;;
> init)
> - init=1
> + command="modules_$1"
> ;;
> update)
> - update=1
> + command="modules_$1"
> ;;
> status)
> - status=1
> + command="modules_list"
> ;;
> -q|--quiet)
> quiet=1
> @@ -320,7 +324,7 @@ do
> branch="$2"; shift
> ;;
> --cached)
> - cached=1
> + command="modules_list"
Don't remove cached=1 because otherwise --cached is effectively ignored.
> ;;
> --)
> break
> @@ -345,20 +349,8 @@ case "$add,$branch" in
> ;;
> esac
>
> -case "$add,$init,$update,$status,$cached" in
> -1,,,,)
> - module_add "$@"
> - ;;
> -,1,,,)
> - modules_init "$@"
> - ;;
> -,,1,,)
> - modules_update "$@"
> - ;;
> -,,,*,*)
> - modules_list "$@"
> - ;;
> -*)
> +if [ -z $command ]; then
> usage
> - ;;
> -esac
> +else
> + "$command" "$@"
> +fi
- Previously 'git submodule' was equvalent to 'git submodule status', now
it is an error.
- Previously, passing --cached to add, init, or update was an error, now
it is not.
-- Hannes
^ permalink raw reply
* Re: [PATCH] Simplified the invocation of command action in submodule
From: Imran M Yousuf @ 2008-01-09 9:07 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git
In-Reply-To: <47848CDD.7050806@viscovery.net>
I already saw that mistake Johannes, thank you for pointing it out.
On Jan 9, 2008 2:59 PM, Johannes Sixt <j.sixt@viscovery.net> wrote:
> imyousuf@gmail.com schrieb:
>
> > @@ -16,6 +16,7 @@ update=
> > status=
> > quiet=
> > cached=
> > +command=
> >
> > #
> > # print stuff on stdout unless -q was specified
> > @@ -293,20 +294,23 @@ modules_list()
> > done
> > }
> >
> > +# command specifies the whole function name since
> > +# one of theirs prefix is module not modules
> > while test $# != 0
> > do
> > case "$1" in
> > add)
> > add=1
> > + command="module_$1"
> > ;;
> > init)
> > - init=1
> > + command="modules_$1"
> > ;;
> > update)
> > - update=1
> > + command="modules_$1"
> > ;;
> > status)
> > - status=1
> > + command="modules_list"
> > ;;
> > -q|--quiet)
> > quiet=1
> > @@ -320,7 +324,7 @@ do
> > branch="$2"; shift
> > ;;
> > --cached)
> > - cached=1
> > + command="modules_list"
>
> Don't remove cached=1 because otherwise --cached is effectively ignored.
>
> > ;;
> > --)
> > break
> > @@ -345,20 +349,8 @@ case "$add,$branch" in
> > ;;
> > esac
> >
> > -case "$add,$init,$update,$status,$cached" in
> > -1,,,,)
> > - module_add "$@"
> > - ;;
> > -,1,,,)
> > - modules_init "$@"
> > - ;;
> > -,,1,,)
> > - modules_update "$@"
> > - ;;
> > -,,,*,*)
> > - modules_list "$@"
> > - ;;
> > -*)
> > +if [ -z $command ]; then
> > usage
> > - ;;
> > -esac
> > +else
> > + "$command" "$@"
> > +fi
>
> - Previously 'git submodule' was equvalent to 'git submodule status', now
> it is an error.
>
> - Previously, passing --cached to add, init, or update was an error, now
> it is not.
>
> -- Hannes
>
--
Imran M Yousuf
Entrepreneur & Software Engineer
Smart IT Engineering
Dhaka, Bangladesh
Email: imran@smartitengineering.com
Mobile: +880-1711402557
^ permalink raw reply
* Re: [PATCH] Simplified the invocation of command action in submodule
From: Johannes Sixt @ 2008-01-09 9:15 UTC (permalink / raw)
To: Imran M Yousuf; +Cc: git
In-Reply-To: <7bfdc29a0801090107j292eeaf5u2b49651ed23ca783@mail.gmail.com>
-- Hannes
of them.
"that mistake" with no clue on which one you mean when I pointed out three
BTW, on this list we don't top-post. In particular not when you write only
Imran M Yousuf schrieb:
> I already saw that mistake Johannes, thank you for pointing it out.
>
> On Jan 9, 2008 2:59 PM, Johannes Sixt <j.sixt@viscovery.net> wrote:
>> imyousuf@gmail.com schrieb:
>>
[...]
^ permalink raw reply
* Re: [PATCH] Simplified the invocation of command action in submodule
From: Imran M Yousuf @ 2008-01-09 9:51 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git, gitster
In-Reply-To: <47848CDD.7050806@viscovery.net>
On Jan 9, 2008 2:59 PM, Johannes Sixt <j.sixt@viscovery.net> wrote:
> imyousuf@gmail.com schrieb:
>
> > @@ -16,6 +16,7 @@ update=
> > status=
> > quiet=
> > cached=
> > +command=
> >
> > #
> > # print stuff on stdout unless -q was specified
> > @@ -293,20 +294,23 @@ modules_list()
> > done
> > }
> >
> > +# command specifies the whole function name since
> > +# one of theirs prefix is module not modules
> > while test $# != 0
> > do
> > case "$1" in
> > add)
> > add=1
> > + command="module_$1"
> > ;;
> > init)
> > - init=1
> > + command="modules_$1"
> > ;;
> > update)
> > - update=1
> > + command="modules_$1"
> > ;;
> > status)
> > - status=1
> > + command="modules_list"
> > ;;
> > -q|--quiet)
> > quiet=1
> > @@ -320,7 +324,7 @@ do
> > branch="$2"; shift
> > ;;
> > --cached)
> > - cached=1
> > + command="modules_list"
>
> Don't remove cached=1 because otherwise --cached is effectively ignored.
>
> > ;;
> > --)
> > break
> > @@ -345,20 +349,8 @@ case "$add,$branch" in
> > ;;
> > esac
> >
> > -case "$add,$init,$update,$status,$cached" in
> > -1,,,,)
> > - module_add "$@"
> > - ;;
> > -,1,,,)
> > - modules_init "$@"
> > - ;;
> > -,,1,,)
> > - modules_update "$@"
> > - ;;
> > -,,,*,*)
> > - modules_list "$@"
> > - ;;
> > -*)
> > +if [ -z $command ]; then
> > usage
> > - ;;
> > -esac
> > +else
> > + "$command" "$@"
> > +fi
>
> - Previously 'git submodule' was equvalent to 'git submodule status', now
> it is an error.
Yes, I forgot to add that status is the default command. Thanks for
pointing it out.
>
> - Previously, passing --cached to add, init, or update was an error, now
> it is not.
The usage statement and this behaviour is rather contradicting. The
usage says that --cached can be used with all commands; so I am not
sure whether using --cached with add should be an error or not. IMHO,
if the previous implementation was right than the USAGE has to be
changed, and if the previous implementation was incorrect, than if the
default command is set to status than current implementation is right.
I would like to get comment on this until I fix the patch and resend it.
>
> -- Hannes
>
Thank you,
--
Imran M Yousuf
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox