Git development
 help / color / mirror / Atom feed
* Re: Git branch bug
From: Johannes Schindelin @ 2007-05-05 16:42 UTC (permalink / raw)
  To: Guido Ostkamp; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0705051823300.13988@bianca.dialin.t-online.de>

Hi,

On Sat, 5 May 2007, Guido Ostkamp wrote:

> $ mkdir test
> $ cd test
> $ git init
> Initialized empty Git repository in .git/
> $ git branch experimental
> fatal: Not a valid object name: 'master'.
> 
> So, it seems it is not possible to create a named branch on an empty
> repository.

Actually, it is not possible to branch from a non-existing branch. So, 
this is somewhat expected from my POV.

Ciao,
Dscho

^ permalink raw reply

* Git branch bug
From: Guido Ostkamp @ 2007-05-05 16:24 UTC (permalink / raw)
  To: git

Hello,

I would like to report the following bug:

$ mkdir test
$ cd test
$ git init
Initialized empty Git repository in .git/
$ git branch experimental
fatal: Not a valid object name: 'master'.

So, it seems it is not possible to create a named branch on an empty 
repository.

I'm not subscribed to the list, so please keep me directly on CC for any 
reply, thanks.

Regards

Guido

^ permalink raw reply

* [StGIT PATCH] Avoid contrib/stg-swallow deleting unrelated empty patches.
From: Yann Dirson @ 2007-05-05 13:58 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git


Let's just check ourselves if the patch is empty after pushing instead
of using clean: no more surprise with patches getting abusively
cleaned, and we should now be faster when there are many unapplied
patches.

Signed-off-by: Yann Dirson <ydirson@altern.org>
---

 contrib/stg-swallow |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/contrib/stg-swallow b/contrib/stg-swallow
index 5014f39..924c079 100755
--- a/contrib/stg-swallow
+++ b/contrib/stg-swallow
@@ -15,5 +15,9 @@ patch="$1"
 stg pick --fold "$patch"
 stg refresh
 stg push "$patch"
+
 #stg clean "$patch"
-stg pop; stg clean -u
+#stg pop; stg clean -u
+[ $(stg id "$patch//top") != $(stg id "$patch//bottom") ] ||
+    { echo >&2 "Assertion failed: patch '$patch' is not empty after swallowing, not deleting it."; exit 1; }    
+stg delete "$patch"

^ permalink raw reply related

* Re: [RFC?] Telling git about more complex relationships between commits (Was: Re: FFmpeg considering GIT)
From: Johan Herland @ 2007-05-05 16:13 UTC (permalink / raw)
  To: git
In-Reply-To: <20070504221152.GF4033@steel.home>

[-- Attachment #1: Type: text/plain, Size: 4260 bytes --]

On Saturday 05 May 2007, Alex Riesen wrote:
> Johan Herland, Fri, May 04, 2007 13:53:10 +0200:
> > As for "Reverts", the commit pointed to should already be in your
> > history, since you cannot revert something that hasn't already been
> > applied at an earlier point in your history. In other words, the
> > reverted commit will automatically be included in your "git gc
> > --prune" or "git clone" regardless of the "Reverts" fields, since
> > "Reverts" can only point to an ancestor.
>
> So it becomes useless after rebase

Only if rebase also rebases the commit pointed to by "Reverts" (the 
reverted commit). And even in that case, it should be possible for 
rebase to detect the "Reverts" relationship and rewrite it properly, 
or - if people want to - skip both the reverted and the reverting 
commit in the rebase process.

> > As for "Cherry-Pick", it's a fairly weak relationship that
> > shouldn't affect anything except to give a hint to merge, blame,
> > and similar tools.
>
> In which case, just put it in the message part of commit (in fact, it
> was there for some time. And was mostly useless, and got dropped).

Ok. If merging branches which have had cherry-picks between them is such 
a rare occurrence that there is no point in adding hints for merge (to 
do better conflict resolution), blame (to see who _really_ wrote the 
piece of code that was cherry-picked by someone else), etc. then there 
is indeed no justification for the "Cherry-Pick" header field.

> And how exactly do you think the tools _can_ use this hint?
> Especially merge, which should be absolutely certain about what
> inputs and hints gets.

When merging two branches where one branch has a commit that is later 
reverted, and the other branch has cherry-picked the first/reverted 
commit, but not the second/reverting: With these hints, git can now ask 
the user a more intelligent question like "The following commit was 
reverted in one of the branches. Do you want to keep it or revert it?". 
The current alternative seems to be to auto-choose one or the other (in 
my testing, the reverting commit was dropped in the merge). Will git 
always make the correct decision? If git is always correct, then what I 
suggest is obviously useless.

> And what use is it for blame? How do you prioritze the hint? Is it
> more important than the history (which describes each and every
> line), or less? If the hint is more important, than how (and how
> often) do you tell the user that the hint was not found (because the
> commit is long pruned) and the tool switched back to looking into
> history.

Consider the following scenario:

----
$ mkdir test
$ cd test
$ git init
Initialized empty Git repository in .git/
$ git config user.name "User A"
$ cat >f <<\EOF
foo
bar
baz
EOF
$ git add f && git commit -m "User A: foo, bar, baz"
Created initial commit bb0203aabb4936d95dca30f946cb1d849df59f24
 1 files changed, 3 insertions(+), 0 deletions(-)
 create mode 100644 f
$ git config user.name "User B"
$ cat >f <<\EOF
foo
barf
baz
EOF
$ git commit -a -m "User B: bar -> barf"
Created commit 5ced0ccaba0bf4a982dc2cdd792a1a0e7b1883eb
 1 files changed, 1 insertions(+), 1 deletions(-)
$ git config user.name "User C"
$ git revert HEAD
Created commit 38da1083ae4677000f8bb70729f474f358c71a3e
 1 files changed, 1 insertions(+), 1 deletions(-)
----

At this point, what output do we _really_ want from "git blame f"?

Currently we get:
----
^bb0203a (User A 2007-05-05 12:25:44 +0200 1) foo
38da1083 (User C 2007-05-05 12:28:00 +0200 2) bar
^bb0203a (User A 2007-05-05 12:25:44 +0200 3) baz
----

Can you categorically say that there is no use for the following output? 
(even if you need to pass an option to "git blame" to get it):
----
^bb0203a (User A 2007-05-05 12:25:44 +0200 1) foo
^bb0203a (User A 2007-05-05 12:25:44 +0200 1) bar
^bb0203a (User A 2007-05-05 12:25:44 +0200 3) baz
----

> It's useless.

Maybe. At least some of the fields I proposed are probably useless. But 
I don't think we should throw away the core idea unless we can show 
that _all_ fields are useless.


Have fun!

...Johan
-- 
Johan Herland, <johan@herland.net>
www.herland.net

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* [PATCH 2/2] Copy patchlogs when cloning a stack or picking a patch.
From: Yann Dirson @ 2007-05-05 14:00 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git
In-Reply-To: <20070505135830.5194.90861.stgit@gandelf.nowhere.earth>


This will allow to keep patch history accross those operations, and
see when they diverged when looking at the respective patchlogs.  With
more work, that will allow to locate the common ancestor when sync'ing
a patch across branches.

The work on "pick" can still be improved: currently the patchlog is
preserved on "pick patch[@branch][//top]", but we could make it work
for //top.old as well (or any rev in the patchlog when we'll be able
to name those).

Patchlog preservation in pick can only work when referencing a patch
by name.  When picking an arbitrary commit we could also improve
things by recording a "pick" operation instead of a "new".

Signed-off-by: Yann Dirson <ydirson@altern.org>
---

 stgit/commands/pick.py |   31 ++++++++++++++++++++++++++-----
 stgit/stack.py         |   22 +++++++++++++++-------
 2 files changed, 41 insertions(+), 12 deletions(-)

diff --git a/stgit/commands/pick.py b/stgit/commands/pick.py
index 4eb79a3..4ef9860 100644
--- a/stgit/commands/pick.py
+++ b/stgit/commands/pick.py
@@ -21,6 +21,7 @@ from optparse import OptionParser, make_option
 from stgit.commands.common import *
 from stgit.utils import *
 from stgit import stack, git
+from stgit.stack import Series
 
 
 help = 'import a patch from a different branch or a commit object'
@@ -118,11 +119,31 @@ def func(parser, options, args):
         print 'Importing commit %s...' % commit_id,
         sys.stdout.flush()
 
