[PATCH] t0070: Use precondition CANNOTWRITE
POSIX like file systems allows to create a file when the user has write permissions to the directory. Filesystems like VFAT or NTFS allow to create files regardless of the write permissions of the directory. Therefore "mktemp to unwritable directory" in t0700 will always fail on Windows using NTFS. This TC has been disabled for MINGW, and needs to be disabled for CYGWIN. Use the precondition CANNOTWRITE which is probing the file system and works for MINGW, CYGWIN and even for Linux using VFAT. Signed-off-by: Torsten Bögershausen --- t/t0070-fundamental.sh | 19 --- t/test-lib.sh | 12 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/t/t0070-fundamental.sh b/t/t0070-fundamental.sh index da2c504..a907445 100755 --- a/t/t0070-fundamental.sh +++ b/t/t0070-fundamental.sh @@ -17,13 +17,18 @@ test_expect_success 'mktemp to nonexistent directory prints filename' ' grep "doesnotexist/test" err ' -test_expect_success POSIXPERM 'mktemp to unwritable directory prints filename' ' - mkdir cannotwrite && - chmod -w cannotwrite && - test_when_finished "chmod +w cannotwrite" && - test_must_fail test-mktemp cannotwrite/testXX 2>err && - grep "cannotwrite/test" err -' +if test_have_prereq CANNOTWRITE +then + test_expect_success 'mktemp to unwritable directory prints filename' ' + mkdir cannotwrite && + chmod -w cannotwrite && + test_when_finished "chmod +w cannotwrite" && + test_must_fail test-mktemp cannotwrite/testXX 2>err && + grep "cannotwrite/test" err + ' +else + say "Skipping mktemp to unwritable directory prints filename" +fi test_expect_success 'check for a bug in the regex routines' ' # if this test fails, re-build git with NO_REGEX=1 diff --git a/t/test-lib.sh b/t/test-lib.sh index ca6bdef..1342630 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -770,6 +770,18 @@ test_lazy_prereq AUTOIDENT ' git var GIT_AUTHOR_IDENT ' +test_lazy_prereq CANNOTWRITE ' + chmod -w . + >e || : + chmod +w . + case "$(echo *)" in + e) + false ;; + *) + true ;; + esac +' + # 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 -- 1.8.2.411.g65a544e -- 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/2] Move sequencer
Felipe Contreras wrote: > sequencer.c => builtin/sequencer.c | 160 +--- > sequencer.h => builtin/sequencer.h | 4 - Why exactly? The plan was to unify continuation semantics, and get all the continuation-commands to use the sequencer. That clearly hasn't materialized, but I don't know what this move buys us. -- 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] tests: fix autostash
Felipe Contreras wrote:
> diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh
> index a5e69f3..ff370a3 100755
> --- a/t/t3420-rebase-autostash.sh
> +++ b/t/t3420-rebase-autostash.sh
> @@ -71,8 +71,7 @@ testrebase() {
> test_must_fail git rebase$type related-onto-branch &&
> test_path_is_file $dotest/autostash &&
> ! grep dirty file3 &&
> - rm -rf $dotest &&
> - git reset --hard &&
> + git rebase --abort &&
> git checkout feature-branch
> '
Incorrect. I don't assume that --abort works yet, in this test. It
is non-trivial to get --abort to work with autostash, and this is
tested in a separate test: see "rebase$type: --abort" in the same
file.
--
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 02/22] git-remote-mediawiki: Use the Readonly module instead of the constant pragma
On Fri, Jun 07, 2013 at 11:42:03PM +0200, Célestin Matte wrote: > diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl > b/contrib/mw-to-git/git-remote-mediawiki.perl > index 4893442..e60793a 100755 > --- a/contrib/mw-to-git/git-remote-mediawiki.perl > +++ b/contrib/mw-to-git/git-remote-mediawiki.perl > @@ -26,21 +26,21 @@ use IPC::Open2; > use Readonly; What does this series apply on top of? The existing version in "master" does not have "use Readonly" in it at all. The first version of your series introduced that line, but here it is shown as an existing line. Did you miss a commit when putting your patches together? > # Mediawiki filenames can contain forward slashes. This variable decides by > which pattern they should be replaced > -use constant SLASH_REPLACEMENT => "%2F"; > +Readonly my $SLASH_REPLACEMENT => "%2F"; What advantage does this have over "use constant"? I do not mind following guidelines from perlcritic if they are a matter of style, but in this case there is a cost: we now depend on the "Readonly" module, which is not part of the standard distribution. I.e., users now have to deal with installing an extra dependency. Is it worth it? -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] test: improve rebase -q test
On Sat, Jun 8, 2013 at 3:32 AM, Felipe Contreras
wrote:
> Let's show the output so it's clear why it failed.
I think you can always look into trash-directory.t3400 and figure why.
But if you show this, I think you should use test_cmp to make it clear
what is not wanted. Something like
: >expected &&
test_cmp expected output.out
>
> Signed-off-by: Felipe Contreras
> ---
> t/t3400-rebase.sh | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
> index b58fa1a..fb39531 100755
> --- a/t/t3400-rebase.sh
> +++ b/t/t3400-rebase.sh
> @@ -185,6 +185,7 @@ test_expect_success 'default to @{upstream} when upstream
> arg is missing' '
> test_expect_success 'rebase -q is quiet' '
> git checkout -b quiet topic &&
> git rebase -q master >output.out 2>&1 &&
> + cat output.out &&
> test ! -s output.out
> '
>
> --
> 1.8.3.698.g079b096
>
> --
> 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
--
Duy
--
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 2/2] Move sequencer to builtin
On Sat, Jun 8, 2013 at 5:16 AM, Felipe Contreras wrote: > This code is only useful for cherry-pick and revert built-ins, nothing > else, so let's make it a builtin object, but make sure 'git-sequencer' > is not generated. As you can see, the convention is builtin/foo.c corresponds to git-foo (and maybe more). Why make an exception for sequencer? What do we gain from this? A lot of code in libgit.a is only used by builtin commands, e.g. fetch-pack.c, should we move it to? I ask because I moved fetch-pack from builtin out because of linking issues and I don't want the same happen to sequencer.c. -- Duy -- 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: [Administrivia] On ruby and contrib/
On Sat, Jun 8, 2013 at 3:24 AM, Felipe Contreras wrote: >> The reviewer pool for code written in a new language _must_ be >> seeded by some from the current set of reviewers whose judgement >> I/we can trust. > > By that standard nothing will ever change. Ever. > > Even twenty years from now, you will still only trust people that are > familiar with shell, Perl, and C. Because the only way to gain your > trust, is by being proficient in shell, Perl, and C. I don't see why a trusted person cannot learn a new language and convince the community to give it a try (well given that enough reviewers support the new language, which was Junio's point). -- Duy -- 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: [Administrivia] On ruby and contrib/
On Thu, Jun 6, 2013 at 11:22 PM, Johannes Schindelin wrote: > Hi Greg, > > On Thu, 6 Jun 2013, Greg Troxel wrote: > >> As one of the people who helps maintain git packages in pkgsrc, my >> initial reaction is negative to adding a ruby dependency. > > My initial reaction, too. It was hard enough to get Perl included with Git > for Windows (because of that pesky Subversion dependency). > > As you can see from the commit history, I was the primary force behind > trying to get everything "core" in Git away from requiring scripting > languages (I think it is an awesome thing to provide APIs for as many > languages as possible, but a not-so-cool thing to use more than one > language in the core code). It does not seem that anybody picked up that > task when I left, though. Nobody seems to mention it yet. There's another reason behind the C rewrite effort: fork is costly on Windows. The C rewrite allows us to run with one process (most of the time). This applies for shell, perl and even ruby scripts because libgit.a is never meant to be used outside git.c context (unlike libgit2). In this regard, ruby is just as bad as currently supported non-C languages. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug: `gitsubmodule` does not list modules with unicode characters
On Mon, Mar 25, 2013 at 09:30:44AM +0100, Jens Lehmann wrote:
> Am 23.03.2013 17:28, schrieb Ilya Kulakov:
> > The `git submodule` commands seem to ignore modules which paths contain
> > unicode characters.
> >
> > Consider the following steps to reproduce the problem:
> >
> > 1. Create a directory with name that contains at least one unicode
> > character
> > (e.g. "ûñïçödé-rèpø")
> >
> > 2. Initialize git repository within this directory
> >
> > 3. Add this repository as a submodule to another repository so that
> > unicode characters will appear in the path to the module
> > (e.g. "../ûñïçödé-rèpø")
> >
> > 4. Check that .gitmodules file is updated and contains record
> > about just added module
> >
> > 5. List submodules with using `git submodule` and find out
> > that just added module is not listed
>
> Thanks for your report. It is known that git submodule does not behave
> very well when path names contain special characters. I'll look into
> that when I find some time to see if we can easily fix your problem.
I've looked into this a bit.
git ls-files will return all filenames "c-style quoted". Hence the
filename åäö will be returned as "303245303244303266". This is of course
also wrong as it should be "\303\245\303\244\303\266".
However, if we tell git ls-files to use \0 instead of \n for line
termination. We get åäö returned. So how can the choose of line termination
effect the encoding?
Look in quote.c. The following patch will solve this particular problem
(but break other usecases!)
diff --git a/quote.c b/quote.c
index 911229f..2870ca5 100644
--- a/quote.c
+++ b/quote.c
@@ -284,7 +284,7 @@ void quote_two_c_style(struct strbuf *sb, const char
*prefix, const char *path,
void write_name_quoted(const char *name, FILE *fp, int terminator)
{
if (terminator) {
- quote_c_style(name, NULL, fp, 0);
+ fputs(name, fp);
} else {
fputs(name, fp);
}
Why don't we always print names quoted? IMHO the choose of line
termination should not do anything else than alter the line termination.
However, an other solution would be to use git ls-files -z in
git-submodule.sh and then rewrite the perl-code to handle \0 instead
of \n.
(The same perl-code I wanted to throw away 13 months ago but
Junio wanted to keep because perl can handle \0 and eventually -z should
be used according to him. He was right.)
However, a shortcut would be to the patch below. It will work as long as
there's no newline in the filename (is that really something we want to
support? If not, let's throw away perl and stick with the sed solution
below).
diff --git a/git-submodule.sh b/git-submodule.sh
index 79bfaac..31524d3 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -113,9 +113,10 @@ resolve_relative_url ()
module_list()
{
(
- git ls-files --error-unmatch --stage -- "$@" ||
+ git ls-files --error-unmatch --stage -z -- "$@" ||
echo "unmatched pathspec exists"
) |
+ sed -e 's/\x00/\n/g' |
perl -e '
my %unmerged = ();
my ($null_sha1) = ("0" x 40);
--
Med vänliga hälsningar
Fredrik Gustafsson
tel: 0733-608274
e-post: iv...@iveqy.com
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Git exile Issues
Hey Git Experts, I need your advice. I have lot of png/jpg images in my codebase (which is currently under git) which causes the repo size to be very heavy. We have migrated these images to a storage server using git exile technique. This has been working fine so far (with some glitches) on unix platform. However to make it work on windows has been a big pain. I got it work to some extent that I can pull stuff from storage and replace the references here but its not complete. Also it made the other commands like git status to be very slow. Does anybody have done this before? If so can you please share your experience on it? I tried using Cygwin but my Cygwin installation on windows 7 is not going fine. Having failure in running the post install rebase script. I am unable to checkout a package in Cygwin and I suspect it is because of the post install stuff failure. Any help/hints will be greatly appreciated. -- Thanks & Regards Sudhir Kumar -- 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: Respond Quickly
I am Mrs.Flora Yu Yeung, a staff of Lloyd's TSB Group Plc. here in Hong Kong; I have a secured business proposal for you. If you are interested please reach me on my email: (mrs.floraye...@live.com) for more details. -- 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 17/22] git-remote-mediawiki: Brace file handles for print for more clarity
On Fri, Jun 7, 2013 at 5:42 PM, Célestin Matte
wrote:
> This follows the following rule:
> InputOutput::RequireBracedFileHandleWithPrint (Severity: 1)
> The `print' and `printf' functions have a unique syntax that supports an
> optional file handle argument. Conway suggests wrapping this argument in
> braces to make it visually stand out from the other arguments. When you
> put braces around any of the special package-level file handles like
> `STDOUT', `STDERR', and `DATA', you must the `'*'' sigil or else it
> won't compile under `use strict 'subs''.
>
> print $FH "Mary had a little lamb\n"; #not ok
> print {$FH} "Mary had a little lamb\n"; #ok
>
> print STDERR $foo, $bar, $baz; #not ok
> print {STDERR} $foo, $bar, $baz; #won't compile under 'strict'
> print {*STDERR} $foo, $bar, $baz; #perfect!
>
> Signed-off-by: Célestin Matte
> Signed-off-by: Matthieu Moy
>
> Conflicts:
>
> contrib/mw-to-git/git-remote-mediawiki.perl
Uninteresting conflict information can be dropped.
--
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 16/22] git-remote-mediawiki: Modify strings for a better coding-style
On Fri, Jun 7, 2013 at 5:42 PM, Célestin Matte
wrote:
> - strings which don't need interpolation are single-quoted for more clarity
> and
> slight gain of performance
> - interpolation is preferred over concatenation in many cases, for more
> clarity
> - variables are always used with the ${} operator inside strings
> - strings including double-quotes are written with qq() so that the quotes do
> not have to be escaped
Distinct changes could (IMHO) be split into separate patches for easier review.
> Signed-off-by: Célestin Matte
> Signed-off-by: Matthieu Moy
>
> Conflicts:
>
> contrib/mw-to-git/git-remote-mediawiki.perl
Conflict information is not interesting to reviewers or even in final
commit messages since (hopefully) you will have resolved conflicts
before committing. They can be stripped.
More below.
> ---
> contrib/mw-to-git/git-remote-mediawiki.perl | 244
> +--
> 1 file changed, 121 insertions(+), 123 deletions(-)
>
> diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
> b/contrib/mw-to-git/git-remote-mediawiki.perl
> index f37488b..2d4ea1d 100755
> --- a/contrib/mw-to-git/git-remote-mediawiki.perl
> +++ b/contrib/mw-to-git/git-remote-mediawiki.perl
> @@ -199,10 +199,10 @@ sub mw_connect_maybe {
>lgdomain => $wiki_domain};
> if ($mediawiki->login($request)) {
> Git::credential \%credential, 'approve';
> - print STDERR "Logged in mediawiki user
> \"$credential{username}\".\n";
> + print STDERR qq(Logged in mediawiki user
> "$credential{username}".\n);
> } else {
> - print STDERR "Failed to log in mediawiki user
> \"$credential{username}\" on $url\n";
> - print STDERR " (error " .
> + print {*STDERR} qq(Failed to log in mediawiki user
> "$credential{username}" on ${url}\n);
> + print {*STDERR} ' (error ' .
This change from patch 17/22 (adding braces around file handles)
sneaked in early with this patch (16/22).
> $mediawiki->{error}->{code} . ': ' .
> $mediawiki->{error}->{details} . ")\n";
> Git::credential \%credential, 'reject';
> @@ -473,27 +473,27 @@ sub download_mw_mediafile {
> if ($response->code == 200) {
> return $response->decoded_content;
> } else {
> - print STDERR "Error downloading mediafile from :\n";
> - print STDERR "URL: $download_url\n";
> - print STDERR "Server response: " . $response->code . " " .
> $response->message . "\n";
> + print {*STDERR} "Error downloading mediafile from :\n";
> + print {*STDERR} "URL: ${download_url}\n";
> + print {*STDERR} 'Server response: ' . $response->code . q{ }
> . $response->message . "\n";
Ditto: Sneak in from 17/22.
> exit 1;
> }
> }
>
> sub get_last_local_revision {
> # Get note regarding last mediawiki revision
> - my $note = run_git("notes --ref=$remotename/mediawiki show
> refs/mediawiki/$remotename/master 2>/dev/null");
> + my $note = run_git("notes --ref=${remotename}/mediawiki show
> refs/mediawiki/${remotename}/master 2>/dev/null");
> my @note_info = split(/ /, $note);
>
> my $lastrevision_number;
> - if (!(defined($note_info[0]) && $note_info[0] eq
> "mediawiki_revision:")) {
> - print STDERR "No previous mediawiki revision found";
> + if (!(defined($note_info[0]) && $note_info[0] eq
> 'mediawiki_revision:')) {
> + print STDERR 'No previous mediawiki revision found';
> $lastrevision_number = 0;
> } else {
> # Notes are formatted : mediawiki_revision: #number
> $lastrevision_number = $note_info[1];
> chomp($lastrevision_number);
> - print STDERR "Last local mediawiki revision found is
> $lastrevision_number";
> + print STDERR "Last local mediawiki revision found is
> ${lastrevision_number}";
> }
> return $lastrevision_number;
> }
> @@ -690,8 +690,7 @@ sub fetch_mw_revisions {
> my $n = 1;
> foreach my $page (@pages) {
> my $id = $page->{pageid};
> -
> - print STDERR "page $n/", scalar(@pages), ": ". $page->{title}
> ."\n";
> + print {*STDERR} "page ${n}/", scalar(@pages), ': ',
> $page->{title}, "\n";
Again 17/22.
> $n++;
> my @page_revs = fetch_mw_revisions_for_page($page, $id,
> $fetch_from);
> @revisions = (@page_revs, @revisions);
> @@ -705,7 +704,7 @@ sub fe_escape_path {
> $path =~ s/\\//g;
> $path =~ s/"/\\"/g;
> $path =~ s/\n/\\n/g;
> -return '"' . $path . '"';
> +return qq("${path}");
> }
Re: [PATCH] completion: add deprecated __git_complete_file ()
On Fri, Jun 07, 2013 at 02:25:54PM -0500, Felipe Contreras wrote: > On Fri, Jun 7, 2013 at 2:09 PM, Ramkumar Ramachandra > wrote: > > 77c130 (completion: clarify ls-tree, archive, show completion, > > 2013-06-02) removed __git_complete_file () because it had no callers > > left in the file. However, to avoid breaking user scripts that may > > depend on this, add it back as a deprecated alias. > > This is fine by me, but at some point we need to decide how we should > prefix the functions that are supposed to be used by external scripts. Or rather how we should prefix the functions that are _not_ supposed to be used by external scripts. That way all public functions would retain the "__git" prefix and existing scripts calling only common functions like __git_(heads|git_refs|find_on_cmdline|etc.) would work without modification. Best, Gábor -- 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 15/22] git-remote-mediawiki: Put long code into a subroutine
On Fri, Jun 7, 2013 at 5:42 PM, Célestin Matte
wrote:
> Signed-off-by: Célestin Matte
> Signed-off-by: Matthieu Moy
> ---
> contrib/mw-to-git/git-remote-mediawiki.perl | 42
> +--
> 1 file changed, 26 insertions(+), 16 deletions(-)
>
> diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
> b/contrib/mw-to-git/git-remote-mediawiki.perl
> index 1c34ada..f37488b 100755
> --- a/contrib/mw-to-git/git-remote-mediawiki.perl
> +++ b/contrib/mw-to-git/git-remote-mediawiki.perl
> @@ -133,22 +133,7 @@ while () {
> @cmd = split(/ /);
> if (defined($cmd[0])) {
> # Line not blank
> - if ($cmd[0] eq "capabilities") {
> - die("Too many arguments for capabilities\n") if
> (defined($cmd[1]));
> - mw_capabilities();
> - } elsif ($cmd[0] eq "list") {
> - die("Too many arguments for list\n") if
> (defined($cmd[2]));
> - mw_list($cmd[1]);
> - } elsif ($cmd[0] eq "import") {
> - die("Invalid arguments for import\n") if ($cmd[1] eq
> "" || defined($cmd[2]));
> - mw_import($cmd[1]);
> - } elsif ($cmd[0] eq "option") {
> - die("Too many arguments for option\n") if ($cmd[1] eq
> "" || $cmd[2] eq "" || defined($cmd[3]));
> - mw_option($cmd[1],$cmd[2]);
> - } elsif ($cmd[0] eq "push") {
> - mw_push($cmd[1]);
> - } else {
> - print STDERR "Unknown command. Aborting...\n";
> + if (parse_commands() == 1) {
> last;
Better. A few minor nits:
The new subroutine is only parsing/invoking a single command at a time
from the input line rather than multiple commands, so the name
parse_commands() is slightly misleading. Perhaps parse_command() would
be more appropriate.
Now that the functionality has been pushed into a subroutine, it does
not necessarily need to be accessing global @cmd. It might be
appropriate to pass @cmd as an argument to parse_command() (or not
depending upon your preference).
The "Unknown command. Aborting" case indicates a failure. It's pretty
typical for failures to return false rather than true. The resulting
conditional would then read a bit more idiomatically:
if (!parse_command(\@cmd)) {
last;
}
> }
> } else {
> @@ -168,6 +153,31 @@ sub exit_error_usage {
> die "ERROR: git-remote-mediawiki module was not called with a correct
> number of parameters\n";
> }
>
> +sub parse_commands {
> + if ($cmd[0] eq "capabilities") {
> + die("Too many arguments for capabilities\n")
> + if (defined($cmd[1]));
> + mw_capabilities();
> + } elsif ($cmd[0] eq "list") {
> + die("Too many arguments for list\n") if (defined($cmd[2]));
> + mw_list($cmd[1]);
> + } elsif ($cmd[0] eq "import") {
> + die("Invalid arguments for import\n")
> + if ($cmd[1] eq "" || defined($cmd[2]));
> + mw_import($cmd[1]);
> + } elsif ($cmd[0] eq "option") {
> + die("Too many arguments for option\n")
> + if ($cmd[1] eq "" || $cmd[2] eq "" || defined($cmd[3]));
> + mw_option($cmd[1],$cmd[2]);
> + } elsif ($cmd[0] eq "push") {
> + mw_push($cmd[1]);
> + } else {
> + print STDERR "Unknown command. Aborting...\n";
> + return 1;
> + }
> + return 0;
> +}
> +
> # MediaWiki API instance, created lazily.
> my $mediawiki;
>
> --
--
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] completion: add deprecated __git_complete_file ()
On Fri, Jun 07, 2013 at 01:38:16PM -0700, Junio C Hamano wrote: > Ramkumar Ramachandra writes: > > > 77c130 (completion: clarify ls-tree, archive, show completion, > > 2013-06-02) removed __git_complete_file () because it had no callers > > left in the file. However, to avoid breaking user scripts that may > > depend on this, add it back as a deprecated alias. > > > > Signed-off-by: Ramkumar Ramachandra > > --- > > Based on pu. > > Will queue; thanks. With this, I think it will be safe to push the > series in question to 'master'. Safe? Yes, at least scripts won't break because of the missing function. However, I still think it would be worth reverting at least the hunks modifying the completion functions of ls-tree and archive. Or better yet, the whole series. -- 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 14/22] git-remote-mediawiki: Check return value of open + remove import of unused open2
On Fri, Jun 7, 2013 at 5:42 PM, Célestin Matte
wrote:
> Signed-off-by: Célestin Matte
> Signed-off-by: Matthieu Moy
> ---
> contrib/mw-to-git/git-remote-mediawiki.perl |4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
> b/contrib/mw-to-git/git-remote-mediawiki.perl
> index ae6dd2e..1c34ada 100755
> --- a/contrib/mw-to-git/git-remote-mediawiki.perl
> +++ b/contrib/mw-to-git/git-remote-mediawiki.perl
> @@ -22,7 +22,6 @@ binmode STDERR, ":encoding(UTF-8)";
> binmode STDOUT, ":encoding(UTF-8)";
>
> use URI::Escape;
> -use IPC::Open2;
> use Readonly;
>
> # Mediawiki filenames can contain forward slashes. This variable decides by
> which pattern they should be replaced
> @@ -338,7 +337,8 @@ sub get_mw_pages {
> sub run_git {
> my $args = shift;
> my $encoding = (shift || "encoding(UTF-8)");
> - open(my $git, "-|:$encoding", "git " . $args);
> + open(my $git, "-|:$encoding", "git " . $args)
> + or die "Unable to open: $!\n";
> my $res = do {
> local $/ = undef;
> <$git>
These two changes are unrelated and could be split into distinct
patches (IMHO, though others may disagree).
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jun 2013, #03; Thu, 6)
On Fri, Jun 07, 2013 at 02:53:02PM -0700, Junio C Hamano wrote: > SZEDER Gábor writes: > > > On Fri, Jun 07, 2013 at 12:46:25PM -0700, Junio C Hamano wrote: > >> Thanks for a pointer. I think what I was suggesting was slightly > >> different in that I was hoping to see a single helper that knows to > >> complete to object names (possibly including trees/blobs with the > >> treeish:path notation), ranges, and pathnames (not treeish:path > >> notation) until it sees a "--" and then complete only to pathnames. > > > > We already got that except the completion of pathnames before "--", > > and I don't know how that could be done properly for commands taking > > both refs and paths. > > ... > > git diff git.c > > git diff master git.c > > git diff master next git.c > > It is somewhat annoying that "git diff gi" stops at expanding > it to "git diff git" and then upon another "git diff git" > offers tags whose names begin with "git" (e.g. gitgui-0.10.0) but > the pathname git.c is not included in the choices. My wish was to > take the union in such a fairly limited case. I tend to agree with > you that "git diff " that expands to all refs and pathnames > would be useless, but so is expansion to all pathnames (or refnames > for that matter). ... or trying to complete a branch name starting with a 'v', and then getting all the vx.y.z tags. If you know you want git.c, then you can force filename completion either by entering "--" before hitting tab or by using the Alt-/ Bash (readline?) keybinding, otherwise you'll get refs. I think this is more than adequate, as it brings the best of both worlds: you can quickly and easily get both ref-only and file-only completion. Training your fingers to type "--" is perhaps better, just in case we'll ever do tracked-file-aware filename completion for e.g. 'git log -- g' in the future. Best, Gábor -- 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 07/22] git-remote-mediawiki: Change style of some regular expressions
On Fri, Jun 7, 2013 at 5:42 PM, Célestin Matte
wrote:
> - Remove m modifier when useless (m// and // was used randomly; this makes the
> code more coherent)
> - Remove stringy split (split('c', ...) instead of split(/c/, ...))
> - Use {}{} instead of /// when slashes are used inside the regexp so as not to
> escape it.
When you split up 1/22 from v1 into distinct patches, review became
much easier. The same can be done here (IMHO). The above bullet points
are each conceptually distinct (even if they may happen to overlap
textually in some cases).
> A "split ' '" is turned into a "split / /", which changes its behaviour: the
> old
> method matched a run of whtespaces (/\s*/), while the new one will match a
> single whitespace, which is what we want here.
The phrase "which is what we want here" does not convey much. Taking a
hint from Junio's responses explaining why he finds this subtle change
acceptable, perhaps a paragraph like this might be appropriate:
Note the special case split(' ') for which Perl splits on runs of
whitespace, whereas split(/ /) splits on exactly one space. In
other contexts, changing split(' ') to split(/ /) could potentially
be a regression, however, here, when parsing the output of
"rev-list --parents", whose output SHA-1's are each separated by a
single space, splitting on a single space is perfectly correct.
More comments below.
> Signed-off-by: Célestin Matte
> Signed-off-by: Matthieu Moy
> ---
> contrib/mw-to-git/git-remote-mediawiki.perl | 20 ++--
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
> b/contrib/mw-to-git/git-remote-mediawiki.perl
> index a5c963b..482cd95 100755
> --- a/contrib/mw-to-git/git-remote-mediawiki.perl
> +++ b/contrib/mw-to-git/git-remote-mediawiki.perl
> @@ -121,7 +121,7 @@ chomp($dumb_push);
> $dumb_push = ($dumb_push eq "true");
>
> my $wiki_name = $url;
> -$wiki_name =~ s/[^\/]*:\/\///;
> +$wiki_name =~ s{[^/]*://}{};
> # If URL is like http://user:passw...@example.com/, we clearly don't
> # want the password in $wiki_name. While we're there, also remove user
> # and '@' sign, to avoid author like MWUser@httpu...@host.com
> @@ -565,7 +565,7 @@ sub mediawiki_smudge {
>
> sub mediawiki_clean_filename {
> my $filename = shift;
> - $filename =~ s/@{[SLASH_REPLACEMENT]}/\//g;
> + $filename =~ s{$SLASH_REPLACEMENT}{/}g;
The change from SLASH_REPLACEMENT to $SLASH_REPLACEMENT should have
happened in patch 2/22 where 'constant' was replaced with 'Readonly'.
> # [, ], |, {, and } are forbidden by MediaWiki, even URL-encoded.
> # Do a variant of URL-encoding, i.e. looks like URL-encoding,
> # but with _ added to prevent MediaWiki from thinking this is
> @@ -579,7 +579,7 @@ sub mediawiki_clean_filename {
>
> sub mediawiki_smudge_filename {
> my $filename = shift;
> - $filename =~ s/\//@{[SLASH_REPLACEMENT]}/g;
> + $filename =~ s{/}{$SLASH_REPLACEMENT}g;
Ditto regarding patch 2/22.
> $filename =~ s/ /_/g;
> # Decode forbidden characters encoded in mediawiki_clean_filename
> $filename =~ s/_%_([0-9a-fA-F][0-9a-fA-F])/sprintf("%c", hex($1))/ge;
> @@ -762,7 +762,7 @@ sub get_more_refs {
> my @refs;
> while (1) {
> my $line = ;
> - if ($line =~ m/^$cmd (.*)$/) {
> + if ($line =~ /^$cmd (.*)$/) {
> push(@refs, $1);
> } elsif ($line eq "\n") {
> return @refs;
> @@ -1168,11 +1168,11 @@ sub mw_push_revision {
> my @local_ancestry = split(/\n/, run_git("rev-list --boundary
> --parents $local ^$parsed_sha1"));
> my %local_ancestry;
> foreach my $line (@local_ancestry) {
> - if (my ($child, $parents) = $line =~ m/^-?([a-f0-9]+)
> ([a-f0-9 ]+)/) {
> - foreach my $parent (split(' ', $parents)) {
> + if (my ($child, $parents) = $line =~ /^-?([a-f0-9]+)
> ([a-f0-9 ]+)/) {
> + foreach my $parent (split(/ /, $parents)) {
> $local_ancestry{$parent} = $child;
> }
> - } elsif (!$line =~ m/^([a-f0-9]+)/) {
> + } elsif (!$line =~ /^([a-f0-9]+)/) {
> die "Unexpected output from git rev-list:
> $line";
> }
> }
> @@ -1190,10 +1190,10 @@ sub mw_push_revision {
> # history (linearized with --first-parent)
> print STDERR "Warning: no common ancestor, pushing complete
> history\n";
> my $history = run_git("rev-list --first-parent --children
> $local");
> - my @history = split('\n', $history);
> + my @history = split(/\n/, $history);
This looks fishy.
[PATCH] tests: fix autostash
We should call 'git rebase --abort', like a normal user would do.
Signed-off-by: Felipe Contreras
---
t/t3420-rebase-autostash.sh | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh
index a5e69f3..ff370a3 100755
--- a/t/t3420-rebase-autostash.sh
+++ b/t/t3420-rebase-autostash.sh
@@ -71,8 +71,7 @@ testrebase() {
test_must_fail git rebase$type related-onto-branch &&
test_path_is_file $dotest/autostash &&
! grep dirty file3 &&
- rm -rf $dotest &&
- git reset --hard &&
+ git rebase --abort &&
git checkout feature-branch
'
--
1.8.3.698.g079b096
--
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 22/22] git-remote-mediawiki: Clearly rewrite double dereference
On Fri, Jun 7, 2013 at 5:42 PM, Célestin Matte
wrote:
> @$var structures are re-written in the following way: @{$var}
> It makes them more readable.
>
> Signed-off-by: Célestin Matte
> Signed-off-by: Matthieu Moy
> ---
> contrib/mw-to-git/git-remote-mediawiki.perl | 12 ++--
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
> b/contrib/mw-to-git/git-remote-mediawiki.perl
> index be860c8..a13c33f 100755
> --- a/contrib/mw-to-git/git-remote-mediawiki.perl
> +++ b/contrib/mw-to-git/git-remote-mediawiki.perl
> @@ -234,7 +234,7 @@ sub get_mw_tracked_pages {
> sub get_mw_page_list {
> my $page_list = shift;
> my $pages = shift;
> - my @some_pages = @$page_list;
> + my @some_pages = @{$page_list};
> while (@some_pages) {
> my $last_page = $SLICE_SIZE;
> if ($#some_pages < $last_page) {
> @@ -736,7 +736,7 @@ sub import_file_revision {
>
> print {*STDOUT} "commit refs/mediawiki/${remotename}/master\n";
> print {*STDOUT} "mark :${n}\n";
> - print {*STDOUT} "committer ${author} <${author}\@${wiki_name}> " .
> $date->epoch . " +\n";
> + print {*STDOUT} "committer ${author} <${author}\@{${wiki_name}}> " .
> $date->epoch . " +\n";
In this case, \@ is a literal '@' in the string, so it is not a @$var
reference, thus this change is incorrect.
> literal_data($comment);
>
> # If it's not a clone, we need to know where to start from
> @@ -762,7 +762,7 @@ sub import_file_revision {
> print {*STDOUT} "reset refs/notes/${remotename}/mediawiki\n";
> }
> print {*STDOUT} "commit refs/notes/${remotename}/mediawiki\n";
> - print {*STDOUT} "committer ${author} <${author}\@${wiki_name}> " .
> $date->epoch . " +\n";
> + print {*STDOUT} "committer ${author} <${author}\@{${wiki_name}}> " .
> $date->epoch . " +\n";
Ditto: \@ is a literal '@' in the string.
> literal_data('Note added by git-mediawiki during import');
> if (!$full_import && $n == 1) {
> print {*STDOUT} "from refs/notes/${remotename}/mediawiki^0\n";
> @@ -884,7 +884,7 @@ sub mw_import_revids {
> my $n_actual = 0;
> my $last_timestamp = 0; # Placeholer in case $rev->timestamp is
> undefined
>
> - foreach my $pagerevid (@$revision_ids) {
> + foreach my $pagerevid (@{$revision_ids}) {
> # Count page even if we skip it, since we display
> # $n/$total and $total includes skipped pages.
> $n++;
> @@ -919,7 +919,7 @@ sub mw_import_revids {
> my $page_title = $result_page->{title};
>
> if (!exists($pages->{$page_title})) {
> - print {*STDERR} "${n}/", scalar(@$revision_ids),
> + print {*STDERR} "${n}/", scalar(@{$revision_ids}),
> ": Skipping revision #$rev->{revid} of
> ${page_title}\n";
> next;
> }
> @@ -952,7 +952,7 @@ sub mw_import_revids {
> # If this is a revision of the media page for new version
> # of a file do one common commit for both file and media page.
> # Else do commit only for that page.
> - print {*STDERR} "${n}/", scalar(@$revision_ids), ": Revision
> #$rev->{revid} of $commit{title}\n";
> + print {*STDERR} "${n}/", scalar(@{$revision_ids}), ":
> Revision #$rev->{revid} of $commit{title}\n";
> import_file_revision(\%commit, ($fetch_from == 1), $n_actual,
> \%mediafile);
> }
>
> --
--
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/2] build: generate test scripts
Felipe Contreras writes: > It is generated, in next. If it's not generated, there's no need to > add it to NO_INSTALL. OK, that makes sense. 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 1/3] sequencer: trivial fix
On Fri, Jun 7, 2013 at 5:49 PM, Junio C Hamano wrote: > I thought the conclusion was that combination of c8d1351 and 706728a > we already queued was the right change. Is this meant to replace > them? Yes, those would do, but I'm not going to work on that series any more. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] sequencer: trivial fix
Thanks. I thought the conclusion was that combination of c8d1351 and 706728a we already queued was the right change. Is this meant to replace 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: [PATCH 1/2] build: generate test scripts
On Fri, Jun 7, 2013 at 5:40 PM, Junio C Hamano wrote: > Felipe Contreras writes: > >>> Perhaps like this? >>> >>> Makefile | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/Makefile b/Makefile >>> index a748133..03fda50 100644 >>> --- a/Makefile >>> +++ b/Makefile >>> @@ -2239,6 +2239,7 @@ endif >>> >>> test_bindir_programs := $(patsubst >>> %,bin-wrappers/%,$(BINDIR_PROGRAMS_NEED_X) $(BINDIR_PROGRAMS_NO_X) >>> $(TEST_PROGRAMS_NEED_X)) >>> >>> +all:: $(NO_INSTALL) >>> all:: $(TEST_PROGRAMS) $(test_bindir_programs) >>> >>> bin-wrappers/%: wrap-for-bin.sh >>> @@ -2489,7 +2490,7 @@ clean: profile-clean coverage-clean >>> $(RM) *.o *.res block-sha1/*.o ppc/*.o compat/*.o compat/*/*.o >>> xdiff/*.o vcs-svn/*.o \ >>> builtin/*.o $(LIB_FILE) $(XDIFF_LIB) $(VCSSVN_LIB) >>> $(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) git$X >>> - $(RM) $(TEST_PROGRAMS) >>> + $(RM) $(TEST_PROGRAMS) $(NO_INSTALL) >>> $(RM) -r bin-wrappers $(dep_dirs) >>> $(RM) -r po/build/ >>> $(RM) *.spec *.pyc *.pyo */*.pyc */*.pyo common-cmds.h >>> $(ETAGS_TARGET) tags cscope* >> >> Looks good to me. > > Actually the above would not work well. This is because... > >>> ..., assuming that NO_INSTALL will mean "We always >>> want to build these, but we never do not want to install them" >>> forever (which I am OK to assume). > > ... the assumption does *not* hold already with git-remote-testgit, > which is (or will be with patch 2/2) NO_INSTALL and we do not want > to install it, but it is not built, hence we do not want to remove > it, either. It is generated, in next. If it's not generated, there's no need to add it to NO_INSTALL. -- Felipe Contreras -- 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/2] build: generate test scripts
Felipe Contreras writes: >> Perhaps like this? >> >> Makefile | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/Makefile b/Makefile >> index a748133..03fda50 100644 >> --- a/Makefile >> +++ b/Makefile >> @@ -2239,6 +2239,7 @@ endif >> >> test_bindir_programs := $(patsubst >> %,bin-wrappers/%,$(BINDIR_PROGRAMS_NEED_X) $(BINDIR_PROGRAMS_NO_X) >> $(TEST_PROGRAMS_NEED_X)) >> >> +all:: $(NO_INSTALL) >> all:: $(TEST_PROGRAMS) $(test_bindir_programs) >> >> bin-wrappers/%: wrap-for-bin.sh >> @@ -2489,7 +2490,7 @@ clean: profile-clean coverage-clean >> $(RM) *.o *.res block-sha1/*.o ppc/*.o compat/*.o compat/*/*.o >> xdiff/*.o vcs-svn/*.o \ >> builtin/*.o $(LIB_FILE) $(XDIFF_LIB) $(VCSSVN_LIB) >> $(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) git$X >> - $(RM) $(TEST_PROGRAMS) >> + $(RM) $(TEST_PROGRAMS) $(NO_INSTALL) >> $(RM) -r bin-wrappers $(dep_dirs) >> $(RM) -r po/build/ >> $(RM) *.spec *.pyc *.pyo */*.pyc */*.pyo common-cmds.h >> $(ETAGS_TARGET) tags cscope* > > Looks good to me. Actually the above would not work well. This is because... >> ..., assuming that NO_INSTALL will mean "We always >> want to build these, but we never do not want to install them" >> forever (which I am OK to assume). ... the assumption does *not* hold already with git-remote-testgit, which is (or will be with patch 2/2) NO_INSTALL and we do not want to install it, but it is not built, hence we do not want to remove it, either. -- 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/2] build: generate test scripts
On Fri, Jun 7, 2013 at 5:28 PM, Junio C Hamano wrote: > Felipe Contreras writes: > >> On Fri, Jun 7, 2013 at 5:03 PM, Felipe Contreras >> wrote: >> >>> -all:: $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) $(OTHER_PROGRAMS) >>> GIT-BUILD-OPTIONS >>> +all:: $(ALL_PROGRAMS) $(SCRIPTS_GEN) $(SCRIPT_LIB) $(BUILT_INS) >>> $(OTHER_PROGRAMS) GIT-BUILD-OPTIONS >> >> Alternatively, we could add $(NO_INSTALL) here. > > As ALL_PROGRAMS overlap with most of SCRIPTS_GEN, the above looks > overly heavy-fisted. I tend to agree that a separate > > all:: $(NO_INSTALL) > > would be much better, assuming that NO_INSTALL will mean "We always > want to build these, but we never do not want to install them" > forever (which I am OK to assume). > > Also > > make clean > make --test=5800 test > > did not fail for me, and it turns out that "clean" somehow fails to > clean git-remote-testpy script. > > As git-remote-testpy is only for testing, another possibility is to > do > > -all:: $(TEST_PROGRAMS) $(test_bindir_programs) > +all:: $(TEST_PROGRAMS) $(test_bindir_programs) git-remote-testpy > > but I think $(NO_INSTALL) is the cleanest. > > Perhaps like this? > > Makefile | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/Makefile b/Makefile > index a748133..03fda50 100644 > --- a/Makefile > +++ b/Makefile > @@ -2239,6 +2239,7 @@ endif > > test_bindir_programs := $(patsubst > %,bin-wrappers/%,$(BINDIR_PROGRAMS_NEED_X) $(BINDIR_PROGRAMS_NO_X) > $(TEST_PROGRAMS_NEED_X)) > > +all:: $(NO_INSTALL) > all:: $(TEST_PROGRAMS) $(test_bindir_programs) > > bin-wrappers/%: wrap-for-bin.sh > @@ -2489,7 +2490,7 @@ clean: profile-clean coverage-clean > $(RM) *.o *.res block-sha1/*.o ppc/*.o compat/*.o compat/*/*.o > xdiff/*.o vcs-svn/*.o \ > builtin/*.o $(LIB_FILE) $(XDIFF_LIB) $(VCSSVN_LIB) > $(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) git$X > - $(RM) $(TEST_PROGRAMS) > + $(RM) $(TEST_PROGRAMS) $(NO_INSTALL) > $(RM) -r bin-wrappers $(dep_dirs) > $(RM) -r po/build/ > $(RM) *.spec *.pyc *.pyo */*.pyc */*.pyo common-cmds.h > $(ETAGS_TARGET) tags cscope* Looks good to me. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 2/2] read-cache: plug a few leaks
We are not freeing 'istate->cache' properly. We can't rely on 'initialized' to keep track of the 'istate->cache', because it doesn't really mean it's initialized. So assume it always has data, and free it before overwriting it. Signed-off-by: Felipe Contreras --- read-cache.c | 4 1 file changed, 4 insertions(+) diff --git a/read-cache.c b/read-cache.c index 5e30746..a1dd04d 100644 --- a/read-cache.c +++ b/read-cache.c @@ -1451,6 +1451,7 @@ int read_index_from(struct index_state *istate, const char *path) istate->version = ntohl(hdr->hdr_version); istate->cache_nr = ntohl(hdr->hdr_entries); istate->cache_alloc = alloc_nr(istate->cache_nr); + free(istate->cache); istate->cache = xcalloc(istate->cache_alloc, sizeof(*istate->cache)); istate->initialized = 1; @@ -1512,6 +1513,9 @@ int discard_index(struct index_state *istate) for (i = 0; i < istate->cache_nr; i++) free(istate->cache[i]); + free(istate->cache); + istate->cache = NULL; + istate->cache_alloc = 0; resolve_undo_clear_index(istate); istate->cache_nr = 0; istate->cache_changed = 0; -- 1.8.3.698.g079b096 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 1/2] unpack-trees: plug a memory leak
Before overwriting the destination index, first let's discard its
contents.
Signed-off-by: Felipe Contreras
---
unpack-trees.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/unpack-trees.c b/unpack-trees.c
index 57b4074..abe2576 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1166,8 +1166,10 @@ int unpack_trees(unsigned len, struct tree_desc *t,
struct unpack_trees_options
o->src_index = NULL;
ret = check_updates(o) ? (-2) : 0;
- if (o->dst_index)
+ if (o->dst_index) {
+ discard_index(o->dst_index);
*o->dst_index = o->result;
+ }
done:
clear_exclude_list(&el);
--
1.8.3.698.g079b096
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 0/2] cherry-pick: fix memory leaks
Felipe Contreras (2): unpack-trees: plug a memory leak read-cache: plug a few leaks read-cache.c | 4 unpack-trees.c | 4 +++- 2 files changed, 7 insertions(+), 1 deletion(-) -- 1.8.3.698.g079b096 -- 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/2] build: generate test scripts
Felipe Contreras writes: > On Fri, Jun 7, 2013 at 5:03 PM, Felipe Contreras > wrote: > >> -all:: $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) $(OTHER_PROGRAMS) >> GIT-BUILD-OPTIONS >> +all:: $(ALL_PROGRAMS) $(SCRIPTS_GEN) $(SCRIPT_LIB) $(BUILT_INS) >> $(OTHER_PROGRAMS) GIT-BUILD-OPTIONS > > Alternatively, we could add $(NO_INSTALL) here. As ALL_PROGRAMS overlap with most of SCRIPTS_GEN, the above looks overly heavy-fisted. I tend to agree that a separate all:: $(NO_INSTALL) would be much better, assuming that NO_INSTALL will mean "We always want to build these, but we never do not want to install them" forever (which I am OK to assume). Also make clean make --test=5800 test did not fail for me, and it turns out that "clean" somehow fails to clean git-remote-testpy script. As git-remote-testpy is only for testing, another possibility is to do -all:: $(TEST_PROGRAMS) $(test_bindir_programs) +all:: $(TEST_PROGRAMS) $(test_bindir_programs) git-remote-testpy but I think $(NO_INSTALL) is the cleanest. Perhaps like this? Makefile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index a748133..03fda50 100644 --- a/Makefile +++ b/Makefile @@ -2239,6 +2239,7 @@ endif test_bindir_programs := $(patsubst %,bin-wrappers/%,$(BINDIR_PROGRAMS_NEED_X) $(BINDIR_PROGRAMS_NO_X) $(TEST_PROGRAMS_NEED_X)) +all:: $(NO_INSTALL) all:: $(TEST_PROGRAMS) $(test_bindir_programs) bin-wrappers/%: wrap-for-bin.sh @@ -2489,7 +2490,7 @@ clean: profile-clean coverage-clean $(RM) *.o *.res block-sha1/*.o ppc/*.o compat/*.o compat/*/*.o xdiff/*.o vcs-svn/*.o \ builtin/*.o $(LIB_FILE) $(XDIFF_LIB) $(VCSSVN_LIB) $(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) git$X - $(RM) $(TEST_PROGRAMS) + $(RM) $(TEST_PROGRAMS) $(NO_INSTALL) $(RM) -r bin-wrappers $(dep_dirs) $(RM) -r po/build/ $(RM) *.spec *.pyc *.pyo */*.pyc */*.pyo common-cmds.h $(ETAGS_TARGET) tags cscope* -- 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] unpack-trees: improve maintainability
Clarify that ce can be NULL by adding a check (probably more efficient
also), and get rid of unnecessary indentation (since it's a single line
it doesn't matter, but in the future it might).
Signed-off-by: Felipe Contreras
---
unpack-trees.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/unpack-trees.c b/unpack-trees.c
index 57b4074..9c0100d 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -605,8 +605,9 @@ static int unpack_nondirectories(int n, unsigned long mask,
o);
for (i = 0; i < n; i++) {
struct cache_entry *ce = src[i + o->merge];
- if (ce != o->df_conflict_entry)
- free(ce);
+ if (!ce || ce == o->df_conflict_entry)
+ continue;
+ free(ce);
}
return rc;
}
--
1.8.3.698.g079b096
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] Move sequencer
Hi, This patch series moves the sequencer code which is only useful for builtin commands, to builtin. This also creates the concept of 'builtin helpers'. Felipe Contreras (2): log-tree: remove dependency from sequencer Move sequencer to builtin Makefile | 9 ++- sequencer.c => builtin/sequencer.c | 160 +--- sequencer.h => builtin/sequencer.h | 4 - log-tree.c | 161 - log-tree.h | 3 + 5 files changed, 172 insertions(+), 165 deletions(-) rename sequencer.c => builtin/sequencer.c (87%) rename sequencer.h => builtin/sequencer.h (88%) -- 1.8.3.698.g079b096 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] log-tree: remove dependency from sequencer
Move the relevant code from sequencer to log-tree. This code is not
specific to sequencer, and this allows the sequencer to move out of
libgit.
Signed-off-by: Felipe Contreras
---
log-tree.c | 161 +++-
log-tree.h | 3 ++
sequencer.c | 160 ++-
sequencer.h | 4 --
4 files changed, 166 insertions(+), 162 deletions(-)
diff --git a/log-tree.c b/log-tree.c
index 2eb69bc..654f5db 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -9,10 +9,13 @@
#include "string-list.h"
#include "color.h"
#include "gpg-interface.h"
-#include "sequencer.h"
#include "line-log.h"
+#define APPEND_SIGNOFF_DEDUP (1u << 0)
+
struct decoration name_decoration = { "object names" };
+const char sign_off_header[] = "Signed-off-by: ";
+static const char cherry_picked_prefix[] = "(cherry picked from commit ";
enum decoration_type {
DECORATION_NONE = 0,
@@ -472,6 +475,162 @@ static void show_mergetag(struct rev_info *opt, struct
commit *commit)
free_commit_extra_headers(to_free);
}
+static int is_rfc2822_line(const char *buf, int len)
+{
+ int i;
+
+ for (i = 0; i < len; i++) {
+ int ch = buf[i];
+ if (ch == ':')
+ return 1;
+ if (!isalnum(ch) && ch != '-')
+ break;
+ }
+
+ return 0;
+}
+
+static int is_cherry_picked_from_line(const char *buf, int len)
+{
+ /*
+* We only care that it looks roughly like (cherry picked from ...)
+*/
+ return len > strlen(cherry_picked_prefix) + 1 &&
+ !prefixcmp(buf, cherry_picked_prefix) && buf[len - 1] == ')';
+}
+
+/*
+ * Returns 0 for non-conforming footer
+ * Returns 1 for conforming footer
+ * Returns 2 when sob exists within conforming footer
+ * Returns 3 when sob exists within conforming footer as last entry
+ */
+int has_conforming_footer(struct strbuf *sb, struct strbuf *sob,
+ int ignore_footer)
+{
+ char prev;
+ int i, k;
+ int len = sb->len - ignore_footer;
+ const char *buf = sb->buf;
+ int found_sob = 0;
+
+ /* footer must end with newline */
+ if (!len || buf[len - 1] != '\n')
+ return 0;
+
+ prev = '\0';
+ for (i = len - 1; i > 0; i--) {
+ char ch = buf[i];
+ if (prev == '\n' && ch == '\n') /* paragraph break */
+ break;
+ prev = ch;
+ }
+
+ /* require at least one blank line */
+ if (prev != '\n' || buf[i] != '\n')
+ return 0;
+
+ /* advance to start of last paragraph */
+ while (i < len - 1 && buf[i] == '\n')
+ i++;
+
+ for (; i < len; i = k) {
+ int found_rfc2822;
+
+ for (k = i; k < len && buf[k] != '\n'; k++)
+ ; /* do nothing */
+ k++;
+
+ found_rfc2822 = is_rfc2822_line(buf + i, k - i - 1);
+ if (found_rfc2822 && sob &&
+ !strncmp(buf + i, sob->buf, sob->len))
+ found_sob = k;
+
+ if (!(found_rfc2822 ||
+ is_cherry_picked_from_line(buf + i, k - i - 1)))
+ return 0;
+ }
+ if (found_sob == i)
+ return 3;
+ if (found_sob)
+ return 2;
+ return 1;
+}
+
+void append_cherrypick(struct strbuf *msgbuf, struct object *obj)
+{
+ if (!has_conforming_footer(msgbuf, NULL, 0))
+ strbuf_addch(msgbuf, '\n');
+ strbuf_addstr(msgbuf, cherry_picked_prefix);
+ strbuf_addstr(msgbuf, sha1_to_hex(obj->sha1));
+ strbuf_addstr(msgbuf, ")\n");
+}
+
+void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag)
+{
+ unsigned no_dup_sob = flag & APPEND_SIGNOFF_DEDUP;
+ struct strbuf sob = STRBUF_INIT;
+ int has_footer;
+
+ strbuf_addstr(&sob, sign_off_header);
+ strbuf_addstr(&sob, fmt_name(getenv("GIT_COMMITTER_NAME"),
+ getenv("GIT_COMMITTER_EMAIL")));
+ strbuf_addch(&sob, '\n');
+
+ /*
+* If the whole message buffer is equal to the sob, pretend that we
+* found a conforming footer with a matching sob
+*/
+ if (msgbuf->len - ignore_footer == sob.len &&
+ !strncmp(msgbuf->buf, sob.buf, sob.len))
+ has_footer = 3;
+ else
+ has_footer = has_conforming_footer(msgbuf, &sob, ignore_footer);
+
+ if (!has_footer) {
+ const char *append_newlines = NULL;
+ size_t len = msgbuf->len - ignore_footer;
+
+ if (!len) {
+ /*
+* The buffer is completely empty. Leave foom for
+* the title and body to be filled in by the user.
+*/
+ append_newlines =
[PATCH 2/2] Move sequencer to builtin
This code is only useful for cherry-pick and revert built-ins, nothing else, so let's make it a builtin object, but make sure 'git-sequencer' is not generated. Signed-off-by: Felipe Contreras --- Makefile | 9 ++--- sequencer.c => builtin/sequencer.c | 0 sequencer.h => builtin/sequencer.h | 0 3 files changed, 6 insertions(+), 3 deletions(-) rename sequencer.c => builtin/sequencer.c (100%) rename sequencer.h => builtin/sequencer.h (100%) diff --git a/Makefile b/Makefile index 03524d0..d28bf7f 100644 --- a/Makefile +++ b/Makefile @@ -583,7 +583,8 @@ TEST_PROGRAMS = $(patsubst %,%$X,$(TEST_PROGRAMS_NEED_X)) # List built-in command $C whose implementation cmd_$C() is not in # builtin/$C.o but is linked in as part of some other command. -BUILT_INS += $(patsubst builtin/%.o,git-%$X,$(BUILTIN_OBJS)) +BUILT_INS_OBJS = $(filter-out $(BUILTIN_HELPER_OBJS),$(BUILTIN_OBJS)) +BUILT_INS += $(patsubst builtin/%.o,git-%$X,$(BUILT_INS_OBJS)) BUILT_INS += git-cherry$X BUILT_INS += git-cherry-pick$X @@ -714,7 +715,6 @@ LIB_H += resolve-undo.h LIB_H += revision.h LIB_H += run-command.h LIB_H += send-pack.h -LIB_H += sequencer.h LIB_H += sha1-array.h LIB_H += sha1-lookup.h LIB_H += shortlog.h @@ -856,7 +856,6 @@ LIB_OBJS += resolve-undo.o LIB_OBJS += revision.o LIB_OBJS += run-command.o LIB_OBJS += send-pack.o -LIB_OBJS += sequencer.o LIB_OBJS += server-info.o LIB_OBJS += setup.o LIB_OBJS += sha1-array.o @@ -894,6 +893,8 @@ LIB_OBJS += wt-status.o LIB_OBJS += xdiff-interface.o LIB_OBJS += zlib.o +BUILTIN_HELPER_OBJS += builtin/sequencer.o + BUILTIN_OBJS += builtin/add.o BUILTIN_OBJS += builtin/annotate.o BUILTIN_OBJS += builtin/apply.o @@ -990,6 +991,8 @@ BUILTIN_OBJS += builtin/verify-pack.o BUILTIN_OBJS += builtin/verify-tag.o BUILTIN_OBJS += builtin/write-tree.o +BUILTIN_OBJS += $(BUILTIN_HELPER_OBJS) + GITLIBS = $(LIB_FILE) $(XDIFF_LIB) EXTLIBS = diff --git a/sequencer.c b/builtin/sequencer.c similarity index 100% rename from sequencer.c rename to builtin/sequencer.c diff --git a/sequencer.h b/builtin/sequencer.h similarity index 100% rename from sequencer.h rename to builtin/sequencer.h -- 1.8.3.698.g079b096 -- 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] contrib: related: print the amount of involvement
100% means the person was involved in all the commits, in one way or the other. Signed-off-by: Felipe Contreras --- It's barely useful for me without this. This sits on top of the last git-related series. contrib/related/git-related | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/related/git-related b/contrib/related/git-related index bded6f6..f0bda55 100755 --- a/contrib/related/git-related +++ b/contrib/related/git-related @@ -168,5 +168,5 @@ end count_per_person.each do |person, count| percent = count.to_f * 100 / commits.size next if percent < $min_percent - puts person + puts '%s (involved: %u%%)' % [person, percent] end -- 1.8.3.698.g079b096 -- 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/2] build: generate test scripts
On Fri, Jun 7, 2013 at 5:03 PM, Felipe Contreras wrote: > -all:: $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) $(OTHER_PROGRAMS) > GIT-BUILD-OPTIONS > +all:: $(ALL_PROGRAMS) $(SCRIPTS_GEN) $(SCRIPT_LIB) $(BUILT_INS) > $(OTHER_PROGRAMS) GIT-BUILD-OPTIONS Alternatively, we could add $(NO_INSTALL) here. -- Felipe Contreras -- 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 00/10] Increase test coverage on Windows by removing SYMLINKS from many tests
Thanks; the interdiff looks sane. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] build: generate test scripts
Commit 416fda6 (build: do not install git-remote-testpy) made it so git-remote-testpy is not only not installed, but also not generated by default, let's make sure all the scripts are generated. Signed-off-by: Felipe Contreras --- Makefile | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 03524d0..126be01 100644 --- a/Makefile +++ b/Makefile @@ -531,6 +531,11 @@ SCRIPTS = $(SCRIPT_SH_INS) \ $(SCRIPT_PYTHON_INS) \ git-instaweb +SCRIPTS_GEN = $(SCRIPT_SH_GEN) \ + $(SCRIPT_PERL_GEN) \ + $(SCRIPT_PYTHON_GEN) \ + git-instaweb + ETAGS_TARGET = TAGS # Empty... @@ -1647,7 +1652,7 @@ all:: profile-clean endif endif -all:: $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) $(OTHER_PROGRAMS) GIT-BUILD-OPTIONS +all:: $(ALL_PROGRAMS) $(SCRIPTS_GEN) $(SCRIPT_LIB) $(BUILT_INS) $(OTHER_PROGRAMS) GIT-BUILD-OPTIONS ifneq (,$X) $(QUIET_BUILT_IN)$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), test -d '$p' -o '$p' -ef '$p$X' || $(RM) '$p';) endif -- 1.8.3.698.g079b096 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] Build fixes
Felipe Contreras (2): build: generate test scripts build: do not install git-remote-testgit Makefile | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) -- 1.8.3.698.g079b096 -- 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/2] build: do not install git-remote-testgit
Signed-off-by: Felipe Contreras --- Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile b/Makefile index 126be01..02e3d43 100644 --- a/Makefile +++ b/Makefile @@ -491,6 +491,7 @@ SCRIPT_PERL += git-svn.perl SCRIPT_PYTHON += git-remote-testpy.py SCRIPT_PYTHON += git-p4.py +NO_INSTALL += git-remote-testgit NO_INSTALL += git-remote-testpy # Generated files for scripts -- 1.8.3.698.g079b096 -- 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] gitweb: fix problem causing erroneous project list
Jakub Narębski writes: >>> Instead, clear $search_regexp before dispatching each request. >>> >>> Signed-off-by: Charles McGarvey > > Acked-by: Jakub Narebski Thanks (the ack was a few hours too late and the commit is already in 'next', so I won't be able to rewind it though). >> By the way, I looked at how $search_regexp is used in the code: > > How $search_regexp is used does not matter. What was intended > (but was not implemented) is for $search_regexp to matter and to > be used only if $searchtext is defined. $searchtext is reset on each > request, so $search_regexp should be also reset... like in Charles's > patch. Oh, we are in total agreement about that. That is why the part is marked with "By the way"---it is an orthogonal issue (which turned out to be a non-issue). >> x git_search_files and git_search_grep_body assume that >>$search_regexp can be interpolated in m//, which is not very >>nice. They want an empty string. > > But both git_search_files() and git_search_grep_body() are run from > git_search(), which "dies" (returns HTTP 400 "Text field is empty" error) > if $searchtext is not defined; if $searchtext is defined then $search_regexp > is string and is never undef. Thanks; that is what I missed. >> So as an independent fix, the two subs may want to be fixed if we >> want to be undef clean. Or am I missing something? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-gui: bring Wish process to front on Mac
Junio C Hamano wrote: > Stefan (as your name appears in 76bf6ff93e, I am assuming that you > were the OSX-osascript guru in that commit) could you keep an eye on > the list traffic to see if users of latest gitk have issues with > that change, please? Sure, will do. -- Stefan Haller Berlin, Germany http://www.haller-berlin.de/ -- 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] git-gui: bring Wish process to front on Mac
On Mac OS X, any application that is started from the Terminal will open
behind all running applications; as a work-around, manually bring ourselves
to the front. (Stolen from gitk, commit 76bf6ff93e.)
We do this as the very first thing, so that any message boxes that might pop
up during the rest of the startup sequence are actually seen by the user.
Signed-off-by: Stefan Haller
---
Changes since the first patch:
- add catch
- specify full path to /usr/bin/osascript
git-gui.sh | 15 +++
1 file changed, 15 insertions(+)
diff --git a/git-gui.sh b/git-gui.sh
index e11..a792924 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -29,6 +29,21 @@ Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
02111-1307 USA}]
##
##
+## On Mac, bring the current Wish process window to front
+
+if {[tk windowingsystem] eq "aqua"} {
+ catch {
+ exec /usr/bin/osascript -e [format {
+ tell application "System Events"
+ set frontmost of processes whose unix id is %d
to true
+ end tell
+ } [pid] ]
+ }
+}
+
+
+##
+##
## Tcl/Tk sanity check
if {[catch {package require Tcl 8.4} err]
--
1.8.3.14.g33f718c
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jun 2013, #03; Thu, 6)
SZEDER Gábor writes: > On Fri, Jun 07, 2013 at 12:46:25PM -0700, Junio C Hamano wrote: >> Thanks for a pointer. I think what I was suggesting was slightly >> different in that I was hoping to see a single helper that knows to >> complete to object names (possibly including trees/blobs with the >> treeish:path notation), ranges, and pathnames (not treeish:path >> notation) until it sees a "--" and then complete only to pathnames. > > We already got that except the completion of pathnames before "--", > and I don't know how that could be done properly for commands taking > both refs and paths. > ... > git diff git.c > git diff master git.c > git diff master next git.c It is somewhat annoying that "git diff gi" stops at expanding it to "git diff git" and then upon another "git diff git" offers tags whose names begin with "git" (e.g. gitgui-0.10.0) but the pathname git.c is not included in the choices. My wish was to take the union in such a fairly limited case. I tend to agree with you that "git diff " that expands to all refs and pathnames would be useless, but so is expansion to all pathnames (or refnames for that matter). In any case, I wouldn't insist on AI perfection in the first place ;-). As long as it works reasonably well, I am happy (and I think the current completion code already works better than reasonably well, at least for me). 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
[PATCH/RFC] git-remote-mediawiki: new tool to preview local changes without pushing
From: Benoit Person
The #7 issue on git-mediawiki's issue tracker [1] states that the ability to
preview content without pushing would be a nice thing to have.
This commit is a first attempt to achieve it. It adds a new git command,
named `git mw`. This command accepts the subcommands `help` and `preview`
for now.
The default behaviour for the `preview` subcommand is:
1- Find the remote name of the current branch's upstream and check if it's a
wiki one with its url (ie: mediawiki://)
2- Parse the content of the local file (given as argument) using the distant
wiki's API.
3- Retrieve the current page on the distant mediawiki.
4- Merge those those contents.
5- Convert relative links to absolute ones.
6- Save the result on disk.
The command also accepts argument for more controls:
--autoload | -a tries to launch the newly generated file in the user's
default browser
--remote | -r provides a way to select the distant mediawiki in which the
user wants to preview his file.
--output | -o enables the user to choose the output filename. Default output
filename is based on the input filename in which the extension
`.mw` is replaced with `.html`.
It works but a couple of points trouble me:
1- I had to copy two functions from `git-remote-mediawiki.perl`, I don't
really know how we could "factorize" those things ? I don't think it makes
much sense to create a package just for them ?
2- The current behavior is to crash if the current branch do not have an
upstream branch on a valid mediawiki remote. To find that specific remote,
it runs `git rev-parse --symbolic-full-name @{upstream}` which will return
something like `/refs/remotes/$remote_name/master`.
2a- maybe there is a better way to find that remote name ?
2b- would it be useful to add a fallback if that search fails ? searching
for a valid mediawiki remote url in all the remotes returned by
`git remote` for instance ?
3- The current idea of merging the content of the mediawiki's current page
with new content fails when the page is a local creation. (Hence the
error message when we get a bad result from the `get` call l.129). I
thought about two ways to overcome this:
3a- Use the wiki's homepage for the template.
3b- A two-step process:
- first we create a general template (based on the wiki's homepage)
with a specific flag at the content position. This step is done only
if that template do not exists locally.
- replace the specific flag with the newly parsed content.
The downfall of those two "solutions" is on links like 'talk',
'view source' etc ... those will need to be updated to. And evven then,
it will still fails for page created only locally.
4- In the Makefile, there is certainly a more "Makefily" way to do it but I
had no luck finding it and still preserving the factorization for the
`build`, `install` and `clean` target. I ended up with something like this:
build: $(BUILD_SCRIPTS)
$(BUILD_SCRIPTS):
$(MAKE) -C $(GIT_ROOT_DIR) SCRIPT_PERL=$@ \
build-perl-script
install: ...
clean: ...
but clearly, for only two scripts, it's more a refucktoring than a
refactoring :/ .
[1] https://github.com/moy/Git-Mediawiki/issues/7
Signed-off-by: Benoit Person
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/Makefile| 4 +
contrib/mw-to-git/git-mw.perl | 249 ++
2 files changed, 253 insertions(+)
create mode 100644 contrib/mw-to-git/git-mw.perl
diff --git a/contrib/mw-to-git/Makefile b/contrib/mw-to-git/Makefile
index f149719..0cba1e3 100644
--- a/contrib/mw-to-git/Makefile
+++ b/contrib/mw-to-git/Makefile
@@ -5,13 +5,17 @@
## Build git-remote-mediawiki
SCRIPT_PERL=git-remote-mediawiki.perl
+SCRIPT_PERL_MW=git-mw.perl
GIT_ROOT_DIR=../..
HERE=contrib/mw-to-git/
SCRIPT_PERL_FULL=$(patsubst %,$(HERE)/%,$(SCRIPT_PERL))
+SCRIPT_PERL_MW_FULL=$(patsubst %,$(HERE)/%,$(SCRIPT_PERL_MW))
all: build
build install clean:
$(MAKE) -C $(GIT_ROOT_DIR) SCRIPT_PERL=$(SCRIPT_PERL_FULL) \
$@-perl-script
+ $(MAKE) -C $(GIT_ROOT_DIR) SCRIPT_PERL=$(SCRIPT_PERL_MW_FULL) \
+$@-perl-script
\ No newline at end of file
diff --git a/contrib/mw-to-git/git-mw.perl b/contrib/mw-to-git/git-mw.perl
new file mode 100644
index 000..a92c459
--- /dev/null
+++ b/contrib/mw-to-git/git-mw.perl
@@ -0,0 +1,249 @@
+#! /usr/bin/perl
+
+# Copyright (C) 2013
+# Benoit Person
+# Celestin Matte
+# License: GPL v2 or later
+
+# Tools attached to git-remote-mediawiki (help, preview ...)
+# Documentation & bugtracker: https://github.com/moy/Git-Mediawiki/
+
+use strict;
+use warnings;
+
+use Git;
+use MediaWiki::API;
+
+use Getopt::Long;
+use HTML::TreeBuilder;
+use URI::URL qw(url);
+use IO::File;
+use LWP::Simple;
+use Carp;
[PATCH v2 22/22] git-remote-mediawiki: Clearly rewrite double dereference
@$var structures are re-written in the following way: @{$var}
It makes them more readable.
Signed-off-by: Célestin Matte
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-remote-mediawiki.perl | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index be860c8..a13c33f 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -234,7 +234,7 @@ sub get_mw_tracked_pages {
sub get_mw_page_list {
my $page_list = shift;
my $pages = shift;
- my @some_pages = @$page_list;
+ my @some_pages = @{$page_list};
while (@some_pages) {
my $last_page = $SLICE_SIZE;
if ($#some_pages < $last_page) {
@@ -736,7 +736,7 @@ sub import_file_revision {
print {*STDOUT} "commit refs/mediawiki/${remotename}/master\n";
print {*STDOUT} "mark :${n}\n";
- print {*STDOUT} "committer ${author} <${author}\@${wiki_name}> " .
$date->epoch . " +\n";
+ print {*STDOUT} "committer ${author} <${author}\@{${wiki_name}}> " .
$date->epoch . " +\n";
literal_data($comment);
# If it's not a clone, we need to know where to start from
@@ -762,7 +762,7 @@ sub import_file_revision {
print {*STDOUT} "reset refs/notes/${remotename}/mediawiki\n";
}
print {*STDOUT} "commit refs/notes/${remotename}/mediawiki\n";
- print {*STDOUT} "committer ${author} <${author}\@${wiki_name}> " .
$date->epoch . " +\n";
+ print {*STDOUT} "committer ${author} <${author}\@{${wiki_name}}> " .
$date->epoch . " +\n";
literal_data('Note added by git-mediawiki during import');
if (!$full_import && $n == 1) {
print {*STDOUT} "from refs/notes/${remotename}/mediawiki^0\n";
@@ -884,7 +884,7 @@ sub mw_import_revids {
my $n_actual = 0;
my $last_timestamp = 0; # Placeholer in case $rev->timestamp is
undefined
- foreach my $pagerevid (@$revision_ids) {
+ foreach my $pagerevid (@{$revision_ids}) {
# Count page even if we skip it, since we display
# $n/$total and $total includes skipped pages.
$n++;
@@ -919,7 +919,7 @@ sub mw_import_revids {
my $page_title = $result_page->{title};
if (!exists($pages->{$page_title})) {
- print {*STDERR} "${n}/", scalar(@$revision_ids),
+ print {*STDERR} "${n}/", scalar(@{$revision_ids}),
": Skipping revision #$rev->{revid} of
${page_title}\n";
next;
}
@@ -952,7 +952,7 @@ sub mw_import_revids {
# If this is a revision of the media page for new version
# of a file do one common commit for both file and media page.
# Else do commit only for that page.
- print {*STDERR} "${n}/", scalar(@$revision_ids), ": Revision
#$rev->{revid} of $commit{title}\n";
+ print {*STDERR} "${n}/", scalar(@{$revision_ids}), ": Revision
#$rev->{revid} of $commit{title}\n";
import_file_revision(\%commit, ($fetch_from == 1), $n_actual,
\%mediafile);
}
--
1.7.9.5
--
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 07/22] git-remote-mediawiki: Change style of some regular expressions
- Remove m modifier when useless (m// and // was used randomly; this makes the
code more coherent)
- Remove stringy split (split('c', ...) instead of split(/c/, ...))
- Use {}{} instead of /// when slashes are used inside the regexp so as not to
escape it.
A "split ' '" is turned into a "split / /", which changes its behaviour: the old
method matched a run of whtespaces (/\s*/), while the new one will match a
single whitespace, which is what we want here.
Signed-off-by: Célestin Matte
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-remote-mediawiki.perl | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index a5c963b..482cd95 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -121,7 +121,7 @@ chomp($dumb_push);
$dumb_push = ($dumb_push eq "true");
my $wiki_name = $url;
-$wiki_name =~ s/[^\/]*:\/\///;
+$wiki_name =~ s{[^/]*://}{};
# If URL is like http://user:passw...@example.com/, we clearly don't
# want the password in $wiki_name. While we're there, also remove user
# and '@' sign, to avoid author like MWUser@httpu...@host.com
@@ -565,7 +565,7 @@ sub mediawiki_smudge {
sub mediawiki_clean_filename {
my $filename = shift;
- $filename =~ s/@{[SLASH_REPLACEMENT]}/\//g;
+ $filename =~ s{$SLASH_REPLACEMENT}{/}g;
# [, ], |, {, and } are forbidden by MediaWiki, even URL-encoded.
# Do a variant of URL-encoding, i.e. looks like URL-encoding,
# but with _ added to prevent MediaWiki from thinking this is
@@ -579,7 +579,7 @@ sub mediawiki_clean_filename {
sub mediawiki_smudge_filename {
my $filename = shift;
- $filename =~ s/\//@{[SLASH_REPLACEMENT]}/g;
+ $filename =~ s{/}{$SLASH_REPLACEMENT}g;
$filename =~ s/ /_/g;
# Decode forbidden characters encoded in mediawiki_clean_filename
$filename =~ s/_%_([0-9a-fA-F][0-9a-fA-F])/sprintf("%c", hex($1))/ge;
@@ -762,7 +762,7 @@ sub get_more_refs {
my @refs;
while (1) {
my $line = ;
- if ($line =~ m/^$cmd (.*)$/) {
+ if ($line =~ /^$cmd (.*)$/) {
push(@refs, $1);
} elsif ($line eq "\n") {
return @refs;
@@ -1168,11 +1168,11 @@ sub mw_push_revision {
my @local_ancestry = split(/\n/, run_git("rev-list --boundary
--parents $local ^$parsed_sha1"));
my %local_ancestry;
foreach my $line (@local_ancestry) {
- if (my ($child, $parents) = $line =~ m/^-?([a-f0-9]+)
([a-f0-9 ]+)/) {
- foreach my $parent (split(' ', $parents)) {
+ if (my ($child, $parents) = $line =~ /^-?([a-f0-9]+)
([a-f0-9 ]+)/) {
+ foreach my $parent (split(/ /, $parents)) {
$local_ancestry{$parent} = $child;
}
- } elsif (!$line =~ m/^([a-f0-9]+)/) {
+ } elsif (!$line =~ /^([a-f0-9]+)/) {
die "Unexpected output from git rev-list:
$line";
}
}
@@ -1190,10 +1190,10 @@ sub mw_push_revision {
# history (linearized with --first-parent)
print STDERR "Warning: no common ancestor, pushing complete
history\n";
my $history = run_git("rev-list --first-parent --children
$local");
- my @history = split('\n', $history);
+ my @history = split(/\n/, $history);
@history = @history[1..$#history];
foreach my $line (reverse @history) {
- my @commit_info_split = split(/ |\n/, $line);
+ my @commit_info_split = split(/[ \n]/, $line);
push(@commit_pairs, \@commit_info_split);
}
}
@@ -1272,7 +1272,7 @@ sub get_mw_namespace_id {
# Look at configuration file, if the record for that namespace
is
# already cached. Namespaces are stored in form:
# "Name_of_namespace:Id_namespace", ex.: "File:6".
- my @temp = split(/[\n]/, run_git("config --get-all remote."
+ my @temp = split(/\n/, run_git("config --get-all remote."
. $remotename
.".namespaceCache"));
chomp(@temp);
foreach my $ns (@temp) {
--
1.7.9.5
--
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 20/22] git-remote-mediawiki: Put non-trivial numeric values in constants.
Non-trivial numeric values (e.g., different from 0, 1 and 2) are placed in
constants at the top of the code to be easily modifiable and to make more sense
Signed-off-by: Célestin Matte
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-remote-mediawiki.perl | 20 ++--
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index e6dc6db..94a0411 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -43,6 +43,16 @@ Readonly my $EMPTY_MESSAGE => '*Empty MediaWiki Message*';
Readonly my $EMPTY => q{};
+# Number of pages taken into account at once in submodule get_mw_page_list
+Readonly my $SLICE_SIZE => 50;
+
+# Number of linked mediafile to get at once in get_linked_mediafiles
+# The query is split in small batches because of the MW API limit of
+# the number of links to be returned (500 links max).
+Readonly my $BATCH_SIZE => 10;
+
+Readonly my $HTTP_CODE_OK => 200;
+
if (@ARGV != 2) {
exit_error_usage();
}
@@ -226,13 +236,13 @@ sub get_mw_page_list {
my $pages = shift;
my @some_pages = @$page_list;
while (@some_pages) {
- my $last_page = 50;
+ my $last_page = $SLICE_SIZE;
if ($#some_pages < $last_page) {
$last_page = $#some_pages;
}
my @slice = @some_pages[0..$last_page];
get_mw_first_pages(\@slice, $pages);
- @some_pages = @some_pages[51..$#some_pages];
+ @some_pages = @some_pages[($SLICE_SIZE + 1)..$#some_pages];
}
return;
}
@@ -388,9 +398,7 @@ sub get_linked_mediafiles {
my $pages = shift;
my @titles = map { $_->{title} } values(%{$pages});
- # The query is split in small batches because of the MW API limit of
- # the number of links to be returned (500 links max).
- my $batch = 10;
+ my $batch = $BATCH_SIZE;
while (@titles) {
if ($#titles < $batch) {
$batch = $#titles;
@@ -472,7 +480,7 @@ sub download_mw_mediafile {
my $download_url = shift;
my $response = $mediawiki->{ua}->get($download_url);
- if ($response->code == 200) {
+ if ($response->code == $HTTP_CODE_OK) {
return $response->decoded_content;
} else {
print {*STDERR} "Error downloading mediafile from :\n";
--
1.7.9.5
--
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 14/22] git-remote-mediawiki: Check return value of open + remove import of unused open2
Signed-off-by: Célestin Matte
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-remote-mediawiki.perl |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index ae6dd2e..1c34ada 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -22,7 +22,6 @@ binmode STDERR, ":encoding(UTF-8)";
binmode STDOUT, ":encoding(UTF-8)";
use URI::Escape;
-use IPC::Open2;
use Readonly;
# Mediawiki filenames can contain forward slashes. This variable decides by
which pattern they should be replaced
@@ -338,7 +337,8 @@ sub get_mw_pages {
sub run_git {
my $args = shift;
my $encoding = (shift || "encoding(UTF-8)");
- open(my $git, "-|:$encoding", "git " . $args);
+ open(my $git, "-|:$encoding", "git " . $args)
+ or die "Unable to open: $!\n";
my $res = do {
local $/ = undef;
<$git>
--
1.7.9.5
--
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 12/22] git-remote-mediawiki: Rename a variable ($last) which has the name of a keyword
Signed-off-by: Célestin Matte
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-remote-mediawiki.perl |8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index cf8dfc8..7fbc998 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -215,11 +215,11 @@ sub get_mw_page_list {
my $pages = shift;
my @some_pages = @$page_list;
while (@some_pages) {
- my $last = 50;
- if ($#some_pages < $last) {
- $last = $#some_pages;
+ my $last_page = 50;
+ if ($#some_pages < $last_page) {
+ $last_page = $#some_pages;
}
- my @slice = @some_pages[0..$last];
+ my @slice = @some_pages[0..$last_page];
get_mw_first_pages(\@slice, $pages);
@some_pages = @some_pages[51..$#some_pages];
}
--
1.7.9.5
--
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 15/22] git-remote-mediawiki: Put long code into a subroutine
Signed-off-by: Célestin Matte
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-remote-mediawiki.perl | 42 +--
1 file changed, 26 insertions(+), 16 deletions(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index 1c34ada..f37488b 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -133,22 +133,7 @@ while () {
@cmd = split(/ /);
if (defined($cmd[0])) {
# Line not blank
- if ($cmd[0] eq "capabilities") {
- die("Too many arguments for capabilities\n") if
(defined($cmd[1]));
- mw_capabilities();
- } elsif ($cmd[0] eq "list") {
- die("Too many arguments for list\n") if
(defined($cmd[2]));
- mw_list($cmd[1]);
- } elsif ($cmd[0] eq "import") {
- die("Invalid arguments for import\n") if ($cmd[1] eq ""
|| defined($cmd[2]));
- mw_import($cmd[1]);
- } elsif ($cmd[0] eq "option") {
- die("Too many arguments for option\n") if ($cmd[1] eq
"" || $cmd[2] eq "" || defined($cmd[3]));
- mw_option($cmd[1],$cmd[2]);
- } elsif ($cmd[0] eq "push") {
- mw_push($cmd[1]);
- } else {
- print STDERR "Unknown command. Aborting...\n";
+ if (parse_commands() == 1) {
last;
}
} else {
@@ -168,6 +153,31 @@ sub exit_error_usage {
die "ERROR: git-remote-mediawiki module was not called with a correct
number of parameters\n";
}
+sub parse_commands {
+ if ($cmd[0] eq "capabilities") {
+ die("Too many arguments for capabilities\n")
+ if (defined($cmd[1]));
+ mw_capabilities();
+ } elsif ($cmd[0] eq "list") {
+ die("Too many arguments for list\n") if (defined($cmd[2]));
+ mw_list($cmd[1]);
+ } elsif ($cmd[0] eq "import") {
+ die("Invalid arguments for import\n")
+ if ($cmd[1] eq "" || defined($cmd[2]));
+ mw_import($cmd[1]);
+ } elsif ($cmd[0] eq "option") {
+ die("Too many arguments for option\n")
+ if ($cmd[1] eq "" || $cmd[2] eq "" || defined($cmd[3]));
+ mw_option($cmd[1],$cmd[2]);
+ } elsif ($cmd[0] eq "push") {
+ mw_push($cmd[1]);
+ } else {
+ print STDERR "Unknown command. Aborting...\n";
+ return 1;
+ }
+ return 0;
+}
+
# MediaWiki API instance, created lazily.
my $mediawiki;
--
1.7.9.5
--
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 21/22] git-remote-mediawiki: Fix a typo ("mediwiki" instead of "mediawiki")
Signed-off-by: Célestin Matte
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-remote-mediawiki.perl |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index 94a0411..be860c8 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -1093,14 +1093,14 @@ sub mw_push_file {
# edit conflicts, considered as non-fast-forward
print {*STDERR} 'Warning: Error ' .
$mediawiki->{error}->{code} .
- ' from mediwiki: ' .
$mediawiki->{error}->{details} .
+ ' from mediawiki: ' .
$mediawiki->{error}->{details} .
".\n";
return ($oldrevid, 'non-fast-forward');
} else {
# Other errors. Shouldn't happen => just die()
die 'Fatal: Error ' .
$mediawiki->{error}->{code} .
- ' from mediwiki: ' .
$mediawiki->{error}->{details} . "\n";
+ ' from mediawiki: ' .
$mediawiki->{error}->{details} . "\n";
}
}
$newrevid = $result->{edit}->{newrevid};
--
1.7.9.5
--
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 03/22] git-remote-mediawiki: Always end a subroutine with a return
Follow Subroutines::RequireFinalReturn
Signed-off-by: Célestin Matte
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-remote-mediawiki.perl | 18 ++
1 file changed, 18 insertions(+)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index e60793a..bbde9fd 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -198,12 +198,14 @@ sub mw_connect_maybe {
exit 1;
}
}
+ return;
}
## Functions for listing pages on the remote wiki
sub get_mw_tracked_pages {
my $pages = shift;
get_mw_page_list(\@tracked_pages, $pages);
+ return;
}
sub get_mw_page_list {
@@ -219,6 +221,7 @@ sub get_mw_page_list {
get_mw_first_pages(\@slice, $pages);
@some_pages = @some_pages[51..$#some_pages];
}
+ return;
}
sub get_mw_tracked_categories {
@@ -241,6 +244,7 @@ sub get_mw_tracked_categories {
$pages->{$page->{title}} = $page;
}
}
+ return;
}
sub get_mw_all_pages {
@@ -260,6 +264,7 @@ sub get_mw_all_pages {
foreach my $page (@{$mw_pages}) {
$pages->{$page->{title}} = $page;
}
+ return;
}
# queries the wiki for a set of pages. Meant to be used within a loop
@@ -290,6 +295,7 @@ sub get_mw_first_pages {
$pages->{$page->{title}} = $page;
}
}
+ return;
}
# Get the list of pages to be fetched according to configuration.
@@ -358,6 +364,7 @@ sub get_all_mediafiles {
foreach my $page (@{$mw_pages}) {
$pages->{$page->{title}} = $page;
}
+ return;
}
sub get_linked_mediafiles {
@@ -404,6 +411,7 @@ sub get_linked_mediafiles {
@titles = @titles[($batch+1)..$#titles];
}
+ return;
}
sub get_mw_mediafile_for_page_revision {
@@ -579,6 +587,7 @@ sub mediawiki_smudge_filename {
sub literal_data {
my ($content) = @_;
print STDOUT "data ", bytes::length($content), "\n", $content;
+ return;
}
sub literal_data_raw {
@@ -589,6 +598,7 @@ sub literal_data_raw {
binmode STDOUT, ":raw";
print STDOUT "data ", bytes::length($content), "\n", $content;
binmode STDOUT, ":encoding(UTF-8)";
+ return;
}
sub mw_capabilities {
@@ -600,6 +610,7 @@ sub mw_capabilities {
print STDOUT "list\n";
print STDOUT "push\n";
print STDOUT "\n";
+ return;
}
sub mw_list {
@@ -608,11 +619,13 @@ sub mw_list {
print STDOUT "? refs/heads/master\n";
print STDOUT "\@refs/heads/master HEAD\n";
print STDOUT "\n";
+ return;
}
sub mw_option {
print STDERR "remote-helper command 'option $_[0]' not yet
implemented\n";
print STDOUT "unsupported\n";
+ return;
}
sub fetch_mw_revisions_for_page {
@@ -734,6 +747,7 @@ sub import_file_revision {
print STDOUT "N inline :$n\n";
literal_data("mediawiki_revision: " . $commit{mw_revision});
print STDOUT "\n\n";
+ return;
}
# parse a sequence of
@@ -754,6 +768,7 @@ sub get_more_refs {
die("Invalid command in a '$cmd' batch: ". $_);
}
}
+ return;
}
sub mw_import {
@@ -763,6 +778,7 @@ sub mw_import {
mw_import_ref($ref);
}
print STDOUT "done\n";
+ return;
}
sub mw_import_ref {
@@ -806,6 +822,7 @@ sub mw_import_ref {
# thrown saying that HEAD is referring to unknown object
000
# and the clone fails.
}
+ return;
}
sub mw_import_ref_by_pages {
@@ -1112,6 +1129,7 @@ sub mw_push {
print STDERR " git pull --rebase\n";
print STDERR "\n";
}
+ return;
}
sub mw_push_revision {
--
1.7.9.5
--
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 08/22] git-remote-mediawiki: Add newline in the end of die() error messages
Signed-off-by: Célestin Matte
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-remote-mediawiki.perl | 26 +-
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index 482cd95..4baad95 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -136,16 +136,16 @@ while () {
if (defined($cmd[0])) {
# Line not blank
if ($cmd[0] eq "capabilities") {
- die("Too many arguments for capabilities") unless
(!defined($cmd[1]));
+ die("Too many arguments for capabilities\n") unless
(!defined($cmd[1]));
mw_capabilities();
} elsif ($cmd[0] eq "list") {
- die("Too many arguments for list") unless
(!defined($cmd[2]));
+ die("Too many arguments for list\n") unless
(!defined($cmd[2]));
mw_list($cmd[1]);
} elsif ($cmd[0] eq "import") {
- die("Invalid arguments for import") unless ($cmd[1] ne
"" && !defined($cmd[2]));
+ die("Invalid arguments for import\n") unless ($cmd[1]
ne "" && !defined($cmd[2]));
mw_import($cmd[1]);
} elsif ($cmd[0] eq "option") {
- die("Too many arguments for option") unless ($cmd[1] ne
"" && $cmd[2] ne "" && !defined($cmd[3]));
+ die("Too many arguments for option\n") unless ($cmd[1]
ne "" && $cmd[2] ne "" && !defined($cmd[3]));
mw_option($cmd[1],$cmd[2]);
} elsif ($cmd[0] eq "push") {
mw_push($cmd[1]);
@@ -242,7 +242,7 @@ sub get_mw_tracked_categories {
cmtitle => $category,
cmlimit => 'max' } )
|| die $mediawiki->{error}->{code} . ': '
- . $mediawiki->{error}->{details};
+ . $mediawiki->{error}->{details} . "\n";
foreach my $page (@{$mw_pages}) {
$pages->{$page->{title}} = $page;
}
@@ -767,7 +767,7 @@ sub get_more_refs {
} elsif ($line eq "\n") {
return @refs;
} else {
- die("Invalid command in a '$cmd' batch: ". $_);
+ die("Invalid command in a '$cmd' batch: $_\n");
}
}
return;
@@ -879,7 +879,7 @@ sub mw_import_revids {
my $result = $mediawiki->api($query);
if (!$result) {
- die "Failed to retrieve modified page for revision
$pagerevid";
+ die "Failed to retrieve modified page for revision
$pagerevid\n";
}
if (defined($result->{query}->{badrevids}->{$pagerevid})) {
@@ -888,7 +888,7 @@ sub mw_import_revids {
}
if (!defined($result->{query}->{pages})) {
- die "Invalid revision $pagerevid.";
+ die "Invalid revision $pagerevid.\n";
}
my @result_pages = values(%{$result->{query}->{pages}});
@@ -999,7 +999,7 @@ sub mw_upload_file {
}, {
skip_encoding => 1
} ) || die $mediawiki->{error}->{code} . ':'
-. $mediawiki->{error}->{details};
+. $mediawiki->{error}->{details} . "\n";
my $last_file_page = $mediawiki->get_page({title =>
$path});
$newrevid = $last_file_page->{revid};
print STDERR "Pushed file: $new_sha1 -
$complete_file_name.\n";
@@ -1079,7 +1079,7 @@ sub mw_push_file {
# Other errors. Shouldn't happen => just die()
die 'Fatal: Error ' .
$mediawiki->{error}->{code} .
- ' from mediwiki: ' .
$mediawiki->{error}->{details};
+ ' from mediwiki: ' .
$mediawiki->{error}->{details} . "\n";
}
}
$newrevid = $result->{edit}->{newrevid};
@@ -1101,7 +1101,7 @@ sub mw_push {
my $pushed;
for my $refspec (@refsspecs) {
my ($force, $local, $remote) = $refspec =~
/^(\+)?([^:]*):([^:]*)$/
- or die("Invalid refspec for push. Expected : or
+:");
+ or die("Invalid refspec for push. Expected : or
+:\n");
if ($force) {
print STDERR "Warning: forced push not allowed on a
MediaWiki.\n";
}
@@ -1173,7 +1173,7 @@ sub mw_push_revision {
[PATCH v2 19/22] git-remote-mediawiki: Don't use quotes for empty strings
Empty strings are replaced by an $EMPTY constant.
Signed-off-by: Célestin Matte
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-remote-mediawiki.perl | 18 ++
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index 123730f..e6dc6db 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -41,6 +41,8 @@ Readonly my $NULL_SHA1 =>
'';
# Used on Git's side to reflect empty edit messages on the wiki
Readonly my $EMPTY_MESSAGE => '*Empty MediaWiki Message*';
+Readonly my $EMPTY => q{};
+
if (@ARGV != 2) {
exit_error_usage();
}
@@ -163,11 +165,11 @@ sub parse_commands {
mw_list($cmd[1]);
} elsif ($cmd[0] eq 'import') {
die("Invalid arguments for import\n")
- if ($cmd[1] eq "" || defined($cmd[2]));
+ if ($cmd[1] eq $EMPTY || defined($cmd[2]));
mw_import($cmd[1]);
} elsif ($cmd[0] eq 'option') {
die("Too many arguments for option\n")
- if ($cmd[1] eq "" || $cmd[2] eq "" || defined($cmd[3]));
+ if ($cmd[1] eq $EMPTY || $cmd[2] eq $EMPTY ||
defined($cmd[3]));
mw_option($cmd[1],$cmd[2]);
} elsif ($cmd[0] eq 'push') {
mw_push($cmd[1]);
@@ -558,7 +560,7 @@ sub mediawiki_clean {
# Mediawiki does not allow blank space at the end of a page and ends
with a single \n.
# This function right trims a string and adds a \n at the end to follow
this rule
$string =~ s/\s+$//;
- if ($string eq "" && $page_created) {
+ if ($string eq $EMPTY && $page_created) {
# Creating empty pages is forbidden.
$string = $EMPTY_CONTENT;
}
@@ -569,7 +571,7 @@ sub mediawiki_clean {
sub mediawiki_smudge {
my $string = shift;
if ($string eq $EMPTY_CONTENT) {
- $string = "";
+ $string = $EMPTY;
}
# This \n is important. This is due to mediawiki's way to handle end of
files.
return "${string}\n";
@@ -995,7 +997,7 @@ sub mw_upload_file {
} else {
# Don't let perl try to interpret file content as UTF-8 => use
"raw"
my $content = run_git("cat-file blob ${new_sha1}", 'raw');
- if ($content ne "") {
+ if ($content ne $EMPTY) {
mw_connect_maybe();
$mediawiki->{config}->{upload_url} =
"${url}/index.php/Special:Upload";
@@ -1037,7 +1039,7 @@ sub mw_push_file {
my $newrevid;
if ($summary eq $EMPTY_MESSAGE) {
- $summary = '';
+ $summary = $EMPTY;
}
my $new_sha1 = $diff_info_split[3];
@@ -1048,7 +1050,7 @@ sub mw_push_file {
my ($title, $extension) = $complete_file_name =~ /^(.*)\.([^\.]*)$/;
if (!defined($extension)) {
- $extension = "";
+ $extension = $EMPTY;
}
if ($extension eq 'mw') {
my $ns = get_mw_namespace_id_for_page($complete_file_name);
@@ -1116,7 +1118,7 @@ sub mw_push {
if ($force) {
print {*STDERR} "Warning: forced push not allowed on a
MediaWiki.\n";
}
- if ($local eq "") {
+ if ($local eq $EMPTY) {
print {*STDERR} "Cannot delete remote branch on a
MediaWiki\n";
print {*STDOUT} "error ${remote} cannot delete\n";
next;
--
1.7.9.5
--
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 04/22] git-remote-mediawiki: Move a variable declaration at the top of the code
%basetimestamps declaration was lost in the middle of subroutines
Signed-off-by: Célestin Matte
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-remote-mediawiki.perl |6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index bbde9fd..dc8dd1f 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -96,6 +96,9 @@ unless ($fetch_strategy) {
$fetch_strategy = "by_page";
}
+# Remember the timestamp corresponding to a revision id.
+my %basetimestamps;
+
# Dumb push: don't update notes and mediawiki ref to reflect the last push.
#
# Configurable with mediawiki.dumbPush, or per-remote with
@@ -481,9 +484,6 @@ sub get_last_local_revision {
return $lastrevision_number;
}
-# Remember the timestamp corresponding to a revision id.
-my %basetimestamps;
-
# Get the last remote revision without taking in account which pages are
# tracked or not. This function makes a single request to the wiki thus
# avoid a loop onto all tracked pages. This is useful for the fetch-by-rev
--
1.7.9.5
--
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 16/22] git-remote-mediawiki: Modify strings for a better coding-style
- strings which don't need interpolation are single-quoted for more clarity and
slight gain of performance
- interpolation is preferred over concatenation in many cases, for more clarity
- variables are always used with the ${} operator inside strings
- strings including double-quotes are written with qq() so that the quotes do
not have to be escaped
Signed-off-by: Célestin Matte
Signed-off-by: Matthieu Moy
Conflicts:
contrib/mw-to-git/git-remote-mediawiki.perl
---
contrib/mw-to-git/git-remote-mediawiki.perl | 244 +--
1 file changed, 121 insertions(+), 123 deletions(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index f37488b..2d4ea1d 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -18,14 +18,14 @@ use DateTime::Format::ISO8601;
use warnings;
# By default, use UTF-8 to communicate with Git and the user
-binmode STDERR, ":encoding(UTF-8)";
-binmode STDOUT, ":encoding(UTF-8)";
+binmode STDERR, ':encoding(UTF-8)';
+binmode STDOUT, ':encoding(UTF-8)';
use URI::Escape;
use Readonly;
# Mediawiki filenames can contain forward slashes. This variable decides by
which pattern they should be replaced
-Readonly my $SLASH_REPLACEMENT => "%2F";
+Readonly my $SLASH_REPLACEMENT => '%2F';
# It's not always possible to delete pages (may require some
# privileges). Deleted pages are replaced with this content.
@@ -36,7 +36,7 @@ Readonly my $DELETED_CONTENT => "[[Category:Deleted]]\n";
Readonly my $EMPTY_CONTENT => "\n";
# used to reflect file creation or deletion in diff.
-Readonly my $NULL_SHA1 => "";
+Readonly my $NULL_SHA1 => '';
# Used on Git's side to reflect empty edit messages on the wiki
Readonly my $EMPTY_MESSAGE => '*Empty MediaWiki Message*';
@@ -50,35 +50,35 @@ my $url = $ARGV[1];
# Accept both space-separated and multiple keys in config file.
# Spaces should be written as _ anyway because we'll use chomp.
-my @tracked_pages = split(/[ \n]/, run_git("config --get-all remote.".
$remotename .".pages"));
+my @tracked_pages = split(/[ \n]/, run_git("config --get-all
remote.${remotename}.pages"));
chomp(@tracked_pages);
# Just like @tracked_pages, but for MediaWiki categories.
-my @tracked_categories = split(/[ \n]/, run_git("config --get-all remote.".
$remotename .".categories"));
+my @tracked_categories = split(/[ \n]/, run_git("config --get-all
remote.${remotename}.categories"));
chomp(@tracked_categories);
# Import media files on pull
-my $import_media = run_git("config --get --bool remote.". $remotename
.".mediaimport");
+my $import_media = run_git("config --get --bool
remote.${remotename}.mediaimport");
chomp($import_media);
-$import_media = ($import_media eq "true");
+$import_media = ($import_media eq 'true');
# Export media files on push
-my $export_media = run_git("config --get --bool remote.". $remotename
.".mediaexport");
+my $export_media = run_git("config --get --bool
remote.${remotename}.mediaexport");
chomp($export_media);
-$export_media = !($export_media eq "false");
+$export_media = !($export_media eq 'false');
-my $wiki_login = run_git("config --get remote.". $remotename .".mwLogin");
+my $wiki_login = run_git("config --get remote.${remotename}.mwLogin");
# Note: mwPassword is discourraged. Use the credential system instead.
-my $wiki_passwd = run_git("config --get remote.". $remotename .".mwPassword");
-my $wiki_domain = run_git("config --get remote.". $remotename .".mwDomain");
+my $wiki_passwd = run_git("config --get remote.${remotename}.mwPassword");
+my $wiki_domain = run_git("config --get remote.${remotename}.mwDomain");
chomp($wiki_login);
chomp($wiki_passwd);
chomp($wiki_domain);
# Import only last revisions (both for clone and fetch)
-my $shallow_import = run_git("config --get --bool remote.". $remotename
.".shallow");
+my $shallow_import = run_git("config --get --bool
remote.${remotename}.shallow");
chomp($shallow_import);
-$shallow_import = ($shallow_import eq "true");
+$shallow_import = ($shallow_import eq 'true');
# Fetch (clone and pull) by revisions instead of by pages. This behavior
# is more efficient when we have a wiki with lots of pages and we fetch
@@ -86,13 +86,13 @@ $shallow_import = ($shallow_import eq "true");
# Possible values:
# - by_rev: perform one query per new revision on the remote wiki
# - by_page: query each tracked page for new revision
-my $fetch_strategy = run_git("config --get remote.$remotename.fetchStrategy");
+my $fetch_strategy = run_git("config --get
remote.${remotename}.fetchStrategy");
unless ($fetch_strategy) {
- $fetch_strategy = run_git("config --get mediawiki.fetchStrategy");
+ $fetch_strategy = run_git('config --get mediawiki.fetchStrategy');
}
chomp($fetch_strategy);
unless ($fetch_strategy) {
- $fetch_strategy = "by_page"
[PATCH v2 13/22] git-remote-mediawiki: Assign a variable as undef and make proper indentation
Explicitly assign local variable $/ as undef and make a proper
one-instruction-by-line indentation
Signed-off-by: Célestin Matte
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-remote-mediawiki.perl |5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index 7fbc998..ae6dd2e 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -339,7 +339,10 @@ sub run_git {
my $args = shift;
my $encoding = (shift || "encoding(UTF-8)");
open(my $git, "-|:$encoding", "git " . $args);
- my $res = do { local $/; <$git> };
+ my $res = do {
+ local $/ = undef;
+ <$git>
+ };
close($git);
return $res;
--
1.7.9.5
--
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 10/22] git-remote-mediawiki: Turn double-negated expressions into simple expressions
Signed-off-by: Célestin Matte
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-remote-mediawiki.perl |8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index 68fd129..a6c7de2 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -136,16 +136,16 @@ while () {
if (defined($cmd[0])) {
# Line not blank
if ($cmd[0] eq "capabilities") {
- die("Too many arguments for capabilities\n") unless
(!defined($cmd[1]));
+ die("Too many arguments for capabilities\n") if
(defined($cmd[1]));
mw_capabilities();
} elsif ($cmd[0] eq "list") {
- die("Too many arguments for list\n") unless
(!defined($cmd[2]));
+ die("Too many arguments for list\n") if
(defined($cmd[2]));
mw_list($cmd[1]);
} elsif ($cmd[0] eq "import") {
- die("Invalid arguments for import\n") unless ($cmd[1]
ne "" && !defined($cmd[2]));
+ die("Invalid arguments for import\n") if ($cmd[1] eq ""
|| defined($cmd[2]));
mw_import($cmd[1]);
} elsif ($cmd[0] eq "option") {
- die("Too many arguments for option\n") unless ($cmd[1]
ne "" && $cmd[2] ne "" && !defined($cmd[3]));
+ die("Too many arguments for option\n") if ($cmd[1] eq
"" || $cmd[2] eq "" || defined($cmd[3]));
mw_option($cmd[1],$cmd[2]);
} elsif ($cmd[0] eq "push") {
mw_push($cmd[1]);
--
1.7.9.5
--
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 18/22] git-remote-mediawiki: Replace "unless" statements with negated "if" statements
Signed-off-by: Célestin Matte
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-remote-mediawiki.perl | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index 1024de1..123730f 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -87,11 +87,11 @@ $shallow_import = ($shallow_import eq 'true');
# - by_rev: perform one query per new revision on the remote wiki
# - by_page: query each tracked page for new revision
my $fetch_strategy = run_git("config --get
remote.${remotename}.fetchStrategy");
-unless ($fetch_strategy) {
+if (!$fetch_strategy) {
$fetch_strategy = run_git('config --get mediawiki.fetchStrategy');
}
chomp($fetch_strategy);
-unless ($fetch_strategy) {
+if (!$fetch_strategy) {
$fetch_strategy = 'by_page';
}
@@ -113,7 +113,7 @@ my %basetimestamps;
# deterministic, this means everybody gets the same sha1 for each
# MediaWiki revision.
my $dumb_push = run_git("config --get --bool remote.${remotename}.dumbPush");
-unless ($dumb_push) {
+if (!$dumb_push) {
$dumb_push = run_git('config --get --bool mediawiki.dumbPush');
}
chomp($dumb_push);
@@ -670,7 +670,7 @@ sub fetch_mw_revisions_for_page {
push(@page_revs, $page_rev_ids);
$revnum++;
}
- last unless $result->{'query-continue'};
+ last if (!$result->{'query-continue'});
$query->{rvstartid} =
$result->{'query-continue'}->{revisions}->{rvstartid};
}
if ($shallow_import && @page_revs) {
@@ -1242,7 +1242,7 @@ sub mw_push_revision {
die("Unknown error from mw_push_file()\n");
}
}
- unless ($dumb_push) {
+ if (!$dumb_push) {
run_git(qq(notes --ref=${remotename}/mediawiki add -f
-m "mediawiki_revision: ${mw_revision}" ${sha1_commit}));
run_git(qq(update-ref -m "Git-MediaWiki push"
refs/mediawiki/${remotename}/master ${sha1_commit} ${sha1_child}));
}
@@ -1322,7 +1322,7 @@ sub get_mw_namespace_id {
my $ns = $namespace_id{$name};
my $id;
- unless (defined $ns) {
+ if (!defined $ns) {
print {*STDERR} "No such namespace ${name} on MediaWiki.\n";
$ns = {is_namespace => 0};
$namespace_id{$name} = $ns;
--
1.7.9.5
--
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 17/22] git-remote-mediawiki: Brace file handles for print for more clarity
This follows the following rule:
InputOutput::RequireBracedFileHandleWithPrint (Severity: 1)
The `print' and `printf' functions have a unique syntax that supports an
optional file handle argument. Conway suggests wrapping this argument in
braces to make it visually stand out from the other arguments. When you
put braces around any of the special package-level file handles like
`STDOUT', `STDERR', and `DATA', you must the `'*'' sigil or else it
won't compile under `use strict 'subs''.
print $FH "Mary had a little lamb\n"; #not ok
print {$FH} "Mary had a little lamb\n"; #ok
print STDERR $foo, $bar, $baz; #not ok
print {STDERR} $foo, $bar, $baz; #won't compile under 'strict'
print {*STDERR} $foo, $bar, $baz; #perfect!
Signed-off-by: Célestin Matte
Signed-off-by: Matthieu Moy
Conflicts:
contrib/mw-to-git/git-remote-mediawiki.perl
---
contrib/mw-to-git/git-remote-mediawiki.perl | 184 +--
1 file changed, 92 insertions(+), 92 deletions(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index 2d4ea1d..1024de1 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -172,8 +172,8 @@ sub parse_commands {
} elsif ($cmd[0] eq 'push') {
mw_push($cmd[1]);
} else {
- print STDERR "Unknown command. Aborting...\n";
- return 1;
+ print {*STDERR} "Unknown command. Aborting...\n";
+ last;
}
return 0;
}
@@ -199,7 +199,7 @@ sub mw_connect_maybe {
lgdomain => $wiki_domain};
if ($mediawiki->login($request)) {
Git::credential \%credential, 'approve';
- print STDERR qq(Logged in mediawiki user
"$credential{username}".\n);
+ print {*STDERR} qq(Logged in mediawiki user
"$credential{username}".\n);
} else {
print {*STDERR} qq(Failed to log in mediawiki user
"$credential{username}" on ${url}\n);
print {*STDERR} ' (error ' .
@@ -267,9 +267,9 @@ sub get_mw_all_pages {
aplimit => 'max'
});
if (!defined($mw_pages)) {
- print STDERR "fatal: could not get the list of wiki pages.\n";
- print STDERR "fatal: '${url}' does not appear to be a
mediawiki\n";
- print STDERR "fatal: make sure '${url}/api.php' is a valid
page.\n";
+ print {*STDERR} "fatal: could not get the list of wiki
pages.\n";
+ print {*STDERR} "fatal: '${url}' does not appear to be a
mediawiki\n";
+ print {*STDERR} "fatal: make sure '${url}/api.php' is a valid
page.\n";
exit 1;
}
foreach my $page (@{$mw_pages}) {
@@ -294,14 +294,14 @@ sub get_mw_first_pages {
titles => $titles,
});
if (!defined($mw_pages)) {
- print STDERR "fatal: could not query the list of wiki pages.\n";
- print STDERR "fatal: '${url}' does not appear to be a
mediawiki\n";
- print STDERR "fatal: make sure '${url}/api.php' is a valid
page.\n";
+ print {*STDERR} "fatal: could not query the list of wiki
pages.\n";
+ print {*STDERR} "fatal: '${url}' does not appear to be a
mediawiki\n";
+ print {*STDERR} "fatal: make sure '${url}/api.php' is a valid
page.\n";
exit 1;
}
while (my ($id, $page) = each(%{$mw_pages->{query}->{pages}})) {
if ($id < 0) {
- print STDERR "Warning: page $page->{title} not found on
wiki\n";
+ print {*STDERR} "Warning: page $page->{title} not found
on wiki\n";
} else {
$pages->{$page->{title}} = $page;
}
@@ -313,7 +313,7 @@ sub get_mw_first_pages {
sub get_mw_pages {
mw_connect_maybe();
- print STDERR "Listing pages on remote wiki...\n";
+ print {*STDERR} "Listing pages on remote wiki...\n";
my %pages; # hash on page titles to avoid duplicates
my $user_defined;
@@ -331,14 +331,14 @@ sub get_mw_pages {
get_mw_all_pages(\%pages);
}
if ($import_media) {
- print STDERR "Getting media files for selected pages...\n";
+ print {*STDERR} "Getting media files for selected pages...\n";
if ($user_defined) {
get_linked_mediafiles(\%pages);
} else {
get_all_mediafiles(\%pages);
}
}
- print STDERR (scalar keys %pages) . " pages found.\n";
+ print {*STDERR} (scalar keys %pages) . " pages found.\n";
return %pages;
}
@@ -371,9 +371,9 @@ sub get_all_mediafiles {
[PATCH v2 11/22] git-remote-mediawiki: Remove unused variable $entry
Signed-off-by: Célestin Matte
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-remote-mediawiki.perl |1 -
1 file changed, 1 deletion(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index a6c7de2..cf8dfc8 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -128,7 +128,6 @@ $wiki_name =~ s{[^/]*://}{};
$wiki_name =~ s/^.*@//;
# Commands parser
-my $entry;
my @cmd;
while () {
chomp;
--
1.7.9.5
--
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 06/22] git-remote-mediawiki: Rewrite unclear line of instructions
Subroutines' parameters should be affected to variable before doing anything
else
Besides, existing instruction affected a variable inside a "if", which break
Git's coding style
Signed-off-by: Célestin Matte
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-remote-mediawiki.perl |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index 5e348cb..a5c963b 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -1334,7 +1334,8 @@ sub get_mw_namespace_id {
}
sub get_mw_namespace_id_for_page {
- if (my ($namespace) = $_[0] =~ /^([^:]*):/) {
+ my $namespace = shift;
+ if ($namespace =~ /^([^:]*):/) {
return get_mw_namespace_id($namespace);
} else {
return;
--
1.7.9.5
--
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 02/22] git-remote-mediawiki: Use the Readonly module instead of the constant pragma
Follow ValuesAndExpressions::ProhibitConstantPragma
Signed-off-by: Célestin Matte
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-remote-mediawiki.perl | 26 +-
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index 4893442..e60793a 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -26,21 +26,21 @@ use IPC::Open2;
use Readonly;
# Mediawiki filenames can contain forward slashes. This variable decides by
which pattern they should be replaced
-use constant SLASH_REPLACEMENT => "%2F";
+Readonly my $SLASH_REPLACEMENT => "%2F";
# It's not always possible to delete pages (may require some
# privileges). Deleted pages are replaced with this content.
-use constant DELETED_CONTENT => "[[Category:Deleted]]\n";
+Readonly my $DELETED_CONTENT => "[[Category:Deleted]]\n";
# It's not possible to create empty pages. New empty files in Git are
# sent with this content instead.
-use constant EMPTY_CONTENT => "\n";
+Readonly my $EMPTY_CONTENT => "\n";
# used to reflect file creation or deletion in diff.
-use constant NULL_SHA1 => "";
+Readonly my $NULL_SHA1 => "";
# Used on Git's side to reflect empty edit messages on the wiki
-use constant EMPTY_MESSAGE => '*Empty MediaWiki Message*';
+Readonly my $EMPTY_MESSAGE => '*Empty MediaWiki Message*';
if (@ARGV != 2) {
exit_error_usage();
@@ -538,7 +538,7 @@ sub mediawiki_clean {
$string =~ s/\s+$//;
if ($string eq "" && $page_created) {
# Creating empty pages is forbidden.
- $string = EMPTY_CONTENT;
+ $string = $EMPTY_CONTENT;
}
return $string."\n";
}
@@ -546,7 +546,7 @@ sub mediawiki_clean {
# Filter applied on MediaWiki data before adding them to Git
sub mediawiki_smudge {
my $string = shift;
- if ($string eq EMPTY_CONTENT) {
+ if ($string eq $EMPTY_CONTENT) {
$string = "";
}
# This \n is important. This is due to mediawiki's way to handle end of
files.
@@ -707,7 +707,7 @@ sub import_file_revision {
if (!$full_import && $n == 1) {
print STDOUT "from refs/mediawiki/$remotename/master^0\n";
}
- if ($content ne DELETED_CONTENT) {
+ if ($content ne $DELETED_CONTENT) {
print STDOUT "M 644 inline " .
fe_escape_path($title . ".mw") . "\n";
literal_data($content);
@@ -888,7 +888,7 @@ sub mw_import_revids {
my %commit;
$commit{author} = $rev->{user} || 'Anonymous';
- $commit{comment} = $rev->{comment} || EMPTY_MESSAGE;
+ $commit{comment} = $rev->{comment} || $EMPTY_MESSAGE;
$commit{title} = mediawiki_smudge_filename($page_title);
$commit{mw_revision} = $rev->{revid};
$commit{content} = mediawiki_smudge($rev->{'*'});
@@ -1006,14 +1006,14 @@ sub mw_push_file {
my $oldrevid = shift;
my $newrevid;
- if ($summary eq EMPTY_MESSAGE) {
+ if ($summary eq $EMPTY_MESSAGE) {
$summary = '';
}
my $new_sha1 = $diff_info_split[3];
my $old_sha1 = $diff_info_split[2];
- my $page_created = ($old_sha1 eq NULL_SHA1);
- my $page_deleted = ($new_sha1 eq NULL_SHA1);
+ my $page_created = ($old_sha1 eq $NULL_SHA1);
+ my $page_deleted = ($new_sha1 eq $NULL_SHA1);
$complete_file_name = mediawiki_clean_filename($complete_file_name);
my ($title, $extension) = $complete_file_name =~ /^(.*)\.([^\.]*)$/;
@@ -1032,7 +1032,7 @@ sub mw_push_file {
# special privileges. A common
# convention is to replace the page
# with this content instead:
- $file_content = DELETED_CONTENT;
+ $file_content = $DELETED_CONTENT;
} else {
$file_content = run_git("cat-file blob $new_sha1");
}
--
1.7.9.5
--
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 05/22] git-remote-mediawiki: Change syntax of map calls
Put first parameter of map inside a block, for better readability.
Follow BuiltinFunctions::RequireBlockMap
Signed-off-by: Célestin Matte
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-remote-mediawiki.perl | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index dc8dd1f..5e348cb 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -372,7 +372,7 @@ sub get_all_mediafiles {
sub get_linked_mediafiles {
my $pages = shift;
- my @titles = map $_->{title}, values(%{$pages});
+ my @titles = map { $_->{title} } values(%{$pages});
# The query is split in small batches because of the MW API limit of
# the number of links to be returned (500 links max).
@@ -400,11 +400,13 @@ sub get_linked_mediafiles {
while (my ($id, $page) = each(%{$result->{query}->{pages}})) {
my @media_titles;
if (defined($page->{links})) {
- my @link_titles = map $_->{title},
@{$page->{links}};
+ my @link_titles
+ = map { $_->{title} } @{$page->{links}};
push(@media_titles, @link_titles);
}
if (defined($page->{images})) {
- my @image_titles = map $_->{title},
@{$page->{images}};
+ my @image_titles
+ = map { $_->{title} } @{$page->{images}};
push(@media_titles, @image_titles);
}
if (@media_titles) {
@@ -834,7 +836,7 @@ sub mw_import_ref_by_pages {
my ($n, @revisions) = fetch_mw_revisions(\@pages, $fetch_from);
@revisions = sort {$a->{revid} <=> $b->{revid}} @revisions;
- my @revision_ids = map $_->{revid}, @revisions;
+ my @revision_ids = map { $_->{revid} } @revisions;
return mw_import_revids($fetch_from, \@revision_ids, \%pages_hash);
}
@@ -1247,8 +1249,8 @@ sub get_allowed_file_extensions {
siprop => 'fileextensions'
};
my $result = $mediawiki->api($query);
- my @file_extensions= map
$_->{ext},@{$result->{query}->{fileextensions}};
- my %hashFile = map {$_ => 1}@file_extensions;
+ my @file_extensions = map { $_->{ext}}
@{$result->{query}->{fileextensions}};
+ my %hashFile = map { $_ => 1 } @file_extensions;
return %hashFile;
}
--
1.7.9.5
--
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 09/22] git-remote-mediawiki: Change the name of a variable
Local variable $url has the same name as a global variable. Changing the name
of the local variable prevents future possible misunderstanding.
Signed-off-by: Célestin Matte
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-remote-mediawiki.perl |6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index 4baad95..68fd129 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -455,14 +455,14 @@ sub get_mw_mediafile_for_page_revision {
}
sub download_mw_mediafile {
- my $url = shift;
+ my $download_url = shift;
- my $response = $mediawiki->{ua}->get($url);
+ my $response = $mediawiki->{ua}->get($download_url);
if ($response->code == 200) {
return $response->decoded_content;
} else {
print STDERR "Error downloading mediafile from :\n";
- print STDERR "URL: $url\n";
+ print STDERR "URL: $download_url\n";
print STDERR "Server response: " . $response->code . " " .
$response->message . "\n";
exit 1;
}
--
1.7.9.5
--
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 01/22] git-remote-mediawiki: Replace :utf8 by :encoding(UTF-8)
Follow perlcritic's InputOutput::RequireEncodingWithUTF8Layer policy
Signed-off-by: Célestin Matte
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-remote-mediawiki.perl |6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index 7b61e73..4893442 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -18,8 +18,8 @@ use DateTime::Format::ISO8601;
use warnings;
# By default, use UTF-8 to communicate with Git and the user
-binmode STDERR, ":utf8";
-binmode STDOUT, ":utf8";
+binmode STDERR, ":encoding(UTF-8)";
+binmode STDOUT, ":encoding(UTF-8)";
use URI::Escape;
use IPC::Open2;
@@ -588,7 +588,7 @@ sub literal_data_raw {
utf8::downgrade($content);
binmode STDOUT, ":raw";
print STDOUT "data ", bytes::length($content), "\n", $content;
- binmode STDOUT, ":utf8";
+ binmode STDOUT, ":encoding(UTF-8)";
}
sub mw_capabilities {
--
1.7.9.5
--
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 00/22] git-remote-mediawiki: Follow perlcritic's recommandations
v2 of patch to follow perlcritic's recommandations ([1])
Changes with v1:
- split first commit into 6 different commits
- remove commit [17/18] about moving open() call
- took every other comment into account
[1]: http://thread.gmane.org/gmane.comp.version-control.git/226533
Célestin Matte (22):
git-remote-mediawiki: Replace :utf8 by :encoding(UTF-8)
git-remote-mediawiki: Use the Readonly module instead of the constant
pragma
git-remote-mediawiki: Always end a subroutine with a return
git-remote-mediawiki: Move a variable declaration at the top of the
code
git-remote-mediawiki: Change syntax of map calls
git-remote-mediawiki: Rewrite unclear line of instructions
git-remote-mediawiki: Change style of some regular expressions
git-remote-mediawiki: Add newline in the end of die() error messages
git-remote-mediawiki: Change the name of a variable
git-remote-mediawiki: Turn double-negated expressions into simple
expressions
git-remote-mediawiki: Remove unused variable $entry
git-remote-mediawiki: Rename a variable ($last) which has the name of
a keyword
git-remote-mediawiki: Assign a variable as undef and make proper
indentation
git-remote-mediawiki: Check return value of open + remove import of
unused open2
git-remote-mediawiki: Put long code into a subroutine
git-remote-mediawiki: Modify strings for a better coding-style
git-remote-mediawiki: Brace file handles for print for more clarity
git-remote-mediawiki: Replace "unless" statements with negated "if"
statements
git-remote-mediawiki: Don't use quotes for empty strings
git-remote-mediawiki: Put non-trivial numeric values in constants.
git-remote-mediawiki: Fix a typo ("mediwiki" instead of "mediawiki")
git-remote-mediawiki: Clearly rewrite double dereference
contrib/mw-to-git/git-remote-mediawiki.perl | 545 ++-
1 file changed, 293 insertions(+), 252 deletions(-)
--
1.7.9.5
--
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 04/10] tests: use test_ln_s_add to remove SYMLINKS prerequisite (trivial cases)
There are many instances where the treatment of symbolic links in the object model and the algorithms are tested, but where it is not necessary to actually have a symbolic link in the worktree. Make adjustments to the tests and remove the SYMLINKS prerequisite when appropriate in trivial cases, where "trivial" means: - merely a replacement of 'ln -s a b && git add b' by test_ln_s_add is needed; - a test for symbolic link on the file system can be split off (and remains protected by SYMLINKS); - existing code is equivalent to test_ln_s_add. Signed-off-by: Johannes Sixt --- t/t1004-read-tree-m-u-wf.sh| 7 +++--- t/t2001-checkout-cache-clash.sh| 7 +++--- t/t2004-checkout-cache-temp.sh | 5 ++--- t/t2007-checkout-symlink.sh| 12 +-- t/t2021-checkout-overwrite.sh | 12 +++ t/t2200-add-update.sh | 5 ++--- t/t3010-ls-files-killed-modified.sh| 9 ++-- t/t3700-add.sh | 15 ++--- t/t3903-stash.sh | 39 -- t/t4008-diff-break-rewrite.sh | 12 +-- t/t4030-diff-textconv.sh | 8 +++ t/t4115-apply-symlink.sh | 10 - t/t4122-apply-symlink-inside.sh| 8 +++ t/t7001-mv.sh | 18 ++-- t/t7607-merge-overwrite.sh | 5 ++--- t/t8006-blame-textconv.sh | 14 +--- t/t8007-cat-file-textconv.sh | 10 - t/t9350-fast-export.sh | 5 ++--- t/t9500-gitweb-standalone-no-errors.sh | 15 + 19 files changed, 106 insertions(+), 110 deletions(-) diff --git a/t/t1004-read-tree-m-u-wf.sh b/t/t1004-read-tree-m-u-wf.sh index b3ae7d5..3e72aff 100755 --- a/t/t1004-read-tree-m-u-wf.sh +++ b/t/t1004-read-tree-m-u-wf.sh @@ -158,7 +158,7 @@ test_expect_success '3-way not overwriting local changes (their side)' ' ' -test_expect_success SYMLINKS 'funny symlink in work tree' ' +test_expect_success 'funny symlink in work tree' ' git reset --hard && git checkout -b sym-b side-b && @@ -170,15 +170,14 @@ test_expect_success SYMLINKS 'funny symlink in work tree' ' rm -fr a && git checkout -b sym-a side-a && mkdir -p a && - ln -s ../b a/b && - git add a/b && + test_ln_s_add ../b a/b && git commit -m "we add a/b" && read_tree_u_must_succeed -m -u sym-a sym-a sym-b ' -test_expect_success SYMLINKS,SANITY 'funny symlink in work tree, un-unlink-able' ' +test_expect_success SANITY 'funny symlink in work tree, un-unlink-able' ' rm -fr a b && git reset --hard && diff --git a/t/t2001-checkout-cache-clash.sh b/t/t2001-checkout-cache-clash.sh index 98aa73e..1fc8e63 100755 --- a/t/t2001-checkout-cache-clash.sh +++ b/t/t2001-checkout-cache-clash.sh @@ -59,10 +59,9 @@ test_expect_success \ 'git read-tree -m $tree1 && git checkout-index -f -a' test_debug 'show_files $tree1' -test_expect_success SYMLINKS \ -'git update-index --add a symlink.' \ -'ln -s path0 path1 && - git update-index --add path1' +test_expect_success \ +'add a symlink' \ +'test_ln_s_add path0 path1' test_expect_success \ 'writing tree out with git write-tree' \ 'tree3=$(git write-tree)' diff --git a/t/t2004-checkout-cache-temp.sh b/t/t2004-checkout-cache-temp.sh index 0f4b289..f171a55 100755 --- a/t/t2004-checkout-cache-temp.sh +++ b/t/t2004-checkout-cache-temp.sh @@ -194,11 +194,10 @@ test_expect_success \ test $(cat ../$s1) = tree1asubdir/path5) )' -test_expect_success SYMLINKS \ +test_expect_success \ 'checkout --temp symlink' ' rm -f path* .merge_* out .git/index && -ln -s b a && -git update-index --add a && +test_ln_s_add b a && t4=$(git write-tree) && rm -f .git/index && git read-tree $t4 && diff --git a/t/t2007-checkout-symlink.sh b/t/t2007-checkout-symlink.sh index e6f59f1..fc9aad5 100755 --- a/t/t2007-checkout-symlink.sh +++ b/t/t2007-checkout-symlink.sh @@ -6,7 +6,7 @@ test_description='git checkout to switch between branches with symlink<->dir' . ./test-lib.sh -test_expect_success SYMLINKS setup ' +test_expect_success setup ' mkdir frotz && echo hello >frotz/filfre && @@ -25,25 +25,25 @@ test_expect_success SYMLINKS setup ' git rm --cached frotz/filfre && mv frotz xyzzy && - ln -s xyzzy frotz && - git add xyzzy/filfre frotz && + test_ln_s_add xyzzy frotz && + git add xyzzy/filfre && test_tick && git commit -m "side moves frotz/ to xyzzy/ and adds frotz->xyzzy/" ' -test_expect_success SYMLINKS 'switch from symlink to dir' ' +test_expect_success 'switch from symlink to dir' ' git checkout master ' -test_expect_success SYMLINKS 'Remove temporary directories & switch to master' ' +test_expect_success 'Remove temporary directories & switch to master' ' rm -fr frotz
[PATCH v2 10/10] t4011: remove SYMLINKS prerequisite
The part of the test that is about symbolic links in the index does not require that the corresponding file system entry is actually a symbolic link. Use test_ln_s_add to insert a symbolic link in the index. When the file system does not support symbolic links, we actually have a regular file in the worktree, which we can update as if it were a symbolic link. diff-index picks up the symbolic link property from the index. Signed-off-by: Johannes Sixt --- t/t4011-diff-symlink.sh | 35 +-- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/t/t4011-diff-symlink.sh b/t/t4011-diff-symlink.sh index f0d5041..13e7f62 100755 --- a/t/t4011-diff-symlink.sh +++ b/t/t4011-diff-symlink.sh @@ -9,7 +9,7 @@ test_description='Test diff of symlinks. . ./test-lib.sh . "$TEST_DIRECTORY"/diff-lib.sh -test_expect_success SYMLINKS 'diff new symlink and file' ' +test_expect_success 'diff new symlink and file' ' cat >expected <<-\EOF && diff --git a/frotz b/frotz new file mode 12 @@ -27,22 +27,25 @@ test_expect_success SYMLINKS 'diff new symlink and file' ' @@ -0,0 +1 @@ +xyzzy EOF - ln -s xyzzy frotz && - echo xyzzy >nitfol && + + # the empty tree git update-index && tree=$(git write-tree) && - git update-index --add frotz nitfol && + + test_ln_s_add xyzzy frotz && + echo xyzzy >nitfol && + git update-index --add nitfol && GIT_DIFF_OPTS=--unified=0 git diff-index -M -p $tree >current && compare_diff_patch expected current ' -test_expect_success SYMLINKS 'diff unchanged symlink and file' ' +test_expect_success 'diff unchanged symlink and file' ' tree=$(git write-tree) && git update-index frotz nitfol && test -z "$(git diff-index --name-only $tree)" ' -test_expect_success SYMLINKS 'diff removed symlink and file' ' +test_expect_success 'diff removed symlink and file' ' cat >expected <<-\EOF && diff --git a/frotz b/frotz deleted file mode 12 @@ -66,12 +69,18 @@ test_expect_success SYMLINKS 'diff removed symlink and file' ' compare_diff_patch expected current ' -test_expect_success SYMLINKS 'diff identical, but newly created symlink and file' ' +test_expect_success 'diff identical, but newly created symlink and file' ' >expected && rm -f frotz nitfol && echo xyzzy >nitfol && test-chmtime +10 nitfol && - ln -s xyzzy frotz && + if test_have_prereq SYMLINKS + then + ln -s xyzzy frotz + else + printf xyzzy >frotz + # the symlink property propagates from the index + fi && git diff-index -M -p $tree >current && compare_diff_patch expected current && @@ -80,7 +89,7 @@ test_expect_success SYMLINKS 'diff identical, but newly created symlink and file compare_diff_patch expected current ' -test_expect_success SYMLINKS 'diff different symlink and file' ' +test_expect_success 'diff different symlink and file' ' cat >expected <<-\EOF && diff --git a/frotz b/frotz index 7c465af..df1db54 12 @@ -100,7 +109,13 @@ test_expect_success SYMLINKS 'diff different symlink and file' ' +yxyyz EOF rm -f frotz && - ln -s yxyyz frotz && + if test_have_prereq SYMLINKS + then + ln -s yxyyz frotz + else + printf yxyyz >frotz + # the symlink property propagates from the index + fi && echo yxyyz >nitfol && git diff-index -M -p $tree >current && compare_diff_patch expected current -- 1.8.3.rc1.32.g8b61cbb -- 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 03/10] tests: introduce test_ln_s_add
Add a new function that creates a symbolic link and adds it to the index
to be used in cases where a symbolic link is not required on the file
system. We will use it to remove many SYMLINKS prerequisites from test
cases.
Signed-off-by: Johannes Sixt
---
t/README| 14 ++
t/test-lib-functions.sh | 17 +
2 files changed, 31 insertions(+)
diff --git a/t/README b/t/README
index e669bb3..bbe25c3 100644
--- a/t/README
+++ b/t/README
@@ -592,6 +592,20 @@ library for your script to use.
test_cmp expected actual
'
+ - test_ln_s_add
+
+ This function helps systems whose filesystem does not support symbolic
+ links. Use it to add a symbolic link entry to the index when it is not
+ important that the file system entry is a symbolic link, i.e., instead
+ of the sequence
+
+ ln -s foo bar &&
+ git add bar
+
+ Sometimes it is possible to split a test in a part that does not need
+ the symbolic link in the file system and a part that does; then only
+ the latter part need be protected by a SYMLINKS prerequisite (see below).
+
Prerequisites
-
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 5251009..fac9234 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -679,3 +679,20 @@ test_create_repo () {
mv .git/hooks .git/hooks-disabled
) || exit
}
+
+# This function helps on symlink challenged file systems when it is not
+# important that the file system entry is a symbolic link.
+# Use test_ln_s_add instead of "ln -s x y && git add y" to add a
+# symbolic link entry y to the index.
+
+test_ln_s_add () {
+ if test_have_prereq SYMLINKS
+ then
+ ln -s "$1" "$2" &&
+ git update-index --add "$2"
+ else
+ printf '%s' "$1" >"$2" &&
+ ln_s_obj=$(git hash-object -w "$2") &&
+ git update-index --add --cacheinfo 12 $ln_s_obj "$2"
+ fi
+}
--
1.8.3.rc1.32.g8b61cbb
--
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 09/10] t6035: use test_ln_s_add to remove SYMLINKS prerequisite
All tests in t6035 are protected by SYMLINKS. But that is not necessary, because a lot of the functionality can be tested provided symbolic link entries enter the index and object data base. Use test_ln_s_add for this purpose. Some test cases do test the presence of symbolic links on the file system. Move these tests into separate test cases that remain protected by SYMLINKS. There is one instance of expect_failure. There is a possibility that this test case fails differently depending on whether SYMLINKS is present or not; but this is not the case. Signed-off-by: Johannes Sixt --- t/t6035-merge-dir-to-symlink.sh | 73 ++--- 1 file changed, 47 insertions(+), 26 deletions(-) diff --git a/t/t6035-merge-dir-to-symlink.sh b/t/t6035-merge-dir-to-symlink.sh index 2599ae5..9324ea4 100755 --- a/t/t6035-merge-dir-to-symlink.sh +++ b/t/t6035-merge-dir-to-symlink.sh @@ -3,7 +3,7 @@ test_description='merging when a directory was replaced with a symlink' . ./test-lib.sh -test_expect_success SYMLINKS 'create a commit where dir a/b changed to symlink' ' +test_expect_success 'create a commit where dir a/b changed to symlink' ' mkdir -p a/b/c a/b-2/c && > a/b/c/d && > a/b-2/c/d && @@ -12,12 +12,12 @@ test_expect_success SYMLINKS 'create a commit where dir a/b changed to symlink' git commit -m base && git tag start && rm -rf a/b && - ln -s b-2 a/b && git add -A && + test_ln_s_add b-2 a/b && git commit -m "dir to symlink" ' -test_expect_success SYMLINKS 'checkout does not clobber untracked symlink' ' +test_expect_success 'checkout does not clobber untracked symlink' ' git checkout HEAD^0 && git reset --hard master && git rm --cached a/b && @@ -25,7 +25,7 @@ test_expect_success SYMLINKS 'checkout does not clobber untracked symlink' ' test_must_fail git checkout start^0 ' -test_expect_success SYMLINKS 'a/b-2/c/d is kept when clobbering symlink b' ' +test_expect_success 'a/b-2/c/d is kept when clobbering symlink b' ' git checkout HEAD^0 && git reset --hard master && git rm --cached a/b && @@ -34,14 +34,14 @@ test_expect_success SYMLINKS 'a/b-2/c/d is kept when clobbering symlink b' ' test -f a/b-2/c/d ' -test_expect_success SYMLINKS 'checkout should not have deleted a/b-2/c/d' ' +test_expect_success 'checkout should not have deleted a/b-2/c/d' ' git checkout HEAD^0 && git reset --hard master && git checkout start^0 && test -f a/b-2/c/d ' -test_expect_success SYMLINKS 'setup for merge test' ' +test_expect_success 'setup for merge test' ' git reset --hard && test -f a/b-2/c/d && echo x > a/x && @@ -50,39 +50,51 @@ test_expect_success SYMLINKS 'setup for merge test' ' git tag baseline ' -test_expect_success SYMLINKS 'Handle D/F conflict, do not lose a/b-2/c/d in merge (resolve)' ' +test_expect_success 'Handle D/F conflict, do not lose a/b-2/c/d in merge (resolve)' ' git reset --hard && git checkout baseline^0 && git merge -s resolve master && - test -h a/b && test -f a/b-2/c/d ' -test_expect_success SYMLINKS 'Handle D/F conflict, do not lose a/b-2/c/d in merge (recursive)' ' +test_expect_success SYMLINKS 'a/b was resolved as symlink' ' + test -h a/b +' + +test_expect_success 'Handle D/F conflict, do not lose a/b-2/c/d in merge (recursive)' ' git reset --hard && git checkout baseline^0 && git merge -s recursive master && - test -h a/b && test -f a/b-2/c/d ' -test_expect_success SYMLINKS 'Handle F/D conflict, do not lose a/b-2/c/d in merge (resolve)' ' +test_expect_success SYMLINKS 'a/b was resolved as symlink' ' + test -h a/b +' + +test_expect_success 'Handle F/D conflict, do not lose a/b-2/c/d in merge (resolve)' ' git reset --hard && git checkout master^0 && git merge -s resolve baseline^0 && - test -h a/b && test -f a/b-2/c/d ' -test_expect_success SYMLINKS 'Handle F/D conflict, do not lose a/b-2/c/d in merge (recursive)' ' +test_expect_success SYMLINKS 'a/b was resolved as symlink' ' + test -h a/b +' + +test_expect_success 'Handle F/D conflict, do not lose a/b-2/c/d in merge (recursive)' ' git reset --hard && git checkout master^0 && git merge -s recursive baseline^0 && - test -h a/b && test -f a/b-2/c/d ' -test_expect_failure SYMLINKS 'do not lose untracked in merge (resolve)' ' +test_expect_success SYMLINKS 'a/b was resolved as symlink' ' + test -h a/b +' + +test_expect_failure 'do not lose untracked in merge (resolve)' ' git reset --hard && git checkout baseline^0 && >a/b/c/e && @@ -91,7 +103,7 @@ test_expect_failure SYMLINKS 'do not lose untracked in merge (resolve)' ' test -f a/b-2/c/d ' -test_expect_success SYMLINKS 'do not lose
[PATCH v2 07/10] t3100: use test_ln_s_add to remove SYMLINKS prerequisite
This undoes the special casing introduced in this test by 704a3143
(Use prerequisite tags to skip tests that depend on symbolic links,
2009-03-04).
Signed-off-by: Johannes Sixt
---
t/t3100-ls-tree-restrict.sh | 42 +++---
1 file changed, 15 insertions(+), 27 deletions(-)
diff --git a/t/t3100-ls-tree-restrict.sh b/t/t3100-ls-tree-restrict.sh
index 81d90b6..eb73c06 100755
--- a/t/t3100-ls-tree-restrict.sh
+++ b/t/t3100-ls-tree-restrict.sh
@@ -22,20 +22,8 @@ test_expect_success \
'setup' \
'mkdir path2 path2/baz &&
echo Hi >path0 &&
- if test_have_prereq SYMLINKS
- then
- ln -s path0 path1 &&
- ln -s ../path1 path2/bazbo
- make_expected () {
- cat >expected
- }
- else
- printf path0 > path1 &&
- printf ../path1 > path2/bazbo
- make_expected () {
- sed -e "s/12 /100644 /" >expected
- }
- fi &&
+ test_ln_s_add path0 path1 &&
+ test_ln_s_add ../path1 path2/bazbo &&
echo Lo >path2/foo &&
echo Mi >path2/baz/b &&
find path? \( -type f -o -type l \) -print |
@@ -51,7 +39,7 @@ test_output () {
test_expect_success \
'ls-tree plain' \
'git ls-tree $tree >current &&
- make_expected <<\EOF &&
+ cat >expected <<\EOF &&
100644 blob X path0
12 blob X path1
04 tree X path2
@@ -61,7 +49,7 @@ EOF
test_expect_success \
'ls-tree recursive' \
'git ls-tree -r $tree >current &&
- make_expected <<\EOF &&
+ cat >expected <<\EOF &&
100644 blob X path0
12 blob X path1
100644 blob X path2/baz/b
@@ -73,7 +61,7 @@ EOF
test_expect_success \
'ls-tree recursive with -t' \
'git ls-tree -r -t $tree >current &&
- make_expected <<\EOF &&
+ cat >expected <<\EOF &&
100644 blob X path0
12 blob X path1
04 tree X path2
@@ -87,7 +75,7 @@ EOF
test_expect_success \
'ls-tree recursive with -d' \
'git ls-tree -r -d $tree >current &&
- make_expected <<\EOF &&
+ cat >expected <<\EOF &&
04 tree X path2
04 tree X path2/baz
EOF
@@ -96,7 +84,7 @@ EOF
test_expect_success \
'ls-tree filtered with path' \
'git ls-tree $tree path >current &&
- make_expected <<\EOF &&
+ cat >expected <<\EOF &&
EOF
test_output'
@@ -106,7 +94,7 @@ EOF
test_expect_success \
'ls-tree filtered with path1 path0' \
'git ls-tree $tree path1 path0 >current &&
- make_expected <<\EOF &&
+ cat >expected <<\EOF &&
100644 blob X path0
12 blob X path1
EOF
@@ -115,7 +103,7 @@ EOF
test_expect_success \
'ls-tree filtered with path0/' \
'git ls-tree $tree path0/ >current &&
- make_expected <<\EOF &&
+ cat >expected <<\EOF &&
EOF
test_output'
@@ -124,7 +112,7 @@ EOF
test_expect_success \
'ls-tree filtered with path2' \
'git ls-tree $tree path2 >current &&
- make_expected <<\EOF &&
+ cat >expected <<\EOF &&
04 tree X path2
EOF
test_output'
@@ -133,7 +121,7 @@ EOF
test_expect_success \
'ls-tree filtered with path2/' \
'git ls-tree $tree path2/ >current &&
- make_expected <<\EOF &&
+ cat >expected <<\EOF &&
04 tree X path2/baz
12 blob X path2/bazbo
100644 blob X path2/foo
@@ -145,7 +133,7 @@ EOF
test_expect_success \
'ls-tree filtered with path2/baz' \
'git ls-tree $tree path2/baz >current &&
- make_expected <<\EOF &&
+ cat >expected <<\EOF &&
04 tree X path2/baz
EOF
test_output'
@@ -153,14 +141,14 @@ EOF
test_expect_success \
'ls-tree filtered with path2/bak' \
'git ls-tree $tree path2/bak >current &&
- make_expected <<\EOF &&
+ cat >expected <<\EOF &&
EOF
test_output'
test_expect_success \
'ls-tree -t filtered with path2/bak' \
'git ls-tree -t $tree path2/bak >current &&
- make_expected <<\EOF &&
+ cat >expected <<\EOF &&
04 tree X path2
EOF
test_output'
@@ -168,7 +156,7 @@ EOF
test_expect_success \
'ls-tree with one path a prefix of the other' \
'git ls-tree $tree path2/baz path2/bazbo >current &&
- make_expected <<\EOF &&
+ cat >expected <<\EOF &&
04 tree X path2/baz
12 blob X path2/bazbo
EOF
--
1.8.3.rc1.32.g8b61cbb
--
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 05/10] t0000: use test_ln_s_add to remove SYMLINKS prerequisite
t-basic hard-codes many object IDs. To cater to file systems that do
not support symbolic links, different IDs are used depending on the
SYMLINKS prerequisite. But we can observe the symbolic links are only
needed to generate index entries. Use test_ln_s_add to generate the
index entries and get rid of explicit SYMLINKS checks.
This undoes the special casing introduced in this test by 704a3143
(Use prerequisite tags to skip tests that depend on symbolic links,
2009-03-04).
Signed-off-by: Johannes Sixt
---
t/t-basic.sh | 39 ++-
1 file changed, 10 insertions(+), 29 deletions(-)
diff --git a/t/t-basic.sh b/t/t-basic.sh
index cefe33d..0f13180 100755
--- a/t/t-basic.sh
+++ b/t/t-basic.sh
@@ -367,22 +367,6 @@ test_expect_success 'validate object ID of a known tree' '
# Various types of objects
-# Some filesystems do not support symblic links; on such systems
-# some expected values are different
-if test_have_prereq SYMLINKS
-then
- expectfilter=cat
- expectedtree=087704a96baf1c2d1c869a8b084481e121c88b5b
- expectedptree1=21ae8269cacbe57ae09138dcc3a2887f904d02b3
- expectedptree2=3c5e5399f3a333eddecce7a9b9465b63f65f51e2
-else
- expectfilter='grep -v sym'
- expectedtree=8e18edf7d7edcf4371a3ac6ae5f07c2641db7c46
- expectedptree1=cfb8591b2f65de8b8cc1020cd7d9e67e7793b325
- expectedptree2=ce580448f0148b985a513b693fdf7d802cacb44f
-fi
-
-
test_expect_success 'adding various types of objects with git update-index
--add' '
mkdir path2 path3 path3/subp3 &&
paths="path0 path2/file2 path3/file3 path3/subp3/file3" &&
@@ -390,10 +374,7 @@ test_expect_success 'adding various types of objects with
git update-index --add
for p in $paths
do
echo "hello $p" >$p || exit 1
- if test_have_prereq SYMLINKS
- then
- ln -s "hello $p" ${p}sym || exit 1
- fi
+ test_ln_s_add "hello $p" ${p}sym || exit 1
done
) &&
find path* ! -type d -print | xargs git update-index --add
@@ -405,7 +386,7 @@ test_expect_success 'showing stage with git ls-files
--stage' '
'
test_expect_success 'validate git ls-files output for a known tree' '
- $expectfilter >expected <<-\EOF &&
+ cat >expected <<-\EOF &&
100644 f87290f8eb2cbbea7857214459a0739927eab154 0 path0
12 15a98433ae33114b085f3eb3bb03b832b3180a01 0 path0sym
100644 3feff949ed00a62d9f7af97c15cd8a30595e7ac7 0 path2/file2
@@ -423,14 +404,14 @@ test_expect_success 'writing tree out with git
write-tree' '
'
test_expect_success 'validate object ID for a known tree' '
- test "$tree" = "$expectedtree"
+ test "$tree" = 087704a96baf1c2d1c869a8b084481e121c88b5b
'
test_expect_success 'showing tree with git ls-tree' '
git ls-tree $tree >current
'
-test_expect_success SYMLINKS 'git ls-tree output for a known tree' '
+test_expect_success 'git ls-tree output for a known tree' '
cat >expected <<-\EOF &&
100644 blob f87290f8eb2cbbea7857214459a0739927eab154path0
12 blob 15a98433ae33114b085f3eb3bb03b832b3180a01path0sym
@@ -447,7 +428,7 @@ test_expect_success 'showing tree with git ls-tree -r' '
'
test_expect_success 'git ls-tree -r output for a known tree' '
- $expectfilter >expected <<-\EOF &&
+ cat >expected <<-\EOF &&
100644 blob f87290f8eb2cbbea7857214459a0739927eab154path0
12 blob 15a98433ae33114b085f3eb3bb03b832b3180a01path0sym
100644 blob 3feff949ed00a62d9f7af97c15cd8a30595e7ac7path2/file2
@@ -465,7 +446,7 @@ test_expect_success 'showing tree with git ls-tree -r -t' '
git ls-tree -r -t $tree >current
'
-test_expect_success SYMLINKS 'git ls-tree -r output for a known tree' '
+test_expect_success 'git ls-tree -r output for a known tree' '
cat >expected <<-\EOF &&
100644 blob f87290f8eb2cbbea7857214459a0739927eab154path0
12 blob 15a98433ae33114b085f3eb3bb03b832b3180a01path0sym
@@ -487,7 +468,7 @@ test_expect_success 'writing partial tree out with git
write-tree --prefix' '
'
test_expect_success 'validate object ID for a known tree' '
- test "$ptree" = "$expectedptree1"
+ test "$ptree" = 21ae8269cacbe57ae09138dcc3a2887f904d02b3
'
test_expect_success 'writing partial tree out with git write-tree --prefix' '
@@ -495,7 +476,7 @@ test_expect_success 'writing partial tree out with git
write-tree --prefix' '
'
test_expect_success 'validate object ID for a known tree' '
- test "$ptree" = "$expectedptree2"
+ test "$ptree" = 3c5e5399f3a333eddecce7a9b9465b63f65f51e2
'
test_expect_success 'put invalid objects into the index' '
@@ -529,7 +510,7 @@ test_expect_success 'git read-tree followed by write-tree
should be idempotent'
'
[PATCH v2 08/10] t3509, t4023, t4114: use test_ln_s_add to remove SYMLINKS prerequisite
In t4023 and t4114, we have to remove the entries using 'git rm' because
otherwise the entries that must turn from symbolic links to regular files
would stay symbolic links in the index. For the same reason, we have to
use 'git mv' instead of plain 'mv' in t3509.
Signed-off-by: Johannes Sixt
---
t/t3509-cherry-pick-merge-df.sh | 12 +---
t/t4023-diff-rename-typechange.sh | 28 ++--
t/t4114-apply-typechange.sh | 29 ++---
3 files changed, 33 insertions(+), 36 deletions(-)
diff --git a/t/t3509-cherry-pick-merge-df.sh b/t/t3509-cherry-pick-merge-df.sh
index df921d1..a5b6a5f 100755
--- a/t/t3509-cherry-pick-merge-df.sh
+++ b/t/t3509-cherry-pick-merge-df.sh
@@ -10,17 +10,15 @@ test_expect_success 'Initialize repository' '
git commit -m a
'
-test_expect_success SYMLINKS 'Setup rename across paths each below D/F
conflicts' '
+test_expect_success 'Setup rename across paths each below D/F conflicts' '
mkdir b &&
- ln -s ../a b/a &&
- git add b &&
+ test_ln_s_add ../a b/a &&
git commit -m b &&
git checkout -b branch &&
rm b/a &&
- mv a b/a &&
- ln -s b/a a &&
- git add . &&
+ git mv a b/a &&
+ test_ln_s_add b/a a &&
git commit -m swap &&
>f1 &&
@@ -28,7 +26,7 @@ test_expect_success SYMLINKS 'Setup rename across paths each
below D/F conflicts
git commit -m f1
'
-test_expect_success SYMLINKS 'Cherry-pick succeeds with rename across D/F
conflicts' '
+test_expect_success 'Cherry-pick succeeds with rename across D/F conflicts' '
git reset --hard &&
git checkout master^0 &&
git cherry-pick branch
diff --git a/t/t4023-diff-rename-typechange.sh
b/t/t4023-diff-rename-typechange.sh
index 5d20acf..55d549f 100755
--- a/t/t4023-diff-rename-typechange.sh
+++ b/t/t4023-diff-rename-typechange.sh
@@ -4,44 +4,44 @@ test_description='typechange rename detection'
. ./test-lib.sh
-test_expect_success SYMLINKS setup '
+test_expect_success setup '
rm -f foo bar &&
cat "$TEST_DIRECTORY"/../COPYING >foo &&
- ln -s linklink bar &&
- git add foo bar &&
+ test_ln_s_add linklink bar &&
+ git add foo &&
git commit -a -m Initial &&
git tag one &&
- rm -f foo bar &&
+ git rm -f foo bar &&
cat "$TEST_DIRECTORY"/../COPYING >bar &&
- ln -s linklink foo &&
- git add foo bar &&
+ test_ln_s_add linklink foo &&
+ git add bar &&
git commit -a -m Second &&
git tag two &&
- rm -f foo bar &&
+ git rm -f foo bar &&
cat "$TEST_DIRECTORY"/../COPYING >foo &&
git add foo &&
git commit -a -m Third &&
git tag three &&
mv foo bar &&
- ln -s linklink foo &&
- git add foo bar &&
+ test_ln_s_add linklink foo &&
+ git add bar &&
git commit -a -m Fourth &&
git tag four &&
# This is purely for sanity check
- rm -f foo bar &&
+ git rm -f foo bar &&
cat "$TEST_DIRECTORY"/../COPYING >foo &&
cat "$TEST_DIRECTORY"/../Makefile >bar &&
git add foo bar &&
git commit -a -m Fifth &&
git tag five &&
- rm -f foo bar &&
+ git rm -f foo bar &&
cat "$TEST_DIRECTORY"/../Makefile >foo &&
cat "$TEST_DIRECTORY"/../COPYING >bar &&
git add foo bar &&
@@ -50,7 +50,7 @@ test_expect_success SYMLINKS setup '
'
-test_expect_success SYMLINKS 'cross renames to be detected for regular files' '
+test_expect_success 'cross renames to be detected for regular files' '
git diff-tree five six -r --name-status -B -M | sort >actual &&
{
@@ -61,7 +61,7 @@ test_expect_success SYMLINKS 'cross renames to be detected
for regular files' '
'
-test_expect_success SYMLINKS 'cross renames to be detected for typechange' '
+test_expect_success 'cross renames to be detected for typechange' '
git diff-tree one two -r --name-status -B -M | sort >actual &&
{
@@ -72,7 +72,7 @@ test_expect_success SYMLINKS 'cross renames to be detected
for typechange' '
'
-test_expect_success SYMLINKS 'moves and renames' '
+test_expect_success 'moves and renames' '
git diff-tree three four -r --name-status -B -M | sort >actual &&
{
diff --git a/t/t4114-apply-typechange.sh b/t/t4114-apply-typechange.sh
index f12826f..ebadbc3 100755
--- a/t/t4114-apply-typechange.sh
+++ b/t/t4114-apply-typechange.sh
@@ -9,20 +9,19 @@ test_description='git apply should not get confused with type
changes.
. ./test-lib.sh
-test_expect_success SYMLINKS 'setup repository and commits' '
+test_expect_success 'setup repository and commits' '
echo "hello world" > foo &&
echo "hi planet" > bar &&
git update-index --add foo bar &&
git commit -m initial &&
git branch initial &&
rm -f foo &&
- ln -s bar foo &&
-
[PATCH v2 00/10] Increase test coverage on Windows by removing SYMLINKS from many tests
Many tests that involve symbolic links actually check only whether our algorithms are correct by investigating the contents of the object database and the index. Only some of them check the filesystem. This series introduces a function test_ln_s_add that inserts a symbolic link in the index even if the filesystem does not support symbolic links. By using this function, many more tests can be run when the filesystem does not have symblic links, aka Windows. Changes since v1: - Ripped out test_ln_s and corresponding conversions; they were dubious. - There are no changes to t2100 anymore; the corresponding modernization patch is gone. - Moved the t4011 change from the "trivial cases" to its own patch. It still contains the effects of the former test_ln_s, but open-coded and with a comment. Johannes Sixt (10): test-chmtime: Fix exit code on Windows t3010: modernize style tests: introduce test_ln_s_add tests: use test_ln_s_add to remove SYMLINKS prerequisite (trivial cases) t: use test_ln_s_add to remove SYMLINKS prerequisite t3030: use test_ln_s_add to remove SYMLINKS prerequisite t3100: use test_ln_s_add to remove SYMLINKS prerequisite t3509, t4023, t4114: use test_ln_s_add to remove SYMLINKS prerequisite t6035: use test_ln_s_add to remove SYMLINKS prerequisite t4011: remove SYMLINKS prerequisite t/README | 14 t/t-basic.sh | 39 +++ t/t1004-read-tree-m-u-wf.sh| 7 +- t/t2001-checkout-cache-clash.sh| 7 +- t/t2004-checkout-cache-temp.sh | 5 +- t/t2007-checkout-symlink.sh| 12 ++-- t/t2021-checkout-overwrite.sh | 12 ++-- t/t2200-add-update.sh | 5 +- t/t3010-ls-files-killed-modified.sh| 118 - t/t3030-merge-recursive.sh | 62 - t/t3100-ls-tree-restrict.sh| 42 +--- t/t3509-cherry-pick-merge-df.sh| 12 ++-- t/t3700-add.sh | 15 ++--- t/t3903-stash.sh | 39 --- t/t4008-diff-break-rewrite.sh | 12 ++-- t/t4011-diff-symlink.sh| 35 +++--- t/t4023-diff-rename-typechange.sh | 28 t/t4030-diff-textconv.sh | 8 +-- t/t4114-apply-typechange.sh| 29 t/t4115-apply-symlink.sh | 10 ++- t/t4122-apply-symlink-inside.sh| 8 +-- t/t6035-merge-dir-to-symlink.sh| 73 t/t7001-mv.sh | 18 +++-- t/t7607-merge-overwrite.sh | 5 +- t/t8006-blame-textconv.sh | 14 ++-- t/t8007-cat-file-textconv.sh | 10 ++- t/t9350-fast-export.sh | 5 +- t/t9500-gitweb-standalone-no-errors.sh | 15 ++--- t/test-lib-functions.sh| 17 + test-chmtime.c | 8 +-- 30 files changed, 351 insertions(+), 333 deletions(-) -- 1.8.3.rc1.32.g8b61cbb -- 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 02/10] t3010: modernize style
In particular: - move test preparations inside test_expect_success - place test description on the test_expect_success line - indent with a tab Signed-off-by: Johannes Sixt --- t/t3010-ls-files-killed-modified.sh | 123 ++-- 1 file changed, 61 insertions(+), 62 deletions(-) diff --git a/t/t3010-ls-files-killed-modified.sh b/t/t3010-ls-files-killed-modified.sh index 95671c2..2d0ff2d 100755 --- a/t/t3010-ls-files-killed-modified.sh +++ b/t/t3010-ls-files-killed-modified.sh @@ -37,71 +37,70 @@ modified without reporting path9 and path10. ' . ./test-lib.sh -date >path0 -if test_have_prereq SYMLINKS -then - ln -s xyzzy path1 -else - date > path1 -fi -mkdir path2 path3 -date >path2/file2 -date >path3/file3 -: >path7 -date >path8 -: >path9 -date >path10 -test_expect_success \ -'git update-index --add to add various paths.' \ -"git update-index --add -- path0 path1 path?/file? path7 path8 path9 path10" - -rm -fr path? ;# leave path10 alone -date >path2 -if test_have_prereq SYMLINKS -then - ln -s frotz path3 - ln -s nitfol path5 -else - date > path3 - date > path5 -fi -mkdir path0 path1 path6 -date >path0/file0 -date >path1/file1 -date >path6/file6 -date >path7 -: >path8 -: >path9 -touch path10 +test_expect_success 'git update-index --add to add various paths.' ' + date >path0 && + if test_have_prereq SYMLINKS + then + ln -s xyzzy path1 + else + date > path1 + fi && + mkdir path2 path3 && + date >path2/file2 && + date >path3/file3 && + : >path7 && + date >path8 && + : >path9 && + date >path10 && + git update-index --add -- path0 path1 path?/file? path7 path8 path9 path10 && + rm -fr path?# leave path10 alone +' -test_expect_success \ -'git ls-files -k to show killed files.' \ -'git ls-files -k >.output' -cat >.expectedpath3 && + date >path5 + fi && + mkdir path0 path1 path6 && + date >path0/file0 && + date >path1/file1 && + date >path6/file6 && + date >path7 && + : >path8 && + : >path9 && + touch path10 && + git ls-files -k >.output +' -test_expect_success \ -'validate git ls-files -k output.' \ -'test_cmp .expected .output' +test_expect_success 'validate git ls-files -k output.' ' + cat >.expected <<-\EOF && + path0/file0 + path1/file1 + path2 + path3 + EOF + test_cmp .expected .output +' -test_expect_success \ -'git ls-files -m to show modified files.' \ -'git ls-files -m >.output' -cat >.expected <.output +' -test_expect_success \ -'validate git ls-files -m output.' \ -'test_cmp .expected .output' +test_expect_success 'validate git ls-files -m output.' ' + cat >.expected <<-\EOF && + path0 + path1 + path2/file2 + path3/file3 + path7 + path8 + EOF + test_cmp .expected .output +' test_done -- 1.8.3.rc1.32.g8b61cbb -- 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 06/10] t3030: use test_ln_s_add to remove SYMLINKS prerequisite
The test cases include many corner-cases of merge-recursive's behavior, some of them involve type changes and symbolic links. All cases, including those that are protected by SYMLINKS check only whether the result of merge-recursive is correctly stored in the database and the index; the file system is not investigated. Use test_ln_s_add to enter a symbolic link in the index in the test setup and run the tests without the SYMLINKS prerequisite. Notice that one test that has the SYMLINKS protection removed is an expect_failure. There is a possibility that the test fails differently depending on whether SYMLINKS is present or not; but this is not the case presently. Signed-off-by: Johannes Sixt --- t/t3030-merge-recursive.sh | 62 +++--- 1 file changed, 26 insertions(+), 36 deletions(-) diff --git a/t/t3030-merge-recursive.sh b/t/t3030-merge-recursive.sh index a5e3da7..2f96100 100755 --- a/t/t3030-merge-recursive.sh +++ b/t/t3030-merge-recursive.sh @@ -25,10 +25,7 @@ test_expect_success 'setup 1' ' git branch submod && git branch copy && git branch rename && - if test_have_prereq SYMLINKS - then - git branch rename-ln - fi && + git branch rename-ln && echo hello >>a && cp a d/e && @@ -260,16 +257,12 @@ test_expect_success 'setup 8' ' git add e && test_tick && git commit -m "rename a->e" && - if test_have_prereq SYMLINKS - then - git checkout rename-ln && - git mv a e && - ln -s e a && - git add a e && - test_tick && - git commit -m "rename a->e, symlink a->e" && - oln=`printf e | git hash-object --stdin` - fi + git checkout rename-ln && + git mv a e && + test_ln_s_add e a && + test_tick && + git commit -m "rename a->e, symlink a->e" && + oln=`printf e | git hash-object --stdin` ' test_expect_success 'setup 9' ' @@ -569,28 +562,25 @@ test_expect_success 'merge-recursive copy vs. rename' ' test_cmp expected actual ' -if test_have_prereq SYMLINKS -then - test_expect_failure 'merge-recursive rename vs. rename/symlink' ' - - git checkout -f rename && - git merge rename-ln && - ( git ls-tree -r HEAD ; git ls-files -s ) >actual && - ( - echo "12 blob $oln a" - echo "100644 blob $o0 b" - echo "100644 blob $o0 c" - echo "100644 blob $o0 d/e" - echo "100644 blob $o0 e" - echo "12 $oln 0 a" - echo "100644 $o0 0 b" - echo "100644 $o0 0 c" - echo "100644 $o0 0 d/e" - echo "100644 $o0 0 e" - ) >expected && - test_cmp expected actual - ' -fi +test_expect_failure 'merge-recursive rename vs. rename/symlink' ' + + git checkout -f rename && + git merge rename-ln && + ( git ls-tree -r HEAD ; git ls-files -s ) >actual && + ( + echo "12 blob $oln a" + echo "100644 blob $o0 b" + echo "100644 blob $o0 c" + echo "100644 blob $o0 d/e" + echo "100644 blob $o0 e" + echo "12 $oln 0 a" + echo "100644 $o0 0 b" + echo "100644 $o0 0 c" + echo "100644 $o0 0 d/e" + echo "100644 $o0 0 e" + ) >expected && + test_cmp expected actual +' test_done -- 1.8.3.rc1.32.g8b61cbb -- 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 01/10] test-chmtime: Fix exit code on Windows
MinGW's bash does not recognize an exit code -1 as failure. See also
47e3de0e (MinGW: truncate exit()'s argument to lowest 8 bits) and 2488df84
(builtin run_command: do not exit with -1). Exit code 1 is good enough.
Signed-off-by: Johannes Sixt
---
test-chmtime.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/test-chmtime.c b/test-chmtime.c
index 02b42ba..2e601a8 100644
--- a/test-chmtime.c
+++ b/test-chmtime.c
@@ -84,7 +84,7 @@ int main(int argc, const char *argv[])
if (stat(argv[i], &sb) < 0) {
fprintf(stderr, "Failed to stat %s: %s\n",
argv[i], strerror(errno));
- return -1;
+ return 1;
}
#ifdef WIN32
@@ -92,7 +92,7 @@ int main(int argc, const char *argv[])
chmod(argv[i], sb.st_mode | S_IWUSR)) {
fprintf(stderr, "Could not make user-writable %s: %s",
argv[i], strerror(errno));
- return -1;
+ return 1;
}
#endif
@@ -107,7 +107,7 @@ int main(int argc, const char *argv[])
if (utb.modtime != sb.st_mtime && utime(argv[i], &utb) < 0) {
fprintf(stderr, "Failed to modify time on %s: %s\n",
argv[i], strerror(errno));
- return -1;
+ return 1;
}
}
@@ -115,5 +115,5 @@ int main(int argc, const char *argv[])
usage:
fprintf(stderr, "usage: %s %s\n", argv[0], usage_str);
- return -1;
+ return 1;
}
--
1.8.3.rc1.32.g8b61cbb
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jun 2013, #03; Thu, 6)
On Fri, Jun 07, 2013 at 12:46:25PM -0700, Junio C Hamano wrote: > Thanks for a pointer. I think what I was suggesting was slightly > different in that I was hoping to see a single helper that knows to > complete to object names (possibly including trees/blobs with the > treeish:path notation), ranges, and pathnames (not treeish:path > notation) until it sees a "--" and then complete only to pathnames. We already got that except the completion of pathnames before "--", and I don't know how that could be done properly for commands taking both refs and paths. Just consider git diff git.c git diff master git.c git diff master next git.c We can't guess whether the user wants refs or paths when he first hits tab after 'git diff ', not even after 'git diff master '. I definitely don't want to see refs and paths all mixed up. As for the _single_ helper: I think it has some value that we have different helper functions and we can indicate whether a certain git command can take a ref or ref:path or ref1...ref2 or even ref1..ref2:path by calling the appropriate helper function (however badly it might have been named), even if all these functions happen to boil down to a single implementation. Gábor -- 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] gitweb: fix problem causing erroneous project list
On Wed, Jun 5, 2013 at 9:17 PM, Junio C Hamano wrote:
> Charles McGarvey writes:
>
>> The bug is manifest when running gitweb in a persistent process (e.g.
>> FastCGI, PSGI), and it's easy to reproduce. If a gitweb request
>> includes the searchtext parameter (i.e. s), subsequent requests using
>> the project_list action--which is the default action--and without
>> a searchtext parameter will be filtered by the searchtext value of the
>> first request. This is because the value of the $search_regexp global
>> (the value of which is based on the searchtext parameter) is currently
>> being persisted between requests.
>>
>> Instead, clear $search_regexp before dispatching each request.
>>
>> Signed-off-by: Charles McGarvey
Acked-by: Jakub Narebski
>> ---
>> I don't think there are currently any persistent-process gitweb tests to
>> copy from, so writing a test for this seems to be non-trivial.
Well, there is Plack::Test, and gitweb supports running as PSGI app.
Though all such test would have to be under new PLACK or PSGI
prerequisite.
> The problem description makes sense to me, and clearing with "undef"
> is in line with the case where the CGI is run for the first time, so
> I think this patch is correct.
>
> Will wait for a while to give Jakub some time to comment on (like:
> Ack ;-) and then apply. Thanks.
>
> By the way, I looked at how $search_regexp is used in the code:
How $search_regexp is used does not matter. What was intended
(but was not implemented) is for $search_regexp to matter and to
be used only if $searchtext is defined. $searchtext is reset on each
request, so $search_regexp should be also reset... like in Charles's
patch.
Anyway in the analysis below we need to check if code checks
$searchtext first.
> * esc_html_match_hl and esc_html_match_hl__chopped (called from
>git_project_list_rows, for example) want to have "undef"; an
>empty string will not do.
>
> * search_projects_list (called from git_project_list_body)
>
> x git_search_files and git_search_grep_body assume that
>$search_regexp can be interpolated in m//, which is not very
>nice. They want an empty string.
But both git_search_files() and git_search_grep_body() are run from
git_search(), which "dies" (returns HTTP 400 "Text field is empty" error)
if $searchtext is not defined; if $searchtext is defined then $search_regexp
is string and is never undef.
> So as an independent fix, the two subs may want to be fixed if we
> want to be undef clean. Or am I missing something? Jakub, isn't
> this kind of "undef" reference t9500 was designed to catch?
>
>> gitweb/gitweb.perl | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
>> index 80950c0..8d69ada 100755
>> --- a/gitweb/gitweb.perl
>> +++ b/gitweb/gitweb.perl
>> @@ -1086,7 +1086,7 @@ sub evaluate_and_validate_params {
>> our $search_use_regexp = $input_params{'search_use_regexp'};
>>
>> our $searchtext = $input_params{'searchtext'};
>> - our $search_regexp;
>> + our $search_regexp = undef;
>> if (defined $searchtext) {
>> if (length($searchtext) < 2) {
>> die_error(403, "At least two characters are required
>> for search parameter");
--
Jakub Narebski
--
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] completion: add deprecated __git_complete_file ()
Ramkumar Ramachandra writes: > 77c130 (completion: clarify ls-tree, archive, show completion, > 2013-06-02) removed __git_complete_file () because it had no callers > left in the file. However, to avoid breaking user scripts that may > depend on this, add it back as a deprecated alias. > > Signed-off-by: Ramkumar Ramachandra > --- > Based on pu. Will queue; thanks. With this, I think it will be safe to push the series in question to 'master'. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] test: improve rebase -q test
Let's show the output so it's clear why it failed.
Signed-off-by: Felipe Contreras
---
t/t3400-rebase.sh | 1 +
1 file changed, 1 insertion(+)
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index b58fa1a..fb39531 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -185,6 +185,7 @@ test_expect_success 'default to @{upstream} when upstream
arg is missing' '
test_expect_success 'rebase -q is quiet' '
git checkout -b quiet topic &&
git rebase -q master >output.out 2>&1 &&
+ cat output.out &&
test ! -s output.out
'
--
1.8.3.698.g079b096
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] submodule: remove unnecessary check
read_cache() already does that check.
Signed-off-by: Felipe Contreras
---
submodule.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/submodule.c b/submodule.c
index ad476ce..8685424 100644
--- a/submodule.c
+++ b/submodule.c
@@ -603,9 +603,8 @@ int fetch_populated_submodules(const struct argv_array
*options,
if (!work_tree)
goto out;
- if (!the_index.initialized)
- if (read_cache() < 0)
- die("index file corrupt");
+ if (read_cache() < 0)
+ die("index file corrupt");
argv_array_push(&argv, "fetch");
for (i = 0; i < options->argc; i++)
--
1.8.3.698.g079b096
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] sequencer: trivial fix
We should free objects before leaving.
Signed-off-by: Felipe Contreras
---
sequencer.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index ab6f8a7..7eeae2f 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -626,12 +626,15 @@ static int do_pick_commit(struct commit *commit, struct
replay_opts *opts)
rerere(opts->allow_rerere_auto);
} else {
int allow = allow_empty(opts, commit);
- if (allow < 0)
- return allow;
+ if (allow < 0) {
+ res = allow;
+ goto leave;
+ }
if (!opts->no_commit)
res = run_git_commit(defmsg, opts, allow);
}
+leave:
free_message(&msg);
free(defmsg);
--
1.8.3.698.g079b096
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/3] Trivial patches
Felipe Contreras (3): sequencer: trivial fix test: improve rebase -q test submodule: remove unnecessary check sequencer.c | 7 +-- submodule.c | 5 ++--- t/t3400-rebase.sh | 1 + 3 files changed, 8 insertions(+), 5 deletions(-) -- 1.8.3.698.g079b096 -- 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 05/18] Turn double-negated expressions into simple expressions
Le 07/06/2013 22:25, Eric Sunshine a écrit : > If you do choose to be more precise, it should be done as a separate > patch. Each conceptually distinct change should have its own patch. > Doing so makes changes easier to review and (generally) easier to > cherry-pick. For example, in this particular case, "simplify > doubly-negated expressions" is quite conceptually distinct from "emit > more precise diagnostics". (Textually the changes may happen to > overlap, but conceptually they are unrelated.) OK, I will do another patch. -- Célestin Matte -- 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 3/9] cherry-pick: add --skip-empty option
On Thu, Jun 6, 2013 at 2:21 PM, Junio C Hamano wrote: > Felipe Contreras writes: > >> On Thu, Jun 6, 2013 at 1:30 PM, Junio C Hamano wrote: >>> Felipe Contreras writes: >>> Pretty much what it says on the tin. >>> >>> And a bit more, isn't it? >>> >>> The --keep-redundant-commits option implies the --allow-empty option >>> and it was perfectly acceptable to give both. By making sure that >>> only at most one of -k-r-d, -a-e or -s-e is given, this forbids that >>> usage. >>> >>> "It is implied so there is no *need* to give it redundantly" is >>> different from "It is implied so you shouldn't give it redundantly". >> >> Remove that line then. > > That's what the submitter does. Apparently "the submitter" does everything. It seems sending 800 patches in the past 3 months is not enough work for you. And what's the point? If nobody is interested in shifting from shell scripts to C, neither am I. Consider this series abandoned. -- Felipe Contreras -- 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 05/18] Turn double-negated expressions into simple expressions
On Fri, Jun 7, 2013 at 1:04 PM, Célestin Matte
wrote:
> Le 07/06/2013 06:12, Eric Sunshine a écrit :
>> On Thu, Jun 6, 2013 at 3:34 PM, Célestin Matte
>> wrote:
>>> } elsif ($cmd[0] eq "import") {
>>> - die("Invalid arguments for import\n") unless
>>> ($cmd[1] ne "" && !defined($cmd[2]));
>>> + die("Invalid arguments for import\n") if ($cmd[1]
>>> eq "" || defined($cmd[2]));
>>> mw_import($cmd[1]);
>>> } elsif ($cmd[0] eq "option") {
>>> - die("Too many arguments for option\n") unless
>>> ($cmd[1] ne "" && $cmd[2] ne "" && !defined($cmd[3]));
>>> + die("Too many arguments for option\n") if ($cmd[1]
>>> eq "" || $cmd[2] eq "" || defined($cmd[3]));
>>
>> Not new in this patch, but isn't this diagnostic misleading? It will
>> (falsely) claim "too many arguments" if $cmd[1] or $cmd[2] is an empty
>> string. Perhaps it should be reworded like the 'import' diagnostic and
>> say "Invalid arguments for option".
>
> We could even be more precise and separate the cases, i.e., die("Too
> many arguments") when too many arguments are defined and die("Invalid
> arguments") when there are empty strings.
> Not sure if I should integrate it in this patch, though.
If you do choose to be more precise, it should be done as a separate
patch. Each conceptually distinct change should have its own patch.
Doing so makes changes easier to review and (generally) easier to
cherry-pick. For example, in this particular case, "simplify
doubly-negated expressions" is quite conceptually distinct from "emit
more precise diagnostics". (Textually the changes may happen to
overlap, but conceptually they are unrelated.)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Administrivia] On ruby and contrib/
On Fri, Jun 7, 2013 at 2:55 PM, Junio C Hamano wrote: > Felipe Contreras writes: > >>> I think we heard enough from packaging folks that a new dependency >>> is unwelcome. >> >> What are you talking about? Which are these "packaging folks" we heard from? > > Dscho is one of the primary people behind msysgit effort, and I > consulted with others from the circle with an draft before I sent > the message to the list for sanity checking (fearing that I may be > worrying about adding new dependencies needlessly). He said he won't do it, but I said I would. Doesn't that solve the problem? > Jonathan > packages git for Debian and he is negative on adding new dependency > needlessly. I don't see any comment from Jonathan. > It was unexpected that we hear from a pkgsrc person but > the response was also negative. You mean Greg Troxel? He is only one of the persons that help, and I did shut down his argument, didn't I? >>> Also we heard from no regular/high-value reviewers >>> that they feel comfortable reviewing additions in Ruby. >> >> Correction; *current* regular/high-value reviewers. > > That is exactly what I meant. > > The code review is not only about following best practices in the > implementation language. If somebody who is an expert in a language > we do not currently depend on, but who does not know how the parts > of Git are supposed to fit together enough to judge the soundness of > the design of new code written in that new language, or does not > know how the tests, documentation and log messages are supposed to > written around here, that person cannot be the only reviewer for > changes written in that language to ensure quality standard. > > The reviewer pool for code written in a new language _must_ be > seeded by some from the current set of reviewers whose judgement > I/we can trust. By that standard nothing will ever change. Ever. Even twenty years from now, you will still only trust people that are familiar with shell, Perl, and C. Because the only way to gain your trust, is by being proficient in shell, Perl, and C. -- Felipe Contreras -- 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: is there a fast web-interface to git for huge repos?
On 7 June 2013 13:13, Charles McGarvey wrote: > On 06/07/2013 01:02 PM, Constantine A. Murenin wrote: >>> That's a one-time penalty. Why would that be a problem? And why is wget >>> even mentioned? Did we misunderstood eachother? >> >> `wget` or `curl --head` would be used to trigger the caching. >> >> I don't understand how it's a one-time penalty. Noone wants to look >> at an old copy of the repository, so, pretty much, if, say, I want to >> have a gitweb of all 4 BSDs, updated daily, then, pretty much, even >> with lots of ram (e.g. to eliminate the cold-case 5s penalty, and >> reduce each page to 0.5s), on a quad-core box, I'd be kinda be lucky >> to complete a generation of all the pages within 12h or so, obviously >> using the machine at, or above, 50% capacity just for the caching. Or >> several days or even a couple of weeks on an Intel Atom or VIA Nano >> with 2GB of RAM or so. Obviously not acceptable, there has to be a >> better solution. >> >> One could, I guess, only regenerate the pages which have changed, but >> it still sounds like an ugly solution, where you'd have to be >> generating a list of files that have changed between one gen and the >> next, and you'd still have to have a very high cpu, cache and storage >> requirements. > > Have you already ruled out caching on a proxy? Pages would only be generated > on demand, so the first visitor would still experience the delay but the rest > would be fast until the page expires. Even expiring pages as often as five > minutes or less would probably provide significant processing savings > (depending on how many users you have), and that level of staleness and the > occasional delays may be acceptable to your users. > > As you say, generating the entire cache upfront and continuously is wasteful > and probably unrealistic, but any type of caching, by definition, is going to > involve users seeing stale content, and I don't see that you have any other > option but some type of caching. Well, you could reproduce what git does in a > bunch of distributed algorithms and run your app on a farm--which, I guess, is > probably what GitHub is doing--but throwing up a caching reverse proxy is a > lot quicker if you can accept the caveats. I don't think GitHub / Gitorious / whatever have solved this problem at all. They're terribly slow on big repos, some pages don't even generate the first time you click on the link. I'm totally fine with daily updates; but I think there still has to be some better way of doing this than wasting 0.5s of CPU time and 5s of HDD time (if completely cold) for each blame / log, at the price of more storage and some pre-caching, and (daily (in my use-case)) fine-grained incremental updates. C. -- 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: is there a fast web-interface to git for huge repos?
On 06/07/2013 01:02 PM, Constantine A. Murenin wrote: >> That's a one-time penalty. Why would that be a problem? And why is wget >> even mentioned? Did we misunderstood eachother? > > `wget` or `curl --head` would be used to trigger the caching. > > I don't understand how it's a one-time penalty. Noone wants to look > at an old copy of the repository, so, pretty much, if, say, I want to > have a gitweb of all 4 BSDs, updated daily, then, pretty much, even > with lots of ram (e.g. to eliminate the cold-case 5s penalty, and > reduce each page to 0.5s), on a quad-core box, I'd be kinda be lucky > to complete a generation of all the pages within 12h or so, obviously > using the machine at, or above, 50% capacity just for the caching. Or > several days or even a couple of weeks on an Intel Atom or VIA Nano > with 2GB of RAM or so. Obviously not acceptable, there has to be a > better solution. > > One could, I guess, only regenerate the pages which have changed, but > it still sounds like an ugly solution, where you'd have to be > generating a list of files that have changed between one gen and the > next, and you'd still have to have a very high cpu, cache and storage > requirements. Have you already ruled out caching on a proxy? Pages would only be generated on demand, so the first visitor would still experience the delay but the rest would be fast until the page expires. Even expiring pages as often as five minutes or less would probably provide significant processing savings (depending on how many users you have), and that level of staleness and the occasional delays may be acceptable to your users. As you say, generating the entire cache upfront and continuously is wasteful and probably unrealistic, but any type of caching, by definition, is going to involve users seeing stale content, and I don't see that you have any other option but some type of caching. Well, you could reproduce what git does in a bunch of distributed algorithms and run your app on a farm--which, I guess, is probably what GitHub is doing--but throwing up a caching reverse proxy is a lot quicker if you can accept the caveats. -- Charles McGarvey signature.asc Description: OpenPGP digital signature
Re: What's cooking in git.git (Jun 2013, #03; Thu, 6)
On Sat, Jun 08, 2013 at 01:17:28AM +0530, Ramkumar Ramachandra wrote: > SZEDER Gábor wrote: > > because nowadays __git_complete_file() is a wrapper around > > __git_complete_revlist_file(). > > What? It was never anything different from a poorly-named alias for > __git_complete_revlist_file(). Again: __git_complete_file() has been there since the completion script was first included in git. > You have already agreed that > __git_complete_file() is a horrible name, so why not deprecate it? I am not against deprecating it, and by "it" I mean the function name. > Whom is this confusion benefiting, and how is it any "cleaner"? It's clearer because of the reasons I gave in my other email in the discussion of the patch. Plus it would avoid commits on master with incorrect commit messages. > If > you're arguing about expanding __git_complete_file(), don't do that: > just write a new function and give it a proper name. I am not arguing about that. -- 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: [Administrivia] On ruby and contrib/
Felipe Contreras writes: >> I think we heard enough from packaging folks that a new dependency >> is unwelcome. > > What are you talking about? Which are these "packaging folks" we heard from? Dscho is one of the primary people behind msysgit effort, and I consulted with others from the circle with an draft before I sent the message to the list for sanity checking (fearing that I may be worrying about adding new dependencies needlessly). Jonathan packages git for Debian and he is negative on adding new dependency needlessly. It was unexpected that we hear from a pkgsrc person but the response was also negative. >> Also we heard from no regular/high-value reviewers >> that they feel comfortable reviewing additions in Ruby. > > Correction; *current* regular/high-value reviewers. That is exactly what I meant. The code review is not only about following best practices in the implementation language. If somebody who is an expert in a language we do not currently depend on, but who does not know how the parts of Git are supposed to fit together enough to judge the soundness of the design of new code written in that new language, or does not know how the tests, documentation and log messages are supposed to written around here, that person cannot be the only reviewer for changes written in that language to ensure quality standard. The reviewer pool for code written in a new language _must_ be seeded by some from the current set of reviewers whose judgement I/we can trust. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jun 2013, #03; Thu, 6)
SZEDER Gábor wrote: > because nowadays __git_complete_file() is a wrapper around > __git_complete_revlist_file(). What? It was never anything different from a poorly-named alias for __git_complete_revlist_file(). You have already agreed that __git_complete_file() is a horrible name, so why not deprecate it? Whom is this confusion benefiting, and how is it any "cleaner"? If you're arguing about expanding __git_complete_file(), don't do that: just write a new function and give it a proper name. -- 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: [Administrivia] On ruby and contrib/
Ramkumar Ramachandra wrote: > commit a lot of good ruby code to contrib* Oh, by the way: I have a project idea. There's this really popular project called hub[1] that has an implementation of the GitHub API in ruby. Unfortunately, it's a terrible piece of software because it creates an extra layer of indirection by putting a ruby wrapper on top of git, and this slows git down: I cannot tolerate even a microsecond of delay in git. Maybe it's worth ripping out the GitHub API implementation and writing some useful subcommands? [1]: https://github.com/defunkt/hub -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jun 2013, #03; Thu, 6)
SZEDER Gábor writes: >> Now I do not recall suggesting it, and you (and I today after 2 >> years) may disagree with the rationale, but at least we can read >> what was the "intended" meaning, I think. > > See > > http://thread.gmane.org/gmane.comp.version-control.git/167728/focus=168838 > > I still agree with the rationale,... Thanks for a pointer. I think what I was suggesting was slightly different in that I was hoping to see a single helper that knows to complete to object names (possibly including trees/blobs with the treeish:path notation), ranges, and pathnames (not treeish:path notation) until it sees a "--" and then complete only to pathnames. It can be improved by teaching the unified one that some command like "log" can never take treeish:path but only committishes, some command take individual object names but never ranges, and/or details like that. But I still agree that "git log HEAD:dir" that completes to a blob or a tree object name is not an issue (because what was before cannot possibly name anything "git log" would appreciate), even though it might not be ideal. -- 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: [Administrivia] On ruby and contrib/
Felipe Contreras wrote: > While at it, why not re-evaluate the whole msysgit approach? I bet we > don't need a whole separate project just to create a Windows > installer. I've written Windows installers before, it's very easy to > do from Linux. Yeah, taking the pain out of msysgit packaging would be a great way to counter this new-dependency-fud. The main problem, as mm pointed out is subversion + perlxs [1]. Any idea how to tackle that? [1]: https://github.com/msysgit/msysgit/wiki/Frequently-Asked-Questions -- 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 0/8] Rebase topology test
Am 07.06.2013 08:11, schrieb Martin von Zweigbergk: > Changes since v5: > > * Improved test_linear_range > * Changed TODOs to be about consistency, not --topo-order > > Martin von Zweigbergk (7): > add simple tests of consistency across rebase types > add tests for rebasing with patch-equivalence present > add tests for rebasing of empty commits > add tests for rebasing root > add tests for rebasing merged history > t3406: modernize style > tests: move test for rebase messages from t3400 to t3406 I looked at the interdiff to v3 and have nothing to add. Well done! Thanks. -- Hannes -- 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
