Git development
 help / color / mirror / Atom feed
* Re: [PATCH] Teach remote machinery about remotes.default config variable
From: Johannes Schindelin @ 2008-01-14 11:18 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Mark Levedahl, git
In-Reply-To: <7vir1xmazm.fsf@gitster.siamese.dyndns.org>

Hi,

On Sun, 13 Jan 2008, Junio C Hamano wrote:

> And we already have "clone -o" and claim to support that option.

My understanding was _always_ that the "-o" option was meant for the case 
that you want to clone from somewhere else than where you want to pull 
from.  Something like an initial clone from a USB disk.

>  * test scripts to use valgrind (Jeff King, but there was another
>    one in the past -- can their efforts compared and coordinated
>    better?).

Yes, that was written in Perl by Christian Couder:

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

Peff's version does not need Perl, and is better integrated with the 
testsuite (via the new option -m).  Christian's version parses the output, 
and might therefore be nicer to look at.

However, I think that both versions do not account for scripts, and I 
imagine that going through Git.pm and git-sh-setup is necessary for that.

Also, it might be a good idea to be able to provide extra arguments, such 
as "--attach-db=yes".

Post 1.5.4, definitely.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] git-remote - Unset core.origin when deleting the default remote
From: Jeff King @ 2008-01-14 11:05 UTC (permalink / raw)
  To: Mark Levedahl; +Cc: gitster, git
In-Reply-To: <1200241631-3300-2-git-send-email-mlevedahl@gmail.com>

On Sun, Jan 13, 2008 at 11:27:08AM -0500, Mark Levedahl wrote:

> --- a/git-remote.perl
> +++ b/git-remote.perl
> @@ -328,6 +328,11 @@ sub rm_remote {
>  
>  	$git->command('config', '--remove-section', "remote.$name");
>  
> +	my $defremote = $git->config("core.origin");
> +	if (defined $defremote && $defremote eq $name) {
> +	       $git->command("config", "--unset", "core.origin");
> +	}
> +

I'm not sure I see the use case that this helps. Presumably you are
doing one of (assuming your core.origin is 'foo'):

  - delete 'foo', and then proceed with usual git commands. In this
    case, your core.origin has reverted to 'origin', but what is the
    chance that you actually have such a remote (since you presumably
    cloned with -o foo)?

  - delete 'foo', then re-add 'foo'. I would expect this to be
    equivalent to editing the config, but as a side effect, your
    core.origin has mysteriously changed.

  - delete 'foo', then re-add 'bar' with the intent of making it your
    new origin. This doesn't help at all, since there's nothing
    automatically setting core.origin to 'bar', so you might as well
    leave it as the bogus 'foo' rather than the bogus 'origin'. And to
    help this use case, something like a "-d" flag to git-remote to set
    the new origin as the default might make sense. I.e.,

      git remote rm foo
      git remote add -d bar git://bar/project.git

    Alternatively, when adding a remote, if it is the _only_ remote (or
    perhaps if the current core.origin doesn't exist), we could set
    core.origin which would automagically cover the latter two cases.
    Although it feels a little too DWIM.

-Peff

^ permalink raw reply

* Re: Revert git push
From: Markus Korber @ 2008-01-14 10:39 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: git
In-Reply-To: <478627E2.4060808@viscovery.net>

Thus spake Johannes Sixt:

> Markus Korber schrieb:
>
>> revert this push if nobody has pushed something since my last pull from
>> the linux repository?
>
> $ cd /the/linux/repo
> $ git checkout 94545baded
> $ git branch -f master 94545baded
> $ git checkout master
>
> Omit the two checkouts if the repo is bare or if it did not have 'master'
> checked out.

Thanks a lot, that worked like a charm.

Regards,
Markus Korber

^ permalink raw reply

* Re: CRLF problems with Git on Win32
From: David Kågedal @ 2008-01-14  9:41 UTC (permalink / raw)
  To: git
In-Reply-To: <alpine.LFD.1.00.0801111005360.3148@woody.linux-foundation.org>

Linus Torvalds <torvalds@linux-foundation.org> writes:

> On Fri, 11 Jan 2008, Steffen Prohaska wrote:
>> 
>> Ah sorry, I misunderstood you in [1].  I thought your last point
>> "Mixed Windows usage" meant what I have in mind:  A user working
>> in a mixed Windows/Unix environment who creates a file using
>> Windows tools and commits it in the Unix environment.  In this
>> case the CRLF file will be transferred from Windows to Unix
>> without git being involved.  The right thing for git on Unix is
>> to remove CRLF during a commit but still write only LF during
>> check out.  So autocrlf=input is the right choice.
>
> Oh, ok, I didn't realize.
>
> But yes, if you use a network share across windows and Unixand actually 
> *share* the working tree over it, then yes, you'd want "autocrlf=input" on 
> the unix side.
>
> However, I think that falls under the "0.1%" case, not the "99.9%" case.
>
> I realize that people probably do that more often with centralized 
> systems, but with a distributed thing, it probably makes a *ton* more 
> sense to have separate trees. But I could kind of see having a shared 
> development directory and accessing it from different types of machines 
> too.

One case is when you only want to commit compiling code, and to
test-compile on all platforms that you are supposed to be portable to
you need to access the source tree on different systems before
committing anything.

You could of course commit optimistically and checkout on the other
system, and then go back and rewrite the commits if you need to fix
something. But that is a lot more work.

-- 
David Kågedal

^ permalink raw reply

* Re: safecrlf not in 1.5.4
From: Dmitry Potapov @ 2008-01-14  9:04 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Steffen Prohaska, Johannes Schindelin, Mark Levedahl,
	Git Mailing List, msysGit
In-Reply-To: <7vejcklv84.fsf-jO8aZxhGsIagbBziECNbOZn29agUkmeCHZ5vskTnxNA@public.gmane.org>


On Sun, Jan 13, 2008 at 11:30:51PM -0800, Junio C Hamano wrote:
> 
> But we could end up having a short cycle for 1.5.5 if we agree
> that the lack of crlf=safe is a severe bug that is worth fixing
> post 1.5.4.

Hopefully, the cycle for 1.5.5 will be a bit shorter than 1.5.4, because
1.5.4 seems to have the longest development cycle of all versions, and
it already contains almost as much changes as three previous versions
("git diff v1.5.3 master" is almost as big as "git diff v1.5.0 v1.5.3").

Dmitry

^ permalink raw reply

* Re: safecrlf not in 1.5.4
From: Steffen Prohaska @ 2008-01-14  8:29 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Johannes Schindelin, Mark Levedahl, Git Mailing List, msysGit
In-Reply-To: <7vejcklv84.fsf-jO8aZxhGsIagbBziECNbOZn29agUkmeCHZ5vskTnxNA@public.gmane.org>



On Jan 14, 2008, at 8:30 AM, Junio C Hamano wrote:

>
> Steffen Prohaska <prohaska-wjoc1KHpMeg@public.gmane.org> writes:
>
>> Do you see a chance to have safecrlf in 1.5.4.1?
>
> By definition of 'maint', 1.5.4.X are to fix bugs in the
> features that are in 1.5.4, so the answer is no.

I expected this answer.


> But we could end up having a short cycle for 1.5.5 if we agree
> that the lack of crlf=safe is a severe bug that is worth fixing
> post 1.5.4.
>
> Currently I am not convinced that the lack of crlf=safe is a
> severe and urgent bug worth "fixing" for 1.5.4.  I see it as a
> new feature, a different variant of crlf behaviour we will be
> introducing for audience who are not satisfied with existing the
> crlf variants.


So I should try harder to find better arguments.  But this has
time until the 1.5.4 release is out.  For now, I am being quiet.

(Well, I'll continue to improve the safecrlf patch and most
likely will send it to the list, too.  But I don't expect to find
it anywhere in your repo before 1.5.4.)

	Steffen

^ permalink raw reply

* Re: safecrlf not in 1.5.4
From: Junio C Hamano @ 2008-01-14  7:30 UTC (permalink / raw)
  To: Steffen Prohaska
  Cc: Johannes Schindelin, Mark Levedahl, Git Mailing List, msysGit
In-Reply-To: <31687420-EB17-4651-AD6C-07213311ABDA-wjoc1KHpMeg@public.gmane.org>


Steffen Prohaska <prohaska-wjoc1KHpMeg@public.gmane.org> writes:

> Do you see a chance to have safecrlf in 1.5.4.1?

By definition of 'maint', 1.5.4.X are to fix bugs in the
features that are in 1.5.4, so the answer is no.

But we could end up having a short cycle for 1.5.5 if we agree
that the lack of crlf=safe is a severe bug that is worth fixing
post 1.5.4.

Currently I am not convinced that the lack of crlf=safe is a
severe and urgent bug worth "fixing" for 1.5.4.  I see it as a
new feature, a different variant of crlf behaviour we will be
introducing for audience who are not satisfied with existing the
crlf variants.

^ permalink raw reply

* Re: [PATCH 1/2] parse_commit_buffer: don't parse invalid commits
From: Junio C Hamano @ 2008-01-14  7:23 UTC (permalink / raw)
  To: Martin Koegler; +Cc: git
In-Reply-To: <12002478702664-git-send-email-mkoegler@auto.tuwien.ac.at>

Martin Koegler <mkoegler@auto.tuwien.ac.at> writes:

> +	if (!parse_commit_date(bufptr, tail, &item->date))
> +		return error("bogus commit date in object %s", sha1_to_hex(item->object.sha1));
>  
>  	if (track_object_refs) {
>  		unsigned i = 0;

I suspect this might be an undesirable regression.

If somebody managed to create a commit with a bogus "author"
line and wanted to clean up the history, your previous one at
least gave something usable back, even though it had to come up
with a bogus date.  It gave the rest of the data back without
barfing.  And it was easy to see which "resurrected" commit had
a missing author date (bogus ones always gave 0 timestamp).

This round you made it to error out, and callers that check the
return value of parse_commit() would stop traversing the
history, even if the commit in question has perfectly valid
"parent " lines, thinking "ah, this commit object is faulty".
It actively interferes with attempts to resurrect data from
history that contains a faulty commit.

Your previous version was much better with respect to this
issue.  It was about being more careful not to read outside the
commit object buffer, while still allowing the data from a
history that has an unfortunate commit with broken author line
to be resurrected more easily.

I do not think the checks done by fsck and parse_commit should
share the same strictness.  They serve different purposes.

^ permalink raw reply

* Re: Adding Git to Better SCM Initiative : Comparison
From: Dmitry Potapov @ 2008-01-14  6:58 UTC (permalink / raw)
  To: Jakub Narebski
  Cc: Shlomi Fish, git, Eyvind Bernhardsen, David Kastrup,
	Florian Weimer, Chris Shoemaker
In-Reply-To: <200801140131.23027.jnareb@gmail.com>

On Mon, Jan 14, 2008 at 01:31:19AM +0100, Jakub Narebski wrote:
> On Mon, 14 January 2008, Dmitry Potapov wrote:
> 
> > Yes. Git can automatically detects renames and show history together,
> > however being content oriented rather than file oriented, the notion of
> > "retaining the history of the file" can not exactly applied to it.
> 
> "History of a file" can be defined as "<scm> log 'file'", and this is
> well defined also for git. 

You missed the key word here -- *retaining*. In fact, if you define the
history of a file just as something what "<scm> log" produce then what
is the problem with CVS here? Why do most people say that CVS does not
retain file history over rename? Certainly, you can type "cvs old-name"
and see history of one file, and if you type "cvs new-name" then history
of another... But somehow most people think about these two pieces as
being the history of *one* file... So, your definition is incorrect or,
at least, very different from what most people mean by that.

BTW, when you type "git log 'file'", it shows you not history of a file,
but history of changes that affect the specified paths...

> And 'rename support' for file means just
> that this history of a file (of a current file contents) follows file
> renames.

To equate a file with its contents is like to equate a variable with
its value. They are not exactly the same. If you renamed a file and
completely changed its contents, is it still the same file or not?
If you think about file as being inode then the answer is yes, but if
look at its content then the answer is no.

Git tracks contents, while many other SCMs tracks files regardless of
their contents. So, Git can show the history of file contents, but
not the history of a file...

> 
> IIRC this des not work for directories... 

Git works for directories, it is just that the --follow option cannot
applied to it, because this option means to follow the file contents,
which does not make much sense for directories.

> but on the other hand git,
> tracking contents only as a goal, does not track directories.

Exactly.

> > 
> > Git is different in that it tracks the content as the whole rather than
> > tracking a set of files. When you look at some source code, what you
> > really want to know who and why wrote *this*, and usually it does not
> > matter to you whether it was written in this file or another one. CVS
> > is really bad at that, because if you renamed a file, it would be very
> > difficult to go back to history and find that. Many file-ids based SCMs
> > have solved this problem, however, they do not do any better than CVS
> > in another very common case -- when your code is moved around as result
> > of refactoring, but Git addresses both problems, not just one!
> 
> AFAIK Mercurial (hg) is not file-id based, but does explicitely track
> renames. There was even an idea presented on git mailing list to mark
> renames in commit object in some "note" header.

I suspect the main reason why Mercurial support that is that a lot of
programmers whose mind was mangled by many years of CVS experience asked
for that feature. In practice, what you really want to track is contents.
And it is not difficult to add some "note" to the commit and teach Git to
follow it, but I don't see any practical value in that...

> > So, it is not as much about explicit renaming vs automatic, but about
> > different design goals. After finishing reading this questionnaire,
> > it seems to me that a more proper title for it would be "Better CVS
> > Initiative", so it is not surprisingly that Git does not fit into it
> > well. It is like trying to put characteristics of your LCD into a
> > questionnaire for CRT monitors -- some does not make sense, other
> > misleading, and most important ones are not mentioned anyway...
> 
> Please remember that AFAIK this table is _older_ than Git itself.
> But it is a fact that some characteristics are much patterned after
> CVS features and misfeatures.

Well, if it is so old, that explains why it gave me such impression...

> 
> It would be much better if for each feature there was some test
> described which would allow to check if the feature is supported.

Wanna test your LCD monitor with some old CRT tests? -:)

> 
> By the way, even before "git log --follow" you could have "this file
> was renamed to that file" in the commit/revision patchset. 

You could write that in CVS message too, but I don't think it can
be considered as retaining the history of the file...


Dmitry

^ permalink raw reply

* Re: [PATCH] Don't display crlf warning twice
From: Steffen Prohaska @ 2008-01-14  6:53 UTC (permalink / raw)
  To: Dmitry Potapov; +Cc: gitster, torvalds, git
In-Reply-To: <20080114064036.GX2963@dpotapov.dyndns.org>


On Jan 14, 2008, at 7:40 AM, Dmitry Potapov wrote:

> On Mon, Jan 14, 2008 at 07:17:26AM +0100, Steffen Prohaska wrote:
>>
>> Your traces reveal that it is a racy condition that can trigger
>> the double warnings.
>
> It is not a racy condition. It is just another branch in the
> code, which happens when an existing file is modified. Your
> tests always added a new file, so they could not reproduce
> the problem.
>
>>
>> Do you have a test case that reliably triggers the second call to
>> convert_to_git()?
>
> ===========================================
> git config core.autocrlf input
>
> echo Hello > foo
> git add foo
> git commit -m 'add foo'
>
> echo Hello >> foo
> unix2dos foo
> # This should trigger double crlf warning
> git add foo
> ===========================================

Yes, this reproduces the problem.

Thanks,
Steffen

^ permalink raw reply

* safecrlf not in 1.5.4 (was Re: [PATCH] Teach remote machinery about remotes.default config variable)
From: Steffen Prohaska @ 2008-01-14  6:49 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Johannes Schindelin, Mark Levedahl, Git Mailing List, msysGit
In-Reply-To: <7vir1xmazm.fsf@gitster.siamese.dyndns.org>


On Jan 14, 2008, at 2:50 AM, Junio C Hamano wrote:

> Recently I looked at the following patches and topics but I do
> not think any of them belongs to 1.5.4.  None of them is obvious
> and trivially correct fix to regressions or serious existing
> bugs:
>
[...]


>  * crlf (Steffen Prohaska and Dmitry Potapov)

[...]


> I am hoping that authors will resend the ones they really care
> about after 1.5.4, as I do not want to take patches early.

Thanks for your update on this.  I agree with your opinion,
although I'd prefer to have the safecrlf option soon.

Without safecrlf I'll not enable core.autocrlf=true in msysgit.
Now that I see a reasonable way of having at least a warning
about potential data corruption when core.autocrlf=true, I'm
even stronger against enabling it without a safety valve.

As I pointed out in the recent CRLF discussion, I believe the
problem is not specific to Windows but is about a reasonable
default configuration for cross-platform projects.  CRLF
conversion must be enabled on all platforms to have good defaults
for a mixed Unix/Windows environment, and hence the safecrlf if
also needed on all platforms.

So I don't see much value in having the safecrlf only in msysgit
and not in official git.

Junio,
Do you see a chance to have safecrlf in 1.5.4.1?

I am currently considering wether it is worth following the maint
series of official git with msysgit.  That is we'd have a maint
branch in msysgit, which would merge from Junio's maint.
Although we're still in preview mode with msysgit I tend to
believe that this would be a good idea.

The preview tag is mostly due to the unspecified set of features
of msysgit, not that I think part of msysgit is not already very
stable and usable.  But msysgit only supports a subset of the
commands of official git and we don't really say or even know
which commands currently work reliably.  It could be worth to
compile such a list and only install the commands that we are
convinced of being ready for production.

	Steffen

^ permalink raw reply

* Re: [PATCH 1/2] parse_commit_buffer: don't parse invalid commits
From: Martin Koegler @ 2008-01-14  6:46 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vzlv9nwdj.fsf@gitster.siamese.dyndns.org>

On Sun, Jan 13, 2008 at 03:23:04PM -0800, Junio C Hamano wrote:
> I think I saw this patch and vaguely recall commenting on it,
> but your message is not very helpful locating the previous
> thread to see what kind of improvements are made to this new
> round.

http://marc.info/?l=git&m=119965688320973&w=2

mfg Martin Kögler

^ permalink raw reply

* Re: [PATCH] Don't display crlf warning twice
From: Dmitry Potapov @ 2008-01-14  6:40 UTC (permalink / raw)
  To: Steffen Prohaska; +Cc: gitster, torvalds, git
In-Reply-To: <B4297A2E-9F8F-4167-AB48-10AC1FB4E1D5@zib.de>

On Mon, Jan 14, 2008 at 07:17:26AM +0100, Steffen Prohaska wrote:
> 
> Your traces reveal that it is a racy condition that can trigger
> the double warnings.

It is not a racy condition. It is just another branch in the
code, which happens when an existing file is modified. Your
tests always added a new file, so they could not reproduce
the problem.

> 
> Do you have a test case that reliably triggers the second call to
> convert_to_git()?

===========================================
git config core.autocrlf input

echo Hello > foo
git add foo
git commit -m 'add foo'

echo Hello >> foo
unix2dos foo
# This should trigger double crlf warning
git add foo
===========================================

Dmitry

^ permalink raw reply

* Re: [PATCH] Don't display crlf warning twice
From: Steffen Prohaska @ 2008-01-14  6:17 UTC (permalink / raw)
  To: Dmitry Potapov; +Cc: gitster, torvalds, git
In-Reply-To: <1200267979-17683-1-git-send-email-dpotapov@gmail.com>

Dimitry,
Thanks for finding the code path that leads to double printing
the warning.

Your traces reveal that it is a racy condition that can trigger
the double warnings.  In my test cases this apparently does not
happen, at least not on my machine.

Do you have a test case that reliably triggers the second call to
convert_to_git()?



On Jan 14, 2008, at 12:46 AM, Dmitry Potapov wrote:

> 'git add' could call crlf_to_git() twice, and this caused that the  
> same
> crlf warning being display twice. The first time crlf_to_git() is  
> called
> when a file is added to index, and it could be called the second time
> during writing the index.
>
> This patches sets safe_crlf to false before the second call.

This would certainly work.

However I do not think it is the right place to fix ...


> Signed-off-by: Dmitry Potapov <dpotapov@gmail.com>
> ---
>
> On Sun, Jan 13, 2008 at 05:30:47PM +0100, Steffen Prohaska wrote:
>> _
>> I mentioned earlier that crlf_to_git() would be called twice.   
>> Unfortunately,
>> I can't reproduce this behaviour and are not even sure if it ever  
>> happend.
>
> I think I have found the cause. It can be seen from the following  
> trace:
>
> ==============
> #0  crlf_to_git (path=0x814b37e "a", src=0xb7fb4000 "Hello\r\nHello 
> \r\nHello\n", len=20, buf=0xbfad504c, action=-1) at convert.c:89
> #1  0x080e0454 in convert_to_git (path=0x814b37e "a",  
> src=0xb7fb4000 "Hello\r\nHello\r\nHello\n", len=20, dst=0xbfad504c)  
> at convert.c:578
> #2  0x080b7194 in index_fd (sha1=0xbfad508c "", fd=7,  
> st=0xbfad50d8, write_object=0, type=OBJ_BLOB, path=0x814b37e "a")  
> at sha1_file.c:2345