-        crt_series.new_patch(patchname, message = message, can_edit = False,
-                             unapplied = True, bottom = bottom, top = top,
-                             author_name = author_name,
-                             author_email = author_email,
-                             author_date = author_date)
+        newpatch = crt_series.new_patch(patchname, message = message, can_edit = False,
+                                        unapplied = True, bottom = bottom, top = top,
+                                        author_name = author_name,
+                                        author_email = author_email,
+                                        author_date = author_date)
+        # find a patchlog to fork from
+        (refpatchname, refbranchname, refpatchid) = parse_rev(commit_str)
+        if refpatchname and not refpatchid and \
+               (not refpatchid or refpatchid == 'top'):
+            # FIXME: should also support picking //top.old
+            if refbranchname:
+                # assume the refseries is OK, since we already resolved
+                # commit_str to a git_id
+                refseries = Series(refbranchname)
+            else:
+                refseries = crt_series
+            patch = refseries.get_patch(refpatchname)
+            if patch.get_log():
+                print"log was %s" % newpatch.get_log()
+                print "setting log to %s\n" %  patch.get_log()
+                newpatch.set_log(patch.get_log())
+                print"log is now %s" % newpatch.get_log()
+            else:
+                print "no log for %s\n" % patchname
+ 
         if not options.unapplied:
             modified = crt_series.push_patch(patchname)
         else:
diff --git a/stgit/stack.py b/stgit/stack.py
index b0a01dd..044348a 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -23,6 +23,7 @@ import sys, os, re
 from stgit.utils import *
 from stgit import git, basedir, templates
 from stgit.config import config
+from shutil import copyfile
 
 
 # stack exception class
@@ -628,13 +629,18 @@ class Series(StgitObject):
             patches = applied = unapplied = []
         for p in patches:
             patch = self.get_patch(p)
-            new_series.new_patch(p, message = patch.get_description(),
-                                 can_edit = False, unapplied = True,
-                                 bottom = patch.get_bottom(),
-                                 top = patch.get_top(),
-                                 author_name = patch.get_authname(),
-                                 author_email = patch.get_authemail(),
-                                 author_date = patch.get_authdate())
+            newpatch = new_series.new_patch(p, message = patch.get_description(),
+                                            can_edit = False, unapplied = True,
+                                            bottom = patch.get_bottom(),
+                                            top = patch.get_top(),
+                                            author_name = patch.get_authname(),
+                                            author_email = patch.get_authemail(),
+                                            author_date = patch.get_authdate())
+            if patch.get_log():
+                print "setting log to %s" %  patch.get_log()
+                newpatch.set_log(patch.get_log())
+            else:
+                print "no log for %s" % patchname
 
         # fast forward the cloned series to self's top
         new_series.forward_patches(applied)
@@ -865,6 +871,8 @@ class Series(StgitObject):
             if refresh:
                 self.refresh_patch(cache_update = False, log = 'new')
 
+        return patch
+
     def delete_patch(self, name):
         """Deletes a patch
         """

^ permalink raw reply related

* [PATCH 1/2] Cleanup variable names in pick.
From: Yann Dirson @ 2007-05-05 14:00 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git
In-Reply-To: <20070505135830.5194.90861.stgit@gandelf.nowhere.earth>


I'm going to need patch objects there, so avoid using variable "patch"
to store only a patchname.

Signed-off-by: Yann Dirson <ydirson@altern.org>
---

 stgit/commands/pick.py |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/stgit/commands/pick.py b/stgit/commands/pick.py
index edd614d..4eb79a3 100644
--- a/stgit/commands/pick.py
+++ b/stgit/commands/pick.py
@@ -71,11 +71,11 @@ def func(parser, options, args):
     else:
         patch_branch = commit_str.split('@')
         if options.name:
-            patch = options.name
+            patchname = options.name
         elif len(patch_branch) == 2:
-            patch = patch_branch[0]
+            patchname = patch_branch[0]
         else:
-            patch = make_patch_name(commit.get_log(), crt_series.patch_exists)
+            patchname = make_patch_name(commit.get_log(), crt_series.patch_exists)
 
     if options.parent:
         parent = git_id(options.parent)
@@ -118,17 +118,17 @@ def func(parser, options, args):
         print 'Importing commit %s...' % commit_id,
         sys.stdout.flush()
 
-        crt_series.new_patch(patch, message = message, can_edit = False,
+        crt_series.new_patch(patchname, message = message, can_edit = False,
                              unapplied = True, bottom = bottom, top = top,
                              author_name = author_name,
                              author_email = author_email,
                              author_date = author_date)
         if not options.unapplied:
-            modified = crt_series.push_patch(patch)
+            modified = crt_series.push_patch(patchname)
         else:
             modified = False
 
-        if crt_series.empty_patch(patch):
+        if crt_series.empty_patch(patchname):
             print 'done (empty patch)'
         elif modified:
             print 'done (modified)'

^ permalink raw reply related

* [StGIT PATCH 0/2] Improving patchlogs
From: Yann Dirson @ 2007-05-05 14:00 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git

This is a first pass on recording missing history in the patchlogs, as
described in a former email.  At least "sync" still has unrecorded
(merge) information.

-- 
Yann Dirson    <ydirson@altern.org> |
Debian-related: <dirson@debian.org> |   Support Debian GNU/Linux:
                                    |  Freedom, Power, Stability, Gratis
     http://ydirson.free.fr/        | Check <http://www.debian.org/>

^ permalink raw reply

* 'upstream' branches.
From: David Woodhouse @ 2007-05-05 12:29 UTC (permalink / raw)
  To: git

When creating a repository which may pull from one or more 'upstream'
repositories, it's useful to keep 'branches' which keep track of the
last pull from those upstream repositories -- either directly or
indirectly.

At http://www.linux-mtd.infradead.org/doc/git.html I've described the
setup I'm currently using to achieve this, which looks something like
the following:

[remote "origin"]
        url = ssh://git.infradead.org/~/public_git/foo-2.6.git
        fetch = +refs/heads/*:refs/remotes/origin/*
	fetch = +refs/heads/mtd:refs/heads/mtd
	fetch = +refs/heads/linus:refs/heads/linus
        push = refs/heads/master:refs/heads/master
        push = refs/heads/mtd:refs/heads/mtd
        push = refs/heads/linus:refs/heads/linus
[branch "master"]
        remote = origin
        merge = refs/heads/master
[remote "mtd"]
        url = git://git.infradead.org/mtd-2.6.git
        fetch = refs/heads/master:refs/heads/mtd
        fetch = +refs/heads/linus:refs/heads/linus
[remote "linus"]
        url = git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.$
        fetch = refs/heads/master:refs/heads/linus

Is there a better way to do this? Preferably which doesn't involve
directing the user to edit .git/config directly?

Basically, I want the local 'linus' branch to be updated whenever the
user pulls from _any_ other repository with a 'linus' branch, so that
the 'linus' branch always represents the latest commit pulled from
upstream. Likewise, the 'mtd' branch should be updated when pulling from
that tree (or any other dependent tree which will have an 'mtd' branch).

These branches should be pushed back to the origin each time.

What I have at the moment isn't ideal because I think pulling from the
'mtd' tree will fail if the 'linus' branch there is older than the local
clone's 'linus' branch. But it mostly works.

Is there a better way?

-- 
dwmw2

^ permalink raw reply

* Re: [PATCH v3] Support ent:relative_path
From: Johannes Schindelin @ 2007-05-05 14:39 UTC (permalink / raw)
  To: Dana How; +Cc: Junio C Hamano, Git Mailing List, Shawn O. Pearce
In-Reply-To: <56b7f5510705042346s759a2ef9tfa3a223fe7af7c16@mail.gmail.com>

Hi,

On Fri, 4 May 2007, Dana How wrote:

> (a) In a bare repository, I believe setup.c:setup_git_directory_gently()
> determines the prefix to be NULL.  This means my patch will see
> ALL paths as absolute,  except :../path which will result in an error.

My point was that it feels inconsistent to take the current path into 
account in one case, but not in the other.

Ciao,
Dscho

^ permalink raw reply

* Re: FFmpeg considering GIT
From: Karl Hasselström @ 2007-05-05 13:35 UTC (permalink / raw)
  To: Carl Worth; +Cc: Michael Niedermayer, git
In-Reply-To: <87y7k4lahq.wl%cworth@cworth.org>

On 2007-05-04 11:17:05 -0700, Carl Worth wrote:

> or by content of the patches themselves:
>
>       git log -p -S'snippet of interest'

Somewhat unrelated: how can I make gitk display these (and only these)
commits? git-log is not bad, but in 95% of cases I find gitk easier to
use.

I know that I can ask it to highlight commits that insert or remove
"snippet of interest", but frequently the highlighted commits are ten
out of ten thousand, and not that easy to find even when boldfaced.
What I want is to make it display only those commits.

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

^ permalink raw reply

* Re: [PATCH] Makefile: use --unsafe option under Cygwin with asciidoc
From: Junio C Hamano @ 2007-05-05  9:06 UTC (permalink / raw)
  To: Eric Blake; +Cc: Jari Aalto, git
In-Reply-To: <7v7irnlmgo.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> writes:

> I haven't dug into the list archive article I quoted yet (the
> pointer is also found in TODO file in 'todo' branch) and haven't
> tried the backward compatibility pragma, but you can clearly see
> that the above differences are simply unacceptable.  They are
> not insignificant cosmetic differences -- the most important
> techinical details are being mangled, rendering the
> documentation useless.  We _do_ need the backward compatiblity
> enabled in asciidoc.conf or somewhere.

For AsciiDoc 8, this is minimally necessary.  I vaguely recalled
that this needs to be conditional on the actual version of
asciidoc as AsciiDoc 7 did not like it, but I haven't tried it
recently.

-- >8 snip >8 --
diff --git a/Documentation/Makefile b/Documentation/Makefile
index ad87736..28c33f0 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -32,7 +32,7 @@ man1dir=$(mandir)/man1
 man7dir=$(mandir)/man7
 # DESTDIR=
 
-ASCIIDOC=asciidoc
+ASCIIDOC = asciidoc -a asciidoc7compatible
 ASCIIDOC_EXTRA =
 INSTALL?=install
 DOC_REF = origin/man

-- 8< snap 8< --

This seems to make the build-product from asciidoc 8.2.1 mostly
equivalent to asciidoc 7, but there still is one troubling
difference I spotted.

    --- 7	2007-05-05 01:56:18.000000000 -0700
    +++ 8	2007-05-05 01:56:29.000000000 -0700
    @@ -2,6 +2,6 @@
     A suffix \fI~<n>\fR to a revision parameter means the
     commit object that is the <n>th generation grand\-parent
     of the named commit object, following only the first parent.
    -I.e. rev~3 is equivalent to rev^^^ which is equivalent to
    -rev^1^1^1. See below for a illustration of the usage of this form.
    +I.e. rev~3 is equivalent to rev^ which is equivalent to
    +rev11^1. See below for a illustration of the usage of this form.
     .TP 3n

Again, this is an unacceptable breakage that makes this part of the
documentation useless.  However, this part of the documentation
uses our own "[attributes] caret=^" to work around the bug/misfeature
in AsciiDoc 7, so maybe this could be (and needs to be) worked
around by conditionally adjusting that macro to the version of
AsciiDoc.

I do not think we can say the current documentation set can be
formatted sanely with AsciiDoc 8.  Although I can say with
reasonable comfort level that the output with AsciiDoc 7 has
been proofread by enough people already, I cannot say the same
for AsciiDoc 8.

Somebody needs to do some homework to devise a compatibility
study between two versions, as we would eventually need to
support both versions (iow, make our documentation set
formattable with either version) at the same time.

While "use --unsafe" (I did not trigger any unsafe_error(), so
it may not be an issue) and giving "-a asciidoc7compatible" at
the command line might be one part of that compatibility study,
I do not think that is the end of it.  Proofreading the output
and making sure the technical details are not lost in formatting
errors is the most important part.

^ permalink raw reply related

* Re: Initial support for cloning submodules
From: Junio C Hamano @ 2007-05-05  8:46 UTC (permalink / raw)
  To: skimo; +Cc: git
In-Reply-To: <20070505081404.GR955MdfPADPa@greensroom.kotnet.org>

Sven Verdoolaege <skimo@kotnet.org> writes:

> On Fri, May 04, 2007 at 03:52:15PM -0700, Junio C Hamano wrote:
>> I do not like the Porcelain part very much, though.  I do not
>> think we would want to add anything new to git-clone.  We should
>> lose as much code from git-clone that is common with git-fetch
>> as we can first, and add new features to git-fetch, with
>> possibly passthru options added to git-clone as needed (e.g. a
>> new --submodule option).
>
> So what would you want to keep in git-clone ?

 - Figuring out the name of the new directory we create (think
   "git clone git://repo.or.cz/git.git" -- it does "mkdir git &&
   cd git" for you before doing the other things).

 - Run "git init" there, obviously.

 - Probably run "git ls-remote" to figure out which branch HEAD
   should point at; you would definitely want to add a mechanism
   to pass the ls-remote result to the "git fetch" you are going
   to call next, as it is the first thing "git fetch" usually
   does -- we would want to reuse it.

 - Recently suggested good addition is to have "--track $branch"
   option to "git clone" to point remotes/origin/HEAD to
   something other than what the remote's HEAD actually points
   at (the discussion was primarily between Carl Worth and
   Linus; see archive).

 - Run "git remote add origin" with the given URL to set up the
   standard "separate remotes" tracking structure.

 - Run "git fetch" for initial fetch ("git remote add -f" could
   do that as well).  You would probably need to pass --tags to
   this "git fetch" to mimick what "git clone" does today.

 - Run initial checkout if asked.

>>  (3) "git-fetch --submodules", after finishing what it would do
>>      without "--submodules" option, would inspect the fetched
>>      tree (or the index derived from it), find the tree entries
>>      with mode 160000 (i.e. submodule graft points), and _then_
>>      uses the pathnames of these tree entries to consult the
>>      config mechanism to see which URL(s) can be used to
>>      retrieve them, probably only for new submodules.
>
> Would git-fetch then call git-clone for these new submodules?

Most likely yes but that is just my gut feeling -- I readily
admit I haven't thought it through.

^ permalink raw reply

* Re: Initial support for cloning submodules
From: Sven Verdoolaege @ 2007-05-05  8:14 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vfy6cqk0w.fsf@assigned-by-dhcp.cox.net>

On Fri, May 04, 2007 at 03:52:15PM -0700, Junio C Hamano wrote:
> I do not like the Porcelain part very much, though.  I do not
> think we would want to add anything new to git-clone.  We should
> lose as much code from git-clone that is common with git-fetch
> as we can first, and add new features to git-fetch, with
> possibly passthru options added to git-clone as needed (e.g. a
> new --submodule option).

So what would you want to keep in git-clone ?

> If you --submodule cloned a remote repository when it had two
> submodules, and then later the remote adds another submodule,
> you would need to have a way to fetch that can discover the
> presense of the new submodule and add it for you, and at that
> point, having the code that knows much about submodules in clone
> would not help you much.

True.

>  (3) "git-fetch --submodules", after finishing what it would do
>      without "--submodules" option, would inspect the fetched
>      tree (or the index derived from it), find the tree entries
>      with mode 160000 (i.e. submodule graft points), and _then_
>      uses the pathnames of these tree entries to consult the
>      config mechanism to see which URL(s) can be used to
>      retrieve them, probably only for new submodules.

Would git-fetch then call git-clone for these new submodules?

> Having a generic program and protocol to
> dump the whole configuration file is certainly simpler, easier
> to debug, and easier to repurpose, it makes me somewhat worried
> about security implications (if it is open to http then worrying
> about it is not very useful, though).

We could easily have dump-config only dump a predefined "known safe"
set of config options, although that would mean you have to upgrade
the server side each time you add a new dumpable config option.
Or we could do the preselection only when called from git-daemon.

skimo

^ permalink raw reply

* Re: [PATCH] Makefile: use --unsafe option under Cygwin with asciidoc
From: Junio C Hamano @ 2007-05-05  8:10 UTC (permalink / raw)
  To: Eric Blake; +Cc: Jari Aalto, git
In-Reply-To: <7vslaf4121.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> writes:

> Eric Blake <ebb9@byu.net> writes:
>
>> According to Jari Aalto on 5/2/2007 5:49 AM:
>>> New: variable ASCIIDOC_FLAGS was introduced. It was also added to two
>>> new targets. The old ASCIIDOC_EXTRA is set to --unsafe under Cygwin to
>>> ignore asciidoc error about unsafe include.
>>
>> This isn't just for cygwin, since it benefits any platform where the
>> installed asciidoc is 8.1 or better (asciidoc is currently at 8.2.1 if you
>> build the from tarballs).

I got curious and tried this myself.

> What unsafe things do we include?  Maybe _that_ is what should
> be fixed?

I couldn't trigger "unsafe" error.

> Also the last time we checked (I cannot take credits for this
> work -- see "git show origin/todo:TODO" and look for
> "AsciiDoc"), AsciiDoc 8 had backward compatibility problems, and
> you needed to add some pragma in asciidoc.conf to make it
> compatible with sources written for AsciiDoc 7 (i.e. our
> documentation).

Here are randomly selected differences between the documentation
pages formatted with AsciiDoc 7 and 8.

        --- 7	2007-05-05 01:06:39.000000000 -0700
        +++ 8	2007-05-05 01:06:36.000000000 -0700
        @@ -1,33 +1,33 @@
         git\-bundle will only package references that are shown by git\-show\-ref:
        -this includes heads, tags, and remote heads. References such as master~1 
        +this includes heads, tags, and remote heads. References such as master1
         cannot be packaged, but are perfectly suitable for defining the basis.
         More than one reference may be packaged, and more than one basis can be
         specified. The objects packaged are those not contained in the union of
        -the given bases. Each basis can be specified explicitly (e.g., ^master~10),
        +the given bases. Each basis can be specified explicitly (e.g., ^master10),
         or implicitly (e.g., master~10..master, master \-\-since=10.days.ago).

Notice broken tilde.

        -After this three\-way merge, the local modifications are _not_
        +After this three\-way merge, the local modifications are \fInot\fR
         registered in your index file, so git diff would show you what changes
         you made since the tip of the new branch.

        -<p>which will simply _create_ the branch, but will not do anything further.
        +<p>which will simply <em>create</em> the branch, but will not do anything further.

AsciiDoc 8 knows _emphasis_ which is nice.

        -Allow ~user notation to be used in requests. When specified with no
        -parameter, requests to git://host/~alice/foo is taken as a request to
        +Allow user notation to be used in requests. When specified with no
        +parameter, requests to git://host/alice/foo is taken as a request to
         access \fIfoo\fR repository in the home directory of user alice. If
         \-\-user\-path=path is specified, the same request is taken as a
         request to access path/foo repository in the home directory of user
         alice.

Again, broken tilde.

         In the above example output, the function signature was changed from
        -both files (hence two \- removals from both file1 and file2, plus ++
        -to mean one line that was added does not appear in either file1 nor
        +both files (hence two \- removals from both file1 and file2, plus + to
        +mean one line that was added does not appear in either file1 nor
         file2). Also two other lines are the same from file1 but do not appear
        -in file2 (hence prefixed with +).
        +in file2 (hence prefixed with ).

Missing plus (in manpage output).

         <p>In the above example output, the function signature was changed
         from both files (hence two <tt>-</tt> removals from both file1 and
        -file2, plus <tt>++</tt> to mean one line that was added does not appear
        +file2, plus <tt><tt>+</tt> to mean one line that was added does not appear
         in either file1 nor file2).  Also two other lines are the same
        -from file1 but do not appear in file2 (hence prefixed with <tt> +</tt>).</p>
        +from file1 but do not appear in file2 (hence prefixed with <tt> </tt></tt>).</p>
         <p>When shown by <tt>git diff-tree -c</tt>, it compares the parents of a

Same (in HTML output).

I haven't dug into the list archive article I quoted yet (the
pointer is also found in TODO file in 'todo' branch) and haven't
tried the backward compatibility pragma, but you can clearly see
that the above differences are simply unacceptable.  They are
not insignificant cosmetic differences -- the most important
techinical details are being mangled, rendering the
documentation useless.  We _do_ need the backward compatiblity
enabled in asciidoc.conf or somewhere.

^ permalink raw reply

* Re: [PATCH] git-parse-remote: fix ambiguous shell bug in expand_refs_wildcard
From: Herbert Xu @ 2007-05-05  8:03 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20061219003557.GA17799@gondor.apana.org.au>

On Tue, Dec 19, 2006 at 11:35:57AM +1100, herbert wrote:
> On Mon, Dec 18, 2006 at 05:45:05PM -0500, Jeff King wrote:
> > 
> > but doing it inside an interpolated string doesn't:
> > 
> > $ foo=bar}
> > $ echo "${foo%'}'}"
> > bar}'}
> 
> Yes it's a bug in dash.  Both quote marks (" and ') are represented
> by the same char internally before processing which is where the
> mix-up occurs.
> 
> I'll work on a fix.

Sorry for the delay.  I've finally looked at fixing this.  It turns out
that dash's behaviour is actually correct and POSIX compliant.

It's correct because dash treats all single quotes within double
quotes (except those within command substitutions) as literals.
This interpretation is also supported by POSIX.

In fact the rationale (C.2.2.3) in the POSIX document explicitly
disallows the aformentioned usage as it violates the rule that an
even number of single quotes if any can occur in an ${...} expression
enclosed by double quotes.

So the correct and portable expression in this case would be either

echo "${foo%\}}"

or

brace=}
echo "${foo%$brace}"

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH v3] Support ent:relative_path
From: Alex Riesen @ 2007-05-05  8:03 UTC (permalink / raw)
  To: Dana How
  Cc: Junio C Hamano, Johannes Schindelin, Git Mailing List,
	Shawn O. Pearce
In-Reply-To: <56b7f5510705042346s759a2ef9tfa3a223fe7af7c16@mail.gmail.com>

Dana How, Sat, May 05, 2007 08:46:46 +0200:
> Thanks for your reply.  I can wait for 1.5.2.

Actually, you (and everyone who likes your patch and the proposed
semantics) can wait indefinitely. Just keep the patch in a side branch
and rebase it or merge with Junio's master occasionally.
I believe many of us have some features they keep in their repos which
they never send upstream (for their own reasons. I customized some
warnings and added printing of ip addresses to git-fetch. Not
interesting for everyone, but is useful next time kernel.org has DNS
problems). Just become the maintainer of the feature.

^ permalink raw reply

* Re: [PATCH 4/5] Add --remote option to send-pack
From: Junio C Hamano @ 2007-05-05  7:54 UTC (permalink / raw)
  To: Daniel Barkalow; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0705050235210.28708@iabervon.org>

Daniel Barkalow <barkalow@iabervon.org> writes:

> Wouldn't it be better to to this as:
>
> [remote "origin"]
> 	url = git.kernel.org:/pub/scm/git/git.git/
> [branch "master"]
> 	remote = origin
> 	merge = refs/heads/master

As I am likely to pull from other people, and I happen to expect
the old fashioned "git pull gfi" without explicit refspec on the
command line to get the first Pull: line from remotes file to
tell which one to merge from, I am in favor of _not_ having that
[branch "master"] stuff in my repository.

remote.*.fetch is about what remote branches are fetched;
tracking may or may not happen as a side effect.  What I do with
the resulting .git/FETCH_HEAD is my business and branch.*.merge
should _not_ be the only way to access it.

> Merging without tracking is fine, in general; the "without tracking" was 
> intentionally parenthetical. 

Actually, fetching with or without tracking are both valid
options, and it does not make much sense to say which one is
norm and which one is exception.

> I just don't think always merging a 
> particular remote head into the current branch, regardless of what branch 
> is current, is a good idea.

Exactly.  That's why I do not think [branch "master"] for
toplevel maintainer usage is not very useful.

There are a few improvements we probably would want.

One is the interaction between the config "fetch =..." refspecs
and the command line ones.  Currently the rule is:

 - If you do not say refspecs on the command line, fetch config
   (or Pull: lines from remotes/* file) are used.

 - If you do have refspecs on the command line, fetch config are
   not used (they are ignored).

Which is fine when we talk about "git fetch", but if you _are_
using remote tracking, and if you are running "git fetch" as an
implementation detail of running "git pull" (IOW, you said "git
pull origin next"), it is less than optimum.  Instead of "only
fetch 'next' without tracking and then merge it", we would
certainly want "fetch to track everything as usual, and then
merge 'next' instead of what is usually merged".

Another thing is that which branch is merged into the current
branch should not be a function of the current branch, as the
current configuration mechanism suggests.  The current way maps
the current branch to "which repository's which branch".  In
addition to that, I think it should be a function of ("current
branch", "remote repository") pair.  IOW, allow you to say "If I
pull from this remote without saying which one to merge, merge
that branch.  If I pull from this other remote, merge that other
branch".  Something like:

        [branch "master"]
                ; Use this when "git pull" did not say which remote
                remote = origin

                ; Use this when "git pull origin" did not say which
                ; branch(es) to merge
                merge = refs/heads/master
                ; the above should be a synonym for
		; merge = refs/heads/master for origin

                ; "git pull fast-import" while on my "master"
                ; would merge 'for-junio' branch there.
                merge = refs/heads/for-junio for fast-import

                ; similarly, but use subtree strategy.
                merge = refs/heads/for-junio with subtree for git-gui

        [remote "origin"]
                url = git://git.kernel.org/pub/scm/git/git.git/
                fetch = refs/heads/*:remotes/origin/*
        [remote "fast-import"]
                url = git://repo.or.cz/git/fastimport.git/
                fetch = refs/heads/*:remotes/gfi/*
        [remote "git-gui"]
                url = git://repo.or.cz/git-gui.git/
                fetch = refs/heads/*:refs/remotes/git-gui/*

^ permalink raw reply

* Re: [PATCH 4/5] Add --remote option to send-pack
From: Daniel Barkalow @ 2007-05-05  6:52 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vr6pvn5k0.fsf@assigned-by-dhcp.cox.net>

On Fri, 4 May 2007, Junio C Hamano wrote:

> Daniel Barkalow <barkalow@iabervon.org> writes:
> 
> > Is this actually supported in config files? At least for pulls, the "tag 
> > <tag>" notation is only available on the command line, afaict.
> 
> That is the only difference as far as I remember.  But you are
> right in that the set of refspecs allowed on the command line
> does not have to coincide with the ones in remotes file or the
> config (the latter two should match, though), and the latter may
> not be documented (the intention was to match all of them so the
> quoted documentation was enough, but having "tag <tag>" in
> config does not make sense).  Documentation/config.txt could use
> a bit of updates.  Hint, hint.

Once I've figured out what the intended behavior is, and have a canonical 
implementation so that it clearly does that, I'll document it. Until then, 
it's a bit tricky. :)

> > I think that having "<ref>:" or "<ref>" in a config file fetch 
> > line would be really bad; these refs would always be merged into any 
> > current head (without any remote tracking) when the remote is fetched 
> > from?
> 
> It's not bad at all.  Why people are so eager to _force_
> tracking on other people, I do not understand.
> 
> I had a moral equivalent of this in .git/remotes/origin (back
> then .git/config did not exist) when I was a contributor:
> 
> 	[remote "origin"]
>         	url = git.kernel.org:/pub/scm/git/git.git/
>                 fetch = refs/heads/master
> 
> If the user does not want to track, he does not have to.

Wouldn't it be better to to this as:

[remote "origin"]
	url = git.kernel.org:/pub/scm/git/git.git/
[branch "master"]
	remote = origin
	merge = refs/heads/master

If you're not tracking, the only thing you do with remote refs is merge 
them (after getting an anonymous temporary copy, of course), which should 
be the "merge" line, not the "fetch" line. Furthermore, you certainly 
don't want to merge refs/heads/master if you fetch (or pull) when you have 
todo (or a branch derived from it) checked out.

Merging without tracking is fine, in general; the "without tracking" was 
intentionally parenthetical. I just don't think always merging a 
particular remote head into the current branch, regardless of what branch 
is current, is a good idea.

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

* Re: [PATCH v3] Support ent:relative_path
From: Dana How @ 2007-05-05  6:46 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Johannes Schindelin, Git Mailing List, Shawn O. Pearce, danahow
In-Reply-To: <7vwszolz26.fsf@assigned-by-dhcp.cox.net>

Thanks for your reply.  I can wait for 1.5.2.
So I will _quickly_ address some other points JS and SP mentioned.

(a) In a bare repository, I believe setup.c:setup_git_directory_gently()
determines the prefix to be NULL.  This means my patch will see
ALL paths as absolute,  except :../path which will result in an error.

(b) For :path and :stage:path , relative is useful for me.

(c) For <commit-ish>:path ,  again relative is useful for me.
All my examples were of this form.

(d) Where <tree-ish> in <tree-ish>:path is just a tree and no
commit is involved, relative paths make no sense since you don't
know where the tree is. (Imagine it moved between commits.)
So don't call my prepend_prefix() or call it with prefix faked to NULL.

Unfortunately I was never clear about (a) and my patch fails
to discriminate between (c) and (d).  I'll try to fix the latter later.

Thanks

Dana

On 5/4/07, Junio C Hamano <junkio@cox.net> wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > Think of the "todo" branch, for example. And this is not even far fetched.
> > In many repositories I have to have separate related, but non-branch
> > HEADs. For example, I track some projects with custom scripts. These
> > scripts do _not_ belong into that branch. However, they are related, so I
> > store them in the same repository.
>
> I said I won't talk about it until 1.5.2 final, but you forced
> me into it.
>
> > Plus, my example of the bare-repository still has not been answered by
> > _anyone_ in favour of changing the current behaviour.
>
> I am not sure if there is anything to answer on this one.
>
> When you are talking to a bare-repository, your place relative
> to the root of the working tree is clearly undefined, and it is
> natural that "<tree-ish>:./<path>", "<tree-ish>:/<path>" and
> "<tree-ish>:<path>" cannot mean anything but relative to the
> root of the tree-ish.  "<tree-ish>:../<path>" is obviously
> nonsense.  So it does not matter if unadorned <path> is relative
> to cwd or root in this case.
>
> Your other example, however, gives a much better illustration.
> An unrelated 'todo' branch where your cwd does not have any
> relevance to the contents of that tree-ish, or worse yet, a
> commit from git-gui project in git.git repository, where taking
> your cwd into account has an actively wrong effect, demonstrates
> why we would need a way to say "By this path, I mean from the
> top, I do not want you to take it as relative to where I am".
>
> One way to ensure that is to keep the current "it is a path from
> the top" behaviour, and extended it with "... unless it begins
> with ./".  Doing this forever however penalizes the case where
> you want to use relative paths by requiring ./ at the beginning.
>
> Another is to do the usual POSIXy path interpretation and
> "unless it begins with /, it is taken as relative to where you
> are".  This penalizes the 'todo' and git-gui commit use case
> because the user explicitly needs to say "where I am does not
> matter" by prefixing the path with '/', and also necessitates a
> change to the syntax for looking backwards for a commit with
> that message, because the existing syntax to look for a string
> clashes with it [*1*].
>
> Both have merits and demerits.  If we did not have any existing
> code and users, the latter is clearly what we would have done,
> as it is more consistent.  The path handling feels more natural
> (in line with the way we expect paths to be handled on POSIX
> systems), the "look backwards" search feels more natural ( you
> use '/' for forward search, '?' for backwards).
>
> I also suspect the latter is more often convenient.  When
> working on a flat project, it does not matter if the default is
> relative to cwd or to the root.  But if your project is deep,
> and if you somehow do "git show" more often than "git diff" (I
> don't, but different people may do so for different reasons), it
> would start to hurt if you always have to say "./".
>
> It is however clearly a bigger change to existing users.
> Correcting earlier mistakes is painful, so it certainly is
> tempting to take the approach that the path is always absolute
> and require "./" for relative.  I agree it is an easier change,
> but I am not convinced yet that it is the right design in the
> longer term.
>
> [Footnote]
>
> *1* This is only true for looking for a path in the index case,
> as <commit>:/<string> does not seem to work.  I think this is a
> bug in the current code -- shouldn't it limit the search to
> commits that are reachable from that named one?

-- 
Dana L. How  danahow@gmail.com  +1 650 804 5991 cell

^ permalink raw reply

* Re: [PATCH 4/5] Add --remote option to send-pack
From: Junio C Hamano @ 2007-05-05  6:33 UTC (permalink / raw)
  To: Daniel Barkalow; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0705050115270.28708@iabervon.org>

Daniel Barkalow <barkalow@iabervon.org> writes:

> Is this actually supported in config files? At least for pulls, the "tag 
> <tag>" notation is only available on the command line, afaict.

That is the only difference as far as I remember.  But you are
right in that the set of refspecs allowed on the command line
does not have to coincide with the ones in remotes file or the
config (the latter two should match, though), and the latter may
not be documented (the intention was to match all of them so the
quoted documentation was enough, but having "tag <tag>" in
config does not make sense).  Documentation/config.txt could use
a bit of updates.  Hint, hint.

> I think that having "<ref>:" or "<ref>" in a config file fetch 
> line would be really bad; these refs would always be merged into any 
> current head (without any remote tracking) when the remote is fetched 
> from?

It's not bad at all.  Why people are so eager to _force_
tracking on other people, I do not understand.

I had a moral equivalent of this in .git/remotes/origin (back
then .git/config did not exist) when I was a contributor:

	[remote "origin"]
        	url = git.kernel.org:/pub/scm/git/git.git/
                fetch = refs/heads/master

If the user does not want to track, he does not have to.

^ permalink raw reply

* Re: [PATCH 4/5] Add --remote option to send-pack
From: Daniel Barkalow @ 2007-05-05  5:21 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vejly305u.fsf@assigned-by-dhcp.cox.net>

On Wed, 2 May 2007, Junio C Hamano wrote:

> Daniel Barkalow <barkalow@iabervon.org> writes:
> 
> > Indeed. Is there documentation on all the possibilities for refspecs 
> > somewhere all together? I could only find it for particular programs, and 
> > didn't see a no-colon option permitted anywhere.
> 
> Documentation/pull-fetch-param.txt which is included by various
> manpages would be the best place to start.
> 
>     <refspec>::
>             The canonical format of a <refspec> parameter is
>             `+?<src>:<dst>`; that is, an optional plus `+`, followed
>             by the source ref, followed by a colon `:`, followed by
>             the destination ref.
>     +
>     The remote ref that matches <src>
>     is fetched, and if <dst> is not empty string, the local
>     ref that matches it is fast forwarded using <src>.
>     Again, if the optional plus `+` is used, the local ref
>     is updated even if it does not result in a fast forward
>     update.
> 
> Hmph.  So <dst> could be empty, and in such a case there is no
> tracking.  But this does not say missing colon is allowed (but
> that was intentionally left out as we clarify it later).  Let's
> read on.  After a few [Notes], we find this.
> 
>     Some short-cut notations are also supported.
>     +
>     * `tag <tag>` means the same as `refs/tags/<tag>:refs/tags/<tag>`; 
>       it requests fetching everything up to the given tag.
>     * A parameter <ref> without a colon is equivalent to
>       <ref>: when pulling/fetching, so it merges <ref> into the current
>       branch without storing the remote branch anywhere locally
> 
> Ahh, so a refspec that does not have a colon, and ends with a
> colon (hence an empty <dst>) are equivalent, and does not result
> in remote tracking.

Is this actually supported in config files? At least for pulls, the "tag 
<tag>" notation is only available on the command line, afaict. For that 
matter, I think that having "<ref>:" or "<ref>" in a config file fetch 
line would be really bad; these refs would always be merged into any 
current head (without any remote tracking) when the remote is fetched 
from?

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

* [PATCH] Move remote parsing into a library file out of builtin-push.
From: Daniel Barkalow @ 2007-05-05  5:09 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

The new parser is different from the one in builtin-push in two ways:
the default is to use the current branch's remote, if there is one,
before "origin"; and config is used in preference to remotes.

Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
 Makefile       |    5 +-
 builtin-push.c |  201 ++++++++------------------------------------------------
 remote.c       |  201 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 remote.h       |   18 +++++
 4 files changed, 250 insertions(+), 175 deletions(-)
 create mode 100644 remote.c
 create mode 100644 remote.h

diff --git a/Makefile b/Makefile
index e0a1308..dd64b7d 100644
--- a/Makefile
+++ b/Makefile
@@ -288,7 +288,8 @@ LIB_H = \
 	diff.h object.h pack.h pkt-line.h quote.h refs.h list-objects.h sideband.h \
 	run-command.h strbuf.h tag.h tree.h git-compat-util.h revision.h \
 	tree-walk.h log-tree.h dir.h path-list.h unpack-trees.h builtin.h \
-	utf8.h reflog-walk.h patch-ids.h attr.h decorate.h progress.h mailmap.h
+	utf8.h reflog-walk.h patch-ids.h attr.h decorate.h progress.h \
+	mailmap.h remote.h
 
 DIFF_OBJS = \
 	diff.o diff-lib.o diffcore-break.o diffcore-order.o \
@@ -310,7 +311,7 @@ LIB_OBJS = \
 	write_or_die.o trace.o list-objects.o grep.o match-trees.o \
 	alloc.o merge-file.o path-list.o help.o unpack-trees.o $(DIFF_OBJS) \
 	color.o wt-status.o archive-zip.o archive-tar.o shallow.o utf8.o \
-	convert.o attr.o decorate.o progress.o mailmap.o
+	convert.o attr.o decorate.o progress.o mailmap.o remote.o
 
 BUILTIN_OBJS = \
 	builtin-add.o \
diff --git a/builtin-push.c b/builtin-push.c
index cb78401..49a83bb 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -5,17 +5,13 @@
 #include "refs.h"
 #include "run-command.h"
 #include "builtin.h"
-
-#define MAX_URI (16)
+#include "remote.h"
 
 static const char push_usage[] = "git-push [--all] [--tags] [--receive-pack=<git-receive-pack>] [--repo=all] [-f | --force] [-v] [<repository> <refspec>...]";
 
 static int all, tags, force, thin = 1, verbose;
 static const char *receivepack;
 
-#define BUF_SIZE (2084)
-static char buffer[BUF_SIZE];
-
 static const char **refspec;
 static int refspec_nr;
 
@@ -137,176 +133,30 @@ static void set_refspecs(const char **refs, int nr)
 	expand_refspecs();
 }
 
-static int get_remotes_uri(const char *repo, const char *uri[MAX_URI])
-{
-	int n = 0;
-	FILE *f = fopen(git_path("remotes/%s", repo), "r");
-	int has_explicit_refspec = refspec_nr || all || tags;
-
-	if (!f)
-		return -1;
-	while (fgets(buffer, BUF_SIZE, f)) {
-		int is_refspec;
-		char *s, *p;
-
-		if (!prefixcmp(buffer, "URL:")) {
-			is_refspec = 0;
-			s = buffer + 4;
-		} else if (!prefixcmp(buffer, "Push:")) {
-			is_refspec = 1;
-			s = buffer + 5;
-		} else
-			continue;
-
-		/* Remove whitespace at the head.. */
-		while (isspace(*s))
-			s++;
-		if (!*s)
-			continue;
-
-		/* ..and at the end */
-		p = s + strlen(s);
-		while (isspace(p[-1]))
-			*--p = 0;
-
-		if (!is_refspec) {
-			if (n < MAX_URI)
-				uri[n++] = xstrdup(s);
-			else
-				error("more than %d URL's specified, ignoring the rest", MAX_URI);
-		}
-		else if (is_refspec && !has_explicit_refspec) {
-			if (!wildcard_ref(s))
-				add_refspec(xstrdup(s));
-		}
-	}
-	fclose(f);
-	if (!n)
-		die("remote '%s' has no URL", repo);
-	return n;
-}
-
-static const char **config_uri;
-static const char *config_repo;
-static int config_repo_len;
-static int config_current_uri;
-static int config_get_refspecs;
-static int config_get_receivepack;
-
-static int get_remote_config(const char* key, const char* value)
-{
-	if (!prefixcmp(key, "remote.") &&
-	    !strncmp(key + 7, config_repo, config_repo_len)) {
-		if (!strcmp(key + 7 + config_repo_len, ".url")) {
-			if (config_current_uri < MAX_URI)
-				config_uri[config_current_uri++] = xstrdup(value);
-			else
-				error("more than %d URL's specified, ignoring the rest", MAX_URI);
-		}
-		else if (config_get_refspecs &&
-			 !strcmp(key + 7 + config_repo_len, ".push")) {
-			if (!wildcard_ref(value))
-				add_refspec(xstrdup(value));
-		}
-		else if (config_get_receivepack &&
-			 !strcmp(key + 7 + config_repo_len, ".receivepack")) {
-			if (!receivepack) {
-				char *rp = xmalloc(strlen(value) + 16);
-				sprintf(rp, "--receive-pack=%s", value);
-				receivepack = rp;
-			} else
-				error("more than one receivepack given, using the first");
-		}
-	}
-	return 0;
-}
-
-static int get_config_remotes_uri(const char *repo, const char *uri[MAX_URI])
-{
-	config_repo_len = strlen(repo);
-	config_repo = repo;
-	config_current_uri = 0;
-	config_uri = uri;
-	config_get_refspecs = !(refspec_nr || all || tags);
-	config_get_receivepack = (receivepack == NULL);
-
-	git_config(get_remote_config);
-	return config_current_uri;
-}
-
-static int get_branches_uri(const char *repo, const char *uri[MAX_URI])
-{
-	const char *slash = strchr(repo, '/');
-	int n = slash ? slash - repo : 1000;
-	FILE *f = fopen(git_path("branches/%.*s", n, repo), "r");
-	char *s, *p;
-	int len;
-
-	if (!f)
-		return 0;
-	s = fgets(buffer, BUF_SIZE, f);
-	fclose(f);
-	if (!s)
-		return 0;
-	while (isspace(*s))
-		s++;
-	if (!*s)
-		return 0;
-	p = s + strlen(s);
-	while (isspace(p[-1]))
-		*--p = 0;
-	len = p - s;
-	if (slash)
-		len += strlen(slash);
-	p = xmalloc(len + 1);
-	strcpy(p, s);
-	if (slash)
-		strcat(p, slash);
-	uri[0] = p;
-	return 1;
-}
-
-/*
- * Read remotes and branches file, fill the push target URI
- * list.  If there is no command line refspecs, read Push: lines
- * to set up the *refspec list as well.
- * return the number of push target URIs
- */
-static int read_config(const char *repo, const char *uri[MAX_URI])
-{
-	int n;
-
-	if (*repo != '/') {
-		n = get_remotes_uri(repo, uri);
-		if (n > 0)
-			return n;
-
-		n = get_config_remotes_uri(repo, uri);
-		if (n > 0)
-			return n;
-
-		n = get_branches_uri(repo, uri);
-		if (n > 0)
-			return n;
-	}
-
-	uri[0] = repo;
-	return 1;
-}
-
 static int do_push(const char *repo)
 {
-	const char *uri[MAX_URI];
-	int i, n, errs;
+	int i, errs;
 	int common_argc;
 	const char **argv;
 	int argc;
+	struct remote *remote;
+
+	remote = remote_get(repo);
 
-	n = read_config(repo, uri);
-	if (n <= 0)
+	if (!remote)
 		die("bad repository '%s'", repo);
 
-	argv = xmalloc((refspec_nr + 10) * sizeof(char *));
+	if (remote->receivepack) {
+		char *rp = xmalloc(strlen(remote->receivepack) + 16);
+		sprintf(rp, "--receive-pack=%s", remote->receivepack);
+		receivepack = rp;
+	}
+	if (!refspec && !all && !tags && remote->push_refspec_nr) {
+		refspec_nr = remote->push_refspec_nr;
+		refspec = remote->push_refspec;
+	}
+
+	argv = xmalloc((refspec_nr + 11) * sizeof(char *));
 	argv[0] = "dummy-send-pack";
 	argc = 1;
 	if (all)
@@ -318,18 +168,23 @@ static int do_push(const char *repo)
 	common_argc = argc;
 
 	errs = 0;
-	for (i = 0; i < n; i++) {
+	for (i = 0; i < remote->uri_nr; i++) {
 		int err;
 		int dest_argc = common_argc;
 		int dest_refspec_nr = refspec_nr;
 		const char **dest_refspec = refspec;
-		const char *dest = uri[i];
+		const char *dest = remote->uri[i];
 		const char *sender = "send-pack";
 		if (!prefixcmp(dest, "http://") ||
 		    !prefixcmp(dest, "https://"))
 			sender = "http-push";
-		else if (thin)
-			argv[dest_argc++] = "--thin";
+		else {
+			char *rem = xmalloc(strlen(remote->name) + 10);
+			sprintf(rem, "--remote=%s", remote->name);
+			argv[dest_argc++] = rem;
+			if (thin)
+				argv[dest_argc++] = "--thin";
+		}
 		argv[0] = sender;
 		argv[dest_argc++] = dest;
 		while (dest_refspec_nr--)
@@ -341,7 +196,7 @@ static int do_push(const char *repo)
 		if (!err)
 			continue;
 
-		error("failed to push to '%s'", uri[i]);
+		error("failed to push to '%s'", remote->uri[i]);
 		switch (err) {
 		case -ERR_RUN_COMMAND_FORK:
 			error("unable to fork for %s", sender);
@@ -362,7 +217,7 @@ static int do_push(const char *repo)
 int cmd_push(int argc, const char **argv, const char *prefix)
 {
 	int i;
-	const char *repo = "origin";	/* default repository */
+	const char *repo = NULL;	/* default repository */
 
 	for (i = 1; i < argc; i++) {
 		const char *arg = argv[i];
diff --git a/remote.c b/remote.c
new file mode 100644
index 0000000..32a0acf
--- /dev/null
+++ b/remote.c
@@ -0,0 +1,201 @@
+#include "cache.h"
+#include "remote.h"
+#include "refs.h"
+
+static struct remote **remotes;
+static int allocated_remotes;
+
+#define BUF_SIZE (2084)
+static char buffer[BUF_SIZE];
+
+static void add_push_refspec(struct remote *remote, const char *ref)
+{
+	int nr = remote->push_refspec_nr + 1;
+	remote->push_refspec =
+		xrealloc(remote->push_refspec, nr * sizeof(char *));
+	remote->push_refspec[nr-1] = ref;
+	remote->push_refspec_nr = nr;
+}
+
+static void add_uri(struct remote *remote, const char *uri)
+{
+	int nr = remote->uri_nr + 1;
+	remote->uri =
+		xrealloc(remote->uri, nr * sizeof(char *));
+	remote->uri[nr-1] = uri;
+	remote->uri_nr = nr;
+}
+
+static struct remote *make_remote(const char *name, int len)
+{
+	int i, empty = -1;
+
+	for (i = 0; i < allocated_remotes; i++) {
+		if (!remotes[i]) {
+			if (empty < 0)
+				empty = i;
+		} else {
+			if (len ? (!strncmp(name, remotes[i]->name, len) &&
+				   !remotes[i]->name[len]) :
+			    !strcmp(name, remotes[i]->name))
+				return remotes[i];
+		}
+	}
+
+	if (empty < 0) {
+		empty = allocated_remotes;
+		allocated_remotes += allocated_remotes ? allocated_remotes : 1;
+		remotes = xrealloc(remotes,
+				   sizeof(*remotes) * allocated_remotes);
+		memset(remotes + empty, 0,
+		       (allocated_remotes - empty) * sizeof(*remotes));
+	}
+	remotes[empty] = xcalloc(1, sizeof(struct remote));
+	if (len)
+		remotes[empty]->name = xstrndup(name, len);
+	else
+		remotes[empty]->name = xstrdup(name);
+	return remotes[empty];
+}
+
+static void read_remotes_file(struct remote *remote)
+{
+	FILE *f = fopen(git_path("remotes/%s", remote->name), "r");
+
+	if (!f)
+		return;
+	while (fgets(buffer, BUF_SIZE, f)) {
+		int value_list;
+		char *s, *p;
+
+		if (!prefixcmp(buffer, "URL:")) {
+			value_list = 0;
+			s = buffer + 4;
+		} else if (!prefixcmp(buffer, "Push:")) {
+			value_list = 1;
+			s = buffer + 5;
+		} else
+			continue;
+
+		while (isspace(*s))
+			s++;
+		if (!*s)
+			continue;
+
+		p = s + strlen(s);
+		while (isspace(p[-1]))
+			*--p = 0;
+
+		switch (value_list) {
+		case 0:
+			add_uri(remote, xstrdup(s));
+			break;
+		case 1:
+			add_push_refspec(remote, xstrdup(s));
+			break;
+		}
+	}
+}
+
+static void read_branches_file(struct remote *remote)
+{
+	const char *slash = strchr(remote->name, '/');
+	int n = slash ? slash - remote->name : 1000;
+	FILE *f = fopen(git_path("branches/%.*s", n, remote->name), "r");
+	char *s, *p;
+	int len;
+
+	if (!f)
+		return;
+	s = fgets(buffer, BUF_SIZE, f);
+	fclose(f);
+	if (!s)
+		return;
+	while (isspace(*s))
+		s++;
+	if (!*s)
+		return;
+	p = s + strlen(s);
+	while (isspace(p[-1]))
+		*--p = 0;
+	len = p - s;
+	if (slash)
+		len += strlen(slash);
+	p = xmalloc(len + 1);
+	strcpy(p, s);
+	if (slash)
+		strcat(p, slash);
+	add_uri(remote, p);
+}
+
+static char *default_remote_name = NULL;
+static const char *current_branch = NULL;
+static int current_branch_len = 0;
+
+static int handle_config(const char *key, const char *value)
+{
+	const char *name;
+	const char *subkey;
+	struct remote *remote;
+	if (!prefixcmp(key, "branch.") && current_branch &&
+	    !strncmp(key + 7, current_branch, current_branch_len) &&
+	    !strcmp(key + 7 + current_branch_len, ".remote")) {
+		free(default_remote_name);
+		default_remote_name = xstrdup(value);
+	}
+	if (prefixcmp(key,  "remote."))
+		return 0;
+	name = key + 7;
+	subkey = strrchr(name, '.');
+	if (!subkey)
+		return error("Config with no key for remote %s", name);
+	remote = make_remote(name, subkey - name);
+	if (!strcmp(subkey, ".url")) {
+		add_uri(remote, xstrdup(value));
+	} else if (!strcmp(subkey, ".push")) {
+		add_push_refspec(remote, xstrdup(value));
+	} else if (!strcmp(subkey, ".receivepack")) {
+		if (!remote->receivepack)
+			remote->receivepack = xstrdup(value);
+		else
+			error("more than one receivepack given, using the first");
+	}
+	return 0;
+}
+
+static void read_config(void)
+{
+	unsigned char sha1[20];
+	const char *head_ref;
+	int flag;
+	if (default_remote_name) // did this already
+		return;
+	default_remote_name = xstrdup("origin");
+	current_branch = NULL;
+	head_ref = resolve_ref("HEAD", sha1, 0, &flag);
+	if (head_ref && (flag & REF_ISSYMREF) &&
+	    !prefixcmp(head_ref, "refs/heads/")) {
+		current_branch = head_ref + strlen("refs/heads/");
+		current_branch_len = strlen(current_branch);
+	}
+	git_config(handle_config);
+}
+
+struct remote *remote_get(const char *name)
+{
+	struct remote *ret;
+
+	read_config();
+	if (!name)
+		name = default_remote_name;
+	ret = make_remote(name, 0);
+	if (*name == '/')
+		add_uri(ret, name);
+	if (!ret->uri)
+		read_remotes_file(ret);
+	if (!ret->uri)
+		read_branches_file(ret);
+	if (!ret->uri)
+		return NULL;
+	return ret;
+}
diff --git a/remote.h b/remote.h
new file mode 100644
index 0000000..73747a8
--- /dev/null
+++ b/remote.h
@@ -0,0 +1,18 @@
+#ifndef REMOTE_H
+#define REMOTE_H
+
+struct remote {
+	const char *name;
+
+	const char **uri;
+	int uri_nr;
+
+	const char **push_refspec;
+	int push_refspec_nr;
+
+	const char *receivepack;
+};
+
+struct remote *remote_get(const char *name);
+
+#endif
-- 
1.5.2.rc1.24.gf413-dirty

^ permalink raw reply related

* Re: FFmpeg considering GIT
From: Linus Torvalds @ 2007-05-05  4:15 UTC (permalink / raw)
  To: Michael Niedermayer; +Cc: Carl Worth, git
In-Reply-To: <20070504202448.GD14859@MichaelsNB>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2895 bytes --]



On Fri, 4 May 2007, Michael Niedermayer wrote:
> 
> we have a nice svn policy which explains that, also people wont receive
> write access without having submitted a few clean patches first
> so i dont know if more education would really help, the problems are IMHO
> rather caused by a mix of lazyness, arrogance and plain oversight
> but please dont missunderstand, these problems are not that common, its
> rather once every few month

[ I was away for a few days, so others probably answered already ... ]

With git, the right way to do thigns is to not ever give "write access" to 
the "standard" tree to developers, but to make each developer have their 
own tree, and then one or more developers are the ones that merge other 
peoples work. 

Since I'm the one who does the merging for the kernel, I've made damn sure 
that merging other peoples work is as easy as humanly possible, so that I 
can just sit there, sipping my foofy tropical drink, drunk as a skunk and 
enjoying every moment of seeing my peons work their little fingers to the 
bone, when I do a "git pull ..." and in two seconds I've downloaded their 
work and merged it, and I can take another sip of the Piña Colada. 

Burp.

And git also makes it really easy to see when somebody does something 
stupid. The one thing it always shows to the person doing the merging is 
the diffstat from the result, so if somebody re-indented the source base, 
the merger goes "Whaa", and assuming he's not too drunk to type, he should 
just send a sternly worded message to the developer who did the bad deed, 
and tell them that their work was unacceptable, and won't be pulled.

A simple "git reset --hard ORIG_HEAD" will undo the merge, so the 
person(s) who actually does the integration again doesn't actually have to 
work all that hard.

In other words, the proper sequence really should be to *not* let the 
horribly buggy commits into the standard version in the first place! Sure, 
individual developers will make mistakes, but the fact that they screwed 
up should in _no_ way mean that they can screw up the main repository. The 
whole point in being distributed is that developers can screw up in their 
own _private_ repositories and still have all the power of a proper SCM 
tool, but without actually getting to screw up the main repo.

(And yes, then very occasionally both the developer *and* the maintainer 
screws up, and something bad gets through, and yeah, then you need to 
revert, but the point I'm arguing is that with a fairly good flow of 
development, you don't have to worry about the more clueless people 
screwing up - they can still do development, and you can still pull from 
them, but *if* they screw up, you can tell them to clean up their mess 
*before* you actually put it into any standard tree, and the mess can be 
entirely their _local_ mistake and never visible anywhere else).

			Linus

^ permalink raw reply

* Re: Git benchmarks at OpenOffice.org wiki
From: Linus Torvalds @ 2007-05-05  3:56 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Johannes Schindelin, git, dev, Jan Holesovsky
In-Reply-To: <8fe92b430705020433v7ae5c117qdefccc791cd07fff@mail.gmail.com>



On Wed, 2 May 2007, Jakub Narebski wrote:
> On 5/2/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > 
> > Something I realized with pain is that the refs/ directory is 24MB big.
> > Yep. Really. They have 3464 heads and 2639 tags. I suspect that this is
> > the reason why.
> 
> Then packed refs would certainly help with speed and a bit with size.

Btw, this reminds me: we really should start out clones with a fully 
packed set of refs. It seems stupid to get the refs in one go, and then 
explode them into thousands of files.

A trivial patch is to just do

	git pack-refs --all --prune

in the "git-clone.sh" script rather than force people to do it themselves, 
but we really probably shouldn't have ever even unpacked them in the first 
place. That is kind of stupid, but especially since that thing is written 
in shell, it's hard to do anything smarter.

Of course, I don't know what the hell openoffice is doing with that many 
branches and tags, but I guess it's a normal result of having used CVS/SVN 
- you want to tag every single merge you do, and all branches stay around 
forever, because you can never merge them back and get rid of them.

It's always sad to see the crap that is CVS, and how bad decisions in CVS 
end up resulting in pain downstream.

		Linus

^ permalink raw reply

* Re: [PATCH] Have git-revert, git-cherry-pick cleanup ./.msg upon successful completion.
From: Shawn O. Pearce @ 2007-05-05  3:46 UTC (permalink / raw)
  To: Gerrit Pape; +Cc: git, Alex Riesen
In-Reply-To: <20070504143210.8690.qmail@a4171e0dbe98e3.315fe32.mid.smarden.org>

Gerrit Pape <pape@smarden.org> wrote:
> git-revert and git-cherry-pick left behind the commit message file ./.msg,
> have them use the -f option to git-commit to properly cleanup the
> automatically created file.

I'm actually sort of against changing the behavior of git-commit
-f to mean "delete the file".  We never did that before.  Users
might get surprised when their file goes away!

What about this change instead?  We make cherry-pick/revert
use the same temporary file as merge, which is under .git/
(something Alex mentioned he wanted).  I think the use of ".msg"
in cherry-pick/revert has always just been a bug, and not a feature,
so I'm really not against changing things around like this.


diff --git a/builtin-revert.c b/builtin-revert.c
index 4ba0ee6..67c13a3 100644
--- a/builtin-revert.c
+++ b/builtin-revert.c
@@ -237,6 +237,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 	int i;
 	char *oneline, *reencoded_message = NULL;
 	const char *message, *encoding;
+	const char *defmsg = git_path("MERGE_MSG");
 
 	git_config(git_default_config);
 	me = action == REVERT ? "revert" : "cherry-pick";
@@ -280,7 +281,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 	 * reverse of it if we are revert.
 	 */
 
-	msg_fd = hold_lock_file_for_update(&msg_file, ".msg", 1);
+	msg_fd = hold_lock_file_for_update(&msg_file, defmsg, 1);
 
 	encoding = get_encoding(message);
 	if (!encoding)
@@ -330,7 +331,6 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 				sha1_to_hex(head), "HEAD",
 				sha1_to_hex(next->object.sha1), oneline) ||
 			write_tree(head, 0, NULL)) {
-		const char *target = git_path("MERGE_MSG");
 		add_to_msg("\nConflicts:\n\n");
 		read_cache();
 		for (i = 0; i < active_nr;) {
@@ -345,10 +345,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 			}
 		}
 		if (close(msg_fd) || commit_lock_file(&msg_file) < 0)
-			die ("Error wrapping up .msg");
-		unlink(target);
-		if (rename(".msg", target))
-			die ("Could not move .msg to %s", target);
+			die ("Error wrapping up %s", defmsg);
 		fprintf(stderr, "Automatic %s failed.  "
 			"After resolving the conflicts,\n"
 			"mark the corrected paths with 'git-add <paths>'\n"
@@ -362,7 +359,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 		exit(1);
 	}
 	if (close(msg_fd) || commit_lock_file(&msg_file) < 0)
-		die ("Error wrapping up .msg");
+		die ("Error wrapping up %s", defmsg);
 	fprintf(stderr, "Finished one %s.\n", me);
 
 	/*
@@ -376,11 +373,9 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 
 	if (!no_commit) {
 		if (edit)
-			return execl_git_cmd("commit", "-n", "-F", ".msg",
-				"-e", NULL);
+			return execl_git_cmd("commit", "-n", NULL);
 		else
-			return execl_git_cmd("commit", "-n", "-F", ".msg",
-				NULL);
+			return execl_git_cmd("commit", "-n", "-F", defmsg, NULL);
 	}
 	if (reencoded_message)
 		free(reencoded_message);

-- 
Shawn.

^ permalink raw reply related


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