Move Git::SVN into its own .pm file
This is a refactoring to move Git::SVN out of git-svn and into its own .pm file. This will make it easier to work with and test. This is just the extraction with minimal work to keep all tests passing. A couple of utility functions which were used by Git::SVN, git-svn and others were also extracted from git-svn into a new Git::SVN::Utils. Not the most imaginitive name, but it's better than Git::SVN grabbing at git-svn internals and it allows Git::SVN (and later other classes) to stand alone without git-svn. Its was reworked to be done backwards (instead of extracting and then fixing the resulting problems, the problems were fixed in place and then it's extracted) in order to keep every commit passing tests and provide a useful history. This was something of a pain. -- 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/4] Prepare Git::SVN for extraction into its own file.
From: Michael G. Schwern schw...@pobox.com
This means it should be able to load without git-svn being loaded.
* Load Git.pm on its own and all the needed command functions.
* It needs to grab at a git-svn lexical $_prefix representing the --prefix
option. Provide opt_prefix() for that. This is a refactoring artifact.
The prefix should really be passed into Git::SVN-new.
* Unqualify unnecessarily fully qualified globals like
$Git::SVN::default_repo_id.
* Lexically isolate the class just to make sure nothing is leaking out.
---
git-svn.perl | 22 ++
1 file changed, 18 insertions(+), 4 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index 79fe4a4..9cdf6fc 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -89,7 +89,7 @@ BEGIN {
foreach (qw/command command_oneline command_noisy command_output_pipe
command_input_pipe command_close_pipe
command_bidi_pipe command_close_bidi_pipe/) {
- for my $package ( qw(Git::SVN::Migration Git::SVN::Log
Git::SVN),
+ for my $package ( qw(Git::SVN::Migration Git::SVN::Log),
__PACKAGE__) {
*{${package}::$_} = \{Git::$_};
}
@@ -109,6 +109,10 @@ my ($_stdin, $_help, $_edit,
$_merge, $_strategy, $_preserve_merges, $_dry_run, $_local,
$_prefix, $_no_checkout, $_url, $_verbose,
$_git_format, $_commit_url, $_tag, $_merge_info, $_interactive);
+
+# This is a refactoring artifact so Git::SVN can get at this git-svn switch.
+sub opt_prefix { return $_prefix || '' }
+
$Git::SVN::_follow_parent = 1;
$Git::SVN::Fetcher::_placeholder_filename = .gitignore;
$_q ||= 0;
@@ -2038,6 +2042,7 @@ sub gc_directory {
}
}
+{
package Git::SVN;
use strict;
use warnings;
@@ -2056,6 +2061,13 @@ use Memoize; # core since 5.8.0, Jul 2002
use Memoize::Storable;
use POSIX qw(:signal_h);
+use Git qw(
+command
+command_oneline
+command_noisy
+command_output_pipe
+command_close_pipe
+);
use Git::SVN::Utils qw(fatal can_compress);
my $can_use_yaml;
@@ -4280,12 +4292,13 @@ sub find_rev_after {
sub _new {
my ($class, $repo_id, $ref_id, $path) = @_;
unless (defined $repo_id length $repo_id) {
- $repo_id = $Git::SVN::default_repo_id;
+ $repo_id = $default_repo_id;
}
unless (defined $ref_id length $ref_id) {
- $_prefix = '' unless defined($_prefix);
+ # Access the prefix option from the git-svn main program if
it's loaded.
+ my $prefix = defined ::opt_prefix ? ::opt_prefix() : ;
$_[2] = $ref_id =
-refs/remotes/$_prefix$Git::SVN::default_ref_id;
+refs/remotes/$prefix$default_ref_id;
}
$_[1] = $repo_id;
my $dir = $ENV{GIT_DIR}/svn/$ref_id;
@@ -4347,6 +4360,7 @@ sub uri_decode {
sub remove_username {
$_[0] =~ s{^([^:]*://)[^@]+@}{$1};
}
+}
package Git::SVN::Log;
use strict;
--
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 4/4] Move initialization of Git::SVN variables into Git::SVN.
From: Michael G. Schwern schw...@pobox.com
Also it can compile on its own now, yay!
---
git-svn.perl | 4
perl/Git/SVN.pm | 9 +++--
t/Git-SVN/00compile.t | 3 ++-
3 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index 4c77f69..ef10f6f 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -20,10 +20,7 @@ my $cmd_dir_prefix = eval {
my $git_dir_user_set = 1 if defined $ENV{GIT_DIR};
$ENV{GIT_DIR} ||= '.git';
-$Git::SVN::default_repo_id = 'svn';
-$Git::SVN::default_ref_id = $ENV{GIT_SVN_ID} || 'git-svn';
$Git::SVN::Ra::_log_window_size = 100;
-$Git::SVN::_minimize_url = 'unset';
if (! exists $ENV{SVN_SSH} exists $ENV{GIT_SSH}) {
$ENV{SVN_SSH} = $ENV{GIT_SSH};
@@ -114,7 +111,6 @@ my ($_stdin, $_help, $_edit,
# This is a refactoring artifact so Git::SVN can get at this git-svn switch.
sub opt_prefix { return $_prefix || '' }
-$Git::SVN::_follow_parent = 1;
$Git::SVN::Fetcher::_placeholder_filename = .gitignore;
$_q ||= 0;
my %remote_opts = ( 'username=s' = \$Git::SVN::Prompt::_username,
diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm
index c71c041..2e0d7f0 100644
--- a/perl/Git/SVN.pm
+++ b/perl/Git/SVN.pm
@@ -3,9 +3,9 @@ use strict;
use warnings;
use Fcntl qw/:DEFAULT :seek/;
use constant rev_map_fmt = 'NH40';
-use vars qw/$default_repo_id $default_ref_id $_no_metadata $_follow_parent
+use vars qw/$_no_metadata
$_repack $_repack_flags $_use_svm_props $_head
-$_use_svnsync_props $no_reuse_existing $_minimize_url
+$_use_svnsync_props $no_reuse_existing
$_use_log_author $_add_author_from $_localtime/;
use Carp qw/croak/;
use File::Path qw/mkpath/;
@@ -30,6 +30,11 @@ BEGIN {
$can_use_yaml = eval { require Git::SVN::Memoize::YAML; 1};
}
+our $_follow_parent = 1;
+our $_minimize_url = 'unset';
+our $default_repo_id = 'svn';
+our $default_ref_id = $ENV{GIT_SVN_ID} || 'git-svn';
+
my ($_gc_nr, $_gc_period);
# properties that we do not log:
diff --git a/t/Git-SVN/00compile.t b/t/Git-SVN/00compile.t
index a7aa85a..97475d9 100644
--- a/t/Git-SVN/00compile.t
+++ b/t/Git-SVN/00compile.t
@@ -3,6 +3,7 @@
use strict;
use warnings;
-use Test::More tests = 1;
+use Test::More tests = 2;
require_ok 'Git::SVN::Utils';
+require_ok 'Git::SVN';
--
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] difftool: Do not remove temporary files on error
Keep the temporary directory around when either compare() or
the difftool returns a non-zero exit status.
Tell the user about the location of the temporary files so that
they can recover from the failure.
Signed-off-by: David Aguilar dav...@gmail.com
---
git-difftool.perl | 36 ++--
1 file changed, 26 insertions(+), 10 deletions(-)
diff --git a/git-difftool.perl b/git-difftool.perl
index 10d3d97..f4f7d4a 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -18,7 +18,7 @@ use File::Copy;
use File::Compare;
use File::Find;
use File::stat;
-use File::Path qw(mkpath);
+use File::Path qw(mkpath rmtree);
use File::Temp qw(tempdir);
use Getopt::Long qw(:config pass_through);
use Git;
@@ -119,7 +119,7 @@ sub setup_dir_diff
exit(0) if (length($diffrtn) == 0);
# Setup temp directories
- my $tmpdir = tempdir('git-difftool.X', CLEANUP = 1, TMPDIR = 1);
+ my $tmpdir = tempdir('git-difftool.X', CLEANUP = 0, TMPDIR = 1);
my $ldir = $tmpdir/left;
my $rdir = $tmpdir/right;
mkpath($ldir) or die $!;
@@ -257,7 +257,7 @@ sub setup_dir_diff
}
}
- return ($ldir, $rdir, @working_tree);
+ return ($ldir, $rdir, $tmpdir, @working_tree);
}
sub write_to_file
@@ -349,20 +349,23 @@ sub main
sub dir_diff
{
my ($extcmd, $symlinks) = @_;
-
my $rc;
+ my $error = 0;
my $repo = Git-repository();
-
my $workdir = find_worktree($repo);
- my ($a, $b, @worktree) = setup_dir_diff($repo, $workdir, $symlinks);
+ my ($a, $b, $tmpdir, @worktree) =
+ setup_dir_diff($repo, $workdir, $symlinks);
+
if (defined($extcmd)) {
$rc = system($extcmd, $a, $b);
} else {
$ENV{GIT_DIFFTOOL_DIRDIFF} = 'true';
$rc = system('git', 'difftool--helper', $a, $b);
}
-
- exit($rc | ($rc 8)) if ($rc != 0);
+ if ($rc != 0) {
+ dir_diff_tmpdir_warning($tmpdir);
+ exit($rc | ($rc 8));
+ }
# If the diff including working copy files and those
# files were modified during the diff, then the changes
@@ -377,16 +380,29 @@ sub dir_diff
if ($diff == 0) {
next;
} elsif ($diff == -1 ) {
- my $errmsg = warning: could not compare ;
+ my $errmsg = warning: Could not compare ;
$errmsg += '$b/$file' with '$workdir/$file'\n;
warn $errmsg;
+ $error = 1;
} elsif ($diff == 1) {
copy($b/$file, $workdir/$file) or die $!;
my $mode = stat($b/$file)-mode;
chmod($mode, $workdir/$file) or die $!;
}
}
- exit(0);
+ if ($error) {
+ dir_diff_tmpdir_warning($tmpdir);
+ } else {
+ rmtree($tmpdir);
+ }
+ exit($error);
+}
+
+sub dir_diff_tmpdir_warning
+{
+ my ($tmpdir) = @_;
+ warn warning: Temporary files exist in '$tmpdir'.\n;
+ warn warning: You may want to cleanup or recover these.\n;
}
sub file_diff
--
1.7.11.9.g2b1cfc7.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: OT: mail-based interfaces and web-based interfaces (Re: Extract Git classes from git-svn (1/10))
On 2012.7.24 10:54 PM, Jonathan Nieder wrote: And again, it *does not have to be zero sum*. It doesn't have to be email VS GUI. You can have your cake and eat it too. I assume you're talking about web-based interfaces that have gateways to email, that produce inboxes like this: 24 Jul 02:46 GitHub [github] msysgit/msysgit was forked by peters 23 Jul 10:27 GitHub [msysgit/git] ce8ebc: vcs-svn: rename check_o 23 Jul 10:01 GitHub [github] Comment created on issue 44 (new git 23 Jul 09:50 GitHub [github] Comment created on issue 44 (new git 23 Jul 09:33 GitHub [github] Comment created on issue 44 (new git 23 Jul 09:39 GitHub [github] Comment created on issue 24 (Long fi 23 Jul 09:31 GitHub [github] Comment created on issue 44 (new git 23 Jul 09:30 GitHub [github] Comment created on issue 24 (Long fi 22 Jul 23:57 GitHub [github] Comment created on issue 44 (new git I call that pretending to have my cake, rather than having it. :) That's kind of like pointing at RCS and saying version control sucks and its pointless to try and make it better! Mail gateways built by web sites suck because they live in the web browser and email is an after thought. Sound familiar? Here is a much better example of the RT mail gateway that Perl 5 development uses. They're a dev community still centered around email, so it has to integrate well. http://www.nntp.perl.org/group/perl.perl5.porters/2012/07/msg189716.html And the corresponding ticket in the tracker. https://rt.perl.org/rt3/Public/Bug/Display.html?id=113684 The initial report comes in either via the web tracker or via a command line program (perlbug) that sends an email to the list. Replies on either the tracker or the mailing list are mirrored. Duplicates are detected etc... That's the sort of mail gateways I'm used to. Maybe some day someone will prove me wrong and make a nice web-based tool that I don't even need to know about that mines project mailing lists. If I have to tweak my subject lines a little to help it out, that's fine with me. I think patchwork is supposed to work this way. But unless we're talking about splitting the mailing list into a bunch of mini mailing lists (like some bug trackers do), it doesn't change anything fundamental, so I'm not sure why we're discussing this. I don't follow the bit about splitting the mailing list. -- emacs -- THAT'S NO EDITOR... IT'S AN OPERATING SYSTEM! -- 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 Tuesday 24 July 2012 14:50:49 Jonathan Nieder wrote: 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. Exactly, the purpose is to transparently replace svnrdump. Callers of svnrdump usually will specify -rSTART:HEAD, because they want to fetch everything they don't yet have. This feature allows to limit HEAD and to simulate incremental fetches using the same dump file. For me it proved very useful. Florian, do you mind if I make the revlimit feature a separate patch when applying this? No problem. Anyway, it looks good and reasonable to me, so will apply. Thanks. Jonathan -- Florian -- 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
Junio C Hamano gits...@pobox.com writes:
Jonathan Nieder jrnie...@gmail.com writes:
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.
Yes. The original plan was to free up -b, but since we don't need
--binary any more either, it's better if we can get rid of it the same
way.
--
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
Re: [GSoC] Designing a faster index format - Progress report week 13
Junio C Hamano gits...@pobox.com writes:
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.
True, sorry for being so confusing. 'Throw that out' was meant to
refer to the observation that smaller indexes are generally faster.
Whether this matters in the long run is another question. Perhaps
partial loading combined with something like inotify can avoid full
reads in most operations.
--
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
Re: git-svn SVN 1.7 fix, take 2
Michael G Schwern schw...@pobox.com writes: 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. Are you saying that the function assumes that a local pathname would not have '%' in it, returns its input as-is when it sees one, and if the caller really needs to express a path with '%' in it, it is the responsibility of the caller to escape it? That makes it even more confusing 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;. In other words, if your DocumentRoot was /var/www and you have a directory /var/www/per%cent you want to expose to the outside world, you have to say http://www.example.com/per%25cent; yourself and the canonicalize function will be an identity function? I have this vague suspicion that Jonathan was asking about what your Git::SVN::Utils::canonicalize_path() sub does, so all of the above might be moot, though... -- 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 v5 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 v5 2/7] i18n: rebase: mark messages for translation
Mark messages in git-rebase.sh for translation. While doing this
Jonathan 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
Acked-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| 9 ++-
5 files changed, 53 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 there
[PATCH v5 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 v5 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 25d0e..bd962 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 v5 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..25d0e 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.
+ gettextln 2 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
Re: [PATCH v4 2/7] i18n: rebase: mark strings for translation
Jiang Xin wrote: 2012/7/25 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. How about Acked-by: ? I'm not area expert so my ack wouldn't count for git rebase or i18n, anyway. I'm also fine with going unsung --- too many names add up to so much noise, and in general there are some names (author, bug reporter, and so on) that are important and shouldn't be drowned out. -- 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.25 12:14 AM, Junio C Hamano wrote: 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. Are you saying that the function assumes that a local pathname would URI and path canonicalization are done differently and by different functions. svn_uri_canonicalize() vs svn_dirent_canonicalize(). Or maybe you're referring to the path portion of the URL? I don't think that makes a difference for what you're asking, but its important to keep in mind. not have '%' in it, returns its input as-is when it sees one, and if the caller really needs to express a path with '%' in it, it is the responsibility of the caller to escape it? It appears that if the % is followed by hex it assumes its an escape. Otherwise it escapes it. Thus... http://www.google.com/per%%nt - http://www.google.com/per%25%25nt http://www.google.com/per%ant - http://www.google.com/per%25ant http://www.google.com/per%cent - http://www.google.com/per%CEnt Which makes sense if the idea is to not double escape. That makes it even more confusing Straight out of the RFC. http://pretty-rfc.herokuapp.com/RFC3986#when-to-percent-encode Implementations must not percent-encode or decode the same string more than once, as decoding an already decoded string might lead to misinterpreting a percent data octet as the beginning of a percent- encoding, or vice versa in the case of percent-encoding an already percent-encoded string. It makes it far simpler to use. You can't read the mind of the user, but its a fair guess that they're not really thinking too deeply about how escaping works. It makes URI and path canonicalization safer and simpler. Otherwise you'd need to keep track of whether a thing was already escaped or not! Just begging for loads of bugs. (If SVN were using URI and path objects they'd just take care of it and none of this would be a problem in the first place). This way you have no double escaping concerns. No need to track if a thing is already canonicalized. Do it as often and as early as you like. Making a corner case a little harder is a small price to pay for making the common case much, much easier. This also appears to be what Firefox does. 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;. In other words, if your DocumentRoot was /var/www and you have a directory /var/www/per%cent you want to expose to the outside world, you have to say http://www.example.com/per%25cent; yourself and the canonicalize function will be an identity function? Yes. It can be made to work better. There's a number of places in the code which effectively do this: my $full_url = $url . '/' . $path; And I was canonicalizing them like this: my $full_url = canonicalize_url($url . '/' . $path); I'd been pondering whether it would be worthwhile to have a function which added a path to a base URL and canonicalized. Now I see that yes, it would be to deal with this corner case. my $full_url = append_path_to_url($url, $path); That would properly URI encode any % in $path before appending and then canonicalizing the whole thing as a URI. I'm pretty sure the code in master doesn't handle this at all. I have this vague suspicion that Jonathan was asking about what your Git::SVN::Utils::canonicalize_path() sub does, so all of the above might be moot, though... Its just a pass through to the SVN API. -- 44. I am not the atheist chaplain. -- 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 v4 5/7] i18n: am: mark more strings for translation
Sorry to be so nit-picky, but ... On 07/25/2012 05:53 AM, 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. ... here you should s/it's/its/. 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 v4 5/7] i18n: am: mark more strings for translation
2012/7/25 Stefano Lattarini stefano.lattar...@gmail.com: * 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. ... here you should s/it's/its/. OMG. You find what some grammer/spelling checking website hasn't. If there are no other serious problems, I won't make noises (send another series of patches) to this list, should I? -- 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 v5 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:
s/Junathan/Jonathan/, please, unless you are trying to say that both
Junio and I pointed it out at the same time.
--
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 v5 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 v5 2/7] i18n: rebase: mark messages for translation
Mark messages in git-rebase.sh for translation. While doing this
Jonathan 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
---
git-am.sh| 6 ++---
git-rebase.sh| 64 +++-
t/t0201-gettext-fallbacks.sh | 8 +++---
t/t3400-rebase.sh| 8 +++---
t/t3406-rebase-message.sh| 9 ++-
5 files changed, 53 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 there is already a '${state_dir##*/}' directory,
[PATCH v5 7/7] i18n: merge-recursive: mark strings for translation
Mark strings in merge-recursive for translation.
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
---
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
[PATCH v6 0/7] i18n for git-am, git-rebase and git-merge
[Sorry, bad patch series number, resend again. I'm sleepy.] Marked messages for translation in git-am, git-rebase, and git-merge. Also fixed suffered tests when turning GETTEXT_POISON switch on. Jiang Xin (7): i18n: New keywords for xgettext extraction from sh i18n: rebase: mark messages 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| 9 ++- t/t6022-merge-rename.sh | 16 ++-- t/t6042-merge-rename-corner-cases.sh | 2 +- 11 files changed, 150 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 v6 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 v6 2/7] i18n: rebase: mark messages for translation
Mark messages in git-rebase.sh for translation. While doing this
Jonathan 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
---
git-am.sh| 6 ++---
git-rebase.sh| 64 +++-
t/t0201-gettext-fallbacks.sh | 8 +++---
t/t3400-rebase.sh| 8 +++---
t/t3406-rebase-message.sh| 9 ++-
5 files changed, 53 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 there is already a '${state_dir##*/}' directory,
[PATCH v6 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
lose its 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..25d0e 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.
+ gettextln 2 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 v6 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 break 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 v6 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 Jonathan
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 25d0e..bd962 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
RE: git cloning paths
Hi Doug, The method I have been using to achieve this is to create a wrapper script that does the following: git clone -n # clone, but don't checkout cd repo name git config core.sparseCheckout true# configure sparse-checkout on # echo the list of bits you want into .git/info/sparse-checkout git checkout Do watch out though, the interpretation of the sparse-checkout file has changed since git 1.7, I would suggest you use the latest git and record the git version as a comment in top of the sparse checkout file, in case it changes again. I hope this helps, Caleb -Original Message- From: git-ow...@vger.kernel.org [mailto:git-ow...@vger.kernel.org] On Behalf Of Douglas Garstang Sent: 12 July 2012 23:17 To: git@vger.kernel.org Subject: git cloning paths All, I'm a relative newcomer to git and I've just inherited a setup where all of the company's code is in a single git repository. Within this repository are multiple projects. It seems that git doesn't natively allow cloning/checking out of individual paths within the repo (ie projects), which would seem to make integrating git with a continuous build system rather difficult. That is, the build system has to clone the entire repo, and therefore a change to any project will result in the entire contents of the repo being built. Correct? Doug. -- 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 -- 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
How to unignore files in certain directories?
Hi, I have the following in .gitignore to ignore *.txt files. *.txt But I want to keep the *.txt files in, for example, data/ and all its subdirectories. I don't know should the the correct way to unignore these *.txt files. Could anybody show me the command that I should add to .gitignore? Thanks! -- Regards, Peng -- 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 v2 0/4] allow recovery from command name typos
Patch 4 has the meat of this series. While running valgrind to check that I didn't leak any memory, a couple of leaks were spotted. Patches 1-3 address them. Major change in v2: implement Thomas' idea [1] about using help.autocorrect to configure this behaviour. [1] 878vh4con4@thomas.inf.ethz.ch Jeff King (1): help.c::uniq: plug a leak Tay Ray Chuan (3): help.c::exclude_cmds: realloc() before copy, plug a leak help.c: plug leaks with(out) help.autocorrect allow recovery from command name typos Documentation/config.txt | 30 advice.c | 2 ++ advice.h | 1 + help.c | 94 ++-- 4 files changed, 110 insertions(+), 17 deletions(-) -- 1.7.11.1.116.g8228a23 -- 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 v2 2/4] help.c::exclude_cmds: realloc() before copy, plug a leak
Copying with structural assignment may not take into account that the
LHS struct has sufficient memory, especially since the cmdname-name
member is nonfixed in size. Be unambiguous about it by realloc()'ing it
to be of sufficient size.
Additionally, free the unused cmdnames, which are no longer accessible
anyway.
Signed-off-by: Tay Ray Chuan rcta...@gmail.com
---
help.c | 20 ++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/help.c b/help.c
index 6991492..dfb2e9d 100644
--- a/help.c
+++ b/help.c
@@ -20,6 +20,17 @@ void add_cmdname(struct cmdnames *cmds, const char *name,
int len)
cmds-names[cmds-cnt++] = ent;
}
+static void copy_cmdname(struct cmdname **dest, struct cmdname *src)
+{
+ struct cmdname *ent = xrealloc(*dest, sizeof(*ent) + src-len + 1);
+
+ ent-len = src-len;
+ memcpy(ent-name, src-name, src-len);
+ ent-name[src-len] = 0;
+
+ *dest = ent;
+}
+
static void clean_cmdnames(struct cmdnames *cmds)
{
int i;
@@ -58,20 +69,25 @@ void exclude_cmds(struct cmdnames *cmds, struct cmdnames
*excludes)
{
int ci, cj, ei;
int cmp;
+ int last_cj;
ci = cj = ei = 0;
while (ci cmds-cnt ei excludes-cnt) {
cmp = strcmp(cmds-names[ci]-name, excludes-names[ei]-name);
if (cmp 0)
- cmds-names[cj++] = cmds-names[ci++];
+ copy_cmdname(cmds-names[cj++], cmds-names[ci++]);
else if (cmp == 0)
ci++, ei++;
else if (cmp 0)
ei++;
}
+ last_cj = cj;
while (ci cmds-cnt)
- cmds-names[cj++] = cmds-names[ci++];
+ copy_cmdname(cmds-names[cj++], cmds-names[ci++]);
+
+ while (last_cj cmds-cnt)
+ free(cmds-names[last_cj++]);
cmds-cnt = cj;
}
--
1.7.11.1.116.g8228a23
--
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 v2 4/4] allow recovery from command name typos
If suggestions are available (based on Levenshtein distance) and if the
terminal isatty(), present a prompt to the user to select one of the
computed suggestions.
In the case where there is a single suggestion, present the prompt
[Y/n], such that (ie. the default), y and Y as input leads git
to proceed executing the suggestion, while everything else (possibly
n) leads git to terminate.
In the case where there are multiple suggestions, number the suggestions
1 to n (the number of suggestions), and accept an integer as input,
while everything else (possibly n) leads git to terminate. In this
case there is no default; an empty input leads git to terminate. A
sample run:
$ git sh --pretty=oneline
git: 'sh' is not a git command. See 'git --help'.
Did you mean one of these?
1:show
2:push
[N/1/2/...]
This prompt is enabled only if help.autocorrect is set to ask; if unset,
advise the user about this ability.
Signed-off-by: Tay Ray Chuan rcta...@gmail.com
---
Changed in v2: implement Thomas' idea [1] to hijack help.autocorrect to
configure this behaviour.
[1] 878vh4con4@thomas.inf.ethz.ch
---
Documentation/config.txt | 30 +--
advice.c | 2 ++
advice.h | 1 +
help.c | 63 +---
4 files changed, 85 insertions(+), 11 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 0bcea8a..0bb175a 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -177,6 +177,10 @@ advice.*::
Advice shown when you used linkgit:git-checkout[1] to
move to the detach HEAD state, to instruct how to create
a local branch after the fact.
+ typoPrompt::
+ Upon a mistyped command, if 'help.autocorrect' is unset
+ advise that an interactive prompt can be displayed to
+ recover from the typo.
--
core.fileMode::
@@ -1323,13 +1327,25 @@ help.format::
the default. 'web' and 'html' are the same.
help.autocorrect::
- Automatically correct and execute mistyped commands after
- waiting for the given number of deciseconds (0.1 sec). If more
- than one command can be deduced from the entered text, nothing
- will be executed. If the value of this option is negative,
- the corrected command will be executed immediately. If the
- value is 0 - the command will be just shown but not executed.
- This is the default.
+ Specifies behaviour to recover from mistyped commands.
++
+When set to `ask`, an interactive prompt is displayed, allowing the user
+to select a suggested command for execution.
++
+When set to `off`, no attempt to recover is made.
++
+If a number is given, it will be interpreted as the deciseconds (0.1
+sec) to wait before automatically correcting and executing the mistyped
+command, with the following behaviour:
++
+* If more than one command can be deduced from the entered text, nothing
+ will be executed.
+* If the value of this option is negative, the corrected command will be
+ executed immediately.
+* If the value is 0 - the command will be just shown but not executed.
++
+The default is to display a message suggesting that this option be set
+to `ask`, without attempting to recover (see `advice.typoPrompt`).
http.proxy::
Override the HTTP proxy, normally configured using the 'http_proxy',
diff --git a/advice.c b/advice.c
index a492eea..d070a05 100644
--- a/advice.c
+++ b/advice.c
@@ -9,6 +9,7 @@ int advice_commit_before_merge = 1;
int advice_resolve_conflict = 1;
int advice_implicit_identity = 1;
int advice_detached_head = 1;
+int advice_typo_prompt = 1;
static struct {
const char *name;
@@ -23,6 +24,7 @@ static struct {
{ resolveconflict, advice_resolve_conflict },
{ implicitidentity, advice_implicit_identity },
{ detachedhead, advice_detached_head },
+ { typoprompt, advice_typo_prompt },
};
void advise(const char *advice, ...)
diff --git a/advice.h b/advice.h
index f3cdbbf..050068d 100644
--- a/advice.h
+++ b/advice.h
@@ -12,6 +12,7 @@ extern int advice_commit_before_merge;
extern int advice_resolve_conflict;
extern int advice_implicit_identity;
extern int advice_detached_head;
+extern int advice_typo_prompt;
int git_default_advice_config(const char *var, const char *value);
void advise(const char *advice, ...);
diff --git a/help.c b/help.c
index ee261f4..4b45e43 100644
--- a/help.c
+++ b/help.c
@@ -7,6 +7,7 @@
#include string-list.h
#include column.h
#include version.h
+#include compat/terminal.h
void add_cmdname(struct cmdnames *cmds, const char *name, int len)
{
@@ -248,12 +249,30 @@ int is_in_cmdlist(struct cmdnames *c, const char *s)
}
static int autocorrect;
+static int shall_advise = 1;
+static int shall_prompt;
+static const char message_advice_prompt_ability[] =
+ N_(I can display an interactive
Re: ninja build
On Fri, Jul 20, 2012 at 11:42 AM, Thiago Farina tfrans...@gmail.com wrote: Is there any interest in building git with ninja? [1]. I think it would be very interesting to move forward from make to ninja. [1] http://martine.github.com/ninja/ While it might be interesting, would it be a productive use of the community's time? So many platforms include standard varieties of MAKE and it is well supported by the autotools framework. Tons of people know how to maintain it. So it may sound cool, but that alone does not make it a good idea. -- -Drew Northup -- As opposed to vegetable or mineral error? -John Pescatore, SANS NewsBites Vol. 12 Num. 59 -- 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 0/3] Incremental updates for da/difftool-updates
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] difftool: Do not remove temporary files on error
David Aguilar dav...@gmail.com writes:
Keep the temporary directory around when either compare() or
the difftool returns a non-zero exit status.
Tell the user about the location of the temporary files so that
they can recover from the failure.
Signed-off-by: David Aguilar dav...@gmail.com
---
git-difftool.perl | 36 ++--
1 file changed, 26 insertions(+), 10 deletions(-)
diff --git a/git-difftool.perl b/git-difftool.perl
index 10d3d97..f4f7d4a 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -18,7 +18,7 @@ use File::Copy;
use File::Compare;
use File::Find;
use File::stat;
-use File::Path qw(mkpath);
+use File::Path qw(mkpath rmtree);
use File::Temp qw(tempdir);
use Getopt::Long qw(:config pass_through);
use Git;
@@ -119,7 +119,7 @@ sub setup_dir_diff
exit(0) if (length($diffrtn) == 0);
# Setup temp directories
- my $tmpdir = tempdir('git-difftool.X', CLEANUP = 1, TMPDIR = 1);
+ my $tmpdir = tempdir('git-difftool.X', CLEANUP = 0, TMPDIR = 1);
my $ldir = $tmpdir/left;
my $rdir = $tmpdir/right;
mkpath($ldir) or die $!;
@@ -257,7 +257,7 @@ sub setup_dir_diff
}
}
- return ($ldir, $rdir, @working_tree);
+ return ($ldir, $rdir, $tmpdir, @working_tree);
}
sub write_to_file
@@ -349,20 +349,23 @@ sub main
sub dir_diff
{
my ($extcmd, $symlinks) = @_;
-
my $rc;
+ my $error = 0;
my $repo = Git-repository();
-
my $workdir = find_worktree($repo);
- my ($a, $b, @worktree) = setup_dir_diff($repo, $workdir, $symlinks);
+ my ($a, $b, $tmpdir, @worktree) =
+ setup_dir_diff($repo, $workdir, $symlinks);
+
if (defined($extcmd)) {
$rc = system($extcmd, $a, $b);
} else {
$ENV{GIT_DIFFTOOL_DIRDIFF} = 'true';
$rc = system('git', 'difftool--helper', $a, $b);
}
-
- exit($rc | ($rc 8)) if ($rc != 0);
+ if ($rc != 0) {
+ dir_diff_tmpdir_warning($tmpdir);
+ exit($rc | ($rc 8));
+ }
Hrm.
What does a non-zero exit code from these compare two trees diff
tools (e.g. diff -r a/ b/) mean? Isn't there are difference
between the two trees the usual meaning? And we treat that as a
failure and make the user clean up after us?
The patch is not making things any worse with respect to that point,
so I'd queue it as-is, but it smells like a fishy design decision to
me.
# If the diff including working copy files and those
# files were modified during the diff, then the changes
@@ -377,16 +380,29 @@ sub dir_diff
if ($diff == 0) {
next;
} elsif ($diff == -1 ) {
- my $errmsg = warning: could not compare ;
+ my $errmsg = warning: Could not compare ;
$errmsg += '$b/$file' with '$workdir/$file'\n;
warn $errmsg;
+ $error = 1;
} elsif ($diff == 1) {
copy($b/$file, $workdir/$file) or die $!;
my $mode = stat($b/$file)-mode;
chmod($mode, $workdir/$file) or die $!;
}
}
- exit(0);
+ if ($error) {
+ dir_diff_tmpdir_warning($tmpdir);
+ } else {
+ rmtree($tmpdir);
+ }
+ exit($error);
+}
+
+sub dir_diff_tmpdir_warning
+{
+ my ($tmpdir) = @_;
+ warn warning: Temporary files exist in '$tmpdir'.\n;
+ warn warning: You may want to cleanup or recover these.\n;
}
sub file_diff
--
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: Teach Makefile.PL to find .pm files on its own
Looks sensible. Will queue. 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 v6 4/7] Remove obsolete LONG_USAGE which breaks xgettext
Jiang Xin worldhello@gmail.com writes: The obsolete LONG_USAGE variable has the following message in it: A'\''--B'\''--C'\'' And such complex LONG_USAGE message will break 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 I've retitled it and swapped the paragraphs around when queuing this. LONG_USAGE and USAGE are not just obsolete but completely unused, so they should be removed, even if xgettext did not have a bug to choke on their contents. 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 v6 5/7] i18n: am: mark more strings for translation
Jiang Xin worldhello@gmail.com writes: 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 lose its 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 Thanks. I've reworded the rationale for -b/--binary, though, as the above sounds as if we are clueless as to how to i18n this message properly even if we wanted to, which is not the case. Mark strings in 'git-am.sh' for translation. In the last chunk, change '$1' to '-b/--binary', as it is not worth turning this message to The %s option has been... and using printf on it. -- 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 v2 2/4] help.c::exclude_cmds: realloc() before copy, plug a leak
Tay Ray Chuan rcta...@gmail.com writes:
Copying with structural assignment may not take into account that the
LHS struct has sufficient memory, especially since the cmdname-name
member is nonfixed in size. Be unambiguous about it by realloc()'ing it
to be of sufficient size.
If the original code were
*(cmd-names[cj++]) = *(cmd-names[ci++]);
there may be a structural assignment involved, but
cmds-names[dst] = cmd-names[src]
just copies the pointer that points at a struct cmdname that records
the src command name to another slot of cmds-names[] array, whose
elements are pointers, no? What's there to realloc?
@@ -58,20 +69,25 @@ void exclude_cmds(struct cmdnames *cmds, struct cmdnames
*excludes)
{
int ci, cj, ei;
int cmp;
+ int last_cj;
ci = cj = ei = 0;
while (ci cmds-cnt ei excludes-cnt) {
cmp = strcmp(cmds-names[ci]-name, excludes-names[ei]-name);
if (cmp 0)
- cmds-names[cj++] = cmds-names[ci++];
+ copy_cmdname(cmds-names[cj++], cmds-names[ci++]);
else if (cmp == 0)
ci++, ei++;
else if (cmp 0)
ei++;
}
+ last_cj = cj;
while (ci cmds-cnt)
- cmds-names[cj++] = cmds-names[ci++];
+ copy_cmdname(cmds-names[cj++], cmds-names[ci++]);
+
+ while (last_cj cmds-cnt)
+ free(cmds-names[last_cj++]);
cmds-cnt = cj;
}
We shifted cmds-names[] array to skip entries that appear in
excludes. If original cmds-names[] had 0, 1, 2, 3, ...
and excludes had 0 and 1, cmds-names[] would contain 2, 3,
2, 3; the first two are copied over 0 and 1 that are
excluded, and the latter two are leftover beyond last_cj. The
corresponding names share the same structure (cmds-names[] is an
array of pointers). Doesn't freeing cmds-names[2] free the
structure that is used by both cmds-names[0] and cmds-names[2]?
Confused.
The function drops cmds-names[ci] when it appears in excludes, so
you may want to free it when it happens, though.
help.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/help.c b/help.c
index 6991492..cae389b 100644
--- a/help.c
+++ b/help.c
@@ -64,9 +64,10 @@ void exclude_cmds(struct cmdnames *cmds, struct cmdnames
*excludes)
cmp = strcmp(cmds-names[ci]-name, excludes-names[ei]-name);
if (cmp 0)
cmds-names[cj++] = cmds-names[ci++];
- else if (cmp == 0)
- ci++, ei++;
- else if (cmp 0)
+ else if (cmp == 0) {
+ ei++;
+ free(cmd-names[ci++]);
+ } else if (cmp 0)
ei++;
}
--
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 v2 3/4] help.c: plug leaks with(out) help.autocorrect
Tay Ray Chuan rcta...@gmail.com writes:
When help.autocorrect is set, in an attempt to retain the memory to the
string name in main_cmds, we unfortunately leaked the struct cmdname
that held it. Fix this by creating a copy of the string name.
If you are updating help_unknown_cmd() so that the caller can free
(and is responsible for freeing if it wants to avoid leaks) the
piece of memory it returns, that change to assumed makes sense.
What we were returning were not freeable.
Butthe caller does not free it; what you did is merely to shift the
leakage from here to its caller.
Is it worth the churn and one extra allocation to still leak
slightly (namely, by sizeof(size_t)) smaller chunk of memory than
what the code currently does? I doubt it.
When help.autocorrect is not set, we exit()'d without free'ing it like
we do when the config is set; fix this.
I don't see any point in doing this, immediately in the same
function on the previous line of exit(1).
Signed-off-by: Tay Ray Chuan rcta...@gmail.com
---
Changed in v2: plug leak when help.autocorrect is not set.
---
help.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/help.c b/help.c
index dfb2e9d..ee261f4 100644
--- a/help.c
+++ b/help.c
@@ -362,8 +362,7 @@ const char *help_unknown_cmd(const char *cmd)
; /* still counting */
}
if (autocorrect n == 1 SIMILAR_ENOUGH(best_similarity)) {
- const char *assumed = main_cmds.names[0]-name;
- main_cmds.names[0] = NULL;
+ const char *assumed = xstrdup(main_cmds.names[0]-name);
clean_cmdnames(main_cmds);
fprintf_ln(stderr,
_(WARNING: You called a Git command named '%s',
@@ -390,6 +389,7 @@ const char *help_unknown_cmd(const char *cmd)
fprintf(stderr, \t%s\n, main_cmds.names[i]-name);
}
+ clean_cmdnames(main_cmds);
exit(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
Re: [PATCH v2 4/4] allow recovery from command name typos
Tay Ray Chuan rcta...@gmail.com writes:
If suggestions are available (based on Levenshtein distance) and if the
terminal isatty(), present a prompt to the user to select one of the
computed suggestions.
The way to determine If the terminal is a tty used in this patch
looks overly dangerous, given that we do not know what kind of git
command we may be invoking at this point.
Perhaps we should audit isatty() calls and replace them with a
helper function that does this kind of thing consistently in a more
robust way (my recent favorite is Linus's somewhat anal logic used
in builtin/merge.c::default_edit_option()).
+static int shall_advise = 1;
+static int shall_prompt;
Naming shall_foo is a first here. It is not wrong per-se, but I
think we tend to call these do we use/perform/etc X do_X in our
codebase (see builtin/{config.c,fetch-pack.c,notes.c} for examples).
--
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] Make 'git submodule update --force' always check out submodules.
Currently, it will only do a checkout if the sha1 registered in the containing repository doesn't match the HEAD of the submodule, regardless of whether the submodule is dirty. As discussed on the mailing list, the '--force' flag is a strong indicator that the state of the submodule is suspect, and should be reset to HEAD. Signed-off-by: Stefan Zager sza...@google.com --- git-submodule.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-submodule.sh b/git-submodule.sh index dba4d39..621eff7 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -575,7 +575,7 @@ Maybe you want to use 'update --init'?) die $(eval_gettext Unable to find current revision in submodule path '\$sm_path') fi - if test $subsha1 != $sha1 + if test $subsha1 != $sha1 -o -n $force then subforce=$force # If we don't already have a -f flag and the submodule has never been checked out -- 1.7.11.rc2 -- 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] difftool: Do not remove temporary files on error
On Wed, Jul 25, 2012 at 9:49 AM, Junio C Hamano gits...@pobox.com wrote:
David Aguilar dav...@gmail.com writes:
Keep the temporary directory around when either compare() or
the difftool returns a non-zero exit status.
Tell the user about the location of the temporary files so that
they can recover from the failure.
Signed-off-by: David Aguilar dav...@gmail.com
---
git-difftool.perl | 36 ++--
1 file changed, 26 insertions(+), 10 deletions(-)
diff --git a/git-difftool.perl b/git-difftool.perl
index 10d3d97..f4f7d4a 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -18,7 +18,7 @@ use File::Copy;
use File::Compare;
use File::Find;
use File::stat;
-use File::Path qw(mkpath);
+use File::Path qw(mkpath rmtree);
use File::Temp qw(tempdir);
use Getopt::Long qw(:config pass_through);
use Git;
@@ -119,7 +119,7 @@ sub setup_dir_diff
exit(0) if (length($diffrtn) == 0);
# Setup temp directories
- my $tmpdir = tempdir('git-difftool.X', CLEANUP = 1, TMPDIR = 1);
+ my $tmpdir = tempdir('git-difftool.X', CLEANUP = 0, TMPDIR = 1);
my $ldir = $tmpdir/left;
my $rdir = $tmpdir/right;
mkpath($ldir) or die $!;
@@ -257,7 +257,7 @@ sub setup_dir_diff
}
}
- return ($ldir, $rdir, @working_tree);
+ return ($ldir, $rdir, $tmpdir, @working_tree);
}
sub write_to_file
@@ -349,20 +349,23 @@ sub main
sub dir_diff
{
my ($extcmd, $symlinks) = @_;
-
my $rc;
+ my $error = 0;
my $repo = Git-repository();
-
my $workdir = find_worktree($repo);
- my ($a, $b, @worktree) = setup_dir_diff($repo, $workdir, $symlinks);
+ my ($a, $b, $tmpdir, @worktree) =
+ setup_dir_diff($repo, $workdir, $symlinks);
+
if (defined($extcmd)) {
$rc = system($extcmd, $a, $b);
} else {
$ENV{GIT_DIFFTOOL_DIRDIFF} = 'true';
$rc = system('git', 'difftool--helper', $a, $b);
}
-
- exit($rc | ($rc 8)) if ($rc != 0);
+ if ($rc != 0) {
+ dir_diff_tmpdir_warning($tmpdir);
+ exit($rc | ($rc 8));
+ }
Hrm.
What does a non-zero exit code from these compare two trees diff
tools (e.g. diff -r a/ b/) mean? Isn't there are difference
between the two trees the usual meaning? And we treat that as a
failure and make the user clean up after us?
The patch is not making things any worse with respect to that point,
so I'd queue it as-is, but it smells like a fishy design decision to
me.
This is true. We do have mergetool.tool.trustExitCode, but I don't
think that's really what we want here.
I'm slightly on the fence about this. The do not cleanup mode was
really added to deal with compare() returning -1 (which should be
extremely rare, if not non-existent in practice), and we'd be making
things worse by leaving junk around for the common case.
I personally think this should be redone so that we leave files around
for the compare() == -1 failure case only.
# If the diff including working copy files and those
# files were modified during the diff, then the changes
@@ -377,16 +380,29 @@ sub dir_diff
if ($diff == 0) {
next;
} elsif ($diff == -1 ) {
- my $errmsg = warning: could not compare ;
+ my $errmsg = warning: Could not compare ;
$errmsg += '$b/$file' with '$workdir/$file'\n;
warn $errmsg;
+ $error = 1;
} elsif ($diff == 1) {
copy($b/$file, $workdir/$file) or die $!;
my $mode = stat($b/$file)-mode;
chmod($mode, $workdir/$file) or die $!;
}
}
- exit(0);
+ if ($error) {
+ dir_diff_tmpdir_warning($tmpdir);
+ } else {
+ rmtree($tmpdir);
+ }
+ exit($error);
+}
+
+sub dir_diff_tmpdir_warning
+{
+ my ($tmpdir) = @_;
+ warn warning: Temporary files exist in '$tmpdir'.\n;
+ warn warning: You may want to cleanup or recover these.\n;
}
sub file_diff
--
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] Make 'git submodule update --force' always check out submodules.
Stefan Zager sza...@google.com writes: Currently, it will only do a checkout if the sha1 registered in the containing repository doesn't match the HEAD of the submodule, regardless of whether the submodule is dirty. As discussed on the mailing list, the '--force' flag is a strong indicator that the state of the submodule is suspect, and should be reset to HEAD. Signed-off-by: Stefan Zager sza...@google.com --- Looks sensible (again -- see http://thread.gmane.org/gmane.comp.version-control.git/197532 for the original discussion). Can submodule folks Ack it? Thanks. git-submodule.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-submodule.sh b/git-submodule.sh index dba4d39..621eff7 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -575,7 +575,7 @@ Maybe you want to use 'update --init'?) die $(eval_gettext Unable to find current revision in submodule path '\$sm_path') fi - if test $subsha1 != $sha1 + if test $subsha1 != $sha1 -o -n $force then subforce=$force # If we don't already have a -f flag and the submodule has never been checked out -- 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
False positive from orphaned_commit_warning() ?
Has anyone else noticed false positives coming from the orphan check? It is warning me about commits that are clearly on master. Here is an example, where I checkout master~2 and then switch back to master. It somehow thinks that master~2 is orphaned, when master~2 is by definition in the commit chain leading to master. The repo is tiny, so anyone can try and reproduce this. (I've done so on v1.7.9 and v1.7.11, on two different machines). git://git.yoctoproject.org/yocto-kernel-tools.git Paul. - paul@foo:~/git/yocto-kernel-tools$ git checkout master~2 Note: checking out 'master~2'. You are in 'detached HEAD' state. You can look around, make experimental changes and commit them, and you can discard any commits you make in this state without impacting any branches by performing another checkout. If you want to create a new branch to retain commits you create, you may do so (now or later) by using -b with the checkout command again. Example: git checkout -b new_branch_name HEAD is now at e693754... kgit-checkpoint: fix verify_branch variable name typo paul@foo:~/git/yocto-kernel-tools$ git checkout master Warning: you are leaving 38 commits behind, not connected to any of your branches: e693754 kgit-checkpoint: fix verify_branch variable name typo ee67a7b kgit-config-cleaner: fix redefintion processing 579b1ba meta: support flexible meta branch naming 4673bdb scc: allow kconf fragment searching ... and 34 more. If you want to keep them by creating a new branch, this may be a good time to do so with: git branch new_branch_name e6937544e030637cec029edee34737846a036ece Switched to branch 'master' paul@foo:~/git/yocto-kernel-tools$ git branch --contains e6937544e030637cec029edee34737846a036ece * master paul@foo:~/git/yocto-kernel-tools$ git --version git version 1.7.11.1 paul@foo:~/git/yocto-kernel-tools$ cat .git/config [core] repositoryformatversion = 0 filemode = true bare = false logallrefupdates = true [remote origin] fetch = +refs/heads/*:refs/remotes/origin/* url = git://git.yoctoproject.org/yocto-kernel-tools.git [branch master] remote = origin merge = refs/heads/master paul@foo:~/git/yocto-kernel-tools$ --- -- 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 3/3] difftool: Disable --symlinks on cygwin
On 07/25/2012 05:14 AM, David Aguilar wrote: 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? Apparently yes: $ uname -rso CYGWIN_NT-5.1 1.5.25(0.156/4/2) Cygwin $ perl -e 'print $^O' cygwin 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: [RFC/PATCH] t3300-*.sh: Fix a TAP parse error
Jonathan Nieder wrote:
[...]
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?
Yes, I will happily create a proper (tested) patch and send it to the list.
However, given that we are now in the RC period, I probably won't get to it
immediately; I need to set aside *at least* one full evening to running the
testsuite on cygwin! ;-)
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
Junio C Hamano wrote:
[...]
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).
Heh, I did exactly this (except mine was called test_fixture) as part
of my first (abandoned) attempt to address this problem. :-D
I've attached the patch below, just for discussion.
I didn't test it very much, but it seems to work with a superficial
workout:
$ cd t
$ ./t3300-funny-names.sh
# passed all 0 test(s)
1..0 # SKIP Your filesystem does not allow tabs in filenames
$
$ ./t3300-funny-names.sh -v
Initialized empty Git repository in /home/ramsay/git/t/trash
directory.t3300-fun
ny-names/.git/
test fixture:
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 $p1 || :; }
{ echo Foo Bar Baz $p2 || :; }
if test -f $p1 cmp $p0 $p1
then
test_set_prereq TABS_IN_FILENAMES
fi
./test-lib.sh: line 274: tabs , (dq) and spaces: No such file or
directory
# 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
filename
s
Files=1, Tests=0, 1 wallclock secs ( 0.03 usr 0.05 sys + 0.87 cusr 0.41
csys
= 1.36 CPU)
Result: NOTESTS
$
So why did I abandon this patch? Well, I didn't really; I just decided
that I needed *much* more time to polish[1] 'test_fixture'. I wanted to
fix the TAP parse error problem before v1.7.12-rc0! :(
HTH
ATB,
Ramsay Jones
[1] For example, what should/will happen if someone uses test_must_fail,
test_might_fail, etc., within the test_fixture script? Should they simply
be banned within a text_fixture?
--- 8 ---
Subject: [PATCH] test-lib-functions.sh: Add a test_fixture function
Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk
---
t/t3300-funny-names.sh | 2 +-
t/test-lib-functions.sh | 11 +++
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/t/t3300-funny-names.sh b/t/t3300-funny-names.sh
index 1f35e55..59331a0 100755
--- a/t/t3300-funny-names.sh
+++ b/t/t3300-funny-names.sh
@@ -15,7 +15,7 @@ p0='no-funny'
p1='tabs , (dq) and spaces'
p2='just space'
-test_expect_success 'setup' '
+test_fixture '
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.
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 80daaca..b70c963 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -302,6 +302,17 @@ test_expect_success () {
echo 3
}
+test_fixture () {
+ test $# = 1 ||
+ error bug in test script: must be single argument to test_fixture
+ say 3 test fixture: $1
+ if ! test_run_ $1
+ then
+ error failure in test_fixture code
+ fi
+ echo 3
+}
+
# test_external runs external test scripts that provide continuous
# test output about their progress, and succeeds/fails on
# zero/non-zero exit code. It outputs the test output on stdout even
--
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 v2] t3300-*.sh: Fix a TAP parse error
Jonathan Nieder wrote: 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. You don't see the point of fixing the TAP Parse error? :-D 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? Hmm, well I guess I'm not going to suggest anything! However, what is anything complicated? At the end of test-lib.sh we find: # test whether the filesystem supports symbolic links ln -s x y 2/dev/null test -h y 2/dev/null test_set_prereq SYMLINKS rm -f y # When the tests are run as root, permission tests will report that # things are writable when they shouldn't be. test -w / || test_set_prereq SANITY Is this too complicated? If not, why not? If yes, should it be added to a test assertion? Would it be acceptable for me to add some code, here at the end of test-lib.sh, to set the TABS_IN_FILENAME test prerequisite and use it in tests t3300-funny-names.sh, t3902-quoted.sh, t3600-rm.sh, t4016-diff-quote.sh and t4135-apply-weird-filenames.sh? How about some of the test library files: diff-lib.sh lib-git-p4.shlib-read-tree.sh gitweb-lib.shlib-git-svn.sh lib-rebase.sh lib-bash.sh lib-gpg.sh* lib-t6000.sh lib-credential.sh* lib-httpd.sh lib-terminal.sh lib-cvs.sh lib-pager.sh test-lib-functions.sh lib-diff-alternative.sh lib-patch-mode.shtest-lib.sh lib-gettext.sh lib-prereq-FILEMODE.sh lib-git-daemon.shlib-read-tree-m-3way.sh Several of these files contain executable code (rather than just a library of functions). For example, look at lib-cvs.sh, lib-httpd.sh, and lib-prereq-FILEMODE.sh. Is this code too complicated? Would it be acceptable for me to create an lib-prereq-TABSINFILE.sh file so that I could source it only in the test files that require the TABS_IN_FILENAME prerequisite? 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: Teach Makefile.PL to find .pm files on its own
On 2012.7.25 9:56 AM, Junio C Hamano wrote: Looks sensible. Will queue. Thanks. Thanks! What's the lag time on it showing up in the repo, and which branch will it appear in? Also I just realized I've been basing my work on master. Should I move to maint? -- If you want the truth to stand clear before you, never be for or against. The struggle between for and against is the mind's worst disease. -- Sent-ts'an -- 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: Teach Makefile.PL to find .pm files on its own
Michael G Schwern schw...@pobox.com writes: What's the lag time on it showing up in the repo, and which branch will it appear in? There is nothing special in this topic, so it is likely to start on 'pu', and unlikely to come to 'master' before 1.7.12 ships sometime next month. Also I just realized I've been basing my work on master. Should I move to maint? I don't think so. It is not fixing any urgent breakage (iow, by being told about .pm explicitly, it knows about them just fine without being taught how to find them). -- 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: False positive from orphaned_commit_warning() ?
On Wed, Jul 25, 2012 at 2:53 PM, Paul Gortmaker paul.gortma...@windriver.com wrote: Has anyone else noticed false positives coming from the orphan check? It is warning me about commits that are clearly on master. Here is an example, where I checkout master~2 and then switch back to master. It somehow thinks that master~2 is orphaned, when master~2 is by definition in the commit chain leading to master. I've been able to reproduce this with the following simplified recipe, although I still don't know what is causing the failure (I'm not very familiar with the code) git init test cd test #make 3 commits touch a git add a git commit -m a touch b git add b git commit -m b touch c git add c git commit -m c #clone it cd .. git clone test test2 cd test2 git checkout master~2 git checkout master #Warning: you are leaving 1 commit behind, not connected to #any of your branches I can't figure out what's going wrong here, but the clone is important; it doesn't fail without it. It appears to have something to do with the fact that the cloned repository has a remote, as: #in test2 git remote rm origin git checkout master~2 git checkout master Does not throw the warning, but it's not just the presence of origin/master that triggers it, as: cd ../test git remote add origin ../test2 git fetch origin git checkout master~2 git checkout master Does not trigger it either. Confused, -- -Dan -- 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] git on Mac OS and precomposed unicode
Torsten Bögershausen skrev 2012-06-24 17.47: Do we have a motivation for pushing a solution that ignores the unicode composition ? I say we do. I tried your patch and it worked fine. I'll send a V5 version with hopefully a better motivation -- robin -- 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: Junio C Hamano wrote: 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. Neat. This could be used for later set-up tests, too, perhaps with a long-term goal of making non set-up tests independent of each other (reorderable and skippable). [...] [1] For example, what should/will happen if someone uses test_must_fail, test_might_fail, etc., within the test_fixture script? Should they simply be banned within a text_fixture? Why wouldn't they act just like they do in test_expect_success blocks? FWIW I find Junio's test_setup name more self-explanatory. What mnemonic should I be using to remember the _fixture name? 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] Make 'git submodule update --force' always check out submodules.
Am 25.07.2012 20:44, schrieb Junio C Hamano: Stefan Zager sza...@google.com writes: Currently, it will only do a checkout if the sha1 registered in the containing repository doesn't match the HEAD of the submodule, regardless of whether the submodule is dirty. As discussed on the mailing list, the '--force' flag is a strong indicator that the state of the submodule is suspect, and should be reset to HEAD. Signed-off-by: Stefan Zager sza...@google.com --- Looks sensible (again -- see http://thread.gmane.org/gmane.comp.version-control.git/197532 for the original discussion). Can submodule folks Ack it? I like it. Still I'd vote for amending the documentation like the original thread proposed and would appreciate to have a test or two, but apart from that I have no objections. git-submodule.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-submodule.sh b/git-submodule.sh index dba4d39..621eff7 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -575,7 +575,7 @@ Maybe you want to use 'update --init'?) die $(eval_gettext Unable to find current revision in submodule path '\$sm_path') fi -if test $subsha1 != $sha1 +if test $subsha1 != $sha1 -o -n $force then subforce=$force # If we don't already have a -f flag and the submodule has never been checked out -- 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 -- 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 3/3] The Makefile.PL will now find .pm files itself.
Hi, Michael G. Schwern wrote: It is no longer necessary to manually add new .pm files to the Makefile.PL. This makes it easier to add modules. Thanks! Sorry I missed this. [...] --- 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 In a previous apartment I lived in, there was a note taped to the lightswitch reminding us to turn off the heat, take keys with us, and lock the door. The note was useful because by force of habit we would be turning off the light, and as a result see the note, on the way out. Who are these comments in perl/Makefile.PL addressed to? Why would such a person be looking at perl/Makefile.PL? Sorry to sound like a broken record, but I don't think these questions were answered yet. How about this patch for squashing in, which would avoid the question and save me from having to worry that my words are going to stay in this file after the no-makemaker option no longer exists because nobody looks at them here? diff --git i/perl/Makefile.PL w/perl/Makefile.PL index 3d88a6b9..377fd042 100644 --- i/perl/Makefile.PL +++ w/perl/Makefile.PL @@ -4,9 +4,6 @@ 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 /; -- 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: Move Git::SVN into its own .pm file
Hi, Michael G. Schwern wrote: This is a refactoring to move Git::SVN out of git-svn and into its own .pm file. This will make it easier to work with and test. This is just the extraction with minimal work to keep all tests passing. Patch 3 doesn't seem to have hit the list archive. I am not subscribed and was not cc-ed for some reason, so I do not have it. I'd appreciate a copy if that's not too inconvenient. 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 1/4] Extract some utilities from git-svn to allow extracting Git::SVN.
Michael G. Schwern schw...@pobox.com wrote:
From: Michael G. Schwern schw...@pobox.com
Put them in a new module called Git::SVN::Utils. Yeah, not terribly
original and it will be a dumping ground. But its better than having
them in the main git-svn program. At least they can be documented
and tested.
* fatal() is used by many classes.
* Change the $can_compress lexical into a function.
This should be enough to extract Git::SVN.
Please keep Jonathan Cc:-ed, he's been very helpful with this series
(and very helpful in general :)
This series is mostly alright by me, a few minor comments inline.
--- /dev/null
+++ b/t/Git-SVN/00compile.t
+
+use Test::More tests = 1;
+++ b/t/Git-SVN/Utils/fatal.t
@@ -0,0 +1,34 @@
+
+use Test::More 'no_plan';
Didn't we agree to use done_testing()? Perhaps (as you suggested) with
a private copy of Test::More? It's probably easier to start using
done_testing() earlier rather than later.
+BEGIN {
+# Override exit at BEGIN time before Git::SVN::Utils is loaded
+# so it will see our local exit later.
+*CORE::GLOBAL::exit = sub(;$) {
+return @_ ? CORE::exit($_[0]) : CORE::exit();
+};
+}
For new code related to git-svn, please match the existing indentation
style (tabs) prevalent in git-svn. Most of the Perl found in git also
uses tabs for indentation.
--
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 3/3] The Makefile.PL will now find .pm files itself.
Jonathan Nieder wrote: Michael G. Schwern wrote: --- 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 [...] Who are these comments in perl/Makefile.PL addressed to? Why would such a person be looking at perl/Makefile.PL? Sorry to sound like a broken record, but I don't think these questions were answered yet. To maybe answer my own question: are these comments addressed to people making other changes to perl/Makefile.PL, rather than people adding new modules? That could make sense --- it would just be a change in purpose from the original comments. It also means there's no reminder when adding new modules to list them in perl/Makefile any more, but that's probably inevitable as long as we don't have a perl coding style document. 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: False positive from orphaned_commit_warning() ?
Paul Gortmaker paul.gortma...@windriver.com writes:
Has anyone else noticed false positives coming from the
orphan check?
Thanks. This should fix it.
builtin/checkout.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 6acca75..d812219 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -606,7 +606,7 @@ static int add_pending_uninteresting_ref(const char
*refname,
const unsigned char *sha1,
int flags, void *cb_data)
{
- add_pending_sha1(cb_data, refname, sha1, flags | UNINTERESTING);
+ add_pending_sha1(cb_data, refname, sha1, UNINTERESTING);
return 0;
}
--
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: Move Git::SVN into its own .pm file
On 2012.7.25 2:15 PM, Jonathan Nieder wrote: Michael G. Schwern wrote: This is a refactoring to move Git::SVN out of git-svn and into its own .pm file. This will make it easier to work with and test. This is just the extraction with minimal work to keep all tests passing. Patch 3 doesn't seem to have hit the list archive. I am not subscribed and was not cc-ed for some reason, so I do not have it. I'd appreciate a copy if that's not too inconvenient. The patch was 136k. I'm going to guess there's some old 50k filter somewhere that blocked it. insert ironic statement about email based development here The patch is attached. -- 87. If the thought of something makes me giggle for longer than 15 seconds, I am to assume that I am not allowed to do it. -- The 213 Things Skippy Is No Longer Allowed To Do In The U.S. Army http://skippyslist.com/list/ 0001-Extract-Git-SVN-from-git-svn-into-its-own-.pm-file.patch.gz Description: application/tar-gz
Re: [RFC/PATCH] t3300-*.sh: Fix a TAP parse error
Jonathan Nieder jrnie...@gmail.com writes: FWIW I find Junio's test_setup name more self-explanatory. What mnemonic should I be using to remember the _fixture name? Previous exposure to things like Rails? -- 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] Make 'git submodule update --force' always check out submodules.
Jens Lehmann jens.lehm...@web.de writes: Am 25.07.2012 20:44, schrieb Junio C Hamano: Stefan Zager sza...@google.com writes: Currently, it will only do a checkout if the sha1 registered in the containing repository doesn't match the HEAD of the submodule, regardless of whether the submodule is dirty. As discussed on the mailing list, the '--force' flag is a strong indicator that the state of the submodule is suspect, and should be reset to HEAD. Signed-off-by: Stefan Zager sza...@google.com --- Looks sensible (again -- see http://thread.gmane.org/gmane.comp.version-control.git/197532 for the original discussion). Can submodule folks Ack it? I like it. Still I'd vote for amending the documentation like the original thread proposed and would appreciate to have a test or two, but apart from that I have no objections. OK, then I'll queue this so that we won't forget about the topic for now, and docs and tests can be done as follow-up patches to the topic. 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: Teach Makefile.PL to find .pm files on its own
Michael G Schwern schw...@pobox.com writes: Also I just realized I've been basing my work on master. Should I move to maint? I don't think so. It is not fixing any urgent breakage (iow, by being told about .pm explicitly, it knows about them just fine without being taught how to find them). How about the git-svn SVN 1.7 fix in general? All of these patch sets I'm sending build on one another, is that going to be a problem? It's going to come in about six parts. Judging from the rate of the discussion this is progressing, I was imagining that this series would be ready by 1.7.13 at the earliest, possibly back-merged to 1.7.12.X maintenance series, and 1.7.11.X maintenance series is no longer relevant by then. But I certainly do not mind seeing the series based on earlier maintenance releases, e.g. maint-1.7.9. There however are tons of other git-svn.perl and perl/ updates since then, so basing the series on the current maint branch to abandon 1.7.10.X and earlier but still leaving the door open to downmerge to 1.7.11.X may be a good trade-off. -- 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: Teach Makefile.PL to find .pm files on its own
On 2012.7.25 3:19 PM, Junio C Hamano wrote: Michael G Schwern schw...@pobox.com writes: How about the git-svn SVN 1.7 fix in general? All of these patch sets I'm sending build on one another, is that going to be a problem? It's going to come in about six parts. Judging from the rate of the discussion this is progressing, I was imagining that this series would be ready by 1.7.13 at the earliest, possibly back-merged to 1.7.12.X maintenance series, and 1.7.11.X maintenance series is no longer relevant by then. But I certainly do not mind seeing the series based on earlier maintenance releases, e.g. maint-1.7.9. There however are tons of other git-svn.perl and perl/ updates since then, so basing the series on the current maint branch to abandon 1.7.10.X and earlier but still leaving the door open to downmerge to 1.7.11.X may be a good trade-off. So... is that master or maint? Just let me know which one. -- 91. I am not authorized to initiate Jihad. -- 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/4] Extract some utilities from git-svn to allow extracting Git::SVN.
Michael G Schwern schw...@pobox.com wrote:
On 2012.7.25 2:24 PM, Eric Wong wrote:
Didn't we agree to use done_testing()? Perhaps (as you suggested) with
a private copy of Test::More? It's probably easier to start using
done_testing() earlier rather than later.
Yes, we agreed done_testing is the way forward. Given how much work I've had
to do to get even basic patches in I decided to ditch anything extra. That
includes adding a t/lib and I didn't want to make it silently depend on an
upgraded Test::More either.
OK.
+BEGIN {
+# Override exit at BEGIN time before Git::SVN::Utils is loaded
+# so it will see our local exit later.
+*CORE::GLOBAL::exit = sub(;$) {
+return @_ ? CORE::exit($_[0]) : CORE::exit();
+};
+}
For new code related to git-svn, please match the existing indentation
style (tabs) prevalent in git-svn. Most of the Perl found in git also
uses tabs for indentation.
About that. I followed kernel style in existing code, but felt that new code
would do better to follow Perl style. The existing Perl code mixes tabs and
spaces, so I felt it wasn't a strongly held style. You'll get more Perl
programmers to work on the Perl code by following Perl style in the Perl code
rather than kernel style.
git-svn should be all tabs (though some spaces accidentally slipped in
over the years). git maintainers are mostly C programmers used to tabs,
so non-C code should favor their expectations.
I also favor tabs since they're more space-efficient and leads to faster
git grep on large source trees (more important for bigger projects).
I remember many years ago git grep was shown to be ~20% faster on
a source tree with tabs.
(I'm neither a kernel hacker nor a regular Perl hacker)
Alternatively, how about allowing emacs/vim configuration comments? The
Kernel coding style doesn't allow them, how do you folks feel? Then people
don't have to guess the style and reconfigure their editor, their editor will
do it for them.
I don't like them, and I think others here frowns upon them, too. They
take too much space and shows favor/preference towards certain editors.
It'll be a bigger problem if more editors become popular and we start
supporting them.
The important thing is to have one less special thing a new-to-your-project
Perl programmer has to do.
Historically git development has catered to C programmers used to tabs.
I think the mixing of indentation styles in current Perl files is the
most confusing.
--
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: Teach Makefile.PL to find .pm files on its own
Junio C Hamano gits...@pobox.com wrote: Michael G Schwern schw...@pobox.com writes: So... is that master or maint? Just let me know which one. I do not care too deeply either way, and in the end I think Eric should have the final say. Given that git://git.bogomips.org/git-svn.git/ has 'master' but nothing to build on 'maint', I would imagine that basing on master is just fine. Yes, master is fine. -- 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: OT: mail-based interfaces and web-based interfaces (Re: Extract
Michael G Schwern schw...@pobox.com wrote: On 2012.7.24 7:55 PM, Eric Wong wrote: 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). 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 agree commonality is important. But to me, it's not worth the cost of reliance on centralized, single-vendor solutions. My distrust and dissatisfaction with centralized/single-vendor solutions is the reason I'm involved with Free Software (and decentralized version control) in the first place. 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. Of course, I'd be in favor of shifting development to something less centralized than a mailing list. OStatus may become a good choice one day, but the adoption/tools just aren't there right now. 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. Sure, every project/culture has different norms. It's no big deal, learn them, or avoid them. Projects all have different coding styles, different conventions for writing commit messages, changelogs and so on... I live with that and I'll do my best to adjust my style/editor settings/grammar/vocabulary accordingly for each project I contribute to. I won't send patches to mailing lists of projects which prefer patches/pull-requests on their web tracker, either. If the project is important enough to me, I'll (grudgingly) use whatever tools/formats the maintainers prefer. 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. It's documented in the gitworkflows(7) manpage. They should probably be linked somehow. 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! I live in my mailer/$EDITOR/shell and I'm happy with that. The last thing I want is more cookies, non-Free JavaScript code, logins, user tracking/profiling, memory/CPU usage. I doubt either of us will get what we want, though : 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. We need to use something. Right now our choice of mailer is the best choice for _existing_ contributors. 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 :) You want to know what's creepy? That you'd think people work like that. It doesn't work out that way. People have far better things to do than stalk your Github commits to see how you're spending your time. You're just not that interesting. Neither am I! (If I really wanted to I could just compile your activity from public list archives and repositories, so you're really not broadcasting any less information about yourself by staying off Github. But I wouldn't want to, because you're just not that interesting and I have better things to do with my time!) There's a big difference between making information easy-to-access vs. merely possible-to-access. For the most part, people don't
Re: [PATCH 1/4] Extract some utilities from git-svn to allow extracting Git::SVN.
This is rapidly getting into the weeds. Regardless of the debate below, what
would you like me to do? Switch indentation to tabs and resubmit? AFAIK only
the new tests are affected.
On 2012.7.25 4:08 PM, Eric Wong wrote:
+BEGIN {
+# Override exit at BEGIN time before Git::SVN::Utils is loaded
+# so it will see our local exit later.
+*CORE::GLOBAL::exit = sub(;$) {
+return @_ ? CORE::exit($_[0]) : CORE::exit();
+};
+}
For new code related to git-svn, please match the existing indentation
style (tabs) prevalent in git-svn. Most of the Perl found in git also
uses tabs for indentation.
About that. I followed kernel style in existing code, but felt that new code
would do better to follow Perl style. The existing Perl code mixes tabs and
spaces, so I felt it wasn't a strongly held style. You'll get more Perl
programmers to work on the Perl code by following Perl style in the Perl code
rather than kernel style.
git-svn should be all tabs (though some spaces accidentally slipped in
over the years). git maintainers are mostly C programmers used to tabs,
so non-C code should favor their expectations.
Perhaps this is self fulfilling. Would you like to attract more Perl
programmers?
I also favor tabs since they're more space-efficient and leads to faster
git grep on large source trees (more important for bigger projects).
I remember many years ago git grep was shown to be ~20% faster on
a source tree with tabs.
Storage costs a dime a gig. One extra music file negates your space savings.
I have more CPU power on my phone than I had on my desktop many years ago.
It doesn't matter if tabs make git-grep 20% faster because it's going to be
200ms vs 240ms.
Regardless, you don't choose your coding style because it's easier for the
computer. You choose it because it makes the code easier for humans to work
with.
(I'm neither a kernel hacker nor a regular Perl hacker)
Alternatively, how about allowing emacs/vim configuration comments? The
Kernel coding style doesn't allow them, how do you folks feel? Then people
don't have to guess the style and reconfigure their editor, their editor will
do it for them.
I don't like them, and I think others here frowns upon them, too. They
take too much space and shows favor/preference towards certain editors.
It'll be a bigger problem if more editors become popular and we start
supporting them.
What you say above is correct, but the extra space is not wasted. It would be
like complaining about all the space that Documentation takes up, and that it
shows preference towards English. Its *useful* to somebody not already using
your style. It's useful for new-to-your-project folks. It's also useful for
somebody switching between the C and Perl code (though a good editor should
already be set up to do C and Perl differently).
Are the editor wars still going on here? Is somebody going to be *offended*
if their editor isn't represented? If so, shouldn't they grow up?
The important thing is to have one less special thing a new-to-your-project
Perl programmer has to do.
Historically git development has catered to C programmers used to tabs.
I think the mixing of indentation styles in current Perl files is the
most confusing.
I agree that mixing the style within the Perl code isn't good. Perl code can
very quickly be reformatted. A basic perltidy config can help keep it that way.
--
Alligator sandwich, and make it snappy!
--
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/4] Extract some utilities from git-svn to allow extracting Git::SVN.
Michael G Schwern wrote: This is rapidly getting into the weeds. Regardless of the debate below, what would you like me to do? Switch indentation to tabs and resubmit? AFAIK only the new tests are affected. Yes, please do so at some point before the final submission. (If there's more feedback coming for this generation of the patches, no need to resubmit right away unless you want to. In general, when getting review a useful strategy can be to discuss proposed changes and batch them until the discussion seems to have died down and only then resubmit the next version.) If we want to switch indentation styles (not a traditional way to start working with an existing project, but whatever), that would be a separate change, affecting all the code consistently. 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: OT: mail-based interfaces and web-based interfaces (Re: Extract
Michael G Schwern schw...@pobox.com wrote: On 2012.7.25 4:48 PM, Eric Wong wrote: We need to use something. Right now our choice of mailer is the best choice for _existing_ contributors. I believe this entire discussion can be reduced to that right there. If your process is optimized for existing contributors, it will work well for existing contributors, who will want to optimize it for themselves. Repeat. If the main way you evaluate your process is asking is this more convenient for me then you're probably in that spiral. This creates a process very well tuned to the existing contributors, and its very convenient for them. But the consequence is it becomes more and more work for a new contributor to join. The process is _not_ a lot of work. At least no more than any other project: observe the regulars - imitate the regulars Many/most regular git contributors are not Linux kernel developers, yet were able to quickly able to get up-to-speed with git. AFAIK, the Linux kernel gets plenty of new contributors every year, too. Before talking about anything else, the existing contributors have to ask themselves a simple question: Do we care about getting new contributors? Yes, if contributors are willing to learn/respect existing conventions. We do take time to help new contributors out :) For me, it's certainly no if there's any endorsement of non-Free Software or centralized/commercial services involved. The answer can be no (yes, but not if I'm inconvenienced is a no). Maybe you're happy with the people you've got. But there's no point in getting into detail until that's settled. -- 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