Here, index_fd() is called with write_object=0 ...


> #3  0x080a7dd4 in ce_compare_data (ce=0x814b340, st=0xbfad50d8) at  
> read-cache.c:56
> #4  0x080a8045 in ce_modified_check_fs (ce=0x814b340,  
> st=0xbfad50d8) at read-cache.c:111
> #5  0x080aa66d in ce_smudge_racily_clean_entry (ce=0x814b340) at  
> read-cache.c:1121
> #6  0x080aa79d in write_index (istate=0x814a3e0, newfd=6) at read- 
> cache.c:1177
> #7  0x0804c66a in cmd_add (argc=1, argv=0xbfad6408, prefix=0x0) at  
> builtin-add.c:261
> ==============
> #0  crlf_to_git (path=0x814b094 "a", src=0xb7fb4000 "Hello\r\nHello 
> \r\nHello\n", len=20, buf=0xbfad50ec, action=-1) at convert.c:89
> #1  0x080e0454 in convert_to_git (path=0x814b094 "a",  
> src=0xb7fb4000 "Hello\r\nHello\r\nHello\n", len=20, dst=0xbfad50ec)  
> at convert.c:578
> #2  0x080b7194 in index_fd (sha1=0x814b368 "", fd=7, st=0xbfad5174,  
> write_object=1, type=OBJ_BLOB, path=0x814b094 "a") at sha1_file.c:2345

while here index_fd() is called with write_object=1.


> #3  0x080b731f in index_path (sha1=0x814b368 "", path=0x814b094  
> "a", st=0xbfad5174, write_object=1) at sha1_file.c:2377
> #4  0x080a8c00 in add_file_to_index (istate=0x814a3e0,  
> path=0x814b094 "a", verbose=0) at read-cache.c:433
> #5  0x0804c640 in cmd_add (argc=1, argv=0xbfad6408, prefix=0x0) at  
> builtin-add.c:257
> ==============


Without digging deeply through the code, this looks to me as if
we could base our decision wether to print a warning or not on
this difference.

My first take is: we should print a warning if write_object=1
and should be quiet if write_object=0.  The flag can easily be
passed to convert_to_git().

My assumption is that users would only be interested in the
warning if data is really written to the repository (and that
this is the meaning of write_object=1).  But maybe there are
cases where a warning is also appropriate even if the data
is not written?  I don't now.

Another question is if we should die() even if write_object=0;
or only if write_object=1?

	Steffen

^ permalink raw reply

* [PATCH] cvsimport: remove last use of repo-config from git standard tools
From: Dan McGee @ 2008-01-14  4:51 UTC (permalink / raw)
  To: git; +Cc: Dan McGee

git cvsimport was the last tool to use repo-config instead of config. Update
it to use plain git config.

Signed-off-by: Dan McGee <dpmcgee@gmail.com>
---
 git-cvsimport.perl |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index 6d8ff93..5694978 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -88,7 +88,7 @@ sub write_author_info($) {
 	close ($f);
 }
 
-# convert getopts specs for use by git-repo-config
+# convert getopts specs for use by git config
 sub read_repo_config {
     # Split the string between characters, unless there is a ':'
     # So "abc:de" becomes ["a", "b", "c:", "d", "e"]
@@ -96,7 +96,7 @@ sub read_repo_config {
 	foreach my $o (@opts) {
 		my $key = $o;
 		$key =~ s/://g;
-		my $arg = 'git-repo-config';
+		my $arg = 'git config';
 		$arg .= ' --bool' if ($o !~ /:$/);
 
         chomp(my $tmp = `$arg --get cvsimport.$key`);
@@ -116,7 +116,7 @@ getopts($opts) or usage();
 usage if $opt_h;
 
 if (@ARGV == 0) {
-		chomp(my $module = `git-repo-config --get cvsimport.module`);
+		chomp(my $module = `git config --get cvsimport.module`);
 		push(@ARGV, $module) if $? == 0;
 }
 @ARGV <= 1 or usage("You can't specify more than one CVS module");
-- 
1.5.4.rc3

^ permalink raw reply related

* [PATCH] Remove usage of git- (dash) commands from email hook
From: Dan McGee @ 2008-01-14  4:51 UTC (permalink / raw)
  To: git; +Cc: Dan McGee

Switch all git command calls to use the git (space) command format, and
remove the use of git-repo-config in place of git config.

Signed-off-by: Dan McGee <dpmcgee@gmail.com>
---
 contrib/hooks/post-receive-email |   34 +++++++++++++++++-----------------
 1 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/contrib/hooks/post-receive-email b/contrib/hooks/post-receive-email
index 7511ea0..77c88eb 100644
--- a/contrib/hooks/post-receive-email
+++ b/contrib/hooks/post-receive-email
@@ -248,24 +248,24 @@ generate_update_branch_email()
 	# In this case we want to issue an email containing only revisions
 	# 3, 4, and N.  Given (almost) by
 	#
-	#  git-rev-list N ^O --not --all
+	#  git rev-list N ^O --not --all
 	#
 	# The reason for the "almost", is that the "--not --all" will take
 	# precedence over the "N", and effectively will translate to
 	#
-	#  git-rev-list N ^O ^X ^N
+	#  git rev-list N ^O ^X ^N
 	#
-	# So, we need to build up the list more carefully.  git-rev-parse
-	# will generate a list of revs that may be fed into git-rev-list.
+	# So, we need to build up the list more carefully.  git rev-parse
+	# will generate a list of revs that may be fed into git rev-list.
 	# We can get it to make the "--not --all" part and then filter out
 	# the "^N" with:
 	#
-	#  git-rev-parse --not --all | grep -v N
+	#  git rev-parse --not --all | grep -v N
 	#
-	# Then, using the --stdin switch to git-rev-list we have effectively
+	# Then, using the --stdin switch to git rev-list we have effectively
 	# manufactured
 	#
-	#  git-rev-list N ^O ^X
+	#  git rev-list N ^O ^X
 	#
 	# This leaves a problem when someone else updates the repository
 	# while this script is running.  Their new value of the ref we're
@@ -274,10 +274,10 @@ generate_update_branch_email()
 	# all of our commits.  What we really want is to exclude the current
 	# value of $refname from the --not list, rather than N itself.  So:
 	#
-	#  git-rev-parse --not --all | grep -v $(git-rev-parse $refname)
+	#  git rev-parse --not --all | grep -v $(git rev-parse $refname)
 	#
 	# Get's us to something pretty safe (apart from the small time
-	# between refname being read, and git-rev-parse running - for that,
+	# between refname being read, and git rev-parse running - for that,
 	# I give up)
 	#
 	#
@@ -295,7 +295,7 @@ generate_update_branch_email()
 	# As above, we need to take into account the presence of X; if
 	# another branch is already in the repository and points at some of
 	# the revisions that we are about to output - we don't want them.
-	# The solution is as before: git-rev-parse output filtered.
+	# The solution is as before: git rev-parse output filtered.
 	#
 	# Finally, tags: 1 --- 2 --- O --- T --- 3 --- 4 --- N
 	#
@@ -305,7 +305,7 @@ generate_update_branch_email()
 	# for a branch update.  Therefore we still want to output revisions
 	# that have been output on a tag email.
 	#
-	# Luckily, git-rev-parse includes just the tool.  Instead of using
+	# Luckily, git rev-parse includes just the tool.  Instead of using
 	# "--all" we use "--branches"; this has the added benefit that
 	# "remotes/" will be ignored as well.
 
@@ -454,7 +454,7 @@ generate_update_atag_email()
 #
 generate_atag_email()
 {
-	# Use git-for-each-ref to pull out the individual fields from the
+	# Use git for-each-ref to pull out the individual fields from the
 	# tag
 	eval $(git for-each-ref --shell --format='
 	tagobject=%(*objectname)
@@ -572,7 +572,7 @@ generate_general_email()
 	else
 		# What can we do here?  The tag marks an object that is not
 		# a commit, so there is no log for us to display.  It's
-		# probably not wise to output git-cat-file as it could be a
+		# probably not wise to output git cat-file as it could be a
 		# binary blob.  We'll just say how big it is
 		echo "$newrev is a $newrev_type, and is $(git cat-file -s $newrev) bytes long."
 	fi
@@ -622,10 +622,10 @@ then
 	projectdesc="UNNAMED PROJECT"
 fi
 
-recipients=$(git repo-config hooks.mailinglist)
-announcerecipients=$(git repo-config hooks.announcelist)
-envelopesender=$(git-repo-config hooks.envelopesender)
-emailprefix=$(git-repo-config hooks.emailprefix || echo '[SCM] ')
+recipients=$(git config hooks.mailinglist)
+announcerecipients=$(git config hooks.announcelist)
+envelopesender=$(git config hooks.envelopesender)
+emailprefix=$(git config hooks.emailprefix || echo '[SCM] ')
 
 # --- Main loop
 # Allow dual mode: run from the command line just like the update hook, or
-- 
1.5.4.rc3

^ permalink raw reply related

* Re: [PATCH] Teach remote machinery about remotes.default config variable
From: Junio C Hamano @ 2008-01-14  4:42 UTC (permalink / raw)
  To: Mark Levedahl; +Cc: Johannes Schindelin, git
In-Reply-To: <478AD5A0.50900@gmail.com>

Mark Levedahl <mlevedahl@gmail.com> writes:

> ... if what
> we need to fetch from is in fact not origin (even though the master
> project knows what server to access), submodule update in fact doesn't
> even work ...

This is an interesting point.

Perhaps git-submodule.sh::modules_update should use $url it
obtains from the configuration in the upper level when running
git-fetch in the submodule.

If you view the problem this way, your earlier "git fetch while
the HEAD is detached always uses 'origin'" may turn out to be a
non-issue.

Which again brings us back to Johannes's earlier point.  If the
issue is about submodule, maybe what needs to be fixes is in
git-submodule, and not the defaulting to 'origin' git-fetch and
friends do.

^ permalink raw reply

* [PATCH] - Introduced 'recurse' command for git-submodule
From: imyousuf @ 2008-01-14  3:23 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. 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.

- The recurse commands behavior can be customized with several arguments
that it accepts. The synopsis for the recurse command is:

	git-submodule [-q|--quiet] recurse [-i|--initialize]
	[-e|--exit-after-error] [-d|--depth <recursion depth>]
	[-df|--depth-first] [-ca|--customized-argument] [-p|--pre-command]
	<command> [<arguments> ...]

- When traversing modules, a module could be uninitialized that is git
submodule init and update has not been called for it; if [-i|--initialize]
option is specified, it will initialize any module that is not initialized;
else if the module is not initialized it will simply skip it.

- There are commands that can fail for a certain submodule but succeed for
others; if one wants to stop execution once the top level module's execution
fails, one can specify [-e|--exit-after-error]. It will ensure that once
execution of git <command> fails in the top level module it will not recurse
into its submodules.

- If the project has submodule hierarchy upto n depth and we want to restrict
recursion to (n-p) depth; we can use the [-d|--depth <recursion depth>] option.
Value has to be greater than 0 and command will at least recurse into the first
depth. If depth is specified to p than all depths <= p will be recursed over.

- While discussion on the recurse command one thing which was put forward
in several occassions is that there might be scenario where a command should be
executed over the child module before the parent module. For such scenario
[-df|--depth-first] option can be used; one use case in particualar presented
as an example is git commit; where almost everybody mentioned that they prefer
to commit the child module before the parent and -df will enable just that.
E.g. p -> a, b, c, e; a ->d is a module structure. If the following command is
used,
	git submodule recurse -df commit -a
it will execute git commit -a in the following sequence - d, a, b, c, e, p.

- There is also another scenario which has been put forward several times in
discussion over the recurse command and it is that commands chould have
different arguments for different modules. For example for the same example
mentioned above, one wants to check a_1 for submdoule a, while it wants to
checkout d_2 for d. It can be achieved by using [-ca|--customized-argument].
This results the script to prompt for user input, which will be passed as
argument to the command for that module.
	git submodule recurse -ca checkout
	Working in mod a .......
	Please provide arguments for this module: a_1
	Working in mod d .......
	Please provide arguments for this module: a_1

- I usually found that when typing a command being able to see some options
come in handy. For example if I can see the available branches before checking
out a branch that would be useful, IOW, if I could git branch before git
checkout; it is now possible using the [-p|--pre-command] option. Using this
command you can actually execute other git commands before specifying the
arguments to the original command. E.g. if the above command is changed to,
	git submodule recurse -ca -p checkout
it will prompt the user for the pre command until one is satisfied and later
the user can actually use them in the argument.

Signed-off-by: Imran M Yousuf <imyousuf@smartitengineering.com>
---
 git-submodule.sh |  183 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 182 insertions(+), 1 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 22e7e5f..ceca985 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -9,7 +9,8 @@
 # git-submodule [-q|--quiet] [status] [-c|--cached] [--] [<path>...]
 # git-submodule [-q|--quiet] init [--] [<path>...]
 # git-submodule [-q|--quiet] update [--] [<path>...]
-USAGE='[-q|--quiet] [[[add [-b|--branch branch] <repo>]|[[[status [-c|--cached]]|init|update] [--]]]  [<path>...]]'
+# git-submodule [-q|--quiet] recurse [-i|--initialize] [-e|--exit-after-error] [-d|--depth <recursion depth>] [-df|--depth-first] [-ca|--customized-argument] [-p|--pre-command] <command> [<arguments> ...]
+USAGE='[-q|--quiet] [[[add [-b|--branch branch] <repo>]|[[[status [-c|--cached]]|init|update] [--]]]  [<path>...]]|[recurse [-i|--initialize] [-e|--exit-after-error] [-d|--depth <recursion depth>] [-df|--depth-first] [-ca|--customized-argument] [-p|--pre-command] <command> [<arguments> ...]]'
 OPTIONS_SPEC=
 . git-sh-setup
 require_work_tree
@@ -21,6 +22,14 @@ branch=
 quiet=
 cached=
 command=
+depth=0
+current_depth=0
+auto_initialize=
+depth_first=
+use_custom_args=
+custom_args=
+pre_cmd=
+on_error=
 
 #
 # print stuff on stdout unless -q was specified
@@ -359,7 +368,174 @@ check_for_terminator()
 	fi
 }
 
+# Initializes the submodule if already not initialized
+# and auto initialize is enabled
+initialize_sub_module()
+{
+	if test ! -d "$1"/.git &&
+	   test -n "$auto_initialize"
+	then
+		say "Initializing and updating $1"
+		git-submodule init "$1" &&
+		git-submodule update "$1" &&
+		return 0
+	# Returns true if module is already initialized
+	elif test -d "$1"/.git
+	then
+		return 0
+	fi
+	say "Module $1 is not initialized and skipped"
+	return 1
+}
+
+# Take command from user and execute it until user wants to discontinue
+do_pre_command()
+{
+	say "Starting pre-comamnd execution!"
+	while :
+	do
+		(
+			read -p "Please provide a git command: " pre_command
+			test -z "$pre_command" || git "$pre_command"
+		)
+		read -p "Press y to continue with another git command... " keypress
+		if test "$keypress" != "y" &&
+			test "$keypress" != "Y"
+		then
+			break
+		fi
+	done
+}
 
+# Take arguments from user to pass as custom arguments
+get_custom_args()
+{
+	read -p "Please provide arguments for this module: " custom_args
+}
+
+traverse_submodule()
+{
+	# If current depth is the range specified than it will continue
+	# else return with success
+	if test "$depth" -gt 0 &&
+		test "$current_depth" -ge "$depth"
+	then
+		return 0;
+	fi
+	# If submodules exists than it will traverse over them
+	if test -f .gitmodules
+	then
+		# Incrementing the depth for the next level of submodules
+		current_depth=$(($current_depth + 1))
+                for mod_path in `sed -n -e 's/path = //p' .gitmodules`; do
+                        traverse_module "$mod_path" "$@"
+                done
+		# Decremented the depth to bring it back to the depth of
+		# the current module
+		current_depth=$(($current_depth - 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()
+{
+	# Will work in the module if and only if the module is initialized
+	initialize_sub_module "$1" &&
+	(
+		submod_path="$1"
+		shift
+		cd "$submod_path"
+		# If depth-first is specified in that case submodules are
+		# are traversed before executing the command on this module
+		test -n "$depth_first" && traverse_submodule "$@"
+		# pwd is mentioned in order to enable the ser to distinguish
+		# between same name modules, e.g. a/lib and b/lib.
+		say "Working in mod $submod_path" @ `pwd` "with $@ ($#)"
+		test -n "$pre_cmd" && do_pre_command
+		test -n "$use_custom_args" && get_custom_args
+		cmd_status=
+		git "$@" "$custom_args" || cmd_status=1
+		# if exit on error is specifed than script will exit if any
+		# command fails. As there is no transaction there will be
+		# no rollback either
+		if  test -n "$cmd_status" && test -n "$on_error"
+		then
+			die "git $@ failed in module $submod_path @ $(pwd)"
+		fi
+		# If depth-first is not specified in that case submodules are
+		# are traversed after executing the command on this module
+		test -z "$depth_first" && traverse_submodule "$@"
+	)
+}
+
+# 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() {
+	while :
+	do
+		case "$1" in
+			-d|--depth)
+				shift
+				if test -z "$1"
+				then
+					echo "No <recursion depth> specified"
+					usage
+				# Arithmatic operation will give an error if depth is not number
+				# thus chose to check intergerness with regular expression
+				elif test "$(expr $1 : '[1-9][0-9]*')" -eq "$(expr $1 : '.*')"
+				then
+					depth="$1"
+				else
+					echo "<recursion depth> not an integer"
+					usage
+				fi
+				;;
+			-df|--depth-first)
+				depth_first=1
+				;;
+			-e|--exit-after-error)
+				on_error=1
+				;;
+			-i|--initialize)
+				auto_initialize=1
+				;;
+			-p|--pre-command)
+				pre_cmd=1
+				;;
+			-ca|--customized-argument)
+				use_custom_args=1
+				;;
+			-*)
+				usage
+				;;
+			*)
+				break
+				;;
+		esac
+		shift
+	done
+	test "$#" -le 0 && die "No git command specified"
+	project_home="$(pwd)"
+	say "Project Home: $project_home"
+	if test "$depth" -gt 0
+	then
+		say Command will recurse upto "$depth" depth
+	fi
+	if test -d "$project_home"/.git/
+	then
+		say "Command to recurse: git $@"
+		traverse_module . "$@"
+	else
+		die "$project_home not a git repo thus exiting"
+	fi
+}
 
 # Command synopsis clearly shows that all arguments after
 # subcommand are arguments to the command itself. Thus
@@ -394,6 +570,11 @@ do
 	-c|--cached)
 		cached=1
 		;;
+	recurse)
+		command="modules_$1"
+		shift
+		break
+		;;
 	--)
 		# It is shifted so that it is not passed
 		# as an argument to the default subcommand
-- 
1.5.3.7

^ permalink raw reply related

* Re: [PATCH] Teach remote machinery about remotes.default config variable
From: Mark Levedahl @ 2008-01-14  3:23 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.LSU.1.00.0801132220200.8333@wbgn129.biozentrum.uni-wuerzburg.de>

Johannes Schindelin wrote:
> Unfortunately, you _still_ fail to even ackonwledge that you still need a 
> lookup, and that I have a point when saying "your patch does not help, 
> since you still have a lookup".
>   
I don't acknowledge this point as it is not true.

I will try to explain one more time.

1 - "origin" is fine as an abstraction IFF everyone on the project can 
reasonably expect that "git fetch origin" will yield the same results 
for anyone who executes that command. As I have tried (and apparently 
failed) to explain, for numerous complex and unchangeable reasons, "git 
fetch origin" is *NOT* expected to give the same results around this 
project and therefore, "origin" is not a useful abstraction.

2 - Until we used submodules, we never defined "origin", but instead 
explicitly named the server using standard nicknames (e.g., "git fetch 
frotz", not "git fetch"). Everyone who can access frotz gets the same 
result. Those who can't, and have to use server "zorft", get a different 
result but it is obvious to all they didn't go to frotz.

3 - As the servers are different and have different content, it is just 
part of the workflow to know the server names and use them. We have 
agreed upon a common set of server nicknames. There is no lookup, we 
just know them. Maybe you call this a lookup step, but frankly this is 
part of *required* job knowledge (which server has what and which to use 
for which purpose).

4 - The problem came with submodules. Now, the recipe of "git clone -o 
frotz" no longer really works, as all of the submodules now have remote 
origin = frotz, and remote frotz is undefined while the master-project 
is the other way around. This is confusing to say the least. As 
submodules forced origin in, the path of least resistance  was just 
giving in and using origin. Now we have to be very aware not only of 
what server to use, but now also what origin is so that we can know how 
to reach the server we wanted. (i.e., Do I use "origin" or "frotz" to 
reach frotz?) This *IS* an extra lookup step required because of origin 
being defined.

Now, we can recusively use "git remote add" to just go ahead and add all 
remotes needed regardless of what origin says, and *partly* get back to 
where we were. Only partly, because git submodule update will still use 
origin, and *only* origin for fetching. So we still have to know what 
origin is (problem #1, fixed by my patches 1-4), and if what we need to 
fetch from is in fact not origin (even though the master project knows 
what server to access), submodule update in fact doesn't even work 
(fixed by the fifth patch that flows the remote down from the master).

So no, I do not acknowledge that we have an extra lookup step and that 
just using origin is the same or simpler or whatever. It absolutely is 
not. Quite the opposite, for us using "origin" forces an unnecessary 
extra lookup and translation step and complicates, not simplifies, use 
of submodules.

Mark

^ permalink raw reply

* [PATCH] - git submodule subcommand parsing modified.
From: imyousuf @ 2008-01-14  3:22 UTC (permalink / raw)
  To: git; +Cc: gitster, Imran M Yousuf

From: Imran M Yousuf <imyousuf@smartitengineering.com>

- manual page of git-submodule and usage mentioned in git-subcommand.sh
were not same, thus synchronized them. In doing so also had to change the
way the subcommands were parsed.

- Previous version did not allow commands such as
	git-submodule add init update
as the command parser incorrectly made subcommand names reserve.
Thus refusing them to be used as parameters to subcommands. As a result it
was impossible to add a submodule whose (symbolic) name is "init" and that
resides at path "update" was refused. For more details the following case
can be considered -

mkdir g; mkdir f; cd g/
touch g.txt; echo "sample text for g.txt" >> ./g.txt; git-init;
git-add g.txt; git-commit -a -m "First commit on g"
cd ../f/; ln -s ../g/ init
git-init; git-submodule add init update;
git-commit -a -m "With module update"
mkdir ../test; cd ../test
git-clone ../f/; cd f
git-submodule init update; git-submodule update update
cd ../..; rm -rf ./f/ ./test/ ./g/

This patch fixes this issue and allows it as well.

- Status currently is implemented to show list only but later
implementation might change and list and status could coexists. Thus
status module is introduced. The module is also used to parse its
arguments

- Subcommands will also parse their own commands; thus enabling command
specific arguments to be passed after the command. For example,
	git-submodule -q add -b master module_a
	git-submodule -q status -c
It is to be noted that -q or --quiet is specified before the subcommand
since it is for the submodule command in general rather than the
subcommand. It is mention worthy that backward compatibility exists and
thus commands like git submodule --cached status will also work as expected

- Subcommands that currently do not take any arguments (init and update)
has a case which is introduced just to ensure that no argument is
deliberately sent as the first argument and also to serve the purpose of
providing a future extension point for its arguments.

- Though ther was short and long version for quiet (-q or --quiet and
branch (-b or --branch) but there was no short version for cached. Thus
it is now introduced (-c or --cached).

- Added 3 specific messages for usage error related to branch and cached

- Simplified subcommand action invocation by simply invoking the action if
all conditions are fulfilled. Excepting for parsing command line arguments
case statements are avoided and instead more direct if statement is
introduced.
---
 git-submodule.sh |  158 +++++++++++++++++++++++++++++++++++++++---------------
 1 files changed, 114 insertions(+), 44 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index ad9fe62..22e7e5f 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -4,18 +4,23 @@
 #
 # Copyright (c) 2007 Lars Hjemli
 
-USAGE='[--quiet] [--cached] [add <repo> [-b branch]|status|init|update] [--] [<path>...]'
+# Synopsis of this commands are as follows
+# git-submodule [-q|--quiet] add [-b|--branch branch] <repository> [<path>]
+# git-submodule [-q|--quiet] [status] [-c|--cached] [--] [<path>...]
+# git-submodule [-q|--quiet] init [--] [<path>...]
+# git-submodule [-q|--quiet] update [--] [<path>...]
+USAGE='[-q|--quiet] [[[add [-b|--branch branch] <repo>]|[[[status [-c|--cached]]|init|update] [--]]]  [<path>...]]'
 OPTIONS_SPEC=
 . git-sh-setup
 require_work_tree
 
+MODULES_LIST='modules_list'
+
 add=
 branch=
-init=
-update=
-status=
 quiet=
 cached=
+command=
 
 #
 # print stuff on stdout unless -q was specified
@@ -114,6 +119,17 @@ module_clone()
 	die "Clone of '$url' into submodule path '$path' failed"
 }
 
+# Parses the branch name and exits if not present
+parse_branch_name()
+{
+	branch="$1"; 
+	if test -z "$branch"
+	then
+		echo Branch name must me specified	
+		usage
+	fi
+}
+
 #
 # Add a new submodule to the working tree, .gitmodules and the index
 #
@@ -123,6 +139,16 @@ module_clone()
 #
 module_add()
 {
+	case "$1" in
+		-b|--branch)
+			shift
+			parse_branch_name "$@" &&
+			shift
+			;;
+		-*)
+			usage
+			;;
+	esac
 	repo=$1
 	path=$2
 
@@ -176,6 +202,14 @@ module_add()
 #
 modules_init()
 {
+	# Added here to ensure that no argument is passed to be treated as
+	# parameter to the sub command. This will be used to parse any 
+	# to the subcommand
+	case "$1" in
+		-*)
+			usage
+			;;
+	esac
 	git ls-files --stage -- "$@" | grep -e '^160000 ' |
 	while read mode sha1 stage path
 	do
@@ -209,6 +243,14 @@ modules_init()
 #
 modules_update()
 {
+	# Added here to ensure that no argument is passed to be treated as
+	# parameter to the sub command. This will be used to parse any 
+	# to the subcommand
+	case "$1" in
+		-*)
+			usage
+			;;
+	esac
 	git ls-files --stage -- "$@" | grep -e '^160000 ' |
 	while read mode sha1 stage path
 	do
@@ -293,36 +335,69 @@ modules_list()
 	done
 }
 
+# Delgates to modules_list after parsing its arguments
+modules_status()
+{
+	case "$1" in
+		-c|--cached)
+			shift
+			cached=1
+			;;
+		-*)
+			usage
+			;;
+	esac
+	"$MODULES_LIST" "$@"
+}
+
+# If there is '--' as the first argument simply ignores it and thus shifts
+check_for_terminator()
+{
+	if test -n "$1" && test "$1" = "--"
+	then
+		shift
+	fi
+}
+
+
+
+# Command synopsis clearly shows that all arguments after
+# subcommand are arguments to the command itself. Thus
+# there lies no command that has configuration argument
+# after the mention of the subcommand. Thus once the
+# subcommand is found and the separator ('--') is ignored
+# rest can be safely sent the subcommand action
+# It is to be noted that pre-subcommand arguments are parsed
+# just to have backward compatibility.
 while test $# != 0
 do
 	case "$1" in
 	add)
 		add=1
+		command="module_$1"
+		shift
+		break
 		;;
-	init)
-		init=1
-		;;
-	update)
-		update=1
-		;;
-	status)
-		status=1
+	init|update|status)
+		command="modules_$1"
+		shift
+		check_for_terminator "$1"
+		break
 		;;
 	-q|--quiet)
 		quiet=1
 		;;
 	-b|--branch)
