Git development
 help / color / mirror / Atom feed
* Re: Something weird is happening...
From: Junio C Hamano @ 2009-02-01  1:31 UTC (permalink / raw)
  To: Sverre Rabbelier; +Cc: H. Peter Anvin, Git Mailing List, Ingo Molnar
In-Reply-To: <bd6139dc0901290545s1a5ba310u549338e823762ae1@mail.gmail.com>

Sverre Rabbelier <srabbelier@gmail.com> writes:

> On Thu, Jan 29, 2009 at 07:56, Junio C Hamano <gitster@pobox.com> wrote:
>>> Okay, what is going on here?
>
> Is this perhaps related to the problem the github guys were having,
> its sounds similar, problem not showing up until after upgrade,
> missing objects, no?

I do not think so.  The push problem does not lead to any repository
corruption, and does not have anything to do with the fsck misreporting a
blob that exists in an alternate object store as missing, either.

.

^ permalink raw reply

* Re: 'git clone' doesn't use alternates automatically?
From: James Pickens @ 2009-02-01  1:32 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git ML, Jeff King
In-Reply-To: <7vtz7f2cy6.fsf@gitster.siamese.dyndns.org>

On Sat, Jan 31, 2009, Junio C Hamano <gitster@pobox.com> wrote:
> When you say "git clone" without -s, you are saying "I do not want to use
> the repository I am cloning from as my alternate, because I do not know if
> will stay stable.  I do not trust it."

Yes, I'm aware of the caveats of -s.  I was talking about what
happens when I *don't* use -s.

> Now, if you are cloning from a local filesystem, by default we will copy
> the objects/info/alternates from the source repository to the new one.  It

Crap, I didn't realize the alternates were only copied when you
clone from the local filesystem.  I wanted to use this when
cloning over ssh from site A to site B, to automatically add a
mirror at site B as an alternate.  Sounds like I have no choice
but to use --reference for that.

> I suspect you are trying to improve the other extreme end: trusting all
> the other repositories involved in the cloning process a lot more than the
> code currently does.

What I was suggesting did not involve trusting anything any more
than the current code does.  It just meant taking immediate
advantage of the trust that was already there.

Thanks for your input,
James

^ permalink raw reply

* Re: [PATCHv5 0/3] gitweb: make static files accessible with PATH_INFO
From: Junio C Hamano @ 2009-02-01  1:32 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Giuseppe Bilotta, git
In-Reply-To: <200901310304.51205.jnareb@gmail.com>

Jakub Narebski <jnareb@gmail.com> writes:

> Giuseppe Bilotta wrote:
>
>> This patchset includes
>> 
>> 1. the single patch to make static files accessible with PATH_INFO, with
>>    the same quoting standard as the nearby code;
>> 2. an additional patch that extends the README with some examples on how
>>    to configure Apache to use gitweb with PATH_INFO enabled by default
>>    (and gitweb as directory index);
>> 3. the repeatedly recommended comment fix to align comments to code in the
>>    git_header_html() sub.
>
> I like it. Ack for the whole series.

Thanks, both.  Applied.

^ permalink raw reply

* Re: [PATCH 2/2] mergetool: fix running mergetool in sub-directories
From: Junio C Hamano @ 2009-02-01  1:32 UTC (permalink / raw)
  To: Charles Bailey; +Cc: git, Jonas Flodén
In-Reply-To: <1233357611-31820-2-git-send-email-charles@hashpling.org>

Charles Bailey <charles@hashpling.org> writes:

> diff --git a/git-mergetool.sh b/git-mergetool.sh
> index aefdca7..87fa88a 100755
> --- a/git-mergetool.sh
> +++ b/git-mergetool.sh
> @@ -13,7 +13,6 @@ SUBDIRECTORY_OK=Yes
>  OPTIONS_SPEC=
>  . git-sh-setup
>  require_work_tree
> -prefix=$(git rev-parse --show-prefix)
>  
>  # Returns true if the mode reflects a symlink
>  is_symlink () {
> @@ -131,7 +130,7 @@ checkout_staged_file () {
>      tmpfile=$(expr "$(git checkout-index --temp --stage="$1" "$2")" : '\([^	]*\)	')
>  
>      if test $? -eq 0 -a -n "$tmpfile" ; then
> -	mv -- "$tmpfile" "$3"
> +	mv -- "$(git rev-parse --show-cdup)$tmpfile" "$3"
>      fi
>  }

Looking at the above change together with this disabled patch in the previous,

> +# We can't merge files from parent directories when running mergetool
> +# from a subdir. Is this a bug?
> +#
> +#test_expect_failure 'mergetool in subdir' '
> +#    cd subdir && (
> +#    ( yes "" | git mergetool ../file1 >/dev/null 2>&1 ) &&
> +#    ( yes "" | git mergetool ../file2 >/dev/null 2>&1 ) &&
> +#    test "$(cat ../file1)" = "master updated" &&
> +#    test "$(cat ../file2)" = "master new" &&
> +#    git commit -m "branch1 resolved with mergetool - subdir" )
> +#'

I wonder if it would be cleaner to keep $prefix and instead cd_to_toplevel
at the beginning.

There are two ways for Porcelain scripts to work from inside a
subdirectory of a project.

 * Stay in the original subdirectory and use show-cdup to convert a path
   that is relative to the repository root to a path relative to your
   current subdirectory.

 * Go up to the root of the work tree, use prefix to convert a path that
   is relative to the original subdirectory to a path relative to the root
   of the work tree.

The former way is probably the clunkier one between the two.  I think the
latter is how most of the Porcelain scripts work when they need to worry
about the Plumbing commands that return/accept repository relative paths.
Also, all the plumbing commands work in the latter way.

^ permalink raw reply

* Re: [PATCH] contrib/difftool: Don't repeat merge tool candidates
From: Junio C Hamano @ 2009-02-01  1:35 UTC (permalink / raw)
  To: David Aguilar; +Cc: git, Markus Heidelberg, Johannes Gilger
In-Reply-To: <1233433676-21944-1-git-send-email-davvid@gmail.com>

David Aguilar <davvid@gmail.com> writes:

> git difftool listed some candidates for mergetools twice, depending on
> the environment.
>
> This slightly changes the behavior when both KDE_FULL_SESSION and
> GNOME_DESKTOP_SESSION_ID are set at the same time; in such a case
> meld is used in favor of kdiff3 (the old code favored kdiff3 in such a
> case), but it should not matter in practice.
>
> Signed-off-by: David Aguilar <davvid@gmail.com>
> ---
>
> Hi Junio
>
> This is based on top of Markus's kompare patch:
> 	"contrib/difftool: add support for Kompare"
> 	http://article.gmane.org/gmane.comp.version-control.git/107883

Ok, so I'll add "Acked-by: David Aguilar <davvid@gmail.com>" to Markus's
patch and apply it.

Thanks

^ permalink raw reply

* Re: 'git clone' doesn't use alternates automatically?
From: Junio C Hamano @ 2009-02-01  1:38 UTC (permalink / raw)
  To: James Pickens; +Cc: Git ML, Jeff King
In-Reply-To: <885649360901311732t6e4f8651p6c3955f018b9a36c@mail.gmail.com>

James Pickens <jepicken@gmail.com> writes:

> ...  Sounds like I have no choice
> but to use --reference for that.

As --reference was invented exactly for that use case, I think using it to
instruct where to borrow your object from is a very sensible thing to do.

^ permalink raw reply

* Re: [PATCH] Switch receive.denyCurrentBranch to "refuse"
From: Junio C Hamano @ 2009-02-01  1:39 UTC (permalink / raw)
  To: Nanako Shiraishi; +Cc: Johannes Schindelin, Jay Soffian, git
In-Reply-To: <7vy6wr0wvi.fsf@gitster.siamese.dyndns.org>

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

> I haven't manged to convince myself about the "git init" change (I have
> the code and also I've looked at the extent of damage the change causes to
> the existing test suite),...

And here is such a patch.

-- >8 --
Subject: [PATCH] Set receive.denyCurrentBranch to true in a new non-bare repository

This prepares new people to get used to the default planned for 1.7.0;
necessary adjustments are done to many tests, as they all assumed the
traditional "only warn but allow updating" semantics.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-init-db.c           |    2 ++
 t/t5400-send-pack.sh        |    2 ++
 t/t5401-update-hooks.sh     |    1 +
 t/t5405-send-pack-rewind.sh |    1 +
 t/t5516-fetch-push.sh       |    1 +
 t/t5517-push-mirror.sh      |    3 ++-
 t/t5521-pull-symlink.sh     |   20 +++++++++++++-------
 t/t5701-clone-local.sh      |    4 +++-
 8 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/builtin-init-db.c b/builtin-init-db.c
index ee3911f..26c10cc 100644
--- a/builtin-init-db.c
+++ b/builtin-init-db.c
@@ -250,6 +250,8 @@ static int create_default_files(const char *template_path)
 		    strcmp(git_dir + strlen(work_tree), "/.git")) {
 			git_config_set("core.worktree", work_tree);
 		}
