Bug report : bad filter-branch (OSX only)

2015-04-26 Thread Olivier ROLAND
Hello,

Seem to be a bug.

OSX 10.10.3 git 2.3.6 HFS+ case-sensitive

How to reproduce :
Step 1 : git clone https://github.com/begeric/FastParsers.git
Step 2 : cd FastParsers/
Step 3 : git filter-branch --env-filter 'if [ 0 = 1 ]; then echo 0; fi' -- --all

Result on OSX :
Rewrite 65df7c5ac1ed956252b07b8c911ad7eba0a15c2b (206/206)
Ref 'refs/heads/experiment' was rewritten
Ref 'refs/remotes/origin/experiment' was rewritten
WARNING: Ref 'refs/remotes/origin/experiment' is unchanged
Ref 'refs/remotes/origin/master' was rewritten

Result on Debian :
Rewrite 65df7c5ac1ed956252b07b8c911ad7eba0a15c2b (206/206)
WARNING: Ref 'refs/heads/experiment' is unchanged
WARNING: Ref 'refs/remotes/origin/experiment' is unchanged
WARNING: Ref 'refs/remotes/origin/experiment' is unchanged
WARNING: Ref 'refs/remotes/origin/master' is unchanged

Do you have any thoughts on this ?

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 v5 2/5] setup: sanity check file size in read_gitfile_gently

2015-04-26 Thread Erik Elfström
read_gitfile_gently will allocate a buffer to fit the entire file that
should be read. Add a sanity check of the file size before opening to
avoid allocating a potentially huge amount of memory if we come across
a large file that someone happened to name .git. The limit is set to
a sufficiently unreasonable size that should never be exceeded by a
genuine .git file.

Signed-off-by: Erik Elfström erik.elfst...@gmail.com
---
 cache.h | 1 +
 setup.c | 6 ++
 2 files changed, 7 insertions(+)

diff --git a/cache.h b/cache.h
index 868e4d3..c9f1f8e 100644
--- a/cache.h
+++ b/cache.h
@@ -439,6 +439,7 @@ extern const char *get_git_work_tree(void);
 #define READ_GITFILE_ERR_INVALID_FORMAT 5
 #define READ_GITFILE_ERR_NO_PATH 6
 #define READ_GITFILE_ERR_NOT_A_REPO 7
+#define READ_GITFILE_ERR_TOO_LARGE 8
 extern const char *read_gitfile_gently(const char *path, int 
*return_error_code);
 #define read_gitfile(path) read_gitfile_gently((path), NULL)
 extern const char *resolve_gitdir(const char *suspect);
diff --git a/setup.c b/setup.c
index c4538ca..792c37b 100644
--- a/setup.c
+++ b/setup.c
@@ -364,6 +364,10 @@ const char *read_gitfile_gently(const char *path, int 
*return_error_code)
error_code = READ_GITFILE_ERR_OPEN_FAILED;
goto cleanup_return;
}
+   if (st.st_size  PATH_MAX * 4) {
+   error_code = READ_GITFILE_ERR_TOO_LARGE;
+   goto cleanup_return;
+   }
buf = xmalloc(st.st_size + 1);
len = read_in_full(fd, buf, st.st_size);
close(fd);
@@ -418,6 +422,8 @@ cleanup_return:
return NULL;
case READ_GITFILE_ERR_OPEN_FAILED:
die_errno(Error opening '%s', path);
+   case READ_GITFILE_ERR_TOO_LARGE:
+   die(Too large to be a .git file: '%s', path);
case READ_GITFILE_ERR_READ_FAILED:
die(Error reading %s, path);
case READ_GITFILE_ERR_INVALID_FORMAT:
-- 
2.4.0.rc3.8.gbb31afb

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v5 4/5] p7300: add performance tests for clean

2015-04-26 Thread Erik Elfström
The tests are run in dry-run mode to avoid having to restore the test
directories for each timed iteration. Using dry-run is an acceptable
compromise since we are mostly interested in the initial computation
of what to clean and not so much in the cleaning it self.

Signed-off-by: Erik Elfström erik.elfst...@gmail.com
---
 t/perf/p7300-clean.sh | 31 +++
 1 file changed, 31 insertions(+)
 create mode 100755 t/perf/p7300-clean.sh

diff --git a/t/perf/p7300-clean.sh b/t/perf/p7300-clean.sh
new file mode 100755
index 000..6ae55ec
--- /dev/null
+++ b/t/perf/p7300-clean.sh
@@ -0,0 +1,31 @@
+#!/bin/sh
+
+test_description=Test git-clean performance
+
+. ./perf-lib.sh
+
+test_perf_large_repo
+test_checkout_worktree
+
+test_expect_success 'setup untracked directory with many sub dirs' '
+   rm -rf 500_sub_dirs 10_sub_dirs clean_test_dir 
+   mkdir 500_sub_dirs 10_sub_dirs clean_test_dir 
+   for i in $(test_seq 1 500)
+   do
+   mkdir 500_sub_dirs/dir$i || return $?
+   done 
+   for i in $(test_seq 1 200)
+   do
+   cp -r 500_sub_dirs 10_sub_dirs/dir$i || return $?
+   done
+'
+
+test_perf 'clean many untracked sub dirs, check for nested git' '
+   git clean -n -q -f -d 10_sub_dirs/
+'
+
+test_perf 'clean many untracked sub dirs, ignore nested git' '
+   git clean -n -q -f -f -d 10_sub_dirs/
+'
+
+test_done
-- 
2.4.0.rc3.8.gbb31afb

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v5 3/5] t7300: add tests to document behavior of clean and nested git

2015-04-26 Thread Erik Elfström
Signed-off-by: Erik Elfström erik.elfst...@gmail.com
---
 t/t7300-clean.sh | 128 +++
 1 file changed, 128 insertions(+)

diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh
index 99be5d9..11f3a6d 100755
--- a/t/t7300-clean.sh
+++ b/t/t7300-clean.sh
@@ -455,6 +455,134 @@ test_expect_success 'nested git work tree' '
! test -d bar
 '
 
+test_expect_failure 'should clean things that almost look like git but are 
not' '
+   rm -fr almost_git almost_bare_git almost_submodule 
+   mkdir -p almost_git/.git/objects 
+   mkdir -p almost_git/.git/refs 
+   cat almost_git/.git/HEAD -\EOF 
+   garbage
+   EOF
+   cp -r almost_git/.git/ almost_bare_git 
+   mkdir almost_submodule/ 
+   cat almost_submodule/.git -\EOF 
+   garbage
+   EOF
+   test_when_finished rm -rf almost_* 
+   ## This will fail due to die(Invalid gitfile format: %s, path); in
+   ## setup.c:read_gitfile.
+   git clean -f -d 
+   test_path_is_missing almost_git 
+   test_path_is_missing almost_bare_git 
+   test_path_is_missing almost_submodule
+'
+
+test_expect_success 'should not clean submodules' '
+   rm -fr repo to_clean sub1 sub2 
+   mkdir repo to_clean 
+   (
+   cd repo 
+   git init 
+   test_commit msg hello.world
+   ) 
+   git submodule add ./repo/.git sub1 
+   git commit -m sub1 
+   git branch before_sub2 
+   git submodule add ./repo/.git sub2 
+   git commit -m sub2 
+   git checkout before_sub2 
+   to_clean/should_clean.this 
+   git clean -f -d 
+   test_path_is_file repo/.git/index 
+   test_path_is_file repo/hello.world 
+   test_path_is_file sub1/.git 
+   test_path_is_file sub1/hello.world 
+   test_path_is_file sub2/.git 
+   test_path_is_file sub2/hello.world 
+   test_path_is_missing to_clean
+'
+
+test_expect_failure 'nested (empty) git should be kept' '
+   rm -fr empty_repo to_clean 
+   git init empty_repo 
+   mkdir to_clean 
+   to_clean/should_clean.this 
+   git clean -f -d 
+   test_path_is_file empty_repo/.git/HEAD 
+   test_path_is_missing to_clean
+'
+
+test_expect_success 'nested bare repositories should be cleaned' '
+   rm -fr bare1 bare2 subdir 
+   git init --bare bare1 
+   git clone --local --bare . bare2 
+   mkdir subdir 
+   cp -r bare2 subdir/bare3 
+   git clean -f -d 
+   test_path_is_missing bare1 
+   test_path_is_missing bare2 
+   test_path_is_missing subdir
+'
+
+test_expect_success 'nested (empty) bare repositories should be cleaned even 
when in .git' '
+   rm -fr strange_bare 
+   mkdir strange_bare 
+   git init --bare strange_bare/.git 
+   git clean -f -d 
+   test_path_is_missing strange_bare
+'
+
+test_expect_failure 'nested (non-empty) bare repositories should be cleaned 
even when in .git' '
+   rm -fr strange_bare 
+   mkdir strange_bare 
+   git clone --local --bare . strange_bare/.git 
+   git clean -f -d 
+   test_path_is_missing strange_bare
+'
+
+test_expect_success 'giving path in nested git work tree will remove it' '
+   rm -fr repo 
+   mkdir repo 
+   (
+   cd repo 
+   git init 
+   mkdir -p bar/baz 
+   test_commit msg bar/baz/hello.world
+   ) 
+   git clean -f -d repo/bar/baz 
+   test_path_is_file repo/.git/HEAD 
+   test_path_is_dir repo/bar/ 
+   test_path_is_missing repo/bar/baz
+'
+
+test_expect_success 'giving path to nested .git will not remove it' '
+   rm -fr repo 
+   mkdir repo untracked 
+   (
+   cd repo 
+   git init 
+   test_commit msg hello.world
+   ) 
+   git clean -f -d repo/.git 
+   test_path_is_file repo/.git/HEAD 
+   test_path_is_dir repo/.git/refs 
+   test_path_is_dir repo/.git/objects 
+   test_path_is_dir untracked/
+'
+
+test_expect_success 'giving path to nested .git/ will remove contents' '
+   rm -fr repo untracked 
+   mkdir repo untracked 
+   (
+   cd repo 
+   git init 
+   test_commit msg hello.world
+   ) 
+   git clean -f -d repo/.git/ 
+   test_path_is_dir repo/.git 
+   test_dir_is_empty repo/.git 
+   test_path_is_dir untracked/
+'
+
 test_expect_success 'force removal of nested git work tree' '
rm -fr foo bar baz 
mkdir -p foo bar baz/boo 
-- 
2.4.0.rc3.8.gbb31afb

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v5 1/5] setup: add gentle version of read_gitfile

2015-04-26 Thread Erik Elfström
read_gitfile will die on most error cases. This makes it unsuitable
for speculative calls. Extract the core logic and provide a gentle
version that returns NULL on failure.

The first usecase of the new gentle version will be to probe for
submodules during git clean.

Helped-by: Junio C Hamano gits...@pobox.com
Helped-by: Jeff King p...@peff.net
Signed-off-by: Erik Elfström erik.elfst...@gmail.com
---
 cache.h | 11 -
 setup.c | 82 +++--
 2 files changed, 75 insertions(+), 18 deletions(-)

diff --git a/cache.h b/cache.h
index 3d3244b..868e4d3 100644
--- a/cache.h
+++ b/cache.h
@@ -431,7 +431,16 @@ extern int set_git_dir(const char *path);
 extern const char *get_git_namespace(void);
 extern const char *strip_namespace(const char *namespaced_ref);
 extern const char *get_git_work_tree(void);
-extern const char *read_gitfile(const char *path);
+
+#define READ_GITFILE_ERR_STAT_FAILED 1
+#define READ_GITFILE_ERR_NOT_A_FILE 2
+#define READ_GITFILE_ERR_OPEN_FAILED 3
+#define READ_GITFILE_ERR_READ_FAILED 4
+#define READ_GITFILE_ERR_INVALID_FORMAT 5
+#define READ_GITFILE_ERR_NO_PATH 6
+#define READ_GITFILE_ERR_NOT_A_REPO 7
+extern const char *read_gitfile_gently(const char *path, int 
*return_error_code);
+#define read_gitfile(path) read_gitfile_gently((path), NULL)
 extern const char *resolve_gitdir(const char *suspect);
 extern void set_git_work_tree(const char *tree);
 
diff --git a/setup.c b/setup.c
index 979b13f..c4538ca 100644
--- a/setup.c
+++ b/setup.c
@@ -335,35 +335,53 @@ static int check_repository_format_gently(const char 
*gitdir, int *nongit_ok)
 /*
  * Try to read the location of the git directory from the .git file,
  * return path to git directory if found.
+ *
+ * On failure, if return_error_code is not NULL, return_error_code
+ * will be set to an error code and NULL will be returned. If
+ * return_error_code is NULL the function will die instead (for most
+ * cases).
  */
-const char *read_gitfile(const char *path)
+const char *read_gitfile_gently(const char *path, int *return_error_code)
 {
-   char *buf;
-   char *dir;
+   int error_code = 0;
+   char *buf = NULL;
+   char *dir = NULL;
const char *slash;
struct stat st;
int fd;
ssize_t len;
 
-   if (stat(path, st))
-   return NULL;
-   if (!S_ISREG(st.st_mode))
-   return NULL;
+   if (stat(path, st)) {
+   error_code = READ_GITFILE_ERR_STAT_FAILED;
+   goto cleanup_return;
+   }
+   if (!S_ISREG(st.st_mode)) {
+   error_code = READ_GITFILE_ERR_NOT_A_FILE;
+   goto cleanup_return;
+   }
fd = open(path, O_RDONLY);
-   if (fd  0)
-   die_errno(Error opening '%s', path);
+   if (fd  0) {
+   error_code = READ_GITFILE_ERR_OPEN_FAILED;
+   goto cleanup_return;
+   }
buf = xmalloc(st.st_size + 1);
len = read_in_full(fd, buf, st.st_size);
close(fd);
-   if (len != st.st_size)
-   die(Error reading %s, path);
+   if (len != st.st_size) {
+   error_code = READ_GITFILE_ERR_READ_FAILED;
+   goto cleanup_return;
+   }
buf[len] = '\0';
-   if (!starts_with(buf, gitdir: ))
-   die(Invalid gitfile format: %s, path);
+   if (!starts_with(buf, gitdir: )) {
+   error_code = READ_GITFILE_ERR_INVALID_FORMAT;
+   goto cleanup_return;
+   }
while (buf[len - 1] == '\n' || buf[len - 1] == '\r')
len--;
-   if (len  9)
-   die(No path in gitfile: %s, path);
+   if (len  9) {
+   error_code = READ_GITFILE_ERR_NO_PATH;
+   goto cleanup_return;
+   }
buf[len] = '\0';
dir = buf + 8;
 
@@ -378,11 +396,41 @@ const char *read_gitfile(const char *path)
buf = dir;
}
 
-   if (!is_git_directory(dir))
-   die(Not a git repository: %s, dir);
+   if (!is_git_directory(dir)) {
+   error_code = READ_GITFILE_ERR_NOT_A_REPO;
+   goto cleanup_return;
+   }
path = real_path(dir);
 
+cleanup_return:
free(buf);
+
+   if (return_error_code)
+   *return_error_code = error_code;
+
+   if (error_code) {
+   if (return_error_code)
+   return NULL;
+
+   switch (error_code) {
+   case READ_GITFILE_ERR_STAT_FAILED:
+   case READ_GITFILE_ERR_NOT_A_FILE:
+   return NULL;
+   case READ_GITFILE_ERR_OPEN_FAILED:
+   die_errno(Error opening '%s', path);
+   case READ_GITFILE_ERR_READ_FAILED:
+   die(Error reading %s, path);
+   case READ_GITFILE_ERR_INVALID_FORMAT:
+   die(Invalid gitfile format: %s, path);
+   case 

[PATCH v5 5/5] clean: improve performance when removing lots of directories

2015-04-26 Thread Erik Elfström
git clean uses resolve_gitlink_ref() to check for the presence of
nested git repositories, but it has the drawback of creating a
ref_cache entry for every directory that should potentially be
cleaned. The linear search through the ref_cache list causes a massive
performance hit for large number of directories.

Modify clean.c:remove_dirs to use setup.c:is_git_directory and
setup.c:read_gitfile_gently instead.

Both these functions will open files and parse contents when they find
something that looks like a git repository. This is ok from a
performance standpoint since finding repository candidates should be
comparatively rare.

Using is_git_directory and read_gitfile_gently should give a more
standardized check for what is and what isn't a git repository but
also gives three behavioral changes.

The first change is that we will now detect and avoid cleaning empty
nested git repositories (only init run). This is desirable.

Second, we will no longer die when cleaning a file named .git with
garbage content (it will be cleaned instead). This is also desirable.

The last change is that we will detect and avoid cleaning empty bare
repositories that have been placed in a directory named .git. This
is not desirable but should have no real user impact since we already
fail to clean non-empty bare repositories in the same scenario. This
is thus deemed acceptable.

Update t7300 to reflect these changes in behavior.

The time to clean an untracked directory containing 10 sub
directories went from 61s to 1.7s after this change.

Helped-by: Jeff King p...@peff.net
Signed-off-by: Erik Elfström erik.elfst...@gmail.com
---
 builtin/clean.c  | 26 ++
 t/t7300-clean.sh |  8 +++-
 2 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/builtin/clean.c b/builtin/clean.c
index 98c103f..17a1c13 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -10,7 +10,6 @@
 #include cache.h
 #include dir.h
 #include parse-options.h
-#include refs.h
 #include string-list.h
 #include quote.h
 #include column.h
@@ -148,6 +147,27 @@ static int exclude_cb(const struct option *opt, const char 
*arg, int unset)
return 0;
 }
 
+/*
+ * Return 1 if the given path is the root of a git repository or
+ * submodule else 0. Will not return 1 for bare repositories with the
+ * exception of creating a bare repository in foo/.git and calling
+ * is_git_repository(foo).
+ */
+static int is_git_repository(struct strbuf *path)
+{
+   int ret = 0;
+   int unused_error_code;
+   size_t orig_path_len = path-len;
+   assert(orig_path_len != 0);
+   if (path-buf[orig_path_len - 1] != '/')
+   strbuf_addch(path, '/');
+   strbuf_addstr(path, .git);
+   if (read_gitfile_gently(path-buf, unused_error_code) || 
is_git_directory(path-buf))
+   ret = 1;
+   strbuf_setlen(path, orig_path_len);
+   return ret;
+}
+
 static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,
int dry_run, int quiet, int *dir_gone)
 {
@@ -155,13 +175,11 @@ static int remove_dirs(struct strbuf *path, const char 
*prefix, int force_flag,
struct strbuf quoted = STRBUF_INIT;
struct dirent *e;
int res = 0, ret = 0, gone = 1, original_len = path-len, len;
-   unsigned char submodule_head[20];
struct string_list dels = STRING_LIST_INIT_DUP;
 
*dir_gone = 1;
 
-   if ((force_flag  REMOVE_DIR_KEEP_NESTED_GIT) 
-   !resolve_gitlink_ref(path-buf, HEAD, 
submodule_head)) {
+   if ((force_flag  REMOVE_DIR_KEEP_NESTED_GIT)  
is_git_repository(path)) {
if (!quiet) {
quote_path_relative(path-buf, prefix, quoted);
printf(dry_run ?  _(msg_would_skip_git_dir) : 
_(msg_skip_git_dir),
diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh
index 11f3a6d..4d07a4a 100755
--- a/t/t7300-clean.sh
+++ b/t/t7300-clean.sh
@@ -455,7 +455,7 @@ test_expect_success 'nested git work tree' '
! test -d bar
 '
 
-test_expect_failure 'should clean things that almost look like git but are 
not' '
+test_expect_success 'should clean things that almost look like git but are 
not' '
rm -fr almost_git almost_bare_git almost_submodule 
mkdir -p almost_git/.git/objects 
mkdir -p almost_git/.git/refs 
@@ -468,8 +468,6 @@ test_expect_failure 'should clean things that almost look 
like git but are not'
garbage
EOF
test_when_finished rm -rf almost_* 
-   ## This will fail due to die(Invalid gitfile format: %s, path); in
-   ## setup.c:read_gitfile.
git clean -f -d 
test_path_is_missing almost_git 
test_path_is_missing almost_bare_git 
@@ -501,7 +499,7 @@ test_expect_success 'should not clean submodules' '
test_path_is_missing to_clean
 '
 
-test_expect_failure 'nested (empty) git should be kept' '
+test_expect_success 'nested (empty) git should be kept' '
rm -fr 

[PATCH v5 0/5] Improving performance of git clean

2015-04-26 Thread Erik Elfström
Changes in v5:
* Added defines for read_gitfile_gently error codes. 
  This was a silly mistake, sorry about that.

Erik Elfström (5):
  setup: add gentle version of read_gitfile
  setup: sanity check file size in read_gitfile_gently
  t7300: add tests to document behavior of clean and nested git
  p7300: add performance tests for clean
  clean: improve performance when removing lots of directories

 builtin/clean.c   |  26 +--
 cache.h   |  12 -
 setup.c   |  88 ---
 t/perf/p7300-clean.sh |  31 +
 t/t7300-clean.sh  | 126 ++
 5 files changed, 261 insertions(+), 22 deletions(-)
 create mode 100755 t/perf/p7300-clean.sh

-- 
2.4.0.rc3.8.gbb31afb

--
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] blame: CRLF in the working tree and LF in the repo

2015-04-26 Thread Torsten Bögershausen
A typicall setup under Windows:
core.eol is CRLF and a file is marked as text in .gitattributes.

After 4d4813a5 git blame no longer works as expected,
every line is annotated as Not Committed Yet,
even though the working directory is clean.

commit 4d4813a5 removed the conversion in blame.c for all files,
with or without CRLF in the repo.

Having files with CRLF in the repo and core.autocrlf=input is a temporary
situation, the files should be normalized in the repo.
Blaming them with Not Committed Yet is OK.

The solution is to revert commit 4d4813a5.

Reported-By: Stepan Kasal ka...@ucw.cz
Signed-off-by: Torsten Bögershausen tbo...@web.de
---
Reference:
https://github.com/git-for-windows/git/issues/105
Although the intention of 4d4813a5 is good, it breaks
the usual EOL-handling for Windows.
Until we have a better solution, we suggest to revert it.

 builtin/blame.c   |  1 +
 t/t8003-blame-corner-cases.sh | 26 +-
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 06484c2..8d70623 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2348,6 +2348,7 @@ static struct commit *fake_working_tree_commit(struct 
diff_options *opt,
if (strbuf_read(buf, 0, 0)  0)
die_errno(failed to read from stdin);
}
+   convert_to_git(path, buf.buf, buf.len, buf, 0);
origin-file.ptr = buf.buf;
origin-file.size = buf.len;
pretend_sha1_file(buf.buf, buf.len, OBJ_BLOB, origin-blob_sha1);
diff --git a/t/t8003-blame-corner-cases.sh b/t/t8003-blame-corner-cases.sh
index 32895e5..dcc9827 100755
--- a/t/t8003-blame-corner-cases.sh
+++ b/t/t8003-blame-corner-cases.sh
@@ -191,7 +191,7 @@ test_expect_success 'indent of line numbers, ten lines' '
test $(grep -cactual) = 9
 '
 
-test_expect_success 'blaming files with CRLF newlines' '
+test_expect_failure 'blaming files with CRLF newlines in repo, 
core.autoclrf=input' '
git config core.autocrlf false 
printf testcase\r\n crlffile 
git add crlffile 
@@ -199,5 +199,29 @@ test_expect_success 'blaming files with CRLF newlines' '
git -c core.autocrlf=input blame crlffile actual 
grep A U Thor actual
 '
+test_expect_success 'blaming files with CRLF newlines core.autocrlf=true' '
+   test_create_repo blamerepo 
+   (
+   cd blamerepo 
+   git config core.autocrlf true 
+   printf testcase\r\n crlffile 
+   git add crlffile 
+   git commit -m TRUE 
+   git blame crlffile actual 
+   grep A U Thor actual
+   )
+'
+
+test_expect_success 'blaming files with CRLF newlines core.autocrlf=false' '
+   (
+   cd blamerepo 
+   git config core.autocrlf false 
+   printf .gitattributes text\r\n .gitattributes 
+   git add .gitattributes 
+   git commit -m FALSE 
+   git blame .gitattributes actual 
+   grep A U Thor actual
+   )
+'
 
 test_done
-- 
2.2.0.rc1.790.ge19fcd2

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: git-p4 Question

2015-04-26 Thread Luke Diamand

On 24/04/15 15:36, FusionX86 wrote:

I get an error if I misspell part of the path. For example, if I type
//depot/maain instead of //depot/main I will get the no such files
message you indicated. BUT using incorrect case like //depot/main
instead of //depot/Main doesn't return any error, but still completes
and creates an empty repo. If it does require correct case, then it
should throw an error for //depot/main as well.


Thanks, that's a somewhat subtle bug!

Luke

--
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 0/3] Improve robustness of putty detection

2015-04-26 Thread Junio C Hamano
brian m. carlson sand...@crustytoothpaste.net writes:

 While I was adding tests, I noticed that we had a broken test due to the
 use of single quotes within a test, which resulted in the test always
 being skipped.

Good eyes.  While fixing the test is necessary, we should also be
able to improve the test framework to prevent such mistakes at the
same time.

ok 38 # skip
git clone [myhost:123]:src ssh-bracket-clone 
expect_ssh myhost -p (missing bracketed hostnames are still
ssh)

The test scripts are expected to take either 3 or 4 parameters, and
the extra parameter when it takes 4 is the comma separated list of
prerequisites.  bracketed hostnames are still ssh does not look
like prerequisites at all to us humans, and the framework should
also be able to notice that and barf, I would think.

Perhaps something like this?

 t/test-lib-functions.sh | 9 +
 1 file changed, 9 insertions(+)

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 0698ce7..0e4f2a6 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -348,11 +348,18 @@ test_declared_prereq () {
return 1
 }
 
+test_verify_prereq () {
+   test -z $test_prereq ||
+   expr /dev/null $test_prereq : '^[A-Z0-9_,!]*$' ||
+   error bug in the test script: '$test_prereq' does not look like a 
prereq
+}
+
 test_expect_failure () {
test_start_
test $# = 3  { test_prereq=$1; shift; } || test_prereq=
test $# = 2 ||
error bug in the test script: not 2 or 3 parameters to 
test-expect-failure
+   test_verify_prereq
export test_prereq
if ! test_skip $@
then
@@ -372,6 +379,7 @@ test_expect_success () {
test $# = 3  { test_prereq=$1; shift; } || test_prereq=
test $# = 2 ||
error bug in the test script: not 2 or 3 parameters to 
test-expect-success
+   test_verify_prereq
export test_prereq
if ! test_skip $@
then
@@ -400,6 +408,7 @@ test_external () {
error 5 bug in the test script: not 3 or 4 parameters to 
test_external
descr=$1
shift
+   test_verify_prereq
export test_prereq
if ! test_skip $descr $@
then
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Question about how git determines the minimum packfile for a push.

2015-04-26 Thread Brad Litterell
Hi,

I'm using git with a submodule containing a (large) binary toolchain where I 
updated the version from GCC-4.7 to 4.8.  When I added 4.8 I deleted 4.7 and 
now want to add 4.7 back to the HEAD.  As shown in the tree objects below, the 
4.7 bits are still in the repository (as expected), but when I try to push a 
commit and tree object that puts them back, git tries to send all 4000+ objects 
again, even though the objects are already be on the server from a previous 
commit. (I have confirmed this with git cat-file on the server.) From my 
research it seems like git is supposed to identify the common objects and not 
do this.

For example, consider the last three commits (most recent on top).

$ git log
bb384915e12e925ead5ab8ad5161c84e0ef2b7f7 (HEAD, master) Add GCC-4.7 back to the 
image for side-by-side testing. Will delete it later.
2dfd226e6d2cc0a1dc58770d1dcaec1ba863df72 (origin/master) Upgrade toolchain to 
GCC-4.8. (delete GCC-4.7)
816fde0fdec1506600f19de4e3e4e02a6fe08639 (tag: release-1) Compiler toolchain 4.7

Here are the internal tree objects for those 3 commits:

# git cat-file -p 816fde^{tree}
$ cat old.tree
100644 blob 841b8359fce4edaf7549b87e1a81e7091c1dff6c.arcconfig
100644 blob 21cb788d9f6e99367d96ba19d8c7470164e7d298.gitmodules
04 tree 470dfed1791b3ab7b4c731e354fd7685609c057aarcanist
100644 blob 588866ae2c8a815ff26c5930b98d2a8ac9b934a0
gcc-linaro-arm-linux-gnueabihf-4.7-2013.03-20130313_linux.tar.bz2
04 tree 7a1ae67dd2ff1fd358ad52d25852b340355339fe
gcc-linaro-arm-linux-gnueabihf-4.7-2013.03-20130313_linux

# git cat-file -p 2dfd22^{tree}
$ cat middle.tree
100644 blob 841b8359fce4edaf7549b87e1a81e7091c1dff6c.arcconfig
100644 blob 21cb788d9f6e99367d96ba19d8c7470164e7d298.gitmodules
100644 blob 79c4d3271c1177822786f82201c80b928fc35c6eREADME
04 tree 470dfed1791b3ab7b4c731e354fd7685609c057aarcanist
100644 blob f71d6dd4e13a3de4b0c38dd37e4c2bc94c503f26
gcc-linaro-arm-linux-gnueabihf-4.8-2013.12_linux.tar.bz2
100644 blob 026e2d232bd7bb1cf0b9efb61c8cd307a52526ec
gcc-linaro-arm-linux-gnueabihf-4.8-2013.12_linux.tar.bz2.asc
04 tree d03b4cf20163ca7a0ab5c02365890855146d8e0c
gcc-linaro-arm-linux-gnueabihf-4.8-2013.12_linux

# git cat-file -p HEAD^{tree}
$ cat new.tree
100644 blob 841b8359fce4edaf7549b87e1a81e7091c1dff6c.arcconfig
100644 blob 21cb788d9f6e99367d96ba19d8c7470164e7d298.gitmodules
100644 blob 79c4d3271c1177822786f82201c80b928fc35c6eREADME
04 tree 470dfed1791b3ab7b4c731e354fd7685609c057aarcanist
100644 blob 588866ae2c8a815ff26c5930b98d2a8ac9b934a0
gcc-linaro-arm-linux-gnueabihf-4.7-2013.03-20130313_linux.tar.bz2
04 tree 7a1ae67dd2ff1fd358ad52d25852b340355339fe
gcc-linaro-arm-linux-gnueabihf-4.7-2013.03-20130313_linux
100644 blob f71d6dd4e13a3de4b0c38dd37e4c2bc94c503f26
gcc-linaro-arm-linux-gnueabihf-4.8-2013.12_linux.tar.bz2
100644 blob 026e2d232bd7bb1cf0b9efb61c8cd307a52526ec
gcc-linaro-arm-linux-gnueabihf-4.8-2013.12_linux.tar.bz2.asc
04 tree d03b4cf20163ca7a0ab5c02365890855146d8e0c
gcc-linaro-arm-linux-gnueabihf-4.8-2013.12_linux

By examining the tree objects with git cat-file -p it is clear that adding back 
GCC-4.7 added back the same exact blobs, which was expected, namely:

100644 blob 588866ae2c8a815ff26c5930b98d2a8ac9b934a0
gcc-linaro-arm-linux-gnueabihf-4.7-2013.03-20130313_linux.tar.bz2
04 tree 7a1ae67dd2ff1fd358ad52d25852b340355339fe
gcc-linaro-arm-linux-gnueabihf-4.7-2013.03-20130313_linux

But, whenever I try to push, git tries to write 4000+ objects across the wire:

$ git push origin master
Counting objects: 4854, done.
Delta compression using up to 6 threads.
Compressing objects: 100% (2295/2295), done.
Writing objects:   0% (2/4853), 9.26 MiB | 3.08 MiB/s
^C   

All the parts are already on the server so it seems like the only objects to 
upload would be the commit object and the new associated tree (not the blob 
(5888) and tree (7a1a) below it).

Further, it seems like my local git client should be able to discern this from 
its knowledge of the origin/master ref.

I don't want to let this push complete if it will result in duplication on the 
server because this repo is already 400MB and slow to clone.

Can someone please explain what is happening here? using git push -thin doesn't 
seem to make a difference.

Is it possible git is not computing the delta correctly?  Or does git only look 
at the top-level commit objects to figure out what to include in the push 
packfile?

Will it upload the larger pack only to have the server correctly handle the 
duplicates?

In case it matters, the server in question is running Atlassian Stash.

Thanks,
Brad

--
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/16] Convert parts of refs.c to struct object_id

2015-04-26 Thread Michael Haggerty
On 04/23/2015 01:24 AM, brian m. carlson wrote:
 This is a conversion of parts of refs.c to use struct object_id.
 
 refs.c, and the for_each_ref series of functions explicitly, is the
 source for many instances of object IDs in the codebase.  Therefore, it
 makes sense to convert this series of functions to provide a basis for
 further conversions.
 
 This series is essentially just for_each_ref and friends, the callbacks,
 and callers.  Other parts of refs.c will be converted in a later series,
 so as to keep the number of patches to a reasonable size.
 
 There should be no functional change from this patch series.

I wanted to review your patches, but wasn't really sure how to go about
it in a way that would make me confident in the result. In a way these
refactoring patch series are easier to implement than to review.

...so that's what I did. I reimplemented your changes from scratch [1]
and then checked how my result differed from yours. My conclusion is
that the final result of your patches looks good, though there are some
other changes in the neighborhood that could sensibly be added.


However, I reached the destination via a different route and I thought
I'd describe it in case you are interested, and as the technique might
be useful for future object_id refactorings. My patch series is
available on my GitHub account at

https://github.com/mhagger/git branch oid-refs-adapter

My starting point was to change each_ref_fn to take a const object_id
* parameter instead of const unsigned char *. This change requires
all call sites of all of the for_each_ref functions to be modified,
because they currently pass callback functions that match the old signature.

So I kept the old typedef (the one that takes const unsigned char *)
but renamed it to each_ref_sha1_fn. And I added an adapter that allows
such functions to be wrapped then passed to the new for_each_ref
functions. It looks like this:

typedef int each_ref_sha1_fn(const char *refname,
 const unsigned char *sha1, int flags, void 
*cb_data);

struct each_ref_fn_sha1_adapter {
each_ref_sha1_fn *original_fn;
void *original_cb_data;
};

extern int each_ref_fn_adapter(const char *refname,
   const struct object_id *oid, int flags, void 
*cb_data);

Each callsite has to be changed, but the changes are quite
straightforward. At a callsite that would have called

for_each_ref(my_function, my_data)

you wrap my_function and my_data in an each_ref_fn_sha1_adapter and call
for_each_ref using each_ref_fn_adapter as the callback:

struct each_ref_fn_sha1_adapter wrapped_my_function =
{my_function, my_data};

for_each_ref(each_ref_fn_adapter, wrapped_my_function);

The function each_ref_fn_adapter extracts the SHA-1 out of the oid and
calls my_function, passing it my_data as extra data.

This patch is thus giant but very straightforward.

After that, there is one patch for each callsite, rewriting it to use
for_each_ref natively (which usually entails modifying my_function to
take an object_id parameter then undoing the wrapper). These patches
involve a little bit of thought, but not too much. And the results are
very bisectable because each patch makes a single small change. I also
suspect it might be easier to rebase and/or merge my patch series, for
the same reason.

The end result was very similar to yours, so I am confident that the net
result of your patch series is correct. But the remaining differences in
the end results are also interesting. I made a few more changes in the
neighborhood of the patches, not to mention a few formatting
improvements in code that I touched. If you compare the tip of my
branch, above, to the tip of yours (I uploaded that to my repo too, as
branch bc-oid-refs), it may give you some ideas for other code that
can be changed to object_id.

Yours,
Michael

[1] Obviously I glanced at your patches while I was working to make sure
that I was headed in the same direction as you, and to minimize
gratuitous differences between our versions.

-- 
Michael Haggerty
mhag...@alum.mit.edu

--
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/RFC] blame: CRLF in the working tree and LF in the repo

2015-04-26 Thread Eric Sunshine
On Sun, Apr 26, 2015 at 8:02 AM, Torsten Bögershausen tbo...@web.de wrote:
 A typicall setup under Windows:

s/typicall/typical/

 core.eol is CRLF and a file is marked as text in .gitattributes.

 After 4d4813a5 git blame no longer works as expected,
 every line is annotated as Not Committed Yet,
 even though the working directory is clean.

 commit 4d4813a5 removed the conversion in blame.c for all files,
 with or without CRLF in the repo.

 Having files with CRLF in the repo and core.autocrlf=input is a temporary
 situation, the files should be normalized in the repo.
 Blaming them with Not Committed Yet is OK.

 The solution is to revert commit 4d4813a5.

 Reported-By: Stepan Kasal ka...@ucw.cz
 Signed-off-by: Torsten Bögershausen tbo...@web.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 v3 3/3] connect: improve check for plink to reduce false positives

2015-04-26 Thread brian m. carlson
The git_connect function has code to handle plink and tortoiseplink
specially, as they require different command line arguments from
OpenSSH (-P instead of -p for ports; tortoiseplink additionally requires
-batch).  However, the match was done by checking for plink anywhere
in the string, which led to a GIT_SSH value containing uplink being
treated as an invocation of putty's plink.

Improve the check by looking for plink or tortoiseplink (or those
names suffixed with .exe) only in the final component of the path.
This has the downside that a program such as plink-0.63 would no
longer be recognized, but the increased robustness is likely worth it.
Add tests to cover these cases to avoid regressions.

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 connect.c| 18 +++---
 t/t5601-clone.sh | 33 +
 2 files changed, 48 insertions(+), 3 deletions(-)

diff --git a/connect.c b/connect.c
index 749a07b..c0144d8 100644
--- a/connect.c
+++ b/connect.c
@@ -724,7 +724,7 @@ struct child_process *git_connect(int fd[2], const char 
*url,
conn-in = conn-out = -1;
if (protocol == PROTO_SSH) {
const char *ssh;
-   int putty;
+   int putty, tortoiseplink = 0;
char *ssh_host = hostandport;
const char *port = NULL;
get_host_and_port(ssh_host, port);
@@ -750,14 +750,26 @@ struct child_process *git_connect(int fd[2], const char 
*url,
conn-use_shell = 1;
putty = 0;
} else {
+   const char *base;
+   char *ssh_dup;
+
ssh = getenv(GIT_SSH);
if (!ssh)
ssh = ssh;
-   putty = !!strcasestr(ssh, plink);
+
+   ssh_dup = xstrdup(ssh);
+   base = basename(ssh_dup);
+
+   tortoiseplink = !strcasecmp(base, 
tortoiseplink) ||
+   !strcasecmp(base, tortoiseplink.exe);
+   putty = !strcasecmp(base, plink) ||
+   !strcasecmp(base, plink.exe) || 
tortoiseplink;
+
+   free(ssh_dup);
}
 
argv_array_push(conn-args, ssh);
-   if (putty  !strcasestr(ssh, tortoiseplink))
+   if (tortoiseplink)
argv_array_push(conn-args, -batch);
if (port) {
/* P is for PuTTY, p is for OpenSSH */
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index aae2324..bfdaf75 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -296,6 +296,12 @@ setup_ssh_wrapper () {
'
 }
 
+copy_ssh_wrapper_as () {
+   cp $TRASH_DIRECTORY/ssh-wrapper $1 
+   GIT_SSH=$1 
+   export GIT_SSH
+}
+
 expect_ssh () {
test_when_finished '
(cd $TRASH_DIRECTORY  rm -f ssh-expect  ssh-output)
@@ -335,6 +341,33 @@ test_expect_success 'bracketed hostnames are still ssh' '
expect_ssh -p 123 myhost src
 '
 
+test_expect_success 'uplink is not treated as putty' '
+   copy_ssh_wrapper_as $TRASH_DIRECTORY/uplink 
+   git clone [myhost:123]:src ssh-bracket-clone-uplink 
+   expect_ssh -p 123 myhost src
+'
+
+test_expect_success 'plink is treated specially (as putty)' '
+   copy_ssh_wrapper_as $TRASH_DIRECTORY/plink 
+   git clone [myhost:123]:src ssh-bracket-clone-plink-0 
+   expect_ssh -P 123 myhost src
+'
+
+test_expect_success 'plink.exe is treated specially (as putty)' '
+   copy_ssh_wrapper_as $TRASH_DIRECTORY/plink.exe 
+   git clone [myhost:123]:src ssh-bracket-clone-plink-1 
+   expect_ssh -P 123 myhost src
+'
+
+test_expect_success 'tortoiseplink is like putty, with extra arguments' '
+   copy_ssh_wrapper_as $TRASH_DIRECTORY/tortoiseplink 
+   git clone [myhost:123]:src ssh-bracket-clone-plink-2 
+   expect_ssh -batch -P 123 myhost src
+'
+
+# Reset the GIT_SSH environment variable for clone tests.
+setup_ssh_wrapper
+
 counter=0
 # $1 url
 # $2 none|host
-- 
2.3.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 v3 0/3] Improve robustness of putty detection

2015-04-26 Thread brian m. carlson
This series improves detection of plink (the putty utility).

While I was adding tests, I noticed that we had a broken test due to the
use of single quotes within a test, which resulted in the test always
being skipped.  Therefore, nobody noticed that the test was actually
broken.

brian m. carlson (3):
  connect: simplify SSH connection code path
  t5601: fix quotation error leading to skipped tests
  connect: improve check for plink to reduce false positives

 connect.c| 56 ++--
 t/t5601-clone.sh | 35 ++-
 2 files changed, 68 insertions(+), 23 deletions(-)

-- 
2.3.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 v3 2/3] t5601: fix quotation error leading to skipped tests

2015-04-26 Thread brian m. carlson
One of the tests in t5601 used single quotes to delimit an argument
containing spaces.  However, this caused test_expect_success to be
passed three arguments instead of two, which in turn caused the test
name to be treated as a prerequisite instead of a test name.  As there
was no prerequisite called bracketed hostnames are still ssh, the test
was always skipped.

Because this test was always skipped, the fact that it passed the
arguments in the wrong order was obscured.  Use double quotes inside the
test and reorder the arguments so that the test runs and properly
reflects the arguments that are passed to ssh.

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 t/t5601-clone.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 1befc45..aae2324 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -332,7 +332,7 @@ test_expect_success !MINGW,!CYGWIN 'clone local path 
foo:bar' '
 
 test_expect_success 'bracketed hostnames are still ssh' '
git clone [myhost:123]:src ssh-bracket-clone 
-   expect_ssh myhost '-p 123' src
+   expect_ssh -p 123 myhost src
 '
 
 counter=0
-- 
2.3.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 v3 1/3] connect: simplify SSH connection code path

2015-04-26 Thread brian m. carlson
The code path used in git_connect pushed the majority of the SSH
connection code into an else block, even though the if block returns.
Simplify the code by eliminating the else block, as it is unneeded.

Signed-off-by: Jeff King p...@peff.net
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 connect.c | 42 +-
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/connect.c b/connect.c
index 391d211..749a07b 100644
--- a/connect.c
+++ b/connect.c
@@ -743,28 +743,28 @@ struct child_process *git_connect(int fd[2], const char 
*url,
free(path);
free(conn);
return NULL;
-   } else {
-   ssh = getenv(GIT_SSH_COMMAND);
-   if (ssh) {
-   conn-use_shell = 1;
-   putty = 0;
-   } else {
-   ssh = getenv(GIT_SSH);
-   if (!ssh)
-   ssh = ssh;
-   putty = !!strcasestr(ssh, plink);
-   }
-
-   argv_array_push(conn-args, ssh);
-   if (putty  !strcasestr(ssh, tortoiseplink))
-   argv_array_push(conn-args, -batch);
-   if (port) {
-   /* P is for PuTTY, p is for OpenSSH */
-   argv_array_push(conn-args, putty ? 
-P : -p);
-   argv_array_push(conn-args, port);
-   }
-   argv_array_push(conn-args, ssh_host);
}
+
+   ssh = getenv(GIT_SSH_COMMAND);
+   if (ssh) {
+   conn-use_shell = 1;
+   putty = 0;
+   } else {
+   ssh = getenv(GIT_SSH);
+   if (!ssh)
+   ssh = ssh;
+   putty = !!strcasestr(ssh, plink);
+   }
+
+   argv_array_push(conn-args, ssh);
+   if (putty  !strcasestr(ssh, tortoiseplink))
+   argv_array_push(conn-args, -batch);
+   if (port) {
+   /* P is for PuTTY, p is for OpenSSH */
+   argv_array_push(conn-args, putty ? -P : 
-p);
+   argv_array_push(conn-args, port);
+   }
+   argv_array_push(conn-args, ssh_host);
} else {
/* remove repo-local variables from the environment */
conn-env = local_repo_env;
-- 
2.3.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 RFC] add: Do not open editor if patch is empty

2015-04-26 Thread Alexander Kuleshov
If we'll run 'git add -e path' on a path which has no
difference with the current index, empty editor will open. This
patch prevents this behaviour and checks that patch is not empty
before an editor with patch will be opened.

Signed-off-by: Alexander Kuleshov kuleshovm...@gmail.com
---
 builtin/add.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/add.c b/builtin/add.c
index ee370b0..4fc6b13 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -209,13 +209,13 @@ static int edit_patch(int argc, const char **argv, const 
char *prefix)
if (run_diff_files(rev, 0))
die(_(Could not write patch));
 
-   launch_editor(file, NULL, NULL);
-
if (stat(file, st))
die_errno(_(Could not stat '%s'), file);
if (!st.st_size)
die(_(Empty patch. Aborted.));
 
+   launch_editor(file, NULL, NULL);
+
child.git_cmd = 1;
child.argv = apply_argv;
if (run_command(child))
-- 
2.3.3.611.g09038fc.dirty

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH RFC] add: Do not open editor if patch is empty

2015-04-26 Thread Eric Sunshine
On Sun, Apr 26, 2015 at 1:03 PM, Alexander Kuleshov
kuleshovm...@gmail.com wrote:
 If we'll run 'git add -e path' on a path which has no
 difference with the current index, empty editor will open. This
 patch prevents this behaviour and checks that patch is not empty
 before an editor with patch will be opened.

 Signed-off-by: Alexander Kuleshov kuleshovm...@gmail.com
 ---
 diff --git a/builtin/add.c b/builtin/add.c
 index ee370b0..4fc6b13 100644
 --- a/builtin/add.c
 +++ b/builtin/add.c
 @@ -209,13 +209,13 @@ static int edit_patch(int argc, const char **argv, 
 const char *prefix)
 if (run_diff_files(rev, 0))
 die(_(Could not write patch));

 -   launch_editor(file, NULL, NULL);
 -
 if (stat(file, st))
 die_errno(_(Could not stat '%s'), file);
 if (!st.st_size)
 die(_(Empty patch. Aborted.));

 +   launch_editor(file, NULL, NULL);

But this breaks intended behavior[1] in which git add -e aborts with
message Empty patch if the user deletes everything from the patch
file in the editor.

[1]: c59cb03 (git-add: introduce --edit (to edit the diff vs. the
index), 2009-04-08)

 child.git_cmd = 1;
 child.argv = apply_argv;
 if (run_command(child))
 --
 2.3.3.611.g09038fc.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 2/2] connect: improve check for plink to reduce false positives

2015-04-26 Thread brian m. carlson
On Sat, Apr 25, 2015 at 06:03:22PM +0200, Torsten Bögershausen wrote:
 (I'm not sute if the commit message describes the problem deep enough
 for readers which are not familar with all the details of the original
 report):
 A feature implemented for Windows may break things for e.g. Linux users)
 The following may read exaggerated, so please read it as a suggestion.
 
 The git_connect function has code to handle plink and tortoiseplink
 specially, as they require different command line arguments compared to
 OpenSSH (-P instead of -p, tortoiseplink uses -batch), commit 36ad53ffee6ed.
 The special handling is only needed for Windows, and a sloppy
 case-insensitve search for plink will trigger that an the extra
 parameter -batch is added to the command line.
 
 This was observed on a Linux system where a command line including
 /uplink_deploy/ was used.
 
 There are different ways to improve the situation:
 (The following mentions only plink, but assumes that tortoiseplink is handled
  similar)
 a) Disable the plink/tortoiseplink special handling on non-Windows systems
 b) Tighten the search for plink:
Allow basename() == plink || !!strcasestr(ssh, plink.exe)
 c) Tighten the search for plink:
Allow basename() == plink || !!strcasestr(ssh, plink.exe)
 d) Tighten the check for tortoiseplink.
Today we set int putty to true when plink is found, and -batch
is set when tortoiseplink is not found.
This fixes the reported bug, but still has the -P problem.
 e) Unix users typically use shell scripts and could use GIT_SSH_COMMAND.
Declare the GIT_SSH as not-well-documented (and to obsolete ?) for 
 non-Windows systems,
 
 This patch implements c):
 Extract the basename and compare it to plink, plink.exe respective
 tortoiseplink/tortoiseplink.exe
 
 Note that there is a slight risk of breakage for Windows users:
 Strings like myplink or plink-0.83 are no longer accepted.

I can certainly expand the commit message when I reroll.

 -
 I would probably vote for a), as Unix/Linux/Mac OS users don't use 
 plink/tortoiseplink
 at all.

I have putty on my system:

vauxhall ok % uname -a  which plink
Linux vauxhall 3.16.0-4-amd64 #1 SMP Debian 3.16.7-ckt7-1 (2015-03-01) x86_64 
GNU/Linux
/usr/bin/plink

While it's clearly not very common to use putty on Unix systems, it
certainly is possible and it would need to work the same way.

 -
 What about adding test-cases in t5601,
 this will ease the documentation later.
 f:/util/plink
 /c/util/plink.exe
 f:/util/tortoiseplink
 /c/util/tortoiseplink.exe
 /usr/local/uplink/sshwrapper.sh

It looks like there's already a framework for that, so sure, I can add
some tests.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


Re: Question about how git determines the minimum packfile for a push.

2015-04-26 Thread Junio C Hamano
On Sun, Apr 26, 2015 at 5:41 PM, Brad Litterell b...@evidence.com wrote:

 Is it possible git is not computing the delta correctly?
 Or does git only look at the top-level commit objects to figure out what to
 include in the push packfile?

We walk the commit graph backwards to discover the common ancestries to
minimize the network cost when fetching, but I do not think the
reverse direction
has such smart in the protocol.

If you fetch (not pull) first to remote tracking branches and then push, that
probably will reduce the transfer, as the side that pushes is the only one that
decides what objects are sent in git push - git receive-pack direction.
--
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/RFC] blame: CRLF in the working tree and LF in the repo

2015-04-26 Thread Junio C Hamano
Torsten Bögershausen tbo...@web.de writes:

 Although the intention of 4d4813a5 is good, it breaks
 the usual EOL-handling for Windows.
 Until we have a better solution, we suggest to revert it.

That makes it sound like you are proposing to rob Peter to pay Paul,
but that is not how we do things around here.  If both the case
4d4813a5 tried to solve and the issue reported by Stepan need to be
satisfied, the current code will stay as-is until you can find a
good solution to make both happy.

Having said that.

I suspect (I haven't looked very carefully for this round yet to be
sure, though) that it may turn out that the commit you are proposing
to revert was a misguided attempt to fix a non issue, or to break
the behaviour to match a mistaken expectation.  If that is the case
then definitely the reversion is a good idea, and you should argue
along that line of justification.

We'd just be fixing an old misguided and bad change in such a case.

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/RFC] blame: CRLF in the working tree and LF in the repo

2015-04-26 Thread Stepan Kasal
Hello,

thank you Torsten for the patch [I'm the reporter, but could not do
it myself]

 -test_expect_success 'blaming files with CRLF newlines' '
 +test_expect_failure 'blaming files with CRLF newlines in repo, 
 core.autoclrf=input' '

Shouldn't the old test be rather removed?
It deals with an invalid situation.

I thought that having crlf in the repo is incorrect, so no wonder
that it fails if the files in the working tree are changed to LF.

And changing the autocrlf transformation is effectively the same,
no matter that the files _physically_ are the same as the files in
the repo.

Have a nice day,
Stepan Kasal
--
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