Git development
 help / color / mirror / Atom feed
* gitsub{module,project}
From: Michael J Gruber @ 2008-11-24 15:54 UTC (permalink / raw)
  To: Git Mailing List

Dear all:

Parts of git still use the term subproject rather then submodule. A
simple grep/wc -l shows submodule as the winner by 10:1 or so. Are there
any objections to consolidating that naming (in favour of submodule)?

Michael

^ permalink raw reply

* Re: How do I delete a git submodule?
From: Michael J Gruber @ 2008-11-24 15:51 UTC (permalink / raw)
  To: Peter Krefting; +Cc: Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0811241537580.22925@ds9.cixit.se>

Peter Krefting venit, vidit, dixit 24.11.2008 15:44:
> Hi!
> 
> I am trying to remove a submodule from a project, but can't figure out
> how to do it. With "modules/aldc" as a submodule:
> 
>   $ cat .gitmodules
>   [submodule "modules/aldc"]
>           path = modules/aldc
>           url = [...cut...]
>   [...more entries here...]
>   $ git status
>   [...cut...]
>   nothing to commit (working directory clean)
> 
> Starting from a clean set-up.
> 
>   $ git submodule rm modules/aldc
>   -ff17d9e0e76d01e99d26aa16b28cf327c5abb48a modules/aldc
>   $ git status
>   [...cut...]
>   nothing to commit (working directory clean)
> 
> The obvious command didn't give any errors, but didn't do anything,
> either. (I notice that there is no mention of a "git submodule rm" on
> the git-submodule man page.)
> 
>   $ git rm modules/aldc
>   rm 'modules/aldc'
>   fatal: git rm: modules/aldc: Permission denied
>   $ git status
>   [...cut...]
>   nothing to commit (working directory clean)
> 
> Nope, that didn't work either.
> 
> What am I missing?

The last line of git help/man on git submodule status:

"This command is the default command for git-submodule."

git submodule ignores the "rm" (because it doesn't know that command)
and does a "status" on modules/aldc.

OTOH, git rm refuses to do its thing, as you noticed. The brute force
method seems to work:

rm -Rf modules/aldc
#remove module from .gitmodules
#remove module from .git/config
git add -u
git commit -m "get thee hence, modules/aldc"

I edited the two config files manually, but this can be scripted.
git log -p shows a perfect diff including removal of the subproject
commit, so I think the result of the above is fine. I think I'll work on
a patch if nobody stops or beats me...

Cheers,
Michael

^ permalink raw reply

* Re: Best practices for maintaining a subversion mirror?
From: Peter Harris @ 2008-11-24 15:37 UTC (permalink / raw)
  To: Oliver Charles; +Cc: git
In-Reply-To: <5641883d0811240255i21aef729ld202bd5a3211dad9@mail.gmail.com>

On Mon, Nov 24, 2008 at 5:55 AM, Oliver Charles wrote:
> This seems to be working nicely as a readonly interface, but when I
> clone this repository (git clone git://foo/repo.git) - served via
> git-daemon - I can't seem to get svn rebase to work on it, which means
> I can't dcommit my changes back to Subversion.
>
> Am I doing things generally the right way, or am I doing it completely
> wrong? :-) Any tips appreciated!

First, make sure you aren't using the --no-metadata flag. git svn
rebase needs that metadata to rebuild its index.

Second, a simple "git clone" will not set up the svn remotes; you have
to do that by hand. This is what I do when I clone my svn mirror:

mkdir project
cd project
git init
git remote add origin git://mirror/project
git config --add remote.origin.fetch +refs/remotes/*:refs/remotes/*
git fetch
git svn init -Ttrunk svn://upstream/project
git config --add svn-remote.svn.fetch branches/foo:refs/remotes/foo
git config --add svn-remote.svn.fetch branches/bar:refs/remotes/bar
git reset --hard trunk
git svn rebase

Once you have it nailed down, you can put it into a script so all your
co-workers don't have to do that by hand too.

(Also, I quickly reset my cron job to run every five minutes; I found
an hour was way too long for my taste. YMMV, of course)

Peter Harris

^ permalink raw reply

* Re: [StGit PATCH] stgit.el: Show running commands
From: Karl Hasselström @ 2008-11-24 15:34 UTC (permalink / raw)
  To: David Kågedal; +Cc: catalin.marinas, git
In-Reply-To: <20081124120136.9163.70947.stgit@krank>

Thanks, looks useful.

Sorry I haven't taken the time to apply & push out your patches. I'm
hoping it'll happen Real Soon Now(tm) ...

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

^ permalink raw reply

* Re: Not-quite-a-bug in pickaxe
From: Mikael Magnusson @ 2008-11-24 15:08 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, gitster
In-Reply-To: <alpine.DEB.1.00.0811241238450.30769@pacific.mpi-cbg.de>

2008/11/24 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
> Hi people,
>
> I do not know if I hit that behavior before,

I'm pretty sure it has,
http://thread.gmane.org/gmane.comp.version-control.git/100113/focus=100115
(at least i think that's the same case?)

> but now it hit me big time:
> if a commit moves a certain string from one place to another, "git log
> -S<string>" does not pick up that commit.  The code responsible for this
> is in diffcore-pickaxe.c:
>
>                        else if (!diff_unmodified_pair(p) &&
>                                 contains(p->one, needle, len, regexp) !=
>                                 contains(p->two, needle, len, regexp))
>                                has_changes++;
>
> Basically, the "contains()" method says how often the needle was found,
> and pickaxe just assumes that a move of a string is not interesting
> enough.
>
> Now, this behavior is probably intended, as searching files is much
> cheaper than generating the diffs between them, yet it is something you'll
> have to keep in mind when using "-S".

-- 
Mikael Magnusson

^ permalink raw reply

* How do I delete a git submodule?
From: Peter Krefting @ 2008-11-24 14:44 UTC (permalink / raw)
  To: Git Mailing List

Hi!

I am trying to remove a submodule from a project, but can't figure out
how to do it. With "modules/aldc" as a submodule:

  $ cat .gitmodules
  [submodule "modules/aldc"]
          path = modules/aldc
          url = [...cut...]
  [...more entries here...]
  $ git status
  [...cut...]
  nothing to commit (working directory clean)

Starting from a clean set-up.

  $ git submodule rm modules/aldc
  -ff17d9e0e76d01e99d26aa16b28cf327c5abb48a modules/aldc
  $ git status
  [...cut...]
  nothing to commit (working directory clean)

The obvious command didn't give any errors, but didn't do anything,
either. (I notice that there is no mention of a "git submodule rm" on
the git-submodule man page.)

  $ git rm modules/aldc
  rm 'modules/aldc'
  fatal: git rm: modules/aldc: Permission denied
  $ git status
  [...cut...]
  nothing to commit (working directory clean)

Nope, that didn't work either.

What am I missing?

  $ git --version
  git version 1.6.0.2.1172.ga5ed0

(This is Git-1.6.0.2-preview20080923 on Windows XP)

-- 
\\// Peter - http://www.softwolves.pp.se/

^ permalink raw reply

* Re: Stgit and refresh-temp
From: Jon Smirl @ 2008-11-24 14:11 UTC (permalink / raw)
  To: Karl Hasselström; +Cc: Catalin Marinas, Git Mailing List
In-Reply-To: <20081124111635.GA31986@diana.vm.bytemark.co.uk>

A useful future enhancement would be to adjust patches that insert
adjacent areas so that they can pass each other in a pop/pop
operation. The most common case of this being appends to end of file.

This would be a form of automatic conflict resolution. The pending
patch would be automatically corrected to resolve the adjacent insert
conflict.  You might want a prompt asking if this was ok andt then
remember the answer so that question is not asked repeatedly.

It's a mechanism to say that multiple insertions at point X in the
original file don't matter in their order of insertion.

-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* Re: git-svn and svnsync
From: Andriy Gapon @ 2008-11-24 12:56 UTC (permalink / raw)
  To: git
In-Reply-To: <48F6F631.90709@icyb.net.ua>

on 16/10/2008 11:07 Andriy Gapon said the following:
> 
> I did the following:
> 1. mirror svn repository using svnsync
> 2. clone the mirror with git-svn --use-svnsync-props and some tweaking[*]
> 3. run git svn info in the clone
> 4. get error "Unable to determine upstream SVN information from working
> tree history"
> 
> git svn log and git svn rebase do not work either.
> git log does work.
> 
> I have git version 1.6.0.2 (from FreeBSD ports).
> [*] About the tweaking - I manually massaged config file to get the
> branches I was interested in, so .git/config is this:
> [core]
>         repositoryformatversion = 0
>         filemode = true
>         bare = false
>         logallrefupdates = true
> [svn-remote "svn"]
>         useSvnsyncProps = 1
>         url = file:///system/devel/svn/base
>         fetch = head:refs/remotes/trunk
>         fetch = stable/6:refs/remotes/stable_6
>         fetch = stable/7:refs/remotes/stable_7
>         fetch = releng/6.3:refs/remotes/releng_6_3
>         fetch = releng/6.4:refs/remotes/releng_6_4
>         fetch = releng/7.0:refs/remotes/releng_7_0
>         fetch = release/6.3.0:refs/remotes/release_6_3_0
>         fetch = release/7.0.0:refs/remotes/release_7_0_0
> 
> This is .git/svn/.metadata:
> ; This file is used internally by git-svn
> ; You should not have to edit it
> [svn-remote "svn"]
>         reposRoot = file:///system/devel/svn/base
>         uuid = ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f
>         svnsync-uuid = ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f
>         svnsync-url = svn://svn.freebsd.org/base
> 
> So you can see the original svn repository URL.
> 
> git log reports svn info like the following:
> git-svn-id: svn://svn.freebsd.org/base/stable/7@183898
> ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f
> 
> It seems that the problem is that the code in find_by_url() and/or
> read_all_remotes() subroutines (in git-svn.perl) are not aware of
> svnsync and do not realize an URL in logs and URL in svn-remote are
> different.
> BTW, I see that there is some special svm logic in read_all_remotes.
> 
> Thank you very much for any help in advance.
> 

Anyone?
I wonder if this is my local problem/misconfiguration or an issue with
git-svn.


-- 
Andriy Gapon

^ permalink raw reply

* [StGit PATCH] stgit.el: Show running commands
From: David Kågedal @ 2008-11-24 12:01 UTC (permalink / raw)
  To: kha, catalin.marinas; +Cc: git


---
 contrib/stgit.el |   16 +++++++++++-----
 1 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/contrib/stgit.el b/contrib/stgit.el
index 65af068..ec08a5b 100644
--- a/contrib/stgit.el
+++ b/contrib/stgit.el
@@ -68,9 +68,15 @@ Argument DIR is the repository path."
            (display-buffer output-buf t)))))
 (put 'stgit-capture-output 'lisp-indent-function 1)
 
-(defun stgit-run (&rest args)
+(defun stgit-run-silent (&rest args)
   (apply 'call-process "stg" nil standard-output nil args))
 
+(defun stgit-run (&rest args)
+  (let ((msgcmd (mapconcat #'identity args " ")))
+    (message "Running stg %s..." msgcmd)
+    (apply 'call-process "stg" nil standard-output nil args)
+    (message "Running stg %s...done" msgcmd)))
+
 (defun stgit-reload ()
   "Update the contents of the stgit buffer"
   (interactive)
@@ -79,8 +85,8 @@ Argument DIR is the repository path."
         (curpatch (stgit-patch-at-point)))
     (erase-buffer)
     (insert "Branch: ")
-    (stgit-run "branch")
-    (stgit-run "series" "--description")
+    (stgit-run-silent "branch")
+    (stgit-run-silent "series" "--description")
     (stgit-rescan)
     (if curpatch
         (stgit-goto-patch curpatch)
@@ -338,7 +344,7 @@ With numeric prefix argument, pop that many patches."
     (set (make-local-variable 'stgit-edit-patch) patch)
     (setq default-directory dir)
     (let ((standard-output edit-buf))
-      (stgit-run "edit" "--save-template=-" patch))))
+      (stgit-run-silent "edit" "--save-template=-" patch))))
 
 (defun stgit-confirm-edit ()
   (interactive)
@@ -402,7 +408,7 @@ With numeric prefix argument, pop that many patches."
     (set (make-local-variable 'stgit-patches) patch-names)
     (setq default-directory dir)
     (let ((standard-output edit-buf))
-      (apply 'stgit-run "coalesce" "--save-template=-" patch-names))))
+      (apply 'stgit-run-silent "coalesce" "--save-template=-" patch-names))))
 
 (defun stgit-confirm-coalesce ()
   (interactive)

^ permalink raw reply related

* Not-quite-a-bug in pickaxe
From: Johannes Schindelin @ 2008-11-24 11:43 UTC (permalink / raw)
  To: git, gitster

Hi people,

I do not know if I hit that behavior before, but now it hit me big time: 
if a commit moves a certain string from one place to another, "git log 
-S<string>" does not pick up that commit.  The code responsible for this 
is in diffcore-pickaxe.c:

                        else if (!diff_unmodified_pair(p) &&
                                 contains(p->one, needle, len, regexp) !=
                                 contains(p->two, needle, len, regexp))
                                has_changes++;

Basically, the "contains()" method says how often the needle was found, 
and pickaxe just assumes that a move of a string is not interesting 
enough.

Now, this behavior is probably intended, as searching files is much 
cheaper than generating the diffs between them, yet it is something you'll 
have to keep in mind when using "-S".

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] Display (empty) when appropriate for the goto command
From: Karl Hasselström @ 2008-11-24 11:19 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git
In-Reply-To: <20081123212318.9835.53505.stgit@localhost.localdomain>

On 2008-11-23 21:23:18 +0000, Catalin Marinas wrote:

> This is a fix for bug #11810. The original implementation of goto
> used to display (empty patch) when a patch became empty during a
> push + merge operation. This patch adds this feature again.
>
> Signed-off-by: Catalin Marinas <catalin.marinas@gmail.com>

Acked-by: Karl Hasselström <kha@treskal.com>

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

^ permalink raw reply

* Re: Stgit and refresh-temp
From: Karl Hasselström @ 2008-11-24 11:16 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: Jon Smirl, Git Mailing List
In-Reply-To: <b0943d9e0811231320s804eff0k5aecbac84cb2ffe7@mail.gmail.com>

On 2008-11-23 21:20:59 +0000, Catalin Marinas wrote:

> 2008/11/12 Karl Hasselström <kha@treskal.com>:
>
> > On 2008-11-12 10:02:10 +0000, Catalin Marinas wrote:
> >
> > > I think it's just a matter of updating HEAD on the
> > > "merge_conflict" path but I'm still not fully confident in
> > > modifying the new lib infrastructure.
> >
> > You're probably right.
>
> The simple patch below seems to fix it the goto issue. Could you
> please confirm its correctness (the patch might be wrapped by the web
> interface)?
>
> diff --git a/stgit/lib/transaction.py b/stgit/lib/transaction.py
> index 6623645..0f414d8 100644
> --- a/stgit/lib/transaction.py
> +++ b/stgit/lib/transaction.py
> @@ -321,6 +321,7 @@ class StackTransaction(object):
>          if any(getattr(cd, a) != getattr(orig_cd, a) for a in
>                 ['parent', 'tree', 'author', 'message']):
>              comm = self.__stack.repository.commit(cd)
> +            self.head = comm
>          else:
>              comm = None
>              s = ' (unmodified)'

OK, so I just took a quick look, and my understanding of the problem
(without having had time to actually run any code to confirm) is that
the following happens:

  1. In push_patch(), we delay the final stack update (the update()
     function) since we want to record the state just before the
     conflict in the stack log.

  2. In run(), we update the branch head before running the delayed
     stack update (self.__conflicting_push()).

Your patch works around this problem by explicitly specifying what the
branch head should be; this mechanism is used by undo etc. to be able
to set the branch head to something that isn't the stack top.

At first I thought it was something of a hack, and that the run()
function should probably be modified to do something clever when
self.__conflicting_push != None, such as writing the branch head
later. But there's a reason the checkout is done first in run(): it's
the one and only thing that can go wrong, in which case we have to
roll back. So I guess your solution is indeed at least as right and
proper as anything I would have come up with.

But I would recommend a big fat comment just before the line you
insert that explains why we have to set self.head hard in this case
(namely, that the update() function is run _after_ checkout in run()).

With that comment,

Acked-by: Karl Hasselström <kha@treskal.com>

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

^ permalink raw reply

* Re: [RFC PATCH 1/4] Refactor builtin-verify-tag.c
From: Johannes Schindelin @ 2008-11-24 11:04 UTC (permalink / raw)
  To: Deskin Miller; +Cc: git
In-Reply-To: <1227497000-8684-2-git-send-email-deskinm@umich.edu>

Hi,

On Sun, 23 Nov 2008, Deskin Miller wrote:

> builtin-verify-tag.c didn't expose any of its functionality to be used
> internally.  Refactor some of it into new verify-tag.c and expose
> verify_tag_sha1 able to be called from elsewhere in git.
> 
> Signed-off-by: Deskin Miller <deskinm@umich.edu>
> ---
>  Makefile             |    2 +
>  builtin-verify-tag.c |   61 ++-------------------------------------
>  verify-tag.c         |   77 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  verify-tag.h         |   10 ++++++
>  4 files changed, 93 insertions(+), 57 deletions(-)
>  create mode 100644 verify-tag.c
>  create mode 100644 verify-tag.h

I'll comment on the output of "format-patch -n -C -C" instead, as that 
makes it much easier to see what you actually did:

>  Makefile                             |    2 +
>  builtin-verify-tag.c                 |   61 ++-------------------------------
>  builtin-verify-tag.c => verify-tag.c |   48 ++++-----------------------
>  verify-tag.h                         |   10 +++++
>  4 files changed, 23 insertions(+), 98 deletions(-)
>  copy builtin-verify-tag.c => verify-tag.c (56%)
>  create mode 100644 verify-tag.h
>
> [...] 
> diff --git a/builtin-verify-tag.c b/verify-tag.c
> similarity index 56%
> copy from builtin-verify-tag.c
> copy to verify-tag.c
> index 729a159..c9be331 100644
> --- a/builtin-verify-tag.c
> +++ b/verify-tag.c
> @@ -1,18 +1,12 @@
>  /*
> - * Builtin "git verify-tag"
> + * Internals for "git verify-tag"

Agree.

>   *
> - * Copyright (c) 2007 Carlos Rica <jasampler@gmail.com>
> + * Copyright (c) 2008 Deskin Miller <deskinm@umich.edu>

Disagree.

Even if Carlos seemed to stop his work on Git entirely, which I find 
disappointing, you are _not_ free to pretend his work is yours.  And given 
this diff:

>   *
> - * Based on git-verify-tag.sh
>   */
>  #include "cache.h"
> -#include "builtin.h"
> -#include "tag.h"
> +#include "object.h"
>  #include "run-command.h"
> -#include <signal.h>
> -
> -static const char builtin_verify_tag_usage[] =
> -		"git verify-tag [-v|--verbose] <tag>...";
>  
>  #define PGP_SIGNATURE "-----BEGIN PGP SIGNATURE-----"
>  
> @@ -60,52 +54,24 @@ static int run_gpg_verify(const char *buf, unsigned long size, int verbose)
>  	return ret;
>  }
>  
> -static int verify_tag(const char *name, int verbose)
> +int verify_tag_sha1(const unsigned char *sha1, int verbose)
>  {
>  	enum object_type type;
> -	unsigned char sha1[20];
>  	char *buf;
>  	unsigned long size;
>  	int ret;
>  
> -	if (get_sha1(name, sha1))
> -		return error("tag '%s' not found.", name);
> -
>  	type = sha1_object_info(sha1, NULL);
>  	if (type != OBJ_TAG)
> -		return error("%s: cannot verify a non-tag object of type %s.",
> -				name, typename(type));
> +		return error("Cannot verify a non-tag object of type %s.",
> +				typename(type));
>  
>  	buf = read_sha1_file(sha1, &type, &size);
>  	if (!buf)
> -		return error("%s: unable to read file.", name);
> +		return error("Cnable to read file.");
>  
>  	ret = run_gpg_verify(buf, size, verbose);
>  
>  	free(buf);
>  	return ret;
>  }
> -
> -int cmd_verify_tag(int argc, const char **argv, const char *prefix)
> -{
> -	int i = 1, verbose = 0, had_error = 0;
> -
> -	git_config(git_default_config, NULL);
> -
> -	if (argc > 1 &&
> -	    (!strcmp(argv[i], "-v") || !strcmp(argv[i], "--verbose"))) {
> -		verbose = 1;
> -		i++;
> -	}
> -
> -	if (argc <= i)
> -		usage(builtin_verify_tag_usage);
> -
> -	/* sometimes the program was terminated because this signal
> -	 * was received in the process of writing the gpg input: */
> -	signal(SIGPIPE, SIG_IGN);
> -	while (i < argc)
> -		if (verify_tag(argv[i++], verbose))
> -			had_error = 1;
> -	return had_error;
> -}

I think pretty much all you did was deleting (and thereby you do not gain 
any copyright).

Except for one change: why on earth did you think it a good idea to 
suppress telling the user the _name_ of the tag when an error occurs?

I, for one, would find it way less than helpful to read

	Cannot verify a non-tag object of type blob.

than to read

	refs/tags/dscho-key: cannot verify a non-tag object of type blob.

Besides, I do not see where you warn that "tag <name> not found."  Changes 
like this one need to be justified (by saying in the commit message where 
the warning is already issued, and not letting the reviewer/reader leave 
wondering).

Please, next time you submit a patch like this, do the -C -C yourself.  
Letting all the reviewers do it looks lousy on the overall time balance 
sheet, and it may also lead to a potential reviewer preferring to do 
something else instead.

Now, Junio already said that he is not (yet) convinced that this change 
should be in Git proper, rather than a hook, so it is up to you to decide 
if you deem it important enough to try harder to convince people.

I, for one, would think that it may be a good change: AFAIK only hard-core 
gits use hooks, everybody else avoids them.  So if we deem verifying 
signatures important enough, we might want to have better support for it 
than some example hooks.

So color me half-convinced.

Ciao,
Dscho

^ permalink raw reply

* Best practices for maintaining a subversion mirror?
From: Oliver Charles @ 2008-11-24 10:55 UTC (permalink / raw)
  To: git