+		if (!reinit)
+			git_config_set("receive.denyCurrentBranch", "refuse");
 	}
 
 	if (!reinit) {
diff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh
index b21317d..5c9c277 100755
--- a/t/t5400-send-pack.sh
+++ b/t/t5400-send-pack.sh
@@ -33,6 +33,7 @@ test_expect_success setup '
 	git update-ref HEAD "$commit" &&
 	git clone ./. victim &&
 	cd victim &&
+	git config receive.denyCurrentBranch warn &&
 	git log &&
 	cd .. &&
 	git update-ref HEAD "$zero" &&
@@ -138,6 +139,7 @@ rewound_push_setup() {
 	rm -rf parent child &&
 	mkdir parent && cd parent &&
 	git init && echo one >file && git add file && git commit -m one &&
+	git config receive.denyCurrentBranch warn &&
 	echo two >file && git commit -a -m two &&
 	cd .. &&
 	git clone parent child && cd child && git reset --hard HEAD^
diff --git a/t/t5401-update-hooks.sh b/t/t5401-update-hooks.sh
index 64f66c9..325714e 100755
--- a/t/t5401-update-hooks.sh
+++ b/t/t5401-update-hooks.sh
@@ -18,6 +18,7 @@ test_expect_success setup '
 	git update-ref refs/heads/master $commit0 &&
 	git update-ref refs/heads/tofail $commit1 &&
 	git clone ./. victim &&
+	GIT_DIR=victim/.git git config receive.denyCurrentBranch warn &&
 	GIT_DIR=victim/.git git update-ref refs/heads/tofail $commit1 &&
 	git update-ref refs/heads/master $commit1 &&
 	git update-ref refs/heads/tofail $commit0
diff --git a/t/t5405-send-pack-rewind.sh b/t/t5405-send-pack-rewind.sh
index cb9aacc..4bda18a 100755
--- a/t/t5405-send-pack-rewind.sh
+++ b/t/t5405-send-pack-rewind.sh
@@ -8,6 +8,7 @@ test_expect_success setup '
 
 	>file1 && git add file1 && test_tick &&
 	git commit -m Initial &&
+	git config receive.denyCurrentBranch warn &&
 
 	mkdir another && (
 		cd another &&
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 89649e7..a67ebd0 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -12,6 +12,7 @@ mk_empty () {
 	(
 		cd testrepo &&
 		git init &&
+		git config receive.denyCurrentBranch warn &&
 		mv .git/hooks .git/hooks-disabled
 	)
 }
diff --git a/t/t5517-push-mirror.sh b/t/t5517-push-mirror.sh
index ea49ded..e2ad260 100755
--- a/t/t5517-push-mirror.sh
+++ b/t/t5517-push-mirror.sh
@@ -19,7 +19,8 @@ mk_repo_pair () {
 	mkdir mirror &&
 	(
 		cd mirror &&
-		git init
+		git init &&
+		git config receive.denyCurrentBranch warn
 	) &&
 	mkdir master &&
 	(
diff --git a/t/t5521-pull-symlink.sh b/t/t5521-pull-symlink.sh
index 5672b51..66b5ac1 100755
--- a/t/t5521-pull-symlink.sh
+++ b/t/t5521-pull-symlink.sh
@@ -14,13 +14,19 @@ test_description='pulling from symlinked subdir'
 #
 # The working directory is subdir-link.
 
-mkdir subdir
-echo file >subdir/file
-git add subdir/file
-git commit -q -m file
-git clone -q . clone-repo
-ln -s clone-repo/subdir/ subdir-link
-
+test_expect_success setup '
+	mkdir subdir &&
+	echo file >subdir/file &&
+	git add subdir/file &&
+	git commit -q -m file &&
+	git clone -q . clone-repo &&
+	ln -s clone-repo/subdir/ subdir-link &&
+	(
+		cd clone-repo &&
+		git config receive.denyCurrentBranch warn
+	) &&
+	git config receive.denyCurrentBranch warn
+'
 
 # Demonstrate that things work if we just avoid the symlink
 #
diff --git a/t/t5701-clone-local.sh b/t/t5701-clone-local.sh
index 3559d17..10accc2 100755
--- a/t/t5701-clone-local.sh
+++ b/t/t5701-clone-local.sh
@@ -119,7 +119,9 @@ test_expect_success 'bundle clone with nonexistent HEAD' '
 test_expect_success 'clone empty repository' '
 	cd "$D" &&
 	mkdir empty &&
-	(cd empty && git init) &&
+	(cd empty &&
+	 git init &&
+	 git config receive.denyCurrentBranch warn) &&
 	git clone empty empty-clone &&
 	test_tick &&
 	(cd empty-clone
-- 
1.6.1.2.312.g5be3c

^ permalink raw reply related

* Re: [PATCH v2 3/5] chain kill signals for cleanup functions
From: Junio C Hamano @ 2009-02-01  1:58 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20090131065049.GA3130@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Fri, Jan 30, 2009 at 08:44:20PM -0500, Jeff King wrote:
>
>> > Will do, but I've been sick today, haven't caught up with the list
>> > traffic, and I do not think I'll be reading my mails for the rest of the
>> > day either.  It may take some time for it to appear in the public
>> > repositories.
>> [...]
>> Now I'm off to go make fun of you, buried deep in a thread where you
>> won't see it.
>
> Hmm, reading that again, it sounds mean, and I didn't mean it that way.
> I should have put a ";P" at the end. What I meant was "while the cat is
> away, the mice will play" (i.e., make mischief while our benevolent
> dictator is out of commission).
>
> Hope you feel better soon.

Thanks.  I guess not much happened on the list while I was sick in bed.

I'll try to take a bit more rest and catch up tomorrow.

^ permalink raw reply

* Re: [PATCH] Switch receive.denyCurrentBranch to "refuse"
From: Junio C Hamano @ 2009-02-01  2:06 UTC (permalink / raw)
  To: Nanako Shiraishi; +Cc: Johannes Schindelin, Jay Soffian, git
In-Reply-To: <20090131095622.6117@nanako3.lavabit.com>

Nanako Shiraishi <nanako3@lavabit.com> writes:

> I assume that everybody, including the maintainer who is too heavyweight
> and has too much inertia to accept too sudden a change of the course,
> wants to eventually make the default to deny pushing to the current
> branch. But I think such a change should come at 1.7.0 release at the
> earliest, and a constructive thing to do is to put in a patch to 1.6.2
> that helps the users with the eventual transition.

I am not opposed to eventually change the default to refuse at some point,
but I have to say that now would not be the best time to do so.  Jeff's
986e823 (receive-pack: detect push to current branch of non-bare repo,
2008-11-08) that is v1.6.1-rc1~59^2 was the one we started warning about
this, and we only had one major release since then, and I'd love to see a
solid rc or even the final release by mid February.

By the way, I do not appreciate other people who I have never met
speculate about my body mass very much.  I am on the skinner end of the
spectrum, if you need to know.

^ permalink raw reply

* [PATCH] git-svn: allow disabling expensive broken symlink checks
From: Eric Wong @ 2009-02-01  2:18 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Markus Heidelberg
In-Reply-To: <200901311414.58205.markus.heidelberg@web.de>

Since dbc6c74d0858d77e61e092a48d467e725211f8e9, git-svn has had
an expensive check for broken symlinks that exist in some
repositories.  This leads to a heavy performance hit on
repositories with many empty blobs that are not supposed to be
symlinks.

The workaround is enabled by default; and may be disabled via:

  git config svn.brokenSymlinkWorkaround false

Reported by Markus Heidelberg.

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---

  Markus Heidelberg <markus.heidelberg@web.de> wrote:
  > Hi,
  > 
  > since several days "git svn fetch" didn't seem to work any more. I
  > bisected it down to
  > 
  >     commit dbc6c74d0858d77e61e092a48d467e725211f8e9
  >     git-svn: handle empty files marked as symlinks in SVN
  >     2009-01-11
  > 
  > In the new function _mark_empty_symlinks() there is a loop that takes
  > about 36 seconds for me. That means each svn revision takes 36+x seconds
  > for downloading. So it still works, but I aborted it before waiting so
  > much time, so I thought, it didn't work any more.
  > 
  > The loop loops over each blob ("git ls-tree -r git-svn | wc -l" times).
  > The project I'm using git-svn with is Buildroot and it has currently
  > 3074 blobs in the tree. Printing a loop counter every time the loop is
  > executed, I can see that it mostly goes really fast, but there are
  > files, where it needs much time then.
  > 
  > Could there be a way to avoid this time consuming step?
  > 
  > Markus

 Documentation/git-svn.txt        |    8 ++++++++
 git-svn.perl                     |   20 ++++++++++++++++++++
 t/t9131-git-svn-empty-symlink.sh |   10 ++++++++++
 3 files changed, 38 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index 7b654f7..3d45654 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -499,6 +499,14 @@ svn-remote.<name>.rewriteRoot::
 	the repository with a public http:// or svn:// URL in the
 	metadata so users of it will see the public URL.
 
+svn.brokenSymlinkWorkaround::
+This disables potentially expensive checks to workaround broken symlinks
+checked into SVN by broken clients.  Set this option to "false" if you
+track a SVN repository with many empty blobs that are not symlinks.
+This option may be changed while "git-svn" is running and take effect on
+the next revision fetched.  If unset, git-svn assumes this option to be
+"true".
+
 --
 
 Since the noMetadata, rewriteRoot, useSvnsyncProps and useSvmProps
diff --git a/git-svn.perl b/git-svn.perl
index 79888a0..bebcbde 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -3271,10 +3271,18 @@ sub new {
 # do_{switch,update}
 sub _mark_empty_symlinks {
 	my ($git_svn) = @_;
+	my $bool = Git::config_bool('svn.brokenSymlinkWorkaround');
+	return {} if (defined($bool) && ! $bool);
+
 	my %ret;
 	my ($rev, $cmt) = $git_svn->last_rev_commit;
 	return {} unless ($rev && $cmt);
 
+	# allow the warning to be printed for each revision we fetch to
+	# ensure the user sees it.  The user can also disable the workaround
+	# on the repository even while git svn is running and the next
+	# revision fetched will skip this expensive function.
+	my $printed_warning;
 	chomp(my $empty_blob = `git hash-object -t blob --stdin < /dev/null`);
 	my ($ls, $ctx) = command_output_pipe(qw/ls-tree -r -z/, $cmt);
 	local $/ = "\0";
@@ -3283,6 +3291,18 @@ sub _mark_empty_symlinks {
 	while (<$ls>) {
 		chomp;
 		s/\A100644 blob $empty_blob\t//o or next;
+		unless ($printed_warning) {
+			print STDERR "Scanning for empty symlinks, ",
+			             "this may take a while if you have ",
+				     "many empty files\n",
+				     "You may disable this with `",
+				     "git config svn.brokenSymlinkWorkaround ",
+				     "false'.\n",
+				     "This may be done in a different ",
+				     "terminal without restarting ",
+				     "git svn\n";
+			$printed_warning = 1;
+		}
 		my $path = $_;
 		my (undef, $props) =
 		               $git_svn->ra->get_file($pfx.$path, $rev, undef);
diff --git a/t/t9131-git-svn-empty-symlink.sh b/t/t9131-git-svn-empty-symlink.sh
index 704a4f8..c3c3e42 100755
--- a/t/t9131-git-svn-empty-symlink.sh
+++ b/t/t9131-git-svn-empty-symlink.sh
@@ -87,4 +87,14 @@ test_expect_success '"bar" is an empty file' 'test -f x/bar && ! test -s x/bar'
 test_expect_success 'get "bar" => symlink fix from svn' \
 		'(cd x && git svn rebase)'
 test_expect_success '"bar" becomes a symlink' 'test -L x/bar'
+
+
+test_expect_success 'clone using git svn' 'git svn clone -r1 "$svnrepo" y'
+test_expect_success 'disable broken symlink workaround' \
+  '(cd y && git config svn.brokenSymlinkWorkaround false)'
+test_expect_success '"bar" is an empty file' 'test -f y/bar && ! test -s y/bar'
+test_expect_success 'get "bar" => symlink fix from svn' \
+		'(cd y && git svn rebase)'
+test_expect_success '"bar" becomes a symlink' '! test -L y/bar'
+
 test_done
-- 
Eric Wong

^ permalink raw reply related

* Re: [RFC PATCH 3/3] Support fetching from foreign VCSes
From: Daniel Barkalow @ 2009-02-01  2:35 UTC (permalink / raw)
  To: Johan Herland; +Cc: git, Junio C Hamano
In-Reply-To: <200902010320.28128.johan@herland.net>

On Sun, 1 Feb 2009, Johan Herland wrote:

> On Sunday 11 January 2009, Daniel Barkalow wrote:
> > This supports a useful subset of the usual fetch logic, mostly in the
> > config file.
> 
> Hi,
> 
> I love the idea of this patch series, and have started working on a CVS 
> backend for this. I have a question though...
> 
> > Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
> > ---
> >  builtin-fetch.c |  135
> > +++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed,
> > 132 insertions(+), 3 deletions(-)
> >
> > diff --git a/builtin-fetch.c b/builtin-fetch.c
> > index 7b46f8f..14e037e 100644
> > --- a/builtin-fetch.c
> > +++ b/builtin-fetch.c
> > @@ -614,6 +614,136 @@ static void set_option(const char *name, const char
> > *value) name, transport->url);
> >  }
> >
> > +static struct ref *list_foreign(struct remote *remote)
> > +{
> 
> [...]
> 
> > +}
> > +
> > +static int import_foreign(struct remote *remote, struct ref *refs)
> > +{
> 
> [...]
> 
> > +}
> > +
> > +static int fetch_foreign(struct remote *remote)
> > +{
> > +	struct ref *remote_refs = list_foreign(remote);
> 
> We retrieve a list of all refs available at the given remote...
> 
> > +	struct ref *ref_map = NULL;
> > +	struct ref *rm;
> > +	struct ref **tail = &ref_map;
> > +	struct branch *branch;
> > +	int i;
> > +
> > +	int exit_code = import_foreign(remote, remote_refs);
> 
> ...and then we start fetching _all_ the refs returned by list_foreign().
> 
> When I call "git fetch <vcs-remote> <ref>", I expect _only_ <ref> to be 
> fetched from the remote, but it seems the above code doesn't even concern 
> itself with the ref(s) specified on the "git fetch" command-line
> 
> I'm not even sure why list_foreign() should be called in this case (except, 
> maybe, to verify the existence of <ref> before attempting to fetch it).
> 
> AFAICS list_foreign() is only needed by "git fetch" if <ref> contains a 
> wildcard (like the default refspec: +refs/heads/*:refs/remotes/<remote>/*).
> 
> ...or have I misunderstood something fundamental about how this is going to 
> work?

Nope, I was just a bit lazy; I just didn't implement looking at the 
command line (I just use "git fetch" when I'm using this), and my goal is 
to use wildcards (although I haven't gotten wildcards implemented that can 
match the branch names than come naturally out of the particular foreign 
system I'm using).

I was also staying as close as possible to the git design, where it always 
fetches the complete list and then looks at what matches. It would 
probably be a good optimization to only ask for the complete list if it's 
needed to determine what specific refs to ask for.

Incidently, you probably want this patch so it doesn't crash if you don't 
have a merge config:

commit 71266fe87dae351c0f77b0ecf8802dab2909db05
Author: Daniel Barkalow <barkalow@iabervon.org>
Date:   Fri Jan 30 20:30:10 2009 -0500

    Fix bug with not having merge config
    
    Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>

diff --git a/builtin-fetch.c b/builtin-fetch.c
index 14e037e..63e0637 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -710,6 +710,7 @@ static int fetch_foreign(struct remote *remote)
 	struct ref **tail = &ref_map;
 	struct branch *branch;
 	int i;
+	int has_merge;
 
 	int exit_code = import_foreign(remote, remote_refs);
 	if (exit_code)
@@ -729,9 +730,11 @@ static int fetch_foreign(struct remote *remote)
 
 	branch = branch_get(NULL);
 
+	has_merge = branch_has_merge_config(branch);
+
 	for (i = 0; i < remote->fetch_refspec_nr; i++) {
 		get_fetch_map(remote_refs, &remote->fetch[i], &tail, 0);
-		if (!strcmp(branch->remote_name, remote->name))
+		if (has_merge && !strcmp(branch->remote_name, remote->name))
 			add_merge_config(&ref_map, remote_refs, branch, &tail);
 	}
 

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply related

* [PATCH] builtin-config: add --exec-editor for use in scripts
From: Eric Wong @ 2009-02-01  2:53 UTC (permalink / raw)
  To: git

This exposes the launch_editor() library function for use by
various scripting languages.  This allows the ensure consistent
handling of GIT_EDITOR/VISUAL/EDITOR environment variables as
well as the handling of special characters such as spaces in the
various environment variables.

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---

 I'm not sure if git-config is the best place to stick it.  I plan to
 start using this in git-svn but I don't want to implement Git::Editor
 in Perl and have to keep track of editor.c.  Of course this also makes
 the logic/rules used in libgit usable to any other scripting language
 capable of launching other programs.

 I'll probably also do something like this with setup_pager(), too...

 Documentation/git-config.txt |   10 ++++++++++
 builtin-config.c             |    6 +++++-
 2 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 19a8917..2dd9f1c 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -22,6 +22,7 @@ SYNOPSIS
 'git config' [<file-option>] [-z|--null] -l | --list
 'git config' [<file-option>] --get-color name [default]
 'git config' [<file-option>] --get-colorbool name [stdout-is-tty]
+'git config' [<file-option>] --exec-editor filename
 
 DESCRIPTION
 -----------
@@ -157,6 +158,15 @@ See also <<FILES>>.
 	output.  The optional `default` parameter is used instead, if
 	there is no color configured for `name`.
 
+--exec-editor filename::
+
+	Executes the editor on filename as configured by the
+	GIT_EDITOR/VISUAL/EDITOR environment variables (in that order).
+	This exposes the launch_editor() library function for use with
+	scripting languages that may not have C bindings.  The
+	launch_editor() function takes into account special characters
+	such as spaces in the editor argument.
+
 [[FILES]]
 FILES
 -----
diff --git a/builtin-config.c b/builtin-config.c
index f710162..1c805da 100644
--- a/builtin-config.c
+++ b/builtin-config.c
@@ -3,7 +3,7 @@
 #include "color.h"
 
 static const char git_config_set_usage[] =
-"git config [ --global | --system | [ -f | --file ] config-file ] [ --bool | --int | --bool-or-int ] [ -z | --null ] [--get | --get-all | --get-regexp | --replace-all | --add | --unset | --unset-all] name [value [value_regex]] | --rename-section old_name new_name | --remove-section name | --list | --get-color var [default] | --get-colorbool name [stdout-is-tty]";
+"git config [ --global | --system | [ -f | --file ] config-file ] [ --bool | --int | --bool-or-int ] [ -z | --null ] [--get | --get-all | --get-regexp | --replace-all | --add | --unset | --unset-all] name [value [value_regex]] | --rename-section old_name new_name | --remove-section name | --list | --get-color var [default] | --get-colorbool name [stdout-is-tty] | --exec-editor filename";
 
 static char *key;
 static regex_t *key_regexp;
@@ -362,6 +362,10 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 			return get_color(argc-2, argv+2);
 		} else if (!strcmp(argv[1], "--get-colorbool")) {
 			return get_colorbool(argc-2, argv+2);
+		} else if (!strcmp(argv[1], "--exec-editor")) {
+			if (argc != 3)
+				usage(git_config_set_usage);
+			return launch_editor(argv[2], NULL, NULL);
 		} else
 			break;
 		argc--;
-- 
Eric Wong

^ permalink raw reply related

* [PATCH] split notes [was: Re: What's cooking in git.git (Jan 2009, #05; Wed, 21)]
From: Sam Vilain @ 2009-02-01  2:39 UTC (permalink / raw)
  To: Jeff King; +Cc: Johannes Schindelin, Junio C Hamano, git
In-Reply-To: <20090131073640.GF3033@coredump.intra.peff.net>

On Sat, 2009-01-31 at 02:36 -0500, Jeff King wrote:
> Actually, lookup is even easier than that: we iterate through the entire
> tree recursively and add everything to a flat hash. So we really don't
> care there what the layout is like (just take the first 40 characters of
> any directory name as a hash).

Sure, but if you do want to scale to a hundred thousand notes, then I
think it would pay to have a plan for making it lazy as required.  ie,
if a run just wants notes for 20 commits and there are 256 sub-trees
then only read 20 of them.  Doesn't matter if it's not implemented
initially of course, so long as the on-disk format is supported by the
tools in the first release they will be backward compatible.  And it's
not that complicated; see attached.

> But it violates the usual git principle of "content has a unique name".
> What happens when I add "a/b" and you add "ab"? A dumb merge will let
> both co-exist, but which one do you return for lookup?

It should only be tools adding to it, the trees shouldn't be modified
directly by users.  In the below patch I make these all get deleted on
'git notes edit'.

Depending on the semantics of the notes, it might not be an error to
have multiple notes for a commit.  In my patch this is "tolerated" to
some extent but not supported by git-notes.sh porcelain yet.

> I agree that there should be multiple note hierarchies, and multiple
> keys within each hierarchy. I have posted some thoughts on that before
> (and you should be able to find them searching for "notes" in the list
> archive), but unfortunately I have not had time to sit down and really
> work out a notes implementation that matches what I posted (which I
> don't think is that far from Dscho's work in next).

I had a brief look and couldn't find it, this was about the best one I
found from you in terms of links to previous discussions;
http://kerneltrap.org/mailarchive/git/2008/12/16/4427794 If there's
another thread you'd like me to read please fish it out and respond!
The more messages we have linking to the previous discussions the
better :).

> And I think what you are proposing (here and in the rest of your
> message) is that certain notes hierarchies may have particular formats
> and semantics. And that sounds reasonable to me.

Yes that was one part of it.  But also make a convention that the
'commits' notes, ie the default ones, an RFC822 message if they begin
with "known" headers.  Then porcelain such as log can inject them into
the fields at the appropriate point.

Anyway, without further ado here's the XX/XXXX split patch.

Subject: [PATCH] git-notes: allow for arbitrary split of entries into sub-trees

It might later turn out for performance reasons that a single tree for
notes will not be sufficient.  While this does not solve the
performance problem as it still loads the entire lot of notes into a
hash at start-up, it does mean that such a change does not have to
worry about backward compatibility with git versions that don't yet
support it.

Signed-off-by: Sam Vilain <sam@vilain.net>
---
 git-notes.sh     |   45 ++++++++++++++++++++++++++++++++++++++-------
 notes.c          |   39 ++++++++++++++++++++++++++++++++-------
 t/t3301-notes.sh |   11 +++++++++++
 3 files changed, 81 insertions(+), 14 deletions(-)

diff --git a/git-notes.sh b/git-notes.sh
index bfdbaa8..e07499f 100755
--- a/git-notes.sh
+++ b/git-notes.sh
@@ -10,15 +10,35 @@ ACTION="$1"; shift
 
 test -z "$GIT_NOTES_REF" && GIT_NOTES_REF="$(git config core.notesref)"
 test -z "$GIT_NOTES_REF" && GIT_NOTES_REF="refs/notes/commits"
+export GIT_NOTES_REF
 
 COMMIT=$(git rev-parse --verify --default HEAD "$@") ||
 die "Invalid commit: $@"
+NOTES_PATH=$COMMIT
+case "$GIT_NOTES_SPLIT" in
+	[1-9]|[1-4][0-9])
+		NOTES_PATH=$( echo $COMMIT | perl -pe 's{^(.{'$GIT_NOTES_SPLIT'})}{$1/}' )
+		;;
+esac
 
 MESSAGE="$GIT_DIR"/new-notes-$COMMIT
 trap '
 	test -f "$MESSAGE" && rm "$MESSAGE"
 ' 0
 
+show_note() {
+	COMMIT=$1
+	NOTE_PATH=$( git ls-tree --name-only -r $GIT_NOTES_REF | perl -nle '
+		$x = $_; s{/}{}g;
+		if (m{'$COMMIT'}) {
+			print $x;
+			exit;
+		}
+	' )
+	[ -n "$NOTE_PATH" ] &&
+		git cat-file blob $GIT_NOTES_REF:$NOTE_PATH
+}
+
 case "$ACTION" in
 edit)
 	GIT_NOTES_REF= git log -1 $COMMIT | sed "s/^/#/" > "$MESSAGE"
@@ -32,7 +52,7 @@ edit)
 	else
 		PARENT="-p $CURRENT_HEAD"
 		git read-tree "$GIT_NOTES_REF" || die "Could not read index"
-		git cat-file blob :$COMMIT >> "$MESSAGE" 2> /dev/null
+		show_note $COMMIT >> "$MESSAGE"
 	fi
 
 	${VISUAL:-${EDITOR:-vi}} "$MESSAGE"
@@ -42,15 +62,26 @@ edit)
 	if [ -s "$MESSAGE" ]; then
 		BLOB=$(git hash-object -w "$MESSAGE") ||
 			die "Could not write into object database"
-		git update-index --add --cacheinfo 0644 $BLOB $COMMIT ||
+		git update-index --add --cacheinfo 0644 $BLOB $NOTES_PATH ||
 			die "Could not write index"
 	else
-		test -z "$CURRENT_HEAD" &&
-			die "Will not initialise with empty tree"
-		git update-index --force-remove $COMMIT ||
-			die "Could not update index"
+		NOTES_PATH=dummy
 	fi
 
+	git ls-files | perl -nle '
+		$x = $_; s{/}{}g;
+		if (m{'$COMMIT'} and $x ne q{'$NOTES_PATH'}) {
+			print $x;
+		}' |
+		while read path
+			do
+				git update-index --force-remove $path ||
+			    		die "Could not update index"
+			done
+	
+	[ -z "$(git ls-files)" -a -z "$CURRENT_HEAD" ] &&
+		die "Will not initialise with empty tree"
+
 	TREE=$(git write-tree) || die "Could not write tree"
 	NEW_HEAD=$(echo Annotate $COMMIT | git commit-tree $TREE $PARENT) ||
 		die "Could not annotate"
@@ -58,7 +89,7 @@ edit)
 		"$GIT_NOTES_REF" $NEW_HEAD $CURRENT_HEAD
 ;;
 show)
-	git show "$GIT_NOTES_REF":$COMMIT
+	show_note $COMMIT
 ;;
 *)
 	usage
diff --git a/notes.c b/notes.c
index bd73784..d763b50 100644
--- a/notes.c
+++ b/notes.c
@@ -70,28 +70,53 @@ static void add_entry(const unsigned char *commit_sha1,
 	hashcpy(hash_map.entries[index].notes_sha1, notes_sha1);
 }
 
-static void initialize_hash_map(const char *notes_ref_name)
+static void initialize_hash_map_recursive(const char *tree_sha1, const char *path)
 {
 	unsigned char sha1[20], commit_sha1[20];
+	unsigned char path_combined[40];
 	unsigned mode;
 	struct tree_desc desc;
 	struct name_entry entry;
+	int length = strlen(path);
+	int length_combined;
 	void *buf;
 
-	if (!notes_ref_name || read_ref(notes_ref_name, commit_sha1) ||
-	    get_tree_entry(commit_sha1, "", sha1, &mode))
+	strcpy(path_combined, path);
+
+	if (get_tree_entry(tree_sha1, "", sha1, &mode))
 		return;
 
 	buf = fill_tree_descriptor(&desc, sha1);
 	if (!buf)
-		die("Could not read %s for notes-index", sha1_to_hex(sha1));
+		die("Could not read %s for notes-index", sha1_to_hex(tree_sha1));
+
+	while (tree_entry(&desc, &entry)) {
+		length_combined = length + strlen(entry.path);
+		if (length_combined >= 40) {
+			strncpy(path_combined + length, entry.path,
+				41 - length);
+			if (!get_sha1(path_combined, commit_sha1))
+				add_entry(commit_sha1, entry.sha1);
+		}
+		else {
+			strcpy(path_combined + length, entry.path);
+			initialize_hash_map_recursive(entry.sha1, path_combined);
+		}
+	}
 
-	while (tree_entry(&desc, &entry))
-		if (!get_sha1(entry.path, commit_sha1))
-			add_entry(commit_sha1, entry.sha1);
 	free(buf);
 }
 
+static void initialize_hash_map(const char *notes_ref_name)
+{
+	unsigned char commit_sha1[20];
+
+	if (!notes_ref_name || read_ref(notes_ref_name, commit_sha1))
+		return;
+
+	initialize_hash_map_recursive( commit_sha1, "" );
+}
+
 static unsigned char *lookup_notes(const unsigned char *commit_sha1)
 {
 	int index;
diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index 9393a25..3734b55 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -92,4 +92,15 @@ test_expect_success 'show multi-line notes' '
 	test_cmp expect-multiline output
 '
 
+test_expect_success 'create split notes tree' '
+	: > a4 &&
+	git add a4 &&
+	test_tick &&
+	git commit -m 4th &&
+	MSG="b4" GIT_NOTES_SPLIT=2 git notes edit &&
+	[ "$(git notes show)" = "b4" ] &&
+	[ -n "$(git ls-tree --name-only -r refs/notes/commits | grep /)" ] &&
+	[ -n "$(git log -1 | grep Notes:)" ]
+'
+
 test_done
-- 
debian.1.5.6.1

^ permalink raw reply related

* [PATCH] split notes [was: Re: What's cooking in git.git (Jan 2009, #05; Wed, 21)]
From: Sam Vilain @ 2009-02-01  3:09 UTC (permalink / raw)
  To: Jeff King; +Cc: Johannes Schindelin, Junio C Hamano, git
In-Reply-To: <20090131073640.GF3033@coredump.intra.peff.net>

[re-sent with correct envelope from so hopefully it will get to the
list this time]

On Sat, 2009-01-31 at 02:36 -0500, Jeff King wrote:
> Actually, lookup is even easier than that: we iterate through the entire
> tree recursively and add everything to a flat hash. So we really don't
> care there what the layout is like (just take the first 40 characters of
> any directory name as a hash).

Sure, but if you do want to scale to a hundred thousand notes, then I
think it would pay to have a plan for making it lazy as required.  ie,
if a run just wants notes for 20 commits and there are 256 sub-trees
then only read 20 of them.  Doesn't matter if it's not implemented
initially of course, so long as the on-disk format is supported by the
tools in the first release they will be backward compatible.  And it's
not that complicated; see attached.

> But it violates the usual git principle of "content has a unique name".
> What happens when I add "a/b" and you add "ab"? A dumb merge will let
> both co-exist, but which one do you return for lookup?

It should only be tools adding to it, the trees shouldn't be modified
directly by users.  In the below patch I make these all get deleted on
'git notes edit'.

Depending on the semantics of the notes, it might not be an error to
have multiple notes for a commit.  In my patch this is "tolerated" to
some extent but not supported by git-notes.sh porcelain yet.

> I agree that there should be multiple note hierarchies, and multiple
> keys within each hierarchy. I have posted some thoughts on that before
> (and you should be able to find them searching for "notes" in the list
> archive), but unfortunately I have not had time to sit down and really
> work out a notes implementation that matches what I posted (which I
> don't think is that far from Dscho's work in next).

I had a brief look and couldn't find it, this was about the best one I
found from you in terms of links to previous discussions;
http://kerneltrap.org/mailarchive/git/2008/12/16/4427794 If there's
another thread you'd like me to read please fish it out and respond!
The more messages we have linking to the previous discussions the
better :).

> And I think what you are proposing (here and in the rest of your
> message) is that certain notes hierarchies may have particular formats
> and semantics. And that sounds reasonable to me.

Yes that was one part of it.  But also make a convention that the
'commits' notes, ie the default ones, an RFC822 message if they begin
with "known" headers.  Then porcelain such as log can inject them into
the fields at the appropriate point.

Anyway, without further ado here's the XX/XXXX split patch.

Subject: [PATCH] git-notes: allow for arbitrary split of entries into sub-trees

It might later turn out for performance reasons that a single tree for
notes will not be sufficient.  While this does not solve the
performance problem as it still loads the entire lot of notes into a
hash at start-up, it does mean that such a change does not have to
worry about backward compatibility with git versions that don't yet
support it.

Signed-off-by: Sam Vilain <sam@vilain.net>
---
 git-notes.sh     |   45 ++++++++++++++++++++++++++++++++++++++-------
 notes.c          |   39 ++++++++++++++++++++++++++++++++-------
 t/t3301-notes.sh |   11 +++++++++++
 3 files changed, 81 insertions(+), 14 deletions(-)

diff --git a/git-notes.sh b/git-notes.sh
index bfdbaa8..e07499f 100755
--- a/git-notes.sh
+++ b/git-notes.sh
@@ -10,15 +10,35 @@ ACTION="$1"; shift
 
 test -z "$GIT_NOTES_REF" && GIT_NOTES_REF="$(git config core.notesref)"
 test -z "$GIT_NOTES_REF" && GIT_NOTES_REF="refs/notes/commits"
+export GIT_NOTES_REF
 
 COMMIT=$(git rev-parse --verify --default HEAD "$@") ||
 die "Invalid commit: $@"
+NOTES_PATH=$COMMIT
+case "$GIT_NOTES_SPLIT" in
+	[1-9]|[1-4][0-9])
+		NOTES_PATH=$( echo $COMMIT | perl -pe 's{^(.{'$GIT_NOTES_SPLIT'})}{$1/}' )
+		;;
+esac
 
 MESSAGE="$GIT_DIR"/new-notes-$COMMIT
 trap '
 	test -f "$MESSAGE" && rm "$MESSAGE"
 ' 0
 
+show_note() {
+	COMMIT=$1
+	NOTE_PATH=$( git ls-tree --name-only -r $GIT_NOTES_REF | perl -nle '
+		$x = $_; s{/}{}g;
+		if (m{'$COMMIT'}) {
+			print $x;
+			exit;
+		}
+	' )
+	[ -n "$NOTE_PATH" ] &&
+		git cat-file blob $GIT_NOTES_REF:$NOTE_PATH
+}
+
 case "$ACTION" in
 edit)
 	GIT_NOTES_REF= git log -1 $COMMIT | sed "s/^/#/" > "$MESSAGE"
@@ -32,7 +52,7 @@ edit)
 	else
 		PARENT="-p $CURRENT_HEAD"
 		git read-tree "$GIT_NOTES_REF" || die "Could not read index"
-		git cat-file blob :$COMMIT >> "$MESSAGE" 2> /dev/null
+		show_note $COMMIT >> "$MESSAGE"
 	fi
 
 	${VISUAL:-${EDITOR:-vi}} "$MESSAGE"
@@ -42,15 +62,26 @@ edit)
 	if [ -s "$MESSAGE" ]; then
 		BLOB=$(git hash-object -w "$MESSAGE") ||
 			die "Could not write into object database"
-		git update-index --add --cacheinfo 0644 $BLOB $COMMIT ||
+		git update-index --add --cacheinfo 0644 $BLOB $NOTES_PATH ||
 			die "Could not write index"
 	else
-		test -z "$CURRENT_HEAD" &&
-			die "Will not initialise with empty tree"
-		git update-index --force-remove $COMMIT ||
-			die "Could not update index"
+		NOTES_PATH=dummy
 	fi
 
+	git ls-files | perl -nle '
+		$x = $_; s{/}{}g;
+		if (m{'$COMMIT'} and $x ne q{'$NOTES_PATH'}) {
+			print $x;
+		}' |
+		while read path
+			do
+				git update-index --force-remove $path ||
+			    		die "Could not update index"
+			done
+	
+	[ -z "$(git ls-files)" -a -z "$CURRENT_HEAD" ] &&
+		die "Will not initialise with empty tree"
+
 	TREE=$(git write-tree) || die "Could not write tree"
 	NEW_HEAD=$(echo Annotate $COMMIT | git commit-tree $TREE $PARENT) ||
 		die "Could not annotate"
@@ -58,7 +89,7 @@ edit)
 		"$GIT_NOTES_REF" $NEW_HEAD $CURRENT_HEAD
 ;;
 show)
-	git show "$GIT_NOTES_REF":$COMMIT
+	show_note $COMMIT
 ;;
 *)
 	usage
diff --git a/notes.c b/notes.c
index bd73784..d763b50 100644
--- a/notes.c
+++ b/notes.c
@@ -70,28 +70,53 @@ static void add_entry(const unsigned char *commit_sha1,
 	hashcpy(hash_map.entries[index].notes_sha1, notes_sha1);
 }
 
-static void initialize_hash_map(const char *notes_ref_name)
+static void initialize_hash_map_recursive(const char *tree_sha1, const char *path)
 {
 	unsigned char sha1[20], commit_sha1[20];
+	unsigned char path_combined[40];
 	unsigned mode;
 	struct tree_desc desc;
 	struct name_entry entry;
+	int length = strlen(path);
+	int length_combined;
 	void *buf;
 
-	if (!notes_ref_name || read_ref(notes_ref_name, commit_sha1) ||
-	    get_tree_entry(commit_sha1, "", sha1, &mode))
+	strcpy(path_combined, path);
+
+	if (get_tree_entry(tree_sha1, "", sha1, &mode))
 		return;
 
 	buf = fill_tree_descriptor(&desc, sha1);
 	if (!buf)
-		die("Could not read %s for notes-index", sha1_to_hex(sha1));
+		die("Could not read %s for notes-index", sha1_to_hex(tree_sha1));
+
+	while (tree_entry(&desc, &entry)) {
+		length_combined = length + strlen(entry.path);
+		if (length_combined >= 40) {
+			strncpy(path_combined + length, entry.path,
+				41 - length);
+			if (!get_sha1(path_combined, commit_sha1))
+				add_entry(commit_sha1, entry.sha1);
+		}
+		else {
+			strcpy(path_combined + length, entry.path);
+			initialize_hash_map_recursive(entry.sha1, path_combined);
+		}
+	}
 
-	while (tree_entry(&desc, &entry))
-		if (!get_sha1(entry.path, commit_sha1))
-			add_entry(commit_sha1, entry.sha1);
 	free(buf);
 }
 
+static void initialize_hash_map(const char *notes_ref_name)
+{
+	unsigned char commit_sha1[20];
+
+	if (!notes_ref_name || read_ref(notes_ref_name, commit_sha1))
+		return;
+
+	initialize_hash_map_recursive( commit_sha1, "" );
+}
+
 static unsigned char *lookup_notes(const unsigned char *commit_sha1)
 {
 	int index;
diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index 9393a25..3734b55 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -92,4 +92,15 @@ test_expect_success 'show multi-line notes' '
 	test_cmp expect-multiline output
 '
 
+test_expect_success 'create split notes tree' '
+	: > a4 &&
+	git add a4 &&
+	test_tick &&
+	git commit -m 4th &&
+	MSG="b4" GIT_NOTES_SPLIT=2 git notes edit &&
+	[ "$(git notes show)" = "b4" ] &&
+	[ -n "$(git ls-tree --name-only -r refs/notes/commits | grep /)" ] &&
+	[ -n "$(git log -1 | grep Notes:)" ]
+'
+
 test_done
-- 
debian.1.5.6.1

^ permalink raw reply related

* Re: [RFC PATCH 3/3] Support fetching from foreign VCSes
From: Johan Herland @ 2009-02-01  2:20 UTC (permalink / raw)
  To: Daniel Barkalow; +Cc: git, Junio C Hamano
In-Reply-To: <alpine.LNX.1.00.0901110335520.19665@iabervon.org>

On Sunday 11 January 2009, Daniel Barkalow wrote:
> This supports a useful subset of the usual fetch logic, mostly in the
> config file.

Hi,

I love the idea of this patch series, and have started working on a CVS 
backend for this. I have a question though...

> Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
> ---
>  builtin-fetch.c |  135
> +++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed,
> 132 insertions(+), 3 deletions(-)
>
> diff --git a/builtin-fetch.c b/builtin-fetch.c
> index 7b46f8f..14e037e 100644
> --- a/builtin-fetch.c
> +++ b/builtin-fetch.c
> @@ -614,6 +614,136 @@ static void set_option(const char *name, const char
> *value) name, transport->url);
>  }
>
> +static struct ref *list_foreign(struct remote *remote)
> +{

[...]

> +}
> +
> +static int import_foreign(struct remote *remote, struct ref *refs)
> +{

[...]

> +}
> +
> +static int fetch_foreign(struct remote *remote)
> +{
> +	struct ref *remote_refs = list_foreign(remote);

We retrieve a list of all refs available at the given remote...

> +	struct ref *ref_map = NULL;
> +	struct ref *rm;
> +	struct ref **tail = &ref_map;
> +	struct branch *branch;
> +	int i;
> +
> +	int exit_code = import_foreign(remote, remote_refs);

...and then we start fetching _all_ the refs returned by list_foreign().

When I call "git fetch <vcs-remote> <ref>", I expect _only_ <ref> to be 
fetched from the remote, but it seems the above code doesn't even concern 
itself with the ref(s) specified on the "git fetch" command-line

I'm not even sure why list_foreign() should be called in this case (except, 
maybe, to verify the existence of <ref> before attempting to fetch it).

AFAICS list_foreign() is only needed by "git fetch" if <ref> contains a 
wildcard (like the default refspec: +refs/heads/*:refs/remotes/<remote>/*).

...or have I misunderstood something fundamental about how this is going to 
work?


Have fun! :)

...Johan

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

^ permalink raw reply

* Re: [PATCH] Switch receive.denyCurrentBranch to "refuse"
From: Sam Vilain @ 2009-02-01  3:37 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nanako Shiraishi, Johannes Schindelin, Jay Soffian, git
In-Reply-To: <7v63juzz9m.fsf@gitster.siamese.dyndns.org>

On Sat, 2009-01-31 at 18:06 -0800, Junio C Hamano wrote:
> Nanako Shiraishi <nanako3@lavabit.com> writes:
> 
> > I assume that everybody, including the maintainer who is too heavyweight
> > and has too much inertia to accept too sudden a change of the course,
> > wants to eventually make the default to deny pushing to the current
> > branch. But I think such a change should come at 1.7.0 release at the
> > earliest, and a constructive thing to do is to put in a patch to 1.6.2
> > that helps the users with the eventual transition.
> 
> I am not opposed to eventually change the default to refuse at some point,
> but I have to say that now would not be the best time to do so.  Jeff's
> 986e823 (receive-pack: detect push to current branch of non-bare repo,
> 2008-11-08) that is v1.6.1-rc1~59^2 was the one we started warning about
> this, and we only had one major release since then, and I'd love to see a
> solid rc or even the final release by mid February.

Personally I think it's worth fast tracking, because I think very few
people are actually using push to a checked out branch whereas many
people are confused by the behaviour.  I just can't understand the
resistance to this safety feature.  People who encounter the bug can
just change the setting and move on... it seems like an argument based
on "principles", usually a sign that one has run out of actual
arguments..

> By the way, I do not appreciate other people who I have never met
> speculate about my body mass very much.  I am on the skinner end of the
> spectrum, if you need to know.

lol.  It was a metaphorical use of the term from my reading ;-)

Sam.

^ permalink raw reply

* Re: [PATCH 4/4] Change current mailmap usage to do matching on both name and email of author/committer.
From: Marius Storm-Olsen @ 2009-02-01  7:07 UTC (permalink / raw)
  To: git
In-Reply-To: <34f625b57597810e4c084fcd3eaf2cfd9def803e.1233438174.git.marius@trolltech.com>

Marius Storm-Olsen said the following on 31.01.2009 22:48:
>  Documentation/git-shortlog.txt   |   13 +----

Obviously the correcting parts to the git-shortlog documentation 
should be folded into PATCH 3. I'll do that later.

--
.marius

^ permalink raw reply

* Re: [PATCH] split notes [was: Re: What's cooking in git.git (Jan 2009, #05; Wed, 21)]
From: Jakub Narebski @ 2009-02-01 12:01 UTC (permalink / raw)
  To: git
In-Reply-To: <1233455960.17688.122.camel@maia.lan>

Sam Vilain wrote:

>  test -z "$GIT_NOTES_REF" && GIT_NOTES_REF="$(git config core.notesref)"
>  test -z "$GIT_NOTES_REF" && GIT_NOTES_REF="refs/notes/commits"

GIT_NOTES_REF=$($(GIT_NOTES_REF:-$(git config core.notesref):-refs/notes/commits)

Or something like that.
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

^ permalink raw reply

* Re: [PATCH JGIT] fix typo
From: Baz @ 2009-02-01 12:42 UTC (permalink / raw)
  To: Yann Simon; +Cc: Shawn O. Pearce, Robin Rosenberg, git
In-Reply-To: <1233411967.8213.4.camel@localhost>

2009/1/31 Yann Simon <yann.simon.fr@gmail.com>:
> fix a little typo
> Signed-off-by: Yann Simon <yann.simon.fr@gmail.com>
> ---
>  .../src/org/spearce/jgit/lib/WindowedFile.java     |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowedFile.java
> b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowedFile.java
> index 5eb8465..1ff8caa 100644
> --- a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowedFile.java
> +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowedFile.java
> @@ -61,7 +61,7 @@
>  * before any single byte can be accessed.
>  * </p>
>  * <p>
> - * Using a specific MapMode will avoid the complete copy by mmaping in
> the
> + * Using a specific MapMode will avoid the complete copy by mapping in

Is this actually a typo? java.nio being used to mmap() files here (see
the later use of mmap in this class), its more informative and common
to use the double m to describe that. I'd have said the typo was at
the end of the comment, ie "fast enough to permit new windows to be
mapped." should read "fast enough to permit new windows to be
mmapped."

> the
>  * operating system's file buffers, however this may cause problems if
> a large
>  * number of windows are being heavily accessed as the Java garbage
> collector
>  * may not be able to unmap old windows fast enough to permit new
> windows to be
> --
> 1.6.0.6
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply

* Re: Newbie question regarding 3way merge order.
From: Raimund Berger @ 2009-02-01 14:13 UTC (permalink / raw)
  To: git
In-Reply-To: <20090201064543.6117@nanako3.lavabit.com>

Nanako Shiraishi <nanako3@lavabit.com> writes:

> Quoting "Raimund Berger" <raimund.berger@gmail.com> writes:
>
>> I'm myself, especially since a conflicting rebase leaves the index in an
>> "unmerged" state. Much like a regular merge does. It's still all
>> assumptions though, or maybe I'm missing documentation .... (?)
>
> The rebase command you run with neither -m nor -i option replays
> your work on top of the upstream by running git-am with the --3way
> option.
>
> This commit introduced the feature.
>
> commit 7f59dbbb8f8d479c1d31453eac06ec765436a780
> Author: Junio C Hamano <junkio@cox.net>
> Date:   Mon Nov 14 00:41:53 2005 -0800
>
>     Rewrite rebase to use git-format-patch piped to git-am.
>     
>     The current rebase implementation finds commits in our tree but
>     not in the upstream tree using git-cherry, and tries to apply
>     them using git-cherry-pick (i.e. always use 3-way) one by one.
>     
>     Which is fine, but when some of the changes do not apply
>     cleanly, it punts, and punts badly.
>
>     [omitting the rest]
>
> The message talks about what was wrong with the original, what
> benefit it gives the users, and how to use it, but it doesn't
> discuss how the magic works in detail.
>
> Junio much later describes how it works, taking a real-world example
> in this message:
>
> http://thread.gmane.org/gmane.comp.version-control.git/46569/focus=46609
>
> In short, it works by applying your changes as patches but when a
> patch doesn't apply it falls back to a simplified three-way merge.
>
> http://thread.gmane.org/gmane.comp.version-control.git/100579/focus=100602
>
> I think Junio misremembered the history in his last message in the thread.
> He says that rebase was originally a format-patch piped to am, but
> before the commit 7f59dbbb8f8d479c1d31453eac06ec765436a780 it was
> done by a series of cherry-pick.


Ugh. OK, that really explains a lot alright. Especially that fundamental
change in rebase behavior makes it quite clear that the various
techniques for "merging" content might not be as consistent as I had
hoped. In my ideal world they'd only differ in how they map out commit
history, but obviously that's not the case.

To me, this really leaves questions open regarding a hassle free
workflow. Since one apparently can't be confident that conflict
resolutions obtained in some testing or early integration branch will
easily carry over to stable it looks like special measures need to be
taken, like strict observation of merge order/technique and employment
of replay tools.

So serialization apparently can't entirely be lost, and where e.g. CVS
kind of enforces it at an early stage through expensive branching it
seems git still requires some of the same coordination just at a
different level resp. point in time.

Hey, thanks a bunch for taking the time carrying those links
together. They've been very informative.

^ permalink raw reply

* git remote rm considered harmful?
From: Jay Soffian @ 2009-02-01 14:52 UTC (permalink / raw)
  To: Git Mailing List

Yesterday I was not paying attention to a repository I was in when I
used git remote to make a mirror:

$ git remote add --mirror bar git://foo/bar

I realized my mistake and removed the mirror:

$ git remote rm bar

Poof. All of my local branches and their reflogs were gone. Reproduce with:

# Setup a repository
mkdir foo && cd foo && git init
echo foo > foo && git add foo && git commit -m foo

# Make some branches for demonstration purposes
git co -b branch1
git co -b branch2
git co -b branch3
git co master
find .git/refs .git/logs
.git/refs
.git/refs/heads
.git/refs/heads/branch1
.git/refs/heads/branch2
.git/refs/heads/branch3
.git/refs/heads/master
.git/refs/tags
.git/logs
.git/logs/HEAD
.git/logs/refs
.git/logs/refs/heads
.git/logs/refs/heads/branch1
.git/logs/refs/heads/branch2
.git/logs/refs/heads/branch3
.git/logs/refs/heads/master

# Setup a remote
git remote add --mirror bar git://foo/bar

# Oops, didn't mean to do that, remove the repo
git remote rm bar # poof!
find .git/refs .git/logs
.git/refs
.git/refs/heads
.git/refs/tags
.git/logs
.git/logs/HEAD
.git/logs/refs
.git/logs/refs/heads

I just realized that at least the HEAD log is still there. I missed
that originally since:

git reflog
fatal: bad default revision 'HEAD'

(Separate issue, but shouldn't "git reflog" work even if .git/HEAD is invalid?)

Anyway, it would seem to me it should be harder to remove local refs.
This one was somewhat painful to recover from. Not sure what better
behavior would be: should it be harder to do "remote add --mirror" on
a repository with content, should "remote rm" on mirrored repository
require a --force switch with a stern warning first, or...?

Thoughts?

j.

^ permalink raw reply

* [PATCH] builtin-remote: make rm operation safer in mirrored repository
From: Jay Soffian @ 2009-02-01 15:48 UTC (permalink / raw)
  To: git; +Cc: Jay Soffian
In-Reply-To: <76718490902010652w7a332550w5e0658a390c7dea9@mail.gmail.com>

"git remote rm <repo>" is happy to remove non-remote branches (and their
reflogs). This may be okay if the repository truely is a mirror, but if the
user had done "git remote add --mirror <repo>" by accident and was just
undoing their mistake, then they are left in a situation that is difficult to
recover from.

After this commit, "git remote rm" skips over non-remote branches and instead
advises the user on how to remove such branches using "git branch -d", which
itself has nice safety checks wrt to branch removal lacking from "git remote
rm".

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
On Sun, Feb 1, 2009 at 9:52 AM, Jay Soffian <jaysoffian@gmail.com> wrote:
> Anyway, it would seem to me it should be harder to remove local refs.
> This one was somewhat painful to recover from. Not sure what better
> behavior would be: should it be harder to do "remote add --mirror" on
> a repository with content, should "remote rm" on mirrored repository
> require a --force switch with a stern warning first, or...?

Perhaps something like this?

 builtin-remote.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/builtin-remote.c b/builtin-remote.c
index abc8dd8..2629bc5 100644
--- a/builtin-remote.c
+++ b/builtin-remote.c
@@ -310,6 +310,13 @@ static int add_branch_for_removal(const char *refname,
 	struct string_list_item *item;
 	struct known_remote *kr;
 
+	/* don't delete non-remote branches */
+	if (prefixcmp(refname, "refs/remotes")) {
+		warning("not removing non-remote branch; use git branch -d %s to remove",
+			abbrev_branch(refname));
+		return 0;
+	}
+
 	memset(&refspec, 0, sizeof(refspec));
 	refspec.dst = (char *)refname;
 	if (remote_find_tracking(branches->remote, &refspec))
-- 
1.6.1.224.gb56c

^ permalink raw reply related

* git monthly links: 2009-01
From: Felipe Contreras @ 2009-02-01 17:01 UTC (permalink / raw)
  To: git list

Hi,

git monthly links is my attempt to gather all the links people have
been tagging as "git" in delicious.com[1] (these are not chosen by
me). It's a manual process so I might miss some links; I subscribe to
the RSS feed and go through all the links simply picking the ones
tagged the most.

Previously I was doing this on a weekly basis but I decided there
where too few links per week so now I'll be trying with monthly links,
let's see how it goes.

The fancier blog version is here:
http://gitlog.wordpress.com/2009/02/01/git-monthly-links-2009-01/

== Hot ==

This month was overwhelmed by git ready, a site devoted to share all kinds of
tips for git. I'm grouping these together as well as other hugely popular links.

git ready: daily tips for the noob to the guru
http://gitready.com/

branching and merging
http://gitready.com/beginner/2009/01/25/branching-and-merging.html

zsh git status
http://gitready.com/advanced/2009/01/28/zsh-git-status.html

visualizing your repo
http://gitready.com/intermediate/2009/01/13/visualizing-your-repo.html

My Git Workflow
Michael Ivey explains his choice of git, how he uses it and loves it.
http://gweezlebur.com/2009/01/19/my-git-workflow.html

An Illustrated Guide to Git on Windows
Very complete instructions to use git on windows (with nice screenshots).
http://nathanj.github.com/gitguide/index.html

== Other links ==

Fine. Git is Awesome.
Justin M. Tulloss explains his switch from mercurial to git, advantages,
disadvantages and comes to the conclusion that git is awesome.
http://justin.harmonize.fm/index.php/2009/01/fine-git-is-awesome/

Github: Making Code More Social
Brady Forrest talks about github, and how it increased the activity of the Ruby
on Rails project.
http://radar.oreilly.com/2009/01/github-making-code-more-social.html

Tracking an entire Windows system inside Git
Avery Pennarun explains his endeavor of tracking windows with git...
"If I get a virus, I can 'git revert' it."
http://alumnit.ca/~apenwarr/log/?m=200901#21

Ruby Swarms: Visualizing Rails & Git
Different visualizations of github activity and Ruby on Rails project
with codeswarm.
http://www.igvita.com/2009/01/27/ruby-swarms-visualizing-rails-git/

Setting up a new remote git repository
Pretty extensive explanation of how to setup a remote git repo.
http://toolmantim.com/articles/setting_up_a_new_remote_git_repository

Agile git and the story branch pattern
http://blog.hasmanythrough.com/2008/12/18/agile-git-and-the-story-branch-pattern

GIT 101
List of links to learn about git, from Xebia.
http://blog.xebia.com/2009/01/26/git-101/

Git and Rails: A detailed tutorial including plugins, submodules,
development and production
http://harryseldon.thinkosphere.com/2009/01/14/git-and-rails-a-detailed-tutorial-including-plugins-submodules-development-and-production

Commit Chat
A site to gather github conversations in one place.
http://commitchat.com/

Sharing git repositories via OS X's built-in web sharing
http://toolmantim.com/articles/sharing_git_repositories_via_os_xs_built_in_web_sharing

cvsgit.png
A nice diagram showing the advantage of git merging compared to cvs.
http://annoware.com/images/cvsgit.png

The Best Environment for Rails on Windows
Nice and extensive explanation to setup a Ruby on Rails development environment
on Windows without an IDE: vim, ctags, console2, etc.
http://akitaonrails.com/2009/1/13/the-best-environment-for-rails-on-windows/

Hack && Ship
Rein Henrichs explains his development process: hack & ship
http://reinh.com/blog/2008/08/27/hack-and-and-ship.html

Craft the perfect gem with Jeweler @ Technical Pickles
Explanation of rubygems and how to automate the process of making releases of
them with rake and other git-related automated procedures.
http://technicalpickles.com/posts/craft-the-perfect-gem-with-jeweler

Git Guide (SourceMage Wiki)
http://www.sourcemage.org/Git_Guide

3 Reasons to Switch to Git from Subversion
http://markmcb.com/2008/10/18/3-reasons-to-switch-to-git-from-subversion/

A Gaggle Of Git Tips
http://www.viget.com/extend/a-gaggle-of-git-tips/

Effectively Using Git With Subversion
http://www.viget.com/extend/effectively-using-git-with-subversion/

Show Git dirty state (and branch) in the prompt
http://henrik.nyh.se/2008/12/git-dirty-prompt

Perl Migrates To the Git Version Control System
http://developers.slashdot.org/comments.pl?sid=1079921&cid=26321869

== Japanese ==

分散バージョン管理Git/Mercurial/Bazaar徹底比較
http://www.atmarkit.co.jp/fjava/rensai4/devtool03/devtool03_1.html

github で人のコードをいじる「前」にforkする必要はない
http://subtech.g.hatena.ne.jp/miyagawa/20090114/1231910461

[1] http://delicious.com/tag/git

-- 
Felipe Contreras

^ permalink raw reply

* Re: What's cooking in git.git (Jan 2009, #07; Wed, 28)
From: Kirill Smelkov @ 2009-02-01 17:45 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vwscej26i.fsf@gitster.siamese.dyndns.org>

On Wed, Jan 28, 2009 at 06:06:45PM -0800, Junio C Hamano wrote:

[...]

> * ks/maint-mailinfo-folded (Tue Jan 13 01:21:04 2009 +0300) 4 commits
>  + mailinfo: tests for RFC2047 examples
>  + mailinfo: add explicit test for mails like '<a.u.thor@example.com>
>    (A U Thor)'
>  + mailinfo: 'From:' header should be unfold as well
>  + mailinfo: correctly handle multiline 'Subject:' header
> 
> I just got tired of waiting and cleaned up the series myself.

Sorry about that. Here is the missing bit (based on master)

--- 8< ---

Subject: [PATCH] mailinfo: cleanup extra spaces for complex 'From:'

currently for cases like

    From: A U Thor <a.u.thor@example.com> (Comment)

mailinfo extracts the following 'Author:' field:

    Author: A U Thor   (Comment)
                     ^^
which has two extra spaces left in there after removed email part.

I think this is wrong so here is a fix.

Signed-off-by: Kirill Smelkov <kirr@landau.phys.spbu.ru>
---
 builtin-mailinfo.c        |   19 +++++++++++++++----
 t/t5100/info0001          |    2 +-
 t/t5100/rfc2047-info-0004 |    2 +-
 t/t5100/sample.mbox       |    4 ++--
 4 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/builtin-mailinfo.c b/builtin-mailinfo.c
index d4dc23a..2789ccd 100644
--- a/builtin-mailinfo.c
+++ b/builtin-mailinfo.c
@@ -29,6 +29,9 @@ static struct strbuf **p_hdr_data, **s_hdr_data;
 #define MAX_HDR_PARSED 10
 #define MAX_BOUNDARIES 5
 
+static void cleanup_space(struct strbuf *sb);
+
+
 static void get_sane_name(struct strbuf *out, struct strbuf *name, struct strbuf *email)
 {
 	struct strbuf *src = name;
@@ -109,11 +112,19 @@ static void handle_from(const struct strbuf *from)
 	strbuf_add(&email, at, el);
 	strbuf_remove(&f, at - f.buf, el + (at[el] ? 1 : 0));
 
-	/* The remainder is name.  It could be "John Doe <john.doe@xz>"
-	 * or "john.doe@xz (John Doe)", but we have removed the
-	 * email part, so trim from both ends, possibly removing
-	 * the () pair at the end.
+	/* The remainder is name.  It could be
+	 *
+	 * - "John Doe <john.doe@xz>"			(a), or
+	 * - "john.doe@xz (John Doe)"			(b), or
+	 * - "John (zzz) Doe <john.doe@xz> (Comment)"	(c)
+	 *
+	 * but we have removed the email part, so
+	 *
+	 * - remove extra spaces which could stay after email (case 'c'), and
+	 * - trim from both ends, possibly removing the () pair at the end
+	 *   (cases 'a' and 'b').
 	 */
+	cleanup_space(&f);
 	strbuf_trim(&f);
 	if (f.buf[0] == '(' && f.len && f.buf[f.len - 1] == ')') {
 		strbuf_remove(&f, 0, 1);
diff --git a/t/t5100/info0001 b/t/t5100/info0001
index 8c05277..f951538 100644
--- a/t/t5100/info0001
+++ b/t/t5100/info0001
@@ -1,4 +1,4 @@
-Author: A U Thor
+Author: A (zzz) U Thor (Comment)
 Email: a.u.thor@example.com
 Subject: a commit.
 Date: Fri, 9 Jun 2006 00:44:16 -0700
diff --git a/t/t5100/rfc2047-info-0004 b/t/t5100/rfc2047-info-0004
index 0ca7ff0..f67a90a 100644
--- a/t/t5100/rfc2047-info-0004
+++ b/t/t5100/rfc2047-info-0004
@@ -1,4 +1,4 @@
-Author: Nathaniel Borenstein   (םולש ןב ילטפנ)
+Author: Nathaniel Borenstein (םולש ןב ילטפנ)
 Email: nsb@thumper.bellcore.com
 Subject: Test of new header generator
 
diff --git a/t/t5100/sample.mbox b/t/t5100/sample.mbox
index 85df55f..c5ad206 100644
--- a/t/t5100/sample.mbox
+++ b/t/t5100/sample.mbox
@@ -2,10 +2,10 @@
 	
     
 From nobody Mon Sep 17 00:00:00 2001
-From: A
+From: A (zzz)
       U
       Thor
-      <a.u.thor@example.com>
+      <a.u.thor@example.com> (Comment)
 Date: Fri, 9 Jun 2006 00:44:16 -0700
 Subject: [PATCH] a commit.
 
-- 
1.6.1.284.g5dc13

--- 8< ---


Thanks,
Kirill

^ permalink raw reply related

* 026fa0d5ad Breaks installs with absolue $(gitexecdir) and $(template_dir) variables using older GNU makes
From: A Large Angry SCM @ 2009-02-01 18:24 UTC (permalink / raw)
  To: Steffen Prohaska; +Cc: git, Junio C Hamano

In 026fa0d5ad9538ca76838070861531c037d7b9ba, "Move computation of 
absolute paths from Makefile to runtime (in preparation for 
RUNTIME_PREFIX)", the following change was made to the Makefile. The 
problem is that $(abspath names...) is a relatively recent addition to 
GNU make and when used in an older GNU make, the test always fails 
resulting incorrect installation dirs for the templates and commands.

The new test is also wrong; (for *nix systems) in that it really wants 
to test if the first character is a '/' but GNU make doesn't have a way 
to do that directly. Instead, it tests if the first character is a / 
_AND_ the path string does not include . or .. components,

The older test has problems itself but at least it allowed you to 
specify absolute paths.

@@ -1407,17 +1417,17 @@ remove-dashes:

  ### Installation rules

-ifeq ($(firstword $(subst /, ,$(template_dir))),..)
-template_instdir = $(bindir)/$(template_dir)
-else
+ifeq ($(abspath $(template_dir)),$(template_dir))
  template_instdir = $(template_dir)
+else
+template_instdir = $(prefix)/$(template_dir)
  endif
  export template_instdir

-ifeq ($(firstword $(subst /, ,$(gitexecdir))),..)
-gitexec_instdir = $(bindir)/$(gitexecdir)
-else
+ifeq ($(abspath $(gitexecdir)),$(gitexecdir))
  gitexec_instdir = $(gitexecdir)
+else
+gitexec_instdir = $(prefix)/$(gitexecdir)
  endif
  gitexec_instdir_SQ = $(subst ','\'',$(gitexec_instdir))
  export gitexec_instdir


I see 3 ways to fix the problem:
	1) go back to using the old test.
	2) keep the new test but add a test that will break the build if
		$(abspath names...) does not work.
	3) something like the following (untested)


  ### Installation rules


  temp = $(subst " ",x,$(template_dir))
  temp = $(subst //,/,$(temp))
  temp = $(addsuffix x,$(temp))
  temp = $(subst /, ,$(temp))
  ifeq ($(strip $(temp)),$(temp))
  template_instdir = $(template_dir)
  else
  template_instdir = $(prefix)/$(template_dir)
  endif
  export template_instdir

  temp = $(subst " ",x,$(gitexecdir))
  temp = $(subst //,/,$(temp))
  temp = $(addsuffix x,$(temp))
  temp = $(subst /, ,$(temp))
  ifeq ($(strip $(temp)),$(temp))
  gitexec_instdir = $(gitexecdir)
  else
  gitexec_instdir = $(prefix)/$(gitexecdir)
  endif
  gitexec_instdir_SQ = $(subst ','\'',$(gitexec_instdir))
  export gitexec_instdir

^ permalink raw reply


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