-		case "$2" in
-		'')
-			usage
-			;;
-		esac
-		branch="$2"; shift
+		shift
+		parse_branch_name "$@"
 		;;
-	--cached)
+	-c|--cached)
 		cached=1
 		;;
 	--)
+		# It is shifted so that it is not passed
+		# as an argument to the default subcommand
+		shift
 		break
 		;;
 	-*)
@@ -335,30 +410,25 @@ do
 	shift
 done
 
-case "$add,$branch" in
-1,*)
-	;;
-,)
-	;;
-,*)
+# Throws usage error if branch is not used with add command
+if test -n "$branch" &&
+   test -z "$add"
+then
+	echo Branch can not be specified without add subcommand
 	usage
-	;;
-esac
-
-case "$add,$init,$update,$status,$cached" in
-1,,,,)
-	module_add "$@"
-	;;
-,1,,,)
-	modules_init "$@"
-	;;
-,,1,,)
-	modules_update "$@"
-	;;
-,,,*,*)
-	modules_list "$@"
-	;;
-*)
+fi
+
+# If no command is specified then default command
+# is - git submodule status
+test -z "$command" && command="modules_status"
+
+# Throws usage if --cached is used by other than status, init or update
+# that is used with add command
+if test -n "$cached" &&
+   test "$command" != "modules_status"
+then
+	echo Cached can only be used with the status subcommand
 	usage
-	;;
-esac
+fi
+
+"$command" "$@"
-- 
1.5.3.7

^ permalink raw reply related

* What's in git.git (stable)
From: Junio C Hamano @ 2008-01-14  1:53 UTC (permalink / raw)
  To: git

* The 'master' branch has these since v1.5.4-rc3

Eric Wong (1):
  git-svn: handle leading/trailing whitespace from svnsync revprops

Jeff King (1):
  git-clean: fix off-by-one memory access when given no arguments

Junio C Hamano (1):
  builtin-commit.c: remove useless check added by faulty cut and paste

Linus Torvalds (1):
  Fix performance regression for partial commits

^ permalink raw reply

* Re: [PATCH] Teach remote machinery about remotes.default config variable
From: Junio C Hamano @ 2008-01-14  1:50 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Mark Levedahl, git
In-Reply-To: <alpine.LSU.1.00.0801132224540.8333@wbgn129.biozentrum.uni-wuerzburg.de>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> I never understood why people want to complicate things by being able to 
> name default _keys_ differently.  Why not letting "origin" being the 
> default being pulled from, and be done with it?

That happens to match my personal feeling.

HOWEVER.

We treat 'origin' in a special way when you do this:

	$ git clone somewhere new.git
        $ cd new.git
        $ git checkout HEAD^0
        $ git pull

And we already have "clone -o" and claim to support that option.
I think that it is very reasonable from the consistency point of
view to make sure that the following sequence treats 'frotz' the
same special way the above treats 'origin' specially:

	$ git clone -o frotz somewhere new.git
        $ cd new.git
        $ git checkout HEAD^0
        $ git pull

A purist alternative is to deprecate "git clone -o" and
eventually remove it.

Note that I was agreeing only with this specific aspect of the
argument.  I am not at all interested in getting involved in
refining or re-defining the existing submodule semantics this
late in the cycle before 1.5.4.  But I can very well see that
fixing this specific inconsistency can be separated out from the
rest of Mark's series and viewed as a set of trivially correct
fixes.

> Besides, I _really_ do not understand why we have such a
> discussion in rc phase.  There are _many_ more interesting
> discussions now that _also_ do not belong into a freeze phase.

Currently the ones I looked at and consider possible 1.5.4
material are http-push fixes from Grégoire Barbier and
parse_commit_buffer() tightening from Martin Koegler.

Recently I looked at the following patches and topics but I do
not think any of them belongs to 1.5.4.  None of them is obvious
and trivially correct fix to regressions or serious existing
bugs:

 * compress/decompress abstraction (Marco)

 * crlf (Steffen Prohaska and Dmitry Potapov)

 * whitespace error: "cr at eol is ok" (me)

 * various conflicting submodule changes(Ping Yin, Mark
   Levedahl, Imran M Yousuf)

 * unconfigured ident safety (Stephen Sinclair)

 * gitweb feed from commit to commitdiff (Florian La Rouche --
   Jakub seems to be on top of this so I am not worried about it
   too much).

 * color.ui (Matthias Kestenholz)

 * test scripts to use valgrind (Jeff King, but there was another
   one in the past -- can their efforts compared and coordinated
   better?).

 * various lstat(2) reduction changes (me).

 * pathname safety on insane filesystems (Linus, Robin
   Rosenberg, me).

(yes, some of the above list do not even have any code).

I am hoping that authors will resend the ones they really care
about after 1.5.4, as I do not want to take patches early.

Thanks.

^ permalink raw reply

* Re: format-patch usage
From: Junio C Hamano @ 2008-01-14  1:02 UTC (permalink / raw)
  To: William Morgan; +Cc: Git Mailing List
In-Reply-To: <1200270370-sup-4560@south>

William Morgan <wmorgan-git@masanjin.net> writes:

> Two questions from a git neophyte about using format-patch:

Both come from the established kernel Documentation/SubmittingPatches
convention.

^ permalink raw reply

* Re: performance problem: "git commit filename"
From: Junio C Hamano @ 2008-01-14  1:00 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Git Mailing List, Kristian H?gsberg
In-Reply-To: <alpine.LFD.1.00.0801130922030.2806@woody.linux-foundation.org>

Linus Torvalds <torvalds@linux-foundation.org> writes:

> So at a minimum, you should also make ie_match_stat() set CE_UPTODATE if 
> it matches. Or something.

I've reworked the patch, and in the kernel repository, a
single-path commit after touching that path now calls 23k
lstat(2).  It used to call 46k lstat(2) after your fix.

This depends on the "index: be careful when handling long names"
patch I sent earlier.

-- >8 --
[PATCH] Avoid running lstat(2) on the same cache entry.

Aside from the lstat(2) done for work tree files, there are
quite many lstat(2) calls in refname dwimming codepath.  This
patch is not about reducing them.

 * It adds a new ce_flag, CE_UPTODATE, that is meant to mark the
   cache entries that record a regular file blob that is up to
   date in the work tree.  If somebody later walks the index and
   wants to see if the work tree has changes, they do not have
   to be checked with lstat(2) again.

 * This reduces the maximum length cached from 4k down to 2k to
   introduce the new flag.  People with an index that records
   paths longer than that needs to first commit their changes
   with the old git, discard the cache with "rm -f .git/index"
   and re-read with updated git by running "git-read-tree HEAD"
   followed by "git update-index --refresh".

 * fill_stat_cache_info() marks the cache entry it just added
   with CE_UPTODATE.  This has the effect of marking the paths
   we write out of the index and lstat(2) immediately as "no
   need to lstat -- we know it is up-to-date", from quite a lot
   fo callers:

    - git-apply --index
    - git-update-index
    - git-checkout-index
    - git-add (uses add_file_to_index())
    - git-commit (ditto)
    - git-mv (ditto)

 * refresh_cache_ent() also marks the cache entry that are clean
   with CE_UPTODATE.

 * write_index is changed not to write CE_UPTODATE out to the
   index file, because CE_UPTODATE is meant to be transient only
   in core.  For the same reason, CE_UPDATE is not written to
   prevent an accident from happening.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 cache.h      |    4 +++-
 diff.c       |    4 ++++
 read-cache.c |   20 +++++++++++++++++++-
 3 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/cache.h b/cache.h
index 7f1457a..3e1cdf9 100644
--- a/cache.h
+++ b/cache.h
@@ -108,7 +108,8 @@ struct cache_entry {
 	char name[FLEX_ARRAY]; /* more */
 };
 
-#define CE_NAMEMASK  (0x0fff)
+#define CE_NAMEMASK  (0x07ff)
+#define CE_UPTODATE  (0x0800)
 #define CE_STAGEMASK (0x3000)
 #define CE_UPDATE    (0x4000)
 #define CE_VALID     (0x8000)
@@ -129,6 +130,7 @@ static inline size_t ce_namelen(const struct cache_entry *ce)
 	return strlen(ce->name + CE_NAMEMASK) + CE_NAMEMASK;
 }
 
+#define ce_uptodate(ce) (!!((ce)->ce_flags & htons(CE_UPTODATE)))
 #define ce_size(ce) cache_entry_size(ce_namelen(ce))
 #define ce_stage(ce) ((CE_STAGEMASK & ntohs((ce)->ce_flags)) >> CE_STAGESHIFT)
 
diff --git a/diff.c b/diff.c
index b18c140..62d0c06 100644
--- a/diff.c
+++ b/diff.c
@@ -1510,6 +1510,10 @@ static int reuse_worktree_file(const char *name, const unsigned char *sha1, int
 	if (pos < 0)
 		return 0;
 	ce = active_cache[pos];
+
+	if (ce_uptodate(ce))
+		return 1;
+
 	if ((lstat(name, &st) < 0) ||
 	    !S_ISREG(st.st_mode) || /* careful! */
 	    ce_match_stat(ce, &st, 0) ||
diff --git a/read-cache.c b/read-cache.c
index 928f49b..6e40e37 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -44,6 +44,9 @@ void fill_stat_cache_info(struct cache_entry *ce, struct stat *st)
 
 	if (assume_unchanged)
 		ce->ce_flags |= htons(CE_VALID);
+
+	if (S_ISREG(st->st_mode))
+		ce->ce_flags |= htons(CE_UPTODATE);
 }
 
 static int ce_compare_data(struct cache_entry *ce, struct stat *st)
@@ -428,6 +431,7 @@ int add_file_to_index_with_stat(struct index_state *istate,
 	    !ie_match_stat(istate, istate->cache[pos], st, ce_option)) {
 		/* Nothing changed, really */
 		free(ce);
+		istate->cache[pos]->ce_flags |= htons(CE_UPTODATE);
 		return 0;
 	}
 
@@ -804,6 +808,9 @@ static struct cache_entry *refresh_cache_ent(struct index_state *istate,
 	int changed, size;
 	int ignore_valid = options & CE_MATCH_IGNORE_VALID;
 
+	if (ce_uptodate(ce))
+		return ce;
+
 	if (lstat(ce->name, &st) < 0) {
 		if (err)
 			*err = errno;
@@ -822,8 +829,15 @@ static struct cache_entry *refresh_cache_ent(struct index_state *istate,
 		if (ignore_valid && assume_unchanged &&
 		    !(ce->ce_flags & htons(CE_VALID)))
 			; /* mark this one VALID again */
-		else
+		else {
+			/*
+			 * We do not mark the index itself "modified"
+			 * because CE_UPTODATE flag is in-core only;
+			 * we are not going to write this change out.
+			 */
+			ce->ce_flags |= htons(CE_UPTODATE);
 			return ce;
+		}
 	}
 
 	if (ie_modified(istate, ce, &st, options)) {
@@ -1180,13 +1194,17 @@ int write_index(struct index_state *istate, int newfd)
 
 	for (i = 0; i < entries; i++) {
 		struct cache_entry *ce = cache[i];
+		unsigned short ce_flags;
 		if (!ce->ce_mode)
 			continue;
 		if (istate->timestamp &&
 		    istate->timestamp <= ntohl(ce->ce_mtime.sec))
 			ce_smudge_racily_clean_entry(ce);
+		ce_flags = ce->ce_flags;
+		ce->ce_flags &= htons(~(CE_UPDATE | CE_UPTODATE));
 		if (ce_write(&c, newfd, ce, ce_size(ce)) < 0)
 			return -1;
+		ce->ce_flags = ce_flags;
 	}
 
 	/* Write extension data here */
-- 
1.5.4.rc3.4.g16335

^ permalink raw reply related

* format-patch usage
From: William Morgan @ 2008-01-14  0:40 UTC (permalink / raw)
  To: Git Mailing List

Hi all,

Two questions from a git neophyte about using format-patch:

First, what's the point of adding the Signed-off-by footer, if you're
the author of the patch? I can see the utility of the committer adding
this, as a record of who committed it, but if you wrote a patch and are
submitting it to the list, then you've presumably "signed off", haven't
you? Or is there some subtle distinction between exploratory patches and
"real" patches being made?

Second, it seems like commentary about a patch can be placed after the
"---" but before the "diff --git" lines. Is that correct? Is that
accepted practice? It seems a little backwards to have the commit
message first, then the commentary. Am I correct in assuming that
git-apply ignores anything between those two lines, including the diff
stat, and so that's the reason people put it there?

Thanks,

-- 
William <wmorgan-git@masanjin.net>

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox