Re: [PATCH V2 1/2] t9801: check git-p4's branch detection and client view together

2015-04-21 Thread Vitor Antunes
On April 20, 2015 11:45:15 PM GMT+01:00, Junio C Hamano gits...@pobox.com 
wrote:
Vitor Antunes vitor@gmail.com writes:

 On April 20, 2015 6:43:49 AM GMT+01:00, Junio C Hamano
gits...@pobox.com wrote:
Vitor Antunes vitor@gmail.com writes:

 Add failing scenario where branch detection is enabled together
with
 use client view. In this specific scenario git-p4 will break when
the
 perforce client view removes part of the depot path.

I somehow cannot parse together with use client view, especially
the word use.  Is it user client view or something (I am not
familiar with p4 lingo), or perhaps use of client view?

 I meant spec instead of view. As in - -use-client-spec.

 In perforce you need to configure your workspace using a client
specification.
 One of the configured values is the client view, which maps
files/folders in the
 server to locations in your local workspace. What I'm trying to fix
with these
 patches is the ability of git-p4 to process the client view
definition through
 the use of p4 where to determine the correct location of the local
files, such
 that it is able to apply the necessary patches for submission to the
perforce
 server.

 While thinking about client views I completely forgot that the git-p4
argument
 that enables thos feature uses spec and not view.

So,... what's the conclusion?  Should the log message be written
like this perhaps?

t9801: check git-p4's branch detection and client spec together

Add failing scenario where branch detection is enabled together
with use of client spec.  In this specific scenario git-p4 will
break when the perforce client spec removes part of the depot
path.

The test case also includes an extra sub-file mapping to enforce
robustness check on git-p4 implementation.

Signed-off-by: Vitor Antunes vitor@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com

Unfortunately at the moment I have limited computer access at
home. I will, obviously, update the descriptions as soon as
possible.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [BUG] git pull will regress between 'master' and 'pu'

2015-04-21 Thread Johannes Schindelin
Hi Junio,

On 2015-04-20 21:28, Junio C Hamano wrote:
 Junio C Hamano gits...@pobox.com writes:
 
 This is primarily note-to-self; even though I haven't got around
 bisecting yet, I think I know I did some bad change myself.

 git pull $URL $tag seems to:

  * fail to invoke the editor without --edit.
  * show the summary merge log message twice.
 
 We seem to be making a good progress on the discussion on this
 specific issue, but a larger tangent of this is that we do not seem
 to have test coverage to catch this regression.  As we are planning
 to rewrite git pull, we need to make sure we have enough test
 coverage to ensure that nothing will regress before doing so.

Yes, the plan is to use code coverage tools to ensure that a decent amount of 
code paths/parameters is covered.

Ciao,
Dscho
--
To unsubscribe from this list: send the line 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] refs.c: enable large transactions

2015-04-21 Thread Junio C Hamano
Stefan Beller sbel...@google.com writes:

 This replaces the latest patch on origin/sb/remove-fd-from-ref-lock

Thanks, will replace.
--
To unsubscribe from this list: send the line 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] refs.c: enable large transactions

2015-04-21 Thread Junio C Hamano
Stefan Beller sbel...@google.com writes:

 I thought about putting a cap on it to not let it go negative in the first
 place, but I did not find an easily accessible max() function, as I'd like
 to write it as

 int remaining_fds = max(get_max_fd_limit() - 32, 0);

 to have it in one line. The alternative of

 int remaining_fds = get_max_fd_limit() - 32;
 ...
 if (remaining_fds  0)
 remaining_fds = 0

 seemed to cuttered to me.

Just to set the standard yardstick straight, either is fine, but I
would say the latter is slightly preferrable from readability's
point of view.  Rows of dense single lines get tiring to read pretty
quickly.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCHv2] refs.c: enable large transactions

2015-04-21 Thread Stefan Beller
This is another attempt on enabling large transactions
(large in terms of open file descriptors). We keep track of how many
lock files are opened by the ref_transaction_commit function.
When more than a reasonable amount of files is open, we close
the file descriptors to make sure the transaction can continue.

Another idea I had during implementing this was to move this file
closing into the lock file API, such that only a certain amount of
lock files can be open at any given point in time and we'd be 'garbage
collecting' open fds when necessary in any relevant call to the lock
file API. This would have brought the advantage of having such
functionality available in other users of the lock file API as well.
The downside however is the over complication, you really need to always
check for (lock-fd != -1) all the time, which may slow down other parts
of the code, which did not ask for such a feature.

Signed-off-by: Stefan Beller sbel...@google.com
---

* Removed unneeded braces in the condition to check if we want to close
  the lock file.
* made the counter for the remaining fds an unsigned int. That is what   
  get_max_fd_limit() returns, so there are no concerns for an overflow.
  Also it cannot go below 0 any more.
* moved the initialisation of the remaining_fds a bit down and added a comment  
  
 refs.c| 21 +
 t/t1400-update-ref.sh |  4 ++--
 2 files changed, 23 insertions(+), 2 deletions(-)
 
 

diff --git a/refs.c b/refs.c
index 4f495bd..34cfcdf 100644
--- a/refs.c
+++ b/refs.c
@@ -3041,6 +3041,8 @@ static int write_ref_sha1(struct ref_lock *lock,
errno = EINVAL;
return -1;
}
+   if (lock-lk-fd == -1)
+   reopen_lock_file(lock-lk);
if (write_in_full(lock-lk-fd, sha1_to_hex(sha1), 40) != 40 ||
write_in_full(lock-lk-fd, term, 1) != 1 ||
close_ref(lock)  0) {
@@ -3718,6 +3720,7 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
   struct strbuf *err)
 {
int ret = 0, i;
+   unsigned int remaining_fds;
int n = transaction-nr;
struct ref_update **updates = transaction-updates;
struct string_list refs_to_delete = STRING_LIST_INIT_NODUP;
@@ -3733,6 +3736,20 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
return 0;
}
 
+   /*
+* We need to open many files in a large transaction, so come up with
+* a reasonable maximum. We still keep some spares for stdin/out and
+* other open files. Experiments determined we need more fds when
+* running inside our test suite than directly in the shell. It's
+* unclear where these fds come from. 32 should be a reasonable large
+* number though.
+*/
+   remaining_fds = get_max_fd_limit();
+   if (remaining_fds  32)
+   remaining_fds -= 32;
+   else
+   remaining_fds = 0;
+
/* Copy, sort, and reject duplicate refs */
qsort(updates, n, sizeof(*updates), ref_update_compare);
if (ref_update_reject_duplicates(updates, n, err)) {
@@ -3762,6 +3779,10 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
update-refname);
goto cleanup;
}
+   if (remaining_fds  0)
+   remaining_fds--;
+   else
+   close_lock_file(update-lock-lk);
}
 
/* Perform updates first so live commits remain referenced */
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index 7a69f1a..636d3a1 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -1071,7 +1071,7 @@ run_with_limited_open_files () {
 
 test_lazy_prereq ULIMIT_FILE_DESCRIPTORS 'run_with_limited_open_files true'
 
-test_expect_failure ULIMIT_FILE_DESCRIPTORS 'large transaction creating 
branches does not burst open file limit' '
+test_expect_success ULIMIT_FILE_DESCRIPTORS 'large transaction creating 
branches does not burst open file limit' '
 (
for i in $(test_seq 33)
do
@@ -1082,7 +1082,7 @@ test_expect_failure ULIMIT_FILE_DESCRIPTORS 'large 
transaction creating branches
 )
 '
 
-test_expect_failure ULIMIT_FILE_DESCRIPTORS 'large transaction deleting 
branches does not burst open file limit' '
+test_expect_success ULIMIT_FILE_DESCRIPTORS 'large transaction deleting 
branches does not burst open file limit' '
 (
for i in $(test_seq 33)
do
-- 
2.4.0.rc2.5.g4c2045b.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


Regular Rebase Failure

2015-04-21 Thread Adam

About two weeks ago I started getting a regular rebase failure. I get this
error several times a day and at least once a day I lose work to it.
---
fatal: Unable to create '/Users/asteel/path/to/repo/.git/index.lock': File
exists.

If no other git process is currently running, this probably means a

git process crashed in this repository earlier. Make sure no other git

process is running and remove the file manually to continue.

Could not apply 71a...
---
The weird part is that the file does not exist

Any ideas? 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] refs.c: enable large transactions

2015-04-21 Thread Stefan Beller
On Tue, Apr 21, 2015 at 10:16 AM, Junio C Hamano gits...@pobox.com wrote:
 Stefan Beller sbel...@google.com writes:

 + /*
 +  * We may want to open many files in a large transaction, so come up 
 with
 +  * a reasonable maximum, keep some spares for stdin/out and other open
 +  * files.
 +  */
 + int remaining_fds = get_max_fd_limit() - 32;

 Can this go negative?  If it does so, does it matter?  I think the

Yes it can go negative. It doesn't matter as we'd then run into the
close_lock_file(update-lock-lk); case below.

I thought about putting a cap on it to not let it go negative in the first
place, but I did not find an easily accessible max() function, as I'd like
to write it as

int remaining_fds = max(get_max_fd_limit() - 32, 0);

to have it in one line. The alternative of

int remaining_fds = get_max_fd_limit() - 32;
...
if (remaining_fds  0)
remaining_fds = 0

seemed to cuttered to me. Yet another alternative

 int remaining_fds = get_max_fd_limit() - 32  0 ? 0 :
get_max_fd_limit() - 32;

is also not appealing as we'd have to call get_max_fd_limit 2 times.
That's why I thought going negative is not as bad, but have readable
code instead.

As you think about the possibility of remaining_fds being negative,
this might not
be the best idea though


 code doesn't barf, but starting from a negative remaining feels
 cryptic, compared to starting from a zero remaining.

   struct ref_update **updates = transaction-updates;
   struct string_list refs_to_delete = STRING_LIST_INIT_NODUP;
   struct string_list_item *ref_to_delete;
 @@ -3762,6 +3770,11 @@ int ref_transaction_commit(struct ref_transaction 
 *transaction,
   update-refname);
   goto cleanup;
   }
 + if (remaining_fds  0) {
 + remaining_fds--;
 + } else {
 + close_lock_file(update-lock-lk);
 + }

 Any plan to add more code to these blocks in future updates?

