* Re: difftool -d symlinks, under what conditions
From: Junio C Hamano @ 2013-03-14 17:33 UTC (permalink / raw)
To: John Keeping
Cc: David Aguilar, Matt McClure, git@vger.kernel.org, Tim Henigan
In-Reply-To: <20130314172515.GB4256@serenity.lan>
John Keeping <john@keeping.me.uk> writes:
> The path passed to get_stream_filter is only used to decide what filters
> apply to the file, so shouldn't it be using "ce->name" and not "path"
> for the same reason that the call to convert_to_working_tree() further
> down the same function does?
Correct and well spotted.
>
> -- >8 --
> diff --git a/entry.c b/entry.c
> index 17a6bcc..63c52ed 100644
> --- a/entry.c
> +++ b/entry.c
> @@ -145,7 +145,7 @@ static int write_entry(struct cache_entry *ce, char *path, const struct checkout
> struct stat st;
>
> if (ce_mode_s_ifmt == S_IFREG) {
> - struct stream_filter *filter = get_stream_filter(path, ce->sha1);
> + struct stream_filter *filter = get_stream_filter(ce->name, ce->sha1);
> if (filter &&
> !streaming_write_entry(ce, path, filter,
> state, to_tempfile,
^ permalink raw reply
* Re: [PATCH v3 3/3] git-merge-one-file: revise merge error reporting
From: Kevin Bracey @ 2013-03-14 17:39 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, David Aguilar <davvid@gmail.com>l Antoine Pelisse,
Ciaran Jessup, Jeff King, Uwe Kleine-König, Scott Chacon,
Alex Riesen
In-Reply-To: <5142097B.1080105@bracey.fi>
On 14/03/2013 19:31, Kevin Bracey wrote:
> On 14/03/2013 16:56, Junio C Hamano wrote:
>>
> Well, yes, but I would assume that we would forcibly select normal
> diff here somehow, if we aren't already. We should be - turning
> ABCDEFGH vs ABCD into ABCD<EFGH|EFGH=> is silly.
Doh. But anyway, we don't want to waste space with |= markers, and make
the same "surrounding code is in the base" suggestion. So we should be
selecting diff.
Kevin
^ permalink raw reply
* git merge problem
From: Коньков Евгений @ 2013-03-14 19:27 UTC (permalink / raw)
To: git
Hi
I have started branch from master
then on master were changes in whitespace (code formatting)
in branch on that lines changes were not.
Now when I try to merge branch to master I get lost changes at space on master
So I try to ignore space change
git merge -Xignore-space-change my_branch
I have lost formatting in master.
I wanna to give priority to changes on branch master:
git merge -Xours my_branch
and now also no results: I get old space formatting which were on
branch master before starting my_branch
Please tell me, how can I fix that problem?
# git --version
git version 1.8.1.5
git diff master...new_function_find Domain.pm
index 7eb8191..5abba1c 100644
--- a/lib/FrontOffice/Controller/API/RegRu2/Domain.pm
+++ b/lib/FrontOffice/Controller/API/RegRu2/Domain.pm
@@ -124,7 +124,7 @@ sub get_prices : Local {
return;
}
- my $show_renew_data = $p->{show_renew_data} || 0;
+ my $show_renew_data = $p->{show_renew_data} || 0;
my $show_update_data = $p->{show_update_data} || 0;
my $lang = $r->{lang};
as you see on branch master right formatting will be replaced by wrong
formatting on branch new_function_find
as I have said: doing #git merge -Xignore-space-change new_finction_find
no result :_-((
the wrong formatting on branch new_function_find are old formatting
that were in branch master, before branch new_function_find were started
no changes were made to that line on branch new_function_find
while on branch new_function_find all new changes from master
were merged as:
#git merge -s recursive -Xignore-space-change master
please help
^ permalink raw reply related
* [PATCH 0/2] checkout-index: fix .gitattributes handling with --prefix
From: John Keeping @ 2013-03-14 20:00 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, David Aguilar, Matt McClure, Tim Henigan, John Keeping
In-Reply-To: <7vehfhyjgv.fsf@alter.siamese.dyndns.org>
This is from the recent "difftool --dir-diff" discussion. With these
patches applied I think "difftool --dir-diff" should correctly apply
filters to the files that it checks out with no changes to the
git-difftool code.
John Keeping (2):
t2003: modernize style
entry: fix streaming filter path
entry.c | 2 +-
t/t2003-checkout-cache-mkdir.sh | 169 +++++++++++++++++++++++-----------------
2 files changed, 97 insertions(+), 74 deletions(-)
--
1.8.2.rc2.4.g7799588
^ permalink raw reply
* [PATCH 1/2] t2003: modernize style
From: John Keeping @ 2013-03-14 20:00 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, David Aguilar, Matt McClure, Tim Henigan, John Keeping
In-Reply-To: <cover.1363291173.git.john@keeping.me.uk>
- Description goes on the test_expect_* line
- Open SQ of test goes on the test_expect_* line
- Closing SQ of test goes on its own line
- Use TAB for indent
Also remove three comments that appear to relate to the development of
the patch before it was committed.
Signed-off-by: John Keeping <john@keeping.me.uk>
---
t/t2003-checkout-cache-mkdir.sh | 143 ++++++++++++++++++++--------------------
1 file changed, 70 insertions(+), 73 deletions(-)
diff --git a/t/t2003-checkout-cache-mkdir.sh b/t/t2003-checkout-cache-mkdir.sh
index 02a4fc5..63fd0a8 100755
--- a/t/t2003-checkout-cache-mkdir.sh
+++ b/t/t2003-checkout-cache-mkdir.sh
@@ -12,85 +12,82 @@ the GIT controlled paths.
. ./test-lib.sh
-test_expect_success \
- 'setup' \
- 'mkdir path1 &&
- echo frotz >path0 &&
- echo rezrov >path1/file1 &&
- git update-index --add path0 path1/file1'
+test_expect_success 'setup' '
+ mkdir path1 &&
+ echo frotz >path0 &&
+ echo rezrov >path1/file1 &&
+ git update-index --add path0 path1/file1
+'
-test_expect_success SYMLINKS \
- 'have symlink in place where dir is expected.' \
- 'rm -fr path0 path1 &&
- mkdir path2 &&
- ln -s path2 path1 &&
- git checkout-index -f -a &&
- test ! -h path1 && test -d path1 &&
- test -f path1/file1 && test ! -f path2/file1'
+test_expect_success SYMLINKS 'have symlink in place where dir is expected.' '
+ rm -fr path0 path1 &&
+ mkdir path2 &&
+ ln -s path2 path1 &&
+ git checkout-index -f -a &&
+ test ! -h path1 && test -d path1 &&
+ test -f path1/file1 && test ! -f path2/file1
+'
-test_expect_success \
- 'use --prefix=path2/' \
- 'rm -fr path0 path1 path2 &&
- mkdir path2 &&
- git checkout-index --prefix=path2/ -f -a &&
- test -f path2/path0 &&
- test -f path2/path1/file1 &&
- test ! -f path0 &&
- test ! -f path1/file1'
+test_expect_success 'use --prefix=path2/' '
+ rm -fr path0 path1 path2 &&
+ mkdir path2 &&
+ git checkout-index --prefix=path2/ -f -a &&
+ test -f path2/path0 &&
+ test -f path2/path1/file1 &&
+ test ! -f path0 &&
+ test ! -f path1/file1
+'
-test_expect_success \
- 'use --prefix=tmp-' \
- 'rm -fr path0 path1 path2 tmp* &&
- git checkout-index --prefix=tmp- -f -a &&
- test -f tmp-path0 &&
- test -f tmp-path1/file1 &&
- test ! -f path0 &&
- test ! -f path1/file1'
+test_expect_success 'use --prefix=tmp-' '
+ rm -fr path0 path1 path2 tmp* &&
+ git checkout-index --prefix=tmp- -f -a &&
+ test -f tmp-path0 &&
+ test -f tmp-path1/file1 &&
+ test ! -f path0 &&
+ test ! -f path1/file1
+'
-test_expect_success \
- 'use --prefix=tmp- but with a conflicting file and dir' \
- 'rm -fr path0 path1 path2 tmp* &&
- echo nitfol >tmp-path1 &&
- mkdir tmp-path0 &&
- git checkout-index --prefix=tmp- -f -a &&
- test -f tmp-path0 &&
- test -f tmp-path1/file1 &&
- test ! -f path0 &&
- test ! -f path1/file1'
+test_expect_success 'use --prefix=tmp- but with a conflicting file and dir' '
+ rm -fr path0 path1 path2 tmp* &&
+ echo nitfol >tmp-path1 &&
+ mkdir tmp-path0 &&
+ git checkout-index --prefix=tmp- -f -a &&
+ test -f tmp-path0 &&
+ test -f tmp-path1/file1 &&
+ test ! -f path0 &&
+ test ! -f path1/file1
+'
-# Linus fix #1
-test_expect_success SYMLINKS \
- 'use --prefix=tmp/orary/ where tmp is a symlink' \
- 'rm -fr path0 path1 path2 tmp* &&
- mkdir tmp1 tmp1/orary &&
- ln -s tmp1 tmp &&
- git checkout-index --prefix=tmp/orary/ -f -a &&
- test -d tmp1/orary &&
- test -f tmp1/orary/path0 &&
- test -f tmp1/orary/path1/file1 &&
- test -h tmp'
+test_expect_success SYMLINKS 'use --prefix=tmp/orary/ where tmp is a symlink' '
+ rm -fr path0 path1 path2 tmp* &&
+ mkdir tmp1 tmp1/orary &&
+ ln -s tmp1 tmp &&
+ git checkout-index --prefix=tmp/orary/ -f -a &&
+ test -d tmp1/orary &&
+ test -f tmp1/orary/path0 &&
+ test -f tmp1/orary/path1/file1 &&
+ test -h tmp
+'
-# Linus fix #2
-test_expect_success SYMLINKS \
- 'use --prefix=tmp/orary- where tmp is a symlink' \
- 'rm -fr path0 path1 path2 tmp* &&
- mkdir tmp1 &&
- ln -s tmp1 tmp &&
- git checkout-index --prefix=tmp/orary- -f -a &&
- test -f tmp1/orary-path0 &&
- test -f tmp1/orary-path1/file1 &&
- test -h tmp'
+test_expect_success SYMLINKS 'use --prefix=tmp/orary- where tmp is a symlink' '
+ rm -fr path0 path1 path2 tmp* &&
+ mkdir tmp1 &&
+ ln -s tmp1 tmp &&
+ git checkout-index --prefix=tmp/orary- -f -a &&
+ test -f tmp1/orary-path0 &&
+ test -f tmp1/orary-path1/file1 &&
+ test -h tmp
+'
-# Linus fix #3
-test_expect_success SYMLINKS \
- 'use --prefix=tmp- where tmp-path1 is a symlink' \
- 'rm -fr path0 path1 path2 tmp* &&
- mkdir tmp1 &&
- ln -s tmp1 tmp-path1 &&
- git checkout-index --prefix=tmp- -f -a &&
- test -f tmp-path0 &&
- test ! -h tmp-path1 &&
- test -d tmp-path1 &&
- test -f tmp-path1/file1'
+test_expect_success SYMLINKS 'use --prefix=tmp- where tmp-path1 is a symlink' '
+ rm -fr path0 path1 path2 tmp* &&
+ mkdir tmp1 &&
+ ln -s tmp1 tmp-path1 &&
+ git checkout-index --prefix=tmp- -f -a &&
+ test -f tmp-path0 &&
+ test ! -h tmp-path1 &&
+ test -d tmp-path1 &&
+ test -f tmp-path1/file1
+'
test_done
--
1.8.2.rc2.4.g7799588
^ permalink raw reply related
* [PATCH 2/2] entry: fix filter lookup
From: John Keeping @ 2013-03-14 20:00 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, David Aguilar, Matt McClure, Tim Henigan, John Keeping
In-Reply-To: <cover.1363291173.git.john@keeping.me.uk>
When looking up the stream filter, write_entry() should be passing the
path of the file in the repository, not the path to which the content is
going to be written. This allows the file to be correctly looked up
against the .gitattributes files in the working tree.
This change makes the streaming case match the non-streaming case which
passes ce->name to convert_to_working_tree later in the same function.
The two tests added here test the different paths through write_entry
since the CRLF filter is a streaming filter but the user-defined smudge
filter is not streamed.
Signed-off-by: John Keeping <john@keeping.me.uk>
---
entry.c | 2 +-
t/t2003-checkout-cache-mkdir.sh | 26 ++++++++++++++++++++++++++
2 files changed, 27 insertions(+), 1 deletion(-)
diff --git a/entry.c b/entry.c
index 17a6bcc..63c52ed 100644
--- a/entry.c
+++ b/entry.c
@@ -145,7 +145,7 @@ static int write_entry(struct cache_entry *ce, char *path, const struct checkout
struct stat st;
if (ce_mode_s_ifmt == S_IFREG) {
- struct stream_filter *filter = get_stream_filter(path, ce->sha1);
+ struct stream_filter *filter = get_stream_filter(ce->name, ce->sha1);
if (filter &&
!streaming_write_entry(ce, path, filter,
state, to_tempfile,
diff --git a/t/t2003-checkout-cache-mkdir.sh b/t/t2003-checkout-cache-mkdir.sh
index 63fd0a8..4c97468 100755
--- a/t/t2003-checkout-cache-mkdir.sh
+++ b/t/t2003-checkout-cache-mkdir.sh
@@ -90,4 +90,30 @@ test_expect_success SYMLINKS 'use --prefix=tmp- where tmp-path1 is a symlink' '
test -f tmp-path1/file1
'
+test_expect_success 'apply filter from working tree .gitattributes with --prefix' '
+ rm -fr path0 path1 path2 tmp* &&
+ mkdir path1 &&
+ mkdir tmp &&
+ git config filter.replace-all.smudge "sed -e s/./=/g" &&
+ git config filter.replace-all.clean cat &&
+ git config filter.replace-all.required true &&
+ echo "file1 filter=replace-all" >path1/.gitattributes &&
+ git checkout-index --prefix=tmp/ -f -a &&
+ echo frotz >expected &&
+ test_cmp expected tmp/path0 &&
+ echo ====== >expected &&
+ test_cmp expected tmp/path1/file1
+'
+
+test_expect_success 'apply CRLF filter from working tree .gitattributes with --prefix' '
+ rm -fr path0 path1 path2 tmp* &&
+ mkdir path1 &&
+ mkdir tmp &&
+ echo "file1 eol=crlf" >path1/.gitattributes &&
+ git checkout-index --prefix=tmp/ -f -a &&
+ echo rezrovQ >expected &&
+ tr \\015 Q <tmp/path1/file1 >actual &&
+ test_cmp expected actual
+'
+
test_done
--
1.8.2.rc2.4.g7799588
^ permalink raw reply related
* [PATCH v2 0/3] difftool --dir-diff: symlink all files matching the working tree
From: John Keeping @ 2013-03-14 20:19 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, David Aguilar, Matt McClure, Tim Henigan,
John Keeping
In-Reply-To: <cover.1363206651.git.john@keeping.me.uk>
Changes since v1:
- A new second patch to make it safer to compare symlink targets in
tests
- Test in patch 3 (formerly patch 2) re-written thanks to feedback from
David Aguilar
This series is based on next, although I think Git's clever enough to
ignore the changes in the context of the t7800 hunk so it should apply
to master as well.
John Keeping (3):
git-difftool(1): fix formatting of --symlink description
difftool: avoid double slashes in symlink targets
difftool --dir-diff: symlink all files matching the working tree
Documentation/git-difftool.txt | 8 +++++---
git-difftool.perl | 25 +++++++++++++++++++++----
t/t7800-difftool.sh | 22 ++++++++++++++++++++++
3 files changed, 48 insertions(+), 7 deletions(-)
--
1.8.2.396.g36b63d6
^ permalink raw reply
* [PATCH v2 1/3] git-difftool(1): fix formatting of --symlink description
From: John Keeping @ 2013-03-14 20:19 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, David Aguilar, Matt McClure, Tim Henigan,
John Keeping
In-Reply-To: <cover.1363291949.git.john@keeping.me.uk>
Signed-off-by: John Keeping <john@keeping.me.uk>
---
Documentation/git-difftool.txt | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt
index e0e12e9..e575fea 100644
--- a/Documentation/git-difftool.txt
+++ b/Documentation/git-difftool.txt
@@ -74,8 +74,8 @@ with custom merge tool commands and has the same value as `$MERGED`.
'git difftool''s default behavior is create symlinks to the
working tree when run in `--dir-diff` mode.
+
- Specifying `--no-symlinks` instructs 'git difftool' to create
- copies instead. `--no-symlinks` is the default on Windows.
+Specifying `--no-symlinks` instructs 'git difftool' to create copies
+instead. `--no-symlinks` is the default on Windows.
-x <command>::
--extcmd=<command>::
--
1.8.2.396.g36b63d6
^ permalink raw reply related
* [PATCH v2 2/3] difftool: avoid double slashes in symlink targets
From: John Keeping @ 2013-03-14 20:19 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, David Aguilar, Matt McClure, Tim Henigan,
John Keeping
In-Reply-To: <cover.1363291949.git.john@keeping.me.uk>
When we add tests for symlinks in "git difftool --dir-diff" it's easier
to check the target path if we don't have to worry about double slashes
separating directories. Remove the trailing slash (if present) from
$workdir before creating the symlinks in order to avoid this.
Signed-off-by: John Keeping <john@keeping.me.uk>
---
git-difftool.perl | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/git-difftool.perl b/git-difftool.perl
index 12231fb..e594f9c 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -209,7 +209,9 @@ EOF
delete($ENV{GIT_INDEX_FILE});
# Changes in the working tree need special treatment since they are
- # not part of the index
+ # not part of the index. Remove any trailing slash from $workdir
+ # before starting to avoid double slashes in symlink targets.
+ $workdir =~ s|/$||;
for my $file (@working_tree) {
my $dir = dirname($file);
unless (-d "$rdir/$dir") {
--
1.8.2.396.g36b63d6
^ permalink raw reply related
* [PATCH v2 3/3] difftool --dir-diff: symlink all files matching the working tree
From: John Keeping @ 2013-03-14 20:19 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, David Aguilar, Matt McClure, Tim Henigan,
John Keeping
In-Reply-To: <cover.1363291949.git.john@keeping.me.uk>
Some users like to edit files in their diff tool when using "git
difftool --dir-diff --symlink" to compare against the working tree but
difftool currently only created symlinks when a file contains unstaged
changes.
Change this behaviour so that symlinks are created whenever the
right-hand side of the comparison has the same SHA1 as the file in the
working tree.
Note that textconv filters are handled in the same way as by git-diff
and if a clean filter is not the inverse of its smudge filter we already
get a null SHA1 from "diff --raw" and will symlink the file without
going through the new hash-object based check.
Signed-off-by: John Keeping <john@keeping.me.uk>
---
Documentation/git-difftool.txt | 4 +++-
git-difftool.perl | 21 ++++++++++++++++++---
t/t7800-difftool.sh | 22 ++++++++++++++++++++++
3 files changed, 43 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt
index e575fea..8361e6e 100644
--- a/Documentation/git-difftool.txt
+++ b/Documentation/git-difftool.txt
@@ -72,7 +72,9 @@ with custom merge tool commands and has the same value as `$MERGED`.
--symlinks::
--no-symlinks::
'git difftool''s default behavior is create symlinks to the
- working tree when run in `--dir-diff` mode.
+ working tree when run in `--dir-diff` mode and the right-hand
+ side of the comparison yields the same content as the file in
+ the working tree.
+
Specifying `--no-symlinks` instructs 'git difftool' to create copies
instead. `--no-symlinks` is the default on Windows.
diff --git a/git-difftool.perl b/git-difftool.perl
index e594f9c..663640d 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -83,6 +83,21 @@ sub exit_cleanup
exit($status | ($status >> 8));
}
+sub use_wt_file
+{
+ my ($repo, $workdir, $file, $sha1, $symlinks) = @_;
+ my $null_sha1 = '0' x 40;
+
+ if ($sha1 eq $null_sha1) {
+ return 1;
+ } elsif (not $symlinks) {
+ return 0;
+ }
+
+ my $wt_sha1 = $repo->command_oneline('hash-object', "$workdir/$file");
+ return $sha1 eq $wt_sha1;
+}
+
sub setup_dir_diff
{
my ($repo, $workdir, $symlinks) = @_;
@@ -159,10 +174,10 @@ EOF
}
if ($rmode ne $null_mode) {
- if ($rsha1 ne $null_sha1) {
- $rindex .= "$rmode $rsha1\t$dst_path\0";
- } else {
+ if (use_wt_file($repo, $workdir, $dst_path, $rsha1, $symlinks)) {
push(@working_tree, $dst_path);
+ } else {
+ $rindex .= "$rmode $rsha1\t$dst_path\0";
}
}
}
diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index 3aab6e1..70e09b6 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -340,6 +340,28 @@ test_expect_success PERL 'difftool --dir-diff' '
stdin_contains file <output
'
+write_script .git/CHECK_SYMLINKS <<\EOF
+for f in file file2 sub/sub
+do
+ echo "$f"
+ readlink "$2/$f"
+done >actual
+EOF
+
+test_expect_success PERL,SYMLINKS 'difftool --dir-diff --symlink without unstaged changes' '
+ cat <<EOF >expect &&
+file
+$(pwd)/file
+file2
+$(pwd)/file2
+sub/sub
+$(pwd)/sub/sub
+EOF
+ git difftool --dir-diff --symlink \
+ --extcmd "./.git/CHECK_SYMLINKS" branch HEAD &&
+ test_cmp actual expect
+'
+
test_expect_success PERL 'difftool --dir-diff ignores --prompt' '
git difftool --dir-diff --prompt --extcmd ls branch >output &&
stdin_contains sub <output &&
--
1.8.2.396.g36b63d6
^ permalink raw reply related
* [PATCH v3] Allow combined diff to ignore white-spaces
From: Antoine Pelisse @ 2013-03-14 21:03 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Johannes Sixt, Antoine Pelisse
In-Reply-To: <7v7glayp4l.fsf@alter.siamese.dyndns.org>
Currently, it's not possible to use the space-ignoring options (-b, -w,
--ignore-space-at-eol) with combined diff. It makes it pretty impossible
to read a merge between a branch that changed all tabs to spaces, and a
branch with functional changes.
Pass diff flags to diff engine, so that combined diff behaves as normal
diff does with spaces.
Also coalesce lines that are removed from both (or more) parents.
It also means that a conflict-less merge done using a ignore-* strategy
option will not show any conflict if shown in combined-diff using the
same option.
Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
---
Changes:
- Fixed comments
- Fixed tests (following Johannes suggestions)
combine-diff.c | 57 +++++++++++++++++++++---
t/t4038-diff-combined.sh | 111 ++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 161 insertions(+), 7 deletions(-)
diff --git a/combine-diff.c b/combine-diff.c
index 35d41cd..6288485 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -5,6 +5,7 @@
#include "diffcore.h"
#include "quote.h"
#include "xdiff-interface.h"
+#include "xdiff/xmacros.h"
#include "log-tree.h"
#include "refs.h"
#include "userdiff.h"
@@ -122,7 +123,47 @@ static char *grab_blob(const unsigned char *sha1, unsigned int mode,
return blob;
}
-static void append_lost(struct sline *sline, int n, const char *line, int len)
+static int match_string_spaces(const char *line1, int len1,
+ const char *line2, int len2,
+ long flags)
+{
+ if (flags & XDF_WHITESPACE_FLAGS) {
+ for (; len1 > 0 && XDL_ISSPACE(line1[len1 - 1]); len1--);
+ for (; len2 > 0 && XDL_ISSPACE(line2[len2 - 1]); len2--);
+ }
+
+ if (!(flags & (XDF_IGNORE_WHITESPACE | XDF_IGNORE_WHITESPACE_CHANGE)))
+ return (len1 == len2 && !memcmp(line1, line2, len1));
+
+ while (len1 > 0 && len2 > 0) {
+ len1--;
+ len2--;
+ if (XDL_ISSPACE(line1[len1]) || XDL_ISSPACE(line2[len2])) {
+ if ((flags & XDF_IGNORE_WHITESPACE_CHANGE) &&
+ (!XDL_ISSPACE(line1[len1]) || !XDL_ISSPACE(line2[len2])))
+ return 0;
+
+ for (; len1 > 0 && XDL_ISSPACE(line1[len1]); len1--);
+ for (; len2 > 0 && XDL_ISSPACE(line2[len2]); len2--);
+ }
+ if (line1[len1] != line2[len2])
+ return 0;
+ }
+
+ if (flags & XDF_IGNORE_WHITESPACE) {
+ /* Consume remaining spaces */
+ for (; len1 > 0 && XDL_ISSPACE(line1[len1 - 1]); len1--);
+ for (; len2 > 0 && XDL_ISSPACE(line2[len2 - 1]); len2--);
+ }
+
+ /* We matched full line1 and line2 */
+ if (!len1 && !len2)
+ return 1;
+
+ return 0;
+}
+
+static void append_lost(struct sline *sline, int n, const char *line, int len, long flags)
{
struct lline *lline;
unsigned long this_mask = (1UL<<n);
@@ -133,8 +174,8 @@ static void append_lost(struct sline *sline, int n, const char *line, int len)
if (sline->lost_head) {
lline = sline->next_lost;
while (lline) {
- if (lline->len == len &&
- !memcmp(lline->line, line, len)) {
+ if (match_string_spaces(lline->line, lline->len,
+ line, len, flags)) {
lline->parent_map |= this_mask;
sline->next_lost = lline->next;
return;
@@ -162,6 +203,7 @@ struct combine_diff_state {
int n;
struct sline *sline;
struct sline *lost_bucket;
+ long flags;
};
static void consume_line(void *state_, char *line, unsigned long len)
@@ -201,7 +243,7 @@ static void consume_line(void *state_, char *line, unsigned long len)
return; /* not in any hunk yet */
switch (line[0]) {
case '-':
- append_lost(state->lost_bucket, state->n, line+1, len-1);
+ append_lost(state->lost_bucket, state->n, line+1, len-1, state->flags);
break;
case '+':
state->sline[state->lno-1].flag |= state->nmask;
@@ -215,7 +257,7 @@ static void combine_diff(const unsigned char *parent, unsigned int mode,
struct sline *sline, unsigned int cnt, int n,
int num_parent, int result_deleted,
struct userdiff_driver *textconv,
- const char *path)
+ const char *path, long flags)
{
unsigned int p_lno, lno;
unsigned long nmask = (1UL << n);
@@ -231,9 +273,10 @@ static void combine_diff(const unsigned char *parent, unsigned int mode,
parent_file.ptr = grab_blob(parent, mode, &sz, textconv, path);
parent_file.size = sz;
memset(&xpp, 0, sizeof(xpp));
- xpp.flags = 0;
+ xpp.flags = flags;
memset(&xecfg, 0, sizeof(xecfg));
memset(&state, 0, sizeof(state));
+ state.flags = flags;
state.nmask = nmask;
state.sline = sline;
state.lno = 1;
@@ -962,7 +1005,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
elem->parent[i].mode,
&result_file, sline,
cnt, i, num_parent, result_deleted,
- textconv, elem->path);
+ textconv, elem->path, opt->xdl_opts);
}
show_hunks = make_hunks(sline, cnt, num_parent, dense);
diff --git a/t/t4038-diff-combined.sh b/t/t4038-diff-combined.sh
index 614425a..b7e16a7 100755
--- a/t/t4038-diff-combined.sh
+++ b/t/t4038-diff-combined.sh
@@ -3,6 +3,7 @@
test_description='combined diff'
. ./test-lib.sh
+. "$TEST_DIRECTORY"/diff-lib.sh
setup_helper () {
one=$1 branch=$2 side=$3 &&
@@ -113,4 +114,114 @@ test_expect_success 'check --cc --raw with forty trees' '
grep "^::::::::::::::::::::::::::::::::::::::::[^:]" out
'
+test_expect_success 'setup combined ignore spaces' '
+ git checkout master &&
+ >test &&
+ git add test &&
+ git commit -m initial &&
+
+ tr -d Q <<-\EOF >test &&
+ always coalesce
+ eol space coalesce Q
+ space change coalesce
+ all spa ces coalesce
+ eol spaces Q
+ space change
+ all spa ces
+ EOF
+ git commit -m "test space change" -a &&
+
+ git checkout -b side HEAD^ &&
+ tr -d Q <<-\EOF >test &&
+ always coalesce
+ eol space coalesce
+ space change coalesce
+ all spaces coalesce
+ eol spaces
+ space change
+ all spaces
+ EOF
+ git commit -m "test other space changes" -a &&
+
+ test_must_fail git merge master &&
+ tr -d Q <<-\EOF >test &&
+ eol spaces Q
+ space change
+ all spa ces
+ EOF
+ git commit -m merged -a
+'
+
+test_expect_success 'check combined output (no ignore space)' '
+ git show >actual.tmp &&
+ sed -e "1,/^@@@/d" < actual.tmp >actual &&
+ tr -d Q <<-\EOF >expected &&
+ --always coalesce
+ - eol space coalesce
+ - space change coalesce
+ - all spaces coalesce
+ - eol spaces
+ - space change
+ - all spaces
+ -eol space coalesce Q
+ -space change coalesce
+ -all spa ces coalesce
+ + eol spaces Q
+ + space change
+ + all spa ces
+ EOF
+ compare_diff_patch expected actual
+'
+
+test_expect_success 'check combined output (ignore space at eol)' '
+ git show --ignore-space-at-eol >actual.tmp &&
+ sed -e "1,/^@@@/d" < actual.tmp >actual &&
+ tr -d Q <<-\EOF >expected &&
+ --always coalesce
+ --eol space coalesce
+ - space change coalesce
+ - all spaces coalesce
+ -space change coalesce
+ -all spa ces coalesce
+ eol spaces Q
+ - space change
+ - all spaces
+ + space change
+ + all spa ces
+ EOF
+ compare_diff_patch expected actual
+'
+
+test_expect_success 'check combined output (ignore space change)' '
+ git show -b >actual.tmp &&
+ sed -e "1,/^@@@/d" < actual.tmp >actual &&
+ tr -d Q <<-\EOF >expected &&
+ --always coalesce
+ --eol space coalesce
+ --space change coalesce
+ - all spaces coalesce
+ -all spa ces coalesce
+ eol spaces Q
+ space change
+ - all spaces
+ + all spa ces
+ EOF
+ compare_diff_patch expected actual
+'
+
+test_expect_success 'check combined output (ignore all spaces)' '
+ git show -w >actual.tmp &&
+ sed -e "1,/^@@@/d" < actual.tmp >actual &&
+ tr -d Q <<-\EOF >expected &&
+ --always coalesce
+ --eol space coalesce
+ --space change coalesce
+ --all spaces coalesce
+ eol spaces Q
+ space change
+ all spa ces
+ EOF
+ compare_diff_patch expected actual
+'
+
test_done
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH v2 2/3] difftool: avoid double slashes in symlink targets
From: Junio C Hamano @ 2013-03-14 21:19 UTC (permalink / raw)
To: John Keeping; +Cc: git, David Aguilar, Matt McClure, Tim Henigan
In-Reply-To: <b10c6d19bd4004887868dd7626320bd676fee540.1363291949.git.john@keeping.me.uk>
John Keeping <john@keeping.me.uk> writes:
> When we add tests for symlinks in "git difftool --dir-diff" it's easier
> to check the target path if we don't have to worry about double slashes
> separating directories. Remove the trailing slash (if present) from
> $workdir before creating the symlinks in order to avoid this.
Yup, and it is a good basic hygiene even without tests that expect
the exact pathnames.
The code would work even when your $workdir is at the root of the
filesystem; the patch looks good.
Thanks.
> Signed-off-by: John Keeping <john@keeping.me.uk>
> ---
> git-difftool.perl | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/git-difftool.perl b/git-difftool.perl
> index 12231fb..e594f9c 100755
> --- a/git-difftool.perl
> +++ b/git-difftool.perl
> @@ -209,7 +209,9 @@ EOF
> delete($ENV{GIT_INDEX_FILE});
>
> # Changes in the working tree need special treatment since they are
> - # not part of the index
> + # not part of the index. Remove any trailing slash from $workdir
> + # before starting to avoid double slashes in symlink targets.
> + $workdir =~ s|/$||;
> for my $file (@working_tree) {
> my $dir = dirname($file);
> unless (-d "$rdir/$dir") {
^ permalink raw reply
* Re: [PATCH v2 3/3] difftool --dir-diff: symlink all files matching the working tree
From: Junio C Hamano @ 2013-03-14 21:28 UTC (permalink / raw)
To: John Keeping; +Cc: git, David Aguilar, Matt McClure, Tim Henigan
In-Reply-To: <ae17a152cadc650920c6446a4493384cc2e77309.1363291949.git.john@keeping.me.uk>
John Keeping <john@keeping.me.uk> writes:
> diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
> index 3aab6e1..70e09b6 100755
> --- a/t/t7800-difftool.sh
> +++ b/t/t7800-difftool.sh
> @@ -340,6 +340,28 @@ test_expect_success PERL 'difftool --dir-diff' '
> stdin_contains file <output
> '
>
> +write_script .git/CHECK_SYMLINKS <<\EOF
> +for f in file file2 sub/sub
> +do
> + echo "$f"
> + readlink "$2/$f"
> +done >actual
> +EOF
When you later want to enhance the test to check a combination of
difftool arguments where some paths are expected to become links and
others are expected to become real files, wouldn't this helper
become a bit awkward to use? The element that expects a real file
could be an empty line to what corresponds to the output from
readlink, but still...
If t/ directory (or when the test is run with --root=<there>) is
aliased with symlinks in such a way that "cd <there> && $(pwd)" does
not match <there>, would this check with $(pwd) still work, I have
to wonder?
> +test_expect_success PERL,SYMLINKS 'difftool --dir-diff --symlink without unstaged changes' '
> + cat <<EOF >expect &&
> +file
> +$(pwd)/file
> +file2
> +$(pwd)/file2
> +sub/sub
> +$(pwd)/sub/sub
> +EOF
You can do this to align them nicer (note the "-" before EOF):
cat >expect <<-EOF &&
file
$(pwd)/file
...
EOF
> + git difftool --dir-diff --symlink \
> + --extcmd "./.git/CHECK_SYMLINKS" branch HEAD &&
> + test_cmp actual expect
> +'
> +
Thanks.
^ permalink raw reply
* Re: [PATCH v3] Allow combined diff to ignore white-spaces
From: Junio C Hamano @ 2013-03-14 21:43 UTC (permalink / raw)
To: Antoine Pelisse; +Cc: git, Johannes Sixt
In-Reply-To: <1363294994-3127-1-git-send-email-apelisse@gmail.com>
Antoine Pelisse <apelisse@gmail.com> writes:
> +test_expect_success 'check combined output (no ignore space)' '
> + git show >actual.tmp &&
> + sed -e "1,/^@@@/d" < actual.tmp >actual &&
> + tr -d Q <<-\EOF >expected &&
> + --always coalesce
> + - eol space coalesce
> + - space change coalesce
> + - all spaces coalesce
> + - eol spaces
> + - space change
> + - all spaces
> + -eol space coalesce Q
> + -space change coalesce
> + -all spa ces coalesce
> + + eol spaces Q
> + + space change
> + + all spa ces
> + EOF
> + compare_diff_patch expected actual
> +'
Nicely constructed ;-)
Thanks.
^ permalink raw reply
* Re: [PATCH 2/2] entry: fix filter lookup
From: Junio C Hamano @ 2013-03-14 21:50 UTC (permalink / raw)
To: John Keeping; +Cc: git, David Aguilar, Matt McClure, Tim Henigan
In-Reply-To: <bede6d48dd44f7ed4a11da5821bb112b700475d5.1363291173.git.john@keeping.me.uk>
John Keeping <john@keeping.me.uk> writes:
> diff --git a/t/t2003-checkout-cache-mkdir.sh b/t/t2003-checkout-cache-mkdir.sh
> index 63fd0a8..4c97468 100755
> --- a/t/t2003-checkout-cache-mkdir.sh
> +++ b/t/t2003-checkout-cache-mkdir.sh
> @@ -90,4 +90,30 @@ test_expect_success SYMLINKS 'use --prefix=tmp- where tmp-path1 is a symlink' '
> test -f tmp-path1/file1
> '
>
> +test_expect_success 'apply filter from working tree .gitattributes with --prefix' '
> + rm -fr path0 path1 path2 tmp* &&
> + mkdir path1 &&
> + mkdir tmp &&
> + git config filter.replace-all.smudge "sed -e s/./=/g" &&
> + git config filter.replace-all.clean cat &&
> + git config filter.replace-all.required true &&
> + echo "file1 filter=replace-all" >path1/.gitattributes &&
> + git checkout-index --prefix=tmp/ -f -a &&
> + echo frotz >expected &&
> + test_cmp expected tmp/path0 &&
> + echo ====== >expected &&
> + test_cmp expected tmp/path1/file1
> +'
> +
> +test_expect_success 'apply CRLF filter from working tree .gitattributes with --prefix' '
> + rm -fr path0 path1 path2 tmp* &&
> + mkdir path1 &&
> + mkdir tmp &&
> + echo "file1 eol=crlf" >path1/.gitattributes &&
> + git checkout-index --prefix=tmp/ -f -a &&
> + echo rezrovQ >expected &&
> + tr \\015 Q <tmp/path1/file1 >actual &&
> + test_cmp expected actual
> +'
Nicely done. Thanks.
^ permalink raw reply
* Re: Delivery Status Notification (Failure)
From: Harring Figueiredo @ 2013-03-14 22:01 UTC (permalink / raw)
To: git
In-Reply-To: <bcaec5396b50ba47be04d7e99955@google.com>
Hello folks,
Just want to confirm that this is not a bug:
<code>
bunde.c: 443
printf_ln(Q_("The bundle requires this ref",
"The bundle requires these %d refs",
r->nr),
r->nr);
</code>
seems to be missing %d parameter on the first message.
Please let me know if this is a bug or if it indeed should be like this.
I am not a expert on gettext :-(
Thank you!
-hff
^ permalink raw reply
* Re: Delivery Status Notification (Failure)
From: Junio C Hamano @ 2013-03-14 22:12 UTC (permalink / raw)
To: Harring Figueiredo; +Cc: git
In-Reply-To: <CAN_hzmomGYpfh1MchCnrSwv3jv201DL2wLEXeYrG+=eNKwX4Yw@mail.gmail.com>
Harring Figueiredo <harringf@gmail.com> writes:
> Just want to confirm that this is not a bug:
> ...
> printf_ln(Q_("The bundle requires this ref",
> "The bundle requires these %d refs",
> r->nr),
> r->nr);
> ...
> seems to be missing %d parameter on the first message.
Perhaps visit
http://www.gnu.org/software/gettext/manual/gettext.html#Plural-forms
look for "In the English singular case,..." and read the
explanation there?
^ permalink raw reply
* Re: [PATCH v2 3/3] difftool --dir-diff: symlink all files matching the working tree
From: John Keeping @ 2013-03-14 22:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, David Aguilar, Matt McClure, Tim Henigan
In-Reply-To: <7v620ty8lc.fsf@alter.siamese.dyndns.org>
On Thu, Mar 14, 2013 at 02:28:31PM -0700, Junio C Hamano wrote:
> John Keeping <john@keeping.me.uk> writes:
>
> > diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
> > index 3aab6e1..70e09b6 100755
> > --- a/t/t7800-difftool.sh
> > +++ b/t/t7800-difftool.sh
> > @@ -340,6 +340,28 @@ test_expect_success PERL 'difftool --dir-diff' '
> > stdin_contains file <output
> > '
> >
> > +write_script .git/CHECK_SYMLINKS <<\EOF
> > +for f in file file2 sub/sub
> > +do
> > + echo "$f"
> > + readlink "$2/$f"
> > +done >actual
> > +EOF
>
> When you later want to enhance the test to check a combination of
> difftool arguments where some paths are expected to become links and
> others are expected to become real files, wouldn't this helper
> become a bit awkward to use? The element that expects a real file
> could be an empty line to what corresponds to the output from
> readlink, but still...
>
> If t/ directory (or when the test is run with --root=<there>) is
> aliased with symlinks in such a way that "cd <there> && $(pwd)" does
> not match <there>, would this check with $(pwd) still work, I have
> to wonder?
It looks like t3903 uses "ls -l" for this sort of test, perhaps
something like this covers these cases better:
write_script .git/CHECK_SYMLINKS <<\EOF
for f in file file2 sub/sub
do
ls -l "$2/$f" >"$f".actual
done
EOF
...
workdir=$(git rev-parse --show-toplevel)
grep "-> $workdir/file" file.actual
grep "-> $workdir/file2" file2.actual
grep "-> $workdir/sub/sub" sub/sub.actual
It looks like we already rely on that output format in t3903 so I think
that is safe, but it would be nice to have a better way to say "does
this link point to that file?". I can't think of a way to do that that
doesn't seem far too complicated for what's required here.
> > +test_expect_success PERL,SYMLINKS 'difftool --dir-diff --symlink without unstaged changes' '
> > + cat <<EOF >expect &&
> > +file
> > +$(pwd)/file
> > +file2
> > +$(pwd)/file2
> > +sub/sub
> > +$(pwd)/sub/sub
> > +EOF
>
> You can do this to align them nicer (note the "-" before EOF):
>
> cat >expect <<-EOF &&
> file
> $(pwd)/file
> ...
> EOF
>
> > + git difftool --dir-diff --symlink \
> > + --extcmd "./.git/CHECK_SYMLINKS" branch HEAD &&
> > + test_cmp actual expect
> > +'
> > +
>
> Thanks.
^ permalink raw reply
* Re: [PATCH v2 3/3] difftool --dir-diff: symlink all files matching the working tree
From: Junio C Hamano @ 2013-03-14 22:31 UTC (permalink / raw)
To: John Keeping; +Cc: git, David Aguilar, Matt McClure, Tim Henigan
In-Reply-To: <20130314222415.GC4256@serenity.lan>
John Keeping <john@keeping.me.uk> writes:
>> > +for f in file file2 sub/sub
>> > +do
>> > + echo "$f"
>> > + readlink "$2/$f"
>> > +done >actual
>> > +EOF
>>
>> When you later want to enhance the test to check a combination of
>> difftool arguments where some paths are expected to become links and
>> others are expected to become real files, wouldn't this helper
>> become a bit awkward to use? The element that expects a real file
>> could be an empty line to what corresponds to the output from
>> readlink, but still...
>> ...
>
> It looks like t3903 uses "ls -l" for this sort of test, perhaps
> something like this covers these cases better:
> ...
> grep "-> $workdir/file" file.actual
Writing it without -e would confuse some implementations of grep
into thinking "-" introduces an option, realizing it does not
support the "->" option, and then barfing ;-)
What I had in mind was more along the lines of...
for f
do
echo "$f"
readlink "$2/$f" || echo "# not a link $f"
done
so that your "expect" list can become
file
$(pwd)/realdir/file
modifiedone.txt
# not a link modifiedone.txt
In any case, this "say blank if you expect a non symlink" is not an
urgent issue that needs to be fixed or anything like that, so let's
queue the v2 for now and see what happens.
Thanks.
^ permalink raw reply
* [RFD] Combine diff improvements
From: Antoine Pelisse @ 2013-03-14 22:32 UTC (permalink / raw)
To: git
Hi,
I've been working on combine diff recently and have seen that the current
implementation does not produce optimal results for coalescing lost lines.
I would like to discuss some improvements.
I think we can define an optimal solution as the shortest diff
output. That is that we coalesced as much line as we could.
It means that if each parent has the same file removed, the output will
be n lines long (all lines coalesce to each others).
The current implementation doesn't look for optimal solution but for the
easiest/greedy solution. It also stops matching with a parent if it
can't find a match. As an example, it means that losing [1, 2, 3, 4] from
parent A, and [3, 1, 2, 4] from parent B would give:
- 1
- 2
--3
- 4
-1
-2
-4
While if we invert the two parents order we will have:
- 3
--1
--2
- 4
-3
-4
While clearly, in both cases, the optimal solution would be close to:
- 3
--1
--2
-3
--4
Let's say we have only one empty file (deleted), with p parents. In each
parent, the file was n lines long, but the lines don't have to be the
same.
It clearly looks like an extension/variation of the Longest Common
Subsequence (LCS) problem, but with p sequences (and the common
subsequences doesn't have to be common to all sequences).
LCS dynamic programming is O(n²) in time and space complexity for two
sequences.
We could extend it this way:
After processing two of the p parents, let's find LCS and build a new
optimal list of removals. Then find LCS again between this new list and
the third parent removals. And repeat until we made each parents.
Best-case analysis:
All p parents have the same n lines.
We will find LCS and provide a n lines (the same lines) new list in
O(n²), and then run it again in O(n²) with the next parent, etc.
It will end-up being O(pn²).
Worst-case analysis:
All p parents have no lines in common.
We will find LCS and provide a 2n new list in O(n²).
Then we run it again in O(2n x n), and again O(3n x n), etc, until
O(pn x n).
When we sum these all, we end-up with O(p² x n²)
Does it make any sense ? And is it worth implementing ?
Cheers,
Antoine
^ permalink raw reply
* Re: [PATCH/RFC] http_init: only initialize SSL for https
From: Johannes Schindelin @ 2013-03-14 22:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: kusmabite, git, msysgit
In-Reply-To: <7vmwu6x72q.fsf@alter.siamese.dyndns.org>
Hi Junio,
On Thu, 14 Mar 2013, Junio C Hamano wrote:
> Erik Faye-Lund <kusmabite@gmail.com> writes:
>
> >> I wonder whether we want to have something like this instead:
> >>
> >> flags = CURL_GLOBAL_ALL;
> >> if (prefixcmp(url, "https:"))
> >> flags &= ^CURL_GLOBAL_SSL;
> >> curl_global_init(flags);
> >>
> >> I do see that CURL_GLOBAL_ALL is #define'd as CURL_GLOBAL_WIN32 |
> >> CURL_GLOBAL_SSL in curl.h, but that might change in the future, no?
> >
> > Good suggestion. But perhaps we'd want to use CURL_GLOBAL_DEFAULT
> > instead?
>
> That as a follow-up suggestion may be fine but if you go that route,
> you would need to explicitly flip SSL on when you know it is going
> to an SSL destination.
>
> The way to determine SSL-ness has to be rock solid and that is much
> more important than ALL vs DEFAULT. Is prefixcmp(url, "https://")
> the right way to do so? Do we use this codepath only for HTTPS, or
> does anybody use other protocol cURL supports over SSL with this,
> too?
Apparently, ftps is also handled by cURL and most likely requires SSL.
How about optimizing for the common case and instead of prefixcmp(url,
"https:")) ask for !prefixcmp(url, "http:")?
Ciao,
Dscho
--
--
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.
You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en
---
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
^ permalink raw reply
* Re: [PATCH/RFC] http_init: only initialize SSL for https
From: Junio C Hamano @ 2013-03-14 22:45 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: kusmabite, git, msysgit
In-Reply-To: <alpine.DEB.1.00.1303142333170.3794@s15462909.onlinehome-server.info>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> Apparently, ftps is also handled by cURL and most likely requires SSL.
>
> How about optimizing for the common case and instead of prefixcmp(url,
> "https:")) ask for !prefixcmp(url, "http:")?
I think that is a very sensible way to go.
As to ALL vs DEFAULT, given that its manual page is riddled with a
scary warning:
This function must be called at least once within a program (a
program is all the code that shares a memory space) before the
program calls any other function in libcurl. The environment it sets
up is constant for the life of the program and is the same for every
program, so multiple calls have the same effect as one call. ... In
normal operation, you must specify CURL_GLOBAL_ALL. Don't use any
other value unless you are familiar with it and mean to control
internal operations of libcurl.
I think we should stick to ALL. So
flags = CURL_GLOBAL_ALL;
if (!prefixcmp(url, "http:"))
flags &= ~CURL_GLOBAL_SSL;
would be the way to go.
But this is assuming that nobody feeds our client a http:// URL to
the server that redirects us to the https:// version (or we do not
follow such a redirect). I offhand do not know if that is a valid
assumption, though.
--
--
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.
You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en
---
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
^ permalink raw reply
* Re: [RFD] Combine diff improvements
From: Junio C Hamano @ 2013-03-14 22:52 UTC (permalink / raw)
To: Antoine Pelisse; +Cc: git
In-Reply-To: <CALWbr2zY3uo==QTd69t1eXNS4-CX1w3T-AaMjOLVmxj-SMJyvg@mail.gmail.com>
Antoine Pelisse <apelisse@gmail.com> writes:
> I would like to discuss some improvements.
;-)
> The current implementation doesn't look for optimal solution but for the
> easiest/greedy solution.
Yes, you nailed it.
> Does it make any sense? And is it worth implementing?
Probably yes, and definitely.
^ permalink raw reply
* Re: [PATCH/RFC] http_init: only initialize SSL for https
From: Erik Faye-Lund @ 2013-03-14 23:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, git, msysgit
In-Reply-To: <7vk3p9wqh5.fsf@alter.siamese.dyndns.org>
On Thu, Mar 14, 2013 at 11:45 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
>> Apparently, ftps is also handled by cURL and most likely requires SSL.
>>
>> How about optimizing for the common case and instead of prefixcmp(url,
>> "https:")) ask for !prefixcmp(url, "http:")?
>
> I think that is a very sensible way to go.
>
> As to ALL vs DEFAULT, given that its manual page is riddled with a
> scary warning:
>
> This function must be called at least once within a program (a
> program is all the code that shares a memory space) before the
> program calls any other function in libcurl. The environment it sets
> up is constant for the life of the program and is the same for every
> program, so multiple calls have the same effect as one call. ... In
> normal operation, you must specify CURL_GLOBAL_ALL. Don't use any
> other value unless you are familiar with it and mean to control
> internal operations of libcurl.
>
> I think we should stick to ALL. So
>
> flags = CURL_GLOBAL_ALL;
> if (!prefixcmp(url, "http:"))
> flags &= ~CURL_GLOBAL_SSL;
>
> would be the way to go.
>
> But this is assuming that nobody feeds our client a http:// URL to
> the server that redirects us to the https:// version (or we do not
> follow such a redirect). I offhand do not know if that is a valid
> assumption, though.
>
Thanks, both. Very sensible points. I'll re-roll a new version
tomorrow, but it could indeed be that the redirect-case can make this
a no-go.
--
--
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.
You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en
---
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
^ permalink raw reply
* Re: Delivery Status Notification (Failure)
From: Harring Figueiredo @ 2013-03-14 23:36 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vsj3xwrzn.fsf@alter.siamese.dyndns.org>
Got it!
From the recommended site:
--
In the English singular case, the number – always 1 – can be replaced
with "one":
printf (ngettext ("One file removed", "%d files removed", n), n);
This works because the ‘printf’ function discards excess arguments
that are not consumed by the format string.
--
Junio, thank you so much for the reply and link to the docs.
-hff
On Thu, Mar 14, 2013 at 6:12 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Harring Figueiredo <harringf@gmail.com> writes:
>
>> Just want to confirm that this is not a bug:
>> ...
>> printf_ln(Q_("The bundle requires this ref",
>> "The bundle requires these %d refs",
>> r->nr),
>> r->nr);
>> ...
>> seems to be missing %d parameter on the first message.
>
> Perhaps visit
>
> http://www.gnu.org/software/gettext/manual/gettext.html#Plural-forms
>
> look for "In the English singular case,..." and read the
> explanation there?
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox