Git development
 help / color / mirror / Atom feed
* [RFC/PATCH v6 2/2] gitweb: Smarter snapshot names
From: Jakub Narebski @ 2009-10-29 22:07 UTC (permalink / raw)
  To: git; +Cc: Mark Rada, Shawn O. Pearce, Jakub Narebski
In-Reply-To: <1256854062-25496-2-git-send-email-jnareb@gmail.com>

From: Mark Rada <marada@uwaterloo.ca>

Teach gitweb how to produce nicer snapshot names by only using the
short hash id.  If clients make requests using a tree-ish that is not
a partial or full SHA-1 hash, then the short hash will also be appended
to whatever they asked for.  If clients request snapshot of a tag
(which means that $hash ('h') parameter has 'refs/tags/' prefix),
use only tag name.

This also includes tests cases for t9502-gitweb-standalone-parse-output.

Signed-off-by: Mark Rada <marada@uwaterloo.ca>
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This is currently an RFC because commit message does not describe
changes completely.  Among others it doesn't describe current rules.
Also tests could be better described.

Changes since v5 (by Mark Rada):
- put common parts of git_get_full_hash and git_get_short_hash 
  into git_get_hash, do not do double verification
- separate finding snapshot name and prefix in snapshot_name()
  subroutine, more smarts for snapshot name
- improved tests (use 'tar' snapshot format, testing new smarts,
  testing prefix)

 gitweb/gitweb.perl                        |   76 +++++++++++++++----
 t/t9502-gitweb-standalone-parse-output.sh |  112 +++++++++++++++++++++++++++++
 2 files changed, 172 insertions(+), 16 deletions(-)
 create mode 100755 t/t9502-gitweb-standalone-parse-output.sh

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 8d4a2ae..d8dfd95 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1983,16 +1983,27 @@ sub quote_command {
 
 # get HEAD ref of given project as hash
 sub git_get_head_hash {
-	my $project = shift;
+	return git_get_full_hash(shift, 'HEAD');
+}
+
+sub git_get_full_hash {
+	return git_get_hash(@_);
+}
+
+sub git_get_short_hash {
+	return git_get_hash(@_, '--short=7');
+}
+
+sub git_get_hash {
+	my ($project, $hash, @options) = @_;
 	my $o_git_dir = $git_dir;
 	my $retval = undef;
 	$git_dir = "$projectroot/$project";
-	if (open my $fd, "-|", git_cmd(), "rev-parse", "--verify", "HEAD") {
-		my $head = <$fd>;
+	if (open my $fd, '-|', git_cmd(), 'rev-parse',
+	    '--verify', '-q', @options, $hash) {
+		$retval = <$fd>;
+		chomp $retval if defined $retval;
 		close $fd;
-		if (defined $head && $head =~ /^([0-9a-fA-F]{40})$/) {
-			$retval = $1;
-		}
 	}
 	if (defined $o_git_dir) {
 		$git_dir = $o_git_dir;
@@ -5179,6 +5190,43 @@ sub git_tree {
 	git_footer_html();
 }
 
+sub snapshot_name {
+	my ($project, $hash) = @_;
+
+	# path/to/project.git  -> project
+	# path/to/project/.git -> project
+	my $name = to_utf8($project);
+	$name =~ s,([^/])/*\.git$,$1,;
+	$name = basename($name);
+	# sanitize name
+	$name =~ s/[[:cntrl:]]/?/g;
+
+	my $ver = $hash;
+	if ($hash =~ /^[0-9a-fA-F]+$/) {
+		# shorten SHA-1 hash
+		my $full_hash = git_get_full_hash($project, $hash);
+		if ($full_hash =~ /^$hash/ && length($hash) > 7) {
+			$ver = git_get_short_hash($project, $hash);
+		}
+	} elsif ($hash =~ m!^refs/tags/(.*)$!) {
+		# tags don't need shortened SHA-1 hash
+		$ver = $1;
+	} else {
+		# branches and other need shortened SHA-1 hash
+		if ($hash =~ m!^refs/(?:heads|remotes)/(.*)$!) {
+			$ver = $1;
+		}
+		$ver .= '-' . git_get_short_hash($project, $hash);
+	}
+	# in case of hierarchical branch names
+	$ver =~ s!/!.!g;
+
+	# name = project-version_string
+	$name = "$name-$ver";
+
+	return wantarray ? ($name, $name) : $name;
+}
+
 sub git_snapshot {
 	my $format = $input_params{'snapshot_format'};
 	if (!@snapshot_fmts) {
@@ -5203,24 +5251,20 @@ sub git_snapshot {
 		die_error(400, 'Object is not a tree-ish');
 	}
 
-	my $name = $project;
-	$name =~ s,([^/])/*\.git$,$1,;
-	$name = basename($name);
-	my $filename = to_utf8($name);
-	$name =~ s/\047/\047\\\047\047/g;
-	my $cmd;
-	$filename .= "-$hash$known_snapshot_formats{$format}{'suffix'}";
-	$cmd = quote_command(
+	my ($name, $prefix) = snapshot_name($project, $hash);
+	my $filename = "$name$known_snapshot_formats{$format}{'suffix'}";
+	my $cmd = quote_command(
 		git_cmd(), 'archive',
 		"--format=$known_snapshot_formats{$format}{'format'}",
-		"--prefix=$name/", $hash);
+		"--prefix=$prefix/", $hash);
 	if (exists $known_snapshot_formats{$format}{'compressor'}) {
 		$cmd .= ' | ' . quote_command(@{$known_snapshot_formats{$format}{'compressor'}});
 	}
 
+	$filename =~ s/(["\\])/\\$1/g;
 	print $cgi->header(
 		-type => $known_snapshot_formats{$format}{'type'},
-		-content_disposition => 'inline; filename="' . "$filename" . '"',
+		-content_disposition => 'inline; filename="' . $filename . '"',
 		-status => '200 OK');
 
 	open my $fd, "-|", $cmd
diff --git a/t/t9502-gitweb-standalone-parse-output.sh b/t/t9502-gitweb-standalone-parse-output.sh
new file mode 100755
index 0000000..fbf4e26
--- /dev/null
+++ b/t/t9502-gitweb-standalone-parse-output.sh
@@ -0,0 +1,112 @@
+#!/bin/sh
+#
+# Copyright (c) 2009 Mark Rada
+#
+
+test_description='gitweb as standalone script (parsing script output).
+
+This test runs gitweb (git web interface) as a CGI script from the
+commandline, and checks that it produces the correct output, either
+in the HTTP header or the actual script output.'
+
+
+. ./gitweb-lib.sh
+
+# ----------------------------------------------------------------------
+# snapshot file name
+
+cat >>gitweb_config.perl <<\EOF
+
+$known_snapshot_formats{'tar'} = {
+	'display' => 'tar',
+	'type' => 'application/x-tar',
+	'suffix' => '.tar',
+	'format' => 'tar',
+};
+
+$feature{'snapshot'}{'default'} = ['tar'];
+EOF
+
+test_expect_success setup '
+	test_commit first foo &&
+	git branch xx/test &&
+	FULL_ID=$(git rev-parse --verify HEAD) &&
+	SHORT_ID=$(git rev-parse --verify --short=7 HEAD)
+'
+test_debug '
+	echo "FULL_ID  = $FULL_ID"
+	echo "SHORT_ID = $SHORT_ID"
+'
+
+test_expect_success 'snapshots: give full hash' '
+	gitweb_run "p=.git;a=snapshot;h=$FULL_ID;sf=tar" &&
+	grep ".git-$SHORT_ID.tar" gitweb.headers
+'
+test_debug 'cat gitweb.headers'
+test_debug 'cat gitweb.log'
+
+test_expect_success 'snapshots: give short hash' '
+	gitweb_run "p=.git;a=snapshot;h=$SHORT_ID;sf=tar" &&
+	grep ".git-$SHORT_ID.tar" gitweb.headers
+'
+test_debug 'cat gitweb.headers'
+
+test_expect_success 'snapshots: give almost full hash' '
+	ID=$(git rev-parse --short=30 HEAD) &&
+	gitweb_run "p=.git;a=snapshot;h=$ID;sf=tar" &&
+	grep ".git-$SHORT_ID.tar" gitweb.headers
+'
+test_debug 'cat gitweb.headers'
+
+test_expect_success 'snapshots: give HEAD' '
+	gitweb_run "p=.git;a=snapshot;h=HEAD;sf=tar" &&
+	grep ".git-HEAD-$SHORT_ID.tar" gitweb.headers
+'
+test_debug 'cat gitweb.headers'
+
+test_expect_success 'snapshots: give short branch name' '
+	gitweb_run "p=.git;a=snapshot;h=master;sf=tar" &&
+	ID=$(git rev-parse --verify --short=7 master) &&
+	grep ".git-master-$ID.tar" gitweb.headers
+'
+test_debug 'cat gitweb.headers'
+
+test_expect_success 'snapshots: give short tag name' '
+	gitweb_run "p=.git;a=snapshot;h=first;sf=tar" &&
+	ID=$(git rev-parse --verify --short=7 first) &&
+	grep ".git-first-$ID.tar" gitweb.headers
+'
+test_debug 'cat gitweb.headers'
+
+test_expect_success 'snapshots: give full branch name' '
+	gitweb_run "p=.git;a=snapshot;h=refs/heads/master;sf=tar" &&
+	ID=$(git rev-parse --verify --short=7 master) &&
+	grep ".git-master-$ID.tar" gitweb.headers
+'
+test_debug 'cat gitweb.headers'
+
+test_expect_success 'snapshots: give full tag name' '
+	gitweb_run "p=.git;a=snapshot;h=refs/tags/first;sf=tar" &&
+	grep ".git-first.tar" gitweb.headers
+'
+test_debug 'cat gitweb.headers'
+
+test_expect_success 'snapshots: give hierarchical branch name' '
+	gitweb_run "p=.git;a=snapshot;h=xx/test;sf=tar" &&
+	grep -v "filename=.*/" gitweb.headers
+'
+test_debug 'cat gitweb.headers'
+
+cat >expected <<EOF
+.git-HEAD-$SHORT_ID/
+.git-HEAD-$SHORT_ID/foo
+EOF
+test_expect_success 'snapshots: check prefix' '
+	gitweb_run "p=.git;a=snapshot;h=HEAD;sf=tar" &&
+	grep ".git-HEAD-$SHORT_ID.tar" gitweb.headers &&
+	"$TAR" tf gitweb.body >actual &&
+	test_cmp expected actual
+'
+test_debug 'cat gitweb.headers'
+
+test_done
-- 
1.6.5

^ permalink raw reply related

* [PATCH 0/2] gitweb: Smarter snapshot names
From: Jakub Narebski @ 2009-10-29 22:07 UTC (permalink / raw)
  To: git; +Cc: Mark Rada, Jakub Narebski

This series is proposed proposed replacement for a single top commit
in 'mr/gitweb-snapshot' branch, which was merged into pu.

Those two patches are meant to replace the following commit:
b411a7a (gitweb: append short hash ids to snapshot files, 2009-09-26)

Jakub Narebski (1):
  t/gitweb-lib.sh: Split gitweb output into headers and body

Mark Rada (1):
  gitweb: Smarter snapshot names
  (formerly: gitweb: append short hash ids to snapshot files)

Table of contents:
~~~~~~~~~~~~~~~~~~
 [PATCH 1/2] t/gitweb-lib.sh: Split gitweb output into headers and body
 [RFC/PATCH v6 2/2] gitweb: Smarter snapshot names

 gitweb/gitweb.perl                        |   76 +++++++++++++++----
 t/gitweb-lib.sh                           |    6 ++-
 t/t9502-gitweb-standalone-parse-output.sh |  112 +++++++++++++++++++++++++++++
 3 files changed, 177 insertions(+), 17 deletions(-)
 create mode 100755 t/t9502-gitweb-standalone-parse-output.sh

-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: [PATCH/RFC 2/2] Provide a build time default-editor setting
From: Junio C Hamano @ 2009-10-29 22:12 UTC (permalink / raw)
  To: Johannes Sixt
  Cc: Junio C Hamano, David Roundy, Jonathan Nieder, Ben Walton,
	GIT List
In-Reply-To: <200910292157.37474.j.sixt@viscovery.net>

Johannes Sixt <j.sixt@viscovery.net> writes:

> Yeah, whatever, I didn't take the time to think it through. But this may be an 
> opportunity to give some life back to the zombie that git-var currently is, 
> that is, to make it the plumbing that does value discovery for variables like 
> GIT_AUTHOR_INDENT, GIT_COMMITTER_IDENT, GIT_EDITOR, and perhaps also 
> GIT_PAGER.

Hmm, wouldn't it make even more sense to "run" them for the calling
Porcelain script?

A shell script Porcelain can already ". git-sh-setup" and say

	git_editor this-file

when it needs to spawn the editor of choice.  Your new plumbing support
could make the definition of git_editor in git-sh-setup.sh into something
like:

    git_editor() {
    	git var --run GIT_EDITOR "$@"
    }
    git_pager() {
    	git var --run GIT_PAGER "$@"
    }

^ permalink raw reply

* Re: [PATCH] mergetool--lib: add p4merge as a pre-configured mergetool option
From: Charles Bailey @ 2009-10-29 22:12 UTC (permalink / raw)
  To: Scott Chacon, Jay Soffian; +Cc: git list, Junio C Hamano, David Aguilar
In-Reply-To: <d411cc4a0910281439v3388c243v42b3700f73744623@mail.gmail.com>

On Wed, Oct 28, 2009 at 02:39:32PM -0700, Scott Chacon wrote:
> p4merge is now a built-in diff/merge tool.
> This adds p4merge to git-completion and updates
> the documentation to mention p4merge.
> 
> Signed-Off-By: Scott Chacon <schacon@gmail.com>
> ---
> 
> This is the same patch, but I tested it on Linux as well as Mac and it
> works fine as long as the [difftool|mergetool].p4merge.path configs
> are set or it's in your path.

I've examined the two patches and I feel more comfortable with
Scott's, mainly due to it's simplicity.

I'm not sure I understand why only p4merge on Mac OS X is special, we
don't seem to treat any other mergetool specially and we don't seem to
need absolute paths anywhere else.

If it's a Mac OS X only thing, can we (and should we) avoid special
treatment for p4merge on other platforms?

The only other question I have is what are the merits of using
/dev/null as the base vs. a second copy of the local version in the
baseless merge case? It's the only other difference between the two
p4merge patches that I noticed.

Perhaps we could consider having both p4merge and launchp4merge as
separate options?

-- 
Charles Bailey
http://ccgi.hashpling.plus.com/blog/

^ permalink raw reply

* [PATCH] Teach 'git merge' and 'git pull' the option --ff-only
From: Björn Gustavsson @ 2009-10-29 22:08 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

For convenience in scripts and aliases, add the option
--ff-only to only allow fast-forwards (and up-to-date,
despite the name).

Disallow combining --ff-only and --no-ff, since they
flatly contradict each other.

Allow all other options to be combined with --ff-only
(i.e. do not add any code to handle them specially),
including the following options:

* --strategy (one or more): As long as the chosen merge
  strategy results in up-to-date or fast-forward, the
  command will succeed.

* --squash: I cannot imagine why anyone would want to
  squash commits only if fast-forward is possible, but I
  also see no reason why it should not be allowed.

* --message: The message will always be ignored, but I see
  no need to explicitly disallow providing a redundant message.

Acknowledgements: I did look at Yuval Kogman's earlier
patch (107768 in gmane), mainly as shortcut to find my
way in the code, but I did not copy anything directly.

Signed-off-by: Björn Gustavsson <bgustavsson@gmail.com>
---
Here is a re-roll of my previous patch with improvements
suggested by Junio's review. Compared to the previous version,
I have changed the following:

* The documentation now clearly states that up-to-date
  is also allowed.

* The redundant and incorrect test to reject merge strategies
  that do not allow fast-forward has been removed (along
  with the corresponding test case).

* I have added some background on the design decisions to
  the commit message.

* I have added two test cases to test --squash in combination
  with --ff-only.

 Documentation/merge-options.txt |    5 ++++
 builtin-merge.c                 |   11 +++++++++-
 git-pull.sh                     |    7 ++++-
 t/t7600-merge.sh                |   41 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 61 insertions(+), 3 deletions(-)

diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
index adadf8e..27a9a84 100644
--- a/Documentation/merge-options.txt
+++ b/Documentation/merge-options.txt
@@ -60,6 +60,11 @@
 	a fast-forward, only update the branch pointer. This is
 	the default behavior of git-merge.
 
+--ff-only::
+	Refuse to merge and exit with a non-zero status unless the
+	current `HEAD` is already up-to-date or the merge can be
+	resolved as a fast-forward.
+
 -s <strategy>::
 --strategy=<strategy>::
 	Use the given merge strategy; can be supplied more than
diff --git a/builtin-merge.c b/builtin-merge.c
index b6b8428..5e8c4b5 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -43,6 +43,7 @@ static const char * const builtin_merge_usage[] = {
 
 static int show_diffstat = 1, option_log, squash;
 static int option_commit = 1, allow_fast_forward = 1;
+static int fast_forward_only;
 static int allow_trivial = 1, have_message;
 static struct strbuf merge_msg;
 static struct commit_list *remoteheads;
@@ -167,6 +168,8 @@ static struct option builtin_merge_options[] = {
 		"perform a commit if the merge succeeds (default)"),
 	OPT_BOOLEAN(0, "ff", &allow_fast_forward,
 		"allow fast forward (default)"),
+	OPT_BOOLEAN(0, "ff-only", &fast_forward_only,
+		"abort if fast forward is not possible"),
 	OPT_CALLBACK('s', "strategy", &use_strategies, "strategy",
 		"merge strategy to use", option_parse_strategy),
 	OPT_CALLBACK('m', "message", &merge_msg, "message",
@@ -874,6 +877,9 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		option_commit = 0;
 	}
 
+	if (!allow_fast_forward && fast_forward_only)
+		die("You cannot combine --no-ff with --ff-only.");
+
 	if (!argc)
 		usage_with_options(builtin_merge_usage,
 			builtin_merge_options);
@@ -1040,7 +1046,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		 * only one common.
 		 */
 		refresh_cache(REFRESH_QUIET);
-		if (allow_trivial) {
+		if (allow_trivial && !fast_forward_only) {
 			/* See if it is really trivial. */
 			git_committer_info(IDENT_ERROR_ON_NO_NAME);
 			printf("Trying really trivial in-index merge...\n");
@@ -1079,6 +1085,9 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		}
 	}
 
+	if (fast_forward_only)
+		die("Not possible to fast forward, aborting.");
+
 	/* We are going to make a new commit. */
 	git_committer_info(IDENT_ERROR_ON_NO_NAME);
 
diff --git a/git-pull.sh b/git-pull.sh
index fc78592..37f3d93 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -16,7 +16,8 @@ cd_to_toplevel
 test -z "$(git ls-files -u)" ||
 	die "You are in the middle of a conflicted merge."
 
-strategy_args= diffstat= no_commit= squash= no_ff= log_arg= verbosity=
+strategy_args= diffstat= no_commit= squash= no_ff= ff_only=
+log_arg= verbosity=
 curr_branch=$(git symbolic-ref -q HEAD)
 curr_branch_short=$(echo "$curr_branch" | sed "s|refs/heads/||")
 rebase=$(git config --bool branch.$curr_branch_short.rebase)
@@ -45,6 +46,8 @@ do
 		no_ff=--ff ;;
 	--no-ff)
 		no_ff=--no-ff ;;
+	--ff-only)
+		ff_only=--ff-only ;;
 	-s=*|--s=*|--st=*|--str=*|--stra=*|--strat=*|--strate=*|\
 		--strateg=*|--strategy=*|\
 	-s|--s|--st|--str|--stra|--strat|--strate|--strateg|--strategy)
@@ -215,5 +218,5 @@ merge_name=$(git fmt-merge-msg $log_arg <"$GIT_DIR/FETCH_HEAD") || exit
 test true = "$rebase" &&
 	exec git-rebase $diffstat $strategy_args --onto $merge_head \
 	${oldremoteref:-$merge_head}
-exec git-merge $diffstat $no_commit $squash $no_ff $log_arg $strategy_args \
+exec git-merge $diffstat $no_commit $squash $no_ff $ff_only $log_arg $strategy_args \
 	"$merge_name" HEAD $merge_head $verbosity
diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index e5b210b..57f6d2b 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -243,6 +243,16 @@ test_expect_success 'merge c0 with c1' '
 
 test_debug 'gitk --all'
 
+test_expect_success 'merge c0 with c1 with --ff-only' '
+	git reset --hard c0 &&
+	git merge --ff-only c1 &&
+	git merge --ff-only HEAD c0 c1 &&
+	verify_merge file result.1 &&
+	verify_head "$c1"
+'
+
+test_debug 'gitk --all'
+
 test_expect_success 'merge c1 with c2' '
 	git reset --hard c1 &&
 	test_tick &&
@@ -263,6 +273,14 @@ test_expect_success 'merge c1 with c2 and c3' '
 
 test_debug 'gitk --all'
 
+test_expect_success 'failing merges with --ff-only' '
+	git reset --hard c1 &&
+	test_tick &&
+	test_must_fail git merge --ff-only c2 &&
+	test_must_fail git merge --ff-only c3 &&
+	test_must_fail git merge --ff-only c2 c3
+'
+
 test_expect_success 'merge c0 with c1 (no-commit)' '
 	git reset --hard c0 &&
 	git merge --no-commit c1 &&
@@ -303,6 +321,17 @@ test_expect_success 'merge c0 with c1 (squash)' '
 
 test_debug 'gitk --all'
 
+test_expect_success 'merge c0 with c1 (squash, ff-only)' '
+	git reset --hard c0 &&
+	git merge --squash --ff-only c1 &&
+	verify_merge file result.1 &&
+	verify_head $c0 &&
+	verify_no_mergehead &&
+	verify_diff squash.1 .git/SQUASH_MSG "[OOPS] bad squash message"
+'
+
+test_debug 'gitk --all'
+
 test_expect_success 'merge c1 with c2 (squash)' '
 	git reset --hard c1 &&
 	git merge --squash c2 &&
@@ -314,6 +343,13 @@ test_expect_success 'merge c1 with c2 (squash)' '
 
 test_debug 'gitk --all'
 
+test_expect_success 'unsuccesful merge of c1 with c2 (squash, ff-only)' '
+	git reset --hard c1 &&
+	test_must_fail git merge --squash --ff-only c2
+'
+
+test_debug 'gitk --all'
+
 test_expect_success 'merge c1 with c2 and c3 (squash)' '
 	git reset --hard c1 &&
 	git merge --squash c2 c3 &&
@@ -432,6 +468,11 @@ test_expect_success 'combining --squash and --no-ff is refused' '
 	test_must_fail git merge --no-ff --squash c1
 '
 
+test_expect_success 'combining --ff-only and --no-ff is refused' '
+	test_must_fail git merge --ff-only --no-ff c1 &&
+	test_must_fail git merge --no-ff --ff-only c1
+'
+
 test_expect_success 'merge c0 with c1 (ff overrides no-ff)' '
 	git reset --hard c0 &&
 	git config branch.master.mergeoptions "--no-ff" &&
-- 
1.6.5.1.69.g36942

^ permalink raw reply related

* Add gitk-hit/po Hungarian translation
From: Laszlo Papp @ 2009-10-29 22:40 UTC (permalink / raw)
  To: git

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

Hello,

See the Hungarian translation for gitk-git in the attachment!

Thanks in advance

Best Regards,
Laszlo Papp

[-- Attachment #2: hu.po.tar.bz2 --]
[-- Type: application/x-bzip2, Size: 6776 bytes --]

^ permalink raw reply

* Re: [PATCH] More precise description of 'git describe --abbrev'
From: Junio C Hamano @ 2009-10-29 22:47 UTC (permalink / raw)
  To: Gisle Aas; +Cc: git
In-Reply-To: <b48ea8a00910291438r8b66a0fq9e821393ecfff0bf@mail.gmail.com>

Gisle Aas <gisle@aas.no> writes:

> Also make the examples show what 'git describe' actually outputs
> currently.  I guess the default --abbrev value has been changed from 4
> to 7 at some point.

Some are good changes, but I do not think the example with --abbrev=4 is.

$ git describe 975bf9cf5ad5d440f98f464ae8124609a4835ce1
v1.3.2-216-g975bf9c
$ git describe 975b31dc6e12fba8f7b067ddbe32230995e05400
v1.0.0-21-g975b31d

Next time somebody adds a new object whose name happens to begin with
975b3 you would need to update the example output.

> Signed-off-by: Gisle Aas <gisle@aas.no>
> ---
>  Documentation/git-describe.txt |   12 +++++++-----
>  1 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/git-describe.txt b/Documentation/git-describe.txt
> index b231dbb..743eb95 100644
> --- a/Documentation/git-describe.txt
> +++ b/Documentation/git-describe.txt
> @@ -44,7 +44,9 @@ OPTIONS
>
>  --abbrev=<n>::
>  	Instead of using the default 7 hexadecimal digits as the
> -	abbreviated object name, use <n> digits.
> +	abbreviated object name, use <n> digits or as many digits
> +	are needed to form a unique object name.  An <n> of 0
> +	will suppress long format, only showing the closest tag.
>
>  --candidates=<n>::
>  	Instead of considering only the 10 most recent tags as
> @@ -68,8 +70,8 @@ OPTIONS
>  	This is useful when you want to see parts of the commit object name
>  	in "describe" output, even when the commit in question happens to be
>  	a tagged version.  Instead of just emitting the tag name, it will
> -	describe such a commit as v1.2-0-deadbeef (0th commit since tag v1.2
> -	that points at object deadbeef....).
> +	describe such a commit as v1.2-0-gdeadbee (0th commit since tag v1.2
> +	that points at object deadbee....).
>
>  --match <pattern>::
>  	Only consider tags matching the given pattern (can be used to avoid
> @@ -106,10 +108,10 @@ With --all, the command can use branch heads as
> references, so
>  the output shows the reference path as well:
>
>  	[torvalds@g5 git]$ git describe --all --abbrev=4 v1.0.5^2
> -	tags/v1.0.0-21-g975b
> +	tags/v1.0.0-21-g975b3
>
>  	[torvalds@g5 git]$ git describe --all HEAD^
> -	heads/lt/describe-7-g975b
> +	heads/lt/describe-7-g975b31d
>
>  With --abbrev set to 0, the command can be used to find the
>  closest tagname without any suffix:
> -- 
> 1.6.2.95.g934f7

^ permalink raw reply

* Re: Tracking a remote branch in git
From: Tim Mazid @ 2009-10-30  0:26 UTC (permalink / raw)
  To: git
In-Reply-To: <26119537.post@talk.nabble.com>



Sips wrote:
> 
> Then I created a branch in the repository on the removable drive W and
> named it newfeature. The question is: How can I work with this branch on
> the local repository as well? I mean, I need the same 'relationship'
> between branches as in case of the master branch.
> 
> ...
> 
> I mean, I am able to track the master branch from the remote repository.
> Why can't I track some other branch as well? Any ideas what I'm doing
> wrong?
> 

Have you tried doing 'git fetch'? That should get and track all new remote
branches automatically.

Once you have a remote branch, if you've deleted the local branch, you can
just do 'git branch LOCAL REMOTE', and that should set up a tracking branch.

If this doesn't work for you, check your git version with 'git --version'.
It works for v1.6.5.2.

Good luck,
Tim.
-- 
View this message in context: http://www.nabble.com/Tracking-a-remote-branch-in-git-tp26119537p26123156.html
Sent from the git mailing list archive at Nabble.com.

^ permalink raw reply

* Re: [PATCH] mergetool--lib: add p4merge as a pre-configured mergetool  option
From: Jay Soffian @ 2009-10-30  0:47 UTC (permalink / raw)
  To: Charles Bailey; +Cc: Scott Chacon, git list, Junio C Hamano, David Aguilar
In-Reply-To: <20091029221234.GB32590@hashpling.org>

On Thu, Oct 29, 2009 at 6:12 PM, Charles Bailey <charles@hashpling.org> wrote:
> I'm not sure I understand why only p4merge on Mac OS X is special, we
> don't seem to treat any other mergetool specially and we don't seem to
> need absolute paths anywhere else.

On other platforms, the merge tool is very likely to be in your PATH.

On OS X, p4merge is going to be installed as part of an application
bundle (/Applications/p4merge.app or $HOME/Applications/p4merge.app).
This is virtually never going to be in a user's PATH.

So in order to provide equivalent behavior for OS X as Linux (i.e., so
that you can just specify p4merge as the mergetool without having to
provide it's path), we need to look in these additional locations.

> If it's a Mac OS X only thing, can we (and should we) avoid special
> treatment for p4merge on other platforms?

It is a Mac OS X only thing. Yes, we could avoid looking in these
locations on other platforms, but why? Using type to look for the
executable is virtually no cost. The alternative (calling uname to
determine the platform) requires running a separate process.

> The only other question I have is what are the merits of using
> /dev/null as the base vs. a second copy of the local version in the
> baseless merge case? It's the only other difference between the two
> p4merge patches that I noticed.

p4merge's argument handling is stupid, you need to pass it a dummy
argument in some cases. A second copy of the local version is probably
better than /dev/null. Actually, I think just passing it an empty
argument ("") works too.

> Perhaps we could consider having both p4merge and launchp4merge as
> separate options?

I think that would be over-engineered. Decide on one or the other.
They have slightly different semantics as I've previously described.
Personally I think calling launchp4merge provides a more Mac-like
behavior, but honestly it doesn't make much difference.

j.

^ permalink raw reply

* Re: [PATCH] mergetool--lib: add p4merge as a pre-configured mergetool  option
From: Markus Heidelberg @ 2009-10-30  1:02 UTC (permalink / raw)
  To: Jay Soffian
  Cc: Charles Bailey, Scott Chacon, git list, Junio C Hamano,
	David Aguilar
In-Reply-To: <76718490910291747l165baf49tab781727d010610a@mail.gmail.com>

Jay Soffian, 30.10.2009:
> On Thu, Oct 29, 2009 at 6:12 PM, Charles Bailey <charles@hashpling.org> wrote:
> > I'm not sure I understand why only p4merge on Mac OS X is special, we
> > don't seem to treat any other mergetool specially and we don't seem to
> > need absolute paths anywhere else.
> 
> On other platforms, the merge tool is very likely to be in your PATH.

He didn't mean p4merge on other platforms, but other merge tools on Mac
OS X. What about all the other merge tools already in mergetool--lib?
Should they get special handling, too?

> On OS X, p4merge is going to be installed as part of an application
> bundle (/Applications/p4merge.app or $HOME/Applications/p4merge.app).
> This is virtually never going to be in a user's PATH.
> 
> So in order to provide equivalent behavior for OS X as Linux (i.e., so
> that you can just specify p4merge as the mergetool without having to
> provide it's path), we need to look in these additional locations.

And for Windows we could add C:\Program Files\MergeToolX\tool.exe for
every merge tool.

But where will we end?

Markus

^ permalink raw reply

* [PATCH] RFC Allow case insensitive search flag with git-grep for fixed-strings
From: Brian Collins @ 2009-10-30  1:21 UTC (permalink / raw)
  To: git

You will have to excuse me, this is my first patch and I don't know if  
this is the right place to post this. Apologies in advance if I'm in  
the wrong place.

git-grep currently throws an error when you combine the -F and -i  
flags. This isn't in line with how GNU grep handles it. This patch  
allows the simultaneous use of those flags.

---
  builtin-grep.c |    8 +++++---
  grep.c         |   16 ++++++++++++----
  grep.h         |    2 ++
  3 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/builtin-grep.c b/builtin-grep.c
index 761799d..c73f05b 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -367,7 +367,7 @@ static int external_grep(struct grep_opt *opt,  
const char **paths, int cached)
  		push_arg("-h");
  	if (opt->regflags & REG_EXTENDED)
  		push_arg("-E");
-	if (opt->regflags & REG_ICASE)
+	if (opt->caseless)
  		push_arg("-i");
  	if (opt->binary == GREP_BINARY_NOMATCH)
  		push_arg("-I");
@@ -706,8 +706,8 @@ int cmd_grep(int argc, const char **argv, const  
char *prefix)
  		OPT_GROUP(""),
  		OPT_BOOLEAN('v', "invert-match", &opt.invert,
  			"show non-matching lines"),
-		OPT_BIT('i', "ignore-case", &opt.regflags,
-			"case insensitive matching", REG_ICASE),
+		OPT_BOOLEAN('i', "ignore-case", &opt.caseless,
+			"case insensitive matching"),
  		OPT_BOOLEAN('w', "word-regexp", &opt.word_regexp,
  			"match patterns only at word boundaries"),
  		OPT_SET_INT('a', "text", &opt.binary,
@@ -830,6 +830,8 @@ int cmd_grep(int argc, const char **argv, const  
char *prefix)
  		external_grep_allowed = 0;
  	if (!opt.pattern_list)
  		die("no pattern given.");
+	if (!opt.fixed && opt.caseless)
+		opt.regflags |= REG_ICASE;
  	if ((opt.regflags != REG_NEWLINE) && opt.fixed)
  		die("cannot mix --fixed-strings and regexp");
  	compile_grep_patterns(&opt);
diff --git a/grep.c b/grep.c
index 5d162da..d8f14be 100644
--- a/grep.c
+++ b/grep.c
@@ -41,7 +41,8 @@ static void compile_regexp(struct grep_pat *p,  
struct grep_opt *opt)
  	int err;

  	p->word_regexp = opt->word_regexp;
-
+	p->caseless = opt->caseless;
+	
  	if (opt->fixed || is_fixed(p->pattern))
  		p->fixed = 1;
  	if (opt->regflags & REG_ICASE)
@@ -262,9 +263,16 @@ static void show_name(struct grep_opt *opt, const  
char *name)
  	printf("%s%c", name, opt->null_following_name ? '\0' : '\n');
  }

-static int fixmatch(const char *pattern, char *line, regmatch_t *match)
+
+static int fixmatch(const char *pattern, char *line, int caseless,  
regmatch_t *match)
  {
-	char *hit = strstr(line, pattern);
+	char *hit;
+	if (caseless) {
+		hit = strcasestr(line, pattern);
+	} else {
+		hit = strstr(line, pattern);
+	}
+	
  	if (!hit) {
  		match->rm_so = match->rm_eo = -1;
  		return REG_NOMATCH;
@@ -326,7 +334,7 @@ static int match_one_pattern(struct grep_pat *p,  
char *bol, char *eol,

   again:
  	if (p->fixed)
-		hit = !fixmatch(p->pattern, bol, pmatch);
+		hit = !fixmatch(p->pattern, bol, p->caseless, pmatch);
  	else
  		hit = !regexec(&p->regexp, bol, 1, pmatch, eflags);

diff --git a/grep.h b/grep.h
index f6eecc6..24b7d44 100644
--- a/grep.h
+++ b/grep.h
@@ -32,6 +32,7 @@ struct grep_pat {
  	enum grep_header_field field;
  	regex_t regexp;
  	unsigned fixed:1;
+	unsigned caseless:1;
  	unsigned word_regexp:1;
  };

@@ -64,6 +65,7 @@ struct grep_opt {
  	regex_t regexp;
  	int linenum;
  	int invert;
+	int caseless;
  	int status_only;
  	int name_only;
  	int unmatch_name_only;
-- 
1.6.4.4

^ permalink raw reply related

* Re: [PATCH/RFC 2/2] Provide a build time default-editor setting
From: David Roundy @ 2009-10-30  2:21 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Sixt, Jonathan Nieder, Ben Walton, GIT List
In-Reply-To: <7vfx916ea6.fsf@alter.siamese.dyndns.org>

On Thu, Oct 29, 2009 at 6:12 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Johannes Sixt <j.sixt@viscovery.net> writes:
>
>> Yeah, whatever, I didn't take the time to think it through. But this may be an
>> opportunity to give some life back to the zombie that git-var currently is,
>> that is, to make it the plumbing that does value discovery for variables like
>> GIT_AUTHOR_INDENT, GIT_COMMITTER_IDENT, GIT_EDITOR, and perhaps also
>> GIT_PAGER.
>
> Hmm, wouldn't it make even more sense to "run" them for the calling
> Porcelain script?

That was what I had been thinking.  That way the caller doesn't need
to know whether it may be a space-containing absolute path or an
executable with flags, as long as git knows what to do.

David

^ permalink raw reply

* Re: [PATCH] mergetool--lib: add p4merge as a pre-configured mergetool  option
From: Jay Soffian @ 2009-10-30  3:00 UTC (permalink / raw)
  To: markus.heidelberg
  Cc: Charles Bailey, Scott Chacon, git list, Junio C Hamano,
	David Aguilar
In-Reply-To: <200910300202.02016.markus.heidelberg@web.de>

On Thu, Oct 29, 2009 at 9:02 PM, Markus Heidelberg
<markus.heidelberg@web.de> wrote:
> He didn't mean p4merge on other platforms, but other merge tools on Mac
> OS X. What about all the other merge tools already in mergetool--lib?
> Should they get special handling, too?

If someone wants to scratch that itch, then yes. The default diff tool
for OS X has its helper already in /usr/bin (opendiff). p4merge is
arguably a better merge tool, and it installs as an app bundle in
/Applications. I'm not sure about the other diff tools, I haven't
looked.

> And for Windows we could add C:\Program Files\MergeToolX\tool.exe for
> every merge tool.

If it makes those tools easier to use with git, and if someone on
Windows wants to scratch that itch, then yes, we should.

> But where will we end?

I don't understand this argument. It's a few lines of code to make git
a little friendlier. We end when folks stop contributing patches
because either no one cares of there's nothing left to improve.

j.

^ permalink raw reply

* Re: [Vcs-fast-import-devs] What's cooking in git.git (Oct 2009, #01; Wed, 07)
From: Ian Clatworthy @ 2009-10-30  3:50 UTC (permalink / raw)
  To: Sverre Rabbelier
  Cc: Shawn O. Pearce, vcs-fast-import-devs, git, Johannes Schindelin
In-Reply-To: <fabb9a1e0910081058m59527600o392a6b438b18512e@mail.gmail.com>

Sverre Rabbelier wrote:
> Heya,
> 
> [edited Shawn's message somewhat to be more relevant to vcs-fast-import-dev]

Thanks Sverre. Before I start, sorry for taking so long to reply to this.

> On Thu, Oct 8, 2009 at 19:39, Shawn O. Pearce <spearce@spearce.org> wrote:
>> IIRC my problem with options was we weren't enforcing them, and yet
>> they were necessary for a successful import, e.g. import-marks or
>> export-marks.  A minor error could cause a successful looking import
>> that is wrong due to the marks being messed up, or not saved out.
>>
>> So I was leaning towards making these features, but then they
>> aren't necessarily compatible with the other fast-import tools.

My strong preference is for:

* feature = anything impacting semantics
* option = tool-specific with no impact on semantics permitted.

Both features and options ought to OS independent (where possible).

>> I think we want to declare features for import-marks and export-marks:
>>
>>  feature import-marks=in.marks
>>  feature export-marks=out.marks
>>
>> and define these as paths to local files which store a VCS specific
>> formatted mapping of fast-import mark numbers to VCS labels.

+1 to making these features and to tightening up the semantics so we can
reliably use them across tools. Explicitly specifying the local path
names worries me though. Consider someone using fastimport tools to
maintain multiple mirrors in different tools:

1. Step 1 is fast-export from tool A
2. Step 2 is fast-import into tool B
3. Step 3 is fast-import into tool C

What should the stream look like then? Does it need to change if we want
an additional mirror in tool D? (Note that the mark files will need to
be reused to transfer changes back to the master.)

>> Other options that are clearly git should be declared as:
>>
>>  option git max-pack-size=2048
>>
>> with the meaning of option being declared something like:
>>
>>  If the parsing VCS name appears as the first argument, the parsing
>>  VCS must recognize and support the supplied option, and if not
>>  recognized or not supported must abort parsing altogether.
>>
>>  If the parsing VCS name is not the first argument, it must entirely
>>  ignore the option command and not try to process its contents.

+1. By forcing tools to know about options specific to them, we avoid a
range of bugs processing newer streams with older tools.

> I think it makes to ignore options that are not for our vcs, as long
> as options that change import behavior (such as marks, date-format)
> are combined with, say, 'feature tool=git'. This way we can be sure
> that when outputting out a vcs specific stream, it is only parsed by
> that vcs.

I don't think options should be permitted to change import behavior. In
other words, we should actively discourage vcs-specific streams. Any VCS
using features has a (moral) responsibility IMO to at least define those
publicly. Here's a poor start (EBNF syntax would be far better than just
text) on the Bazaar side:
http://doc.bazaar-vcs.org/migration/en/data-migration/fast-export.html#interoperability.

Maybe we need a central wiki page (say) where these can be registered?
I'd offer to setup a "fastimport" web site in a Bazaar branch and track
feature specification bugs in Launchpad but maybe a wiki page would be a
little more neutral ground. :-) :-)

Ian C.

^ permalink raw reply

* Re: [PATCH 02/19] Allow fetch to modify refs
From: Daniel Barkalow @ 2009-10-30  5:56 UTC (permalink / raw)
  To: Sverre Rabbelier; +Cc: Git List, Johannes Schindelin, Johan Herland
In-Reply-To: <1256839287-19016-3-git-send-email-srabbelier@gmail.com>

On Thu, 29 Oct 2009, Sverre Rabbelier wrote:

> From: Daniel Barkalow <barkalow@iabervon.org>
> 
> This allows the transport to use the null sha1 for a ref reported to
> be present in the remote repository to indicate that a ref exists but
> its actual value is presently unknown and will be set if the objects
> are fetched.
> 
> Also adds documentation to the API to specify exactly what the methods
> should do and how they should interpret arguments.
> 
> Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
> Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
> ---
> 
> 	Signed off by me because I rebased it on master and fixed the
> 	conflicts with nico's patch.
> 
>  builtin-clone.c    |    5 +++--
>  transport-helper.c |    4 ++--
>  transport.c        |   13 +++++++------
>  transport.h        |   41 +++++++++++++++++++++++++++++++++++++++--
>  4 files changed, 51 insertions(+), 12 deletions(-)
> 
> diff --git a/builtin-clone.c b/builtin-clone.c
> index 5762a6f..0042bee 100644
> --- a/builtin-clone.c
> +++ b/builtin-clone.c
> @@ -526,8 +526,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
>  
>  		refs = transport_get_remote_refs(transport);
>  		if (refs) {
> -			mapped_refs = wanted_peer_refs(refs, refspec);
> -			transport_fetch_refs(transport, mapped_refs);
> +			struct ref *ref_cpy = wanted_peer_refs(refs, refspec);
> +			mapped_refs = ref_cpy;
> +			transport_fetch_refs(transport, ref_cpy);

Just drop this hunk; the change doesn't actually do anything. Otherwise, 
the patch matches what I have.

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

* Re: [UNSTABLE PATCH 03/19] Allow programs to not depend on remotes having urls
From: Daniel Barkalow @ 2009-10-30  6:02 UTC (permalink / raw)
  To: Sverre Rabbelier; +Cc: Git List, Johannes Schindelin, Johan Herland
In-Reply-To: <1256839287-19016-4-git-send-email-srabbelier@gmail.com>

On Thu, 29 Oct 2009, Sverre Rabbelier wrote:

> From: Daniel Barkalow <barkalow@iabervon.org>
> 
> For fetch and ls-remote, which use the first url of a remote, have
> transport_get() determine this by passing a remote and passing NULL
> for the url. For push, which uses every url of a remote, use each url
> in turn if there are any, and use NULL if there are none.
> 
> This will allow the transport code to do something different if the
> location is not specified with a url.
> 
> Also, have the message for a fetch say "foreign" if there is no url.
> 
> Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
> Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
> ---
> 
> 	I rebased this on master and had major conflicts with the
> 	recent 'advice' series, Daniel, please have a look at this to
> 	see whether it is sane at all.

Yup, this is right. Perhaps I should try to get the refactor in 
builtin-push to use push_with_options() without any behavior change into 
master ahead of the rest of this series, to avoid having to deal with this 
sort of conflict every time someone touches this section of code, which 
has happened several times recently now. Anyway, you correctly understood 
the intended change here.

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

* Re: git svn show-ignore is excrutiatingly slow
From: Eric Wong @ 2009-10-30  6:39 UTC (permalink / raw)
  To: Adam Spiers; +Cc: git mailing list
In-Reply-To: <20091028174307.GA5691@atlantic.linksys.moosehall>

Adam Spiers <git@adamspiers.org> wrote:
> Mail-Followup-To: git mailing list <git@vger.kernel.org>

Hi Adam, MFT is frowned upon here.

> Something is badly wrong here ...
> 
> $ cd $svn_wd
> $ time svn propget -R svn:ignore >/dev/null
> svn propget -R svn:ignore > /dev/null  0.28s user 0.20s system 98% cpu 0.490 total
> $ cd $git_wd
> $ time git svn show-ignore > show-ignore.out
> git svn show-ignore > show-ignore.out  20.52s user 33.69s system 1% cpu 1:23:42.17 total
> 
> That's 10,000 times slower for what is effectively the same source
> tree!  Admittedly the svn propget was a "warm" run and took longer the
> first time around, but even so there are several orders of magnitude
> difference.

It's the difference between reading locally vs remotely.  git svn always
looks at the remote repository for this information.  In your example,
svn was looking at the working copy where it already has that
information in an easily accessible form.

Try running svn against your repository URL instead for a comparison:

  time svn propget -R svn:ignore $SVN_URL >/dev/null

> I had a quick look at the code and it seemed to be doing the svn tree
> recursion itself via Git::SVN::prop_walk(), which might explain why.
> However I did not have time to dig deeper, so would welcome any ideas.

It would be possible to reconstruct the information given untouched
copies of unhandled.log inside $GIT_DIR/svn/**.  On the other hand, it
does require running through the history of the project and I don't
think it's worth it for an operation that should be rarely needed.  I
designed the output of "git svn show-ignore" be stored in
$GIT_DIR/info/exclude

-- 
Eric Wong

^ permalink raw reply

* Re: [RFC PATCH 06/19] Factor ref updating out of fetch_with_import
From: Daniel Barkalow @ 2009-10-30  7:10 UTC (permalink / raw)
  To: Sverre Rabbelier; +Cc: Git List, Johannes Schindelin, Johan Herland
In-Reply-To: <1256839287-19016-7-git-send-email-srabbelier@gmail.com>

On Thu, 29 Oct 2009, Sverre Rabbelier wrote:

> Also allow the new update_refs to actually update the refs set, this
> way the remote helper can set the value of previously unknown refs.
> 
> Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
> ---
> 
> 	Daniel, if we can get wanted_peer_refs to keep HEAD as a
> 	wanted ref somehow this patch could be a lot simpler.

Actually, I think the problem is that builtin-clone will always default to 
setting up a refspec of "refs/heads/*:refs/remotes/origin/*", but that 
doesn't actually make any sense if the source repository isn't presented 
as having refs names like "refs/heads/*". The immediate problem that you 
don't get HEAD because it's a symref to something outside the pattern is 
somewhat secondary to the general problem that you don't get anything at 
all, because everything's outside the pattern.

I don't really think that presenting the real refs in refs/<vcs>/*, and 
having the list give symrefs to them from refs/heads/* and refs/tags/* 
really makes sense; I think it would be better to rework fetch_with_import 
and the list provided by a foreign vcs helper such that it can present 
refs with normal names (refs/heads/*, refs/tags/*, etc) if the foreign vcs 
has standards that correspond to branches and tags. Then "clone" would 
work normally.

Or, perhaps, there should be some other way of allowing git users to take 
advantage of foreign vcs standards; I don't have a good perspective on 
this, since I only presently use git and a foreign vcs without any useful
standards. But I think it should be such that the transport user sees as 
close as possible to a native git layout, and clone continues to rely on 
the layout being normal, instead of presenting a weird layout mapped into 
a normal layout or something like that and working around transport users 
not expecting this.

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

* Re: [PATCH] More precise description of 'git describe --abbrev'
From: Gisle Aas @ 2009-10-30  8:01 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vws2d4y3d.fsf@alter.siamese.dyndns.org>

On Thu, Oct 29, 2009 at 23:47, Junio C Hamano <gitster@pobox.com> wrote:
> Gisle Aas <gisle@aas.no> writes:
>
>> Also make the examples show what 'git describe' actually outputs
>> currently.  I guess the default --abbrev value has been changed from 4
>> to 7 at some point.
>
> Some are good changes, but I do not think the example with --abbrev=4 is.
>
> $ git describe 975bf9cf5ad5d440f98f464ae8124609a4835ce1
> v1.3.2-216-g975bf9c
> $ git describe 975b31dc6e12fba8f7b067ddbe32230995e05400
> v1.0.0-21-g975b31d
>
> Next time somebody adds a new object whose name happens to begin with
> 975b3 you would need to update the example output.

Yeah, I know, but I don't think that's a big deal.  So do you want an
updated patch for that?  We could either simply remove this example or
make it use --abbrev=10 or something like that.

--Gisle


>
>> Signed-off-by: Gisle Aas <gisle@aas.no>
>> ---
>>  Documentation/git-describe.txt |   12 +++++++-----
>>  1 files changed, 7 insertions(+), 5 deletions(-)
>>
>> diff --git a/Documentation/git-describe.txt b/Documentation/git-describe.txt
>> index b231dbb..743eb95 100644
>> --- a/Documentation/git-describe.txt
>> +++ b/Documentation/git-describe.txt
>> @@ -44,7 +44,9 @@ OPTIONS
>>
>>  --abbrev=<n>::
>>       Instead of using the default 7 hexadecimal digits as the
>> -     abbreviated object name, use <n> digits.
>> +     abbreviated object name, use <n> digits or as many digits
>> +     are needed to form a unique object name.  An <n> of 0
>> +     will suppress long format, only showing the closest tag.
>>
>>  --candidates=<n>::
>>       Instead of considering only the 10 most recent tags as
>> @@ -68,8 +70,8 @@ OPTIONS
>>       This is useful when you want to see parts of the commit object name
>>       in "describe" output, even when the commit in question happens to be
>>       a tagged version.  Instead of just emitting the tag name, it will
>> -     describe such a commit as v1.2-0-deadbeef (0th commit since tag v1.2
>> -     that points at object deadbeef....).
>> +     describe such a commit as v1.2-0-gdeadbee (0th commit since tag v1.2
>> +     that points at object deadbee....).
>>
>>  --match <pattern>::
>>       Only consider tags matching the given pattern (can be used to avoid
>> @@ -106,10 +108,10 @@ With --all, the command can use branch heads as
>> references, so
>>  the output shows the reference path as well:
>>
>>       [torvalds@g5 git]$ git describe --all --abbrev=4 v1.0.5^2
>> -     tags/v1.0.0-21-g975b
>> +     tags/v1.0.0-21-g975b3
>>
>>       [torvalds@g5 git]$ git describe --all HEAD^
>> -     heads/lt/describe-7-g975b
>> +     heads/lt/describe-7-g975b31d
>>
>>  With --abbrev set to 0, the command can be used to find the
>>  closest tagname without any suffix:
>> --
>> 1.6.2.95.g934f7
>

^ permalink raw reply

* Re: [PATCH 10/19] Allow helpers to request marks for fast-import
From: Johan Herland @ 2009-10-30  8:21 UTC (permalink / raw)
  To: Sverre Rabbelier; +Cc: Git List, Johannes Schindelin, Daniel Barkalow
In-Reply-To: <1256839287-19016-11-git-send-email-srabbelier@gmail.com>

On Thursday 29 October 2009, Sverre Rabbelier wrote:
> From: Johan Herland <johan@herland.net>
> 
> The 'marks' capability is reported by the remote helper if it requires
> the fast-import marks database to loaded/saved by any git-fast-import
> process that is provided by the transport machinery. The feature is
> advertised along with exactly one argument: the location of the file
> containing the marks database.
> 
> Signed-off-by: Johan Herland <johan@herland.net>
> Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
> ---
> 
> 	Unchanged.

Please drop this patch from the series. The functionality is not needed, 
since we'll use the fast-import "option" command from the sr/gfi-options 
series instead.


...Johan

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

^ permalink raw reply

* Re: keeping track of where a patch begins
From: Pascal Obry @ 2009-10-30  7:25 UTC (permalink / raw)
  To: Thomas Rast; +Cc: Junio C Hamano, Nicolas Pitre, E R, git, Jeff King
In-Reply-To: <200910221027.32739.trast@student.ethz.ch>


Le 22/10/2009 10:27, Thomas Rast a écrit :
> I think this not only changes the model of branches, but also commits,
> to some extent.  Currently, commit have no intrinsic branch
> membership; if you say
>
>    git branch foo bar
>
> you cannot distinguish whether the commits on 'bar' were created on
> 'foo' or on 'bar'.  (By git's means; of course the decision would
> favour 'master' if I had used that instead.)

I have been looking for a way to know that. I've even post a question 
about this on this mailing-list long time ago IIRC.

To me there is case where it is important to know which are the commits 
done on a topic branch for example. When working on multiple topic it is 
difficult to remember which commits have been done on this specific 
branch. This is needed to rebase onto:

    $ git rebase --onto somebranch <topic_base> <topic_head>

A common idiom, but one as to think hard (& right) to properly get the 
topic_base today.

Just my 2 cents!

Pascal.

-- 

--|------------------------------------------------------
--| Pascal Obry                           Team-Ada Member
--| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE
--|------------------------------------------------------
--|    http://www.obry.net  -  http://v2p.fr.eu.org
--| "The best way to travel is by means of imagination"
--|
--| gpg --keyserver keys.gnupg.net --recv-key F949BD3B

^ permalink raw reply

* Re: [PATCH 12/19] 1/2: Add Python support library for CVS remote helper
From: Johan Herland @ 2009-10-30  8:33 UTC (permalink / raw)
  To: Sverre Rabbelier
  Cc: Git List, Johannes Schindelin, Daniel Barkalow, David Aguilar
In-Reply-To: <1256839287-19016-13-git-send-email-srabbelier@gmail.com>

On Thursday 29 October 2009, Sverre Rabbelier wrote:
> From: Johan Herland <johan@herland.net>
> 
> This patch introduces parts of a Python package called "git_remote_cvs"
> containing the building blocks of the CVS remote helper.
> The CVS remote helper itself is NOT part of this patch.
> 
> This patch has been improved by the following contributions:
> - David Aguilar: Lots of Python coding style fixes
> 
> Cc: David Aguilar <davvid@gmail.com>
> Signed-off-by: Johan Herland <johan@herland.net>
> Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
> ---
> 
> 	This has my patch to util.py squashed in.

Why? Or: why that one, and not the others? Also, you might want to mention 
your contribution in the commit message itself.

> diff --git a/git_remote_cvs/util.py b/git_remote_cvs/util.py
> new file mode 100644
> index 0000000..d3ca487
> --- /dev/null
> +++ b/git_remote_cvs/util.py
> @@ -0,0 +1,194 @@
[snip]
> +
> +def notify(msg, *args):
> +	"""Print a message to stderr."""
> +	print >> sys.stderr, msg % args
> +
> +def debug (msg, *args):
> +    """Print a debug message to stderr when DEBUG is enabled."""
> +    if DEBUG:
> +        print >> sys.stderr, msg % args
> +
> +def error (msg, *args):
> +    """Print an error message to stderr."""
> +    print >> sys.stderr, "ERROR:", msg % args
> +
> +def warn(msg, *args):
> +	"""Print a warning message to stderr."""
> +	print >> sys.stderr, "warning:", msg % args
> +
> +def die (msg, *args):
> +    """Print as error message to stderr and exit the program."""
> +    error(msg, *args)
> +    sys.exit(1)
> +
> +

It seems the two functions you add (notify() and warn()) have a different 
indentation than the existing code (which uses 4 spaces). Please fix.

(When I first introduced these Python patches, there was a discussion on the 
differences in indentation/style between the Git C code, and Python code, 
and it was decided to follow the Python conventions, to make the code more 
inviting to the Python community.)


...Johan

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

^ permalink raw reply

* Re: keeping track of where a patch begins
From: Thomas Rast @ 2009-10-30  8:37 UTC (permalink / raw)
  To: pascal; +Cc: Junio C Hamano, Nicolas Pitre, E R, git, Jeff King
In-Reply-To: <4AEA94EB.8080304@obry.net>

Pascal Obry wrote:
> 
> Le 22/10/2009 10:27, Thomas Rast a écrit :
> > I think this not only changes the model of branches, but also commits,
> > to some extent.  Currently, commit have no intrinsic branch
> > membership
[...]
> To me there is case where it is important to know which are the commits 
> done on a topic branch for example. When working on multiple topic it is 
> difficult to remember which commits have been done on this specific 
> branch. This is needed to rebase onto:
> 
>     $ git rebase --onto somebranch <topic_base> <topic_head>
> 
> A common idiom, but one as to think hard (& right) to properly get the 
> topic_base today.

But how frequently do your topics start on other topics?  Otherwise
they will start on an integration or maint branch, and in the git.git
model where each integration branch is contained in the next "less
stable" one, that means you can just specify 'pu' or equivalent.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

^ permalink raw reply

* Re: My custom cccmd
From: Felipe Contreras @ 2009-10-30  8:39 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vzl7c4i81.fsf@alter.siamese.dyndns.org>

On Tue, Oct 27, 2009 at 11:53 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> I explored this a bit more and I've come to the conclusion that we
>> actually don't wand to discard the previous commits in the patch
>> series. Let's think about this example:
>> 0001 <- John
>> 0002 <- Me overriding some changes from John
>>
>> In this case we want John to appear in the CC list of 0002, because we
>> are changing his code.
>
> There are two cases: your patch partially overrides his code, and your
> patch completely rewrites/removes his code.
>
> Obviously in the former case you do want to Cc, but there are parts of his
> change that remain in the final result, so this case does not matter in
> this discussion.  You will Cc him because of these remaining lines anyway.
>
> In the latter case, the only contribution that remains from him is a
> commit with his log message that does not help describing anything in the
> end result, cluttering the history.
>
> In such a case, I would imagine that the reviewers would want to see a
> cleaned up series that does not have his patch that is irrelevant for
> understanding the final result.  John might want to know about it, if only
> to raise objection to the way how you arranged your series.  For that
> reason, I think it makes sense to Cc him.
>
> But it is a rather a convoluted logic, if you ask me.  You find John and
> Cc him, primarily so that he can point out that you should be redoing the
> series not to have his patch as an intermediate state in the series to
> begin with, because his commit does not contribute to the end result.
>
> It might make more sense if your tool told you about such a case directly,
> rather than helping you find John so that he can tell you ;-).

But that's not the purpose of the cccmd tool.

I agree that such "patch series simplificator" tool would be very
useful, but that's out of scope for this. Isn't it?

-- 
Felipe Contreras

^ permalink raw reply

* Re: [PATCH 18/19] Refactor git_remote_cvs to a more generic git_remote_helpers
From: Johan Herland @ 2009-10-30  8:42 UTC (permalink / raw)
  To: Sverre Rabbelier; +Cc: Git List, Johannes Schindelin, Daniel Barkalow
In-Reply-To: <1256839287-19016-19-git-send-email-srabbelier@gmail.com>

On Thursday 29 October 2009, Sverre Rabbelier wrote:
> This in an effort to allow future remote helpers written in python to
> re-use the non-cvs-specific code.
> 
> Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
> ---

[snip]

> diff --git a/git_remote_helpers/__init__.py
>  b/git_remote_helpers/__init__.py new file mode 100644
> index 0000000..38c7b5f
> --- /dev/null
> +++ b/git_remote_helpers/__init__.py
> @@ -0,0 +1,27 @@
> +#!/usr/bin/env python
> +
> +"""Support library package for git remote helpers.
> +
> +Git remote helpers are helper commands that interfaces with a non-git
> +repository to provide automatic import of non-git history into a Git
> +repository.
> +
> +This package provides the support library needed by these helpers..
> +The following modules are included:
> +
> +- cvs/cvs - Interaction with CVS repositories
> +
> +- cvs/symbol_cache - Local CVS symbol cache
> +
> +- cvs/changeset - Collect individual CVS revisions into commits
> +
> +- cvs/commit_states - Map Git commits to CVS states
> +
> +- cvs/revision_map - Map CVS revisions to various metainformation
> +
> +- git/git - Interaction with Git repositories

Since this is Python documentation within a package, I'd rather refer to the 
python modules as _modules_ and not files. I.e. please use '.' instead of 
'/':

+- cvs.cvs - Interaction with CVS repositories
+
+- cvs.symbol_cache - Local CVS symbol cache
+
+- cvs.changeset - Collect individual CVS revisions into commits
+
+- cvs.commit_states - Map Git commits to CVS states
+
+- cvs.revision_map - Map CVS revisions to various metainformation
+
+- git.git - Interaction with Git repositories


> +
> +- util - General utility functionality use by the other modules in
> +         this package, and also used directly by git-remote-cvs.

Probably you should drop the direct reference to git-remote-cvs.

> diff --git a/git_remote_cvs/util.py b/git_remote_helpers/util.py
> similarity index 100%
> rename from git_remote_cvs/util.py
> rename to git_remote_helpers/util.py
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 77ad23e..c7530aa 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -640,9 +640,9 @@ test -d ../templates/blt || {
> 
>  if test -z "$GIT_TEST_INSTALLED"
>  then
> -	GITPYTHONLIB="$(pwd)/../git_remote_cvs/build/lib"
> +	GITPYTHONLIB="$(pwd)/../git_remote_helpers/build/lib"
>  	export GITPYTHONLIB
> -	test -d ../git_remote_cvs/build || {
> +	test -d ../git_remote_helpers/build || {
>  		error "You haven't built git_remote_cvs yet, have you?"

Please also s/git_remote_cvs/git_remote_helpers/ in the error message.

Otherwise, it all looks good :)


...Johan

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

^ permalink raw reply


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