I'll remove the braces. :(


 Thanks.

 diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
 index 7a69f1a..636d3a1 100755
 --- a/t/t1400-update-ref.sh
 +++ b/t/t1400-update-ref.sh
 @@ -1071,7 +1071,7 @@ run_with_limited_open_files () {

  test_lazy_prereq ULIMIT_FILE_DESCRIPTORS 'run_with_limited_open_files true'

 -test_expect_failure ULIMIT_FILE_DESCRIPTORS 'large transaction creating 
 branches does not burst open file limit' '
 +test_expect_success ULIMIT_FILE_DESCRIPTORS 'large transaction creating 
 branches does not burst open file limit' '
  (
   for i in $(test_seq 33)
   do
 @@ -1082,7 +1082,7 @@ test_expect_failure ULIMIT_FILE_DESCRIPTORS 'large 
 transaction creating branches
  )
  '

 -test_expect_failure ULIMIT_FILE_DESCRIPTORS 'large transaction deleting 
 branches does not burst open file limit' '
 +test_expect_success ULIMIT_FILE_DESCRIPTORS 'large transaction deleting 
 branches does not burst open file limit' '
  (
   for i in $(test_seq 33)
   do
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/3] Another approach to large transactions

2015-04-21 Thread Stefan Beller
On Tue, Apr 21, 2015 at 10:19 AM, Junio C Hamano gits...@pobox.com wrote:
 Stefan Beller sbel...@google.com writes:

 On Mon, Apr 20, 2015 at 3:51 PM, Junio C Hamano gits...@pobox.com wrote:

 On the core management side, xmalloc() and friends retry upon
 failure, after attempting to free the resource.  I wonder if your
 codepath can do something similar to that, perhaps?

 But then we'd need to think about which fds can be 'garbage collected'.
 The lock files certainly can be closed and reopened.

 ... and that is the natural thing to garbage collect, no?  After
 all, this approach allows lock-file subsystem to keep fds open even
 when they do not absolutely have to, so they are the best candidates
 to be shot down first when things gets tight.

 A good thing is that you have a list of all them already for use by
 the atexit handler ;-)

Yes, but when such a garbage collection is in place, it is part of the API,
which means you need to check all places, where lock files are used,
so that you have the pattern

   open lock file
   ...
   if lock_file-lk == -1
   lock_file_reopen(...)
   use(lock_file-lk)

I think that could be easily integrated into the lock_file API when the API
users don't directly read the values of the lock file struct, but rather call
a method
int lock_file_get_fd(lock);

which guarantees to return a valid fd, but that is not long term stable. You
can use the fd for the next action, but it may become garbage collected again.

So then you don't know how long the fd is valid as the garbage collection
may be either triggered from the lock file code or by yourself?

There is some uncertainty on when which data is valid, which is why I dislike
this approach and rather prefer to make it explicit. If we run into such
a problem in another place, we can still think about a general solution in the
lock file API.
--
To unsubscribe from this list: send the line 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 v3 0/4] Improving performance of git clean

2015-04-21 Thread Junio C Hamano
erik elfström erik.elfst...@gmail.com writes:

 Ok, thanks for looking into this.

 I have no well founded opinions on the implementation but I do
 think the performance tests would be more meaningful if the
 setup/cleanup code could be removed from the timed section.
 If the community agrees on an implementation I would be happy
 to convert the new tests, either directly in this series or as a follow
 up if that is preferred.

Let's not delay the fix and do the perf thing as a follow-up series,
possibly an even independent one.

In other words, let's keep the topics small.


 /Erik

 On Tue, Apr 21, 2015 at 12:14 AM, Thomas Gummerer t.gumme...@gmail.com 
 wrote:
 On 04/18, Erik Elfström wrote:
 * Still have issues in the performance tests, see comments
   from Thomas Gummerer on v2

 I've looked at the modern style tests again, and I don't the code
 churn is worth it just for using them for the performance tests.  If
 anyone wants to take a look at the code, it's at
 github.com/tgummerer/git tg/perf-lib.

 I think adding the test_perf_setup_cleanup command would make more
 sense in this case.  If you want I can send a patch for that.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/3] Another approach to large transactions

2015-04-21 Thread Junio C Hamano
Stefan Beller sbel...@google.com writes:

 On Mon, Apr 20, 2015 at 3:51 PM, Junio C Hamano gits...@pobox.com wrote:

 On the core management side, xmalloc() and friends retry upon
 failure, after attempting to free the resource.  I wonder if your
 codepath can do something similar to that, perhaps?

 But then we'd need to think about which fds can be 'garbage collected'.
 The lock files certainly can be closed and reopened.

... and that is the natural thing to garbage collect, no?  After
all, this approach allows lock-file subsystem to keep fds open even
when they do not absolutely have to, so they are the best candidates
to be shot down first when things gets tight.

A good thing is that you have a list of all them already for use by
the atexit handler ;-)
--
To unsubscribe from this list: send the line 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: Regular Rebase Failure

2015-04-21 Thread Stefan Beller
[+mailing list]

On Tue, Apr 21, 2015 at 11:20 AM, Adam adamgst...@gmail.com wrote:
 I'm using git version 2.3.2 (Apple Git-55).

We should loop in the maintainers of the Apple Git version, they'd know
what changed in git about two weeks ago.
I have no idea who that is though.


 That explains why I can't find the index.lock file, since the error that was
 thrown deleted it. I'm still not sure what to do about this, though.

Complain at the right people so it gets fixed. ;)


 Thanks for responding.

 Adam

--
To unsubscribe from this list: send the line 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] refs.c: enable large transactions

2015-04-21 Thread Junio C Hamano
Stefan Beller sbel...@google.com writes:

 + /*
 +  * We may want to open many files in a large transaction, so come up 
 with
 +  * a reasonable maximum, keep some spares for stdin/out and other open
 +  * files.
 +  */
 + int remaining_fds = get_max_fd_limit() - 32;

Can this go negative?  If it does so, does it matter?  I think the
code doesn't barf, but starting from a negative remaining feels
cryptic, compared to starting from a zero remaining.

   struct ref_update **updates = transaction-updates;
   struct string_list refs_to_delete = STRING_LIST_INIT_NODUP;
   struct string_list_item *ref_to_delete;
 @@ -3762,6 +3770,11 @@ int ref_transaction_commit(struct ref_transaction 
 *transaction,
   update-refname);
   goto cleanup;
   }
 + if (remaining_fds  0) {
 + remaining_fds--;
 + } else {
 + close_lock_file(update-lock-lk);
 + }

Any plan to add more code to these blocks in future updates?

Thanks.

 diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
 index 7a69f1a..636d3a1 100755
 --- a/t/t1400-update-ref.sh
 +++ b/t/t1400-update-ref.sh
 @@ -1071,7 +1071,7 @@ run_with_limited_open_files () {
  
  test_lazy_prereq ULIMIT_FILE_DESCRIPTORS 'run_with_limited_open_files true'
  
 -test_expect_failure ULIMIT_FILE_DESCRIPTORS 'large transaction creating 
 branches does not burst open file limit' '
 +test_expect_success ULIMIT_FILE_DESCRIPTORS 'large transaction creating 
 branches does not burst open file limit' '
  (
   for i in $(test_seq 33)
   do
 @@ -1082,7 +1082,7 @@ test_expect_failure ULIMIT_FILE_DESCRIPTORS 'large 
 transaction creating branches
  )
  '
  
 -test_expect_failure ULIMIT_FILE_DESCRIPTORS 'large transaction deleting 
 branches does not burst open file limit' '
 +test_expect_success ULIMIT_FILE_DESCRIPTORS 'large transaction deleting 
 branches does not burst open file limit' '
  (
   for i in $(test_seq 33)
   do
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [BUG] Git does not convert CRLF=LF on files with \r not before \n

2015-04-21 Thread Junio C Hamano
Alexandre Garnier zig...@gmail.com writes:

 echo '*   text=auto'  .gitattributes
 git add .gitattributes
 git commit -q -m Normalize EOL
 echo -ne 'some content\r\nother \rcontent with CR\r\ncontent\r\nagain

With text=auto, the user instructs us to guess, and we expect either
LF or CRLF line-terminated files that is *TEXT*.  A lone CR in the
middle of the line would mean we cannot reliably guess---it may be
LF terminated file with CRs sprinkled inside text, some of which
happen to be at the end of the line, or it may be CRLF terminated
file with CRs sprinkled in.  We try to preserve the user input by
not munging when we are not sure.

You are seeing the designed and intended behaviour.

But it would be a bug if the same thing happens when the user
explicitly tells us that the file has CRLF line endings, and I
suspect we have that bug, which may want to be corrected.

I've Cc'ed various people who worked on convert.c around line
endings.  I recall we saw a few other discussion threads on
text=auto and eol settings.  Stakeholders may want to have a unified
discussion to first list the issues in the current implementation
and come up with fixes for them.

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: [BUG] Git does not convert CRLF=LF on files with \r not before \n

2015-04-21 Thread Torsten Bögershausen
On 2015-04-21 15.51, Alexandre Garnier wrote:
 Here is a test:
 
 git init -q crlf-test
 cd crlf-test
 echo '*   text=auto'  .gitattributes
 git add .gitattributes
 git commit -q -m Normalize EOL
 echo -ne 'some content\r\nother \rcontent with CR\r\ncontent\r\nagain
 content with\r\r\n'  inline-cr.txt
 echo Working directory content:
 cat -A inline-cr.txt
 echo
 git add inline-cr.txt
 echo Indexed content:
 git show :inline-cr.txt | cat -A
 
 Result
 --
 File content:
 some content^M$
 other ^Mcontent with CR^M$
 content^M$
 again content with^M^M$
 
 Indexed content:
 some content^M$
 other ^Mcontent with CR^M$
 content^M$
 again content with^M^M$
 
 Expected result
 ---
 File content:
 some content^M$
 other ^Mcontent with CR^M$
 content^M$
 again content with^M^M$
 
 Indexed content:
 some content$
 other ^Mcontent with CR$
 content$
 again content with^M$
 # or even 'again content with$' for this last line
 
 If you remove the \r that are not at the end of the lines, EOL are
 converted as expected:
 File content:
 some content^M$
 other content with CR^M$
 content^M$
 again content with^M$
 
 Indexed content:
 some content$
 other content with CR$
 content$
 again content with$
 

First of all, thanks for the info.

The current implementation of Git does an auto-detection
if a file is text or binary.

For a file which is suspected to be text, it is expected to have either LF or 
CRLF as
line endings, but a bare CR make Git wonder:
Should this still be treated as a text file ?
If yes, should the CR be kept as is, or should it be converted into LF (or 
CRLF) ?

The current implementation may simply be explained by the fact that nobody has 
so far asked 
to treat this file as text, so the implementation assumes it to be binary.

(Which makes the code a little bit easier, at the time it was written)

So the status of today is that you can force Git to let the CR as is,
when you specify that the file is text.

Is there a real life problem behind it ?
And what should happen to the CRs ?





--
To unsubscribe from this list: send the line 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] squash! bash-completion: add support for git-log --merges= and log.merges

2015-04-21 Thread Junio C Hamano
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: Regular Rebase Failure

2015-04-21 Thread Stefan Beller
On Tue, Apr 21, 2015 at 10:16 AM, Adam adamgst...@gmail.com wrote:
 About two weeks ago I started getting a regular rebase failure. I get this
 error several times a day and at least once a day I lose work to it.

Which git version are you using?

 ---
 fatal: Unable to create '/Users/asteel/path/to/repo/.git/index.lock': File
 exists.

Searching through the code base there are only 3 occurrences of
Unable to create matching the capitalization of the letters. (we
have many more error messages where the U is lower case)

The first occurrence is in refs.c create_symref
if (rename(lockpath, git_HEAD)  0) {
error(Unable to create %s, git_HEAD);
goto error_unlink_return;
}

The other 2 occurrences are in the lockfile.c so any place using
lock files may trigger that error.


 If no other git process is currently running, this probably means a

 git process crashed in this repository earlier. Make sure no other git

 process is running and remove the file manually to continue.

 Could not apply 71a...
 ---
 The weird part is that the file does not exist

Not anymore when you're looking I guess, as git creates and deletes a lock file
for the index quite frequently. Also when a git command shuts down roughly
as expected (such as when calling a die(error message) as opposed to being
terminated by the operating system), then the exit handler makes sure to remove
all the lock files it held.


 Any ideas? 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
--
To unsubscribe from this list: send the line 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 v3 0/4] Improving performance of git clean

2015-04-21 Thread erik elfström
On Sun, Apr 19, 2015 at 3:14 AM, Junio C Hamano gits...@pobox.com wrote:
 Erik Elfström erik.elfst...@gmail.com writes:

 Known Problems:
 * Unsure about the setup.c:read_gitfile refactor, feels a bit
   messy?

 The interface indeed feels somewhat messy.  I suspect that a better
 interface might be more like setup_git_directory_gently() that is a
 gentler version of setup_git_directory().  The function takes an
 optional pointer to an int, and it behaves not-gently-at-all when
 the optional argument is NULL.  The location the optional pointer
 points at, when it is not NULL, can be used to return the error
 state to the caller.  That function pair only uses the optional
 argument to convey one bit of information (i.e. are we in any git
 repository or not?) back to the caller, but the interface could be
 used to tell the caller a lot more if we wanted to.

 If you model read_gitfile_gently() after that pattern, I would
 expect that

  - The extra pattern would be int *error;
  - The implementation of read_gitfile() would be

#define read_gitfile(path) read_gitfile_gently((path), NULL)

and the _gently() version will die when error parameter is set
to NULL and finds any error.

  - The caller of the gentle variant can use the error code to
determine what went wrong, not just the fact that it failed.  I
do not think your caller does not have an immediate need to tell
between invalid gitfile format and No path in gitfile, but
such an interface leaves that possibility open.

 Thanks.


Ok, I'll try that approach, thanks. (and apologies for the late reply)

/Erik
--
To unsubscribe from this list: send the line 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 v3 0/4] Improving performance of git clean

2015-04-21 Thread erik elfström
Ok, thanks for looking into this.

I have no well founded opinions on the implementation but I do
think the performance tests would be more meaningful if the
setup/cleanup code could be removed from the timed section.
If the community agrees on an implementation I would be happy
to convert the new tests, either directly in this series or as a follow
up if that is preferred.

/Erik

On Tue, Apr 21, 2015 at 12:14 AM, Thomas Gummerer t.gumme...@gmail.com wrote:
 On 04/18, Erik Elfström wrote:
 * Still have issues in the performance tests, see comments
   from Thomas Gummerer on v2

 I've looked at the modern style tests again, and I don't the code
 churn is worth it just for using them for the performance tests.  If
 anyone wants to take a look at the code, it's at
 github.com/tgummerer/git tg/perf-lib.

 I think adding the test_perf_setup_cleanup command would make more
 sense in this case.  If you want I can send a patch for that.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v8 2/4] cat-file: teach cat-file a '--literally' option

2015-04-21 Thread Eric Sunshine
On Tue, Apr 21, 2015 at 6:16 AM, Charles Bailey char...@hashpling.org wrote:
 On Mon, Apr 20, 2015 at 09:22:47PM +0530, karthik nayak wrote:
 On 04/20/2015 02:49 PM, Charles Bailey wrote:
 As far as I could tell - and please correct me if I've misunderstood,
 cat-file's literally is about dealing with unrecognized types whereas
 hash-object's --literally is about both creating objects with bad types
 and invalid objects of recognized types. This latter scenario is where
 the option name literally makes the most sense.
 Yes. What you're saying is correct, but it also makes sense as we're asking
 cat-file to give us information about the object irrespective of the type 
 of the
 object, hence asking it to literally print the information. Also it stays as 
 a compliment
 to hash-object --literally, which is already existing.

 OK, I think you've hit the main point which I was trying to make.

 To me, literally means without transformation or exactly as
 written/recorded/transmitted (which -t/-s do anyway) and doesn't really
 encompass the irrespective of type meaning that it has been given here.

 In any case, I've made my point so I won't labour it any further. I
 think that --no-validation or --allow-any-type might be more accurate
 but if everyone else is happy enough with --literally then I'm happy to
 live with that too.

It's easy to be blinded into thinking that cat-file's new option
should be named --literally since it was inspired by the --literally
option of hash-object, but indeed it may not be the best choice.

In addition to your above suggestions (and --unchecked which you
proposed earlier), if we take inspiration from existing Git options,
perhaps one of the following (or something derived from them) would be
better?

--force
--ignore-errors
--no-check
--unsafe
--no-strict
--aggressive

Or, some pure made-up bike-shedding?

--try-harder
--allow-bogus-type
--ignore-bogus-type
--loose
--gently
--
To unsubscribe from this list: send the line 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] Documentation/git-merge.txt: improve SYNOPSIS

2015-04-21 Thread sorganov
From: Sergey Organov sorga...@gmail.com

Get rid of outdated explicit list of options.

Reflect that the obsolete form:

'git merge' msg HEAD commit...

could also have options.

Signed-off-by: Sergey Organov sorga...@gmail.com
---
 Documentation/git-merge.txt | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index 1f94908..1de61a4 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -9,10 +9,8 @@ git-merge - Join two or more development histories together
 SYNOPSIS
 
 [verse]
-'git merge' [-n] [--stat] [--no-commit] [--squash] [--[no-]edit]
-   [-s strategy] [-X strategy-option] [-S[key-id]]
-   [--[no-]rerere-autoupdate] [-m msg] [commit...]
-'git merge' msg HEAD commit...
+'git merge' [options] [-m msg] [commit...]
+'git merge' [options] msg HEAD commit...
 'git merge' --abort
 
 DESCRIPTION
-- 
1.9.3

--
To unsubscribe from this list: send the line 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 v8 1/4] sha1_file.c: support reading from a loose object of unknown type

2015-04-21 Thread karthik nayak



On 04/21/2015 12:21 AM, Jeff King wrote:

On Tue, Apr 21, 2015 at 12:13:30AM +0530, karthik nayak wrote:


+static int unpack_sha1_header_to_strbuf(git_zstream *stream, unsigned char
*map,
+   unsigned long mapsize, void *buffer,
+   unsigned long bufsiz, struct strbuf
*header)
+{
+   unsigned char *cp;
+   int status;
+   int i = 0;
+
+   status = unpack_sha1_header(stream, map, mapsize, buffer, bufsiz);


I wonder if we would feel comfortable just running this NUL-check as
part of unpack_sha1_header (i.e., in all code paths). It _shouldn't_
trigger in normal use, but I wonder if there would be any downsides
(e.g., maliciously crafted objects getting us to allocate memory or
something; I think it is fairly easy to convince git to allocate memory,
though).


But why would we want it to be a part of unpack_sha1_header?


+   for (cp = buffer; cp  stream-next_out; cp++)
+   if (!*cp) {
+   /* Found the NUL at the end of the header */
+   return 0;
+   }


I think we can spell this as:

   if (memchr(buffer, '\0', stream-next_out - buffer))
return 0;

which is shorter and possibly more efficient.

Noted. Thanks :)


In theory we could also just start trying to parse the type/size header,
and notice there when we don't find the NUL. That's probably not worth
doing, though. The parsing is separated from the unpacking here, so it
would require combining those two operations in a single function. And
the extra NUL search here is likely not very expensive.

Yes, even I though about doing that, but wasn't keen on combining those 
two functions, they're meant to do two different things.

-Peff


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


[PATCH] squash! bash-completion: add support for git-log --merges= and log.merges

2015-04-21 Thread SZEDER Gábor
---
The patch at the tip of kk/log-merges-config misindented one of the case
arms, this one amends it.

 contrib/completion/git-completion.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index a75d7f5..83b2359 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1454,9 +1454,9 @@ _git_log ()
--merges=*)
__gitcomp show hide only  ${cur##--merges=}
return
;;
-   --*)
+   --*)
__gitcomp 
$__git_log_common_options
$__git_log_shortlog_options
$__git_log_gitk_options
-- 
1.9.5.msysgit.0

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


Re: git-p4 Question

2015-04-21 Thread FusionX86
Thanks Sam, I'll check it out.

On Mon, Apr 20, 2015 at 12:23 PM, Sam Vilain s...@vilain.net wrote:
 On 04/20/2015 09:41 AM, FusionX86 wrote:

 Hopefully this is an appropriate place to ask questions about git-p4.

 I started at a company that wants to migrate from Perforce to Git. I'm
 new to Perforce and have been trying to learn just enough about it to
 get through this migration.


 You might also like to check out my git-p4raw project which imports directly
 from the raw repository files into a git repo using git fast-import

 http://github.com/samv/git-p4raw

 Apparently it's my most popular github project :-).  YMMV.

 Sam.
--
To unsubscribe from this list: send the line 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: Why does git log -Gregex works with regexp-ignore-case but not with other regexp-related options?

2015-04-21 Thread Michael J Gruber
Junio C Hamano venit, vidit, dixit 20.04.2015 20:44:
 Linus Torvalds torva...@linux-foundation.org writes:
 
 And to clarify: I don't suggest always building with libpcre. I
 literally suggest having something like

  /* hacky mac-hack hack */
 if (strncmp((?i), p-pattern, 4)) {
 p-pattern += 4;
 p-ignore_case = true;
 }

 just in front of the regcomp() call, and nothing more fancy than that.
 
 Yeah, looking at the way grep.c:compile_regexp() is structured, we
 are already prepared to allow
 
 $ git log --grep='(?i)torvalds' --grep='Linus'
 
 that wants to find one piece of text case insensitively while
 another case sensitively in the same text (i.e. the log message
 part), so per-pattern customization may be a good way to do this.
 

And '(?f)foo' switches to fixed strings ;)

We have engine-switching options and engine-modification options. The
latter are certainly good in the expression itself. Maybe even the
former, though I don't know how to switch away from fixed-strings in
that way...

I had forgotten about pcre. Maybe switching options independently is so
unusual that use pcre is good enough as a solution to suggest to those
few users?

In any case, that leaves us with:

- resolve the existing inconsistencies around --regexp-ignore-case
- allow to switch the engine for all greppy operations

Maybe have all command line options apply to all greppy ops as a first
step, which allows pcre and thus '(?i)' for all fields?

Michael
--
To unsubscribe from this list: send the line 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 v8 2/4] cat-file: teach cat-file a '--literally' option

2015-04-21 Thread Charles Bailey
On Mon, Apr 20, 2015 at 09:22:47PM +0530, karthik nayak wrote:
 
 
 On 04/20/2015 02:49 PM, Charles Bailey wrote:
 As far as I could tell - and please correct me if I've misunderstood,
 cat-file's literally is about dealing with unrecognized types whereas
 hash-object's --literally is about both creating objects with bad types
 and invalid objects of recognized types. This latter scenario is where
 the option name literally makes the most sense.
 Yes. What you're saying is correct, but it also makes sense as we're asking
 cat-file to give us information about the object irrespective of the type 
 of the
 object, hence asking it to literally print the information. Also it stays as 
 a compliment
 to hash-object --literally, which is already existing.

OK, I think you've hit the main point which I was trying to make.

To me, literally means without transformation or exactly as
written/recorded/transmitted (which -t/-s do anyway) and doesn't really
encompass the irrespective of type meaning that it has been given here.

In any case, I've made my point so I won't labour it any further. I
think that --no-validation or --allow-any-type might be more accurate
but if everyone else is happy enough with --literally then I'm happy to
live with that too.
--
To unsubscribe from this list: send the line 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 v8 1/4] sha1_file.c: support reading from a loose object of unknown type

2015-04-21 Thread Jeff King
On Tue, Apr 21, 2015 at 04:56:08PM +0530, karthik nayak wrote:

 +   status = unpack_sha1_header(stream, map, mapsize, buffer, bufsiz);
 
 I wonder if we would feel comfortable just running this NUL-check as
 part of unpack_sha1_header (i.e., in all code paths). It _shouldn't_
 trigger in normal use, but I wonder if there would be any downsides
 (e.g., maliciously crafted objects getting us to allocate memory or
 something; I think it is fairly easy to convince git to allocate memory,
 though).
 
 But why would we want it to be a part of unpack_sha1_header?

Just to reduce the number of functions and the complexity of the caller.
But I guess it doesn't help that much if the caller would then need to
speculatively pass in a strbuf. So it's probably not a good idea.

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


Re: git-p4 Question

2015-04-21 Thread FusionX86
Hi Luke,

Using -v was a good suggestion. Unfortunately I still don't see what
the problem is. I'm starting to think that maybe I should just create
the client views I need and setup a cron job that p4 syncs and then
git commits/pushes.

The --preserve-user option is for submitting back to Perforce correct?
I'm hoping to get away with a one-way sync from Perforce to Git...and
then eventually just cut over to Git.

I also looked at git fusion, but unfortunately the version of Perforce
we're running (2012.1) doesn't meet the requirements for fusion. I
wish it did.

Good point on developer mindset. I think we definitely have some
training and habit changing in the future.

Thanks for the suggestions and pointers, it's much appreciated.

On Mon, Apr 20, 2015 at 1:26 PM, Luke Diamand l...@diamand.org wrote:
 On 20/04/15 17:41, FusionX86 wrote:

 Hello,

 Hopefully this is an appropriate place to ask questions about git-p4.

 I started at a company that wants to migrate from Perforce to Git. I'm
 new to Perforce and have been trying to learn just enough about it to
 get through this migration. Anyway, I've been playing with git-p4 and
 have one question/problem to discuss.

 After setting up the p4 cli client I can 'p4 sync' some
 //depot/main/app1 which pulls down the files I would expect from the
 Perforce server. If I use 'git p4 clone //depot/main/app1', I get:

 Doing initial import of //depot/main/app1/ from revision #head into
 refs/remotes/p4/master

 But I don't get any files from that depot/folder pulled down. I can
 git p4 clone other depot/folders though and get some files. I suspect
 that I'm just not understanding how the git-p4 module works.


 You could try doing the clone with '-v' to get a bit more information.


 Basically, I'm hoping to setup a live sync of Perforce to Git of
 certain depots in preparation for the migration. Also, if anyone has
 pointers or guides for this type of migration, any help is
 appreciated.


 I've done something similar in the past. You'll want to enable the
 --preserve-user option, for which you will need admin rights.

 If it's a one-way mirror (p4-to-git) then just run git-p4 periodically (if
 you use cron, then try to avoid having two or more instances running at the
 same time).

 If you want it to be two-way then it gets a bit more complicated.

 You might also want to consider using git fusion, which is Perforce's take
 on this problem. I've not used it myself.

 From past experience though I would say the biggest problem is getting
 developers to switch from the P4 mindset (centralized; code review hard to
 do or ignored) to the git mindset (decentralized; code review actively
 supported by the version control system).

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


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


[BUG] Git does not convert CRLF=LF on files with \r not before \n

2015-04-21 Thread Alexandre Garnier
Here is a test:

git init -q crlf-test
cd crlf-test
echo '*   text=auto'  .gitattributes
git add .gitattributes
git commit -q -m Normalize EOL
echo -ne 'some content\r\nother \rcontent with CR\r\ncontent\r\nagain
content with\r\r\n'  inline-cr.txt
echo Working directory content:
cat -A inline-cr.txt
echo
git add inline-cr.txt
echo Indexed content:
git show :inline-cr.txt | cat -A

Result
--
File content:
some content^M$
other ^Mcontent with CR^M$
content^M$
again content with^M^M$

Indexed content:
some content^M$
other ^Mcontent with CR^M$
content^M$
again content with^M^M$

Expected result
---
File content:
some content^M$
other ^Mcontent with CR^M$
content^M$
again content with^M^M$

Indexed content:
some content$
other ^Mcontent with CR$
content$
again content with^M$
# or even 'again content with$' for this last line

If you remove the \r that are not at the end of the lines, EOL are
converted as expected:
File content:
some content^M$
other content with CR^M$
content^M$
again content with^M$

Indexed content:
some content$
other content with CR$
content$
again content with$

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


Re: [PATCH 0/3] Another approach to large transactions

2015-04-21 Thread Michael Haggerty
On 04/21/2015 12:51 AM, Junio C Hamano wrote:
 Stefan Beller sbel...@google.com writes:
 
 The problem comes from guessing the number of fds we're allowed to use.
 At first I thought it was a fundamental issue with the code being broken, but
 it turns out we just need a larger offset as we apparently have 9 files open
 already, before the transaction even starts.
 I did not expect the number to be that high, which is why I came up with the
 arbitrary number of 8 (3 for stdin/out/err, maybe packed refs and reflog so I
 guessed, 8 would do fine).

 I am not sure if the 9 is a constant or if it scales to some unknown
 property yet.
 So to make the series work, all we need is:

 - int remaining_fds = get_max_fd_limit() - 8;
 + int remaining_fds = get_max_fd_limit() - 9;

 I am going to try to understand where the 9 comes from and resend the 
 patches.
 
 I have a suspicion that the above is an indication that the approach
 is fundamentally not sound.  9 may be OK in your test repository,
 but that may fail in a repository with different resource usage
 patterns.
 
 On the core management side, xmalloc() and friends retry upon
 failure, after attempting to free the resource.  I wonder if your
 codepath can do something similar to that, perhaps?
 
 On the other hand, it may be that this let's keep it open as long
 as possible, as creat-close-open-write-close is more expensive may
 not be worth the complexity.  I wonder if it might not be a bad idea
 to start with a simpler rule, e.g. use creat-write-close for ref
 updates outside transactions, and creat-close-open-write-close for
 inside transactions, as that is likely to be multi-ref updates or
 something stupid and simple like that?
 
 Michael?

Given that the release is so close, I think we should use the simplest
thing that could work, which I think is Stefan's original
N*(creat-close),N*(open-write-close),N*rename patch. I don't think there
are many code paths that might build up a big transaction anyway (I
guess only git update-ref --stdin and git push --atomic?) Neither of
these has been around very long, so I don't think the small performance
hit will bother anybody.

The correct solution is clearly N*(creat-write-close),N*rename, but that
is too complicated for this release. So let's get the bug fixed for the
release and try to get the better fix in the next release.

It would be possible to optimize the N=1 case (I guess it's by far the
most common case) really stupidly using something like

if (n  1)
close_lock_file(update-lock-lk);

but I doubt even that's worth it.

Michael

-- 
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: [BUG] Performance regression due to #33d4221: write_sha1_file: freshen existing objects

2015-04-21 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

 Stefan Saasen ssaa...@atlassian.com writes:

 I've noticed Peff's patches on pu which suggest they will be available
 in git 2.5?

 Being on 'pu' (or 'next' for that matter) is not a suggestion for a
 change to appear in any future version at all, even though it often
 means that it would soon be merged to 'master' and will be in the
 upcoming release to be on 'next' in early part of a development
 cycle.  Some larger topics would stay on 'next' for a few cycles.

 Do you Junio, have plans to merge them to maint (2.3.x) and/or next (2.4)?

 The topic will hopefully be merged to 'master' after 2.4 final is
 released end of this month, down to 'maint' early May and will ship
 with 2.4.1, unless there is unforeseen issues discovered in the
 change while people try it out while it is in 'next' (which will
 happen today, hopefully).

... and then if I do not forget and if the topic is really important
for real-world users, I am OK to merge it down to 2.3 and even 2.2
maintenance tracks later.  But that will happen only after the topic
hits 'maint', which will happen only after the topic hits 'master'.

What you _can_ help is the if I do not forget part ;-)  Also see a
similar discussion we had recently

  http://thread.gmane.org/gmane.comp.version-control.git/264365

The key sentence from my part in the thread is 

 When I say the tip of 'master' is meant to be more stable than
 any tagged versions, I do mean it.

and the reasoning behind it that is given in the paragraph before
that, though.

Perhaps companies like Atlassian that rely on the stability of the
open source Git can spare some resources and join forces with like
minded folks on LTS of older maintenance tracks, if they are truly
interested in.

--
To unsubscribe from this list: send the line 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's directory is _prepended_ to PATH when called with an absolute path

2015-04-21 Thread David Rodríguez

Thanks Junio for the useful link, I'll comment there.

On 21/04/15 18:48, Junio C Hamano wrote:

http://thread.gmane.org/gmane.comp.version-control.git/267143/focus=267251


--
To unsubscribe from this list: send the line 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] pathspec: adjust prefixlen after striping trailing slash

2015-04-21 Thread Junio C Hamano
Duy Nguyen pclo...@gmail.com writes:

 On Mon, Apr 20, 2015 at 12:37 PM, Junio C Hamano gits...@pobox.com wrote:
 Duy Nguyen pclo...@gmail.com writes:

 But if you look at it another way, cd subrepo; git add . should be
 the same as git add subrepo ...

 Once you cd into subrepo, you are in a different world, a world
 different from the toplevel project.  git add . over there should
 mean add everything in subproject's working tree to subproject's
 index, shouldn't it?  On the other hand, git add subrepo without
 leavingin the working tree of the superproject is about binding the
 submodule to the superproject's index.

 I do not think these two should be the same.  Where am I mistaken?

 I think I wrote this sentence and deleted it: I didn't say which way
 was right.

OK.

I looked at the test script update.  The new test does (I am
rephrasing to make it clearer):

mkdir -p dir/ectory
git init dir/ectory ;# a new directory inside top-level project
(
cd dir/ectory 
test  git add test  git commit -m test
)
git add dir/ectory

to set it up.  At this point, the top-level index knows dir/ectory
is a submodule.

Then the test goes on to turn it a non submodule by

mv dir/ectory/.git dir/ectory/dotgit

At this point, I think there are two valid things that can happen.

 (1) git add test inside dir/ectory removes the submodule entry
 dir/ectory and then adds dir/ectory/test as an individual
 entry.  After all that is what happens when you replace a file
 with a directory, e.g.

   folder  git add folder
   rm folder
   mkdir folder  folder/file 
   git -C folder add file

 will first register a regular file folder and then replace
 that with paths underneath.

 It would be the same if you did any of the following:

   git -C . add dir/ectory/test
   git -C dir add ectory/test
   git -C dir/ectory add test

 (2) git add test inside dir/ectory would barf, saying
 dir/ectory is supposed to be a submodule and you have to first
 remove it.  Again, it would be the same if you did so from any
 directory with relative paths.

I think (2) is less optimal than (1), but it could be the best we
could do if the submodule infrastracture around .gitmodules and
links to $GIT_DIR/modules/$name may have to get involved in an
operation like this (I didn't check).

In my quick test, it appeared that the behaviour with this patch
applied was neither of the two and instead to silently do nothing,
and if that is the case I think it is quite wrong.
--
To unsubscribe from this list: send the line 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 v8 2/4] cat-file: teach cat-file a '--literally' option

2015-04-21 Thread Junio C Hamano
Eric Sunshine sunsh...@sunshineco.com writes:

 It's easy to be blinded into thinking that cat-file's new option
 should be named --literally since it was inspired by the --literally
 option of hash-object, but indeed it may not be the best choice.

Yeah, I wouldn't even say inspired.  It was envisioned as a
counter-part debugging aid, nothing more.

Is there any other way to make cat-file looser other than accepting
an unknown type name from the future?  If so, then perhaps it may
make sense to give it a generic name that implys that we would
trigger such additional looseness in the future.  But as the
inventor of it as a debugging aid, I would say a name that spells
out the specific way this option is being loose, e.g.

 --allow-bogus-type

or with s/bogus/unknown/, best describes what it currently does.

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: Regular Rebase Failure

2015-04-21 Thread Jeff King
On Tue, Apr 21, 2015 at 11:34:37AM -0700, Stefan Beller wrote:

 [+mailing list]
 
 On Tue, Apr 21, 2015 at 11:20 AM, Adam adamgst...@gmail.com wrote:
  I'm using git version 2.3.2 (Apple Git-55).
 
 We should loop in the maintainers of the Apple Git version, they'd know
 what changed in git about two weeks ago.
 I have no idea who that is though.

You can get some releases from:

  http://www.opensource.apple.com/tarballs/Git/

and at least view the diff against stock git. But they do not seem to
have updated that in some time (the latest release is Apple Git-48,
based off of v1.8.5.2).

It would be nice if they kept that up to date (actually not just nice,
but required by the GPL[1]). It would be nicer still if they actually
published their patches with real commit messages, or better yet
actually engaged with upstream to work on fixes or enhancements.

This isn't strictly relevant to the discussion at hand, but in case any
Apple Git folks do see this, perhaps it will serve as a gentle nudge.

-Peff

[1] By the GPL, they don't strictly need to publish the source; they
just have to make an offer to provide it on request. But given the
existence of opensource.apple.com, I think their strategy is to just
publish everything, and they have simply let updating that site fall
behind the binaries they are pushing. Or maybe they have switched to
an alternate site that I don't know about.
--
To unsubscribe from this list: send the line 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 v3 0/4] Improving performance of git clean

2015-04-21 Thread Jeff King
On Tue, Apr 21, 2015 at 08:21:37PM +0200, erik elfström wrote:

 Ok, thanks for looking into this.
 
 I have no well founded opinions on the implementation but I do
 think the performance tests would be more meaningful if the
 setup/cleanup code could be removed from the timed section.
 If the community agrees on an implementation I would be happy
 to convert the new tests, either directly in this series or as a follow
 up if that is preferred.

If I understand correctly, the reason that you need per-run setup is
that your git clean command actually cleans things, and you need to
restore the original state for each time-trial. Can you instead use git
clean -n to do a dry-run? I think what you are timing is really the
figure out what to clean step, and not the cleaning itself.

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


[ANNOUNCE] Git v2.3.6

2015-04-21 Thread Junio C Hamano
The latest maintenance release Git v2.3.6 is now available at the
usual places.  It is comprised of 14 non-merge commits since v2.3.5,
contributed by 8 people, 4 of which are new faces.

There is not much to see there (the changes are mostly documentation
and test updates), except for updates to diff-highlight in contrib/.

The tarballs are found at:

https://www.kernel.org/pub/software/scm/git/

The following public repositories all have a copy of the 'v2.3.6'
tag and the 'maint' branch that the tag points at:

  url = https://kernel.googlesource.com/pub/scm/git/git
  url = git://repo.or.cz/alt-git.git
  url = https://code.google.com/p/git-core/
  url = git://git.sourceforge.jp/gitroot/git-core/git.git
  url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core
  url = https://github.com/gitster/git

New contributors whose contributions weren't in v2.3.5 are as follows.
Welcome to the Git development community!

  Ivan Ukhov, Jérôme Zago, Julian Gindi, and Paul Tan.

Returning contributors who helped this release are as follows.
Thanks for your continued support.

  Jeff King, Jonathan Nieder, Junio C Hamano, and Kyle J. McKay.



Git v2.3.6 Release Notes


Fixes since v2.3.5
--

 * diff-highlight (in contrib/) used to show byte-by-byte
   differences, which meant that multi-byte characters can be chopped
   in the middle.  It learned to pay attention to character boundaries
   (assuming the UTF-8 payload).

Also contains typofixes, documentation updates and trivial code
clean-ups.



Changes since v2.3.5 are as follows:

Ivan Ukhov (1):
  parse-options.h: OPTION_{BIT,SET_INT} do not store pointer to defval

Jeff King (7):
  t: translate SIGINT to an exit
  t: redirect stderr GIT_TRACE to descriptor 4
  t: pass GIT_TRACE through Apache
  t5541: move run_with_cmdline_limit to test-lib.sh
  t5551: make EXPENSIVE test cheaper
  cherry-pick: fix docs describing handling of empty commits
  howto: document more tools for recovery corruption

Jonathan Nieder (1):
  fast-import doc: remove suggested 16-parent limit

Julian Gindi (1):
  CodingGuidelines: update 'rough' rule count

Junio C Hamano (1):
  Git 2.3.6

Jérôme Zago (1):
  gitweb.conf.txt: say build-time, not built-time

Kyle J. McKay (1):
  diff-highlight: do not split multibyte characters

Paul Tan (1):
  enter_repo(): fix docs to match code

--
To unsubscribe from this list: send the line 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: [PATCHv2] refs.c: enable large transactions

2015-04-21 Thread Stefan Beller

 * Removed unneeded braces in the condition to check if we want to close
   the lock file.
 * made the counter for the remaining fds an unsigned int. That is what
   get_max_fd_limit() returns, so there are no concerns for an overflow.
   Also it cannot go below 0 any more.
 * moved the initialisation of the remaining_fds a bit down and added a comment

* Once again this replaces the last patch on top of
origin/sb/remove-fd-from-ref-lock
--
To unsubscribe from this list: send the line 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's directory is _prepended_ to PATH when called with an absolute path

2015-04-21 Thread Junio C Hamano
http://thread.gmane.org/gmane.comp.version-control.git/267143/focus=267251
--
To unsubscribe from this list: send the line 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: How do I resolve conflict after popping stash without adding the file to index?

2015-04-21 Thread Dmitry Gutov

On 04/22/2015 12:29 AM, Jeff King wrote:


Hmm, interestingly, if you do _not_ stage the changes (i.e., drop the
final git add there), you get:

   $ git stash pop
   error: Your local changes to the following files would be overwritten by 
merge:
test
   Please, commit your changes or stash them before you can merge.
   Aborting

which makes sense. Writing conflict markers into the file would leave
you in a situation where it is hard to recover the b content.


Indeed.


But we seem to skip that safety valve when the content has been staged,
which seems questionable to me (technically we are slightly better off
than the protected case because b was written to a git blob
object, so you can recover it.  But it may be difficult to find the
correct blob in the object database).


Any suggestions how to restore that content in the index 
programmatically? If it's non-trivial to do, maybe that is indeed a bug, 
and 'git stash pop' should abort before creating the conflict.

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


git's directory is _prepended_ to PATH when called with an absolute path

2015-04-21 Thread David Rodríguez

For example, if I run git using:

$ /usr/bin/git

then /usr/bin is prepended to the path. Is this intentional? If it is, why?

This causes issues with Ruby git hooks, because Ruby version managers 
rely on custom settings in PATH to select the Ruby executable, and 
there's usually a system Ruby living in /usr/bin. See 
https://github.com/github/hub/issues/855 for an example.


Thanks a lot,

David Rodríguez.
--
To unsubscribe from this list: send the line 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] builtin/merge.c: add the merge.verifysignatures config option

2015-04-21 Thread Bruno Vieira
Signed-off-by: Bruno Vieira m...@bmpvieira.com
---
This seemed to be missing. Sorry if otherwise or if I'm doing something wrong 
(first time contributing).

 builtin/merge.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/builtin/merge.c b/builtin/merge.c
index 3b0f8f9..5dbc10f 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -598,6 +598,9 @@ static int git_merge_config(const char *k, const char *v, 
void *cb)
} else if (!strcmp(k, merge.defaulttoupstream)) {
default_to_upstream = git_config_bool(k, v);
return 0;
+   } else if (!strcmp(k, merge.verifysignatures)) {
+   verify_signatures = git_config_bool(k, v);
+   return 0;
} else if (!strcmp(k, commit.gpgsign)) {
sign_commit = git_config_bool(k, v) ?  : NULL;
return 0;
-- 
2.3.2 (Apple Git-55)

--
To unsubscribe from this list: send the line 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: PATH modifications for git-hook processes

2015-04-21 Thread David Rodríguez
Jeff King peff at peff.net writes:
 
 If we can get away with just dropping this element from the PATH, I'd
 much rather do that than try to implement a complicated path-precedence
 scheme.
 
 -Peff
 

I agree, GIT_EXEC_DIR should be enough and this surprising behavior would be
avoided.

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


Re: [BUG] Performance regression due to #33d4221: write_sha1_file: freshen existing objects

2015-04-21 Thread Stefan Saasen
 Perhaps companies like Atlassian that rely on the stability of the
 open source Git can spare some resources and join forces with like
 minded folks on LTS of older maintenance tracks, if they are truly
 interested in.

We certainly can and would like to. I'm not entirely sure what that
would entail though?

From reading through $gmane/264365 I've identified the following
responsibilities/opportunities to help:

- Monitor git log --first-parent maint-lts..master and find
  the tip of topic branches that need to be down-merged;

- Down-merge such topics to maint-lts; this might involve
  cherry-picking instead of merge, as the bugfix topics may
  originally be done on the codebase newer than maint-lts;

and more importantly testing the maint-lts version to ensure
backported changes don't introduce regressions and the maint-lts
branch is stable.

This suggests specific, spaced LTS versions but in the same thread you
mention maint-2.1or maint-2.2.
So a different model could be maintaining old versions in a sliding
window fashion (e.g. critical issues would be backported to the last 6
months worth of git releases).

Maybe I'm getting ahead of myself here :)

Anyway, long story short. We're interested to help but I'm not
entirely sure what that would look like at the moment. Are there
formed ideas floating around or would you be looking for some form of
proposal instead?

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


[PATCH 2/3] git-multimail: Add a quiet option to prevent outputting email addresses from hook

2015-04-21 Thread Dave Boutcher
We have a very log list of recipients...displaying that list
on every push is annoying

Add an option to prevent printing the list of email addresses from
the hook

Signed-off-by: Dave Boutcher daveboutc...@gmail.com
---
 git-multimail/README   |  3 +++
 git-multimail/git_multimail.py | 11 ++-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/git-multimail/README b/git-multimail/README
index a40847a..49e0937 100644
--- a/git-multimail/README
+++ b/git-multimail/README
@@ -396,6 +396,9 @@ multimailhook.replyToRefchange
 - The value none, in which case the Reply-To: field will be
   omitted.
 
+multimailhook.quiet
+
+Do not output the list of email recipients from the hook
 
 Email filtering aids
 
diff --git a/git-multimail/git_multimail.py b/git-multimail/git_multimail.py
index 5ed253f..095110a 100755
--- a/git-multimail/git_multimail.py
+++ b/git-multimail/git_multimail.py
@@ -1582,6 +1582,9 @@ class Environment(object):
 commit mail.  The value should be a list of strings
 representing words to be passed to the command.
 
+quiet (bool)
+On success do not write to stderr
+
 
 
 REPO_NAME_RE = re.compile(r'^(?Pname.+?)(?:\.git)$')
@@ -1594,6 +1597,7 @@ class Environment(object):
 self.logopts = []
 self.refchange_showlog = False
 self.commitlogopts = ['-C', '--stat', '-p', '--cc']
+self.quiet = False
 
 self.COMPUTED_KEYS = [
 'administrator',
@@ -1745,6 +1749,10 @@ class 
ConfigOptionsEnvironmentMixin(ConfigEnvironmentMixin):
 'refchangeshowlog', default=self.refchange_showlog
 )
 
+self.quiet = config.get_bool(
+'quiet', default=False
+)
+
 maxcommitemails = config.get('maxcommitemails')
 if maxcommitemails is not None:
 try:
@@ -2376,7 +2384,8 @@ class Push(object):
 % (change.refname, change.old.sha1, change.new.sha1,)
 )
 else:
-sys.stderr.write('Sending notification emails to: %s\n' % 
(change.recipients,))
+if not self.environment.quiet:
+sys.stderr.write('Sending notification emails to: %s\n' % 
(change.recipients,))
 extra_values = {'send_date': send_date.next()}
 mailer.send(
 change.generate_email(self, body_filter, extra_values),
-- 
2.3.0

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


[PATCH 3/3] git-multimail: Add stdout option to config

2015-04-21 Thread Dave Boutcher
Add a stdout option to the config that is equivalent to the --stdout
command line option. This allows for easier debugging for a hook

Signed-off-by: Dave Boutcher daveboutc...@gmail.com
---
 git-multimail/README   |  5 +
 git-multimail/git_multimail.py | 10 +-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/git-multimail/README b/git-multimail/README
index 49e0937..0235b83 100644
--- a/git-multimail/README
+++ b/git-multimail/README
@@ -400,6 +400,11 @@ multimailhook.quiet
 
 Do not output the list of email recipients from the hook
 
+multimailhook.stdout
+
+For debugging, send emails to stdout rather than to the
+mailer.  Equivalent to the --stdout command line option
+
 Email filtering aids
 
 
diff --git a/git-multimail/git_multimail.py b/git-multimail/git_multimail.py
index 095110a..35a1140 100755
--- a/git-multimail/git_multimail.py
+++ b/git-multimail/git_multimail.py
@@ -1585,6 +1585,9 @@ class Environment(object):
 quiet (bool)
 On success do not write to stderr
 
+stdout (bool)
+Write email to stdout rather than emailing. Useful for debugging
+
 
 
 REPO_NAME_RE = re.compile(r'^(?Pname.+?)(?:\.git)$')
@@ -1598,6 +1601,7 @@ class Environment(object):
 self.refchange_showlog = False
 self.commitlogopts = ['-C', '--stat', '-p', '--cc']
 self.quiet = False
+self.stdout = False
 
 self.COMPUTED_KEYS = [
 'administrator',
@@ -1753,6 +1757,10 @@ class 
ConfigOptionsEnvironmentMixin(ConfigEnvironmentMixin):
 'quiet', default=False
 )
 
+self.stdout = config.get_bool(
+'stdout', default=False
+)
+
 maxcommitemails = config.get('maxcommitemails')
 if maxcommitemails is not None:
 try:
@@ -2563,7 +2571,7 @@ def main(args):
 sys.stderr.write('%s : %r\n' % (k, v))
 sys.stderr.write('\n')
 
-if options.stdout:
+if options.stdout or environment.stdout:
 mailer = OutputMailer(sys.stdout)
 else:
 mailer = choose_mailer(config, environment)
-- 
2.3.0

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


[PATCH 1/3] git-multimail: Add an option to filter on branches

2015-04-21 Thread Dave Boutcher
Add a branches option to the config.  Only changes
pushed to specified branches will generate emails. Changes to tags
will continue to generate emails.

Signed-off-by: Dave Boutcher daveboutc...@gmail.com
---
 git-multimail/README   |  7 +++
 git-multimail/git_multimail.py | 44 ++
 2 files changed, 43 insertions(+), 8 deletions(-)

diff --git a/git-multimail/README b/git-multimail/README
index 51add52..a40847a 100644
--- a/git-multimail/README
+++ b/git-multimail/README
@@ -170,6 +170,13 @@ multimailhook.repoName
 for gitolite repositories, or otherwise to derive this value from
 the repository path name.
 
+multimailhook.branches
+
+A comma separated list of branches to monitor. If not set,
+notifications will be sent for updates to any branch.  Branch
+names can contain regular expressions, and this configuration
+option can be multivalued.
+
 multimailhook.mailingList
 
 The list of email addresses to which notification emails should be
diff --git a/git-multimail/git_multimail.py b/git-multimail/git_multimail.py
index 4374907..5ed253f 100755
--- a/git-multimail/git_multimail.py
+++ b/git-multimail/git_multimail.py
@@ -1548,6 +1548,12 @@ class Environment(object):
 get_reply_to_commit() is used for individual commit
 emails.
 
+get_branches()
+
+Return a list of branches to be handled as a list of regex
+patterns.  If empty list, all branches are handled.  Branches
+can contain regular expressions (such as foo/.*)
+
 They should also define the following attributes:
 
 announce_show_shortlog (bool)
@@ -1619,6 +1625,9 @@ class Environment(object):
 def get_pusher_email(self):
 return None
 
+def get_branches(self):
+return []
+
 def get_administrator(self):
 return 'the administrator of this repository'
 
@@ -1809,6 +1818,10 @@ class 
ConfigOptionsEnvironmentMixin(ConfigEnvironmentMixin):
 else:
 return self.get_sender()
 
+def get_branches(self):
+branches = self.config.get_all('branches',[])
+return [re.compile('refs/heads/'+b.strip()) for bs in branches for b 
in bs.split(',')]
+
 def get_reply_to_refchange(self, refchange):
 if self.__reply_to_refchange is None:
 return super(ConfigOptionsEnvironmentMixin, 
self).get_reply_to_refchange(refchange)
@@ -2220,8 +2233,9 @@ class Push(object):
 ])
 )
 
-def __init__(self, changes):
+def __init__(self, changes, environment):
 self.changes = sorted(changes, key=self._sort_key)
+self.environment = environment
 
 # The SHA-1s of commits referred to by references unaffected
 # by this push:
@@ -2246,6 +2260,12 @@ class Push(object):
 def _sort_key(klass, change):
 return (klass.SORT_ORDER[change.__class__, change.change_type], 
change.refname,)
 
+def branches_match(self, branch):
+branches = self.environment.get_branches()
+if not branches:
+return [True]
+return [x for x in [r.match(branch) for r in branches] if x]
+
 def _compute_other_ref_sha1s(self):
 Return the GitObjects referred to by references unaffected by this 
push.
 
@@ -2264,6 +2284,10 @@ class Push(object):
 )
 for line in read_git_lines(['for-each-ref', '--format=%s' % (fmt,)]):
 (sha1, type, name) = line.split(' ', 2)
+# If we are using a branch filter, skip other branches
+if type == 'commit' and not self.branches_match(name):
+continue
+
 if sha1 and type == 'commit' and name not in updated_refs:
 sha1s.add(sha1)
 
@@ -2334,6 +2358,16 @@ class Push(object):
 unhandled_sha1s = set(self.get_new_commits())
 send_date = IncrementalDateTime()
 for change in self.changes:
+sha1s = []
+for sha1 in reversed(list(self.get_new_commits(change))):
+if sha1 in unhandled_sha1s:
+sha1s.append(sha1)
+unhandled_sha1s.remove(sha1)
+
+# if we are filtering on branches, skip branches we don't care 
about
+if change.refname_type == 'branch' and not 
self.branches_match(change.refname):
+continue
+
 # Check if we've got anyone to send to
 if not change.recipients:
 sys.stderr.write(
@@ -2349,12 +2383,6 @@ class Push(object):
 change.recipients,
 )
 
-sha1s = []
-for sha1 in reversed(list(self.get_new_commits(change))):
-if sha1 in unhandled_sha1s:
-sha1s.append(sha1)
-unhandled_sha1s.remove(sha1)
-
 max_emails = change.environment.maxcommitemails
 if max_emails and len(sha1s)  max_emails:
 

Re: git's directory is _prepended_ to PATH when called with an absolute path

2015-04-21 Thread David Rodríguez

I'm actually not sure how to reply to an old thread...

From the thread I gather that the intention is to prevent this behavior 
and stop prepending git's directory to the path. Is that right?


On 21/04/15 18:59, David Rodríguez wrote:

Thanks Junio for the useful link, I'll comment there.

On 21/04/15 18:48, Junio C Hamano wrote:
http://thread.gmane.org/gmane.comp.versionlist-control.git/267143/focus=267251 





--
To unsubscribe from this list: send the line 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: How do I resolve conflict after popping stash without adding the file to index?

2015-04-21 Thread Jeff King
On Wed, Apr 22, 2015 at 01:35:05AM +0300, Dmitry Gutov wrote:

 But we seem to skip that safety valve when the content has been staged,
 which seems questionable to me (technically we are slightly better off
 than the protected case because b was written to a git blob
 object, so you can recover it.  But it may be difficult to find the
 correct blob in the object database).
 
 Any suggestions how to restore that content in the index programmatically?
 If it's non-trivial to do, maybe that is indeed a bug, and 'git stash pop'
 should abort before creating the conflict.

Right, I am suggesting that latter: that stash should abort if the index
has modified entries. The abort for modified working tree files is done
by git-merge, which can be selective about which entries will be changed
(since it knows which ones need written).  I haven't thought hard enough
to say whether it should be doing the same for the index (i.e., whether
this is a merge problem or a stash problem).

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


[PATCH V3 1/2] t9801: check git-p4's branch detection with client spec enabled

2015-04-21 Thread Vitor Antunes
Add failing scenario when branch detection (--detect-branches) is
enabled together with use client spec (--use-client-spec). In this
specific scenario git-p4 will break when the Perforce client view
removes part of the depot path, as in the following example:

  //depot/branch1/base/... //client/branch1/...

The test case also includes an extra sub-file mapping to enforce
robustness check of git-p4's client view support:

  //depot/branch1/base/dir/sub_file1 //client/branch1/sub_file1

Signed-off-by: Vitor Antunes vitor@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
 t/t9801-git-p4-branch.sh |  106 ++
 1 file changed, 106 insertions(+)

diff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh
index 2bf142d..36a7f51 100755
--- a/t/t9801-git-p4-branch.sh
+++ b/t/t9801-git-p4-branch.sh
@@ -504,6 +504,112 @@ test_expect_success 'use-client-spec detect-branches 
skips files in branches' '
)
 '
 
+test_expect_success 'restart p4d' '
+   kill_p4d 
+   start_p4d
+'
+
+#
+# 1: //depot/branch1/base/file1
+#//depot/branch1/base/file2
+#//depot/branch1/base/dir/sub_file1
+# 2: integrate //depot/branch1/base/... - //depot/branch2/base/...
+# 3: //depot/branch1/base/file3
+# 4: //depot/branch1/base/file2 (edit)
+# 5: integrate //depot/branch1/base/... - //depot/branch3/base/...
+#
+# Note: the client view removes the base folder from the workspace
+#   and moves sub_file1 one level up.
+test_expect_success 'add simple p4 branches with common base folder on each 
branch' '
+   (
+   cd $cli 
+   client_view //depot/branch1/base/... //client/branch1/... \
+   //depot/branch1/base/dir/sub_file1 
//client/branch1/sub_file1 \
+   //depot/branch2/base/... //client/branch2/... \
+   //depot/branch3/base/... //client/branch3/... 
+   mkdir -p branch1 
+   cd branch1 
+   echo file1 file1 
+   echo file2 file2 
+   mkdir dir 
+   echo sub_file1 sub_file1 
+   p4 add file1 file2 sub_file1 
+   p4 submit -d Create branch1 
+   p4 integrate //depot/branch1/base/... //depot/branch2/base/... 

+   p4 submit -d Integrate branch2 from branch1 
+   echo file3 file3 
+   p4 add file3 
+   p4 submit -d add file3 in branch1 
+   p4 open file2 
+   echo update file2 
+   p4 submit -d update file2 in branch1 
+   p4 integrate //depot/branch1/base/... //depot/branch3/base/... 

+   p4 submit -d Integrate branch3 from branch1
+   )
+'
+
+# Configure branches through git-config and clone them.
+# All files are tested to make sure branches were cloned correctly.
+# Finally, make an update to branch1 on P4 side to check if it is imported
+# correctly by git p4.
+# git p4 is expected to use the client view to also not include the common
+# base folder in the imported directory structure.
+test_expect_success 'git p4 clone simple branches with base folder on server 
side' '
+   test_create_repo $git 
+   (
+   cd $git 
+   git config git-p4.branchList branch1:branch2 
+   git config --add git-p4.branchList branch1:branch3 
+   git p4 clone --dest=. --use-client-spec  --detect-branches 
//depot@all 
+   git log --all --graph --decorate --stat 
+   git reset --hard p4/depot/branch1 
+   test -f file1 
+   test -f file2 
+   test -f file3 
+   test -f sub_file1 
+   grep update file2 
+   git reset --hard p4/depot/branch2 
+   test -f file1 
+   test -f file2 
+   test ! -f file3 
+   test -f sub_file1 
+   ! grep update file2 
+   git reset --hard p4/depot/branch3 
+   test -f file1 
+   test -f file2 
+   test -f file3 
+   test -f sub_file1 
+   grep update file2 
+   cd $cli 
+   cd branch1 
+   p4 edit file2 
+   echo file2_ file2 
+   p4 submit -d update file2 in branch1 
+   cd $git 
+   git reset --hard p4/depot/branch1 
+   git p4 rebase 
+   grep file2_ file2
+   )
+'
+
+# Now update a file in one of the branches in git and submit to P4
+test_expect_failure 'Update a file in git side and submit to P4 using client 
view' '
+   test_when_finished cleanup_git 
+   (
+   cd $git 
+   git reset --hard p4/depot/branch1 
+   echo client spec  file1 
+   git add -u . 
+   git commit -m update file1 in branch1 
+   git config git-p4.skipSubmitEdit true 
+   

Re: [PATCH 7/9] strbuf_getwholeline: use getdelim if it is available

2015-04-21 Thread Jeff King
On Fri, Apr 17, 2015 at 06:16:48AM -0400, Eric Sunshine wrote:

  If somebody has a FreeBSD or OS X system to test on, I'd
  love to see what is needed to compile with HAVE_GETDELIM
  there.
 
 Modern Mac OS X, 10.10.x Yosemite, has getdelim() and git builds fine
 with HAVE_GETDELIM. I also tested on old Snow Leopard 10.5.8 from
 2009. It does not have getdelim(). Unfortunately, I haven't been able
 to determine when getdelim() was introduced on the Mac OS X, thus have
 been unable to craft a simple rule for config.mak.uname.

Thanks for looking into it. Since there haven't been any other takers in
the meantime, do you want to prepare a patch that checks $(uname_R) for
10.10.x?  That's likely more conservative than is necessary, but we can
loosen it later if somebody on 10.9.x shows up with test results.

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


Re: [PATCH 0/3] Another approach to large transactions

2015-04-21 Thread Jeff King
On Mon, Apr 20, 2015 at 05:31:11PM -0700, Stefan Beller wrote:

 When running the test locally, i.e. not in the test suite, but typing
 the commands
 myself into the shell, Git is fine with having just 5 file descriptors left.
 The additional 4 required fds come from beign run inside the test suite.
 
 When strace-ing git, I cannot see any possible other fds which would require
 having some left over space required. So I'd propose we'd just take a 
 reasonable
 number not too small for various test setups like 32 and then go with the
 proposed patches.

FWIW, we already use a magic value of 25 extra in open_packed_git_1. I
don't know if that means the number has been proven in practice, or if
it is simply that nobody actually exercises the pack_max_fds code. I
suspect it is the latter, especially since d131b7a (sha1_file.c: Don't
retain open fds on small packs, 2011-03-02).

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


[PATCH V3 0/2] git-p4: improve client path detection when branches are used

2015-04-21 Thread Vitor Antunes
The updates introduced in the third revision of these two patches consist only
on updates to the commit messages to better clarify what they implement.

Vitor Antunes (2):
  t9801: check git-p4's branch detection with client spec enabled
  git-p4: improve client path detection when branches are used

 git-p4.py|   13 --
 t/t9801-git-p4-branch.sh |  106 ++
 2 files changed, 115 insertions(+), 4 deletions(-)

-- 
1.7.10.4

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


[PATCH V3 2/2] git-p4: improve client path detection when branches are used

2015-04-21 Thread Vitor Antunes
Perforce allows client side file/directory remapping through
the use of the client view definition that is part of the
user's client spec.

To support this functionality while branch detection is
enabled it is important to determine the branch location in
the workspace such that the correct files are patched before
Perforce submission. Perforce provides a command that
facilitates this process: p4 where.

This patch does two things to fix improve file location
detection when git-p4 has branch detection and use of client
spec enabled:

 1. Enable usage of p4 where when Perforce branches exist
in the git repository, even when client specification is
used. This makes use of the already existing function
p4Where.

 2. Allow identifying partial matches of the branch's depot
path while processing the output of p4 where. For
robustness, paths will only match if ending in /

Signed-off-by: Vitor Antunes vitor@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
 git-p4.py|   13 +
 t/t9801-git-p4-branch.sh |2 +-
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index 549022e..34e4fdd 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -502,12 +502,14 @@ def p4Cmd(cmd):
 def p4Where(depotPath):
 if not depotPath.endswith(/):
 depotPath += /
-depotPath = depotPath + ...
-outputList = p4CmdList([where, depotPath])
+depotPathLong = depotPath + ...
+outputList = p4CmdList([where, depotPathLong])
 output = None
 for entry in outputList:
 if depotFile in entry:
-if entry[depotFile] == depotPath:
+# Search for the base client side depot path, as long as it starts 
with the branch's P4 path.
+# The base path always ends with /
+if entry[depotFile].find(depotPath) == 0 and 
entry[depotFile][-4:] == /...:
 output = entry
 break
 elif data in entry:
@@ -1627,7 +1629,10 @@ class P4Submit(Command, P4UserMap):
 if self.useClientSpec:
 self.clientSpecDirs = getClientSpec()
 
-if self.useClientSpec:
+# Check for the existance of P4 branches
+branchesDetected = (len(p4BranchesInGit().keys())  1)
+
+if self.useClientSpec and not branchesDetected:
 # all files are relative to the client spec
 self.clientPath = getClientRoot()
 else:
diff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh
index 36a7f51..0aafd03 100755
--- a/t/t9801-git-p4-branch.sh
+++ b/t/t9801-git-p4-branch.sh
@@ -593,7 +593,7 @@ test_expect_success 'git p4 clone simple branches with base 
folder on server sid
 '
 
 # Now update a file in one of the branches in git and submit to P4
-test_expect_failure 'Update a file in git side and submit to P4 using client 
view' '
+test_expect_success 'Update a file in git side and submit to P4 using client 
view' '
test_when_finished cleanup_git 
(
cd $git 
-- 
1.7.10.4

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


[PATCH] RFC/Add documentation for version protocol 2

2015-04-21 Thread Stefan Beller
This adds the design document for protocol version 2.
It's better to rewrite the design document instead of trying to
squash it into the existing pack-protocol.txt and then differentiating
between version 1 and 2 all the time.

Signed-off-by: Stefan Beller sbel...@google.com
---
  
 As we discussed at Git Merge in Paris, I'd just start out implementing the new
 protocol to deliver something you can play around with. Unfortunately I
 did not come up with an implementation straight away, but I think we should be
 coming to a consensus on the rough design at first. If there are no objections
 in the design I'll go for implementation.

 Documentation/technical/pack-protocol-2.txt | 88 +
 1 file changed, 88 insertions(+)
 create mode 100644 Documentation/technical/pack-protocol-2.txt

diff --git a/Documentation/technical/pack-protocol-2.txt 
b/Documentation/technical/pack-protocol-2.txt
new file mode 100644
index 000..36ddf3e
--- /dev/null
+++ b/Documentation/technical/pack-protocol-2.txt
@@ -0,0 +1,88 @@
+Packfile transfer protocols version 2
+=
+
+This document describes an updated protocol to transfer packs over ssh://,
+git:// and file:// links. All three transports (ssh, git, file) use the same
+protocol to transfer data. This document describes the version 2 of the pack
+file protocol, which is incompatible with the previous pack protocol.
+
+The http:// transport is not yet thought about in this phase of the protocol
+design.
+
+As this protocol is introduced rather late in the game, just after Gits 10th
+anniversary, a client SHOULD NOT assume a server speaks protocol version 2
+unless the server advertised protocol in a prior exchange.
+
+General structure
+=
+
+There are four phases involved in the protocol, which are described below:
+
+1) capability negotiation
+2) goal annoncement
+3) reference advertisement
+4) pack transfer
+
+
+1) Capability negotiation
+-
+
+In this phase both client and server send their capabilities to the other side
+using the following protocol:
+
+---
+list-of-capabilities = *capability flush-pkt
+capability   =  PKT-LINE(1*(LC_ALPHA / DIGIT / - / _))
+
+
+The capabilities itself are described in protocol-capabilities.txt
+Sending the capabilities to the other side MAY happen concurrently or
+one after another. There is no order who sends first.
+
+Note for developers:
+The amount of data SHOULD be kept very small. Future extensions to the protocol
+SHOULD only add a capability flag to this phase, adding possible data
+transfers into later phases. This ensures the protocol is extendable over
+time without having to spent to send huge chunks of data in the first phase.
+If both sides agree on a certain feature being used, it is easy to introduce 
more
+phases at any convenieant point after the phase 1 is finished.
+
+Notes as a design rationale:
+I thought about caching
+https://www.ll.mit.edu/HPEC/agendas/proc04/invited/patterson_keynote.pdf
+
+2) Goal annoncement
+---
+
+The goal of this phase is for the client to tell the server what
+outcome it expects from this communication, such as pushing or
+pulling data from the server.
+
+---
+list-of-goals= *goal
+goal = PKT-LINE(action-line)
+action-line  = action *(SP action-parameter)
+action   = noop / ls-remote / fetch / push / fetch-shallow
+action-parameter = parameter-key *(= parameter-value)
+parameter-key= 1*(LC_ALPHA / DIGIT / - / _)
+---
+
+You MAY specify multiple goals such as fetch and push or fetch-shallow.
+You MAY also specify the same goal multiple times with different parameters.
+You MUST omit goals which are part of other goals, such as ls-remote being part
+of fetch.
+
+The action parameter is dependant on the action itself. For now only fetch and 
push
+take the parameter mode, whose only allowed value is version1.
+
+Note:
+The parameters should follow a key=value pattern, where the value can consist 
of
+arbitrary characters. Having such a pattern would allow us to easily add a new
+capability for narrow clones (e.g. 
fetch-narrow=Documentation/*,.git*,.mailmap
+to fetch only the Documentation and .gitignore/attributes)
+
+3) Ref advertisement
+
+3) and 4) are highly dependant on the mode for fetch and push. As currently
+only mode version1 is supported, the these phases follow the ref 
advertisement
+in pack protocol version 1 without capabilities on the first line of the refs.
-- 
2.4.0.rc2.5.g4c2045b.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: [BUG] Performance regression due to #33d4221: write_sha1_file: freshen existing objects

2015-04-21 Thread Stefan Saasen
 I've noticed Peff's patches on pu which suggest they will be available
 in git 2.5?

 Being on 'pu' (or 'next' for that matter) is not a suggestion for a
 change to appear in any future version at all, even though it often
 means that it would soon be merged to 'master' and will be in the
 upcoming release to be on 'next' in early part of a development
 cycle.  Some larger topics would stay on 'next' for a few cycles.

 Do you Junio, have plans to merge them to maint (2.3.x) and/or next (2.4)?

 The topic will hopefully be merged to 'master' after 2.4 final is
 released end of this month, down to 'maint' early May and will ship
 with 2.4.1, unless there is unforeseen issues discovered in the
 change while people try it out while it is in 'next' (which will
 happen today, hopefully).

Thanks for the clarification Junio.
--
To unsubscribe from this list: send the line 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-21 Thread Luke Diamand
Can you post up the output from 'git p4 clone', and also see what the
output from doing this is:

$ p4 print //depot/some/branch/missingfile.c



On 21 April 2015 at 14:33, FusionX86 fusion...@gmail.com wrote:
 Hi Luke,

 Using -v was a good suggestion. Unfortunately I still don't see what
 the problem is. I'm starting to think that maybe I should just create
 the client views I need and setup a cron job that p4 syncs and then
 git commits/pushes.

 The --preserve-user option is for submitting back to Perforce correct?
 I'm hoping to get away with a one-way sync from Perforce to Git...and
 then eventually just cut over to Git.

 I also looked at git fusion, but unfortunately the version of Perforce
 we're running (2012.1) doesn't meet the requirements for fusion. I
 wish it did.

 Good point on developer mindset. I think we definitely have some
 training and habit changing in the future.

 Thanks for the suggestions and pointers, it's much appreciated.

 On Mon, Apr 20, 2015 at 1:26 PM, Luke Diamand l...@diamand.org wrote:
 On 20/04/15 17:41, FusionX86 wrote:

 Hello,

 Hopefully this is an appropriate place to ask questions about git-p4.

 I started at a company that wants to migrate from Perforce to Git. I'm
 new to Perforce and have been trying to learn just enough about it to
 get through this migration. Anyway, I've been playing with git-p4 and
 have one question/problem to discuss.

 After setting up the p4 cli client I can 'p4 sync' some
 //depot/main/app1 which pulls down the files I would expect from the
 Perforce server. If I use 'git p4 clone //depot/main/app1', I get:

 Doing initial import of //depot/main/app1/ from revision #head into
 refs/remotes/p4/master

 But I don't get any files from that depot/folder pulled down. I can
 git p4 clone other depot/folders though and get some files. I suspect
 that I'm just not understanding how the git-p4 module works.


 You could try doing the clone with '-v' to get a bit more information.


 Basically, I'm hoping to setup a live sync of Perforce to Git of
 certain depots in preparation for the migration. Also, if anyone has
 pointers or guides for this type of migration, any help is
 appreciated.


 I've done something similar in the past. You'll want to enable the
 --preserve-user option, for which you will need admin rights.

 If it's a one-way mirror (p4-to-git) then just run git-p4 periodically (if
 you use cron, then try to avoid having two or more instances running at the
 same time).

 If you want it to be two-way then it gets a bit more complicated.

 You might also want to consider using git fusion, which is Perforce's take
 on this problem. I've not used it myself.

 From past experience though I would say the biggest problem is getting
 developers to switch from the P4 mindset (centralized; code review hard to
 do or ignored) to the git mindset (decentralized; code review actively
 supported by the version control system).

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


--
To unsubscribe from this list: send the line 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: Why does git log -Gregex works with regexp-ignore-case but not with other regexp-related options?

2015-04-21 Thread Junio C Hamano
Michael J Gruber g...@drmicha.warpmail.net writes:

 We have engine-switching options and engine-modification options. The
 latter are certainly good in the expression itself. Maybe even the
 former, though I don't know how to switch away from fixed-strings in
 that way...

I do not think mixing matching engines in a single request makes
much sense. As the internal machinery is not even prepared to do
that, even though it is prepared to apply engine-modifications ones
to each grep term AFAIK, let's not go there.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [BUG] Performance regression due to #33d4221: write_sha1_file: freshen existing objects

2015-04-21 Thread Junio C Hamano
Stefan Saasen ssaa...@atlassian.com writes:

 I've noticed Peff's patches on pu which suggest they will be available
 in git 2.5?

Being on 'pu' (or 'next' for that matter) is not a suggestion for a
change to appear in any future version at all, even though it often
means that it would soon be merged to 'master' and will be in the
upcoming release to be on 'next' in early part of a development
cycle.  Some larger topics would stay on 'next' for a few cycles.

 Do you Junio, have plans to merge them to maint (2.3.x) and/or next (2.4)?

The topic will hopefully be merged to 'master' after 2.4 final is
released end of this month, down to 'maint' early May and will ship
with 2.4.1, unless there is unforeseen issues discovered in the
change while people try it out while it is in 'next' (which will
happen today, hopefully).
--
To unsubscribe from this list: send the line 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: How do I resolve conflict after popping stash without adding the file to index?

2015-04-21 Thread Jeff King
On Tue, Apr 21, 2015 at 01:54:56AM +0300, Dmitry Gutov wrote:

 I'm not really sure what higher stage entries are, but this scenario seems
 to be a counter-example:
 
 git init
 echo a  test
 git add test
 git commit -m first
 echo aaa  test
 git stash save
 echo b  test
 git add test
 git stash pop
 
 Either that, or 'git stash pop' was a destructive operation, and ate the
 staged changes.

Hmm, interestingly, if you do _not_ stage the changes (i.e., drop the
final git add there), you get:

  $ git stash pop
  error: Your local changes to the following files would be overwritten by 
merge:
test
  Please, commit your changes or stash them before you can merge.
  Aborting

which makes sense. Writing conflict markers into the file would leave
you in a situation where it is hard to recover the b content.

But we seem to skip that safety valve when the content has been staged,
which seems questionable to me (technically we are slightly better off
than the protected case because b was written to a git blob
object, so you can recover it.  But it may be difficult to find the
correct blob in the object database).

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