Hi,

I just got myself a new server, and as I'm the major Git-using person
at my work, I'd like to set a mirror of our Subversion repository up
to hopefully convert a few more people. At the moment, I've done git
svn clone http://svn..., and then git clone --bare to make a --bare
version of this clone.

Now that I have this mirror, I have this entry in my crontab to update
it hourly:

0 * * * * cd /.git && /usr/bin/git --git-dir=. svn fetch &&
/usr/bin/git update-ref refs/heads/master refs/remotes/git-svn

This seems to be working nicely as a readonly interface, but when I
clone this repository (git clone git://foo/repo.git) - served via
git-daemon - I can't seem to get svn rebase to work on it, which means
I can't dcommit my changes back to Subversion.

Am I doing things generally the right way, or am I doing it completely
wrong? :-) Any tips appreciated!

- Oliver Charles

^ permalink raw reply

* Re: [RFC PATCH 4/4] Make git fetch verify signed tags
From: Johannes Schindelin @ 2008-11-24 10:44 UTC (permalink / raw)
  To: Deskin Miller; +Cc: git
In-Reply-To: <1227497000-8684-5-git-send-email-deskinm@umich.edu>

Hi,

On Sun, 23 Nov 2008, Deskin Miller wrote:

> When git fetch downloads signed tag objects, make it verify them right 
> then.  This extends the output summary of fetch to include "(good 
> signature)" for valid tags and "(BAD SIGNATURE)" for invalid tags.  If 
> the user does not have the correct key in the gpg keyring, gpg returns 
> 2, verify_tag_sha1 returns -2 and nothing additional is output about the 
> tag's validity.

This must be turned off by default, IMO.  You cannot expect each and every 
developer to have gpg _and_ all those public keys installed.

Ciao,
Dscho

^ permalink raw reply

* Re: [RFC PATCH 0/4] Teach git fetch to verify signed tags automatically
From: Johannes Schindelin @ 2008-11-24 10:41 UTC (permalink / raw)
  To: Deskin Miller; +Cc: git
In-Reply-To: <1227497000-8684-1-git-send-email-deskinm@umich.edu>

Hi,

On Sun, 23 Nov 2008, Deskin Miller wrote:

> -What to do if a tag is found to have a bad signature?

Or even worse: if the public key was not found?  In dubio pro reo, they 
say, but OTOH you asked to verify the signatures...

Ciao,
Dscho

^ permalink raw reply

* Re: [RFC/PATCH] bisect: teach "skip" to accept special arguments   like "A..B"
From: Johannes Schindelin @ 2008-11-24 10:35 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Christian Couder, Junio C Hamano, git, H. Peter Anvin
In-Reply-To: <492A5346.1040405@viscovery.net>

Hi,

On Mon, 24 Nov 2008, Johannes Sixt wrote:

> Johannes Schindelin schrieb:
> > On Sun, 23 Nov 2008, Christian Couder wrote:
> >> 	Dscho wrote:
> >> 	> Would it not be more intuitive to have support for
> >> 	>
> >>         > git bisect skip A..B
> >> 	>
> >> 	> ?
> >>
> >> 	Here is a patch to do that. I am not sure it's worth it
> >> 	because this is a special case in many ways.
> > 
> > Why not have something like
> > 
> > 	skip)
> > 		for arg in $(git rev-list "$@")
> > 		do
> > 			bisect_state skip $arg
> > 		done
> 
> Because if you say
> 
> 	$ git bisect skip
> 
> this would be incorrectly calling rev-list; but more importantly, if you say
> 
> 	$ git bisect skip A
> 
> then this would skip A *and all its ancestors*. Not quite what you intended.

Good point indeed.

Thanks,
Dscho

^ permalink raw reply

* Re: [PATCH] config.txt: alphabetize configuration variable groups
From: Johannes Sixt @ 2008-11-24  7:28 UTC (permalink / raw)
  To: Matt McCutchen; +Cc: git
In-Reply-To: <1227510109.32583.2.camel@mattlaptop2.local>

Matt McCutchen schrieb:
> @@ -963,6 +953,8 @@ man.<tool>.path::
>  	Override the path for the given tool that may be used to
>  	display help in the 'man' format. See linkgit:git-help[1].
>  
> +include::merge-config.txt[]
> +
>  merge.conflictstyle::
>  	Specify the style in which conflicted hunks are written out to
>  	working tree files upon merge.  The default is "merge", which

Here, the list is not in alphabetic order anymore.

BTW, your commit message should emphasize the use-cases where an
alphabetic order is a real benefit. Otherwise, this is just code churn.

-- Hannes

^ permalink raw reply

* Re: [RFC/PATCH] bisect: teach "skip" to accept special arguments like "A..B"
From: Johannes Sixt @ 2008-11-24  7:09 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Christian Couder, Junio C Hamano, git, H. Peter Anvin
In-Reply-To: <alpine.DEB.1.00.0811240126430.30769@pacific.mpi-cbg.de>

Johannes Schindelin schrieb:
> On Sun, 23 Nov 2008, Christian Couder wrote:
>> 	Dscho wrote:
>> 	> Would it not be more intuitive to have support for
>> 	>
>>         > git bisect skip A..B
>> 	>
>> 	> ?
>>
>> 	Here is a patch to do that. I am not sure it's worth it
>> 	because this is a special case in many ways.
> 
> Why not have something like
> 
> 	skip)
> 		for arg in $(git rev-list "$@")
> 		do
> 			bisect_state skip $arg
> 		done

Because if you say

	$ git bisect skip

this would be incorrectly calling rev-list; but more importantly, if you say

	$ git bisect skip A

then this would skip A *and all its ancestors*. Not quite what you intended.

-- Hannes

^ permalink raw reply

* [PATCH] config.txt: alphabetize configuration variable groups
From: Matt McCutchen @ 2008-11-24  7:01 UTC (permalink / raw)
  To: git

Signed-off-by: Matt McCutchen <matt@mattmccutchen.net>
---

This is an update of the following patch I sent on October 18, which
received no comment:
http://marc.info/?l=git&m=122438056720340&w=2

 Documentation/config.txt |  122 +++++++++++++++++++++++-----------------------
 1 files changed, 61 insertions(+), 61 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index d536732..c339f56 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -572,9 +572,6 @@ color.status.<slot>::
 	to red). The values of these variables may be specified as in
 	color.branch.<slot>.
 
-commit.template::
-	Specify a file to use as the template for new commit messages.
-
 color.ui::
 	When set to `always`, always use colors in all git commands which
 	are capable of colored output. When false (or `never`), never. When
@@ -582,6 +579,9 @@ color.ui::
 	terminal. When more specific variables of color.* are set, they always
 	take precedence over this setting. Defaults to false.
 
+commit.template::
+	Specify a file to use as the template for new commit messages.
+
 diff.autorefreshindex::
 	When using 'git-diff' to compare with work tree
 	files, do not consider stat-only change as changed.
@@ -714,18 +714,6 @@ gc.rerereunresolved::
 	kept for this many days when 'git-rerere gc' is run.
 	The default is 15 days.  See linkgit:git-rerere[1].
 
-rerere.autoupdate::
-	When set to true, `git-rerere` updates the index with the
-	resulting contents after it cleanly resolves conflicts using
-	previously recorded resolution.  Defaults to false.
-
-rerere.enabled::
-	Activate recording of resolved conflicts, so that identical
-	conflict hunks can be resolved automatically, should they
-	be encountered again.  linkgit:git-rerere[1] command is by
-	default enabled if you create `rr-cache` directory under
-	`$GIT_DIR`, but can be disabled by setting this option to false.
-
 gitcvs.enabled::
 	Whether the CVS server interface is enabled for this repository.
 	See linkgit:git-cvsserver[1].
@@ -917,6 +905,10 @@ i18n.logOutputEncoding::
 	Character encoding the commit messages are converted to when
 	running 'git-log' and friends.
 
+imap::
+	The configuration variables in the 'imap' section are described
+	in linkgit:git-imap-send[1].
+
 instaweb.browser::
 	Specify the program that will be used to browse your working
 	repository in gitweb. See linkgit:git-instaweb[1].
@@ -952,8 +944,6 @@ man.viewer::
 	Specify the programs that may be used to display help in the
 	'man' format. See linkgit:git-help[1].
 
-include::merge-config.txt[]
-
 man.<tool>.cmd::
 	Specify the command to invoke the specified man viewer. The
 	specified command is evaluated in shell with the man page
@@ -963,6 +953,8 @@ man.<tool>.path::
 	Override the path for the given tool that may be used to
 	display help in the 'man' format. See linkgit:git-help[1].
 
+include::merge-config.txt[]
+
 merge.conflictstyle::
 	Specify the style in which conflicted hunks are written out to
 	working tree files upon merge.  The default is "merge", which
@@ -1079,6 +1071,41 @@ pull.octopus::
 pull.twohead::
 	The default merge strategy to use when pulling a single branch.
 
+receive.fsckObjects::
+	If it is set to true, git-receive-pack will check all received
+	objects. It will abort in the case of a malformed object or a
+	broken link. The result of an abort are only dangling objects.
+	Defaults to false.
+
+receive.unpackLimit::
+	If the number of objects received in a push is below this
+	limit then the objects will be unpacked into loose object
+	files. However if the number of received objects equals or
+	exceeds this limit then the received pack will be stored as
+	a pack, after adding any missing delta bases.  Storing the
+	pack from a push can make the push operation complete faster,
+	especially on slow filesystems.  If not set, the value of
+	`transfer.unpackLimit` is used instead.
+
+receive.denyDeletes::
+	If set to true, git-receive-pack will deny a ref update that deletes
+	the ref. Use this to prevent such a ref deletion via a push.
+
+receive.denyNonFastForwards::
+	If set to true, git-receive-pack will deny a ref update which is
+	not a fast forward. Use this to prevent such an update via a push,
+	even if that push is forced. This configuration variable is
+	set when initializing a shared repository.
+
+receive.denyCurrentBranch::
+	If set to true or "refuse", receive-pack will deny a ref update
+	to the currently checked out branch of a non-bare repository.
+	Such a push is potentially dangerous because it brings the HEAD
+	out of sync with the index and working tree. If set to "warn",
+	print a warning of such a push to stderr, but allow the push to
+	proceed. If set to false or "ignore", allow such pushes with no
+	message. Defaults to "warn".
+
 remote.<name>.url::
 	The URL of a remote repository.  See linkgit:git-fetch[1] or
 	linkgit:git-push[1].
@@ -1128,6 +1155,18 @@ repack.usedeltabaseoffset::
 	"false" and repack. Access from old git versions over the
 	native protocol are unaffected by this option.
 
+rerere.autoupdate::
+	When set to true, `git-rerere` updates the index with the
+	resulting contents after it cleanly resolves conflicts using
+	previously recorded resolution.  Defaults to false.
+
+rerere.enabled::
+	Activate recording of resolved conflicts, so that identical
+	conflict hunks can be resolved automatically, should they
+	be encountered again.  linkgit:git-rerere[1] command is by
+	default enabled if you create `rr-cache` directory under
+	`$GIT_DIR`, but can be disabled by setting this option to false.
+
 showbranch.default::
 	The default set of branches for linkgit:git-show-branch[1].
 	See linkgit:git-show-branch[1].
@@ -1164,6 +1203,11 @@ tar.umask::
 	archiving user's umask will be used instead.  See umask(2) and
 	linkgit:git-archive[1].
 
+transfer.unpackLimit::
+	When `fetch.unpackLimit` or `receive.unpackLimit` are
+	not set, the value of this variable is used instead.
+	The default value is 100.
+
 url.<base>.insteadOf::
 	Any URL that starts with this value will be rewritten to
 	start, instead, with <base>. In cases where some site serves a
@@ -1192,50 +1236,6 @@ user.signingkey::
 	unchanged to gpg's --local-user parameter, so you may specify a key
 	using any method that gpg supports.
 
-imap::
-	The configuration variables in the 'imap' section are described
-	in linkgit:git-imap-send[1].
-
-receive.fsckObjects::
-	If it is set to true, git-receive-pack will check all received
-	objects. It will abort in the case of a malformed object or a
-	broken link. The result of an abort are only dangling objects.
-	Defaults to false.
-
-receive.unpackLimit::
-	If the number of objects received in a push is below this
-	limit then the objects will be unpacked into loose object
-	files. However if the number of received objects equals or
-	exceeds this limit then the received pack will be stored as
-	a pack, after adding any missing delta bases.  Storing the
-	pack from a push can make the push operation complete faster,
-	especially on slow filesystems.  If not set, the value of
-	`transfer.unpackLimit` is used instead.
-
-receive.denyDeletes::
-	If set to true, git-receive-pack will deny a ref update that deletes
-	the ref. Use this to prevent such a ref deletion via a push.
-
-receive.denyNonFastForwards::
-	If set to true, git-receive-pack will deny a ref update which is
-	not a fast forward. Use this to prevent such an update via a push,
-	even if that push is forced. This configuration variable is
-	set when initializing a shared repository.
-
-receive.denyCurrentBranch::
-	If set to true or "refuse", receive-pack will deny a ref update
-	to the currently checked out branch of a non-bare repository.
-	Such a push is potentially dangerous because it brings the HEAD
-	out of sync with the index and working tree. If set to "warn",
-	print a warning of such a push to stderr, but allow the push to
-	proceed. If set to false or "ignore", allow such pushes with no
-	message. Defaults to "warn".
-
-transfer.unpackLimit::
-	When `fetch.unpackLimit` or `receive.unpackLimit` are
-	not set, the value of this variable is used instead.
-	The default value is 100.
-
 web.browser::
 	Specify a web browser that may be used by some commands.
 	Currently only linkgit:git-instaweb[1] and linkgit:git-help[1]
-- 
1.6.0.2.593.g91df

^ permalink raw reply related

* [PATCH] git checkout: don't warn about unborn branch if -f is already passed
From: Matt McCutchen @ 2008-11-24  6:55 UTC (permalink / raw)
  To: git

I think it's unnecessary to warn that the checkout has been forced due to an
unborn current branch if -f has been explicitly passed.  For one project, I am
using git-new-workdir to create workdirs from a bare repository whose HEAD is
set to an unborn branch, and this warning started to irritate me.

Signed-off-by: Matt McCutchen <matt@mattmccutchen.net>
---
 builtin-checkout.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/builtin-checkout.c b/builtin-checkout.c
index 464fd25..7f3bd7b 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -553,7 +553,7 @@ static int switch_branches(struct checkout_opts *opts, struct branch_info *new)
 	if (!opts->quiet && !old.path && old.commit && new->commit != old.commit)
 		describe_detached_head("Previous HEAD position was", old.commit);
 
-	if (!old.commit) {
+	if (!old.commit && !opts->force) {
 		if (!opts->quiet) {
 			fprintf(stderr, "warning: You appear to be on a branch yet to be born.\n");
 			fprintf(stderr, "warning: Forcing checkout of %s.\n", new->name);
-- 
1.6.0.2.593.g91df

^ permalink raw reply related

* Re: [RFC PATCH 0/4] Teach git fetch to verify signed tags automatically
From: Junio C Hamano @ 2008-11-24  5:30 UTC (permalink / raw)
  To: Deskin Miller; +Cc: git
In-Reply-To: <7v4p1xohbw.fsf@gitster.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> Deskin Miller <deskinm@umich.edu> writes:
>
>> It struck me a while back when I fetched a new tagged release from git.git that
>> if I wanted to verify the tag's signature, I'd have to issue another command to
>> do so.  Shouldn't git be able to do that for me automatically, when it fetches
>> signed tags?  Now it does.  Also, 'git remote update' gets this for free.
>
> I think this should be done inside your own hook.  Not interested at all
> in a solution to touch builtin-fetch.c, unless if the patch is about
> adding a new hook so that people with other needs can use it as well.

... or a much stronger case can be made why this shouldn't be done in a
hook.

I realize "not interested at all" was a bit too strong, so I am trying to
rephrase it here.  The cycle that begins with an RFC that leads to
discussion and review is about clarifying the rationale and design
incrementally, so please do not get offended by my no, and sorry for using
unnecessarily strong wording.

What I meant was more like "The justification as given in the message does
not interest me in the patch at all as it stands.  I do not understand why
this has to be done as a patch to git-fetch itself, not in a hook script,
or why doing it inside git-fetch is a better approach than doing it in a
hook (if there already is a hook mechanism to do this)".

^ permalink raw reply

* Re: [RFC PATCH 0/4] Teach git fetch to verify signed tags automatically
From: Junio C Hamano @ 2008-11-24  4:53 UTC (permalink / raw)
  To: Deskin Miller; +Cc: git
In-Reply-To: <1227497000-8684-1-git-send-email-deskinm@umich.edu>

Deskin Miller <deskinm@umich.edu> writes:

> It struck me a while back when I fetched a new tagged release from git.git that
> if I wanted to verify the tag's signature, I'd have to issue another command to
> do so.  Shouldn't git be able to do that for me automatically, when it fetches
> signed tags?  Now it does.  Also, 'git remote update' gets this for free.

I think this should be done inside your own hook.  Not interested at all
in a solution to touch builtin-fetch.c, unless if the patch is about
adding a new hook so that people with other needs can use it as well.

^ permalink raw reply

* Re: [PATCH 1/4] builtin-clone: fix a memory leak in cmd_clone()
From: Junio C Hamano @ 2008-11-24  3:51 UTC (permalink / raw)
  To: Miklos Vajna; +Cc: git
In-Reply-To: <6bd31bceb3840f14d747972b4858e1c5b215744d.1227227976.git.vmiklos@frugalware.org>

Miklos Vajna <vmiklos@frugalware.org> writes:

> Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
> ---
>  builtin-clone.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/builtin-clone.c b/builtin-clone.c
> index 8e1a1d3..da21cab 100644
> --- a/builtin-clone.c
> +++ b/builtin-clone.c
> @@ -516,6 +516,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
>  		refs = transport_get_remote_refs(transport);
>  		transport_fetch_refs(transport, refs);
>  	}
> +	free(dir);
>  
>  	clear_extra_refs();

Can't this be done much earlier?  This variable can potentially be
assigned to work_tree, but after we set up the atexit handler neither dir
nor work_tree are not used (it is a bit hard to see as this function
itself is a bit too big to be maintainable).

^ permalink raw reply

* [RFC PATCH 4/4] Make git fetch verify signed tags
From: Deskin Miller @ 2008-11-24  3:23 UTC (permalink / raw)
  To: git; +Cc: Deskin Miller
In-Reply-To: <1227497000-8684-4-git-send-email-deskinm@umich.edu>

When git fetch downloads signed tag objects, make it verify them right
then.  This extends the output summary of fetch to include "(good
signature)" for valid tags and "(BAD SIGNATURE)" for invalid tags.  If
the user does not have the correct key in the gpg keyring, gpg returns
2, verify_tag_sha1 returns -2 and nothing additional is output about
the tag's validity.

Alternate fetch method 'git remote update' gets this check as well due
to the use of the fetch routines.

Signed-off-by: Deskin Miller <deskinm@umich.edu>
---
 builtin-fetch.c |   25 ++++++++++++++++++-------
 t/t7004-tag.sh  |   37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 7 deletions(-)

diff --git a/builtin-fetch.c b/builtin-fetch.c
index f151cfa..f7a50b7 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -10,6 +10,7 @@
 #include "transport.h"
 #include "run-command.h"
 #include "parse-options.h"
+#include "verify-tag.h"
 
 static const char * const builtin_fetch_usage[] = {
 	"git fetch [options] [<repository> <refspec>...]",
@@ -233,11 +234,16 @@ static int update_local_ref(struct ref *ref,
 
 	if (!is_null_sha1(ref->old_sha1) &&
 	    !prefixcmp(ref->name, "refs/tags/")) {
-		int r;
+		int r, v;
 		r = s_update_ref("updating tag", ref, 0);
-		sprintf(display, "%c %-*s %-*s -> %s%s", r ? '!' : '-',
+		if (type == OBJ_TAG)
+			v = verify_tag_sha1(ref->new_sha1, -1);
+		sprintf(display, "%c %-*s %-*s -> %s%s", r ? '!' :
+			(type == OBJ_TAG ? (v == -1 ? '!' : '-') : '-'),
 			SUMMARY_WIDTH, "[tag update]", REFCOL_WIDTH, remote,
-			pretty_ref, r ? "  (unable to update local ref)" : "");
+			pretty_ref, r ? "  (unable to update local ref)" :
+			(type == OBJ_TAG ? (v == 0 ? " (good signature)" :
+			(v == -1 ? " (BAD SIGNATURE)" : "")) : ""));
 		return r;
 	}
 
@@ -246,7 +252,7 @@ static int update_local_ref(struct ref *ref,
 	if (!current || !updated) {
 		const char *msg;
 		const char *what;
-		int r;
+		int r, v;
 		if (!strncmp(ref->name, "refs/tags/", 10)) {
 			msg = "storing tag";
 			what = "[new tag]";
@@ -257,9 +263,14 @@ static int update_local_ref(struct ref *ref,
 		}
 
 		r = s_update_ref(msg, ref, 0);
-		sprintf(display, "%c %-*s %-*s -> %s%s", r ? '!' : '*',
-			SUMMARY_WIDTH, what, REFCOL_WIDTH, remote, pretty_ref,
-			r ? "  (unable to update local ref)" : "");
+		if (type == OBJ_TAG)
+			v = verify_tag_sha1(ref->new_sha1, -1);
+		sprintf(display, "%c %-*s %-*s -> %s%s", r ? '!' :
+			(type == OBJ_TAG ? (v == -1 ? '!' : '*') : '*'),
+			SUMMARY_WIDTH, what, REFCOL_WIDTH, remote,
+			pretty_ref, r ? "  (unable to update local ref)" :
+			(type == OBJ_TAG ? (v == 0 ? " (good signature)" :
+			(v == -1 ? " (BAD SIGNATURE)" : "")) : ""));
 		return r;
 	}
 
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index f377fea..00327cc 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1037,6 +1037,43 @@ test_expect_success \
 	'test_must_fail git tag -s -m tail tag-gpg-failure'
 git config --unset user.signingkey
 
+git tag -s -m 'good tag' good-tag HEAD
+bad=$(git cat-file tag good-tag | sed -e 's/good-tag/bad-tag/' | git mktag)
+git tag bad-tag $bad
+head=$(git rev-parse HEAD)
+nonkey=$(cat <<EOF | git mktag
+object $head
+type commit
+tag v1.6.0.4
+tagger Junio C Hamano <gitster@pobox.com> 1226208581 -0800
+
+GIT 1.6.0.4
+-----BEGIN PGP SIGNATURE-----
+Version: GnuPG v1.4.9 (GNU/Linux)
+
+iEYEABECAAYFAkkWdUUACgkQwMbZpPMRm5rSmwCfWu+K/hXyLUnEWoOMYy1eKuMK
+KcoAnjB2qir794ibWPy6cn11uUbk7AlC
+=eaFZ
+-----END PGP SIGNATURE-----
+EOF
+)
+git tag nonkey-tag $nonkey
+
+echo 'bad-tag (BAD SIGNATURE)' > expect
+echo 'good-tag (good signature)' >> expect
+echo 'nonkey-tag' >> expect
+
+test_expect_success \
+	'git fetch verifies tags' \
+	'mkdir clone &&
+	(cd clone &&
+	git init &&
+	git remote add origin file://"$(cd .. && pwd)" &&
+	git fetch origin 2>../actual) &&
+	sed -i -ne "/ \(bad\|good\|nonkey\)-tag/s/^.*->[^a-z]*//p" actual &&
+	test_cmp expect actual
+	'
+
 # try to verify without gpg:
 
 rm -rf gpghome
-- 
1.6.0.4.770.ga8394

^ 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