[PATCH v3 0/7] i18n for git-am, git-rebase and git-merge
Marked messages for translation in git-am, git-rebase, and git-merge. Also fixed affected test cases when turn GETTEXT_POISON switch on. Jiang Xin (7): i18n: New keywords for xgettext extraction from sh i18n: rebase: mark strings for translation i18n: Rewrite gettext messages start with dash Remove obsolete LONG_USAGE which breaks xgettext i18n: am: mark more strings for translation Remove unused and bad gettext block from git-am i18n: merge-recursive: mark strings for translation Makefile | 3 +- git-am.sh| 14 ++-- git-rebase.sh| 87 git-submodule.sh | 2 +- merge-recursive.c| 148 +++ t/t3400-rebase.sh| 8 +- t/t3404-rebase-interactive.sh| 2 +- t/t3406-rebase-message.sh| 2 +- t/t6022-merge-rename.sh | 16 ++-- t/t6042-merge-rename-corner-cases.sh | 2 +- 10 files changed, 134 insertions(+), 150 deletions(-) -- 1.7.12.rc0.17.gcb766d3 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 1/7] i18n: New keywords for xgettext extraction from sh
Since we have additional shell wrappers (gettextln and eval_gettextln) for gettext, we need to take into account these wrappers when run 'make pot' to extract messages from shell scripts. Signed-off-by: Jiang Xin worldhello@gmail.com Reviewed-by: Stefano Lattarini stefano.lattar...@gmail.com --- Makefile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index b0b34..d3cd9 100644 --- a/Makefile +++ b/Makefile @@ -2387,7 +2387,8 @@ XGETTEXT_FLAGS = \ --from-code=UTF-8 XGETTEXT_FLAGS_C = $(XGETTEXT_FLAGS) --language=C \ --keyword=_ --keyword=N_ --keyword=Q_:1,2 -XGETTEXT_FLAGS_SH = $(XGETTEXT_FLAGS) --language=Shell +XGETTEXT_FLAGS_SH = $(XGETTEXT_FLAGS) --language=Shell \ + --keyword=gettextln --keyword=eval_gettextln XGETTEXT_FLAGS_PERL = $(XGETTEXT_FLAGS) --keyword=__ --language=Perl LOCALIZED_C := $(C_OBJ:o=c) $(LIB_H) $(GENERATED_H) LOCALIZED_SH := $(SCRIPT_SH) -- 1.7.12.rc0.17.gcb766d3 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 3/7] i18n: Rewrite gettext messages start with dash
Gettext message in a shell script should not start with '-', one
workaround is adding '--' between gettext and the message, like:
gettext -- --exec option ...
But due to a bug in the xgettext extraction, xgettext can not
extract the actual message for this case. Rewriting the message
is a simpler and better solution.
Signed-off-by: Jiang Xin worldhello@gmail.com
Signed-off-by: Ævar Arnfjörð Bjarmason ava...@gmail.com
Reported-by: Vincent van Ravesteijn v...@lyx.org
Reviewed-by: Stefano Lattarini stefano.lattar...@gmail.com
---
git-rebase.sh | 2 +-
git-submodule.sh | 2 +-
t/t3404-rebase-interactive.sh | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/git-rebase.sh b/git-rebase.sh
index f07269..09a3 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -317,7 +317,7 @@ test $# -gt 2 usage
if test -n $cmd
test $interactive_rebase != explicit
then
- die $(gettext -- --exec option must be used with --interactive
option)
+ die $(gettext The --exec option must be used with the --interactive
option)
fi
if test -n $action
diff --git a/git-submodule.sh b/git-submodule.sh
index dba4d..5b019 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -748,7 +748,7 @@ cmd_summary() {
if [ -n $files ]
then
test -n $cached
- die $(gettext -- --cached cannot be used with --files)
+ die $(gettext The --cached option cannot be used with the
--files option)
diff_cmd=diff-files
head=
fi
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 8078..f206a 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -858,7 +858,7 @@ test_expect_success 'rebase -ix with --autosquash' '
test_expect_success 'rebase --exec without -i shows error message' '
git reset --hard execute
test_must_fail git rebase --exec git show HEAD HEAD~2 2actual
- echo --exec option must be used with --interactive option expected
+ echo The --exec option must be used with the --interactive option
expected
test_i18ncmp expected actual
'
--
1.7.12.rc0.17.gcb766d3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 4/7] Remove obsolete LONG_USAGE which breaks xgettext
The obsolete LONG_USAGE variable has the following message in it: A'\''--B'\''--C'\'' And such complex LONG_USAGE message will breaks xgettext when extracting l10n messages. But if single quotes are removed from the message, xgettext works fine on 'git-rebase.sh'. Since there is a modern OPTIONS_SPEC variable in use in this script, it's safe to remove the obsolete USAGE and LONG_USAGE variables. Signed-off-by: Jiang Xin worldhello@gmail.com Reviewed-by: Stefano Lattarini stefano.lattar...@gmail.com --- git-rebase.sh | 25 - 1 file changed, 25 deletions(-) diff --git a/git-rebase.sh b/git-rebase.sh index 09a3..63485 100755 --- a/git-rebase.sh +++ b/git-rebase.sh @@ -3,31 +3,6 @@ # Copyright (c) 2005 Junio C Hamano. # -USAGE='[--interactive | -i] [--exec | -x cmd] [-v] [--force-rebase | -f] - [--no-ff] [--onto newbase] [upstream|--root] [branch] [--quiet | -q]' -LONG_USAGE='git-rebase replaces branch with a new branch of the -same name. When the --onto option is provided the new branch starts -out with a HEAD equal to newbase, otherwise it is equal to upstream -It then attempts to create a new commit for each commit from the original -branch that does not exist in the upstream branch. - -It is possible that a merge failure will prevent this process from being -completely automatic. You will have to resolve any such merge failure -and run git rebase --continue. Another option is to bypass the commit -that caused the merge failure with git rebase --skip. To check out the -original branch and remove the .git/rebase-apply working files, use the -command git rebase --abort instead. - -Note that if branch is not specified on the command line, the -currently checked out branch is used. - -Example: git-rebase master~1 topic - - A---B---C topic A'\''--B'\''--C'\'' topic - / -- / - D---E---F---G master D---E---F---G master -' - SUBDIRECTORY_OK=Yes OPTIONS_KEEPDASHDASH= OPTIONS_SPEC=\ -- 1.7.12.rc0.17.gcb766d3 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 5/7] i18n: am: mark more strings for translation
Mark additional 3 strings for translation, and reduce one indentation
level for one gettextln clause introduced in commit de88c1c.
Signed-off-by: Jiang Xin worldhello@gmail.com
Reviewed-by: Stefano Lattarini stefano.lattar...@gmail.com
---
git-am.sh | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/git-am.sh b/git-am.sh
index c02e6..b7183 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -92,7 +92,7 @@ safe_to_abort () {
then
return 0
fi
- gettextln You seem to have moved HEAD since the last 'am'
failure.
+ gettextln You seem to have moved HEAD since the last 'am' failure.
Not rewinding to ORIG_HEAD 2
return 1
}
@@ -136,7 +136,7 @@ fall_back_3way () {
git write-tree $dotest/patch-merge-base+ ||
cannot_fallback $(gettext Repository lacks necessary blobs to fall back
on 3-way merge.)
-say Using index info to reconstruct a base tree...
+say $(gettext Using index info to reconstruct a base tree...)
cmd='GIT_INDEX_FILE=$dotest/patch-merge-tmp-index'
@@ -176,8 +176,7 @@ It does not apply to blobs recorded in its index.)
fi
git-merge-recursive $orig_tree -- HEAD $his_tree || {
git rerere $allow_rerere_autoupdate
- echo Failed to merge in the changes.
- exit 1
+ die $(gettext Failed to merge in the changes.)
}
unset GITHEAD_$his_tree
}
@@ -387,8 +386,8 @@ do
-i|--interactive)
interactive=t ;;
-b|--binary)
- echo 2 The $1 option has been a no-op for long time, and
- echo 2 it will be removed. Please do not use it anymore.
+ echo 2 $(gettext The -b option has been a no-op for long
time, and
+it will be removed. Please do not use it anymore.)
;;
-3|--3way)
threeway=t ;;
--
1.7.12.rc0.17.gcb766d3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 6/7] Remove unused and bad gettext block from git-am
Gettext message should not start with '-' nor '--'. Since the '-d' and '--dotest' options do not exist in OPTIONS_SPEC variable, it's safe to remove the block. Signed-off-by: Jiang Xin worldhello@gmail.com Reviewed-by: Stefano Lattarini stefano.lattar...@gmail.com --- git-am.sh | 3 --- 1 file changed, 3 deletions(-) diff --git a/git-am.sh b/git-am.sh index b7183..a2e3d 100755 --- a/git-am.sh +++ b/git-am.sh @@ -413,9 +413,6 @@ it will be removed. Please do not use it anymore.) abort=t ;; --rebasing) rebasing=t threeway=t ;; - -d|--dotest) - die $(gettext -d option is no longer supported. Do not use.) - ;; --resolvemsg) shift; resolvemsg=$1 ;; --whitespace|--directory|--exclude|--include) -- 1.7.12.rc0.17.gcb766d3 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 7/7] i18n: merge-recursive: mark strings for translation
Mark strings in merge-recursive for translation.
Some test scripts are affected by this update, and would fail if are
tested with GETTEXT_POISON switch turned on. Use i18n-specific test
functions, such as test_i18ngrep in the related test scripts will fix
these issues.
Signed-off-by: Jiang Xin worldhello@gmail.com
---
merge-recursive.c| 148 +++
t/t6022-merge-rename.sh | 16 ++--
t/t6042-merge-rename-corner-cases.sh | 2 +-
3 files changed, 88 insertions(+), 78 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index 68093..8903 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -187,7 +187,7 @@ static void output_commit_title(struct merge_options *o,
struct commit *commit)
else {
printf(%s , find_unique_abbrev(commit-object.sha1,
DEFAULT_ABBREV));
if (parse_commit(commit) != 0)
- printf((bad commit)\n);
+ printf(_((bad commit)\n));
else {
const char *title;
int len = find_commit_subject(commit-buffer, title);
@@ -203,7 +203,7 @@ static int add_cacheinfo(unsigned int mode, const unsigned
char *sha1,
struct cache_entry *ce;
ce = make_cache_entry(mode, sha1 ? sha1 : null_sha1, path, stage,
refresh);
if (!ce)
- return error(addinfo_cache failed for path '%s', path);
+ return error(_(addinfo_cache failed for path '%s'), path);
return add_cache_entry(ce, options);
}
@@ -265,7 +265,7 @@ struct tree *write_tree_from_memory(struct merge_options *o)
if (!cache_tree_fully_valid(active_cache_tree)
cache_tree_update(active_cache_tree,
active_cache, active_nr, 0) 0)
- die(error building trees);
+ die(_(error building trees));
result = lookup_tree(active_cache_tree-sha1);
@@ -494,7 +494,7 @@ static struct string_list *get_renames(struct merge_options
*o,
opts.show_rename_progress = o-show_rename_progress;
opts.output_format = DIFF_FORMAT_NO_OUTPUT;
if (diff_setup_done(opts) 0)
- die(diff setup failed);
+ die(_(diff setup failed));
diff_tree_sha1(o_tree-object.sha1, tree-object.sha1, , opts);
diffcore_std(opts);
if (opts.needed_rename_limit o-needed_rename_limit)
@@ -624,7 +624,7 @@ static void flush_buffer(int fd, const char *buf, unsigned
long size)
break;
die_errno(merge-recursive);
} else if (!ret) {
- die(merge-recursive: disk full?);
+ die(_(merge-recursive: disk full?));
}
size -= ret;
buf += ret;
@@ -687,7 +687,7 @@ static int would_lose_untracked(const char *path)
static int make_room_for_path(struct merge_options *o, const char *path)
{
int status, i;
- const char *msg = failed to create path '%s'%s;
+ const char *msg = _(failed to create path '%s'%s);
/* Unlink any D/F conflict files that are in the way */
for (i = 0; i o-df_conflict_file_set.nr; i++) {
@@ -698,7 +698,7 @@ static int make_room_for_path(struct merge_options *o,
const char *path)
path[df_pathlen] == '/'
strncmp(path, df_path, df_pathlen) == 0) {
output(o, 3,
- Removing %s to make room for subdirectory\n,
+ _(Removing %s to make room for subdirectory\n),
df_path);
unlink(df_path);
unsorted_string_list_delete_item(o-df_conflict_file_set,
@@ -712,7 +712,7 @@ static int make_room_for_path(struct merge_options *o,
const char *path)
if (status) {
if (status == -3) {
/* something else exists */
- error(msg, path, : perhaps a D/F conflict?);
+ error(msg, path, _(: perhaps a D/F conflict?));
return -1;
}
die(msg, path, );
@@ -723,7 +723,7 @@ static int make_room_for_path(struct merge_options *o,
const char *path)
* tracking it.
*/
if (would_lose_untracked(path))
- return error(refusing to lose untracked file at '%s',
+ return error(_(refusing to lose untracked file at '%s'),
path);
/* Successful unlink is good.. */
@@ -733,7 +733,7 @@ static int make_room_for_path(struct merge_options *o,
const char *path)
if (errno == ENOENT)
return 0;
/* .. but not some other error (who really cares what?) */
- return error(msg, path, : perhaps a D/F conflict?);
+ return error(msg, path, _(: perhaps a D/F
Re: [PATCH] test: some testcases failed if cwd is on a symlink
Some grammatical nits about the commit message. I hope this doesn't come across as too picky/annoying ... And you might want to wait for a native to confirm whether these nits are actually all warranted. On 07/24/2012 10:00 AM, Jiang Xin wrote: Run s/Run/Running/ command 'git rev-parse --git-dir' under subdir s/under subdir/under a subdir/. Or even better IMHO, s/under subdir/in a subdir/ will return realpath s/realpath/the realpath/ of '.git' directory. s/of/of the/ Some test scripts compare this realpath against $TRASH_DIRECTORY, they are not equal s/they are not/but they are not/ if current working directory is on a symlink. s/current/the current/ In this fix, get realpath s/realpath/the realpath/ of $TRASH_DIRECTORY, store it in $TRASH_REALPATH variable, and use it when necessary. Signed-off-by: Jiang Xin worldhello@gmail.com --- t/t4035-diff-quiet.sh | 8 +--- t/t9903-bash-prompt.sh | 13 +++-- 2 个文件被修改，插入 12 行(+)，删除 9 行(-) diff --git a/t/t4035-diff-quiet.sh b/t/t4035-diff-quiet.sh index 23141..5855 100755 --- a/t/t4035-diff-quiet.sh +++ b/t/t4035-diff-quiet.sh @@ -4,6 +4,8 @@ test_description='Return value of diffs' . ./test-lib.sh +TRASH_REALPATH=$(cd $TRASH_DIRECTORY; pwd -P) + BTW, the outer quotes are not needed; this is enough: TRASH_REALPATH=$(cd $TRASH_DIRECTORY; pwd -P) Regards, Stefano -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/7] i18n: New keywords for xgettext extraction from sh
On 07/24/2012 08:59 AM, Jiang Xin wrote: Since we have additional shell wrappers (gettextln and eval_gettextln) for gettext, we need to take into account these wrappers when run s/when run/when running/ or s/when we run/. Sorry for not spotting that in my first review! 'make pot' to extract messages from shell scripts. Regards, Stefano -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/7] i18n: rebase: mark strings for translation
On 07/24/2012 08:59 AM, Jiang Xin wrote: Mark strings in git-rebase.sh for translation. Some test scripts are affected by this update, and would fail if are s/if are/if/ tested with GETTEXT_POISON switch turned on. Use i18n-specific test s/Use/Using/, or s/Use/Use of/ functions, such as test_i18ngrep Missing comma after 'test_i18ngrep' here. in the related test scripts will fix these issues. Regards, Stefano -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 7/7] i18n: merge-recursive: mark strings for translation
On 07/24/2012 08:59 AM, Jiang Xin wrote: Mark strings in merge-recursive for translation. Some test scripts are affected by this update, and would fail if are tested with GETTEXT_POISON switch turned on. Use i18n-specific test functions, such as test_i18ngrep in the related test scripts will fix these issues. The same comments I made for [PATCH 2/7] in this series apply here. Regards, Stefano -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] test: some testcases failed if cwd is on a symlink
worldhello@gmail.com wrote on Tue, 24 Jul 2012 16:00 +0800: Run command 'git rev-parse --git-dir' under subdir will return realpath of '.git' directory. Some test scripts compare this realpath against $TRASH_DIRECTORY, they are not equal if current working directory is on a symlink. In this fix, get realpath of $TRASH_DIRECTORY, store it in $TRASH_REALPATH variable, and use it when necessary. We have the same problem with p4. I fixed it in t98* in 23bd0c9 (git p4 test: use real_path to resolve p4 client symlinks, 2012-06-27), but maybe an exported TRASH_DIRECTORY_REAL_PATH might be generally useful. +TRASH_REALPATH=$(cd $TRASH_DIRECTORY; pwd -P) There's a portable test helper that does this already: path=$(test-path-utils real_path $path) -- Pete -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] test-lib.sh: unset XDG_CONFIG_HOME
Now that git respects XDG_CONFIG_HOME for some lookups, we must be sure to cleanse the test environment. Otherwise, the user's XDG_CONFIG_HOME could influence the test results. Signed-off-by: Jeff King p...@peff.net --- [oops, re-sending because I forgot to cc git@vger] You can test this trivially with: XDG_CONFIG_HOME=/whatever ./t1306-xdg-files.sh t/test-lib.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/t/test-lib.sh b/t/test-lib.sh index acda33d..5e7f435 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -61,6 +61,7 @@ unset VISUAL EMAIL LANGUAGE COLUMNS $(perl -e ' my @vars = grep(/^GIT_/ !/^GIT_($ok)/o, @env); print join(\n, @vars); ') +unset XDG_CONFIG_HOME GIT_AUTHOR_EMAIL=aut...@example.com GIT_AUTHOR_NAME='A U Thor' GIT_COMMITTER_EMAIL=commit...@example.com -- 1.7.11.3.4.g9f70dbb -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] attr: make sure we have an xdg path before using it
If we don't have a core.attributesfile configured, we fall
back to checking XDG config. This is usually
$HOME/.config/attributes. However, if $HOME is unset, then
home_config_paths will return NULL, and we end up calling
fopen(NULL).
Depending on your system, this may or may not cause the
accompanying test to fail (e.g., on Linux and glibc, the
address will go straight to open, which will return EFAULT).
However, valgrind will reliably notice the error.
Signed-off-by: Jeff King p...@peff.net
---
[oops, resending because I forgot to cc git@vger]
This is another instance of Matthieu's e3ebc35 (config: fix several
access(NULL) calls, 2012-07-12). I guess it wasn't caught
because of the lack of a test without HOME set (I found it
because t5541 can trigger it in a very roundabout way).
attr.c | 12 +++-
t/t1306-xdg-files.sh | 6 ++
2 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/attr.c b/attr.c
index aef93d8..b52efb5 100644
--- a/attr.c
+++ b/attr.c
@@ -520,11 +520,13 @@ static void bootstrap_attr_stack(void)
home_config_paths(NULL, xdg_attributes_file, attributes);
git_attributes_file = xdg_attributes_file;
}
- elem = read_attr_from_file(git_attributes_file, 1);
- if (elem) {
- elem-origin = NULL;
- elem-prev = attr_stack;
- attr_stack = elem;
+ if (git_attributes_file) {
+ elem = read_attr_from_file(git_attributes_file, 1);
+ if (elem) {
+ elem-origin = NULL;
+ elem-prev = attr_stack;
+ attr_stack = elem;
+ }
}
if (!is_bare_repository() || direction == GIT_ATTR_INDEX) {
diff --git a/t/t1306-xdg-files.sh b/t/t1306-xdg-files.sh
index 3c75c3f..1569596 100755
--- a/t/t1306-xdg-files.sh
+++ b/t/t1306-xdg-files.sh
@@ -106,6 +106,12 @@ test_expect_success 'Checking attributes in the XDG
attributes file' '
test_cmp expected actual
'
+test_expect_success 'Checking XDG attributes when HOME is unset' '
+ expected
+ (sane_unset HOME
+git check-attr -a f actual)
+ test_cmp expected actual
+'
test_expect_success 'Checking attributes in both XDG and local attributes
files' '
echo f -attr_f .gitattributes
--
1.7.11.3.4.g9f70dbb
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GSoC] Designing a faster index format - Progress report week 13
Robin Rosenberg robin.rosenb...@dewire.com writes:
Junio C Hamano skrev 2012-07-22 23.08:
Thomas Rast tr...@student.ethz.ch writes:
What is the status quo? I take it JGit does not have any of ctime, dev,
ino etc., and either leaves the existing value or puts a 0
an argument in favor of splitting stat_crc into its fields again?
A difference is that JGit already has such code, and we would be
adding a burden to do so yet again. It also may not just be JGit,
but anything that wants to be compatible with systems whose
filesystem interface does not give enough data by omitting fields
the current index pays attention to.
It isn't really a discussion about splitting again, but more about
not squishing them into a new field in the first place---IIUC, even
outside Windows, ctime is already problematic on some systems where
background processes muck with extended attributes Git does not pay
attention to. If the patch makes us lose the ability to selectively
ignore changes to certain fields (e.g. changes to dev and ino are
noticed but ctime are ignored) by squishing them into one new field,
wouldn't removing them without adding such a useless field a simpler
way to go?
I wasnt't thinking of splitting, but now I read it again, I do think
it should split.
Aren't you two going off in different directions? I read Junio as
implying that if size/ctime/dev/ino are a pain to deal with, we should
just drop them altogether. You seem to be saying the opposite:
Having size accessible is a good thing, and even
better if it a 64-bit value so we don't have the modulo-4G problem
when looking at it. Current size is 4G + 33 bytes, index says 33. Did
the
file change or not?
Having access to size make the need for actually
invoking the racy git logic and comparing file content less likely.
I don't think this is true. Racy git logic is needed every time that
the file *looks* unchanged, but isn't. In the case where the file is
certified (by mtime) unchanged, we don't go checking. But in the case
where it *looks* changed, we still have to go and read it to know if,
perhaps, the only thing the user did was hit save again.
Not to mention that this really hurts in terms of index size. Our
benchmark for index-v5 is the Webkit project, which stands at 180k
files. So every 6B/entry is about an MB of final size, which needs to
be loaded, hashed (or crc'd), then hashed/crc'd again and written.
Junio's index-v4 was a speed boost mainly because it cuts down on the
size of the index. Do we want to throw that out?
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] t1306: check that XDG_CONFIG_HOME works
This should override $HOME/.config, but we never actually tested it. Signed-off-by: Jeff King p...@peff.net --- t/t1306-xdg-files.sh | 8 1 file changed, 8 insertions(+) diff --git a/t/t1306-xdg-files.sh b/t/t1306-xdg-files.sh index 1569596..4447949 100755 --- a/t/t1306-xdg-files.sh +++ b/t/t1306-xdg-files.sh @@ -113,6 +113,14 @@ test_expect_success 'Checking XDG attributes when HOME is unset' ' test_cmp expected actual ' +test_expect_success 'Prefer XDG_CONFIG_HOME to HOME/.config' ' + echo f: attr_f: foo expected + mkdir -p foo/git + echo f attr_f=foo foo/git/attributes + XDG_CONFIG_HOME=foo git check-attr -a f actual + test_cmp expected actual +' + test_expect_success 'Checking attributes in both XDG and local attributes files' ' echo f -attr_f .gitattributes echo f: attr_f: unset expected -- 1.7.11.3.4.g9f70dbb -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Add a svnrdump-simulator replaying a dump file for testing.
On Mon, Jul 23, 2012 at 2:44 PM, Florian Achleitner florian.achleitner.2.6...@gmail.com wrote: + sys.exit(ret) \ No newline at end of file Nit: add a \n after sys.exit(ret), perhaps? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] test-lib.sh: unset XDG_CONFIG_HOME
Thanks (for the 3 patches, all of them look good). the unset XDG_CONFIG_HOME part was already discussed here: http://thread.gmane.org/gmane.comp.version-control.git/201609 But Michael did not continue the thread. I think your solution (unset $XDG_CONFIG_HOME instead of setting it to $HOME/.config/git) is better. In the thread above, I also proposed checking that $XDG_CONFIG_HOME was taken into account, but for the git config part (while you test the attributes part). I think it makes sense to add stg like this to your PATCH 3: diff --git a/t/t1306-xdg-files.sh b/t/t1306-xdg-files.sh index 3c75c3f..f1ea9f1 100755 --- a/t/t1306-xdg-files.sh +++ b/t/t1306-xdg-files.sh @@ -38,6 +38,19 @@ test_expect_success 'read with --get: xdg file exists and ~/.gitconfig doesn'\'' test_cmp expected actual ' +test_expect_success '$XDG_CONFIG_HOME overrides $HOME/.config/git' ' + mkdir -p $HOME/xdg/git/ + echo [user] $HOME/xdg/git/config + echo name = in_xdg $HOME/xdg/git/config + echo in_xdg expected + ( + XDG_CONFIG_HOME=$HOME/xdg/ + export XDG_CONFIG_HOME + git config --get-all user.name actual + ) + test_cmp expected actual +' + test_expect_success 'read with --get: xdg file exists and ~/.gitconfig exists' ' .gitconfig -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] rebase -i: handle fixup of root commit correctly
There is a bug with git rebase -i --root when a fixup or squash line is
applied to the new root. We attempt to amend the commit onto which they
apply with git reset --soft HEAD^ followed by a normal commit. Unlike a
real commit --amend, this sequence will fail against a root commit as it
has no parent.
Fix rebase -i to use commit --amend for fixup and squash instead, and
add a test for the case of a fixup of the root commit.
Signed-off-by: Chris Webb ch...@arachsys.com
---
Sorry, I should have spotted this issue when I did the original root-rebase
series. I've checked that this patch doesn't break any of the existing
tests, as well as satisfying the newly introduced check for the root-fixup
case.
git-rebase--interactive.sh| 25 +
t/t3404-rebase-interactive.sh | 8
2 files changed, 21 insertions(+), 12 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index bef7bc0..0d2056f 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -493,25 +493,28 @@ do_next () {
author_script_content=$(get_author_ident_from_commit HEAD)
echo $author_script_content $author_script
eval $author_script_content
- output git reset --soft HEAD^
- pick_one -n $sha1 || die_failed_squash $sha1 $rest
+ if ! pick_one -n $sha1
+ then
+ git rev-parse --verify HEAD $amend
+ die_failed_squash $sha1 $rest
+ fi
case $(peek_next_command) in
squash|s|fixup|f)
# This is an intermediate commit; its message will only
be
# used in case of trouble. So use the long version:
- do_with_author output git commit --no-verify -F
$squash_msg ||
+ do_with_author output git commit --amend --no-verify -F
$squash_msg ||
die_failed_squash $sha1 $rest
;;
*)
# This is the final command of this squash/fixup group
if test -f $fixup_msg
then
- do_with_author git commit --no-verify -F
$fixup_msg ||
+ do_with_author git commit --amend --no-verify
-F $fixup_msg ||
die_failed_squash $sha1 $rest
else
cp $squash_msg $GIT_DIR/SQUASH_MSG || exit
rm -f $GIT_DIR/MERGE_MSG
- do_with_author git commit --no-verify -e ||
+ do_with_author git commit --amend --no-verify
-F $GIT_DIR/SQUASH_MSG -e ||
die_failed_squash $sha1 $rest
fi
rm -f $squash_msg $fixup_msg
@@ -748,7 +751,6 @@ In both case, once you're done, continue with:
fi
. $author_script ||
die Error trying to find the author identity to amend
commit
- current_head=
if test -f $amend
then
current_head=$(git rev-parse --verify HEAD)
@@ -756,13 +758,12 @@ In both case, once you're done, continue with:
die \
You have uncommitted changes in your working tree. Please, commit them
first and then run 'git rebase --continue' again.
- git reset --soft HEAD^ ||
- die Cannot rewind the HEAD
+ do_with_author git commit --amend --no-verify -F $msg
-e ||
+ die Could not commit staged changes.
+ else
+ do_with_author git commit --no-verify -F $msg -e ||
+ die Could not commit staged changes.
fi
- do_with_author git commit --no-verify -F $msg -e || {
- test -n $current_head git reset --soft
$current_head
- die Could not commit staged changes.
- }
fi
record_in_rewritten $(cat $state_dir/stopped-sha)
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 8078db6..3f75d32 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -903,4 +903,12 @@ test_expect_success 'rebase -i --root temporary sentinel
commit' '
git rebase --abort
'
+test_expect_success 'rebase -i --root fixup root commit' '
+ git checkout B
+ FAKE_LINES=1 fixup 2 git rebase -i --root
+ test A = $(git cat-file commit HEAD | sed -ne \$p)
+ test B = $(git show HEAD:file1)
+ test 0 = $(git cat-file commit HEAD | grep -c ^parent\ )
+'
+
test_done
--
1.7.11.2.251.g6a928a6
--
To unsubscribe from this list: send the
Re: [PATCH 1/3] test-lib.sh: unset XDG_CONFIG_HOME
On Tue, Jul 24, 2012 at 02:06:43PM +0200, Matthieu Moy wrote: Thanks (for the 3 patches, all of them look good). the unset XDG_CONFIG_HOME part was already discussed here: http://thread.gmane.org/gmane.comp.version-control.git/201609 But Michael did not continue the thread. I think your solution (unset $XDG_CONFIG_HOME instead of setting it to $HOME/.config/git) is better. Yeah, setting it to $HOME/.config/git is actively wrong; I agree with the reasoning in that thread (which I did not read until just now). In the thread above, I also proposed checking that $XDG_CONFIG_HOME was taken into account, but for the git config part (while you test the attributes part). Yeah, I see. I think it makes sense to add stg like this to your PATCH 3: Agreed. And one for the exclude section, too, just for completeness. Revised patch 3 is below. -- 8 -- Subject: [PATCHv2 3/3] t1306: check that XDG_CONFIG_HOME works This should override $HOME/.config, but we never actually tested it. --- t/t1306-xdg-files.sh | 26 ++ 1 file changed, 26 insertions(+) diff --git a/t/t1306-xdg-files.sh b/t/t1306-xdg-files.sh index 1569596..a62c3fb 100755 --- a/t/t1306-xdg-files.sh +++ b/t/t1306-xdg-files.sh @@ -38,6 +38,13 @@ test_expect_success 'read with --get: xdg file exists and ~/.gitconfig doesn'\'' test_cmp expected actual ' +test_expect_success '$XDG_CONFIG_HOME overrides $HOME/.config/git' ' + mkdir -p $HOME/xdg/git + echo [user]name = in_xdg $HOME/xdg/git/config + echo in_xdg expected + XDG_CONFIG_HOME=$HOME/xdg git config --get-all user.name actual + test_cmp expected actual +' test_expect_success 'read with --get: xdg file exists and ~/.gitconfig exists' ' .gitconfig @@ -80,6 +87,17 @@ test_expect_success 'Exclusion of a file in the XDG ignore file' ' test_must_fail git add to_be_excluded ' +test_expect_success '$XDG_CONFIG_HOME overrides $HOME/.config/ignore' ' + mkdir -p $HOME/xdg/git + echo content excluded_by_xdg_only + echo excluded_by_xdg_only $HOME/xdg/git/ignore + test_when_finished git read-tree --empty + (XDG_CONFIG_HOME=$HOME/xdg +export XDG_CONFIG_HOME +git add to_be_excluded +test_must_fail git add excluded_by_xdg_only + ) +' test_expect_success 'Exclusion in both XDG and local ignore files' ' echo to_be_excluded .gitignore @@ -113,6 +131,14 @@ test_expect_success 'Checking XDG attributes when HOME is unset' ' test_cmp expected actual ' +test_expect_success '$XDG_CONFIG_HOME overrides $HOME/.config/attributes' ' + mkdir -p $HOME/xdg/git + echo f attr_f=xdg $HOME/xdg/git/attributes + echo f: attr_f: xdg expected + XDG_CONFIG_HOME=$HOME/xdg git check-attr -a f actual + test_cmp expected actual +' + test_expect_success 'Checking attributes in both XDG and local attributes files' ' echo f -attr_f .gitattributes echo f: attr_f: unset expected -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] ignore: make sure we have an xdg path before using it
Commit e3ebc35 (config: fix several access(NULL) calls, 2012-07-12) was fixing access(NULL) calls when trying to access $HOME/.config/git/config, but missed the ones when trying to access $HOME/.config/git/ignore. Fix and test this. Signed-off-by: Matthieu Moy matthieu@imag.fr --- This can be appended to Jeff's serie. I thought if we had 3 bug instances and already fixed 2, why not fix the (hopefully last) one ;-). dir.c| 2 +- t/t1306-xdg-files.sh | 7 +++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/dir.c b/dir.c index a772c6d..240bf0c 100644 --- a/dir.c +++ b/dir.c @@ -1313,7 +1313,7 @@ void setup_standard_excludes(struct dir_struct *dir) } if (!access(path, R_OK)) add_excludes_from_file(dir, path); - if (!access(excludes_file, R_OK)) + if (excludes_file !access(excludes_file, R_OK)) add_excludes_from_file(dir, excludes_file); } diff --git a/t/t1306-xdg-files.sh b/t/t1306-xdg-files.sh index 097184b..6d9e8cd 100755 --- a/t/t1306-xdg-files.sh +++ b/t/t1306-xdg-files.sh @@ -108,6 +108,13 @@ test_expect_success 'Exclusion in a non-XDG global ignore file' ' test_must_fail git add to_be_excluded ' +test_expect_success 'Checking XDG ignore file when HOME is unset' ' + expected + (sane_unset HOME +git config --unset core.excludesfile +git ls-files --exclude-standard --ignored actual) + test_cmp expected actual +' test_expect_success 'Checking attributes in the XDG attributes file' ' echo foo f -- 1.7.11.2.402.g04c525e.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] difftool: Simplify print_tool_help()
On Sun, Jul 22, 2012 at 11:42 PM, David Aguilar dav...@gmail.com wrote: Eliminate a global variable and File::Find usage by building upon basename() and glob() instead. glob was used in an early revision of the patch that led to bf73fc2 (difftool: print list of valid tools with '--tool-help') as well [1]. However, if the path to git or the path under 'mergetools' includes spaces, glob fails. To work around the problem, File::Find was used instead [2]. Does this implementation handle that case? I'm sorry, but I haven't had time to apply and test myself. [1]: http://thread.gmane.org/gmane.comp.version-control.git/193233/focus=193925 [2]: http://thread.gmane.org/gmane.comp.version-control.git/194158 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 4/5] difftool: Use symlinks when diffing against the worktree
I'm sorry I am so late to see and comment on this...I am just getting
caught up after a few busy weeks due to $dayjob and vacation.
On Mon, Jul 23, 2012 at 2:05 AM, David Aguilar dav...@gmail.com wrote:
diff --git a/git-difftool.perl b/git-difftool.perl
index 2ae344c..a5b371f 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -271,6 +276,7 @@ sub main
gui = undef,
help = undef,
prompt = undef,
+ symlinks = $^O ne 'MSWin32' $^O ne 'msys',
Should this test for cygwin as well?
@@ -342,13 +350,18 @@ sub dir_diff
# If the diff including working copy files and those
# files were modified during the diff, then the changes
- # should be copied back to the working tree
- for my $file (@working_tree) {
- if (-e $b/$file compare($b/$file, $workdir/$file)) {
+ # should be copied back to the working tree.
+ # Do not copy back files when symlinks are used and the
+ # external tool did not replace the original link with a file.
+ for my $file (@worktree) {
+ next if $symlinks -l $b/$file;
+ if (-f $b/$file compare($b/$file, $workdir/$file)) {
compare returns '-1' if an error is encountered while reading a file.
In this (unlikely) case, should it still overwrite the working copy
file? I think the answer is 'yes', but thought it was worth
mentioning.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] t/lib-httpd: handle running under --valgrind
Running the http tests with valgrind does not work for two
reasons:
1. Apache complains about following the symbolic link from
git-http-backend to valgrind.sh.
2. Apache does not pass through the GIT_VALGRIND variable
to the backend CGI.
This patch fixes both problems. Unfortunately, there is a
slight hack we need to handle passing environment variables
through Apache. If we just tell it:
PassEnv GIT_VALGRIND
then Apache will complain when GIT_VALGRIND is not set. If
we try:
SetEnv GIT_VALGRIND ${GIT_VALGRIND}
then when GIT_VALGRIND is not set, it will pass through the
literal ${GIT_VALGRIND}. Instead, we now unconditionally
pass through GIT_VALGRIND from lib-httpd.sh into apache,
even if it is empty.
Signed-off-by: Jeff King p...@peff.net
---
I rolled this a few weeks ago while searching for a bug that turned out
not to be in git at all. But even without hunting a specific bug, it
allows us to expand our valgrind test coverage, which is a good thing.
t/lib-httpd.sh | 4
t/lib-httpd/apache.conf | 5 -
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index 094d490..d773542 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -43,6 +43,10 @@ TEST_PATH=$TEST_DIRECTORY/lib-httpd
HTTPD_ROOT_PATH=$PWD/httpd
HTTPD_DOCUMENT_ROOT_PATH=$HTTPD_ROOT_PATH/www
+# hack to suppress apache PassEnv warnings
+GIT_VALGRIND=$GIT_VALGRIND; export GIT_VALGRIND
+GIT_VALGRIND_OPTIONS=$GIT_VALGRIND_OPTIONS; export GIT_VALGRIND_OPTIONS
+
if ! test -x $LIB_HTTPD_PATH
then
skip_all=skipping test, no web server found at '$LIB_HTTPD_PATH'
diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf
index de3762e..36b1596 100644
--- a/t/lib-httpd/apache.conf
+++ b/t/lib-httpd/apache.conf
@@ -42,6 +42,9 @@ ErrorLog error.log
/IfModule
/IfVersion
+PassEnv GIT_VALGRIND
+PassEnv GIT_VALGRIND_OPTIONS
+
Alias /dumb/ www/
Alias /auth/ www/auth/
@@ -62,7 +65,7 @@ ScriptAlias /smart/ ${GIT_EXEC_PATH}/git-http-backend/
ScriptAlias /smart_noexport/ ${GIT_EXEC_PATH}/git-http-backend/
ScriptAlias /smart_custom_env/ ${GIT_EXEC_PATH}/git-http-backend/
Directory ${GIT_EXEC_PATH}
- Options None
+ Options FollowSymlinks
/Directory
Files ${GIT_EXEC_PATH}/git-http-backend
Options ExecCGI
--
1.7.11.3.4.g9f70dbb
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jul 2012, #07; Mon, 23)
On Mon, Jul 23, 2012 at 10:10:00PM -0700, Junio C Hamano wrote: * jc/test-lib-source-build-options-early (2012-06-24) 1 commit - test-lib: reorder and include GIT-BUILD-OPTIONS a lot earlier Reorders t/test-lib.sh so that we dot-source GIT-BUILD-OPTIONS that records the shell and Perl the user told us to use with Git a lot early, so that test-lib.sh script itself can use $PERL_PATH in one of its early operations. Needs to be eyeballed by people who run tests with exotic options like valgrind, --root=/dev/shm/somewhere, etc. I'm such a people. Both --valgrind and --root work OK with the patch. Reading the patch, I don't see any other problematic options, either. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jul 2012, #07; Mon, 23)
Jeff King p...@peff.net writes: On Mon, Jul 23, 2012 at 10:10:00PM -0700, Junio C Hamano wrote: * jc/test-lib-source-build-options-early (2012-06-24) 1 commit - test-lib: reorder and include GIT-BUILD-OPTIONS a lot earlier Reorders t/test-lib.sh so that we dot-source GIT-BUILD-OPTIONS that records the shell and Perl the user told us to use with Git a lot early, so that test-lib.sh script itself can use $PERL_PATH in one of its early operations. Needs to be eyeballed by people who run tests with exotic options like valgrind, --root=/dev/shm/somewhere, etc. I'm such a people. Both --valgrind and --root work OK with the patch. Reading the patch, I don't see any other problematic options, either. Thanks; I've been running tests for 'pu' with --root pointing elsewhere as well. I probably should push this forward before the tree gets busy again post relesae. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 4/5] difftool: Use symlinks when diffing against the worktree
Tim Henigan tim.heni...@gmail.com writes:
I'm sorry I am so late to see and comment on this...I am just getting
caught up after a few busy weeks due to $dayjob and vacation.
On Mon, Jul 23, 2012 at 2:05 AM, David Aguilar dav...@gmail.com wrote:
diff --git a/git-difftool.perl b/git-difftool.perl
index 2ae344c..a5b371f 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -271,6 +276,7 @@ sub main
gui = undef,
help = undef,
prompt = undef,
+ symlinks = $^O ne 'MSWin32' $^O ne 'msys',
Should this test for cygwin as well?
@@ -342,13 +350,18 @@ sub dir_diff
# If the diff including working copy files and those
# files were modified during the diff, then the changes
- # should be copied back to the working tree
- for my $file (@working_tree) {
- if (-e $b/$file compare($b/$file, $workdir/$file)) {
+ # should be copied back to the working tree.
+ # Do not copy back files when symlinks are used and the
+ # external tool did not replace the original link with a file.
+ for my $file (@worktree) {
+ next if $symlinks -l $b/$file;
+ if (-f $b/$file compare($b/$file, $workdir/$file)) {
compare returns '-1' if an error is encountered while reading a file.
In this (unlikely) case, should it still overwrite the working copy
file? I think the answer is 'yes', but thought it was worth
mentioning.
It probably is safer to report the error, not touch anything and let
the user take an appropriate action.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Enhanced git branch list (proposal)
On Mon, Jul 23, 2012 at 2:17 PM, John Bartholomew
jpa.bartholo...@gmail.com wrote:
I find the output of `git branch' to be quite bare, and would like to
see more information; most importantly, what the state of the branch
is in relation to its upstream. For some time I have been using my
own script to do this. It produces output like this:
$ git lsb
commodity-market-lua [behind 'brianetta/commodity-market-lua' by 2
commits]
filesystem [up-to-date with 'jpab/filesystem']
fix-ring-blending [ahead of 'jpab/fix-ring-blending' by 1 commit]
galaxy-refactor
galaxy-refactor-2 [diverged from 'jpab/galaxy-refactor', by 6
commits/626 commits (us/them)]
hud-pitch-ladder [up-to-date with 'jpab/hud-pitch-ladder']
= issue-1388
issue-695
lmr-mtllib-improvements
marcel-stations
* master [up-to-date with 'jpab/master']
refcounted-body [up-to-date with 'jpab/refcounted-body']
string-formatter [up-to-date with 'jpab/string-formatter']
The first column indicates the relation to HEAD: '*' marks the current
head, '=' marks a branch which is identical with the current HEAD.
Branches which have a configured upstream (branch.remote and
branch.merge are set) show the relation to the corresponding remote
branch.
Some key text ('up-to-date', 'ahead', 'behind' or 'diverged', and the
name of the current HEAD) is displayed with colour if colour is
enabled.
Arguments can be passed to show remote branches (for all remotes, or
for a specified remote), or all branches, and to show each branch
in relation to a specified target branch instead of the configured
remote tracking branch.
I would like to know whether there is any interest in incorporating
this functionality into the main git distribution, either as a
separate command, or within `git branch'. For my purposes I have it
aliased under the name `git lsb' for `list branches'.
You can examine the script I'm using for this at:
https://github.com/johnbartholomew/gitvoodoo/blob/master/bin/git-xbranch
Thanks. You might also find this one interesting:
http://masanjin.net/blog/label/git-wtf/
Phil
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GSoC] Designing a faster index format - Progress report week 13
Thomas Rast tr...@student.ethz.ch writes: Junio's index-v4 was a speed boost mainly because it cuts down on the size of the index. Do we want to throw that out? That's pretty much orthogonal, isn't it? The index-v4 is merely to show how a stupid prefix compression of pathnames without nothing else would reduce the file size and I/O cost, in order to set the bar for anything more clever than that. I thought that this discussion is about keeping, squishing, or discarding part of the cached stat info, and nobody is suggesting what to do with the prefix compression of pathnames. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/7] i18n: New keywords for xgettext extraction from sh
Jiang Xin wrote: Since we have additional shell wrappers (gettextln and eval_gettextln) for gettext, we need to take into account these wrappers when run 'make pot' to extract messages from shell scripts. Yes, thanks for fixing it. As Stefano mentioned, s/run/running/ would make the above clearer. With or without that change, Reviewed-by: Jonathan Nieder jrnie...@gmail.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 4/7] Remove obsolete LONG_USAGE which breaks xgettext
Hi, Jiang Xin wrote: The obsolete LONG_USAGE variable [...] It's a shame to lose the information that was in the LONG_USAGE message, though. Maybe it could be incorporated into the OPTIONS_SPEC before the opening --, or maybe it could be used to clarify the description in git-rebase(1). Cc-ing Martin who carried out the parseoptification for advice. Patch left unsnipped below for reference. Thanks and hope that helps, Jonathan [...] --- a/git-rebase.sh +++ b/git-rebase.sh @@ -3,31 +3,6 @@ # Copyright (c) 2005 Junio C Hamano. # -USAGE='[--interactive | -i] [--exec | -x cmd] [-v] [--force-rebase | -f] - [--no-ff] [--onto newbase] [upstream|--root] [branch] [--quiet | -q]' -LONG_USAGE='git-rebase replaces branch with a new branch of the -same name. When the --onto option is provided the new branch starts -out with a HEAD equal to newbase, otherwise it is equal to upstream -It then attempts to create a new commit for each commit from the original -branch that does not exist in the upstream branch. - -It is possible that a merge failure will prevent this process from being -completely automatic. You will have to resolve any such merge failure -and run git rebase --continue. Another option is to bypass the commit -that caused the merge failure with git rebase --skip. To check out the -original branch and remove the .git/rebase-apply working files, use the -command git rebase --abort instead. - -Note that if branch is not specified on the command line, the -currently checked out branch is used. - -Example: git-rebase master~1 topic - - A---B---C topic A'\''--B'\''--C'\'' topic - / -- / - D---E---F---G master D---E---F---G master -' - SUBDIRECTORY_OK=Yes OPTIONS_KEEPDASHDASH= OPTIONS_SPEC=\ -- 1.7.12.rc0.17.gcb766d3 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/7] i18n: rebase: mark strings for translation
Hi,
Jiang Xin wrote:
Mark strings in git-rebase.sh for translation.
Thanks.
[...]
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -65,6 +65,7 @@ abort! abort and check out the original branch
skip! skip current patch and continue
. git-sh-setup
+. git-sh-i18n
set_reflog_action rebase
require_work_tree_exists
cd_to_toplevel
@@ -72,11 +73,11 @@ cd_to_toplevel
LF='
'
ok_to_skip_pre_rebase=
-resolvemsg=
-When you have resolved this problem run \git rebase --continue\.
-If you would prefer to skip this patch, instead run \git rebase --skip\.
-To check out the original branch and stop rebasing run \git rebase
--abort\.
-
+resolvemsg=$(gettext '
+When you have resolved this problem run git rebase --continue.
+If you would prefer to skip this patch, instead run git rebase --skip.
+To check out the original branch and stop rebasing run git rebase --abort.
+')
Functional change: command substitution strips off the trailing newline.
Intentional?
Probably it would make sense to do
resolvemsg=
$(gettext 'When you have resolved this problem, run git rebase
--continue.
If you prefer to skip this patch, run git rebase --skip instead.
To check out the original branch and stop rebasing, run git rebase
--abort.')
anyway, so the translators could have fewer newlines at the edges to
fuss about.
[...]
git diff-files --quiet --ignore-submodules || {
- echo You must edit all merge conflicts and then
- echo mark them as resolved using git add
+ echo $(gettext You must edit all merge conflicts and then
+mark them as resolved using git add)
exit 1
Nice.
[...]
@@ -367,15 +368,16 @@ esac
# Make sure no rebase is in progress
if test -n $in_progress
then
- die '
-It seems that there is already a '${state_dir##*/}' directory, and
+ state_dir_base=${state_dir##*/}
+ die $(eval_gettext
+It seems that there is already a \$state_dir_base directory, and
I wonder if you are in the middle of another rebase. If that is the
case, please try
git rebase (--continue | --abort | --skip)
If that is not the case, please
- rm -fr '$state_dir'
+ rm -fr \\$state_dir\
and run me again. I am stopping in case you still have something
-valuable there.'
+valuable there.)
Maybe, to allow changing the commands without having to update
translations:
state_dir_base=...
cmd_live_rebase='git rebase (--continue | --abort | --skip)'
cmd_clear_stale_rebase=rm -fr \$state_dir\
die
$(eval_gettext 'It seems that there is already a $state_dir_base
directory, and
I wonder if you ware in the middle of another rebase. If that is the
case, please try
$cmd_live_rebase
If that is not the case, please
$cmd_clear_stale_rebase
and run me again. I am stopping in case you still have something
valuable there.')
[...]
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -68,24 +68,24 @@ test_expect_success 'rebase against master' '
Thanks for updating tests! The expected output you had to change all
seems to be intended for humans, which is a good sign.
[...]
--- a/t/t3406-rebase-message.sh
+++ b/t/t3406-rebase-message.sh
@@ -64,7 +64,7 @@ test_expect_success 'rebase -n overrides config rebase.stat
config' '
test_expect_success 'rebase --onto outputs the invalid ref' '
test_must_fail git rebase --onto invalid-ref HEAD HEAD 2err
- grep invalid-ref err
+ test_i18ngrep invalid-ref err
'
This is probably part of a message intended for humans, but the test
does not say. What is the full message being checked?
Hope that helps,
Jonathan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 5/7] i18n: am: mark more strings for translation
Hi, Jiang Xin wrote: Mark additional 3 strings for translation, and reduce one indentation level for one gettextln clause introduced in commit de88c1c. The above description doesn't mention: [...] @@ -387,8 +386,8 @@ do -i|--interactive) interactive=t ;; -b|--binary) - echo 2 The $1 option has been a no-op for long time, and - echo 2 it will be removed. Please do not use it anymore. + echo 2 $(gettext The -b option has been a no-op for long time, and +it will be removed. Please do not use it anymore.) ... that this changes the message when the --binary option is passed. Before this patch, it says The --binary option has been a no-op for a long time, and ... After the patch, it says The -b option has been a no-op for a long time, and ... Intentional? That may be a good change or a bad one (I haven't thought clearly about it), but it seems at least worth mentioning. Cc-ing Thomas in case he has advice. Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 6/7] Remove unused and bad gettext block from git-am
Hi, Jiang Xin wrote: Gettext message should not start with '-' nor '--'. Since the '-d' and '--dotest' options do not exist in OPTIONS_SPEC variable, it's safe to remove the block. The above justification is not a sufficient reason to stop giving helpful advice when someone uses an option that was historically supported: --- a/git-am.sh +++ b/git-am.sh @@ -413,9 +413,6 @@ it will be removed. Please do not use it anymore.) abort=t ;; --rebasing) rebasing=t threeway=t ;; - -d|--dotest) - die $(gettext -d option is no longer supported. Do not use.) - ;; Luckily the support was removed 4 years ago and I don't think anyone is going to run into this, so a different justification could apply. Support for the git am -d/--dotest option was removed four years ago (see e72c7406, am: remove support for -d .dotest, 2008-03-04) and presumably no one is trying to use it any more. Simplify the code and free the short-and-sweet -d option for other uses by no longer parsing it. The motivation: xgettext copes poorly with messages starting with '-'. Rather than fixing this ancient message, let's take this as a reminder to remove it. See http://thread.gmane.org/gmane.comp.version-control.git/75896 for context. Though also see http://thread.gmane.org/gmane.comp.security.selinux/1424/focus=1430 Hope that helps, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/7] i18n: rebase: mark strings for translation
Jonathan Nieder jrnie...@gmail.com writes: Probably it would make sense to do resolvemsg= $(gettext 'When you have resolved this problem, run git rebase --continue. If you prefer to skip this patch, run git rebase --skip instead. To check out the original branch and stop rebasing, run git rebase --abort.') anyway, so the translators could have fewer newlines at the edges to fuss about. Nice. Maybe, to allow changing the commands without having to update translations: state_dir_base=... cmd_live_rebase='git rebase (--continue | --abort | --skip)' cmd_clear_stale_rebase=rm -fr \$state_dir\ die $(eval_gettext 'It seems that there is already a $state_dir_base directory, and I wonder if you ware in the middle of another rebase. If that is the case, please try $cmd_live_rebase If that is not the case, please $cmd_clear_stale_rebase and run me again. I am stopping in case you still have something valuable there.') Again, nice. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 4/7] Remove obsolete LONG_USAGE which breaks xgettext
Jonathan Nieder jrnie...@gmail.com writes: Jiang Xin wrote: The obsolete LONG_USAGE variable [...] It's a shame to lose the information that was in the LONG_USAGE message, though. Maybe it could be incorporated into the OPTIONS_SPEC before the opening --, or maybe it could be used to clarify the description in git-rebase(1). I personally think the original long-usage was overkill to be part of the help text, and I am happy to see it go. I wouldn't mind seeing it incorporated in the documentation if there is something in there that is missing, but I suspect that the first part of the DESCRIPTION should be sufficiently clear already. [...] --- a/git-rebase.sh +++ b/git-rebase.sh @@ -3,31 +3,6 @@ # Copyright (c) 2005 Junio C Hamano. # -USAGE='[--interactive | -i] [--exec | -x cmd] [-v] [--force-rebase | -f] - [--no-ff] [--onto newbase] [upstream|--root] [branch] [--quiet | -q]' -LONG_USAGE='git-rebase replaces branch with a new branch of the -same name. When the --onto option is provided the new branch starts -out with a HEAD equal to newbase, otherwise it is equal to upstream -It then attempts to create a new commit for each commit from the original -branch that does not exist in the upstream branch. - -It is possible that a merge failure will prevent this process from being -completely automatic. You will have to resolve any such merge failure -and run git rebase --continue. Another option is to bypass the commit -that caused the merge failure with git rebase --skip. To check out the -original branch and remove the .git/rebase-apply working files, use the -command git rebase --abort instead. - -Note that if branch is not specified on the command line, the -currently checked out branch is used. - -Example: git-rebase master~1 topic - -A---B---C topic A'\''--B'\''--C'\'' topic - / -- / - D---E---F---G master D---E---F---G master -' - SUBDIRECTORY_OK=Yes OPTIONS_KEEPDASHDASH= OPTIONS_SPEC=\ -- 1.7.12.rc0.17.gcb766d3 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH v2] t3300-*.sh: Fix a TAP parse error
Hi, Ramsay Jones wrote: Hi Jonathan, This version of the patch only moves code to determine the test prerequisite to the outer level, while leaving the 'setup' aspects of the first test in place. I guess I don't see the point. The current convention of don't do anything complicated outside test assertions is easy to explain. What new convention are you suggesting to replace it? Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH v2] t3300-*.sh: Fix a TAP parse error
At present, running the t3300-*.sh test on cygwin looks like:
$ cd t
$ ./t3300-funny-names.sh
ok 1 - setup
# passed all 1 test(s)
1..1 # SKIP Your filesystem does not allow tabs in filenames
$
Unfortunately, this is not valid TAP output, which prove notes
as follows:
$ prove --exec sh t3300-funny-names.sh
t3300-funny-names.sh .. All 1 subtests passed
Test Summary Report
---
t3300-funny-names.sh (Wstat: 0 Tests: 1 Failed: 0)
Parse errors: No plan found in TAP output
Files=1, Tests=1, 2 wallclock secs ( 0.05 usr 0.00 sys + \
0.90 cusr 0.49 csys = 1.43 CPU)
Result: FAIL
$
This is due to the 'trailing_plan' having a 'skip_directive'
attached to it. This is not allowed by the TAP grammar, which
only allows a 'leading_plan' to be followed by an optional
'skip_directive'. (see perldoc TAP::Parser::Grammar).
A trailing_plan is one that appears in the TAP output after one or
more test status lines (that start 'not '? 'ok ' ...), whereas a
leading_plan must appear before all test status lines (if any).
In practice, this means that the test script cannot contain a use
of the 'skip all' facility:
skip_all='Some reason to skip *all* tests in this file'
test_done
after having already executed one or more tests with (for example)
'test_expect_success'. Unfortunately, this is exactly what this
test script is doing. The first 'setup' test is actually used to
determine if the test prerequisite is satisfied by the filesystem
(ie does it allow tabs in filenames?).
In order to fix the parse errors, place the code to determine the
test prerequisite at the top level of the script, prior to the
first test, rather than as a parameter to test_expect_success.
This allows us to correctly use 'skip_all', thus:
$ ./t3300-funny-names.sh
# passed all 0 test(s)
1..0 # SKIP Your filesystem does not allow tabs in filenames
$
$ prove --exec sh t3300-funny-names.sh
t3300-funny-names.sh .. skipped: Your filesystem does not \
allow tabs in filenames
Files=1, Tests=0, 2 wallclock secs ( 0.02 usr 0.03 sys + \
0.84 cusr 0.41 csys = 1.29 CPU)
Result: NOTESTS
$
Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk
---
Hi Jonathan,
This version of the patch only moves code to determine the test
prerequisite to the outer level, while leaving the 'setup' aspects
of the first test in place.
I did think about merging the following test (setup: populate index
and tree) into the first test, but I wanted to keep it simple for now.
What do you think?
ATB,
Ramsay Jones
t/t3300-funny-names.sh | 24 +++-
1 file changed, 11 insertions(+), 13 deletions(-)
diff --git a/t/t3300-funny-names.sh b/t/t3300-funny-names.sh
index 1f35e55..7480d6e 100755
--- a/t/t3300-funny-names.sh
+++ b/t/t3300-funny-names.sh
@@ -11,6 +11,16 @@ tree, index, and tree objects.
. ./test-lib.sh
+HT=' '
+
+echo 2/dev/null Name with an${HT}HT
+if ! test -f Name with an${HT}HT
+then
+ # since FAT/NTFS does not allow tabs in filenames, skip this test
+ skip_all='Your filesystem does not allow tabs in filenames'
+ test_done
+fi
+
p0='no-funny'
p1='tabs , (dq) and spaces'
p2='just space'
@@ -23,21 +33,9 @@ test_expect_success 'setup' '
EOF
{ cat $p0 $p1 || :; }
- { echo Foo Bar Baz $p2 || :; }
-
- if test -f $p1 cmp $p0 $p1
- then
- test_set_prereq TABS_IN_FILENAMES
- fi
+ { echo Foo Bar Baz $p2 || :; }
'
-if ! test_have_prereq TABS_IN_FILENAMES
-then
- # since FAT/NTFS does not allow tabs in filenames, skip this test
- skip_all='Your filesystem does not allow tabs in filenames'
- test_done
-fi
-
test_expect_success 'setup: populate index and tree' '
git update-index --add $p0 $p2
t0=$(git write-tree)
--
1.7.11.2
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] t3300-*.sh: Fix a TAP parse error
Jonathan Nieder wrote:
Needless to say, I much prefer the patch below. :-D
Thanks for a nice explanation. In general I definitely like getting
rid of these setup tests when possible. Let's see:
[...]
--- a/t/t3300-funny-names.sh
+++ b/t/t3300-funny-names.sh
@@ -15,28 +15,20 @@ p0='no-funny'
p1='tabs, (dq) and spaces'
p2='just space'
-test_expect_success 'setup' '
-cat $p0 -\EOF
-1. A quick brown fox jumps over the lazy cat, oops dog.
-2. A quick brown fox jumps over the lazy cat, oops dog.
-3. A quick brown fox jumps over the lazy cat, oops dog.
-EOF
+cat $p0 \EOF
+1. A quick brown fox jumps over the lazy cat, oops dog.
+2. A quick brown fox jumps over the lazy cat, oops dog.
+3. A quick brown fox jumps over the lazy cat, oops dog.
+EOF
The problem is that on platforms not supporting funny filenames, it
will write a complaint to stderr and because the code is not guarded
by test_expect_success, that output goes to the terminal. So I think
this is a wrong approach.
Huh? Which platforms are we talking about?
The only problematic platforms I test on are NTFS/bash on cygwin and MinGW.
Since commit 2b843732 (Suppress some bash redirection error messages,
26-08-2008), I have not noticed any complaints regarding this problem.
What have I missed?
Assuming we are not talking about errors like ENOSPC, EROFS etc., then the
only command which would issue a complaint to stderr would be the line
following the above snippet, thus:
+cat 2/dev/null $p1 $p0
(note the stderr redirection). This does not output an error to the terminal
when using bash (I think I also tested with dash). However, this does rely
on the shell performing the redirections in the order, left to right, on the
command line. [I had intended to check with POSIX to see if this order was
mandated or not, but didn't get around to it ...]
Have you found a shell were this does not work?
Would it make sense to avoid the # SKIP comment when a test has
been run, like this?
diff --git i/t/test-lib.sh w/t/test-lib.sh
index acda33d1..038f6e9f 100644
--- i/t/test-lib.sh
+++ w/t/test-lib.sh
@@ -354,6 +354,11 @@ test_done () {
case $test_failure in
0)
# Maybe print SKIP message
+ if test -n $skip_all test $test_count != 0
+ then
+ say # SKIP $skill_all
+ skip_all=
+ fi
[ -z $skip_all ] || skip_all= # SKIP $skip_all
if test $test_external_has_tap -eq 0; then
No, I don't think this would be a good direction to go in. This may
not be a good idea either, but if you wanted to add a check here, then
maybe something like this (totally untested):
diff --git a/t/test-lib.sh b/t/test-lib.sh
index acda33d..53a2422 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -354,6 +354,9 @@ test_done () {
case $test_failure in
0)
# Maybe print SKIP message
+ if test -n $skip_all test $test_count -gt 0; then
+ error Can't use skip_all after running some tests
+ fi
[ -z $skip_all ] || skip_all= # SKIP $skip_all
if test $test_external_has_tap -eq 0; then
Dunno! :-D
I will be sending a v2 patch soon.
ATB,
Ramsay Jones
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] t3300-*.sh: Fix a TAP parse error
Hi,
Ramsay Jones wrote:
The only problematic platforms I test on are NTFS/bash on cygwin and MinGW.
Since commit 2b843732 (Suppress some bash redirection error messages,
26-08-2008), I have not noticed any complaints regarding this problem.
Yeah, that probably took care of squashing the messages. Maybe my
memory is too long. ;-)
[...]
No, I don't think this would be a good direction to go in. This may
not be a good idea either, but if you wanted to add a check here, then
maybe something like this (totally untested):
diff --git a/t/test-lib.sh b/t/test-lib.sh
index acda33d..53a2422 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -354,6 +354,9 @@ test_done () {
case $test_failure in
0)
# Maybe print SKIP message
+ if test -n $skip_all test $test_count -gt 0; then
+ error Can't use skip_all after running some tests
+ fi
[ -z $skip_all ] || skip_all= # SKIP $skip_all
if test $test_external_has_tap -eq 0; then
I think preventing invalid TAP output like this would be a very good
thing! As a start, this patchlet looks good to me, and then I guess
we'll have to think more about what happens when a person wants to
skip_all_remaining after a test has already been run.
Care to format it as a git am-able patch, or should I?
Thanks again,
Jonathan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rebase -i: handle fixup of root commit correctly
Chris Webb ch...@arachsys.com writes:
There is a bug with git rebase -i --root when a fixup or squash line is
applied to the new root. We attempt to amend the commit onto which they
apply with git reset --soft HEAD^ followed by a normal commit. Unlike a
real commit --amend, this sequence will fail against a root commit as it
has no parent.
Fix rebase -i to use commit --amend for fixup and squash instead, and
add a test for the case of a fixup of the root commit.
Signed-off-by: Chris Webb ch...@arachsys.com
---
Sorry, I should have spotted this issue when I did the original root-rebase
series. I've checked that this patch doesn't break any of the existing
tests, as well as satisfying the newly introduced check for the root-fixup
case.
OK, so instead of reset --soft HEAD^ pick -n commit -F msg
to back up one step and then build on top of it, the new sequence
pick -n commit --amend -F msg modifies and then amends, whose
end result should be the same but the important difference is that
the latter would work even if the current commit is a root one.
Makes sense. Thanks for catching and fixing it.
git-rebase--interactive.sh| 25 +
t/t3404-rebase-interactive.sh | 8
2 files changed, 21 insertions(+), 12 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index bef7bc0..0d2056f 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -493,25 +493,28 @@ do_next () {
author_script_content=$(get_author_ident_from_commit HEAD)
echo $author_script_content $author_script
eval $author_script_content
- output git reset --soft HEAD^
- pick_one -n $sha1 || die_failed_squash $sha1 $rest
+ if ! pick_one -n $sha1
+ then
+ git rev-parse --verify HEAD $amend
+ die_failed_squash $sha1 $rest
+ fi
case $(peek_next_command) in
squash|s|fixup|f)
# This is an intermediate commit; its message will only
be
# used in case of trouble. So use the long version:
- do_with_author output git commit --no-verify -F
$squash_msg ||
+ do_with_author output git commit --amend --no-verify -F
$squash_msg ||
die_failed_squash $sha1 $rest
;;
*)
# This is the final command of this squash/fixup group
if test -f $fixup_msg
then
- do_with_author git commit --no-verify -F
$fixup_msg ||
+ do_with_author git commit --amend --no-verify
-F $fixup_msg ||
die_failed_squash $sha1 $rest
else
cp $squash_msg $GIT_DIR/SQUASH_MSG || exit
rm -f $GIT_DIR/MERGE_MSG
- do_with_author git commit --no-verify -e ||
+ do_with_author git commit --amend --no-verify
-F $GIT_DIR/SQUASH_MSG -e ||
die_failed_squash $sha1 $rest
fi
rm -f $squash_msg $fixup_msg
@@ -748,7 +751,6 @@ In both case, once you're done, continue with:
fi
. $author_script ||
die Error trying to find the author identity to amend
commit
- current_head=
if test -f $amend
then
current_head=$(git rev-parse --verify HEAD)
@@ -756,13 +758,12 @@ In both case, once you're done, continue with:
die \
You have uncommitted changes in your working tree. Please, commit them
first and then run 'git rebase --continue' again.
- git reset --soft HEAD^ ||
- die Cannot rewind the HEAD
+ do_with_author git commit --amend --no-verify -F $msg
-e ||
+ die Could not commit staged changes.
+ else
+ do_with_author git commit --no-verify -F $msg -e ||
+ die Could not commit staged changes.
fi
- do_with_author git commit --no-verify -F $msg -e || {
- test -n $current_head git reset --soft
$current_head
- die Could not commit staged changes.
- }
fi
record_in_rewritten $(cat $state_dir/stopped-sha)
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 8078db6..3f75d32 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -903,4 +903,12 @@ test_expect_success 'rebase -i --root temporary sentinel
commit' '
git rebase
Re: [PATCH v2 1/3] fast-import: do not write null_sha1 as a merge parent
Hi,
In June, Dmitry Ivankov wrote:
null_sha1 is used in fast-import to indicate empty branches and
should never be actually written out as a commit parent. 'merge'
command lacks is_null_sha1 checks and must be fixed.
It looks like using null_sha1 or empty branches in 'from' command
is legal and/or an intended option (it has been here from the very
beginning and survived). So leave it allowed for 'merge' command too,
and just like with 'from' command silently skip null_sha1 parents.
As Junio mentioned, this might have just been an implementation
accident --- without a use case in mind, it is hard to say that
support for the 'from 0{40}' was really intended to be part of the
supported fast-import syntax.
On the other hand it seems possible and even likely that some frontend
has taken advantage of the feature to avoid having to use conditional
logic to decide whether to emit a from command, since it has been
around so long. So you are right that it's safest not to remove it.
That means that adding the same support for the merge command could
be a pretty bad thing, since it would be making a new promise of
continued support and would place a new burden on other implementers
of backends.
[...]
--- a/fast-import.c
+++ b/fast-import.c
@@ -2734,7 +2734,8 @@ static void parse_new_commit(void)
strbuf_addf(new_data, parent %s\n, sha1_to_hex(b-sha1));
while (merge_list) {
struct hash_list *next = merge_list-next;
- strbuf_addf(new_data, parent %s\n,
sha1_to_hex(merge_list-sha1));
+ if (!is_null_sha1(merge_list-sha1))
+ strbuf_addf(new_data, parent %s\n,
sha1_to_hex(merge_list-sha1));
Since these merge commands produced invalid results in the past,
would it be safe to do
if (is_null_sha1(merge_list-sha1))
die(cannot use unborn branch or all-zeroes hash as
merge parent;
instead?
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -850,6 +850,27 @@ INPUT_END
test_expect_success \
'J: tag must fail on empty branch' \
'test_must_fail git fast-import input'
+
+cat input INPUT_END
+reset refs/heads/J3
+
+reset refs/heads/J4
+from
+
+commit refs/heads/J5
+committer $GIT_COMMITTER_NAME $GIT_COMMITTER_EMAIL $GIT_COMMITTER_DATE
+data COMMIT
+Merge J3, J4 into fresh J5.
+COMMIT
+merge refs/heads/J3
+merge refs/heads/J4
+
+INPUT_END
+test_expect_success \
+ 'J: allow merge with empty branch' \
+ 'git fast-import input
+ git rev-parse --verify J5
+ test_must_fail git rev-parse --verify J5^'
Thanks for the test --- in any case, we should test the behavior. How
about this, for now?
-- 8 --
From: Dmitry Ivankov divanor...@gmail.com
Subject: test: demonstrate fast-import bug that produces invalid commits with
null parent
null_sha1 is used in fast-import to indicate empty branches and
should never be actually written out as a commit parent. 'merge'
command lacks is_null_sha1 checks and must be fixed.
[jn: extracted from a patch with a proposed fix; split into two tests]
Signed-off-by: Dmitry Ivankov divanor...@gmail.com
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
t/t9300-fast-import.sh | 36
1 file changed, 36 insertions(+)
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 2fcf2694..f13b85b8 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -850,6 +850,42 @@ INPUT_END
test_expect_success \
'J: tag must fail on empty branch' \
'test_must_fail git fast-import input'
+
+cat input INPUT_END
+reset refs/heads/J-unborn
+
+commit refs/heads/J-merge-unborn
+committer $GIT_COMMITTER_NAME $GIT_COMMITTER_EMAIL $GIT_COMMITTER_DATE
+data COMMIT
+Merge J-unborn into fresh J-merge-unborn.
+COMMIT
+merge refs/heads/J-unborn
+
+INPUT_END
+test_expect_failure \
+ 'J: reject or ignore merge with unborn branch' \
+ 'test_when_finished git update-ref -d refs/heads/J-merge-unborn
+test_might_fail git fast-import input
+git fsck'
+
+cat input INPUT_END
+reset refs/heads/J-null-sha1
+from
+
+commit refs/heads/J-merge-null
+committer $GIT_COMMITTER_NAME $GIT_COMMITTER_EMAIL $GIT_COMMITTER_DATE
+data COMMIT
+Merge J-null-sha1 into fresh J-merge-null.
+COMMIT
+merge refs/heads/J-null-sha1
+
+INPUT_END
+test_expect_failure \
+ 'J: reject or ignore merge with unborn branch' \
+ 'test_when_finished git update-ref -d refs/heads/J-merge-null
+test_might_fail git fast-import input
+git fsck'
+
###
### series K
###
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Add a svnrdump-simulator replaying a dump file for testing.
Hi, Junio C Hamano wrote: Florian Achleitner florian.achleitner.2.6...@gmail.com writes: To ease testing without depending on a reachable svn server, this compact python script mimics parts of svnrdumps behaviour. It requires the remote url to start with sim://. [...] To allow using the same dump file for simulating multiple incremental imports the highest visible revision can be limited by setting the environment variable SVNRMAX to that value. This effectively limits HEAD to simulate the situation where higher revs don't exist yet. It is unclear how this is different from giving the ceiling by specifying it as the END in -rSTART:END command line. Is this feature really needed? I think the idea is that you put this script (or a symlink to it) on your $PATH with higher precedence than svnrdump and run a command that expected to be able to use svnrdump. Then instead of going to the network, the command you run magically uses your test data instead. If the command you are testing wanted to run svnrdump without the upper endpoint set, we need to handle that request, either by emitting all the revs we have, or by stopping somewhere. The revlimit feature provides the stopping somewhere behavior which is not strictly needed but is presumably very useful when testing incremental fetch. Florian, do you mind if I make the revlimit feature a separate patch when applying this? Anyway, it looks good and reasonable to me, so will apply. Thanks. Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] t3300-*.sh: Fix a TAP parse error
Ramsay Jones ram...@ramsay1.demon.co.uk writes: The only problematic platforms I test on are NTFS/bash on cygwin and MinGW. Since commit 2b843732 (Suppress some bash redirection error messages, 26-08-2008), I have not noticed any complaints regarding this problem. What have I missed? Assuming we are not talking about errors like ENOSPC, EROFS etc., then the only command which would issue a complaint to stderr would be the line following the above snippet, thus: +cat 2/dev/null $p1 $p0 (note the stderr redirection). As I am more worried about longer-term health of the codebase, what the part you moved outside test_expect_* with this patch happens to do _right now_ is of secondary importance, at least from my point of view. The next patch that updates this scirpt may want to run more involved commands that can fail in different ways. Being able to rely on the protection test_expect_* gives us in the set-up phase of the test has been very valuable (in addition to making the result more readable) to us. Can we keep that property in some way while also keeping the ability to skip the remainder of the test script? Observing that all well-written test scripts we have begin with this boilerplate line: test_expect_success setup ' I wouldn't mind introducing a new helper function test_setup that behaves like test_expect_success but is meant to be used in the first set-up phase of the tests in a test script. Perhaps we can give its failure a meaning different than failures in other normal tests (e.g. even set-up fails, and the remainder of the test is N/A here, hence abort the whole thing), and do not count test_setup as part of the test (i.e. do not increment $test_count and friends). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug: Recursive submodules fail when the repo path contains spaces
Am 24.07.2012 21:01, schrieb Justin Spahr-Summers: This occurs on Mac OS X 10.7.4, on git versions 1.7.10.2 (Apple Git-33) and 1.7.11.3. Steps: 1. Create or clone a repository to an absolute path that contains spaces. 2. Add a submodule to the repository, if it does not already have one. 3. Within that submodule, attempt to add another submodule. The result is an error fatal: Not a git repository, followed by the relative path to the submodule directory within .git/modules of the top-level repository. Similarly, using git submodule update --init --recursive in a freshly-cloned repository that matches the above configuration will fail with the same error. git clone --recursive does not seem to suffer from the same problem at clone time, but will still fail to add recursive submodules. Hmm, I don't understand how that is different from what t7407 does, it uses git submodule update --init --recursive in to populate recursive submodules in a freshly cloned repository whose path contains a space (in the trash directory name) in test number 8. Does the git test suite succeed for you? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug: Recursive submodules fail when the repo path contains spaces
Jens Lehmann jens.lehm...@web.de writes: Am 24.07.2012 21:01, schrieb Justin Spahr-Summers: This occurs on Mac OS X 10.7.4, on git versions 1.7.10.2 (Apple Git-33) and 1.7.11.3. Steps: 1. Create or clone a repository to an absolute path that contains spaces. 2. Add a submodule to the repository, if it does not already have one. 3. Within that submodule, attempt to add another submodule. The result is an error fatal: Not a git repository, followed by the relative path to the submodule directory within .git/modules of the top-level repository. Similarly, using git submodule update --init --recursive in a freshly-cloned repository that matches the above configuration will fail with the same error. git clone --recursive does not seem to suffer from the same problem at clone time, but will still fail to add recursive submodules. Hmm, I don't understand how that is different from what t7407 does, it uses git submodule update --init --recursive in to populate recursive submodules in a freshly cloned repository whose path contains a space (in the trash directory name) in test number 8. I can see one codepath that would behave incorrectly, especially if the submodule path relative to the superproject has whitespaces in it. In module_clone(), you have: # We already are at the root of the work tree but cd_to_toplevel will # resolve any symlinks that might be present in $PWD a=$(cd_to_toplevel cd $gitdir pwd)/ b=$(cd_to_toplevel cd $sm_path pwd)/ ... # Turn each leading */ component into ../ rel=$(echo $b | sed -e 's|[^/][^/]*|..|g') echo gitdir: $rel/$a $sm_path/.git I _think_ $sm_path is computed correctly by the codeflow leading to this place, and $b is also computed correctly, but notice the lack of quoting around $b when you echo it? It will be split at $IFS, so if b='/Program Files/My Stupidity/', the sed script will see a single SP between My and Stupidity, which is different from what you wanted to feed, I presume. Having said that, I do not think git-submodule is prepared to take paths with path-unsafe characters in it, given that many part of it has loops like while read mode sha1 stage sm_path that reads from ls-files/ls-tree output without -z (which means it cannot handle pathnames with LF in them). My recommendation at this point (i.e. not a long term) for people with problems Justin saw is Don't do it then. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug: Recursive submodules fail when the repo path contains spaces
On Tuesday, 24. July 2012 at 13:08, Jens Lehmann wrote: Am 24.07.2012 21:01, schrieb Justin Spahr-Summers: This occurs on Mac OS X 10.7.4, on git versions 1.7.10.2 (Apple Git-33) and 1.7.11.3. Steps: 1. Create or clone a repository to an absolute path that contains spaces. 2. Add a submodule to the repository, if it does not already have one. 3. Within that submodule, attempt to add another submodule. The result is an error fatal: Not a git repository, followed by the relative path to the submodule directory within .git/modules of the top-level repository. Similarly, using git submodule update --init --recursive in a freshly-cloned repository that matches the above configuration will fail with the same error. git clone --recursive does not seem to suffer from the same problem at clone time, but will still fail to add recursive submodules. Hmm, I don't understand how that is different from what t7407 does, it uses git submodule update --init --recursive in to populate recursive submodules in a freshly cloned repository whose path contains a space (in the trash directory name) in test number 8. Does the git test suite succeed for you? The only test that fails is: *** t7501-commit.sh *** *** t7502-commit.sh *** … not ok - 13 commit message from template with whitespace issue (which I assume is unrelated) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug: Recursive submodules fail when the repo path contains spaces
On Tuesday, 24. July 2012 at 13:26, Junio C Hamano wrote: Jens Lehmann jens.lehm...@web.de (http://web.de) writes: Am 24.07.2012 21:01, schrieb Justin Spahr-Summers: This occurs on Mac OS X 10.7.4, on git versions 1.7.10.2 (Apple Git-33) and 1.7.11.3. Steps: 1. Create or clone a repository to an absolute path that contains spaces. 2. Add a submodule to the repository, if it does not already have one. 3. Within that submodule, attempt to add another submodule. The result is an error fatal: Not a git repository, followed by the relative path to the submodule directory within .git/modules of the top-level repository. Similarly, using git submodule update --init --recursive in a freshly-cloned repository that matches the above configuration will fail with the same error. git clone --recursive does not seem to suffer from the same problem at clone time, but will still fail to add recursive submodules. Hmm, I don't understand how that is different from what t7407 does, it uses git submodule update --init --recursive in to populate recursive submodules in a freshly cloned repository whose path contains a space (in the trash directory name) in test number 8. I can see one codepath that would behave incorrectly, especially if the submodule path relative to the superproject has whitespaces in it. In module_clone(), you have: # We already are at the root of the work tree but cd_to_toplevel will # resolve any symlinks that might be present in $PWD a=$(cd_to_toplevel cd $gitdir pwd)/ b=$(cd_to_toplevel cd $sm_path pwd)/ ... # Turn each leading */ component into ../ rel=$(echo $b | sed -e 's|[^/][^/]*|..|g') echo gitdir: $rel/$a $sm_path/.git I _think_ $sm_path is computed correctly by the codeflow leading to this place, and $b is also computed correctly, but notice the lack of quoting around $b when you echo it? It will be split at $IFS, so if b='/Program Files/My Stupidity/', the sed script will see a single SP between My and Stupidity, which is different from what you wanted to feed, I presume. Having said that, I do not think git-submodule is prepared to take paths with path-unsafe characters in it, given that many part of it has loops like while read mode sha1 stage sm_path that reads from ls-files/ls-tree output without -z (which means it cannot handle pathnames with LF in them). My recommendation at this point (i.e. not a long term) for people with problems Justin saw is Don't do it then. I appreciate the debugging work. Unfortunately, none of the relative submodule paths have had whitespace in them, so I'm not sure that's the issue. Here's some real output, with a couple specific names removed, starting from the root of the top-level repository (where External/twui is a submodule): $ cd External/twui $ git submodule add git://github.com/petejkim/expecta.git TwUITests/expecta Cloning into 'TwUITests/expecta'... remote: Counting objects: 988, done. remote: Compressing objects: 100% (404/404), done. remote: Total 988 (delta 680), reused 842 (delta 535) Receiving objects: 100% (988/988), 156.30 KiB, done. Resolving deltas: 100% (680/680), done. fatal: Not a git repository: ../../../../../../../../Volumes/drive name with spaces/Users/justin/Documents/Programming/project name with spaces/.git/modules/External/twui/modules/TwUITests/expecta -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug: Recursive submodules fail when the repo path contains spaces
Justin Spahr-Summers justin.spahrsumm...@gmail.com writes: On Tuesday, 24. July 2012 at 13:26, Junio C Hamano wrote: ... I can see one codepath that would behave incorrectly,... ... My recommendation at this point (i.e. not a long term) for people with problems Justin saw is Don't do it then. I appreciate the debugging work. That was not a debug, and I didn't mean to say that was the only place that is problematic. In fact I think I said the script is not prepared to work with paths with paths-unsafe characters because it has many problematic constructs, and ended the message with Don't do it then. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 6/7] Remove unused and bad gettext block from git-am
Jonathan Nieder jrnie...@gmail.com writes: --d|--dotest) -die $(gettext -d option is no longer supported. Do not use.) -;; Luckily the support was removed 4 years ago and I don't think anyone is going to run into this, so a different justification could apply. Still I'd prefer a deprecation/removal not buried in an unrelated topic. Can we just leave it untranslated, and send a removal patch during pre 1.8.0 timeframe? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 5/7] i18n: am: mark more strings for translation
Jonathan Nieder jrnie...@gmail.com writes: -b|--binary) -echo 2 The $1 option has been a no-op for long time, and -echo 2 it will be removed. Please do not use it anymore. +echo 2 $(gettext The -b option has been a no-op for long time, and +it will be removed. Please do not use it anymore.) ... that this changes the message when the --binary option is passed. Before this patch, it says The --binary option has been a no-op for a long time, and ... After the patch, it says The -b option has been a no-op for a long time, and ... Intentional? That may be a good change or a bad one (I haven't thought clearly about it), but it seems at least worth mentioning. Cc-ing Thomas in case he has advice. If we really care we could printf $1, but I think we usually do The -b/--binary option has been... in a case like this, especially in codepaths that no longer has an easy access to $1 after parsing the command line but knows that either one of them is given from the parse result, and that would be an appropriate solution for this particular one as well. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 6/7] Remove unused and bad gettext block from git-am
On Tue, Jul 24, 2012 at 11:27 AM, Jonathan Nieder jrnie...@gmail.com wrote: Hi, Jiang Xin wrote: Gettext message should not start with '-' nor '--'. Since the '-d' and '--dotest' options do not exist in OPTIONS_SPEC variable, it's safe to remove the block. The above justification is not a sufficient reason to stop giving helpful advice when someone uses an option that was historically supported: I think Jiang is saying that git am --dotest=... already errors out because dotest is not in the OPTIONS_SPEC. See 98ef23b (git-am: minor cleanups, 2009-01-28). Or am I missing something? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 6/7] Remove unused and bad gettext block from git-am
Martin von Zweigbergk martin.von.zweigbe...@gmail.com writes: On Tue, Jul 24, 2012 at 11:27 AM, Jonathan Nieder jrnie...@gmail.com wrote: Hi, Jiang Xin wrote: Gettext message should not start with '-' nor '--'. Since the '-d' and '--dotest' options do not exist in OPTIONS_SPEC variable, it's safe to remove the block. The above justification is not a sufficient reason to stop giving helpful advice when someone uses an option that was historically supported: I think Jiang is saying that git am --dotest=... already errors out because dotest is not in the OPTIONS_SPEC. See 98ef23b (git-am: minor cleanups, 2009-01-28). Or am I missing something? No you are not missing anything, I would think. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
git-svn SVN 1.7 fix, take 2
It's post OSCON so I can take another crack at this again. I'm struggling with how best to present all this to you folks. There's etiquette for how one presents a git pull request... but there's conflicting etiquette about how one presents patches to a mailing list. I'm not sure which bit of which applies when here. Documentation/SubmittingPatches focuses on single patches and basic commit etiquette. A big one is do not blast 10 emails to a mailing list but I gather that's ok here if a submission needs 10 commits to be well expressed and its done via git-send-email? And then if patch #3 needs revision I'm to do it in a rebase and resend the whole 10 commits? Am I to think of git-send-email less as a means of sending patches to a mailing list and more as a git transport mechanism? I'm trying to bust it up into easier to digest pieces. I came into this cold without much knowledge of the problem (something to do with canonicalization) and no knowledge of the code. While each commit is sharp, the work as a whole is mixed up. Here's the first pieces, as I see them, along with their branches. The whole work is in https://github.com/schwern/git/tree/git-svn/fix-canonical * Change the Makefile.PL so it automatically finds the .pm files. https://github.com/schwern/git/tree/git-svn/easier_modules (Going to remove the Error.pm movement as off-topic) * Extract each of the internal Git::* packages from inside git-svn. https://github.com/schwern/git/tree/git-svn/extract_classes There's five classes, and I did each in at least two commits. First is a straight cut paste with no further changes. Second (or more) fixes it so things work again. This is better for review (if it were done in a single commit the real change would be lost in the cut paste), but it means you have a commit that breaks thing which will be a problem for bisecting. I'm inclined to stick with two commits and you folks can squash them if you decide bisecting is more important. The Git::SVN extraction is more complicated than the rest, so I'll probably do that separately and bust it up into a few commits. Next I'm going to... 1) Submit easier_modules. 2) Break up the Git::SVN fix into more commits. 3) Submit the Git::SVN extraction. -- Reality is that which, when you stop believing in it, doesn't go away. -- Phillip K. Dick -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git-svn SVN 1.7 fix, take 2
Michael G Schwern schw...@pobox.com writes: A big one is do not blast 10 emails to a mailing list but I gather that's ok here if a submission needs 10 commits to be well expressed and its done via git-send-email? And then if patch #3 needs revision I'm to do it in a rebase and resend the whole 10 commits? Am I to think of git-send-email less as a means of sending patches to a mailing list and more as a git transport mechanism? Yes, yes and whatever (even though I think send-email is just a better MUA/MSA when you want to send patches and isn't restricted for a _git_ transport, I do not think it matters how you look at it). I'm trying to bust it up into easier to digest pieces. I came into this cold without much knowledge of the problem (something to do with canonicalization) and no knowledge of the code. Perhaps it is a good idea to lurk and see how others submit their topics first? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git-svn SVN 1.7 fix, take 2
Hi, Michael G Schwern wrote: I'm trying to bust it up into easier to digest pieces. I have a crazy idea. You might not like it, but maybe it's worth giving it a try. [...] The Git::SVN extraction is more complicated than the rest, so I'll probably do that separately and bust it up into a few commits. All of these changes are supposed to have zero functional effect, right? Could you send the first five patches that *are* supposed to have a functional effect? I know that they will not apply cleanly to git-svn from git master or on top of each other; that's fine with me. If the approach looks right, interested people (read: probably Ben or I :)) can make the corresponding change in the code layout from master. Afterwards, we can look into all that refactoring to make later changes easier. What do you think? Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] test: some testcases failed if cwd is on a symlink
2012/7/24 Junio C Hamano gits...@pobox.com: I wonder if running test in a real directory (in other words, fix your cwd) may be a simpler, more robust and generally a better solution, e.g. something silly like... diff --git a/t/test-lib.sh b/t/test-lib.sh index acda33d..7f6fb0a 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -15,6 +15,8 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see http://www.gnu.org/licenses/ . +cd $(pwd -P) + Yes, it's much simpler. -- Jiang Xin -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 6/7] Remove unused and bad gettext block from git-am
Jonathan Nieder jrnie...@gmail.com writes: I guess that means the intended justification is the following? The git am -d/--dotest option has errored out with a message since e72c7406 (am: remove support for -d .dotest, 2008-03-04). The error message about lack of support was eliminated along with other cleanups (probably by mistake) a year later by removing the option from the option table in 98ef23b3 (git-am: minor cleanups, 2009-01-28). But the code to handle -d and --dotest stayed around even though ever since then it could not be tripped. Remove this dead code. Noticed because the error message starts with a '-' and xgettext does not cope well with that. Looks good. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Extract Git classes from git-svn (1/10)
On 2012.7.17 10:49 PM, Junio C Hamano wrote: By allowing people to easily publish a completed work, and making it easier for them to let others peek at their work, Git hosting services like GitHub are wonderful. But I am not conviced that quality code reviews like we do on the mailing list can be done with existing Web based interface to a satisfactory degree. In this instance, I was just using Github for repository storage. I was hoping I could just submit a remote git repository and people would look at it from there. No Github required. I understand this makes things very convenient for you to review patches, let me convey my POV... After I'm exhausted from volunteering all the coding work, rather than submitting a URL to a remote repository I find I have to learn new specialized tools. It's extra learning and work, an extra step to screw up, and foreign to me (even as a experienced git user). It is of little benefit to me as a casual volunteer submitter. I can see if you've been on the git mailing list for a while and have git-am and all that set up, this system is great. But it comes at a cost which is offloaded onto new and casual contributors. Patches with proposed commit log messages are sent via e-mail, people can review them and comment on them with quotes from the relevant part of the patch. The review can even be made offline, yet at the end, the list archive is an easy one-stop location you need to go to see how the changes progressed, what the background thinking was, etc. for all the changes that matter. Look at recent ones (randomly, $gmane/199492, $gmane/199497, $gmane/200750, $gmane/201477, $gmane/201434), and their re-rolls, and admire how well the process works. I've played with GitHub's in-line code comment interface, but honestly, it is cumbersome to use, for one thing, but more importantly, you have to click around various repositories of pull requestors, dig around to see in-line comments, and I do not see how we can keep a coherent discussion like we do on the mailing list. There may be a hosting site with better code review features, but all the code review of Git happens on this mailing list, and that is not likely to change in the near future. Everything you've said above is correct... but it creates a procedure optimized for the convenience of the long time reviewers at the expense of new and casual submissions. I'm going to guess you live in email and have your client setup to do fancy things to extract patches from your mailbox and the like. This sort of specialized setup makes people bounce right off the submission process. At OSCON I was asking around for help getting things setup so I could submit patches here properly. As soon as they said which mail daemon are you running?, I said stop! I don't want to know any more. I have too many things to do to be fiddling with my mailer configuration just to submit volunteer work in the right form (that said, I'm pleased as punch that git-send-email now has instructions for sending via GMail). You're volunteers, too. We're all volunteers, so a more balanced submission process would be nice. But since you brought Github up... (I get the impression its kind of a dirty word around here) As somebody who doesn't live in email anymore (once upon a time I did), I find the Github Pull Request model to be an excellent... I'm not even going to use the word compromise because I don't feel like either side has been compromised... it's an excellent enhancement. The commits and conversations about the commits are all there on one page. Looking at a commit is a click away (I usually open them all in tabs at once, much faster). You can comment on them as a whole or inline. Those comments appear BOTH in the commit AND in the larger conversation on the pull request keeping it coherent, no clicking around. And it has email mirroring. All that and its tracked and organized in an issue tracker. Here's an example that includes commits, discussion about the larger issue, comments on commits, more commits based on those comments, and so on. As you can see, the conversation is complete and coherent. It wasn't always this way, but they're constantly improving. https://github.com/schwern/test-more/pull/319 A concrete downside it is that it does not work offline. I agree that's a problem. I don't think it's a veto. There are various work arounds which are less complicated than your typical git-to-email-to-git setup. We can talk about that you're interested. I've gone all in on Github Pull Requests such that most of my projects don't even have mailing lists (issues are used for discussion). This avoids a split community between Github and the mailing list. And they have email mirroring, so issue discussion can be done all in email (I prefer email for things that involve push notification and replies). Github has a nice API and it would be possible to create a Github Pull Request - Mailing
Re: [PATCH v3 2/7] i18n: rebase: mark strings for translation
2012/7/25 Jonathan Nieder jrnie...@gmail.com: @@ -64,7 +64,7 @@ test_expect_success 'rebase -n overrides config rebase.stat config' ' test_expect_success 'rebase --onto outputs the invalid ref' ' test_must_fail git rebase --onto invalid-ref HEAD HEAD 2err - grep invalid-ref err + test_i18ngrep invalid-ref err ' This is probably part of a message intended for humans, but the test does not say. What is the full message being checked? The error messages are: fatal: Needed a single revision Does not point to a valid commit: invalid-ref The first line has not been marked for translation (comes from builtin/rev-parse.c), but the second line is marked for translation in this patch. The output in my locale would be: fatal: Needed a single revision 没有指向一个有效的提交：invalid-ref -- Jiang Xin -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 5/7] i18n: am: mark more strings for translation
2012/7/25 Jonathan Nieder jrnie...@gmail.com: -b|--binary) - echo 2 The $1 option has been a no-op for long time, and - echo 2 it will be removed. Please do not use it anymore. + echo 2 $(gettext The -b option has been a no-op for long time, and +it will be removed. Please do not use it anymore.) ... that this changes the message when the --binary option is passed. Before this patch, it says The --binary option has been a no-op for a long time, and ... After the patch, it says The -b option has been a no-op for a long time, and ... Intentional? That may be a good change or a bad one (I haven't thought clearly about it), but it seems at least worth mentioning. Cc-ing Thomas in case he has advice. It's intentional. * First, if a variable in the message, we could not use gettext, for the variable will be expanded (evaluated) and never match the entry in po file. * Second, if there is a positional parameter ($1, $2,...) in the message, we could not use eval_gettext either. Because eval_gettext may be a wapper for gettext, and the positional parameter would loose it's original context. -- Jiang Xin -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git-svn SVN 1.7 fix, take 2
Hi again, Michael G Schwern wrote: On 2012.7.24 3:02 PM, Jonathan Nieder wrote: Could you send the first five patches that *are* supposed to have a functional effect? I know that they will not apply cleanly to git-svn from git master or on top of each other; that's fine with me. If the approach looks right, interested people (read: probably Ben or I :)) can make the corresponding change in the code layout from master. [...] I think that would be a lot of extra work for me, create a big mess and be harder to understand. :-/ Since I'm creating new files to store the classes, the functional changes cannot be applied without the class extractions, so I'd have to rewrite them. I don't understand. Didn't I ask to send the changes as-is and say that *I or someone else interested* would do the work to get them to apply? [...] That should give you the information you need... if I understand what you need. I feel like I don't and we're talking past each other. Basically, you are offering a code fix that's at least worth $500. Lots of people have wanted the bug fixed for a long time. As long as it does not involve sacrificing maintainability, we should apply the fix as soon as possible! It's great that you've done this work. Meanwhile that change is being held hostage by lots of cleanups that are unquestionably going in the right direction but are going to be a pain in the neck to safely apply. And no one has reviewed your fix that comes _after_ the cleanups. Maybe the fix goes in a wrong direction --- we don't know yet. Maybe once we understand the fix we'll have ideas that obsolete the previous cleanups and can more simply accomplish the same thing by organizing the code a different way. You are saying that, to make your life easier, we should take your cleanups and fixes as-is, all in one big pull. Maybe you're right! But it will take a lot longer this way than applying a smaller set of changes that just fixes the bug. I am saying that that, before anything else, it would be helpful for us to *see* the relevant patches and understand your fix. You are more knowledgeable than anyone else about your code, so presumably it should be straightforward to pick out which patches are the important ones. Using git format-patch -1 commit you can get a patch for each. Then you can use your favorite text editor to edit their subject lines and change descriptions to describe what they do and where they fall in the series of patches you are sending. Finally you can use your favorite mail user agent (e.g., git send-email) to send out the patches. Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git-svn SVN 1.7 fix, take 2
On 2012.7.24 2:51 PM, Junio C Hamano wrote: Michael G Schwern schw...@pobox.com writes: A big one is do not blast 10 emails to a mailing list but I gather that's ok here if a submission needs 10 commits to be well expressed and its done via git-send-email? And then if patch #3 needs revision I'm to do it in a rebase and resend the whole 10 commits? Am I to think of git-send-email less as a means of sending patches to a mailing list and more as a git transport mechanism? Yes, yes and whatever (even though I think send-email is just a better MUA/MSA when you want to send patches and isn't restricted for a _git_ transport, I do not think it matters how you look at it). #3 was not intended as a dig. If I can think about git-send-email like a funny way to do a git-push then that fits better in my head. I worry about sending too many emails to a list at once. I don't worry about sending too many commits in one push. I'm trying to bust it up into easier to digest pieces. I came into this cold without much knowledge of the problem (something to do with canonicalization) and no knowledge of the code. Perhaps it is a good idea to lurk and see how others submit their topics first? While I use git heavily I'm not invested in working on it. I work on a lot of projects. I'd like to be able to do the work, submit it, work through review, and get out without joining another mailing list and studying their culture. Is there a document I could look at for submitting a large body of work, or could I help improve SubmittingPatches to document the process better? -- I do have a cause though. It's obscenity. I'm for it. - Tom Lehrer -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git-svn SVN 1.7 fix, take 2
Michael G Schwern wrote: While I use git heavily I'm not invested in working on it. I work on a lot of projects. I'd like to be able to do the work, submit it, work through review, and get out without joining another mailing list and studying their culture. An alternative approach would be to _just_ explain how your patch series works, and then say Ok, here is the git branch. Have fun with it. If someone wants to incorporate it, that's great. Let me know if you have any questions. That's totally fine. Then someone else can submit the patches and help to massage the patches into a form that is ready for application on your behalf. Is there a document I could look at for submitting a large body of work, or could I help improve SubmittingPatches to document the process better? I thought SubmittingPatches did describe how to send patch series. Could you point to the section that was lacking? Or are you just still in disbelief that people review each patch in a thread, one by one, sending emails to each other? Grateful but impatient, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Extract Git classes from git-svn (1/10)
Hi, Michael G Schwern wrote: But since you brought Github up... (I get the impression its kind of a dirty word around here) On the contrary, one of the main contributors is employed by github, github hosts the git website, and all in all, github has done great work. Many people on the git list have also interacted with projects that went all-in on github and other online code review tools. I imagine experiences varied from person to person. When I said that if you don't like emails I'd be open to other ideas for how to review your code, I didn't mean Github's web interface. :) It doesn't work for me. Maybe it works better for other people. For reference, Github pull requests wouldn't be the right apples-to-apples comparison to make, anyway. There are also pull requests from time to time on this list --- see for example http://thread.gmane.org/gmane.comp.version-control.git/201728 http://thread.gmane.org/gmane.comp.version-control.git/201186 http://thread.gmane.org/gmane.comp.version-control.git/200844 http://thread.gmane.org/gmane.comp.version-control.git/199577 http://thread.gmane.org/gmane.comp.version-control.git/193817 http://thread.gmane.org/gmane.comp.version-control.git/189178 http://thread.gmane.org/gmane.comp.version-control.git/187082 Those are for code that has already been reviewed. Hoping that clarifies, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git-svn SVN 1.7 fix, take 2
Michael G Schwern wrote:
git log -p schwern/git-svn/extract-classes..schwern/git-svn/fix-canonical
That should give you the information you need...
I guess so. May we have your sign-off on these changes? (A simple
reply of yes is enough, no need to resend patches to do this.)
Here it is in patch form for reviewers. If I understand correctly,
the idea is to replace accesses to $gs-{path} with calls to a
$gs-path function that canonicalizes (and likewise for s/path/url/).
There are probably other subtleties, but that seems to be the gist.
---
git-svn.perl | 96 +++--
perl/Git/SVN.pm | 148 +
perl/Git/SVN/Fetcher.pm |2 +-
perl/Git/SVN/Migration.pm |6 +-
perl/Git/SVN/Ra.pm| 96 -
perl/Git/SVN/Utils.pm | 123 ++-
t/Git-SVN/Ra/fix_dir.t| 24 ++
t/lib-git-svn.sh |2 +
t/t9118-git-svn-funky-branch-names.sh |6 +-
t/t9119-git-svn-info.sh |7 +-
10 files changed, 341 insertions(+), 169 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index 7b54f43f..74aeb4df 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -12,7 +12,12 @@ $VERSION = '@@GIT_VERSION@@';
use Git::SVN;
-use Git::SVN::Utils qw(fatal can_compress);
+use Git::SVN::Utils qw(
+fatal
+can_compress
+canonicalize_path
+canonicalize_url
+);
# From which subdir have we been invoked?
my $cmd_dir_prefix = eval {
@@ -100,8 +105,6 @@ BEGIN {
Memoize::memoize 'Git::config_bool';
}
-my ($SVN);
-
$sha1 = qr/[a-f\d]{40}/;
$sha1_short = qr/[a-f\d]{4,40}/;
my ($_stdin, $_help, $_edit,
@@ -518,7 +521,6 @@ sub cmd_init {
}
my $url = shift or die SVN repository location required ,
as a command-line argument\n;
- $url = canonicalize_url($url);
init_subdir(@_);
do_git_init_db();
@@ -1199,7 +1201,7 @@ sub cmd_show_ignore {
my ($url, $rev, $uuid, $gs) = working_head_info('HEAD');
$gs ||= Git::SVN-new;
my $r = (defined $_revision ? $_revision : $gs-ra-get_latest_revnum);
- $gs-prop_walk($gs-{path}, $r, sub {
+ $gs-prop_walk($gs-path, $r, sub {
my ($gs, $path, $props) = @_;
print STDOUT \n# $path\n;
my $s = $props-{'svn:ignore'} or return;
@@ -1215,7 +1217,7 @@ sub cmd_show_externals {
my ($url, $rev, $uuid, $gs) = working_head_info('HEAD');
$gs ||= Git::SVN-new;
my $r = (defined $_revision ? $_revision : $gs-ra-get_latest_revnum);
- $gs-prop_walk($gs-{path}, $r, sub {
+ $gs-prop_walk($gs-path, $r, sub {
my ($gs, $path, $props) = @_;
print STDOUT \n# $path\n;
my $s = $props-{'svn:externals'} or return;
@@ -1230,7 +1232,7 @@ sub cmd_create_ignore {
my ($url, $rev, $uuid, $gs) = working_head_info('HEAD');
$gs ||= Git::SVN-new;
my $r = (defined $_revision ? $_revision : $gs-ra-get_latest_revnum);
- $gs-prop_walk($gs-{path}, $r, sub {
+ $gs-prop_walk($gs-path, $r, sub {
my ($gs, $path, $props) = @_;
# $path is of the form /path/to/dir/
$path = '.' . $path;
@@ -1260,36 +1262,12 @@ sub cmd_mkdirs {
$gs-mkemptydirs($_revision);
}
-sub canonicalize_path {
- my ($path) = @_;
- my $dot_slash_added = 0;
- if (substr($path, 0, 1) ne /) {
- $path = ./ . $path;
- $dot_slash_added = 1;
- }
- # File::Spec-canonpath doesn't collapse x/../y into y (for a
- # good reason), so let's do this manually.
- $path =~ s#/+#/#g;
- $path =~ s#/\.(?:/|$)#/#g;
- $path =~ s#/[^/]+/\.\.##g;
- $path =~ s#/$##g;
- $path =~ s#^\./## if $dot_slash_added;
- $path =~ s#^/##;
- $path =~ s#^\.$##;
- return $path;
-}
-
-sub canonicalize_url {
- my ($url) = @_;
- $url =~ s#^([^:]+://[^/]*/)(.*)$#$1 . canonicalize_path($2)#e;
- return $url;
-}
-
# get_svnprops(PATH)
# --
# Helper for cmd_propget and cmd_proplist below.
sub get_svnprops {
my $path = shift;
+
my ($url, $rev, $uuid, $gs) = working_head_info('HEAD');
$gs ||= Git::SVN-new;
@@ -1298,7 +1276,8 @@ sub get_svnprops {
$path = $cmd_dir_prefix . $path;
fatal(No such file or directory: $path) unless -e $path;
my $is_dir = -d $path ? 1 : 0;
- $path = $gs-{path} . '/' . $path;
+
+ $path = $gs-path . '/' . $path if $gs-path;
# canonicalize the path (otherwise libsvn will abort or fail to
# find the file)
@@ -1399,8 +1378,8 @@ sub cmd_commit_diff {
fatal(Needed URL or usable git-svn --id in ,
the command-line\n, $usage);
Re: git-svn SVN 1.7 fix, take 2
Jonathan Nieder jrnie...@gmail.com writes:
Michael G Schwern wrote:
git log -p schwern/git-svn/extract-classes..schwern/git-svn/fix-canonical
That should give you the information you need...
I guess so. May we have your sign-off on these changes? (A simple
reply of yes is enough, no need to resend patches to do this.)
Here it is in patch form for reviewers. If I understand correctly,
the idea is to replace accesses to $gs-{path} with calls to a
$gs-path function that canonicalizes (and likewise for s/path/url/).
There are probably other subtleties, but that seems to be the gist.
The impression I am getting is that the updated code wants to handle
URL and paths without any funny encoding, but it is unclear from my
cursory read (e.g. what goes on with escape_url?).
if ($old_url =~ m#^svn(\+ssh)?://# ||
($full_url =~ m#^https?://#
- escape_url($full_url) ne $full_url)) {
+ $full_url ne $full_url)) {
How can the latter part of this conditional be true?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git-svn SVN 1.7 fix, take 2
On 2012.7.24 4:45 PM, Junio C Hamano wrote:
git log -p schwern/git-svn/extract-classes..schwern/git-svn/fix-canonical
That should give you the information you need...
I guess so. May we have your sign-off on these changes? (A simple
reply of yes is enough, no need to resend patches to do this.)
Here it is in patch form for reviewers. If I understand correctly,
the idea is to replace accesses to $gs-{path} with calls to a
$gs-path function that canonicalizes (and likewise for s/path/url/).
There are probably other subtleties, but that seems to be the gist.
The impression I am getting is that the updated code wants to handle
URL and paths without any funny encoding, but it is unclear from my
cursory read (e.g. what goes on with escape_url?).
No, now it's just canonicalizing as early as possible. Preferably within the
object accessor rather than at the point of use. So in the code below,
$full_url is already escaped/canonicalized.
In general this blob patch isn't going to make a lot of overall sense. I'm
working with Jonathan to get it submitted in manageable pieces.
if ($old_url =~ m#^svn(\+ssh)?://# ||
($full_url =~ m#^https?://#
- escape_url($full_url) ne $full_url)) {
+ $full_url ne $full_url)) {
How can the latter part of this conditional be true?
Good point. More importantly, what was it trying to accomplish before and
does it need to be preserved?
If the URL is svn OR its http and needs to be escaped... do something
special. I don't really understand what the special stuff in the following
block is. Anything that undef's the invocant (ie. $self) is probably broken.
a51cdb0c0420ee3bef26bbd1a9aa75e1d464e5b7 and
2a679c7a3148978a3f58f1c12100383638e744c5 shed some light. 2a679 looks like it
specifically holds off on escaping $full_url. It would be very nice if that
was not necessary. It would be helpful if the bug mentioned in 2a679 could be
reproduced to see if it still applies or can be dealt with in another way.
--
185. My name is not a killing word.
-- The 213 Things Skippy Is No Longer Allowed To Do In The U.S. Army
http://skippyslist.com/list/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] difftool: Simplify print_tool_help()
On Tue, Jul 24, 2012 at 5:43 AM, Tim Henigan tim.heni...@gmail.com wrote: On Sun, Jul 22, 2012 at 11:42 PM, David Aguilar dav...@gmail.com wrote: Eliminate a global variable and File::Find usage by building upon basename() and glob() instead. glob was used in an early revision of the patch that led to bf73fc2 (difftool: print list of valid tools with '--tool-help') as well [1]. However, if the path to git or the path under 'mergetools' includes spaces, glob fails. To work around the problem, File::Find was used instead [2]. Does this implementation handle that case? I'm sorry, but I haven't had time to apply and test myself. [1]: http://thread.gmane.org/gmane.comp.version-control.git/193233/focus=193925 [2]: http://thread.gmane.org/gmane.comp.version-control.git/194158 Good catch. Eliminating the globals should be the goal, then. I'll have to re-roll. -- David -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] difftool: Simplify print_tool_help()
On Tue, Jul 24, 2012 at 6:52 PM, David Aguilar dav...@gmail.com wrote: On Tue, Jul 24, 2012 at 5:43 AM, Tim Henigan tim.heni...@gmail.com wrote: On Sun, Jul 22, 2012 at 11:42 PM, David Aguilar dav...@gmail.com wrote: Eliminate a global variable and File::Find usage by building upon basename() and glob() instead. glob was used in an early revision of the patch that led to bf73fc2 (difftool: print list of valid tools with '--tool-help') as well [1]. However, if the path to git or the path under 'mergetools' includes spaces, glob fails. To work around the problem, File::Find was used instead [2]. Does this implementation handle that case? I'm sorry, but I haven't had time to apply and test myself. [1]: http://thread.gmane.org/gmane.comp.version-control.git/193233/focus=193925 [2]: http://thread.gmane.org/gmane.comp.version-control.git/194158 Good catch. Eliminating the globals should be the goal, then. I'll have to re-roll. These landed in next. Junio, would you prefer follow-up patches or a rebased series? -- David -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] difftool: Simplify print_tool_help()
David Aguilar dav...@gmail.com writes: Does this implementation handle that case? I'm sorry, but I haven't had time to apply and test myself. [1]: http://thread.gmane.org/gmane.comp.version-control.git/193233/focus=193925 [2]: http://thread.gmane.org/gmane.comp.version-control.git/194158 Good catch. Eliminating the globals should be the goal, then. I'll have to re-roll. Junio, would you prefer follow-up patches or a rebased series? Incremental patches, please. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Extract Git classes from git-svn (1/10)
Michael G Schwern schw...@pobox.com wrote: On 2012.7.17 10:49 PM, Junio C Hamano wrote: By allowing people to easily publish a completed work, and making it easier for them to let others peek at their work, Git hosting services like GitHub are wonderful. But I am not conviced that quality code reviews like we do on the mailing list can be done with existing Web based interface to a satisfactory degree. In this instance, I was just using Github for repository storage. I was hoping I could just submit a remote git repository and people would look at it from there. No Github required. I understand this makes things very convenient for you to review patches, let me convey my POV... After I'm exhausted from volunteering all the coding work, rather than submitting a URL to a remote repository I find I have to learn new specialized tools. It's extra learning and work, an extra step to screw up, and foreign to me (even as a experienced git user). It is of little benefit to me as a casual volunteer submitter. Except git is also a new specialized tool. Your examples are exactly why I'm saddened many projects only adopted git, but not the workflow which _built_ git (and Linux). I can see if you've been on the git mailing list for a while and have git-am and all that set up, this system is great. But it comes at a cost which is offloaded onto new and casual contributors. Email is integral to Free/Open Source development and remains one of the few things on the Internet not (yet) controlled by any central entity. Once setup, the same email setup can work across all projects which use email. These projects need not be hosted on the same websites/servers at all. This sort of specialized setup makes people bounce right off the submission process. At OSCON I was asking around for help getting things setup so I could submit patches here properly. As soon as they said which mail daemon are you running?, I said stop! I don't want to know any more. I have too many things to do to be fiddling with my mailer configuration just to submit volunteer work in the right form (that said, I'm pleased as punch that git-send-email now has instructions for sending via GMail). You're volunteers, too. We're all volunteers, so a more balanced submission process would be nice. How about we educate users about a proper email setup instead? If they're capable of learning git, they're surely capable of setting up an email client properly, and perhaps more projects can adopt an email-centric workflow. But since you brought Github up... (I get the impression its kind of a dirty word around here) (Not speaking for the git project) I'm entirely against the way GitHub (or Ohloh or similar services) gamifies software development and tries to tie a person to all their other projects. Much of my code is public, but I am a private person. I want code to be judged solely on its own merits of that code; not from what the author's achieved or how popular the person might be in the development world. Unfortunately, GitHub (and other social networks) is structured to encourage that sort of thing (which I know is appealing to many). For me, the whole social network followers/timeline thing also has a _huge_ creepiness factor to it. How one prioritizes and spends time between different different (especially unrelated) projects should be nobody else's business. I don't make it /easy/ for someone (e.g. Junio) to know I'm slacking off on my git work to hack on ProjectNoOneUses :) One could try using a different account for every project, but that's also violating the terms of service. If all the clicking and opening tabs in a browser feels uncomfortable to you... its something you learn like anything else. Less and less people are comfortable in mail clients. Who is the system optimized for? It doesn't have to be a zero sum game. (Still not speaking for others) I believe GUIs are (mostly) harmful. Graphical browsers don't interact well with command-line tools. Browsers have no concept of a working directory; I can't fire up a browser tab/window for each project I work on and edit/apply patches directly from the browser like I can with an MUA. We need to figure out how to teach folks to use email (properly) instead. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 0/5] difftool: Use symlinks in dir-diff mode
Teach difftool to use symlinks when diffing against the worktree. David Aguilar (5): difftool: print_tool_help() globals difftool: Eliminate global variables difftool: Move option values into a hash difftool: Call the temp directory git-difftool difftool: Use symlinks when diffing against the worktree Documentation/git-difftool.txt | 8 ++ git-difftool.perl | 180 ++--- 2 files changed, 124 insertions(+), 64 deletions(-) -- 1.7.12.rc0.15.g8157c39 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 1/5] difftool: print_tool_help() globals
Replace a global variable with a closure.
Signed-off-by: David Aguilar dav...@gmail.com
---
Differences from last time:
This keeps the original File::Find implementation and wraps the
global variable in a closure as the first step in the
globals-elimination cleanup.
git-difftool.perl | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/git-difftool.perl b/git-difftool.perl
index c079854..d4737e1 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -23,7 +23,6 @@ use File::Temp qw(tempdir);
use Getopt::Long qw(:config pass_through);
use Git;
-my @tools;
my @working_tree;
my $rc;
my $repo = Git-repository();
@@ -67,6 +66,7 @@ my $workdir = find_worktree();
sub filter_tool_scripts
{
+ my ($tools) = @_;
if (-d $_) {
if ($_ ne .) {
# Ignore files in subdirectories
@@ -74,17 +74,17 @@ sub filter_tool_scripts
}
} else {
if ((-f $_) ($_ ne defaults)) {
- push(@tools, $_);
+ push(@$tools, $_);
}
}
}
sub print_tool_help
{
- my ($cmd, @found, @notfound);
+ my ($cmd, @found, @notfound, @tools);
my $gitpath = Git::exec_path();
- find(\filter_tool_scripts, $gitpath/mergetools);
+ find(sub { filter_tool_scripts(\@tools) }, $gitpath/mergetools);
foreach my $tool (@tools) {
$cmd = TOOL_MODE=diff;
--
1.7.12.rc0.15.g8157c39
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 2/5] difftool: Eliminate global variables
Organize the script so that it has a single main() function which
calls out to dir_diff() and file_diff() functions. This eliminates
dir-diff-specific variables that do not need to be calculated when
performing a regular file-diff.
Signed-off-by: David Aguilar dav...@gmail.com
---
Same as last time, but rebased against the new 1/5
git-difftool.perl | 128 --
1 file changed, 75 insertions(+), 53 deletions(-)
diff --git a/git-difftool.perl b/git-difftool.perl
index d4737e1..fa787d6 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -23,11 +23,6 @@ use File::Temp qw(tempdir);
use Getopt::Long qw(:config pass_through);
use Git;
-my @working_tree;
-my $rc;
-my $repo = Git-repository();
-my $repo_path = $repo-repo_path();
-
sub usage
{
my $exitcode = shift;
@@ -44,6 +39,8 @@ USAGE
sub find_worktree
{
+ my ($repo) = @_;
+
# Git-repository-wc_path() does not honor changes to the working
# tree location made by $ENV{GIT_WORK_TREE} or the 'core.worktree'
# config variable.
@@ -62,8 +59,6 @@ sub find_worktree
return $worktree;
}
-my $workdir = find_worktree();
-
sub filter_tool_scripts
{
my ($tools) = @_;
@@ -112,10 +107,13 @@ sub print_tool_help
sub setup_dir_diff
{
+ my ($repo, $workdir) = @_;
+
# Run the diff; exit immediately if no diff found
# 'Repository' and 'WorkingCopy' must be explicitly set to insure that
# if $GIT_DIR and $GIT_WORK_TREE are set in ENV, they are actually used
# by Git-repository-command*.
+ my $repo_path = $repo-repo_path();
my $diffrepo = Git-repository(Repository = $repo_path, WorkingCopy =
$workdir);
my $diffrtn = $diffrepo-command_oneline('diff', '--raw',
'--no-abbrev', '-z', @ARGV);
exit(0) if (length($diffrtn) == 0);
@@ -136,6 +134,7 @@ sub setup_dir_diff
my $rindex = '';
my %submodule;
my %symlink;
+ my @working_tree = ();
my @rawdiff = split('\0', $diffrtn);
my $i = 0;
@@ -203,7 +202,7 @@ sub setup_dir_diff
($inpipe, $ctx) = $repo-command_input_pipe(qw/update-index -z
--index-info/);
print($inpipe $lindex);
$repo-command_close_pipe($inpipe, $ctx);
- $rc = system('git', 'checkout-index', '--all', --prefix=$ldir/);
+ my $rc = system('git', 'checkout-index', '--all', --prefix=$ldir/);
exit($rc | ($rc 8)) if ($rc != 0);
$ENV{GIT_INDEX_FILE} = $tmpdir/rindex;
@@ -253,7 +252,7 @@ sub setup_dir_diff
}
}
- return ($ldir, $rdir);
+ return ($ldir, $rdir, @working_tree);
}
sub write_to_file
@@ -276,54 +275,70 @@ sub write_to_file
close($fh);
}
-# parse command-line options. all unrecognized options and arguments
-# are passed through to the 'git diff' command.
-my ($difftool_cmd, $dirdiff, $extcmd, $gui, $help, $prompt, $tool_help);
-GetOptions('g|gui!' = \$gui,
- 'd|dir-diff' = \$dirdiff,
- 'h' = \$help,
- 'prompt!' = \$prompt,
- 'y' = sub { $prompt = 0; },
- 't|tool:s' = \$difftool_cmd,
- 'tool-help' = \$tool_help,
- 'x|extcmd:s' = \$extcmd);
-
-if (defined($help)) {
- usage(0);
-}
-if (defined($tool_help)) {
- print_tool_help();
-}
-if (defined($difftool_cmd)) {
- if (length($difftool_cmd) 0) {
- $ENV{GIT_DIFF_TOOL} = $difftool_cmd;
- } else {
- print No tool given for --tool=tool\n;
- usage(1);
+sub main
+{
+ # parse command-line options. all unrecognized options and arguments
+ # are passed through to the 'git diff' command.
+ my ($difftool_cmd, $dirdiff, $extcmd, $gui, $help, $prompt, $tool_help);
+ GetOptions('g|gui!' = \$gui,
+ 'd|dir-diff' = \$dirdiff,
+ 'h' = \$help,
+ 'prompt!' = \$prompt,
+ 'y' = sub { $prompt = 0; },
+ 't|tool:s' = \$difftool_cmd,
+ 'tool-help' = \$tool_help,
+ 'x|extcmd:s' = \$extcmd);
+
+ if (defined($help)) {
+ usage(0);
}
-}
-if (defined($extcmd)) {
- if (length($extcmd) 0) {
- $ENV{GIT_DIFFTOOL_EXTCMD} = $extcmd;
- } else {
- print No cmd given for --extcmd=cmd\n;
- usage(1);
+ if (defined($tool_help)) {
+ print_tool_help();
}
-}
-if ($gui) {
- my $guitool = '';
- $guitool = Git::config('diff.guitool');
- if (length($guitool) 0) {
- $ENV{GIT_DIFF_TOOL} = $guitool;
+ if (defined($difftool_cmd)) {
+ if (length($difftool_cmd) 0) {
+ $ENV{GIT_DIFF_TOOL} = $difftool_cmd;
+ } else {
+ print No tool given for --tool=tool\n;
+ usage(1);
+ }
+ }
+ if (defined($extcmd)) {
+ if (length($extcmd) 0) {
+
[PATCH v4 3/5] difftool: Move option values into a hash
Shorten the my declaration for all of the option-specific variables
by wrapping all of them in a hash. This also gives us a place to
specify default values, should we need them.
Signed-off-by: David Aguilar dav...@gmail.com
---
Same as last time, rebased
git-difftool.perl | 55 +++
1 file changed, 31 insertions(+), 24 deletions(-)
diff --git a/git-difftool.perl b/git-difftool.perl
index fa787d6..685887d 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -279,41 +279,48 @@ sub main
{
# parse command-line options. all unrecognized options and arguments
# are passed through to the 'git diff' command.
- my ($difftool_cmd, $dirdiff, $extcmd, $gui, $help, $prompt, $tool_help);
- GetOptions('g|gui!' = \$gui,
- 'd|dir-diff' = \$dirdiff,
- 'h' = \$help,
- 'prompt!' = \$prompt,
- 'y' = sub { $prompt = 0; },
- 't|tool:s' = \$difftool_cmd,
- 'tool-help' = \$tool_help,
- 'x|extcmd:s' = \$extcmd);
-
- if (defined($help)) {
+ my %opts = (
+ difftool_cmd = undef,
+ dirdiff = undef,
+ extcmd = undef,
+ gui = undef,
+ help = undef,
+ prompt = undef,
+ tool_help = undef,
+ );
+ GetOptions('g|gui!' = \$opts{gui},
+ 'd|dir-diff' = \$opts{dirdiff},
+ 'h' = \$opts{help},
+ 'prompt!' = \$opts{prompt},
+ 'y' = sub { $opts{prompt} = 0; },
+ 't|tool:s' = \$opts{difftool_cmd},
+ 'tool-help' = \$opts{tool_help},
+ 'x|extcmd:s' = \$opts{extcmd});
+
+ if (defined($opts{help})) {
usage(0);
}
- if (defined($tool_help)) {
+ if (defined($opts{tool_help})) {
print_tool_help();
}
- if (defined($difftool_cmd)) {
- if (length($difftool_cmd) 0) {
- $ENV{GIT_DIFF_TOOL} = $difftool_cmd;
+ if (defined($opts{difftool_cmd})) {
+ if (length($opts{difftool_cmd}) 0) {
+ $ENV{GIT_DIFF_TOOL} = $opts{difftool_cmd};
} else {
print No tool given for --tool=tool\n;
usage(1);
}
}
- if (defined($extcmd)) {
- if (length($extcmd) 0) {
- $ENV{GIT_DIFFTOOL_EXTCMD} = $extcmd;
+ if (defined($opts{extcmd})) {
+ if (length($opts{extcmd}) 0) {
+ $ENV{GIT_DIFFTOOL_EXTCMD} = $opts{extcmd};
} else {
print No cmd given for --extcmd=cmd\n;
usage(1);
}
}
- if ($gui) {
- my $guitool = '';
- $guitool = Git::config('diff.guitool');
+ if ($opts{gui}) {
+ my $guitool = Git::config('diff.guitool');
if (length($guitool) 0) {
$ENV{GIT_DIFF_TOOL} = $guitool;
}
@@ -323,10 +330,10 @@ sub main
# to compare the a/b directories. In file diff mode, 'git diff'
# will invoke a separate instance of 'git-difftool--helper' for
# each file that changed.
- if (defined($dirdiff)) {
- dir_diff($extcmd);
+ if (defined($opts{dirdiff})) {
+ dir_diff($opts{extcmd});
} else {
- file_diff($prompt);
+ file_diff($opts{prompt});
}
}
--
1.7.12.rc0.15.g8157c39
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 4/5] difftool: Call the temp directory git-difftool
The diffall name was left over from when this functionality was part of
the git-diffall script in contrib/. Make the naming consistent.
Signed-off-by: David Aguilar dav...@gmail.com
---
Same as last time, rebased
git-difftool.perl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git-difftool.perl b/git-difftool.perl
index 685887d..b4f2fc6 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -119,7 +119,7 @@ sub setup_dir_diff
exit(0) if (length($diffrtn) == 0);
# Setup temp directories
- my $tmpdir = tempdir('git-diffall.X', CLEANUP = 1, TMPDIR = 1);
+ my $tmpdir = tempdir('git-difftool.X', CLEANUP = 1, TMPDIR = 1);
my $ldir = $tmpdir/left;
my $rdir = $tmpdir/right;
mkpath($ldir) or die $!;
--
1.7.12.rc0.15.g8157c39
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 5/5] difftool: Use symlinks when diffing against the worktree
Teach difftool's --dir-diff mode to use symlinks to represent
files from the working copy, and make it the default behavior
for the non-Windows platforms.
Using symlinks is safer since we avoid needing to copy temporary
files into the worktree.
The original behavior is available as --no-symlinks.
Signed-off-by: David Aguilar dav...@gmail.com
---
Differences:
This version checks for compare() returning -1 and emit a warning.
Documentation/git-difftool.txt | 8
git-difftool.perl | 43 --
2 files changed, 41 insertions(+), 10 deletions(-)
diff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt
index 31fc2e3..313d54e 100644
--- a/Documentation/git-difftool.txt
+++ b/Documentation/git-difftool.txt
@@ -66,6 +66,14 @@ of the diff post-image. `$MERGED` is the name of the file
which is
being compared. `$BASE` is provided for compatibility
with custom merge tool commands and has the same value as `$MERGED`.
+--symlinks::
+--no-symlinks::
+ 'git difftool''s default behavior is create symlinks to the
+ working tree when run in `--dir-diff` mode.
++
+ Specifying `--no-symlinks` instructs 'git difftool' to create
+ copies instead. `--no-symlinks` is the default on Windows.
+
--tool-help::
Print a list of diff tools that may be used with `--tool`.
diff --git a/git-difftool.perl b/git-difftool.perl
index b4f2fc6..10d3d97 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -107,7 +107,7 @@ sub print_tool_help
sub setup_dir_diff
{
- my ($repo, $workdir) = @_;
+ my ($repo, $workdir, $symlinks) = @_;
# Run the diff; exit immediately if no diff found
# 'Repository' and 'WorkingCopy' must be explicitly set to insure that
@@ -224,8 +224,13 @@ sub setup_dir_diff
unless (-d $rdir/$dir) {
mkpath($rdir/$dir) or die $!;
}
- copy($workdir/$file, $rdir/$file) or die $!;
- chmod(stat($workdir/$file)-mode, $rdir/$file) or die $!;
+ if ($symlinks) {
+ symlink($workdir/$file, $rdir/$file) or die $!;
+ } else {
+ copy($workdir/$file, $rdir/$file) or die $!;
+ my $mode = stat($workdir/$file)-mode;
+ chmod($mode, $rdir/$file) or die $!;
+ }
}
# Changes to submodules require special treatment. This loop writes a
@@ -286,6 +291,8 @@ sub main
gui = undef,
help = undef,
prompt = undef,
+ symlinks = $^O ne 'cygwin'
+ $^O ne 'MSWin32' $^O ne 'msys',
tool_help = undef,
);
GetOptions('g|gui!' = \$opts{gui},
@@ -293,6 +300,8 @@ sub main
'h' = \$opts{help},
'prompt!' = \$opts{prompt},
'y' = sub { $opts{prompt} = 0; },
+ 'symlinks' = \$opts{symlinks},
+ 'no-symlinks' = sub { $opts{symlinks} = 0; },
't|tool:s' = \$opts{difftool_cmd},
'tool-help' = \$opts{tool_help},
'x|extcmd:s' = \$opts{extcmd});
@@ -331,7 +340,7 @@ sub main
# will invoke a separate instance of 'git-difftool--helper' for
# each file that changed.
if (defined($opts{dirdiff})) {
- dir_diff($opts{extcmd});
+ dir_diff($opts{extcmd}, $opts{symlinks});
} else {
file_diff($opts{prompt});
}
@@ -339,13 +348,13 @@ sub main
sub dir_diff
{
- my ($extcmd) = @_;
+ my ($extcmd, $symlinks) = @_;
my $rc;
my $repo = Git-repository();
my $workdir = find_worktree($repo);
- my ($a, $b, @working_tree) = setup_dir_diff($repo, $workdir);
+ my ($a, $b, @worktree) = setup_dir_diff($repo, $workdir, $symlinks);
if (defined($extcmd)) {
$rc = system($extcmd, $a, $b);
} else {
@@ -357,13 +366,27 @@ sub dir_diff
# If the diff including working copy files and those
# files were modified during the diff, then the changes
- # should be copied back to the working tree
- for my $file (@working_tree) {
- if (-e $b/$file compare($b/$file, $workdir/$file)) {
+ # should be copied back to the working tree.
+ # Do not copy back files when symlinks are used and the
+ # external tool did not replace the original link with a file.
+ for my $file (@worktree) {
+ next if $symlinks -l $b/$file;
+ next if ! -f $b/$file;
+
+ my $diff = compare($b/$file, $workdir/$file);
+ if ($diff == 0) {
+ next;
+ } elsif ($diff == -1 ) {
+ my $errmsg = warning: could not compare ;
+ $errmsg += '$b/$file' with '$workdir/$file'\n;
+
Re: [PATCH 1/5] difftool: Simplify print_tool_help()
On Tue, Jul 24, 2012 at 7:40 PM, Junio C Hamano gits...@pobox.com wrote: David Aguilar dav...@gmail.com writes: Does this implementation handle that case? I'm sorry, but I haven't had time to apply and test myself. [1]: http://thread.gmane.org/gmane.comp.version-control.git/193233/focus=193925 [2]: http://thread.gmane.org/gmane.comp.version-control.git/194158 Good catch. Eliminating the globals should be the goal, then. I'll have to re-roll. Junio, would you prefer follow-up patches or a rebased series? Incremental patches, please. Thanks. Okay, I saw this just before hitting send-email (and sent the whole series). I'll prep the incremental ones shortly, so please ignore the one I just sent... -- David -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/3] Incremental updates for da/difftool-updates
These are incremental updates on top of da/difftool-updates, which is currently in next. David Aguilar (3): difftool: Handle finding mergetools/ in a path with spaces difftool: Check all return codes from compare() difftool: Disable --symlinks on cygwin git-difftool.perl | 41 + 1 file changed, 33 insertions(+), 8 deletions(-) -- 1.7.12.rc0.15.g8157c39 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] difftool: Handle finding mergetools/ in a path with spaces
Use the original File::Find implementation from
bf73fc212a159210398b6d46ed5e9101c650e7db
so that we properly handle mergetools/ being located in a path containing
spaces.
One small difference is that we avoid using a global variable by
passing a reference to the list of tools.
Signed-off-by: David Aguilar dav...@gmail.com
---
git-difftool.perl | 27 +--
1 file changed, 21 insertions(+), 6 deletions(-)
diff --git a/git-difftool.perl b/git-difftool.perl
index a5b371f..3057480 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -13,9 +13,10 @@
use 5.008;
use strict;
use warnings;
-use File::Basename qw(basename dirname);
+use File::Basename qw(dirname);
use File::Copy;
use File::Compare;
+use File::Find;
use File::stat;
use File::Path qw(mkpath);
use File::Temp qw(tempdir);
@@ -58,13 +59,27 @@ sub find_worktree
return $worktree;
}
+sub filter_tool_scripts
+{
+ my ($tools) = @_;
+ if (-d $_) {
+ if ($_ ne .) {
+ # Ignore files in subdirectories
+ $File::Find::prune = 1;
+ }
+ } else {
+ if ((-f $_) ($_ ne defaults)) {
+ push(@$tools, $_);
+ }
+ }
+}
+
sub print_tool_help
{
- my ($cmd, @found, @notfound);
+ my ($cmd, @found, @notfound, @tools);
my $gitpath = Git::exec_path();
- my @files = map { basename($_) } glob($gitpath/mergetools/*);
- my @tools = sort(grep { !m{^defaults$} } @files);
+ find(sub { filter_tool_scripts(\@tools) }, $gitpath/mergetools);
foreach my $tool (@tools) {
$cmd = TOOL_MODE=diff;
@@ -79,10 +94,10 @@ sub print_tool_help
}
print 'git difftool --tool=tool' may be set to one of the
following:\n;
- print \t$_\n for (@found);
+ print \t$_\n for (sort(@found));
print \nThe following tools are valid, but not currently available:\n;
- print \t$_\n for (@notfound);
+ print \t$_\n for (sort(@notfound));
print \nNOTE: Some of the tools listed above only work in a
windowed\n;
print environment. If run in a terminal-only session, they will
fail.\n;
--
1.7.12.rc0.15.g8157c39
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] difftool: Disable --symlinks on cygwin
Symlinks are not ubiquitous on Windows so make --no-symlinks the default.
Signed-off-by: David Aguilar dav...@gmail.com
---
I don't have cygwin so I can't verify this one myself.
Is 'cygwin' really the value of $^O there?
git-difftool.perl | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/git-difftool.perl b/git-difftool.perl
index 591ee75..10d3d97 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -291,7 +291,8 @@ sub main
gui = undef,
help = undef,
prompt = undef,
- symlinks = $^O ne 'MSWin32' $^O ne 'msys',
+ symlinks = $^O ne 'cygwin'
+ $^O ne 'MSWin32' $^O ne 'msys',
tool_help = undef,
);
GetOptions('g|gui!' = \$opts{gui},
--
1.7.12.rc0.15.g8157c39
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Teach Makefile.PL to find .pm files on its own
This makes it so you no longer must edit the Makefile.PL every time you add, rename or delete a Perl module. This is convenient, and I'm about to extract a bunch of .pm files out of git-svn. You still have to edit the Makefile. That parallel build system should be able to be removed at a later date and replaced with the right Makefile.PL flags. Patch 1 and 2 are just things I noticed in the Makefile.PL along the way. Patch 3 is the meat. It doesn't depend on 1 2 but I figured it would be silly to send them separately. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] Quiet warning if Makefile.PL is run with -w and no --localedir
From: Michael G. Schwern schw...@pobox.com
Usually it isn't, but its nice if it can be run with warnings on.
Signed-off-by: Michael G Schwern schw...@pobox.com
---
perl/Makefile.PL | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/perl/Makefile.PL b/perl/Makefile.PL
index b54b04a..87e1f62 100644
--- a/perl/Makefile.PL
+++ b/perl/Makefile.PL
@@ -6,7 +6,8 @@ use Getopt::Long;
# Sanity: die at first unknown option
Getopt::Long::Configure qw/ pass_through /;
-GetOptions(localedir=s = \my $localedir);
+my $localedir = '';
+GetOptions(localedir=s = \$localedir);
sub MY::postamble {
return 'MAKE_FRAG';
--
1.7.11.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] Don't lose Error.pm if $@ gets clobbered.
From: Michael G. Schwern schw...@pobox.com
In older Perls, sometimes $@ can become unset between the eval and
checking $@. Its safer to check the eval directly.
Signed-off-by: Michael G Schwern schw...@pobox.com
---
perl/Makefile.PL | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/perl/Makefile.PL b/perl/Makefile.PL
index 87e1f62..887fa1b 100644
--- a/perl/Makefile.PL
+++ b/perl/Makefile.PL
@@ -41,8 +41,7 @@ my %pm = (
# We come with our own bundled Error.pm. It's not in the set of default
# Perl modules so install it if it's not available on the system yet.
-eval { require Error };
-if ($@ || $Error::VERSION 0.15009) {
+if ( !eval { require Error } || $Error::VERSION 0.15009) {
$pm{'private-Error.pm'} = '$(INST_LIBDIR)/Error.pm';
}
--
1.7.11.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] The Makefile.PL will now find .pm files itself.
From: Michael G. Schwern schw...@pobox.com
It is no longer necessary to manually add new .pm files to the
Makefile.PL. This makes it easier to add modules.
It is still necessary to add them to the Makefile, but that extra work
should be removed at a future date.
Signed-off-by: Michael G Schwern schw...@pobox.com
---
perl/Makefile.PL | 29 -
1 file changed, 16 insertions(+), 13 deletions(-)
diff --git a/perl/Makefile.PL b/perl/Makefile.PL
index 887fa1b..3f29ba9 100644
--- a/perl/Makefile.PL
+++ b/perl/Makefile.PL
@@ -2,6 +2,10 @@ use strict;
use warnings;
use ExtUtils::MakeMaker;
use Getopt::Long;
+use File::Find;
+
+# Don't forget to update the perl/Makefile, too.
+# Don't forget to test with NO_PERL_MAKEMAKER=YesPlease
# Sanity: die at first unknown option
Getopt::Long::Configure qw/ pass_through /;
@@ -25,19 +29,18 @@ endif
MAKE_FRAG
}
-# XXX. When editing this list:
-#
-# * Please update perl/Makefile, too.
-# * Don't forget to test with NO_PERL_MAKEMAKER=YesPlease
-my %pm = (
- 'Git.pm' = '$(INST_LIBDIR)/Git.pm',
- 'Git/I18N.pm' = '$(INST_LIBDIR)/Git/I18N.pm',
- 'Git/SVN/Memoize/YAML.pm' = '$(INST_LIBDIR)/Git/SVN/Memoize/YAML.pm',
- 'Git/SVN/Fetcher.pm' = '$(INST_LIBDIR)/Git/SVN/Fetcher.pm',
- 'Git/SVN/Editor.pm' = '$(INST_LIBDIR)/Git/SVN/Editor.pm',
- 'Git/SVN/Prompt.pm' = '$(INST_LIBDIR)/Git/SVN/Prompt.pm',
- 'Git/SVN/Ra.pm' = '$(INST_LIBDIR)/Git/SVN/Ra.pm',
-);
+# Find all the .pm files in Git/ and Git.pm
+my %pm;
+find sub {
+ return unless /\.pm$/;
+
+ # sometimes File::Find prepends a ./ Strip it.
+ my $pm_path = $File::Find::name;
+ $pm_path =~ s{^\./}{};
+
+ $pm{$pm_path} = '$(INST_LIBDIR)/'.$pm_path;
+}, Git, Git.pm;
+
# We come with our own bundled Error.pm. It's not in the set of default
# Perl modules so install it if it's not available on the system yet.
--
1.7.11.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 0/7] i18n for git-am, git-rebase and git-merge
Marked messages for translation in git-am, git-rebase, and git-merge. Also fixed affected test cases when turn GETTEXT_POISON switch on. Jiang Xin (7): i18n: New keywords for xgettext extraction from sh i18n: rebase: mark strings for translation i18n: Rewrite gettext messages start with dash Remove obsolete LONG_USAGE which breaks xgettext i18n: am: mark more strings for translation Remove dead code which contains bad gettext block i18n: merge-recursive: mark strings for translation Makefile | 3 +- git-am.sh| 20 ++--- git-rebase.sh| 89 - git-submodule.sh | 2 +- merge-recursive.c| 148 +++ t/t0201-gettext-fallbacks.sh | 8 +- t/t3400-rebase.sh| 8 +- t/t3404-rebase-interactive.sh| 2 +- t/t3406-rebase-message.sh| 2 +- t/t6022-merge-rename.sh | 16 ++-- t/t6042-merge-rename-corner-cases.sh | 2 +- 11 files changed, 143 insertions(+), 157 deletions(-) -- 1.7.12.rc0.16.gf4916ac -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 1/7] i18n: New keywords for xgettext extraction from sh
Since we have additional shell wrappers (gettextln and eval_gettextln) for gettext, we need to take into account these wrappers when running 'make pot' to extract messages from shell scripts. Signed-off-by: Jiang Xin worldhello@gmail.com Reviewed-by: Stefano Lattarini stefano.lattar...@gmail.com Reviewed-by: Jonathan Nieder jrnie...@gmail.com --- Makefile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index b0b34..d3cd9 100644 --- a/Makefile +++ b/Makefile @@ -2387,7 +2387,8 @@ XGETTEXT_FLAGS = \ --from-code=UTF-8 XGETTEXT_FLAGS_C = $(XGETTEXT_FLAGS) --language=C \ --keyword=_ --keyword=N_ --keyword=Q_:1,2 -XGETTEXT_FLAGS_SH = $(XGETTEXT_FLAGS) --language=Shell +XGETTEXT_FLAGS_SH = $(XGETTEXT_FLAGS) --language=Shell \ + --keyword=gettextln --keyword=eval_gettextln XGETTEXT_FLAGS_PERL = $(XGETTEXT_FLAGS) --keyword=__ --language=Perl LOCALIZED_C := $(C_OBJ:o=c) $(LIB_H) $(GENERATED_H) LOCALIZED_SH := $(SCRIPT_SH) -- 1.7.12.rc0.16.gf4916ac -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 2/7] i18n: rebase: mark strings for translation
Mark strings in git-rebase.sh for translation. Jonathan offers a help
for reorgnization of the resolvemsg variable in 'git-rebase.sh', since
there is a likely message in git-am.sh, I update it in this commit for
consistency. And so does to 't/t0201-gettext-fallbacks.sh'.
Some test scripts are affected by this update, and would fail if tested
with GETTEXT_POISON switch turned on. Using i18n-specific test
functions, such as test_i18ngrep, in the related test scripts will fix
these issues.
Signed-off-by: Jiang Xin worldhello@gmail.com
Reviewed-by: Stefano Lattarini stefano.lattar...@gmail.com
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
---
git-am.sh| 6 ++---
git-rebase.sh| 64 +++-
t/t0201-gettext-fallbacks.sh | 8 +++---
t/t3400-rebase.sh| 8 +++---
t/t3406-rebase-message.sh| 2 +-
5 files changed, 46 insertions(+), 42 deletions(-)
diff --git a/git-am.sh b/git-am.sh
index c02e6..8961a 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -102,9 +102,9 @@ stop_here_user_resolve () {
printf '%s\n' $resolvemsg
stop_here $1
fi
-eval_gettextln When you have resolved this problem run \\$cmdline
--resolved\.
-If you would prefer to skip this patch, instead run \\$cmdline --skip\.
-To restore the original branch and stop patching run \\$cmdline --abort\.
+eval_gettextln When you have resolved this problem, run \\$cmdline
--resolved\.
+If you prefer to skip this patch, run \\$cmdline --skip\ instead.
+To restore the original branch and stop patching, run \\$cmdline --abort\.
stop_here $1
}
diff --git a/git-rebase.sh b/git-rebase.sh
index 1cd06..8710d 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -65,6 +65,7 @@ abort! abort and check out the original branch
skip! skip current patch and continue
. git-sh-setup
+. git-sh-i18n
set_reflog_action rebase
require_work_tree_exists
cd_to_toplevel
@@ -73,9 +74,9 @@ LF='
'
ok_to_skip_pre_rebase=
resolvemsg=
-When you have resolved this problem run \git rebase --continue\.
-If you would prefer to skip this patch, instead run \git rebase --skip\.
-To check out the original branch and stop rebasing run \git rebase --abort\.
+$(gettext 'When you have resolved this problem, run git rebase --continue.
+If you prefer to skip this patch, run git rebase --skip instead.
+To check out the original branch and stop rebasing, run git rebase --abort.')
unset onto
cmd=
@@ -161,7 +162,7 @@ move_to_original_branch () {
git symbolic-ref \
-m rebase finished: returning to $head_name \
HEAD $head_name ||
- die Could not move back to $head_name
+ die $(gettext Could not move back to $head_name)
;;
esac
}
@@ -180,12 +181,12 @@ run_pre_rebase_hook () {
test -x $GIT_DIR/hooks/pre-rebase
then
$GIT_DIR/hooks/pre-rebase ${1+$@} ||
- die The pre-rebase hook refused to rebase.
+ die $(gettext The pre-rebase hook refused to rebase.)
fi
}
test -f $apply_dir/applying
- die 'It looks like git-am is in progress. Cannot rebase.'
+ die $(gettext It looks like git-am is in progress. Cannot rebase.)
if test -d $apply_dir
then
@@ -316,12 +317,12 @@ test $# -gt 2 usage
if test -n $cmd
test $interactive_rebase != explicit
then
- die --exec option must be used with --interactive option
+ die $(gettext -- --exec option must be used with --interactive
option)
fi
if test -n $action
then
- test -z $in_progress die No rebase in progress?
+ test -z $in_progress die $(gettext No rebase in progress?)
# Only interactive rebase uses detailed reflog messages
if test $type = interactive test $GIT_REFLOG_ACTION = rebase
then
@@ -334,11 +335,11 @@ case $action in
continue)
# Sanity check
git rev-parse --verify HEAD /dev/null ||
- die Cannot read HEAD
+ die $(gettext Cannot read HEAD)
git update-index --ignore-submodules --refresh
git diff-files --quiet --ignore-submodules || {
- echo You must edit all merge conflicts and then
- echo mark them as resolved using git add
+ echo $(gettext You must edit all merge conflicts and then
+mark them as resolved using git add)
exit 1
}
read_basic_state
@@ -355,7 +356,7 @@ abort)
case $head_name in
refs/*)
git symbolic-ref -m rebase: aborting HEAD $head_name ||
- die Could not move back to $head_name
+ die $(eval_gettext Could not move back to \$head_name)
;;
esac
output git reset --hard $orig_head
@@ -367,15 +368,18 @@ esac
# Make sure no rebase is in progress
if test -n $in_progress
then
- die '
-It seems that
[PATCH v4 3/7] i18n: Rewrite gettext messages start with dash
Gettext message in a shell script should not start with '-', one
workaround is adding '--' between gettext and the message, like:
gettext -- --exec option ...
But due to a bug in the xgettext extraction, xgettext can not
extract the actual message for this case. Rewriting the message
is a simpler and better solution.
Signed-off-by: Jiang Xin worldhello@gmail.com
Signed-off-by: Ævar Arnfjörð Bjarmason ava...@gmail.com
Reported-by: Vincent van Ravesteijn v...@lyx.org
Reviewed-by: Stefano Lattarini stefano.lattar...@gmail.com
---
git-rebase.sh | 2 +-
git-submodule.sh | 2 +-
t/t3404-rebase-interactive.sh | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/git-rebase.sh b/git-rebase.sh
index 8710d..705bd 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -317,7 +317,7 @@ test $# -gt 2 usage
if test -n $cmd
test $interactive_rebase != explicit
then
- die $(gettext -- --exec option must be used with --interactive
option)
+ die $(gettext The --exec option must be used with the --interactive
option)
fi
if test -n $action
diff --git a/git-submodule.sh b/git-submodule.sh
index dba4d..5b019 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -748,7 +748,7 @@ cmd_summary() {
if [ -n $files ]
then
test -n $cached
- die $(gettext -- --cached cannot be used with --files)
+ die $(gettext The --cached option cannot be used with the
--files option)
diff_cmd=diff-files
head=
fi
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 8078..f206a 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -858,7 +858,7 @@ test_expect_success 'rebase -ix with --autosquash' '
test_expect_success 'rebase --exec without -i shows error message' '
git reset --hard execute
test_must_fail git rebase --exec git show HEAD HEAD~2 2actual
- echo --exec option must be used with --interactive option expected
+ echo The --exec option must be used with the --interactive option
expected
test_i18ncmp expected actual
'
--
1.7.12.rc0.16.gf4916ac
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 4/7] Remove obsolete LONG_USAGE which breaks xgettext
The obsolete LONG_USAGE variable has the following message in it: A'\''--B'\''--C'\'' And such complex LONG_USAGE message will breaks xgettext when extracting l10n messages. But if single quotes are removed from the message, xgettext works fine on 'git-rebase.sh'. Since there is a modern OPTIONS_SPEC variable in use in this script, it's safe to remove the obsolete USAGE and LONG_USAGE variables. Signed-off-by: Jiang Xin worldhello@gmail.com Reviewed-by: Stefano Lattarini stefano.lattar...@gmail.com --- git-rebase.sh | 25 - 1 file changed, 25 deletions(-) diff --git a/git-rebase.sh b/git-rebase.sh index 705bd..0e6fd 100755 --- a/git-rebase.sh +++ b/git-rebase.sh @@ -3,31 +3,6 @@ # Copyright (c) 2005 Junio C Hamano. # -USAGE='[--interactive | -i] [--exec | -x cmd] [-v] [--force-rebase | -f] - [--no-ff] [--onto newbase] [upstream|--root] [branch] [--quiet | -q]' -LONG_USAGE='git-rebase replaces branch with a new branch of the -same name. When the --onto option is provided the new branch starts -out with a HEAD equal to newbase, otherwise it is equal to upstream -It then attempts to create a new commit for each commit from the original -branch that does not exist in the upstream branch. - -It is possible that a merge failure will prevent this process from being -completely automatic. You will have to resolve any such merge failure -and run git rebase --continue. Another option is to bypass the commit -that caused the merge failure with git rebase --skip. To check out the -original branch and remove the .git/rebase-apply working files, use the -command git rebase --abort instead. - -Note that if branch is not specified on the command line, the -currently checked out branch is used. - -Example: git-rebase master~1 topic - - A---B---C topic A'\''--B'\''--C'\'' topic - / -- / - D---E---F---G master D---E---F---G master -' - SUBDIRECTORY_OK=Yes OPTIONS_KEEPDASHDASH= OPTIONS_SPEC=\ -- 1.7.12.rc0.16.gf4916ac -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 5/7] i18n: am: mark more strings for translation
Mark strings in 'git-am.sh' for translation. In the last chunk, I
changed '$1' to '-b/--binary' for this reason:
* First, if there is a variable in the l10n message, we could not use
gettext. Because the variable will be expanded (evaluated) and will
never match the entry in the po file.
* Second, if there is a positional parameter ($1, $2,...) in the
message, we could not use eval_gettext either. Because
eval_gettext may be a wapper for gettext, and the positional
parameter would loose it's original context.
Also reduce one indentation level for one gettextln clause introduced
in commit de88c1c.
Signed-off-by: Jiang Xin worldhello@gmail.com
Reviewed-by: Stefano Lattarini stefano.lattar...@gmail.com
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
---
git-am.sh | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/git-am.sh b/git-am.sh
index 8961a..3f654 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -92,7 +92,7 @@ safe_to_abort () {
then
return 0
fi
- gettextln You seem to have moved HEAD since the last 'am'
failure.
+ gettextln You seem to have moved HEAD since the last 'am' failure.
Not rewinding to ORIG_HEAD 2
return 1
}
@@ -136,7 +136,7 @@ fall_back_3way () {
git write-tree $dotest/patch-merge-base+ ||
cannot_fallback $(gettext Repository lacks necessary blobs to fall back
on 3-way merge.)
-say Using index info to reconstruct a base tree...
+say $(gettext Using index info to reconstruct a base tree...)
cmd='GIT_INDEX_FILE=$dotest/patch-merge-tmp-index'
@@ -176,8 +176,7 @@ It does not apply to blobs recorded in its index.)
fi
git-merge-recursive $orig_tree -- HEAD $his_tree || {
git rerere $allow_rerere_autoupdate
- echo Failed to merge in the changes.
- exit 1
+ die $(gettext Failed to merge in the changes.)
}
unset GITHEAD_$his_tree
}
@@ -387,8 +386,8 @@ do
-i|--interactive)
interactive=t ;;
-b|--binary)
- echo 2 The $1 option has been a no-op for long time, and
- echo 2 it will be removed. Please do not use it anymore.
+ echo 2 $(gettext The -b/--binary option has been a no-op for
long time, and
+it will be removed. Please do not use it anymore.)
;;
-3|--3way)
threeway=t ;;
--
1.7.12.rc0.16.gf4916ac
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 6/7] Remove dead code which contains bad gettext block
Found this dead code when I examine gettext messages in shell scripts
start with dash ('-' or '--'). An error will be raised for this case,
like:
$ gettext -d option is no longer supported. Do not use.
gettext: missing arguments
Indead, this code has been left as dead for a long time, as Junathan
points out:
The git am -d/--dotest option has errored out with a message
since e72c7406 (am: remove support for -d .dotest, 2008-03-04).
The error message about lack of support was eliminated along
with other cleanups (probably by mistake) a year later by
removing the option from the option table in 98ef23b3 (git-am:
minor cleanups, 2009-01-28).
But the code to handle -d and --dotest stayed around even though
ever since then it could not be tripped. Remove this dead code.
Signed-off-by: Jiang Xin worldhello@gmail.com
Reviewed-by: Stefano Lattarini stefano.lattar...@gmail.com
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
---
git-am.sh | 3 ---
1 file changed, 3 deletions(-)
diff --git a/git-am.sh b/git-am.sh
index 3f654..c81a2 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -413,9 +413,6 @@ it will be removed. Please do not use it anymore.)
abort=t ;;
--rebasing)
rebasing=t threeway=t ;;
- -d|--dotest)
- die $(gettext -d option is no longer supported. Do not
use.)
- ;;
--resolvemsg)
shift; resolvemsg=$1 ;;
--whitespace|--directory|--exclude|--include)
--
1.7.12.rc0.16.gf4916ac
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 7/7] i18n: merge-recursive: mark strings for translation
Mark strings in merge-recursive for translation.
Some test scripts are affected by this update, and would fail if tested
with GETTEXT_POISON switch turned on. Using i18n-specific test
functions, such as test_i18ngrep, in the related test scripts will fix
these issues.
Signed-off-by: Jiang Xin worldhello@gmail.com
Reviewed-by: Stefano Lattarini stefano.lattar...@gmail.com
---
merge-recursive.c| 148 +++
t/t6022-merge-rename.sh | 16 ++--
t/t6042-merge-rename-corner-cases.sh | 2 +-
3 files changed, 88 insertions(+), 78 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index 68093..8903 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -187,7 +187,7 @@ static void output_commit_title(struct merge_options *o,
struct commit *commit)
else {
printf(%s , find_unique_abbrev(commit-object.sha1,
DEFAULT_ABBREV));
if (parse_commit(commit) != 0)
- printf((bad commit)\n);
+ printf(_((bad commit)\n));
else {
const char *title;
int len = find_commit_subject(commit-buffer, title);
@@ -203,7 +203,7 @@ static int add_cacheinfo(unsigned int mode, const unsigned
char *sha1,
struct cache_entry *ce;
ce = make_cache_entry(mode, sha1 ? sha1 : null_sha1, path, stage,
refresh);
if (!ce)
- return error(addinfo_cache failed for path '%s', path);
+ return error(_(addinfo_cache failed for path '%s'), path);
return add_cache_entry(ce, options);
}
@@ -265,7 +265,7 @@ struct tree *write_tree_from_memory(struct merge_options *o)
if (!cache_tree_fully_valid(active_cache_tree)
cache_tree_update(active_cache_tree,
active_cache, active_nr, 0) 0)
- die(error building trees);
+ die(_(error building trees));
result = lookup_tree(active_cache_tree-sha1);
@@ -494,7 +494,7 @@ static struct string_list *get_renames(struct merge_options
*o,
opts.show_rename_progress = o-show_rename_progress;
opts.output_format = DIFF_FORMAT_NO_OUTPUT;
if (diff_setup_done(opts) 0)
- die(diff setup failed);
+ die(_(diff setup failed));
diff_tree_sha1(o_tree-object.sha1, tree-object.sha1, , opts);
diffcore_std(opts);
if (opts.needed_rename_limit o-needed_rename_limit)
@@ -624,7 +624,7 @@ static void flush_buffer(int fd, const char *buf, unsigned
long size)
break;
die_errno(merge-recursive);
} else if (!ret) {
- die(merge-recursive: disk full?);
+ die(_(merge-recursive: disk full?));
}
size -= ret;
buf += ret;
@@ -687,7 +687,7 @@ static int would_lose_untracked(const char *path)
static int make_room_for_path(struct merge_options *o, const char *path)
{
int status, i;
- const char *msg = failed to create path '%s'%s;
+ const char *msg = _(failed to create path '%s'%s);
/* Unlink any D/F conflict files that are in the way */
for (i = 0; i o-df_conflict_file_set.nr; i++) {
@@ -698,7 +698,7 @@ static int make_room_for_path(struct merge_options *o,
const char *path)
path[df_pathlen] == '/'
strncmp(path, df_path, df_pathlen) == 0) {
output(o, 3,
- Removing %s to make room for subdirectory\n,
+ _(Removing %s to make room for subdirectory\n),
df_path);
unlink(df_path);
unsorted_string_list_delete_item(o-df_conflict_file_set,
@@ -712,7 +712,7 @@ static int make_room_for_path(struct merge_options *o,
const char *path)
if (status) {
if (status == -3) {
/* something else exists */
- error(msg, path, : perhaps a D/F conflict?);
+ error(msg, path, _(: perhaps a D/F conflict?));
return -1;
}
die(msg, path, );
@@ -723,7 +723,7 @@ static int make_room_for_path(struct merge_options *o,
const char *path)
* tracking it.
*/
if (would_lose_untracked(path))
- return error(refusing to lose untracked file at '%s',
+ return error(_(refusing to lose untracked file at '%s'),
path);
/* Successful unlink is good.. */
@@ -733,7 +733,7 @@ static int make_room_for_path(struct merge_options *o,
const char *path)
if (errno == ENOENT)
return 0;
/* .. but not some other error (who really cares what?) */
- return error(msg, path, : perhaps a D/F
Re: [PATCH v4 2/7] i18n: rebase: mark strings for translation
(cc-ing Duy because of a mention of his nice GETTEXT_POISON tweak[*]) Hi, Jiang Xin wrote: Mark strings in git-rebase.sh for translation. Jonathan offers a help for reorgnization of the resolvemsg variable in 'git-rebase.sh', since there is a likely message in git-am.sh, I update it in this commit for consistency. And so does to 't/t0201-gettext-fallbacks.sh'. Ah. Looks like I tweaked the comma usage and sentence structure a little. Sorry, force of habit --- I shouldn't have. Some test scripts are affected by this update, and would fail if tested with GETTEXT_POISON switch turned on. Using i18n-specific test functions, such as test_i18ngrep, in the related test scripts will fix these issues. If we're going to keep the changes together, here's how I would phrase the commit message: Mark messages in git-rebase.sh for translation. While doing this it was noticed that the comma usage and sentence structure of the resolvemsg was not quite right, so correct that and its cousins in git-am.sh and t/t0201-gettext-fallbacks.sh at the same time. Some tests would start to fail with GETTEXT_POISON turned on after this update. Use test_i18ncmp and test_i18ngrep where appropriate to mark strings that should only be checked in the C locale output to avoid such issues. Signed-off-by: Jiang Xin worldhello@gmail.com Reviewed-by: Stefano Lattarini stefano.lattar...@gmail.com Reviewed-by: Jonathan Nieder jrnie...@gmail.com I haven't tested or reviewed this patch in detail, so even though it looks good, I'd prefer it not to have my Reviewed-by. (See Documentation/SubmittingPatches: 'Reviewed-by:, unlike the other extra tags, can only be offered by the reviewer'.) If you'd like to credit my help, something like With advice from Jonathan. would be fine. [...] --- a/t/t3406-rebase-message.sh +++ b/t/t3406-rebase-message.sh @@ -64,7 +64,7 @@ test_expect_success 'rebase -n overrides config rebase.stat config' ' test_expect_success 'rebase --onto outputs the invalid ref' ' test_must_fail git rebase --onto invalid-ref HEAD HEAD 2err - grep invalid-ref err + test_i18ngrep invalid-ref err ' Could we add a comment so others do not have to wonder what human-readable message prompts the test_i18ngrep here? e.g # Does not point to a valid commit: invalid-ref # # NEEDSWORK: This grep is fine in real non-C locales, but # GETTEXT_POISON poisons the refname along with the enclosing # error message. test_i18ngrep invalid-ref err In the long run we may be able to turn this back to a grep again, since any reasonable translation will keep the $onto_name somewhere in the message. But changing it to test_i18ngrep for now is the right thing to do, until something like Duy's more sophisticated version of GETTEXT_POISON arrives. (-- [*]) Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 5/7] i18n: am: mark more strings for translation
Jiang Xin wrote: Mark strings in 'git-am.sh' for translation. In the last chunk, I changed '$1' to '-b/--binary' for this reason: * First, if there is a variable in the l10n message, we could not use gettext. Because the variable will be expanded (evaluated) and will never match the entry in the po file. * Second, if there is a positional parameter ($1, $2,...) in the message, we could not use eval_gettext either. Because eval_gettext may be a wapper for gettext, and the positional parameter would loose it's original context. Yes, I think it's a good change. --- a/git-am.sh +++ b/git-am.sh [...] @@ -387,8 +386,8 @@ do -i|--interactive) interactive=t ;; -b|--binary) - echo 2 The $1 option has been a no-op for long time, and - echo 2 it will be removed. Please do not use it anymore. + echo 2 $(gettext The -b/--binary option has been a no-op for long time, and +it will be removed. Please do not use it anymore.) Could this be simplified to gettextln 2 'The -b/--binary option... ... any more.' or gettextln 'The -b/--binary option ... ... any more.' 2 or would that confuse xgettext? That would be more comforting than echo because if the translated string includes backslashes then the behavior of echo can be unpredictable. Sorry I missed that last time. The rest is indeed Reviewed-by: Jonathan Nieder jrnie...@gmail.com Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 6/7] Remove dead code which contains bad gettext block
Jiang Xin wrote:
Found this dead code when I examine gettext messages in shell scripts
start with dash ('-' or '--'). An error will be raised for this case,
like:
$ gettext -d option is no longer supported. Do not use.
gettext: missing arguments
Indead, this code has been left as dead for a long time, as Junathan
points out:
Jonathan, not Junathan. :)
The git am -d/--dotest option has errored out with a message
since e72c7406 (am: remove support for -d .dotest, 2008-03-04).
The error message about lack of support was eliminated along
with other cleanups (probably by mistake) a year later by
removing the option from the option table in 98ef23b3 (git-am:
minor cleanups, 2009-01-28).
But the code to handle -d and --dotest stayed around even though
ever since then it could not be tripped. Remove this dead code.
Signed-off-by: Jiang Xin worldhello@gmail.com
Your explanation is certainly clearer than mine. So: yes, for what
it's worth this is
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git-svn SVN 1.7 fix, take 2
Michael G Schwern wrote: No, now it's just canonicalizing as early as possible. Preferably within the object accessor rather than at the point of use. So in the code below, $full_url is already escaped/canonicalized. Let's start with this. Is svn_path_canonicalize() idempotent? What does it do when it encounters a percent-sign? Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git-svn SVN 1.7 fix, take 2
On 2012.7.24 9:53 PM, Jonathan Nieder wrote: Michael G Schwern wrote: No, now it's just canonicalizing as early as possible. Preferably within the object accessor rather than at the point of use. So in the code below, $full_url is already escaped/canonicalized. Let's start with this. Is svn_path_canonicalize() idempotent? What does it do when it encounters a percent-sign? Nothing, because paths are not URI escaped. :) You probably meant svn_uri_canonicalize(). And no, it does not double escape, so its safe to escape as early as possible. use SVN::Core; my $uri = http://www.example.com/ foo; print SVN::_Core::svn_uri_canonicalize( SVN::_Core::svn_uri_canonicalize($uri) ); That produces http://www.example.com/%20foo;. The API docs don't say it specifically, but if it were otherwise it would be impossible to use. You'd have to check first if anything were escaped before canonicalizing. And a user couldn't pass in an escaped URL without risking it being double escaped. http://subversion.apache.org/docs/api/latest/svn__dirent__uri_8h.html#a8bae33a2fbf86857869f7b0e39a553e7 -- 'All anyone gets in a mirror is themselves,' she said. 'But what you gets in a good gumbo is everything.' -- Witches Abroad by Terry Prachett -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Extract Git classes from git-svn (1/10)
On 2012.7.24 7:55 PM, Eric Wong wrote: After I'm exhausted from volunteering all the coding work, rather than submitting a URL to a remote repository I find I have to learn new specialized tools. It's extra learning and work, an extra step to screw up, and foreign to me (even as a experienced git user). It is of little benefit to me as a casual volunteer submitter. Except git is also a new specialized tool. Your examples are exactly why I'm saddened many projects only adopted git, but not the workflow which _built_ git (and Linux). There is an important difference between a tool which is useful for one or two projects and one which is useful for a broad spectrum of projects. I learn git once (or diff or bash or Perl or whatever) and I'm going to use it again and again all over the place. I learn git-send-email and (if I'm not a kernel developer) I'm going to use it on a handful of projects maybe. It's O(1) vs O(n) effort. Github also has broad spectrum utility. I learn how to fork and work with a Github pull request once, and I can repeat that on thousands of different projects of all different sorts of things. This commonality of tools and techniques is very important to easing the on ramp for new (to-your-project) developers. I can see if you've been on the git mailing list for a while and have git-am and all that set up, this system is great. But it comes at a cost which is offloaded onto new and casual contributors. Email is integral to Free/Open Source development and remains one of the few things on the Internet not (yet) controlled by any central entity. Once setup, the same email setup can work across all projects which use email. These projects need not be hosted on the same websites/servers at all. While I hear your concern about being centrally controlled, it is largely irrelevant to the new user experience. And remaining independent does not mean you can't use web tools. Be wary of a false dichotomy between Free and web. We use a mailing list is by no means an indication of commonality. Every project of the send patches to the list form has their own quirks and ways of doing it. Usually they're not written down. This is what I've been struggling with. I've been sending patches to mailing lists for decades and I can tell you everybody does it differently. Send a patch to the list is one of the steeper project on-ramps. This sort of specialized setup makes people bounce right off the submission process. At OSCON I was asking around for help getting things setup so I could submit patches here properly. As soon as they said which mail daemon are you running?, I said stop! I don't want to know any more. I have too many things to do to be fiddling with my mailer configuration just to submit volunteer work in the right form (that said, I'm pleased as punch that git-send-email now has instructions for sending via GMail). You're volunteers, too. We're all volunteers, so a more balanced submission process would be nice. How about we educate users about a proper email setup instead? If they're capable of learning git, they're surely capable of setting up an email client properly, and perhaps more projects can adopt an email-centric workflow. SubmittingPatches would be helped by that, particularly with a clear step-by-step example of using git-send-email and all its numerous command line switches. I was showing Jonathan the guide I have for releasing Perl modules which is both A) step-by-step and also B) covers the numerous little problems that are usually only in somebody's head or scattered around the docs. It was built in order to allow a person who had never released a module to release a module. Then we watched just such a person follow the directions. As they asked questions, struggled or made mistakes we filled in the gaps in the docs. https://github.com/scrottie/autobox-Core/wiki/How-To-Make-A-Release But this is also not about capability. Yes, people are capable of figuring out git-send-email, but its Yet Another Special Thing to learn before they can submit a patch and call their work done. Volunteers, especially brand new ones, have only so much volunteerism to burn before they'll walk away. You want them burning that on productive patching and contributions, not learning specialty tools. And, finally, the last thing most people want is more email. Seriously. It sounds like you live in your mailer, but fewer and fewer people do that. Me? I don't want to join another mailing list. My email management is a disaster! What it comes down to is this: is it enough to contribute to git.git to know how to work on git.git? Or do you also need to live in your mailer? Bolt on that extra requirement and you lose a large swath of contributors. But since you brought Github up... (I get the impression its kind of a dirty word around here) (Not speaking for the git project) I'm entirely against the way GitHub (or
